feat(overengineering): add the delta lane — recurring audits that report only what moved (0.2.0) - #3146
Conversation
Adds `overengineering:delta`, the recurring lane the findings artifact's stable spine was designed for: capture the prior spine, re-run `overengineering:audit`, compare the two, and report only what moved. The load-bearing mechanic is the ordering. The findings artifact is rewritten in place per layer as the audit walks, so there is no previous artifact left to diff against once the audit has run. The lane captures the prior spine to a memory-tier `spine-baseline.md` BEFORE invoking the audit, and the skill states why a maintainer who reverses those two steps silently turns every cycle into a first run. Read-only always: it never invokes or enters `overengineering:realign` and never writes a Status. Verdict changes queue for the human, in the report always and through a presence-gated tracker route with a named inline fallback. Recurring wiring is documented consumer-agnostically in the lane doc — four shapes with their trades — and adopted nowhere. No schedule file ships with the plugin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQGLRXTnzrQFQ1a1ZanojJ
…(0.2.0) (#2898) - `reference/consumer-config.md`: new `delta_noise_budget` key group, seven keys with types and defaults, in the ordinary refinement cascade class with the classification justified — no key can remove a finding, change a verdict, suppress a judgment, or weaken the protected cap, so none carries the hazard that puts `protected_categories` and `suppressions` in policy-floor. Two delta classes are deliberately not keys at all. - `context/findings-artifact.md`: additive spine-capture obligation naming the capture-before-audit ordering and the `spine-baseline.md` sibling. `schema` stays 1; no merge rule changed. The forward reference to "a future delta lane" now names the shipped one. - `plugin.json` 0.1.1 -> 0.2.0 (new skill = minor), CHANGELOG entry, README skill row plus a cadence section pointing at the lane doc. - `skills/delta/evals/evals.json` matching the sibling skills' shape. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQGLRXTnzrQFQ1a1ZanojJ
… docs (#2898) - The `spine-baseline.md` frontmatter and body rules now live once, in `context/findings-artifact.md`; the skill points at them instead of restating them (pointer-not-copy). - Precompute block trimmed to one line. A git command plus more than one injection line is refused outright in a worktree-isolated agent (#1619), which is exactly the dispatched context a scheduled run arrives in; the gate flagged the two-line form. UTC stamps are read at write time. - `docs/CATALOG.md` and `docs/SKILL-CHEAT-SHEET.md` regenerated with the repo's own generators (`validate-plugins.sh` runs both in --check mode). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQGLRXTnzrQFQ1a1ZanojJ
…nding topic-docs.md listed only findings.md and asserted "Nothing else is written", which the delta lane's spine baseline falsifies. Add the baseline row (writer, type, location), point at findings-artifact.md's spine-capture obligation for its shape rather than restating it, and reconcile the stable-filename and ephemerality paragraphs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQGLRXTnzrQFQ1a1ZanojJ
…icate the class spec The delta lane documented a `queue_route` key it never read: `consumer-config.md` defines `inline` as declining the tracker route unconditionally, but the skill's route table had one row (tracker reachable -> maintain a queue item), so an operator who set `inline` to stop the lane opening tracker items got them anyway. The route table now branches on the key; the queue still always appears in the report and the route decision and presence answer are still recorded. Also: the volume cap now cites the artifact's stable total order as its intra-class tiebreak, so two runs over identical input truncate to the same head; the "not configurable" justification lives only in `consumer-config.md`, with the skill keeping the always-listed rule and pointing there for why; the skill stops restating numeric defaults it attributes to that file; the obligations table gains a `delta` column; and two over-broad sentences about placement are made accurate. Additive only in `findings-artifact.md`: `schema:` and every merge rule unchanged.
|
Claude finished @kyle-sexton's task in 1m 37s —— View job Security review (
|
|
Last security-reviewed head: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be498981c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…st-audit spine The status-change delta class could never fire. The lane captured its baseline from the findings artifact as it stood at the start of a cycle, but a Status changes when a human runs realign BETWEEN cycles and realign writes it into that same artifact — so the new status was already in the baseline, the audit carried it forward, and both sides of the comparison agreed for every pre-existing finding. The baseline is now the previous cycle's post-audit spine, captured at the end of a cycle. A pre-audit capture survives only as an explicitly named bootstrap, which cannot observe a status change and says so. Also: refuse to treat detached HEAD as a branch identity (a scheduled runner's ordinary shape, where rev-parse --abbrev-ref answers the literal HEAD and the branch-match guard would accept another ref's spine), and make the durable queue route opt-in, since work-items:track refuses to file on inferred intent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQGLRXTnzrQFQ1a1ZanojJ
|
Claude finished @kyle-sexton's task in 1m 6s —— View job Security review (
|
README's skill row and the CHANGELOG's lead delta bullet still described the lane as capturing the prior spine and then re-auditing. Also tighten the run's end-of-cycle capture step: a no-baseline cycle still establishes one, while a cycle that stopped short or never resolved a branch identity writes nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RQGLRXTnzrQFQ1a1ZanojJ
|
Claude finished @kyle-sexton's task in 46s —— View job Security review (
|
|
Claude finished @kyle-sexton's task in 57s —— View job Security review (
|
|
Claude finished @kyle-sexton's task in 2m 1s —— View job Code review (
|
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
Resolves an overengineering version collision: main shipped the delta lane as 0.2.0 (#3146) while this branch had also claimed 0.2.0. Main's entry stays at 0.2.0 and this branch's documentation-only entries move to 0.2.1, taking main's plugin description (which now names the third lane). The delta lane also shipped as its OWN skill composing audit, rather than as an argument selecting a mode inside it, which is independent corroboration of the decision ADR 0017 records. The ADR now cites it, and its context section is corrected: #2897 said "a third skill" when the plugin had two, but delta has taken that slot, so the product-code lane is the fourth skill and third lane. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RiAekqvuMHJs3CjusBbNWH
Second overengineering version collision in a row: main took 0.2.1 for the findings-artifact date-rationale fix (#3167) after taking 0.2.0 for the delta lane (#3146). Main's entries stay where they are and this branch's documentation-only entry moves to 0.2.2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RiAekqvuMHJs3CjusBbNWH
…y (0.3.0) (#3222) ## Summary `overengineering:audit` and `overengineering:realign` precomputed the current branch with `git rev-parse --abbrev-ref HEAD`, which answers the literal string `HEAD` when HEAD is detached — the ordinary shape for a scheduled CI runner. `HEAD` looks like a branch name, is the same string for every ref, and compares equal to itself, so three consequences followed and all of them were silent: - `audit` wrote `branch: HEAD` into the findings artifact, giving it an identity that is not a branch. - `realign`'s branch-match refusal compared `HEAD` to `HEAD`, passed by construction, and could accept another ref's artifact as this ref's own — in the only skill in this plugin that mutates anything, where per-item human consent is spent down a rollback ladder as far as deletion. - The `<branch-slug>` memory-tier home key resolved every detached ref to one directory, so unrelated runs shared a single artifact and merged each other's carried-forward operator judgments. The `delta` lane already resolved identity correctly with `git symbolic-ref` and could not fix any of these from where it sits, which is why this was filed separately from #3146. ## Fix Both precomputes now use `git symbolic-ref --quiet --short HEAD`, which fails rather than inventing a name, and emit the same sentinel string `delta` already used. All three skills now agree on one resolution contract: prefer a logical ref where the environment supplies one naming a branch — with no CI vendor's variables named or assumed — and otherwise treat the identity as unresolved. On an unresolved identity: - **`audit` persists no findings artifact at all.** Not the file with `branch:` omitted, not a placeholder value, not a file at a substitute home. The walk still runs and the inline summary is still emitted, so the operator still gets the findings; only the persisted write is declined, and the read-only disclosure line says that instead of naming a path. - **`realign` refuses**, in both unresolved states: when its own checkout has no identity (it stops before reading the artifact at all) and when the artifact it finds carries `branch:` absent, empty, or `HEAD`. Neither path reaches the comparison. The refusal is a positive precondition rather than a consequence of the file being missing, which also renders any stale pre-fix artifact already on disk unreachable without a cleanup step. **Identity resolution is a stated body step, not only a precompute.** A worktree-isolated or dispatched executor may decline to inject the precomputed context block entirely — and that is the same `unattended` context in which a detached checkout is most likely — so both skills state the resolution command in the step that uses it and treat the precompute as a convenience that may be absent. Confined to the precompute line, the fix would have verified green on an attached local checkout and done nothing in the environment the defect actually lives in. Two contract documents now state the case rather than leaving it to be inferred. `context/findings-artifact.md` documents `branch:` as resolved with `git symbolic-ref` and never the literal `HEAD`, adds a "No branch identity, no artifact" section, and carries a per-skill obligations row. `reference/topic-docs.md` records that an unresolved identity keys no home and does not run the rung order, rejecting each substitute on its own terms: `HEAD` is one directory for every ref, a commit sha is a fresh home every commit that never resumes, and a fixed literal such as `detached` is `HEAD` under another name. Three adjacent surfaces were made inconsistent by the change and are corrected in the same PR. `skills/audit/context/report-template.md`, which owns the read-only disclosure line, still declared the artifact written "always" and hardcoded the path-naming form. `skills/delta/SKILL.md` closed its own detached section with "the audit still runs, exactly as it otherwise would" — true when written, false once the audit began declining its write — and its step 4 would have looked for a post-run artifact that no longer exists; `delta`'s behavior is unchanged, only its account of the audit's. And `skills/delta/context/recurring-wiring.md`, the wiring document read by exactly the affected population, now names the detached case and the logical-ref remedy. **The `branch:` decision was made by an independent panel, not by the implementing agent**, and is recorded in full on the issue with its reasoning, its rejected alternatives, and its caveats: [#3149 (comment)](#3149 (comment)). Two fresh-context agents on different models, spawned separately with the implementing agent's rationale withheld, independently reached the same answer as that agent's own prior read. ## Verification **The behavior the fix turns on, observed directly.** The marketplace's own primary clone happens to sit detached, so both forms were run against a real detached checkout and against an attached one: | Checkout | `git rev-parse --abbrev-ref HEAD` | `git symbolic-ref --quiet --short HEAD` | |---|---|---| | Detached (`## HEAD (no branch)`) | `HEAD` | *(no output, exit 1)* | | Attached (`fix/3149-…`) | `fix/3149-…` | `fix/3149-…` | The two forms agree exactly where a branch exists and diverge exactly where one does not, which is the whole of the guard. `git branch --show-current` was checked on the same detached checkout and returns an empty string at exit 0 — the basis for the cross-plugin finding below. **Repo gates.** This plugin ships no scripts and no `*.test.sh`, so its coverage mechanism is the eval sets plus the shared static gates. All were re-run after the review fixes: - `check-skill.sh` — `audit`, `realign`, and `delta` each PASS with 0 errors. The warnings each reports (description length, the 200-line soft target) are pre-existing on `main`. - `check-evals-quality.sh` — PASS, 0 warnings across both eval sets. - `check-jsonschema` against `plugins/skill-quality/reference/evals.schema.json` — valid. - `check-skill-portability.sh` — no unexcused coupling tokens. - `markdownlint-cli2` — 0 issues across all eight changed markdown files. **Eval coverage for the detached path, which existed for neither skill before.** Four cases added: `audit` declining to persist on a detached runner; `audit` preferring an environment-supplied logical ref and naming its origin; `realign` stopping before it reads the artifact when its own checkout is detached; and `realign` refusing an artifact carrying `branch: HEAD`. Each prompt narrates the checkout state it depends on, following the convention the existing shallow-clone case already uses, so none asserts a condition it never establishes. **Independent review.** A fresh-context reviewer with the author's rationale withheld checked the change against the issue's five acceptance criteria and returned NOT READY on the first commit. Its blocker (the stale `report-template.md`) and every should-fix (the cross-plugin count, the `mutation-testing` overclaim, the `babysit-prs` characterization, three under-specified eval prompts, the missing wiring guidance) are addressed in the second commit. The version moved to 0.3.0 rather than a patch on its recommendation: a run that previously wrote a file now writes none. ## Related Closes #3149 - #3146 — the `delta` lane PR where this was found; carries the delta-side defense this change converges `audit` and `realign` onto. **Do other plugins share the pattern?** No other plugin exhibits the `HEAD` identity-collapse. The repo-wide precompute boilerplate uses `git branch --show-current`, which returns an empty string on a detached checkout rather than `HEAD`. The literal `git rev-parse --abbrev-ref HEAD` form survives at seven occurrences across six files outside this plugin: `claude-ops`' two skill-usage telemetry hooks, `source-control`'s `babysit-prs` parking-branch capture and `worktree` current-branch display, and three test-scaffolding sites in `guardrails`' `stale-path-verify.test.sh` and `source-control`'s `worktree-create.test.sh`. None compares the value against a stored identity to decide whether a mutation may proceed. `babysit-prs` is the closest call and is still a different defect class — its captured value parameterizes a same-session `git checkout`, so a detached capture fails to restore the starting commit rather than authorizing another ref's work. All are left as-is deliberately. This corrects the triage note, which recorded two of these sites rather than six. **One neighbor has the same shape under a different string, and is out of scope.** `mutation-testing:audit` documents `branch:` as `git branch --show-current` verbatim (`skills/audit/context/persist-findings.md:202`) and ships no script guarding the empty result, so a detached run there writes an empty identity that compares equal to itself. Three sibling findings-writers (`ai-slop`, `docs-hygiene:audit-noise`, `claude-config:audit-instructions`) already guard that case and exit rather than persist. Recorded rather than fixed — this issue is scoped to one plugin — and worth its own item. **Adjacent pre-existing condition, deliberately not fixed here.** `scripts/check-skill-precompute-compose.sh` reports both `audit/SKILL.md` and `realign/SKILL.md` as carrying a git command plus more than one injection line (#1619). Both files already had two precompute lines on `main`, so this change neither introduces nor worsens it, and the gate is warn-only in CI pending that remediation wave. Making resolution a body step is what keeps this fix correct in the meantime, since a refused precompute block no longer means an unguarded identity. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Closes #2898
Summary
Adds
overengineering:delta, the recurring lane the findings artifact's stable spine was explicitly designed for.context/findings-artifact.mdalready said so in as many words — "A future delta lane inherits this as its contract" — and this PR is that consumer.The lane re-runs
overengineering:auditand reports only what moved since the last run: new clutter, verdict moves, closures, status changes, filtered through a configurable noise budget. It is read-only always, never invokes or entersrealign, and queues verdict changes for a human instead of acting on them. The plugin adopts no cadence and ships no schedule file — a plugin that scheduled itself on install would be an unratified standing commitment in someone else's repo, which is the exact class of thing this plugin exists to find and retire.Plugin
0.1.1→0.2.0.Fix
The load-bearing mechanic is an ordering, and it is not the obvious one. The findings artifact is rewritten in place, per layer, as the audit walks. So the lane cannot be "run the audit, then diff" — by the time the audit returns there is nothing left to diff against. It captures the prior spine to a memory-tier
spine-baseline.mdsibling before invoking the audit. Reversing those two steps does not error; it reports "no baseline" every cycle forever, and that failure is invisible from the report. The skill states the ordering as a contract for exactly that reason.skills/delta/SKILL.md— the lane. Composesauditvia the Skill tool, passing layer scope andunattendedthrough verbatim; free-text hints are deliberately not forwarded (they would make the walk incomparable with the baseline) and are reported as declined.## Closed since last runcarries a reason class a spine diff cannot produce; merge rule 5 already flags a verdict that moved under a carried-forward judgment. A second derivation is a second answer that can disagree with the first.REALIGNEDexactly, anunproven_headoff the audit's own carry-cost ranking), a volume cap with a stated rank order, and a one-line quiet cycle. Two classes — a rule-5 judgment flag and a status change — are not configurable at all, because a key that could hide them would hide the operator's own concern from them.reference/consumer-config.md—delta_noise_budget, 7 keys, refinement class with the classification argued rather than asserted. NouserConfig; the plugin's recorded trade stands.context/findings-artifact.md— additive only: a new spine-capture obligation section and adeltacolumn on the obligations table.schema:stays1; no merge rule altered.reference/topic-docs.md— the placement binding said "Nothing else is written", which the baseline falsifies. That file is whatauditstep 1 tells every run to consult, so a reader trusting it could delete a baseline as stray and silently destroy the lane's only comparison point. Corrected, with the hazard named.skills/delta/context/recurring-wiring.md— four consumer-agnostic wiring shapes with the trade each makes; none is a default.Verification
Every gate below was run by the orchestrator against the worktree, not taken from a worker's claim. All exit 0:
validate-plugins.shvalidate-plugin-contracts.mjsgenerate-catalog.mjs --checkgenerate-cheatsheet.mjs --checkcheck-changelog-parity.sh --check-bump origin/maincheck-skill-precompute-compose.sh --strictcheck-skill-portability.shcheck-skill-leaf-names.sh/check-skill-count-claims.shAn independent fresh-context verifier checked all acceptance criteria against the diff with the authoring rationale withheld, resolved every relative link on disk, verified every
§Nreference against real headings inscrutiny-method.md, and confirmed no fabricated invocations. It returned PASS — and found two real defects, both fixed here before this PR opened:queue_routewas an orphan key. It was documented inconsumer-config.mdbut read nowhere, so an operator settingqueue_route: inlineto stop the lane filing tracker items would have been silently overridden and got them anyway. For a lane whose entire justification is not becoming nagging automation, that was the worst bug available. The skill now reads the key before probing for a tracker, with a three-row route table.Test plan
/overengineering:deltaon a branch with no prior artifact → reports "No baseline; this run establishes one", runs the audit, reports nothing as a delta, does not restate the surface.ci-lanes unattended) → unwalked layers appear once as a coverage line with counts, never as findings or closures.queue_route: inlinein.claude/overengineering.md→ no tracker probe, route declined naming the setting, queue still present in the report.Related
[Maintenance]recurring items, as precedent for the tracker-item wiring shape the lane's doc argues for.branch: HEAD, collapsing every ref to one identity #3149 —overengineering/audit: a detached checkout writesbranch: HEAD, collapsing every ref to one identity. Filed from the review round below; the same root cause as the detached-HEAD finding, but inauditandrealign, which are outside this PR. The delta lane is defended on its own side here.Review round
An automated cross-vendor reviewer filed three findings on
be498981, all verified against the files and all real. Fixed in7dc00c8a:realignwrites a status between cycles, so the pre-audit capture already contained it and the audit carried the same value forward: both sides of the comparison agreed, and the one case the class existed for was the one it could not see. The baseline is now the previous cycle's post-audit spine, captured at end of cycle. The original "a spine must be persisted separately" reasoning was still sound — the artifact is rewritten in place per layer — but the capture belonged at the end of the cycle, not the start. Pre-audit capture survives only as a named bootstrap, which reports that a status change is unobservable on that first cycle.git rev-parse --abbrev-ref HEADreturns literalHEADwhen detached, so the branch guard comparedHEADtoHEADand passed — on exactly the scheduled runners this lane targets. Nowgit symbolic-ref --quiet --short HEAD, which fails closed; the lane takes no baseline, no comparison, and no capture when identity is unresolved. Root cause also affectsauditandrealign→ overengineering/audit: a detached checkout writesbranch: HEAD, collapsing every ref to one identity #3149.work-items:track's gate is explicit that "model-initiated filing is not" authorization, so the defaultqueue_route: autodurable route was unreachable in the unattended mode it existed for.queue_routenow defaults toinline; the durable route is opt-in, and the operator setting it is the recorded authorization. The report-side queue is unchanged and still unconditional.The earlier CHANGELOG imprecision noted in review is resolved — fix 3 made that clause actually wrong rather than merely incomplete, so the
## [0.2.0]entry was amended in place.One follow-up left for an operator, not taken here
No row was added to
.github/recurring-schedule.json. Building the lane and adopting a cadence for this repo are different decisions; registering it would create a live recurring commitment nobody ratified. The wiring doc argues shape 4 (a recurring tracker item) as the one to prefer if this repo adopts it.Generated by Claude Code
Generated by Claude Code