From 815cd30715a3a4e13e5cdfdd6b54995627b7d59e Mon Sep 17 00:00:00 2001
From: fullsend-code
 <278716306+fullsend-ai-coder[bot]@users.noreply.github.com>
Date: Mon, 31 Aug 2026 14:19:18 +0000
Subject: [PATCH 1/3] fix(#6798): preserve child top-level fields over
 inherited forge values

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
---
 internal/harness/compose.go      | 104 ++++++++++++++++
 internal/harness/compose_test.go | 203 +++++++++++++++++++++++++++++++
 2 files changed, 307 insertions(+)

diff --git a/internal/harness/compose.go b/internal/harness/compose.go
index aa7608546..c19c20cab 100644
--- a/internal/harness/compose.go
+++ b/internal/harness/compose.go
@@ -544,6 +544,42 @@ func matchingAllowedPrefix(rawURL string, allowlist []string) string {
 //   - forge: key-by-key merge; per-platform uses same rules
 //   - allowed_remote_resources: NOT merged (security; child must declare its own)
 func mergeBaseIntoChild(base, child *Harness) {
+	// Snapshot child's explicit top-level fields BEFORE any merge so we
+	// can clear inherited forge-level fields that would override them
+	// during ResolveForge (see #6798).
+	childHasPreScript := child.PreScript != ""
+	childHasPostScript := child.PostScript != ""
+	childHasPolicy := child.Policy != ""
+	childHasSkills := len(child.Skills) > 0
+	childHasProviders := len(child.Providers) > 0
+	childHasOpenShellProfiles := child.OpenShell != nil && len(child.OpenShell.Profiles) > 0
+	childHasHostFiles := len(child.HostFiles) > 0
+
+	// Snapshot child's explicit runner_env keys so we only remove matching
+	// inherited forge keys, not all inherited env vars.
+	childRunnerEnvKeys := make(map[string]bool, len(child.RunnerEnv))
+	for k := range child.RunnerEnv {
+		childRunnerEnvKeys[k] = true
+	}
+
+	// Snapshot child's explicit env sub-map keys.
+	childEnvRunnerKeys := make(map[string]bool)
+	childEnvSandboxKeys := make(map[string]bool)
+	if child.Env != nil {
+		for k := range child.Env.Runner {
+			childEnvRunnerKeys[k] = true
+		}
+		for k := range child.Env.Sandbox {
+			childEnvSandboxKeys[k] = true
+		}
+	}
+
+	// Snapshot child's existing forge platform keys before mergeForgeBlocks.
+	childForgeKeys := make(map[string]bool, len(child.Forge))
+	for k := range child.Forge {
+		childForgeKeys[k] = true
+	}
+
 	// Scalars: child overrides if non-zero
 	if child.Agent == "" {
 		child.Agent = base.Agent
@@ -671,6 +707,74 @@ func mergeBaseIntoChild(base, child *Harness) {
 	// Forge: key-by-key merge
 	if base.Forge != nil {
 		child.Forge = mergeForgeBlocks(base.Forge, child.Forge)
+
+		// Clear inherited forge-level fields that would override the child's
+		// explicit top-level values during ResolveForge (#6798).
+		//
+		// mergeForgeBlocks inherits the entire base ForgeConfig wholesale for
+		// platforms the child doesn't define. mergeForgeConfig (called by
+		// ResolveForge) then blindly applies those inherited forge-level
+		// values over the child's top-level values. By clearing inherited
+		// fields here, we ensure the child's explicit top-level intent is
+		// preserved.
+		for key, fc := range child.Forge {
+			if childForgeKeys[key] || fc == nil {
+				continue // child explicitly defined this platform
+			}
+			// Scalars: clear if child explicitly set the top-level value
+			if childHasPreScript {
+				fc.PreScript = ""
+			}
+			if childHasPostScript {
+				fc.PostScript = ""
+			}
+			if childHasPolicy {
+				fc.Policy = ""
+			}
+			// Slices: clear if child explicitly defined its own top-level entries
+			if childHasSkills {
+				fc.Skills = nil
+			}
+			if childHasProviders {
+				fc.Providers = nil
+			}
+			if childHasOpenShellProfiles && fc.OpenShell != nil {
+				fc.OpenShell.Profiles = nil
+			}
+			if childHasHostFiles {
+				fc.HostFiles = nil
+			}
+			// Maps: remove only keys the child explicitly set at top level
+			if fc.RunnerEnv != nil {
+				for k := range childRunnerEnvKeys {
+					delete(fc.RunnerEnv, k)
+				}
+				if len(fc.RunnerEnv) == 0 {
+					fc.RunnerEnv = nil
+				}
+			}
+			if fc.Env != nil {
+				if fc.Env.Runner != nil {
+					for k := range childEnvRunnerKeys {
+						delete(fc.Env.Runner, k)
+					}
+					if len(fc.Env.Runner) == 0 {
+						fc.Env.Runner = nil
+					}
+				}
+				if fc.Env.Sandbox != nil {
+					for k := range childEnvSandboxKeys {
+						delete(fc.Env.Sandbox, k)
+					}
+					if len(fc.Env.Sandbox) == 0 {
+						fc.Env.Sandbox = nil
+					}
+				}
+				if fc.Env.Runner == nil && fc.Env.Sandbox == nil {
+					fc.Env = nil
+				}
+			}
+		}
 	}
 
 	// Overlays: concatenated (base first, child appended) — same as plugins,
diff --git a/internal/harness/compose_test.go b/internal/harness/compose_test.go
index 709244bc4..3c93ade83 100644
--- a/internal/harness/compose_test.go
+++ b/internal/harness/compose_test.go
@@ -629,6 +629,209 @@ model: opus
 	assert.Equal(t, "gl-pre.sh", h.PreScript)
 }
 
+func TestLoadWithBase_ChildTopLevelOverridesInheritedForge(t *testing.T) {
+	t.Run("scalars", func(t *testing.T) {
+		dir := t.TempDir()
+
+		writeTestHarness(t, dir, "base.yaml", `
+agent: agents/test.md
+role: test
+pre_script: base-top-pre.sh
+post_script: base-top-post.sh
+forge:
+  github:
+    pre_script: base-forge-pre.sh
+    post_script: base-forge-post.sh
+    policy: base-forge-policy.yaml
+  gitlab:
+    pre_script: base-forge-gl-pre.sh
+    post_script: base-forge-gl-post.sh
+`)
+
+		// Child sets top-level scripts but has no forge block.
+		// Child's top-level values must survive ResolveForge.
+		path := writeTestHarness(t, dir, "child.yaml", `
+base: base.yaml
+pre_script: child-pre.sh
+post_script: child-post.sh
+`)
+
+		h, _, err := LoadWithBase(context.Background(), path, ComposeOpts{
+			ForgePlatform: "github",
+		})
+		require.NoError(t, err)
+
+		assert.Equal(t, "child-pre.sh", h.PreScript, "child top-level pre_script must survive inherited forge")
+		assert.Equal(t, "child-post.sh", h.PostScript, "child top-level post_script must survive inherited forge")
+		// Policy was not set by child, so inherited forge value should apply.
+		assert.Equal(t, "base-forge-policy.yaml", h.Policy, "inherited forge policy should apply when child did not set it")
+	})
+
+	t.Run("partial_scalars", func(t *testing.T) {
+		dir := t.TempDir()
+
+		writeTestHarness(t, dir, "base.yaml", `
+agent: agents/test.md
+role: test
+forge:
+  github:
+    pre_script: base-forge-pre.sh
+    post_script: base-forge-post.sh
+`)
+
+		// Child sets only pre_script but not post_script.
+		path := writeTestHarness(t, dir, "child.yaml", `
+base: base.yaml
+pre_script: child-pre.sh
+`)
+
+		h, _, err := LoadWithBase(context.Background(), path, ComposeOpts{
+			ForgePlatform: "github",
+		})
+		require.NoError(t, err)
+
+		assert.Equal(t, "child-pre.sh", h.PreScript, "child top-level pre_script must win")
+		assert.Equal(t, "base-forge-post.sh", h.PostScript, "inherited forge post_script should apply when child did not set it")
+	})
+
+	t.Run("child_has_forge_for_one_platform", func(t *testing.T) {
+		dir := t.TempDir()
+
+		writeTestHarness(t, dir, "base.yaml", `
+agent: agents/test.md
+role: test
+forge:
+  github:
+    pre_script: base-forge-gh-pre.sh
+  gitlab:
+    pre_script: base-forge-gl-pre.sh
+`)
+
+		// Child has a forge block for github but not gitlab.
+		// Only gitlab should have inherited fields cleared.
+		path := writeTestHarness(t, dir, "child.yaml", `
+base: base.yaml
+pre_script: child-pre.sh
+forge:
+  github:
+    post_script: child-forge-gh-post.sh
+`)
+
+		h, _, err := LoadWithBase(context.Background(), path, ComposeOpts{
+			ForgePlatform: "gitlab",
+		})
+		require.NoError(t, err)
+
+		// GitLab was inherited; child's top-level pre_script should win.
+		assert.Equal(t, "child-pre.sh", h.PreScript, "child top-level pre_script must survive inherited gitlab forge")
+	})
+
+	t.Run("skills_and_providers", func(t *testing.T) {
+		dir := t.TempDir()
+
+		writeTestHarness(t, dir, "base.yaml", `
+agent: agents/test.md
+role: test
+forge:
+  github:
+    skills:
+      - base-forge-skill
+    providers:
+      - base-forge-provider
+`)
+
+		// Child sets top-level skills and providers.
+		path := writeTestHarness(t, dir, "child.yaml", `
+base: base.yaml
+skills:
+  - child-skill
+providers:
+  - child-provider
+`)
+
+		h, _, err := LoadWithBase(context.Background(), path, ComposeOpts{
+			ForgePlatform: "github",
+		})
+		require.NoError(t, err)
+
+		// Inherited forge skills/providers should not be concatenated onto child's.
+		assert.Equal(t, []string{"child-skill"}, SkillSources(h.Skills),
+			"child top-level skills must not be polluted by inherited forge skills")
+		assert.Equal(t, []string{"child-provider"}, h.Providers,
+			"child top-level providers must not be polluted by inherited forge providers")
+	})
+
+	t.Run("runner_env", func(t *testing.T) {
+		dir := t.TempDir()
+
+		writeTestHarness(t, dir, "base.yaml", `
+agent: agents/test.md
+role: test
+forge:
+  github:
+    runner_env:
+      SHARED_KEY: base-forge-value
+      FORGE_ONLY: forge-only-value
+`)
+
+		// Child sets a matching top-level runner_env key.
+		path := writeTestHarness(t, dir, "child.yaml", `
+base: base.yaml
+runner_env:
+  SHARED_KEY: child-value
+`)
+
+		h, _, err := LoadWithBase(context.Background(), path, ComposeOpts{
+			ForgePlatform: "github",
+		})
+		require.NoError(t, err)
+
+		assert.Equal(t, "child-value", h.RunnerEnv["SHARED_KEY"],
+			"child top-level runner_env key must survive inherited forge override")
+		assert.Equal(t, "forge-only-value", h.RunnerEnv["FORGE_ONLY"],
+			"inherited forge env key not set by child should still apply")
+	})
+
+	t.Run("env_sub_maps", func(t *testing.T) {
+		dir := t.TempDir()
+
+		writeTestHarness(t, dir, "base.yaml", `
+agent: agents/test.md
+role: test
+forge:
+  github:
+    env:
+      runner:
+        SHARED_KEY: base-forge-runner-val
+        FORGE_ONLY: forge-runner-val
+      sandbox:
+        SB_SHARED: base-forge-sb-val
+`)
+
+		// Child sets top-level env with a matching key.
+		path := writeTestHarness(t, dir, "child.yaml", `
+base: base.yaml
+env:
+  runner:
+    SHARED_KEY: child-runner-val
+  sandbox:
+    SB_SHARED: child-sb-val
+`)
+
+		h, _, err := LoadWithBase(context.Background(), path, ComposeOpts{
+			ForgePlatform: "github",
+		})
+		require.NoError(t, err)
+
+		assert.Equal(t, "child-runner-val", h.Env.Runner["SHARED_KEY"],
+			"child top-level env.runner key must survive inherited forge override")
+		assert.Equal(t, "forge-runner-val", h.Env.Runner["FORGE_ONLY"],
+			"inherited forge env.runner key not set by child should still apply")
+		assert.Equal(t, "child-sb-val", h.Env.Sandbox["SB_SHARED"],
+			"child top-level env.sandbox key must survive inherited forge override")
+	})
+}
+
 func TestLoadWithBase_URLBase(t *testing.T) {
 	baseContent := []byte(`
 agent: agents/remote.md

From 7042c291542936a5291a95453cd767d00d726e8d Mon Sep 17 00:00:00 2001
From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com>
Date: Mon, 31 Aug 2026 15:12:50 +0000
Subject: [PATCH 2/3] fix(harness): address review feedback on PR #6801

- 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
---
 docs/contributing/harness-composition.md |  8 +++++
 docs/contributing/harness-fields.md      |  7 +++-
 internal/harness/compose.go              |  4 +++
 internal/harness/compose_test.go         | 43 +++++++++++++++++++++---
 4 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/docs/contributing/harness-composition.md b/docs/contributing/harness-composition.md
index edf3ddb6d..95d4eb534 100644
--- a/docs/contributing/harness-composition.md
+++ b/docs/contributing/harness-composition.md
@@ -63,6 +63,14 @@ For example, if `mergeBaseIntoChild` gains handling for a new
 `foo_script` scalar field, then `mergeForgeConfigInto` must also
 handle `foo_script` if it appears inside `ForgeConfig`.
 
+> **Note — post-merge clearing.** After `mergeForgeBlocks`,
+> `mergeBaseIntoChild` clears inherited forge-level fields that would
+> conflict with the child's explicit top-level values during
+> `ResolveForge` ([#6798](https://github.com/fullsend-ai/fullsend/issues/6798)).
+> This clearing step is specific to base composition — `ResolveForge`
+> itself does not perform it. When adding a new field to `ForgeConfig`,
+> ensure the clearing loop in `mergeBaseIntoChild` handles it as well.
+
 > **Note — removed counterparts.** Earlier versions of this document
 > referenced path-rewriting functions in `internal/cli/migrate.go` and
 > diff functions (`DiffHarness`, `diffForgeConfig`) as counterparts to
diff --git a/docs/contributing/harness-fields.md b/docs/contributing/harness-fields.md
index 42e47d781..7ce7113de 100644
--- a/docs/contributing/harness-fields.md
+++ b/docs/contributing/harness-fields.md
@@ -66,7 +66,12 @@ per-overlay:
 
 When a forge block or overlay is merged into the harness top level, each
 field type follows specific merge semantics. The same rules apply during
-`base:` composition (base → child merging).
+`base:` composition (base → child merging), with one addition: during
+base composition, `mergeBaseIntoChild` clears inherited forge-level fields
+that would conflict with the child's explicit top-level values before
+`ResolveForge` runs (see [#6798](https://github.com/fullsend-ai/fullsend/issues/6798)).
+This prevents inherited forge platforms from silently overriding a child's
+explicit top-level intent.
 
 | Field type       | Merge behavior                                       | Nil vs empty                                          |
 |------------------|------------------------------------------------------|-------------------------------------------------------|
diff --git a/internal/harness/compose.go b/internal/harness/compose.go
index c19c20cab..b7002c3cf 100644
--- a/internal/harness/compose.go
+++ b/internal/harness/compose.go
@@ -554,6 +554,7 @@ func mergeBaseIntoChild(base, child *Harness) {
 	childHasProviders := len(child.Providers) > 0
 	childHasOpenShellProfiles := child.OpenShell != nil && len(child.OpenShell.Profiles) > 0
 	childHasHostFiles := len(child.HostFiles) > 0
+	childHasValidationLoop := child.ValidationLoop != nil
 
 	// Snapshot child's explicit runner_env keys so we only remove matching
 	// inherited forge keys, not all inherited env vars.
@@ -744,6 +745,9 @@ func mergeBaseIntoChild(base, child *Harness) {
 			if childHasHostFiles {
 				fc.HostFiles = nil
 			}
+			if childHasValidationLoop {
+				fc.ValidationLoop = nil
+			}
 			// Maps: remove only keys the child explicitly set at top level
 			if fc.RunnerEnv != nil {
 				for k := range childRunnerEnvKeys {
diff --git a/internal/harness/compose_test.go b/internal/harness/compose_test.go
index 3c93ade83..442bbd10b 100644
--- a/internal/harness/compose_test.go
+++ b/internal/harness/compose_test.go
@@ -667,7 +667,7 @@ post_script: child-post.sh
 		assert.Equal(t, "base-forge-policy.yaml", h.Policy, "inherited forge policy should apply when child did not set it")
 	})
 
-	t.Run("partial_scalars", func(t *testing.T) {
+	t.Run("partial scalars", func(t *testing.T) {
 		dir := t.TempDir()
 
 		writeTestHarness(t, dir, "base.yaml", `
@@ -694,7 +694,7 @@ pre_script: child-pre.sh
 		assert.Equal(t, "base-forge-post.sh", h.PostScript, "inherited forge post_script should apply when child did not set it")
 	})
 
-	t.Run("child_has_forge_for_one_platform", func(t *testing.T) {
+	t.Run("child has forge for one platform", func(t *testing.T) {
 		dir := t.TempDir()
 
 		writeTestHarness(t, dir, "base.yaml", `
@@ -726,7 +726,7 @@ forge:
 		assert.Equal(t, "child-pre.sh", h.PreScript, "child top-level pre_script must survive inherited gitlab forge")
 	})
 
-	t.Run("skills_and_providers", func(t *testing.T) {
+	t.Run("skills and providers", func(t *testing.T) {
 		dir := t.TempDir()
 
 		writeTestHarness(t, dir, "base.yaml", `
@@ -761,7 +761,7 @@ providers:
 			"child top-level providers must not be polluted by inherited forge providers")
 	})
 
-	t.Run("runner_env", func(t *testing.T) {
+	t.Run("runner env", func(t *testing.T) {
 		dir := t.TempDir()
 
 		writeTestHarness(t, dir, "base.yaml", `
@@ -792,7 +792,40 @@ runner_env:
 			"inherited forge env key not set by child should still apply")
 	})
 
-	t.Run("env_sub_maps", func(t *testing.T) {
+	t.Run("validation loop", func(t *testing.T) {
+		dir := t.TempDir()
+
+		writeTestHarness(t, dir, "base.yaml", `
+agent: agents/test.md
+role: test
+forge:
+  github:
+    validation_loop:
+      script: base-forge-validate.sh
+      max_iterations: 3
+`)
+
+		// Child sets a top-level validation_loop.
+		path := writeTestHarness(t, dir, "child.yaml", `
+base: base.yaml
+validation_loop:
+  script: child-validate.sh
+  max_iterations: 5
+`)
+
+		h, _, err := LoadWithBase(context.Background(), path, ComposeOpts{
+			ForgePlatform: "github",
+		})
+		require.NoError(t, err)
+
+		require.NotNil(t, h.ValidationLoop, "validation_loop must not be nil")
+		assert.Equal(t, "child-validate.sh", h.ValidationLoop.Script,
+			"child top-level validation_loop must survive inherited forge override")
+		assert.Equal(t, 5, h.ValidationLoop.MaxIterations,
+			"child top-level validation_loop max_iterations must survive inherited forge override")
+	})
+
+	t.Run("env sub maps", func(t *testing.T) {
 		dir := t.TempDir()
 
 		writeTestHarness(t, dir, "base.yaml", `

From 728bffb10cb10702361068da7663a6f36e20c66e Mon Sep 17 00:00:00 2001
From: fullsend-fix <278716306+fullsend-ai-coder[bot]@users.noreply.github.com>
Date: Mon, 31 Aug 2026 15:39:44 +0000
Subject: [PATCH 3/3] fix(harness): update mergeBaseIntoChild doc comment for
 forge clearing 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
---
 internal/harness/compose.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/internal/harness/compose.go b/internal/harness/compose.go
index b7002c3cf..4bdbff968 100644
--- a/internal/harness/compose.go
+++ b/internal/harness/compose.go
@@ -541,7 +541,8 @@ func matchingAllowedPrefix(rawURL string, allowlist []string) string {
 //   - Maps (runner_env): base merged with child; child keys win
 //   - Pointer structs (validation_loop, security): child replaces if non-nil
 //   - host_files: concatenated with last-writer-wins dedup by Dest
-//   - forge: key-by-key merge; per-platform uses same rules
+//   - forge: key-by-key merge; per-platform uses same rules;
+//     inherited platforms have fields cleared where child set explicit top-level values (#6798)
 //   - allowed_remote_resources: NOT merged (security; child must declare its own)
 func mergeBaseIntoChild(base, child *Harness) {
 	// Snapshot child's explicit top-level fields BEFORE any merge so we