test(capture): decouple completion proof from frame cadence

[steipete]

Main CI's delayed-runner test assumed that a valid frame would begin in the final 150 ms of capture. Under load, the last frame began earlier and finished after the cap, so the new post-action evidence check correctly refused success. That coupled the runner-delivery regression to frame cadence.

Keep the 2.5-second cap and deliberately deliver the runner result after its completion deadline, but record completion early enough to leave sampling headroom. Preserve the success and elapsed-coverage assertions, and add explicit checks for late delivery, the recorded completion boundary, and retained post-action sample proof.

Deadline-edge coverage now calls the same throwing admission helper as production for 50 ms before, exactly at, one nanosecond after, 20 ms after, and overflowing boundaries. Remove the unused Boolean helper that only tests called. Production behavior and error messages are unchanged.

Validation: 22 focused capture tests passed, lint/format/diff checks passed, and isolated Codex review found no actionable P0–P2 findings. The signed, source-stamped CLI at 859b64176b79a437a874f2382d176ecc0da2b8e9 ran a real /usr/bin/true child during local classic screen capture: exit 0, two retained frames, child completion 275988000 ns, last sample start 2032758291 ns, and valid retained post-action proof. Captured images were removed after structural verification.

Failure evidence: https://github.com/openclaw/Peekaboo/actions/runs/34733287499 (CLI job; Capture action admits post roll from the runner completion boundary). No assertion, timeout, capture cap, or CI gate was removed or weakened.

