diff --git a/devlog/_plan/260817_wave5_execution/080_wave5d_antigravity.md b/devlog/_plan/260817_wave5_execution/080_wave5d_antigravity.md index 01ae3fbe0b..9086500b74 100644 --- a/devlog/_plan/260817_wave5_execution/080_wave5d_antigravity.md +++ b/devlog/_plan/260817_wave5_execution/080_wave5d_antigravity.md @@ -185,3 +185,80 @@ make that record false rather than merely skip a step. Reported, not cleared. The planned order (`#1889 → #1891 → #1897`) is therefore moot: #1897 is in, and the remaining two are gated on a human decision each — one a readiness checklist, one a security review. +### Corrections from the WP8 audit + +**Failing-check count.** #1889 has **two** distinct failing checks, `hygiene` and +`enforce-target`. Earlier text said four, which was the count of failing check *runs* across +re-runs (`enforce-target` appears three times). Verified with `unique`. + +**#1891's head moved, and the reviewer's staleness finding is itself stale.** The audit reported +the head 62 commits behind `origin/dev`, which would have mattered: +`READINESS_LATEST_DEV_BEHIND_MAX = 10` in `.github/scripts/pr-quality-state.cjs` unticks the +`latest_dev` box past that, so ticking without rebasing would have re-drafted the PR. Re-checked +against the live head `81236807f`: **0 commits behind**. The author rebased in the interim, so +ticking alone is now sufficient — which is what my comment on the PR says. + +Worth keeping as a lesson rather than deleting: a rejected finding was still worth chasing, +because the mechanism it named is real and would have made my advice wrong on a different day. + +### Work found and done instead of held + +The audit asked whether anything here could be landed rather than recorded. One thing could, +and it was a live defect on `dev` independent of both PRs: `metadata.ide_version` in +`src/oauth/google-antigravity.ts` was set to `antigravityUserAgent()` — the whole header, +`antigravity/ide/2.5.5 (aidev_client; os_type=...; arch=...)` — where the real client sends +`2.5.5`. + +Nothing failed, which is why it survived: the request succeeds, it just does not look like +Antigravity. `ANTIGRAVITY_IDE_VERSION` already existed one import away. Fixed in **#1955**, with +a regression that pins the field and asserts the shapes it must not have; driven red first. + +That is also the honest answer to "is the sponsorship refusal over-cautious": I hold #1889 +because reviewing *someone else's* auth change is the maintainer act the label records — but a +one-line auth fix I wrote and verified myself is exactly the case where a maintainer sponsors +their own work, so it ships. +### #1891 landed after all + +The hold expired four minutes after I wrote it. The gate bot marked #1891 `review-ready` at +02:10:50Z — the author rebased onto `9eb3a101a` and ticked all four boxes — so the checklist +block described above and in my PR comments was accurate when posted and false shortly after. + +Merged as `5c66ad205`, verified as an ancestor of `origin/dev`. No file overlap with #1955 +(`src/adapters/` vs `src/oauth/`), so nothing conflicted. + +Wave 5D final state: **#1897 and #1891 and the #1955 fix landed; #1889 alone remains**, blocked +on maintainer sponsorship of an auth surface. + +### Full-suite result and the one failure + +`bun test --isolate tests` on the merged tree: **12805 pass, 10 skip, 1 fail** across 826 files. + +The failure is `Codex autostart shim > Unix shim permits a real Codex process to start a new +child invocation`, failing with `status 126` — permission denied on exec. It is **environmental +and pre-existing**, established three ways rather than assumed: + +1. it reproduces solo, so it is not cross-test interference; +2. it fails identically at the campaign baseline `1208bd25c`, which predates every change in + this campaign; +3. all four `test 1/4..4/4` shards passed in the dev CI run for `9eb3a101a`. + +**Correction — I had the mechanism wrong, and a reviewer traced the real one.** I wrote that 126 +was the shell's "found but not executable" and that this sandbox blocks execution from a temp +path. Neither is true: a `chmod 755` script in `mktemp -d` runs fine here, and `/var/folders` is +not mounted `noexec`. + +126 is **opencodex's own recursion-guard sentinel**. This shell exports +`OCX_SHIM_ACTIVE_DEPTH=1` and `OCX_SHIM_ACTIVE_PID`, because the session itself was launched +through an installed Codex shim. The test deleted only the pid, so the outer shim started at +depth 1 instead of 0, the child re-entry reached depth 2, and the guard fired with its +launcher-loop message — the shim behaving exactly as designed, on a test that meant to start +from a clean slate. CI is green because CI has no shimmed ancestor, which is what made the +failure look environmental rather than under-sanitized. + +So the fix is a one-line test change, not an environment note: `delete env.OCX_SHIM_ACTIVE_DEPTH` +beside the existing pid deletion. Left alone it stays red for every developer running the suite +under an installed shim. Fixed here; the suite is now **12806 pass, 0 fail** locally. + +Worth keeping as the lesson: "environmental" was the right disposition and the wrong +explanation, and a plausible-sounding mechanism in a durable devlog is exactly what misleads +whoever hits this next. diff --git a/devlog/_plan/260817_wave5_execution/090_wave6_closeout.md b/devlog/_plan/260817_wave5_execution/090_wave6_closeout.md index 5d3c8bffd1..0b1828725d 100644 --- a/devlog/_plan/260817_wave5_execution/090_wave6_closeout.md +++ b/devlog/_plan/260817_wave5_execution/090_wave6_closeout.md @@ -88,11 +88,16 @@ Run on the promotion candidate (local `dev`, 6 commits ahead of `origin/dev` at ### What actually closed, under the close-on-dev-merge decision -| Closed | Landed via | -|--------|-----------| -| #1894 | #1739 through PR #1921 | -| #1843 | #1860, already released in v2.24.0 | -| #1899 | superseded by the ordering assertion in PR #1923 | +**Two issues** closed, plus one pull request: + +| Closed | Kind | Landed via | +|--------|------|-----------| +| #1894 | issue | #1739 through PR #1921 | +| #1843 | issue | #1860, already released in v2.24.0 | +| #1899 | **pull request** | superseded by the ordering assertion in PR #1923 | + +The first version of this table listed all three as issues, which overstated the run. +#1899 is a PR; two issues closed, not three. Everything else stayed open, and none of it for release-timing reasons — which is the point worth making about the policy change. It removed a gate that was never what held these back. @@ -104,3 +109,22 @@ worth making about the policy change. It removed a gate that was never what held and `aca3c0241` were both cancelled by supersession as later merges landed. The local full suite above is the evidence that exists; a hosted run on the exact promotion head is the evidence that does not, and promotion should carry that distinction rather than bury it. +### One merge landed on a red run + +PR #1921's merge commit `9dbc5fc42` has a failing hosted run (`32026536154`). The failure is +`provider request pacing queue > spaces concurrent starts in one provider FIFO` in +`tests/request-pacing.test.ts` — a wall-clock assertion, which is the classic flake shape on a +loaded macOS runner. Evidence it is not a live regression: the file passes locally, and every +subsequent hosted run on `dev` is green including the current head. + +It is recorded here because it happened, not because it blocks anything. A campaign record that +omits the one merge that landed red is exactly the kind of record you cannot trust later. + +### Promotion evidence, updated + +The "no completed green run" statement above is **stale and superseded**. Run `32090176020` on +`9eb3a101a` is `completed/success` with every job green — four test shards, macOS, keyring on +all three OSes, npm-global on all three, gates, storage policy, api usage. + +So the hosted evidence now exists. Promote the head CI actually evaluated; promoting a local ref +that no run has seen would re-open the exact gap this section was written about. diff --git a/tests/codex-shim.test.ts b/tests/codex-shim.test.ts index 0c73d91d7a..177964e987 100644 --- a/tests/codex-shim.test.ts +++ b/tests/codex-shim.test.ts @@ -1043,7 +1043,14 @@ printf '%s\\n' child-codex chmodSync(realCodexPath, 0o755); chmodSync(shimPath, 0o755); const env = { ...process.env, OCX_SHIM_BYPASS: "1" }; + // Both recursion-guard variables, not just the pid. A developer running this suite from a + // shell that was itself launched through an installed shim inherits + // OCX_SHIM_ACTIVE_DEPTH=1, so the outer shim starts at depth 1, the child re-entry lands on + // depth 2, and the guard exits 126 with the launcher-loop message — the shim behaving + // exactly as designed, on a test that meant to start from a clean slate. CI never sees it + // because CI has no shimmed ancestor, which is what made this look environmental. delete env.OCX_SHIM_ACTIVE_PID; + delete env.OCX_SHIM_ACTIVE_DEPTH; const result = spawnSync(shimPath, ["--help"], { encoding: "utf8",