Skip to content

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

Merged
ralphbean merged 1 commit into
mainfrom
migrate-triage-env-schema
Jun 30, 2026
Merged

refactor(harness): migrate triage to env.runner/env.sandbox (ADR 0055)#2763
ralphbean merged 1 commit into
mainfrom
migrate-triage-env-schema

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

  • Replace runner_env with env.runner (top-level and forge.github)
  • Move triage.env passthroughs to forge.github.env.sandbox, delete the .env file
  • Update scaffold and integration tests for new env schema

Phase 2 of ADR 0055 (#2582).

Test plan

  • go test ./internal/harness/ passes
  • go test ./internal/scaffold/ passes
  • Trigger a triage agent run and verify env vars reach both runner and sandbox

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Migrate triage harness env to env.runner/env.sandbox (ADR 0055)

⚙️ Configuration changes 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Migrate triage harness YAML from runner_env to env.runner (top-level and forge.github).
• Move triage passthrough vars into forge.github.env.sandbox and drop the .env host file.
• Update scaffold/harness tests to validate both legacy RunnerEnv and the new Env schema.
Diagram

graph TD
  A["triage.yaml (scaffold)"] --> B["harness.LoadWithOpts"] --> C["ResolveForge"]
  C --> D["Env.Runner"] --> E["Runner process"]
  C --> S["Env.Sandbox"] --> X["Sandbox container"]
  T["Scaffold/harness tests"] --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep host_files .env passthrough (env/triage.env)
  • ➕ Avoids introducing sandbox-specific env structure in templates
  • ➕ Keeps all passthrough env handling in the existing host_files mechanism
  • ➖ Extra file and host_files wiring to maintain
  • ➖ Relies on file expansion behavior rather than explicit env targeting
  • ➖ Harder to reason about runner vs sandbox scoping (especially for secrets)
2. Implicitly mirror env.runner into env.sandbox
  • ➕ Removes duplication for variables needed in both contexts
  • ➕ Simplifies template authoring for common cases
  • ➖ Blurs security boundary between runner and sandbox env
  • ➖ Surprising/implicit behavior that may leak vars unintentionally
  • ➖ Makes it harder to audit which vars are intended for the sandbox

Recommendation: Prefer the PR’s approach: explicitly modeling runner vs sandbox variables via env.runner and forge.github.env.sandbox aligns with ADR 0055 and makes env scoping auditable. The remaining risk is runtime correctness (vars must actually reach both targets), so the follow-up/verification should focus on an end-to-end triage run confirming sandbox injection for env.sandbox while keeping backward compatibility for legacy runner_env during the transition.

Files changed (3) +49 / -20

Tests (2) +39 / -12
scaffold_integration_test.goUpdate harness scaffold integration assertions for Env.Runner/Env.Sandbox +24/-8

Update harness scaffold integration assertions for Env.Runner/Env.Sandbox

• Adjusts integration tests to assert merged environment variables are present under h.Env.Runner (top-level + forge.github). Adds explicit checks for forge.github sandbox variables via h.Env.Sandbox and relaxes some assertions to accept either legacy RunnerEnv or the new schema during migration.

internal/harness/scaffold_integration_test.go

scaffold_test.goAlign scaffold tests with triage env schema migration and file removal +15/-4

Align scaffold tests with triage env schema migration and file removal

• Removes the expectation that env/triage.env exists in the embedded scaffold. Updates env merge assertions to validate merged keys across either legacy RunnerEnv or new Env.Runner by building a combined map for assertions.

internal/scaffold/scaffold_test.go

Other (1) +10 / -8
triage.yamlMigrate triage harness template to env.runner and forge.github env.runner/sandbox +10/-8

Migrate triage harness template to env.runner and forge.github env.runner/sandbox

• Replaces top-level runner_env with env.runner for schema compliance. Moves forge.github environment configuration under forge.github.env with explicit runner and sandbox maps, and removes the host_files reference to the deleted env/triage.env passthrough file.

internal/scaffold/fullsend-repo/harness/triage.yaml

@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://6a771dd5-site.fullsend-ai.workers.dev

Commit: 3ce10f4d06788dd4b9a1799688e6f1197428842a

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 6:12 PM UTC · Completed 6:25 PM UTC
Commit: 3ce10f4 · View workflow run →

@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 58 rules

Grey Divider


Action required

1. Missing sandbox vars locally 🐞 Bug ≡ Correctness
Description
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.
Code

internal/scaffold/fullsend-repo/harness/triage.yaml[R41-47]

+    env:
+      runner:
+        GITHUB_ISSUE_URL: ${GITHUB_ISSUE_URL}
+        GH_TOKEN: ${GH_TOKEN}
+      sandbox:
+        GITHUB_ISSUE_URL: "${GITHUB_ISSUE_URL}"
+        GH_TOKEN: "${GH_TOKEN}"
Relevance

⭐⭐ Medium

Team cares about local-run breakages (accepted local guard in PR #2456), but no prior evidence on
forge-skip sandbox env.

PR-#2456
PR-#2142
PR-#2260

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The harness only applies forge.github.env.sandbox when forge resolution runs; outside CI, forge
auto-detection returns empty and ResolveForge is a no-op, so Env.Sandbox stays empty. The
sandbox bootstrap does not automatically inherit the host environment (runtime exports are minimal),
and the triage agent requires GITHUB_ISSUE_URL inside the sandbox to call gh issue view.

internal/scaffold/fullsend-repo/harness/triage.yaml[31-47]
internal/cli/run.go[2054-2071]
internal/harness/forge.go[88-103]
internal/runtime/claude.go[30-32]
internal/cli/run.go[1343-1393]
internal/scaffold/fullsend-repo/agents/triage.md[12-20]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### 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



Remediation recommended

2. Stale triage.env references 🐞 Bug ⚙ Maintainability
Description
The scaffold deletes env/triage.env and stops delivering it via host_files, but docs/ADRs/specs
still reference env/triage.env and older runner_env-based examples. This documentation drift will
mislead users during the migration to env.runner/env.sandbox.
Code

internal/scaffold/fullsend-repo/env/triage.env[L1-2]

-export GITHUB_ISSUE_URL="${GITHUB_ISSUE_URL}"
-export GH_TOKEN=${GH_TOKEN}
Relevance

⭐⭐⭐ High

Docs drift fixes are commonly accepted; env-delivery ADR/doc alignment explicitly accepted in PR
#2582.

PR-#2582
PR-#1039
PR-#2244

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The scaffold no longer contains env/triage.env (test fixture list no longer expects it), and
triage harness now injects vars via env blocks; however, ADR/spec docs still describe
env/triage.env being copied into .env.d via host_files.

internal/scaffold/scaffold_test.go[52-67]
internal/scaffold/fullsend-repo/harness/triage.yaml[11-47]
docs/ADRs/0045-forge-portable-harness-schema.md[301-332]
docs/superpowers/specs/2026-04-17-installer-agent-content-design.md[41-60]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`env/triage.env` was removed from the scaffold, but multiple documentation sources still describe it as part of the scaffold layout and show harness examples that deliver it via `host_files`.

### Issue Context
This PR migrates triage to `env.runner` / `env.sandbox` and removes `.env.d/triage.env` delivery. Any docs that still reference `env/triage.env` (or imply it is required) are now incorrect.

### Fix Focus Areas
- docs/ADRs/0045-forge-portable-harness-schema.md[301-332]
- docs/superpowers/specs/2026-04-17-installer-agent-content-design.md[41-60]
- internal/scaffold/fullsend-repo/harness/triage.yaml[11-47]

### Implementation guidance
- Update the ADR/spec examples to remove `env/triage.env` and show the new `env.runner`/`env.sandbox` schema.
- If keeping forge-specific env in triage.yaml, ensure docs reflect that local runs may require `--forge github` (or describe the chosen fallback approach).

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


Grey Divider

Qodo Logo

Comment on lines +41 to +47
env:
runner:
GITHUB_ISSUE_URL: ${GITHUB_ISSUE_URL}
GH_TOKEN: ${GH_TOKEN}
sandbox:
GITHUB_ISSUE_URL: "${GITHUB_ISSUE_URL}"
GH_TOKEN: "${GH_TOKEN}"

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

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Low

  • [scope-drift] internal/scaffold/fullsend-repo/harness/triage.yaml — ADR 0055 Phase 2 specifies updating all scaffold harnesses, but this PR only migrates triage.yaml. Other scaffold harnesses (code.yaml, fix.yaml, review.yaml, prioritize.yaml, retro.yaml) still use runner_env. The inconsistency is intentional and temporary, but tracking issues for the remaining migrations would help ensure completion.
    Remediation: Update the PR description to clarify this is a partial Phase 2 migration with follow-up PRs planned for remaining harnesses, and add tracking issues for the remaining migrations.

  • [design-coherence] internal/harness/scaffold_integration_test.go:137 — The combined env map pattern (merging legacy RunnerEnv and new Env.Runner) is duplicated across scaffold_integration_test.go and scaffold_test.go. This transition-period logic will need cleanup after Phase 3.
    Remediation: Add a TODO comment referencing ADR 0055 Phase 3 removal timeline, or extract the merge logic into a shared test helper function.

  • [code-duplication] internal/harness/scaffold_integration_test.go:341 — The combined map construction block is duplicated verbatim in scaffold_test.go (~line 703). See also: [design-coherence] finding at this location.
    Remediation: Extract a helper function like combinedRunnerEnv(h *Harness) map[string]string to reduce duplication.

  • [test-comment-style] internal/harness/scaffold_integration_test.go:115 — Test function doc comment references "runner_env merged" using old terminology, though the diff does not modify this line.

  • [test-assertion-message-style] internal/harness/scaffold_integration_test.go:134 — Assertion message "RunnerEnv or Env.Runner should be non-empty after merge" references both old and new field names. The message accurately describes the dual-path check but could be clearer.


Labels: PR migrates triage harness YAML schema from runner_env to env.runner/env.sandbox, touching harness config and tests.

@ralphbean ralphbean added this pull request to the merge queue Jun 30, 2026
Merged via the queue into main with commit 8d5bd72 Jun 30, 2026
17 checks passed
@ralphbean ralphbean deleted the migrate-triage-env-schema branch June 30, 2026 11:55
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 30, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 11:59 AM UTC · Completed 12:09 PM UTC
Commit: 3ce10f4 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2763 — migrate triage to env.runner/env.sandbox

Timeline:

  1. PR opened by ralphbean (2026-06-29 18:10 UTC) — single-commit refactor moving triage harness env vars from runner_env/host_files to env.runner/env.sandbox (ADR 0055 Phase 2).
  2. Review agent dispatched (run 28392962441) — completed analysis with APPROVE verdict and 5 low-severity findings, but the run failed due to a 422 error when posting inline comments referencing lines outside the PR diff.
  3. Qodo bot posted 2 bug findings: (a) sandbox vars missing in local mode without --forge github, (b) stale doc references to triage.env/runner_env.
  4. Human reviewer (rh-hemartin) approved (2026-06-30 09:06 UTC), PR merged (11:55 UTC).

Observations:

Area Finding Existing coverage
422 on inline comments Review agent analysis succeeded but post-review failed submitting inline comments outside diff hunks. This is at least the 5th occurrence of this class of bug. Tracked by #2569 (regression) and #1067 (original).
Review quality gap Qodo caught a functional bug (sandbox vars unreachable in local execution path) that the review agent missed entirely. The review agent found only cosmetic issues. Covered by #1525 (cross-file impact gap) and #2108 (sandbox env checking).
Documentation drift Stale references to deleted triage.env and old runner_env schema in docs/ADRs. Covered by #2676 (incomplete migration detection) and #2383 (config→doc invalidation).

No new proposals filed — all improvement opportunities are already tracked by existing open issues. The 422 regression (#2569) is the most impactful; this PR provides another concrete reproduction case.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants