[lidge-jun]

Summary

• Use the real portable fixture executable for the injected catalog executor, so macOS execution-boundary validation reaches the asynchronous contract under test.
• Assert that the uncooperative executor actually starts and receives the deadline abort before checking that a later load succeeds. The runtime trust validation remains unchanged.
• Bisect identified c0014da: its macOS execution-boundary validation requires an executable, while the affected tests used a nonexistent Windows-style path.

Verification

• macOS arm64, Bun 1.4.4, Rust 1.99.0: bun install --frozen-lockfile passed.
• cargo +1.99.0 test --locked --manifest-path scripts/diagnostics/windows-version-control/Cargo.toml --test async_contracts -- --test-threads=4 passed three consecutive times, 16 tests each. Before the fix, the same command failed the two reported cases (14 passed, 2 failed).
• cargo +1.99.0 fmt --manifest-path scripts/diagnostics/windows-version-control/Cargo.toml --check passed.
• cargo +1.99.0 clippy --locked --manifest-path scripts/diagnostics/windows-version-control/Cargo.toml --all-targets -- -D warnings passed.
• bun run typecheck passed; bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts passed (38 tests).
• Independent GOAT review of 448256b: no findings. Existing tests using injected dependencies do not cover shared-flight cache publication or cleanup; this change does not alter those paths.
• NOT RUN: full Bun suite; disproportionately broad for a fixture-only change. PR CI covers the applicable Ubuntu checks; macOS evidence is recorded above because the PR workflow runs Ubuntu only. Linux and Windows local tests were not run.

Checklist

• Scope stays focused and avoids unrelated cleanup.
• Docs or release notes were updated when needed (no user-facing change).
• Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults (fixture-only change; runtime trust boundary unchanged).

Summary by CodeRabbit

• Tests
  • Updated diagnostic tests to use the configured fixture executable rather than a hard-coded Windows path.
  • Added checks that a timed-out refresh starts the executor and triggers its abort signal; the tests also verify that a later load succeeds.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-03T11:49:02.017588Z	448256b	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The Windows async contract tests now use the configured executable as the catalog command candidate. The deadline test also checks that execution starts and receives the abort signal, while retaining its timeout and subsequent-load assertions.

Changes: Windows async contract tests

Layer / File(s)	Summary
Executable candidate and deadline assertions scripts/diagnostics/windows-version-control/tests/async_contracts.test.rs	The invalidation and deadline tests use CODEX_CLI_PATH instead of a hard-coded Windows path. The deadline test asserts executor entry and abort-signal delivery, and retains checks that the timed-out load returns null and a subsequent load succeeds.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Possibly related PRs

• lidge-jun/opencodex#7758: Adds executable validation and deadline-abort behavior for async bundled catalog loads, which this PR tests.

Merge Risk: Minimal · up to 44825

The changes improve fixture portability and deadline-test coverage without changing runtime behavior. No actionable merge-blocking risk was identified.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "test(diagnostics): fix macOS catalog async fixtures" matches the changeset. The raw summary shows the change replaces a hard-coded Windows path with the configured fixture executable in the…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "test(diagnostics): fix macOS catalog async fixtures" matches the changeset. The raw summary shows the change replaces a hard-coded Windows path with the configured fixture executable in the async contracts test file. It also adds assertions that the uncooperative executor starts and its abort signal fires. The PR objectives describe the same fixture change. The title names the main change and the test scope, so it is a clear and specific summary.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Run configuration

• Source mode: Auto
• Repository: lidge-jun/opencodex
• Branch: fix/catalog-macos-async-contracts
• File: .coderabbit.yaml
• Commit: 448256b
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 398f24c2-7257-4435-90f6-2b2c376c8250

Commits

Reviewing files that changed from the base of the PR and between 345c428 and 448256b.

Files selected for processing: 1

• scripts/diagnostics/windows-version-control/tests/async_contracts.test.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

[lidge-jun]

Maintainer integration into dev (account lidge-jun) at head 448256b76633a47774c2e83fd4916e5b7fbc523f.

The macOS leg of Catalog async contracts had failed on every main push since c0014da53 (#7758) added macOS execution-boundary validation: two contract tests passed a nonexistent Windows-style executable path, so macOS refused it before the mocked executor ran. The tests now use the existing fixture executable, and the deadline test asserts that the executor is entered and receives an abort.

• Exact-head required CI passed (ci, enforce-target, hygiene, test 1/4–4/4). PR CI does not run the macOS leg of this workflow; local macOS evidence: the exact workflow cargo test --test async_contracts -- --test-threads=4 passed 16/16 three times in a row, plus cargo fmt --check, Clippy -D warnings, typecheck and layout/ratchet guards.
• Independent review (gpt-6.1-terra, high): no defects. Test-only change.