chore(#904): deduplicate harness config across forge sections

[fullsend-ai-coder]

Summary

Applies the shared-config-at-top-level pattern (established in PR #817 for retro.yaml) to code.yaml, triage.yaml, and review.yaml. Removes redundant duplication of scalars and env vars that are identical across all forge sections, hoisting them to the top-level config where mergeForgeConfig will use them as defaults.

Changes by file

harness/code.yaml

• Removed duplicate pre_script/post_script from forge.github and forge.gitlab (already set at top level)
• Hoisted PUSH_TOKEN, PUSH_TOKEN_SOURCE, REPO_FULL_NAME, ISSUE_NUMBER to top-level env.runner
• Hoisted GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL to top-level env.sandbox

harness/triage.yaml

• Removed duplicate pre_script/post_script from forge.github, forge.gitlab, and forge.jira (already set at top level)

harness/review.yaml

• Hoisted pre_script/post_script to top level (identical in both forge sections, previously missing from top level)
• Hoisted REPO_FULL_NAME to top-level env.runner
• Hoisted REPO_FULL_NAME, PRIOR_REVIEW_SHA, PRIOR_REVIEW_PROVENANCE to top-level env.sandbox

Why this is safe

mergeForgeConfig in internal/harness/forge.go merges forge sections into the top-level harness config:

• Scalars (pre_script, post_script, policy): forge overrides only when non-empty
• Env maps: top-level merged with forge; forge keys win on collision

All removed values were identical to their top-level counterparts, so the resolved config is unchanged for every forge platform.

Testing

• YAML syntax validation passed for all three files
• make test passes (16 pre-existing failures in post-retro-test.sh unrelated to this change — same count on main)
• make check-bundle passed
• Secret scan passed

---

Closes #904

Post-script verification

• Branch is not main/master (agent/904-deduplicate-harness-config)
• Secret scan passed (gitleaks — 2c3e23f474b9dcac3e8837d32fba3bf08cec0236..HEAD)
• PR body secret scan passed (gitleaks — no-git)

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:38 PM UTC · Completed 9:51 PM UTC

Commit: 326d761 · View workflow run →

[ggallen]

/ok-to-test

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] AGENTS.md, harness/code.yaml, harness/review.yaml, harness/triage.yaml — This PR modifies files under protected paths (AGENTS.md, harness/). The linked issue (chore: deduplicate harness config across forge sections #904) and PR description explain the rationale (deduplication following the pattern established in PR feat(#816): make retro agent multi-forge (GitHub + GitLab) #817). Human approval is always required for protected-path changes, regardless of context.

Low

• [secret-scope-expansion] harness/code.yaml:58 — PUSH_TOKEN, PUSH_TOKEN_SOURCE, REPO_FULL_NAME, and ISSUE_NUMBER were previously scoped to forge.github.env.runner and forge.gitlab.env.runner. They are now at top-level env.runner, making them available to all forge configurations including any future forge. Sandbox isolation is preserved (token stays in runner:, not sandbox:). No immediate remediation required — verify future forge additions are trusted with PUSH_TOKEN access.

Previous run

Review

Findings

Medium

• [protected-path] AGENTS.md, harness/code.yaml, harness/review.yaml, harness/triage.yaml — This PR modifies files under protected paths (AGENTS.md, harness/). The PR links to issue chore: deduplicate harness config across forge sections #904 and provides a clear rationale for the changes (deduplicating config shared across forge sections per the established pattern from PR feat(#816): make retro agent multi-forge (GitHub + GitLab) #817). Human approval is required for protected-path changes regardless of context.

---

Labels: PR modifies harness configuration for code, review, and triage agents.

Previous run (2)

Review

Findings

Medium

• [protected-path] AGENTS.md, harness/code.yaml, harness/review.yaml, harness/triage.yaml — PR modifies files under protected paths (AGENTS.md, harness/). The linked issue chore: deduplicate harness config across forge sections #904 and PR description provide rationale for the changes (applying the shared-config-at-top-level pattern from PR feat(#816): make retro agent multi-forge (GitHub + GitLab) #817). Human approval is always required for protected-path changes regardless of justification.

Low

• [code-organization] harness/code.yaml:58 — The comment above the env: block states variables are "available to pre/post scripts on the runner" and "NEVER enter the sandbox." With GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, GIT_COMMITTER_NAME, GIT_COMMITTER_EMAIL now added to top-level env.sandbox, this claim is factually inaccurate.
  Remediation: Update the comment to acknowledge both runner and sandbox entries, e.g. "Environment variables for runner pre/post scripts and sandbox sessions."

Info

• [provenance-warning] Prior review context discarded: provenance validation failed (unverifiable-wrong-app). This review treats all findings as first-time assessments.

Previous run (3)

Review

Findings

Medium

• [protected-path] harness/code.yaml, harness/review.yaml, harness/triage.yaml — All three changed files are under harness/, which is a protected path. The PR links to issue chore: deduplicate harness config across forge sections #904 and provides sufficient justification (deduplication of harness config following the established pattern from PR feat(#816): make retro agent multi-forge (GitHub + GitLab) #817). Human approval is always required for protected-path changes, regardless of context.

• [stale-documentation] docs/triage.md:236 — The migration notes state that pre_script and post_script are "set at both levels (identical values — the forge-level entries are redundant but kept explicit for clarity)." After this PR, the forge-level entries are removed from harness/triage.yaml, so they are no longer set at both levels. The parenthetical claim is now false.
  Remediation: Update the sentence to reflect the new structure, e.g., "pre_script and post_script are set at the top level only; forge sections inherit them via ResolveForge."

Low

• [stale-documentation] AGENTS.md:160 — Section 8 lists REPO_FULL_NAME alongside REVIEW_TOKEN, PR_NUMBER, and PR_URL as examples of computed passthrough values in forge.<platform>.env.runner blocks. After this PR, REPO_FULL_NAME is moved to top-level env.runner in both review.yaml and code.yaml, making the example slightly inaccurate about location — though the principle (it must remain a ${VAR} passthrough) is unchanged.
  Remediation: Remove REPO_FULL_NAME from the forge-level example list, or note that some passthroughs live at top-level when identical across forges.

• [stale-documentation] docs/code.md:173 — The "Multi-forge support" section states forge sections configure "platform-specific policies, skills, env vars, and scripts." After this PR, pre_script/post_script are removed from forge sections in code.yaml, making "scripts" inaccurate. Forge sections still contain platform-specific env vars, policies, and skills.
  Remediation: Remove "scripts" from the list or soften to say forge sections configure platform-specific values that differ between forges.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:28 AM UTC · Completed 12:48 AM UTC

Commit: 20e72f0 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:54 AM UTC · Completed 1:14 AM UTC

Commit: b707cac · View workflow run →

[ggallen]

/ok-to-test

[ggallen]

/ok-to-test

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:25 AM UTC · Completed 3:40 AM UTC

Commit: 8e1b9f5 · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 11:01 AM UTC · Completed 11:13 AM UTC

Commit: 8e1b9f5 · View workflow run →

[fullsend-ai-retro]

Retro: PR #908 — deduplicate harness config across forge sections

Workflow: Issue #904 (filed by ggallen) → triage agent → code agent (run 32419257028) → PR #908 → 4 review agent runs → 3 human force-pushes → human approval → merged.

What went well:

• The code agent correctly performed all YAML deduplication across code.yaml, triage.yaml, and review.yaml in ~12 minutes at $2.90. The harness config changes were correct on the first attempt.
• The triage agent verified all duplication claims against the codebase and confirmed no merge-conflict risk with PR feat(#816): make retro agent multi-forge (GitHub + GitLab) #817.
• The review agent caught 3 stale-documentation issues across separate files (AGENTS.md, docs/code.md, docs/triage.md) in its first run — strong cross-file reasoning. It also caught a self-contradicting comment in code.yaml on its second run and a secret-scope-expansion concern on its fourth run.

What required rework:

• The code agent missed 4 collateral updates that its structural changes made necessary: 3 documentation files became factually inaccurate, and 1 inline comment in code.yaml directly contradicted the new code (stating env vars "NEVER enter the sandbox" while the agent had just added env.sandbox entries). The human (ggallen) needed 3 force-pushes over ~3 hours to fix these, each triggering another review cycle.
• Root cause: the code-implementation skill has no step that checks whether structural changes (moves, renames, config restructuring) make existing documentation or comments stale. The "surgical changes" rule in AGENTS.md says "Do not improve comments on lines you did not change," and the dead-code clause only covers code, not docs. The agent followed its guidance correctly — the guidance has a gap.

Observations on review agent:

• The [secret-scope-expansion] finding (PUSH_TOKEN et al. moving to top-level env.runner) was present in the diff from Run 1 but only surfaced in Run 4. This suggests non-deterministic coverage across review runs — not necessarily a bug, but worth noting.
• Run 3 (on b707cac) was the most redundant: its only finding was the standing [protected-path] policy gate, already surfaced in all prior runs. Issue #108 (short-circuit review on rebase-only pushes) is open and partially addresses this, though PR chore(#904): deduplicate harness config across forge sections #908 was not a rebase-only scenario.

Existing issue evidence: This retro provides additional evidence for #108 — Run 3 on PR #908 consumed a full review cycle to re-surface only the standing protected-path gate, with no new actionable findings.

Proposals filed

• Code-implementation skill should verify documentation and comment consistency after structural changes (in fullsend-ai/agents)