From 7cc9cc59036d0e7dec0d8a63c618cb14861d3b6f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 16:33:48 +0000 Subject: [PATCH] refactor(spec)!: retire connector.rateLimitConfig and the outbound rate-limit shape (#4911) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0049 enforce-or-remove: `ConnectorSchema.rateLimitConfig` declared an outbound throttle that no engine ever applied. The platform's only token bucket (runtime security/rate-limit.ts) is INBOUND; no connector provider reads the key and no seam exists that could. Removed rather than kept — the vocabulary returns with an implementation (#4834 / PR #4878 ruling). - `retiredKey()` tombstone on `ConnectorSchema.rateLimitConfig` (non-strict schema — a plain delete would be an ADR-0104 silent strip) - `ConnectorRateLimitConfigSchema`/`ConnectorRateLimitConfig` and the orphaned `RateLimitStrategySchema`/`RateLimitStrategy` removed with it - D2 conversion `connector-rate-limit-config-removed` (retiredFromLoadPath) + D3 chain step at major 17 - #4684's RENAMED_DEFS entry absorbed: rename-then-delete in the same unreleased major is a delete - baselines updated deliberately (manifest -2 defs, authorable -6 lines +2 [RETIRED], api-surface -4 exports); docs/spec-changes/upgrade guide regenerated Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ehu85kbvMcrNTUJjwxvLJ9 --- .../connector-rate-limit-config-removed.md | 64 ++++++ .../docs/references/integration/connector.mdx | 38 +--- docs/protocol-upgrade-guide.md | 3 + packages/spec/api-surface.json | 4 - packages/spec/authorable-surface.json | 10 +- packages/spec/docs/SYNC_ARCHITECTURE.md | 12 +- packages/spec/json-schema.manifest.json | 2 - .../scripts/build-schemas-check-mode.test.ts | 17 +- packages/spec/scripts/lib/renamed-defs.ts | 20 +- packages/spec/scripts/renamed-defs.test.ts | 29 ++- packages/spec/spec-changes.json | 12 ++ packages/spec/src/conversions/registry.ts | 84 ++++++++ .../spec/src/integration/connector.test.ts | 197 ++++++++++-------- .../spec/src/integration/connector.zod.ts | 125 +++++------ packages/spec/src/migrations/registry.ts | 19 +- 15 files changed, 409 insertions(+), 227 deletions(-) create mode 100644 .changeset/connector-rate-limit-config-removed.md diff --git a/.changeset/connector-rate-limit-config-removed.md b/.changeset/connector-rate-limit-config-removed.md new file mode 100644 index 0000000000..b5e5ec97e2 --- /dev/null +++ b/.changeset/connector-rate-limit-config-removed.md @@ -0,0 +1,64 @@ +--- +"@objectstack/spec": major +--- + +refactor(spec)!: remove `connector.rateLimitConfig` and the whole outbound rate-limit shape — the engine never existed (#4911, ADR-0049) + +`ConnectorSchema.rateLimitConfig` let an author declare an outbound throttle for +their connector — `strategy`, `maxRequests`, `windowSeconds`, `burstCapacity`, +`respectUpstreamLimits`, `rateLimitHeaders` — and nothing anywhere applied it. +This is not the ordinary declared-but-unread case; it is a step worse: +**there is no outbound rate-limiting engine to wire it to.** The only token +bucket the platform owns is `packages/runtime/src/security/rate-limit.ts`, and it +is INBOUND — the dispatcher calls `consume(key)` on a request fingerprint and +answers 429. No connector provider (`connector-rest`, `connector-openapi`, +`connector-mcp`, `connector-slack`) reads the key, and no seam exists that could. + +So a well-formed, schema-validated block told the author they had capped their +call rate against a third party's quota, and capped nothing — the false-compliance +class ADR-0049 exists for. With no implementation and no committed roadmap, +`experimental` would be a promise nobody made; **absent** is the honest +disposition. The vocabulary comes back *with* the engine, in one change +(implementation-first — the #4834 / PR #4878 ruling for the plugin-runtime family). + +FROM → TO: + +| Removed | Replacement | +| :--- | :--- | +| `connector.rateLimitConfig` (key) | **none** — delete it; throttle at the connector provider or upstream gateway | +| `ConnectorRateLimitConfigSchema` / `ConnectorRateLimitConfig` | **none** — importing either is TS2305 in v17 | +| `RateLimitStrategySchema` / `RateLimitStrategy` | **none** — the enum had no other consumer | + +**Do NOT substitute `shared`'s `RateLimitConfig`.** That is the INBOUND limiter +(`enabled` / `windowMs` / `maxRequests`) and caps the calls others make to *us* — +the opposite direction. #4684 split the two names for exactly this confusion; the +conversion deliberately does not rewrite one into the other, because that would +silently change behaviour rather than losing a no-op. + +The retirement kit: + +- **Tombstone.** `ConnectorSchema` is not `.strict()`, so a plain delete would be + a silent strip (ADR-0104). `retiredKey()` makes the removal audible in the two + channels an upgrading author hits — `tsc` (the key types `never`) and the parse + (the prescription itself). It reaches `stack.connectors[]` and + `DeclarativeConnectorEntry`, which is `ConnectorSchema.superRefine(…)`. +- **ADR-0087 D2 conversion + D3 chain step** (`connector-rate-limit-config-removed`, + `retiredFromLoadPath`): `os migrate meta --from 16` deletes the key from author + sources and stored rows replay clean. A lossless delete — the block never had an + effect to lose. +- **The shape goes with the key.** `ConnectorRateLimitConfigSchema` and the + `RateLimitStrategySchema` enum it embedded had no other consumer, and an + exported schema with no consumer reads as a capability to whoever finds it + (#3950). +- **#4684's rename is absorbed.** `integration/RateLimitConfig` → + `integration/ConnectorRateLimitConfig` and this retirement landed in the same + unreleased major; composed they are a plain delete, so the `RENAMED_DEFS` entry + is removed rather than pointing at a def this build no longer emits. +- Baselines updated deliberately: `json-schema.manifest.json` (−2 defs), + `authorable-surface.json` (−6 def lines; `Connector` / + `DeclarativeConnectorEntry` gain `… [RETIRED]`), `api-surface.json` (−4 + exports). `api-surface-signatures.json` is unchanged by construction — it hashes + each `defineX` parameter as TypeScript *prints* it, a reference + (`z.input`), so key-level narrowing never reaches it. + +No runtime behaviour changes — that impossibility is the reason for the removal. diff --git a/content/docs/references/integration/connector.mdx b/content/docs/references/integration/connector.mdx index 89b4fef6d5..db1b39f3a0 100644 --- a/content/docs/references/integration/connector.mdx +++ b/content/docs/references/integration/connector.mdx @@ -134,8 +134,8 @@ a dead end of the same class in #4738.) ## TypeScript Usage ```typescript -import { CircuitBreakerConfigSchema, ConnectorSchema, ConnectorActionSchema, ConnectorConflictResolutionSchema, ConnectorErrorCategorySchema, ConnectorFieldMappingSchema, ConnectorHealthSchema, ConnectorRateLimitConfigSchema, ConnectorRetryStrategySchema, ConnectorStatusSchema, ConnectorTriggerSchema, ConnectorTypeSchema, DataSyncConfigSchema, DeclarativeConnectorEntrySchema, ErrorMappingConfigSchema, ErrorMappingRuleSchema, HealthCheckConfigSchema, RateLimitStrategySchema, RetryConfigSchema, SyncStrategySchema, WebhookConfigSchema, WebhookEventSchema, WebhookSignatureAlgorithmSchema } from '@objectstack/spec/integration'; -import type { CircuitBreakerConfig, Connector, ConnectorConflictResolution, ConnectorErrorCategory, ConnectorFieldMapping, ConnectorHealth, ConnectorRateLimitConfig, ConnectorRetryStrategy, ConnectorStatus, ConnectorType, DataSyncConfig, DeclarativeConnectorEntry, ErrorMappingConfig, ErrorMappingRule, HealthCheckConfig, RateLimitStrategy, RetryConfig, SyncStrategy, WebhookConfig, WebhookEvent, WebhookSignatureAlgorithm } from '@objectstack/spec/integration'; +import { CircuitBreakerConfigSchema, ConnectorSchema, ConnectorActionSchema, ConnectorConflictResolutionSchema, ConnectorErrorCategorySchema, ConnectorFieldMappingSchema, ConnectorHealthSchema, ConnectorRetryStrategySchema, ConnectorStatusSchema, ConnectorTriggerSchema, ConnectorTypeSchema, DataSyncConfigSchema, DeclarativeConnectorEntrySchema, ErrorMappingConfigSchema, ErrorMappingRuleSchema, HealthCheckConfigSchema, RetryConfigSchema, SyncStrategySchema, WebhookConfigSchema, WebhookEventSchema, WebhookSignatureAlgorithmSchema } from '@objectstack/spec/integration'; +import type { CircuitBreakerConfig, Connector, ConnectorConflictResolution, ConnectorErrorCategory, ConnectorFieldMapping, ConnectorHealth, ConnectorRetryStrategy, ConnectorStatus, ConnectorType, DataSyncConfig, DeclarativeConnectorEntry, ErrorMappingConfig, ErrorMappingRule, HealthCheckConfig, RetryConfig, SyncStrategy, WebhookConfig, WebhookEvent, WebhookSignatureAlgorithm } from '@objectstack/spec/integration'; // Validate data const result = CircuitBreakerConfigSchema.parse(data); @@ -181,7 +181,7 @@ Circuit breaker configuration | **syncConfig** | `{ strategy?: Enum<'full' \| 'incremental' \| 'upsert' \| 'append_only'>; direction?: Enum<'import' \| 'export' \| 'bidirectional'>; schedule?: string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }; realtimeSync?: boolean; … }` | optional | Data sync configuration | | **fieldMappings** | `{ source: string; target: string; transform?: { type: 'constant'; value: any } \| { type: 'cast'; targetType: Enum<'string' \| 'number' \| 'boolean' \| 'date'> } \| { type: 'lookup'; table: string; keyField: string; valueField: string } \| { type: 'javascript'; expression: string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object } } \| { type: 'map'; mappings: Record }; defaultValue?: any; … }[]` | optional | Field mapping rules | | **webhooks** | `{ name: string; label?: string; object?: string; triggers?: Enum<'create' \| 'update' \| 'delete' \| 'bulk_update' \| 'bulk_delete'>[]; … }[]` | optional | Webhook configurations (not yet enforced — never read at registration; see #3197) | -| **rateLimitConfig** | `{ strategy?: Enum<'fixed_window' \| 'sliding_window' \| 'token_bucket' \| 'leaky_bucket'>; maxRequests: number; windowSeconds: number; burstCapacity?: number; … }` | optional | Rate limiting configuration | +| **rateLimitConfig** | `any` | optional | [REMOVED] `connector.rateLimitConfig` was removed in @objectstack/spec 17.0.0 (#4911, ADR-0049 D2) — the entire shape is gone, not just this key: `ConnectorRateLimitConfig` and its `RateLimitStrategy` enum were removed with it, because no outbound rate-limiting engine ever existed. The platform's only token bucket (runtime `security/rate-limit.ts`) throttles INBOUND requests to us; nothing throttled the calls a connector makes out, so every knob here was inert while reading like a configured cap. Delete the key. Do NOT substitute `shared` `RateLimitConfig` — that is the inbound limiter and would cap the wrong direction; until an outbound throttle exists, rate-limit at the connector provider or upstream gateway. Run `os migrate meta --from 16` to rewrite it automatically. | | **retryConfig** | `{ strategy?: Enum<'exponential_backoff' \| 'linear_backoff' \| 'fixed_delay' \| 'no_retry'>; maxAttempts?: number; initialDelayMs?: number; maxDelayMs?: number; … }` | optional | Retry configuration | | **connectionTimeoutMs** | `number` | optional | Connection timeout in ms | | **requestTimeoutMs** | `number` | optional | Request timeout in ms | @@ -270,22 +270,6 @@ Connector health configuration | **circuitBreaker** | `{ enabled: boolean; failureThreshold: number; resetTimeoutMs: number; halfOpenMaxRequests: number; … }` | optional | Circuit breaker configuration | ---- - -## ConnectorRateLimitConfig - -### Properties - -| Property | Type | Required | Description | -| :--- | :--- | :--- | :--- | -| **strategy** | `Enum<'fixed_window' \| 'sliding_window' \| 'token_bucket' \| 'leaky_bucket'>` | ✅ | Rate limiting strategy | -| **maxRequests** | `number` | ✅ | Maximum requests per window | -| **windowSeconds** | `number` | ✅ | Time window in seconds | -| **burstCapacity** | `number` | optional | Burst capacity | -| **respectUpstreamLimits** | `boolean` | ✅ | Respect external rate limit headers | -| **rateLimitHeaders** | `{ remaining: string; limit: string; reset: string }` | optional | Custom rate limit headers | - - --- ## ConnectorRetryStrategy @@ -386,7 +370,7 @@ Connector type | **syncConfig** | `{ strategy?: Enum<'full' \| 'incremental' \| 'upsert' \| 'append_only'>; direction?: Enum<'import' \| 'export' \| 'bidirectional'>; schedule?: string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }; realtimeSync?: boolean; … }` | optional | Data sync configuration | | **fieldMappings** | `{ source: string; target: string; transform?: { type: 'constant'; value: any } \| { type: 'cast'; targetType: Enum<'string' \| 'number' \| 'boolean' \| 'date'> } \| { type: 'lookup'; table: string; keyField: string; valueField: string } \| { type: 'javascript'; expression: string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object } } \| { type: 'map'; mappings: Record }; defaultValue?: any; … }[]` | optional | Field mapping rules | | **webhooks** | `{ name: string; label?: string; object?: string; triggers?: Enum<'create' \| 'update' \| 'delete' \| 'bulk_update' \| 'bulk_delete'>[]; … }[]` | optional | Webhook configurations (not yet enforced — never read at registration; see #3197) | -| **rateLimitConfig** | `{ strategy?: Enum<'fixed_window' \| 'sliding_window' \| 'token_bucket' \| 'leaky_bucket'>; maxRequests: number; windowSeconds: number; burstCapacity?: number; … }` | optional | Rate limiting configuration | +| **rateLimitConfig** | `any` | optional | [REMOVED] `connector.rateLimitConfig` was removed in @objectstack/spec 17.0.0 (#4911, ADR-0049 D2) — the entire shape is gone, not just this key: `ConnectorRateLimitConfig` and its `RateLimitStrategy` enum were removed with it, because no outbound rate-limiting engine ever existed. The platform's only token bucket (runtime `security/rate-limit.ts`) throttles INBOUND requests to us; nothing throttled the calls a connector makes out, so every knob here was inert while reading like a configured cap. Delete the key. Do NOT substitute `shared` `RateLimitConfig` — that is the inbound limiter and would cap the wrong direction; until an outbound throttle exists, rate-limit at the connector provider or upstream gateway. Run `os migrate meta --from 16` to rewrite it automatically. | | **retryConfig** | `{ strategy?: Enum<'exponential_backoff' \| 'linear_backoff' \| 'fixed_delay' \| 'no_retry'>; maxAttempts?: number; initialDelayMs?: number; maxDelayMs?: number; … }` | optional | Retry configuration | | **connectionTimeoutMs** | `number` | optional | Connection timeout in ms | | **requestTimeoutMs** | `number` | optional | Request timeout in ms | @@ -452,20 +436,6 @@ Health check configuration | **healthyThreshold** | `number` | ✅ | Consecutive successes before marking healthy | ---- - -## RateLimitStrategy - -Rate limiting strategy - -### Allowed Values - -* `fixed_window` -* `sliding_window` -* `token_bucket` -* `leaky_bucket` - - --- ## RetryConfig diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 2c7ead56ee..3f368fc411 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -188,6 +188,8 @@ That kernel-side tombstone was then SUPERSEDED inside the same unreleased major Finally it removes the script-body capability token 'crypto.hash' (#4391). Four layers declared it — the `HookBodyCapability` enum, the doc table beside it, the CLI extractor and `ScriptContext.crypto.hash` — and none implemented it: `installCtx` wired only `randomUUID`, so the one call the token authorised threw inside the VM every time. The build-time inference made it worse than an ordinary declared-but-unenforced key: writing `ctx.crypto.hash(...)` made the CLI ADD the capability for you, so `os build` went green on the body that was guaranteed to fail at the first record write. Removed rather than implemented (ADR-0049) — hashing inside the sandbox widens its capability and security-review surface, and a capability that throws on every use yet drew zero complaints in its whole life is its own liveness verdict. This is an enum VALUE, not a key, so there is no `retiredKey()` tombstone: the enum error map carries the prescription, keyed on the received value so that only the spelling which used to be legal is told it "was removed". The conversion strips the dead token from `body.capabilities` on hooks and actions; it deliberately does NOT touch the `ctx.crypto.hash(...)` call the body made under it, which never returned a value and which the author must delete. Hashing returns only WITH an implementation, through the capability admission process. +It also removes `connector.rateLimitConfig` and its whole shape (#4911). This one is not "declared but unread" — it is declared but UNIMPLEMENTED, one step worse. The only token bucket the platform owns (runtime `security/rate-limit.ts`) is INBOUND: the dispatcher calls `consume(key)` on a request fingerprint and answers 429. Nothing anywhere throttles the calls a connector makes OUT, and no provider — `connector-rest`, `connector-openapi`, `connector-mcp`, `connector-slack` — reads the key or has a seam that could. So `strategy`, `maxRequests`, `windowSeconds`, `burstCapacity`, `respectUpstreamLimits` and `rateLimitHeaders` parsed cleanly and capped nothing, on a surface where the author believed they had bounded their spend against a third party's quota. `ConnectorRateLimitConfig` and the `RateLimitStrategy` enum it embedded had no other consumer and are removed with the key, so importing either is TS2305 in v17 — the #4834 shape, and the same implementation-first ruling: the vocabulary comes back WITH the engine, in one change. It is deliberately NOT converted to `shared` `RateLimitConfig`, which limits the calls others make to US; #4684 split their names for precisely this confusion, and rewriting an outbound cap into an inbound one would throttle the wrong direction. Delete the key and rate-limit where the calls are actually made — the connector provider or upstream gateway. + ### Mechanical (applied for you) | Conversion | Surface | Change | Load window | @@ -229,6 +231,7 @@ Finally it removes the script-body capability token 'crypto.hash' (#4391). Four | `object-managed-by-system-to-system-data` | `object.managedBy` | object managedBy 'system' → 'system-data' (#3355 — ADR-0103's residual bucket named the engine-owned half v16 had already moved out to `engine-owned`; the rename leaves the name describing what the bucket actually holds: admin/user-writable platform data) | retired — `migrate meta` only | | `object-enable-trash-mru-removed` | `object.enable.trash / object.enable.mru` | object capability flags 'enable.trash'/'enable.mru' removed (#3207, #2377 close-out — no recycle bin and no MRU tracking ever ran; both default-true flags gated nothing) | retired — `migrate meta` only | | `hook-body-crypto-hash-removed` | `hook.body.capabilities / action.body.capabilities` | script-body capability token 'crypto.hash' removed (#4391 — the sandbox never installed ctx.crypto.hash, so the token granted a call that always threw; the CLI inferred it too) | retired — `migrate meta` only | +| `connector-rate-limit-config-removed` | `connector.rateLimitConfig` | connector key 'rateLimitConfig' removed (#4911 — no outbound rate-limiting engine exists; the runtime's only token bucket limits INBOUND requests, so every knob here was inert while reading like a configured cap. The whole ConnectorRateLimitConfig shape went with it) | retired — `migrate meta` only | ### Semantic (delegated to you, with acceptance criteria) diff --git a/packages/spec/api-surface.json b/packages/spec/api-surface.json index af816023f9..3217d7fab0 100644 --- a/packages/spec/api-surface.json +++ b/packages/spec/api-surface.json @@ -3865,8 +3865,6 @@ "ConnectorOrigin (type)", "ConnectorProviderContext (interface)", "ConnectorProviderFactory (type)", - "ConnectorRateLimitConfig (type)", - "ConnectorRateLimitConfigSchema (const)", "ConnectorRetryStrategy (type)", "ConnectorRetryStrategySchema (const)", "ConnectorSchema (const)", @@ -3887,8 +3885,6 @@ "ErrorMappingRuleSchema (const)", "HealthCheckConfig (type)", "HealthCheckConfigSchema (const)", - "RateLimitStrategy (type)", - "RateLimitStrategySchema (const)", "ResolvedConnectorAuth (type)", "RetryConfig (type)", "RetryConfigSchema (const)", diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index e1e005ce6b..64a1191624 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -4073,7 +4073,7 @@ "integration/Connector:name", "integration/Connector:provider", "integration/Connector:providerConfig", - "integration/Connector:rateLimitConfig", + "integration/Connector:rateLimitConfig [RETIRED]", "integration/Connector:requestTimeoutMs", "integration/Connector:retryConfig", "integration/Connector:status", @@ -4105,12 +4105,6 @@ "integration/ConnectorInstanceBearerAuth:credentialRef", "integration/ConnectorInstanceBearerAuth:type", "integration/ConnectorInstanceNoAuth:type", - "integration/ConnectorRateLimitConfig:burstCapacity", - "integration/ConnectorRateLimitConfig:maxRequests", - "integration/ConnectorRateLimitConfig:rateLimitHeaders", - "integration/ConnectorRateLimitConfig:respectUpstreamLimits", - "integration/ConnectorRateLimitConfig:strategy", - "integration/ConnectorRateLimitConfig:windowSeconds", "integration/ConnectorTrigger:description", "integration/ConnectorTrigger:interval", "integration/ConnectorTrigger:key", @@ -4140,7 +4134,7 @@ "integration/DeclarativeConnectorEntry:name", "integration/DeclarativeConnectorEntry:provider", "integration/DeclarativeConnectorEntry:providerConfig", - "integration/DeclarativeConnectorEntry:rateLimitConfig", + "integration/DeclarativeConnectorEntry:rateLimitConfig [RETIRED]", "integration/DeclarativeConnectorEntry:requestTimeoutMs", "integration/DeclarativeConnectorEntry:retryConfig", "integration/DeclarativeConnectorEntry:status", diff --git a/packages/spec/docs/SYNC_ARCHITECTURE.md b/packages/spec/docs/SYNC_ARCHITECTURE.md index 80387680fd..1f0420e3d9 100644 --- a/packages/spec/docs/SYNC_ARCHITECTURE.md +++ b/packages/spec/docs/SYNC_ARCHITECTURE.md @@ -264,14 +264,8 @@ const sapConnector: Connector = { } ], - // Rate Limiting - rateLimitConfig: { - strategy: 'token_bucket', - maxRequests: 100, - windowSeconds: 60, - burstCapacity: 150, - respectUpstreamLimits: true - }, + // (`rateLimitConfig` sat here until #4911 retired it — no outbound + // rate-limiting engine ever existed. Throttle at the provider/gateway.) // Retry Configuration retryConfig: { @@ -404,7 +398,7 @@ const pipeline: ETLPipeline = { const connector: Connector = { authentication: { type: 'oauth2', ... }, webhooks: [...], - rateLimitConfig: { ... } + retryConfig: { ... } }; ``` diff --git a/packages/spec/json-schema.manifest.json b/packages/spec/json-schema.manifest.json index 51571b27f9..88c49721ab 100644 --- a/packages/spec/json-schema.manifest.json +++ b/packages/spec/json-schema.manifest.json @@ -867,7 +867,6 @@ "integration/ConnectorInstanceBasicAuth", "integration/ConnectorInstanceBearerAuth", "integration/ConnectorInstanceNoAuth", - "integration/ConnectorRateLimitConfig", "integration/ConnectorRetryStrategy", "integration/ConnectorStatus", "integration/ConnectorTrigger", @@ -877,7 +876,6 @@ "integration/ErrorMappingConfig", "integration/ErrorMappingRule", "integration/HealthCheckConfig", - "integration/RateLimitStrategy", "integration/RetryConfig", "integration/SyncStrategy", "integration/WebhookConfig", diff --git a/packages/spec/scripts/build-schemas-check-mode.test.ts b/packages/spec/scripts/build-schemas-check-mode.test.ts index 35310acdf6..3f7e90dec6 100644 --- a/packages/spec/scripts/build-schemas-check-mode.test.ts +++ b/packages/spec/scripts/build-schemas-check-mode.test.ts @@ -324,8 +324,14 @@ const DELETED_GONE_DEF = ['identity/Session:userId', 'identity/Session:token']; * ≥ 2 majors behind any current major, so this fixture never goes stale. */ const DELETED_AGED_LEAF = 'compactLayout'; const DELETED_AGED = `data/Object:${DELETED_AGED_LEAF} [RETIRED]`; -/** Base key under a def RENAMED_DEFS moved: carried, so never a deletion. */ -const DELETED_BY_RENAME = 'integration/RateLimitConfig:maxRequests'; +/** Base key under a def RENAMED_DEFS moved: carried, so never a deletion. + * Was `integration/RateLimitConfig:maxRequests` until #4911 retired that def + * outright and its rename entry was absorbed (a rename whose target stops + * being emitted cannot stay in the table). Re-pointed at the #4703 rename, + * which carries 7 keys — an ENUM rename (0 keys carried) would make this + * fixture vacuous. */ +const DELETED_BY_RENAME_SOURCE_DEF = 'integration/FieldMapping'; +const DELETED_BY_RENAME = `${DELETED_BY_RENAME_SOURCE_DEF}:source`; describe('build-schemas.ts — deleted baseline lines must prove themselves (#4650)', () => { beforeAll(() => { @@ -501,7 +507,12 @@ describe('build-schemas.ts — deleted baseline lines must prove themselves (#46 'a declared def rename is not a deletion: base keys are carried through RENAMED_DEFS before comparing', { timeout: SPAWN_TIMEOUT_MS }, () => { - expect(Object.keys(RENAMED_DEFS)).toContain('integration/RateLimitConfig'); + expect(Object.keys(RENAMED_DEFS)).toContain(DELETED_BY_RENAME_SOURCE_DEF); + // The carried key must actually land under the NEW def, or this asserts + // nothing — a rename entry whose target lost the key is check (a0)'s case. + expect((JSON.parse(pristineSurface) as { keys: string[] }).keys).toContain( + `${RENAMED_DEFS[DELETED_BY_RENAME_SOURCE_DEF]}:source`, + ); seedBase((s) => [...s, DELETED_BY_RENAME].sort()); seedSurface((s) => s); diff --git a/packages/spec/scripts/lib/renamed-defs.ts b/packages/spec/scripts/lib/renamed-defs.ts index 976e02b57b..924a18c344 100644 --- a/packages/spec/scripts/lib/renamed-defs.ts +++ b/packages/spec/scripts/lib/renamed-defs.ts @@ -54,9 +54,23 @@ * only when the old name has aged out — same discipline as a tombstone. */ export const RENAMED_DEFS: Readonly> = { - // #4684 / ADR-0112 D9a — the connector-side (outbound throttling) config no - // longer shares a name with `shared/RateLimitConfig` (inbound API limiting). - 'integration/RateLimitConfig': 'integration/ConnectorRateLimitConfig', + // ─── ABSORBED, do not re-add: `integration/RateLimitConfig` ──────────── + // + // #4684 / ADR-0112 D9a renamed the connector-side (outbound throttling) def + // to `integration/ConnectorRateLimitConfig` so it no longer shared a name + // with `shared/RateLimitConfig` (inbound API limiting). #4911 then RETIRED + // that def outright — no outbound rate-limiting engine ever existed + // (ADR-0049) — inside the SAME unreleased major. + // + // Composed, rename-then-delete is just a delete, and the entry could not + // survive either way: `checkRenameTable` rejects a target this build no + // longer emits, which is precisely the decay guard that made this table + // trustworthy. The removal is carried by the real retirement kit instead — + // a `retiredKey()` tombstone on `ConnectorSchema.rateLimitConfig`, the + // `connector-rate-limit-config-removed` D2 conversion, and the deliberate + // manifest deletion of both `integration/ConnectorRateLimitConfig` and + // `integration/RateLimitStrategy`. A retirement must never ride this table + // (same ruling as `automation/ConflictResolution` below). // #4703 / ADR-0112 D9a — `FieldMapping` was published by THREE defs at once. // The two domain-specific sides take a domain prefix; `shared/FieldMapping` diff --git a/packages/spec/scripts/renamed-defs.test.ts b/packages/spec/scripts/renamed-defs.test.ts index 205ce301c7..265a100d9a 100644 --- a/packages/spec/scripts/renamed-defs.test.ts +++ b/packages/spec/scripts/renamed-defs.test.ts @@ -138,15 +138,26 @@ describe('checkRenameTable', () => { }); describe('the committed RENAMED_DEFS table', () => { - it('records the #4684 connector rate-limit rename', () => { - expect(RENAMED_DEFS['integration/RateLimitConfig']).toBe( - 'integration/ConnectorRateLimitConfig', - ); - }); - - it('leaves the shared (inbound) declaration alone — only the connector side moved', () => { - // ADR-0112 D9a renames the CONNECTOR side so one name means one thing; - // `shared/RateLimitConfig` is the incumbent and keeps its name and its keys. + it('no longer carries the #4684 connector rate-limit rename — #4911 absorbed it', () => { + // The #4684 rename (`integration/RateLimitConfig` → + // `integration/ConnectorRateLimitConfig`) and the #4911 retirement of the + // renamed def landed in the SAME unreleased major, so composed they are a + // plain delete. Two independent reasons the entry must be gone: + // 1. `checkRenameTable` rejects a target this build no longer emits — + // leaving it would fail `gen:schema` before either ratchet runs; + // 2. a retirement must never ride this table (see the `automation/ + // ConflictResolution` note in lib/renamed-defs.ts) — it is carried by + // the tombstone + D2 conversion + deliberate manifest deletion. + // Re-adding it would claim the def still exists under a new name. + expect(RENAMED_DEFS['integration/RateLimitConfig']).toBeUndefined(); + expect(Object.values(RENAMED_DEFS)).not.toContain('integration/ConnectorRateLimitConfig'); + }); + + it('leaves the shared (inbound) declaration alone — only the connector side ever moved', () => { + // ADR-0112 D9a renamed the CONNECTOR side so one name means one thing; + // `shared/RateLimitConfig` is the incumbent, keeps its name and its keys, + // and #4911 (which retired the connector side) did not touch it — the + // inbound limiter is a real, enforced engine. expect(RENAMED_DEFS['shared/RateLimitConfig']).toBeUndefined(); }); diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index eed53d0e6a..12afec70d3 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -289,6 +289,12 @@ "to": "script-body capability token 'crypto.hash' removed (#4391 — the sandbox never installed ctx.crypto.hash, so the token granted a call that always threw; the CLI inferred it too)", "conversionId": "hook-body-crypto-hash-removed", "toMajor": 17 + }, + { + "surface": "connector.rateLimitConfig", + "to": "connector key 'rateLimitConfig' removed (#4911 — no outbound rate-limiting engine exists; the runtime's only token bucket limits INBOUND requests, so every knob here was inert while reading like a configured cap. The whole ConnectorRateLimitConfig shape went with it)", + "conversionId": "connector-rate-limit-config-removed", + "toMajor": 17 } ], "migrated": [ @@ -919,6 +925,12 @@ "to": "script-body capability token 'crypto.hash' removed (#4391 — the sandbox never installed ctx.crypto.hash, so the token granted a call that always threw; the CLI inferred it too)", "conversionId": "hook-body-crypto-hash-removed", "toMajor": 17 + }, + { + "surface": "connector.rateLimitConfig", + "to": "connector key 'rateLimitConfig' removed (#4911 — no outbound rate-limiting engine exists; the runtime's only token bucket limits INBOUND requests, so every knob here was inert while reading like a configured cap. The whole ConnectorRateLimitConfig shape went with it)", + "conversionId": "connector-rate-limit-config-removed", + "toMajor": 17 } ], "migrated": [ diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 1211939df3..bcd1d5d300 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -3558,6 +3558,89 @@ const hookBodyCryptoHashRemoved: MetadataConversion = { }, }; +/** + * `connector.rateLimitConfig` — OUTBOUND throttling for an engine that does not + * exist (#4911, ADR-0049). + * + * Not "declared but unread" — *declared but unimplemented*, which is a step + * worse. The platform's only token bucket is `packages/runtime/src/security/ + * rate-limit.ts`, and it is INBOUND: the dispatcher calls `consume(key)` on a + * request fingerprint and short-circuits with 429. There is no counterpart on + * the way out; no connector provider (`connector-rest`, `connector-openapi`, + * `connector-mcp`, `connector-slack`) reads the key, and no seam exists that + * could. So `strategy` / `maxRequests` / `windowSeconds` / `burstCapacity` / + * `respectUpstreamLimits` / `rateLimitHeaders` were six precise knobs an author + * could set — and would then believe capped their outbound call rate against a + * third-party API's quota. That belief is the defect: this is the false- + * compliance class ADR-0049 exists for, not cosmetic debt. + * + * The whole SHAPE goes, not just the key: `ConnectorRateLimitConfigSchema` and + * the `RateLimitStrategySchema` enum it embedded had no other consumer, and an + * exported schema with no consumer reads as a capability to whoever finds it + * (#3950). The vocabulary returns *with* an implementation, in one change — + * implementation-first, the ruling #4834 / PR #4878 set for the plugin-runtime + * family. + * + * Deliberately NOT converted to `shared/RateLimitConfig` (inbound API limiting, + * `windowMs`), the obvious-looking target: it limits the calls *others* make to + * us. Rewriting an outbound cap into an inbound one would throttle the wrong + * direction — a migration that silently changes behaviour, well outside D2's + * lossless scope. #4684 split their NAMES for this exact confusion; the honest + * conversion is a delete plus a prescription that says where throttling really + * lives (the connector provider / upstream gateway). + * + * `retiredFromLoadPath`: the key lies rather than being misspelled — the + * `flow-node-wait-timeout-keys-removed` distinction. Silently absorbing it at + * load would let the author keep believing they had configured a cap. The entry + * exists so stored 16.x/17-rc rows replay clean + * (`applyConversionsToStoredItem`) and `os migrate meta --from 16` rewrites + * author sources; live parses hit the `retiredKey()` tombstone instead. + */ +const connectorRateLimitConfigRemoved: MetadataConversion = { + id: 'connector-rate-limit-config-removed', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'connector.rateLimitConfig', + summary: + "connector key 'rateLimitConfig' removed (#4911 — no outbound rate-limiting engine exists; " + + "the runtime's only token bucket limits INBOUND requests, so every knob here was inert " + + 'while reading like a configured cap. The whole ConnectorRateLimitConfig shape went with it)', + apply(stack, emit) { + return mapCollection(stack, 'connectors', (c, path) => + stripKeys(c, ['rateLimitConfig'], emit, path)); + }, + fixture: { + before: { + connectors: [ + { + name: 'billing_api', + label: 'Billing API', + type: 'api', + rateLimitConfig: { + strategy: 'token_bucket', + maxRequests: 100, + windowSeconds: 60, + burstCapacity: 150, + respectUpstreamLimits: true, + }, + }, + // A connector that never authored the key keeps its identity — the + // copy-on-write contract `stripKeys` / `mapCollection` are built on. + { name: 'crm_sync', label: 'CRM Sync', type: 'saas' }, + ], + }, + // One notice per connector, not per knob: the block is what was removed. + // `retryConfig` and the timeouts beside it are untouched — they are live. + after: { + connectors: [ + { name: 'billing_api', label: 'Billing API', type: 'api' }, + { name: 'crm_sync', label: 'CRM Sync', type: 'saas' }, + ], + }, + expectedNotices: 1, + }, +}; + export const CONVERSIONS_BY_MAJOR: Readonly> = { 11: [flowNodeHttpRename, pageKindJsxToHtml, flowNodeFilterAlias, objectCompactLayoutRename], 13: [stackRolesToPositions, owdLegacyReadAliases, sharingRecipientRoleToPosition], @@ -3603,6 +3686,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly { }); // ============================================================================ -// Rate Limiting & Retry Tests +// Retry Tests // ============================================================================ - -describe('ConnectorRateLimitConfigSchema', () => { - it('should accept valid rate limit configuration', () => { - const config = { - strategy: 'token_bucket', - maxRequests: 100, - windowSeconds: 60, - burstCapacity: 150, - respectUpstreamLimits: true, - }; - - expect(() => ConnectorRateLimitConfigSchema.parse(config)).not.toThrow(); - }); - - it('should use default values', () => { - const config = { - maxRequests: 100, - windowSeconds: 60, - }; - - const parsed = ConnectorRateLimitConfigSchema.parse(config); - expect(parsed.strategy).toBe('token_bucket'); - expect(parsed.respectUpstreamLimits).toBe(true); - }); -}); +// (`ConnectorRateLimitConfigSchema` tests lived here until #4911 retired the +// whole shape — no outbound rate-limiting engine ever existed. The retirement +// is pinned in the "[#4911]" block at the bottom of this file.) describe('RetryConfigSchema', () => { it('should accept valid retry configuration', () => { @@ -411,10 +388,7 @@ describe('ConnectorSchema', () => { events: ['record.created'], }, ], - rateLimitConfig: { - maxRequests: 100, - windowSeconds: 60, - }, + // `rateLimitConfig` was authored here until #4911 retired it. retryConfig: { maxAttempts: 3, }, @@ -667,69 +641,124 @@ describe('ConnectorHealthSchema', () => { }); }); -// ─── [#4684] Dual-source regression pin ────────────────────────────── +// ─── [#4911] Outbound rate limiting retired — with the [#4684] pin folded in ── // // RUNTIME + compiler-API assertions, deliberately. #4642 established that a // compile-time pin in `packages/spec` is a no-op: `tsconfig.json` excludes // `**/*.test.ts` and `vitest.config.ts` never enables `typecheck`, so an -// `Assert< Equal< … > >` here would be dead text. The third test below is the -// only shape in this repo that actually pins a TYPE. +// `Assert< Equal< … > >` here would be dead text. The last test below is the +// only shape in this repo that actually pins a TYPE — which is what makes it the +// load-bearing one here: `ConnectorRateLimitConfig` is a TYPE, erased before any +// runtime assertion can see it, so re-adding `export type ConnectorRateLimitConfig` +// would slip past every `in`-check. +// +// History, because it explains what these now defend. #4684 found `RateLimitConfig` +// naming TWO declarations: `./shared` limits INBOUND API traffic (`enabled` / +// `windowMs` / `maxRequests`, every key defaulted) while `./integration` claimed to +// throttle OUTBOUND connector calls (`strategy` / `maxRequests` / `windowSeconds`, +// plus upstream `X-RateLimit-*` header names). Neither schema is `.strict()`, so a +// snippet copied across parsed clean with its foreign keys silently stripped +// (ADR-0104). #4684's remedy was ADR-0112 D9a's prefix. // -// What these defend: `RateLimitConfig` naming exactly ONE declaration across -// the published entries. `./shared` limits INBOUND API traffic (`enabled` / -// `windowMs` / `maxRequests`, every key defaulted); `./integration` throttles -// OUTBOUND connector calls (`strategy` / `maxRequests` / `windowSeconds` -// required, plus the upstream `X-RateLimit-*` header names). Neither is a -// superset of the other and neither schema is `.strict()`, so before #4684 a -// snippet copied from one side to the other PARSED CLEAN with its foreign keys -// silently stripped — ADR-0104's silent-strip class, in the export surface -// rather than in stored metadata. ADR-0112 D9a's remedy applies: the -// connector-side name gets a `Connector` prefix so one name means one thing. -describe('[#4684] RateLimitConfig no longer names two declarations', () => { - it('./integration exposes the connector shape only under its prefixed name', async () => { +// #4911 then found the deeper defect: the outbound side had no ENGINE. The only +// token bucket in the platform is `runtime/src/security/rate-limit.ts`, and it +// serves the INBOUND dispatcher. So the connector-side shape is gone entirely +// (ADR-0049 enforce-or-remove), the key is tombstoned, and what these tests pin +// is now an ASYMMETRY that must not be "tidied up": one direction has a real +// implementation and keeps its schema, the other does not and has none. The +// tempting wrong fix — pointing `connector.rateLimitConfig` at the shared inbound +// schema — would throttle the opposite direction, so the absence assertions below +// are as load-bearing as the presence ones. +describe('[#4911] `./integration` no longer publishes an outbound rate-limit shape', () => { + it('every retired name is absent from the entry — no alias, no re-export', async () => { const integrationEntry = await import('./index'); - expect(integrationEntry.ConnectorRateLimitConfigSchema).toBeDefined(); - // The bare name must be gone from this entry — an alias re-export kept "for - // compatibility" would be a THIRD declaration of the same name and would - // re-open the trap this issue closed. + // The retired shape and its orphaned enum. `in` (not `=== undefined`) so an + // explicitly-`undefined` re-export would still fail. + expect('ConnectorRateLimitConfigSchema' in integrationEntry).toBe(false); + expect('RateLimitStrategySchema' in integrationEntry).toBe(false); + // And the pre-#4684 bare name stays gone: re-adding it would re-open the + // dual-source trap on top of un-retiring the shape. expect('RateLimitConfigSchema' in integrationEntry).toBe(false); + + // Guard against a vacuous pass — if `./index` ever stopped resolving, every + // `in` above would be false for the wrong reason (#4642). + expect(integrationEntry.ConnectorSchema).toBeDefined(); + expect(integrationEntry.RetryConfigSchema).toBeDefined(); }); - it('./shared keeps the inbound declaration untouched', async () => { + it('authoring `rateLimitConfig` is rejected with the prescription, not silently stripped', async () => { + const { ConnectorSchema: Connector } = await import('./index'); + + const authored = { + name: 'billing_api', + label: 'Billing API', + type: 'api', + rateLimitConfig: { maxRequests: 100, windowSeconds: 60 }, + }; + + // `ConnectorSchema` is NOT `.strict()`, so a plain delete would have parsed + // clean and dropped the key — the exact ADR-0104 failure the tombstone exists + // to prevent. The message must carry the fix, not just "invalid". + expect(() => Connector.parse(authored)).toThrow(/rateLimitConfig.*removed.*Delete the key/s); + const failed = Connector.safeParse(authored); + expect(failed.success).toBe(false); + // It must not point authors at the INBOUND limiter as a replacement. + expect(JSON.stringify(failed.error)).toMatch(/wrong direction/); + + // A connector without the key is untouched: the tombstone rejects a VALUE, + // it does not make the key required. + const clean = Connector.parse({ name: 'billing_api', label: 'Billing API', type: 'api' }); + expect(clean).not.toHaveProperty('rateLimitConfig'); + }); + + it('the same rejection reaches `connectors[]` in a stack — the real authoring path', async () => { + const { ObjectStackSchema } = await import('../stack.zod'); + + const connector = { name: 'billing_api', label: 'Billing API', type: 'api' } as const; + + // Reachability, demonstrated rather than asserted from prose: the tombstone + // is only worth anything if it fires through `stack.connectors[]`, which is + // the surface an author actually writes (`DeclarativeConnectorEntrySchema` + // is `ConnectorSchema.superRefine(…)`, so it inherits the tombstone). + const rejected = ObjectStackSchema.safeParse({ + name: 'acme', + connectors: [{ ...connector, rateLimitConfig: { maxRequests: 1 } }], + }); + expect(rejected.success).toBe(false); + expect(JSON.stringify(rejected.error)).toMatch(/rateLimitConfig.*was removed/); + + // Positive control: the identical stack minus the retired key parses, so the + // failure above is attributable to `rateLimitConfig` and nothing else. + expect(ObjectStackSchema.safeParse({ name: 'acme', connectors: [connector] }).success) + .toBe(true); + }); + + it('./shared keeps the INBOUND declaration untouched — a real engine backs it', async () => { const sharedEntry = await import('../shared/index'); - // Same object identity as before the rename, same three defaulted keys. + // Same object identity and same three defaulted keys as before #4684/#4911. + // The asymmetry is the point: this one is enforced by the dispatcher's token + // bucket, so it stays while the outbound side goes. expect(sharedEntry.RateLimitConfigSchema.parse({})).toEqual({ enabled: false, windowMs: 60000, maxRequests: 100, }); - }); - - it('the two schemas remain distinct declarations that reject each other’s shape', async () => { - const integrationEntry = await import('./index'); - const sharedEntry = await import('../shared/index'); - - expect(integrationEntry.ConnectorRateLimitConfigSchema).not.toBe( - sharedEntry.RateLimitConfigSchema, - ); - - // The outbound shape demands what the inbound shape defaults away. - expect(integrationEntry.ConnectorRateLimitConfigSchema.safeParse({}).success).toBe(false); - // And the inbound shape still strips outbound keys — which is exactly why - // the two must not answer to one name. Pinned, not fixed: it is correct - // behaviour for a non-strict schema; the defect was the shared NAME. + // It still strips outbound-shaped keys — pinned, not fixed: correct + // behaviour for a non-strict schema, and the reason it is NOT the + // replacement for the retired key. expect(sharedEntry.RateLimitConfigSchema.parse({ windowSeconds: 60, strategy: 'token_bucket' })) .toEqual({ enabled: false, windowMs: 60000, maxRequests: 100 }); }); - // The load-bearing one. `RateLimitConfig` / `ConnectorRateLimitConfig` are - // TYPES — erased before any runtime assertion can see them — so every test - // above would stay green if `./integration` re-added `export type - // RateLimitConfig = z.infer<…>`, which is precisely the defect. This resolves - // each entry's exports through their alias chains to the ORIGINAL - // declaration: the same symbol-identity measurement + // The load-bearing one. `RateLimitConfig` / `ConnectorRateLimitConfig` / + // `RateLimitStrategy` are TYPES — erased before any runtime assertion can see + // them — so every test above would stay green if `./integration` re-added + // `export type ConnectorRateLimitConfig = z.infer<…>`, which un-retires the + // shape for every TypeScript author while the runtime namespace looks clean. + // This resolves each entry's exports through their alias chains to the + // ORIGINAL declaration: the same symbol-identity measurement // `check:dual-source-exports` makes, but over `src/` so it runs in `pnpm test` // without a build. it('no name resolves to two declarations across ./shared and ./integration (types included)', async () => { @@ -781,16 +810,20 @@ describe('[#4684] RateLimitConfig no longer names two declarations', () => { const shared = originsByEntry.get('./shared')!; const integration = originsByEntry.get('./integration')!; - // The names this issue is about: present on the right entry, absent from - // the wrong one, and each resolving to its own file. - expect(integration.get('ConnectorRateLimitConfig')).toMatch( - /^src\/integration\/connector\.zod\.ts:\d+$/, - ); - expect(integration.get('ConnectorRateLimitConfigSchema')).toMatch( - /^src\/integration\/connector\.zod\.ts:\d+$/, - ); + // The names this issue is about. TYPE-level absence of the retired shape — + // the assertion the runtime `in`-checks above structurally cannot make. + expect(integration.get('ConnectorRateLimitConfig')).toBeUndefined(); + expect(integration.get('ConnectorRateLimitConfigSchema')).toBeUndefined(); + expect(integration.get('RateLimitStrategy')).toBeUndefined(); + expect(integration.get('RateLimitStrategySchema')).toBeUndefined(); + // …and the pre-#4684 bare names stay off this entry too. expect(integration.get('RateLimitConfig')).toBeUndefined(); expect(integration.get('RateLimitConfigSchema')).toBeUndefined(); + // Positive control for the four `toBeUndefined()`s above: a live neighbour + // in the same file must still resolve, or they would pass vacuously. + expect(integration.get('RetryConfigSchema')).toMatch( + /^src\/integration\/connector\.zod\.ts:\d+$/, + ); expect(shared.get('RateLimitConfig')).toMatch(/^src\/shared\/http\.zod\.ts:\d+$/); expect(shared.get('RateLimitConfigSchema')).toMatch(/^src\/shared\/http\.zod\.ts:\d+$/); diff --git a/packages/spec/src/integration/connector.zod.ts b/packages/spec/src/integration/connector.zod.ts index 1e65e5dc08..afb6a5b647 100644 --- a/packages/spec/src/integration/connector.zod.ts +++ b/packages/spec/src/integration/connector.zod.ts @@ -5,6 +5,7 @@ import { CronExpressionInputSchema } from '../shared/expression.zod'; import { WebhookSchema } from '../automation/webhook.zod'; import { ConnectorAuthConfigSchema, ConnectorInstanceAuthSchema } from '../shared/connector-auth.zod'; import { FieldMappingSchema as BaseFieldMappingSchema } from '../shared/mapping.zod'; +import { retiredKey } from '../shared/retired-key'; /** * Connector Protocol - LEVEL 3: Enterprise Connector @@ -109,8 +110,9 @@ import { FieldMappingSchema as BaseFieldMappingSchema } from '../shared/mapping. * `./data` (`ImportFieldMappingSchema`, a CSV/table column mapping that is not * a connector mapping at all). Which type an importer got depended only on the * import path — the #4411 trap. Prefixing the domain-specific sides keeps the - * base name for the base, matching `ConnectorRateLimitConfig` (#4684), - * `ConnectorErrorCategory` and `ConnectorRetryStrategy` in this same file, and + * base name for the base, matching `ConnectorErrorCategory` and + * `ConnectorRetryStrategy` in this same file (`ConnectorRateLimitConfig`, + * #4684, was the fourth until its whole shape was retired in #4911), and * `ExternalFieldMappingSchema` in `data/external-lookup.zod.ts` — which extends * the same base and, precisely because it carries a domain prefix, never * entered the dual-source baseline. @@ -304,73 +306,38 @@ export const WebhookConfigSchema = lazySchema(() => WebhookSchema.extend({ export type WebhookConfig = z.infer; // ============================================================================ -// Rate Limiting and Retry Configuration +// Retry Configuration // ============================================================================ - -/** - * Rate Limiting Strategy - */ -export const RateLimitStrategySchema = lazySchema(() => z.enum([ - 'fixed_window', // Fixed time window - 'sliding_window', // Sliding time window - 'token_bucket', // Token bucket algorithm - 'leaky_bucket', // Leaky bucket algorithm -]).describe('Rate limiting strategy')); - -export type RateLimitStrategy = z.infer; - -/** - * Rate Limiting Configuration — connector-side (OUTBOUND throttling). - * - * `Connector`-prefixed because `shared/http.zod.ts` exports a different - * `RateLimitConfig` for INBOUND API rate limiting (`enabled` / `windowMs` / - * `maxRequests`, all defaulted). The two describe opposite directions and are - * not interchangeable: this one throttles the calls *we* make to an external - * system (token bucket, burst capacity, and the upstream `X-RateLimit-*` - * response headers we read back), while the shared one limits the calls - * *others* make to us — where `respectUpstreamLimits` / `rateLimitHeaders` are - * structurally meaningless. Same name, different units (`windowSeconds` vs - * `windowMs`) and different requiredness, so an author copying a snippet from - * one side to the other got a clean parse with the foreign keys silently - * stripped (#4684, ADR-0104). They may not share a name (ADR-0112 D9a). - */ -export const ConnectorRateLimitConfigSchema = lazySchema(() => z.object({ - /** - * Rate limiting strategy - */ - strategy: RateLimitStrategySchema.optional().default('token_bucket'), - - /** - * Maximum requests per window - */ - maxRequests: z.number().min(1).describe('Maximum requests per window'), - - /** - * Time window in seconds - */ - windowSeconds: z.number().min(1).describe('Time window in seconds'), - - /** - * Burst capacity (for token bucket) - */ - burstCapacity: z.number().min(1).optional().describe('Burst capacity'), - - /** - * Respect external system rate limits - */ - respectUpstreamLimits: z.boolean().optional().default(true).describe('Respect external rate limit headers'), - - /** - * Custom rate limit headers to check - */ - rateLimitHeaders: z.object({ - remaining: z.string().optional().default('X-RateLimit-Remaining').describe('Header for remaining requests'), - limit: z.string().optional().default('X-RateLimit-Limit').describe('Header for rate limit'), - reset: z.string().optional().default('X-RateLimit-Reset').describe('Header for reset time'), - }).optional().describe('Custom rate limit headers'), -})); - -export type ConnectorRateLimitConfig = z.infer; +// +// ─── REMOVED: outbound rate limiting (#4911, ADR-0049) ────────────────── +// +// `ConnectorRateLimitConfigSchema` / `ConnectorRateLimitConfig` and the +// `RateLimitStrategySchema` / `RateLimitStrategy` enum it embedded were removed +// wholesale in @objectstack/spec 17.0.0. The key that carried them +// (`ConnectorSchema.rateLimitConfig`) is tombstoned below. +// +// The reason is not "no reader yet" — it is that **the engine does not exist**. +// The platform's only token bucket is `packages/runtime/src/security/ +// rate-limit.ts`, and it is INBOUND: the dispatcher calls `consume(key)` with a +// request fingerprint and short-circuits with 429. Nothing anywhere throttles +// the calls *we* make to an external system, so `strategy` / `maxRequests` / +// `windowSeconds` / `burstCapacity` / `respectUpstreamLimits` / +// `rateLimitHeaders` were six well-formed knobs wired to nothing, on the most +// safety-shaped surface a connector has: an author who set them believed they +// had capped their outbound call rate. ADR-0049 says such a property is +// enforced, marked `experimental`, or absent; with no implementation and no +// committed roadmap, absent is the honest disposition. The vocabulary comes +// back **with** the engine, in the same change — implementation-first, the +// #4834 / PR #4878 precedent. +// +// Do NOT reach for `shared/http.zod.ts`'s `RateLimitConfig` as a replacement: +// that one is the INBOUND limiter (`enabled` / `windowMs` / `maxRequests`) and +// limits the calls *others* make to us. The two describe opposite directions and +// were separated by name for exactly that reason (#4684, ADR-0112 D9a) — the +// rename is absorbed by this removal (`scripts/lib/renamed-defs.ts`). +// +// Until an outbound throttle exists, rate-limit an integration where the calls +// are actually made: at the connector provider / upstream gateway. /** * Retry Strategy — connector-side. @@ -706,10 +673,24 @@ export const ConnectorSchema = lazySchema(() => z.object({ webhooks: z.array(WebhookConfigSchema).optional().describe('Webhook configurations (not yet enforced — never read at registration; see #3197)'), /** - * Rate limiting configuration - */ - rateLimitConfig: ConnectorRateLimitConfigSchema.optional().describe('Rate limiting configuration'), - + * REMOVED (#4911) — outbound rate limiting. See the block above + * "REMOVED: outbound rate limiting" for why the whole shape went, not just + * this key. `ConnectorSchema` is NOT `.strict()`, so a plain delete would be + * a silent strip (ADR-0104); the tombstone makes the removal audible in the + * two channels an upgrading author actually hits — `tsc` and the parse. + */ + rateLimitConfig: retiredKey( + '`connector.rateLimitConfig` was removed in @objectstack/spec 17.0.0 (#4911, ADR-0049 D2) — ' + + 'the entire shape is gone, not just this key: `ConnectorRateLimitConfig` and its ' + + '`RateLimitStrategy` enum were removed with it, because no outbound rate-limiting engine ' + + 'ever existed. The platform\'s only token bucket (runtime `security/rate-limit.ts`) throttles ' + + 'INBOUND requests to us; nothing throttled the calls a connector makes out, so every knob ' + + 'here was inert while reading like a configured cap. Delete the key. Do NOT substitute ' + + '`shared` `RateLimitConfig` — that is the inbound limiter and would cap the wrong direction; ' + + 'until an outbound throttle exists, rate-limit at the connector provider or upstream gateway. ' + + 'Run `os migrate meta --from 16` to rewrite it automatically.', + ), + /** * Retry configuration */ diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index d36ade59d1..2ed5cb22e3 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -783,7 +783,23 @@ const step17: MigrationStep = { + '`body.capabilities` on hooks and actions; it deliberately does NOT touch the ' + '`ctx.crypto.hash(...)` call the body made under it, which never returned a value and ' + 'which the author must delete. Hashing returns only WITH an implementation, through the ' - + 'capability admission process.', + + 'capability admission process.\n\n' + + "It also removes `connector.rateLimitConfig` and its whole shape (#4911). This one is not " + + '"declared but unread" — it is declared but UNIMPLEMENTED, one step worse. The only token ' + + "bucket the platform owns (runtime `security/rate-limit.ts`) is INBOUND: the dispatcher " + + 'calls `consume(key)` on a request fingerprint and answers 429. Nothing anywhere throttles ' + + 'the calls a connector makes OUT, and no provider — `connector-rest`, `connector-openapi`, ' + + '`connector-mcp`, `connector-slack` — reads the key or has a seam that could. So ' + + '`strategy`, `maxRequests`, `windowSeconds`, `burstCapacity`, `respectUpstreamLimits` and ' + + '`rateLimitHeaders` parsed cleanly and capped nothing, on a surface where the author ' + + "believed they had bounded their spend against a third party's quota. `ConnectorRateLimit" + + 'Config` and the `RateLimitStrategy` enum it embedded had no other consumer and are removed ' + + 'with the key, so importing either is TS2305 in v17 — the #4834 shape, and the same ' + + 'implementation-first ruling: the vocabulary comes back WITH the engine, in one change. ' + + 'It is deliberately NOT converted to `shared` `RateLimitConfig`, which limits the calls ' + + 'others make to US; #4684 split their names for precisely this confusion, and rewriting an ' + + 'outbound cap into an inbound one would throttle the wrong direction. Delete the key and ' + + 'rate-limit where the calls are actually made — the connector provider or upstream gateway.', conversionIds: [ 'action-execute-to-target', 'field-conditionalRequired-to-requiredWhen', @@ -822,6 +838,7 @@ const step17: MigrationStep = { 'retry-policy-converged', 'object-enable-trash-mru-removed', 'hook-body-crypto-hash-removed', + 'connector-rate-limit-config-removed', ], semantic: [ {