Skip to content

feat(config): remember that onboarding happened — tui.onboarding (v43) - #220

Open
plombeer31 wants to merge 1 commit into
mainfrom
valeryb/onboarding-config-state
Open

feat(config): remember that onboarding happened — tui.onboarding (v43)#220
plombeer31 wants to merge 1 commit into
mainfrom
valeryb/onboarding-config-state

Conversation

@plombeer31

Copy link
Copy Markdown
Collaborator

The bug this removes

The first-run flow has no memory. runLocalModelsStartupGateIfNeeded re-derives "does this user need setup?" on every launch from a health probe, and the Esc branch writes nothing. So an operator who skips the backend picker meets it again on the next launch, and every launch after that — a fresh 10-second gate, forever.

What this adds

tui.onboarding (config v43), four nullable ISO-8601 stamps:

field meaning
introSeenAt the splash was dismissed at least once
completedAt a backend was configured and the flow handed over to the agent
skippedAt the operator escaped out — the flow does not reopen by itself
proposedSecondBackendAt the "you have one, want the other too?" screen was already offered

Plus:

  • src/tui/persist-onboarding-state.ts — read → merge → validate → write → reset, same shape as the other persist-* helpers. The caller stamps the instant, so tests write a fixed one instead of freezing the clock.
  • src/tui/onboarding/needs-onboarding.tsdecideOnboarding() returns { needed, reason }; needsOnboarding() is the boolean.

Timestamps rather than booleans: knowing when a run completed is what lets a later release decide whether an install predates a flow it would like to show again, at the same byte cost.

decideOnboarding answers from config alone, never a probe. A backend that is configured counts as onboarded even when it is not answering right now — a server that is down is the status bar's problem, not a reason to re-run setup. It reuses the three existing predicates (isCloudTextProviderReady, isManagedModeReadyOnDisk, isLocalBackendConfigured) rather than inventing a fourth definition of "configured".

Scope

Nothing consumes the predicate yet — the first-run flow itself is the next PR in the series. This lands alone so the schema bump has its own reviewable diff.

42 also joins SUPPORTED_INPUT_VERSIONS, which the bump would otherwise drop (the list carries the previous versions explicitly and the current one by constant).

Compatibility

Additive. parseUserConfigFile fills absent keys, so a pre-v43 file parses with four nulls and behaves exactly as it does today. No migration code, and a v42 file is still accepted.

Tests

  • config-schema.test.ts — defaults on a v42 file, ISO round-trip, a non-date stamp and a non-object block both rejected.
  • persist-onboarding-state.test.ts — write + getConfig() pickup, merge semantics across two writes, and the rest of tui untouched.
  • onboarding/needs-onboarding.test.ts — fresh install opens; completed, skipped, cloud-configured, external-URL and managed-model-picked all stay shut; the shipped default URL nobody chose still counts as unconfigured.

Full suite: 5132 passed. The two failures on this machine (fs-glob-real — a dev-machine path; send-message-concurrency — timing) also fail on main unchanged.

The first-run flow has no memory. Whether to run setup is re-derived on
every launch from `checkLlamaServer()`, so an operator who pressed Esc at
the backend picker meets it again on the next start, and the next, for as
long as the install lives. Nothing is written when they escape, so there
is nothing to read back.

Adds `tui.onboarding` — four nullable ISO stamps (`introSeenAt`,
`completedAt`, `skippedAt`, `proposedSecondBackendAt`) — plus the
`persistOnboardingState` helper and the `decideOnboarding` predicate that
reads them. Timestamps rather than booleans so a later release can tell
whether an install predates a flow it wants to show again; the caller
stamps the instant so tests can write a fixed one.

`decideOnboarding` answers from config alone. A backend that is
*configured* counts as onboarded even when it is not answering right now
— a server that is down is the status bar's problem, not a reason to
re-run setup.

Nothing consumes the predicate yet; the flow itself follows in the next
PR. Additive schema change, so a pre-v43 file parses with four nulls and
behaves exactly as before.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant