Skip to content

Commit 7bdf931

Browse files
instigator24jhegeman-dsclaude
authored
Feat/pi deferred extension model resolution (#1509)
* feat(pi): support extension-registered provider models via deferred resolution Extension providers (e.g. pi-provider-kiro) register their models on the ModelRegistry during session.bindExtensions(), not during the initial modelRegistry.find() call. The previous approach threw immediately when find() returned undefined, blocking extension models. New flow: 1. modelRegistry.find() checks static catalog + models.json (step 3) 2. If found: pass to createAgentSession, fail-fast auth as before 3. If not found: log info, skip auth (extension providers manage their own credentials), create session without model 4. After bindExtensions() (step 4g): retry modelRegistry.find() — now extension-registered models are discoverable — and call session.setModel() to switch to the resolved model 5. If still not found after bindExtensions(): throw with a hint about installing the provider extension with enableExtensions: true This preserves all existing behavior for built-in providers and models.json custom models while adding support for extension-registered providers like pi-provider-kiro (Kiro API — 20 free models including Claude, DeepSeek, Qwen, etc.). Changes: - provider.ts: deferred model resolution after bindExtensions(); conditional model arg to createAgentSession; conditional auth fail-fast; session.setModel() for extension-resolved models - provider.test.ts: add setModel mock; update model-not-found tests for two-phase find() behavior * test: add archon-test-pi example workflow for Kiro extension validation Simple two-node workflow that verifies the Pi provider works with extension-registered models (e.g. pi-provider-kiro). Node 1 asks a factual question, node 2 validates the answer. Tested with kiro/claude-sonnet-4-6 and kiro/minimax-m2-5. * docs: add Pi/Kiro prerequisites and setup to examples README * fix(pi): load models.json custom providers and surface SDK error messages - Switch ModelRegistry.inMemory() to ModelRegistry.create() so custom providers from ~/.pi/agent/models.json (ollama, LM Studio, etc.) resolve correctly. Both return the same mutable class supporting registerProvider() for extensions. - Surface Pi's AssistantMessage.errorMessage in the result chunk errors array so dag-executor shows actual errors instead of generic 'SDK returned error'. - Add ollama/qwen3.5 node to archon-test-pi workflow to validate both extension (kiro) and models.json (ollama) provider paths. * fix(pi): address Wirasm PR review — comment clarity, session cleanup, test mock alignment - event-bridge: document intentional design of yielding error chunks (isError:true propagates in chunk, not via throw); fix event name to pi.result_chunk_error; remove noisy model field from error log - provider: replace verbose multi-paragraph step-numbered comments with concise WHY explanations; fix stale ModelRegistry.inMemory() reference (code uses create()); add session.dispose() on model-not-found throw and setModel() failure to close the session cleanup gap - provider.test: align mock from ModelRegistry.inMemory → ModelRegistry.create to match actual provider code (was already broken on the branch); update test description and comments to describe behavior rather than step numbers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(pi): add happy-path test for deferred extension model resolution Verifies that when find() returns undefined on the first call (static catalog miss) and a model on the second call (after bindExtensions()), session.setModel() is invoked with the resolved model and no error is returned. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: jhegeman-ds <joel.hegeman@deepseas.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent c94803e commit 7bdf931

6 files changed

Lines changed: 219 additions & 92 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
name: archon-test-pi
2+
description: |
3+
Simple test workflow to verify the pi provider works with Archon.
4+
Triggers: "test pi", "test pi provider".
5+
6+
provider: pi
7+
model: kiro/minimax-m2-5
8+
9+
nodes:
10+
- id: hello
11+
prompt: |
12+
Who won the 1984 Major League Baseball World Series? Answer in one sentence.
13+
14+
- id: hello-ollama
15+
model: ollama/qwen3.5
16+
prompt: |
17+
Who won the 1984 Major League Baseball World Series? Answer in one sentence.
18+
19+
- id: verify
20+
prompt: |
21+
The extension model node said:
22+
23+
$hello.output
24+
25+
The ollama node said:
26+
27+
$hello-ollama.output
28+
29+
If both answers mention the Detroit Tigers, say "Pi provider verified ✅ (extension + ollama)".
30+
If only the extension model is correct, say "Pi extension verified ✅, ollama FAILED ❌".
31+
If only ollama is correct, say "Pi ollama verified ✅, extension FAILED ❌".
32+
If neither is correct, say "Pi provider FAILED ❌ — both wrong".
33+
Say nothing else.
34+
depends_on: [hello, hello-ollama]

examples/workflows/README.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Prerequisites
2+
3+
## Install Pi
4+
5+
```bash
6+
npm install -g @mariozechner/pi-coding-agent
7+
```
8+
9+
## Install Kiro provider and dependencies
10+
11+
```bash
12+
pi install npm:pi-provider-kiro
13+
npm install -g @mariozechner/pi-tui @mariozechner/pi-ai
14+
```
15+
16+
## Install MCP for Pi
17+
18+
```bash
19+
pi install npm:pi-mcp-adapter
20+
```
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
name: archon-test-pi
2+
description: |
3+
Simple test workflow to verify the pi provider works with Archon.
4+
Triggers: "test pi", "test pi provider".
5+
6+
provider: pi
7+
model: kiro/minimax-m2-5
8+
9+
nodes:
10+
- id: hello
11+
model: kiro/minimax-m2-5
12+
prompt: |
13+
Who won the 1984 Major League Baseball World Series? Answer in one sentence.
14+
15+
- id: hello-ollama
16+
model: ollama/qwen3.5
17+
prompt: |
18+
Who won the 1984 Major League Baseball World Series? Answer in one sentence.
19+
20+
- id: verify
21+
prompt: |
22+
The extension model node said:
23+
24+
$hello.output
25+
26+
The ollama node said:
27+
28+
$hello-ollama.output
29+
30+
If both answers mention the Detroit Tigers, say "Pi provider verified ✅ (extension + ollama)".
31+
If only the extension model is correct, say "Pi extension verified ✅, ollama FAILED ❌".
32+
If only ollama is correct, say "Pi ollama verified ✅, extension FAILED ❌".
33+
If neither is correct, say "Pi provider FAILED ❌ — both wrong".
34+
Say nothing else.
35+
depends_on: [hello, hello-ollama]

packages/providers/src/community/pi/event-bridge.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,15 @@ export function buildResultChunk(messages: readonly unknown[]): MessageChunk {
179179
}
180180
: {}),
181181
};
182+
if (isError) {
183+
// Intentional design: error chunks are yielded, not thrown. isError:true in the chunk
184+
// is the signal — callers (bridgeSession, dag-executor) check result.isError to classify
185+
// failures and still receive full token/stopReason context from the same chunk.
186+
getLog().error(
187+
{ stopReason: last.stopReason, errorMessage: last.errorMessage },
188+
'pi.result_chunk_error'
189+
);
190+
}
182191
return chunk;
183192
}
184193

