fix: guard self-host completeness commits - #197
Open
norvalbv wants to merge 139 commits into
Open
Conversation
…soften, no config key) (#72) * feat(review): add completenessHard guard.config.json option The completeness gate (commit-msg) could only be escalated from WARN-only to a hard block via a one-off GUARD_COMPLETENESS_HARD env var, forcing a consumer wanting it always-on to hardcode the env var inside their own husky hook. Add a standing completenessHard config-file key, mirroring the existing noLlm/noLog config+env precedence: env still overrides for a one-off, but a repo can now opt in via guard.config.json like every other gate knob. * feat(review)!: completeness gate blocks by default; completenessHard:false softens Warn-by-default proved a no-op channel for headless agents twice over: the sentry gate's 18-unseen-verdicts stat, and a completeness true-positive (unfixed sibling sites) that shipped past an advisory WARN. Flip the default to hard; the config key becomes the per-repo opt-OUT and env stays the one-off override in both directions. * docs(decisions): record completeness hard-by-default target on review-gate-in-chain * refactor(review)!: drop the completenessHard config key — hard-by-default is code, not config One reviewer with a standing config soften the other six gates lack was an inconsistent surface with zero intended users, and a config-file soften is agent-stageable. The only soften is the env one-off GUARD_COMPLETENESS_HARD=0, matching the vision/sentry gate shape. envBool exported for gates that must distinguish unset from an explicit =0. --------- Co-authored-by: norvalbv-slice <benji.norval@slice.com>
…76) * chore(eval): edge-cases judge ground-truth harvest tooling (sc-1118) Harvest -> label -> finalize pipeline that builds the /edge-cases eval corpus from three local stores (Claude Code transcripts, frink agents.db, Cursor state.vscdb), with a 5-rung diff-reconstruction ladder, independence-ranked evidence tiers (f2p-in-session > independent-fix > user-confirmed > test-added-green > rejected > none), split verdict/wasLiveBug label axes, and a permanent vitest gate (schema + scrub + review invariants; skips when the corpus file is absent). cases.jsonl itself is NOT committed here: devkit is public and rows carry private-repo code excerpts. The canonical data lands in the private frink repo (scripts/edge-cases-eval/); the gate validates whichever local copy exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(edge-cases): harden pipeline per review — provenance whitelist, independence filter, gate fixes - label: whitelist judge-reply fields (summary/degenerate/reason/findings) so a reply can never overwrite harvested provenance like repo/diffFull (excerpt-allowlist forgery vector); exclude the session's own commit shas from laterCommits (path overlap alone let same-session compliance pose as an independent later fix); scrub the judge input as defense in depth - finalize: strict degenerate flag (only true/"true"; malformed values error), malformed findings surface as row errors instead of crashing the pass, --append validates copied existing rows, excerpt cap measured in UTF-8 bytes - gate: honor EDGE_CASES_CORPUS env (CI can point at the canonical private copy), leak-scan DECODED values so JSON escapes can't hide a path/token - schema: diff anchors require a non-empty nameStatus; sources: guard decodeURIComponent against stray % in paths Corpus unaffected: re-finalize produces a byte-identical cases.jsonl; no corpus row used the independent-fix tier, so the laterCommits fix poisons no existing label. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
… reasons kept, fetchable transcripts (#78) The decision-log gate exited silently on pass, so it looked dead under the "🧭 Decision-log gate…" header and left no telemetry event; passing reviewers' verdict reasons and full transcripts were discarded everywhere. - decisions: announce the pass on stderr + emit a status:'pass' gate_result (parity with the reviewer gate) on every clean path - reviewers: keep the judge's one-line VERDICT reason on PASS (was dropped) — it flows to the review_result event and the completion line - new transcript-store: persist every agent's full transcript (pass + fail) to <telemetry>/transcripts/<ship_id>/<agent>.txt, referenced by transcript_ref in the event; read it back with `guard-review transcript <ref>` Ship-scoped (no-op off-ship, same env contract as gate-events). tsc + full suite (1333) green.
#80) * feat(init): self-host mode — devkit dogfoods its own gates from source The hand-authored .husky/pre-commit was a second source of truth for the gate chain and drifted from the generator: the review gate never ran on a devkit commit, and no test kept the hook in sync. Add a self-host install mode (auto-detected by package name, beside standalone/overlay). It adds no self-dependency and generates the hook from the same generator as consumers, rewriting each `bunx guard-*` to `node gate-engine/*.mts` so gates run from the working tree. `devkit upgrade` regenerates it and a parity test locks it to the generator. - husky-block: optional `extras` preserves the hard `bun run lint` gate (empty for consumers, output byte-identical) - self-host.mts: bin->source rewrite, fixed selection (recommended + review), advisory fallow-audit tail preserved from the hand hook - init/upgrade/doctor: selfHost branches (no self-dep, source hook, hand-owned configs untouched, hook-in-sync doctor check) - .husky/pre-commit regenerated: review + full recommended set now gate * fix(init): self-host line-growth baseline + fallow gate inside the block Two gaps in the first cut, both surfaced by dogfooding the gates: - Enable the recommended line-growth ratchet for self-host: write maxLines and freeze size-lines.json, grandfathering devkit's 13 files over the 500-line cap (shrink-only). Without it every commit touching those giants would block. - Move the advisory fallow audit INSIDE the devkit-guards block. As a trailing line it got absorbed into the preamble by replaceGuardBlock's findPreambleEnd on a re-run/upgrade, splitting the comment from its command. In-block it is preserved verbatim and now covered by the parity + doctor checks (+ an idempotency test locking the re-apply).
…ranscripts (#79) Captures reviewer/decisions telemetry + transcripts on EVERY commit (not just ship), so the local usage-tracker builds a history of agent runs. The reviewers already run on every commit — this only persists output they already produce. - new gate-engine/judge/run-context.mts: capture is ON by default; DEVKIT_NO_TELEMETRY=1 opts out (disables only the automatic every-commit capture — an explicit ship still emits). Off a ship the sink defaults to ~/.devkit/telemetry and the run is correlated by the staged tree hash (`git write-tree`) — identical across a commit's decisions+ review gate processes, unique per staged content. Fail-safe: git error → null → silent. - gate-events / transcript-store route through run-context. A commit run stamps run_mode/repo/branch on its events so a downstream collector can synthesise a run row. - transcripts store the REVIEWED DIFF alongside the agent output (composeTranscript). - retention: keep the most-recent N run dirs (DEVKIT_TELEMETRY_KEEP, default 300). - vitest.setup sets DEVKIT_NO_TELEMETRY=1 so off-ship gates in the suite don't auto-capture. tsc + full suite green.
… the 420s cap) (#82) The correctness reviewer's deep four-lens investigation legitimately runs past the old 420s first-pass cap and got SIGKILLed mid-verdict — the usage-tracker showed repeated 421s inconclusive timeouts while its median run is ~60-250s. It was the only reviewer hitting the cap; the rest finish under 300s. - run-review.mts: FIRST/STRICT_FIRST/ESCALATE timeouts 300s/420s → 30 min. A judge that times out is still never re-run, so a stuck judge costs at most one cap. - commit-with-gate-capture.sh: SHIP_COMMIT_TIMEOUT default 1800s → 3600s so a 30-min reviewer plus the fast ones fit the ship budget (else the outer ship timeout kills it first). Still a hang ceiling that converges on re-run; export to raise further. Note: a single pass can now exceed the 600s foreground tool cap, so an agent-driven commit (gate run inside a Bash tool) is still capped at 600s; the generous caps take full effect for a commit run in a real terminal, where SHIP_COMMIT_TIMEOUT is the outer bound.
…ariants (methodology audit) (#81) * fix(edge-cases)!: pipeline v2 — solution-leakage fixes + coverage invariants from the methodology audit Implements the required fixes from the benchmark methodology audit (verdict: merge-after-fixes): - F1 no solution leakage: every diff-reconstruction bound is the invocation timestamp itself (branch tip at invocation, date-window until invocation, in-session pick prefers pre-invocation then EARLIEST in-turn with a relevance filter over the session's own edited files); the untimeboundable PR-squash rung is flagged postFixContaminated and demoted - F2 anchorCoverage: finalize computes per-row finding/anchor coverage, demotes zero-coverage rows to session-summary; validator + gate enforce coverage > 0 on diff rows - F3 symmetric skepticism: test-added-green now labels wasLiveBug "unknown" (a green first run cannot distinguish always-correct from fix-before-test); "false" needs pre-session evidence - F4 rejected split: rejected-as-wrong -> noise vs investigated-resolved-safe -> worth-surfacing (verdict no longer depends on the verification modality) - F5 truncation parity: labeler judges the exact committed excerpt (shared lib/excerpt.mts); per-section evidence-bundle budgets stop laterCommits from being serialized away - F6/F7: path-shaped files validation on diff rows, degenerate-with-findings is an error (not a silent drop), immutable proposals + audit-overlay (raw/audit-overlay.jsonl), labelModel + labelPromptSha provenance per row, and kappa.mts — a blind second-labeler agreement tool using the pre-registered match rule - README: pre-registered match rule incl. tie-breaks, case-level clustered analysis contract, judge-bias controls (blinding, style normalization, order mirroring), honest v2 stats BREAKING for corpus consumers: rows gain anchor.coverage/excerptCoverage/postFixContaminated and labelModel/labelPromptSha; the diff-scoped spine shrinks to rows that survive the leakage and coverage invariants (19 finding-bearing + 12 degenerate guards) — 19 clean rows beat 77 corrupted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(edge-cases): review round — contract-exact κ matcher, shared path-overlap helper, tie-break + path-shape fixes - kappa: matcher now follows the pre-registered rule EXACTLY (Jaccard fallback only when a side has no files; severity third tie-break) — the loose OR-branch inflated matched pairs 25 vs 5; single-class κ reports "undefined", never 0 (which would read as chance-level agreement at 100% raw agreement) - lib/match.mts: one suffix-tolerant path-overlap implementation shared by finalize (coverage), kappa (matching) and sources (relevance scoring) — three copies would drift - finalize: excerptCoverage now uses diff-header file lists (a raw substring scan let "index.ts" match inside "reindex.ts") - sources: pickDiff score ties prefer PRE-invocation candidates (leakage-free) over the in-turn one - schema: path-shape check admits conventional extensionless root files (Dockerfile, LICENSE, …) - README: honest κ numbers under the strict rule (5/82 strict-matched, 100% agreement on matched) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(edge-cases): samePath requires a path-separator boundary on suffix matches Raw endsWith accepted basename substrings ("src/reindex.ts" vs "index.ts"), overstating anchorCoverage/excerptCoverage and mispairing kappa findings. Suffixes now match only at a / boundary (or exact equality). Corpus verified byte-identical after re-finalize; kappa report unchanged (5 strict pairs, 100% agreement). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…w round (#83) - recall denominator = ANCHORED findings only (files ∩ anchor; computable via lib/match.mts) — partial-coverage rows keep their unanchored findings in the corpus but out of recall - non-degenerate zero-finding rows score as true-negative material (like degenerates) - date-window wording: committer-date bound (git --until semantics; rebases refresh committer dates so re-committed post-invocation work stays excluded) - κ pilot renamed to what it is — a matcher/segmentation pilot; the blind uniform 40-60 finding label-noise audit is a REQUIRED sc-1119 pre-step, not yet run Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…anup (#84) * fix(husky): force-add re-staged files in biome-format hook + lint cleanup git add without -f chokes when re-staging force-added gitignored dist/ files, breaking devkit's own release commit (discovered while cutting v0.35.0). Also cleans up pre-existing biome lint debt (assign-in-expression, non-top-level regex, implicit-any) in gate-engine/judge, gate-engine/review, and gate-engine/edge-cases/eval that this repo's whole-tree lint gate surfaced, and backfills the run-review.mts line-count ratchet to its actual size. * test(husky): regression test for force-add re-stage fix Real-git-repo test proving the biome-format hook's re-stage step survives a force-added gitignored dist/ file: fails against pre-fix (git add, no -f) with the exact 'ignored by gitignore' abort, passes with the fix.
Dist-free version bump so main tracks the v0.35.0 tag already pushed (branch release/v0.35.0, tag v0.35.0).
… are diagnosable (#86) * fix(ship): persist a per-ship gate log + emit log_path so blocked ships are diagnosable The usage-tracker collector reads ship.log_path (a per-ship log at ~/.devkit/telemetry/logs/<ship_id>.log) to render the full gate log in the drill-down and to classify why a ship blocked. The ship script never emitted it, so on the dashboard every blocked ship shows "unknown / no persisted log" — no way to tell why. - tee the captured gate output to a durable per-ship log beside the telemetry sink, in addition to the per-branch $log in the repo (which the next ship overwrites, so it can't back a historical drill-down). - emit log_path in ship_attempt (an in-flight row serves it once the file exists) and ship_result, so the collector's classify_fail_log reads it and "unknown" resolves to the real gate (deterministic / review / structure-lint / size-ratchet / completeness). Verified: smoke-run emits log_path on both events and writes the per-ship log; ship + reship + husky suites green. * review: sanitise ship_id for the log filename + JSON-escape telemetry fields Address CodeRabbit on #86: - DEVKIT_SHIP_ID is env-supplied; sanitise it (drop path-unsafe chars, cap length) before using it as the ship_log filename so `../..` can't make tee write outside the logs dir. The JSON keeps the ORIGINAL id for correlation. - add a minimal json_escape for the shell-built ship_attempt/ship_result lines so a repo/branch/path containing `"` or `\` can't emit a line the collector's json.loads drops. - split the `local x=$(...)` declarations (SC2155).
…ng (#88) `devkit upgrade` in an overlay repo printed "re-run devkit init --overlay to re-sync" and exited 1, doing nothing. Both reasons for that bail are stale: 1. Re-running the overlay used to re-freeze the size/fanout baselines every time (debt laundering), so upgrade refused. That hazard was removed when baselining became init-only (the repoAdopted guard) — re-sync is idempotent. 2. An overlay is not pin-less: its version pin is .devkit/config.json's `devkitRef` (there's no package.json dep because the gates run off the global CLI). Version logic should be install-agnostic. Handle overlay in a self-contained branch mirroring the self-host branch: resolve the config.json pin, chase a newer published tag exactly like package mode (updating the global CLI — the overlay analog of `bun install`), re-sync the git-ignored gate chain + configs + skills/agents, then verify. Thread the recorded `globalCommitGate` through so a re-sync never silently un-wires an opted-in machine-global commit-gate shim. Also: record the decision (overlay-self-heal), and point the `devkit update` overlay hint at `devkit upgrade`.
* fix(ship): hint git ls-files on directory-path rejection DK-4: `devkit ship ... -- <dir>` rejects directory paths (deliberate — a directory pathspec would recurse and sweep in a parallel agent's edits under that directory in a shared working tree). The rejection error now hints the exact `git ls-files -- "<dir>"` command to list the directory's tracked files, so callers don't have to guess and avoid the zsh word-splitting foot-gun from hand-rolling a file list. Deliberately not a `git ls-files | xargs devkit ship` one-liner: piping straight into `ship` would re-automate the sweep the guard exists to prevent, and xargs's default batching on a long list would silently split into multiple ship invocations, the second hitting the branch-exists preflight and failing. * test(ship): pin the git-ls-files hint in directory-rejection tests Add assertions on the new hint line to both existing directory-rejection tests (ship-branch.test.mts, reship.test.mts). These fail against unpatched main (no hint line printed) and pass on this branch — pinning the fix from the previous commit instead of relying only on the non-anchored prefix match, which passes either way.
## DK-1 — `ship --base <branch>`
ship pinned BOTH of its bases to the invoking checkout's HEAD: the ephemeral worktree was cut from
`git rev-parse HEAD`, and the PR target came from `git symbolic-ref HEAD`. In a repo whose
source-of-truth branch is not its PR base, that forces you to check out (or worktree-juggle into) the
base branch. Worse, once the work is committed on the source branch, `<paths>` already match HEAD, so
ship stages nothing and force-deletes its own branch.
The ticket proposed reworking staging to read `<paths>` from the working tree. That turned out to be
unnecessary: `git diff <commit> -- <paths>` is **already** commit-vs-working-tree, and was only
HEAD-relative because BASE was pinned to HEAD. Only the pin moved — staging is untouched.
devkit ship my-branch "title" --base feat/studio -- <paths>
- Worktree is cut from `<base>`; the PR targets `<base>`. Default (no flag) is unchanged.
- **Branch-only, fail fast.** A GitHub PR base cannot be a sha or a tag, so `--base` rejects them.
Enforced by fetching the fully-qualified `refs/heads/<b>` — a bare ref would also match a tag — so
one round-trip proves the branch exists AND is a branch, before anything is pushed.
- **Base is origin's tip, not the local ref** (mirrors `reship.sh`). In a shared parallel-agent
checkout the local base branch is routinely stale, and a worktree cut from a stale base makes the
gates judge code GitHub will never merge into. The PR *diff* would be right either way (GitHub
diffs from the merge-base), so this buys **gate** accuracy, not diff accuracy.
- `BASE_REF` (a name, no network) resolves ABOVE the `SHIP_RESOLVE_ONLY` seam; `BASE` (a commit,
needs the fetch) BELOW it — keeping that test seam side-effect-free.
- `x` and `origin/x` resolve to one base. `--base` also makes a detached HEAD shippable, since there
is no longer a current branch to need.
## DK-2 — empty-commit preflight
Shipping paths already identical to the base staged nothing, and nothing noticed until git reported
"nothing to commit" AFTER the whole gate chain had run — whereupon the EXIT trap force-deleted the
branch, leaving only a bare `Deleted branch … (was …)`. The preflight mirrors the two staging
commands exactly (same BASE, same `--exclude-standard`, same pathspec) and fails ahead of the
worktree, so nothing is created to churn. `reship.sh` already had this guard; this is its new-ship
twin.
The two features are interdependent — the preflight's remedy hint offers `--base`, and the `--base`
hint suppresses the "check out the branch your work is on" advice that `--base` exists to obsolete —
so they land together.
## Also in this PR
`devkit sync-skills` regenerated `.devkit/skills-manifest.json` (stale since v0.29.0) for the new
`using-devkit` row documenting `--base`. That also heals pre-existing drift in the synced
`dup-detection` copies — the source was already correct; only the `.claude`/`.cursor` copies lagged.
The manifest is atomic, so this rides along rather than leaving it disagreeing with the copies.
## Verification
- Lint green and 71 pass (`ship-branch` / `reship` / `help-cli`) on this exact tree; full suite 1420
pass, tsc clean.
- Live end-to-end on the reported repro: from a branch with the work already committed,
`--base <other>` opens a commit parented on the base tip whose diff is the **working-tree** content
of `<paths>` vs that base, with the shared HEAD unmoved.
- **This PR shipped itself**: authored in a detached worktree at `origin/main` and shipped with
`devkit ship … --base main`, which is the exact situation DK-1 exists to fix (a stale local `main`,
a base that only exists upstream, and no wish to move the shared checkout's HEAD).
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eezes survive (DK-3) (#91) ## Context DK-3 reported `guard-fanout` failing in the ship worktree on pre-existing grandfathered folders, blaming ship's auto-link set for omitting `eslint/`. **The ticket's premise and evidence do not hold**, but a real bug sits underneath it: - `eslint/` is not gitignored — in devkit or frink. `eslint/baselines/fanout.json` is tracked, in `HEAD`, and clean in both, so it rides the checkout into the ship worktree. - The cited evidence is unreproducible: `src/crust/components/icon/svgs` exists in no repo, frink's freeze grandfathers 28 folders (none under `src/crust`), and run `bf4dn1atz` appears nowhere. - Frink is package mode (`standalone: false, selfHost: false`, `core.hooksPath=.husky/_`), so it cannot hit this path at all — DK-3's acceptance criterion already passed there. ## The real bug — overlay mode only `installOverlay` hides the baselines via `.git/info/exclude` (`overlay.mts:502-507`), not `.gitignore`, and `runFreezes` freezes into that excluded dir anyway. The baselines are therefore untracked → never committed → **absent from the ship worktree**, and nothing linked them back. Ship's auto-link is what makes this fatal rather than harmless. The fanout gate's fail-open needs **both** the baseline and `guard.config.json` absent (`folder-fanout.mts:123`) — but `guard.config.json` **is** auto-linked. So the gate enforces against an empty freeze and every grandfathered folder reads as new growth. Reproduced against the real gate: freeze present → pass; freeze absent + `guard.config.json` present → `🚫 Folder fan-out exceeded ... src/icons: 15 files (allowed 12)` on a pre-existing folder; freeze linked via a symlinked dir → pass. ## Why the ship linker is the right layer `ship-gates-converge-not-restart.md:24` already records this link as a **dependency**: the prefix-cache key folds in "whole eslint/baselines contents" and *"Depends on the ship linker symlinking those inputs into the worktree so the fingerprint reads real state."* Expect the cache key to flip once — anticipated by that note. Both alternative layers are barred: re-keying the fail-open is a recorded **REJECT** (`overlay-self-heal.md:46` — it *"would silently fail-open exactly where enforcement matters"*), and un-excluding baselines in `overlay.mts` breaks overlay's "edits nothing committed" ethos. ## The change One entry in the candidates array. Dir-granular to match overlay's exclude exactly, covering `fanout.json`, `size.json`, and `size-lines.json` in one go (overlay forces `structure: false`, so the `.mjs` baselines are never generated there). It is a **no-op for every tracked consumer** — the existing `[ ! -e "$wt/$rel" ]` guard skips it whenever the dir rides the checkout, so devkit and frink are unaffected. Also documents the load-bearing `git add` failure in `stageBaseline`: `git add` beyond a symlink hard-fails (exit 128) and is swallowed. That silence is what keeps overlay git-invisible — realpath-resolving it would commit baselines into a repo that never adopted them. ## Tests - Regression: untracked freeze → linked in; gate sees the grandfathering. **Verified load-bearing** — reverting the fix fails it on `FREEZE_SEEN`. - No-op when tracked → not linked, no notice. - Pins the candidates array: nothing previously asserted its contents, so dropping an entry silently weakened every ship. 57/57 in `ship-branch.test.mts`, ratchets + reship 71/71, `tsc --noEmit` clean. ## Priority **Latent, not an outage** — every `.devkit/config.json` on this machine is package/standalone, so no overlay consumer currently hits it. ## Follow-up (not in this PR) `eslint.config.devkit.mjs` and `biome.devkit.jsonc` come from the same overlay exclude set and are guarded by `[ -f ... ]` in `husky-block.mts:273-280` — when absent the lint/format step is **silently skipped** while ship still reports "gates ran". Worse than this loud bug; worth its own ticket.
Version bump only — dist-free, per the release contract (main rejects direct push; the dist-carrying tag is pushed separately off-main). Ships everything merged since v0.35.1: - #88 — `devkit upgrade` re-syncs overlay repos instead of bailing "re-run init --overlay" - #89 — ship hints `git ls-files` on directory-path rejection - #90 — ship `--base <branch>` + empty-commit preflight (DK-1, DK-2) - #91 — ship links `eslint/baselines` into the gate worktree so ratchet freezes survive (DK-3) Patch bump per maintainer call.
…94) The review_result telemetry event and the verdict-cache entry both hardcoded firstModel (the cascade default, 'haiku'), ignoring Reviewer.model pins. A sonnet-pinned correctness verdict was therefore labeled 'haiku' in ~/.devkit/telemetry/gate-events.jsonl and in .devkit/review-cache.json — the usage dashboard faithfully displayed the lie, prompting a model-downgrade hunt that never happened. cascadeVerdict now returns the model that actually ran its first pass (the pin, else the cascade default) on every judge-backed outcome; the emit and savePasses sites report it, falling back to firstModel only when no judge ran at all (missing brief / engine error) so the field stays always-present for consumers. Covered by a new run-review test asserting the emitted events carry sonnet for the pinned correctness reviewer, haiku for conventions (pinned) and unpinned reviewers, and that the cached PASS records the pinned model. 58/58 in the file green.
…very-commit runs (#96) Every-commit telemetry runs (run-context.mts) have no wrapper process, so nothing marks them finished: the usage dashboard could only settle a commit-mode run after a 35-minute event-quiet window, leaving rows 'in flight' long after the chain ended. The hook is the one process that knows the whole chain's outcome, so the devkit-guards block now arms an EXIT trap that appends a commit_result event ({type, ship_id: commit-<write-tree>, run_mode, repo, branch, exit_code, duration_s, ts}) to the telemetry sink. The tree hash is computed at EMIT time — the biome fragment restages formatted files, and the gates correlate under the post-format write-tree. Same opt-outs as the capture itself: DEVKIT_NO_TELEMETRY disables it, and inside a ship (DEVKIT_SHIP_ID set) it stays silent because ship_result is that run's terminal. All three hook shapes covered: package block, standalone block, and the overlay — where the pass path must emit explicitly before the exec chain (exec never fires EXIT traps). Exec-level tests run the assembled hook inside a real temp git repo: one terminal per run, correlated to the staged write-tree, exit_code 0/1 for pass/blocked chains, silent under DEVKIT_SHIP_ID and DEVKIT_NO_TELEMETRY. (The suite-wide DEVKIT_NO_TELEMETRY=1 from vitest.setup is deliberately stripped inside these tests — they point the sink at a temp file.) 49/49 across both hook suites. The usage dashboard's collector already consumes the event (falls back to the quiet window for hooks predating this). Consumers pick it up on their next devkit upgrade / init.
…23-row eval tier + amend hard-block guard (#99) Field bug (frink, cross-agent): the sentry gate hard-blocks commits that FIX silent error-classes. frink's adoption of guard-sentry v3 is benord-labs/frink#105; this PR closes the two gaps that remain in devkit's own judge. ## Gap 1 — fix-ELIMINATES-the-class has no self-clear (the repro class) The diff-tier directive self-cleared only capture-ADDING fixes. A fix that eliminates the silent path (swallow deleted so the failure propagates, or pure-logic/state fix with no error hunks) had zero corpus rows and no prompt license — measured 15/23 on the new tier, with one shape (silent strand fixed by explicit state handling) reliably wrong 3/3. **Corpus:** +23 authored `elimination` rows (14 SKIP eliminations across varied shapes — catch deleted, rethrow added, .catch→await, throw-on-invalid, rejecting timeout, pure-logic, explicit-state, path deletion — and 9 MONITOR traps: sibling swallow remains, swallow moved/narrowed, capture-in-test-only, message-diff mismatch, swallowing timeout, crash→silent-no-op, new swallowed fallback). Tagged + provenance-noted; slice out via `category` to score the real-derived 104 alone (methodology: synthetic rows reported with/without). **Prompt:** one clause added to the diff-tier directive — elimination ⇒ SKIP only when the failure now propagates to auto-capture or is prevented by pure logic AND no swallowed handling remains or is added; explicit non-elimination cases (caller still swallows, moved/narrowed, crash→no-op) stay MONITOR. **Measured (haiku, diff tier, full 127-row paired A/B):** | | no clause | clause | |---|---|---| | elimination tier | 15/23 | **19/23** (K=3: 18/23 stable, 20/23 majority — the live samples=3 config) | | MONITOR precision | 0.79 | **0.84** | | MONITOR recall | 0.95 | 0.90 | | instrumented (24 self-clear rows) | 24/24 | 24/24 | The apparent recall dip is noise, not clause-caused: a K=3 dual-arm panel on every disputed real row showed no stable loss (mon-mcp-project-discovery actually stabilized 1/3→3/3) and the stable failures are IDENTICAL in both arms (multi-permissions-two FN 0/6; bord-approval-away-sound + bord-gate-judges-fail-visibly FP 0/6) — pre-existing boundary rows, flagged as label-review candidates (methodology item 19). Real-derived 104-slice: P 0.88 / R 0.90 / F1 ~0.89. Honest residuals (stable at K=3, in-corpus): skip-elim-explicit-user-error (judge won't accept surface-to-user as elimination, 0/3), skip-elim-zombie-status-reconcile (1/3), mon-elim-early-return-drops-work (1/3 — subtlest trap). ## Gap 2 — hard mode blocked on message-only evidence (amend shape) Live-verified on frink: `git commit --amend` with nothing restaged → empty staged diff → the diff tier silently degrades to message-only judging → an honest "fix silent X" subject hard-blocks with zero diff evidence (the exact config the eval rejected for blocking). New `effectiveHard()`: on the diff tier a hard block requires diff evidence; the empty-diff fallback downgrades to warn + watchlist. An explicitly configured message/names tier keeps its hard block. Unit-tested (6 new rows). ## Drive-bys required to unblock shipping at all (main is merged-red) Every devkit ship currently fails the whole-tree lint gate on debt that landed with #96: - `gate-engine/review/run-review.mts`: biome format drift → formatted. Staging it then trips the size ratchet (file was ALREADY over its 623 cap on main; formatted = 642 gate-counted lines; `guard-size freeze` is shrink-only by design). **size-lines.json bumped 623→642 with the user's explicit OK** — the honest alternative is the split the gate wants, deferred because the file has in-flight edits in the shared checkout. - `cli/__tests__/husky-block-exec.test.mts:240`: noTemplateCurlyInString on an intentional shell `${VAR:-default}` in a stub script → biome-ignore with reason (still 13/13). - `cli/lib/husky/husky-block.mts: 524` grandfather entry — written by `guard-size freeze` itself (#96 grew it past 500 with no entry). ## Also - README/bench/header count + default-tier doc drift fixed (103/104→127; BENCH_CONTEXT default is `diff`, not `message`; dataset-format doc gains the `diff` field). - Repro verified end-to-end: the reported frink commit shape judges SKIP 3/3 via the diff tier; a swallow-introducing change still blocks (exit 1) through frink's hard-mode hook. - Suite: typecheck + build green; 87/87 sentry tests; full vitest green except 2 failures (overlay git-ci alias, self-host hook parity) confirmed pre-existing on pristine origin/main. - NOT done deliberately: dropping "or touches" from the guide (the regression-guard MONITOR design the corpus encodes) and any advisory revert (hard mode is a recorded, evidence-backed ruling).
…odetect (DK-5) (#98) ## DK-5 — `ship --base` audits against `main`, not the real ship base `devkit ship --base <b>` cuts the ephemeral gate worktree from `<b>`'s origin tip, but the in-chain `fallow audit` (both the self-host advisory fragment and the overlay gate) always ran bare — `fallow audit`, no `--base` — so fallow auto-detected `main` as its comparison ref regardless of what the worktree was actually cut from. Shipping `--base feat/studio` from a finalized checkout, where `feat/studio` is itself a long-lived stack with its own pre-existing (not-yet-merged) dead-code / duplication findings vs `main`, misreported those as "new" and failed the audit. Fallow's own `audit` command already grandfathers correctly when given the right ref: its `gate: new-only` default runs the base ref too and marks inherited findings `introduced=false`. The fix is to thread the exact commit ship already resolves the worktree from — through as `DEVKIT_SHIP_BASE_SHA` (exported by `ship-branch.sh`/`reship.sh`, alongside the existing `DEVKIT_SHIP_MODE`) — so devkit's own fallow fragments pass `fallow audit --base "$DEVKIT_SHIP_BASE_SHA"` when it's set, falling back to fallow's own default on a plain `git commit`. This is unconditional (not just under `--base`): even the default ship case is more precise than `main`-autodetection for any branch that isn't a fresh cut off main. `.husky/pre-commit` (this repo's own self-hosted hook) is regenerated in the same commit — required, since `self-host.test.mts`'s parity test compares it byte-for-byte against the generator. A narrower, separate bug (`link-gate-configs.sh` symlinking `fallow-baselines/` from the invoking checkout rather than the ship base — relevant only to consumers using fallow's own self-installed git hook) is deliberately out of scope here, mirroring how DK-3 split an analogous baseline-follows- ROOT bug into its own PR. ## Also in this PR `.husky/pre-push` runs the full, untargeted test suite on every push, so any pre-existing failure blocks shipping anything. `overlay.test.mts`'s `workRepo()`-based tests call `applyInit` in-process (unlike the ship-branch/reship/reconcile suites, which spawn devkit as a subprocess and isolate git via an explicit per-call `env`), so its internal `execFileSync('git', …)` calls (e.g. `installHealAlias`'s `alias.ci` collision check) inherited whatever `process.env` already was — including a developer machine's own real `~/.gitconfig`. `overlay.mts` deliberately never clobbers an existing `git ci` alias, so a machine with one already set makes that check correctly skip installing devkit's self-heal alias, which then read as a false test failure. Isolated `GIT_CONFIG_GLOBAL`/`GIT_CONFIG_SYSTEM` to `/dev/null` for the file's duration (restored after), matching the isolation every sibling git-integration test file already does. Unrelated to DK-5 itself, but bundled in since it's what blocked the push. ## Verification - `bun run test:run -- cli/__tests__/ship-branch.test.mts cli/__tests__/reship.test.mts cli/__tests__/self-host.test.mts cli/__tests__/husky-block.test.mts cli/__tests__/overlay.test.mts` — 153 pass - `bun run typecheck` clean - `bun run lint` clean - `bun run lint:structure` clean - `git diff .husky/pre-commit` confirmed scoped to only the `# devkit:fallow-advisory` fragment ## Also bundled: a one-line formatting fix `origin/main` advanced (PR #94) between this branch's creation and shipping; `gate-engine/review/ run-review.mts`'s `savePasses(...)` call in that merge violates biome's line-length rule. Since `bun run lint` (`biome check .`) scans the whole repo, this now blocks any ship off `main` — bundled the trivial reflow biome itself proposes (no logic change) so this PR can land. Unrelated to DK-5. That reflow also pushed `run-review.mts` (already at 640 lines, 17 over its recorded `eslint/baselines/size-lines.json` ceiling of 623 from #94 itself) over the line-count ratchet. Bumped that one entry to its true current count (642) — a surgical 1-line baseline update rather than a full `guard-size freeze` (which re-walks the whole repo and could touch unrelated entries).
…t items + SKILL.md de-vendoring (#97) * feat(review): catalog refresh from licensed sources — 11 new checklist items + SKILL.md de-vendoring # feat(review): catalog refresh from licensed sources — 11 new checklist items + SKILL.md de-vendoring Closes the last piece of the pre-commit-agents improvement plan (bench #31, correctness reviewer #31/#57/#59/#64, haiku first pass #33–#37 all shipped earlier). ## Why 1. **Coverage**: the domain catalogs predate the OWASP API top risks — the gate had no item for object-level authorization (BOLA, the #1 real-world API vulnerability), mass assignment, command/path injection, SSRF, open redirects, cross-origin messaging, event-loop blocking, or rendering-cost classes. 2. **License**: three SKILL.mds carried near-verbatim roadmap.sh best-practices text. That content is licensed personal-use-only and this repo is public. All rule text is now written fresh for this repo; roadmap.sh was used as a topic-coverage checklist only. ## What **11 new checklist items** (regex-triggered, judge-checkable, each with an explicit FAIL bar in SKILL.md): | skill | new items | |---|---| | api-security (14→20) | `mass-assignment`, `command-injection`, `path-traversal`, `object-level-authz`, `open-redirect`, `ssrf-prevention` | | backend-performance (15→17) | `sync-io`, `unbounded-cache` | | frontend-performance (17→19) | `layout-thrash`, `animation-performance` | | frontend-security (19→20) | `postmessage-origin` | Two-regex items (`path-traversal`, `object-level-authz`) AND a sink pattern with a request-input marker so they only fire when both halves are present. **SKILL.md rewrites** (api-security, backend-performance, frontend-performance): every rule states greppable evidence + an explicit FAIL condition; no "consider/ensure" phrasing. Each SKILL.md ends with a **Provenance** block: sources used for topic coverage (roadmap.sh topics-only; OWASP ASVS v5.0 CC-BY-SA chapters), covered-topic → item mapping, and rejected topics with reasons — future refreshes become a diff, not a re-derivation. **Prose-file filter** (methodology lesson: every judge hallucinates on prose diffs): the four checklist scripts now exclude `.md/.mdx/.markdown/.txt` from the staged scan — a README mentioning "password" no longer adds judge items. Gate-level docs-only skip already existed (`selectReviewers` extension filter); this closes the script-level path where prose rode along with a source commit. **Bench fidelity (reviewer-eval)**: `buildAssets`/`benchGateHash` now include the skill's SKILL.md — the brief's workflow sends the judge there, consumers always have it synced, so fixtures without it under-equipped the judge and SKILL.md edits didn't invalidate baselines. Also replaced a literal NUL byte in `bench.mts` (gateHash join separator) with escaped `\x00` — same runtime string, but grep treated the file as binary. **Corpus**: +5 gold rows (one per flagship new item: `apisec-mass-assignment`, `apisec-idor-lookup`, `beperf-sync-io-handler`, `feperf-scroll-reflow`, `fesec-postmessage-wildcard`) and `apisec-command-injection` retagged to its now-dedicated item. Domain corpus 48→53 rows. **Drift repair**: `.claude`/`.cursor` synced copies of `correctness-reviewer.md` still said `model: haiku`; source moved to `sonnet` in #64. `sync-agents` output included. ## Evidence - `bench.mts validate` green across all 5 cohorts (119 rows) with the new catalogs — selection + `expectItems ⊆ generate` + injection guard. - New vitest file `cli/__tests__/checklist-catalog-triggers.test.mts`: 23 tests — a fires/stays-quiet fixture pair per new item (real `checklist.mjs generate` in disposable repos) + the prose-exclusion test. - Slim live proof (K=2, haiku→opus cascade): **10/10** — all 5 new-item gold rows (`apisec-mass-assignment`, `apisec-idor-lookup`, `beperf-sync-io-handler`, `feperf-scroll-reflow`, `fesec-postmessage-wildcard`) blocked with `right-item` attribution on both independent runs (`first=FAIL final=fail (escalated) [right-item]`). - **Full before/after haiku A/B** (BEFORE = origin/main catalogs in a frozen worktree; AFTER = this PR; per-row join on the 48 shared rows): **zero flips** — end-to-end block recall 24/24 both sides, clean-pass 23/24 both sides. The single decoy miss (`beperf-nplusone-statement-loop-fixed`, an adversarial minimal pair opus confirms) is **stable in both states** — pre-existing calibration, not this PR. Final state across all 53 rows: **block recall 29/29, clean-pass 23/24** — floors (≥0.75 / ≥0.85) hold with room. - **The bench caught a real regression in this PR during development**: the first SKILL.md rewrite softened the "memoize expensive computations" bar and `feperf-sku-rank-per-render` (adversarial gold: O(n·m) ranking re-run per render, decoy `useMemo` on a trivial label) went from caught to stable-missed (2-of-2). Bar sharpened to name the actual rule (memoize on real inputs; unrelated re-renders re-run it; trivial-downstream memo doesn't count) → row recovered 2-of-2 and the frontend-performance re-baseline is 13/13 perfect. Working exactly as designed. - `tsc` + `biome` green; full suite 1402 tests green. One **pre-existing** env-sensitive test fixed en route: `overlay.test.mts`'s alias-install tests didn't isolate `GIT_CONFIG_GLOBAL`, so they fail on any branch once the developer machine has its own global `ci` alias (the installer's skip-on-collision then correctly skips). Same isolation idiom as the adjacent skip-on-collision test; the vacuously-green `clean` twin now asserts install first. ## Drive-bys (unblocking main) - `gate-engine/review/run-review.mts`: **format-only reflow** of the `savePasses` line #94 merged unformatted — `biome check .` failed for every ship at tip. - `eslint/baselines/size-lines.json`: run-review.mts entry 623 → 641 — #94 grew the file past its ratchet ceiling without the entry moving (its path bypassed the local gate), so guard-size also blocked every ship touching the file. This bump reconciles the ratchet with already-merged main — it launders nothing of this PR's own growth. - `bench.mts` split: this PR's additions pushed bench.mts past **its own** ratchet (986 > 977) — resolved the honest way, extracting the corpus/fixture layer into `corpus.mts` (re-exported, test imports unchanged; completeness-eval's separate `matcher.mts` is the precedent). bench.mts now 883 lines. ## Notes for review - Item text lives in SKILL.md; triggers in `skills/<skill>/scripts/checklist.mjs`; briefs' category lists updated to match. - No dist changes (release-time artifact). - Consumers pick this up at the next release via `devkit upgrade` (sync-skills/sync-agents). * fix(review): prose-only selection skip + corpus.mts in gateHash (CodeRabbit) + results-history README Three things onto PR #97: 1. CodeRabbit finding addressed: corpus.mts (the extracted fixture layer) now hashes its own source into benchGateHash, so a fixture-behavior edit can never be compared against an incompatible baseline. README gateHash inputs updated. 2. selectReviewers: backend/frontend domain reviewers no longer select for prose files (.md/.mdx/.markdown/.txt) under their roots — their checklist scripts skip prose, so a prose-only selection stranded the judge with an empty checklist (inconclusive, fail-closed under strict ship; bit this PR's own README-only re-push). Selection and script now share the same exclusion; two new selectReviewers tests (prose-only skips, prose-alongside-source drops the prose from the file list). 3. Bench README results-history section: the before/after table (zero flips on 48 shared rows; pooled block recall 24/24 → 29/29), the sku-rank regression-caught note, sc-1147/sc-1148 pointers, stale 48→53 pool figure fixed.
#100) A ship's telemetry recorded its branch but never the PR it opened, so a downstream reader (the usage dashboard) had to resolve branch->PR itself via gh. ship-branch.sh already captures the PR url + number from gh pr create; it now emits a ship_pr event ({type, ship_id, pr_url, pr_number, ts}) correlated to the same DEVKIT_SHIP_ID as the run's gate/ship_result events. It's a separate line rather than a field on ship_result because ship_result fires during the gate chain — before the PR exists — and the sink is append-only. Best-effort (>> || true) like every other telemetry write, guarded on the sink+id being set, emitted only on PR-create success (the failure path already records pr:null for reconcile). reship/older ships without this line are still linkable by the dashboard's gh-by-branch backfill. Covered by a ship-branch exec test: a stubbed gh pr create -> a ship_pr event with the url, a bare-number pr_number, and the same ship_id as ship_attempt. 60/60 in the file.
…y-default (3-sample contract) (#101) * feat(sentry): registry guard + devkit-managed commit-msg hook, hard-by-default (3-sample contract) Closes the wiring gap behind benord-labs/frink#105: the sentry judge shipped as a bin with zero consumer wiring, so frink hand-wired the hook line (and the completeness commit-msg line before it). Hooks now come from devkit. ## What ships - **Registry**: `sentry` joins `GUARD_IDS` (offerable at init/upgrade; NOT recommended — needs a Sentry-using product). Wizard/help/doctor entries. - **Devkit-managed `.husky/commit-msg`**: new `cli/lib/husky/commit-msg-block.mts` generates a marker-wrapped block when `review` and/or `sentry` is selected — PATH preamble, `|| var=$?` tested-status plumbing (husky `sh -e` vs exit-2 fail-open), completeness + sentry invocations with block guidance, explicit `exit 0`. Idempotent install/removal wired through init/upgrade/clean/doctor (`--fix` handles missing AND drift); consumer content outside markers preserved; standalone mode emits `command -v`-guarded global-bin lines. Package + standalone only (self-host opts out — would newly gate devkit's own commits; overlay uses core.hooksPath). - **Hard-by-default**: `envBool('SENTRY_HARD') ?? true` (completeness precedent; env-only soften, no config key). Samples follow the frink Target's confidence contract: default 3 when hard, 1 when warn (`resolveSamples`, override wins). `effectiveHard` (from #99) still downgrades empty-staged-diff/amend to warn. - Decision note appended under the 2026-07-12 hard-by-default Target; README bins table updated. - Drive-by: committed `.husky/pre-commit` regenerated (+22 lines, the #96 commit-terminal fragment) — fixes the pre-existing self-host parity test red on main. ## Review-round fix (first ship attempt) The correctness reviewer correctly FAILED the first attempt: the worktree base predated #98 (DK-5), so the whole-file ship would have reverted the `DEVKIT_SHIP_BASE_SHA` fallow scoping. Rebased onto current main (post-#97/#98); the DK-5 overlay-fallow test block is restored alongside the new commit-msg tests (67/67). ## Ratchet bump (cross-PR race, user-approved pattern) `size-lines.json` husky-block.mts 524→528: #99 froze the cap at 524 while #98 (merged after) grew the file to 528 — main was over its own cap before this PR touched it (this PR's only husky-block.mts change is exporting PATH_SETUP, zero growth). ## Verified Touched suites green (131 + 67); typecheck clean; e2e: init with review+sentry → doctor reports both hook blocks OK; deselect/clean remove the block; upgrade offers sentry as opt-in. Only remaining suite red is the pre-existing overlay `git ci` alias test (red on pristine origin/main). ## Review-round fix (second attempt): completeness fragment now fails CLOSED on exit 3 (strict-ship judge outage), mirroring the pre-commit AI fragments — the reviewer correctly rejected the silent continue. ## Flagged, not done (follow-ups) - critique is the next half-migrated judge (devkit engine + eval exist, no CLI, frink runs local check-critique). - frink redo after next release + pin bump: select sentry guard, delete hand-wired lines + `check:sentry` script, rework #105. * fix(sentry-guard): review round — self-host block removal, deselected-fragment drift, test tmp cleanup CodeRabbit round: (1) package→self-host re-init now DROPS a previously-installed commit-msg block (stale hard gate would keep judging devkit's own commits); the doctor half of that finding is not applicable — self-host routes to runSelfHostDoctor before checkCommitMsgHook can run. (2) checkCommitMsgHook now flags DESELECTED lingering fragments as DRIFT (symmetric sentinel check, same depth as pre-commit's checkHusky; full-text equality would exceed the house standard). (3) test watchlist tmp dirs tracked + reclaimed in afterEach.
* feat: add universal benchmark evidence tracker ## Summary - rewrite the homepage README for external developers with quick start, operating modes, limitations, and a complete benchmark dashboard - add an append-only benchmark evidence ledger with content-addressed checkpoints, provenance, freshness, comparability, privacy, and reconciliation rules - add deterministic render/publish/check/backfill tooling, catalog discovery coverage, staged/tree CI enforcement, and regression tests - backfill the available benchmark history without claiming an unsupported exponential or diminishing growth curve ## Historical conclusions - feature critique B1 to B2 is a genuine multi-metric improvement - correctness gained recall from model choice while precision stayed flat; cascade then reduced recall - reviewer-catalog expansion is coverage growth with zero shared-row quality flips - completeness is a methodology reset, conventions has one accepted point, and edge cases records a valid no-ship outcome - reported/local sentry and decisions evidence remains provenance-graded rather than mixed into accepted trend lines - current evidence is insufficient to characterize the overall growth curve ## Validation - tracker adapter/source/history/render tests - scoped strict tracker typecheck - staged and working benchmark checks - structure lint and deterministic render checks - commit-guard duplication review - devkit ship reviewers and pre-push suite * fix: address benchmark tracker review feedback Addresses all unresolved benchmark-tracker review findings with regression coverage. * fix: run TypeScript ratchets in CI Updates stale CI entrypoints from .mjs to the tracked .mts ratchet sources. * fix: make benchmark publish locks crash-safe
…anscript, exec seam) (#117) * feat(telemetry): judge_exec event per claude -p invocation, opt-in transcript at the exec seam Every judge devkit spawns now records one spend/outage line (type judge_exec: label, model, outcome ok/timeout/transient/empty, duration, input/output sizes) into the existing gate-events sink the usage tracker already tails — emitted from execJudge/execJudgeAsync themselves, so gates that never called the gate-level writers (the factory-built judges, sentry) become visible by construction instead of by remembering. Transcript persistence is opt-in (transcript: true) to avoid double-storing what the review gate already saves; the factory and sentry call sites opt in since they had no transcript store at all. Transcripts for the per-invocation ledger use a new exclusive-create store (saveTranscriptUnique, wx + suffix probe): a durably-appended event's transcript_ref must keep resolving to its own invocation's output across multi-sample votes AND across retried/amended-commit processes sharing a run id — the correctness reviewer caught both weaker designs. Best-effort throughout: writers never throw and no-op without a sink; judge contracts and exit codes untouched. * feat(telemetry)!: transcripts collected by default; record the vision-gate road-not-taken Transcript collection flips to DEFAULT-ON at the exec seam (user ruling: the ledger's point is that judgements stay inspectable without any caller remembering to ask). transcript: false becomes the opt-OUT, applied where a gate persists its own gate-level transcript (run-review first pass + escalation) so nothing stores twice. decisions detect keeps default-on for now — a temporary, reviewer-acknowledged double-store — because its file already sits past its frozen size cap from mainline drift and staging it walls any commit; its opt-out belongs to that file's own cleanup. completeness and check-alignment, which stored nothing, gain transcripts for free. Also records the vision-gate-hosted Target on main's decision log: PR #77 closed unmerged (vision gate stays consumer-local; a benched mechanism over unvalidated vision content still ships false confidence), preserving the road-not-taken that would otherwise vanish with the closed branch. --------- Co-authored-by: norvalbv-slice <benji.norval@slice.com>
…the ship worktree's HEAD (#122) * fix(ship): pin fallow >=3.6.0 — its audit worktree cleanup clobbered the ship worktree's HEAD A devkit ship could fail at the finalize step with fatal: cannot lock ref 'HEAD': is at <origin/main> but expected <ship base> after every gate had already reported PASS, leaving nothing pushed. Reported as "--pr is unusable for a PR based on a non-default branch", but the base logic was never at fault: reship.sh has cut its worktree from the fetched PR-branch tip (BASE=$(git rev-parse FETCH_HEAD)) and cleaned up on a trap since v0.35.1. The clobber came from fallow. Through 2.x, `fallow audit` materialised its comparison base as a REGISTERED git worktree and its cleanup was not scoped to the entry it owned, so an audit running inside a ship's multi-minute gate chain could reset THAT worktree's HEAD to the audit's own base. git's finalize ref-update is a compare-and-swap, so the commit then died. Telemetry bears this out: the "is at" sha was identical across every recorded failure (origin/main) while "expected" tracked the correct per-branch ship base, and it hit normal ship mode too — a race, not a --pr base bug. fallow 3.4.2 unregisters the snapshot immediately and restricts git administration cleanup to the repository's own verified worktree entry; 3.6.0 holds the reuse lock for the audit lifetime. 3.4.2 also auto-deregisters the entries leaked by earlier versions (16 of them were live in the repo this was diagnosed against; one audit on 3.6.0 cleared all of them). 3.0.0 is a platform milestone, not a breaking change — CLI flags, config and the audit --format json contract that gate-engine/fallow/staged-filter.mts parses are unchanged. Also attribute the failure instead of burying it. blocked_gate fell through to "unknown", which is what sent the original report chasing the push step; it is now "worktree_head_clobbered", with a banner saying the gates passed, nothing was pushed and a re-run is safe (verdicts are cached). The new arm is tested BEFORE the gate greps: guard-review INCONCLUSIVE is fail-open, so it can sit in the same log and would otherwise claim a failure it did not cause — without the fix that case reports "review". * fix(ship): prove the HEAD clobber from worktree state, not a log phrase The captured gate log is a COMBINED stream — `git commit … 2>&1 | tee` folds every hook's output in with git's own — so matching `cannot lock ref 'HEAD'` as text could not tell OUR commit's failed HEAD finalize apart from a GATE that merely printed the same string and exited non-zero. That misattribution had teeth: the banner would tell the operator "every gate PASSED, nothing was pushed, re-running is safe" about a genuine gate block. devkit's own suite emits this exact string, so a gate running the tests would forge the phrase. Decide on evidence instead: the ship worktree's HEAD must actually have moved off the commit it was cut from (DEVKIT_SHIP_BASE_SHA, exported by both ship-branch.sh and reship.sh). No commit exists on the failure path, so HEAD==base is the only honest "nothing moved" state, and a gate that prints the error leaves it there. With the env var unset nothing can be proven, so it falls through to the gate greps rather than claiming the gates passed. The stderr banner now reuses that one validated verdict instead of running its own second grep, which could have drifted from the telemetry's. Adds the negative test: a gate that prints the fatal verbatim and exits 1 must be attributed to that gate ("deterministic"), with no all-clear banner. It fails against the previous text-only match — reported as "deterministic" expected, "worktree_head_clobbered" received.
* refactor: share gate worktree preparation ## Stack **1 of 7 — review only this slice.** - Supersedes part of #95 - Base: `main` - Next head: `codex/review-stack-02-profile-hooks` - Landing: keep the stack frozen for review; after approval, collapse top-down into PR 1, then squash PR 1 to `main`. ## Scope Extract the worktree preparation and gate-output capture shared by ship and reship, preserving existing behavior while giving the later review command one implementation to reuse. ## Validation - Focused ship/reship integration tests - Commit guard: PASS - Final cumulative stack: 92 test files / 1,535 tests passed - Typecheck, Biome, structure lint, build, and CLI smoke: PASS * fix: keep archive logging best-effort Keep the required per-branch gate log strict while treating the telemetry archive as best-effort. Warn on archive failures without changing a successful gate result, with regression coverage for an unwritable archive path.
… two shim-dead bins Five defects found while diagnosing a 35-minute pre-commit stall in a linked worktree. Each one either weakens a gate silently or corrupts state. - matcher: add an optional `indexCommand` refresh so the gate judges the code being committed, not the last indexed state. Heavily conditioned (new index-refresh.mts), because both misfires are severe: cold-building a corpus inside a commit hook is minutes to hours and reads as a hang, and refreshing through a worktree's linked index rewrites the PRIMARY checkout's chunk rows with this checkout's code. It runs only when the index already exists AND physically resolves inside this checkout, once, under a hard wall-clock kill. The missing-index message now names the worktree remedy (link the primary's index) instead of leaving a bare "no index" to read as a hang or a bug. - clone-detector: drop the `/^.*\/src\//` -> `src/` rewrite from relPath. It collapsed every scan root containing `/src/` onto a phantom top-level `src/`, which fused two real files onto one allowlist key (approving one silently approved the other) and made `--changed` compare a path that does not exist against git's real one, dropping every non-first-root clone out of the scoped gate. Repo-root-relative is now the only normalisation. Approvals survive: clone entries key on fragmentHash, which hashes text, not paths. - clone-detector + allowlist-cli: fix the run-as-main guard. `argv[1] === fileURLToPath(import.meta.url)` compares the bin SHIM path (always a symlink) against the real module path, so both bins parsed their args, dispatched nothing and exited 0 when invoked by name -- `guard-clone scan --gate` was a silently passing gate, and `guard-dup-allowlist`, the remedy both gates print, did nothing. Adopts the realpath form the other ~15 gate modules already use. - changed-files: subtract files with unstaged edits on top of what is staged. Both detectors read the WORKING TREE, so for a partially-staged file the content they judge is not the content being committed -- gating on it blocks a commit over code it does not contain. - config: add `cloneRoots` (falls back to scanRoots) so clone scope can be narrower than matcher scope, and restore the `check-clone` / `baseline-clones` verbs plus a day count in `list`, which consumers still need for clone triage and re-freezing. The two new regression guards are mutation-tested: reverting either fix fails its test. The shared `flag` reader (argv.mts) is extracted because touching both CLIs surfaced it as a real pre-existing dup, not to satisfy the gate. No version bump here -- release is a separate commit (see #182). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`checkHusky` proves a hook FILE exists. Nothing proved git would ever RUN it — and on this machine three real repos, devkit included, were gating nothing. ## The failures this catches **1. The runner never reaches a new worktree.** Husky pins a RELATIVE `core.hooksPath` (`.husky/_`) and gitignores the runner it points at (`.husky/_/.gitignore` = `*`). A linked worktree checks out with hooksPath resolving to a MISSING directory, and git treats "no runner" as "no hooks": every commit made there is **silently ungated**. Hit for real — a commit reported as "fully gated" ran zero gates. **2. The hook is installed but nothing points at it.** `.husky/pre-commit` is committed while `core.hooksPath` is unset, because husky never ran (not a dependency, or an install that skipped `prepare`). Git falls back to its own hooks dir and runs nothing — in the MAIN checkout, not just worktrees. Found in two repos committing with zero gates; `doctor` called both healthy. A pre-commit sitting in git's own hooks dir only counts if it actually delegates to `.husky` — an unrelated one runs INSTEAD of the devkit hook, not as well as it. **3. One hook wired, its sibling not.** `.husky/` declares `pre-commit` and `commit-msg` but only `_/pre-commit` was ever generated — that hook silently runs nothing while its siblings look perfect. Hooks are matched by real git hook NAME, so a stray `README.md` in `.husky/` never counts as a hook owing a stub. ## Untracked vs gitignored — the distinction that matters Case 1 flags untracked **AND gitignored**, not merely untracked. A merely-uncommitted runner is transient (the next commit carries it); untracked *and* ignored is permanent, because no ordinary `git add` can ever pick it up. Flagging plain-untracked would fire on every freshly-`init`ed repo before its first commit — caught by `standalone.test.mts` while building this. ## Layouts and scope - Husky keeps hooks in `.husky/` beside the `_` runner; a custom hooksPath (`.githooks/…`, which standalone deliberately leaves alone) has no `.husky` dir, so the runner dir is read instead — reading `.husky` unconditionally would crash `devkit doctor` on that layout. - Reads `--local` and `--worktree` separately and judges the SHARED value, since that is what `git worktree add` inherits. - Case 2 is guarded on being in a git repo at all, and resolves git's real fallback dir via `rev-parse --git-path hooks` (a linked worktree has no `.git/hooks` of its own). ## Detection only Never marked `fixable`: the repair stages files or rewrites git config, which `--fix` must not do in a consumer's repo unasked. The remedy string names the exact command. ## Self-host dogfood The self-host doctor path never called `collectResults`, so devkit's own repo — which HAS this defect — could not see it. It now runs the same check and gates its exit code on it, and devkit's runner is tracked here so it stays green (`devkit doctor` went exit 1 → 0 on that change). ## Why `checkHusky` moved `doctor.mts` sat exactly at its `guard-size` cap (772), so even one import line breaks the shrink-only ratchet. Both hook checks now live in `cli/lib/doctor/hook-checks.mts` — complementary halves of one question, matching the existing `asset-checks.mts` pattern. `checkHusky` is a **pure move**, no logic change. Verified against five real repos plus 13 real-git fixtures (62 tests). The consumer-side fix this detects is now merged in frink, and proven end-to-end: a commit in a worktree cut from that merge runs the gate chain and is rejected, where pre-fix the same commit landed with zero output.
feat(doctor): catch hook wiring that silently gates nothing
… timeout env override Four of the five CodeRabbit findings on #183 were valid: - argv: `flag('--min-loc')` as the LAST token returned undefined, so callers coercing with Number() got NaN and every comparison against that threshold silently became false — disabling the tier rather than erroring. Falls back to the default now. (Pre-existing in both original readers; carried in by the extraction, fixed here.) - clone-detector relPath: normalise BOTH sides to '/' before stripping, and strip prefix-only. One-sided normalisation never matches on Windows (repoRoot keeps native '\'), leaving every allowlist key absolute; a bare .replace() would also rewrite a repo path recurring mid-string. - index-refresh: use path.relative()/isAbsolute() instead of a `${cwd}/` string prefix. On Windows the prefix never matches, so a safe local refresh was always declined. - config: `indexCommandTimeoutMs` had no env override while indexCommand did, breaking the documented env > file > default precedence. Added, and both env and file values must now be POSITIVE — node reads a 0 timeout as "no timeout", which is exactly what `Number('')` yields for an empty env var, so accepting it would silently remove the wall-clock bound this exists to enforce. Rejected: the suggestion to extract a shared `rangeOf`. matcher.mts's `rangeOf` is a Map<string,string> of sqlite start/end_line values, unrelated to the `number | {line}` position helper in allowlist-cli — same name, different thing, nothing to share. New: argv.test.mts and index-refresh.test.mts pin these contracts, plus baseline-clones added to the MODES coverage loop. The argv fallback is mutation-tested. The Windows path fixes are NOT covered by a failing-before test — both implementations agree on POSIX, so the delta is only observable on Windows, which this machine cannot exercise. Suite: 1987 pass. Committed --no-verify with the owner's explicit OK. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d-dead-bins fix(co-occurrence): make the dup/clone gates worktree-safe and revive two shim-dead bins
… a manual git add -f This closes the class of bug fixed by #184 (a fresh `git worktree add` finds no husky runner and gates nothing) permanently, for every NEW consumer repo — not just this one. ## The mechanism Husky pins a RELATIVE `core.hooksPath` and gitignores the runner it points at. Nothing in devkit can change what a fresh checkout materializes — that's determined by what's TRACKED in the consumer's own history, which is why #184 shipped as a detection-only doctor check with a manual `git add -f` remedy. This closes the loop for new repos: a fresh `devkit init` (package mode, husky selected) now generates ``` "prepare": "husky && (command -v devkit >/dev/null 2>&1 && devkit sync-hook-runner || true)" ``` `sync-hook-runner` is a new command that `git add -f`s exactly the runner files that are untracked-and-gitignored (reusing the same matching logic `checkHookRunner` already uses — real git hook names, not directory-listing existence). Every `bun install` re-stages the runner past husky's own ignore, so the untracked-runner state can never persist past a repo's first install. ## Why this doesn't repeat the mistakes #184's reviewer caught, or the ones the original absolute-pin proposal made - **Never touches `core.hooksPath`.** No config write at all — so none of the three things that killed the earlier absolute-pin idea apply here: `devkit review`'s exact-equality hooksPath check is untouched, there's no `--worktree`-scope shadowing to get wrong, and it needs no `package.json` devkit devDependency bump (package-mode installs already carry one; the guard is `command -v` + `|| true` regardless, so it degrades safely even without it). - **Never folded into `--fix`.** `devkit doctor --fix` is documented and tested as file-content-only; it never mutates the git index. Staging stays an explicit, separate, user-invoked command — consistent with why `checkHookRunner` was never marked `fixable` in #184. - **A missing `devkit` binary can't break an install.** `command -v devkit >/dev/null 2>&1 && … || true` — verified: with `devkit` genuinely absent from PATH, the chain exits 0. ## Verified end-to-end (not just unit tests) In a scratch repo: `devkit init --yes` writes the exact chained `prepare` script above. Running that exact string as a real shell would (husky + devkit both resolvable) results in the runner being tracked — proving the full mechanism, not just its pieces. Running it with `devkit` absent from PATH exits 0 without touching anything. ## Scope - New: `cli/commands/sync/sync-hook-runner.mts`, wired into `cli/index.mts` (COMMANDS + GIT_COMMANDS). - `cli/lib/doctor/hook-checks.mts`: exports `unreachableRunnerFiles` (the same matching logic, reused — `checkHookRunner`'s own shape is untouched to avoid touching already-reviewed logic) and its DRIFT remedy now names `sync-hook-runner` first. - `cli/commands/init.mts`: the new chained `prepare` script for package-mode + husky-selected installs only; standalone/self-host untouched (verified by test). - Decision recorded: `devkit-owned-hook-runner-delivery` (Target), including the two rejected approaches from #184's own review history and why folding this into `--fix` was rejected. - `guard-size freeze` will be needed for `cli/commands/init.mts` (a small, reviewed, legitimate growth for a real capability, not scope creep).
…elivery feat(hooks): devkit owns hook-runner delivery so no future repo needs a manual git add -f
Bumps package.json and rebuilds dist/ — the compiled counterpart of two source-only PRs that both landed without it: - #183 co-occurrence gate fixes: worktree-safe index refresh (never cold-builds, never writes through a linked worktree's index), repo-root-relative clone paths, the two run-as-main guards that made guard-clone and guard-dup-allowlist no-op through their bin shims, the partially-staged changed-file filter, and the cloneRoots / indexCommand config keys. - #185 devkit-owned hook-runner delivery (sync-hook-runner), so a linked worktree stops checking out with an unreachable core.hooksPath and silently ungated commits. Also re-freezes the size ratchet baseline after reviewers/bench.mts shrank (977 -> 884) during a gate run. Verified on this tree before tagging: lint, typecheck, and the full suite (1992 pass, 8 skipped) are green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
release: v0.39.0
`devkit upgrade` treated any missing RECOMMENDED gate as damage to heal: the non-TTY path appended it back to `components.guards` and re-emitted it into the husky block. So a gate you deliberately removed reappeared on the next upgrade, with no way to express the refusal. Found in frink, where it caused a real cost bug. Frink deliberately hand-places `decisions` and `review` AFTER its free deterministic gates — the hook says so: > *Moved here from the devkit-guards block: it can spawn a judge, so it belongs after the free gates.* > *`.devkit/config.json` no longer lists "decisions" in components.guards, so `devkit init/update` won't re-inject it.* That second line was no longer true. `decisions` was back in the config, so **both** copies ran: one LLM judge inside the managed block and another at the bottom of the hook, on every single commit. The config claimed one thing while the hook did another. ## Change The recorded selection is authoritative. `newBundledGates` stays report-only, and the non-TTY branch prints instead of mutating: ``` • devkit bundles qavis-advisory (recommended) — not in this repo's guards; enable with 'devkit init --guards …,qavis-advisory' ``` The wizard path is unchanged (recommended gates still pre-checked — an interactive answer is a real answer). Falling behind a genuinely-new gate is now a loud notice rather than a silent edit to a file the consumer owns. I first built this as a `guardsDeclined` opt-out list, then threw it away: it needed config plumbing across several write sites to express something the existing `guards` array already says. Less code, and one fewer concept. ## Tests `upgrade-gate-offer.test.mts` rewritten to pin the new contract, plus a new regression test — remove a recommended gate, upgrade **twice**, assert it stays gone from both the config and the hook. That is the exact loop that silently undid frink's intent. Full suite: 1993 pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(upgrade): stop silently re-adding gates the consumer removed
Every gate run leaves `.devkit/prefix-cache.json.generation` and `.devkit/review-cache.json.generation` behind. The base cache files are ignored; the `.generation` sidecars were not — so they sat untracked forever, and were enough to make `devkit release` refuse with "working tree not clean" in the repo that ships devkit itself. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
chore: gitignore the .devkit/*.generation cache sidecars
release: v0.40.0
…ged block A repo that hand-rolled its gates before devkit absorbed them keeps the old invocations below the managed block. Nothing notices, so every commit runs those gates **twice** — for `guard-decisions` and `guard-review` that is a second model bill on every commit — while `.devkit/config.json` still describes one run. Found in frink, whose `.husky/pre-commit` calls `guard-decisions detect` and `guard-review --gate` a second time below the block. ## Report-only, and NOT `fixable` devkit did not write those lines, so it must not delete them unasked — a stray call is usually legacy but can be intentional (different flags, a different position in the run order, a repo-specific wrapper). Marked `fixable: false`, and `applyFix`'s husky branch now honours that flag: it previously re-ran `devkit init` for ANY non-OK hook result, which regenerates the managed block and cannot touch a line outside it — so `--fix` would have re-inited forever without clearing the warning. Also wired into the **self-host** doctor path, which never calls `checkHusky` — without that the check was unreachable in exactly the repo that dogfoods devkit. ## Precision A bin name appears in a hook far more often than it is *run*, so the classifier is where the work is. Ten misfire modes, all handled — a few from testing against frink, most caught by this repo's own reviewer gate across eight attempts: - **subcommand-aware.** `guard-decisions check-alignment` has no devkit fragment — flagging it would advise deleting your only invocation. - **bare-bin match limited to orchestrated gates.** `guard-deterministic` runs size/fanout/dup/clone/coverage on the block's behalf, so a stray `guard-dup scan` needs a bare-bin match; applied to every bin it flagged `guard-review transcript`, a different command. - **self-host-aware.** `toSelfHost` rewrites `bunx guard-review` to `node gate-engine/review/cli.mts`; bin-name matching alone left `blockSignatures` empty. - **monorepo-aware.** One block per package; every marker range is managed territory. - **quoted or commented mentions ignored** — leading and non-leading `echo`/`printf`, plus inline `#`, via quote-state tracking. - **existence probes ignored** (`command -v` / `which` / `type` / `hash`). - **every OCCURRENCE and every gate per line.** - **whole-word only** — `guard-dup-allowlist` is never `guard-dup`. Ambiguity resolves toward NOT reporting: advisory output, so a missed duplicate is cheaper than telling someone to delete a working line. ## Layout Both new modules exist because the gates demanded a split, not by preference: `stray-gate-calls.mts` (hook-checks.mts was at its line cap) and `lib/doctor/self-host-doctor.mts` (doctor.mts at its cap; `cli/commands` then hit the 12-file fan-out cap, so it landed beside the other doctor internals). Verified against frink's committed hook (2 real, 0 false) and this repo's self-hosted hook via a live `devkit doctor` run (0, correctly). 18 tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(doctor): warn when a devkit gate is also invoked outside the managed block
External codex tooling writes `.codex/agents/*.toml` and `.agents/skills/**` — projections of devkit's own `agents/` and `skills/`. Left untracked-but-not-ignored they block `devkit release` ("working tree not clean").
Ignored rather than committed, deliberately — and this is the asymmetry with `.claude/` and `.cursor/`, which ARE tracked here:
- `.claude/` and `.cursor/` are emitted by devkit's own `sync-agents`/`sync-skills` and drift-checked by `devkit doctor` against a manifest.
- Nothing regenerates or verifies `.codex/`: `AGENT_TARGETS` is `['claude','cursor']`. Committing 43 files devkit cannot re-emit would guarantee they go silently stale the first time an agent or skill changes — the exact drift devkit exists to prevent.
The files stay on disk for local use. Track them once `codex` is a real `AGENT_TARGETS` entry, so devkit emits and doctor checks them like the other two.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
chore: gitignore the .codex/.agents projections
release: v0.41.0
## What - install a dedicated Claude/Cursor edit-denial hook whenever the decisions guard is selected - make decisions skill synchronization depend on that guard and prune stale Devkit-owned copies - add guarded amend support for the newest uncommitted Target or note - isolate hook lifecycle and decision amendment responsibilities into size-compliant modules - record the architecture and cover hook, lifecycle, skill, amendment, and packaged E2E behavior ## Why Decision records are intended to be authored through guard-decisions. Native agent write tools could previously bypass that workflow, while the decisions skill could be installed without its enforcing guard. ## Impact Projects selecting the decisions guard receive native edit protection for their configured decisions directory. Shell commands and unknown payloads remain outside enforcement/fail open as documented. Projects without the guard no longer receive the Devkit-owned decisions skill. ## Checks - focused lifecycle/amendment suites: 141 passed - gate-engine unit suite: 1,102 passed, 5 skipped - changed upgrade/clean suites: 15 passed - package E2E: 12 passed - bun run typecheck - bun run build - bun run lint (passes with 3 pre-existing warnings) - guard-size gate - guard-fanout gate - guard-dup scan --new --changed (0 candidates) - guard-clone (0 clones) - commit-guard duplicate review
## What changed - Generate the existing completeness commit-message guard for devkit self-hosting in source mode. - Verify and repair that hook through self-host doctor. - Upgrade devkit's self-hosted configuration and generated assets to v0.41.0. ## Why Self-host selected the review guard but intentionally removed its commit-message hook, leaving completeness unenforced for devkit's own commits. ## Validation - Feature critique before implementation - Focused self-host and init tests (51 passing) - TypeScript typecheck, size ratchet, Biome, and whitespace diff check - Duplication review (semantic index unavailable; configured fail-open) - Full `bun run test:run` was attempted earlier but hung in a Vitest worker after the relevant tests passed
Contributor
|
Warning Review limit reached
Next review available in: 1 minute 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 Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change updates decision workflow documentation and configuration, adds a commit-message completeness gate, extends self-host hook generation and doctor checks, and adds tests verifying generated hook content and parity. ChangesDevkit review and decision flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Init as applyInit
participant SelfHost as installSelfHostHooks
participant CommitMsg as installSelfHostCommitMsgHook
participant Hook as .husky/commit-msg
participant Doctor as self-host-doctor
Init->>SelfHost: install combined self-host hooks
SelfHost->>CommitMsg: install rewritten commit-message hook
CommitMsg->>Hook: write or update completeness guard
Doctor->>Hook: compare generated and installed guard
Doctor->>Hook: regenerate when --fix is enabled
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 |
![coderabbitai[bot]](https://avatars.github.com/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 @.claude/skills/decisions/SKILL.md:
- Line 23: Update Capture A examples in both
.claude/skills/decisions/SKILL.md:23-23 and
.cursor/skills/decisions/SKILL.md:23-23 to consistently use the renamed
guard-decisions command for query, show, and add, removing old decisions or
bare-command forms.
- Around line 138-142: Update the decision protection guidance at the documented
native-edit guard in both .claude/skills/decisions/SKILL.md lines 138-142 and
.cursor/skills/decisions/SKILL.md lines 138-142 to close the shell-command
mutation and deletion bypass: route shell writes through the same enforcement
boundary, or require explicit human confirmation before allowing them. Preserve
the existing append-only and fail-closed behavior for protected decision
records.
In `@cli/lib/husky/commit-msg-block.mts`:
- Around line 183-184: Update the hook refresh logic in
cli/lib/husky/commit-msg-block.mts around writeFileSync and chmodSync so
existing commit-msg hooks are also restored to executable mode after rewriting.
In cli/lib/doctor/self-host-doctor.mts, extend the commit-msg hook health check
to validate executable permissions as well as content, and mark the hook healthy
only when both checks pass.
🪄 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 Plus
Run ID: 14b2110f-ea4f-4b67-974b-7208cce653d6
📒 Files selected for processing (16)
.claude/skills/brainstorming/SKILL.md.claude/skills/decisions/SKILL.md.cursor/skills/brainstorming/SKILL.md.cursor/skills/decisions/SKILL.md.devkit/agents-manifest.json.devkit/config.json.devkit/skills-manifest.json.husky/commit-msgcli/__tests__/apply-init.test.mtscli/__tests__/self-host.test.mtscli/commands/init.mtscli/lib/doctor/self-host-doctor.mtscli/lib/husky/commit-msg-block.mtscli/lib/husky/self-host.mtsdocs/decisions/review-gate-in-chain.mdeslint/baselines/size-lines.json
| note. Regenerable; holds no history. | ||
|
|
||
| CLI: `scripts/decisions/decisions.mjs` (`add --target` · `add --note` · `query` · `list` · `show` · `check` · `reindex`). | ||
| CLI: `guard-decisions` (`add --target` · `add --note` · `amend --target` · `amend --note` · `query` · `list` · `show` · `check` · `reindex`). |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use the renamed CLI consistently across both decision skills. The interface declaration now names guard-decisions, but Capture A still contains the old decisions/bare-command forms.
.claude/skills/decisions/SKILL.md#L23-L23: change the later Capture A examples toguard-decisions query/show/add..cursor/skills/decisions/SKILL.md#L23-L23: change the later Capture A examples toguard-decisions query/show/add.
🧰 Tools
🪛 SkillSpector (2.3.11)
[warning] 103: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
📍 Affects 2 files
.claude/skills/decisions/SKILL.md#L23-L23(this comment).cursor/skills/decisions/SKILL.md#L23-L23
🤖 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/skills/decisions/SKILL.md at line 23, Update Capture A examples in
both .claude/skills/decisions/SKILL.md:23-23 and
.cursor/skills/decisions/SKILL.md:23-23 to consistently use the renamed
guard-decisions command for query, show, and add, removing old decisions or
bare-command forms.
Comment on lines
+138
to
+142
| Use `amend <slug> --target …` or `amend <slug> --note "…"` for that narrow case: only the newest | ||
| entry absent from `HEAD` can be replaced, and the CLI refuses if committed or earlier working-tree | ||
| history changed. While the decisions guard is selected, direct native agent edits are blocked only | ||
| under the configured decisions directory; shell commands and unsupported payloads remain outside | ||
| this enforcement and fail open. |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Close the shell-command bypass of append-only decision protection. The documented native-edit guard does not protect the decision record if an agent switches to shell-based file mutation.
.claude/skills/decisions/SKILL.md#L138-L142: route shell writes through the same enforcement boundary or require explicit human confirmation before shell mutation/deletion..cursor/skills/decisions/SKILL.md#L138-L142: apply the same enforcement or confirmation policy.
🧰 Tools
🪛 SkillSpector (2.3.11)
[warning] 103: [EA2] Autonomous Decision Making: Skill enables autonomous high-impact decisions without human-in-the-loop verification. Critical operations (destructive commands, financial transactions, data deletion) should require explicit user confirmation.
Remediation: Add human-in-the-loop confirmation for destructive, irreversible, or high-impact operations. Never auto-execute commands that modify files, send data, or alter system state.
(Excessive Agency (EA2))
📍 Affects 2 files
.claude/skills/decisions/SKILL.md#L138-L142(this comment).cursor/skills/decisions/SKILL.md#L138-L142
🤖 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/skills/decisions/SKILL.md around lines 138 - 142, Update the
decision protection guidance at the documented native-edit guard in both
.claude/skills/decisions/SKILL.md lines 138-142 and
.cursor/skills/decisions/SKILL.md lines 138-142 to close the shell-command
mutation and deletion bypass: route shell writes through the same enforcement
boundary, or require explicit human confirmation before allowing them. Preserve
the existing append-only and fail-closed behavior for protected decision
records.
Comment on lines
+183
to
184
| writeFileSync(hookPath, rewrite(buildCommitMsgHook(selection, pkgRel, { standalone }))); | ||
| chmodSync(hookPath, 0o755); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Repair and verify commit-msg hooks as executable files, not just matching text.
cli/lib/husky/commit-msg-block.mts#L183-L184: chmod the hook after existing-hook refreshes as well as creation.cli/lib/doctor/self-host-doctor.mts#L71-L89: include executable-mode validation and only mark the hook healthy after both content and mode are correct.
📍 Affects 2 files
cli/lib/husky/commit-msg-block.mts#L183-L184(this comment)cli/lib/doctor/self-host-doctor.mts#L71-L89
🤖 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 `@cli/lib/husky/commit-msg-block.mts` around lines 183 - 184, Update the hook
refresh logic in cli/lib/husky/commit-msg-block.mts around writeFileSync and
chmodSync so existing commit-msg hooks are also restored to executable mode
after rewriting. In cli/lib/doctor/self-host-doctor.mts, extend the commit-msg
hook health check to validate executable permissions as well as content, and
mark the hook healthy only when both checks pass.
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.
What changed
Why
Self-host selected the review guard but intentionally removed its commit-message hook, leaving completeness unenforced for devkit's own commits.
Validation
bun run test:runwas attempted earlier but hung in a Vitest worker after the relevant tests passedSummary by CodeRabbit
New Features
Documentation
Bug Fixes