docs(skills): encode integration-hardening learnings into the authoring skills

Eight traps found across a 15-PR integration-hardening sweep, each written
into the skill that owns it and cross-referenced rather than duplicated.

add-tools
- Reserved param names. The shared transport reads `timeout`, `proxyUrl`, and
  `method` off `params` before `request` sees them; `timeout` is its own HTTP
  deadline in milliseconds, so Daytona's documented 10-second sandbox timeout
  aborts the call after 10ms.
- Path traversal. `encodeURIComponent` does not stop `.`/`..` — they are
  unreserved and the URL parser removes dot segments after decoding. Documents
  when each of the three `tools/url-path.ts` helpers applies, and why
  `params.x?.trim()` guards `undefined` rather than the type.

add-block
- Omitting a key from `tools.config.params` does not drop it; the executor
  merges the patch over the raw inputs, so clearing a key needs an explicit
  `undefined`.
- Renaming a subBlock id orphans saved workflow state. Rename the tool param
  and map it; `_removed_` migrations cover genuine removals.
- Declared `outputs` do not drive variable resolution — the resolver walks the
  runtime object, so changing an output's shape breaks references that were
  never declared.

validate-integration
- Path-safety harness design: enumerate (tool, param) pairs, fuzz one at a time,
  assert named rejection rather than path shape, probe conditional and presence
  branches, and assert the skip ledger is empty.
- Test files are type-checked by nothing — tsconfig excludes them and Vitest
  transpiles without checking.
- Replaces the two checklist lines that taught the now-known-defective
  `${params.id.trim()}` path pattern.

add-integration gets pointers only.

Author: waleedlatif1

.agents/skills/add-block/SKILL.md

@@ -550,6 +550,30 @@ Maps multiple UI fields to a single serialized parameter:
 - ONLY use `canonicalParamId` to link basic/advanced mode alternatives for the same logical parameter
 - Do NOT use it for any other purpose
 
+## Renaming a SubBlock Id Orphans Saved Workflow State
+
+A subBlock `id` is the storage key for every value users have already saved in deployed workflows.
+Renaming it silently orphans that state — the field renders empty and the workflow runs without it.
+
+**Rename the *tool* param and map it; keep the subBlock id.** `tools.config.params` is where the two
+names meet:
+
+```typescript
+// subBlock id stays `timeout` — saved state keeps resolving
+params: (params) => ({ timeoutSeconds: params.timeout, timeout: undefined }),
+```
+
+`apps/sim/scripts/check-block-registry.ts` enforces this: it diffs the block definitions and fails
+when a subblock id disappears (`check-block-registry.ts:181`), pointing you at the migration table.
+It separately fails when a required `user-only` tool param has no subBlock whose `id` **or**
+`canonicalParamId` equals it (`check-block-registry.ts:225`), which is the other half of the same
+contract.
+
+For a field that is genuinely gone — not renamed — record it in `SUBBLOCK_ID_MIGRATIONS` in
+`apps/sim/lib/workflows/migrations/subblock-migrations.ts`. A `to` value prefixed with `_removed_`
+(`subblock-migrations.ts:20`, `:109`) means "deleted outright", and is what lets the check pass
+without pretending the value moved somewhere.
+
 ## WandConfig Pattern
 
 Enables AI-assisted field generation.
@@ -607,6 +631,29 @@ tools: {
 }
 ```
 
+### Omitting a key from `tools.config.params` does NOT drop it
+
+`tools.config.params` returns a **patch**, not a replacement. The executor merges it over the raw
+inputs — `apps/sim/executor/handlers/generic/generic-handler.ts:191`:
+
+```typescript
+const transformedParams = blockConfig.tools.config.params(inputs)
+finalInputs = { ...inputs, ...transformedParams }
+```
+
+So a subBlock value reaches the tool even when `params` never mentions its key. This matters when a
+subBlock id collides with a name the shared transport reserves (`timeout`, `proxyUrl`, `method` — see
+**Reserved Parameter Names** in the `add-tools` skill): renaming only the tool-side param leaves the
+old key merging straight back in. Clearing it requires an **explicit** `undefined`:
+
+```typescript
+params: (params) => ({
+  // ✓ Removes the reserved key from the outbound params
+  timeout: undefined,
+  timeoutSeconds: params.timeout,
+}),
+```
+
 ### V2 Versioned Tool Selector
 ```typescript
 import { createVersionedToolSelector } from '@/blocks/utils'
