feat: v2.6 follow-ups — apps/server close-hook caller + DiffPanel staging hint (3.7)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -4,6 +4,7 @@ import type { Sql } from '../db.js';
|
|||||||
import type { Broker } from '../services/broker.js';
|
import type { Broker } from '../services/broker.js';
|
||||||
import type { Chat, Message } from '../types/api.js';
|
import type { Chat, Message } from '../types/api.js';
|
||||||
import { getModelContext } from '../services/model-context.js';
|
import { getModelContext } from '../services/model-context.js';
|
||||||
|
import { notifyCoderClose } from '../services/coder-notify.js';
|
||||||
|
|
||||||
const CreateBody = z.object({
|
const CreateBody = z.object({
|
||||||
name: z.string().min(1).max(200).optional(),
|
name: z.string().min(1).max(200).optional(),
|
||||||
@@ -167,6 +168,9 @@ export function registerChatRoutes(
|
|||||||
chat_id: id,
|
chat_id: id,
|
||||||
session_id: req.params.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 };
|
return { archived: ids.length, ids };
|
||||||
}
|
}
|
||||||
@@ -208,6 +212,9 @@ export function registerChatRoutes(
|
|||||||
chat_id: row.id,
|
chat_id: row.id,
|
||||||
session_id: row.session_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);
|
reply.code(204);
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
@@ -248,6 +255,9 @@ export function registerChatRoutes(
|
|||||||
chat_id: row.id,
|
chat_id: row.id,
|
||||||
session_id: row.session_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);
|
reply.code(204);
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import type { Config } from '../config.js';
|
|||||||
import type { Broker } from '../services/broker.js';
|
import type { Broker } from '../services/broker.js';
|
||||||
import type { Session, WorktreeRiskReport } from '../types/api.js';
|
import type { Session, WorktreeRiskReport } from '../types/api.js';
|
||||||
import { getSetting } from './settings.js';
|
import { getSetting } from './settings.js';
|
||||||
|
import { notifyCoderClose } from '../services/coder-notify.js';
|
||||||
|
|
||||||
const CreateBody = z.object({
|
const CreateBody = z.object({
|
||||||
name: z.string().min(1).max(200).optional(),
|
name: z.string().min(1).max(200).optional(),
|
||||||
@@ -513,6 +514,10 @@ export function registerSessionRoutes(
|
|||||||
}
|
}
|
||||||
const project_id = deleted[0]!.project_id;
|
const project_id = deleted[0]!.project_id;
|
||||||
broker.publishUserFrame('default', { type: 'session_deleted', session_id: id, 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);
|
reply.code(204);
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|||||||
67
apps/server/src/services/__tests__/coder-notify.test.ts
Normal file
67
apps/server/src/services/__tests__/coder-notify.test.ts
Normal file
@@ -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);
|
||||||
|
});
|
||||||
|
});
|
||||||
64
apps/server/src/services/coder-notify.ts
Normal file
64
apps/server/src/services/coder-notify.ts
Normal file
@@ -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<FastifyBaseLogger, 'debug'>,
|
||||||
|
fetcher: typeof fetch = fetch,
|
||||||
|
): Promise<boolean> {
|
||||||
|
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;
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -388,12 +388,14 @@ function usePendingChanges(sessionId: string) {
|
|||||||
function DiffPanel({
|
function DiffPanel({
|
||||||
changes,
|
changes,
|
||||||
loading,
|
loading,
|
||||||
|
currentProvider,
|
||||||
onRefresh,
|
onRefresh,
|
||||||
onApprove,
|
onApprove,
|
||||||
onReject,
|
onReject,
|
||||||
}: {
|
}: {
|
||||||
changes: PendingChange[];
|
changes: PendingChange[];
|
||||||
loading: boolean;
|
loading: boolean;
|
||||||
|
currentProvider: string;
|
||||||
onRefresh: () => void;
|
onRefresh: () => void;
|
||||||
onApprove: (id: string) => void;
|
onApprove: (id: string) => void;
|
||||||
onReject: (id: string) => void;
|
onReject: (id: string) => void;
|
||||||
@@ -409,6 +411,29 @@ function DiffPanel({
|
|||||||
? `Changes from ${distinctAgents.map((a) => providerLabel(a)).join(', ')}`
|
? `Changes from ${distinctAgents.map((a) => providerLabel(a)).join(', ')}`
|
||||||
: null;
|
: 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 (
|
return (
|
||||||
<div className="flex flex-col h-full border-t border-border">
|
<div className="flex flex-col h-full border-t border-border">
|
||||||
<div className="flex items-center justify-between px-3 py-1.5 border-b border-border bg-muted/30">
|
<div className="flex items-center justify-between px-3 py-1.5 border-b border-border bg-muted/30">
|
||||||
@@ -430,6 +455,14 @@ function DiffPanel({
|
|||||||
{mixedNote}
|
{mixedNote}
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
{boundaryHint && (
|
||||||
|
<div
|
||||||
|
className="px-3 py-1 border-b border-border bg-muted/10 text-xs text-muted-foreground"
|
||||||
|
title={boundaryHint}
|
||||||
|
>
|
||||||
|
{boundaryHint}
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
<div className="flex-1 overflow-y-auto">
|
<div className="flex-1 overflow-y-auto">
|
||||||
{pending.length === 0 ? (
|
{pending.length === 0 ? (
|
||||||
<div className="flex items-center justify-center h-full text-sm text-muted-foreground">
|
<div className="flex items-center justify-center h-full text-sm text-muted-foreground">
|
||||||
@@ -914,6 +947,7 @@ export function CoderPane({
|
|||||||
<DiffPanel
|
<DiffPanel
|
||||||
changes={changes}
|
changes={changes}
|
||||||
loading={loading}
|
loading={loading}
|
||||||
|
currentProvider={agentConfig.provider}
|
||||||
onRefresh={refresh}
|
onRefresh={refresh}
|
||||||
onApprove={approve}
|
onApprove={approve}
|
||||||
onReject={reject}
|
onReject={reject}
|
||||||
|
|||||||
Reference in New Issue
Block a user