Skip to content

fix(app): release workspace catalogs when no tab or route holds them - #46789

Closed
Hona wants to merge 8 commits into
anomalyco:v2from
Hona:location-catalog-evict
Closed

Hona wants to merge 8 commits into
anomalyco:v2from
Hona:location-catalog-evict

Conversation

@Hona

@Hona Hona commented Sep 2, 2026 •

Copy link
Copy Markdown
Member
  • Release loaded location catalogs when the last tab or mounted catalog reader releases its hold. Keep lightweight metadata, running shells, and the default location.
  • Snapshot directory and workspaceID at retain time. A live Solid session-location proxy can change in place during movement; cleanup must invalidate the original location, not its destination.
  • Capture the location generation at public read invocation, before a callback can queue behind an earlier read. Check it before requesting and before publishing, including the aggregate location.sync() continuation after syncInfo().
  • Pending sync entries carry the same predicate: credential/catalog events cannot revive obsolete work after release, and discarded reads cannot mark a cache entry complete. A genuine explicit load still works without a hold, including existing TUI callers. Same-task owner handoff keeps the current generation.
  • No request-abort framework, session transcript eviction changes, or Core/server implementation changes.
const current = locationCurrent(location)
return sync.run(key, async () => {
  if (!current()) return
  const response = await load(locationQuery(location))
  if (!current()) return
  publish(response)
}, current)

Movement Identity
Actual createData + createLocationResidency, tested with both session.moved and remembered metadata reconciliation, confirms that the retained proxy changes from non-default A to non-default B. Before d19561c6bc, cleanup cleared A but invalidated B's cache markers, leaving A unable to reload.

Model/provider reads after A moves to B Reviewed 519ec6772b Corrected d19561c6bc
Reuse B 2 unnecessary requests 0 requests; catalogs remain correct
Revisit A 0 requests; both catalogs stay empty 2 requests; both catalogs restored

The real-client regression covers all nine released catalog categories. The production correction is one two-field identity snapshot; generation guards and direct unheld callers are unchanged.

Current Controls
Unchanged production catalog workload below, 30 workspaces, three forced-GC samples per case. Reviewed source 519ec6772b uses its previously verified repair-after-dist bundle; corrected source d19561c6bc has a new build. Six production source-map entries and 770 asset hashes per bundle were verified/recorded separately. These controls preserve the earlier retention behavior, not establish a new performance gain.

Control Reviewed median MiB (samples) Corrected median MiB (samples)
Loaded, then closed 9.73 (8.49, 9.73, 9.80) 9.70 (8.52, 9.70, 9.70)
Closed before first response; sampled after bodies settle 8.20 (8.25, 6.48, 8.20) 8.04 (6.74, 8.04, 8.22)

Every sample on both builds: zero late refresh requests, nine reopen reads, zero warm-switch reads, and nine reconnect reads. Heap is post-GC renderer-main-isolate memory minus initial Home, not total desktop RAM. No new latency claim.

Earlier Benchmark

  • Production Chromium 147, 1440x900, Playwright route transport, service workers blocked. Each directory has 1,200 models (488,790 response bytes), eight agents with system prompts (24,620 B), 24 commands (3,313 B), 12 skills (40,625 B), and a four-exchange session.
  • Restore one tab per directory, visit each, close all tabs to Home, emit credential.switched, reopen one workspace, then reconnect. The pending case gates all first catalog responses until tabs are closed and the event is handled, then releases them on Home and waits for body readers to finish.
  • These historical generation-repair measurements used frozen before afb3a14299 and after 7c8a32d2cb, with six changed production sources verified against source maps and 770 asset hashes recorded per build. Their frontend source is equivalent to public revisions cbbc0dd69c and 304dfd04e8/519ec6772b. They are not measurements of the later identity snapshot in d19561c6bc.

Three serial forced-GC samples per case. Heap is renderer-main-isolate retained memory minus the initial Home sample, in MiB. Loaded cases sample after close; pending cases sample after late bodies settle.

Case Directories Before MiB, median (samples) After MiB, median (samples) Late refresh requests before / after
Loaded, then closed 5 8.39 (8.62, 8.37, 8.39) 8.39 (8.70, 8.39, 8.39) 0 / 0
Loaded, then closed 15 8.97 (9.29, 8.97, 8.97) 8.98 (8.97, 8.98, 9.05) 0 / 0
Loaded, then closed 30 9.71 (8.46, 9.75, 9.71) 9.78 (9.70, 9.78, 9.78) 0 / 0
Closed before first response 5 9.74 (9.74, 9.74, 9.74) 7.03 (7.03, 7.02, 7.33) 10 / 0
Closed before first response 15 15.71 (15.71, 15.71, 15.79) 7.63 (7.63, 7.55, 7.97) 30 / 0
Closed before first response 30 24.48 (24.52, 24.32, 24.48) 8.07 (8.04, 8.27, 8.07) 60 / 0
  • Loaded-then-closed retention is preserved. For context, the original upstream 499e22bf52 baseline retained 13.0 / 24.0 / 40.5 MiB after closing 5 / 15 / 30 directories. Those historical measurements used the older base and are not the controlled repair comparison above.
  • Reopening fetches nine catalogs. The unrepaired pending case fetched seven because its late credential event had already refilled models/providers. Warm open-tab switches still make zero catalog requests; reconnect makes nine for the reopened workspace in both builds.

Separate natural-GC timing: five loaded directories, 20 samples per build, counterbalanced in 10-sample ABBA blocks. Playwright-observed reopen-to-model-ready:

