fix: preserve real errors, add MCP call timeout, remove dead scaffolding - #19
Merged
Conversation
…_workflow code Addresses four harness defects found in QA/cleanup review. #15 — Pipeline no longer collapses every failure to the literal "Execution failed". The caught error's real message (transport error, Zod schema mismatch, timeout) is now carried onto the WorkflowRunResult. A failed query_trace is distinguished from "trace not requested" via a new RunnerReport.traceError field and a stderr log, instead of silently nulling. #16 — Added a configurable RunnerConfig.timeoutMs enforced per MCP call via AbortController + Promise.race (ToolCallTimeoutError). A hung server now yields an error result for the offending workflow and the run proceeds instead of hanging forever. The signal is forwarded to the caller for best-effort cancellation; omitting timeoutMs preserves the prior unbounded behavior. #17 — Removed the live-integration mode that never worked: run-live.ts imported a non-existent ./live-bridge.js (createMcpCaller existed only in that failing import), createLiveCaller was a dead export, and the `live` script always exited 1. Verified via grep that nothing else references run-live.ts, live-caller.ts, createLiveCaller, isLiveMode, createMcpCaller, or live-bridge. Removed both files, the `live` script, the now-unused tsx devDependency, and the README section. #18 — Removed the run_workflow contract scaffolding (RunWorkflow* schemas /types in types.ts, MOCK_RUN_WORKFLOW_* fixtures, the ~17 schema tests, and the index.ts re-exports). Verified run_workflow / RunWorkflow* are never referenced by runner-pipeline.ts — the pipeline only chains list_workflows, run_graph_workflow, query_trace, matching the README and package description. Tests: removed ~17 dead run_workflow schema tests; added 8 tests covering error-message preservation, Zod-mismatch surfacing, trace-error vs not-requested, AbortSignal forwarding, and timeout enforcement. Fixes #15 Fixes #16 Fixes #17 Fixes #18 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Fixes four harness defects from the QA/cleanup review. Full CI green locally (
pnpm install --frozen-lockfile,pnpm typecheck,pnpm test,pnpm build).#15 — Pipeline swallowed real errors (fixed)
runWorkflowPipelinecollapsed every failed graph execution to the literal"Execution failed", discarding the actual cause. Now the caught error's real message is preserved on theWorkflowRunResult(transport errors, Zod schema mismatches, and timeouts each surface distinctly). A failedquery_traceis now distinguishable from "trace never requested": a newRunnerReport.traceErrorfield carries the message and the error is logged to stderr instead of being silently nulled.#16 — No timeout/abort on MCP calls (fixed)
Added
RunnerConfig.timeoutMs, enforced per MCP call viaAbortController+Promise.race(newToolCallTimeoutError). A hung server now produces an error result for the offending workflow and the run proceeds instead of hanging forever. TheAbortSignalis forwarded to the caller for best-effort cancellation. OmittingtimeoutMs(or<= 0) preserves the prior unbounded behavior, so existing callers are unaffected.#17 — Broken live mode (removed)
Live integration mode never worked:
run-live.tsdynamically imported a non-existent./live-bridge.js(createMcpCallerappeared only inside that failing import), sopnpm livealways exited 1.createLiveCaller/isLiveModeinlive-caller.tswere dead. Verified via grep that nothing else references any of these. Removedrun-live.ts,live-caller.ts, thelivepackage script, the now-unusedtsxdevDependency, and the README "Live integration mode" section.#18 — Dead run_workflow scaffolding (removed)
The
run_workflowcontract (schemas/types intypes.ts,MOCK_RUN_WORKFLOW_*fixtures, ~17 schema tests, and theindex.tsre-exports) was never called by the pipeline, which only chainslist_workflows,run_graph_workflow,query_trace(matching the README and package description). Verified no caller exists. Removed all of it.Tests
run_workflowschema tests.AbortSignalforwarding, no-signal when timeout omitted, and timeout enforcement (hung call aborts and reports).Verification
pnpm install --frozen-lockfilepnpm typecheckpnpm testpnpm build🤖 Generated with Claude Code