fix(capture): honor cancellation during ScreenCaptureKit retry sleep

[SebTardif]

What Problem This Solves

Transient ScreenCaptureKit denial handling in ScreenCaptureFallbackRunner (image runCapture and generic run paths) sleeps with try? await Task.sleep before retrying the capture. try? swallows CancellationError, so a canceled CLI/MCP capture can still enter a second full capture attempt after the delay instead of stopping promptly.

This is the same cancellation-swallow class as merged #270 / #271 / #230 / #267.

Evidence

Before (main)

try? await Task.sleep(nanoseconds: delay)
// then attempt(api) again — cancel is swallowed

After (this branch)

try await Task.sleep(nanoseconds: delay)
try Task.checkCancellation()

Live terminal transcript (after fix)

Pattern proof (scripts/prove-screencapture-engine-cancel.swift) on macOS 26.5.2, cancel 50ms into the 350ms denial sleep:

$ swiftc -parse-as-library -o /tmp/prove-sck-engine scripts/prove-screencapture-engine-cancel.swift
$ /tmp/prove-sck-engine
=== ScreenCaptureKit engine transient-denial cancel proof (PR #279) ===
Pattern matches ScreenCaptureFallbackRunner.run / runCapture

Test 1: Unfixed (try? sleep) — cancel after 50ms into 350ms delay
  attempts=2 elapsed_ms=55 outcome=retry-failed
Test 2: Fixed (try sleep + checkCancellation) — cancel after 50ms into 350ms delay
  attempts=1 elapsed_ms=52 outcome=cancelled

PASS: unfixed burned a second capture attempt after cancel (2 attempts)
PASS: fixed cancelled during denial sleep without second attempt (1 attempt, 52ms)

Production helper exercised (same host, branch 4e12fa53):

$ swift test --package-path Core/PeekabooCore --filter ScreenCaptureFallbackRunnerTests
✔ cancel during transient denial sleep skips second generic attempt
✔ cancel during transient denial sleep skips second capture attempt
✔ transient ScreenCaptureKit denial retries before fallback
✔ Suite ScreenCaptureFallbackRunnerTests passed after 0.363 seconds
✔ Test run with 6 tests in 1 suite passed after 0.364 seconds

Both image (runCapture) and generic/video-style (run) paths assert attempts == 1 when canceled mid-sleep.

Real behavior proof

• Behavior or issue addressed: Canceling during a ScreenCaptureKit transient-denial sleep must not run a second capture attempt (image and video-style runners).

• Real environment tested: macOS 26.5.2 (Build 25F84), arm64, Peekaboo worktree at commit 4e12fa53 (this PR), Swift toolchain 6.x.

• Exact steps or command run after this patch:

  swiftc -parse-as-library -o /tmp/prove-sck-engine scripts/prove-screencapture-engine-cancel.swift
  /tmp/prove-sck-engine
  swift test --package-path Core/PeekabooCore --filter ScreenCaptureFallbackRunnerTests

• Evidence after fix: Terminal transcript above: unfixed path runs 2 attempts after cancel; fixed path stops at 1 attempt with cancelled in ~52ms. Focused production-helper runs on ScreenCaptureFallbackRunner also keep attempts at 1 for both run and runCapture.

• Observed result after fix: Cancellation during the 350ms denial sleep aborts before the retry attempt(api). Non-cancelled transient denial still retries once (existing green case).

• What was not tested: Live ScreenCaptureKit TCC denial from a real display capture under a canceled CLI task (needs TCC denial injection on a signed binary). Retry delay duration and cancel contract match production ScreenCaptureKitTransientError (350ms).

Summary

• Fix both transient SCK retry sleeps in ScreenCaptureEngineSupport / ScreenCaptureFallbackRunner.
• Add focused cancel-during-sleep coverage for run and runCapture.
• Standalone red/green prove script for terminal proof.
• Changelog credit for @SebTardif.

Related: #271, #270, #267, #230

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 4:05 PM ET / 20:05 UTC.

Summary
The PR changes both ScreenCaptureKit transient-denial retry sleeps to propagate cancellation before a second image or video capture attempt and adds a changelog entry.

Reproducibility: no. live reproduction is provided, but the failure is source-reproducible with high confidence: try? suppresses the sleep's CancellationError, after which control reaches the retry attempt.

Review metrics: 2 noteworthy metrics.

• Retry coverage: 2 production sites changed, 0 targeted tests added. Both image and video paths change cancellation behavior, but neither path is locked by a focused regression test.
• Patch scope: 2 files, 7 additions, 2 deletions. The behavioral change is narrow and reviewable, with one production file and one release-owned changelog file affected.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P1] Add deterministic cancellation tests covering both image and video transient-denial retry paths.
• [P1] Provide redacted after-fix macOS runtime evidence showing that cancellation prevents the second ScreenCaptureKit attempt.

