Skip to content

Commit 1cb3f8f

Browse files
committed
docs(url-path): drop the false claim that widening restores prior behaviour
The module TSDoc said the number/bigint widening fixed 'a regression for the call sites whose pre-guard form was a bare ${params.id} template that stringified a number fine'. It did not. Every pre-guard form in a422990 used .trim() (`/v13/deployments/${params.deploymentId.trim()}`, `sandboxId?.trim()`), so a numeric id threw there too - no importer has ever accepted one. The cited examples were also wrong: only Vercel and Daytona import this module, and neither Box nor X does. Replaced with the real motivation - params are declared type: 'string' but nothing enforces it before the guard, and the old coercion-to-'' turned a supplied numeric id into a misleading 'is required'. Two test comments made the same claim ('still stringifies', 'replaced bare ${params.id} templates') and are corrected; no assertion is weakened.
1 parent d817e09 commit 1cb3f8f

2 files changed

Lines changed: 26 additions & 16 deletions

File tree

apps/sim/tools/url-path.test.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,10 @@ describe('safeUrlPathSegment', () => {
124124
})
125125

126126
/**
127-
* The guard replaced bare `${params.id}` templates at call sites where a
128-
* numeric-looking id (Stripe `id`, Spotify ids, X `woeid`) could arrive as a
129-
* JSON number. Coercing a non-string to `''` reported it as missing.
127+
* Tool params are declared `type: 'string'` but nothing enforces that before
128+
* the value reaches the guard: an LLM tool call or stored workflow state can
129+
* hand a numeric-looking id over as a JSON number. Coercing a non-string to
130+
* `''` reported such a value as missing, which names the wrong problem.
130131
*/
131132
describe('non-string inputs', () => {
132133
it.concurrent.each([
@@ -318,8 +319,11 @@ describe('exponential number spellings', () => {
318319

319320
/**
320321
* The 44 live call sites (Vercel x43, Daytona x1) pass provider ids and
321-
* hostnames as strings, occasionally a numeric id. Their output must be
322-
* byte-identical across this change.
322+
* hostnames, and every one of them interpolated `${params.id.trim()}` before
323+
* these guards existed — so a string is the only shape any of them has ever
324+
* handled, and a string's output must stay byte-identical across this change.
325+
* A numeric id is the newly accepted shape, not a restored one: it threw
326+
* `TypeError: .trim is not a function` at those same call sites before.
323327
*/
324328
describe('live call-site values', () => {
325329
it.concurrent.each([
@@ -336,8 +340,8 @@ describe('live call-site values', () => {
336340
expect(safeUrlPathSegment(value, 'id')).toBe(value)
337341
})
338342

339-
it.concurrent('still stringifies a numeric id', () => {
340-
expect(safeUrlPathSegment(2487956, 'woeid')).toBe('2487956')
341-
expect(safeUrlPathSegment(0, 'folderId')).toBe('0')
343+
it.concurrent('stringifies a numeric id these call sites used to reject', () => {
344+
expect(safeUrlPathSegment(2487956, 'deploymentId')).toBe('2487956')
345+
expect(safeUrlPathSegment(0, 'sandboxId')).toBe('0')
342346
})
343347
})

apps/sim/tools/url-path.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,20 @@
3939
* Trimming is left to the caller, because whether surrounding whitespace is
4040
* copy-paste noise or part of the value is a per-helper decision.
4141
*
42-
* Tool params are declared `type: 'string'`, but the value arrives from an LLM
43-
* tool call or user input and a numeric-looking id (a Box `folderId`, whose
44-
* root folder is literally `0`; an X `woeid`) can land as a JSON **number**.
45-
* The previous `typeof value === 'string' ? value.trim() : ''` turned any such
46-
* value into `''`, which the guards then reported as *"<param> is required"* —
47-
* a confusing error for a value the caller did supply, and a regression for
48-
* the call sites whose pre-guard form was a bare `${params.id}` template that
49-
* stringified a number fine.
42+
* Tool params are declared `type: 'string'`, but that declaration is not
43+
* enforced anywhere before the value reaches here: it arrives from an LLM tool
44+
* call or from stored workflow state, where a numeric-looking id (a Vercel
45+
* `deploymentId`, a Daytona `sandboxId`) can be serialized as a JSON **number**
46+
* and stays one. The previous `typeof value === 'string' ? value.trim() : ''`
47+
* turned any such value into `''`, which the guards then reported as
48+
* *"<param> is required"* — the least actionable message available for a value
49+
* the caller did supply, and one that points at the wrong fix.
50+
*
51+
* This is not a restoration of prior behaviour. Every call site that predates
52+
* these guards interpolated `${params.id.trim()}`, so a numeric id threw
53+
* `TypeError: params.id.trim is not a function` there too. The widening is a
54+
* deliberate improvement: it accepts what callers actually send, and where it
55+
* still refuses (below) it says why by name.
5056
*
5157
* An id too large for a `double` — a Discord snowflake, a Twitter id — is
5258
* **not** in scope here and cannot be: `JSON.parse` destroys the precision

0 commit comments

Comments
 (0)