Conversation
Port the Windows hidden-process workflow from the reporter's fork-scoped trigger to the org repository: run on pull_request and push to main, and drop the fork-only repository gate. Without this, the new contract suite never executes on Windows in CI, breaking the cross-OS validation chain this fix relies on.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe change adds ChangesWindows hidden-process coverage
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The audited internal launches now set Windows process hiding, with cross-platform contract coverage and no concrete merge-blocking defect identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
The inline win32 spread widened its element type to plain strings and tripped the recorded-diagnostics ratchet (TS2322 against ExpectedCall, 201 vs 200). Extract the fallback into an ExpectedCall[] constant so the literal kinds stay narrowed; no behavior change.
|
The first CI run failed on two pre-existing or unrelated causes, both addressed or scoped:
|
Port the Windows hidden-process workflow from the reporter's fork-scoped trigger to the org repository: run on pull_request and push to main, and drop the fork-only repository gate. Without this, the new contract suite never executes on Windows in CI, breaking the cross-OS validation chain this fix relies on.
|
Rebased onto current main (d8abc81). The situation changed since this PR opened: the fix itself and the contract tests are already on main ( What this PR now carries is the one piece main is missing: the workflow trigger. Main's copy of If you'd rather keep Windows runners off every PR, closing this in favor of a maintainer-side trigger tweak works too; otherwise this is ready to merge as-is. |
🔗 Linked Issue
Closes #967
🏷️ PR Type
type:feature— New featuretype:bug— Bug fixtype:question— Question requiring tracked worktype:docs— Documentation onlytype:refactor— Code refactoring (no behavior change)type:chore— Maintenance, dependencies, toolingtype:breaking-change— Breaking change📝 Summary
windowsHide: trueat the twenty audited Windows-capable internal Node launch callsites across six files (CodeGraph CLI and Node fallback, review workspace identity, and the review publication-gate/snapshot/transaction/repositories Git and gh wrappers). Arguments, environment, shell selection, stdio, exception handling, and the user-requested external editor are untouched.tests/windows-hidden-processes.test.ts, mocked Node boundaries asserting the launch option at every mapped boundary) onto currentmain, plus its dedicated dual-OS workflow, retriggered for this repository:pull_request/pushonmaininstead of the reporter's fork-scoped branch, with the fork-only repository gate removed.Credit: the audit, the regression suite, and the verified fix are @decode2's work (issue regression
7d5b58ee, verified candidatedcedd074, with hosted RED/GREEN runs linked in the issue). This PR cherry-picks both commits preserving authorship and adds the CI trigger port as a third commit.📂 Changes
extensions/codegraph-tools.tswindowsHide: trueon the root-probe exec and the shared CLI/Node fallback options (3 callsites).extensions/gentle-ai.tswindowsHide: trueon review workspace Git identity (1).lib/review-publication-gate.tswindowsHide: trueon gate Git probes, push-destination resolution, and the gh runner (11).lib/review-repository.tswindowsHide: trueon the legacy Git wrapper (1).lib/review-snapshot.tswindowsHide: trueon snapshot Git runs (1).lib/review-transaction.tswindowsHide: trueon store-root resolution and gate Git runs (4).tests/windows-hidden-processes.test.tswindowsHide === true; external editor stays exempt..github/workflows/windows-hidden-processes.ymlpull_request/pushonmain.🧪 Test Plan
mainbefore the fix:node --experimental-strip-types --test tests/windows-hidden-processes.test.tsreports the editor-exemption test passing and the internal-launch contract failing (windowsHideundefined).pnpm test0 failures (the 10 cancelledrdd-status-linesubtests reproduce on pristineorigin/main, pre-existing).🔍 Review Provenance
Two native four-lens review cycles (risk, resilience, readability, reliability), both approved with zero corrections:
fix+testcommits (targetsha256:825e1d45…): approved at revisionsha256:308a11bc…, acknowledgement burned.sha256:d2080c6b…): approved at revisionsha256:ab967959…, acknowledgement burned.All findings in both cycles are informational advisories (test-suite hygiene items like indent/cleanup nuances, argv contract coupling notes, and an uncached-install suggestion for the new workflow); none block this change.
Contributor Checklist
status:approved)type:*label — maintainer-side: fork author cannot add labels; requestingtype:bugtest(windows):,fix(windows):,ci(windows):)Co-Authored-BytrailersSummary by CodeRabbit
Bug Fixes
Tests