[EVI-11]: fix capture-pr video artifacts

[riddim-developer-bot]

Summary

• drain command stdout/stderr while child processes run to avoid noisy build stalls
• stop simulator video recording with SIGINT so QuickTime files finalize cleanly
• validate video artifacts before they enter the manifest and report missing implicit whole-flow videos

Verification

• swift test passed: 123 tests, 2 skipped, 0 failures
• swift build passed
• Re-ran EPAC PR #479 repro with the fixed CLI; both before/home-status-card.mov and after/home-status-card.mov were produced with non-zero manifest sizes

Linear

• EVI-11

[linear-code]

EVI-11 Fix capture-pr video artifacts and noisy build hangs

Context / background

Parent Project: Evidence PR Change Proof MVP (0c4adda0-5d2d-4ee1-b4bd-717499212ec6). The EVI-10 human handoff ran a real local capture-pr demo against RiddimSoftware/epac PR #479 using the new MVP. The generated screenshots, side-by-side comparison image, manifest, build logs, and report worked, but the video path is not reliable enough to close the MVP.

Root cause analysis

Likely root-cause candidate 1: ProcessCommandRunner waits for a child process to exit before draining stdout/stderr. The initial EPAC #479 run without ios.extra_build_arguments: ["-quiet"] left xcodebuild idle after producing the before app bundle, which is consistent with a pipe buffer deadlock when a child emits enough output.

Likely root-cause candidate 2: SimctlVideoRecorder.stop(_:) terminates the simctl io recordVideo process and waits, but it does not validate the exit status or verify that the expected video file exists and has non-zero size. The handoff run with video.enabled = true produced before/home-status-card.mov on disk, recorded an after/home-status-card.mov artifact in manifest.json, but did not create the after video file.

Likely root-cause candidate 3: the report renderer does not make video artifact completeness visible. report.md showed screenshot comparisons and step status, but did not surface the missing after video even though the manifest contained a video artifact entry.

Concrete evidence from EVI-10 handoff run on 2026-05-18 America/Toronto:

• Command: /Users/sunny/code/evidence/.build/debug/evidence capture-pr --repo RiddimSoftware/epac --pr 479 --plan /Users/sunny/code/evidence-handoff/evi-10/epac-pr479-plan.json --output /Users/sunny/code/evidence-handoff/evi-10/epac-pr479-capture
• Report path: /Users/sunny/code/evidence-handoff/evi-10/epac-pr479-capture/report.md
• Manifest path: /Users/sunny/code/evidence-handoff/evi-10/epac-pr479-capture/manifest.json
• Before video exists: /Users/sunny/code/evidence-handoff/evi-10/epac-pr479-capture/before/home-status-card.mov, 9,391,567 bytes, 17.185 seconds by ffprobe.
• After video missing: /Users/sunny/code/evidence-handoff/evi-10/epac-pr479-capture/after/home-status-card.mov did not exist even though manifest.json listed it as an after video artifact.
• Screenshots exist: before and after home-status-card.png plus comparisons/home-status-card.png.

Use this representative plan shape for reproduction:

{
  "repo": "RiddimSoftware/epac",
  "pr": 479,
  "platform": "ios",
  "runner": "simctl",
  "ios": {
    "project": "ios/epac.xcodeproj",
    "scheme": "epac",
    "bundle_id": "net.dinglebox.cabinetdoor",
    "simulator_udid": "<available-iPhone-simulator-udid>",
    "destination": "platform=iOS Simulator,id=<available-iPhone-simulator-udid>",
    "extra_build_arguments": ["-quiet"]
  },
  "launch": {
    "arguments": ["--evidence-mode", "-UIAnimationsDisabled", "YES"],
    "environment": { "EPAC_EVIDENCE_MODE": "1" }
  },
  "video": { "enabled": true, "name": "home-status-card" },
  "steps": [
    { "name": "launch home", "kind": "launch" },
    { "name": "settle home", "kind": "wait", "seconds": 4 },
    { "name": "home status card", "kind": "screenshot", "path": "home-status-card.png" }
  ]
}

Acceptance criteria

