feat(#5858): add Policy field to ForgeConfig

[fullsend-ai-coder]

Summary

Add a policy field to ForgeConfig so harness files can select different sandbox policies per forge platform. This enables least-privilege network policies — e.g. the triage agent can use a GitHub-scoped policy by default and a GitLab-scoped policy when running against GitLab.

Changes

• Add Policy string to ForgeConfig struct with yaml:"policy,omitempty" tag
• mergeForgeConfig: scalar override — forge policy replaces top-level if non-empty (same pattern as PreScript/PostScript)
• validateForge: URL policies require #sha256=... integrity hash (mirrors ValidateResourceTypes for declarative fields)
• mergeForgeConfigInto (compose.go): base composition inherits forge-level policy when child is empty
• resolveBaseScripts (compose.go): resolve forge-level policy paths against base URL during base composition
• ADR-0045: updated ForgeConfig struct listing to include policy

Testing

• TestResolveForge_PolicyOverride — forge policy replaces top-level
• TestResolveForge_PolicyNotOverriddenWhenEmpty — top-level preserved when forge policy absent
• TestValidate_ForgePolicyURLWithoutHash — URL without integrity hash rejected
• TestValidate_ForgePolicyURLWithHash — URL with integrity hash accepted
• TestValidate_ForgePolicyLocalPath — local path accepted
• TestForgeConfig_PolicyParsesFromYAML — YAML round-trip parsing
• go test -race ./internal/harness/... passes
• go vet ./internal/harness/... passes
• make lint — pre-commit could not run in sandbox (network restriction); post-script runs authoritatively

---

Closes #5858

Post-script verification

• Branch is not main/master (agent/5858-forge-policy-field)
• Secret scan passed (gitleaks — 7a6f5043b510f43851d3414bc98f63080d1970f1..HEAD)
• PR body secret scan passed (gitleaks — no-git)

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 2:37 PM UTC · Completed 2:50 PM UTC
Commit: db06a0d · View workflow run →

[github-actions]

Site preview

Preview: https://fa20dbf8-site.fullsend-ai.workers.dev

Commit: af5dd12ffecc3ee7cb62af6b53bbbcf83465d5be

[codecov]

Codecov Report

❌ Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/harness/compose.go	81.81%	1 Missing and 1 partial ⚠️
internal/cli/lock.go	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Medium

• [consumer-completeness] internal/cli/migrate.go:433 — The rewriteCustomizedPaths function strips the customized/ prefix from forge config path fields (PreScript, PostScript, Skills, ValidationLoop.Script, ValidationLoop.Schema) but does not strip the new fc.Policy field. The top-level h.Policy IS stripped (line 403), confirming policy paths can carry this prefix. A forge config with a customized/-prefixed policy path will produce an incorrect path after migration.
  Remediation: Add fc.Policy = strip(fc.Policy) inside the forge config iteration loop (line 428-446), after fc.PostScript = strip(fc.PostScript) at line 433.

• [adr-immutability] docs/ADRs/0045-forge-portable-harness-schema.md:252 — ADR-0045 has status Accepted on main. This PR moves policy from the "Fields that stay at top level only" table to the "Fields that can appear at both levels" table and adds the Policy field to the ForgeConfig struct definition in the Decision section. Per the ADR contributing guide (docs/contributing/adrs.md), Accepted ADRs are point-in-time records: "Do not substantially rewrite its Context, Decision, or Consequences sections." Moving a field's classification between the two architectural tables is a substantive change to the decision.
  Remediation: Write a short amendment ADR (e.g., "ADR-00XX: Extend ForgeConfig with policy field") that references ADR-0045 and records the rationale for making policy forge-overridable. In ADR-0045, add only a cross-reference annotation pointing to the new ADR.

---

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

Medium

• [consumer-completeness] internal/cli/migrate.go:433 — The rewriteCustomizedPaths function strips the customized/ prefix from forge config path fields (PreScript, PostScript, Skills, ValidationLoop.Script, ValidationLoop.Schema) but does not strip the new fc.Policy field. The top-level h.Policy IS stripped (line 403), confirming policy paths can carry this prefix. A forge config with a customized/-prefixed policy path will produce an incorrect path after migration.
  Remediation: Add fc.Policy = strip(fc.Policy) inside the forge config iteration loop, after fc.PostScript = strip(fc.PostScript).

• [adr-immutability] docs/ADRs/0045-forge-portable-harness-schema.md:252 — ADR-0045 has status Accepted on main. This PR moves policy from the "Fields that stay at top level only" table to the "Fields that can appear at both levels" table and changes the ForgeConfig struct definition in the Decision section. Per the ADR contributing guide (docs/contributing/adrs.md), Accepted ADRs are point-in-time records: "Do not substantially rewrite its Context, Decision, or Consequences sections." Moving a field's classification between the two architectural tables is a substantive change to the decision.
  Remediation: Write a short amendment ADR (e.g., "Extend ForgeConfig with policy field") that references ADR-0045 and records the rationale for making policy forge-overridable. In ADR-0045, add only a cross-reference annotation pointing to the new ADR.

Low

• [scope-vs-authorization] docs/ADRs/0045-forge-portable-harness-schema.md:252 — Issue Add Policy to ForgeConfig for per-forge policy selection #5858 justifies per-forge policy with "different forges need different network permissions." ADR-0065 designates providers as the sole mechanism for network access. The updated rationale in this PR correctly acknowledges ADR-0065 and pivots to non-network policy sections, resolving the prior review's concern. However, the issue description still frames the need in terms of network permissions, creating a mismatch in the authorization trail. See also: [adr-immutability] finding at this location.
  Remediation: Update issue Add Policy to ForgeConfig for per-forge policy selection #5858's description to replace the network-permission rationale with the actual use case (per-forge filesystem or process restrictions).

---

Labels: PR modifies documentation files under docs/ADRs/, docs/guides/, and docs/plans/ alongside harness Go code

---

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

Medium

• [consumer-completeness] internal/cli/migrate.go:432 — The rewriteCustomizedPaths function strips the customized/ prefix from forge config path fields (PreScript, PostScript, Skills, ValidationLoop.Script, ValidationLoop.Schema) but does not strip the new fc.Policy field. The top-level h.Policy is stripped (line 403), confirming policy paths can carry this prefix. A forge config with a customized/-prefixed policy path will produce an incorrect path after migration.
  Remediation: Add fc.Policy = strip(fc.Policy) inside the forge config iteration loop (around line 432).

• [consumer-completeness] internal/cli/lock.go:894 — The resolveFromLock dispatch switch handles forge-level fields forge.*.pre_script, forge.*.post_script, forge.*.validation_loop.script, and forge.*.validation_loop.schema as informational no-ops, but has no case for forge.*.policy. When resolveBaseScripts records a forge policy dependency with field name forge.<platform>.policy, resolveFromLock falls through to the default case, which fails to parse it as skills[N] and appends the cache path to h.Skills. This silently pollutes the skills list with a policy file path.
  Remediation: Add a no-op case alongside the other forge field handlers.

• [architectural-coherence] docs/ADRs/0045-forge-portable-harness-schema.md:252 — ADR-0045 originally classified policy as forge-agnostic and placed it in the "Fields that stay at top level only" section. This PR moves policy to forge-overridable with rationale "Network policies differ per forge." ADR-0065 establishes that network access should be managed through providers, not policy files, with policies containing only filesystem access, landlock, and process identity. The rationale for making policy forge-specific should be reconciled with ADR-0065's separation of concerns.
  Remediation: Update ADR-0045's rationale for policy to clarify whether forge-specific policies are needed for non-network sandbox restrictions or acknowledge the tension with ADR-0065.

Low

• [authorization] internal/harness/forge.go:128 — The forge-level Policy field allows selecting a different sandbox policy per forge platform with no validation that it is restricted to a subset of the top-level policy. This matches existing behavior and is by-design per the merge rules.

• [stale-field-list] docs/plans/adr-0045-forge-portable-harness-phase1.md:48 — The implementation plan lists the ForgeConfig struct fields without Policy. The field enumeration is now incomplete.

---

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

Review

Findings

Medium

• [consumer-completeness] internal/cli/migrate.go:432 — The rewriteCustomizedPaths function strips the customized/ prefix from forge config path fields (PreScript, PostScript, Skills, ValidationLoop.Script, ValidationLoop.Schema) but does not strip the new fc.Policy field. The top-level h.Policy is stripped (line 403), confirming policy paths can carry this prefix. A forge config with a customized/-prefixed policy path will produce an incorrect path after migration.
  Remediation: Add fc.Policy = strip(fc.Policy) inside the forge config iteration loop (before fc.PreScript = strip(fc.PreScript) at line 432).

Low

• [function-field-ordering] internal/harness/forge.go:120 — In mergeForgeConfig, Policy is merged before PreScript, which does not match the struct field declaration order (PreScript, PostScript, Policy, Skills, ...). See also: [resource-processing-ordering] finding at compose.go:698.
• [resource-processing-ordering] internal/harness/compose.go:698 — In resolveBaseScripts, the Policy field processing is placed before the forgeScripts loop (which processes PreScript and PostScript), not matching the struct declaration order. See also: [function-field-ordering] finding at forge.go:120.
• [incomplete-doc] docs/guides/user/bring-your-own-agent.md:299 — The "Field merge rules" table does not explicitly mention policy in the Scalars row. The table uses "etc." to cover unlisted scalars, so policy is implicitly covered, but being explicit would help users since policy was previously documented as a top-level-only field.
• [authorization] internal/harness/forge.go:46 — The forge-level Policy field allows selecting a different sandbox policy per forge platform with no validation that it is restricted to a subset of the top-level policy. This matches existing behavior and is by-design per the merge rules.

---

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

Review

Findings

High

• [architecture-contradiction] docs/ADRs/0045-forge-portable-harness-schema.md:260 — ADR 0045's "Fields that stay at top level only" table (line 260) classifies policy as platform-neutral with rationale "Sandbox policies describe capabilities, not forges." The PR adds Policy to ForgeConfig and updates the struct listing in the same document, but does not update the classification table. The document now contradicts itself: one section says policy is excluded from forge blocks, while the code example shows it included.
  Remediation: Move policy from the "Fields that stay at top level only" table to the "Fields that can appear at both levels" table with revised rationale explaining why per-forge policy selection is now needed.

Medium

• [consumer-completeness] internal/cli/migrate.go:432 — The rewriteCustomizedPaths function strips the customized/ prefix from forge config path fields (PreScript, PostScript, Skills, ValidationLoop.Script, ValidationLoop.Schema) but does not strip the new fc.Policy field. The top-level h.Policy is stripped (line 403), confirming policy paths can carry this prefix. A forge config with a customized/-prefixed policy path will produce an incorrect path after migration.
  Remediation: Add fc.Policy = strip(fc.Policy) before fc.PreScript = strip(fc.PreScript) inside the forge config iteration loop.

Low

• [struct-field-ordering] internal/harness/forge.go:15 — Policy is placed first in ForgeConfig, before the script fields (PreScript, PostScript). The established pattern groups script fields together, then resource lists, then config objects, then maps. Consider placing Policy after PostScript and before Skills for consistency.

---

Labels: PR modifies internal/harness/ Go code for forge config schema

---

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

[ggallen]

Re: [medium] consumer-completeness (internal/cli/migrate.go:432) — Not fixing. The rewriteCustomizedPaths function is being removed entirely in #5697 / #5836. Adding fc.Policy to a function that is about to be deleted would be churn.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:02 PM UTC · Completed 3:18 PM UTC
Commit: 6394880 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:30 PM UTC · Completed 3:47 PM UTC
Commit: 4f44453 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:58 PM UTC · Completed 4:14 PM UTC
Commit: 352f229 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:23 PM UTC · Completed 4:38 PM UTC
Commit: af5dd12 · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 5:03 PM UTC · Completed 5:20 PM UTC
Commit: af5dd12 · View workflow run →

[fullsend-ai-retro]

Retro: PR #5859 — Add Policy field to ForgeConfig

Overall: The workflow delivered a correct result — the code agent produced an 8-file, test-covered PR in ~11 minutes, and the review agent caught a genuine bug (missing resolveFromLock switch case in lock.go that would have polluted the skills list). The PR merged in ~2.5 hours. However, two known bugs compounded to force 5 manual force-push iterations and significant human toil.

Timeline

Time (UTC)	Event
14:16	Triage agent runs on issue #5858 (4 min)
14:25	/fs-code dispatched; code agent creates PR in ~11 min
14:36	PR #5859 opened by fullsend-ai-coder[bot]
14:50	Review round 1: HIGH architecture-contradiction in ADR-0045, MEDIUM consumer-completeness in migrate.go
14:51	Fix agent run #1 fails (eligibility check, <10s)
14:58	ggallen dismisses migrate.go finding (function being removed in #5697/#5836)
15:01–16:22	4 force-pushes by ggallen addressing findings; review rounds 2–4 each re-raise migrate.go
15:48–16:38	Fix agent runs #2–5 all fail identically
16:13	Review round 4 introduces adr-immutability finding
16:15	ggallen dismisses adr-immutability with explanation
16:38	Review round 5 re-raises both migrate.go AND adr-immutability despite prior dismissals
16:48	ggallen manually approves
17:01	PR merged

Key observations

Review quality was good. The review agent found real issues: the ADR-0045 self-contradiction (round 1), the missing resolveFromLock case in lock.go (round 3, a genuine bug), and the incomplete doc update (round 2). It also found legitimate design concerns (authorization model, ADR coherence) that the human evaluated and made reasoned decisions about.

Two known bugs compounded badly. (1) The fix agent's bot-detection regex ([bot]$) doesn't match app/fullsend-ai-coder returned by gh pr view, so all 5 fix runs failed in <15 seconds (#5536). (2) The review agent re-raised dismissed findings because it only receives its own prior output, not human replies (#5265). Together, this forced the human to both manually fix everything AND repeatedly dismiss the same findings.

Evidence for existing issues:

• Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 (fix agent eligibility): 5 consecutive failed fix runs on this PR, all with identical error Human-authored PR #5859 without 'fullsend-fix' label. This bug has been open since 2026-05-27 (Fix agent eligibility gate misclassifies bot-authored PRs as human-authored #1569) and affects every bot-authored PR.
• Review re-review anchoring should incorporate author response context to suppress explained-away findings #5265 (review anchoring with author responses): consumer-completeness finding on migrate.go repeated in all 5 rounds despite explicit dismissal with explanation after round 1. adr-immutability repeated in round 5 despite dismissal 23 minutes earlier.
• Post PR comment when fix agent guard-rail blocks execution #5863 (no user feedback on fix agent block): ggallen had no visibility that fix runs were failing — no comment posted on the PR.
• Review agent should deduplicate findings across iterations on the same PR #1013 / Review agent should deduplicate findings across re-review iterations on the same PR #2959 / Deduplicate inline review comments across force-push re-reviews #5007 (review dedup): additional evidence of the same pattern.

No new proposals. All improvement opportunities identified in this retro are already tracked as open issues. The highest-impact fix would be #5536 (fix agent eligibility), which has been open for over 2 months and is systematically blocking the fix agent on every bot-authored PR.