docs: add configuration checklist for agent feature work - #566
Conversation
PR Summary by QodoDocs: add configuration checklist for agent configuration work
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
|
🤖 Finished Review · ✅ Success · Started 3:02 PM UTC · Completed 3:17 PM UTC |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
Previous run (3)ReviewFindingsMedium
Previous run (4)ReviewFindingsLow
Previous run (5)ReviewFindingsLow
Previous run (6)ReviewFindingsMedium
Low-severity observationsLow
Previous run (7)Looks good to me Low-severity observationsLow
Previous run (8)Looks good to me Previous run (9)ReviewFindingsLow
Previous run (10)ReviewFindingsLow
Previous run (11)ReviewFindingsMedium
Low
Labels: PR adds new documentation file (FEATURES.md) and is purely a documentation change Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
documentation
|
🤖 Finished Fix · ❌ Failure · Started 3:19 PM UTC · Completed 3:25 PM UTC |
- Document both env var patterns (env.runner/sandbox vs runner_env) in the harness placement table with guidance to match existing convention - Replace nonexistent `make lint` with `make test` in review checklist - Note that `## Inputs` may need to be created in agent definition files Addresses review feedback on #566
|
The fix agent completed, but the post-fix script failed before finishing. Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/30555954743 Details: |
|
🤖 Finished Review · ✅ Success · Started 3:26 PM UTC · Completed 3:42 PM UTC |
Superseded by updated review
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
the
ready-for-merge
|
🤖 Finished Review · ✅ Success · Started 4:02 PM UTC · Completed 4:17 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 4:42 PM UTC · Completed 4:55 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Review-only pass: 4 findings below, none overlapping the existing bot/human comment threads on this PR (checked by file, line proximity, and issue substance).
|
🤖 Finished Review · ✅ Success · Started 5:38 PM UTC · Completed 5:53 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
🤖 Finished Review · ✅ Success · Started 6:17 PM UTC · Completed 6:32 PM UTC |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Review-only pass; posted one additional finding not yet covered by prior threads.
- Pin --output-dir /tmp/fullsend in LOCAL.md examples so the cat glob in step 4 works on macOS, where os.TempDir() differs from Linux. - Note the pinned openshell version in Prerequisites to avoid an opaque sandbox pre-flight failure on older Homebrew builds. - Cite ADR 0080/0081 in the env var guidance in FEATURES.md. - Fix issue citation from fullsend-ai/fullsend#5408 (bug report) to #5409 (the merged fix) in FEATURES.md and docs/triage.md. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 8:49 PM UTC · Completed 9:04 PM UTC |
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Review-only pass: 3 new findings, verified against the current PR head (7452b23) and upstream fullsend-ai/fullsend source/ADRs, deduplicated against existing bot/human comment threads on this PR (checked by file, line proximity, and issue substance).
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
FEATURES.md provides a step-by-step checklist for adding configurable options to agents, covering env var placement, agent prompt updates, pre/post script changes, schema updates, skill impact, and documentation. CONTRIBUTING.md links to it at the top. Motivated by fullsend/fullsend#1754 (triage agent configuration for code) as an example of the kind of change that touches multiple layers. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
- Document both env var patterns (env.runner/sandbox vs runner_env) in the harness placement table with guidance to match existing convention - Replace nonexistent `make lint` with `make test` in review checklist - Note that `## Inputs` may need to be created in agent definition files Addresses review feedback on #566
LOCAL.md covers setting up the sandbox infrastructure (podman socket, openshell gateway), running agents with fullsend run, and testing new configuration options. FEATURES.md gains a manual testing checkbox and an optional functional eval section. CONTRIBUTING.md links to LOCAL.md. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
- Clarify that runner_env: is deprecated; env: runner: / env: sandbox:
is canonical
- Fix step 4 to reference ## Variables instead of ## Inputs
- Warn that harness YAML uses Go os.Expand which does not support shell
default syntax like ${VAR:-default} — recommend setting defaults in
pre/post scripts instead
- Remove incorrect review.yaml reference as top-level runner_env: example
- Update forge.github guidance to prefer forge.github.env.runner
Signed-off-by: Ralph Bean <rbean@redhat.com>
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
- FEATURES.md step 4: follow ADR 0049 — no required section structure for env var references in agent prompts - FEATURES.md step 5: replace deprecated docs link with normative contract doc URL - LOCAL.md: replace Linux-only systemd instructions with link to upstream guide covering both platforms - LOCAL.md: add required GCP/Vertex AI credentials to env var setup Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Remove all runner_env mentions from the configuration checklist — only document the canonical env.runner/env.sandbox pattern. Add guidance that default values belong in the base harness YAML, where users override them in base-derived harness files. Fix "overlay" terminology. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
Per team discussion, config.yaml options should be global (no
agent-specific prefix, no env var override) and require a schema
update in fullsend-ai/fullsend plus a follow-up campaign for agents to
adopt them. Env var options are agent-specific, use the {AGENT}_
prefix per ADR 0049, and can be delivered entirely within this repo.
Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
- FEATURES.md: point the skill-override row at harness `skills:` with paths/URLs (ADR 0038/0045), since `.fullsend/customized/skills/` is deprecated by ADR 0064. - FEATURES.md: ADR 0049 doesn't say anything about config.yaml exclusivity — reword that as project convention instead. - LOCAL.md: `--target-repo .` pointed at the harness repo itself instead of the target repo from the example issue URL. Add a step cloning the target repo to /tmp/target-repo and use that path. - LOCAL.md: CLOUD_ML_REGION example used us-east5; switch to the documented default (global). Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
- LOCAL.md: fix invalid SSH URL in git clone example (missing .com) - FEATURES.md: soften "mutually exclusive" env-var/config.yaml claim, which had no traceable source, to a recommendation - docs/triage.md: note that .fullsend/customized/skills/ overlay is deprecated by ADR 0064, matching the harness skills: guidance we already added to FEATURES.md's skill-override row Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
- Skill override rows/paragraphs (FEATURES.md, docs/triage.md) now describe the actual `base:` composition + basename-dedup mechanism (ADR 0045) needed to replace a skill, instead of implying a bare `skills:` entry does it. - Fix path confusion between the org-level `.fullsend` config repo overlay (`customized/skills/...`) and the per-repo overlay (`.fullsend/customized/skills/...`). - FEATURES.md Section 5 now calls out that post-code.sh, post-fix.sh, and post-prioritize.sh are generated from .src.sh sources and must be rebuilt with `make script-build` / verified with `make check-bundle`. Addresses review feedback from waynesun09. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
- Pin --output-dir /tmp/fullsend in LOCAL.md examples so the cat glob in step 4 works on macOS, where os.TempDir() differs from Linux. - Note the pinned openshell version in Prerequisites to avoid an opaque sandbox pre-flight failure on older Homebrew builds. - Cite ADR 0080/0081 in the env var guidance in FEATURES.md. - Fix issue citation from fullsend-ai/fullsend#5408 (bug report) to #5409 (the merged fix) in FEATURES.md and docs/triage.md. Assisted-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
7452b23 to
579fb94
Compare
|
🤖 Review · |
- LOCAL.md: drop the hardcoded per-agent "additional variables" list (it drifted out of sync with harness/code.yaml); point readers at the target agent's harness/<agent>.yaml instead. - FEATURES.md: swap the env var Example from CODE_ALLOWED_TARGET_BRANCHES (a known non-conformant case per ADR 0081) to TRIAGE_AUTO_CODE. - FEATURES.md: add scripts/pre-code.sh to the list of generated scripts, matching the Makefile's BUNDLE_SRCS. Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Ralph Bean <rbean@redhat.com>
|
🤖 Finished Review · ✅ Success · Started 9:16 PM UTC · Completed 9:31 PM UTC |
waynesun09
left a comment
waynesun09
left a comment
There was a problem hiding this comment.
Approving — both flagged issues confirmed fixed: LOCAL.md now points readers at the target agent's harness/.yaml instead of a hardcoded (and drift-prone) variable list, and FEATURES.md's Example column now cites a fully-conformant variable instead of the ADR-0081-flagged non-conformant one. PR diff is doc-only (CONTRIBUTING.md, FEATURES.md, LOCAL.md, docs/triage.md) — the large commit-range history is just upstream commits picked up in a rebase, not part of this PR's actual change. All 37 review threads resolved. Not gating on the CI run in progress.
|
🤖 Finished Retro · ✅ Success · Started 2:53 PM UTC · Completed 3:07 PM UTC Commit: |
Retro: PR #566 — docs: add configuration checklist for agent feature workTimelinePR #566 was a human-authored documentation PR by ralphbean that added The PR went through 12 commits over 6 days (Jul 30 – Aug 5, 2026) with:
AnalysisReview quality gap. The review agent found ~7 surface-level issues (env key naming mismatches, nonexistent
The review agent issued 8+ APPROVED verdicts across the PR lifecycle while the human was still actively finding high-severity issues. The agent's approval tracked surface-level consistency, not the accuracy of claims about system behavior. Agent confabulation. In one review cycle, the agent fabricated a claim that Fix agent post-script failure. Run 30555954743 successfully fixed 3 review findings and pushed the branch, but failed when Token cost. 14 review runs on a 4-file documentation PR represents significant compute. The agent also re-raised already-resolved issues across review cycles (notably Existing issue evidenceAll identified improvement areas map to existing open issues. New evidence from this PR:
Autonomy assessmentThe review agent is not ready for approval autonomy on documentation PRs about system internals. The agent's value was in fast surface-level checks (naming, links, headings), but it systematically missed every issue requiring cross-repo verification, ADR compliance checking, or runtime behavior understanding. The ConclusionNo new proposals are filed — all findings map to existing tracked issues. The PR's review history provides strong supporting evidence for the issues noted above, particularly agents#420 (confabulation) and agents#638 (cross-repo verification gap). |
2 participants
Summary
FEATURES.md— a step-by-step checklist for adding configurable options to agents. Covers env var placement (sandbox vs runner), agent prompt updates, pre/post script changes, schema updates, skill impact, documentation, and tests.CONTRIBUTING.md.docs/triage.md's skill-override section to note the.fullsend/customized/skills/overlay is deprecated by ADR 0064, so it matches the harnessskills:guidance inFEATURES.md.Test plan
FEATURES.mdrenders correctly on GitHubCONTRIBUTING.mdlink resolves to the new file🤖 Generated with Claude Code