Skip to content

fix(#5722): inject FULLSEND_OUTPUT_SCHEMA into post-script env - #5729

Merged
ggallen merged 1 commit into
mainfrom
agent/5722-inject-schema-post-env
Jul 29, 2026
Merged

fix(#5722): inject FULLSEND_OUTPUT_SCHEMA into post-script env#5729
ggallen merged 1 commit into
mainfrom
agent/5722-inject-schema-post-env

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Inject FULLSEND_OUTPUT_SCHEMA from h.ValidationLoop.Schema into the post-script environment so that post-scripts (e.g. post-fix.sh calling process-fix-result.py) can locate the JSON schema without constructing a relative path. After PR #5688 introduced directory-level script caching, the schemas/ directory is no longer co-located with scripts/ in the cache tree, breaking the relative-path fallback. This adds the env var at the post-script callsite using the same pattern as validationEnv().

Related Issue

Fixes #5722

Companion: fullsend-ai/agents#526 (updates process-fix-result.py to read the env var)

Changes

  • internal/cli/run.go: After childScriptEnv() returns, conditionally append FULLSEND_OUTPUT_SCHEMA from h.ValidationLoop.Schema to postCmd.Env (lines 992-997). Uses exec.Cmd last-value-wins semantics to override any stale value from the process environment.
  • internal/cli/run_test.go: Add three tests verifying the post-script schema injection: schema present when ValidationLoop.Schema is set, no append when schema is empty, no append when ValidationLoop is nil.

Testing

  • New unit tests pass (TestPostScriptEnv_IncludesOutputSchema, TestPostScriptEnv_NoSchemaAppendedWhenEmpty, TestPostScriptEnv_NoSchemaAppendedWhenNoValidationLoop)
  • All existing related tests pass (TestValidationEnv_*, TestPostScriptRepoEnv, TestChildScriptEnv_*, TestBootstrapEnv_*)
  • go vet ./internal/cli/... passes
  • go build ./internal/cli/... passes

Closes #5722

Post-script verification

  • Branch is not main/master (agent/5722-inject-schema-post-env)
  • Secret scan passed (gitleaks — 8ac8468aeabcc94dc3be41d911c7fa97508211db..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 29, 2026 15:23
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Jul 29, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:25 PM UTC · Completed 3:39 PM UTC
Commit: 1a70eb1 · View workflow run →

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

Looks good to me

Previous run

Review

Findings

Low

  • [test-adequacy] internal/cli/run_test.go:2163 — The three new tests replicate the inline append logic from run.go rather than exercising the actual production code path. The production logic is a 3-line conditional inside a deferred closure in runAgent, and the tests copy that conditional verbatim. The parallel validationEnv() function is already extracted as a named helper and tested directly — the same pattern should apply here.
    Remediation: Extract the post-script env construction into a named helper function (e.g. postScriptEnv) so tests can call the real function rather than duplicating the conditional logic.

  • [assertion-pattern] internal/cli/run_test.go:2193TestPostScriptEnv_NoSchemaAppendedWhenEmpty and TestPostScriptEnv_NoSchemaAppendedWhenNoValidationLoop use a length-comparison pattern (before := len(env); assert.Equal(t, before, len(env))) to verify no entry was appended. The existing TestValidationEnv_OmitsSchema* tests use an iteration+HasPrefix pattern instead, which is both more consistent with the codebase and slightly more robust.
    Remediation: Replace the length-comparison with the iteration pattern from existing tests: for _, e := range env { assert.False(t, strings.HasPrefix(e, "FULLSEND_OUTPUT_SCHEMA="), "message") }


Labels: PR modifies runner lifecycle code (internal/cli/run.go) fixing a bug in post-script environment injection

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added ready-for-merge All reviewers approved — ready to merge component/runner Agent runner behavior and lifecycle type/bug Confirmed defect in existing behavior Review effort 1/5 labels Jul 29, 2026
@ggallen
ggallen force-pushed the agent/5722-inject-schema-post-env branch from 1a70eb1 to def46ec Compare July 29, 2026 17:22
@ggallen

ggallen commented Jul 29, 2026

Copy link
Copy Markdown
Member

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 29, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:24 PM UTC · Completed 5:35 PM UTC
Commit: def46ec · View workflow run →

@ggallen
ggallen enabled auto-merge July 29, 2026 18:08
Post-scripts run via childScriptEnv(h.RunnerEnv, traceparent) which
merges os.Environ() with RunnerEnv but does not include
FULLSEND_OUTPUT_SCHEMA from h.ValidationLoop.Schema. After PR #5688
introduced directory-level script caching, the schemas/ directory
is no longer co-located with scripts/ in the cache, so
process-fix-result.py's relative-path fallback broke.

Extract the schema injection into a postScriptEnv helper (matching the
existing validationEnv pattern) so the conditional is testable without
duplicating production logic. exec.Cmd uses last-value-wins semantics,
so the appended entry correctly overrides any stale process env value.

Closes #5722

Signed-off-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
@ggallen
ggallen force-pushed the agent/5722-inject-schema-post-env branch from 42bc7ac to 08c551f Compare July 29, 2026 20:44
@ggallen
ggallen added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit 74bb946 Jul 29, 2026
12 of 13 checks passed
@ggallen
ggallen deleted the agent/5722-inject-schema-post-env branch July 29, 2026 21:05
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 29, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 9:08 PM UTC · Completed 9:17 PM UTC
Commit: 08c551f · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5729 — inject FULLSEND_OUTPUT_SCHEMA into post-script env

Verdict: Clean workflow. No new proposals — all improvement opportunities are covered by existing open issues.

Timeline

Time (UTC) Event
14:07–14:33 Two fix agent runs fail after PR #5688 broke process-fix-result.py's relative-path schema lookup
14:58 ggallen files issue #5722 with exact file locations, line numbers, and a ready-to-use code snippet
14:58–15:05 Triage agent runs (3 dispatched, 1 cancelled, 2 succeed). Correctly identifies root cause, severity, and adds appropriate labels (ready-to-code, priority/high, component/runner)
15:10 ggallen triggers /fs-code
15:11–15:23 Code agent produces PR #5729 in ~12 minutes — adds env var injection in run.go and 3 unit tests
15:25 rh-hemartin approves (human review, 2 min after PR creation)
15:24–15:39 Review bot approves with 2 low-severity findings: (1) tests duplicate inline logic instead of calling an extracted helper, (2) negative tests use len() comparison instead of the established for/HasPrefix iteration pattern
17:20–17:22 ggallen addresses both findings: extracts postScriptEnv helper and aligns assertion patterns. Force-pushes.
17:23–17:35 Re-review via /fs-review — review bot approves clean
21:05 Merged via merge queue

What went well

  • Triage quality: Fast (3 min), accurate root-cause analysis, correct labels, concrete proposed test.
  • Code agent output: Functionally correct fix in 12 minutes from a well-specified issue. The env var injection works and all three test cases are valid.
  • Review bot quality: Both findings were real and actionable — the bot caught style/pattern issues that the human reviewer did not flag. The suggestions (extract helper, match assertion pattern) directly improved code quality.
  • End-to-end pipeline: Issue → triage → code → review → fix → re-review → merge all worked smoothly.

Evidence for existing issues

Autonomy observations

  • The review bot's findings exceeded the human reviewer's in specificity — the bot caught two pattern inconsistencies that the human approval did not flag. For this class of change (small bug fixes in internal/cli with test coverage), the review bot demonstrates strong competence.
  • The human reviewer correctly determined the code was functionally correct. The bot's value-add was style/convention enforcement, not correctness.

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

Labels

component/runner Agent runner behavior and lifecycle ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch Review effort 1/5 type/bug Confirmed defect in existing behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment

2 participants