Compare server manifests with the client for script execution - #27
Compare server manifests with the client for script execution#27Pelotrio wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dcd1c2f582
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var sources = TotalDebug.get().runtimeSources(); | ||
| return sources.withCurrentSources(() -> catalog.details(request.source())); |
There was a problem hiding this comment.
When an unmatched source is requested—and again for every later player even after Catalog.details has cached its bytes—this calls TotalDebug.runtimeSources() first. That routine runs RuntimeSourceMaterializer.prepare, fingerprinting every runtime input and revalidating materialized artifacts, so comparing N differing sources repeats an O(total runtime size) scan N times and can make the server-script handshake extremely slow on large modpacks. Retain the prepared runtime identity and lock with the catalog, or serve cached details without rebuilding the full runtime.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
| this.baseline = new Baseline(message.sessionId(), decoded); | ||
| currentGeneration = this.manifestGeneration; | ||
| } | ||
| prepareComparison(currentGeneration); |
There was a problem hiding this comment.
After a server baseline arrives, this synchronously enters prepareComparison, which fingerprints every client archive and enumerates all class entries on the same single-thread executor used by compile and submit. Any client-side compilation queued behind this work stalls until the entire local comparison scan finishes even though it does not require server compatibility; on a large modpack this can noticeably block script execution after every join, reconnect, or index rebind. Run the comparison I/O on a separate worker or otherwise keep client compilations from queuing behind it.
Useful? React with 👍 / 👎.
| if (session == null || session.player() != player || !session.id().equals(request.sessionId()) | ||
| || this.manifest == null) return; | ||
| MinecraftServer server = Objects.requireNonNull(player.getServer()); | ||
| this.manifest.thenApplyAsync(catalog -> { |
There was a problem hiding this comment.
Any connected client that has received its session ID can submit unlimited requests with arbitrary request IDs and source indexes because this path validates only the manifest session before scheduling work. A modified client can repeatedly request the same cached source, causing every request to split and enqueue up to 32 MiB of response data on the server thread, in addition to the runtime rescans above; this lets even a non-operator saturate server CPU, memory, and outbound bandwidth regardless of the script policy. Track the expected per-player comparison request or enforce a strict rate/duplicate limit before scheduling it.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
1 participant
The server sends Companion a manifest so it can identify differences from the client before compiling scripts for server execution.
This updates the existing manifest handshake to compare ordered archive hashes first and request class declarations only for differing archives. Companion keeps the client index as its compilation source and records incompatible classes locally. Comparison results are tied to the current server session and client inventory.
Validation
./gradlew.bat :evaluation:test :protocol:test :companion:test :mod:test -PtotaldebugUseMavenLocal=truesucceeded; all tasks were up to date. A live client/server handshake remains to be verified.Outstanding review findings
InnerClassesmetadata.