Skip to content

fix(plugin-security, spec): name the ADR-0025 reading of manifest.permissions when the audience-binding reconciler cannot use it, and make both halves of the key point at each other - #18035

Merged
hotlong merged 3 commits into
mainfrom
claude/issue-18031-permissions-key-collision
Sep 13, 2026
Merged

fix(plugin-security, spec): name the ADR-0025 reading of manifest.permissions when the audience-binding reconciler cannot use it, and make both halves of the key point at each other#18035
hotlong merged 3 commits into
mainfrom
claude/issue-18031-permissions-key-collision

Conversation

@hotlong

@hotlong hotlong commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

Fixes #18031

Clause-②: yes

permissions carries two incompatible readings and SchemaRegistry.installPackage records both in one slot. At the AUTHORING stage ManifestSchema.permissions is the ADR-0025 §3.2 capability grant a plugin requests — the legacy flat string[], or the structured { services, hooks, network, fs } block. At the ASSEMBLED stage the collection wins and the same key is the ADR-0090 PermissionSet[] (AssembledPackageBodySchema, ADR-0130 D4). A package declares one of them and the other has nowhere to go.

What changed

1. The drop is loud (@objectstack/plugin-security). collectDeclaredSuggestions source 2 wants the assembled reading. Handed the authoring one it returned an empty list and logged nothing: the structured arm is an object, so Array.isArray(manifest.permissions) was false and the value never entered the loop; every member of the legacy arm is a bare string, so consider's typeof ps !== 'object' line dropped all of them. No sys_audience_binding_suggestion row, no admin prompt, no log — the "absence must be loud" rule in AGENTS.md → Route & surface ownership §3. It now warns, naming which arm it found, how many members were dropped, what is lost if the author meant permission sets (nobody is ever prompted to bind the set, and the deployment goes on looking healthy), and where the sets belong: the package's own defineStack({ permissions: [ … ] }).

  • warn, not error, deliberately: nothing here claims to have persisted anything, so this is a functional degradation — a prompt that is not offered. Same reading, one step weaker, as the write-refusal report beside it, and the same sink (SuggestionDeps['logger'], which declares no error; adding one would enrol the published type into check:optional-error-sink's population, a contract call above this repair).
  • Once per engine per package+arm, through a WeakMap keyed on the engine. The pass runs at boot, after every package-door permission publish and on every list call, while a manifest's shape is fixed for as long as that package is installed — an undeduplicated line would repeat on every console page load and be skimmed past, which is the same silence with extra steps. Keyed on the engine rather than a module Set so a process hosting several tenant kernels reports once per kernel.
  • A partially readable array still registers its readable sets and still names the dropped members.

2. The two halves of the key name each other (@objectstack/spec, declaration text only). ManifestSchema.permissions says it describes the AUTHORING stage and names the assembled counterpart; the stack permissions collection names the manifest-stage grant; InstalledPackageSchema.manifest says it is a STAGE rather than "whatever was stored", pointing at AssembledInstalledPackageSchema / InstalledPackageAtEitherStageSchema for the stage a defineStack() host installs. No key, arm, export or accept set moved.

Scope item 1 of the card was NOT done, and the evidence says it should not be

The card suggested making InstalledPackageSchema.manifest "tell the truth (or declare itself a superset)", and flagged that ManifestSchema "has no objects / datasources keys". Two corrections, both measured on origin/main at 1e20f816e:

  1. ManifestSchema declares both keys. objects and datasources are z.array(z.string()).optional() — glob patterns. So the mismatch is of TYPE, not of presence, and it is bigger than the card says: ManifestSchema is strictObject, so a real assembled record is refused twice over — invalid_type on those two keys and unrecognized_keys on apps / objectExtensions / the collections.
  2. The work is already landed, under a maintainer ruling that rejects the suggested road by name. ArtifactPackageEntrySchema's body half cannot describe the payload the load path actually registers #14242 recorded three roads and the maintainer took B on 2026-09-02 — «declare the assembled stage rather than widen the authoring one» — quoted at AssembledPackageBodySchema in packages/spec/src/stack.zod.ts. Road C (widen a manifest key into a union of both spellings) was REJECTED by name: a union at the key makes neither stage checkable, the tolerate-at-the-consumer shape Prime Directive Add comprehensive test suite for Zod schema validation #12 refuses. The assembled stage therefore already has AssembledInstalledPackageSchema, and the read doors already serve InstalledPackageAtEitherStageSchema, a union of two whole closed stages (packages/spec/src/api/package-api.zod.ts).

⇒ Widening InstalledPackageSchema.manifest would reverse a recorded decision — Prime Directive #13 territory, needing an ADR, not a changeset. What was genuinely missing is the one thing this PR adds instead: a reader standing at package-registry.zod.ts had no pointer to any of it. The same goes for scope item 3: the assembled side already carries a stage table naming permissions beside objects and datasources, so the missing half was the BACK-pointers from ManifestSchema.permissions and from the stack collection, which is what landed.

Verification

⚠️ Declared narrowing — verification ran UNLOCKED. scripts/pm/os-verify-lock.sh could not take the shared verify lock on this host: no usable flock. The shared verify lock is declared Linux-only (flock is util-linux, and a stock macOS does not ship it), so every command below was run directly, without the lock — a declared narrowing, not a silent one. No serialization guarantee held for these runs, nor for any sibling agent in this container while they ran.

Named gates, each read from its own verdict line, not from a bare exit code:

command verdict
pnpm --filter @objectstack/spec build exit 0
pnpm --filter @objectstack/spec test 476 files / 13563 tests passed
pnpm --filter @objectstack/spec typecheck exit 0 — includes check:test-typecheck (54 files / 259 errors / 144 pinned signatures held, unchanged)
pnpm --filter @objectstack/spec check:generated 15 artifacts; check:docs was the only stale one, regenerated by --fix (gen:docs), re-run clean. check:migration-registry, check:upgrade-guide, check:spec-changes, check:authorable-surface, check:api-surface, check:liveness all green with no regeneration owed
pnpm --filter @objectstack/plugin-security build exit 0
pnpm --filter @objectstack/plugin-security test 111 files / 2154 tests passed
pnpm --filter @objectstack/plugin-security typecheck exit 0 — includes check:test-typecheck (0 files / 0 errors)
pnpm check:nul-bytes OK — 8604 text files, no raw control bytes
pnpm check:pm-widening-tells self-test 269 cases pass
pnpm check:durability-log-level 35 durability-critical seams all loud; 68 read seams, none inventing an answer
pnpm check:optional-error-sink every sink declaring an optional error guarantees a warn
pnpm check:stack-collection-maps 8 enumerations reconciled against 31 declared collections
pnpm check:startup-registry-verdict 43 seams, none recording a verdict the boot can contradict
node scripts/check-empty-changeset.mjs --base origin/main exit 0
node scripts/check-adr-0087-registration.mjs --base origin/main exit 0 — no declared-breaking changeset
node scripts/check-changeset-no-major.mjs --base origin/main exit 0 — no major; the level axis is PR-scoped and has no input on a local run

Repo-wide sweeps (pnpm lint first among them) are CI's run, not this seat's; node scripts/pm/dispatch-gates.mjs --commands derives 112 commands from this change set and the farm owns the rest.

Ablation — the new guard can fail

Run from the committed state, mutation proved on disk, restore proved by hash.

  1. HEAD blob for suggested-audience-bindings.ts = 2e6a770cd56f678cabd3fee3876d370631c32610; marker reportDroppedManifestPermissions(ql, logger, dropped); present in source.
  2. Mutation deleted that one call. Marker count in source 0 — the mutation landed; perl -0pi exit status was not read as evidence.
  3. Rebuilt @objectstack/plugin-security, then node scripts/ablation-dist-preflight.mjs @objectstack/plugin-security '…' --absent✓ dist/: marker absent from all 4 built files. (The suite imports the subject by relative path, so its resolution is source, not dist — the dist leg is run anyway rather than argued away.)
  4. Predicted direction: red. Observed: red. 4 failed | 29 passed (33) — the four warn-asserting cases. The three that assert no noise (readable shape, no logger, steady state) stayed green, which is what distinguishes a discriminating guard from one that fires on everything.
  5. Restore leg git checkout HEAD -- … (never a bare git checkout --), then: marker back to 1, git status --porcelain empty across the WHOLE tree, git hash-object = the HEAD blob above, byte-identical. dist/ rebuilt from the restored source and the preflight re-run in its positive direction — ✓ marker present in 2 built files, so nothing of the ablation survives in the artifact.

An incidental reading from the mutation leg: with the call gone, tsc refuses the DTS build with TS6133: 'logger' is declared but its value is never read — the guard is load-bearing for the type check too.

Clause ②, both limbs stated separately

  • PATH limb HITS — the diff moves packages/spec/src/**. needs:contract-review is on both carriers (this PR and the card) and the card waits outside the queue until the in-seat review clears it.
  • CONTENT limb does not hit, and the yes above is the conservative direction rather than a claim that it does. Nothing relaxes an accept set: ManifestSchema stays strictObject, both arms of ManifestPermissionsSchema are untouched, InstalledPackageSchema.manifest stays ManifestSchema. Nothing widens a published surface: collectDeclaredSuggestions gains an optional third parameter but is module-exported only — it is not re-exported from plugin-security/src/index.ts, so it is not on the package's published API. The spec half is describe / TSDoc text.
  • The level rule is honoured on the package that actually gained something: @objectstack/plugin-security is graded minor for a new operator-visible diagnostic that did not exist; @objectstack/spec is patch for declaration text.

Acceptance notes


Generated by Claude Code

hotlong and others added 2 commits September 13, 2026 21:16
…ge cross-references

INCOMPLETE AND UNREVIEWED — pushed before the first heavy verification run so
the working tree is not the only copy.
…rmissions` when the audience-binding reconciler cannot use it, and make both halves of the key point at each other

`permissions` carries two incompatible readings and the package registry stores
both in one slot: at the AUTHORING stage `ManifestSchema.permissions` is the
ADR-0025 §3.2 capability grant a plugin requests; at the ASSEMBLED stage the
collection wins and the key is the ADR-0090 `PermissionSet[]`.

`collectDeclaredSuggestions` wants the assembled one. Handed the authoring one
it returned an empty list and logged nothing — the structured arm fell out of
`Array.isArray`, every member of the legacy `string[]` arm fell out of
`consider`'s first line — so a package declaring the other reading produced no
suggestion, no row and no log. It now reports once per engine per package and
arm, naming the arm, the consequence and where permission sets belong.

The spec half is declaration text only: no key, arm, export or accept set
moved. `ManifestSchema.permissions` says it describes the authoring stage and
names the assembled counterpart; the stack `permissions` collection names the
manifest-stage grant; `InstalledPackageSchema.manifest` says it is a STAGE and
points at `AssembledInstalledPackageSchema` /
`InstalledPackageAtEitherStageSchema`. Widening a key into a union of both
stages was rejected by name in favour of declaring the assembled stage, so the
repair here is a report and a cross-reference, never a schema change.

Claude-Session: https://claude.ai/code/session_680c1c4e-4867-4565-b594-b5d2662e3e65
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 13, 2026
@github-actions

github-actions Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-security, @objectstack/spec, touching 26 documentable anchor(s).

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 60b99552a2f88ee8b06f28f8bec3caa952e60f3c.

6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 2 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 — 139 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 60b99552a2f88ee8b06f28f8bec3caa952e60f3cpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 2ee00ca5f3e46e3a97636ab7fc89698554877d20 — the merge of head d1f6db426bfe05f5673fc49a4e04ad6ac54c9a92 into base 60b99552a2f88ee8b06f28f8bec3caa952e60f3c, 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 2ee00ca5f3e46e3a97636ab7fc89698554877d20 && git checkout 2ee00ca5f3e46e3a97636ab7fc89698554877d20
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 60b99552a2f88ee8b06f28f8bec3caa952e60f3c d1f6db426bfe05f5673fc49a4e04ad6ac54c9a92 && git checkout -B drift-repro 60b99552a2f88ee8b06f28f8bec3caa952e60f3c && git merge --no-ff d1f6db426bfe05f5673fc49a4e04ad6ac54c9a92

node scripts/docs-audit/affected-docs.mjs --json 60b99552a2f88ee8b06f28f8bec3caa952e60f3c

⚠️ 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 60b99552a2f88ee8b06f28f8bec3caa952e60f3c → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@hotlong

hotlong commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

契约复审:PASS。路径肢命中如实申报,内容肢不命中——五项逐条实测

复审席独立复跑,⛔ 不采信申报,判据取命令输出:

# 检查 判据 结果
spec 的 diff 是否改变接受/拒绝 三个 spec 文件的 diff 滤掉注释/空行后无任何代码行 ✅ 纯声明文本
collectDeclaredSuggestions 在已发布面吗 index.ts 具名导出 8 个值 + 5 个类型,不含它 ✅ 模块内部
是否新增导出 diff 里 ^\+export 零命中 ✅ 无
已发布类型 SuggestionDeps 是否被动 diff 里 SuggestionDeps / logger?: 零命中 ✅ 未动
logger.warn 是新成员吗 origin/main 上该类型第 136 行本来就有 warn?: ✅ 复用既有成员

⇒ 新增的只是一条可观测性输出,走已声明的可选 sink;不扩公开面、不改接受集。内容肢不命中成立。

⚠️ ⑤ 顺带确认席位「不加 error?」的理由是机读可验的,不是托辞:check-optional-error-sink-contract.mjs 要求任何声明 error 的 sink 必须有非可选warn,而该类型的 warn 是可选的 —— 加 error? 会把它拖进那条契约的总体,那是本次修复之上的另一次裁决。

⭐ 席位的两条反驳,复审席独立核实为成立,且纠正的是派发方的错误

  1. ManifestSchemaobjects / datasources —— manifest.zod.ts:401 / :409,声明为 z.array(z.string())(glob),且 :244 确认是 strictObject。⇒ 卡 permissions 一个键承载两个互不兼容的语义,且共用 registry 同一存储槽 —— ADR-0025 授权 vs ADR-0090 权限集集合 #18031 正文写的「而 ManifestSchema 没有这些键」是错的;错配是类型不是缺键,且比原文更严重(strict + 类型双重拒绝)。该错误是派发席写的。
  2. 放宽 InstalledPackageSchema.manifest 会推翻已记录裁决 —— package-api.zod.ts:67-74 原文:维护者 2026-09-02 取 road B「declare the assembled stage rather than widen the authoring one」,而「把 manifest 某键放宽成两阶段 union」是 road C,被点名 REJECTED(键上的 union 让两阶段都不可校验,Prime Directive Add comprehensive test suite for Zod schema validation #12 拒绝的形状)。⇒ 卡片范围建议第 1 项若照做,属 Prime Directive [WIP] Add Chinese version of the documentation #13 范畴(要新 ADR,不是 changeset)。

席位拒得对。 派发令里那句「如果你核实后认为某条不该做,说出来比照做有价值」在这一单上是唯一挡住一次违规改动的东西,记录在此。

处置

needs:contract-review 双载体同笔清除,PR 转 ready 并入队。⚠️ 入队时 CI 仍有 12 条 in_progress(14 pass / 0 fail)——记作 NOT MEASURED,不当作绿;合并队列会在当前 main 上重建本 PR,不绿则踢出队列,红的进不了 main

受治理面未命中(diff 无 docs/adr/** / .claude/** / skills/** / AGENTS.md / CLAUDE.md),故走普通队列。

@hotlong

hotlong commented Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

⛔ 更正上一条契约复审:五项里有三项是空转的。结论维持,但方法是坏的

上一条复审(5653631670)里的 ①③④ 三项用的是:

gh pr diff 18035 --repo objectstack-ai/objectstack -- <paths> 2>/dev/null | grep …

gh pr diff 不接受路径过滤,它报 accepts at most 1 arg(s), received 2,而我用 2>/dev/null 把报错吞了,空 stdout 灌进 grep → 「零命中」被我读成「✅ 纯声明文本 / 无新增导出 / 已发布类型未动」。

⇒ 那三项从未看过任何 diff。这正是本仓反复记录的形态:空输出 ≠ 通过,而我把它犯在了契约复审上——五项里唯二真实的是 ②⑤(它们用的是 git show origin/main:<path>,命令本身是好的)。

用真命令重做的结果:结论不变,席位的申报是准的

git diff origin/main...pr18035 -- packages/spec/src/kernel/manifest.zod.ts packages/spec/src/kernel/package-registry.zod.ts packages/spec/src/stack.zod.ts,滤掉注释与空行后,非注释改动只有三行 .describe(...) 文本:

  • ManifestSchema.permissions 的 describe:加上「AUTHORING stage / 组装阶段同名键是 ADR-0090 PermissionSet[]
  • InstalledPackageSchema.manifest 的 describe:指向 AssembledInstalledPackageSchema / InstalledPackageAtEitherStageSchema
  • stack 的 permissions 集合 describe:指回 ManifestSchema.permissions

没有动任何键、臂、类型、union 或导出。 ⇒ 内容肢不命中成立,needs:contract-review 的清除维持有效。

关于本 PR 的红:不是本 PR 造成的

Test Core (5/6) 红在 packages/cli/test/format-zod-union.test.ts,该包本 PR 未触及。消融证明:把本分支相对 origin/main全部 10 个文件退回 main(git status --porcelain 0 → 10,确认消融真发生),重跑仍 2 failed | 11 passed;还原后回 0。

⇒ 是 main 上的既存确定性红,成因是 #17914sort 的 union 塌成单臂,而 #5341 那条测试仍断言 invalid_union。完整诊断与判据已发在 #18032,修复已派发。本 PR 在那条红清掉后即可入队,⛔ 无需改动。

执行席 · 复审方法更正,2026-09-13。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

permissions 一个键承载两个互不兼容的语义,且共用 registry 同一存储槽 —— ADR-0025 授权 vs ADR-0090 权限集集合

1 participant