Skip to content

Commit 539a612

Browse files
backnotpropbacknotprop
committed
Fix daemon runtime compatibility regressions
1 parent 678ebf2 commit 539a612

12 files changed

Lines changed: 163 additions & 53 deletions

File tree

‎apps/hook/server/index.ts‎

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ import { openBrowser } from "@plannotator/server/browser";
7575
import { cleanupDaemonState, discoverDaemon } from "@plannotator/server/daemon/client";
7676
import { startDaemonRuntime } from "@plannotator/server/daemon/runtime";
7777
import { createDaemonSessionFactory } from "@plannotator/server/daemon/session-factory";
78+
import { getDaemonStartCommand } from "@plannotator/server/daemon/start-command";
7879
import { formatRemoteShareNotice } from "@plannotator/server/share-url";
7980
import { hostnameOrFallback } from "@plannotator/shared/project";
8081
import { readImprovementHook } from "@plannotator/shared/improvement-hooks";
@@ -438,15 +439,6 @@ function resolvePluginCwd(request: Partial<PluginBaseRequest>): string {
438439
return cwd;
439440
}
440441

441-
function getDaemonStartCommand(): string[] {
442-
const executable = process.argv[0];
443-
const entry = process.argv[1];
444-
if (entry && /\.(?:[cm]?[jt]s)$/.test(entry)) {
445-
return [executable, entry, "daemon", "start", "--foreground"];
446-
}
447-
return [executable, "daemon", "start", "--foreground"];
448-
}
449-
450442
async function ensureDaemonClient(options: { pluginError?: boolean } = {}) {
451443
const fail = options.pluginError ? emitPluginError : emitCommandError;
452444
const existing = await discoverDaemon();
@@ -479,6 +471,23 @@ async function runDaemonSessionRequest(request: PluginRequest): Promise<{
479471
result: PluginActionResult;
480472
session: PluginSessionInfo;
481473
}>;
474+
function registerDaemonSessionInterruptCleanup(cancelSession: () => Promise<void>): () => void {
475+
let cancelling = false;
476+
const handleSignal = (exitCode: number) => {
477+
if (cancelling) return;
478+
cancelling = true;
479+
void cancelSession().finally(() => process.exit(exitCode));
480+
};
481+
const onSigint = () => handleSignal(130);
482+
const onSigterm = () => handleSignal(143);
483+
process.once("SIGINT", onSigint);
484+
process.once("SIGTERM", onSigterm);
485+
return () => {
486+
process.off("SIGINT", onSigint);
487+
process.off("SIGTERM", onSigterm);
488+
};
489+
}
490+
482491
async function runDaemonSessionRequest(request: PluginRequest, options: { pluginError?: boolean } = {}): Promise<{
483492
result: PluginActionResult;
484493
session: PluginSessionInfo;
@@ -490,6 +499,11 @@ async function runDaemonSessionRequest(request: PluginRequest, options: { plugin
490499
fail(created.error.code, created.error.message);
491500
}
492501

502+
const cancelCreatedSession = async () => {
503+
await daemon.cancelSession(created.session.id).catch(() => undefined);
504+
};
505+
const unregisterInterruptCleanup = registerDaemonSessionInterruptCleanup(cancelCreatedSession);
506+
493507
const sessionUrl = new URL(created.session.url);
494508
const sessionPort = Number(sessionUrl.port);
495509
const session: PluginSessionInfo = {
@@ -505,29 +519,37 @@ async function runDaemonSessionRequest(request: PluginRequest, options: { plugin
505519
emitPluginSessionReady(session);
506520
}
507521

508-
if (request.action === "review") {
509-
await handleReviewServerReady(created.session.url, daemon.state.isRemote, sessionPort);
510-
} else if (request.action === "annotate" || request.action === "annotate-last") {
511-
await handleAnnotateServerReady(created.session.url, daemon.state.isRemote, sessionPort);
512-
} else {
513-
await handleServerReady(created.session.url, daemon.state.isRemote, sessionPort);
514-
}
522+
try {
523+
if (request.action === "review") {
524+
await handleReviewServerReady(created.session.url, daemon.state.isRemote, sessionPort);
525+
} else if (request.action === "annotate" || request.action === "annotate-last") {
526+
await handleAnnotateServerReady(created.session.url, daemon.state.isRemote, sessionPort);
527+
} else {
528+
await handleServerReady(created.session.url, daemon.state.isRemote, sessionPort);
529+
}
515530

516-
const completed = await daemon.waitForResult<PluginActionResult>(created.session.id);
517-
if (completed.ok !== true) {
518-
fail(completed.error.code, completed.error.message);
519-
}
520-
if (completed.session.status !== "completed") {
521-
fail(
522-
completed.session.status,
523-
completed.session.error ?? `Plannotator session ${completed.session.id} ended with status ${completed.session.status}.`,
524-
);
525-
}
531+
const completed = await daemon.waitForResult<PluginActionResult>(created.session.id);
532+
if (completed.ok !== true) {
533+
await cancelCreatedSession();
534+
fail(completed.error.code, completed.error.message);
535+
}
536+
if (completed.session.status !== "completed") {
537+
fail(
538+
completed.session.status,
539+
completed.session.error ?? `Plannotator session ${completed.session.id} ended with status ${completed.session.status}.`,
540+
);
541+
}
526542

527-
return {
528-
result: completed.result,
529-
session,
530-
};
543+
unregisterInterruptCleanup();
544+
return {
545+
result: completed.result,
546+
session,
547+
};
548+
} catch (err) {
549+
unregisterInterruptCleanup();
550+
await cancelCreatedSession();
551+
throw err;
552+
}
531553
}
532554

533555
async function runDaemonBackedPluginRequest(request: PluginRequest): Promise<void> {

‎packages/server/annotate.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ export async function createAnnotateSession(
191191

192192
// API: Serve images (local paths or temp uploads)
193193
if (url.pathname === "/api/image") {
194-
return handleImage(req);
194+
return handleImage(req, cwd);
195195
}
196196

197197
// API: Serve a linked markdown document

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ describe("daemon HTTP router", () => {
4040
expect(body.multiSession).toBe(true);
4141
});
4242

43+
test("serves the favicon at the daemon root", async () => {
44+
const { handler } = makeHandler();
45+
const res = await handler(new Request("http://127.0.0.1:4321/favicon.svg"));
46+
expect(res.status).toBe(200);
47+
expect(res.headers.get("content-type")).toContain("image/svg+xml");
48+
});
49+
4350
test("reports daemon status with active session count", async () => {
4451
const { handler } = makeHandler();
4552
await handler(new Request("http://127.0.0.1:4321/daemon/sessions", {

‎packages/server/daemon/server.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
import type { DaemonState } from "./state";
99
import { DaemonSessionStore, type DaemonSessionRecord } from "./session-store";
1010
import type { SessionRequestContext } from "../session-handler";
11+
import { handleFavicon } from "../shared-handlers";
1112

1213
const RESULT_DELETE_GRACE_MS = 2_000;
1314

@@ -76,6 +77,10 @@ export function createDaemonFetchHandler(options: DaemonServerOptions) {
7677
return json(getDaemonCapabilities());
7778
}
7879

80+
if (url.pathname === "/favicon.svg" && req.method === "GET") {
81+
return handleFavicon();
82+
}
83+
7984
if (url.pathname === "/daemon/status" && req.method === "GET") {
8085
const status: DaemonStatus = {
8186
ok: true,

‎packages/server/daemon/start-command.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { describe, expect, test } from "bun:test";
2+
import { getDaemonStartCommand } from "./start-command";
3+
4+
describe("getDaemonStartCommand", () => {
5+
test("uses Bun plus the source entry when running from TypeScript", () => {
6+
expect(getDaemonStartCommand(["bun", "apps/hook/server/index.ts"], "/usr/local/bin/bun")).toEqual([
7+
"/usr/local/bin/bun",
8+
"apps/hook/server/index.ts",
9+
"daemon",
10+
"start",
11+
"--foreground",
12+
]);
13+
});
14+
15+
test("uses the executable itself for compiled Bun binaries", () => {
16+
expect(getDaemonStartCommand(["bun", "/$bunfs/root/index"], "/usr/local/bin/plannotator")).toEqual([
17+
"/usr/local/bin/plannotator",
18+
"daemon",
19+
"start",
20+
"--foreground",
21+
]);
22+
});
23+
});

‎packages/server/daemon/start-command.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
export function getDaemonStartCommand(
2+
argv: string[] = process.argv,
3+
execPath = process.execPath,
4+
): string[] {
5+
const entry = argv[1];
6+
if (entry && /\.(?:[cm]?[jt]s)$/.test(entry)) {
7+
return [execPath, entry, "daemon", "start", "--foreground"];
8+
}
9+
return [execPath, "daemon", "start", "--foreground"];
10+
}

‎packages/server/image.test.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,24 @@
44
* Run: bun test packages/server/image.test.ts
55
*/
66

7-
import { describe, expect, test } from "bun:test";
7+
import { afterEach, describe, expect, test } from "bun:test";
8+
import { mkdtempSync, rmSync } from "node:fs";
89
import { tmpdir } from "node:os";
10+
import { join } from "node:path";
911
import { validateImagePath, validateUploadExtension, UPLOAD_DIR } from "./image";
12+
import { handleImage } from "./shared-handlers";
13+
14+
const dirs: string[] = [];
15+
16+
afterEach(() => {
17+
for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true });
18+
});
19+
20+
function tempDir(): string {
21+
const dir = mkdtempSync(join(tmpdir(), "plannotator-image-"));
22+
dirs.push(dir);
23+
return dir;
24+
}
1025

1126
describe("UPLOAD_DIR", () => {
1227
test("uses os.tmpdir(), not hardcoded /tmp", () => {
@@ -61,3 +76,21 @@ describe("validateUploadExtension", () => {
6176
expect(result.ext).toBe("png");
6277
});
6378
});
79+
80+
describe("handleImage", () => {
81+
test("resolves relative image paths against the session cwd before process cwd", async () => {
82+
const cwd = tempDir();
83+
const session = tempDir();
84+
await Bun.write(join(cwd, "mock.png"), "wrong");
85+
await Bun.write(join(session, "mock.png"), "right");
86+
const originalCwd = process.cwd();
87+
88+
try {
89+
process.chdir(cwd);
90+
const response = await handleImage(new Request("http://localhost/api/image?path=mock.png"), session);
91+
expect(await response.text()).toBe("right");
92+
} finally {
93+
process.chdir(originalCwd);
94+
}
95+
});
96+
});

‎packages/server/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@ export async function createPlannotatorSession(
340340

341341
// API: Serve images (local paths or temp uploads)
342342
if (url.pathname === "/api/image") {
343-
return handleImage(req);
343+
return handleImage(req, cwd);
344344
}
345345

346346
// API: Upload image -> save to temp -> return path

‎packages/server/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
"./daemon/client": "./daemon/client.ts",
2626
"./daemon/runtime": "./daemon/runtime.ts",
2727
"./daemon/session-factory": "./daemon/session-factory.ts",
28+
"./daemon/start-command": "./daemon/start-command.ts",
2829
"./project": "./project.ts",
2930
"./pr": "./pr.ts"
3031
},

‎packages/server/review.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -976,7 +976,7 @@ export async function createReviewSession(
976976

977977
// API: Serve images (local paths or temp uploads)
978978
if (url.pathname === "/api/image") {
979-
return handleImage(req);
979+
return handleImage(req, cwd);
980980
}
981981

982982
// API: Upload image -> save to temp -> return path

0 commit comments

Comments
 (0)