Skip to content

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

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

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

Conversation

@guyoron1

Copy link
Copy Markdown
Owner

Mirror of upstream fullsend-ai#2381

When the code agent exits non-zero but produces no commits, the status comment previously reported "Success". Fixes lastExitCode scoping so the post-script can detect agent failures.

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>
ifireball and others added 11 commits June 16, 2026 21:23
…-ids

feat(mint): share ROLE_APP_IDS per role across orgs
…ase3-pr5

refactor(cli): migrate uninstall flows to harness-first agent discovery
ADR-0045 Phase 3, PR 4: loadKnownSlugs now discovers agent identity
from harness wrapper files in the config repo via DiscoverRemoteAgents
before falling back to the config.yaml agents: block. When the legacy
path is used, a deprecation warning is emitted.

Signed-off-by: Greg Allen <gallen@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
Call h.Lint() after harness loading in both `fullsend run` and
`fullsend lock` commands to surface non-fatal warnings. Currently
warns when the `role` field is missing from a harness file.

This is Phase 3 PR 3 of ADR-0045. Lint diagnostics are informational
only — commands still succeed regardless of warnings.

For `fullsend lock`, diagnostics are deduplicated across forge
variants and include the agent name for context.

Severity-aware emission: warnings use StepWarn, errors use StepFail
to ensure future SeverityError diagnostics are visually distinct.

Signed-off-by: Greg Allen <gallen@redhat.com>
Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
…ase3-pr4

refactor(harness): migrate loadKnownSlugs to harness-first discovery
…ase3-pr3

feat(harness): wire Lint() diagnostics into fullsend run and lock
…tus-token-deprecations

refactor: remove deprecated status-token fallback paths
…ovate

Signed-off-by: Hector Martinez <hemartin@redhat.com>
TestVendoredDefaultsInfraPathsMatchPredicate and
TestEnumerateVendoredPathsMatchesCollectInCheckout failed because
the new .github/scripts/{install,version}-openshell.sh files are
matched by isVendoredDefaultsInfra but were absent from the
hardcoded vendoredDefaultsInfraPaths slice.

Signed-off-by: Hector Martinez <hemartin@redhat.com>
…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 8:00 AM UTC · Completed 8:17 AM UTC
Commit: 53ee5b2 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [breaking-api-change] action.yml:39 — Breaking change to GitHub Action input interface: renamed status-token input to mint-url. Any workflows in other repositories that invoke this action with status-token will break, as the input parameter has been removed without a deprecation period.
    Remediation: Keep both status-token (deprecated) and mint-url inputs during a transition period. Add fallback logic and remove status-token only in a major version bump.

  • [protected-path] .github/, .pre-commit-config.yaml, CLAUDE.md, skills/ — This PR modifies 14 protected files (.github/dependabot.yml, .github/scripts/install-openshell.sh, .github/scripts/openshell-version.sh, .github/workflows/reusable-code.yml, .github/workflows/reusable-fix.yml, .github/workflows/reusable-prioritize.yml, .github/workflows/reusable-retro.yml, .github/workflows/reusable-review.yml, .github/workflows/reusable-triage.yml, .pre-commit-config.yaml, CLAUDE.md, skills/e2e-health/SKILL.md, skills/e2e-health/scripts/list-runs.sh, skills/mint-enroll/SKILL.md). No linked issue justifies why these governance and infrastructure files are being changed. Human approval is required for all protected-path changes.

  • [Protected-path enforcement removal] internal/scaffold/fullsend-repo/scripts/post-fix.sh — The entire protected-path enforcement block (PROTECTED_PATHS array and check loop) is removed from post-fix.sh, shifting enforcement from pre-push blocking (hard gate) to post-review enforcement (soft gate via post-review.sh). If the review stage is skipped or fails, protected-path changes could be pushed without enforcement.
    Remediation: Retain the protected-path check in post-fix.sh as defense-in-depth, or document the security model change and verify all fix agent pushes go through review before merge.

  • [schema-breaking-change] internal/scaffold/fullsend-repo/schemas/triage-result.schema.json:12 — Breaking change to triage agent output schema: blocked enum value replaced with prerequisites, blocked_by string field replaced with prerequisites complex object. Existing triage agents outputting the blocked action will fail schema validation.
    Remediation: Accept both blocked and prerequisites enum values during a transition period. Support both field formats. Add migration logic in post-triage script.

Medium

  • [scope-creep] — PR title claims fix(#2378) (a single bug fix) but the diff includes 96 commits changing 122 files: mint infrastructure refactoring (ROLE_APP_IDS model change), new ADR 0047, triage prerequisites feature, status-token → mint-url migration, workflow changes. The actual status-comment fix is <50 lines.

  • [GHA workflow command injection] internal/scaffold/fullsend-repo/scripts/post-triage.sh — The new prerequisites handler emits GHA ::warning:: commands with agent-controlled TARGET_REPO and CREATED_URL values. While TARGET_REPO is constrained by schema regex and an allowlist, sanitization for :: sequences and encoded newlines is not applied.

  • [architectural-drift] internal/dispatch/gcf/provisioner.go — ROLE_APP_IDS changes from per-org keyed entries ({org}/{role}) to shared role-only keys ({role}), removing org isolation at the app ID mapping level. This is documented in architecture.md updates but is a significant design change bundled with the stated fix. See also: [authorization-model-change] on internal/mintcore/handler.go.

  • [cross-repo-issue-authorization] internal/scaffold/fullsend-repo/scripts/post-triage.sh — New prerequisites handler creates GitHub issues in external repositories with agent-controlled title and body. Allowlist mitigates unauthorized repo targeting, but schema has no maxLength on title or body fields.

  • [curl-pipe-to-shell] .github/scripts/install-openshell.sh:14 — Downloads and executes remote script via curl | sh. URL is pinned to a specific commit SHA but no checksum validation is performed on the downloaded content.

  • [interface-extension] internal/forge/forge.go — New methods DeleteFiles (line 193) and GetWorkflow (line 260) added to forge.Client interface. Any alternative implementations must be updated.

Low

  • [nil-deref] internal/statuscomment/statuscomment.go:62New() receives nil client when ClientFactory is configured. Currently safe because refreshClient is called before every API path, but no defensive nil guard exists in the no-factory path.

  • [test-inadequate] internal/statuscomment/statuscomment_test.go:869 — No test verifying refreshClient is called across both PostStart and PostCompletion in a single lifecycle.

  • [fail-open] internal/dispatch/gcf/provisioner.go — Removed defense-in-depth cross-check between ALLOWED_ORGS and ROLE_APP_IDS. No replacement consistency check added.

  • [GHA workflow command injection] internal/scaffold/fullsend-repo/scripts/post-code.shAGENT_EXIT_CODE interpolated into ::error:: messages. Value is set via Go fmt.Sprintf("%d", ...) guaranteeing numeric output, so risk is minimal.

Info

  • [runtime-mechanism] internal/mintcore/handler.go:253handleHealth returns HTTP 503 when mint has legacy ROLE_APP_IDS but no role-only keys. May cause load balancers to remove the service during migration while the handler still processes requests correctly via fallback.

  • [secrets-handling] action.yml — Removed add-mask for STATUS_TOKEN, replaced with mint URL. Security improvement (no long-lived token in environment).

  • [mirror-pr-process] — This PR is a mirror of upstream fix(#2378): report failure when agent errors with no commits fullsend-ai/fullsend#2381 bundling many changes. Consider adopting a clear mirror-PR naming convention.

Previous run

Review

Reason: stale-head

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

Previous run (2)

Review

Reason: stale-head

The review agent reviewed commit e3859411ba49546e5bf86c0fa9eb98ba22fbc390 but the PR HEAD is now 92fe4759c9761ef6ecb4dde523e26d516cbb2f22. 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 8:20 AM UTC · Completed 8:59 AM UTC
Commit: 53ee5b2 · View workflow run →

QualityFlow and others added 6 commits June 21, 2026 08:21
Refined STP from APPROVED_WITH_FINDINGS to APPROVED.
Addressed 14 findings (8 major, 6 minor) in 1 iteration.
Weighted score improved from 72.5 to 94.25.
All 17 STD scenarios are Documentation Review tier — no automated
test code generated. Summary output only.
Replaces intermediate pipeline artifacts with organized test files.

Total: 0 test files → qf-tests/GH-55/
Jira: GH-55
[skip ci]
@github-actions

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
0 files qf-tests/GH-55/

Issue: GH-55


Generated by QualityFlow

@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 9:01 AM UTC · Completed 9:18 AM UTC
Commit: 53ee5b2 · 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 action.yml
default: ""
status-token:
description: Token for status comments (defaults to GH_TOKEN env var).
mint-url:

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] breaking-api-change

Breaking change to GitHub Action input interface: renamed status-token input to mint-url. Any workflows in other repositories that invoke this action with status-token will break, as the input parameter has been removed without a deprecation period.

Suggested fix: Keep both status-token (deprecated) and mint-url inputs during a transition period. Add fallback logic and remove status-token only in a major version bump.

"action": {
"type": "string",
"enum": ["insufficient", "duplicate", "sufficient", "blocked", "question"]
"enum": ["insufficient", "duplicate", "sufficient", "prerequisites", "question"]

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] schema-breaking-change

Breaking change to triage agent output schema: blocked enum value replaced with prerequisites, blocked_by string field replaced with prerequisites complex object. Existing triage agents outputting blocked action will fail schema validation.

Suggested fix: Accept both blocked and prerequisites enum values during a transition period. Support both field formats. Add migration logic in post-triage script.

SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
source "${SCRIPT_DIR}/openshell-version.sh"

echo "Installing OpenShell ${OPENSHELL_VERSION} (${OPENSHELL_SHA})"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] curl-pipe-to-shell

Downloads and executes remote script via curl | sh. URL is pinned to a specific commit SHA but no checksum validation is performed.

Suggested fix: Download to file first, compute SHA-256 hash, and compare against pinned expected hash before execution.

Comment thread internal/forge/forge.go
@@ -185,6 +193,11 @@ type Client interface {
GetFileContent(ctx context.Context, owner, repo, path string) ([]byte, 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.

[medium] interface-extension

New methods DeleteFiles and GetWorkflow added to forge.Client interface. Any alternative implementations must be updated.

@guyoron1 guyoron1 closed this Jun 21, 2026
@guyoron1 guyoron1 deleted the mirror/2381-2378-status-comment-agent-error branch June 21, 2026 10:51
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