[bawoodruff]

Closes the two open deliverables on #29.

Can a plugin ship workflows? No — determined empirically

I added a workflows/ directory to plugin/, installed it with make update-plugin, confirmed the file present in the installed cache copy, then invoked it by name:

Workflow "plugin-workflow-probe" not found. Available: deep-research, shepherd-pr

It ships but never registers. deep-research is built in; shepherd-pr resolves from .claude/workflows/. claude plugin validate passed without acknowledging the directory either way.

Corroborating: across the 207 plugins in the marketplace catalog, the only component types that appear are agents, commands, hooks, lspServers, mcpServers, skills. No workflows component exists, and no installed plugin ships such a directory.

The probe was removed and the installed plugin restored — working tree and plugin cache are clean.

So: workflow-backed orchestration is repo-local by necessity. That is why the agent and skill variants are kept rather than collapsed into the workflow — they are the ones that travel. The probe is written down so it costs one command to re-confirm if a workflow component type ever lands.

A gap found while writing this up

.claude/ is gitignored, so .claude/workflows/shepherd-pr.js was tracked by nothing — zero tracked files under .claude/. The script #29 asks for existed only on this machine: unreviewable, unshareable, one rm from gone.

Since plugins can't distribute workflows, .claude/workflows/ is the only place the file can live and run — so it has to be version-controlled in place, not copied somewhere tracked. A second copy is exactly how the skill and agent variants have drifted twice already.

