Skip to content

fix(plugin-security): a data-door edit of a permission set saved into a writable runtime package updates its own row instead of forking it - #21881

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-21861-write-through-update-keeps-package
Oct 5, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-21861-write-through-update-keeps-package

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21861
Clause-②: no

What this changes

Setup saves a permission set through the data door (PATCH /api/v1/data/sys_permission_set/:id). The security plugin's write-through (createPermissionSetWriteThrough, packages/plugins/plugin-security/src/permission-set-projection.ts) redirects that edit into the metadata store. Its update leg merged the patch into the stored body and called saveMetaItem with no packageId.

A sys_metadata row is keyed (org, type, name, package_id), and a save that names no package targets the package-less row (SysMetadataRepository.put: an omitted packageId is the unbound row). So for a set saved into a writable runtime package (PUT /api/v1/meta/permission/:name?package=PKG), whose only stored row is bound to that package, the edit answered 200 and minted a second, package-less active row carrying the edit. The package-bound row stayed as it was.

The update leg now reads the package binding of the row it edits and passes it as packageId:

  • The row is the overlay layer of the layered envelope the leg already holds. effectiveBodyForRow takes that layer as the base the patch merges into.
  • Its binding is read from the metadata door's own single-item read, protocol.getMetaItem({ type: 'permission', name }). That read serves the row through the same served-row resolution as the layered read (findServedOverlayRow, no package), and it states the row's package_id on the item as _packageId ("surface the persisted software-package binding", protocol.ts). The binding is not taken from the patch, from the projected record, or from a registry item.
  • No overlay layer means no stored row. Then there is nothing to fork, and the call is unchanged.
  • A package-less row carries no _packageId, so its save still names no package.
  • A code-shipped set is refused by the lock before this read (assertPermissionSetNotPackageDeclared runs in the pre-pass). The lock and its refusal do not move.
  • A failed binding read is not caught. Guessing the binding would choose which row the save lands in.

One file of production code, one new non-exported helper. No packages/spec or packages/metadata-protocol edit, no new error code, no new exported symbol, and no second classifier: every target reaching the save has already been classified org by classifyPackagedPermissionSet, so the binding read only tells a package-bound org set from a package-less one.

The dispatch hypotheses, measured

  • H1 (the update leg saves package-less and forks): confirmed. On origin/main 88a39c09 (this branch's base, which carries PR fix(plugin-security): the permission-set lock reads the row's provenance, so org-owned sets, clones and runtime-package sets edit again #21857), through the new door pin's own steps, the runtime-package set's first data-door edit answered 200. Its active rows went from one, bound to the package, to two: { package_id: null, description: "Edited at the data door" } and { package_id: "com.dogfood.bind21861", description: undefined }. The update leg is the saveMetaItem call at :1374 on this base. The :1297 named in the dispatch is the insert leg's call.
  • H2 (where the binding can be read): measured before any edit. The projected sys_permission_set record reads managed_by: "admin", package_id: null, so the projection does not carry the binding. The layered read's overlay layer has no _packageId, and its envelope packageId is null: the code layer is the projection echo. getMetaItem's item._packageId is com.dogfood.bind21861, both through GET /api/v1/meta/permission/:name and through protocol.getMetaItem in-process. That existing reader is the one used here.
  • H3 (only the runtime-package set needs the binding): held. It is pinned at the door: the package-less set saves package-less, and the code-shipped showcase_contributor is still refused with 403 NOT_OVERRIDABLE with its row count unchanged.
  • H4 (other legs): none forks the same way, so none is changed. Details are under Acceptance notes.

Pins

  • Door pin (new file packages/qa/dogfood/test/permission-set-write-through-package-binding.dogfood.test.ts, showcase). Every case counts the active sys_metadata rows for the name under both type spellings and in every scope, before and after the edit:
    • a runtime-package set: 200, exactly one row before and after, still bound to the package and carrying the edit. Edited twice, before and after the list read (GET /meta/permission) that every Studio page load issues;
    • a package-less set: 200, one package-less row before and after, carrying the edit;
    • a code-shipped set: 403 with code: NOT_OVERRIDABLE, and the stored rows are unchanged.
  • Unit (permission-set-projection.test.ts). The suite's existing protocol double now keys a stored row by its package, as the repository does, and serves the row's binding on getMetaItem. No new engine double, so the engine-double ledger does not move. Four cases: package-bound, package-less, a filtered edit spanning one of each (each saved into its own row), and the code-shipped refusal (code and status asserted, no save, the binding never read). Each case asserts the row count before and after.

Ablation (committed fix first, then mutated, then restored)

At 32048afa30, scripts/ablation-replace.mjs replaced the anchor item: body, ...packageArg, ...actorArg }); (1 hit, then 0) with item: body, ...actorArg }); (0 hits, then 1). This puts the package-less save back. Blob e77bd871 became 4186f983. The run was wrapped in a trap restore on EXIT, INT and TERM.

  • Rebuild: pnpm turbo run build --filter=@objectstack/plugin-security exited 1, at DTS only (TS6133: 'packageArg' is declared but its value is never read). The JS was already emitted, and node scripts/ablation-dist-preflight.mjs @objectstack/plugin-security "item: body, ...actorArg }" found the marker in 2 built files (dist/index.js, dist/index.mjs). The ablation was live in the artifact the door pin consumes.
  • Unit: 2 failed, 72 passed. The first red is the row count: expected [ …(2) ], the package-bound row plus { package_id: null, description: "edited at the data door" }.
  • Door pin: 2 failed, 2 passed. The runtime-package set's two cases went red with two active rows, { package_id: "com.dogfood.bind21861", description: undefined } and { package_id: null, description: "Edited at the data door" }. The package-less and code-shipped cases stayed green, as they should.
  • Restore: blob equal to HEAD (e77bd871), git diff HEAD empty, git status --porcelain empty across the whole tree. After the rebuild, --absent found the marker in 0 of 6 built files. Unit 74 of 74 passed, door pin 4 of 4 passed.

