Skip to content

feat(#16): final output validation and artifact retention - #124

Draft
qinhaihong-red wants to merge 2 commits into
mainfrom
feat/issue-16-artifact-hardening
Draft

feat(#16): final output validation and artifact retention#124
qinhaihong-red wants to merge 2 commits into
mainfrom
feat/issue-16-artifact-hardening

Conversation

@qinhaihong-red

@qinhaihong-red qinhaihong-red commented Jun 18, 2026

Copy link
Copy Markdown
Member

Implements the #16 v0.1 hardening slice. With this change, every #16 acceptance criterion is met.

What's in this change

Slice 1 — report-time final output validation (feat(report))

  • Adds top-level final_output (node, field, schema) so a Workflow can declare the persisted output that reports should treat as the final result.
  • caw report validates that value against the declared JSON Schema at report time and renders the validation result in JSON, JSONL, text, and Markdown.
  • Run Group reports now carry each iteration's final-output validation through JSON, JSONL, text, and Markdown.
  • Schema violations render clearly as diagnostic report content without hiding the run conclusion or trace evidence.

Slice 2 — real adapter artifact capture + run-owned collection (feat(adapters), feat(executor))

  • claude.print and codex.exec now report files created or modified by a writable Agent CLI run.
  • Artifact discovery is scoped to the node-owned working directory for that invocation, so concurrent agent nodes do not claim each other's changed files from a shared cwd.
  • The executor collects those files into artifacts/<node-id>/ inside the run directory and persists only those run-owned copies in State.
  • Missing paths and directories are still ignored, so State never claims a non-durable artifact.

Slice 3 — configurable artifact retention (feat(executor))

  • Adds artifact_cleanup.keep_last_runs as a conservative keep-last-N policy.
  • The default is no cleanup.
  • Retention ordering uses persisted run creation time, not mutable run directory mtime.
  • When configured, cleanup prunes only old run artifact directories, never the active run and never run State or Events.

Slice 4 — sample and docs sync (docs)

  • The fan-out synthesis mock and real samples now declare final_output.
  • README, PRD, ADR 0006, quickstart, and the glossary describe the new report and artifact lifecycle surface.

Tests / gates

  • New: final-output valid and invalid report seams; Run Group final-output validation in all report formats; run-owned artifact collection; node-scoped subprocess artifact capture; phantom/directory artifact filtering; default no-cleanup behavior; keep-last-N cleanup preserving the active run and newest retained runs; real-adapter artifact capture for both claude.print and codex.exec; shipped real fan-out workflow report evidence for final output, artifacts section, and trace.
  • env UV_CACHE_DIR=/tmp/caw-uv-cache uv run ruff check
  • env UV_CACHE_DIR=/tmp/caw-uv-cache uv run ruff format --check
  • env UV_CACHE_DIR=/tmp/caw-uv-cache uv run mypy
  • env UV_CACHE_DIR=/tmp/caw-uv-cache uv run pytest -m 'not e2e' — 514 passed, 14 deselected
  • env UV_CACHE_DIR=/tmp/caw-uv-cache CAW_E2E_AGENT=codex uv run pytest -m e2e — 14 passed, 514 deselected (passed outside the sandbox; the sandboxed run failed because Codex CLI could not initialize its app-server client with Operation not permitted)

#16 acceptance criteria — all met

  • Configurable artifact cleanup policy with a conservative default that never touches the active run
  • Final workflow outputs are validated against a declared schema and violations are reported clearly
  • A real repository workflow produces a reviewable report with trace evidence
  • Real Adapters (claude.print, codex.exec) capture files a writable agent run produces and populate AgentResult.artifacts symmetrically

Closes #16.

Add report-time final_output schema validation, collect real adapter artifacts into run-owned paths, and support artifact_cleanup.keep_last_runs with a conservative default.

Fixes: #16
@qinhaihong-red

Copy link
Copy Markdown
Member Author

Review pass: Standards / Spec

Standards

  1. Required: Artifact cleanup does not reliably preserve the newest retained runs. The documented contract says artifact_cleanup.keep_last_runs retains artifacts for the newest N runs while preserving the current run (docs/prd/0001-cli-agentic-workflow.md, docs/adr/0006-adapter-interface-contract.md). The implementation sorts prior run directories by mutable directory mtime, then removes run_dir / "artifacts" from pruned runs (src/caw/executor.py:1182-1188). Removing that child directory updates the old run directory's mtime, so a later cleanup can treat an already-pruned old run as newer than an actually newer run and prune the newer run's artifacts. I reproduced the sequence with keep_last_runs=2: after run C, B and C were retained; after creating run D, C's artifacts were pruned and only D's artifacts remained. This violates the "newest N runs" retention rule.

  2. Required: Run Group reports drop per-run Final Output validation. CONTEXT.md defines Final Output as a report-time validation result rendered alongside the run conclusion and trace evidence, and ADR 0009 says a group report wraps each iteration's per-run report into one Run Group result. _gather_group() calls _gather(run_dir), which computes final_output, but then copies only iteration_index, run_id, status, nodes, graph, and trace into each iteration (src/caw/report.py:292-301). The group JSON/JSONL/text/Markdown renderers therefore cannot show Final Output validation for workflows run through Pattern Controllers.

Spec

  1. Required: Artifact capture is not scoped to the agent node that produced the file. The issue comment says real adapters should "capture the files a writable agent run produces and populate AgentResult.artifacts". SubprocessAdapter.run_cli() snapshots Path.cwd() before and after each subprocess (src/caw/subprocess_adapter.py:248-273), while the scheduler can launch independent nodes concurrently (src/caw/executor.py:753-773). If two real agent nodes run in parallel from the same working directory, each adapter can observe files created or modified by the other node during its subprocess window and report them as its own artifacts. State can then index the same produced file under the wrong node, which breaks the spec's per-agent-run artifact ownership.

  2. Required verification gap: The acceptance criteria include "A real repository workflow produces a reviewable report with trace evidence." The shipped real fan-out workflow now declares final_output, but the real e2e test stops after asserting State rows and does not render caw report or verify the real workflow report contains the final-output validation, artifacts, and trace evidence (tests/e2e/test_fanout_synthesis_runs.py:133-151). The new report assertions are offline/mock, so this criterion is only partially covered for the real repository workflow.

Summary: Standards found 2 required issues; Spec found 2 required issues. The highest-risk issue is artifact capture/retention misattribution because it can persist incorrect trace evidence for parallel real-agent runs.

@qinhaihong-red

Copy link
Copy Markdown
Member Author

Code review and quality pass

Findings

  • Critical: artifact_cleanup.keep_last_runs can delete artifacts from a newer retained run after an earlier cleanup pass. src/caw/executor.py:1182 sorts old runs by directory mtime, then src/caw/executor.py:1188 removes the artifacts/ child from pruned runs. That removal mutates the old run directory mtime, so on a later run an already-pruned old directory can sort ahead of a newer run. With keep_last_runs=2, I reproduced: after run C, B and C artifacts were retained; after run D, C artifacts were deleted and only D remained. This is correctness/data-loss behavior for the retention feature. Use an immutable ordering source, such as the run id timestamp or the State run.created_at, instead of mutable directory mtime.

  • src/caw/subprocess_adapter.py:248 captures artifacts by diffing the entire shared Path.cwd() before and after the subprocess. Because src/caw/executor.py:753-773 can run independent agent nodes concurrently, two real agents in the same working directory can observe each other's changed files and both report them as their own AgentResult.artifacts. That breaks artifact ownership and can persist misleading trace evidence under the wrong node. The capture boundary needs to be node-specific, or the adapter needs a way to distinguish files produced by that invocation rather than every cwd change during the time window.

  • src/caw/report.py:292 computes each iteration report with _gather(run_dir), but src/caw/report.py:293-301 drops the resulting final_output field before group rendering. Single-run reports now include final-output validation, but workflows executed through Pattern Controllers lose that validation in JSON, JSONL, text, and Markdown group reports. Since group reports are meant to wrap the per-run report shape, carry final_output through each iteration and render it consistently.

  • tests/e2e/test_fanout_synthesis_runs.py:133-151 still stops at State assertions for the real repository workflow. The PR adds mock/offline report assertions, but the issue asks for a real repository workflow to produce a reviewable report with trace evidence. Add a real e2e assertion that renders caw report for the shipped real fan-out workflow and checks the report includes the final-output validation plus trace evidence.

Axis notes

  • Correctness: blocked by the retention ordering bug, shared-cwd artifact attribution, and dropped group-report final output.
  • Readability/Simplicity: no additional readability issues found beyond the capture boundary being implicit rather than represented in the model.
  • Architecture: the artifact lifecycle is split cleanly between adapter discovery and kernel collection, but the discovery boundary is too broad for concurrent node execution.
  • Security: no direct secret exposure was found in the diff, but broad cwd artifact capture increases the chance of persisting unrelated generated files.
  • Performance: full-tree cwd snapshots happen before and after every real CLI invocation; this may become expensive in large repositories, and a node-scoped artifact root would reduce both cost and false positives.
  • Verification: GitHub checks gates (3.12) and gates (3.13) are passing. I also ran an isolated /tmp reproduction for the cleanup ordering issue.

Verdict

Request changes. The implementation adds useful hardening surface, but artifact ownership/retention and group-report final-output behavior need fixes before merge.

@qinhaihong-red qinhaihong-red changed the title [codex] Add final output validation and artifact retention feat(#16): final output validation and artifact retention Jun 18, 2026
Use persisted run creation time for artifact retention so pruning old artifact directories cannot mutate retention ordering.

Scope real-adapter artifact discovery to a node-owned working directory, carry final-output validation through Run Group reports, and assert the shipped real fan-out workflow renders final output and trace evidence.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hardening: artifact cleanup policy and final output schema validation

1 participant