diff --git a/docs/ADRs/0024-harness-definitions.md b/docs/ADRs/0024-harness-definitions.md index 5304240b85..22762ea578 100644 --- a/docs/ADRs/0024-harness-definitions.md +++ b/docs/ADRs/0024-harness-definitions.md @@ -28,6 +28,11 @@ and the manual `.env` file convention.* adds `openshell.profiles` and URL-based `providers` fields to the harness schema for portable provider and profile resolution.* +*See also [ADR 0081](0081-reserve-workflow-env-for-infra-plumbing.md), which +narrows the CI workflow `env:` injection path described in "Template +instantiation" below to infrastructure plumbing and CI-runtime-only values; +static agent behavior defaults go through harness composition instead.* + ## Context Each agent invocation requires configuration that ties together several moving diff --git a/docs/ADRs/0049-agent-configuration-env-var-convention.md b/docs/ADRs/0049-agent-configuration-env-var-convention.md index 9ebadcb72a..d0924cf1b7 100644 --- a/docs/ADRs/0049-agent-configuration-env-var-convention.md +++ b/docs/ADRs/0049-agent-configuration-env-var-convention.md @@ -19,6 +19,12 @@ Date: 2026-06-16 Accepted +*Amended by [ADR 0080](0080-config-yaml-vs-agent-env-var-scope.md), which +decides when a knob should be an `{AGENT}_` env var at all versus a +`config.yaml` field, and [ADR 0081](0081-reserve-workflow-env-for-infra-plumbing.md), +which narrows the "CI workflow injection" delivery mechanism to +infrastructure values.* + ## Context Agents need behavioral knobs — settings that tune *how* they work without @@ -93,7 +99,10 @@ on the host. A config var needed by both must appear in both places. 3. **For CI workflow injection:** The CI workflow sets the value from org secrets, repo variables, or hardcoded defaults. This is the same mechanism - used for all other env vars — no change needed. + used for all other env vars — no change needed. *Note: [ADR 0081](0081-reserve-workflow-env-for-infra-plumbing.md) + narrows this to infrastructure values and CI-runtime-only computed + values; static agent behavior defaults go through harness composition + instead.* ### Defaults diff --git a/docs/ADRs/0055-unified-env-var-delivery.md b/docs/ADRs/0055-unified-env-var-delivery.md index 6b6a260c37..cfbcd7bbc3 100644 --- a/docs/ADRs/0055-unified-env-var-delivery.md +++ b/docs/ADRs/0055-unified-env-var-delivery.md @@ -180,6 +180,9 @@ for harnesses that still reference it. `runner_env` deprecation. - ADR 0049's env var naming convention applies unchanged — the delivery mechanism changes but the `{AGENT}_{SETTING_NAME}` convention does not. + See also [ADR 0080](0080-config-yaml-vs-agent-env-var-scope.md), which + clarifies when a knob should use the `{AGENT}_` env var pattern versus + being a `config.yaml` field. - Modular `.env` files via `host_files` remain the right choice for per-tool env groups shared across multiple harnesses. - This change extends the harness schema; runners older than Phase 1 will diff --git a/docs/ADRs/0080-config-yaml-vs-agent-env-var-scope.md b/docs/ADRs/0080-config-yaml-vs-agent-env-var-scope.md new file mode 100644 index 0000000000..6fa0d2dc38 --- /dev/null +++ b/docs/ADRs/0080-config-yaml-vs-agent-env-var-scope.md @@ -0,0 +1,118 @@ +--- +title: "80. config.yaml vs. agent env vars: where a config option belongs" +status: Accepted +relates_to: + - agent-infrastructure + - governance +topics: + - configuration + - harness + - conventions +--- + +# 80. config.yaml vs. agent env vars: where a config option belongs + +Date: 2026-07-31 + +Amends: [ADR 0049](0049-agent-configuration-env-var-convention.md) + +## Status + +Accepted + +## Context + +[fullsend-ai/agents#567](https://github.com/fullsend-ai/agents/pull/567) +added `TRIAGE_AUTO_CODE` as a harness `env.runner` default in +`harness/triage.yaml`. A reviewer flagged that fullsend-ai/fullsend#1754, +the issue that requested the knob, asked for it to "live in the per-repo/ +per-org config surface" — i.e. `.fullsend/config.yaml` — not a harness +default (see the [review +discussion](https://github.com/fullsend-ai/agents/pull/567#discussion_r3686020058)). +`config.yaml` already carries fields like `create_issues.allow_targets` +that are unrelated to any single agent, per the [governance](../problems/governance.md) +and [agent infrastructure](../problems/agent-infrastructure.md) problem +docs. [ADR 0049](0049-agent-configuration-env-var-convention.md) defines +how agent config env vars are *named* (`{AGENT}_{SETTING_NAME}`) and +requires separate per-agent vars when the same concept is independently +tunable per agent (e.g. `CODE_MAX_FILE_SIZE` vs `REVIEW_MAX_FILE_SIZE`), +but it draws no line against `config.yaml` — nothing in it says when a +knob should live there instead of as an env var, so there was no rule to +check the PR against. + +## Decision + +A config option belongs in exactly one of the two surfaces, based on +whether its behavior is meaningful to more than one agent. This narrows +ADR 0049's per-agent-vars rule (a setting that applies to multiple agents +gets separate vars per agent) to the case where each agent needs its own +independently tunable value; a single value meant to apply the same way +across every agent is a different case, and belongs in `config.yaml` +instead: + +- **Pipeline/dispatch policy — governs whether or how agents run, or + applies the same way across every agent, rather than tuning one agent's + own inference-time logic:** it is a `config.yaml` field. It gets a plain + name with no `{AGENT}_` prefix, and it is not also settable via + environment variable — `config.yaml` (`internal/config` accessors) is + the single source of truth. `roles`, `kill_switch`, and + `create_issues.allow_targets` are existing examples. +- **Single-agent behavior tuning — adjusts how one specific agent does its + own job:** it is an `{AGENT}_`-prefixed env var per ADR 0049, delivered + via that agent's `env.runner`/`env.sandbox`. The prefix matters even + though the var lives in one agent's harness: `.env` files can be sourced + together and `runner_env`/`env.sandbox` can share a host environment, so + the agent name scopes the var and prevents collisions in those shared + contexts (ADR 0049, Consequences). It is not also settable as + a `config.yaml` field — overriding it per repo or org means overriding + the harness (e.g. via `base:` composition, per ADR 0045), not adding a + parallel field to `config.yaml`. + +**Override convention:** `env.runner`/`env.sandbox` values are agent +defaults. A per-repo or per-org override edits the harness (`base:` +composition, per ADR 0045), not the CI workflow `env:` block. +[ADR 0081](0081-reserve-workflow-env-for-infra-plumbing.md) reserves that +block for infrastructure plumbing (credentials, project IDs, regions), +not agent behavior knobs — overriding ADR 0049's "CI workflow injection" +delivery mechanism for behavior knobs specifically; see that ADR for the +full rule and its exceptions. This also means behavior defaults in +`env.runner`/`env.sandbox` must be literals (e.g. `TRIAGE_AUTO_CODE: +"on"`), not shell-style passthrough expressions (e.g. +`${TRIAGE_AUTO_CODE:-on}`) — `env.runner`/`env.sandbox` support `${VAR}` +host-variable expansion (see [ADR 0055](0055-unified-env-var-delivery.md), +§ Runner behavior), not shell default-value syntax, so passthrough syntax +is rejected at harness load: env validation treats `TRIAGE_AUTO_CODE:-on` +as a host variable name and fails with `host variable … is not set`; even +absent validation, `os.Expand` would resolve the whole reference to an +empty string, not the intended default. + +A knob only moves from one surface to the other by a deliberate migration, +not by adding a second way to set the same value. Applying this rule to +`TRIAGE_AUTO_CODE` is a boundary case: it decides whether the code agent +runs next, which sounds like dispatch policy, but that decision is made +inside triage's own post-script, as part of triage's inference-time +behavior — not by a shared dispatch/CLI layer gating multiple agents +uniformly. That keeps it single-agent behavior tuning today, so it +correctly belongs in `harness/triage.yaml` `env.runner`, not +`config.yaml`. If the check is ever lifted out of triage's post-script +into a shared dispatch layer, it becomes pipeline/dispatch policy and +should move to `config.yaml` as a deliberate migration, not before. +fullsend-ai/fullsend#1754's "per-repo/per-org config +surface" request is satisfied by documenting the existing harness override +path (`base:` composition or an org/repo harness copy), not by adding a +`config.yaml` field — the gap the reviewer found is a documentation gap, +not a placement gap. + +## Consequences + +- Resolves the fullsend-ai/agents#567 ambiguity: `TRIAGE_AUTO_CODE` stays + an env var; `docs/agents/.md`'s Variables table must state how to + override it (which harness layer to edit), matching the guidance ADR + 0049 already expects. +- `config.yaml` cannot accumulate `{agent}_foo`-style fields — any such + field is a signal the knob was misplaced. +- An env var cannot quietly gain a `config.yaml` mirror with its own + precedence rules; there is one settable location per config option. +- A knob whose scope grows from one agent to several requires a new + decision (an ADR update or explicit review), not a silent field + addition to `config.yaml`. diff --git a/docs/ADRs/0081-reserve-workflow-env-for-infra-plumbing.md b/docs/ADRs/0081-reserve-workflow-env-for-infra-plumbing.md new file mode 100644 index 0000000000..9d49638c25 --- /dev/null +++ b/docs/ADRs/0081-reserve-workflow-env-for-infra-plumbing.md @@ -0,0 +1,84 @@ +--- +title: "81. Reserve CI workflow env: for infrastructure plumbing, not agent behavior" +status: Accepted +relates_to: + - agent-infrastructure + - governance +topics: + - configuration + - harness + - conventions +--- + +# 81. Reserve CI workflow env: for infrastructure plumbing, not agent behavior + +Date: 2026-07-31 + +Amends: [ADR 0049](0049-agent-configuration-env-var-convention.md) + +## Status + +Accepted + +## Context + +[ADR 0080](0080-config-yaml-vs-agent-env-var-scope.md) states that a +per-repo or per-org override of an agent behavior knob means overriding +the harness (`base:` composition, per [ADR 0045](0045-forge-portable-harness-schema.md)), +not the CI workflow `env:` block. Review of that ADR found the practice +already drifting: fullsend-ai/agents#567's docs describe setting +`TRIAGE_AUTO_CODE` via the workflow `env:` block, "matching the +convention used for `REVIEW_FINDING_SEVERITY_THRESHOLD`" — but +[ADR 0055](0055-unified-env-var-delivery.md)'s own canonical example sets +`REVIEW_FINDING_SEVERITY_THRESHOLD` in the harness's `env.sandbox`, not +the workflow file. Nothing had stated the workflow-env path was out of +bounds for behavior knobs, so nothing caught the drift. + +ADR 0049 lists "CI workflow injection" as one of three delivery +mechanisms for agent config vars, alongside `.env` files and +`runner_env`, without distinguishing infrastructure values (credentials, +project IDs, regions) from agent behavior knobs. + +## Decision + +The CI workflow `env:` block (`.github/workflows/.yml`) is +reserved for infrastructure plumbing — credentials, project IDs, +regions, and other infrastructure values sourced from CI-native inputs +(secrets, org/repo variables). Agent behavior knobs, as scoped by ADR +0080 and named per ADR 0049, are never set there. They go through harness +composition instead: `env.runner`/`env.sandbox` defaults live in the +canonical harness, and a per-repo or per-org override edits those +defaults via `base:` composition (ADR 0045). + +The one exception: a value that can only be computed at CI runtime — +derived from `github.event.*`, a build matrix variable, or a secret that +cannot be expressed as static harness data — may be set in the workflow +`env:` block even if it configures agent behavior. This ADR does not try +to enumerate every such case up front; a genuine new one can be added +here by minor annotation as it turns up. + +This narrows ADR 0049's "CI workflow injection" delivery mechanism: it +remains valid for infrastructure vars and CI-runtime-only values, not for +static agent behavior defaults. + +## Consequences + +- fullsend-ai/agents#567's docs need correcting: `TRIAGE_AUTO_CODE`'s + override path is harness composition, not CI workflow `env:` — the + precedent it cited was itself non-conformant. The fullsend-ai/agents + repo's own `docs/review.md` carries the same non-conformant guidance + for `REVIEW_FINDING_SEVERITY_THRESHOLD` that this PR fixed in + `fullsend`'s copy, and needs the equivalent fix. +- `CODE_ALLOWED_TARGET_BRANCHES: ''` is still hardcoded in + `reusable-code.yml`/`reusable-dispatch.yml`'s workflow `env:` block, + even though [ADR 0053](0053-agent-driven-branch-targeting.md) already + decided this value belongs in the harness's `runner_env`, not the + workflow YAML. It's a pre-existing non-conformance this ADR's rule + makes explicit; removing it from the workflow files is a follow-up, + not part of this decision. +- New agent behavior knobs get one documented override path (harness + `base:` composition), removing the ambiguity between three candidate + mechanisms. +- A workflow `env:` entry that sets an agent behavior default, and isn't + one of the CI-runtime-only exceptions, is a signal the knob was placed + in the wrong layer. diff --git a/docs/agents/review.md b/docs/agents/review.md index b7e8c1ea55..dbeeefff49 100644 --- a/docs/agents/review.md +++ b/docs/agents/review.md @@ -83,9 +83,14 @@ See [Configuring with AGENTS.md](../guides/user/customizing-with-agents-md.md) a |----------|-------------|---------|--------------| | `REVIEW_FINDING_SEVERITY_THRESHOLD` | Minimum severity for findings to include in the review. Findings below this level are omitted from both the narrative body and the posted inline comments. | `low` | `info`, `low`, `medium`, `high`, `critical` | -Set this in the CI workflow `env:` block. The env file passes it to the -sandbox automatically, and the post-script reads it from the runner -environment directly — no separate configuration is needed. +Set this in the harness's `env.sandbox` (the upstream default lives in +`harness/review.yaml`). To override per repo or org, use `base:` +composition ([ADR 0045](../ADRs/0045-forge-portable-harness-schema.md)) +rather than the CI workflow `env:` block — workflow `env:` is reserved +for infrastructure plumbing per +[ADR 0081](../ADRs/0081-reserve-workflow-env-for-infra-plumbing.md). +The post-script reads the value from the runner environment directly — +no separate configuration is needed. The review agent omits findings below the threshold from its output. The post-script also filters the structured `findings` array as diff --git a/docs/architecture.md b/docs/architecture.md index e0deddd5a6..cdd9b7c420 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -119,6 +119,17 @@ repo baseline and overrides) `env.sandbox` in the harness YAML. Each agent documents its config vars in `docs/agents/.md` ([ADR 0049](ADRs/0049-agent-configuration-env-var-convention.md)). +- Config surface boundary: a knob that applies to one agent is an + `{AGENT}_`-prefixed harness env var (never a `config.yaml` field); a + knob that applies across agents or governs dispatch/policy is a + `config.yaml` field (never also an env var) + ([ADR 0080](ADRs/0080-config-yaml-vs-agent-env-var-scope.md)). +- CI workflow `env:` scope: the workflow `env:` block is reserved for + infrastructure plumbing (credentials, project IDs, regions) and values + computable only at CI runtime; agent behavior defaults are set via + harness `env.runner`/`env.sandbox` and overridden through `base:` + composition, never the workflow file + ([ADR 0081](ADRs/0081-reserve-workflow-env-for-infra-plumbing.md)). - Agent-driven branch targeting: the code agent writes its chosen target branch to structured output. The post-script validates the choice against an allowlist and falls back to the repo's auto-detected default branch. diff --git a/docs/guides/user/bring-your-own-agent.md b/docs/guides/user/bring-your-own-agent.md index 2b9878fe51..662637ebbf 100644 --- a/docs/guides/user/bring-your-own-agent.md +++ b/docs/guides/user/bring-your-own-agent.md @@ -289,6 +289,14 @@ security: fail_mode: closed # "closed" (default) or "open" ``` +> **Naming convention:** Prefix settings that tune one agent's behavior with +> that agent's role in caps, e.g. `REVIEW_SEVERITY_THRESHOLD` — this avoids +> collisions when multiple agents share a sandbox or env file. +> +> A setting meant to apply the same way across every agent (like +> `roles` or `create_issues.allow_targets`) belongs in `config.yaml` +> instead, not as an env var. + ### Deprecated fields > **Deprecated:** `runner_env` is deprecated. Use `env.runner`