The gitignore change uses .claude/* rather than .claude/, because git does not descend into an excluded directory and the negation would otherwise be unreachable. Verified all three ways:

.claude/workflows/shepherd-pr.js   not ignored   ✓
.claude/settings.local.json        ignored       ✓
.claude/worktrees                  ignored       ✓

Docs

docs/SHEPHERDING.md — which of the three variants to use when, the distribution finding with its evidence, and the parity discipline the skill/agent pair needs (including the two CHANGELOG claims of parity that turned out false, and the complement-over-enumeration rule that prevents the next one). README points at it.

It also records what the PR #46 workflow run did not prove: it exited at round 1, so MIN_ROUNDS, the non-convergence check, and the hard cap are all still unexercised, and no stuck or blocked-on-owner outcome has been observed. A PR needing three or more rounds is what would demonstrate the budget machinery.

Scope

No plugin/ changes, so no version bump — the gate from #44 should confirm that.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WDPKxAK1dJvyn3gXaGfsFc

[cortexa-llc-reviewer]

Review: PR #48 — Deterministic shepherd-pr workflow + SHEPHERDING.md

Files reviewed: .claude/workflows/shepherd-pr.js, .gitignore, docs/SHEPHERDING.md, README.md

---

Major Issues (should fix)

.claude/workflows/shepherd-pr.js:45,49 — Personal machine path and hardcoded repo slug make the file broken-by-default for any other user

`…for PR #${pr} in /Users/bryanw/Projects/Vibe/ai-pack (repo Cortexa-LLC/ai-pack, default branch main)…`
// line 49:
`gh api repos/Cortexa-LLC/ai-pack/pulls/${pr}/reviews`

Two non-parameterized strings survive into this version-controlled file:

1. /Users/bryanw/Projects/Vibe/ai-pack — a personal machine path that does not exist on any other contributor's workstation (or in the isolated worktrees the workflow spawns). Every subagent invoked by the loop is told it is working in a path that doesn't exist on its host. Beyond the privacy concern of committing a personal home-directory path to a public repo, this gives wrong orientating context on every machine that isn't Bryan's.

2. Cortexa-LLC/ai-pack appears literally in the gh api call on line 49. The docs explicitly say consumers must copy this file into their own .claude/workflows/; a copied-and-un-edited file will issue all review-fetch requests to the wrong repository. The local-path hint and the gh api slug are the two most operation-critical strings in the prompt, and neither is parameterized.

Fix: Drop the local path entirely (the subagent works in whatever worktree the harness gives it — the path adds nothing). Promote the repo slug to a parameter:

const repo = args.repo  // e.g. "Cortexa-LLC/ai-pack"
if (!pr || !branch || !repo) throw new Error('args {pr, branch, repo} are required')

…and thread ${repo} through lines 45 and 49. Add a one-liner to docs/SHEPHERDING.md noting that callers must pass repo.

---

Minor Issues (optional)

.claude/workflows/shepherd-pr.js:90-95 — noProgress is not reset on a null (failed) round, producing a non-consecutive "consecutive" check

When a round agent fails and continue is hit (line 79), noProgress is neither reset nor incremented. On the next successful round, prev = history[history.length - 2] compares the new result against the last successful round before the null, not the immediately preceding round. If those two successful rounds independently reproduced the same findings (even though a real fix attempt occurred between them and just happened to crash), noProgress increments toward the stuck exit as if no progress was attempted.

The window is small — nullRounds >= 2 kills the workflow after back-to-back failures — but a single intervening null can cause a spurious stuck exit at round MIN_ROUNDS.

Fix: Add noProgress = 0 in the null-round branch alongside nullRounds = 0 (line 81), consistent with the intent that a null round represents an interrupted fix attempt, not a confirmed no-progress signal.

.claude/workflows/shepherd-pr.js:53 — False-positives from past PRs are presented as having occurred "in this PR's history"

reviews in this PR's history have already produced false positives
(a heredoc-indentation claim disproven by YAML block-scalar semantics;
 a suggested action-pin SHA for a nonexistent upstream tag)

Those two examples are from PR #46, not from whichever PR the workflow is driving. For any fresh PR with zero review history, the statement "this PR's history has already produced false positives" is empirically false — and primes the subagent toward disputing findings on the basis of pattern-matched historical FPs that are not in scope.

Fix: Rephrase to "past reviews in this repo have produced false positives; dispute findings only with empirical evidence (test, spec citation)" and drop the specific examples, or move them to a static list clearly attributed to "past PRs in this repo."

---

Positive Observations

• The .gitignore change is correct and well-reasoned: .claude/* + !.claude/workflows/ properly makes the negation reachable (a plain .claude/ exclude would not be). The comment explaining why is genuinely useful and will prevent future "clean it up" regressions.
• The structured-output schema is thorough: required covers the fields the loop control actually branches on, and blockedReason is wisely left optional.
• Documenting what the one tested run did not prove (MIN_ROUNDS, non-convergence, stuck, blocked-on-owner all unexercised) is an honest and valuable admission — it prevents the docs from overclaiming.
• The complement-over-enumeration rule at the end of SHEPHERDING.md is the right lesson from the Route G / review-gate drifts.

[bawoodruff]

Fixed in 2d861dd.

Major — correct, and worse than a portability bug: I committed a personal machine path to a public repo. /Users/bryanw/Projects/Vibe/ai-pack was in the round prompt, in the very file this PR makes version-controlled. That is on me and I should have caught it before opening the PR.

Both halves fixed:

• Path removed entirely. The round agent works in whatever checkout the harness gives it; the path added nothing but a wrong claim on every machine that isn't mine.
• Slug parameterized. You're right that it directly defeats the copy-this-file guidance in the doc this PR adds — a consumer's unedited copy would have fetched reviews from this repository.

I made repo optional rather than required, which differs from your suggestion. Two reasons: a required arg breaks the {pr, branch} call shape that meta.whenToUse advertises and that I used earlier today, and an explicit slug is itself a footgun if mistyped in a copy. Omitted, the round agent resolves it from its own checkout with gh repo view --json nameWithOwner — self-correcting wherever the file is copied. whenToUse now names the argument, and SHEPHERDING.md states the portability rule for copiers. Happy to make it required if you'd rather have the explicitness.

Minor (noProgress not reset on a null round) — fixed, and your analysis of the window is exactly right. The null branch continues without recording history, so the next successful round compared against the pre-crash round. Two rounds independently reporting the same findings, with a real fix attempt between them that happened to crash, could trip stuck as though nothing had been tried. A crashed round is an interrupted attempt, not evidence of no progress.

Minor (false-positive framing) — fixed. You're right that the two examples came from PR #46, not from whatever PR the loop is driving, and that asserting "this PR's history" on a fresh PR is both false and priming. Reworded to past reviews in this repo.

Also pruned the stale worktree the completed run left behind — git worktree prune was a no-op there, since it only clears entries whose directories are already gone.

[cortexa-llc-reviewer]

Review: PR #48 — Deterministic shepherd-pr workflow + SHEPHERDING.md

Files reviewed: .claude/workflows/shepherd-pr.js, .gitignore, docs/SHEPHERDING.md, README.md, plugin/agents/pr-shepherd.md

Previous review's blockers confirmed fixed: the personal filesystem path and hardcoded Cortexa-LLC/ai-pack slug are gone — repoLine is now parameterised or instructs the subagent to resolve it at runtime. The noProgress = 0 reset on null rounds is present with an accurate explanatory comment.

---

Major Issues (should fix)

.claude/workflows/shepherd-pr.js:107 — Order-sensitive title join defeats stuck detection

const sameTitles = prev.titles.join('||') === r.findingTitles.join('||')

findingTitles is generated by a language model whose output order is non-deterministic. Two rounds that raise exactly the same Critical/Major findings in a different order produce sameTitles = false, noProgress resets to 0, and the stuck exit never fires. For a genuinely stuck PR (e.g. a finding the agent cannot resolve), the loop runs all 12 rounds — each synchronously blocking on a full CI run — before falling through to cap-reached. The convergence guard that the whole design point of this workflow exists to provide simply doesn't work when title order varies.

Fix:

const sameTitles = [...prev.titles].sort().join('||') === [...r.findingTitles].sort().join('||')

Sort both sides before joining. If titles themselves contain || (unlikely but possible), use JSON.stringify([...arr].sort()) instead.

---

Minor Issues (optional)

docs/SHEPHERDING.md:71 — Exit outcome label mismatch with code

The exits table reads:

merge-ready · blocked-on-owner · stuck · cap (hard backstop)

The code at line 118 returns { outcome: 'cap-reached', … }. A consumer pattern-matching on the outcome string (e.g. the skill or a CI step) would miss the cap exit. Fix: change cap → cap-reached in the table.

.claude/workflows/shepherd-pr.js:31 — MIN_ROUNDS comment overstates its scope

const MIN_ROUNDS = 5  // owner policy: never call fewer rounds "enough effort"

MIN_ROUNDS only guards the stuck exit (line 112). A verdictClean=true result at round 1 exits immediately as merge-ready (line 101) — which is the right behaviour, but the comment implies the policy applies to all exits. That misleads anyone who reads it expecting MIN_ROUNDS to bound clean exits too. Fix: // minimum rounds before 'stuck' is declared — does not apply to clean or blocked-on-owner exits.

---

Positive Observations

• The prior review's hardcoded personal path and repo slug have been properly parameterised, with a comment explaining why consumers must never bake a slug in.
• The null-round noProgress reset is correct and the explanation (an interrupted fix attempt is not confirmed non-progress) is accurate and will prevent false stuck exits.
• The .gitignore pattern change from .claude/ to .claude/* + !.claude/workflows/ is the right form — the PR description correctly notes that git won't descend into an excluded directory, and the file in this diff confirms the pattern works.
• SHEPHERDING.md is unusually honest about what the single PR #46 run did not prove (MIN_ROUNDS, the convergence check, stuck, and blocked-on-owner are all unexercised). Documenting known gaps is better than asserting unverified coverage.

[bawoodruff]

Fixed in the push above.

Major — confirmed and fixed. Reproduced before changing anything:

same findings, different order → old sameTitles: false   fixed: true
genuinely different findings   → fixed sameTitles: false

Your diagnosis of the consequence is exactly right: noProgress resets, the stuck exit becomes unreachable, and a genuinely stuck PR grinds all 12 rounds — each blocking on a full CI run — before falling out at cap-reached. The convergence guard the workflow exists to provide, not working.

I took the JSON.stringify variant from your parenthetical rather than the sorted join('||'), because the collision runs both ways:

["a||b"].join("||") === ["a","b"].join("||")   → true

That is a false match, which would declare stuck on two rounds whose findings genuinely differ — the opposite error, and the more damaging one since it stops work early. Verified across six cases: identical/same-order and identical/reordered compare equal; one finding resolved, a new finding appearing, and the pipe collision all compare unequal; empty/empty equal. 6/6.

Minor (cap vs cap-reached) — fixed, and it was worse than the mismatch you cited: the docs also omitted error entirely, which is a real return from the two-consecutive-failures branch. All five outcome strings now agree between code and docs.

Minor (MIN_ROUNDS comment) — fixed. You're right that it only floors the stuck exit; a clean verdict or owner blocker returns at round 1. Both the comment and the docs now say so explicitly.

[cortexa-llc-reviewer]

Review: PR #48 — Deterministic shepherd-pr workflow + SHEPHERDING.md (round 3)

Files reviewed: .claude/workflows/shepherd-pr.js, .gitignore, docs/SHEPHERDING.md, README.md

Previous blockers confirmed fixed:

• Round 1: personal machine path and hardcoded Cortexa-LLC/ai-pack slug — gone; repoLine is now parameterised or instructs the agent to self-resolve at runtime.
• Round 2: order-sensitive title join — fixed; JSON.stringify([...a].sort()) is used on both sides (line 116).
• Round 2 minor: cap label mismatch — fixed; docs/SHEPHERDING.md:70-71 now reads cap-reached matching code.

---

Minor Issues (optional)

.claude/workflows/shepherd-pr.js:124 — detail string says "two consecutive rounds" but the guard requires three

detail: 'two consecutive rounds re-raised the same Critical/Major findings unchanged'

noProgress is incremented when a round matches its predecessor. It reaches 2 only after three consecutive matching rounds (round N-1→N increments once, round N→N+1 increments again). A consumer pattern-matching on the detail string, or a human reading the stuck return value, is told "two" when the invariant is "three or more". Fix: change to 'three or more consecutive rounds re-raised the same Critical/Major findings unchanged'.

.claude/workflows/shepherd-pr.js:60 — Reviewer bot name is hardcoded to this repo's bot

the reviewer posts as cortexa-llc-reviewer[bot] or github-actions[bot]

docs/SHEPHERDING.md (line 65-68) warns consumers who copy this file to remove absolute paths and hardcoded owner/name slugs, but does not mention the bot name. A project with a different reviewer bot (or the same workflow triggered under a different actor name) would silently skip every existing review and fall through to the gh run watch path for every round. Suggestion: add one sentence to docs/SHEPHERDING.md noting that the bot actor name in step 2 of the round prompt must also be updated after copying.

---

Positive Observations

• The .claude/* + !.claude/workflows/ gitignore pattern is correct; the PR description's three-case verification (workflow file not ignored, settings.local.json ignored, worktrees ignored) makes it easy to audit.
• noProgress reset on null rounds is well-reasoned and well-commented (lines 88-93); the design choice is justified in the comment itself.
• JSON.stringify([...a].sort()) for title comparison is the right fix from round 2 and handles the ||-in-title edge case the previous review identified.
• The "not yet proven" section in SHEPHERDING.md is unusually honest about what the single-round validation run does and does not cover. That's a strong signal the author has accurately modelled the gap.
• The HARD_CAP cap-reached return value reporting rounds: HARD_CAP is equivalent to reporting round (the loop runs round <= HARD_CAP), so there is no off-by-one there.