Repository navigation
refactor: reuse shared definitions with frozen client behavior - #321
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Repeated lifecycle comparisons, MCP projections, effort lists, and trace types could drift independently. This reuses existing definitions where their semantics match and adds regression checks where separate representations must remain.
Follows #320. The dependency/security repair in #329 was incorporated for fresh validation; the incremental 16-file DRY patch is unchanged from the original reviewed head.
Changes
Behavior constraints
SDK and protocol sources, generated contracts, migrations, and the original compatibility baseline are unchanged. No response, event, ordering, acceptance, cancellation, reconnect, schema, or provider behavior is intentionally changed. No translation layer is added.
Turn, command, and agent state machines remain separate. Existing TUI versus desktop/web effort fallback differences remain. Model conversion implementations remain unchanged; their distinct projections are protected by tests instead of being forced into one struct.
Validation
e1e5e36cee54575d52730902176117bf0e2bba91passed all 22 checks in fresh CI, including security, compatibility, SDK/browser, mobile, desktop lifecycle/SSH, and distribution gates. Go coverage is 90.1% against the unchanged 90% floor. The finaldevelopmentmerge commit58860dc7715a4122c713a205cca0a2194b648a1ehas exactly the tested full-stack tree; frozen SDK/protocol/database paths remain unchanged.1bb0b4661fa4f7a0236dfdb9f377656c10f4e05dagainst original baseline271c0f8d2a35648d1b45056d57432590b783483cand refactor(backend): separate responsibilities with a frozen client contract #320: byte-identical generated/SDK artifacts, calibrated Unix/WebSocket responses and ordered events, mutation negative controls, lifecycle checks, and both-engine retained-data/schema/trace rollback.task acceptancepassed on the final head, including all 49 SDK acceptance cases with zero failures or skips, native/recursive-agent behavior, reconnect/cancellation, persistence, and model accounting.Note
Medium Risk
Dependency pins from unmerged upstream archives and Go 1.27.2 shift coverage weighting; lifecycle and client cancellation paths are refactored but heavily regression-tested.
Overview
Bumps Go to 1.27.2 and refreshes
golang.org/x/*, golangci-lint v2.14.0, and docs TanStack pins. CI’s SDK job now runsnpm run audit:productioninstead of rawnpm audit.Adds npm overrides for braces and node-forge from reviewed upstream tarballs, plus
scripts/audit-production.mjs(pin/hash verification and a single allowed GHSA disposition for fixed-but-still-3.0.3 braces).docs/dependency-security.mddocuments the process.Centralizes lifecycle semantics with
IsTerminalCommandStatusandIsTerminalTurnStatus, wired through session store, daemon admission, and root client (command tracking, turn cleanup, cancel targeting). MCPmcp.statususesmcp.ServerStatusesinstead of hand-built JSON.Reuses SDK trace types in the app (
trace-math.ts) and derives TUIdefaultEffortsfrom completion metadata; a fewCutLastcall sites satisfy new linter modernization hints.Adds regression tests only for status boundaries, root-client cancellation/provider validation, MCP/config/model contract parity, and LLM encode/decode/presentation bounds—documented as behavior-preserving with no SDK/protocol/migration changes.
Reviewed by Cursor Bugbot for commit e1e5e36. Bugbot is set up for automated code reviews on this repo. Configure here.