Skip to content

Add AGENTS.md review guidance for .fullsend/ harness configuration changes #3531

Description

@fullsend-ai-retro

What happened

PR #3523 attempted to fix broken ci-diagnose and e2e-triage agent harnesses by committing a local copy of .fullsend/env/gcp-vertex.env — a scaffold file that fullsend v0.33 deliberately deleted. The review agent (run 33637056214) recommended approval after verifying no secrets were committed, path mappings were correct, and env var expansion was sound. However, it did not flag that the approach was architecturally wrong: fullsend v0.33 removed this file as part of migrating env var delivery to env.sandbox (ADR 0055). The human reviewer (durandom) caught this ~1.5 hours later and opened the correct fix in PR #3527, which removes the host_files reference and passes Vertex env vars via env.sandbox instead.

What could go better

The review agent lacks context about fullsend's harness architecture — specifically, that .fullsend/env/ files are scaffold-managed artifacts that should not be committed locally, and that env.sandbox is the current mechanism for passing environment variables to custom harness sandboxes. Without this context, the review agent cannot distinguish a correct harness config change from one that fights against the platform's architectural direction.

This is a high-confidence finding: the review agent's comment explicitly verified implementation correctness (paths, secrets, expansion) but had zero awareness of the scaffold lifecycle or the env.sandbox migration. The gap is purely informational — the agent had no way to know that restoring this file was wrong.

Note: platform-level fixes for this class of issue exist as open proposals (fullsend-ai/fullsend#1472, #3476), but those are longer-term. Repo-specific AGENTS.md guidance can close this gap immediately for this repo's review agent.

Proposed change

Add a new section to AGENTS.md (after the existing 'E2E Nightly Fix Conventions' section) covering .fullsend/ harness configuration review guidance. The section should include:

  1. Scaffold vs local files: Files under .fullsend/env/ are scaffold-managed by the fullsend CLI. Do not commit new env files there or restore deleted ones — if upstream fullsend removed a file, the removal was intentional.
  2. env.sandbox is the preferred mechanism: Custom harnesses (ci-diagnose, e2e-triage) should pass environment variables to the sandbox via the env.sandbox section in harness YAML (per ADR 0055), not via host_files referencing scaffold env files.
  3. Review checklist for .fullsend/ changes: When reviewing PRs that add or modify files under .fullsend/, verify the change follows fullsend's current architecture rather than reverting to deprecated patterns. Check whether host_files entries reference scaffold paths that may have been removed upstream.

This guidance is complementary to the existing prohibited-modifications list in 'E2E Nightly Fix Conventions' (which lists .fullsend/ as off-limits for E2E fix PRs but doesn't explain the harness architecture).

Validation criteria

The next review agent run on a PR that modifies .fullsend/ harness configuration should reference env.sandbox or ADR 0055 in its review when the change involves environment variable delivery. Specifically: if a PR adds a host_files entry referencing a scaffold env file, the review agent should flag it as potentially using a deprecated pattern. Validate on the next 3 PRs touching .fullsend/rhdh/harness/ files.


Generated by retro agent from #3523

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions