From 3f2fd08713a3dd5a70151b61138b05b5d46d6fc6 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Tue, 22 Sep 2026 21:37:51 +0200 Subject: [PATCH] fix: create the state directory before the auto-spawn path writes into it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `~/.appduct` was only ever created by `startDaemon`, i.e. inside the daemon child — but the spawn-lock and `daemon.log`'s fd are opened by the *parent* process, before that child exists. On a machine that has never run Appduct, every command that auto-spawns a daemon (`appduct ls`, `appduct daemon start|status`, and `appduct mcp`, which died before an MCP client could finish `initialize`) failed with a bare `ENOENT: ... open '~/.appduct/daemon.spawn.lock'` until someone ran `appduct daemon run` in the foreground once. `spawnDaemonAndWait` now calls `ensureStateDir` before taking the lock, which covers the log fd behind it too; the directory gets the same `0700` the daemon would have applied. Every existing test starts from a `mkdtemp`'d state dir, which is why this went unnoticed — the regression test deliberately points at a path that does not exist, and asserts the directory is there by the time the spawn runs. --- CHANGELOG.md | 12 ++++++ docs/ARCHITECTURE.md | 11 +++-- .../appduct/src/__tests__/rpc-client.test.ts | 42 ++++++++++++++++++- packages/appduct/src/rpc/client.ts | 14 +++++-- 4 files changed, 71 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94409dc..da1cbfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,18 @@ This file is maintained by hand. There is no automated changelog tooling (see `docs/CI.md#release-policy` for why) — update this file as part of the commit that bumps the package versions for a release. +## Unreleased + +- **Fix: the first Appduct command on a clean machine no longer fails with a bare `ENOENT`.** + Nothing created the state directory before the auto-spawn path wrote into it: `~/.appduct` is + created by `startDaemon`, but the spawn-lock and `daemon.log`'s fd are opened by the *parent* + process, before the daemon it spawns exists. So with no `~/.appduct` yet, every command that + auto-spawns a daemon — `appduct ls`, `appduct daemon start|status`, and `appduct mcp`, which + died before an MCP client could finish `initialize` — failed with + `ENOENT: ... open '~/.appduct/daemon.spawn.lock'` until someone ran `appduct daemon run` in the + foreground once. The auto-spawn path now creates the directory (mode `0700`, same as the daemon + would) before taking the lock. + ## 0.11.0 (2026-09-22) - **Docs: designing tools for agents.** `docs/TOOLS.md` gains a "Designing tools for agents" diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5e07208..d3f31ca 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -66,7 +66,9 @@ Deliberately out of scope, so the boundaries of the design are explicit: ## 3. State directory Default `~/.appduct/`, overridable with `APPDUCT_STATE_DIR` (tests rely on the -override). Created lazily with mode `0700`. Layout: +override). Created lazily with mode `0700` — by `startDaemon`, and by the auto-spawn path before +it takes the spawn-lock (§4), since that writes the lock and `daemon.log` from the *parent* +process, before the daemon it spawns exists. Layout: | Path | Purpose | Mode | | --- | --- | --- | @@ -154,9 +156,10 @@ only-when-no-sessions-are-live (§4, "Version drift"). `daemonLogMaxBytes` — §3); `stop` sends `daemon.shutdown` over RPC (SIGTERM fallback via pidfile); `status` renders `daemon.status`. - **Auto-spawn:** the shared RPC client library used by every CLI command attempts to - connect to `daemon.sock`. On `ENOENT`/`ECONNREFUSED` it (1) takes an exclusive - spawn-lock file to prevent double-spawn races, (2) spawns `daemon run` detached, - (3) polls the socket until ready (timeout 5 s), (4) retries the original request. + connect to `daemon.sock`. On `ENOENT`/`ECONNREFUSED` it (1) creates the state directory if it + is not there yet (§3), (2) takes an exclusive spawn-lock file to prevent double-spawn races, + (3) spawns `daemon run` detached, (4) polls the socket until ready (timeout 5 s), (5) retries + the original request. A stale socket file with a dead pid is unlinked before spawning. - Single instance is enforced via the pidfile (write with `O_EXCL`; on conflict, check liveness and take over only if dead). Liveness is `process.kill(pid, 0)` — with `EPERM` counted diff --git a/packages/appduct/src/__tests__/rpc-client.test.ts b/packages/appduct/src/__tests__/rpc-client.test.ts index 2ad8259..f11ec9c 100644 --- a/packages/appduct/src/__tests__/rpc-client.test.ts +++ b/packages/appduct/src/__tests__/rpc-client.test.ts @@ -6,8 +6,12 @@ * The cases that genuinely need a real daemon on the other end of the socket live in * `rpc-client.integration.test.ts`. */ -import { rm, utimes, writeFile } from "node:fs/promises"; +import { rm, stat, utimes, writeFile } from "node:fs/promises"; import { createServer, type Server, type Socket } from "node:net"; +import { existsSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { randomUUID } from "node:crypto"; import { afterEach, describe, expect, test } from "vitest"; @@ -217,6 +221,42 @@ describe("callDaemon", () => { await rm(stateDir, { force: true, recursive: true }); }); + + test("a state dir that does not exist yet is created before the spawn-lock is taken", async () => { + // The fresh-install case: nothing has ever run `startDaemon`, so `~/.appduct` is absent. Every + // other test in this file starts from a `mkdtemp`'d state dir, which is exactly how this + // slipped through — the parent writes the spawn-lock and `daemon.log` itself, long before the + // child it spawns would create the directory. + // Short name on purpose: the control socket lives inside this directory, and a UDS path + // over the platform limit (104 bytes on macOS) fails to connect with EINVAL. + const stateDir = path.join(tmpdir(), `appduct-fresh-${randomUUID().slice(0, 8)}`); + stateDirs.push(stateDir); + expect(existsSync(stateDir)).toBe(false); + + let spawnCalls = 0; + // Recorded *inside* the spawn: `defaultSpawn` opens `daemon.log`'s fd before the child exists, + // so "the directory is there by the time spawn runs" is the property that actually matters. + let stateDirExistedAtSpawn = false; + const spawn: SpawnFn = () => { + spawnCalls += 1; + stateDirExistedAtSpawn = existsSync(stateDir); + }; + + await expect( + callDaemon( + "daemon.status", + {}, + { stateDir, autoSpawn: true, spawn, spawnPollIntervalMs: 20, spawnWaitTimeoutMs: 150 }, + ), + // Times out waiting for the daemon this fake spawn never starts — the point is that it gets + // that far at all, instead of throwing ENOENT on the lock path. + ).rejects.toThrow(DaemonUnavailableError); + + expect(spawnCalls).toBe(1); + expect(stateDirExistedAtSpawn).toBe(true); + // Same 0700 the daemon itself would have applied (ARCHITECTURE.md §3). + expect((await stat(stateDir)).mode & 0o777).toBe(0o700); + }); }); diff --git a/packages/appduct/src/rpc/client.ts b/packages/appduct/src/rpc/client.ts index bbc0dbc..07f9527 100644 --- a/packages/appduct/src/rpc/client.ts +++ b/packages/appduct/src/rpc/client.ts @@ -1,7 +1,8 @@ /** * The auto-spawning RPC client library (ARCHITECTURE.md §4, §5) used by the CLI, MCP server, and - * tests to talk to `daemon.sock`. On `ENOENT`/`ECONNREFUSED` it takes an exclusive spawn-lock, - * spawns `daemon run` detached, polls the socket until ready, then retries the request once. + * tests to talk to `daemon.sock`. On `ENOENT`/`ECONNREFUSED` it creates the state dir if it is + * missing, takes an exclusive spawn-lock, spawns `daemon run` detached, polls the socket until + * ready, then retries the request once. */ import { connect, type Socket } from "node:net"; @@ -18,7 +19,7 @@ import { } from "../daemon/log-rotation.js"; import { isProcessAlive } from "../daemon/pidfile.js"; import { isSocketConnectable } from "../daemon/socket-probe.js"; -import { getStateDirPaths, type StateDirPaths } from "../daemon/state-dir.js"; +import { ensureStateDir, getStateDirPaths, type StateDirPaths } from "../daemon/state-dir.js"; import { getPackageRoot } from "../package-root.js"; import { DAEMON_VERSION_OVERRIDE_ENV } from "../package-version.js"; @@ -584,6 +585,13 @@ const spawnDaemonAndWait = async ( pollIntervalMs: number, options: SpawnDaemonOptions = {}, ): Promise => { + // On a fresh machine nothing has created the state dir yet — `startDaemon` does it, but only + // once the child is already running, and everything below this line writes into that directory + // *from the parent*: the spawn-lock right here, then `daemon.log`'s fd inside `defaultSpawn`. + // Without this, the first command on a clean install (including `appduct mcp`, which dies + // before an MCP client ever finishes `initialize`) fails with a bare `ENOENT` on the lock path. + await ensureStateDir(paths.root); + const acquiredLock = await acquireSpawnLock(paths.spawnLockPath); let lockWaitTimedOut = false;