• Given ProcessCommandRunner runs a child process that writes more stdout/stderr than the platform pipe buffer, when the command is still running, then Evidence drains output concurrently and does not deadlock waiting for process exit.
• Given an iOS capture-pr plan uses video.enabled = true with no explicit video steps, when both before and after runs succeed, then both <output>/before/<name>.mov and <output>/after/<name>.mov exist on disk and have non-zero size before the manifest is written.
• Given a video recorder cannot create or finalize a declared video artifact, when capture-pr completes the affected revision, then the command fails with a named failure instead of writing a successful manifest that points at a missing file.
• Given the manifest records video artifacts, when report.md is rendered, then the report lists the video artifacts or clearly names any missing video artifact.
• Given the representative EPAC PR #479 plan above is run locally with a real simulator and -quiet, when the command exits, then manifest.json, report.md, before screenshot, after screenshot, comparison image, before video, and after video are all present.

External validation gates

None. The implementer can reproduce with the local EPAC repo and simulator tooling; EVI-10 owns the final human closeout after this fix lands.

Out of scope

• Adding arbitrary UI interaction support beyond the existing simctl and xctest runner contracts.
• Changing the PR SHA resolution behavior.
• Changing the Evidence Action public inputs except where required to reflect fixed report/video behavior.
• Creating an EPAC consuming-app integration project.

Inputs / dependencies

Repository context: EVI team owns RiddimSoftware/evidence.

Local reproduction requires:

• /Users/sunny/code/evidence on main.
• /Users/sunny/code/epac on main with PR #479 history available from origin.
• Xcode and an available iPhone simulator.
• ffprobe or equivalent optional video validation tooling for local inspection.

EVI-10 is blocked by this issue because its verification checklist includes videos when enabled.

Risks / notes for implementer

Do not require plan authors to pass -quiet as the only workaround for deadlock risk. The command runner should be robust for noisy tools because Evidence intentionally wraps xcodebuild, simctl, ffmpeg, ImageMagick, and Node.

Be careful with simctl io recordVideo: process termination semantics matter. Prefer the stop signal that reliably finalizes QuickTime output, then verify file existence and size before appending the artifact.

Definition of Done

Done when unit tests cover concurrent process output draining, video artifact validation, and report visibility, and a real or representative local capture-pr run can produce a complete before/after screenshot and video bundle without stale success metadata.

Architecture Impact

Affected layers: framework adapters and report rendering in EvidenceCLIKit.

Intended dependency direction: CapturePullRequestEvidence remains the use-case orchestrator. Process execution, video recording, and report rendering stay behind adapter/protocol boundaries and do not leak simulator or Process details into value objects.

Test strategy: unit-test ProcessCommandRunner with a noisy child process; unit-test SimctlVideoRecorder/plan execution with fake recorders that fail, omit files, or write zero-byte files; unit-test report output for complete and missing video artifacts.

Clean Architecture Shape

Use case:

• Changed: CapturePullRequestEvidence must not claim success for missing declared video artifacts.

Entities / value objects:

• PRChangeEvidenceManifest
• CapturedArtifact
• CaptureStepResult
• PRChangeEvidenceFailureSummary

Ports:

• CommandRunning
• VideoRecording
• PullRequestEvidenceReporting

Adapters:

• ProcessCommandRunner
• SimctlVideoRecorder
• RenderPullRequestEvidenceReport

Boundary rule:

• Process and simctl details must stay in adapters; manifest/report value objects should contain artifact state, not shell-process policy.

Catalog update:

• Update docs/architecture/use-case-catalog.md if the artifact validation responsibility changes the documented CapturePullRequestEvidence or ExecuteEvidencePlanAgainstRevisions boundary.

Mergeability / change ownership

Single reason to change: make capture-pr output truthful and complete for videos and noisy build tools. Primary code owner: Evidence CLI runtime adapters. Expected hot files/modules: Sources/EvidenceCLIKit/CommandRunner.swift, Sources/EvidenceCLIKit/EvidencePlanExecution.swift, Sources/EvidenceCLIKit/PullRequestEvidenceReport.swift, and matching tests under Tests/EvidenceCLIKitTests/. Sibling collisions: none known after EVI-1 through EVI-9 completed. Conflict risk: medium — this touches shared command execution and report rendering. Sequencing lane: serial, blocks EVI-10 closeout.

Review in Linear