Skip to content

improvement(mothership): allow mship to send function execute timeout#4581

Merged
Sg312 merged 2 commits into
stagingfrom
improvement/mship-function-execute
May 13, 2026
Merged

improvement(mothership): allow mship to send function execute timeout#4581
Sg312 merged 2 commits into
stagingfrom
improvement/mship-function-execute

Conversation

@Sg312
Copy link
Copy Markdown
Collaborator

@Sg312 Sg312 commented May 13, 2026

Summary

Allows mship to send function execute timeout

Type of Change

  • Bug fix

Testing

Manual

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 13, 2026 9:20pm

Request Review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 13, 2026

Greptile Summary

This PR lets the mothership/copilot side pass an optional timeout (in seconds) when invoking function_execute, with automatic conversion to milliseconds, a 10-second default, and a hard cap at DEFAULT_EXECUTION_TIMEOUT_MS at the executor boundary.

  • executor.ts adds a buildAppToolParams pre-processing step that converts the seconds value to ms and applies the cap before forwarding to the app tool executor; three new tests cover the conversion, default, and cap cases.
  • Both generated schema files (tool-catalog-v1.ts and tool-schemas-v1.ts) are updated to advertise the new timeout parameter; the same files also shorten the rowIds description in UserTable, removing its documented role as an optional row-scope filter for run_column.

Confidence Score: 3/5

The core timeout feature is sound, but the rowIds description truncation in both generated schema files silently removes the run_column scoping capability from AI callers, which could cause unintended full-table column runs.

The timeout conversion, default, and cap logic are correct and well-tested. The bundled rowIds description change drops the documented optional row-scope for run_column, meaning AI callers will always run run_column across the entire table rather than a requested subset.

Both tool-catalog-v1.ts and tool-schemas-v1.ts need attention for the rowIds description truncation.

Important Files Changed

Filename Overview
apps/sim/lib/copilot/tool-executor/executor.ts Adds timeout seconds-to-ms conversion for copilot function_execute calls with correct capping and default; invalid/zero timeout values are silently dropped rather than defaulted.
apps/sim/lib/copilot/generated/tool-catalog-v1.ts Adds timeout parameter to FunctionExecute tool schema; also simplifies rowIds description in UserTable, losing the run_column optional-scope documentation.
apps/sim/lib/copilot/generated/tool-schemas-v1.ts Runtime schema mirrors catalog changes: adds timeout to FunctionExecute, truncates rowIds description to batch_delete_rows only.
apps/sim/lib/copilot/tool-executor/executor.test.ts Good coverage: tests seconds-to-ms conversion, default-timeout injection, and the cap against DEFAULT_EXECUTION_TIMEOUT_MS.
apps/sim/tools/function/types.ts Adds copilotToolExecution flag to _context and a clarifying JSDoc comment on the dual-unit nature of timeout; clean and accurate.

Sequence Diagram

sequenceDiagram
    participant M as Mothership (mship)
    participant E as executeTool (copilot executor)
    participant B as buildAppToolParams
    participant A as executeAppTool (tools/index)
    participant F as function/execute handler

    M->>E: "executeTool("function_execute", {code, timeout: 7}, {copilotToolExecution: true})"
    E->>B: buildAppToolParams("function_execute", params, context)
    Note over B: copilotToolExecution=true<br/>convert seconds to ms<br/>cap at DEFAULT_EXECUTION_TIMEOUT_MS<br/>timeout: 7000 ms
    B-->>E: "{code, timeout: 7000, _context: {...}}"
    E->>A: "executeAppTool("function_execute", {code, timeout: 7000}, false)"
    A->>F: execute with timeout: 7000ms
    F-->>A: result
    A-->>E: "{success, output}"
    E-->>M: ToolExecutionResult
Loading

Reviews (1): Last reviewed commit: "Improve mship fexecute" | Re-trigger Greptile

Comment thread apps/sim/lib/copilot/generated/tool-catalog-v1.ts Outdated
Comment thread apps/sim/lib/copilot/tool-executor/executor.ts Outdated
@Sg312 Sg312 merged commit cdc7513 into staging May 13, 2026
13 checks passed
@waleedlatif1 waleedlatif1 deleted the improvement/mship-function-execute branch May 14, 2026 02:49
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