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>
This commit is contained in:
Tom Boucher
2026-05-13 18:33:32 -04:00
committed by GitHub
parent ba625c0978
commit 639e4d603a
13 changed files with 161 additions and 223 deletions

View File

@@ -0,0 +1,5 @@
---
type: Changed
pr: 3476
---
Migrate all subprocess call sites to the shell-command-projection seam (`execGit`, `execNpm`, `execTool`, `probeTty`). Removes scattered platform-conditional logic and normalizes subprocess error handling. Local `execGit` wrapper removed from `core.cjs`; `verify.cjs` and `worktree-safety.cjs` migrated to the seam's `(args, opts)` signature. See #3466.

View File

@@ -20,7 +20,7 @@
* Text Matching on Test Outputs".
*/
const cp = require('node:child_process');
const { execNpm } = require('./lib/shell-command-projection.cjs');
// Hardcoded. Do not parameterise — the whole point of this script is that
// the package name is not a runtime choice for the caller.
@@ -36,19 +36,24 @@ const SEMVER_RE = /^\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?$/;
/**
* Pure-ish: takes an injected spawn function so tests don't actually run npm.
* In production, defaults to cp.spawnSync('npm', ...).
* In production, defaults to execNpm() from the shell-projection seam.
*/
function checkLatestVersion(opts = {}) {
const defaultSpawn = () => cp.spawnSync('npm', ['view', PACKAGE_NAME, 'version'], {
encoding: 'utf8',
stdio: ['ignore', 'pipe', 'pipe'],
shell: process.platform === 'win32', // npm is npm.cmd on Windows
// Bound the registry call so a hung network/registry doesn't block the
// entire /gsd-update workflow indefinitely (#2993 CR). 15s is generous
// for `npm view <pkg> version`; on timeout, spawnSync returns with
// signal !== null and the existing failure path emits FAIL_NPM_FAILED.
timeout: 15_000,
});
// Default path routes through the shell-projection seam (execNpm owns the
// Windows shell-flag policy and timeout default). The injection point
// remains spawnSync-shaped for test compatibility — the adapter below
// translates { exitCode } → { status } so the consumer logic is unchanged.
// Bounded at 15s so a hung registry doesn't block /gsd-update (#2993 CR).
const defaultSpawn = () => {
const r = execNpm(['view', PACKAGE_NAME, 'version'], { timeout: 15_000 });
return {
status: r.exitCode,
stdout: r.stdout,
stderr: r.stderr,
signal: r.signal,
error: r.error,
};
};
const spawn = opts.spawn || defaultSpawn;
const r = spawn();

View File

@@ -3,8 +3,8 @@
*/
const fs = require('fs');
const path = require('path');
const { execSync } = require('child_process');
const { safeReadFile, loadConfig, isGitIgnored, execGit, normalizePhaseName, comparePhaseNum, getArchivedPhaseDirs, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, resolveModelInternal, stripShippedMilestones, extractCurrentMilestone, toPosixPath, output, error, findPhaseInternal, extractOneLinerFromBody, getRoadmapPhaseInternal } = require('./core.cjs');
const { execGit } = require('./shell-command-projection.cjs');
const { safeReadFile, loadConfig, isGitIgnored, normalizePhaseName, comparePhaseNum, getArchivedPhaseDirs, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, resolveModelInternal, stripShippedMilestones, extractCurrentMilestone, toPosixPath, output, error, findPhaseInternal, extractOneLinerFromBody, getRoadmapPhaseInternal } = require('./core.cjs');
const { planningDir, planningPaths } = require('./planning-workspace.cjs');
const { extractFrontmatter } = require('./frontmatter.cjs');
const { MODEL_PROFILES } = require('./model-profiles.cjs');
@@ -302,12 +302,12 @@ function cmdCommit(cwd, message, files, raw, amend, noVerify) {
}
}
if (branchName) {
const currentBranch = execGit(cwd, ['rev-parse', '--abbrev-ref', 'HEAD']);
const currentBranch = execGit(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd });
if (currentBranch.exitCode === 0 && currentBranch.stdout.trim() !== branchName) {
// Create branch if it doesn't exist, or switch to it if it does
const create = execGit(cwd, ['checkout', '-b', branchName]);
const create = execGit(['checkout', '-b', branchName], { cwd });
if (create.exitCode !== 0) {
execGit(cwd, ['checkout', branchName]);
execGit(['checkout', branchName], { cwd });
}
}
}
@@ -327,29 +327,34 @@ function cmdCommit(cwd, message, files, raw, amend, noVerify) {
}
// Default mode (staging all of .planning/): stage the deletion so
// removed planning files are not left dangling in the index.
execGit(cwd, ['rm', '--cached', '--ignore-unmatch', file]);
execGit(['rm', '--cached', '--ignore-unmatch', file], { cwd });
} else {
execGit(cwd, ['add', file]);
execGit(['add', file], { cwd });
}
}
// Commit (--no-verify skips pre-commit hooks, used by parallel executor agents)
const commitArgs = amend ? ['commit', '--amend', '--no-edit'] : ['commit', '-m', message];
if (noVerify) commitArgs.push('--no-verify');
const commitResult = execGit(cwd, commitArgs);
const commitResult = execGit(commitArgs, { cwd });
if (commitResult.exitCode !== 0) {
if (commitResult.stdout.includes('nothing to commit') || commitResult.stderr.includes('nothing to commit')) {
const result = { committed: false, hash: null, reason: 'nothing_to_commit' };
output(result, raw, 'nothing');
return;
}
const result = { committed: false, hash: null, reason: 'nothing_to_commit', error: commitResult.stderr };
output(result, raw, 'nothing');
const result = {
committed: false,
hash: null,
reason: 'commit_failed',
error: commitResult.stderr || commitResult.stdout,
};
output(result, raw, 'failed');
return;
}
// Get short hash
const hashResult = execGit(cwd, ['rev-parse', '--short', 'HEAD']);
const hashResult = execGit(['rev-parse', '--short', 'HEAD'], { cwd });
const hash = hashResult.exitCode === 0 ? hashResult.stdout : null;
const result = { committed: true, hash, reason: 'committed' };
output(result, raw, hash || 'committed');
@@ -395,11 +400,11 @@ function cmdCommitToSubrepo(cwd, message, files, raw) {
// Stage files (strip sub-repo prefix for paths relative to that repo)
for (const file of repoFiles) {
const relativePath = file.slice(repo.length + 1);
execGit(repoCwd, ['add', relativePath]);
execGit(['add', relativePath], { cwd: repoCwd });
}
// Commit
const commitResult = execGit(repoCwd, ['commit', '-m', message]);
const commitResult = execGit(['commit', '-m', message], { cwd: repoCwd });
if (commitResult.exitCode !== 0) {
if (commitResult.stdout.includes('nothing to commit') || commitResult.stderr.includes('nothing to commit')) {
repos[repo] = { committed: false, hash: null, files: repoFiles, reason: 'nothing_to_commit' };
@@ -410,7 +415,7 @@ function cmdCommitToSubrepo(cwd, message, files, raw) {
}
// Get hash
const hashResult = execGit(repoCwd, ['rev-parse', '--short', 'HEAD']);
const hashResult = execGit(['rev-parse', '--short', 'HEAD'], { cwd: repoCwd });
const hash = hashResult.exitCode === 0 ? hashResult.stdout : null;
repos[repo] = { committed: true, hash, files: repoFiles };
}
@@ -914,14 +919,14 @@ function cmdStats(cwd, format, raw) {
// Git stats
let gitCommits = 0;
let gitFirstCommitDate = null;
const commitCount = execGit(cwd, ['rev-list', '--count', 'HEAD']);
const commitCount = execGit(['rev-list', '--count', 'HEAD'], { cwd });
if (commitCount.exitCode === 0) {
gitCommits = parseInt(commitCount.stdout, 10) || 0;
}
const rootHash = execGit(cwd, ['rev-list', '--max-parents=0', 'HEAD']);
const rootHash = execGit(['rev-list', '--max-parents=0', 'HEAD'], { cwd });
if (rootHash.exitCode === 0 && rootHash.stdout) {
const firstCommit = rootHash.stdout.split('\n')[0].trim();
const firstDate = execGit(cwd, ['show', '-s', '--format=%as', firstCommit]);
const firstDate = execGit(['show', '-s', '--format=%as', firstCommit], { cwd });
if (firstDate.exitCode === 0) {
gitFirstCommitDate = firstDate.stdout || null;
}
@@ -990,9 +995,9 @@ function cmdCheckCommit(cwd, raw) {
}
// commit_docs is false — check if any .planning/ files are staged
try {
const staged = execSync('git diff --cached --name-only', { cwd, encoding: 'utf-8' }).trim();
const planningFiles = staged.split('\n').filter(f => f.startsWith('.planning/') || f.startsWith('.planning\\'));
const stagedResult = execGit(['diff', '--cached', '--name-only'], { cwd });
if (stagedResult.exitCode === 0) {
const planningFiles = stagedResult.stdout.split('\n').filter(f => f.startsWith('.planning/') || f.startsWith('.planning\\'));
if (planningFiles.length > 0) {
error(
@@ -1001,9 +1006,8 @@ function cmdCheckCommit(cwd, raw) {
`\n\nTo unstage: git reset HEAD ${planningFiles.join(' ')}`
);
}
} catch {
// git diff --cached failed (no staged files or not a git repo) — allow
}
// exitCode !== 0 → no staged files or not a git repo — allow
output({ allowed: true, reason: 'no_planning_files_staged' }, raw, 'allowed');
}

View File

@@ -5,7 +5,7 @@
const fs = require('fs');
const os = require('os');
const path = require('path');
const { execSync, execFileSync, spawnSync } = require('child_process');
const { execGit } = require('./shell-command-projection.cjs');
const { MODEL_PROFILES, AGENT_TO_PHASE_TYPE, VALID_PHASE_TYPES, AGENT_DEFAULT_TIERS, VALID_AGENT_TIERS, nextTier } = require('./model-profiles.cjs');
const { MODEL_ALIAS_MAP, RUNTIME_PROFILE_MAP, KNOWN_RUNTIMES, RUNTIMES_WITH_REASONING_EFFORT } = require('./model-catalog.cjs');
const {
@@ -593,23 +593,16 @@ const _gitIgnoredCache = new Map();
function isGitIgnored(cwd, targetPath) {
const key = cwd + '::' + targetPath;
if (_gitIgnoredCache.has(key)) return _gitIgnoredCache.get(key);
try {
// --no-index checks .gitignore rules regardless of whether the file is tracked.
// Without it, git check-ignore returns "not ignored" for tracked files even when
// .gitignore explicitly lists them — a common source of confusion when .planning/
// was committed before being added to .gitignore.
// Use execFileSync (array args) to prevent shell interpretation of special characters
// in file paths — avoids command injection via crafted path names.
execFileSync('git', ['check-ignore', '-q', '--no-index', '--', targetPath], {
cwd,
stdio: 'pipe',
});
_gitIgnoredCache.set(key, true);
return true;
} catch {
_gitIgnoredCache.set(key, false);
return false;
}
// --no-index checks .gitignore rules regardless of whether the file is tracked.
// Without it, git check-ignore returns "not ignored" for tracked files even when
// .gitignore explicitly lists them — a common source of confusion when .planning/
// was committed before being added to .gitignore.
// Array args (via the seam) prevent shell interpretation of special characters in
// file paths — avoids command injection via crafted path names.
const result = execGit(['check-ignore', '-q', '--no-index', '--', targetPath], { cwd });
const ignored = result.exitCode === 0;
_gitIgnoredCache.set(key, ignored);
return ignored;
}
// ─── Markdown normalization ─────────────────────────────────────────────────
@@ -724,39 +717,6 @@ function normalizeMd(content) {
// Default timeout for worktree-related git subprocess calls (matches worktree-safety.cjs).
// Prevents `git worktree list --porcelain` and similar calls from blocking the parent
// process indefinitely when git is stalled (locked index, hung remote, NFS mount freeze).
// Callers can override via an options bag if needed.
const DEFAULT_GIT_TIMEOUT_MS = 10000;
/**
* Execute a git command with a bounded timeout.
*
* Return shape: { exitCode, stdout, stderr, timedOut, error }
* - timedOut: true when spawnSync reports SIGTERM + ETIMEDOUT — callers must
* branch on this to surface a structured warning (PRED.k302).
* - error: spawnSync error object or null
*
* Backward-compatible: existing callers that only read exitCode/stdout/stderr
* continue to work unchanged.
*/
function execGit(cwd, args, options = {}) {
const timeout = options.timeout ?? DEFAULT_GIT_TIMEOUT_MS;
const result = spawnSync('git', args, {
cwd,
stdio: 'pipe',
encoding: 'utf-8',
timeout,
});
const timedOut = result.signal === 'SIGTERM' && result.error?.code === 'ETIMEDOUT';
return {
exitCode: result.status ?? 1,
stdout: (result.stdout ?? '').toString().trim(),
stderr: (result.stderr ?? '').toString().trim(),
timedOut,
error: result.error ?? null,
};
}
// ─── Common path helpers ──────────────────────────────────────────────────────
/**
@@ -765,8 +725,10 @@ function execGit(cwd, args, options = {}) {
* Returns the main worktree path, or cwd if not in a worktree.
*/
function resolveWorktreeRoot(cwd) {
// Omit execGit so worktree-safety uses its own execGitDefault — that wrapper
// delegates to the seam and derives the `timedOut` field that pruneResult
// branches on below.
const context = resolveWorktreeContext(cwd, {
execGit,
existsSync: fs.existsSync,
});
return context.effectiveRoot;
@@ -798,9 +760,9 @@ function pruneOrphanedWorktrees(repoRoot) {
const plan = planWorktreePrune(
repoRoot,
{ allowDestructive: false },
{ execGit, parseWorktreePorcelain }
{ parseWorktreePorcelain }
);
const pruneResult = executeWorktreePrunePlan(plan, { execGit });
const pruneResult = executeWorktreePrunePlan(plan);
if (pruneResult && pruneResult.timedOut) {
// AC2: surface structured warning instead of silently swallowing the timeout.
// Uses process.stderr.write to match the [gsd-tools] WARNING prefix style.
@@ -1980,7 +1942,6 @@ module.exports = {
safeReadFile,
loadConfig,
isGitIgnored,
execGit,
normalizeMd,
escapeRegex,
normalizePhaseName,

View File

@@ -2,8 +2,8 @@
const fs = require('fs');
const path = require('path');
const childProcess = require('child_process');
const { atomicWriteFileSync, execGit } = require('./core.cjs');
const { execTool, execGit } = require('./shell-command-projection.cjs');
const { atomicWriteFileSync } = require('./core.cjs');
// ─── Config Gate ─────────────────────────────────────────────────────────────
@@ -58,15 +58,13 @@ const GRAPHIFY_REASON = Object.freeze({
function execGraphify(cwd, args, options = {}) {
const timeout = options.timeout ?? 30000;
const result = childProcess.spawnSync('graphify', args, {
const result = execTool('graphify', args, {
cwd,
stdio: 'pipe',
encoding: 'utf-8',
timeout,
env: { ...process.env, PYTHONUNBUFFERED: '1' },
});
// ENOENT -- graphify binary not found on PATH
// ENOENT — seam normalizes to exitCode 127. Surface as typed reason.
if (result.error && result.error.code === 'ENOENT') {
return {
exitCode: 127,
@@ -76,23 +74,22 @@ function execGraphify(cwd, args, options = {}) {
};
}
// Timeout -- subprocess killed via SIGTERM
// Timeout — seam exposes signal; spawnSync sets SIGTERM when killed by timeout.
if (result.signal === 'SIGTERM') {
return {
exitCode: 124,
stdout: (result.stdout ?? '').toString().trim(),
stdout: result.stdout,
stderr: 'graphify timed out after ' + timeout + 'ms',
reason: GRAPHIFY_REASON.TIMEOUT,
timeout_ms: timeout,
};
}
const exitCode = result.status ?? 1;
return {
exitCode,
stdout: (result.stdout ?? '').toString().trim(),
stderr: (result.stderr ?? '').toString().trim(),
reason: exitCode === 0 ? GRAPHIFY_REASON.OK : GRAPHIFY_REASON.EXIT_NONZERO,
exitCode: result.exitCode,
stdout: result.stdout,
stderr: result.stderr,
reason: result.exitCode === 0 ? GRAPHIFY_REASON.OK : GRAPHIFY_REASON.EXIT_NONZERO,
};
}
@@ -105,11 +102,7 @@ function execGraphify(cwd, args, options = {}) {
* @returns {{ installed: boolean, message?: string }}
*/
function checkGraphifyInstalled() {
const result = childProcess.spawnSync('graphify', ['--help'], {
stdio: 'pipe',
encoding: 'utf-8',
timeout: 5000,
});
const result = execTool('graphify', ['--help'], { timeout: 5000 });
if (result.error) {
return {
@@ -134,18 +127,13 @@ function checkGraphifyInstalled() {
*/
function checkGraphifyVersion() {
// Strategy 1: try `graphify --version` directly (2s timeout -- fast path)
const versionResult = childProcess.spawnSync('graphify', ['--version'], {
stdio: 'pipe',
encoding: 'utf-8',
timeout: 2000,
});
const versionResult = execTool('graphify', ['--version'], { timeout: 2000 });
let versionStr = null;
if (!versionResult.error && versionResult.status === 0) {
const raw = (versionResult.stdout || '').trim();
if (!versionResult.error && versionResult.exitCode === 0) {
// graphify --version may emit "graphify 0.4.23" or just "0.4.23"
const match = raw.match(/(\d+\.\d+(?:\.\d+)*)/);
const match = versionResult.stdout.match(/(\d+\.\d+(?:\.\d+)*)/);
if (match) {
versionStr = match[1];
}
@@ -153,17 +141,13 @@ function checkGraphifyVersion() {
// Strategy 2: fall back to python3 importlib.metadata
if (!versionStr) {
const pyResult = childProcess.spawnSync('python3', [
const pyResult = execTool('python3', [
'-c',
'from importlib.metadata import version; print(version("graphifyy"))',
], {
stdio: 'pipe',
encoding: 'utf-8',
timeout: 5000,
});
], { timeout: 5000 });
if (!pyResult.error && pyResult.status === 0 && pyResult.stdout && pyResult.stdout.trim()) {
versionStr = pyResult.stdout.trim();
if (!pyResult.error && pyResult.exitCode === 0 && pyResult.stdout) {
versionStr = pyResult.stdout;
}
}
@@ -370,7 +354,7 @@ const COMMIT_HASH_RE = /^[0-9a-f]{4,40}$/i;
* success, or null when cwd is not a git repo / `git` is not on PATH.
*/
function readGitHead(cwd) {
const r = execGit(cwd, ['rev-parse', 'HEAD']);
const r = execGit(['rev-parse', 'HEAD'], { cwd });
if (r.exitCode !== 0) return null;
return r.stdout.trim() || null;
}
@@ -381,7 +365,7 @@ function readGitHead(cwd) {
* or the cwd is not a git repo.
*/
function countCommitsBetween(cwd, from, to) {
const r = execGit(cwd, ['rev-list', '--count', `${from}..${to}`]);
const r = execGit(['rev-list', '--count', `${from}..${to}`], { cwd });
if (r.exitCode !== 0) return null;
const n = parseInt(r.stdout.trim(), 10);
return Number.isFinite(n) ? n : null;

View File

@@ -4,7 +4,7 @@
const fs = require('fs');
const path = require('path');
const { execSync } = require('child_process');
const { execGit } = require('./shell-command-projection.cjs');
const { loadConfig, resolveModelInternal, findPhaseInternal, getRoadmapPhaseInternal, pathExistsInternal, generateSlugInternal, getMilestoneInfo, getMilestonePhaseFilter, stripShippedMilestones, extractCurrentMilestone, normalizePhaseName, toPosixPath, output, error, checkAgentsInstalled, phaseTokenMatches } = require('./core.cjs');
const { planningPaths, planningDir, planningRoot } = require('./planning-workspace.cjs');
const { maskIfSecret } = require('./secrets.cjs');
@@ -1511,11 +1511,8 @@ function detectChildRepos(dir) {
const fullPath = path.join(dir, entry.name);
const gitDir = path.join(fullPath, '.git');
if (fs.existsSync(gitDir)) {
let hasUncommitted = false;
try {
const status = execSync('git status --porcelain', { cwd: fullPath, encoding: 'utf8', timeout: 5000 });
hasUncommitted = status.trim().length > 0;
} catch { /* best-effort */ }
const statusResult = execGit(['status', '--porcelain'], { cwd: fullPath, timeout: 5000 });
const hasUncommitted = statusResult.exitCode === 0 && statusResult.stdout.length > 0;
repos.push({ name: entry.name, path: fullPath, has_uncommitted: hasUncommitted });
}
}
@@ -1530,11 +1527,8 @@ function cmdInitNewWorkspace(cwd, raw) {
const childRepos = detectChildRepos(cwd);
// Check if git worktree is available
let worktreeAvailable = false;
try {
execSync('git --version', { encoding: 'utf8', timeout: 5000, stdio: 'pipe' });
worktreeAvailable = true;
} catch { /* no git at all */ }
const gitVersion = execGit(['--version'], { timeout: 5000 });
const worktreeAvailable = gitVersion.exitCode === 0;
const result = {
default_workspace_base: defaultBase,
@@ -1634,12 +1628,10 @@ function cmdInitRemoveWorkspace(cwd, name, raw) {
for (const repo of repos) {
const repoPath = path.join(wsPath, repo.name);
if (!fs.existsSync(repoPath)) continue;
try {
const status = execSync('git status --porcelain', { cwd: repoPath, encoding: 'utf8', timeout: 5000, stdio: 'pipe' });
if (status.trim().length > 0) {
dirtyRepos.push(repo.name);
}
} catch { /* best-effort */ }
const statusResult = execGit(['status', '--porcelain'], { cwd: repoPath, timeout: 5000 });
if (statusResult.exitCode === 0 && statusResult.stdout.length > 0) {
dirtyRepos.push(repo.name);
}
}
const result = {

View File

@@ -11,7 +11,7 @@ const fs = require('fs');
const os = require('os');
const path = require('path');
const crypto = require('crypto');
const { execFileSync } = require('child_process');
const { probeTty } = require('./shell-command-projection.cjs');
const { isValidActiveWorkstreamName } = require('./workstream-name-policy.cjs');
const WORKSTREAM_SESSION_ENV_KEYS = [
@@ -92,16 +92,11 @@ function probeControllingTtyToken() {
return cachedControllingTtyToken;
}
try {
const ttyPath = execFileSync('tty', [], {
encoding: 'utf-8',
stdio: ['inherit', 'pipe', 'ignore'],
}).trim();
if (ttyPath && ttyPath !== 'not a tty') {
const token = sanitizeWorkstreamSessionToken(ttyPath.replace(/^\/dev\//, ''));
if (token) cachedControllingTtyToken = `tty-${token}`;
}
} catch {}
const ttyPath = probeTty();
if (ttyPath) {
const token = sanitizeWorkstreamSessionToken(ttyPath.replace(/^\/dev\//, ''));
if (token) cachedControllingTtyToken = `tty-${token}`;
}
return cachedControllingTtyToken;
}

View File

@@ -2,7 +2,10 @@
const path = require('path');
const fs = require('fs');
const { spawnSync, execFileSync } = require('child_process');
// Use non-destructured access so test-time mock.method(childProcess, 'spawnSync')
// can intercept calls from this seam — destructured imports capture references
// at load time and become un-mockable.
const childProcess = require('child_process');
/**
* Shell Command Projection Module
@@ -358,18 +361,30 @@ function formatSdkPathDiagnostic({ shimDir, platform, runDir }) {
function _spawnResult(result, program) {
if (result.error && result.error.code === 'ENOENT') {
return { exitCode: 127, stdout: '', stderr: `${program}: not found` };
return { exitCode: 127, stdout: '', stderr: `${program}: not found`, signal: null, error: result.error };
}
return {
exitCode: result.status ?? 1,
stdout: (result.stdout ?? '').toString().trim(),
stderr: (result.stderr ?? '').toString().trim(),
signal: result.signal ?? null,
error: result.error ?? null,
};
}
function execGit(args, opts = {}) {
const result = spawnSync('git', args, {
// Non-interactive defaults: a hung credential prompt or terminal-input
// probe must surface as a timeout, not block the tool forever. Callers
// can override via opts.env.
const env = {
...process.env,
GIT_TERMINAL_PROMPT: '0',
GCM_INTERACTIVE: 'never',
...(opts.env || {}),
};
const result = childProcess.spawnSync('git', args, {
cwd: opts.cwd,
env,
encoding: 'utf-8',
stdio: 'pipe',
timeout: opts.timeout ?? 10_000,
@@ -378,7 +393,7 @@ function execGit(args, opts = {}) {
}
function execNpm(args, opts = {}) {
const result = spawnSync('npm', args, {
const result = childProcess.spawnSync('npm', args, {
cwd: opts.cwd,
shell: process.platform === 'win32',
encoding: 'utf-8',
@@ -389,8 +404,9 @@ function execNpm(args, opts = {}) {
}
function execTool(program, args, opts = {}) {
const result = spawnSync(program, args, {
const result = childProcess.spawnSync(program, args, {
cwd: opts.cwd,
env: opts.env ? { ...process.env, ...opts.env } : undefined,
encoding: 'utf-8',
stdio: 'pipe',
timeout: opts.timeout ?? 30_000,
@@ -402,7 +418,7 @@ function probeTty(opts = {}) {
const platform = opts.platform ?? process.platform;
if (platform === 'win32') return null;
try {
const ttyPath = execFileSync('tty', [], {
const ttyPath = childProcess.execFileSync('tty', [], {
encoding: 'utf-8',
stdio: ['inherit', 'pipe', 'ignore'],
}).trim();

View File

@@ -5,7 +5,8 @@
const fs = require('fs');
const path = require('path');
const os = require('os');
const { safeReadFile, loadConfig, normalizePhaseName, escapeRegex, execGit, findPhaseInternal, getMilestoneInfo, stripShippedMilestones, extractCurrentMilestone, output, error, checkAgentsInstalled, CONFIG_DEFAULTS, inspectWorktreeHealth } = require('./core.cjs');
const { safeReadFile, loadConfig, normalizePhaseName, escapeRegex, findPhaseInternal, getMilestoneInfo, stripShippedMilestones, extractCurrentMilestone, output, error, checkAgentsInstalled, CONFIG_DEFAULTS, inspectWorktreeHealth } = require('./core.cjs');
const { execGit } = require('./shell-command-projection.cjs');
const { planningDir } = require('./planning-workspace.cjs');
const { extractFrontmatter, parseMustHavesBlock } = require('./frontmatter.cjs');
const { writeStateMd } = require('./state.cjs');
@@ -68,8 +69,8 @@ function cmdVerifySummary(cwd, summaryPath, checkFileCount, raw) {
let commitsExist = false;
if (hashes.length > 0) {
for (const hash of hashes.slice(0, 3)) {
const result = execGit(cwd, ['cat-file', '-t', hash]);
if (result.exitCode === 0 && result.stdout === 'commit') {
const result = execGit(['cat-file', '-t', hash], { cwd });
if (result.exitCode === 0 && result.stdout.trim() === 'commit') {
commitsExist = true;
break;
}
@@ -265,7 +266,7 @@ function cmdVerifyCommits(cwd, hashes, raw) {
const valid = [];
const invalid = [];
for (const hash of hashes) {
const result = execGit(cwd, ['cat-file', '-t', hash]);
const result = execGit(['cat-file', '-t', hash], { cwd });
if (result.exitCode === 0 && result.stdout.trim() === 'commit') {
valid.push(hash);
} else {
@@ -1226,7 +1227,7 @@ function cmdVerifySchemaDrift(cwd, phaseArg, skipFlag, raw) {
}
// Also check git commit messages for push evidence
const gitLog = execGit(cwd, ['log', '--oneline', '--all', '-50']);
const gitLog = execGit(['log', '--oneline', '--all', '-50'], { cwd });
if (gitLog.exitCode === 0) {
executionLog += '\n' + gitLog.stdout;
}
@@ -1288,7 +1289,7 @@ function cmdVerifyCodebaseDrift(cwd, raw) {
const lastMapped = drift.readMappedCommit(structurePath);
// Verify we're inside a git repo and resolve the diff range.
const revProbe = execGit(cwd, ['rev-parse', 'HEAD']);
const revProbe = execGit(['rev-parse', 'HEAD'], { cwd });
if (revProbe.exitCode !== 0) {
emit({
skipped: true,
@@ -1307,11 +1308,11 @@ function cmdVerifyCodebaseDrift(cwd, raw) {
base = EMPTY_TREE;
} else {
// Verify the commit is reachable; if not, fall back to EMPTY_TREE.
const verify = execGit(cwd, ['cat-file', '-t', base]);
const verify = execGit(['cat-file', '-t', base], { cwd });
if (verify.exitCode !== 0) base = EMPTY_TREE;
}
const diff = execGit(cwd, ['diff', '--name-status', base, 'HEAD']);
const diff = execGit(['diff', '--name-status', base, 'HEAD'], { cwd });
if (diff.exitCode !== 0) {
emit({
skipped: true,

View File

@@ -6,7 +6,7 @@
const fs = require('fs');
const path = require('path');
const { spawnSync } = require('child_process');
const { execGit: execGitSeam } = require('./shell-command-projection.cjs');
// Default timeout for worktree-related git subprocess calls.
// 10 s is generous enough for normal git operations on large repos while still
@@ -15,42 +15,17 @@ const { spawnSync } = require('child_process');
const DEFAULT_GIT_TIMEOUT_MS = 10000;
/**
* Execute a git command with a bounded timeout.
* Execute a git command via the shell-projection seam, with a derived
* `timedOut` field. Tests inject mocks via deps.execGit using the new
* (args, opts) shape — see worktree-safety-policy.test.cjs.
*
* Return shape: { exitCode, stdout, stderr, timedOut, error }
* - exitCode: process exit status (null when killed by signal)
* - timedOut: true when spawnSync reports SIGTERM + ETIMEDOUT — callers must
* branch on this to surface a structured warning instead of
* silently treating the empty output as success (PRED.k302)
* - error: the Error object from spawnSync when the process could not start
* or was killed; null otherwise
*
* Backward-compatible: existing callers that only read exitCode/stdout/stderr
* continue to work unchanged.
* Return shape: { exitCode, stdout, stderr, timedOut, error, signal }
* - timedOut: true when spawnSync reports SIGTERM + ETIMEDOUT
*/
function execGitDefault(cwd, args, options = {}) {
const timeout = options.timeout ?? DEFAULT_GIT_TIMEOUT_MS;
const result = spawnSync('git', args, {
cwd,
stdio: 'pipe',
encoding: 'utf-8',
timeout,
env: {
...process.env,
GIT_TERMINAL_PROMPT: '0',
GCM_INTERACTIVE: 'never',
},
});
// spawnSync sets signal='SIGTERM' and error.code='ETIMEDOUT' when the timeout
// fires and the subprocess is killed.
function execGitDefault(args, opts = {}) {
const result = execGitSeam(args, { ...opts, timeout: opts.timeout ?? DEFAULT_GIT_TIMEOUT_MS });
const timedOut = result.signal === 'SIGTERM' && result.error?.code === 'ETIMEDOUT';
return {
exitCode: result.status ?? 1,
stdout: (result.stdout ?? '').toString().trim(),
stderr: (result.stderr ?? '').toString().trim(),
timedOut,
error: result.error ?? null,
};
return { ...result, timedOut };
}
function parseWorktreePorcelain(porcelain) {
@@ -82,7 +57,7 @@ function parseWorktreeListPaths(porcelain) {
function readWorktreeList(repoRoot, deps = {}) {
const execGit = deps.execGit || execGitDefault;
const listResult = execGit(repoRoot, ['worktree', 'list', '--porcelain']);
const listResult = execGit(['worktree', 'list', '--porcelain'], { cwd: repoRoot });
if (listResult.timedOut) {
// AC2 / AC4: surface timeout as a distinct reason so callers can emit a
// structured warning rather than silently treating the failure as a generic
@@ -127,8 +102,8 @@ function resolveWorktreeContext(cwd, deps = {}) {
};
}
const gitDir = execGit(cwd, ['rev-parse', '--git-dir']);
const commonDir = execGit(cwd, ['rev-parse', '--git-common-dir']);
const gitDir = execGit(['rev-parse', '--git-dir'], { cwd });
const commonDir = execGit(['rev-parse', '--git-common-dir'], { cwd });
if (gitDir.exitCode !== 0 || commonDir.exitCode !== 0) {
return {
effectiveRoot: cwd,
@@ -203,7 +178,7 @@ function executeWorktreePrunePlan(plan, deps = {}) {
};
}
const result = execGit(plan.repoRoot, ['worktree', 'prune']);
const result = execGit(['worktree', 'prune'], { cwd: plan.repoRoot });
if (result.timedOut) {
// AC4: surface timedOut as a first-class field so callers (e.g.
// pruneOrphanedWorktrees in core.cjs) can log a structured WARNING rather
@@ -436,8 +411,8 @@ function executeWorktreeWaveCleanupPlan(plan, deps = {}) {
stderr: '',
};
const branchCheck = execGit(plan.repoRoot, ['-C', entry.worktree_path, 'rev-parse', '--abbrev-ref', 'HEAD']);
if (!gitResultOk(branchCheck) || branchCheck.stdout !== entry.branch) {
const branchCheck = execGit(['-C', entry.worktree_path, 'rev-parse', '--abbrev-ref', 'HEAD'], { cwd: plan.repoRoot });
if (!gitResultOk(branchCheck) || branchCheck.stdout.trim() !== entry.branch) {
result.status = 'blocked';
result.reason = 'branch_mismatch';
result.stderr = branchCheck?.stderr || '';
@@ -447,8 +422,8 @@ function executeWorktreeWaveCleanupPlan(plan, deps = {}) {
break;
}
const mergeBase = execGit(plan.repoRoot, ['merge-base', 'HEAD', entry.branch]);
if (!gitResultOk(mergeBase) || mergeBase.stdout !== entry.expected_base) {
const mergeBase = execGit(['merge-base', 'HEAD', entry.branch], { cwd: plan.repoRoot });
if (!gitResultOk(mergeBase) || mergeBase.stdout.trim() !== entry.expected_base) {
result.status = 'blocked';
result.reason = 'base_mismatch';
result.stderr = mergeBase?.stderr || '';
@@ -458,7 +433,7 @@ function executeWorktreeWaveCleanupPlan(plan, deps = {}) {
break;
}
const deletions = execGit(plan.repoRoot, ['diff', '--diff-filter=D', '--name-only', `HEAD...${entry.branch}`]);
const deletions = execGit(['diff', '--diff-filter=D', '--name-only', `HEAD...${entry.branch}`], { cwd: plan.repoRoot });
if (!gitResultOk(deletions)) {
result.status = 'blocked';
result.reason = 'deletion_check_failed';
@@ -478,7 +453,7 @@ function executeWorktreeWaveCleanupPlan(plan, deps = {}) {
break;
}
const worktreeStatus = execGit(plan.repoRoot, ['-C', entry.worktree_path, 'status', '--porcelain', '--untracked-files=all']);
const worktreeStatus = execGit(['-C', entry.worktree_path, 'status', '--porcelain', '--untracked-files=all'], { cwd: plan.repoRoot });
if (!gitResultOk(worktreeStatus) || worktreeStatus.stdout) {
result.status = 'blocked';
result.reason = 'worktree_dirty';
@@ -489,7 +464,7 @@ function executeWorktreeWaveCleanupPlan(plan, deps = {}) {
break;
}
const merge = execGit(plan.repoRoot, ['merge', entry.branch, '--no-ff', '--no-edit', '-m', `chore: merge executor worktree (${entry.branch})`]);
const merge = execGit(['merge', entry.branch, '--no-ff', '--no-edit', '-m', `chore: merge executor worktree (${entry.branch})`], { cwd: plan.repoRoot });
if (!gitResultOk(merge)) {
result.status = 'blocked';
result.reason = 'merge_failed';
@@ -500,7 +475,7 @@ function executeWorktreeWaveCleanupPlan(plan, deps = {}) {
break;
}
const remove = execGit(plan.repoRoot, ['worktree', 'remove', entry.worktree_path, '--force']);
const remove = execGit(['worktree', 'remove', entry.worktree_path, '--force'], { cwd: plan.repoRoot });
if (!gitResultOk(remove)) {
result.status = 'blocked';
result.reason = 'worktree_remove_failed';
@@ -511,7 +486,7 @@ function executeWorktreeWaveCleanupPlan(plan, deps = {}) {
break;
}
const branchDelete = execGit(plan.repoRoot, ['branch', '-D', entry.branch]);
const branchDelete = execGit(['branch', '-D', entry.branch], { cwd: plan.repoRoot });
if (!gitResultOk(branchDelete)) {
result.status = 'warning';
result.reason = 'branch_delete_failed';

View File

@@ -35,7 +35,7 @@ const WORKTREE_SAFETY_PATH = path.join(
* - not throw
*/
function makeTimeoutStub() {
return function stubTimedOutExecGit(_cwd, _args) {
return function stubTimedOutExecGit(_args, _opts) {
return {
exitCode: null,
stdout: '',

View File

@@ -87,8 +87,8 @@ describe('bug #3384: worktree cleanup is manifest-scoped and fail-closed', () =>
};
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (cwd, args) => {
calls.push({ cwd, args });
execGit: (args, opts) => {
calls.push({ cwd: opts.cwd, args });
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
@@ -141,7 +141,7 @@ describe('bug #3384: worktree cleanup is manifest-scoped and fail-closed', () =>
};
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (_cwd, args) => {
execGit: (args) => {
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {
return { exitCode: 0, stdout: 'worktree-agent-a1', stderr: '' };
@@ -184,7 +184,7 @@ describe('bug #3384: worktree cleanup is manifest-scoped and fail-closed', () =>
};
const result = executeWorktreeWaveCleanupPlan(plan, {
execGit: (_cwd, args) => {
execGit: (args) => {
calls.push(args.join(' '));
const key = args.join(' ');
if (key === '-C /repo/.claude/worktrees/agent-a1 rev-parse --abbrev-ref HEAD') {

View File

@@ -27,7 +27,7 @@ describe('worktree-safety policy module', () => {
test('resolveWorktreeContext maps linked worktree to common-dir parent', () => {
const context = resolveWorktreeContext('/repo/wt', {
existsSync: () => false,
execGit: (_, args) => {
execGit: (args) => {
if (args[1] === '--git-dir') return { exitCode: 0, stdout: '.git/worktrees/wt', stderr: '' };
if (args[1] === '--git-common-dir') return { exitCode: 0, stdout: '../.git', stderr: '' };
return { exitCode: 1, stdout: '', stderr: '' };
@@ -50,7 +50,7 @@ describe('worktree-safety policy module', () => {
test('resolveWorktreeContext keeps cwd for main worktree checkout', () => {
const context = resolveWorktreeContext('/repo/main', {
existsSync: () => false,
execGit: (_, args) => {
execGit: (args) => {
if (args[1] === '--git-dir') return { exitCode: 0, stdout: '.git', stderr: '' };
if (args[1] === '--git-common-dir') return { exitCode: 0, stdout: '.git', stderr: '' };
return { exitCode: 1, stdout: '', stderr: '' };
@@ -127,8 +127,8 @@ describe('worktree-safety policy module', () => {
const result = executeWorktreePrunePlan(
{ repoRoot: '/repo/main', action: 'metadata_prune_only', reason: 'worktrees_present' },
{
execGit: (cwd, args) => {
calls.push({ cwd, args });
execGit: (args, opts) => {
calls.push({ cwd: opts.cwd, args });
return { exitCode: 0, stdout: '', stderr: '' };
},
}