fix(setup): close the setup-corpus audit's findings and two falsified reference claims - #1377
fix(setup): close the setup-corpus audit's findings and two falsified reference claims#1377kyle-sexton wants to merge 12 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>
…r than share a source
`discovery`'s and `verification`'s setup skills are byte-identical across 61
lines (`discovery` 21-81, `verification` 25-85, sha256 726a8fb4...), and nothing
on either page said whether that is a shared source awaiting extraction or a
coincidence to leave alone. A reader can only re-litigate it or "deduplicate"
two skills that must stay free to diverge.
The path-shape question came out two ways, and both matter. In place,
registration is impossible -- but not for the reason that ruled out the criteria
catalogs. `SKILL.md` is in `check-cross-plugin-source-drift.sh`'s
`skip_basenames`, so its containing file can never form a cluster, and the
checker hashes whole files, never line ranges. After extraction, registration
WOULD be available: both skills are literally named `setup`, so a
`skills/setup/context/<name>.md` is the same path-within-plugin in both and a
cluster forms. The differing-skill-name objection that gave the criteria
catalogs zero clusters does not apply here. Registration is therefore available
only at the cost of creating the very artifact in dispute, so the decision rests
on merits.
Accepted the drift, on three grounds. First, ownership: the block substantially
restates rules two owner docs already carry -- this contract (defaults, the
resolution order's inference rung and its "prose is an inference source" rule,
the committed-tier guard down to the representative-path detail, the root
`.gitignore` prohibition, "ask once, recommended option first", "preserve and
offer every schema key -- a re-run never drops one", the GitBook deferral) and
PLUGIN-PHILOSOPHY's setup contract (check/apply, idempotent and safe to rerun,
non-interactive on complete arguments, state-assessing `apply`). A shared skill
fragment would be a second owner for those rules, against the convention
registry's one-owner-per-concern rule. Restating is what a `SKILL.md` must do:
it is the instruction surface a session loads, and it cannot defer at runtime to
a document the consuming repo does not have.
Second, the exceptions prove the same point. Three parts of the block are
plugin-authored, not contract-derived -- "always write at least one explicit
key" (stricter than the schema, which has no `required` and admits `{}`),
verify-or-create of the memory-root `.gitignore` (this contract assigns the
self-ignore guard to the session's first memory-tier write), and the Output
paragraph. `planning`, the third implementer of the same seam, contradicts the
first two outright and renders everything else in its own prose. Freezing two of
three byte-identical would ratify rules the contract does not own and a sibling
rejects.
Third, mechanism fit. Every registry entry today is a canonical artifact
vendored from one source with a dedicated sync script; `docs/conventions/shell-test-helpers`
names that `lib/` + `sync-*.sh` + registry path as this marketplace's sanctioned
way to share source, and reserves it for clusters meant to stay byte-identical.
Registering skill prose instead makes byte-identity a gate, so the next
legitimate divergence -- the kind `planning` already shows -- fails CI.
Recorded in the owner doc rather than a new one: the concern already has an
owner, and a second doc about it would be the duplication being argued against.
The reason, the live evidence, and the reopen trigger (a canonical `lib/` source
with a `sync-*.sh`, at which point extraction is the smaller change and
registration follows it) live in one new subsection under Implementers; minor
per the contract's own Versioning rule, since this is additive guidance with no
tier, key, slug-spec, or visibility change.
The per-site pointers are HTML maintainer notes in the two `SKILL.md` files,
placed at the block boundary where a maintainer would otherwise reach for the
delete key. They name the contract's section rather than a repository path:
commit 3568a8d05 had just closed the second upstream-coupling site in each of
these skills, and citing `docs/conventions/...` from a shipped skill would
reopen one.
Two divergences surfaced during the coverage check and are deliberately left
alone -- each is a behavior conflict needing its own decision, not a drift note:
`discovery` and `verification` have setup create the memory-root `.gitignore`
while the contract assigns that to the first memory-tier write and `planning`
explicitly disclaims it, and the two demand an explicit key where the schema and
`planning` accept `{}`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 5s —— View job Security Review — PR #1377
Scope reviewed: 38 changed files — No security vulnerabilities found. The changes divide into four categories, each assessed below. Documentation and instruction files (
|
|
Claude finished @kyle-sexton's task in 5m 9s —— View job PR Review — fix(setup): close the setup-corpus audit's findings and two falsified reference claims
SummaryThis is a well-structured, carefully reasoned PR. The core fixes are correct, the evidence for the two reference-claim corrections is documented, and the internal consistency across SKILL.md / eval / argument-hint / CHANGELOG is solid throughout. The version laddering for the three rebase collisions (discovery 0.8.4, session-flow 0.15.2, rate-limit-guard 0.2.1) is handled cleanly. Findings1. machine-health split-state-root prose implies the glob covers the legacy path when it doesn't
The legacy path is
2. Four un-changed plugins describe themselves against the old, narrower carve-out framing Not introduced by this PR — these are out-of-scope — but worth tracking as a follow-up. The four plugins below still say "userConfig-only carve-out" in their setup skill Purpose sections, which is now stale relative to the widened definition in PLUGIN-PHILOSOPHY ("no writable owned artifact"):
All four are still technically conforming (their surfaces fit the new definition), but a reader matching their prose against the updated philosophy will find inconsistency. This warrants a targeted follow-up PR. What holds up well
Approved with the note that Finding 1 is a low-risk prose clarification and Finding 2 is a deferred follow-up. |
![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: 90ce686a4f
ℹ️ 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 ${CLAUDE_PLUGIN_DATA} remains unexpanded, omitting -StateBase does not reliably fall back to this plugin's storage: Invoke-MachineHealthCheck.ps1 next reads the subprocess's CLAUDE_PLUGIN_DATA, while docs/extensibility-contract-smoke-tests.md documents that a skill-invoked tool subprocess can inherit an unrelated installed plugin's data directory. In that context, an audit writes its generic state/, logs/, and catalog files into another plugin's directory. Clear that environment fallback, explicitly use the report root, or stop when the plugin-specific root cannot be resolved.
Useful? React with 👍 / 👎.
| So a headless reconfigure is `claude plugin uninstall session-flow` then `claude plugin install | ||
| session-flow@<marketplace> --config <key>=<value> ...`, supplying every key to change in the one | ||
| install. |
There was a problem hiding this comment.
For a headless user who has customized multiple observer options, supplying only every key being changed loses the unchanged custom values: uninstalling removes the stored pluginConfigs options, and omitted values then fall back in observer-arm.sh to the manifest defaults. For example, reinstalling only to enable the observer resets a custom model, idle threshold, bare mode, and maximum lifetime. The reinstall guidance should require every desired non-default option, not merely the keys changed in this operation.
Useful? React with 👍 / 👎.
|
Superseded by #1393 — rebased onto current |
… 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>
…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
Supersedes #1315, which could not be rebased in place. Same work, rebased onto current
mainwith three version collisions resolved.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. 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. Resolveseducation,repo-hygiene, andvisualizationat once.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:checkprints the exact edit, 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.claude/rules/files load unconditionally regardless ofpaths:. A first-party repro on 2.1.219 disproved it — a rule scopedpaths: ["**/*.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. No version floor is claimed, because nothing pins when the behavior changed.CLAUDE.md / AGENTS.mdcompaction row. Claude Code does not readAGENTS.md— official docs say so, 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 defaults·discovery+verificationschema reference made publisher-neutral · the repo-level doc citation that cannot resolve from an installed cache dropped from both guard plugins ·machine-health's data-directory anchor and its audit-fallback/stop-rule corrections ·claude-config:setup's missing evals.Ranked fix 4 — decided, and the answer is "accept the drift"
The 61-line block shared by
discoveryandverificationis left duplicated, with the reason recorded at the owner doc. The path-shape question came out both ways: in place, registration is impossible becauseSKILL.mdis in the drift checker'sskip_basenamesand the checker hashes whole files, never line ranges; after extraction it would work, since both skills are namedsetupso the path-within-plugin matches. Registration is therefore 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, against the one-owner-per-concern rule. Restating is what a
SKILL.mdmust do — it is the surface a session loads and cannot defer at runtime to a document the consuming repo does not have. Three parts of the block are plugin-authored rather than contract-derived, andplanning, a third implementer of the same seam, contradicts two of them outright — so freezing two of three would misrepresent a live divergence as settled.Rebase resolutions
Three version collisions with
main, each resolved by laddering rather than clobbering:discovery0.8.3 → 0.8.4,session-flow0.15.1 → 0.15.2,rate-limit-guard0.1.1 → 0.2.1. In every casemain's entry is preserved intact above ours in the changelog.Verification
check-changelog-parity.sh --checkand--check-bump origin/mainpass, andvalidate-plugin-contracts.mjspasses (43 setup skills, 2044 plugin files). The remaining gates ran clean on this content before the rebase; CI runs the full set here.Related
audit-passskill🤖 Generated with Claude Code