Skip to content

feat(#6581): per-agent runtime, model and effort on agents: entries - #6583

Merged
waynesun09 merged 3 commits into
mainfrom
agent/6581-per-role-overrides
Aug 25, 2026
Merged

feat(#6581): per-agent runtime, model and effort on agents: entries#6583
waynesun09 merged 3 commits into
mainfrom
agent/6581-per-role-overrides

Conversation

@fullsend-ai-coder

@fullsend-ai-coder fullsend-ai-coder Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Lets .fullsend/config.yaml say, per agent, how it runs — runtime, model, effort — in the place that already says which harness runs: the agents: entry. A built-in agent is tuned with a name-only entry; a custom agent carries the settings on its source: entry. This replaces the out-of-band repository variables (TRIAGE_FULLSEND_MODEL, CODE_FULLSEND_RUNTIME, ...) with something reviewable, and makes runtime a per-agent choice (the repo-wide runtime: stays as the default for agents that set none).

runtime: pi                    # repo default
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/<sha>/harness/lint.yaml#sha256=<hash>
    model: haiku

Or: fullsend agent set code --runtime claude --model sonnet --effort high.

Names are agent names as passed to fullsend run <agent> (and used by the workflow stages / the CODE_-style variable prefixes) — not harness role: values, since code and fix both carry role: coder. Case-insensitive. A name-only entry that is not a built-in fails validation (coder gets a "did you mean code" hint).

Design note: the first cut of this PR used a separate role_overrides: map. It was folded onto agents: entries before merge — config.yaml already had a per-agent list 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". ADR 0091 records both the decision and the rejected shape.

Folds in the scope of two sibling issues so they ship together:

Changes

  • internal/config: AgentEntry gains runtime/model/effort (source becomes optional); IsOverrideOnly, HasSettings, AgentSettingsFor, UpsertAgentSettings; ValidateAgentEntries validates the settings and the override-only rule (built-in names only, with the coder hint; duplicates case-insensitive); the keyed layered merge carries the three fields per field (empty inherits — no tombstones); Validate() checks the merged list so an overlay entry tuning a base-registered custom agent is valid; shared ValidModelRef, ValidAgentNames, ValidEffortLevels/ValidEffort (effort validators move here from harness, which imports config); OverlayConfigFile/BaseConfigFile constants.
  • internal/harness: model: validated with config.ValidModelRef; RegisteredAgents skips override-only entries (a built-in tuned by name is not a custom harness to enumerate, lock or list); BuildConfigMap exposes the fields on config.agents for overlay CEL.
  • internal/runtime: ResolveForAgent(agents, repoRuntime, agent) — the entry's runtime when set, else the repo-wide key, both validated against ValidRuntimes (stub runtimes cannot be activated through an entry either).
  • internal/cli/run.go: the config is loaded once per run (loadRunConfig, layered config.yaml over config.base.yaml, base-only directories and the nested .fullsend/ layout included) and feeds runtime selection, the FULLSEND_PI_MODEL gate and the model/effort application. Because fullsend run never calls Validate() on the config it loads, agentSettings runs ValidateAgentEntries on the effective list itself — a mistyped entry fails the run for every agent, in the overlay or in config.base.yaml — and applies model/effort beneath the flag/env overrides. Org configs' agents: entries are honoured too. runtime: claude + a provider/id model gets a plan-block warning. defaultAgentsRepoKnownAgents derives from config.ValidAgentNames().
  • internal/cli/agent.go: new fullsend agent set <name> [--runtime] [--model] [--effort] (per-repo; upsert onto the existing entry, name-only entry for built-ins, overlay entry for a base-registered agent; only given flags change, "" clears; validated before write); agent list shows settings and (built-in); agent remove drops the entry and its settings.
  • internal/cli/github.go (setup re-run, per-repo GitHub only): fullsend github setup <owner/repo> re-run used to regenerate .fullsend/config.yaml from flags and would have dropped agents: entries, allowlist edits and comments — and its runtime prompt could flip a pi repo back to claude on Enter. Now an existing per-repo config.yaml is kept out of the scaffold entirely (verbatim) unless a config-targeting flag is passed (--runtime, --agents, --mint-url, --inference-*), in which case only that key is changed on the loaded file (loaded layeredconfig.base.yaml is fetched too, so an overlay entry tuning a base-registered custom agent validates; config.ParsePerRepoConfigWriterLayered parses both layers from raw bytes); the runtime prompt is skipped on re-run; the setup PR reports the runtime the file already selects and points at agents: entries / agent set; --config rewrites config.base.yaml and keeps the existing overlay; an unparsable config.yaml fails the re-run; dry-run without repo credentials plans as a first install with a warning. Scope: the converge full-rescaffold repair and the GitLab setup path keep their previous behaviour.
  • .github/workflows/reusable-dispatch.yml: the config.yaml yq guard allows an enabled agents: entry without source when it sets runtime/model/effort (the message says so). A repo must bump its pin before adding such an entry — an older pinned workflow rejects it — documented in docs/runtimes.md.
  • Surfacing: plan block, stderr runtime: selected ... and metrics.json (runtime_source, override_source) name the source as <config path> agents.<name>.
  • Behaviour tests (e2e/behaviour/features/runtime/agent-settings.feature): (1) every run at dummy cost — repo-wide runtime: claude, entries pin triage (and code, which triage hands off to) to dummy; asserts the run selected the "dummy" runtime from "agents.triage" (runtime and runtime_source). (2) gated @requires:capability:runtime-pi — repo stays on dummy; a custom agent pi-override (harness model: opus) is put on pi with model: haiku through its entry; asserts selected "pi" runtime from "agents.pi-override", requested model "haiku" from "agents.pi-override" with a provider-reported haiku model and num_turns > 0, a pi tool call in the transcript, and tokens. New step the repository agents are configured with: (only the settings given change; CleanupScenario restores the agents: list) plus the model assertion step, with unit tests in pkg/behaviourtest/steps.
    • make behaviour-test declares BEHAVIOUR_CAPABILITIES=runtime-pi by default (a Makefile variable rather than workflow env, because E2E Tests runs on pull_request_target and takes its workflow file from main); BEHAVIOUR_CAPABILITIES= make behaviour-test skips them. This also un-skips the existing pi.feature, which had never run in CI and was broken as written twice over: the custom harnesses carried neither the Vertex project env ([pi-anthropic-vertex] disabled: set GOOGLE_CLOUD_PROJECT or ANTHROPIC_VERTEX_PROJECT_ID → every anthropic-vertex/... id "not found") nor the vertex-ai provider/profile that grants egress since ADR-0065 (policy_denied). Both pi scenarios now declare profiles/providers like the fleet harnesses (the custom-harness step commits the real scaffold files; the per-repo install ships only .gitkeeps for those dirs — also a real gap for BYOA harnesses on per-repo installs) and inline what common/env/gcp-vertex.env + the credential host_files do. On pi, haiku resolves to anthropic-vertex/claude-haiku-4-5 (pi-ai's catalog has it since 0.84.1; the extension registers that catalog verbatim and calls Vertex via @anthropic-ai/vertex-sdk, which maps region=global to the global endpoint).
  • .codecov.yml: the per-file ignore list for behaviour-test code is replaced by pkg/behaviourtest/** and e2e/behaviour/**.
  • Docs: docs/runtimes.md (precedence diagram + table, per-agent section, migration note), docs/cli/run.md, docs/cli/agent.md (agent set), docs/cli/github.md (re-run semantics), choosing-a-runtime.md, docs/runtimes/pi.md + runtime-implementation.md, layered-config-reference.md (merge rule), architecture.md, behaviour-testing.md, ADR 0091 (0091-per-agent-runtime-model-effort.md; records the rejected role_overrides shape, how it narrows ADR 0045, the deliberate non-removal of repo-wide runtime:, and why Add per-role models:/effort: to .fullsend/config.yaml with models.aliases remap #6529/Allow forward slash in model identifiers to support provider/model format #6570 are folded in).

Backward compatibility / user impact (audited)

  • Repos without per-agent settings behave exactly as before: no entry → no override; validation of existing agents: lists is unchanged for sourced/disabled entries; Marshal omits empty fields (and now omits an empty source: on disable-only entries); org-mode configs untouched.
  • An older pinned CLI/workflow rejects an enabled entry without source (the yq guard), so a repo must bump its workflow pin before adding a name-only entry — documented.
  • CI: agent jobs check out the config repo fully (only the route job sparse-checks config.yaml), so config.base.yaml entries work; setup-agent-env.sh repository variables still win over config; action.yml passes no runtime/model flags, so config is authoritative absent variables. Runtimes receive --model/--effort (Claude Code) or --model/--thinking (pi) from the resolved harness and never read config themselves.
  • repos status/converge skip config.yaml for drift by design; repos migrate carries config across explicitly.

Precedence

flag > env var (incl. role-prefixed repository variables) > the agent's agents: entry > repo-wide runtime: / harness model: effort: > default

Testing

  • internal/config: parse/validate/round-trip for entries with settings (override-only built-ins, sourced custom agents, coder hint, unknown custom without source, invalid model/runtime/effort, case-insensitive duplicates), layered per-field merge incl. an overlay entry tuning a base-registered agent and a bad entry in the base, UpsertAgentSettings, ValidModelRef, ValidEffort.
  • internal/harness: provider/id accepted, malformed forms rejected; RegisteredAgents skips override-only entries; BuildConfigMap exposes the fields.
  • internal/runtime: ResolveForAgent precedence and stub-runtime rejection.
  • internal/cli: per-agent runtime via resolveBackend (flag still wins, source label, org configs), stub runtime rejected at run time, layered/base-only/nested config discovery, bad entries rejected on the run path (overlay, base with overlay, base only, bad value), applyAgentSettings, agent set (built-in, custom, base-registered, validation), agent remove, setup re-run (flag-less re-run leaves config.yaml out; --runtime changes only that key and keeps agents: entries; unparsable config fails; --config keeps the overlay).
  • go test ./internal/... ./pkg/behaviourtest/... green locally (the two TestDummyRuntime_* failures in internal/runtime reproduce identically on origin/main on this host); make lint clean; both feature files parse.
  • End-to-end: a real triage run on pi took model/effort from its entry (from-source build on a Mac), and in CI both behaviour scenarios passed on the previous shape (d548303: 24/24 scenarios, real Vertex Haiku reply with tool calls and tokens); this head re-runs them on the folded shape.

Live target: fullsend-ai/pi-xai-vertex#1 now carries exactly this configuration as agents: entries (six built-in agents: Grok on pi for triage/review, Gemini on pi for prioritize, Claude Code Sonnet for code/fix/retro) pinned to fullsend @main, replacing its nine role-prefixed repository variables; it merges after this PR lands.


Closes #6581
Closes #6570
Closes #6529

@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 25, 2026 03:21
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Aug 25, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 3:22 AM UTC · Ended 3:29 AM UTC

Commit: 5ff4f6d · View workflow run →

@github-actions

github-actions Bot commented Aug 25, 2026

Copy link
Copy Markdown

Site preview

Preview: https://b680cf26-site.fullsend-ai.workers.dev

Commit: 245adea9e09e1e40819a6f859735e8b564632f58

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 3:30 AM UTC · Ended 3:47 AM UTC

Commit: b0d20f7 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:48 AM UTC · Completed 4:07 AM UTC

Commit: aeb4428 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $10.01

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

Review

Findings

Medium

Low

  • [config-schema-extension] internal/config/config.goAgentEntry.Source yaml tag changes from yaml:"source" to yaml:"source,omitempty", so a marshalled override-only entry no longer emits source: "". The reusable-dispatch.yml workflow guard is updated, but repos pinned to an older fullsend version will reject entries without source:. ADR 0091 documents the pin-before-use requirement.

  • [struct-field-addition] internal/config/config.go — Three new exported fields added to AgentEntry: Runtime, Model, Effort (all string, yaml:"...,omitempty"). Additive struct change visible in the config.agents CEL expression surface and BuildConfigMap output.

  • [exported-function-removal] internal/harness/harness.goharness.ValidEffort() and harness.ValidEffortLevels() are relocated to config.ValidEffort() and config.ValidEffortLevels(). The internal/ path prevents external import, so blast radius is limited to in-tree consumers (which are migrated in this PR).

  • [model-validation-relaxation] internal/harness/harness.go — Harness model validation regex changed from ^[a-zA-Z0-9_.@-]+$ to ^[a-zA-Z0-9_.@-]+(/[a-zA-Z0-9_.@-]+)*$, accepting provider/id forms. Backward-compatible superset; harness files using / will be rejected by older fullsend versions. ADR 0091 documents pin-before-use.

  • [edge-case] pkg/behaviourtest/steps/runtime.gogivenRepositoryAgentSettings parses the config via readPerRepoConfig (single file, no base layer). If an enrolled test repo has a config.base.yaml, the step would not see those agents in AgentEntries(). No behaviour test scenario currently triggers this.

  • [edge-case] pkg/behaviourtest/steps/runtime.gogivenRepositoryAgentSettings iterates a map[string]struct{...} when applying settings. Go map iteration order is non-deterministic, so when two settings entries target the same agent name with different casing, the final result depends on iteration order. Test-only path; no scenario duplicates agent names.

  • [redundant-parse] internal/cli/run.go — For org-mode configs, runConfig stores raw bytes (orgData) rather than a parsed object. Both backend() and agentSettings() re-parse the same bytes via config.ParseOrgConfig each time they are called. Minor inefficiency on the deprecated org-mode path.

  • [scope-creep] internal/cli/run.go:109defaultAgentsRepoKnownAgents now derives from config.ValidAgentNames() instead of a literal list. The coupling is intentional and documented as transitional (to be removed at ADR 0058 Phase 5).

  • [scope-creep] internal/cli/github.go — The setup re-run idempotency logic (loadExistingPerRepoConfig, keepExistingConfig, applySetupFlagsToConfig) is not requested by the linked issues but is a necessary consequence: without it, fullsend github setup re-runs would silently overwrite per-agent settings.

  • [api-shape-pattern] internal/runtime/registry.go:68ResolveForAgent returns (Backend, bool, error) while siblings ResolveFromConfig and ResolveFromPerRepoConfig return (Backend, error). The extra bool distinguishes per-agent from repo-wide source. Accepted as a deliberate design choice.

  • [code-organization] internal/cli/run.go:174 — The runConfig struct and its five methods add ~130 lines to run.go, the largest file in the package. The struct is cohesive (all methods serve the same loadRunConfig entry point); extraction would not improve discoverability.

  • [naming-convention] internal/cli/agent.go:152agentSetFlags uses bare bools (runtimeSet, modelSet, effortSet) to track which flags were given. The existing githubSetupConfig in the same package uses map[string]bool changedFlags for the same purpose, introducing a second idiom.

  • [behavior-change] internal/harness/registry.go:48RegisteredAgents() now skips override-only entries (IsOverrideOnly() == true). The new entry type did not exist before, so no existing caller can observe a regression.

  • [workflow-contract] .github/workflows/reusable-dispatch.yml:40 — The yq guard for enabled agent entries is relaxed: an entry without source is now valid when it sets runtime, model, or effort. Repos pinning an older version of this workflow will reject override-only entries. ADR 0091 documents the pin-before-use requirement.

  • [stale-doc] docs/guides/user/bring-your-own-agent.md:576 — The CLI quick-reference section lists "List / update / remove" operations but does not mention the new fullsend agent set subcommand. Custom agents registered via agents: entries can also carry per-agent settings.
    Remediation: Add fullsend agent set <name> to the CLI quick-reference alongside the existing commands.


Labels: PR modifies harness validation, runtime resolution, CI workflow, install setup, and documentation

Previous run

Review

Findings

Medium

  • [stale-doc] docs/guides/dev/cli-internals.md:76 — The CLI command tree lists agent subcommands as add, list, update, remove but omits the new set subcommand added in this PR. This file is listed in docs/contributing/documentation.md as a touchpoint for the agent command group.
    Remediation: Add set between update and remove in the agent command tree.

  • [config-schema-extension] internal/config/config.goAgentEntry.Source yaml tag changes from yaml:"source" to yaml:"source,omitempty", so a marshalled override-only entry no longer emits source: "". The reusable-dispatch.yml workflow guard is updated, but repos pinned to an older fullsend version will reject entries without source:. ADR 0091 documents the pin-before-use requirement.

  • [protected-path] .github/workflows/reusable-dispatch.yml — This file is under the .github/ protected path. The PR links to config.yaml cannot express per-role model or runtime; repo variables are the only path #6581 and explains the change (the yq guard must allow sourceless entries with settings per ADR 0091). Human approval is required for protected-path changes regardless of context.

Low

  • [stale-doc] docs/cli/agent.md:7 — The page description says "Add, list, update, and remove agents" but the commands table below now includes set. The description was not updated to match.

  • [workflow-contract] .github/workflows/reusable-dispatch.yml:40 — The yq guard for enabled agent entries is relaxed: an entry without source is now valid when it sets runtime, model, or effort. Repos pinning an older version of this workflow will reject override-only entries. ADR 0091 documents the pin-before-use requirement.

  • [consumer-completeness] .codecov.yml:23 — Replaces 9 per-file codecov ignores under pkg/behaviourtest/ with a broad pkg/behaviourtest/** glob, newly excluding ~24 non-test Go source files from unit coverage tracking. The excluded code is test infrastructure outside the cmd//internal/ production coverage gate.

  • [scope-creep] internal/cli/run.go:109defaultAgentsRepoKnownAgents now derives from config.ValidAgentNames() instead of a literal list. The coupling is intentional and documented as transitional (to be removed at ADR 0058 Phase 5).

  • [edge-case] pkg/behaviourtest/steps/runtime.gogivenRepositoryAgentSettings parses the config via readPerRepoConfig (single file, no base layer). If an enrolled test repo has a config.base.yaml, the step would not see those agents in AgentEntries(). No behaviour test scenario currently triggers this.

  • [api-shape-pattern] internal/runtime/registry.go:68ResolveForAgent returns (Backend, bool, error) while siblings ResolveFromConfig and ResolveFromPerRepoConfig return (Backend, error). The extra bool distinguishes per-agent from repo-wide source. Accepted as a deliberate design choice.

  • [metrics-output-contract] docs/cli/run.md:274runtime_source and override_source in metrics.json gain a new suffix pattern: <config path> agents.<name>. Additive — existing values unchanged. Consumers with exact-string matching on the source field should be aware.

  • [code-organization] internal/cli/run.go:174 — The runConfig struct and its five methods add ~130 lines to run.go, the largest file in the package. The struct is cohesive (all methods serve the same loadRunConfig entry point); extraction would not improve discoverability.

Previous run (2)

Review

Findings

Medium

  • [error-handling] internal/cli/github.goloadExistingPerRepoConfig parses the overlay config.yaml from the repo without its config.base.yaml layer. When applySetupFlagsToConfig then calls existingCfg.Validate(), the validation runs ValidateAgentEntries on the overlay’s entries alone (no merge with the base). An overlay entry that tunes a custom agent registered only in config.base.yaml (e.g. - name: lint / effort: medium where lint is defined with source: in the base) would fail validation with “is not a built-in agent” because the merged source is not visible. This only fires on a setup re-run with a config-targeting flag (--runtime, --agents, etc.) on a repo with this specific layering pattern.
    Remediation: fetch config.base.yaml from the repo in loadExistingPerRepoConfig when it exists (so the parsed config carries the merged parent chain), or skip agent-entry re-validation when no agent-targeting flag was passed.

  • [stale-doc] docs/cli/README.md:17 — The fullsend agent command group description says “add, list, update, remove” but the PR adds agent set. The description is now incomplete.

  • [stale-doc] docs/guides/getting-started/operations.md:114 — The operations quick-reference table lists agent add, list, update, remove but does not include agent set.

  • [workflow-contract] .github/workflows/reusable-dispatch.yml:40 — The yq guard for enabled agent entries is relaxed: an entry without source is now valid when it sets runtime, model, or effort. Repos pinning an older version of this workflow will reject override-only entries. ADR 0091 documents the pin-before-use requirement. The yq condition correctly checks all three fields with AND so no bypass path exists.

  • [protected-path] .github/workflows/reusable-dispatch.yml — This file is under the .github/ protected path. The PR links to config.yaml cannot express per-role model or runtime; repo variables are the only path #6581 and explains the change (the yq guard must allow sourceless entries with settings per ADR 0091). Human approval is required for protected-path changes regardless of context.

Low

  • [consumer-completeness] .codecov.yml:23 — Replaces 9 per-file codecov ignores under pkg/behaviourtest/ with a broad pkg/behaviourtest/** glob, newly excluding ~24 non-test Go source files from unit coverage tracking. The excluded code is test infrastructure outside the cmd//internal/ production coverage gate.

  • [code-organization] internal/cli/run.go:174 — The runConfig struct and its five methods add ~130 lines to run.go, the largest file in the package. The struct is cohesive (all methods serve the same loadRunConfig entry point); extraction would not improve discoverability.

  • [api-shape-pattern] internal/runtime/registry.go:68ResolveForAgent returns (Backend, bool, error) while siblings ResolveFromConfig and ResolveFromPerRepoConfig return (Backend, error). The extra bool distinguishes per-agent from repo-wide source. Accepted as a deliberate design choice.

  • [scope-creep] internal/cli/run.go:109defaultAgentsRepoKnownAgents now derives from config.ValidAgentNames() instead of a literal list. The coupling is intentional and documented as transitional (to be removed at ADR 0058 Phase 5).

  • [stale-doc] docs/architecture.md:386 — Architecture overview says fullsend agent add|list|update|remove; missing set.

  • [metrics-output-contract] docs/cli/run.md:274runtime_source and override_source in metrics.json gain a new suffix pattern: <config path> agents.<name>. Additive — existing values unchanged. Consumers with exact-string matching on the source field should be aware.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (3)

Review

Findings

Low

  • [consumer-completeness] .codecov.yml:23 — The diff replaces 9 specific per-file codecov ignores under pkg/behaviourtest/ with a broad pkg/behaviourtest/** glob, newly excluding approximately 24 non-test Go source files from unit coverage tracking — including drivers/scm/*.go, drivers/ci/githubactions/*.go, drivers/install/*.go, world/*.go, steps/runtime.go, steps/cleanup.go, and artifacts/find.go. These files have companion unit tests whose coverage is now invisible to Codecov's patch check. The change is intentional (integration-only code exercised by the behaviour/e2e CI jobs) and the excluded code is test infrastructure (pkg/behaviourtest/) outside the cmd//internal/ production coverage gate, but the broader glob permanently opts out the entire package tree.

  • [code-organization] internal/cli/run.go:174 — The new runConfig struct and its five methods add ~130 lines to run.go, the largest file in the package. The codebase has a pattern of extracting related functionality into sibling files (e.g., run_overrides.go). Consider extracting runConfig, loadRunConfig, and roleOverrideSource into run_overrides.go or a new run_config.go.

  • [api-shape-pattern] internal/runtime/registry.go:68ResolveForAgent returns (Backend, bool, error) while the sibling ResolveFromConfig and ResolveFromPerRepoConfig return (Backend, error). The extra bool is consumed by exactly two callers to decide the source label. Accepted as a deliberate design choice: the source label must distinguish per-agent from repo-wide.

  • [scope-creep] internal/cli/run.go:109defaultAgentsRepoKnownAgents now derives from config.ValidAgentNames() instead of a literal list. The coupling is intentional and documented as transitional (to be removed at ADR 0058 Phase 5), and the refactor removes a second source of truth that would otherwise drift.

  • [config-schema-evolution] internal/config/config.go:637 — The per-repo config YAML schema gains a new top-level role_overrides key. This is backward compatible: an older CLI silently ignores unknown keys. The ADR and migration docs correctly warn that an older pinned CLI will not enforce validation, so entries would silently do nothing — repos must bump the workflow pin before adding the block.

  • [metrics-output-contract] docs/cli/run.md:274runtime_source and override_source in metrics.json gain a new suffix pattern: <config path> role_overrides.<agent>. This is additive — all previous values remain unchanged. Consumers with exact-string matching on the source field should be aware of the new suffix.

Previous run (4)

Review

Findings

Low

  • [edge-case] internal/cli/agent.go:374runAgentRemove checks for base-layer role_overrides entries using the string literal "config.yaml" rather than the overlayConfigFile constant defined in config.go. If the overlay filename constant ever changes, this comparison would silently become wrong, causing the function to error on overlay entries it should handle. The prior finding about dual diagnostics (warning + validation error) is resolved: the function now returns a single clear error before writing, leaving the config file untouched.

  • [api-shape-pattern] internal/runtime/registry.go:68ResolveForAgent returns (Backend, bool, error) while the sibling ResolveFromConfig and ResolveFromPerRepoConfig return (Backend, error). The extra bool is consumed by exactly two callers to decide the source label. Accepted as a deliberate design choice: the source label must distinguish per-agent from repo-wide.

  • [scope-creep] internal/cli/run.go:109defaultAgentsRepoKnownAgents now derives from config.ValidAgentNames() instead of a literal list. The coupling is intentional and documented as transitional (to be removed at ADR 0058 Phase 5), and the refactor removes a second source of truth that would otherwise drift.

  • [edge-case] internal/cli/github.goloadExistingPerRepoConfig parses the existing config.yaml without loading config.base.yaml (no parent layer). When a re-run with config-targeting flags calls Validate(), ValidateRoleOverrides only checks the overlay's entries. A hand-edited config.base.yaml with an invalid role_overrides entry would not be caught during setup re-run, even though the layered config loaded by fullsend run would reject it. This is a narrow gap since base layers are generated by the preset flow.

Previous run (5)

Review

Findings

Low

  • [edge-case] internal/cli/agent.go:374runAgentRemove checks for base-layer role_overrides entries after deleting from the overlay. If the base layer entry exists, the user sees both a warning ("role_overrides entry for agent X lives in config.base.yaml; remove it there") AND a subsequent Validate() error (the agent name is no longer in the valid set). The validation error prevents writing a broken config (correct safety behavior), but the dual diagnostics could be consolidated — e.g., by including the validation guidance in the warning and skipping validation of entries the user was already told to fix externally.

  • [api-shape-pattern] internal/runtime/registry.go:68ResolveForAgent returns (Backend, bool, error) while the sibling ResolveFromConfig and ResolveFromPerRepoConfig return (Backend, error). The extra bool is consumed by exactly one caller to decide the source label.

  • [error-handling-idiom] internal/cli/run.go:386roleOverrideFor prefixes errors using fmt.Errorf("%s%c%w", filepath.Dir(rc.source), os.PathSeparator, err). The rest of the codebase uses filepath.Join for path construction and fmt.Errorf with ": " for error prefixing. Using os.PathSeparator as a format verb is unusual.

  • [scope-creep] internal/cli/run.go:109defaultAgentsRepoKnownAgents now derives from config.ValidAgentNames() instead of a literal list. The coupling is intentional and documented as transitional (to be removed at ADR 0058 Phase 5), but the two serve different purposes (agents-repo fallback vs. role_overrides validation).

Previous run (6)

Review

Findings

Low

  • [incomplete-validation] internal/config/config.go:940ValidateRoleOverrides validates entries in base layers against the overlay's effective agent set. When ParsePerRepoConfig() is used on a standalone file (without layers), layerFile is empty and the error message loses the file context. This is a minor UX issue limited to in-memory parsing paths.

  • [edge-case] internal/cli/agent.go:374runAgentRemove checks for base-layer role_overrides entries after deleting from the overlay. If the base layer entry exists, the subsequent cfg.Validate() call will fail because the agent's name is no longer in the valid agent set. The user sees both the warning AND a validation error. The validation error prevents writing a broken config (right safety behavior), but the dual diagnostics could be clearer.

  • [api-shape-pattern] internal/runtime/registry.go:62ResolveForAgent returns (Backend, bool, error) while the sibling ResolveFromConfig and ResolveFromPerRepoConfig return (Backend, error). The extra bool is consumed by exactly one caller to decide the source label.

  • [scope-creep] internal/cli/run.go:106defaultAgentsRepoKnownAgents now derives from config.ValidAgentNames() instead of a literal list. The two serve different purposes (agents-repo fallback vs. role_overrides validation); the coupling could surprise if ValidAgentNames evolves.

Previous run (7)

Review

Findings

Medium

  • [redundant-io] internal/cli/run.go — The config file is loaded via loadRunConfig twice during runAgent: once inside backendFromConfigFile (via resolveBackend), and a second time directly. Each call re-reads and re-parses config.yaml and config.base.yaml independently, producing separate config objects. The comment "The same loaded config (file and layering) decided the runtime above, so all three fields of an entry come from one place" is misleading: it is the same file but not the same loaded config object. While both loads produce equivalent results in practice, the double load is an unnecessary code smell.
    Remediation: Load the config once and pass the runConfig to both the backend resolution and the role override application.

Low

  • [scope-creep] internal/cli/run.goloadRunConfig adds new base-only config directory discovery (scanning for config.base.yaml when config.yaml is absent, in both the requested path and under .fullsend/). This is a logical necessity for preset base layers carrying role_overrides, but extends config loading beyond what backendFromConfigFile previously supported. Consider documenting this behavior in ADR 0091.

  • [naming-convention] internal/config/config.goValidEffort and ValidEffortLevels are exported from both config and harness packages, with harness delegating to config. This duplicate-export pattern is unique in the codebase. Other shared validators (e.g., ValidModelRef) live in one package only.

  • [error-handling-idiom] internal/cli/run.goroleOverrideFor calls ValidateRoleOverrides() on every invocation rather than validating once at config load time. Currently called once per run, so no performance impact, but differs from the validate-once pattern used elsewhere in the flow.

  • [incomplete-validation] internal/config/config.goValidateRoleOverrides validates only the overlay layer's entries. A mistyped key in config.base.yaml (e.g., coder instead of code) would pass validation and be silently ignored at runtime.

  • [edge-case] internal/cli/agent.go:368runAgentRemove deletes the role override from the overlay only. If the entry exists only in config.base.yaml, it remains and would fail validation on the next fullsend run.

  • [scope-creep] internal/cli/run.go:106defaultAgentsRepoKnownAgents now derives from config.ValidAgentNames() instead of a literal list. The two serve different purposes (agents-repo fallback vs. role_overrides validation); the coupling could surprise if ValidAgentNames evolves.

  • [comment-format] internal/runtime/pi_run.go:16 — The updated block comment wraps inconsistently compared to surrounding comments.

  • [naming-convention] internal/config/interfaces.go:93ValidateRoleOverrides() is the only method on PerRepoConfigReader without the Config prefix convention used by all other reader methods.

  • [stale-doc] docs/guides/user/running-agents-locally.md:335 — References backendFromConfigFile → ResolveFromConfig as the runtime resolution code path. Still correct for org-mode, but the per-repo path now uses ResolveForAgent.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 25, 2026
@waynesun09
waynesun09 force-pushed the agent/6581-per-role-overrides branch from aeb4428 to bac1f51 Compare August 25, 2026 04:10
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:11 AM UTC · Completed 4:27 AM UTC

Commit: bac1f51 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $11.99

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge and removed requires-manual-review Review requires human judgment labels Aug 25, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 11:22 AM UTC · Ended 11:30 AM UTC

Commit: e7b6665 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:35 PM UTC · Completed 3:55 PM UTC

Commit: 8ade5de · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $14.26

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review August 25, 2026 15:55

Superseded by updated review

fullsend-ai-review[bot]

This comment was marked as outdated.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 25, 2026
An agents: entry in the per-repo config.yaml can now set runtime, model
and effort for one agent, in the place that already says which harness
runs. A built-in agent (triage, code, review, fix, retro, prioritize)
is tuned with a name-only entry — an enabled entry without source, valid
only for a built-in name — and a custom agent carries the settings on
its source: entry. `fullsend agent set <name> --runtime/--model/--effort`
writes them. This replaces the out-of-band repository variables
(TRIAGE_FULLSEND_MODEL, CODE_FULLSEND_RUNTIME, ...) with something
reviewable, and makes runtime a per-agent choice; the repo-wide runtime:
stays as the default for agents that set none.

Names are agent names as passed to `fullsend run <agent>` — not harness
role: values, since code and fix both carry role: coder — matched
case-insensitively. Precedence: flag > env > the agent's entry >
repo-wide config / harness default. Entries merge per field across the
layered config (empty inherits, no tombstones). The source is surfaced
as "<config path> agents.<name>" in the plan block, stderr and
metrics.json.

`fullsend run` loads one layered per-repo config (config.yaml over
config.base.yaml, ADR 0069; base-only and nested .fullsend/ layouts
included) for runtime selection and the model/effort application, and
— because it never calls Validate() on the config it loads — validates
the effective agents: list itself on every run: a mistyped entry fails
the run for every agent, in the overlay or in config.base.yaml, and a
stub runtime cannot be activated through an entry any more than through
runtime:. Override-only entries register no harness: RegisteredAgents
skips them (not a custom harness to enumerate, lock or list), the
built-in keeps resolving through the agents-repo fallback, and the
reusable workflow's config.yaml guard accepts them. The effective
entries are exposed to overlay CEL as config.agents.

Also relaxes the harness validModelName regex to accept provider/id
format (e.g. xai-vertex/xai/grok-4.6) via a shared config.ValidModelRef
function (#6570); the effort level list moves to config.ValidEffortLevels
(harness imports config). A runtime: claude entry paired with a
provider/id model gets a plan-block warning rather than a rejection.

The first cut of this change used a separate role_overrides: map; it
was folded onto agents: entries before merge — config.yaml already had
a per-agent list keyed by the same names with the same layered merge.
ADR 0091 records both. models.aliases from #6529 is deferred; #6577
tracks the alias bug.

Closes #6581
Closes #6570
Closes #6529

Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
…ings

Add features/runtime/agent-settings.feature.

Scenario 1 (every run, dummy cost): the leased repo's runtime: is
flipped to claude and agents: entries pin triage (and code, which triage
hands off to via ready-to-code) back to dummy, so a passing run proves
the per-agent entry beat the repo-wide key without inference. The new
assertion `the run selected the "<runtime>" runtime from "<source>"`
also checks metrics.json runtime_source ends with agents.triage — which
config entry decided, not just which runtime ran.

Scenario 2 (gated @requires:capability:runtime-pi): the repo stays on
dummy and one custom agent is put on pi with `model: haiku` through its
agents: entry while its harness says `model: opus`. `the run requested
model "<m>" from "<source>" and the provider reported a "<family>"
model` checks requested_model, override_source, the provider-reported
model and num_turns > 0 in metrics.json, so both the runtime and the
model of one agent demonstrably came from config.yaml. Existing pi
transcript/token assertions complete it.

New step `the repository agents are configured with:` takes a YAML
mapping of agent name -> {runtime, model, effort}, sets only the given
settings on the existing entry (a name-only entry for a built-in) —
validated the way `fullsend run` validates them — and CleanupScenario
restores the agents: list.

Both pi scenarios (this one and the pre-existing pi.feature, which had
never run in CI) need what the fleet harnesses carry and the behaviour
custom harnesses did not: the vertex-ai provider/profile that grants
egress (ADR-0065; the custom-harness step now commits the real scaffold
files, since a per-repo install ships only .gitkeeps for those dirs) and
the Vertex project env plus credential host files inlined into the
sandbox. `make behaviour-test` declares BEHAVIOUR_CAPABILITIES=runtime-pi
by default (a Makefile variable, because E2E Tests runs on
pull_request_target and takes its workflow file from main).

Assisted-by: Claude

.codecov.yml: the per-file ignore list for behaviour-test code is
replaced by pkg/behaviourtest/** and e2e/behaviour/** — integration-only
code exercised by the behaviour/e2e jobs, not by the unit upload.

Signed-off-by: Wayne Sun <gsun@redhat.com>
`fullsend github setup <owner/repo>` regenerated .fullsend/config.yaml
from flags on every run, so a re-run (for example after a fullsend
upgrade) would have dropped agents: entries and their per-agent
settings, allowlist edits and hand-written comments, and its runtime
prompt could flip a pi repo back to claude on Enter. Now an existing
per-repo config.yaml is left out of the scaffold files entirely — kept
verbatim, comments included — unless a flag that targets a config key
is passed (--runtime, --agents, --mint-url, --inference-*), in which
case only that key is changed on the loaded file. The runtime prompt
runs only on a first install; the setup PR body reports the runtime the
file already selects and points at agents: entries / `fullsend agent
set`. --config rewrites config.base.yaml and keeps the existing
overlay. A config.yaml that no longer parses fails the re-run instead
of being regenerated; dry runs without repo credentials plan as a first
install with a warning.

Scope: per-repo GitHub setup. The converge full-rescaffold repair and
the GitLab setup path keep their previous behaviour.

docs/cli/github.md documents the re-run semantics.

Assisted-by: Claude
Signed-off-by: Wayne Sun <gsun@redhat.com>
@waynesun09
waynesun09 force-pushed the agent/6581-per-role-overrides branch from 8ade5de to 245adea Compare August 25, 2026 15:59
@waynesun09

Copy link
Copy Markdown
Member

Re-review on 8ade5de addressed in 245adea9: docs/guides/dev/cli-internals.md command tree and the docs/cli/agent.md page description now include agent set. On the behaviour step edge case: the pool repos are installed without a config.base.yaml and givenRepositoryAgentSettings deliberately edits only the overlay (the layer the suite owns and restores), so it reads that single file; a scenario that needs base-layer agents would set them through the base-harness steps instead. The config-schema-extension / workflow-contract notes are the documented pin-before-use requirement (ADR 0091, docs/runtimes.md), and the protected-path change to reusable-dispatch.yml is the one-line yq guard relaxation described in the PR body — awaiting the human approval that gate requires.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 25, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 4:00 PM UTC · Ended 4:19 PM UTC

Commit: 245adea · View workflow run →

@waynesun09

Copy link
Copy Markdown
Member

@ggallen — local verification matrix for the folded shape, in case it helps the review. All runs use a from-source build of this branch on a Mac (podman + openshell), a per-repo fullsend dir carrying the fleet harnesses locally (agents: entries with source: harness/<name>.yaml), and the fleet's Vertex WIF credentials; fullsend run … --no-post-script so nothing writes back.

# Check Command / config Result
1 Set settings on existing sourced entries (UX path) fullsend agent set triage --model sonnet --effort medium; fullsend agent set code --runtime claude --model sonnet entries gain model: sonnet / effort: medium and runtime: claude / model: sonnet; rest of the file untouched
2 List shows settings fullsend agent list triage harness/triage.yaml [model=sonnet effort=medium], code harness/code.yaml [runtime=claude model=sonnet]
3 Invalid value refused before write fullsend agent set triage --effort turbo config validation failed: agents[0] (harness/triage.yaml): invalid effort "turbo": must be one of low, medium, high, xhigh, max; file unchanged
4 Unknown built-in refused before write fullsend agent set coder --model sonnet agents[6] (coder): entry without a source tunes a built-in agent, but "coder" is not one (did you mean "code"?): built-in agents are triage, code, review, fix, retro, prioritize; give a custom agent its source
5 Hand-edited invalid value fails the run at load effort: turbo on the triage entry → fullsend run triage exit 1 before any sandbox: invalid agent config: agents[0] (harness/triage.yaml): invalid effort "turbo" …
6 Hand-edited name-only unknown agent fails the run at load - name: coder / model: sonnetfullsend run triage exit 1 before any sandbox, same "did you mean code" error
7 Per-agent runtime beats repo-wide (runtime: pi) fullsend run code plan block Runtime: claude (from …/config.yaml agents.code), Model: sonnet (from …/config.yaml agents.code), Effort: high (harness)
8 Per-agent model/effort reach pi, repo-wide runtime kept fullsend run triage (full run on pi) plan block Model: sonnet (from … agents.triage), Effort: medium (from … agents.triage), Runtime: pi (from …/config.yaml); stderr model: requested "sonnet" from … agents.triage; triage result sufficient, Result: stop, $0.42; metrics.json: runtime: pi, override_source: "…/config.yaml agents.triage", model: claude-sonnet-4-6, num_turns: 14

CI on the same shape (behaviour job, real pool repos): an agents: entry selects the runtime for one agent over the repo-wide keyselected the "dummy" runtime from "agents.triage" with repo-wide runtime: claude; an agents: entry puts one custom agent on pi with its own modelselected the "pi" runtime from "agents.pi-override", requested model "haiku" from "agents.pi-override", provider reported claude-haiku-4-5 with tool calls and tokens; the pre-existing pi.feature also passes now that the capability is declared.

Two things the matrix surfaced that are in the PR rather than around it: fullsend run validates the effective agents: list itself (rows 5–6 — it never calls Validate()), and a name-only entry resolves the built-in through the agents-repo fallback (row 7–8; the first cut resolved the empty source to the .fullsend directory).

@fullsend-ai-review fullsend-ai-review Bot added component/harness Agent harness, config, and skills loading component/runner Agent runner behavior and lifecycle component/docs User-facing documentation labels Aug 25, 2026
@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:00 PM UTC · Completed 4:19 PM UTC

Commit: 245adea · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $16.63

@ggallen ggallen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The refactored design is clean — putting model/runtime/effort on agents: entries instead of a separate role_overrides key is the right call. Reviewed the config schema (AgentEntry changes, validation, keyed merge across layers), the run/runtime path (precedence chain, source labels, stub runtime rejection), the agent CLI (agent set/agent remove), the GitHub setup re-run preservation, the workflow yq guard, and the ADR. No bugs found. Test coverage is thorough across all paths.

@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 25, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 5:38 PM UTC · Completed 5:52 PM UTC

Commit: 245adea · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.59

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #6583 — per-agent runtime, model and effort on agents: entries

Workflow shape: Human-authored PR (Claude Code–assisted, no autonomous code agent — issue #6581 was triaged needs-design). 10 review agent runs (5 succeeded, 5 cancelled by rapid force-pushes), 1 fix agent run, ~$116 total review cost. Merged after human approval from ggallen.

What went well:

  • The review agent caught genuine implementation bugs: a base-layer edge case in agent remove, dual-diagnostic consolidation, a magic string literal, an unconventional error-formatting idiom, and two stale-doc references. The fix agent addressed a real layered-validation gap (loadExistingPerRepoConfig not loading config.base.yaml).
  • Triage correctly labeled the issue needs-design, preventing autonomous code dispatch.
  • Human reviewers (ggallen, rh-hemartin) caught the critical design issue — role_overrides as a parallel structure to agents: was unnecessary because AgentEntry already had precedent for name-only override entries (enabled: false). This led to a clean refactoring.

Evidence for existing issues (not filed as new proposals):

Agents repo: fullsend-ai/agents@main (commit dc7c805632eb).

No new proposals. All identified improvement opportunities are covered by existing open issues with substantial prior discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/docs User-facing documentation component/harness Agent harness, config, and skills loading component/install CLI install and app setup component/runner Agent runner behavior and lifecycle ready-for-review Triggers review agent dispatch requires-manual-review Review requires human judgment

Projects

None yet

3 participants