Commit 51d2118
feat(tools): run one integration tool over v2, and fix the ones that could not be run (#7374)
* feat(tools): run one integration tool over v2, and fix the ones that could not be run
Adds `POST /api/v2/tools/{toolId}/execute` and `sim tools execute <toolId>`,
then fixes the 119 tool parameters that made the same call impossible through
Copilot.
The catalog already published every tool's parameters and outputs; there was
no way to supply them. The only execution endpoint on v2 was
`POST /workflows/{id}/execute`, so a caller could read that `gmail_send` takes
`{to, subject, body}` and still had to wrap it in a workflow to send the mail.
Building it surfaced the second half. Copilot authorizes integration tools when
their schemas are built — `projectIntegrationToolsForViewer` decides what the
model is told exists — so by dispatch the id has been vouched for. An endpoint
whose caller types the id has to make those decisions against it, which is what
the use case does, and which is how the parameters below came to light.
117 of them are mislabels: mailchimp and zendesk mark a user-typed API key
`hidden`, which firecrawl spells `user-only` for the identical shape, and
pipedrive and wealthbox omit the `oauth` declaration that fills the
`accessToken` they correctly hide. The other 2 are not — calcom's `attendee`
and mistral's `file` are composed by their block at serialization time, so
`hidden` was right until a caller could reach the tool directly.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(mistral): align MistralParseV3Block on the tool's `file` parameter
`check-block-registry.ts` requires every required `user-only` tool parameter to
have a subBlock whose `id` or `canonicalParamId` matches it, because the
serializer resolves those by direct lookup and a mismatch false-flags the field
as missing at submit time. V3's subBlocks carried `canonicalParamId: 'document'`
against a tool parameter named `file`, bridged by the block's params mapper.
Aligning them needs no migration. Saved state is keyed by subBlock id
(`fileUpload`, `fileReference`), which is unchanged; `canonicalParamId` is a
config-derived index rather than a storage key. `data.canonicalModes` is keyed
by canonical id, but `backfillCanonicalModes` re-derives a missing entry from
whichever value is populated, so a workflow using the advanced file-reference
field still resolves to `advanced` on open.
V2 keeps `document`: only V3's `file` parameter is `required`, so only V3 is
under the contract.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(tools): reject a direct call missing a required user-only input
`validateRequiredParametersAfterMerge` checks `user-or-llm` parameters alone,
because on the workflow path a `user-only` parameter was already validated
during serialization against the block field that holds it. The v2 execute path
has no serialization step, so nothing had checked them: omitting
`zendesk_get_ticket`'s `subdomain` reached Zendesk as `undefined` and came back
a provider authentication failure — the same undiagnosable shape this branch
set out to remove.
The check exempts a parameter Sim supplies itself, mirroring
`injectHostedKeyIfNeeded`'s three tests in the same order so the two cannot
disagree about whether a value is coming. `firecrawl_scrape` stays callable
with no `apiKey` where keys are hosted, and requires one where they are not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(tools): require every input the caller is the only source for
The previous commit gated the check on `user-only`, which reads the visibility
taxonomy as if it constrained who may send a value. It does not. `visibility`
describes editor roles — a human filling a block field, the agent block's model
choosing an argument, either, or neither — and a direct call has no editor and
no agent block, so those roles collapse to one caller.
`createUserToolSchema`, which this endpoint and Copilot's
`call_integration_tool` both publish, already says so by omitting `hidden` and
nothing else.
So the rule is not about roles: Sim supplies it, or the caller must. Skipping
`hidden` stays safe because `check-tool-param-reachability` fails any required
hidden parameter without a declared filler.
Concretely this closes `thinking_tool.thought`, the one required `llm-only`
parameter in the registry, which the narrower check let through as `undefined`.
It also moves required `user-or-llm` inputs to a pre-dispatch failure naming
every missing field at once, rather than the merge validator's first-failure
mid-execution.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(tools): address the cubic review round on direct tool execution
Two were real defects rather than polish:
The usage ledger de-duplicates on `eventKey`, and the derived key hashes actor,
workspace, source and description — identical for every call to the same tool.
`onConflictDoNothing` therefore billed the first hosted-key call and silently
dropped every one after it. A workflow run has an `executionId` to separate its
rows; a direct call has nothing, so it now mints one.
Undeclared input keys reached the executor. `impersonateUserEmail` is read
straight out of params and forwarded to credential-token resolution as an
impersonation request, and no tool declares it. The two ad-hoc denylists are
replaced by a declared-parameter allowlist — the actual boundary, and the one
`GET /api/v2/tools/{toolId}` already publishes.
Also: renaming a canonical id orphans `data.canonicalModes`, and the backfill
recovers it only when one side holds a value. `setBlockCanonicalMode` writes the
mode without clearing its sibling, so a workflow holding both would have
silently switched from the typed reference to the uploaded file.
`migrateCanonicalModeIds` carries the key across, ahead of the backfill.
Smaller: the reachability audit now mirrors execution exactly, exempting
`credentialType` only under `authoritativeParams` and refusing to treat
conditional hosting as a guarantee; the boundary guard splits on both path
separators so it still excludes the execute route on Windows; the route stops
documenting the 409 and 423 it cannot produce; and `sim tools execute` prints
the output it promised in the human formats.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(tools): bill only spend that was Sim's, and scope the boundary predicate
Metering read `output.cost.total` and billed it. That field is not a hosted-key
marker: `knowledge_upload_chunk` and the enrichment runner report their own cost
there, so a workspace would have been charged `api-tool` spend for something
already metered elsewhere, and a failed call carrying a cost billed too. The
registry writes hosted-key cost under exactly two conditions —
`hostedKeyInfo.isUsingHostedKey && finalResult.success` — and metering now
matches them.
`hostedKeyParamFor` derives the first of those the way `injectHostedKeyIfNeeded`
does, including the test the earlier version omitted: a caller's own key wins,
so supplying one means Sim's key was never spent. The same helper answers the
other question that turns on it, since a required parameter Sim fills is not a
missing one.
The boundary predicate matched `execute` anywhere in the absolute path, so a
checkout living under a directory of that name would have excluded every
catalog route and retired the guard silently. Now derived from the path relative
to the app, still split on either separator for Windows.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(tools): refuse a caller-supplied hidden parameter
Declared was being treated as accepted. A `hidden` parameter is Sim's to fill —
a resolved credential's `accessToken`, a hosted key, a block-composed shape —
and `createUserToolSchema` omits it from what this endpoint and Copilot
publish. Accepting it anyway either let a caller pre-empt the executor's value
or silently discarded theirs when the executor overwrote it; either way the
published schema made no such promise.
The accept-set is now exactly the publish-set: a key is taken if and only if
`GET /api/v2/tools/{toolId}` lists it as something the caller may send. This is
the rule the required-input check already followed ("Sim fills it, or the caller
must"), applied to the other direction.
Verified live against a dev server: a forged `accessToken` on `gmail_read_v2`
is refused before dispatch, and the credential-backed read still succeeds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(tools): honor a tool's declared credential selector, and refuse its aliases first
Sixty-eight tools — Snowflake among them — declare the credential selector as a
required `user-only` parameter (`oauthCredential` or `credential`) with no
`oauth` block, filled by their block from an `oauth-input` field. The
required-input check ran against the raw body, before the top-level
`credentialId` had been placed anywhere, so a valid credential was rejected as
a missing `oauthCredential`. And the alias refusal ran only over *undeclared*
keys, so `input.oauthCredential` on such a tool passed the declared-key check
and bypassed the top-level field — credential precedence differing per tool.
The credential is still named once, at the top level. It now lands under
whichever selector the tool declares (or `credential`, which the executor reads
for OAuth resolution, when it declares none), and required inputs are validated
against what the executor will actually receive. A declared required selector
also demands `credentialId` up front, the same as an `oauth` block does. The
alias refusal is unconditional and runs first.
Verified live: a Snowflake call with a top-level credential passes the
validator and fails downstream at resolution; `input.oauthCredential` is
refused; omitting the credential names `credentialId` as required.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(tools): read the registry's hosted-key verdict instead of re-deriving it
The metering gate re-derived "did Sim's key pay" from the tool's hosting config
and whether the caller omitted the key. That cannot see a workspace or
organization BYOK key, which `injectHostedKeyIfNeeded` injects while reporting
`isUsingHostedKey: false` — the org's own key, not billable — so the derivation
called a BYOK call hosted.
The registry's verdict does propagate, by one path. On a tool with `hosting`,
`output.cost` has a single writer, `applyHostedKeyCostToResult`, and it runs only
under `isUsingHostedKey && success`. A BYOK call, a caller-keyed call, and a
failed call all leave the field absent. The gate now reads that: `hosting`
present, success, cost present. The derivation stays for the one question it
can answer before dispatch — a parameter Sim will fill is not missing.
The reading rests on hosted tools not reporting their own cost in that field,
which is true of all 131 today and now pinned by the reachability audit, so a
future hosted tool that self-reports fails CI rather than billing BYOK calls.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(audit): exempt a token-response field only where the tool declares it
The reachability audit treated seven credential-derived fields as guaranteed
fillers for a required hidden parameter on any OAuth tool. Only `accessToken`
is: the resolver assigns it unconditionally. `idToken`, `instanceUrl`,
`apiDomain`, `cloudId`, `domain` and `authStyle` are assigned under
`if (data.X)` — present on some providers' credentials and absent on others —
and `credentialType` additionally only when the tool lists it in
`authoritativeParams`. Whether a credential carries one is a fact about the
provider that the resolver cannot vouch for.
The tool can. `oauth.authoritativeParams` is already the declaration that the
token response supplies the named field, and every real case — the eight
`microsoft_dynamics_365_*` tools hiding `instanceUrl` — already lists it. So a
required hidden parameter in that set is now exempt only when its tool declares
it there, generalising the rule `credentialType` alone had. A tool that hides
one without declaring it is asserting a filler the resolver may never run,
which is the shape this audit exists to reject.
Mutation-tested at the real declaration site: stripping `authoritativeParams`
from `DYNAMICS_365_OAUTH_CONFIG` fails the audit naming all eight tools with the
remedy; restoring passes. Nothing is flagged on the current tree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>1 parent 452e542 commit 51d2118
159 files changed
Lines changed: 23402 additions & 166 deletions
File tree
- apps
- docs
- content/docs
- cli
- integrations
- sim
- app/api/v2/tools/[toolId]/execute
- blocks/blocks
- lib
- api/contracts/v2
- openapi
- billing
- catalog/application
- copilot/tool-executor
- core/application
- tool-execution/application
- workflows
- migrations
- persistence
- tools
- calcom
- generated
- mailchimp
- mistral
- pipedrive
- wealthbox
- zendesk
- packages
- db
- migrations
- meta
- sim-cli/src
- contract
- generated
- http
- runtime
- scripts
- openapi
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
42 | | - | |
| 42 | + | |
43 | 43 | | |
44 | 44 | | |
45 | 45 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
263 | 263 | | |
264 | 264 | | |
265 | 265 | | |
266 | | - | |
| 266 | + | |
267 | 267 | | |
268 | 268 | | |
269 | 269 | | |
| |||
4366 | 4366 | | |
4367 | 4367 | | |
4368 | 4368 | | |
| 4369 | + | |
| 4370 | + | |
| 4371 | + | |
| 4372 | + | |
| 4373 | + | |
| 4374 | + | |
| 4375 | + | |
| 4376 | + | |
| 4377 | + | |
| 4378 | + | |
| 4379 | + | |
| 4380 | + | |
| 4381 | + | |
| 4382 | + | |
| 4383 | + | |
| 4384 | + | |
| 4385 | + | |
| 4386 | + | |
| 4387 | + | |
| 4388 | + | |
| 4389 | + | |
| 4390 | + | |
| 4391 | + | |
| 4392 | + | |
| 4393 | + | |
| 4394 | + | |
| 4395 | + | |
| 4396 | + | |
| 4397 | + | |
| 4398 | + | |
4369 | 4399 | | |
4370 | 4400 | | |
4371 | 4401 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
10 | 40 | | |
11 | 41 | | |
12 | 42 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
47 | | - | |
| 47 | + | |
48 | 48 | | |
49 | 49 | | |
50 | 50 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
122 | 122 | | |
123 | 123 | | |
124 | 124 | | |
125 | | - | |
| 125 | + | |
| 126 | + | |
126 | 127 | | |
127 | 128 | | |
128 | 129 | | |
| |||
452 | 453 | | |
453 | 454 | | |
454 | 455 | | |
455 | | - | |
| 456 | + | |
456 | 457 | | |
457 | 458 | | |
458 | 459 | | |
| |||
636 | 637 | | |
637 | 638 | | |
638 | 639 | | |
639 | | - | |
| 640 | + | |
| 641 | + | |
640 | 642 | | |
641 | 643 | | |
642 | 644 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2793 | 2793 | | |
2794 | 2794 | | |
2795 | 2795 | | |
2796 | | - | |
| 2796 | + | |
2797 | 2797 | | |
2798 | 2798 | | |
2799 | 2799 | | |
| |||
0 commit comments