Repository navigation
fix(codex): await recovery ACL checks in async parent admission - #7931
Conversation
Hold an exact generation across fresh Windows recovery inspection and revalidate pinned identity after awaits. Migrate asynchronous parent callers with a root-bound synchronous commit scope and keep stale-result checks on both admission success and denial. Preserve synchronous compatibility exports and document native validation and remaining synchronous publication boundaries. Follow-up for lidge-jun#7872 item 2; not a completion claim.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 8 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Deterministic PR hygiene checks passed. |
Fix stale mocks, spies and source oracles behind the 11 failures in run 40797643817, plus five reproduced failures in related consumers. Preserve refusal, ordering, timeout and handover assertions. Add three held-promise preflight ordering regressions. Tests: 280 passed across all seven affected test files; typecheck, structure, privacy and layout/size gates passed. No production source changes.
|
Follow-up for the 11 failures in CI 40797643817 is published as a2b2892, a normal fast-forward from Cause and correction
Evidence and remaining acceptanceExact-base reproduction: 11 fail / 1 pass for the reported selection; 5 fail / 4 pass for the additional selection. Final full affected-file run on Bun 1.4.4/Linux: 280 pass / 0 fail / 0 skip, 1,439 assertions. Typecheck (2,228 files), structure, privacy and whitespace checks passed; layout/size checks 11 pass. No production source, workflow or security policy changed in this follow-up. A separate reader reviewed the final immutable diff, SHA-256 New-head automatic Cross-platform CI 40803792663, Catalog 40803792647, and Service lifecycle 40803792666 are in progress at publication. The initial run failed; its prior queued status is superseded. Full repository/test:changed was not rerun locally for this tests-only correction. Exact-head Windows/native checks and maintainer/security acceptance remain separate and pending. No manual CI rerun/dispatch/approval was requested. |
|
Exact-head verification update for a2b2892, confirmed 2026-10-29 11:15 KST:
Native Windows/macOS jobs were policy-skipped. The 38 prior native skips (including eight new Windows x64 cases), exact-head native evidence and maintainer/security acceptance remain separate; #7872 item 2 is not declared complete. Keeping Draft. No manual CI dispatch/approval/rerun, merge, force-push, deployment or permission change. |
Relocate the existing async parent test mapping in the registry and independent golden table without changing either mapping or the four-shard execution plan. Preserve the layout size cap.
|
Published 52799f3 as a normal single-parent fast-forward from a2b, resolving the later dev integration conflict. GitHub now reports The 11-case mock/fixture correction and previous a2b CI success remain valid for that exact earlier head. All seven corrected test/fixture files are unchanged. The new two-file carry only relocates the existing async-parent registration in the layout registry and independent golden. All 2,754 head mappings, four-shard selection/order/batch boundaries, size caps and 1,999 layout lines are preserved. Both file-level three-way merges were clean; no product/ACL policy, timeout or workflow changed. Local layout/tooling/ratchet: 38 pass / 0 fail, 664 assertions; typecheck: 2,228 files; whitespace: pass. Independent read-only source review of exact delta For the new head, Catalog 40819794129 and Service lifecycle 40819794101 succeeded. Cross-platform 40819794077 is still in progress, with test/runner queues remaining. Actual completed checkout logs confirm Still Draft. The 38 earlier native skips, including eight new Windows x64 cases, and exact-head maintainer/security acceptance remain open; #7872 item 2 is not closed. No merge, force-push, deployment, permission change or manual CI action was performed. Final exact-head CI update — 2026-10-29 05:32 UTCHead remains |
|
Windows x64 follow-up on unchanged head
Environment: Windows 11 x64 10.0.26300, pinned Bun 1.4.4; repository-root preload and test lock enabled, Files: The second run used: This supplies the previously missing Windows-gated fixture evidence. It does not establish installed Desktop/updater acceptance or independent security acceptance, and does not complete every remaining synchronous path in #7872. No live app/account/settings change, manual CI dispatch, merge or Ready transition was performed. |
Recovery admission can now wait on ACL checks. Read the native feature state and the OpenCodex config inside the admitted callback, recheck the hybrid-pin guard there, and save that fresh config, so a change made meanwhile is neither overruled nor overwritten. Also restores the executable mode of src/cli/index.ts.
The admission is released before the awaiting command resumes, so a save made after the callback could overwrite a competing write with the read copy.
|
Maintainer integration into Security review. A dedicated fresh gpt-6.1-terra security review of Correctness review. Fresh reviews found one real issue:
A doctor Union. Verification. Windows x64, Bun 1.4.4, on
Exact-head CI. Cross-platform run 41058615039 on Scope. This lands #7872 item 2 (async recovery admission). The synchronous compatibility exports listed in the description remain. Not run: ARM64, other-SID behavior, installed Desktop success and a second real apply. |
Summary
Parent selection admission can block the Windows event loop while recovery ancestry, directory and record ACL subprocesses run. This follows up #7872 item 2 with fresh asynchronous recovery admission and the coordinated caller migration. It does not repeat the merged #7875 cleanup/identity work or close #7872.
Initial implementation:
391f48bd66251ece5167d777eeab80938a311f6c, based ondev@8eb970d9dcd63d119c9a9d20de4c18a27ca542c2. Before publication, current dev was9c33e4658b7b180ec8dd20f0e6a21daf3699f76e; its added CI-sharding commit has no path overlap with these 30 files. The latest maintainer disposition remains item 2 open; targeted open-PR searches found no duplicate.Behavior
No external target, app allowlist, HTTP authentication or ACL policy is broadened.
Verification
Draft / native and formal maintainer-security acceptance pending. Published file blobs were checked against the tested files. Current follow-up head:
52799f3f4724780d8d3649f029dde2b946ae2ccb. New-head automatic checks are tracked separately below.Current dev-conflict follow-up —
52799f3f4724780d8d3649f029dde2b946ae2ccbAfter the successful
a2b2892cCI, fresh deve3ebd82aintroduced two textual registration conflicts: adjacent Kiro shard insertion at the layout prefix and LM Studio golden entries at the oracle's first line. No product file or earlier seven-file test fix conflicted. The latest ordinary single-parent fast-forward relocates only this PR's existingcodex-cli-update-parent-async.test.tsmapping in the registry and independent golden table. No upstream-only entries were copied into the head, and the independent oracle was not regenerated from the resolver.Head mapping remains 2,754 entries. The two file-level three-way merges are clean, yielding the expected 2,765-entry union with no missing files. Layout remains 1,999 lines on both the head and synthetic integration result; caps, exemptions, runners and workflows are unchanged. The actual planner's four Linux shard selections, order and batch boundaries are identical before/after in both head and dev-integration contexts. The maintainer's #7943 correction is preserved rather than duplicated.
Focused layout/tooling/ratchet verification: 38 pass / 0 fail, 664 assertions; typecheck: 2,228 files; whitespace check: pass. The earlier seven corrected test/fixture blobs remain byte-identical to
a2b2892c. Independent read-only source review of the exact two-file delta, SHA-256f6b59fbce58ebdc194ac7496fb8cf0fff83384e0c349cb8f2f042e7ab2ba0bdd, found no blocking finding. This review did not execute tests or constitute formal maintainer/native acceptance.Current-head CI complete, verified 2026-10-29 05:32 UTC: Cross-platform 40819794077, Catalog 40819794129 and Service lifecycle 40819794101 all succeeded on attempt 1 for
52799f3f4724780d8d3649f029dde2b946ae2ccb. Cross-platform finished with 14 successful jobs / 10 policy-skipped jobs, including all four Ubuntu shards and the final aggregate. Completed logs confirm actual checkoutd0fdd1cb7ad7a87460e2bb798851570b98b86ff6= head52799f3f+dev@b09c9302c4832a12c17da5f57f5d2240fe20f817, containing #7943 and the subsequent #7889/#7745/#7937 changes.The original 11/11, additional stale-consumer 5/5, and held-Promise ordering 3/3 cases all actually passed in these new-head logs (19/19); these are not results copied from
a2b2892c. File-size ratchet, golden membership and layout guards also passed. The Linux desktop job ran Rust fmt/clippy and 247 Rust tests passed / 2 ignored; its bundled-sidecar, packaging and packaged-shell E2E steps were skipped. The eight new Windows native cases were likewise skipped, not passed. Native OS validation and explicit maintainer/security acceptance therefore remain open. GitHub reportsmergeable=true, which is separate from merge acceptance. No workflow dispatch, rerun, approval, assertion weakening or CI-policy change was performed.CI failure follow-up —
a2b2892ccc03900c875c2f6c77dd2400bce09b97Normal fast-forward from
391f48bd; all seven published test/fixture blobs match the tested files. No production source, ACL policy, timeout or workflow was changed in this follow-up.The initial Cross-platform run 40797643817 failed with 11 cases. Six missing-export errors came from a whole-module mock exposing only the former sync name; the real
maybeAutoRestoreCodexShimAsyncexport exists. One help mock, one capability source oracle and three pinned-sync spies also targeted the old contract. Static inventory reproduced five further stale-consumer failures in CLI head/ready/version-skew tests. All are aligned with the actual async boundary, preserving refusal, ordering, exit-code and call-count assertions.Exact-base RED: 11 fail / 1 pass for the reported selection, then 5 fail / 4 pass for additional consumers. Final complete affected-file run: 280 pass / 0 fail / 0 skip, 1,439 assertions, seven files, Bun 1.4.4/Linux. Three new held-Promise tests require ready/resolve/ordinary-command preflight to finish before dispatch becomes possible. Typecheck (2,228 files), structure, privacy, whitespace and layout/size (11 pass) gates passed. Full repository/test:changed was not rerun locally for this tests-only correction.
A separate source reader reviewed the immutable seven-file delta, SHA-256
d836ca63edeadd0ac23fb2a96a815a4ea06198b16e57a7474847714bd8352707, with no blocking finding. This read-only review is separate from the initial implementation review below, automatic CI, formal security acceptance and native execution.Previous head
a2b2892cautomatic checks: all three workflows succeeded; reconfirmed 2026-10-29 13:38 KST. These results do not claim completion on the new head.Windows/macOS native jobs were policy-skipped; no Windows/native acceptance is inferred. The 38 earlier native skips, including eight new Windows x64 cases, and explicit maintainer/security disposition remain open. No manual workflow dispatch, approval or rerun was requested. See the diagnosis and publication record.
Earlier implementation verification —
391f48bdPinned Bun 1.4.4, Linux:
test:changedwas attempted, then cancelled with SIGTERM/exit 143 after at least 335 observed seconds because review-driven edits occurred while it was running. It emitted no completion summary. Incomplete, not a pass. Full root/GUI suites were not run; no timeout, assertion, workflow gate or exclusion was relaxed.Independent source review
A separate reader reviewed the immutable 30-file diff over
8eb970d, SHA-2564a319c78d59dabffe8bf5596848a9d1e00d61b1526123985dbdbea780a54b679. Two findings were fixed before publication:Final review found no additional blocking code finding. It was read-only source/log review, not an independent test rerun, external security workflow, Windows execution or maintainer acceptance.
Remaining scope and required native evidence
Checklist