[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

Low Risk
Small timing-helper refactor with equivalent validation messages; remaining risk is limited to capture-action deadline edge cases covered by updated tests.

Overview
Post-roll admission is consolidated: postRollDeadline now takes captureDeadlineNs and throws when the computed post-roll end exceeds the capture cap (same message as before). finishPostRoll delegates to that helper instead of a separate guard, and the unused postRollFits boolean helper is removed.

Tests decouple the “late runner return” scenario from frame cadence: the stub records action completion at launch while still returning after the completion deadline, and asserts late delivery, the recorded completion boundary, and provesPostActionSample. Timing edge cases now exercise the throwing postRollDeadline API at several boundaries instead of postRollFits.

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

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 12, 2026, 11:33 PM ET / September 13, 2026, 03:33 UTC.

ClawSweeper review

What this changes

The PR gives a delayed-result capture test more sampling headroom and consolidates post-roll deadline validation so boundary tests exercise the production helper.

Merge readiness

✅ Ready for maintainer review

Keep open: this is a focused test reliability improvement absent from the pinned main revision, with no actionable correctness findings. The related merged capture fix does not supersede it.

Priority: P3
Reviewed head: 859b64176b79a437a874f2382d176ecc0da2b8e9

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, behavior-preserving repair with relevant native observations and stronger targeted assertions.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (live_output): The captured body records the exact head exercising production capture-action post-roll completion with a real child and classic screen capture, yielding success and a retained sample after completion. Focused tests separately cover delayed delivery and deadline edges.
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 records the exact head exercising production capture-action post-roll completion with a real child and classic screen capture, yielding success and a retained sample after completion. Focused tests separately cover delayed delivery and deadline edges.
Evidence reviewed	6 items	Introduced change preserves production validation: The complete pinned diff moves the existing capture-cap guard into the throwing deadline helper without changing arithmetic, error messages, or subsequent stop behavior. Regression assertions remain meaningful: The runner still returns after its completion deadline; the test now explicitly checks late delivery, the recorded completion boundary, retained post-action sample proof, and elapsed post-roll coverage. Five arithmetic boundaries exercise the production helper. Existing coverage contract: Documentation requires a valid sample begun after child completion for positive post-roll, while preserving duration, frame, and size caps. Separate slow-frame tests retain capped-failure and explicit zero-post-roll coverage.
Findings	None	None.
Security	None	None.

How this fits together

Peekaboo’s capture action command records the screen around a child process. It combines recorded process completion, capture deadlines, and retained sample timing to decide whether the resulting artifacts prove post-action coverage.

flowchart LR
  A[Child command] --> B[Recorded completion time]
  B --> C[Post-roll deadline validation]
  D[Capture duration cap] --> C
  C --> E[Screen sampling]
  E --> F[Coverage validation]
  F --> G[Result and artifact manifest]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

None.

Technical review

Best possible solution:

Keep deadline arithmetic coverage separate from frame scheduling while preserving production capture caps and retained post-action evidence requirements.

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

No deterministic live reproduction was established during this read-only review. Main’s fixture leaves only 150 ms for a qualifying sample, supporting the reported scheduling-dependent failure; GitHub confirms the cited CLI test job failed.

Is this the best way to solve the issue?

Yes. Separating delayed delivery from sampling cadence retains the regression’s purpose, and moving the unchanged guard lets arithmetic tests exercise the production admission path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 8ecf8a9159ed.

Labels

Label changes:

• add P3: This is a bounded test reliability repair with behavior-preserving production refactoring.
• add proof: sufficient: Contributor real behavior proof is sufficient. The captured body records the exact head exercising production capture-action post-roll completion with a real child and classic screen capture, yielding success and a retained sample after completion. Focused tests separately cover delayed delivery and deadline edges.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The captured body records the exact head exercising production capture-action post-roll completion with a real child and classic screen capture, yielding success and a retained sample after completion. Focused tests separately cover delayed delivery and deadline edges.

Label justifications:

• P3: This is a bounded test reliability repair with behavior-preserving production refactoring.
• 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 records the exact head exercising production capture-action post-roll completion with a real child and classic screen capture, yielding success and a retained sample after completion. Focused tests separately cover delayed delivery and deadline edges.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured body records the exact head exercising production capture-action post-roll completion with a real child and classic screen capture, yielding success and a retained sample after completion. Focused tests separately cover delayed delivery and deadline edges.

Evidence

What I checked:

• Introduced change preserves production validation: The complete pinned diff moves the existing capture-cap guard into the throwing deadline helper without changing arithmetic, error messages, or subsequent stop behavior. (Apps/CLI/Sources/PeekabooCLI/Commands/Core/CaptureCommand+Action.swift:1040, 859b64176b79)
• Regression assertions remain meaningful: The runner still returns after its completion deadline; the test now explicitly checks late delivery, the recorded completion boundary, retained post-action sample proof, and elapsed post-roll coverage. Five arithmetic boundaries exercise the production helper. (Apps/CLI/Tests/CoreCLITests/CaptureActionCommandEndToEndTests.swift:993, 859b64176b79)
• Existing coverage contract: Documentation requires a valid sample begun after child completion for positive post-roll, while preserving duration, frame, and size caps. Separate slow-frame tests retain capped-failure and explicit zero-post-roll coverage. (Apps/CLI/Tests/CoreCLITests/CaptureActionPostRollTests.swift:8, 859b64176b79)
• Related merged work and routing: GitHub confirms fix: require and retain post-action sampling evidence #723 merged as 6c639be. It establishes the post-action sampling contract; this PR repairs a test fixture affected by that contract. Main-path history also shows repeated capture work by Peter Steinberger. (Apps/CLI/Sources/PeekabooCLI/Commands/Core/CaptureCommand+Action.swift, 6c639be4f30d)
• Recorded after-fix native behavior: The supplied complete PR body records a signed, source-stamped head running a real /usr/bin/true child during local classic screen capture: exit 0, two retained frames, completion offset 275988000 ns, and last sample start 2032758291 ns. This exercises production post-roll completion; delayed delivery and exact deadline edges are covered separately by the reported 22 focused tests. (859b64176b79)
• Inspection scope and provenance limits: Root AGENTS.md was read fully; no nested AGENTS.md or maintainer-notes directory was found. Local source and main comparison succeeded. Historical blame could not retrieve a required object, and the local v4.3.4 source lookup was unavailable, so no exact line-introduction or shipped-fix claim is made. No builds, tests, or target review helpers were executed; final git status was clean. (AGENTS.md:1, 859b64176b79)

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.