Skip to content

fix(fleet): route AgentProfile pins through custom providers#4262

Merged
Hmbown merged 3 commits into
mainfrom
codex/v0868-fix-3965
Jul 9, 2026
Merged

fix(fleet): route AgentProfile pins through custom providers#4262
Hmbown merged 3 commits into
mainfrom
codex/v0868-fix-3965

Conversation

@Hmbown

@Hmbown Hmbown commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #3965 by making AgentProfile / Fleet profiles the canonical surface for per-child provider routing, including user-named OpenAI-compatible custom providers such as lm-studio.

This intentionally does not merge stale PR #3969 as-is. That PR adds a separate [subagents.routes] config surface that would compete with the post-fleet redesign AgentProfile model. Refs #3969 by @heyparth1 for the original use-case report and alternate design; this PR implements the routing outcome on the current canonical surface rather than harvesting that branch.

Behavior

  • Profile provider = "lm-studio" (or any safe user-named id) is preserved as a raw string through:
    • interactive agent(profile: ...) in-process spawn
    • Fleet worker launch --provider
    • codewhale exec --provider validation
  • Built-in providers continue to work unchanged.
  • Unknown / unconfigured provider ids fail closed with a clear error — no silent inherit and no silent DeepSeek fallback.
  • Offline Fleet route receipts omit custom providers when there is no session Config snapshot (they need [providers.<id>] resolution), rather than fabricating DeepSeek details.

Docs

  • docs/SUBAGENTS.md: per-profile provider routes example (LM Studio)
  • docs/CONFIGURATION.md, docs/FLEET.md, docs/PROVIDERS.md: named custom provider + AgentProfile pin guidance

Testing

  • cargo fmt --all -- --check
  • cargo test -p codewhale-tui --locked custom_provider -- --nocapture (20 passed)
  • cargo test -p codewhale-tui --locked fleet_worker_launch_route -- --nocapture
  • cargo test -p codewhale-tui --locked worker_command_threads_custom_profile_provider_name -- --nocapture
  • cargo test -p codewhale-tui --locked agent_profile_loader_accepts_custom_provider_name -- --nocapture
  • cargo test -p codewhale-tui --locked exec_provider_override_accepts_configured_custom_provider -- --nocapture
  • ./scripts/release/check-versions.sh (workspace=0.8.67, lockfile in sync)
  • git diff --check
  • cargo clippy --workspace --all-targets --all-features (CI)
  • cargo test --workspace --all-features (CI)

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Verified TUI behavior manually if UI changes (config/runtime path; no new UI widgets)
  • Harvested/co-authored credit uses a GitHub numeric noreply address (N/A — original work on AgentProfile surface; not a harvest of Add per-sub-agent provider routing #3969)

Allow AgentProfile/Fleet provider pins to preserve user-named
OpenAI-compatible custom providers (e.g. provider = "lm-studio") and
thread that raw id through interactive agent(profile: ...), Fleet
worker --provider launch, and codewhale exec --provider validation.
Unknown or unconfigured provider ids fail closed instead of silently
inheriting or falling back to DeepSeek. AgentProfile remains the
canonical routing surface; do not introduce [subagents.routes].

Fixes #3965

Refs #3969 (stale alternate surface held; not merged as-is)
Copilot AI review requested due to automatic review settings July 8, 2026 23:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Claude finished @Hmbown's task in 3m 43s —— View job


Review: route AgentProfile pins through custom providers

Tasks

  • Gather context (PR body, changed files, repo guidance)
  • Read AGENTS.md / CLAUDE.md for repo conventions
  • Review crates/tui/src/fleet/profile.rs
  • Review crates/tui/src/fleet/executor.rs
  • Review crates/tui/src/fleet/worker_runtime.rs
  • Review crates/tui/src/main.rs
  • Review crates/tui/src/tools/subagent/mod.rs and tests
  • Review docs changes
  • Check provider/model routing safety invariant (EPIC v0.8.65 EPIC: Separate provider facts, model facts, offerings, and route resolution #2608)
  • Post final review summary

Note: I could not run cargo check/cargo test in this sandbox (tool approval required for cargo invocations), so this is a static read-through, not a build-verified review.

Summary

The EPIC #2608 invariant is preserved throughout: provider selection stays explicit-config-only everywhere touched (validate_agent_profile_provider, explicit_fleet_provider_id, client_for_provider_id, apply_exec_provider_override) — nothing infers a provider from a model prefix/substring. The fail-closed behavior is consistent and well-tested: unconfigured custom providers error out rather than silently falling back to DeepSeek or the session provider, both in codewhale exec --provider (crates/tui/src/main.rs:656) and the in-process Fleet child-spawn path (crates/tui/src/tools/subagent/mod.rs:1598). The offline receipt path (resolve_fleet_route) correctly omits a route for unresolvable custom providers instead of fabricating DeepSeek details, matching the new resolve_fleet_route_omits_custom_provider_without_config_snapshot test.

Minor findings (reuse/simplification, not correctness bugs):

  1. validate_agent_profile_provider (crates/tui/src/fleet/profile.rs:293-305) reimplements the exact trim/empty/char-set check that validate_agent_profile_token (crates/tui/src/fleet/profile.rs:259-271) already provides and is used for id/role_hint. Could call validate_agent_profile_token(path, "provider", value) instead of duplicating the logic — the only reason not to is the custom error message text ("provider must be a simple provider id" vs "... must be a simple token"), which one existing test (agent_profile_loader_rejects_malformed_provider_name) asserts on.

  2. explicit_fleet_provider (crates/tui/src/fleet/worker_runtime.rs:467) is now dead code — its only remaining reference is a doc-comment link (crates/tui/src/tools/subagent/mod.rs:4189), not a call. All real call sites were migrated to explicit_fleet_provider_id. The module's blanket #![allow(dead_code)] hides this from the compiler, so it won't fail CI, but it's an orphaned function + a stale doc link that should probably point at explicit_fleet_provider_id now. Worth deleting explicit_fleet_provider and fixing the doc link, unless it's intentionally kept as a public-ish helper for a near-future caller.

Both are small and non-blocking. No correctness, provider-safety, or security issues found in the diff; test coverage for the new custom-provider paths (profile load, worker exec argv, launch-route resolver, receipt omission, --provider CLI validation, in-process child client) is thorough and mirrors the existing built-in-provider test shapes.

broken_workspace_dir_degrades_to_built_ins_and_config used
provider = "not-a-real-provider", which is now a valid simple custom
provider token under #3965. Switch the fixture to a malformed id so
workspace-dir load still fails closed and degrades to built-ins +
config.
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Claude finished @Hmbown's task in 1m 11s —— View job


Review: route AgentProfile pins through custom providers (update)

Tasks

  • Gather context (new commits since last review, diff vs origin/main)
  • Review changed files for correctness, provider/model safety, reuse, tests, security
  • Post final review summary

Summary

This update adds one new commit (5dbe6d0) since my last pass, which only touches crates/tui/src/fleet/roster.rs:412-436. It's a correct, minimal fixup:

broken_workspace_dir_degrades_to_built_ins_and_config previously asserted that provider = "not-a-real-provider" fails to load a workspace profile. Under this PR, validate_agent_profile_provider (crates/tui/src/fleet/profile.rs:293-305) now accepts any string matching is_agent_profile_token_char (crates/tui/src/fleet/profile.rs:330-332: ascii-alphanumeric, -, _, .) as a valid custom provider id — so "not-a-real-provider" would now pass validation and no longer exercise the "broken workspace dir degrades" path the test is named for. Switching the fixture to "not a real provider" (with spaces) restores the intent: spaces aren't in the allowed token charset, so validate_agent_profile_token's trimmed != value || !chars().all(is_agent_profile_token_char) check still fails closed, and the test continues to assert the workspace degrades to built-ins + config. Verified the char-set logic directly — this is correct and doesn't weaken coverage.

No correctness, provider/model routing-safety, or security issues in this delta. The rest of the diff is unchanged from my previous pass (provider selection remains explicit-config-only everywhere; unconfigured custom providers still fail closed rather than silently falling back).

Still-open minor items from the previous review (not addressed in this commit, both non-blocking):

  1. validate_agent_profile_provider (crates/tui/src/fleet/profile.rs:293-305) still duplicates the trim/empty/char-set logic in validate_agent_profile_token (crates/tui/src/fleet/profile.rs:259-271) rather than delegating to it, apparently only to keep a distinct error message wording that one test asserts on.
  2. explicit_fleet_provider (crates/tui/src/fleet/worker_runtime.rs:467) is still unused dead code (only referenced from a doc-comment link at crates/tui/src/tools/subagent/mod.rs:4189); #![allow(dead_code)] on the module hides it from the compiler.

Neither blocks merge.

Ensure each emit is durable before the next sequential emit or test
read observes the file. Unflushed tokio File buffers can leave only one
line visible under CI load (jsonl_sink_creates_parent_dir_and_appends_events).
@claude

claude Bot commented Jul 8, 2026

Copy link
Copy Markdown

Claude finished @Hmbown's task in 1m 59s —— View job


Review: route AgentProfile pins through custom providers (update 2)

Tasks

  • Diff against previous review pass (5dbe6d0e6c5587)
  • Review the new commit for correctness / safety / scope
  • Post final review summary

Summary

One new commit since my last pass: e6c5587fix(hooks): flush JsonlHookSink writes before drop, touching only crates/hooks/src/lib.rs:171-173.

Correctness: correct and minimal. tokio::fs::File's AsyncWrite impl queues writes and completes the underlying syscall via a background blocking task rather than guaranteeing it's done when write_all().await returns; Drop doesn't await that completion either. In jsonl_sink_creates_parent_dir_and_appends_events (crates/hooks/src/lib.rs:359-388), two sequential sink.emit(...) calls followed immediately by a synchronous std::fs::read_to_string could race with an in-flight background write from the prior emit's dropped File handle, occasionally losing a line under CI load — matching the commit message exactly. Adding file.flush().await before the function returns forces that pending write to complete first, which closes the race. No panics, no unwrap/expect on new fallible paths, error is propagated via .context(...) consistent with the rest of the function.

Scope note: this commit is unrelated to the PR's stated purpose (AgentProfile/custom-provider routing) — it's a hooks-crate flakiness fix, likely bundled in to unblock CI on this branch. Not a problem, just flagging it's orthogonal to the routing changes for anyone auditing the diff by topic.

Provider/model/route safety: no change in this delta; the explicit-config-only invariant (EPIC #2608) reviewed in the previous two passes is untouched.

Everything else (profile.rs, worker_runtime.rs, main.rs, subagent/mod.rs, roster.rs, docs) is unchanged since the previous update — see prior comments in this thread. Still-open, non-blocking items from before:

  1. validate_agent_profile_provider (crates/tui/src/fleet/profile.rs:293-305) still duplicates validate_agent_profile_token's trim/empty/char-set logic rather than delegating, apparently only to keep a distinct error message wording asserted by one test.
  2. explicit_fleet_provider (crates/tui/src/fleet/worker_runtime.rs:467) is still unused dead code (only referenced from a doc-comment link), hidden from the compiler by the module's #![allow(dead_code)].

No correctness, provider-safety, or security issues found. Nothing here blocks merge.

@Hmbown Hmbown merged commit ac4248d into main Jul 9, 2026
18 checks passed
Hmbown added a commit that referenced this pull request Jul 9, 2026
Implement parent --disallowed-tools inheritance into sub-agents:

- SpawnRequest carries disallowed_tools + inherit_disallowed_tools (default true)
- WorkerRuntimeProfile gains denied_tools with derive_child() union semantics
- SubAgentToolRegistry applies deny filtering in is_tool_allowed(),
  tools_for_model(), and execute() — restricted tools never reach the model
- Wildcard matching: exact, prefix *, case-insensitive
- Fleet worker_runtime threads disallowed_tools into WorkerRuntimeProfile

22 new tests: inheritance flows down, opt-out works, two-generation
inheritance, wildcards, case-insensitive, deny-wins-over-allow, catalog
filtering, execution gates, background runtime propagation.

Harvested from PR #4096 by @JayBeest (rebased onto current main; the
original PR was dirty against main after #4262 landed).

Refs #4042

Signed-off-by: Hunter B <hmbown@gmail.com>
Co-authored-by: JayBeest <67948678+JayBeest@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Per-sub-agent provider assignment (explicit routing) + LM Studio support

2 participants