Skip to content

Should "what is configured" and "what is in use" be the same question? #491

Open

Description

@pjdoland

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) checks is_acp_mode and is_claude_code_mode before 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_rows when 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:

  1. Nothing. The two instances are fixed; treat them as unrelated bugs and handle the next one when it arrives.
  2. Name the distinction. A short note in AGENTS.md or CONTRIBUTING.md saying that configured-provider fields do not describe the active path, and pointing at perf_backend_label and run_readiness as the pattern. Cheapest thing that would make the next instance visible in review.
  3. Give it one place to live. Something like NBIConfig.active_chat_provider / active_inline_completion_provider that 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, since using_github_copilot_service is in the capabilities response and I would not want to change its meaning quietly.
  4. 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    questionFurther information is requested

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions