fix(#1782): skip review stage on closed PR or skipped author by fullsend-ai-coder[bot] · Pull Request #1783 · fullsend-ai/agents

fullsend-ai-coder · 2026-10-24T19:53:00Z

Summary

Changes in scripts/pre-review.src.sh (and the regenerated bundle scripts/pre-review.sh): both early skip exits now emit the pre-script output protocol skip signal, so fullsend run stops before creating the sandbox:

Closed/merged PR: writes skipped=true and reason=PR #<n> is already <state>
Author in REVIEW_SKIP_AUTHORS: writes skipped=true and reason=PR author <author> is in REVIEW_SKIP_AUTHORS (the author goes through _gha_sanitize, which strips newlines, so the reason stays on one line)

Before this change both paths posted a "Review skipped" comment and then ran exit 0 without the signal. The review agent still ran (about 4 minutes plus model spend), and on closed PRs post-review's own guard posted a second "Review skipped" comment.

The script now sources lib/prescript-output.lib.sh, as pre-code.src.sh does. With a CLI that predates the protocol, FULLSEND_PRESCRIPT_OUTPUT is unset and prescript_output does nothing, so behavior there is unchanged.

Behaviour change on upgrade

A PR whose author is in REVIEW_SKIP_AUTHORS now gets no agent review at all, as docs/review.md describes. Before, the agent still reviewed it, so an approval could re-add ready-for-merge.
On a closed or merged PR, post-review no longer runs, so its duplicate "Review skipped" comment is gone. The run's status comment reports the skip and its reason.

Testing

New protocol tests in scripts/pre-review-test.sh:

MERGED and CLOSED PRs write exactly skipped=true plus the reason line
A skipped author, including case and whitespace variants, writes exactly skipped=true plus the reason line
An open PR with an unlisted author, and the no-token path, leave the file empty
With FULLSEND_PRESCRIPT_OUTPUT unset, both skip paths still exit 0, print the skip notice and post the comment

All new tests pass. shellcheck (v0.10.0.1, repo args) and the pre-commit-hooks v6.0.0 checks were run directly on the changed files and pass; pre-commit run itself couldn't fetch hook repos in the sandbox because of a TLS error. The bundled pre-review.sh was regenerated with scripts/bundle-sh.sh.

Caveats

The four fixture-00x-*-projection tests in pre-review-test.sh fail in the sandbox because yq isn't installed. They fail the same way on main and are unrelated to this change.
make script-test and the behaviour suite could not run in the sandbox (make couldn't spawn /bin/sh) and need to be checked in CI.
Triage noted that older issue pre-review.sh should use the prescript output protocol to skip sandbox creation on merged/closed PRs #642 covers the closed/merged half of this problem. This PR may also resolve it.

🤖 Generated with Claude Code

Closes #1782

Post-script verification

Branch is not main/master (agent/1782-pre-review-skip-signal)
Secret scan passed (gitleaks — 372c909351b43d71d23e7d95978ebe607a3ddcfa..HEAD)
PR body secret scan passed (gitleaks — no-git)

waynesun09 · 2026-10-24T19:54:05Z

/fs-fix-stop

github-actions · 2026-10-24T19:54:15Z

Fix agent disabled for this PR. Remove the fullsend-no-fix label or use /fs-fix to re-engage.

fullsend-ai-review · 2026-10-24T19:55:17Z

🤖 Finished Review · ⚠️ Cancelled · Started 7:55 PM UTC · Completed 7:57 PM UTC

Commit: 8378800 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $0.24

This review was superseded by a newer commit. Check the current pull request HEAD's review status before merging.

fullsend-ai-review · 2026-10-24T19:58:51Z

🤖 Finished Review · ⚠️ Cancelled · Started 7:58 PM UTC · Completed 8:01 PM UTC

Commit: 5beeb79 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $0.29

This review was superseded by a newer commit. Check the current pull request HEAD's review status before merging.

pre-review.src.sh exited 0 after posting a "Review skipped" comment for closed/merged PRs and for authors in REVIEW_SKIP_AUTHORS, but never wrote the pre-script output protocol skip signal. fullsend run therefore still created the sandbox and ran the review agent, and post-review's own closed-PR guard posted a second "Review skipped" comment. Source lib/prescript-output.lib.sh (as pre-code.src.sh does) and write skipped=true plus a single-line reason on both skip exits before exiting 0. Under a CLI that predates the protocol the variable is unset and prescript_output is a no-op, so behavior is unchanged there. Regenerate the bundled scripts/pre-review.sh. pre-review-test.sh gains protocol tests: closed/merged PRs and skipped authors (incl. case/whitespace variants) write the exact skip lines; an open PR and the no-token path leave the file empty; with FULLSEND_PRESCRIPT_OUTPUT unset both skip paths still comment and exit 0. Note: `pre-commit run` could not fetch hook repos in the sandbox (TLS error); the configured hooks (pre-commit-hooks v6.0.0, shellcheck v0.10.0.1, lint-release-cases) were run directly and passed, and gitleaks was covered by scan-secrets. The four fixture-00x projection tests in pre-review-test.sh fail in the sandbox because yq is not installed; they fail identically on main. Closes #1782 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>

fullsend-ai-review · 2026-10-24T20:02:45Z

🤖 Finished Review · ✅ Success · Started 8:02 PM UTC · Completed 8:23 PM UTC

Commit: c22dc88 · View workflow run →

Runtime: pi · Model: sonnet → claude-sonnet-5-5 · Effort: high · Cost: $1.63

fullsend-ai-review · 2026-10-24T20:23:00Z

Risk Assessment: moderate (2/5)

Details

Moderate risk change modifying protected pre-review scripts without a feature flag and touching churned files, balanced by small diff size, full acceptance criteria alignment, and bot authorship.

fullsend-ai-review · 2026-10-24T20:23:02Z

Review

Findings

Medium

[protected-path] scripts/pre-review.src.sh — This PR modifies protected paths: scripts/pre-review.src.sh, scripts/pre-review.sh, scripts/pre-review-test.sh. The PR links issue fix(pre-review): skip the stage when the PR is closed or the author is skipped #1782 and explains the rationale (skip signal for closed/merged PRs and REVIEW_SKIP_AUTHORS), but human approval is always required for protected-path changes, regardless of context.

waynesun09

Approving at c22dc88: pre-review's closed/merged and skip-author exits now emit the pre-script skip, so no sandbox starts; behaviour run 40127849705 passed 11/11 and the reject scenario dropped from ~440 s to 332 s. Protected path (scripts/) human-approved here.

The reject scenario's second /fs-review round only checked that the review workflow succeeded and posted the "Review skipped" comment. Both hold whether or not the agent ran, so the scenario could not prove that pre-review stopped the stage (fixed in fullsend-ai#1783). Bump github.com/fullsend-ai/fullsend to main 3ea9dd1b8 (fullsend-ai/fullsend#8793, for fullsend-ai/fullsend#8792), which adds the step "the harness <agent> run was skipped by its pre-script". It reads the run's host telemetry for fullsend.prescript.skipped=true and requires no agent output. Add it after the second round's workflow step. The old pin's go.sum entries are replaced, matching previous bumps; fullsend's go.mod hash is unchanged. Verification: the behaviour package vets and compiles against the new pin with the CI-style -modfile replace setup, and the new line matches the registered step regex. harness/triggertest passes. Live behaviour scenarios could not run in the sandbox. make cannot run here (sandbox blocks /bin/sh); running the script-test commands directly gives the same 15 environment-only failures on main and on this branch. skillsaw --strict passed. pre-commit could not fetch hook repos; pre-commit-hooks v6.0.0 checks and lint-release-cases run directly passed, and scan-secrets (gitleaks) passed. Closes fullsend-ai#1784 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

fullsend-ai-coder Bot requested a review from a team as a code owner October 24, 2026 19:53

fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Oct 24, 2026

fullsend-ai-coder Bot assigned waynesun09 Oct 24, 2026

fullsend-ai-coder Bot had a problem deploying to dev October 24, 2026 19:53 Error

github-actions Bot added the fullsend-no-fix Skip bot-triggered fix agent runs label Oct 24, 2026

waynesun09 had a problem deploying to dev October 24, 2026 19:57 — with GitHub Actions Error

fullsend-ai-coder Bot and others added 3 commits October 24, 2026 16:00

refactor(#1782): trim the pre-review skip-signal comment

bdb1f84

Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>

test(#1782): describe the closed-PR skip as stopping the run

c22dc88

Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>

waynesun09 force-pushed the agent/1782-pre-review-skip-signal branch from 5beeb79 to c22dc88 Compare October 24, 2026 20:00

waynesun09 deployed to dev October 24, 2026 20:01 — with GitHub Actions Active

fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Oct 24, 2026

fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Oct 24, 2026

waynesun09 approved these changes Oct 24, 2026

View reviewed changes

waynesun09 added this pull request to the merge queue Oct 24, 2026

Merged via the queue into main with commit 5543e09 Oct 24, 2026
66 checks passed

waynesun09 deleted the agent/1782-pre-review-skip-signal branch October 24, 2026 20:34

waynesun09 mentioned this pull request Oct 24, 2026

test(behaviour): prove the closed-PR review skip stops the stage #1784

Closed

fullsend-ai-coder Bot mentioned this pull request Oct 24, 2026

test(#1784): assert the closed-PR review skip stops the stage #1785

Merged

3 tasks done

Navigation Menu

Repository navigation

fix(#1782): skip review stage on closed PR or skipped author - #1783

fix(#1782): skip review stage on closed PR or skipped author#1783
waynesun09 merged 3 commits into
mainfrom
agent/1782-pre-review-skip-signal

fullsend-ai-coder Bot commented Oct 24, 2026 •

edited by waynesun09

Loading

Uh oh!

waynesun09 commented Oct 24, 2026

Uh oh!

github-actions Bot commented Oct 24, 2026

Uh oh!

fullsend-ai-review Bot commented Oct 24, 2026 •

edited

Loading

Uh oh!

fullsend-ai-review Bot commented Oct 24, 2026 •

edited

Loading

Uh oh!

fullsend-ai-review Bot commented Oct 24, 2026 •

edited

Loading

Uh oh!

fullsend-ai-review Bot commented Oct 24, 2026

Uh oh!

fullsend-ai-review Bot commented Oct 24, 2026

Uh oh!

waynesun09 left a comment

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

fullsend-ai-coder Bot commented Oct 24, 2026 • edited by waynesun09 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Behaviour change on upgrade

Testing

Caveats

Post-script verification

Uh oh!

waynesun09 commented Oct 24, 2026

Uh oh!

github-actions Bot commented Oct 24, 2026

Uh oh!

fullsend-ai-review Bot commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Oct 24, 2026

Uh oh!

fullsend-ai-review Bot commented Oct 24, 2026

Review

Findings

Medium

Uh oh!

waynesun09 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fullsend-ai-coder Bot commented Oct 24, 2026 •

edited by waynesun09

Loading

fullsend-ai-review Bot commented Oct 24, 2026 •

edited

Loading

fullsend-ai-review Bot commented Oct 24, 2026 •

edited

Loading

fullsend-ai-review Bot commented Oct 24, 2026 •

edited

Loading