Skip to content

fix(core): keep locations with running terminals out of eviction - #48730

Open
iyernaveenr wants to merge 1 commit into
anomalyco:v2from
iyernaveenr:pty-eviction
Open

fix(core): keep locations with running terminals out of eviction#48730
iyernaveenr wants to merge 1 commit into
anomalyco:v2from
iyernaveenr:pty-eviction

Conversation

@iyernaveenr

@iyernaveenr iyernaveenr commented Sep 12, 2026

Copy link
Copy Markdown

Issue for this PR

Closes #48691

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

LocationActivity evicts a location 60 minutes after its last durable session event. Terminals produce no session events, so a location whose only activity is a running terminal is invalidated on schedule and the Pty finalizer kills every terminal in it: blank pane, scrollback gone, and the desktop app recreates empty terminals.

Before evicting an expired location, the sweep now asks that location's Pty service for running sessions and renews the deadline while any exist. The probe uses contextEffectOption, so it only reads the cached graph and never boots a location. A graph that failed to build counts as having no terminals, so failed locations are still cleaned up as before, and running executions are only interrupted when the location is actually going to be evicted.

How did you verify your code works?

  • New test in packages/core/test/location-activity.test.ts: a location built from the real Pty node with a running cat terminal stays cached when its deadline passes (terminal still running); once the terminal is removed, the next deadline evicts it. The test fails without the fix.
  • Existing LocationActivity eviction tests pass unchanged; packages/core typecheck, the pty and location suites, oxlint and prettier are clean.
  • On my dev-based daily build the same eviction (there driven by the LayerMap idle TTL) killed the terminals every hour on the dot, which is how I found this.

Screenshots / recordings

Not a UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@iyernaveenr
fix(core): keep locations with running terminals out of eviction
1d259c0 
LocationActivity evicts a location 60 minutes after its last durable
session event. Terminals produce no session events, so a location whose
only activity is a running terminal is invalidated on schedule and the
Pty finalizer kills every terminal in it, disconnecting attached clients
and discarding their scrollback.

Before evicting an expired location, ask its cached Pty service for
running sessions and renew the deadline while any exist. The probe uses
contextEffectOption, so it never boots a location that is not cached.
@github-actions

github-actions Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions github-actions Bot mentioned this pull request Sep 13, 2026
Closed
@thiago-carneiro thiago-carneiro mentioned this pull request Oct 1, 2026
Open
@SamSpiri

SamSpiri commented Oct 3, 2026

Copy link
Copy Markdown

Independent data point supporting this PR's direction.

On v2.0.22 a background: true shell job outlived the 60-min idle TTL of its location. Eviction closed the location scope, which interrupted the background completion watcher (ShellTool.notifyWhenDone, forked into that scope at tool/plugin/shell.ts:185), so sessions.synthetic(...) never ran: no completion notification was delivered, the session stayed idle, and the shell .out was removed by the watcher's interrupt finalizer. A second background job in the same session that finished under 60 min delivered normally.

So a retention/exemption rule for "locations with a backgrounded or running shell job" is what preserves the completion notification, not just the process. Requesting the same treatment for backgrounded shells as this PR gives terminals.

@SamSpiri SamSpiri mentioned this pull request Oct 3, 2026
Open
@mpj mpj mentioned this pull request Oct 6, 2026
Open

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

Reviewers

No reviews

Assignees

No one assigned

Labels

needs:issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@iyernaveenr @SamSpiri