Before After
Median / p95 227 / 261 ms 193 / 319 ms
Range 158-295 ms 147-372 ms

The distribution does not establish reduced reopen latency; on-demand catalog reads remain the trade-off.

Scope

  • Old-generation first responses and queued refreshes are discarded, not cancelled. Already-started HTTP reads and JSON parsing still finish. Explicit new loads remain supported; this is not a global memory cap.
  • Lightweight location entries and generation counters remain. Retained location-entry counts were not instrumented, and the residual heap was not attributed to individual caches.
  • Browser renderer only, not total desktop RAM, server memory, worker/GPU memory, allocation peaks, or a reproduction of the reported approximately 1 GB spike. No cross-directory payload sharing was added.

Hona added 7 commits September 3, 2026 09:43
Location catalogs (models, providers, agents, commands, skills, MCP,
references, integrations) accumulated for every workspace a renderer ever
visited, and credential events reloaded them for every known location.

Consumers now hold a location while they need it: the current
LocationProvider, provider and integration readers, and every open tab.
Releasing the last hold drops the catalogs and their sync state after the
current task, so route swaps and draft promotion do not reload them.
Light metadata (info, vcs, running shells) and the default location stay
resident. Event-driven refreshes only reload catalogs that are loaded or
loading.

Adds a production benchmark that visits and closes 5/15/30 workspaces with
1,200-model catalogs and records retained heap and catalog requests.
Credential events now refresh only loaded catalogs, matching the TUI, which syncs the current location before the account manager opens.
@Hona
Hona force-pushed the location-catalog-evict branch from b704940 to 519ec67 Compare September 2, 2026 23:47
@Hona
Hona marked this pull request as ready for review September 3, 2026 00:31
@Hona
Hona requested a review from Brendonovich as a code owner September 3, 2026 00:31
Copilot AI lite review requested due to automatic review settings September 3, 2026 00:31

Copilot AI 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.

🔵 Needs a closer look

It changes core client-side caching/residency and invalidation behavior across app + client, so it warrants final human validation despite strong test coverage.

Pull request overview

This PR introduces explicit “location residency” for workspace catalogs: catalogs stay loaded while a tab/route holds a location, and are released (dropping heavy catalogs but keeping lightweight metadata) once the last holder releases—while also guarding against Solid location proxies mutating during session moves.

Changes:

  • Add data.location.retain() with hold counting + generation-based guards so obsolete queued reads and late responses don’t repopulate released caches.
  • Wire residency holds into the app (route + open tabs) so switching among open tabs avoids refetches, and closing the last tab releases catalogs.
  • Add targeted client/app tests plus an e2e performance benchmark to validate correctness and request/memory behavior across close/reopen, late responses, and session movement.
File summaries
File Description
packages/tui/test/cli/tui/dialog-integration.test.tsx Updates TUI integration dialog integration test to pre-sync required catalogs and align expectations with “refresh only loaded catalogs” behavior.
packages/client/test/solid-location.test.ts Adds focused unit tests covering retain/release, generation guards, late responses, and event-driven refresh semantics.
packages/client/src/solid/data.ts Implements hold-based catalog residency (retain) and generation-guarded sync/invalidation to prevent stale work from repopulating released locations.
packages/app/test-browser/location-residency.test.ts Adds browser-side tests validating tab-driven residency and correct behavior across session moves.
packages/app/src/workspaces/location.tsx Ensures mounted workspace route holds its location catalogs via data.location.retain.
packages/app/src/runtime/server/runtime.tsx Installs tab-driven residency in the runtime by wiring tabs into createLocationResidency.
packages/app/src/runtime/server/residency.ts New helper that retains/release locations based on currently open tabs for a given server connection.
packages/app/src/providers/catalog/providers.ts Retains location while provider catalog UI is mounted so its catalogs don’t get released mid-use.
packages/app/src/providers/catalog/integrations.ts Retains location while integration catalog UI is mounted for the same residency semantics.
packages/app/e2e/utils/mock-server.ts Extends mock server to serve per-directory locations and configurable catalogs (agents/commands/skills) for new benchmarks.
packages/app/e2e/performance/timeline/location-catalog.fixture.ts New fixture building a large, realistic per-directory catalog payload used by the benchmark.
packages/app/e2e/performance/timeline/location-catalog-benchmark.spec.ts New Playwright benchmark exercising restore/visit/close/credential refresh/reopen/reconnect to validate request patterns and retained heap.
packages/app/e2e/performance/README.md Documents the new location catalog benchmark scenarios and environment knobs.
packages/app/e2e/performance/playwright.catalogs.config.ts Adds a dedicated Playwright config for running the location-catalog benchmark against a prebuilt dist.
Review details
  • Files reviewed: 14/14 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +1 to +13
import config from "./playwright.config"

const port = Number(process.env.PLAYWRIGHT_PORT ?? 4795)
export default {
...config,
testMatch: "timeline/location-catalog-benchmark.spec.ts",
webServer: {
command: `bun x vite preview --outDir "${process.env.CATALOGS_DIST}" --host 127.0.0.1 --port ${port} --strictPort`,
url: `http://127.0.0.1:${port}`,
reuseExistingServer: false,
timeout: 120_000,
},
}
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Automated PR Cleanup

Thank you for contributing to opencode.

Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions.

This PR was closed because it matched the following cleanup criteria:

  • The PR was created more than 1 month ago
  • The PR had fewer than 2 positive reactions
  • Positive reactions are counted as thumbs-up, heart, celebration, or rocket reactions on the PR

PRs created within the last month are not affected by this cleanup.

If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate.

Thanks again for taking the time to contribute.

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.

2 participants