Skip to content

feat(run): add --no-post-script flag#666

Merged
ralphbean merged 2 commits into
mainfrom
feat-no-post-script
May 6, 2026
Merged

feat(run): add --no-post-script flag#666
ralphbean merged 2 commits into
mainfrom
feat-no-post-script

Conversation

@waynesun09
Copy link
Copy Markdown
Contributor

Summary

  • Add --no-post-script flag to fullsend run that skips post-script execution while running agent inference normally
  • Named --no-post-script instead of --dry-run because the agent still runs full inference with real GCP costs — dry-run would imply no work is done
  • Post-scripts handle side effects like posting PR review comments, pushing branches, and creating PRs — skipping them lets users test agent output locally without writing to GitHub

Test plan

Allow users to run agent inference without post-script side effects
(posting PR comments, pushing branches, creating PRs). The agent
runs normally inside the sandbox, but the post-script is skipped
with a warning message.

Named --no-post-script instead of --dry-run because the agent still
runs full inference with real GCP costs — dry-run would be misleading.

Signed-off-by: Wayne Sun <gsun@redhat.com>
@fullsend-ai-review
Copy link
Copy Markdown

fullsend-ai-review Bot commented May 5, 2026

Review: #666

Head SHA: ec40bf0
Timestamp: 2026-05-05T18:18:43Z
Outcome: approve

Summary

Clean, well-scoped addition of a --no-post-script flag to fullsend run. The flag is properly threaded from cobra registration through to the defer closure that gates post-script execution. The naming choice (--no-post-script over --dry-run) is well-reasoned since the agent still runs full inference with real GCP costs. The security comment in action.yml is a strong defensive measure — it explicitly documents why this flag must never be exposed as a GitHub Actions input, since post-scripts enforce secret scanning, protected-path blocks, and review-downgrade controls. The GitHub Action has no input that could set this flag; an attacker would need to modify action.yml itself, which would be caught in code review. No findings across all six review dimensions.

Findings

None.

Footer

Outcome: approve
This review applies to SHA ec40bf0ae256f25c8f3c77aa1b3b76db5e2dab7d. Any push to the PR head clears this review and requires a new evaluation.

Previous run

Review: #666

Head SHA: 9f3c18e
Timestamp: 2026-05-05T00:00:00Z
Outcome: approve

Summary

Clean, well-scoped addition of a --no-post-script flag to fullsend run. The flag is correctly threaded from the cobra command through to the runAgent function, where it short-circuits the post-script deferred closure before any other checks. The naming choice over --dry-run is well-reasoned since agent inference still executes with real GCP costs. No correctness, security, or style issues found across all six review dimensions.

Findings

No findings.

Footer

Outcome: approve
This review applies to SHA 9f3c18e9b17f0ae987964fe6e88e7b5ca6fa017e. Any push to the PR head clears this review and requires a new evaluation.

Copy link
Copy Markdown

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

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.

- Log flag state in initial plan output so users see it immediately
- Include post-script name in skip warning for easier debugging
- Add defensive comment to composite action YAML warning against
  exposing --no-post-script as a workflow input in CI

Signed-off-by: Wayne Sun <gsun@redhat.com>
Copy link
Copy Markdown

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

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.

@waynesun09 waynesun09 requested a review from rh-hemartin May 5, 2026 18:50
Copy link
Copy Markdown
Contributor

@ralphbean ralphbean left a comment

Choose a reason for hiding this comment

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

Clean, well-scoped addition. The naming choice (--no-post-script over --dry-run) is precise, the flag is correctly threaded through the defer closure, and the security comment in action.yml accurately documents the risk. One note below on future hardening — not blocking.

# SECURITY: Never expose --no-post-script as a workflow input.
# Post-scripts enforce secret scanning, protected-path blocks,
# and review-downgrade controls. Skipping them in CI bypasses
# all post-push security gates.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[important — deferred] Good defensive comment. Worth noting: this is documentation of intent, not enforcement. A workflow that calls fullsend run directly (bypassing the composite action) could pass --no-post-script in CI.

A future hardening pass could add a programmatic guard — e.g., detect GITHUB_ACTIONS=true and refuse or warn when --no-post-script is used in CI. CODEOWNERS protection on .github/ provides a secondary control for now.

Not blocking this PR — flagging for future consideration.

@ralphbean ralphbean added this pull request to the merge queue May 6, 2026
Merged via the queue into main with commit e18e077 May 6, 2026
22 checks passed
@ralphbean ralphbean deleted the feat-no-post-script branch May 6, 2026 01:25
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.

2 participants