From f079245accf118dfae5478aad809e849842ce86d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 16 Jul 2026 20:20:59 -0700 Subject: [PATCH] fix(engine): defensive getAuth guard on session routing seam + realign pi tests to ModelRuntime seam (#2261) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What / Why **Reworked after FN-8179 (`fd43a57a4`) landed on `main`.** FN-8179 did most of what the original PR #2261 did — pinned `@earendil-works/pi-ai` / `pi-coding-agent` to `^0.80.10` everywhere, added the `createSessionOptions` `NonNullable` typing in `pi.ts`, and regenerated `pnpm-lock.yaml`. This PR was rebased onto current `origin/main` and reduced to **only the unique residual not covered by FN-8179**. ### Residual change 1 — defensive `getAuth` guard The FN-8142 migration rewrote `attachSessionRoutingHeaders` from the `ModelRegistry.getApiKeyAndHeaders` seam to `ModelRuntime.getAuth`, but dropped the pre-migration defensive invariant: a missing resolution method must **not** break session creation. On `main` the function now calls `modelRuntime.getAuth.bind(...)` unguarded, which throws if `getAuth` is ever absent. This restores the guard: no-op (warn) when `getAuth` is missing, so a future pi rename degrades to un-tagged requests instead of a hard failure at every agent start. ### Residual change 2 — test realignment (needed against current main) FN-8179 aligned the SDK but did **not** update the two `#1675` routing-header test suites, which still asserted the old `getApiKeyAndHeaders` seam. **Verified RED on current `main` before touching them:** - `pi-create-fn-agent.test.ts` — **60 / 104 failing** (mock had no `ModelRuntime` export; `createAgentSession` now receives `modelRuntime`). - `pi-session-routing-headers.test.ts` — **4 / 5 failing** (`attachSessionRoutingHeaders` signature is `getAuth`, not `getApiKeyAndHeaders`). Both are realigned to the `ModelRuntime.getAuth` seam (the mock gains the `ModelRuntime` export) → **109 / 109 green**. Assertions were strengthened to the new behavior, not weakened; the #1675 precedence invariant (taskId > pi session id > no-wrap), header merge, apiKey/provider-header passthrough, failed/undefined passthrough, and absent-method no-op are all still asserted. ## Surface enumeration - **Routing-header seam**: both the `createFnAgent` path and the `attachSessionRoutingHeaders` unit (taskId / pi-session-id / no-id precedence; header merge; apiKey + provider-header passthrough; failed/undefined resolution; absent-method no-op). - **Both mock forms** in `pi-create-fn-agent.test.ts`: the top-level `vi.mock` and all three `vi.doMock` skill-selection blocks now export `ModelRuntime`. ## Test evidence - `pnpm --filter @fusion/engine exec tsc --noEmit` → **exit 0**. - `pnpm --filter @fusion/engine exec vitest run` on both suites → **109/109 pass** (was 64 failing on main). - `pnpm verify:fast` → **PASS** (typecheck + build scoped to changed packages + CLI build + boot smoke `GET /api/health 200`; no tests run). Do not merge without CI. No release performed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Bug Fixes** * Improved session creation reliability when authentication support is unavailable. * Preserved existing authentication details while adding session-routing headers when supported. * Prevented session creation from failing when authentication information cannot be resolved. Co-authored-by: Claude Opus --- .../fix-session-routing-getauth-guard.md | 7 ++ .../src/__tests__/pi-create-fn-agent.test.ts | 67 +++++++++++----- .../pi-session-routing-headers.test.ts | 76 ++++++++++--------- packages/engine/src/pi.ts | 11 +++ 4 files changed, 110 insertions(+), 51 deletions(-) create mode 100644 .changeset/fix-session-routing-getauth-guard.md diff --git a/.changeset/fix-session-routing-getauth-guard.md b/.changeset/fix-session-routing-getauth-guard.md new file mode 100644 index 0000000000..9822357cd4 --- /dev/null +++ b/.changeset/fix-session-routing-getauth-guard.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Harden session-routing header wiring so a missing model-auth method can't break agent startup. +category: fix +dev: attachSessionRoutingHeaders now no-ops (warns) when ModelRuntime.getAuth is absent instead of throwing on getAuth.bind, restoring the pre-FN-8142 defensive invariant. Also realigns the pi-create-fn-agent and pi-session-routing-headers engine tests to the ModelRuntime.getAuth routing seam (mocks add the ModelRuntime export) so both suites pass against the 0.80.10 SDK landed by FN-8179. diff --git a/packages/engine/src/__tests__/pi-create-fn-agent.test.ts b/packages/engine/src/__tests__/pi-create-fn-agent.test.ts index 7f45b52eb1..ab89d28b00 100644 --- a/packages/engine/src/__tests__/pi-create-fn-agent.test.ts +++ b/packages/engine/src/__tests__/pi-create-fn-agent.test.ts @@ -16,9 +16,13 @@ const getAllMock = vi.fn(() => [] as any[]); const registerProviderMock = vi.fn(); const refreshMock = vi.fn(); // FNXC:SessionRouting 2026-06-24-11:30: -// #1675: capture model-registry auth resolution + session id so the wiring +// #1675: capture model-runtime auth resolution + session id so the wiring // test can assert X-Session-Id/X-Session-Affinity precedence end-to-end. +// FNXC:SessionRouting 2026-07-16-19:05: FN-8142 moved the routing-header seam from +// ModelRegistry.getApiKeyAndHeaders to ModelRuntime.getAuth; base getAuth returns a +// resolvable auth so attachSessionRoutingHeaders' header merge is observable. const getApiKeyAndHeadersMock = vi.fn(async () => ({ ok: true, apiKey: undefined, headers: undefined })); +const modelRuntimeGetAuthMock = vi.fn(async (..._args: unknown[]) => ({ auth: { headers: {} as Record } })); const sessionManagerGetSessionIdMock = vi.fn(() => undefined); const settingsManagerCreateMock = vi.fn(() => ({ kind: "settings-manager-create" })); const settingsManagerInMemoryMock = vi.fn(() => ({ kind: "settings-manager" })); @@ -147,6 +151,15 @@ vi.mock("@earendil-works/pi-coding-agent", () => ({ }, discoverAndLoadExtensions: discoverAndLoadExtensionsMock, getAgentDir: () => "/mock-agent-dir", + /* + FNXC:ModelRegistry 2026-07-16-19:05: + pi 0.80.8+ (FN-8142 migration) made model init async via ModelRuntime; createFusionModelRegistry + now awaits ModelRuntime.create(...) before constructing the registry. The stale ^0.80.6 pin masked + this until FN-8142's SDK bump (this PR); mock ModelRuntime so createFnAgent's registry path resolves. + */ + ModelRuntime: { + create: async () => ({ getAuth: modelRuntimeGetAuthMock }), + }, ModelRegistry: class { static create(...args: unknown[]) { return new (this as unknown as new () => unknown)(); @@ -1227,6 +1240,7 @@ describe("createFnAgent", () => { authStorageGetAllMock.mockReturnValue({}); authStorageListMock.mockReturnValue([]); getApiKeyAndHeadersMock.mockResolvedValue({ ok: true, apiKey: undefined, headers: undefined }); + modelRuntimeGetAuthMock.mockImplementation(async () => ({ auth: { headers: {} as Record } })); sessionManagerGetSessionIdMock.mockReturnValue(undefined); createBashToolMock.mockClear(); createAgentSessionMock.mockResolvedValue({ @@ -2750,10 +2764,18 @@ describe("createFnAgent", () => { // #1675: createFnAgent must resolve sessionRoutingId = taskId ?? piSessionId and // wrap the registry's getApiKeyAndHeaders so outbound requests carry routing // headers. These assert the wiring precedence end-to-end, not just the helper. + /* + FNXC:SessionRouting 2026-07-16-19:05: + FN-8142 (pi 0.80.8+) moved the #1675 routing-header seam off ModelRegistry.getApiKeyAndHeaders + onto ModelRuntime.getAuth (attachSessionRoutingHeaders). createAgentSession now receives the + runtime via `modelRuntime`, so the wiring test captures that runtime and asserts the decorated + getAuth merges X-Session-Id/X-Session-Affinity into the resolved auth headers. Precedence + (taskId > pi session id > no wrap) is the invariant under test, unchanged by the migration. + */ describe("session routing headers wiring (#1675)", () => { const anyModel = { provider: "anthropic", id: "claude" } as never; - async function createAndCaptureRegistry(overrides: Record = {}) { + async function createAndCaptureRuntime(overrides: Record = {}) { const { createFnAgent } = await import("../pi.js"); await createFnAgent({ cwd: "/tmp", @@ -2762,18 +2784,17 @@ describe("createFnAgent", () => { ...overrides, }); const sessionOptions = createAgentSessionMock.mock.calls.at(-1)?.[0] as { - modelRegistry: { getApiKeyAndHeaders: (model: unknown) => Promise }; + modelRuntime: { getAuth: (model: unknown) => Promise<{ auth: { headers?: Record } } | undefined> }; }; - return sessionOptions.modelRegistry; + return sessionOptions.modelRuntime; } it("uses taskId as the routing id when provided", async () => { - const registry = await createAndCaptureRegistry({ taskId: "FN-7788" }); + const runtime = await createAndCaptureRuntime({ taskId: "FN-7788" }); - const result = await registry.getApiKeyAndHeaders(anyModel) as { ok: boolean; headers?: Record }; + const result = await runtime.getAuth(anyModel); - expect(result.ok).toBe(true); - expect(result.headers).toEqual({ + expect(result?.auth.headers).toEqual({ "X-Session-Id": "FN-7788", "X-Session-Affinity": "FN-7788", }); @@ -2781,24 +2802,24 @@ describe("createFnAgent", () => { it("falls back to the pi session id when taskId is absent", async () => { sessionManagerGetSessionIdMock.mockReturnValue("pi-session-abc"); - const registry = await createAndCaptureRegistry(); + const runtime = await createAndCaptureRuntime(); - const result = await registry.getApiKeyAndHeaders(anyModel) as { ok: boolean; headers?: Record }; + const result = await runtime.getAuth(anyModel); - expect(result.headers).toEqual({ + expect(result?.auth.headers).toEqual({ "X-Session-Id": "pi-session-abc", "X-Session-Affinity": "pi-session-abc", }); }); - it("does not wrap getApiKeyAndHeaders when neither taskId nor a session id is available", async () => { - // getApiKeyAndHeadersMock returns { ok: true, headers: undefined }; if the - // wrapper were applied, headers would be populated with X-Session-*. - const registry = await createAndCaptureRegistry(); + it("does not wrap getAuth when neither taskId nor a session id is available", async () => { + // Base getAuth resolves { auth: { headers: {} } }; if the wrapper were + // applied, headers would be populated with X-Session-*. + const runtime = await createAndCaptureRuntime(); - const result = await registry.getApiKeyAndHeaders(anyModel) as { ok: boolean; headers?: Record }; + const result = await runtime.getAuth(anyModel); - expect(result.headers).toBeUndefined(); + expect(result?.auth.headers).toEqual({}); }); }); @@ -2844,6 +2865,10 @@ describe("createFnAgent", () => { } }, getAgentDir: () => "/mock-agent-dir", + // FNXC:ModelRegistry 2026-07-16-19:05: FN-8142 async ModelRuntime; mock so registry path resolves (see main mock above). + ModelRuntime: { + create: async () => ({ getAuth: async () => undefined }), + }, ModelRegistry: class { static create(...args: unknown[]) { return new (this as unknown as new () => unknown)(); @@ -2931,6 +2956,10 @@ describe("createFnAgent", () => { } }, getAgentDir: () => "/mock-agent-dir", + // FNXC:ModelRegistry 2026-07-16-19:05: FN-8142 async ModelRuntime; mock so registry path resolves (see main mock above). + ModelRuntime: { + create: async () => ({ getAuth: async () => undefined }), + }, ModelRegistry: class { static create(...args: unknown[]) { return new (this as unknown as new () => unknown)(); @@ -3015,6 +3044,10 @@ describe("createFnAgent", () => { } }, getAgentDir: () => "/mock-agent-dir", + // FNXC:ModelRegistry 2026-07-16-19:05: FN-8142 async ModelRuntime; mock so registry path resolves (see main mock above). + ModelRuntime: { + create: async () => ({ getAuth: async () => undefined }), + }, ModelRegistry: class { static create(...args: unknown[]) { return new (this as unknown as new () => unknown)(); diff --git a/packages/engine/src/__tests__/pi-session-routing-headers.test.ts b/packages/engine/src/__tests__/pi-session-routing-headers.test.ts index 5fd9edf4e5..cc77ab2020 100644 --- a/packages/engine/src/__tests__/pi-session-routing-headers.test.ts +++ b/packages/engine/src/__tests__/pi-session-routing-headers.test.ts @@ -1,11 +1,18 @@ import { describe, it, expect } from "vitest"; -import type { ModelRegistry } from "@earendil-works/pi-coding-agent"; +import type { AuthResult } from "@earendil-works/pi-ai"; +import type { ModelRuntime } from "@earendil-works/pi-coding-agent"; import { attachSessionRoutingHeaders, buildSessionRoutingHeaders } from "../pi.js"; // FNXC:SessionRouting 2026-06-23-16:40: // Issue #1675: chat completion requests must carry X-Session-Id and // X-Session-Affinity so LLM gateways can sticky-route and observability tools // can group the stateless API calls of one conversation into a single trace. +// +// FNXC:SessionRouting 2026-07-16-19:05: +// FN-8142 (pi 0.80.8+) moved the routing-header seam off ModelRegistry.getApiKeyAndHeaders +// onto ModelRuntime.getAuth. The invariant is unchanged: the resolved auth's headers must +// carry the routing pair, provider-specific headers/apiKey must survive, failed/absent +// resolutions must pass through untouched, and a missing getAuth must not break session creation. describe("buildSessionRoutingHeaders", () => { it("emits X-Session-Id and X-Session-Affinity with the same identifier", () => { @@ -17,45 +24,46 @@ describe("buildSessionRoutingHeaders", () => { }); describe("attachSessionRoutingHeaders", () => { - // Minimal stand-in for the bits of ModelRegistry the wrapper touches. - function makeRegistry( - resolve: (model: unknown) => Promise<{ ok: boolean; apiKey?: string; headers?: Record; error?: string }>, - ): ModelRegistry { - return { getApiKeyAndHeaders: resolve } as unknown as ModelRegistry; + // Minimal stand-in for the bits of ModelRuntime the wrapper touches. + function makeRuntime( + resolve: (model: unknown) => Promise, + ): ModelRuntime { + return { getAuth: resolve } as unknown as ModelRuntime; } const anyModel = { provider: "anthropic", id: "claude" } as never; it("merges the routing headers into resolved request headers", async () => { - const registry = makeRegistry(async () => ({ ok: true, apiKey: "sk-live", headers: undefined })); - attachSessionRoutingHeaders(registry, "sess-abc"); + const runtime = makeRuntime(async () => ({ auth: { apiKey: "sk-live" } })); + attachSessionRoutingHeaders(runtime, "sess-abc"); - const result = await registry.getApiKeyAndHeaders(anyModel); + const result = await runtime.getAuth(anyModel); expect(result).toEqual({ - ok: true, - apiKey: "sk-live", - headers: { - "X-Session-Id": "sess-abc", - "X-Session-Affinity": "sess-abc", + auth: { + apiKey: "sk-live", + headers: { + "X-Session-Id": "sess-abc", + "X-Session-Affinity": "sess-abc", + }, }, }); }); it("preserves the resolved apiKey and any provider-specific headers", async () => { - const registry = makeRegistry(async () => ({ - ok: true, - apiKey: "sk-custom", - headers: { "HTTP-Referer": "https://example.com", "X-Title": "Fusion" }, + const runtime = makeRuntime(async () => ({ + auth: { + apiKey: "sk-custom", + headers: { "HTTP-Referer": "https://example.com", "X-Title": "Fusion" }, + }, })); - attachSessionRoutingHeaders(registry, "sess-xyz"); + attachSessionRoutingHeaders(runtime, "sess-xyz"); - const result = await registry.getApiKeyAndHeaders(anyModel); + const result = await runtime.getAuth(anyModel); - expect(result.ok).toBe(true); - if (!result.ok) throw new Error("expected ok auth result"); - expect(result.apiKey).toBe("sk-custom"); - expect(result.headers).toEqual({ + if (!result) throw new Error("expected an auth result"); + expect(result.auth.apiKey).toBe("sk-custom"); + expect(result.auth.headers).toEqual({ "HTTP-Referer": "https://example.com", "X-Title": "Fusion", "X-Session-Id": "sess-xyz", @@ -63,21 +71,21 @@ describe("attachSessionRoutingHeaders", () => { }); }); - it("does not alter failed auth resolutions", async () => { - const registry = makeRegistry(async () => ({ ok: false, error: "No API key found" })); - attachSessionRoutingHeaders(registry, "sess-fail"); + it("does not alter failed (undefined) auth resolutions", async () => { + const runtime = makeRuntime(async () => undefined); + attachSessionRoutingHeaders(runtime, "sess-fail"); - const result = await registry.getApiKeyAndHeaders(anyModel); + const result = await runtime.getAuth(anyModel); - expect(result).toEqual({ ok: false, error: "No API key found" }); + expect(result).toBeUndefined(); }); - it("no-ops without throwing when getApiKeyAndHeaders is absent", () => { + it("no-ops without throwing when getAuth is absent", () => { // If a future pi-coding-agent rename removes the method, the wrapper must not - // break session creation. It leaves the registry untouched and warns instead. - const registry = {} as ModelRegistry; + // break session creation. It leaves the runtime untouched and warns instead. + const runtime = {} as ModelRuntime; - expect(() => attachSessionRoutingHeaders(registry, "sess-none")).not.toThrow(); - expect((registry as unknown as Record).getApiKeyAndHeaders).toBeUndefined(); + expect(() => attachSessionRoutingHeaders(runtime, "sess-none")).not.toThrow(); + expect((runtime as unknown as Record).getAuth).toBeUndefined(); }); }); diff --git a/packages/engine/src/pi.ts b/packages/engine/src/pi.ts index 0d5ae4d2fd..bc44a6e135 100644 --- a/packages/engine/src/pi.ts +++ b/packages/engine/src/pi.ts @@ -2057,7 +2057,18 @@ export function attachSessionRoutingHeaders(modelRuntime: ModelRuntime, sessionI pi 0.80.8 routes session request auth through ModelRuntime.getAuth rather than ModelRegistry.getApiKeyAndHeaders. Decorate the runtime seam so routing headers still reach every SDK-dispatched request before createAgentSession receives it. + + FNXC:SessionRouting 2026-07-16-19:05: + The FN-8142 migration to ModelRuntime.getAuth dropped the pre-migration defensive + invariant that a missing resolution method must NOT break session creation. Restore it: + no-op (warn) instead of throwing on `getAuth.bind` when the runtime lacks getAuth, so a + future pi rename removing the method degrades to un-tagged requests rather than a hard fail. */ + const runtimeWithAuth = modelRuntime as unknown as { getAuth?: ModelRuntime["getAuth"] }; + if (typeof runtimeWithAuth.getAuth !== "function") { + piLog.warn("attachSessionRoutingHeaders: modelRuntime.getAuth missing; skipping session-routing header wiring"); + return; + } const routingHeaders = buildSessionRoutingHeaders(sessionId); const resolveAuth = modelRuntime.getAuth.bind(modelRuntime) as ModelRuntime["getAuth"]; (modelRuntime as unknown as { getAuth: ModelRuntime["getAuth"] }).getAuth = (async (providerOrModel: Parameters[0], overrides?: Parameters[1]) => {