refactor(core): split permission policy from global ledger - #46706
Open
kitlangton wants to merge 1 commit into
Open
refactor(core): split permission policy from global ledger#46706kitlangton wants to merge 1 commit into
kitlangton wants to merge 1 commit into
Conversation
6 tasks
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.
Why
Permissionfused two kinds of state in one Location node: policy inputs that legitimately belong to the instance (Location,Agent,PluginHooks,PermissionSaved) andconst pending = new Map<ID, Pending>(), transient runtime state keyed by request and Session that clients need host-wide. Because the map lived in the instance,GET /api/session/:sessionID/permission(andget/reply) ransessionLocationMiddleware, which calledlocations.get(...)and booted the whole instance (config, plugins, MCP) just to read a map that is empty by construction for an unloaded instance. The TUI does this for every descendant subagent Session, which reactivated ~70 idle locations at boot.The rule adopted here: config-derived state stays in the instance; transient state keyed by a Session, or that clients read host-wide, goes in a global node so reads never boot an instance.
What Changes
flowchart LR subgraph before["Before: one Location node"] P0["Permission<br/>evaluate + pending Map"] end subgraph after["After"] subgraph inst["Location node (per instance)"] F["Permission (facade)<br/>ask / assert"] PP["PermissionPolicy<br/>saved rules · agent policy · hooks"] end subgraph glob["Global node (process-wide)"] L["PermissionLedger<br/>pending Map · register · reply · get · forSession · list"] end F -->|evaluate| PP F -->|register when ask| L HTTP["server.permission handlers"] -->|list / get / reply| L HTTP -->|create| F endGET /api/session/:id/permission,GET …/:requestID,POST …/replysessionLocationMiddlewareSessionexistence check (DB read)POST /api/session/:id/permission(manual ask)GET /api/permission/request{ location, data }, booted the location{ data }host-wide;?location[directory]=…filters to requests asked from that location, no bootpermission.assert) and plugin host (list/get/reply)Permission.ServicePermissionkeeps its public name andInterfaceso the ~15 tool call sites,plugin/host.ts, andplugin/internal.tsare untouched.PermissionSavedstays where it is (project-keyed, already global).Ledger Ownership
The instance-layer finalizer that failed every pending
Deferredon instance close is gone; a global service has no instance lifetime to hook. Instead the awaiting fiber owns its entry:ledger.registerreturns{ request, await, cancel }. The facade attachescancelwithEffect.ensuringinside the same uninterruptible region that registered. This matters:Effect.interruptiblechecks for a pending interrupt the momentrestoreopens, so a guard attached after that point would never run when the interrupt landed during registration.permission.repliedwithreply: "reject"so every client drops the prompt. Previously the entry vanished silently and clients kept a stale prompt until a refetch.Permission.askreturning"ask", used bysession.permission.create) park a waiter fiber in the instance scope, so closing the instance still evicts them and notifies clients.locationexplicitly (bus.publish(..., { location })) since the ledger has no ambientLocation.Service. No Bus routing changes.reply: "always"needs the asker's project forsaved.addand the asker's policy to auto-approve other pending requests; both are captured at registration (projectID,reevaluate), so the ledger never resolves an instance.Scope
Permission only. Form keeps its instance-scoped map and
formLocationMiddleware; a sibling change handles that. The app'spermission.request.listsweep still lists per location and works unchanged aside from the unwrapped response.Verification
New coverage in
packages/core/test/permission.test.ts:shares pending requests through the host-wide ledger: a request registered through the facade is listable and answerable viaPermissionLedger.Service, including the location filter.cancels an interrupted asker and tells clients: interrupting the asker removes the entry and emitspermission.replied(reject) stamped with the instance location.cancels detached requests when their instance closes: a second facade built on the same globals is closed and its detached ask is evicted with arejectevent.lets plugin hooks override the evaluated effect: hook chain still runs inPermissionPolicy.packages/server/test/session-instances.test.tsnow seeds requests for a third Session that has no instance configuration (booting it would throw), then lists and replies to them over HTTP and assertsbootsis unchanged.