Skip to content

Commit 6530a78

Browse files
committed
Fix daemon session lifecycle and scoped callers
1 parent fc8a009 commit 6530a78

19 files changed

Lines changed: 314 additions & 39 deletions

.github/workflows/release.yml

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -170,22 +170,29 @@ jobs:
170170
local ok=0
171171
172172
for _ in $(seq 1 60); do
173-
if curl -sf "http://127.0.0.1:${port}${endpoint}" -o /dev/null 2>/dev/null; then
174-
ok=1
175-
break
173+
local sessions
174+
sessions="$(curl -sf "http://127.0.0.1:${port}/daemon/sessions" 2>/dev/null || true)"
175+
if [ -n "$sessions" ]; then
176+
local session_url
177+
session_url="$(python3 -c 'import json,sys; data=json.load(sys.stdin); sessions=data.get("sessions") or []; print(sessions[0].get("url","") if sessions else "")' <<< "$sessions")"
178+
if [ -n "$session_url" ] && curl -sf "${session_url}${endpoint}" -o /dev/null 2>/dev/null; then
179+
ok=1
180+
break
181+
fi
176182
fi
177183
sleep 0.5
178184
done
179185
180186
kill "$pid" 2>/dev/null || true
181187
wait "$pid" 2>/dev/null || true
188+
PLANNOTATOR_PORT="$port" "$BINARY" daemon stop >/dev/null 2>&1 || true
182189
183190
if [ "$ok" = "0" ]; then
184-
echo "FAIL: ${label} did not respond on :${port}${endpoint}"
191+
echo "FAIL: ${label} did not expose a daemon-scoped ${endpoint}"
185192
exit 1
186193
fi
187194
188-
echo "OK: ${label} responded on :${port}${endpoint}"
195+
echo "OK: ${label} exposed daemon-scoped ${endpoint}"
189196
}
190197
191198
# 2. review: exercises server startup, bundled HTML, git diff, and HTTP.
@@ -232,9 +239,14 @@ jobs:
232239
try {
233240
for ($i = 0; $i -lt 60; $i++) {
234241
try {
235-
Invoke-WebRequest -Uri "http://127.0.0.1:$Port$Endpoint" -UseBasicParsing -TimeoutSec 1 | Out-Null
236-
$ok = $true
237-
break
242+
$sessionsResponse = Invoke-WebRequest -Uri "http://127.0.0.1:$Port/daemon/sessions" -UseBasicParsing -TimeoutSec 1
243+
$sessionsBody = $sessionsResponse.Content | ConvertFrom-Json
244+
if ($sessionsBody.sessions.Count -gt 0) {
245+
$sessionUrl = $sessionsBody.sessions[0].url
246+
Invoke-WebRequest -Uri "$sessionUrl$Endpoint" -UseBasicParsing -TimeoutSec 1 | Out-Null
247+
$ok = $true
248+
break
249+
}
238250
} catch {
239251
if ($process.HasExited) {
240252
break
@@ -247,6 +259,7 @@ jobs:
247259
Stop-Process -Id $process.Id -Force
248260
Wait-Process -Id $process.Id -ErrorAction SilentlyContinue
249261
}
262+
& $binary daemon stop *> $null
250263
Remove-Item Env:\PLANNOTATOR_PORT -ErrorAction SilentlyContinue
251264
}
252265
@@ -255,10 +268,10 @@ jobs:
255268
Get-Content $stdout -ErrorAction SilentlyContinue
256269
Write-Host "stderr:"
257270
Get-Content $stderr -ErrorAction SilentlyContinue
258-
throw "FAIL: $Label did not respond on :$Port$Endpoint"
271+
throw "FAIL: $Label did not expose a daemon-scoped $Endpoint"
259272
}
260273
261-
Write-Host "OK: $Label responded on :$Port$Endpoint"
274+
Write-Host "OK: $Label exposed daemon-scoped $Endpoint"
262275
}
263276
264277
# 2. review: exercises server startup, bundled HTML, git diff, and HTTP.

