Repository navigation
Honor configured agent model profiles - #47
Conversation
563994c to
cbe6cd8
Compare
|
Smithers verifier approved this PR for merge. Verified PR #47 in its worktree and approve. Branch codex/full-run-overseer sits at cbe6cd8 — the PR's single commit 'Honor configured agent model profiles' rebased cleanly onto the origin/main tip (75d8533), so it is trivially mergeable; GitHub also reports MERGEABLE (UNSTABLE only because heavy CI checks are still running, intentionally left to GitHub Actions after the workflow pushes). Diff vs origin/main is exactly the PR's 23-file scope (reasoning field through config schema/defaults/resolve/loader, topology expanded-graph schema, runtime task specs/metadata/validate/summarizeConfig, createCodexAgent factory + agentFactories registry + agentForTask wiring in the generated workflow template, docs, ultrafuzz.toml, and tests); diff vs the origin PR head (2be6cfd) contains only main's PR #46 dashboard changes, confirming the PR's own change set is preserved. Implementation reviewed and coherent: the synthesized default profile now carries model=gpt-5.5/reasoning=xhigh so the factory-based CodexAgent receives effective overrides per task spec, with null-stripping for absent modelName/reasoningEffort. Independent fast validation all passed: config tests (16), topology tests (27), runtime tests (48 pass, 1 env-skipped), CLI tests (6), workspace typecheck, eslint, plus the deterministic gate (git diff --check, prettier, prompts tests, docs check, clean git merge-tree vs origin/main). Refreshed PR comments/reviews: zero approved-author items; only greptile-apps[bot] activity, ignored per security policy. Working tree clean; no corrective changes needed. Branch is ready for the workflow to push (rebased head requires the workflow's push handling, not a fast-forward of the old origin head). GitHub checks passed for PR #47; merge state is CLEAN. |
* Qualify generated agents import to avoid file/directory shadowing (#28) Generated workflows imported the project agent registry via the bare specifier "../agents". Smithers' own project scaffolding can create .smithers/agents.ts (a generic example file) alongside the .smithers/agents/ directory ultrafuzz owns; bun's module resolution prefers the sibling file over the same-named directory, so the workflow silently loaded Smithers' unrelated demo agents instead of the generated registry. agentRegistry[task.agentRef] then resolved to undefined, which made Smithers treat the Task as a static no-op node whose "output" was its own prompt text -- surfacing as an opaque "returned plain text" schema validation error with no indication an agent never ran. Importing "../agents/index.ts" directly is unambiguous and cannot be shadowed by a sibling file, regardless of what Smithers' bootstrap scaffolds alongside it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Construct project agents per task instead of at registry import (#28) The generated registry eagerly constructed its agent at module scope (`export const CodexAgent = createCodexAgent()`). Constructing an agent reads its auth from ultrafuzz.toml, so importing the registry required every configured agent's credentials, not just the ones a run actually uses. With CodexAgent shipping `auth = "api-key"` by default, any project that did not bill through an OpenAI key still failed at import with "agents.CodexAgent auth is api-key, but OPENAI_API_KEY is not set", forcing an unrelated placeholder key just to launch. Adding a second backend would multiply that: every project would need every backend's credentials. Generated workflows resolve each task's agent through agentFactories, so the eager singleton had no remaining consumer. Dropping it makes construction lazy: a missing credential now surfaces only when that agent is actually selected, and still surfaces as the same explicit error rather than an undefined registry entry. validate.ts matches agent references against the registry text, and the factory map still names the agent, so launch-posture validation is unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Add opt-in Claude agent backend (#28) Generates a ClaudeAgent alongside CodexAgent, backed by the Claude Code CLI, so a project can run agent tasks against a Claude subscription or the Anthropic API instead of Codex. The default model profile still selects CodexAgent; Claude is opt-in through the new `claude` profile, selectable per node in topology.yml or with `ultrafuzz run --model claude`. The adapter follows the model-profile contract from #47: model selection arrives as a factory argument from the resolved profile rather than being read from `agents.ClaudeAgent`, so profile and CLI overrides reach the spawned command. Auth stays in TOML (subscription or api-key), mirroring CodexAgent. Both auth modes drive the `claude` CLI; api-key changes only how that CLI authenticates, so the CLI must be installed either way. ClaudeCodeAgent exposes no reasoning-effort option, so the factory accepts reasoningEffort for contract parity and ignores it, and a `reasoning` value on a Claude profile has no effect. Agent templates are emitted as text and are not covered by the workspace typecheck, so the generated adapters were verified against smithers-orchestrator 0.27.0 types out of band; that check rejects Codex-only options such as skipGitRepoCheck on ClaudeCodeAgent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Keep agent overrides and the agent registry honest (#28) Three defects found reviewing the Claude backend, all reachable only once a second agent exists: `--agent <ref>` set the default profile's agent but left its model, so `ultrafuzz run --agent ClaudeAgent` launched Claude with CodexAgent's `gpt-5.5` and `xhigh` reasoning. A profile's model and reasoning are chosen for its agent, so switching agents without naming a model now drops both and lets the agent's own default apply; an explicit --model still pins it. The override logic was duplicated verbatim in validate.ts and plan-run.ts and now lives once in @ultrafuzz/config alongside the other model-profile rules. The docs and shipped config told users to select Claude with `ultrafuzz run --model claude`. `--model` overrides the default profile's model *name*, it does not select a profile by id, so that command ran CodexAgent against a model called "claude". Corrected to `--agent ClaudeAgent`, with topology `model_profiles` as the per-node selector. init preserves project-owned files, so a project scaffolded before this change kept its old registry: claude.ts was written but nothing exported it, and the agent was only rejected later, at launch. init now reports a stale registry as a warning naming the missing agent, rather than leaving the mismatch silent. Regenerating with --force clears it. Also drops a docs claim that subscription auth makes cost accounting unavailable: nothing ties accounting to auth mode, and model-pricing resolves claude-* through the anthropic provider regardless. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Match the agent factory entry when checking for a stale registry (#28) The staleness check matched the agent name anywhere in the registry text, which a real project defeated: a registry left over from an earlier adapter carried `export { ClaudeAgent } from "./claude"` while agentFactories held only CodexAgent. The name matched, init stayed quiet, and the agent was still unselectable -- the exact case the warning exists to catch. Generated adapters export only their factory, so agentFactories is the sole path that resolves them; naming the agent elsewhere does not make it selectable. The check now matches the entry inside the agentFactories object literal instead of the bare name. validate.ts keeps its looser name match on purpose: a hand-registered agent exported as an instance resolves through the registry fallback, so any mention there really can satisfy an agent reference. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Decode agent TOML values with a shared TOML-aware parser (#28) The generated adapters unescaped ultrafuzz.toml values by handing them to JSON's string parser. A TOML basic string is not a JSON string: TOML defines \UXXXXXXXX, which JSON rejects. `config_dir = "vault-\U0001F600"` therefore threw an uncaught SyntaxError naming a character offset in an anonymous string. It surfaced while the workflow was rendering its agents, so it took down the whole run rather than reporting a bad config value. Values are now decoded by a TOML-aware parser that handles both \uXXXX and \UXXXXXXXX and rejects malformed, unsupported, and out-of-scalar-range escapes with an error naming the offending key, e.g. "agents.ClaudeAgent.config_dir has an unsupported escape \q". The parser is emitted once as .smithers/agents/toml.ts and imported by both adapters. Copying a corrected decoder into each would have doubled the duplication the fix exists to remove, and a third backend would triple it. Adapter-specific helpers stay local, since their messages name their agent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Document that ClaudeAgent bypasses Claude Code's permission checks (#28) permissionMode was pinned to "bypassPermissions" with nothing saying so outside the generated adapter. [agents.ClaudeAgent] exposes only auth, api_key_env, and config_dir, so a reader of ultrafuzz.toml had no way to know every task runs with unattended filesystem and shell access. Records it where a reader will actually look: in the config table, in docs/config.md, and beside the option itself. The behaviour is unchanged and still deliberate -- agents run unattended in a throwaway worktree with nobody to answer a prompt, permissions.trust_model = "skip-permissions" is the only trust model ultrafuzz accepts, and CodexAgent already runs the same way. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: jason <jason@Mac.lan> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: thankyou <>
Summary:
Validation:
Greptile Summary
This PR applies configured model profiles to generated task agents. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "Merge concurrent branch updates" | Re-trigger Greptile