packages/providers/src/community/pi/provider.test.ts

Lines changed: 43 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,15 @@ const mockSetFlagValue = mock((_name: string, _value: boolean | string) => undef
4343
const mockExtensionRunner = {
4444
setFlagValue: mockSetFlagValue,
4545
};
46+
const mockSetModel = mock(async (_model: unknown) => undefined);
4647
const mockSession = {
4748
subscribe: mockSubscribe,
4849
prompt: mockPrompt,
4950
abort: mockAbort,
5051
dispose: mockDispose,
5152
bindExtensions: mockBindExtensions,
5253
extensionRunner: mockExtensionRunner,
54+
setModel: mockSetModel,
5355
isStreaming: false,
5456
sessionId: 'mock-session-uuid',
5557
};
@@ -188,6 +190,7 @@ describe('PiProvider', () => {
188190
mockDispose.mockClear();
189191
mockSubscribe.mockClear();
190192
mockBindExtensions.mockClear();
193+
mockSetModel.mockClear();
191194
mockSetFlagValue.mockClear();
192195
mockResourceLoaderReload.mockClear();
193196
mockCreateAgentSession.mockClear();
@@ -281,11 +284,9 @@ describe('PiProvider', () => {
281284
});
282285

283286
test('ModelRegistry.create receives the AuthStorage instance', async () => {
284-
// Headline-fix wiring: ModelRegistry.create must receive the same
285-
// AuthStorage instance returned by AuthStorage.create(), so registry
286-
// lookups can resolve user-configured custom models from
287-
// ~/.pi/agent/models.json (LM Studio, ollama, llamacpp, etc.). Without
288-
// this wiring the registry only sees the static built-in catalog.
287+
// ModelRegistry.create must receive the same AuthStorage instance
288+
// returned by AuthStorage.create(), so extension providers can resolve
289+
// credentials and register models during bindExtensions().
289290
process.env.GEMINI_API_KEY = 'sk-test';
290291
resetScript(scriptedAgentEnd());
291292

@@ -327,33 +328,32 @@ describe('PiProvider', () => {
327328
});
328329

329330
test('Pi model not found includes models.json load error when registry reports one', async () => {
330-
// ModelRegistry swallows models.json parse/validation errors into an
331-
// internal loadError. When find() returns undefined we surface that
332-
// error in both the structured log and the throw message so users
333-
// debugging a custom-provider config see the actual reason.
334331
process.env.GEMINI_API_KEY = 'sk-test';
332+
// find() is called twice — first on the static catalog, then again after bindExtensions()
333+
// resolves extension providers. Both must return undefined to trigger the error.
334+
mockModelRegistryFind.mockImplementationOnce(() => undefined);
335335
mockModelRegistryFind.mockImplementationOnce(() => undefined);
336336
mockModelRegistryCreate.mockImplementationOnce(() => ({
337337
find: mockModelRegistryFind,
338338
getError: () => 'Provider lm-studio: "baseUrl" is required when defining custom models.',
339339
}));
340340

341+
resetScript(scriptedAgentEnd());
341342
const { error } = await consume(
342343
new PiProvider().sendQuery('hi', '/tmp', undefined, {
343344
model: 'lm-studio/some-model',
344345
})
345346
);
346347

347348
expect(error?.message).toContain('Pi model not found');
348-
expect(error?.message).toContain('models.json failed to load');
349-
expect(error?.message).toContain('"baseUrl" is required');
350-
expect(mockLogger.error).toHaveBeenCalledWith(
349+
expect(error?.message).toContain('provider extension');
350+
expect(mockLogger.warn).toHaveBeenCalledWith(
351351
expect.objectContaining({
352352
piProvider: 'lm-studio',
353353
modelId: 'some-model',
354354
loadError: expect.stringContaining('"baseUrl" is required'),
355355
}),
356-
'pi.model_not_found'
356+
'pi.model_registry_load_error'
357357
);
358358
});
359359

@@ -407,17 +407,43 @@ describe('PiProvider', () => {
407407

408408
test('throws when ModelRegistry.find returns undefined', async () => {
409409
process.env.GEMINI_API_KEY = 'sk-test';
410-
// 'nonexistent' is handled in mockModelRegistryFind to return undefined, but
411-
// the adapter rejects unknown providers. To exercise
412-
// the not-found branch, use a known provider but unknown modelId by
413-
// temporarily swapping mockModelRegistryFind to always return undefined.
410+
// find() is called twice — first on the static catalog, then again after bindExtensions()
411+
// resolves extension providers. Both must return undefined to trigger the error.
412+
mockModelRegistryFind.mockImplementationOnce(() => undefined);
414413
mockModelRegistryFind.mockImplementationOnce(() => undefined);
414+
resetScript(scriptedAgentEnd());
415415
const { error } = await consume(
416416
new PiProvider().sendQuery('hi', '/tmp', undefined, {
417417
model: 'google/unknown-model-id',
418418
})
419419
);
420420
expect(error?.message).toContain('Pi model not found');
421+
expect(error?.message).toContain('provider extension');
422+
});
423+
424+
test('deferred resolution: calls session.setModel when find() resolves after bindExtensions', async () => {
425+
// Phase 1: model not in static catalog (extension provider path).
426+
// Phase 2: extension registers the model during bindExtensions() and find() succeeds.
427+
mockModelRegistryFind.mockImplementationOnce(() => undefined);
428+
mockModelRegistryFind.mockImplementationOnce(() => ({
429+
id: 'custom-model',
430+
provider: 'extension-provider',
431+
name: 'extension-provider/custom-model',
432+
}));
433+
resetScript(scriptedAgentEnd());
434+
435+
const { error } = await consume(
436+
new PiProvider().sendQuery('hi', '/tmp', undefined, {
437+
model: 'extension-provider/custom-model',
438+
})
439+
);
440+
441+
expect(error).toBeUndefined();
442+
expect(mockModelRegistryFind).toHaveBeenCalledTimes(2);
443+
expect(mockSetModel).toHaveBeenCalledTimes(1);
444+
expect(mockSetModel).toHaveBeenCalledWith(
445+
expect.objectContaining({ id: 'custom-model', provider: 'extension-provider' })
446+
);
421447
});
422448

423449
test('request env (codebase env vars) overrides process.env via setRuntimeApiKey', async () => {

0 commit comments

Comments
 (0)