Skip to content

fix(session): mark session Stopped even when daemon is unreachable#639

Merged
Wirasm merged 2 commits intomainfrom
fix/stop-daemon-unreachable
Mar 23, 2026
Merged

fix(session): mark session Stopped even when daemon is unreachable#639
Wirasm merged 2 commits intomainfrom
fix/stop-daemon-unreachable

Conversation

@Wirasm
Copy link
Copy Markdown
Owner

@Wirasm Wirasm commented Mar 19, 2026

Summary

  • Consolidate is_daemon_unreachable helper from list.rs into DaemonClientError::is_unreachable() method on the error type itself
  • In stop_session(), classify daemon errors at collection point — unreachable errors (NotRunning, ConnectionFailed, ProtocolError, Io) are treated as "PTY already dead" with warn! instead of being pushed to daemon_errors
  • The early return at line 124 now only fires for real DaemonError protocol errors where the daemon is alive
  • Apply same fix to stop_teammate() which had identical unconditional error mapping

Test plan

  • cargo fmt --check passes
  • cargo clippy --all -- -D warnings passes
  • cargo test -p kild-core passes (5 new tests for error classification)
  • Manual: kild create test --daemon → kill daemon → kild stop test → session marked Stopped

When the daemon isn't running, `kild stop` would fail with DaemonError
and leave the session stuck Active on disk. The early return at
stop.rs:124 fired before the session status was updated.

Consolidate the `is_daemon_unreachable` check from list.rs into a proper
`is_unreachable()` method on DaemonClientError. Use it in both
stop_session() and stop_teammate() to treat unreachable daemon errors
(NotRunning, ConnectionFailed, ProtocolError, Io) as "PTY already dead"
instead of blocking the stop flow.
@Wirasm
Copy link
Copy Markdown
Owner Author

Wirasm commented Mar 19, 2026

PR Review Summary

Reviewed by: code-reviewer, pr-test-analyzer, silent-failure-hunter, type-design-analyzer, code-simplifier
Skipped: docs-impact-agent (internal refactor, no public API/config/docs changes)

Critical Issues (0 found)

None.

Important Issues (0 found)

None.

Suggestions (1 found)

Agent Suggestion Location
code-reviewer destroy.rs:219 only special-cases NotRunning for daemon errors, not all unreachable variants (ConnectionFailed, ProtocolError, Io). Consider adopting is_unreachable() there too for consistency. Not blocking — separate concern, separate PR. crates/kild-core/src/sessions/destroy.rs:219

Strengths

  • Clean consolidation — moving is_daemon_unreachable from a private function in list.rs to DaemonClientError::is_unreachable() follows SRP and makes the classification reusable across list.rs, stop.rs, and any future consumer
  • Safe default for future variants — the !matches!(DaemonError { .. }) approach means new error variants are classified as unreachable by default, which is the correct safety posture
  • Tests fully migrated — all 5 classification tests moved to their natural home on the error type with no coverage loss
  • stop_teammate fix — previously any daemon error was a hard failure; now unreachable errors are gracefully handled, matching the intent that "daemon dead = PTY dead = stop succeeded"
  • Event naming — new _unreachable events follow the established {layer}.{domain}.{action}_{state} convention

Verdict

READY TO MERGE

Clean, focused refactor that fixes a real bug (stop failing when daemon is unreachable) with proper error classification, good test coverage, and no regressions.

@Wirasm Wirasm merged commit 35a68b9 into main Mar 23, 2026
6 checks passed
@Wirasm Wirasm deleted the fix/stop-daemon-unreachable branch March 23, 2026 12:41
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.

1 participant