feat: bounded launch-path recovery so crash loops can't outrun the report (#10)

[krassx]

Summary

A host that crashes again within milliseconds of launch (systemd/Kubernetes restart loop, launch crash) could die before the worker thread recovered the previous crash, so the report was never produced. Recovery of the newest crashed sessions now finishes on the launch path as queue entries on disk; upload stays asynchronous.

Design

• Bugsee::launch_with ends with Recorder::recover_on_launch_path(), called after the native crash handler and panic hook are installed and outside every lock. A fault inside the pass is therefore recorded with a stack against the new session, and the previous session's evidence stays on disk until its report is queued (attempts are counted before processing, capped at MAX_RECOVERY_ATTEMPTS).
• The pass runs on a helper thread (bugsee-launch-recovery); launch waits at most the budget by polling a flag (not parking: a parker used before fork traps in the child under libdispatch on macOS). That is a hard bound whatever before_send does, so a host holding its own lock cannot deadlock against the callback. On timeout the pass finishes in the background.
• Knob: LaunchOptions::launch_recovery_budget (default 500 ms; Duration::ZERO disables the pass). Core RecorderConfig::launch_recovery_budget defaults to zero; with a non-zero budget the worker holds its own recovery until the pass is done (budget plus 5 s grace) and skips the generations the pass claimed. Multiple processes sharing one data directory can double-deliver reports #14 claims keep the two exclusive in any interleaving.
• Bounds: newest first, at most 8 sessions, no new session after the budget (the newest is always attempted), scan of at most 4096 session markers. Newest first because a crash loop makes only recent evidence reachable and the latest crash most likely belongs to the running build; the worker recovers whatever is left, so nothing is lost.
• Crash-loop escalation: in the pass, a recovered crash whose signature set equals one already queued is dropped (the first copy is kept), and recovered crashes stop being queued at 64 reports or 64 MiB.
• scan_cap counts session markers only; recovered crashes carry no scope from the new process on either path.
• Docs: DESIGN.md section 8 and before_send / launch_recovery_budget rustdoc (threading contract).

Launch latency (Apple silicon, synthetic log-only windows, default 8 MiB / 60 s caps)

Case	release	debug
empty directory	0.1 to 0.3 ms	similar
1 crashed session	about 7 ms	about 21 ms
8 identical sessions	about 17 ms	about 43 ms

