ship-stage: add /pharn-ship product command and gated orchestrator#28
Merged
Conversation
Introduce the terminal product-pipeline stage as a meta-orchestrator over spec→verify with two human gates, floor-gated proceed reads, and zero new floor primitives. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR adds a new Changespharn-ship command and feature artifacts
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Record the completed build-loop gates (verify PASS, review GREEN, GATE 2) for the /pharn-ship increment. Co-authored-by: Cursor <cursoragent@cursor.com>
Address the GATE-2 review's advisory finding: /pharn-ship reads the regression-report.json / verify-report.json .verdict OUTPUTS, not the check-regress.mjs / check-verify.mjs scripts — so drop those two from reads:, leaving the three checkers it actually invokes (check-spec-approved, check-plan-spec-agree, validate) plus the two report JSONs it reads for .verdict. Floor GREEN; npm run check green (167 pass, prettier/eslint/markdownlint clean). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/commands/pharn-ship.md:
- Around line 194-226: Step 3 currently writes SHIP.md only in the
post-/pharn-verify green path, so STOPs at SPEC/PLAN/GRILL/BUILD/REGRESS never
get recorded. Update the /pharn-ship flow in pharn-ship.md so SHIP.md is written
on every terminal exit, either by emitting a minimal roll-up before each STOP or
by narrowing the documented contract to successful runs only. Keep the guidance
aligned with the existing stage sequence and the SHIP.md requirements around
stage order and end point.
In @.dev/features/ship-stage/PLAN.md:
- Around line 46-49: The resolved feature slug is being inferred again in later
stages instead of being carried forward from the initial `/pharn-spec`
resolution. Update the workflow around the entry step and downstream stage
handoff so the resolved `<name>` is captured once and passed verbatim through
every command that reads or writes under features/<name>/, using the existing
slug value rather than re-resolving it.
- Around line 75-83: The `/pharn-build` verdict handling in the ship-stage plan
only covers the exit-code path; add an explicit STOP path for spawn failures or
any other no-status result so `/pharn-ship` does not assume Step 4 completed.
Update the `/pharn-build` / `/pharn-verify` flow text to treat missing or
unreadable verdicts as a hard failure alongside non-zero exits, and make the
failure branch clearly tell the operator to stop and hand off to a human.
In @.dev/features/ship-stage/REGRESSION.md:
- Around line 39-43: The residual note is using outdated command names, which
makes the guidance inconsistent with the new `/pharn-*` namespace. Update the
wording in REGRESSION.md to refer to the current `/pharn-regress` and
`/pharn-verify` commands instead of `/pharn-dev-regress` and
`/pharn-dev-verify`, and keep the rest of the explanation aligned with those
command names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46f0d878-46db-4c2d-a5f0-47491edc0a7b
📒 Files selected for processing (5)
.claude/commands/pharn-ship.md.dev/features/ship-stage/GRILL.md.dev/features/ship-stage/PLAN.md.dev/features/ship-stage/REGRESSION.md.dev/features/ship-stage/regression-report.json
Comment on lines
+194
to
+226
| ## Step 3 — Set the writes-scope (fix #7, fail-closed), then write `features/<name>/SHIP.md` | ||
|
|
||
| `/pharn-ship` sets **no global scope** and never an over-broad one. Each sub-stage already runs its **own** | ||
| Step 0 writes-scope setter (overwriting `.pharn/writes-scope.json` per stage — the per-stage propagation). | ||
| `/pharn-ship`'s **only** Write-tool output is `SHIP.md`; scope it to itself **immediately before writing**, | ||
| after `/pharn-verify`: | ||
|
|
||
| ```bash | ||
| node .claude/hooks/set-writes-scope.cjs --from-frontmatter .claude/commands/pharn-ship.md --target features/<name>/SHIP.md | ||
| ``` | ||
|
|
||
| Deterministic floor step (P0/P5): scope is parsed from `writes:` and narrowed to `--target` — never chosen | ||
| by a model. (Invoking the stages is not a `Write|Edit|MultiEdit`, so the hook gates only this `SHIP.md` | ||
| write; each stage's own writes are gated by **its** own Step 0 scope.) If the write is blocked with the | ||
| `writes-scope guard` message, the fix is to **declare the path in `writes:` and re-run this setter** — never | ||
| bypass the hook (see CLAUDE.md, "Writes-scope"). | ||
|
|
||
| Write **`features/<name>/SHIP.md`** — a thin, **advisory** roll-up: | ||
|
|
||
| - **which stages ran**, in order, and **where the run ended** (GATE 2, or which stage's non-proceed verdict | ||
| STOPped it); | ||
| - **each structural verdict read, verbatim:** `/pharn-spec` → `check-spec-approved.mjs` exit (Approved); | ||
| `/pharn-grill` → `check-plan-spec-agree.mjs` exit (chain GREEN); `/pharn-build` → the project-gate exit; | ||
| `/pharn-regress` → `regression-report.json` `.verdict`; `/pharn-verify` → `verify-report.json` `.verdict`; | ||
| - a **pointer** to `features/<name>/GRILL.md` / `REGRESSION.md` / `VERIFY.md` (cite the files; do **not** | ||
| restate their findings — P4); | ||
| - the **standing decision is the human's.** `SHIP.md` records **that the chain ran and its floor verdicts** — | ||
| it is **never** a self-issued "shipped", an approval, or a `PHARN ✓ reviewed` seal (that would be the | ||
| disease, P0). End with the honest line: _"chain ran; the named floor verdicts are as shown, and the human | ||
| approved the intent at the SPEC gate — this is NOT a judgment that the increment is good or wise; that is | ||
| the human's call at the post-verify gate."_ | ||
|
|
||
| Then **end your turn** at the human gate. `/pharn-ship` does not merge, push, or seal. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Write SHIP.md on every terminal exit, not only after /pharn-verify.
As written, Step 3 only runs in the green path. Any STOP at SPEC/PLAN/GRILL/BUILD/REGRESS exits before this write, so the promised roll-up never records where the run ended. Either emit a minimal SHIP.md before each STOP or narrow the contract to successful runs only.
Suggested wording
-Then **end your turn** at the human gate. `/pharn-ship` does not merge, push, or seal.
+On any terminal STOP, write `features/<name>/SHIP.md` before ending the turn. `/pharn-ship` does not merge, push, or seal.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## Step 3 — Set the writes-scope (fix #7, fail-closed), then write `features/<name>/SHIP.md` | |
| `/pharn-ship` sets **no global scope** and never an over-broad one. Each sub-stage already runs its **own** | |
| Step 0 writes-scope setter (overwriting `.pharn/writes-scope.json` per stage — the per-stage propagation). | |
| `/pharn-ship`'s **only** Write-tool output is `SHIP.md`; scope it to itself **immediately before writing**, | |
| after `/pharn-verify`: | |
| ```bash | |
| node .claude/hooks/set-writes-scope.cjs --from-frontmatter .claude/commands/pharn-ship.md --target features/<name>/SHIP.md | |
| ``` | |
| Deterministic floor step (P0/P5): scope is parsed from `writes:` and narrowed to `--target` — never chosen | |
| by a model. (Invoking the stages is not a `Write|Edit|MultiEdit`, so the hook gates only this `SHIP.md` | |
| write; each stage's own writes are gated by **its** own Step 0 scope.) If the write is blocked with the | |
| `writes-scope guard` message, the fix is to **declare the path in `writes:` and re-run this setter** — never | |
| bypass the hook (see CLAUDE.md, "Writes-scope"). | |
| Write **`features/<name>/SHIP.md`** — a thin, **advisory** roll-up: | |
| - **which stages ran**, in order, and **where the run ended** (GATE 2, or which stage's non-proceed verdict | |
| STOPped it); | |
| - **each structural verdict read, verbatim:** `/pharn-spec` → `check-spec-approved.mjs` exit (Approved); | |
| `/pharn-grill` → `check-plan-spec-agree.mjs` exit (chain GREEN); `/pharn-build` → the project-gate exit; | |
| `/pharn-regress` → `regression-report.json` `.verdict`; `/pharn-verify` → `verify-report.json` `.verdict`; | |
| - a **pointer** to `features/<name>/GRILL.md` / `REGRESSION.md` / `VERIFY.md` (cite the files; do **not** | |
| restate their findings — P4); | |
| - the **standing decision is the human's.** `SHIP.md` records **that the chain ran and its floor verdicts** — | |
| it is **never** a self-issued "shipped", an approval, or a `PHARN ✓ reviewed` seal (that would be the | |
| disease, P0). End with the honest line: _"chain ran; the named floor verdicts are as shown, and the human | |
| approved the intent at the SPEC gate — this is NOT a judgment that the increment is good or wise; that is | |
| the human's call at the post-verify gate."_ | |
| Then **end your turn** at the human gate. `/pharn-ship` does not merge, push, or seal. | |
| Then **end your turn** at the human gate. On any terminal STOP, write `features/<name>/SHIP.md` before ending the turn. `/pharn-ship` does not merge, push, or seal. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/commands/pharn-ship.md around lines 194 - 226, Step 3 currently
writes SHIP.md only in the post-/pharn-verify green path, so STOPs at
SPEC/PLAN/GRILL/BUILD/REGRESS never get recorded. Update the /pharn-ship flow in
pharn-ship.md so SHIP.md is written on every terminal exit, either by emitting a
minimal roll-up before each STOP or by narrowing the documented contract to
successful runs only. Keep the guidance aligned with the existing stage sequence
and the SHIP.md requirements around stage order and end point.
Comment on lines
+46
to
+49
| **Step 1 — Entry.** `<increment description>` is the feature intent; `/pharn-ship` passes it to `/pharn-spec`. | ||
| `<name>` is the kebab-case slug `/pharn-spec` resolves; **reuse that one slug** across every stage. All | ||
| product artifacts live in root `features/<name>/` (never `.dev/` — `features/README.md`, the product boundary). | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Thread the resolved slug explicitly.
“Reuse that one slug” is not enough here; a later stage can still re-resolve and write to a different features/<name>/ tree. Make the resolved <name> a carried value and pass it verbatim into every downstream command.
♻️ Proposed fix
- `<name>` is the kebab-case slug `/pharn-spec` resolves; **reuse that one slug** across every stage.
+ Resolve `<name>` once in `/pharn-spec`, carry it forward, and pass that exact slug to every downstream stage.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **Step 1 — Entry.** `<increment description>` is the feature intent; `/pharn-ship` passes it to `/pharn-spec`. | |
| `<name>` is the kebab-case slug `/pharn-spec` resolves; **reuse that one slug** across every stage. All | |
| product artifacts live in root `features/<name>/` (never `.dev/` — `features/README.md`, the product boundary). | |
| **Step 1 — Entry.** `<increment description>` is the feature intent; `/pharn-ship` passes it to `/pharn-spec`. | |
| Resolve `<name>` once in `/pharn-spec`, carry it forward, and pass that exact slug to every downstream stage. All | |
| product artifacts live in root `features/<name>/` (never `.dev/` — `features/README.md`, the product boundary). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/ship-stage/PLAN.md around lines 46 - 49, The resolved feature
slug is being inferred again in later stages instead of being carried forward
from the initial `/pharn-spec` resolution. Update the workflow around the entry
step and downstream stage handoff so the resolved `<name>` is captured once and
passed verbatim through every command that reads or writes under
features/<name>/, using the existing slug value rather than re-resolving it.
Comment on lines
+75
to
+83
| 4. **`/pharn-build`** → writes the user's code + a thin `features/<name>/BUILD.md`. `/pharn-build` re-checks | ||
| the chain (2nd consumer) and the fix #7 writes-scope itself, and **HALTs on a RED floor** at its Step 4. | ||
| **Verdict read (FLOOR):** the exit code of the **same deterministic project gate `/pharn-build` ran at its | ||
| Step 4** — for a PHARN-shaped capability build (this repo's dogfood) that is `node .dev/floor/validate.mjs .` | ||
| (identical to `/pharn-dev-ship`); for a general user project it is the gate resolved **exactly as | ||
| `/pharn-build` Step 4 / `/pharn-verify` Step 3a resolve it** (`--gates` or the closed allowlist ∩ | ||
| `package.json` scripts — reused, NOT hard-coded `validate.mjs`, P3). `0` → proceed; non-zero → **STOP**, | ||
| present the RED floor, hand to the human. (This floor is **re-confirmed** structurally two stages later | ||
| by `/pharn-verify`'s absolute all-green-at-HEAD `.verdict` — belt-and-suspenders.) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail closed when build never yields a readable verdict.
This only covers the exit code path. Add an explicit STOP branch for spawn errors or any other no-status case so /pharn-ship never has to guess whether /pharn-build actually reached Step 4.
♻️ Proposed fix
+ If `/pharn-build` does not return a readable Step-4 verdict/status, STOP and surface the refusal to the human.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| 4. **`/pharn-build`** → writes the user's code + a thin `features/<name>/BUILD.md`. `/pharn-build` re-checks | |
| the chain (2nd consumer) and the fix #7 writes-scope itself, and **HALTs on a RED floor** at its Step 4. | |
| **Verdict read (FLOOR):** the exit code of the **same deterministic project gate `/pharn-build` ran at its | |
| Step 4** — for a PHARN-shaped capability build (this repo's dogfood) that is `node .dev/floor/validate.mjs .` | |
| (identical to `/pharn-dev-ship`); for a general user project it is the gate resolved **exactly as | |
| `/pharn-build` Step 4 / `/pharn-verify` Step 3a resolve it** (`--gates` or the closed allowlist ∩ | |
| `package.json` scripts — reused, NOT hard-coded `validate.mjs`, P3). `0` → proceed; non-zero → **STOP**, | |
| present the RED floor, hand to the human. (This floor is **re-confirmed** structurally two stages later | |
| by `/pharn-verify`'s absolute all-green-at-HEAD `.verdict` — belt-and-suspenders.) | |
| 4. **`/pharn-build`** → writes the user's code + a thin `features/<name>/BUILD.md`. `/pharn-build` re-checks | |
| the chain (2nd consumer) and the fix `#7` writes-scope itself, and **HALTs on a RED floor** at its Step 4. | |
| **Verdict read (FLOOR):** the exit code of the **same deterministic project gate `/pharn-build` ran at its | |
| Step 4** — for a PHARN-shaped capability build (this repo's dogfood) that is `node .dev/floor/validate.mjs .` | |
| (identical to `/pharn-dev-ship`); for a general user project it is the gate resolved **exactly as | |
| `/pharn-build` Step 4 / `/pharn-verify` Step 3a resolve it** (`--gates` or the closed allowlist ∩ | |
| `package.json` scripts — reused, NOT hard-coded `validate.mjs`, P3). `0` → proceed; non-zero → **STOP**, | |
| present the RED floor, hand to the human. (This floor is **re-confirmed** structurally two stages later | |
| by `/pharn-verify`'s absolute all-green-at-HEAD `.verdict` — belt-and-suspenders.) | |
| If `/pharn-build` does not return a readable Step-4 verdict/status, STOP and surface the refusal to the human. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/ship-stage/PLAN.md around lines 75 - 83, The `/pharn-build`
verdict handling in the ship-stage plan only covers the exit-code path; add an
explicit STOP path for spawn failures or any other no-status result so
`/pharn-ship` does not assume Step 4 completed. Update the `/pharn-build` /
`/pharn-verify` flow text to treat missing or unreadable verdicts as a hard
failure alongside non-zero exits, and make the failure branch clearly tell the
operator to stop and hand off to a human.
Comment on lines
+39
to
+43
| **Honest residual (P0/P7):** `/pharn-dev-regress` catches exactly what its deterministic suite catches — | ||
| nothing more. "No regressions" means **no deterministically-detectable breakage outside the feature flipped | ||
| pass→fail**, _not_ "nothing broke" and _not_ a judgment that the `/pharn-ship` command is correct or | ||
| well-designed (that is `/pharn-dev-verify` + human review). The orchestration (base resolution, inside/outside | ||
| partition, the scaffolding exclusion) is advisory; only the exit-code **comparison** is the guarantee. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the current /pharn-* command names.
/pharn-dev-regress and /pharn-dev-verify look stale relative to the /pharn-regress and /pharn-verify commands described in this PR. As written, the residual note points readers at commands that don't match the new namespace.
🔧 Suggested edit
- `/pharn-dev-regress` catches exactly what its deterministic suite catches —
+ `/pharn-regress` catches exactly what its deterministic suite catches —
nothing more. "No regressions" means **no deterministically-detectable breakage outside the feature flipped
pass→fail**, _not_ "nothing broke" and _not_ a judgment that the `/pharn-ship` command is correct or
- well-designed (that is `/pharn-dev-verify` + human review).
+ well-designed (that is `/pharn-verify` + human review).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| **Honest residual (P0/P7):** `/pharn-dev-regress` catches exactly what its deterministic suite catches — | |
| nothing more. "No regressions" means **no deterministically-detectable breakage outside the feature flipped | |
| pass→fail**, _not_ "nothing broke" and _not_ a judgment that the `/pharn-ship` command is correct or | |
| well-designed (that is `/pharn-dev-verify` + human review). The orchestration (base resolution, inside/outside | |
| partition, the scaffolding exclusion) is advisory; only the exit-code **comparison** is the guarantee. | |
| **Honest residual (P0/P7):** `/pharn-regress` catches exactly what its deterministic suite catches — | |
| nothing more. "No regressions" means **no deterministically-detectable breakage outside the feature flipped | |
| pass→fail**, _not_ "nothing broke" and _not_ a judgment that the `/pharn-ship` command is correct or | |
| well-designed (that is `/pharn-verify` + human review). The orchestration (base resolution, inside/outside | |
| partition, the scaffolding exclusion) is advisory; only the exit-code **comparison** is the guarantee. |
🧰 Tools
🪛 LanguageTool
[grammar] ~40-~40: Use a hyphen to join words.
Context: ...-detectable breakage outside the feature flipped pass→fail**, not "nothing brok...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/ship-stage/REGRESSION.md around lines 39 - 43, The residual
note is using outdated command names, which makes the guidance inconsistent with
the new `/pharn-*` namespace. Update the wording in REGRESSION.md to refer to
the current `/pharn-regress` and `/pharn-verify` commands instead of
`/pharn-dev-regress` and `/pharn-dev-verify`, and keep the rest of the
explanation aligned with those command names.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/pharn-ship, the terminal product pipeline stage: a gated meta-orchestrator that runsspec → plan → grill → build → regress → verifyin order, branching only on each stage's structural floor verdict.features/<name>/SHIP.mdroll-up — never auto-ships or seals.check-spec-approved,check-plan-spec-agree,check-regress,check-verify, build project-gate) with zero new floor primitives;--loopdeferred to a follow-up increment.Test plan
npm run check— format, lint, markdownlint, 167 tests greennode .dev/floor/validate.mjs .— GREEN (1 capability; command is floor-ignored)/pharn-dev-regressaudit trail —regression-report.jsonverdictno-regressions/pharn-dev-reviewon this increment (advisory lenses)/pharn-shipon a small feature intent end-to-endMade with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Documentation