') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix(doctor): render optional provider credentials neutrally (#319) by jrob5756 · Pull Request #322 · microsoft/conductor · GitHub
Skip to content

fix(doctor): render optional provider credentials neutrally (#319) - #322

Merged
Jason Robert (jrob5756) merged 2 commits into
mainfrom
fix/319-doctor-copilot-optional-creds
Jul 21, 2026
Merged

fix(doctor): render optional provider credentials neutrally (#319)#322
Jason Robert (jrob5756) merged 2 commits into
mainfrom
fix/319-doctor-copilot-optional-creds

Conversation

@jrob5756

@jrob5756Jason Robert (jrob5756) commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Fixes#319.

Problem

The default (offline) conductor doctor output made the copilot provider look unconfigured/not-ready even when fully authenticated. Every credential env var rendered as a red ✗ with an empty Notes column, so new users reasonably read it as "copilot is broken." In reality copilot authenticates via the GitHub/Copilot CLI login on disk, so those env vars are optional overrides — a fact documented only in a code comment that never reached the rendered report.

Fix

Combines options (1) and (2) from the issue — the principled version:

  1. Model credential optionality per provider. New _CredentialSpec (env_vars / optional / note) replaces the bare env-var tuple map in diagnostics.py. ProviderDiagnostic gains a credentials_optional field (serialized in --json).
  2. Render optional creds neutrally. In doctor.py, an absent credential for an optional-auth provider renders as a neutral dim ○ (not red ✗), and the provider's auth-path note surfaces in the Notes column. Present creds still show ✓; genuinely required creds (claude / direct Anthropic API) keep the ✗ so real misconfigurations stay visible.

claude-agent-sdk gets the same treatment for parity — it delegates to the claude CLI (claude login), so its ANTHROPIC_API_KEY is likewise an optional override.

Before / After (no credential env vars set)

Before — every copilot row is red ✗, Notes empty (looks broken):

│ copilot │ ✓ │ stable │ ✗ GITHUB_TOKEN ... │ — │

After — neutral ○ + explanatory note:

│ copilot │ ✓ │ stable │ ○ GITHUB_TOKEN ... │ authenticates via GitHub/Copilot CLI login; env vars are optional overrides │
│ claude │ ✓ │ stable │ ✗ ANTHROPIC_API_KEY ... │ — │

Changes

  • src/conductor/providers/diagnostics.py — _CredentialSpec, _CREDENTIAL_SPECS, ProviderDiagnostic.credentials_optional (+ JSON key), auth notes
  • src/conductor/cli/doctor.py — neutral ○ marker for absent optional credentials
  • docs/cli-reference.md — document optional vs required credentials and the --check readiness hint
  • tests — diagnostics optionality/note + JSON key; render ○ vs ✗

Verification

  • make format / make lint — clean
  • make typecheck — clean (1 pre-existing unrelated dialog_evaluator.py warning)
  • tests/test_cli/ + tests/test_providers/test_diagnostics.py — 511 passed, 3 skipped
  • Eyeballed live conductor doctor providers with all credential env vars unset

Note / secret safety

Unchanged: only credential env-var presence is ever reported — values are never read or printed.

Jason Robertand others added 2 commits July 20, 2026 21:46
Jason Robert
fix(doctor): render optional provider credentials neutrally (#319)
6e936f7
The offline `conductor doctor` view rendered every absent credential env
var as a red ✗, so copilot looked unconfigured even though it
authenticates via the GitHub/Copilot CLI login on disk. New users read the
all-✗ credentials cell as "copilot is broken."
Model credential optionality per provider (`_CredentialSpec`) and expose
`ProviderDiagnostic.credentials_optional`. Absent *optional* credentials now
render as a neutral dim ○ (not red ✗) with an explanatory note in the Notes
column ("authenticates via ... CLI login; env vars optional"). Required
credentials (claude / direct Anthropic API) keep the ✗ so real
misconfigurations stay visible. claude-agent-sdk gets the same treatment for
parity — it authenticates via `claude login`, so its ANTHROPIC_API_KEY is
likewise an optional override.
- diagnostics.py: `_CredentialSpec` (env_vars/optional/note),
`_CREDENTIAL_SPECS`, `credentials_optional` field + JSON key, auth note
- doctor.py: neutral ○ marker for an absent optional credential
- docs/cli-reference.md: document optional vs required credentials + --check
- tests: diagnostics optionality/note; render ○ vs ✗
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Jason Robert
address code-review feedback on#319fix
f40dd35
Recommendations:
- Collapse _CredentialSpec's `optional: bool` + `note: str | None` into a
single `optional_auth_note: str | None` field so the pairing can't drift
apart; `optional` becomes a derived property. Makes the illegal
"optional=True, note=None" state unrepresentable.
- Add a test for the null-object fallback (_CREDENTIAL_SPECS.get(name,
_CredentialSpec())) with an unregistered provider name — never raises.
- Add an end-to-end `doctor --json` test asserting `credentials_optional`
survives the full report -> to_dict -> print_json round trip.
- Generalize the docs' `--check` caveat: the offline view never validates
credential *values* for any provider, not just CLI-login ones.
Nits:
- Resolve the provider's _CredentialSpec once in gather_provider and pass
it into _credential_env_vars, instead of looking the name up twice.
- Add a one-line comment explaining why "not yet implemented" always wins
over a provider's own credential note.
- Add the missing `openai-agents` row to the credential-detection table.
- Tighten the substring assertion for the auth note to the exact string.
- Add a symmetric to_dict() test for a required provider's
credentials_optional: False.
Verified: ruff format/lint clean, ty typecheck clean (1 pre-existing
unrelated warning in dialog_evaluator.py), 1241 passed / 11 skipped across
tests/test_cli + tests/test_providers, and re-eyeballed the live
`conductor doctor providers` render (unchanged from before this cleanup).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jrob5756
Jason Robert (jrob5756) merged commit 0785c9b into mainJul 21, 2026
10 checks passed
@jrob5756
Jason Robert (jrob5756) deleted the fix/319-doctor-copilot-optional-creds branch July 21, 2026 15:25
@jrob5756Jason Robert (jrob5756) mentioned this pull request Jul 21, 2026
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

conductor doctor: offline view misleadingly shows copilot as unconfigured (all credential env vars ✗)

1 participant

@jrob5756