Skip to content

fix(#2378): report failure when agent errors with no commits #2381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fullsend-ai-coder wants to merge 1 commit into
base: main
Choose a base branch
from
+123 −10
Open

fix(#2378): report failure when agent errors with no commits #2381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion internal/cli/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,14 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep
// ADR 0022's zero-trust model.
var validationPassed bool

// lastExitCode is declared here (before the post-script and status

@fullsend-ai-review fullsend-ai-review Bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

// defers) so both closures can read the agent's final exit code.
// When the agent exits non-zero but the Go-level execution succeeds,
// runErr stays nil — this variable lets the post-script and status
// comment distinguish "agent errored" from "agent chose to do nothing".
// See #2378.
var lastExitCode int

// Post-script runs after sandbox cleanup (defers are LIFO).
// When a validation_loop is configured, the post-script only runs if
// validation passed (ADR 0022). When no validation_loop exists (e.g.,
Expand All @@ -532,6 +540,7 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep
postCmd := exec.Command(h.PostScript)

@fullsend-ai-review fullsend-ai-review Bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

postCmd.Dir = runDir
postCmd.Env = append(os.Environ(), envToList(h.RunnerEnv)...)
postCmd.Env = append(postCmd.Env, fmt.Sprintf("AGENT_EXIT_CODE=%d", lastExitCode))
postCmd.Stdout = os.Stdout
postCmd.Stderr = os.Stderr
if err := postCmd.Run(); err != nil {
Expand Down Expand Up @@ -797,7 +806,6 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep
oidcWg.Wait()
}()

var lastExitCode int
var runCount int

for iteration := 1; iteration <= maxIterations; iteration++ {
Expand Down
97 changes: 89 additions & 8 deletions internal/scaffold/fullsend-repo/scripts/post-code-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,15 +259,24 @@ count_closes_test "single-closes-empty-body" \
detect_noop() {
local branch="$1"
local changed_files="$2"
local agent_exit_code="${3:-0}"

# Step 1: branch check (mirrors lines 64-67 of post-code.sh)
# Step 1: branch check (mirrors post-code.sh section 1)
if [ -z "${branch}" ] || [ "${branch}" = "main" ] || [ "${branch}" = "master" ]; then
if [ "${agent_exit_code}" != "0" ]; then
echo "error:branch:Agent exited with code ${agent_exit_code} and did not create a feature branch"
return 1
fi
echo "noop:branch:Agent did not create a feature branch (current: '${branch:-detached HEAD}') — nothing to do"
return 0
fi

# Step 2: changed files check (mirrors lines 84-87 of post-code.sh)
# Step 2: changed files check (mirrors post-code.sh section 2)
if [ -z "${changed_files}" ]; then
if [ "${agent_exit_code}" != "0" ]; then
echo "error:files:Agent exited with code ${agent_exit_code} and produced no changes"
return 1
fi
echo "noop:files:No changed files in agent's commit(s) — nothing to do"
return 0
fi
Expand All @@ -280,15 +289,17 @@ run_noop_test() {
local test_name="$1"

@fullsend-ai-review fullsend-ai-review Bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

local branch="$2"
local changed_files="$3"
local expected_prefix="$4" # "noop:branch", "noop:files", or "proceed"
local expected_prefix="$4" # "noop:branch", "noop:files", "error:branch", "error:files", or "proceed"
local agent_exit_code="${5:-0}"

local actual
actual="$(detect_noop "${branch}" "${changed_files}")"
actual="$(detect_noop "${branch}" "${changed_files}" "${agent_exit_code}" 2>&1)" || true

if [[ "${actual}" != ${expected_prefix}* ]]; then
echo "FAIL: ${test_name}"
echo " branch: '${branch}'"
echo " changed_files: '${changed_files}'"
echo " branch: '${branch}'"
echo " changed_files: '${changed_files}'"
echo " agent_exit_code: '${agent_exit_code}'"
echo " expected prefix: '${expected_prefix}'"
echo " actual: '${actual}'"
FAILURES=$((FAILURES + 1))
Expand Down Expand Up @@ -324,6 +335,28 @@ run_noop_test "proceed-feature-branch-with-changes" \
run_noop_test "noop-on-main-with-changes" \
"main" "src/widget.go" "noop:branch"

# --- Agent error detection test cases (#2378) ---

# Agent errored (exit 1) on main with no changes → error via branch check
run_noop_test "error-agent-failed-on-main" \
"main" "" "error:branch" "1"

# Agent errored (exit 1) on feature branch with no changes → error via files check
run_noop_test "error-agent-failed-no-changes" \
"agent/42-fix-widget" "" "error:files" "1"

# Agent succeeded (exit 0) on feature branch with no changes → noop (not error)
run_noop_test "noop-agent-success-no-changes" \
"agent/42-fix-widget" "" "noop:files" "0"

# Agent errored but produced changes → proceed (changes take precedence)
run_noop_test "proceed-agent-failed-with-changes" \
"agent/42-fix-widget" "src/widget.go" "proceed" "1"

# Agent errored (exit 2) on detached HEAD → error via branch check
run_noop_test "error-agent-failed-detached-head" \
"" "" "error:branch" "2"

# ---------------------------------------------------------------------------
# Test helper — reimplements the stale branch cleanup decision logic from
# post-code.sh section 7a. Given whether a remote branch exists and whether
Expand Down Expand Up @@ -454,10 +487,23 @@ build_error_comment() {
local repo_full_name="$2"
local run_id="$3"
local github_repository="${4:-}" # GITHUB_REPOSITORY override (org-mode)
local agent_error_exit="${5:-false}"
local agent_exit_code="${6:-unknown}"

local run_repo="${github_repository:-${repo_full_name}}"
local run_url="https://github.com/${run_repo}/actions/runs/${run_id}"
echo "⚠️ **Post-code script failed** (exit code ${exit_code})

if [ "${agent_error_exit}" = "true" ]; then
echo "⚠️ **Code agent failed** (agent exit code ${agent_exit_code})

The code agent terminated with an error and produced no PR.

**Workflow run:** ${run_url}

Please check the workflow logs for details and retry with \`/fs-code\` \
if appropriate."
else
echo "⚠️ **Post-code script failed** (exit code ${exit_code})

The code agent completed, but the post-code script failed while \
pushing the branch or creating the PR.
Expand All @@ -466,6 +512,7 @@ pushing the branch or creating the PR.

Please check the workflow logs for details and retry with \`/fs-code\` \
if appropriate."
fi
}

run_error_comment_test() {
Expand All @@ -476,9 +523,11 @@ run_error_comment_test() {
local check_pattern="$5"
local expect_present="$6"
local github_repository="${7:-}" # optional GITHUB_REPOSITORY override
local agent_error_exit="${8:-false}"
local agent_exit_code="${9:-unknown}"

local actual
actual="$(build_error_comment "${exit_code}" "${repo}" "${run_id}" "${github_repository}")"
actual="$(build_error_comment "${exit_code}" "${repo}" "${run_id}" "${github_repository}" "${agent_error_exit}" "${agent_exit_code}")"

if [ "${expect_present}" = "yes" ]; then
if ! echo "${actual}" | grep -qF "${check_pattern}"; then
Expand Down Expand Up @@ -539,6 +588,38 @@ run_error_comment_test "error-comment-non-org-mode-fallback" \
"https://github.com/my-org/my-repo/actions/runs/67890" "yes" \
""

# --- Agent error comment test cases (#2378) ---

# Agent error comment should say "Code agent failed"
run_error_comment_test "agent-error-comment-title" \
"1" "my-org/my-repo" "12345" \
"Code agent failed" "yes" \
"" "true" "1"

# Agent error comment should include agent exit code
run_error_comment_test "agent-error-comment-exit-code" \
"1" "my-org/my-repo" "12345" \
"agent exit code 1" "yes" \
"" "true" "1"

# Agent error comment should NOT say "Post-code script failed"
run_error_comment_test "agent-error-comment-not-postcode" \
"1" "my-org/my-repo" "12345" \
"Post-code script failed" "no" \
"" "true" "1"

# Agent error comment should mention no PR was created
run_error_comment_test "agent-error-comment-no-pr" \
"1" "my-org/my-repo" "12345" \
"produced no PR" "yes" \
"" "true" "1"

# Non-agent error (default) should still say "Post-code script failed"
run_error_comment_test "non-agent-error-default" \
"1" "my-org/my-repo" "12345" \
"Post-code script failed" "yes" \
"" "false" "0"

# ---------------------------------------------------------------------------
# Test helper — reimplements the agent artifact stripping logic from
# post-code.sh section 2b. Given a list of changed files, returns which
Expand Down
26 changes: 25 additions & 1 deletion internal/scaffold/fullsend-repo/scripts/post-code.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,20 @@ report_failure_to_issue() {
export GH_TOKEN="${PUSH_TOKEN}"
fi
local run_url="${GITHUB_SERVER_URL:-https://github.com}/${GITHUB_REPOSITORY:-${REPO_FULL_NAME}}/actions/runs/${GITHUB_RUN_ID:-unknown}"
local comment_body="⚠️ **Post-code script failed** (exit code ${exit_code})

local comment_body
if [ "${AGENT_ERROR_EXIT:-false}" = "true" ]; then
# Agent itself errored (non-zero exit) and produced no changes.
comment_body="⚠️ **Code agent failed** (agent exit code ${AGENT_EXIT_CODE:-unknown})

The code agent terminated with an error and produced no PR.

**Workflow run:** ${run_url}

Please check the workflow logs for details and retry with \`/fs-code\` \
if appropriate."
else
comment_body="⚠️ **Post-code script failed** (exit code ${exit_code})

The code agent completed, but the post-code script failed while \
pushing the branch or creating the PR.
Expand All @@ -84,6 +97,7 @@ pushing the branch or creating the PR.

Please check the workflow logs for details and retry with \`/fs-code\` \
if appropriate."
fi

echo "::warning::Posting failure comment to issue #${ISSUE_NUMBER}..."
gh issue comment "${ISSUE_NUMBER}" \
Expand All @@ -99,6 +113,11 @@ trap report_failure_to_issue ERR
BRANCH="$(git branch --show-current)"

if [ -z "${BRANCH}" ] || [ "${BRANCH}" = "main" ] || [ "${BRANCH}" = "master" ]; then
if [ "${AGENT_EXIT_CODE:-0}" != "0" ]; then
echo "::error::Agent exited with code ${AGENT_EXIT_CODE} and did not create a feature branch"
AGENT_ERROR_EXIT=true
exit 1
fi
echo "::notice::Agent did not create a feature branch (current: '${BRANCH:-detached HEAD}') — nothing to do"
exit 0
fi
Expand All @@ -119,6 +138,11 @@ else
fi

if [ -z "${CHANGED_FILES}" ]; then
if [ "${AGENT_EXIT_CODE:-0}" != "0" ]; then
echo "::error::Agent exited with code ${AGENT_EXIT_CODE} and produced no changes"
AGENT_ERROR_EXIT=true
exit 1
fi
echo "::notice::No changed files in agent's commit(s) — nothing to do"
exit 0
fi
Expand Down
Loading