Skip to content

Commit d66b909

Browse files
icecrasher321claude
andcommitted
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>
1 parent 17872f6 commit d66b909

3 files changed

Lines changed: 79 additions & 11 deletions

File tree

apps/sim/lib/tool-execution/application/execute-tool.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -606,17 +606,40 @@ describe('executeToolForCaller', () => {
606606
expect(mocks.recordUsage).not.toHaveBeenCalled()
607607
})
608608

609+
/**
610+
* Mirrors the real registry contract: a caller's own key means
611+
* `isUsingHostedKey` is false, so `applyHostedKeyCostToResult` never runs and
612+
* no `output.cost` is written. The meter reads that absence as the verdict.
613+
*/
609614
it('does not bill when the caller brought their own key', async () => {
610615
mocks.executeRegistryTool.mockResolvedValue({
611616
success: true,
612-
output: { cost: { total: 0.004 } },
617+
output: { markdown: '# Hi' },
613618
})
614619

615620
await run({ input: { url: 'https://a.co', apiKey: 'sk-mine' } })
616621

617622
expect(mocks.recordUsage).not.toHaveBeenCalled()
618623
})
619624

625+
/**
626+
* The BYOK shape. The registry injected the org's own key and returned
627+
* `isUsingHostedKey: false`, so it wrote no `output.cost` — and the caller
628+
* omitted the key, which a pre-dispatch derivation reads as "Sim's". Only the
629+
* registry's verdict, carried by the presence of the cost it alone writes,
630+
* gets this right.
631+
*/
632+
it('does not bill a BYOK call, where the key was omitted but Sim did not pay', async () => {
633+
mocks.executeRegistryTool.mockResolvedValue({
634+
success: true,
635+
output: { markdown: '# Hi' },
636+
})
637+
638+
await run({ input: { url: 'https://a.co' } })
639+
640+
expect(mocks.recordUsage).not.toHaveBeenCalled()
641+
})
642+
620643
it('does not bill a failed call', async () => {
621644
mocks.executeRegistryTool.mockResolvedValue({
622645
success: false,

apps/sim/lib/tool-execution/application/execute-tool.ts

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@ export interface ExecuteToolResult {
4949
* deployment hosts keys, any `enabled` predicate accepts these params, and the
5050
* caller has not brought a key of their own — which wins where present.
5151
*
52-
* Read twice, for the two questions that both turn on it. A required parameter
53-
* Sim fills is not missing, and spend is only Sim's to bill when Sim's key paid
54-
* for it.
52+
* Pre-dispatch only, for the required-input exemption: a parameter Sim will
53+
* fill is not missing. It is deliberately NOT the metering gate — it cannot see
54+
* a BYOK key, which the registry injects while reporting the call as *not*
55+
* hosted, so after dispatch the registry's own verdict is read instead.
5556
*/
5657
function hostedKeyParamFor(
5758
tool: ExecutableToolConfig,
@@ -342,14 +343,26 @@ export const executeToolForCaller = defineAuthorizedWorkspaceUseCase({
342343
})
343344

344345
/**
345-
* Only a successful call that spent Sim's key. `output.cost` is not a
346-
* hosted-key marker: `knowledge_upload_chunk` and the enrichment runner
347-
* report their own cost there, and billing those as `api-tool` spend would
348-
* charge a second time for something already metered elsewhere. The registry
349-
* writes hosted-key cost under the same two conditions
350-
* (`hostedKeyInfo.isUsingHostedKey && finalResult.success`).
346+
* Only a successful call that spent Sim's key — and that verdict is the
347+
* registry's, not re-derived here.
348+
*
349+
* The registry decides whether Sim's key was used inside
350+
* `injectHostedKeyIfNeeded`, and a workspace or organization BYOK key is
351+
* one of the ways it decides *no*: the org's own key is injected and
352+
* `isUsingHostedKey` is false. A pre-dispatch derivation cannot see that
353+
* (it would need the BYOK lookup), so an earlier version of this gate
354+
* treated every omitted key as Sim's and was wrong for BYOK.
355+
*
356+
* The verdict does propagate, by one path: on a tool with `hosting`,
357+
* `output.cost` has a single writer, `applyHostedKeyCostToResult`, and it
358+
* runs only under `hostedKeyInfo.isUsingHostedKey && finalResult.success`.
359+
* So `hosting` present + success + cost present *is* "Sim's key paid".
360+
* `hosting` is checked because tools without it — `knowledge_upload_chunk`,
361+
* the enrichment runner — report their own cost in that field and are
362+
* metered elsewhere. That no hosted tool does the same is what
363+
* `check-tool-param-reachability` now pins.
351364
*/
352-
if (result.success && hostedKeyParamFor(tool, params)) {
365+
if (result.success && tool.hosting) {
353366
await meterHostedKeySpend({
354367
callId,
355368
toolId,

scripts/check-tool-param-reachability.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,27 @@ interface Finding {
8585
reason: string
8686
}
8787

88+
/**
89+
* A hosted tool must not declare its own `cost` output.
90+
*
91+
* Direct execution (`POST /api/v2/tools/{toolId}/execute`) bills hosted-key
92+
* spend by reading `output.cost` on a tool with `hosting` — because on such a
93+
* tool that field has exactly one writer, `applyHostedKeyCostToResult`, which
94+
* runs only when the registry actually used Sim's key on a successful call. A
95+
* BYOK call leaves it absent and so is not billed. A hosted tool that also
96+
* reported its own cost there would break that reading: its self-reported
97+
* number would bill as Sim's spend on a BYOK call or a caller-keyed call. Tools
98+
* without `hosting` may report cost freely; the meter never looks at them.
99+
*/
100+
function findHostedToolsReportingCost(): string[] {
101+
return Object.entries(tools as Record<string, ToolConfig>)
102+
.filter(
103+
([, config]) => config.hosting && config.outputs && Object.hasOwn(config.outputs, 'cost')
104+
)
105+
.map(([toolId]) => toolId)
106+
.sort()
107+
}
108+
88109
function findUnreachableParams(): Finding[] {
89110
const findings: Finding[] = []
90111

@@ -129,6 +150,17 @@ function findUnreachableParams(): Finding[] {
129150
function main(): void {
130151
const findings = findUnreachableParams()
131152
const toolCount = Object.keys(tools).length
153+
const costReporters = findHostedToolsReportingCost()
154+
155+
if (costReporters.length > 0) {
156+
console.error('Tool parameter reachability audit failed:\n')
157+
for (const toolId of costReporters) {
158+
console.error(
159+
` ${toolId} — declares hosting AND a 'cost' output; direct execution reads output.cost on a hosted tool as "Sim's key paid", so a self-reported cost would bill BYOK and caller-keyed calls`
160+
)
161+
}
162+
process.exit(1)
163+
}
132164

133165
if (findings.length === 0) {
134166
console.log(

0 commit comments

Comments
 (0)