Skip to content

chore: migrate remaining runner_env keys to env.runner - #631

Merged
ralphbean merged 2 commits into
mainfrom
chore/env-runner-syntax
Aug 3, 2026
Merged

chore: migrate remaining runner_env keys to env.runner#631
ralphbean merged 2 commits into
mainfrom
chore/env-runner-syntax

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

  • harness/code.yaml, review.yaml, and retro.yaml were the last three harness files still using the legacy top-level runner_env key.
  • fix.yaml, triage.yaml, prioritize.yaml, and scribe.yaml already use the env.runner / env.sandbox nesting, so this brings the rest in line.
  • Vars nested under forge.github (tokens, REPO_FULL_NAME, issue/PR numbers, GITHUB_PR_URL, ORIGINATING_URL) stayed within the forge selector since they're GitHub-specific; code.yaml's generic CODE_ALLOWED_TARGET_BRANCHES moved to the root env.runner.
  • Updated a few doc/comment references (docs/code.md, env/code.env, eval/scripts/run-fullsend.sh) that named the old key.

Test plan

  • python3 -c "import yaml; yaml.safe_load(open(...))" on each edited harness YAML file
  • grep -r runner_env returns no matches

harness/code.yaml, review.yaml, and retro.yaml still used the legacy
top-level runner_env key. fix.yaml, triage.yaml, prioritize.yaml, and
scribe.yaml had already moved to the nicer env.runner / env.sandbox
nesting. Bring the remaining three harness files in line, and update
the doc/comment references that named the old key.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@ralphbean
ralphbean requested a review from a team as a code owner August 3, 2026 18:29
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

chore: migrate remaining runner_env to env.runner in harness YAMLs

⚙️ Configuration changes 📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Migrate code/review/retro harness configs from legacy runner_env to env.runner.
• Preserve GitHub-specific variables under forge.github while aligning structure across harnesses.
• Update docs/comments and env templates to reference env.runner instead of runner_env.
Diagram

graph TD
  A["Harness YAMLs (code/review/retro)"] --> B(["Fullsend runner"]) --> C(["Sandbox"])
  A --> D["Forge GitHub config"] --> B
  B --> E(["Runner env (env.runner)"])
  F["eval/scripts/run-fullsend.sh"] --> B
  G["Docs & env templates"] --> A
Loading
High-Level Assessment

The direct migration to env.runner is the right approach: it standardizes harness configuration, matches existing harnesses already using env.runner/env.sandbox, and reduces cognitive overhead. A compatibility shim (supporting both runner_env and env.runner) could be considered if external consumers depended on runner_env, but this PR only updates in-repo harness files and references, so a clean cutover is preferable.

Files changed (6) +23 / -19

Documentation (3) +3 / -3
code.mdUpdate CODE_ALLOWED_TARGET_BRANCHES doc to reference env.runner +1/-1

Update CODE_ALLOWED_TARGET_BRANCHES doc to reference env.runner

• Adjusts documentation to say CODE_ALLOWED_TARGET_BRANCHES is set via env.runner in harness/code.yaml instead of runner_env.

docs/code.md

code.envFix comment referencing PUSH_TOKEN injection key +1/-1

Fix comment referencing PUSH_TOKEN injection key

• Updates a comment to refer to the write-enabled PUSH_TOKEN as coming from env.runner rather than runner_env, reinforcing that it never enters the sandbox.

env/code.env

run-fullsend.shUpdate eval script comment to env.runner terminology +1/-1

Update eval script comment to env.runner terminology

• Renames a comment describing harness environment variable references from runner_env to env.runner (no functional behavior change).

eval/scripts/run-fullsend.sh

Other (3) +20 / -16
code.yamlMigrate runner-only vars to env.runner for code harness +9/-7

Migrate runner-only vars to env.runner for code harness

• Moves CODE_ALLOWED_TARGET_BRANCHES from top-level runner_env into env.runner. Also migrates the forge.github runner_env block (PUSH_TOKEN, repo metadata, issue number) into forge.github.env.runner.

harness/code.yaml

retro.yamlMigrate GitHub runner_env to forge.github.env.runner +5/-4

Migrate GitHub runner_env to forge.github.env.runner

• Replaces forge.github.runner_env with forge.github.env.runner for ORIGINATING_URL, REPO_FULL_NAME, and GH_TOKEN.

harness/retro.yaml

review.yamlMigrate GitHub runner_env to forge.github.env.runner +6/-5

Migrate GitHub runner_env to forge.github.env.runner

• Replaces forge.github.runner_env with forge.github.env.runner for REVIEW_TOKEN, REPO_FULL_NAME, PR_NUMBER, and GITHUB_PR_URL.

harness/review.yaml

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:30 PM UTC · Completed 6:44 PM UTC
Commit: 02dc8bb · View workflow run →

@qodo-code-review

qodo-code-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Context used
✅ Compliance rules (platform): 55 rules
✅ Skills: 4 invoked
  code-review
  code-implementation
  pr-review
  docs-review

Grey Divider


Action required

1. Protected paths modified without issue ✗ Dismissed 📜 Skill insight § Compliance
Description
This PR modifies protected governance/infrastructure paths (harness/ and eval/scripts/), which
must never be auto-approved and require explicit authorization (linked issue/ADR) for the change
set. The PR description explains intent, but does not include a linked authorization artifact.
Code

harness/code.yaml[R51-53]

+env:
+  runner:
+    CODE_ALLOWED_TARGET_BRANCHES: "${CODE_ALLOWED_TARGET_BRANCHES}"
Relevance

●● Moderate

