[zm2231]

Summary

Three daemon bugs in 0.17.0, all reproduced with the real CLI on published mcporter@0.17.0 and on main (21624e8):

1. A daemon stop or restart refused for active calls leaves the daemon permanently draining. It stays alive and rejects every call from every client.
2. An MCP server with a setuid-root child fails to connect. On macOS this hits any server that shells out to ps, including xcodebuildmcp.
3. A stop refused because a transport's retirement is already unverified causes the same permanent draining as (1), even with zero active calls.

(2) can set up (3): a server hitting (2) is left with unverified retirement, so the next daemon stop wedges the whole daemon, and SIGTERM doesn't get it out either.

What's included

• src/daemon/broker.ts: close() makes both refusals before setting draining.
• src/daemon/process-retirement.ts: ownedProcessTree leaves out descendants that have a verified foreign owner and start identity, and keeps walking through them so owned processes beneath are still tracked. A row with no owner or no start identity is still refused, including a foreign one.

Evidence

Environment: macOS 27.0.1 arm64, Node 25.9.0. Each scenario used a fresh daemon in its own MCPORTER_DAEMON_DIR and a config with no OAuth servers. Output is trimmed to the relevant lines; stack traces are removed. release and main behaved the same in every scenario, so only one is shown where noted. The fixture and config are at the bottom.

1. Stop refused for an active call

plain and other are fixture stdio servers. plain.slow is a 15s tool, started in the background.

Before (main; release is the same):

$ mcporter daemon status --json  -> activeCalls = 1
$ mcporter daemon stop
Error: Active calls are still running; wait for completion before stopping.   [exit 1]
$ mcporter call plain.ping
Error: Daemon is draining.  code: 'daemon_draining'                           [exit 1]
$ mcporter list other --json
status=error  rawMessage: 'Daemon is draining.'
# in-flight call has finished
$ mcporter call plain.ping
Error: Daemon is draining.  code: 'daemon_draining'                           [exit 1]

After:

$ mcporter daemon stop
Error: Active calls are still running; wait for completion before stopping.   [exit 1]
$ mcporter call plain.ping
pong                                                                          [exit 0]
$ mcporter list other --json
status=ok tools=2
# in-flight call has finished
$ mcporter call plain.ping
pong                                                                          [exit 0]

2. Server with a setuid-root child

Real server, xcodebuildmcp@2.7.0. Sampling ps while it started found the uid 0 child on every build:

uid 0 parent=xcodebuildmcp (ps)

Before (main; release is the same):

$ mcporter list xcode --json
status=error  'Transport ownership could not be verified; inspect it before recovery.'
$ mcporter list xcode --json
status=error  'Old transport retirement is unverified; no replacement was launched.'
$ mcporter list xcode --json
status=error  'Old transport retirement is unverified; no replacement was launched.'

After:

$ mcporter list xcode --json     (three runs)
status=ok tools=24
status=ok tools=24
status=ok tools=24

Deterministic fixture: a server that spawns /usr/bin/top (-r-sr-xr-x root), so the foreign child is alive at capture.

Before:

$ mcporter list setuid --json
status=error  'Transport ownership could not be verified; inspect it before recovery.'
  99087 99080 501 node fixture.mjs setuid-child
  99088 99087   0 /usr/bin/top -l 0 -s 3131
$ mcporter daemon stop
Error: Retirement is unverified; inspect the previous transport before stopping this owner.   [exit 1]

After:

$ mcporter list setuid --json
status=ok tools=2
   2385  2372 501 node fixture.mjs setuid-child
   2391  2385   0 /usr/bin/top -l 0 -s 3131
$ mcporter call setuid.ping
pong                                [exit 0]
$ mcporter daemon stop
Daemon stopped (if it was running). [exit 0]
# no fixture or top process left afterwards

Owned process beneath a foreign parent: a real cross-user tree, using crontab -e (setuid root) with EDITOR set to a script that execs sleep. crontab -l was compared before and after: unchanged.

$ mcporter list grandchild --json     (after; before fails as above)
status=ok tools=2
   4106  4074 501 node fixture.mjs owned-under-setuid
   4111  4106   0 /usr/bin/crontab -e
   4112  4111 501 sleep 313
$ mcporter daemon stop
Daemon stopped (if it was running). [exit 0]
# no fixture, crontab, or sleep process left afterwards

3. Stop refused for unverified retirement, zero active calls

With (2) fixed, the patched build needs another way to fail capture. Here the server's root process is setuid: command: /usr/bin/crontab -e with EDITOR set to a script that execs the fixture. MCP works over the inherited stdio, and the root process is uid 0, so capture fails on every build. crontab -l unchanged.

Before (main; release is the same):

$ mcporter list other --json
status=ok tools=2
$ mcporter list setuidroot --json
status=error  'Transport ownership could not be verified; inspect it before recovery.'
$ mcporter daemon stop
Error: Retirement is unverified; inspect the previous transport before stopping this owner.   [exit 1]
$ mcporter call other.ping
Error: Daemon is draining.                [exit 1]
$ kill -TERM 279
daemon pid 279 alive
$ mcporter call other.ping
Error: Daemon is draining.                [exit 1]

