docs: harness env var literals are not hardcoded mistakes

[ralphbean]

Summary

• Agents reviewing PRs keep flagging literal values in harness env.runner/env.sandbox blocks as hardcoded bugs, suggesting ${VAR} passthrough or a CI workflow env: entry instead.
• Per ADRs 0080 and 0081 in fullsend-ai/fullsend, literals are the intended shape and base: composition is the only supported override path.
• Adds a terse new AGENTS.md section so this stops recurring.

Test plan

• Docs-only change, no tests to run

Assisted-by: Claude Opus 4.6 noreply@anthropic.com

[qodo-code-review]

PR Summary by Qodo

Docs: clarify harness env var literals aren’t hardcoded mistakes

📝 Documentation 🕐 Less than 10 minutes

AI Description

• Document that literal values in harness env blocks are intentional agent defaults.
• Explain supported override mechanism: extend via base: composition.
• Prevent reviewers from suggesting ${VAR} passthrough or CI workflow env: usage.

Diagram

graph TD
  A["PR review agent"] --> B["AGENTS.md guidance"] --> C["Harness env blocks"]
  B --> D["ADR 0080"]
  B --> E["ADR 0081"]
  C --> F["Review harness YAML"]

  subgraph Legend
    direction LR
    _actor["Agent/Reviewer"] ~~~ _doc["Documentation"] ~~~ _cfg["Config/YAML"] ~~~ _ext{{"External ADR"}}
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Encode the rule in reviewer/agent linting or prompts

• ➕ Prevents recurring suggestions automatically, without relying on humans reading AGENTS.md
• ➕ Can be applied consistently across repos/PRs
• ➖ Requires changes to agent infrastructure/configuration and rollout coordination
• ➖ Harder to discover/justify without nearby written rationale

2. Add inline comments in harness YAML near env blocks

• ➕ Guidance is colocated with the configuration that triggers the confusion
• ➕ Helps even when AGENTS.md isn’t consulted
• ➖ Can clutter YAML and must be repeated across harness files
• ➖ Still doesn’t centralize the ADR rationale and override policy as cleanly

Recommendation: Keeping the guidance in AGENTS.md is the best first step: it centralizes the policy and ties it directly to the ADR rationale reviewers should follow. If the confusion persists, layering an automated reviewer/agent rule on top would reduce recurrence further.

Files changed (1) +16 / -0

Documentation (1) +16 / -0

AGENTS.mdAdd guidance that harness env literals are intended defaults +16/-0

Add guidance that harness env literals are intended defaults

• Introduces a new section explaining that literal values in harness 'env.runner'/'env.sandbox' blocks are the intended default shape. Documents that overrides must be done via 'base:' composition and explicitly warns against '${VAR}' passthrough or CI workflow 'env:' overrides, citing ADR 0080/0081.

AGENTS.md

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:29 PM UTC · Completed 7:41 PM UTC

Commit: 4efba1d · View workflow run →

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Protected AGENTS.md modified ✗ Dismissed 📜 Skill insight § Compliance

Description

This PR modifies AGENTS.md, which is a protected governance/infrastructure path and must not be
auto-approved; it requires explicit human review. Ensure the change is manually reviewed and that
the PR’s justification/authorization (e.g., linked issue/ADR) is clearly recorded for auditability.

Code

AGENTS.md[R115-118]

+## 8. Harness env var literals are not "hardcoded" mistakes
+
+A literal value in a harness `env.runner`/`env.sandbox` block (e.g.
+`REVIEW_FINDING_SEVERITY_THRESHOLD: "low"` in `harness/review.yaml`)

Relevance

●●● Strong

Repo has repeated protected-path governance checks; changes should require explicit human
review/justification.

PR-#569
PR-#184

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The compliance checklist explicitly marks AGENTS.md as a protected governance/infrastructure path;
this PR adds a new section to that file, so the protected-path rule is triggered and requires human
review (no auto-approval).

AGENTS.md[115-129]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`AGENTS.md` is a protected governance/infrastructure file; changes to it must not be auto-approved and should carry explicit authorization/justification.

## Issue Context
This PR adds new reviewer guidance in `AGENTS.md`. Because the file is on the protected-path list, the repository policy requires a compliance finding to ensure human review and clear authorization.

## Fix Focus Areas
- AGENTS.md[115-129]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Overbroad ${VAR} claim ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

AGENTS.md says converting literals to ${VAR} passthrough will “fail validation,” but this repo’s
harness files already use ${VAR} host-variable expansion in env.runner/forge.github.env for
required runtime values. The wording should be narrowed to the real pitfall: ${VAR} is host
expansion (not shell defaulting) and will change semantics and/or fail only when the referenced host
variable is unset or when using unsupported defaulting syntax like :-.

Code

AGENTS.md[R125-126]

+`${VAR}` passthrough (harness env blocks resolve `${VAR}` as
+host-variable expansion, not shell defaulting, and fail validation) or

Relevance

●●● Strong

Narrowing absolute docs claims is typically accepted; similar doc rewording to avoid incorrect
guidance was accepted.

PR-#326

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

AGENTS.md introduces an absolute claim that ${VAR} passthrough “fail[s] validation,” but the repo
itself uses ${VAR} expansion in harness env blocks for required host-provided variables,
indicating ${VAR} is supported and not inherently invalid. Separately, harness/review.yaml shows
the intended pattern for behavior defaults: literals in env.runner/env.sandbox with overrides
via base: composition, so the guidance should focus on “don’t replace defaults with ${VAR}”
rather than implying ${VAR} is invalid.

AGENTS.md[115-129]
harness/code.yaml[49-54]
harness/review.yaml[42-55]
FEATURES.md[31-37]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new AGENTS.md guidance states that `${VAR}` passthrough in harness `env.*` blocks “fail[s] validation”. In this repo, `${VAR}` expansion is already a supported/intentional pattern for required runtime-provided values (e.g., secrets/CI-provided vars), so the statement is too absolute and may mislead reviewers into rejecting valid `${VAR}` usage.

## Issue Context
- `${VAR}` is used in harness configs in this repo (e.g., `harness/code.yaml`) and is documented there as runner-environment expansion.
- The intended warning appears to be about using `${VAR}` *as a defaulting mechanism* (shell-style `${VAR:-default}`), which the harness doesn’t appear to use, and/or about the failure mode when the referenced host variable is unset.

## Fix Focus Areas
- AGENTS.md[115-129]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context

✅ Compliance rules (platform): 55 rules

✅ Skills: 4 invoked

  code-review

  code-implementation

  pr-review

  docs-review

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[fullsend-ai-review]

Review

Findings

High

• [protected-path] AGENTS.md — This PR modifies AGENTS.md, which is under protected-path enforcement. The PR has no formally linked issue authorizing changes to this governance file. The content is technically sound — the correctness sub-agent verified all claims about harness env var literal semantics, base: composition overrides, and ${VAR} host-variable expansion behavior against the actual harness files. However, human approval is always required for protected-path changes regardless of content quality.
  Remediation: Obtain human maintainer approval for this protected-path change.

---

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

Review

Findings

High

• [protected-path] AGENTS.md — This PR modifies AGENTS.md, which is under protected-path enforcement. The PR has no formally linked issue authorizing changes to this governance file. The content is technically sound — the correctness sub-agent verified all claims about harness env var literal semantics, base: composition overrides, and ${VAR} host-variable expansion behavior against the actual harness files. However, human approval is always required for protected-path changes regardless of content quality.
  Remediation: Obtain human maintainer approval for this protected-path change.

---

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 (2)

Review

Findings

High

• [protected-path] AGENTS.md — This PR modifies AGENTS.md, which is under protected-path enforcement. The PR has no formally linked issue authorizing changes to this governance file. The content itself is technically sound — claims about harness env var literal semantics, base: composition overrides, and ${VAR} host-variable expansion behavior are consistent with the referenced ADRs and verified against harness/review.yaml. However, human approval is always required for protected-path changes regardless of content quality.
  Remediation: Obtain human maintainer approval for this protected-path change.

---

Labels: PR is a docs-only change to AGENTS.md addressing recurring review-agent false positives about harness env var literals

---

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

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 7:42 PM UTC · Completed 7:46 PM UTC

Commit: 4efba1d · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (bot-triggered)

No code changes made. The sole review finding is a protected-path governance requirement (AGENTS.md requires human maintainer approval). The reviewer confirmed the content is technically sound. The fix agent cannot fulfill this finding — human maintainer approval must be obtained directly.

Disagreed (1):

