fix: require and retain post-action sampling evidence

[steipete]

A slow earlier frame could keep the sampling loop alive past post-roll while no valid sample began after the child command finished. capture action then reported success from elapsed loop time alone.

For positive post-roll, defer stopping until a valid post-action sample arrives, subject to the existing duration, frame, and size caps. If a cap prevents that sample, report incomplete coverage. Zero post-roll and immediate cancellation retain their existing stop behavior. Child execution and post-roll completion now have scoped helpers.

New action.json manifests retain canonical decimal-string nanosecond offsets for child completion and the last valid sample start, validate them against the millisecond timeline, and reject claimed valid post-roll with an earlier sample. Existing version-1 manifests remain readable as legacy elapsed-time evidence and do not gain a post-action sample proof; consumers requiring it must require the new field and verify the retained hash. No application or schema version bump.

Validation:

• A controlled regression with a real /usr/bin/true child and a delayed frame source failed before the fix. It now passes alongside capped-failure and explicit zero-post-roll cases.
• swift test --package-path Apps/CLI --filter 'CaptureActionCommandEndToEndTests|CaptureActionSampleBoundaryTests|CaptureActionTimingTests': 22 tests passed, including exact decimal encoding, one-nanosecond ordering, malformed offsets, and legacy decoding.
• swift test --package-path Core/PeekabooAutomationKit --filter WatchCaptureCadenceSchedulingTests: 9 tests passed, including deferred/immediate stop boundaries and existing cadence/deadline contracts.
• Complete isolated Codex autoreview is clean at P0–P2. SwiftFormat, SwiftLint (zero violations), docs lint, and diff checks passed.
• Built the clean commit with scripts/build-swift-debug.sh, using the matching Developer ID. The signed, source-stamped CLI ran a real local classic screen capture with a real child:

  ./peekaboo capture action --mode screen --no-remote --capture-engine classic \
    --duration-limit 15s --pre-roll 250ms --post-roll 100ms \
    --idle-fps 0.1 --active-fps 0.5 --heartbeat 100ms --threshold 100 \
    --max-frames 5 --path /tmp/peekaboo-postroll-proof --json -- /usr/bin/true

  Exit 0; child exit 0; two retained frames. The manifest bound source 6057e54b05b923e53d261e4eab9d2b0f22250064, action completion offset 295337959 ns, and last sample start 2309033209 ns. The latter is after the action. Desktop images stayed local and were removed after structural verification.
• Exact-head hosted checks must pass before merge. Changelog and capture-action documentation updated.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[cursor]

PR Summary

Medium Risk
Changes when capture action succeeds and what action.json proves about post-roll timing; behavior shifts for positive post-roll, though caps, zero post-roll, and legacy manifests are preserved.

Overview
Fixes capture action reporting success after post-roll when only elapsed sampling time passed, even if no valid frame started after the child finished (e.g. a slow in-flight frame could extend the loop without a post-action sample).

For positive --post-roll, capture now waits for a valid sample whose start is at or after action completion—still bounded by duration, frame, and size caps—via WatchCaptureSession.requestStop(afterSampleStartedAtOrAfter:) and tracking lastSampleStartedAtMonotonicNanoseconds. --post-roll 0 keeps immediate stop. Validation fails with a clear error when caps block the required sample.

New manifests add optional timeline.sampleBoundary (canonical decimal nanosecond offsets from capture start), semantic checks that tie offsets to the millisecond timeline, and provesPostActionSample. Older version-1 manifests without the field remain decodable as legacy elapsed-time evidence. Child run and post-roll finish are refactored into runChildAction / finishPostRoll helpers; docs and changelog describe the contract.

^{Reviewed by Cursor Bugbot for commit 6057e54. Bugbot is set up for automated code reviews on this repo. Configure here.}

[clawsweeper]

Codex review: blocked before merge. Reviewed September 12, 2026, 9:03 PM ET / September 13, 2026, 01:03 UTC (Revision 2).

ClawSweeper review

What this changes

Require a valid post-action sample for positive post-roll and retain precise sample timing in backward-compatible capture manifests.

Merge readiness

⛔ Blocked before merge - 1 item remains

The fix remains necessary: current main and v4.3.4 still accept elapsed post-roll without proving a post-action sample. No actionable patch defect was found, and the supplied native proof remains relevant to the unchanged implementation.

Priority: P2
Reviewed head: 6057e54b05b923e53d261e4eab9d2b0f22250064

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with relevant native proof, boundary coverage, documented compatibility behavior, and no actionable findings.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (live_output): The captured body reports a signed native CLI exercising real local screen capture and a real child, with retained offsets showing a post-action sample; the relevant production files are unchanged from that proven revision. Controlled regressions supplement this with capped failure, zero post-roll, and legacy decoding.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (live_output): The captured body reports a signed native CLI exercising real local screen capture and a real child, with retained offsets showing a post-action sample; the relevant production files are unchanged from that proven revision. Controlled regressions supplement this with capped failure, zero post-roll, and legacy decoding.
Evidence reviewed	7 items	Current main still lacks the sample requirement: Main sleeps until the post-roll deadline, requests an unconditional stop, and validates elapsed sampling time without a sample-start boundary. Latest release retains the same behavior: The v4.3.4 source contains the same unconditional post-roll stop; the release endpoint confirms this published release. Valid samples drive deferred stopping: The loop records sample start only after obtaining a valid image. Deferred stopping preserves the existing duration, frame, size, and immediate-stop paths.
Findings	None	None.
Security	None	None.

How this fits together

Peekaboo’s action capture runs a child command while sampling the desktop, then validates and publishes capture artifacts. The sampling loop supplies timing evidence used by the CLI to determine success and write the action manifest.

flowchart TD
    A[Capture options] --> B[Desktop sampling loop]
    C[Child command completion] --> D[Post-roll stop decision]
    B --> D
    D --> E[Sample and artifact validation]
    E --> F[Result and action manifest]

Loading

Before merge

• Resolve merge risk (P1) - Existing scripts using positive post-roll can take longer or receive incomplete-coverage failure where elapsed-time validation previously succeeded; this intentional compatibility change is documented and covered by capped-failure and zero-post-roll cases.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +180/-32; tests +175/-3	Production growth implements retained timing evidence and deferred stopping, with focused boundary and compatibility coverage.

Merge-risk options

Maintainer options:

1. Retain the documented coverage correction (recommended)
   Accept the explicitly intended longer wait or incomplete-coverage result for positive post-roll while preserving capture caps, zero post-roll, and legacy decoding.

Technical review

Best possible solution:

Retain the documented stronger sampling guarantee, bounded capture limits, explicit zero-post-roll behavior, and readable legacy manifests without granting them new proof.

Do we have a high-confidence way to reproduce the issue?

Yes: current main’s elapsed-only validation and the delayed-frame regression establish the failure path; this read-only review did not execute it.

Is this the best way to solve the issue?

Yes: tracking valid sample starts in the existing sampler directly repairs the evidence gap, and the optional manifest field preserves legacy decoding without overstating old evidence.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 01b8b3118043.

Labels

Label justifications:

• P2: This corrects a bounded false-success condition in action-capture evidence.
• merge-risk: 🚨 compatibility: Positive post-roll can now wait longer or fail incomplete coverage for existing callers that previously succeeded from elapsed time alone.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The captured body reports a signed native CLI exercising real local screen capture and a real child, with retained offsets showing a post-action sample; the relevant production files are unchanged from that proven revision. Controlled regressions supplement this with capped failure, zero post-roll, and legacy decoding.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured body reports a signed native CLI exercising real local screen capture and a real child, with retained offsets showing a post-action sample; the relevant production files are unchanged from that proven revision. Controlled regressions supplement this with capped failure, zero post-roll, and legacy decoding.

Evidence

What I checked:

• Current main still lacks the sample requirement: Main sleeps until the post-roll deadline, requests an unconditional stop, and validates elapsed sampling time without a sample-start boundary. (Apps/CLI/Sources/PeekabooCLI/Commands/Core/CaptureCommand+Action.swift:257, 01b8b3118043)
• Latest release retains the same behavior: The v4.3.4 source contains the same unconditional post-roll stop; the release endpoint confirms this published release. (Apps/CLI/Sources/PeekabooCLI/Commands/Core/CaptureCommand+Action.swift:257, 59a805b253c1)
• Valid samples drive deferred stopping: The loop records sample start only after obtaining a valid image. Deferred stopping preserves the existing duration, frame, size, and immediate-stop paths. (Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/WatchCaptureSession+Loop.swift:212, 6057e54b05b9)
• Manifest compatibility and regression coverage: Tests cover delayed earlier frames, cap-induced failure, explicit zero post-roll, rejection of reversed sample ordering, legacy manifests without sampleBoundary, and exact decimal encoding above JSON integer precision. (Apps/CLI/Tests/CoreCLITests/CaptureActionPostRollTests.swift:8, 6057e54b05b9)
• Supplied native behavior proof: The captured PR body reports a signed, source-stamped CLI running local classic screen capture with /usr/bin/true: exit 0, two retained frames, action completion at 313152625 ns and last sample start at 4515359708 ns. This exercises the changed action-capture and sampling owners. Evidence sourceRevision: 68e8190286d1d6d29259f0dad87701b4b1e8b31e3187eaa0a90b6c3bb2e1ced1. This is contributor-reported live output, not a reviewer-executed run. (00965c92b0d7)
• Proof and previous-review continuity: No differences exist between the previously reviewed/proven revision and HEAD in the four changed production files. The previous review contained no findings or published rank-up moves; its compatibility observation is already addressed as intentional behavior in the body and docs. (6057e54b05b9)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-09-13T00:29:51.288Z sha 00965c9 :: blocked before merge. :: none

[clawsweeper]

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix: require and retain post-action sampling evidence This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.