After:

$ mcporter daemon stop
Error: Retirement is unverified; inspect the previous transport before stopping this owner.   [exit 1]
$ mcporter call other.ping
pong                                      [exit 0]
$ kill -TERM 3461
daemon pid 3461 alive
$ mcporter call other.ping
pong                                      [exit 0]

The same wedge happens on main with no contrived setup: after the setuid fixture from (2) fails capture, daemon stop leaves other.ping failing with Daemon is draining., and the daemon survives SIGTERM.

Tests

New or extended regression tests, run against main source. Four of the five fail there; 48 other tests in those files pass:

• tests/daemon-liveness.test.ts: a call succeeds after a refused stop.
• tests/daemon-broker-transport-capacity.test.ts: after a refused stop for unverified retirement, nothing further is retired and a call succeeds.
• tests/process-retirement.test.ts: foreign-owned descendant excluded; owned process beneath a foreign parent kept.
• tests/process-retirement.test.ts: foreign row with no start identity refused. This one passes on main, which refuses all foreign rows. It stops the new exclusion from accepting incomplete identity.

pnpm check passes. pnpm test: 2,367 passed, 8 failed. On main: 2,364 passed, 8 failed. The same 8 tests fail on both, all in tests/generate-cli.test.ts and tests/cli-generate-cli.integration.test.ts; the error is Bun 1.4.4 or newer is required for native builds; found 1.4.2, so they come from my local Bun version.

Limits of this evidence

• Tracking the owned grandchild is only unit-tested. Scenario 2 shows the real tree connects and shuts down cleanly. I tried to make the grandchild outlive shutdown, to watch retirement wait for it, with an editor ignoring TERM, HUP and INT, but it was still gone after daemon stop. So that wait isn't demonstrated end to end.
• Windows is covered only by mocked inventories. Nothing here ran on a Windows host. The owner comparison uses SIDs there.
• Linux wasn't run. The evidence is macOS only.

Not changed

• A retirement that fails after shutdown has begun, inside the close() loop, still leaves the daemon draining. Some transports may already be closed by then, so resuming needs recovery logic; a repeated stop can retry. I left that for separate work.
• A daemon that refuses a stop also refuses SIGTERM, because the handler calls the same close(). It now keeps serving rather than sitting wedged, but it still needs SIGKILL to replace it. That may deserve its own change.
• Foreign-owned processes aren't waited for or signalled by retirement.

Fixture and config used

fixture.mjs:

import { spawn } from 'node:child_process';
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js';

const mode = process.argv[2] ?? 'plain';
if (mode === 'setuid-child') spawn('/usr/bin/top', ['-l', '0', '-s', '3131'], { stdio: 'ignore' });
if (mode === 'owned-under-setuid') {
  const editor = process.env.REPRO_EDITOR;
  spawn('/usr/bin/crontab', ['-e'], { stdio: 'ignore', env: { ...process.env, EDITOR: editor, VISUAL: editor } });
}
await new Promise((r) => setTimeout(r, 1500));
const server = new McpServer({ name: `fixture-${mode}`, version: '1.0.0' });
server.tool('ping', {}, async () => ({ content: [{ type: 'text', text: 'pong' }] }));
server.tool('slow', {}, async () => {
  await new Promise((r) => setTimeout(r, 15000));
  return { content: [{ type: 'text', text: 'slow done' }] };
});
await server.connect(new StdioServerTransport());

Config (all "lifecycle": "keep-alive", "imports": []):

Server	Command
plain, other	node fixture.mjs plain
setuid	node fixture.mjs setuid-child
grandchild	node fixture.mjs owned-under-setuid, REPRO_EDITOR = script running exec sleep 313
setuidroot	/usr/bin/crontab -e, EDITOR and VISUAL = script running exec node fixture.mjs plain
xcode	npx -y xcodebuildmcp@latest mcp

Isolation: export MCPORTER_DAEMON_DIR=$(mktemp -d "$HOME/.mi.XXXX"), mode 0700. Keep the path short because of the socket length limit. Overriding HOME doesn't isolate the daemon.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[chatgpt-codex-connector]

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-07T19:28:48.670766Z	aa94664	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

This PR keeps the daemon serving after an upfront stop refusal and excludes verified foreign-owned descendants while retaining owned descendants beneath them.

Example: Run daemon stop while plain.slow is executing a 15-second call.

• Before: Stop refuses, but subsequent plain.ping calls permanently fail with daemon_draining.
• After: Stop refuses and plain.ping continues returning pong.

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, correct repair with concrete shipped-CLI evidence, useful regressions, and passing checks on the reviewed head.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The supplied macOS CLI output exercises broker stop refusals, subsequent successful calls, real xcodebuildmcp discovery, setuid-child shutdown, an owned grandchild beneath a foreign parent, and rejection of a foreign-owned root; surviving-grandchild waiting is covered only by unit tests and unchanged retirement logic.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A refused daemon stop disables later calls, and otherwise usable MCP servers fail when their process trees contain verified setuid helpers.
Reason: The narrowly scoped repairs restore daemon availability and common-server compatibility within VISION.md without adding configuration or changing public APIs.

