Skip to content

Conversation

@AshishKumar4
Copy link
Contributor

@AshishKumar4 AshishKumar4 commented Dec 27, 2025

Summary

Refactor SDK types to re-export original worker types instead of maintaining duplicate type definitions, improving type consistency between SDK and platform.

Changes

  • sdk/src/protocol.ts: Added new type re-exports from worker modules:

    • WebSocketMessage, WebSocketMessageData, CodeFixEdits, ModelConfigsInfoMessage, AgentDisplayConfig, ModelConfigsInfo from websocket types
    • AgentState from agent core state
    • BehaviorType, ProjectType from agent core types
    • FileOutputType from agent schemas
    • TemplateDetails from sandbox types
    • AgentConnectionData, PlatformCodeGenArgs, AgentPreviewResponse from agent controller types
  • sdk/src/types.ts: Updated to import types from protocol.ts instead of defining them locally

    • BehaviorType and ProjectType now alias platform types
    • CodeGenArgs uses PlatformCodeGenArgs from protocol
  • sdk/src/index.ts: Added new exports for AgentState, AgentConnectionData, AgentPreviewResponse, WebSocketMessage, WebSocketMessageData

  • sdk/package.json: Updated dependencies:

    • Added @cloudflare/workers-types, miniflare, puppeteer to devDependencies
    • Removed @types/ws as explicit devDependency (types bundled from ws)
    • ws remains as direct dependency

Motivation

Eliminates type duplication between SDK and platform code. When platform types change, SDK types automatically stay in sync rather than requiring manual updates. This reduces maintenance burden and prevents type drift.

Testing

  • Existing SDK tests should pass
  • Type checking with bun run typecheck in sdk/
  • Type bundling with bun run bundle-types in sdk/

Generated with Claude Code

@AshishKumar4 AshishKumar4 merged commit 6745f04 into nightly Dec 27, 2025
4 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Dec 27, 2025

Claude finished @AshishKumar4's task in 2m 22s —— View job


Code Quality & Security Review

Recommendation: COMMENT

PR Summary

Refactors SDK to re-export original worker types instead of maintaining duplicate type definitions, improving type consistency and reducing maintenance burden.

PR Description

  • Updated: Yes (was empty, now includes comprehensive summary)
  • Notes: Added summary, changes list, motivation, and testing sections

Addressed Issues

  • None identified (refactoring task)

Code Quality

Critical: None

High: None

Medium:

  • ⚠️ Unused dependency added - puppeteer was added as a devDependency but there's no usage of it in the SDK codebase. This is a heavy dependency (~150+ transitive packages based on lock file changes). Consider removing if not needed, or document the intended use case.

Low:

  • The lock file changes (+157/-8 lines) are significant due to puppeteer's transitive dependencies. If puppeteer is truly needed, this is acceptable.

Security

Critical: None

High: None

Medium: None

Low: None

Positive Observations

  • Good DRY principle - Eliminates type duplication by re-exporting from worker modules
  • Backward compatible - Type aliases (BehaviorType, ProjectType) maintain existing API
  • Proper TypeScript config - tsconfig.protocol.json correctly configured with path mappings for type bundling
  • Clean separation - protocol.ts handles platform type re-exports, types.ts handles SDK-specific types
  • New exports useful - AgentState, WebSocketMessage, etc. expose useful platform types to SDK consumers

Testing

  • Verify bun run typecheck passes in sdk/
  • Verify bun run bundle-types generates correct .d.ts files
  • Existing SDK tests should continue to pass

Final Recommendation

COMMENT - The type centralization pattern is well-implemented. The only concern is the unused puppeteer devDependency - recommend either removing it or documenting why it's needed. Since the PR is already merged, this can be addressed in a follow-up PR if needed.


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant