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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- A CLI command that fails with a non-`Error` value now reports it. A protocol-shaped rejection such as `{ status: 401 }` from a broker client printed `[object Object]`, and an `Error` with an empty message printed nothing at all; both now surface the message, status, and code, with credentials redacted. Applies to the top-level failure handler, `node agent attach`, and broker request failures.
- `agent-relay integration webhook create` now works. It took a `<url>` argument and sent `{ url, event }`, but `POST /v1/webhooks` accepts `{ channel, name? }` and returns the URL — so every invocation failed with `channel is required`. It now takes `<channel>` with an optional `--name`, matching `create-inbound`, which posts to the same endpoint.
- `@agent-relay/sdk` `RelayCreateWebhookInput` declared a required `url` and an `event`, neither of which the endpoint accepts. It is now `{ channel, name? }`. Code passing `url`/`event` was already failing at runtime.

Expand Down
8 changes: 4 additions & 4 deletions packages/cli/src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { fileURLToPath } from 'node:url';
import { runAiSdkSidecarMain } from '@agent-relay/harnesses';

import { runCli } from './bootstrap.js';
import { describeError } from './lib/describe-error.js';

export * from './bootstrap.js';

Expand All @@ -25,10 +26,9 @@ if (isEntrypoint()) {
main.catch((err) => {
// Commander will have already printed a helpful message for parse errors.
// For other top-level failures, surface them to stderr and exit non-zero.
const message = err instanceof Error ? err.message : String(err);
if (message) {
process.stderr.write(`${message}\n`);
}
// `describeError` keeps a non-Error rejection (e.g. `{ status: 401 }` from
// a broker client) readable instead of collapsing it to `[object Object]`.
process.stderr.write(`${describeError(err)}\n`);
process.exit(1);
});
}
3 changes: 2 additions & 1 deletion packages/cli/src/cli/lib/attach-broker.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { HarnessDriverClient } from '@agent-relay/harness-driver';

import type { BrokerConnection } from './broker-connection.js';
import { describeError } from './describe-error.js';

export type {
InboundDeliveryMode,
Expand Down Expand Up @@ -40,5 +41,5 @@ export function mapBrokerSdkFailure(error: unknown): BrokerSdkFailure {
typeof (error as { status: unknown }).status === 'number'
? (error as { status: number }).status
: 0;
return { status, message: error instanceof Error ? error.message : String(error) };
return { status, message: describeError(error) };
}
9 changes: 5 additions & 4 deletions packages/cli/src/cli/lib/attach-drive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ import {
type PtyInputStreamOptions,
type PtyInputWriteResult,
} from '../lib/attach-broker.js';
import { describeError } from './describe-error.js';
import { createPredictiveEcho, type CreatePredictiveEchoOptions } from './predictive-echo-screen.js';
import type { PredictiveEcho } from '@agent-relay/harness-driver';

Expand Down Expand Up @@ -926,7 +927,7 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies):
// event loop on every keystroke.
void stream.send(decoded).catch((err: unknown) => {
if (settled) return;
const message = err instanceof Error ? err.message : String(err);
const message = describeError(err);
deps.log(`[drive] input stream send failed: ${message}`);
// The keystroke never reached the PTY — drop any optimistic echo
// for it so the screen doesn't show input the agent didn't get.
Expand Down Expand Up @@ -1126,7 +1127,7 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies):
}
} catch (err: unknown) {
if (settled) return;
const message = err instanceof Error ? err.message : String(err);
const message = describeError(err);
deps.error(`[drive] could not open PTY input stream: ${message}`);
finish(1);
}
Expand Down Expand Up @@ -1163,7 +1164,7 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies):
}
} catch (err: unknown) {
if (settled) return;
const message = err instanceof Error ? err.message : String(err);
const message = describeError(err);
deps.error(`[drive] could not take terminal input: ${message}`);
finish(1);
}
Expand Down Expand Up @@ -1214,7 +1215,7 @@ function runDriveSessionLoop(state: DriveSessionState, deps: DriveDependencies):
try {
await predictiveEcho.seed(snapshotBytes);
} catch (err: unknown) {
const message = err instanceof Error ? err.message : String(err);
const message = describeError(err);
deps.log(`[drive] could not seed predictive echo: ${message}`);
finish(1);
return;
Expand Down
7 changes: 4 additions & 3 deletions packages/cli/src/cli/lib/attach-native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
type BrokerConnection,
} from './broker-connection.js';
import { createBrokerClient } from './attach-broker.js';
import { describeError } from './describe-error.js';

export type NativeAttachMode = 'view' | 'drive' | 'passthrough';

Expand Down Expand Up @@ -146,7 +147,7 @@ export async function attachNative(
try {
brokerCursor = await client.currentEventSeq();
} catch (error) {
deps.output.stderr(`Error: ${error instanceof Error ? error.message : String(error)}\n`);
deps.output.stderr(`Error: ${describeError(error)}\n`);
client.disconnect();
return 1;
}
Expand Down Expand Up @@ -255,7 +256,7 @@ export async function attachNative(
}
})
.catch((error: unknown) => {
deps.output.stderr(`Input failed: ${error instanceof Error ? error.message : String(error)}\n`);
deps.output.stderr(`Input failed: ${describeError(error)}\n`);
})
.then(() => undefined);
commandChain = pendingCommand.catch(() => undefined);
Expand All @@ -279,7 +280,7 @@ export async function attachNative(
await Promise.allSettled(pendingCommands);
return 0;
} catch (error) {
deps.output.stderr(`Error: ${error instanceof Error ? error.message : String(error)}\n`);
deps.output.stderr(`Error: ${describeError(error)}\n`);
return 1;
} finally {
stopped = true;
Expand Down
9 changes: 5 additions & 4 deletions packages/cli/src/cli/lib/attach-passthrough.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ import {
resizeWorker,
type InboundDeliveryMode,
} from './attach-drive.js';
import { describeError } from './describe-error.js';
import { createPredictiveEcho, type CreatePredictiveEchoOptions } from './predictive-echo-screen.js';
import type { PredictiveEcho } from '@agent-relay/harness-driver';

Expand Down Expand Up @@ -560,7 +561,7 @@ export async function runPassthroughSession(
if (decoded.length > 0) {
void stream.send(decoded).catch((err: unknown) => {
if (settled) return;
const message = err instanceof Error ? err.message : String(err);
const message = describeError(err);
deps.log(`[passthrough] input stream send failed: ${message}`);
// The keystroke never reached the PTY — drop any optimistic echo
// for it so the screen doesn't show input the agent didn't get.
Expand Down Expand Up @@ -740,7 +741,7 @@ export async function runPassthroughSession(
}
} catch (err: unknown) {
if (settled) return;
const message = err instanceof Error ? err.message : String(err);
const message = describeError(err);
deps.error(`[passthrough] could not open PTY input stream: ${message}`);
finish(1);
}
Expand Down Expand Up @@ -775,7 +776,7 @@ export async function runPassthroughSession(
}
} catch (err: unknown) {
if (settled) return;
const message = err instanceof Error ? err.message : String(err);
const message = describeError(err);
deps.error(`[passthrough] could not take terminal input: ${message}`);
finish(1);
}
Expand Down Expand Up @@ -825,7 +826,7 @@ export async function runPassthroughSession(
try {
await predictiveEcho.seed(snapshotBytes);
} catch (err: unknown) {
const message = err instanceof Error ? err.message : String(err);
const message = describeError(err);
deps.log(`[passthrough] could not seed predictive echo: ${message}`);
finish(1);
return;
Expand Down
18 changes: 9 additions & 9 deletions packages/cli/src/cli/lib/broker-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest';
import {
classifyBrokerStartError,
classifyBrokerStartStage,
describeError,
describeErrorWithCause,
isBundledBunExecutableEntrypoint,
readNodeDeliveryStatus,
resolveNodeIdentityFromSession,
Expand Down Expand Up @@ -34,9 +34,9 @@ describe('isBundledBunExecutableEntrypoint', () => {
});
});

describe('describeError', () => {
describe('describeErrorWithCause', () => {
it('returns plain message for a bare Error', () => {
expect(describeError(new Error('boom'))).toBe('boom');
expect(describeErrorWithCause(new Error('boom'))).toBe('boom');
});

it('unwraps the Node fetch failed cause and surfaces the network code', () => {
Expand All @@ -46,7 +46,7 @@ describe('describeError', () => {
});
const err = new TypeError('fetch failed', { cause });

const result = describeError(err);
const result = describeErrorWithCause(err);
expect(result).toContain('fetch failed');
expect(result).toContain('ECONNREFUSED');
expect(result).toContain('127.0.0.1');
Expand All @@ -58,15 +58,15 @@ describe('describeError', () => {
});
const err = new TypeError('fetch failed', { cause });

const result = describeError(err);
const result = describeErrorWithCause(err);
expect(result).toContain('ENOTFOUND');
expect(result).toContain('agentrelay.com');
});

it('handles non-Error values without throwing', () => {
expect(describeError('something went wrong')).toBe('something went wrong');
expect(describeError(undefined)).toBe('undefined');
expect(describeError(null)).toBe('null');
expect(describeErrorWithCause('something went wrong')).toBe('something went wrong');
expect(describeErrorWithCause(undefined)).toBe('undefined');
expect(describeErrorWithCause(null)).toBe('null');
});

it('caps the cause-chain walk so a cycle cannot loop forever', () => {
Expand All @@ -75,7 +75,7 @@ describe('describeError', () => {
a.cause = b;
b.cause = a;
// Just needs to terminate — the assertion is the absence of a hang.
expect(typeof describeError(a)).toBe('string');
expect(typeof describeErrorWithCause(a)).toBe('string');
});
});

Expand Down
12 changes: 8 additions & 4 deletions packages/cli/src/cli/lib/broker-lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
type NodeDefinitionDescriptor,
type RunningNodeProviderChild,
} from './node-provider-child.js';
import { describeError } from './describe-error.js';
import { maskSecret } from './redact.js';
import { startReflexCapture, type RunningReflexCapture } from './reflex-capture.js';
import { projectWorkspaceKeyPath, writeProjectWorkspaceKey } from './project-workspace-key.js';
Expand Down Expand Up @@ -152,7 +153,7 @@ export function readBrokerConnection(dataDir: string): BrokerConnection | null {
}

function toErrorMessage(err: unknown): string {
return err instanceof Error ? err.message : String(err);
return describeError(err);
}

/** Emit a `[verbose]`-prefixed step marker via `deps.log` when `--verbose` is set. */
Expand Down Expand Up @@ -258,14 +259,17 @@ function errorCode(err: unknown): string | undefined {
/**
* Extract a human-meaningful detail string from an error, walking `err.cause`.
*
* The broker-start specialization of {@link describeError}: it starts from the
* shared description and appends the cause chain's detail and error codes.
*
* Node's native `fetch()` throws `TypeError: fetch failed` for any network
* problem and stuffs the real reason (ECONNREFUSED, ENOTFOUND, AbortError,
* UND_ERR_CONNECT_TIMEOUT, …) into `err.cause`. Without unwrapping, every
* outbound HTTP failure looks identical to the user.
*
* Exported for testing.
*/
export function describeError(err: unknown): string {
export function describeErrorWithCause(err: unknown): string {
const top = toErrorMessage(err);
if (!(err instanceof Error) || !err.cause) return top;

Expand Down Expand Up @@ -1392,7 +1396,7 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies):
env: deps.env,
});
} catch (err: unknown) {
deps.error(`Failed to start broker in background: ${describeError(err)}`);
deps.error(`Failed to start broker in background: ${describeErrorWithCause(err)}`);
deps.exit(1);
return;
}
Expand Down Expand Up @@ -1666,7 +1670,7 @@ export async function runUpCommand(options: UpOptions, deps: CoreDependencies):
if (isBrokerAlreadyRunningError(message)) {
reportAlreadyRunningError(message, paths.dataDir, deps);
} else {
deps.error(`Failed to start broker: ${describeError(err)}`);
deps.error(`Failed to start broker: ${describeErrorWithCause(err)}`);
}
deps.exit(1);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/cli/src/cli/lib/core-maintenance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import path from 'node:path';
import type { CoreDependencies, CoreFileSystem } from '../commands/core.js';
import { track } from '../telemetry/index.js';
import { readBrokerConnection } from './broker-lifecycle.js';
import { describeError } from './describe-error.js';
import { errorClassName } from './telemetry-helpers.js';

const SNIPPET_MARKER_START_PREFIX = '<!-- prpm:snippet:start @agent-relay/agent-relay-snippet@';
Expand All @@ -17,7 +18,7 @@ const DEFAULT_ZED_SERVER_NAME = 'Agent Relay';
const INSTALL_DIR_NAMES = ['.agentworkforce/relay', '.agent-relay'] as const;

function toErrorMessage(err: unknown): string {
return err instanceof Error ? err.message : String(err);
return describeError(err);
}

/**
Expand Down
87 changes: 87 additions & 0 deletions packages/cli/src/cli/lib/describe-error.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import { describe, expect, it } from 'vitest';

import { describeError } from './describe-error.js';

describe('describeError', () => {
it('renders an Error as its message', () => {
expect(describeError(new Error('broker refused the attach'))).toBe('broker refused the attach');
});

it('falls back to name and cause when an Error has no message', () => {
expect(describeError(new Error(''))).toBe('Error');
const named = new Error('');
named.name = 'AttachError';
expect(describeError(named)).toBe('AttachError');
expect(describeError(new Error('', { cause: new Error('socket hang up') }))).toBe(
'Error: socket hang up'
);
});

it('keeps a protocol-shaped rejection readable instead of [object Object]', () => {
expect(describeError({ status: 401 })).toBe('{"status":401}');
expect(describeError({ status: 401, code: 'unauthorized', message: 'invalid workspace key' })).toBe(
'invalid workspace key (status 401, code unauthorized)'
);
expect(describeError({ error: 'invalid api key', status: 403 })).toBe('invalid api key (status 403)');
});

it('unwraps a nested error object', () => {
expect(describeError({ error: { message: 'node offline' }, code: 'node_unreachable' })).toBe(
'node offline (code node_unreachable)'
);
});

it('redacts credentials in the JSON fallback', () => {
const described = describeError({ status: 401, workspaceKey: 'rk_live_supersecret' });
expect(described).not.toContain('rk_live_supersecret');
expect(described).toContain('[redacted]');
});

it('redacts a credential echoed back inside the message text', () => {
// A server error payload is free to quote whatever was sent to it, and that
// text is lifted out whole — key-name redaction cannot reach it.
expect(describeError({ status: 401, message: 'invalid key rk_live_abcdefghij1234' })).toBe(
'invalid key rk_live_…1234 (status 401)'
);
expect(describeError(new Error('enroll failed for nt_live_0123456789abcdef'))).toBe(
'enroll failed for nt_live_…cdef'
);
});

it('keeps JSON-hostile members readable', () => {
// JSON.stringify throws on BigInt; the old fallback answered [object Object].
expect(describeError({ status: 429, retryAfter: 30n })).toBe('{"status":429,"retryAfter":"30n"}');

const throwingGetter = {
get status(): number {
throw new Error('nope');
},
};
expect(describeError(throwingGetter)).toBe('[unserializable Object]');
});

it('tolerates cycles and truncates oversized payloads', () => {
const cyclic: Record<string, unknown> = { status: 500 };
cyclic.self = cyclic;
expect(describeError(cyclic)).toContain('[circular]');

const huge = describeError({ status: 500, blob: 'x'.repeat(5000) });
expect(huge.length).toBeLessThanOrEqual(501);
expect(huge.endsWith('…')).toBe(true);
});

it('renders primitives and empty values without going blank', () => {
expect(describeError('connection reset')).toBe('connection reset');
expect(describeError(404)).toBe('404');
expect(describeError(null)).toBe('null');
expect(describeError(undefined)).toBe('undefined');
expect(describeError('')).toBe('Unknown error');
expect(describeError({})).toBe('{}');
});

it('never returns the useless [object Object] rendering', () => {
for (const value of [{ status: 401 }, {}, Object.create(null), [{ status: 401 }]]) {
expect(describeError(value)).not.toBe('[object Object]');
}
});
});
Loading
Loading