refactor: code quality cleanup — DRY schemas, consolidate dispatch, remove dead code#46
Merged
Merged
Conversation
…emove dead code - Add createResponseSchema() factory to eliminate ~26 repeated z.union() blocks - Extract shared dispatchNotification() from duplicated protocol/client logic - Remove orphaned aggregateMessages() and isPartialMessage() (knip-verified) - Fix misleading @deprecated on LEGACY_FACTORY_API_VERSION (it's required by wire protocol) - Remove ~70 name-restating JSDoc comments that add no information beyond type names - Remove stale internal monorepo path references from enums.ts header - Add 11 tests for createResponseSchema and dispatchNotification Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
factory-davidgu
approved these changes
May 22, 2026
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
2 participants
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.
Summary
Targeted code quality improvements identified through systematic analysis (knip, madge, manual audit). Net result: -70 lines, 11 new tests, zero behavior changes.
Changes
DRY: Response schema factory (
src/schemas/shared.ts,client.ts,server.ts)createResponseSchema(resultSchema)helperz.union([JsonRpcResponseSuccessSchema.extend({result: X}), JsonRpcResponseFailureSchema])blocksConsolidate notification dispatch (
protocol.ts,client.ts)dispatchNotification()from duplicated logic inProtocolEngineandDroidClientRemove dead code (
session.ts,stream.ts)aggregateMessages()— orphaned after turn API simplification, zero references (knip-verified)isPartialMessage()— orphaned after streaming result API refactor, zero references (knip-verified)Fix misleading annotation (
constants.ts)LEGACY_FACTORY_API_VERSIONwas marked@deprecatedbut is required in every JSON-RPC envelopeRemove noise comments (
enums.ts,client.ts,server.ts,mcp.ts)/** Tool result notification. */onToolResultNotificationSchema)enums.tsheaderNew tests (
tests/code-quality.test.ts)createResponseSchema: valid success/failure parsing, rejection of wrong shapes, equivalence with hand-written schemasdispatchNotification: filter matching, error swallowing (listener throws without crashing), unparseable notification handlingValidation
npm test— 646 tests pass (including 11 new)npm run typecheck— cleannpm run lint— cleannpm run format:check— clean