fix(agent-core): surface git context failures for explore subagents - #1067
Conversation
🦋 Changeset detectedLatest commit: dd580d2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24833b5590
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const failed = results.filter(({ result }) => !result.ok); | ||
| if (failed.length > 0) { | ||
| for (const { args, result } of failed) { | ||
| if (!result.ok) logGitFailure(cwd, args, result); | ||
| } | ||
| return ''; |
There was a problem hiding this comment.
Don't drop context when optional git probes are absent
In valid repositories that lack an origin remote or have no commits yet, git remote get-url origin returns error: No such remote 'origin' with exit 2, and git log -3 exits 128, so this failure branch returns '' and suppresses otherwise-useful branch/status context. Since those are normal repo states rather than collection failures, treating every failed optional probe as fatal regresses explore subagents in local/new repositories; consider allowing missing remote/log to produce an empty section while still failing the block on commands that can make the snapshot misleading, such as status.
Useful? React with 👍 / 👎.
collectGitContext collapsed every git failure (spawn error, non-zero exit, timeout) into null, so explore subagents silently lost git context with no signal. Now a definitive 'not a git repository' injects an explicit unavailable signal so the subagent does not waste turns probing git history, while other failures are logged and surface as an empty block. The block is all-or-nothing so a partial snapshot (e.g. a timed-out status making a dirty tree look clean) is never shown.
24833b5 to
8f6aa98
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f6aa989fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| commandArgs.map(async (args) => ({ args, result: await runGit(kaos, cwd, args) })), | ||
| )) as unknown as [TaggedGitResult, TaggedGitResult, TaggedGitResult, TaggedGitResult]; | ||
|
|
||
| const fatal = [branch, status].filter(({ result }) => !result.ok); |
There was a problem hiding this comment.
In environments with Git older than 2.22, git branch --show-current is an unsupported option even in a valid repository (I checked the git-branch docs: 2.21's synopsis omits it, while 2.22 adds --show-current and documents it as printing the current branch). Because this line now treats the branch probe as fatal, explore subagents return no git context at all before using the still-valid git status result; the previous per-command guard would have continued to surface dirty files/recent commits. Please either keep branch optional or fall back to an older branch command so older-Git users don't lose the whole context block.
Useful? React with 👍 / 👎.
`git branch --show-current` was added in Git 2.22 and fails (exit 129) on older Git even in a valid repository. Because the branch probe is fatal, this dropped the whole git-context block for older-Git users. Switch to `git rev-parse --abbrev-ref HEAD`, which is supported across Git versions, and filter the `HEAD` output produced in detached-HEAD state.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8969a525d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // prints `HEAD` in detached-HEAD state, which is filtered out below. | ||
| const commandArgs = [ | ||
| ['remote', 'get-url', 'origin'], | ||
| ['rev-parse', '--abbrev-ref', 'HEAD'], |
There was a problem hiding this comment.
Handle unborn branches before failing branch probe
Fresh evidence beyond the prior log-related comment: the replacement branch command itself fails in unborn repos. In a newly initialized repo or an orphan branch before the first commit, HEAD does not yet name an object—git-rev-parse documents --abbrev-ref as returning a short name of the object name—so git rev-parse --abbrev-ref HEAD exits 128 even though git status and git branch --show-current still work. Because this probe is treated as fatal, explore subagents lose all dirty/untracked-file context for valid new repositories; please use symbolic-ref --short HEAD/a fallback or make this failure non-fatal.
Useful? React with 👍 / 👎.
Git probes fail in perfectly normal states — no `origin` remote, no commits yet (unborn branch), detached HEAD, older Git — so a failed probe no longer aborts the whole collection. Each probe is now best-effort: failures are logged and their section is omitted, and the block is dropped only when nothing useful was collected. Branch is read via `symbolic-ref --short HEAD`, which works in unborn repositories and on older Git; it fails in detached-HEAD state, where the Branch section is just omitted.
Related Issue
No related issue. This addresses the silent loss of git context for explore subagents (internal bug M3).
Problem
When an explore subagent starts, git context (remote, branch, dirty files, recent commits) is collected to orient it. Every git failure — spawn error, non-zero exit, or the 5s timeout — was collapsed into
null, so the subagent silently lost all git context with no signal to it and no log. In the worst case a partial result (e.g. a timed-outgit status) could make a dirty tree look clean.What changed
<git-context status="unavailable" reason="not-a-repo"/>so the subagent does not waste turns probing git history.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.