feat(desktop): start and stop an owned local daemon #167
No Reviewers
Dismiss Review
Are you sure you want to dismiss this review?
Labels
Clear labels
accessibility
all-platforms
area:agents
area:ai
area:chain
area:ci
area:client
area:daemon
area:p2p
area:project
area:protocol
area:runtime
area:storage
bug
chore
community
dependencies
discussion
discussion/Announcements
discussion/General
discussion/Ideas
discussion/Q&A
discussion/Show and tell
discussion/answered
documentation
duplicate
enhancement
good first issue
help wanted
i18n
infra
invalid
needs:decision
needs:hardware
paired
question
research
security
status:in-review
test
testing
tracking
wontfix
Barrier affecting people with disabilities
Run every platform's packaging and checks on this pull request, not only the three main ones
MCP server, skill, and thinking on the pool
Planner and model providers
Blockchain, SISY, verification
Continuous integration and build workflows
Desktop and web clients
sisyphusd coordinator, worker, scheduler
libp2p hosts, relays, discovery, connections between nodes
Project planning, roadmap, and governance
Protobuf schemas and RPC contracts
Workload execution and environments
Content-addressed storage, IPFS, Kubo, Cluster, S3, names, pinning
Something isn't working
Repository health, templates, Discussions, contributor experience
Updates to actions, Go modules or npm packages
Migrated from GitHub Discussions
GitHub Discussions category: Announcements
GitHub Discussions category: General
GitHub Discussions category: Ideas
GitHub Discussions category: Q&A
GitHub Discussions category: Show and tell
Q&A discussion with an accepted answer
Improvements or additions to documentation
This issue or pull request already exists
New feature or request
Good for newcomers
Extra attention is needed
Translations and right-to-left layouts
Build, CI, releases
This doesn't seem right
Waits on a decision from both owners
Waits on a machine we do not have yet
Built together by both maintainers, with co-authored commits
Further information is requested
Needs investigation or a design decision
Sealing, keys, access control, tokens, the security model
Built and in an open pull request; closes when it merges
Tests, test harnesses and the rehearsal
Roadmap and meta issues
This will not be worked on
type
Feature
A request, idea, or new functionality (GitHub issue type)
type
Task
A specific piece of work (GitHub issue type)
Milestone
No items
No Milestone
V0: prove the loop
Projects
Clear projects
No projects
Sisyphus roadmap
Done
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Sisyphus/Sisyphus#167
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
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.
797f53bmerges current dev06b9e15(#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 head7975ffbpassed (38025498814); merged-head CI is pending. No changes to user daemon/data.Follow-up
d0e1e81merges devc2cc383(#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 preceding797f53bpassed all steps, while its Go job was still running at last inspection.Full CI for
d0e1e81now 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
876c5ccsupersedes 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.Read the main-process side while it is a draft. Starting only on
UNAVAILABLEat 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:--listen 127.0.0.1:0gives the node a different main port each start. The CLI andsisyphusd mcpboth default to127.0.0.1:7700for that address, so against a desktop-started nodesisyphusd nodes,job submit, and the MCP server's pin, name and record tools fail until given--addr, and nothing says which port to give. Trying127.0.0.1:7700first and falling back to:0when it is taken would keep the defaults working in the common case.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 itsipfschild running. The app does not pass--kubotoday, so this only matters if the flags grow; a longer grace before the kill would cover it.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.
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.
Merged, with #172 and #173, each by squash once green. Thanks for taking the port and shutdown points; preferring
127.0.0.1:7700keeps 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 todevwhen 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.