Skip to content

wait for workflow to complete#13

Merged
j4ys0n merged 1 commit into
mainfrom
tools-fix
Apr 6, 2026
Merged

wait for workflow to complete#13
j4ys0n merged 1 commit into
mainfrom
tools-fix

Conversation

@j4ys0n
Copy link
Copy Markdown
Contributor

@j4ys0n j4ys0n commented Apr 5, 2026

No description provided.

@j4ys0n
Copy link
Copy Markdown
Contributor Author

j4ys0n commented Apr 5, 2026

Automated review 🤖

Summary of Changes
This PR modifies msq_get_workflow_run_status to block until a workflow run reaches a terminal state by consuming the MissionSquad Server-Sent Events (SSE) stream when the initial status is non-terminal. It adds a new consumeServerSentEvents method to MissionSquadClient, a parseServerSentEvent parser, and updates the tool implementation to poll the workflow run after streaming completes. Version is bumped to 0.4.0, HTTP timeout increased to 120s to accommodate long-running workflows, and tests updated to cover blocking behavior and edge cases where the stream ends prematurely.

Key Changes & Positives

  • Added robust SSE parsing and streaming consumer in src/msq-client.ts with proper buffer handling and event boundary detection 🟢
  • msq_get_workflow_run_status now guarantees terminal state before returning, eliminating race conditions for callers expecting final status 🟢
  • Updated README to clarify blocking semantics for msq_get_workflow_run_status and msq_get_workflow_result for improved user expectations 🟢

Potential Issues & Recommendations

  1. Issue / Risk: The waitForWorkflowRunRecord function in src/tools.ts calls fetchWorkflowRunRecord after the SSE stream completes, but does not re-validate the status against the last-seen SSE event data (e.g., [DONE] alone doesn’t confirm terminal state).
    Impact: A race could occur where the stream ends with [DONE] but the final GET still returns non-terminal status due to eventual consistency, leading to a misleading error or retry loop.
    Recommendation: Store and use the last parsed record from the SSE handler (via onEvent) instead of fetching again, or assert that the final GET status matches the last SSE event.
    Status: 🟡 Needs review

  2. Issue / Risk: The consumeServerSentEvents implementation in src/msq-client.ts uses \r?\n\r?\n to split events, but the SSE spec allows single \n separators; mixed line endings could cause missed events.
    Impact: Inconsistent parsing across clients or network intermediaries may drop events.
    Recommendation: Normalize line endings first (e.g., replace \r\n\n) before splitting on \n\n.
    Status: 🟡 Needs review

Language/Framework Checks

  • TypeScript types (ServerSentEvent, EventStreamRequestOptions) added cleanly in src/msq-client.ts with no runtime side effects 🟢
  • Zod schema usage preserved (WorkflowStatusSchema) in src/tools.ts for status checks, maintaining consistency with existing validation patterns 🟢
  • Error messages in waitForWorkflowRunRecord (src/tools.ts) include actionable guidance ("Use msq_get_workflow_run_status again") for retry logic 🟢

Security & Privacy

  • No new secrets or credentials introduced; SSE requests reuse existing x-api-key header pattern from MissionSquadClient 🟢
  • SSE stream consumption avoids buffering sensitive payloads beyond necessary event chunks; no unbounded memory growth observed 🟢

Build/CI & Ops

  • Increased DEFAULT_HTTP_TIMEOUT_MS from 30s to 120s in src/config.ts aligns with longer-running workflow waits but may impact CI test timeouts—verify test suites accommodate this 🟡
  • No changes to build tooling or deployment manifests; version bump to 0.4.0 in package.json indicates minor release with behavioral change (blocking tool) requiring semver update 🟢

Tests

  • New test cases in test/workflow-tools.test.ts cover SSE streaming and premature stream termination for msq_get_workflow_run_status 🟢
  • Add unit test for parseServerSentEvent with malformed events (e.g., missing data: field, empty stream) to ensure robustness 🟡

@j4ys0n j4ys0n merged commit d026150 into main Apr 6, 2026
1 check passed
@j4ys0n j4ys0n deleted the tools-fix branch April 6, 2026 14:41
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