Skip to content

examples(pr-reviewer): keep the shell gate on the review file, and document why #459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
khaliqgant merged 1 commit into from
Sep 18, 2026
Merged

examples(pr-reviewer): keep the shell gate on the review file, and document why #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions examples/pr-reviewer/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,10 @@ mount), and the merge path (needs an approval event).
(`evaluateMergeOnGreenState`, `matchesConflictDirective`,
`isAuthorizedConflictCommander`) and tested, but the generated trigger
vocabulary has no such events yet.
- **`artifact_exists`.** The review step is gated with
`{ type: "subprocess_gate", command: "test -s .workforce/review.md" }`.
When the `artifact_exists` named gate lands, the swap is one line:
`.gate({ type: "artifact_exists", path: REVIEW_FILE })`.
- **Why `subprocess_gate`, not `artifact_exists`.** The review step is gated
with `{ type: "subprocess_gate", command: "test -s .workforce/review.md" }`
on purpose. `artifact_exists` judges the worker's journaled artifact list,
and that scanner skips dot-directories, so a file under `.workforce/` is
never listed; it also records path presence on content change only, so an
empty file would pass and a rerun writing identical text would fail. The
shell test requires a non-empty file and is idempotent.
10 changes: 6 additions & 4 deletions examples/pr-reviewer/pr-reviewer.flow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@
// receives a single authored source and does not resolve sibling imports, so
// the pure functions live below the flow and are exported for the tests.
//
// TODO(flows#434 follow-up): once the `artifact_exists` named gate lands,
// replace the `subprocess_gate` on the review step with
// `.gate({ type: "artifact_exists", path: REVIEW_FILE })`.
// The review step is deliberately gated with a shell test, not the
// `artifact_exists` named gate (flows#449): the worker's artifact scanner
// skips dot-directories, so `.workforce/review.md` is never journaled; and
// `test -s` also demands a non-empty file and passes on a rerun that rewrites
// identical text, which `artifact_exists` (path present, content changed)
// would not.
//
// Not yet wired, deliberately: the `check_run.completed` (merge-on-green) and
// `issue_comment.created` (`@relay fix conflicts`) events are not in the
Expand Down Expand Up @@ -130,7 +133,6 @@ const reviewerBody = flow<Input>(
cli: input.reviewerCli ?? "claude",
task: reviewHarnessPrompt(pr) + `\nWrite the review to ${REVIEW_FILE}. Read .workforce/threads.json for the existing bot and reviewer comments.`,
})
// TODO: `.gate({ type: "artifact_exists", path: REVIEW_FILE })` once flows#434's follow-up lands.
.gate({ type: "subprocess_gate", command: `test -s ${REVIEW_FILE}` });

// ── verification, outside the agent. The exit code is the kernel's. ──
Expand Down
Loading