docs(#5751): document compose/diff bidirectional invariant

[fullsend-ai-coder]

Summary

Documents the bidirectional invariant between merge/compose functions and path-rewriting/diff functions in the harness package. This constraint was undiscoverable by agents, causing PR #5450 to fail through 6 fix iterations when field-level merge changes in compose.go were not mirrored in the migration path-rewriting code.

Related Issue

Closes #5751

Changes

• Add docs/contributing/harness-composition.md with:
  • Explanation of the invariant and why it matters (PR fix(#5448): use field-level merge for validation_loop in base composition #5450 case study)
  • Table of paired functions on the merge side (compose.go, forge.go) and path-rewriting side (migrate.go)
  • Concrete examples of how changes propagate between the two sides
  • Checklist for harness field additions covering all affected functions and test files
  • Cross-references to ADR-0045, ADR-0064, and related issue Document harness field integration pipeline as a contributor checklist #5579
• Add row to AGENTS.md guidance table linking to the new doc with trigger: "Changing merge, diff, or path-rewriting functions in internal/harness/ or internal/cli/migrate.go"

Testing

• Documentation-only change — no Go code modified
• Secret scan passed
• Gitlint passed
• make lint deferred to post-script (pre-commit sandbox network issue)

Checklist

• PR title follows Conventional Commits (correct type, ! for breaking changes)
• No secrets or sensitive data in changes

---

Closes #5751

Post-script verification

• Branch is not main/master (agent/5751-compose-diff-invariant-docs)
• Secret scan passed (gitleaks — c9df5fedf91d54a0d0debffed98598908959e4c0..HEAD)
• PR body secret scan passed (gitleaks — no-git)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:40 AM UTC · Completed 8:52 AM UTC
Commit: 24c881e · View workflow run →

[github-actions]

Site preview

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

Commit: ed42dd2b8ed96509e8502ce82b1340fa9f237d75

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] AGENTS.md — This PR modifies AGENTS.md, a governance file on the protected paths list. The change adds a guidance table row linking to the new harness composition doc, authorized by issue Document compose/diff bidirectional invariant in contributing docs #5751. Human approval is always required for protected-path changes, regardless of context.

Low

• [section-structure] docs/contributing/harness-composition.md:108 — The doc uses ## When reviewing PRs as an H2 section heading, but all six other contributing docs (forge-abstraction.md, shell-scripting.md, workflow-contracts.md, cel-triggers.md, adrs.md, sandbox-topology.md) embed reviewer guidance as a bold inline paragraph (**When reviewing PRs:**), not a standalone H2 heading.

• [section-structure] docs/contributing/harness-composition.md:115 — The doc includes a ## Related section with ADR and issue cross-references, but no other contributing docs use this pattern. Two of the three references (ADR-0045 and ADR-0064) are already cited inline earlier in the document.

Previous run

Review

Findings

Medium

• [protected-path] AGENTS.md — This PR modifies AGENTS.md, a governance file on the protected paths list. The change adds a guidance table row linking to the new harness composition doc, authorized by issue Document compose/diff bidirectional invariant in contributing docs #5751. Human approval is always required for protected-path changes, regardless of context.

• [consumer completeness] docs/contributing/harness-composition.md — Issue Document compose/diff bidirectional invariant in contributing docs #5751 requests documentation of the invariant between merge/compose functions and diff functions (DiffHarness, diffForgeConfig) in internal/harness/diff.go. The document instead covers path-rewriting functions in internal/cli/migrate.go. This pivot is correct — internal/harness/diff.go does not exist (removed per ADR-0045's scaffold agent extraction). However, the document does not explain why diff functions are absent, which may confuse readers arriving from issue Document compose/diff bidirectional invariant in contributing docs #5751. Consider adding a brief note in the "How they correspond" section explaining that DiffHarness was removed and rewriteCustomizedPaths is the remaining counterpart.

Low

• [documentation-completeness] docs/contributing/harness-composition.md — Some contributing docs include a "When reviewing PRs" section with severity guidance for reviewers (e.g., shell-scripting.md, forge-abstraction.md). This doc omits such a section. The pattern is present in roughly half the contributing docs so it is not a mandatory convention, but adding reviewer guidance would help agents consistently flag invariant violations.

• [terminology-consistency] docs/contributing/harness-composition.md — The PR title and issue Document compose/diff bidirectional invariant in contributing docs #5751 reference "compose/diff bidirectional invariant" but the document describes the compose/migration (path-rewriting) invariant. Adding the bridging note from the consumer completeness finding above would also resolve this mismatch.

Previous run (2)

Review

Findings

Medium

• [protected-path] AGENTS.md — This PR modifies AGENTS.md, a governance file on the protected paths list. The change is authorized by issue Document compose/diff bidirectional invariant in contributing docs #5751 and the PR description explains the rationale (adding a guidance table row for the new harness composition doc). Human approval is always required for protected-path changes, regardless of context.

Low

• [internal consistency] docs/contributing/harness-composition.md:28 — The invariant statement says changes must be mirrored in "the corresponding path-rewriting and diff functions," and the AGENTS.md entry says "merge, diff, or path-rewriting functions." However, the paired functions tables list only merge functions and path-rewriting functions — no diff functions exist in the codebase (internal/harness/ has no Diff* or diff* functions, and internal/cli/migrate.go contains only rewrite functions). The document is internally inconsistent: it names "diff" as part of the invariant but correctly omits it from the concrete function tables. Consider removing "diff" from the invariant description and the AGENTS.md trigger, or adding a "Diff side" table if diff functions exist elsewhere.

---

Labels: PR adds documentation about harness composition invariant

[rh-hemartin]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 11:58 AM UTC · Completed 12:04 PM UTC
Commit: 24c881e · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 2 of 2 actionable review findings by removing spurious 'diff' references from the harness composition doc (lines 3 and 28) and the AGENTS.md trigger column. The protected-path finding is informational and handled by existing enforcement.

Fixed (2):

1. internal consistency: spurious 'diff' references in invariant description (docs/contributing/harness-composition.md): Removed 'diff' from the opening paragraph (line 3) and invariant statement (line 28). No diff functions exist in internal/harness/ or internal/cli/migrate.go — the paired function tables correctly omitted them, so the prose now matches the concrete function inventory.
2. internal consistency: spurious 'diff' in AGENTS.md trigger (AGENTS.md): Removed 'diff' from the 'When to read' column for the Harness Composition row, changing 'merge, diff, or path-rewriting functions' to 'merge or path-rewriting functions' to match the actual function inventory.

Disagreed (1):

1. protected-path: AGENTS.md modification requires human approval: This is an informational finding, not a code issue. The AGENTS.md change is part of the original PR scope (authorized by issue Document compose/diff bidirectional invariant in contributing docs #5751) and is also required by the internal consistency fix. Human approval is handled by the protected-path enforcement in post-review.sh.

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 12:05 PM UTC · Completed 12:21 PM UTC
Commit: 6402861 · View workflow run →

[rh-hemartin]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 1:39 PM UTC · Completed 1:45 PM UTC
Commit: 6402861 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 2 (human-triggered)

Addressed 3 of 4 review findings. Added a diff-function provenance note explaining why DiffHarness/diffForgeConfig are absent (resolving both the consumer-completeness and terminology-consistency findings), and added a 'When reviewing PRs' section with reviewer severity guidance. Disagreed with 1 finding (protected-path is informational enforcement, not fixable).

Fixed (3):

1. consumer completeness: missing explanation of why diff functions are absent (docs/contributing/harness-composition.md): Added a blockquote note in the 'How they correspond' section explaining that DiffHarness and diffForgeConfig were removed when ADR-0045 extracted the scaffold agent, and that the remaining counterpart is the path-rewriting code in migrate.go. This bridges the gap between issue Document compose/diff bidirectional invariant in contributing docs #5751's 'compose/diff' framing and the actual compose/migration invariant.
2. documentation-completeness: missing 'When reviewing PRs' section (docs/contributing/harness-composition.md): Added a 'When reviewing PRs' section with reviewer guidance: flag merge-function changes without corresponding rewrite-function updates (and vice versa) as medium-severity findings, matching the pattern in forge-abstraction.md, shell-scripting.md, and cel-triggers.md.
3. terminology-consistency: PR title says 'compose/diff' but doc covers compose/migration (docs/contributing/harness-composition.md): Resolved by the diff-function provenance note added for the consumer completeness finding. The note explicitly explains the terminology shift from 'diff' to 'migration/path-rewriting', bridging the mismatch.

Disagreed (1):

1. protected-path: AGENTS.md modification requires human approval: This is an informational enforcement finding, not a code issue. The AGENTS.md change was part of the original PR scope (authorized by issue Document compose/diff bidirectional invariant in contributing docs #5751) and is handled by post-review.sh's protected-path enforcement. No code fix is applicable.

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 1:46 PM UTC · Completed 2:01 PM UTC
Commit: ed42dd2 · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 9:29 AM UTC · Completed 9:47 AM UTC
Commit: ed42dd2 · View workflow run →

[fullsend-ai-retro]

Retro: PR #5753 — docs(#5751): document compose/diff bidirectional invariant

Timeline

1. Jul 30 08:25 — Issue #5751 filed by the retro agent after analyzing PR fix(#5448): use field-level merge for validation_loop in base composition #5450's 6-iteration failure. Triaged as ready-to-code within minutes.
2. Jul 30 08:31 — Code agent dispatched (code run).
3. Jul 30 08:39 — PR #5753 opened: 2 files (new docs/contributing/harness-composition.md + 1-line AGENTS.md row), 122 additions.
4. Jul 30 08:52 — Review 1 (run): [medium] protected-path on AGENTS.md (informational), [low] internal consistency — document mentions "diff functions" that don't exist in the codebase.
5. Aug 3 11:58 — Human (rh-hemartin) triggers /fs-fix.
6. Aug 3 12:01 — Fix iteration 1 (run): Removes spurious "diff" references from prose and AGENTS.md trigger.
7. Aug 3 12:21 — Review 2 (run): Raises 3 new findings not in Review 1 — [medium] consumer completeness (missing explanation of why diff functions are absent), [low] documentation-completeness (missing reviewer guidance section), [low] terminology-consistency (PR title vs. doc content mismatch).
8. Aug 3 13:38 — Human triggers /fs-fix again.
9. Aug 3 13:43 — Fix iteration 2 (run): Adds diff-function provenance note and "When reviewing PRs" section.
10. Aug 3 14:01 — Review 3 (run): Raises 2 new [low] section-structure findings (H2 heading vs. bold inline convention, ## Related section pattern not used by other docs).
11. Aug 5 09:20 — Human approves (empty body), correctly judging remaining low-severity nits as not worth another iteration.
12. Aug 5 09:27 — PR merged.

Total: 3 review cycles, 2 fix iterations, 6 days wall-clock for a 122-line docs-only PR.

What went well

• Review 1 caught a real issue. The code agent wrote about "diff functions" (DiffHarness, diffForgeConfig) that had been removed per ADR-0045. The review correctly identified the internal inconsistency.
• Fix agent was effective. Both fix iterations were targeted and correct. The fix agent properly triaged the protected-path finding as informational and made appropriate changes.
• Human judgment was sound. rh-hemartin approved after Review 3 despite two remaining low-severity nits, avoiding a third fix cycle for diminishing returns.

Improvement opportunities (all covered by existing issues)

All candidate proposals overlap with existing open issues. Evidence from this PR is noted below for each cluster.

1. Cascading new findings on re-review — Each review iteration raised entirely new findings on largely unchanged content. Review 2 raised a medium-severity "consumer completeness" finding that was equally true during Review 1 (the doc always covered migration rather than diff). Review 3 raised stylistic nits about heading format that existed since the fix iteration 2 commit. The root cause (per configuration analysis of fullsend-ai/agents:skills/pr-review/SKILL.md) is that the system has severity anchoring but no finding anchoring — nothing prevents new findings on unchanged code. This is additional evidence for:

• #1367 — Review agent surfaces new low-severity findings across multiple approved passes
• #3687 — Review agent should scope-gate findings about unmodified files to prevent cascading scope creep
• #3864 — Review agent flags scope-creep for changes that remediate its own prior findings

2. Code agent did not verify referenced code artifacts exist — Issue #5751 used "compose/diff" framing. The code agent faithfully wrote about "diff functions" without checking whether DiffHarness/diffForgeConfig still exist (they were removed per ADR-0045). This is additional evidence for:

• #5297 — Review docs-currency sub-agent should cross-validate documentation examples against actual source files
• #1322 — Code agent should validate CLI flag semantics in documentation against source code

3. Low-severity stylistic nits driving fix cycles on docs PRs — Reviews 2 and 3 were dominated by low-severity findings about section structure conventions. The human chose to ignore them. The REVIEW_FINDING_SEVERITY_THRESHOLD is set to low, so these are always included. This is additional evidence for:

• #2029 — Review agent: use COMMENT verdict for re-reviews with only low-severity findings
• #3438 — Review agent should use comment-only on governance/process document PRs

Workflow quality

The overall workflow functioned correctly — triage → code → review → fix → re-review → human approval. The fix agent and human reviewer both performed well. The primary inefficiency was the review agent's cascading findings pattern, which turned a simple docs PR into a 3-review, 2-fix cycle. This is a known architectural property with extensive existing issue coverage, not a novel problem.