feat(readiness): configuration preflight that names the missing piece - #410
Merged
Conversation
Enterprise deployments (internal LLM gateways, EFS-mounted homes, intercepting proxies) see much worse latency than desktop installs with no way to tell where a turn's time goes. This adds the foundation: a turn recorder and an on-demand environment probe, both inert until enabled. The recorder keys turns by message id in a locked registry rather than a ContextVar because a turn crosses three threads that share no contextvars context (tornado loop, per-request asyncio.run thread, SDK client thread). Spans carry a fixed name vocabulary and a schema-checked attribute allowlist enforced in the recorder itself, so the privacy guarantee cannot erode when a later change passes a new kwarg. In the default redacted mode, file basenames and model/tool/server names are hashed, including external tool names embedded in span names. Disabled cost is one boolean test per call. Finished turns land in a ring buffer and, optionally, a JSONL sink on a dedicated writer thread that batches, tolerates write failures, self-disables rather than blocking on the slow filesystem it may be diagnosing, and creates its directory and files 0700/0600. The probe measures the usual suspects directly: per-directory stat, read, and write+fsync latency with cold-vs-warm first iteration and a sustained-throughput pass, filesystem type and mount options, node vs CLI cold-start cost, contention signals, and an opt-in network leg that is proxy-aware, sends one request to the configured base URL only, captures the presented TLS chain (interception shows up as the issuer), and reports unauthenticated TTFB with an honest caption. Checks run on abandonable worker threads because a hung NFS stat is uninterruptible; the pool is recreated per run so abandoned workers cannot poison later probes. Output is scrubbed of home paths (including the resolved real home), usernames, and hostname fields. Config follows the repo's policy conventions: a perf_diagnostics section, NBI_PERF_DIAGNOSTICS value lock, a policy triad with force-on for fleet-wide baselining (which pins redacted mode), and a separate NBI_PERF_PROBE_NETWORK policy so admins can keep the probe while disabling its network leg.
Wires the recorder through the turn lifecycle in all three modes. The turn opens at websocket ingress (ChatRequest branch only), covers the real per-turn prep work in a context_prep span, and is force-closed in the request thread's finally so cancellation and errors cannot leak it; a guard around the pre-thread stretch closes the turn as errored if a malformed request raises before the worker starts, since one leaked turn would otherwise disable the single-open-turn tool guard for the process lifetime. Cancelled turns are recorded as cancelled, not as fast successes that poison the percentiles. first_token is an event recorded on the first content-bearing chunk; the locally generated "Thinking" spinner previously triggered it and made time-to-first-token read ~0 on every turn, exactly in the slow deployments the feature targets, while pulling the connect phase inside the stream span. Stall events start from the first SDK message rather than the loop entry (the gap before the first message is TTFT, not a stall) and are tagged with the preceding message kind so a gap after tool use is distinguishable from a gateway stall. Token counts sum cache reads and writes the same way the usage footer does, so the two surfaces agree on cached turns. Tool timing uses the three real hook points: the Copilot/base dispatch loop, the Claude in-process tool wrappers, and the ACP session-update events (those tools run in a separate process). MCP calls annotate the enclosing dispatch span with the server name instead of nesting a duplicate, which had double-counted every MCP call in the aggregates. The report endpoint serves the ring snapshot plus a probe_target reduced to scheme and host (never netloc, which preserves embedded credentials); the probe endpoint accepts only the network boolean and enforces the NBI_PERF_PROBE_NETWORK policy server-side. Both are authenticated and 404 while diagnostics is disabled. Each closed turn also logs one INFO summary line with the per-phase breakdown so headless installs get the signal from the server log alone.
A new tab in NBI Settings hosting the diagnostics controls (enabled, log to file, attribute detail), a last-N-turns table, and the probe. Controls respect both the settingLocks value-locks and the perf_diagnostics policy, so force-on and force-off render as locked rather than as a toggle that silently reverts. The first-token column reads the event mark the backend actually records instead of a span that never exists. The network check is behind an explicit inline confirmation that shows the exact URL the probe will contact, taken from the report's probe_target. Report and probe output can be copied as JSON for support tickets. The capabilities payload now carries the perf_diagnostics section; without it the panel rendered defaults and a save clobbered stored settings.
README gets the user-facing section: what a turn records, what is never recorded, the probe, the env vars. The admin guide covers the operator knobs: the policy triad including fleet-wide force-on, redirecting the JSONL log off network homes with NBI_PERF_LOG_DIR, how to read total-vs-api and the EFS signatures, the probe's SOC-relevant behavior, and the new endpoints in the HTTP API table.
…anel Follow-ups from the post-implementation review pass on the performance diagnostics feature. Recorder: - run the whole compare-dir/teardown/reassign/recreate sequence in configure() as one critical section under _sink_lock, so a concurrent TurnHandle.close() cannot recreate a sink pinned to the old log dir - recreate a self-disabled sink on the next configure(), so a settings save retries the sink after an operator fixes the filesystem - add set_current_span_attr() for callees that only annotate one attr Instrumentation: - exclude Progress chunks from every stream perf signal (span open, first_token, chunk and byte counters), not just the span open - stop running asdict + json.dumps on every stream chunk purely to count bytes; estimate from the content field and write the final counts once in finish() - attribute in-process tool spans via get_current_response() first, so concurrent turns do not fall back to the process-wide single-open-turn guess - collapse input_tokens to None only when no usage token key is present, so a payload that legitimately reports zero still records 0 - apply the perf_diagnostics value lock on the config POST write path, so a scripted POST cannot persist enabled:true under NBI_PERF_DIAGNOSTICS - log perf turn close failures at warning, not debug Probe: - submit every independent check before blocking on any result, so wall time is roughly the slowest check rather than the sum of the timeouts - give the claude_home session scan an outer budget above its own internal bound, so a scan that runs to that bound is not abandoned with its samples discarded - return partial results plus tls_error on a sub-timeout TLS handshake failure instead of discarding the dns/tcp timings - close the raw socket when the handshake never produced an SSLSocket - report process RSS as max_rss_kb, which is what ru_maxrss actually is - skip, rather than error, when the resource module is unavailable - fall back to a top-level base_url for openai-compatible providers Panel: - read the perf_diagnostics lock from the feature policy entry instead of comparing a policy object against policy-name strings, which never matched; register perf_diagnostics in the api.ts policy name list - gate the network checkbox on perf_probe_network_allowed - exclude probe_target from the copied report JSON - correct IPerfTurn.t_wall to a number and add the skipped check status Docs: describe the probe's network leg as the several connections it actually makes, and note that only the HTTP legs carry the probe User-Agent.
Second review pass on the performance diagnostics feature. - The stream byte estimator read a flat "content" key, which no chunk actually has: raw LLM chunks are OpenAI-shaped, so the stream span's "bytes" attribute and the egress event's byte count both read exactly 0 for the whole of Copilot chat mode. Read through choices[0].delta.content, and make the test fixtures use the shape the providers really emit instead of a synthetic flat dict. - The probe submitted every check up front onto a shared pool, so the filesystem latency loop timed individual stat and fsync operations while the sustained-IO check was writing a 4 MB file into the same directory. That makes the probe measure its own contention, and it is worst on the network-mounted homes the probe exists to diagnose. Run the filesystem, subprocess, and contention groups strictly one at a time, and overlap only the network check, which touches neither disk nor CPU and is the long pole anyway. - TurnHandle.close reads _log_to_file outside _sink_lock, so a configure() that turns file logging off could land between that read and the sink lookup, and the close would then build a fresh sink and leave a live writer thread behind. Re-check _log_to_file inside the locked factory. - The in-process tool span comment claimed the current-response lookup disambiguates concurrent sessions. It does not: _current_response is a module global, not a contextvar. What it does provide is the query actually running on the client thread, which is what makes it better than giving up whenever a second turn merely sits open. Say that.
The recorder and probe collected the right data, but the panel showed twelve numeric columns and a raw JSON dump, and the knowledge needed to interpret either lived in one paragraph of the admin guide. This adds the part between "the data exists" and "the user reaches a conclusion". Turns table: - a Verdict column naming the phase that dominated the turn (model or gateway, tool execution, agent cold start, context preparation, mid-stream stalls), derived from the columns already present rather than from any new recorded field - Time and Stalls columns, both from data that was already transmitted and dropped on the floor - a tooltip on every column, including what Active excludes and why Active against API ms is the comparison that matters - expandable rows showing that turn's spans as proportional bars with their attributes, then its events with offsets, and a warning when the per-turn caps truncated the record - an empty state that says to run a turn, instead of an empty table Probe: - checks are grouped and banded rather than dumped as JSON, with check ids translated into what they measure and a plain-language note on anything that trips a threshold (network filesystem, EFS burst-credit exhaustion, cgroup throttling, TLS interception, clock skew) - skipped and timed-out checks read differently from healthy ones - the raw JSON is still there, behind a toggle, since that is what gets pasted into a ticket - sub-millisecond latencies keep two decimals instead of rounding to "0 ms", which read as "not measured" rather than "fast" Two probe bugs the new rendering made visible: - macOS firmlinks: /Users is presented at /Users but lives on the Data volume, and Path.resolve does not follow the firmlink, so the mount match landed on "/" and reported every home directory as sealed read-only apfs. Re-match through /System/Volumes/Data when that mount exists. - the panel labelled the config-directory checks "config" while the probe emits "nbi_user_dir", so that group rendered with a raw id. Model name: - the Claude connect span now carries the model, which is the only place the turn header can learn it. Before this the Model column was empty on every Claude turn. - TurnHandle captures it through the same redaction the span attr gets. It was being stored in the clear while the span copy was hashed, which disagreed with itself and put the model name into reports and the JSONL sink that document it as hashed. Copy: a one-paragraph description at the top of each section, and an explanation of what redacted and full actually trade off, at the point where that choice is made. Docs: new docs/performance-diagnostics.md with the column and verdict reference, the span and event vocabulary, the probe thresholds, worked diagnoses for a slow gateway, a slow home directory, and an intercepting proxy, the JSONL schema, and what to attach to a support ticket. The README section is restructured around that and the admin guide keeps only the operator-owned knobs, pointing at the new document for the rest.
…panel Findings from a full-PR review pass, plus one the live check turned up. Probe, TLS. The network check could not detect interception, which is its headline capability. The timing leg used a verifying context, so an untrusted certificate failed that handshake and the function returned early with only a tls_error and no certificate at all; a trusted one made the second leg's identical verifying handshake succeed too. There was no input for which verified_against_default_bundle came back false, so the panel branch and the documented diagnosis were both dead code. The capture leg now deliberately does not verify (it sends nothing, and the certificate you need to see is exactly the one a verifying handshake refuses), issuer and subject are parsed from the DER because getpeercert() returns an empty dict for an unvalidated peer, and a separate verifying leg produces the verdict. Both legs, and the HTTP request, now go through the configured proxy: the verification leg used to connect directly, which in a proxy-only egress environment is refused, so the field silently stayed null in precisely the deployments that intercept. A live check against a self-signed server then showed the HTTP leg raising through and discarding every one of those findings, so its failure is reported as a field instead. Covered by a test that stands up a TLS server with an untrusted certificate. Probe, other: - the ~/.claude presence stat ran inline on the caller's thread with no timeout, so a hung mount (the condition this exists to diagnose) hung run_probe forever and burned a Jupyter executor thread - mount options were copied verbatim out of /proc/mounts, where CIFS/SMB carries addr= and username=, into a document that promises no hostnames and no credentials. Bare flags are kept, key=value only on a safe list, and the withheld count is shown rather than silently dropped - the subprocess and mount checks had an outer budget equal to their own inner timeout, so the outer timer always won and the documented slow-but-completing band could never be produced Recorder: - get_turn is gated on _enabled, so a turn in flight when diagnostics were switched off never closed and leaked in the registry for the process lifetime, which also poisons the single-open-turn lookup and silently disables tool spans from then on. Added take_turn for the close path; a turn closed after disabling is released but not recorded - the JSONL writer caught only OSError, so an unserializable document killed the thread while enqueue kept filling an unbounded queue with no consumer. Documents are now serialized individually, any failure counts toward the documented self-disable, and the thread cannot die silently - span_id and parent_id were computed on every span, at the cost of a lock acquisition, and then discarded. They are emitted now, which is what lets a reader tell a phase from the wrapper containing it - _detect_fs_type re-implemented mount resolution with a bare-prefix match and no firmlink handling, so it named the wrong filesystem for the one thing it exists to reveal. It delegates to perf_probe - SPAN_NAMES and the attr allowlist advertised names and fields no producer sets, including a gateway_host that would have put a hostname in a document promising none Instrumentation: - the stream byte counter counted characters for an attribute named bytes, undercounting CJK and emoji three to four times - stream() called get_turn per chunk, taking the global registry lock on the hottest path in the product. The handle is resolved once - the tool-dispatch loop never marked tools builtin, so every NBI tool was recorded with a hashed name in the default mode, the opposite of what the docs say. MCPTool revokes the flag for itself - the turns report embedded the gateway host, contradicting the README, the guide, and the panel's own copy. It moved to capabilities, which already carries the base URL for the settings UI Panel: - fmtMs only guarded undefined, so a null duration_api_ms rendered as "0 ms" on every non-Claude turn, in the column the docs call the most useful comparison - nested spans are indented - the network checkbox handler did not check the policy it renders - the directory-level claude_home row no longer renders a dangling colon - the Performance tab is hidden under force-off, where both endpoints 404
A misconfigured NBI fails at the far end of a chat turn, where the only thing the user sees is base_chat_participant.py's "Oops! There was a problem handling chat request. Please try again with a different prompt." That advice is actively wrong: the cause is usually an expired key, a base URL pointing at the wrong path, a CLI that is not on PATH, or a model id the endpoint no longer serves, and it goes to the server log a notebook user never reads. Four sections of docs/troubleshooting.md are the same class of diagnosis written out as manual steps, and every one of them is a predicate a machine can evaluate. GET /notebook-intelligence/readiness evaluates them and returns a verdict, a one-line headline, and a row per check. Every row that is not ok carries a remedy naming the next action; a test fails the build if one does not, because a check that can only say "something is wrong" is the generic apology with extra steps. The same document backs a Status card at the top of NBI Settings, General. Not gated on perf diagnostics, unlike the two /perf routes: a user who cannot tell "the admin has not set this up" from "I did something wrong" needs the answer whether or not diagnostics are on. The default run bills nothing. The one check that costs money is opt-in per run and asks first, because two failures are invisible to everything cheaper: a gateway that returns 200s but buffers instead of streaming, and a proxy that strips the tools field so agent mode silently never fires. It has an admin off switch (NBI_READINESS_LIVE_CHECK) and runs one at a time per server. checks.py extracts the bounded-check harness out of perf_probe so both surfaces share one copy, along with the subprocess version probe and the scrub pass. perf_probe keeps private aliases, and its 23 tests pass unchanged, which is the evidence the extraction is behavior-preserving. Fixed from the xhigh review before this commit, all reproduced first: - A signed-out GitHub Copilot user reported "Ready. Nothing needs configuring." The provider falls back to a hardcoded 12-model list when it has no token, so "the list is non-empty" was true while every turn 401s. This was the exact failure the feature exists to prevent. Copilot readiness now consults the login status. - Any first-call failure in the live probe was blamed on the tool schema, so one 503 made readiness accuse a proxy of stripping tools and flipped the verdict. The tools field is now only implicated when the retry without tools succeeds where the one with tools failed; when both fail, the tool question is reported as unanswered rather than answered no. - provider.chat_models, the Claude CLI resolve, and the ACP binary lookup all ran inline outside the harness, so a hung provider hung the request that exists to diagnose hangs. The model list is now bounded, and the helper that names the effective model takes the already-fetched list so it cannot re-trigger the fetch. - run_readiness propagated exceptions from config properties, so a corrupt config produced a 500 from the endpoint that exists to explain breakage. Each group is guarded and becomes a row. - The ACP launch command and the Claude version banner went out unscrubbed, carrying absolute home paths, the login name, and anything passed in argv into a document meant for a support ticket. - The ACP runtime check only fired for a literal "npx", so an NBI_ACP_AGENT_COMMAND override pointing at a nonexistent binary reported Ready. It now checks whatever argv[0] actually is. - provider.model_exists could never fire for the openai-compatible and litellm-compatible providers, whose single model carries a constant placeholder id, and the ok row showed that placeholder rather than the model the user configured. - The probe tool schema was a module-level mutable passed by reference into third-party SDKs; litellm and ollama pass it through unmodified, so one mutating callee would corrupt it for the process lifetime. - A first chunk at 0.0 ms rendered as "n/a" through a truthiness check. - _ProbeResponse did not subclass ChatResponse and its stream() signature was narrower than the published one, so a conforming third-party provider would raise TypeError and be misreported as a broken endpoint. - The billing POST had no admin gate and no concurrency guard. - A failed re-check left the green Ready pill and stale rows on screen next to "Readiness check failed". - The settings dialog refetched on every open and tab switch, forking `claude --version` each time. Cached for 30s. - A test mutated the process-global perf recorder without restoring it.
mbektas
approved these changes
Aug 27, 2026
Resolves conflicts with plmbr#403 (terminal-safe dispatch), plmbr#404 (bounded UI tool results), and plmbr#405 (inline edit system prompt). Four files conflicted, all where perf instrumentation sits on lines plmbr#403 rewrote. - ai_service_manager: kept the dispatch span, dropped a duplicate participant_id assignment that plmbr#403 had already moved earlier in the function. The span's turn lookup is now guarded with getattr, because plmbr#403's own dispatch tests use a ChatResponse double without message_id and diagnostics must never change dispatch behavior. - api.py: took plmbr#403's future-based wait_for_run_ui_command_response wholesale. It registers the waiter before dispatch and supports cancellation, which the polling loop it replaced did not. The ui_command span moved to run_ui_command in extension.py, which already knows the command name and now also covers the dispatch half. - extension.py stream(): plmbr#403 split it into a lifecycle-guarded wrapper plus _stream_unlocked. The perf block belongs in the inner one, so a chunk dropped because the response already finished does not open the stream span, fire first_token, or count toward egress. - extension.py _run_request_thread: merged both error paths. Upstream's cancellation exceptions now set status "cancelled" directly, its Exception handler sets "error" and streams the terminal notice, and a BaseException clause preserves the status for what neither catches. The finally calls upstream's guaranteed finish() first, then reads the handler entry before upstream's conditional pop so the perf close still gets the user-wait total and the cancel token. - extension.py request dispatch: upstream's side of that hunk was a re-indented duplicate of what had already auto-merged; the only real delta was plmbr#403's new response_emitter argument to _run_request_thread. Test updates for the new contracts, not to make failures go away: - the emitter fixture gains the lifecycle state plmbr#403 added - _run_request_thread tests pass the emitter, and register the same object in the handler dict, since the conditional pop compares identity - the exception test no longer expects a re-raise. plmbr#403 deliberately swallows and streams a terminal notice, so the guarantee worth pinning is that the turn still closes as "error" and the notice goes out once.
Brings main (via plmbr#408) up under the stacked readiness branch. No conflicts: the readiness module lives in its own files and the shared check harness in checks.py was unaffected by plmbr#403, plmbr#404, and plmbr#405. Verified rather than assumed, since readiness._ProbeResponse subclasses the ChatResponse that plmbr#403 reworked: 1645 pytest, 418 jest, tsc, eslint, stylelint, and prettier all clean.
pjdoland
added a commit
to pjdoland/notebook-intelligence
that referenced
this pull request
Aug 28, 2026
Bring the 5.4.0 changelog up to what the release will actually contain, and close the doc gaps the merged and milestoned work left behind. Changelog: add entries for the readiness preflight (plmbr#410) and performance diagnostics (plmbr#408), which merged with no changelog trace at all, and for the empty-code-block fix (plmbr#407). Carry the dispatch (plmbr#403), bounded tool result (plmbr#404), and inline-edit system prompt (plmbr#405) entries out of the section they were parked in and cite their PR numbers, matching how every other entry in the file references its change. Add entries for the three open PRs carrying the v5.4.x milestone: ask-mode history budgeting (plmbr#412), the dedicated inline chat model (plmbr#413), and the Copilot stop parameter (plmbr#397). Rewrite the release summary, which described only the two agent-surface features and predated both diagnostic surfaces. Docs: document NBI_READINESS_LIVE_CHECK in the README, which described the live check but never named its off switch. Add troubleshooting guidance for ask-mode history budgeting, including the requirement that OpenAI-compatible and LiteLLM-compatible providers carry an explicit context window before any pruning happens. Add the Performance diagnostics section to the admin guide table of contents, which was the only heading in the file missing from it. Resolve the HTTP API route table against main: keep the perf and readiness rows added by plmbr#408 and plmbr#410, add the ui-tools row, and restore rules/reload beside its siblings, where the merge had separated it.
herikwebb
added a commit
to herikwebb/notebook-intelligence
that referenced
this pull request
Aug 28, 2026
…lmbr#408/plmbr#410), 0 new findings Pass A: Python closure (121 pkgs, pip-audit) and root yarn.lock (950 pkgs, npm bulk) clean; no manifests changed in delta. Pass B: full review of 10248-line delta (perf diagnostics, readiness preflight, chat markdown) clean; 3 new authenticated handlers verified secret-safe and SSRF-free. list_files glob-escape (high) remains active but already tracked; not re-filed.
pjdoland
added a commit
to pjdoland/notebook-intelligence
that referenced
this pull request
Aug 28, 2026
Bring the 5.4.0 changelog up to what the release will actually contain, and close the doc gaps the merged and milestoned work left behind. Changelog: add entries for the readiness preflight (plmbr#410) and performance diagnostics (plmbr#408), which merged with no changelog trace at all, and for the empty-code-block fix (plmbr#407). Carry the dispatch (plmbr#403), bounded tool result (plmbr#404), and inline-edit system prompt (plmbr#405) entries out of the section they were parked in and cite their PR numbers, matching how every other entry in the file references its change. Add entries for the three open PRs carrying the v5.4.x milestone: ask-mode history budgeting (plmbr#412), the dedicated inline chat model (plmbr#413), and the Copilot stop parameter (plmbr#397). Rewrite the release summary, which described only the two agent-surface features and predated both diagnostic surfaces. Docs: document NBI_READINESS_LIVE_CHECK in the README, which described the live check but never named its off switch. Add troubleshooting guidance for ask-mode history budgeting, including the requirement that OpenAI-compatible and LiteLLM-compatible providers carry an explicit context window before any pruning happens. Add the Performance diagnostics section to the admin guide table of contents, which was the only heading in the file missing from it. Resolve the HTTP API route table against main: keep the perf and readiness rows added by plmbr#408 and plmbr#410, add the ui-tools row, and restore rules/reload beside its siblings, where the merge had separated it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A misconfigured NBI fails at the far end of a chat turn. The only thing the user sees is
base_chat_participant.py:561:That advice is actively wrong. The cause is usually an expired key, a base URL pointing at the wrong path, a CLI that is not on
PATH, or a model id the endpoint no longer serves, and the real error goes to the server log a notebook user never reads. So the user rewrites the prompt, it fails again, and they conclude the product is broken.docs/troubleshooting.mdhas eleven sections and four of them are the same class of diagnosis written out as manual steps ("no models available", "I'm getting a 401", "Claude mode hangs on Thinking...", "MCP server crashes"). Every one is a predicate a machine can evaluate. A documented troubleshooting step is a bug report against the product.What this adds
GET /notebook-intelligence/readinessruns those checks and returns a verdict (ready/degraded/not_ready), a one-line headline, and a row per check. The same document backs a Status card at the top of NBI Settings, General.Every row that is not
okcarries a remedy. Not "provider unreachable" but "usually an expired or missing API key, or a Base URL that does not point at the provider's API root; for GitHub Copilot, sign in again from NBI Settings". A test fails the build if any blocked or warning row lacks one, because a check that can only say "something is wrong" is the generic apology with extra steps.Not gated on perf diagnostics, unlike the two
/perfroutes. A user who cannot tell "the admin has not set this up" from "I did something wrong" needs the answer whether or not diagnostics are on, and an admin verifying a rollout needs it before anyone has opened the settings dialog. It reports state the authenticated user can already read from/capabilities.The default run bills nothing. Config resolution, credential presence, CLI probing, and model listing are free. One check costs money and is opt-in per run behind a confirm, because two failures are invisible to everything cheaper:
toolsfield. Agent mode and every built-in tool then silently never fire. The probe sends a trivial tool schema and retries without it; only if the retry succeeds where the first failed is the tools field implicated.It has an admin off switch (
NBI_READINESS_LIVE_CHECK=off) and runs one at a time per server.Missing credentials are a warning, not a blocker, in both agent modes: the Claude CLI and ACP agents hold their own subscription and OAuth logins that NBI cannot see, and calling that broken would be wrong.
Refactor
checks.pyextracts the bounded-check harness out ofperf_probe.pyso both surfaces share one copy, along with the subprocess version probe and the scrub pass.perf_probekeeps private aliases and its 23 tests pass unchanged, which is the evidence the extraction is behavior-preserving.Review provenance
This went through an xhigh multi-angle review before the commit; all fifteen findings are fixed in the commit rather than as follow-ups, and the reviewer reproduced most of them with scripts rather than inferring them. The two that mattered most:
not_ready. The tools field is now only implicated when the retry without tools succeeds; when both fail, the tool question is reported as unanswered rather than answered "no".Also fixed: unbounded
provider.chat_models(a hung provider hung the request that exists to diagnose hangs), a corrupt config producing a 500 from the endpoint that explains breakage, home paths and login names going out unscrubbed, an ACP override pointing at a nonexistent binary reporting Ready,provider.model_existsbeing unreachable for the two providers internal-gateway deployments actually use, a module-level mutable tool schema passed into third-party SDKs, a 0.0 ms first chunk rendering as "n/a",_ProbeResponsenot matching the publishedChatResponse.streamsignature, no admin gate or concurrency guard on the billing POST, a failed re-check leaving a green "Ready" pill on screen, and the settings dialog forkingclaude --versionon every open.One reviewer claim I checked and did not act on: a race between the mount fetch and a button. Both buttons are disabled while a run is in flight, so the UI cannot start two itself. I kept the guard, corrected its comment to say so, and dropped the test that would have implied the race was reachable.
Testing
tsc/eslint/stylelint/prettierclean.test_a_hung_model_list_is_bounded_rather_than_hanging_the_requestcaught my own first fix reintroducing the unbounded fetch through a helper.Follow-ups deliberately not in scope
not_readycase. The endpoint and the Status card are the load-bearing parts; the banner touches a 5,000-line file and belongs in its own change.