Ask for a workspace even when the model is already set - #185
Merged
Conversation
The setup flow gained a workspace step, but the condition that opens it never grew the term. It read the model settings alone, so a server with `ZORP_MODEL` exported, or one configured before the step existed, was judged set up and the question was never asked. That person lands on a composer whose every tool call runs in whatever directory the server was started in, which is the zorp checkout itself. The two are set in different places and either one alone is enough to make the agent unable to do the thing it was opened for, so `shouldOnboard` asks about both. It lives in onboarding.ts beside `isFirstRun` rather than in main.ts, because main.ts has no tests and this is a decision worth pinning. The dismissal flag carries a version now. A dismissal is a person saying no to what they were shown, and a browser that dismissed the old flow was never shown the workspace step, so the old flag is not an answer to the question this flow now asks. Claude-Session: https://claude.ai/code/session_01PG2vuux5tMsjnZBLWJzMyp
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.
The bug
The setup flow gained a workspace step, but
maybeStartOnboardingnevergrew the term. It read the model settings alone through
isFirstRun, sotwo people were never asked where the agent should work:
ZORP_MODELorZORP_BASE_URLBoth land on a composer whose every tool call runs in whatever directory
the server was started in, which is the zorp checkout itself. That is
the exact thing the workspace step was added to prevent.
The fix
shouldOnboard(settings, workspaceConfigured)asks about both. The twoare set in different places and either one alone leaves the agent unable
to do the thing it was opened for.
It lives in
onboarding.tsbesideisFirstRunrather than inmain.ts, becausemain.tshas no test harness and this is a decisionworth pinning. Three cases now cover it: configured with a workspace
asks nothing, configured without one still asks, and a workspace does
not stand in for a model that was never set.
The dismissal key carries a version. A dismissal is a person saying no
to what they were shown, and a browser that dismissed the old flow was
never shown the workspace step, so the old flag is not an answer to the
question this flow now asks. Every browser is asked once more, and one
click still skips it.
The workspace is already read before the settings read on the connect
path, so a configured one is known by the time this decides.
Checks
npm run checkcleannpm test: 422 pass, 0 fail, up from 419npm run buildproduces the bundlehttps://claude.ai/code/session_01PG2vuux5tMsjnZBLWJzMyp