Skip to content

perf(tests): reuse cached CLI for Windows prompt fixtures - #2195

Merged
Alan-TheGentleman merged 3 commits into
Gentleman-Programming:mainfrom
dnlrsls:perf/2193-windows-fixture-reuse
Oct 31, 2026
Merged

Alan-TheGentleman merged 3 commits into
Gentleman-Programming:mainfrom
dnlrsls:perf/2193-windows-fixture-reuse

Conversation

@dnlrsls

@dnlrsls dnlrsls commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

🔗 Linked Issue

Closes #2193

Lifecycle Evidence

  • Linked issue is approved and assigned; verified before publication.
  • Approved scope/design, regression evidence, non-goals and independent review are recorded below.
  • Evidence is proportional; no separate design document is required.
  • Required checks remain mandatory before merge; no merge is authorized.

🏷️ PR Type

  • type:bug
  • type:feature
  • type:question
  • type:docs
  • type:refactor
  • type:chore
  • type:breaking-change

📝 Summary

  • Replace two redundant identical fixture CLI builds with copies of the existing process-owned cached CLI.
  • Keep caller-owned private paths, spaces, cleanup and lifetime semantics.
  • Pin build elision, independent byte copies and cached-owner usability with regressions.

📂 Changes

File Change
plugin/codex_windows_user_prompt_test.go Reuse cached CLI bytes via private copies; add isolation/lifetime regression.

🧪 Test Plan

  • Observe regression RED before the helper fix, then GREEN.
  • Native Windows/Go 1.26.10 focused regression: go test ./plugin -json -count=1 -run "^(TestCodexWindowsEngramPrivateCopyReused|TestCodexFixtureCLIReused)$" — passed.
  • Focused affected cases: go test ./plugin -json -count=1 -run "^(TestCodexWindowsNativeUserPrompt.*|TestCodexWindowsSessionEndAdapter|TestCodexWindowsEndEnvPortIsolation|TestCodexWindowsEngramPrivateCopyReused|TestCodexFixtureCLIReused)$" — 13 top-level tests / 50 passing records.
  • Affected package: go test ./plugin -json -count=1, with OS=Windows_NT, installed Git/jq and isolated profile/cache/temp — final uninstrumented run: 542 PASS, 0 FAIL, 6 SKIP. Those six platform/prerequisite exclusions are not executed coverage.
  • Independent security/lifetime audit passed: equivalent build inputs, private byte copies, process-owned cache lifetime, cleanup and unchanged source hashes.
  • Whitespace/format verified; no runtime hook, assertion or deadline changes.
  • Hosted full unit/E2E/lint and applicable platform checks — pending actual results.

🤖 Automated Checks

Local evidence is not a hosted pass. All statuses below are pending at creation.

Check Status
Check Issue Reference Pending
Check Issue Has status:approved Pending
Check PR Has type:* Label Pending
Check PR Has No Transient Artifacts Pending
Unit Tests Pending
E2E Tests Pending
Plugin Tests Pending
Lint Pending
Windows Setup Test Pending
Cloud Sync Wrapper Tests (Windows) Pending

✅ Contributor Checklist

  • Linked an approved, assigned issue with Closes #N.
  • Apply exactly the selected type:* label in this separately authorized creation request.
  • Recorded focused regression and affected-check commands/outcomes, including exclusions.
  • Recorded additional local checks and independent review; identified missing hosted evidence.
  • Documentation N/A: test-only helper reuse; no user or contributor workflow behavior change.
  • Conventional Commit; no Co-Authored-By trailer.
  • Changed paths comply with the Transient Artifact Policy; no local runners/evidence included.

💬 Notes for Reviewers

Base: a6f66c2ea21f6ec2fcec4b4d6d54c576bb069897; head: 0b181d6a2f7f0aebc222d2a42f6ee1e865685a9e (73 additions / 8 deletions).

A prior local runner omitted OS=Windows_NT, selected the POSIX background branch and hit the existing pipe-EOF deadline. An isolated two-case diagnostic confirmed the environment distinction; the final package run used the correct Windows environment without instrumentation or source/assertion/deadline changes. Prior failures were preserved, not relabeled.

This changes tests only, not production hooks/APIs/cache ownership. Eliminating redundant compilation does not establish hosted Windows/full-CI speedup. Native assessment was medium with reviewDue=false; independent verification is not a fabricated native approval.

Risk: item 2 (fixture executable/path trust), item 4 (shared fixture lifetime). No merge or manual CI dispatch/rerun is authorized.

Summary by CodeRabbit

  • Tests
    • Improved Windows test coverage to verify that repeated executable requests use separate copies without modifying the cached executable, caller-owned files, or working directory.
    • Expanded checks for race-test cache policies and workflow safeguards.
  • Chores
    • Updated the race-test workflow to restore compatible Go caches and save them only after successful runs on eligible pushes.
  • Documentation
    • Added guidance on race-test caching, cache outcomes, and related checks.

Current-main verification

