fix(runtime): name the ADR-0025 reading of manifest.permissions when AppPlugin's security registrar cannot use it - #18039
Merged
hotlong merged 2 commits intoSep 13, 2026
Conversation
…7 security registrar cannot read `AppPlugin`'s security-metadata registrar flattens the manifest under the stack's own collections, so `manifest.permissions` is read whenever the stack declares no `permissions` collection of its own. That key is the ADR-0025 §3.2 capability grant a package requests; the registrar wants ADR-0090 `PermissionSet[]`. Both arms were skipped with nothing logged — the structured arm is not an array, and every member of the legacy flat list carries no `name`. Skipping stays the outcome: the registrar is NOT made tolerant of the grant reading, and which items register is byte-for-byte unchanged. What changes is that the drop is audible — once per boot, naming the field, how many entries were lost, both readings of the key, and where permission sets belong. It stays silent on every shape where nothing was lost. Claude-Session: https://claude.ai/code/session_680c1c4e-4867-4565-b594-b5d2662e3e65 Co-authored-by: Claude <noreply@anthropic.com>
Contributor
📓 Docs Drift CheckNothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs. What this run could not see
Coarse fallback — 25 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
hotlong
marked this pull request as ready for review
September 13, 2026 15:25
hotlong
enabled auto-merge
September 13, 2026 15:25
hotlong
deleted the
claude/issue-18034-appplugin-silent-permissions-drop
branch
September 13, 2026 16:13
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #18034
Clause-②: no
AppPlugin's ADR-0057 security-metadata registrar flattens the manifest under the stack's own collections —{ ...manifest, ...collections }— somanifest.permissionsis read whenever the stack declares nopermissionscollection of its own. At the authoring stage that key is the ADR-0025 §3.2 capability grant a package requests (a flat list of permission strings, or{ services, hooks, network, fs }); the registrar wants ADR-0090PermissionSet[]. Both arms were skipped with nothing logged — the structured arm is an object, soArray.isArraywas false and the value never entered the loop; every member of the flat list carries noname, so!item?.namedropped all of them.What changed
One file of runtime behaviour:
packages/runtime/src/app-plugin.ts.The loop now measures what it lost and says so, once per boot, naming the field, how many entries were dropped, both readings of the key, and where permission sets belong. Example, on the legacy arm:
warn, noterror— nothing here claimed to persist anything, so this is a functional degradation: a permission set is simply not registered and the next person to look for it finds out (AGENTS.md → Degradation log levels).start()runs once per app per kernel, so the report sits in the loop that already runs once. (The sibling repair needed aWeakMapbecause its pass also runs on every list call; this one does not.)roles→positions) and a key the conversion produced came from the stack.The card asked for one measurement first — here it is, and it changes the scope
The card closed with "whether [the other four fields] can be reached by a colliding manifest key is unmeasured here and is the first thing to check." Measured against
ManifestSchema(packages/spec/src/kernel/manifest.zod.ts), onorigin/mainata0dd872c1, by parsing a probe manifest per field:SECURITY_FIELDSentryManifestSchemapermissionsManifestPermissionsSchemastring[]accepted ·{ services, … }acceptedcapabilitiesretiredKey()tombstoneinvalid_type@capabilities(expectednever)positionsunrecognized_keyssharingRulesunrecognized_keysManifestSchemais astrictObject, so the two undeclared keys are not merely absent — they are refused, which is a stronger reading than "absent" and is why no repair is owed on that side.capabilitiesis the opposite of this defect: the tombstone types the keyneverfortscand raises its own prescription at parse. Onlypermissionsis a real collision.The report is still written per field rather than for
permissionsalone — the bundle reaching this block is not necessarily one that passed an authoring parse (AppPlugin's constructor does not parse), and this loop is the last reader before the value is gone. Cost is one branch; the non-permissionstext says what it can honestly say.Two corrections to the card's premise, both measured
Neither kills the finding — the silence is real and the repair stands — but the scenario the card gives as its motivation does not exist, and the one that does is worse.
1.
manifest: { permissions: [{ name, isDefault }] }is not a silent drop in either direction. The card describes the trap as "an author who writes permission sets on the manifest … gets no set registered". Measured:defineStack()at its default strictness, that shape is REFUSED at authoring:defineStack validation failed (1 issue): ✗ manifest.permissions: Invalid input. It never reaches this loop.name, so the loop reads them. Pinned as a discrimination case below — the guard must not turn a working shape into a warning.2. The reachable trap is the one the card did not name, and it passes the strict parse.
manifest: { permissions: ['sales_rep'] }— an author naming a permission set — is schema-VALID, because a flatstring[]is the ADR-0025 legacy arm. It reaches the loop, every member is dropped, and nothing is logged. That is the shape this PR makes audible, and it is the one an AI author writing "permissions" on the manifest actually produces.Verification
scripts/pm/os-verify-lock.shcould not take the shared verify lock on this host: no usableflock. The shared verify lock is declared Linux-only (flockis util-linux, and a stock macOS does not ship it), so the commands below were 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.Each verdict is read from the line the gate itself prints, never from a bare exit code.
pnpm --filter @objectstack/runtime testpnpm --filter @objectstack/runtime buildcheck-dts-emitted: 2/2 declared declaration file(s) presentpnpm --filter @objectstack/runtime typecheckcheck:test-typecheck(27 files / 191 errors / 69 pinned signatures held, unchanged)pnpm --filter '@objectstack/runtime^...' buildGate families derived from the real change set rather than guessed:
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackreports 60 commands from 3 paths at7049b4ccf. All 60 were run, plus the 5 artifact-roster families the tool marks ⛔ because their roster sits in a directory one of these paths is in (check-changeset-fixed,check:authz-resolver,check:error-code-casing,check:filter-alias-parity,check:route-ledger-census). 63 green.Two are NOT MEASURED, not failures — both exit 3,
PREREQUISITE NOT MET, and both say so in their own words:pnpm check:dual-build-cjs-loads— "this gate reads built output, and some package has no dist/ … ⛔ This is NOT a pass: nothing was measured" (37 packages unbuilt in this worktree).pnpm check:type-check-debt— "--re-measure cannot run: 1 workspace dependency … has no built type entry point on disk". CI builds the whole closure and owns both.Among the green:
check:stack-collection-maps(8 enumerations reconciled against 31 declared collections — the gate that pins this verySECURITY_FIELDSlist),check:route-ledger-census,check:nul-bytes,check:registry-log-declared,check:cross-package-test-inputs,check:test-source-alias,check:empty-changeset --base origin/main,check:adr-0087-registration --base origin/main,check:changeset-no-major --base origin/main.Lint, narrowed and proved narrow. Repo-wide
pnpm lintis CI's run. The narrowing here is a measurement, with all three pieces: ① the population is read from eslint's own config, which declares noparserOptions.projectand no typed@typescript-eslintrules (eslint.config.mjs); ②--format jsoncounts 2 files linted, 0 errors, 0 warnings on the two changed sources; ③ because type-aware linting is not enabled, this diff cannot move the verdict on any file it does not contain. Run at7049b4ccf.Ablation — the new guard can fail, and it discriminates
Run from the committed state. Mutation proved on disk by marker count and blob hash, never by an editor's exit code; the mutating shell carried
trap '… git checkout HEAD -- <abs path>' EXIT INT TERM.HEADblob forpackages/runtime/src/app-plugin.ts=37fce89a82c9d776adecc012d4a7b0cb6d942603, equal to the on-disk hash. Markerregistrar, which cannot read itpresent 1× in source.fe3500f7835cecd6d1def60127dfecce176cfdd9;git diff --stat= 23 deletions.@objectstack/runtime, thennode scripts/ablation-dist-preflight.mjs @objectstack/runtime 'registrar, which cannot read it' --absent→✓ dist/: marker absent from all 4 built files. (The suite imports the subject by relative path, so it resolves source, notdist— the dist leg was run anyway rather than argued away.)3 failed | 4 passed (7)— and the split is the point. The three failures are exactly the cases that assert a line exists. The four that assert silence stayed green: a stack declaring its ownpermissionscollection, a manifest with no such key, a manifest whose entries the registrar really can read, and thesecurityMetadataRegistrar: 'artifact-door'composition. A guard that fired on every boot would have kept those green too only by never being asserted — they are asserted, so this is discrimination, not noise.git checkout HEAD -- <abs path>(⛔ never a baregit checkout --), then: marker back to 1, disk hash back to37fce89a…byte-identical to the HEAD blob, and a whole-treegit status --porcelainprinting nothing.dist/rebuilt from the restored source and the preflight re-run in its positive direction —✓ dist/: marker present in 2 built files·✓ tree: working tree clean against HEAD— so nothing of the ablation survives in the artifact. Suite back to7 passed (7).An incidental reading from the mutation leg: with the report gone,
tscrefuses the DTS build —TS6133: 'fromManifest' is declared but its value is never readand the same for'members'. The guard is load-bearing for the type check too, so it cannot rot into a dead branch unnoticed.Clause ②
no. The PATH limb does not hit: the diff ispackages/runtime/src/app-plugin.ts, one new test file beside it, and a changeset. Nothing underpackages/spec/src/**moved — verified on the actual diff, not on the plan. The CONTENT limb does not hit either: no accept set is relaxed, no published surface widens (the new code is inside a private method body), and no schema, key or arm is touched.Acceptance notes
docs/adr/**,.claude/**,skills/**,AGENTS.md,CLAUDE.md), so this is an ordinary merge-queue candidate once CI is green.!item?.nameline also drops a nameless entry arriving from the stack's own collection, wherePermissionSetSchemarequiresnameand a strictdefineStack()already refuses it. That is a different origin with a different remedy (and the producer is already loud), so no card: the guard here is gated on manifest origin precisely to keep the two apart. Carrier: none — the shape is unreachable through an authoring parse.packages/runtime/src/app-plugin.ts's flatten exists to support a bundle whose collections sit undermanifest:, which is why the narrower repair ("stop reading the manifest for these four fields") was rejected here: it would silently un-register that shape, replacing one silence with another. Recorded because a future reader will consider the same shortcut. Carrier: this PR's diff comment.permissions一个键承载两个互不兼容的语义,且共用 registry 同一存储槽 —— ADR-0025 授权 vs ADR-0090 权限集集合 #18031, repaired in a separate PR onplugin-securityandspec. Neither PR depends on the other;permissions一个键承载两个互不兼容的语义,且共用 registry 同一存储槽 —— ADR-0025 授权 vs ADR-0090 权限集集合 #18031 is not addressed here.CI on this PR — the one red is not this diff
Lint & Repo Gates,Type Check(all four legs),Build Core,Dogfood Regression Gate(3/3),Dogfood Verify CLI,Temporal Conformance (live PG + MySQL)and five of sixTest Coreshards are green: 30 pass · 2 fail · 3 skipping, the two failures beingTest Core (3/6)and theTest Corerollup that reports it.Test Core (3/6)fails inpackages/cli/test/format-zod-union.test.ts, and it is #18032's signature — the standing queue-flake anchor for that exact file, already dispatched to another seat. Excluded from this diff by measurement, not by argument:a0dd872c1is exactly three paths —packages/runtime/src/app-plugin.ts, one test file beside it, and a changeset. Nothing underpackages/spec/**orpackages/cli/**.ObjectStackDefinitionSchema.safeParsedirectly and spawns no CLI. Reproduced locally against@objectstack/specbuilt froma0dd872c1with nothing modified: the specimen yields 2 issues (invalid_valueonviews.0.list.sort.0.order,unrecognized_keysfordirection) where the test assertstoHaveLength(1).So it is an assertion, deterministic and off-queue — not a timeout and not a queue interaction. The full reproduction, the schema reading behind it (
order: z.enum(['asc','desc'])carries neither.optional()nor.default()) and the two open dispositions are recorded on #18032 as evidence rather than as a duplicate card; #18032 is not addressed here.⛔ Not armed for auto-merge while that shard is red — a red PR does not queue, it hides.
Generated by Claude Code