fix(plugin-security): a capability name collision now reaches the author

[claude]

Fixes #18023

Clause-②: yes

A package's declared capability was dropped on a capability-name collision and the drop reached nobody. The branch reported through logger?.warn?.(…) — optionally chained twice — so a caller that passed no logger produced no output at all, and the declaration vanished with one internal counter incremented. This module's own header called such a row "skipped loudly"; nothing about it was loud.

⛔ The skip is unchanged. Refusing to write into a sys_capability row another package owns is correct under ADR-0086 D4, and every case below asserts it still happens, still counts, still leaves the foreign row untouched, and still reports the name as materialized so the back-compat derivation does not clobber that row (#4967 Part 1). What changed is that the refusal is now legible to the author who caused it.

Reproduced first, then measured again

Both runs drive a real collision through bootstrapDeclaredCapabilities with no logger passed, spying on all five console channels. The permission-set axis runs beside it in the same harness as the lit control.

	before (57343f761, pre-change)	after (9cbca2136)
skippedForeign	1	1 — unchanged
author-visible console lines	0	1
diagnostic records on outcome	undefined	1
LIT CONTROL — permission-set axis, same harness	1	1
DISCRIMINATING CONTROL — non-colliding pass that really seeds, updates, claims and leaves a row unchanged	—	0 lines

The after line, verbatim:

warn: [security] [capability_name_collision] 1 declared capability was NOT applied — another
package already owns that capability name (ADR-0086 D4). The declaring package's authored label,
description and scope are NOT in effect, and the capability is attributed to the owning package.
{"event":"capability_name_collision","collisions":[{"name":"shared_cap","declaredBy":"com.example.b",
"ownedBy":"com.example.a","grantedBy":[],"fix":"Rename the capability in package \"com.example.b\" …"}]}

The design choice: a parallel module, not an extension, and not a copy

The card asked for "its own diagnostic record type, its own key on the capability seeder's outcome, and its own pins — a second design rather than a mechanical copy". Measured against the shared machinery on main, four of the six things a collision report is made of are axis-specific here — the token, the record type, the wording and the consequence — so they live in a new capability-name-collision.ts. Folding them into a file named permission-set-name-collision.ts would make that name lie, and renaming that file churns pins that landed one card ago.

The consequence really is different, and that is the reason this is not a copy. On the permission-set axis the entire declared set is not materialized and none of its permissions are in effect. Here the name still resolves — the owning package's row answers for it. What is lost is narrower, and the diagnostic says so precisely: the declaring package's authored label / description / scope are not applied, and sys_capability.package_id attributes the capability to the other package, so the declaring package has no provenance claim over it. A message borrowing the sibling's wording would send the author hunting a broken grant that is not broken; a pin asserts that it does not (expect(d.message).not.toContain('none of its object, field, tab')).

⛔ What is axis-independent is reused, never re-derived — that is the drift this card exists to avoid:

• permissionSetNameIsForeign — THE owner-comparison predicate. The seeder's branch now routes through it instead of its own existing.package_id === packageId, so a nullish owner reads FOREIGN on both axes by construction (ADR-0086 D3), and a pin exercises exactly that row shape.
• CollisionReportSink — the minimal sink type, imported from the sibling and re-exported rather than structurally re-declared.

reportPermissionSetNameCollisions is deliberately not reused: its message states a permission-set consequence. Its sink discipline is restated, because that discipline is the contract this card buys rather than a derivation that can drift — and both axes pin it independently, including the (logger?.warn ?? console.warn)(…) receiver-detach trap (a class-based host sink that reaches for this is injected and must not throw).

reportPermissionSetNameCollisions's judgement call, which the dispatch left to this lane: it does not generalise. It takes readonly PermissionSetNameCollisionDiagnostic[] and words a permission-set sentence, and an axis parameter threaded through it would produce one function that words two different consequences — the shape that makes both wrong.

Reverse verification

Fix committed first (aafc3ce26), then the branch mutated back to the old logger?.warn?. form and the pass-end report deleted, on disk, from the committed state:

BASELINE HEAD blob hash = 3cb4ec4a3ce5d6983fa6ca5e3fda13bd0e8ca926
anchor counts BEFORE:  capabilityNameCollisionDiagnostic({ = 1   logger?.warn?.('[security] capability name owned = 0
anchor counts AFTER:   capabilityNameCollisionDiagnostic({ = 0   logger?.warn?.('[security] capability name owned = 1
MUTATED  blob hash     = 0b2ec0f476ccad54c39d8fa003dfd256081da91f   (differs ⇒ the mutation really landed)
→ Tests  4 failed | 39 passed (43)
restore: git checkout HEAD -- THE-FILE;  git diff HEAD → empty;  hash back to 3cb4ec4a3…

The direction is turns red, and the four that redden are exactly the four that assert output. The fifth — the no-collision silence control — stays green under the ablation, which is correct and is the point: a mute seeder also prints nothing on a non-colliding pass, so that case is discriminating in the other direction only.

Clause-② re-derivation — yes, with a discriminating control

Re-derived from the delivered diff, not carried from the claim. The new symbols are re-exported from src/index.ts, and package.json publishes exports["."] → ./dist/index.{js,mjs,d.ts} with files: ["dist", …], so the test is reachability from the built entry:

                                            dist/index.d.ts  dist/index.js  dist/index.mjs
CAPABILITY_NAME_COLLISION                          5              6              5
capabilityNameCollisionDiagnostic                  2              4              3
formatCapabilityNameCollisionDiagnostic            2              3              2
reportCapabilityNameCollisions                     3              4              3
CapabilityNameCollisionDiagnostic (type)           5              3              2
permissionSetNameIsForeign  (POSITIVE control —
  a symbol known published before this diff)       4              5              4
nonexistentSymbolXyz18023   (NEGATIVE control)     0              0              0

Both controls behave, so the probe discriminates. New exported symbols on a published entry ⇒ Clause-②: yes, and the changeset is graded minor accordingly (a patch with yes is refused, #16361). needs:contract-review is carried on this PR and on the card.

CapabilitySeedOutcome.collisions is the other limb the claim predicted; it is not what decides yes here, because bootstrapDeclaredCapabilities is not itself re-exported from index.ts — measured, not assumed. The index.ts re-exports are what carry it.

Verification

• pnpm --filter @objectstack/plugin-security test — 113 files / 2181 tests, all passing.
• pnpm --filter @objectstack/plugin-security typecheck — clean (tsc --noEmit, scripts project, and check:test-typecheck: 0 files / 0 errors).
• pnpm --filter '@objectstack/plugin-security...' build — closure green; the dist probe above is taken from that build.
• Gates run locally and their four-number reconciliation are in the report on the card.

Two existing bootstrap-seed-round-trips cases pinned this refusal's loudness by matching the prose "owned by another package". They now match the stable capability_name_collision grep token and additionally assert the record on the pass outcome — a strictly stronger observable, and one that does not re-red on every rewording. ⛔ No test was skipped, quarantined or weakened.

Acceptance notes

• Not filed, noted: permissionSetNameIsForeign carries the axis it was first written for in its name, and the capability seeder now imports it. Renaming it to something axis-neutral is a breaking change to a published export and buys only readability — a naming nit, excluded from the file-a-card classes. The import site documents why one name for one derivation beats two.
• Not filed, noted: the sibling axis states its refusal direction as "nothing is over-granted". This record deliberately makes the narrower claim — closed for the declaration, because nothing of the declaring package's is written anywhere — and says nothing about the owner's retained scope. Measured: the only non-test readers of sys_capability rows in this tree are the two seeders, the object definition, security-plugin.ts and packages/lint, and none was found reading a row's scope column for an authorization decision (capability scope is read off the spec-level PLATFORM_CAPABILITIES declaration instead). That is an observation about today's readers, ⛔ not a proof of absence, which is exactly why the record's severity note routes a future over-grant path to its own card rather than pre-judging it here.
• Not filed, noted: cleanupPackagePermissions covers sys_permission_set and does not touch sys_capability, so uninstalling the owning package does not currently remove the row. The wording avoids claiming a disappearing capability for exactly this reason.
• A separate finding for the remaining doubly-optional refusal branches in the two seeders is filed on the card's report; it is the same defect class one branch over and is deliberately not ridden in here, for the same reason this card was not ridden into [finding] A package's whole declared permission set is dropped on a set-name collision with no diagnostic and no build gate — the comment says "refuse loudly", the code warns through a doubly-optional logger #17516.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-security, touching 12 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/plugins/plugin-security/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/permissions/authorization.mdx (via bootstrapDeclaredCapabilities (symbol, a top-level function))
• content/docs/permissions/capabilities.mdx (via bootstrapDeclaredCapabilities (symbol, a top-level function))
• content/docs/permissions/permissions-matrix.mdx (via ownedBy (symbol, a field of interface CapabilityNameCollisionDiagnostic))
• content/docs/permissions/sharing-rules.mdx (via ownedBy (symbol, a field of interface CapabilityNameCollisionDiagnostic))
• content/docs/protocol/objectql/security.mdx (via ownedBy (symbol, a field of interface CapabilityNameCollisionDiagnostic))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v17/17-0.mdx (via ownedBy (symbol, a field of interface CapabilityNameCollisionDiagnostic))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

• 1 changed file(s) yielded no anchor (packages/plugins/plugin-security/src/index.ts) — pages documenting those are invisible to this run
• 3 name(s) were too generic to anchor anything (single lowercase words)
• the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
• a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 15 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json b3d6918b86abcc31f82e963d2fabc37308bc4bb7 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 2d6efa6fa32bfc85809754d0e7e3a4f5afae9929 — the merge of head 9cbca2136fde1f610cd7d1f28f77ae5afe9b0852 into base b3d6918b86abcc31f82e963d2fabc37308bc4bb7, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 2d6efa6fa32bfc85809754d0e7e3a4f5afae9929 && git checkout 2d6efa6fa32bfc85809754d0e7e3a4f5afae9929
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b3d6918b86abcc31f82e963d2fabc37308bc4bb7 9cbca2136fde1f610cd7d1f28f77ae5afe9b0852 && git checkout -B drift-repro b3d6918b86abcc31f82e963d2fabc37308bc4bb7 && git merge --no-ff 9cbca2136fde1f610cd7d1f28f77ae5afe9b0852

node scripts/docs-audit/affected-docs.mjs --json b3d6918b86abcc31f82e963d2fabc37308bc4bb7

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs b3d6918b86abcc31f82e963d2fabc37308bc4bb7 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

[claude]

Contract review

Served-tier: claude-fable-5-1

Reviewed head: 9cbca2136fde1f610cd7d1f28f77ae5afe9b0852 (PR #18088, card #18023, base b3d6918b8). Independent at-tier review; the PR body was treated as the claim under test, not as evidence. Everything below was re-measured in a fresh checkout of the head: pnpm install, closure build, the full package suite, the dist probe, and the ablation.

① Derived judgments — acceptance set and public surface, item by item

Item	PR's claim	Reading
Foreign-owner refusal reaches the author with no logger passed	0 → 1 console line, skippedForeign unchanged at 1	Right. Pinned in bootstrap-declared-capabilities.test.ts (no-logger case, all five console channels spied); the skip, the untouched foreign row and materializedNames are asserted unchanged in the same case.
Own diagnostic record type + own key on the seeder outcome	CapabilityNameCollisionDiagnostic, CapabilitySeedOutcome.collisions? (absent, never [])	Right. The absent-not-empty invariant is pinned by the silence control.
Reuse of the shared predicate, no re-derivation	seeder branch now calls permissionSetNameIsForeign instead of ===	Right. Read the predicate: (owner ?? null) !== (declarer ?? null). For a string owner this is identical to the old ===; for a nullish owner both the old and the new form read FOREIGN, so the skip behaviour is preserved by construction, and the PR pins that row shape.
permissionSetNameCollisionDiagnostic / format… / report… deliberately not reused	consequence differs; sink discipline restated	Right. reportPermissionSetNameCollisions is typed on readonly PermissionSetNameCollisionDiagnostic[] and words "permissions are NOT in effect"; the capability record words "the name still resolves". The restated discipline (no-sink prints, injected sink preferred, receiver kept, empty pass silent) is four pins, all present in capability-name-collision.test.ts.
New exported symbols reachable from the published entry	four values + one type, probed on dist/index.{d.ts,js,mjs}	Right, and the probe discriminates. package.json publishes only exports["."] → dist/index.*, files includes dist; src/index.ts re-exports them by name (no export * anywhere in that file). My rebuild reproduces the PR's counts exactly for the four values and both controls (positive permissionSetNameIsForeign 4/5/4, negative nonexistentSymbolXyz18023 0/0/0). A runtime import of both dist/index.js and dist/index.mjs returns string/function for all four values and undefined for the negative control.
CapabilitySeedOutcome.collisions is NOT what decides yes	the seeder is not re-exported	Right. bootstrapDeclaredCapabilities is undefined on both built entries, and CapabilitySeedOutcome has 0 hits in dist/index.d.ts. So the new key is not a key on a published payload; the yes rests on the index.ts re-exports alone, exactly as the PR says.
Reverse verification turns red for the right reasons	4 failed / 39 passed under the mutation	Right. I reproduced the mutation from the committed state: HEAD blob 3cb4ec4a3… → mutated 0b2ec0f47… (same hashes the PR reports), anchors 1/0 → 0/1. Running the three affected files: the same four seeder pins redden, and additionally both repurposed bootstrap-seed-round-trips cases redden (6 failed / 79 passed over 85), while the no-collision silence control and every pure-function pin stay green. Restore verified by blob hash and a clean git status.
No test skipped, disabled, quarantined or weakened	—	Right. No .skip/.only/.todo added and no it(/describe( line removed anywhere in the diff.
The two repurposed round-trip cases are strictly stronger	prose match → token match + record assertion	Right. Each case keeps every prior assertion (skippedForeign, untouched package_id, materializedNames, a warn line present) and adds the outcome record. The ablation proves the new token assertion is itself discriminating: both cases redden under it.

One precision remark that does not change the reading: the PR's CapabilityNameCollisionDiagnostic (type) row of 5/3/2 is a substring artefact. With word boundaries the type reads 5/0/0, because the 3 and 2 in the JS files are hits inside formatCapabilityNameCollisionDiagnostic. A type living only in the .d.ts is the correct shape, and the types condition of the published entry reaches it, so the symbol is genuinely published.

Head-state facts: full package suite on this head is 113 files / 2181 tests passing in my run; node scripts/pm/check-clause2-carriers.mjs --pair 18088 exits 0; all 38 check runs on the head are success or skipped; production still injects ctx.logger at security-plugin.ts, so the console.warn fallback changes only sink-less direct callers.

② Semver grading

Declared @objectstack/plugin-security: minor. The diff adds four value exports and one type export on the sole published entry, plus an optional key on an unpublished outcome type; nothing is removed or renamed and no wire byte moves. That is additive public surface, which is minor. Level and changeset agree; the patch-under-yes refusal (#16361) is not triggered.

③ Boundary flags

• permissionSetNameIsForeign keeps its axis-bearing name while now serving both axes — accepted. The predicate's signature and docblock are already axis-independent; renaming a published export is a breaking change outside this card's scope, and the import site documents the choice. Not escalated.
• reportPermissionSetNameCollisions not generalised — accepted. Verified by reading it: its record type and its sentence are permission-set-specific. Threading an axis through it would make one function state two consequences. The two lines of sink discipline that are duplicated are pinned on both axes independently, which is the right price.
• sys_capability.scope reader survey — accepted as the bounded observation it claims to be. My own grep finds no non-test reader of a scope column adjacent to a sys_capability read. Note that the PR's list of "readers" is narrower than the set of files that merely name the table (spec constants, plugin-sharing provenance, runtime/app-plugin.ts also name it); the operative claim, that none reads scope for an authorization decision, holds on the head, and the record's severity docblock already routes any future over-grant path to its own card. Not escalated.
• cleanupPackagePermissions coverage — accepted. Verified: it touches sys_permission_set, sys_position_permission_set and sys_user_permission_set only, so the diagnostic is right not to promise a disappearing row. A capability-uninstall change is an ADR-0086 D3 card of its own, not this one's.
• The remaining doubly-optional refusal sites (five, listed in the dev report on the card) — escalated to the PM seat for triage filing, as the report already does; it is the same defect class one branch over and correctly not ridden in here.

Implemented-by: claude/issue-18023-capability-collision-diagnostic
Reviewed-by: session_019iNh833mJXnpWMFDeSufcW

Verdict: PASS

---

Generated by Claude Code

[claude]

needs:contract-review cleared from BOTH carriers — clause-② contract review PASS.

Record of governing verdict: comment 5657757282 on this PR, judging head 9cbca2136fde1f610cd7d1f28f77ae5afe9b0852 — the head this PR still points at, re-read immediately before this clear (⚠️ a record binds to the head it names; had the head moved, re-verifying would have been necessary and ⛔ not sufficient).

• Served-tier: claude-fable-5-1 — equals CONTRACT_REVIEW_TIER (scripts/pm/dispatch-gates.mjs:10507).
• Implemented-by: claude/issue-18023-capability-collision-diagnostic (a mode:subagent dev, named by its BRANCH).
• Reviewed-by: session_019iNh833mJXnpWMFDeSufcW — a DIFFERENT identity, so the independence pair holds and this is ⛔ not a self-review.
• Commissioned under the maintainer ruling 「你可以直接派fable复核」 as a separate session; ⛔ an in-session subagent carrying a model parameter does not lift the tier, which is exactly what gate row C7 refuses.

The declaration Clause-②: yes was re-derived by the dev from the delivered diff and independently re-measured by the reviewer against a rebuilt dist/, with a positive control (permissionSetNameIsForeign) and a negative control (nonexistentSymbolXyz18023) — so no correction comment is owed and the claim and the delivery agree.

⚠️ The review recorded one precision defect that does ⛔ not change the verdict: the PR body's CapabilityNameCollisionDiagnostic (type) row of 5/3/2 is a substring artefact — with word boundaries the type reads 5/0/0, the 3 and 2 being hits inside formatCapabilityNameCollisionDiagnostic. A type resident only in dist/index.d.ts is the correct shape and the published entry's types condition reaches it, so the symbol is genuinely published and yes stands.

---

Generated by Claude Code