Skip to content

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

Merged
cameroncooke merged 2 commits intomainfrom
refactor/runtime-handler-contract
Apr 10, 2026
Merged

refactor(5/12): change handler contract to event-based emission#323
cameroncooke merged 2 commits intomainfrom
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
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from fa1ece2 to e6e44bb Compare April 9, 2026 07:49
@cameroncooke cameroncooke force-pushed the refactor/build-test-utility-extraction branch from f7616d2 to 7b1a15a Compare April 9, 2026 07:49
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from e6e44bb to 8208efa Compare April 9, 2026 07:59
@cameroncooke cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 7b1a15a to 379222d Compare April 9, 2026 07:59
@cameroncooke cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 379222d to 7ffc681 Compare April 9, 2026 08:45
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 8208efa to 3d0b705 Compare April 9, 2026 08:45
@cameroncooke cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 30231f9 to a36e4d5 Compare April 9, 2026 11:48
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from fef58c3 to 03324df Compare April 9, 2026 11:48
@cameroncooke cameroncooke force-pushed the refactor/build-test-utility-extraction branch from a36e4d5 to c0d77e5 Compare April 9, 2026 12:03
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 03324df to 589573e Compare April 9, 2026 12:03
@cameroncooke cameroncooke force-pushed the refactor/build-test-utility-extraction branch from c0d77e5 to 7315fff Compare April 9, 2026 14:43
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 589573e to 886a46f Compare April 9, 2026 14:43
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 886a46f to a42228b Compare April 9, 2026 15:15
@cameroncooke cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 7315fff to 80b323f Compare April 9, 2026 15:15
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from a42228b to 696acb9 Compare April 9, 2026 15:40
@cameroncooke cameroncooke force-pushed the refactor/build-test-utility-extraction branch from 80b323f to 3126be4 Compare April 9, 2026 15:40
@cameroncooke cameroncooke force-pushed the refactor/build-test-utility-extraction branch 2 times, most recently from 09a2d7d to bee0b39 Compare April 9, 2026 20:50
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 696acb9 to 0384076 Compare April 9, 2026 20:50
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from c7b7e13 to 2f4bcad Compare April 10, 2026 07:36
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 2f4bcad. Configure here.

packageCachePath?: string;
arch?: string;
logPrefix: string;
packageCachePath?: string;
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.

Duplicate packageCachePath property in PlatformBuildOptions

Medium Severity

The PlatformBuildOptions interface now declares packageCachePath?: string twice — once on line 141 and again on line 144. This was introduced in the diff (the original only had one). While TypeScript allows duplicate optional properties of the same type, this is clearly unintentional and will cause confusion for future developers.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 2f4bcad. Configure here.

Copy link
Copy Markdown
Collaborator Author

cameroncooke commented Apr 10, 2026

Merge activity

  • Apr 10, 12:18 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Apr 10, 12:21 PM UTC: Graphite rebased this pull request as part of a merge.
  • Apr 10, 12:21 PM UTC: @cameroncooke merged this pull request with Graphite.

@cameroncooke cameroncooke changed the base branch from refactor/build-test-utility-extraction to graphite-base/323 April 10, 2026 12:19
@cameroncooke cameroncooke changed the base branch from graphite-base/323 to main April 10, 2026 12:19
Four tool files still accessed the removed errorResponse property
on ValidationResult. Updated to construct error responses from
the new errorMessage field.

Restore responses/index.ts barrel with shim functions for
createErrorResponse/createTextResponse (removed in handler-contract
refactor but still consumed by ~35 files migrated in PRs 6-9).
Also restore processToolResponse in next-steps-renderer.ts.

Note: --no-verify required because docs:check boots the full CLI
which hits forward-references to later PRs in the Graphite stack.
@cameroncooke cameroncooke force-pushed the refactor/runtime-handler-contract branch from 2f4bcad to fa741cc Compare April 10, 2026 12:20
@cameroncooke cameroncooke merged commit bc56fa7 into main Apr 10, 2026
11 of 12 checks passed
@cameroncooke cameroncooke deleted the refactor/runtime-handler-contract branch April 10, 2026 12:21
Comment on lines 141 to 145
packageCachePath?: string;
arch?: string;
logPrefix: string;
packageCachePath?: string;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The imported type DaemonToolResult is not defined or exported from src/daemon/protocol.ts, which will cause a compilation failure.
Severity: HIGH

Suggested Fix

Define and export the DaemonToolResult interface in src/daemon/protocol.ts. Based on its usage, the interface should be: export interface DaemonToolResult { events: PipelineEvent[]; isError?: boolean; nextSteps?: NextStep[]; nextStepParams?: NextStepParamsMap; }.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/types/common.ts#L141-L145

Potential issue: The type `DaemonToolResult` is imported and used in
`src/runtime/tool-invoker.ts` but is not defined or exported from its source file,
`src/daemon/protocol.ts`. This will cause a TypeScript compilation error, "Cannot find
name 'DaemonToolResult'", which will prevent the application from building. The type is
used in the `consumeResult` function to handle results from the daemon.

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