Skip to content

fix(server): avoid booting unloaded locations on pending form and permission reads - #57019

Open
kitlangton wants to merge 1 commit into
devfrom
pending-read-no-boot
Open

kitlangton wants to merge 1 commit into
devfrom
pending-read-no-boot

Conversation

@kitlangton

@kitlangton kitlangton commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What it does

Opening a project or session in the app used to boot that session's Location just to list pending permissions and forms. A booted Location costs about 35 MB, so browsing the sidebar left them piling up: one server had 133 Locations loaded on Oct 19, and Studio's server had 33 on Oct 23. Now these two list reads answer from a Location that's already loaded, or return an empty list without booting one.

Dev already unloads idle Locations after 60 minutes (location-activity.ts). This PR stops browsing from loading them again and doesn't add any eviction.

How it works

  • Instance.Service gets provideIfLoaded(session). It runs the effect with the session's instance only if that instance is already loaded (or still booting, in which case it waits), and returns None otherwise.
  • GET /api/session/:sessionID/permission and GET /api/session/:sessionID/form use it and return { data: [] } when nothing is loaded. The global form list does the same through Instance.provideContextEffectOption. Pending permissions and forms only exist in a loaded Location's memory, so an unloaded one has nothing to report.
  • A missing session still returns SessionNotFoundError. These routes can no longer produce LocationNotFoundError, so the OpenAPI 404 type narrows to just SessionNotFoundError.
  • App: pending request lookups in session/requests/model.ts and project-avatar-state.ts are scoped by session family, so a child session's pending request still shows on its parent and siblings.

provideIfLoaded was added in #46442 and then removed in #46639 as part of the plugin/config decoupling refactor, which moved these handlers back to instances.provide. I couldn't find a stated reason for removing it. This use is narrower: two read-only list routes whose answer only lives in a loaded Location's memory, so booting one to answer them is pure cost. @thdxr, was there a reason to drop provideIfLoaded in #46639 that should rule it out here?

Risk

  • Server, SDK and app. Writes and every other route still boot Locations exactly as before.
  • If an unloaded Location somehow had pending requests, the list would show them only once something loads it. They're held in memory today, so that shouldn't happen.
  • Clients that matched LocationNotFoundError on these two routes would never see it now. The generated clients don't change.

Proof

No media; this is server behavior.

  • bun turbo typecheck for core, server, sdk, protocol and app: pass
  • core bun test test/session-remove.test.ts test/instance-vanilla.test.ts test/session-move.test.ts: 22 pass
  • server bun test test/fetch.test.ts test/session-instances.test.ts: 13 pass. Includes checks that the permission and form lists don't load an instance.
  • sdk bun test test/instances-lifecycle.test.ts test/instances.test.ts: 6 pass
  • app bun test src/session/requests/session-request-tree.test.ts: 14 pass
  • bun script/lint-changed.ts origin/dev: 0 problems
  • bun --cwd services/www script/generate-openapi.ts --check: pass

Added by the opencode bot (AI-generated).

Fixes #53918

Related: #51340

Public API: narrows the error types of GET /api/session/{id}/permission and /form; unloaded Locations now return an empty list (backwards compatible)

@kitlangton
kitlangton force-pushed the pending-read-no-boot branch from c07b9b2 to e67f713 Compare October 23, 2026 17:27
@kitlangton
kitlangton marked this pull request as ready for review October 23, 2026 17:35

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sound. I ran the PR's server test routes pending requests by Session without loading an instance against the base checkout. It fails there: listing pending permissions or forms loads the session's Location. With this PR it passes, and loaded Locations still return their real pending forms. The server, core, sdk and app tests closest to the change pass, and tsc -b passes for core, server, sdk, protocol and app. provideIfLoaded is built on RcMap.getOption, so it waits for a Location that is still starting and never starts a new one. That's the behaviour the description promises.

Public API (HTTP/OpenAPI):

  • GET /api/session/{sessionID}/permission: errors go from SessionNotFoundError | LocationNotFoundError to SessionNotFoundError.
  • GET /api/session/{sessionID}/form: the formLocationMiddleware is removed, so InvalidRequestError and LocationNotFoundError are no longer returned. SessionNotFoundError stays.
  • When the Location isn't loaded, both routes now return { data: [] } instead of loading it. Success shapes are unchanged.

This is backwards compatible. Clients that matched LocationNotFoundError on these routes just won't see it anymore. Internally, Instance.Interface gains a required provideIfLoaded, so any custom Instance.Service implementation needs it. The PR updates all of them in this repo.

One optional note on the new core test, inline.

Fixes #53918
Related: #51340

})

describe("Instance.provideIfLoaded", () => {
it.live("skips absent instances and scopes loaded borrows without replacing the caller's Scope", () =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This packs several separate behaviours into one fairly dense case: absent → None without loading, loaded → Some, failure passthrough, the caller's Scope being kept, and invalidation during a borrow. It also lives in session-remove.test.ts, which isn't about provideIfLoaded. The server test in fetch.test.ts already covers the behaviour that matters (no Location is loaded, and loaded ones return real data). Optional: cut this down to the absent/loaded cases, or move it to an instance-focused test file, so it's easier to follow.

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

V2: pending form and permission reads bootstrap idle session locations

1 participant