feat: defined shutdown/flush contract and opt-in SIGTERM/SIGINT flush (#8)

[krassx]

Closes #8. Stacked on #24 (fix/issue-9) — base is fix/issue-9; retarget to main after #24 merges.

Summary

• Contract: flush(timeout) persists everything captured and tries to drain the queue (true only if empty; idempotent; not signal-safe). stop()/guard Drop persists and removes the liveness marker but never uploads — undelivered reports stay in the durable queue (disk mode). New Bugsee::shutdown(timeout) = flush then stop. Exit-path table in DESIGN.md §8 (at-least-once delivery).
• Opt-in termination hook: Bugsee::install_termination_flush(timeout) (unix): async-signal-safe handler that only writes to a self-pipe and chains to the host's previous handler (SA_SIGINFO/plain, mask/ONSTACK/NODEFER carried, SA_RESETHAND emulated); a watcher thread flushes, then restores SIG_DFL and re-raises only if the disposition really is still ours and was default. SIG_IGN is left alone; handlers the host installs after ours are detected (flush only, never kill the host). Second signal kills at once. Windows: SetConsoleCtrlHandler returning FALSE (chain on).
• LaunchOptions::flush_at_exit(timeout) via atexit (unix; no-op on Windows; skipped in fork children).
• Stop no longer depends on the uploader (a hung upload used to leave the marker → false abnormal-exit next launch); stopping aborts in-flight flush/drain.

Review

Opus review (13 findings incl. late-installed host handlers, in-flight upload at stop, fork-child pipe race) addressed; 14 subprocess tests in crates/bugsee/tests/shutdown.rs (unix-only), ~27 s.

Not verified

Windows handler and Linux-specific paths (pipe2) were never run; SA_RESETHAND emulation has no test.

Test plan

• fmt, clippy -D warnings, workspace tests on macOS (skipping the pre-existing hanging fork_safety test)
• CI matrix

🤖 Generated with Claude Code

[cursor]

Stale comment

Deep review: shutdown/flush contract and opt-in termination hook

Reviewed the stacked diff (fix/issue-9…c462eef) against issue #8, Recorder::drop / drain_until_empty, the fork protocol, and the subprocess matrix in crates/bugsee/tests/shutdown.rs. This is not a style pass.

The contract itself is sound: flush drains, stop/Drop persist and drop the marker without uploading, shutdown is flush-then-stop, and a hung HTTP call no longer turns a deliberate stop into a false abnormal-exit. The unix self-pipe + watcher + late-handler sigaction probe is the right shape, and the 14 child-process tests cover the paths they claim.

Three issues should be fixed before merge. I did not run the test suite here (toolchain in this environment is 1.83; MSRV is 1.88).

Findings

1. P1 — Windows Clippy is red because flush_at_exit state is not cfg(unix) (crates/bugsee/src/api.rs)
2. P2 — SA_RESETHAND emulation kills the process on the first signal (crates/bugsee/src/termination.rs)
3. P2 — Windows Ctrl+C/Break with the hook still files an abnormal-exit (crates/bugsee/src/termination.rs)

CI (at review time)

• windows-latest: build OK, tests OK, Clippy failed (-D warnings)
• android aarch64: Clippy failed on bugsee-core / bugsee-native / bugsee-ffi (job logs were not available; this crate’s new Windows/atexit code is not in that job)
• ubuntu / macos were still running

Overall risk: Medium

Merge recommendation: Request changes

Most important to fix

1. cfg(unix) the EXIT_FLUSH_* statics and the AtomicU32 import so Windows Clippy is green.
2. Snapshot “should this wakeup terminate?” at handler time (pipe byte), instead of re-reading a slot that SA_RESETHAND has already rewritten. Add a Linux SA_RESETHAND subprocess test.
3. On Windows Ctrl+C/Break, shutdown when this handler is the one that will let the process die (or document that hosts must still stop themselves, and drop the implication that the hook makes Ctrl+C a clean exit).

Residual (not blocking)

• C ABI still has bugsee_flush / bugsee_stop only — no bugsee_shutdown / termination hook. C hosts can compose the two calls; they cannot get the signal helper.
• DESIGN.md §6 still says the launch guard’s Drop flushes; §8 now says it never uploads. Pre-existing inaccuracy, now contradictory.
• SA_RESETHAND / pipe2 / the Windows handler were not exercised (noted in the PR). The first of those is a real bug, not just a coverage gap.

What looks good

Stop no longer gates the marker on the uploader, so a slow PUT cannot invent a crash. stopping aborts drain_until_empty between deliveries. Late host handlers (tokio / signal-hook) are detected by asking the kernel, not by trusting the slot. Fork children without a watcher die() immediately instead of hanging on a dead pipe. Host SA_SIGINFO / sa_mask / SA_ONSTACK / SA_NODEFER are carried through. The subprocess tests actually kill children and relaunch over the same data dir.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review: shutdown/flush contract (re-review at e739839)

Re-reviewed the stacked diff (fix/issue-9…e739839) against issue #8, Recorder::drop / drain_until_empty, the fork protocol, and crates/bugsee/tests/shutdown.rs. This is not a style pass.

Prior findings

All three from the last review are fully addressed:

1. P1 EXIT_FLUSH_* / AtomicU32 are #[cfg(unix)]. Windows Clippy is green.
2. P2 SA_RESETHAND: the handler snapshots terminate before rewriting the slot and ships it in the pipe byte (TERMINATE = 0x80). The watcher no longer re-reads the slot. sigterm_oneshot_handler covers the first-signal-must-not-kill case.
3. P2 Windows Ctrl+C/Break now call Bugsee::shutdown; the docs state that a swallowing host must register after the SDK.

CI at e739839 (38554525334) is SUCCESS on ubuntu, macos, windows (including Clippy -D warnings), Android/iOS lint, and MSRV.

Findings

1. P2 — drain_until_empty starts a blocking disk upload after stopping already aborted the memory drain (crates/bugsee-core/src/runtime.rs)

No P0/P1. I did not run the suite here; the judgement is from the current code plus the green CI run above.

Overall risk: Medium

Merge recommendation: Request changes

Most important to fix

Re-check stopping after drain_memory returns, and again before each deliver_queued. When it is set, return immediately so the uploader can take UploadMsg::Stop and run the 1.5 s memory drain that the contract depends on.

Residual (not blocking)

• C ABI still exposes bugsee_flush / bugsee_stop only. C hosts can compose the two calls; they cannot get install_termination_flush.
• Windows console handler and Linux pipe2 are still not exercised by tests (shutdown.rs is #![cfg(unix)]). The Windows path is now the right shape on paper.
• macOS still does not report SA_RESETHAND (already documented on the native crash path). One-shot / BSD signal() hosts stay on the flush-only side; safer than the old first-signal kill, but a second SIGTERM will not become default-terminate there.
• Service-control stop is not a console event; documented, host must call shutdown.

What looks good

The contract is still sound: flush drains, stop/Drop persist and drop the marker without uploading, shutdown is flush-then-stop, and a hung HTTP call no longer invents a crash. The unix self-pipe + watcher + late-handler sigaction probe is the right shape. Fork children without a watcher die() immediately. flush_at_exit is unix-only and skipped in fork children. The 15 subprocess cases actually kill children and relaunch over the same data dir, including the new oneshot test.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review: shutdown/flush contract (re-review at 50da593)

Re-reviewed the unique diff versus the current base (fix/issue-12 @ 5a844b8 … 50da593) and the merge of a newer origin/fix/issue-12 (native CrashBackend, origin walks, FFI audit, lock_backtrace). Unique shutdown/flush code is unchanged since 3703010. This is not a style pass.

What changed this round

50da593 remarges origin/fix/issue-12 @ 5a844b8. Unique delta vs that base is still the eight #8 files (termination.rs, shutdown.rs, api.rs lifecycle, options.rs flush_at_exit, drain_until_empty / stopping, DESIGN §8, the storage-failures stop-during-flush test). termination.rs and shutdown.rs have an empty diff against 3703010. The only unique-file edit in the merge itself is Bugsee::upload() taking the origin walk off the global lock — that is #1/#12 work, not #8.

Prior findings

All four from earlier rounds stay fully addressed:

1. P1 EXIT_FLUSH_* / AtomicU32 are #[cfg(unix)].
2. P2 SA_RESETHAND: terminate is decided in the handler and shipped in the pipe byte (TERMINATE = 0x80). sigterm_oneshot_handler covers the first-signal-must-not-kill case.
3. P2 Windows Ctrl+C/Break call Bugsee::shutdown; docs say a swallowing host registers after the SDK.
4. P2 drain_until_empty (546024b): re-checks stopping after drain_memory and after each claim, so a stop that landed in the memory pass cannot start a blocking disk PUT.

Merge intersection (re-checked against the new base)

Still coherent with #12/#13/#10/#14/#1/#23:

• Marker and session lock still require the capture worker’s ack; health finish_clean still runs on any non-panicking stop. Uploader detach no longer invents a crash.
• drain_memory honors both stopping (flush abort) and is_dying() (crash-time flush). New disk claims are refused while dying (try_claim_lenient).
• revive_after_fork still forgets the parent session and crash-flush helper, so a child shutdown cannot clear the parent’s marker. A fork child that has not re-armed still die()s immediately (OWNER_PID / WATCHER_ALIVE).
• register_exit_flush still runs before recover_on_launch_path; atexit is a no-op until there is something to flush, and a child whose pid does not match EXIT_FLUSH_PID does not run it.
• Native POSIX capture still owns SIGABRT/SIGSEGV/friends, not SIGTERM/SIGINT. The termination hook and the crash handler do not share a slot.
• lock_backtrace (origin walks) is not taken on the flush/stop path; a concurrent fork waits the walk out instead of handing the child a stuck backtrace mutex.

Tried to break: second signal vs SA_RESETHAND, late host handler, ignored SIGTERM, fork child without watcher, atexit in a child, stop during an in-flight memory pass then disk PUT, Drop vs health vs marker. Those paths still match the contract and the tests that exist for them.

Findings

No remaining unique P0–P3.

CI (run 38577916981 at 50da593)

All 8 checks SUCCESS: ubuntu (includes the unix shutdown.rs subprocess matrix), windows Build/Test/Clippy, macos, MSRV 1.88, Android aarch64, iOS device and sim, macOS private-API job.

I did not run the suite locally (this environment is rustc 1.83; MSRV is 1.88). The judgement is from the current code plus the green CI jobs above.

Overall risk: Low

Merge recommendation: Approve (after/with the stacked #12 base as before).

Residual (not blocking)

• C ABI still exposes bugsee_flush / bugsee_stop only. C hosts can compose the two calls; they cannot get install_termination_flush or flush_at_exit. bugsee_stop’s rustdoc still says “flushing pending work” — persist + the 1.5 s memory drain, not a disk-queue upload.
• shutdown.rs is #![cfg(unix)]. The Windows console handler is the right shape and now compiles and Clippy-cleans on windows-latest; it is still not exercised as a subprocess.
• macOS still does not report SA_RESETHAND. One-shot / BSD signal() hosts stay flush-only after the first signal.
• Service-control stop is not a console event; documented, host must call shutdown.
• An HTTP call already in flight cannot be interrupted. That is the documented at-least-once / 2 s detach bound. Scheduled drain_all still does not honor stopping (intentional; only an explicit flush aborts).
• DESIGN §12 still says shutdown / install_termination_flush “neither exists on this branch yet”. They do, in §8. Stale #12 wording; not a behavioral bug.

What looks good

The contract still matches the code after the stack merge: flush drains, stop/Drop persist and drop the marker without uploading, shutdown is flush-then-stop, and a hung HTTP call no longer invents a crash. The unix self-pipe + watcher + late-handler probe is unchanged and still the right shape. Fork children without a watcher die() immediately. The subprocess matrix still kills children and relaunches over the same data dir. Windows Clippy is green on this tree.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review: shutdown/flush contract (re-review at f5dcac4)

Re-reviewed the unique diff versus the current base (fix/issue-12 @ fe63783 … f5dcac4) and the surrounding Drop / drain / crash-flush / fork / native paths. Unique shutdown/flush code is unchanged since 3703010. This is not a style pass.

What changed this round

f5dcac4 remarges origin/fix/issue-12 @ fe63783 (libc de-dupe 1ecfca8 already in that stack). The trees of f5dcac4 and the last-reviewed 50da593 are identical (9444ea77); the bases fe63783 and 5a844b8 are likewise identical (5722f5e9). Unique delta vs base is still the eight #8 files (termination.rs, shutdown.rs, api.rs lifecycle, options.rs flush_at_exit, drain_until_empty / stopping, DESIGN §8, the storage-failures stop-during-flush test). crates/bugsee/Cargo.toml has a single [target.'cfg(unix)'.dependencies] libc table; cargo metadata loads (the duplicate-table P0 that hit sibling stack PRs is not on this tree).

Prior findings

All four from earlier rounds stay fully addressed:

1. P1 EXIT_FLUSH_* / AtomicU32 are #[cfg(unix)].
2. P2 SA_RESETHAND: terminate is decided in the handler and shipped in the pipe byte (TERMINATE = 0x80). sigterm_oneshot_handler covers the first-signal-must-not-kill case.
3. P2 Windows Ctrl+C/Break call Bugsee::shutdown; docs say a swallowing host registers after the SDK.
4. P2 drain_until_empty (546024b): re-checks stopping after drain_memory and after each claim, so a stop that landed in the memory pass cannot start a blocking disk PUT.

Merge intersection (re-checked)

Still coherent with #12/#13/#10/#14/#1/#23/#9:

• Marker and session lock still require the capture worker’s ack; health finish_clean still runs on any non-panicking stop. Uploader detach no longer invents a crash. Drop still waits 2 s for the uploader ack; it just does not gate the marker on the result.
• drain_memory honors both stopping (flush abort) and is_dying() (crash-time flush). New disk claims are refused while dying (try_claim_lenient).
• revive_after_fork still forgets the parent session and crash-flush helper, so a child shutdown cannot clear the parent’s marker. A fork child that has not re-armed still die()s immediately (OWNER_PID / WATCHER_ALIVE).
• register_exit_flush still runs before recover_on_launch_path; atexit is a no-op until there is something to flush, and a child whose pid does not match EXIT_FLUSH_PID does not run it.
• Native POSIX capture still owns SIGABRT/SIGSEGV/friends, not SIGTERM/SIGINT. The termination hook and the crash handler do not share a slot.
• lock_backtrace (origin walks) is not taken on the flush/stop path.

Tried to break, and did not file:

• Ignored write return in the unix handler: on this O_NONBLOCK 1-byte pipe, EINTR is from pipe_wait (full + blocking); a full pipe is EAGAIN; the default-disposition path cannot fill PIPE_BUF. OWNER_PID is stored after PIPE_W with release/acquire, so a live owner always sees a valid fd. Watcher read already retries EINTR.
• Drop vs is_dying() during a hung upload: Drop still sits in recv_timeout(2s) for the uploader; a concurrent native crash aborts at the 50 ms budget first, so session.end() does not run and the marker stays. The unique change (not gating the marker on uploader_acked) only applies after that wait returns.

Findings

No remaining unique P0–P3.

CI (run 38579287636 at f5dcac4)

All 8 checks SUCCESS: ubuntu (includes the unix shutdown.rs subprocess matrix), windows Build/Test/Clippy, macos, MSRV 1.88, Android aarch64, iOS device and sim, macOS private-API job.

I did not run the suite locally (this environment is rustc 1.83; MSRV is 1.88). The judgement is from the current code plus the green CI jobs above. cargo metadata --offline succeeds on this tree.

Overall risk: Low

Merge recommendation: Approve (after/with the stacked #12 base as before).

Residual (not blocking)

• C ABI still exposes bugsee_flush / bugsee_stop only. C hosts can compose the two calls; they cannot get install_termination_flush or flush_at_exit. bugsee_stop’s rustdoc still says “flushing pending work” — persist + the 1.5 s memory drain, not a disk-queue upload.
• shutdown.rs is #![cfg(unix)]. The Windows console handler is the right shape and now compiles and Clippy-cleans on windows-latest; it is still not exercised as a subprocess.
• macOS still does not report SA_RESETHAND. One-shot / BSD signal() hosts stay flush-only after the first signal.
• Service-control stop is not a console event; documented, host must call shutdown.
• An HTTP call already in flight cannot be interrupted. That is the documented at-least-once / 2 s detach bound. Scheduled drain_all still does not honor stopping (intentional; only an explicit flush aborts).
• DESIGN §12 still says shutdown / install_termination_flush “neither exists on this branch yet”. They do, in §8. Stale #12 wording; not a behavioral bug.

What looks good

The contract still matches the code after the stack merge: flush drains, stop/Drop persist and drop the marker without uploading, shutdown is flush-then-stop, and a hung HTTP call no longer invents a crash. The unix self-pipe + watcher + late-handler probe is unchanged and still the right shape. Fork children without a watcher die() immediately. The subprocess matrix still kills children and relaunches over the same data dir. Windows Clippy is green on this tree.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Deep review: shutdown/flush contract (re-review at fa0a34e)

Re-reviewed the unique diff versus the current GitHub base (main @ 78424d4 … fa0a34e) and the surrounding Drop / drain / crash-flush / fork / health / native paths. Unique shutdown/flush code is unchanged in substance since 3703010. This is not a style pass.

What changed this round

fa0a34e remarges origin/main (squash of #12) after #12 landed; GitHub retargeted this PR to main. Unique delta vs main is still the eight #8 files (termination.rs, shutdown.rs, api.rs lifecycle, options.rs flush_at_exit, drain_until_empty / stopping, DESIGN §8, the storage-failures stop-during-flush test). Against last-reviewed f5dcac4, those files differ only by inherited #36 wording in DESIGN §10 and the already-on-main CRASH_FLUSH_ACK_TIMEOUT 10 s comment in runtime.rs. termination.rs / shutdown.rs / the flush/stop/shutdown API are an empty diff versus f5dcac4.

crates/bugsee/Cargo.toml still has a single [target.'cfg(unix)'.dependencies] libc table; cargo metadata --offline --no-deps loads.

Prior findings

All four from earlier rounds stay fully addressed:

1. P1 EXIT_FLUSH_* / AtomicU32 are #[cfg(unix)].
2. P2 SA_RESETHAND: terminate is decided in the handler and shipped in the pipe byte (TERMINATE = 0x80). sigterm_oneshot_handler covers the first-signal-must-not-kill case.
3. P2 Windows Ctrl+C/Break call Bugsee::shutdown; docs say a swallowing host registers after the SDK.
4. P2 drain_until_empty (546024b): re-checks stopping after drain_memory and after each claim, so a stop that landed in the memory pass cannot start a blocking disk PUT.

Merge intersection (re-checked against landed #12 / #13 on main)

Still coherent:

• Marker and session lock still require the capture worker’s ack; health finish_clean still runs on any non-panicking stop. Uploader detach no longer invents a crash. Drop still waits 2 s for the uploader ack; it just does not gate the marker on the result.
• drain_memory honors both stopping (flush abort) and is_dying() (crash-time flush). New disk claims are refused while dying (try_claim_lenient). Stop-time memory drain still passes honor_stop = false so the 1.5 s last attempt is not aborted by the flag it itself depends on.
• revive_after_fork still forgets the parent session and crash-flush helper, so a child shutdown cannot clear the parent’s marker. A fork child that has not re-armed still die()s immediately (OWNER_PID / WATCHER_ALIVE). Re-arm starts a new watcher and returns because *installed survives the fork (ForkMutex does not reset the bool), so it does not re-sigaction over itself.
• register_exit_flush still runs before recover_on_launch_path; atexit is a no-op until there is something to flush, and a child whose pid does not match EXIT_FLUSH_PID does not run it.
• Native POSIX capture still owns SIGABRT/SIGSEGV/friends, not SIGTERM/SIGINT. The termination hook and the crash handler do not share a slot.

Tried to break, and did not file:

• Ignored write return in the unix handler: on this O_NONBLOCK 1-byte pipe, a full pipe is EAGAIN and the default-disposition path cannot fill PIPE_BUF. OWNER_PID is stored after PIPE_W with release/acquire.
• Drop vs is_dying() during a hung upload: Drop still sits in recv_timeout(2s) for the uploader; a concurrent native crash aborts at the 50 ms budget first (or immediately once crash_flush is dropped), so session.end() does not run and the marker stays.
• break 'dirs after a just-finished disk delivery when stopping is set: the next loop iteration returns false at the top; it does not start another PUT.

Findings

No remaining unique P0–P3.

CI (run 38598330393 at fa0a34e)

• ubuntu: SUCCESS (includes the unix shutdown.rs subprocess matrix)
• windows: SUCCESS — Build, Test, and Clippy (-D warnings)
• MSRV 1.88, Android aarch64, iOS sim, macOS private-API: SUCCESS
• macos-latest tests: still in progress at review time. Unique unix tests already passed on ubuntu.
• Prior SHA f5dcac4 run 38579287636 was all 8 SUCCESS.

I did not run the suite locally (this environment is rustc 1.83; MSRV is 1.88). The judgement is from the current code plus the green CI jobs above. cargo metadata --offline succeeds on this tree.

Overall risk: Low

Merge recommendation: Approve

The stacked #12 base is now on main, so this can land on its own.

Residual (not blocking)

• C ABI still exposes bugsee_flush / bugsee_stop only. C hosts can compose the two calls; they cannot get install_termination_flush or flush_at_exit. bugsee_stop’s rustdoc still says “flushing pending work” — persist + the 1.5 s memory drain, not a disk-queue upload.
• shutdown.rs is #![cfg(unix)]. The Windows console handler is the right shape and compiles and Clippy-cleans on windows-latest; it is still not exercised as a subprocess.
• macOS still does not report SA_RESETHAND. One-shot / BSD signal() hosts stay flush-only after the first signal.
• Service-control stop is not a console event; documented, host must call shutdown.
• An HTTP call already in flight cannot be interrupted. That is the documented at-least-once / 2 s detach bound. Scheduled drain_all still does not honor stopping (intentional; only an explicit flush aborts).
• DESIGN §10 still says shutdown / install_termination_flush “neither exists on this branch yet”. They do, in §8. Stale #12 wording; not a behavioral bug.

What looks good

The contract still matches the code after the remarge onto main: flush drains, stop/Drop persist and drop the marker without uploading, shutdown is flush-then-stop, and a hung HTTP call no longer invents a crash. The unix self-pipe + watcher + late-handler probe is unchanged and still the right shape. Fork children without a watcher die() immediately. The subprocess matrix still kills children and relaunches over the same data dir. Windows Clippy is green on this tree.

  

_{Sent by Cursor Automation: Bugsee code review}