fix(engine): defensive getAuth guard on session routing seam + realign pi tests to ModelRuntime seam (#2261)
## 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) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus <noreply@anthropic.com>
This commit is contained in:
7
.changeset/fix-session-routing-getauth-guard.md
Normal file
7
.changeset/fix-session-routing-getauth-guard.md
Normal file
@@ -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.
|
||||
@@ -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<string, string> } }));
|
||||
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<string, string> } }));
|
||||
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<string, unknown> = {}) {
|
||||
async function createAndCaptureRuntime(overrides: Record<string, unknown> = {}) {
|
||||
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<unknown> };
|
||||
modelRuntime: { getAuth: (model: unknown) => Promise<{ auth: { headers?: Record<string, string> } } | 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<string, string> };
|
||||
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<string, string> };
|
||||
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<string, string> };
|
||||
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)();
|
||||
|
||||
@@ -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<string, string>; 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<AuthResult | undefined>,
|
||||
): 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<string, unknown>).getApiKeyAndHeaders).toBeUndefined();
|
||||
expect(() => attachSessionRoutingHeaders(runtime, "sess-none")).not.toThrow();
|
||||
expect((runtime as unknown as Record<string, unknown>).getAuth).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<ModelRuntime["getAuth"]>[0], overrides?: Parameters<ModelRuntime["getAuth"]>[1]) => {
|
||||
|
||||
Reference in New Issue
Block a user