Skip to content

feat(spec): ISecurityService declares discardPermissionSetOverlay and contributeOwnershipFloorAlternates as optional, feature-detected members - #21781

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-21756-security-service-members
Oct 5, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-21756-security-service-members

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21756

Clause-②: yes (widening)

ISecurityService now declares the two members that the registered security service already served without a declaration: discardPermissionSetOverlay and contributeOwnershipFloorAlternates. Both are optional, documented as feature-detected, and pinned by a contract-test row each. A new test-only enumeration pin in plugin-security turns red, by name, when the registered service serves a member that is neither declared on the contract nor ledgered with a reason. This follows the direction triage set (5981803299) and the claim 5982111525. No runtime source changes, and no behaviour changes.

Measured first: what is served vs what is declared

Read at base a6a7547074. registeredSecurityService in packages/plugins/plugin-security/src/security-plugin.ts (:1881 typed literal + :2106 Object.assign extension) serves 21 members. I measured them by enumerating the object the real plugin registers, not by reading the source:

canExport, canReadObject, checkAuthoredRowWrite, confirmAudienceBindingSuggestion, contributeOwnershipFloorAlternates, describeDelegableScope, describeDelegationNarrowing, discardPermissionSetOverlay, dismissAudienceBindingSuggestion, explain, getEffectiveObjectPermissions, getMetadataReadableFields, getQueryableFields, getReadFilter, getReadableFields, getWritableFields, hasWriteBypass, listAudienceBindingSuggestions, resolvePermissionSetNames, resolvePermissionSetsForContext, resolveWriteScope.

  • Served but not declared: exactly the two this card names. Every other member is in the typed literal, so the compiler already holds it to the contract. No third member was found, so the pin's ledger is empty.
  • Declared but not served: none. All 19 previously declared members (11 required, 8 optional) are served.

The two callers and what the members really refuse

  • discardPermissionSetOverlay(callerContext, id): called by packages/rest/src/rest-server.ts (POST …/security/permission-sets/:id/discard-overlay, about :12699). The route feature-detects it and answers 501 NOT_IMPLEMENTED when it is absent. The implementation (permission-set-overlay-discard.ts) refuses with PERMISSION_DENIED 403 (the caller is not a tenant-level admin, or no installed package declares the set), NOT_FOUND 404 (unknown row) and INVALID_STATE 409 (no active overlay). The last two are the ERROR_CODE_LEDGER['@objectstack/plugin-security'] rows. A refused re-projection write still resolves (healedObjectGrantCount then equals the pre-discard count). The docblock says so.
  • contributeOwnershipFloorAlternates(plugin, alternates): called at boot by packages/services/service-storage/src/attachment-delete-floor-alternate.ts, through a local seam interface and feature detection. The implementation (ownership-floor-alternates.ts) throws a plain Error with no registered code when: plugin is empty, the list is not an array, an alternate names no object or names '*', its operation is not exactly update or delete, its using is missing, or the policy does not parse as RowLevelSecurityPolicySchema.

What changed

  • packages/spec/src/contracts/security-service.ts: two optional members, documented in the getMetadataReadableFields pattern (what each does, what it refuses with which codes, that callers feature-detect, that the unguarded call does not compile). Two parameter and result types are plugin-internal (PermissionSetOverlayDiscardResult and OwnershipFloorAlternate in plugin-security), so the spec declares the minimal contract shape of each under the same name, and the docblocks say so. They are the only new public exports, both type-only (api-surface/contracts.json +2, export-origins/contracts.json +2, regenerated by check:generated --fix, not by hand).
  • packages/spec/src/contracts/security-service.test.ts: one contract-test row per member, appended after the existing rows. Each shows that absence is typed (the unguarded call is a @ts-expect-error), how the caller handles the absent branch, and that the present member is called as declared. The second row also shows the type rejects operation: 'all'. No existing title is edited.
  • packages/plugins/plugin-security/src/registered-security-service-members.pin.test.ts (new, test-only): the enumeration pin.
  • .changeset/21756-security-service-declared-members.md: @objectstack/spec minor, Clause-②: yes (widening).

The pin, and why the declared list is test-local

The pin boots the real SecurityPlugin (init + start) and takes the object it passes to registerService('security', …). It walks every own key along the prototype chain, so a class-backed service would not pass over zero members. It fails, by name, on any member that is in neither DECLARED_MEMBERS nor SERVED_NOT_DECLARED. Its non-vacuity control requires every required member to be among the enumerated ones.

