Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/ade-cli/src/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2296,6 +2296,8 @@ describe("ADE CLI", () => {
"--reasoning-effort",
"xhigh",
"--no-fast",
"--note",
"Focus on the failing handoff tests first.",
]));
expect(handoff.label).toBe("chat handoff");
expect(handoff.steps[0]?.params).toEqual({
Expand All @@ -2310,6 +2312,7 @@ describe("ADE CLI", () => {
reasoningEffort: "xhigh",
fastMode: false,
codexFastMode: false,
handoffNote: "Focus on the failing handoff tests first.",
},
},
});
Expand Down
4 changes: 4 additions & 0 deletions apps/ade-cli/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1521,6 +1521,8 @@ const HELP_BY_COMMAND: Record<string, string> = {
$ ade chat read <session> --limit 20 --text Read recent chat messages
$ ade chat goal <session> --objective "Ship it" Set or inspect a Codex goal
$ ade chat goal <session> --status paused Update a Codex goal status
$ ade chat handoff <session> --model openai/gpt-5.5 --note "focus on tests"
Start a new chat with an extra handoff note
$ ade chat fork <session> --model openai/gpt-5.5
Fork full provider history into a new chat
$ ade chat rewind-files <session> --message <user-message-id> --dry-run
Expand Down Expand Up @@ -6845,6 +6847,7 @@ function buildChatPlan(args: string[]): CliPlan {
const codexApprovalPolicy = readValue(args, ["--codex-approval-policy", "--approval-policy"]);
const codexSandbox = readValue(args, ["--codex-sandbox", "--sandbox"]);
const codexConfigSource = readValue(args, ["--codex-config-source", "--config-source"]);
const handoffNote = readValue(args, ["--handoff-note", "--note"]);
return {
kind: "execute",
label: mode === "fork" ? "chat fork" : "chat handoff",
Expand All @@ -6863,6 +6866,7 @@ function buildChatPlan(args: string[]): CliPlan {
...(codexApprovalPolicy !== null ? { codexApprovalPolicy } : {}),
...(codexSandbox !== null ? { codexSandbox } : {}),
...(codexConfigSource !== null ? { codexConfigSource } : {}),
...(handoffNote !== null ? { handoffNote } : {}),
}),
),
],
Expand Down
26 changes: 26 additions & 0 deletions apps/ade-cli/src/services/sync/syncRemoteCommandService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,32 @@ describe("createSyncRemoteCommandService", () => {
expect(service.getDescriptor("prs.cleanupBranch")?.policy).toEqual({ viewerAllowed: false, queueable: true });
});

it("routes chat.handoff with a trimmed handoff note", async () => {
const handoffSession = vi.fn().mockResolvedValue({
session: { id: "session-2" },
usedFallbackSummary: false,
});
const { service } = createService({
agentChatService: { handoffSession },
});

const result = await service.execute(makePayload("chat.handoff", {
sourceSessionId: " session-1 ",
targetModelId: " openai/gpt-5.5 ",
handoffNote: " Focus the first pass on the drawer regression. ",
}));

expect(result).toEqual({
session: { id: "session-2" },
usedFallbackSummary: false,
});
expect(handoffSession).toHaveBeenCalledWith(expect.objectContaining({
sourceSessionId: "session-1",
targetModelId: "openai/gpt-5.5",
handoffNote: "Focus the first pass on the drawer regression.",
}));
});

