Skip to content

docs(#6160): extract harness field reference into living doc - #6398

Merged
ralphbean merged 2 commits into
mainfrom
agent/6160-harness-fields-living-doc
Aug 20, 2026
Merged

docs(#6160): extract harness field reference into living doc#6398
ralphbean merged 2 commits into
mainfrom
agent/6160-harness-fields-living-doc

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Extract evolving harness field classification tables, merge rules, and ForgeConfig struct definition from ADR-0045 and ADR-0088 into a new living reference document (docs/contributing/harness-fields.md). Add step 6 to the harness-composition.md checklist directing developers to update this document when adding or modifying harness fields. Mark ADR-0045 as Superseded by ADR-0088.

Changes

  • New file: docs/contributing/harness-fields.md — authoritative living reference for field classifications (both-level vs top-level-only), merge/inheritance rules, ForgeConfig struct, and overlay resolution pipeline
  • docs/contributing/harness-composition.md — add step 6 to the checklist pointing to the living doc; update step 1 cross-reference from ADR-0045 to the living doc; add living doc and ADR-0088 to Related section
  • docs/ADRs/0045-forge-portable-harness-schema.md — change status to Superseded by ADR-0088; add cross-reference note to living doc
  • docs/ADRs/0088-cel-guarded-overlays.md — add cross-reference note to living doc
  • docs/architecture.md — update ADR-0045 reference to note supersession and link to living doc
  • AGENTS.md — add harness-fields.md to topic-specific guidance table

Testing

  • No code changes — documentation only
  • Verified all cross-references between documents are consistent
  • gitlint --commit HEAD passes
  • Secret scan passes

Closes #6160

Post-script verification

  • Branch is not main/master (agent/6160-harness-fields-living-doc)
  • Secret scan passed (gitleaks — 4c61f4ca8850b08d5685213923abafd6ece2d84a..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

The harness-composition.md checklist (steps 1-5) covered merge function
updates but did not include maintaining field classification tables when
a field's forge-level status changed. This caused a HIGH review finding
on PR #6139 where ADR-0045's tables contradicted the code change.

Per maintainer direction, instead of adding a step that points to
the frozen ADR, create a living reference document
(docs/contributing/harness-fields.md) that extracts the evolving
field classifications, merge rules, and ForgeConfig struct from
ADR-0045 and overlay rules from ADR-0088.

Changes:
- Create docs/contributing/harness-fields.md as the authoritative
  living reference for field classifications and merge rules
- Add step 6 to harness-composition.md checklist pointing to
  the living doc
- Mark ADR-0045 as Superseded by ADR-0088 with cross-reference
  to the living doc
- Add cross-reference note to ADR-0088 pointing to the living doc
- Update docs/architecture.md to note ADR-0045 supersession and
  link to the living doc
- Add harness-fields.md to AGENTS.md topic-specific guidance table

Note: pre-commit could not run (sandbox network policy blocks git
fetch for hook environment setup). The post-script runs pre-commit
authoritatively on the runner.

Closes #6160
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 19, 2026 20:39
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Agent PR ready for human review label Aug 19, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:41 PM UTC · Completed 8:56 PM UTC

Commit: f2c6cde · View workflow run →

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

Site preview

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

Commit: 947812b29459e5b14fa109f13639b3b8e4ad281e

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] AGENTS.md, skills/author-fullsend-augmentations/SKILL.md — This PR modifies protected governance files (AGENTS.md and skills/). The PR links to issue Add ADR-0045 maintenance step to harness-composition.md checklist #6160 and the description explains the rationale (adding the new harness-fields.md entry to the AGENTS.md guidance table and updating a stale reference in the augmentations skill). Human approval is always required for protected-path changes, regardless of context.

Low

  • [Inaccurate merge rule description] docs/contributing/harness-fields.md:75 — The merge rules table describes validation_loop as "Forge/child value replaces top-level/base value entirely" and the heading claims "The same rules apply during base: composition." This is mostly accurate but omits a nuance: during base: composition (mergeBaseIntoChild and mergeForgeConfigInto), PreflightCheck is carried forward from the base when the child sets a ValidationLoop without its own preflight_check. During forge resolution (mergeForgeConfig), the forge value replaces entirely with no carry-forward.
Previous run

Review

Findings

High

  • [Inaccurate struct definition] docs/contributing/harness-fields.md:99 — The ForgeConfig struct definition is missing the Env *EnvConfig field. The actual struct in internal/harness/forge.go:24 includes Env *EnvConfig \yaml:"env,omitempty"`, with merge behavior implemented across mergeForgeConfig(forge.go:211–217),mergeBaseIntoChild(compose.go:625–631), andmergeForgeConfigInto(compose.go:2032–2038). A document claiming to be "the authoritative reference" for theForgeConfigstruct must not omit a field with three distinct merge-code paths. Theenvfield must also be added to the "Fields that can appear at both levels" table and the merge rules table. Remediation: AddEnv *EnvConfig `yaml:"env,omitempty"`to the struct definition, addenvto the both-levels classification table with rationale, and add anenv` row to the merge rules table describing sub-map merge behavior (sub-maps merged independently, forge/child keys win).

Medium

  • [Incomplete field classification] docs/contributing/harness-fields.md:40 — The "Fields that stay at top level only" table is missing several fields from the Harness struct that do not appear in ForgeConfig: doc, effort, readonly_repo, allow_runtime_fetch, max_runtime_fetches, and trigger. An authoritative field classification reference should enumerate all fields.
    Remediation: Add the missing fields with appropriate rationale.

  • [Documents unimplemented behavior as current] docs/contributing/harness-fields.md:110 — The "Overlay resolution" section presents validateOverlays and ResolveOverlays(event, config) as current implementation, but neither function exists in internal/harness/. The Harness struct has no Overlays field. ADR-0088 is accepted but the overlay feature has not been implemented. Presenting planned behavior as current fact will mislead contributors.
    Remediation: Either clearly mark the overlay resolution section as planned/future behavior from ADR-0088 (noting the current pipeline is Unmarshal → validateForge → ResolveForge → Validate), or defer overlay details until implementation lands.

  • [Missing merge rule] docs/contributing/harness-fields.md:85 — The merge rules table includes an overlays row for behavior that does not exist in the codebase, while omitting the implemented env field's merge behavior entirely. See also: [Inaccurate struct definition] finding above.
    Remediation: Add an env row to the merge rules table; mark or remove the overlays row as planned/future.

  • [protected-path] AGENTS.md — This PR modifies AGENTS.md, a protected governance file. The PR links to issue Add ADR-0045 maintenance step to harness-composition.md checklist #6160 and the description explains the rationale (adding the new harness-fields.md entry to the guidance table). Human approval is always required for protected-path changes, regardless of context.

Low

  • [frontmatter convention] docs/ADRs/0045-forge-portable-harness-schema.md:4 — The superseded_by YAML frontmatter key is new to this codebase. All four existing Superseded ADRs (0010, 0035, 0039, 0070) use only status: Superseded in frontmatter with no superseded_by field. This creates a frontmatter inconsistency.
    Remediation: Either remove the superseded_by key (matching existing pattern) or adopt it as the new convention by backfilling into the other superseded ADRs and documenting the key.

  • [stale-reference] skills/author-fullsend-augmentations/SKILL.md:273 — The parenthetical "(still verify in ADR 0045 / compose docs)" references ADR-0045 for merge behavior verification, but ADR-0045 is now superseded. The authoritative source is docs/contributing/harness-fields.md.
    Remediation: Update to reference the living doc.

  • [Inaccurate merge description] docs/contributing/harness-fields.md:66 — The skills merge description in the "Nil vs empty" column says "skills: [] = no forge-specific additions (top-level skills still apply)". The wording implies a special preservation mechanism rather than the normal merge function returning base entries when the child slice is empty.


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

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 8:58 PM UTC · Completed 9:06 PM UTC

Commit: f2c6cde · View workflow run →

- Add missing `Env *EnvConfig` field to ForgeConfig struct definition,
  both-levels classification table, and merge rules table
- Add missing top-level-only fields: doc, effort, readonly_repo,
  allow_runtime_fetch, max_runtime_fetches, trigger
- Mark overlay resolution section as planned/future (ADR-0088 not yet
  implemented); add current resolution pipeline section
- Mark overlays merge rule row as planned
- Remove superseded_by frontmatter key from ADR-0045 to match existing
  convention (other superseded ADRs use only status field)
- Update stale ADR-0045 reference in SKILL.md to point to living doc
- Clarify skills merge description wording

Addresses review feedback on #6398
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed all 8 review findings. Fixed 7 (missing env field in struct/tables, incomplete field classification, overlay section presented as current, missing env merge rule, frontmatter inconsistency, stale ADR reference in SKILL.md, misleading skills merge wording). Disagreed with 1 (protected-path AGENTS.md — informational finding about human approval requirement, not a code change request).

Fixed (7):

  1. *Inaccurate struct definition — missing Env EnvConfig field (docs/contributing/harness-fields.md): Added Env *EnvConfig field to ForgeConfig struct definition, added env to both-levels classification table with rationale, and added env row to merge rules table describing sub-map merge behavior
  2. Incomplete field classification — missing top-level-only fields (docs/contributing/harness-fields.md): Added doc, effort, readonly_repo, allow_runtime_fetch, max_runtime_fetches, and trigger to the top-level-only classification table with rationale for each
  3. Documents unimplemented behavior as current — overlay resolution (docs/contributing/harness-fields.md): Added a Current resolution pipeline section showing the actual pipeline. Renamed overlay section to planned with a note that the feature has not been implemented yet. Changed language from present to future tense.
  4. Missing merge rule for env, overlays row documents unimplemented behavior (docs/contributing/harness-fields.md): Added env row to merge rules table. Marked overlays row with planned annotation and not-yet-implemented note.
  5. frontmatter convention — superseded_by key inconsistency (docs/ADRs/0045-forge-portable-harness-schema.md): Removed superseded_by frontmatter key to match existing convention used by all other superseded ADRs (0010, 0035, 0039, 0070).
  6. stale-reference — SKILL.md references superseded ADR-0045 (skills/author-fullsend-augmentations/SKILL.md): Updated parenthetical reference from ADR 0045 / compose docs to link to the living doc harness-fields.md
  7. Inaccurate merge description for skills (docs/contributing/harness-fields.md): Clarified nil-vs-empty column for skills to avoid implying a special preservation mechanism. Now reads: empty list merged with base (base entries are returned)

Disagreed (1):

  1. protected-path — AGENTS.md modification: The AGENTS.md modification was part of the original PR commit, not introduced by this fix. The review correctly notes human approval is required for protected-path changes. This is an informational finding about the approval requirement, not a code change request.

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

fullsend-ai-review Bot commented Aug 19, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:08 PM UTC · Completed 9:24 PM UTC

Commit: 947812b · View workflow run →

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review August 19, 2026 21:24

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 19, 2026
@ralphbean
ralphbean enabled auto-merge August 20, 2026 17:19
@ralphbean
ralphbean added this pull request to the merge queue Aug 20, 2026
Merged via the queue into main with commit 2b92be9 Aug 20, 2026
39 checks passed
@ralphbean
ralphbean deleted the agent/6160-harness-fields-living-doc branch August 20, 2026 17:21
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 20, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 5:24 PM UTC · Completed 5:39 PM UTC

Commit: 947812b · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #6398 — extract harness field reference into living doc

Timeline

This PR has a two-act history. Issue #6160 was auto-generated by the retro agent. On Aug 12, the code agent produced PR #6161, which added a checklist step directing agents to update ADR-0045 in place. The review agent approved it with a blanket "Looks good to me" (zero inline comments) and the PR was auto-labeled ready-for-merge. After sitting for 7 days, maintainer ralphbean closed it without merging, explaining that ADRs should be immutable point-in-time records and the evolving field tables should be extracted into a living reference document.

The system correctly re-triaged issue #6160, incorporating Ralph's feedback. On Aug 19, the code agent produced PR #6398 with the correct approach: a new docs/contributing/harness-fields.md living doc, ADR-0045 marked as superseded, and cross-references updated across 7 files.

PR #6398 workflow quality

The PR #6398 pipeline performed well:

  • Review agent (run 32299704901): found 8 legitimate findings (1 High, 4 Medium, 3 Low) including a missing Env field in the struct definition, 6 missing field classifications, and overlay behavior documented as current when unimplemented.
  • Fix agent (run 32301304174): addressed 7/8 findings in a single iteration. The one disagreement (protected-path on AGENTS.md) was correctly identified as informational.
  • Re-review (run 32302199246): only 2 residual findings remained — the recurring protected-path informational finding and a low-severity nuance about validation_loop merge rules.
  • Human reviewers (rh-hemartin, ralphbean): approved without finding any issues the agent missed. ralphbean's sole inline comment ("out of scope for now") was a scoping acknowledgment on residual field incompleteness — a judgment call that properly requires human authority.

Agent review coverage was a strict superset of human review on PR #6398.

Evidence for existing issues

Proposals filed

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

Labels

ready-for-review Agent PR ready for human review requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ADR-0045 maintenance step to harness-composition.md checklist

2 participants