Repository navigation
Conversation
|
Updated per @AmethystLiang's recommendations on the issue (#9949 (comment) — 18:09Z comment):
Also |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between b1a6dd069fdae441198b9b3172082c8476f09a64 and 907a08e. 📒 Files selected for processing (19)
🚧 Files skipped from review as they are similar to previous changes (18)
📝 WalkthroughWalkthroughThe changes add structured attribution to daemon and runtime kill-capable RPC events, including request, device, origin, and target details. Renderer cleanup paths now record 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)src/renderer/src/i18n/locales/es.jsonTraceback (most recent call last): src/renderer/src/i18n/locales/en.jsonTraceback (most recent call last): src/renderer/src/i18n/locales/ja.jsonTraceback (most recent call last):
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a3112bd4-563f-4338-b264-1f4557f9633d
📥 Commits
Reviewing files that changed from the base of the PR and between 01bcc57 and 70ff80abdc7eb30ef90284db14dbce7c9776e4bd.
📒 Files selected for processing (19)
src/main/daemon/daemon-server-kill-attribution.test.tssrc/main/daemon/daemon-server.tssrc/main/runtime/runtime-rpc.test.tssrc/main/runtime/runtime-rpc.tssrc/renderer/src/components/shared/kill-all-terminal-surfaces.test.tssrc/renderer/src/components/shared/kill-all-terminal-surfaces.tssrc/renderer/src/components/sidebar/sleep-worktree-flow.test.tssrc/renderer/src/components/sidebar/sleep-worktree-flow.tssrc/renderer/src/components/status-bar/ResourceUsageStatusSegment.tsxsrc/renderer/src/components/status-bar/orphan-terminal-kill-gate.test.tssrc/renderer/src/components/status-bar/orphan-terminal-kill-gate.tssrc/renderer/src/hooks/useIpcEvents.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/lib/agent-hibernation-coordinator.test.tssrc/renderer/src/lib/agent-hibernation-coordinator.ts
|
Rebased onto current Conflict resolution (2nd commit only):
Focused tests green (109): kill-attribution, runtime-rpc, orphan-kill-gate, kill-all surfaces, sleep-worktree, hibernation coordinator. Heads: |
3f6e39f to
b1a6dd0
Compare
… sweep An unattributable close storm mass-killed ~25 agent PTY sessions across ~20 worktrees while the app stayed alive (stablyai#9949). No mass-kill path recorded who invoked it, so the initiator could not be identified. - daemon: session-killed log now carries clientId + requestId - renderer: one terminal_mass_kill breadcrumb per invocation at every mass-kill entry (kill-all surfaces, worktree sleep, hibernation tick, orphan sweep) - runtime-rpc: kill-capable dispatches (both WS and local-socket transports) log an attribution line naming the calling device/scope - resource manager: bulk "Kill N orphan terminals" now requires a confirmation dialog, and confirm re-filters against live tab bindings so a session adopted while the dialog is open is never swept
… kill subcommands A packaged GUI launch discards stdout, exactly where the stablyai#9949 kills happened, so the console-only audit line left no post-incident record. Route the record through the durable crash-breadcrumb store (which also lands in the persisted trace) with console.info kept as the dev echo. Audit agentTeams.tmuxCompat only for kill-pane/kill-window/kill-session subcommands; the method itself is high-volume (list-panes polling).
Restack noteRebased onto latest Scope (unique vs main):
Coordinates with (do not duplicate UX):
Prefer one confirm surface for orphan/unbound bulk actions: this PR’s confirm gate for orphan sweep; #8467’s review dialog for idle-shell classification. #8888 should soft-deny at the protocol layer without adding a third confirm modal. |
b1a6dd0 to
907a08e
Compare
|
Superseded by #10129. The deterministic mixed-generation harness reproduces the burst through sequential reasonless |
|
Closing in favor of #10129 per @OrcaWin's review — agreed that the orphan-sweep confirmation guard here does not intercept the Noting for the record that the kill-attribution diagnostics in this PR (daemon-side attribution audit + the |
Addresses #9949 — implements both asks from the issue: make every mass-terminal-kill path attributable in one log read, and put a confirmation gate on the one bulk kill that had none.
Background
On 2026-07-22T14:04:33Z, ~25 daemon PTY sessions across ~20 worktrees were killed in ~1.5 s by a sequential per-terminal close storm executed through the desktop app, while the app itself stayed alive (continuous renderer heartbeat, no daemon restart, no new daemon client). Post-incident forensics ruled out the updater (#8261), Manage-Sessions/status-bar killAll, mass worktree sleep (zero keepHistory checkpoints, no sleep records), and agent hibernation (feature flag off). The kill signature matches per-session
pty:kill(immediate:true) plus aterminal.close→RuntimePtyController.kill(immediate:false) leg for runtime-tracked terminals.The investigation dead-ended on attribution: the daemon's
session-killedlog carries no client/request identity, none of the renderer's mass-kill entry points leave a breadcrumb, and runtime RPC dispatches (orca-cli / paired devices — two were active that day) log nothing durable. This PR closes that gap so the next occurrence is diagnosable from the logs directly.Changes
Daemon (
daemon-server.ts): thesession-killedlog line now includesclientIdandrequestId. (keepHistoryis not in the kill protocol payload; the protocol is untouched.)Renderer — one best-effort
terminal_mass_killcrash-breadcrumb per invocation (never per session) at every mass-kill entry point, landing in the persistedrenderer.breadcrumbtrace:kill-all-surfacesrunKillAllTerminalSurfaces(injectable dependency, tested)worktree-sleeprunSleepWorktrees(covers UI multi-sleep and externally-relayedui:sleepWorktree; single producer, no relay-site duplicate)hibernationrunAgentHibernationTickwhen candidates > 0orphan-sweepRuntime RPC (
runtime-rpc.ts): dispatches of kill-capable methods (terminal.close/closeTab/stop/stopExact,session.tabs.close,agentTeams.tmuxCompat,worktree.sleep/rm) log one attribution line with the calling device id/name/scope on both transports — WebSocket (paired devices) and the local socket (orca-cli), the latter with a syntheticlocal-socketorigin. Ids-only target summary, no payload dumps.Resource Manager (
ResourceUsageStatusSegment.tsx+ neworphan-terminal-kill-gate.ts): the bulk "Kill N orphan terminals" button no longer kills on click. It stages the orphan snapshot and opens a confirmation dialog; kills run only from confirm, which re-filters against live tab bindings so a session adopted by a tab while the dialog sits open is never swept (the no-confirm path was the #8459 hazard). Per-row single-session kill is unchanged. New strings localized in es/ja/ko/zh following the file's existing conventions.Out of scope
No kill-semantics or ordering changes; no daemon protocol changes; the underlying external trigger of the 2026-07-22 incident remains unidentified — this instrumentation is what identifies it next time.
Testing
tscnode + web projects: no errors.oxlint: clean. max-lines ratchet: no new bypasses. Localization catalog + locale parity: pass (coverage check fails only on 3 pre-existingai-vaultstrings present on the base commit).agentTeams.tmuxCompatkill path, and the confirm-time rebind race.