fix(tui): stop telling fresh installs that llama-server is down - #136
Conversation
|
Thanks for this, the intent is right: a fresh install should not be told its llama-server is down when no local backend was ever chosen. Two things block the merge as the branch stands: 1. The branch does not compile. 2. The Both come from the missing |
|
Both were the same missing file — thanks for catching it, you were right on the diagnosis. 1. Compiles. 2. Prop is threaded. The real call site now passes One thing I changed while writing the predicate, because the obvious version reintroduced the bug this PR exists to fix. Picking "Local models (llama.cpp)" writes That is the same "your install is broken" tone, just relocated to mid-onboarding. The predicate therefore requires something that could actually be serving: export function isLocalBackendConfigured(): boolean {
const cfg = getConfig();
if (cfg.localModels.managed.modelId !== null) return true;
if (cfg.localModels.mode !== "external") return false;
return cfg.localModels.url !== USER_CONFIG_DEFAULTS.localModels.url;
}The Known limitation, deliberately not fixed here. llama.cpp's own default port is 8080, which is also Testing. 11 tests in Full |
What
A fresh install tells the user twice that a llama-server they never set up is unreachable:
llama-server not reachable, with a rawlast error: connect ECONNREFUSED 127.0.0.1:8080underneath — on a machine where noserver has ever existed. It reads as a broken install rather than an un-started one.
○ down. It appears onthe splash/home screen and on the LLM tab, because
USER_CONFIG_DEFAULTSshipslocalModels.mode: "external"athttp://127.0.0.1:8080and the config layer synthesizeslocal-llamaas the active provider before the user has configured anything.Neither surface asks whether local is actually the user's route. Both now do.
Approach
Not deleted — suppressed until it means something. Deleting the indicator would blind the
people who genuinely run llama-server, which is exactly who it is for.
A new
isLocalBackendConfigured()(src/tui/run-local-models-config-wizard.ts) answers"did the user ever choose local?", as opposed to the existing
isManagedModeReadyOnDisk()which answers "can local serve right now":
managed.modelId→true(including mid-download: the userpicked it, so health about it is wanted)
trueonly whenlocalModels.urldiffers from the shipped default, sincethe default URL is not evidence of intent
falseIt is carried into the TUI as
TuiSessionInfo.localBackendConfigured(read after thestartup gate, so a model picked in the wizard seconds earlier already counts) and seeds
llmHealth.localConfigured.Self-healing, so no real local user loses the signal: a
healthyprobe latcheslocalConfiguredon. Somebody running llama-server on the default URL without ever touchingconfig gets the indicator after the first probe, and keeps seeing
○ downwhen their serverlater dies.
Behaviour
○ downon the splash screenllama-server not reachable+ ECONNREFUSEDChoose how atomic-agent should run models○ down○ down(unchanged)llama-server not reachable+ error○ downThe stderr line printed before the alt screen follows the same split: the honest
local-llm unreachable at <url>when a backend was configured,no model configured yet — starting setup…when not.Known limitation
llama.cpp's own default port is 8080, which is also
USER_CONFIG_DEFAULTS.localModels.url,and the wizard prefills it. A user who picks "Remote llama.cpp" and accepts the prefilled
default is stored byte-identically to a fresh install, so the honest error does not appear
for them at the wizard. Telling those two apart needs an explicit "setup completed" marker
in the config schema (a version bump), which is out of scope here. The footer indicator is
unaffected — it latches on after the first healthy probe.
Testing
npm run lint(tsc --noEmit) clean, verified against the branch as pushed to originrather than only a local working copy
run-local-models-config-wizard.test.ts: 7 pinningisLocalBackendConfigured(fresh defaults, managed without a model, managed with a model, model selected while still
in external mode, user-typed URL, URL rewritten back to the default, embeddings-only), and
2 that mock Ink's
renderto assert the element handed to the wizard actually carrieshadConfiguredBackend; plus the 2 first-run headline tests and the 3localConfiguredlatch tests already in the PR
expected undefined to be false; stashing the source yieldsTypeError: isLocalBackendConfigured is not a functionsrc/tuisuite, serial: 931 passed / 5 failed, against 917 passed / 5 failedon unmodified
main— the same 5 pre-existing failures (tui-app.test.tsx×2,splash-banner,chat-log,persist-embedding-hybrid-recall).llm-health-pollerfailed once early on, so the suite was re-run three more times: 0/3 recurrences, and it
passes 12/12 solo.
One bug this shook out along the way:
createInitialTuiStateis typed to require a sessionbut several tests call it with none (test files sit outside tsconfig's
include, so nothingcatches it). Reading a field off
sessionturned that latent bug into 28 crashes inmcp-reducer.test.ts; the call site uses optional chaining so those callers keep working.