Repository navigation
Conversation
… JDK it holds ProjectTreeTest built the runtime index, the whole JDK, every time a project opened, and its await for the index came near its five-second limit on a loaded machine. Its mod archive now holds a class and its index is cached before the project opens, as cache-loading tests already do. Two waits wait for what they assert instead of a fixed time. The index read Companion's own JDK but recorded the game's Java home, and a runtime cache is keyed by the game's inventory alone: a Companion on another JVM could load an index of another JDK. The JDK source now names the one read, and a cache holding another is rebuilt. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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: a0ee9011ce
ℹ️ 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".
| if (!Files.isRegularFile(path) && !Files.isDirectory(path)) { | ||
| throw new IOException("Prepared runtime source is unavailable: " + path); | ||
| } | ||
| if (source.module().kind() == ModuleKind.JAVA_RUNTIME && !path.equals(javaHome())) { |
There was a problem hiding this comment.
When Companion is launched with a valid but non-normalized java.home such as /opt/jdk/. or a relative path, every generated Java-runtime Source stores an absolute normalized path, but this comparison uses the raw system-property path. The paths therefore compare unequal; cached indexes are rejected, and rebuilding also fails when IndexCache.write validates the newly written manifest. Return an absolute normalized path from javaHome() before comparing it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2751799: javaHome() returns the absolute, normalized path, as Source stores it. aJavaHomeWrittenWithRedundantPartsIsTheSameRuntime sets java.home to <home>/.; it fails without the fix.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2751799b4e
ℹ️ 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".
| if (source.module().kind() == ModuleKind.JAVA_RUNTIME && !path.equals(javaHome())) { | ||
| throw new IOException("The index holds the classes of another Java runtime: " + path); |
There was a problem hiding this comment.
When Companion uses a different JVM from the game—explicitly supported for IDE/offline launches—and that JVM is upgraded or replaced in place, the saved game inventory ID remains unchanged and this path equality still accepts the old cache. The loaded index then describes the previous JDK while RuntimeSnapshotBytecodeSource.readJdk reads classes from the current JRT filesystem, causing stale or missing search results and potentially mismatched class metadata. Persist and validate a Companion-JDK version/build fingerprint (for example, the runtime version plus the JDK release metadata), not just java.home.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 747455e: the manifest records java.runtime.version (version and build) beside the path, and an index written on another build is rebuilt. anIndexWrittenOnAnotherBuildOfTheJavaRuntimeIsNotUsed changes the version under the same path.
…e in place changes Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
CompanionRenderingDefaultsTest's probe did not start within 15 s while two CI runs shared the runner. The waits end when the process exits, so the limit costs nothing when it starts at once; a deadlock still fails. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
M2, first PR (plan:
docs/TEST_SUITE.md, local on 1.21.1, after Codex's review).What was wrong
ProjectTreeTestbuilt the whole runtime index, 27,837 JDK classes, every time a project opened. Its await for the index (withWindow:91) came near its 5 s limit, and on 2026-10-05 a full local run failed there twice. Its mod archive held onlypack.mcmeta, so the index had nothing but the JDK, and an index without classes is refused.assertRevealsslept 300 ms after every reveal, eight times in one case, though the reveal's future had already completed.InspectionReadsTestslept 500 ms before asserting a count it could wait for.What changes
ProjectTreeTest's mod archive holds one class, and its index is cached before the project opens withRuntimeTestSources.writeLocalCache, as the cache-loading tests already do. No production seam.java.homeof the JVM that read it.IndexCache.requireSourcePaths, which both loading paths use, refuses a cache holding another Java runtime, so it is rebuilt. Existing caches made by a Companion the game started keep loading.Measured
Companion's suite alone, on this machine:
ProjectTreeTestIn the three runs no
UiTestScope.awaittook more than 1 s (logged with temporary instrumentation, not committed). The index's own suites still index the whole JDK. About 50 s of suite time remain there, which is the plan's second PR.Tests
IndexCacheTest.anIndexOfAnotherJavaRuntimeIsNotUsedProjectTreeTest,InspectionReadsTest,LocalIndexTest,RuntimeIndexRecoveryTest,RuntimeIndexServiceTest,OfflineProjectIntegrationTest,LiveRuntimeStartupTest,RuntimeInstallationTest.🤖 Generated with Claude Code