diff --git a/.codecov.yml b/.codecov.yml index ed75d24ad9..567803fba4 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -18,19 +18,10 @@ coverage: ignore: - "**/*_test.go" - "**/testdata/**" - - "e2e/behaviour/suite_test.go" - - "e2e/behaviour/features/**" - - "e2e/behaviour/fixtures/**" - # Integration-only; covered by behaviour/e2e CI jobs (not unit coverage upload). - - "pkg/behaviourtest/steps/dispatch.go" - - "pkg/behaviourtest/steps/dispatch_count.go" - - "pkg/behaviourtest/steps/dummy_agent.go" - - "pkg/behaviourtest/steps/fork.go" - - "pkg/behaviourtest/steps/jirapoll.go" - - "pkg/behaviourtest/steps/registry.go" - - "pkg/behaviourtest/steps/triage.go" - - "pkg/behaviourtest/suite/init.go" - - "pkg/behaviourtest/drivers/install/perrepo_github.go" + # Behaviour-test harness and scenarios: integration-only, exercised by the + # behaviour/e2e CI jobs against real repos (not by the unit coverage upload). + - "e2e/behaviour/**" + - "pkg/behaviourtest/**" - "pkg/e2etest/cleanup.go" - "pkg/e2etest/build.go" - "docs/**" diff --git a/.github/workflows/reusable-dispatch.yml b/.github/workflows/reusable-dispatch.yml index 62b072bd4e..9f882a5b80 100644 --- a/.github/workflows/reusable-dispatch.yml +++ b/.github/workflows/reusable-dispatch.yml @@ -457,9 +457,11 @@ jobs: echo "::error::config.yaml: disabled agent entry without a name field — add 'name:' to identify which agent to disable" exit 1 fi - BAD_SOURCE=$(yq '.agents[] | select(type == "!!map" and .enabled != false and (.source == null or .source == "")) | line' .fullsend/config.yaml 2>/dev/null || echo "") + # An enabled entry may omit source when it only tunes a built-in + # agent by name (runtime/model/effort; ADR 0091). + BAD_SOURCE=$(yq '.agents[] | select(type == "!!map" and .enabled != false and (.source == null or .source == "") and (.runtime == null and .model == null and .effort == null)) | line' .fullsend/config.yaml 2>/dev/null || echo "") if [[ -n "$BAD_SOURCE" ]]; then - echo "::error::config.yaml: enabled agent entry without a source field" + echo "::error::config.yaml: enabled agent entry without a source field (an entry with only a name must set runtime, model or effort to tune a built-in agent)" exit 1 fi DUPES_ENABLED=$(yq '.agents[] | select(type == "!!map" and .enabled != false) | (.name // "") | select(. != "") | downcase' .fullsend/config.yaml 2>/dev/null | sort | uniq -d) diff --git a/Makefile b/Makefile index 7d1d673285..726052ed30 100644 --- a/Makefile +++ b/Makefile @@ -207,8 +207,17 @@ test: lint-all go-test script-test lint-eval-cases e2e-test: go test -tags e2e -v -count=1 -timeout 30m ./e2e/admin/ +# Capabilities the runner declares for @requires:capability: scenarios. +# Declared here rather than in the e2e workflow so a PR that adds a gated +# scenario exercises it on its own CI run (E2E Tests runs on +# pull_request_target, whose workflow file comes from main). runtime-pi: +# fullsend-sandbox:latest ships pi since v0.37.0; each pi scenario costs one +# small haiku run on the pool repo's Vertex WIF. Override to skip them: +# BEHAVIOUR_CAPABILITIES= make behaviour-test +BEHAVIOUR_CAPABILITIES ?= runtime-pi + behaviour-test: - go test -tags behaviour -race -v -count=1 -timeout 45m ./e2e/behaviour/ + BEHAVIOUR_CAPABILITIES="$(BEHAVIOUR_CAPABILITIES)" go test -tags behaviour -race -v -count=1 -timeout 45m ./e2e/behaviour/ # Functional agent evals — run agents against ephemeral GitHub repos and judge results. # Required env: EVAL_ORG (GitHub org for ephemeral repos), plus GCP creds for Vertex AI. diff --git a/docs/ADRs/0091-per-agent-runtime-model-effort.md b/docs/ADRs/0091-per-agent-runtime-model-effort.md new file mode 100644 index 0000000000..9dab0c4af8 --- /dev/null +++ b/docs/ADRs/0091-per-agent-runtime-model-effort.md @@ -0,0 +1,190 @@ +--- +title: "91. Per-agent runtime, model and effort on agents: entries" +status: Accepted +relates_to: + - agent-architecture +topics: + - config + - harness + - runtime +--- + +# 91. Per-agent runtime, model and effort on agents: entries + +Date: 2026-08-25 + +## Status + +Accepted + +## Context + +`.fullsend/config.yaml` could not express per-agent `model`, `runtime`, or +`effort`. A repo that needed different agents on different models or +runtimes had to set GitHub repository variables (`TRIAGE_FULLSEND_MODEL`, +`CODE_FULLSEND_RUNTIME`, ...), which live outside the repository: no review, +no git history, no diff, and nothing in the repo records the intent. + +The per-role `model:` field in the harness is per-agent by construction, +but repos consuming remote harnesses cannot reach it. The `validModelName` +regex (`^[a-zA-Z0-9_.@-]+$`) also prevented provider-qualified model +identifiers (e.g. `xai-vertex/xai/grok-4.6`) in harness files, forcing +repos on pi to use repository variables as the only path. + +`runtime:` was a single repo-wide key (ADR 0033), yet the case that +motivates this record is precisely a repo whose agents run on different +runtimes: fullsend-ai/pi-xai-vertex runs triage and review on pi (Grok) +and code, fix and retro on Claude Code. + +Related issues: #6529 (per-role model + effort, scoped out runtime), +#6570 (validModelName `/` restriction), #6577 (alias resolution bug, +independent). #6529 and #6570 are folded into this record deliberately: +a per-agent model field that cannot name `provider/id` would not serve +the pi case that motivates it, so the schema and the validation rule are +one decision, not two. + +## Options + +**A separate `role_overrides:` map** (the first cut of the implementing +PR, #6583) — keyed by agent name, with `runtime`/`model`/`effort` per key. +Rejected before merge: `config.yaml` already has a per-agent place — the +`agents:` list (ADR 0058) — keyed by the same names with the same layered +merge, so a second per-agent section would have split "what runs" from +"how it runs" and carried a misleading name (it was keyed by agent name, +not harness role, because `code` and `fix` share `role: coder`). + +**Fields on the `agents:` entry** — chosen; see Decision. + +## Decision + +### 1. `runtime`, `model`, `effort` on `agents:` entries + +An `agents:` entry may set the three fields. A built-in agent is tuned +with a **name-only entry** (no `source:`); a custom agent carries them on +its `source:` entry: + +```yaml +runtime: pi # repo default for agents that set none +agents: + - name: triage + model: xai-vertex/xai/grok-4.6 + - name: code + runtime: claude + model: sonnet + effort: high + - source: https://raw.githubusercontent.com/acme/agents//harness/lint.yaml#sha256= + model: haiku # custom agent "lint" (name derived from the file, ADR 0058) +``` + +Names are **agent names** as passed to `fullsend run ` (triage, +code, review, fix, retro, prioritize) or a custom entry's name, matched +case-insensitively. They are NOT harness `role:` values — `code` and +`fix` both carry `role: coder`, while the existing role-prefixed +repository variables (`CODE_FULLSEND_MODEL`, `FIX_FULLSEND_MODEL`) already +key on agent name. + +An enabled entry without `source:` must name a built-in agent and set at +least one field; anything else is rejected (`coder` gets a "did you mean +`code`?" hint). Such an *override-only* entry registers no harness: the +built-in keeps resolving through the agents-repo fallback, and it is not +enumerated as a custom harness, locked, or listed as one. + +This narrows ADR 0045's "config.yaml does not gain deep-merge +capabilities or per-agent override entries": that decision kept an +agent's *definition* out of `config.yaml`, and it still holds — these +fields tune three operational knobs of an already resolved harness; they +do not define or compose one. Anything beyond runtime/model/effort still +belongs in a harness (`base` composition). + +`fullsend agent set [--runtime] [--model] [--effort]` writes the +entry so the file need not be edited by hand. + +### 2. Precedence + +Config-layer addition, slotting in below per-run overrides: + +``` +--runtime/--model/--effort flag + > FULLSEND_* env (including role-prefixed repository variables) + > the agent's agents: entry + > repo-wide runtime: / harness model: effort: + > default +``` + +The repo-wide `runtime:` key is kept as the default for agents that set +none — deliberately not removed: existing configs, `fullsend github setup +--runtime` and the behaviour-test installs depend on it, and a default is +still the right shape for a repo where every agent runs the same way. +Per-agent is now the primary place to select a runtime; the docs steer +there. + +Entries merge per field across the layered config (ADR 0069): the +overlay's non-empty value wins, an empty value inherits the base's; there +is no tombstone to unset a base value short of restating the entry. + +Plan output, stderr `runtime: selected ...` lines, and `metrics.json` +(`runtime_source`, `override_source`) name the source as +` agents.` (the effective config file). + +`fullsend run` does not call `Validate()` on the config it loads, so it +validates the effective `agents:` list itself on every run — names, +runtime against `ValidRuntimes()` (a stub runtime cannot be activated +through an entry any more than through `runtime:`), effort against the +shared levels, model against the shared model-reference syntax — in the +overlay and in `config.base.yaml` alike, and fails the run with an error +naming the file and entry rather than silently running without the +settings. Runtime selection and the model/effort application read one +loaded config, so an entry's three fields always agree on their source. + +The effective entries, with these fields, are exposed to overlay CEL +expressions as `config.agents` (ADR 0088). + +### 3. Segment-based model validation + +A shared `ValidModelRef` regex (`^[a-zA-Z0-9_.@-]+(/[a-zA-Z0-9_.@-]+)*$`) +replaces the harness-local `validModelName`. This is a superset of the +previous rule: existing single-segment model names continue to validate, +and `provider/id` forms are now accepted in both harness `model:` fields +and `agents:` entry `model:` values. Malformed forms (`/leading`, +`trailing/`, `a//b`) are rejected. + +The effort level list moves to `config.ValidEffortLevels()` for the same +reason: `harness` imports `config`, so the shared lists live in `config` +and both validators read one source of truth. + +### 4. Deferred: `models.aliases` + +Provider-qualified per-agent models remove most of the need for the +`models.aliases` map proposed in #6529. Alias resolution is tracked +separately in #6577. This ADR does not introduce an alias system. + +### 5. Scope: per-repo first + +The fields are read wherever `agents:` entries are (org-mode configs +included, since the list is shared), but the CLI (`agent set`), the setup +PR text and the docs target per-repo installs; per-org installation mode +is deprecated (ADR 0044). + +## Consequences + +- Repos express per-agent runtime, model and effort in one reviewable, + version-controlled list that also says which harness runs — one place + per agent. +- Repository variables remain valid for one-off experiments and for + repos that prefer out-of-band configuration. +- The harness `model:` field now accepts `/` in model identifiers, + unblocking provider-qualified models in harness files. +- Because `config.yaml` now carries durable per-agent intent, a + `fullsend github setup` re-run keeps an existing per-repo `config.yaml` + untouched unless a flag targets a config key, and then changes only + that key; managed workflow files still refresh. The converge + full-rescaffold repair (missing workflow) and the GitLab setup path keep + regenerating the file as before. +- The reusable workflow's `config.yaml` guard allows an enabled entry + without `source:` when it sets a field. A repo must bump its pin to a + version that carries this change before adding such an entry: an older + pinned workflow rejects it for every agent, whereas a separate unknown + key would have been ignored. The docs say so. +- `review` and `retro` can be put on another runtime through their + entries but are documented to stay on Claude Code today; this is not + enforced in validation. diff --git a/docs/architecture.md b/docs/architecture.md index 96628f51ca..6f61f3ae8f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -212,7 +212,7 @@ flowchart TB **Decided (implementation):** -- The `fullsend run` runner delegates in-sandbox agent execution to a `runtime.Runtime` interface; production orgs default to Claude Code, with [pi](https://github.com/earendil-works/pi) available as an opt-in second runtime (`runtime: pi`, Claude-on-Vertex through the same WIF credential path). Runtime selection is configured in `defaults.runtime` on the org `config.yaml` and resolved via `runtime.ResolveFromConfig()`. A **dummy** runtime executes scripted operations in the real OpenShell sandbox for behaviour tests (inference removed). Bootstrap uses a portable `BootstrapInput` interface with optional extensions such as `SandboxHooksBootstrap` for the runtime-neutral sandbox tool hooks ([ADR 0090](ADRs/0090-runtime-neutral-sandbox-hooks-contract.md)); runtimes declare further capabilities through small optional interfaces (`DebugLogNamer`, `ContextBridger`) rather than `Name()` checks in the runner. Transcript and debug artifact handling use a separate `TranscriptHandler` interface. See [runtimes.md](runtimes.md) for the per-runtime security feature matrix required when adding a new backend. +- The `fullsend run` runner delegates in-sandbox agent execution to a `runtime.Runtime` interface; production orgs default to Claude Code, with [pi](https://github.com/earendil-works/pi) available as an opt-in second runtime (`runtime: pi`, Claude-on-Vertex through the same WIF credential path). Runtime selection is configured per repo with `runtime:` in `.fullsend/config.yaml` (per-agent `runtime`/`model`/`effort` on the agent's `agents:` entry sit above it and below the `--runtime`/`--model`/`--effort` flags and `FULLSEND_*` variables, [ADR 0091](ADRs/0091-per-agent-runtime-model-effort.md)) and resolved via `runtime.ResolveForAgent()`; org-mode configs use `defaults.runtime` and `runtime.ResolveFromConfig()`. A **dummy** runtime executes scripted operations in the real OpenShell sandbox for behaviour tests (inference removed). Bootstrap uses a portable `BootstrapInput` interface with optional extensions such as `SandboxHooksBootstrap` for the runtime-neutral sandbox tool hooks ([ADR 0090](ADRs/0090-runtime-neutral-sandbox-hooks-contract.md)); runtimes declare further capabilities through small optional interfaces (`DebugLogNamer`, `ContextBridger`) rather than `Name()` checks in the runner. Transcript and debug artifact handling use a separate `TranscriptHandler` interface. See [runtimes.md](runtimes.md) for the per-runtime security feature matrix required when adding a new backend. ### Behaviour testing @@ -383,7 +383,7 @@ the inheritance model: fullsend defaults, then repo baseline (`config.base.yaml` - Config-level agent registration: an `agents` list in both `OrgConfig` and `PerRepoConfig` declares agent harness sources as pinned URLs or local paths, replacing compiled-in agent discovery ([ADR 0058](ADRs/0058-agent-registration.md)). - Runtime resolution: `fullsend run ` resolves agents in two tiers: (1) config entries from `OrgConfig.Agents` (highest priority), (2) runtime fallback to the `fullsend-ai/agents` repository for known first-party agents not in config. The agents-repo fallback is a transitional mechanism for the agent extraction; it will be removed once all users have migrated to config-driven registration (ADR 0058 Phase 5). - Config lookup: config entries are looked up directly via `findConfigAgentEntry`; the agents-repo fallback operates independently when the agent is not found in config. Builds on [ADR 0045](ADRs/0045-forge-portable-harness-schema.md) harness identity model. -- CLI management: `fullsend agent add|list|update|remove` manages config entries and auto-pins URLs to a commit SHA with an integrity hash. +- CLI management: `fullsend agent add|list|set|update|remove` manages config entries and auto-pins URLs to a commit SHA with an integrity hash. **Open questions:** diff --git a/docs/cli/README.md b/docs/cli/README.md index a5786b222f..51297619a5 100644 --- a/docs/cli/README.md +++ b/docs/cli/README.md @@ -14,7 +14,7 @@ Download the latest binary from [GitHub Releases](https://github.com/fullsend-ai | Command group | Description | |--------------|-------------| -| [`fullsend agent`](agent.md) | Manage agent registrations — add, list, update, remove | +| [`fullsend agent`](agent.md) | Manage agent registrations — add, list, set, update, remove | | [`fullsend github`](github.md) | Configure GitHub orgs and repos — setup, enrollment, day-2 operations | | [`fullsend inference`](inference.md) | Manage GCP Workload Identity Federation for Agent Platform access | | [`fullsend mint`](mint.md) | Deploy and manage the OIDC token mint service | diff --git a/docs/cli/agent.md b/docs/cli/agent.md index 50d27af44a..04b106013c 100644 --- a/docs/cli/agent.md +++ b/docs/cli/agent.md @@ -4,7 +4,7 @@ sidebar_label: fullsend agent # fullsend agent -Manage agent registrations in fullsend config. Add, list, update, and remove agents. +Manage agent registrations in fullsend config. Add, list, set (runtime, model, effort), update, and remove agents. `agent add` and `agent update` fetch remote content and resolve GitHub URLs. Authentication is via `gh` CLI or `GH_TOKEN` environment variable. @@ -15,6 +15,7 @@ Manage agent registrations in fullsend config. Add, list, update, and remove age | `fullsend agent add ` | Register an agent in config | | `fullsend agent list` | List registered agents | | `fullsend agent update [sha]` | Update a URL agent to a new commit SHA | +| `fullsend agent set ` | Set an agent's runtime, model or effort | | `fullsend agent remove ` | Remove an agent from config | ## `agent add` @@ -75,6 +76,31 @@ fullsend agent update triage a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2 --fullsend Only URL agents can be updated — local path agents have nothing to pin. Non-GitHub URL agents require an explicit SHA argument. The integrity hash is recomputed by fetching the content at the new SHA. +## `agent set` + +Sets `runtime`, `model` and/or `effort` for one agent in `.fullsend/config.yaml` (per-repo +configs). A built-in agent (`triage`, `code`, `review`, `fix`, `retro`, `prioritize`) without an +entry gets a name-only entry; a custom agent's settings land on its `source:` entry (or, for an +agent registered in `config.base.yaml`, on a name-only overlay entry that merges onto it). Only the +flags given change; pass an empty value (`--model ""`) to clear a setting. The result is validated +before it is written. + +```bash +fullsend agent set code --fullsend-dir .fullsend --runtime claude --model sonnet --effort high +fullsend agent set triage --fullsend-dir .fullsend --model xai-vertex/xai/grok-4.6 +``` + +### Flags + +| Flag | Description | +|------|-------------| +| `--fullsend-dir` | Path to the `.fullsend` configuration directory (required) | +| `--runtime` | Agent runtime for this agent (`claude` or `pi`) | +| `--model` | Model for this agent — an alias, a model id, or `provider/id` on pi | +| `--effort` | Effort level for this agent (`low`, `medium`, `high`, `xhigh`, `max`) | + +See [Runtimes — per-agent settings](../runtimes.md#per-agent-runtime-model-and-effort-in-configyaml) for precedence. + ## `agent remove` Remove an agent from config. If the removed agent was the last one using a given `allowed_remote_resources` prefix, that prefix is also cleaned up. diff --git a/docs/cli/github.md b/docs/cli/github.md index 16bb86c131..1056149445 100644 --- a/docs/cli/github.md +++ b/docs/cli/github.md @@ -40,6 +40,15 @@ fullsend github setup \ --inference-wif-provider "" ``` +**Re-running per-repo setup** (for example after a fullsend upgrade) refreshes the managed +workflow files but never rewrites an existing `.fullsend/config.yaml` on its own: `agents:` entries and +their per-agent settings, allowlists and hand-written comments stay as they are, the runtime prompt is skipped, +and the setup PR reports the runtime the file already selects. Passing a flag that targets a +config key — `--runtime`, `--agents`, `--mint-url`, `--inference-*` — changes that key on the +existing file and keeps the rest (the file is re-serialized, so comments are not preserved in +that case). `--config` rewrites `config.base.yaml` and keeps the existing overlay. A +`config.yaml` that no longer parses fails the re-run rather than being regenerated. + ### Flags | Flag | Default | Description | diff --git a/docs/cli/run.md b/docs/cli/run.md index 5a38af3d4b..aacb999841 100644 --- a/docs/cli/run.md +++ b/docs/cli/run.md @@ -48,9 +48,9 @@ The **Runtime** line shows which runtime was selected and the config source it w ## Runtime selection -The runtime for a run is resolved once, in this order: `--runtime` flag, `FULLSEND_RUNTIME`, the per-repo `runtime:` in `config.yaml` / `.fullsend/config.yaml`, then the built-in `claude`. The same order applies to the model (`--model`, `FULLSEND_MODEL`, harness `model:`, agent frontmatter; `FULLSEND_PI_MODEL` is a lower-precedence alias on pi) and to effort (`--effort`, `FULLSEND_EFFORT`, harness `effort:`). `FULLSEND_FALLBACK_MODELS=a,b` becomes Claude Code's `--fallback-model`; pi ignores it with a warning. +The runtime for a run is resolved once, in this order: `--runtime` flag, `FULLSEND_RUNTIME`, `runtime:` on the agent's `agents:` entry in `config.yaml` / `.fullsend/config.yaml`, the repo-wide `runtime:` there, then the built-in `claude`. The same order applies to the model (`--model`, `FULLSEND_MODEL`, `model:` on the agent's `agents:` entry, harness `model:`, agent frontmatter; `FULLSEND_PI_MODEL` is a lower-precedence alias on pi) and to effort (`--effort`, `FULLSEND_EFFORT`, `effort:` on the agent's `agents:` entry, harness `effort:`). `` is the name given to `fullsend run` (`triage`, `code`, …); see [Runtimes — per-agent settings](../runtimes.md#per-agent-runtime-model-and-effort-in-configyaml). `FULLSEND_FALLBACK_MODELS=a,b` becomes Claude Code's `--fallback-model`; pi ignores it with a warning. -The plan block prints `Runtime: (from )` and, when an override applied, `Model: (from )`; stderr carries `runtime: selected "" from ` (and `model: requested "" from `) for scripts. An invalid override (unknown runtime, unknown effort level) fails before the sandbox is created. +The plan block prints `Runtime: (from )` and, when an override applied, `Model: (from )`; stderr carries `runtime: selected "" from ` (and `model: requested "" from `) for scripts. A value from the config file is labelled with the file path, suffixed ` agents.` when the agent's entry decided. An invalid override (unknown runtime, unknown effort level, an `agents:` entry that names no agent) fails before the sandbox is created. ```bash # try a repo's triage on pi with Gemini Flash, without touching its config @@ -76,8 +76,8 @@ Each run produces artifacts in the output directory: | `model` | Model the provider reported using | | `requested_runtime` | Runtime selected for the run (config file, or a `--runtime`/`FULLSEND_RUNTIME` override) | | `requested_model` | Model the harness/agent requested | -| `override_source` | Where `requested_model` came from (`--model flag`, `FULLSEND_MODEL`, `FULLSEND_PI_MODEL`, `harness`, `default`) | -| `runtime_source` | Where `requested_runtime` came from (`--runtime flag`, `FULLSEND_RUNTIME`, the config file path, or `default (config not found)`) | +| `override_source` | Where `requested_model` came from (`--model flag`, `FULLSEND_MODEL`, `FULLSEND_PI_MODEL`, ` agents.`, `harness`, `default`) | +| `runtime_source` | Where `requested_runtime` came from (`--runtime flag`, `FULLSEND_RUNTIME`, the config file path — suffixed ` agents.` when the agent's entry decided — or `default (config not found)`) | | `total_cost_usd` | Total inference cost | | `num_turns` | Number of conversation turns | | `iterations` | Number of retry iterations | diff --git a/docs/contributing/runtime-implementation.md b/docs/contributing/runtime-implementation.md index b8bd1771b7..429e777496 100644 --- a/docs/contributing/runtime-implementation.md +++ b/docs/contributing/runtime-implementation.md @@ -242,7 +242,7 @@ flowchart TB - **Runs unattended** (parity with `claude -p --dangerously-skip-permissions`, verified against pi v0.84.2 source and empirically on the pinned build) — pi has no tool-approval layer at all (nothing in `core/tools/*` or `core/bash-executor.ts` prompts); in `--print` mode extensions get a no-op UI context, so `ctx.ui.confirm/select/input/editor` resolve immediately (`modes/print-mode.ts`, `core/extensions/runner.ts`); `--no-approve` sets the project-trust override, so the trust-gated project resources — `.pi/{settings.json,extensions,skills,prompts,themes,SYSTEM.md,APPEND_SYSTEM.md}` and `.agents/skills` (`core/trust-manager.ts`); `AGENTS.md` itself is still read as context — are ignored without a dialog (`cli/args.ts`, `main.ts`), and `defaultProjectTrust: never` in the global settings covers the no-flag case (verified on the pinned build: a planted `.pi/extensions/evil.js` in the repo does not load under `--no-approve` and does under `--approve`); first-run setup, theme selection, telemetry consent and the version check are interactive-only code paths (`PI_TELEMETRY=0`, `PI_SKIP_VERSION_CHECK=1`/`PI_OFFLINE=1` set anyway); a missing credential raises `No API key found` and exits 1 — no `/login` prompt (`core/agent-session.ts`, `modes/print-mode.ts`); retries are bounded (`retry.maxRetries: 3`, 2/4/8 s) and compaction is automatic. The one blocker found: print mode reads a non-TTY stdin to EOF before the first prompt, even with a positional message (`main.ts` `readPipedStdin`), so an exec that keeps stdin open with no writer hangs pi — `Run` therefore appends `"` commits `runtime: ` to the leased repo's config for this scenario only (CleanupScenario restores `dummy` — slots are reused, so never set it any other way; the step refuses if the slot is not on `dummy` to begin with). The custom-harness step commits only a placeholder for a relative `agent:` path, which a real runtime cannot act on, so follow it with `And a pi agent "" defined as:` and a docstring holding the full agent file (frontmatter + body) — `{{fixture:fixtures//.json}}` inlines a result fixture so the model has a concrete, deterministic file to write (the custom harness carries no post-script, so nothing validates it; the assertions are on the transcript and metrics). Then the scenario dispatches the harness and asserts on artifacts: `the run selected the "pi" runtime`, `the pi session transcript records at least one tool call` (the agent used a tool through pi; with security enabled the run refuses to start without the intact hook adapter, so the call was mediated by it — the step does not inspect hook output), `the run metrics report tokens`. Such scenarios cost a real model run on the pool repo's repo-scoped Vertex WIF and must be tagged `@requires:capability:runtime-` so they stay off until the runner declares the capability (for pi: after `fullsend-sandbox:latest` ships `PI_VERSION`). See `features/runtime/pi.feature`. +- **Runtime-specific (gated):** `Given the repository runtime is ""` commits `runtime: ` to the leased repo's config for this scenario only (CleanupScenario restores `dummy` — slots are reused, so never set it any other way; the step refuses if the slot is not on `dummy` to begin with). The custom-harness step commits only a placeholder for a relative `agent:` path, which a real runtime cannot act on, so follow it with `And a pi agent "" defined as:` and a docstring holding the full agent file (frontmatter + body) — `{{fixture:fixtures//.json}}` inlines a result fixture so the model has a concrete, deterministic file to write (the custom harness carries no post-script, so nothing validates it; the assertions are on the transcript and metrics). Then the scenario dispatches the harness and asserts on artifacts: `the run selected the "pi" runtime`, `the pi session transcript records at least one tool call` (the agent used a tool through pi; with security enabled the run refuses to start without the intact hook adapter, so the call was mediated by it — the step does not inspect hook output), `the run metrics report tokens`. Such scenarios cost a real model run on the pool repo's repo-scoped Vertex WIF and must be tagged `@requires:capability:runtime-` so they only run where the runner declares the capability; `make behaviour-test` declares `runtime-pi` by default (a `Makefile` variable, so a PR adding a gated scenario exercises it on its own `pull_request_target` run — the workflow file itself comes from `main`); `BEHAVIOUR_CAPABILITIES= make behaviour-test` skips them. See `features/runtime/pi.feature`. +- **Per-agent (every run):** `Given the repository agents are configured with:` with a YAML docstring (`triage:\n runtime: dummy`) sets runtime/model/effort on the leased repo's `agents:` entries (a name-only entry for a built-in, the sourced entry for a custom agent; only the settings given change) — validated the way `fullsend run` validates them — and CleanupScenario restores the pre-scenario `agents:` list. Pair it with `the repository runtime is ""` and pin every agent the scenario can dispatch (triage hands off to `code` via `ready-to-code`) back to `dummy`, then assert `the run selected the "dummy" runtime from "agents.triage"`, which also checks `runtime_source` in `metrics.json` ends with that entry — proof the per-agent entry decided, at dummy cost. The gated second scenario in the same file leaves the repo on `dummy` and puts one custom agent on pi with `model: haiku` from its entry (the harness says `opus`); `the run requested model "haiku" from "agents." and the provider reported a "haiku" model` checks `requested_model`, `override_source`, the reported `model` and `num_turns` in `metrics.json`. See `features/runtime/agent-settings.feature`. Do not add runtime coverage to `e2e/admin` (org-mode install, deprecated per ADR 0044) or behind new `fullsend admin` flags. diff --git a/docs/guides/dev/cli-internals.md b/docs/guides/dev/cli-internals.md index 5d45a9fc83..0512901718 100644 --- a/docs/guides/dev/cli-internals.md +++ b/docs/guides/dev/cli-internals.md @@ -76,6 +76,10 @@ fullsend ├── agent # Manage agent registrations in config │ ├── add # Register an agent (URL auto-pinned) │ ├── list # List registered agents +│ ├── set # Set an agent's runtime, model or effort (per-repo) +│ │ ├── --runtime # Runtime for this agent +│ │ ├── --model # Model for this agent +│ │ └── --effort # Effort level for this agent │ ├── update [sha] # Re-pin URL agent to new commit SHA │ └── remove # Unregister agent from config ├── lock [agent-name] # Pin remote deps to lock.yaml diff --git a/docs/guides/getting-started/choosing-a-runtime.md b/docs/guides/getting-started/choosing-a-runtime.md index e15528364c..dc614efb3c 100644 --- a/docs/guides/getting-started/choosing-a-runtime.md +++ b/docs/guides/getting-started/choosing-a-runtime.md @@ -20,8 +20,8 @@ Fullsend supports multiple agent runtimes. A runtime is the program that runs in ## When and how the runtime is selected 1. **Next step — Configuring GitHub.** `fullsend github setup ` asks which runtime to use when run from a terminal; press Enter to keep `claude`. Passing `--runtime` skips the prompt. The setup PR it opens records the choice in `.fullsend/config.yaml` and describes how to change it. Nothing runs on this page — continue with [Configuring GitHub](configuring-github.md). -2. **Later — changing it.** Edit `runtime:` in the repo's `.fullsend/config.yaml` (the setup PR shows the key), or re-run `fullsend github setup --runtime `. Fleets managed through `repos.yaml` set `defaults.runtime` (or a per-entry `runtime`) — `fullsend repos set-default defaults.runtime pi` — and run `fullsend repos install`; see [fullsend repos](../../cli/repos.md). -3. **Per run — trying without changing the repo.** `fullsend run --runtime pi --model google-vertex/gemini-2.5-flash`, or the `FULLSEND_RUNTIME` / `FULLSEND_MODEL` / `FULLSEND_EFFORT` environment variables (flag beats environment beats config). In CI the same names work as repository variables. Reference: [fullsend run](../../cli/run.md) and [Runtimes — selecting and overriding](../../runtimes.md#selecting-a-runtime-and-model). +2. **Later — changing it.** Edit `runtime:` in the repo's `.fullsend/config.yaml` (the setup PR shows the key), or re-run `fullsend github setup --runtime `. To put one agent on a different runtime or model than the rest — say `code` on Claude Code while `triage` runs Grok on pi — set `runtime:` on that agent's `agents:` entry in the same file (`fullsend agent set code --runtime claude`); see [Runtimes — per-agent settings](../../runtimes.md#per-agent-runtime-model-and-effort-in-configyaml). Fleets managed through `repos.yaml` set `defaults.runtime` (or a per-entry `runtime`) — `fullsend repos set-default defaults.runtime pi` — and run `fullsend repos install`; see [fullsend repos](../../cli/repos.md). +3. **Per run — trying without changing the repo.** `fullsend run --runtime pi --model google-vertex/gemini-2.5-flash`, or the `FULLSEND_RUNTIME` / `FULLSEND_MODEL` / `FULLSEND_EFFORT` environment variables (flag beats environment beats the agent's `agents:` entry beats repo-wide config). In CI the same names work as repository variables. Reference: [fullsend run](../../cli/run.md) and [Runtimes — selecting and overriding](../../runtimes.md#selecting-a-runtime-and-model). ## Where to see what ran diff --git a/docs/guides/getting-started/operations.md b/docs/guides/getting-started/operations.md index 708115731b..c8c08169df 100644 --- a/docs/guides/getting-started/operations.md +++ b/docs/guides/getting-started/operations.md @@ -113,6 +113,7 @@ For organizations that separate GCP and GitHub responsibilities across teams, fu | Developer | `fullsend agent add ` | Register an agent in config (URL auto-pinned to commit SHA) | | Developer | `fullsend agent list` | List registered agents and their sources | +| Developer | `fullsend agent set ` | Set an agent's runtime, model or effort | | Developer | `fullsend agent update [sha]` | Re-pin a URL agent to a new commit SHA | | Developer | `fullsend agent remove ` | Unregister an agent from config | diff --git a/docs/guides/infrastructure/layered-config-reference.md b/docs/guides/infrastructure/layered-config-reference.md index c855f4b219..fb366d0c75 100644 --- a/docs/guides/infrastructure/layered-config-reference.md +++ b/docs/guides/infrastructure/layered-config-reference.md @@ -87,6 +87,32 @@ the overlay → base → code defaults chain. | `create_issues` | `*CreateIssuesConfig` | Replace whole object if set | `nil` | | `status_notifications` | `*StatusNotificationConfig` | Replace whole object if set | `nil` | +### Per-agent `runtime`, `model`, `effort` on `agents:` entries + +An `agents:` entry may set `runtime`, `model` and `effort` for that agent. An +enabled entry without `source:` is an *override-only* entry that tunes a +built-in agent by name (or, in an overlay, a custom agent registered in the +base layer). The keyed merge by `DerivedName()` carries the three settings +field by field: the overlay's non-empty value wins, an empty value inherits +the parent's. There is no way to unset a parent's value from the overlay +short of restating the entry. + +```yaml +# config.base.yaml (preset) +agents: + - source: harness/lint.yaml # name derived from the file: lint + model: opus +# config.yaml (overlay) +agents: + - name: lint # merges onto the base entry: source kept, effort added + effort: medium + - name: code # built-in agent tuned by name + runtime: claude +``` + +Precedence at run time: flag > env var > the agent's entry > repo-wide +`runtime:` / harness default. + ### Scalar override fields **`version`**, **`runtime`**, and **`kill_switch`** use simple scalar diff --git a/docs/guides/user/running-agents-locally.md b/docs/guides/user/running-agents-locally.md index fcc15a24a5..a611c0aad8 100644 --- a/docs/guides/user/running-agents-locally.md +++ b/docs/guides/user/running-agents-locally.md @@ -332,7 +332,9 @@ The two are checked in different places and fail differently: nothing, fails with `resolving agent "pi-smoke": no config and agents-repo fallback unavailable`. - **No `defaults.runtime: pi`** — the run *succeeds* and silently uses the - default `claude` runtime (`backendFromConfigFile` → `ResolveFromConfig`). + default `claude` runtime (`backendFromConfigFile` → `ResolveFromConfig` + for this org-style config; a per-repo config resolves through + `ResolveForAgent`, which also honours `runtime:` on the agent's `agents:` entry). The give-away is the `runtime: selected "claude"` line; pi is never started. diff --git a/docs/runtimes.md b/docs/runtimes.md index 85170bc7c1..6496103730 100644 --- a/docs/runtimes.md +++ b/docs/runtimes.md @@ -69,24 +69,80 @@ override variables themselves. ```mermaid flowchart LR F["--runtime / --model
(flag)"] --> E["FULLSEND_RUNTIME
FULLSEND_MODEL"] - E --> C["config.yaml runtime:
harness model:"] + E --> R["config.yaml
agents: entry for the agent"] + R --> C["config.yaml runtime:
harness model:"] C --> A["agent frontmatter
model:"] A --> D["default
claude · opus"] classDef s fill:#e3e9fb,stroke:#2d5be3,color:#1b2230; classDef d fill:#eceee8,stroke:#a9afa4,color:#1b2230; - class F,E,C,A s; + class F,E,R,C,A s; class D d; ``` -| Setting | Flag | Env | Config | -|---|---|---|---| -| Runtime | `--runtime` | `FULLSEND_RUNTIME` | `runtime:` in `.fullsend/config.yaml` | -| Model | `--model` | `FULLSEND_MODEL` (`FULLSEND_PI_MODEL` is a lower-precedence alias on pi) | harness `model:`, then agent frontmatter `model:` | -| Effort | `--effort` | `FULLSEND_EFFORT` | harness `effort:` | +| Setting | Flag | Env | Config (per-agent) | Config (repo-wide) | +|---|---|---|---|---| +| Runtime | `--runtime` | `FULLSEND_RUNTIME` | `runtime:` on the agent's `agents:` entry | `runtime:` in `.fullsend/config.yaml` (repo default) | +| Model | `--model` | `FULLSEND_MODEL` (`FULLSEND_PI_MODEL` is a lower-precedence alias on pi) | `model:` on the agent's `agents:` entry | harness `model:`, then agent frontmatter `model:` | +| Effort | `--effort` | `FULLSEND_EFFORT` | `effort:` on the agent's `agents:` entry | harness `effort:` | In CI these are repository variables of the same name, plain or role-prefixed -(`TRIAGE_FULLSEND_MODEL`), so a repo can switch one role's model without a pull request. Harness -`env.runner` does **not** reach the `fullsend` process. +(`TRIAGE_FULLSEND_MODEL`), so a repo can switch one role's model without a pull request. For +**durable** per-agent configuration that lives in the repository and is reviewable, use +the agent's `agents:` entry in `.fullsend/config.yaml` instead. Harness `env.runner` does **not** reach the +`fullsend` process. + +### Per-agent runtime, model and effort in config.yaml + +The `agents:` list is the per-agent place in `config.yaml`: an entry names an agent and can set +its `runtime`, `model` and `effort`. A built-in agent (`triage`, `code`, `review`, `fix`, `retro`, +`prioritize`) is tuned with a name-only entry; a custom agent carries the settings on its +`source:` entry. + +```yaml +runtime: pi # repo default for agents that set none +agents: + - name: triage + model: xai-vertex/xai/grok-4.6 + - name: code + runtime: claude + model: sonnet + effort: high + - source: https://raw.githubusercontent.com/acme/agents//harness/lint.yaml#sha256=… + model: haiku +``` + +Or from the CLI: `fullsend agent set code --runtime claude --model sonnet --effort high`. + +A `source:` entry needs no `name:` — the agent's name is derived from the source file +(`harness/lint.yaml` → `lint`, ADR 0058), and that is the name the settings, `fullsend run lint` +and `fullsend agent set lint` all use; add `name:` only to override it. + +Names are agent names as passed to `fullsend run ` — **not** harness `role:` values (`code` +and `fix` both carry `role: coder`) — matched case-insensitively. A name-only entry for anything +that is not a built-in agent fails validation (`coder` gets a "did you mean `code`" hint); a custom +agent gets its settings on its own entry. + +Precedence: flag > env var > the agent's `agents:` entry > repo-wide `runtime:` / harness +`model:` `effort:` > default. Entries merge per field across the layered config (`config.yaml` +over `config.base.yaml`), so a preset base can tune agents too. `fullsend run` validates the +whole `agents:` list in every layer (names, runtime, model syntax, effort) and fails the run with +an error naming the file and entry rather than silently skipping a mistyped entry or handing a +bad value to the runtime. + +A value that came from here shows up as ` agents.` wherever the selection is +surfaced (plan block, stderr, `metrics.json` — see below); the path is the effective config file. + +`provider/id` is pi's model form. The syntax is accepted for every runtime (model ids are not a +closed set), but an entry that pairs `runtime: claude` with a `provider/id` model gets a warning +in the plan block — Claude Code expects an alias (`opus`, `sonnet`, …) or an Anthropic model id. + +**Migrating from repository variables.** A repo that carries `_FULLSEND_MODEL` / +`_FULLSEND_RUNTIME` variables can move them onto `agents:` entries one-to-one: the variable +prefix is the agent name (`CODE_FULLSEND_RUNTIME=claude` → `- name: code` / `runtime: claude`). +Delete the variable afterwards — while it exists it still wins, so the config entry would be +silently shadowed. Bump the workflow's fullsend pin to a version that carries per-agent settings +*before* adding them: an older pinned CLI rejects an enabled `agents:` entry without a `source`, +whereas a current CLI validates the settings on every run. Set the runtime per repo with `fullsend github setup --runtime pi`. Repos on pi need a sandbox image that carries `PI_VERSION`. @@ -99,14 +155,15 @@ On pi, a model is `provider/id` — aliases and bare ids still work, and the pro `FULLSEND_PI_PROVIDER` (default `anthropic-vertex`). pi reaches Claude, Gemini **and** Grok, each through its own provider; see [Pi › Models and providers](runtimes/pi.md#models-and-providers). -Because harness `model:` cannot contain `/` (`validModelName` is `^[a-zA-Z0-9_.@-]+$`), a harness -selects a pi provider with a bare `model:` plus `FULLSEND_PI_PROVIDER`. +Harness `model:` and `agents:` entry `model:` values accept provider-qualified `provider/id` syntax +(e.g. `google-vertex/gemini-3.7-flash`). On pi, a harness can also select a provider with a bare +`model:` plus `FULLSEND_PI_PROVIDER`. ## Where the selection appears | Surface | What it shows | |---|---| -| Run plan block | `Runtime: (from )` next to Model and Effort | +| Run plan block | `Runtime: (from )` next to Model and Effort; `` is the flag, the variable, or `` (suffixed ` agents.` when the agent's entry decided) | | stderr | `runtime: selected "" from ` | | Status comment / `::notice::` | `Runtime · Model: · Effort · Cost` | | OTel span | `fullsend.runtime`, next to `gen_ai.request.model` | diff --git a/docs/runtimes/pi.md b/docs/runtimes/pi.md index aae2a28158..d5c7059e72 100644 --- a/docs/runtimes/pi.md +++ b/docs/runtimes/pi.md @@ -30,8 +30,9 @@ through fullsend's table, and a bare id gets the provider from `FULLSEND_PI_PROV > and a bare id under `FULLSEND_PI_PROVIDER=xai-vertex`, case-insensitively, so both land on the > canonical spec. -Because harness `model:` cannot contain `/` (`validModelName` is `^[a-zA-Z0-9_.@-]+$`), a harness -selects a pi provider with a bare `model:` plus `FULLSEND_PI_PROVIDER`. +Harness `model:` and `agents:` entry `model:` values accept the `provider/id` form directly +(`xai-vertex/xai/grok-4.6`); a harness can also select a provider with a bare `model:` plus +`FULLSEND_PI_PROVIDER`. ### Each provider has its own GCP project diff --git a/e2e/behaviour/features/runtime/agent-settings.feature b/e2e/behaviour/features/runtime/agent-settings.feature new file mode 100644 index 0000000000..717feafc71 --- /dev/null +++ b/e2e/behaviour/features/runtime/agent-settings.feature @@ -0,0 +1,105 @@ +# Per-agent runtime selection through agents: entries in .fullsend/config.yaml +# (#6581, ADR 0091). The repo-wide `runtime:` is flipped to a real runtime for +# this scenario and the agents the scenario can dispatch are pinned back to +# dummy on their own agents: entries, so a passing run proves the per-agent +# entry beat the repo-wide key without any inference: the runner records which config entry +# decided in metrics.json (`runtime_source` ends with `agents.triage`). +# +# `code` is pinned too because triage hands off with `ready-to-code`, which +# dispatches the code stage on the same repo; without its own entry that run +# would start on the repo-wide runtime for real. +Feature: Per-agent runtime and model on agents: entries in config.yaml + + Scenario: an agents: entry selects the runtime for one agent over the repo-wide key + Given the enrolled test repository + And the repository runtime is "claude" + And the repository agents are configured with: + """ + triage: + runtime: dummy + code: + runtime: dummy + """ + And a dummy agent that would: + | description | op | args | + | Emit triage JSON | write_fixture | output/agent-result.json, fixtures/triage/sufficient.json | + And an issue + When the issue is labeled "ready-for-triage" + Then the triage workflow completes successfully + And the run selected the "dummy" runtime from "agents.triage" + And the agent will succeed to Emit triage JSON + And the issue has label "ready-to-code" + + # Real-runtime half: the repo stays on the install default (dummy) and a + # single custom agent is put on pi with its own model through + # agents: entries. The harness itself says `model: opus`; the entry says + # `haiku`, so a passing run proves both the runtime and the model of one + # agent came from config.yaml, not from the harness or the repo-wide key. + # Gated like pi.feature: one small haiku run on Vertex per suite run. + @requires:capability:runtime-pi + Scenario: an agents: entry puts one custom agent on pi with its own model + Given the enrolled test repository + And a custom harness "pi-override" with: + """ + agent: agents/pi-override.md + role: triage + slug: fullsend-ai-pi-override + model: opus + image: ghcr.io/fullsend-ai/fullsend-sandbox:latest + trigger: | + event.entity.kind == "work_item" + && event.transition.kind == "label_changed" + && event.transition.label.name == "ready-for-pi-override" + # Vertex reaches the sandbox the way the fleet harnesses wire it: + # egress is granted by the vertex-ai provider (ADR-0065; the per-repo + # scaffold ships these provider/profile files), credentials arrive as + # host files, and the project/region env is inlined here because the + # scaffold ships no gcp-vertex.env. ${VAR} expands from the runner + # environment set by setup-gcp. + profiles: + - profiles/fullsend-vertex-ai.yaml + providers: + - providers/vertex-ai.yaml + host_files: + - src: ${GOOGLE_APPLICATION_CREDENTIALS} + dest: /tmp/.gcp-credentials.json + - src: ${GCP_OIDC_TOKEN_FILE} + dest: /sandbox/workspace/.gcp-oidc-token + optional: true + env: + sandbox: + ANTHROPIC_VERTEX_PROJECT_ID: ${ANTHROPIC_VERTEX_PROJECT_ID} + GOOGLE_CLOUD_PROJECT: ${ANTHROPIC_VERTEX_PROJECT_ID} + CLOUD_ML_REGION: ${CLOUD_ML_REGION} + GOOGLE_APPLICATION_CREDENTIALS: /tmp/.gcp-credentials.json + """ + And a pi agent "pi-override" defined as: + """ + --- + name: pi-override + description: Behaviour agent proving a per-agent config entry selects pi and a model for one agent. + tools: Bash(ls), Write + --- + You are an unattended smoke-test agent. Do exactly the following, in + order, then stop. Do not ask questions, do not explain, do not read or + modify any other file. + + 1. Using the bash tool, run: ls . + 2. Using the write tool, create the file /sandbox/workspace/output/agent-result.json + with exactly this content and nothing else: + + {{fixture:fixtures/triage/sufficient.json}} + """ + And the repository agents are configured with: + """ + pi-override: + runtime: pi + model: haiku + """ + And an issue + When the issue is labeled "ready-for-pi-override" + Then the harness "pi-override" workflow completes successfully + And the run selected the "pi" runtime from "agents.pi-override" + And the run requested model "haiku" from "agents.pi-override" and the provider reported a "haiku" model + And the pi session transcript records at least one tool call + And the run metrics report tokens diff --git a/e2e/behaviour/features/runtime/pi.feature b/e2e/behaviour/features/runtime/pi.feature index 8902148154..242c88ba1d 100644 --- a/e2e/behaviour/features/runtime/pi.feature +++ b/e2e/behaviour/features/runtime/pi.feature @@ -28,6 +28,28 @@ Feature: pi runtime runs an agent unattended event.entity.kind == "work_item" && event.transition.kind == "label_changed" && event.transition.label.name == "ready-for-pi-smoke" + # Vertex reaches the sandbox the way the fleet harnesses wire it: + # egress is granted by the vertex-ai provider (ADR-0065; the per-repo + # scaffold ships these provider/profile files), credentials arrive as + # host files, and the project/region env is inlined here because the + # scaffold ships no gcp-vertex.env. ${VAR} expands from the runner + # environment set by setup-gcp. + profiles: + - profiles/fullsend-vertex-ai.yaml + providers: + - providers/vertex-ai.yaml + host_files: + - src: ${GOOGLE_APPLICATION_CREDENTIALS} + dest: /tmp/.gcp-credentials.json + - src: ${GCP_OIDC_TOKEN_FILE} + dest: /sandbox/workspace/.gcp-oidc-token + optional: true + env: + sandbox: + ANTHROPIC_VERTEX_PROJECT_ID: ${ANTHROPIC_VERTEX_PROJECT_ID} + GOOGLE_CLOUD_PROJECT: ${ANTHROPIC_VERTEX_PROJECT_ID} + CLOUD_ML_REGION: ${CLOUD_ML_REGION} + GOOGLE_APPLICATION_CREDENTIALS: /tmp/.gcp-credentials.json """ And a pi agent "pi-smoke" defined as: """ diff --git a/internal/cli/agent.go b/internal/cli/agent.go index 4d329bf531..861a5e03a4 100644 --- a/internal/cli/agent.go +++ b/internal/cli/agent.go @@ -35,6 +35,7 @@ func newAgentCmd() *cobra.Command { cmd.AddCommand(newAgentListCmd()) cmd.AddCommand(newAgentUpdateCmd()) cmd.AddCommand(newAgentRemoveCmd()) + cmd.AddCommand(newAgentSetCmd()) return cmd } @@ -141,6 +142,102 @@ func newAgentRemoveCmd() *cobra.Command { return cmd } +func newAgentSetCmd() *cobra.Command { + var fullsendDir, runtimeName, model, effort string + + cmd := &cobra.Command{ + Use: "set ", + Short: "Set an agent's runtime, model or effort in config (per-repo)", + Long: `Sets runtime, model and/or effort for one agent in .fullsend/config.yaml. +The agent is a built-in one (triage, code, review, fix, retro, prioritize) +or a custom agents: entry by name. For a built-in agent without an entry a +name-only entry is added. Only the flags given change; pass an empty value +(--model "") to clear a setting. Per-repo configs only.`, + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + printer := ui.New(os.Stdout) + return runAgentSet(fullsendDir, args[0], agentSetFlags{ + runtime: runtimeName, model: model, effort: effort, + runtimeSet: cmd.Flags().Changed("runtime"), + modelSet: cmd.Flags().Changed("model"), + effortSet: cmd.Flags().Changed("effort"), + }, printer) + }, + } + cmd.Flags().StringVar(&fullsendDir, "fullsend-dir", "", "path to the .fullsend configuration directory") + cmd.Flags().StringVar(&runtimeName, "runtime", "", "agent runtime for this agent (claude or pi); \"\" clears it") + cmd.Flags().StringVar(&model, "model", "", "model for this agent (alias, model id, or provider/id on pi); \"\" clears it") + cmd.Flags().StringVar(&effort, "effort", "", "effort level for this agent (low, medium, high, xhigh, max); \"\" clears it") + _ = cmd.MarkFlagRequired("fullsend-dir") + return cmd +} + +// agentSetFlags carries `agent set` flag values and whether each was given. +type agentSetFlags struct { + runtime, model, effort string + runtimeSet, modelSet, effortSet bool +} + +// runAgentSet upserts runtime/model/effort for one agent on the overlay +// config.yaml and validates the result the way `fullsend run` will. +func runAgentSet(fullsendDir, agentName string, f agentSetFlags, printer *ui.Printer) error { + if !f.runtimeSet && !f.modelSet && !f.effortSet { + return fmt.Errorf("nothing to set: pass at least one of --runtime, --model, --effort") + } + absDir, err := filepath.Abs(fullsendDir) + if err != nil { + return fmt.Errorf("resolving fullsend dir: %w", err) + } + configPath := filepath.Join(absDir, config.OverlayConfigFile) + cfg, err := loadAgentConfig(configPath) + if err != nil { + return err + } + w, ok := cfg.(config.PerRepoConfigWriter) + if !ok { + return fmt.Errorf("agent set applies to per-repo configs; %s is an org config", configPath) + } + + // Start from the current effective values so an unset flag keeps them. + current, _ := config.AgentSettingsFor(cfg.AgentEntries(), agentName) + runtimeName, model, effort := current.Runtime, current.Model, current.Effort + if f.runtimeSet { + runtimeName = f.runtime + } + if f.modelSet { + model = f.model + } + if f.effortSet { + effort = f.effort + } + // Only the overlay's own entries are written; an agent registered in + // config.base.yaml gets an overlay entry that merges onto it by name. + local := config.UpsertAgentSettings(append([]config.AgentEntry(nil), localAgentEntries(cfg)...), agentName, runtimeName, model, effort) + w.SetAgents(local) + + if err := cfg.Validate(); err != nil { + return fmt.Errorf("config validation failed: %w", err) + } + data, err := cfg.Marshal() + if err != nil { + return err + } + if err := os.WriteFile(configPath, data, 0o644); err != nil { + return fmt.Errorf("writing config: %w", err) + } + printer.StepDone(fmt.Sprintf("Set agent %q: runtime=%q model=%q effort=%q (empty = inherit)", agentName, runtimeName, model, effort)) + return nil +} + +// localAgentEntries returns the entries the overlay itself declares (the +// parent chain's entries are not rewritten into config.yaml). +func localAgentEntries(cfg config.ConfigReader) []config.AgentEntry { + if local, ok := cfg.(interface{ LocalAgentEntries() []config.AgentEntry }); ok { + return local.LocalAgentEntries() + } + return cfg.AgentEntries() +} + func runAgentAdd(ctx context.Context, source, name, fullsendDir string, forgeClient forge.Client, printer *ui.Printer) error { absDir, err := filepath.Abs(fullsendDir) if err != nil { @@ -238,6 +335,22 @@ func runAgentList(fullsendDir string, printer *ui.Printer) error { if cleanURL, _, hasHash := urlutil.ParseIntegrityHash(a.Source); hasHash { displaySource = cleanURL } + if displaySource == "" { + displaySource = "(built-in)" + } + if a.HasSettings() { + var settings []string + if a.Runtime != "" { + settings = append(settings, "runtime="+a.Runtime) + } + if a.Model != "" { + settings = append(settings, "model="+a.Model) + } + if a.Effort != "" { + settings = append(settings, "effort="+a.Effort) + } + displaySource += " [" + strings.Join(settings, " ") + "]" + } printer.Raw(fmt.Sprintf("%-*s %s\n", maxName, a.DerivedName(), displaySource)) } return nil diff --git a/internal/cli/agent_test.go b/internal/cli/agent_test.go index 79323dbf82..414ef0d7c3 100644 --- a/internal/cli/agent_test.go +++ b/internal/cli/agent_test.go @@ -1,6 +1,7 @@ package cli import ( + "bytes" "context" "fmt" "net" @@ -1201,7 +1202,7 @@ allowed_remote_resources: func TestNewAgentCmd_HasSubcommands(t *testing.T) { cmd := newAgentCmd() - assert.Len(t, cmd.Commands(), 4) + assert.Len(t, cmd.Commands(), 5) names := make([]string, len(cmd.Commands())) for i, c := range cmd.Commands() { names[i] = c.Name() @@ -1212,3 +1213,130 @@ func TestNewAgentCmd_HasSubcommands(t *testing.T) { assert.Contains(t, names, "remove") assert.NotContains(t, names, "migrate-customizations", "should not exist — removed per ADR-0064") } + +func TestRunAgentRemove_DropsSettingsWithTheEntry(t *testing.T) { + dir := t.TempDir() + writePerRepoConfig(t, dir, `agents: + - source: harness/lint.yaml + model: sonnet + - source: harness/review.yaml + effort: high +`) + require.NoError(t, runAgentRemove(dir, "lint", ui.New(os.Stdout))) + cfg, err := loadAgentConfig(filepath.Join(dir, "config.yaml")) + require.NoError(t, err) + require.Len(t, cfg.AgentEntries(), 1) + _, found := config.AgentSettingsFor(cfg.AgentEntries(), "lint") + assert.False(t, found) + review, found := config.AgentSettingsFor(cfg.AgentEntries(), "review") + require.True(t, found) + assert.Equal(t, "high", review.Effort, "other entries keep their settings") +} + +func TestRunAgentSet(t *testing.T) { + dir := t.TempDir() + writePerRepoConfig(t, dir, `runtime: pi +agents: + - source: harness/lint.yaml +`) + var out bytes.Buffer + + // A built-in agent gets a name-only entry. + require.NoError(t, runAgentSet(dir, "code", agentSetFlags{runtime: "claude", runtimeSet: true, model: "sonnet", modelSet: true}, ui.New(&out))) + cfg, err := loadAgentConfig(filepath.Join(dir, "config.yaml")) + require.NoError(t, err) + code, found := config.AgentSettingsFor(cfg.AgentEntries(), "code") + require.True(t, found) + assert.Equal(t, config.AgentEntry{Name: "code", Runtime: "claude", Model: "sonnet"}, code) + + // A second call changes only the flags given; "" clears. + require.NoError(t, runAgentSet(dir, "code", agentSetFlags{effort: "high", effortSet: true, model: "", modelSet: true}, ui.New(&out))) + cfg, err = loadAgentConfig(filepath.Join(dir, "config.yaml")) + require.NoError(t, err) + code, _ = config.AgentSettingsFor(cfg.AgentEntries(), "code") + assert.Equal(t, config.AgentEntry{Name: "code", Runtime: "claude", Effort: "high"}, code) + + // A custom agent's settings land on its sourced entry. + require.NoError(t, runAgentSet(dir, "lint", agentSetFlags{model: "haiku", modelSet: true}, ui.New(&out))) + cfg, err = loadAgentConfig(filepath.Join(dir, "config.yaml")) + require.NoError(t, err) + lint, _ := config.AgentSettingsFor(cfg.AgentEntries(), "lint") + assert.Equal(t, "harness/lint.yaml", lint.Source) + assert.Equal(t, "haiku", lint.Model) + assert.Len(t, cfg.AgentEntries(), 2) + + // Validation guards the write: unknown built-in, bad values, no flags. + err = runAgentSet(dir, "coder", agentSetFlags{model: "sonnet", modelSet: true}, ui.New(&out)) + require.Error(t, err) + assert.Contains(t, err.Error(), `did you mean "code"`) + err = runAgentSet(dir, "triage", agentSetFlags{effort: "turbo", effortSet: true}, ui.New(&out)) + require.Error(t, err) + assert.Contains(t, err.Error(), `invalid effort "turbo"`) + require.Error(t, runAgentSet(dir, "triage", agentSetFlags{}, ui.New(&out))) + cfg, err = loadAgentConfig(filepath.Join(dir, "config.yaml")) + require.NoError(t, err) + _, found = config.AgentSettingsFor(cfg.AgentEntries(), "triage") + assert.False(t, found, "failed sets write nothing") +} + +func TestRunAgentSet_BaseLayerAgentGetsOverlayEntry(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.base.yaml"), []byte(`# fullsend per-repo configuration +version: "1" +agents: + - source: harness/lint.yaml + model: opus +`), 0o644)) + writePerRepoConfig(t, dir, "") + require.NoError(t, runAgentSet(dir, "lint", agentSetFlags{effort: "medium", effortSet: true}, ui.New(os.Stdout))) + + // The overlay gains a name-only entry that merges onto the base one; + // the base file is untouched. + overlay, err := os.ReadFile(filepath.Join(dir, "config.yaml")) + require.NoError(t, err) + assert.Contains(t, string(overlay), "name: lint") + assert.NotContains(t, string(overlay), "harness/lint.yaml") + cfg, err := loadAgentConfig(filepath.Join(dir, "config.yaml")) + require.NoError(t, err) + lint, found := config.AgentSettingsFor(cfg.AgentEntries(), "lint") + require.True(t, found) + assert.Equal(t, "harness/lint.yaml", lint.Source) + assert.Equal(t, "opus", lint.Model, "base model kept (unset flag inherits)") + assert.Equal(t, "medium", lint.Effort) +} + +func TestAgentSetCmd_ExecutesAndTracksChangedFlags(t *testing.T) { + dir := t.TempDir() + writePerRepoConfig(t, dir, "") + cmd := newAgentSetCmd() + cmd.SetArgs([]string{"review", "--fullsend-dir", dir, "--effort", "low"}) + cmd.SetOut(&bytes.Buffer{}) + require.NoError(t, cmd.Execute()) + cfg, err := loadAgentConfig(filepath.Join(dir, "config.yaml")) + require.NoError(t, err) + review, found := config.AgentSettingsFor(cfg.AgentEntries(), "review") + require.True(t, found) + assert.Equal(t, config.AgentEntry{Name: "review", Effort: "low"}, review, "only the flag given is set") + + // No flags is an error before anything is written. + cmd = newAgentSetCmd() + cmd.SetArgs([]string{"review", "--fullsend-dir", dir}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + require.Error(t, cmd.Execute()) +} + +func TestRunAgentSet_RejectsOrgConfig(t *testing.T) { + dir := t.TempDir() + writeOrgConfig(t, dir, "") + err := runAgentSet(dir, "triage", agentSetFlags{model: "sonnet", modelSet: true}, ui.New(os.Stdout)) + require.Error(t, err) + assert.Contains(t, err.Error(), "per-repo configs") +} + +func TestLocalAgentEntries_FallsBackToMergedForOtherReaders(t *testing.T) { + t.Parallel() + org, err := config.ParseOrgConfig([]byte("version: \"1\"\ndispatch:\n platform: github\ndefaults:\n roles: [triage]\nrepos: {}\nagents:\n - source: harness/lint.yaml\n")) + require.NoError(t, err) + assert.Len(t, localAgentEntries(org), 1, "readers without a local view return their entries") +} diff --git a/internal/cli/github.go b/internal/cli/github.go index 59a451c855..2fea169292 100644 --- a/internal/cli/github.go +++ b/internal/cli/github.go @@ -290,22 +290,65 @@ func runGitHubSetupPerRepo(ctx context.Context, client forge.Client, printer *ui } } + // --- Existing per-repo config (re-run) --- + // A re-run must not rewrite what the repo already configured + // (agents: entries and their settings, allowlists, hand-written comments): the + // existing .fullsend/config.yaml is kept verbatim unless a flag that + // targets a config key was passed, in which case only that key is + // changed on the loaded config. Managed workflow files still refresh. + existingCfg, err := loadExistingPerRepoConfig(ctx, client, owner, repo) + if err != nil { + if !cfg.dryRun { + return err + } + // Dry runs may lack credentials for the repo; report and plan as + // a first install rather than failing before printing the plan. + printer.StepWarn("Could not read existing .fullsend/config.yaml (planning as a first install): " + err.Error()) + existingCfg = nil + } + configFlagsChanged := setupConfigFlagsChanged(cfg) + keepExistingConfig := existingCfg != nil && !configFlagsChanged + // Runtime: --runtime wins; otherwise ask once on an interactive - // terminal (Enter keeps claude). Presets carry their own value. - if cfg.runtime == "" && presetData == nil && !cfg.dryRun { + // terminal (Enter keeps claude) — but only on a first install. On a + // re-run the existing config's runtime stays unless --runtime is + // given, so an Enter cannot flip a pi repo back to claude. Presets + // carry their own value. + if cfg.runtime == "" && presetData == nil && !cfg.dryRun && existingCfg == nil { choice, err := promptRuntime(printer, os.Stdin, stdinIsInteractive()) if err != nil { return err } cfg.runtime = choice } + effectiveRuntime := cfg.runtime + if effectiveRuntime == "" && existingCfg != nil { + effectiveRuntime = existingCfg.ConfigRuntime() + } if cfg.runtime == "pi" { printer.StepWarn("runtime pi needs a sandbox image that carries pi (fullsend-sandbox/fullsend-code built from fullsend main after #6467); harnesses pinning an older image will fail at preflight") } // --- Build config files --- + // cfgYAML stays nil when the existing overlay is kept verbatim, and + // .fullsend/config.yaml is then left out of the scaffold files. var cfgYAML []byte - if presetData == nil { + switch { + case keepExistingConfig: + printer.StepInfo("Keeping existing .fullsend/config.yaml unchanged (pass --runtime, --agents, --mint-url or --inference-* to change a key)") + case existingCfg != nil: + // Re-run with config-targeting flags: change only those keys on + // the loaded config so everything else the repo set survives. + changed := applySetupFlagsToConfig(cfg, existingCfg, roles) + if err := existingCfg.Validate(); err != nil { + return fmt.Errorf("invalid config: %w", err) + } + cfgYAML, err = existingCfg.Marshal() + if err != nil { + return fmt.Errorf("marshaling per-repo config: %w", err) + } + printer.StepInfo("Updating existing .fullsend/config.yaml: " + strings.Join(changed, ", ") + " (other keys kept; comments are not preserved)") + case presetData == nil: // No preset: generate a per-repo config.yaml. Only // explicitly-set flags are written to the overlay; unset // values fall through overlay → base → code defaults @@ -337,7 +380,7 @@ func runGitHubSetupPerRepo(ctx context.Context, client forge.Client, printer *ui if err != nil { return fmt.Errorf("marshaling per-repo config: %w", err) } - } else { + default: // Preset provided: base layer carries the preset's values. // Flag-specified values go into the overlay so the base // layer remains identical to the fetched preset (ADR 0069). @@ -377,11 +420,13 @@ func runGitHubSetupPerRepo(ctx context.Context, client forge.Client, printer *ui Mode: "100644", }) } - files = append(files, forge.TreeFile{ - Path: ".fullsend/config.yaml", - Content: cfgYAML, - Mode: "100644", - }) + if cfgYAML != nil { + files = append(files, forge.TreeFile{ + Path: ".fullsend/config.yaml", + Content: cfgYAML, + Mode: "100644", + }) + } // Mint/inference values are stored in config.yaml (ADR 0069 // Decision 1). Repo variables/secrets are ALSO written for backward @@ -493,7 +538,7 @@ func runGitHubSetupPerRepo(ctx context.Context, client forge.Client, printer *ui } } - if err := applyPerRepoScaffold(ctx, client, printer, owner, repo, files, repoVars, repoSecrets, scaffoldOptions{direct: cfg.direct, signOffTrailer: signOffTrailer, runtime: cfg.runtime}); err != nil { + if err := applyPerRepoScaffold(ctx, client, printer, owner, repo, files, repoVars, repoSecrets, scaffoldOptions{direct: cfg.direct, signOffTrailer: signOffTrailer, runtime: effectiveRuntime}); err != nil { return err } @@ -514,6 +559,97 @@ func runGitHubSetupPerRepo(ctx context.Context, client forge.Client, printer *ui // base layer via the layered accessor chain (ADR 0069 Decision 1). // Returns nil when no relevant flags were changed, signaling the // caller to use the stub overlay YAML with human-readable comments. +// setupConfigFlags are the flags that target a key in .fullsend/config.yaml. +// Any of them being passed explicitly turns a re-run from "keep the file" +// into "change that key on the existing file". +var setupConfigFlags = []string{"runtime", "agents", "mint-url", "inference-provider", "inference-project", "inference-region", "inference-wif-provider"} + +// setupConfigFlagsChanged reports whether any config-targeting flag was +// passed explicitly (cobra's Changed, recorded in changedFlags — value +// comparison cannot tell --agents' non-empty default from a request). +func setupConfigFlagsChanged(cfg githubSetupConfig) bool { + for _, name := range setupConfigFlags { + if cfg.changedFlags[name] { + return true + } + } + return false +} + +// loadExistingPerRepoConfig reads the repo's current .fullsend/config.yaml +// (and config.base.yaml when present) so the parsed config carries the +// full parent chain: overlay → base → code defaults. This ensures +// ValidateAgentEntries sees the merged agent set — an overlay entry that +// tunes a custom agent registered only in config.base.yaml would +// otherwise fail with "is not a built-in agent". +// Returns (nil, nil) when config.yaml does not exist (first install) +// and an error when it exists but cannot be parsed — a re-run must not +// silently regenerate over a file the repo edited. +func loadExistingPerRepoConfig(ctx context.Context, client forge.Client, owner, repo string) (config.PerRepoConfigWriter, error) { + data, err := client.GetFileContent(ctx, owner, repo, ".fullsend/config.yaml") + if err != nil { + if forge.IsNotFound(err) { + return nil, nil + } + return nil, fmt.Errorf("reading existing .fullsend/config.yaml: %w", err) + } + if !config.IsPerRepoYAML(data) { + return nil, fmt.Errorf("existing .fullsend/config.yaml in %s/%s is not a per-repo config; fix or remove it before re-running setup", owner, repo) + } + + // Fetch the base layer when present so validation sees the merged + // agent set (an overlay entry tuning a base-registered custom agent + // needs the base's source to pass ValidateAgentEntries). + var baseData []byte + baseContent, baseErr := client.GetFileContent(ctx, owner, repo, ".fullsend/config.base.yaml") + if baseErr == nil { + baseData = baseContent + } else if !forge.IsNotFound(baseErr) { + return nil, fmt.Errorf("reading existing .fullsend/config.base.yaml: %w", baseErr) + } + + parsed, err := config.ParsePerRepoConfigWriterLayered(data, baseData) + if err != nil { + return nil, fmt.Errorf("existing .fullsend/config.yaml in %s/%s: %w — fix or remove it before re-running setup", owner, repo, err) + } + return parsed, nil +} + +// applySetupFlagsToConfig sets the keys targeted by explicitly passed +// flags on an existing config and returns the names of the keys changed. +func applySetupFlagsToConfig(cfg githubSetupConfig, w config.PerRepoConfigWriter, roles []string) []string { + var changed []string + if cfg.changedFlags["runtime"] { + w.SetRuntime(cfg.runtime) + changed = append(changed, "runtime") + } + if cfg.changedFlags["agents"] { + w.SetRoles(roles) + changed = append(changed, "roles") + } + if cfg.changedFlags["mint-url"] { + w.SetMintURL(cfg.mintURL) + changed = append(changed, "mint_url") + } + if cfg.changedFlags["inference-provider"] { + w.SetInferenceProvider(cfg.inferenceProvider) + changed = append(changed, "inference.provider") + } + if cfg.changedFlags["inference-project"] { + w.SetInferenceProject(cfg.inferenceProject) + changed = append(changed, "inference.project") + } + if cfg.changedFlags["inference-region"] { + w.SetInferenceRegion(cfg.inferenceRegion) + changed = append(changed, "inference.region") + } + if cfg.changedFlags["inference-wif-provider"] { + w.SetInferenceWIFProvider(cfg.inferenceWIFProvider) + changed = append(changed, "inference.wif_provider") + } + return changed +} + func buildPresetOverlay(cfg githubSetupConfig) config.PerRepoConfigWriter { flagNames := []string{"mint-url", "inference-provider", "inference-project", "inference-region", "inference-wif-provider"} anyChanged := false diff --git a/internal/cli/github_test.go b/internal/cli/github_test.go index 6b35a570be..2184fafdc3 100644 --- a/internal/cli/github_test.go +++ b/internal/cli/github_test.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "fmt" + "os" + "path/filepath" "strings" "testing" @@ -1491,3 +1493,230 @@ func TestRunGitHubSetupPerRepo_InvalidRuntime(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "invalid runtime") } + +// existingPerRepoConfigForRerun is a customized config as a repo would have +// after editing it by hand: a non-default runtime, per-agent settings, a +// custom agent, and a comment that a struct round-trip would drop. +const existingPerRepoConfigForRerun = `# fullsend per-repo configuration +# hand-written note: triage runs Grok on pi, code stays on Claude Code +version: "1" +runtime: pi +roles: + - triage + - coder +agents: + - source: harness/lint.yaml + - name: triage + model: xai-vertex/xai/grok-4.6 + - name: code + runtime: claude + model: sonnet +` + +func newRerunSetupClient(t *testing.T, existing string) *forge.FakeClient { + t.Helper() + client := forge.NewFakeClient() + client.AuthenticatedUser = "acme" + client.Repos = []forge.Repository{{FullName: "acme/widget", DefaultBranch: "main"}} + client.TokenScopes = []string{"repo", "workflow"} + client.Secrets = map[string]bool{ + "acme/widget/FULLSEND_GCP_PROJECT_ID": true, + "acme/widget/FULLSEND_GCP_WIF_PROVIDER": true, + } + if existing != "" { + client.FileContents = map[string][]byte{"acme/widget/.fullsend/config.yaml": []byte(existing)} + } + return client +} + +func committedScaffoldFile(client *forge.FakeClient, path string) ([]byte, bool) { + for _, batch := range client.CommittedFilesToBranch { + for _, f := range batch.Files { + if f.Path == path { + return f.Content, true + } + } + } + return nil, false +} + +func TestRunGitHubSetupPerRepo_Rerun_KeepsExistingConfigVerbatim(t *testing.T) { + t.Setenv("GH_TOKEN", "test-token") + client := newRerunSetupClient(t, existingPerRepoConfigForRerun) + var out bytes.Buffer + printer := ui.New(&out) + + // No config-targeting flag: the scaffold refreshes the managed files + // but leaves .fullsend/config.yaml out entirely, so the agents: entries, + // the runtime and the hand-written comment survive. + err := runGitHubSetupPerRepo(context.Background(), client, printer, githubSetupConfig{ + target: "acme/widget", + agents: strings.Join(config.PerRepoDefaultRoles(), ","), + changedFlags: map[string]bool{}, + }) + require.NoError(t, err) + _, present := committedScaffoldFile(client, ".fullsend/config.yaml") + assert.False(t, present, "existing config.yaml must not be rewritten on a flag-less re-run") + assert.NotEmpty(t, client.CommittedFilesToBranch, "managed scaffold files are still delivered") + assert.Contains(t, out.String(), "Keeping existing .fullsend/config.yaml") + assert.NotContains(t, out.String(), "runtime?", "no runtime prompt on a re-run") +} + +func TestRunGitHubSetupPerRepo_Rerun_ChangesOnlyTheFlaggedKey(t *testing.T) { + t.Setenv("GH_TOKEN", "test-token") + client := newRerunSetupClient(t, existingPerRepoConfigForRerun) + printer := ui.New(&discardWriter{}) + + err := runGitHubSetupPerRepo(context.Background(), client, printer, githubSetupConfig{ + target: "acme/widget", + agents: strings.Join(config.PerRepoDefaultRoles(), ","), + runtime: "claude", + changedFlags: map[string]bool{"runtime": true}, + }) + require.NoError(t, err) + content, present := committedScaffoldFile(client, ".fullsend/config.yaml") + require.True(t, present, "--runtime targets config.yaml, so it is rewritten") + cfg, err := config.ParsePerRepoConfig(content) + require.NoError(t, err) + pr := cfg.(config.PerRepoConfigReader) + assert.Equal(t, "claude", pr.ConfigRuntime(), "flagged key changed") + assert.Equal(t, []string{"triage", "coder"}, pr.ConfigRoles(), "roles kept (--agents not passed, despite its default)") + require.Len(t, pr.AgentEntries(), 3, "custom agent and per-agent settings kept") + triage, ok := config.AgentSettingsFor(pr.AgentEntries(), "triage") + require.True(t, ok, "agent settings kept") + assert.Equal(t, "xai-vertex/xai/grok-4.6", triage.Model) + code, ok := config.AgentSettingsFor(pr.AgentEntries(), "code") + require.True(t, ok) + assert.Equal(t, "sonnet", code.Model) +} + +func TestRunGitHubSetupPerRepo_Rerun_InvalidExistingConfigFails(t *testing.T) { + t.Setenv("GH_TOKEN", "test-token") + client := newRerunSetupClient(t, "version: \"1\"\nagents: {not: a, list: here}\n") + printer := ui.New(&discardWriter{}) + + err := runGitHubSetupPerRepo(context.Background(), client, printer, githubSetupConfig{ + target: "acme/widget", + agents: strings.Join(config.PerRepoDefaultRoles(), ","), + changedFlags: map[string]bool{}, + }) + require.Error(t, err, "a broken existing config is never silently regenerated") + assert.Contains(t, err.Error(), "existing .fullsend/config.yaml") + assert.Empty(t, client.CommittedFilesToBranch) +} + +func TestRunGitHubSetupPerRepo_Rerun_PresetKeepsExistingOverlay(t *testing.T) { + t.Setenv("GH_TOKEN", "test-token") + client := newRerunSetupClient(t, existingPerRepoConfigForRerun) + printer := ui.New(&discardWriter{}) + preset := filepath.Join(t.TempDir(), "preset.yaml") + require.NoError(t, os.WriteFile(preset, []byte("# fullsend per-repo configuration\nversion: \"1\"\ninference:\n region: europe-west1\n"), 0o644)) + + err := runGitHubSetupPerRepo(context.Background(), client, printer, githubSetupConfig{ + target: "acme/widget", + agents: strings.Join(config.PerRepoDefaultRoles(), ","), + configPreset: preset, + changedFlags: map[string]bool{"config": true}, + }) + require.NoError(t, err) + _, present := committedScaffoldFile(client, ".fullsend/config.yaml") + assert.False(t, present, "--config rewrites config.base.yaml; the existing overlay is kept") + _, basePresent := committedScaffoldFile(client, ".fullsend/config.base.yaml") + assert.True(t, basePresent) +} + +func TestApplySetupFlagsToConfig_EveryFlag(t *testing.T) { + t.Parallel() + w := config.NewPerRepoConfig([]string{"triage"}, "") + changed := applySetupFlagsToConfig(githubSetupConfig{ + runtime: "pi", agents: "triage,coder", mintURL: "https://mint.fullsend.sh", + inferenceProvider: "vertex", inferenceProject: "proj", inferenceRegion: "europe-west1", + inferenceWIFProvider: "projects/1/locations/global/workloadIdentityPools/p/providers/x", + changedFlags: map[string]bool{"runtime": true, "agents": true, "mint-url": true, "inference-provider": true, "inference-project": true, "inference-region": true, "inference-wif-provider": true}, + }, w, []string{"triage", "coder"}) + assert.Equal(t, []string{"runtime", "roles", "mint_url", "inference.provider", "inference.project", "inference.region", "inference.wif_provider"}, changed) + assert.Equal(t, "pi", w.ConfigRuntime()) + assert.Equal(t, []string{"triage", "coder"}, w.ConfigRoles()) + assert.Equal(t, "https://mint.fullsend.sh", w.ConfigMintURL()) + assert.Equal(t, "vertex", w.ConfigInferenceProvider()) + assert.Equal(t, "proj", w.ConfigInferenceProject()) + assert.Equal(t, "europe-west1", w.ConfigInferenceRegion()) + assert.Equal(t, "projects/1/locations/global/workloadIdentityPools/p/providers/x", w.ConfigInferenceWIFProvider()) + + // Nothing flagged: nothing changes. + before, _ := w.Marshal() + assert.Empty(t, applySetupFlagsToConfig(githubSetupConfig{changedFlags: map[string]bool{}}, w, nil)) + after, _ := w.Marshal() + assert.Equal(t, string(before), string(after)) + assert.False(t, setupConfigFlagsChanged(githubSetupConfig{changedFlags: map[string]bool{"dry-run": true}})) + assert.True(t, setupConfigFlagsChanged(githubSetupConfig{changedFlags: map[string]bool{"inference-region": true}})) +} + +func TestLoadExistingPerRepoConfig(t *testing.T) { + t.Parallel() + // Missing file: first install. + client := forge.NewFakeClient() + cfg, err := loadExistingPerRepoConfig(context.Background(), client, "acme", "widget") + require.NoError(t, err) + assert.Nil(t, cfg) + + // Org-style content in the per-repo path is refused, as is a read error. + client.FileContents = map[string][]byte{"acme/widget/.fullsend/config.yaml": []byte("version: \"1\"\ndispatch:\n platform: github\ndefaults:\n roles: [triage]\nrepos: {}\n")} + _, err = loadExistingPerRepoConfig(context.Background(), client, "acme", "widget") + require.Error(t, err) + assert.Contains(t, err.Error(), "not a per-repo config") + client.GetFileContentErrors = map[string]error{"acme/widget/.fullsend/config.yaml": fmt.Errorf("github api: 500")} + _, err = loadExistingPerRepoConfig(context.Background(), client, "acme", "widget") + require.Error(t, err) + assert.Contains(t, err.Error(), "reading existing .fullsend/config.yaml") +} + +func TestLoadExistingPerRepoConfig_WithBaseLayer(t *testing.T) { + t.Parallel() + // An overlay entry tunes a custom agent registered only in the base + // layer. Without the base, ValidateAgentEntries would reject the + // overlay entry as "not a built-in agent". + baseYAML := `version: "1" +agents: + - name: lint + source: https://raw.githubusercontent.com/acme/agents/main/harness/lint.yaml#sha256=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +allowed_remote_resources: + - https://raw.githubusercontent.com/acme/agents/ +` + overlayYAML := `version: "1" +agents: + - name: lint + effort: medium +` + client := forge.NewFakeClient() + client.FileContents = map[string][]byte{ + "acme/widget/.fullsend/config.yaml": []byte(overlayYAML), + "acme/widget/.fullsend/config.base.yaml": []byte(baseYAML), + } + cfg, err := loadExistingPerRepoConfig(context.Background(), client, "acme", "widget") + require.NoError(t, err) + require.NotNil(t, cfg) + // The merged agent list should carry the base's source on the lint entry. + agents := cfg.AgentEntries() + require.Len(t, agents, 1) + assert.Equal(t, "lint", agents[0].Name) + assert.Contains(t, agents[0].Source, "lint.yaml") + assert.Equal(t, "medium", agents[0].Effort) + + // Verify validation passes on the merged set (it would fail without the base). + require.NoError(t, cfg.Validate()) +} + +func TestLoadExistingPerRepoConfig_BaseReadError(t *testing.T) { + t.Parallel() + client := forge.NewFakeClient() + client.FileContents = map[string][]byte{ + "acme/widget/.fullsend/config.yaml": []byte("version: \"1\"\n"), + } + client.GetFileContentErrors = map[string]error{ + "acme/widget/.fullsend/config.base.yaml": fmt.Errorf("github api: 500"), + } + _, err := loadExistingPerRepoConfig(context.Background(), client, "acme", "widget") + require.Error(t, err) + assert.Contains(t, err.Error(), "reading existing .fullsend/config.base.yaml") +} diff --git a/internal/cli/run.go b/internal/cli/run.go index d8784e9408..051d797ace 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -106,14 +106,13 @@ var defaultAgentsRepoURLPrefix = "https://raw.githubusercontent.com/fullsend-ai/ // This is a transitional mechanism to support agent extraction. It will // be removed once all users have migrated to config-driven agent // registration (ADR 0058 Phase 5). -var defaultAgentsRepoKnownAgents = map[string]bool{ - "triage": true, - "code": true, - "fix": true, - "review": true, - "retro": true, - "prioritize": true, -} +var defaultAgentsRepoKnownAgents = func() map[string]bool { + known := make(map[string]bool, len(config.ValidAgentNames())) + for _, name := range config.ValidAgentNames() { + known[name] = true + } + return known +}() // statusMintToken is the test seam for minting tokens. Shared by both // setupStatusNotifier (status comment tokens) and mintAgentToken (agent @@ -193,27 +192,44 @@ var ( errResolvingRuntime = errors.New("resolving runtime") ) -func resolveBackendFromConfigData(orgConfigData []byte) (agentruntime.Backend, error) { - if isOrgConfigData(orgConfigData) { - orgCfg, orgErr := config.ParseOrgConfig(orgConfigData) +// resolveBackendFromConfigData selects the runtime for agentName from raw +// config.yaml bytes (org or per-repo). Only the single file is consulted; +// backendFromConfigFile is the layered (config.base.yaml-aware) entry point. +func resolveBackendFromConfigData(configData []byte, agentName string) (agentruntime.Backend, error) { + if isOrgConfigData(configData) { + orgCfg, orgErr := config.ParseOrgConfig(configData) if orgErr != nil { return agentruntime.Backend{}, fmt.Errorf("%w: %w", errParsingConfigRuntime, orgErr) } - backend, resolveErr := agentruntime.ResolveFromConfig(orgCfg) - if resolveErr != nil { - return agentruntime.Backend{}, fmt.Errorf("%w: %w", errResolvingRuntime, resolveErr) - } - return backend, nil + backend, _, err := resolveBackendForAgent(orgCfg.AgentEntries(), orgCfg.OrgRepoDefaults().Runtime, agentName) + return backend, err } - perRepoCfg, perRepoErr := config.ParsePerRepoConfig(orgConfigData) + perRepoCfg, perRepoErr := config.ParsePerRepoConfig(configData) if perRepoErr != nil { return agentruntime.Backend{}, fmt.Errorf("%w: %w", errParsingConfigRuntime, perRepoErr) } - backend, resolveErr := agentruntime.ResolveFromPerRepoConfig(perRepoCfg) + backend, _, err := resolveBackendForAgent(perRepoCfg.AgentEntries(), perRepoCfg.ConfigRuntime(), agentName) + return backend, err +} + +// resolveBackendForAgent applies the agents: entry's runtime for agentName +// (validated like the repo-wide key) before falling back to repoRuntime. +// The boolean reports whether the per-agent entry was the source. +func resolveBackendForAgent(agents []config.AgentEntry, repoRuntime, agentName string) (agentruntime.Backend, bool, error) { + backend, perAgent, resolveErr := agentruntime.ResolveForAgent(agents, repoRuntime, agentName) if resolveErr != nil { - return agentruntime.Backend{}, fmt.Errorf("%w: %w", errResolvingRuntime, resolveErr) + return agentruntime.Backend{}, false, fmt.Errorf("%w: %w", errResolvingRuntime, resolveErr) } - return backend, nil + return backend, perAgent, nil +} + +// agentSettingsSource is the source label for a value that came from the +// agents: entry for agentName in the config file at path; it appears in the +// plan block, the stderr selection line and metrics.json next to the +// flag/env labels. path is the effective (overlay) config file: an entry +// merged from config.base.yaml is reported through it. +func agentSettingsSource(configPath, agentName string) string { + return fmt.Sprintf("%s agents.%s", configPath, agentName) } func isOrgConfigData(data []byte) bool { @@ -239,27 +255,158 @@ func isOrgConfigData(data []byte) bool { return probe.Dispatch != nil || probe.Defaults != nil || len(probe.Repos) > 0 } -func backendFromConfigFile(path string) (agentruntime.Backend, string, error) { +// runConfig is the config file consulted by `fullsend run` for runtime +// selection and per-agent settings: the file at the requested path, or the +// sibling .fullsend/config.yaml when that is absent. Per-repo configs are +// loaded layered (config.yaml over config.base.yaml, ADR 0069) so a preset +// base can carry runtime: or agents: entries; org configs keep their raw +// bytes and are parsed by resolveBackendFromConfigData. +type runConfig struct { + // source is the file the values came from, or "" when none exists. + source string + // perRepo is the layered per-repo config; nil for org configs and + // when no file exists. + perRepo config.PerRepoConfigReader + // orgData holds the raw bytes of an org-mode config; nil otherwise. + orgData []byte +} + +// loadRunConfig reads the config for `fullsend run` (see runConfig). A +// missing file is not an error: the zero runConfig means "use defaults". +func loadRunConfig(path string) (runConfig, error) { data, readErr := os.ReadFile(path) source := path if readErr != nil && os.IsNotExist(readErr) { - alt := filepath.Join(filepath.Dir(path), ".fullsend", "config.yaml") + alt := filepath.Join(filepath.Dir(path), ".fullsend", config.OverlayConfigFile) data, readErr = os.ReadFile(alt) if readErr == nil { source = alt } } - if readErr == nil { - backend, resolveErr := resolveBackendFromConfigData(data) + if readErr != nil { + if !os.IsNotExist(readErr) { + return runConfig{source: source}, fmt.Errorf("reading config.yaml for runtime selection: %w", readErr) + } + // No overlay anywhere: a base-only directory (config.base.yaml + // without config.yaml) still counts, next to the requested path + // or under the sibling .fullsend/. + for _, dir := range []string{filepath.Dir(path), filepath.Join(filepath.Dir(path), ".fullsend")} { + base := filepath.Join(dir, config.BaseConfigFile) + if _, statErr := os.Stat(base); statErr != nil { + continue + } + cfg, loadErr := config.LoadConfig(dir, config.LoadOpts{MissingOK: false}) + if loadErr != nil { + return runConfig{source: base}, fmt.Errorf("%w: %w", errParsingConfigRuntime, loadErr) + } + if perRepoCfg, ok := cfg.(config.PerRepoConfigReader); ok { + return runConfig{source: base, perRepo: perRepoCfg}, nil + } + } + return runConfig{}, nil + } + if isOrgConfigData(data) { + return runConfig{source: source, orgData: data}, nil + } + cfg, loadErr := config.LoadConfig(filepath.Dir(source), config.LoadOpts{MissingOK: false}) + if loadErr != nil { + return runConfig{source: source}, fmt.Errorf("%w: %w", errParsingConfigRuntime, loadErr) + } + perRepoCfg, ok := cfg.(config.PerRepoConfigReader) + if !ok { + // Header said per-repo but the keys say org: parse as org. + return runConfig{source: source, orgData: data}, nil + } + return runConfig{source: source, perRepo: perRepoCfg}, nil +} + +// backendFromConfigFile selects the runtime for agentName from the config +// file at path (see loadRunConfig for which file and layering). The +// returned source names the file, suffixed with agents. +// when the per-agent entry decided, or the built-in default when no file +// exists. +func backendFromConfigFile(path, agentName string) (agentruntime.Backend, string, error) { + rc, err := loadRunConfig(path) + if err != nil { + return agentruntime.Backend{}, rc.source, err + } + return rc.backend(agentName) +} + +// backend resolves the runtime for agentName from the loaded config. +func (rc runConfig) backend(agentName string) (agentruntime.Backend, string, error) { + switch { + case rc.orgData != nil: + backend, resolveErr := resolveBackendFromConfigData(rc.orgData, agentName) if resolveErr != nil { - return agentruntime.Backend{}, source, resolveErr + return agentruntime.Backend{}, rc.source, resolveErr + } + return backend, rc.source, nil + case rc.perRepo != nil: + backend, perAgent, resolveErr := resolveBackendForAgent(rc.perRepo.AgentEntries(), rc.perRepo.ConfigRuntime(), agentName) + if resolveErr != nil { + return agentruntime.Backend{}, rc.source, resolveErr + } + source := rc.source + if perAgent { + source = agentSettingsSource(rc.source, agentName) } return backend, source, nil - } - if os.IsNotExist(readErr) { + default: return agentruntime.Default(), "default (config not found)", nil } - return agentruntime.Backend{}, source, fmt.Errorf("reading config.yaml for runtime selection: %w", readErr) +} + +// agentSettings returns the effective agents: entry for agentName from the +// loaded config, after validating every entry, so a mistyped entry (a +// name-only entry for "coder") fails the run instead of silently running +// the agent without its settings. `fullsend run` never calls Validate() on +// the config it loads, so this is where those values get checked. Missing +// files carry no entries. +func (rc runConfig) agentSettings(agentName string) (config.AgentEntry, bool, error) { + var ( + agents []config.AgentEntry + allowlist []string + ) + switch { + case rc.perRepo != nil: + agents, allowlist = rc.perRepo.AgentEntries(), rc.perRepo.AllowedResources() + case rc.orgData != nil: + orgCfg, err := config.ParseOrgConfig(rc.orgData) + if err != nil { + return config.AgentEntry{}, false, fmt.Errorf("%w: %w", errParsingConfigRuntime, err) + } + agents, allowlist = orgCfg.AgentEntries(), orgCfg.AllowedResources() + default: + return config.AgentEntry{}, false, nil + } + if len(agents) == 0 { + return config.AgentEntry{}, false, nil + } + if err := config.ValidateAgentEntries(agents, config.EnsureDefaultAllowedRemoteResources(allowlist)); err != nil { + return config.AgentEntry{}, false, fmt.Errorf("%s: %w", rc.source, err) + } + entry, found := config.AgentSettingsFor(agents, agentName) + if !found || !entry.HasSettings() { + return config.AgentEntry{}, false, nil + } + return entry, true, nil +} + +// applyAgentSettings applies the agents: entry's model/effort for the +// running agent to the composed harness, beneath the per-run flag/env +// overrides (which stay in charge when set). The entry was validated by +// agentSettings; the runtime part is applied by backendFromConfigFile. +func applyAgentSettings(h *harness.Harness, o *runOverrides, entry config.AgentEntry, agentName, configPath string) { + source := agentSettingsSource(configPath, agentName) + if o.model == "" && entry.Model != "" { + h.Model = entry.Model + o.modelSource = source + } + if o.effort == "" && entry.Effort != "" { + h.Effort = entry.Effort + o.effortSource = source + } } func newRunCmd() *cobra.Command { @@ -714,10 +861,22 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep printer.StepFail(err.Error()) return err } + // The config file is loaded once here and serves runtime selection, the + // FULLSEND_PI_MODEL gate and the agents: settings application below. + runCfg, runCfgErr := loadRunConfig(orgConfigPath) + if runCfgErr != nil { + if errors.Is(runCfgErr, errParsingConfigRuntime) { + printer.StepFail("Failed to parse config.yaml") + } else { + printer.StepFail("Failed to load config.yaml") + } + return runCfgErr + } if overrides.runtime == "" { // The pi-only FULLSEND_PI_MODEL alias depends on which runtime the - // config selects; resolve the config runtime first, then re-run. - if b, _, e := backendFromConfigFile(orgConfigPath); e == nil { + // config selects; resolve the config runtime first (including + // the agents: entry's runtime), then re-run. + if b, _, e := runCfg.backend(agentName); e == nil { overrides, err = resolveRunOverrides(oFlags, os.Getenv, b.Runtime.Name()) if err != nil { printer.StepFail(err.Error()) @@ -725,7 +884,7 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep } } } - runtimeBackend, runtimeConfigSource, runtimeErr := resolveBackend(overrides, orgConfigPath) + runtimeBackend, runtimeConfigSource, runtimeErr := resolveBackendFrom(overrides, runCfg, agentName) if runtimeErr != nil { switch { case errors.Is(runtimeErr, errParsingConfigRuntime): @@ -738,14 +897,33 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep return runtimeErr } + // Apply the agents: entry's model/effort for this agent to the composed + // harness: flag > env > agents: entry > harness. The same loaded config + // object decided the runtime above, so all three settings of an entry + // come from one place. + entry, entryFound, entryErr := runCfg.agentSettings(agentName) + if entryErr != nil { + printer.StepFail(entryErr.Error()) + return entryErr + } + if entryFound { + applyAgentSettings(h, &overrides, entry, agentName, runCfg.source) + } + // Apply the model/effort overrides to the composed harness so every // consumer (plan, runtime, metrics, status comment) sees one value. if overrides.model != "" { h.Model = overrides.model } + // provider/id is pi's model form; Claude Code takes an alias or an + // Anthropic model id. The syntax is accepted for every runtime (ids are + // not a closed set), so flag the likely mismatch instead of rejecting it. + if runtimeBackend.Runtime.Name() == "claude" && strings.Contains(h.Model, "/") { + printer.StepWarn(fmt.Sprintf("model %q has a provider/id form, which is pi's; Claude Code expects an alias (opus, sonnet, ...) or an Anthropic model id", h.Model)) + } if overrides.effort != "" { - if !harness.ValidEffort(overrides.effort) { - err := fmt.Errorf("%s: invalid effort %q: must be one of %s", overrides.effortSource, overrides.effort, strings.Join(harness.ValidEffortLevels(), ", ")) + if !config.ValidEffort(overrides.effort) { + err := fmt.Errorf("%s: invalid effort %q: must be one of %s", overrides.effortSource, overrides.effort, strings.Join(config.ValidEffortLevels(), ", ")) printer.StepFail(err.Error()) return err } @@ -4240,6 +4418,15 @@ func resolveAgentSource(ctx context.Context, fullsendDir, agentName string, forg } return "", nil, fmt.Errorf("resolving agent %q: not in config and agents-repo fallback unavailable", agentName) } + if entry.Source == "" { + // An override-only entry tunes a built-in agent (runtime/model/ + // effort) but registers no harness: the built-in still comes from + // the agents repo, exactly as if the entry were absent. + if path, deps, ok := tryAgentsRepoFallback(ctx, agentName, forgeClient, composeOpts, printer); ok { + return path, deps, nil + } + return "", nil, fmt.Errorf("resolving agent %q: config entry has no source (it only sets runtime/model/effort) and agents-repo fallback unavailable", agentName) + } if harness.IsURL(entry.Source) { printer.StepStart(fmt.Sprintf("Fetching agent harness: %s", agentName)) diff --git a/internal/cli/run_overrides.go b/internal/cli/run_overrides.go index e5e79ec6d8..8c35f9f658 100644 --- a/internal/cli/run_overrides.go +++ b/internal/cli/run_overrides.go @@ -116,10 +116,22 @@ func validateRuntimeName(name string) error { // resolveBackend returns the runtime backend for the run and a human-readable // source: the override (flag/env) when set, else the config file path (or the -// built-in default when no config exists). -func resolveBackend(o runOverrides, configPath string) (agentruntime.Backend, string, error) { +// built-in default when no config exists). When agentName is non-empty and +// no flag/env override is set, the agents: entry's runtime from the +// config file takes precedence over the repo-wide runtime: key. +func resolveBackend(o runOverrides, configPath, agentName string) (agentruntime.Backend, string, error) { + rc, err := loadRunConfig(configPath) + if err != nil { + return agentruntime.Backend{}, rc.source, err + } + return resolveBackendFrom(o, rc, agentName) +} + +// resolveBackendFrom is resolveBackend over an already loaded config, so a +// run loads config.yaml once for runtime selection and agent settings. +func resolveBackendFrom(o runOverrides, rc runConfig, agentName string) (agentruntime.Backend, string, error) { if o.runtime == "" { - return backendFromConfigFile(configPath) + return rc.backend(agentName) } backend, err := agentruntime.Resolve(o.runtime) if err != nil { diff --git a/internal/cli/run_overrides_test.go b/internal/cli/run_overrides_test.go index 9d5e2ecafc..d56bad4979 100644 --- a/internal/cli/run_overrides_test.go +++ b/internal/cli/run_overrides_test.go @@ -2,11 +2,15 @@ package cli import ( "os" + "path/filepath" "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/fullsend-ai/fullsend/internal/config" + "github.com/fullsend-ai/fullsend/internal/harness" ) func envMap(m map[string]string) func(string) string { @@ -95,12 +99,12 @@ func TestResolveBackend_OverrideWinsOverConfig(t *testing.T) { cfg := dir + "/config.yaml" require.NoError(t, os.WriteFile(cfg, []byte("# fullsend per-repo configuration\nversion: \"1\"\nruntime: claude\n"), 0o644)) - backend, source, err := resolveBackend(runOverrides{}, cfg) + backend, source, err := resolveBackend(runOverrides{}, cfg, "") require.NoError(t, err) assert.Equal(t, "claude", backend.Runtime.Name()) assert.Equal(t, cfg, source) - backend, source, err = resolveBackend(runOverrides{runtime: "pi", runtimeSource: sourceFlagRuntime}, cfg) + backend, source, err = resolveBackend(runOverrides{runtime: "pi", runtimeSource: sourceFlagRuntime}, cfg, "") require.NoError(t, err) assert.Equal(t, "pi", backend.Runtime.Name()) assert.Equal(t, sourceFlagRuntime, source) @@ -135,3 +139,268 @@ func TestEmitRunInfoNotice(t *testing.T) { emitRunInfoNotice(&out, true, runInfoFor(aggregateMetrics{}, "")) assert.Empty(t, out.String(), "nothing known, nothing emitted") } + +func TestResolveBackend_PerAgentRuntime(t *testing.T) { + t.Parallel() + dir := t.TempDir() + cfg := filepath.Join(dir, "config.yaml") + require.NoError(t, os.WriteFile(cfg, []byte(`# fullsend per-repo configuration +version: "1" +runtime: pi +roles: + - triage + - coder +agents: + - name: code + runtime: claude +`), 0o644)) + + // Without agent name: repo-wide runtime applies. + backend, source, err := resolveBackend(runOverrides{}, cfg, "") + require.NoError(t, err) + assert.Equal(t, "pi", backend.Runtime.Name()) + assert.Equal(t, cfg, source) + + // The agents: entry decides for "code", and the source names it. + backend, source, err = resolveBackend(runOverrides{}, cfg, "code") + require.NoError(t, err) + assert.Equal(t, "claude", backend.Runtime.Name()) + assert.Equal(t, cfg+" agents.code", source) + + // No entry for "triage": repo-wide applies. + backend, _, err = resolveBackend(runOverrides{}, cfg, "triage") + require.NoError(t, err) + assert.Equal(t, "pi", backend.Runtime.Name()) + + // Flag override still wins over the entry. + backend, source, err = resolveBackend(runOverrides{runtime: "dummy", runtimeSource: sourceFlagRuntime}, cfg, "code") + require.NoError(t, err) + assert.Equal(t, "dummy", backend.Runtime.Name()) + assert.Equal(t, sourceFlagRuntime, source) + + // Org configs honour agents: entries too. + orgData := []byte(`# fullsend organization configuration +version: "1" +dispatch: + platform: github +defaults: + roles: [triage] + runtime: dummy +repos: {} +agents: + - name: triage + runtime: claude +`) + backend, err = resolveBackendFromConfigData(orgData, "triage") + require.NoError(t, err) + assert.Equal(t, "claude", backend.Runtime.Name()) + backend, err = resolveBackendFromConfigData(orgData, "code") + require.NoError(t, err) + assert.Equal(t, "dummy", backend.Runtime.Name()) +} + +func TestResolveBackend_PerAgentRuntimeRejectsStub(t *testing.T) { + t.Parallel() + dir := t.TempDir() + cfg := filepath.Join(dir, "config.yaml") + require.NoError(t, os.WriteFile(cfg, []byte(`# fullsend per-repo configuration +version: "1" +agents: + - name: code + runtime: opencode +`), 0o644)) + + // `fullsend run` never calls Validate() on the config it loads, so the + // per-agent runtime must be checked against ValidRuntimes here — a stub + // runtime cannot be activated through an agents: entry any more than + // through the repo-wide key. + _, _, err := resolveBackend(runOverrides{}, cfg, "code") + require.Error(t, err) + assert.ErrorIs(t, err, errResolvingRuntime) + assert.Contains(t, err.Error(), "agents.code") + assert.Contains(t, err.Error(), `invalid runtime "opencode"`) + + backend, _, err := resolveBackend(runOverrides{}, cfg, "triage") + require.NoError(t, err) + assert.Equal(t, "claude", backend.Runtime.Name(), "other agents unaffected") +} + +func TestRunConfig_AgentSettings_LayeredAndValidated(t *testing.T) { + t.Parallel() + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.base.yaml"), []byte(`# fullsend per-repo configuration +version: "1" +runtime: claude +agents: + - name: triage + runtime: pi + model: opus +`), 0o644)) + cfg := filepath.Join(dir, "config.yaml") + require.NoError(t, os.WriteFile(cfg, []byte(`# fullsend per-repo configuration +version: "1" +agents: + - name: Triage + model: haiku +`), 0o644)) + + rc, err := loadRunConfig(cfg) + require.NoError(t, err) + + // Runtime selection reads the layered config (overlay over base, ADR + // 0069): the base entry's runtime applies, labelled through the + // effective config file. + backend, source, err := rc.backend("triage") + require.NoError(t, err) + assert.Equal(t, "pi", backend.Runtime.Name()) + assert.Equal(t, cfg+" agents.triage", source) + + // Settings merge per field across layers; lookup is case-insensitive. + entry, found, err := rc.agentSettings("triage") + require.NoError(t, err) + require.True(t, found) + assert.Equal(t, "haiku", entry.Model, "overlay wins per field") + assert.Equal(t, "pi", entry.Runtime, "base value inherited") + + // No entry: nothing to apply. + _, found, err = rc.agentSettings("code") + require.NoError(t, err) + assert.False(t, found) +} + +func TestRunConfig_AgentSettings_RejectsBadEntriesOnRunPath(t *testing.T) { + t.Parallel() + // `fullsend run` never calls Validate(); a mistyped entry must still + // fail the run for every agent rather than silently no-op — in the + // overlay and in config.base.yaml alike. + for _, layout := range []struct{ name, base, overlay string }{ + {"overlay", "", "agents:\n - name: coder\n model: sonnet\n"}, + {"base with overlay", "agents:\n - name: coder\n model: sonnet\n", ""}, + {"base only", "agents:\n - name: coder\n model: sonnet\n", ""}, + {"bad value", "", "agents:\n - name: code\n effort: turbo\n"}, + } { + t.Run(layout.name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + if layout.base != "" { + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.base.yaml"), []byte("# fullsend per-repo configuration\nversion: \"1\"\n"+layout.base), 0o644)) + } + if layout.overlay != "" { + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), []byte("# fullsend per-repo configuration\nversion: \"1\"\n"+layout.overlay), 0o644)) + } + rc, err := loadRunConfig(filepath.Join(dir, "config.yaml")) + require.NoError(t, err) + require.NotNil(t, rc.perRepo) + _, _, err = rc.agentSettings("triage") + require.Error(t, err) + assert.Contains(t, err.Error(), rc.source) + if layout.name == "bad value" { + assert.Contains(t, err.Error(), `invalid effort "turbo"`) + } else { + assert.Contains(t, err.Error(), `did you mean "code"`) + } + }) + } +} + +func TestLoadRunConfig_NestedAndBaseOnly(t *testing.T) { + t.Parallel() + // The config may live at the nested .fullsend/config.yaml, or as a + // config.base.yaml without an overlay; runtime selection and agent + // settings must both find it. + for _, layout := range []struct { + name string + dir func(string) string + file string + }{ + {"nested overlay", func(d string) string { return filepath.Join(d, ".fullsend") }, "config.yaml"}, + {"base only", func(d string) string { return d }, "config.base.yaml"}, + {"nested base only", func(d string) string { return filepath.Join(d, ".fullsend") }, "config.base.yaml"}, + } { + t.Run(layout.name, func(t *testing.T) { + t.Parallel() + root := t.TempDir() + dir := layout.dir(root) + require.NoError(t, os.MkdirAll(dir, 0o755)) + path := filepath.Join(dir, layout.file) + require.NoError(t, os.WriteFile(path, []byte(`# fullsend per-repo configuration +version: "1" +runtime: pi +agents: + - name: code + runtime: claude + model: sonnet +`), 0o644)) + rc, err := loadRunConfig(filepath.Join(root, "config.yaml")) + require.NoError(t, err) + assert.Equal(t, path, rc.source) + backend, source, err := rc.backend("code") + require.NoError(t, err) + assert.Equal(t, "claude", backend.Runtime.Name()) + assert.Equal(t, path+" agents.code", source) + entry, found, err := rc.agentSettings("code") + require.NoError(t, err) + require.True(t, found) + assert.Equal(t, "sonnet", entry.Model) + }) + } + // Missing file: defaults, no settings, no error. + none, err := loadRunConfig(filepath.Join(t.TempDir(), "config.yaml")) + require.NoError(t, err) + _, found, err := none.agentSettings("code") + require.NoError(t, err) + assert.False(t, found) + backend, source, err := none.backend("code") + require.NoError(t, err) + assert.Equal(t, "claude", backend.Runtime.Name()) + assert.Equal(t, "default (config not found)", source) +} + +func TestApplyAgentSettings(t *testing.T) { + t.Parallel() + const path = "/repo/.fullsend/config.yaml" + + h := &harness.Harness{Model: "opus"} + o := runOverrides{} + applyAgentSettings(h, &o, config.AgentEntry{Name: "triage", Model: "xai-vertex/xai/grok-4.6", Effort: "medium"}, "triage", path) + assert.Equal(t, "xai-vertex/xai/grok-4.6", h.Model) + assert.Equal(t, path+" agents.triage", o.modelSource) + assert.Equal(t, "medium", h.Effort) + assert.Equal(t, path+" agents.triage", o.effortSource) + assert.Equal(t, path+" agents.triage", modelOverrideSource(o, h.Model)) + + // Flag/env keep precedence; the caller applies o.model/o.effort next. + h = &harness.Harness{Model: "opus", Effort: "high"} + o = runOverrides{model: "sonnet", modelSource: sourceFlagModel, effort: "low", effortSource: envEffort} + applyAgentSettings(h, &o, config.AgentEntry{Name: "code", Model: "haiku", Effort: "max"}, "code", path) + assert.Equal(t, "opus", h.Model) + assert.Equal(t, "high", h.Effort) + assert.Equal(t, sourceFlagModel, o.modelSource) + assert.Equal(t, envEffort, o.effortSource) + + // Runtime-only entry leaves model and effort alone. + h = &harness.Harness{Model: "opus"} + o = runOverrides{} + applyAgentSettings(h, &o, config.AgentEntry{Name: "code", Runtime: "pi"}, "code", path) + assert.Equal(t, "opus", h.Model) + assert.Empty(t, o.modelSource) + assert.Equal(t, sourceHarness, modelOverrideSource(o, h.Model)) +} + +func TestResolveBackend_ConfigReadErrorSurfaces(t *testing.T) { + t.Parallel() + // A config path that exists but cannot be read as a file (a directory) + // is an error, not "config not found". + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, "config.yaml"), 0o755)) + _, source, err := resolveBackend(runOverrides{}, filepath.Join(dir, "config.yaml"), "triage") + require.Error(t, err) + assert.Contains(t, err.Error(), "reading config.yaml for runtime selection") + assert.Equal(t, filepath.Join(dir, "config.yaml"), source) + + // The flag path never touches the file. + backend, source, err := resolveBackendFrom(runOverrides{runtime: "nope", runtimeSource: sourceFlagRuntime}, runConfig{}, "triage") + require.Error(t, err) + assert.Equal(t, sourceFlagRuntime, source) + assert.Empty(t, backend.Runtime) +} diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index f7843e6274..0dfac4669a 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -5228,7 +5228,7 @@ repos: widget: enabled: true `) - backend, err := resolveBackendFromConfigData(data) + backend, err := resolveBackendFromConfigData(data, "") require.NoError(t, err) assert.Equal(t, "dummy", backend.Runtime.Name()) } @@ -5241,7 +5241,7 @@ func TestResolveBackendFromConfigData_PerRepoConfig(t *testing.T) { data, err := cfg.Marshal() require.NoError(t, err) - backend, err := resolveBackendFromConfigData(data) + backend, err := resolveBackendFromConfigData(data, "") require.NoError(t, err) assert.Equal(t, "dummy", backend.Runtime.Name()) } @@ -5249,7 +5249,7 @@ func TestResolveBackendFromConfigData_PerRepoConfig(t *testing.T) { func TestResolveBackendFromConfigData_Invalid(t *testing.T) { t.Parallel() - _, err := resolveBackendFromConfigData([]byte("not: [valid: yaml")) + _, err := resolveBackendFromConfigData([]byte("not: [valid: yaml"), "") require.Error(t, err) assert.Contains(t, err.Error(), "parsing config for runtime selection") } @@ -5267,7 +5267,7 @@ repos: widget: enabled: true `) - _, err := resolveBackendFromConfigData(data) + _, err := resolveBackendFromConfigData(data, "") require.Error(t, err) assert.Contains(t, err.Error(), "resolving runtime") } @@ -5289,7 +5289,7 @@ func TestIsOrgConfigData(t *testing.T) { func TestBackendFromConfigFile_MissingUsesDefault(t *testing.T) { t.Parallel() - backend, source, err := backendFromConfigFile(filepath.Join(t.TempDir(), "missing.yaml")) + backend, source, err := backendFromConfigFile(filepath.Join(t.TempDir(), "missing.yaml"), "") require.NoError(t, err) assert.Equal(t, "default (config not found)", source) assert.Equal(t, "claude", backend.Runtime.Name()) @@ -5306,7 +5306,7 @@ func TestBackendFromConfigFile_PerRepoConfig(t *testing.T) { path := filepath.Join(dir, "config.yaml") require.NoError(t, os.WriteFile(path, data, 0o644)) - backend, _, err := backendFromConfigFile(path) + backend, _, err := backendFromConfigFile(path, "") require.NoError(t, err) assert.Equal(t, "dummy", backend.Runtime.Name()) } @@ -5322,7 +5322,7 @@ func TestBackendFromConfigFile_PerRepoNestedConfig(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(dir, ".fullsend"), 0o755)) require.NoError(t, os.WriteFile(filepath.Join(dir, ".fullsend", "config.yaml"), data, 0o644)) - backend, source, err := backendFromConfigFile(filepath.Join(dir, "config.yaml")) + backend, source, err := backendFromConfigFile(filepath.Join(dir, "config.yaml"), "") require.NoError(t, err) assert.Contains(t, source, ".fullsend") assert.Equal(t, "dummy", backend.Runtime.Name()) @@ -5337,7 +5337,7 @@ func TestBackendFromConfigFile_ReadError(t *testing.T) { path := filepath.Join(t.TempDir(), "config.yaml") require.NoError(t, os.Mkdir(path, 0o755)) - _, _, err := backendFromConfigFile(path) + _, _, err := backendFromConfigFile(path, "") require.Error(t, err) assert.Contains(t, err.Error(), "reading config.yaml for runtime selection") } @@ -5359,7 +5359,7 @@ repos: path := filepath.Join(dir, "config.yaml") require.NoError(t, os.WriteFile(path, data, 0o644)) - _, _, err := backendFromConfigFile(path) + _, _, err := backendFromConfigFile(path, "") require.Error(t, err) assert.Contains(t, err.Error(), "resolving runtime") } @@ -5973,3 +5973,42 @@ func TestRunCommand_HasEventFileFlag(t *testing.T) { require.NotNil(t, flag) assert.Equal(t, "", flag.DefValue) } + +func TestResolveAgentSource_OverrideOnlyEntryUsesAgentsRepoFallback(t *testing.T) { + dir := t.TempDir() + printer := ui.New(io.Discard) + cfg, err := config.ParsePerRepoConfig([]byte(`# fullsend per-repo configuration +version: "1" +agents: + - name: triage + runtime: pi + model: sonnet +`)) + require.NoError(t, err) + + // A name-only entry registers no harness: the built-in resolves through + // the agents-repo fallback, and without a client that is reported — + // never "read .fullsend: is a directory" from an empty source path. + _, _, err = resolveAgentSource(context.Background(), dir, "triage", nil, cfg, harness.ComposeOpts{}, printer) + require.Error(t, err) + assert.Contains(t, err.Error(), "config entry has no source") + assert.Contains(t, err.Error(), "agents-repo fallback unavailable") + assert.NotContains(t, err.Error(), "is a directory") + + // A sourced entry next to it resolves locally as before. + require.NoError(t, os.MkdirAll(filepath.Join(dir, "harness"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "harness", "lint.yaml"), []byte("agent: agents/lint.md\nrole: triage\nslug: x-lint\n"), 0o644)) + cfg, err = config.ParsePerRepoConfig([]byte(`# fullsend per-repo configuration +version: "1" +agents: + - name: triage + model: sonnet + - source: harness/lint.yaml + model: haiku +`)) + require.NoError(t, err) + path, deps, err := resolveAgentSource(context.Background(), dir, "lint", nil, cfg, harness.ComposeOpts{}, printer) + require.NoError(t, err) + assert.Equal(t, filepath.Join(dir, "harness", "lint.yaml"), path) + assert.Nil(t, deps) +} diff --git a/internal/config/config.go b/internal/config/config.go index 6966b17fdc..fe0336c5bb 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -26,10 +26,60 @@ var validConfigAgentName = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_-]*$`) // explicitly set to false the agent is suppressed — this allows // disabling built-in scaffold agents without removing their role. // A suppression-only entry (Enabled=false, no Source) is valid. +// +// Runtime, Model and Effort tune how the agent runs (ADR 0091): they +// override the repo-wide runtime: key and the harness model:/effort: +// for this agent, beneath the per-run --runtime/--model/--effort flags +// and FULLSEND_* variables. An enabled entry may carry them without a +// Source — an override-only entry — when Name is a built-in agent +// (ValidAgentNames) or matches a sourced entry in a parent layer; the +// built-in keeps resolving through the agents-repo fallback. type AgentEntry struct { Name string `yaml:"name,omitempty"` - Source string `yaml:"source"` + Source string `yaml:"source,omitempty"` Enabled *bool `yaml:"enabled,omitempty"` + Runtime string `yaml:"runtime,omitempty"` + Model string `yaml:"model,omitempty"` + Effort string `yaml:"effort,omitempty"` +} + +// HasSettings reports whether the entry tunes runtime, model or effort. +func (a AgentEntry) HasSettings() bool { + return a.Runtime != "" || a.Model != "" || a.Effort != "" +} + +// IsOverrideOnly reports whether the entry only tunes an agent defined +// elsewhere: enabled, no source, at least one setting. +func (a AgentEntry) IsOverrideOnly() bool { + return a.Source == "" && a.IsEnabled() && a.HasSettings() +} + +// AgentSettingsFor returns the entry for name (case-insensitive) from an +// effective (merged) agent list, and whether one exists. Callers read +// Runtime/Model/Effort from it. +func AgentSettingsFor(agents []AgentEntry, name string) (AgentEntry, bool) { + lower := strings.ToLower(name) + for i := len(agents) - 1; i >= 0; i-- { + if strings.ToLower(agents[i].DerivedName()) == lower { + return agents[i], true + } + } + return AgentEntry{}, false +} + +// UpsertAgentSettings sets runtime/model/effort for name on a layer's +// local agent list: on the entry with that name when present, else as a +// new override-only entry. An empty value clears that setting. Returns +// the updated list. +func UpsertAgentSettings(agents []AgentEntry, name, runtime, model, effort string) []AgentEntry { + lower := strings.ToLower(name) + for i := range agents { + if strings.ToLower(agents[i].DerivedName()) == lower { + agents[i].Runtime, agents[i].Model, agents[i].Effort = runtime, model, effort + return agents + } + } + return append(agents, AgentEntry{Name: name, Runtime: runtime, Model: model, Effort: effort}) } // UnmarshalYAML implements yaml.Unmarshaler so that a plain string @@ -207,6 +257,50 @@ func ValidRuntimes() []string { return []string{"claude", "pi", "dummy"} } +// validModelRef matches a provider-qualified model reference: one or more +// segments of [A-Za-z0-9_.@-]+ joined by single forward slashes. It +// replaced the harness-local single-segment rule (#6570) and is shared +// between config validation and harness validation so both accept the +// same model identifier syntax. +// +// Examples: "opus", "sonnet", "google-vertex/gemini-3.7-flash", +// "xai-vertex/xai/grok-4.6". +// +// Rejected: "/leading", "trailing/", "a//b", empty string. +var validModelRef = regexp.MustCompile(`^[a-zA-Z0-9_.@-]+(/[a-zA-Z0-9_.@-]+)*$`) + +// ValidModelRef reports whether ref is a well-formed model reference +// (single segment or provider/id form). Exported for use by harness +// validation and per-run override validation. +func ValidModelRef(ref string) bool { + return validModelRef.MatchString(ref) +} + +// ValidAgentNames returns the built-in agents fullsend dispatches by name — +// the names an agents: entry may tune without a source. +// These are the names passed to `fullsend run ` and used by +// workflow stages. They are NOT harness role: values — "code" and +// "fix" both carry role: coder. +func ValidAgentNames() []string { + return []string{"triage", "code", "review", "fix", "retro", "prioritize"} +} + +// validEffortLevels are the reasoning effort levels accepted by the claude +// CLI's --effort flag (the version pinned by CLAUDE_CODE_VERSION in +// images/sandbox/Containerfile). The CLI also documents an "ultracode" +// value, deliberately excluded here: it starts the session in ultracode +// (multi-agent workflow) mode rather than selecting a reasoning effort +// level. The list lives in config (not harness) so config validation and +// harness validation share one source of truth without an import cycle +// (harness imports config). +var validEffortLevels = []string{"low", "medium", "high", "xhigh", "max"} + +// ValidEffortLevels returns the accepted effort values, in documentation order. +func ValidEffortLevels() []string { return slices.Clone(validEffortLevels) } + +// ValidEffort reports whether level is an accepted effort value. +func ValidEffort(level string) bool { return slices.Contains(validEffortLevels, level) } + // DefaultAgentRoles returns the standard set of agent roles installed // when no custom roles are specified. The fix stage reuses the coder // app (role: coder) so it does not need a separate app or PEM. @@ -384,6 +478,24 @@ func (c *orgConfig) Validate() error { // urlutil.MatchingAllowedPrefixInList for consistency with runtime // resolution (case-insensitive scheme, percent-decoding, dot-segment // cleaning). +// validateAgentSettings checks an entry's runtime/model/effort values. +func validateAgentSettings(i int, entry AgentEntry) error { + label := entry.Name + if label == "" { + label = entry.Source + } + if entry.Runtime != "" && !slices.Contains(ValidRuntimes(), entry.Runtime) { + return fmt.Errorf("agents[%d] (%s): invalid runtime %q: must be one of %s", i, label, entry.Runtime, strings.Join(ValidRuntimes(), ", ")) + } + if entry.Model != "" && !ValidModelRef(entry.Model) { + return fmt.Errorf("agents[%d] (%s): invalid model %q: must be a model id or provider/id (segments of a-z, A-Z, 0-9, _, -, ., @ joined by /)", i, label, entry.Model) + } + if entry.Effort != "" && !ValidEffort(entry.Effort) { + return fmt.Errorf("agents[%d] (%s): invalid effort %q: must be one of %s", i, label, entry.Effort, strings.Join(ValidEffortLevels(), ", ")) + } + return nil +} + func ValidateAgentEntries(agents []AgentEntry, allowlist []string) error { // seen tracks agent names for duplicate detection. Each state // (enabled/disabled) is tracked independently so that exactly one @@ -420,8 +532,38 @@ func ValidateAgentEntries(agents []AgentEntry, allowlist []string) error { if !entry.IsEnabled() && entry.Name == "" { return fmt.Errorf("agents[%d]: disabled agent entry must have an explicit name", i) } + if err := validateAgentSettings(i, entry); err != nil { + return err + } if entry.Source == "" { - return fmt.Errorf("agents[%d]: enabled agent entry must have a source", i) + // Override-only entry: tunes a built-in agent by name. A custom + // agent registered in a parent layer is tuned through the keyed + // merge (the merged entry carries the parent's source), so by the + // time an entry reaches validation without one it must be built in. + if !entry.HasSettings() { + return fmt.Errorf("agents[%d]: enabled agent entry must have a source (or, to tune a built-in agent, a name plus runtime, model or effort)", i) + } + if entry.Name == "" { + return fmt.Errorf("agents[%d]: agent entry without a source must name the agent it tunes", i) + } + if !validConfigAgentName.MatchString(entry.Name) { + return fmt.Errorf("agents[%d] (%s): name is invalid, must start with alphanumeric and contain only [a-zA-Z0-9_-]", i, entry.Name) + } + lowerName := strings.ToLower(entry.Name) + if !slices.Contains(ValidAgentNames(), lowerName) { + hint := "" + if suggestion, ok := roleAliasHints[lowerName]; ok { + hint = fmt.Sprintf(" (did you mean %q?)", suggestion) + } + return fmt.Errorf("agents[%d] (%s): entry without a source tunes a built-in agent, but %q is not one%s: built-in agents are %s; give a custom agent its source", i, entry.Name, entry.Name, hint, strings.Join(ValidAgentNames(), ", ")) + } + if prev, exists := seen[lowerName]; exists && prev.seenEnabled { + return fmt.Errorf("agents[%d] (%s): duplicate agent name (case-insensitive)", i, entry.Name) + } + prev := seen[lowerName] + prev.seenEnabled = true + seen[lowerName] = prev + continue } name := entry.DerivedName() @@ -596,6 +738,11 @@ const perRepoConfigHeader = `# fullsend per-repo configuration # The "runtime" key selects which agent runtime runs the agents, claude # (default when unset) or pi. For one run, the 'fullsend run --runtime' # flag wins, then FULLSEND_RUNTIME, then this file. See docs/runtimes.md. +# +# An entry in the agents list can set runtime, model and effort for one +# agent (an entry with just a name tunes a built-in agent); those win over +# the repo-wide runtime key and the harness, and per-run flags/variables +# still win over them. See docs/runtimes.md. ` // NewPerRepoConfig creates a new perRepoConfig with the given roles. @@ -827,7 +974,9 @@ func (c *perRepoConfig) Validate() error { // Agents are validated against the resolved allowlist (including // parent resources) so that URL agents covered by a parent or // default prefix pass validation. - if err := ValidateAgentEntries(c.Agents, c.AllowedResources()); err != nil { + // The merged set is validated so an overlay entry that only tunes an + // agent registered in the base layer sees that agent's source. + if err := ValidateAgentEntries(c.AgentEntries(), c.AllowedResources()); err != nil { return err } if err := validateCreateIssues(c.CreateIssues); err != nil { @@ -851,6 +1000,12 @@ func (c *perRepoConfig) Validate() error { return nil } +// roleAliasHints maps common mistakes to the correct agent name, +// used in validation error messages. +var roleAliasHints = map[string]string{ + "coder": "code", +} + func validateCreateIssues(cfg *CreateIssuesConfig) error { if cfg == nil { return nil diff --git a/internal/config/config_test.go b/internal/config/config_test.go index fec159218b..b29b0d950a 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1,6 +1,8 @@ package config import ( + "os" + "path/filepath" "strings" "testing" @@ -2478,3 +2480,228 @@ repos: assert.Contains(t, ci.AllowTargets.Repos, "acme/api") assert.Contains(t, ci.AllowTargets.Repos, "fullsend-ai/fullsend") } + +func TestValidModelRef(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + ref string + want bool + }{ + {"opus", true}, + {"sonnet", true}, + {"claude-opus-4-6", true}, + {"claude-sonnet-4-6@20250514", true}, + {"google-vertex/gemini-3.7-flash", true}, + {"xai-vertex/xai/grok-4.6", true}, + {"anthropic-vertex/claude-opus-4-6", true}, + {"", false}, + {"/leading", false}, + {"trailing/", false}, + {"a//b", false}, + {"has space", false}, + {"has$special", false}, + } { + t.Run(tc.ref, func(t *testing.T) { + assert.Equal(t, tc.want, ValidModelRef(tc.ref), "ValidModelRef(%q)", tc.ref) + }) + } +} + +func TestValidAgentNames(t *testing.T) { + names := ValidAgentNames() + assert.Contains(t, names, "triage") + assert.Contains(t, names, "code") + assert.Contains(t, names, "review") + assert.Contains(t, names, "fix") + assert.Contains(t, names, "retro") + assert.Contains(t, names, "prioritize") + // "coder" is NOT a valid agent name (it's a role name); the + // validation should hint "did you mean code" for it. + assert.NotContains(t, names, "coder") +} + +func TestValidEffort(t *testing.T) { + t.Parallel() + assert.Equal(t, []string{"low", "medium", "high", "xhigh", "max"}, ValidEffortLevels()) + for _, level := range ValidEffortLevels() { + assert.True(t, ValidEffort(level), level) + } + assert.False(t, ValidEffort("")) + assert.False(t, ValidEffort("turbo")) + assert.False(t, ValidEffort("High")) +} + +// --- per-agent settings on agents: entries (ADR 0091) --- + +func parseAgentSettingsConfig(t *testing.T, doc string) PerRepoConfigReader { + t.Helper() + cfg, err := ParsePerRepoConfig([]byte("# fullsend per-repo configuration\nversion: \"1\"\n" + doc)) + require.NoError(t, err) + return cfg.(PerRepoConfigReader) +} + +func TestAgentSettings_ParseAndValidate(t *testing.T) { + t.Parallel() + cfg := parseAgentSettingsConfig(t, `runtime: pi +agents: + - name: triage + model: xai-vertex/xai/grok-4.6 + - name: code + runtime: claude + model: sonnet + effort: high + - source: harness/lint.yaml + model: haiku +`) + require.NoError(t, cfg.(ConfigWriter).Validate()) + assert.Equal(t, "pi", cfg.ConfigRuntime()) + + triage, ok := AgentSettingsFor(cfg.AgentEntries(), "triage") + require.True(t, ok) + assert.Equal(t, "xai-vertex/xai/grok-4.6", triage.Model) + assert.Empty(t, triage.Runtime, "repo-wide runtime applies") + assert.True(t, triage.IsOverrideOnly()) + + code, ok := AgentSettingsFor(cfg.AgentEntries(), "Code") + require.True(t, ok, "lookup is case-insensitive") + assert.Equal(t, AgentEntry{Name: "code", Runtime: "claude", Model: "sonnet", Effort: "high"}, code) + + lint, ok := AgentSettingsFor(cfg.AgentEntries(), "lint") + require.True(t, ok) + assert.Equal(t, "harness/lint.yaml", lint.Source, "a sourced custom agent carries settings too") + assert.Equal(t, "haiku", lint.Model) + assert.False(t, lint.IsOverrideOnly()) + + _, ok = AgentSettingsFor(cfg.AgentEntries(), "review") + assert.False(t, ok) +} + +func TestAgentSettings_Validate(t *testing.T) { + t.Parallel() + cases := []struct { + name, doc, want string + }{ + {"unknown built-in with hint", "agents:\n - name: coder\n model: sonnet\n", `did you mean "code"`}, + {"unknown custom without source", "agents:\n - name: lint\n model: sonnet\n", "give a custom agent its source"}, + {"name-only entry without settings", "agents:\n - name: triage\n", "must have a source"}, + {"settings without a name", "agents:\n - model: sonnet\n", "must name the agent"}, + {"invalid model", "agents:\n - name: triage\n model: bad//id\n", `invalid model "bad//id"`}, + {"leading slash model", "agents:\n - name: triage\n model: /leading\n", "invalid model"}, + {"invalid runtime", "agents:\n - name: triage\n runtime: opencode\n", `invalid runtime "opencode"`}, + {"invalid effort", "agents:\n - name: triage\n effort: turbo\n", `invalid effort "turbo"`}, + {"invalid effort on sourced entry", "agents:\n - source: harness/lint.yaml\n effort: turbo\n", `invalid effort "turbo"`}, + {"duplicate built-in tuning", "agents:\n - name: triage\n model: sonnet\n - name: Triage\n model: haiku\n", "duplicate agent name"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + cfg := parseAgentSettingsConfig(t, tc.doc) + err := cfg.(ConfigWriter).Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), tc.want) + }) + } + for _, model := range []string{"opus", "claude-haiku-4-5@20251001", "google-vertex/gemini-3.7-flash", "xai-vertex/xai/grok-4.6"} { + cfg := parseAgentSettingsConfig(t, "agents:\n - name: triage\n model: "+model+"\n") + assert.NoError(t, cfg.(ConfigWriter).Validate(), model) + } +} + +func TestAgentSettings_MarshalRoundTrip(t *testing.T) { + t.Parallel() + cfg := NewPerRepoConfig([]string{"triage"}, "") + cfg.SetAgents(UpsertAgentSettings(nil, "code", "claude", "sonnet", "high")) + cfg.SetAgents(UpsertAgentSettings(cfg.AgentEntries(), "triage", "", "xai-vertex/xai/grok-4.6", "")) + require.NoError(t, cfg.Validate()) + data, err := cfg.Marshal() + require.NoError(t, err) + s := string(data) + assert.Contains(t, s, "name: code") + assert.Contains(t, s, "runtime: claude") + assert.NotContains(t, s, "source: \"\"", "override-only entries carry no source key") + + back, err := ParsePerRepoConfig(data) + require.NoError(t, err) + code, ok := AgentSettingsFor(back.AgentEntries(), "code") + require.True(t, ok) + assert.Equal(t, AgentEntry{Name: "code", Runtime: "claude", Model: "sonnet", Effort: "high"}, code) + + // Upsert replaces settings on the existing entry; empty clears. + cfg.SetAgents(UpsertAgentSettings(cfg.AgentEntries(), "CODE", "", "haiku", "")) + code, _ = AgentSettingsFor(cfg.AgentEntries(), "code") + assert.Equal(t, AgentEntry{Name: "code", Model: "haiku"}, code) + assert.Len(t, cfg.AgentEntries(), 2) +} + +func TestAgentSettings_LayeredMerge(t *testing.T) { + t.Parallel() + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.base.yaml"), []byte(`# fullsend per-repo configuration +version: "1" +runtime: pi +agents: + - source: harness/lint.yaml + model: opus + effort: high + - name: triage + model: opus +`), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(`# fullsend per-repo configuration +version: "1" +agents: + - name: lint + effort: medium + - name: Triage + runtime: claude + - name: code + model: sonnet +`), 0o644)) + cfg, err := LoadConfigWriter(dir, LoadOpts{}) + require.NoError(t, err) + // An overlay entry that only tunes a base-registered custom agent is + // valid: the merged entry carries the base's source. + require.NoError(t, cfg.Validate()) + agents := cfg.AgentEntries() + + lint, ok := AgentSettingsFor(agents, "lint") + require.True(t, ok) + assert.Equal(t, "harness/lint.yaml", lint.Source) + assert.Equal(t, "opus", lint.Model, "base model inherited (empty overlay value does not unset)") + assert.Equal(t, "medium", lint.Effort, "overlay wins per field") + + triage, ok := AgentSettingsFor(agents, "triage") + require.True(t, ok) + assert.Equal(t, "claude", triage.Runtime) + assert.Equal(t, "opus", triage.Model) + + code, ok := AgentSettingsFor(agents, "code") + require.True(t, ok) + assert.Equal(t, "sonnet", code.Model) + + // A bad entry in the base layer is caught by Validate on the overlay. + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.base.yaml"), []byte("# fullsend per-repo configuration\nversion: \"1\"\nagents:\n - name: coder\n model: sonnet\n"), 0o644)) + cfg, err = LoadConfigWriter(dir, LoadOpts{}) + require.NoError(t, err) + err = cfg.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), `did you mean "code"`) +} + +func TestAgentSettings_DisabledEntryStillValid(t *testing.T) { + t.Parallel() + cfg := parseAgentSettingsConfig(t, "agents:\n - name: retro\n enabled: false\n") + require.NoError(t, cfg.(ConfigWriter).Validate()) + assert.True(t, IsAgentExplicitlyDisabled(cfg.AgentEntries(), "retro")) +} + +func TestPerRepoConfig_LocalAgentEntries(t *testing.T) { + t.Parallel() + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.base.yaml"), []byte("# fullsend per-repo configuration\nversion: \"1\"\nagents:\n - source: harness/lint.yaml\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "config.yaml"), []byte("# fullsend per-repo configuration\nversion: \"1\"\nagents:\n - name: code\n model: sonnet\n"), 0o644)) + cfg, err := LoadConfig(dir, LoadOpts{}) + require.NoError(t, err) + local := cfg.(interface{ LocalAgentEntries() []AgentEntry }).LocalAgentEntries() + assert.Equal(t, []AgentEntry{{Name: "code", Model: "sonnet"}}, local, "only the overlay's own entries") + assert.Len(t, cfg.AgentEntries(), 2, "merged view includes the base") +} diff --git a/internal/config/defaults_test.go b/internal/config/defaults_test.go index 64744c3702..77a03863c8 100644 --- a/internal/config/defaults_test.go +++ b/internal/config/defaults_test.go @@ -654,6 +654,42 @@ roles: assert.Equal(t, "claude", pcr.ConfigRuntime()) } +func TestParsePerRepoConfigWriterLayered_MergesBase(t *testing.T) { + baseYAML := `version: "1" +agents: + - name: lint + source: https://raw.githubusercontent.com/acme/agents/main/harness/lint.yaml#sha256=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +allowed_remote_resources: + - https://raw.githubusercontent.com/acme/agents/ +` + overlayYAML := `version: "1" +agents: + - name: lint + effort: medium +` + cfg, err := ParsePerRepoConfigWriterLayered([]byte(overlayYAML), []byte(baseYAML)) + require.NoError(t, err) + agents := cfg.AgentEntries() + require.Len(t, agents, 1) + assert.Equal(t, "lint", agents[0].Name) + assert.Contains(t, agents[0].Source, "lint.yaml") + assert.Equal(t, "medium", agents[0].Effort) + // Validation must pass on the merged set. + require.NoError(t, cfg.Validate()) +} + +func TestParsePerRepoConfigWriterLayered_NilBase(t *testing.T) { + overlayYAML := `version: "1" +roles: + - triage +` + cfg, err := ParsePerRepoConfigWriterLayered([]byte(overlayYAML), nil) + require.NoError(t, err) + assert.Equal(t, []string{"triage"}, cfg.ConfigRoles()) + // Code defaults surface as the parent. + assert.Equal(t, "claude", cfg.ConfigRuntime()) +} + // --- Existing single-file behavior unchanged --- func TestPerRepoConfig_ExistingSingleFileBehavior(t *testing.T) { diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go index 1b010f2697..8df3e49a85 100644 --- a/internal/config/interfaces.go +++ b/internal/config/interfaces.go @@ -270,6 +270,18 @@ func (c *perRepoConfig) AgentEntries() []AgentEntry { if oi.entry.Name != "" { merged.Name = oi.entry.Name } + // Per-agent settings merge field by field; an empty value + // inherits the parent's (there is no way to unset a parent's + // value from the overlay short of restating the entry). + if oi.entry.Runtime != "" { + merged.Runtime = oi.entry.Runtime + } + if oi.entry.Model != "" { + merged.Model = oi.entry.Model + } + if oi.entry.Effort != "" { + merged.Effort = oi.entry.Effort + } } result = append(result, merged) } @@ -284,6 +296,10 @@ func (c *perRepoConfig) AgentEntries() []AgentEntry { return result } +// LocalAgentEntries returns only this layer's own agents: entries (what +// Marshal writes), as opposed to the merged set from AgentEntries. +func (c *perRepoConfig) LocalAgentEntries() []AgentEntry { return c.Agents } + // IsKillSwitchActive reports whether the kill switch is engaged. // KillSwitch is a *bool: nil falls through to parent, non-nil // (including explicit false) is the local decision. @@ -534,6 +550,12 @@ func (c *perRepoConfig) ensureInference() *PerRepoInferenceConfig { // --- LoadConfig / LoadConfigWriter factories --- +// Layer file names of a per-repo config directory (ADR 0069). +const ( + OverlayConfigFile = "config.yaml" + BaseConfigFile = "config.base.yaml" +) + // LoadOpts controls how LoadConfig handles a missing config.yaml. type LoadOpts struct { // MissingOK returns a default config when config.yaml is absent. @@ -620,8 +642,8 @@ func LoadConfigWriter(dir string, opts LoadOpts) (ConfigWriter, error) { // Returns data and existence flags for each file. Genuine I/O errors // (not "file not found") are returned immediately. func readConfigFiles(dir string) (overlayData []byte, haveOverlay bool, baseData []byte, haveBase bool, err error) { - overlayData, overlayErr := os.ReadFile(filepath.Join(dir, "config.yaml")) - baseData, baseErr := os.ReadFile(filepath.Join(dir, "config.base.yaml")) + overlayData, overlayErr := os.ReadFile(filepath.Join(dir, OverlayConfigFile)) + baseData, baseErr := os.ReadFile(filepath.Join(dir, BaseConfigFile)) if overlayErr != nil && !os.IsNotExist(overlayErr) { return nil, false, nil, false, fmt.Errorf("reading config: %w", overlayErr) @@ -632,6 +654,17 @@ func readConfigFiles(dir string) (overlayData []byte, haveOverlay bool, baseData return overlayData, overlayErr == nil, baseData, baseErr == nil, nil } +// ParsePerRepoConfigWriterLayered parses overlay and optional base YAML +// bytes into a PerRepoConfigWriter with the parent chain +// overlay → base → code defaults. When baseData is nil the result is +// equivalent to ParsePerRepoConfigWriter (parent = code defaults only). +// This is the preferred entry point when both layers are available as +// raw bytes (e.g. fetched from a forge API) rather than on the local +// filesystem (where LoadConfigWriter should be used instead). +func ParsePerRepoConfigWriterLayered(overlayData []byte, baseData []byte) (PerRepoConfigWriter, error) { + return loadPerRepoLayers(overlayData, true, baseData, len(baseData) > 0) +} + // loadPerRepoLayers parses per-repo config layers and wires the parent // chain: overlay → base → code defaults. When haveOverlay is false an // empty overlay is created so writes target config.yaml. The returned diff --git a/internal/harness/forge.go b/internal/harness/forge.go index 4f01364ce2..a1569e2e04 100644 --- a/internal/harness/forge.go +++ b/internal/harness/forge.go @@ -458,8 +458,9 @@ func BuildConfigMap(cfg config.ConfigReader) map[string]any { if agents := pr.AgentEntries(); len(agents) > 0 { anyAgents := make([]any, len(agents)) for i, a := range agents { - agentMap := map[string]any{ - "source": a.Source, + agentMap := map[string]any{} + if a.Source != "" { + agentMap["source"] = a.Source } if a.Name != "" { agentMap["name"] = a.Name @@ -467,6 +468,15 @@ func BuildConfigMap(cfg config.ConfigReader) map[string]any { if a.Enabled != nil { agentMap["enabled"] = *a.Enabled } + if a.Runtime != "" { + agentMap["runtime"] = a.Runtime + } + if a.Model != "" { + agentMap["model"] = a.Model + } + if a.Effort != "" { + agentMap["effort"] = a.Effort + } anyAgents[i] = agentMap } m["agents"] = anyAgents diff --git a/internal/harness/forge_test.go b/internal/harness/forge_test.go index f466aeed64..9698a8d07c 100644 --- a/internal/harness/forge_test.go +++ b/internal/harness/forge_test.go @@ -1614,3 +1614,46 @@ func TestValidateOverlayForgeConfig_SecondIndex(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "overlays[1].pre_script must be a local path, not a URL") } + +func TestBuildConfigMap_AgentSettings(t *testing.T) { + t.Parallel() + cfg, err := config.ParsePerRepoConfig([]byte(`# fullsend per-repo configuration +version: "1" +runtime: pi +agents: + - name: triage + model: xai-vertex/xai/grok-4.6 + - source: harness/lint.yaml + runtime: claude + effort: high +`)) + require.NoError(t, err) + m := BuildConfigMap(cfg) + require.NotNil(t, m) + assert.Equal(t, []any{ + map[string]any{"name": "triage", "model": "xai-vertex/xai/grok-4.6"}, + map[string]any{"source": "harness/lint.yaml", "runtime": "claude", "effort": "high"}, + }, m["agents"]) +} + +func TestRegisteredAgents_SkipsOverrideOnlyEntries(t *testing.T) { + t.Parallel() + cfg, err := config.ParsePerRepoConfig([]byte(`# fullsend per-repo configuration +version: "1" +agents: + - name: triage + model: sonnet + - source: harness/lint.yaml + model: haiku + - name: retro + enabled: false +`)) + require.NoError(t, err) + registered, err := RegisteredAgents(cfg) + require.NoError(t, err) + // Only the sourced custom harness is a registered agent; the built-in + // tuning entry and the disable-only entry are not enumerated. + require.Len(t, registered, 1) + assert.Equal(t, "lint", registered[0].Name) + assert.Equal(t, "haiku", registered[0].Entry.Model) +} diff --git a/internal/harness/harness.go b/internal/harness/harness.go index 570c255061..306eadadae 100644 --- a/internal/harness/harness.go +++ b/internal/harness/harness.go @@ -5,18 +5,17 @@ import ( "os" "path/filepath" "regexp" - "slices" "strings" "gopkg.in/yaml.v3" + "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/internal/forge" "github.com/fullsend-ai/fullsend/internal/urlutil" ) var ( validAgentName = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) - validModelName = regexp.MustCompile(`^[a-zA-Z0-9_.@-]+$`) validPluginName = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) validProviderName = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) // validRoleName mirrors mintcore.RolePattern — duplicated to avoid coupling harness→mintcore. @@ -26,27 +25,17 @@ var ( ) // validEffortLevels are the reasoning effort levels accepted by the claude -// CLI's --effort flag, verified against @anthropic-ai/claude-code 2.1.226 -// (the version pinned in images/sandbox/Containerfile). The CLI also accepts -// the undocumented "ultracode" keyword, which is deliberately excluded here: -// it opts sessions into multi-agent workflow orchestration rather than -// selecting a reasoning effort level. -var validEffortLevels = []string{"low", "medium", "high", "xhigh", "max"} +// CLI's --effort flag. The canonical list lives in config.ValidEffortLevels +// so harness effort:, per-run --effort/FULLSEND_EFFORT and agents: entry +// effort all validate identically. +var validEffortLevels = config.ValidEffortLevels() // validEffortLevel reports whether level is a recognized reasoning effort level // for the claude CLI's --effort flag. func validEffortLevel(level string) bool { - return slices.Contains(validEffortLevels, level) + return config.ValidEffort(level) } -// ValidEffort reports whether level is an accepted effort value; exported so -// per-run overrides (--effort, FULLSEND_EFFORT) are validated the same way -// as the harness field. -func ValidEffort(level string) bool { return validEffortLevel(level) } - -// ValidEffortLevels returns the accepted effort values, in documentation order. -func ValidEffortLevels() []string { return slices.Clone(validEffortLevels) } - // ValidPluginBasename reports whether name matches the allowed plugin name pattern. func ValidPluginBasename(name string) bool { return validPluginName.MatchString(name) @@ -462,8 +451,8 @@ func (h *Harness) Validate() error { return fmt.Errorf("agent name %q contains invalid characters (allowed: a-z, A-Z, 0-9, _, -)", agentBase) } } - if h.Model != "" && !validModelName.MatchString(h.Model) { - return fmt.Errorf("model %q contains invalid characters (allowed: a-z, A-Z, 0-9, _, -, ., @)", h.Model) + if h.Model != "" && !config.ValidModelRef(h.Model) { + return fmt.Errorf("model %q contains invalid characters (allowed: segments of a-z, A-Z, 0-9, _, -, ., @ joined by /)", h.Model) } if h.Effort != "" && !validEffortLevel(h.Effort) { return fmt.Errorf("effort %q is not valid (allowed: %s)", h.Effort, strings.Join(validEffortLevels, ", ")) diff --git a/internal/harness/harness_test.go b/internal/harness/harness_test.go index 99e6efabad..1d977fa048 100644 --- a/internal/harness/harness_test.go +++ b/internal/harness/harness_test.go @@ -673,12 +673,28 @@ func TestValidate_ModelValid(t *testing.T) { "claude-sonnet-4-6@default", "claude-sonnet-4-6@20250514", "claude-opus-4-1@20250805", + "google-vertex/gemini-3.7-flash", + "xai-vertex/xai/grok-4.6", + "anthropic-vertex/claude-opus-4-6", } { h := &Harness{Agent: "agents/test.md", Role: "test", Model: model} require.NoError(t, h.Validate(), "model %q should be valid", model) } } +func TestValidate_ModelInvalid_MalformedSlash(t *testing.T) { + for _, model := range []string{ + "/leading", + "trailing/", + "a//b", + } { + h := &Harness{Agent: "agents/test.md", Role: "test", Model: model} + err := h.Validate() + require.Error(t, err, "model %q should be invalid", model) + assert.Contains(t, err.Error(), "invalid characters") + } +} + func TestValidate_EffortValid(t *testing.T) { for _, level := range []string{"low", "medium", "high", "xhigh", "max"} { h := &Harness{Agent: "agents/test.md", Role: "test", Effort: level} diff --git a/internal/harness/registry.go b/internal/harness/registry.go index e024c79272..b4d5d6d657 100644 --- a/internal/harness/registry.go +++ b/internal/harness/registry.go @@ -45,6 +45,12 @@ func RegisteredAgents(cfg config.ConfigReader) ([]RegisteredAgent, error) { if !entry.IsEnabled() { continue } + // An override-only entry tunes a built-in agent (runtime/model/ + // effort) but registers no harness of its own: the built-in stage + // runs it, so it is not a custom harness to enumerate or lock. + if entry.IsOverrideOnly() { + continue + } out = append(out, RegisteredAgent{ Entry: entry, Name: entry.DerivedName(), diff --git a/internal/repos/scaffold_metadata.go b/internal/repos/scaffold_metadata.go index 751f05acf8..1920659b20 100644 --- a/internal/repos/scaffold_metadata.go +++ b/internal/repos/scaffold_metadata.go @@ -141,5 +141,7 @@ func RuntimeSection(runtime string) string { fmt.Sprintf("Agents in this repository run on **%s**", runtime) + " (`runtime:` in `.fullsend/config.yaml`). To change it later, edit that key, " + "re-run `fullsend github setup --runtime `, or override a " + - "single run with `fullsend run --runtime`. See https://github.com/fullsend-ai/fullsend/blob/main/docs/runtimes.md." + "single run with `fullsend run --runtime`. To put one agent on another runtime or model, " + + "set runtime/model/effort on its `agents:` entry in the same file (`fullsend agent set --runtime pi`). " + + "See https://github.com/fullsend-ai/fullsend/blob/main/docs/runtimes.md." } diff --git a/internal/repos/scaffold_metadata_test.go b/internal/repos/scaffold_metadata_test.go index 58c85eeb20..2b1ceb920a 100644 --- a/internal/repos/scaffold_metadata_test.go +++ b/internal/repos/scaffold_metadata_test.go @@ -178,5 +178,6 @@ func TestRuntimeSection(t *testing.T) { assert.Contains(t, RuntimeSection("pi"), "run on **pi**") assert.Contains(t, def, "`runtime:` in `.fullsend/config.yaml`") assert.Contains(t, def, "fullsend run --runtime") + assert.Contains(t, def, "`agents:` entry") assert.True(t, strings.HasPrefix(def, "\n\n"), "section must be appended after the body with a paragraph break") } diff --git a/internal/runtime/pi_run.go b/internal/runtime/pi_run.go index 4014d507b7..3ec10f872e 100644 --- a/internal/runtime/pi_run.go +++ b/internal/runtime/pi_run.go @@ -14,9 +14,10 @@ import ( "github.com/fullsend-ai/fullsend/internal/ui" ) -// Model selection for pi. Harness `model:` is validated by validModelName -// (no "/"), so the Claude-style aliases the fleet uses are mapped onto pi's -// `provider/id` form here. The ids are pi 0.84.2's Anthropic catalog +// Model selection for pi. The fleet's harnesses name Claude-style aliases +// (opus, sonnet, ...), which are mapped onto pi's `provider/id` form here; a +// harness or agents: entry may also give `provider/id` directly. The +// ids are pi 0.84.2's Anthropic catalog // (packages/ai/src/providers/data/anthropic.json), which the vendored // anthropic-vertex extension registers verbatim; whether Vertex accepts each // id is a lifecycle-test item (docs/runtimes.md). Both the provider and the @@ -92,9 +93,9 @@ func translatePiModel(model string) string { // "xai-vertex/xai/grok-4.6" (any case) -> "xai-vertex/xai/grok-4.6" // "grok-4.6" with FULLSEND_PI_PROVIDER=xai-vertex -> "xai-vertex/xai/grok-4.6" // -// The third matters because harness `model:` cannot contain a slash -// (validModelName), so selecting this provider from a harness means a bare -// id plus the provider env var. Left alone it would render the two-segment +// The third matters because a harness may still select this provider with +// a bare id plus the provider env var (the only way before harness `model:` +// accepted "/", #6570). Left alone it would render the two-segment // "xai-vertex/grok-4.6", which the extension does not register — pi then // substitutes a fallback model with the wrong wire id and only warns. func normalizeXaiVertexModel(provider, model string) (string, bool) { diff --git a/internal/runtime/pi_run_test.go b/internal/runtime/pi_run_test.go index 9174b41de9..db0bce4251 100644 --- a/internal/runtime/pi_run_test.go +++ b/internal/runtime/pi_run_test.go @@ -43,9 +43,9 @@ func TestTranslatePiModel(t *testing.T) { } // A bare id under FULLSEND_PI_PROVIDER=xai-vertex must still get the - // publisher segment. Harness `model:` cannot contain a slash - // (validModelName), so this is the only way a harness reaches Grok -- - // and the two-segment "xai-vertex/grok-4.6" is a model the extension + // publisher segment. Before harness `model:` accepted "/" (#6570) this + // was the only way a harness reached Grok, and it stays supported -- + // the two-segment "xai-vertex/grok-4.6" is a model the extension // does not register, which pi silently substitutes a fallback for. t.Setenv(piProviderEnv, piXaiVertexProvider) assert.Equal(t, "xai-vertex/xai/grok-4.6", translatePiModel("grok-4.6"), "bare id gets the publisher segment too") @@ -249,9 +249,9 @@ func TestBuildPiRunCommand_XaiVertex(t *testing.T) { "XAI_API_KEY is unset after .env is sourced") } -// TestTranslatePiModel_XaiVertexBareIDFromHarness covers the harness path: -// validModelName forbids "/" in harness `model:`, so a harness selecting -// this provider must use a bare id plus FULLSEND_PI_PROVIDER. +// TestTranslatePiModel_XaiVertexBareIDFromHarness covers the legacy harness +// path: a bare id plus FULLSEND_PI_PROVIDER, which predates harness `model:` +// accepting "/" (#6570) and must keep working. func TestTranslatePiModel_XaiVertexBareIDFromHarness(t *testing.T) { t.Setenv(piProviderEnv, piXaiVertexProvider) for _, bare := range []string{"grok-4.6", "grok-4.5"} { diff --git a/internal/runtime/registry.go b/internal/runtime/registry.go index 7295b73cf4..efffafd7c1 100644 --- a/internal/runtime/registry.go +++ b/internal/runtime/registry.go @@ -59,6 +59,36 @@ func ResolveFromPerRepoConfig(cfg config.PerRepoConfigReader) (Backend, error) { return Resolve(rt) } +// ResolveForAgent selects the runtime backend for one agent: the agents: +// entry's runtime when set, else repoRuntime (the repo-wide runtime: key, +// "claude" when empty). The boolean reports whether the per-agent entry +// decided. Both values are validated against [config.ValidRuntimes] so an +// agents: entry cannot activate a stub runtime any more than the repo-wide +// key can. +func ResolveForAgent(agents []config.AgentEntry, repoRuntime, agent string) (Backend, bool, error) { + if agent != "" { + if entry, ok := config.AgentSettingsFor(agents, agent); ok && entry.Runtime != "" { + if err := validateConfigRuntime(entry.Runtime); err != nil { + return Backend{}, false, fmt.Errorf("agents.%s: %w", entry.DerivedName(), err) + } + backend, err := Resolve(entry.Runtime) + if err != nil { + return Backend{}, false, err + } + return backend, true, nil + } + } + rt := repoRuntime + if rt == "" { + rt = "claude" + } + if err := validateConfigRuntime(rt); err != nil { + return Backend{}, false, err + } + backend, err := Resolve(rt) + return backend, false, err +} + // validateConfigRuntime checks that rt is in the set of user-facing // runtimes allowed in config files. Stub runtimes (e.g. "opencode") // are intentionally excluded from [config.ValidRuntimes] so they diff --git a/internal/runtime/registry_test.go b/internal/runtime/registry_test.go index a79950078c..45d276cd12 100644 --- a/internal/runtime/registry_test.go +++ b/internal/runtime/registry_test.go @@ -122,3 +122,58 @@ repos: {} assert.Contains(t, err.Error(), "invalid runtime") } } + +func TestResolveForAgent(t *testing.T) { + t.Parallel() + cfg, err := config.ParsePerRepoConfig([]byte(`# fullsend per-repo configuration +version: "1" +runtime: pi +agents: + - name: code + runtime: claude + - name: fix + model: sonnet +`)) + require.NoError(t, err) + agents := cfg.AgentEntries() + + // The agents: entry's runtime wins over the repo-wide key. + backend, perAgent, err := ResolveForAgent(agents, cfg.(config.PerRepoConfigReader).ConfigRuntime(), "code") + require.NoError(t, err) + assert.Equal(t, "claude", backend.Runtime.Name()) + assert.True(t, perAgent) + + // An entry without runtime falls back to the repo-wide key; so does a + // missing entry or a missing agent name. + for _, agent := range []string{"fix", "triage", ""} { + backend, perAgent, err = ResolveForAgent(agents, "pi", agent) + require.NoError(t, err, agent) + assert.Equal(t, "pi", backend.Runtime.Name(), agent) + assert.False(t, perAgent, agent) + } + + // No entries and no repo-wide value: the code default. + backend, perAgent, err = ResolveForAgent(nil, "", "code") + require.NoError(t, err) + assert.Equal(t, "claude", backend.Runtime.Name()) + assert.False(t, perAgent) +} + +func TestResolveForAgent_RejectsStubRuntimes(t *testing.T) { + t.Parallel() + // A per-agent value is validated like the repo-wide key: stub runtimes + // (opencode) and unknown names cannot be activated through config. + for _, name := range []string{"opencode", "invalid"} { + agents := []config.AgentEntry{{Name: "code", Runtime: name}} + _, _, err := ResolveForAgent(agents, "pi", "code") + require.Error(t, err, name) + assert.Contains(t, err.Error(), "agents.code") + assert.Contains(t, err.Error(), "invalid runtime") + + backend, _, err := ResolveForAgent(agents, "pi", "triage") + require.NoError(t, err) + assert.Equal(t, "pi", backend.Runtime.Name(), "other agents unaffected") + } + _, _, err := ResolveForAgent(nil, "opencode", "code") + require.Error(t, err, "repo-wide stub runtime is rejected too") +} diff --git a/pkg/behaviourtest/steps/dispatch.go b/pkg/behaviourtest/steps/dispatch.go index cf09034305..df6e015b44 100644 --- a/pkg/behaviourtest/steps/dispatch.go +++ b/pkg/behaviourtest/steps/dispatch.go @@ -12,6 +12,7 @@ import ( "gopkg.in/yaml.v3" "github.com/fullsend-ai/fullsend/internal/config" + "github.com/fullsend-ai/fullsend/internal/scaffold" "github.com/fullsend-ai/fullsend/pkg/behaviourtest/world" ) @@ -227,8 +228,10 @@ func commitLocalHarnessResources(ctx context.Context, w *world.World, harnessNam return fmt.Errorf("no repo configured; call 'Given the enrolled test repository' before harness operations") } var h struct { - Agent string `yaml:"agent"` - Policy string `yaml:"policy"` + Agent string `yaml:"agent"` + Policy string `yaml:"policy"` + Profiles []string `yaml:"profiles"` + Providers []string `yaml:"providers"` } if err := yaml.Unmarshal([]byte(doc), &h); err != nil { return fmt.Errorf("parsing harness YAML for resource paths: %w", err) @@ -256,6 +259,33 @@ func commitLocalHarnessResources(ctx context.Context, w *world.World, harnessNam } } + // Relative profiles/providers entries grant the sandbox network egress + // (ADR-0065), so a placeholder would not do: commit the real files the + // per-repo scaffold embeds (e.g. profiles/fullsend-vertex-ai.yaml, + // providers/vertex-ai.yaml). The scaffold install ships only .gitkeeps + // for these directories, and a local harness resolves them relative to + // .fullsend/, so a scenario that needs a real model must reference them + // and they must exist. Entries the scaffold does not carry are an error. + for _, group := range []struct { + field string + paths []string + }{{"profiles", h.Profiles}, {"providers", h.Providers}} { + for _, rel := range group.paths { + if rel == "" || strings.HasPrefix(rel, "/") || strings.HasPrefix(rel, "https://") { + continue + } + data, err := scaffold.FullsendRepoFile(rel) + if err != nil { + return fmt.Errorf("%s entry %q for %s is not a file the per-repo scaffold ships: %w", group.field, rel, harnessName, err) + } + dest := filepath.Join(".fullsend", rel) + if err := w.SCM.CommitFile(ctx, owner, repo, dest, + fmt.Sprintf("behaviour: add %s resource for %s", group.field, harnessName), data); err != nil { + return fmt.Errorf("committing %s resource %s: %w", group.field, dest, err) + } + } + } + return nil } diff --git a/pkg/behaviourtest/steps/dispatch_test.go b/pkg/behaviourtest/steps/dispatch_test.go index 54f05fcd76..18a180f4fa 100644 --- a/pkg/behaviourtest/steps/dispatch_test.go +++ b/pkg/behaviourtest/steps/dispatch_test.go @@ -466,3 +466,31 @@ func TestCommitLocalHarnessResources_InvalidYAML(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "parsing harness YAML") } + +func TestCommitLocalHarnessResources_CommitsScaffoldProfilesAndProviders(t *testing.T) { + scm := &fakeURLSCM{files: map[string][]byte{}} + w := &world.World{Org: "org", RepoName: "repo", SCM: scm} + err := commitLocalHarnessResources(context.Background(), w, "pi-override", + "agent: agents/pi-override.md\nrole: triage\nprofiles:\n - profiles/fullsend-vertex-ai.yaml\nproviders:\n - providers/vertex-ai.yaml\n") + require.NoError(t, err) + + // The real scaffold files are committed, not placeholders: these grant + // the sandbox its Vertex egress. + profile := string(scm.files["org/repo/.fullsend/profiles/fullsend-vertex-ai.yaml"]) + assert.Contains(t, profile, "*.googleapis.com") + provider := string(scm.files["org/repo/.fullsend/providers/vertex-ai.yaml"]) + assert.Contains(t, provider, "type: fullsend-vertex-ai") + + // An entry the scaffold does not ship is an error, not a silent gap. + err = commitLocalHarnessResources(context.Background(), w, "bad", + "agent: agents/bad.md\nrole: triage\nproviders:\n - providers/nope.yaml\n") + require.Error(t, err) + assert.Contains(t, err.Error(), "providers/nope.yaml") + + // URL entries are left to the harness resolver. + scm2 := &fakeURLSCM{files: map[string][]byte{}} + w2 := &world.World{Org: "org", RepoName: "repo", SCM: scm2} + require.NoError(t, commitLocalHarnessResources(context.Background(), w2, "url", + "agent: https://example.com/a.md\nrole: triage\nprofiles:\n - https://example.com/p.yaml#sha256=abc\n")) + assert.Empty(t, scm2.files) +} diff --git a/pkg/behaviourtest/steps/runtime.go b/pkg/behaviourtest/steps/runtime.go index 74991b801f..6c8406342b 100644 --- a/pkg/behaviourtest/steps/runtime.go +++ b/pkg/behaviourtest/steps/runtime.go @@ -13,6 +13,7 @@ import ( "strings" "github.com/cucumber/godog" + "gopkg.in/yaml.v3" "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/pkg/behaviourtest/artifacts" @@ -35,9 +36,18 @@ func registerRuntimeSteps(sc *godog.ScenarioContext) { sc.Step(`^a pi agent "([^"]+)" defined as:$`, func(ctx context.Context, name, doc string) (context.Context, error) { return ctx, givenPiAgent(world.FromContext(ctx), name, doc) }) + sc.Step(`^the repository agents are configured with:$`, func(ctx context.Context, doc string) (context.Context, error) { + return ctx, givenRepositoryAgentSettings(world.FromContext(ctx), doc) + }) sc.Step(`^the run selected the "([^"]+)" runtime$`, func(ctx context.Context, name string) (context.Context, error) { return ctx, assertRunSelectedRuntime(world.FromContext(ctx), name) }) + sc.Step(`^the run selected the "([^"]+)" runtime from "([^"]+)"$`, func(ctx context.Context, name, source string) (context.Context, error) { + return ctx, assertRunSelectedRuntimeFrom(world.FromContext(ctx), name, source) + }) + sc.Step(`^the run requested model "([^"]+)" from "([^"]+)" and the provider reported a "([^"]+)" model$`, func(ctx context.Context, requested, source, reported string) (context.Context, error) { + return ctx, assertRunModelFrom(world.FromContext(ctx), requested, source, reported) + }) sc.Step(`^the run metrics report tokens$`, func(ctx context.Context) (context.Context, error) { return ctx, assertRunMetricsReportTokens(world.FromContext(ctx)) }) @@ -162,6 +172,68 @@ func readPerRepoConfig(w *world.World, cfgPath string) (config.PerRepoConfigWrit return cfg, nil } +// givenRepositoryAgentSettings commits per-agent runtime/model/effort (a +// YAML mapping of agent name → settings) into the enrolled repo's +// .fullsend/config.yaml as agents: entries — on the entry with that name +// when present, else as a name-only entry for a built-in agent — and +// records the pre-scenario agents list so CleanupScenario restores it. +// The entries are validated the way `fullsend run` validates them, so a +// scenario cannot commit a config the runner would refuse. +func givenRepositoryAgentSettings(w *world.World, doc string) error { + if w.Org == "" || w.RepoName == "" { + return fmt.Errorf("no repo configured; call 'Given the enrolled test repository' before runtime operations") + } + var settings map[string]struct { + Runtime string `yaml:"runtime"` + Model string `yaml:"model"` + Effort string `yaml:"effort"` + } + if err := yaml.Unmarshal([]byte(doc), &settings); err != nil { + return fmt.Errorf("parsing agent settings docstring: %w", err) + } + if len(settings) == 0 { + return fmt.Errorf("agent settings docstring must hold at least one agent") + } + if !w.AgentsOverridden { + if err := snapshotAgents(w); err != nil { + return err + } + } + cfgPath := filepath.Join(".fullsend", "config.yaml") + cfg, err := readPerRepoConfig(w, cfgPath) + if err != nil { + return err + } + agents := cfg.AgentEntries() + for name, st := range settings { + // Only the settings given change; the entry's other values stay. + current, _ := config.AgentSettingsFor(agents, name) + runtimeName, model, effort := current.Runtime, current.Model, current.Effort + if st.Runtime != "" { + runtimeName = st.Runtime + } + if st.Model != "" { + model = st.Model + } + if st.Effort != "" { + effort = st.Effort + } + agents = config.UpsertAgentSettings(agents, name, runtimeName, model, effort) + } + cfg.SetAgents(agents) + if err := config.ValidateAgentEntries(cfg.AgentEntries(), cfg.AllowedResources()); err != nil { + return fmt.Errorf("agent settings: %w", err) + } + merged, err := cfg.Marshal() + if err != nil { + return err + } + if err := w.SCM.CommitFile(context.Background(), w.Org, w.RepoName, cfgPath, "behaviour: set agent settings", merged); err != nil { + return fmt.Errorf("updating config: %w", err) + } + return nil +} + // RestoreRuntime puts the install-time runtime back. Exported so // CleanupScenario can call it during scenario teardown. func RestoreRuntime(w *world.World) error { @@ -190,7 +262,21 @@ func RestoreRuntime(w *world.World) error { // runMetrics is the subset of the runner's metrics.json the steps read. type runMetrics struct { - Runtime string `json:"runtime"` + Runtime string `json:"runtime"` + // RuntimeSource is where the runner says the runtime came from: a + // flag/variable name, the config file path, or that path suffixed + // with ` agents.` when the per-agent entry decided. + RuntimeSource string `json:"runtime_source"` + // Model is what the provider reported; RequestedModel is what the + // runner handed the runtime after overrides, and OverrideSource says + // where that came from (flag, variable, harness, or the config path + // suffixed with ` agents.`). + Model string `json:"model"` + RequestedModel string `json:"requested_model"` + OverrideSource string `json:"override_source"` + // NumTurns is 0 when the agent process produced no events (for pi, + // metrics.model is the resolved id echoed back, not a provider reply). + NumTurns int `json:"num_turns"` TokenUsage struct { Input int `json:"input"` Output int `json:"output"` @@ -226,6 +312,51 @@ func assertRunSelectedRuntime(w *world.World, want string) error { return nil } +// assertRunSelectedRuntimeFrom additionally requires the runner's +// runtime_source to end with source — e.g. "agents.triage" — +// proving which config entry decided, not just which runtime ran. +func assertRunSelectedRuntimeFrom(w *world.World, want, source string) error { + m, err := readRunMetrics(w) + if err != nil { + return err + } + if m.Runtime != want { + return fmt.Errorf("metrics.json runtime = %q, want %q", m.Runtime, want) + } + if !strings.HasSuffix(m.RuntimeSource, source) { + return fmt.Errorf("metrics.json runtime_source = %q, want it to end with %q", m.RuntimeSource, source) + } + return nil +} + +// assertRunModelFrom checks the model chain the runner recorded: the +// requested model (after overrides) and its source, and that the model +// the provider actually reported contains the expected family name — +// e.g. requested "haiku" from "agents.pi-smoke", reported +// "claude-haiku-…". Proves the per-agent model reached the runtime. +func assertRunModelFrom(w *world.World, requested, source, reported string) error { + m, err := readRunMetrics(w) + if err != nil { + return err + } + if m.RequestedModel != requested { + return fmt.Errorf("metrics.json requested_model = %q, want %q", m.RequestedModel, requested) + } + if !strings.HasSuffix(m.OverrideSource, source) { + return fmt.Errorf("metrics.json override_source = %q, want it to end with %q", m.OverrideSource, source) + } + if !strings.Contains(m.Model, reported) { + return fmt.Errorf("metrics.json model = %q, want it to contain %q", m.Model, reported) + } + // The runtime records the resolved id before the first reply, so a + // run that never reached the provider still carries model; require + // turns so the assertion means the model actually answered. + if m.NumTurns <= 0 { + return fmt.Errorf("metrics.json num_turns = %d, want > 0 (model %q was resolved but never answered)", m.NumTurns, m.Model) + } + return nil +} + func assertRunMetricsReportTokens(w *world.World) error { m, err := readRunMetrics(w) if err != nil { diff --git a/pkg/behaviourtest/steps/runtime_test.go b/pkg/behaviourtest/steps/runtime_test.go index 3c190b6399..b76cad3cfc 100644 --- a/pkg/behaviourtest/steps/runtime_test.go +++ b/pkg/behaviourtest/steps/runtime_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/pkg/behaviourtest/world" ) @@ -169,3 +170,36 @@ func TestAssertPiTranscriptHasToolCall(t *testing.T) { assert.False(t, isPiSessionFile([]byte(`{"type":"assistant"}`+"\n"))) assert.False(t, isPiSessionFile(nil)) } + +func TestGivenRepositoryAgentSettings_WritesEntriesAndSnapshotsAgents(t *testing.T) { + t.Parallel() + existing := perRepoDummyConfig + "agents:\n - source: harness/lint.yaml\n effort: high\n" + scmDriver := &recordingSCM{fakeCleanupSCM: fakeCleanupSCM{fileContent: []byte(existing)}} + w := &world.World{Org: "org", RepoOwner: "org", RepoName: "repo", SCM: scmDriver} + + require.NoError(t, givenRepositoryAgentSettings(w, "triage:\n runtime: claude\ncode:\n runtime: dummy\nlint:\n model: haiku\n")) + assert.True(t, w.AgentsOverridden) + assert.Equal(t, []config.AgentEntry{{Source: "harness/lint.yaml", Effort: "high"}}, w.AgentsOriginal, + "pre-scenario agents are remembered for cleanup") + assert.Equal(t, filepath.Join(".fullsend", "config.yaml"), scmDriver.lastPath) + written, err := config.ParsePerRepoConfig(scmDriver.lastContent) + require.NoError(t, err) + assert.Equal(t, "dummy", written.(config.PerRepoConfigReader).ConfigRuntime(), "repo-wide key untouched") + lint, ok := config.AgentSettingsFor(written.AgentEntries(), "lint") + require.True(t, ok) + assert.Equal(t, config.AgentEntry{Source: "harness/lint.yaml", Effort: "high", Model: "haiku"}, lint, "settings land on the sourced entry") + triage, ok := config.AgentSettingsFor(written.AgentEntries(), "triage") + require.True(t, ok) + assert.Equal(t, config.AgentEntry{Name: "triage", Runtime: "claude"}, triage, "built-ins get a name-only entry") +} + +func TestGivenRepositoryAgentSettings_RejectsWhatTheRunnerWouldReject(t *testing.T) { + t.Parallel() + for _, doc := range []string{"coder:\n runtime: dummy\n", "triage:\n runtime: opencode\n", "triage:\n effort: turbo\n", ""} { + scmDriver := &recordingSCM{fakeCleanupSCM: fakeCleanupSCM{fileContent: []byte(perRepoDummyConfig)}} + w := &world.World{Org: "org", RepoOwner: "org", RepoName: "repo", SCM: scmDriver} + err := givenRepositoryAgentSettings(w, doc) + require.Error(t, err, "doc %q", doc) + assert.False(t, scmDriver.commitFileCalled, "doc %q", doc) + } +}