Repository navigation
fix(test): drain ACL work and close history before Windows fixture removal #6797
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,101 @@ | ||||||
| # 000 — L7 CI/infra stability: master plan | ||||||
|
|
||||||
| Lane L7 of the 2026-10-09 coordination round. Base: `origin/dev` `c15037b324`. | ||||||
| Branch for this roadmap: `codex/ci-infra-stability`. Each implementation work-phase | ||||||
| ships as its own PR to `dev` from a sibling worktree `.tmp/lanes/L7-ci-infra-<n>`. | ||||||
|
|
||||||
| ## Objective | ||||||
|
|
||||||
| 1. Find and fix the ownership defects behind the recurring Windows test-teardown | ||||||
| failures (`EPERM`/`EBUSY` while removing a fixture root). Raising a timeout is not a fix. | ||||||
| 2. Carry the CI/infra contributor PRs #6783, #6726 and #6723 (split), then the B-tier | ||||||
| PRs #6779, #6754 and #6765, each with `Co-authored-by` credit. | ||||||
| 3. Report #6472 and #6654 to the maintainer without carrying them. | ||||||
|
|
||||||
| Every PR stops at merge-ready: exact-head CI green plus an independent gpt-6.1-sol review | ||||||
| PASS. Merging, releasing, commenting on or closing contributor PRs, and approving fork | ||||||
| workflow runs all need the maintainer. | ||||||
|
|
||||||
| ## Constraints | ||||||
|
|
||||||
| - Local tests are minimal: focused files for the changed behavior and `bun run typecheck`. | ||||||
| Hosted CI is the main evidence. Every PR's Verification section names the exact commands | ||||||
| and what was left to CI. | ||||||
| - AGENTS.md gates: file-size ratchet (`tests/fixtures/file-size-baseline.json`; uncapped | ||||||
| files must stay below 2000 lines), test layout (new test files go in both | ||||||
| `scripts/test-layout/layout.json` and `tests/fixtures/test-layout-expected.json`), | ||||||
| `structure/` ownership sync, PR template, `dev` target. | ||||||
| - Lane boundaries: L1 owns desktop runtime ownership and launchers (so #6765 is coordinated | ||||||
| with L1), L2 owns service/journal, L5 owns auth/quota. L7 does not edit their areas. | ||||||
|
|
||||||
| ## Evidence (2026-10-09) | ||||||
|
|
||||||
| | Failure | Run / job | What the log shows | | ||||||
| | --- | --- | --- | | ||||||
| | (a) management auth | 37833647677 / 113537302143 (#6772, windows 8/9) | `server-management-auth.test.ts:220` afterEach → `removeTreeWithRetry` → `EPERM` after the 15 s retry budget; case time 15993 ms | | ||||||
| | (b) picker startup | 37735258668 / 113173321175 (dev, windows 3/9) | `claude-picker-startup.test.ts:24` raw `rmSync` `EBUSY` after `handle.stop()` | | ||||||
| | (c) OAuth live update | 37636581013 / 112844420207 (dev, windows 9/9) | `oauth-login-cli-live-update.test.ts:60` afterEach `EPERM` | | ||||||
|
|
||||||
| The "ACL hardening timed out" warnings near (a) come from an earlier mocked-timeout case, | ||||||
| so they do not prove a live icacls child under (a)'s root. The logs prove that teardown | ||||||
| failed; they do not name the native handle owner. Static reading of current dev confirms | ||||||
| three concrete ownership gaps: | ||||||
|
|
||||||
| - G1 `src/lib/windows-secret-acl.ts:417-479`: the removal barrier sees only runners that | ||||||
| outlived the outer belt. Normal in-flight icacls runners, and the gaps between successive | ||||||
| commands of one harden, are invisible to `flushWindowsSecretAclReapsBeforeRemoval`. | ||||||
| - G2 `src/routing/history/indexer.ts:356-384,481-487`: the request-history SQLite index | ||||||
| (WAL) stays open after server stop, and the management-auth and OAuth live-update | ||||||
| fixtures never call `closeRequestHistoryIndex()` (only `tests/routing/policy-execution.test.ts:23` does). | ||||||
| - G3 `tests/oauth/oauth-login-cli-live-update.test.ts:55-60`: synchronous teardown | ||||||
| with no producer, config-flight or ACL drain before removal. | ||||||
|
|
||||||
| (b) is already fixed on dev by #6748 (`b6ec62ae43`, `cleanupPickerStartup`). | ||||||
|
|
||||||
| Research reports (scratch, untracked): `.tmp/research/r1_teardown.md`, `r2_6783.md`, | ||||||
| `r3_6723.md`, `r4_btier.md`. | ||||||
|
|
||||||
| ## Decisions | ||||||
|
|
||||||
| | ID | Decision | Reason | | ||||||
| | --- | --- | --- | | ||||||
| | D1 | Fix G1-G3 in one teardown PR (wp1) built from #6723's ACL registry and drain hunks, #6783's fixture extraction and history close, and new OAuth teardown code | Three failures, one ownership contract; neither source PR alone covers it | | ||||||
| | D2 | wp1 adds Bun regression tests for the ACL flight registry and does not introduce #6723's Rust crate | Ordinary CI does not invoke the crate; only #6723's new workflow does, and that needs a separate security review (wp5). Bun tests run in every shard | | ||||||
|
Contributor
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: rg -n --glob '*.yml' --glob '*.yaml' --glob '**/Cargo.toml' --glob '*.rs' \
'async_contracts|cargo[[:space:]]+(test|nextest)|Cargo\.toml' .Repository: lidge-jun/opencodex Length of output: 157 🏁 Script executed: set -eu
printf '%s\n' '--- candidate files ---'
find . -path '*/.git' -prune -o \( -path '*/.github/workflows/*' -o -name 'Cargo.toml' -o -name '*.rs' \) -type f -print | sort
printf '%s\n' '--- relevant references ---'
rg -n -F --glob '*.yml' --glob '*.yaml' --glob 'Cargo.toml' --glob '*.rs' --glob '*.md' \
-e 'async_contracts' -e 'cargo test' -e 'cargo nextest' -e 'Cargo.toml' -e '#6723' -e 'crate' . || test "$?" -eq 1
printf '%s\n' '--- target plan excerpts ---'
nl -ba devlog/_plan/261009_L7-ci-infra/000_plan.md | sed -n '55,68p'
nl -ba devlog/_plan/261009_L7-ci-infra/001_architect_reflection.md | sed -n '1,20p'Repository: lidge-jun/opencodex Length of output: 41267 🏁 Script executed: set -eu
printf '%s\n' '--- workflow references ---'
rg -n -i --glob '*.yml' --glob '*.yaml' \
-e 'cargo' -e 'remote-workspace-helper' -e 'async_contracts' -e 'native/' -e 'manifest-path' \
.github/workflows || test "$?" -eq 1
printf '%s\n' '--- workflow files with relevant commands ---'
for f in .github/workflows/*.yml .github/workflows/*.yaml; do
test -f "$f" || continue
if rg -qi -e 'cargo' -e 'remote-workspace-helper' -e 'async_contracts' "$f"; then
printf '\n### %s\n' "$f"
nl -ba "$f"
fi
done
printf '%s\n' '--- helper and workflow history at reviewed head ---'
git ls-tree -r --name-only 39fe59c062f874fb70f998caa5e13a0ac9e61c29 -- .github/workflows native/remote-workspace-helper | sed -n '1,160p'Repository: lidge-jun/opencodex Length of output: 41380 🏁 Script executed: set -eu
printf '%s\n' '--- helper scope and job ---'
nl -ba .github/workflows/ci.yml | sed -n '210,230p;350,390p;1280,1365p'Repository: lidge-jun/opencodex Length of output: 8856 Correct the Rust workflow statement.
Suggested wording--- "a/devlog/_plan/261009_L7-ci-infra/000_plan.md"
+++ "b/devlog/_plan/261009_L7-ci-infra/000_plan.md"
@@ -60,7 +60,7 @@
| ID | Decision | Reason |
| --- | --- | --- |
| D1 | Fix G1-G3 in one teardown PR (wp1) built from #6723's ACL registry and drain hunks, #6783's fixture extraction and history close, and new OAuth teardown code | Three failures, one ownership contract; neither source PR alone covers it |
-| D2 | wp1 adds Bun regression tests for the ACL flight registry and does not introduce #6723's Rust crate | Ordinary CI does not invoke the crate; only #6723's new workflow does, and that needs a separate security review (wp5). Bun tests run in every shard |
+| D2 | wp1 adds Bun regression tests for the ACL flight registry and does not introduce #6723's Rust crate | Ordinary CI invokes the crate through its conditional `remote-helper` job when `native/remote-workspace-helper/**` changes. #6723's new workflow is separate and needs a security review (wp5). Bun tests run in every shard |
| D3 | Transplant #6723's close-awaiting WebSocket helper into #6783's extracted `tests/helpers/management-auth-fixture.ts` | Applying both inline pushes `server-management-auth.test.ts` to 2005 lines |
| D4 | #6783's retry-only cleanup hunks go into wp1 for files #6723 does not touch; where both touch a file, #6723's drain-then-retry wins | Retry alone does not settle producers |
| D5 | #6783's cache-isolation core, warm-ups, bounded children and serial GUI gate become wp2 | Separate concern (cache ownership) from removal ownership |📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| | D3 | Transplant #6723's close-awaiting WebSocket helper into #6783's extracted `tests/helpers/management-auth-fixture.ts` | Applying both inline pushes `server-management-auth.test.ts` to 2005 lines | | ||||||
| | D4 | #6783's retry-only cleanup hunks go into wp1 for files #6723 does not touch; where both touch a file, #6723's drain-then-retry wins | Retry alone does not settle producers | | ||||||
| | D5 | #6783's cache-isolation core, warm-ups, bounded children and serial GUI gate become wp2 | Separate concern (cache ownership) from removal ownership | | ||||||
| | D6 | #6723 splits into runtime (wp4) and CI/Rust diagnostics (wp5); the storage policy-load barrier moves to wp1 | The storage hunks are test barriers; the runtime has no dependency on the workflow | | ||||||
| | D7 | #6472 is not carried in L7 | It changes credential/session behavior, needs human security review and live Student acceptance, and leaves `core.ts` at 210/210 | | ||||||
| | D8 | #6654 is report-only | Its new 256 KiB generated-attribution cap (HTTP 413) is a product policy only the maintainer can accept | | ||||||
|
|
||||||
| ## Work-phase map (dependency order) | ||||||
|
|
||||||
| | WP | Doc | Unit | Depends on | | ||||||
| | --- | --- | --- | --- | | ||||||
| | wp0 | this file | Roadmap (docs only) | — | | ||||||
| | wp1 | 010 | Windows removal ownership (G1-G3) | wp0 | | ||||||
| | wp2 | 020 | #6783 transpiler-cache isolation | wp1 | | ||||||
| | wp3 | 030 | #6726 link-test isolation | wp0 | | ||||||
| | wp4 | 040 | #6723 runtime: async catalog probes, budget release on abort | wp1 | | ||||||
| | wp5 | 050 | #6723 CI: contracts workflow + Rust diagnostics (security review, draft) | wp4 | | ||||||
| | wp6 | 060 | #6779 and #6754 provider carries | wp0 | | ||||||
| | wp7 | 070 | #6765 desktop hint (with L1) + #6472/#6654 reports | wp0 | | ||||||
|
|
||||||
| Execution model: after this roadmap is locked, gpt-6.1-sol subagents may prepare | ||||||
| independent branches in parallel in their own sibling worktrees. The coordinator still runs | ||||||
| one PABCD cycle per work-phase: P re-verifies the decade doc against the live branch, A is | ||||||
| an independent review, B finalizes and pushes, C reads exact-head CI, D records the result. | ||||||
|
|
||||||
| ## Verification policy per PR | ||||||
|
|
||||||
| - Focused: the changed and new test files, `bun run typecheck`, `bun run privacy:scan`, | ||||||
| `bun run structure:check`, plus `tests/test-layout.test.ts` when layout tables change. | ||||||
| - Hosted: exact-head Cross-platform CI with all Windows shards. One failure on a | ||||||
| Windows shard in a file the PR does not touch is classified from its log before any | ||||||
| single re-run; it is never re-run silently. | ||||||
|
|
||||||
| ## Architect consultation record | ||||||
|
|
||||||
| Proposals: r1 (teardown root cause, agent `01a11e37-20f2`), r3 (#6723 split, | ||||||
| `01a11e37-2262`), r2 (#6783), r4 (B-tier). Main dispositions: D1-D8 above. D2 amends r3's | ||||||
| recommendation to ship a minimal Rust crate in the harness PR. Reflection: see 001. | ||||||
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 @@ | ||
| # 001 — Architect reflection record | ||
|
|
||
| Reflection round 1, both architects: **MISALIGNED**. Every gap was accepted and folded: | ||
|
|
||
| | Gap (architect) | Disposition | | ||
| | --- | --- | | ||
| | `restore()` deletes the Codex home even after a failed drain (r1, r3) | Folded: helper gains `restoreEnvironment()` / `removeAfterDrain()`; G3 removes a root only after its drains succeed | | ||
| | Codex-home config flights never flushed; order of history close vs reaps (r1) | Folded: both roots flushed first, then history close, then ACL drains | | ||
| | Env restore lacks a finally guarantee (r1) | Folded: restoration in `finally` | | ||
| | Structure doc hunks missing from the recipe and pointing at the absent Rust crate; test-sandbox-cleanup doc (r1, r3) | Folded into 010 file map and recipe | | ||
| | Regression list missing survivors, aliases, deleted ancestry, required-failure (r1, r3) | Folded: 010 tests 3, 5, 6, 8; fixture-order changes declared Windows-CI-only | | ||
| | "Fixtures never call history close" is false (r1) | Folded: 000 narrowed | | ||
| | "Crate only runs under the new workflow" overstated (r3) | Folded: 000 D2 reworded | | ||
| | `tests/lib/translator-budget*` path wrong; equivalence list incomplete (r3) | Folded: 040 uses `tests/adapters/translator-budget.test.ts` and the full list | | ||
| | Four vs three docs at 600 lines (r3) | Folded | | ||
| | `cargo fmt` needs `--manifest-path` (r3) | Folded | | ||
| | r3's minimal Rust crate in wp1 | Rejected per D2 | | ||
|
|
||
| Both architects confirmed the recipe paths, the 1999→1931 extraction and `core.ts` 210/210. | ||
|
|
||
| ## A-phase audit round 1 (independent sol reviewer): FAIL, 5 blockers, all folded | ||
|
|
||
| | Blocker | Fold | | ||
| | --- | --- | | ||
| | wp1 applied the whole `structure/runtime.md` diff, including a wp4 catalog paragraph | 010: only the Support-table hunk | | ||
| | wp5 map vs "restore references" instruction | 050: Bun references stay; Rust sentences only in declared files | | ||
| | `cargo nextest` unavailable | 050: workflow's `cargo test --locked ... --test async_contracts` | | ||
| | Fixture-order contract had no deterministic verifier; management-auth env restore not in finally | 010: `tests/helpers/fixture-teardown.ts` + `tests/ci-workflows/fixture-teardown-helper.test.ts` (6 cases), finally restoration | | ||
| | Haiku carry skipped its owner doc and user docs | 060: `structure/providers-and-adapters.md` and `guides/providers.md` rows | | ||
|
|
||
| ## A-phase audit round 2: FAIL, 3 blockers, all folded | ||
|
|
||
| | Blocker | Fold | | ||
| | --- | --- | | ||
| | Default `settleConfigFlights` = `flushConfigDirHardeningAndReaps` already drains ACL before history close | 010: default is `flushConfigDirHardening`; test 8 asserts it | | ||
| | Inline OAuth example disagreed with helper gating; producer failure did not block removal | 010: example replaced by the helper call; gating rules 1-6 written out; tests 6-7 | | ||
| | User-doc fold could do nothing (no Go wire list exists) | 060: required sentence in the OpenCode Go paragraph, locale disposition stated | |
Oops, something went wrong.
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.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 2525
Correct the
closeRequestHistoryIndex()call-site statement.The parenthetical is incorrect. Multiple fixture helpers call
closeRequestHistoryIndex(), sotests/routing/policy-execution.test.ts:23is not the only call site.Suggested documentation fix
📝 Committable suggestion
🤖 Prompt for AI Agents