vscode: self-heal the 'No active terminal' click; recovery as last resort (#982)#1006
Merged
Conversation
… as last resort
… (codex review)
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
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.
PIR Review: Self-heal the "No active terminal" click (recovery as last resort)
Fixes #982
Summary
Clicking a builder row in the Codev sidebar could dead-end on an unactionable
Codev: No active terminal for <id>warning. The sidebar lists a builder from/api/overview(disk-sourced) while the terminal opener needs a live PTY session from/api/state(in-memory registry), which momentarily omits a builder while Tower rehydrates / on-the-fly-reconnects sessions on every call. The dominant cause is therefore transient and self-healing, but the opener warned on the first miss. This change wraps the resolve in a bounded retry (reusing the sharedbackoffDelayMscurve with interactive-tuned params) so the transient case opens silently; only a genuinely persistent miss surfaces an actionable toast that leads with Retry and offers Recover Builders (afx workspace recover, dry-run) as a last resort.Files Changed
packages/vscode/src/terminal-resolve.ts(+91 / -0) — new vscode-free retry/resolve helperpackages/vscode/src/terminal-manager.ts(+81 / -11) — delegate to the helper; new recovery toastpackages/vscode/src/__tests__/terminal-resolve.test.ts(+147 / -0) — 7 behavioral testspackages/vscode/src/__tests__/terminal-manager.test.ts(+36 / -0) — 5 source-level wiring testsCommits
ed8e3ea4[PIR vscode + tower: 'No active terminal for X' warning is unactionable — Tower's session registry and overview disagree, no in-extension recovery path #982] Self-heal terminal open with bounded retry; recovery toast as last resort8a52338b[PIR vscode + tower: 'No active terminal for X' warning is unactionable — Tower's session registry and overview disagree, no in-extension recovery path #982] Tests for retry-resolve helper and recovery-toast wiring(Plus
[PIR #982]plan and thread commits, and porch chore commits, on the branch.)Test Results
pnpm --filter codev-vscode compile(check-types + lint + esbuild): ✓ passpnpm --filter codev-vscode test:unit: ✓ pass (27 files, 348 tests; 12 new)build✓ (6.5s),tests✓ (20.6s)dev-approvalgate (running worktree).Architecture Updates
No
arch.mdchanges needed. The server-side resilience for this window (the startup-reconcile barrier behind/api/stateand/api/overview) is already documented atcodev/resources/arch.md:213. This PR adds a localized, client-side bounded retry inside the existingterminal-manageropen path; it introduces no new module boundary, endpoint, or cross-component contract, so it sits below the altitudearch.mdrecords.Lessons Learned Updates
Added one entry to
codev/resources/lessons-learned.md(Debugging and Root Cause Analysis): two views backed by different freshness sources can diverge transiently, not only on outright failure, and a bounded retry that absorbs the self-healing window beats dead-ending on the first miss. It is filed as a sibling to[From 916](same divergence class, opposite direction: there the whole shared cache nulled; here a single per-builder field is briefly stale).Things to Look At During PR Review
terminal-resolve.tsis the core. It is pure / vscode-free (deps injected:sleep,attempts), which is why the retry behavior is tested for real (terminal-resolve.test.ts) rather than through the heavyTerminalManagervscode harness. The retry returns a discriminated outcome (ok/ambiguous/missing); ambiguity short-circuits without retrying because it is stable.backoffDelayMsfrom@cluesmith/codev-core/reconnect-policy(the curve consolidated in core: extract transport-agnostic reconnect policy; adopt in vscode + dashboard terminals + tunnel client #961), with interactive params{ baseMs: 150, capMs: 800 }over 4 attempts (~150/300/600ms, ~1s total) instead of the module's reconnect-loop defaults (base 1s, cap 30s), which would make a click feel laggy. A test asserts the emitted delays equal the shared curve at those params, so the convention can't silently drift back to a hand-rolled delay.afx workspace recoveronly addresses the rare destroyed-session tail (e.g. a dead shellper) and is workspace-wide (it cannot target one builder), so the toast leads with Retry and the recover button stops at the dry-run preview (no--apply) for the user to review scope.terminal-manager.test.tsfor the vscode-side wiring (delegation, toast labels, recover command), matching that file's existing harness rationale (constructingTerminalManagerneeds broad vscode mocking).How to Test Locally
For reviewers pulling the branch:
pir-982→ View Diffafx dev pir-982afx workspace recover(dry-run) at the workspace root.Build / Test Setup Note
This was a fresh worktree where
@cluesmith/codev-coreand@cluesmith/codev-typeshad nodist/yet, which made unrelated subpath-importing tests fail to resolve until those packages were built (tsc). Not a code issue, but worth knowing when running the vscode unit suite in a new worktree: build the workspace's TS packages first (or runpnpm build).