fix(web): harden typed SPA chat lifecycle
Co-Authored-By: Claude Haiku 4.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Haiku 4.5
parent
b2e005f2b4
commit
caebf9ef70
@@ -1,16 +1,27 @@
|
||||
import { useEffect, useReducer, useRef } from 'react';
|
||||
import { destroySocket, getSocket } from '@/lib/socket';
|
||||
import {
|
||||
MAX_COMMAND_RESULTS,
|
||||
MAX_EXECUTED_APPROVAL_IDS,
|
||||
MAX_MANIFEST_ITEMS,
|
||||
MAX_MESSAGES,
|
||||
MAX_STREAM_CHARS,
|
||||
MAX_TOOLS,
|
||||
} from './limits';
|
||||
import { asConversationId, asFiniteNumber, asString, isRecord } from './runtime-guards';
|
||||
import type {
|
||||
AgentEndPayload,
|
||||
AgentStartPayload,
|
||||
AgentTextPayload,
|
||||
AgentThinkingPayload,
|
||||
CommandDef,
|
||||
CommandManifest,
|
||||
CommandManifestPayload,
|
||||
ErrorPayload,
|
||||
MessageAckPayload,
|
||||
SessionInfoPayload,
|
||||
SessionUsagePayload,
|
||||
SkillCommandDef,
|
||||
SlashCommandApprovalResultPayload,
|
||||
SlashCommandResultPayload,
|
||||
SystemReloadPayload,
|
||||
@@ -21,7 +32,113 @@ import type {
|
||||
export interface ToolCallState {
|
||||
toolCallId: string;
|
||||
toolName: string;
|
||||
status: 'running' | 'success' | 'error';
|
||||
status: 'running' | 'success' | 'error' | 'anomaly';
|
||||
}
|
||||
|
||||
/** Appends `addition` to `existing`, keeping at most `max` characters by
|
||||
* dropping the oldest (leading) characters once the cap is exceeded. */
|
||||
function capAppendString(existing: string, addition: string, max: number): string {
|
||||
const next = existing + addition;
|
||||
return next.length > max ? next.slice(next.length - max) : next;
|
||||
}
|
||||
|
||||
/** Pushes `item` onto `arr`, dropping the oldest entries once `max` is exceeded. */
|
||||
function capPush<T>(arr: T[], item: T, max: number): T[] {
|
||||
const next = [...arr, item];
|
||||
return next.length > max ? next.slice(next.length - max) : next;
|
||||
}
|
||||
|
||||
/** A valid `toolCallId` from the wire is a non-empty string — a malformed or
|
||||
* absent one (non-string, or empty string) must never be treated as if it
|
||||
* named a real tool call. */
|
||||
function isValidToolCallId(value: unknown): value is string {
|
||||
return typeof value === 'string' && value.length > 0;
|
||||
}
|
||||
|
||||
/** Shown when a scoped `error`/`agent:end` cannot be attributed to any
|
||||
* conversation (a malformed/missing conversationId) while a send is still
|
||||
* pending and no conversation has ever been established. There is no valid
|
||||
* identity left to recover the turn under, so it is surfaced as a terminal
|
||||
* startup failure instead of leaving `sending`/`pendingSend` stuck forever. */
|
||||
const CONVERSATION_START_FAILURE = 'Unable to start this conversation. Please try again.';
|
||||
|
||||
/** True only for the narrow case a malformed-conversationId `error`/`agent:end`
|
||||
* must be treated as a terminal startup failure: no conversation has ever been
|
||||
* established yet, and a send is still pending one. Once a conversation is
|
||||
* active, or when no send is pending, an unattributable event stays a no-op —
|
||||
* dropping it (rather than guessing which turn it belongs to) is what lets a
|
||||
* later genuinely valid event still recover the turn. */
|
||||
function isUnrecoverableStartupFailure(state: ChatConnectionState): boolean {
|
||||
return state.conversationId === null && state.pendingSend;
|
||||
}
|
||||
|
||||
/** Deterministic, unique-per-event fallback id for a malformed `toolCallId`.
|
||||
* Sourced from a monotonically increasing counter carried in state (`toolSeq`)
|
||||
* rather than `tools.length`, so it stays collision-free even once `tools` is
|
||||
* capped at MAX_TOOLS and its length stops changing. */
|
||||
function nextToolFallbackId(toolSeq: number): string {
|
||||
return `unknown-tool-call-${toolSeq}`;
|
||||
}
|
||||
|
||||
/** Truncates a server-provided collection to at most `max` entries. `arr` is
|
||||
* `unknown` because it comes straight from a raw socket payload — a
|
||||
* malformed/compromised gateway can send `null` or any non-array value here,
|
||||
* which must normalize to an empty list rather than throw. */
|
||||
function capList<T>(arr: unknown, max: number): T[] {
|
||||
if (!Array.isArray(arr)) return [];
|
||||
return (arr.length > max ? arr.slice(0, max) : arr) as T[];
|
||||
}
|
||||
|
||||
/** Normalizes a raw message:ack payload before it is stored — `messageId` is
|
||||
* given a stable, visible "unknown" fallback rather than storing a raw
|
||||
* non-string value (which would either render as "[object Object]" or,
|
||||
* with an unguarded render site, throw). */
|
||||
function sanitizeAck(payload: MessageAckPayload, conversationId: string): MessageAckPayload {
|
||||
return { conversationId, messageId: asString(payload.messageId, 'unknown') };
|
||||
}
|
||||
|
||||
/** Normalizes a raw command:approval payload before it is stored. A command
|
||||
* string matching `pendingApproval.command` alone is not sufficient proof of
|
||||
* a genuine, executable approval: `success` must be the literal boolean
|
||||
* `true`, and a successful approval requires a non-empty string
|
||||
* `approvalId`. Anything else is normalized to a denial (`success: false`,
|
||||
* no `approvalId`) so the pending request resolves and the approve lock is
|
||||
* released instead of leaving the UI hung on a response it can never trust
|
||||
* enough to enable "Run approved command" for. `expiresAt`/`message` are
|
||||
* retained only when they are strings (message is never rendered as-is). */
|
||||
function sanitizeApproval(
|
||||
payload: SlashCommandApprovalResultPayload,
|
||||
conversationId: string,
|
||||
command: string,
|
||||
): SlashCommandApprovalResultPayload {
|
||||
const approvalId =
|
||||
typeof payload.approvalId === 'string' && payload.approvalId.length > 0
|
||||
? payload.approvalId
|
||||
: undefined;
|
||||
const success = payload.success === true && approvalId !== undefined;
|
||||
return {
|
||||
conversationId,
|
||||
command,
|
||||
success,
|
||||
approvalId: success ? approvalId : undefined,
|
||||
expiresAt: typeof payload.expiresAt === 'string' ? payload.expiresAt : undefined,
|
||||
message: typeof payload.message === 'string' ? payload.message : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
/** Normalizes a raw command:result payload before it is stored — `success`
|
||||
* is coerced to a literal boolean so a truthy non-boolean value (e.g. an
|
||||
* object) can never be displayed as a successful result. */
|
||||
function sanitizeCommandResult(
|
||||
payload: SlashCommandResultPayload,
|
||||
conversationId: string,
|
||||
): SlashCommandResultPayload {
|
||||
return {
|
||||
conversationId,
|
||||
command: asString(payload.command, 'unknown'),
|
||||
success: payload.success === true,
|
||||
message: typeof payload.message === 'string' ? payload.message : undefined,
|
||||
};
|
||||
}
|
||||
|
||||
export interface ChatTranscriptMessage {
|
||||
@@ -42,6 +159,10 @@ export interface ChatConnectionState {
|
||||
* first scoped server event naming a conversation may establish it (the gateway
|
||||
* does not guarantee message:ack is the first event for a new conversation). */
|
||||
pendingSend: boolean;
|
||||
/** True from the moment a message is sent until its turn ends (agent:end),
|
||||
* errors, or the connection drops — guards against a second turn starting
|
||||
* while one is already in flight. */
|
||||
sending: boolean;
|
||||
ack: MessageAckPayload | null;
|
||||
streaming: boolean;
|
||||
text: string;
|
||||
@@ -58,6 +179,16 @@ export interface ChatConnectionState {
|
||||
systemReload: SystemReloadPayload | null;
|
||||
error: string | null;
|
||||
messages: ChatTranscriptMessage[];
|
||||
/** Monotonically increasing counter used to mint transcript message ids —
|
||||
* never reset while retained messages remain, so ids stay unique across the
|
||||
* MAX_MESSAGES cap boundary (unlike a `messages.length`-derived id, which
|
||||
* plateaus once the array is capped). */
|
||||
messageSeq: number;
|
||||
/** Monotonically increasing counter used to mint fallback tool-call ids for
|
||||
* malformed (non-string/empty) `toolCallId`s — never reset while retained
|
||||
* tools remain, so fallback ids stay unique across the MAX_TOOLS cap
|
||||
* boundary. */
|
||||
toolSeq: number;
|
||||
}
|
||||
|
||||
export interface ChatConnectionActions {
|
||||
@@ -77,6 +208,7 @@ export interface ChatConnectionValue {
|
||||
const initialState: ChatConnectionState = {
|
||||
conversationId: null,
|
||||
pendingSend: false,
|
||||
sending: false,
|
||||
ack: null,
|
||||
streaming: false,
|
||||
text: '',
|
||||
@@ -92,6 +224,8 @@ const initialState: ChatConnectionState = {
|
||||
systemReload: null,
|
||||
error: null,
|
||||
messages: [],
|
||||
messageSeq: 0,
|
||||
toolSeq: 0,
|
||||
};
|
||||
|
||||
type Action =
|
||||
@@ -110,7 +244,8 @@ type Action =
|
||||
| { type: 'server/error'; payload: ErrorPayload }
|
||||
| { type: 'local/send'; content: string }
|
||||
| { type: 'local/approve-request'; command: string; args?: string }
|
||||
| { type: 'local/consume-approval' };
|
||||
| { type: 'local/consume-approval' }
|
||||
| { type: 'local/disconnect' };
|
||||
|
||||
/**
|
||||
* Resolves whether a scoped server event (one carrying a conversationId) belongs to
|
||||
@@ -119,11 +254,22 @@ type Action =
|
||||
* failure, can both arrive first) — so while a message is pending and no conversation
|
||||
* is active yet, the first scoped event names the conversation instead of being
|
||||
* dropped. Once a conversation is active, only its own events pass.
|
||||
*
|
||||
* `rawConversationId` is `unknown`, not `string` — every payload is runtime-untrusted
|
||||
* regardless of its compile-time contract, and this is the single place that may
|
||||
* establish `state.conversationId`. A malformed value (non-string/empty) is rejected
|
||||
* via the central `asConversationId` guard and never adopted: the event is treated as
|
||||
* inactive/dropped, leaving state (including `pendingSend`) untouched so a later
|
||||
* genuinely valid event can still establish or recover the turn.
|
||||
*/
|
||||
function resolveScopedConversation(
|
||||
state: ChatConnectionState,
|
||||
conversationId: string,
|
||||
rawConversationId: unknown,
|
||||
): { active: true; state: ChatConnectionState } | { active: false; state: null } {
|
||||
const conversationId = asConversationId(rawConversationId);
|
||||
if (conversationId === null) {
|
||||
return { active: false, state: null };
|
||||
}
|
||||
if (state.conversationId === conversationId) {
|
||||
return { active: true, state };
|
||||
}
|
||||
@@ -133,20 +279,42 @@ function resolveScopedConversation(
|
||||
return { active: false, state: null };
|
||||
}
|
||||
|
||||
type ServerAction = Extract<Action, { type: `server/${string}` }>;
|
||||
|
||||
function isServerAction(action: Action): action is ServerAction {
|
||||
return action.type.startsWith('server/');
|
||||
}
|
||||
|
||||
function reduce(state: ChatConnectionState, action: Action): ChatConnectionState {
|
||||
// A malformed packet (the whole payload is null/undefined/a primitive, not
|
||||
// an object) is ignored outright rather than crashing the reducer on the
|
||||
// first field dereference in the case below — every server/* action shape
|
||||
// declares a `payload` field, so this guard covers all of them uniformly.
|
||||
if (isServerAction(action) && !isRecord(action.payload)) {
|
||||
return state;
|
||||
}
|
||||
|
||||
switch (action.type) {
|
||||
case 'server/message:ack': {
|
||||
const { payload } = action;
|
||||
if (state.conversationId === null) {
|
||||
const conversationId = asConversationId(payload.conversationId);
|
||||
// A malformed first frame (non-string/empty conversationId) is ignored
|
||||
// outright rather than adopted — adopting it would permanently
|
||||
// desynchronize every later scoped event's strict-equality check
|
||||
// against a value that can never again match. Leaving state untouched
|
||||
// (conversationId stays null) lets a later genuinely valid ack/start
|
||||
// still establish the turn.
|
||||
if (conversationId === null) return state;
|
||||
return {
|
||||
...state,
|
||||
conversationId: payload.conversationId,
|
||||
conversationId,
|
||||
pendingSend: false,
|
||||
ack: payload,
|
||||
ack: sanitizeAck(payload, conversationId),
|
||||
};
|
||||
}
|
||||
if (payload.conversationId !== state.conversationId) return state;
|
||||
return { ...state, ack: payload };
|
||||
return { ...state, ack: sanitizeAck(payload, state.conversationId) };
|
||||
}
|
||||
|
||||
case 'server/agent:start': {
|
||||
@@ -158,65 +326,124 @@ function reduce(state: ChatConnectionState, action: Action): ChatConnectionState
|
||||
case 'server/agent:text': {
|
||||
const resolved = resolveScopedConversation(state, action.payload.conversationId);
|
||||
if (!resolved.active) return state;
|
||||
return { ...resolved.state, text: resolved.state.text + action.payload.text };
|
||||
return {
|
||||
...resolved.state,
|
||||
text: capAppendString(resolved.state.text, asString(action.payload.text), MAX_STREAM_CHARS),
|
||||
};
|
||||
}
|
||||
|
||||
case 'server/agent:thinking': {
|
||||
const resolved = resolveScopedConversation(state, action.payload.conversationId);
|
||||
if (!resolved.active) return state;
|
||||
return { ...resolved.state, thinking: resolved.state.thinking + action.payload.text };
|
||||
return {
|
||||
...resolved.state,
|
||||
thinking: capAppendString(
|
||||
resolved.state.thinking,
|
||||
asString(action.payload.text),
|
||||
MAX_STREAM_CHARS,
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
case 'server/agent:tool:start': {
|
||||
const { payload } = action;
|
||||
const resolved = resolveScopedConversation(state, payload.conversationId);
|
||||
if (!resolved.active) return state;
|
||||
const valid = isValidToolCallId(payload.toolCallId);
|
||||
const toolSeq = resolved.state.toolSeq;
|
||||
const tool: ToolCallState = {
|
||||
toolCallId: payload.toolCallId,
|
||||
toolName: payload.toolName,
|
||||
// Each malformed toolCallId gets its own fresh fallback id (never the
|
||||
// fixed 'unknown-tool-call' constant) so distinct malformed starts
|
||||
// never collide with each other.
|
||||
toolCallId: valid ? payload.toolCallId : nextToolFallbackId(toolSeq),
|
||||
toolName: asString(payload.toolName, 'Unknown tool'),
|
||||
status: 'running',
|
||||
};
|
||||
return { ...resolved.state, tools: [...resolved.state.tools, tool] };
|
||||
return {
|
||||
...resolved.state,
|
||||
tools: capPush(resolved.state.tools, tool, MAX_TOOLS),
|
||||
toolSeq: valid ? toolSeq : toolSeq + 1,
|
||||
};
|
||||
}
|
||||
|
||||
case 'server/agent:tool:end': {
|
||||
const { payload } = action;
|
||||
const resolved = resolveScopedConversation(state, payload.conversationId);
|
||||
if (!resolved.active) return state;
|
||||
const valid = isValidToolCallId(payload.toolCallId);
|
||||
const toolSeq = resolved.state.toolSeq;
|
||||
const toolCallId = valid ? payload.toolCallId : nextToolFallbackId(toolSeq);
|
||||
const toolName = asString(payload.toolName, 'Unknown tool');
|
||||
const nextToolSeq = valid ? toolSeq : toolSeq + 1;
|
||||
// A fresh fallback id (malformed toolCallId) can never match an
|
||||
// existing entry, so this only ever finds a genuine prior tool:start.
|
||||
const matchIndex = resolved.state.tools.findIndex((tool) => tool.toolCallId === toolCallId);
|
||||
if (matchIndex === -1) {
|
||||
// A tool:end for an ID we never saw a tool:start for is a protocol
|
||||
// anomaly, not a no-op — surface it visibly instead of silently
|
||||
// dropping it.
|
||||
const anomaly: ToolCallState = { toolCallId, toolName, status: 'anomaly' };
|
||||
return {
|
||||
...resolved.state,
|
||||
tools: capPush(resolved.state.tools, anomaly, MAX_TOOLS),
|
||||
toolSeq: nextToolSeq,
|
||||
};
|
||||
}
|
||||
// Update at most the first matching entry — if `toolCallId` is
|
||||
// (unexpectedly) shared by more than one retained tool, updating every
|
||||
// match would cross-contaminate unrelated statuses (and could silently
|
||||
// overwrite a distinct entry's 'anomaly' status).
|
||||
return {
|
||||
...resolved.state,
|
||||
tools: resolved.state.tools.map((tool) =>
|
||||
tool.toolCallId === payload.toolCallId
|
||||
? { ...tool, status: payload.isError ? 'error' : 'success' }
|
||||
: tool,
|
||||
tools: resolved.state.tools.map((tool, index) =>
|
||||
index === matchIndex ? { ...tool, status: payload.isError ? 'error' : 'success' } : tool,
|
||||
),
|
||||
toolSeq: nextToolSeq,
|
||||
};
|
||||
}
|
||||
|
||||
case 'server/agent:end': {
|
||||
const { payload } = action;
|
||||
const resolved = resolveScopedConversation(state, payload.conversationId);
|
||||
if (!resolved.active) return state;
|
||||
if (!resolved.active) {
|
||||
if (isUnrecoverableStartupFailure(state)) {
|
||||
return {
|
||||
...state,
|
||||
pendingSend: false,
|
||||
sending: false,
|
||||
streaming: false,
|
||||
approvalRequestPending: false,
|
||||
error: CONVERSATION_START_FAILURE,
|
||||
};
|
||||
}
|
||||
return state;
|
||||
}
|
||||
const next = resolved.state;
|
||||
const hasContent = next.text.length > 0 || next.thinking.length > 0;
|
||||
const messages = hasContent
|
||||
? [
|
||||
...next.messages,
|
||||
? capPush(
|
||||
next.messages,
|
||||
{
|
||||
id: `assistant-${payload.conversationId}-${next.messages.length}`,
|
||||
// Sourced from the reducer-owned `messageSeq` counter, not
|
||||
// `messages.length` — the latter plateaus once MAX_MESSAGES is
|
||||
// reached under a flood, producing duplicate ids/React keys.
|
||||
id: `assistant-${payload.conversationId}-${next.messageSeq}`,
|
||||
role: 'assistant' as const,
|
||||
text: next.text,
|
||||
thinking: next.thinking || undefined,
|
||||
},
|
||||
]
|
||||
MAX_MESSAGES,
|
||||
)
|
||||
: next.messages;
|
||||
return {
|
||||
...next,
|
||||
streaming: false,
|
||||
sending: false,
|
||||
text: '',
|
||||
thinking: '',
|
||||
usage: payload.usage ?? next.usage,
|
||||
messages,
|
||||
messageSeq: hasContent ? next.messageSeq + 1 : next.messageSeq,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -227,14 +454,37 @@ function reduce(state: ChatConnectionState, action: Action): ChatConnectionState
|
||||
}
|
||||
|
||||
case 'server/commands:manifest': {
|
||||
return { ...state, manifest: action.payload.manifest };
|
||||
// `manifest` itself, and its `commands`/`skills` fields, are raw
|
||||
// socket payload values — any of them can be null/non-object at
|
||||
// runtime regardless of the compile-time contract.
|
||||
const manifest: Record<string, unknown> = isRecord(action.payload.manifest)
|
||||
? action.payload.manifest
|
||||
: {};
|
||||
return {
|
||||
...state,
|
||||
manifest: {
|
||||
commands: capList<CommandDef>(manifest.commands, MAX_MANIFEST_ITEMS),
|
||||
skills: capList<SkillCommandDef>(manifest.skills, MAX_MANIFEST_ITEMS),
|
||||
version: asFiniteNumber(manifest.version, state.manifest?.version ?? 0),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
case 'server/command:result': {
|
||||
const { payload } = action;
|
||||
const resolved = resolveScopedConversation(state, payload.conversationId);
|
||||
if (!resolved.active) return state;
|
||||
return { ...resolved.state, commandResults: [...resolved.state.commandResults, payload] };
|
||||
const next = resolved.state;
|
||||
const conversationId = next.conversationId;
|
||||
if (conversationId === null) return next;
|
||||
return {
|
||||
...next,
|
||||
commandResults: capPush(
|
||||
next.commandResults,
|
||||
sanitizeCommandResult(payload, conversationId),
|
||||
MAX_COMMAND_RESULTS,
|
||||
),
|
||||
};
|
||||
}
|
||||
|
||||
case 'server/command:approval': {
|
||||
@@ -242,21 +492,32 @@ function reduce(state: ChatConnectionState, action: Action): ChatConnectionState
|
||||
const resolved = resolveScopedConversation(state, payload.conversationId);
|
||||
if (!resolved.active) return state;
|
||||
const next = resolved.state;
|
||||
if (!next.pendingApproval || next.pendingApproval.command !== payload.command) {
|
||||
const conversationId = next.conversationId;
|
||||
if (
|
||||
conversationId === null ||
|
||||
!next.pendingApproval ||
|
||||
typeof payload.command !== 'string' ||
|
||||
payload.command !== next.pendingApproval.command
|
||||
) {
|
||||
// Stale or mismatched response for a request that is no longer (or never was)
|
||||
// the one outstanding approval — do not let it replace active approval state.
|
||||
return next;
|
||||
}
|
||||
return { ...next, approval: payload, approvalRequestPending: false };
|
||||
return {
|
||||
...next,
|
||||
approval: sanitizeApproval(payload, conversationId, payload.command),
|
||||
approvalRequestPending: false,
|
||||
};
|
||||
}
|
||||
|
||||
case 'server/system:reload': {
|
||||
const { payload } = action;
|
||||
return {
|
||||
...state,
|
||||
systemReload: action.payload,
|
||||
systemReload: { ...payload, message: asString(payload.message, 'Commands reloaded.') },
|
||||
manifest: {
|
||||
commands: action.payload.commands,
|
||||
skills: action.payload.skills,
|
||||
commands: capList<CommandDef>(payload.commands, MAX_MANIFEST_ITEMS),
|
||||
skills: capList<SkillCommandDef>(payload.skills, MAX_MANIFEST_ITEMS),
|
||||
version: state.manifest?.version ?? 0,
|
||||
},
|
||||
};
|
||||
@@ -265,24 +526,62 @@ function reduce(state: ChatConnectionState, action: Action): ChatConnectionState
|
||||
case 'server/error': {
|
||||
const { payload } = action;
|
||||
const resolved = resolveScopedConversation(state, payload.conversationId);
|
||||
if (!resolved.active) return state;
|
||||
return { ...resolved.state, error: payload.error, streaming: false };
|
||||
if (!resolved.active) {
|
||||
if (isUnrecoverableStartupFailure(state)) {
|
||||
return {
|
||||
...state,
|
||||
pendingSend: false,
|
||||
sending: false,
|
||||
streaming: false,
|
||||
approvalRequestPending: false,
|
||||
error: CONVERSATION_START_FAILURE,
|
||||
};
|
||||
}
|
||||
return state;
|
||||
}
|
||||
return {
|
||||
...resolved.state,
|
||||
error: asString(payload.error, 'An error occurred.'),
|
||||
streaming: false,
|
||||
sending: false,
|
||||
// A turn-scoped error also invalidates any approval request awaiting
|
||||
// a response — the gateway that just errored is unlikely to still
|
||||
// answer it, and the synchronous approveLockRef mirrors this field.
|
||||
approvalRequestPending: false,
|
||||
};
|
||||
}
|
||||
|
||||
case 'local/send': {
|
||||
const message: ChatTranscriptMessage = {
|
||||
id: `user-${Date.now()}-${state.messages.length}`,
|
||||
// Sourced from the reducer-owned `messageSeq` counter — see the
|
||||
// `server/agent:end` case for why `messages.length`/`Date.now()` are
|
||||
// not collision-safe once MAX_MESSAGES is reached under a flood.
|
||||
id: `user-${state.messageSeq}`,
|
||||
role: 'user',
|
||||
text: action.content,
|
||||
};
|
||||
return {
|
||||
...state,
|
||||
messages: [...state.messages, message],
|
||||
messages: capPush(state.messages, message, MAX_MESSAGES),
|
||||
messageSeq: state.messageSeq + 1,
|
||||
error: null,
|
||||
sending: true,
|
||||
pendingSend: state.conversationId === null ? true : state.pendingSend,
|
||||
};
|
||||
}
|
||||
|
||||
case 'local/disconnect': {
|
||||
// A transient socket disconnect must not leave the UI stuck waiting on
|
||||
// a turn/approval/send that will never resolve on this connection.
|
||||
return {
|
||||
...state,
|
||||
streaming: false,
|
||||
sending: false,
|
||||
pendingSend: false,
|
||||
approvalRequestPending: false,
|
||||
};
|
||||
}
|
||||
|
||||
case 'local/approve-request': {
|
||||
// Only one approval request may be outstanding at a time; a second request
|
||||
// before the first resolves is ignored so it cannot overwrite the original
|
||||
@@ -315,6 +614,32 @@ export function useChatConnection(): ChatConnectionValue {
|
||||
// Reducer state updates are batched/async; a ref lets a double-click on "Run
|
||||
// approved command" be rejected synchronously, before React ever re-renders.
|
||||
const executedApprovalIds = useRef<Set<string>>(new Set());
|
||||
// Synchronous send lock: `state.sending` (reducer) drives the reactive UI
|
||||
// disabled state, but reducer updates are batched/async, so two sendMessage
|
||||
// calls issued in the same tick would both read the same stale `state`. This
|
||||
// ref is the source of truth the guard checks against.
|
||||
//
|
||||
// It is intentionally never cleared directly by an event handler — a stale
|
||||
// agent:end/error for a conversation other than the active one must not be
|
||||
// able to re-arm the lock for a turn that is still genuinely in flight.
|
||||
// Instead it is synchronized from `state.sending` below, which the reducer
|
||||
// only flips on an *accepted* (conversation-scoped, or unconditional
|
||||
// disconnect) transition — resolveScopedConversation already rejects a
|
||||
// foreign-conversation agent:end/error by returning the unchanged state, so
|
||||
// `state.sending` (and therefore this ref) cannot be released by one.
|
||||
const sendLockRef = useRef(false);
|
||||
useEffect(() => {
|
||||
sendLockRef.current = state.sending;
|
||||
}, [state.sending]);
|
||||
|
||||
// Same design as sendLockRef, mirroring `state.approvalRequestPending`:
|
||||
// closes the same-tick double-dispatch race for approveCommand, and is
|
||||
// synchronized from (not cleared by) the reducer's own accepted
|
||||
// command:approval/error/disconnect transitions.
|
||||
const approveLockRef = useRef(false);
|
||||
useEffect(() => {
|
||||
approveLockRef.current = state.approvalRequestPending;
|
||||
}, [state.approvalRequestPending]);
|
||||
|
||||
useEffect(() => {
|
||||
const socket = getSocket();
|
||||
@@ -331,8 +656,9 @@ export function useChatConnection(): ChatConnectionValue {
|
||||
dispatch({ type: 'server/agent:tool:start', payload });
|
||||
const onToolEnd = (payload: ToolEndPayload): void =>
|
||||
dispatch({ type: 'server/agent:tool:end', payload });
|
||||
const onAgentEnd = (payload: AgentEndPayload): void =>
|
||||
const onAgentEnd = (payload: AgentEndPayload): void => {
|
||||
dispatch({ type: 'server/agent:end', payload });
|
||||
};
|
||||
const onSessionInfo = (payload: SessionInfoPayload): void =>
|
||||
dispatch({ type: 'server/session:info', payload });
|
||||
const onCommandsManifest = (payload: CommandManifestPayload): void =>
|
||||
@@ -343,7 +669,12 @@ export function useChatConnection(): ChatConnectionValue {
|
||||
dispatch({ type: 'server/command:approval', payload });
|
||||
const onSystemReload = (payload: SystemReloadPayload): void =>
|
||||
dispatch({ type: 'server/system:reload', payload });
|
||||
const onError = (payload: ErrorPayload): void => dispatch({ type: 'server/error', payload });
|
||||
const onError = (payload: ErrorPayload): void => {
|
||||
dispatch({ type: 'server/error', payload });
|
||||
};
|
||||
const onDisconnect = (): void => {
|
||||
dispatch({ type: 'local/disconnect' });
|
||||
};
|
||||
|
||||
socket.on('message:ack', onMessageAck);
|
||||
socket.on('agent:start', onAgentStart);
|
||||
@@ -358,6 +689,7 @@ export function useChatConnection(): ChatConnectionValue {
|
||||
socket.on('command:approval', onCommandApproval);
|
||||
socket.on('system:reload', onSystemReload);
|
||||
socket.on('error', onError);
|
||||
socket.on('disconnect', onDisconnect);
|
||||
|
||||
if (!socket.connected) {
|
||||
socket.connect();
|
||||
@@ -377,13 +709,15 @@ export function useChatConnection(): ChatConnectionValue {
|
||||
socket.off('command:approval', onCommandApproval);
|
||||
socket.off('system:reload', onSystemReload);
|
||||
socket.off('error', onError);
|
||||
socket.off('disconnect', onDisconnect);
|
||||
destroySocket();
|
||||
};
|
||||
}, []);
|
||||
|
||||
const actions: ChatConnectionActions = {
|
||||
sendMessage: ({ content, provider, modelId }) => {
|
||||
if (state.streaming) return;
|
||||
if (sendLockRef.current || state.streaming || state.sending) return;
|
||||
sendLockRef.current = true;
|
||||
const socket = getSocket();
|
||||
if (!socket.connected) socket.connect();
|
||||
dispatch({ type: 'local/send', content });
|
||||
@@ -415,7 +749,8 @@ export function useChatConnection(): ChatConnectionValue {
|
||||
|
||||
approveCommand: ({ command, args }) => {
|
||||
if (state.conversationId === null) return;
|
||||
if (state.approvalRequestPending) return;
|
||||
if (approveLockRef.current || state.approvalRequestPending) return;
|
||||
approveLockRef.current = true;
|
||||
dispatch({ type: 'local/approve-request', command, args });
|
||||
const socket = getSocket();
|
||||
socket.emit('command:approve', { conversationId: state.conversationId, command, args });
|
||||
@@ -423,14 +758,29 @@ export function useChatConnection(): ChatConnectionValue {
|
||||
|
||||
runApprovedCommand: () => {
|
||||
const { conversationId, approval, pendingApproval } = state;
|
||||
if (conversationId === null || !approval?.success || !approval.approvalId) return;
|
||||
if (conversationId === null) return;
|
||||
// Defense-in-depth: the reducer already normalizes `success`/`approvalId`
|
||||
// before storing `approval` (a matching command string alone is not
|
||||
// proof of a genuine approval), but this action must never rely on that
|
||||
// alone — a literal-`true` and non-empty-string check here too.
|
||||
if (approval?.success !== true) return;
|
||||
if (typeof approval.approvalId !== 'string' || approval.approvalId.length === 0) return;
|
||||
if (!pendingApproval || pendingApproval.command !== approval.command) return;
|
||||
if (executedApprovalIds.current.has(approval.approvalId)) return;
|
||||
if (executedApprovalIds.current.size >= MAX_EXECUTED_APPROVAL_IDS) {
|
||||
// Bounded dedup set: evict the oldest entry (Sets iterate in
|
||||
// insertion order) so this can never grow without limit.
|
||||
const oldest = executedApprovalIds.current.values().next().value;
|
||||
if (oldest !== undefined) executedApprovalIds.current.delete(oldest);
|
||||
}
|
||||
executedApprovalIds.current.add(approval.approvalId);
|
||||
const socket = getSocket();
|
||||
socket.emit('command:execute', {
|
||||
conversationId,
|
||||
command: approval.command,
|
||||
// Sourced entirely from the frozen local pendingApproval, not the
|
||||
// server-echoed approval payload — the equality guard above is
|
||||
// defense-in-depth, not the source of truth for what gets executed.
|
||||
command: pendingApproval.command,
|
||||
args: pendingApproval.args,
|
||||
approvalId: approval.approvalId,
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user