refactor(agent-core-v2): code all domain failure modes as Error2 - #2552
Conversation
- wrap bare throws across agent/session/app/workspace/os/wire/kosong/mcpCore domains in coded Error2, keeping messages verbatim and moving structured data into details with the original error as cause - add new wire codes (agent.already_exists/already_running/not_a_subagent/not_owned/type_not_allowed/max_tokens_exceeded, task.limit_exceeded, cron.expression_invalid, web.invalid_url/private_address/fetch_failed, mcp.oauth_failed, skill.parse_failed/nested_too_deep, wire.migration_missing) to the protocol KimiErrorCode union and the kap-server zod schema; register shell.git_bash_not_found and session.plan_mode_invalid - re-base domain error classes onto Error2 (SkillParseError, UnsupportedSkillTypeError, HostFolder*, AgentFileParseError, NestedSkillTooDeepError, AlreadyAuthorizedError, HttpFetchError) keeping class names and instanceof consumers intact - convert caller-bug and unreachable guards outside _base to BugIndicatingError - fix the agent tool's task-limit remap never firing by branching on the task.limit_exceeded code instead of a stale message string
- move the provider/context code string constants to kosong/contract/errors.ts and compute each class's wire code at construction (status code / finish reason) - move sanitizeStatusErrorMessage to the contract and fold status details (statusCode / requestId / traceId) into Error2 details at birth - slim translateProviderError down to the abort guard plus the foreign-error fallback; ProtocolErrors keeps registering the domain via re-exported constants - update errors.md conventions and tests for the pass-through behavior
- extend StorageErrors with storage.permission_denied / storage.disk_full (both non-retryable, with user-facing actions) - map errno at the backend boundary in toStorageIoError: EACCES/EPERM, ENOSPC, unexpected ENOENT → not_found, everything else io_failed; the message now carries the mapped reason - register the two codes in the KimiErrorCode protocol union and the kap-server zod schema, and document the mapping in errors.md
|
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cf839cea7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| | 'agent.already_exists' | ||
| | 'agent.already_running' | ||
| | 'agent.not_a_subagent' | ||
| | 'agent.not_owned' | ||
| | 'agent.type_not_allowed' |
There was a problem hiding this comment.
Mirror new error codes in protocol schema
When any of the newly added KimiErrorCode values is validated through @moonshot-ai/protocol's kimiErrorPayloadSchema, parsing still fails because the kimiErrorCodeSchema enum later in this same file was not updated (it still jumps from agent.not_found to turn.agent_busy, and similarly omits the new task/MCP/skill/web/storage/wire codes). This means protocol consumers can type these codes but reject them at runtime; please add the new codes to the schema as well.
Useful? React with 👍 / 👎.
the zod enum lagged the type union by 35 codes (agent.*, os.fs.*, os.process.*, storage.*, wire.*, skill/task/mcp/cron/web additions), so protocol consumers could type the new codes but rejected them at runtime validation; spotted by Codex review on MoonshotAI#2552
…nshotAI#2552) * refactor(agent-core-v2): code all domain failure modes as Error2 - wrap bare throws across agent/session/app/workspace/os/wire/kosong/mcpCore domains in coded Error2, keeping messages verbatim and moving structured data into details with the original error as cause - add new wire codes (agent.already_exists/already_running/not_a_subagent/not_owned/type_not_allowed/max_tokens_exceeded, task.limit_exceeded, cron.expression_invalid, web.invalid_url/private_address/fetch_failed, mcp.oauth_failed, skill.parse_failed/nested_too_deep, wire.migration_missing) to the protocol KimiErrorCode union and the kap-server zod schema; register shell.git_bash_not_found and session.plan_mode_invalid - re-base domain error classes onto Error2 (SkillParseError, UnsupportedSkillTypeError, HostFolder*, AgentFileParseError, NestedSkillTooDeepError, AlreadyAuthorizedError, HttpFetchError) keeping class names and instanceof consumers intact - convert caller-bug and unreachable guards outside _base to BugIndicatingError - fix the agent tool's task-limit remap never firing by branching on the task.limit_exceeded code instead of a stale message string * refactor(kosong): make ChatProviderError family born-coded via Error2 - move the provider/context code string constants to kosong/contract/errors.ts and compute each class's wire code at construction (status code / finish reason) - move sanitizeStatusErrorMessage to the contract and fold status details (statusCode / requestId / traceId) into Error2 details at birth - slim translateProviderError down to the abort guard plus the foreign-error fallback; ProtocolErrors keeps registering the domain via re-exported constants - update errors.md conventions and tests for the pass-through behavior * feat(storage): add permission_denied and disk_full error codes - extend StorageErrors with storage.permission_denied / storage.disk_full (both non-retryable, with user-facing actions) - map errno at the backend boundary in toStorageIoError: EACCES/EPERM, ENOSPC, unexpected ENOENT → not_found, everything else io_failed; the message now carries the mapped reason - register the two codes in the KimiErrorCode protocol union and the kap-server zod schema, and document the mapping in errors.md * fix(protocol): mirror all KimiErrorCode values in kimiErrorCodeSchema the zod enum lagged the type union by 35 codes (agent.*, os.fs.*, os.process.*, storage.*, wire.*, skill/task/mcp/cron/web additions), so protocol consumers could type the new codes but rejected them at runtime validation; spotted by Codex review on MoonshotAI#2552
Related Issue
No linked issue — this is an internal error-handling refactor; the problem is explained below.
Problem
In agent-core-v2, many domains still threw bare
Errors or plain strings for their failure modes, so nothing across the wire could branch on a stable error code: SDK clients and the server surface had to pattern-match message text, and one remap (the agent tool's task-limit branch) had already silently broken because it compared against a stale message string. TheChatProviderErrorfamily also relied on boundary translation (translateProviderError) to attach codes after the fact, and storage I/O failures collapsed every errno into a singlestorage.io_failed.What changed
1. Code all domain failure modes as Error2
Error2, keeping messages verbatim and moving structured data intodetailswith the original error ascause.agent.already_exists/already_running/not_a_subagent/not_owned/type_not_allowed/max_tokens_exceeded,task.limit_exceeded,cron.expression_invalid,web.invalid_url/private_address/fetch_failed,mcp.oauth_failed,skill.parse_failed/nested_too_deep,wire.migration_missing) to the protocolKimiErrorCodeunion and the kap-server zod schema; registeredshell.git_bash_not_foundandsession.plan_mode_invalid.Error2(SkillParseError,UnsupportedSkillTypeError,HostFolder*,AgentFileParseError,NestedSkillTooDeepError,AlreadyAuthorizedError,HttpFetchError), keeping class names andinstanceofconsumers intact._basetoBugIndicatingError.task.limit_exceededcode instead of a stale message string.2. Make the ChatProviderError family born-coded
provider.*/context.overflowcode string constants intokosong/contract/errors.ts; every class in the family now extendsError2and computes its wire code at construction from the status code / finish reason.translateProviderErrordown to the abort guard plus the foreign-error fallback — family instances pass through untouched.3. Add storage permission/disk-full codes
StorageErrorswithstorage.permission_deniedandstorage.disk_full(both non-retryable, with user-facing recovery actions).toStorageIoErrornow maps errno at the backend boundary (EACCES/EPERM→ permission_denied,ENOSPC→ disk_full, unexpectedENOENT→ not_found, everything elseio_failed) instead of collapsing all I/O failures into one code.This approach fits the existing error taxonomy documented in
packages/agent-core-v2/docs/errors.md: every domain codes all of its failure modes, codes are defined in the owning domain, and the wire protocol owns the fixed set of valid code strings.Checklist
gen-changesetsskill, or this PR needs no changeset. (internal error-taxonomy refactor on private packages — no user-facing change)gen-docsskill, or this PR needs no doc update. (internal refactor; in-repo conventions doc updated)