Protected-path rigor exists (PR #303), and justification links were requested/partly accepted before
(PR #184), but not consistent.

PR-#303
PR-#184

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 1538392 requires raising a finding whenever protected paths are modified, and
requires explicit authorization (linked issue/ADR) for such changes. The diff shows edits in
harness/ YAML files and eval/scripts/run-fullsend.sh, both listed as protected paths in the
rule.

harness/code.yaml[49-66]
harness/review.yaml[44-53]
harness/retro.yaml[40-48]
eval/scripts/run-fullsend.sh[148-166]
Skill: pr-review

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

## Issue description
The PR modifies protected paths (`harness/`, `eval/scripts/`) but lacks an explicit linked authorization artifact (issue/ADR). This violates the requirement that protected governance/infrastructure changes must not be auto-approved and must be explicitly authorized.

## Issue Context
The PR is a mechanical migration from `runner_env` to `env.runner`, but it still changes protected infrastructure/configuration surfaces.

## Fix Focus Areas
- harness/code.yaml[49-66]
- harness/review.yaml[44-53]
- harness/retro.yaml[40-48]
- eval/scripts/run-fullsend.sh[148-166]

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



Informational

2. Comment-only change in run-fullsend.sh 📘 Rule violation ⚙ Maintainability
Description
A comment was modified without any functional code change on the same or immediately adjacent lines
in the corresponding diff hunk, violating the requirement to avoid standalone comment-only edits
unless part of a refactor of the surrounding block.
Code

eval/scripts/run-fullsend.sh[156]

+  # Code/fix harness env.runner refs — mint normally sets these; eval skips mint.
Relevance

● Weak

Repo often merges standalone comment/doc wording updates (e.g., PR #171), so this comment-only edit
likely tolerated.

PR-#171

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1540690 requires that comment edits be paired with functional code changes in the same
hunk/block unless the block is being structurally refactored. The cited diff context shows only the
comment text changing (e.g., updating runner_env to env.runner) while the surrounding
logic/exports remain unchanged, making this a comment-only edit under the rule.

Rule 1540690: Do not change comments on lines without functional code changes in a diff
eval/scripts/run-fullsend.sh[152-166]
env/code.env[4-10]

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

## Issue description
A comment line was updated without any accompanying functional code change in the same or adjacent lines in the diff hunk, which is not allowed unless it is part of a structural refactor of the surrounding block.

## Issue Context
The change updates wording from `runner_env` -> `env.runner`, but the surrounding logic/code (including exported variables) is otherwise unchanged in this PR.

## Fix Focus Areas
- eval/scripts/run-fullsend.sh[152-166]
- env/code.env[4-10]

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


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread harness/code.yaml
Comment thread harness/code.yaml
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review

Findings

High

  • [protected-path] harness/code.yaml, harness/retro.yaml — PR modifies files under the protected harness/ path without a linked issue. Human approval is always required for changes to governance and infrastructure files.
    Remediation: Link an issue authorizing the harness file changes, or obtain explicit human approval.

Low

  • [breaking-schema-change] harness/code.yaml:51 — Migration from runner_env to env.runner changes the harness YAML schema contract. The downstream consumer (fullsend-ai/fullsend) already supports this schema since fix.yaml, triage.yaml, prioritize.yaml, scribe.yaml, and review.yaml all use env.runner. Two separate runner_env blocks are migrated: one at top level (line 51) and one under forge.github (line 61).

  • [breaking-schema-change] harness/retro.yaml:44 — Migration from runner_env to env.runner changes the harness YAML schema contract under forge.github. The downstream consumer already supports this schema since five other harness files use env.runner.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run

Review

Findings

High

  • [protected-path] harness/code.yaml, harness/retro.yaml, harness/review.yaml — PR modifies files under the protected harness/ path without a linked issue. Human approval is always required for changes to governance and infrastructure files. The PR lacks a linked issue to justify the protected-path modifications.
    Remediation: Link an issue authorizing the harness file changes, or obtain explicit human approval.

Low

  • [missing-authorization] Non-trivial structural change (42 lines, YAML schema migration) lacks a linked issue. This is a mechanical migration completing a pattern already established in fix.yaml, triage.yaml, prioritize.yaml, and scribe.yaml.

  • [breaking-schema-change] harness/code.yaml:51 — Migration from runner_env to env.runner changes the harness YAML schema contract. The downstream consumer (fullsend-ai/fullsend) likely already supports this schema since fix.yaml, triage.yaml, prioritize.yaml, and scribe.yaml all use env.runner.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🤖 Finished Fix · ✅ Success · Started 6:45 PM UTC · Completed 6:49 PM UTC
Commit: 02dc8bb · View workflow run →

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

🔧 Fix agent — iteration 1 (bot-triggered)

All 3 review findings are governance/process concerns (missing linked issue for protected-path modifications) that require human action, not code fixes. The fix agent cannot create or link issues, nor grant human approval. Protected-path enforcement in post-review.sh already prevents auto-approval for these paths — a human reviewer must approve. No code changes are needed.

Disagreed (3):

  1. [protected-path] harness/ files modified without linked issue: This is a governance/process concern requiring human action (linking an issue or obtaining explicit human approval), not a code defect the fix agent can address. Protected-path enforcement already lives in post-review.sh — the review agent cannot approve PRs that touch harness/ paths, so a human reviewer must approve. This mechanism is working as designed. The fix agent cannot create or link issues.
  2. [missing-authorization] Structural change lacks linked issue: This finding is the same governance concern as the protected-path finding — the PR needs a linked authorization artifact. Creating or linking issues to PRs is a human action outside the fix agent's scope. The reviewer notes this is a mechanical migration completing a pattern already established in fix.yaml, triage.yaml, prioritize.yaml, and scribe.yaml.
  3. [breaking-schema-change] runner_env to env.runner schema migration: The reviewer acknowledges downstream (fullsend-ai/fullsend) likely already supports the env.runner schema since fix.yaml, triage.yaml, prioritize.yaml, and scribe.yaml all use it. This is informational context about the change's impact, not a code defect requiring a fix. The migration is completing an established pattern, not introducing a novel schema.

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Clean — no new findings from review squad. Remaining threads are addressed (schema migration already supported downstream) or explicitly deferred by the author with reasoning (ADR 080/081 compliance follow-up). CI green.

@ralphbean
ralphbean enabled auto-merge August 3, 2026 19:12
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:14 PM UTC · Completed 7:30 PM UTC
Commit: 4f665cc · View workflow run →

@ralphbean
ralphbean added this pull request to the merge queue Aug 3, 2026

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

Comment thread harness/code.yaml
# These are expanded from the runner environment and NEVER enter the sandbox.
runner_env:
CODE_ALLOWED_TARGET_BRANCHES: "${CODE_ALLOWED_TARGET_BRANCHES}"
env:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] breaking-schema-change

Migration from runner_env to env.runner changes the harness YAML schema contract. The downstream consumer (fullsend-ai/fullsend) already supports this schema since fix.yaml, triage.yaml, prioritize.yaml, scribe.yaml, and review.yaml all use env.runner. Two separate runner_env blocks are migrated: one at top level (line 51) and one under forge.github (line 61).

Comment thread harness/retro.yaml
ORIGINATING_URL: "${ORIGINATING_URL}"
REPO_FULL_NAME: "${REPO_FULL_NAME}"
GH_TOKEN: "${GH_TOKEN}"
env:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] breaking-schema-change

Migration from runner_env to env.runner changes the harness YAML schema contract under forge.github. The downstream consumer already supports this schema since five other harness files use env.runner.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🤖 Finished Fix · ✅ Success · Started 7:30 PM UTC · Completed 7:32 PM UTC
Commit: 4f665cc · View workflow run →

Merged via the queue into main with commit 5e98b13 Aug 3, 2026
16 checks passed
@ralphbean
ralphbean deleted the chore/env-runner-syntax branch August 3, 2026 19:32
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

🔧 Fix agent — iteration 1 (bot-triggered)

PR #631 is already merged. No fixes were applied because the PR is no longer open.

Disagreed (1):

  1. PR already merged: PR chore: migrate remaining runner_env keys to env.runner #631 is in MERGED state. No fixes can be applied to a merged PR. The review feedback is moot.

Tests: passed

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 7:34 PM UTC · Completed 7:45 PM UTC
Commit: 4f665cc · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #631 — chore: migrate remaining runner_env keys to env.runner

PR #631 was a straightforward 5-file chore PR by the repo maintainer (ralphbean) migrating three harness YAML files from the legacy runner_env key to env.runner and updating doc/comment references. 17 additions, 14 deletions.

Timeline

Time (UTC) Event Duration Outcome
18:29–18:44 First review (run 30841493456) 15 min 1 HIGH protected-path, 2 LOW breaking-schema-change. Verdict: CHANGES_REQUESTED
18:45–18:49 First fix (run 30842666698) 4.5 min Correctly disagreed with all 3 findings as governance/process concerns. No code changes.
19:11 Human approval (waynesun09) "Clean — no new findings from review squad."
19:12 Auto-merge enabled
19:13 Merge commit created (main → branch) No code changes, only merge reconciliation
19:14–19:30 Second review (run 30844865718) 16 min Re-raised same breaking-schema-change findings. Verdict: CHANGES_REQUESTED
19:27 PR entered merge queue
19:30–19:32 Second fix (run 30846063558) 2.5 min PR already merged; stopped immediately.
19:32 PR merged

Total wall-clock: ~63 minutes from PR open to merge.

What went well

  • Fix agent performed correctly in both cycles — disagreed with governance findings on the first run, detected the merged state and stopped cleanly on the second.
  • Human reviewer (waynesun09) provided an accurate, well-calibrated approval.
  • The PR was correct as submitted and merged without any rework.

Waste identified

The second review + fix cycle was entirely unnecessary (~18.5 min of Opus compute). The merge commit (merging main into the branch) contained no code changes but triggered a synchronize event that dispatched a full review. That review re-raised the same breaking-schema-change findings and produced a CHANGES_REQUESTED verdict on a PR that was already human-approved and entering the merge queue. The subsequent fix agent found the PR already merged.

Evidence for existing issues (no new proposals needed)

  • fullsend#1282 — Skip re-review on merge-only commits: the merge commit at 19:13 added zero code changes but triggered a 16-min Opus review. This is the third observed instance of this pattern.
  • agents#106 — Review re-raises findings after author declines scope: the author addressed the breaking-schema-change findings in PR comments (19:21–19:24), but the second review re-raised them at 19:30.
  • fullsend#2115 — COMMENT verdict for human-authored PRs with only medium/low findings: the first review used CHANGES_REQUESTED due to the HIGH protected-path finding, triggering the fix agent on what were purely governance concerns for a maintainer-authored chore PR.
  • agents#301 — Lightweight review for trivial/mechanical changes: this 17-line config migration following an established pattern received two full 6-dimension Opus reviews.
  • fullsend#3690 — Harness pre-script abort when PR is merged: the second fix agent launched despite the PR being in the merge queue, then found it merged and stopped after ~2.5 min.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants