Conversation
github-actions
Bot
added
the
vouch:unvouched
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
size:XXL
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
One consistency finding in the new web handoff card: the action controls re-create the shared Button primitive as raw <button> elements and lose its interaction/accessibility behavior. Details inline.
Posted via Macroscope — UI Consistency
| <button | ||
| type="button" | ||
| disabled={busy} | ||
| onClick={onOpen} | ||
| className="rounded-md bg-primary px-2.5 py-1.5 font-medium text-primary-foreground disabled:opacity-50" | ||
| > | ||
| Open {handoff.title} thread | ||
| </button> | ||
| <button | ||
| type="button" | ||
| disabled={busy} | ||
| onClick={onDismiss} | ||
| className="rounded-md px-2.5 py-1.5 text-muted-foreground hover:bg-muted disabled:opacity-50" | ||
| > | ||
| Dismiss | ||
| </button> |
There was a problem hiding this comment.
These two raw <button> elements reconstruct the shared Button primitive (~/components/ui/button) and drop behavior it owns: cursor-pointer, the focus-visible:ring-2 ring-ring ring-offset-* ring, disabled:pointer-events-none (currently only opacity changes, so a busy button still receives hover/click), the rounded-[var(--control-radius)] token instead of a hard-coded rounded-md, the primary variant's border/inset-shadow and pressed states, and the pointer-coarse 44px minimum hit target. The sibling banner in the same stack (ThreadErrorBanner) composes Alert + Button for exactly this reason.
Suggest using the primitive with its sm size and ghost variant, and adding import { Button } from "../ui/button"; at the top of the file (Button already defaults type="button"):
| <button | |
| type="button" | |
| disabled={busy} | |
| onClick={onOpen} | |
| className="rounded-md bg-primary px-2.5 py-1.5 font-medium text-primary-foreground disabled:opacity-50" | |
| > | |
| Open {handoff.title} thread | |
| </button> | |
| <button | |
| type="button" | |
| disabled={busy} | |
| onClick={onDismiss} | |
| className="rounded-md px-2.5 py-1.5 text-muted-foreground hover:bg-muted disabled:opacity-50" | |
| > | |
| Dismiss | |
| </button> | |
| <Button size="sm" disabled={busy} onClick={onOpen}> | |
| Open {handoff.title} thread | |
| </Button> | |
| <Button size="sm" variant="ghost" disabled={busy} onClick={onDismiss}> | |
| Dismiss | |
| </Button> |
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Reviewed the new Effect-facing code (MCP handoff toolkit, orchestration decider/handoff helpers, client-runtime commands) against the service conventions. Service acquisition, layer wiring, and effect/* subpath namespace imports look correct; the findings below are about how the new MCP tool failure is modeled.
Posted via Macroscope — Effect Service Conventions
| export const ThreadHandoffToolFailure = Schema.Struct({ | ||
| message: Schema.String, | ||
| }); |
There was a problem hiding this comment.
The tool failure is an unstructured { message: string } struct, which stores the message as the only data. Consider a Schema.TaggedErrorClass with stable structural attributes plus a cause, deriving message from those attributes (the sibling preview toolkit already uses tagged PreviewAutomationError classes for failure).
-export const ThreadHandoffToolFailure = Schema.Struct({
- message: Schema.String,
-});
+export class ThreadHandoffToolError extends Schema.TaggedErrorClass<ThreadHandoffToolError>()(
+ "ThreadHandoffToolError",
+ {
+ threadId: ThreadId,
+ handoffId: ThreadHandoffId,
+ cause: Schema.Defect(),
+ },
+) {
+ override get message(): string {
+ return `Unable to request a thread handoff for thread ${this.threadId}.`;
+ }
+}This also needs ThreadId added to the @t3tools/contracts import and failure: ThreadHandoffToolError on the tool.
Posted via Macroscope — Effect Service Conventions
| import * as McpInvocationContext from "../../McpInvocationContext.ts"; | ||
| import { OrchestrationEngineService } from "../../../orchestration/Services/OrchestrationEngine.ts"; |
There was a problem hiding this comment.
At this service boundary the local service module is imported by named tag instead of as a namespace (same in handlers.ts and tools.test.ts). Suggest matching the adjacent McpInvocationContext/PreviewAutomationBroker style so the module shape stays visible:
| import * as McpInvocationContext from "../../McpInvocationContext.ts"; | |
| import { OrchestrationEngineService } from "../../../orchestration/Services/OrchestrationEngine.ts"; | |
| import * as McpInvocationContext from "../../McpInvocationContext.ts"; | |
| import * as OrchestrationEngine from "../../../orchestration/Services/OrchestrationEngine.ts"; |
References then become OrchestrationEngine.OrchestrationEngineService.
Posted via Macroscope — Effect Service Conventions
| requestThreadHandoff(input).pipe( | ||
| Effect.mapError((error) => ({ | ||
| message: error instanceof Error ? error.message : "Unable to request a thread handoff.", | ||
| })), |
There was a problem hiding this comment.
mapError erases the structured OrchestrationDispatchError into a string and derives the wrapper message from cause.message, dropping the cause chain. Consider constructing a tagged error at the failure boundary (the dispatch call) so threadId/handoffId and the original error survive:
- yield* orchestrationEngine.dispatch({
+ yield* orchestrationEngine.dispatch({
type: "thread.handoff.request",
...
- });
+ }).pipe(
+ Effect.mapError(
+ (cause) =>
+ new ThreadHandoffToolError({
+ threadId: invocation.threadId,
+ handoffId: ThreadHandoffId.make(handoffId),
+ cause,
+ }),
+ ),
+ );The toolkit entry can then be request_thread_handoff: (input) => requestThreadHandoff(input) with no message stringification.
Posted via Macroscope — Effect Service Conventions
| if (failure === null) { | ||
| const startedResult = await settlePromise(() => | ||
| waitForStartedServerThread(scopeThreadRef(activeThread.environmentId, nextThreadId)), | ||
| seedHandoffPrompt( |
There was a problem hiding this comment.
🟠 High components/ChatView.tsx:5772
The Plan → new-thread flow leaves the target draft in plan mode, so sending the seeded PLEASE IMPLEMENT THIS PLAN prompt starts another planning turn instead of implementing the plan. acceptThreadHandoff inherits the source thread's mode, and this path only seeds the prompt; set the target draft's interaction mode to default before navigating.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatView.tsx around line 5772:
The Plan → new-thread flow leaves the target draft in `plan` mode, so sending the seeded `PLEASE IMPLEMENT THIS PLAN` prompt starts another planning turn instead of implementing the plan. `acceptThreadHandoff` inherits the source thread's mode, and this path only seeds the prompt; set the target draft's interaction mode to `default` before navigating.
|
|
||
| if (failure === null) { | ||
| const startResult = await startThreadTurn({ | ||
| const acceptResult = await acceptThreadHandoff({ |
There was a problem hiding this comment.
🟡 Medium components/ChatView.tsx:5760
onImplementPlanInNewThread creates the target through acceptThreadHandoff, so the new thread inherits the source thread’s persisted runtimeMode and ignores the composer’s current mode. A user who switches supervision or access mode before opening the implementation thread therefore gets the wrong permission mode; carry runtimeMode through the handoff or apply it to the target before navigation.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatView.tsx around line 5760:
`onImplementPlanInNewThread` creates the target through `acceptThreadHandoff`, so the new thread inherits the source thread’s persisted `runtimeMode` and ignores the composer’s current mode. A user who switches supervision or access mode before opening the implementation thread therefore gets the wrong permission mode; carry `runtimeMode` through the handoff or apply it to the target before navigation.
|
|
||
| if (failure === null) { | ||
| const startResult = await startThreadTurn({ | ||
| const acceptResult = await acceptThreadHandoff({ |
There was a problem hiding this comment.
🟡 Medium components/ChatView.tsx:5760
Choosing “implement in new thread” drops the composer’s current sendCtx.selectedModelSelection, so the accepted target is created with the source thread’s persisted model/provider instead of the user’s latest selection. This also means outgoingImplementationPrompt may be formatted for a different provider than the target uses; pass the current model selection through the handoff creation/accept path.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/ChatView.tsx around line 5760:
Choosing “implement in new thread” drops the composer’s current `sendCtx.selectedModelSelection`, so the accepted target is created with the source thread’s persisted model/provider instead of the user’s latest selection. This also means `outgoingImplementationPrompt` may be formatted for a different provider than the target uses; pass the current model selection through the handoff creation/accept path.
| return ( | ||
| <View | ||
| accessibilityLabel={presentation.accessibilityLabel} | ||
| className="absolute left-3 right-3 top-2 z-20 rounded-2xl border border-border bg-surface px-3 py-2 shadow-sm" |
There was a problem hiding this comment.
🟡 Medium threads/ThreadHandoffCard.tsx:18
When availableHandoffs contains multiple items, only the topmost ThreadHandoffCard is usable; every card renders at the same absolute left-3 right-3 top-2 position, so earlier handoffs and their Open/Dismiss controls are covered. Render a single handoff or give the mapped cards a non-overlapping layout.
Also found in 1 other location(s)
apps/mobile/src/features/threads/ThreadDetailScreen.tsx:590
Every
ThreadHandoffCardrendered by this map uses the same absoluteleft-3 right-3 top-2position. WhenavailableHandoffscontains more than one item, the cards stack directly on top of each other, hiding the earlier handoffs and making their Open/Dismiss controls inaccessible. Render only one card or give the collection a non-overlapping layout.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadHandoffCard.tsx around line 18:
When `availableHandoffs` contains multiple items, only the topmost `ThreadHandoffCard` is usable; every card renders at the same absolute `left-3 right-3 top-2` position, so earlier handoffs and their Open/Dismiss controls are covered. Render a single handoff or give the mapped cards a non-overlapping layout.
Also found in 1 other location(s):
- apps/mobile/src/features/threads/ThreadDetailScreen.tsx:590 -- Every `ThreadHandoffCard` rendered by this map uses the same absolute `left-3 right-3 top-2` position. When `availableHandoffs` contains more than one item, the cards stack directly on top of each other, hiding the earlier handoffs and making their Open/Dismiss controls inaccessible. Render only one card or give the collection a non-overlapping layout.
| command, | ||
| threadId: command.threadId, | ||
| }); | ||
| const requestingTurnId = command.requestingTurnId ?? thread.session?.activeTurnId; |
There was a problem hiding this comment.
🟡 Medium orchestration/decider.ts:1240
A client can bind thread.handoff.request to an old or arbitrary requestingTurnId, leaving the durable handoff permanently pending when that turn's completion has already been ingested and no future thread.handoff.turn-settle can resolve it. Reject an explicitly supplied ID unless it matches the current active turn, while retaining the completed-turn exception for availableImmediately requests.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/decider.ts around line 1240:
A client can bind `thread.handoff.request` to an old or arbitrary `requestingTurnId`, leaving the durable handoff permanently `pending` when that turn's completion has already been ingested and no future `thread.handoff.turn-settle` can resolve it. Reject an explicitly supplied ID unless it matches the current active turn, while retaining the completed-turn exception for `availableImmediately` requests.
| }, [interruptThreadTurn, selectedThread]); | ||
| const handleOpenHandoff = useCallback( | ||
| async (handoff: ReturnType<typeof findThreadHandoffs>[number]) => { | ||
| if (!selectedThread || handoffActionId !== null) { |
There was a problem hiding this comment.
🟡 Medium threads/ThreadRouteScreen.tsx:524
Two rapid Open/Dismiss presses both observe handoffActionId === null before React commits the state update, so they dispatch concurrent handoff commands; duplicate Open presses can create different target threads, while Open/Dismiss can race and produce a spurious failure or navigation. Use a synchronously updated ref/lock before awaiting either command.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/mobile/src/features/threads/ThreadRouteScreen.tsx around line 524:
Two rapid Open/Dismiss presses both observe `handoffActionId === null` before React commits the state update, so they dispatch concurrent handoff commands; duplicate Open presses can create different target threads, while Open/Dismiss can race and produce a spurious failure or navigation. Use a synchronously updated ref/lock before awaiting either command.
|
Note 🤖 GPT-5.6 Sol responding on behalf of Theo Closing this PR after an automated pass over open pull requests. Adds an unsolicited product feature or cross-system architecture change that exceeds the contribution policy. |
2 participants
Summary
Adds a durable, agent-requested thread handoff lifecycle.
Why
Agents can now prepare a new-thread continuation without creating or starting it themselves; users retain the final decision to open, edit, and send the target draft.
Validation
git diff --checkBuilt with gpt-5.6-terra via Codex.
Note
Add agent-requested thread handoffs across server, client, and mobile
request → available → accept/dismiss/cancel) via new orchestration commands, server-side decider logic, and contract schemas inpackages/contracts/src/orchestration.ts.request_thread_handoffMCP tool so AI providers can programmatically hand off to a new thread with a title, prompt, and artifact references.ProviderRuntimeIngestiondetects pending handoffs and dispatchesthread.handoff.turn-settleto resolve them.ChatViewand mobileThreadRouteScreensurface available handoffs as cards with Open/Dismiss actions and auto-seed the composer prompt when a handoff is accepted.seededHandoffIdsso a cleared handoff prompt is not reinserted on reload.📊 Macroscope summarized d8fac27. 19 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.