fix(core): release location holders on shutdown by rekram1-node · Pull Request #59399 · anomalyco/opencode

rekram1-node · 2026-10-26T20:41:46Z

Part of #51003 and #52965. Related: #50363, #51249, #37844, #58795

Problem

invalidate (idle eviction, opencode reload, vcs.init, debug evict) drops a location from the cache, but the RcMap only closes it once nobody holds it. An open PTY websocket or a client-hosted MCP socket holds it for as long as it stays open, so the whole old location keeps running: MCP servers, watchers, shells. The next request boots a fresh location with its own set, so you end up with duplicate MCP server sets.

Running steps already handle this by checking lifecycle.isClosed() at step boundaries. Sockets never did.

Change

 invalidate(location)
   drop from cache
   lifecycle.shutdown
+    -> awaitShutdown resolves
+       PTY / client MCP socket closes with 1012, client MCP reconnects to the new location
+       last holder gone -> location scope closes -> MCP servers, watchers, shells stop
+  still held after 30s: warn "location still held after shutdown"

 idle sweep
-  evict if no running session or job
+  evict if no running session or job, and no open PTY or client MCP socket

LocationLifecycle gets awaitShutdown plus retain/isRetained. One server helper, holdLocation, does both for the PTY and client MCP sockets.
The idle sweep skips retained locations. For a PTY, eviction would kill the shell. For client MCP, connectClientMcp reconnects within 500ms, so evicting would just boot a fresh location (and its MCP servers) right away.
The 30s warning timer is forked into the held location's scope, so it stops as soon as the last holder lets go instead of hanging around.
Explicit invalidations (reload, vcs.init, debug evict) still end location PTYs on purpose. The PTY service belongs to the location, so keeping the shell means keeping the stale location, which is the leak this fixes. The TUI and app terminals use the persistent PTY daemon (/api/experimental/persistent-pty), which isn't tied to a location, so they're unaffected. Only direct /api/pty users see the 1012 close, and reconnecting to the old PTY ID gets a 404.

Verification

packages/core: bun run typecheck, bun run typecheck:tsc, bun run script/test.ts on the location, mcp, pty and browser-idle tests: 200 pass.
packages/server: bun run typecheck, bun run typecheck:tsc, bun test: 117 pass. packages/client bun test test/mcp.test.ts: 4 pass. Root bun run check passes.
New tests, each fails on dev, and passes 5/5 with --rerun-each 5:
- core/test/location-retain.test.ts: the idle sweep evicts an idle sibling but keeps a retained location, then evicts it once released.
- core/test/location-layer.test.ts: warns when a borrower still holds an invalidated location after 30s, stays quiet when it lets go on shutdown.
- server/test/pty-transport.test.ts: a held PTY socket gets 1012 on reload and the old shell process exits.
- server/test/client-mcp.test.ts: a real connectClientMcp client gets 1012 on reload, the old location closes, and the client reattaches to the new one.
Manual run on a local serve (isolated XDG dirs) with one stdio MCP server, a PTY socket and a client MCP socket on the same directory:
- dev, after reload: 2 MCP processes, old shell still alive, sockets still on the old location
- this branch, after reload: both sockets closed with 1012, client MCP reattached, 1 MCP process, old shell gone
- this branch, after debug evict: client MCP closed with 1012 and reattached, 1 MCP process, no "still held" warnings

Visual

<svg xmlns="http://www.w3.org/2000/svg" width="240" height="72" viewBox="0 0 240 72" fill="none" stroke="currentColor" font-family="sans-serif" font-size="9">
  <style>
    .old{animation:old 4s ease-in-out infinite}
    .new{animation:new 4s ease-in-out infinite}
    .sock{animation:sock 4s ease-in-out infinite}
    @keyframes old{0%,40%{opacity:1}60%,100%{opacity:.12}}
    @keyframes new{0%,50%{opacity:.12}70%,100%{opacity:1}}
    @keyframes sock{0%,25%{opacity:.8}35%,100%{opacity:0}}
    @media (prefers-reduced-motion:reduce){.old,.new,.sock{animation:none}.old{opacity:.12}.sock{opacity:0}}
  </style>
  <path class="sock" d="M60 4v10" stroke-width="2" stroke-dasharray="3 2"/>
  <g class="old">
    <rect x="12" y="14" width="96" height="44" rx="8" stroke-opacity=".6"/>
    <text x="22" y="30" fill="currentColor" stroke="none">old location</text>
    <circle cx="30" cy="44" r="5" fill="currentColor" stroke="none"/>
    <circle cx="48" cy="44" r="5" fill="currentColor" stroke="none"/>
    <circle cx="66" cy="44" r="5" fill="currentColor" stroke="none"/>
  </g>
  <path d="M114 36h12" stroke-opacity=".4" stroke-width="2"/>
  <g class="new">
    <rect x="132" y="14" width="96" height="44" rx="8" stroke-opacity=".6"/>
    <text x="142" y="30" fill="currentColor" stroke="none">new location</text>
    <circle cx="150" cy="44" r="5" fill="currentColor" stroke="none"/>
    <circle cx="168" cy="44" r="5" fill="currentColor" stroke="none"/>
    <circle cx="186" cy="44" r="5" fill="currentColor" stroke="none"/>
  </g>
</svg>

Added by the opencode bot (AI-generated).

Related: #51003, #52965, #50363, #51249, #37844

opencode-agent

The change looks sound. I copied the new location-layer test onto the base and it fails there: the held location's MCP process outlives invalidate. With this PR it passes, along with the rest of location-layer.test.ts. The MCP test files pass apart from one descendant-termination test that fails the same way on the base. tsc for packages/core passes.

Public API: unchanged. close is added only to the internal Mcp.Service interface.

I found nothing blocking. A late startServer after close forks from the already-closed servers scope, so its cleanup runs straight away and nothing is left behind. Adding Mcp.node as a lifecycle dependency doesn't change when MCP starts, since instance.ts already builds it for every location.

Related: #51003, #52965, #50363, #51249, #37844

opencode-agent

I couldn't verify the fix end to end. The rewrite drops the earlier Mcp.close approach and its test, and I didn't run a manual serve with a held terminal socket. The core location and MCP-attach tests and the server tests pass, and tsc -b passes for packages/core and packages/server.

Public API: no endpoints, schemas or events change. One behaviour change: PTY and client MCP websockets now close with 1012 ("location shut down") when their location shuts down.

Concerns:

Terminals end on every invalidation, not only reload. Details are inline.
No test now covers the fixed behaviour. A small one would help: a held PTY or MCP socket is closed on invalidate, and the idle sweep skips a retained location.

opencode-agent · 2026-10-26T21:22:12Z

            const socket = yield* Effect.orDie(ctx.request.upgrade)
            // TODO: Integrate graceful-shutdown socket tracking before clients migrate to this route.
            const outbox = yield* Queue.unbounded<Outbound>()
+            yield* closeOnShutdown(outbox)


The PTY service belongs to the location, so closing this socket on shutdown also ends the shell. The client can't reconnect to it either: the new location has no PTY with this ID, so it gets a 404. Besides opencode reload and the debug evict route, vcs.init (packages/server/src/handlers/vcs.ts) also invalidates the location. So "initialize git" in the app would now end every open terminal in that directory, which looks like a regression. Should those call sites skip or defer retained locations, the way the idle sweep now does?

I looked at this again and kept it. The PTY service is location-owned, so keeping the shell on vcs.init or reload means keeping the stale location and its MCP set alive, which is the leak. Deferring vcs.init would leave the old project identity in place. In practice the TUI and app terminals go through the persistent PTY daemon, which is not tied to a location, so they never see this. Only direct /api/pty users get the 1012, and the idle sweep still skips a location with an open PTY or client MCP socket. Client MCP sockets reconnect to the new location on their own (now covered in client-mcp.test.ts).

opencode-agent

The new reload test fails on the base and passes with this PR, 5/5 with --rerun-each 5. The core location tests pass, and tsc passes for packages/core and packages/server. Public API: no change from this round.

The earlier point about terminals ending on vcs.init and other explicit invalidations is now documented as intentional in the description, but the code is unchanged, so that thread stays open for a maintainer to decide. Nothing new to raise.

github-actions Bot added the contributor label Oct 26, 2026

opencode-agent Bot reviewed Oct 26, 2026

View reviewed changes

rekram1-node force-pushed the stop-evicted-mcp branch from bccd255 to 64263be Compare October 26, 2026 21:14

rekram1-node changed the title ~~fix(core): stop MCP servers when a location is evicted~~ fix(core): release location holders on shutdown Oct 26, 2026

opencode-agent Bot reviewed Oct 26, 2026

View reviewed changes

rekram1-node force-pushed the stop-evicted-mcp branch from 64263be to b557542 Compare October 26, 2026 21:30

opencode-agent Bot reviewed Oct 26, 2026

View reviewed changes

opencodejr Bot mentioned this pull request Oct 27, 2026

[FEATURE]: Release a project's cached services when its last client exits #53068

Open

1 task done

iyernaveenr mentioned this pull request Oct 29, 2026

fix(core): keep running terminals out of eviction #60073

Open

3 of 6 tasks

rekram1-node force-pushed the stop-evicted-mcp branch from b557542 to fe8ed14 Compare October 31, 2026 01:55

fix(core): release location holders on shutdown

62e85f2

rekram1-node force-pushed the stop-evicted-mcp branch from fe8ed14 to 62e85f2 Compare October 31, 2026 04:18

rekram1-node merged commit 1908fcb into dev Oct 31, 2026
13 checks passed

rekram1-node deleted the stop-evicted-mcp branch October 31, 2026 04:28

AbizardShafwan pushed a commit to AbizardShafwan/opencode that referenced this pull request Nov 9, 2026

fix(core): release location holders on shutdown (anomalyco#59399)

6a09859

neriousy mentioned this pull request Nov 13, 2026

core: long-lived V2 server enters persistent allocation loop #36677

Closed

Navigation Menu

Repository navigation

fix(core): release location holders on shutdown - #59399

fix(core): release location holders on shutdown#59399
rekram1-node merged 1 commit into
devfrom
stop-evicted-mcp

rekram1-node commented Oct 26, 2026 •

edited

Loading

Uh oh!

opencode-agent Bot left a comment

Uh oh!

opencode-agent Bot left a comment

Uh oh!

opencode-agent Bot Oct 26, 2026

Uh oh!

rekram1-node Oct 31, 2026 •

edited

Loading

Uh oh!

opencode-agent Bot left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

rekram1-node commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Change

Verification

Visual

Uh oh!

opencode-agent Bot left a comment

Choose a reason for hiding this comment

Uh oh!

opencode-agent Bot left a comment

Choose a reason for hiding this comment

Uh oh!

opencode-agent Bot Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

rekram1-node Oct 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

opencode-agent Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

rekram1-node commented Oct 26, 2026 •

edited

Loading

rekram1-node Oct 31, 2026 •

edited

Loading