demo: changed-specs gate skip path (do not merge) - #2
Open
MumuTW wants to merge 2 commits into
Open
Conversation
`e2e.yml` is reachable only from `schedule` and `release-cut`'s `workflow_call`, so a PR that adds an E2E regression test never runs it. The test runs for the first time on `main`, where nothing gates on it — twice now a spec has landed and never passed. Adds a `pull_request` job that runs exactly the specs the PR touched. A PR that changes no spec pays nothing: the file list is resolved first, and every setup/build/run step is gated on the result being non-empty. - No `paths:` filter. The check reports on every PR, so it can be made required without wedging the merge box on PRs that touch no spec. - Changed files come from the pulls API (paginated), not a git diff — the PR checkout is shallow and a deleted spec must not reach Playwright. - Selected paths are argv for a shell-invoked command and a fork PR names its own files, so `select-changed-e2e-specs.mjs` allowlists `tests/e2e/**/*.spec.ts` with no shell metacharacters and no traversal, and the run step expands them from an array rather than the command string. - `--pass-with-no-tests` keeps a spec whose tests are all `@headful` from failing the gate, since the `electron-headless` project filters them out. Gating the full sharded suite on every PR stays a separate decision: it is 22 minutes of wall clock, and `main` is red today, so making it required would block every merge until stablyai#10519 closes.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00e379b195
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+3
to
+7
| # Why: e2e.yml is reachable only from schedule and release-cut's workflow_call, | ||
| # so a PR that adds an E2E regression test never runs that test — it runs for | ||
| # the first time on main, where nobody is gating on it (#10518). This gate runs | ||
| # exactly the specs the PR touched: it closes the "test lands red" hole without | ||
| # putting the full 22-minute sharded suite on every pull request. |
There was a problem hiding this comment.
This five-line header is one of several newly added comments that narrate the implementation at length; the selector also has a 14-line module comment and the test repeats workflow mechanics. Condense these to brief, non-obvious rationale so the change follows the repository's explicit concise-comment convention and avoids maintaining duplicated prose alongside the implementation.
AGENTS.md reference: AGENTS.md:L5-L8
Useful? React with 👍 / 👎.
Comment on lines
+45
to
+47
| gh api --paginate \ | ||
| "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" \ | ||
| -q '.[] | select(.status != "removed") | .filename' > changed-files.txt |
There was a problem hiding this comment.
For PRs with more than 3,000 changed files, --paginate still cannot retrieve files beyond the endpoint's documented maximum, so a changed E2E spec omitted from those results makes changed-specs.txt empty and this required gate succeeds without running it. Compare the retrieved count with github.event.pull_request.changed_files and fail or use a safe fallback when the API result is incomplete; the limit is documented by GitHub's List pull request files endpoint.
Useful? React with 👍 / 👎.
Comment on lines
+111
to
+112
| xvfb-run --auto-servernum env SKIP_BUILD=1 ORCA_E2E_FORWARD_APP_LOGS=1 \ | ||
| pnpm run test:e2e --pass-with-no-tests "${SPECS[@]}" |
There was a problem hiding this comment.
When a changed spec contains only @headful tests, this command invokes test:e2e, which hardcodes the electron-headless project; tests/playwright.config.ts excludes every @headful test from that project, and --pass-with-no-tests then turns the omission into a successful gate. This affects existing files such as dead-terminal-stress.spec.ts and floating-workspace-shared-glyph-atlas.spec.ts, so changes to those regressions are never exercised; run the selected paths through the headful project as well rather than treating this case as success.
Useful? React with 👍 / 👎.
Comment on lines
+10
to
+14
| types: | ||
| - opened | ||
| - synchronize | ||
| - reopened | ||
| - ready_for_review |
There was a problem hiding this comment.
When a PR is retargeted to another base branch without changing its head commit, GitHub emits the edited pull-request activity, but this filtered trigger excludes it. Because retargeting changes the API's changed-file set, the existing check can remain associated with the unchanged head after having selected against the old base, so a spec newly included by the new comparison is not run; include edited or otherwise trigger specifically on base-ref changes.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| // Why: importing this module from the unit test must not consume stdin. | ||
| if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) { |
There was a problem hiding this comment.
When this CLI is executed on Windows, process.argv[1] is a native path such as C:\repo\..., while import.meta.url is a normalized file:///C:/repo/... URL, so this comparison is false and the selector exits successfully without reading stdin or writing specs; the new subprocess test consequently fails on Windows as well. The same mismatch occurs in POSIX checkout paths containing spaces or other URL-escaped characters. Use pathToFileURL(process.argv[1]).href, as the other config scripts do, instead of constructing a file URL manually.
AGENTS.md reference: AGENTS.md:L27-L32
Useful? React with 👍 / 👎.
… selector - Add node-gyp to devDependencies so the changed-specs workflow uses the pnpm-lock version instead of npm install -g. - Remove .trim() on PR-controlled filenames before allowlist matching and add tests for leading/trailing whitespace rejection.
MumuTW
pushed a commit
that referenced
this pull request
Jul 28, 2026
…persist a watermark past them (stablyai#10816) * fix(mobile): keep the reconnect watermark alive across the app's own teardown The catch-up added in stablyai#8690 could never run. app/index.tsx unsubscribes the notification stream on every non-'connected' state and builds a fresh subscription on reconnect, so the closure holding the ready-counter, the delivered watermark and the seen-set is destroyed exactly when a reconnect needs them. Every reconnect looked like a cold open, `reconnectReadyCount` was always 1, and notifications dispatched while the socket was down were never fetched. Move that state to a per-host module-scope session so it survives the teardown. Refs stablyai#8591 Co-authored-by: Orca <help@stably.ai> * fix(mobile): tag the notification watermark with a counter epoch so a desktop restart can't kill catch-up The desktop's notification `seq` is a per-process in-memory counter that starts at 0 on every launch. The mobile client's watermark is persisted in AsyncStorage and monotonic. After a desktop restart the two index different counters, so a client holding seq 57 meets a fresh counter at 2, `57 >= 2` cuts everything, and reconnect catch-up dies silently until the new process out-dispatches the old watermark — 57 notifications later. Users see nothing and get no error (stablyai#8591). Stamp every dispatched notification with an epoch identifying the counter lifetime, ride it on the `ready` frame and the getMissedSince response, and persist it beside the watermark. A watermark whose epoch doesn't match the live counter is void: the client resets to 0 and the desktop returns its retained buffer instead of nothing. The epoch param is optional on the wire in both directions, so a client or daemon that predates it degrades to today's seq-only cut rather than erroring. Also extracts the OS-permission helpers to notification-permissions.ts (re- exported, so no importer changes) to keep mobile-notifications.ts under its max-lines budget. Mutation-tested: 3 mutations applied to the epoch logic, 3 killed — including the storage-seed race guard, whose first mutant survived until the deferred-read test was added. * fix(mobile): make the notification watermark atomic and counter-scoped Round-1 review found four ways the epoch fix could still lose notifications. All four are addressed here. 1. Seen-set survived an epoch change. Seen-keys are seq-derived, and terminal bells carry no notificationId (they key on `seq:N` alone). After a restart the fresh counter re-issues low seqs, so a replayed post-restart bell was dropped as a duplicate of a bell from the previous counter. The dedup window belongs to one counter lifetime, so it is cleared on epoch change. 2. Legacy watermarks were trusted. Pre-upgrade installs stored a bare seq with no epoch. Adopting the first observed epoch as "nothing changed" left that unprovenanced seq cutting a counter it was never measured against — stablyai#8591 through the upgrade path. An epoch-less seq no longer survives adoption. 3. seq and epoch were separate storage keys. A process death between the two writes left epoch-B beside seq-57-from-A: a pair that looks internally valid on the next launch and is therefore trusted. They are now one JSON value, which cannot tear, with a read-only migration from the legacy key. 4. Sessions were never retired. They live at module scope so they survive the subscription teardown a reconnect performs, so host removal is the only thing that can drop them. Removal now retires the session and its watermark. Mutation-tested: 3 mutations, 3 killed. The first version of the bell test passed with the fix removed — it exercised the live path, which only adds to the seen-set; only the replay path consults it. Rewritten against the replay path, it fails with `expected 1 to be 2`: the literal lost notification. Mobile notifications + transport: 355 passed. Desktop replay: 11/11. * fix(mobile): catch up on the first connection after a cold open Catch-up hung off 'has this process connected before', which is false on the first ready of a fresh launch — exactly the post-upgrade / post-eviction case that loses everything between the stored watermark and the next live seq. Wait for the persisted read, then catch up whenever this device has delivered for the host before; a first-ever pairing still gets no replay. Co-authored-by: Orca <help@stably.ai> * fix(mobile): serialize live delivery behind the watermark seed, and key catch-up on the record Co-authored-by: Orca <help@stably.ai> * test(mobile): pin the two catch-up mechanisms mutation testing found unguarded Mutating each mechanism of the stablyai#8591 fix in turn showed two survived with the suite still green: the seed's epoch-provenance check, and the host session outliving the subscription teardown. Both are load-bearing, so pin them. - seen-set survives teardown: the desktop's retained buffer replays a notification already delivered live, and only the session-scoped seen-set stops a duplicate banner. - a seed resolving after a live epoch was adopted must not reinstate the dead watermark. Not reachable through subscribeToDesktopNotifications today ('ready' awaits the seed first), so it asserts on the exported pair and says so. Co-authored-by: Orca <help@stably.ai> * fix(mobile): serialize notification delivery per host so the watermark can't outrun what was shown Addresses two MAJOR findings from review of this branch. MAJOR #1 — the watermark could be persisted past a notification the user never saw. `deliverLive` advanced `lastDeliveredSeq` before awaiting the local show, and replay + live delivery ran concurrently, so a live seq 11 handled while catch-up was still showing seq 6 persisted 11. A process death before 7..10 were shown lost them permanently: the next launch asks the desktop for seq > 11. This predates the branch — `origin/main` advances the watermark at the same point — so it is a residual this fix closes, not a regression the branch introduced. It is fixed here because the branch is what makes the watermark load-bearing. Three changes: - the advance moves AFTER the show/dismiss await, so the watermark means "everything up to here reached the user" rather than "was dispatched" - a per-host `deliveryTail` promise chain (`enqueueHostDelivery`) serializes deliveries, so a monotonic advance is also an in-order one - the catch-up batch is ONE queue entry, not one per event. Awaiting per event returns to the event loop between replays and let a live event slot in between seq 6 and 7 — which is exactly the interleave being fixed. The RPC stays outside the queue: `sendRequest` waits up to 30s and holding the chain for that would stall live delivery on a slow link. MAJOR #2 — every delivery awaits the persisted read, so an AsyncStorage read that never settled disabled the host's notifications for the whole app lifetime, with no error and nothing to see. The seed is now bounded at 3s; a late seed still applies when it lands. Proceeding unseeded is strictly better: the watermark stays 0, so catch-up over-fetches and the seen-set de-duplicates. Serializing removed an overlap the duplicate-suppression relied on: `showLocalNotification` deduped two same-id events by observing the first still pending when the second arrived. With deliveries serialized the first completes first, so the second saw no pending state and scheduled a second banner for the same notification. The claim moves to enqueue time, where the overlap is still observable. Dismisses are deliberately not claimed — a dismiss for a shown id is what retires it. Evidence — each mechanism disabled individually against the unchanged suite: - batch-as-one-entry -> reverted to per-item enqueue: ordering test fails - watermark advance -> moved back before the await: ordering test fails - seed timeout -> removed: wedged-read test fails - live-path claim -> removed: concurrent-dedup test fails - replay-path claim -> removed: cross-path dedup test fails Each kills exactly one test, so no mechanism is unguarded and none is redundant. `mobile-notifications.test.ts`'s local `flushAsync` drained 10 microtask ticks. Deliveries are now several awaits deeper, so a fixed tick count under-drains; it yields to the macrotask queue instead. Verified with real timers that the behavior it asserts is unchanged — only the drain depth was wrong. Full mobile suite: 344 files, 2499 passed, 2 skipped. tsc clean, oxlint clean. --------- Co-authored-by: Orca <help@stably.ai>
MumuTW
pushed a commit
that referenced
this pull request
Jul 29, 2026
…(C1) (stablyai#10625) * fix(terminal): park SSH worktrees like local ones (C1 retention, slice A) SSH ptys were blanket-excluded from hidden-view parking, so a hidden SSH worktree retained every pane forever (C1: renderer heap climbs to the V8 ceiling). SSH bytes transit local main — fact-mode watchers already cover them, and main keeps a headless model served over pty:getMainBufferSnapshot that the SSH reattach path never consulted. - isParkRestorableTerminalPty: snapshot-backed OR (SSH + policy); threaded through both park verdicts, both selectors, watcher coverage, and the watcher start guard. Remote-runtime/fail-open/foreign/null unchanged. - Parked-SSH reveal paints from main's headless model (dimension-matched, ~5k rows) and degrades to the relay 100KiB replay unless the snapshot is a non-empty source==='headless' payload — never a blank/stale paint. - Kill switch: settings.terminalSshViewParking (default on). DESIGN.md records the approved plan and the H1 magnitude non-claim. Co-authored-by: Orca <help@stably.ai> * fix(terminal): bound hidden-worktree retention with a force-park budget (C1, slice B) Un-parkable worktrees (remote-runtime ptys, uncoverable tabs, SSH with the slice-A switch off) had unlimited retention: the parking cap/TTL only ever saw eligibility-passing worktrees, so one bad tab pinned a whole worktree's panes forever. Retention is now memory-bounded, not eligibility-bounded. - terminal-hidden-worktree-retention.ts: retention budget (12 hidden / 45min TTL, sized from the measured 2.5-19MB per-pane V8 cost, DESIGN.md §2) over hidden worktrees ordinary parking can never evict; reuses the hot-retain ranking so last-active exemption, deterministic ties, and deadline-driven rechecks hold. Fail-open/foreign-pty tabs are eviction-exempt (a remount would fresh-spawn and orphan the live shell). - Terminal.tsx: force-parked ids join the parked set AFTER the coverage veto (darkness for uncoverable tabs is the accepted cost); buffers captured via the sleep-flow registry before the unmount render; retention TTL added to the recheck deadlines for budget candidates only. - Verdict stays out of its own effect deps; policy test asserts idempotence and time-monotone membership (flip-loop dwell regression). - Kill switch: settings.terminalHiddenWorktreeRetentionBudget (default on). Co-authored-by: Orca <help@stably.ai> * fix(terminal): demote hidden scrollback for eviction-exempt worktrees (C1, slice C) The retention budget (slice B) must exempt worktrees holding fail-open or foreign-worktree ptys — a remount would fresh-spawn and orphan the live shell — which would leave that class unbounded again. Instead, past the same 45min retention TTL their hidden panes drop to the minimum scrollback tier (measured: ~19MB -> ~1.3MB V8 heap per 50k-row pane; trimmed history is gone by design, reveal restores the configured cap for future output). - terminal-hidden-scrollback-demotion.ts: module-state verdict registry (parked-watcher pattern) with content-equality notify damping; applied in the existing scrollback-rows effect in use-terminal-pane-lifecycle. - selectScrollbackDemotedTerminalWorktrees: pure, TTL-gated, time-monotone. - Retention TTL wakeups now also cover exempt worktrees so demotion fires. - Kill switch: settings.terminalHiddenScrollbackDemotion (default on). Co-authored-by: Orca <help@stably.ai> * fix(terminal): paint the SSH model snapshot inline, not via nested coordinator (C1 slice A fix) applyMainBufferSnapshot runs its own structuralReplayCoordinator.run; calling it from applyReattachPayload (already inside the coordinator when a relay replay exists) deadlocks on the coordinator's tail chain. The model paint now mirrors the daemon-snapshot branch inline (folded scrollback + rehydrate + screen, dimension-matched, escape tail last) and arms the restored-snapshot seq baseline so deferred/live chunks the snapshot covers dedupe instead of double-painting. Also falls through (no early return) so reattachPayloadApplied still latches. Adds the folder-workspace id parity unit case. Co-authored-by: Orca <help@stably.ai> * test(terminal): SSH park+reveal e2e round-trip + as-built design notes (C1) Docker-gated (ORCA_E2E_SSH_DOCKER=1) spec: SSH tab parks behind a decoy and reveal restores marker content at multi-viewport scrollback depth. DESIGN.md records the as-built deltas (inline paint, force-park shape, last-active floor) and the residuals so follow-ups aren't lost. Co-authored-by: Orca <help@stably.ai> * fix(terminal): paint SSH reveal from main's model even when the relay replay is empty (C1 review #1) A relay restart empties the replay buffer; the reveal previously painted nothing even when main's headless model held the session. The reattach now prefetches the model snapshot when no structural replay exists (SSH-shaped ptys only) and paints it inside the coordinator; emptiness is judged on the composed payload (scrollbackAnsi + data + pendingEscapeTailAnsi) so an alt-screen snapshot with an empty screen frame still paints. Co-authored-by: Orca <help@stably.ai> * fix(terminal): decouple scrollback demotion (slice C) from the retention-budget switch (C1 review #2) Per the approved contract each slice reverts behind its own switch: slice C now requires only the master terminalHiddenViewParking plus its own terminalHiddenScrollbackDemotion flag. The TTL wakeup timer fires for demotion candidates even with the budget switch off. No DEFAULT_SETTINGS entries exist for sibling flags (defaults are the '!== false' optional pattern), so no explicit defaults are added. Co-authored-by: Orca <help@stably.ai> * fix(terminal): scope eviction exemption to the tab, not the worktree (C1 review stablyai#3) One eviction-exempt tab (fail-open/foreign pty) previously vetoed force-park for its whole worktree, pinning co-located remote-runtime tabs forever. The worktree now force-parks while exempt tabs keep their mounted panes via a per-tab exclusion mirroring the Activity-portal pattern (legacy watcher sync, legacy render, and the overlay cold-parking hook). Ordinary parking is untouched — a worktree with an exempt tab still cannot ordinary-park. Slice C now also demotes exempt tabs' panes as soon as their worktree force-parks under the count budget (they are the only panes left mounted). Co-authored-by: Orca <help@stably.ai> * fix(terminal): demote un-parkable worktrees the force-park lever spared (C1 review stablyai#4) The last-active exemption means a single hidden un-parkable worktree never force-parks — and slice C previously only targeted exempt-tab worktrees, so its panes held full scrollback forever. Demotion now also covers un-parkable non-exempt worktrees past the retention TTL that are absent from the force-parked set (last-active spared, or slice B switched off). Membership stays time-monotone for fixed inputs; covered by new idempotence/monotone selector tests. Co-authored-by: Orca <help@stably.ai> * fix(terminal): keep the hidden clock running through transient background-measure windows (C1 review stablyai#5) Whole-worktree background mounts (browser-automation bootstrap lease, mobile mounts, agent wakes) open a ~3s self-clearing measure window that previously deleted hiddenSince — every remount restarted the 30s hysteresis and the 45min retention TTL, so a periodically re-mounted force-parked worktree never re-parked. The measure window still pauses parking/eviction verdicts (all selectors skip measuring candidates); only the clock survives, so the prior verdict resumes as soon as the window closes. Visible and portal-holding worktrees still reset the clock. Co-authored-by: Orca <help@stably.ai> * test(terminal): make the SSH park+reveal depth assertion prove the model paint (C1 review #6a) Pad the session with ~180KB of output after the numbered markers so the earliest marker falls outside the relay's 100KiB rolling replay buffer while staying inside main's ~5k-row headless model; asserting marker_1 after reveal now proves the headless-model paint rather than passing under the relay fallback. Co-authored-by: Orca <help@stably.ai> * docs(terminal): rewrite DESIGN.md as the single as-built C1 contract (review stablyai#7) One contract matching the code: status IMPLEMENTED around force-park (not the unmount proposal), real kill-switch names with coupling + revert matrices, the true retention-floor formula with measured per-pane and demotion numbers, an explicit when-OOM-is-still-possible paragraph naming the H2 pendingSideEffects residual, the applyMainBufferSnapshot deadlock constraint inside the slice-A section, stable-signal phrasing instead of a capability latch, fail-open AND foreign-worktree exemption class, verified cites, and a planned/landed/follow-up test matrix. Co-authored-by: Orca <help@stably.ai> * fix(terminal): resolve the eviction exemption per pane, not per tab (C1 review stablyai#8) isEvictionExemptTerminalTab read only tab.ptyId — the FIRST leaf's pty — while the coverage veto that makes a worktree a retention candidate walks every pane. A split tab whose second leaf held an unrestorable pty therefore failed coverage (→ force-park target) yet looked exempt-free, so force-park unmounted it and orphaned the live shell. The exemption now resolves panes through the same resolveParkedTerminalPaneCandidates, keeping tab.ptyId in the union for the no-layout/no-capture case. Also from the same review round: - force-park's capture passes includeLocalBuffers:false like every other shutdownBufferCaptures caller; it was serializing up to 512KB/pane of scrollback into the store inside a fix meant to bound renderer heap. - Terminal.tsx unmount resets the scrollback-demotion registry — module state with no reset path, read by a pane effect that runs before the host effect that would clear it, so a stale verdict trimmed restore replays. - memoize watcher coverage per tab within the parking pass; the retention candidates re-asked it for every mounted worktree, not just the parked few. * docs(terminal): drop DESIGN.md — the as-built C1 contract moves to the PR body Co-authored-by: Orca <help@stably.ai> * fix(terminal): cap the deferred PTY side-effect queue (C1 residual H2) pendingSideEffects grew without bound under background timer throttling (~64 drained/s vs hundreds queued/s overnight). Cap at 512 entries with oldest-first eviction: titles drop (last-wins), a pending bell latches onto the next survivor, agent-status payloads collapse onto the survivor keeping the newest 16 (last-wins store state, KB-scale strings). Co-authored-by: Orca <help@stably.ai> * fix(terminal): carry command-lifecycle facts through parked watchers (C1 follow-up) Parked fact-mode watchers omitted onCommandFinished/onCommandCode*, so OSC 133;D and Command Code scrape signals went dark while parked. New parked-terminal-command-status.ts ports the store-level subset: git-UI nudge on every command finish, same-turn status-row drop for SSH PTYs (exact mounted-path parity — the foreground tracker refuses SSH ids), and the Command Code working seed / 1500ms done settle. Byte mode scans the same shared parsers for authority-off parity. Local-PTY status drops stay with the mounted pane: they need pty-connection's process-confirm ladder to tell a leaked nested-shell 133;D from a real agent exit. Co-authored-by: Orca <help@stably.ai> * test(terminal): retention-budget force-park e2e with a retentionLimit override (C1 6b) ORCA_E2E_TERMINAL_RETENTION_LIMIT flows preload → e2e-config → getTerminalParkingPolicyOverrides (exposeStore-gated, positive-integer only) so a spec can shrink the force-park budget to 1. The Docker-gated spec opens two remote worktrees on one relay target (second pre-seeded remote repo), disables terminalSshViewParking to make both un-parkable, hides both behind the local context, and proves the older one force-parks while the last-active exemption spares the newest; re-activating the evicted worktree restores the marker tail via relay replay. Co-authored-by: Orca <help@stably.ai> * test(terminal): retention-budget e2e via same-repo remote worktrees (passes docker lane) The first draft added a second remote repo mid-session, whose pane pty spawn misroutes to the local daemon with the remote cwd (pre-existing multi-repo issue, reproducible without any retention override — a seeded local repo plus one remote repo shows the same misroute). The spec now budgets across three worktrees of the ONE connected repo, created through the product createWorktree path (an external git-worktree-add only lands as a detected worktree needing adoption) and polled through the relay's transient post-connect reconnect window. Verified green on the local Docker lane in 20.8s. Co-authored-by: Orca <help@stably.ai> * fix(terminal): prevent remount thrashing during post-measure cool-down ( Implements the C1 retention contract: preserve worktree `hiddenSinceMs` through a background-measure window (so TTL/ranking stay honest), but re-park waits for a full `coldParkDelayMs` cool-down after the measure ends. Without the cool-down, every ~3s measure lease on a past-deadline worktree thrashes remount/reattach. Core changes: - Terminal.tsx: add measure clock (measuringTerminalWorktreeIdsRef) and post-measure cool-down tracking (terminalWorktreeParkCooldownUntilRef); gate parking candidates until cool-down expires. - Extract snapshot replay choreography to shared terminal-snapshot-replay-paint.ts (used by SSH reattach + daemon restore paths). - Add SSH model snapshot timeout (750ms) with fallback to relay replay. - Move cold-park recheck deadline logic to terminal-cold-park-recheck-deadlines.ts; add cool-down deadline to scheduling. - useTerminalTabColdParking: implement matching measure-clock contract with per-tab cool-down gate to keep tab deadlines synced with worktree retention clock. - Add resolveTerminalMountScrollbackRows() to demote new xterms under demoted worktrees (pane births during demotion must take the demoted tier at create). - Add kill switches: terminalSshViewParking, terminalHiddenWorktreeRetentionBudget, terminalHiddenScrollbackDemotion. * fix(terminal): detect Command Code completion in parked mid-turn panes Seed the byte watcher with in-flight turn state from agent status: the watcher is recreated per park cycle with no startup command to arm it, and the banner scrolled away before parking. Also memoize eviction-exempt checks and use SSH PTY ID builder in tests. * fix(terminal): flush pending command-code settles on reveal remount When a parked pane reveals mid-Command Code turn, the new detector cannot re-observe the already-passed idle composer. Cancelling the settle leaves the row stranded at 'working', so dispose now flushes the pending settle instead. Extract readInFlightCommandCodeTurn to shared space and seed detectors with in-flight turns so remounts complete mid-flight commands. Also memoize SSH model probes to prevent double timeouts on reattach. * fix(terminal): remove scrollback demotion (C1 slice C) The scrollback demotion feature for eviction-exempt hidden worktrees is no longer needed. Retention budget limits are now sufficient without this additional bound. Remove the terminal-hidden-scrollback-demotion module, the selectScrollbackDemotedTerminalWorktrees function, and related per-pane demotion logic. * test(terminal): assert bounded probe during stalled reveal Add assertion to verify that a stalled reveal operation makes exactly one `getMainBufferSnapshot` call, ensuring retry logic doesn't introduce redundant probes that would extend the timeout window before relay fallback. * fix(terminal): implement C1 retention budget for hidden parked worktrees Addresses OOM regressions in hidden parked terminals by force-evicting worktrees past a retention budget: at most 12 mounted while hidden, none past 45 minutes (absolute, not exempted by last-active). Eviction is least-recently-hidden-first. Exempt tabs (unrestorable local PTYs) keep their panes to avoid orphaning shells; worktrees are force-parked even if they contain exempts, and their buffers released elsewhere. SSH/remote worktrees serialize buffers pre-eviction for reveal; local worktrees keep daemon snapshots. Command Code's done-settle window is transferred across park/reveal boundaries so the row cannot strand at 'working'. Model probe on SSH reattach is scoped to park-reveal only, not ordinary reconnects. Includes new E2E suite proving the budget actually releases memory. * memoize eviction-exempt terminal tabs to avoid redundant store reads Each tab's exemption check re-reads the store and walks the layout tree. Introduce selectEvictionExemptTerminalTabIds() to resolve all exempt tabs for a worktree in a single pass, then memoize the result in Terminal.tsx and useTerminalTabColdParking. This prevents O(n) store reads when checking exemptions across multiple tabs and ensures the set remains stable across unrelated re-renders. * refactor: reformat hidden-worktree retention comments Reflow to 80-character lines and remove internal ticket references (C1, C1 slice C). * fix(lint): split overlay slot and eviction-exempt tabs under max-lines Static analysis failed because TerminalPaneOverlayLayer (401) and terminal-parked-tab-watchers (304) exceeded oxlint max-lines. Extract the slot component and eviction-exempt helpers into dedicated modules. * test(terminal): stabilize retention budget e2e control arm Stage un-parkable remote pty ids only after both worktrees are hidden, and keep re-staging during the control-arm poll so a late updateTabPtyId cannot flip the decoy back to park-restorable and ordinary-park it before budget engages. * test(terminal): pin retention e2e decoy to a mounted pane snapshot Use the active pane-identity snapshot for the decoy tab instead of all worktree tabs, and re-assert un-parkable ids after the control-arm hold so a deferred/empty tab id cannot fail the budget-off mounted-count check. * fix: memoize terminal eviction exemptions on layout leaf PTYs Splits add leaf panes to the layout store without changing the tabs array. A memo keyed only on tabs misses this change, leaving new panes unexempted for unmount. Include layout leaf PTYs in the exemption memo key so it recalculates when splits occur or PTYs are re-minted. --------- Co-authored-by: Orca <help@stably.ai>
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
1 participant
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.
Throwaway PR on the fork. Head is the gate commit with no spec change, so the
e2e changed specscheck should select nothing and finish without installing a toolchain or building Electron.Not for merge.