Skip to content

Commit de22e89

Browse files
backnotpropbacknotprop
committed
Fix daemon startup and scoped resources
1 parent 23a8332 commit de22e89

7 files changed

Lines changed: 166 additions & 50 deletions

File tree

‎apps/hook/server/index.ts‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ const detectedOrigin: Origin =
252252

253253
async function runDaemonCommand(): Promise<void> {
254254
const command = args[1] ?? "status";
255+
const foreground = args.includes("--foreground");
255256

256257
if (command === "status") {
257258
const daemon = await discoverDaemon({ validateEnvironment: false });
@@ -296,6 +297,32 @@ async function runDaemonCommand(): Promise<void> {
296297
process.exit(1);
297298
}
298299

300+
if (!foreground) {
301+
const child = Bun.spawn(getDaemonStartCommand(), {
302+
cwd: getInvocationCwd(),
303+
stdin: "ignore",
304+
stdout: "ignore",
305+
stderr: "ignore",
306+
});
307+
child.unref();
308+
309+
for (let attempt = 0; attempt < 30; attempt++) {
310+
await Bun.sleep(100);
311+
const daemon = await discoverDaemon();
312+
if (daemon.ok) {
313+
console.log(JSON.stringify({ ok: true, started: true, status: daemon.status }));
314+
process.exit(0);
315+
}
316+
}
317+
318+
console.log(JSON.stringify({
319+
ok: false,
320+
code: "daemon-start-failed",
321+
message: "Timed out waiting for the Plannotator daemon to start.",
322+
}));
323+
process.exit(1);
324+
}
325+
299326
let runtime: Awaited<ReturnType<typeof startDaemonRuntime>>;
300327
try {
301328
runtime = await startDaemonRuntime({
@@ -399,16 +426,24 @@ function resolvePluginCwd(request: Partial<PluginBaseRequest>): string {
399426
err instanceof Error ? err.message : `Invalid cwd: ${request.cwd || cwd}`,
400427
);
401428
}
429+
try {
430+
process.chdir(cwd);
431+
} catch (err) {
432+
emitPluginError(
433+
"invalid-cwd",
434+
err instanceof Error ? err.message : `Invalid cwd: ${request.cwd || cwd}`,
435+
);
436+
}
402437
return cwd;
403438
}
404439

405440
function getDaemonStartCommand(): string[] {
406441
const executable = process.argv[0];
407442
const entry = process.argv[1];
408443
if (entry && /\.(?:[cm]?[jt]s)$/.test(entry)) {
409-
return [executable, entry, "daemon", "start"];
444+
return [executable, entry, "daemon", "start", "--foreground"];
410445
}
411-
return [executable, "daemon", "start"];
446+
return [executable, "daemon", "start", "--foreground"];
412447
}
413448

414449
async function ensureDaemonClient(options: { pluginError?: boolean } = {}) {
@@ -621,7 +656,7 @@ if (args[0] === "sessions") {
621656
console.error(`Session #${n} not found. ${sessions.length} active session(s).`);
622657
process.exit(1);
623658
}
624-
await openBrowser(session.url);
659+
await openBrowser(session.url, { isRemote: daemon.status.endpoint.isRemote });
625660
console.error(`Opened ${session.mode} session in browser: ${session.url}`);
626661
process.exit(0);
627662
}

‎packages/server/daemon/client.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,35 @@ describe("DaemonClient", () => {
9292
expect(existsSync(paths.statePath)).toBe(false);
9393
expect(existsSync(paths.lockPath)).toBe(false);
9494
});
95+
96+
test("does not signal recorded PID when endpoint shutdown fails", async () => {
97+
const baseDir = tempBase();
98+
const paths = getDaemonPaths({ baseDir });
99+
writeDaemonState(state(), { baseDir });
100+
writeFileSync(paths.lockPath, "123\n", "utf-8");
101+
const originalKill = process.kill;
102+
let killed = false;
103+
104+
(process as typeof process & { kill: typeof process.kill }).kill = (() => {
105+
killed = true;
106+
return true;
107+
}) as typeof process.kill;
108+
109+
try {
110+
await cleanupDaemonState(state(), {
111+
baseDir,
112+
fetch: async () => {
113+
throw new Error("endpoint is gone");
114+
},
115+
});
116+
} finally {
117+
(process as typeof process & { kill: typeof process.kill }).kill = originalKill;
118+
}
119+
120+
expect(killed).toBe(false);
121+
expect(existsSync(paths.statePath)).toBe(false);
122+
expect(existsSync(paths.lockPath)).toBe(false);
123+
});
95124
});
96125

97126
describe("discoverDaemon", () => {

‎packages/server/daemon/client.ts‎

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -86,49 +86,15 @@ function stateBaseUrl(state: unknown): string | undefined {
8686
return typeof baseUrl === "string" ? baseUrl : undefined;
8787
}
8888

89-
function statePid(state: unknown): number | undefined {
90-
const pid = (state as { pid?: unknown } | null)?.pid;
91-
return typeof pid === "number" && Number.isInteger(pid) && pid > 0 ? pid : undefined;
92-
}
93-
94-
function isProcessAlive(pid: number, options: DaemonClientOptions): boolean {
95-
const isAlive = options.isAlive ?? ((targetPid: number) => {
96-
try {
97-
process.kill(targetPid, 0);
98-
return true;
99-
} catch {
100-
return false;
101-
}
102-
});
103-
return isAlive(pid);
104-
}
105-
106-
async function waitForProcessExit(pid: number, options: DaemonClientOptions): Promise<void> {
107-
for (let attempt = 0; attempt < 10; attempt++) {
108-
if (!isProcessAlive(pid, options)) return;
109-
await new Promise((resolve) => setTimeout(resolve, 50));
110-
}
111-
}
112-
11389
export async function cleanupDaemonState(state: unknown, options: DaemonClientOptions = {}): Promise<void> {
11490
const fetchImpl = options.fetch ?? fetch;
11591
const baseUrl = stateBaseUrl(state);
116-
let shutdownOk = false;
11792
if (baseUrl) {
11893
try {
119-
const response = await fetchImpl(`${baseUrl}/daemon/shutdown`, { method: "POST" });
120-
shutdownOk = response.ok;
121-
} catch {
122-
shutdownOk = false;
123-
}
124-
}
125-
const pid = statePid(state);
126-
if (baseUrl && !shutdownOk && pid && isProcessAlive(pid, options)) {
127-
try {
128-
process.kill(pid, "SIGTERM");
129-
await waitForProcessExit(pid, options);
94+
await fetchImpl(`${baseUrl}/daemon/shutdown`, { method: "POST" });
13095
} catch {
131-
// Best effort; removing the stale state lets the caller start or report the real port error.
96+
// Best effort only. Do not signal the recorded PID here; stale daemon
97+
// state can outlive the process and the PID may now belong to something else.
13298
}
13399
}
134100
removeDaemonFiles(options);

‎packages/server/daemon/session-factory.test.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { afterEach, describe, expect, test } from "bun:test";
2-
import { mkdtempSync, rmSync, writeFileSync } from "fs";
2+
import { existsSync, mkdtempSync, rmSync, writeFileSync } from "fs";
33
import { join } from "path";
44
import { tmpdir } from "os";
55
import { DaemonSessionStore } from "./session-store";
@@ -86,6 +86,51 @@ describe("createDaemonSessionFactory", () => {
8686
expect(completed.result?.approved).toBe(true);
8787
});
8888

89+
test("resolves relative plan save paths against the request cwd", async () => {
90+
const home = tempDir("plannotator-daemon-home-");
91+
const cwd = tempDir("plannotator-daemon-cwd-");
92+
process.env.HOME = home;
93+
94+
const store = new DaemonSessionStore({ now: () => 1_000 });
95+
const factory = createDaemonSessionFactory({
96+
planHtmlContent: "<html><head></head><body>Plan</body></html>",
97+
reviewHtmlContent: "<html><head></head><body>Review</body></html>",
98+
});
99+
const context: DaemonFetchContext = {
100+
endpoint: {
101+
hostname: "127.0.0.1",
102+
port: 4321,
103+
baseUrl: "http://127.0.0.1:4321",
104+
isRemote: false,
105+
},
106+
store,
107+
};
108+
109+
const record = await factory({
110+
request: {
111+
action: "plan",
112+
origin: "opencode",
113+
cwd,
114+
plan: "# Saved Plan\n\nStore this under the session cwd.",
115+
},
116+
}, context);
117+
118+
const response = await record.handleRequest!(
119+
new Request("http://127.0.0.1:4321/api/approve", {
120+
method: "POST",
121+
body: JSON.stringify({ planSave: { enabled: true, customPath: "./plans" } }),
122+
}),
123+
new URL("http://127.0.0.1:4321/api/approve"),
124+
);
125+
const body = await response.json();
126+
127+
expect(body.savedPath.startsWith(join(cwd, "plans"))).toBe(true);
128+
expect(existsSync(body.savedPath)).toBe(true);
129+
130+
const completed = await store.waitForResult<{ savedPath?: string }>(record.id);
131+
expect(completed.result?.savedPath).toBe(body.savedPath);
132+
});
133+
89134
test("preserves at-reference annotate resolution through the daemon", async () => {
90135
const home = tempDir("plannotator-daemon-home-");
91136
const cwd = tempDir("plannotator-daemon-cwd-");

‎packages/server/index.ts‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
*/
1414

1515
import type { Origin } from "@plannotator/shared/agents";
16-
import { resolve } from "path";
1716
import { isRemoteSession, getServerHostname, getServerPort } from "./remote";
1817
import { openEditorDiff } from "./ide";
1918
import {
@@ -47,7 +46,7 @@ import { composeImproveContext } from "@plannotator/shared/pfm-reminder";
4746
import { handleImage, handleUpload, handleAgents, handleServerReady, handleDraftSave, handleDraftLoad, handleDraftDelete, handleFavicon, type OpencodeClient } from "./shared-handlers";
4847
import { contentHash, deleteDraft } from "./draft";
4948
import { handleDoc, handleDocExists, handleObsidianVaults, handleObsidianFiles, handleObsidianDoc, handleFileBrowserFiles } from "./reference-handlers";
50-
import { warmFileListCache } from "@plannotator/shared/resolve-file";
49+
import { resolveUserPath, warmFileListCache } from "@plannotator/shared/resolve-file";
5150
import { createEditorAnnotationHandler } from "./editor-annotations";
5251
import { createExternalAnnotationHandler } from "./external-annotations";
5352
import { isWSL } from "./browser";
@@ -137,6 +136,11 @@ export async function createPlannotatorSession(
137136
options: ServerOptions
138137
): Promise<PlannotatorSession> {
139138
const { cwd = process.cwd(), plan, origin, htmlContent, permissionMode, sharingEnabled = true, shareBaseUrl, pasteApiUrl, mode, customPlanPath } = options;
139+
const resolvePlanStoragePath = (customPath?: string | null): string | undefined => {
140+
if (!customPath?.trim()) return undefined;
141+
return resolveUserPath(customPath, cwd);
142+
};
143+
const archiveCustomPath = resolvePlanStoragePath(customPlanPath);
140144

141145
const wslFlag = await isWSL();
142146
const gitUser = detectGitUser();
@@ -152,9 +156,9 @@ export async function createPlannotatorSession(
152156
let donePromise: Promise<void> | undefined;
153157

154158
if (mode === "archive") {
155-
archivePlans = listArchivedPlans(customPlanPath ?? undefined);
159+
archivePlans = listArchivedPlans(archiveCustomPath);
156160
initialArchivePlan = archivePlans.length > 0
157-
? readArchivedPlan(archivePlans[0].filename, customPlanPath ?? undefined) ?? ""
161+
? readArchivedPlan(archivePlans[0].filename, archiveCustomPath) ?? ""
158162
: "";
159163
donePromise = new Promise<void>((resolve) => { resolveDone = resolve; });
160164
}
@@ -244,7 +248,7 @@ export async function createPlannotatorSession(
244248
// API: List archived plans (from ~/.plannotator/plans/)
245249
// Cached for session lifetime — new plans won't appear during a single review
246250
if (url.pathname === "/api/archive/plans" && req.method === "GET") {
247-
const customPath = url.searchParams.get("customPath") || undefined;
251+
const customPath = resolvePlanStoragePath(url.searchParams.get("customPath"));
248252
if (!cachedArchivePlans) cachedArchivePlans = listArchivedPlans(customPath);
249253
return Response.json({ plans: cachedArchivePlans });
250254
}
@@ -255,7 +259,7 @@ export async function createPlannotatorSession(
255259
if (!filename) {
256260
return Response.json({ error: "Missing filename parameter" }, { status: 400 });
257261
}
258-
const customPath = url.searchParams.get("customPath") || undefined;
262+
const customPath = resolvePlanStoragePath(url.searchParams.get("customPath"));
259263
const content = readArchivedPlan(filename, customPath);
260264
if (content === null) {
261265
return Response.json({ error: "Plan not found" }, { status: 404 });
@@ -485,7 +489,7 @@ export async function createPlannotatorSession(
485489
// Capture plan save settings
486490
if (body.planSave !== undefined) {
487491
planSaveEnabled = body.planSave.enabled;
488-
planSaveCustomPath = body.planSave.customPath;
492+
planSaveCustomPath = resolvePlanStoragePath(body.planSave.customPath);
489493
}
490494

491495
// Run integrations in parallel — they're independent
@@ -546,7 +550,7 @@ export async function createPlannotatorSession(
546550
// Capture plan save settings
547551
if (body.planSave !== undefined) {
548552
planSaveEnabled = body.planSave.enabled;
549-
planSaveCustomPath = body.planSave.customPath;
553+
planSaveCustomPath = resolvePlanStoragePath(body.planSave.customPath);
550554
}
551555
} catch {
552556
// Use default feedback

‎packages/ui/components/ImageThumbnail.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import { afterEach, describe, expect, test } from "bun:test";
2+
import { getImageSrc } from "./ImageThumbnail";
3+
4+
const originalWindow = globalThis.window;
5+
6+
function setWindow(value: Partial<Window>) {
7+
Object.defineProperty(globalThis, "window", {
8+
value,
9+
configurable: true,
10+
});
11+
}
12+
13+
afterEach(() => {
14+
Object.defineProperty(globalThis, "window", {
15+
value: originalWindow,
16+
configurable: true,
17+
});
18+
});
19+
20+
describe("getImageSrc", () => {
21+
test("uses the root API base by default", () => {
22+
setWindow({});
23+
expect(getImageSrc("/tmp/screen shot.png")).toBe("/api/image?path=%2Ftmp%2Fscreen%20shot.png");
24+
});
25+
26+
test("uses the daemon-scoped API base for local image resources", () => {
27+
setWindow({ __PLANNOTATOR_API_BASE__: "/s/sess_123/api" });
28+
expect(getImageSrc("/tmp/screen shot.png")).toBe("/s/sess_123/api/image?path=%2Ftmp%2Fscreen%20shot.png");
29+
expect(getImageSrc("images/mock.png", "/repo")).toBe("/s/sess_123/api/image?path=images%2Fmock.png&base=%2Frepo");
30+
});
31+
32+
test("leaves remote image URLs untouched", () => {
33+
setWindow({ __PLANNOTATOR_API_BASE__: "/s/sess_123/api" });
34+
expect(getImageSrc("https://example.com/image.png")).toBe("https://example.com/image.png");
35+
});
36+
});

‎packages/ui/components/ImageThumbnail.tsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import React, { useState } from 'react';
2+
import { apiPath } from '../utils/api';
23

34
/**
45
* Get the display URL for an image path or URL
@@ -7,7 +8,7 @@ export const getImageSrc = (path: string, base?: string): string => {
78
if (path.startsWith('http://') || path.startsWith('https://')) {
89
return path; // Remote URL, use directly
910
}
10-
let url = `/api/image?path=${encodeURIComponent(path)}`;
11+
let url = `${apiPath('/image')}?path=${encodeURIComponent(path)}`;
1112
if (base && !path.startsWith('/')) {
1213
url += `&base=${encodeURIComponent(base)}`;
1314
}

0 commit comments

Comments
 (0)