fix(setup): close the setup-corpus audit's findings and two falsified reference claims - #1315
Closed
kyle-sexton wants to merge 11 commits into
Closed
fix(setup): close the setup-corpus audit's findings and two falsified reference claims#1315kyle-sexton wants to merge 11 commits into
kyle-sexton wants to merge 11 commits into
Conversation
…nly carve-out Two setup-contract findings against shipped plugins dissolved on inspection into owner-doc gaps rather than plugin defects. Both are fixed here, at the definition site. `userConfig` non-triviality had no definition, so criterion (c) — "a non-trivial userConfig requires a setup skill" — was unfalsifiable and every audit re-litigated it. The line now follows from what the 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. An option is non-trivial when it names an external referent needing verification, carries no default preserving zero-config behavior, or is coupled to another option or to state outside the manifest. Everything else is trivial, however many options a manifest holds: count is not the test and neither is declared `type`. The check-only carve-out was scoped to plugins whose entire configuration is `userConfig`. That excluded a shape the fleet actually ships: a plugin whose behavior is delivered through Claude Code settings this contract forbids setup to mutate — statusline wiring, a settings-level key. Such a plugin has nothing conforming to write, which is the same condition the carve-out already recognized, reached by a different route. The carve-out now names its condition directly — no writable owned artifact — and enumerates three qualifying surfaces: native `userConfig`, forbidden Claude Code settings, and external prerequisites. Silence is not the conforming response on the settings surface: `check` prints the exact edit, fully resolved, states that it is the operator's to apply, and names what re-invalidates it. `apply` is stated as owed wherever a writable owned artifact exists and only there, so the two shapes are selected by surface rather than by author preference, and two plugins sharing an unwritable settings surface can legitimately differ when only one also owns a machine-scope file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`audit`'s reference asserted, dated 2026-04-01, that `.claude/rules/` files "load unconditionally at session start regardless of `paths:` frontmatter", citing four open issues. A first-party repro on Claude Code 2.1.219 disproved it: a rule scoped `paths: ["**/*.tsx"]` was absent at session start, present after reading a matching `.tsx` file, and absent again after reading a non-matching one. Deferral works in both directions. The cited evidence failed independently of the repro. Two of the four issues are closed NOT_PLANNED and never supported the claim — #38487 asks that Write/Edit *also* trigger injection, which presupposes deferral works, and #32906 is a docs issue about subagents. The two still open assert opposite failure modes, so they cannot jointly support one conclusion. No version floor is claimed. No changelog entry or maintainer comment pins when the behavior changed, so the passage states what was verified, on which version, on which date, and nothing beyond it. The caveats that survive are kept and each is sourced: an `@import` inside a path-scoped rule still inlines at session start and defeats the scoping; path-scoped content is invisible to subagents, teammates, and skill-forked contexts (#32906, closed NOT_PLANNED — accepted behavior); writing a new file does not trigger the rule, since the trigger is a read; and before v2.1.211 on-demand rules loaded even when `project` was excluded from `--setting-sources`. The correction is load-bearing beyond the reference: a destination- correctness check that routes content to a path-scoped rule for deferred load would have been blocked by the false claim. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`setup` shipped no evals, against this repo's rule that a skill carrying behavioral warrants demonstrates them. The gap was found auditing the fleet's setup corpus, and it is the audit's own host plugin, so it is fixed before the corpus findings are raised elsewhere. Four cases, each bound to a behavior SKILL.md asserts, and none inventing one: - A bare invocation routes to `check`, reads the bundled scripts to establish their real prerequisites rather than reciting SKILL.md, writes nothing, and makes no network call. - A missing `curl` FAILs scoped to `check-plugin-drift.sh` alone and states that the rest of `audit` and the other three audit skills still run — the failure is not generalized to the plugin. - An install request under `apply` yields platform instructions without executing a package manager, and never reports the prerequisite resolved on an install command's exit code, only on a re-run probe. - An audit request smuggled into a setup invocation routes to the audit skills by name instead of being performed under `setup`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… requires
The setup contract's `apply` is state-assessing: it reads current state and
converges, preserving keys it does not recognize and reporting -- never
silently rewriting -- values it cannot reconcile. This skill asserted the
posture ("never blindly rewrites") and carried an idempotency check, but
neither specific guarantee had a line a reader could cite, so a conforming
implementation and a merge-from-answers rewrite were both consistent with
the text.
Both clauses are now explicit and scoped to `routing.yaml`, the only file
`apply` merges. `conventions.md` is a prose stub already governed by
never-overwrite-or-append, so a preserve-unrecognized-keys guarantee about it
would describe nothing. An unrecognized key may be a consumer extension or a
newer plugin version's; an unreconcilable value converged in silence is config
loss the consumer discovers only when routing misbehaves.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The setup skill read `${user_config.active_profile}` and reported the resolved
profile path, but documented no way to change the stored value. A consumer
whose configured profile is wrong for the repository was left with the per-run
`--profile` override as the only visible lever -- which does not persist -- and
nothing said why the skill itself would not just write the new value.
`check` step 1 now names the three routes and what each one does: the
interactive `/plugin configure ai-briefing` flow, which is the only surface
that changes the stored value; the headless `--config` path, carrying the
fresh-install-only caveat that makes uninstall-then-reinstall the headless
reconfiguration route; and the per-run `--profile` for a one-off that should
not touch stored config at all. The prohibition on this skill writing
`pluginConfigs` is now stated where a reader asks the question, rather than
only in the closing does-not list.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…bserver config Every observer tunable is native `userConfig`, and `apply` routed reconfiguration through `/plugin configure session-flow` -- an interactive-only surface. A headless or CI consumer had no documented path, and the obvious guess is actively misleading: re-running `claude plugin install --config` against an already-installed plugin does not update the stored value, so the reader would come away believing a key was set when it was not. `apply` now names both routes and what each does, including the flag's fresh-install-only behavior, the uninstall-then-reinstall sequence it forces, and the consequence that one install must carry every key being changed. The no-`pluginConfigs` prohibition is unchanged -- the point is that setup cannot write the value, not that the value cannot be changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lisher-neutral Both setup skills cited `topic-docs.schema.json` by a raw.githubusercontent.com URL carrying a hardcoded publisher and repository name. That is a runtime-consulted reference inside plugins that are otherwise publisher-agnostic: under a fork, a mirror, or an account rename the skill keeps resolving someone else's schema, and nothing in the file signals that it should not. Each now names the schema by the convention's own filename and defers to its plugin's `reference/topic-docs.md`, which is the binding whose stated job is to cite the published convention -- and which both setup skills already link. This follows `planning`'s setup skill, which resolved the same seam the same way. The result is one coupling site per plugin instead of two, located in the file that owns the upstream pointer, so the broader forge-lock question (the staged `raw.githubusercontent.com` portability class) has a single place to land in each plugin rather than a scattered set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eadless config route Three defects in the setup skill and its reader contract, all pointer- or justification-level; no behavior changes. The skill claimed the check-only carve-out under its old scoping -- plugins whose entire configuration is native `userConfig`. This plugin does not meet that premise: its statusline wiring lives in the user's own `settings.json`, which is neither `userConfig` nor tracked project config, and the skill itself frames that surface as configuration it must not write. The conclusion was correct and the stated reason was not, which is the worse failure: a reader checking conformance against the premise finds it false and cannot tell whether the plugin or the doc is wrong. The Purpose now names the condition that actually holds -- no writable owned artifact anywhere in the surface -- and enumerates all three surfaces with why each is unwritable. It also says explicitly that the runtime files under `~/.claude/rate-limit-guard/` are the tee's and the hook's data rather than operator-editable configuration, since that is the exact discriminator between a plugin that must not invent an `apply` and one that owes a narrow one. The kill switch's reconfiguration guidance named only the interactive `/plugin configure` flow. A headless consumer had no route, and the natural guess -- re-running `claude plugin install --config` -- silently does nothing once the plugin is installed, so the reader would believe a value was set when it was not. Both routes and that caveat are now stated together. The reader contract cited `docs/PLUGIN-PHILOSOPHY.md` for the no-`experimental.monitors` decision. That path does not exist in an installed plugin's cache, which is precisely where sibling-plugin consumers read this contract, so the citation resolved to nothing for its actual audience. Copying the doc's content in would be the wrong repair; the note now states the reason a reader needs, self-standing, with no pointer to follow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… unreachable citation `reset` is a reserved token in the setup contract: it decomposes to teardown plus `apply` -- converge to the ABSENCE of the plugin's own config, then reconfigure. This skill used it for the opposite operation. `apply reset` converges forward: it sets both recognized band keys to the shipped defaults and explicitly preserves every unrecognized key the operator's `zones.json` carries, removing nothing. An operator who read the argument against the contract's definition would expect their custom keys destroyed and would reasonably refuse to run it. Renamed to `apply defaults`, which says what it does, with the converges-forward/never-removes property stated at the argument rather than left to be inferred. No compatibility alias for the old token, per the contract's clean-break stance; the argument-hint and the eval that names the mode move with it. Separately, the Purpose cited "the narrow-write carve-out" and `docs/PLUGIN-PHILOSOPHY.md`. The first names a shape without naming the condition that selects it, and the second is a repository path that does not exist in an installed plugin's cache. Restating the doc's content locally would trade an unreachable pointer for a drifting copy, so the Purpose instead states the plugin-local fact the citation existed to support: the statusline surface and the `jq` prerequisite are unwritable, and this plugin owns exactly one writable artifact -- `zones.json` -- which is what obliges an `apply` at all. The reader contract's identical dead citation is fixed the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ed plugin state
Both the setup and audit skills resolved `<StateBase>` as `${CLAUDE_PLUGIN_DATA}`
with a fallback to a literal `$HOME/.claude/plugins/data/machine-health`. That
fallback cannot be correct: the directory under `~/.claude/plugins/data/` is
named for the plugin's install identity (`<name>-<marketplace>`, or
`<name>-inline` under `--plugin-dir`), never the bare plugin name, so the
guessed path always names a DIFFERENT directory than the one the plugin reads
and writes.
This is not hypothetical. On the machine this was found, the catalog overlay
and a registered custom check sit under `machine-health/` while the audit's
`state/` and `logs/` sit under `machine-health-melodic-software/`. The operator
disabled checks that kept running, and each root looked complete to whatever
had written it -- the failure mode a fallback path is supposed to prevent,
caused by the fallback path.
Both skills now resolve the anchor and, when the token does not expand, stop
and report an unresolved state root rather than substituting a guess. `check`
FAILs there instead of continuing: with the root unknown, an absent overlay and
an unreadable one are the same observation, so the remaining probes would be
reporting confidence they do not have.
Because the wrong path was shipped, `check` also gains a split-root report --
it probes the legacy path and any `machine-health-*` sibling, names what each
holds, and states that only the resolved root is read. It does not move or
delete anything: that is the operator's data and their decision.
The README's migration step asserted the same wrong resolution and was sending
operators to move `state/` into the directory the plugin does not use; it now
describes how the name is formed and routes to `check` for the real path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…setup's stop rule
Follow-up to f6a4f0b3fa, which overstated the defect and left the setup skill
self-contradictory.
The orchestrator script was never wrong. Its documented ladder is `-StateBase`,
then the `CLAUDE_PLUGIN_DATA` environment variable, then `-OutputBase` -- it
never names the bad literal path, and the `-OutputBase` rung is deliberate,
since the elevated re-launch loses the environment variable and pins the root
explicitly. Telling the audit skill to "stop and report" contradicted a real,
sound fallback. It now says what is actually correct for a caller: pass
`${CLAUDE_PLUGIN_DATA}` when it expands, and when it does not, OMIT
`-StateBase` so the orchestrator's own ladder applies, then report which root
it used. Never substitute a literal path remains the rule, because that is the
part that was broken.
The two skills legitimately differ here, and the setup skill now says why:
`audit` hands a root to a script with a further rung, while `setup` reads and
writes the overlay itself and has none, so an unexpanded token is terminal for
it. Setup's stop rule is also made self-consistent -- the previous commit said
both "stop at step 1" and "every probe reports UNKNOWN", which cannot both
hold. It stops; the remaining probes do not run, and `apply` refuses for the
same reason.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![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: 51cd1d77b5
ℹ️ 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".
| |---|---|---| | ||
| | **Report root** (`-OutputBase`) | `reports/` — the human-facing dated reports | `${user_config.report_dir}` when set to a non-empty path; if it is empty or still shows an unexpanded `${user_config.report_dir}` token (option unset), default to `$env:USERPROFILE\Documents\MachineHealth` | | ||
| | **State root** (`-StateBase`) | `state/` (history, latest snapshot, approvals), `logs/`, catalog overlay, custom checks, `TODO.md` proposals | `${CLAUDE_PLUGIN_DATA}` — the per-plugin data directory that survives plugin updates. If that token is unexpanded (running outside plugin context), default to `$HOME/.claude/plugins/data/machine-health` | | ||
| | **State root** (`-StateBase`) | `state/` (history, latest snapshot, approvals), `logs/`, catalog overlay, custom checks, `TODO.md` proposals | `${CLAUDE_PLUGIN_DATA}` — the per-plugin data directory that survives plugin updates — passed explicitly when the token expands. If it is unexpanded, **omit `-StateBase`** and let the orchestrator apply its own documented ladder (the `CLAUDE_PLUGIN_DATA` environment variable, else `-OutputBase`), then report which root it used. Never substitute a literal path: the directory under `~/.claude/plugins/data/` is named for the plugin's install identity, not the plugin, so a guessed path names a different directory and splits state from the overlay | |
There was a problem hiding this comment.
When the token is unexpanded—the exact scenario this branch handles—omitting -StateBase is unsafe: the repository's smoke test at docs/extensibility-contract-smoke-tests.md:77-82 found that a skill-invoked subprocess can inherit CLAUDE_PLUGIN_DATA for an unrelated plugin. Invoke-MachineHealthCheck.ps1:89-94 will then select that directory, causing the audit to read the wrong overlay/approvals and write machine-health state and logs into another plugin's data. Pass an explicit known fallback such as -OutputBase, or otherwise ensure the ambient variable is ignored.
Useful? React with 👍 / 👎.
Comment on lines
+48
to
+50
| legacy path and any `machine-health-*` sibling of the resolved `<StateBase>`, and for each that | ||
| exists and is not `<StateBase>`, name it and list what it holds. Only `<StateBase>` is read. | ||
| Consolidating is the operator's move — moving or deleting the stray root is a decision about their |
There was a problem hiding this comment.
When machine-health is installed from two marketplaces, or an inline session runs alongside an installed copy, the other legitimate machine-health-* data directory matches this glob and is reported as a split/stray root. Marketplace-scoped identities can coexist, as documented at docs/topics/dometrain-mcp/PLAN.md:601, so this diagnostic can incorrectly invite the operator to consolidate or delete another active installation's state. Restrict the migration check to the known unsuffixed legacy directory, or establish that a sibling identity is inactive before labeling it split state.
Useful? React with 👍 / 👎.
Comment on lines
+305
to
+307
| **Check-only carve-out.** Where a plugin's configuration surface contains no writable artifact, a | ||
| check-only setup is conforming: `check` verifies and reports, and no `apply` is offered because there | ||
| is nothing it could conformingly write. Three kinds of surface qualify, in any combination: |
There was a problem hiding this comment.
This new normative rule says a plugin with no writable artifact offers no apply, but the resulting tree still exposes guidance-only apply actions for exactly that shape—for example plugins/session-flow/skills/setup/SKILL.md:55-69 and plugins/claude-config/skills/setup/SKILL.md:45-57; the newly added claude-config eval even locks that action in. Those actions write nothing and merely repeat remediation already required from check, so setup implementations and contract audits now disagree about the canonical interface. Either migrate these skills to check-only or amend the carve-out to explicitly permit guidance-only apply.
Useful? React with 👍 / 👎.
This was referenced Jul 25, 2026
Merged
Contributor
Author
|
Superseded by #1377 — same work rebased onto current |
kyle-sexton
added a commit
that referenced
this pull request
Jul 25, 2026
… 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>
kyle-sexton
added a commit
that referenced
this pull request
Jul 25, 2026
…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>
kyle-sexton
added a commit
that referenced
this pull request
Jul 26, 2026
…ction-surface pass (#1318) 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 `/doctor` arrive 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-pass` hosts 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 - **Findings carry a content-derived identity**, never line-derived — a line number shifts when anything above it changes, which would churn the whole report on an unrelated edit and destroy exactly the property the contract exists to provide. - **Three determinism tiers.** *Derived* (no model in the path) carries exact equality; *judged* carries a stability tolerance whose breach fails the run's own self-check and is reported as an instability finding against the sweep itself; *delegated* is `/doctor` and carries neither. The earlier two-tier `mechanical`/`behavioral` split 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. - **The scan set is proven untouched.** A run demonstrates it never wrote into the surfaces it read. ## 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 --check` and `--check-bump`, `check-skill-portability.sh`, `check-skill-leaf-names.sh`, `check-silent-skips.sh`, `check-orphaned-fixtures.sh`, and `validate-plugin-contracts.mjs` all pass. `skill-quality:check` on `audit-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: - **Finding identity for *pairwise* findings.** I12 reports that two surfaces contradict each other, which has two anchors, not one. The resolved design is a canonically-sorted anchor **set** with the primary site outside the hash — an ordered pair would hash `X`-vs-`Y` differently from `Y`-vs-`X` and report one conflict twice. Confirm the shipped implementation matches. - **The suppression record must store constituents**, not a bare finding id — an id-only record cannot compute a partial match, so no carry-forward rule is implementable against it. - **Liveness is not a function of the tree.** Launch directory, `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-config` to **0.10.0**. #1315 carries a **0.9.3** for the same plugin that is not on `main` yet; whichever merges second needs the other's changelog entry inserted in order. This branch also adds a row to `docs/PLUGIN-PHILOSOPHY.md`'s conventions index, which #1315 and #1096 both edit. ## Related - #1315 — cross-plugin hygiene; overlapping `claude-config` version and `PLUGIN-PHILOSOPHY.md` - #1316 — the criteria payload this pass dispatches to - #1096 — also edits `docs/PLUGIN-PHILOSOPHY.md` - #1225 — a second sweep-shaped effort; reconciliation is an open row against this work 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Jul 26, 2026
…s and an OPINION tier (#1316) The criteria payload for the `audit-pass` effort: new checks in the two catalogs that already own their surfaces, rather than a new catalog. No linked issue. ## What lands - **`claude-config:audit-instructions` gains I12 and I13**, plus an `OPINION` tier for rules that ship marked and default-off. I12 is the cross-surface instruction-conflict check — the one finding in this effort with no incumbent. It is scoped by routing rather than by assertion: `claude-memory:audit`'s C6 already owns contradictions *within* the memory layer, so I12 takes cross-layer, non-memory, and managed-tier conflicts and does not duplicate it. - **`claude-memory:audit`'s C3 (Content Placement) gains three gaps closed in one revision.** They land as one edit rather than three checks, because the routing table answers one question and three checks would emit three findings for one misplaced section. ## Two corrections absorbed late, both worth calling out - **Imports are resolved before comparison.** A detector that reads `CLAUDE.md` without expanding `@path` imports is reading a different surface than the model sees — imported files are expanded into context at launch. Without this, every `@docs/foo.md` import is invisible to the check. - **`AGENTS.md` is excluded by reachability, not by filename.** Claude Code does not read a bare `AGENTS.md`, but [the official docs](https://code.claude.com/docs/en/memory) steer repositories toward `@AGENTS.md` or `ln -s AGENTS.md CLAUDE.md`, and **both make it live**. A filename-based exclusion would have silently dropped the conflict class in precisely the repositories following official advice. Confirmed empirically: with the import in place, `/context` enumerates `AGENTS.md` as its own loaded Memory Files row. The one empirical claim in the C3 revision is carried with explicit provenance rather than stated as documented behavior. ## Verification `check-changelog-parity.sh --check` and `--check-bump`, `check-skill-portability.sh`, `check-silent-skips.sh`, and `validate-plugin-contracts.mjs` pass on the rebased branch. `skill-quality:check` on `audit-instructions`: **PASS, 0 errors, 0 warnings** — 188/500 lines, description 666/1536 chars, all 6 base-ref trigger phrases preserved, markdownlint clean, and its script test passes. ## Merge-order note This branch takes `claude-memory` to **0.5.0**. #1315 carries a **0.4.1** for the same plugin that is not on `main` yet. Whichever merges second needs the other's changelog entry inserted in order — the resolution is mechanical, but it is not a clean auto-merge. ## Review focus The catalog text was written by one context and has not had an independent read. I12's surface list and the Type B allowlist are the two places where a fresh reviewer is most likely to find something: Type B conflicts (one absolute, one conditional, same act) ship **only** with a curated safety-critical allowlist, and that dependency is meant to be binding rather than advisory. ## Related - #1315 — the cross-plugin hygiene lane; same effort, overlapping `claude-memory` changelog - #1278 — `mcp-tools` tool-search coverage gap, scoped out of this effort - #1225 — a second sweep-shaped effort; reconciliation is an open row against this work 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Aug 12, 2026
…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>
cursor Bot
pushed a commit
that referenced
this pull request
Aug 12, 2026
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audits the fleet's
setup-skill corpus againstdocs/PLUGIN-PHILOSOPHY.md, fixes what it found, and corrects two shipped reference claims that empirical testing disproved.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.
userConfignon-triviality had no definition, leaving criterion (c) unfalsifiable and re-litigated by every audit. 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. An option is non-trivial when it names an external referent needing verification, carries no default preserving zero-config behavior, or is coupled to another option or to out-of-manifest state. Resolveseducation,repo-hygiene, andvisualizationat once.userConfig, which excluded 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 on the settings surface:checkprints the exact edit, fully resolved, states that it is the operator's to apply, and names what re-invalidates it. Resolvescontext-guardandrate-limit-guardtogether.Two reference claims that were false
claude-memory's path-scoping status. The reference asserted that.claude/rules/files load unconditionally regardless ofpaths:frontmatter. A first-party repro on Claude Code 2.1.219 disproved it — a rule scopedpaths: ["**/*.tsx"]was absent at session start, present after reading a matching.tsxfile, and absent again after reading a non-matching one. The cited evidence failed independently too: two of the four issues are closed NOT_PLANNED and never supported the claim, and the two still open assert opposite failure modes. No version floor is claimed, because nothing pins when the behavior changed. The caveats that survive are kept and each is sourced.CLAUDE.md / AGENTS.mdcompaction row. Claude Code does not readAGENTS.md— official docs say so directly, and it is absent from/context's Memory Files enumeration even with aCLAUDE.mdbeside it.The rest of the ranked list
githubapply clauses ·ai-briefingreconfiguration path ·session-flowandrate-limit-guardheadless--confignote ·context-guard'sapply resetrenamed toapply defaultsso it stops colliding with the token the owner doc defines as teardown-plus-apply ·discovery+verificationschema reference made publisher-neutral · the repo-level doc citation that cannot resolve from an installed plugin cache dropped from both guard plugins ·machine-healthusing the data-directory anchor instead of a hardcoded home path ·claude-config:setup's missing evals added, since it is the audit's own host plugin.Verification
check-changelog-parity.sh --checkand--check-bump,check-skill-portability.sh,check-silent-skips.sh,check-skill-leaf-names.sh,check-cross-plugin-source-drift.sh,check-orphaned-fixtures.sh, andvalidate-plugin-contracts.mjsall pass on the rebased branch.Deliberately not done
discoveryandverification. Extract-and-register as a byte-identical cluster versus accept the drift with a recorded reason is an open call, tracked separately. Note the drift checker clusters on full path-within-plugin, which may make registration unavailable here regardless.Related
mcp-toolstool-search coverage gap, found during the same auditplaybooks:borisandwork-items:track, found during the same auditdocs/PLUGIN-PHILOSOPHY.md; whichever merges second rebases🤖 Generated with Claude Code