fix(hooks): honor cancel during terminate wait

[SebTardif]

When a timed-out hook is being terminated, cancelling its HookRunner.run task could be swallowed and reported as HookRunnerError.timedOut. The runner now propagates cancellation from the SIGTERM grace wait, kills any surviving child, and checks cancellation after cleanup so an exited child cannot skip the cancellation checkpoint.

The change keeps the existing timeout contract: uncancelled hooks receive SIGTERM, get up to 100 ms to exit, then receive SIGKILL if still running and report .timedOut. Cancellation before the timeout still does not interrupt the initial process wait; this PR does not change that behavior or add CLI signal handling. The cleanup helper is internal for focused regression coverage; the public API is unchanged.

The cancellation test now waits for a real SIGTERM marker before cancelling, and a second test deterministically covers cleanup with an already-exited child, where no polling sleep can observe cancellation. Removing the final checkpoint makes that regression fail with .timedOut; restoring it passes.

Validation on macOS 26.6.2 with Swift 6.4:

• ./scripts/format.sh: unchanged; ./scripts/lint.sh: passed, six existing warnings.
• swift test --parallel: 18 tests passed.
• node --test scripts/build-docs-site.test.mjs: 2 tests passed.
• swift build -c release: passed.
• Release CLI smoke: health/text and JSON, setup permissions and overwrite refusal, status, help, short hook invocation, and nonzero hook exit passed.
• An external executable linked against the built release Swabble library exercised the public API with a shell hook that records SIGTERM and keeps running. Cancelling during that grace wait returned CancellationError; without cancellation it returned .timedOut. Both cases confirmed the child was no longer alive.

No microphone session was needed for the changed process-cleanup behavior; live speech recognition was not exercised.

Original fix and cancellation regression by @SebTardif; maintainer follow-up completes the post-poll checkpoint, strengthens coverage, and documents the behavior. Related timeout work: #2.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed September 4, 2026, 3:58 AM ET / 07:58 UTC.

ClawSweeper review

What this changes

Updates Swabble’s shell-hook timeout cleanup to propagate cancellation, kill surviving hooks, and cover the behavior with regression tests and documentation.

Merge readiness

✅ Ready for maintainer review

The prior blocker is resolved and no new blocking defect remains. This PR is still needed because current main retains the cancellation bug.

Priority: P2
Reviewed head: 18b4b69364c018d9dce96560a79e84757db3222d

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with the prior finding resolved, relevant regression coverage, and sufficient reported real-process proof.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (live_output): The captured body reports a real macOS executable exercising HookRunner through the release library’s public API: cancellation after SIGTERM produced CancellationError, the uncancelled control produced timedOut, and both confirmed child disappearance. Focused regression tests supplement this process-cleanup evidence; microphone access is unnecessary for the changed path.
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 real macOS executable exercising HookRunner through the release library’s public API: cancellation after SIGTERM produced CancellationError, the uncancelled control produced timedOut, and both confirmed child disappearance. Focused regression tests supplement this process-cleanup evidence; microphone access is unnecessary for the changed path.
Evidence reviewed	9 items	Pinned patch and repository identity: The origin identifies openclaw/Swabble, HEAD matches the supplied original PR head, and the complete base-to-head diff changes only hook cleanup, two regression tests, documentation, and the changelog. Current main still needs the fix: Main suppresses errors from the termination-poll sleep and then unconditionally throws timedOut. It has neither cancellation propagation nor a final cancellation checkpoint. Prior finding addressed: The previous review requested cancellation observation after polling. The current helper checks cancellation after surviving-process cleanup, including when every polling sleep is skipped.
Findings	None	None.
Security	None	None.

How this fits together

Swabble turns wake-word transcripts into configured shell commands. Its hook runner manages process completion and timeouts, returning success or an error to the daemon or an embedding application.

flowchart TD
  A[Transcript or library invocation] --> B[Hook runner]
  B --> C[Configured shell process]
  C --> D{Exit or timeout?}
  D -->|Exit| E[Return process result]
  D -->|Timeout| F[Terminate and briefly wait]
  F --> G[Kill surviving process]
  G --> H[Report cancellation or timeout]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test growth	Production +13 net lines; tests +81 net lines	The internal cleanup helper supports focused coverage of cancellation during polling and after polling is skipped.

Technical review

Best possible solution:

Preserve ordinary timeout behavior while reliably reporting caller cancellation after cleanup, including when the child has already exited.

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

Yes, from source: cancelling a timed-out invocation during its SIGTERM grace wait causes current main to suppress cancellation and report timedOut. This read-only review did not execute that path.

