Skip to content

docs(#1102): document harness host_files execution order constraints - #1103

Open
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/1102-host-files-lifecycle-docs
Open

docs(#1102): document harness host_files execution order constraints#1103
fullsend-ai-coder[bot] wants to merge 2 commits into
mainfrom
agent/1102-host-files-lifecycle-docs

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

  • Add a "Harness execution lifecycle" section to skills/code-implementation/SKILL.md documenting the four-phase execution order: config load/validation → pre-script → sandbox setup → agent execution
  • Document the host_files timing constraint: entries are validated at config load time (before pre-scripts run), so files created by pre-scripts must use optional: true and dynamic path variables (${RUNNER_TEMP})
  • Include correct and incorrect YAML examples showing the proper pattern for pre-script-generated context files
  • Update docs/code.md to reflect the four-phase pipeline with a cross-reference to the skill documentation

Motivation

PR #1075 introduced a Jira-source overlay that added a host_files entry pointing to a file created by the Jira pre-script. The harness loader rejected the config at load time because the file didn't exist yet — the pre-script hadn't run. This caused a multi-day incident requiring 5 manual human commits to fix. The code agent had no guidance about this execution order constraint, making the mistake systematic and likely to recur.

Testing

  • make lint passes (skillsaw baseline regenerated for increased token count)
  • hack/lint-agent-docs passes (doc structure valid)
  • Secret scan passes

Closes #1102

Post-script verification

  • Branch is not main/master (agent/1102-host-files-lifecycle-docs)
  • Secret scan passed (gitleaks — 48511880eaea5ef01f80b69ba4f228147611db33..HEAD)
  • PR body secret scan passed (gitleaks — no-git)

Add a "Harness execution lifecycle" section to the code-implementation
skill documenting the four-phase execution order: config load/validation,
pre-script, sandbox setup, agent execution. This addresses a systematic
documentation gap that caused a multi-day incident on PR #1075, where
the code agent added a host_files entry pointing to a file created by
a pre-script — the harness loader rejected the config at load time
because the file did not yet exist.

The new section documents that host_files entries are validated at
config load time (before pre-scripts run), so files created by
pre-scripts must use `optional: true` and dynamic path variables
like `${RUNNER_TEMP}` instead of hardcoded `/tmp`. Includes correct
and incorrect YAML examples.

Also updates docs/code.md to reflect the four-phase pipeline (adding
config load as step 1) with a cross-reference to the skill docs.

Regenerates .skillsaw-baseline.json to accommodate the increased
token count in the skill file.

Closes #1102
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 31, 2026 17:42
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:44 PM UTC · Completed 6:04 PM UTC

Commit: 68d854b · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.93

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Aug 31, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Docs-only PR by a known bot adding harness execution-order documentation to satisfy all acceptance criteria of a same-day issue; moderate score driven by the high churn and multi-author contention on the touched files (Tier 2), offset by zero security/CI/dependency signals, full acceptance criteria coverage, and trivial rollback risk.

Previous run

Risk Assessment: moderate (2/5)

Details

Docs-only PR by a known bot adding harness execution-order documentation to satisfy all acceptance criteria of a same-day issue; moderate score driven by the high churn and multi-author contention on the touched files (Tier 2), offset by zero security/CI/dependency signals, full acceptance criteria coverage, and trivial rollback risk.

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

Review

Findings

Medium

Previous run

Review

Findings

Medium

  • [protected-path] skills/code-implementation/SKILL.md — This PR modifies files under the protected path skills/: skills/code-implementation/SKILL.md. The PR links to issue Code-implementation skill should document harness host_files execution order constraints #1102 and provides rationale for the change (documenting harness execution order constraints to prevent recurrence of the PR feat(#1074): add Jira-source overlay to code harness #1075 incident). Human approval is always required for protected-path changes, regardless of context.

  • [stale-phase-count] README.md:40 — README.md describes the agent architecture as a "three-phase pipeline" (Pre-script, Sandbox, Post-script). This PR updates docs/code.md to describe a "four-phase pipeline" that adds "Config load" as step 1. The README's phase count and numbered list are now inconsistent with the documented code agent pipeline.
    Remediation: Update README.md line 40 to say "four-phase pipeline" and add a "Config load" step as item 1, renumbering the existing steps 2–4.

Low

  • [internal-consistency] skills/code-implementation/SKILL.md:126 — The documentation states that host_files entries are validated at config load time and that files created by pre-scripts must use optional: true. However, the Jira overlay in harness/code.yaml (lines 140–142) omits optional: true on its ${RUNNER_TEMP}/jira-issue-context.json entry — intentionally removed in commit 6ca729e. Overlay host_files (gated by when conditions) are likely validated after overlay resolution and pre-script execution, not at initial config load. The documentation oversimplifies by not distinguishing between top-level and overlay host_files validation timing.
    Remediation: Clarify the documentation to note that the config-load validation applies to top-level host_files entries, and that conditional overlay entries may be validated after overlay resolution.

  • [internal-consistency] harness/code.yaml:4 — The header comment in harness/code.yaml describes a three-phase flow ("Flow: pre_script → sandbox (agent) → post_script") that is inconsistent with the four-phase pipeline now documented in docs/code.md.
    Remediation: Update the header comment to include the config-load phase, or note that it describes only the script execution phases.


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

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ✅ Success · Started 6:06 PM UTC · Completed 6:14 PM UTC

Commit: 68d854b · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $1.41

- Update README.md architecture section from three-phase to four-phase
  pipeline, adding Config load as step 1
- Clarify host_files timing constraint in code-implementation skill to
  distinguish top-level entries (validated at config load) from
  conditional overlay entries (resolved after overlay evaluation)
- Update harness/code.yaml header comment to note config load occurs
  before the runtime flow

Addresses #1103
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

🔧 Fix agent — iteration 1 (bot-triggered)

Addressed 3 of 4 review findings with fixes: updated README.md phase count from three to four, clarified SKILL.md host_files timing to distinguish top-level vs overlay entries, and updated harness/code.yaml header comment to reference config load. Disagreed with the protected-path finding as it is informational and requires human approval which the fix agent cannot provide.

Fixed (3):

  1. stale-phase-count: README.md describes a three-phase pipeline inconsistent with the four-phase pipeline in docs/code.md (README.md): Updated README.md architecture section from 'three-phase pipeline' to 'four-phase pipeline' and added 'Config load' as step 1, renumbering existing steps to 2-4.
  2. internal-consistency: SKILL.md host_files documentation oversimplifies by not distinguishing top-level vs overlay validation timing (skills/code-implementation/SKILL.md): Clarified the host_files timing constraint to specify it applies to top-level entries. Added a paragraph noting that conditional overlay entries (under a when guard) are resolved after overlay evaluation and may have different validation timing, referencing the Jira-source overlay as an example.
  3. internal-consistency: harness/code.yaml header comment describes three-phase flow inconsistent with four-phase pipeline (harness/code.yaml): Updated the header comment to label the flow as 'Runtime flow' and added a note that config load + validation occurs before this flow, with a cross-reference to docs/code.md.

Disagreed (1):

  1. protected-path: skills/code-implementation/SKILL.md modified under protected path skills/: This is an informational finding noting that human approval is required for protected-path changes. The fix agent cannot grant human approval — this is handled by the existing review process. The PR author already acknowledges this requirement.

Tests: passed

Decision points
  • How to update the harness/code.yaml header comment (alternatives: Rewrite the entire flow to include config load as a step, Note that the comment describes only the runtime execution phases; rationale: Chose to label it 'Runtime flow' and add a parenthetical note about config load, since the comment describes the script execution pipeline and config load is not a script phase. This is the minimal change that resolves the inconsistency without restructuring the comment.)

Next steps:

  • /fs-review — request a re-review of the changes
  • /fs-fix <your instruction> — run another fix pass with specific guidance
  • Push commits directly — review re-runs automatically on push
    Updated by fullsend fix agent

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 31, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:16 PM UTC · Completed 6:37 PM UTC

Commit: d72eac4 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.05

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review August 31, 2026 18:37

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review requires-manual-review Review requires human judgment risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code-implementation skill should document harness host_files execution order constraints

0 participants