Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 13 additions & 0 deletions docs/learnings/2026-04-25-ci-os-specific-path-assertion.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# CI OS-specific path assertion

## What happened

CI failed on `pi-agent-manager.test.ts` because a launch-command assertion expected the quoted agent binary path to contain `'/Users/`. That passed on macOS but failed on the Linux GitHub runner, where `os.homedir()` resolves to `/home/runner`.

## Fix

Assert against `posixShellQuote(mgr.getBinPath())` instead of a hard-coded platform-specific path prefix. This keeps the test focused on the intended behavior: command paths are shell-quoted.

## Prevention

Avoid hard-coding OS-specific home directory prefixes in cross-platform tests. When testing derived paths, assert against the same public path-building API or use platform-neutral path properties.
3 changes: 3 additions & 0 deletions docs/learnings/2026-04-25-provider-ordering-canonical-id.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Provider Ordering Expectations With Canonical IDs

When changing a provider identifier to match an upstream canonical id, update ordering tests to match the actual label/id sort behavior. `amazon-bedrock` sorts before `anthropic` with the existing `localeCompare`-based ordering, so tests should assert that order instead of preserving the previous `bedrock` position.
27 changes: 24 additions & 3 deletions src/main/__tests__/pi-agent-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ describe('posixShellQuote', () => {
describe('PiAgentManager.buildLaunchCommand', () => {
const mgr = new PiAgentManager();

it('produces an empty envOverrides path starting with FLEET_BRIDGE_PORT', () => {
it('produces an empty envOverrides path starting with quoted Fleet env vars', () => {
const cmd = mgr.buildLaunchCommand(8123, 'tok', 'pane-1', {});
expect(
cmd.startsWith('FLEET_BRIDGE_PORT=8123 FLEET_BRIDGE_TOKEN=tok FLEET_PANE_ID=pane-1 ')
cmd.startsWith("FLEET_BRIDGE_PORT='8123' FLEET_BRIDGE_TOKEN='tok' FLEET_PANE_ID='pane-1' ")
).toBe(true);
});

Expand All @@ -35,13 +35,34 @@ describe('PiAgentManager.buildLaunchCommand', () => {
AWS_SECRET_ACCESS_KEY: `it's/a/secret`
});
expect(cmd).toMatch(
/^AWS_REGION='us-east-1' AWS_SECRET_ACCESS_KEY='it'\\''s\/a\/secret' FLEET_BRIDGE_PORT=/
/^AWS_REGION='us-east-1' AWS_SECRET_ACCESS_KEY='it'\\''s\/a\/secret' FLEET_BRIDGE_PORT='8123'/
);
});

it('quotes shell-sensitive Fleet vars and command paths for shell -c', () => {
const cmd = mgr.buildLaunchCommand(8123, "tok'; $(touch nope)", 'pane;`id`', {});

expect(cmd).toContain("FLEET_BRIDGE_TOKEN='tok'\\''; $(touch nope)'");
expect(cmd).toContain("FLEET_PANE_ID='pane;`id`'");
expect(cmd).toContain(posixShellQuote(mgr.getBinPath()));
expect(cmd).toContain(" -e '");
expect(cmd).not.toContain('"');
});

it('serializes envOverrides in stable insertion order', () => {
const cmd = mgr.buildLaunchCommand(0, '', '', { A: '1', B: '2', C: '3' });
expect(cmd.indexOf(`A='1'`)).toBeLessThan(cmd.indexOf(`B='2'`));
expect(cmd.indexOf(`B='2'`)).toBeLessThan(cmd.indexOf(`C='3'`));
});

it('prepends unset directives before env assignments', () => {
const cmd = mgr.buildLaunchCommand(8123, 'tok', 'pane-1', { AWS_PROFILE: 'dev' }, [
'AWS_ACCESS_KEY_ID',
'AWS_SECRET_ACCESS_KEY'
]);

expect(cmd.startsWith("unset AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY; AWS_PROFILE='dev' ")).toBe(
true
);
});
});
36 changes: 36 additions & 0 deletions src/main/__tests__/pi-auth-inspector.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,39 @@ describe('PiAuthInspector.getBuiltInStatus', () => {
expect(list.every((p) => !p.authenticated || p.method === 'env-var')).toBe(true);
});
});

