feat(desktop): start and stop an owned local daemon #167

Merged
admin merged 5 commits from feat/desktop-daemon-lifecycle into dev 2026-10-10 05:53:08 +00:00
Owner

Refs #16. Separate from attachment retention #165.

When GetNodeInfo reports UNAVAILABLE at the default local endpoint, the desktop checks whether the port is unused before launching the matching sisyphusd. Occupied ports, custom API endpoints/token files and opt-out configuration never launch a process. Development uses repository bin/sisyphusd (SISYPHUS_DAEMON_PATH override); packaging will need resources/bin/sisyphusd[.exe]. Spawn uses separate arguments, no shell, the default daemon data directory, loopback-only ephemeral pool listener and discovery off. Existing nodes are never stopped. An owned child receives termination on application quit with a bounded fallback, startup/exit failures are reported without respawn storms, and quitting during probing cannot leave a late child. Single-instance app handling avoids two desktop owners. Node-info calls have a five-second deadline.

Validation: 62 desktop tests, typecheck, production build, make build and diff check pass locally. Real isolated daemon smoke test passes: starts actual Go node, reads GetNodeInfo/token existence, second manager attaches without owning/stopping it, owned shutdown frees API port. Smoke test is added to desktop CI using a matching Go build. User's tmux node/data were not changed; temporary test data was removed. Existing bundle/module-directive warnings remain.

Not a claim that #16 is complete: packaged Electron acceptance and Linux/Windows/macOS artifacts/signing are still required. SIGTERM behavior on Windows is termination, not a promise of POSIX graceful shutdown. No issue closed.

