Repository navigation
fix(llc): refuse to hire an adapter that cannot run here (#12681) - #14797
Conversation
Hiring already rejected an unregistered adapter_type, and the comment on that check names the failure it prevents: an agent whose every heartbeat is skipped, looking degraded forever. A registered type whose CLI is missing produces exactly that symptom one level down — the type is in the registry, the binary is not on the service PATH, and the hire succeeded anyway. Nothing new was needed. is_cli_available() and cli_not_found_message() already exist on the subprocess base, and GET /adapters already reports `available` from them. The hire path simply never asked, so this wires the existing probe in rather than adding a second definition of the same question. A crashing probe fails CLOSED, matching GET /adapters, which reports available=False when the probe raises. Failing open would let a hire succeed for an adapter the UI is simultaneously showing as unavailable. An adapter with no CLI has no probe and is never gated. test_haiku_tier's SQL-bind test now states adapter availability explicitly: it calls hire_agent, CI has no `claude` binary, and that test is not about availability.
…12681) (#14790) The backend image installs claude and gh from signed apt repos and proves both resolve at build time. Nothing did the equivalent on an ansible-provisioned host: searching the ansible tree for claude returned zero files. resolve_cli_binary therefore returned None for every claude_code / copilot_local agent hired on a systemd deployment — the hire succeeded, dispatch could not, and the failure surfaced as a skipped run rather than a provisioning error. Same signed repos as the image, fetched with get_url rather than piped into a shell, plus a resolve check on the host so a broken PATH fails provisioning instead of coming up hosting agents that cannot run. The tasks sit before the #12139 restart-guarded window: they reach two third-party repos, and inside that block a transient outage would abort the whole deploy after the service was stopped. gh is included because copilot_local_adapter resolves it the same way; installing only claude would leave that adapter equally unrunnable. The guard asserts the provisioning rather than the resolver, and every assertion was mutation-tested — removing gh from the package list, deleting the verify task and dropping claude from its loop each turn exactly one assertion red. Refs #12681 — item 1 of three. Item 3 shipped as #14761; item 2 is #14797.
✅ SSOT Configuration Compliance: Passing🎉 No hardcoded values detected that have SSOT config equivalents! |
… runnable (#12681) The gate treated the absence of is_cli_available as "in-process, nothing to check". Among registered adapters that is never what it means: codex_subscription is registered, has no probe, and its invoke raises NotImplementedError. It was the one registered type without the probe, and the gate let it be hired. GET /adapters computes `implemented = hasattr(adapter, "is_cli_available")` and `available = implemented`, so the UI greys codex_subscription out as unavailable while the hire succeeded — the two-surfaces-disagree defect this gate was written to prevent, reproduced by the gate itself. Both now decide from the same hasattr. Verified against a replica: previously the stub was unavailable in the UI and hireable; now both refuse it, and the four subprocess adapters are unaffected. The test used a hand-written stand-in for "an adapter with no probe", which is exactly why this slipped. It now also runs against the real CodexSubscriptionAdapter.
HIGH fixed in
|
…about it (#12681) The gate broke two existing tests, and my earlier survey missed them: I grepped for direct `hire_agent(` calls and found two, but these reach the endpoint over HTTP through a TestClient, which that grep could not see. test_budget_provision (both hire cases) and test_agent_hires_auth's authorized case POST to agent-hires and expect 201. CI has no `claude`/`gh` binary, so the probe refuses and they got 422. They are about budget provisioning and tenant auth, so they now state availability rather than letting the probe decide it. Three other hire POSTs deliberately keep no patch, because they never reach the gate: two expect 404 from the tenant check that runs before it, and one expects 422 from the registration check the gate sits after.
…ring (#14800) (#14808) _import_agent inserted into agent_org_nodes with an adapter_type taken from an imported template, asking neither of the questions the hire route asks. A template naming an unknown adapter, or a known one whose CLI this host lacks, created an agent whose every heartbeat is skipped. The gate added in #14797 therefore covered one of the two paths that create runnable agents; it now covers both. Skipped with a warning rather than failing the import: a template may legitimately be imported onto a host provisioned afterwards, and the response already carries warnings and skipped. Only a declared adapter_type is checked. Review caught that resolving a claude_code default here was wrong twice over: the value checked was not the value persisted, and the scheduler reads a missing adapter_type as autobot_agent, which runs in-process and is deliberately absent from the registry. Defaulting to claude_code spuriously skipped every ordinary in-process agent in a template re-imported onto a CLI-less host. The gate now reads the same expression the INSERT binds. adapter_unavailable_reason moved from the hire route to llc/adapters/base.py beside the registry functions, so a service can ask the question without importing an API module and there is one definition rather than two. Closes #14800. Refs #12681. Dangling reports_to on a skipped manager: #14811.
1 participant
Refs #12681 — item 2 of the three. Not
Closes; see the sequencing note.Note
Prerequisite satisfied. This must not have landed before #14790, because
rejecting a hire while nothing installed the CLIs would have made provisioning
strictly worse. #14790 merged as
2e1457ff1and is verified on base — ansiblenow installs
claudeandghand proves they resolve — so the orderingconstraint is met and this is free to merge on its own checks.
Thinking Path
agent_hires.pyalready rejects an unregisteredadapter_type, and the comment onthat check states exactly what it prevents:
A registered type whose CLI is absent produces the identical symptom one level
down.
claude_codeis in the registry on every deployment; theclaudebinary isnot.
resolve_cli_binaryreturns None, dispatch skips, and the agent looks degradedforever — the very outcome the existing check was written to prevent.
The useful discovery was that nothing new was needed:
subprocess_base.py:172already implementsis_cli_available()subprocess_base.py:186already implementscli_not_found_message(), anactionable message
llc/api/adapters.py:36-43already reportsavailableper adapter type fromexactly those
So the availability question was already asked and answered in one place. The hire
path just never asked it. This wires the existing probe in rather than adding a
second definition that could drift from the first.
What Changed
_adapter_unavailable_reason(adapter_type)— returns the reason, or None.hire_agentrejects with 422 and the actionable message when the adapter cannotrun here.
test_haiku_tier's SQL-bind test now states adapter availability explicitly. Itdrives
hire_agent, CI has noclaudebinary, and that test is not aboutavailability — leaving it to the real probe would have made it fail for an
unrelated reason.
A crashing probe fails CLOSED, matching
GET /adapters(except Exception: available = False). I had it failing open first; that would let a hire succeed foran adapter the UI is simultaneously showing as unavailable — two surfaces
disagreeing about the same question, which is the defect class this issue is full
of.
An adapter with no CLI exposes no probe and is never gated.
Verification
Logic confirmed against a replica before committing:
Six tests. Four cover the reason helper; two drive
hire_agentitself, because thedefect was the missing call and not a wrong answer — a helper-only suite would pass
while the route never consulted it.
The refusal test asserts
session.execute/session.commitare not awaited, soa rejected hire cannot half-persist. The paired control asserts a hire with an
available CLI still reaches both — without it, a gate that rejected everything would
satisfy the first test.
What this does not fix
Item 1 is #14790 and must land first. Item 3 shipped as #14761.
Model Used
Claude Opus 5