fix(architecture,conventions): correct the CLAUDE_PLUGIN_DATA substitution rationale and adopt upstream-drift - #2299
Conversation
…ution rationale and adopt upstream-drift
`improve`'s Gotcha asserted that `${CLAUDE_PLUGIN_DATA}` "does not substitute in
skill markdown content (it is a hook/monitor/MCP path substitution only)". It
does substitute: the plugins reference's per-component table puts skill and
agent content in the "anywhere the placeholder appears" row alongside hook and
monitor commands, with no version qualifier (re-fetched 2026-08-12 UTC over the
raw-markdown channel, 95,338 bytes, SHA-256 f6627de3…).
The routing rule the sentence defended is unchanged and still correct — it rests
on where the path points, not on whether it expands: no project dimension,
deleted on last-scope uninstall, documented for deps/caches/generated code. The
corrected sentence lands as a conforming upstream-drift record with basis, UTC
as-of date, and a divergence-at-fetch recheck trigger.
The 0.3.6 CHANGELOG entry carrying the same false reason is left exactly as it
shipped; an erratum under 0.5.1 points forward instead. It does not claim the
0.3.6 symptom was imaginary — only its stated mechanism was wrong.
Fleet half: `architecture` joins the upstream-drift Adopters table (1.4.0), the
table preamble now distinguishes on-touch adoptions from the 1.0.0 migration,
and a CI gate for adoption is deferred as a recorded decision — the check the
doc already named would not have caught this case, because the surface carried
no stamp for a stamp-anchored grep to match.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![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: a39d400e9d
ℹ️ 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".
| Observed failure history — patterns that have actually bitten. Add here when a new one surfaces. | ||
|
|
||
| - **The durable candidate artifact is a per-project memory-tier file, never `${CLAUDE_PLUGIN_DATA}`.** That token does not substitute in skill markdown content (it is a hook/monitor/MCP path substitution only), and even resolved it points at a plugin-global dir that collides candidates across projects. The artifact resolves through the marketplace topic-docs convention (the plugin's topic-docs [binding](../../reference/topic-docs.md)) — memory tier, default `.work/<topic-slug>/`. A `${CLAUDE_PROJECT_DIR}/.claude/...` path is also wrong: `.claude/` generated output is reserved for observability, and an unignored artifact there leaks scan output into git. | ||
| - **The durable candidate artifact is a per-project memory-tier file, never `${CLAUDE_PLUGIN_DATA}`.** That token **does** resolve in skill content — the plugins reference puts skill and agent content in the "anywhere the placeholder appears" row alongside hook and monitor commands (<https://code.claude.com/docs/en/plugins-reference>, §Environment variables, fetched 2026-08-12 UTC; recheck trigger: a read-time re-fetch of that section finds the "Skill and agent content" row changed or removed). The rule rests on **where the path points**, not on whether it expands: the same page resolves the token to `~/.claude/plugins/data/<id>/` — a plugin-global directory with no project dimension, so candidates from every codebase collide in one place — records that uninstalling from the last remaining scope deletes that directory by default, and names its use as installed dependencies, generated code, and caches. The artifact resolves through the marketplace topic-docs convention (the plugin's topic-docs [binding](../../reference/topic-docs.md)) — memory tier, default `.work/<topic-slug>/`. A `${CLAUDE_PROJECT_DIR}/.claude/...` path is also wrong: `.claude/` generated output is reserved for observability, and an unignored artifact there leaks scan output into git. |
There was a problem hiding this comment.
The trigger only fires when the "Skill and agent content" substitution row changes, but this same record also relies on three separate upstream facts: the resolved directory, last-scope uninstall deletion, and the directory's documented uses. If any of those change while the substitution row remains intact, the trigger never fires and the rationale becomes stale despite being advertised as a conforming four-part record. Expand the trigger to cover divergence in every upstream fact restated here, consistent with docs/conventions/upstream-drift/README.md:64-71 and 104-111.
Useful? React with 👍 / 👎.
| | [MIGRATION-PLAYBOOK](../../MIGRATION-PLAYBOOK.md) decision records | "Revisit trigger", and "Re-trigger" on the plugin-acceptance review record | Mixed — the dated component-decision records cite upstream bases and conform; the org-internal records (e.g. the ratification and plugin-acceptance review records) are named triggers; the skill-quality retrofit record is a third kind, terminal exclusions that state "no recheck trigger" by design — decided out, so nothing fires. | | ||
| | [ecosystem-commands](../ecosystem-commands/README.md) task-runner deferral | "Revisit triggers" | Named triggers only — an undated in-repo deferral; not a four-part record. | | ||
| | [topic-docs](../topic-docs/README.md) §Implementers restate the rules | "What would reopen it" | Named trigger only — an in-repo source-hoisting decision; not a four-part record. | | ||
| | [`architecture`](../../../plugins/architecture/skills/improve/SKILL.md) `improve` §Gotchas — *adopted on touch, added 1.4.0* | bare prose: an upstream substitution claim with no basis, no date, and no trigger | One conforming record — the `${CLAUDE_PLUGIN_DATA}` bullet cites `plugins-reference` §Environment variables with a UTC fetch date and a divergence-at-fetch trigger naming the specific table row the claim rests on. It is the plugin's only surface restating an upstream specific (swept at adoption), so the row covers the plugin, not one bullet. | |
There was a problem hiding this comment.
The claim that this is the plugin's only upstream-specific surface is contradicted by plugins/architecture/skills/improve/actions/deepening.md:26, which directs consumers to the upstream-owned Agent tool syntax subagent_type=Explore without a basis, date, or trigger. Because this row explicitly says it covers the whole plugin rather than the linked bullet, readers are told the plugin was swept and conforms when another durable operational surface remains unstamped; either scope the row to the bullet or disposition the other upstream-specific surfaces before making the plugin-wide claim.
Useful? React with 👍 / 👎.
1 participant
Summary
Two halves of one finding: a false harness fact on an authoritative surface (
architecture), andthe fleet convention that should have made it surface and did not (
upstream-drift).The corrected mechanism
plugins/architecture/skills/improve/SKILL.md:71asserted that${CLAUDE_PLUGIN_DATA}"does notsubstitute in skill markdown content (it is a hook/monitor/MCP path substitution only)". That is
false. The plugins reference's per-component substitution table puts skill and agent content in
the "anywhere the placeholder appears" row, alongside hook and monitor commands, with no version
qualifier.
Re-fetched at rung 1 per
docs/conventions/upstream-drift/§"Reading the basis":Verbatim,
plugins-reference.md:677-683:The rule is unchanged and still correct. Only its stated reason was wrong. The rewritten bullet
rests it on where the path points rather than on whether it expands — three legs the same fetch
confirms:
:709— resolves to~/.claude/plugins/data/{id}/. No project dimension, so candidates fromevery codebase collide in one directory.
:1012— "By default, uninstalling from the last remaining scope also deletes the plugin's${CLAUDE_PLUGIN_DATA}directory.":669— named use is "Installed dependencies such asnode_modulesor Python virtualenvironments, generated code, and caches".
The issue marked the second and third of those "auditor-quoted only and not independently
re-fetched". This fetch discharges that caveat — all three are now primary-read verified.
What was deliberately not done
rationale it shipped with was wrong.
CHANGELOG.md:112-115(the 0.3.6 entry) is not edited. Shipped history is never rewritten, so0.5.1 adds an erratum pointing forward. Mechanically verified: the changelog diff is
38 insertions(+), 0 deletions(-).a
Readreturns literal bytes whatever the loader substitutes, and 0.3.5 carried the token in aRead-loadedactions/*.mdstep — is stated as unsettled and out of scope, not resolved.The fleet half (#2273), all three rows dispositioned
architectureadded to the Adopters table. The table preamble said "Migrated at thiscontract's 1.0.0", which an appended row would have quietly falsified; it now distinguishes the
eight migrated rows from later on-touch adoptions, requires each later row to name the release
that added it, and states that a row is added only once the surface conforms.
architecturequalifies: a repo-wide grep shows
SKILL.md:71was the plugin's only surface restating anupstream specific, so the row covers the plugin rather than one bullet.
this lane's fence — are recorded in chore(plugins): 12 unstamped upstream harness claims across 11 plugins — upstream-drift adoption backlog #2297 with quoted lines, a per-plugin tally, the files
checked and found conforming, and the sweep's own limitation (a file-level citation gate
under-represents mixed files). They are deliberately not added to the Adopters table: that
table's third column is a promise to the reader, so tabling a surface known to be unstamped
would be the same false-claim class this PR is fixing.
finding is that the candidate check the doc already names — flag a
Verified <date>with notrigger — would not have caught this case: the surface carried no stamp at all, so a
stamp-anchored grep had nothing to match. It is shaped for a half-conforming record; the failure
that ships is the zero-part one. Separating an upstream restatement from an in-repo fact is a
judgment about meaning (reasoning-only under the tiers doc), and a harness-vocabulary grep would
fire on every correct citation too. Recorded with a basis and an event trigger — a third
unstamped carrier reaching
main, or a demonstrated detector needing no suppression list — sothe deferral expires on evidence, not on a date.
Versions
architecture0.5.0 -> 0.5.1 (patch). The operative rule and every procedural step areunchanged; only the stated rationale is corrected and stamped.
upstream-drift1.3.0 -> 1.4.0 (minor). Additive guidance per the contract's own rule; norequired part, canonical name, or enforceability verdict changed.
Test plan
Erratum constraint, checked mechanically rather than asserted:
Zero deletions: the 0.3.6 and 0.4.1 entries keep the wording they shipped with.
This change is documentation-only — no script, hook, or executable surface is touched, so no
trust surface is widened and no security review note applies.
Related
20260811-021645-plugin-audit-four-components-and-guard-deadlock-ownership(ledger
I9-021645-four-components.md§ Lane E), and its fleet-level routing recommendation