From 0ae7c8d8ac02bd2f21e352fa407aa380d33a9d10 Mon Sep 17 00:00:00 2001 From: Relayflow Lead Date: Thu, 17 Sep 2026 17:12:15 -0700 Subject: [PATCH] examples(pr-reviewer): keep the shell gate on the review file, and say why Review on #459 showed artifact_exists cannot gate .workforce/review.md: the worker's artifact scanner skips dot-directories, it records path presence on content change only (an empty file would pass, an identical rewrite would fail), while test -s demands a non-empty file and is idempotent. The TODO promising a swap is replaced by that rationale. Co-Authored-By: Claude Opus 5 (1M context) --- examples/pr-reviewer/README.md | 11 +++++++---- examples/pr-reviewer/pr-reviewer.flow.ts | 10 ++++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/examples/pr-reviewer/README.md b/examples/pr-reviewer/README.md index b0813d8bc..394f209a9 100644 --- a/examples/pr-reviewer/README.md +++ b/examples/pr-reviewer/README.md @@ -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. diff --git a/examples/pr-reviewer/pr-reviewer.flow.ts b/examples/pr-reviewer/pr-reviewer.flow.ts index ae460a045..92a030978 100644 --- a/examples/pr-reviewer/pr-reviewer.flow.ts +++ b/examples/pr-reviewer/pr-reviewer.flow.ts @@ -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 @@ -130,7 +133,6 @@ const reviewerBody = flow( 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. ──