describe('PiAuthInspector.listAvailableModels', () => {
let dir: string;

beforeEach(() => {
dir = makeDir();
});

afterEach(() => {
rmSync(dir, { recursive: true, force: true });
});

it('loads models from Pi public getProviders/getModels exports', async () => {
const modulePath = join(dir, 'pi-ai.mjs');
writeFileSync(
modulePath,
`export function getProviders() { return ['anthropic', 'amazon-bedrock']; }
export function getModels(provider) {
if (provider === 'anthropic') return [{ id: 'claude-sonnet-4-5', name: 'Claude Sonnet 4.5' }];
if (provider === 'amazon-bedrock') return [{ id: 'amazon.nova-pro-v1:0', name: 'Nova Pro' }];
return [];
}
`
);

const insp = new PiAuthInspector({
authPath: join(dir, 'auth.json'),
modelCatalogPath: modulePath
});

await expect(insp.listAvailableModels()).resolves.toEqual([
{ providerId: 'anthropic', modelId: 'claude-sonnet-4-5', label: 'Claude Sonnet 4.5' },
{ providerId: 'amazon-bedrock', modelId: 'amazon.nova-pro-v1:0', label: 'Nova Pro' }
]);
});
});
73 changes: 60 additions & 13 deletions src/main/__tests__/pi-env-injection-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ describe('PiBedrockInjectionSchema', () => {
expect(parsed.mode).toBe('chain');
});

it('accepts all three modes', () => {
for (const mode of ['profile', 'keys', 'chain'] as const) {
it('accepts all credential modes', () => {
for (const mode of ['profile', 'keys', 'bearer', 'chain'] as const) {
expect(PiBedrockInjectionSchema.parse({ mode }).mode).toBe(mode);
}
});
Expand Down Expand Up @@ -67,10 +67,21 @@ describe('PiEnvInjectionManager — writeBedrock/getRedactedConfig', () => {
region: 'us-west-2',
accessKeyId: 'AKIA…',
secretAccessKeyPresent: true,
sessionTokenPresent: false
sessionTokenPresent: false,
bearerTokenPresent: false
});
});

it('round-trips bearer token presence without exposing plaintext', () => {
const store = new FakeStore();
const mgr = new PiEnvInjectionManager({ store, safeStorage: fakeSafeStorage });

mgr.writeBedrock({ mode: 'bearer', bearerToken: 'bearer-secret' });

expect(mgr.getRedactedConfig().bedrock?.bearerTokenPresent).toBe(true);
expect(JSON.stringify(store.get())).not.toContain('bearer-secret');
});

it('encrypts secrets before persisting', () => {
const store = new FakeStore();
const mgr = new PiEnvInjectionManager({ store, safeStorage: fakeSafeStorage });
Expand Down Expand Up @@ -98,6 +109,16 @@ describe('PiEnvInjectionManager — writeBedrock/getRedactedConfig', () => {
expect(redacted?.sessionTokenPresent).toBe(true);
});

it('clearBedrockSecret removes bearer token only when requested', () => {
const store = new FakeStore();
const mgr = new PiEnvInjectionManager({ store, safeStorage: fakeSafeStorage });

mgr.writeBedrock({ mode: 'bearer', bearerToken: 'bt' });
mgr.clearBedrockSecret('bearerToken');

expect(mgr.getRedactedConfig().bedrock?.bearerTokenPresent).toBe(false);
});

it('write with safeStorage unavailable throws when secrets are supplied', () => {
const store = new FakeStore();
const mgr = new PiEnvInjectionManager({
Expand All @@ -106,6 +127,7 @@ describe('PiEnvInjectionManager — writeBedrock/getRedactedConfig', () => {
});

expect(() => mgr.writeBedrock({ mode: 'keys', secretAccessKey: 'x' })).toThrow(/encryption/i);
expect(() => mgr.writeBedrock({ mode: 'bearer', bearerToken: 'x' })).toThrow(/encryption/i);
});

it('write with safeStorage unavailable succeeds when no secrets are supplied', () => {
Expand All @@ -130,20 +152,28 @@ describe('PiEnvInjectionManager.getInjectedEnv', () => {

it('mode=chain writes only AWS_REGION when present', () => {
const mgr = buildMgr({ mode: 'chain', region: 'eu-central-1' });
expect(mgr.getInjectedEnv()).toEqual({ AWS_REGION: 'eu-central-1' });
expect(mgr.getInjectedEnv()).toEqual({ set: { AWS_REGION: 'eu-central-1' }, unset: [] });
});

it('mode=chain with no region writes nothing', () => {
const mgr = buildMgr({ mode: 'chain' });
expect(mgr.getInjectedEnv()).toEqual({});
expect(mgr.getInjectedEnv()).toEqual({ set: {}, unset: [] });
});

it('mode=profile writes AWS_PROFILE + AWS_REGION', () => {
it('mode=profile writes AWS_PROFILE + AWS_REGION and unsets key credentials', () => {
const mgr = buildMgr({ mode: 'profile', profile: 'dev', region: 'us-east-1' });
expect(mgr.getInjectedEnv()).toEqual({ AWS_PROFILE: 'dev', AWS_REGION: 'us-east-1' });
expect(mgr.getInjectedEnv()).toEqual({
set: { AWS_PROFILE: 'dev', AWS_REGION: 'us-east-1' },
unset: [
'AWS_ACCESS_KEY_ID',
'AWS_SECRET_ACCESS_KEY',
'AWS_SESSION_TOKEN',
'AWS_BEARER_TOKEN_BEDROCK'
]
});
});

it('mode=keys decrypts secretAccessKey and sessionToken', () => {
it('mode=keys decrypts secretAccessKey and sessionToken and unsets profile/bearer', () => {
const store = new FakeStore();
const mgr = new PiEnvInjectionManager({ store, safeStorage: fakeSafeStorage });
mgr.writeBedrock({
Expand All @@ -154,10 +184,24 @@ describe('PiEnvInjectionManager.getInjectedEnv', () => {
sessionToken: 'tok'
});
expect(mgr.getInjectedEnv()).toEqual({
AWS_REGION: 'us-east-1',
AWS_ACCESS_KEY_ID: 'AKIA',
AWS_SECRET_ACCESS_KEY: 'shh',
AWS_SESSION_TOKEN: 'tok'
set: {
AWS_REGION: 'us-east-1',
AWS_ACCESS_KEY_ID: 'AKIA',
AWS_SECRET_ACCESS_KEY: 'shh',
AWS_SESSION_TOKEN: 'tok'
},
unset: ['AWS_PROFILE', 'AWS_BEARER_TOKEN_BEDROCK']
});
});

it('mode=bearer decrypts AWS_BEARER_TOKEN_BEDROCK and unsets profile/key credentials', () => {
const store = new FakeStore();
const mgr = new PiEnvInjectionManager({ store, safeStorage: fakeSafeStorage });
mgr.writeBedrock({ mode: 'bearer', region: 'us-east-1', bearerToken: 'bt' });

expect(mgr.getInjectedEnv()).toEqual({
set: { AWS_REGION: 'us-east-1', AWS_BEARER_TOKEN_BEDROCK: 'bt' },
unset: ['AWS_PROFILE', 'AWS_ACCESS_KEY_ID', 'AWS_SECRET_ACCESS_KEY', 'AWS_SESSION_TOKEN']
});
});

Expand All @@ -172,6 +216,9 @@ describe('PiEnvInjectionManager.getInjectedEnv', () => {
}
});
const mgr = new PiEnvInjectionManager({ store, safeStorage: fakeSafeStorage });
expect(mgr.getInjectedEnv()).toEqual({ AWS_REGION: 'us-east-1', AWS_ACCESS_KEY_ID: 'AKIA' });
expect(mgr.getInjectedEnv()).toEqual({
set: { AWS_REGION: 'us-east-1', AWS_ACCESS_KEY_ID: 'AKIA' },
unset: ['AWS_PROFILE', 'AWS_BEARER_TOKEN_BEDROCK']
});
});
});
4 changes: 2 additions & 2 deletions src/main/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ const piAuthInspector = new PiAuthInspector({
'node_modules',
'@mariozechner',
'pi-ai',
'src',
'models.generated.ts'
'dist',
'index.js'
)
});
const fleetBridge = new FleetBridgeServer();
Expand Down
6 changes: 3 additions & 3 deletions src/main/ipc-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ import type { PiConfigManager } from './pi-config-manager';
import { PiConfigParseError, PiConfigValidationError } from './pi-config-manager';
import type { PiAuthInspector } from './pi-auth-inspector';
import type { PiEnvInjectionManager } from './pi-env-injection-manager';
import type { BedrockWritePatch } from '../shared/pi-env-injection-types';
import type { BedrockWritePatch, BedrockSecretField } from '../shared/pi-env-injection-types';
import type { PiProvider, PiSettings } from '../shared/pi-config-types';
import type { FleetSettings } from '../shared/types';
import { checkSystemDeps } from './system-checker';
Expand Down Expand Up @@ -539,7 +539,7 @@ export function registerIpcHandlers(
const token = fleetBridge.generateToken();
const port = fleetBridge.getPort();
const env = piEnvInjectionManager.getInjectedEnv();
const cmd = piAgentManager.buildLaunchCommand(port, token, req.paneId, env);
const cmd = piAgentManager.buildLaunchCommand(port, token, req.paneId, env.set, env.unset);
return { cmd };
});

Expand Down Expand Up @@ -647,7 +647,7 @@ export function registerIpcHandlers(

ipcMain.handle(
IPC_CHANNELS.PI_ENV_CLEAR_SECRET,
(_event, field: 'secretAccessKey' | 'sessionToken') => {
(_event, field: BedrockSecretField) => {
piEnvInjectionManager.clearBedrockSecret(field);
}
);
Expand Down
21 changes: 11 additions & 10 deletions src/main/pi-agent-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,32 +98,33 @@ export class PiAgentManager {
bridgePort: number,
bridgeToken: string,
paneId: string,
envOverrides: Record<string, string> = {}
envOverrides: Record<string, string> = {},
envUnsets: string[] = []
): string {
const extensionPaths = this.getExtensionPaths();
const parts: string[] = [];

if (envUnsets.length > 0) {
parts.push(`unset ${envUnsets.join(' ')};`);
}

for (const [key, value] of Object.entries(envOverrides)) {
parts.push(`${key}=${posixShellQuote(value)}`);
}

parts.push(`FLEET_BRIDGE_PORT=${bridgePort}`);
parts.push(`FLEET_BRIDGE_TOKEN=${bridgeToken}`);
parts.push(`FLEET_PANE_ID=${paneId}`);
parts.push(`FLEET_BRIDGE_PORT=${posixShellQuote(String(bridgePort))}`);
parts.push(`FLEET_BRIDGE_TOKEN=${posixShellQuote(bridgeToken)}`);
parts.push(`FLEET_PANE_ID=${posixShellQuote(paneId)}`);

parts.push(this.quoteArg(this.getBinPath()));
parts.push(posixShellQuote(this.getBinPath()));

for (const ext of extensionPaths) {
parts.push('-e', this.quoteArg(ext));
parts.push('-e', posixShellQuote(ext));
}

return parts.join(' ');
}

private quoteArg(arg: string): string {
return arg.includes(' ') ? `"${arg}"` : arg;
}

async ensureInstalled(): Promise<void> {
if (this.isInstalled()) return;

Expand Down
43 changes: 28 additions & 15 deletions src/main/pi-auth-inspector.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { readFile } from 'fs/promises';
import { join } from 'path';
import { pathToFileURL } from 'url';
import { homedir } from 'os';
import { z } from 'zod';
import { createLogger } from './logger';
Expand All @@ -10,12 +11,15 @@ const log = createLogger('pi-auth-inspector');

const AuthMapSchema = z.record(z.string(), z.unknown());

const ModelCatalogItemSchema = z.object({
provider: z.string(),
const PublicModelSchema = z.object({
id: z.string(),
name: z.string().optional()
});
const ModelCatalogSchema = z.array(z.unknown());

type PiModelCatalogModule = {
getProviders?: unknown;
getModels?: unknown;
};

type PiAuthInspectorOptions = {
authPath?: string;
Expand Down Expand Up @@ -70,19 +74,28 @@ export class PiAuthInspector {
async listAvailableModels(): Promise<ModelEntry[]> {
if (!this.modelCatalogPath) return [];
try {
const text = await readFile(this.modelCatalogPath, 'utf-8');
const match = text.match(/MODELS\s*=\s*(\[[\s\S]*?\]);/);
if (!match) return [];
const rawArray = ModelCatalogSchema.parse(JSON.parse(match[1]));
const module = (await import(pathToFileURL(this.modelCatalogPath).href)) as PiModelCatalogModule;
if (typeof module.getProviders !== 'function' || typeof module.getModels !== 'function') {
return [];
}

const providersRaw: unknown = module.getProviders();
if (!Array.isArray(providersRaw)) return [];

const results: ModelEntry[] = [];
for (const raw of rawArray) {
const item = ModelCatalogItemSchema.safeParse(raw);
if (!item.success) continue;
results.push({
providerId: item.data.provider,
modelId: item.data.id,
label: item.data.name ?? item.data.id
});
for (const provider of providersRaw) {
if (typeof provider !== 'string') continue;
const modelsRaw: unknown = module.getModels(provider);
if (!Array.isArray(modelsRaw)) continue;
for (const raw of modelsRaw) {
const item = PublicModelSchema.safeParse(raw);
if (!item.success) continue;
results.push({
providerId: provider,
modelId: item.data.id,
label: item.data.name ?? item.data.id
});
}
}
return results;
} catch (err) {
Expand Down
Loading
Loading