fix(#2378): report failure when agent errors with no commits - #2381
fix(#2378): report failure when agent errors with no commits#2381fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
Site previewPreview: https://b6544880-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 9:54 AM UTC · Completed 10:06 AM UTC |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Looks good to me Low
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| @@ -280,15 +289,17 @@ run_noop_test() { | |||
| local test_name="$1" | |||
There was a problem hiding this comment.
[low] test-integrity
The detect_noop test helper returns 1 for error cases, but run_noop_test suppresses the exit code with || true and only checks the output prefix. Consider adding a test that verifies the return code is non-zero for error:branch and error:files cases, since the production code relies on exit 1 triggering the ERR trap.
| // ADR 0022's zero-trust model. | ||
| var validationPassed bool | ||
|
|
||
| // lastExitCode is declared here (before the post-script and status |
There was a problem hiding this comment.
[low] comment-style
The 5-line comment for lastExitCode is notably longer than the similar validationPassed declaration comment. The extra length is justified by the non-obvious design decision, but could be tightened.
| @@ -532,6 +540,7 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep | |||
| postCmd := exec.Command(h.PostScript) | |||
There was a problem hiding this comment.
[low] comment-formatting
Two-step postCmd.Env = append(...) pattern (first build base env, then add extra var) is a minor style deviation from single-append patterns elsewhere. This is a common Go idiom and reads clearly.
fullsend-ai-review
Bot
added
the
ready-for-merge
When the code agent exits non-zero (e.g., 429 RESOURCE_EXHAUSTED) but
produces no commits, the status comment previously reported "Success"
because runErr stayed nil — the OS-level execution succeeded even
though the agent session failed.
Root cause: lastExitCode was declared after the status and post-script
defers, so neither closure could read it. The post-script then hit the
"no changed files — nothing to do" path and exited 0, leaving runErr
nil and the status as "success".
Changes:
- Move lastExitCode declaration before the post-script defer so both
closures can reference it
- Pass AGENT_EXIT_CODE env var to the post-script so it can distinguish
agent errors from intentional no-ops
- In post-code.sh, check AGENT_EXIT_CODE at both "nothing to do" exit
points (no branch and no changed files); exit 1 when agent errored
- Update report_failure_to_issue() to produce a distinct message for
agent errors ("Code agent failed") vs post-script errors
- Add shell tests covering agent error detection at both exit points
and the error comment content
Intentional no-change runs (agent exits 0, no commits) are unaffected —
the AGENT_EXIT_CODE check only triggers on non-zero exit codes.
Closes #2378
e385941 to
503a6d8
Compare
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
ralphbean
left a comment
ralphbean
left a comment
There was a problem hiding this comment.
This is an automated comment. We are moving agent content from internal/scaffold/fullsend-repo/ to https://github.com/fullsend-ai/agents -- changes should be made to agent definitions there going forwards.
|
This needs to be on the fullsend-ai/agents repo. |
|
🤖 Finished Retro · ✅ Success · Started 6:51 PM UTC · Completed 7:00 PM UTC |
|
PR #2381 was a well-implemented fix for issue #2378 (code agent status comment falsely reporting "Success" when agent errors with no commits). The code agent produced a clean 1-commit PR on June 17 touching Three proposals filed:
Proposals filed
|
1 participant
When the code agent exits non-zero (e.g., 429 RESOURCE_EXHAUSTED) but produces no commits, the status comment previously reported "Success" because runErr stayed nil — the OS-level execution succeeded even though the agent session failed.
Root cause: lastExitCode was declared after the status and post-script defers, so neither closure could read it. The post-script then hit the "no changed files — nothing to do" path and exited 0, leaving runErr nil and the status as "success".
Changes:
closures can reference it
agent errors from intentional no-ops
points (no branch and no changed files); exit 1 when agent errored
agent errors ("Code agent failed") vs post-script errors
and the error comment content
Intentional no-change runs (agent exits 0, no commits) are unaffected — the AGENT_EXIT_CODE check only triggers on non-zero exit codes.
Closes #2378
Post-script verification
agent/2378-status-comment-agent-error)c7ad0264ec1cd794d0f9089f4479ae7614e2321a..HEAD)