Skip to content

Commit cfdba91

Browse files
committed
test(tools): probe sibling branch literals in pairs, not just singly
cubic found the docstring overclaimed. Discovery pinned one sibling to one branch literal at a time, so a parameter reachable only when two siblings hold specific values — action === 'unblock' && kind === 'folder' — was never probed and silently untested, while the comment said every branch is probed. Adds pair probing over distinct parameters, capped so a tool with many parameters and many literals cannot blow up combinatorially. The bound is stated rather than glossed: depth stops at two, so three simultaneous conditions would still be missed. No service here needs even one literal to reach any parameter, so the covered count is unchanged at 75. Verified the machinery is live rather than dead code with a synthetic two-condition builder: singles-only discovery misses its id, pair probing finds it.
1 parent 701d013 commit cfdba91

1 file changed

Lines changed: 57 additions & 10 deletions

File tree

apps/sim/tools/__tests__/path-safety.ts

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@
2323
* `delete_*` family, `box_sign_get_request` — are exactly where that blind spot
2424
* lives, so every value in {@link MUST_REJECT} is asserted to *throw*.
2525
*
26-
* **Every branch.** A parameter that only reaches the path on one branch of a
26+
* **Branches.** A parameter that only reaches the path on one branch of a
2727
* conditional builder is invisible to a single-shot probe. Discovery therefore
2828
* reads the literals the builder compares against out of its own source and
29-
* probes each one.
29+
* probes each one, and each **pair** of them — a parameter can sit behind two
30+
* simultaneous conditions. The depth stops at two rather than being exhaustive;
31+
* `siblingAssignments` says so where the bound is set.
3032
*
3133
* Every assertion resolves the built URL with `new URL(...)` — the same
3234
* normalization `fetch` performs — instead of string-matching the template
@@ -149,6 +151,12 @@ const PROBE_ID = 'PROBEID'
149151
/** Not a declared parameter — leaves every real one at its safe value. */
150152
const ALL_SAFE = '__all_safe__'
151153

154+
/**
155+
* Ceiling on probe assignments per tool, so pair-probing cannot turn a tool
156+
* with many parameters and many branch literals into a combinatorial blowup.
157+
*/
158+
const MAX_BRANCH_ASSIGNMENTS = 600
159+
152160
/**
153161
* Fills every declared parameter with a type-appropriate safe value, then
154162
* overrides the single parameter under test.
@@ -214,6 +222,52 @@ function branchLiterals(tool: PathTool): string[] {
214222
return [...literals]
215223
}
216224

225+
/**
226+
* The sibling assignments to probe: the plain one, then each parameter pinned
227+
* to each branch literal, then every **pair** of those pinnings on distinct
228+
* parameters.
229+
*
230+
* Pairs are not decoration. A parameter can sit behind two simultaneous
231+
* conditions — `action === 'unblock' && type === 'folder'` — and a probe that
232+
* only ever pins one sibling at a time never reaches it, so the parameter is
233+
* invisible to discovery and silently untested. Single-pinning alone would make
234+
* "every branch is probed" an overclaim.
235+
*
236+
* The depth stops at two, and that bound is honest rather than exhaustive:
237+
* three simultaneous conditions would still be missed. Going deeper is
238+
* combinatorial in the number of (parameter, literal) pinnings, so the count is
239+
* also capped — beyond {@link MAX_BRANCH_ASSIGNMENTS} the pairs are dropped and
240+
* the single pinnings are kept, since those cover strictly more builders per
241+
* probe. No service currently needs even one literal to reach any parameter, so
242+
* this is machinery for the builders that come later rather than for today's.
243+
*/
244+
function siblingAssignments(names: string[], literals: string[]): Record<string, unknown>[] {
245+
const singles: Record<string, unknown>[] = []
246+
for (const literal of literals) {
247+
for (const name of names) singles.push({ [name]: literal })
248+
}
249+
250+
/**
251+
* The ceiling is checked against the projected count *before* the pairs are
252+
* built, so a tool with many parameters and many literals does not allocate
253+
* tens of thousands of objects only to discard them.
254+
*/
255+
const projected = 1 + singles.length + (singles.length * (singles.length - 1)) / 2
256+
if (projected > MAX_BRANCH_ASSIGNMENTS) return [{}, ...singles]
257+
258+
const pairs: Record<string, unknown>[] = []
259+
for (let i = 0; i < singles.length; i++) {
260+
const [nameA] = Object.keys(singles[i])
261+
for (let j = i + 1; j < singles.length; j++) {
262+
const [nameB] = Object.keys(singles[j])
263+
if (nameA === nameB) continue
264+
pairs.push({ ...singles[i], ...singles[j] })
265+
}
266+
}
267+
268+
return [{}, ...singles, ...pairs]
269+
}
270+
217271
/**
218272
* Enumerates every (tool, parameter) pair of a service whose value lands in a
219273
* URL **path** segment.
@@ -236,14 +290,7 @@ export function discoverPathParams(
236290

237291
const names = Object.keys(tool.params ?? {}).filter((name) => !(name in fixed))
238292

239-
/**
240-
* Every sibling assignment worth probing: the plain one, then each
241-
* parameter pinned to each literal the builder branches on.
242-
*/
243-
const branches: Record<string, unknown>[] = [{}]
244-
for (const literal of branchLiterals(tool)) {
245-
for (const name of names) branches.push({ [name]: literal })
246-
}
293+
const branches = siblingAssignments(names, branchLiterals(tool))
247294

248295
/**
249296
* Buildability is decided from an all-safe build, independent of the

0 commit comments

Comments
 (0)