feat(claude-config): add audit-pass, one coordinated resumable instruction-surface pass - #1318
Conversation
…ction-surface pass /claude-config:audit-pass runs one bounded, ordered, resumable pass over a named target repository's Claude Code instruction surface. It defines no criteria of its own: every check is delegated to the plugin that owns it through a presence-gated namespaced invocation with a documented fallback, and nothing crosses a plugin boundary but that invocation. What it adds is the run semantics the delegated skills cannot supply individually: - a three-scope inventory before any check runs (managed policy read-only, user scope routed as recommendations, project scope the only editable one) - an exclusion set DERIVED at run time — the target's own shared-source registry when it documents one, the vendor/ layout rule, `git worktree list` plus gitignore-awareness, and the pass's own artifacts — never transcribed, and no count of any class carried in the skill - content-derived finding identity, stable across runs, machines, and separators - a finding_id-keyed suppression record with staleness reporting - per-lane incremental persistence and resume - one human gate per run Read-only on bare invocation; mutation only behind --fix, and never to managed policy or a user-scope file. /doctor is an operator handoff rather than a dispatch, because it proposes fixes only after the operator confirms. Findings report in three tiers — derived (exact equality across runs), judged (a stability tolerance whose violation fails the run's self-check and is reported as an instability finding against the skill itself), delegated (no property) — and every run reports in one line how many OPINION-tier checks were available, were not run, and the argument that enables them. Also lands docs/conventions/finding-suppression as the owner doc for the finding_id-keyed record (keys, required reason and date, per-key merge rather than a closed list, policy-floor precedence inversion), with its Convention registry row and its config-cascade Implementers row. claude-config's plugin-level setup skill gains the consumer-project configuration surface that record introduces — the philosophy fixes setup at one skill per plugin, so this extends the existing one rather than adding a per-skill setup. Platform claims re-verified against current official docs 2026-07-24: output-style system-prompt behavior, keep-coding-instructions, and force-for-plugin; the /doctor v2.1.206 trim floor, its confirm-before-apply behavior, and the v2.1.205 cutover; the InstructionsLoaded hook; and that @path imports do not reduce context. Two carried-in /doctor suppression channels (DISABLE_DOCTOR_COMMAND and a skillOverrides "doctor" entry) are absent from the current env-vars and settings pages, so the skill detects absence by probing rather than asserting either as the cause. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…arget Two wording cuts in the apply-verify and self-check paragraphs. No semantic change; clears the only remaining check-skill warning (202 -> 200). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 0s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c4485a43b
ℹ️ 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".
…1318 ship Task #21, walked against the gate's own body in MIGRATION-PLAYBOOK.md rather than against this plan's seven-word summary of it, and against the two PRs' shipped files rather than their descriptions. Recording a pass on a criterion nobody substantiated would have been the same read-the-incumbent's-body failure this effort keeps recording, applied to the gate itself. One criterion fails. Security-review criterion 4 forbids ../ reach-outs, and audit-pass carries three — SKILL.md:163, run-contract.md:104, and exclusion-set.md:72 — all pointing at docs/conventions/finding-suppression, which #1318 creates at the repository root. Each resolves in this repository and to nothing in an installed plugin. That is this plan's own horizontal-decoupling grounding finding landing on the artifact it was written to protect, and the playbook lists it first under the caveats that break a working in-repo skill. The convention document is the right home for the keys; the pointer is what has to change. One criterion cannot close. The plugin-acceptance security review still owes an adversarial injection fixture corpus and verification that the exclusion-set derivation is not itself an injection target. Its hook, MCP, telemetry, provenance, and PATH surfaces are all clean — audit-pass ships none of them — so the residual is entirely the runtime behavior the threat model covers. Five criteria pass with evidence. Repo-agnosticism is real rather than assumed: the exclusion set derives every class and names the empty case for a target with no registry, and there are zero hardcoded repo or machine identifiers. No userConfig is the correct answer rather than an omission, because the ownership table assigns by kind and audit-pass has no personal-or-administrator scalar. Two divergences from this branch's design are recorded as findings against the implementation lane. The shipped run-contract carries the superseded (surface, check, anchor, claim) tuple, which cannot express the pairwise finding D1 exists to produce, and has no liveness-basis concept at all — so Assertion 1.1 ships unscoped and P1 ships without its liveness clause. Neither is a gate criterion; both would make the shipped contract fail its own idempotence claim. The no-PII pass is today's, with the hazard named forward: when the liveness basis lands it must take the scope-prefixed form surface already takes, or a report that may be redirected into the target tree carries the operator's home path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… moved The proportionality pass downgraded the state key's repo-identity half and deferred per-lane input digests, but only the gate document said so. The re-run contract still presented both as settled, unqualified — which re-created the same two-documents-disagree defect the pass was closing. Fixed at the contract end, pointing at the gate's reasoning rather than restating it. Assertion 5.2 is now explicitly owed only under the per-lane form. Assertion 5.1 holds under both, because a tree-wide refuse-to-resume check closes the P1 hole just as completely; what per-lane digests buy is partial resume, and its value is unmeasured until Phase 10 has lane costs. Also records a cross-PR semver collision the acceptance-gate walk surfaced. #1316 and #1318 both take claude-config from 0.9.2 to 0.10.0. Each passes changelog-parity on its own branch; the pair does not, and the second to merge lands a version already claimed with a duplicate changelog heading. Invisible from either branch and surfacing only at the second merge, so it is recorded where both lanes can see it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The hard wrap put "#1318" at the start of a line, where markdownlint reads it as an ATX heading with no space after the hash (MD018). That failed hygiene, which failed ci-status, which blocks #1322. Reworded so both identifiers sit mid-line and cannot migrate to a wrap point on a later edit. Adding a space after the hash would have satisfied the linter by turning a PR reference into an actual heading, which is the worse repair. Swept the whole topic directory for the same shape rather than fixing only the reported line, since the linter reports the first failure and not all of them — this was the only instance. Verified with the same version and config CI runs: markdownlint-cli2 0.23.0 over all 11 files, 0 errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lar, and versioned The shipped identity was a four-part tuple over one surface and one anchor. That shape cannot name a cross-surface contradiction, which is the finding the pass exists to produce, and several of its parts had no escape hatch. Identity becomes `(check, claim, sites)` where `sites` is a canonically sorted SET of `(surface, anchor)` pairs. Sorted, because an ordered pair hashes X-vs-Y differently from Y-vs-X: one conflict reported twice, and no stable re-run. A conflict is ONE finding with two sites rather than two linked findings — SARIF reserves separate results for occurrences "corrected independently", and a contradiction is retired by fixing either side. `primary_site` and `related_site` are presentation and remediation fields outside the hash, so routing can change without renaming the finding. `surface` is now the canonicalized physical file, never the loading entry point; the importing file is a load edge carried in a non-identity `load_path` capped at five entries. The harness already emits that split — an `InstructionsLoaded` payload carries `file_path` beside `parent_file_path`, verified on 2.1.220. A symlink target escaping the target root takes the scope-prefixed logical form, or one shared rules file yields a different surface per consuming repo. `anchor` gains a granularity discriminator: `e:<digest>:<n>` for an excerpt, bare `s:` for a whole surface. A whole-surface identity reduces to `(surface, check, claim)` and is content-free, so its suppression survives edits to a dead file and dies on a rename. An `s:` anchor on a two-site finding is a hard error. Anchors carry their algorithm version in the field name (`anchor/v1`) and compare on the greatest common version, which is what makes every later algorithm change survivable. Normalization ships with these because it changes the hash: backticks are preserved, so quoted `@README` cannot hash identically to a real import, and block-level HTML comments outside code fences are stripped, because they are removed before the content ever reaches the model. The two divergences from GitHub's implementation are recorded together rather than apart, since they share one premise and one dissent: both adopt SARIF's decomposition and both choose the hash input for this corpus. Pairwise identity hashes both sides where GitHub keys on `locations[0]`; whole-surface identity is content-free where CodeQL hashes the file's first line. The suppression record now stores constituents — `check`, `claim`, every site — under the derived `finding_id`, with the constituents authoritative and a key that does not hash from its own body reported malformed. That is what makes tiered matching computable, and entry resolution becomes a four-row table in which only an exact match is silent: a one-sided anchor change carries the suppression forward marked `needs-reconfirmation` (never silently, because the edit may have BEEN the fix attempt); a deeper change closes the old entry as stale and opens the new finding unsuppressed; and a finding that disappears entirely must be accounted for as a fix, a successor, or an UNEXPLAINED DISAPPEARANCE that fails the run's self-check. That fourth row is the detector P2 never had. P2 said a finding vanishing without a fix is a defect, which was a definition with nothing able to observe it — a silently shrinking report read as an improvement. Tiered matching over a fingerprint that is stable enough rather than exact is prior art: SARIF devotes Appendix B (Normative) to the use of fingerprints by result management systems, and GitHub's documented mismatch behavior is close-and-reopen — "If the filepaths differ for the same result, each time there is a new analysis a new alert will be created, and the old one will be closed." Liveness enters the contract, which previously had no concept of it. A surface is live when the harness actually loads it, read from `InstructionsLoaded` for the memory layer and `/context` for skills, subagents, and MCP tools — either source alone under-covers the surface set silently, and under-coverage reads as a clean report. Assertion 1.1 is scoped to a fixed live surface set and P1 gains its liveness clause, because startup scope depends on the launch directory and on settings the tree does not contain, so two runs over a byte-identical tree can legitimately see different surfaces. A liveness difference is reported as the cause, never silently absorbed. `managed-settings.json`'s `claudeMd` key is observed by neither source and is recorded as a known gap, phrased as a limitation of the two sources rather than a claim about harness behavior. The skill no longer reaches outside its own plugin root. Three relative paths climbed to `docs/conventions/finding-suppression` at the repository root; they resolve in a checkout and resolve to nothing in an installed plugin cache, where a skill asking a runtime question would have got no answer. The operative rules the pass needs to run — record location, layer merge, precedence inversion, entry dispositions — now live in the plugin, and the repo doc is named rather than pathed. The convention remains the cross-consumer key contract and is no longer a runtime dependency of any plugin. The keys land in the convention's unreleased 1.0 rather than a 2.0, because neither the convention nor its first adopter exists on `origin/main` — there is no operator data to migrate, and retrofitting later would be a migration on somebody else's committed file. Version goes to 0.11.0 rather than 0.10.0. Two other branches in flight also claim 0.10.0; each passes changelog parity alone, and the collision is invisible from any single PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 4m 7s —— View job PR Review —
|
|
Claude finished @kyle-sexton's task in 3m 56s —— View job Security Review — PR #1318
No GitHub Actions changes are in this PR. No credentials, tokens, or secrets handling is introduced. No Four findings, in descending severity. IMPORTANT — Personal-layer suppression silently weakens team audit coverageConfidence: HIGH
The policy-floor inversion gives the team layer priority on conflicts for the same
Because the team's absence of an entry is not equivalent to "team has explicitly rejected this suppression", a developer can add entries to The Codex review (comment on line 126) identifies this same gap. It is not a design accident — the convention document consciously chose the "add/tighten-only" model — but it deserves an explicit statement of the threat it accepts: personal layers can suppress findings that the team has never reviewed. IMPORTANT — Apply-verify step sends repository diffs to an external AI vendor without a per-run consent gateConfidence: HIGH
Phase 5 specifies:
Under
In enterprise or regulated environments, this is a confidentiality risk: a developer running IMPORTANT — Prompt injection via target-controlled instruction surfacesConfidence: MEDIUM
The pass reads CLAUDE.md, This is a structural risk of any AI-based audit and not specific to this PR, but: (a) this PR introduces the first skill that reads and processes arbitrary third-party repository content at scale in this plugin set, and (b) no mitigation (fresh-context subagents per lane, explicit content demarcation, or a disclaimer to the operator) is specified. The confidence is medium because actual exploitability depends on the model's instruction-following hierarchy in the session. Relevant question for the author: do the delegated skills ( SUGGESTION — Target-controlled registry can exclude surfaces from audit scopeConfidence: MEDIUM
Class 1 exclusions (byte-identical cluster copies) are derived by reading The fallback when the registry is unreadable is correctly "treat the class as empty, report as coverage gap." The risk is the non-fallback path where the registry exists and is attacker-controlled. No finding
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77dd51e93a
ℹ️ 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".
… reference claims (#1393) Supersedes #1377 (which superseded #1315). Same work, rebased onto current `main` with five version collisions resolved, plus both review findings from #1377 closed and a pre-existing broken-reference failure fixed. No linked issue. ## Why the owner doc moves first Two plugin-level findings dissolved on inspection into owner-doc gaps rather than plugin defects, so the definitions are fixed at their definition site and the plugins then conform without edits. - **`userConfig` non-triviality had no definition**, leaving criterion (c) unfalsifiable. The line now follows from what Claude Code's native configuration prompt *is* — a collector, not a verifier. It stores what the consumer typed; it never confirms the path exists, the token authenticates, or two options agree. Resolves `education`, `repo-hygiene`, and `visualization` at once. - **The check-only carve-out was scoped to plugins whose entire configuration is `userConfig`**, excluding a shape the fleet actually ships: a plugin whose behavior is delivered through Claude Code settings this contract forbids setup to mutate. The carve-out now names its real condition — no writable owned artifact — and enumerates three qualifying surfaces. Silence is not the conforming response: `check` prints the exact edit, states that it is the operator's to apply, and names what re-invalidates it. Resolves `context-guard` and `rate-limit-guard` together. ## Two reference claims that were false - **`claude-memory`'s path-scoping status.** The reference asserted `.claude/rules/` files load unconditionally regardless of `paths:`. A first-party repro on 2.1.219 disproved it — a rule scoped `paths: ["**/*.tsx"]` was absent at session start, present after reading a matching file, absent again after a non-matching one. The cited evidence failed independently: two of four issues are closed NOT_PLANNED and never supported the claim; the two still open assert opposite failure modes. - **The `CLAUDE.md / AGENTS.md` compaction row.** Claude Code does not read `AGENTS.md` — [official docs](https://code.claude.com/docs/en/memory) say so, and it is absent from `/context`'s Memory Files enumeration even with a `CLAUDE.md` beside it. ## Two review findings from #1377, both real, both closed here They are the same shape: a documented fallback that reads as a safe default and is in fact a silent wrong answer. - **`machine-health`** told the caller to omit `-StateBase` when `${CLAUDE_PLUGIN_DATA}` does not expand. But a skill-invoked tool subprocess can inherit an **unrelated installed plugin's** `CLAUDE_PLUGIN_DATA` — the extensibility contract's own smoke tests document this — so the fallback can write this plugin's state, logs, and catalog overlay into another plugin's directory. It now passes the report root explicitly and reports the unresolved root. Colocating state with reports is wrong-but-visible; the inherited variable is wrong-and-silent, and only one of those is recoverable by a reader. - **`session-flow`** told a headless consumer to supply "every key to change" on reinstall. Uninstalling drops the stored `pluginConfigs` entry, so every omitted key reverts to its manifest default — reinstalling purely to enable the observer silently resets a customized analysis model, idle threshold, bare mode, and maximum lifetime. The rule is every key whose value should be **non-default**, and `check` must run **before** the uninstall, because afterwards there is nothing left to read the values from. ## A pre-existing gate failure this PR surfaced `machine-health`'s setup cited five of the audit skill's reference files by bare relative path, which resolve to nothing from the setup skill's own directory — three of them fail `skill-quality:check`. The gate only surfaced them because this PR touches the file. The correct form was already in the same document twice; the remaining five now match it. The files stay where they are, under the audit skill that owns them: approvals and the output schema are the audit's contracts, and copying them would create a second owner. ## Ranked fix 4 — decided, and the answer is "accept the drift" The path-shape question came out **both ways**. In place, registration is impossible: `SKILL.md` is in the drift checker's `skip_basenames` and the checker hashes whole files, never line ranges. After extraction it *would* work, since both skills are named `setup`. So registration is available only at the cost of creating the artifact in dispute. Accepted on merits: the block substantially restates rules two owner docs already carry, so a shared fragment would be a second owner for them. Restating is what a `SKILL.md` must do — it is the surface a session loads and cannot defer at runtime to a document the consuming repo does not have. Three parts are plugin-authored rather than contract-derived, and `planning`, a third implementer, contradicts two of them outright. ## Rebase resolutions Five version collisions with `main`, each laddered rather than clobbered, with `main`'s entry preserved intact above ours: `discovery` → **0.8.4**, `session-flow` → **0.15.3**, `rate-limit-guard` → **0.2.1**, plus `claude-config` at **0.9.3** and `claude-memory` at **0.4.1**, which are the first rungs of the ladder #1316 and #1318 build on. ## Related - #1377, #1315 — the branches this supersedes; both closed, neither could be rebased in place - #1316 — criteria payload (`claude-config` 0.10.0) - #1318 — the `audit-pass` skill (`claude-config` 0.11.0) - #1322 — the design record - #1278, #1279 — couplings scoped out to their own issues 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cross-vendor review against the run contract, the skill, the exclusion
set, the setup readiness check, and the suppression convention. The
load-bearing ones are places the contract required something no
participant could supply:
- Claim-template validation was fatal, and no delegated catalog declares
a template set -- so every catalog finding was unemittable. The pass
dispatches skills and never reads inside one, so a template set it
could learn only by opening another plugin's files is one it can never
learn. Templates now come from the invocation's own output; an
invocation declaring none is claim-unqualified, binding `claim` to the
check id with no parameters and naming that catalog in the coverage
notes as owing a declaration. Coarse deliberately, and stable, which
is the one property identity cannot do without.
- Lanes advertised (check x surface class) could not be dispatched
through the only allowed integration mechanism, because the delegated
skills' own filters are coarser. A lane is now one delegated
invocation at the finest filter that skill's interface accepts, and
lane count is bounded by those interfaces rather than chosen here.
- The personal suppression layer could add an entry for an id the team
layer does not carry, which is not a conflict for precedence to
resolve -- so the overlay silently suppressed exactly the
team-unaccepted finding the policy-floor inversion claims to prevent.
Only the team layer now enacts a suppression; a personal-only entry is
reported `personal-only, not applied`. The inversion is also restated
as what it actually decides: constituents hash to the key, so two
entries sharing a finding_id differ only in reason and date.
- Suppression is now central at every target, with no inline marker
anywhere. A prompt-type hook in JSON has no safe marker representation:
the marker would change the instruction shown to Claude and the bytes
the anchor hashes.
- The determinism precondition counted dirty files, so a dirty file
whose contents changed -- or one dirty path replacing another -- left
HEAD and the count identical and the gate passed a moved tree. It now
measures a worktree digest over dirty paths and content, captured at
Phase 0 and Phase 6.
- Partial-log records carry an attempt id delimited at both ends, so a
lane invalidated on resume cannot leave a crashed attempt's findings
ahead of the next attempt's with nothing to tell them apart.
- A `--report-to` path that did not exist was excluded only from
subsequent runs, so run 1 audited its own report. The redirecting run
now records the path in its own exclusion set.
Setup's readiness check also stops pointing out of the plugin cache
(`${CLAUDE_PLUGIN_ROOT}/skills/audit-pass/reference/run-contract.md`
ships inside the plugin; the marketplace convention path does not
resolve in an installed cache) and validates all five required keys
rather than two, which had let it report green on a record audit-pass
itself rejects.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AdF74i18gkGtHC9fjZyDrU
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 964a29eab6
ℹ️ 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".
claude-config collided on both halves: main landed 0.9.3 (the setup evals, from #1393) while this branch carries 0.11.0 and the five-skill manifest description. Keeps this branch's version and description -- 0.11.0 is ahead of both -- and keeps main's 0.9.3 CHANGELOG section beneath this branch's 0.11.0 section, so the history is continuous rather than losing a released entry to the resolution. The 0.10.0 rung between them is #1316, which merges first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdF74i18gkGtHC9fjZyDrU
Four findings, and the first two are the same class -- a guarantee stated without the mechanism that could hold it. - `target` accepted an arbitrary path while no delegated interface accepts a target at all: `audit-instructions` takes a surface scope and inventories the active project, `claude-memory:audit` takes an action verb. A run given ../other-repo would key, lock, and report against that path while every delegated finding came from the active project -- findings attributed to the wrong repository, undetectable downstream. `target` must now resolve to the active project root and a path that does not exits non-zero. Lifting that is a change to the delegated interfaces, not to this skill. - `--resume` could attach to a live read-only run. Concurrent read-only runs are safe because each owns its own partial artifact, and resume is the one operation that reaches into another run's -- so the no-lock policy that makes concurrency safe is exactly what left resume unable to tell a live run from an interrupted one, making assembly race-dependent. Every active run now maintains a lease on the same heartbeat the apply lock uses, and resume reads it before the manifest. The lease excludes nothing and grants no exclusivity; it answers the one question resume has to ask. - The worktree digest covered every dirty path, so writing a --report-to report inside the target made the redirecting run fail its own determinism gate as indeterminate, every time, for doing what it was asked. The own-artifact exclusion now applies to the digest as well as the scan, as one list so the two cannot diverge. Assertion 2.4 pins it. - Eval 19 still demanded the dirty-file count the contract had already replaced, so a candidate implementing the obsolete count-based gate passed it. It now requires the worktree digest, and new eval 20 covers the scenario the count cannot see: an already-dirty file edited with the count unchanged, plus the swapped-path variant and the own-artifact carve-out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdF74i18gkGtHC9fjZyDrU
|
Claude finished @kyle-sexton's task in 3m 28s —— View job PR Review —
|
| Codex finding | Resolution |
|---|---|
P1: --target dispatches to wrong repo |
SKILL.md §Arguments explicitly refuses any target that doesn't resolve to the active project root, exiting non-zero with both paths named. The argument survives in the interface for forward compatibility; the restriction is in the delegated interfaces. |
P1: --resume can attach to a live read-only run |
SKILL.md Phase 0 adds a lease that every active run (read-only included) holds on the same heartbeat as the applying lock; --resume reads the lease before the manifest and exits non-zero if it finds a live one. |
| P2: Redirected report path perturbs worktree digest | run-contract.md §6 now explicitly excludes class-4 artifacts from the digest on the same list as the scan set, with the reasoning stated. Assertion 2.4 codifies the expected outcome. |
| P2: Eval 19 still checked the dirty-file count | Eval 19 was updated to reference HEAD and the worktree digest; eval 20 was added specifically for the "count unchanged, content changed" case with all three expected behaviors named. |
Remaining items
1. Lease mechanism is described in SKILL.md but has no assertions in run-contract.md
SKILL.md Phase 0 introduces the lease (lines 72–84) and describes its semantics in full: every run writes one, --resume reads it before the manifest, a live lease causes a non-zero exit with the run id named, a stale lease means interrupted and resumable. But run-contract.md §3 (the concurrency section) still only formalizes the applying lock with assertions 3.1–3.3. There is no assertion that:
- a read-only run maintains a lease in its run state at all, and
--resumechecks the lease and exits non-zero on a live one.
Without a contract assertion, the lease guarantee is stated in a prose skill but not in the document whose purpose is to state conditions a test can assert. §5's assertions 5.1–5.3 also don't cover the live-lease case. A candidate implementation that skips the lease check satisfies all existing assertions.
2. "Same heartbeat the applying lock uses" references an undefined mechanism
SKILL.md Phase 0 line 79: "maintains a lease in its own run state on the same heartbeat the applying lock uses." The applying lock in run-contract.md §3 is a file containing a pid and an ISO-8601 start timestamp — no heartbeat refresh is described there. The applying lock's staleness is detected by pid-liveness plus a 30-minute age threshold, which is a one-shot check, not a heartbeat. The phrase "same heartbeat" implies a periodic refresh mechanism that exists nowhere in the spec.
This creates two possible readings: (a) there IS a heartbeat the contract forgot to specify, or (b) both the lock and the lease are one-shot records, and "same heartbeat" is imprecise prose. Either way, defining what makes a lease "live" vs. "stale" — parallel to the lock's pid+30-minute rule — belongs in the contract so an implementer doesn't have to guess.
3. Evals #1 and #7 still have identical prompts
evals.json — eval 1 (bare-invocation-is-read-only) and eval 7 (opinion-tier-discovery-line) both use the exact prompt /claude-config:audit-pass with no arguments. This was flagged in the previous review and was not addressed in either follow-up commit. If the eval framework keys on the prompt string, one will shadow the other. The expected outputs and expectations are distinct — only the prompt is ambiguous.
From the previous security review (19:00:11Z) — not yet addressed
These were flagged by the prior security review pass. None of the author's 22:34 UTC responses addressed them; they remain open.
| Finding | Status |
|---|---|
| IMPORTANT/HIGH: Apply-verify step sends repo diffs to a cross-vendor AI without a per-run consent gate — SKILL.md Phase 5 | Unaddressed. The text at lines 185–191 still delegates to a cross-vendor advisor with no confirmation step. |
| IMPORTANT/MEDIUM: Prompt injection via target-controlled instruction surfaces — SKILL.md Phase 1 | Unaddressed. Structural risk; no isolation or content-demarcation mitigation is stated. |
| SUGGESTION/MEDIUM: Target-controlled registry can exclude surfaces without byte-identity validation — exclusion-set.md Class 1 | Unaddressed. No validation step before accepting a registry path as a cluster copy. |
What is in good shape
- All four Codex round-3 findings are confirmed closed.
- The personal-layer suppression gap (previous review item perf(markdown-formatter): unwired producer builds telemetry payload before the opt-in guard (~90ms/edit on Windows) #6) is fully resolved: the docs now state that a personal-only entry does not suppress, is reported as
personal-only, not applied, and the "policy floor" scope is correctly scoped to justification, not visibility. - The P2 convergence domain error (previous review item feat: markdown-formatter plugin (pilot migration) #3) is resolved: P2 now conditions strict-subset only when a derived-tier fix was accepted.
- The assertion-2.2 / first-run-redirect gap (previous review item ci: onboard to the CI platform (ci-workflows + standards) #4) is resolved: assertion 2.3 and 2.4 both confirm the redirecting run records the path in its own exclusion and digest exclusion before writing.
- run-contract.md §7's attempt-boundary machinery is well-specified: start record, terminating record, highest-ordinal-with-terminator wins, supersession record on invalidation.
The three remaining items above are the only blockers I see against the current state. Items 1 and 2 are spec consistency issues (the lease mechanism is deployed but not formally contracted); item 3 is a test isolation issue. The security items from the prior pass are pre-existing and appropriately flagged there.
MD032/MD022 -- resolving the 0.11.0/0.9.3 collision joined the two sections without the blank line between them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdF74i18gkGtHC9fjZyDrU
|
Claude finished @kyle-sexton's task in 5m 58s —— View job Security Review — PR #1318 (commit b97bd41)
Correctly resolved in this review round (964a29e + b97bd41):
No GitHub Actions files changed. No credentials in code or state keys (remote URL credentials are stripped per run-contract §3). The following three findings remain open. IMPORTANT — Applied diff and findings sent to a third-party AI service with no per-run consent gateConfidence: HIGH
This text is unchanged from the version the prior security review (on commit 77dd51e) flagged. Under
The fresh-context subagent is described as the fallback, not the default, so a developer who installed the Codex plugin and runs The operator acknowledged the prior review raised this but did not comment on or act on it in any commit on this branch. IMPORTANT — Prompt injection via target-controlled instruction surfaces, no isolation statedConfidence: MEDIUM
Phase 1 reads A target whose instruction surfaces contain adversarial content — for example a The confidence is medium because actual exploitability depends on the model's instruction-following hierarchy in the session, and delegated skills may in practice receive isolated contexts. But the spec neither requires nor documents this. SUGGESTION — Target-controlled registry can remove surfaces from audit scopeConfidence: MEDIUM
Class 1 exclusions are derived by reading This was flagged by the prior security review as a SUGGESTION and is unchanged in No finding
|
|
Claude finished @kyle-sexton's task in 4m 22s —— View job PR Review —
|
| Surface | origin/main (0.15.2) |
This branch (0.15.1) |
|---|---|---|
fanout/SKILL.md |
code-review is in the Boundary — built-in/managed surfaces section (not dispatched as a normalized leaf; no parse contract) |
code-review appears as the second orchestrator plugin, dispatched as a fan-out surface with a PR-mutation gate |
fanout/context/findings-normalization.md |
Table carries a "Not in this table" note explaining why /code-review is excluded; no code-review row |
Adds a code-review plugin row with a 0–100, filters <80 confidence column |
fanout/evals/evals.json eval 6 |
References /code-review --comment and the managed Code Review GitHub App service |
References "code-review orchestrator's PR-comment-posting mode" |
fanout/evals/evals.json eval 11 |
Named unscored-surface-severity-derived-not-invented |
Named plugin-findings-severity-derived-not-invented |
The practical consequence: if this PR merges as-is, it re-classifies the bundled /code-review command as a marketplace orchestrator plugin and dispatches it as a fan-out leaf — the treatment that PR #1351 deliberately moved away from — while losing the 0.15.2 changelog entry entirely.
Remaining open items from prior rounds
Evals 1 and 7 have identical prompts — both fire on /claude-config:audit-pass with no arguments (evals.json lines 7 and 79). The expected outputs are different, so the two evals test distinct things, but a framework that keys on the prompt string may shadow one with the other. A trailing comment or a narrow trigger distinguisher in eval 7's prompt would resolve it without changing what the eval tests.
Cross-vendor advisor data egress (from the security review, HIGH confidence) — Phase 5 still reads: "hand the applied diff and the finding it claims to resolve…to a cross-vendor advisor when one is installed and set up (the OpenAI Codex plugin, say, invoked per its own docs)". Nothing in the skill warns the operator that --fix sends repository content — which includes audit findings and instruction-surface excerpts — to a third-party AI vendor, and there is no per-run consent gate before the delegation. The fresh-context subagent fallback stays local, but it is the fallback, not the primary path when the cross-vendor plugin is installed.
Minor spec consistency gap
exclusion-set.md Class 4 (lines 49–59) describes the own-artifact exclusion in terms of the scan set only. The worktree-digest exclusion — now load-bearing for Assertion 2.4 — is documented in run-contract.md §6 but not reflected in exclusion-set.md. A reader whose entry point is exclusion-set.md will not know that class-4 exclusion also governs the digest, and that the two are intentionally one list. Worth a sentence in Class 4 pointing to the digest coverage in run-contract.md §6.
Summary
The audit-pass spec is in good shape after the round-3 fixes. The one item to resolve before merge is the review plugin regression — five files in the review subtree need to be restored to their origin/main state, which the merge conflict resolution accidentally overwrote. The two remaining prior-round open items (identical eval prompts, cross-vendor egress consent) and the minor exclusion-set consistency note are lower priority and do not block merge on their own.
| Branch
…dispatched instruction-surface pass (#1322) The design record behind #1315, #1316, and #1318 — the decomposition of the source article, the proportionality gate that decided what to build, and the contracts the shipped code implements. No linked issue. ## What it decides Seven candidate detectors (D1–D7) went through a proportionality gate. **One survived as a new check** — cross-surface instruction conflict, the only finding with no incumbent. Every other candidate became an edit to a check that already owns its surface, or an explicit exclusion with a recorded reason. The gate's own working is here, including the parts that went against the first answer. ## The corrections are the point This branch documents four occasions where the effort asserted "nothing covers this" and was wrong each time, because nobody read the body of the incumbent — `skill-quality:check`, `mcp-tools:audit`, `claude-memory`'s C6 (on the deliverable's *own* primary check, *after* the failure mode had been named), and an eval runner that already existed as working code. Counts had the same disease. The standing rule that came out of it — derive counts by command, never transcribe them — is recorded with the instances that produced it. ## Independent verification A second derivation of the same source article was made **deliberately blind** to this one, and the two were compared. They substantially agree: the blind reader independently reached "one detector with no incumbent, everything else an edit inside the plugin that already owns the surface", and independently routed its own new rule to the same catalog at the same check number. Six findings moved a disposition and are recorded, along with seven things this derivation found that the blind one missed. Where they genuinely diverge — the blind derivation ratified "no new router" — the disagreement is recorded rather than resolved in this work's favour, with the reconciliation left open as a row due before Phase 9. ## A convention conflict this PR cannot resolve on its own `docs/conventions/topic-docs/` places `docs/topics/<slug>/` in the **contract tier**: "committed on the task branch only; pruned before merge." This PR does not prune, because **fifteen topic directories are already on `main`**, including one merged today. Practice and the written convention disagree wholesale, and this branch follows practice rather than silently picking a side. Reviewer call: merge as-is (consistent with every prior topic branch), or prune first (consistent with the written convention, which would then need every existing topic directory explained). Either way the convention and practice should stop disagreeing. Task #37 was resolved on the assumption the prune is real, and that work stands regardless — the shipped catalog carries its own source URLs and recheck triggers rather than pointing into this tree, so nothing in #1316 or #1318 dangles whichever way this goes. ## Also in here The prompt-injection threat model the design entirely lacked — `audit-pass` reads instruction surfaces that are attacker-controlled text in any repository the operator did not write. Plus the re-run contract, the determinism tiering, the seam resolution, and the naming record for `audit-pass` (32 candidates, five lenses, with what the chosen name *costs* recorded so it is not re-litigated). Rebased onto current `main`, including the `re-anchor` → `discipline` plugin rename from #1276. ## Related - #1315 — cross-plugin hygiene, the fixes this design's audit found - #1316 — the criteria payload - #1318 — the `audit-pass` skill - #1096 — fresh-eyes doctrine and check 21; this design's determinism contract was re-verified against it - #1225 — a second sweep-shaped effort; reconciliation open before Phase 9 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfb54463ec
ℹ️ 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".
|
Claude finished @kyle-sexton's task in 4m 20s —— View job Security Review — PR #1318 (commit cfb5446)
Correctly resolved since the prior security review round (b97bd41):
No GitHub Actions files changed. No credentials in code or state keys. Three findings from the prior security review remain open at this commit; one new finding emerges from the Codex P1 audit of the current HEAD; and one new suggestion. IMPORTANT — Applied diff sent to a third-party AI service with no per-run consent gateConfidence: HIGH
Unchanged from the prior two security reviews. Under
The fresh-context subagent is the fallback, not the default when the cross-vendor plugin is installed. A developer running IMPORTANT —
|
main reached claude-config 0.10.0 via #1349 (audit-instructions checks I12-I14), so this branch's 0.11.0 is the next rung and the manifest keeps it. Both CHANGELOG sections are kept -- 0.11.0 above main's 0.10.0 -- rather than losing a released entry to the resolution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdF74i18gkGtHC9fjZyDrU
…parability Reconciles the shipped run contract with the design record #1322 landed, and corrects that record where it had gone stale about this branch. The real gap: P1 was conditioned on tree and live surface set alone, so a harness or detection-version change read as a determinism failure -- the derived tier reads a versioned registry of harness behavior, so it is a function of the harness version by construction. The shipped contract now carries the same single comparability precondition the design does, stated once and cited by every property rather than restated per-property, which is what let the clause drift in the first place: tree, live surface set, detection version (catalog version plus a digest over the check's own detection-behavior inputs, including the criteria catalog and its imports), and harness version. A non-comparable pair is reported as non-comparable naming the input that moved, never as a pass or a failure. PLAN.md's "two divergences between this branch's design and what #1318 ships" was verified against the branch and is stale on both counts -- the superseded identity tuple was replaced by (check, claim, sites) with the anchor version tag and granularity prefixes, and liveness is present in both Assertion 1.1 and P1. Both are struck through with what closed them rather than deleted, since the record of what the walk found is the traceability the gate rests on. A stale divergence claim left standing on a durable surface is the exact defect this deliverable detects in other people's instruction files. Also: the 0.11.0 changelog still described the determinism gate as capturing a dirty-file count, which an earlier commit on this branch had already replaced with the worktree digest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdF74i18gkGtHC9fjZyDrU
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a25d0788fe
ℹ️ 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".
…sweeps Eleventh round. The first finding is the serious one and it is mine: the resume fallback fixed resumption and left cross-run comparison undecidable. - Comparability required equality of "the catalog version and prompt digest", which §5 had just established no current delegate emits. So no pair of real runs was ever comparable and P1-P4 asserted nothing about anything -- vacuity, silently, forever. Comparability now uses the OBSERVABLE detection version: a qualified check compares its declared values exactly; an unqualified one compares the delegate's semver from the marketplace manifest plus the harness version, and the report marks that check coarse. A sub-semver catalog edit is then missed, which is a real cost, but the failure directions are not symmetric -- coarse costs one wrong finding attributable to a named check, vacuity costs every property. An unknown sentinel comparing equal to itself would have been worse than either: a clean-looking comparison that misses exactly what the input exists to catch. - Behavior-affecting arguments were added to the resume digest and not to the cross-run predicate, so two runs differing only by --opinion were called comparable while one deliberately ran more checks -- enough extra judged findings to fail P4 as instability, produced by using a documented flag. - §7 still said a terminating record marks a lane complete, contradicting the open-terminator rule added two rounds ago. Completion is now read from the terminator's STATE: handed-back, declined, and ordinary completions complete a lane; open does not. - Eval 8 required every unchanged completed lane to be carried forward, which the detection-unqualified rule now forbids -- a candidate could pass the gate only by violating the operative contract. Updated to distinguish qualified from unqualified lanes and to read completion from terminator state. Also, pre-existing and not from this branch: setup's `apply` promised remediation for every FAIL while handling only jq and curl. `check` also FAILs on missing awk/sort (what conflict-scan.sh runs) and on no resolvable bash. Both now have entries, both noting that the remediation is a full userland rather than one package, and the network row is documented as deliberately having none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdF74i18gkGtHC9fjZyDrU
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c7c4c01ac
ℹ️ 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".
Twelfth round. Five findings fixed here; two routed to #1430 because the contract cannot settle them. Contradictions between two normative statements, both shipped: - Assertion 3.8 required a fenced writer to abort and write nothing, while §3's epoch design explicitly permits it to append to its own superseded file. No implementation could satisfy both. 3.8 now tests what actually provides the safety -- isolation from the adopter's epoch -- and records the heartbeat abort as bounding how long the stale writer keeps writing rather than as the isolation mechanism. - P2 still required `D(R2) ⊊ D(R1)` when a fix targeted a derived finding, which forbids the additions the attribution rule one clause above permits: renaming one of two shadowed definitions removes the targeted finding AND adds the renamed definition's inventory identity. Convergence is now stated as removal of the targeted findings, which is what it actually claims; additions stay governed by attribution. - doctor-handoff.md said the pass never parses /doctor output as a lane result, which makes the delegated tier unreachable and contradicts the same file's promise that the output lands there. The distinction is who decides, not whether the pass reads: it never invokes, waits on, or answers for /doctor, and it does record what the operator hands back. Shipped-behavior defects: - --report-to accepted any NEW path, so `--report-to CLAUDE.md` against a repo without one would create a live instruction surface out of a JSON report and then hide it from every later scan -- worse than the overwrite case, since no prior content is lost to signal it. Recognized instruction paths are now refused on name, existing or not. - The apply lock still keyed on `pid not alive AND age`, the PID-reuse hole already closed in the design record: a crashed run whose pid is reused blocks every later --fix for that state key forever. Liveness now requires pid AND start identity to match, with age alone reclaiming where no start identity exists. Found by sweeping before pushing rather than by review: P3 still carried its own shorter precondition list ("tree and catalog versions") instead of citing comparability, and the SKILL's --report-to line never got the destination rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AdF74i18gkGtHC9fjZyDrU
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b29eb4fce1
ℹ️ 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".
| documented way out. Where the platform supplies no start identity, **age alone reclaims** and the | ||
| reclamation says so — an unreclaimable lock is the worse failure, and the lease's heartbeat is what | ||
| makes a merely-slow holder visible rather than assumed dead. |
There was a problem hiding this comment.
On a platform that cannot provide process start identity, a legitimate --fix pass lasting longer than 30 minutes loses its lock to the next applying run even while its process is alive and refreshing its lease. The lease explicitly grants no exclusivity, so this permits both runs to edit the same project concurrently and violates assertion 3.1; retain a PID-liveness or live-heartbeat condition in this fallback rather than making age sufficient by itself.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This reply was generated by AI.
Addressed in #1848 (commit 1f70ec8b). This finding was stranded — it posted after #1318 merged, so the thread-resolution gate never saw it; recovered by the stranded-findings sweep, re-verified against origin/main, and triaged as item 1 of #1786.
The contract said "age alone reclaims" where no start identity exists, and mentioned the lease heartbeat only as prose no reclamation test consulted. The lock now records the holder's run id, and reclamation reads that holder's lease and applies §3's existing two-sided liveness test as the second conjunct: past 30 minutes a stale, released, missing, or unreadable lease reclaims; a live lease refuses exactly as it would inside the window, naming the run id and heartbeat_at. So a run that legitimately exceeds 30 minutes while still heartbeating keeps its lock and assertion 3.1 holds across the whole run.
This does not reintroduce the unreclaimable lock the age bound exists to prevent: a crashed holder stops refreshing, so its lease goes stale within the liveness threshold. Same remedy shape as claude-ops' restart-consumer (#1759/#1760). A follow-up review round also caught the migration case — a lock written before this rule carries no run id, so reclamation establishes the conjunct the other way round by enumerating every lease under runs/<state-key>/, which keeps an upgrade mid-run from handing a live holder's lock away. New assertions 3.12, 3.13, 3.14; new eval 27.
|
|
||
| | Condition | Disposition | Effect on the suppression | | ||
| |---|---|---| | ||
| | **Every** site's anchor matches, `(check, claim)` match | **SAME, UNCHANGED** | Applies silently, as an exact match always has. Phrased over the whole `sites` set rather than "both anchors", because the set holds one entry for an ordinary single-site finding and two for a pairwise one — the two-site phrasing left an unchanged single-site entry matching **no** row, so the commonest case in the table had no disposition at all. | |
There was a problem hiding this comment.
When a previously suppressed excerpt gains an identical duplicate under the same heading, the collided finding retains the same check, claim, site, and anchor, so this row classifies it as SAME, UNCHANGED and silently applies the suppression. That contradicts the collision rule at lines 126–131 requiring no suppression to carry across this ambiguity, and deleting the original afterward can transfer its suppression to the duplicate; collision state must force the entry stale or otherwise bypass exact matching.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This reply was generated by AI.
Addressed in #1848 (commit 1f70ec8b). Stranded after #1318 merged; recovered by the stranded-findings sweep, re-verified against origin/main, and triaged as item 2 of #1786.
The diagnosis holds exactly: a collided site's anchor is by construction unchanged, because the occurrence discriminator digests the heading path — so gaining a duplicate elsewhere in the section moves nothing, the entry satisfied the SAME, UNCHANGED row, and the suppression re-applied silently over a finding the operator's original decision provably cannot be attached to. That contradicted §1's own assertion 1.10a.
The collision test is now stated in both affected rows and evaluated ahead of the anchor comparison, routing to the existing OLD CLOSED, NEW OPENED disposition: the old entry is reported stale per 4.2, the finding appears unsuppressed, and the run names the collision with its occurrence count. It reuses row 3 rather than adding a fifth disposition, since stale-plus-unsuppressed is already the section's fail-closed answer to an ambiguous match. New assertion 4.7; new eval 28.
…n specifies Closes #1417 ## Summary `docs/conventions/topic-docs/README.md` specifies a required check that a merged PR carries no path under the contract-slice dir. The check was never built, so the convention has been unenforced for its entire life and 17 slices reached `main` — 6 of them on a single day. The only place `docs/topics/` reached CI at all was `scripts/docs-only-paths.txt`, as a docs-only ALLOWLIST entry, which makes such a PR cheaper to merge rather than blocking it. Evidence the rule is real and was being enforced by hand: PR #1286 was closed rather than merged, explicitly because its content was contract tier under `docs/topics/`. ## The deletion exemption The convention's own step 4 is a final commit that PRUNES the slice, so a literal "no path under the contract dir appears in the diff" reading would red-line the very commit that satisfies it. This gate keys on where a path LANDS: removals pass, a history-preserving `git mv` out of the contract dir (step 3's graduation) passes, and only an add, edit, or rename-into is red-lined. That requires knowing a path's status, which `--name-only` cannot express, so the gate reads `--name-status`. Deliberate deviation from the letter of the convention in service of its intent; the three-dot `base...HEAD` range is unchanged. ## Existing debt The 17 pre-existing slices are grandfathered by slug in `scripts/contract-slice-baseline.txt`, using the same stale-guarded idiom as `changelog-parity-baseline.txt` and `orphaned-fixtures-baseline.txt`: `--check` fails on an entry whose slice no longer exists, so an exemption cannot outlive its debt and a future slice cannot inherit a grandfathered slug. Graduating and pruning them is tracked separately. This is why the gate can land now instead of after a 71-file cleanup: it stops the bleed immediately while each slice graduates on its own PR, by whoever owns it. ## Verification The 11-case suite covers the add, pure-deletion, untouched, grandfathered, new-slug-despite-baseline, graduation-out, rename-into, unresolvable-base, live-baseline, stale-baseline, and usage paths. Measured against the four open PRs that carry `docs/topics/` paths, rather than asserted: #1318, #1252, and #1096 pass on their baseline exemptions; #1400 fails, correctly, because it adds two slices that are not pre-existing debt. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n specifies (#1429) Closes #1417 ## Problem `docs/conventions/topic-docs/README.md:305-309` specifies a required check that the net PR diff carries no path under the contract-slice dir: > 5. Enforcement: a required check that the net PR diff (`git diff --name-only base...head`) > contains no path under the resolved `<contract_dir>/**` (default `docs/topics/**`). It was never built. The convention has been unenforced for its entire life, and `main` currently holds **19** contract slices — **8 of them landed on 2026-07-25 alone**, two of those while this PR was open. The only place `docs/topics/` reaches CI today is `scripts/docs-only-paths.txt:43`, and it is there as a docs-only **allowlist** entry — which makes a PR confined to it *cheaper* to merge by skipping the heavy lanes. Nothing blocks it. The rule is real and people have been enforcing it by hand: **PR #1286 was closed rather than merged**, explicitly because its content was contract tier under `docs/topics/`. That is the cost of the missing gate — correct behaviour depending on whoever is looking remembering an unenforced rule, and 19 directories showing how reliably that scales. **#1400 merged while this PR was open**, landing two more slices. It was flagged there, and it is the sharpest available evidence for the gate: the failure mode is live, not historical. ## Why the gate permits deletions The convention's own step 4 is *a final commit that prunes the slice*. A literal reading — "no path under the contract dir appears in the diff" — would red-line the very commit that satisfies the convention. So the gate keys on where a path **lands**, not on whether it appears: | Change | Verdict | |---|---| | Delete under `docs/topics/` | pass — this is the prune step | | `git mv docs/topics/x/PLAN.md docs/adr/…` | pass — this is step 3's history-preserving graduation | | Add / edit under `docs/topics/` | **fail** | | Rename *into* `docs/topics/` | **fail** | | Diff never touches `docs/topics/` | pass | Knowing a path's status requires `--name-status`; `--name-only` cannot express it. That is a deliberate deviation from the letter of the convention in service of its intent, called out in the script header. The three-dot `base...HEAD` range is unchanged, so a slice `main` gained after a branch forked stays out of scope and no stale branch is forced to merge-from-main over someone else's violation. ## Why this lands before the cleanup `scripts/contract-slice-baseline.txt` grandfathers the 19 existing slugs, using the same stale-guarded idiom as `changelog-parity-baseline.txt` and `orphaned-fixtures-baseline.txt`: `--check` fails on an entry whose slice no longer exists, so an exemption cannot outlive its debt and a future slice cannot silently inherit a grandfathered slug. Exemptions are resolved from the **base revision**, not the working tree, so a PR cannot add a slice and grandfather its own slug in the same diff. The diff is judged against the union of the base and head contract roots, so a PR that relocates `contract_dir` cannot leave the root it selected uninspected either. Both bypasses were live in earlier pushes and were caught in review. The alternative — prune all 19 first, then gate — is a ~1.3 MB change requiring a graduation judgement on each slice by whoever owns it, and it would conflict every open PR that carries those paths. Gating first stops the bleed immediately while each slice graduates on its own PR at its own pace. The burn-down is #1419; each prune PR drops its own baseline line, and the stale guard means the debt cannot be quietly abandoned half-done. ## Verification `scripts/check-contract-slice-prune.test.sh` — 19 cases, all green: add, pure deletion, untouched tree, grandfathered slug, new slug despite a baseline, graduation out, rename in, unresolvable-base-ref (fail-closed, exit 2), live baseline entry, stale baseline entry, usage, self-grandfathering rejected, a pre-existing entry still exempting, `contract_dir` resolved from the concern file, a relocated root moving the gate's scope, a root-equivalent value exiting 2, a slug-less baseline surviving `set -u`, both base and head roots policed, and a grandfathered slice migrating to a relocated root. Four review findings were raised across two rounds and all four were reproduced before being fixed — two bypasses (self-grandfathering; a relocation leaving its own root uninspected) and two fail-open / crash defects (`contract_dir` ignoring the concern file; the gate aborting under `set -u` once the baseline empties, which is the exact end state #1419 drives toward). See the resolved threads; each carries its reproduction and the case that pins it. **Measured against the open PRs that actually carry `docs/topics/` paths, rather than asserted.** Because the gate reads the baseline from the base revision, these were run against a base that already carries it — the post-merge condition: | PR | Slice | Result | |---|---|---| | #1252 | `plugin-audit-port` | passes on baseline exemption | | #1096 | `fresh-eyes-checkpoint-audit` | passes on baseline exemption | | #1318 | `context-engineering-claude-5` + the two slices #1400 landed | fails until rebased onto a `main` carrying the updated baseline | #1318's failure is an artefact of it predating #1400's merge, not a defect: its branch adds those two files relative to its own fork point. Once rebased, they are on `main` and in the baseline, so they leave its diff entirely. The self-grandfathering bypass was verified closed by re-running the reviewer's own reproduction against the fix. Also verified: `shellcheck` clean, `actionlint` clean, `shfmt` clean, the org comment-hygiene policy reports zero violations in the new files, and both scripts carry the executable bit. ## Wiring `contract-slice-prune-gate` is added to `ci-status`'s `needs:` list. That aggregate derives its lane list from the needs graph, and `ci-status` is already a required status check on the ruleset, so the new gate becomes required with **no ruleset edit**. Job naming matches the existing precedent (`silent-skip-gate`, `orphaned-fixture-gate`, `changelog-parity-gate`). The self-test runs unconditionally so a broken gate cannot mask a regression; the PR-diff step is event-gated. ## Related - #1419 — graduate and prune the grandfathered slices. Each prune PR drops its own baseline line, and this gate's stale guard fails once an entry outlives its slice. Not closed by this PR. Its inventory needs updating to 19 once this lands. - #1400 — merged while this PR was open, landing two more slices; both added to the baseline as debt rather than treated as incoming work. Flagged there before it merged. - #1252, #1096 — open PRs carrying grandfathered slice paths; verified passing on their baseline exemptions. #1318 needs a rebase past #1400 (see Verification). - #1286 — closed by hand for carrying contract-tier content, which is the manual enforcement this gate replaces. - `docs/conventions/topic-docs/README.md` — the convention specifying this check as step 5. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…1318/#1320/#1405 Five review findings landed on those PRs after each merged, so the thread-resolution gate never saw them. All five re-verified live against origin/main before fixing. claude-config / audit-pass run-contract: - Lock reclamation gains a liveness conjunct where the platform exposes no process start identity: the lock now records the holder's run id, and the holder's lease -- classified by the contract's existing two-sided test -- decides. A live lease defers the reclaim instead of losing the lock, so assertion 3.1 holds for a run that legitimately exceeds 30 minutes. Same remedy shape as claude-ops' restart-consumer (#1759/#1760), whose deferral needs a hard ceiling only because its holder publishes no lease. Assertions 3.12/3.13, eval 27. - The matching table gains an anchor-collision clause, tested ahead of the anchor comparison and routed to the existing OLD CLOSED, NEW OPENED disposition, so 1.10a's "no suppression carries forward" is reachable rather than contradicted by the SAME, UNCHANGED row. Assertion 4.7, eval 28. loop-lane prompts and convention: - The prompts file stripped the discipline preamble from every dispatch brief and asserted that dispatched subagents inherit the root sweep's posture, which fresh-context subagents cannot. Reconciled toward the normative owner doc: every dispatch brief carries the preamble; the narrow exemption is sweep-all's own audit forks. The preamble does not recurse -- at a subagent's conversation start that skill reports its posture digest with no audit fan-out. The owner doc drops its hand-copied discipline enumeration, which its own no-enumeration rule forbade. source-control / babysit-loop: - The pre-escalation resolution dispatch is gated on the resolved thread-resolution dimension: dimension flooring binds every capability the cycle exercises, not only the tier keyword handed to babysit-prs, so --thread-resolution safe withholds the dispatch instead of narrating at it. Eval 6. - The dispatch names --independent-resolver as its mode, maps the D7.5 ledger onto that mode's validated evidence flags, and names the two thread shapes it refuses (multi-finding, severity-flagged), which escalate. As written the implied --autonomous could never clear the current non-outdated thread class the dispatch exists for. Eval 7. Refs #1786 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FVoZoMYXqf8ZVbQYixPVPW
…1318/#1320/#1405 Five review findings landed on those PRs after each merged, so the thread-resolution gate never saw them. All five re-verified live against origin/main before fixing. claude-config / audit-pass run-contract: - Lock reclamation gains a liveness conjunct where the platform exposes no process start identity: the lock now records the holder's run id, and the holder's lease -- classified by the contract's existing two-sided test -- decides. A live lease defers the reclaim instead of losing the lock, so assertion 3.1 holds for a run that legitimately exceeds 30 minutes. Same remedy shape as claude-ops' restart-consumer (#1759/#1760), whose deferral needs a hard ceiling only because its holder publishes no lease. Assertions 3.12/3.13, eval 27. - The matching table gains an anchor-collision clause, tested ahead of the anchor comparison and routed to the existing OLD CLOSED, NEW OPENED disposition, so 1.10a's "no suppression carries forward" is reachable rather than contradicted by the SAME, UNCHANGED row. Assertion 4.7, eval 28. loop-lane prompts and convention: - The prompts file stripped the discipline preamble from every dispatch brief and asserted that dispatched subagents inherit the root sweep's posture, which fresh-context subagents cannot. Reconciled toward the normative owner doc: every dispatch brief carries the preamble; the narrow exemption is sweep-all's own audit forks. The preamble does not recurse -- at a subagent's conversation start that skill reports its posture digest with no audit fan-out. The owner doc drops its hand-copied discipline enumeration, which its own no-enumeration rule forbade. source-control / babysit-loop: - The pre-escalation resolution dispatch is gated on the resolved thread-resolution dimension: dimension flooring binds every capability the cycle exercises, not only the tier keyword handed to babysit-prs, so --thread-resolution safe withholds the dispatch instead of narrating at it. Eval 6. - The dispatch names --independent-resolver as its mode, maps the D7.5 ledger onto that mode's validated evidence flags, and names the two thread shapes it refuses (multi-finding, severity-flagged), which escalate. As written the implied --autonomous could never clear the current non-outdated thread class the dispatch exists for. Eval 7. Refs #1786 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FVoZoMYXqf8ZVbQYixPVPW
…1318/#1320/#1405 (#1848) ## Summary Closes the five stranded review findings triaged in #1786 — findings that landed on #1318, #1320, and #1405 *after* each merged, so the thread-resolution gate never saw them. Each was re-verified against `origin/main` before being fixed; none was refuted, and none was already fixed. **1 — Lock reclamation had no liveness conjunct where the platform exposes no process start identity (P1).** `claude-config/skills/audit-pass/reference/run-contract.md` said *"age alone reclaims"* there, and mentioned the lease heartbeat only as prose no reclamation test consulted — so a `--fix` run legitimately exceeding 30 minutes lost its lock to a second run, against the contract's own assertion 3.1 (*"exactly one proceeds"*). The lock now records the holder's **run id**, and reclamation reads that holder's lease and applies the contract's *existing* two-sided liveness test as the second conjunct: a stale, `released`, missing, or unreadable lease reclaims; a **live** lease refuses exactly as it would inside the 30-minute window. Same remedy shape as `claude-ops`' restart-consumer (#1759/#1760) — cited by its actual heading phrase, *"age alone never reclaims"* — with the one deliberate divergence stated at the site: restart-consumer needs a hard 24-hour deferral ceiling because its holder publishes no lease; a lease-bearing run needs none, because a dead holder stops heartbeating within five minutes. New assertions 3.12 / 3.13, new eval 27. **2 — The `SAME, UNCHANGED` table row had no anchor-collision exception (P2).** A collided site's anchor is by construction *unchanged* (the occurrence discriminator digests the heading path), so a previously-suppressed excerpt that later gained an identical duplicate matched row 1 exactly and silently re-suppressed itself — contradicting assertion 1.10a's *"no suppression carries forward"*. Collision is now tested **ahead of** the anchor comparison in every row and routes to the existing `OLD CLOSED, NEW OPENED` disposition (entry stale per 4.2, finding unsuppressed, collision named with its occurrence count), reusing the section's established fail-closed answer rather than adding a fifth disposition. New assertion 4.7, new eval 28. **3 — The prompts file stripped the discipline preamble from every dispatch brief (P2).** `prompts/loops/loop-lane-prompts.md` forbade the sweep *"inside a dispatch brief"* and claimed dispatched subagents *"inherit the posture the root sweep already set"* — which a fresh, non-inherited context cannot — directly contradicting the normative owner doc (`docs/conventions/loop-lane/README.md`, "Subagent discipline preamble"). Reconciled **toward the owner doc**, as the issue's acceptance criterion required: every dispatch brief carries the preamble; the one recursion still forbidden is narrowed to `sweep-all`'s own audit forks. Verified against the skill itself — at a subagent's conversation start `sweep-all` reports its cheap posture digest and runs no audit fan-out, and its preflight degrades to that digest when the fan-out cannot inherit a conversation (a fresh subagent never can), so the preamble costs one skill read per dispatch and does not recurse. The owner doc also drops its hand-copied enumeration of the individual disciplines, which its own no-enumeration rule forbade. **4 — The resolver dispatch ignored per-dimension overrides (P1).** The *"Dimension overrides bind by tier flooring"* rule in `babysit-loop/SKILL.md` was scoped only to *"Before invoking"* the babysit-prs tier, and `reference/pre-escalation-dispatch.md` contained no occurrence of `dimension` at all — so `autopilot --merge c3-this-run --thread-resolution safe` still dispatched a fresh subagent to mutate bot threads the operator's own argument had just denied, against `config-resolution.md`'s *"invocation arguments win"*. Resolving review threads **is** an exercise of autonomy dimension 3, so the flooring rule now binds every capability the cycle exercises, not only the tier keyword it passes on: a floored thread-resolution dimension withholds the dispatch outright and the PR escalates, reported as override-constrained — never a dispatch made and then narratively told not to resolve. New eval 6. **5 — The dispatch named no resolver mode, and the implied one could not work (P2).** As written, *"the full per-PR worker lifecycle"* implied `--autonomous`, which hard-refuses any thread not already `isOutdated` before its own push — precisely the current, non-outdated bot thread D7.5 routes to this dispatch. The mode is now stated as **`--independent-resolver`**, with the D7.5 ledger mapped onto that mode's machine-validated evidence flags, and the two thread shapes it refuses named where the dispatch meets them (multi-finding, severity-flagged — both escalate). The worker-lifecycle sentence is scoped to how a *code change* is made rather than to mode selection. New eval 7. **Issue AC item 4 — checked before any work started.** `9df80ba5aa` is **not** an ancestor of `origin/main`, but `--independent-resolver` **is** present on `origin/main` (`babysit-prs/scripts/babysit_resolve_thread.py`), so the mode landed by another route and finding 5 is not done twice. Every flag, constraint, and refusal this PR asserts about that mode was verified against the script by an independent reader: flag spellings, the `--disposition` requirement, the single-pinned-`--thread-id` requirement, the `--autonomous` / `--include-human` / `--allow-unpinned-thread` conflict set, and the `isOutdated` refusal. **Already fixed — recorded so nobody re-checks it.** The sixth triaged item, #1320:475 (the unbounded babysit-loop launch command), was fixed by `d8575516cd` in #1405; the generic command carries `{{MERGE}}` and both remaining invocation examples carry an explicit merge cap. No change here. **Not in this PR.** The issue's last acceptance criterion — resolving the five threads on #1318 / #1320 / #1405 — can only happen once these fixes land, so it is a post-merge follow-up on #1786's own thread rather than a change in this diff. ## Test plan Every surface here is documentation and contract prose, so the executable regression net is the repo's own gates plus the skills' eval sets. All run locally against this branch: - `python3 scripts/check-contract-clause-coverage.py` — **passed** (4 canonical surfaces, 14 tagged restatements, 10 pointing surfaces). It initially **failed** on this branch, catching an untagged restatement of `D7.5-thread-eligibility` the new resolver-mode section had introduced; reduced to a pointer, which the gate's own guidance calls the stronger fix. - `scripts/check-changed-skills.sh origin/main` — **PASS** for the changed skills, 0 errors (`audit-pass` 2 warnings, `babysit-loop` 1 warning; all pre-existing soft-target line-count and fresh-eyes notes). `babysit-loop/SKILL.md` is 499/500 lines, inside the hard cap — the base itself is at 499, so the net-new prose was reflowed and the dimension-flooring detail pushed down to `reference/pre-escalation-dispatch.md`, which already owns it, rather than restated twice. `babysit-prs` reports one failure, `engine.test.sh`, which is **environment drift and not this branch**: it fails identically on the unmodified base with 82 ruff findings, and under CI's pinned `ruff==0.15.22` (run via `uvx`) the same tree reports `All checks passed!`. Local ruff is 0.16.0, and CI agrees with the pin — the `ci` lane is green. - `scripts/check-changelog-parity.sh` `--check`, `--check-order`, `--check-bump origin/main` — all **passed**; both bumped plugins carry a matching entry and every changelog reads newest-first with no duplicate versions. - `scripts/validate-plugins.sh` — **passed** (all plugin manifests and the catalog). - Eval sets validated against `plugins/skill-quality/reference/evals.schema.json` — **passed**. - `scripts/check-orphaned-fixtures.sh --check` and `scripts/check-contract-slice-prune.sh --check` / `--check-diff origin/main` — **passed**. - `markdownlint-cli2` over every changed tree — **0 issues**. - `typos --config _typos.toml` — **clean**. New regression coverage added with the fixes: `audit-pass` evals **27** (a live lease refuses the lock past the age bound; a stale, missing, or unreadable one reclaims) and **28** (an anchor collision never carries a suppression forward); `babysit-loop` evals **6** (`--thread-resolution safe` withholds the resolver dispatch entirely) and **7** (the dispatch runs `--independent-resolver`, and refuses a multi-finding thread). Two defects were found in the in-flight work and fixed before this PR opened, both by checking the artifact rather than the claim: a **duplicate assertion id `4.6`** in run-contract.md's §4 table (the new row now appends as `4.7`, matching the table's append-at-end convention), and a **fabricated section citation** — restart-consumer.md was cited as *"the lock reclaims"*, a phrase that does not appear in it; corrected to its actual bolded phrase *"age alone never reclaims"*, and the accompanying claim that a live holder *"never loses the lock"* corrected, since restart-consumer does impose a hard 24-hour ceiling. ## Round 2 — review findings, and a rebase The second commit (`e6b91b4c`) addresses three review findings from the first round; each carries a per-thread reply with its disposition. All three were valid. - **P1, run-contract.** A lock persisted by an earlier plugin version carries no run id, so the new lease-locating conjunct could not be constructed for it — and treating that unlocatable lease as stale fell straight back to age-only reclamation, the exact failure this change removes, at the moment it is most likely (an operator who just upgraded and is re-running). Fixed by establishing the conjunct the other way round for such a lock: enumerate every lease under `runs/<state-key>/`, and any one live by the same two-sided test defers the reclaim. Over-deferral on a live read-only lease is stated as the deliberate fail-closed direction and is bounded by that lease's own release/staleness. **New assertion 3.14.** - **P2, `babysit-prs/reference/safety.md`.** The section titled *"Security/P1 escalation: the one named exception"* cast this resolver as that exception, citing the loop-lane convention's §1 — but §1's exception widens the **merge rung** for a single run and never touches severity, while the wrapper refuses a severity-flagged thread in every unattended mode. The documented exception was unreachable, and it contradicted the refusal this PR had just made explicit. Retitled and reframed: the bright line has no exception, the typed pair unlocks the dispatch *path*. `babysit-prs/SKILL.md`'s one-line restatement corrected to match. - **P2, `babysit-loop/SKILL.md`.** The surface that actually builds dispatch briefs still hand-copied the discipline plugin's membership inline — the enumeration this PR had just removed from the owner doc. Now points at the sweep skill, which resolves its own membership. **Rebased onto current `main`.** The branch was several commits behind, and the round-2 push had triggered only the `pull_request_target` workflows, leaving the full `ci` suite unrun against the new code; rebasing forced a clean re-trigger. Two upstream commits had touched the same two files this PR edits (`babysit-loop/SKILL.md`, `babysit-prs/reference/safety.md`) — both auto-merged, and the merged result was re-read rather than assumed. Version bumps re-derived against the moved base: `source-control` 0.42.4 → **0.44.1**; `claude-config` stays **0.16.1**. Every gate above was re-run against the rebased head, and all **32 PR checks are green**, `ci` included. ## Related Fixes #1786 Refs #1318 Refs #1320 Refs #1405 Refs #1777 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…ckRollup running-check pitfall (#2513) ## Summary Lands the two PR-lifecycle facts #1436 recorded from driving #1393/#1316/#1318/#1322 to merge, in the document that was prescribing the expensive path: - **Stale-branch recovery now defaults to merge-forward.** `monitor.md`'s mergeable pre-check (§3.1 step 1) and stale-branch recovery rule (§3.2) both prescribed "force-push with lease" — but auto-mode permission classifiers commonly deny force-push, and the recorded consequence was a fresh branch + fresh PR + closing the old one per rebase (the #1315 → #1377 → #1393 churn, with every review thread re-opened on the successor). Merging the default branch *into* the PR branch resolves staleness and pushes **fast-forward** — no force-push — and under a squash-only default branch the merge commits collapse to one commit on merge, so linear-history requirements stay satisfied. Verified in the issue's own record: #1393 landed that way and #1318 was merge-forwarded five times without needing a new branch. Rebase stays available as the exception for projects requiring a linear PR branch where force-push is actually permitted. - **`statusCheckRollup` reports a running check as `conclusion: ""` (empty string), not `null`.** The complement-shaped filter (`conclusion != null and != "SUCCESS"`) therefore counts every in-progress check as a failure — the exact misreport in the issue (two "failing" checks that were simply still running). The multi-PR scan section (§3.0.6, the one place this skill reads `statusCheckRollup`) now documents the pitfall with value-positive jq selectors for "failed" and "still running". Version `0.53.11` → `0.53.14` (patch; `0.53.12`/`0.53.13` are claimed by in-flight PRs #2450/#2453/#2483/#2510 and #2469 — skipping past them per the #1746 collision pattern). ## Test plan - `npx markdownlint-cli2@0.23.2` on both edited markdown files — 0 issues. - Docs-only change to skill reference text; no scripts or hooks touched. The jq forms added are the ones from the issue, verified against `gh pr view --json statusCheckRollup` semantics. ## Related Fixes #1436 Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…ckRollup running-check pitfall (#2513) ## Summary Lands the two PR-lifecycle facts #1436 recorded from driving #1393/#1316/#1318/#1322 to merge, in the document that was prescribing the expensive path: - **Stale-branch recovery now defaults to merge-forward.** `monitor.md`'s mergeable pre-check (§3.1 step 1) and stale-branch recovery rule (§3.2) both prescribed "force-push with lease" — but auto-mode permission classifiers commonly deny force-push, and the recorded consequence was a fresh branch + fresh PR + closing the old one per rebase (the #1315 → #1377 → #1393 churn, with every review thread re-opened on the successor). Merging the default branch *into* the PR branch resolves staleness and pushes **fast-forward** — no force-push — and under a squash-only default branch the merge commits collapse to one commit on merge, so linear-history requirements stay satisfied. Verified in the issue's own record: #1393 landed that way and #1318 was merge-forwarded five times without needing a new branch. Rebase stays available as the exception for projects requiring a linear PR branch where force-push is actually permitted. - **`statusCheckRollup` reports a running check as `conclusion: ""` (empty string), not `null`.** The complement-shaped filter (`conclusion != null and != "SUCCESS"`) therefore counts every in-progress check as a failure — the exact misreport in the issue (two "failing" checks that were simply still running). The multi-PR scan section (§3.0.6, the one place this skill reads `statusCheckRollup`) now documents the pitfall with value-positive jq selectors for "failed" and "still running". Version `0.53.11` → `0.53.14` (patch; `0.53.12`/`0.53.13` are claimed by in-flight PRs #2450/#2453/#2483/#2510 and #2469 — skipping past them per the #1746 collision pattern). ## Test plan - `npx markdownlint-cli2@0.23.2` on both edited markdown files — 0 issues. - Docs-only change to skill reference text; no scripts or hooks touched. The jq forms added are the ones from the issue, verified against `gh pr view --json statusCheckRollup` semantics. ## Related Fixes #1436 Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
1 participant
Adds
audit-pass— a single coordinated pass over a git repository's instruction surfaces, dispatching to the catalogs that already own each check rather than duplicating them.No linked issue.
Why a pass and not another check
The value is coordination, not new criteria. Findings from
claude-config:audit-instructions,claude-memory:audit, and/doctorarrive today as separate runs with no shared identity, no re-run semantics, and no way to say "I looked at this one and I am keeping it".audit-passhosts no checks of its own — it sequences the ones that exist, gives their findings a stable identity, and makes a re-run comparable to the run before it.The re-run contract is the substance
/doctorand carries neither. The earlier two-tiermechanical/behavioralsplit could not carry the claim: verified against the implementations, no dispatched check reaches the report without model judgment, and half the dispatched catalog never used that vocabulary at all.Suppression is a convention, not a flag
docs/conventions/finding-suppression/defines the keys; the instance lives at.claude/audit-pass.md. Keyed per finding id rather than as a list, because a closed list is taken whole and one personal suppression would silently discard every team suppression. Team layer wins on conflict. Reason and date are required.Verification
check-changelog-parity.sh --checkand--check-bump,check-skill-portability.sh,check-skill-leaf-names.sh,check-silent-skips.sh,check-orphaned-fixtures.sh, andvalidate-plugin-contracts.mjsall pass.skill-quality:checkonaudit-pass: PASS, 0 errors, 0 warnings — 200/500 lines, description 870/1536 chars, all 7 base-ref trigger phrases preserved, markdownlint clean.Known open items, stated rather than hidden
These are tracked and deliberately not blocking this PR, but a reviewer should know them:
X-vs-Ydifferently fromY-vs-Xand report one conflict twice. Confirm the shipped implementation matches.claudeMdExcludes(which merges across settings layers), and a declined external-import dialog all change what is loaded, and the last is persistent, machine-local, and invisible in the repository. Any finding whose truth depends on liveness cannot satisfy a cross-machine stability promise, and must record its liveness basis as evidence.Merge-order note
Takes
claude-configto 0.10.0. #1315 carries a 0.9.3 for the same plugin that is not onmainyet; whichever merges second needs the other's changelog entry inserted in order. This branch also adds a row todocs/PLUGIN-PHILOSOPHY.md's conventions index, which #1315 and #1096 both edit.Related
claude-configversion andPLUGIN-PHILOSOPHY.mddocs/PLUGIN-PHILOSOPHY.md🤖 Generated with Claude Code