Skip to content

fix(tui): stop telling fresh installs that llama-server is down - #136

Merged
sosidudku1 merged 5 commits into
mainfrom
valeryb/hide-local-health-until-configured
Aug 18, 2026
Merged

fix(tui): stop telling fresh installs that llama-server is down#136
sosidudku1 merged 5 commits into
mainfrom
valeryb/hide-local-health-until-configured

Conversation

@plombeer31

@plombeer31 plombeer31 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

What

A fresh install tells the user twice that a llama-server they never set up is unreachable:

  1. The first-run screen is headed literally llama-server not reachable, with a raw
    last error: connect ECONNREFUSED 127.0.0.1:8080 underneath — on a machine where no
    server has ever existed. It reads as a broken install rather than an un-started one.
  2. Within ~1.5 s of the TUI mounting, the prompt meta-row shows ○ down. It appears on
    the splash/home screen and on the LLM tab, because USER_CONFIG_DEFAULTS ships
    localModels.mode: "external" at http://127.0.0.1:8080 and the config layer synthesizes
    local-llama as 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 mode with a known managed.modelIdtrue (including mid-download: the user
    picked it, so health about it is wanted)
  • external mode → true only when localModels.url differs from the shipped default, since
    the default URL is not evidence of intent
  • a ready cloud provider → false

It is carried into the TUI as TuiSessionInfo.localBackendConfigured (read after the
startup 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 healthy probe latches
localConfigured on. Somebody running llama-server on the default URL without ever touching
config gets the indicator after the first probe, and keeps seeing ○ down when their server
later dies.

Behaviour

Situation Before After
Fresh install, nothing configured ○ down on the splash screen nothing
First-run screen, nothing configured llama-server not reachable + ECONNREFUSED Choose how atomic-agent should run models
Configured local backend, server down ○ down ○ down (unchanged)
Own URL configured, first-run screen llama-server not reachable + error unchanged — it really is unreachable
llama-server on default URL, never configured ○ down appears after the first healthy probe, then behaves normally

The 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 origin
    rather than only a local working copy
  • 11 tests in run-local-models-config-wizard.test.ts: 7 pinning isLocalBackendConfigured
    (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 render to assert the element handed to the wizard actually carries
    hadConfiguredBackend; plus the 2 first-run headline tests and the 3 localConfigured
    latch tests already in the PR
  • Each verified to fail without the fix: dropping only the call-site line yields
    expected undefined to be false; stashing the source yields
    TypeError: isLocalBackendConfigured is not a function
  • Full src/tui suite, serial: 931 passed / 5 failed, against 917 passed / 5 failed
    on unmodified main — the same 5 pre-existing failures (tui-app.test.tsx ×2,
    splash-banner, chat-log, persist-embedding-hybrid-recall). llm-health-poller
    failed 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: createInitialTuiState is typed to require a session
but several tests call it with none (test files sit outside tsconfig's include, so nothing
catches it). Reading a field off session turned that latent bug into 28 crashes in
mcp-reducer.test.ts; the call site uses optional chaining so those callers keep working.

@sosidudku1

Copy link
Copy Markdown
Collaborator

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. tui-command.ts imports and calls isLocalBackendConfigured from run-local-models-config-wizard.ts, but that function is not defined or exported anywhere in the diff (run-local-models-config-wizard.ts is not in the changed files). tsc --noEmit fails with TS2305: Module has no exported member 'isLocalBackendConfigured'. It looks like the file that adds it did not get committed.

2. The hadConfiguredBackend prop is never passed in production. It is added to LocalModelsConfigWizard and covered by tests, but the one real call site (run-local-models-config-wizard.ts:57) does not pass it, so it defaults to false. That means the second half of the described behavior (an honest "llama-server not reachable" heading for someone whose configured server is actually down) never runs: the wizard always shows "Choose how..." and hides the error, even when a backend was configured.

Both come from the missing run-local-models-config-wizard.ts changes. Add the isLocalBackendConfigured definition/export and thread hadConfiguredBackend through from that file, re-run tsc and the full src/tui suite, and this should be good. The latch logic in the reducer and tui-app.tsx is correct and well tested on its own.

@plombeer31

Copy link
Copy Markdown
Collaborator Author

Both were the same missing file — thanks for catching it, you were right on the diagnosis. src/tui/run-local-models-config-wizard.ts is now committed (77c93a5).

1. Compiles. isLocalBackendConfigured() is defined and exported there. tsc --noEmit is clean on the branch as pushed (I re-fetched the branch from origin and typechecked that, not just my working copy).

2. Prop is threaded. The real call site now passes hadConfiguredBackend: isLocalBackendConfigured() into LocalModelsConfigWizard, so the configured-but-down half actually runs. grep -rn "LocalModelsConfigWizard" src/ confirms that is the only non-test construction site.

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 mode: "managed" before any weights are pulled (local-models-config-wizard.tsx:90). So if the predicate counted bare managed mode as configured, a user who picks local models and quits before a multi-gigabyte download would be told on next launch:

llama-server not reachable
last error: connect ECONNREFUSED 127.0.0.1:19091

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 modelId check has to precede the URL check: loadConfig() derives localModels.url from managed.port in managed mode, so the URL comparison would report "configured" for any managed config.

Known limitation, deliberately not fixed here. 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 for them the honest error still does not appear at the wizard. Distinguishing that case needs an explicit "setup completed" marker in the config schema — a version bump — which felt out of scope for a review fix. Happy to do it as a follow-up if you want it.

Testing. 11 tests in run-local-models-config-wizard.test.ts: 6 predicate cases (fresh defaults, managed-without-model, managed-with-model, model-selected-in-external-mode, user-typed URL, URL rewritten back to the default, embeddings-only) and 2 that mock Ink's render to assert the element handed to the wizard really carries hadConfiguredBackend. Verified each fails without the fix — dropping only the call-site line gives expected undefined to be false, and stashing the source gives TypeError: isLocalBackendConfigured is not a function.

Full vitest run src/tui --no-file-parallelism: 931 passed, 5 failed — the same 5 that fail on main today. LlmHealthPoller failed in one early run, so I ran the suite three more times to be sure it was not mine: 0/3 recurrences, and it passes solo.

@sosidudku1
sosidudku1 merged commit 64720d1 into main Aug 18, 2026
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.

2 participants