Skip to content

/auth/me/permissions and /me/apps re-implement permission-set resolution instead of calling the plugin's service — the copy that produced #7608 (and #7270, #7560) #7616

Description

@os-help

Restart-when: git grep -c "resolvePermissionSetsForContext" "$(git tag --sort=-creatordate | grep '^@objectstack/spec@' | head -1)" -- packages/spec/src/contracts/security-service.ts returns non-zero — i.e. the newest cut @objectstack/spec release carries the contract method, so plugin-hono-server may assume it and drop the Partial<ISecurityService> fallback.

Filed as a finding while implementing #7608 (PR #7615), which asked whether these endpoints should stop re-implementing this resolution at all. Measured there and deliberately not folded in; recording the measurement so the decision is on file rather than an unexamined copy.

The drift surface

packages/plugins/plugin-hono-server/src/current-user-endpoints.ts resolves the caller's permission sets itself, twice — once for the /auth/me/permissions object/field surface and once for the /me/apps capability + tab surface. SecurityPlugin.resolvePermissionSetsForContext resolves the same thing for the data plane. That is one rule with three copies, and it has now drifted twice in the way that shape always drifts:

PR #7615 shrank the surface where it was cheap — the rule is now one named effectivePermissionSetNames helper documented against the plugin's method by name — but the copy itself is still there.

What a real de-duplication costs (measured, not estimated)

The published ISecurityService (packages/spec/src/contracts/security-service.ts) exposes resolvePermissionSetNames(context), returning names only. Both handlers need the full resolved sets — objects, fields, systemPermissions, tabPermissions — for their merges, so today's contract cannot serve them. Extending it means:

  • packages/spec/src/contracts/security-service.ts — a new resolvePermissionSetsForContext-shaped method, plus security-service.test.ts, whose contract test asserts an exhaustive method list;
  • packages/plugins/plugin-security/src/security-plugin.ts — the method is private today; expose it on the registered service literal;
  • current-user-endpoints.ts — the call site.

3 packages, ~5 files.

Two things make it a net addition rather than a removal, which is why it did not ride on the #7608 fix:

  1. The contract mandates feature-detection. Consumers resolve the service as Partial<ISecurityService> by design, so this file must keep the local resolution as the fallback for a SecurityPlugin too old to carry the method — leaving the copy plus the delegation plus a branch between them, until that fallback can be dropped. De-duplication that adds a third path is not de-duplication yet.
  2. The two are not the same resolution. The plugin resolves through this.metadata / this.bootstrapPermissionSets / this.dbLoader; these endpoints pass their own loaders, and /me/apps deliberately projects a narrower column set (name + systemPermissions + tabPermissions) than /auth/me/permissions does. Delegating changes which columns load on both surfaces — a behaviour change wanting its own before/after measurement, not a refactor.

A third constraint worth stating so it is not rediscovered: plugin-hono-server must not take a runtime dependency on plugin-security, which is optional in the stacks these endpoints serve (the !evaluator degraded branches are exactly its absence). A service-locator call is structural and satisfies this; a package import would not.

Suggested shape

Sequence it rather than doing it in one PR:

  1. Add the method to ISecurityService + plugin-security, unused, so the contract exists and ships.
  2. Once a floor version carrying it can be assumed, switch both call sites and delete the local resolution — measuring the column-projection change on /me/apps at that point, since that is the part with user-visible reach.

Until step 2, the copy stands knowingly. Worth noting the cost of not doing it is measurable: three separate cards so far, each found after the divergence reached a user rather than at author time.

Related stale reference

plugin-security's resolveFallbackPermissionSets doc-comment still describes "the second step /auth/me/permissions takes when a caller's own names resolve to nothing (resolved.length === 0 && fallbackName)". PR #7615 deletes that step. Comment-only, in a package outside that PR's file surface — fold it into whichever PR does the above, or a docs-only pass.


Generated by Claude Code

Activity

  1. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Triage: finding + domain:spec — held observation, routed.

    Classification. Observation-class by the card's own framing ("the copy stands knowingly", sequenced two-step shape, decision recorded rather than requested now): the two user-facing effects of this drift (#7608, #7555) each have their own dispatched card and in-flight PR, so nothing here is something a user hits today beyond what those fix. Held as finding; the findings-triage round grades whether/when to promote, and promotion should split contract-first per multi-repo rule 2 (spec contract method first, then the plugin-security exposure + call-site delegation).

    Routing anchor (read, not guessed). Step 1 of the card's own suggested shape lands in packages/spec/src/contracts/security-service.ts — verified on origin/main @ 4ed4160 that the published ISecurityService carries only resolvePermissionSetNames(context): Promise<string[]> (names-only, line 289), which cannot serve the handlers' merges, exactly as the card measures. contracts/** is domain:spec territory per the lane table. The later call-site half (current-user-endpoints.ts → cli) and the service exposure (plugin-security → identity) get their own sub-issues at promotion time.

    Premise check @ 4ed4160. All live: the fallback-cliff lines sit at current-user-endpoints.ts:767 and :978, and PR #7615's effectivePermissionSetNames helper is not on main yet — the card's "PR #7615 shrank the surface" clause describes the in-flight branch, not main. Re-verify at promotion; the premise set will shift when #7615 and #7605 land.

    Dedup. Not a duplicate: #7608/#7555/#7560/#7270 are the drift's instances (all cross-linked in the body); #6334 is the closed precedent. No other open card asks for this de-duplication — this is the one entry point for it.

    target:v17. No — refactor/dedup with a deliberate deferral, none of the four blocking classes (the user-visible halves are #7608/#7555's cards, already in flight).

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Re-routed domain:spec → domain:cli by the spec-lane seat, executing the maintainer's lane-triage authorization (spec-lane PM session chat, 2026-08-11, verbatim: 「你的车道你可以执行 分类改标」; session session_01JY2Q5Xto1u8YHADgrZDTnk). Routing rationale (anchoring rule — the package the fix lands in): the fix is de-duplicating /auth/me/permissions and /me/apps's hand-rolled permission-set resolution onto the plugin's service — landing in packages/rest / plugin-hono-server, which the domain table routes to domain:cli. No spec acceptance-face change is proposed. finding grade untouched — held for the cli lane's own triage.


    Generated by Claude Code

  3. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Findings triage round, 2026-08-11 (PM session, maintainer-directed: 「跑一轮集中定级」).

    Graded: promote. One rule with three copies that has already drifted twice (#7608, #7270/#7560) will drift a third time; converging the two endpoint re-implementations onto SecurityPlugin.resolvePermissionSetsForContext is the governance-side-wins shape the maintainer has repeatedly ruled. M.

    finding → pm:queue (domain:cli).


    Generated by Claude Code

  4. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Claimed — domain:cli seat.

    • Session: session_011rK3z836HF4CjfpbJuEjaz
    • Branch: claude/issue-7616-permission-set-contract
    • Worktree: ../objectstack-7616 (dedicated)

    Dispatched as STEP 1 ONLY, following this card's own sequencing rather than overriding it: add the full-shape method to ISecurityService (+ its exhaustive-method-list contract test) and expose it on plugin-security's registered service literal, plus the comment-only fold-in for resolveFallbackPermissionSets' now-stale doc-comment (PR #7615 deleted the step it describes).

    ⛔ The call sites in current-user-endpoints.ts are explicitly out of scope, and so is deleting the local resolution. The card measured why and I am holding that line: /me/apps projects a narrower column set than /auth/me/permissions, so delegating changes which columns load on both surfaces — a user-visible behaviour change that wants its own before/after measurement, not a refactor riding along on a contract addition.

    Stating the tension rather than hiding it: step 1 ships a method with no caller, and because the contract mandates feature-detection the local copy stays as a fallback — so this leaves the copy plus the delegation path plus a branch between them. That is a net addition today. It is still the right first move, because step 2 is unreachable until a floor version carrying the method can be assumed. The dev's report is instructed to say plainly that behaviour does not change today rather than claim a fix.

    ⚠️ If a liveness / enforce-or-remove / dead-code gate objects to shipping an unused contract method, the dev reports and stops — no silencing, and no ratchet-ledger raise. I will take that to the maintainer if it comes up.

    Pin: declared = reachable. Assert the method is present on the service as registered and callable through the service locator, not merely declared on the interface — a declaration nothing exposes is exactly the failure this card exists to stop recurring.

    Governance note, since I escalated a different spec addition today (#7728). These are not the same class and I want the distinction on record: #7728 proposes a new authorable metadata property — published vocabulary an app author writes, which ADR-0049 enforce-or-remove governs. This is a TypeScript service contract between plugins, authored by nobody. I am ruling this one and escalating that one.

    Selected because it is the only card in pm:queue I could verify as genuinely free — #7739, #7679 and #7649 all still carry pm:queue while having open PRs (#7791, #7812, #7790) from another seat.


    Generated by Claude Code

  5. self-assigned this
    on Aug 11, 2026
  6. claude commented on Aug 11, 2026

    @claude
    Contributor

    Step 1 landed as a draft PR: #7829 (branch claude/issue-7616-permission-set-contract, session session_011rK3z836HF4CjfpbJuEjaz).

    Step 2 — the call sites — is deliberately not in it.

    What shipped

    file change
    packages/spec/src/contracts/security-service.ts new resolvePermissionSetsForContext?(context): Promise<PermissionSet[]> — the full sets, objects / fields / systemPermissions / tabPermissions, in resolution order
    packages/spec/src/contracts/security-service.test.ts pins the optionality (unguarded call must not compile) and the four columns the names surface cannot reach
    packages/plugins/plugin-security/src/security-plugin.ts exposed on the registered service literal; the sys_permission_set loader now hydrates tab_permissions; resolveFallbackPermissionSets' stale doc-comment corrected
    packages/plugins/plugin-security/src/resolve-permission-sets-for-context.pin.test.ts new — declared = reachable, asserted through the service locator
    .changeset/security-service-resolve-permission-sets.md @objectstack/spec minor, @objectstack/plugin-security patch

    2 packages, 5 files — the card estimated 3 packages / ~5 files for the full two-step de-duplication; step 1 is the two packages that do not include the call sites.

    The method is OPTIONAL

    Following this file's own established precedent — getMetadataReadableFields? (ADR-0106 D7) and checkAuthoredRowWrite? (#5493) were both added optional for exactly the reason the card gives: the availability rule has consumers resolve the service as Partial<ISecurityService>, so the local resolution must survive as the fallback until a floor version can be assumed. Optional makes that a property of the type (the unguarded call does not compile) rather than a promise in prose.

    One implementation gap closed so the declaration is true rather than nominal

    The plugin's sys_permission_set loader hydrated objects, fields and systemPermissions but dropped tab_permissions — so every DB-authored set came back without the very column /me/apps filters its app list with. Measured directly (reverse verification, below): expected undefined to deeply equal { app_crm: 'visible' }.

    Publishing "the sets, whole" over a loader that drops a quarter of them is the declared-≠-delivered defect Prime Directive #10 exists to prevent, and it would have made step 2 a regression on the exact surface this card is about. Inert for enforcement today (tabPermissions appears nowhere in permission-evaluator.ts or the plugin's data-plane paths), no extra query — the row is already fetched in full, so it is one JSON parse.

    The pin — declared = reachable

    Every case resolves the service the way a cross-package consumer does — off ctx.registerService('security', …), typed Partial<ISecurityService> — never off the plugin instance. The class method stays private; the literal is the supported surface.

    Cases: (1) present on the registered literal; (2) callable through that handle and returning all four columns on both authoring paths (metadata-declared and the DB loader, which is where a column silently goes missing); (3) the same resolution the names surface reports — sets.map(s => s.name) deep-equals resolvePermissionSetNames; (4) the D5 baseline is additive, the #7608 shape; (5) an anonymous caller resolves nothing.

    Reverse-verified, both halves:

    removed result
    the literal's resolvePermissionSetsForContext entry 5 / 5 cases fail — expected 'undefined' to be 'function'
    the loader's tabPermissions hydration 1 case fails — expected undefined to deeply equal { app_crm: 'visible' }

    Both restored and re-confirmed green.

    The honest tension — stated, not papered over

    This ships a contract method with no caller, and it is a net addition, not a removal. Because the contract mandates feature-detection, the local copy in current-user-endpoints.ts stays as the fallback for a SecurityPlugin too old to carry the method — so the repo now has the copy plus the delegation path plus a branch between them. De-duplication that adds a third path is not de-duplication yet.

    Nothing here changes behaviour today. This is not a fix and does not close any of #7608 / #7555 / #6334. It is the thing that makes step 2 possible: step 2 is unreachable until a floor version carrying the method can be assumed, and this is what makes that floor exist.

    Why the call sites were held back, restating the card's own measurement rather than overriding it: /me/apps deliberately projects a narrower column set (name + systemPermissions + tabPermissions) than /auth/me/permissions. Delegating changes which columns load on both surfaces — a user-visible change wanting its own before/after measurement, not a refactor riding on a contract addition.

    Gates — measured

    gate result
    pnpm lint (repo-wide, required) clean
    pnpm --filter @objectstack/spec typecheck clean (tsc + scripts + test layer)
    pnpm --filter @objectstack/plugin-security typecheck clean
    pnpm --filter @objectstack/spec check:generated 13 / 13 up to date — nothing needed regenerating
    check:exported-any 2409 types + 1509 schemas, 0 resolve to any
    check:dual-source-exports 4842 names, 0 new dual-source
    check:test-typecheck (spec, exact ratchet) 265 errors / 57 files — unchanged
    check:type-check-coverage 63/77 packages + root, unchanged
    check:service-providers / check:init-service-contract / check:authz-resolver / check:role-word / check:adr-anchors / check:tenant-chokepoint / check:nul-bytes / check:empty-changeset all green
    pnpm --filter @objectstack/plugin-security test 48 files / 978 tests pass
    packages/spec contract test 14 pass

    No liveness / dead-code gate objected to the unused method. check:liveness governs authorable metadata-type properties against the registry, not contracts/ interfaces; it is green. No ratchet ledger was raised — the one exact ratchet in the blast radius came out byte-identical, so there was nothing to report on that front.

    Two things that came out differently than the brief assumed — reporting, not fixing

    1. The contract test's "exhaustive" method list is not exhaustive, on main as well as here.

    The dispatch brief said the list "asserts an exhaustive method list, so it must be updated in the same change or the suite goes red." Measured on main @ 098b629, that is not true today, for two independent reasons:

    • hasWriteBypass, resolveWriteScope and describeDelegableScope are required members of ISecurityService and appear in neither the assertion list nor makeService()'s literal. That is why src/contracts/security-service.test.ts carries exactly 1 held error in test-typecheck-debt.json — TS2322 at line 21, "type … is not assignable to type ISecurityService". The compile-time half of that test is currently being asserted past a suppressed error.
    • An optional method changes neither the runtime assertion nor the error count, so nothing goes red either way.

    I left it alone deliberately. Fixing the stub takes that ledger entry 1 → 0, which under the exact ratchet means deleting the entry — a legitimate shrink, but it belongs to whoever owns those three methods, not to a contract addition, and I would rather report it than fold an unrelated ledger edit into this diff. Worth a card of its own; per the "do not touch any issue other than #7616" instruction I have not filed one — flagging it here for the maintainer to route.

    2. main is red on one spec test, unrelated to this diff.

    src/data/api-methods-batch-conformance.test.ts → sys_api_key: [get, list, update] grants single-record writes but not 'bulk' — /batch and the *Many routes will 405 (packages/platform-objects/src/identity/sys-api-key.object.ts). That file is byte-identical to main in this branch and is not touched by this PR. Also flagging rather than filing.


    Generated by Claude Code


    Generated by Claude Code

  7. 3 remaining items

  8. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Label correction: pm:dispatched → pm:on-hold (step 1 landed, step 2 deferred). domain:cli seat (hotlong).

    • Date: 2026-08-12.
    • What shipped: step 1 landed as PR feat(spec,plugin-security): publish the caller's resolved permission SETS on the security service (#7616) #7829 (MERGED 03:06Z) — resolvePermissionSetsForContext added to ISecurityService, exposed on the registered security literal, sys_permission_set loader tab_permissions gap closed. Unused by design; no behaviour change today.
    • Reason for hold: step 2 (switch current-user-endpoints.ts's two call sites to the service method and delete the local resolution) is deferred, per the deliverer's own ruling (os-zhuang, ACCEPT comment 5256849733): it is unreachable until a floor version carrying the method can be assumed, and it carries a user-visible /me/apps column-projection change that wants its own before/after measurement — not a refactor riding on the contract addition.
    • Restart condition: a released floor carrying resolvePermissionSetsForContext can be assumed by plugin-hono-server (i.e. after the method ships in a cut release consumers may depend on, dropping the Partial<ISecurityService> fallback). At that point step 2 becomes dispatchable — switch both call sites, delete the local copy, and measure the /me/apps column projection.

    pm:dispatched was stale the moment #7829 merged with nothing in flight; correcting it so the board reads true. Assignee cleared (on-hold, not in flight).


    Generated by Claude Code

  9. os-zhuang commented on Aug 17, 2026

    @os-zhuang
    Contributor

    pm:on-hold → pm:queue — maintainer instruction

    Executed by the domain:cli PM seat (#6024), session session_012WKSnqAaoqtW3QX7SSf1Vk. Provenance in full, since a label change is otherwise indistinguishable from a mis-sweep:

    Reasoning is the maintainer's standing branch rule (2026-08-16): the "not being developed soon ⇒ close it" default does not apply to type:Bug holds — a held Bug is a hidden Bug, because the lane's Bug priority only operates on queued cards.

    This card was the sharper of the two. It is not a speculative hold: step 1 already shipped (PR #7829, merged 2026-08-12 — resolvePermissionSetsForContext added to ISecurityService, exposed on the registered security literal, the sys_permission_set loader's tab_permissions gap closed), deliberately unused, with no behaviour change. Step 2 — retiring the duplicated resolution in packages/plugins/plugin-hono-server/src/current-user-endpoints.ts — was deferred with nothing at all that could fire it. So the half that makes step 1 worth anything sat behind no trigger, while the rule it exists to de-duplicate has already drifted three times on record (#7608, #7555/PR #7605, #6334).

    Unassigned and dispatchable. The Restart-when: gap #9388 recorded is resolved by the card leaving pm:on-hold outright rather than by inventing a trigger for it.


    Generated by Claude Code

  10. os-zhuang commented on Aug 17, 2026

    @os-zhuang
    Contributor

    ⛔ DO NOT DISPATCH while this reads pm:queue — the restart condition is measurably NOT met. domain:cli PM seat (#6024), session session_012WKSnqAaoqtW3QX7SSf1Vk. Posting immediately after the release above, because the premise check I ran before dispatching contradicts the label.

    The hold's condition (2026-08-12) is: a released floor carrying resolvePermissionSetsForContext can be assumed by plugin-hono-server — i.e. the method has shipped in a cut release consumers may depend on, so the Partial<ISecurityService> fallback can be dropped.

    Measured, not assumed:

    reading result
    latest cut tag @objectstack/spec@17.0.0-rc.6, cut 2026-08-10
    does that tag's packages/spec/src/contracts/security-service.ts carry the method? No
    when did the method merge? 2026-08-12 (PR #7829) — after the last release was cut
    do the call sites still hand-roll the resolution? Yes — current-user-endpoints.ts names the method only in a comment (:351), with no call and no Partial<ISecurityService> guard. Control: the file has 73 permission hits, so the search works and the zero is real

    So the contract method exists on main and in no released floor. Step 2 is exactly as unreachable today as it was when the hold was written; dispatching it now would mean either shipping a hard dependency on an unreleased method, or writing the very fallback branch the step exists to delete.

    Correction I owe this card too: my audit #9388 listed it as having "no machine-readable exit". Right about the mechanism, wrong about the substance — the condition above is real and checkable, it just lives in a comment instead of a body Restart-when: line, so the unlock scan cannot fire it. That is the actual defect here, and it is narrower than what I filed.

    Worth stating plainly, because it is the reason this card matters: the deferred half is real, scoped work, and the rule it de-duplicates has drifted three times already (#7608, #7555/PR #7605, #6334). It should come back the moment a release carries the method — which is precisely what a machine-readable Restart-when: would guarantee and a comment does not.


    Generated by Claude Code

  11. os-zhuang commented on Aug 17, 2026

    @os-zhuang
    Contributor

    Settled: pm:queue → pm:on-hold, with the restart condition now in the BODY where the scan can read it

    domain:cli PM seat (#6024), session session_012WKSnqAaoqtW3QX7SSf1Vk. Provenance: maintainer, live PM chat 2026-08-17, choosing 「改回 hold,但把条件写进正文」 after this seat measured that step 2's condition is genuinely unmet — the newest cut release predates the contract method by two days.

    Final state: bug + pm:on-hold + domain:cli, unassigned, and the body's first line is now a predicate anyone can run:

    Restart-when: `git grep -c "resolvePermissionSetsForContext" "$(git tag --sort=-creatordate | grep '^@objectstack/spec@' | head -1)" -- packages/spec/src/contracts/security-service.ts` returns non-zero
    

    Read back after writing — the backticks, the nested quotes and the $( ) all survived.

    That predicate is deliberately written against the newest tag at the time it runs, not against a pinned version, so it answers the hold's actual question — has a floor shipped that plugin-hono-server may assume? — on every future sweep without anyone editing it again.

    Nothing was re-decided: grade, substance and condition are exactly what the 2026-08-12 hold set. Only the channel changed, from a comment the unlock scan cannot see to a body line it can. Given this card's deferred half is real scoped work whose rule has already drifted three times (#7608, #7555/PR #7605, #6334), being woken automatically rather than by chance is the whole point.

    The ⛔ do-not-dispatch note above is superseded by this label state.


    Generated by Claude Code

  12. os-zhuang commented on Aug 19, 2026

    @os-zhuang
    Contributor

    On-hold daily batch (triage seat, round of ~14:47Z fire): Restart-when fired — the released spec (@objectstack/spec@17.0.0) carries resolvePermissionSetsForContext in contracts/security-service.ts (grep verified on origin/main). pm:on-hold → pm:queue.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions