grill-stage: add /pharn-grill product command and spec→plan hash-chain gate#21
Conversation
…n gate Introduce the product grill stage with floor checker check-plan-spec-agree.mjs to re-verify the plan's carried spec_content_hash against the approved spec before advisory plan interrogation. 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 (3)
📝 WalkthroughWalkthroughIntroduces the Changespharn-grill stage: hash-chain floor gate + advisory interrogation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.dev/floor/check-plan-spec-agree.mjs:
- Around line 66-74: The carried-hash parsing in readCarriedHash currently
accepts the first spec_content_hash it sees, which can let duplicate frontmatter
keys slip through; update this path to detect repeated spec_content_hash entries
and reject the PLAN before comparing. Use the existing readCarriedHash helper
and the SPEC/frontmatter parsing logic around it to fail closed on duplicates,
rather than returning a value when more than one matching key is present.
🪄 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: 2409bc21-ac3b-4439-bb52-218338bba4de
📒 Files selected for processing (11)
.claude/commands/pharn-grill.md.dev/features/grill-stage/GRILL.md.dev/features/grill-stage/PLAN.md.dev/features/grill-stage/REGRESSION.md.dev/features/grill-stage/REVIEW.md.dev/features/grill-stage/SHIP.md.dev/features/grill-stage/VERIFY.md.dev/features/grill-stage/regression-report.json.dev/features/grill-stage/verify-report.json.dev/floor/check-plan-spec-agree.mjs.dev/floor/check-plan-spec-agree.test.mjs
| function readCarriedHash(text) { | ||
| const m = text.match(FM_RE); | ||
| if (!m) return undefined; | ||
| for (const line of m[1].split(/\r?\n/)) { | ||
| const kv = line.match(/^([A-Za-z_][\w-]*):[ \t]*(.*)$/); | ||
| if (kv && kv[1] === "spec_content_hash") return stripQuotes(kv[2].trim()); | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject duplicate spec_content_hash entries before comparing.
Line 71 returns the first matching key, while the reused SPEC parser’s object assignment gives later duplicate keys precedence. A PLAN with two carried hashes can therefore pass using the first value while a later frontmatter value is stale or malformed. Fail closed on duplicates.
Proposed fix
function readCarriedHash(text) {
const m = text.match(FM_RE);
- if (!m) return undefined;
+ if (!m) return { hash: undefined, duplicate: false };
+ let hash;
for (const line of m[1].split(/\r?\n/)) {
const kv = line.match(/^([A-Za-z_][\w-]*):[ \t]*(.*)$/);
- if (kv && kv[1] === "spec_content_hash") return stripQuotes(kv[2].trim());
+ if (kv && kv[1] === "spec_content_hash") {
+ if (hash !== undefined) return { hash, duplicate: true };
+ hash = stripQuotes(kv[2].trim());
+ }
}
- return undefined;
+ return { hash, duplicate: false };
}
...
- const planHash = readCarriedHash(planText);
+ const { hash: planHash, duplicate } = readCarriedHash(planText);
+ if (duplicate) {
+ return red(`PLAN.md carries multiple spec_content_hash entries (${planPath}) — re-plan via /pharn-plan`);
+ }
if (planHash === undefined) {Also applies to: 122-130
🤖 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/floor/check-plan-spec-agree.mjs around lines 66 - 74, The carried-hash
parsing in readCarriedHash currently accepts the first spec_content_hash it
sees, which can let duplicate frontmatter keys slip through; update this path to
detect repeated spec_content_hash entries and reject the PLAN before comparing.
Use the existing readCarriedHash helper and the SPEC/frontmatter parsing logic
around it to fail closed on duplicates, rather than returning a value when more
than one matching key is present.
…lent Address GATE-2 review finding by writing the grill-log on both chain outcomes — RED records the broken-chain verdict without interrogating; GREEN records findings as before. Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
/pharn-grill, the product-pipeline grill stage (spec → plan → grill → build → …) with two explicit natures: a floor spec→plan hash-chain re-verification and an advisory plan interrogation that never gates..dev/floor/check-plan-spec-agree.mjs(+ tests), a thin deterministic checker that shellscheck-spec-approved.mjsandcheck-spec.mjs --hash, then compares the PLAN frontmatterspec_content_hashto the current Approved SPEC body hash — the first downstream enforcement of/pharn-spec's pin after/pharn-plan..dev/features/grill-stage/(PLAN, GRILL, REVIEW, REGRESSION, VERIFY, SHIP artifacts).Test plan
node .dev/floor/check-plan-spec-agree.test.mjs— chain GREEN/RED cases passnpm test— full hook + floor suite greennode .dev/floor/validate.mjs .— still GREEN (floor capability count unchanged at 1)/pharn-grillagainst a feature with matching PLAN+SPEC hashes → GRILL.md emitted; against a stale/mismatched chain → deterministic RED haltMade with Cursor
Summary by CodeRabbit
New Features
Bug Fixes
Documentation