Merge readiness

✅ Ready for maintainer review

Keep open: this PR fixes current daemon failures with a focused patch, credible CLI evidence, and passing checks; no actionable introduced defect was found.

Priority: P1
Reviewed head: aa9466484ff5a9dcbd902e549f3db843154d3400

Before merge

None.

Findings

None.

Agent review details

How this fits together

The daemon broker accepts authenticated client operations, retains MCP transports, and uses process identities to verify retirement before shutdown or replacement.

flowchart LR
  A[CLI client] --> B[Authenticated daemon host]
  B --> C[Daemon broker]
  C --> D[MCP transport]
  D --> E[Server process tree]
  C --> F[Process retirement verifier]
  E --> F

Loading

Technical review

Best possible solution:

Land the focused refusal-ordering and owned-tree traversal fixes while preserving existing retirement identity checks.

Do we have a high-confidence way to reproduce the issue?

The contributor supplies concrete current-main and 0.17.0 CLI reproductions with fixtures, configuration, environment, and before/after output; source inspection confirms both failure paths, without an independent execution during this review.

Is this the best way to solve the issue?

The patch fixes the existing owners directly: refuse before entering draining state, and separate traversal from the set of verified same-user processes to await.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 21624e8cc823.

Provenance checked

• Daemon stop admission keeps the original intent (95edea1: fix(daemon): share one user daemon across configurations #346 introduced shared ownership, active-work protection, and verified retirement.)
• Owned process-tree capture changes intended behavior with a stated reason (95edea1: fix(daemon): share one user daemon across configurations #346 established fail-closed ownership capture; this PR documents real setuid helpers as the reason to distinguish verified foreign descendants from unknown identities.)

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

• Current-main stop failure and focused repair: Main sets draining before refusing active calls or unverified retirement; the introduced patch checks both upfront refusals before changing state and preserves the check after serialized work. (src/daemon/broker.ts:380, aa9466484ff5)
• Ownership boundary retained: Capture still rejects a foreign-owned root and descendants with incomplete identities, traverses foreign-owned descendants to collect owned grandchildren, and leaves retirement-time owner/start-identity verification intact. (src/daemon/process-retirement.ts:215, aa9466484ff5)
• Supplied shipped-entry-point evidence: The full PR body records macOS 27.0.1 arm64 and Node 25.9.0 CLI traces against published 0.17.0, main, and the patch: both stop refusals preserve subsequent calls, xcodebuildmcp lists 24 tools, setuid-child fixtures connect and stop, and a foreign-owned root remains rejected. The author explicitly limits surviving-grandchild waiting to unit coverage. (aa9466484ff5)
• Feature provenance: Changed base lines trace to the shared-user daemon implementation in fix(daemon): share one user daemon across configurations #346, whose stated contract requires verified retirement and protects active work. The patch explains the setuid compatibility exception without treating unknown ownership as absence. (src/daemon/broker.ts:381, 95edea12d715)
• Existing persisted-state compatibility: Legacy migration persists the same ProcessIdentity array and reads existing markers through unchanged validation and awaitRetirement logic; this patch adds no format, migration, or config change. (src/daemon/migration.ts:120, aa9466484ff5)
• Checks cover the reviewed head: Eight checks succeeded on the exact reviewed head, including Windows, macOS, Ubuntu, and Node 26 builds; one dispatch check was skipped. No target code or tests were executed during this read-only review. (aa9466484ff5)

Likely related people:

• Peter Steinberger: Raw commit 95edea1 adds src/daemon/broker.ts:345 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 95edea12d715; files: src/daemon/broker.ts)
• Vitālijs Silins: Raw commit 50efe6a adds src/daemon/broker.ts:407 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: 50efe6ae7470; files: src/daemon/broker.ts)

Preflight source

Source unknown. Source comparison at the observed PR snapshot only. Completion is self-reported; body, policy, tests, execution, and report bytes are not verified.

Review metrics

Metric	Value	Why it matters
Patch size	Production +19/-10; tests +25/-1; changelog +3	Small production changes address the two underlying defects, with focused coverage for refusal recovery and process-tree filtering.

Labels

Label changes:

• add P1: A refused stop can disable every subsequent client operation on the shared daemon, and ordinary setuid helpers prevent affected MCP servers from connecting.
• add proof: sufficient: Contributor real behavior proof is sufficient.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P1: A refused stop can disable every subsequent client operation on the shared daemon, and ordinary setuid helpers prevent affected MCP servers from connecting.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.
• proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed November 7, 2026, 2:38 PM ET / 19:38 UTC.

[zm2231]

My agent got overzealous and added changelog entries here, but feel free to remove them.