It needs a runtime list of declared members. I chose a test-local DECLARED_MEMBERS map held to the interface by satisfies { readonly [K in keyof ISecurityService]-?: 'required' | 'optional' }, computed per member. If the interface gains a member and the list does not, the list stops compiling. A name the interface lacks, or a wrong required/optional tag, also stops it compiling. The compile half runs in this package's typecheck (tsconfig.test.json compiles every test here, at zero debt). No new spec export was needed. A runtime list exported from packages/spec would have grown the published surface to serve one test, and would still need a clause like this one to keep it equal to the interface. The pin's boot fake has no engine write or read verb (objectql carries only registerMiddleware and getSchema), so it is not a double the check:engine-double-contract family scans. A third case is compile-only: it types a witness of each extension member's contract signature, delegating to the implementation function the registered member delegates to. If the contract and the implementation disagree, that case stops compiling.

Proof the pin can fail (predicted first, run from committed state)

Run from commit ff69d4c4eb. Mutations went through node scripts/ablation-replace.mjs (anchor must hit, blob must move, restore proven by blob == HEAD and an empty git diff HEAD). The pin imports ./security-plugin.js by relative path, so no dist/ sits between the mutation and the run.

  1. Runtime half. Prediction: test 1 red, naming zzScratchServedMember; tests 2 and 3 green. I planted zzScratchServedMember: () => undefined in the Object.assign extension of security-plugin.ts (blob bf796ff10c8d -> cd61be11613c). Observed, as predicted: AssertionError: served by the registered security service but neither declared on ISecurityService … expected [ 'zzScratchServedMember' ] to deeply equal [], Tests 1 failed | 2 passed (3). Restored to blob bf796ff10c8d == HEAD, git diff HEAD empty. (My first attempt used an anchor that the replacement still contained. The tool refused it before running anything, so it measured nothing. The second attempt is the measurement.)
  2. Compile half. Prediction: tsc -p tsconfig.test.json red at the satisfies clause, in this file only. I dropped contributeOwnershipFloorAlternates from DECLARED_MEMBERS. Observed: registered-security-service-members.pin.test.ts(77,12): error TS1360: … does not satisfy the expected type 'DeclaredOptionality', the only error in the program. Restored to blob == HEAD. This also proves the test program read the rebuilt spec .d.ts: against a stale one without the new member, the unmutated list would be the red one (an excess property).

Verification, on the final tree

All runs below are at f2f466c4a9 (the branch merged with origin/main ebfe658c72, artifacts regenerated, changeset committed).

  • pnpm --filter @objectstack/spec exec vitest run --project local --maxWorkers=2 src/contracts/security-service.test.ts: Tests 23 passed (23) (21 before + 2).
  • pnpm --filter @objectstack/spec exec vitest run --project local --maxWorkers=2 (whole package): Test Files 615 passed (615), Tests 18360 passed | 1 todo.
  • pnpm --filter @objectstack/spec typecheck: exit 0. The test layer compiles, and security-service.test.ts has no test-typecheck-debt.json entry, so its @ts-expect-error lines are live checks.
  • pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2 (whole package): Test Files 166 passed (166), Tests 3582 passed | 45 skipped.
  • pnpm --filter @objectstack/plugin-security typecheck: exit 0 (0 file(s) / 0 error(s) in the test-layer ledger).
  • pnpm --filter @objectstack/spec check:generated: exit 1 before the fix (exactly api-surface/ and export-origins/ stale, +2 interfaces each). After --fix re-checked them: ✓ check:api-surface, ✓ check:export-origins.
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 91 commands from the merge-base change set (6 paths). I ran all 91 with their exit codes recorded before any pipe: 89 exited 0. pnpm check:i18n and pnpm check:dual-build-cjs-loads first exited 3 (PREREQUISITE NOT MET, no dist/). Both were re-run after building their prerequisites (the closure check:i18n names, then a workspace pnpm build, all turbo cache hits) and exited 0: check-i18n-bundles: OK (9 package(s) — all bundles in sync …) and ✓ check:dual-build-cjs-loads — 106 published require entry point(s) across 66 package(s) load. dispatch-gates.mjs --ran over the recorded codes: 91 derived famil(ies) accounted for — 91 run, 0 NOT-MEASURED. The six artifact-roster gates whose roster sits under a touched directory (check-changeset-fixed, check:meta-url-spelling, check:spec-changes, check:authz-resolver, check:error-code-casing, check:filter-alias-parity) were also run, and all exited 0.
  • Lint (narrowed, a measurement rather than a skip): pnpm exec eslint --no-inline-config --format json over the three changed .ts files reports files 3 errors 0 warnings 0. All three are in the configured population (eslint --print-config resolves each). This repo's eslint.config.mjs enables no type-aware linting (no parserOptions.project, no projectService), so the diff cannot move the verdict on any untouched file. The full pnpm lint is CI's.

