Skip to content

Commit 08991e9

Browse files
authored
Fix Pi Anthropic routing through the firewall (#58313)
1 parent e28ea11 commit 08991e9

4 files changed

Lines changed: 155 additions & 14 deletions

File tree

actions/setup/js/pi_models_json.cjs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,19 +108,18 @@ function buildModelsJSON(options) {
108108
* routing the "openai" provider through /responses keeps tool calling working for
109109
* all reasoning-capable models without requiring workflow authors to opt in.
110110
*
111-
* Other providers (github/copilot, anthropic) keep their existing chat-completions-
112-
* style gateway protocol, which is unaffected by this OpenAI-specific restriction.
113-
* The AWF api-proxy gateway (used here) exposes a normalized chat-completions-style
114-
* surface for every backend it fronts, including anthropic — this is a distinct
115-
* protocol layer from the native "anthropic-messages" api used in no-firewall mode
116-
* (see pi_agent_core_driver.cjs's buildModel), so anthropic intentionally falls
117-
* through to "openai-completions" here rather than "anthropic-messages".
111+
* GitHub/Copilot models keep their chat-completions-style gateway protocol.
112+
* Anthropic uses its native Messages API so the proxy receives /v1/messages and
113+
* can apply Anthropic prompt caching.
118114
*
119115
* @param {string} provider - normalized GH_AW_LLM_PROVIDER value (e.g. "openai", "anthropic", "github")
120116
* @returns {string}
121117
*/
122118
function resolvePiApiForProvider(provider) {
123-
return provider === "openai" || provider === "codex" ? "openai-responses" : "openai-completions";
119+
if (provider === "openai" || provider === "codex") {
120+
return "openai-responses";
121+
}
122+
return provider === "anthropic" ? "anthropic-messages" : "openai-completions";
124123
}
125124

126125
async function main() {

actions/setup/js/pi_models_json.test.cjs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,8 @@ describe("pi_models_json.cjs", () => {
138138
expect(piModelsJson.resolvePiApiForProvider("github")).toBe("openai-completions");
139139
});
140140

141-
it("keeps the anthropic provider on openai-completions (AWF gateway's normalized wire protocol, distinct from native anthropic-messages)", () => {
142-
expect(piModelsJson.resolvePiApiForProvider("anthropic")).toBe("openai-completions");
141+
it("routes the anthropic provider through its native Messages API", () => {
142+
expect(piModelsJson.resolvePiApiForProvider("anthropic")).toBe("anthropic-messages");
143143
});
144144
});
145145

@@ -186,7 +186,7 @@ describe("pi_models_json.cjs", () => {
186186

187187
const written = JSON.parse(fs.readFileSync(path.join(tmpDir, "models.json"), "utf8"));
188188
expect(written.providers["aw-gateway"].baseUrl).toBe("http://api-proxy:10001");
189-
expect(written.providers["aw-gateway"].api).toBe("openai-completions");
189+
expect(written.providers["aw-gateway"].api).toBe("anthropic-messages");
190190
expect(fetchSpy).not.toHaveBeenCalled();
191191
});
192192

actions/setup/js/pi_provider.cjs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,9 @@ function resolveProviderRequestTarget(model) {
119119
case "openai-codex-responses":
120120
return { api, method, url: joinApiUrl(baseUrl, "/responses") };
121121
case "anthropic":
122-
case "anthropic-messages":
123122
return { api, method, url: joinApiUrl(baseUrl, "/messages") };
123+
case "anthropic-messages":
124+
return { api, method, url: joinApiUrl(baseUrl, "/v1/messages") };
124125
case "mistral-conversations":
125126
return { api, method, url: joinApiUrl(baseUrl, "/conversations") };
126127
default:
@@ -306,11 +307,14 @@ function piProviderExtension(pi) {
306307
const log = DEFAULT_LOGGER;
307308
/** @type {{ api: string, method: string, url: string }|null} */
308309
let lastProviderRequest = null;
309-
/** @type {{ status: number, responseHeaders: string }|null} */
310+
/** @type {{ status: number, responseHeaders: string, succeeded: boolean }|null} */
310311
let lastProviderResponse = null;
312+
let providerRequestCount = 0;
313+
let successfulProviderResponseCount = 0;
311314
registerConfiguredProviders(pi, log);
312315

313316
pi.on("before_provider_request", (_event, ctx) => {
317+
providerRequestCount += 1;
314318
lastProviderRequest = resolveProviderRequestTarget(ctx && ctx.model);
315319
lastProviderResponse = null;
316320
const provider = ctx?.model?.provider || "(unknown provider)";
@@ -319,10 +323,15 @@ function piProviderExtension(pi) {
319323
});
320324

321325
pi.on("after_provider_response", (event, ctx) => {
326+
const succeeded = event.status >= 200 && event.status < 300;
327+
if (succeeded) {
328+
successfulProviderResponseCount += 1;
329+
}
322330
const request = lastProviderRequest || resolveProviderRequestTarget(ctx && ctx.model);
323331
lastProviderResponse = {
324332
status: event.status,
325333
responseHeaders: formatResponseHeaderNames(event.headers),
334+
succeeded,
326335
};
327336
const provider = ctx?.model?.provider || "(unknown provider)";
328337
const model = ctx?.model?.id || getConfiguredModel() || "(unknown model)";
@@ -340,7 +349,10 @@ function piProviderExtension(pi) {
340349
log(
341350
`provider_error provider=${message.provider || "(unknown provider)"} model=${message.model || "(unknown model)"} api=${request.api} status=${status} method=${request.method} url=${request.url} response_headers=${responseHeaders} error=${JSON.stringify(message.errorMessage)}`
342351
);
343-
emitInfrastructureIncompleteIfNoSafeOutputs(`Pi provider request failed before safe outputs were emitted: ${message.errorMessage}`, log);
352+
if (lastProviderResponse?.succeeded) {
353+
successfulProviderResponseCount -= 1;
354+
lastProviderResponse.succeeded = false;
355+
}
344356
});
345357

346358
pi.on("agent_start", async () => {
@@ -389,6 +401,11 @@ function piProviderExtension(pi) {
389401
});
390402
logReflectFailure({ phase: "agent_end", provider, model, result, logger: log });
391403
}
404+
405+
if (providerRequestCount > 0 && successfulProviderResponseCount === 0) {
406+
emitInfrastructureIncompleteIfNoSafeOutputs(`All ${providerRequestCount} Pi provider requests failed before safe outputs were emitted.`, log);
407+
process.exitCode = 1;
408+
}
392409
});
393410
}
394411

actions/setup/js/pi_provider.test.cjs

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ describe("pi_provider.cjs", () => {
77
let module;
88
let originalEnv;
99
let originalFetch;
10+
let originalExitCode;
1011
let stderrOutput;
1112

1213
beforeEach(async () => {
1314
originalEnv = { ...process.env };
1415
originalFetch = global.fetch;
16+
originalExitCode = process.exitCode;
1517
stderrOutput = [];
1618
vi.spyOn(process.stderr, "write").mockImplementation(msg => {
1719
stderrOutput.push(String(msg));
@@ -23,6 +25,7 @@ describe("pi_provider.cjs", () => {
2325
afterEach(() => {
2426
process.env = originalEnv;
2527
global.fetch = originalFetch;
28+
process.exitCode = originalExitCode;
2629
vi.restoreAllMocks();
2730
});
2831

@@ -116,6 +119,127 @@ describe("pi_provider.cjs", () => {
116119
expect(stderrOutput.some(line => line.includes("provider_response provider=copilot model=claude-sonnet-4 status=503 method=POST url=http://api-proxy:10002/v1/chat/completions response_headers=content-type,x-request-id"))).toBe(true);
117120
});
118121

122+
it("resolves native Anthropic requests to the Messages API", () => {
123+
expect(
124+
module.resolveProviderRequestTarget({
125+
api: "anthropic-messages",
126+
baseUrl: "http://api-proxy:10001",
127+
})
128+
).toEqual({
129+
api: "anthropic-messages",
130+
method: "POST",
131+
url: "http://api-proxy:10001/v1/messages",
132+
});
133+
});
134+
135+
it("keeps legacy Anthropic requests on the compatibility endpoint", () => {
136+
expect(
137+
module.resolveProviderRequestTarget({
138+
api: "anthropic",
139+
baseUrl: "http://anthropic.example.test",
140+
})
141+
).toEqual({
142+
api: "anthropic",
143+
method: "POST",
144+
url: "http://anthropic.example.test/messages",
145+
});
146+
});
147+
148+
it("fails the run when every provider request fails", async () => {
149+
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "pi-provider-"));
150+
process.env.GH_AW_SAFE_OUTPUTS = path.join(tempDir, "outputs.jsonl");
151+
process.env.GH_AW_SAFEOUTPUTS_CLI = "true";
152+
153+
const handlers = {};
154+
const pi = {
155+
registerProvider: vi.fn(),
156+
on: vi.fn((event, handler) => {
157+
handlers[event] = handler;
158+
}),
159+
};
160+
const ctx = {
161+
model: {
162+
provider: "aw-gateway",
163+
id: "claude-sonnet-5",
164+
api: "anthropic-messages",
165+
baseUrl: "http://api-proxy:10001",
166+
},
167+
};
168+
169+
module.default(pi);
170+
await handlers.before_provider_request({ type: "before_provider_request", payload: {} }, ctx);
171+
await handlers.after_provider_response({ type: "after_provider_response", status: 404, headers: {} }, ctx);
172+
await handlers.agent_end();
173+
174+
expect(process.exitCode).toBe(1);
175+
expect(stderrOutput.some(line => line.includes("report_incomplete emitted via safeoutputs CLI"))).toBe(true);
176+
});
177+
178+
it("does not fail the run when a provider request succeeds after a failure", async () => {
179+
const handlers = {};
180+
const pi = {
181+
registerProvider: vi.fn(),
182+
on: vi.fn((event, handler) => {
183+
handlers[event] = handler;
184+
}),
185+
};
186+
const ctx = {
187+
model: {
188+
provider: "aw-gateway",
189+
id: "claude-sonnet-5",
190+
api: "anthropic-messages",
191+
baseUrl: "http://api-proxy:10001",
192+
},
193+
};
194+
195+
module.default(pi);
196+
await handlers.before_provider_request({ type: "before_provider_request", payload: {} }, ctx);
197+
await handlers.after_provider_response({ type: "after_provider_response", status: 503, headers: {} }, ctx);
198+
await handlers.before_provider_request({ type: "before_provider_request", payload: {} }, ctx);
199+
await handlers.after_provider_response({ type: "after_provider_response", status: 200, headers: {} }, ctx);
200+
await handlers.agent_end();
201+
202+
expect(process.exitCode).toBe(originalExitCode);
203+
});
204+
205+
it("fails the run when a successful response ends with a stream error", async () => {
206+
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "pi-provider-"));
207+
process.env.GH_AW_SAFE_OUTPUTS = path.join(tempDir, "outputs.jsonl");
208+
process.env.GH_AW_SAFEOUTPUTS_CLI = "true";
209+
210+
const handlers = {};
211+
const pi = {
212+
registerProvider: vi.fn(),
213+
on: vi.fn((event, handler) => {
214+
handlers[event] = handler;
215+
}),
216+
};
217+
const ctx = {
218+
model: {
219+
provider: "aw-gateway",
220+
id: "claude-sonnet-5",
221+
api: "anthropic-messages",
222+
baseUrl: "http://api-proxy:10001",
223+
},
224+
};
225+
226+
module.default(pi);
227+
await handlers.before_provider_request({}, ctx);
228+
await handlers.after_provider_response({ status: 200, headers: {} }, ctx);
229+
await handlers.message_end({
230+
message: {
231+
role: "assistant",
232+
provider: "aw-gateway",
233+
model: "claude-sonnet-5",
234+
stopReason: "error",
235+
errorMessage: "stream interrupted",
236+
},
237+
});
238+
await handlers.agent_end();
239+
240+
expect(process.exitCode).toBe(1);
241+
});
242+
119243
// Triggers the message_end infrastructure-error handler with a given stand-in
120244
// GH_AW_SAFEOUTPUTS_CLI override ('true' simulates a successful CLI call, 'false'
121245
// simulates a failed one) and returns the handlers/stderr output for assertions.
@@ -154,6 +278,7 @@ describe("pi_provider.cjs", () => {
154278
errorMessage: "Connection error.",
155279
},
156280
});
281+
await handlers.agent_end();
157282
}
158283

159284
it("logs assistant inference errors with the last request target", async () => {

0 commit comments

Comments
 (0)