Skip to content

Add an opt-in verification gate to watch dispatches - #2360

Open
omercangumus wants to merge 1 commit into
github:devfrom
omercangumus:omercangumus/1712-watch-verification-gate
Open

omercangumus wants to merge 1 commit into
github:devfrom
omercangumus:omercangumus/1712-watch-verification-gate

Conversation

@omercangumus

Copy link
Copy Markdown
Contributor

Summary

  • Add an opt-in watch.verify capability that runs configured commands, or detected build, test, and lint scripts, after successful dispatches.
  • Run checks sequentially with fail-fast behavior and surface local verification failures separately from remote CI status.
  • Document the config and add focused coverage.

Closes #1712

Checks

  • npm run lint passed.
  • npm run build -w packages/squad-sdk and npm run build -w packages/squad-cli passed.
  • Focused watch tests passed (28/28).
  • Docs lint and spelling passed.
  • The full test suite could not be completed locally: workflow tests require jq, which is unavailable in this Windows environment, and unrelated integration tests timed out. This PR is draft pending GitHub CI.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

🟡 Impact Analysis — PR #2360

Risk tier: 🟡 MEDIUM

📊 Summary

Metric Count
Files changed 12
Files added 4
Files modified 8
Files deleted 0
Modules touched 4
Critical files 2

🎯 Risk Factors

  • 12 files changed (6-20 → MEDIUM)
  • 4 modules touched (2-4 → MEDIUM)
  • Critical files touched: packages/squad-cli/src/cli/commands/watch/capabilities/index.ts, packages/squad-cli/src/cli/commands/watch/index.ts

📦 Modules Affected

docs (1 file)
  • docs/src/content/docs/features/ralph.md
root (1 file)
  • .changeset/watch-verification-gate.md
squad-cli (6 files)
  • packages/squad-cli/src/cli/commands/watch/capabilities/execute.ts
  • packages/squad-cli/src/cli/commands/watch/capabilities/index.ts
  • packages/squad-cli/src/cli/commands/watch/capabilities/verification.ts
  • packages/squad-cli/src/cli/commands/watch/external-loader.ts
  • packages/squad-cli/src/cli/commands/watch/index.ts
  • packages/squad-cli/src/cli/commands/watch/types.ts
tests (4 files)
  • test/cli/watch-execute.test.ts
  • test/cli/watch-process-tree.test.ts
  • test/cli/watch-verification.test.ts
  • test/cli/watch.test.ts

⚠️ Critical Files

  • packages/squad-cli/src/cli/commands/watch/capabilities/index.ts
  • packages/squad-cli/src/cli/commands/watch/index.ts

This report is generated automatically for every PR. See #733 for details.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit 02ac551

PR Scope: 📦🔧 Mixed (product + infrastructure)

⚠️ 2 item(s) to address before review

Status Check Details
✅ Single commit 1 commit — clean history
✅ Not in draft Ready for review
✅ Branch up to date Up to date with dev
❌ Copilot review No Copilot review yet — it may still be processing
✅ Changeset present Changeset file found
✅ Scope clean No .squad/ or docs/proposals/ files
✅ No merge conflicts No merge conflicts
✅ Copilot threads resolved No Copilot review threads
❌ CI passing 15 check(s) still running

Files Changed (12 files, +309 −6)

File +/−
.changeset/watch-verification-gate.md +5 −0
docs/src/content/docs/features/ralph.md +12 −0
packages/squad-cli/src/cli/commands/watch/capabilities/execute.ts +1 −1
packages/squad-cli/src/cli/commands/watch/capabilities/index.ts +2 −0
packages/squad-cli/src/cli/commands/watch/capabilities/verification.ts +97 −0
packages/squad-cli/src/cli/commands/watch/external-loader.ts +1 −1
packages/squad-cli/src/cli/commands/watch/index.ts +38 −2
packages/squad-cli/src/cli/commands/watch/types.ts +1 −1
test/cli/watch-execute.test.ts +22 −0
test/cli/watch-process-tree.test.ts +19 −0
test/cli/watch-verification.test.ts +110 −0
test/cli/watch.test.ts +1 −1

Total: +309 −6


This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

@omercangumus
omercangumus marked this pull request as ready for review October 28, 2026 23:44
@omercangumus
omercangumus force-pushed the omercangumus/1712-watch-verification-gate branch from 9f564c9 to 075ebde Compare October 31, 2026 22:32
@tamirdresher

Copy link
Copy Markdown
Collaborator

Thanks Omer, the sequencing here is solid: verification after post-execute, opt-in, sequential stop on first failure. Two things block this from the security side so I am holding it for now.

  1. In verify: true mode the commands come from package.json in the current working tree, which is the tree the --yolo Ralph session just edited while acting on issue text. That turns any agent file edit, including a pretest hook, into unprompted shell execution with the watch process environment. Our probe hook ran and read an inherited env var. Please pin autodetect to the base ref (git show <base>:package.json) or make explicit commands the only source, and add a SECURITY note like the one external-loader.ts prints.
  2. exec() timeout only kills the shell, so npm run grandchildren survive. Please use the existing runProcessTreeCommand so the tree is reaped.

Separately, the result currently only changes a board counter. Either let it act on the dispatched issue (block the next dispatch) or call it a report rather than a gate.

Happy to re-review at a new head. This one will not make preview.4 as is, no rush.

@omercangumus
omercangumus force-pushed the omercangumus/1712-watch-verification-gate branch from 075ebde to c6a16a5 Compare November 1, 2026 11:32
@omercangumus

Copy link
Copy Markdown
Contributor Author

Good catches, thanks. I removed the package.json autodetect; verification now only runs commands already set in watch.verify.commands, with a warning before they run. Timeouts use runProcessTreeCommand, and a failed check stops watch before another dispatch. I updated the docs and tests too. Pushed as c6a16a5; all checks are green.

Run configured checks after successful dispatches and report local failures separately from remote CI.

Closes github#1712

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@omercangumus
omercangumus force-pushed the omercangumus/1712-watch-verification-gate branch from c6a16a5 to 02ac551 Compare November 1, 2026 19:47

This branch has not been deployed

No deployments
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.

watch: no verification gate - round success is the child process exit code, nothing builds, tests, or lints the resulting work

2 participants