Is this the best way to solve the issue?

Yes. The cancellation catch and final checkpoint repair the identified cleanup paths while preserving the existing timeout sequence and public API.

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

• 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 reports a real macOS executable exercising HookRunner through the release library’s public API: cancellation after SIGTERM produced CancellationError, the uncancelled control produced timedOut, and both confirmed child disappearance. Focused regression tests supplement this process-cleanup evidence; microphone access is unnecessary for the changed path.
• remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
• remove status: ⏳ waiting on author: Current PR status label is status: 👀 ready for maintainer look.
• remove merge-risk: 🚨 other: Current PR review selected no merge-risk labels.

Label justifications:

• P2: This repairs cancellation reporting during a bounded hook-cleanup window without changing broader daemon or setup behavior.
• 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 real macOS executable exercising HookRunner through the release library’s public API: cancellation after SIGTERM produced CancellationError, the uncancelled control produced timedOut, and both confirmed child disappearance. Focused regression tests supplement this process-cleanup evidence; microphone access is unnecessary for the changed path.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured body reports a real macOS executable exercising HookRunner through the release library’s public API: cancellation after SIGTERM produced CancellationError, the uncancelled control produced timedOut, and both confirmed child disappearance. Focused regression tests supplement this process-cleanup evidence; microphone access is unnecessary for the changed path.

Evidence

What I checked:

• Pinned patch and repository identity: The origin identifies openclaw/Swabble, HEAD matches the supplied original PR head, and the complete base-to-head diff changes only hook cleanup, two regression tests, documentation, and the changelog. (18b4b69364c0)
• Current main still needs the fix: Main suppresses errors from the termination-poll sleep and then unconditionally throws timedOut. It has neither cancellation propagation nor a final cancellation checkpoint. (Sources/SwabbleCore/Hooks/HookRunner.swift:104, 0b8b05adbf17)
• Prior finding addressed: The previous review requested cancellation observation after polling. The current helper checks cancellation after surviving-process cleanup, including when every polling sleep is skipped. (Sources/SwabbleCore/Hooks/HookRunner.swift:127, 18b4b69364c0)
• Focused regression coverage: One added test waits for a real SIGTERM marker, cancels the invocation, and checks child disappearance; another uses an already-exited process to cover cancellation without a polling suspension. Existing timeout and escalation tests remain intact. (Tests/swabbleTests/ConfigTests.swift:184, 18b4b69364c0)
• After-fix production-path evidence: The complete captured PR body reports an external executable linked against the release Swabble library on macOS 26.6.2 with Swift 6.4. Through the public API, a SIGTERM-recording shell hook returned CancellationError when cancelled during cleanup and timedOut without cancellation; both runs confirmed child disappearance. This is a reported real-run result, not reviewer-executed validation. Captured context sourceRevision: f21d56b7be4d203576da6c2d42e524acf9a6ba20ef2147128d35f77f29bcbc31. (18b4b69364c0)
• Documented compatibility boundary: The documentation preserves SIGTERM followed by up to 100 ms of grace and SIGKILL escalation, and explicitly limits cancellation handling to timeout cleanup. The helper is internal; no public signature, configuration, dependency, or stored format changes. (docs/hooks.md:26, 18b4b69364c0)

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 (8 earlier review cycles)

• reviewed 2026-09-02T22:03:39.156Z sha 039796b :: blocked before merge. :: [P2] Check cancellation after the grace poll
• reviewed 2026-09-02T23:49:20.818Z sha 039796b :: needs changes before merge. :: [P2] Check cancellation after the grace poll
• reviewed 2026-09-03T06:03:40.709Z sha 039796b :: needs changes before merge. :: [P2] Observe cancellation after the grace poll
• reviewed 2026-09-03T08:34:26.131Z sha 039796b :: blocked before merge. :: [P2] Observe cancellation after the grace poll
• reviewed 2026-09-03T09:52:53.382Z sha 039796b :: needs changes before merge. :: [P2] Observe cancellation after the grace poll
• reviewed 2026-09-03T13:54:57.564Z sha 039796b :: blocked before merge. :: [P2] Check cancellation after timed-out hook cleanup
• reviewed 2026-09-03T14:00:49.866Z sha 039796b :: blocked before merge. :: [P2] Check cancellation after grace-poll cleanup
• reviewed 2026-09-03T17:01:12.295Z sha 039796b :: blocked before merge. :: [P2] Check cancellation after grace-poll cleanup