Skip to content

fix(#2348): stop swallowing gh pr create stderr in post-code.sh - #2349

Merged
ralphbean merged 1 commit into
mainfrom
agent/2348-fix-pr-create-stderr
Jun 16, 2026
Merged

fix(#2348): stop swallowing gh pr create stderr in post-code.sh#2349
ralphbean merged 1 commit into
mainfrom
agent/2348-fix-pr-create-stderr

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Replace the command substitution with 2>&1 redirect on the gh pr create call with the if-! pattern already used in reconcile-repos.sh. Previously, when gh pr create failed, stderr (containing the API error like 403 or 422) was captured into the PR_URL variable instead of flowing to the workflow logs, making failures impossible to debug.

The new pattern lets stderr print to the log naturally while still capturing the PR URL on success. On failure, it emits a GitHub Actions error annotation and exits non-zero.

Note: pre-commit and make lint could not run in the sandbox due to shellcheck-py failing to download (network restriction). The post-script runs an authoritative pre-commit check on the runner. bash -n syntax check passed.


Closes #2348

Post-script verification

  • Branch is not main/master (agent/2348-fix-pr-create-stderr)
  • Secret scan passed (gitleaks — 966abbf5f5daec2d012b559d0e1a9808a8839652..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Replace the command substitution with 2>&1 redirect on the
gh pr create call with the if-! pattern already used in
reconcile-repos.sh. Previously, when gh pr create failed,
stderr (containing the API error like 403 or 422) was captured
into the PR_URL variable instead of flowing to the workflow
logs, making failures impossible to debug.

The new pattern lets stderr print to the log naturally while
still capturing the PR URL on success. On failure, it emits
a GitHub Actions error annotation and exits non-zero.

Note: pre-commit and make lint could not run in the sandbox
due to shellcheck-py failing to download (network restriction).
The post-script runs an authoritative pre-commit check on the
runner. bash -n syntax check passed.

Closes #2348
@github-actions

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://d02c0727-site.fullsend-ai.workers.dev

Commit: e66f2d92fdff4bdbc543d352c678db782d9baa4f

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 16, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:52 PM UTC · Completed 7:00 PM UTC
Commit: e66f2d9 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Low

  • [error-message-consistency] internal/scaffold/fullsend-repo/scripts/post-code.sh:415 — Error message "Failed to create PR: see above for details" is less specific than the established pattern in reconcile-repos.sh, which uses Failed to create PR for $REPO. The post-code.sh context has ISSUE_NUMBER and BRANCH available and could include them for faster log triage. However, since post-code.sh only has one PR creation call (unlike reconcile-repos.sh which loops), the generic message is less ambiguous here.
    Remediation: Consider using Failed to create PR for issue #${ISSUE_NUMBER} or Failed to create PR (branch: ${BRANCH}) to provide more context.

Info

  • [scope-authorization] Change is properly authorized by issue bug: post-code.sh swallows gh pr create error output via 2>&1 redirect #2348 (type/bug, priority/medium, ready-to-code). The fix addresses exactly the scope described in the issue.
  • [architectural-coherence] The change aligns with the defensive scripting patterns evident throughout post-code.sh and follows the if-! pattern already used in reconcile-repos.sh.

--body "${PR_BODY}" \
2>&1)"
--body "${PR_BODY}"); then
echo "::error::Failed to create PR: see above for details"

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] error-message-consistency

Error message 'Failed to create PR: see above for details' is less specific than the established pattern in reconcile-repos.sh, which uses 'Failed to create PR for $REPO'. The post-code.sh context has ISSUE_NUMBER and BRANCH available and could include them for faster log triage. However, since post-code.sh only has one PR creation call (unlike reconcile-repos.sh which loops), the generic message is less ambiguous here.

Suggested fix: Consider using 'Failed to create PR for issue #${ISSUE_NUMBER}' or 'Failed to create PR (branch: ${BRANCH})' to provide more context.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jun 16, 2026
@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM.

@ralphbean
ralphbean added this pull request to the merge queue Jun 16, 2026
Merged via the queue into main with commit 9241475 Jun 16, 2026
16 checks passed
@ralphbean
ralphbean deleted the agent/2348-fix-pr-create-stderr branch June 16, 2026 20:24
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 16, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 8:29 PM UTC · Completed 8:36 PM UTC
Commit: e66f2d9 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2349 — fix(#2348): stop swallowing gh pr create stderr

Workflow quality: Excellent. This was a clean, single-cycle workflow with no rework.

Timeline

  1. 18:31 — Issue #2348 filed (bug: post-code.sh swallows gh pr create stderr)
  2. 18:32–18:37 — Triage agent confirms the bug and recommends the if-! pattern from reconcile-repos.sh
  3. 18:38–18:48 — Code agent produces a correct fix in a single commit (+5/−3, one file), opens PR #2349
  4. 18:49–19:00 — Review agent approves with one low-severity finding (error message could be more specific)
  5. 20:14 — Human (ralphbean) approves with "LGTM"
  6. 20:24 — PR merged

Assessment

  • Review quality: Good. The review agent correctly approved a clean fix and raised a reasonable low-severity suggestion about error message specificity. The finding was appropriately calibrated — it didn't block the PR.
  • Rework rate: Zero. Code agent got it right on the first attempt.
  • Token cost: Efficient. Single commit, single review cycle, no fix iterations.
  • Time to resolution: ~2 hours from issue to merge. The bottleneck was human review latency (~75 minutes), which is normal.

Observations (covered by existing issues)

  • Two retro runs triggered — Already tracked in #2190 (retro dispatch should depend on review completion).
  • Code agent couldn't run pre-commit/lint in sandbox — Already tracked in #2108 and #1719. The post-script ran pre-commit successfully on the runner, so the system handled this correctly.
  • Low finding about error message consistency not addressed before merge — The review correctly did not block on this. Issue #2030 already tracks code agent pattern consistency.

No new proposals. All improvement opportunities identified are already covered by existing open issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: post-code.sh swallows gh pr create error output via 2>&1 redirect

1 participant