apps/hook/server/index.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ import {
6161
import {
6262
handleAnnotateServerReady,
6363
} from "@plannotator/server/annotate";
64-
import { loadConfig } from "@plannotator/shared/config";
64+
import { loadConfig, resolveUseJina } from "@plannotator/shared/config";
6565
import { parseReviewArgs } from "@plannotator/shared/review-args";
6666
import { statSync, existsSync } from "fs";
6767
import {
@@ -72,7 +72,7 @@ import {
7272
buildPlanFileRule,
7373
} from "@plannotator/shared/prompts";
7474
import { openBrowser } from "@plannotator/server/browser";
75-
import { cleanupDaemonState, discoverDaemon } from "@plannotator/server/daemon/client";
75+
import { cleanupDaemonState, discoverDaemon, waitForDaemonShutdown } from "@plannotator/server/daemon/client";
7676
import { startDaemonRuntime } from "@plannotator/server/daemon/runtime";
7777
import { createDaemonSessionFactory } from "@plannotator/server/daemon/session-factory";
7878
import { getDaemonStartCommand } from "@plannotator/server/daemon/start-command";
@@ -127,6 +127,7 @@ const reviewHtmlContent = reviewHtml as unknown as string;
127127

128128
// Check for subcommand
129129
const args = process.argv.slice(2);
130+
const launcherCwd = process.cwd();
130131

131132
// Global flag: --browser <name>
132133
const browserIdx = args.indexOf("--browser");
@@ -282,6 +283,13 @@ async function runDaemonCommand(): Promise<void> {
282283
process.exit(1);
283284
}
284285
const result = await daemon.client.shutdown();
286+
if ("ok" in result && result.ok) {
287+
const stopped = await waitForDaemonShutdown(daemon.state);
288+
if (!stopped) {
289+
console.log(JSON.stringify({ ok: false, code: "daemon-stop-timeout", message: "Timed out waiting for the Plannotator daemon to stop." }));
290+
process.exit(1);
291+
}
292+
}
285293
console.log(JSON.stringify(result));
286294
process.exit("ok" in result && result.ok ? 0 : 1);
287295
}
@@ -300,7 +308,7 @@ async function runDaemonCommand(): Promise<void> {
300308
}
301309

302310
if (!foreground) {
303-
const child = Bun.spawn(getDaemonStartCommand(), {
311+
const child = Bun.spawn(getDaemonStartCommand(process.argv, process.execPath, launcherCwd), {
304312
cwd: getInvocationCwd(),
305313
stdin: "ignore",
306314
stdout: "ignore",
@@ -449,7 +457,7 @@ async function ensureDaemonClient(options: { pluginError?: boolean } = {}) {
449457
fail(`daemon-${existing.code}`, existing.message);
450458
}
451459

452-
const command = getDaemonStartCommand();
460+
const command = getDaemonStartCommand(process.argv, process.execPath, launcherCwd);
453461
const child = Bun.spawn(command, {
454462
cwd: getInvocationCwd(),
455463
stdin: "ignore",
@@ -523,6 +531,8 @@ async function runDaemonSessionRequest(request: PluginRequest, options: { plugin
523531
};
524532
if (created.session.remoteShare) {
525533
process.stderr.write(formatRemoteShareNotice(created.session.remoteShare));
534+
} else if (daemon.state.isRemote) {
535+
process.stderr.write(`\n Open this forwarded Plannotator session URL:\n ${created.session.url}\n\n`);
526536
}
527537
if (options.pluginError) {
528538
emitPluginSessionReady(session);
@@ -597,11 +607,14 @@ async function runPluginArchiveCommand(): Promise<void> {
597607
async function runPluginAnnotateCommand(defaultMode: "annotate" | "annotate-last" = "annotate"): Promise<void> {
598608
const request = await readPluginRequest<PluginAnnotateRequest>();
599609
const origin = getPluginOrigin(request);
610+
const useJina = resolveUseJina(request.noJina === true, loadConfig());
600611
await runDaemonBackedPluginRequest({
601612
...request,
602613
action: defaultMode,
603614
origin,
604615
cwd: resolvePluginCwd(request),
616+
useJina,
617+
jinaApiKey: process.env.JINA_API_KEY,
605618
});
606619
}
607620

@@ -754,6 +767,8 @@ if (args[0] === "sessions") {
754767
cwd: getInvocationCwd(),
755768
args: rawFilePath,
756769
noJina: cliNoJina,
770+
useJina: resolveUseJina(cliNoJina, loadConfig()),
771+
jinaApiKey: process.env.JINA_API_KEY,
757772
gate: gateFlag,
758773
renderHtml: renderHtmlFlag,
759774
sharingEnabled,

apps/opencode-plugin/binary-client.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,27 @@ describe("OpenCode binary client", () => {
260260
]);
261261
});
262262

263+
test("includes the command timeout in plugin requests", async () => {
264+
const response = createPluginSuccessResponse({ approved: true });
265+
let inputBody: unknown;
266+
const run: CommandRunner = (_command, _args, input) => {
267+
inputBody = JSON.parse(input ?? "{}");
268+
return { exitCode: 0, stdout: JSON.stringify(response), stderr: "" };
269+
};
270+
271+
await runPluginPlan(
272+
"/bin/plannotator",
273+
{
274+
origin: "opencode",
275+
plan: "# Plan",
276+
},
277+
run,
278+
{ timeoutMs: 12_000 },
279+
);
280+
281+
expect(inputBody).toMatchObject({ timeoutMs: 12_000 });
282+
});
283+
263284
test("turns malformed plugin plan output into a protocol error", async () => {
264285
const result = await runPluginPlan(
265286
"/bin/plannotator",

apps/vscode-extension/src/editor-annotations.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import * as http from "http";
1313
// ── State ──────────────────────────────────────────────────────────
1414

1515
let activeProxyPort: number | null = null;
16+
let activeProxySessionPath = "";
1617
let commentController: vscode.CommentController | null = null;
1718
let annotationDecorationType: vscode.TextEditorDecorationType | null = null;
1819

@@ -24,11 +25,13 @@ const decoratedRanges = new Map<string, vscode.Range[]>();
2425

2526
// ── Public API ─────────────────────────────────────────────────────
2627

27-
export function setActiveProxyPort(port: number | null): void {
28+
export function setActiveProxyPort(port: number | null, sessionPath = ""): void {
2829
activeProxyPort = port;
30+
activeProxySessionPath = /^\/s\/[^/]+$/.test(sessionPath) ? sessionPath : "";
2931
if (port !== null) {
3032
createController();
3133
} else {
34+
activeProxySessionPath = "";
3235
disposeAllThreads();
3336
clearAllDecorations();
3437
if (commentController) {
@@ -300,7 +303,7 @@ function requestProxy(
300303
}
301304

302305
const req = http.request(
303-
{ hostname: "127.0.0.1", port, path: urlPath, method, headers },
306+
{ hostname: "127.0.0.1", port, path: scopedProxyPath(urlPath), method, headers },
304307
(res) => {
305308
let data = "";
306309
res.on("data", (chunk: string) => (data += chunk));
@@ -318,3 +321,10 @@ function requestProxy(
318321
req.end();
319322
});
320323
}
324+
325+
function scopedProxyPath(urlPath: string): string {
326+
if (activeProxySessionPath && urlPath.startsWith("/api/")) {
327+
return `${activeProxySessionPath}${urlPath}`;
328+
}
329+
return urlPath;
330+
}

apps/vscode-extension/src/extension.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,9 @@ export async function activate(context: vscode.ExtensionContext): Promise<void>
6363
},
6464
});
6565

66+
const parsedUrl = new URL(url);
6667
const panel = await panelManager.open(proxy.rewriteUrl(url));
67-
setActiveProxyPort(proxy.port);
68+
setActiveProxyPort(proxy.port, parsedUrl.pathname);
6869

6970
// Auto-close this specific panel when plannotator signals completion
7071
proxy.events.on("close", () => panel.dispose());

packages/server/daemon/client.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,37 @@ describe("DaemonClient", () => {
9595
expect(existsSync(paths.lockPath)).toBe(false);
9696
});
9797

98+
test("waits for accepted shutdown before removing daemon files", async () => {
99+
const baseDir = tempBase();
100+
const paths = getDaemonPaths({ baseDir });
101+
const daemonState = state();
102+
writeDaemonState(daemonState, { baseDir });
103+
writeFileSync(paths.lockPath, "123\n", "utf-8");
104+
let statusCalls = 0;
105+
let stateFileExistedDuringPoll = false;
106+
107+
await cleanupDaemonState(daemonState, {
108+
baseDir,
109+
isAlive: () => true,
110+
fetch: async (input) => {
111+
const url = String(input);
112+
if (url.endsWith("/daemon/shutdown")) return Response.json({ ok: true });
113+
if (url.endsWith("/daemon/status")) {
114+
statusCalls += 1;
115+
stateFileExistedDuringPoll = stateFileExistedDuringPoll || existsSync(paths.statePath);
116+
if (statusCalls === 1) return Response.json({ ...daemonState, ok: true });
117+
throw new Error("gone");
118+
}
119+
throw new Error(`unexpected request: ${url}`);
120+
},
121+
});
122+
123+
expect(statusCalls).toBe(2);
124+
expect(stateFileExistedDuringPoll).toBe(true);
125+
expect(existsSync(paths.statePath)).toBe(false);
126+
expect(existsSync(paths.lockPath)).toBe(false);
127+
});
128+
98129
test("does not signal recorded PID when endpoint shutdown fails", async () => {
99130
const baseDir = tempBase();
100131
const paths = getDaemonPaths({ baseDir });

packages/server/daemon/client.ts

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { readDaemonState, removeDaemonFiles, type DaemonState, type DaemonStateO
1515
export interface DaemonClientOptions extends DaemonStateOptions {
1616
fetch?: typeof fetch;
1717
validateEnvironment?: boolean;
18+
shutdownTimeoutMs?: number;
1819
}
1920

2021
export type DaemonDiscoveryResult =
@@ -92,21 +93,68 @@ function stateBaseUrl(state: unknown): string | undefined {
9293
return typeof baseUrl === "string" ? baseUrl : undefined;
9394
}
9495

96+
function statePid(state: unknown): number | undefined {
97+
const pid = (state as { pid?: unknown } | null)?.pid;
98+
return typeof pid === "number" && Number.isInteger(pid) && pid > 0 ? pid : undefined;
99+
}
100+
101+
export async function waitForDaemonShutdown(
102+
state: unknown,
103+
options: DaemonClientOptions = {},
104+
): Promise<boolean> {
105+
const fetchImpl = options.fetch ?? fetch;
106+
const baseUrl = stateBaseUrl(state);
107+
const pid = statePid(state);
108+
const isAlive = options.isAlive ?? ((targetPid: number) => {
109+
try {
110+
process.kill(targetPid, 0);
111+
return true;
112+
} catch {
113+
return false;
114+
}
115+
});
116+
const deadline = Date.now() + (options.shutdownTimeoutMs ?? 3_000);
117+
118+
while (Date.now() < deadline) {
119+
if (pid && !isAlive(pid)) return true;
120+
if (!baseUrl) return true;
121+
try {
122+
const res = await fetchImpl(`${baseUrl}/daemon/status`);
123+
const status = await res.json().catch(() => null) as { pid?: unknown } | null;
124+
if (!res.ok) return true;
125+
if (pid && status?.pid !== pid) return true;
126+
} catch {
127+
return true;
128+
}
129+
await new Promise((resolve) => setTimeout(resolve, 100));
130+
}
131+
132+
return false;
133+
}
134+
95135
export async function cleanupDaemonState(state: unknown, options: DaemonClientOptions = {}): Promise<void> {
96136
const fetchImpl = options.fetch ?? fetch;
97137
const baseUrl = stateBaseUrl(state);
138+
let shutdownAccepted = false;
98139
if (baseUrl) {
99140
try {
100-
await fetchImpl(`${baseUrl}/daemon/shutdown`, {
141+
const res = await fetchImpl(`${baseUrl}/daemon/shutdown`, {
101142
method: "POST",
102143
headers: { "content-type": "application/json" },
103144
body: "{}",
104145
});
146+
shutdownAccepted = res.ok;
105147
} catch {
106148
// Best effort only. Do not signal the recorded PID here; stale daemon
107149
// state can outlive the process and the PID may now belong to something else.
108150
}
109151
}
152+
if (shutdownAccepted) {
153+
const stopped = await waitForDaemonShutdown(state, options);
154+
if (!stopped) {
155+
throw new Error("Timed out waiting for the existing Plannotator daemon to stop.");
156+
}
157+
}
110158
removeDaemonFiles(options);
111159
}
112160

packages/server/daemon/server.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe("daemon HTTP router", () => {
4848
});
4949

5050
test("reports daemon status with active session count", async () => {
51-
const { handler } = makeHandler();
51+
const { handler, store } = makeHandler();
5252
await handler(new Request("http://127.0.0.1:4321/daemon/sessions", {
5353
method: "POST",
5454
headers: { "content-type": "application/json" },
@@ -60,6 +60,11 @@ describe("daemon HTTP router", () => {
6060
expect(body.endpoint.baseUrl).toBe("http://localhost:4321");
6161
expect(body.activeSessionCount).toBe(1);
6262
expect(body.sessionCount).toBe(1);
63+
store.complete("s1", { approved: true });
64+
const afterComplete = await handler(new Request("http://127.0.0.1:4321/daemon/status"));
65+
const afterCompleteBody = await afterComplete.json();
66+
expect(afterCompleteBody.activeSessionCount).toBe(0);
67+
expect(afterCompleteBody.sessionCount).toBe(1);
6368
});
6469

6570
test("creates and lists sessions", async () => {
@@ -92,6 +97,7 @@ describe("daemon HTTP router", () => {
9297
expect(html).toContain("apiBase=\"/s/s1/api\"");
9398
expect(html).toContain("window.fetch");
9499
expect(html).toContain("window.EventSource");
100+
expect(html).toContain("window.EventSource.OPEN=OriginalEventSource.OPEN");
95101
expect(html.indexOf("window.__PLANNOTATOR_API_BASE__")).toBeGreaterThan(html.indexOf("const literal"));
96102
expect(html.indexOf("window.__PLANNOTATOR_API_BASE__")).toBeLessThan(html.indexOf("<body>"));
97103
});

0 commit comments

Comments
 (0)