fix(plugin-security): the app default permission set resolves from the first level that NAMES one (#15298)

[hotlong]

Fixes #15298

declaredPermissionSets carried a docblock stating a short-circuit its code did not have:

The packages[] pass only supplies a set where the top level had none — which is precisely the option-B artifact.

The code pushed the flattened top level and then every package body unconditionally, so on today's additive artifact (flattened level and packages[] both present) every permission set was collected twice. Nothing observable came of it: the sole caller is private and takes the first isDefault set, which the flattened copy still supplied. So this corrects a false written contract on a security-path reader, not a live defect — and the sentence was load-bearing, because it was the stated reason the reader half was revertible on its own and safe to land ahead of the emitter half (#14512).

The fix: start from the expression this program replaced

The expression card #15007 replaced was appDefaultPermissionSetName(config.permissions) — a name, whose "came back empty" is undefined. The reader now walks exactly that discipline: resolve from the flattened level, and consult packages[] only where it answered nothing.

The condition is the resolved NAME, never the permissions container. Branching on the container re-creates the silent loss this program exists to remove, one shape further along. Reading the answer also retires the []-is-truthy trap rather than patching around it, since the container is never the predicate.

The package order is resolved BEFORE the top level is consulted, deliberately. resolveArtifactPackageOrder refuses a malformed packages — not an array, an entry inlined instead of wrapped under manifest:, a duplicate package id — with an ADR-0112 envelope this reader does not catch. Moving that line below the early return would make the refusal depend on whether the flattened level happened to name a default first. An artifact is either loadable or refused; which level answered is not part of that question. The docblock says so in the file, anticipating the reading that the line is misplaced.

Why this reader's condition is not the sibling's. resolveStackCollection (packages/cli/src/utils/stack-collections.ts, #15006, now on main) branches on the container — if (Array.isArray(top)) return top; — and is right to: it returns a whole collection, so a present top-level key has by construction already answered. This reader extracts a distinguished element out of the collection, so "the key is present" and "the key answers" are two different facts here. Same discipline, read against what each reader's expression actually returns. That reasoning is now in the docblock, so the convergence pass reads two readers that differ and say why, rather than two that differ while appearing to agree.

The dispatched fix was corrected, and the record stays here

Preserved from the PM's own account on this PR, because the correction matters more than the card did.

The dispatch prescribed branching on the container:

if (Array.isArray(flattened) && flattened.length > 0) return flattened;

with a warning that the bare Array.isArray form would re-create the silent loss via the empty-array truthiness trap. That warning was right and insufficient: length > 0 fixes the empty case and leaves a worse one open. A config whose flattened level carries permission sets but marks none of them isDefault — legal today, hand-writable in any objectstack.config.ts — short-circuits the entire packages[] pass under that version and resolves undefined, with nothing thrown and nothing logged.

That is no longer an argument. Ablation 1 below measures it: against the dispatched snippet, exactly that shape goes red.

Verification

Everything below ran at HEAD 274ab06bc, on this branch merged up to origin/main 3a4373f4c.

1 · The acceptance clause: byte-identical returns, measured rather than argued

A differential harness drove appSecurityPluginOptions over 26 shapes and recorded every answer: 4 emitted shapes (the composed additive artifact, its option-B derivative, and the collection-zoo fixtures behind the #15004 acceptance pin), 14 configs the unit suite drives, 3 malformed-packages refusals recorded as thrown code + status, and 5 hand-authored mixed shapes the platform never emits. Three legs, each rebuilding @objectstack/plugin-security and proving on disk what the suite would consume:

leg	source	dist/ preflight	corpus
A · after	HEAD (0ab95e53, = HEAD blob)	declaredDefaultPermissionSetName present in 4 built files	26 rows
B · before	pre-change file restored from 9690d114b (727c8455, = that blob); on disk: sets.push ×2, new name ×0	sets.push present in 2 built files, new name absent from all 6	26 rows
C · restore	back to HEAD (0ab95e53, = HEAD blob)	present / absent verdicts re-confirmed	26 rows

• diff before after → empty, exit 0.
• diff after after2 → empty, exit 0, so the restore leg is proven by re-measurement, not by a checkout exit code.

Two of the 26 rows are the ones the dispatched snippet would have moved silently: a flattened level naming a default plus a malformed packages still throws INVALID_ARTIFACT_PACKAGES / DUPLICATE_ARTIFACT_PACKAGE at 422. That identity is what resolving the package order first buys.

2 · Ablations — the new tests are load-bearing

Each mutation was confirmed on disk before running (injected marker counted 1, replaced anchor counted 0, file hash printed against the HEAD blob), and each restore was proven by hash equality plus a marker count of 0.

condition under test	result
control (HEAD, answer-shaped)	23 passed
container-shaped with length > 0 (the dispatched snippet)	1 failed · a NON-EMPTY flattened array that marks no default does not either
container-shaped without length > 0	2 failed · that one, plus an EMPTY flattened array does not short-circuit it
restored	23 passed

The card's mandated positive test — top-level permissions: [] plus a package body carrying an isDefault set — is the second failure, so it catches the trap the card named. The first failure is the one a container-shaped condition leaves open at any spelling.

This ablation needs no rebuild: the unit suite imports ./app-default-permission-set relative, inside its own package, so vitest resolves source. The rebuild-and-prove discipline is owed by the CLI pin, which reaches this code through the workspace link to dist/, and it is paid in §1 and §3.

3 · Suites

• @objectstack/plugin-security unit suite: 23 passed (the 18 that were on main, plus 5 new).
• packages/cli/test/option-b-reader-acceptance.pin.test.ts: 7 passed, against a dist/ proven to carry this change. The file is untouched; OPTION_B_LOSSES, its set-equality assertion, and the row fix(plugin-security): the app default permission set resolves from packages[] (#15007) #15226 deleted are all exactly as they were.

4 · Gates

46 families derived from the real change set by node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, re-derived after the merge because the first derivation warned it had read stale copies of its own inputs — and it had: the count moved 45 → 46, gaining check:cli-test-child-env. Every exit code was captured before any pipe (cmd > log 2>&1; rc=$?).

All 46 green. Two needed a second pass and are worth naming, because their first result was neither a pass nor a failure:

• check:dual-build-cjs-loads and check:i18n first exited 3 — PREREQUISITE NOT MET, both saying so in their own words ("Nothing was checked", "This is NOT a pass: nothing was measured"). They read built output across the whole workspace, which a package-scoped closure does not provide. After turbo run build over ./packages/* and ./packages/*/* (71/71 tasks successful) both re-ran green: 102 require entry points across 66 packages load, 610 emitted CJS files parse; 9 i18n packages all in sync.
• check:type-check-debt did measure rather than refuse: 14 ledger entries re-measured in 601.6s, 153 raw tsc errors, none above its recorded number.

Boundaries

• packages/cli/test/option-b-reader-acceptance.pin.test.ts — untouched.
• content/docs/releases/ — untouched.
• Changeset added at patch, per the card: declaredPermissionSets is private, so no published surface moves. It also names the one sentence of the fix(plugin-security): the app default permission set resolves from packages[] (#15007) #15226 entry it supersedes in this same unreleased batch, rather than retro-editing that entry.
• Draft, and auto-merge is not armed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

3 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see

• the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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 — 14 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 6e67b86c03c9e68e95ed93783de02d82b2a6f6d5 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 34409788874a4f85b01d4cca5a57d07e4fc2b742 — the merge of head 274ab06bcf335c82325b65bca0b9ea1c448b753a into base 6e67b86c03c9e68e95ed93783de02d82b2a6f6d5, 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 34409788874a4f85b01d4cca5a57d07e4fc2b742 && git checkout 34409788874a4f85b01d4cca5a57d07e4fc2b742
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 6e67b86c03c9e68e95ed93783de02d82b2a6f6d5 274ab06bcf335c82325b65bca0b9ea1c448b753a && git checkout -B drift-repro 6e67b86c03c9e68e95ed93783de02d82b2a6f6d5 && git merge --no-ff 274ab06bcf335c82325b65bca0b9ea1c448b753a

node scripts/docs-audit/affected-docs.mjs --json 6e67b86c03c9e68e95ed93783de02d82b2a6f6d5

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

[hotlong]

复核:PASS

⚠️ 同会话自审声明:本复核由派发本卡的同一个 PM 席位做出,与 dev 席同会话,不是独立第二意见。下面凡「已核」的都是我自己在 PR head 上跑的读数,不是转述报告。

diff 层面核过的四件事

• OPTION_B_LOSSES 未被动过。 相对分支自身基线 3a4373f4c 的 diff 为空。⚠️ 用的是 merge-base 而不是 git diff origin/main <branch> —— 后者会把 main 自己的前进算进来,我今天早些时候在别的 PR 上正是这么误报过一次。
• 改动面就三个文件:changeset、测试、源文件。没有夹带。
• 判定逻辑确实是按「答案」而非「容器」分支:

  const bodies = packages === undefined || packages === null ? [] : resolveArtifactPackageOrder(config);
  const flattened = (config as { permissions?: unknown } | null | undefined)?.permissions;
  const fromFlattened = appDefaultPermissionSetName(flattened);
  if (fromFlattened !== undefined) return fromFlattened;

  并且 resolveArtifactPackageOrder(config) 在提前返回之前被调用 —— 这让它对畸形 packages 的 ADR-0112 拒绝保持无条件,不取决于顶层是否恰好先命名了一个默认集。
• clause-② 不触发:该文件的 export 行零增删,已发布面没动,所以不锁契约复审档。changeset 取 patch,与该判据一致。

---

我给的方案是错的,而且这次是被测量证伪的

派发单里我写的是按容器分支:

if (Array.isArray(flattened) && flattened.length > 0) return flattened;

席位没照做,改成按答案分支,并且不是凭论证 —— 它建了一个 26 个形状的差分语料(4 个真实产出含 #15004 的 collection-zoo 夹具、14 个单测配置、3 个畸形 packages 拒绝、5 个手写混合),三条腿各自重建 @objectstack/plugin-security 并证明改动到达了 dist/。结论:

我那个片段会移动 26 个形状中的 3 个。

原因是我漏掉的那个 case:顶层带着若干 permission set、却一个都没标 isDefault(今天完全合法、手写得出来)。我的 length > 0 只堵住空数组,这个 case 会短路掉整个 packages[] 段并答 undefined,不抛不记 —— 正是本卡要消灭的失败类,往后挪了一个形状。

消融把这个差距钉死了:

腿	结果
control(实际落地的写法)	23 passed
容器式 带 length > 0(即我派的那个片段)	1 failed — 「a NON-EMPTY flattened array that marks no default does not either」
容器式 不带 length > 0	2 failed — 再加上卡面强制要求的那条空数组正面用例
恢复	23 passed

这是本卡第二次由席位纠正我(第一次是它最初提出按答案分支)。两次我都把更正公开留在记录里,而不是私下认下 —— 派发席的错误判断要能被后来的人看见。

顺带,它还回答了我另一张卡(#15293,packages 守卫的程序级分裂)的一半:docblock 现在写明了为什么本读取器的条件与 #15006 的 resolveStackCollection 不同 —— 那个读取器返回整个集合,所以「有」等于「答得出」;这个读取器要取出一个特定元素,两者是不同的事实。于是收敛卡读到的是「两个故意不同、并且说明了为什么」的读取器,而不是两个看起来该一样却不一样的。

工程纪律里值得点名的三处

1. 第一次消融被整个丢弃而不是上报 —— 它的 control 腿返回 lock exit 99(拿不到槽位)= NOT MEASURED,于是重写成在稳定槽位下重试并从头重跑。拿一次「没测成」当基线,是消融最常见的假绿来源。
2. 发现 EXIT trap 没有触发(bash 在前台子进程返回前会推迟 trap),于是改用显式字节比对来证明恢复 —— 而不是信任 git checkout 的退出码。
3. 门禁族在合并后重新派生 —— 因为第一次派生自己警告它读到了陈旧输入,而它确实读到了:45 → 46,多出 check:cli-test-child-env。46 条全绿;check:dual-build-cjs-loads 与 check:i18n 先报 exit 3「PREREQUISITE NOT MET」,先按 NOT MEASURED 记录,构建补齐后再跑才绿。

---

裁决:那个 open question 走 A(即已落地的做法)

问题是:#15226 的 changeset 已在 main 上、未发布,而本 PR 推翻了它的一句话 ——「The resolution now reads the flattened top level FIRST and then each package body」。两条都是同一个包的 patch,会编进同一个版本。

A(保留 #15226 的条目,由本条 changeset 点名那句被取代的话)成立,理由与席位给的一致,我补一层:

#15226 那句话当时是真的 —— 它准确描述了 #15226 落地时代码在做什么(拼接)。为了让编译出来的发布说明只出现一种说法而去回改它,等于让记录去描述一棵从未存在过的树。而这正是本卡要关掉的那类缺陷(成文契约与实际不符),只是挪到了发布说明这一层。

已核 changeset 第 24-26 行确实点了名,并写明「That described #15226 accurately when it landed」。发布说明的合并是发版时人工编译的动作 —— 这条点名正是让编译者能有意识地把两条收成一条,而不是让它们互相矛盾地并列。

落地前检

• ① 席位契约档 PASS —— 本条即是;
• ② needs:contract-review —— 不适用,export 零增删,clause-② 不触发;
• ③ 全部 check 绿 —— 待核,绿了我武装 auto-merge。⛔ 在那之前保持 draft,⛔ 你不要自己武装。

---

Generated by Claude Code