Skip to content

update for new workflows#7

Merged
j4ys0n merged 1 commit into
mainfrom
tools-fix
Mar 15, 2026
Merged

update for new workflows#7
j4ys0n merged 1 commit into
mainfrom
tools-fix

Conversation

@j4ys0n
Copy link
Copy Markdown
Contributor

@j4ys0n j4ys0n commented Mar 15, 2026

No description provided.

@j4ys0n
Copy link
Copy Markdown
Contributor Author

j4ys0n commented Mar 15, 2026

Automated review 🤖

Summary of Changes
This PR replaces the legacy msq_run_agent_workflow tool with a new suite of seven workflow management tools (msq_list_workflows, msq_get_workflow, msq_create_workflow, msq_update_workflow, msq_run_workflow, msq_get_workflow_run_status, msq_get_workflow_result) to align with the persisted workflow config and workflow run endpoints. The package version is bumped from 0.2.4 to 0.3.0. Documentation in README.md is updated with new tool signatures and a full workflow lifecycle example. A new test file test/workflow-tools.test.ts provides comprehensive coverage for all new tools.

Key Changes & Positives

  • Replaced monolithic msq_run_agent_workflow with granular, composable workflow lifecycle tools in src/tools.ts (hunks +54–+147) 🟢
  • Introduced typed Zod schemas (WorkflowCreateSchema, WorkflowRunCreateSchema, etc.) and robust response parsers in src/schemas.ts and src/tools.ts for type-safe API interaction 🟢
  • Added clear separation between config management (list/get/create/update) and run management (start/status/result) in README.md (lines +152–+169) 🟢
  • New test suite test/workflow-tools.test.ts validates correct HTTP methods, paths, request bodies, and error handling for all tools 🟢

Potential Issues & Recommendations

  1. Issue / Risk: msq_get_workflow performs a client-side filter over the full list response instead of fetching by ID via a dedicated /core/workflows/{id} endpoint (hunk src/tools.ts +447–+458).

    • Impact: Inefficient for large workflow sets and may miss race conditions where the workflow is deleted between list and lookup.
    • Recommendation: Add a dedicated GET /core/workflows/{id} endpoint in the backend and update the tool to use it.
    • Status: 🟡 Needs review
  2. Issue / Risk: msq_get_workflow_result throws an error if main.status !== 'completed' even when status === 'completed' (hunk src/tools.ts +262–+265).

    • Impact: May cause false negatives if the backend transitions main agent asynchronously after overall run completion.
    • Recommendation: Clarify expected semantics in API docs or relax validation to allow main.status === 'completed' || main.status === 'cancelled' if appropriate.
    • Status: 🟡 Needs review

Language/Framework Checks

  • Zod schemas use .passthrough() consistently for extensibility (e.g., WorkflowConfigRecordSchema, WorkflowRunRecordSchema) in src/schemas.ts (lines +162–+185)
  • Path encoding uses encodePathSegment for id and runId parameters in src/tools.ts (hunks +447, +527, +545)
  • Tool definitions use defineTool with explicit parameters and run handlers matching MCP conventions

Security & Privacy

  • Sensitive fields (resolvedInput, chatId, sessionId) are excluded from mapWorkflowRunStatus and mapWorkflowRunResult outputs in src/tools.ts (hunks +227–+232, +245–+250), reducing accidental exposure of PII or session tokens 🟢

Build/CI & Ops

  • Version bump to 0.3.0 in package.json indicates a minor release with backward-incompatible tool replacement (msq_run_agent_workflow removed) — users must migrate to new tool set 🟢
  • Test coverage includes tool-coverage.test.ts sync to ensure new tools are registered (hunk test/tool-coverage.test.ts +20–+26)

Tests

  • New test/workflow-tools.test.ts covers all new tools with mocks for fetch calls, request payloads, and response parsing (402 lines).
  • Critical edge cases validated: missing workflow ID, non-ready results, error/cancelled states, and exclusion of sensitive fields.
  • Ensure CI runs this new test file and that npm test includes it.

Approval Recommendation
Approve with caveats

  • Verify backend implements /core/workflows/{id} endpoint before merging to avoid inefficient client-side filtering
  • Confirm API contract for main.status vs status transition semantics aligns with expected behavior for msq_get_workflow_result

@j4ys0n j4ys0n merged commit fc45d73 into main Mar 15, 2026
1 check passed
@j4ys0n j4ys0n deleted the tools-fix branch March 15, 2026 03:28
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