From 32836634d28a173924a706ae3445b9c2ad69bf6d Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Wed, 19 Aug 2026 18:08:01 +0000 Subject: [PATCH 1/2] fix(metering): register the gateway's api root, not the gateway root --- agentic/metering/__tests__/gateway.test.ts | 18 +++++++++-- agentic/metering/__tests__/reporter.test.ts | 11 ++++--- agentic/metering/src/gateway.ts | 31 +++++++++++++------ agentic/metering/src/index.ts | 1 + agentic/pi/__tests__/embed/lanes.test.ts | 4 +-- .../extensions/metered-model.test.ts | 3 +- 6 files changed, 49 insertions(+), 19 deletions(-) diff --git a/agentic/metering/__tests__/gateway.test.ts b/agentic/metering/__tests__/gateway.test.ts index 2292c92ebc..8ea24f92dc 100644 --- a/agentic/metering/__tests__/gateway.test.ts +++ b/agentic/metering/__tests__/gateway.test.ts @@ -1,6 +1,7 @@ import { ACTOR_ID_HEADER, buildIdentityHeaders, + completionsBaseUrl, DATABASE_ID_HEADER, ENTITY_ID_HEADER, GATEWAY_API, @@ -48,8 +49,9 @@ describe('normalizeGatewayUrl', () => { expect(normalizeGatewayUrl('https://example.com/gateway/')).toBe('https://example.com/gateway'); }); - it('rejects a /v1 suffix, which an openai-compatible client would double up', () => { - expect(() => normalizeGatewayUrl('https://example.com/v1')).toThrow(/drop the \/v1/); + it('answers the root for a caller who named the api root instead', () => { + expect(normalizeGatewayUrl('https://example.com/v1')).toBe('https://example.com'); + expect(normalizeGatewayUrl('https://example.com/gateway/v1/')).toBe('https://example.com/gateway'); }); it('rejects relative and non-http urls', () => { @@ -91,6 +93,16 @@ describe('resolveMeteredModel', () => { }); }); +describe('completionsBaseUrl', () => { + // An openai-completions client appends `/chat/completions` and nothing else, so + // a baseUrl at the gateway root 404s on the first model turn. + it('is the api root under the gateway root, however the root was spelled', () => { + expect(completionsBaseUrl('https://agentic.example.com')).toBe('https://agentic.example.com/v1'); + expect(completionsBaseUrl('https://agentic.example.com/v1')).toBe('https://agentic.example.com/v1'); + expect(completionsBaseUrl('https://example.com/gateway/')).toBe('https://example.com/gateway/v1'); + }); +}); + describe('resolveMeteredGateway', () => { it('resolves the gateway endpoint over the openai-completions api', () => { const config = resolveMeteredGateway({ @@ -99,7 +111,7 @@ describe('resolveMeteredGateway', () => { models }); - expect(config.baseUrl).toBe('https://agentic.example.com'); + expect(config.baseUrl).toBe('https://agentic.example.com/v1'); expect(config.api).toBe(GATEWAY_API); expect(config.models).toHaveLength(1); expect(config.headers?.[DATABASE_ID_HEADER]).toBe('db-1'); diff --git a/agentic/metering/__tests__/reporter.test.ts b/agentic/metering/__tests__/reporter.test.ts index 706e400800..be2069cc22 100644 --- a/agentic/metering/__tests__/reporter.test.ts +++ b/agentic/metering/__tests__/reporter.test.ts @@ -34,10 +34,13 @@ describe('httpUsageSink', () => { expect(JSON.parse(init.body)).toEqual(report); }); - it('rejects a gateway URL that already includes /v1', () => { - expect(() => - httpUsageSink({ gatewayUrl: 'https://gw.example.com/v1', identity, fetch: jest.fn() as never }) - ).toThrow(/drop the \/v1/); + it('posts to the one usage route whether or not the caller named the api root', async () => { + const fetchMock = jest.fn().mockResolvedValue({ ok: true, status: 202 }); + const sink = httpUsageSink({ gatewayUrl: 'https://gw.example.com/v1', identity, fetch: fetchMock as never }); + + await sink(report); + + expect(fetchMock.mock.calls[0][0]).toBe('https://gw.example.com/v1/usage'); }); it('requires a databaseId', () => { diff --git a/agentic/metering/src/gateway.ts b/agentic/metering/src/gateway.ts index 3e31e7d076..717888c7a1 100644 --- a/agentic/metering/src/gateway.ts +++ b/agentic/metering/src/gateway.ts @@ -3,7 +3,7 @@ * * `agentic-server` speaks OpenAI's `/v1/chat/completions`, so routing a harness * through it needs no custom streaming code — only an endpoint whose `baseUrl` is - * the gateway and whose headers carry the run's identity. Every harness worth + * the gateway's OpenAI api root and whose headers carry the run's identity. Every harness worth * adapting can already talk to an OpenAI-compatible endpoint, which is why this * resolution is neutral and only the registration is vendor-specific: an adapter * turns `MeteredGateway` into whatever its harness calls a provider. @@ -41,7 +41,7 @@ export interface MeteredModelSpec { } export interface MeteredGatewayOptions { - /** Gateway root, e.g. `https://agentic.example.com` — not a `/v1` path. */ + /** Gateway root, e.g. `https://agentic.example.com`; a trailing `/v1` is tolerated. */ gatewayUrl: string; identity: MeteredIdentity; models: readonly MeteredModelSpec[]; @@ -83,7 +83,13 @@ export interface MeteredModel { const ZERO_COST = { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 } as const; -/** Normalize the gateway root: absolute http(s), no trailing slash, no `/v1`. */ +/** + * Normalize the gateway root: absolute http(s), no trailing slash, no `/v1`. + * + * A caller who names the api root instead is answered the root it hangs under, + * so a deployment that spells its gateway either way reaches the same routes + * rather than one of them 404ing on the first model turn. + */ export function normalizeGatewayUrl(gatewayUrl: string): string { const raw = gatewayUrl?.trim(); if (!raw) throw new Error('metered model: gatewayUrl is required'); @@ -98,14 +104,21 @@ export function normalizeGatewayUrl(gatewayUrl: string): string { throw new Error(`metered model: gatewayUrl must be http(s), got ${parsed.protocol}`); } - const path = parsed.pathname.replace(/\/+$/, ''); - // An OpenAI-compatible client appends `/v1/chat/completions`, so a baseUrl that - // already ends in `/v1` would request `/v1/v1/chat/completions` and 404 at the - // first model turn. - if (/\/v1$/.test(path)) throw new Error(`metered model: gatewayUrl must be the gateway root, not ${raw} (drop the /v1)`); + const path = parsed.pathname.replace(/\/+$/, '').replace(/\/v1$/, ''); return `${parsed.origin}${path}`; } +/** + * The OpenAI api root under a gateway root. + * + * An `openai-completions` client (pi-ai, the OpenAI SDKs, any harness built on + * either) appends `/chat/completions` to its `baseUrl` and nothing else, so the + * `/v1` belongs here rather than in the caller's environment. + */ +export function completionsBaseUrl(gatewayUrl: string): string { + return `${normalizeGatewayUrl(gatewayUrl)}/v1`; +} + export function resolveMeteredModel(spec: MeteredModelSpec): MeteredModel { if (!spec.id?.trim()) throw new Error('metered model: model id is required'); return { @@ -133,7 +146,7 @@ export function resolveMeteredGateway(options: MeteredGatewayOptions): MeteredGa return { providerName: options.providerName ?? DEFAULT_PROVIDER_NAME, displayName: options.displayName ?? 'Constructive (metered)', - baseUrl: normalizeGatewayUrl(options.gatewayUrl), + baseUrl: completionsBaseUrl(options.gatewayUrl), api: GATEWAY_API, headers, // Harnesses typically require an apiKey once models are declared; the gateway diff --git a/agentic/metering/src/index.ts b/agentic/metering/src/index.ts index 1cc5e7e7d4..95bf802aea 100644 --- a/agentic/metering/src/index.ts +++ b/agentic/metering/src/index.ts @@ -9,6 +9,7 @@ */ export { + completionsBaseUrl, DEFAULT_PROVIDER_NAME, GATEWAY_API, type MeteredGateway, diff --git a/agentic/pi/__tests__/embed/lanes.test.ts b/agentic/pi/__tests__/embed/lanes.test.ts index 9858e6dc9f..d39e86748c 100644 --- a/agentic/pi/__tests__/embed/lanes.test.ts +++ b/agentic/pi/__tests__/embed/lanes.test.ts @@ -75,9 +75,9 @@ describe('composeRun', () => { expect(() => composeRun({ runId: 'run-1', - metering: { mode: 'gateway', gatewayUrl: `${gatewayUrl}/v1`, identity, models } + metering: { mode: 'gateway', gatewayUrl: 'ws://gateway.constructive.io', identity, models } }) - ).toThrow(); + ).toThrow(/http\(s\)/); // A policy that can ask needs somewhere to ask. expect(() => composeRun({ runId: 'run-1', gate: { policy: {} } })).toThrow(/approvals channel is required/); diff --git a/agentic/pi/__tests__/extensions/metered-model.test.ts b/agentic/pi/__tests__/extensions/metered-model.test.ts index c8e6389286..5b1f80f9ee 100644 --- a/agentic/pi/__tests__/extensions/metered-model.test.ts +++ b/agentic/pi/__tests__/extensions/metered-model.test.ts @@ -44,7 +44,8 @@ describe('createMeteredModelExtension', () => { expect(pi.providers).toHaveLength(1); expect(pi.providers[0].name).toBe('constructive-gateway'); - expect(pi.providers[0].config.baseUrl).toBe('https://agentic.example.com'); + // pi-ai appends `/chat/completions`, so the registration must carry the /v1. + expect(pi.providers[0].config.baseUrl).toBe('https://agentic.example.com/v1'); }); it('honours a custom provider name', () => { From 25f8cbaa829314a67524a7d1757c54df460d25fc Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Wed, 19 Aug 2026 18:33:50 +0000 Subject: [PATCH 2/2] fix(cli): let the caller supply the skills fetch so an offline assembly stays offline --- agentic/cli/__tests__/skills.test.ts | 9 +++++++++ agentic/cli/src/config.ts | 8 +++++++- agentic/cli/src/skills.ts | 3 ++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/agentic/cli/__tests__/skills.test.ts b/agentic/cli/__tests__/skills.test.ts index e942ae056c..0b66932106 100644 --- a/agentic/cli/__tests__/skills.test.ts +++ b/agentic/cli/__tests__/skills.test.ts @@ -14,6 +14,13 @@ const writeSkill = (root: string, name: string, body: string) => { ); }; +/** + * The base layer is fetched over HTTP, and these cases are about what assembly + * does when it cannot be had — so the fetch is refused here rather than left to + * reach api.github.com, where a slow lookup read as a test timeout. + */ +const offline = () => Promise.reject(new Error('offline: no network in this test')); + describe('agent skills assembly', () => { let home: string; @@ -29,6 +36,7 @@ describe('agent skills assembly', () => { const config = loadConfig(home); config.skillsRepo = 'example/does-not-exist'; config.skillsPin = 'v0.0.0'; + config.skillsFetch = offline; const resolved = await assembleSkills(config); expect(fs.existsSync(config.overlayDir)).toBe(true); expect(resolved).toEqual([]); @@ -38,6 +46,7 @@ describe('agent skills assembly', () => { const config = loadConfig(home); config.skillsRepo = 'example/does-not-exist'; config.skillsPin = 'v9.9.9'; + config.skillsFetch = offline; // Seed a fake cached release so the offline fallback picks it up. const releaseDir = path.join(config.dirs.skillsRoot, '9.9.9', '.agents', 'skills'); writeSkill(releaseDir, 'alpha', 'base alpha'); diff --git a/agentic/cli/src/config.ts b/agentic/cli/src/config.ts index 94970481d5..76dd6978f1 100644 --- a/agentic/cli/src/config.ts +++ b/agentic/cli/src/config.ts @@ -1,4 +1,4 @@ -import { HarnessDirs, harnessDirs, SkillsManifest } from '@agentic-kit/harness'; +import { FetchLike, HarnessDirs, harnessDirs, SkillsManifest } from '@agentic-kit/harness'; import { ConfigStore, createConfigStore } from 'appstash'; import * as fs from 'fs'; import * as path from 'path'; @@ -34,6 +34,12 @@ export interface AgentCliConfig { manifest: SkillsManifest; skillsRepo: string; skillsPin: string; + /** + * HTTP the skills fetch uses, so a caller that must not reach the network — + * a test, an air-gapped host — decides that rather than discovering it as a + * hang against api.github.com. + */ + skillsFetch?: FetchLike; } interface ManifestFile { diff --git a/agentic/cli/src/skills.ts b/agentic/cli/src/skills.ts index c3233d2be7..14ad10cc46 100644 --- a/agentic/cli/src/skills.ts +++ b/agentic/cli/src/skills.ts @@ -25,7 +25,8 @@ async function baseSource(config: AgentCliConfig, log: (msg: string) => void): P repo: config.skillsRepo, pin: config.skillsPin, skillsRoot: config.dirs.skillsRoot, - token: process.env.GITHUB_TOKEN + token: process.env.GITHUB_TOKEN, + ...(config.skillsFetch ? { fetchImpl: config.skillsFetch } : {}) }); log(`skills base: ${config.skillsRepo}@${release.version}${release.fromCache ? ' (cached)' : ''}`); return new DirectorySkillSource(BASE_LAYER, release.skillsDir);