Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 35 additions & 22 deletions packages/providers/src/community/pi/provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,14 @@ const mockAuthCreate = mock(() => ({
setRuntimeApiKey: mockSetRuntimeApiKey,
getApiKey: mockGetApiKey,
}));
const mockModelRegistryInMemory = mock(() => ({}));

const mockModelRegistryFind = mock((provider: string, modelId: string) => {
if (provider === 'nonexistent') return undefined;
return { id: modelId, provider, name: `${provider}/${modelId}` };
});
const mockModelRegistryCreate = mock(() => ({
find: mockModelRegistryFind,
}));

// SessionManager mocks. Each returns a tagged session-manager stub so tests
// can assert whether resume resolved to an existing session or fell through
Expand Down Expand Up @@ -115,7 +122,7 @@ const mockCreateLsTool = mock((_cwd: string) => ({ __piTool: 'ls' }));
mock.module('@mariozechner/pi-coding-agent', () => ({
createAgentSession: mockCreateAgentSession,
AuthStorage: { create: mockAuthCreate },
ModelRegistry: { inMemory: mockModelRegistryInMemory },
ModelRegistry: { create: mockModelRegistryCreate },
SessionManager: {
create: mockSessionCreate,
open: mockSessionOpen,
Expand All @@ -132,16 +139,6 @@ mock.module('@mariozechner/pi-coding-agent', () => ({
createLsTool: mockCreateLsTool,
}));

// getModel is imported from pi-ai. Return a fake model for known refs and
// undefined for unknown refs so the provider's not-found branch is testable.
const mockGetModel = mock((provider: string, modelId: string) => {
if (provider === 'nonexistent') return undefined;
return { id: modelId, provider, name: `${provider}/${modelId}` };
});
mock.module('@mariozechner/pi-ai', () => ({
getModel: mockGetModel,
}));

// Import AFTER mocks are set — module resolution freezes the mocks.
import { PiProvider } from './provider';
import { PI_CAPABILITIES } from './capabilities';
Expand Down Expand Up @@ -169,6 +166,12 @@ function resetScript(events: FakeEvent[]): void {

describe('PiProvider', () => {
beforeEach(() => {
mockLogger.fatal.mockClear();
mockLogger.error.mockClear();
mockLogger.warn.mockClear();
mockLogger.info.mockClear();
mockLogger.debug.mockClear();
mockLogger.trace.mockClear();
mockPrompt.mockClear();
mockAbort.mockClear();
mockDispose.mockClear();
Expand All @@ -177,8 +180,9 @@ describe('PiProvider', () => {
mockSetFlagValue.mockClear();
mockResourceLoaderReload.mockClear();
mockCreateAgentSession.mockClear();
mockGetModel.mockClear();
mockAuthCreate.mockClear();
mockModelRegistryCreate.mockClear();
mockModelRegistryFind.mockClear();
mockSetRuntimeApiKey.mockClear();
mockGetApiKey.mockClear();
MockDefaultResourceLoader.mockClear();
Expand Down Expand Up @@ -236,15 +240,24 @@ describe('PiProvider', () => {
expect(error?.message).toContain('Invalid Pi model ref');
});

test('throws when Pi provider id is unknown AND no creds available', async () => {
// No env var, no auth.json entry → fail-fast with hint about env-var table
test('logs credential hint when Pi provider id is unknown AND no creds available', async () => {
// No env var, no auth.json entry → log hint, but continue, to support custom providers that don't use credentials or that use non-Pi means of providing credentials.
resetScript(scriptedAgentEnd());
const { error } = await consume(
new PiProvider().sendQuery('hi', '/tmp', undefined, {
model: 'unknownprovider/some-model',
})
);
expect(error?.message).toContain("no credentials for provider 'unknownprovider'");
expect(error?.message).toContain("not in the Archon adapter's env-var table");

expect(error).toBeUndefined();
expect(mockLogger.info).toHaveBeenCalledWith(
{
piProvider: 'unknownprovider',
msg: expect.stringContaining("not in the Archon adapter's env-var table"),
},
'pi.auth_missing'
);
expect(mockCreateAgentSession).toHaveBeenCalledTimes(1);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

test('throws when env var missing AND auth.json has no entry', async () => {
Expand Down Expand Up @@ -295,13 +308,13 @@ describe('PiProvider', () => {
expect(mockGetApiKey).toHaveBeenCalledWith('anthropic');
});

test('throws when getModel returns undefined', async () => {
test('throws when ModelRegistry.find returns undefined', async () => {
process.env.GEMINI_API_KEY = 'sk-test';
// 'nonexistent' is handled in mockGetModel to return undefined, but
// the adapter rejects unknown providers before getModel. To exercise
// 'nonexistent' is handled in mockModelRegistryFind to return undefined, but
// the adapter rejects unknown providers. To exercise
// the not-found branch, use a known provider but unknown modelId by
// temporarily swapping mockGetModel to always return undefined.
mockGetModel.mockImplementationOnce(() => undefined);
// temporarily swapping mockModelRegistryFind to always return undefined.
mockModelRegistryFind.mockImplementationOnce(() => undefined);
const { error } = await consume(
new PiProvider().sendQuery('hi', '/tmp', undefined, {
model: 'google/unknown-model-id',
Expand Down
80 changes: 29 additions & 51 deletions packages/providers/src/community/pi/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { tmpdir } from 'node:os';
import { join } from 'node:path';

import { createLogger } from '@archon/paths';
import type { Api, Model } from '@mariozechner/pi-ai';

import type {
IAgentProvider,
Expand Down Expand Up @@ -95,24 +94,6 @@ function getLog(): ReturnType<typeof createLogger> {
return cachedLog;
}

/**
* Typed wrapper around Pi's `getModel` for a runtime-string provider/model
* pair. Pi's getModel signature constrains `TModelId` to
* `keyof MODELS[TProvider]`, which isn't knowable from a runtime string —
* the local `GetModelFn` alias is the narrowest shape that still lets us
* bypass that constraint. Isolating the escape hatch behind one searchable
* name keeps it auditable. Takes `getModel` as a parameter because the Pi
* SDK is loaded dynamically (see the header comment on this file for why).
*/
type GetModelFn = (provider: string, modelId: string) => Model<Api> | undefined;
function lookupPiModel(
getModel: GetModelFn,
provider: string,
modelId: string
): Model<Api> | undefined {
return getModel(provider, modelId);
}

/**
* Append a "respond with JSON matching this schema" instruction to the user
* prompt so Pi-backed models produce parseable structured output. Pi's SDK
Expand Down Expand Up @@ -140,15 +121,7 @@ ${JSON.stringify(schema, null, 2)}`;
/**
* Pi community provider — wraps `@mariozechner/pi-coding-agent`'s full
* coding-agent harness. Each `sendQuery()` call creates a fresh session
* (no reuse) with in-memory auth/session/settings, so the server never
* touches `~/.pi/` and concurrent calls don't collide.
*
* Capabilities (see `capabilities.ts` for the canonical list): Pi declares
* `sessionResume`, `skills`, `toolRestrictions`, `structuredOutput`,
* `envInjection`, `effortControl`, and `thinkingControl`. Features Pi does
* not currently support through Archon (`mcp`, `hooks`, `agents`,
* `costControl`, `fallbackModel`, `sandbox`) stay off; the dag-executor
* surfaces a warning for any unsupported nodeConfig field.
* (no reuse) so concurrent calls don't collide.
*/
export class PiProvider implements IAgentProvider {
async *sendQuery(
Expand All @@ -174,15 +147,13 @@ export class PiProvider implements IAgentProvider {
// destructured PascalCase bindings trip eslint's naming-convention rule.
const [
piCodingAgent,
piAi,
{ bridgeSession },
{ resolvePiSkills, resolvePiThinkingLevel, resolvePiTools },
{ createNoopResourceLoader },
{ resolvePiSession },
{ createArchonUIBridge, createArchonUIContext },
] = await Promise.all([
import('@mariozechner/pi-coding-agent'),
import('@mariozechner/pi-ai'),
import('./event-bridge'),
import('./options-translator'),
import('./resource-loader'),
Expand Down Expand Up @@ -227,22 +198,23 @@ export class PiProvider implements IAgentProvider {
);
}

// 2. Look up the Model via Pi's static catalog. `lookupPiModel` returns
const authStorage = piCodingAgent.AuthStorage.create();
const modelRegistry = piCodingAgent.ModelRegistry.create(authStorage);
// 2. Look up the Model via Pi's Registry. `modelRegistry.find` returns
// undefined when not found; we guard explicitly below.
// Cast to the runtime-string-friendly shape — see `lookupPiModel`'s docblock.
const model = lookupPiModel(piAi.getModel as GetModelFn, parsed.provider, parsed.modelId);
const model = modelRegistry.find(parsed.provider, parsed.modelId);
if (!model) {
throw new Error(
`Pi model not found: provider='${parsed.provider}' model='${parsed.modelId}'. ` +
'See https://github.com/badlogic/pi-mono/blob/main/packages/ai/src/models.generated.ts for the Pi model catalog.'
);
}

// 3. Build AuthStorage. `AuthStorage.create()` reads ~/.pi/agent/auth.json
// (or $PI_CODING_AGENT_DIR/auth.json), so any credential the user has
// 3. Build AuthStorage. `AuthStorage.create()`, above, read ~/.pi/agent/auth.json
// (or $PI_CODING_AGENT_DIR/auth.json), so any credentials the user has
// populated via `pi` → `/login` (OAuth subscriptions: Claude Pro/Max,
// ChatGPT Plus, GitHub Copilot, Gemini CLI, Antigravity) or by editing
// the file directly (api_key entries) is picked up transparently.
// the file directly (api_key entries) are picked up transparently.
//
// Per-request env vars override the file via setRuntimeApiKey — this
// mirrors Claude's process-env + request-env merge pattern and
Expand All @@ -258,7 +230,6 @@ export class PiProvider implements IAgentProvider {
// OAuth refresh note: Pi refreshes expired access tokens against the
// provider's OAuth server and rewrites ~/.pi/agent/auth.json under a
// file lock (same mechanism pi CLI uses — safe for concurrent access).
const authStorage = piCodingAgent.AuthStorage.create();

const envVarName = PI_PROVIDER_ENV_VARS[parsed.provider];
const envOverride = envVarName
Expand All @@ -268,16 +239,25 @@ export class PiProvider implements IAgentProvider {
authStorage.setRuntimeApiKey(parsed.provider, envOverride);
}

// Fail-fast: resolve creds synchronously before spinning up a session.
// Matches Claude's auth-error fast-fail pattern (no retry on auth failures).
const resolvedKey = await authStorage.getApiKey(parsed.provider);
if (!resolvedKey) {
const envHint = envVarName
? `Set ${envVarName} in the environment or codebase env vars (.archon/config.yaml env: section).`
: `Provider '${parsed.provider}' is not in the Archon adapter's env-var table — file an issue if you want a shortcut env var for it.`;
if (envVarName) {
const envHint = `Set ${envVarName} in the environment or codebase env vars (.archon/config.yaml env: section).`;
const loginHint = `Or run \`pi\` and type \`/login\` locally to authenticate '${parsed.provider}' via OAuth; credentials land in ~/.pi/agent/auth.json and are picked up automatically.`;
throw new Error(
`Pi auth: no credentials for provider '${parsed.provider}'. ${envHint} ${loginHint}`
);
}

const envHint = `Provider '${parsed.provider}' is not in the Archon adapter's env-var table — file an issue if you want a shortcut env var for it.`;
const loginHint = `Or run \`pi\` and type \`/login\` locally to authenticate '${parsed.provider}' via OAuth; credentials land in ~/.pi/agent/auth.json and are picked up automatically.`;
throw new Error(
`Pi auth: no credentials for provider '${parsed.provider}'. ${envHint} ${loginHint}`
// No credentials for unmapped providers is not necessarily a problem, e.g., local models.
getLog().info(
{
piProvider: parsed.provider,
msg: `No Pi credentials found for provider. ${envHint} ${loginHint}`,
},
'pi.auth_missing'
);
}

Expand Down Expand Up @@ -343,13 +323,11 @@ export class PiProvider implements IAgentProvider {
};
}

// ModelRegistry + settings stay in-memory — only sessions persist, to
// match Claude/Codex. Resource loader still suppresses filesystem
// discovery by default, except for explicitly-passed skill paths and —
// when piConfig.enableExtensions is true — Pi's community extension
// ecosystem (tools + lifecycle hooks from ~/.pi/agent/extensions/ and
// packages installed via `pi install npm:<pkg>`).
const modelRegistry = piCodingAgent.ModelRegistry.inMemory(authStorage);
// Settings stay in-memory — only sessions persist, to match Claude/Codex.
// Resource loader still suppresses filesystem except for explicitly-passed
// skill paths and — when piConfig.enableExtensions is true — Pi's community
// extension ecosystem (tools + lifecycle hooks from ~/.pi/agent/extensions/
// and packages installed via `pi install npm:<pkg>`).
const settingsManager = piCodingAgent.SettingsManager.inMemory();
// Default ON: extensions (community packages like @plannotator/pi-extension
// or your own local ones) are a core reason users run Pi. Opt out with
Expand Down
Loading