feat(windows): centralize signal handlers, HOME helper, logical-key path note

[stack72]

Summary

Stream C of the Windows-GA push (after #1278). Three independent work items:

Signal handlers — adds registerShutdownHandler in src/infrastructure/process/shutdown_handlers.ts. Always registers SIGINT; on POSIX also registers SIGTERM and SIGHUP unless the caller opts out. Returns a disposer. Three call sites converted:

• serve.ts and open.ts register one cross-platform handler instead of two POSIX-only listeners (which threw on Windows for SIGTERM)
• The datastore sync coordinator uses includePosixSignals: false to keep its SIGINT-only lock-release fast path

process_executor.ts was inspected and intentionally untouched — its process.kill("SIGTERM") calls target child processes, not signal listeners.

HOME / USERPROFILE — adds homeDirectory() to src/infrastructure/persistence/paths.ts: HOME first, USERPROFILE fallback, throws otherwise.

• source_paths.ts swaps its inline HOME-only helper for the new function (gains a USERPROFILE fallback on Windows)
• input_parser.ts swaps too but wraps the call in a try/catch so the ~/file literal-pass-through behavior pinned by Stream 0 stays intact when both env vars are unset

CLAUDE.md — appends a paragraph after the existing @std/path rule clarifying that the rule covers filesystem paths only; forward slashes in logical keys (model specs, datastore lock keys, URL pathnames, manifest entries) are intentional and must remain forward slashes on every OS. Stream 0's model_type_test.ts and extension_auto_resolver_test.ts enforce this in CI.

Out of scope follow-ups

• Six other inline HOME reads remain (resolve_extension_files.ts, serve/open/http.ts, embedded_deno_runtime.ts, local_encryption_vault_provider.ts, env_path.ts). Not migrated in this PR.
• getSwampDataDir / getSwampConfigDir still use inline HOME-only logic with their own pinned test. Not migrated to homeDirectory() because their error message format differs.

Test Plan

• deno fmt --check clean (1286 files)
• deno lint clean (1165 files)
• deno check clean
• deno run test — 5157 passed, 0 failed, 24 ignored
• Stream 0 regression net green: serve_shutdown_test.ts, datastore_sync_coordinator_test.ts SIGINT, input_parser_test.ts HOME-set/unset, model_type_test.ts and extension_auto_resolver_test.ts forward-slash preservation

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — this PR has no user-visible UX impact. All changes are internal infrastructure (signal handler centralization, cross-platform HOME helper). The only observable behavioral difference is that serve and open now also handle SIGHUP on POSIX, which is strictly an improvement (clean shutdown on terminal hangup). Error messages in input_parser.ts are unchanged — the ~/file literal-passthrough path still produces the same Input file not found UserError as before. No flags, help text, output format, or exit codes changed.

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — This PR is a pure infrastructure refactor (centralized signal handlers, cross-platform HOME helper) with no user-visible UX changes. JSON output shapes, log-mode output, help text, flag names, and error messages are all unchanged. The only observable behavioral delta is that serve and open now correctly handle SIGTERM on Windows (previously threw), and ~/file inputs now expand on Windows via USERPROFILE — both are improvements with no regressions.

[github-actions]

Code Review

Clean, well-scoped PR. The three work items (signal handler centralization, HOME/USERPROFILE helper, and the reverted CLAUDE.md/CI changes) are coherent and the final scope after the revert is appropriately narrow.

Blocking Issues

None.

Suggestions

1. Uncaptured dispose handles in open.ts and serve.ts: Both call sites discard the ShutdownHandlerHandle returned by registerShutdownHandler. This is fine today since the handlers live for the process lifetime and exit follows server.finished, but worth noting for future readers — if either command ever gains a graceful restart path, the handle will need to be captured and disposed.

2. getSwampDataDir / getSwampConfigDir consolidation: The PR body correctly identifies these as out-of-scope, but the inline HOME/USERPROFILE logic in getSwampDataDir is now nearly identical to homeDirectory(). A follow-up to DRY these up (adjusting error messages as needed) would be a nice cleanup.

DDD Assessment

• registerShutdownHandler is correctly placed in the infrastructure layer (src/infrastructure/process/) — OS signal handling is a pure infrastructure concern.
• homeDirectory() is correctly placed in the persistence paths module — environment variable resolution for filesystem paths is infrastructure.
• No domain layer leakage: domain types remain untouched, and the new infrastructure helpers don't introduce domain concepts.
• The datastore_sync_coordinator.ts refactor preserves the existing aggregate-level lock management pattern cleanly.

Other Observations

• AGPLv3 headers present on both new files.
• Tests cover all three branches of homeDirectory() (HOME set, USERPROFILE fallback, both unset) and exercise registerShutdownHandler registration, SIGINT delivery via child process, dispose idempotency, and SIGINT-only mode.
• No libswamp import boundary violations — all CLI imports from infrastructure use direct infrastructure paths (not libswamp internals).
• CI passes on both Ubuntu and Windows.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. Leaked signal listener handles in open.ts:296 and serve.ts:322 — Both call sites discard the ShutdownHandlerHandle returned by registerShutdownHandler. The signal listeners (SIGINT + SIGTERM + SIGHUP on POSIX) remain registered for the entire process lifetime. For these long-running server commands this is benign — the process exits after await server.finished. However, if the server finishes naturally (e.g., graceful close from API) rather than from a signal, the listeners keep the handler closures alive (holding references to logger, ac, state, webhookService, scheduledExecution, etc.) until process exit. This is a resource hygiene issue, not a correctness bug. Suggested fix if desired: capture the handle and call dispose() after await server.finished.

Low

1. Test timeout promise in shutdown_handlers_test.ts:84-92 — The Promise.race between child.status and a 5s timeout leaves the setTimeout timer dangling when the child exits promptly. The sanitizeOps: false suppresses the Deno test sanitizer for this. While Promise.race internally attaches rejection handlers (so no unhandled rejection occurs), the timer occupies an event loop slot for 5s after the test resolves. Could be tightened with a clearTimeout after the race settles, but the impact is negligible in test context.

2. Env-var mutation in paths_test.ts is process-global — The homeDirectory tests delete/set HOME and USERPROFILE in try/finally blocks. If Deno runs tests from other files concurrently in the same process, those tests could see unexpected env values mid-flight. This is a pre-existing pattern in the file (the getSwampDataDir tests above do the same), not introduced by this PR.

Verdict

PASS — Clean, well-scoped abstraction. The registerShutdownHandler helper correctly branches on Deno.build.os, the disposer pattern is implemented correctly with idempotency guard, homeDirectory() is a straightforward HOME→USERPROFILE fallback with proper error throwing, and all three call-site conversions preserve existing behavior (including shuttingDown reentrancy guards in both servers, and includePosixSignals: false for the datastore coordinator's SIGINT-only fast path). Test coverage is thorough — the child-process signal test is the right approach for verifying signal delivery without disturbing the parent test runner.