fix(agents): surface stdout-only CLI errors - #202
Open
jasonqlwilliams-alt wants to merge 1 commit into
Open
Conversation
This was referenced Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Fix the shared child-process error-reporting defect in src/core/agents/stream-utils.ts used by the Codex, Copilot, and Pi adapters. Build on the established PR #190 behavior: a non-zero CLI exit may carry the real error on stdout while stderr is empty. Include a bounded stdout tail when stderr alone lacks the error, parse structured JSON error fields, classify permanent errors from the relevant combined output, and clearly report when both streams are empty. Require behavioral adapter coverage for Codex, Copilot, and Pi stdout-only non-zero exits. Reuse the corrected shared helper for Claude only if it reduces duplication without broadening behavior or risk. Remain compatible with the fully green Pi JSON recovery PR #195 and do not modify its branch. Deliver a review-ready upstream contribution PR with all canonical checks green. The captain explicitly authorizes unattended --yes validation of preserved commit 055f17b46bb6709f42af5e93220561baee792808 under no-mistakes v1.48.0. Do not merge, publish, deploy, change policy, waive checks, alter credentials, reset, force, or discard preserved work.
What Changed
Risk Assessment
✅ Low: The change is well-bounded, preserves the established Claude semantics, fixes all shared adapter paths, and adds behavior-level coverage for each required adapter without conflicting with Pi JSON recovery.
Testing
Focused shared-helper and adapter tests, the Claude stdout/empty-stream regression, and built-CLI stdout-only failures for Codex, Copilot, and Pi all passed; after correcting and cleaning an initial harness working-directory mistake, the final transcript directly demonstrates the user-visible CLI behavior. No screenshot was needed because this changes process-error text rather than graphical layout.
Evidence: Stdout-only adapter failure transcript
Codex, Copilot, and Pi child processes each exited 1 with zero stderr bytes and a structured stdout error. The built gnhf CLI displayed and persisted the exact error for every adapter.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
pnpm exec vitest run src/core/agents/stream-utils.test.ts src/core/agents/codex.test.ts src/core/agents/copilot.test.ts src/core/agents/pi.test.ts src/core/agents/claude.test.tspnpm run buildBuilt CLI runs in isolated repositories usingnode dist/cli.mjs "surface <agent> stdout failure" --agent <codex|copilot|pi> --max-iterations 1 --prevent-sleep offDirect child-process checks confirming exit code 1, structured JSON on stdout, and zero stderr bytes for Codex, Copilot, and Pi fixturespnpm exec vitest run e2e/e2e.test.ts -t "surfaces the claude CLI's own failure text"Verified the terminal summaries and persistednotes.mderrors for all three required adaptersInitial CLI evidence attempt used the wrong working directory; its results were discarded, and all resulting branch, metadata, build, and dependency artifacts were removed✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.