809c2bee39fc314fc97f56a6ea5954ebdec564a1
2 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Tom Boucher
|
639e4d603a |
refactor(shell-projection): migrate all subprocess call sites to exec*/probeTty seam (Phase 2, #3466) (#3476)
* refactor(shell-projection): migrate planning-workspace.cjs tty probe to probeTty seam (#3466) Replaces direct execFileSync('tty') with probeTty() from the shell-projection seam. Removes try/catch — probeTty() returns null on error/non-tty/win32. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(shell-projection): migrate commands.cjs to execGit seam (#3466) - Replaces execSync('git diff --cached --name-only') with execGit array call - Migrates 14 existing execGit(cwd, args) callers from core.cjs's local wrapper to the seam's execGit(args, { cwd }) signature - Drops execGit from the core.cjs destructure to resolve naming collision Drops try/catch around git diff — execGit returns exitCode without throwing, so the no-staged-files / not-a-git-repo case is detected by exitCode !== 0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(shell-projection): migrate check-latest-version.cjs to execNpm seam (#3466) Routes the default-spawn path through execNpm — execNpm owns the win32 shell-flag policy. The injection point remains spawnSync-shaped for test compatibility; an internal adapter translates { exitCode } → { status }. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(shell-projection): migrate init.cjs git calls to execGit seam (#3466) Replaces 3 execSync calls with execGit array-args: - detectChildRepos: git status --porcelain - cmdInitNewWorkspace: git --version (worktree availability probe) - cmdRemoveWorkspace: git status --porcelain Drops 3 try/catch blocks — execGit returns exitCode without throwing, so best-effort handling becomes a clean exitCode === 0 check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(shell-projection): migrate core.cjs to execGit seam delegation (#3466) - Removes direct require('child_process') from core.cjs - Replaces execFileSync('git check-ignore') with seam's execGit - Local execGit wrapper now a thin adapter delegating to seam — keeps the legacy (cwd, args) positional signature and derived timedOut field for the verify.cjs and worktree-safety.cjs consumers that are out of Phase 2 scope (the wrapper proper would only be removed once those consumers migrate, tracked separately) Extends the seam's _spawnResult to expose signal and error fields so callers can compute timedOut without bypassing the seam. The Phase 1 test suite asserts on required field presence only, so the extension is backward-compatible. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(shell-projection): migrate graphify.cjs to execTool seam (#3466) - execGraphify: spawnSync('graphify', ...) → execTool with env passthrough, preserving the ENOENT/TIMEOUT/EXIT_NONZERO typed reason mapping using the seam's signal/error fields - checkGraphifyInstalled: spawnSync('graphify', ['--help']) → execTool - checkGraphifyVersion strategy 1: graphify --version via execTool - checkGraphifyVersion strategy 2: python3 importlib.metadata via execTool Adds env option to execTool — graphify needs PYTHONUNBUFFERED=1 to drain buffered stdout on long-running operations. Changes seam internals to access spawnSync/execFileSync via the non-destructured childProcess module reference. Destructured imports capture references at load time and are un-mockable by mock.method(childProcess, 'spawnSync', ...) — which breaks all the graphify subprocess tests. Non-destructured access restores mockability without changing public behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(shell-projection): execGit defaults to non-interactive git env (#3466) Bakes GIT_TERMINAL_PROMPT=0 and GCM_INTERACTIVE=never into execGit's default env. Without these, a credential prompt or terminal-input probe blocks the git subprocess indefinitely until our 10s timeout kills it — surfacing as a generic timeout instead of the actual auth-prompt cause. These were previously set ad-hoc in worktree-safety.cjs's local execGitDefault wrapper. Moving them to the seam makes them the consistent default for every git call across the codebase. Callers can override via opts.env. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(shell-projection): remove core.cjs local execGit wrapper; migrate verify + worktree-safety (#3466) Completes the Phase 2 "remove local execGit wrapper" criterion. All callers now use the shell-projection seam's execGit(args, opts) signature directly. - core.cjs: delete the local execGit wrapper and the execGit export. The isGitIgnored seam check now calls execGit from the seam. Worktree-safety function calls drop their execGit DI passthrough — worktree-safety's internal execGitDefault now delegates to the seam and adds timedOut. - verify.cjs: 6 callers migrate from execGit(cwd, args) to execGit(args, { cwd }). Imports execGit from the seam directly. The inspectWorktreeHealth DI passes the seam's execGit (worktree-safety now matches that shape). - worktree-safety.cjs: local execGitDefault becomes a thin adapter over the seam — no more direct spawnSync. 11 internal callers migrate to the new shape. DI contract for tests changes from (cwd, args) → (args, opts). - graphify.cjs: 2 remaining execGit callers migrate from core.cjs (now removed) to the seam directly. - test mocks updated in 3 worktree-safety test files to match the new (args, opts) DI shape — most mocks were shape-agnostic and required no changes; only those that destructured cwd/args needed updates. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(changeset): add entry for shell-projection Phase 2 migration (#3466) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(pr3476): address CodeRabbit review findings --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
||
|
Tom Boucher
|
a33cbe72f5 |
fix(worktree): bound git subprocesses with timeout + surface degraded health (#3281) (#3283)
* test: red — bounded git subprocess + structured worktree warnings (#3281) Regression tests for #3281: worktree-related git subprocess calls have no timeout bound, and timeout/error outcomes are not surfaced as structured signals. Failing assertions: - planWorktreePrune / listLinkedWorktreePaths / snapshotWorktreeInventory must return reason=git_timed_out (not generic git_list_failed) when execGit returns timedOut:true — enables callers to distinguish timeout from auth failure - executeWorktreePrunePlan must include timedOut:true in result when the git prune call itself times out Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(worktree): bounded git subprocess + structured warning surfacing (#3281) Root cause (PRED.k014): execGit / execGitDefault called spawnSync with no timeout, so `git worktree list --porcelain` against a hung/locked repo blocked the parent process indefinitely. Downstream callers in core.cjs and verify.cjs then swallowed any resulting failure silently via catch { /* intentionally empty */ } (PRED.k302). Fix: - worktree-safety.cjs: execGitDefault now passes timeout:10000 to spawnSync. Detects SIGTERM+ETIMEDOUT and returns { timedOut:true } in the result shape. readWorktreeList maps timedOut:true -> reason:'git_timed_out' (distinct from generic git_list_failed) so callers can emit a structured warning. executeWorktreePrunePlan propagates timedOut:true as a first-class result field. - core.cjs: execGit receives the same timeout+timedOut treatment (PRED.k014 uniform-fix discipline). pruneOrphanedWorktrees now emits a [gsd-tools] WARNING to stderr when the git prune call times out instead of silent-catch. - verify.cjs: Check 11 branches on worktreeHealth.ok to surface W018 warning when the worktree list times out, instead of silent-catch on ok:false. Backward-compatible: exitCode/stdout/stderr continue to work for all existing callers; timedOut and error are additive new fields. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * changeset: pr=3283 for #3281 * fix(verify): rename W020 for worktree-timeout warning to avoid W018 collision W018 is already used for milestone archive drift (Check 12). The new worktree-health-degraded timeout warning was assigned W018, causing warning-code ambiguity in triage. Rename to W020 (next available code). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |