fix(#526): read schema path from FULLSEND_OUTPUT_SCHEMA env var by fullsend-ai-coder[bot] · Pull Request #527 · fullsend-ai/agents

fullsend-ai-coder · 2026-07-29T15:11:31Z

Summary

Replace relative path construction (os.path.dirname(__file__)/../schemas/...) in process-fix-result.py with os.environ["FULLSEND_OUTPUT_SCHEMA"] (hard require, no fallback)
After directory-level script caching (fix(#3640): directory-level fetching for pre/post scripts fullsend#5688), scripts are cached at sha256/<hash>/scripts/ without co-locating schemas/, breaking the ../schemas/ traversal
Follows the same env var pattern used by validate-output-schema.sh

Testing

All 30 existing tests pass with FULLSEND_OUTPUT_SCHEMA set at module level
New TestSchemaEnvVar.test_missing_env_var_fails verifies exit 1 with a clear error when the env var is missing
New TestSchemaEnvVar.test_env_var_used_for_schema_path verifies successful schema resolution via env var

Context

Companion to fullsend-ai/fullsend#5722 which injects the env var into the post-script environment. Merge order is independent.

Closes #526

Post-script verification

Branch is not main/master (agent/526-schema-path-from-env)
Secret scan passed (gitleaks — 6d0eac284c120df870fa3b518525921096b2d54b..HEAD)
PR body secret scan passed (gitleaks — no-git)
Pre-commit hooks passed (authoritative run on runner)
Tests ran inside sandbox

fullsend-ai-review · 2026-07-29T15:13:20Z

🤖 Finished Review · ✅ Success · Started 3:13 PM UTC · Completed 3:29 PM UTC
Commit: ce0fd19 · View workflow run →

fullsend-ai-review · 2026-07-29T15:29:27Z

Review

Findings

Medium

[api-contract] scripts/process-fix-result.py:170 — The new FULLSEND_OUTPUT_SCHEMA env var requirement is not set by any caller in this repository. Both post-fix.src.sh and post-fix.sh invoke process-fix-result.py without exporting FULLSEND_OUTPUT_SCHEMA, and no workflow or harness config in this repo sets it. When jsonschema is installed, every invocation will exit 1 until the companion change (runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722) lands. Downgraded from high to medium because the existing relative-path approach is already broken after directory-level script caching (#5688), so this PR does not introduce a net-new regression — it changes the failure mode from a file-not-found error to a clear env-var-missing error.
Remediation: Ensure runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722 lands before or simultaneously with this PR. Alternatively, add a fallback that uses the old relative-path resolution when the env var is unset (with a deprecation warning), so the changes can merge independently.
[protected-path] scripts/ — PR modifies files under protected path scripts/: scripts/process-fix-result.py and scripts/process-fix-result-test.py. The PR links to issue post-fix: read FULLSEND_OUTPUT_SCHEMA from env instead of relative path #526 and explains the rationale (fixing broken schema path resolution after directory-level script caching). Human approval is always required for protected-path changes, regardless of context.

Low

[test-integrity] scripts/process-fix-result-test.py:476 — test_missing_env_var_fails uses os.environ.pop('FULLSEND_OUTPUT_SCHEMA', None) inside a method whose class is decorated with @patch.dict(os.environ, _SCHEMA_ENV). The approach is functionally correct — patch.dict wraps each method individually, so the pop is isolated and the env var is restored after the method returns. However, the intent would be clearer with a more explicit env-var exclusion pattern.
[naming-consistency] scripts/process-fix-result-test.py:29 — The new module-level constants _SCHEMA_PATH and _SCHEMA_ENV use leading underscore naming, which is not established in this test file. The existing module-level names (build_summary_body, main, post_summary, MAX_COMMENT_LENGTH) do not use leading underscores. However, leading underscores for module-private test helpers is standard Python convention (PEP 8), so both approaches are defensible.

Previous run

Review

Findings

Medium

[api-contract] scripts/process-fix-result.py:170 — The new FULLSEND_OUTPUT_SCHEMA env var requirement is not set by any caller in this repository. Both post-fix.src.sh and post-fix.sh invoke process-fix-result.py without exporting FULLSEND_OUTPUT_SCHEMA, and no workflow or harness config in this repo sets it. When jsonschema is installed, every invocation will exit 1 until the companion change (runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722) lands. Downgraded from high to medium because the existing relative-path approach is already broken after directory-level script caching (#5688), so this PR does not introduce a net-new regression — it changes the failure mode from a file-not-found error to a clear env-var-missing error.
Remediation: Ensure runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722 lands before or simultaneously with this PR. Alternatively, add a fallback that uses the old relative-path resolution when the env var is unset (with a deprecation warning), so the changes can merge independently.
[protected-path] scripts/ — PR modifies files under protected path scripts/: scripts/process-fix-result.py and scripts/process-fix-result-test.py. The PR links to issue post-fix: read FULLSEND_OUTPUT_SCHEMA from env instead of relative path #526 and explains the rationale (fixing broken schema path resolution after directory-level script caching). Human approval is always required for protected-path changes, regardless of context.

Low

[test-integrity] scripts/process-fix-result-test.py:476 — test_missing_env_var_fails removes the env var by calling os.environ.pop('FULLSEND_OUTPUT_SCHEMA', None) inside a method decorated (via the class-level @patch.dict) with the env var already set. The approach is functionally correct — patch.dict wraps each method individually, so the pop is isolated and the env var is restored after the method completes. However, the intent would be clearer with a more explicit env-var exclusion pattern.
[error-message-consistency] scripts/process-fix-result.py:173 — Error message format deviates from neighboring patterns. Existing errors use Cannot read <resource>: <error> (lines 166, 180). The new message uses Cannot validate schema: <explanation>. While semantically reasonable (the error is about a missing env var, not a file read failure), it breaks the visual consistency of the error block.
Remediation: Consider rephrasing to Cannot resolve schema path: FULLSEND_OUTPUT_SCHEMA environment variable is not set to better match the Cannot <verb> <resource>: <detail> pattern.

Previous run (2)

Review

Findings

Medium

[api-contract] scripts/process-fix-result.py:170 — The new FULLSEND_OUTPUT_SCHEMA env var requirement is not set by any caller in this repository. Both post-fix.src.sh and post-fix.sh invoke process-fix-result.py without exporting FULLSEND_OUTPUT_SCHEMA, and harness/fix.yaml does not include the var in its runner env block. When jsonschema is installed, every invocation will exit 1 until the companion change (runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722) lands. Downgraded from high to medium because the existing relative-path approach is already broken after directory-level script caching (#5688), so this PR does not introduce a net-new regression — it changes the failure mode from a file-not-found error to a clear env-var-missing error.
Remediation: Ensure runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722 lands before or simultaneously with this PR. Alternatively, add a fallback that uses the old relative-path resolution when the env var is unset (with a deprecation warning), so the changes can merge independently.
[protected-path] scripts/ — PR modifies files under protected path scripts/: scripts/process-fix-result.py and scripts/process-fix-result-test.py. The PR links to issue post-fix: read FULLSEND_OUTPUT_SCHEMA from env instead of relative path #526 and explains the rationale (fixing broken schema path resolution after directory-level script caching). Human approval is always required for protected-path changes, regardless of context.

Low

[test-integrity] scripts/process-fix-result-test.py:480 — test_missing_env_var_fails uses a nested patch.dict to remove FULLSEND_OUTPUT_SCHEMA from os.environ, but the approach is slightly indirect: it enters a patch.dict context manager and then calls os.environ.pop() to remove the key set by the class-level decorator. The test is functionally correct, but the intent could be clearer with a more explicit env-var exclusion pattern.

Previous run (3)

Review

Findings

Medium

[api-contract] scripts/process-fix-result.py:170 — The new FULLSEND_OUTPUT_SCHEMA env var requirement is not set by any caller in this repository. Both post-fix.src.sh and post-fix.sh invoke process-fix-result.py without exporting FULLSEND_OUTPUT_SCHEMA, and harness/fix.yaml does not include the var in its runner env block. When jsonschema is installed, every invocation will exit 1 until the companion change (runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722) lands. Downgraded from high to medium because the existing relative-path approach is already broken after directory-level script caching (#5688), so this PR does not introduce a net-new regression — it changes the failure mode from a file-not-found error to a clear env-var-missing error.
Remediation: Ensure runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722 lands before or simultaneously with this PR. Alternatively, add a fallback that uses the old relative-path resolution when the env var is unset (with a deprecation warning), so the changes can merge independently.
[protected-path] scripts/ — PR modifies files under protected path scripts/: scripts/process-fix-result.py and scripts/process-fix-result-test.py. The PR links to issue post-fix: read FULLSEND_OUTPUT_SCHEMA from env instead of relative path #526 and explains the rationale (fixing broken schema path resolution after directory-level script caching). Human approval is always required for protected-path changes, regardless of context.

Low

[error-messages] scripts/process-fix-result.py:172 — Error message FULLSEND_OUTPUT_SCHEMA must be set lacks context about what operation failed, unlike other error messages in this file that include the operation context (e.g., Cannot read {result_file}: {e}).
Remediation: Add context to the error message, such as: Cannot validate schema: FULLSEND_OUTPUT_SCHEMA environment variable is not set.

Previous run (4)

Review

Findings

Medium

[api-contract] scripts/process-fix-result.py:170 — The new FULLSEND_OUTPUT_SCHEMA env var requirement is not set by any caller in this repository. Both post-fix.src.sh and post-fix.sh invoke process-fix-result.py without exporting FULLSEND_OUTPUT_SCHEMA, and harness/fix.yaml does not include the var in its runner env block. When jsonschema is installed, every invocation will exit 1 until the companion change (runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722) lands. Downgraded from high to medium because the existing relative-path approach is already broken after directory-level script caching (#5688), so this PR does not introduce a net-new regression — it changes the failure mode from a file-not-found error to a clear env-var-missing error.
Remediation: Ensure runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722 lands before or simultaneously with this PR. Alternatively, add a fallback that uses the old relative-path resolution when the env var is unset (with a deprecation warning), so the changes can merge independently.
[protected-path] scripts/ — PR modifies files under protected path scripts/: scripts/process-fix-result.py and scripts/process-fix-result-test.py. The PR links to issue post-fix: read FULLSEND_OUTPUT_SCHEMA from env instead of relative path #526 and explains the rationale (fixing broken schema path resolution after directory-level script caching). Human approval is always required for protected-path changes, regardless of context.

Previous run (5)

Review

Findings

High

[api-contract] scripts/process-fix-result.py:170 — The new FULLSEND_OUTPUT_SCHEMA env var requirement is not set by any caller in this repo. Both post-fix.src.sh and post-fix.sh invoke process-fix-result.py without exporting FULLSEND_OUTPUT_SCHEMA. When jsonschema is installed, the script will exit 1 for every invocation until the companion change (runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722) lands. There is no fallback to the old relative-path resolution.
Remediation: Either (a) land runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722 first or simultaneously to ensure the env var is injected, or (b) add a fallback in process-fix-result.py that uses the old relative-path construction when the env var is unset (with a deprecation warning), so the two changes can merge independently.

Medium

[protected-path] scripts/ — PR modifies files under protected path scripts/: scripts/process-fix-result.py and scripts/process-fix-result-test.py. The PR links to issue post-fix: read FULLSEND_OUTPUT_SCHEMA from env instead of relative path #526 and explains the rationale (fixing broken schema path resolution after directory-level script caching). Human approval is always required for protected-path changes, regardless of context.

Low

[test organization] scripts/process-fix-result-test.py:28 — The env var FULLSEND_OUTPUT_SCHEMA is set at module level, which means it leaks into every test class in the file, not just TestSchemaEnvVar. The test_missing_env_var_fails test works around this by manually popping the var, but any future test that depends on the env var being absent will silently inherit it.

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 (6)

Review

Findings

High

[api-contract] scripts/process-fix-result.py:169 — The new FULLSEND_OUTPUT_SCHEMA env var requirement is not set by any caller in this repo (post-fix.src.sh, post-fix.sh). The production code hard-requires this env var (exits 1 when jsonschema is installed and the var is unset). The companion harness change (runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722) must land to inject this env var into the post-script environment. The PR states merge order is independent because the current relative-path resolution is already broken after directory-level script caching — both changes are needed for the script to function.
Remediation: Ensure runner: inject FULLSEND_OUTPUT_SCHEMA into post-script environment fullsend#5722 lands, or add a fallback in process-fix-result.py that uses the old relative-path construction when the env var is unset (with a deprecation warning).

Medium

[protected-path] scripts/ — PR modifies files under protected path scripts/: scripts/process-fix-result.py and scripts/process-fix-result-test.py. The PR links to issue post-fix: read FULLSEND_OUTPUT_SCHEMA from env instead of relative path #526 and explains the rationale (fixing broken schema path resolution after directory-level script caching). Human approval is always required for protected-path changes, regardless of context.

Low

[test-inadequate] scripts/process-fix-result-test.py:481 — In test_missing_env_var_fails, the finally block manually restores FULLSEND_OUTPUT_SCHEMA after patch.dict has already exited and restored os.environ. The dual restoration is redundant — patch.dict snapshots on entry and restores on exit, making the manual os.environ["FULLSEND_OUTPUT_SCHEMA"] = _SCHEMA_PATH unnecessary.

Labels: PR modifies fix-agent post-processing script and fixes a bug in schema path resolution

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

ggallen · 2026-07-29T15:36:24Z

/fs-review

fullsend-ai-review · 2026-07-29T15:38:23Z

🤖 Finished Review · ✅ Success · Started 3:38 PM UTC · Completed 3:53 PM UTC
Commit: c94dc49 · View workflow run →

ggallen · 2026-07-29T15:58:34Z

/fs-review

fullsend-ai-review · 2026-07-29T16:00:46Z

🤖 Finished Review · ✅ Success · Started 4:00 PM UTC · Completed 4:15 PM UTC
Commit: db8ac75 · View workflow run →

Superseded by updated review

ggallen · 2026-07-29T18:07:28Z

/fs-review

fullsend-ai-review · 2026-07-29T18:09:16Z

🤖 Finished Review · ✅ Success · Started 6:09 PM UTC · Completed 6:22 PM UTC
Commit: 7cc49ff · View workflow run →

ggallen · 2026-07-29T19:46:58Z

/fs-review

fullsend-ai-review · 2026-07-29T19:48:50Z

🤖 Review · ⚠️ Cancelled · Started 7:48 PM UTC · Ended 7:53 PM UTC
Commit: 9566d29 · View workflow run →

ggallen · 2026-07-29T19:52:22Z

/fs-review

ggallen · 2026-07-29T19:57:23Z

/fs-review

fullsend-ai-review · 2026-07-29T19:59:19Z

🤖 Finished Review · ✅ Success · Started 7:59 PM UTC · Completed 8:15 PM UTC
Commit: 9566d29 · View workflow run →

ggallen · 2026-07-29T20:19:28Z

/fs-review

ggallen · 2026-07-29T20:39:24Z

/fs-review

fullsend-ai-review · 2026-07-29T20:41:20Z

🤖 Finished Review · ✅ Success · Started 8:41 PM UTC · Completed 8:58 PM UTC
Commit: d95483b · View workflow run →

Signed-off-by: Greg Allen <gallen@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

ggallen · 2026-07-29T21:00:38Z

/fs-review

fullsend-ai-review · 2026-07-29T21:02:33Z

🤖 Finished Review · ✅ Success · Started 9:02 PM UTC · Completed 9:19 PM UTC
Commit: f74596f · View workflow run →

fullsend-ai-review

See the review comment for full details.

ggallen · 2026-07-29T21:24:02Z

/fs-review

fullsend-ai-review · 2026-07-29T21:25:47Z

🤖 Finished Review · ✅ Success · Started 9:25 PM UTC · Completed 9:37 PM UTC
Commit: f74596f · View workflow run →

fullsend-ai-retro · 2026-07-29T21:29:10Z

🤖 Finished Retro · ✅ Success · Started 9:29 PM UTC · Completed 9:43 PM UTC
Commit: f74596f · View workflow run →

fullsend-ai-review · 2026-07-29T21:37:53Z

Review skipped — this PR is already merged.

The /fs-review command only reviews open pull requests.

_{Posted by fullsend post-review check}

fullsend-ai-retro · 2026-07-29T21:43:59Z

Retro: PR #527 — read schema path from FULLSEND_OUTPUT_SCHEMA env var

Timeline

Issue #526 was filed by ggallen at 14:58 UTC on 2026-07-29, describing a broken schema path resolution in process-fix-result.py after directory-level script caching. The triage agent ran successfully in 4 minutes, correctly identifying severity and recommending the env-var approach. The code agent produced PR #527 by 15:11 — a clean 70-addition, 6-deletion change across 2 files.

From there, the PR entered an extended review cycle: 7 distinct commit SHAs across force-pushes, 8+ review agent runs, and 6+ hours of wall-clock time before merge at 21:26. The human approved the PR at 18:24 (round 4), yet 3 more review-and-fix iterations followed post-approval.

What went well

Triage was fast (4 min) and accurate — correct root cause, correct fix recommendation.
Code agent produced the correct core implementation in 5 minutes.
Review agent identified several genuine code quality issues: a redundant finally block, a module-level env var leak across test classes, and an error message format inconsistency. All three were fixed and improved the final code.
Final code quality is good — well-scoped, well-tested, follows established patterns.

What could go better

Repeated api-contract finding (8+ times): The review agent raised the same [api-contract] finding about FULLSEND_OUTPUT_SCHEMA not being set by callers in this repo on every review run. The human dismissed it with the same explanation each time: the old relative-path approach is already broken, and the companion PR (fullsend-ai/fullsend#5722) will inject the env var. This consumed significant human attention. This pattern is well-covered by existing issues: #106, #343, #372, fullsend#1583, fullsend#2959. This PR provides particularly strong evidence for those issues — 8 repetitions of an identical finding with 5+ identical human dismissals.

Fix agent blocked: The fix agent was triggered twice but both runs were blocked by the label gate ("Human-authored PR without 'fullsend-fix' label"), so the human had to manually fix all review findings, force-push, and re-request review. This is expected behavior for human-authored PRs without explicit opt-in.

Excessive review volume: 60 total workflow dispatch runs (source repo) for a 2-file bug fix is substantial infrastructure cost. This is partially covered by fullsend#902 (circuit breaker) and fullsend#2992 (cap review iterations).

Same-SHA duplicate review: SHA f74596f was reviewed twice (runs 30490713918 and 30492241754), covered by #323.

Companion change awareness

The review agent correctly identified the cross-repo dependency with fullsend#5722 but its remediation was internally contradictory (see proposal below). The broader topic of companion PR awareness is covered by fullsend#1414 and fullsend#2394.

Proposals filed

Correctness sub-agent should not suggest remediation that contradicts its own analysis (in fullsend-ai/agents)

fullsend-ai-coder Bot requested a review from a team as a code owner July 29, 2026 15:11

fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Jul 29, 2026

fullsend-ai-coder Bot assigned ggallen Jul 29, 2026