Repository navigation
Conversation
anomalyco#57594 made the TUI report a location as missing only when the server answers with its typed LocationNotFoundError, and to retry any other failure instead of surfacing it. It shipped without a test file, so the behaviour that closed anomalyco#55250 has no regression guard. Provider level (packages/tui/test/cli/tui/location-recovery.test.tsx): - a location the server reports as missing is asked once, since retrying cannot succeed until the folder changes - a location that fails for any other reason is asked three times and is not reported as missing App level (packages/tui/test/location-unavailable.test.tsx): - a 503 from the plugin-activation gate keeps the composer and never mounts the blocking "Session location unavailable" prompt - a location the server reports as missing still asks for another directory Red-green verified: reverting packages/tui/src/context/location.tsx to its state before anomalyco#57594 fails "a transient location failure keeps the composer" and "a location that fails for another reason is retried".
github-actions
Bot
added
the
needs:compliance
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 72 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
The tests are sound and guard #57594's behaviour. I checked by running them against location.tsx from before #57594: the two retry tests fail and the other three pass, as described. Deleting only the !isLocationNotFoundError(cause) guard in the catch also makes both retry tests fail, so they catch that regression too. Type check passes. Public API unchanged (tests only).
Worth addressing:
- Both retry tests wait out the real backoff in
@opencode/util/retry(500 ms, then 1000 ms). That adds about 1.5–3 s per test to a suite that is already short on time (see.opencode/skills/tui-tests). A way to shorten or inject the retry delay inLocationProviderwould keep them fast. - The app-level test named "keeps the composer" only checks that the prompt is absent (inline comment).
- The "missing" app-level test tears down while a session-tabs write is still in flight. Over repeated runs it logs
Failed to persist session tabsand a flockENOENTmost of the time. Waiting for the app to settle before the test ends should stop it. - The extra test "a location sync reads the location once" isn't part of the recovery path and isn't in the description; it can be dropped (inline comment).
| }) | ||
| await setup.ready | ||
| // The retry gives up after three attempts. | ||
| await waitFor(() => attempts >= 3) |
There was a problem hiding this comment.
This only checks that the recovery prompt is absent. It would still pass if the session view never rendered at all, for example if the session failed to load. Since the test is named "keeps the composer", could it also check for something the composer renders, so it actually confirms the composer stayed usable?
| } | ||
| }) | ||
|
|
||
| test("a location sync reads the location once", async () => { |
There was a problem hiding this comment.
This tests data.location.sync reading /api/location once, which isn't the location-unavailable recovery path, and it isn't mentioned in the description. I'd drop it to keep the PR focused on the regression it guards.
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 72-hour window. Feel free to open a new pull request that follows our guidelines. |
github-actions
Bot
removed
the
needs:compliance
Problem
#57594 changed the TUI so a location is reported as missing only when the server answers with its typed
LocationNotFoundError, and so any other failure is retried instead of being surfaced. It touched a single production file,packages/tui/src/context/location.tsx, and committed no test file.That means the behaviour which closed #55250 has no regression guard. The failure is easy to reintroduce: the blocking prompt is mounted from one comparison, and the retry that keeps the composer alive lives in one
catch.Change
Tests only, no production code.
packages/tui/test/cli/tui/location-recovery.test.tsx(provider level):packages/tui/test/location-unavailable.test.tsx(app level, throughcreateAppFixture):Verification
bun test test/location-unavailable.test.tsx test/cli/tui/location-recovery.test.tsxinpackages/tui: 5 pass, 0 failpackages/tui/src/context/location.tsxreverted to its state before fix(tui): only show location unavailable when the server reports it missing #57594, "a transient location failure keeps the composer" and "a location that fails for another reason is retried" both fail, while the other three still passbun run checkinpackages/tui: 34/34 tasks pass; the full repository check is green as wellbunx oxlinton both new files: 0 warnings, 0 errorsNotes
The 503 in the app-level test is the failure this was originally reported against: while a cold location warms up, the server's plugin-activation gate answers 503, and one failed request used to fail the whole location sync and mount the recovery prompt.
The app-level test asserts on the rendered frame, so it covers the prompt itself rather than only the provider state.