@@ -681,6 +728,24 @@ Nested object outputs (`plan: { id: { type: 'string' }, ... }`) are a **tool-out
 
 If the output shape is unknown because the underlying tool response is undocumented, you MUST tell the user and stop. Unknown is not the same as variable. Never guess block outputs.
 
+### Declared `outputs` do not drive variable resolution
+
+`outputs` is documentation and editor autocomplete — it is **not** the contract `<block.path>`
+references resolve against. `apps/sim/executor/utils/block-reference.ts:239` navigates the **runtime**
+output object, and the declared schema is only consulted at `:242` — `if (value === undefined && schema)`
+— to produce a better error for a path that resolved to nothing.
+
+Two consequences:
+
+- `<block.some.path>` resolves fine even when `some.path` was never declared. An undeclared field is
+  still a live reference someone may have wired up.
+- Changing an output's **shape** therefore breaks saved references that never appeared in `outputs`
+  at all, and no check will tell you. Adding a field is safe; re-nesting, renaming, or wrapping the
+  existing keys is not.
+
+When restructuring a `transformResponse`, spread the raw provider keys **last** so every previously
+reachable path survives alongside the new shape.
+
 ## V2 Block Pattern
 
 When creating V2 blocks (alongside legacy V1):
@@ -1028,6 +1093,9 @@ changes.
 - [ ] Tools.access lists all tool IDs (snake_case)
 - [ ] Tools.config.tool returns correct tool ID (snake_case)
 - [ ] Outputs match tool outputs
+- [ ] No existing subBlock `id` was renamed or removed without a `SUBBLOCK_ID_MIGRATIONS` entry
+- [ ] Restructured outputs still expose every previously reachable runtime key (raw keys spread last)
+- [ ] Any reserved transport key a subBlock still sends is explicitly cleared with `undefined` in `tools.config.params`
 - [ ] Block + meta registered in registry-maps.ts (`BLOCK_REGISTRY` / `BLOCK_META_REGISTRY`)
 - [ ] If any tool was added, changed or removed alongside the block: ran `bun run tool-metadata:generate` and committed the artifacts
 - [ ] Ran `bun run scripts/generate-docs.ts`, reviewed the generated diff, and committed the integration catalog changes

.agents/skills/add-integration/SKILL.md

@@ -930,4 +930,15 @@ requiredScopes: getScopesForService('{service}'),
 11. **Never hardcode scopes** - Use `getScopesForService()` in blocks and `getCanonicalScopesForProvider()` in auth.ts
 12. **Always add scope descriptions** - New scopes must have entries in `SCOPE_DESCRIPTIONS` within `lib/oauth/utils.ts`
 13. **OAuth service IDs need deployment capabilities** - Every visible OAuth integration must resolve through `OAUTH_CLIENT_CAPABILITIES`; shared Google/Microsoft aliases map to their provider capability
-14. **Keep runtime and presentation separate** - Runtime OAuth fields live in `packages/deployment-config/src/env-capabilities.ts`; CLI input modes live in the exhaustively checked `packages/sim-setup/src/capability-config.ts` mapping
+14. **`timeout`, `proxyUrl`, and `method` are reserved** - `apps/sim/tools/request-transport.ts`
+    reads all three off `params` (`:191`, `:198`, `:167`); `timeout` is its own HTTP deadline in
+    **milliseconds**. See **Reserved Parameter Names** in `.agents/skills/add-tools/SKILL.md`
+15. **Never interpolate a param into a URL path raw** - `encodeURIComponent` does not stop `.`/`..`
+    traversal. Use the helpers in `apps/sim/tools/url-path.ts`; see **Path Parameters: Reject
+    Traversal, Never Just Encode** in `.agents/skills/add-tools/SKILL.md`, and add the
+    `path_safety.test.ts` shape from `.agents/skills/validate-integration/SKILL.md`
+16. **Never rename or drop a subBlock `id`** - it is the storage key for deployed workflows. Rename
+    the tool param and map it in `tools.config.params`; see the `add-block` skill
+17. **Omitting a key from `tools.config.params` does not drop it** - the executor merges the patch
+    over the raw inputs, so clearing a key needs an explicit `undefined`; see the `add-block` skill
+18. **Keep runtime and presentation separate** - Runtime OAuth fields live in `packages/deployment-config/src/env-capabilities.ts`; CLI input modes live in the exhaustively checked `packages/sim-setup/src/capability-config.ts` mapping

.agents/skills/add-tools/SKILL.md

@@ -199,6 +199,27 @@ fallback, or caller-controlled `_context` authority.
 - Always explicitly set `required: true` or `required: false`
 - Optional params should have `required: false`
 
+### Reserved Parameter Names
+
+The shared transport reads three names off `params` for its own purposes, before your `request`
+config ever sees them (`apps/sim/tools/request-transport.ts`):
+
+| Param | Read at | What the transport does with it |
+|---|---|---|
+| `timeout` | `request-transport.ts:191` | Outbound HTTP deadline **in milliseconds**, clamped to `getMaxExecutionTimeout()` |
+| `proxyUrl` | `request-transport.ts:198` | Egress proxy URL for the request |
+| `method` | `request-transport.ts:167` | Overrides a *static* `request.method` string (a `method` **function** wins over it) |
+
+Never declare a user-facing param with one of these names unless it means exactly what the transport
+means. The collision is silent and unit-blind: `apps/sim/tools/daytona/execute_command.ts:49`
+declares `timeout` as *"Timeout in seconds (defaults to 10 seconds)"*, so a documented 10-second
+sandbox timeout aborts the HTTP call after **10 milliseconds**.
+
+Give the param a distinct Sim-side name (`timeoutSeconds`, `executionTimeout`, `httpMethod`) and emit
+the provider's spelling from `request.body` or `request.url`. Renaming the tool param is not enough
+on its own if a block already sends the reserved key — see **Omitting a key from `tools.config.params`
+does not drop it** in the `add-block` skill.
+
 ## Resolved Secrets and Provenance Boundaries
 
 - Leave ordinary external API inputs and third-party results unchanged. Add provenance handling only
@@ -219,6 +240,48 @@ fallback, or caller-controlled `_context` authority.
 - Add focused tests for named projection, identical unproven public text, malformed/incomplete
   metadata, metadata stripping, scope isolation, and legacy compatibility where applicable.
 