The 8-session figure undercounts assembly: 7 are dropped as duplicates before being assembled. Scrubbing (#11) will add per-session cost. Reproduce with cargo test -p bugsee-core --release --test launch_recovery -- --ignored --nocapture.

Tests

bugsee-core/tests/launch_recovery.rs: previous crash queued when the launch path returns; time cap and count cap (deterministic); a blocked before_send cannot stall launch; zero budget; a subprocess crash loop (children abort() right after launch, each relaunch already holds the first crash, repeats collapse, the healthy run delivers it exactly once). Unit tests for queue::admit_recovered and the bounded scan, and a facade test.

Caveats

• Dedup is a heuristic (identical signature set against what is currently queued); no repeat counter on the kept report. The worker pass does not dedup or cap.
• Recovered crashes carry no scope from the new process.
• A prefork worker boot loop (fork children) recovers on its worker thread, not on a launch path; legacy-dir sessions are worker-only.
• Only macOS was run locally; Linux and Windows are covered by CI only.

Stacked on #32 (#14) which is stacked on #24 (#9); retarget to main after those merge.

Closes #10. Delivers the launch-path guarantee (bounded, measured) and launch-pass dedup + a queue cap; dedup is a signature-set heuristic — if the maintainers want stricter dedup, that can be a follow-up issue.

🤖 Generated with Claude Code

[cursor]

Stale comment

Deep review: bounded launch-path recovery (#10)

Stacked on #32. Intent is right: a supervisor crash loop can die before the worker recovers the previous session, so the newest crashed sessions must become durable queue entries before launch returns. Handler install before the pass, helper-thread budget (no pre-fork parker), before_send threading docs, and the abort-after-launch subprocess test all match that contract.

The same-signature crash loop is implemented and tested. Two holes still lose unique evidence the worker is documented to keep.

CI 38555735857: ubuntu / macOS / Windows test+lint, Android, iOS, MSRV 1.88 all SUCCESS. I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Findings

P1 — Admission::Full deletes unique crash evidence; the worker is told to skip those generations

recover_one treats Full like Duplicate and calls recovery::discard. recover_claimed records launch_attempted at claim time, before enqueue. run_recovery(..., skip_launch_pass=true) then leaves those gens alone. DESIGN.md §8 says the worker pass does not cap and that whatever the launch pass leaves is not lost. The function comment on recover_one says the same: discard only once queued.

admit_recovered counts every pending bundle (errors, bugs, crashes) toward 64 reports / 64 MiB. Duplicate-collapse of the same crash is fine; destroying a unique newest crash because the queue is already busy is not.

P2 — scan_cap is applied in readdir order, before newest-first sort

find_pending_bounded takes scan_cap*4 dirents, counts scan_cap .alive files (including live peers and the current gen), then sorts. A cut-off scan can omit the newest marker. The documented mitigation is the worker’s unbounded pass, which a crash loop never reaches. Walk current_generation-1 downward instead; that is O(limit) probes and always hits the newest.

P2 — newest-first plus a fault during recovery defeats MAX_RECOVERY_ATTEMPTS

bump_attempts runs before recover_one, but a native fault / abort in the helper (not a panic; those are caught) is recorded as this session. The next launch attempts that newer gen first. The original pending stays at attempts=1 and is never retried on the launch path, and the worker still has not run. Poison sessions accumulate instead of being abandoned after 3 launches.

Overall risk: High

Unique recovered crashes can be deleted when the outbound queue is at cap, including a cap filled by unrelated undelivered reports. That is data loss in the exact subsystem this PR exists to protect.

Merge recommendation: request changes

Fix the Full path so unique evidence stays on disk (and is not claimed-as-done for the worker). The scan order and attempt-count starvation should be fixed on the same pass; they are the leftover ways a loop still outruns the report.

Most important to fix

1. Do not discard on Admission::Full. Record launch_attempted only after the session is queued, duplicate-dropped, host-dropped, or abandoned by attempts.
2. Find launch-path candidates by walking generation numbers down from current-1, not by a capped unsorted readdir.
3. Keep MAX_RECOVERY_ATTEMPTS progressing when recovery itself kills the process (include already-attempted gens in the bounded set, not only the newest).

What looks solid

Helper thread + 1 ms poll so a blocking before_send cannot deadlock or stall launch; pass runs after the native handler and panic hook, outside the facade lock; claim/bump_attempts before assemble; real crash-loop test (abort after recover_on_launch_path); zero-budget leaves work on the worker; latency numbers and the threading contract on LaunchOptions::before_send are honest.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at 6d6faa2 (6dfc9c8 + merge of #32)

The round-1 P1 and the scan-order P2 are fixed. Admission::Full now leaves evidence on disk and drops the gen from launch_attempted so the worker can queue it; find_pending_bounded reads every .alive name, sorts newest-first, then probes (and the unit test now asserts gens 9 and 7). Those match the stated contract.

Two holes remain, both still unique-crash data loss on the launch path a crash loop actually takes (the worker never runs).

CI 38558202388: ubuntu / macOS / Windows test+lint, Android, iOS, MSRV 1.88 all SUCCESS. I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At 6d6faa2
P1 Admission::Full discarded unique evidence; worker skipped via launch_attempted	fully addressed — no discard, gen removed from launch_attempted; a_full_queue_defers_a_unique_crash_to_the_worker_instead_of_discarding_it covers the process-lives path
P2 scan_cap in readdir order before sort	fully addressed
P2 newest-first starves MAX_RECOVERY_ATTEMPTS	partially addressed — retries are in the scan set, but the pass still dies on the newest before bumping them

Remaining findings

P2 — Admission::Full still counts as a recovery attempt, so a crash loop with a full queue deletes unique evidence after 4 launches

bump_attempts runs at claim, before admit_recovered. Full un-claims for the worker and returns without discard, but the sidecar already went 1→2→3. On the 4th launch attempts > MAX_RECOVERY_ATTEMPTS (3) discards the session that was never queued.

a_full_queue_defers_… does not abort, so the worker saves it. #10’s loop (abort right after recover_on_launch_path returns) never lets the worker run. Upload already failing with 64 reports (or ~64 MiB) in queue/ is the explicit cap this PR adds; combined with a supervisor restart loop it now destroys the newest unique crash instead of keeping it for a later healthy drain.

With the default 500 ms budget a Full decision is cheap after build_report, so the pass walks the whole bounded set and every unique pending gen burns an attempt each loop.

P2 — including .attempts gens in the scan set does not make MAX_RECOVERY_ATTEMPTS progress

find_pending_bounded can return [newest, …, original], and a_session_with_failed_attempts_is_not_starved_by_newer_ones asserts that. run_launch_recovery still processes that vec newest-first. A native fault / abort in recover_one of the newest (host before_send, or assemble) is recorded as this session; the original stays at attempts=1. Next launch the recovery crash is the new newest. The original is in the list and never reached. find_pending now shares that newest-first order, so the worker path has the same starvation if it is the one that faults.

#10 wanted a crash during recovery to be recoverable; handlers do that. Prioritizing that new session forever still prevents abandoning the poison original after 3 launches, and still prevents queueing it.

Overall risk: Medium

The process-lives Full path and the newest-first scan are correct. Remaining loss is crash-loop only: unique evidence vs a full outbound queue, and the original session when recovery itself keeps dying.

Merge recommendation: request changes

Most important to fix

1. Do not count Full as a recovery attempt (restore the previous .attempts value, or bump only after Admit / intentional drop / assemble). Leave a capped-out unique session untouched until the queue can take it.
2. Make already-attempted gens actually run (or at least bump_attempts) before recovering the newest, so a fault in the newest still drives the original to MAX_RECOVERY_ATTEMPTS — and so a good original can be queued before the recovery-crash is attempted.

What looks solid

Helper thread + 1 ms poll; pass after native handler and panic hook, outside the facade lock; Full no longer deletes; scan names then sorts; crash-loop subprocess test; Full→worker test; before_send threading docs; CI green on this head.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at 4530024

Both remaining P2s from 6d6faa2 are fixed.

• Admission::Full is not an attempt. undo_attempt restores the sidecar (or removes it when the count would return to 0), so a crash loop against a full queue can no longer tick 1→2→3 and discard unique evidence. Unit test an_undone_attempt_is_not_counted covers the counter; the process-lives Full path still lands on the worker.
• Already-tried sessions run first. find_pending_bounded returns .attempts gens before fresh newest-first ([2, 6, 5]). A native fault/abort while recovering the newest still leaves the original at the head of the next launch’s list, so it can be queued or reach MAX_RECOVERY_ATTEMPTS. The worker uses the same order via find_pending.

I did not find a remaining P0–P3 with a credible failure scenario. Residual: the launch pass may spend its budget on retries before the newest fresh session; that is the tradeoff that makes the poison-pill cap work, and a same-signature crash loop still has no retries (previous session was discarded).

CI 38559675665: ubuntu / macOS / Windows test+lint, Android, iOS, MSRV 1.88 all SUCCESS. I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At 4530024
P1 Full discarded unique evidence	fully addressed (earlier)
P2 scan-cap before newest-first sort	fully addressed (earlier)
P2 Full burned MAX_RECOVERY_ATTEMPTS	fully addressed (undo_attempt)
P2 newest-first starved retries	fully addressed (retries first)

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

What looks solid

Helper-thread budget, handler install before the pass, Duplicate vs Full split, name-then-sort scan, crash-loop subprocess test, and now retries-first plus attempt undo — the launch-path guarantee holds for the supervisor loop, a full outbound queue, and a fault during recovery.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at 4530024

Both leftover P2s from 6d6faa2 are fixed. Admission::Full now gives the attempt back (undo_attempt), and find_pending_bounded returns already-tried sessions first (test: [2, 6, 5]), so a fault while recovering the newest still drives the original to MAX_RECOVERY_ATTEMPTS (or queues it). Those match what was asked.

One sibling hole remains: the other launch-path refusal that says nothing about the evidence — queue::enqueue failing — still burns the attempt and still marks the gen handled for the worker.

CI 38559675665: ubuntu / macOS / Windows test+lint, Android, iOS, MSRV 1.88 all SUCCESS. I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At 4530024
P1 Admission::Full discarded unique evidence; worker skipped via launch_attempted	fully addressed
P2 scan_cap in readdir order before sort	fully addressed
P2 newest-first starves MAX_RECOVERY_ATTEMPTS	fully addressed — retries are processed first
P2 Full burned attempts so a crash loop deleted a unique crash on launch 4	fully addressed — undo_attempt

Remaining finding

P2 — enqueue failure after Admit still counts as a recovery attempt, so a crash loop on an almost-full disk deletes unique evidence after 4 launches

admit_recovered returns Full only when 64 reports / 64 MiB are already queued. A volume that is almost full typically has few queued bundles (the writes were already failing), so the new crash is Admitted. recover_claimed has bumped .attempts (a few bytes under sessions/ — that write fits). assemble_with_extras builds the zip in memory. queue::enqueue then fails on the bundle. The Full arm is not taken: no undo_attempt, gen stays in launch_attempted.

#10’s loop (abort right after recover_on_launch_path) never lets the worker run and throws away hold_in_memory. Launch 4 hits attempts > 3 and discards a session that was never queued. That is the same data-loss shape as the last P2, on the path #9/#24 exists for (ENOSPC: sidecars succeed, the zip does not).

A failed assemble (corrupt parts) should still count. A failed queue write should not.

Overall risk: Medium

The process-lives Full path, tried-first retries, and newest-first scan are correct. Remaining loss is crash-loop + almost-full disk only.

Merge recommendation: request changes

Most important to fix

1. Treat queue::enqueue failure like Admission::Full: undo_attempt, and drop the gen from launch_attempted so a living process’s worker can retry. Leave assemble errors counting toward the poison-pill cap.

What looks solid

Helper thread + 1 ms poll; pass after native handler and panic hook, outside the facade lock; Full no longer deletes or burns attempts; retries processed first; crash-loop subprocess test; Full→worker test; before_send threading docs; CI green on this head.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at 4002558

The leftover P2 is fixed. On queue::enqueue failure, recover_one now undo_attempts (same class of refusal as Admission::Full). A crash loop on an almost-full volume can no longer tick .attempts to 4 and discard a unique crash that was never queued. assemble errors still count as poison. The launch pass keeps the gen in launch_attempted so a living process does not hold_in_memory twice — that is the right split: the crash-loop hole was the attempt counter, not the worker skip.

No remaining P0–P3 with a credible failure scenario.

CI 38561613301: MSRV 1.88 and Android lint already SUCCESS; ubuntu/macOS/Windows test+lint and iOS were still running when this review was written. I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At 4002558
P1 Full discarded unique evidence	fully addressed
P2 scan-cap before newest-first sort	fully addressed
P2 newest-first starved retries	fully addressed
P2 Full burned attempts	fully addressed
P2 enqueue failure burned attempts	fully addressed (undo_attempt; claim kept so the worker does not double-hold)

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

What looks solid

Launch-path recovery now treats both queue-cap and disk-write refusal as non-attempts, processes already-tried sessions first, and still bounds launch with the helper-thread budget after the crash handlers are installed.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at b606938

The leftover P2 is fixed. Admission::Full now sets launch_deferred. After launch_done, worker_loop swaps that flag and runs run_recovery again (claims already released; launch_attempted no longer contains the Full gen). A before_send that outruns budget + 5s can no longer strand a unique crash for the process lifetime: capture stays unblocked, and the session is picked up on the next worker tick (≤ HEARTBEAT_EVERY, 2s).

If the helper finishes inside the wait, the first worker pass already recovers it; the deferred pass is a no-op (find_pending empty). Multiple Fulls share one flag and one rescan.

No remaining P0–P3 with a credible failure scenario. Residual: no test drives slow before_send + Full past the wait (the existing full-queue test still only covers the fast path).

CI 38563683907: ubuntu/macOS test+lint, Android, iOS, MSRV 1.88 SUCCESS; Windows still pending. I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At b606938
P1 Full discarded unique evidence	fully addressed
P2 scan-cap before newest-first sort	fully addressed
P2 newest-first starved retries	fully addressed
P2 Full burned attempts	fully addressed
P2 enqueue failure burned attempts	fully addressed
P2 Full→worker skipped if helper outran the wait	fully addressed (launch_deferred + rescan in worker_loop)

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

What looks solid

Launch-path recovery now treats queue-cap and disk-write refusal as non-attempts, processes retries first, and still hands a deferred unique crash to the worker after a slow before_send.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at b606938

launch_deferred plus a follow-up run_recovery in worker_loop fixes the previous P2 on the writable-disk path: launch_done is stored only after the helper drops its claims, so the worker can pick up a unique crash that Admission::Full released after the budget+5s wait. Prior launch-path P1/P2s still hold.

The follow-up pass is aimed at the wrong directory. worker_loop's data_dir is the live capture tree (store.is_some(), None in memory/degraded mode). Startup recovery already uses drain_dir for exactly that case. swap(false) also consumes the flag when the if let Some(dir) fails, so nothing retries after the queue later drains.

CI 38563683907 SUCCESS (ubuntu / macOS / Windows test+lint, Android, iOS, MSRV 1.88). I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At b606938
P1 Full discarded unique evidence	fully addressed
P2 scan-cap before newest-first sort	fully addressed
P2 newest-first starved retries	fully addressed
P2 Full burned attempts	fully addressed
P2 enqueue failure burned attempts	fully addressed
P2 Full→worker handoff was one-shot if before_send outran the wait	fully addressed on a writable disk; residual below when the directory cannot take a new session

Remaining finding

P2 — deferred recovery uses the capture dir, so a unique Full crash is stranded for the life of a memory-mode process

See the inline comment. #9/#24 already run in memory mode against an existing recovery tree (read-only remount, ENOSPC on Session::begin) while still draining queue/ and recovering sessions. That is the situation where the outbound queue is most likely already at 64. Admission::Full does not hold_in_memory; it only sets launch_deferred. The new pickup then no-ops.

Overall risk: Medium

Crash-loop evidence is still kept (undo_attempt). A living process with a writable session dir now queues the deferred crash. The miss is the degraded directory the rest of this stack exists to handle: unique crash stays on disk after the queue drains, until the next restart.

Merge recommendation: request changes

Most important to fix

1. Run the deferred pass against drain_dir (the same path as the worker's first run_recovery), and do not swap(false) when that path is missing. Cover it with a test: full queue, before_send longer than budget + 5s, and open_disk failing / memory_only with the directory still present.

What looks solid

Gating the follow-up on launch_done (set after claims drop) is the right happens-before for the disk path. Retries-first, undo_attempt on both queue-cap and enqueue-write refusal, Duplicate vs Full, name-then-sort scan, abort-after-launch crash-loop test, and handler install before the helper are all still in good shape.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at 12d4672

The leftover P2 is fixed. Deferred recovery now uses recover_dir (drain_dir / the same tree as the worker's first run_recovery), not the live capture data_dir. launch_deferred is only swapped when that directory exists, so memory/degraded mode (#9/#24: no new session, existing tree still drained) no longer eats the flag and no-ops.

No remaining P0–P3 with a credible failure scenario. Residual: still no test for Full + before_send past the wait + unusable-but-present data dir.

CI 38563683907 successor on this head: MSRV 1.88, Android, iOS lint SUCCESS; ubuntu/macOS/Windows test+lint still running. I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At 12d4672
P1 Full discarded unique evidence	fully addressed
P2 scan-cap before newest-first sort	fully addressed
P2 newest-first starved retries	fully addressed
P2 Full / enqueue burned attempts	fully addressed
P2 Full→worker one-shot after wait	fully addressed
P2 deferred pass used capture dir (None in memory mode)	fully addressed (recover_dir = drain tree; swap only when Some)

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at 12d4672

The leftover P2 is fixed. Deferred recovery now uses recover_dir (drain_dir, the same tree as the worker's first run_recovery), not the live capture data_dir. launch_deferred.swap(false) sits inside if let Some(dir), so a missing path no longer eats the flag. In #9/#24 memory mode the existing recovery tree is still walked after a slow before_send + Admission::Full; the worker path does not cap, so a unique crash is enqueued or hold_in_memory'd.

No remaining P0–P3 with a credible failure scenario. Residual: still no test for helper-outliving-the-wait on a drain-only directory.

CI 38565086668: MSRV 1.88, Android, iOS SUCCESS; ubuntu/macOS/Windows tests still running. I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At 12d4672
P1 Full discarded unique evidence	fully addressed
P2 scan-cap before newest-first sort	fully addressed
P2 newest-first starved retries	fully addressed
P2 Full / enqueue burned attempts	fully addressed
P2 Full→worker one-shot after wait	fully addressed
P2 deferred pass used capture data_dir (None in memory mode)	fully addressed (recover_dir = drain_dir; swap only when Some)

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

What looks solid

Launch-path recovery now treats queue-cap and disk-write refusal as non-attempts, processes retries first, hands a deferred unique crash to the worker after a slow before_send, and does that on the drain tree in memory mode too.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at 12d4672

The leftover P2 is fixed. Deferred recovery now uses worker_drain_dir (the same tree as the worker's first run_recovery), and launch_deferred.swap(false) only runs when that path is Some. In memory/degraded mode (#9/#24: directory exists, open_disk failed) data_dir is still None, but the follow-up pass no longer no-ops, and it no longer eats the flag when there is nothing to recover.

launch_done is still stored only after the helper drops its claims, so a unique Admission::Full crash that outlived budget + 5s is picked up on the next worker tick (≤ HEARTBEAT_EVERY, 2s). If the helper finished inside the wait, the first worker pass already queued it and the deferred pass is a no-op.

No remaining P0–P3 with a credible failure scenario. Residual: still no test that drives Full + before_send past the grace + an unusable-but-present session dir (the existing a_full_queue_defers_… case is a writable dir and a helper that finishes inside the wait).

CI 38565086668 SUCCESS (ubuntu / macOS / Windows test+lint, Android, iOS, MSRV 1.88). I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At 12d4672
P1 Full discarded unique evidence	fully addressed
P2 scan_cap before newest-first sort	fully addressed
P2 newest-first starved retries	fully addressed
P2 Full burned attempts	fully addressed
P2 enqueue failure burned attempts	fully addressed
P2 Full→worker skipped if helper outran the wait	fully addressed
P2 deferred recovery used the capture dir (None in memory mode)	fully addressed (recover_dir = drain_dir; swap gated on Some)

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

What looks solid

Launch-path recovery now treats queue-cap and disk-write refusal as non-attempts, processes retries first, hands a deferred unique crash to the worker after a slow before_send, and does that against the recovery tree even when this process cannot take a new session.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at bb281f1

Merge of #9 (fix/issue-9) plus a 60s observable wait in a_full_queue_defers_…. This PR's launch-path machinery is intact: undo_attempt on Full and enqueue failure, retries first, launch_deferred + recover_dir (drain tree, swap only when Some). distinct_extra_dirs is a help here: recovering the same tree twice in one launch would double-bump MAX_RECOVERY_ATTEMPTS. Recovered crashes now honour the in-memory blacklist as well as the on-disk one (intentional drop, same as any_blacklisted).

No remaining P0–P3 on the launch-path work.

Overall risk: Low

Merge recommendation: approve

Stacked on #32 and #24; retarget to main after those merge.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at bb281f1

Two changes on top of 12d4672: a merge of latest #9/#24 (distinct_extra_dirs, retire_queued on every terminal queue::remove, mem-blacklist consult in recover_one), and the full-queue test now waits up to 60s on an observable marker rather than a 10s race against helper+grace.

Launch-path recovery is unchanged in contract. The #9 merge does not reopen Full/undo/deferred-handoff: extras are still worker-only, and recover_dir is still drain_dir. mem_blacklisted on recovered crashes matches the worker; the disk .blacklist check was already there.

No remaining P0–P3 with a credible failure scenario. Residual: still no test for Full + before_send past the wait on a drain-only directory (the full-queue test is a writable dir and a helper that is allowed to finish inside the wait).

I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At bb281f1
P1 Full discarded unique evidence	fully addressed
P2 scan-cap / retries-first / attempt undo	fully addressed
P2 Full→worker one-shot after wait	fully addressed
P2 deferred pass used capture dir	fully addressed

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at bb281f1

New since the last approve (12d4672): merge of #9/#24 (fbd6644) and a full-queue test that waits on list_pending length with a 60 s bound instead of 10 s.

The #9 merge does not reopen the launch-path holes. recover_dir is still the drain tree (worker_drain_dir), so memory/degraded mode still gets the deferred Full pickup. mem_blacklisted on recover_one is the same any-match the worker already applied via the on-disk blacklist; extras/legacy dirs stay worker-only, as DESIGN.md §8 says. distinct_extra_dirs / retire_queued are #9 correctness, not a new #10 interaction.

The test change is a flake bound, not a product fix: helper + worker wait is budget + 5 s, then at most HEARTBEAT_EVERY. 60 s with the marker in the timeout message is the right assertion for a loaded CI machine.

I re-checked the previous failure scenarios against this tree (Full discard, scan-before-sort, retries starved, Full/enqueue burning attempts, one-shot Full→worker, deferred pass on data_dir == None). They still do not reproduce.

No remaining P0–P3 with a credible failure scenario. Residual: still no test that drives Full + before_send past the grace on a drain-only directory (the existing full-queue case is a writable dir and a helper that finishes inside the wait).

CI 38566562287 was still running on this head. Last green: 38565086668 at 12d4672. I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At bb281f1
P1 Full discarded unique evidence	fully addressed
P2 scan_cap before newest-first sort	fully addressed
P2 newest-first starved retries	fully addressed
P2 Full burned attempts	fully addressed
P2 enqueue failure burned attempts	fully addressed
P2 Full→worker skipped if helper outran the wait	fully addressed
P2 deferred recovery used the capture dir (None in memory mode)	fully addressed

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

What looks solid

Launch-path recovery still treats queue-cap and disk-write refusal as non-attempts, processes retries first, hands a deferred unique crash to the worker after a slow before_send (including on the drain tree in memory mode), and the abort-after-launch subprocess test still matches #10.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at e00b09d

Stack merge of latest #14 (which merged #9, which merged main). Launch-path files (runtime.rs, recovery.rs, queue.rs, launch_recovery tests) are unchanged from bb281f1. The only facade delta is Bugsee::upload building manual_upload_meta() off the global lock (#25); recover_on_launch_path still runs last, after bugsee_native::install and the panic hook.

Prior launch-path P1/P2s still hold. No remaining P0–P3 with a credible failure scenario. Residual unchanged: no Full + slow before_send + drain-only-dir test.

I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at e00b09d

This synchronize is a merge of latest #14 (e01a20a, which itself merged #9 and main, including #23 CrashBackend and #27 hostinfo) into fix/issue-10. Launch-path sources (runtime.rs, recovery.rs, queue.rs, launch_recovery tests) are byte-identical to bb281f1. recover_on_launch_path still runs last in launch_with, after bugsee_native::install (now a CrashBackend wrapper) and the panic hook. The only facade delta on this merge is Bugsee::upload building manual_upload_meta() off the global lock (#25).

Prior launch-path P1/P2s still hold. The new hole is not in #10 logic: the merge imported a broken crates/bugsee/Cargo.toml from #32/#14, and this branch no longer loads.

CI 38577801406: ubuntu / Windows / MSRV 1.88 / Android / iOS-sim all FAILURE at cargo metadata (~13–44s). Last green on this branch: 38566562287 at bb281f1. I did not run tests locally (agent rustc 1.83, crate MSRV 1.88).

Findings

P0 — duplicate [target.'cfg(unix)'.dependencies] in crates/bugsee/Cargo.toml makes the workspace unloadable

• Location: crates/bugsee/Cargo.toml:20 (libc = "0.2") and :48 (libc = { version = "0.2", optional = true } plus telemetry = ["dep:sysinfo", "dep:libc"] at :35)
• Problem: Cargo forbids two tables with the same name. e00b09d kept both: the always-on unix libc this crate already needed (options.rs geteuid, fork tests) and the second table #27 added for hostinfo.
• Impact: cargo metadata fails; every CI job that parses the workspace dies before compile. The branch cannot be built, tested, or merged. bb281f1 was green; this merge is what made it red. Same duplicate is on #32 (fix/issue-14) — inherited, not unique #10 code — but it is now this PR's HEAD.
• Scenario: git merge origin/fix/issue-14 did not conflict (the tables sit at different lines). cargo check / CI then: error: duplicate key at line 48, failed to load manifest for workspace member crates/bugsee.
• Fix: one unix-dependencies table. Keep non-optional libc = "0.2" (data-dir uid and tests need it with telemetry off). Drop the second table and restore telemetry = ["dep:sysinfo"]. hostinfo.rs is already cfg(feature = "telemetry") and can use the always-on unix dep. Prefer landing that on #32 and remarging; collapsing it here also unblocks this PR.

Prior findings

Finding	At e00b09d
P1 Full discarded unique evidence	fully addressed
P2 scan_cap before newest-first sort	fully addressed
P2 newest-first starved retries	fully addressed
P2 Full / enqueue burned attempts	fully addressed
P2 Full o worker skipped if helper outran the wait	fully addressed
P2 deferred recovery used the capture dir (None in memory mode)	fully addressed

Overall risk: High

Launch-path recovery is unchanged and still correct. The merge made the crate unloadable, so the #10 guarantee cannot be exercised on this head.

Merge recommendation: request changes

Do not merge until cargo metadata succeeds. Unique #10 work does not need another pass after the manifest is collapsed.

Most important to fix

1. Collapse the two [target.'cfg(unix)'.dependencies] tables (on #32, or here) so CI is green again.

What looks solid

Helper-thread budget after native handler and panic hook, retries-first plus undo_attempt on queue-cap and enqueue-write refusal, launch_deferred on the drain tree, abort-after-launch crash-loop test. CrashBackend install() is still the same entry launch_with uses.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at e00b09d

Merge of origin/fix/issue-14 (e01a20a: #14 + #9 + main #21–#29) onto last-reviewed bb281f1. Unique launch-path files (runtime.rs, recovery.rs, queue.rs, tests) are unchanged. Incoming origin/CrashBackend/hostinfo do not reopen the #10 holes: recovered reports still set source.origin: None, and launch_with still installs the native handler then the panic hook before recover_on_launch_path.

The remarge left the workspace unloadable.

P0 — duplicate [target.'cfg(unix)'.dependencies] tables, so cargo never starts

crates/bugsee/Cargo.toml has both:

• line 20: libc = "0.2" (required; this crate always calls libc::geteuid from options.rs for the private temp dir)
• line 48: libc = { version = "0.2", optional = true } plus telemetry = ["dep:sysinfo", "dep:libc"] from #27

TOML forbids two dependencies keys on the same target table. Confirmed locally:

error: invalid table header
duplicate key `dependencies` in table `target.cfg(unix)`
 --> crates/bugsee/Cargo.toml:48:1

cargo metadata and cargo fmt --all --check fail before any compile. CI 38577801406: MSRV build, Ubuntu format, Windows build, Android clippy all fail in seconds. The same duplicate is on base e01a20a; this remarge imported it (it is not in the three-dot unique diff).

Fix: one unix table, required libc = "0.2". Drop dep:libc from telemetry — the comment on the second table already says libc is in the tree. Optional-only libc would break options.rs when telemetry is off.

Prior findings

Finding	At e00b09d
P1 Full discarded unique evidence	fully addressed
P2 scan-cap before newest-first sort	fully addressed
P2 newest-first starved retries	fully addressed
P2 Full / enqueue burned attempts	fully addressed
P2 Full→worker one-shot after wait	fully addressed
P2 deferred pass used capture data_dir	fully addressed

Overall risk: High

Unique #10 recovery is still Low. HEAD cannot be built or tested.

Merge recommendation: request changes

Most important to fix

1. Collapse the two unix libc tables so cargo loads. Unique launch-path code needs no further change.

What looks solid

Helper-thread budget after the native handler and panic hook, retries-first plus undo_attempt on queue-cap and enqueue-write refusal, Duplicate vs Full, name-then-sort scan, launch_deferred on the drain tree, abort-after-launch crash-loop test. Residual: still no Full + slow before_send + drain-only-dir test.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at 66e4f18

The P0 is fixed. One [target.'cfg(unix)'.dependencies] table, required libc = "0.2", and telemetry = ["dep:sysinfo"] (no dep:libc). I confirmed cargo metadata --offline --no-deps succeeds on this head. --no-default-features still gets unix libc for options.rs geteuid. Launch-path sources are unchanged from bb281f1 / e00b09d.

No remaining P0–P3 with a credible failure scenario. Residual: still no Full + slow before_send + drain-only-dir test.

I did not run the test suite locally (agent rustc 1.83, crate MSRV 1.88).

Prior findings

Finding	At 66e4f18
P0 duplicate unix libc tables	fully addressed
P1 Full discarded unique evidence	fully addressed
P2 scan-cap / retries-first / attempt undo	fully addressed
P2 Full→worker one-shot after wait	fully addressed
P2 deferred pass used capture dir	fully addressed

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at 66e4f18

Remarge of #14 after 1ecfca8 (de-duplicate unix libc). Unique launch-path files are unchanged from e00b09d / bb281f1. The only delta is crates/bugsee/Cargo.toml: one [target.'cfg(unix)'.dependencies] table with required libc = "0.2", and telemetry = ["dep:sysinfo"] (no dep:libc).

Confirmed locally: cargo metadata --offline --no-deps exits 0. recover_on_launch_path still runs last in launch_with (after bugsee_native::install and the panic hook).

Prior findings

Finding	At 66e4f18
P0 duplicate unix libc tables, workspace unloadable	fully addressed
P1 Full discarded unique evidence	fully addressed
P2 scan_cap / retries-first / attempt undo	fully addressed
P2 Full→worker one-shot after wait	fully addressed
P2 deferred pass used capture dir	fully addressed

No remaining P0–P3 with a credible failure scenario. Residual: still no Full + slow before_send + drain-only-dir test.

CI 38579231486: MSRV 1.88 and Android lint already SUCCESS; ubuntu/macOS/Windows tests and iOS still running. I did not run cargo test locally (agent rustc 1.83, crate MSRV 1.88).

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

What looks solid

Helper-thread budget after the native handler and panic hook, retries-first plus undo_attempt on queue-cap and enqueue-write refusal, launch_deferred on the drain tree, abort-after-launch crash-loop test, and the workspace loads again.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Re-review at 66e4f18

Synchronize is a remarge of latest #32 (fbbabb2, which includes 1ecfca8). Unique launch-path files (runtime.rs, recovery.rs, queue.rs, launch_recovery tests, options.rs) are unchanged from bb281f1. recover_on_launch_path still runs last in launch_with, after bugsee_native::install and the panic hook, outside the facade lock. Recovered reports still set source.origin: None.

The previous P0 is fixed: crates/bugsee/Cargo.toml now has a single [target.'cfg(unix)'.dependencies] table with required libc = "0.2", and telemetry is dep:sysinfo only. Confirmed locally: cargo metadata --offline --no-deps exits 0 (it failed at e00b09d with duplicate key 'dependencies' in table 'target.cfg(unix)').

I re-checked the prior failure scenarios against this tree (Full discard, scan-before-sort, retries starved, Full/enqueue burning attempts, one-shot Full→worker, deferred pass on data_dir == None). They still do not reproduce. Admission::Full still undo_attempts, drops the gen from launch_attempted, and sets launch_deferred; enqueue-write refusal still undo_attempts and keeps the claim; deferred pickup still uses recover_dir (drain_dir) and only swaps when that path is Some.

No remaining P0–P3 with a credible failure scenario. Residual unchanged: still no test for Full + before_send past budget + 5s on a drain-only directory (the existing full-queue case is a writable dir and a helper that is allowed to finish inside the wait).

CI 38579231486: MSRV 1.88, Ubuntu test+lint, Windows test+lint, Android lint SUCCESS. macOS test+lint, iOS lint, and the private-API job were still running when this review was written. I did not run the crate tests locally (agent rustc 1.83, crate MSRV 1.88); cargo metadata was run and succeeded.

Prior findings

Finding	At 66e4f18
P0 duplicate unix libc tables (unloadable workspace)	fully addressed (1ecfca8 on #32, remarged)
P1 Full discarded unique evidence	fully addressed
P2 scan_cap before newest-first sort	fully addressed
P2 newest-first starved retries	fully addressed
P2 Full / enqueue burned attempts	fully addressed
P2 Full→worker skipped if helper outran the wait	fully addressed
P2 deferred recovery used the capture dir (None in memory mode)	fully addressed

Overall risk: Low

Merge recommendation: approve

Stacked on #32; retarget to main after that merges.

What looks solid

Workspace loads again. Launch-path recovery still treats queue-cap and disk-write refusal as non-attempts, processes retries first, hands a deferred unique crash to the worker after a slow before_send (including on the drain tree in memory mode), and the abort-after-launch subprocess test still matches #10.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Re-review at e247036 (empty remarge of main’s #14 squash)

git diff 66e4f18..e247036 is empty. Merge message: “Tree unchanged: this branch already contains #14's content.” Unique #10 files are unchanged from the last approve (bb281f1 / 66e4f18). GitHub now shows this PR merged into main (6494a4fa).

Re-read the launch-path machinery against callers, worker handoff, and tests — not just the remarge.

What I re-checked

• Bugsee::launch_with still runs recover_on_launch_path last, after the native handler and panic hook, outside every lock.
• Helper thread + budget poll (no park); panics contained; launch_done set even if spawn fails.
• find_pending_bounded: full name scan, sort newest-first, retries (.attempts) processed first, probe cap limit * 16.
• Admission::Full / enqueue-write failure: undo_attempt, evidence kept; Full also clears launch_attempted and sets launch_deferred.
• Worker wait is one-shot (budget + 5s); worker_loop then runs a second run_recovery on recover_dir (drain_dir) after launch_done, so a Full that outlives the wait is not stranded — including memory mode with a still-present recovery tree.
• Claims (#14) keep the helper and worker exclusive; recovered crashes still take no scope from the new process.
• crates/bugsee/Cargo.toml: one unix libc = "0.2" table (plus unix dev-dependencies). cargo metadata --offline --no-deps exits 0.

Prior findings

Finding	At e247036
P0 duplicate unix libc tables	fully_addressed
P1 Full discarded unique evidence	fully_addressed
P2 scan-cap / retries-first / attempt undo	fully_addressed
P2 Full → worker one-shot after wait	fully_addressed
P2 deferred pass used capture dir	fully_addressed

No remaining P0–P3 with a credible failure scenario.

Residual (not blocking): there is still no test for Full + before_send longer than budget + 5s + drain-only / memory mode. The recover_dir path covers it in code; a_full_queue_defers_a_unique_crash_to_the_worker only hits the writable-disk, helper-finishes-inside-wait case.

CI

38581434695 on e247036: all 8 jobs SUCCESS (MSRV 1.88, Ubuntu, Windows, Android lint, macOS tests, macOS private-API, iOS device + sim). Local cargo test not run (agent rustc 1.83 vs MSRV 1.88).

Verdict

1. Overall risk: Low
2. Merge recommendation: Already merged; this SHA would have been approved.
3. Most important issues to fix: None remaining.
4. Positives: The #10 contract is intact after the #14 squash remarge: bounded launch-path queueing, retries-first so a recovery fault still reaches MAX_RECOVERY_ATTEMPTS, and unique crashes the queue cap refuses stay on disk for the worker instead of being destroyed. Crash-loop test still covers abort-after-launch collapsing repeats to one queued report.

  

_{Sent by Cursor Automation: Bugsee code review}