Merge remote-tracking branch 'origin/main' into claude/issue-18070-authz-resolver-false-green

Author: claude

.changeset/17516-permission-set-collision-diagnostic.md

@@ -0,0 +1,29 @@
+---
+"@objectstack/plugin-security": minor
+---
+
+A **permission-set name collision now reaches the author**. When a package declares a permission set whose name a *different* package already owns, `bootstrapDeclaredPermissions` refuses to write into that row — correct under ADR-0086 D4, and unchanged — but the refusal is no longer invisible (#17516).
+
+Measured on the pre-change tree, with a collision seeded and **no logger passed**:
+
+```
+skippedForeign                 = 1     (the entire declared set was dropped)
+author-visible console lines   = 0     (log, info, warn, error, debug — all five)
+diagnostic records on outcome  = undefined
+```
+
+The branch reported through `logger?.warn?.(…)` — optionally chained **twice** — so a caller that passed no logger produced no output at all, and a package's whole declared permission set vanished with one internal counter incremented. The comment there said *"refuse loudly"*; nothing about it was loud. Same case after the change:
+
+```
+skippedForeign                 = 1     (unchanged — the skip is not what was wrong)
+author-visible console lines   = 1     warn: [security] [permission_set_name_collision] …
+diagnostic records on outcome  = 1     { name, declaredBy, ownedBy, message, fix }
+```
+
+- **It prints with no sink injected.** `reportPermissionSetNameCollisions` falls back to `console.warn`, per the #10556 ruling that silent-by-declaration is rejected — an injected host sink still replaces it rather than printing beside it. The call keeps the receiver (a property-access call, never a detached `logger.warn ?? console.warn`), so a class-based host sink does not throw.
+- **The refusal is also readable without a log.** `PermissionSeedOutcome` gains an optional `collisions` array carrying one diagnostic per dropped set — absent, never `[]`, when the pass hit none. A counter with no record is what made the drop undiagnosable.
+- **One derivation, so two doors cannot drift.** `permissionSetNameIsForeign`, `permissionSetNameCollisionDiagnostic` and `formatPermissionSetNameCollisionDiagnostic` are exported from the package entry so a compile-time door consumes them rather than re-deriving the predicate or re-spelling the wording — the shape #14553 established for `navigationContributions`. ⚠️ Only the **runtime** door ships here; the compile-time door (`os build` / `os validate`) lives in another package and is not part of this change.
+- **A stable, greppable token**, `permission_set_name_collision`, is stamped as `event` on every report. It is a snake_case data value, not an ADR-0112 error code: it is never routed to `error.code` and never reaches a wire refusal, the same discrimination the sibling `position_name_fold_grant` token already makes in this package.
+- **The branch comment's premise is corrected.** It claimed package-namespaced object api names make set-name collisions a packaging bug rather than a merge case. **ADR-0130 D1 falsifies that** — N packages may co-own one namespace — so a collision is a legal configuration that gets *more* common, not an error that should never happen. The diagnostic's `fix` text names both legal resolutions.
+
+⛔ **No wire byte moves and no skip changes.** The foreign row is still never written; `skippedForeign` still counts it; the ADR-0086 P2 publish materializer still returns its existing `permission set name is owned by another package` failure text. A non-colliding pass stays completely silent on all five console channels, asserted over a pass that really does seed and re-seed.

.claude/agents/os-dev.md

@@ -47,7 +47,7 @@ model: opus
    - 其余 ⛔ 不立卡:观察、死代码、未演练漂移、抛光、风格、文档 nit、命名。
    - 它们进 PR `## Acceptance notes`,报告 `out_of_scope_findings` 记 `noted, not filed: …`,席位 ACCEPT 读。
    - 写 `noted, not filed` 前先答哪一个 PR 或人会碰到这个文件;答不出就写明「承接者:无」。
-   - 先搜再立:关键词 + 文件路径扫 open issues;并行 dev 同一小时立的卡只有这一搜能看见。
+   - 立卡者不查重,只在卡面附 3–5 个查重词;查重归分诊席,⛔ 不扫 open issues、不拉板。
    - 通道先探后选:同容器先测一条 repo-scoped REST 读;通 ⇒ 走 REST 列表端点 + 本地 grep。
    - 通道对照表见 `.claude/skills/pm-dispatch/references/rest-channel.md`,其 ✓ 按座位实测。
    - 403 ⇒ 改用一次定向 MCP `search_issues`,并在报告申报换道。

.claude/skills/pm-dispatch/SKILL.md

@@ -103,14 +103,14 @@ PM 的工作是循环:选卡 → 认领 → 派发 → 收集 → 复核 → 报
 | issue 上的信号 | 含义 |
 |---|---|
 | open + 队列标签 + 无 assignee | 可派发(同卡带 `pm:retriage` 的除外);`pm:queue` 卡恒无 assignee,有即半态 |
-| assignee 已设 | 已认领/在飞,不是你的就永不碰;离手恒走释放 —— 四因 = 改路由、前提证伪、弃飞无接管、跨车道移交,去向 = 新标签态或车道;⛔ 不静默摘 assignee |
+| assignee 已设 | 已认领/在飞,不是你的就永不碰;离手恒走释放 —— 三因 = 改路由(限未派发)、前提证伪、弃飞无接管,去向 = 新标签态或车道;在飞卡跟到 MERGED,⛔ 不静默摘 assignee |
 | `pm:dispatched` | 已派发(派发评论记轮次),恒带 assignee;与摘 `pm:queue` 同一次标签写入成对落地 |
 | `needs-user-decision` | 决定待做:永不派发、除代裁通道外永不代答;维护者的收件箱 |
 | `pm:on-hold` | 决定已做且答案是暂不做:不派发不催;仅当带机器可读 `Restart-when:` 行才合法 |
 | `pm:blocked` + 正文行 `Blocked-by: #N` | 等上游:选择期跳过,#N 关闭时由解锁扫描放回;工已完、PR 被外部门禁卡住的同用本态 |
 | `pm:awaiting-maintainer` | 决定已做,只剩一次 GitHub 之外的人工动作:不派发不催;与其它 pm 状态标签互斥;入态恒带行首 `Maintainer-action:` 行(细则见 `references/state-machine.md`),无行即半态 |
 | `pm:blocking` | 有 open 下游依赖者(自 `Blocked-by:` 索引推导的缓存,⛔ 不手工挂);进选择全序 |
-| `pm:retriage` | 向分诊提问(改判、跨域 PR 指定车道、改路由、拆卡、裁 dev 报告留下的分叉),异议评论写明所求;与现行 `pm:*` 并存、⛔ 不摘原标;带本标签的 `pm:queue` 卡跳过派发 |
+| `pm:retriage` | 向分诊提问(改判、跨域 PR 指定车道、改路由、拆卡、裁 dev 报告留下的分叉),定车道与改路由限未派发卡,异议评论写明所求;与现行 `pm:*` 并存、⛔ 不摘原标;带本标签的 `pm:queue` 卡跳过派发 |
 | `finding` | 立卡三类内待首次定级,定级即离标;三类外关 not planned;不占队列不进收件箱 |
 | `target:<major>` | 发版阻塞:每个 backlog 恰好一个生产者 |
 | `pm:epic`(父单或 sub-issue) | 已由 epic PM 保留;其它 PM 永不取;⛔ 永不与 `pm:queue` 同挂 |

.claude/skills/pm-dispatch/references/lanes/ui.md

@@ -22,7 +22,7 @@
 - 凡触 `packages/spec` 一律转 `domain:spec` 座位,见 SKILL.md 〈多仓协调〉。
 - `scripts/pm/**` 单写手恒在 objectstack 侧 ⇒ 本席的工具需求走上游卡带 `Blocked-by:` 回链。
 - 受管面五项含仓根发布 `skills/**`,清单、判据与禁令见 objectui AGENTS.md §9 受管面段。
-- ⇒ 命中即整 PR 停在 draft 等人合,⛔ 不 ready、不入队、不 auto-merge、不自合、不留批准。
+- ⇒ 命中即停 draft;⛔ 未获授权批准不 ready 不入队不自合、永不批准,获批后认领席落地。
 - 未命中的 PR 按同节走合并队列落地;changeset 与版本纪律见 objectui AGENTS.md §9。
 - 半状态巡查在本仓有载体:定时 workflow 与 sweeper 皆在,自 objectstack 移植。
 - 锚 issue 未配置 ⇒ 写锚步骤 skip,发现只落 run summary,绿不等于已交付读锚腿。

packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts

@@ -105,7 +105,16 @@ describe('bootstrapDeclaredPermissions (ADR-0086 D5)', () => {
     });
     expect(r.skippedForeign).toBe(1);
     expect(ql.rows[0].package_id).toBe('com.example.crm');
-    expect(warns.some((w) => String(w.m).includes('owned by another package'))).toBe(true);
+    // [#17516] Re-anchored from the old prose ('owned by another package') to
+    // the stable token the report now stamps. The substance this pin asserts is
+    // unchanged — the refusal is reported — but the token is what an operator
+    // greps and what the sibling doors key on, so prose drift can no longer
+    // quietly unpin it. The read-back half is asserted beside it: a counter
+    // with no record is what made this drop invisible.
+    expect(warns.some((w) => String(w.m).includes('permission_set_name_collision'))).toBe(true);
+    expect(r.collisions).toEqual([
+      expect.objectContaining({ name: 'crm_sales_rep', declaredBy: 'com.example.other', ownedBy: 'com.example.crm' }),
+    ]);
   });
 
   it('skips a declared set with no resolvable owning package (warned, not seeded)', async () => {

packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts

@@ -68,6 +68,12 @@ import {
   reportSeedWriteRefusals,
   type SeedWriteRefusals,
 } from './per-organization-catalog.js';
+import {
+  permissionSetNameCollisionDiagnostic,
+  permissionSetNameIsForeign,
+  reportPermissionSetNameCollisions,
+  type PermissionSetNameCollisionDiagnostic,
+} from './permission-set-name-collision.js';
 
 export type { PermissionSeedOutcome } from './permission-set-projection.js';
 
@@ -206,6 +212,14 @@ export async function upsertPackagePermissionSet(
      * materialized nothing).
      */
     refusals?: SeedWriteRefusals;
+    /**
+     * [#17516] Collects set-name collisions so the pass reports them ONCE
+     * instead of a line per dropped set. Passed by the boot catalog loop; the
+     * ADR-0086 P2 publish materializer passes nothing and the refusal is
+     * reported at the branch instead — ⛔ never dropped, which is the whole
+     * point of this card.
+     */
+    collisions?: PermissionSetNameCollisionDiagnostic[];
   },
 ): Promise<PermissionSeedOutcome> {
   const out: PermissionSeedOutcome = { seeded: 0, updated: 0, unchanged: 0, unreadable: 0, skippedEnvAuthored: 0, skippedForeign: 0 };
@@ -260,7 +274,7 @@ export async function upsertPackagePermissionSet(
   }
 
   if (existing.managed_by === 'package') {
-    if (existing.package_id === packageId) {
+    if (!permissionSetNameIsForeign(existing.package_id, packageId)) {
       // Our own row — re-seed so the record always reflects the shipped/published
       // declaration (idempotent; covers version bumps without bookkeeping).
       //
@@ -283,13 +297,40 @@ export async function upsertPackagePermissionSet(
         out.updated += 1;
       }
     } else {
-      // Package-namespaced object api names make set-name collisions a
-      // packaging bug, not a merge case — refuse loudly (ADR-0086 D4:
-      // a package never writes into a foreign record).
+      // [#17516] The SKIP is unchanged and correct — ADR-0086 D4: a package
+      // never writes into a foreign record. What changed is that it is no
+      // longer invisible.
+      //
+      // ⚠️ The premise this branch used to state — "Package-namespaced object
+      // api names make set-name collisions a packaging bug, not a merge case"
+      // — is FALSIFIED by ADR-0130 D1, which lets N packages co-own one
+      // namespace (the ADR records it under "What was NOT decided"). A
+      // collision is therefore a legal configuration that gets MORE common as
+      // co-ownership lands, not a packaging error that should never happen. So
+      // the author reading it is the normal case, not the pathological one.
+      //
+      // ⛔ And the old line did not refuse loudly, whatever it claimed:
+      // `logger?.warn?.(…)` is optionally chained TWICE, so a caller passing no
+      // logger produced NO OUTPUT AT ALL and an entire declared permission set
+      // disappeared with one counter moved. The report now goes through
+      // `reportPermissionSetNameCollisions`, which prints with no sink injected
+      // (#10556: silent-by-declaration is rejected), and the diagnostic RECORD
+      // travels back on the outcome so a caller that reads no log at all — a
+      // boot report, a test — can still ask what happened.
       out.skippedForeign += 1;
-      logger?.warn?.('[security] permission set name owned by another package — skipped', {
-        name: ps.name, declaredBy: packageId, ownedBy: existing.package_id,
+      const diagnostic = permissionSetNameCollisionDiagnostic({
+        name: String(ps.name),
+        declaredBy: packageId,
+        ownedBy: typeof existing.package_id === 'string' ? existing.package_id : null,
+        ...(organizationId ? { organizationId } : {}),
       });
+      out.collisions = [diagnostic];
+      // The boot loop collects and reports ONCE per pass. The ADR-0086 P2
+      // publish materializer upserts a single set and passes no collector, so
+      // it reports here — it has no pass to summarise, and inheriting the old
+      // silence is the one outcome this card forbids.
+      if (opts?.collisions) opts.collisions.push(diagnostic);
+      else reportPermissionSetNameCollisions(logger, [diagnostic], organizationId);
     }
     return out;
   }
@@ -344,13 +385,15 @@ export async function bootstrapDeclaredPermissions(
   // One log per pass, not per refused row: a legacy platform-wide unique index
   // refuses EVERY declared permission set, and a line each would bury the remedy.
   const refusals = createSeedWriteRefusals();
+  // [#17516] Declared sets dropped because another package owns the name.
+  const collisions: PermissionSetNameCollisionDiagnostic[] = [];
 
   for (const ps of sets) {
     if (!ps?.name) continue;
     // Registry provenance first (ADR-0010 `_packageId`), author-declared
     // spec `packageId` (ADR-0086 D3) as fallback.
     const packageId: string | undefined = ps._packageId ?? ps.packageId ?? undefined;
-    const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger, { existingByName, organizationId, residue, refusals });
+    const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger, { existingByName, organizationId, residue, refusals, collisions });
     out.seeded += r.seeded;
     out.updated += r.updated;
     out.unchanged += r.unchanged;
@@ -370,6 +413,11 @@ export async function bootstrapDeclaredPermissions(
   }
   // Before the counts, so an operator reads WHY the count is zero beside it.
   reportSeedWriteRefusals(options.logger, refusals, organizationId);
+  // [#17516] Said once per pass, and said even when no logger was injected —
+  // the whole defect was that this refusal reached nobody. The records go back
+  // on the outcome too, for a caller that reads no log at all.
+  reportPermissionSetNameCollisions(options.logger, collisions, organizationId);
+  if (collisions.length > 0) out.collisions = collisions;
   if (out.unreadable > 0) {
     // Said once, with the count: these sets were neither seeded nor reconciled
     // because the record could not be READ. Silence here would read exactly

packages/plugins/plugin-security/src/bootstrap-seed-round-trips.test.ts

@@ -431,7 +431,14 @@ describe('#10946 — a name declared twice in one batch keeps its loud refusal',
     expect(r.skippedForeign).toBe(1);
     expect(ql.rows).toHaveLength(1);
     expect(ql.rows[0].package_id).toBe('com.example.a');
-    expect(warns.some((w) => w.includes('owned by another package'))).toBe(true);
+    // [#17516] Re-anchored from the old prose to the stable token the report
+    // stamps — the assertion's substance (the refusal is REPORTED, not merely
+    // counted) is unchanged, and the record is asserted beside it so "loud"
+    // means reaching a reader rather than moving a counter.
+    expect(warns.some((w) => w.includes('permission_set_name_collision'))).toBe(true);
+    expect(r.collisions).toEqual([
+      expect.objectContaining({ name: 'shared_name', declaredBy: 'com.example.b', ownedBy: 'com.example.a' }),
+    ]);
   });
 });
 

packages/plugins/plugin-security/src/index.ts

@@ -55,6 +55,23 @@ export type {
   InvitationPlacementService,
 } from './invitation-placement.js';
 export { bootstrapDeclaredPermissions } from './bootstrap-declared-permissions.js';
+// [#17516] The set-name collision diagnostic. EXPORTED because its whole
+// purpose is to be the ONE derivation every door shares: the runtime door below
+// raises it today, and the compile-time door (`os build` / `os validate`, which
+// lives in another package) must consume these rather than re-deriving either
+// the predicate or the wording — that drift is what this card is about, one
+// layer up.
+export {
+  PERMISSION_SET_NAME_COLLISION,
+  formatPermissionSetNameCollisionDiagnostic,
+  permissionSetNameCollisionDiagnostic,
+  permissionSetNameIsForeign,
+  reportPermissionSetNameCollisions,
+} from './permission-set-name-collision.js';
+export type {
+  CollisionReportSink,
+  PermissionSetNameCollisionDiagnostic,
+} from './permission-set-name-collision.js';
 // [ADR-0094] sys_permission_set pure-projection machinery.
 export {
   permissionSetRowFields,