Skip to content

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

Open
guyoron1 wants to merge 102 commits into
mainfrom
mirror/2381-2378-status-comment-agent-error
Open

fix(#2378): report failure when agent errors with no commits#71
guyoron1 wants to merge 102 commits into
mainfrom
mirror/2381-2378-status-comment-agent-error

Conversation

@guyoron1

Copy link
Copy Markdown
Owner

Mirror of upstream fullsend-ai#2381

Reports failure status when agent errors out without producing any commits.

ifireball and others added 30 commits June 10, 2026 16:18
Introduce --vendor to install vendored binaries, reusable workflows,
actions, and agent content. Vendored upstream mirror content is committed
under .defaults/ (same layout as runtime sparse checkout); layered installs
fetch fullsend-ai/fullsend@v0 into .defaults when the marker file is absent.

Reusable workflows use inline workspace preparation and reference infra
from ./.defaults/, matching the pre-vendor layered design. Thin callers
render local reusable paths when --vendor is set.

--fullsend-source pins the source tree for both content and binary
cross-compile; --fullsend-binary remains an explicit ELF override.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Write vendor-manifest.yaml on --vendor installs so cleanup and analyze work
without a local fullsend checkout. Workflows analyze stays embed-only;
vendor layer reports presence, manifest alignment, and optional source
alignment via admin analyze --fullsend-source.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Consolidate thin-stage caller registry, reuse resolved source root for
binary vendoring, reject oversized tar members during extraction, restore
workflows scope comment, fix testing-workflows prose, and introduce
InstallFiles as the canonical collector return type.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Re-add the full download_test.go suite and append extractSourceTree size
limit coverage.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Delete vendored paths atomically via forge.DeleteFiles, reuse resolved
source root for cross-compile, preserve extracted file modes, and tighten
WouldFix deduplication to exact path matches.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Document intentional breaking change: old flag callers should use --vendor;
only known usage was e2e, already updated in this branch.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Document VendorBinaryLayer legacy naming, restore Uninstall/Analyze
comments, and use Title Case for stale-cleanup progress messages.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Batch binary, content, and manifest in one CommitFiles call; validate
manifest version on read; trim leading slash in extractSourceTree; wrap
DeleteFiles ref PATCH in retryOnTransient.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Use the existing blob mode from the recursive tree and set type blob
so deletion entries match GitHub Trees API expectations.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Guard against regressions in delete-entry construction per review.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	internal/forge/fake.go
#	internal/forge/forge.go

Signed-off-by: Barak Korren <bkorren@redhat.com>
Encode CommitFiles tree entries as base64 to preserve ELF binaries,
add tar extract containment check, consolidate stale cleanup with a
manifest/binary quick-check, and deduplicate cleanup between CLI and layer.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	action.yml
#	docs/guides/dev/testing-workflows.md

Signed-off-by: Barak Korren <bkorren@redhat.com>
Clarify removed distribution-mode artifacts, drop e2e vendor line, and
document action.yml source-build fallback.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Empty commit to re-dispatch review; prior synchronize dispatch was cancelled.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Keep enumerateVendoredPaths aligned with CollectVendoredAssets after
main added the composite action (fullsend-ai#2106); fixes CI parity test.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…t dispatch

GitHub Actions may return 422 when repo-maintenance is dispatched immediately
after a separate vendor CommitFiles on a fresh .fullsend repo. Merge scaffold
and vendored assets into one atomic commit and retry dispatch on indexing lag.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…nance

Poll GitHub until repo-maintenance.yml is active before dispatch, re-touch
config.yaml after scaffold so the push trigger can run enrollment when
dispatch is still rejected, and fall back to awaiting a push-triggered run.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…nary

Tree entries with encoding:base64 stored base64 text literally on GitHub,
corrupting YAML workflows and vendor-manifest.yaml. Restore UTF-8 inline
content for text and upload binary via the Git Blob API instead.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Design for a new `prerequisites` triage action that replaces `blocked`.
The agent can now express both existing blockers and new issues that need
to be created upstream before progress can happen. Includes allowlist
configuration for cross-repo issue creation and a degraded path when
targets are not authorized.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
…nd-ai#401)

Seven-task plan covering config structs, JSON schema, agent prompt,
post-script, user docs, and caller updates. TDD approach with exact
file paths and code blocks.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
Add CreateIssuesConfig and AllowTargets types to both OrgConfig and
PerRepoConfig. NewOrgConfig populates defaults with the org and
fullsend-ai/fullsend. NewPerRepoConfig populates with the target repo
and fullsend-ai/fullsend.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
…ues (fullsend-ai#401)

Pass org name and target repo to config constructors so create_issues
defaults are populated at install time.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
)

Replace the blocked action and blocked_by field with a prerequisites
action containing existing[] and create[] arrays. At least one array
must be non-empty.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
…pt (fullsend-ai#401)

The triage agent can now recommend creating upstream issues via the
prerequisites action's create array, in addition to referencing existing
blockers. Adds hard constraint against emitting sufficient when
prerequisites exist.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
…d-ai#401)

Update triage agent docs to explain the new prerequisites action and the
create_issues.allow_targets configuration surface.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
…#401)

Replace the blocked handler with prerequisites. The post-script reads
the create_issues allowlist from config.yaml, creates permitted upstream
issues via gh, and includes collapsed draft bodies for disallowed or
failed creates so humans can file them manually.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
…ullsend-ai#401)

The agent prompt referenced a nonexistent `prerequisites` label when
checking for prior blockers — the post-script actually applies the
`blocked` label. Also removed unused SOURCE_ORG variable from
post-triage.sh.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
…end-ai#401)

Replace the four blocked-action test cases with five prerequisites-action
test cases that exercise the new schema (existing[], create[], allowlist
validation). Set up GITHUB_WORKSPACE with a config.yaml fixture and add
a mock gh issue-create handler that returns a fake URL.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
rh-hemartin and others added 2 commits June 17, 2026 08:34
…pdate-and-others

chore(ci): bump OpenShell to 0.0.63, extract install scripts, add Renovate
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 fullsend-ai#2378
@guyoron1

Copy link
Copy Markdown
Owner Author

/fs-qf

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:45 PM UTC · Completed 3:03 PM UTC
Commit: 0535520 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review

Reason: stale-head

The review agent reviewed commit 81280eb3fda83b59098a2f6ec0479d386102ef4b but the PR HEAD is now d2fcb420ad7866c459a3cdcf7103d7820a4655d2. This review was discarded to avoid approving unreviewed code.

Previous run

Review

Reason: stale-head

The review agent reviewed commit f82544cc6242524b4fa813e8e54788d6878c7cdb but the PR HEAD is now 81280eb3fda83b59098a2f6ec0479d386102ef4b. This review was discarded to avoid approving unreviewed code.

Previous run (2)

Review

Findings

High

  • [logic-error] internal/cli/run.go:413 — The status notification defer block does not check lastExitCode. When the agent exits non-zero but rt.Run returns err==nil (sandbox succeeded), and no validation loop is configured, runErr is nil and ctx.Err() is nil, so status is reported as "success". This directly contradicts the PR's stated goal of reporting failure when the agent errors. Runtime.Run (runtime.go:43) returns (exitCode int, err error) separately — a non-zero exit code does not set runErr. The function returns nil at line 984 without ever translating a non-zero exit code into an error.
    Remediation: Add a check for lastExitCode != 0 in the status defer block between the ctx.Err() and runErr checks, e.g., else if lastExitCode != 0 { status = "failure" }.

  • [protected-path] N/A — This PR modifies multiple protected paths (.github/dependabot.yml, .github/scripts/, .github/workflows/reusable-*.yml, .pre-commit-config.yaml, CLAUDE.md, skills/e2e-health/, skills/mint-enroll/) without a linked issue or explanation in the PR description for why governance/infrastructure files need modification. Human approval is required for all protected-path changes.

Medium

  • [scope-exceeded] N/A — PR claims to be fix(#2378) for agent error reporting but modifies 122 files with 10,428 additions and 2,063 deletions spanning: vendor system (ADR 0047), mint refactoring (ROLE_APP_IDS format, reconcile-status CLI), forge interface additions (5 new methods), harness discovery, scaffold updates, CI/CD changes, and documentation. The actual Code agent status comment should reflect actual outcome when no PR is created fullsend-ai/fullsend#2378 fix is approximately 30 lines in run.go and post-code.sh. Per COMMITS.md, fix is reserved for bug fixes visible to users.

  • [breaking-api] action.yml / internal/cli/reconcilestatus.go — The status-token input in action.yml was replaced by mint-url. The reconcile-status CLI command removed --token (test asserts --token flag should no longer exist) and replaced it with --mint-url + --role. Any external workflow passing status-token to this action will silently lose status comment functionality. See also: [breaking-change-unauthorized] finding.

  • [breaking-change-unauthorized] N/A — PR includes breaking changes to action.yml inputs (status-token removed), CLI flags (reconcile-status --token removed), and ROLE_APP_IDS key format (org/rolerole in appsetup). Per COMMITS.md, breaking changes require a ! suffix (e.g., feat!:) and a BREAKING CHANGE: trailer. The PR title fix(#2378) has no breaking change marker.

Low

  • [injection-vuln] internal/scaffold/fullsend-repo/scripts/post-triage.sh:226LA_LABEL and LA_ACTION from untrusted agent JSON are interpolated into ::warning:: workflow commands without sanitization. When LA_LABEL fails regex validation, the raw unsanitized value is directly interpolated. LA_ACTION in the fallback branch has no validation. Impact is limited to cosmetic log annotation injection (cannot execute code or escalate privileges).

  • [test-adequacy] internal/cli/run.go:413 — No tests cover the interaction between non-zero lastExitCode, absence of a validation loop, and the status notification defer block — the exact scenario described in issue Code agent status comment should reflect actual outcome when no PR is created fullsend-ai/fullsend#2378.

  • [edge-case] internal/cli/run.go:508 — Post-script runs unconditionally when there is no validation loop and runErr==nil, even when the agent exited non-zero. AGENT_EXIT_CODE is not passed in the post-script environment, so the post-script has no way to know the agent failed. Code comments indicate this is by design, but it limits failure reporting capability.

  • [supply-chain] .github/scripts/install-openshell.sh:15 — Downloads and executes installer from remote URL via curl | sh. While pinned to a commit SHA, the downloaded script content is not integrity-verified (no checksum). Other tools in the repo (gitleaks, lychee, uv) perform SHA-256 verification, making this inconsistent.

Previous run (3)

Review

Reason: stale-head

The review agent reviewed commit 7de059da6482076a29da94ec29af1e910fdae08b but the PR HEAD is now cda7a2ae3aa4dd450be0d7c3a83363884fff98b9. This review was discarded to avoid approving unreviewed code.

Previous run (4)

Review

Reason: stale-head

The review agent reviewed commit e3859411ba49546e5bf86c0fa9eb98ba22fbc390 but the PR HEAD is now 11f00661d0a0a25b1f9bdf7dd809227ab66f5b14. This review was discarded to avoid approving unreviewed code.

@fullsend-ai-review

Copy link
Copy Markdown

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:10 PM UTC · Completed 3:29 PM UTC
Commit: 0535520 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

/fs-review

QualityFlow and others added 2 commits June 21, 2026 15:33
Replaces intermediate pipeline artifacts with organized test files.

Total: 0 test files → qf-tests/fullsend-aiGH-2378/
Jira: fullsend-aiGH-2378
[skip ci]
@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown

QualityFlow Pipeline Summary

Stage Agent Status
1 STP Builder
2 STP Reviewer
3 STP Refiner
4 STD Builder
5 STD Reviewer
6 STD Refiner
7 Test Generator

Test Output

Language Count Location
Go 4 files internal/cli/qf_*_test.go

Issue: GH-71


Generated by QualityFlow

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:36 PM UTC · Completed 4:17 PM UTC
Commit: 0535520 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

Comment thread internal/cli/run.go
notifier, notifyErr := setupStatusNotifier(absFullsendDir, sOpts, printer)
notifier, notifyErr := setupStatusNotifier(absFullsendDir, agentName, sOpts, printer)
if notifyErr != nil {
printer.StepWarn("Status notifications disabled: " + notifyErr.Error())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[high] logic-error

Status notification defer block does not check lastExitCode. When the agent exits non-zero but rt.Run returns err==nil, runErr is nil and ctx.Err() is nil, so status is reported as 'success'. This directly contradicts the PR's stated goal.

Suggested fix: Add a check for lastExitCode != 0 in the status defer block between the ctx.Err() and runErr checks.

- ${url}"
done
COMMENT="${COMMENT}

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] injection-vuln

LA_LABEL and LA_ACTION from untrusted agent JSON interpolated into ::warning:: workflow commands without sanitization. Impact limited to cosmetic log annotation injection.

Comment thread internal/cli/run.go
notifier, notifyErr := setupStatusNotifier(absFullsendDir, sOpts, printer)
notifier, notifyErr := setupStatusNotifier(absFullsendDir, agentName, sOpts, printer)
if notifyErr != nil {
printer.StepWarn("Status notifications disabled: " + notifyErr.Error())

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-adequacy

No tests cover the interaction between non-zero lastExitCode, absence of validation loop, and status notification defer block.

Comment thread internal/cli/run.go
@@ -499,6 +507,14 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep
// ADR 0022's zero-trust model.
var validationPassed bool

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] edge-case

Post-script runs unconditionally when runErr==nil even when agent exited non-zero. AGENT_EXIT_CODE not passed to post-script environment.

source "${SCRIPT_DIR}/openshell-version.sh"

echo "Installing OpenShell ${OPENSHELL_VERSION} (${OPENSHELL_SHA})"
curl -LsSf "https://raw.githubusercontent.com/NVIDIA/OpenShell/${OPENSHELL_SHA}/install.sh" \

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] supply-chain

Downloads and executes installer via curl | sh pinned to SHA but without checksum verification, inconsistent with other tool installs in the repo.

@guyoron1

Copy link
Copy Markdown
Owner Author

/fs-qf

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:05 AM UTC · Completed 7:26 AM UTC
Commit: 6ead6d0 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 22, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:29 AM UTC · Completed 7:50 AM UTC
Commit: 6ead6d0 · View workflow run →

QualityFlow and others added 2 commits June 22, 2026 07:30
- Remove related_prs from document_metadata (content policy violation)
- Replace flat package_name with target_packages map for multi-package support
- Add cleanup steps for scenarios 2, 4, 6, 7

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@fullsend-ai-review

Copy link
Copy Markdown

/fs-review

QualityFlow and others added 2 commits June 22, 2026 07:51
22 test cases covering exit code propagation, post-code failure
detection, status comment handling, and reconcile-status idempotency.

All tests compile and pass.
Removes intermediate pipeline artifacts (STP, STD, reviews).
Test files (4) are co-located in source tree with qf_ prefix.
Jira: GH-71
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants