Skip to content

Commit c936ca9

Browse files
t3dotggclaude
andcommitted
fix: side chat review round 5
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 8894d8c commit c936ca9

11 files changed

Lines changed: 155 additions & 47 deletions

‎apps/mobile/src/features/threads/ThreadSideChatsRow.tsx‎

Lines changed: 31 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,33 +28,37 @@ export function ThreadSideChatsRow(props: {
2828
<Text className="text-xs text-foreground-muted">Side chats</Text>
2929
<ScrollView horizontal showsHorizontalScrollIndicator={false} className="flex-1">
3030
<View className="flex-row gap-1 pr-4">
31-
{sideChats.map((sideChat) => (
32-
<Pressable
33-
key={sideChat.id}
34-
accessibilityRole="button"
35-
accessibilityLabel={`Open side chat ${sideChat.title}`}
36-
className="max-w-48 flex-row items-center gap-1.5 rounded-full bg-subtle px-2.5 py-1"
37-
onPress={() =>
38-
openLinkedThread({ environmentId: props.environmentId, threadId: sideChat.id })
39-
}
40-
>
41-
{sideChat.hasPendingApprovals ||
42-
sideChat.hasPendingUserInput ||
43-
sideChat.pendingSubagentRequest ? (
44-
<View
45-
className={cn(
46-
"h-1.5 w-1.5 shrink-0 rounded-full",
47-
sideChat.hasPendingApprovals || sideChat.pendingSubagentRequest === "approval"
48-
? "bg-warning-foreground"
49-
: "bg-adaptive-indigo-600-300",
50-
)}
51-
/>
52-
) : null}
53-
<Text className="shrink text-xs text-foreground" numberOfLines={1}>
54-
{sideChat.title}
55-
</Text>
56-
</Pressable>
57-
))}
31+
{sideChats.map((sideChat) => {
32+
const needs =
33+
sideChat.hasPendingApprovals || sideChat.pendingSubagentRequest === "approval"
34+
? "approval"
35+
: sideChat.hasPendingUserInput || sideChat.pendingSubagentRequest
36+
? "input"
37+
: null;
38+
return (
39+
<Pressable
40+
key={sideChat.id}
41+
accessibilityRole="button"
42+
accessibilityLabel={`Open side chat ${sideChat.title}${needs === null ? "" : `, needs ${needs}`}`}
43+
className="max-w-48 flex-row items-center gap-1.5 rounded-full bg-subtle px-2.5 py-1"
44+
onPress={() =>
45+
openLinkedThread({ environmentId: props.environmentId, threadId: sideChat.id })
46+
}
47+
>
48+
{needs === null ? null : (
49+
<View
50+
className={cn(
51+
"h-1.5 w-1.5 shrink-0 rounded-full",
52+
needs === "approval" ? "bg-warning-foreground" : "bg-adaptive-indigo-600-300",
53+
)}
54+
/>
55+
)}
56+
<Text className="shrink text-xs text-foreground" numberOfLines={1}>
57+
{sideChat.title}
58+
</Text>
59+
</Pressable>
60+
);
61+
})}
5862
</View>
5963
</ScrollView>
6064
</View>

‎apps/server/src/orchestration-v2/Orchestrator.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4282,18 +4282,18 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio
42824282
cause: `Thread ${command.targetThreadId} is still running. Bring back is available when it is idle.`,
42834283
});
42844284
}
4285-
const otherPendingMergeBack = targetProjection.contextTransfers.find(
4285+
const otherPendingMergeBack = targetProjection.contextTransfers.some(
42864286
(transfer) =>
42874287
transfer.type === "merge_back" &&
42884288
transfer.status === "pending" &&
42894289
transfer.targetThreadId === command.targetThreadId &&
42904290
transfer.sourceThreadId !== command.sourceThreadId,
42914291
);
4292-
if (otherPendingMergeBack !== undefined) {
4292+
if (otherPendingMergeBack) {
42934293
return yield* new OrchestratorDispatchError({
42944294
commandId: command.commandId,
42954295
commandType: command.type,
4296-
cause: `Thread ${command.targetThreadId} already has pending context from ${otherPendingMergeBack.sourceThreadId}. Send a message there first.`,
4296+
cause: `Thread ${command.targetThreadId} already has pending context from another side chat or fork. Send a message there first.`,
42974297
});
42984298
}
42994299

‎apps/server/src/orchestration-v2/ProjectionStore.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4219,6 +4219,8 @@ export const layer: Layer.Layer<ProjectionStoreV2, never, SqlClient.SqlClient> =
42194219
message_id IN (SELECT json_extract(payload_json, '$.userMessageId') FROM orchestration_v2_projection_runs
42204220
WHERE thread_id = ${threadId} AND run_id = ${runId})
42214221
OR (role = 'user' AND lower(trim(json_extract(payload_json, '$.text'), ${javascriptTrimWhitespace})) = '/compact')
4222+
-- Earlier /goal turns defer context too; parseGoalCommand narrows this prefix match.
4223+
OR (role = 'user' AND trim(json_extract(payload_json, '$.text'), ${javascriptTrimWhitespace}) GLOB '/goal*')
42224224
) ORDER BY created_at ASC, message_id ASC
42234225
`.pipe(Effect.flatMap(decodeRows(decodeMessagePayload, threadId)));
42244226
const conversation = yield* sql<{ present: number }>`

‎apps/server/src/orchestration-v2/ProviderTurnStartService.test.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -291,11 +291,12 @@ function makeLocalCommandHarness(input: {
291291
readonly interruptCurrentRun?: boolean;
292292
};
293293
/**
294-
* A side chat whose context targets this run, an earlier run that failed before the provider,
295-
* or an earlier run that delivered it. `nativeForkFrom` plans a native fork of the parent.
294+
* A side chat whose context targets this run, an earlier run that failed or was cancelled before
295+
* the provider, or an earlier run that delivered it. `nativeForkFrom` plans a native fork of the
296+
* parent.
296297
*/
297298
readonly sideChat?: {
298-
readonly contextRun: "current" | "failed" | "delivered";
299+
readonly contextRun: "current" | "failed" | "cancelled" | "delivered";
299300
readonly nativeForkFrom?: "idle" | "busy";
300301
};
301302
}) {
@@ -539,7 +540,10 @@ function makeLocalCommandHarness(input: {
539540
ordinal: 1,
540541
userMessageId: MessageId.make("side-chat-earlier-message"),
541542
activeAttemptId: RunAttemptId.make("side-chat-earlier-attempt"),
542-
status: input.sideChat.contextRun === "failed" ? "failed" : "completed",
543+
status:
544+
input.sideChat.contextRun === "delivered"
545+
? "completed"
546+
: input.sideChat.contextRun,
543547
completedAt: now,
544548
},
545549
run,
@@ -1162,6 +1166,7 @@ effectIt.effect("recovers an abandoned failed turn before continuing its native
11621166
effectIt.effect.each([
11631167
{ contextRun: "current", runtimeMode: "approval-required", context: "SIDE_CHAT_CONTEXT" },
11641168
{ contextRun: "failed", runtimeMode: "approval-required", context: "SIDE_CHAT_CONTEXT" },
1169+
{ contextRun: "cancelled", runtimeMode: "approval-required", context: "SIDE_CHAT_CONTEXT" },
11651170
{ contextRun: "delivered", runtimeMode: "full-access", context: "" },
11661171
] as const)(
11671172
"sends side chat context inline once, for the $contextRun run's turn",

‎apps/server/src/orchestration-v2/ProviderTurnStartService.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@ export const layer: Layer.Layer<
335335
source.id === handoff.targetRunId &&
336336
(source.status === "failed" ||
337337
source.status === "interrupted" ||
338+
source.status === "cancelled" ||
338339
(source.status === "completed" &&
339340
handoff.delivery === undefined &&
340341
// Compaction and /goal turns defer history to the next turn.

‎apps/server/src/orchestration-v2/ShellStream.test.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ describe("omitOlderSettledShells", () => {
277277
expect(result.olderSettled.counts).toEqual([]);
278278
});
279279

280-
it("keeps older rows that are snoozed, live, or above a kept subagent, and still counts them", () => {
280+
it("keeps older rows that are snoozed, live, or above a kept subagent or side chat, and still counts them", () => {
281281
const threads = [
282282
...newest,
283283
row("snoozed", 30, { snoozedUntil: DateTime.makeUnsafe(101 * DAY) }),
@@ -295,15 +295,26 @@ describe("omitOlderSettledShells", () => {
295295
settledOverride: null,
296296
settledAt: null,
297297
}),
298+
row("side-chat-parent", 30),
299+
row("side-chat", null, {
300+
lineage: {
301+
parentThreadId: ThreadId.make("side-chat-parent"),
302+
relationshipToParent: "fork",
303+
rootThreadId: ThreadId.make("side-chat-parent"),
304+
sideChat: true,
305+
},
306+
}),
298307
];
299308
const result = omit(threads);
300309
expect(result.threads.map((thread) => thread.id).slice(25)).toEqual([
301310
"snoozed",
302311
"running",
303312
"parent",
304313
"child",
314+
"side-chat-parent",
315+
"side-chat",
305316
]);
306-
expect(result.olderSettled.counts).toEqual([{ projectId: "project-a", count: 4 }]);
317+
expect(result.olderSettled.counts).toEqual([{ projectId: "project-a", count: 5 }]);
307318
});
308319
});
309320

‎apps/server/src/orchestration-v2/ShellStream.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,8 @@ const NEWEST_SETTLED_KEPT = 25;
119119
/**
120120
* Opted-in snapshots keep the 25 newest settled top-level threads (and ties) and count the rest per
121121
* project in `olderSettled`; clients page them with `getOlderSettledThreads`. Older rows stay while
122-
* pinned, snoozed, live, or above a kept subagent, so walks over shell rows still find them.
122+
* pinned, snoozed, live, or above a kept subagent or side chat, so walks over shell rows still find
123+
* them.
123124
*/
124125
export function omitOlderSettledShells(input: {
125126
readonly threads: ReadonlyArray<OrchestrationV2ThreadShell>;
@@ -143,7 +144,7 @@ export function omitOlderSettledShells(input: {
143144
const beforeMs = settledTimes[NEWEST_SETTLED_KEPT - 1] ?? settledTimes.at(-1) ?? nowMs;
144145
const ancestorIds = new Set<string>();
145146
for (const thread of input.threads) {
146-
if (thread.lineage.relationshipToParent !== "subagent") continue;
147+
if (isListedThreadLineage(thread.lineage)) continue;
147148
if (thread.lineage.parentThreadId !== null) ancestorIds.add(thread.lineage.parentThreadId);
148149
ancestorIds.add(thread.lineage.rootThreadId);
149150
}

‎apps/server/src/orchestration-v2/SteeringCompletion.integration.test.ts‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1466,3 +1466,82 @@ it.effect("stops the running turn when a goal clear ends the goal driving it", (
14661466
}),
14671467
),
14681468
);
1469+
1470+
it.effect("a side chat that opens with /goal gets its context on the next question", () =>
1471+
Effect.scoped(
1472+
Effect.gen(function* () {
1473+
const { events, layer, started, startFirstTurn } = yield* goalHarness("side-chat-goal");
1474+
yield* Effect.gen(function* () {
1475+
const orchestrator = yield* Orchestrator.OrchestratorV2;
1476+
const worker = yield* EffectWorker.OrchestrationEffectWorkerV2;
1477+
const parentId = yield* startFirstTurn;
1478+
const sideChatId = ThreadId.make("thread:side-chat-goal-child");
1479+
yield* orchestrator.dispatch({
1480+
type: "thread.fork",
1481+
commandId: CommandId.make("side-chat"),
1482+
sourceThreadId: parentId,
1483+
targetThreadId: sideChatId,
1484+
sourcePoint: { type: "latest_stable" },
1485+
sideChat: true,
1486+
createdBy: "user",
1487+
creationSource: "web",
1488+
});
1489+
const ask = (id: string, text: string) =>
1490+
orchestrator.dispatch({
1491+
type: "message.dispatch",
1492+
commandId: CommandId.make(id),
1493+
threadId: sideChatId,
1494+
messageId: MessageId.make(`message:${id}`),
1495+
text,
1496+
attachments: [],
1497+
modelSelection: runSelection,
1498+
dispatchMode: { type: "start_immediately" },
1499+
createdBy: "user",
1500+
creationSource: "web",
1501+
});
1502+
const sideChatEvent = (matches: (event: OrchestrationV2DomainEvent) => boolean) =>
1503+
orchestrator.streamDomainEvents.pipe(
1504+
Stream.filter((event) => event.threadId === sideChatId && matches(event)),
1505+
Stream.take(1),
1506+
Stream.runDrain,
1507+
Effect.forkScoped({ startImmediately: true }),
1508+
);
1509+
const running = yield* sideChatEvent(
1510+
(event) => event.type === "provider-turn.updated" && event.payload.status === "running",
1511+
);
1512+
const settled = yield* sideChatEvent(
1513+
(event) => event.type === "run.updated" && event.payload.status === "waiting",
1514+
);
1515+
yield* ask("goal", "/goal clear");
1516+
yield* worker.drain();
1517+
yield* Fiber.join(running);
1518+
const turn = (yield* orchestrator.getThreadProjection(sideChatId)).providerTurns[0]!;
1519+
yield* Queue.offer(events, {
1520+
type: "provider_turn.updated",
1521+
driver,
1522+
providerTurn: { ...turn, status: "completed", completedAt: yield* DateTime.now },
1523+
});
1524+
yield* Queue.offer(events, {
1525+
type: "turn.terminal",
1526+
driver,
1527+
providerThreadId: turn.providerThreadId,
1528+
providerTurnId: turn.id,
1529+
runOrdinal: started[1]!.runOrdinal,
1530+
status: "completed",
1531+
failure: null,
1532+
threadDisposition: "reusable",
1533+
});
1534+
yield* Fiber.join(settled);
1535+
yield* worker.drain();
1536+
yield* ask("question", "What is the main thread doing?");
1537+
yield* worker.drain();
1538+
1539+
const [first, goal, question] = started.map((turn) => turn.message.text);
1540+
assert.deepEqual([first, goal], ["first", "/goal clear"]);
1541+
assert.include(question, "You are in a side chat beside the main thread.");
1542+
assert.include(question, "- User: first");
1543+
assert.isTrue(question?.endsWith("What is the main thread doing?"));
1544+
}).pipe(Effect.provide(layer));
1545+
}),
1546+
),
1547+
);

‎apps/server/src/orchestration-v2/ThreadFork.execution.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1070,7 +1070,7 @@ it.effect("merge-back rejects a second fork's context until the first is consume
10701070

10711071
yield* mergeBack("merge-back-guard-first", first);
10721072
const rejected = yield* mergeBack("merge-back-guard-second", second).pipe(Effect.flip);
1073-
assert.include(String(rejected.cause), first.threadId);
1073+
assert.include(String(rejected.cause), "pending context from another side chat or fork");
10741074
yield* mergeBack("merge-back-guard-first-again", first);
10751075
// That pending merge-back targets the parent, so it does not block one into `first`.
10761076
const nested = { threadId: ThreadId.make("merge-back-guard-nested") };

‎apps/web/src/components/ChatView.tsx‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2772,13 +2772,15 @@ export default function ChatView(props: ChatViewProps) {
27722772
const sideChatDisabledReason =
27732773
activeThread?.lineage.sideChat === true
27742774
? "Side chats cannot start side chats."
2775-
: sideChat.available &&
2776-
orchestrationV2WorktreePending(
2777-
activeThread?.worktreePath ?? null,
2778-
serverProjection?.runs ?? [],
2779-
)
2780-
? "Available once the thread's worktree is ready."
2781-
: null;
2775+
: activeThread?.lineage.relationshipToParent === "subagent"
2776+
? "Subagent threads cannot start side chats."
2777+
: sideChat.available &&
2778+
orchestrationV2WorktreePending(
2779+
activeThread?.worktreePath ?? null,
2780+
serverProjection?.runs ?? [],
2781+
)
2782+
? "Available once the thread's worktree is ready."
2783+
: null;
27822784
const openSideChatStable = useStableCallback(() => void sideChat.open());
27832785
const sendSideChatStable = useStableCallback((prompt: string) => sideChat.open(prompt));
27842786
const { onConfirmRevertStable, onForkFromRunStable, onRevertTimelineTurnStable } =

0 commit comments

Comments
 (0)