Skip to content

refactor(harness): migrate triage to env.runner/env.sandbox (ADR 0055) #2763

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ralphbean merged 1 commit into from
Jun 30, 2026
Merged

refactor(harness): migrate triage to env.runner/env.sandbox (ADR 0055) #2763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions internal/harness/scaffold_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
}
})
}
Expand Down
2 changes: 0 additions & 2 deletions internal/scaffold/fullsend-repo/env/triage.env
View file
Open in desktop

This file was deleted.

18 changes: 10 additions & 8 deletions internal/scaffold/fullsend-repo/harness/triage.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -31,15 +28,20 @@ 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

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}"
Comment on lines +41 to +47

@qodo-code-review qodo-code-review Bot Jun 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Missing sandbox vars locally 🐞 Bug ≡ Correctness

In triage.yaml, GITHUB_ISSUE_URL and GH_TOKEN are now only provided via forge.github.env.sandbox, so
when fullsend run triage is executed outside CI without --forge github, ResolveForge is skipped
and the sandbox will not receive these required variables. The triage agent inside the sandbox
relies on GITHUB_ISSUE_URL (and GH auth) for gh issue view, so the run will fail despite the host
env-file being loaded.
Agent Prompt 
### Issue description
`internal/scaffold/fullsend-repo/harness/triage.yaml` now injects `GITHUB_ISSUE_URL` and `GH_TOKEN` into the sandbox only via `forge.github.env.sandbox`. When `fullsend run` is executed outside CI and the user does not pass `--forge github`, `detectForgePlatform()` returns an empty string and `ResolveForge()` becomes a no-op, so these sandbox vars are never merged and the triage agent cannot run.

### Issue Context
- Outside CI, `detectForgePlatform()` returns `""` unless `--forge` is explicitly set.
- The sandbox environment only gets runtime exports (e.g., `CLAUDE_CONFIG_DIR`), `.env.d` sourced files, and `env.sandbox` exports.
- The triage agent prompt uses `GITHUB_ISSUE_URL` inside the sandbox.

### Fix Focus Areas
- internal/scaffold/fullsend-repo/harness/triage.yaml[31-47]

### Implementation guidance
Choose one (prefer #1 for backwards-compatible local runs):
1) Add top-level `env.sandbox` passthroughs for `GITHUB_ISSUE_URL` and `GH_TOKEN` (and optionally top-level `env.runner` for the same keys) so the sandbox receives them even when forge is not resolved.
2) Alternatively, if triage is intended to be forge-required, update the local-run docs to require `--forge github` (but this keeps the harness fragile for ad-hoc local runs).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

19 changes: 15 additions & 4 deletions internal/scaffold/scaffold_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
}
})
}
Expand Down
Loading