1. protected-path: AGENTS.md requires human maintainer approval: The review agent's sole finding is a governance/process requirement — AGENTS.md is a protected path and human approval is always required for changes to it. The reviewer explicitly acknowledges the content is 'technically sound' and 'consistent with the referenced ADRs.' The remediation ('Obtain human maintainer approval') is a human action, not a code fix. The fix agent cannot approve PRs, modify labels, or perform any PR mutation that would satisfy this finding. No code changes are needed or appropriate.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[waynesun09]

Review sweep — two findings below, posted as inline comments. Not requesting changes; flagging for maintainer awareness.

[ralphbean]

Re: #731 (comment)

Yeah, the protected-path flag here is expected — this PR's whole reason for existing is to add guidance to AGENTS.md, so touching it isn't a bypass, it's the point. Treating this as the human review/approval step now.

[ralphbean]

Re: #731 (comment)

Both findings here are the same two raised as inline comments — already replied to and resolved those threads (protected-path touch is intentional, and the ${VAR} wording got narrowed with a scoping clause).

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:28 PM UTC · Completed 2:40 PM UTC

Commit: e36efc5 · View workflow run →

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 2:41 PM UTC · Completed 2:45 PM UTC

Commit: e36efc5 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (bot-triggered)

No code changes required. The review agent's only finding is that AGENTS.md is a protected path requiring human maintainer approval — a governance action the fix agent cannot perform. The review agent explicitly confirmed the content is technically sound. A human maintainer must approve this PR.

Disagreed (1):

1. protected-path: AGENTS.md requires human maintainer approval: The review agent confirms the content is technically sound — all claims about harness env var literal semantics, base: composition overrides, and ${VAR} host-variable expansion behavior were verified against the actual harness files. The sole finding is that AGENTS.md is a protected path requiring human maintainer approval, which is a governance/process action outside the fix agent's scope. No code changes are needed or appropriate. A human maintainer must review and approve this protected-path change directly.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[waynesun09]

Review sweep — one inline finding below, plus one additional finding on the PR description that doesn't map to a diff line.

[MEDIUM] PR description cites unrelated PR #682 as evidence of the recurring reviewer-flagging problem

The PR body states: "Agents reviewing PRs (see #682) keep flagging literal values in harness env.runner/env.sandbox blocks as hardcoded bugs." Checked PR #682 directly (title "feat(#677): add needs_input pushback for the code agent"): its diff, description, and all review comments concern a needs_input schema field and post-script behavior for the code agent's PR-vs-no-PR pushback path. There is no mention anywhere in #682 of harness env var literals, REVIEW_FINDING_SEVERITY_THRESHOLD, or hardcoded-value findings. The cited evidence does not support the claimed recurring pattern; this reads as an unverified/incorrect citation rather than a fact-checked reference.

Suggestion: fix the citation to point at the PR(s) where this pattern actually occurred, or drop the parenthetical reference entirely.

Not requesting changes; flagging for maintainer awareness.

[ralphbean]

/fs-fix address the "[MEDIUM] AGENTS.md drops ADR 0081's CI-runtime-only-value exception for the workflow env: prohibition" finding

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 8:36 PM UTC · Completed 8:41 PM UTC

Commit: e36efc5 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 1 finding: added the ADR 0081 CI-runtime exception clause to the workflow env: prohibition so reviewers don't incorrectly flag legitimate CI-runtime-computed values.

Fixed (1):

1. AGENTS.md drops ADR 0081's CI-runtime-only-value exception for the workflow env: prohibition (AGENTS.md): Added exception clause after the workflow env: block prohibition noting that ADR 0081 carves out CI-runtime-only values (from github.event.*, build matrix variables, or non-static secrets) which may live in the workflow env: block even if they configure agent behavior.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:42 PM UTC · Completed 8:52 PM UTC

Commit: 89dfb3f · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 8:53 PM UTC · Completed 8:56 PM UTC

Commit: 89dfb3f · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 2 (bot-triggered)

No code changes needed. The review's only finding is a protected-path governance requirement (AGENTS.md requires human maintainer approval), which the fix agent cannot fulfill. The reviewer confirms the content is technically sound. A human maintainer has already acknowledged and accepted the protected-path touch in the PR comments.

Disagreed (1):

1. protected-path: AGENTS.md requires human maintainer approval: The review agent's sole finding is a governance/process requirement — AGENTS.md is a protected path and human approval is always required regardless of content quality. The reviewer explicitly confirms the content is 'technically sound' with all claims verified against the actual harness files. A human maintainer has already acknowledged this in the PR comments ('the protected-path flag here is expected — this PR's whole reason for existing is to add guidance to AGENTS.md, so touching it isn't a bypass, it's the point. Treating this as the human review/approval step now.'). The fix agent cannot approve PRs, modify labels, or perform governance actions — this finding requires human action, not a code change.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[waynesun09]

Review sweep — one finding below on the PR description (no diff line applies).

[MEDIUM] PR description cites unrelated PR #682 as evidence of the recurring problem

The PR body states: "Agents reviewing PRs (see #682) keep flagging literal values in harness env.runner/env.sandbox blocks as hardcoded bugs." Directly fetched PR #682 (title: "feat(#677): add needs_input pushback for the code agent") — its body and scope are entirely about a needs_input schema field and post-script pushback behavior for the code agent; there is no mention of harness env var literals, REVIEW_FINDING_SEVERITY_THRESHOLD, or a hardcoded-value finding anywhere in it. This citation defect has not been raised or acknowledged in any prior review round on this PR.

Suggestion: fix the citation to point at the PR(s)/issue where this pattern actually recurred, or drop the parenthetical reference entirely — an incorrect evidentiary citation undermines the stated justification for adding this rule.

[ralphbean]

Suggestion: fix the citation to point at the PR(s)/issue where this pattern actually recurred, or drop the parenthetical reference entirely — an incorrect evidentiary citation undermines the stated justification for adding this rule.

I just dropped it.

[waynesun09]

LGTM. Docs-only guideline that codifies the ADR 0080/0081 stance so reviewers stop flagging static harness env literals as hardcoded bugs. The static-default vs. computed-passthrough distinction is clearly drawn and the examples (CODE_ALLOWED_TARGET_BRANCHES, REVIEW_TOKEN, PR_NUMBER, etc.) match the actual harness files.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 9:48 PM UTC · Completed 10:00 PM UTC

Commit: 89dfb3f · View workflow run →

[fullsend-ai-retro]

Retro: PR #731 — docs: harness env var literals are not hardcoded mistakes

Timeline: Human-authored docs PR adding Section 8 to AGENTS.md, codifying ADR 0080/0081 guidance to stop review agents from flagging literal harness env var defaults as bugs. 4 commits over Aug 10–13 (3 human, 1 fix-agent), 3 review runs, 4 fix runs, substantive human review from waynesun09 and Qodo. Merged after waynesun09 LGTM.

What worked well:

• Fix agent correctly identified protected-path governance as requiring human action and disagreed appropriately in all 3 bot-triggered runs
• Human-triggered /fs-fix successfully addressed waynesun09's ADR 0081 exception finding (commit 89dfb3f)
• waynesun09 and Qodo reviews were thorough — they identified 3 substantive docs-quality gaps (overbroad ${VAR} claim, missing static-vs-computed scope, missing ADR 0081 CI-runtime exception) that significantly improved the final guidance
• The review agent correctly verified the accuracy of stated claims against actual harness files

Evidence for existing issues:

• Fix agent governance loop (fullsend#1068, #902): 3 of 4 fix runs were bot-triggered no-ops against a protected-path-only finding. The review agent structurally cannot approve PRs touching protected paths (schema-enforced), and the fix agent structurally cannot modify protected-path files, creating a deterministic no-op cycle. This PR burned 3 wasted fix runs before the iteration cap would have kicked in.
• Concurrent PR collision detection (agents#334): waynesun09 caught that PRs docs: harness env var literals are not hardcoded mistakes #731 and docs(#581): document review pipeline propagation requirement #583 both add ## 8. sections to AGENTS.md. The review agent did not detect this.
• Docs-only PR routing (agents#373): The docs-currency sub-agent is designed for staleness detection (code changes → stale docs) and had nothing to analyze on this docs-only PR.

New finding (1 proposal below): The correctness sub-agent verified the accuracy of stated claims but did not check completeness against the cited ADRs. Two of the three human findings (missing ADR 0081 exception, missing static-vs-computed scoping) were discoverable by reading the referenced ADRs and comparing their full provisions against the documentation's summary. Related to but distinct from agents#638 (following ADR references in code, not verifying completeness of docs citing ADRs) and agents#265 (misleading narrative synthesis, not omitted provisions).

Proposals filed

• Correctness sub-agent should verify docs cover all significant provisions of cited ADRs (in fullsend-ai/agents)