diff --git a/.agents/skills/agent-core-review/SKILL.md b/.agents/skills/agent-core-review/SKILL.md index 172732cbf..64df636e0 100644 --- a/.agents/skills/agent-core-review/SKILL.md +++ b/.agents/skills/agent-core-review/SKILL.md @@ -1,5 +1,5 @@ --- -name: kc-review +name: agent-core-review description: Use ONLY for code review and test write/review guidance in `packages/agent-core-v2` (the DI × Scope agent engine). Does NOT apply to the legacy `packages/agent-core` or to any other package — for those, do not load this skill. Groups the review and testing lenses used for agent-core-v2 — `slop` (single-level-of-abstraction / layered error-handling review, invoked only on explicit request) and `test` (contract-driven per-test rules for both authoring and reviewing tests). Apply the sub-skill that matches the task; do not apply `slop` unprompted. has-sub-skill: true --- diff --git a/packages/kap-server/src/error-handler.ts b/packages/kap-server/src/error-handler.ts index b48083a6e..e2d17e75c 100644 --- a/packages/kap-server/src/error-handler.ts +++ b/packages/kap-server/src/error-handler.ts @@ -45,6 +45,7 @@ export function installErrorHandler(app: ErrorHandlerHost): void { ErrorCode.INTERNAL_ERROR, err.message !== undefined && err.message !== '' ? err.message : 'internal error', requestId, + err.stack, ), ); }); diff --git a/packages/kap-server/src/routes/fs.ts b/packages/kap-server/src/routes/fs.ts index 007c57c0b..7a68b3aca 100644 --- a/packages/kap-server/src/routes/fs.ts +++ b/packages/kap-server/src/routes/fs.ts @@ -483,34 +483,34 @@ function sendMappedError(reply: Reply, requestId: string, err: unknown): void { if (isKimiError(err)) { switch (err.code) { case ErrorCodes.FS_PATH_ESCAPES: - reply.send(errEnvelope(ErrorCode.FS_PATH_ESCAPES_SESSION, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_PATH_ESCAPES_SESSION, err.message, requestId, err.stack)); return; case ErrorCodes.FS_PATH_NOT_FOUND: - reply.send(errEnvelope(ErrorCode.FS_PATH_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_PATH_NOT_FOUND, err.message, requestId, err.stack)); return; case ErrorCodes.FS_IS_DIRECTORY: - reply.send(errEnvelope(ErrorCode.FS_IS_DIRECTORY, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_IS_DIRECTORY, err.message, requestId, err.stack)); return; case ErrorCodes.FS_ALREADY_EXISTS: - reply.send(errEnvelope(ErrorCode.FS_ALREADY_EXISTS, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_ALREADY_EXISTS, err.message, requestId, err.stack)); return; case ErrorCodes.FS_IS_BINARY: - reply.send(errEnvelope(ErrorCode.FS_IS_BINARY, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_IS_BINARY, err.message, requestId, err.stack)); return; case ErrorCodes.FS_TOO_LARGE: - reply.send(errEnvelope(ErrorCode.FS_TOO_LARGE, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_TOO_LARGE, err.message, requestId, err.stack)); return; case ErrorCodes.FS_TOO_MANY_RESULTS: - reply.send(errEnvelope(ErrorCode.FS_TOO_MANY_RESULTS, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_TOO_MANY_RESULTS, err.message, requestId, err.stack)); return; case ErrorCodes.FS_GREP_TIMEOUT: - reply.send(errEnvelope(ErrorCode.FS_GREP_TIMEOUT, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_GREP_TIMEOUT, err.message, requestId, err.stack)); return; case ErrorCodes.FS_GIT_UNAVAILABLE: - reply.send(errEnvelope(ErrorCode.FS_GIT_UNAVAILABLE, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_GIT_UNAVAILABLE, err.message, requestId, err.stack)); return; case ErrorCodes.SESSION_NOT_FOUND: - reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, requestId, err.stack)); return; } } @@ -519,6 +519,7 @@ function sendMappedError(reply: Reply, requestId: string, err: unknown): void { ErrorCode.INTERNAL_ERROR, err instanceof Error ? err.message : String(err), requestId, + err instanceof Error ? err.stack : undefined, ), ); } diff --git a/packages/kap-server/src/routes/messages.ts b/packages/kap-server/src/routes/messages.ts index 398057f22..da205b8b1 100644 --- a/packages/kap-server/src/routes/messages.ts +++ b/packages/kap-server/src/routes/messages.ts @@ -158,14 +158,14 @@ function sendMappedError( if (isKimiError(err)) { switch (err.code) { case 'session.not_found': - reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, requestId, err.stack)); return; case 'message.not_found': - reply.send(errEnvelope(ErrorCode.MESSAGE_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.MESSAGE_NOT_FOUND, err.message, requestId, err.stack)); return; } } reply.send( - errEnvelope(ErrorCode.INTERNAL_ERROR, err instanceof Error ? err.message : String(err), requestId), + errEnvelope(ErrorCode.INTERNAL_ERROR, err instanceof Error ? err.message : String(err), requestId, err instanceof Error ? err.stack : undefined), ); } diff --git a/packages/kap-server/src/routes/modelCatalog.ts b/packages/kap-server/src/routes/modelCatalog.ts index 18c21374b..4aebbad6a 100644 --- a/packages/kap-server/src/routes/modelCatalog.ts +++ b/packages/kap-server/src/routes/modelCatalog.ts @@ -290,11 +290,11 @@ function sendMappedError( ): boolean { if (!isKimiError(err)) return false; if (err.code === 'provider.not_found') { - reply.send(errEnvelope(ErrorCode.PROVIDER_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.PROVIDER_NOT_FOUND, err.message, requestId, err.stack)); return true; } if (err.code === 'model.not_found') { - reply.send(errEnvelope(ErrorCode.MODEL_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.MODEL_NOT_FOUND, err.message, requestId, err.stack)); return true; } return false; diff --git a/packages/kap-server/src/routes/prompts.ts b/packages/kap-server/src/routes/prompts.ts index 1fe0a7b25..462aa7002 100644 --- a/packages/kap-server/src/routes/prompts.ts +++ b/packages/kap-server/src/routes/prompts.ts @@ -221,13 +221,13 @@ function sendMappedError( switch (err.code) { case 'session.not_found': case 'agent.not_found': - reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, requestId, err.stack)); return; case 'prompt.not_found': - reply.send(errEnvelope(ErrorCode.PROMPT_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.PROMPT_NOT_FOUND, err.message, requestId, err.stack)); return; case 'session.busy': - reply.send(errEnvelope(ErrorCode.SESSION_BUSY, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.SESSION_BUSY, err.message, requestId, err.stack)); return; case 'prompt.already_completed': reply.send({ @@ -235,11 +235,12 @@ function sendMappedError( msg: err.message, data: { aborted: false }, request_id: requestId, + stack: err.stack, }); return; case 'request.invalid': case 'validation.failed': - reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, err.message, requestId, err.stack)); return; case 'auth.provisioning_required': reply.send({ @@ -247,6 +248,7 @@ function sendMappedError( msg: err.message, data: null, request_id: requestId, + stack: err.stack, details: null, }); return; @@ -267,6 +269,7 @@ function sendMappedError( msg: err.message, data: null, request_id: requestId, + stack: err.stack, details, }); return; @@ -288,6 +291,7 @@ function sendMappedError( msg: err.message, data: null, request_id: requestId, + stack: err.stack, details, }); return; @@ -298,6 +302,7 @@ function sendMappedError( msg: err.message, data: null, request_id: requestId, + stack: err.stack, details: authModelDetails(err), }); return; @@ -308,6 +313,7 @@ function sendMappedError( ErrorCode.INTERNAL_ERROR, err instanceof Error ? err.message : String(err), requestId, + err instanceof Error ? err.stack : undefined, ), ); } diff --git a/packages/kap-server/src/routes/sessions.ts b/packages/kap-server/src/routes/sessions.ts index c3b327cdc..2a6b8eac5 100644 --- a/packages/kap-server/src/routes/sessions.ts +++ b/packages/kap-server/src/routes/sessions.ts @@ -913,38 +913,44 @@ function sendMappedError( switch (err.code) { case 'session.not_found': case 'agent.not_found': - reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, requestId, err.stack)); return; case 'session.fork_active_turn': - reply.send(errEnvelope(ErrorCode.SESSION_BUSY, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.SESSION_BUSY, err.message, requestId, err.stack)); return; case 'compaction.unable': - reply.send(errEnvelope(ErrorCode.COMPACTION_UNABLE, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.COMPACTION_UNABLE, err.message, requestId, err.stack)); return; case 'session.undo_unavailable': - reply.send(errEnvelope(ErrorCode.SESSION_UNDO_UNAVAILABLE, err.message, requestId)); + reply.send({ + code: ErrorCode.SESSION_UNDO_UNAVAILABLE, + msg: err.message, + data: (err as { details?: unknown }).details ?? null, + request_id: requestId, + stack: err.stack, + }); return; case ErrorCodes.GOAL_ALREADY_EXISTS: - reply.send(errEnvelope(ErrorCode.GOAL_ALREADY_EXISTS, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.GOAL_ALREADY_EXISTS, err.message, requestId, err.stack)); return; case ErrorCodes.GOAL_NOT_FOUND: - reply.send(errEnvelope(ErrorCode.GOAL_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.GOAL_NOT_FOUND, err.message, requestId, err.stack)); return; case ErrorCodes.GOAL_STATUS_INVALID: - reply.send(errEnvelope(ErrorCode.GOAL_STATUS_INVALID, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.GOAL_STATUS_INVALID, err.message, requestId, err.stack)); return; case ErrorCodes.GOAL_NOT_RESUMABLE: - reply.send(errEnvelope(ErrorCode.GOAL_NOT_RESUMABLE, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.GOAL_NOT_RESUMABLE, err.message, requestId, err.stack)); return; case ErrorCodes.GOAL_OBJECTIVE_EMPTY: - reply.send(errEnvelope(ErrorCode.GOAL_OBJECTIVE_EMPTY, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.GOAL_OBJECTIVE_EMPTY, err.message, requestId, err.stack)); return; case ErrorCodes.GOAL_OBJECTIVE_TOO_LONG: - reply.send(errEnvelope(ErrorCode.GOAL_OBJECTIVE_TOO_LONG, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.GOAL_OBJECTIVE_TOO_LONG, err.message, requestId, err.stack)); return; case 'request.invalid': case 'validation.failed': - reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, err.message, requestId, err.stack)); return; } } @@ -953,6 +959,7 @@ function sendMappedError( ErrorCode.INTERNAL_ERROR, err instanceof Error ? err.message : String(err), requestId, + err instanceof Error ? err.stack : undefined, ), ); } diff --git a/packages/kap-server/src/routes/skills.ts b/packages/kap-server/src/routes/skills.ts index c3387d36c..5ec0b1886 100644 --- a/packages/kap-server/src/routes/skills.ts +++ b/packages/kap-server/src/routes/skills.ts @@ -375,10 +375,10 @@ function sendMappedError( switch (err.code) { case ErrorCodes.SKILL_NOT_FOUND: case ErrorCodes.SKILL_NAME_EMPTY: - reply.send(errEnvelope(ErrorCode.SKILL_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.SKILL_NOT_FOUND, err.message, requestId, err.stack)); return; case ErrorCodes.SKILL_TYPE_UNSUPPORTED: - reply.send(errEnvelope(ErrorCode.SKILL_NOT_ACTIVATABLE, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.SKILL_NOT_ACTIVATABLE, err.message, requestId, err.stack)); return; } } diff --git a/packages/kap-server/src/routes/snapshot.ts b/packages/kap-server/src/routes/snapshot.ts index a6cac5803..18cdf615f 100644 --- a/packages/kap-server/src/routes/snapshot.ts +++ b/packages/kap-server/src/routes/snapshot.ts @@ -107,14 +107,14 @@ export function registerSnapshotRoutes(app: SnapshotRouteHost, deps: SnapshotRou reply.send(okEnvelope(data, req.id)); } catch (err) { if (err instanceof SnapshotNotFoundError) { - reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, req.id)); + reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, req.id, err.stack)); return; } if (err instanceof SnapshotTimeoutError) { core.accessor .get(ILogService) .warn('snapshot.timeout', { sid: session_id, duration_ms: err.timeoutMs }); - reply.send(errEnvelope(ErrorCode.INTERNAL_ERROR, err.message, req.id)); + reply.send(errEnvelope(ErrorCode.INTERNAL_ERROR, err.message, req.id, err.stack)); return; } throw err; diff --git a/packages/kap-server/src/routes/terminals.ts b/packages/kap-server/src/routes/terminals.ts index c5e695f98..c442fef6f 100644 --- a/packages/kap-server/src/routes/terminals.ts +++ b/packages/kap-server/src/routes/terminals.ts @@ -232,10 +232,10 @@ function sendMappedError( if (isKimiError(err)) { switch (err.code) { case ErrorCodes.SESSION_NOT_FOUND: - reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.SESSION_NOT_FOUND, err.message, requestId, err.stack)); return; case ErrorCodes.TERMINAL_NOT_FOUND: - reply.send(errEnvelope(ErrorCode.TERMINAL_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.TERMINAL_NOT_FOUND, err.message, requestId, err.stack)); return; } } @@ -244,7 +244,7 @@ function sendMappedError( // escapes. TODO: push a coded error into `assertAllowed` so this branch can // be folded into the `isKimiError` switch above. if (err instanceof Error && err.message.startsWith('Path outside workspace')) { - reply.send(errEnvelope(ErrorCode.FS_PATH_ESCAPES_SESSION, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_PATH_ESCAPES_SESSION, err.message, requestId, err.stack)); return; } throw err; diff --git a/packages/kap-server/src/routes/tools.ts b/packages/kap-server/src/routes/tools.ts index 4372b42c1..584084b29 100644 --- a/packages/kap-server/src/routes/tools.ts +++ b/packages/kap-server/src/routes/tools.ts @@ -321,7 +321,7 @@ function sendMappedError( err: unknown, ): void { if (err instanceof KimiError && err.code === ErrorCodes.MCP_SERVER_NOT_FOUND) { - reply.send(errEnvelope(ErrorCode.MCP_SERVER_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.MCP_SERVER_NOT_FOUND, err.message, requestId, err.stack)); return; } throw err; diff --git a/packages/kap-server/src/routes/workspaceFs.ts b/packages/kap-server/src/routes/workspaceFs.ts index 5d39ee965..53667df4a 100644 --- a/packages/kap-server/src/routes/workspaceFs.ts +++ b/packages/kap-server/src/routes/workspaceFs.ts @@ -113,15 +113,15 @@ function sendMappedError( err: unknown, ): void { if (err instanceof HostFolderNotAbsoluteError) { - reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, err.message, requestId, err.stack)); return; } if (err instanceof HostFolderNotFoundError) { - reply.send(errEnvelope(ErrorCode.FS_PATH_NOT_FOUND, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_PATH_NOT_FOUND, err.message, requestId, err.stack)); return; } if (err instanceof HostFolderPermissionError) { - reply.send(errEnvelope(ErrorCode.FS_PERMISSION_DENIED, err.message, requestId)); + reply.send(errEnvelope(ErrorCode.FS_PERMISSION_DENIED, err.message, requestId, err.stack)); return; } throw err; diff --git a/packages/kap-server/src/transport/errors.ts b/packages/kap-server/src/transport/errors.ts index 9d0331b97..5b6f283dd 100644 --- a/packages/kap-server/src/transport/errors.ts +++ b/packages/kap-server/src/transport/errors.ts @@ -50,15 +50,16 @@ const KIMI_TO_PROTOCOL: Record = { export function mapError(err: unknown, requestId: string): ReturnType { if (err instanceof KimiError) { const code = KIMI_TO_PROTOCOL[err.code] ?? ErrorCode.INTERNAL_ERROR; - return errEnvelope(code, err.message, requestId); + return errEnvelope(code, err.message, requestId, err.stack); } if (err instanceof TimeoutError) { - return errEnvelope(ErrorCode.INTERNAL_ERROR, err.message, requestId); + return errEnvelope(ErrorCode.INTERNAL_ERROR, err.message, requestId, err.stack); } return errEnvelope( ErrorCode.INTERNAL_ERROR, err instanceof Error ? err.message : String(err), requestId, + err instanceof Error ? err.stack : undefined, ); } diff --git a/packages/kap-server/test/rpc.test.ts b/packages/kap-server/test/rpc.test.ts index 72df45015..ba0eb65f7 100644 --- a/packages/kap-server/test/rpc.test.ts +++ b/packages/kap-server/test/rpc.test.ts @@ -499,9 +499,13 @@ describe('server-v2 /api/v2 RPC', () => { expect(code).not.toBe(0); }); - it('does not leak stack traces on error', async () => { + it('surfaces the originating stack trace on error', async () => { const { body } = await call('POST', '/api/v2/session/nope/session:read'); - expect(JSON.stringify(body)).not.toContain('stack'); + // Contract: error envelopes carry the thrown error's stack so operators can + // locate the source (the 40401 below originates in `dispatch`). + const json = JSON.stringify(body); + expect(json).toContain('"stack"'); + expect(json).toContain('dispatch'); }); }); diff --git a/packages/kap-server/test/sessions.test.ts b/packages/kap-server/test/sessions.test.ts index 673909c91..46847d370 100644 --- a/packages/kap-server/test/sessions.test.ts +++ b/packages/kap-server/test/sessions.test.ts @@ -16,6 +16,7 @@ interface Envelope { data: T; request_id: string; details?: { path: string; message: string }[]; + stack?: string; } interface SessionWire { @@ -339,6 +340,8 @@ describe('server-v2 /api/v1/sessions', () => { // (40911), not the pre-fix "session does not exist" (40401). expect(res.body.code).toBe(40911); expect(res.body.msg).toMatch(/nothing to undo/i); + // The thrown KimiError's stack is surfaced so operators can locate the source. + expect(res.body.stack).toEqual(expect.stringContaining('sessionLegacyService')); }); it('rejects an unsupported action suffix (40001)', async () => { diff --git a/packages/protocol/src/__tests__/envelope.test.ts b/packages/protocol/src/__tests__/envelope.test.ts index ba8282097..ba4135092 100644 --- a/packages/protocol/src/__tests__/envelope.test.ts +++ b/packages/protocol/src/__tests__/envelope.test.ts @@ -57,6 +57,20 @@ describe('envelope', () => { '{"code":40001,"msg":"validation failed","data":null,"request_id":"req_z"}', ); }); + + it('errEnvelope surfaces stack when provided and omits it when absent', () => { + const err = new Error('boom'); + const withStack = errEnvelope(ErrorCode.INTERNAL_ERROR, 'boom', 'req_s', err.stack); + expect(withStack.stack).toBe(err.stack); + expect(JSON.stringify(withStack)).toContain('"stack":'); + expect(envelopeSchema(z.any()).parse(withStack).stack).toBe(err.stack); + + // No stack → field is absent and the wire shape is byte-identical to before. + const without = errEnvelope(ErrorCode.INTERNAL_ERROR, 'boom', 'req_s'); + expect(JSON.stringify(without)).toBe( + '{"code":50001,"msg":"boom","data":null,"request_id":"req_s"}', + ); + }); }); describe('error-codes', () => { diff --git a/packages/protocol/src/envelope.ts b/packages/protocol/src/envelope.ts index 892a0d768..581087ddd 100644 --- a/packages/protocol/src/envelope.ts +++ b/packages/protocol/src/envelope.ts @@ -7,6 +7,7 @@ export const envelopeSchema = (data: T) => data: data.nullable(), request_id: z.string(), details: z.unknown().optional(), + stack: z.string().optional(), }); export interface Envelope { @@ -15,12 +16,25 @@ export interface Envelope { data: T | null; request_id: string; details?: unknown; + stack?: string; } export function okEnvelope(data: T, requestId: string): Envelope { return { code: 0, msg: 'success', data, request_id: requestId }; } -export function errEnvelope(code: number, msg: string, requestId: string): Envelope { - return { code, msg, data: null, request_id: requestId }; +/** + * Build an error envelope. When `stack` is provided it is surfaced verbatim on + * the wire so operators can see where a thrown error originated; when omitted + * (or `undefined`) the field is absent and the wire shape stays byte-identical + * to the original `{ code, msg, data: null, request_id }` — `JSON.stringify` + * drops `undefined` properties, so callers that have no stack are unaffected. + */ +export function errEnvelope( + code: number, + msg: string, + requestId: string, + stack?: string, +): Envelope { + return { code, msg, data: null, request_id: requestId, stack }; }