diff --git a/AGENTS.md b/AGENTS.md index 59c4dd4..aa7baac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -19,6 +19,7 @@ Pygienium is a `pi` extension: code-hygiene checks that scan a target with isola - Checks: `gate` (skip reason), `buildScanTask`, optional `buildFixTask`, optional `verify`. Scan tasks embed `!write`/`!echo` lines as deterministic fallback for the fake runner. - Sub-agent sessions must be pinned to the invoking session's model: thread `ctx.model` through `PygieniumCtx` → run options → `AgentTaskOptions.model` → `createAgentSession({ model })`. +- A pinned model needs its provider too: a provider an extension registered (`pi.registerProvider`) exists only in the invoking `ModelRegistry`, and the sub-session runs `noExtensions`, so thread `ctx.modelRegistry` (narrowed by `asProviderRegistry`) through the same chain to `AgentTaskOptions.modelRegistry` → `providerForwardingExtension`, which re-registers that one provider through an inline factory. Drop that link and every check dies with `No API key found for `. - Keep files modular; no compatibility shims. ## Commit process (IMPORTANT) @@ -29,7 +30,7 @@ Commits are gated by the pre-commit hook (enable: `git config core.hooksPath .gi - `src/index.ts` — removes `mode: ctx.mode` from the `pygieniumCtx` literal - `src/commands.ts` — removes `mode` from the `PygieniumCtx` Pick; swaps `ctx.mode` for `ctx.hasUI` in `print` -- `src/agent-runner.ts` — swaps the SDK import + loader for the omp SDK's `createAgentSession` options (`toolNames`, `systemPrompt`, `agentRegistry`, …) +- `src/agent-runner.ts` — swaps the SDK import + loader for the omp SDK's `createAgentSession` options (`toolNames`, `systemPrompt`, `agentRegistry`, …); reduces `ProviderRegistry` to an SDK-free shape and stubs `providerForwardingExtension` (the port disables extension discovery, so it has nothing to forward) - `src/agents.ts` — `find` → `glob` After touching any of those, run `bun port-to-omp.mjs --out ` and `(cd && bun run typecheck)` before committing; the hook does the same. diff --git a/port-to-omp.mjs b/port-to-omp.mjs index a6d9aaf..642fcdd 100644 --- a/port-to-omp.mjs +++ b/port-to-omp.mjs @@ -36,6 +36,10 @@ const OMP = join(HOME, ".omp", "agent", "extensions"); const SKIP = new Set([ "port-to-omp.mjs", "release-tag.sh", + // Pins pi-only behaviour: provider forwarding into an in-memory session. The + // port disables extension discovery, so it has no such behaviour to assert + // (see the agent-runner rules below). + "provider-forwarding.test.ts", ".gitea", ".github", "node_modules", @@ -246,9 +250,28 @@ const FILE_RULES = { }, ], "src/agent-runner.ts": [ + { + // The port has no extension provider registrations to read: its sessions + // are created with extension discovery disabled and its ModelRegistry + // has no getRegistered*Provider lookups. Keep only the shape the plumbing + // passes around, so the ported files stay type-correct. + re: /\/\*\* The invoking session's provider registry[\s\S]*?\n\}\n/, + to: + "/**\n * Provider registrations as the omp port passes them around. The port does not\n * forward any: omp sessions disable extension discovery, and its ModelRegistry\n * keeps no extension provider registrations to forward.\n */\nexport interface ProviderRegistry {\n\tgetRegisteredNativeProvider(providerId: string): object | undefined;\n\tgetRegisteredProviderConfig(providerId: string): object | undefined;\n}\n", + label: "provider registry typing", + }, + { + // The inline provider extension has no purpose in the port — extension + // discovery is disabled when the session is created — so the port carries + // only the stub the plumbing references. + re: /\n\/\*\*\n \* Build the inline extension that re-registers a provider inside a sub-session\.[\s\S]*?\n\t\};\n\}\n/, + to: + "\n/**\n * The omp port forwards no provider registrations (see `ProviderRegistry`), so\n * sub-agent sessions get no inline extension.\n */\nexport function providerForwardingExtension(): undefined {\n\treturn undefined;\n}\n", + label: "provider forwarding stub", + }, { from: - "\t// Lazily import the SDK so the rest of the module graph (and tests using the\n\t// fake runner) never resolve the heavy pi-coding-agent package.\n\tconst {\n\t\tcreateAgentSession,\n\t\tDefaultResourceLoader,\n\t\tgetAgentDir,\n\t\tSessionManager,\n\t} = await import(\"@oh-my-pi/pi-coding-agent\");\n\tconst loader = new DefaultResourceLoader({\n\t\tcwd: opts.cwd,\n\t\tagentDir: getAgentDir(),\n\t\tsystemPromptOverride: () => agent.systemPrompt,\n\t\t// Keep the sub-agent isolated: no nested extensions/skills/themes/etc.\n\t\tnoExtensions: true,\n\t\tnoSkills: true,\n\t\tnoThemes: true,\n\t\tnoPromptTemplates: true,\n\t});\n\tawait loader.reload();", + "\t// Lazily import the SDK so the rest of the module graph (and tests using the\n\t// fake runner) never resolve the heavy pi-coding-agent package.\n\tconst {\n\t\tcreateAgentSession,\n\t\tDefaultResourceLoader,\n\t\tgetAgentDir,\n\t\tSessionManager,\n\t} = await import(\"@oh-my-pi/pi-coding-agent\");\n\t// Re-register the pinned model's provider inside the sub-session: inline\n\t// extension factories load even under `noExtensions`, so this is the one\n\t// registration a pinned model needs in order to authenticate.\n\tconst providerExtension = providerForwardingExtension(\n\t\topts.modelRegistry,\n\t\topts.model?.provider,\n\t);\n\tconst loader = new DefaultResourceLoader({\n\t\tcwd: opts.cwd,\n\t\tagentDir: getAgentDir(),\n\t\tsystemPromptOverride: () => agent.systemPrompt,\n\t\t// Keep the sub-agent isolated: no nested extensions/skills/themes/etc.\n\t\tnoExtensions: true,\n\t\tnoSkills: true,\n\t\tnoThemes: true,\n\t\tnoPromptTemplates: true,\n\t\textensionFactories: providerExtension ? [providerExtension] : [],\n\t});\n\tawait loader.reload();", to: "\t// Lazily import the SDK so the rest of the module graph (and tests using the\n\t// fake runner) never resolve the heavy pi-coding-agent package. The specifier\n\t// is static by intent (dynamic-import exception: module is intentionally\n\t// excluded from the import-time graph to keep fake-runner tests SDK-free).\n\tconst { createAgentSession, AgentRegistry, SessionManager } = await import(\n\t\t\"@oh-my-pi/pi-coding-agent\"\n\t);", label: "SDK import + loader removal", diff --git a/src/agent-runner.ts b/src/agent-runner.ts index 88734f4..914b237 100644 --- a/src/agent-runner.ts +++ b/src/agent-runner.ts @@ -20,10 +20,61 @@ import { dirname, isAbsolute, join } from "node:path"; import type { AgentSession, AgentSessionEvent, + ExtensionAPI, ExtensionCommandContext, } from "@earendil-works/pi-coding-agent"; import { loadAgents, extensionRoot, type AgentDef } from "./agents.js"; +/** The invoking session's provider registry, as pi types it. */ +type HostModelRegistry = ExtensionCommandContext["modelRegistry"]; + +/** + * The two capabilities a sub-agent needs from the invoking session's + * `ModelRegistry`: the registrations extensions made through + * `pi.registerProvider`. Values are left in their host types — this module + * relays them, it never interprets them. + */ +export interface ProviderRegistry { + getRegisteredNativeProvider( + providerId: string, + ): ReturnType; + getRegisteredProviderConfig( + providerId: string, + ): ReturnType; +} + +/** + * The inline extension that re-registers a provider in a sub-session. Spelled out + * structurally — an `InlineExtension` in pi — so the generated omp port, which + * names the type differently, still compiles. + */ +export interface InlineProviderExtension { + name: string; + hidden: boolean; + factory: (pi: ExtensionAPI) => void; +} + +/** + * Narrow a host context's `modelRegistry` to a {@link ProviderRegistry}, + * returning `undefined` when it cannot answer the two questions this module + * asks. This is the decode boundary: everything downstream works with the + * narrow interface, and a host whose registry does not know extension + * registrations simply gets no provider forwarding. + */ +export function asProviderRegistry( + candidate: unknown, +): ProviderRegistry | undefined { + if (!candidate || typeof candidate !== "object") return undefined; + const registry = candidate as Partial; + if ( + typeof registry.getRegisteredNativeProvider !== "function" || + typeof registry.getRegisteredProviderConfig !== "function" + ) { + return undefined; + } + return registry as ProviderRegistry; +} + /** * Env var overriding the per-agent settle timeout (ms). Guards against a * sub-agent session that never settles (stalled provider stream, hung retry / @@ -58,6 +109,12 @@ export interface AgentTaskOptions { * `createAgentSession` falls back to the settings default model. */ model?: ExtensionCommandContext["model"]; + /** + * The invoking session's provider registry, used to make the pinned + * `model`'s provider resolvable inside the sub-session (see + * {@link providerForwardingExtension}). + */ + modelRegistry?: ProviderRegistry; /** Optional explicit agent definition (skips `loadAgents`). */ agent?: AgentDef; /** @@ -166,6 +223,49 @@ export function resetAgentRunner(): void { currentRunner = defaultAgentRunner; } +/** + * Build the inline extension that re-registers a provider inside a sub-session. + * + * A provider that exists only because an extension called + * `pi.registerProvider` (custom base URL, wrapped stream, its own auth wiring) + * lives in the invoking session's registry and nowhere else — not in + * `auth.json`, not in `models.json`. Sub-sessions are deliberately created with + * `noExtensions: true`, so their own `ModelRuntime` cannot resolve credentials + * for such a provider: pinning the invoking session's model then dies with + * pi's `No API key found for ` before the first token, which reads as + * a pygienium failure on a perfectly healthy login. + * + * Inline extension factories load regardless of `noExtensions`, so registering + * the pinned model's provider there is the narrow cost of pinning: the + * sub-agent authenticates as the host does, while every other extension stays + * out of its scope. + * + * Returns `undefined` when there is nothing to forward — no registry, no pinned + * model, or a provider pi already knows from its own catalogue (built-ins and + * `models.json` providers need no registration). + */ +export function providerForwardingExtension( + registry: ProviderRegistry | undefined, + providerId: string | undefined, +): InlineProviderExtension | undefined { + if (!registry || !providerId) return undefined; + // A native provider carries its own stream/auth implementation, so it wins + // over a config-only registration (which only augments a built-in one). + const native = registry.getRegisteredNativeProvider(providerId); + const config = native + ? undefined + : registry.getRegisteredProviderConfig(providerId); + if (!native && !config) return undefined; + return { + name: "pygienium-provider", + hidden: true, + factory: (pi) => { + if (native) pi.registerProvider(native); + else if (config) pi.registerProvider(providerId, config); + }, + }; +} + /** * Real sub-agent runner: spins up an in-memory `AgentSession` scoped to `cwd`, * overrides the system prompt with the agent definition's body, restricts tools @@ -197,6 +297,13 @@ export async function defaultAgentRunner( getAgentDir, SessionManager, } = await import("@earendil-works/pi-coding-agent"); + // Re-register the pinned model's provider inside the sub-session: inline + // extension factories load even under `noExtensions`, so this is the one + // registration a pinned model needs in order to authenticate. + const providerExtension = providerForwardingExtension( + opts.modelRegistry, + opts.model?.provider, + ); const loader = new DefaultResourceLoader({ cwd: opts.cwd, agentDir: getAgentDir(), @@ -206,6 +313,7 @@ export async function defaultAgentRunner( noSkills: true, noThemes: true, noPromptTemplates: true, + extensionFactories: providerExtension ? [providerExtension] : [], }); await loader.reload(); diff --git a/src/commands.ts b/src/commands.ts index 379e571..4e6c3c1 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -44,12 +44,20 @@ import { type ExportFilters, } from "./export.js"; import type { SendChatMessage } from "./phases.js"; +import type { ProviderRegistry } from "./agent-runner.js"; /** Narrow context slice handlers need (a subset of `ExtensionCommandContext`). */ export type PygieniumCtx = Pick< ExtensionCommandContext, "cwd" | "mode" | "hasUI" | "ui" | "model" > & { + /** + * The invoking session's extension provider registrations, already narrowed + * at the extension entry point (see `asProviderRegistry`). Carried into every + * sub-agent so a pinned model whose provider only an extension registered + * stays resolvable there. + */ + modelRegistry?: ProviderRegistry; /** Optional callback to post messages to the chat window. */ sendChatMessage?: SendChatMessage; /** Optional callback forwarding raw sub-agent events to the chat stream. */ @@ -177,6 +185,7 @@ export async function handleCheckCommand( ui: ctx.ui, hasUI: ctx.hasUI, model: ctx.model, + modelRegistry: ctx.modelRegistry, sendChatMessage: ctx.sendChatMessage, onAgentEvent: ctx.onAgentEvent, sendPhaseLine: ctx.sendPhaseLine, @@ -217,6 +226,7 @@ export async function handleAllCommand( ui: ctx.ui, hasUI: ctx.hasUI, model: ctx.model, + modelRegistry: ctx.modelRegistry, sendChatMessage: ctx.sendChatMessage, onAgentEvent: ctx.onAgentEvent, sendPhaseLine: ctx.sendPhaseLine, @@ -306,6 +316,7 @@ export async function handleResumeCommand( ui: ctx.ui, hasUI: ctx.hasUI, model: ctx.model, + modelRegistry: ctx.modelRegistry, existingState: state, sendChatMessage: ctx.sendChatMessage, onAgentEvent: ctx.onAgentEvent, diff --git a/src/index.ts b/src/index.ts index 2a1a2c3..0db0fd1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -31,6 +31,7 @@ import type { import { Box, Text } from "@earendil-works/pi-tui"; import "./checks/all.js"; import { registerPygieniumCommands, type PygieniumCtx } from "./commands.js"; +import { asProviderRegistry } from "./agent-runner.js"; import { type SendChatMessage, type CheckCompletionDetails, @@ -405,6 +406,7 @@ export default async function pygieniumExtension( hasUI: ctx.hasUI, ui: ctx.ui, model: ctx.model, + modelRegistry: asProviderRegistry(ctx.modelRegistry), sendChatMessage, onAgentEvent, sendPhaseLine, diff --git a/src/modes/all.ts b/src/modes/all.ts index 0ecfd6a..5718d7f 100644 --- a/src/modes/all.ts +++ b/src/modes/all.ts @@ -29,6 +29,7 @@ import type { } from "@earendil-works/pi-coding-agent"; import { getAllChecks, type CheckDefinition } from "../checks/registry.js"; import { runCheck } from "./check-runner.js"; +import type { ProviderRegistry } from "../agent-runner.js"; import { createPhaseStrip, type SendChatMessage } from "../phases.js"; import { createPipelineFooter, type ItemStatus } from "../footer.js"; import { runRecon } from "../recon.js"; @@ -83,6 +84,12 @@ export interface AllRunOptions { * the settings default. */ model?: ExtensionCommandContext["model"]; + /** + * The invoking session's provider registry; carried into every check's + * sub-agents so a provider only an extension registered stays resolvable + * (see `providerForwardingExtension` in `agent-runner.ts`). + */ + modelRegistry?: ProviderRegistry; /** Optional callback to post completion messages into the chat. */ sendChatMessage?: SendChatMessage; /** Optional callback forwarding raw sub-agent events to the chat stream. */ @@ -379,6 +386,7 @@ export async function runAllChecks( ui: opts.ui, hasUI, model: opts.model, + modelRegistry: opts.modelRegistry, existingState: state, sendChatMessage: opts.sendChatMessage, onAgentEvent: opts.onAgentEvent, diff --git a/src/modes/check-runner.ts b/src/modes/check-runner.ts index e97bd54..68a3e04 100644 --- a/src/modes/check-runner.ts +++ b/src/modes/check-runner.ts @@ -22,7 +22,7 @@ import type { ExtensionUIContext, } from "@earendil-works/pi-coding-agent"; import type { CheckDefinition, CheckScope } from "../checks/registry.js"; -import { runAgentTask } from "../agent-runner.js"; +import { runAgentTask, type ProviderRegistry } from "../agent-runner.js"; import { runRecon } from "../recon.js"; import { createPhaseStrip, @@ -93,6 +93,12 @@ export interface RunCheckOptions { * settings default. */ model?: ExtensionCommandContext["model"]; + /** + * The invoking session's provider registry; carried into every sub-agent so a + * provider only an extension registered stays resolvable (see + * `providerForwardingExtension` in `agent-runner.ts`). + */ + modelRegistry?: ProviderRegistry; /** Pre-existing run state to update (for `/pygienium-all` and resume). */ existingState?: RunState; /** Optional callback to post completion messages into the chat. */ @@ -288,6 +294,7 @@ async function runCheckImplInner( agentName: check.agentName, task: scanTask, model: opts.model, + modelRegistry: opts.modelRegistry, onEvent: forward(PHASE_ANALYSIS), }); findings = scanResult.text; @@ -321,6 +328,7 @@ async function runCheckImplInner( agentName: check.fixAgentName ?? "fixer", task: fixTask, model: opts.model, + modelRegistry: opts.modelRegistry, onEvent: forward(PHASE_FIX), }); changes = fixResult.text; diff --git a/tests/provider-forwarding.test.ts b/tests/provider-forwarding.test.ts new file mode 100644 index 0000000..6d763e5 --- /dev/null +++ b/tests/provider-forwarding.test.ts @@ -0,0 +1,105 @@ +/** + * provider-forwarding.test.ts — the provider hand-off from the invoking session + * into a sub-agent session. + * + * A sub-agent session is created with `noExtensions: true`, so a provider that + * exists only because an extension called `pi.registerProvider` is absent from + * its `ModelRuntime` and its credentials cannot be resolved — pinning the + * invoking session's model then fails with `No API key found for ` + * before a single token, on a perfectly healthy login. These tests pin the two + * halves of the fix: narrowing the host registry at the boundary, and the inline + * extension that re-registers the pinned model's provider inside the sub-session + * (inline factories load even under `noExtensions`). + */ +import { describe, expect, it } from "bun:test"; +import { + asProviderRegistry, + providerForwardingExtension, + type ProviderRegistry, +} from "../src/agent-runner.js"; +import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; + +/** Record `registerProvider` calls so the relay can be asserted on. */ +function recorder(): { + pi: ExtensionAPI; + calls: unknown[][]; +} { + const calls: unknown[][] = []; + const pi = { + registerProvider: (...args: unknown[]) => { + calls.push(args); + }, + } as unknown as ExtensionAPI; + return { pi, calls }; +} + +/** Registry stub holding one provider id, native and/or config form. */ +function registry(entry: { + native?: unknown; + config?: unknown; +}): ProviderRegistry { + return { + getRegisteredNativeProvider: (_id) => + entry.native === undefined ? undefined : entry.native, + getRegisteredProviderConfig: (_id) => + entry.config === undefined ? undefined : entry.config, + }; +} + +describe("asProviderRegistry", () => { + it("accepts a registry that exposes both registration lookups", () => { + const host = registry({ native: { id: "neuralwatt" } }); + expect(asProviderRegistry(host)).toBe(host); + }); + + it("rejects values that cannot answer both lookups", () => { + expect(asProviderRegistry(undefined)).toBeUndefined(); + expect(asProviderRegistry(null)).toBeUndefined(); + expect(asProviderRegistry("modelRegistry")).toBeUndefined(); + // A registry from a host that never records extension providers, such as + // the omp port, degrades to "nothing to forward" instead of throwing. + expect(asProviderRegistry({})).toBeUndefined(); + expect( + asProviderRegistry({ getRegisteredNativeProvider: () => undefined }), + ).toBeUndefined(); + }); +}); + +describe("providerForwardingExtension", () => { + it("is a no-op without a registry, a pinned model, or a registered provider", () => { + // Built-in and models.json providers need no forwarding: pi's own + // catalogue already answers for them. + const empty = registry({}); + expect(providerForwardingExtension(undefined, "neuralwatt")).toBeUndefined(); + expect(providerForwardingExtension(empty, undefined)).toBeUndefined(); + expect(providerForwardingExtension(empty, "neuralwatt")).toBeUndefined(); + }); + + it("re-registers a native provider, keeping its own stream and auth wiring", () => { + const native = { id: "neuralwatt" }; + const extension = providerForwardingExtension( + registry({ native, config: { baseUrl: "ignored-when-native" } }), + "neuralwatt", + ); + if (!extension) throw new Error("expected a forwarding extension"); + expect(extension.name).toBe("pygienium-provider"); + // Hidden: the sub-agent's extension list must not advertise an extension + // the user never installed for it. + expect(extension.hidden).toBe(true); + + const { pi, calls } = recorder(); + extension.factory(pi); + // The native provider wins over a config-only registration, which would + // only augment a built-in provider and lose the wrapped stream. + expect(calls).toEqual([[native]]); + }); + + it("re-registers a config-only provider under its provider id", () => { + const config = { baseUrl: "config-only-provider" }; + const extension = providerForwardingExtension(registry({ config }), "probe"); + if (!extension) throw new Error("expected a forwarding extension"); + const { pi, calls } = recorder(); + extension.factory(pi); + expect(calls).toEqual([["probe", config]]); + }); +});