From 2dfbef4c413161ca07c3efa9cf2964a92060fcbb Mon Sep 17 00:00:00 2001 From: indifferentketchup Date: Mon, 1 Jun 2026 02:35:11 +0000 Subject: [PATCH] =?UTF-8?q?feat:=20v2.6=20follow-ups=20=E2=80=94=20apps/se?= =?UTF-8?q?rver=20close-hook=20caller=20+=20DiffPanel=20staging=20hint=20(?= =?UTF-8?q?3.7)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit apps/server fire-and-forgets BooCoder's Phase-3 close hooks (new coder-notify.ts, reuses BOOCODER_URL, never-rejects) on session-delete + chat archive/archive-all/delete, so warm backends + worktrees tear down immediately (idle-evict/reaper was the backstop). 3.7: BooCoder DiffPanel shows a muted one-liner when the selected provider can't see another agent's unapplied worktree edits (pure derivation from per-change agent + current provider, no new state). 6 new server tests (coder-notify); 537 server tests pass; web+server tsc/build clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/server/src/routes/chats.ts | 10 +++ apps/server/src/routes/sessions.ts | 5 ++ .../services/__tests__/coder-notify.test.ts | 67 +++++++++++++++++++ apps/server/src/services/coder-notify.ts | 64 ++++++++++++++++++ apps/web/src/components/panes/CoderPane.tsx | 34 ++++++++++ 5 files changed, 180 insertions(+) create mode 100644 apps/server/src/services/__tests__/coder-notify.test.ts create mode 100644 apps/server/src/services/coder-notify.ts diff --git a/apps/server/src/routes/chats.ts b/apps/server/src/routes/chats.ts index ad6bc5c..c39d210 100644 --- a/apps/server/src/routes/chats.ts +++ b/apps/server/src/routes/chats.ts @@ -4,6 +4,7 @@ import type { Sql } from '../db.js'; import type { Broker } from '../services/broker.js'; import type { Chat, Message } from '../types/api.js'; import { getModelContext } from '../services/model-context.js'; +import { notifyCoderClose } from '../services/coder-notify.js'; const CreateBody = z.object({ name: z.string().min(1).max(200).optional(), @@ -167,6 +168,9 @@ export function registerChatRoutes( chat_id: id, session_id: req.params.id, }); + // Fire-and-forget per archived chat: tear down its warm agent backends + // on the coder. Best-effort — never blocks/fails the bulk archive. + void notifyCoderClose('chat', id, req.log); } return { archived: ids.length, ids }; } @@ -208,6 +212,9 @@ export function registerChatRoutes( chat_id: row.id, session_id: row.session_id, }); + // Fire-and-forget: tear down this chat's warm agent backends + (last-chat) + // worktree on the coder. Best-effort — never blocks/fails the archive. + void notifyCoderClose('chat', row.id, req.log); reply.code(204); return null; } @@ -248,6 +255,9 @@ export function registerChatRoutes( chat_id: row.id, session_id: row.session_id, }); + // Fire-and-forget: tear down this chat's warm agent backends + (last-chat) + // worktree on the coder. Best-effort — never blocks/fails the delete. + void notifyCoderClose('chat', row.id, req.log); reply.code(204); return null; } diff --git a/apps/server/src/routes/sessions.ts b/apps/server/src/routes/sessions.ts index dbc24f3..d7a2e4b 100644 --- a/apps/server/src/routes/sessions.ts +++ b/apps/server/src/routes/sessions.ts @@ -5,6 +5,7 @@ import type { Config } from '../config.js'; import type { Broker } from '../services/broker.js'; import type { Session, WorktreeRiskReport } from '../types/api.js'; import { getSetting } from './settings.js'; +import { notifyCoderClose } from '../services/coder-notify.js'; const CreateBody = z.object({ name: z.string().min(1).max(200).optional(), @@ -513,6 +514,10 @@ export function registerSessionRoutes( } const project_id = deleted[0]!.project_id; broker.publishUserFrame('default', { type: 'session_deleted', session_id: id, project_id }); + // Fire-and-forget: ask BooCoder to tear down this session's warm agent + // backends + worktree immediately. Best-effort — never blocks/fails the + // delete; the coder's idle-evict + orphan reaper backstop a missed call. + void notifyCoderClose('session', id, req.log); reply.code(204); return null; } diff --git a/apps/server/src/services/__tests__/coder-notify.test.ts b/apps/server/src/services/__tests__/coder-notify.test.ts new file mode 100644 index 0000000..acd6aa2 --- /dev/null +++ b/apps/server/src/services/__tests__/coder-notify.test.ts @@ -0,0 +1,67 @@ +// v2.6.10 Phase 3 (server wiring) — notifyCoderClose fire-and-forget helper. +// +// The guarantee under test: the helper NEVER throws (so it can't break the +// user's delete/archive path), targets the correct coder URL shape, and folds +// every failure mode (non-2xx, network error) into a `false` result. + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { notifyCoderClose } from '../coder-notify.js'; + +const ORIGINAL_BOOCODER_URL = process.env.BOOCODER_URL; + +describe('notifyCoderClose', () => { + beforeEach(() => { + delete process.env.BOOCODER_URL; + }); + afterEach(() => { + if (ORIGINAL_BOOCODER_URL === undefined) delete process.env.BOOCODER_URL; + else process.env.BOOCODER_URL = ORIGINAL_BOOCODER_URL; + }); + + it('POSTs the chat close hook at the default coder origin and resolves true on 2xx', async () => { + const fetcher = vi.fn().mockResolvedValue(new Response(null, { status: 200 })); + const ok = await notifyCoderClose('chat', 'chat-123', undefined, fetcher as unknown as typeof fetch); + expect(ok).toBe(true); + expect(fetcher).toHaveBeenCalledTimes(1); + const [url, init] = fetcher.mock.calls[0]!; + expect(url).toBe('http://boocoder:3000/api/chats/chat-123/close'); + expect(init).toEqual({ method: 'POST' }); + }); + + it('POSTs the session close hook with the sessions segment', async () => { + const fetcher = vi.fn().mockResolvedValue(new Response(null, { status: 200 })); + const ok = await notifyCoderClose('session', 'sess-abc', undefined, fetcher as unknown as typeof fetch); + expect(ok).toBe(true); + expect(fetcher.mock.calls[0]![0]).toBe('http://boocoder:3000/api/sessions/sess-abc/close'); + }); + + it('honors BOOCODER_URL for the origin', async () => { + process.env.BOOCODER_URL = 'http://100.114.205.53:9502'; + const fetcher = vi.fn().mockResolvedValue(new Response(null, { status: 200 })); + await notifyCoderClose('chat', 'c1', undefined, fetcher as unknown as typeof fetch); + expect(fetcher.mock.calls[0]![0]).toBe('http://100.114.205.53:9502/api/chats/c1/close'); + }); + + it('resolves false on a non-2xx response (does not throw)', async () => { + const fetcher = vi.fn().mockResolvedValue(new Response(null, { status: 500 })); + const log = { debug: vi.fn() }; + const ok = await notifyCoderClose('chat', 'c1', log, fetcher as unknown as typeof fetch); + expect(ok).toBe(false); + expect(log.debug).toHaveBeenCalledTimes(1); + }); + + it('resolves false on a network error (coder unreachable) — never rejects', async () => { + const fetcher = vi.fn().mockRejectedValue(new Error('ECONNREFUSED')); + const log = { debug: vi.fn() }; + const ok = await notifyCoderClose('session', 's1', log, fetcher as unknown as typeof fetch); + expect(ok).toBe(false); + expect(log.debug).toHaveBeenCalledTimes(1); + }); + + it('does not require a logger', async () => { + const fetcher = vi.fn().mockRejectedValue(new Error('boom')); + await expect( + notifyCoderClose('chat', 'c1', undefined, fetcher as unknown as typeof fetch), + ).resolves.toBe(false); + }); +}); diff --git a/apps/server/src/services/coder-notify.ts b/apps/server/src/services/coder-notify.ts new file mode 100644 index 0000000..ac7f12e --- /dev/null +++ b/apps/server/src/services/coder-notify.ts @@ -0,0 +1,64 @@ +// v2.6.10 Phase 3 (server wiring) — fire-and-forget BooCoder close hooks. +// +// BooCoder (apps/coder, host systemd) added close hooks in +// apps/coder/src/routes/lifecycle.ts: +// POST /api/chats/:chatId/close — evict the chat's warm (chat,agent) +// backends, close its opencode session, +// mark agent_sessions closed, and remove +// the shared worktree on the last chat. +// POST /api/sessions/:sessionId/close — loop the chat-close path for every +// chat in the session. +// +// apps/server (Docker) can't see the host worktree dirs or reach the warm agent +// processes, so — exactly like the existing `worktree-risk` guard in +// routes/sessions.ts — it signals the coder over HTTP and the coder does the +// real teardown. This call is BEST-EFFORT: the coder's idle-pool eviction and +// the orphan-worktree reaper backstop a missed/failed call. It MUST NEVER block +// or fail the user's delete/archive — hence fire-and-forget with a swallowed +// catch. We do not await the returned promise at the call sites. + +import type { FastifyBaseLogger } from 'fastify'; + +export type CoderCloseKind = 'chat' | 'session'; + +function coderOrigin(): string { + // Same env + default as routes/sessions.ts' worktree-risk fetch. + return process.env.BOOCODER_URL ?? 'http://boocoder:3000'; +} + +/** + * Fire-and-forget POST to the BooCoder close hook for a chat or session. + * + * Resolves to `true` if the coder acknowledged (HTTP 2xx), `false` otherwise + * (non-2xx or network error). Callers SHOULD NOT await this — invoke it and + * move on. The returned promise never rejects: every failure path is caught, + * logged at debug, and folded into a `false` result so an unreachable or + * erroring coder can't surface to the user's delete/archive request. + */ +export async function notifyCoderClose( + kind: CoderCloseKind, + id: string, + log?: Pick, + fetcher: typeof fetch = fetch, +): Promise { + const segment = kind === 'chat' ? 'chats' : 'sessions'; + const url = `${coderOrigin()}/api/${segment}/${id}/close`; + try { + const res = await fetcher(url, { method: 'POST' }); + if (!res.ok) { + log?.debug( + { kind, id, status: res.status }, + 'coder close hook returned non-2xx (best-effort; reaper backstops)', + ); + return false; + } + log?.debug({ kind, id }, 'coder close hook acknowledged'); + return true; + } catch (err) { + log?.debug( + { kind, id, err: err instanceof Error ? err.message : String(err) }, + 'coder close hook unreachable (best-effort; reaper backstops)', + ); + return false; + } +} diff --git a/apps/web/src/components/panes/CoderPane.tsx b/apps/web/src/components/panes/CoderPane.tsx index 266107c..dfdff48 100644 --- a/apps/web/src/components/panes/CoderPane.tsx +++ b/apps/web/src/components/panes/CoderPane.tsx @@ -388,12 +388,14 @@ function usePendingChanges(sessionId: string) { function DiffPanel({ changes, loading, + currentProvider, onRefresh, onApprove, onReject, }: { changes: PendingChange[]; loading: boolean; + currentProvider: string; onRefresh: () => void; onApprove: (id: string) => void; onReject: (id: string) => void; @@ -409,6 +411,29 @@ function DiffPanel({ ? `Changes from ${distinctAgents.map((a) => providerLabel(a)).join(', ')}` : null; + // v2.6 §9c: staging-boundary caveat. External agents (opencode/goose/qwen/ + // claude) edit *inside their worktree*; native boocode reads/writes the + // *project root* via pending_changes. Unapplied edits don't cross that + // boundary. When the currently-selected provider can't see another side's + // staged-but-unapplied edits, surface a muted one-liner. agent===null + // (manual) is boundary-neutral. Pure derivation — no new state/fetch. + const isNativeProvider = currentProvider === 'boocode'; + const boundaryHint = (() => { + if (isNativeProvider) { + // Native boocode is selected: it won't see external-worktree edits. + const external = distinctAgents.filter((a) => a !== null && a !== 'boocode'); + if (external.length === 0) return null; + const who = + external.length === 1 + ? providerLabel(external[0]!) + : external.map((a) => providerLabel(a)).join(', '); + return `${who}'s edits live in its worktree — BooCode won't see them until applied.`; + } + // An external agent is selected: it won't see boocode's project-root edits. + if (!distinctAgents.includes('boocode')) return null; + return `BooCode's edits live in the project root — ${providerLabel(currentProvider)} won't see them until applied.`; + })(); + return (
@@ -430,6 +455,14 @@ function DiffPanel({ {mixedNote}
)} + {boundaryHint && ( +
+ {boundaryHint} +
+ )}
{pending.length === 0 ? (
@@ -914,6 +947,7 @@ export function CoderPane({