fix: timeout GitHub CLI calls in repair github-cli - #1302
Conversation
Add timeoutMs to GhRunOptions and pass it through execFileSync, execFile, and spawnSync. Default to the existing 2-minute repair GitHub CLI budget so a stalled gh child fails instead of blocking comment-router and other mutation paths. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs changes before merge. Reviewed August 29, 2026, 8:01 PM ET / August 30, 2026, 00:01 UTC. ClawSweeper reviewWhat this changesThe PR adds configurable deadlines to repair-lane GitHub CLI helper calls so stalled child processes time out instead of blocking worker operations. Merge readinessKeep open: the timeout change is useful and its central timeout behavior has real runtime proof, but the new resolver ignores timeout settings supplied through a call’s existing Priority: P2 Review scores
Verification
How this fits togetherThe repair lane invokes the GitHub CLI to read and mutate repository state for review and repair workflows. These helpers turn command arguments and environment settings into child-process results that feed routing, intake, and repair operations. flowchart LR
A[Repair and review workers] --> B[GitHub CLI helper]
B --> C[Timeout selection]
C --> D[GitHub CLI child process]
D --> E[Command result or timeout]
E --> F[Routing and repair operations]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Resolve timeout variables from Do we have a high-confidence way to reproduce the issue? Yes: the supplied post-fix terminal trace demonstrates the central child-process timeout path, and source inspection directly shows that an Is this the best way to solve the issue? No: applying deadlines is the right narrow approach, but the resolver should honor the per-call environment already accepted by the helper before this implementation is merged. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9f4da9302aea. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
What Problem This Solves
Fixes an issue where ClawSweeper repair and review mutation workers would hang forever when a
ghchild stopped making progress.ghText,ghTextAsync, andghSpawnlaunched the GitHub CLI throughexecFileSync,execFile, andspawnSyncwith no deadline, so one stalled API call blocked comment routing, PR intake, cluster planning, and other mutation paths until the process was killed.Why This Change Was Made
GhRunOptionsnow acceptstimeoutMsand every GitHub CLI spawn passes a resolved timeout through to Node. Callers can set a per-call budget. When they omit it, the helper usesCLAWSWEEPER_GH_COMMAND_TIMEOUT_MSorCLAWSWEEPER_NETWORK_COMMAND_TIMEOUT_MS, then a 2-minute default that matches the existing repairexecute-fix-githubbudget. This is the same class of hang fix as media-proof and review-blob timeouts.User Impact
Operators and hosted workers no longer lose an entire repair or comment-router process to one stuck
ghinvocation. A hung GitHub CLI call fails with a timeout instead of sitting on the event loop until the runner is cancelled.OpenClaw Bay Impact
Unaffected. This change only bounds worker-side GitHub CLI child processes. Bay remains an observer-only surface and does not call
gh.Documentation Impact
No documentation change.
docs/repair/README.mdalready describesCLAWSWEEPER_GH_COMMAND_TIMEOUT_MSandCLAWSWEEPER_NETWORK_COMMAND_TIMEOUT_MSas the GitHub CLI budget knobs. The repair helper now honors those same variables. The existing contract stays accurate.Evidence
Live
noderun against compileddist/repair/github-cli.json this branch.GH_BINpointed at a 30-second sleeper. Each helper was called withtimeoutMs: 250.Before this patch,
ghTextignoredtimeoutMsand waited for the child to finish (lateafter 2000ms, no exception).After this patch, all three helpers returned in 759ms instead of 90 seconds:
A fast command still succeeds under the same timeout field:
Real behavior proof
Behavior or issue addressed: Repair GitHub CLI helpers (
ghText,ghTextAsync,ghSpawn) no longer wait forever when theghchild hangs.Real environment tested: macOS Darwin 25.6.0 arm64, Node v26.7.0, branch
fix/f005-gh-cli-timeoutat/tmp/oc-pr-clawsweeper-F005, compileddist/repair/github-cli.js.Exact steps or command run after this patch:
Evidence after fix: terminal output from the patched helper is in the Evidence section above. The 30-second sleeper was killed at 250ms per call (
ETIMEDOUT/SIGTERM). The healthy command returnedokin 25ms.Observed result after fix: Hung GitHub CLI children fail closed on a deadline. Successful commands still return their stdout. Default and env budgets remain available when
timeoutMsis omitted.What was not tested: Live
gh apiagainst api.github.com, Windows job-object kill behavior, and hosted comment-router workers under a real GitHub outage.Summary
Related same-repo hang bounds: #1173 (ffmpeg/ffprobe), #1175 (git fetch and state-blob fetch). Repair execution already times out
ghinsrc/repair/execute-fix-github.ts. This patch closes the shared helper used by comment-router and other mutation paths.The unbounded
execFileSyncpath has been present since62dd779eb(2026-04-29, repair-lane unification), about 122 days.