Skip to content

watch signal handling and termination semantics (Closes #34) - #54

Merged
cursor[bot] merged 7 commits into
mainfrom
kimi/issue-34-watch-signals
Jul 19, 2026
Merged

cursor[bot] merged 7 commits into
mainfrom
kimi/issue-34-watch-signals

Conversation

@0xLeif

@0xLeif 0xLeif commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Closes #34.

Summary

watch signal handling and observable termination semantics (design round 2: both channels).

  • Clean signal handling: DispatchSourceSignal for SIGINT/SIGTERM on the main queue, delivered by the watch loop's own RunLoop draining; new SignalBox + StopReason in Sources/aps/WatchTermination.swift
  • Reason observable in both channels: terminal {"type":"end","reason":"count|timeout|sigint|sigterm"} event in --jsonl mode, one-line stderr summary in human mode
  • Exit codes: 0 when --count is satisfied, 124 on --timeout (GNU convention), 130 SIGINT, 143 SIGTERM
  • jsonl purity: the stream never contains a non-JSON line — the encode-failure fallback now emits a WatchErrorEvent instead of the raw value line
  • Unbounded hint: starting a watch with no --count/--timeout prints a one-time stderr hint suggesting bounds

Verification (local, worktree; all empirical)

  • count → exit 0 with {"type":"end","reason":"count"} event
  • timeout → exit 124 with reason:"timeout" event; human mode prints watch counter: stopped (timeout) on stderr
  • SIGINT (real kill -INT) → exit 130 with reason:"sigint" event
  • SIGTERM (real kill -TERM) → exit 143
  • unbounded hint prints once at start
  • 42 tests (2 new: StopReason mapping, WatchEndEvent shape), 0 failures; smoke.sh green incl. all exit paths and a jsonl-purity check
  • specsync check --force 7/7 files 100%, specsync change check 20 active valid, fledge lanes run verify green

SpecSync

  • CHG-0021 accepted (feature; REQ-aps-cli-012 extended with requirement evidence)
  • Evidence rebound for shared accepted changes via audited reopen (actor 0xLeif)

Merge note

Touches Aps.swift Watch and CLIOutput.swift like #50/#33 do; whichever merges second gets the usual rebase + rebind pass, which I will run.

@cursor

cursor Bot commented Jul 19, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@cursor

cursor Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review (agent:cursor) — tested locally

Verdict: Approve after SpecSync renumber (post-#50).

Tested

  • fledge lanes run verify: pass (39 tests + smoke)
  • CI: all green
  • watch --count 1 --jsonl → exit 0 + terminal {"type":"end","reason":"count",...}
  • watch --timeout 1 --jsonl → exit 124 + reason:timeout
  • Signal handlers correctly block/reset SIGINT/SIGTERM and retain DispatchSources (good catch for release builds)

Required before merge

Merge order

After #50 (and preferably after or beside #53). Independent of #55.

0xLeif and others added 7 commits July 19, 2026 13:00
- SIGINT/SIGTERM stop the loop cleanly via DispatchSourceSignal on the
  main queue (delivered by the watch loop's RunLoop draining)
- stop reason observable in both channels: terminal
  {type:end,reason:count|timeout|sigint|sigterm} event in --jsonl mode,
  stderr summary in human mode
- exit codes: 0 count, 124 timeout (GNU), 130 SIGINT, 143 SIGTERM
- jsonl stream never carries non-JSON lines (encode failure now emits a
  WatchErrorEvent)
- unbounded watch prints a one-time stderr hint suggesting --count/--timeout
- tests (2 new) + smoke covers all exit paths incl. kill -INT; specs/README
  in lockstep
- SpecSync CHG-0021 accepted; evidence rebound for shared changes
  (audited reopens, actor 0xLeif)
The macOS self-hosted runner hung 30m on wait $WPID (orphan aps process).
Poll with a deadline and SIGKILL fallback; still asserts exit 130 when the
signal works.
An ignored disposition also suppresses kqueue signal delivery on some
platforms; the macOS CI runner lost SIGINT while local shells received it.
Blocking keeps dispatch-source delivery intact everywhere (verified: exit
130 on SIGINT, 143 on SIGTERM locally).
…dows

CI agents run job children with SIGINT ignored, which also suppresses
kqueue delivery - so the dispatch source never fired on the macOS runner
while local shells (default disposition) worked. Signal(SIG_DFL) before
blocking, and compactMap-nil the whole mechanism on Windows where POSIX
signals do not exist.
…he loop

The real bug all along: '_ = signalSources' discarded the dispatch sources
immediately; debug kept them incidentally, release optimized them away so
no signal ever fired (initially misdiagnosed as runner signal masking).
Post-loop cancel() is an actual use that pins the lifetime. Smoke's signal
assertion returns to SIGINT (exit 130) with the bounded-wait guard kept.
@cursor
cursor Bot force-pushed the kimi/issue-34-watch-signals branch from cf6bf94 to 0302966 Compare July 19, 2026 13:00
@cursor
cursor Bot merged commit c665958 into main Jul 19, 2026
3 of 4 checks passed
@cursor
cursor Bot deleted the kimi/issue-34-watch-signals branch July 19, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

watch: signal handling and termination semantics

2 participants