Skip to content

refactor(5/12): change handler contract to event-based emission#323

Open
cameroncooke wants to merge 1 commit intorefactor/build-test-utility-extractionfrom
refactor/runtime-handler-contract
Open

refactor(5/12): change handler contract to event-based emission#323
cameroncooke wants to merge 1 commit intorefactor/build-test-utility-extractionfrom
refactor/runtime-handler-contract

Conversation

@cameroncooke
Copy link
Copy Markdown
Collaborator

Summary

This is PR 5 of 12 in a stacked PR series that decouples the rendering pipeline from MCP transport. Depends on PR 4 (utility extraction). This is the most architecturally significant PR in the stack -- it changes the fundamental contract between tool handlers and the runtime.

The contract change

Before: Tool handlers returned a ToolResponse object containing pre-rendered MCP content:
```typescript
handler: (params) => Promise
```

After: Tool handlers receive a context object and emit events through it, returning nothing:
```typescript
handler: (params, ctx: ToolHandlerContext) => Promise
```

The ToolHandlerContext provides:

  • emit(event: PipelineEvent): Push a structured event into the render session
  • attach(image: ImageAttachment): Attach binary content (screenshots)

This decouples tools from rendering entirely. A tool says "the build succeeded" via ctx.emit(statusLine('success', 'Build succeeded')) without knowing whether the output will be rendered as colored terminal text, JSON, or MCP protocol content.

Tool invoker refactor

The invoker (tool-invoker.ts) is significantly simplified:

  • Deleted postProcessToolResponse, emitNextStepsEvent, renderNextStepsIntoContent, finalizeResult
  • The invoker creates a ToolHandlerContext, passes it to the handler, then emits next-steps into the same render session after the handler completes
  • No more PendingBuildResult pattern -- the handler just awaits the pipeline and the render session accumulates events progressively

Typed tool factory

typed-tool-factory.ts updated to produce handlers matching the new signature. Session-aware wrappers now thread the context through.

Core manifest and schema

  • import-resource-module.ts added for resource manifest loading (split from tool module loading)
  • Schema and load-manifest updated for the simplified handler contract
  • plugin-types.ts updated to reflect new handler signatures

Deleted

  • typed-tool-factory-consolidation.test.ts (tested removed consolidation logic)
  • load-manifest.test.ts (moved to schema test coverage)

Stack navigation

  • PR 1-4/12: Foundation and utility extraction
  • PR 5/12 (this PR): Runtime handler contract and tool invoker
  • PR 6-9/12: Tool migrations (mechanical -- update handlers to use ctx.emit)
  • PR 10-12/12: Boundaries, config, tests

Note for reviewers

PRs 6-9 (tool migrations) depend on this contract change. Each tool handler is updated from return toolResponse([...]) to ctx.emit(...) calls. Those are mechanical transformations but they cannot compile without this PR landing first. If reviewing the stack incrementally, this PR is the critical design decision point.

Test plan

  • npx vitest run passes -- invoker tests updated for new contract
  • Handler context correctly propagates emit/attach to render session
  • Next-steps are emitted into the render session after handler completion
  • Typed tool factory produces handlers with correct signatures

@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 089f6a3 to fa1ece2 Compare April 8, 2026 21:29
@cameroncooke cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 344aeba to f7616d2 Compare April 8, 2026 21:29
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit fa1ece2. Configure here.

throw error;
}
};
return impl as ToolHandler;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated context setup logic across handler factories

Low Severity

createValidatedHandler and createSessionAwareHandler duplicate the entire ToolHandlerContext bootstrapping sequence: isToolHandlerContext check, internal session creation, ctx/context resolution, handlerContextStorage.run, ZodError handling, and sessionToTestResult finalization. If the context propagation or session lifecycle logic changes, both paths need identical updates, creating a maintenance risk.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit fa1ece2. Configure here.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant