feat: real local ACP engine + per-harness model override - #1
Closed
sanil-23 wants to merge 23 commits into
Closed
Conversation
GET .../workflows/runs now returns a WorkflowRunsPage envelope
({ runs, hasMore }) instead of a bare array, ordered newest-first by
the run's actual settle time (seq/atMillis), with beforeSeq-based
cursor pagination so a page can be fetched further back without an
unbounded read. RunHistoryPanel gains a "Load older" affordance gated
on hasMore.
Every existing call site and test that read the runs endpoint as a
bare array needed updating to the new envelope shape.
Closes tinyhumansai#1012
…i#1245) Named harnesses (tinyhumansai#993) let a teammate bind to an `acp` harness, but no engine existed to actually run that turn -- lanes::build unconditionally recorded every acp harness `unavailable`. This wires a real one for `transport = "local"`, plus a `model` field so a power user can pin a specific model on it, mirroring the pattern block/buzz already ships (a teammate carries harness + model as independent settings, and the host injects the model into the harness's own startup lever). - `AcpHarness.model: Option<String>` (src/company/types.rs) + validation (manifest.rs): a plain string hint forwarded to the agent's own lever, not a credential, so it does not join `[harness.inference]`'s prohibition on acp harnesses. Rejected on `transport = "runner"` (no wire protocol for it yet) and when empty. - The `AcpAgent`/`AcpAgentFactory`/`AcpTurn`/`AcpUpdate` port moved from `harness::acp::run_turn` (behind the `openhuman` feature) to `src/ports/acp.rs`, ungated. The desktop shell -- the only implementation this crate does not itself provide -- does not enable `openhuman` on its `opencompany` dependency at all, so the port had to live somewhere it could actually see without pulling in the whole embedded-engine dependency tree. `harness::acp::run_turn` re-exports the types and keeps `AcpRunTurn`/`fold`, which do need `openhuman`'s `TurnStep`/`RunTurn`. - `lanes::build` resolves a real engine for `transport = "local"` when given a factory (`Option<&dyn AcpAgentFactory>`, `#[cfg(feature = "acp")]` with an uninhabited-type fallback for `openhuman`-without-`acp` builds); `transport = "runner"` still resolves `unavailable` (its own, larger piece of work). - `LocalAcpAgent`/`LocalAcpAgentFactory` (src-tauri/src/acp/local_agent.rs): spawns the harness's CLI via the existing `AcpClient`, demultiplexes ACP's single global `session/update` stream by session id (one subprocess serves every teammate on the harness), and injects the model via a per-CLI env var confirmed live against the real adapter -- `ANTHROPIC_MODEL` for claude, `GOOSE_MODEL` for goose. `codex` has no confirmed lever yet (validated but not injected, rather than guessed). V1 fails closed on ACP permission requests rather than routing them through the company's approval-policy gate -- a known, documented gap, not the intended end state. - `AppState::with_acp_agents` (src/app/types.rs) threads the factory to `desktop::register`, mirroring `with_rebuilder`'s exact pattern; wired for real in src-tauri/src/embedded.rs. - Found and fixed a real bug via live testing: discovery.rs's catalog still named the legacy `claude-code-acp` binary; the current package installs `claude-agent-acp`. Would have silently failed every spawn on a current install. Live-tested against a real, authenticated claude-agent-acp (not just the scripted fixture): a real prompt/response round trip, `session/new` advertising a model config option, `ANTHROPIC_MODEL` actually steering the reported current model, and the full `LocalAcpAgent` path through the `AcpAgent` trait -- see src-tauri/tests/acp_live_smoke.rs (`#[ignore]`d, costs real usage, never runs in CI). Co-Authored-By: Claude <noreply@anthropic.com>
tinyhumansai#1196) Resolves the tinyhumansai#1106 park-and-ask for the specific tie tinyhumansai#1196 reports: a global baseline teammate (globals/agents/) plausibly fits alongside a role the company staffed itself. The company has already expressed a preference by staffing that role, so the baseline candidate is dropped and the card dispatches instead of parking. A tie between two baseline teammates, or between two company teammates, is untouched — tinyhumansai#1106's park-and-ask stands. Carries Agent::global into TeammateBrief so the planner's roster prompt also distinguishes a baseline teammate ("— from the shared baseline"). Co-Authored-By: Claude <noreply@anthropic.com>
…nyhumansai#1196) Addresses CodeRabbit review on tinyhumansai#1246: direct coverage that a global teammate's prompt line carries "— from the shared baseline" and a company-authored teammate's does not. Co-Authored-By: Claude <noreply@anthropic.com>
…yhumansai#1245) Verified live rather than left as a documented gap. Tried four candidate startup env vars against a real codex-acp (OPENAI_MODEL, CODEX_MODEL, MODEL, OPENAI_DEFAULT_MODEL) -- none moved the reported current model off its default. codex-acp does advertise a real configOptions entry with category "model" though, and session/set_config_option against it, right after session/new, does work -- confirmed live, and that it's per-session state (a second, independent session reverts to the adapter's default). LocalAcpAgent::session_for now tries that fallback whenever this build has no known startup env var for the agent (model_env_var returned None) and a model was requested -- not codex-specific in the code, so any future agent in the same position gets it for free. Guarded so it never fires redundantly when an env var already carried the model at spawn. New: a deterministic (non-live, runs in CI) test pinning the configId lookup against codex-acp's real captured response shape, plus a live test proving the full LocalAcpAgent path completes through the fallback without error against the real adapter. Co-Authored-By: Claude <noreply@anthropic.com>
…inyhumansai#1260) A server can require OAuth and still be undrivable from this console. Slack's MCP endpoint answers 401 with a proper resource-metadata challenge and advertises no `registration_endpoint`, so `begin` has no client to mint and refuses -- correctly, but only once the operator has pressed a button that could never work, and the refusal is the only place the real remedy appears. The probe could not say so because `oauth_required` covered both states. It answers one question, "did this server ask for OAuth", and the console reads it as the answer to a different one, "can we complete a sign-in". Those come apart exactly when dynamic client registration is missing. `select_auth_server` already decides this -- authorize + token + registration + authorization_code -- inside `begin`, at click time. `supports_console_oauth` reuses it rather than restating the rule, so the question the probe asks and the one `begin` enforces cannot drift. The refinement sits in `probe_server`, not in `classify_mcp_error`, which is pure and synchronous by design and holds only the resource-metadata URL, never the fetched authorization-server document. It runs on the `OauthRequired` arm alone, so a healthy server pays nothing: by then this server has already answered 401. Bounded at 5s, because a metadata endpoint that accepts a connection and then hangs would otherwise stall the probe. Every uncertain answer is `true`. Discovery is a live call that fails for reasons that say nothing about capability, and answering `false` on a timeout would replace a working Sign in button with "paste a token" on a server that signs in perfectly well -- worse than the behaviour being fixed. Unsure means leave it as it was. Both credential messages also stop naming Connections, which carries no MCP server row at all. An instruction that cannot be followed reads as a broken feature rather than a missing token. Gated on `mcp` with a no-op fallback: a build without it has no `oauth/start` route, so there is no button to withdraw. Part of tinyhumansai#1260
…ansai#1260) Which control a row offers is now a function of the host's hint and nothing else, stated once in `credentialAffordance` rather than as two inline conditions. `sign_in` for `oauth_required`; `add_token` for the new `static_token_required` and for a plain `credential_required`, which wanted the same field and simply never had a button to withdraw. The inline field is the half that had to come with it. Following the old error was impossible: the only Token input on the page belongs to the *add* form, so an operator told to paste a token for an existing server would have created a second copy of it. The host has accepted a credential rotation on `PUT .../mcp/servers/{name}` all along -- `token`, `authKind`, `headerName`, `paramName` -- and the control was simply never built. Per-row, write-only, Enter to save and Escape to close. It re-tests on success rather than trusting the write, because the operator cannot know whether the token was the right one until the server answers, and a silent save would leave the amber badge sitting there with no way to tell a wrong token from an unsaved one. Verified against a live host: a junk token saves, re-probes, and stays `needs_config` rather than turning green, and the value appears nowhere in the server list response. A no-auth server grows no credential control at all -- the easy way to get this wrong is to sprout a token prompt on every row. Closes tinyhumansai#1260
…y-pagination # Conflicts: # src/server/ops/workflows.rs
The console's "ask for a reason before closing" guard never fired on any declared ledger: the server sent StatusSpec::needs_reason as snake_case while the frontend's LedgerStatus.needsReason expected camelCase, so it always read undefined. StatusSpec itself must stay snake_case in both directions — it's also what a declared ledger round-trips through in every store backend, so an asymmetric rename there would silently drop the flag back to false on save-then-reload (verified this against closing_without_a_reason_is_a_400_that_says_so, which broke under that approach). Instead, add a wire-only LedgerStatusDto in src/server/ops/ledgers.rs with #[serde(rename_all = "camelCase")], and have LedgerSummary.statuses build through it. Closes tinyhumansai#1266
The tinyhumansai#1189 stranded-approvals tests merged in from upstream still indexed the run history response as a bare array (body[0][...]), a leftover from before tinyhumansai#1012 wrapped it in { runs, hasMore }. Point them at body["runs"][0] so they assert against the actual response shape.
The `openhuman,tinycortex` CI lane builds without the `mcp` feature, so refine_oauth_capability's real body (the only place that constructs this variant) compiles out in favor of its no-op stub. Match the existing cfg_attr(not(feature = "..."), allow(dead_code)) idiom used elsewhere in this file's siblings (paypal.rs, rpc.rs, workflows.rs) rather than widening the feature gate.
…mansai#1196) Addresses CodeRabbit review: prefer_company_over_baseline classified any non-baseline candidate id as company-side, so a desk (which resolves to neither) could count as "company" and wrongly trigger dropping a genuine baseline teammate from a tie the company never actually resolved. Replaced the boolean classifier with a three-way Provenance (Baseline / Company / neither) so a desk stays neutral on both sides — it neither triggers the drop nor is dropped by it. Co-Authored-By: Claude <noreply@anthropic.com>
The route stub for GET …/workflows/runs still fulfilled with a bare JSON array. Since tinyhumansai#1012 wrapped that response as { runs, hasMore }, the console's fetch never found `data.runs` and the list never rendered, timing out both geometry specs in this file (CI: Console E2E and Console E2E (live brain)).
…dcr-hint fix(mcp): stop offering a Sign in button where OAuth cannot complete (tinyhumansai#1260)
…ason-case-mismatch fix: send ledger status needsReason as camelCase on the wire
…tiebreak-precedence fix(harness): baseline teammate steps aside for a company-authored tie
…-agent (tinyhumansai#1245) LocalAcpAgent previously failed closed on every session/request_permission call it wasn't explicitly configured to allow. Replace that with AutoApprovingFiles, mirroring buzz-agent's handle_permission_request: pick the allow_once-kind option the CLI offered, falling back to reject_once/ reject_always, never a hardcoded optionId. The CLI's own permission mode is the trust boundary, same as running it interactively. Co-Authored-By: Claude <noreply@anthropic.com>
…ory-pagination workflows: run history ordered by finish time, cursor pagination, bounded read
…lt-harness-kind-gap fix: a lone default acp harness no longer silently runs on the embedded engine
Owner
Author
|
Superseded — fix/1244 merged upstream (tinyhumansai#1247), so this now targets main directly: tinyhumansai#1280 |
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
3 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
Stacked on tinyhumansai#1247 (targets
tinyhumansai/opencompany) — this PR is opened against my own fork'sfix/1244-acp-default-harness-kind-gapbranch, since tinyhumansai#1245's work depends on that fix and the base branch doesn't exist upstream yet. Once tinyhumansai#1247 merges totinyhumansai/opencompany:main, I'll retarget this one there.Named harnesses (tinyhumansai#993) let a teammate bind to an
acpharness, but no engine existed to actually run that turn —lanes::buildunconditionally recorded everyacpharnessunavailable. This wires a real one fortransport = "local", plus amodelfield so a power user can pin a specific model on it — mirroring the pattern block/buzz already ships: a teammate carries harness + model as independent settings, and the host injects the model into the harness's own startup lever.Changes
AcpHarness.model: Option<String>(src/company/types.rs) + validation (manifest.rs) — a plain string hint forwarded to the agent's own lever, not a credential, so it does not join[harness.inference]'s prohibition onacpharnesses. Rejected ontransport = "runner"(no wire protocol for it yet) and when empty.AcpAgent/AcpAgentFactory/AcpTurn/AcpUpdateport moved fromharness::acp::run_turn(behind theopenhumanfeature) tosrc/ports/acp.rs, ungated. Found this the hard way: the desktop shell — the only implementation this crate does not itself provide — does not enableopenhumanon itsopencompanydependency at all (deliberately, to avoid pulling the embedded-engine dependency tree into every desktop build), so the port had to live somewhere it could actually see.harness::acp::run_turnre-exports the types and keepsAcpRunTurn/fold, which genuinely do needopenhuman'sTurnStep/RunTurn.lanes::buildresolves a real engine fortransport = "local"when given a factory (Option<&dyn AcpAgentFactory>,#[cfg(feature = "acp")]with an uninhabited-type fallback soopenhuman-without-acpbuilds still compile).transport = "runner"still resolvesunavailable— its own, larger piece of work (a socket wire protocol).LocalAcpAgent/LocalAcpAgentFactory(src-tauri/src/acp/local_agent.rs): spawns the harness's CLI via the existingAcpClient, demultiplexes ACP's single globalsession/updatestream by session id (one subprocess serves every teammate bound to the harness), and injects the model via a per-CLI env var — confirmed live against the real adapter, not guessed:ANTHROPIC_MODELforclaude,GOOSE_MODELforgoose.codexhas no confirmed startup-model lever yet —modelvalidates but isn't injected for it, rather than guessing wrong and silently spawning a process that ignores the setting.session/request_permission) rather than routing them through the company'sApprovalRequestQueue— flagged explicitly in the module docs andharnesses.mdas a known gap, not the intended end state. Safe direction to be wrong in: a refused edit is visible and actionable; a silent auto-approval would not be.AppState::with_acp_agents(src/app/types.rs) threads the factory todesktop::register, mirroringwith_rebuilder's exact existing pattern; wired for real insrc-tauri/src/embedded.rs.discovery.rs's catalog still named the legacyclaude-code-acpbinary. The current package (@agentclientprotocol/claude-agent-acp) installs a binary literally namedclaude-agent-acp. Would have silently failed every spawn on a current install.Live testing
Validated against a real, authenticated
claude-agent-acp(not just the scripted fixtureacp_client.rsalready covers):session/newactually advertising a model-categoryconfigOptionsentryANTHROPIC_MODELset on the spawned process actually steering the reported current model (ran it twice under two different values, confirmed they differ)LocalAcpAgentpath, through theAcpAgenttrait — the same seamlanes::buildcalls in production — including per-agent workspace directory creationSee
src-tauri/tests/acp_live_smoke.rs.#[ignore]d and never selected by CI (needs real credentials, costs real usage); run explicitly:API Or Behavior Changes
New, additive manifest field (
[harness.acp].model); new, additiveAppStatebuilder method. No behavior change for any company that doesn't setmodelor isn't running on the desktop app. Desktop companies bound to a lone/defaultacpharness now actually run turns on it (previously alwaysunavailable— or, before tinyhumansai#1244's fix, silently ran the embedded engine instead).Tests
cargo fmt --all -- --check(both crates)cargo clippy --features acp --all-targets/cargo clippy --all-targets(desktop) — cleancargo test --features acp --lib— 4691 passedcargo test --features openhuman --lib— 4657 passedcargo test --lib(desktop, fixture-based) — clean, no regressionsacp_live_smoke.rs,--ignored) — 4/4 passed against realclaude-agent-acpNew tests: manifest validation table test for
model; the live suite above.Documentation
docs/spec/runtime/harnesses.mdupdated: new "Model" section, corrected theRunnerDispatch/AcpAgentclaim (it doesn't actually implement the port — that was aspirational even before this PR), updated "A harness with no engine fails the turn" and "What a harness does not decide" (the permission-request gap), and the implementation map.