fix(#349): use runner_env YAML key matching Go struct tags#502
fix(#349): use runner_env YAML key matching Go struct tags#502fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
Harness YAML files used nested env.runner for runner environment variables, but Harness.RunnerEnv and ForgeConfig.RunnerEnv are tagged yaml:"runner_env" and expect a flat runner_env key. The YAML parser silently ignored the env.runner sections, leaving RunnerEnv nil after unmarshalling. This caused REVIEW_TOKEN (and other forge-specific runner env vars like PUSH_TOKEN, GH_TOKEN) to never reach post-scripts via the harness config. Post-scripts fell back to inheriting vars from os.Environ() via the GHA step env, which breaks when the workflow identity matches the PR author (GitHub rejects self-reviews with 422). Changes all harness YAML files (scaffold defaults and customized overrides) from nested env.runner to flat runner_env matching the Go struct tags. Also removes env.sandbox sections which were equally silently ignored (no SandboxEnv field exists in Harness or ForgeConfig). Fixes code.yaml test expectations to match actual runner_env keys. Closes #349
E2E tests did not runE2E tests run automatically for org/repo members and collaborators on pull requests. For other contributors, a maintainer must add the See E2E testing guide for details. |
|
🤖 Finished Review · ✅ Success · Started 3:14 PM UTC · Completed 3:24 PM UTC |
Review — PR #502Verdict: Comment · 1 medium finding SummaryCorrect root-cause fix for #349. The harness YAML files used The The test correction for Findings1. Missed cleanup:
|
| Dimension | Result |
|---|---|
| Correctness | ✅ Core fix is correct; one missed file |
| Security | ✅ No privilege escalation; runner/sandbox boundary preserved |
| Intent & coherence | ✅ Scope matches issue #349 authorization |
| Style & conventions | ✅ Matches established code.yaml pattern |
| Documentation | ✅ Docs already reference runner_env — no staleness |
| Cross-repo contracts | ⏭ Skipped — no exported API changes |
Harness YAML files used nested env.runner for runner environment variables, but Harness.RunnerEnv and ForgeConfig.RunnerEnv are tagged yaml:"runner_env" and expect a flat runner_env key. The YAML parser silently ignored the env.runner sections, leaving RunnerEnv nil after unmarshalling.
This caused REVIEW_TOKEN (and other forge-specific runner env vars like PUSH_TOKEN, GH_TOKEN) to never reach post-scripts via the harness config. Post-scripts fell back to inheriting vars from os.Environ() via the GHA step env, which breaks when the workflow identity matches the PR author (GitHub rejects self-reviews with 422).
Changes all harness YAML files (scaffold defaults and customized overrides) from nested env.runner to flat runner_env matching the Go struct tags. Also removes env.sandbox sections which were equally silently ignored (no SandboxEnv field exists in Harness or ForgeConfig). Fixes code.yaml test expectations to match actual runner_env keys.
Closes #349
Post-script verification
agent/349-fix-harness-runner-env)b776cf6006a12016eaa21412dc65a3001100e801..HEAD)