mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): let the SDK stream resolve auth for summaries (#11556)
In sessions created by `createAgentSession`, compaction, branch summaries and bug-report summaries resolved the stored OAuth credential in `AgentSession._getSummarizationRequestAuth`, then passed the access token on as `options.apiKey`. The SDK's stream function calls `modelRuntime.streamSimple`, which resolves auth again. Because it was given an explicit `apiKey`, `resolveProviderAuth` took the API-key override branch instead of using stored OAuth. Built-in providers accept the token either way. A provider whose API-key path differs from its OAuth path, such as one that refuses explicit keys, failed every summary while normal prompts worked. `createAgentSession` now marks its stream function with `markRuntimeStreamFunction`. This records it in a module-level `WeakMap` keyed by the function, mapped to its `ModelRuntime`. `_getSummarizationRequestAuth` skips pre-resolution when the agent's stream function is marked for the session's own runtime, so that runtime resolves auth once, as it does for normal prompts. The check runs after virtual-model routing. Bare `streamSimple` and custom `streamFn`s still receive resolved credentials, so the fixes for #6324 and #7579 are unchanged. If a caller replaces `agent.streamFunction`, the old behavior applies. **Testing** - Added `test/suite/regressions/11520-sdk-summary-oauth-auth.test.ts`. It builds an SDK session on a provider that has stored OAuth and also an API-key method, runs a branch summary, and asserts two things: no `getAuth` call carries an `apiKey` override, and the provider receives the OAuth access token. It fails without the change and passes with it. - `npm run check` passes and `./test.sh` passes. I also ran the related tests: 6324 branch-summary auth, compaction model overrides, auto-compaction cancellation, SDK stream options, virtual models, and the compaction and branch-summary tests. - Manual reproduction: an SDK script with an `openai` OAuth credential that logs `ModelRuntime.getAuth` during `navigateTree(..., { summarize: true })`. Before the change there were two calls, the second with `apiKey = <oauth token>`. After the change there is one call, with no override. Fixes #11520 Requested by @badlogic Co-authored-by: triagent <triagent@localhost>
This commit is contained in:
parent
249f24bfc5
commit
6fed3cfb99
4 changed files with 142 additions and 1 deletions
|
|
@ -15,6 +15,7 @@
|
|||
|
||||
### Fixed
|
||||
|
||||
- Fixed compaction, branch summaries, and bug-report summaries in SDK sessions passing the stored OAuth token back to the model runtime as an explicit API key. Provider auth that treats explicit keys differently from stored OAuth now sees the OAuth login, as normal prompts do ([#11520](https://github.com/earendil-works/pi/issues/11520))
|
||||
- Fixed a redundant system prompt update (tools, rules) when an extension set the same active tools in a different order, for example a reversed `pi.setActiveTools()` list. Tools that stay active now keep their order and newly activated tools are appended ([#11495](https://github.com/earendil-works/pi/issues/11495))
|
||||
- Fixed `pi.exec()` crashing pi with `RangeError: Invalid string length` when a command printed more output than fits in a string. The default output cap is the smaller of 256 MiB and the runtime string limit; past the cap, the command is terminated and the result has `truncated: true` and a nonzero exit code ([#11360](https://github.com/earendil-works/pi/issues/11360))
|
||||
- Fixed `pi.exec()` never returning when a command ignored SIGTERM after a timeout or abort. pi now sends SIGKILL 5 seconds later, as intended
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ import {
|
|||
type BeforeToolCallResult,
|
||||
type PrepareNextTurnContext,
|
||||
runToolCall,
|
||||
type StreamFn,
|
||||
type ThinkingLevel,
|
||||
} from "@earendil-works/pi-agent-core";
|
||||
import { contentText, getCurrentSystemMessage, getCurrentTools, retryDelayMs } from "@earendil-works/pi-ai";
|
||||
|
|
@ -158,6 +159,18 @@ import {
|
|||
type VirtualModelStateData,
|
||||
} from "./virtual-models.ts";
|
||||
|
||||
/**
|
||||
* Stream functions that resolve auth through a given ModelRuntime themselves, such as the one
|
||||
* createAgentSession installs. Summaries sent through them must not pass pre-resolved credentials:
|
||||
* the runtime would take them as an explicit API key instead of the stored OAuth login (#11520).
|
||||
*/
|
||||
const runtimeStreamFunctions = new WeakMap<StreamFn, ModelRuntime>();
|
||||
|
||||
/** Mark `streamFn` as resolving auth through `modelRuntime` on its own. */
|
||||
export function markRuntimeStreamFunction(streamFn: StreamFn, modelRuntime: ModelRuntime): void {
|
||||
runtimeStreamFunctions.set(streamFn, modelRuntime);
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Skill Block Parsing
|
||||
// ============================================================================
|
||||
|
|
@ -590,6 +603,9 @@ export class AgentSession {
|
|||
if (this.agent.streamFunction === streamSimple) {
|
||||
return { ...(await this._getRequiredRequestAuth(model, signal)), thinkingLevel };
|
||||
}
|
||||
if (runtimeStreamFunctions.get(this.agent.streamFunction) === this._modelRuntime) {
|
||||
return { model, thinkingLevel };
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await this._modelRuntime.getAuth(model, { signal });
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import type { ModelsSimpleStreamOptions } from "@earendil-works/pi-ai";
|
|||
import { clampThinkingLevel, type Message, type Model, streamSimple } from "@earendil-works/pi-ai/compat";
|
||||
import { getAgentDir } from "../config.ts";
|
||||
import { resolvePath } from "../utils/paths.ts";
|
||||
import { AgentSession } from "./agent-session.ts";
|
||||
import { AgentSession, markRuntimeStreamFunction } from "./agent-session.ts";
|
||||
import { formatNoModelsAvailableMessage } from "./auth-guidance.ts";
|
||||
import { CacheWarmer } from "./cache-warmer.ts";
|
||||
import { getClientIdentity } from "./client-identity.ts";
|
||||
|
|
@ -436,6 +436,8 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {}
|
|||
thinkingBudgets: settingsManager.getThinkingBudgets(),
|
||||
maxRetryDelayMs: settingsManager.getProviderRetrySettings().maxRetryDelayMs,
|
||||
});
|
||||
// This stream resolves auth through modelRuntime, so summaries must not pre-resolve it (#11520).
|
||||
markRuntimeStreamFunction(agent.streamFunction, modelRuntime);
|
||||
|
||||
// Restore missing settings metadata for older sessions.
|
||||
if (hasExistingSession) {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,122 @@
|
|||
import { mkdtempSync, rmSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import {
|
||||
type Api,
|
||||
createAssistantMessageEventStream,
|
||||
type Model,
|
||||
type SimpleStreamOptions,
|
||||
} from "@earendil-works/pi-ai";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { AuthStorage } from "../../../src/core/auth-storage.ts";
|
||||
import { ModelRuntime } from "../../../src/core/model-runtime.ts";
|
||||
import { createAgentSession } from "../../../src/core/sdk.ts";
|
||||
import { SessionManager } from "../../../src/core/session-manager.ts";
|
||||
import { SettingsManager } from "../../../src/core/settings-manager.ts";
|
||||
import { assistantMsg, createTestResourceLoader, userMsg } from "../../utilities.ts";
|
||||
|
||||
// Issue #11520: summaries in SDK sessions passed the resolved OAuth token back to the
|
||||
// ModelRuntime as an explicit API key, so provider auth took the API-key override branch.
|
||||
describe("issue #11520 SDK summary OAuth auth", () => {
|
||||
const tempDirs: string[] = [];
|
||||
|
||||
afterEach(() => {
|
||||
while (tempDirs.length > 0) rmSync(tempDirs.pop()!, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("lets the runtime resolve stored OAuth for branch summaries", async () => {
|
||||
const cwd = mkdtempSync(join(tmpdir(), "pi-11520-"));
|
||||
tempDirs.push(cwd);
|
||||
const credentials = AuthStorage.inMemory({
|
||||
"oauth-only-provider": {
|
||||
type: "oauth",
|
||||
access: "oauth-access",
|
||||
refresh: "refresh",
|
||||
expires: Date.now() + 3_600_000,
|
||||
},
|
||||
});
|
||||
const modelRuntime = await ModelRuntime.create({ credentials, modelsPath: null, allowModelNetwork: false });
|
||||
const model: Model<Api> = {
|
||||
id: "oauth-model",
|
||||
name: "OAuth Model",
|
||||
api: "openai-completions",
|
||||
provider: "oauth-only-provider",
|
||||
baseUrl: "https://example.test/v1",
|
||||
reasoning: false,
|
||||
input: ["text"],
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||
contextWindow: 128000,
|
||||
maxTokens: 4096,
|
||||
};
|
||||
const requests: SimpleStreamOptions[] = [];
|
||||
modelRuntime.registerProvider(model.provider, {
|
||||
name: "OAuth Only",
|
||||
baseUrl: model.baseUrl,
|
||||
api: model.api,
|
||||
// With an API key method as well, an explicit key would bypass the stored OAuth login.
|
||||
apiKey: "UNUSED_ENV_KEY",
|
||||
oauth: {
|
||||
name: "Subscription",
|
||||
login: async () => ({ access: "oauth-access", refresh: "refresh", expires: Date.now() + 60_000 }),
|
||||
refreshToken: async (credential) => credential,
|
||||
getApiKey: (credential) => credential.access,
|
||||
},
|
||||
models: [model],
|
||||
streamSimple: (requestModel, _context, options) => {
|
||||
requests.push(options ?? {});
|
||||
const stream = createAssistantMessageEventStream();
|
||||
stream.end({
|
||||
role: "assistant",
|
||||
content: [{ type: "text", text: "branch summary text" }],
|
||||
api: requestModel.api,
|
||||
provider: requestModel.provider,
|
||||
model: requestModel.id,
|
||||
usage: {
|
||||
input: 1,
|
||||
output: 1,
|
||||
cacheRead: 0,
|
||||
cacheWrite: 0,
|
||||
totalTokens: 2,
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||
},
|
||||
stopReason: "stop",
|
||||
timestamp: Date.now(),
|
||||
});
|
||||
return stream;
|
||||
},
|
||||
});
|
||||
const authOverrides: Array<string | undefined> = [];
|
||||
const getAuth = modelRuntime.getAuth.bind(modelRuntime);
|
||||
modelRuntime.getAuth = (async (...args: Parameters<ModelRuntime["getAuth"]>) => {
|
||||
authOverrides.push(args[1]?.apiKey);
|
||||
return getAuth(...args);
|
||||
}) as ModelRuntime["getAuth"];
|
||||
|
||||
const sessionManager = SessionManager.inMemory(cwd);
|
||||
const { session } = await createAgentSession({
|
||||
cwd,
|
||||
agentDir: cwd,
|
||||
model,
|
||||
modelRuntime,
|
||||
settingsManager: SettingsManager.inMemory(),
|
||||
sessionManager,
|
||||
resourceLoader: createTestResourceLoader(),
|
||||
});
|
||||
try {
|
||||
const targetId = sessionManager.appendMessage(userMsg("first branch"));
|
||||
sessionManager.appendMessage(assistantMsg("first reply"));
|
||||
sessionManager.appendMessage(userMsg("abandoned branch work"));
|
||||
sessionManager.appendMessage(assistantMsg("abandoned reply"));
|
||||
|
||||
const result = await session.navigateTree(targetId, { summarize: true });
|
||||
|
||||
expect(result.summaryEntry?.summary).toContain("branch summary text");
|
||||
expect(requests).toHaveLength(1);
|
||||
expect(requests[0].apiKey).toBe("oauth-access");
|
||||
expect(authOverrides.length).toBeGreaterThan(0);
|
||||
expect(authOverrides.every((apiKey) => apiKey === undefined)).toBe(true);
|
||||
} finally {
|
||||
session.dispose();
|
||||
}
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue