Repository navigation
test: keep the suite runnable from Codex-managed worktrees and beside a live proxy #6307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # 260930 Local test stability in Codex-managed worktrees | ||
|
|
||
| Status: open. Loop session `01a0f144-fb4d-7c32-940c-8b8a99874ba2` (wp2, after #6304 closed wp1), branch `codex/test-guard-managed-worktree` from `origin/dev` `2405624f39`. | ||
|
|
||
| | Doc | Work-phase | Content | | ||
| |---|---|---| | ||
| | 010_diagnosis_and_plan.md | wp2 | Two host-dependent failure classes, their causes, the diff, verification | |
37 changes: 37 additions & 0 deletions
37
devlog/_plan/260930_local_test_stability/010_diagnosis_and_plan.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # 010 Diagnosis and diff-level plan | ||
|
|
||
| ## Symptoms (2026-09-30, while verifying #6304) | ||
|
|
||
| 1. `bun run test:changed` from the Codex-managed worktree `~/.codex/worktrees/77b7/opencodex`: 863 failures. A sample file (`tests/codex-integration/model-visibility-management-api.test.ts`) fails 23/23 on its own with `refusing to remove a path inside the real Codex home (~/.codex) from a test process: ".../tests/codex-integration/.tmp-model-visibility-management-<pid>" resolves there`. The same head cloned to `/private/tmp` drops to 56 failures. | ||
| 2. `tests/service/shutdown-launcher.test.ts` fails SIGINT/SIGTERM/SIGHUP at both `dev` `6538001e2f` and the #6304 head: `The proxy answered /healthz but did not inject Codex config ... Proxy already running on port 10100; requested a second instance on port <n>`. | ||
|
|
||
| ## Causes | ||
|
|
||
| 1. `protectedRemovalReason` (`src/lib/test-home-guard.ts`) refuses any removal target inside `~/.codex`. Forty test files keep their fixture directory next to the test (`join(import.meta.dir, ".tmp-...")`) and clean it through `removeTreeWithRetry`. When the checkout itself is a Codex-app worktree under `~/.codex/worktrees/`, every such cleanup is refused, although the directory is repository content the test created, not Codex state. | ||
| 2. The launcher test gives the child a fresh `OPENCODEX_HOME` with no `config.json`. `handleStart` probes the configured port (`findProxyOwnerBeforeJournalRecovery({ probeConfiguredPort: true })`), which defaults to 10100. On a developer machine running ocx there, the child finds that live owner, takes the sibling path, and by design never injects Codex config, so the test's precondition never arrives. CI has no proxy on 10100, which is why it only fails locally. | ||
|
|
||
| ## Diff | ||
|
|
||
| - `src/lib/test-home-guard.ts`: a removal target strictly inside the checkout that loaded the guard (`resolve(import.meta.dir, "../..")`, both canonical and lexical spellings) is not a protected location. Equality and ancestor checks still run first for every protected tree, so the checkout root itself and anything above it stay refused; outside a checkout that lives under a protected tree the exemption changes nothing. Computed lazily so production module load does no extra work. | ||
| - `tests/ci-workflows/test-home-guard.test.ts`: a probe with a sentinel real home whose `.codex` is a symlink to the checkout's parent, so the checkout sits inside the protected Codex home. Asserts a fixture path under `tests/` is allowed, while the checkout root, its parent and a sibling of the checkout are refused. Skipped where symlinks are unavailable, like the existing symlink probe. | ||
| - `tests/service/shutdown-launcher.test.ts`: write `{ "port": <freePort> }` to the fresh home's `config.json` so the configured-port probe targets the test's own port, not the host's 10100. | ||
| - `structure/`: record the checkout exemption where the guard is described, if a doc owns it. | ||
|
|
||
| ## Verification | ||
|
|
||
| - `bun test tests/ci-workflows/test-home-guard.test.ts tests/service/shutdown-launcher.test.ts` from the managed worktree, with the host proxy live on 10100. | ||
| - `bun test ./tests/codex-integration/model-visibility-management-api.test.ts` from the managed worktree (23/23 failing before). | ||
| - `bun run test:changed` from the managed worktree: failures must fall to the `/private/tmp` level and contain no `refusing to remove`. | ||
| - `bun run typecheck`, `bun run structure:check`, `bun run privacy:scan`; exact-head CI; after merge, the `dev` push CI for both merge commits. | ||
|
|
||
| ## Residual, out of scope | ||
|
|
||
| The `/private/tmp` full run still showed 5 s timeouts in the Claude picker/intercept TLS suites and service-state suites when 1566 files share one machine; each passes in isolation and on the sharded CI. Recorded, not changed here. | ||
|
|
||
|
|
||
| ## Audit fold (NEAR-PASS, two blockers) | ||
|
|
||
| 1. The exemption is tied to one protected tree. It lifts only the "inside `protectedPath`" refusal, and only when a checkout root spelling is strictly inside that same `protectedPath` and the candidate is strictly inside that checkout root. Equality, ancestor and real-home checks stay unconditional. A bunfs build (`import.meta.dir` under `/$bunfs/root`, root `/`) or a checkout at `$HOME` never satisfies "checkout inside the tree", so the guard is unchanged there. Probe: `OCX_REAL_HOME=<checkout root>` still refuses `<root>/.codex/x`. | ||
| 2. Each spelling (canonical, lexical) is judged on its own and any refusal wins. Probe: a symlink inside the checkout that points at the protected tree outside the checkout is refused through its canonical form. | ||
|
|
||
| Notes taken: the launcher test also pins `GROK_HOME` and `OCX_OWNER_REGISTRY_DIR` into the fixture home (the `sibling-home-client-sync` pattern) so inherited real state cannot mark the child a sibling; the `protectedRemovalReason` doc comment is updated; verification adds a run from a checkout outside `~/.codex`. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On macOS with a live proxy in the user's default home, these overrides still do not isolate the child:
findCrossHomeOwnerDetailed()separately readsjoin(homedir(), ".opencodex", "runtime-port.json"), and this repository documents thatos.homedir()there ignores the rewrittenHOME.OCX_OWNER_REGISTRY_DIRonly relocates the registry, somarkCrossHomeSibling()can still find the host proxy, mark this child as a sibling, skip Codex config injection, and make all three signal cases time out exactly as before. Add a test-only seam for the default-home lookup and point it atidentity.homeDirhere.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked this against the running host rather than the comment in the repo.
findCrossHomeOwnerDetailedresolves the default home asjoin(options.homeDir ?? homedir(), ".opencodex")(src/cli/cross-home-owner.ts:162). On this macOS host both runtimes honorHOME:HOME=/private/tmp/mm31/fakehome bun -e 'console.log(require("os").homedir())'prints the fake home, andnode -eprints the same. The child here getsHOME: identity.homeDir.~/.opencodex/runtime-port.jsonand ocx 2.73.0 answering on 10100, andbun test ./tests/service/shutdown-launcher.test.tspasses all three signal cases from this branch. The same three failed atdev6538001e2fon the same host.The passwd-database note in the repo applies to the LaunchAgents path comment. It does not hold for
homedir()in these runtimes, so no extra seam is needed for this test.