Consumers: the public-surface change is two optional interface members plus two type-only exports. The two callers do not type their access against ISecurityService: rest-server.ts holds the service as any (its provider returns a Promise of any), and service-storage uses its own local seam interface. So their compiled verdicts cannot move, and they are not re-run here. Every other package that references ISecurityService reads it as a Partial of ISecurityService or calls only pre-existing members (git grep over packages/**). The full consumer sweep is left to CI's workspace type-check lane.

Overlap

#21763 (#20749 stage 13) rewrites tracker ids in test titles of security-service.test.ts at lines 258, 287, 309, 471, 494 and 518. When this PR opened it was still in the merge queue, not on main. This PR edits no existing title. It adds one import specifier (line 8) and two rows after the last existing row (after line 560), so no added line is next to a title #21763 changes. Neither new title carries a tracker id. A local trial merge of this branch with #21763's head (git merge-tree --write-tree; this file is not routed to the regen merge driver, so the text merge is the same one GitHub runs) is clean. Whichever lands later merges origin/main (no rebase).

Acceptance notes

Noted, not filed. These are observations, not defects or contract violations. Each is out of this card's scope: the dispatch forbids editing security-plugin.ts, rest-server.ts and service-storage source.

  • Comments now stale. These comments still describe the members as undeclared extensions whose spec seat is "a separate change": security-plugin.ts around :2100–:2122 (the comments above the Object.assign), the header of ownership-floor-alternates.ts ("an EXTENSION of the published contract"), and the header of attachment-delete-floor-alternate.ts. Carrier: the next PR that edits those files.
  • The registration log line drifts. security-plugin.ts:2137 prints a hand-written member list. It names the two extension members, but omits hasWriteBypass, resolveWriteScope, describeDelegationNarrowing, getEffectiveObjectPermissions and describeDelegableScope. Log text only. Carrier: the next editor of security-plugin.ts. Holder: none.
  • Possible follow-up shape. With both members now on the contract, they could move from the Object.assign extension into the typed literal, where the compiler holds their signatures directly. That would make the pin's compile witness redundant for them. It is a plugin-security source change, so it is not done here.

Generated by Claude Code

@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/spec, touching 9 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/spec/api-surface/contracts.json, packages/spec/export-origins/contracts.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/kernel/contracts/index.mdx (via ISecurityService (symbol, a top-level interface))
  • content/docs/kernel/runtime-services/index.mdx (via ISecurityService (symbol, a top-level interface))
  • content/docs/permissions/access-matrix.mdx (via permissionSet (symbol, a field of interface PermissionSetOverlayDiscardResult))
  • content/docs/permissions/permission-sets.mdx (via /security/permission-sets/:id/discard-overlay (route, a path literal in a comment in ISecurityService; bridged from symbol discardPermissionSetOverlay — its route source's handler names it))
  • content/docs/ui/doc-pages.mdx (via permissionSet (symbol, a field of interface PermissionSetOverlayDiscardResult))

⛔ 5 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v13.mdx (via permissionSet (symbol, a field of interface PermissionSetOverlayDiscardResult))
  • content/docs/releases/v14.mdx (via permissionSet (symbol, a field of interface PermissionSetOverlayDiscardResult))
  • content/docs/releases/v17/17-0.mdx (via ISecurityService (symbol, a top-level interface))
  • content/docs/releases/v17/17-5.mdx (via ISecurityService (symbol, a top-level interface))
  • content/docs/releases/v17/17-6.mdx (via ISecurityService (symbol, a top-level interface))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 changed file(s) yielded no anchor (packages/spec/api-surface/contracts.json, packages/spec/export-origins/contracts.json) — pages documenting those are invisible to this run
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 138 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 8256a4b272f92908376d39518d1eb20914be483d → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 8256a4b272f92908376d39518d1eb20914be483d

⚠️ 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 8256a4b272f92908376d39518d1eb20914be483d → 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 01:13
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 5, 2026 01:13
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 5, 2026
Merged via the queue into main with commit 045f764 Oct 5, 2026
37 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-21756-security-service-members branch October 5, 2026 01:56
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