fix(system-prompt): harden spawn-time rendering pipeline with try-catch fallbacks - #128
Open
weiesky wants to merge 3 commits into
Open
fix(system-prompt): harden spawn-time rendering pipeline with try-catch fallbacks#128weiesky wants to merge 3 commits into
weiesky wants to merge 3 commits into
Conversation
…ch fallbacks
Four guard layers so no failure in the system-prompt build/render path can
crash the process or block the claude spawn:
1. server.js listen callback: non-async wrapper + try-catch IIFE — any
await rejection (setupTerminalWebSocket / runParallelHook / startBridge)
is caught and logged; resolve(server) always fires so the bound port
never becomes an unhandled-rejection crash after cli.js passes the port
check and proceeds to spawnClaude.
2. pty-manager.js spawnClaude: wrap the entire system-prompt pipe
(_spawnModelReader → buildSystemPromptFileArgs →
renderSystemPromptFileArgs) in try-catch; any throw falls back to
{args:[], loaded:[], model:null} so claude launches with its own
default system prompt.
3. system-prompt-render.js: defensive null/type guard at function entry
returns a safe default instead of throwing TypeError.
4. create_system_prompt.js: execFileSync → spawnSync with 15 s timeout
so a hanging git subprocess (NFS / huge repo / broken index) cannot
block the spawn forever.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
added 2 commits
July 13, 2026 13:00
…en callback Root cause: pty.spawn() runs the child process synchronously, but onData and onExit were registered after emitSpawnNotice (which appends to outputBuffer and broadcasts). If the spawned claude binary exits during that gap — e.g. it rejects --system-prompt-file as an unknown option — the exit event is lost, the process handle releases, and the event loop can drain despite the HTTP servers being alive (observed on Darwin). Fix: - Move onData/onExit registration IMMEDIATELY after pty.spawn(), before any other sync work including the spawn-notice emit. - Keep the async listen-callback IIFE try-catch (prior commit) so an await rejection inside setupTerminalWebSocket / runParallelHook never becomes an unhandled rejection that crashes the process. - Keep the system-prompt pipeline try-catch and spawnSync timeout guards (prior commit). Tested: npm run test (7877 pass) + npm run test:cli (8248 pass).
… 2.x Root cause: Claude Code 2.1.206+ ships as a pure native binary — no cli.js exists inside the npm package. resolveNpmClaudePath() only looked for cli.js, returned null, and the fallback resolveNativePath() picked the platform-specific native binary (claude-code-darwin-arm64/claude). That binary SIGKILLs (137) when passed --settings, killing the PTY immediately — the HTTP server survived (200 OK) but the terminal was dead because no claude process was running. Fix: when cli.js is absent in the resolved npm package, fall back to the binary in bin/claude.exe (macOS/Linux) or bin/claude / bin/claude.exe (Windows). This is the binary installed by npm and it correctly supports --settings. Also added the same bin/ fallback to the global-node_modules lookup (step 2). Tested locally: npm binary now resolved correctly, claude PTY stays alive, HTTP 200 + terminal functional.
weiesky
pushed a commit
that referenced
this pull request
Jul 13, 2026
…28 guard layers; release 1.6.348 Model-entry injection criterion: new server/lib/spawn-model-resolver.js resolves the spawn-time model from the ACTIVE configuration (workspace active proxy profile mapping > env CLAUDE_MODEL/ANTHROPIC_MODEL > settings.json env.ANTHROPIC_MODEL > top-level model; alias "default" is no signal; no signal -> no injection), replacing the stale lastModelUsage statistic that force-injected third-party override prompts into unconfigured sessions. readClaudeProjectModel stays for context-bar calibration. All 18 README locales updated (Arabic priority chain spelled in words to avoid RTL bidi mirroring of ">"). Tiered boot fallback for injected system prompts: tier 1 retries once WITHOUT injection on non-signal exit!=0 within a 5s boot window via a one-shot skip token consumed at the top of the next spawn (permanent rejected set reserved for the deterministic "unknown option" signal, which still self-heals IM workers); !signal and !insideLogDir gates keep user kills and IM personas out. Tier 2 prints a diagnostic notice on quick exit-0 without auto-restart or auto-disable. PR #128 integration (supersedes its branch): findcc resolveNpmClaudePath falls back to the package bin/claude(.exe) for the Claude Code 2.x layout that ships no cli.js (extracted findNpmBinFallback; claude.exe is the real bin filename on every platform in 2.1.x); pty-manager wraps the whole system-prompt pipeline in try-catch and registers PTY handlers immediately after spawn; create_system_prompt runs git via spawnSync with a 15s timeout; server.js's listen callback wraps its async setup in a caught IIFE so post-bind rejections can't kill the process. Two review rounds (six roles each) applied: exactly-once token consumption, weakened causality wording in retry notices, corrected SIGKILL claim in findcc comments (verified same-inode hard link), 2.x-fallback and null-guard test coverage, history.md dedup. Tests: npm run test 7883 pass, npm run test:cli 8242 pass, 0 fail. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Four guard layers so no failure in the system-prompt build/render path can crash the process or block the claude spawn.
Changes
server.js listen callback — non-async wrapper + try-catch IIFE. Any await rejection (setupTerminalWebSocket / runParallelHook / startBridge) is caught and logged; resolve(server) always fires so the bound port never becomes an unhandled-rejection crash after cli.js passes the port check and proceeds to spawnClaude.
pty-manager.js spawnClaude — wrap the entire system-prompt pipe (
_spawnModelReader→buildSystemPromptFileArgs→renderSystemPromptFileArgs) in try-catch; any throw falls back to{args:[], loaded:[], model:null}so claude launches with its own default system prompt.system-prompt-render.js — defensive null/type guard at function entry returns a safe default instead of throwing TypeError.
create_system_prompt.js —
execFileSync→spawnSyncwith 15s timeout so a hanging git subprocess (NFS / huge repo / broken index) cannot block the spawn forever.Test Results
npm run test: 7867 pass, 0 failnpm run test:cli: 8248 pass, 0 fail🤖 Generated with Claude Code