Bound synchronous workspace Git discovery commands with an explicit process policy

[mohanagy]

User story

As a developer, I want workspace discovery to finish within an explicit bound and explain any failure, so that Madar cannot hang or silently treat a failed Git command as a non-repository directory.

Acceptance criteria

• Every Git invocation owned by workspace discovery has a justified time bound and documented termination behavior.
• Callers can distinguish a non-repository directory from a timeout, missing Git or another command failure, with an attributable diagnostic.
• Existing root, worktree and non-Git behavior is preserved deliberately; affected callers and Ubuntu/macOS/Windows behavior are covered by appropriate deterministic checks.
• The correction introduces no retry, skipped assertion or blanket timeout increase, and closes only with integrated evidence for this behavior.

Status and boundary — 10 September 2026

Open; implementation remains paused. Bounding execution and identifying failure are one user outcome. Limit work to workspace discovery and its callers; this is not a repository-wide subprocess rewrite.

Static inspection still finds unbounded execution and catch-all fallback in next workspace discovery. No test was rerun for this triage.

Independent of the product-usefulness story #754. It blocks that work only if discovery actually prevents execution. Planning updates do not resume coding; future source/test implementation remains assigned to Codex CLI.

Historical issue body — preserved; current disposition above takes precedence

Current disposition — 5 September 2026

OPEN — independent process-policy maintenance. Current roadmap: #740. This issue retains its bounded workspace Git discovery policy scope and remains independent of #741's research and evaluation-contract preparation. It is not a demonstrated cause of the #736 product result and blocks a future evaluation only if workspace discovery actually prevents that evaluation. Refresh the invocation inventory before implementation. This reconciliation does not authorize coding or automatically resume implementation.

The original scope and acceptance criteria below, and all existing comments, are preserved as history.

---

Parent: #654
Surfaced by: review of PR #696 (#695)
Target branch: next

Outcome

Give synchronous workspace Git discovery an explicit process policy: a bounded execution time and an error contract that distinguishes "not a Git repository" from "Git was killed after exceeding its bound".

Both halves are required. Fixing either alone makes the code worse.

Confirmed evidence

src/shared/workspace.ts:30:

function gitPath(rootPath: string, args: string[]): string | null {
  try {
    const value = execFileSync('git', ['-C', rootPath, ...args], {
      encoding: 'utf8',
      stdio: ['ignore', 'pipe', 'pipe'],
      windowsHide: true,
    }).trim()
    return value.length > 0 ? value : null
  } catch {
    return null
  }
}

Verified directly: grep -c timeout src/shared/workspace.ts → 0, with 4 gitPath() call sites.

Two independent defects sit in those twelve lines.

No bound. execFileSync sets encoding, stdio and windowsHide but no timeout, so a wedged Git blocks the calling thread indefinitely. Every resolveMadarWorkspace() spawns up to three of these, and generateGraph() reaches the same path through resolveMadarOutputDirectory() (src/infrastructure/generate.ts:360). Any Madar command performing workspace discovery can hang with no recovery.

Errors are swallowed. catch { return null } collapses every failure — missing Git binary, permission error, corrupt repository, killed process — into the same null that legitimately means "this is not a Git checkout".

These interact, which is why they must be fixed together. Adding a timeout on its own would convert a hang into a silent null: the caller could not distinguish a directory that is not a repository from one where Git was killed mid-discovery, and Madar would proceed with a confidently wrong workspace resolution instead of hanging visibly. Trading a detectable hang for an undetectable wrong answer is not an improvement.

Why this is separate from #695

PR #696 is deliberately test-only. It bounds the Git commands its own fixture invokes and now states plainly that indirect discovery is unbounded — that boundary is documented rather than papered over.

The observed Windows failure that motivated #695 was an aggregate elapsed-time verdict, not a hung Git process: the slowest individual Git command across the six-lane matrix was ~177 ms. So no evidence currently links this gap to that failure.

It is nonetheless the larger exposure. Measurements in #695 showed indirect spawns dominate Windows cost, so the unbounded path is exactly where the risk concentrates.