it("routes github.publishCurrentProject through the GitHub service with validated args", async () => {
const publishCurrentProject = vi.fn().mockResolvedValue({
state: "pushed",
Expand Down
2 changes: 2 additions & 0 deletions apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -641,10 +641,12 @@ function agentChatParallelLaunchStateKey(projectRoot: string, parentLaneId: stri
}

function parseAgentChatHandoffArgs(value: Record<string, unknown>): AgentChatHandoffArgs {
const handoffNote = asTrimmedString(value.handoffNote);
return {
...(value as AgentChatHandoffArgs),
sourceSessionId: requireString(value.sourceSessionId, "chat.handoff requires sourceSessionId."),
targetModelId: requireString(value.targetModelId, "chat.handoff requires targetModelId.") as AgentChatHandoffArgs["targetModelId"],
...(handoffNote ? { handoffNote } : {}),
Comment on lines +644 to +649

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Whitespace-only handoffNote leaks through the spread.

Line 646 spreads ...(value as AgentChatHandoffArgs), which includes the raw handoffNote from value. When the note is whitespace-only, asTrimmedString returns null, so the conditional on line 649 spreads {} and the untrimmed value from the spread persists. The downstream normalizeHandoffNote re-trims today, but this layer's trimming intent is incomplete.

🔧 Proposed fix
 function parseAgentChatHandoffArgs(value: Record<string, unknown>): AgentChatHandoffArgs {
   const handoffNote = asTrimmedString(value.handoffNote);
   return {
     ...(value as AgentChatHandoffArgs),
     sourceSessionId: requireString(value.sourceSessionId, "chat.handoff requires sourceSessionId."),
     targetModelId: requireString(value.targetModelId, "chat.handoff requires targetModelId.") as AgentChatHandoffArgs["targetModelId"],
-    ...(handoffNote ? { handoffNote } : {}),
+    handoffNote,
   };
 }

This always sets handoffNote to the trimmed string or null, overriding the raw value from the spread. The downstream normalizeHandoffNote already handles null correctly.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handoffNote = asTrimmedString(value.handoffNote);
return {
...(value as AgentChatHandoffArgs),
sourceSessionId: requireString(value.sourceSessionId, "chat.handoff requires sourceSessionId."),
targetModelId: requireString(value.targetModelId, "chat.handoff requires targetModelId.") as AgentChatHandoffArgs["targetModelId"],
...(handoffNote ? { handoffNote } : {}),
const handoffNote = asTrimmedString(value.handoffNote);
return {
...(value as AgentChatHandoffArgs),
sourceSessionId: requireString(value.sourceSessionId, "chat.handoff requires sourceSessionId."),
targetModelId: requireString(value.targetModelId, "chat.handoff requires targetModelId.") as AgentChatHandoffArgs["targetModelId"],
handoffNote,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/ade-cli/src/services/sync/syncRemoteCommandService.ts` around lines 644
- 649, The object built in the handoff payload still carries the raw handoffNote
through the spread in syncRemoteCommandService’s AgentChatHandoffArgs mapping,
so whitespace-only notes can survive even though asTrimmedString already returns
null. Update this construction to explicitly override handoffNote with the
trimmed result (or null) after spreading value, using the existing
requireString/asTrimmedString logic so the returned AgentChatHandoffArgs always
contains the normalized note rather than the original input.

};
}

Expand Down
68 changes: 50 additions & 18 deletions apps/desktop/src/main/services/chat/agentChatService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3623,7 +3623,7 @@ describe("createAgentChatService", () => {
}, { timeout: 2000, interval: 50 });
});

it("sends Codex brief handoff text before syncing the inherited goal", async () => {
it("does not seed Codex brief handoffs as provider goals", async () => {
const { service, sessionService } = createService();
const source = await service.createSession({
laneId: "lane-1",
Expand All @@ -3647,29 +3647,23 @@ describe("createAgentChatService", () => {
});

expect(result.session.provider).toBe("codex");
expect(mockState.sessions.get(result.session.id)?.goal).toBe("No Machine State Polish");

const handoffPayloads = mockState.codexRequestPayloads.slice(handoffStart);
const requestMethods = handoffPayloads.map((payload) => String(payload.method ?? ""));
const turnStartIndex = requestMethods.indexOf("turn/start");
const goalSetIndex = requestMethods.indexOf("thread/goal/set");
expect(turnStartIndex).toBeGreaterThanOrEqual(0);
expect(goalSetIndex).toBeGreaterThan(turnStartIndex);
expect(requestMethods).not.toContain("thread/goal/set");

const turnStartRequest = handoffPayloads[turnStartIndex] as {
params?: { input?: Array<{ text?: unknown }> };
};
const inputText = turnStartRequest.params?.input?.map((entry) => String(entry.text ?? "")).join("\n") ?? "";
expect(inputText).toContain("This message was injected automatically by ADE during a chat handoff.");
expect(inputText).toContain("No Machine State Polish");

const goalSetRequest = handoffPayloads[goalSetIndex] as {
params?: { objective?: unknown };
};
expect(goalSetRequest.params?.objective).toBe("No Machine State Polish");
expect(mockState.sessions.get(result.session.id)?.goal ?? null).toBeNull();
});

it("keeps Codex brief handoff successful when deferred goal seeding throws", async () => {
it("appends an optional user note to a brief handoff prompt", async () => {
const { service, sessionService } = createService();
const source = await service.createSession({
laneId: "lane-1",
Expand All @@ -3681,23 +3675,23 @@ describe("createAgentChatService", () => {
sessionId: source.id,
goal: "No Machine State Polish",
});
mockState.codexResponseOverrides.set("thread/goal/set", () => {
throw new Error("goal seed unavailable");
});

const handoffStart = mockState.codexRequestPayloads.length;
const result = await service.handoffSession({
sourceSessionId: source.id,
targetModelId: "openai/gpt-5.5",
handoffNote: "Focus on the collapsed drawer regression before broader cleanup.",
});

expect(result.session.provider).toBe("codex");
expect(mockState.sessions.get(result.session.id)?.goal).toBe("No Machine State Polish");
const handoffMethods = mockState.codexRequestPayloads
const turnStartRequest = mockState.codexRequestPayloads
.slice(handoffStart)
.map((payload) => String(payload.method ?? ""));
expect(handoffMethods).toContain("turn/start");
expect(handoffMethods).toContain("thread/goal/set");
.find((payload) => payload.method === "turn/start") as {
params?: { input?: Array<{ text?: unknown }> };
} | undefined;
const inputText = turnStartRequest?.params?.input?.map((entry) => String(entry.text ?? "")).join("\n") ?? "";
expect(inputText).toContain("## User handoff note");
expect(inputText).toContain("Focus on the collapsed drawer regression before broader cleanup.");
});

it("forks Codex handoff from the source provider thread without injecting a summary prompt", async () => {
Expand All @@ -3723,6 +3717,7 @@ describe("createAgentChatService", () => {
expect(result.usedFallbackSummary).toBe(false);
expect(result.session.provider).toBe("codex");
expect(result.session.threadId).toBe("forked-thread-1");
expect(mockState.sessions.get(result.session.id)?.goal ?? null).toBeNull();
expect(aiIntegrationService.summarizeTerminal).not.toHaveBeenCalled();
const handoffPayloads = mockState.codexRequestPayloads.slice(handoffStart);
expect(handoffPayloads).toEqual(expect.arrayContaining([
Expand All @@ -3733,10 +3728,47 @@ describe("createAgentChatService", () => {
excludeTurns: true,
}),
}),
expect.objectContaining({
method: "thread/goal/clear",
params: expect.objectContaining({
threadId: "forked-thread-1",
}),
}),
]));
expect(handoffPayloads.some((payload) => payload.method === "turn/start")).toBe(false);
});

it("sends only the user note when forking with a handoff note", async () => {
const { service } = createService();
const source = await service.createSession({
laneId: "lane-1",
provider: "codex",
model: "gpt-5.5",
modelId: "openai/gpt-5.5",
});
source.threadId = "source-thread-1";
mockState.codexResponseOverrides.set("thread/fork", () => ({
thread: { id: "forked-thread-1" },
}));

const handoffStart = mockState.codexRequestPayloads.length;
await service.handoffSession({
sourceSessionId: source.id,
targetModelId: "openai/gpt-5.5",
mode: "fork",
handoffNote: "Start by checking the current test failure, then continue.",
});

const turnStartRequest = mockState.codexRequestPayloads
.slice(handoffStart)
.find((payload) => payload.method === "turn/start") as {
params?: { input?: Array<{ text?: unknown }> };
} | undefined;
const inputText = turnStartRequest?.params?.input?.map((entry) => String(entry.text ?? "")).join("\n") ?? "";
expect(inputText).toContain("Start by checking the current test failure, then continue.");
expect(inputText).not.toContain("This message was injected automatically by ADE during a chat handoff.");
});

it("does not delete files during Codex rewind when git cannot prove the path was absent", async () => {
const { service, sessionService } = createService();
const source = await service.createSession({
Expand Down
82 changes: 50 additions & 32 deletions apps/desktop/src/main/services/chat/agentChatService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1457,6 +1457,15 @@ function trimLine(value: string | null | undefined): string | null {
return trimmed.length ? trimmed : null;
}

function normalizeHandoffNote(value: string | null | undefined): string | null {
const note = trimLine(value);
if (!note) return null;
if (note.length > HANDOFF_NOTE_MAX_CHARS) {
throw new Error(HANDOFF_NOTE_TOO_LONG_MESSAGE);
}
return note;
}

function uniqueNonEmpty(values: Array<string | null | undefined>, limit = values.length): string[] {
const seen = new Set<string>();
const result: string[] = [];
Expand Down Expand Up @@ -1965,6 +1974,8 @@ const CODEX_NO_FIRST_EVENT_WATCHDOG_MS = 120_000;
const CODEX_GOAL_OBJECTIVE_MAX_CHARS = 4_000;
const CODEX_GOAL_OBJECTIVE_REQUIRED_MESSAGE = "Goal text is required.";
const CODEX_GOAL_OBJECTIVE_TOO_LONG_MESSAGE = "Goal is too long. Keep it under 4,000 characters.";
const HANDOFF_NOTE_MAX_CHARS = 4_000;
const HANDOFF_NOTE_TOO_LONG_MESSAGE = "Handoff note is too long. Keep it under 4,000 characters.";
// Idle stream watchdog removed — time-based idle detection produced false
// positives during long-running tool calls (Agent, Bash, etc.) where no
// stream events are emitted while the SDK waits for tool results. The user
Expand Down Expand Up @@ -8104,15 +8115,24 @@ export function createAgentChatService(args: {
}
};

const buildHandoffPrompt = (brief: string): string => {
const buildHandoffPrompt = (brief: string, handoffNote: string | null = null): string => {
return [
"This message was injected automatically by ADE during a chat handoff.",
"You are taking over from a previous ADE work chat that the user is handing off to this new model.",
"Continue the same task in the same lane. Do not restart discovery from scratch unless the brief below is clearly missing a required detail.",
"The user will keep discussing the same work in this new chat.",
handoffNote ? "Apply the user's handoff note below as the highest-priority instruction for this handoff." : null,
"",
brief.trim(),
].join("\n");
handoffNote ? ["", "## User handoff note", handoffNote].join("\n") : null,
].filter((line): line is string => line != null).join("\n");
};

const buildHandoffDisplayText = (handoffNote: string | null): string => {
return [
"Chat handoff from previous session",
handoffNote ? `User note: ${handoffNote}` : null,
].filter((line): line is string => line != null).join("\n\n");
};

const sessionIsManuallyNamed = (managed: ManagedChatSession): boolean => {
Expand Down Expand Up @@ -22032,6 +22052,7 @@ export function createAgentChatService(args: {
const handoffSession = async (args: AgentChatHandoffArgs): Promise<AgentChatHandoffResult> => {
const sourceId = args.sourceSessionId.trim();
const targetId = args.targetModelId.trim();
const handoffNote = normalizeHandoffNote(args.handoffNote);
if (!sourceId.length) {
throw new Error("A source session is required to hand off a chat.");
}
Expand Down Expand Up @@ -22172,45 +22193,42 @@ export function createAgentChatService(args: {
goal: inheritedGoal,
});
};
const deferInheritedGoalUntilHandoffDispatch =
handoffMode === "brief" && createdManaged.session.provider === "codex";
if (!deferInheritedGoalUntilHandoffDispatch) {
if (createdManaged.session.provider !== "codex") {
applyInheritedGoal();
}
persistChatState(createdManaged);

if (handoffMode === "brief") {
if (createdManaged.session.provider === "codex" && createdManaged.session.threadId) {
try {
await sendMessage({
sessionId: created.id,
text: buildHandoffPrompt(brief),
displayText: "Chat handoff from previous session",
metadata: { kind: "handoff", hideFullPrompt: true },
reasoningEffort: targetReasoningEffort,
executionMode: createdManaged.session.executionMode ?? null,
interactionMode: createdManaged.session.interactionMode ?? null,
}, {
awaitDispatch: true,
const runtime = await ensureCodexSessionRuntime(createdManaged);
const threadId = await ensureCodexControlThread(createdManaged, runtime, "handoff goal cleanup");
await runtime.request("thread/goal/clear", {
threadId,
}, { timeoutMs: CODEX_INLINE_COMMAND_TIMEOUT_MS });
clearKnownCodexGoal(createdManaged, runtime);
persistChatState(createdManaged);
} catch (error) {
logger.warn("agent_chat.codex_goal_clear_after_handoff_failed", {
sessionId: createdManaged.session.id,
error: error instanceof Error ? error.message : String(error),
});
} finally {
if (deferInheritedGoalUntilHandoffDispatch) {
applyInheritedGoal();
persistChatState(createdManaged);
if (createdManaged.runtime?.kind === "codex") {
try {
await seedCodexThreadGoalFromSessionGoal(createdManaged, createdManaged.runtime);
} catch (error) {
logger.warn("agent_chat.codex_goal_seed_after_handoff_failed", {
sessionId: createdManaged.session.id,
error: error instanceof Error ? error.message : String(error),
});
persistChatState(createdManaged);
}
}
}
}
}

if (handoffMode === "brief" || handoffNote) {
await sendMessage({
sessionId: created.id,
text: handoffMode === "brief" ? buildHandoffPrompt(brief, handoffNote) : handoffNote!,
displayText: handoffMode === "brief" ? buildHandoffDisplayText(handoffNote) : handoffNote!,
...(handoffMode === "brief" ? { metadata: { kind: "handoff", hideFullPrompt: true } } : {}),
reasoningEffort: targetReasoningEffort,
executionMode: createdManaged.session.executionMode ?? null,
interactionMode: createdManaged.session.interactionMode ?? null,
}, {
awaitDispatch: true,
});
}

return {
session: createdManaged.session,
usedFallbackSummary: handoffMode === "brief" ? usedFallbackSummary : false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3404,13 +3404,17 @@ describe("AgentChatPane submit recovery", () => {
fireEvent.click(await screen.findByRole("button", { name: "Open chat actions drawer" }));
fireEvent.click(await screen.findByRole("button", { name: "Handoff" }));
expect(await screen.findByText("Fork keeps the Codex provider thread history. Brief sends a summary as the first message.")).toBeTruthy();
fireEvent.change(await screen.findByLabelText("Handoff note"), {
target: { value: "Prioritize the drawer regression before broad cleanup." },
});
fireEvent.click(await screen.findByRole("button", { name: "Brief handoff" }));

await waitFor(() => {
expect(handoff).toHaveBeenCalledWith(expect.objectContaining({
sourceSessionId: session.sessionId,
targetModelId: "openai/gpt-5.4-mini",
mode: "brief",
handoffNote: "Prioritize the drawer regression before broad cleanup.",
reasoningEffort: "xhigh",
permissionMode: "default",
claudePermissionMode: "default",
Expand Down
Loading
Loading