Skip to content

fix(#1997): notify issue when pre-commit blocks PR creation - #1998

Closed
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/1997-precommit-failure-notification
Closed

fix(#1997): notify issue when pre-commit blocks PR creation#1998
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/1997-precommit-failure-notification

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

When post-code.sh's authoritative pre-commit check fails, the bash ERR trap does not fire because the pre-commit command runs inside an if conditional. The script exits non-zero but never posts a comment to the source issue, leaving humans with no feedback.

Add an explicit failure comment in the pre-commit block path that:

  • Distinguishes "agent wrote code, PR blocked by pre-commit" from
    generic push/PR failures and agent crashes
  • Includes the workflow run link for easy log access
  • Shows the last 30 lines of hook output in a collapsed details
    block so the human can see which hooks failed
  • Suggests retrying with /fs-code or fixing locally

Also capture pre-commit output to a temp file via tee so the excerpt can be included in the comment. The temp file is cleaned up on both success and failure paths.

Add corresponding tests to post-code-test.sh that verify the comment body content, org-mode URL routing, and absence of generic push/PR failure language.

Note: pre-commit could not run in sandbox (Go toolchain permission error). The post-script runs pre-commit authoritatively on the runner.


Closes #1997

Post-script verification

  • Branch is not main/master (agent/1997-precommit-failure-notification)
  • Secret scan passed (gitleaks — 9ea038f26b4c37a1000c1fe5a42303217a79357b..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

When post-code.sh's authoritative pre-commit check fails, the bash
ERR trap does not fire because the pre-commit command runs inside
an `if` conditional. The script exits non-zero but never posts a
comment to the source issue, leaving humans with no feedback.

Add an explicit failure comment in the pre-commit block path that:
- Distinguishes "agent wrote code, PR blocked by pre-commit" from
  generic push/PR failures and agent crashes
- Includes the workflow run link for easy log access
- Shows the last 30 lines of hook output in a collapsed details
  block so the human can see which hooks failed
- Suggests retrying with /fs-code or fixing locally

Also capture pre-commit output to a temp file via tee so the
excerpt can be included in the comment. The temp file is cleaned
up on both success and failure paths.

Add corresponding tests to post-code-test.sh that verify the
comment body content, org-mode URL routing, and absence of generic
push/PR failure language.

Note: pre-commit could not run in sandbox (Go toolchain permission
error). The post-script runs pre-commit authoritatively on the
runner.

Closes #1997
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

Site preview

Preview: https://8ba89b4b-site.fullsend-ai.workers.dev

Commit: 61e4ce3c1bbdbcc7e62c8f73fbd9676758cdfc01

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Medium

  • [injection] internal/scaffold/fullsend-repo/scripts/post-code.sh:228 — Markdown injection via pre-commit hook output. The hook_excerpt variable (last 30 lines of pre-commit output) is interpolated inside a fenced code block in the GitHub issue comment. A malicious .pre-commit-config.yaml in the target repo could define a hook that prints output containing triple-backtick sequences, breaking out of the code fence and injecting arbitrary Markdown into the issue comment — e.g., misleading remediation instructions or phishing links presented as authoritative bot guidance. Blast radius is limited to the repo's own issues.
    Remediation: Sanitize hook_excerpt before interpolation by escaping triple-backtick sequences, e.g.: hook_excerpt="$(tail -n 30 "${PRECOMMIT_OUTPUT}" | sed 's/\x60\x60\x60/\x60 \x60 \x60/g')". Alternatively, strip backtick-only lines from the excerpt.

Low

  • [injection] internal/scaffold/fullsend-repo/scripts/post-code.sh:234 — The BRANCH variable is interpolated inside an inline code span. Branch names can contain backtick characters, which would break the inline code span. Low risk because branch names are set by the code agent, not external attackers.

  • [edge-case] internal/scaffold/fullsend-repo/scripts/post-code.sh:209 — The pre-commit output capture relies on set -o pipefail (line 31) to propagate pre-commit's non-zero exit through the | tee pipeline. If a future maintainer removes pipefail, tee would mask the failure. Consider adding a comment near the pipeline noting this dependency.

  • [comment-footer-consistency] internal/scaffold/fullsend-repo/scripts/post-code.sh:228 — Sibling scripts (pre-code.sh, post-review.sh, pre-review.sh) append a <sub>Posted by fullsend</sub> footer to issue/PR comments. The new pre-commit failure comment (and the existing report_failure_to_issue comment) omit this footer. Not blocking, but aligning all post-code.sh comments with the sibling convention would improve attribution consistency.

Info

  • [test-integrity] internal/scaffold/fullsend-repo/scripts/post-code-test.sh — Tests verify a reimplemented build_precommit_failure_comment helper rather than the actual production code. This is consistent with the established test pattern in the file (all helpers reimplement production logic), but means tests cannot catch drift between the helper and production template.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 8, 2026
@rh-hemartin

rh-hemartin commented Jun 8, 2026

Copy link
Copy Markdown
Member

We have something opened that will shape this how this is done (closing for now until #1871 gets merged).

@rh-hemartin rh-hemartin closed this Jun 8, 2026
@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #1998 — pre-commit failure notification (closed without merge)

Timeline

  1. 05:58 — Human (Omeramsc) filed issue post-code: notify issue when pre-commit block prevents PR creation #1997: post-code.sh doesn't notify the source issue when pre-commit blocks PR creation.
  2. 05:58–06:02 — Triage agent ran, labeled ready-to-code, and noted #1859 as a "complementary" related issue.
  3. 06:03–06:09 — Code agent produced PR #1998 with 170 additions across 2 files.
  4. 06:09–06:19 — Review agent posted findings: one medium-severity markdown injection, two low-severity issues, one info-level note.
  5. 08:27 — Human (rh-hemartin) closed PR without merge, stating PR #1871 (which closes feat: post agent status comments on workflow start and completion #1859) needs to land first because it reshapes how agent status comments work.

Assessment

Wasted work: The full triage → code → review pipeline completed (~30 min of agent compute), but the PR was discarded because a related in-flight PR changes the implementation surface. The triage agent identified #1859 as related but didn't escalate this to a blocking dependency.

Review quality was strong: The injection finding on hook_excerpt interpolation was legitimate and actionable.

Existing issue overlap: #1650 covers issues that directly reference open PRs in their text, but not the transitive case demonstrated here (triage discovers related issue → that issue has an open PR that changes the implementation approach). One proposal filed below to address this gap.

Proposals filed

@github-actions
github-actions Bot deleted the agent/1997-precommit-failure-notification branch July 12, 2026 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

post-code: notify issue when pre-commit block prevents PR creation

1 participant