fix(setup): close the setup-corpus audit's findings and two falsified reference claims - #1393
Merged
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>
Five citations in setup's SKILL.md named the audit skill's reference
files by a bare relative path — `references/shared/approvals.md` and
siblings — which resolve to nothing from the setup skill's own
directory. `skill-quality:check` reports three of them as broken
skill-internal refs, and the gate only surfaced them here because this
PR touches the file.
The correct form was already in the same document twice, at the
catalog-overlay citations: `${CLAUDE_PLUGIN_ROOT}/skills/audit/
references/…`. The remaining five now match it, so every reference in
the file resolves the same way and none depends on which skill directory
the reader happens to be in.
Not a relocation: the files stay where they are, under the audit skill
that owns them. Setup reads them because approvals and the output schema
are the audit's contracts, not setup's — copying them would create a
second owner for both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both are the same shape: a documented fallback that looks like a safe
default and is in fact a silent wrong answer.
machine-health's audit told the caller to omit `-StateBase` when
`${CLAUDE_PLUGIN_DATA}` does not expand, letting the orchestrator apply
its environment ladder. 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 data
directory. The guidance now passes the report root explicitly and reports
that the plugin-specific root could not be resolved. Colocating state
with the reports is wrong-but-visible; the inherited variable is
wrong-and-silent, and only one of those is recoverable by a reader.
session-flow's setup told a headless consumer to supply "every key to
change" on the reinstall. Uninstalling drops the stored `pluginConfigs`
entry, so every key omitted from the reinstall falls back 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, not
every key being changed, and `check` must be run BEFORE the uninstall
because afterwards there is nothing left to read the current values from.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 4m 1s —— View job PR Review: fix(setup): close the setup-corpus audit's findings and two falsified reference claims
|
|
Claude finished @kyle-sexton's task in 1m 43s —— View job Security Review — PR #1393
FindingsNo security issues found. Scope reviewed: All 38 changed files across 10 plugins and 3 docs paths. The PR is exclusively documentation and instruction prose — SKILL.md instruction texts, reference markdown, eval test scenarios (JSON), plugin version bumps, and CHANGELOG entries. No executable code, no scripts, no workflow files, no dependency manifests were modified. Specific checks performed:
|
![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: e24305361f
ℹ️ 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".
…ew threads Resolves the session-flow collision: main shipped 0.17.0 (the find-handoff skill, thirteen skills in the manifest description) while this branch was bumping 0.15.2 -> 0.15.3. Takes main's description and re-ladders this branch's change onto it as 0.17.1, with the CHANGELOG entry moved to match. Carries the three review findings against this branch, each verified against the repo's own contracts before being accepted: - The headless reconfigure recipe this branch introduced to session-flow and rate-limit-guard passed neither `-s <scope>` nor `-y`. Both `claude plugin uninstall` and `claude plugin install` default to `-s user` (confirmed from the CLI's own --help on 2.1.220), so on a project- or local-scoped install the pair removed a separate user record and reinstalled at a scope that does not load; and converge.md records that a non-TTY uninstall requires `-y`, so the recipe stalled in exactly the headless case it exists for. Both halves now take the scope, the uninstall takes the confirmation, and `claude plugin list` is named as where the installed scope is read. The same defect sits in 19 other setup skills, filed as #1406 rather than widened into this branch. - machine-health's audit skill cited docs/extensibility-contract-smoke-tests.md, a repository path absent from the isolated plugin cache the skill runs from -- a plugin-form-safe violation under CLAUDE.md's "reference only files inside the plugin" rule, and one that made the evidence for a safety-critical fallback unreachable at runtime. The mechanism is stated inline instead. Also fixes a drift this branch introduced into its own record: machine-health's 0.7.0 CHANGELOG still said audit "omits -StateBase", which a later commit on this branch had already reversed to passing it explicitly. 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 36s —— View job PR Review: fix(setup): close the setup-corpus audit's findings and two falsified reference claims
This is a correctness-focused PR with no executable code — all changes are SKILL.md instruction prose, reference markdown, evals JSON, CHANGELOG entries, and Finding 1 — Three CHANGELOG entries are undated (nit)
Finding 2 — eval #2 names an internal script file (low, carry-over from prior review)
The expectation reads:
This is a claim about a specific script file's internals. If the script is renamed or another audit script gains a curl dependency, the eval passes its expectation check while the assertion is silently wrong. Phrasing it as a behavioral claim — "Names the one audit check that requires curl and states the others still run" — would keep the eval stable under internal script changes while still verifying the scoping behavior. This matches the other three expectations in that eval, which are all behavior-framed. Finding 3 —
|
|
Claude finished @kyle-sexton's task in 2m 9s —— View job Security Review — PR #1393
FindingsNo security issues found. Scope reviewed: All 38 changed files — SKILL.md instruction texts, reference markdown, eval test scenarios (JSON), plugin version bumps, CHANGELOG entries, and one owner doc (PLUGIN-PHILOSOPHY.md). No executable code, scripts, workflow files, or dependency manifests were modified. Specific checks:
|
kyle-sexton
added a commit
that referenced
this pull request
Jul 25, 2026
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
This was referenced Jul 25, 2026
Merged
kyle-sexton
added a commit
that referenced
this pull request
Jul 26, 2026
… (18 plugins) (#1411) *This was generated by AI during work-loop execution.* ## Summary - Both halves of the headless reconfigure recipe (`claude plugin uninstall <p>` then `claude plugin install <p>@<marketplace> --config ...`) defaulted to `-s user`. When a plugin is actually installed at `project` or `local` scope, that combination silently uninstalls a *separate* user-scope record while the effective project/local install keeps loading, and the reinstall lands at a scope that does not load. - Both commands now carry `-s <scope>`, sourced from what `claude plugin list` reports per plugin — the same fix already merged for `session-flow` and `rate-limit-guard` in #1393. - 18 plugins patched, each with a patch version bump and a CHANGELOG entry: `actionlint`, `bash-format`, `biome-format`, `claude-ops`, `desktop-notification`, `disk-hygiene`, `eol-normalizer`, `go-format`, `guardrails`, `knowledge`, `machine-health`, `markdown-format`, `planning`, `powershell-format`, `ruff-format`, `skill-quality`, `source-control`, `typos-format`. ## `ai-briefing` excluded #1406 named `ai-briefing` as one of the 19 remaining plugins, but it carries no reconfigure recipe of this shape at all — no `uninstall`, `--config`, `headless`, `reconfigur`, or `/plugin configure` anywhere in the plugin (verified by search across the whole plugin directory). There is nothing to fix there for this defect; its `active_profile` userConfig having no headless-reconfigure guidance at all is a different, unrelated gap. ## Does not add `-y` to the uninstall — verified against the live CLI #1406's second claimed defect was that a non-TTY `claude plugin uninstall` requires `-y`. I live-tested this against `claude` 2.1.220 (the exact version #1406 cites) before propagating it to 18 more files: - Current fetched docs and live `--help` both scope `-y` to skipping the `--prune` confirmation only. - Empirically: created a throwaway local marketplace + dummy plugin, installed at `-s local`, then ran `claude plugin uninstall <plugin> -s local < /dev/null` (non-TTY, no `--prune`, no `-y`) — exit 0, no prompt. Repeated with a populated `${CLAUDE_PLUGIN_DATA}` directory (triggers last-scope data deletion) — same result, silent success as documented. None of these recipes pass `--prune`, so `-y` would be a harmless no-op attached to a false rationale. I filed #1410 to track the existing overbroad claim in `converge.md` and the two already-merged setup skills (`session-flow`, `rate-limit-guard`) rather than silently leaving 18 files disagreeing with 2 + a reference doc. ## Verification - `markdownlint-cli2` (repo config): 0 issues across all 36 touched markdown files. - `skill-quality:check` (`check-skill.sh`) on all 18 touched `skills/setup`: 18/18 PASS — no new FAILs, no new WARNs (pre-existing WARNs unrelated to this change: missing Gotchas surface on several, `source-control`'s pre-existing soft line-count target). - All 18 `plugin.json` files validated as parseable JSON post-bump. - Grep-verified: every touched file's `uninstall`/`install --config` pair now carries `-s <scope>`; no unscoped recipe remains among the 18. ## Related - Closes #1406 - #1393 — the reference fix this mirrors (`session-flow`, `rate-limit-guard`) - #1410 — follow-up: reword the overbroad `-y`-required-for-non-TTY-uninstall claim in `converge.md` and the two `#1393`-merged setup skills ## Test plan - [x] `markdownlint-cli2` clean on all touched files - [x] `skill-quality:check` PASS on all 18 touched setup skills - [x] JSON-valid `plugin.json` for all 18 bumps - [x] Manual grep sweep confirms no unscoped recipe remains - [ ] CI green (pending) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
kyle-sexton
added a commit
that referenced
this pull request
Jul 26, 2026
…sion-flow, rate-limit-guard) (#1443) *This was generated by AI during work-loop execution.* ## Summary - The headless-uninstall recipes in three places claimed `-y` is CLI-required for any `claude plugin uninstall` whose stdin/stdout isn't a TTY. That claim is overbroad. - Verified against `claude` 2.1.220 (the version #1406/#1410 both cite): current fetched docs, live `--help`, and an empirical uninstall test (throwaway local marketplace + dummy plugin, non-TTY, no `--prune`, no `-y` → exit 0, no prompt) all agree `-y` only skips `uninstall`'s `--prune` confirmation. - None of the three recipes touched here pass `--prune`, so `-y` was a no-op attached to a false rationale. Removed it and reworded the claim to the narrower, verified scope in all three sites: - `plugins/claude-ops/skills/plugins/context/converge.md` (`converge`'s non-interactive-execution note) - `plugins/session-flow/skills/setup/SKILL.md` (headless reconfigure recipe, merged in #1393) - `plugins/rate-limit-guard/skills/setup/SKILL.md` (headless reconfigure recipe, merged in #1393) - Patch version bump + CHANGELOG entry for each of the three touched plugins (`claude-ops` 0.21.1 → 0.21.2, `session-flow` 0.17.1 → 0.17.2, `rate-limit-guard` 0.3.0 → 0.3.1). ## Why this is safe `-y`'s only documented and observed effect is skipping a confirmation prompt these recipes never trigger (none pass `--prune`), so dropping it changes no runtime behavior — this is a pure doc-accuracy correction, consistent with #1406's sibling fix (#1411), which deliberately did not propagate `-y` or this rationale into the 18 other plugins it touched. ## Verification - `markdownlint-cli2` (repo config): 0 issues across all 6 touched markdown files. - `skill-quality:check` (`check-skill.sh`) on both touched `skills/setup`: 2/2 PASS — no new FAILs, no new WARNs (`rate-limit-guard`'s pre-existing "no Gotchas surface" WARN is unrelated to this change). - All 3 `plugin.json` files validated as parseable JSON post-bump. - Grep-verified: no remaining `-y`/TTY reference in any of the three touched files beyond the corrected sentences. ## Related - Closes #1410 - #1406 / #1411 — the sibling fix this follow-up completes (18 plugins, deliberately excluding this claim) - #1393 — the original merge point for the `session-flow`/`rate-limit-guard` recipes this corrects ## Test plan - [x] `markdownlint-cli2` clean on all touched files - [x] `skill-quality:check` PASS on both touched setup skills - [x] JSON-valid `plugin.json` for all 3 bumps - [x] Manual grep sweep confirms no stray `-y`/TTY claim remains - [ ] CI green (pending) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Sonnet 5 <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.
Supersedes #1377 (which superseded #1315). Same work, rebased onto current
mainwith 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.
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.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.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-healthtold the caller to omit-StateBasewhen${CLAUDE_PLUGIN_DATA}does not expand. But a skill-invoked tool subprocess can inherit an unrelated installed plugin'sCLAUDE_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-flowtold a headless consumer to supply "every key to change" on reinstall. Uninstalling drops the storedpluginConfigsentry, 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, andcheckmust 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 failskill-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.mdis in the drift checker'sskip_basenamesand the checker hashes whole files, never line ranges. After extraction it would work, since both skills are namedsetup. 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.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 are plugin-authored rather than contract-derived, andplanning, a third implementer, contradicts two of them outright.Rebase resolutions
Five version collisions with
main, each laddered rather than clobbered, withmain's entry preserved intact above ours:discovery→ 0.8.4,session-flow→ 0.15.3,rate-limit-guard→ 0.2.1, plusclaude-configat 0.9.3 andclaude-memoryat 0.4.1, which are the first rungs of the ladder #1316 and #1318 build on.Related
claude-config0.10.0)audit-passskill (claude-config0.11.0)🤖 Generated with Claude Code