Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
This looks correct and safe to merge. I ran your two new tests against the base code and both fail there (a session move is sent). With this PR the whole prompt-move.test.tsx suite passes (21 tests), and bun run typecheck in packages/tui passes. I couldn't trigger the false prompt end to end, so I'm relying on these component-level tests.
On the code: location.set re-runs sync() even when the location hasn't changed, so the retry really happens and a successful sync clears the prompt. Spreading location.ref keeps the location that failed, because the error is only recorded when it matches the current location. And a move to the same directory never helped anyway: if the folder is really gone, the server rejects the move.
Public API: unchanged.
One small suggestion about the test, inline.
| } | ||
| }) | ||
|
|
||
| test.each([true, false])("choosing the current directory retries without moving (recovered=%s)", async (recovered) => { |
There was a problem hiding this comment.
This test is harder to follow than the fix needs: a test.each that branches on recovered, with an early return in the middle, plus a new onlyOnError fixture flag and <Show> wrapper, and restoreLocation() changing the fixture's input. Could you keep just the recovered case: open the picker, choose the current directory, and check that no move is sent and the prompt goes away? That covers the fix. The still-failing branch adds little and makes the test longer.
There was a problem hiding this comment.
Simplified in b345583: kept one recovered-case test, removed the conditional test branch and onlyOnError option, and stopped mutating the fixture input. The prompt now follows actual location info in the fixture. The regression still fails on the base implementation; the full file passes 20 tests, independent review passed 200 tests across ten repetitions, and typecheck/root checks pass.
There was a problem hiding this comment.
Thanks, b345583 covers it. I read the diff: the test is now a single clear case, and the conditional branch, the onlyOnError option and the mutation of the fixture's input are all gone. The new <Show> condition, which follows the actual location info, still shows the prompt for the existing recovery tests. Nothing more needed from my side on the test.
|
One additional end-to-end reproduction from Windows 11, in case it's useful for testing beyond the component-level regression test. Environment: Windows 11 25H2, OpenCode v2.0.32, official npm installation, Windows Terminal with PowerShell 5. I've encountered the false location-unavailable prompt after server restarts, system time changes, skills-directory changes, and computer restarts followed by reopening a previously used project. Occasionally, selecting the existing directory repeatedly returns me to the same prompt, leaving the TUI unusable until I force-terminate it with I haven't tested a build containing this PR or the related location-sync fixes, so I can't confirm whether the loop persists with these changes. I'm sharing this as an additional Windows reproduction, particularly for the repeated unsuccessful directory-selection case. |
Issue for this PR
Closes #56958
Type of change
What does this PR do?
Choosing the session's current directory from the location recovery prompt now retries the existing location sync instead of requesting a session move. This avoids queueing a no-op move behind an active step, preserves the location reference, and lets a successful sync clear the prompt. A failed retry keeps the recovery prompt available.
Moves to other directories and worktree creation retain their existing paths. This addresses the recovery action reported by @ZhenningLang; it does not change the transient-error trigger or reconnect policy covered by #55258 and #55940.
How did you verify your code works?
bun test --timeout 30000 test/cli/tui/prompt-move.test.tsxfrompackages/tui: 20 passed, 103 assertions.bun run typecheckfrompackages/tui: passed.bun run check: passed, including lint (zero errors) and all 34 workspace typecheck tasks. On Windows, the tracked app declaration symlink was represented by a temporary hardlink to its actual target for this check.The tests inject a 503 into location synchronization and then exercise the actual recovery picker. They do not reproduce the hourly eviction sweep or run an active model step.
Screenshots / recordings
No layout changes. Renderer assertions verify that the existing prompt disappears after recovery.
Checklist
AI-assisted implementation and independent review; reviewed and tested locally.
Added by the opencode bot (AI-generated).
Related: #43415, #55250, #49352