You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
ci(release): read the watcher child from the supervisor's slot in the fault harness - #27120
The v1.4.224 release build failed a release-only check, even though nothing in the app was wrong. The check looked for the file watcher's helper process in a field that #24863 renamed, so it always concluded there was no helper. This points the check at the new field.
What Changed
config/scripts/runtime-file-watcher-fault-harness.mjs now reads the watcher child and its canary directory through supervisor.slot (added by #24863), falling back to the old fields for release tags that predate it. No product code changes.
Why
#24863 moved WatcherProcessSupervisor's child and canaryDir into a WatcherChildSlot. The harness still read supervisor.child, which is now always undefined, so the "Gate runtime file-watcher process isolation" step in release-cut.yml fails with "Watcher supervisor did not expose a live child". That step only runs in release cuts on Linux, so normal PR CI never caught it. Every release cut from a base that contains #24863 fails until this lands (v1.4.224 run 38019848007 failed on both Linux legs).
The fallback keeps the harness working on older tags, because release cuts run it from the tag's own checkout.
Linked Issue
N/A, found while cutting v1.4.224.
Visual Proof
N/A, test-harness change.
Testing
On the v1.4.224 release tree after pnpm run build:electron-vite (macOS; the harness skips only Windows):
Before: node config/scripts/runtime-file-watcher-fault-harness.mjs fails with "Watcher supervisor did not expose a live child", the same error as the release run.
After: passes under Node and under ELECTRON_RUN_AS_NODE=1 pnpm exec electron: the host survives SIGSEGV of the watcher child, a replacement child starts, and post-crash events are delivered.
The next gate, relay-watcher-fault-harness.mjs, also passes on that tree.
I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.
Notes
Release-gate harness only; no app, SSH, platform or mobile impact.
Checklist
This PR is small and focused
I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
Before/after screenshots or videos attached for UI changes, or N/A with reason
Self-reviewed for correctness, security, and performance
Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
📝 Walkthrough
Walkthrough
The fault harness now reads the canary directory and child PIDs through a resolved watcher slot. It uses supervisor.slot when available and otherwise uses supervisor. The live-child check, SIGSEGV injection, and interruption wait remain unchanged.
Priority: ⬇️ Low
Merge Risk:⚪ Minimal · up to a3d12
The harness now supports both the current and legacy supervisor layouts, addressing the reported release-check failure. No material merge-blocking risk remains.
Pre-merge checks | 4 | 1
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.
Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name
Status
Explanation
Title check
The title clearly and concisely describes the main change: reading the watcher child from the supervisor slot in the fault harness.
Description check
The description covers the required explanation, change, rationale, testing, visual-proof exception, scope, and checklist. It also explains why no issue is linked and records the relevant validation. …
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.
Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ELI5
The v1.4.224 release build failed a release-only check, even though nothing in the app was wrong. The check looked for the file watcher's helper process in a field that #24863 renamed, so it always concluded there was no helper. This points the check at the new field.
What Changed
config/scripts/runtime-file-watcher-fault-harness.mjsnow reads the watcher child and its canary directory throughsupervisor.slot(added by #24863), falling back to the old fields for release tags that predate it. No product code changes.Why
#24863 moved
WatcherProcessSupervisor'schildandcanaryDirinto aWatcherChildSlot. The harness still readsupervisor.child, which is now always undefined, so the "Gate runtime file-watcher process isolation" step inrelease-cut.ymlfails with "Watcher supervisor did not expose a live child". That step only runs in release cuts on Linux, so normal PR CI never caught it. Every release cut from a base that contains #24863 fails until this lands (v1.4.224 run 38019848007 failed on both Linux legs).The fallback keeps the harness working on older tags, because release cuts run it from the tag's own checkout.
Linked Issue
N/A, found while cutting v1.4.224.
Visual Proof
N/A, test-harness change.
Testing
On the v1.4.224 release tree after
pnpm run build:electron-vite(macOS; the harness skips only Windows):Before:
node config/scripts/runtime-file-watcher-fault-harness.mjsfails with "Watcher supervisor did not expose a live child", the same error as the release run.After: passes under Node and under
ELECTRON_RUN_AS_NODE=1 pnpm exec electron: the host survives SIGSEGV of the watcher child, a replacement child starts, and post-crash events are delivered.The next gate,
relay-watcher-fault-harness.mjs, also passes on that tree.I manually tested these changes locally
Automated tests added/updated, or explained why not below
AI Disclosure
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Release-gate harness only; no app, SSH, platform or mobile impact.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)