feat(harness): add validation_loop.schema for URL-resolved output schemas

[ggallen]

Summary

• Add Schema field to ValidationLoop struct so output schemas can be declared alongside the validation script
• Resolve the schema using the same fetchBaseFile mechanism that already works for validation_loop.script — URL-sourced harnesses get the schema fetched and cached automatically
• Runner checks ValidationLoop.Schema first, falls back to RunnerEnv["FULLSEND_OUTPUT_SCHEMA"] for backward compatibility
• Add lock file handling for the new field

Closes #2849

Context

When a harness is loaded from a URL via config-driven agent registration (ADR-0058), the output schema referenced by FULLSEND_OUTPUT_SCHEMA is silently skipped — the env var expands to a local ${FULLSEND_DIR}/schemas/... path that doesn't exist. This affects all 6 agents. The fix lets harnesses declare the schema as a file reference that gets resolved like other resources.

Files changed

• internal/harness/harness.go — Schema field + ResolveRelativeTo, ValidateFilesExist, ValidateResourceTypes updates
• internal/harness/compose.go — fetchBaseFile calls for validation_loop.schema at top level and forge level
• internal/cli/run.go — prefer ValidationLoop.Schema over RunnerEnv["FULLSEND_OUTPUT_SCHEMA"]
• internal/cli/lock.go — lock file handling for the new field
• Tests in compose_test.go and harness_test.go

Test plan

• go test ./internal/harness/... — all tests pass (new + existing)
• go build ./internal/harness/... — compiles clean
• Pre-commit hooks pass (gofmt, go vet)
• Follow-up: update harness YAML files to use validation_loop.schema instead of env.runner.FULLSEND_OUTPUT_SCHEMA

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Add validation_loop.schema to resolve output schemas for URL-loaded harnesses

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 40+ Minutes

AI Description

• Add validation_loop.schema so output schemas are resolved and cached like validation scripts.
• Prefer resolved schema in the runner, falling back to FULLSEND_OUTPUT_SCHEMA for compatibility.
• Extend lockfile mapping and add coverage for base + forge schema fetching.

Diagram

graph TD
  A["Harness YAML (URL/base)"] --> B["compose.LoadWithBase"] --> C[("Fetched cache") ] --> D["Harness.ValidationLoop.Schema"] --> E["cli bootstrapEnv"] --> F["Sandbox output-schema.json"]
  G["lockfile resolveFromLock"] --> D

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Teach FULLSEND_OUTPUT_SCHEMA to support URL/base resolution

• ➕ Avoids adding a new harness field; keeps configuration surface area unchanged
• ➕ Works for any future schema-like resources referenced via runner env
• ➖ Implicit behavior: env var becomes a fetchable resource, which may surprise users
• ➖ Harder to validate and lock consistently because the semantics are indirect

2. Allow validation_loop.schema to be a URL directly

• ➕ Most direct authoring model for remote schemas
• ➕ Avoids relying on base URL directory resolution rules
• ➖ Expands URL allowlist/security surface area in the harness model
• ➖ Would require runner/lock to handle URL fetch semantics or to centralize fetching elsewhere

3. Introduce a top-level output_schema field (separate from validation_loop)

• ➕ More semantically precise if the schema is not strictly part of validation loop behavior
• ➕ Could be reused by multiple execution modes beyond validation
• ➖ Requires a broader config refactor and migration of existing usage patterns
• ➖ More plumbing changes across compose/runner/lock than the current targeted fix

Recommendation: The chosen approach (a dedicated validation_loop.schema resolved via existing fetchBaseFile) is the best tradeoff: it fixes URL-loaded harnesses with minimal new plumbing, keeps URL fetching centralized in compose (not the runner), and preserves backward compatibility by falling back to FULLSEND_OUTPUT_SCHEMA. Alternatives were considered but are either more implicit (env-var magic) or broader in scope (new top-level schema concept).

Files changed (6) +185 / -1

Enhancement (1) +14 / -0

harness.goAdd Schema to ValidationLoop and validate/resolve it like other resources +14/-0

