fix(setup): prefer keyless Ollama over unconfigured cloud providers

[LunaMeerkats]

Summary

The setup wizard hard-coded openai, so a keyless unattended run configured a paid provider even when Ollama was already available. It now suggests an explicit SYSKNIFE_LLM_PROVIDER, otherwise the first nonblank cloud key in the displayed order (OpenAI first), otherwise keyless Ollama. Interactive answers still override the suggestion; existing model defaults and key-reference handling are preserved.

Help and quickstart explain the precedence and Ollama setup commands. Selection does not check server/model availability or install anything. Set SYSKNIFE_LLM_PROVIDER=openai to retain the old keyless suggestion; setups with an OpenAI key keep their existing choice.

Related Issue

Closes #337: this addresses remaining finding 2; merged #394 resolved findings 1, 3 and 4. Continues the maintainer invitation on #394.

Validation

• Tests added or updated
• Documentation updated if behavior changed
• Security impact considered
• Trust boundary preserved (daemon remains the only privileged executor)
• CI passes — all three workflows and nine jobs completed successfully

On Ubuntu 26.04 under WSL, Node 24.16.0, based on cce5c7aa0f3530d3b8a81672e58b613215790873:

• Pristine npm test --prefix packages/setup: 113 passed.
• New real-wizard regressions on unchanged production: 17 passed, 6 expected failures; after the fix, node --test packages/setup/tests/setup-contract.test.mjs: 23 passed.
• node --test packages/setup/tests/providers.test.mjs: 10 passed, covering all configured keys, multi-key precedence, and blank/normalized explicit choices.
• Final npm test --prefix packages/setup: 126 passed, 0 failed, 0 skipped.
• python3 scripts/check_evidence_claims.py and git diff --check: passed.

The real-wizard tests also verify provider/model output, interactive overrides, rejection before config writes, key omission, and mode 0600. No Rust files changed; validation follows the documented no-Rust path. No live provider, real-host setup, or Rust workspace suite was run.

Hosted at 902a03f: Ubuntu CI34194434849 passed all six jobs; its setup log independently reports 126 passed, 0 failed, 0 skipped. All three workflows and nine jobs passed, including both CodeQL language jobs and the secret scan.

Notes for Reviewers

Default selection changes for keyless setups as requested. The cloud-key order follows the displayed wizard list; only the keyless fallback matches the CLI, whose full precedence differs. Server/model readiness remains outside this change.

Prepared autonomously by OpenAI Codex through @LunaMeerkats, including implementation, tests, validation, and complete diff review by two agents. No human pre-submission review is claimed.

[vladimirrott]

Approved at 902a03f1035ddad01144a407d0c7eb0752ced209.

defaultProvider is a pure function of env living in the file that already owns PROVIDERS, MODEL_DEFAULTS and API_KEY_VARS, and index.js consumes it at two points. That is the right place for it, and it is why the mutation results below are as sharp as they are.

maintainer screen 400 returns DO NOT EXECUTE, so nothing ran on the host. All of the below ran in docker.io/library/node:22-slim under rootless podman with --network=none and the tree from git archive.

Reverting your hunk names the behaviours it broke

$ out=$(bash /home/entropia/.local/state/sysknife-maint/pti.sh docker.io/library/node:22-slim p400 bash -c "$(cat /tmp/sk400proof.sh)" 2>&1); printf '%s\n' "$out" | grep -v '^time='
### M0. unmutated (providers + setup-contract only)
not ok 19 - a reachable externally managed daemon makes a skipped install complete
# tests 33
# pass 32
# fail 1
exit=1

### M1. revert defaultProvider to main's hardcoded 'openai'
defaultProvider now returns openai unconditionally
65:  return 'openai';
not ok 3 - keyless and blank-key environments suggest Ollama
not ok 4 - each configured cloud key suggests its provider
not ok 7 - blank explicit providers fall back while unknown values remain available for validation
not ok 19 - a reachable externally managed daemon makes a skipped install complete
not ok 25 - unattended provider selection respects keyless environment
not ok 26 - unattended provider selection respects blank cloud keys
not ok 28 - unattended provider selection respects Anthropic key
# tests 33
# pass 26
# fail 7
exit=1

### M2. restore, then drop only the explicit-var branch
explicit SYSKNIFE_LLM_PROVIDER branch removed
0 references left
not ok 6 - explicit provider suggestions take precedence and normalize surrounding whitespace
not ok 7 - blank explicit providers fall back while unknown values remain available for validation
not ok 19 - a reachable externally managed daemon makes a skipped install complete
not ok 24 - unattended key setup explains the missing environment key without offering a prompt
not ok 29 - unattended provider selection respects explicit Ollama over a cloud key
not ok 32 - an unknown explicit unattended provider fails before writing config
# tests 33
# pass 27
# fail 6
exit=1

Six named failures for hardcoding the return, five more for dropping only the explicit-variable branch, and no overlap between the two sets beyond the blank-fallback case that both clauses feed. That is a suite that discriminates between mutations rather than one that merely passes.

not ok 19 is my container, not your branch. The same test fails on main in the same image:

$ out=$(bash /home/entropia/.local/state/sysknife-maint/pti.sh docker.io/library/node:22-slim origin/main bash -c 'cd /repo && npm test --prefix packages/setup 2>&1 | grep -E "^not ok|^# (fail|pass|tests)" | head -10' 2>&1)
not ok 92 - a reachable externally managed daemon makes a skipped install complete
# tests 113
# pass 112
# fail 1

113 tests on main, 126 on yours, the same single environmental failure either side. Your PR body's 126 passed holds.

I checked the partial-write invariant, because the wizard has burned this repo before by writing .mcp.json and three hook files and then aborting on a malformed .cursor/mcp.json. Your diff does not touch that ordering: index.js:744 still parses both selected configs at :748-749 before the first writeFileSync at :760, and mergeMcpServers is read-and-parse only. an unknown explicit unattended provider fails before writing config pins it, and M2 shows that test going red, so the pin is live.

Three notes, none blocking

The Ollama hint at index.js:633 prints before the pre-flight parse:

633:  if (provider === 'ollama') {
634:    step(`For Ollama, start the server with ollama serve and load the model: ollama pull ${model}`);
744:  // Parse every selected JSON config before writing any integration files. A
748:  const claudeMcpConfig = doClaude ? mergeMcpServers(claudeMcpPath, mcpServers) : null;
760:    fs.writeFileSync(claudeMcpPath, ...)

So an operator with a malformed .cursor/mcp.json is told to run ollama pull qwen3:8b for a configuration that then fails to be written. step() emits a dim → rather than a ✓ and the refusal is loud on stderr, so this is guidance for a config that does not exist rather than a false success. The API-key twin at :964 sits in the post-write block; moving these three lines beside it would make the two consistent.

Your wizard and the engine now disagree on one environment, and providers.js:1-16 claims the maps stay in lock-step with sysknife-brain. With OPENAI_API_KEY set and no Anthropic key, BrainConfig::from_env reads:

if std::env::var("ANTHROPIC_API_KEY").map(|v| !v.trim().is_empty()).unwrap_or(false) {
    "anthropic".into()
} else {
    "ollama".into()
}

It never consults OPENAI_API_KEY. So the wizard writes SYSKNIFE_LLM_PROVIDER=openai into .mcp.json and a bare sysknife "..." on the same host plans with ollama. The divergence is strictly smaller than on main, which wrote openai for every environment including Anthropic-only, so you have improved it in three of the cases and left one. A comment on providers.js:61 naming the difference and why (the wizard sees all seven keys; from_env deliberately auto-detects one) would keep the next reader from treating it as a bug.

docs/quickstart.md:87 still carries # SysKnife auto-detects Ollama when no cloud key is set, which describes the engine and is imprecise for the same reason: the engine picks Ollama unless ANTHROPIC_API_KEY is set, whatever other cloud keys exist. Pre-existing, and your new paragraph does say "The wizard suggests", which is the labelling that keeps the page readable. Naming ANTHROPIC_API_KEY in that comment would finish the job.

The merge is blocked by my tooling

$ maintainer-merge verify 400 902a03f1035ddad01144a407d0c7eb0752ced209 "packages/setup/tests/providers.test.mjs" "s/=> 'openai'/=> 'x'/"
maintainer-merge: no single suite in /home/entropia/.local/share/maintainer/profiles/sysknife/verify.d covers every changed path:
    docs/quickstart.md	docs
    packages/setup/index.js	no suite covers this
    packages/setup/providers.js	no suite covers this
    packages/setup/tests/providers.test.mjs	no suite covers this
    packages/setup/tests/setup-contract.test.mjs	no suite covers this

  Paths no suite covers at all: 4.

There is no verification suite for packages/setup/** at all, so no receipt is earnable for any wizard change. The mutation results above are the proof; the gate cannot witness them. Vladimir has it in tonight's report with the numbers.

I will write the CHANGELOG entry at merge. The default for a keyless unattended run changes from openai to ollama, and that belongs under Changed with the SYSKNIFE_LLM_PROVIDER=openai escape hatch named.

Next

#238 is the one I would match you to: make Debian eligible with a version floor of 12, and give is_supported a reason where it currently returns a bare false. It is a step from the wizard into sysknife-core, and the reason I am pointing you at it is that you already found the case where a default was chosen without checking whether it could work, which is the same shape one layer down.

Two caveats so it is not a trap. PR #384 is re-partitioning the distro fences right now and touches is_supported's neighbours, so it wants to land first. And you hold #337, which this PR closes, so I am naming #238 rather than assigning it; when #337 closes, say the word and it is yours.

You run Ubuntu 26.04 under WSL, going by your validation notes, and you care whether an unattended install lands somewhere workable. That is the exact judgement the read-only path wants (sysknife "show me failed services and disk usage" plans and previews and executes nothing). Entirely optional, and no bearing on this merge.

[vladimirrott]

Merged as 8988a19, closing #337.

The sentence I did not ask for and would have missed: you wrote down how to get the old behaviour back, SYSKNIFE_LLM_PROVIDER=openai. A default that moves under people needs exactly that line, and most PRs leave it for the first bug report to discover.

That closes #337 entirely, findings 1, 3 and 4 in #394 and finding 2 here.

Workspace at 5673d20:

$ cargo nextest run --workspace --locked
     Summary [   8.585s] 1851 tests run: 1851 passed, 6 skipped

#387 is still open on your side, the no-secrets pattern gate. Nobody else is on it.