Merge PR #8226: fix(advisor): accept silent Gemini reviews (@roboomp)
This commit is contained in:
@@ -10,6 +10,7 @@
|
||||
- Fixed Cursor sessions double-executing settled tools when `tools.format` is an owned dialect (e.g. `gemini`): `wrapInbandToolStream` rebuilt toolCall blocks without copying `kCursorExecResolved`, so agent-loop re-ran bash/grep/todo and appended a second result for the same call id.
|
||||
- Fixed Codex Responses Lite requests for opaque model codenames such as Daybreak omitting the required `reasoning.context: "all_turns"` value and failing with HTTP 400.
|
||||
- Fixed Cursor personal usage reporting for current Pro / Pro+ / Ultra `/api/usage-summary` payloads that expose `individualUsage.plan` (and optional `onDemand`) instead of the older `individualUsage.overall` bucket ([#7998](https://github.com/can1357/oh-my-pi/pull/7998) by [@dnth](https://github.com/dnth)).
|
||||
- Allowed passive Google callers to accept empty or thinking-only `STOP` responses as successful silence instead of exhausting the provider's empty-response retry budget. ([#8223](https://github.com/can1357/oh-my-pi/issues/8223))
|
||||
|
||||
## [17.2.12] - 2026-08-08
|
||||
|
||||
|
||||
@@ -660,7 +660,9 @@ export const streamGoogleGeminiCli: StreamFunction<"google-gemini-cli"> = (
|
||||
sawFinishReason = false;
|
||||
};
|
||||
|
||||
const streamResponse = async (activeResponse: Response): Promise<boolean> => {
|
||||
const streamResponse = async (
|
||||
activeResponse: Response,
|
||||
): Promise<{ meaningful: boolean; strippedPlanningLeak: boolean }> => {
|
||||
if (!activeResponse.body) {
|
||||
throw new AIError.ProviderResponseError("No response body", {
|
||||
provider: model.provider,
|
||||
@@ -680,6 +682,7 @@ export const streamGoogleGeminiCli: StreamFunction<"google-gemini-cli"> = (
|
||||
let isBuffering = false;
|
||||
let textBuffer = "";
|
||||
let bufferedTextSignature: string | undefined;
|
||||
let strippedPlanningLeak = false;
|
||||
|
||||
const endCurrentBlock = (): void => {
|
||||
if (!currentBlock) return;
|
||||
@@ -835,6 +838,7 @@ export const streamGoogleGeminiCli: StreamFunction<"google-gemini-cli"> = (
|
||||
if (isBuffering) {
|
||||
const buffered = consumePlanningBuffer(textBuffer, toolNames);
|
||||
if (buffered.kind !== "incomplete") {
|
||||
if (buffered.kind === "leak") strippedPlanningLeak = true;
|
||||
const visibleSignature = bufferedTextSignature;
|
||||
isBuffering = false;
|
||||
textBuffer = "";
|
||||
@@ -915,6 +919,7 @@ export const streamGoogleGeminiCli: StreamFunction<"google-gemini-cli"> = (
|
||||
const buffered = consumePlanningBuffer(textBuffer, toolNames, true);
|
||||
|
||||
if (buffered.kind !== "incomplete") {
|
||||
if (buffered.kind === "leak") strippedPlanningLeak = true;
|
||||
feedVisibleText(buffered.visibleText, bufferedTextSignature);
|
||||
}
|
||||
bufferedTextSignature = undefined;
|
||||
@@ -925,7 +930,10 @@ export const streamGoogleGeminiCli: StreamFunction<"google-gemini-cli"> = (
|
||||
flushVisibleText(bufferedTextSignature);
|
||||
endCurrentBlock();
|
||||
|
||||
return hasMeaningfulGoogleContent(output);
|
||||
return {
|
||||
meaningful: hasMeaningfulGoogleContent(output),
|
||||
strippedPlanningLeak,
|
||||
};
|
||||
};
|
||||
|
||||
let receivedContent = false;
|
||||
@@ -1018,8 +1026,14 @@ export const streamGoogleGeminiCli: StreamFunction<"google-gemini-cli"> = (
|
||||
}
|
||||
|
||||
const streamed = await streamResponse(currentResponse);
|
||||
if (output.stopReason !== "stop" || streamed) {
|
||||
receivedContent = streamed;
|
||||
// Only accept an empty STOP as valid silence once every fallback
|
||||
// endpoint is exhausted: an earlier endpoint returning empty
|
||||
// successful streams must still fail over (Antigravity auto mode)
|
||||
// rather than be recorded as a real silent review.
|
||||
const acceptedSilence =
|
||||
options?.acceptEmptyResponse === true && !streamed.strippedPlanningLeak && isLastEndpoint;
|
||||
if (output.stopReason !== "stop" || streamed.meaningful || acceptedSilence) {
|
||||
receivedContent = streamed.meaningful || acceptedSilence;
|
||||
break;
|
||||
}
|
||||
|
||||
|
||||
@@ -1031,7 +1031,13 @@ export function streamGoogleGenAI<T extends "google-generative-ai" | "google-ver
|
||||
},
|
||||
});
|
||||
|
||||
if (output.stopReason !== "stop" || hasMeaningfulGoogleContent(output)) break;
|
||||
if (
|
||||
output.stopReason !== "stop" ||
|
||||
hasMeaningfulGoogleContent(output) ||
|
||||
options?.acceptEmptyResponse === true
|
||||
) {
|
||||
break;
|
||||
}
|
||||
if (emptyAttempt >= MAX_EMPTY_STREAM_RETRIES) {
|
||||
throw new AIError.ProviderResponseError(
|
||||
`Google API returned an empty response (finishReason STOP with no content) after ${MAX_EMPTY_STREAM_RETRIES + 1} attempts`,
|
||||
|
||||
@@ -78,6 +78,7 @@ const ALLOWED_OPTION_KEYS: ReadonlySet<keyof SimpleStreamOptions> = new Set([
|
||||
"preferWebsockets",
|
||||
"openrouterVariant",
|
||||
"loopGuard",
|
||||
"acceptEmptyResponse",
|
||||
] as const satisfies readonly (keyof SimpleStreamOptions)[]);
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -1508,6 +1508,7 @@ function mapOptionsForApi<TApi extends Api>(
|
||||
execHandlers: options?.execHandlers,
|
||||
fetch: options?.fetch,
|
||||
fallbacks: options?.fallbacks,
|
||||
acceptEmptyResponse: options?.acceptEmptyResponse,
|
||||
...simpleProviderOptions,
|
||||
};
|
||||
|
||||
|
||||
@@ -552,6 +552,13 @@ export interface StreamOptions {
|
||||
* Optional retry delay hook for tests and transports that need custom scheduling.
|
||||
*/
|
||||
providerRetryWait?: (delayMs: number, signal?: AbortSignal) => Promise<void>;
|
||||
/**
|
||||
* Accept a Google `STOP` response with no visible text or tool call as a
|
||||
* successful completion. Passive callers such as advisors use this because
|
||||
* silence is a valid result; interactive agent turns retain empty-response
|
||||
* retries by default. Ignored by non-Google providers.
|
||||
*/
|
||||
acceptEmptyResponse?: boolean;
|
||||
/**
|
||||
* Optional `fetch` implementation override. Providers route every HTTP
|
||||
* request — direct calls, SDK clients, and retry helpers — through this
|
||||
|
||||
@@ -145,6 +145,15 @@ describe("pi-native parseRequest", () => {
|
||||
expect(parsed.options.loopGuard).toEqual({ enabled: false });
|
||||
});
|
||||
|
||||
it("forwards acceptEmptyResponse so a passive Google advisor can accept silence server-side", () => {
|
||||
const parsed = parseRequest({
|
||||
modelId: "google/gemini-3.6-flash",
|
||||
context: baseContext,
|
||||
options: { acceptEmptyResponse: true },
|
||||
});
|
||||
expect(parsed.options.acceptEmptyResponse).toBe(true);
|
||||
});
|
||||
|
||||
it("forwards an explicit statefulResponses disablement to the native stream", () => {
|
||||
const parsed = parseRequest({
|
||||
modelId: "openai/gpt-5",
|
||||
|
||||
@@ -134,6 +134,25 @@ describe("Google empty-response retry (public + Vertex path)", () => {
|
||||
expect(result.errorMessage).toContain("empty response");
|
||||
});
|
||||
|
||||
it("accepts an empty STOP when silence is a valid caller result", async () => {
|
||||
let calls = 0;
|
||||
const fetchMock: FetchImpl = async () => {
|
||||
calls += 1;
|
||||
return sse(genaiChunk(""));
|
||||
};
|
||||
|
||||
const stream = streamGoogle(genaiModel, context, {
|
||||
apiKey: "k",
|
||||
fetch: fetchMock,
|
||||
acceptEmptyResponse: true,
|
||||
});
|
||||
const result = await stream.result();
|
||||
|
||||
expect(calls).toBe(1);
|
||||
expect(result.stopReason).toBe("stop");
|
||||
expect(result.errorMessage).toBeUndefined();
|
||||
});
|
||||
|
||||
it("filters out empty text parts at stream end but preserves terminal thought signatures", async () => {
|
||||
const chunks = [
|
||||
{ candidates: [{ content: { parts: [{ text: "Hello" }] } }] },
|
||||
@@ -254,7 +273,26 @@ describe("Google empty-response retry (Cloud Code Assist path)", () => {
|
||||
void events;
|
||||
});
|
||||
|
||||
it("retries after discarding a planning leak and delivers one structured function call", async () => {
|
||||
it("accepts an empty STOP when silence is a valid caller result", async () => {
|
||||
let calls = 0;
|
||||
const fetchMock: FetchImpl = async () => {
|
||||
calls += 1;
|
||||
return sse(ccaChunk(""));
|
||||
};
|
||||
|
||||
const stream = streamGoogleGeminiCli(cliModel, context, {
|
||||
apiKey: JSON.stringify({ token: "token", projectId: "proj-123" }),
|
||||
fetch: fetchMock,
|
||||
acceptEmptyResponse: true,
|
||||
});
|
||||
const result = await stream.result();
|
||||
|
||||
expect(calls).toBe(1);
|
||||
expect(result.stopReason).toBe("stop");
|
||||
expect(result.errorMessage).toBeUndefined();
|
||||
});
|
||||
|
||||
it("retries a stripped planning leak when empty STOPs are accepted", async () => {
|
||||
let calls = 0;
|
||||
const fetchMock: FetchImpl = async () => {
|
||||
calls += 1;
|
||||
@@ -280,6 +318,7 @@ describe("Google empty-response retry (Cloud Code Assist path)", () => {
|
||||
const stream = streamGoogleGeminiCli(cliModel, context, {
|
||||
apiKey: JSON.stringify({ token: "token", projectId: "proj-123" }),
|
||||
fetch: fetchMock,
|
||||
acceptEmptyResponse: true,
|
||||
});
|
||||
const { events, starts } = await drain(stream);
|
||||
const result = await stream.result();
|
||||
@@ -325,6 +364,34 @@ describe("Google empty-response retry (Cloud Code Assist path)", () => {
|
||||
expect(textOf(result)).toBe("Recovered.");
|
||||
});
|
||||
|
||||
it("exhausts Antigravity auto failover before accepting silence", async () => {
|
||||
const requestedEndpoints: string[] = [];
|
||||
const fetchMock: FetchImpl = async input => {
|
||||
const endpoint = endpointFromInput(input);
|
||||
requestedEndpoints.push(endpoint);
|
||||
return withResponseUrl(sse(ccaChunk("")), endpoint);
|
||||
};
|
||||
|
||||
const stream = streamGoogleGeminiCli(antigravityModel, context, {
|
||||
apiKey: JSON.stringify({ token: "token", projectId: "proj-123" }),
|
||||
antigravityEndpointMode: "auto",
|
||||
acceptEmptyResponse: true,
|
||||
fetch: fetchMock,
|
||||
});
|
||||
const result = await stream.result();
|
||||
|
||||
// Daily still burns its empty-response budget and fails over; only the
|
||||
// last (sandbox) endpoint records the empty STOP as valid silence.
|
||||
expect(requestedEndpoints).toEqual([
|
||||
ANTIGRAVITY_DAILY_ENDPOINT,
|
||||
ANTIGRAVITY_DAILY_ENDPOINT,
|
||||
ANTIGRAVITY_DAILY_ENDPOINT,
|
||||
ANTIGRAVITY_SANDBOX_ENDPOINT,
|
||||
]);
|
||||
expect(result.stopReason).toBe("stop");
|
||||
expect(result.errorMessage).toBeUndefined();
|
||||
});
|
||||
|
||||
for (const { mode, endpoint } of [
|
||||
{ mode: "production", endpoint: ANTIGRAVITY_DAILY_ENDPOINT },
|
||||
{ mode: "sandbox", endpoint: ANTIGRAVITY_SANDBOX_ENDPOINT },
|
||||
|
||||
@@ -44,6 +44,9 @@
|
||||
|
||||
- Removed the `resolveAgentModelSource` model-resolver export, whose only use was being fed to `resolveExplicitModelRole`. Replaced by `resolveAgentModelSelection`, which returns the expanded `patterns` and the pre-expansion `role` together so a spawn path cannot derive one without the other ([#7910](https://github.com/can1357/oh-my-pi/pull/7910) by [@enieuwy](https://github.com/enieuwy)).
|
||||
- A run is now attributed to the model that actually produced its output, not whichever model the session was last pointed at. A retry fallback that errored on its first request — an exhausted quota, a hard provider error — was credited with the whole run in the Agent Hub row and the settled task result, even when the previous model did every turn. Sessions expose the serving model directly, holding the last model that produced output while a candidate is armed but unproven, and transcript-derived history stops at the newest turn that produced output.
|
||||
### Fixed
|
||||
|
||||
- Fixed Gemini advisors treating a valid silent review as an empty-response failure, repeatedly retrying the turn and eventually dropping the advisor backlog. ([#8223](https://github.com/can1357/oh-my-pi/issues/8223))
|
||||
|
||||
## [17.2.12] - 2026-08-08
|
||||
|
||||
|
||||
@@ -785,14 +785,22 @@ export class SessionAdvisors {
|
||||
mcpResources: this.#advisorMcpResources,
|
||||
});
|
||||
const baseAdvisorStreamFn = this.#advisorStreamFn ?? streamSimple;
|
||||
const advisorStreamFn: StreamFn = (requestModel, context, options) =>
|
||||
baseAdvisorStreamFn(
|
||||
requestModel,
|
||||
context,
|
||||
requestModel.api === "openai-codex-responses"
|
||||
? { ...options, codexSseMaxAttempts: ADVISOR_CODEX_SSE_MAX_ATTEMPTS }
|
||||
: options,
|
||||
);
|
||||
const advisorStreamFn: StreamFn = (requestModel, context, options) => {
|
||||
if (requestModel.api === "openai-codex-responses") {
|
||||
return baseAdvisorStreamFn(requestModel, context, {
|
||||
...options,
|
||||
codexSseMaxAttempts: ADVISOR_CODEX_SSE_MAX_ATTEMPTS,
|
||||
});
|
||||
}
|
||||
if (
|
||||
requestModel.api === "google-generative-ai" ||
|
||||
requestModel.api === "google-gemini-cli" ||
|
||||
requestModel.api === "google-vertex"
|
||||
) {
|
||||
return baseAdvisorStreamFn(requestModel, context, { ...options, acceptEmptyResponse: true });
|
||||
}
|
||||
return baseAdvisorStreamFn(requestModel, context, options);
|
||||
};
|
||||
const advisorAgent = new Agent({
|
||||
initialState: {
|
||||
systemPrompt,
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import { Agent, type StreamFn } from "@oh-my-pi/pi-agent-core";
|
||||
import { type FetchImpl, streamSimple } from "@oh-my-pi/pi-ai";
|
||||
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||||
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
test("keeps Gemini 3.6 advisor context and accepts a silent review", async () => {
|
||||
const temp = TempDir.createSync("@issue-8223-");
|
||||
const auth = await AuthStorage.create(path.join(temp.path(), "auth.db"));
|
||||
auth.setRuntimeApiKey("google", "test-key");
|
||||
const registry = new ModelRegistry(auth);
|
||||
const model = getBundledModel("google", "gemini-3.6-flash");
|
||||
if (!model) throw new Error("missing bundled model");
|
||||
const bodies: unknown[] = [];
|
||||
const fetchMock: FetchImpl = async (_input, init) => {
|
||||
bodies.push(JSON.parse(String(init?.body)));
|
||||
const chunk = {
|
||||
candidates: [
|
||||
{
|
||||
content: { role: "model", parts: [{ thought: true, text: "Analyzing only" }] },
|
||||
finishReason: "STOP",
|
||||
},
|
||||
],
|
||||
usageMetadata: {
|
||||
promptTokenCount: 10,
|
||||
candidatesTokenCount: 5,
|
||||
thoughtsTokenCount: 5,
|
||||
totalTokenCount: 15,
|
||||
},
|
||||
};
|
||||
return new Response(`data: ${JSON.stringify(chunk)}\n\n`, {
|
||||
status: 200,
|
||||
headers: { "content-type": "text/event-stream" },
|
||||
});
|
||||
};
|
||||
const advisorStreamFn: StreamFn = (requestModel, context, options) =>
|
||||
streamSimple(requestModel, context, { ...options, fetch: fetchMock });
|
||||
const agent = new Agent({ initialState: { model, systemPrompt: ["Primary"], tools: [] } });
|
||||
const session = new AgentSession({
|
||||
agent,
|
||||
sessionManager: SessionManager.create(temp.path(), temp.path()),
|
||||
settings: Settings.isolated({ "compaction.enabled": false }),
|
||||
modelRegistry: registry,
|
||||
advisorTools: [],
|
||||
advisorStreamFn,
|
||||
});
|
||||
try {
|
||||
session.settings.setModelRole("advisor", "google/gemini-3.6-flash");
|
||||
expect(session.setAdvisorEnabled(true)).toBe(true);
|
||||
const advisor = session.getAdvisorAgent();
|
||||
if (!advisor) throw new Error("advisor did not start");
|
||||
await advisor.prompt("### Session update [in progress — more steps follow]\nImplement an order book.");
|
||||
expect(advisor.state.error).toBeUndefined();
|
||||
expect(bodies).toHaveLength(1);
|
||||
expect(bodies[0]).toMatchObject({
|
||||
systemInstruction: {
|
||||
parts: [{ text: expect.stringContaining("You bring a different angle") }],
|
||||
},
|
||||
tools: [
|
||||
{
|
||||
functionDeclarations: [
|
||||
{
|
||||
name: "advise",
|
||||
description: expect.stringContaining("Send one concrete"),
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
});
|
||||
} finally {
|
||||
await session.dispose();
|
||||
auth.close();
|
||||
await temp.remove();
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user