Add Schema to ValidationLoop and validate/resolve it like other resources

• Introduces 'Schema' on 'ValidationLoop' and wires it into relative path resolution and file existence validation. Adds a resource type check that rejects URL values for 'validation_loop.schema' to keep the harness model consistent (URLs are fetched by compose, not stored directly).

internal/harness/harness.go

Bug fix (3) +38 / -1

lock.goAdd lockfile hydration for validation_loop.schema +6/-0

Add lockfile hydration for validation_loop.schema

• Updates lockfile resolution to map 'validation_loop.schema' back onto 'h.ValidationLoop.Schema' when present. Adds the parallel forge field case to keep behavior consistent with other forge-scoped resources.

internal/cli/lock.go

run.goPrefer ValidationLoop.Schema over FULLSEND_OUTPUT_SCHEMA +9/-1

Prefer ValidationLoop.Schema over FULLSEND_OUTPUT_SCHEMA

• Changes sandbox bootstrap to select the schema path from 'h.ValidationLoop.Schema' first. Falls back to 'RunnerEnv["FULLSEND_OUTPUT_SCHEMA"]' to keep older harnesses working unchanged.

internal/cli/run.go

compose.goFetch and cache validation_loop.schema for base and forge harnesses +23/-0

Fetch and cache validation_loop.schema for base and forge harnesses

• Extends base URL resolution to fetch 'validation_loop.schema' using the same validation + 'fetchBaseFile' pipeline as 'validation_loop.script'. Applies the same behavior for forge-specific validation loop configs and records dependencies for locking.

internal/harness/compose.go

Tests (2) +133 / -0

compose_test.goAdd URL-base compose tests for validation_loop.schema fetching +107/-0

Add URL-base compose tests for validation_loop.schema fetching

• Adds coverage ensuring schemas referenced under 'validation_loop.schema' are fetched, cached to an absolute path, and included in dependency lists. Includes both top-level validation loop and forge-specific validation loop schema cases.

internal/harness/compose_test.go

harness_test.goTest ValidationLoop.Schema URL rejection and local acceptance +26/-0

Test ValidationLoop.Schema URL rejection and local acceptance

• Adds validation tests ensuring 'validation_loop.schema' cannot be a URL and that local paths are accepted. Mirrors the existing 'validation_loop.script' resource-type guarantees.

internal/harness/harness_test.go

[github-actions]

Site preview

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

Commit: ece7f5bda886d8b9482b064c470ef32643340034

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:29 PM UTC · Completed 3:41 PM UTC
Commit: 8141ce5 · View workflow run →

[codecov]

Codecov Report

❌ Patch coverage is 92.85714% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/run.go	75.00%	1 Missing and 1 partial ⚠️
internal/cli/lock.go	75.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 Output schema not resolved for URL-sourced harnesses (config-driven agents)

✅ Compliance rules (platform): 54 rules

✅ Skills: writing-user-docs, writing-adrs

1. Schema env vars mishandled ✓ Resolved 🐞 Bug ≡ Correctness

Description

validation_loop.schema is resolved and existence-checked as a normal relative path, so values like
"${FULLSEND_DIR}/schemas/..." get joined onto baseDir and then fail os.Stat. This will break
harnesses that migrate the existing FULLSEND_OUTPUT_SCHEMA pattern into validation_loop.schema.

Code

internal/harness/harness.go[R529-533]

+		if h.ValidationLoop.Schema != "" {
+			if h.ValidationLoop.Schema, err = resolve("validation_loop.schema", h.ValidationLoop.Schema); err != nil {
+				return err
+			}
+		}

Relevance

⭐⭐⭐ High

Repo allows ${VAR} paths in ResolveRelativeTo; team accepts env/FULLSEND_DIR path correctness fixes.

PR-#1555
PR-#279
PR-#2582

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

ResolveRelativeTo does not special-case ${VAR} paths (despite the comment) and the new schema field
is routed through that resolver; later, ValidateFilesExist will stat the resulting path.
bootstrapEnv then uses ValidationLoop.Schema verbatim, while only RunnerEnv/env.runner get
FULLSEND_DIR expansion, and existing harnesses use the ${FULLSEND_DIR}/schemas/... pattern today.

internal/harness/harness.go[469-533]
internal/harness/harness.go[642-651]
internal/cli/run.go[385-406]
internal/cli/run.go[1407-1421]
internal/scaffold/fullsend-repo/harness/triage.yaml[27-34]

Agent prompt

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

### Issue description
`validation_loop.schema` currently behaves like a plain relative path. If a harness sets it to a value containing `${VAR}` (especially `${FULLSEND_DIR}`), `ResolveRelativeTo` incorrectly `filepath.Join`s it under `baseDir`, and `ValidateFilesExist` will `os.Stat` the unexpanded/mangled path.

### Issue Context
- `RunnerEnv`/`env.runner` values are expanded with an expander that injects `FULLSEND_DIR`, but `ValidationLoop.Schema` is not.
- Existing scaffold harnesses already use `${FULLSEND_DIR}/schemas/...` for `FULLSEND_OUTPUT_SCHEMA`, so a straightforward migration to `validation_loop.schema` with the same value will fail.

### Fix Focus Areas
- internal/harness/harness.go[469-536]
- internal/harness/harness.go[587-653]
- internal/cli/run.go[385-420]
- internal/cli/run.go[1407-1432]

### Suggested fix
1. In `Harness.ResolveRelativeTo`, treat paths containing `${` as "already-resolved" (similar to `host_files[].src`) and return them unchanged (do not `Join`).
2. In `Harness.ValidateFilesExist`, skip `os.Stat` for `validation_loop.schema` when it contains `${` (or alternatively require callers to expand first and document/enforce that).
3. In `internal/cli/run.go`, expand `h.ValidationLoop.Schema` with the same `expander` used for `RunnerEnv` before calling `ValidateFilesExist()` / before `bootstrapEnv` uses it.

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

---

[fullsend-ai-review]

Looks good to me

Previous run

Looks good to me

Previous run (2)

Review

Findings

Medium

• [undocumented-config-option] docs/ADRs/0024-harness-definitions.md — ADR-0024 documents the validation_loop YAML structure showing script, max_iterations, and feedback_mode but no schema field. As the canonical harness definition ADR, it should reflect the new field.
  Remediation: Add a minor annotation to ADR-0024 documenting the schema field in the validation_loop block.

• [undocumented-config-option] docs/ADRs/0045-forge-portable-harness-schema.md — ADR 0045 documents the validation_loop configuration structure but does not mention the new schema field. The ADR shows validation_loop with only script and max_iterations fields.
  Remediation: Add a minor annotation documenting the validation_loop.schema field.

• [undocumented-config-option] docs/guides/user/building-custom-agents.md — The guide shows validation_loop configuration with only script and max_iterations, and documents FULLSEND_OUTPUT_SCHEMA as the mechanism for specifying the schema path. The new validation_loop.schema field is not mentioned as an alternative.
  Remediation: Update the validation_loop example to include the schema field as an option.

• [undocumented-config-option] docs/guides/user/customizing-agents.md — The customizing agents guide shows a validation_loop example with only script and max_iterations fields. It does not document the new schema field option.
  Remediation: Add the schema field to the validation_loop example.

Previous run (3)

Review

Findings

Medium

• [env validation gap] internal/harness/harness.go — ValidateRunnerEnvWith does not check ${VAR} references in validation_loop.schema. Other fields that support ${VAR} expansion (runner_env values, host_files[].src, env.runner, env.sandbox) are validated to ensure the referenced host variables exist. If validation_loop.schema contains a reference to an undefined variable (e.g., ${MISSING_DIR}/schema.json), os.Expand will silently expand it to an empty string, and the error will surface later as a confusing "file not found" from ValidateFilesExist rather than a clear "host variable X is not set" message.
  Remediation: Add a checkVarRefs call for h.ValidationLoop.Schema inside ValidateRunnerEnvWith, guarded by h.ValidationLoop != nil && h.ValidationLoop.Schema != "".

---

Labels: PR modifies harness struct/compose/validation and runner bootstrap logic for schema resolution.

---

Labels: PR modifies harness struct definition, compose resolution, runner bootstrap, and lock file handling for the validation_loop component

Previous run (4)

Looks good to me

---

Labels: PR modifies harness struct/compose/validation and runner bootstrap logic for schema resolution.

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 4:00 PM UTC · Ended 4:10 PM UTC
Commit: f417572 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:13 PM UTC · Completed 4:25 PM UTC
Commit: 4a51b3e · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:34 PM UTC · Completed 4:50 PM UTC
Commit: 2577090 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:58 PM UTC · Completed 5:11 PM UTC
Commit: cc1563f · View workflow run →

[waynesun09]

Review Squad — 5 MEDIUM findings (8 agents, 3 models)

Agents: 2x Claude coder, 2x Claude researcher, 2x Gemini, 2x Codex
Findings posted inline: 4

Not inline (targets unchanged file)

MEDIUM — Missing lint diagnostic when both validation_loop.schema and FULLSEND_OUTPUT_SCHEMA are set
File: internal/harness/lint.go (not changed in this PR)
When both are set, bootstrapEnv silently prefers validation_loop.schema. The stale env var still gets expanded and validated, requiring its referenced variables to be set even though the value is unused. Consider adding a lint diagnostic warning when both h.ValidationLoop.Schema != "" and h.RunnerEnv["FULLSEND_OUTPUT_SCHEMA"] are set. Can be a follow-up.

Assisted-by: Claude (review), Gemini (review), Codex (review)

[fullsend-ai-review]

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

[ralphbean]

LGTM. I'd suggest addressing @waynesun09's inline findings before merging — the validateForge URL check gap in particular.

[ggallen]

LGTM. I'd suggest addressing @waynesun09's inline findings before merging — the validateForge URL check gap in particular.

Thanks, @ralphbean. All of those issues have been resolved.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:03 PM UTC · Completed 7:20 PM UTC
Commit: ece7f5b · View workflow run →

[fullsend-ai-retro]

Retro: PR #2851 — feat(harness): add validation_loop.schema for URL-resolved output schemas

Timeline: Issue #2849 filed at 14:50 UTC → triage applied ready-to-code at 14:55 → code agent attempt 1 failed (gofmt hook) at 15:11 → PR opened at 15:26 → code agent attempt 2 succeeded at 16:00 → 6 review bot runs (all approved) → human Review Squad found 5 MEDIUM issues at 17:56 → author fixed all → human approvals → merged at 18:58. Total: ~4 hours.

What went well:

• Triage was fast and accurate (5 min).
• The review bot found 5 real issues across intermediate runs (1 env validation gap, 4 undocumented-config-option findings), and the author addressed them.
• Human Review Squad (waynesun09) found 5 additional MEDIUM issues the bot missed, all fixed before merge.
• Clean merge with good test coverage (92.86% patch).

What could go better:

1. Code agent gofmt failure caused a full retry cycle (~65 min delay). Already tracked by #2852 (post-code auto-fix salvage) and #1865 (run formatters before committing).
2. 6 review dispatches consumed significant tokens. Already heavily tracked by #893, #1418, #2599, and others.
3. Review quality gap: The bot approved the final state but missed 5 findings the human squad caught — notably a forge validation parity gap and a doc example missing a required field. Partially covered by #1525 (cross-file impact analysis) and #2589 (missing test coverage detection).
4. Novel gap — forge-scoped validation parity: When ValidateResourceTypes checked ValidationLoop.Schema for URLs at top-level, the corresponding validateForge() did not. This class of bug (validation logic not applied symmetrically across top-level and forge-scoped config) is not covered by any existing issue. One proposal filed below.

Most improvement opportunities for this workflow are already tracked in the backlog. The one novel finding is a review heuristic for forge/top-level validation symmetry.

Proposals filed

• Review agent should check forge-scoped validation parity with top-level validation (in fullsend-ai/fullsend)