Proof guidance:

• [P1] Needs real behavior proof before merge: Only lint, test-suite, and static control-flow evidence is supplied; add redacted macOS logs, terminal output, or a recording showing cancellation during the denial sleep prevents a second attempt, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

• [P1] The supplied validation cannot demonstrate that a real canceled ScreenCaptureKit denial retry avoids the second capture attempt; the broad suite may remain green without exercising this branch.

Maintainer options:

1. Decide the mitigation before merge
   Retain the throwing-sleep approach, add deterministic tests that cancel both image and video denial retries during the delay and assert only one attempt occurs, then provide a redacted macOS runtime transcript or log showing prompt cancellation.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

• [P1] The contributor should add targeted regression coverage and real macOS behavior proof; automation cannot supply evidence from the contributor's capture environment.

Security
Cleared: The narrow Swift control-flow and changelog changes introduce no concrete security or supply-chain concern.

Review findings

• [P2] Add cancellation regression coverage for both retry paths — Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCaptureEngineSupport.swift:182-183

Review details

Best possible solution:

Retain the throwing-sleep approach, add deterministic tests that cancel both image and video denial retries during the delay and assert only one attempt occurs, then provide a redacted macOS runtime transcript or log showing prompt cancellation.

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

No live reproduction is provided, but the failure is source-reproducible with high confidence: try? suppresses the sleep's CancellationError, after which control reaches the retry attempt.

Is this the best way to solve the issue?

Yes in direction: allowing the throwing sleep to propagate cancellation is the narrowest maintainable fix, but the PR should add focused regression coverage and real behavior proof before merge.

Full review comments:

• [P2] Add cancellation regression coverage for both retry paths — Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCaptureEngineSupport.swift:182-183
  Add a focused test that cancels during the injected transient-denial delay and verifies that attempt is called only once for both image and video capture. Repository policy requires regression tests alongside fixes, and the existing broad suite can pass without exercising either changed cancellation branch.
  Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against e2a46347138d.

Label changes

Label changes:

• add P2: This is a bounded cancellation bug that can cause an unnecessary second capture attempt, without evidence of data loss, security impact, or broad runtime failure.
• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Only lint, test-suite, and static control-flow evidence is supplied; add redacted macOS logs, terminal output, or a recording showing cancellation during the denial sleep prevents a second attempt, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Label justifications:

• P2: This is a bounded cancellation bug that can cause an unnecessary second capture attempt, without evidence of data loss, security impact, or broad runtime failure.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Only lint, test-suite, and static control-flow evidence is supplied; add redacted macOS logs, terminal output, or a recording showing cancellation during the denial sleep prevents a second attempt, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Evidence reviewed

What I checked:

• Current-main behavior: At the reviewed main SHA, both image and video transient-denial paths use try? await Task.sleep, allowing CancellationError to be swallowed before the retry attempt. (Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCaptureEngineSupport.swift:182, e2a46347138d)
• Proposed cancellation fix: The PR replaces both suppressing sleeps with throwing sleeps followed by Task.checkCancellation(), preventing a canceled task from entering the second capture attempt. (Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCaptureEngineSupport.swift:182, f7af6e79c4f4)
• No focused regression coverage: The branch changes production code and CHANGELOG.md but adds no test that cancels during the transient-denial delay and asserts that the retry closure is not invoked. (Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCaptureEngineSupport.swift:182, f7af6e79c4f4)
• Repository testing policy: AGENTS.md directs contributors to add regression tests alongside fixes; this guidance applies to the capture change and materially affects merge readiness. (AGENTS.md:35, e2a46347138d)
• Related accepted pattern: Merged pull requests fix(app): honor task cancellation in app running-state wait loop #230, fix(cli): honor task cancellation in menu bar click wait loops #267, fix(window): honor task cancellation while waiting for window disappear #270, and fix(window): honor task cancellation while settling maximize frame #271 establish cancellation hygiene for similar swallowed-sleep paths, while addressing distinct call sites. (a7b5094ec9ca)
• Proof limitation: The PR reports green lint and safe-suite results but explicitly states that cancellation during a live ScreenCaptureKit denial retry was not exercised. (f7af6e79c4f4)

