Skip to content

Commit 8a98541

Browse files
Nabeel MNabeel M
Nabeel M
authored and
Nabeel M
committed
fix(threads): /goal can never reach a provider turn
A persisted or edited mobile outbox entry carrying /goal text could drain into a thread.turn.start through both the existing-thread and thread- creation branches, sending a T3-local metadata command to the provider as a prompt. The parser also accepted U+FEFF as a command separator because JS \s includes it (and String.trim strips it at the edges), so invisible formatting characters could forge or mangle a recognized command. - Centralize the /goal delimiter policy on Unicode White_Space via \p{White_Space} with the u flag: space/tab/LF/CRLF/NBSP separate, U+200B/U+FEFF/U+2060 never do, and outer trimming uses the same class so acceptance and trimming cannot drift. - Guard the decider thread.turn.start (the single pre-event seam every provider-bound message funnels through) and the WS bootstrap handler (before thread/worktree side effects) with the same shared parser. - Classify blocked goal text at the mobile outbox delivery boundary: it stays queued with its text preserved, is reported once, shows a Blocked label on pending rows, and never retries or silently discards. - Existing /goal metadata paths (set/show/clear, capability gating, attachments feedback, drafts, web/mobile headers) are unchanged. Model: GPT-5.6 Sol (max), pi harness.
1 parent 97f93de commit 8a98541

13 files changed

Lines changed: 343 additions & 12 deletions

