fix(tui): ignore stale Zed poll when switching session directories - #26
Open
detail-app[bot] wants to merge 1 commit into
Open
detail-app[bot] wants to merge 1 commit into
detail-app[bot] wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
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.
Detail bug report: View on Detail
Issue for this PR
No separate issue was opened for this transient TUI editor-context bug.
Type of change
What does this PR do?
Bug: In Zed-terminal fallback mode (no editor WebSocket), the
EditorContextpolls Zed's state DB scoped to the current session directory viaeditor.selection(directory). When the user navigates to a session in a different project,reconnectWithDirectoryswitchesdirectory, resets the selection, and callsconnect()— but an already-startededitor.selection(oldDir)promise is still in flight. Its.thenhandler was guarded only withif (closed || socket) return, which is a no-op in Zed mode (socketis alwaysundefined), so when the stale poll resolved it wrote the old project's selection into the new session's store. BecausereconnectWithDirectoryhad resetlastZedSelectionKeytoundefined, the stale selection passed the "new key" check and contaminated the new session until the next 1s poll overwrote it.Fix (
packages/tui/src/context/editor.ts):.thennow capturesconst pollDirectory = directoryat poll start and adds|| directory !== pollDirectoryto the guard, so a resolution belonging to a previous directory is dropped. This is the necessary-and-sufficient fix.reconnectWithDirectorynow setszedSelection = undefinedbeforeconnect(), so the new directory's poll starts immediately instead of waiting for the old poll's.then/.finallyto release the??=memo slot (a recovery-latency improvement, not the stale-write guard — clearing the reference alone does not cancel the already-attached.then, since JS promises aren't cancellable).How did you verify your code works?
The Zed-poll path previously had no test coverage at all; I added three tests in
packages/opencode/test/cli/tui/editor-context.test.tsxthat mount the context in Zed-terminal mode with a fakeEditorIntegration(connection: () => undefined, a controllable deferred forselection(directory)):connected/pending(baseline Zed path).reconnect(newDir)while anoldDirpoll is in flight, resolving the old poll leavesselection()undefined,labelState()"none",connected()false— the stale poll is ignored.connectedtotrue— the new directory's poll starts promptly.I also mutation-verified each fix line is load-bearing: reverting only the directory guard (keeping
zedSelection = undefined) makes test 2 fail with the staleoldDirselection leaking into the new session — confirmingzedSelection = undefinedalone does not fix the bug, matching the report. Re-applying the guard and reverting only thezedSelection = undefinedline makes test 3 fail with the new-directory selection never arriving — confirming the belt-and-braces line delivers the prompt recovery.Routine checks all pass: the focused suite (8 pass), the existing Zed-helper suite
editor-context-zed.test.ts(16 pass), the fulltest/cli/tui/suite (70 pass), thepackages/tuisuite (191 pass, 1 skip),tsgo --noEmiton bothpackages/tuiandpackages/opencode,oxlinton both changed files (0 errors), andprettier --check.Not verified: the two manual end-to-end smoke procedures (running the live TUI against a real Zed install in Zed-terminal mode, navigating between sessions in different project directories, and the cross-mode Zed→WebSocket reconnect case with an IDE lock file). These require a local Zed installation and a running editor IDE that this environment doesn't have, so I relied on the unit/behavioral tests above, which exercise the same
editor.selection(directory)poll andreconnectWithDirectorycode paths against a controllable deferred.Screenshots / recordings
Not a UI change.
Checklist
Automatic Fixes PRs can be configured here.