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

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

@@ -550,6 +550,39 @@ 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 }),
+```
+
+**This mapper form only works when the renamed tool param is not both `required` and `user-only`.**
+`check-block-registry.ts:222` narrows to exactly that pair, and for those it demands a subBlock whose
+`id` or `canonicalParamId` equals the **tool param id** — a mapper that renames at execution time does
+not satisfy it, because the raw subBlock id is not a valid lookup key once a canonical group resolves
+(`check-block-registry.ts:201`). For a **required user-only** param the two rules cannot both be met by
+a mapper — keeping the tool param name requires sending the reserved key, and clearing it leaves a
+required input undefined — so the only correct move is to **rename the subBlock and record the old id
+in `SUBBLOCK_ID_MIGRATIONS`**, accepting the one-time state migration.
+
+`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 +640,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 +737,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 +1102,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)
+- [ ] Where a subBlock id collides with a reserved transport key but means something provider-specific, the reserved key is explicitly cleared with `undefined` in `tools.config.params` (leave it alone when the block genuinely means the transport's timeout/proxy/method)
 - [ ] 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,80 @@ 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`.
+
+**Check what your checkout exports before you start.** `safeUrlPathSegment` is on `staging`; the rest
+of this module — `safeUrlPath`, `safeEncodedUrlPathSegment`, `strictUrlPathSegment`,
+`strictEncodedUrlPathSegment` — and the `path_safety.test.ts` suites cited throughout arrive with the
+path-safety sweep. If a helper you need is absent, **add it to `apps/sim/tools/url-path.ts`** with the
+semantics this section specifies — rejection of a bare `.`/`..`, per-segment where the value is
+hierarchical — and never hand-roll a local encoder at the call site. Citations into those files name
+**module and symbol** rather than a line, because they are still being rebased.
+
+| Helper | Use when the parameter is | Trims? |
+|---|---|---|
+| `safeUrlPathSegment` | **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.
+
+GitHub is the worked example, and it settles the case that looks ambiguous: `branch`, `ref`, `base`,
+and `head` take **`safeUrlPath`**, not `safeEncodedUrlPathSegment`. `GET /repos/{o}/{r}/branches/{branch}`
+is greedy on its final parameter, so a branch named `feature/api` is addressed as
+`/branches/feature/api` with a real separator — emitting `%2F` there would 404. The `%2F` form is for
+a parameter the provider reads as one value *and* does not treat as greedy, such as a label name in
+`DELETE .../labels/{name}`. Read the provider's route, not the value's shape.
+
+### Never let a new guard rescue a request that used to fail
+
+If a parameter you are now routing through a helper previously went out raw or through a bare
+`encodeURIComponent`, the helper's trimming is a **behaviour change**, not a tightening: a padded
+value that used to 404 now names a real resource. On a DELETE, a cancel, or a revoke that is a
+destructive action the caller never asked for.
+
+For a newly-trimmed identifier on an irreversible request, use `strictUrlPathSegment` in
+`apps/sim/tools/url-path.ts` — `safeUrlPathSegment` plus a refusal of surrounding whitespace —
+rather than trimming (`strictEncodedUrlPathSegment` is its `%2F`-preserving counterpart). Identifiers
+that were already trimmed before your change keep plain `safeUrlPathSegment`. The full rule, its two confirmed instances, and how to scope the check are
+in **A hardening change must not turn a failing request into a succeeding one** in
+`.agents/skills/validate-integration/SKILL.md`.
+
+Note the matching asymmetry inside `safeUrlPath`: it rejects only a **truly empty** path component,
+never a whitespace-only one. Git tracks a file and a directory whose entire name is spaces, and the URL parser never removes `%20%20%20` the way it removes a dot segment — so
+rejecting it has no security value and breaks a legitimate path. `safeUrlPathSegment` still rejects an
+all-whitespace value, because it trims opaque ids first.
+
+### `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 +622,10 @@ 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
+- [ ] No newly-guarded parameter on a destructive request turns a previously failing call into a
+      succeeding one — those use `strictUrlPathSegment`, not `safeUrlPathSegment`
 - [ ] All nullable response fields use `?? null`
 - [ ] All optional outputs have `optional: true`
 - [ ] No raw JSON dumps in outputs