+## Path Parameters: Reject Traversal, Never Just Encode
+
+`encodeURIComponent` does **not** stop path traversal. `.` and `..` are *unreserved* characters, so
+they survive encoding verbatim, and the WHATWG URL parser that `fetch` uses removes dot segments
+**after** decoding — the percent-encoded spellings included:
+
+```
+new URL('https://x/v1/a/b/..').pathname     // => '/v1/a/'
+new URL('https://x/v1/a/b/%2e%2e').pathname // => '/v1/a/'   (still removed)
+```
+
+A removed segment pops a path segment on a fixed host with the workspace's bearer token still
+attached, including on DELETE routes. Path IDs are typically `visibility: 'user-or-llm'`, so prompt
+injection controls them. Rejection is the only mechanism that closes this; the module note at the top
+of `apps/sim/tools/url-path.ts` states the rule in full.
+
+Never interpolate a param into a request path yourself. Use the helpers in `apps/sim/tools/url-path.ts`:
+
+| Helper | Use when the parameter is | Trims? |
+|---|---|---|
+| `safeUrlPathSegment` (`url-path.ts:172`) | **One opaque id** — `user_abc`, `12345`, a repo name. Rejects any `/` or `\`: a separator means the caller passed something other than what the parameter addresses. | Yes — surrounding whitespace on a copy-pasted id is transport noise. |
+| `safeUrlPath` | **A real slash-delimited path** the provider documents as such (GitHub `path`, `branch`, `ref`). Splits on `/`, rejects any `.`/`..` segment, percent-encodes each segment, keeps the separators. | **No** — a leading or trailing space is a legal git filename, so trimming would read, update, or *delete* a different file. |
+| `safeEncodedUrlPathSegment` | **One value that may itself contain `/`** but the provider reads as a single parameter (a GitHub label `area/api` in `DELETE .../labels/{name}`). Preserves the separator as `%2F`. | Yes. |
+
+Prefer `safeUrlPathSegment`. Reach for the other two only when the provider documents the parameter
+as slash-bearing — never to make a separator stop erroring on a single-segment id. (`safeUrlPath` and
+`safeEncodedUrlPathSegment` arrive with the path-safety sweep; if your checkout only exports
+`safeUrlPathSegment`, add them there rather than hand-rolling a local encoder.)
+
+### `params.x?.trim()` guards `undefined`, not the type
+
+A param's declared `type: 'string'` is enforced nowhere between the LLM tool call — or a
+`<Block.output>` reference resolved out of stored workflow state — and your URL builder. A
+numeric-looking id (a Vercel `deploymentId`, a Daytona `sandboxId`) arrives as a JSON **number** and
+stays one, so `params.id?.trim()` throws a bare `TypeError: params.id.trim is not a function` naming
+neither the tool nor the parameter.
+
+`toGuardedString` (`apps/sim/tools/url-path.ts:98`) is why the helpers do not have this problem: it
+accepts `string`, `number`, and `bigint`, rejects everything else **by parameter name**, and refuses
+number spellings whose decimal text is not the id the caller meant. Route path params through the
+helpers instead of hand-rolling an optional-chained `trim()`.
+
 ## Critical Rules for Outputs
 
 ### Output Types
@@ -527,6 +590,8 @@ All tool IDs MUST use `snake_case`: `{service}_{action}` (e.g., `x_create_tweet`
 - [ ] No tool declares `directExecution`; in-process work uses a registered operation
 - [ ] All params have explicit `required: true` or `required: false`
 - [ ] All params have appropriate `visibility`
+- [ ] No param is named `timeout`, `proxyUrl`, or `method` unless it means what the transport means
+- [ ] Every param interpolated into a request path goes through a `tools/url-path.ts` helper
 - [ ] All nullable response fields use `?? null`
 - [ ] All optional outputs have `optional: true`
 - [ ] No raw JSON dumps in outputs

.agents/skills/validate-integration/SKILL.md

@@ -82,6 +82,10 @@ For **every** tool file, check:
   - `'user-only'` — for API keys, credentials, and account-specific IDs the user must provide
   - `'user-or-llm'` — for everything else (search queries, content, filters, IDs that could come from other blocks)
 - [ ] Every param has a `description` that explains what it does
+- [ ] No param is named `timeout`, `proxyUrl`, or `method` unless it means exactly what the shared
+      transport means — `apps/sim/tools/request-transport.ts` reads all three off `params`
+      (`:191`, `:198`, `:167`), and `timeout` is milliseconds. See **Reserved Parameter Names** in
+      `.agents/skills/add-tools/SKILL.md`
 
 ### Request
 - [ ] URL matches the API endpoint exactly (correct base URL, path segments, path params)
@@ -92,8 +96,12 @@ For **every** tool file, check:
 - [ ] `Content-Type` header is set for POST/PUT/PATCH requests
 - [ ] Body sends all required fields and only includes optional fields when provided
 - [ ] For GET requests with query params: URL is constructed correctly with query string
-- [ ] ID fields in URL paths are `.trim()`-ed to prevent copy-paste whitespace errors
-- [ ] Path params use template literals correctly: `` `https://api.service.com/v1/${params.id.trim()}` ``
+- [ ] Every param interpolated into a URL path goes through a helper from `apps/sim/tools/url-path.ts`
+      (`safeUrlPathSegment` for an opaque id, `safeUrlPath` for a documented slash-delimited path,
+      `safeEncodedUrlPathSegment` for a single value that may contain `/`) — never a bare
+      `` `${params.id.trim()}` `` and never a bare `encodeURIComponent`. See **Path Parameters:
+      Reject Traversal, Never Just Encode** in `.agents/skills/add-tools/SKILL.md` for why encoding
+      is insufficient and why `params.id?.trim()` throws a raw `TypeError` on a numeric id
 
 ### Response / transformResponse
 - [ ] Correctly parses the API response (`await response.json()`)
@@ -329,13 +337,68 @@ If any tool lists, searches, exports, imports, downloads, uploads, paginates, ba
 - [ ] Large result payloads are summarized, paginated, referenced, or capped rather than raw-dumped
 - [ ] Pagination and download tests cover caps, early stop behavior, or partial-result preservation when relevant
 