‎apps/mobile/src/features/threads/thread-list-items.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,9 @@ export const PendingTaskListRow = memo(function PendingTaskListRow(props: {
295295

296296
const statusPill = (
297297
<View className="rounded-full bg-zinc-500/12 px-1.5 py-0.5 dark:bg-zinc-500/16">
298-
<Text className="text-3xs font-t3-bold text-zinc-600 dark:text-zinc-300">Pending</Text>
298+
<Text className="text-3xs font-t3-bold text-zinc-600 dark:text-zinc-300">
299+
{pendingTask.blocked ? "Blocked" : "Pending"}
300+
</Text>
299301
</View>
300302
);
301303

‎apps/mobile/src/features/threads/thread-list-v2-items.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,9 @@ export const ThreadListV2PendingRow = memo(function ThreadListV2PendingRow(props
242242
<Text className="flex-1 text-sm font-t3-medium text-foreground-muted" numberOfLines={1}>
243243
{projectTitle}
244244
</Text>
245-
<Text className="text-xs text-foreground-tertiary">Queued</Text>
245+
<Text className="text-xs text-foreground-tertiary">
246+
{pendingTask.blocked ? "Blocked" : "Queued"}
247+
</Text>
246248
</View>
247249
{/* One line, unlike the two an active row allows: a queued title is
248250
derived from the whole prompt rather than written as a title, so the

‎apps/mobile/src/features/threads/threadListV2.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,7 @@ function makePendingTask(id: string): PendingNewTask {
938938
worktreePath: null,
939939
},
940940
title: id,
941+
blocked: false,
941942
};
942943
}
943944

‎apps/mobile/src/state/thread-outbox-model.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
type ProviderInteractionMode as ProviderInteractionModeType,
1616
type RuntimeMode as RuntimeModeType,
1717
} from "@t3tools/contracts";
18+
import { parseThreadGoalCommand } from "@t3tools/shared/composerTrigger";
1819
import * as Schema from "effect/Schema";
1920

2021
import { DraftComposerImageAttachmentSchema } from "../lib/composer-image-schema";
@@ -146,15 +147,24 @@ export function threadOutboxRetryDelayMs(attempt: number): number {
146147
return Math.min(1_000 * 2 ** Math.max(0, attempt - 1), THREAD_OUTBOX_MAX_RETRY_DELAY_MS);
147148
}
148149

149-
export type ThreadOutboxDeliveryAction = "wait" | "remove" | "send";
150+
export type ThreadOutboxDeliveryAction = "wait" | "remove" | "send" | "blocked";
150151

151152
export function resolveThreadOutboxDeliveryAction(input: {
153+
readonly text: string;
152154
readonly isCreation: boolean;
153155
readonly threadExists: boolean;
154156
readonly shellStatus: EnvironmentShellStatus;
155157
readonly environmentConnected: boolean;
156158
readonly threadBusy: boolean;
157159
}): ThreadOutboxDeliveryAction {
160+
// A persisted or edited queued entry can carry T3-local /goal text past the
161+
// live composer interception (offline outbox, pending-task edits, older app
162+
// versions). It must never start a provider turn through either branch, so
163+
// classify it before anything else: no send, no retry loop, and the entry
164+
// stays queued with its text until the user edits or deletes it.
165+
if (parseThreadGoalCommand(input.text) !== null) {
166+
return "blocked";
167+
}
158168
if (input.isCreation) {
159169
// A pending task creates its thread on delivery. If the thread already
160170
// exists the creation command went through and only cleanup remains.

‎apps/mobile/src/state/thread-outbox.test.ts‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,7 @@ describe("thread outbox", () => {
460460
it("only removes a missing-thread message after shell synchronization is live", () => {
461461
expect(
462462
resolveThreadOutboxDeliveryAction({
463+
text: "ship it",
463464
isCreation: false,
464465
threadExists: false,
465466
shellStatus: "synchronizing",
@@ -469,6 +470,7 @@ describe("thread outbox", () => {
469470
).toBe("wait");
470471
expect(
471472
resolveThreadOutboxDeliveryAction({
473+
text: "ship it",
472474
isCreation: false,
473475
threadExists: false,
474476
shellStatus: "live",
@@ -478,6 +480,7 @@ describe("thread outbox", () => {
478480
).toBe("remove");
479481
expect(
480482
resolveThreadOutboxDeliveryAction({
483+
text: "ship it",
481484
isCreation: false,
482485
threadExists: true,
483486
shellStatus: "live",
@@ -490,6 +493,7 @@ describe("thread outbox", () => {
490493
it("sends existing-thread messages whenever connected so queued messages can steer", () => {
491494
expect(
492495
resolveThreadOutboxDeliveryAction({
496+
text: "ship it",
493497
isCreation: false,
494498
threadExists: true,
495499
shellStatus: "live",
@@ -499,6 +503,7 @@ describe("thread outbox", () => {
499503
).toBe("send");
500504
expect(
501505
resolveThreadOutboxDeliveryAction({
506+
text: "ship it",
502507
isCreation: false,
503508
threadExists: true,
504509
shellStatus: "live",
@@ -508,9 +513,85 @@ describe("thread outbox", () => {
508513
).toBe("wait");
509514
});
510515

516+
it("blocks T3-local /goal text from both delivery branches before anything sends", () => {
517+
// A persisted or edited outbox entry can carry /goal text past the live
518+
// composer interception. It must never drain into a provider turn — not
519+
// through the existing-thread branch nor through thread creation — and it
520+
// must stay queued (never sent, never discarded) for the user to fix.
521+
const goalTexts = ["/goal ship it", "/goal", "/goal clear", " /goal ship it "];
522+
for (const text of goalTexts) {
523+
expect(
524+
resolveThreadOutboxDeliveryAction({
525+
text,
526+
isCreation: false,
527+
threadExists: true,
528+
shellStatus: "live",
529+
environmentConnected: true,
530+
threadBusy: false,
531+
}),
532+
).toBe("blocked");
533+
expect(
534+
resolveThreadOutboxDeliveryAction({
535+
text,
536+
isCreation: true,
537+
threadExists: false,
538+
shellStatus: "live",
539+
environmentConnected: true,
540+
threadBusy: false,
541+
}),
542+
).toBe("blocked");
543+
}
544+
// Blocked wins even while offline, so a later reconnect cannot drain it.
545+
expect(
546+
resolveThreadOutboxDeliveryAction({
547+
text: "/goal ship it",
548+
isCreation: true,
549+
threadExists: false,
550+
shellStatus: "empty",
551+
environmentConnected: false,
552+
threadBusy: false,
553+
}),
554+
).toBe("blocked");
555+
});
556+
557+
it("keeps sending ordinary queued text through both delivery branches", () => {
558+
expect(
559+
resolveThreadOutboxDeliveryAction({
560+
text: "ship the login fix",
561+
isCreation: false,
562+
threadExists: true,
563+
shellStatus: "live",
564+
environmentConnected: true,
565+
threadBusy: false,
566+
}),
567+
).toBe("send");
568+
expect(
569+
resolveThreadOutboxDeliveryAction({
570+
text: "ship the login fix",
571+
isCreation: true,
572+
threadExists: false,
573+
shellStatus: "live",
574+
environmentConnected: true,
575+
threadBusy: false,
576+
}),
577+
).toBe("send");
578+
// A prompt that merely mentions /goal mid-text is an ordinary prompt.
579+
expect(
580+
resolveThreadOutboxDeliveryAction({
581+
text: "please run /goal now",
582+
isCreation: false,
583+
threadExists: true,
584+
shellStatus: "live",
585+
environmentConnected: true,
586+
threadBusy: false,
587+
}),
588+
).toBe("send");
589+
});
590+
511591
it("sends queued creations once connected and live, removing already-created ones", () => {
512592
expect(
513593
resolveThreadOutboxDeliveryAction({
594+
text: "ship it",
514595
isCreation: true,
515596
threadExists: false,
516597
shellStatus: "cached",
@@ -522,6 +603,7 @@ describe("thread outbox", () => {
522603
// simply not be visible yet — sending now could duplicate the thread.
523604
expect(
524605
resolveThreadOutboxDeliveryAction({
606+
text: "ship it",
525607
isCreation: true,
526608
threadExists: false,
527609
shellStatus: "synchronizing",
@@ -531,6 +613,7 @@ describe("thread outbox", () => {
531613
).toBe("wait");
532614
expect(
533615
resolveThreadOutboxDeliveryAction({
616+
text: "ship it",
534617
isCreation: true,
535618
threadExists: false,
536619
shellStatus: "live",
@@ -540,6 +623,7 @@ describe("thread outbox", () => {
540623
).toBe("send");
541624
expect(
542625
resolveThreadOutboxDeliveryAction({
626+
text: "ship it",
543627
isCreation: true,
544628
threadExists: true,
545629
shellStatus: "live",

‎apps/mobile/src/state/use-pending-new-tasks.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { useMemo } from "react";
22

3+
import { parseThreadGoalCommand } from "@t3tools/shared/composerTrigger";
4+
35
import { deriveThreadTitleFromPrompt } from "../lib/projectThreadStartTurn";
46
import {
57
flattenQueuedThreadMessages,
@@ -13,6 +15,8 @@ export interface PendingNewTask {
1315
readonly message: QueuedThreadMessage;
1416
readonly creation: QueuedThreadCreation;
1517
readonly title: string;
18+
/** True when the queued text is a T3-local /goal command: it can never send. */
19+
readonly blocked: boolean;
1620
}
1721

1822
export function usePendingNewTasks(): ReadonlyArray<PendingNewTask> {
@@ -27,6 +31,7 @@ export function usePendingNewTasks(): ReadonlyArray<PendingNewTask> {
2731
message,
2832
creation: message.creation,
2933
title: deriveThreadTitleFromPrompt(message.text),
34+
blocked: parseThreadGoalCommand(message.text) !== null,
3035
});
3136
}
3237
tasks.sort((left, right) => right.message.createdAt.localeCompare(left.message.createdAt));

‎apps/mobile/src/state/use-thread-outbox-drain.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { buildProjectThreadStartTurnInput } from "../lib/projectThreadStartTurn"
2020
import { toUploadChatImageAttachments } from "../lib/composerImages";
2121
import { randomHex } from "../lib/uuid";
2222
import { appAtomRegistry } from "./atom-registry";
23+
import { setPendingConnectionError } from "./use-remote-environment-registry";
2324
import { useProjects, useThreadShells } from "./entities";
2425
import {
2526
confirmThreadOutboxMessageQueued,
@@ -107,6 +108,7 @@ export function useThreadOutboxDrain(): void {
107108
const retryAttemptRef = useRef(new Map<MessageId, number>());
108109
const retryNotBeforeRef = useRef(new Map<MessageId, number>());
109110
const retryTimersRef = useRef(new Map<MessageId, ReturnType<typeof setTimeout>>());
111+
const reportedBlockedMessageIdsRef = useRef(new Set<MessageId>());
110112

111113
useEffect(() => {
112114
ensureThreadOutboxLoaded();
@@ -310,6 +312,7 @@ export function useThreadOutboxDrain(): void {
310312
);
311313
const shellStatus = shellStatuses.get(nextQueuedMessage.environmentId) ?? "empty";
312314
const deliveryAction = resolveThreadOutboxDeliveryAction({
315+
text: nextQueuedMessage.text,
313316
isCreation: creation !== undefined,
314317
threadExists: thread !== undefined,
315318
shellStatus,
@@ -319,6 +322,22 @@ export function useThreadOutboxDrain(): void {
319322
if (deliveryAction === "wait") {
320323
continue;
321324
}
325+
// Blocked entries stay queued forever by design (no send, no retry),
326+
// so the drain reports each one once instead of looping silently.
327+
if (deliveryAction === "blocked") {
328+
if (!reportedBlockedMessageIdsRef.current.has(nextQueuedMessage.messageId)) {
329+
reportedBlockedMessageIdsRef.current.add(nextQueuedMessage.messageId);
330+
console.warn("[thread-outbox] queued message blocked: /goal cannot start a turn", {
331+
environmentId: nextQueuedMessage.environmentId,
332+
threadId: nextQueuedMessage.threadId,
333+
messageId: nextQueuedMessage.messageId,
334+
});
335+
setPendingConnectionError(
336+
"A queued /goal command cannot be sent to the agent. Edit the pending task to change its text — your message was kept.",
337+
);
338+
}
339+
continue;
340+
}
322341
// The live project shell is preferred for the workspace path, with the
323342
// snapshot taken at enqueue time as the fallback so a task never dies
324343
// just because its project shell is not loaded.

‎apps/server/src/orchestration/Layers/ProviderCommandReactor.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,41 @@ describe("ProviderCommandReactor", () => {
678678
}),
679679
);
680680

681+
effectIt.effect("never sends a provider turn for T3-local /goal text", () =>
682+
Effect.gen(function* () {
683+
const harness = yield* Effect.promise(() => createHarness());
684+
const now = "2026-01-01T00:00:00.000Z";
685+
686+
const failure = yield* harness.engine
687+
.dispatch({
688+
type: "thread.turn.start",
689+
commandId: CommandId.make("cmd-turn-start-goal"),
690+
threadId: ThreadId.make("thread-1"),
691+
message: {
692+
messageId: asMessageId("user-message-goal"),
693+
role: "user",
694+
text: "/goal ship it",
695+
attachments: [],
696+
},
697+
interactionMode: DEFAULT_PROVIDER_INTERACTION_MODE,
698+
runtimeMode: "approval-required",
699+
createdAt: now,
700+
})
701+
.pipe(Effect.flip);
702+
expect(failure._tag).toBe("OrchestrationCommandInvariantError");
703+
704+
// The rejection happened before any event existed, so there is nothing
705+
// for the reactor to act on: no provider payload, no chat history.
706+
expect(harness.startSession).not.toHaveBeenCalled();
707+
expect(harness.sendTurn).not.toHaveBeenCalled();
708+
const readModel = yield* Effect.promise(() => harness.readModel());
709+
const thread = readModel.threads.find((entry) => entry.id === ThreadId.make("thread-1"));
710+
expect(thread?.messages).toEqual([]);
711+
expect(thread?.activities).toEqual([]);
712+
expect(thread?.session).toBeNull();
713+
}),
714+
);
715+
681716
it("generates a thread title on the first turn", async () => {
682717
const harness = await createHarness();
683718
const now = "2026-01-01T00:00:00.000Z";

0 commit comments

Comments
 (0)