From 3ce10f4d06788dd4b9a1799688e6f1197428842a Mon Sep 17 00:00:00 2001 From: Ralph Bean Date: Mon, 29 Jun 2026 14:06:58 -0400 Subject: [PATCH] refactor(harness): migrate triage to env.runner/env.sandbox (ADR 0055) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace runner_env with env.runner and forge.github.runner_env with forge.github.env.runner in the triage harness template. Migrate the passthrough vars from triage.env (host_files) to forge.github.env.sandbox, eliminating the need for the .env host file. - triage.yaml: runner_env → env.runner, forge runner_env → forge env.runner/sandbox - Delete env/triage.env (pure passthrough, now in env.sandbox) - Update scaffold and harness tests to check both legacy RunnerEnv and new Env.Runner Assisted-by: Claude Opus 4.6 Signed-off-by: Ralph Bean --- internal/harness/scaffold_integration_test.go | 32 ++++++++++++++----- .../scaffold/fullsend-repo/env/triage.env | 2 -- .../fullsend-repo/harness/triage.yaml | 18 ++++++----- internal/scaffold/scaffold_test.go | 19 ++++++++--- 4 files changed, 49 insertions(+), 22 deletions(-) delete mode 100644 internal/scaffold/fullsend-repo/env/triage.env diff --git a/internal/harness/scaffold_integration_test.go b/internal/harness/scaffold_integration_test.go index 795b08df16..1a3bbe0ccb 100644 --- a/internal/harness/scaffold_integration_test.go +++ b/internal/harness/scaffold_integration_test.go @@ -61,13 +61,17 @@ slug: test-triage assert.NotEmpty(t, h.PreScript, "PreScript should be set after forge resolution") assert.NotEmpty(t, h.PostScript, "PostScript should be set after forge resolution") - // RunnerEnv contains both top-level keys and forge.github keys after merge. - assert.Contains(t, h.RunnerEnv, "FULLSEND_OUTPUT_SCHEMA", "should have top-level runner_env key") - assert.Contains(t, h.RunnerEnv, "GH_TOKEN", "should have forge.github runner_env key") - assert.Contains(t, h.RunnerEnv, "GITHUB_ISSUE_URL", "should have forge.github runner_env key") + // Env.Runner contains both top-level keys and forge.github keys after merge. + assert.Contains(t, h.Env.Runner, "FULLSEND_OUTPUT_SCHEMA", "should have top-level env.runner key") + assert.Contains(t, h.Env.Runner, "GH_TOKEN", "should have forge.github env.runner key") + assert.Contains(t, h.Env.Runner, "GITHUB_ISSUE_URL", "should have forge.github env.runner key") + + // Env.Sandbox contains forge.github sandbox vars. + assert.Contains(t, h.Env.Sandbox, "GH_TOKEN", "should have forge.github env.sandbox key") + assert.Contains(t, h.Env.Sandbox, "GITHUB_ISSUE_URL", "should have forge.github env.sandbox key") // Skills includes base top-level skills (forge skills are concatenated by ResolveForge, - // but the triage template has no forge-specific skills — only runner_env and scripts). + // but the triage template has no forge-specific skills — only env and scripts). assert.Contains(t, h.Skills, "skills/issue-labels") // Forge map is nil (consumed by ResolveForge). @@ -130,7 +134,8 @@ func TestLoadWithOpts_ScaffoldTemplatesForgeResolution(t *testing.T) { assert.NotEmpty(t, h.PreScript, "PreScript should be set after forge resolution") assert.NotEmpty(t, h.PostScript, "PostScript should be set after forge resolution") - assert.NotEmpty(t, h.RunnerEnv, "RunnerEnv should be non-empty after merge") + hasRunnerEnv := len(h.RunnerEnv) > 0 || (h.Env != nil && len(h.Env.Runner) > 0) + assert.True(t, hasRunnerEnv, "RunnerEnv or Env.Runner should be non-empty after merge") assert.Nil(t, h.Forge, "Forge should be nil after resolution") assert.NotEmpty(t, h.Role, "Role should be set in scaffold template") assert.NotEmpty(t, h.Slug, "Slug should be set in scaffold template") @@ -333,11 +338,22 @@ func TestResolveForge_ScaffoldRunnerEnvMerge(t *testing.T) { h, loadErr := LoadWithOpts(path, LoadOpts{ForgePlatform: "github"}) require.NoError(t, loadErr) + // Build a combined env map from both legacy RunnerEnv and new Env.Runner. + combined := make(map[string]string) + for k, v := range h.RunnerEnv { + combined[k] = v + } + if h.Env != nil { + for k, v := range h.Env.Runner { + combined[k] = v + } + } + for _, key := range tt.topLevelKeys { - assert.Contains(t, h.RunnerEnv, key, "merged RunnerEnv should contain top-level key %s", key) + assert.Contains(t, combined, key, "merged env should contain top-level key %s", key) } for _, key := range tt.forgeGithubKeys { - assert.Contains(t, h.RunnerEnv, key, "merged RunnerEnv should contain forge.github key %s", key) + assert.Contains(t, combined, key, "merged env should contain forge.github key %s", key) } }) } diff --git a/internal/scaffold/fullsend-repo/env/triage.env b/internal/scaffold/fullsend-repo/env/triage.env deleted file mode 100644 index e79c7e667e..0000000000 --- a/internal/scaffold/fullsend-repo/env/triage.env +++ /dev/null @@ -1,2 +0,0 @@ -export GITHUB_ISSUE_URL="${GITHUB_ISSUE_URL}" -export GH_TOKEN=${GH_TOKEN} diff --git a/internal/scaffold/fullsend-repo/harness/triage.yaml b/internal/scaffold/fullsend-repo/harness/triage.yaml index 284d7d5f36..5618c05901 100644 --- a/internal/scaffold/fullsend-repo/harness/triage.yaml +++ b/internal/scaffold/fullsend-repo/harness/triage.yaml @@ -17,9 +17,6 @@ host_files: - src: ${GCP_OIDC_TOKEN_FILE} dest: /sandbox/workspace/.gcp-oidc-token optional: true - - src: env/triage.env - dest: /sandbox/workspace/.env.d/triage.env - expand: true skills: - skills/issue-labels @@ -31,8 +28,9 @@ validation_loop: script: scripts/validate-output-schema.sh max_iterations: 2 -runner_env: - FULLSEND_OUTPUT_SCHEMA: ${FULLSEND_DIR}/schemas/triage-result.schema.json +env: + runner: + FULLSEND_OUTPUT_SCHEMA: ${FULLSEND_DIR}/schemas/triage-result.schema.json timeout_minutes: 10 @@ -40,6 +38,10 @@ forge: github: pre_script: scripts/pre-triage.sh post_script: scripts/post-triage.sh - runner_env: - GITHUB_ISSUE_URL: ${GITHUB_ISSUE_URL} - GH_TOKEN: ${GH_TOKEN} + env: + runner: + GITHUB_ISSUE_URL: ${GITHUB_ISSUE_URL} + GH_TOKEN: ${GH_TOKEN} + sandbox: + GITHUB_ISSUE_URL: "${GITHUB_ISSUE_URL}" + GH_TOKEN: "${GH_TOKEN}" diff --git a/internal/scaffold/scaffold_test.go b/internal/scaffold/scaffold_test.go index 8f0902c5c1..490a779d40 100644 --- a/internal/scaffold/scaffold_test.go +++ b/internal/scaffold/scaffold_test.go @@ -61,7 +61,6 @@ func TestFullsendRepoFilesExist(t *testing.T) { "agents/triage.md", "agents/code.md", "env/gcp-vertex.env", - "env/triage.env", "env/code-agent.env", "harness/triage.yaml", "harness/code.yaml", @@ -632,7 +631,8 @@ func TestHarnessesLoadAndValidate(t *testing.T) { assert.Nil(t, h.Forge, "Forge should be nil after resolution") assert.NotEmpty(t, h.PreScript, "PreScript should be set after forge resolution") assert.NotEmpty(t, h.PostScript, "PostScript should be set after forge resolution") - assert.NotEmpty(t, h.RunnerEnv, "RunnerEnv should be non-empty after merge") + hasRunnerEnv := len(h.RunnerEnv) > 0 || (h.Env != nil && len(h.Env.Runner) > 0) + assert.True(t, hasRunnerEnv, "RunnerEnv or Env.Runner should be non-empty after merge") resolveErr := h.ResolveRelativeTo(dir) require.NoError(t, resolveErr, "ResolveRelativeTo should succeed") @@ -700,11 +700,22 @@ func TestHarnessForgeRunnerEnvMerge(t *testing.T) { h, loadErr := harness.LoadWithOpts(harnessPath, harness.LoadOpts{ForgePlatform: "github"}) require.NoError(t, loadErr) + // Build a combined env map from both legacy RunnerEnv and new Env.Runner. + combined := make(map[string]string) + for k, v := range h.RunnerEnv { + combined[k] = v + } + if h.Env != nil { + for k, v := range h.Env.Runner { + combined[k] = v + } + } + for _, key := range tt.topLevelKeys { - assert.Contains(t, h.RunnerEnv, key, "merged RunnerEnv should contain top-level key %s", key) + assert.Contains(t, combined, key, "merged env should contain top-level key %s", key) } for _, key := range tt.forgeGithubKeys { - assert.Contains(t, h.RunnerEnv, key, "merged RunnerEnv should contain forge.github key %s", key) + assert.Contains(t, combined, key, "merged env should contain forge.github key %s", key) } }) }