Add served mode (streamable HTTP), the com.setlist.mcp LaunchAgent, and a CLI on PATH - #85
Merged
Conversation
…nd a CLI on PATH setlist was designed single-machine: one MCP host spawns setlist-mcp over stdio and talks to a co-located SQLite file. The ecosystem stopped being one machine (the Mini is now the execution home while the laptop still runs sessions), and the tempting alternative — both machines opening one registry.db over a network share — is the SQLite corruption case, not a deployment option. Served mode shares the tool surface instead of the file: - packages/mcp/src/http-server.ts: the same 63-tool Server over the MCP streamable-HTTP transport. One Server + transport per session in a map that is capped (503 past maxSessions) and idle-reaped at 30 min, because an MCP client that sleeps without sending DELETE otherwise leaks a session per reconnect for the life of an always-on process. Capability self-registration runs once per service start, not once per session. SIGTERM/SIGINT drain open sessions. - packages/mcp/src/serve-config.ts: the policy, separated from the plumbing so it is testable without a socket. A bearer token is MANDATORY for any non-loopback bind — resolveServeConfig throws before binding rather than serving open, because bootstrap_project runs shell primitives and doctor --apply rewrites rows. Host headers are validated against the addresses the bound port can legitimately be reached at; cross-origin requests are rejected. No TLS: the token protects use, not the wire (deploy over Tailscale). - stdio stays the default and its startup path is unchanged, so the four-ABI tenant launcher behaves exactly as before. Service + CLI plumbing: - packages/cli/src/serve-service.ts + `setlist service <plan|install|uninstall| status|config>`: the com.setlist.mcp LaunchAgent (KeepAlive), modeled on context-export-schedule.ts — plist backup, bootout/bootstrap, restore-and- reload on a failed bootstrap, orphan probing so a hand-deleted plist whose job still serves never reports not_installed. A non-loopback bind generates a 0600 bearer token rather than installing a service the server will refuse to start. - scripts/install-cli.sh puts a `setlist` shim on PATH (fctry #447: close-time lesson routing failed on this machine for want of one). Marker-gated: it never clobbers or removes a `setlist` it does not own, and it smoke-tests the shim. - scripts/setlist-runtime.sh factors the two reconciliations every entry point needs — refuse a stale dist, repair a stranded better-sqlite3 ABI — out of launch-mcp.sh and into one place shared with launch-mcp-http.sh and setlist-cli.sh. dist-freshness.sh grows dist_is_stale_pkgs (core+cli as well as core+mcp) with dist_is_stale reimplemented on top, contract unchanged. Docs: - docs/served-mode.md — the architecture study: how the DB is shared today, what served mode adds, and what breaks in remote mode ranked by blast radius (doctor --apply first, bootstrap_project second, then every path-reading tool, mcp_clients, memory-source scanning, context export, ports' missing machine dimension, and the desktop app's lack of a remote mode). - docs/mini-cutover-runbook.md — the operator procedure, unexecuted: freeze laptop writes, VACUUM INTO snapshot, verified transfer, service start, laptop repoint, neutralize the laptop copy, stale-path cleanup, rollback. No live database was touched and no laptop state was changed; the cutover is a separate operator-approved step. Agent model reconciled: D-017, a served-mode Runtime invariant, two SURFACES edges, CLAUDE.md. 1149 tests green (+44 new), typecheck clean, verify:mcp-abi OK. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AzDu4UiRWMDEoBSAY5hf7P
Owner
Author
🤖 Codex review — REQUEST_CHANGESLocal Codex review of this PR's changes against The served-mode implementation has blocking lifecycle/operator-flow bugs: LAN installs can report success while crash-looping, shutdown can hang with active HTTP sessions, and the printed remote config command can emit a config that cannot authenticate to the installed service. Findings (3)
|
…ig from the installed plist Codex review round 1 on #85 returned REQUEST_CHANGES with three P2 lifecycle / operator-flow bugs. All three were real: 1. serve-service.ts — a non-loopback install accepted an existing token file on `existsSync` alone. A blank file therefore skipped generation, wrote and loaded the plist, and reported success, while the server exited on start (resolveServeConfig rejects empty token files) — leaving a KeepAlive job crash-looping behind an "installed" message. Token validation now happens before any launchctl call: `token_file_present` means *usable* (non-blank), a blank file is regenerated when the bind requires a token, and a blank file on a loopback bind is a synchronous install failure rather than silently minting a token (which would turn an unauthenticated loopback service into an authenticated one behind the operator's back). An unreadable token file throws with its path instead of reading as "no token". 2. http-server.ts — `close()` awaited `http.close()` before tearing sessions down. `http.close()` waits for in-flight connections and a streamable-HTTP session holds a long-lived GET/SSE request, so SIGTERM could hang forever and never reach the teardown that would end those streams. Now: register the close, drop the sessions, force-drop whatever is still attached (closeIdleConnections + closeAllConnections — after teardown a live socket is an unresponsive client, not work in progress), then await. close() is also idempotent now. 3. index.ts / serve-service.ts — `setlist service config` built its snippet from the current flags and defaults, so the very command `install --host 0.0.0.0` prints as the next step emitted a loopback URL with no Authorization header: a config that cannot reach the running service. New `readInstalledServeConfig` reads host/port/token-file back out of the installed plist and the snippet is built from that; explicit flags still override for pre-install use. A plist that exists but carries no recognizable bind config throws telling the operator to pass flags — never a silent fall back to defaults, which is the failure being fixed. 9 new tests (1158 total, all green); typecheck clean. The shutdown test asserts close() terminates under 3s with three sessions holding SSE streams open — the pre-fix code hung to the test timeout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AzDu4UiRWMDEoBSAY5hf7P
Owner
Author
🤖 Codex review — REQUEST_CHANGESLocal Codex review of this PR's changes against Found a blocking security issue in the served-mode installer: it can leave the bearer token file readable by broader local users, which weakens the only auth barrier for a LAN-visible MCP server. Findings (1)
|
…xisting one Codex review round 2 on #85: one P2, correct. `writeFileSync(path, token, { mode: 0o600 })` applies the mode only when it CREATES the file, so overwriting a pre-existing blank token file preserved that file's old permissions — leaving the bearer token potentially group/world-readable. That token is the only barrier in front of a tool surface that runs filesystem and shell primitives with this user's authority. - After generating a token, `chmodSync(0o600)` unconditionally. - Before trusting an *existing* token, check its mode; if any group/other bit is set, tighten to 0600 and report it via the new `token_permissions_repaired` result field. The token value is preserved — repairing the mode must not silently invalidate client configs already carrying it — and the CLI prints an explicit warning that anyone who read it already holds a working token, with the rotation step. Also fixed a smaller honesty bug in the same output while here: `service plan/status` reported "Auth: none (loopback bind)" whenever the bind was loopback, but the server reads the token file whenever one exists — so a loopback bind with a leftover token does require the header. The line now distinguishes all three states. 3 new tests (1161 total, green); typecheck clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AzDu4UiRWMDEoBSAY5hf7P
Owner
Author
🤖 Codex review — REQUEST_CHANGESLocal Codex review of this PR's changes against The served-mode LaunchAgent path has a blocking startup regression for the default loopback install. I also found one lower-severity config validation gap in the direct MCP HTTP entrypoint. Findings (2)
|
…d parse ports strictly Codex review round 3 on #85: one P2 and one P3, both correct. P2 (serve-service.ts) — the generated plist always carried SETLIST_MCP_TOKEN_FILE, but `resolveServeConfig` treats an env-provided token file as an EXPLICIT request and throws `Token file not found` when it is absent. The default loopback install deliberately creates no token, so `setlist service install --yes` reported success and launchd started a crash-looping job. Same failure shape as round 2's finding, one layer over. The key is now written only when there is a usable token at that path or the operator named it explicitly. With it omitted the server still falls back to the same default path, so a token dropped there later is picked up on restart. And a loopback install that names a nonexistent --token-file is now refused synchronously rather than silently dropping the chosen path or installing a job that cannot start. P3 (serve-config.ts) — port parsing used `parseInt`, so `--port 8788abc` or SETLIST_MCP_PORT=8788abc silently bound 8788. `setlist-mcp --http` is a supported entrypoint in its own right, so the strict check belongs here and not only in the CLI wrapper: the whole string must be digits, with surrounding whitespace tolerated. 4 new tests (1165 total, green); typecheck clean. Env-table note added to docs/served-mode.md explaining that naming a token file makes it required. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AzDu4UiRWMDEoBSAY5hf7P
Owner
Author
🤖 Codex review — REQUEST_CHANGESLocal Codex review of this PR's changes against Found one blocking producer/consumer contract regression in the new service config path. Targeted tests could not be run because Findings (1)
|
…ed service back Codex review round 4 on #85: one P2, correct — and self-inflicted by round 3. `readInstalledServeConfig` required SETLIST_MCP_TOKEN_FILE, which round 3 taught the writer to deliberately omit for an unauthenticated loopback install. So `setlist service install --yes` followed by `setlist service config` failed on the very plist the installer had just written ("does not carry the expected setlist bind configuration"). The key is optional by contract: its absence means "the default path, no token expected", which is what the server's resolver wants. The reader now requires only SETLIST_MCP_HOST / SETLIST_MCP_PORT — the two the writer always emits, and the two whose silent defaulting was the original bug — and falls back to the default token path otherwise. Port reading is strict here too, matching resolveServeConfig. The new test walks the operator's actual two-command flow (install → read back) across four bind shapes rather than asserting on one plist in isolation; a single-artifact test is what let a producer/consumer split through. 2 new tests (1167 total, green); typecheck clean. Smoke-checked the read-only CLI paths (plan / status / config) against the real environment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AzDu4UiRWMDEoBSAY5hf7P
Owner
Author
🤖 Codex review — REQUEST_CHANGESLocal Codex review of this PR's changes against Found one blocking correctness issue in the served-mode client config generation. Findings (1)
|
… address Codex review round 5 on #85: one P2, correct. `serveClientConfigSnippet` emitted an Authorization header only when the bind was non-loopback, but the server enables bearer auth whenever the resolved token file holds a usable token. A loopback service with a leftover serve-token therefore got a snippet with no header — a config that cannot connect. This is the same honesty rule round 4 applied to the `plan`/`status` auth line ("bearer token in use even on loopback"), which I fixed there and failed to mirror here. Both now read from the same condition: what the SERVER will demand, not what the bind address suggests. A blank token file still counts as no token, consistent with `token_file_present` everywhere else. 2 new tests (1169 total, green); typecheck clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AzDu4UiRWMDEoBSAY5hf7P
Owner
Author
🤖 Codex review — APPROVELocal Codex review of this PR's changes against No blocking correctness, security, IPC-contract, or better-sqlite3 ABI regressions found in the served-mode HTTP transport, service CLI, or launcher/runtime changes. Verification was limited because this checkout lacks local dev tooling ( No issues found. |
This was referenced Jul 29, 2026
spaceshipmike
added a commit
that referenced
this pull request
Jul 29, 2026
spaceshipmike
added a commit
that referenced
this pull request
Jul 30, 2026
* Refuse to open a registry this machine does not own (#87) The desktop app is the one surface with no protocol client: served mode (#85, D-017) moved the database boundary for agent sessions, but the Electron main process still builds `new Registry(...)` over whatever file sits at the canonical path. On a machine that is not the registry owner that is a *different* database, and it does not error — it silently shows and mutates stale data, and when the file is absent `initDb` creates an empty registry that presents as total data loss. This is option (b) of #87 — the refusal, not the remote mode. `packages/app/src/main/db-owner.ts` decides, from evidence on disk, whether this machine may open the local registry. It runs in `app.whenReady()` before `registerIpcHandlers` (which constructs the `Registry`), before the tray and before the shortcuts, so a refusal has no IPC surface at all and nothing reads or writes the database file. The refusal is a window, not a log line: scriptless static HTML, no preload, sandboxed, naming the owner, the served URL, the file it did not open, and what to do about it. Closing it quits, whatever menu-bar persistence says. Two signals, in precedence order: - `<data-dir>/registry-remote.json` (or `SETLIST_REGISTRY_REMOTE_URL`), an operator's declaration that this machine's registry is served elsewhere. This is the only signal that can catch an *absent* local file, which is the laptop's state after the cutover runbook. - `<data-dir>/db-owner.json` naming a different machine — a copied or cloned data directory, caught with no operator action. The app writes this marker for itself on first launch. The asymmetries are the design: - Refusal requires positive evidence. A missing marker, an unparseable marker, and an undeterminable machine identity all allow: a guard whose false positive is "the app will not start" must not fire on ambiguity, because the failure it prevents is silent divergence, not startup. - A blank `SETLIST_REGISTRY_REMOTE_URL` means unset (env vars get exported empty by accident), but a `registry-remote.json` that exists without a usable URL is an error rather than a fallback to the local file — that file exists only because somebody wrote it. Same rule as served mode's empty token file. - Identity is hardware-derived (macOS IOPlatformUUID, Linux machine-id), never the hostname: hostnames churn, and a hostname-only mismatch would refuse to start on the owner machine. - `SETLIST_APP_DB_OWNER_OVERRIDE=1` is the documented last resort. Tests: 28 unit cases over the predicate (both refusal signals, every allow-on-ambiguity path, the pointer shapes an operator would write, the blank-vs-invalid asymmetry, the override, HTML escaping, and a temp-dir claim/refuse walk asserting no database file appears), plus an Electron e2e launch against a data dir claimed by another machine asserting the refusal window, the absence of registry.db/-wal/-shm, and that no `window.setlist` bridge exists. Docs: served-mode §3.8.1 (the guard, the files, the env vars, the honest limits), runbook Phase 7 now writes the pointer as its first step, a new State-ownership invariant (ENFORCED), and CLAUDE.md. Still open in #87: the app resolving its ~60 IPC channels through an MCP client against the served registry. The CLI and MCP server on a non-owner machine remain unguarded, and a machine with neither file gets no protection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Twbdbg1U4BJo5tz7vFegsx * chore(app): rebuild the committed Electron bundle Mechanical: `npm run build -w packages/app` output for the owner guard. The tracked bundle also picks up drift from earlier merges that did not rebuild it (#63 — nothing asserts `out/` matches source). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Twbdbg1U4BJo5tz7vFegsx * Isolate the owner guard from the database stack (#88 review) Codex flagged that `index.ts` reached `resolveAppDbPath` through `./ipc.js`, which imports `@setlist/core` (and so `better-sqlite3`) at module load — before the guard could run. The stated crash does not reproduce (better-sqlite3 loads its addon lazily inside the `Database` constructor, `lib/database.js:48`, so refusal mode survived even with the native binary removed entirely — verified), but relying on a dependency's internal laziness to keep the "do not open this database" window alive is the wrong guarantee. Make it structural: - `db-path.ts` resolves the path with no `@setlist/core` / `better-sqlite3` import. It re-derives core's canonical location as a *checked* duplicate: `db-path.test.ts` asserts it equals `getDbPath()`, so the mirror cannot drift silently. - `./ipc.js` is now loaded with `await import()` only after ownership is allowed, and lands in its own bundle chunk (`out/main/ipc-*.js`). Verified: the refusal e2e passes with the native binding absent; the allow path still claims the data dir and loads the renderer normally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Twbdbg1U4BJo5tz7vFegsx --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Why
setlist was designed single-machine: one MCP host spawns
setlist-mcpover stdio and talks to a co-located SQLite file. The ecosystem stopped being one machine — the Mini is now the execution home while the laptop still runs sessions. The tempting alternative (both machines opening oneregistry.dbover a share) is the SQLite corruption case, not a deployment option: locking is unreliable over SMB/NFS and unsafe under file-sync layers.So sharing happens one layer up: one machine owns the file, other machines are protocol clients.
This PR is code + runbook only. No live database was touched and no laptop state was changed — the cutover is a separate operator-approved step.
What's here
1. Architecture study —
docs/served-mode.mdHow the DB is shared today (core is the sole
new Database(...)site;Registryholds only a path and opens a short-lived connection per operation, which is why multi-process access already works), and what breaks when the caller is not on the server machine — ranked by blast radius:doctor --applybootstrap_projectgit init, mailboxes, shell primitives all land on the serverassess_health/assess_attentiongit signals)mcp_clientsinstall/remove~/.claude/projects(the fctry #447 shape)Also stated: what is fine, so nobody re-derives it —
switch_projectkeeps no server state, concurrent writes are ordinary multi-process WAL, and backups now happen in exactly one place.2. Served mode —
packages/mcp/src/{http-server,serve-config}.tsThe same 63-tool
Serverover MCP streamable HTTP.POST/GET/DELETE /mcpplus an unauthenticatedGET /healthzthat carries no registry data.resolveServeConfigthrows before any socket binds.bootstrap_projectrunsshell-commandprimitives anddoctor --applyrewrites rows; an open LAN listener is RCE with the operator's authority. An empty token file is an error, not "no auth requested".Hostvalidated against the addresses the bound port can be reached at (port 0 taught me that one); cross-originOriginrejected.Server+ transport per session, capped (503) and idle-reaped (30 min) — an MCP client that sleeps withoutDELETEotherwise leaks a session per reconnect forever. The in-flight-init counter keeps a burst from overshooting the cap.SIGTERM/SIGINTdrain sessions. No TLS — the token protects use, not the wire; deploy over Tailscale.server.connect(transport)without--http, so the four-ABI-tenant launcher path is unchanged.3. Service + PATH —
setlist service,scripts/packages/cli/src/serve-service.ts+setlist service <plan|install|uninstall|status|config>: thecom.setlist.mcpLaunchAgent (KeepAlive), modeled oncontext-export-schedule.ts— plist backup, bootout/bootstrap, restore-and-reload on a failed bootstrap, orphan probing so a hand-deleted plist whose job still serves never reportsnot_installed. A non-loopback bind generates a 0600 token rather than installing a service the server will refuse to start.service configprints the exact client entry for the remote machine.scripts/install-cli.shputs asetlistshim on PATH — fctry #447. Marker-gated: never clobbers or removes asetlistit does not own; smoke-tests the shim before reporting success; warns that launchd/GUI callers need the absolute path.scripts/setlist-runtime.shfactors the two reconciliations every entry point needs (refuse a stale dist, repair a stranded better-sqlite3 ABI) out oflaunch-mcp.shinto one place shared withlaunch-mcp-http.shandsetlist-cli.sh.dist-freshness.shgrowsdist_is_stale_pkgs(core+cli as well as core+mcp) withdist_is_stalereimplemented on top — same contract, same tests.4. Cutover runbook —
docs/mini-cutover-runbook.mdNine phases with verification at each: Mini prep → freeze laptop writes (incl.
lsofproof) →VACUUM INTOsnapshot with recorded row counts → placement with count/integrity verification → loopback-then-LAN service start → laptop repoint → neutralize the laptop copy (CLI off PATH, app out of Login Items,chmod 444so an accidental write fails loudly instead of forking reality) → stale-path cleanup (project-registry-service→setlist,mcpoyle→ensemble, distinguishing genuine stale paths from off-server false positives) → re-establish scheduled work. Plus a verification checklist, rollback, and the limitations being accepted.Leads with the trap worth internalizing: any surface that opens the default path creates an empty registry if none is there — which has already happened on the Mini, so step 4 moves that copy aside.
Testing
packages/mcp/tests/http-server.test.ts— a real MCP client over HTTP against the real 63-tool server: cross-session write/read round-trip, session tracking + DELETE release, idle reaping of a vanished client,/healthz, and the abuse paths (401 unauth, 401 wrong token, 421 forged Host via rawnode:httpsince undici won't set it, 403 cross-origin, 404 unknown session, 400 non-initialize without a session, 413 oversized body, 503 past the cap).packages/mcp/tests/serve-config.test.ts— the refusal to serve LAN without a token, token precedence, empty/missing token files, port validation, Host derivation.packages/cli/tests/serve-service.test.ts— plan/install idempotency, token generation + reuse, XML escaping, and every launchd failure mode (rollback, honestnot_installed, orphan bootout,installed_not_loaded).scripts/install-cli.test.js— installs, runs the CLI through the shim, idempotency, refusal to clobber/remove a foreignsetlist, arg validation, PATH warning.1149 tests green (+44 new),
npm run typecheckclean,npm run verify:mcp-abiOK. Verified by hand against a scratch DB:/healthzresponds and SIGTERM drains.Agent model
D-017 (five decisions incl. the deferred alternative: a machine dimension in the schema), a served-mode Runtime invariant (ENFORCED for auth/Host/Origin/cap, PROSE for "never a shared file"), two new SURFACES edges (the
scripts/entry points; served mode ↔ the filesystem boundary), and CLAUDE.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01AzDu4UiRWMDEoBSAY5hf7P