feat(#5568): add top-level preflight_check to harness schema - #6009
feat(#5568): add top-level preflight_check to harness schema#6009fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
|
🤖 Finished Review · ✅ Success · Started 1:27 PM UTC · Completed 1:45 PM UTC Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Looks good to me Previous runLooks good to me Previous run (2)ReviewFindingsMedium
Low
Labels: PR adds a new harness schema field and wires its execution through the agent runner. |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
requires-manual-review
576eb95 to
a031486
Compare
|
🤖 Finished Review · ✅ Success · Started 3:12 PM UTC · Completed 3:28 PM UTC Commit: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
fullsend-ai-review
Bot
added
ready-for-merge
Add a top-level `preflight_check` field to the Harness struct so
harness authors can validate host dependencies for all scripts
(pre_script, post_script, validation_loop) in a single command
before sandbox creation.
Design: a top-level `Harness.PreflightCheck` string field that
runs once at the same execution point as the existing
`validation_loop.preflight_check`. The validation_loop-level
field is preserved for backward compatibility and runs after
the top-level check.
Changes:
- harness.go: add PreflightCheck field to Harness, add
ValidateRunnerEnvWith coverage for ${VAR} expansion
- forge.go: add PreflightCheck to ForgeConfig, wire into
mergeForgeConfig for forge-specific overrides
- compose.go: add PreflightCheck to mergeBaseIntoChild and
mergeForgeConfigInto for base composition
- run.go: wire ${VAR} expansion and preflight execution at
the same point as the existing validation_loop check
Note: pre-commit could not run in sandbox (network 403).
The post-script runs it authoritatively on the runner.
Closes #5568
Assisted-by: Claude (fix, review), Grok (review)
Signed-off-by: Wayne Sun <gsun@redhat.com>
a031486 to
b87d7f1
Compare
|
🤖 Review · ❌ Terminated · Started 3:48 PM UTC · Ended 4:01 PM UTC Commit: |
Site previewPreview: https://b8355628-site.fullsend-ai.workers.dev Commit: |
|
🤖 Finished Review · ✅ Success · Started 3:48 PM UTC · Completed 4:01 PM UTC Commit: |
1 participant
Summary
preflight_checkfield to theHarnessstruct, enabling harness authors to validate host-side dependencies for all scripts (pre_script,post_script,validation_loop) in a single command before sandbox creationForgeConfig), base composition (mergeBaseIntoChild,mergeForgeConfigInto),${VAR}expansion, andValidateRunnerEnvWith— matching the same infrastructure as the existingvalidation_loop.preflight_checkvalidation_loop.preflight_checkremains functional and runs after the top-level checkDesign choice
The issue offered two options: a top-level field vs per-script siblings. A top-level field was chosen because
pre_scriptandpost_scriptare flat strings (not structs), so adding per-script siblings would require either breaking the schema or adding awkwardpre_script_preflight_check/post_script_preflight_checkfields. A single top-level command that validates all dependencies is simpler and more general.Testing
TestLoad_TopLevelPreflightCheck*)ValidateRunnerEnvWithcoverage of${VAR}refsTestLoadWithBase_LocalBase_TopLevelPreflightCheck*)TestResolveForge_PreflightCheck*)mergeForgeConfigInto(TestMergeForgeConfigInto_TopLevelPreflightCheck*)runAgentexecution: pass, fail, timeout, and both-checks-configured scenariosCloses #5568
Post-script verification
agent/5568-top-level-preflight-check)6efbe40026557d24e2643701167e7a53079c57ee..HEAD)