fix(#6798): preserve child top-level fields over inherited forge values by fullsend-ai-coder[bot] · Pull Request #6801 · fullsend-ai/fullsend

fullsend-ai-coder · 2026-08-31T14:21:39Z

Summary

Fix silent override of child harness top-level fields (pre_script, post_script, policy, skills, providers, runner_env, env) by inherited forge-level values from base harnesses
In mergeBaseIntoChild, snapshot the child's explicit top-level fields and forge platform keys before merge, then clear inherited forge-level fields that would collide with the child's explicit intent
Handles all field types: scalars (cleared entirely), slices (cleared entirely), and maps (only matching keys removed)

Testing

Added TestLoadWithBase_ChildTopLevelOverridesInheritedForge with 6 subtests covering:
- Scalar override (pre_script, post_script survive; unset policy inherits)
- Partial scalar override (child sets pre_script only; post_script inherits)
- Mixed forge ownership (child has github forge block, gitlab inherited — only gitlab cleared)
- Skills and providers not polluted by inherited forge entries
- RunnerEnv matching keys preserved, non-matching forge keys still apply
- Env sub-maps (runner, sandbox) matching keys preserved
All existing forge composition tests pass (TestLoadWithBase_ForgeBlockMerge, TestLoadWithBase_ForgeInheritPlatform, etc.)
Full go test ./internal/harness/... passes
Patch coverage for mergeBaseIntoChild: 95.5%

Post-script verification

Branch is not main/master (agent/6798-fix-forge-inherit-override)
Secret scan passed (gitleaks — 92c1d6498c16ee9fa490150a6405c1bb974aff4a..HEAD)
PR body secret scan passed (gitleaks — no-git)

When a child harness extends a base via `base:` and sets top-level fields (pre_script, post_script, policy, skills, providers, env, etc.), those values were silently overridden by forge-level values inherited from the base during ResolveForge. mergeForgeBlocks inherited the entire base ForgeConfig wholesale for platforms the child didn't define, and mergeForgeConfig could not distinguish between forge values the child explicitly set and values inherited from the base. Fix: snapshot the child's explicit top-level fields and forge platform keys at the start of mergeBaseIntoChild, before any merging. After mergeForgeBlocks, clear inherited forge-level fields that would override the child's explicit top-level values: - Scalars (PreScript, PostScript, Policy): cleared on inherited platforms when the child set the corresponding top-level value - Slices (Skills, Providers, OpenShell.Profiles, HostFiles): cleared when the child defined its own top-level entries - Maps (RunnerEnv, Env): only matching keys removed, preserving forge-only keys the child did not set Note: pre-commit hooks could not run in sandbox (network blocked for git fetch). gofmt and go vet passed via direct execution. Closes #6798

codecov · 2026-08-31T14:25:21Z

Codecov Report

❌ Patch coverage is 79.31034% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/harness/compose.go	79.31%	6 Missing and 6 partials ⚠️

📢 Thoughts on this report? Let us know!

fullsend-ai-review · 2026-08-31T14:28:34Z

🤖 Finished Review · ✅ Success · Started 2:28 PM UTC · Completed 3:04 PM UTC

Commit: 815cd30 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.46

fullsend-ai-review · 2026-08-31T15:04:35Z

Risk Assessment: elevated (3/5)

Details

Elevated risk driven by Tier 2: compose.go remains a high-churn hotspot (54 commits, 7 authors, 32 fix/revert commits in 6 months). Tier 1 signals are effectively unchanged from prior assessment (4 files, 359 lines, medium blast radius). Tier 3 remains low — well-scoped priority/high bug fix with all acceptance criteria met. Score anchored at prior value of 3 as no signals shifted materially.

Previous run

Risk Assessment: elevated (3/5)

Details

Elevated risk driven by Tier 2: compose.go is a high-churn hotspot (62 commits, 7 authors, 10+ fix commits in 6 months) in a large, complex module under active evolution. The PR itself is well-scoped (2 files, 50% test ratio, priority/high bug fix with all acceptance criteria met), keeping Tier 1 and Tier 3 moderate-to-low.

fullsend-ai-review · 2026-08-31T15:04:37Z

Review

Findings

Low

[documentation-comment] internal/harness/compose.go:544 — The mergeBaseIntoChild doc comment enumerates all merge rules as a bulleted list but does not mention the new post-merge clearing step for inherited forge-level fields. The existing bullet forge: key-by-key merge; per-platform uses same rules is now incomplete — after mergeForgeBlocks, inherited forge platforms have conflicting fields cleared. The contributing docs (harness-composition.md, harness-fields.md) were updated, but the function's own doc comment was not.
Remediation: Extend the forge bullet or add a new bullet, e.g., forge: key-by-key merge; per-platform uses same rules; inherited platforms have fields cleared where child set explicit top-level values (#6798).

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

[missing-field] internal/harness/compose.go — The fix clears inherited forge-level fields (PreScript, PostScript, Policy, Skills, Providers, OpenShell.Profiles, HostFiles, RunnerEnv, Env) on inherited platforms, but omits ValidationLoop. mergeForgeConfig in forge.go (line 387–389) replaces the top-level ValidationLoop entirely when the forge config's is non-nil (h.ValidationLoop = fc.ValidationLoop). If a child harness explicitly sets a top-level validation_loop and extends a base whose forge block also defines one, the inherited forge validation_loop will override the child's during ResolveForge — the same class of bug this PR fixes for other fields.
Remediation: Add childHasValidationLoop := child.ValidationLoop != nil to the snapshot block, and if childHasValidationLoop { fc.ValidationLoop = nil } to the clearing loop.

Low

[naming-convention] internal/harness/compose_test.go:632 — Subtest names use snake_case (e.g., "partial_scalars", "child_has_forge_for_one_platform") while existing subtests in compose_test.go use space-separated lower-case strings (e.g., "cache hit", "no overlap appends", "child overrides base by basename").
Remediation: Rename subtests to use spaces instead of underscores for consistency.
[stale-behavior] docs/contributing/harness-fields.md:67 — The "Merge and inheritance rules" section states "The same rules apply during base: composition." This is no longer fully accurate: mergeBaseIntoChild now clears inherited forge-level fields that would override the child's explicit top-level values during ResolveForge. This clearing step is specific to base composition and has no counterpart in forge resolution.
Remediation: Add a note explaining that during base composition, inherited forge-level fields conflicting with child's explicit top-level values are cleared before ResolveForge (see bug: child harness top-level fields silently overridden by inherited forge-level values #6798).
[stale-behavior] docs/contributing/harness-composition.md:57 — The "How they correspond" section describes the invariant between merge functions but does not mention the new post-merge clearing step in mergeBaseIntoChild. After mergeForgeBlocks, inherited forge-level fields that conflict with the child's explicit top-level values are now cleared.
Remediation: Add a note explaining the post-merge clearing step and reference bug: child harness top-level fields silently overridden by inherited forge-level values #6798.

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-coder · 2026-08-31T15:06:48Z

🤖 Finished Fix · ✅ Success · Started 3:06 PM UTC · Completed 3:14 PM UTC

Commit: 815cd30 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.16

- Add missing ValidationLoop to the inherited forge-level clearing loop in mergeBaseIntoChild, preventing the same silent-override bug that the PR fixes for other fields (missing-field finding) - Rename subtests in TestLoadWithBase_ChildTopLevelOverridesInheritedForge from snake_case to space-separated lower-case to match existing compose_test.go conventions (naming-convention finding) - Add a new "validation loop" subtest covering the ValidationLoop fix - Update docs/contributing/harness-fields.md to note the post-merge clearing step specific to base composition (stale-behavior finding) - Update docs/contributing/harness-composition.md to document the post-merge clearing step after mergeForgeBlocks (stale-behavior finding) Addresses review feedback on #6801

fullsend-ai-coder · 2026-08-31T15:14:58Z

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed all 4 review findings. Added missing ValidationLoop handling to the forge-level clearing loop with a new test. Renamed subtests to match repo conventions. Updated both harness-fields.md and harness-composition.md to document the post-merge clearing step.

Fixed (4):

missing ValidationLoop in inherited forge-level clearing loop (internal/harness/compose.go): Added childHasValidationLoop snapshot and clearing logic (fc.ValidationLoop = nil) for inherited forge platforms, preventing the same silent-override bug that the PR fixes for other fields
subtest names use snake_case instead of space-separated lower-case (internal/harness/compose_test.go): Renamed subtests from snake_case (partial_scalars, child_has_forge_for_one_platform, skills_and_providers, runner_env, env_sub_maps) to space-separated lower-case to match existing compose_test.go conventions
harness-fields.md merge rules section is stale after post-merge clearing step (docs/contributing/harness-fields.md): Added note explaining that during base composition, mergeBaseIntoChild clears inherited forge-level fields conflicting with child's explicit top-level values before ResolveForge runs, referencing bug: child harness top-level fields silently overridden by inherited forge-level values #6798
harness-composition.md does not mention post-merge clearing step (docs/contributing/harness-composition.md): Added blockquote note documenting the post-merge clearing step after mergeForgeBlocks, explaining it is specific to base composition and advising that new ForgeConfig fields must be handled in the clearing loop

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}

github-actions · 2026-08-31T15:16:46Z

Site preview

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

Commit: 728bffb10cb10702361068da7663a6f36e20c66e

fullsend-ai-review · 2026-08-31T15:17:04Z

🤖 Finished Review · ✅ Success · Started 3:17 PM UTC · Completed 3:34 PM UTC

Commit: 7042c29 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.49

fullsend-ai-review

See the review comment for full details.

fullsend-ai-review · 2026-08-31T15:34:19Z

@@ -544,6 +544,43 @@ func matchingAllowedPrefix(rawURL string, allowlist []string) string {
 //   - forge: key-by-key merge; per-platform uses same rules


[low] documentation-comment

The mergeBaseIntoChild doc comment enumerates all merge rules as a bulleted list but does not mention the new post-merge clearing step for inherited forge-level fields. The existing bullet 'forge: key-by-key merge; per-platform uses same rules' is now incomplete — after mergeForgeBlocks, inherited forge platforms have conflicting fields cleared. The contributing docs (harness-composition.md, harness-fields.md) were updated, but the function's own doc comment was not.

Suggested fix: Extend the forge bullet or add a new bullet, e.g., 'forge: key-by-key merge; per-platform uses same rules; inherited platforms have fields cleared where child set explicit top-level values (#6798)'.

fullsend-ai-coder · 2026-08-31T15:36:10Z

🤖 Finished Fix · ✅ Success · Started 3:36 PM UTC · Completed 3:41 PM UTC

Commit: 7042c29 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $0.94

…g step Extend the forge bullet in the mergeBaseIntoChild doc comment to mention the post-merge clearing of inherited forge-level fields where the child set explicit top-level values (#6798). Addresses review feedback on #6801

fullsend-ai-coder · 2026-08-31T15:41:58Z

🔧 Fix agent — iteration 2 (bot-triggered)

Addressed 1 review finding. Updated the mergeBaseIntoChild function doc comment to document the post-merge clearing step for inherited forge-level fields.

Fixed (1):

mergeBaseIntoChild doc comment does not mention post-merge clearing step for inherited forge-level fields (internal/harness/compose.go): Extended the forge bullet in the mergeBaseIntoChild doc comment to mention that inherited platforms have fields cleared where child set explicit top-level values (bug: child harness top-level fields silently overridden by inherited forge-level values #6798)

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-coder Bot requested a review from a team as a code owner August 31, 2026 14:21

fullsend-ai-coder Bot added the ready-for-review Agent PR ready for human review label Aug 31, 2026

fullsend-ai-coder Bot assigned ggallen Aug 31, 2026

fullsend-ai-coder Bot temporarily deployed to dev August 31, 2026 14:21 Inactive

fullsend-ai-triage Bot mentioned this pull request Aug 31, 2026

bug: child harness top-level fields silently overridden by inherited forge-level values #6798

Open

fullsend-ai-review Bot added the risk/elevated PR risk: elevated label Aug 31, 2026

This comment was marked as outdated.

Sign in to view

fullsend-ai-coder Bot temporarily deployed to dev August 31, 2026 15:15 Inactive

github-actions Bot deployed to site-preview August 31, 2026 15:16 View deployment

fullsend-ai-review Bot suggested changes Aug 31, 2026

View reviewed changes

fullsend-ai-coder Bot temporarily deployed to dev August 31, 2026 15:42 Inactive

github-actions Bot deployed to site-preview August 31, 2026 15:43 View deployment

		@@ -544,6 +544,43 @@ func matchingAllowedPrefix(rawURL string, allowlist []string) string {
		// - forge: key-by-key merge; per-platform uses same rules

Uh oh!

Conversation

fullsend-ai-coder Bot commented Aug 31, 2026

Summary

Testing

Post-script verification

Uh oh!

codecov Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fullsend-ai-review Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Findings

Low

Review

Findings

Medium

Low

Uh oh!

This comment was marked as outdated.

Uh oh!

fullsend-ai-coder Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-coder Bot commented Aug 31, 2026

🔧 Fix agent — iteration 1 (bot-triggered)

Tests: passed

Uh oh!

github-actions Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Site preview

Uh oh!

fullsend-ai-review Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-review Bot Aug 31, 2026

Choose a reason for hiding this comment

Uh oh!

fullsend-ai-coder Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fullsend-ai-coder Bot commented Aug 31, 2026

🔧 Fix agent — iteration 2 (bot-triggered)

Tests: passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov Bot commented Aug 31, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 31, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 31, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 31, 2026 •

edited

Loading

fullsend-ai-coder Bot commented Aug 31, 2026 •

edited

Loading

github-actions Bot commented Aug 31, 2026 •

edited

Loading

fullsend-ai-review Bot commented Aug 31, 2026 •

edited

Loading

fullsend-ai-coder Bot commented Aug 31, 2026 •

edited

Loading