Codex: Differentiate 404/permission failures from genuine errors in clone

[reakaleek]

Background

The CloneRepository catch block in CodexCloneService downgrades all exceptions to warnings:

// Emit warning instead of error: repos may be in the link index before the clone
// workflow has permission to access them. Continue with repos we can clone.
context.Collector.EmitWarning(..., $"Could not clone repository '{repoName}': {ex.Message}");

This is intentional for the "repo in the link index we don't have permission to yet" case (404/private), but it also silently swallows genuine errors: bad git references in config, network failures on repos we do have access to, disk errors, etc.

Problem

There's currently no way to distinguish these cases because ExecIn uses Proc.Exec, which streams git's stderr directly to the console and throws ProcExecException with only the exit code + command in the message — not git's actual output (e.g. "fatal: repository not found").

What's needed

Add a capturing variant to ExternalCommandExecutor — something like ExecCapturing(...) that uses Proc.Start with a writer that records stderr — so CodexGitRepository.Fetch can return enough context to let CloneRepository distinguish:

• "repository not found" / auth failure (HTTP 404) → warning, skip (current behaviour, keep)
• Any other failure → error, fail the build (currently silently warned)

Related

PR #3460 fixed codex index resilience. This issue tracks making codex clone equally precise about which failures it tolerates.

[cotti]

AI triage

Automated investigation against the current codebase. A human still needs to confirm before any code change.

Original concern

codex clone downgrades every clone failure to a warning, so genuine errors (bad git refs, network failures on accessible repos, disk errors) are silently swallowed alongside the intentionally-tolerated 404/permission case.

Findings

• src/Elastic.Codex/Sourcing/CodexCloneService.cs:L237-L246 — the broad catch (Exception ex) in CloneRepository downgrades all exceptions to EmitWarning(... "Could not clone repository '{repoName}': {ex.Message}") and returns null. There is no classification of the failure — confirms the issue's core claim.
• src/Elastic.Codex/Sourcing/CodexGitRepository.cs:L35-L36 — Fetch is ExecIn(EnvironmentVars, "git", "fetch", …). The clone path (CodexCloneService.cs:L211-L212) is Fetch → Checkout("FETCH_HEAD"), both routed through ExecIn.
• src/Elastic.Documentation.Tooling/ExternalCommands/ExternalCommandExecutor.cs:L26-L37 — ExecIn runs Proc.Exec and, on a non-zero exit, only surfaces a generic Exit code: {result} while executing {binary} {args} (L36). Git's real stderr (e.g. fatal: repository not found) is streamed to the console and not captured. Per the issue, Proc.Exec throws ProcExecException carrying just that exit-code/command text, which propagates to the L237 catch — so the message available for classification never contains git's diagnostic.
• src/Elastic.Documentation.Tooling/ExternalCommands/ExternalCommandExecutor.cs:L53-L123 — the only capturing helpers (Capture/CaptureMultiple/CaptureQuiet) capture stdout of short commands with a 3s timeout + 10× retry loop; there is no stderr-capturing variant suited to fetch. No ExecCapturing exists anywhere in the tree (confirmed by source + git log -S "ExecCapturing").
• src/services/Elastic.Documentation.Assembler/Sourcing/GitFacade.cs:L47 — SingleCommitOptimizedGitRepository (the assembler's git wrapper) subclasses the same ExternalCommandExecutor and uses the identical Fetch → ExecIn pattern, so any base-class change here is shared with the assembler.
• No fix has landed: CodexCloneService.cs was last touched by PR #3460 (commit c10789af, 2026-06-02), which is the sibling work this issue explicitly follows. No test covers clone-failure classification (only tests/Navigation.Tests/Codex/FindDocsetFileTests.cs exercises CodexCloneService, and only its FindDocsetFile helper).

Verdict

Needs stakeholder input (engineering)

The issue is an accurate description of the live code and the proposed direction (add a capturing exec variant so the catch block can classify) is sound. It is not a clean quick-fix, though, because resolving it means deciding (1) which git stderr signatures count as "tolerable" — a brittle allowlist that varies by git version and transport (HTTPS-with-token vs SSH; GitHub emits remote: Repository not found, fatal: could not read Username, Authentication failed, remote: Not Found, etc.), and getting it wrong silently re-buries genuine errors or starts failing on benign ones; and (2) that genuine clone failures should now break the build where they currently pass with a warning — a change to build/exit-code semantics that callers and CI rely on. Both are engineering/error-handling design calls rather than a mechanical edit, and the change touches a base class shared with the assembler.

Proposal

Implementation shape (matches the issue author's sketch):

• Add an ExecCapturing(...)-style method to ExternalCommandExecutor (ExternalCommandExecutor.cs) using Proc.Start with a writer that records stderr, returning the exit code + captured output. Keep it additive so existing ExecIn callers are unaffected, and AOT-safe.
• Have CodexGitRepository.Fetch (and likely Checkout) use it and surface git's stderr to CloneRepository.
• In the CloneRepository catch (CodexCloneService.cs:L237), classify on the captured stderr: "repository not found"/auth/404 → warn + skip (current behaviour); anything else → EmitError + fail.

Open questions for eng to confirm before implementing:

• What is the authoritative allowlist of "tolerated" failure signatures, and do we match on git stderr text, HTTP status, or both? How robust must it be across git versions / SSH vs HTTPS?
• Is escalating genuine clone failures to build-breaking errors the desired contract for all callers (codex clone in CI, local runs)? Should it be gated (e.g. only when a GITHUB_TOKEN is present, mirroring the "permission not granted yet" rationale)?
• Should the new capturing variant live on the shared ExternalCommandExecutor (also used by the assembler's SingleCommitOptimizedGitRepository) or stay Codex-local?

Related issues

PR #3460 (merged, c10789af) made codex index resilient to repos that failed to clone via the HasHead() short-circuit (CodexCloneService.cs:L185-L196); this issue is the explicit follow-up to make codex clone equally precise. No open duplicate found ("codex clone" returns only this issue).

Notes

Inconclusive on one library detail that couldn't be verified from source: whether Proc.Exec (Proc 0.13.0) throws ProcExecException on every non-zero exit vs. returning the code so ExecIn's L36 EmitError fires. The issue author (a maintainer) states it throws; the practical outcome for this issue is the same either way — git's stderr is lost. Applying ai:eng-question given the design decisions above.