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
11 changes: 11 additions & 0 deletions packages/cli/src/acp-integration/session/Session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ describe('Session', () => {
subscribe: ReturnType<typeof vi.fn>;
beginTurn: ReturnType<typeof vi.fn>;
releaseTurn: ReturnType<typeof vi.fn>;
markTurnDelivered: ReturnType<typeof vi.fn>;
permitForTurn: ReturnType<typeof vi.fn>;
getVerifierFeedback: ReturnType<typeof vi.fn>;
finishTurn: ReturnType<typeof vi.fn>;
Expand Down Expand Up @@ -759,6 +760,7 @@ describe('Session', () => {
subscribe: vi.fn().mockReturnValue(() => {}),
beginTurn: vi.fn(),
releaseTurn: vi.fn().mockResolvedValue(false),
markTurnDelivered: vi.fn(),
permitForTurn: vi.fn(),
getVerifierFeedback: vi.fn(),
finishTurn: vi.fn().mockResolvedValue(undefined),
Expand Down Expand Up @@ -19600,6 +19602,7 @@ describe('Session', () => {
await boundGoalHost!.startGoalTurn({
permit,
continuationContext: 'check weather',
objectiveUpdated: true,
windDown: true,
verifierFeedback: 'Need independent evidence',
});
Expand Down Expand Up @@ -19631,6 +19634,11 @@ describe('Session', () => {
'not evidence that the user supplied it',
),
}),
expect.objectContaining({
text: expect.stringContaining(
'The Goal objective changed since your last turn',
),
}),
expect.objectContaining({
text: expect.stringContaining(
'The autonomous token budget for this Goal window is spent.',
Expand All @@ -19646,6 +19654,9 @@ describe('Session', () => {
expect.any(String),
permit,
);
expect(mockGoalRuntime.markTurnDelivered).toHaveBeenCalledWith(
'goal-runtime:turn-1',
);
expect(
mockChatRecordingService.recordGoalRuntimeMessage,
).toHaveBeenCalledWith(expect.any(Array), permit);
Expand Down
32 changes: 31 additions & 1 deletion packages/cli/src/acp-integration/session/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ interface AcpGoalTurn {
controller: AbortController;
origin: 'runtime' | 'user';
continuationContext: string;
objectiveUpdated?: boolean;
windDown?: boolean;
verifierFeedback?: string;
modelStarted: boolean;
Expand Down Expand Up @@ -2151,6 +2152,9 @@ export class Session implements SessionContext {
controller: new AbortController(),
origin: 'runtime',
continuationContext: input.continuationContext,
...(input.objectiveUpdated
? { objectiveUpdated: input.objectiveUpdated }
: {}),
...(input.windDown ? { windDown: true } : {}),
...(input.verifierFeedback
? { verifierFeedback: input.verifierFeedback }
Expand Down Expand Up @@ -2414,6 +2418,27 @@ export class Session implements SessionContext {
}
}

/**
* Confirms the continuation's prompt reached the model.
*
* `startGoalTurn` resolves at enqueue time, so the runtime cannot tell a
* delivered turn from a queued one when it settles; `#settleGoalTurn`'s
* degraded-persistence fallback settles a model-started turn through
* `releaseTurn`, and only this confirmation keeps that turn's objective
* announcement from rolling back and re-firing on the next continuation.
*/
#markGoalTurnDelivered(turnKey: string): void {
try {
this.config.getGoalRuntime().markTurnDelivered(turnKey);
} catch (error) {
debugLogger.debug(
`Failed to confirm ACP Goal turn delivery: ${
error instanceof Error ? error.message : String(error)
}`,
);
}
}

async #settleGoalTurn(
turn: AcpGoalTurn,
result: PromptResponse | undefined,
Expand Down Expand Up @@ -5495,7 +5520,12 @@ export class Session implements SessionContext {
// a completed iteration — a phantom turn on the goal's
// count and a checkpoint recording work that never ran.
// Re-assigning on later loop laps is harmless.
if (goalTurn) goalTurn.modelStarted = true;
if (goalTurn) {
goalTurn.modelStarted = true;
if (goalTurn.origin === 'runtime') {
this.#markGoalTurnDelivered(goalTurn.turnKey);
}
}
const sendResult =
await this.#sendMessageStreamWithAutoCompression(
promptId,
Expand Down
154 changes: 154 additions & 0 deletions packages/cli/src/nonInteractiveCli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,160 @@ describe('runNonInteractive', () => {
);
});

it('carries the objective-updated notice into a scheduled Goal continuation', async () => {
setupMetricsMock();
mockGetCommands.mockReturnValue([goalCommand]);
await prepareGoalState('paused');
mockFinishedGoalWorker();
const deliveredSpy = vi.spyOn(goalRuntime, 'markTurnDelivered');
vi.mocked(mockConfig.bindGoalTurnHost).mockImplementation((host) =>
goalRuntime.bindHost({
startGoalTurn: (input) =>
host.startGoalTurn({
...input,
objectiveUpdated: true,
}),
preemptGoalTurn: (reason) => host.preemptGoalTurn(reason),
}),
);

await runNonInteractive(
mockConfig,
mockSettings,
'/goal resume',
'goal-runtime-notice',
);

expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledOnce();
const [parts, , , options] =
mockGeminiClient.sendMessageStream.mock.calls[0]!;
expect(parts[0]?.text).toContain(
'The Goal objective changed since your last turn',
);
expect(deliveredSpy).toHaveBeenCalledWith(options.goalTurnKey);
});

it('marks a follow-up continuation delivered when the finished turn schedules it', async () => {
// The first segment finishes through the real runtime, which schedules
// the next continuation into the headless queue; the promotion site has
// to mark that turn delivered before its prompt goes out, or a later
// fail-closed settle would roll its announcement back and re-fire the
// notice.
setupMetricsMock();
mockGetCommands.mockReturnValue([goalCommand]);
await prepareGoalState('paused');
const realFinishTurn = goalRuntime.finishTurn.bind(goalRuntime);
const finishTurn = vi
.spyOn(goalRuntime, 'finishTurn')
.mockImplementationOnce(realFinishTurn)
.mockResolvedValue(undefined);
mockGeminiClient.sendMessageStream.mockImplementation(() =>
createStreamFromEvents([
{
type: GeminiEventType.Finished,
value: {
reason: undefined,
usageMetadata: { totalTokenCount: 0 },
},
},
]),
);
const deliveredSpy = vi.spyOn(goalRuntime, 'markTurnDelivered');

await runNonInteractive(
mockConfig,
mockSettings,
'/goal resume',
'goal-runtime-promoted',
);

expect(finishTurn).toHaveBeenCalledTimes(2);
expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledTimes(2);
const sentKeys = mockGeminiClient.sendMessageStream.mock.calls.map(
(call) => call[3].goalTurnKey,
);
expect(sentKeys[1]).not.toBe(sentKeys[0]);
expect(deliveredSpy.mock.calls.map((call) => call[0])).toEqual(sentKeys);
// The promoted turn was marked before its prompt was sent.
expect(deliveredSpy.mock.invocationCallOrder[1]!).toBeLessThan(
mockGeminiClient.sendMessageStream.mock.invocationCallOrder[1]!,
);
});

it('marks a follow-up continuation delivered after a tool-terminated segment', async () => {
// Same promotion, other branch: the first segment ends because a tool
// result terminated the turn (an update_goal proposal), and the real
// runtime schedules the next continuation from there.
setupMetricsMock();
mockGetCommands.mockReturnValue([goalCommand]);
await prepareGoalState('paused');
const realFinishTurn = goalRuntime.finishTurn.bind(goalRuntime);
const finishTurn = vi
.spyOn(goalRuntime, 'finishTurn')
.mockImplementationOnce(realFinishTurn)
.mockResolvedValue(undefined);
mockCoreExecuteToolCall.mockResolvedValue({
callId: 'update-goal-promoted',
responseParts: [{ text: 'proposal recorded' }],
resultDisplay: 'proposal recorded',
error: undefined,
errorType: undefined,
terminateTurn: true,
});
const finished = () =>
createStreamFromEvents([
{
type: GeminiEventType.Finished,
value: {
reason: undefined,
usageMetadata: { totalTokenCount: 0 },
},
},
]);
mockGeminiClient.sendMessageStream
.mockImplementationOnce(
(
_parts: Part[],
_signal: AbortSignal,
_promptId: string,
sendOptions: { goalPermit?: GoalTurnPermit },
) =>
createStreamFromEvents([
{
type: GeminiEventType.ToolCallRequest,
value: {
callId: 'update-goal-promoted',
name: 'update_goal',
args: {},
isClientInitiated: false,
prompt_id: 'goal-runtime-promoted-tool',
goalContext: sendOptions.goalPermit,
},
},
]),
)
.mockImplementation(finished);
const deliveredSpy = vi.spyOn(goalRuntime, 'markTurnDelivered');

await runNonInteractive(
mockConfig,
mockSettings,
'/goal resume',
'goal-runtime-promoted-tool',
);

expect(finishTurn).toHaveBeenCalledTimes(2);
expect(mockGeminiClient.sendMessageStream).toHaveBeenCalledTimes(2);
const sentKeys = mockGeminiClient.sendMessageStream.mock.calls.map(
(call) => call[3].goalTurnKey,
);
expect(sentKeys[1]).not.toBe(sentKeys[0]);
expect(deliveredSpy.mock.calls.map((call) => call[0])).toEqual(sentKeys);
expect(deliveredSpy.mock.invocationCallOrder[1]!).toBeLessThan(
mockGeminiClient.sendMessageStream.mock.invocationCallOrder[1]!,
);
});

it('renders the wind-down hand-off on a budget-spent Goal continuation', async () => {
setupMetricsMock();
mockGetCommands.mockReturnValue([goalCommand]);
Expand Down
14 changes: 14 additions & 0 deletions packages/cli/src/nonInteractiveCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ interface HeadlessGoalTurn {
controller: AbortController;
origin: 'runtime' | 'user';
continuationContext: string;
objectiveUpdated?: boolean;
windDown?: boolean;
verifierFeedback?: string;
}
Expand Down Expand Up @@ -632,6 +633,9 @@ export async function runNonInteractive(
controller: new AbortController(),
origin: 'runtime',
continuationContext: input.continuationContext,
...(input.objectiveUpdated
? { objectiveUpdated: input.objectiveUpdated }
: {}),
...(input.windDown ? { windDown: true } : {}),
...(input.verifierFeedback
? { verifierFeedback: input.verifierFeedback }
Expand All @@ -648,6 +652,13 @@ export async function runNonInteractive(
const bindGoalHost = () => {
goalHostUnbind ??= config.bindGoalTurnHost(goalHost);
};
const markGoalTurnDelivered = (turn: HeadlessGoalTurn): void => {
try {
config.getGoalRuntime().markTurnDelivered(turn.turnKey);
} catch {
// Goal runtime is optional during early initialization.
}
};
let settlingGoalTurn: HeadlessGoalTurn | undefined;
let goalTurnSettlement: Promise<void> | undefined;
const failClosedActiveGoalTurn = (reason: string): Promise<void> => {
Expand Down Expand Up @@ -1203,6 +1214,7 @@ export async function runNonInteractive(
'The Goal runtime did not schedule a continuation.',
);
}
markGoalTurnDelivered(activeGoalTurn);
initialPartList = buildGoalContinuationParts(activeGoalTurn);
slashHandled = true;
break;
Expand Down Expand Up @@ -2490,6 +2502,7 @@ export async function runNonInteractive(
const nextGoalTurn = queuedGoalTurns.shift();
if (nextGoalTurn) {
activeGoalTurn = nextGoalTurn;
markGoalTurnDelivered(nextGoalTurn);
Comment thread
qwen-code-dev-bot marked this conversation as resolved.
isFirstGoalSegment = true;
currentMessages = [
{
Expand Down Expand Up @@ -2520,6 +2533,7 @@ export async function runNonInteractive(
const nextGoalTurn = queuedGoalTurns.shift();
if (nextGoalTurn) {
activeGoalTurn = nextGoalTurn;
markGoalTurnDelivered(nextGoalTurn);
Comment thread
qwen-code-dev-bot marked this conversation as resolved.
isFirstGoalSegment = true;
currentMessages = [
{
Expand Down
11 changes: 10 additions & 1 deletion packages/cli/src/ui/hooks/useGeminiStream.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -475,12 +475,17 @@ describe('useGeminiStream', () => {
permit,
turnKey: 'goal-runtime:turn-automatic',
continuationContext: 'continue from the last accepted evidence',
objectiveUpdated: true,
windDown: true,
verifierFeedback: 'show the final verification result',
};
const peekNextUserBatchKey = vi.fn((goalTurnActive?: boolean) =>
goalTurnActive ? undefined : 'message-queue:next-user',
);
const markTurnDelivered = vi.fn();
mockConfig.getGoalRuntime = vi.fn(() => ({
markTurnDelivered,
})) as unknown as ReturnType<Config['getGoalRuntime']>;
const { result, mockSendMessageStream: streamMock } = renderTestHook(
[],
undefined,
Expand Down Expand Up @@ -513,7 +518,8 @@ describe('useGeminiStream', () => {
'<goal_runtime_data>',
`{"goalId":"${permit.goalId}","revision":${permit.revision},"objective":"${goal.continuationContext}"}`,
'</goal_runtime_data>',
'The objective in that data block is the current one and supersedes any earlier Goal objective in this conversation, including one you already started working on.',
'The objective in that data block is the current one and supersedes any other Goal objective text in this conversation.',
'The Goal objective changed since your last turn: the objective above replaces the one you were working on. Stop work that only served the previous objective, and carry over only what also serves this one.',
'The autonomous token budget for this Goal window is spent. This is the final turn before the Goal stops and waits for the user; do not start new work.',
'Deliver a concise hand-off: what was accomplished, citing evidence references from get_goal; what remains; and the one concrete next step. Call update_goal only if the objective is already complete or genuinely blocked on the evidence you have. Then end the turn.',
`Verifier feedback: ${goal.verifierFeedback}`,
Expand All @@ -528,6 +534,9 @@ describe('useGeminiStream', () => {
getQueuedGoalTurnKey: expect.any(Function),
}),
);
expect(markTurnDelivered).toHaveBeenCalledWith(
'goal-runtime:turn-automatic',
);
const options = streamMock.mock.calls[0][3] as {
goalSignal: AbortSignal;
getQueuedGoalTurnKey: () => string | undefined;
Expand Down
8 changes: 8 additions & 0 deletions packages/cli/src/ui/hooks/useGeminiStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3581,6 +3581,7 @@ export const useGeminiStream = (
goalId: queuedGoal.permit.goalId,
revision: queuedGoal.permit.revision,
objective: queuedGoal.continuationContext,
objectiveUpdated: queuedGoal.objectiveUpdated,
windDown: queuedGoal.windDown,
verifierFeedback: queuedGoal.verifierFeedback,
}),
Expand Down Expand Up @@ -3816,6 +3817,13 @@ export const useGeminiStream = (
? { getSteerInput: drainSteerAtBoundary }
: {}),
};
if (submitType === SendMessageType.Goal && goalBinding) {
try {
config.getGoalRuntime().markTurnDelivered(goalBinding.turnKey);
} catch {
// Goal runtime is optional during early initialization.
}
}
const providerSignal = inheritedToolContinuationOwner
? processingSignal
: abortSignal;
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/ui/hooks/useMessageQueue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ describe('useMessageQueue', () => {
const input: Parameters<GoalTurnHost['startGoalTurn']>[0] = {
permit,
continuationContext: 'Continue the active Goal',
objectiveUpdated: true,
windDown: true,
verifierFeedback: 'Need stronger evidence',
};
Expand Down Expand Up @@ -123,6 +124,7 @@ describe('useMessageQueue', () => {
permit,
turnKey: 'goal-runtime:turn-1',
continuationContext: 'Continue the active Goal',
objectiveUpdated: true,
windDown: true,
verifierFeedback: 'Need stronger evidence',
});
Expand Down
Loading
Loading