Scope

Cover every workspace Git command, not only gitPath(). Decide and document:

• timeout and termination semantics, per command class rather than one arbitrary global value;
• what a timeout returns or throws, and how callers distinguish it from "not a repository";
• error compatibility for existing callers that rely on null;
• platform behavior, given that Windows process handling differs materially;
• direct and indirect callers, including resolveMadarWorkspace() and generateGraph();
• production and test diagnostics — a killed Git should be attributable;
• cancellation, if a caller can abandon discovery.

Note that some production Git calls elsewhere already pass timeout, so the codebase is inconsistent rather than uniformly unbounded. Reconcile deliberately.

Explicit non-goals

• No arbitrary one-size-fits-all timeout without measured justification.
• No change to graph, retrieval, Pack, extraction, MCP, installer, or release behavior.
• No retry.
• No absorption into [P0] Replace the Windows worktree artifact-routing wall-clock timeout with deterministic phase evidence #695 or PR test(workspace): replace aggregate worktree timeout with phase-aware assertions #696.

Acceptance criteria

• Every workspace Git invocation carries an explicit, justified bound.
• A bound-exceeded outcome is distinguishable by callers from "not a Git repository".
• Existing callers relying on null are audited and updated or explicitly preserved.
• Behavior verified on Ubuntu, macOS, and Windows.
• A killed Git produces an attributable diagnostic rather than a silent fallback.
• Timeout values are justified against measured command durations.
• Deterministic tests cover bound-exceeded, not-a-repository, and command-failure paths without depending on real elapsed time.
• No retry, skip, or global timeout inflation introduced.

Relationship to the roadmap

This does not block #654 unless new evidence shows an accepted qualification failure originated in one of these indirect commands. No such evidence exists today.

Related parent: #654. Refs #695.

[mohanagy]

Verified inventory of synchronous child-process call sites in src/

Recording this because the "add a timeout" framing is too narrow, and I now have the numbers rather than an impression.

Nine files, ~29 synchronous execFileSync/spawnSync call sites. Exactly one file bounds its Git calls.

File	Git/child calls	Explicit timeout on the Git call?
src/runtime/pr-impact.ts	2 git	yes — 10_000 at :312, 5_000 at :488
src/shared/workspace.ts	1 git (gitPath, 4 call sites)	no
src/shared/git.ts	3 git	no
src/infrastructure/generation-policy.ts	2 git	no
src/infrastructure/time-travel.ts	1 git	no
src/pipeline/spi/diff-overlay.ts	1 git	no
src/infrastructure/benchmark/suite.ts	5 git	no

So the codebase is inconsistent, not uniformly unbounded — which is the more useful statement, because it means there is already an in-repo precedent to standardise on rather than a convention to invent.

The precedent is itself incomplete

Both bounded sites pair the timeout with a bare catch:

// :305 gitDiffPatch
    return execFileSync('git', ['diff', '--no-ext-diff', '--unified=0', ...gitArgs], {
      ..., timeout: 10_000,
    })
  } catch {
    return ''

On timeout this returns '' — indistinguishable from "this diff is genuinely empty." The :488 site behaves the same way, falling through to the next candidate branch name as though main simply did not exist.

This is the exact defect pair described in the issue body, and it is not hypothetical — the repository already contains two instances of a timeout whose expiry is silently absorbed. That is the strongest argument that the process policy and the error contract have to land together: bounding gitPath alone would make it the third.

Highest-risk unbounded site

src/infrastructure/benchmark/suite.ts:522 runs git fetch --depth 1 origin <ref> with no timeout. It is the only network Git call in the inventory and therefore the most hang-prone one, and it is unbounded.

Scope note: this is a production-behaviour issue and is deliberately out of scope for #696, which is test-only. Refs #695. Parent: #654 remains open.

[mohanagy]

Retained as architecture-independent maintenance under #735. Non-blocking for #736 unless the workspace-discovery defect reproduces in the prototype.