Skip to content

fix(daemon,cli): graceful systemd stop; memory subcommand --help handling (canary findings)#1612

Merged
Aaronontheweb merged 1 commit into
netclaw-dev:feature/memory-embeddingsfrom
Aaronontheweb:fix/canary-shutdown-and-help
Jul 9, 2026
Merged

fix(daemon,cli): graceful systemd stop; memory subcommand --help handling (canary findings)#1612
Aaronontheweb merged 1 commit into
netclaw-dev:feature/memory-embeddingsfrom
Aaronontheweb:fix/canary-shutdown-and-help

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Summary

Two production-canary regressions, root-caused and fixed.

Bug A — systemctl --user stop netclaw.service lands in failed (Result: signal)

Root cause: DaemonManager.StopAsync (netclaw daemon stop, the unit's ExecStop=) sent SIGTERM and waited only 10 seconds before giving up and self-escalating to SIGKILL. But the daemon's own Akka CoordinatedShutdown before-service-unbind phase — where SessionDrainHelper.DrainAsync runs — is deliberately allotted 200 seconds, specifically because an in-flight LLM turn (SessionConfig.TurnLlmTimeout, default 3 minutes) must be allowed to finish before a session can passivate. When a session was genuinely still mid-LLM-call at shutdown, the CLI's 10s window elapsed long before the daemon's own graceful drain could complete, so the CLI itself sent SIGKILL. The daemon died, so netclaw daemon stop reported success (exit 0) — but via a hard kill mid-shutdown, not a clean exit. That SIGKILL is exactly what systemd's failed (Result: signal) was observing.

A secondary contributor: the generic host's HostOptions.ShutdownTimeout was a bare 30s, also shorter than the 200s Akka phase it's supposed to bound, and the generated systemd unit had no explicit TimeoutStopSec=, leaving it at systemd's own (typically ~90s) default — also short of 200s.

Fix: introduced DaemonConfig.GracefulShutdownBudget (200s) as the single source of truth, now shared by:

  • the Akka coordinated-shutdown.phases.before-service-unbind.timeout HOCON (via a new, testable DaemonShutdownConfiguration.BuildCoordinatedShutdownHocon)
  • the host's HostOptions.ShutdownTimeout (budget + 30s headroom)
  • DaemonManager.StopAsync's SIGTERM grace period (was hardcoded 10s, now the full budget)
  • the generated systemd unit's new TimeoutStopSec= (budget + 30s), so systemd never SIGKILLs the whole cgroup out from under a still-legitimately-waiting ExecStop=

Also fixed a latent bug in DaemonManager.WaitForExitAsync: its poll loop used a bare real-time Task.Delay(200) instead of routing through the injected TimeProvider (the convention used everywhere else in this repo, e.g. ConfigWatcherService). Fixing this was required to test the new ~200s wait deterministically with FakeTimeProvider instead of a real sleep, and closes a design inconsistency that could otherwise mask the same class of bug in tests.

The success message now distinguishes a clean exit from a forced kill, so operators get a signal if this recurs.

⚠️ Existing installs must re-run netclaw daemon install (or netclaw daemon uninstall && netclaw daemon install) to regenerate the unit file with the new TimeoutStopSec=. Will call this out in release notes.

Untested live: per instructions, the live daemon on this canary host was not stopped/restarted to validate. Validated via unit tests (see below) + code reading of the Akka.Hosting/CoordinatedShutdown/systemd interaction.

Bug B — netclaw memory backfill-embeddings --help executes instead of printing help

Root cause: MemoryCommand.RunAsync only checked args[1] (the subcommand slot) for a help token before dispatching. netclaw memory --help worked (subcommand is --help), but netclaw memory backfill-embeddings --help had subcommand "backfill-embeddings" — not a help token — so it fell through to RunBackfillEmbeddingsAsync, which has no required positional args (only optional --force), silently absorbed the trailing --help, and ran the real provision-and-embed pass.

Fix: added CliArgsParser.HasTrailingHelpToken(args, startIndex) — scans the full argument list, not just the subcommand slot — and wired it into MemoryCommand.

Audit of other subcommand handlers found the same pattern in three more places, all fixed:

  • netclaw daemon start/stop/status/install/uninstall --help (Program.cs) — most severe: e.g. daemon stop --help would have actually stopped the daemon, daemon install --help would install the systemd unit, etc. Extracted into a new, independently testable DaemonCommandDispatch.ShouldShowHelpInsteadOfExecuting (Program.cs is top-level statements and can't be unit-tested directly — mirrors the existing DaemonCliArgs extraction pattern for the same reason).
  • netclaw webhooks list/show/delete/validate --help — e.g. webhooks list --help still listed routes. Fixed; webhooks set --help is excluded since it already had its own more-specific WriteSetHelp().
  • netclaw reminder <subcommand> --help — e.g. reminder list --help still hit the live daemon. Fixed for all subcommands.

Audited but left unchanged (confirmed non-destructive, just a confusing error message instead of help — e.g. netclaw mcp get --help prints "MCP server '--help' not found" rather than crashing or mutating anything): model, provider, mcp add/get/remove/enable/disable/auth, skill, approvals. These all have positional-arg-count guards that catch the case before anything executes.

Baseline impact: none — no help text changed anywhere, only when help is shown vs. the real action runs. tests/smoke/tapes/screenshots/help.tape captures netclaw --help (top-level WriteGeneralHelp()), which this PR never touches, so help.approved.png needs no update.

Test coverage

  • DaemonConfigTests.GracefulShutdownBudget_exceeds_default_TurnLlmTimeout — ties the shared constant to SessionConfig.TurnLlmTimeout's default.
  • DaemonShutdownConfigurationTests — the CoordinatedShutdown HOCON interpolates the budget correctly.
  • DaemonManagerGracefulShutdownTestsWaitForExitAsync returns true immediately for an already-exited process, returns false once a FakeTimeProvider is advanced past the budget (no real sleep — proves the poll loop is now fully virtualized), and BuildDaemonUnitContent emits the expected TimeoutStopSec=.
  • MemoryCommandTestsbackfill-embeddings --help/-h/help prints help and does not touch the embeddings store (seeded doc stays unembedded, vs. the existing "real run" test that embeds it).
  • CliArgsParserTestsHasTrailingHelpToken unit coverage.
  • DaemonCommandDispatchTests — the extracted daemon lifecycle-verb guard.
  • WebhooksCommandTestslist --help/-h prints help without listing; set --help still shows its own more-specific help (not shadowed).
  • ReminderCommandTests (new file) — list --help prints help without requiring/reaching the daemon; plain list still requires it (regression guard); create <full args> --help also short-circuits.

All new/changed suites green: Netclaw.Cli.Tests (1282), Netclaw.Daemon.Tests (858), Netclaw.Configuration.Tests (468). Full solution build clean. dotnet slopwatch analyze: 0 issues. Header verify: pass.

Test plan

  • dotnet build Netclaw.slnx — clean
  • dotnet test src/Netclaw.Cli.Tests — 1282 passed
  • dotnet test src/Netclaw.Daemon.Tests — 858 passed
  • dotnet test src/Netclaw.Configuration.Tests — 468 passed
  • dotnet slopwatch analyze — 0 issues
  • ./scripts/Add-FileHeaders.ps1 -Verify — pass
  • Live canary validation of systemctl --user stop netclaw.service reaching clean inactive — intentionally not done in this session (do not restart/stop the live daemon); recommend validating on next daemon deploy/restart window.

@Aaronontheweb
Aaronontheweb merged commit f822b7d into netclaw-dev:feature/memory-embeddings Jul 9, 2026
15 checks passed
Aaronontheweb added a commit that referenced this pull request Jul 10, 2026
Operational alert on model provisioning failure (#1611), embedder-failure
no longer blocks relevance model (#1611), graceful daemon shutdown budget
(#1612), --help no longer executes commands (#1612). Memory eval 6/6
(run 6cc18657); code validated by full PR CI on #1611/#1612.
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