797f53b merges current dev 06b9e15 (#168 security hardening) without conflict resolution. Matching Go build, all 62 desktop tests, diff check and real isolated daemon startup/reuse/owned shutdown pass after the merge. Full CI for previous head 7975ffb passed (38025498814); merged-head CI is pending. No changes to user daemon/data.

Follow-up d0e1e81 merges dev c2cc383 (#170 MCP/planner/address-book additions). Merge is clean; matching daemon build and real startup/reuse/owned shutdown smoke pass. New full CI remains pending; desktop CI for preceding 797f53b passed all steps, while its Go job was still running at last inspection.

Full CI for d0e1e81 now passes (38026914646): full Go race suite/statement coverage and desktop build/tests/real daemon lifecycle smoke. Current dev advanced to #171; integration with that change still needs checking.

Austin review follow-up 876c5cc supersedes earlier launch/shutdown details: prefer loopback pool port 7700 when unused (and separate from API), otherwise use an ephemeral listener without taking over the occupied service. Documentation describes obtaining fallback --addr from Node overview and safe bind-race failure. Shutdown now permits 30 seconds before forced termination, with Windows immediate termination and future sidecar limits explicit. Typecheck, all 66 desktop tests, diff check and real daemon startup/reuse/owned-shutdown smoke pass. New full CI is pending; no user runtime restarted.

Refs #16. Separate from attachment retention #165. When GetNodeInfo reports UNAVAILABLE at the default local endpoint, the desktop checks whether the port is unused before launching the matching sisyphusd. Occupied ports, custom API endpoints/token files and opt-out configuration never launch a process. Development uses repository bin/sisyphusd (SISYPHUS_DAEMON_PATH override); packaging will need resources/bin/sisyphusd[.exe]. Spawn uses separate arguments, no shell, the default daemon data directory, loopback-only ephemeral pool listener and discovery off. Existing nodes are never stopped. An owned child receives termination on application quit with a bounded fallback, startup/exit failures are reported without respawn storms, and quitting during probing cannot leave a late child. Single-instance app handling avoids two desktop owners. Node-info calls have a five-second deadline. Validation: 62 desktop tests, typecheck, production build, make build and diff check pass locally. Real isolated daemon smoke test passes: starts actual Go node, reads GetNodeInfo/token existence, second manager attaches without owning/stopping it, owned shutdown frees API port. Smoke test is added to desktop CI using a matching Go build. User's tmux node/data were not changed; temporary test data was removed. Existing bundle/module-directive warnings remain. Not a claim that #16 is complete: packaged Electron acceptance and Linux/Windows/macOS artifacts/signing are still required. SIGTERM behavior on Windows is termination, not a promise of POSIX graceful shutdown. No issue closed. 797f53b merges current dev 06b9e15 (#168 security hardening) without conflict resolution. Matching Go build, all 62 desktop tests, diff check and real isolated daemon startup/reuse/owned shutdown pass after the merge. Full CI for previous head 7975ffb passed (38025498814); merged-head CI is pending. No changes to user daemon/data. Follow-up d0e1e81 merges dev c2cc383 (#170 MCP/planner/address-book additions). Merge is clean; matching daemon build and real startup/reuse/owned shutdown smoke pass. New full CI remains pending; desktop CI for preceding 797f53b passed all steps, while its Go job was still running at last inspection. Full CI for d0e1e81 now passes (38026914646): full Go race suite/statement coverage and desktop build/tests/real daemon lifecycle smoke. Current dev advanced to #171; integration with that change still needs checking. Austin review follow-up 876c5cc supersedes earlier launch/shutdown details: prefer loopback pool port 7700 when unused (and separate from API), otherwise use an ephemeral listener without taking over the occupied service. Documentation describes obtaining fallback --addr from Node overview and safe bind-race failure. Shutdown now permits 30 seconds before forced termination, with Windows immediate termination and future sidecar limits explicit. Typecheck, all 66 desktop tests, diff check and real daemon startup/reuse/owned-shutdown smoke pass. New full CI is pending; no user runtime restarted.
rvnminers-A-and-N left a comment
Owner

Read the main-process side while it is a draft. Starting only on UNAVAILABLE at the default endpoint with the port unused, never touching a node the app did not start, no shell, and the single-instance lock all look right to me. Three things from the daemon's side:

  1. --listen 127.0.0.1:0 gives the node a different main port each start. The CLI and sisyphusd mcp both default to 127.0.0.1:7700 for that address, so against a desktop-started node sisyphusd nodes, job submit, and the MCP server's pin, name and record tools fail until given --addr, and nothing says which port to give. Trying 127.0.0.1:7700 first and falling back to :0 when it is taken would keep the defaults working in the common case.
  2. Three seconds, then SIGKILL. A node that is combining a job's results or stopping a Kubo sidecar (--kubo) can take longer. The journal survives a kill, but a killed node leaves its ipfs child running. The app does not pass --kubo today, so this only matters if the flags grow; a longer grace before the kill would cover it.
  3. Windows. child.kill('SIGTERM') there ends the process at once, with no shutdown at all. You may already have this in the part of the description I cannot see past the cut. The daemon is safe to kill, so it is a note for the README more than a bug.

One thing changed under you on dev, with no effect on this PR: since #166 reading jobs needs the token, which the app already sends on every call.

Mark it ready when you are and I will run it and merge.

Read the main-process side while it is a draft. Starting only on `UNAVAILABLE` at the default endpoint with the port unused, never touching a node the app did not start, no shell, and the single-instance lock all look right to me. Three things from the daemon's side: 1. **`--listen 127.0.0.1:0` gives the node a different main port each start.** The CLI and `sisyphusd mcp` both default to `127.0.0.1:7700` for that address, so against a desktop-started node `sisyphusd nodes`, `job submit`, and the MCP server's pin, name and record tools fail until given `--addr`, and nothing says which port to give. Trying `127.0.0.1:7700` first and falling back to `:0` when it is taken would keep the defaults working in the common case. 2. **Three seconds, then `SIGKILL`.** A node that is combining a job's results or stopping a Kubo sidecar (`--kubo`) can take longer. The journal survives a kill, but a killed node leaves its `ipfs` child running. The app does not pass `--kubo` today, so this only matters if the flags grow; a longer grace before the kill would cover it. 3. **Windows.** `child.kill('SIGTERM')` there ends the process at once, with no shutdown at all. You may already have this in the part of the description I cannot see past the cut. The daemon is safe to kill, so it is a note for the README more than a bug. One thing changed under you on `dev`, with no effect on this PR: since #166 reading jobs needs the token, which the app already sends on every call. Mark it ready when you are and I will run it and merge.
Author
Owner

Thanks for the draft review. Addressed the port and shutdown points: prefer 127.0.0.1:7700 when it is unused and not the selected API port; otherwise use :0 without touching the existing service. README directs fallback users to Node overview's listen address for CLI/MCP --addr, and explicitly documents the advisory probe/bind race (safe failure, no takeover). Shutdown grace is now 30 seconds before SIGKILL; Windows immediate termination is documented, and future Kubo-sidecar support is explicitly not claimed.

Tests cover preferred-port startup, occupied-port fallback using an isolated real listener, API/pool port collision avoidance, quitting during the second probe, delayed graceful close and bounded forced shutdown. Typecheck, all 66 desktop tests and real daemon startup/reuse/owned-shutdown smoke pass. Existing user's node and test window were not restarted. New head still needs full CI before ready.

Thanks for the draft review. Addressed the port and shutdown points: prefer 127.0.0.1:7700 when it is unused and not the selected API port; otherwise use :0 without touching the existing service. README directs fallback users to Node overview's listen address for CLI/MCP --addr, and explicitly documents the advisory probe/bind race (safe failure, no takeover). Shutdown grace is now 30 seconds before SIGKILL; Windows immediate termination is documented, and future Kubo-sidecar support is explicitly not claimed. Tests cover preferred-port startup, occupied-port fallback using an isolated real listener, API/pool port collision avoidance, quitting during the second probe, delayed graceful close and bounded forced shutdown. Typecheck, all 66 desktop tests and real daemon startup/reuse/owned-shutdown smoke pass. Existing user's node and test window were not restarted. New head still needs full CI before ready.
Owner

Merged, with #172 and #173, each by squash once green. Thanks for taking the port and shutdown points; preferring 127.0.0.1:7700 keeps the CLI and MCP defaults working. I left your branches in place rather than delete them, since #169 is based on this one: retarget it to dev when it suits you and I will keep off it until you mark it ready. #165 is waiting only on the two uncovered statements I described there.

Merged, with #172 and #173, each by squash once green. Thanks for taking the port and shutdown points; preferring `127.0.0.1:7700` keeps the CLI and MCP defaults working. I left your branches in place rather than delete them, since #169 is based on this one: retarget it to `dev` when it suits you and I will keep off it until you mark it ready. #165 is waiting only on the two uncovered statements I described there.
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Sisyphus/Sisyphus#167