Repository navigation
Should "what is configured" and "what is in use" be the same question? #491
Description
The question
#481 and #489 are the same mistake in two places: code that reads what the configuration says and treats it as what the deployment does. Both were fixed locally. The question for you is whether that is the right resolution, or whether the distinction deserves a name and a single place to ask it.
I would rather raise this than keep patching instances, because the two we found were both reported from outside rather than caught in review, and a third would look just as ordinary.
The two instances
#489 (Copilot login). using_github_copilot_service reads the configured chat_model and inline_completion_model provider fields (config.py:143-145). Both default to github-copilot, so a Claude-mode deployment that never opened the General tab logged into Copilot, refreshed the token, and polled api.github.com for the life of the server. Fixed in #490 by asking, surface by surface, what will actually serve a request.
#481 (the local-only preset). disabled_providers gates the native provider path, and the documented HIPAA recipe treated it as gating everything. Neither agent mode is a provider: _agent_mode_enabled reads only claude_settings.enabled / acp_settings.enabled (ai_service_manager.py:441-447), and claude_mode defaults to user-choice. A deployment built exactly as documented could still send data to Anthropic. Fixed in #482, in the documentation.
The shape is identical. What differs is only which question the code happened to ask.
The codebase already knows the answer in some places
This is the part that makes me think a rule is findable rather than invented. Two surfaces already do it correctly, and neither is documented as the pattern to copy:
perf_backend_label(ai_service_manager.py:481-484) checksis_acp_modeandis_claude_code_modebefore reading the configured provider, with a comment explaining that a report naming a provider the user does not have is worse than none.run_readiness(readiness.py:903-909) branches on the mode and only reaches_provider_rowswhen neither agent mode is active, which is why it does not show a spurious "not signed in" row for Copilot in Claude mode.
So the codebase contains both the bug and its remedy, a few hundred lines apart, and nothing tells a reader which to imitate.
A case where the answer may legitimately differ
perf_probe.py:548-570 collects candidate base URLs from the ACP settings, the Claude settings, and the configured openai-compatible provider, and probes all three. For a diagnostic whose job is to measure the endpoints an operator has configured, probing what is configured rather than what is in use looks defensible rather than wrong.
I am flagging it as a case where the same question has a different defensible answer, not as a third bug. That is also why I do not think "always ask what is in use" can simply be applied everywhere.
What I am asking
Roughly in order of cost:
- Nothing. The two instances are fixed; treat them as unrelated bugs and handle the next one when it arrives.
- Name the distinction. A short note in
AGENTS.mdorCONTRIBUTING.mdsaying that configured-provider fields do not describe the active path, and pointing atperf_backend_labelandrun_readinessas the pattern. Cheapest thing that would make the next instance visible in review. - Give it one place to live. Something like
NBIConfig.active_chat_provider/active_inline_completion_providerthat resolve the mode once, so callers ask a question that cannot be answered wrongly. This is the real fix and also the one with the widest blast radius, sinceusing_github_copilot_serviceis in the capabilities response and I would not want to change its meaning quietly. - Audit for a third. I checked the obvious readers of those fields for this issue and found only the two, plus the probe above. A deliberate sweep would cost more and might find nothing.
My own inclination is 2 now and 3 only if a third instance turns up, but this is a judgement about how much structure the project wants, which is yours rather than mine.
Environment
NBI main at 6333cd6. Raised from the #489 investigation; both instances were user-reported.
Activity