Skip to content

fix(#2869): inject FULLSEND_OUTPUT_SCHEMA into validation script env#2870

Merged
ggallen merged 1 commit into
mainfrom
fix-2869-validation-schema-env
Jul 2, 2026
Merged

fix(#2869): inject FULLSEND_OUTPUT_SCHEMA into validation script env#2870
ggallen merged 1 commit into
mainfrom
fix-2869-validation-schema-env

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

When a harness uses validation_loop.schema, the schema path is resolved and available at h.ValidationLoop.Schema but was not injected into the validation script's environment. Extract a validationEnv helper that conditionally adds FULLSEND_OUTPUT_SCHEMA when the schema path is set, fixing validation failures for harnesses that rely on the new field instead of env.runner.FULLSEND_OUTPUT_SCHEMA.

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com


Closes #2869

Post-script verification

  • Branch is not main/master (fix-2869-validation-schema-env)
  • Secret scan passed (gitleaks — 5638def7e6d23e4d0b3e34a721394064be855c60..HEAD)
  • 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 1, 2026 20:55
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Agent PR ready for human review label Jul 1, 2026
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Site preview

Preview: https://fb73dffa-site.fullsend-ai.workers.dev

Commit: 1c2514eb8701ff10a14ddbcb39790f19641e5294

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/cli/run.go 88.88% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment thread internal/cli/run.go
@waynesun09

Copy link
Copy Markdown
Member

/review

@qodo-code-review

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Warning

/review is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

2869 - Partially compliant

Compliant requirements:

  • Add FULLSEND_OUTPUT_SCHEMA to the runner-side validation script environment when h.ValidationLoop.Schema is set
  • Build validation env from h.RunnerEnv + TARGET_REPO_DIR + FULLSEND_RUN_DIR, conditionally adding schema
  • Apply the fix at the valCmd.Env construction point for runner-side validation execution
  • Prevent validation failures for harnesses using validation_loop.schema

Non-compliant requirements:

(empty)

Requires further human verification:

  • Verify end-to-end that a URL-sourced harness with validation_loop.schema successfully validates on a real workflow run (runner-side validation reading $FULLSEND_OUTPUT_SCHEMA)
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Env Precedence

valCmd.Env is constructed as append(os.Environ(), validationEnv(...)), so any overlapping keys from validationEnv should override the process environment. Confirm this ordering is intentional and matches expectations for TARGET_REPO_DIR, FULLSEND_RUN_DIR, and FULLSEND_OUTPUT_SCHEMA when those might already exist in the runner environment.

valCmd.Env = append(os.Environ(), validationEnv(h, hostRepositoryDir, runDir)...)
valOut, valErr := valCmd.CombinedOutput()
Nil/Empty Handling

validationEnv conditionally adds FULLSEND_OUTPUT_SCHEMA only when h.ValidationLoop != nil and Schema != "". Validate that all harness-loading paths consistently populate h.ValidationLoop.Schema with a host-accessible path (not sandbox path) and that no other validation-time code expects the variable to exist when Schema is omitted.

func validationEnv(h *harness.Harness, hostRepoDir, runDir string) []string {
	env := append(envToList(h.RunnerEnv),
		fmt.Sprintf("TARGET_REPO_DIR=%s", hostRepoDir),
		fmt.Sprintf("FULLSEND_RUN_DIR=%s", runDir),
	)
	if h.ValidationLoop != nil && h.ValidationLoop.Schema != "" {
		env = append(env, fmt.Sprintf("FULLSEND_OUTPUT_SCHEMA=%s", h.ValidationLoop.Schema))
	}
	return env

@waynesun09

Copy link
Copy Markdown
Member

/fs-fix #2870 (comment) and #2870 (comment)

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Fix · ⚠️ Cancelled · Started 9:42 PM UTC · Ended 9:47 PM UTC
Commit: 0123b0b · View workflow run →

When a harness uses validation_loop.schema, the schema path is resolved
and available at h.ValidationLoop.Schema but was not injected into the
validation script's environment. Extract a validationEnv helper that
conditionally adds FULLSEND_OUTPUT_SCHEMA when the schema path is set,
fixing validation failures for harnesses that rely on the new field
instead of env.runner.FULLSEND_OUTPUT_SCHEMA.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ggallen ggallen force-pushed the fix-2869-validation-schema-env branch from f77f88c to 1c2514e Compare July 1, 2026 23:11
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@ggallen ggallen added this pull request to the merge queue Jul 1, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 1, 2026
@ggallen ggallen added this pull request to the merge queue Jul 1, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 1, 2026
@ggallen ggallen added this pull request to the merge queue Jul 2, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 2, 2026
@ggallen ggallen added this pull request to the merge queue Jul 2, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 2, 2026
@ggallen ggallen added this pull request to the merge queue Jul 2, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 2, 2026
@ggallen ggallen added this pull request to the merge queue Jul 2, 2026
Merged via the queue into main with commit 1e7877a Jul 2, 2026
16 checks passed
@ggallen ggallen deleted the fix-2869-validation-schema-env branch July 2, 2026 00:40
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 2, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 12:44 AM UTC · Completed 12:52 AM UTC
Commit: 1c2514e · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2870 — inject FULLSEND_OUTPUT_SCHEMA into validation script env

Timeline

Time (UTC) Event
Jul 1, 19:41 Issue #2869 filed by ggallen — validation_loop.schema not exposed to validation script
Jul 1, 20:43 Code agent run 28546546028 starts
Jul 1, 20:55 PR #2870 opened by code agent (~12 min, +56/-6 lines, 2 files)
Jul 1, 21:01 Review agent run 28546636271 completes
Jul 1, 21:11 waynesun09 posts review: duplicate FULLSEND_OUTPUT_SCHEMA when both RunnerEnv and ValidationLoop.Schema are set (flagged by 3/5 review agents)
Jul 1, 21:38 waynesun09 triggers /fs-fix
Jul 1, 21:42 ggallen replies — explains this is a transient state per migration plan (#2849), asks to revert any fix
Jul 1, 21:42 Fix agent cancelled
Jul 1, 23:11 ggallen force-pushes (manual adjustment)
Jul 1, 23:28 ggallen approves and adds to merge queue
Jul 1, 23:28–00:32 5 merge queue ejectionsTestAdminInstallUninstall fails from GitHub API rate limiting on e2e pool orgs
Jul 2, 00:40 Merged on 6th attempt (~72 min in merge queue)

Assessment

Code agent: excellent. Produced a clean, well-structured fix in 12 minutes with a helper function extraction and 3 targeted test cases. 88.9% patch coverage.

Review agent: technically correct but lacked migration context. The duplicate env var finding was valid from a code quality standpoint, but the maintainer explained it's a planned transient state (phase 1 of a 3-phase migration). This triggered a /fs-fix cycle that was immediately cancelled — wasted tokens but minimal impact. Existing issues #1273 and #1351 cover the general need for better context incorporation; not proposing a new issue.

Merge queue: major time sink. 5 ejections over 72 minutes due to e2e rate limiting, not related to the PR's changes at all. This is the primary improvement opportunity.

Proposals filed

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

Labels

ready-for-review Agent PR ready for human review Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

validation_loop.schema not exposed to validation script environment

2 participants