fix(*): keep migration notices off stdout, and stop planting defaults

[arelchan]

Summary

Three follow-ups from #341, which fixed a retired contextWindowTokens: 65536 capping every upgraded install at 64k. Two are its loose ends; the third is the reason it happened at all.

The notice no longer prints on stdout. #341 drains config-migration notices in the run() wrapper so every command tells the user, which caught the two whose stdout is a machine-readable document: raven doctor --json and raven import --json, both advertised for automation. Reproduced -- a config still carrying the retired pin makes doctor --json emit JSON with a sentence after it, and jq refuses the result. Moved to stderr, where the rest of this class of message already goes (the ConfigReadError branch a few lines up in the same wrapper, and the loader's malformed-config warning). That fixes any future --json command without it having to know this exists, which allow-listing the two would not. The window is narrow -- the migration fires once per install -- but the run it can spoil is a first run after upgrading, and a CI job is exactly the caller not watching.

raven doctor gains a Config section and a --fix. The premise changed while building it: the migrations run at load, so by the time this command looks, nothing is pending -- they already happened, silently, one launch ago. What is left for a person to ask about is what Raven deliberately will not decide for them. So it reports a contextWindowTokens pinned below what the configured model holds, and names what rides on it: the per-turn history budget, when the Curator starts paying for a slow path, and when memory consolidation archives. --fix removes the pin.

Reported without --fix, never written: that pin is a real configuration for a local model or an endpoint served smaller than the catalogue thinks, and the flag is the user saying which case theirs is. The write carries the file mode across (config.json holds providers.*.apiKey) and uses a per-PID temp name -- the same rules the loader's migration writer follows.

One check rather than two. A config naming no provider is the other thing worth asking about, but it reads differently on either side of the explicit-provider rule in #284, so it belongs to that change; a model that routes to nothing is already reported above it.

The bootstrap stops writing every default into the user's config. This is the cause of the bug #341 cured. save_config dumped the whole Config(), so a new install started life with eight kilobytes of settings its owner had never heard of and, worse, with that day's defaults frozen into their file -- after which a default we improve never reaches anyone who already has one. contextWindowTokens: 65536 got onto disk exactly this way.

It now writes only what differs from the defaults. Lossless on reload, since a value equal to its default reloads as that default. A fresh config drops from 7,985 bytes to 2, and to 634 once the extension blocks are seeded -- what is left is the handful of lines someone actually chose, which is also a config they can read.

That third one is the only change here with a broad blast radius, and it has one visible consequence, which is an improvement: the onboarding picker used to lead with the schema's default model, because the bootstrap had written it into the config and the wizard read it back as "the current model" -- a value nobody had chosen, offered as though somebody had. With nothing written, the wizard leads with the provider's own recommendation instead. The test that pinned the old list now pins the new one and says why.

Type

• Fix
• Feature
• Docs
• CI / tooling
• Refactor
• Other

Verification

pytest tests/                       6650 passed, 1 failed, 33 skipped
                                    (tests/test_cli_theme.py::test_bold_accent_renders_styled_not_bare,
                                    which fails identically on github/main when its file runs
                                    together and passes alone on both)
ruff check raven/ tests/            All checks passed
ruff format                         clean

Driven by hand as well as by tests, each against a throwaway home:

doctor --json on a fossil config   stdout parses as JSON again; the notice is on stderr
doctor    (pin 65536, model 1M)    reports the pin and what is sized against it
doctor --fix                       pin removed, file still 0600, a second run reports nothing
save_config on a fresh Config()    2 bytes; 634 after the extension blocks are seeded

New cases cover the stderr split, both doctor paths including the mode preservation, and the save round-trip keeping every value the user chose.

• Relevant tests pass locally
• Relevant lint / type checks pass locally
• User-facing docs or screenshots are updated when needed

Risk

The bootstrap change alters what every new install's config file looks like: nearly empty instead of a full dump. Existing configs are untouched -- nothing rewrites them -- and both shapes load identically, so the risk is confined to anyone reading config.json as documentation of what can be set. raven doctor and the schema remain the answer to that question.

The onboarding picker's default moves from the schema default to the provider's recommendation, as described above.

doctor --fix writes only when asked, and only removes a key. Everything else here is output routing.

Rollback: revert the commits. Nothing persists a new format, so a reverted build reads every config written under this one.

Related Issues

N/A

[claude]

Claude finished @arelchan's task in 4m 36s —— View job

---

Review complete

• Read existing comments/reviews on this PR
• Review diff against origin/main
• Check correctness (bugs, edge cases, error handling, back-compat)
• Check repo conventions (commits, branch naming, uv, test naming, CONTEXT-MAP)
• Check tests for weakening/skipping
• Post final findings

Blocking: the network-call-in-a-zero-network-path issue flagged in the earlier review on this PR is still unresolved. The latest commit (9617194, pushed after that review) only adds test coverage and mocks resolve_context_window in every new test — it doesn't pass allow_fetch=False at raven/cli/doctor_commands.py:187, so plain raven doctor / raven doctor --json on a config with an explicit contextWindowTokens pin against a model LiteLLM's static table doesn't know (the exact case this feature targets — self-hosted/custom endpoints) still falls through to a live OpenRouter HTTP call, contradicting this module's documented "zero-network, millisecond-fast" default mode. Not re-filing it as a new inline comment since it's already on the thread, but it stands as-is.

Everything else I checked is solid:

• save_config's move to exclude_defaults=True is safe — confirmed no other caller (onboard_commands.py, tui_rpc/methods/config.py) depends on a full dump; the extension-block seeding uses its own setdefault-based raw-JSON writer, untouched by this change.
• The stderr move for migration notices reuses the file's existing Console(stderr=True) pattern (already used a few lines up) and is well-tested (stdout/stderr split, no-notice case).
• _inspect_config_health / --fix / _write_config_preserving_mode: the raw-dict pop, mode-preservation, and per-PID temp file all check out against the loader's established rules; the read-only-home and unstat-able-mode failure paths are exercised and degrade correctly (report-and-continue, never crash).
• Commit messages are Conventional-Commits compliant, ASCII-only, with Co-authored-by trailers; branch name matches <type>/<snake_desc>; all test edits are on pre-existing files (test_cli_doctor_commands.py, test_cli_helpers.py, test_config_loader.py, test_cli_onboard_commands.py), no new test files. No large/asset files added.
• No test was weakened to go green — the onboarding default-model assertion changed because the underlying wizard behavior legitimately changed (documented in the commit body), and it still asserts a specific list/default rather than loosening to something permissive.

One new nit posted inline (dead/untested branch in the Config-section renderer).
· branch fix/config_notices_and_doctor_fix

[claude]

[blocker] This breaks the module's own "zero-network" contract for plain raven doctor (see the docstring at the top of this file: "Default mode is zero-network, millisecond-fast"; _gather_static_checks is explicitly "Strictly zero-network").

resolve_context_window(model) defaults to allow_fetch=True, and unlike resolve_max_output_tokens (used a few lines up in _gather_static_checks, which only ever consults LiteLLM's static table), resolve_context_window falls through to _lookup_openrouter_entry → _fetch_openrouter_models, which does a live httpx GET against OpenRouter (10s timeout) whenever the model isn't in LiteLLM's static table and no fresh disk/in-process cache exists.

_inspect_config_health is called unconditionally in register() whenever report.config_loaded is true — not gated behind --probe — so any config with an explicit contextWindowTokens pin now makes plain raven doctor (and raven doctor --json, the automation path this PR is otherwise protecting) attempt a network call. Worse, the case this feature is built for — a self-hosted/local model or custom endpoint "served smaller than the catalogue thinks" — is exactly the kind of model id LiteLLM's static table won't have, so it's the most likely to hit the network fallback. In a sandboxed/no-egress CI runner this adds a multi-second stall to what's documented as a millisecond-fast, offline check before silently degrading.

Every other synchronous/latency-sensitive caller in the repo passes allow_fetch=False for this reason (e.g. raven/agent/loop/main.py:369,738,1048). Suggest the same here: resolve_context_window(model, allow_fetch=False).

[gloryfromca]

Confirming this independently -- it reproduces, and the failure is not only the theoretical contract break.

With agents.defaults.model = "openrouter/acme/nonexistent-model-xyz" and contextWindowTokens: 32768 in a throwaway home, _inspect_config_health(load_config(cfg), fix=False) with httpx.Client swapped for a recorder:

network calls attempted: [('GET', 'https://openrouter.ai/api/v1/models')]

So plain raven doctor -- no --probe -- reaches _fetch_openrouter_models, and that client is built with timeout=10.0. Offline or behind a firewall, the documented millisecond-fast default mode now blocks for up to ten seconds, and --json is the flag advertised for CI.

Reachable whenever the pin is set and the LiteLLM tier misses, which is exactly the self-hosted / newer-than-the-table case the finding text is written for. Not every model: _lookup_openrouter_entry returns early unless the id starts with openrouter/.

Worth adding for whoever fixes it: allow_fetch=False costs nothing in coverage here. It also turns off the LiteLLM import, but _gather_static_checks has already called resolve_max_output_tokens(defaults.model) by this point, so LiteLLM is in sys.modules and _try_litellm_context_window(allow_import=False) proceeds exactly as it would with True. The only tier lost is the one that should not be running.

[claude]

[nit] line.startswith(" ") (and the else branch that strips/re-indents it) has no producer: every finding built by _inspect_config_health — the pin message and f"could not write the fix: {exc}" — is a plain, unindented string, so this branch is dead and untested. Either drop the conditional or wire an actual indented-finding case if one is coming.

[gloryfromca]

Blocking: the zero-network break already flagged at doctor_commands.py:187 reproduces -- plain raven doctor issues a live GET to openrouter.ai. Repro and a note on why the fix is free are in that thread. Everything else here I would merge as it stands.

Covered: the full diff against origin/main, the callers of what it changes (save_config's only real caller, read_raw_or_raise, the loader's own migration writer, tui_rpc.methods.config), backward compatibility of the on-disk config, AGENTS.md conventions, and the tests for weakening.

What I checked and am satisfied with

• save_config(exclude_defaults=True): the blast radius really is one caller. onboard_commands._bootstrap_empty_config is the only one, and only when the file is absent; every other config write goes through the surgical update_* helpers. The round trip is lossless in the way that matters -- the schema's only model_fields_set consumer is should_warn_deprecated_memory_window, and memory_window is Field(exclude=True), so it never survived a dump anyway. tui_rpc reads raw keys and falls back to its own _DEFAULTS, but its keys (agent.temperature, tui.*) are not schema paths, so a thinner file changes nothing there either.
• The onboarding picker change is a genuine behaviour change, and the test was repinned with a reason rather than loosened. anthropic/claude-sonnet-5 is the registry's own default_model for that provider (providers/registry.py:322), so leading with it is the right answer, not an accident of the diff.
• The stderr move follows the pattern already in the file (_helpers.py:311 writes the config-path notice to stderr), and the two new cases assert both directions -- stderr carries it, stdout is empty.
• Conventions: commits are conventional, ASCII-clean, and carry Co-authored-by with a real model id; the branch name fits section 2.1; no new test files, and the touched ones are the right three per section 5.4.
• Tests, run here rather than taken on faith: tests/test_cli_doctor_commands.py tests/test_cli_helpers.py tests/test_config_loader.py tests/test_cli_onboard_commands.py -> 360 passed, 0 skipped. The other four files that touch save_config (status / agent / gateway / cli_config_precedence) -> 59 passed. Nothing skipped, xfailed, or env-gated.

I also agree with the dead-branch nit at line 582: _inspect_config_health is the only producer of findings, and neither of its two strings is indented, so that else cannot fire.

Two nits of my own inline. And one forward-looking note that is not a finding: a user who legitimately pins a smaller window now gets the ! on every raven doctor run with no way to say "yes, I meant it". Acceptable while the section holds one check, but if it grows, an acknowledge path will be wanted before the section starts being ignored.

[gloryfromca]

[nit] Unlike the loader's writer this says it mirrors, this one does not unlink the temp file when the write fails. That one ends except OSError: ... tmp.unlink(missing_ok=True) (loader.py:258-261); here nothing cleans up.

Concrete: on ENOSPC or a quota, tmp.write_text creates the file, fails partway, and the caller's except Exception turns it into a "could not write the fix" finding -- leaving ~/.raven/config.json.doctorfix.<pid> behind holding a partial copy of the config. The mode is the part that stings: chmod runs after the write, so the orphan keeps the default umask (0644) while containing providers.*.apiKey, sitting next to a config.json the user may have tightened to 0600 -- which is the exact exposure the docstring above exists to prevent. A failing os.replace leaves the same thing, fully written.

tmp.unlink(missing_ok=True) in an except OSError: that re-raises closes it, and makes the "same rule the loader's migration writer follows" claim true.

[gloryfromca]

[nit] This makes a sibling comment false. _persist_migrations still justifies itself with "save_config is no use here either -- it dumps every default (~8 KB), re-planting the very kind of fossil this migration exists to pull out" (lines 224-226), and after this change it dumps no defaults at all.

The paragraph's other reason still holds and is now the whole justification: the mapping load_config migrated has the extension blocks popped, and a config that needed no edit must stay byte for byte. So this is a one-line trim, not a rethink. Left as is, the next reader takes a present-tense comment as documenting current behaviour and walks away with the wrong picture of what save_config writes -- the same drift the previous commit in this branch fixed for the watermark comment.