Likely related people:

• SebTardif: Their previously merged work introduced cancellation handling across several analogous wait and polling paths, making them closely familiar with the accepted behavior and validation pattern. (role: recent cancellation-path contributor; confidence: high; commits: b6a089828101, ee03f0df87b5, ed1a72186cd3; files: Apps/CLI/Sources/PeekabooCLI/Helpers/MenuBarClickVerifier.swift, Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCaptureEngineSupport.swift)
• steipete: Repository history and release work connect them to the current capture architecture and recent maintenance of the affected automation subsystem. (role: capture subsystem owner; confidence: medium; commits: 41180ca, e2a46347138d; files: Core/PeekabooAutomationKit/Sources/PeekabooAutomationKit/Services/Capture/ScreenCaptureEngineSupport.swift)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

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.

How this review workflow works

• 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.

[SebTardif]

@clawsweeper re-review

Added focused cancel-during-sleep coverage for both image (runCapture) and generic (run) transient-denial paths (attempts == 1), plus a red/green terminal prove script showing unfixed burns a second attempt after cancel while fixed stops at one attempt (~52ms). PR body Evidence and Real behavior proof updated with the live transcript.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The item is closed; no stale verdict was published.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29803133834
• Updated: 2026-07-21T05:10:02.126Z

[steipete]

Triage Wave 5 closeout: closing without merge, not requesting changes.

Current main still has the reported bug: both ScreenCaptureFallbackRunner retry paths suppress CancellationError with try? await Task.sleep, so a canceled task can enter a second capture attempt. PR #280 finds the same bug class in the adjacent screen-recording permission probe. The coherent maintainer shape is one combined fix covering all three retry sites, with one changelog entry and focused regression tests; #279 was selected as that canonical lane.

Local maintainer proof against current-main behavior:

• Red: swift test --package-path Core/PeekabooCore --filter ScreenCaptureFallbackRunnerTests made 2 attempts in both new cancellation cases and threw the original ScreenCaptureKit -3801 error instead of CancellationError.
• Red: swift test --package-path Core/PeekabooAutomationKit --filter ScreenRecordingPermissionCancelTests made 2 permission probes instead of 1.
• Green on the combined patch: the same focused commands passed 6/6 runner tests and 2/2 non-live permission tests; the non-canceled retry control still made exactly 2 probes.
• pnpm run build:cli passed.
• Canonical AutoReview (gpt-5.6-sol, xhigh) returned no actionable findings and rated the combined patch correct at 0.93 confidence.
• Contributor head 4e12fa53ab367d14cadad02ad498671af5b10eb5 has hosted macOS CI run 29779797318 green across all 5 jobs.

The mandatory live gate could not be cleared. The installed GUI host reported Screen Recording and Accessibility granted, but caller-local, GUI/on-demand, and fallback captures all timed out. A generated XCTest bundle containing the production permission checker was signed with the current OpenClaw Foundation Developer ID and run with an ungranted Screen Recording identity; its real SCShareableContent probe did not return. Restarting the signed host and the per-user replayd service did not restore a bounded capture. The full safe suite also hit an unrelated existing AppKit hang in MenuBarFocusVerificationTests, and local SwiftLint stalled under the current Xcode-beta toolchain.

Per the strict requirement, deterministic injected-error tests and standalone mirrored scripts are not being presented as live ScreenCaptureKit proof. No commit was pushed and there is no merge SHA. Thanks @SebTardif for the accurate root-cause report and the focused cancellation work.