-## Step 9: Validate Error Handling
+## Step 8: Validate Path-Traversal Safety
+
+If any tool interpolates a param into a URL path, the integration needs a path-safety suite. Design it
+as follows — the naive shape does not work, and looks like it does.
+
+**Why the naive harness is worthless.** A suite that fuzzes every param at once and wraps the build in
+`try { ... } catch { return }` reports green on completely unguarded parameters: the first *guarded*
+sibling throws, the case is skipped, and every unguarded sibling is never exercised. Whole tools drop
+out of coverage the same way, and an aggregate count cannot detect it, because a case that never
+existed cannot fail. `x_manage_block.targetUserId` and `okta_remove_user_from_group.userId` were both
+fully unguarded while passing every vector a suite of that shape threw at them.
+
+A sound suite has five properties:
+
+1. **Enumerate (tool, param) pairs, fuzz one at a time.** Discover pairs by probing — build the URL
+   with a sentinel in one param and check whether it lands in `pathname` — and hold every sibling at a
+   safe value. Give *other* number params a real number so a sibling's own validation cannot abort the
+   build. Reference: `apps/sim/tools/rootly/path_safety.test.ts:151`,
+   `apps/sim/tools/github/path_safety.test.ts:171`.
+2. **Assert rejection, not path shape.** A shape assertion (origin + segment count + unchanged
+   segments) catches only a minority of the vectors: `%2F` is never decoded back into a separator, and
+   a trailing bare `.` collapses to the parent with the segment count intact. Add an explicit
+   `expect(() => build(param, '..')).toThrow(new RegExp(paramName))` — the error must *name the
+   parameter*. Reference: `apps/sim/tools/clerk/path_safety.test.ts:206`,
+   `apps/sim/tools/rootly/path_safety.test.ts:193`.
+3. **Probe conditional branches.** A param that only appears on one branch of a conditional URL builder
+   is invisible to a single all-params probe. Harvest the string literals the builder compares against
+   and re-probe under each (`apps/sim/tools/spotify/path_safety.test.ts:185`), **and** probe the
+   presence branches — the shape taken when an optional param is *absent*
+   (`apps/sim/tools/discord/path_safety.test.ts:203`).
+4. **Assert the skipped and unbuildable sets are empty.** Record every failed baseline build with its
+   reason and assert the ledger against an explicit, justified allowlist
+   (`apps/sim/tools/github/path_safety.test.ts:229`, `:260`, `:293`), plus a second assertion that no
+   URL-building tool sits outside the suite unaccounted for (`:301`). This is the check that surfaced a
+   tool missing from an entire suite.
+5. **Pin the legitimate values too.** `..foo`, `foo..`, `v1.2.3`, and a UUID must pass through
+   unaltered — a guard that over-rejects is its own bug.
+
+- [ ] A `path_safety.test.ts` exists for the service and enumerates (tool, param) pairs
+- [ ] Each pair asserts a *named* rejection of the bare `.` and `..` segments
+- [ ] Conditional and presence branches of every conditional URL builder are probed
+- [ ] The skipped/unbuildable ledger is asserted empty against a justified allowlist
+- [ ] Legitimate dot-bearing values pass through unchanged
+
+### These tests are type-checked by nothing
+
+`apps/sim/tsconfig.json` excludes `**/*.test.ts` and `**/*.test.tsx` from `include`, and
+`apps/sim/vitest.config.ts` declares no `typecheck` block — Vitest transpiles with esbuild and never
+type-checks. So `bun run type-check` will pass over a harness full of type errors, and a harness that
+silently narrows to `any` will still run.
+
+To type-check one, write a temporary tsconfig that extends `apps/sim/tsconfig.json`, drops the
+`**/*.test.ts` exclusion, and includes only the harness — then delete it. Do not commit it; the
+exclusion exists deliberately.
+
+## Step 10: Validate Error Handling
 
 - [ ] `transformResponse` checks for error conditions before accessing data
 - [ ] Error responses include meaningful messages (not just generic "failed")
 - [ ] HTTP error status codes are handled (check `response.ok` or status codes)
 
-## Step 10: Report and Fix
+## Step 11: Report and Fix
 
 ### Report Format
 
@@ -446,6 +509,10 @@ After fixing, confirm:
 - [ ] Regenerated deployment config when block/OAuth metadata changed and ran both catalog checks
 - [ ] Validated pagination consistency across tools and block
 - [ ] Validated memory load safety using `.agents/skills/memory-load-check/SKILL.md` when tools list/search/download/import/export/batch data
+- [ ] Validated path-traversal safety: url-path helpers at every path interpolation, and a
+      `path_safety.test.ts` that enumerates (tool, param) pairs, asserts named rejection, probes
+      conditional and presence branches, and asserts an empty skip ledger
+- [ ] Validated no param collides with a transport-reserved name (`timeout`, `proxyUrl`, `method`)
 - [ ] Validated error handling (error checks, meaningful messages)
 - [ ] Validated registry entries (tools and block, alphabetical, correct imports)
 - [ ] Validated model-visible/opaque inputs and Sim-durable/internal-execution provenance at their