-
Notifications
You must be signed in to change notification settings - Fork 94
docs(adr): ADR 0047 — agent configuration env var convention #2334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
81848a5
5ce3e65
dce83dd
f77a94b
6cf0bb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,186 @@ | ||
| --- | ||
| title: "49. Agent configuration environment variable convention" | ||
| status: Accepted | ||
| relates_to: | ||
| - agent-architecture | ||
| - agent-infrastructure | ||
| topics: | ||
| - configuration | ||
| - harness | ||
| - agents | ||
| - conventions | ||
| --- | ||
|
|
||
| # 49. Agent configuration environment variable convention | ||
|
|
||
| Date: 2026-06-16 | ||
|
|
||
| ## Status | ||
|
|
||
| Accepted | ||
|
|
||
| ## Context | ||
|
|
||
| Agents need behavioral knobs — settings that tune *how* they work without | ||
| changing the agent definition itself. Issue | ||
| [#2333](https://github.com/fullsend-ai/fullsend/issues/2333) surfaced | ||
| a concrete case: the review agent should let repo owners set a minimum | ||
| severity threshold for reported findings. More knobs will follow for other | ||
| agents. | ||
|
|
||
| The harness already delivers environment variables into the sandbox via `.env` | ||
| files with `expand: true` | ||
| ([ADR 0024](0024-harness-definitions.md)), and pre/post scripts read env vars | ||
| from `runner_env` ([ADR 0045](0045-forge-portable-harness-schema.md)). The | ||
| infrastructure for carrying configuration exists. What is missing is a | ||
| **naming convention** that establishes a consistent pattern for every agent | ||
| going forward. | ||
|
|
||
| This ADR covers only **agent configuration** env vars — behavioral knobs that | ||
| tune agent behavior. It does not retroactively rename existing context vars | ||
| (event data like `GITHUB_PR_URL`, `ISSUE_NUMBER`) or infrastructure vars | ||
| (tokens, paths, credentials). Those remain as they are. | ||
|
|
||
| ## Decision | ||
|
|
||
| Agent configuration environment variables follow a single convention: | ||
|
|
||
| ### Naming | ||
|
|
||
| ``` | ||
| {AGENT}_{SETTING_NAME} | ||
| ``` | ||
|
|
||
| - `{AGENT}` is the agent's **name** in uppercase, derived from the harness | ||
| filename: `REVIEW`, `CODE`, `TRIAGE`, `FIX`, `PRIORITIZE`, `RETRO`, etc. | ||
| - `{SETTING_NAME}` is `SCREAMING_SNAKE_CASE` describing the setting. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] internal-consistency The ADR lists CODE as the role prefix for the code agent (example: CODE_MAX_FILE_SIZE), but the actual harness role for the code agent is coder (see internal/scaffold/fullsend-repo/harness/code.yaml role field). Similarly, the fix agent shares role: coder. The ADR does not clarify whether the env var prefix should match the harness role field value (which would be CODER_MAX_FILE_SIZE) or a human-friendly shorthand. Two agents with the same role value but different env var prefixes (CODE_ vs FIX_) creates further inconsistency. Suggested fix: Either (a) update the examples to use CODER_ to match the actual role value, or (b) explicitly state that the prefix is a human-readable short name that may differ from the harness role field, and document the canonical prefix for each agent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] internal-consistency The ADR states the prefix is derived from the harness filename but does not explicitly contrast this with the role field. Both code.yaml and fix.yaml share role: coder, which could cause confusion about whether the prefix is CODE/FIX (filename) or CODER (role). Suggested fix: Add an explicit statement: The prefix is the harness YAML filename stem (e.g., code from code.yaml), NOT the role field value. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] internal-consistency The ADR states the {AGENT} prefix is derived from the harness filename but does not contrast this with the harness role field, which uses different values: code.yaml and fix.yaml both have role: coder. ROLE_APP_IDS in the codebase uses the role field value as its key, so a reader may assume config var prefixes also derive from role. Suggested fix: Add a brief note: The prefix is always the harness filename stem, not the role field value. For example, code.yaml and fix.yaml both have role: coder, but their config var prefixes are CODE_ and FIX_ respectively. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] internal-consistency The ADR states the {AGENT} prefix is derived from the harness filename, but does not contrast this with the harness role field, which uses different values: code.yaml and fix.yaml both have role: coder. A reader familiar with ROLE_APP_IDS may assume config var prefixes also derive from role. Suggested fix: Add a brief note in the Naming section clarifying the prefix is the harness filename stem, not the role field value. |
||
| - Examples: `REVIEW_SEVERITY_THRESHOLD`, `CODE_MAX_FILE_SIZE`, | ||
| `REVIEW_POST_INLINE`, `TRIAGE_SKIP_DUPLICATE_CHECK`. | ||
| - A setting that applies to multiple agents gets separate vars per agent | ||
| (e.g., `CODE_MAX_FILE_SIZE` and `REVIEW_MAX_FILE_SIZE`), keeping each | ||
| agent's configuration independent. | ||
|
|
||
| The agent name prefix prevents collisions when multiple agents share an | ||
| execution environment or when env files are sourced together. Existing context | ||
| vars (e.g., `PRIOR_REVIEW_SHA`) and credential vars (e.g., `FIX_GH_TOKEN`) | ||
| already use agent-name prefixes — the `{AGENT}_` prefix alone does not | ||
| distinguish config vars from those. The distinction is by purpose and | ||
| documentation: config vars are behavioral knobs listed in | ||
| `docs/agents/<agent>.md`. | ||
|
|
||
| ### Where config vars live in the harness | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Low] This section enumerates three delivery paths but doesn't mention providers (OpenShell credential injection). While providers are for credentials rather than behavioral config knobs, explicitly noting them as out of scope would make the coverage complete and prevent readers from wondering whether config vars could use the provider mechanism.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The scope paragraph already excludes credentials/infrastructure vars, which is what providers carry. Leaving as-is.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, I thought that mentioning the word It's only for clarification so if you think it's ok as is, it's not a blocker for me :) |
||
|
|
||
| Config vars are carried the same way as other agent env vars — no new schema | ||
| fields are needed. The `.env` file and `runner_env` serve different | ||
| audiences: the `.env` file delivers vars into the sandbox for the agent at | ||
| inference time, while `runner_env` makes vars available to pre/post scripts | ||
| on the host. A config var needed by both must appear in both places. | ||
|
|
||
| 1. **For sandbox access (inference time):** Add the variable to the agent's | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Low] The ADR doesn't explicitly recommend keeping all config vars for an agent in its existing
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The text already says "the agent's
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, when I read it I thought of this because it wasn't clear enough. I guess that I was reading "the .env file" as one of multiple possible env files, because we currently use several env files per agent (because of the vertex env vars and other contextual env vars) so it might be confusing talking in singular here. I agree everything is there already: config vars, talking in singular, previously define that config vars are behavioural knobs... so if you think it's clear it's ok for me :)
maruiz93 marked this conversation as resolved.
|
||
| `.env` file (e.g., `env/review.env`) with `${VAR}` expansion. The harness | ||
| `host_files` entry with `expand: true` resolves the value from the host | ||
| environment before copying into the sandbox. The agent reads it at runtime. | ||
|
|
||
| 2. **For pre/post scripts (host side):** Add the variable to the harness's | ||
| `runner_env` or the forge-specific `runner_env` block. Scripts read it | ||
| from the environment. This is independent of the `.env` file — `runner_env` | ||
| controls the host-side environment, not the sandbox. | ||
|
|
||
| 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. | ||
|
|
||
| ### Defaults | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which is the reationale to not have the defaults on the harness?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, the very nature of a default is that it isn't present in the config file, right? How could they be in the harness and still be default values - values that are used when the harness doesn't supply them?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fact that something is a default depends on the level of abstraction your are at. A default value for a flag on a command is a default value if it lives on the command flag, or should be resolved just before execution on the same function it is making use of it? The standard level to look at this is the user level, so it is a default if the user does not see it. In our future the user won't see the harness, it will inherit from it and add values to it. So it is a default value to them. If they choose to write the full harness without inheriting, then it is required. We move the defaults to the agent .md, what if they want to overwrite the full agent .md file? Then it is not a default anymore. It is the problem of having too many levels. If the standard use case is to have users inherit the harness, I would have them the defaults on the harness.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see - move defaults to the canonical harness file.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense — rewrote the Defaults section. Defaults now live in the canonical harness, downstream layers override via |
||
|
|
||
| Default values live in the **canonical harness** (the scaffold's | ||
| `harness/<agent>.yaml`). Downstream layers — the org `.fullsend` repo or a | ||
| per-repo `.fullsend/` — override them via `base` composition | ||
| ([ADR 0045](0045-forge-portable-harness-schema.md)). Defaults are also | ||
| **documented** in `docs/agents/<agent>.md` so users can discover them without | ||
| reading harness YAML. | ||
|
|
||
| **For agent prompts,** the agent treats an unset or empty variable the same as | ||
| "use the default." The `.env` file's `expand: true` mechanism resolves unset | ||
| host vars to an empty string, not an absent var — so agents and scripts must | ||
| handle both cases. | ||
|
|
||
| **For pre/post scripts,** use standard shell defaulting, which already handles | ||
| both empty and unset: `${REVIEW_SEVERITY_THRESHOLD:-low}`. | ||
|
|
||
| ### Documentation | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [info] edge-case-handling The default for REVIEW_SEVERITY_THRESHOLD is low, meaning info-level findings are suppressed by default. Internally consistent but the ADR does not call out that the default filters out a severity level. |
||
|
|
||
| Each agent's user-facing documentation (`docs/agents/<agent>.md`) includes a | ||
| **Variables** subsection under the existing "Configuration and extension" | ||
| section: | ||
|
|
||
| ```markdown | ||
| ## Configuration and extension | ||
|
|
||
| See [Customizing with AGENTS.md](../guides/user/customizing-with-agents-md.md) and | ||
| [Customizing with Skills](../guides/user/customizing-with-skills.md). | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] runtime-mechanism-correctness The severity filtering mechanism at inference time relies entirely on the LLM faithfully following system prompt instructions. There is no programmatic enforcement. The ADR does not acknowledge that inference-time config var handling is best-effort and non-deterministic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] runtime-mechanism-correctness The severity filtering example at inference time relies entirely on the LLM following system prompt instructions with no programmatic enforcement. The ADR does not acknowledge that inference-time config var handling is best-effort rather than deterministic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [info] runtime-mechanism-correctness The severity filtering example at inference time relies entirely on the LLM following system prompt instructions with no programmatic enforcement. This is inherent to the agent architecture rather than a gap specific to this ADR. |
||
| ### Variables | ||
|
|
||
| | Variable | Description | Default | Valid values | | ||
| |----------|-------------|---------|--------------| | ||
| | `REVIEW_SEVERITY_THRESHOLD` | Minimum severity for reported findings | `low` | `info`, `low`, `medium`, `high`, `critical` | | ||
| | `REVIEW_POST_INLINE` | Post inline comments on individual findings | `true` | `true`, `false` | | ||
| ``` | ||
|
|
||
| This is the single place a user looks to discover what knobs an agent | ||
| supports. Every agent doc includes this subsection for consistency — agents | ||
| that accept no configuration vars state "None" in the section. The agent's | ||
| system prompt (`agents/<agent>.md`) references config vars wherever they are | ||
| naturally needed in the instructions — no prescribed section structure. | ||
|
|
||
| ### Using config vars at inference time | ||
|
|
||
| The agent's system prompt references config vars in context where the | ||
| behavior is conditioned. For example, in the review agent: | ||
|
|
||
| ```markdown | ||
| ## Severity filtering | ||
|
|
||
| If `$REVIEW_SEVERITY_THRESHOLD` is set, suppress findings below that level. | ||
| The severity order is: info < low < medium < high < critical. Suppressed | ||
| findings do not appear in the output — they are dropped entirely, not | ||
| downgraded. | ||
| ``` | ||
|
|
||
| The agent reads the value from its sandbox environment (e.g., via | ||
| `printenv REVIEW_SEVERITY_THRESHOLD` or by referencing it in tool calls) | ||
| and conditions its behavior accordingly. This is no different from how | ||
| agents already read `$GITHUB_PR_URL` or `$ISSUE_NUMBER`. | ||
|
|
||
| ### Precedence | ||
|
|
||
| Config var values follow the existing harness layering from | ||
| [ADR 0045](0045-forge-portable-harness-schema.md) and | ||
| [ADR 0003](0003-org-config-repo-convention.md): fullsend defaults (scaffold) | ||
| can be overridden by the org `.fullsend` repo, which can be overridden by | ||
| per-repo `.fullsend/`. This layering already applies to `.env` files and | ||
| `runner_env` — config vars inherit it for free. | ||
|
|
||
| ## Consequences | ||
|
|
||
| - **No runner changes required.** The convention uses existing env var | ||
| delivery mechanisms (`host_files` with `expand: true`, `runner_env`, | ||
| CI workflow `env:`). Agents start accepting config vars immediately by | ||
| documenting them and referencing them in their prompts and scripts. | ||
| - **Discoverability is centralized.** Users check `docs/agents/<agent>.md` | ||
| to see what knobs an agent supports. Agent authors document new config | ||
| vars there when adding them. | ||
| - **Collision-free by convention.** The `{AGENT}_` prefix scopes config vars | ||
| to the agent that owns them. | ||
| - **Agent system prompts stay flexible.** There is no required section | ||
| structure for how `agents/<agent>.md` references config vars. Agent | ||
| authors place references where they make sense in the prompt flow. | ||
| - **Each new config var may require updates in several places:** | ||
| 1. Agent `.env` file (sandbox delivery) | ||
| 2. Harness `runner_env` (host-side script access) | ||
| 3. Agent system prompt (behavioral conditioning) | ||
| 4. Pre/post scripts (host-side logic) | ||
| 5. `docs/agents/<agent>.md` (user documentation) | ||
|
|
||
| Not every var needs all five — a var used only at inference time skips 2 | ||
| and 4; a var used only in scripts skips 1 and 3. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -91,6 +91,11 @@ The harness draws its configuration from the adopting organization's **`.fullsen | |
| runner_env) from platform-neutral fields. Forge blocks inherit from | ||
| top-level defaults and override only deltas | ||
| ([ADR 0045](ADRs/0045-forge-portable-harness-schema.md)). | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] naming-convention-ambiguity The ADR addresses the overlap with existing credential vars (FIX_GH_TOKEN) by noting the distinction is by purpose and documentation, but does not explain why credential vars use a FULLSEND_ namespace prefix while config vars do not. |
||
| - Agent configuration env vars: behavioral knobs use `{AGENT}_{SETTING_NAME}` | ||
| naming (e.g., `REVIEW_SEVERITY_THRESHOLD`), delivered via existing env var | ||
| mechanisms (`.env` files, `runner_env`). Each agent documents its config | ||
| vars in `docs/agents/<agent>.md` | ||
| ([ADR 0049](ADRs/0049-agent-configuration-env-var-convention.md)). | ||
|
|
||
| **Open questions:** | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[high] adr-number-collision
ADR number collision: this PR adds 0047-agent-configuration-env-var-convention.md, but docs/ADRs/0047-vendored-installs-with-vendor-flag.md already exists on main. ADR 0048 also exists. Must renumber to 0049.
Suggested fix: Renumber this ADR to 0049. Update the title, filename, docs/architecture.md reference, and the heading inside the file accordingly.