feat(sdk): expose the error envelope's type and code on API errors (#470)

`ManagedAgentsApiError` exposed only `status` and a message built from
`error.message`, so the runtime's error taxonomy was unreachable from the SDK:

  API error 400: agent must be a standard agent id

The published envelope is structural, and the runtime builds it in one place
(`src/api/routes/sessions.ts:766`):

  {"error":{"type":"invalid_request_error","code":"invalid_agent_ref","message":"..."}}

A caller is meant to branch on that identity. Without it, distinguishing a wrong
agent reference from a malformed body means substring-matching English prose that
is not a stable interface, and any rewording of a message is a silent breaking
change for the consumer that was forced to match on it. This closes the client
half of the error taxonomy (#464) and does not change the canonical type (D11).

* `error.type` and `error.code` carry the runtime's own values, and are
  `undefined` when the response carried no envelope or named no specific cause
  (`not_found` is a type, not a code under `invalid_request_error`).

* The message is deliberately **unchanged** — `API error 400: agent must be a
  standard agent id` — so this is additive for anyone already matching on it.
  Measured: only one assertion in the suite matches an error message
  (`tests/integration/sdk.test.ts:251`, `/API error 404/`).

* The error body is now read exactly once. The previous implementation called
  `res.json()` and then `res.text()` on the same `Response`, whose body is
  single-use, so the non-JSON fallback could never succeed: a non-JSON failure
  was reported as `statusText`, not as the body the runtime sent. The fallback
  was dead code that looked alive.

* `requestText` — which backs artifact and file content plus metrics — goes
  through the same reader. It used the raw body as the message, so a JSON
  envelope was reported as raw JSON text with no fields readable.

* A JSON body that is not an envelope keeps the previous `statusText` fallback,
  so only the envelope's fields are newly reachable; an unrelated JSON payload
  cannot leak into a log line.

Verification: `npm run release:check` — see PR body for counts. The integration
test drives a real listener over real SQLite, so the codes are produced by the
routes: `invalid_agent_ref` and `agent_required` from `POST /v1/sessions`,
`not_found` from `GET /v1/sessions/{id}` and from the `requestText` path, and the
message pinned byte-for-byte so adding fields cannot alter the sentence existing
callers log. The non-JSON and malformed-envelope branches are covered by a
stubbed fetch, because no route in the suite answers a failure with a non-JSON
body.

Not covered: no Docker daemon, no Kubernetes cluster, and no model provider
credentials exist on this host, so no provider boundary is exercised;
`invalid_initial_events` was not reachable, because that route resolves a default
environment before it validates `initial_events` and answers
`Environment not found: env_default` with no code.

Reverse probes, each differing from the fix in one respect:
  A. type/code dropped from the constructor -> see PR body
  B. body read twice, as before             -> see PR body
  C. envelope required to be read from the
     raw text rather than parsed            -> see PR body
This commit is contained in:
Yao
2026-09-25 17:17:07 +08:00
committed by GitHub
parent 316f92ae3f
commit e8c021bc10
5 changed files with 310 additions and 12 deletions
+2
View File
@@ -4,6 +4,8 @@
### Added
- Exposes the published error envelope's `type` and `code` on `ManagedAgentsApiError`, closing the client half of the error taxonomy. The envelope is structural — `{"error":{"type":"invalid_request_error","code":"invalid_agent_ref","message":"..."}}` — and a caller is meant to branch on it, but the SDK read only `error.message`, so the runtime's entire taxonomy was unreachable and a caller wanting to tell a wrong agent reference from a malformed body had to substring-match English prose that is not a stable interface. `error.type` and `error.code` now carry the runtime's own values, which makes a future rewording of a message safe rather than a silent breaking change for a consumer that was forced to match on it — the code that the previous change had to print as prose (`model.api_key: TYPO is not set`) is now readable as a value. The message is deliberately **unchanged** (`API error 400: agent must be a standard agent id`), so this is additive for anyone already matching on it. The error body is also now read exactly once: the previous implementation called `res.json()` and then `res.text()` on the same `Response`, whose body is single-use, so the non-JSON fallback could never succeed — a non-JSON failure was reported as `statusText` rather than as the body the runtime actually sent, and the fallback was dead code that looked alive. `requestText`, which backs artifact and file content plus metrics, goes through the same reader, so a JSON envelope is no longer reported as raw JSON text with no fields readable.
- Registers the documented `managed-agents settings` group and moves the SDK's canonical runtime settings helpers onto the routes they claim to call. The group is documented as covered in `docs/api-matrix.md:86` — "Get, set model boundary, and validate canonical runtime settings" — and ticked in `docs/spec/tasks.md:144`, and its three commands are implemented in `src/cli/runtime-management-commands.ts`, the module the previous change registered only halfway: these are the commands it deliberately left unregistered rather than register broken (**#466**). Measured against a live listener, `settings get` threw `Cannot read properties of undefined (reading 'metadata')`, `settings validate` threw `result.checks is not iterable`, and `settings set-model` failed with `API error 404: No route matches this request`. The first two read a document taken from a type no route produces — the runtime's internal registry report, with `model_provider`, `loop_engine.implemented`, `storage.metadata.type` and `validation.checks` — so every field a reader could reach for was `undefined` at runtime while type-checking; the third sent `PATCH /v1/x/settings`, where the route mounts `PUT`. `settings.patch()` is therefore a read-merge-write: it reads the current `revision` and `saved_config`, merges the partial document over it, validates the result, and writes it with the revision it read, so a masked secret survives the round trip and a concurrent writer is refused with `409` rather than silently overwritten — there is no automatic retry, because the SDK cannot know whether re-applying the patch is still meaningful. A document the runtime would reject throws `RuntimeSettingsValidationError` before the write, naming each failing field, including the environment variable behind a `${NAME}` reference that does not resolve in the **runtime's** environment; that detail is precisely what the shared error path drops (**#464**), and without it `settings set-model --api-key-env TYPO` would report only `Settings configuration is invalid`. The SDK's `RuntimeSettingsSummary` and `RuntimeSettingsPatch` now describe the routes' own fields, secrets are documented as surviving the round trip because reads mask a stored key as `********` and the store treats that sentinel as "keep", and an integration test drives a real HTTP listener through a recording `fetch` that delegates to it rather than replacing it, so the verb asserted is the verb served.
- Registers the documented `managed-agents workspace` group. All five commands — `create`, `open`, `list`, `resolve`, `remove` — are implemented in `src/cli/workspace-commands.ts`, documented as covered in `docs/api-matrix.md:88`, and ticked off in `docs/spec/tasks.md:192`, but the module was imported by nothing, so every one answered `error: unknown command 'workspace'`. This is the fourth and last instance of that pattern (#459/#463 worker, #460/#465 session, #461/#467 environments). It is also the one group that is not an HTTP client: `src/core/workspace/registry.ts` reads and writes `$MANAGED_AGENTS_HOME/workspaces.json`, so these commands take no `--port` or `--api-key`, and the whole workspace-registry subsystem becomes reachable for the first time — the orphaned CLI module was its **only** consumer, so before this it was dead code with a ticked task-list entry. Tests point the registry at a temporary directory through `MANAGED_AGENTS_HOME`, which is what makes them runnable without writing to the developer's real `~/.managed-agents`; that variable was documented only in `CONTRIBUTING.md` and is now in `docs/usage.md` as well, since the CLI is the surface that needs it. Three behaviors are pinned that the printed output alone would not catch. `create` scaffolds `<root>/agents`, `<root>/skills`, and `<root>/.managed-agents/config.yaml`, asserted **on disk**, while `open` must create nothing at all — asserted by the absence of both scaffold directories, so the two are not interchangeable. `remove` drops the registry entry and **leaves every file behind**, which is asserted because the same command turning into a recursive delete would destroy a user's agents and sessions without a prompt. And `resolve` is not a read: it bumps `last_opened_at` and therefore reorders the listing, so a caller resolving in a loop silently rewrites the registry. `resolve` and `remove` set `process.exitCode = 1` on a miss, which the tests capture and restore rather than leaving set. Documentation updated in `docs/usage.md`: the registry location and `MANAGED_AGENTS_HOME` under Workspace Layout, the five commands in the CLI list, and a paragraph distinguishing `create` from `open`.
+24
View File
@@ -414,6 +414,30 @@ for await (const event of client.sessions.chat(session.id, 'Hello')) {
}
```
A refused request throws `ManagedAgentsApiError`, which carries the published
error envelope's identity as well as its prose: `status`, `type` (for example
`invalid_request_error`, `not_found`, or `conflict`), and `code` when the
runtime names a specific cause (`invalid_agent_ref`, `budget_reached`,
`unsupported_model_field`, and others). Branch on those instead of on the
message, which is prose and may be reworded:
```typescript
import { ManagedAgentsApiError } from 'managed-agents/sdk';
try {
await client.sessions.create({ agent: 'assistant' });
} catch (error) {
if (error instanceof ManagedAgentsApiError && error.code === 'invalid_agent_ref') {
// `agent` takes an agent id, not a name.
}
throw error;
}
```
`type` and `code` are `undefined` when the response carried no envelope or the
runtime named no specific cause. The message is unchanged, so existing code that
matches on it keeps working.
## CLI Commands
```bash
+58 -9
View File
@@ -414,14 +414,8 @@ export class ManagedAgentsClient {
});
if (!res.ok) {
let detail = '';
try {
const j = (await res.json()) as { error?: { message?: string } };
detail = j.error?.message ?? '';
} catch {
detail = await res.text().catch(() => '');
}
throw new ManagedAgentsApiError(res.status, detail || res.statusText);
const envelope = await readErrorEnvelope(res);
throw new ManagedAgentsApiError(res.status, envelope.message || res.statusText, envelope);
}
if (res.status === 204) return undefined as T;
@@ -435,7 +429,8 @@ export class ManagedAgentsClient {
const res = await this.fetchImpl(`${this.baseUrl}${path}`, { method, headers });
if (!res.ok) {
throw new ManagedAgentsApiError(res.status, await res.text().catch(() => res.statusText));
const envelope = await readErrorEnvelope(res);
throw new ManagedAgentsApiError(res.status, envelope.message || res.statusText, envelope);
}
return res.text();
}
@@ -917,13 +912,67 @@ class EnvironmentsResource {
// Errors + SSE parsing
// ============================================================
/**
* The published error envelope: `{"error":{"type":..., "code":..., "message":...}}`.
*
* `type` is the canonical error class (D11: `invalid_request_error`, with `not_found` and
* `conflict` deliberately kept as their own), and `code` names a specific cause within it.
* A caller branches on these; the message is prose and is not a stable interface.
*/
export interface ApiErrorEnvelope {
type?: string;
code?: string;
}
export class ManagedAgentsApiError extends Error {
/**
* The envelope's `type`, e.g. `invalid_request_error`.
*
* `undefined` when the response carried no envelope — a non-JSON body, or a transport that
* failed before the runtime answered.
*/
readonly type?: string;
/** The envelope's `code`, e.g. `invalid_agent_ref`, when the cause is specific enough to name. */
readonly code?: string;
constructor(
public readonly status: number,
message: string,
envelope: ApiErrorEnvelope = {},
) {
super(`API error ${status}: ${message}`);
this.name = 'ManagedAgentsApiError';
this.type = envelope.type;
this.code = envelope.code;
}
}
/**
* Read a failed response's published envelope, consuming the body exactly once.
*
* The previous version called `res.json()` and then `res.text()` on the same response. A
* `Response` body is single-use, so the fallback could never succeed — and `error.type` and
* `error.code` were never read at all, which is what made the runtime's whole error taxonomy
* unreachable from the SDK.
*
* A body that is not JSON is returned as the message, which is what the dead fallback was for;
* a JSON body that is not an envelope keeps the previous `statusText` fallback, so only the
* envelope's fields are newly reachable.
*/
async function readErrorEnvelope(res: Response): Promise<{ message: string } & ApiErrorEnvelope> {
const raw = await res.text().catch(() => '');
if (!raw) return { message: '' };
try {
const parsed = JSON.parse(raw) as { error?: { type?: unknown; code?: unknown; message?: unknown } } | null;
const error = parsed?.error;
if (!error || typeof error !== 'object') return { message: '' };
return {
message: typeof error.message === 'string' ? error.message : '',
...(typeof error.type === 'string' ? { type: error.type } : {}),
...(typeof error.code === 'string' ? { code: error.code } : {}),
};
} catch {
return { message: raw };
}
}
@@ -0,0 +1,150 @@
/**
* Integration test: the published error envelope's `type` and `code` reach the SDK caller.
*
* `ManagedAgentsApiError` exposed only `status` and a message built from `error.message`, so
* the runtime's error taxonomy was unreachable from the SDK: a caller wanting to tell a wrong
* agent reference from a malformed body had to substring-match English prose that is not a
* stable interface. The envelope is structural —
* `{"error":{"type":"invalid_request_error","code":"invalid_agent_ref","message":"..."}}` —
* and `src/api/routes/sessions.ts:766` builds it in one place.
*
* Every assertion here is against a **real listener over real SQLite**: the codes are produced
* by the routes themselves, so a test that agreed with a wrong SDK would fail. The message is
* also pinned byte-for-byte, because adding fields must not change the sentence existing
* callers already log.
*/
import { afterEach, describe, expect, it } from 'vitest';
import { join } from 'node:path';
import { mkdirSync, mkdtempSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { serve } from '@hono/node-server';
import { Database } from '@/core/db/database.js';
import { SessionManager } from '@/core/session/session-manager.js';
import { createServer } from '@/api/server.js';
import { ManagedAgentsApiError, ManagedAgentsClient } from '@/sdk/client.js';
type RealServer = { close: (cb?: () => void) => void; closeAllConnections?: () => void };
describe('SDK error envelope', () => {
let db: Database | undefined;
let tmpDir: string | undefined;
let listening: RealServer | undefined;
afterEach(async () => {
if (listening) {
const server = listening;
listening = undefined;
server.closeAllConnections?.();
await new Promise<void>((resolve) => server.close(() => resolve()));
}
db?.close();
db = undefined;
if (tmpDir) rmSync(tmpDir, { recursive: true, force: true });
tmpDir = undefined;
});
async function startRuntime() {
const dir = mkdtempSync(join(tmpdir(), 'ma-sdk-error-'));
tmpDir = dir;
const dataDir = join(dir, '.managed-agents');
mkdirSync(dataDir, { recursive: true });
db = new Database(join(dir, 'test.db'));
db.runMigrations();
const app = createServer({
db,
sessionManager: new SessionManager(db),
agents: [],
reloadAgents: () => ({ agents: [], errors: [] }),
workspace: {
root: dir,
dataDir,
agentsDir: join(dir, 'agents'),
skillsDir: join(dir, 'skills'),
configPath: join(dir, 'managed-agents.config.yaml'),
target: 'local',
},
});
const port = await new Promise<number>((resolve) => {
const server = serve({ fetch: app.fetch, port: 0, hostname: '127.0.0.1' }, (info) => {
listening = server as unknown as RealServer;
resolve(info.port);
});
});
return { client: new ManagedAgentsClient({ baseUrl: `http://127.0.0.1:${port}` }), app };
}
/** Run a call whose refusal is the point, and return the error it threw. */
async function captureError(run: () => Promise<unknown>): Promise<ManagedAgentsApiError> {
const failure = await run().then(() => undefined, (error: unknown) => error);
expect(failure).toBeInstanceOf(ManagedAgentsApiError);
return failure as ManagedAgentsApiError;
}
it('carries the canonical type and the specific code of a real 400', async () => {
const { client } = await startRuntime();
const error = await captureError(() => client.sessions.create({ agent: 'not-an-agent-id' }));
expect(error.status).toBe(400);
// The whole point: a caller can branch on this instead of matching prose.
expect(error.type).toBe('invalid_request_error');
expect(error.code).toBe('invalid_agent_ref');
// `code`/`type` are additive: the sentence existing callers log is unchanged, so this is
// not a breaking change for anyone already matching on it.
expect(error.message).toBe('API error 400: agent must be a standard agent id');
});
it('carries a different code from the same route, so the field is read not guessed', async () => {
const { client } = await startRuntime();
// A second code from a second failure on the same path: if the SDK were reporting a
// constant, or reading the wrong field, one of these two would disagree.
const error = await captureError(() => client.sessions.create({} as never));
expect(error.status).toBe(400);
expect(error.type).toBe('invalid_request_error');
expect(error.code).toBe('agent_required');
expect(error.message).toBe('API error 400: agent field is required');
});
it('reports a type with no code when the refusal names no specific cause', async () => {
const { client } = await startRuntime();
const error = await captureError(() => client.sessions.get('sess_does_not_exist'));
expect(error.status).toBe(404);
// `not_found` is deliberately its own type, not a code under `invalid_request_error`
// (D11) — and this refusal carries no code.
expect(error.type).toBe('not_found');
expect(error.code).toBeUndefined();
expect(error.message).toBe('API error 404: Session not found');
});
it('carries the envelope on the text-response path too', async () => {
const { client } = await startRuntime();
// `requestText` backs artifact and file content plus metrics. It built its error from the
// raw body, so a JSON envelope was reported as raw JSON with no fields readable.
const error = await captureError(() => client.sessions.artifactText('sess_missing', 'art_missing'));
expect(error.status).toBe(404);
expect(error.type).toBe('not_found');
// The prose, not the `{"error":{...}}` body that the raw-text path used to report.
expect(error.message).not.toContain('{');
expect(error.message).toContain('not found');
});
it('is an Error, so existing catch blocks and rethrows keep working', async () => {
const { client } = await startRuntime();
const error = await captureError(() => client.sessions.get('sess_does_not_exist'));
expect(error).toBeInstanceOf(Error);
expect(error.name).toBe('ManagedAgentsApiError');
expect(typeof error.stack).toBe('string');
});
});
+76 -3
View File
@@ -1,5 +1,77 @@
import { describe, expect, it, vi } from 'vitest';
import { ManagedAgentsClient } from '@/sdk/client.js';
import { ManagedAgentsApiError, ManagedAgentsClient } from '@/sdk/client.js';
describe('ManagedAgentsClient error envelope', () => {
const clientReturning = (response: Response) =>
new ManagedAgentsClient({
baseUrl: 'http://localhost:3000',
fetch: (async () => response) as unknown as typeof fetch,
});
it('reports a non-JSON error body as the message', async () => {
// A stubbed fetch, because no route in the suite answers a failure with a non-JSON body:
// this covers the fallback branch, which is also the branch that was dead. The previous
// implementation read `res.json()` and then `res.text()` on the same response, so the
// single-use body was already consumed and the fallback always produced `statusText`.
const client = clientReturning(new Response('upstream is unavailable', { status: 503 }));
const error = await client.sessions.get('sess_1').then(() => undefined, (e: unknown) => e) as ManagedAgentsApiError;
expect(error).toBeInstanceOf(ManagedAgentsApiError);
expect(error.status).toBe(503);
// The body that was actually sent, not `Service Unavailable`.
expect(error.message).toBe('API error 503: upstream is unavailable');
expect(error.type).toBeUndefined();
expect(error.code).toBeUndefined();
});
it('falls back to the status text for an empty error body', async () => {
const client = clientReturning(new Response('', { status: 502, statusText: 'Bad Gateway' }));
const error = await client.sessions.get('sess_1').then(() => undefined, (e: unknown) => e) as ManagedAgentsApiError;
expect(error.message).toBe('API error 502: Bad Gateway');
});
it('falls back to the status text for JSON that is not an envelope', async () => {
// A JSON body without an `error` object is not the published envelope, so it must not be
// reported as the message — otherwise an unrelated JSON payload would leak into logs.
const client = clientReturning(jsonResponse({ valid: false }, 500, 'Internal Server Error'));
const error = await client.sessions.get('sess_1').then(() => undefined, (e: unknown) => e) as ManagedAgentsApiError;
expect(error.message).toBe('API error 500: Internal Server Error');
});
it('reads type and code from a well-formed envelope', async () => {
const response = jsonResponse(
{ error: { type: 'invalid_request_error', code: 'invalid_agent_ref', message: 'agent must be a standard agent id' } },
400,
);
const client = clientReturning(response);
const error = await client.sessions.get('sess_1').then(() => undefined, (e: unknown) => e) as ManagedAgentsApiError;
expect(error.type).toBe('invalid_request_error');
expect(error.code).toBe('invalid_agent_ref');
expect(error.message).toBe('API error 400: agent must be a standard agent id');
});
it('ignores non-string envelope fields instead of reporting them', async () => {
// A malformed envelope must not put `[object Object]` or a number into a typed field.
const response = jsonResponse(
{ error: { type: 7, code: { nested: true }, message: 'bad request' } },
400,
);
const client = clientReturning(response);
const error = await client.sessions.get('sess_1').then(() => undefined, (e: unknown) => e) as ManagedAgentsApiError;
expect(error.type).toBeUndefined();
expect(error.code).toBeUndefined();
expect(error.message).toBe('API error 400: bad request');
});
});
describe('ManagedAgentsClient runtime management resources', () => {
it('sends tool confirmation and custom tool result events', async () => {
@@ -139,9 +211,10 @@ describe('ManagedAgentsClient runtime management resources', () => {
});
});
function jsonResponse(value: unknown): Response {
function jsonResponse(value: unknown, status = 200, statusText?: string): Response {
return new Response(JSON.stringify(value), {
status: 200,
status,
...(statusText === undefined ? {} : { statusText }),
headers: { 'Content-Type': 'application/json' },
});
}