Current candidate: f3c247d, integrated with canonical main 7cf4ed2 without rewriting existing history. Original one-file scope retained.

Native Windows Go 1.26.10: go test -mod=readonly ./plugin -run '^TestCodexWindows(EngramPrivateCopyReused|NativeUserPromptTrustedPinForwardsInputAndMeetsTimingBudget|NativeUserPromptIgnoresInterpreterAndBinaryHijacks)$' -count=1 -timeout=2m -v passed (18.438s). All three named tests passed, preserving private independent executable copies, existing native timing assertions and hostile-executable rejection. No deadline, skip allowance or production behavior changed.

Fresh current-head hosted CI and review remain pending. This does not repair every Windows full-suite failure, prove global package savings independently, or activate the required-check rollout.

Avoid rebuilding the same CLI for trusted-pin fixtures while preserving caller-owned executable paths. Pin build elision, byte identity, isolation and cached lifetime with a regression.

Refs Gentleman-Programming#2193
@dnlrsls dnlrsls added the type:refactor Code refactoring label Oct 29, 2026
@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 0b181d6 and f3c247d.

Files selected for processing: 5
  • .github/scripts/ci-technical-contracts.test.mjs
  • .github/scripts/ci-unit-node.test.mjs
  • .github/scripts/issue-refresh-router-workflow.test.mjs
  • .github/workflows/ci.yml
  • docs/ci-go-race.md

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

Walkthrough

The Windows Codex tests now use the cached CLI fixture for executable targets. A new test checks that repeated requests create distinct copies and leave the cached CLI, caller-owned files, and working directory unchanged.

Changes: Windows Codex fixture reuse

Layer / File(s) Summary
Copy and validate the cached fixture
plugin/codex_windows_user_prompt_test.go
buildCodexWindowsEngram copies the cached CLI fixture to the requested target. Tests use the updated helper signature and verify distinct copies, unchanged fixture bytes, caller-owned files, and working directory.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: gentleman-programming

Merge Risk: Minimal · up to f3c24

No merge-blocking issue was established; the cached fixture remains available throughout the test run.

Pre-merge checks: Passed 3 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Out of Scope Changes check Warning The whole-PR diff contains unrelated race-cache workflow work. It changes .github/workflows/ci.yml, adds docs/ci-go-race.md content for #2192, and modifies `.github/scripts/ci-technical-contracts.… Remove the unrelated race-cache workflow, documentation, contract-test, and baseline changes from this pull request, or move them to a separately linked and authorized pull request. Retain the plugin/codex_windows_user_prompt_test.go chan…
Passed Passed checks: 3 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: reusing the cached CLI for Windows prompt test fixtures.
Linked Issues check Passed Issue #2193 requires reuse only for equivalent CLI fixtures and preservation of private paths, isolation, trusted-path behavior, cleanup, and lifetime. In plugin/codex_windows_user_prompt_test.go, `…
Full details: Linked Issues check

Explanation

Issue #2193 requires reuse only for equivalent CLI fixtures and preservation of private paths, isolation, trusted-path behavior, cleanup, and lifetime. In plugin/codex_windows_user_prompt_test.go, buildCodexWindowsEngram now obtains buildCodexFixtureCLI and copies its bytes to each requested target. TestCodexWindowsEngramPrivateCopyReused checks separate space-containing executable paths, byte equality, cached-file survival, non-aliasing, caller-owned files, and working-directory isolation. The native tests retain their existing assertions and identities. The reported tests provide build-count and functional evidence. The PR does not claim that reuse fixes the package timeout or HTTP timing failure.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff contains unrelated race-cache workflow work. It changes .github/workflows/ci.yml, adds docs/ci-go-race.md content for #2192, and modifies .github/scripts/ci-technical-contracts.test.mjs, .github/scripts/ci-unit-node.test.mjs, and the workflow baseline hash. These changes implement CI cache policy and contract testing, not Windows Codex fixture reuse for #2193. The current PR description and linked issue define a narrow test-helper scope.

Resolution

Remove the unrelated race-cache workflow, documentation, contract-test, and baseline changes from this pull request, or move them to a separately linked and authorized pull request. Retain the plugin/codex_windows_user_prompt_test.go changes for #2193.

Finishing Touches
🧪 Generate unit tests (beta)
  • 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

Comment @coderabbitai help to get the list of available commands.

…gramming#2194)

* ci(race): refresh compatible Go caches on trusted main

Refresh immutable cache populations by tested revision while retaining one Race cache owner and full suites. Restrict workflow saves to successful canonical main pushes. Hosted net benefit remains unproven.

Refs Gentleman-Programming#2192.

* test(ci): refresh approved Race workflow baseline

Accept only the independently verified Race cache delta in the pinned CI baseline. Retain full-file hashing, readiness guards and every other workflow hash.

Refs Gentleman-Programming#2192.
@Alan-TheGentleman

Alan-TheGentleman commented Oct 31, 2026 •

Copy link
Copy Markdown
Collaborator

Focused native Windows qualification completed for this exact head (f3c247d48b217eba121bbf7e84eab132fefe13a3) versus its base (7cf4ed29b6f99ffbebb5f74e2cf33819b24775c6). This closes the previously unexecuted fixture-security/regression evidence, not the inherited full-suite failures.

Snapshot Top-level tests Complete paths including subtests Fail / skip Go exit Focused elapsed
Base 18 47 passed 0 / 0 0 47.512643 s
Candidate 19 50 passed 0 / 0 0 42.932395 s

Run: https://github.com/Alan-TheGentleman/engram-ci-qualification-01a19147/actions/runs/41229358002

Both snapshots ran on the same native Windows x64 image 20261021.278.1, Go1.26.10 and Node24.22.1, with identical module/sum hashes and separate home/app-data/data/temp/Go-cache/module-cache states. The original timing assertions were enabled. The candidate includes both new owner subtests. Actual Git Bash and absolute jq prerequisites were checked; JSON, stderr, original exits, timing and SHA/image metadata were retained. The strict verifier and a parent replay require every expected path exactly once, package completion, zero failures/skips and exit zero. The auxiliary harness had independent review and 117 local passes plus one explicitly Windows-only local omission; native execution is the evidence above.

Limit: this is one focused comparison with extra candidate regression coverage, not proof that the complete Windows suite is green or a full-CI latency guarantee. The existing observational failure and its original 600-second/HTTP budgets remain visible and unchanged. All canonical required contexts and the merge queue remain mandatory; this auxiliary run does not impersonate a required check. No source, approval, label or guard was weakened. No human responsibility attestation is inferred.


Post-merge observation — global Windows qualification remains open

The mandatory native queue merged the qualified head as f616a7efe37c920a2c9113746abe3664dedc0c47 at 2026-10-31T18:43:42Z, without an admin bypass, direct-main push or required-status substitution.

Exact-main run: https://github.com/Gentleman-Programming/engram/actions/runs/41230813688

  • 16/17 jobs succeeded, including mandatory Unit/E2E/Plugin checks, race, vulnerability, native macOS/adapter, Windows setup/cloud-sync, lint and workflow checks; zero jobs skipped.
  • The complete workflow failed, exit 1, solely at Windows Full Suite (Observational). Its retained suite data contains 8574 passes, 127 skips and 12 failures. This is not a full-suite pass or valid full-CI speed sample.
  • All 50 focused Windows paths also ran and passed exactly once in that full post-merge suite, including both new private-copy owner cases; none was failed, skipped, missing or duplicated.
  • The failed leaves show Git Bash could not find /tmp stderr in session-handoff and deadcode-ratchet assertions, plus the original HTTP two-second boundary (2.0544323s). These failure classes already occurred in archived base runs, but the individual failed subcases differ: this does not prove every present failure inherited.
  • The original 8710-path baseline inventory and 8713-path main inventory differ only by the three added fixture regression paths and a temp-directory-derived subtest name. Package completion is observed; the old candidate's unfinished plugin timeout is not carried forward as a pass.

No retry, gate/label/rules change, timeout increase, source fix or reduced security assertion was used to obtain this result. The fixture optimization and normal delivery are observed; a green complete Windows suite remains unproven and outside this focused qualification's outcome. The remaining Windows debt needs explicit disposition rather than being silently declared fixed.


Final canonical main verification — Windows repair complete

Both complementary source PRs merged through the mandatory native queue at 2026-10-31T21:47:37Z, with exact reviewed heads and no administrative bypass:

The final canonical tree 47de8548e3452d855ca6e4f92c7daad558dc35b3 is byte-identical to the independently/native-qualified integration. Both root issues #2190 and #2208 closed through their existing references; no approval/ownership/label mutation was used.

Post-merge exact-main CI: https://github.com/Gentleman-Programming/engram/actions/runs/41245230929

All 17 jobs succeeded. Final native Windows JSON independently checked against the qualified inventory: 8603 PASS / 127 unchanged SKIP / 0 FAIL, 8730 unique paths, original exit0, no missing/extra cases, changed outcomes, duplicates or unfinished tests. All 38 package outcomes match (34 passed, four have no tests). Exact checkout d1b5dcdff35d9160e71cf276db9fcbecd371733a; Windows2025 x64 image 20261021.278.1, Go1.26.10, Node24.22.1.

Production, workflow, native1.5s deadline, public2s assertion, package10m budget and all executable/path/ownership guards remain unchanged. No filtering, new skip, forced success, status substitution, or required-check/queue/settings change was needed. Earlier failed baseline/local checks remain failed historical evidence; final green is an observed result at this exact source, not a claim that every intermittent failure was explained or a stable whole-CI speed guarantee. The wider #1891 required-check rollout is not activated or closed by this repair.

@Alan-TheGentleman
Alan-TheGentleman added this pull request to the merge queue Oct 31, 2026
Merged via the queue into Gentleman-Programming:main with commit f616a7e Oct 31, 2026
48 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:refactor Code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(tests): reuse identical Windows Codex CLI fixture builds

2 participants