refactor(protocol): remove package and relocate wire types into agent-core-v2 - #3646
Conversation
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70cf49c534
ℹ️ 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".
| type: 'tool_result'; | ||
| tool_call_id: string; | ||
| output: unknown; | ||
| is_error?: boolean | undefined; |
There was a problem hiding this comment.
undefined from optional properties
Declare this as is_error?: boolean rather than is_error?: boolean | undefined. The same prohibited pattern is repeated across the newly added MessageContent, config, and session protocol interfaces, so update all of those optional properties consistently.
AGENTS.md reference: AGENTS.md:L55-L57
Useful? React with 👍 / 👎.
70cf49c to
107b464
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 107b4643a0
ℹ️ 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".
| export interface TurnStartedEvent extends Omit<TurnStartedPayload, 'agentId'> { | ||
| readonly type: 'turn.started'; | ||
| } |
There was a problem hiding this comment.
When SDK consumers handle a replayed or legacy turn.started event whose origin is { kind: 'background_task', ... }, this type now inherits agent-core-v2's PromptOrigin, which omits that variant even though the SDK still publicly exports BackgroundTaskOrigin and includes it in its own PromptOrigin. Updating the SDK therefore makes existing exhaustive origin handlers fail to compile or incorrectly treat a valid legacy event as impossible; keep the wire-specific origin union rather than deriving this event directly from the engine payload.
AGENTS.md reference: AGENTS.md:L22-L22
Useful? React with 👍 / 👎.
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { | ||
| APIConnectionError, | ||
| APIProviderRateLimitError, | ||
| APIStatusError, | ||
| } from '#/llm-adapter/contract/errors'; | ||
| import { emptyUsage } from '#human/llm/usage'; | ||
| import { IEventBus } from '#/app/event/eventBus'; | ||
| import { retryBackoffDelays } from '#/_base/utils/retry'; |
There was a problem hiding this comment.
This protocol relocation deletes all loop-level retry tests from this file—including finite retry limits, retryability classification, cancellation during backoff, persisted wire events, and per-turn budget reset—while leaving only backoff arithmetic, and no equivalent cases were relocated elsewhere under packages/agent-core-v2/test. That unrelated deletion allows regressions in the still-active retry path in loopService.ts to pass CI, so retain or relocate these tests as part of this focused refactor.
AGENTS.md reference: AGENTS.md:L12-L12
Useful? React with 👍 / 👎.
Sync 147 upstream commits onto the fork. Notable upstream changes: - Remove the protocol package and relocate wire types into agent-core-v2/src/events.ts (MoonshotAI#3646) - Absorb stepRetry into the LLM requester turn state machine (already followed in the 2026-09-07 sync) - NotifyUser tool + mid-turn update panel (MoonshotAI#3524) - HEIC/HEIF/BMP image support (MoonshotAI#3649), resumable large file reads (MoonshotAI#3645), reasoning_details round-trip (MoonshotAI#3492), zstd updater manifests (MoonshotAI#3669) Conflict resolutions (all by hand, fork features preserved): - package.json: drop the empty simple-git-hooks block (upstream cleanup; not a fork feature). - docs/config-manifest.toml: take the union, then regenerate with pnpm gen:config-manifest (33 sections, fork sections intact). - test/agent/stepRetry/stepRetry.test.ts: keep upstream's slimmed retryBackoffDelays test — the fork had deleted the old fat version when stepRetry was absorbed; the util is fork-used code with no other coverage. - protocol/src/events.ts: accept upstream's package removal; the fork's compaction.started model/model_display fields already auto-merged into their new home (agent/fullCompaction/compactionOps.ts). No source still imports @moonshot-ai/protocol. Merge follow-ups: - Regenerate state/wire/config manifests; freshness tests pass. - node-sdk update-all-session-models flag test: drop the v1-engine parity half (legacy agent-core v1 is removed upstream); keep the v2 registration pin. - Fix pre-existing lint errors in merge-touched files so the lint-staged pre-commit gate passes: unused imports/variables (sdk-rpc-client-v2.ts, tui/commands/config.ts, check-import-boundaries.mjs), floating promises (sdk-rpc-client-v2.ts, fullCompaction.test.ts). - Add compactionOps.ts (model_display) to .github/FORK_OWNED_FILES so the relocated compaction-model fields are guard-listed. Verification: - pnpm run typecheck green; pnpm run lint at fork baseline (no new findings vs pre-merge HEAD). - FORK_OWNED_FILES markers all resolve. - Fork suites pass: TUI commands + session CLI (75 tests), x-opencode-session (7), update-all-session-models flag, oauth openai-compatible. - fullCompaction.test.ts: 2 failures, both pre-existing at pre-merge HEAD (baseline had 5; upstream's macOS stabilization fixed 3. EOF )
1 participant
Related Issue
Internal refactor; no linked issue.
Problem
packages/protocol(@moonshot-ai/protocol) was the legacy shared REST + WS wire-schema package. After kap-server became the serving daemon, it kept its own diverged fork of the protocol underkap-server/src/protocol/, leaving the standalone package as a stale, mostly dead copy: its zod schemas had no runtime consumers, and several of its types (ToolInputDisplay,CompactionResult,UsageStatus,TurnEndReason, …) already existed in agent-core-v2, with the engine importing the protocol copy instead of its own. Four packages still depended on it, almost entirely type-only.What changed
TaskInfo,KimiErrorCode, wireSession/Workspace/MessageContent/ConfigResponse, …) moved frompackages/protocol/src/events.tsinto their respective domain files (agent/loop/turnEvents.ts,agent/toolExecutor/toolExecutorEvents.ts,agent/fullCompaction/compactionOps.ts,agent/task/types.ts,app/sessionLegacy/sessionProtocol.ts,app/config/configEvents.ts, newapp/workspace/workspaceProtocol.ts, newagent/prompt/messageContent.ts,errors.ts, …). Types the engine already had are reused, not duplicated. A newsrc/events.tsholds theAgentEvent/Eventunions (same 54 members as before).MCP_OAUTH_AUTHORIZATION_URL_TOOL_UPDATEmoved totool/toolContract.ts. The engine's own two imports were flipped to its localtool/toolInputDisplay.ts../protocol/*subpath export) and engine wire schemas (os/interface/terminal,llm-adapter/model/catalog,app/authLegacy,app/file,app/hostFolderBrowser,app/sessionLegacy)../protocol/*; the terminal create-request compat schema (runtime_idoptional) moved from an inline route definition intoprotocol/rest-terminal.ts(wire unchanged).packages/protocoldeleted, including its now-dead zod schemas andToolResultDisplay/VOLATILE_EVENT_TYPES(kap-server already has live copies).flake.nixand changeset docs updated.Verified: per-package typechecks, full-repo
pnpm typecheck, node-sdk build + api-extractor (public dts unchanged), acp-server 149/149 tests, klient e2e (live suites skip withoutKIMI_SERVER_URL), full vitest suite on Linux CI-equivalent host.Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.