Tests (all at 32048afa30)

  • pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2: 167 files, 3604 passed, 45 skipped.
  • pnpm --filter @objectstack/plugin-security run typecheck, which includes check:test-typecheck over tsconfig.test.json: pass. pnpm --filter @objectstack/dogfood run typecheck: pass.
  • After a fresh closure rebuild (turbo run build --filter=@objectstack/dogfood^..., 63 tasks): dist preflight shows the fix's spelling present and the ablation marker absent. permission-set-projection.test.ts 74 of 74 passed. The door pins permission-set-write-through-package-binding and permission-set-lock-row-provenance (PR fix(plugin-security): the permission-set lock reads the row's provenance, so org-owned sets, clones and runtime-package sets edit again #21857's) passed 18 of 18.
  • Narrowed lint: eslint --no-inline-config --format json over the three changed .ts files reported 3 files, 0 errors, 0 warnings. The population is the config's **/*.{ts,…} globs, and the fourth path is the .md changeset. The config never enables type-aware linting (no parserOptions.project), so the diff cannot move a verdict on an untouched file. The repo-wide pnpm lint is left to CI.
  • integration-tier tests are not relevant (no packages/cli path).

Gates

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 70 commands at 32048afa30. All 70 were run, and each exit code was recorded. 69 exited 0 on the first pass. check:dual-build-cjs-loads exited 3 (PREREQUISITE NOT MET: 8 packages outside the dogfood closure had no dist/). Those packages were built (all cache hits), and the gate exited 0 on the rerun. --ran reports: 70 derived, 70 run, 0 NOT-MEASURED, 0 UNRUN. The derivation names 8 commands the dispatch-time list did not have, all changeset-related because this PR adds a changeset: check-adr-0087-registration (both), check-empty-changeset (both), release-rehearsal-clone --self-test, release-pending-publish --self-test, check:objectui-changeset and check:pm-changeset-deadline-census.

Acceptance notes


Generated by Claude Code

claude added 3 commits October 5, 2026 12:49
…ermission set updates its own row

The unit double now keys a stored row by its package as the repository
does, and serves the row's binding on the single-item read. The door pin
counts the active sys_metadata rows before and after each edit.

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
…set saves into its own row's package

The write-through's update leg saved the merged body with no package, and a
sys_metadata row is keyed by its package: for a set whose only row is bound
to a writable runtime package, the save minted a second, package-less row.
The leg now reads the edited row's binding from the metadata door's
single-item read (the row's package_id, stated as _packageId) and passes it
as packageId. A set with no stored row, or a package-less one, saves as
before; a code-shipped set is still refused by the lock first.

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
The row count is the contract; the save argument is the mechanism, so an
ablation's first red names the second row.

Claude-Session: https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 5, 2026
@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/permissions/system-context.mdx (via createPermissionSetWriteThrough (symbol, a top-level function))
What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 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; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 16 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 5e0b489bcacecf2ab6e91e20ed856ca2452d9c2e → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 5e0b489bcacecf2ab6e91e20ed856ca2452d9c2e

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

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 5, 2026 14:32
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 5, 2026 14:32
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 5, 2026
Merged via the queue into main with commit 07c842d Oct 5, 2026
37 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-21861-write-through-update-keeps-package branch October 5, 2026 15:08
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

2 participants