fix(daemon,cli): graceful systemd stop; memory subcommand --help handling (canary findings)#1612
Merged
Aaronontheweb merged 1 commit intoJul 9, 2026
Conversation
…ling (canary findings)
Aaronontheweb
merged commit Jul 9, 2026
f822b7d
into
netclaw-dev:feature/memory-embeddings
15 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two production-canary regressions, root-caused and fixed.
Bug A —
systemctl --user stop netclaw.servicelands infailed (Result: signal)Root cause:
DaemonManager.StopAsync(netclaw daemon stop, the unit'sExecStop=) sent SIGTERM and waited only 10 seconds before giving up and self-escalating to SIGKILL. But the daemon's own Akka CoordinatedShutdownbefore-service-unbindphase — whereSessionDrainHelper.DrainAsyncruns — 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, sonetclaw daemon stopreported success (exit 0) — but via a hard kill mid-shutdown, not a clean exit. That SIGKILL is exactly what systemd'sfailed (Result: signal)was observing.A secondary contributor: the generic host's
HostOptions.ShutdownTimeoutwas a bare 30s, also shorter than the 200s Akka phase it's supposed to bound, and the generated systemd unit had no explicitTimeoutStopSec=, 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:coordinated-shutdown.phases.before-service-unbind.timeoutHOCON (via a new, testableDaemonShutdownConfiguration.BuildCoordinatedShutdownHocon)HostOptions.ShutdownTimeout(budget + 30s headroom)DaemonManager.StopAsync's SIGTERM grace period (was hardcoded 10s, now the full budget)TimeoutStopSec=(budget + 30s), so systemd never SIGKILLs the whole cgroup out from under a still-legitimately-waitingExecStop=Also fixed a latent bug in
DaemonManager.WaitForExitAsync: its poll loop used a bare real-timeTask.Delay(200)instead of routing through the injectedTimeProvider(the convention used everywhere else in this repo, e.g.ConfigWatcherService). Fixing this was required to test the new ~200s wait deterministically withFakeTimeProviderinstead 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.
netclaw daemon install(ornetclaw daemon uninstall && netclaw daemon install) to regenerate the unit file with the newTimeoutStopSec=. 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 --helpexecutes instead of printing helpRoot cause:
MemoryCommand.RunAsynconly checkedargs[1](the subcommand slot) for a help token before dispatching.netclaw memory --helpworked (subcommand is--help), butnetclaw memory backfill-embeddings --helphad subcommand"backfill-embeddings"— not a help token — so it fell through toRunBackfillEmbeddingsAsync, 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 intoMemoryCommand.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 --helpwould have actually stopped the daemon,daemon install --helpwould install the systemd unit, etc. Extracted into a new, independently testableDaemonCommandDispatch.ShouldShowHelpInsteadOfExecuting(Program.cs is top-level statements and can't be unit-tested directly — mirrors the existingDaemonCliArgsextraction pattern for the same reason).netclaw webhooks list/show/delete/validate --help— e.g.webhooks list --helpstill listed routes. Fixed;webhooks set --helpis excluded since it already had its own more-specificWriteSetHelp().netclaw reminder <subcommand> --help— e.g.reminder list --helpstill 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 --helpprints "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.tapecapturesnetclaw --help(top-levelWriteGeneralHelp()), which this PR never touches, sohelp.approved.pngneeds no update.Test coverage
DaemonConfigTests.GracefulShutdownBudget_exceeds_default_TurnLlmTimeout— ties the shared constant toSessionConfig.TurnLlmTimeout's default.DaemonShutdownConfigurationTests— the CoordinatedShutdown HOCON interpolates the budget correctly.DaemonManagerGracefulShutdownTests—WaitForExitAsyncreturns true immediately for an already-exited process, returns false once aFakeTimeProvideris advanced past the budget (no real sleep — proves the poll loop is now fully virtualized), andBuildDaemonUnitContentemits the expectedTimeoutStopSec=.MemoryCommandTests—backfill-embeddings --help/-h/helpprints help and does not touch the embeddings store (seeded doc stays unembedded, vs. the existing "real run" test that embeds it).CliArgsParserTests—HasTrailingHelpTokenunit coverage.DaemonCommandDispatchTests— the extracted daemon lifecycle-verb guard.WebhooksCommandTests—list --help/-hprints help without listing;set --helpstill shows its own more-specific help (not shadowed).ReminderCommandTests(new file) —list --helpprints help without requiring/reaching the daemon; plainliststill requires it (regression guard);create <full args> --helpalso 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— cleandotnet test src/Netclaw.Cli.Tests— 1282 passeddotnet test src/Netclaw.Daemon.Tests— 858 passeddotnet test src/Netclaw.Configuration.Tests— 468 passeddotnet slopwatch analyze— 0 issues./scripts/Add-FileHeaders.ps1 -Verify— passsystemctl --user stop netclaw.servicereaching cleaninactive— intentionally not done in this session (do not restart/stop the live daemon); recommend validating on next daemon deploy/restart window.