Skip to content

Commit c1f344b

Browse files
os-zhuangclaude
andauthored
refactor(spec)!: rename connector-side RateLimitConfig (#4684, C9) (#4695)
`@objectstack/spec` exported `RateLimitConfig` / `RateLimitConfigSchema` from two entry points for two DIFFERENT declarations — `./shared` limits INBOUND API traffic (`enabled` / `windowMs` / `maxRequests`, all defaulted), `./integration` throttles OUTBOUND connector calls (`strategy` / `maxRequests` / `windowSeconds` required, plus the upstream `X-RateLimit-*` header names). Neither schema is `.strict()`, so a snippet copied between the two parsed clean with its foreign keys silently stripped (#4411 trap / ADR-0104 silent-strip class). They are two concepts, not two spellings, so ADR-0112 D9(a) applies — the same ruling that produced `ConnectorErrorCategory` and `ConnectorRetryStrategy` ten lines below. The connector side is renamed to `ConnectorRateLimitConfig`; `./shared` keeps its name, keys and defaults. No back-compat alias: it would be a third declaration of the name this change is removing. Zero authorable-key change — all six keys under `connectors[].rateLimitConfig` parse exactly as before — hence no ADR-0087 conversion and no tombstone. Riding along: `scripts/build-schemas.ts` learns a declarative `RENAMED_DEFS` table (`scripts/lib/renamed-defs.ts`). Its two ratchets measure in `$def` units, so a def rename previously read as six authorable keys vanishing at once, with all three suggested remedies wrong for a rename (hand-editing the surface is banned by #4650; a tombstone + conversion would register a migration nobody must run). The table enforces the rule a rename must obey — every key under the old def must exist under the new one, the target must be emitted, and the source must not (a def still published is a copy, not a rename) — which is strictly stronger than the hand-edited baseline it replaces. dual-source-exports baseline: 18 -> 16. Claude-Session: https://claude.ai/code/session_0176qgxgCXTJCUv4YFLtusP9 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 742cebb commit c1f344b

13 files changed

Lines changed: 536 additions & 60 deletions
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
---
2+
"@objectstack/spec": major
3+
---
4+
5+
BREAKING(spec): `@objectstack/spec/integration` renames `RateLimitConfig`
6+
`ConnectorRateLimitConfig` (#4684, C9)
7+
8+
Two entry points exported `RateLimitConfig` for **two different declarations**,
9+
so which one you got depended only on the import path — the #4411 trap. They are
10+
not variants of one concept; they describe opposite directions of traffic:
11+
12+
| | `@objectstack/spec/shared` (unchanged) | `@objectstack/spec/integration` (renamed) |
13+
|:--|:--|:--|
14+
| what it limits | **inbound** — calls others make to our API | **outbound** — calls we make to an external system |
15+
| written at | `apis[].rateLimit`, `httpServer.security.rateLimit` | `connectors[].rateLimitConfig` |
16+
| window | `windowMs` (ms), defaults to 60000 | `windowSeconds` (s), **required**, min 1 |
17+
| quota | `maxRequests`, defaults to 100 | `maxRequests`, **required**, min 1 |
18+
| extras | `enabled` (default `false`) | `strategy`, `burstCapacity`, `respectUpstreamLimits`, `rateLimitHeaders` |
19+
20+
Neither schema is `.strict()`, so a snippet copied from one side to the other
21+
parsed **clean** with its foreign keys silently stripped — `RateLimitConfigSchema
22+
.parse({ windowSeconds: 60, strategy: 'token_bucket' })` returned
23+
`{ enabled: false, windowMs: 60000, maxRequests: 100 }` and nothing said a word.
24+
Per ADR-0112 D9(a) — the same ruling that produced `ConnectorErrorCategory` and
25+
`ConnectorRetryStrategy` in the same file — the **connector side is renamed** so
26+
one name means one thing.
27+
28+
## FROM → TO
29+
30+
```ts
31+
// before
32+
import { RateLimitConfigSchema, type RateLimitConfig } from '@objectstack/spec/integration';
33+
34+
// after
35+
import {
36+
ConnectorRateLimitConfigSchema,
37+
type ConnectorRateLimitConfig,
38+
} from '@objectstack/spec/integration';
39+
```
40+
41+
No deprecated alias is kept: re-exporting the old name would be a third
42+
declaration of it and would re-open the trap this change closes.
43+
44+
**Importing from `@objectstack/spec/shared` (or `/api`, `/system`)? Nothing
45+
changes** — that `RateLimitConfig` keeps its name, its keys and its defaults.
46+
47+
## Authored metadata needs no migration
48+
49+
This renames a TypeScript export and an internal JSON Schema `$def`, not an
50+
authorable key. Every one of the six keys an author can write under
51+
`connectors[].rateLimitConfig``strategy`, `maxRequests`, `windowSeconds`,
52+
`burstCapacity`, `respectUpstreamLimits`, `rateLimitHeaders` — parses exactly as
53+
before. Existing stack metadata, stored `sys_metadata` rows and published apps
54+
are byte-for-byte unaffected, which is why this change ships with **no ADR-0087
55+
conversion and no tombstone**: nothing was retired.
56+
57+
The only edit an upgrade needs is the import above, in TypeScript that named the
58+
type. The published JSON Schema `$id` moves with it:
59+
`…/integration/RateLimitConfig.json``…/integration/ConnectorRateLimitConfig.json`.
60+
61+
## Gate change riding along
62+
63+
`scripts/build-schemas.ts` learns a declarative `RENAMED_DEFS` table
64+
(`scripts/lib/renamed-defs.ts`). Its two ratchets measure in `$def` units, so a
65+
def rename previously read as six authorable keys vanishing at once. The table
66+
carries the old snapshot forward under the new name and enforces the rule a
67+
rename must obey: **every key under the old def must exist under the new one, or
68+
the build fails** — plus the target must be emitted and the source must not (a
69+
def that is still published is a copy, not a rename). This is stricter than the
70+
hand-edited baseline it replaces, which could drop any line without a trace.

content/docs/references/integration/connector.mdx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ with simple `auth` — or by `[automation/sync.zod.ts](/docs/references/automati
132132
## TypeScript Usage
133133

134134
```typescript
135-
import { CircuitBreakerConfigSchema, ConnectorSchema, ConnectorActionSchema, ConnectorErrorCategorySchema, ConnectorHealthSchema, ConnectorRetryStrategySchema, ConnectorStatusSchema, ConnectorTriggerSchema, ConnectorTypeSchema, DataSyncConfigSchema, DeclarativeConnectorEntrySchema, ErrorMappingConfigSchema, ErrorMappingRuleSchema, HealthCheckConfigSchema, RateLimitStrategySchema, RetryConfigSchema, SyncStrategySchema, WebhookConfigSchema, WebhookEventSchema, WebhookSignatureAlgorithmSchema } from '@objectstack/spec/integration';
136-
import type { CircuitBreakerConfig, Connector, ConnectorErrorCategory, ConnectorHealth, ConnectorRetryStrategy, ConnectorStatus, ConnectorType, DataSyncConfig, DeclarativeConnectorEntry, ErrorMappingConfig, ErrorMappingRule, HealthCheckConfig, RateLimitStrategy, RetryConfig, SyncStrategy, WebhookConfig, WebhookEvent, WebhookSignatureAlgorithm } from '@objectstack/spec/integration';
135+
import { CircuitBreakerConfigSchema, ConnectorSchema, ConnectorActionSchema, ConnectorErrorCategorySchema, ConnectorHealthSchema, ConnectorRateLimitConfigSchema, ConnectorRetryStrategySchema, ConnectorStatusSchema, ConnectorTriggerSchema, ConnectorTypeSchema, DataSyncConfigSchema, DeclarativeConnectorEntrySchema, ErrorMappingConfigSchema, ErrorMappingRuleSchema, HealthCheckConfigSchema, RateLimitStrategySchema, RetryConfigSchema, SyncStrategySchema, WebhookConfigSchema, WebhookEventSchema, WebhookSignatureAlgorithmSchema } from '@objectstack/spec/integration';
136+
import type { CircuitBreakerConfig, Connector, ConnectorErrorCategory, ConnectorHealth, ConnectorRateLimitConfig, ConnectorRetryStrategy, ConnectorStatus, ConnectorType, DataSyncConfig, DeclarativeConnectorEntry, ErrorMappingConfig, ErrorMappingRule, HealthCheckConfig, RateLimitStrategy, RetryConfig, SyncStrategy, WebhookConfig, WebhookEvent, WebhookSignatureAlgorithm } from '@objectstack/spec/integration';
137137

138138
// Validate data
139139
const result = CircuitBreakerConfigSchema.parse(data);
@@ -237,6 +237,22 @@ Connector health configuration
237237
| **circuitBreaker** | `{ enabled: boolean; failureThreshold: number; resetTimeoutMs: number; halfOpenMaxRequests: number; … }` | optional | Circuit breaker configuration |
238238

239239

240+
---
241+
242+
## ConnectorRateLimitConfig
243+
244+
### Properties
245+
246+
| Property | Type | Required | Description |
247+
| :--- | :--- | :--- | :--- |
248+
| **strategy** | `Enum<'fixed_window' \| 'sliding_window' \| 'token_bucket' \| 'leaky_bucket'>` || Rate limiting strategy |
249+
| **maxRequests** | `number` || Maximum requests per window |
250+
| **windowSeconds** | `number` || Time window in seconds |
251+
| **burstCapacity** | `number` | optional | Burst capacity |
252+
| **respectUpstreamLimits** | `boolean` || Respect external rate limit headers |
253+
| **rateLimitHeaders** | `{ remaining: string; limit: string; reset: string }` | optional | Custom rate limit headers |
254+
255+
240256
---
241257

242258
## ConnectorRetryStrategy

content/docs/references/integration/http.mdx

Lines changed: 0 additions & 35 deletions
This file was deleted.

content/docs/references/integration/meta.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
"connector-auth",
77
"mapping",
88
"---Transport & Storage---",
9-
"http",
109
"offline"
1110
]
1211
}

packages/spec/api-surface.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3939,6 +3939,8 @@
39393939
"ConnectorOrigin (type)",
39403940
"ConnectorProviderContext (interface)",
39413941
"ConnectorProviderFactory (type)",
3942+
"ConnectorRateLimitConfig (type)",
3943+
"ConnectorRateLimitConfigSchema (const)",
39423944
"ConnectorRetryStrategy (type)",
39433945
"ConnectorRetryStrategySchema (const)",
39443946
"ConnectorSchema (const)",
@@ -3961,8 +3963,6 @@
39613963
"FieldMappingSchema (const)",
39623964
"HealthCheckConfig (type)",
39633965
"HealthCheckConfigSchema (const)",
3964-
"RateLimitConfig (type)",
3965-
"RateLimitConfigSchema (const)",
39663966
"RateLimitStrategy (type)",
39673967
"RateLimitStrategySchema (const)",
39683968
"ResolvedConnectorAuth (type)",

packages/spec/authorable-surface.json

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4156,6 +4156,12 @@
41564156
"integration/ConnectorInstanceBearerAuth:credentialRef",
41574157
"integration/ConnectorInstanceBearerAuth:type",
41584158
"integration/ConnectorInstanceNoAuth:type",
4159+
"integration/ConnectorRateLimitConfig:burstCapacity",
4160+
"integration/ConnectorRateLimitConfig:maxRequests",
4161+
"integration/ConnectorRateLimitConfig:rateLimitHeaders",
4162+
"integration/ConnectorRateLimitConfig:respectUpstreamLimits",
4163+
"integration/ConnectorRateLimitConfig:strategy",
4164+
"integration/ConnectorRateLimitConfig:windowSeconds",
41594165
"integration/ConnectorTrigger:description",
41604166
"integration/ConnectorTrigger:interval",
41614167
"integration/ConnectorTrigger:key",
@@ -4219,12 +4225,6 @@
42194225
"integration/HealthCheckConfig:method",
42204226
"integration/HealthCheckConfig:timeoutMs",
42214227
"integration/HealthCheckConfig:unhealthyThreshold",
4222-
"integration/RateLimitConfig:burstCapacity",
4223-
"integration/RateLimitConfig:maxRequests",
4224-
"integration/RateLimitConfig:rateLimitHeaders",
4225-
"integration/RateLimitConfig:respectUpstreamLimits",
4226-
"integration/RateLimitConfig:strategy",
4227-
"integration/RateLimitConfig:windowSeconds",
42284228
"integration/RetryConfig:backoffMultiplier",
42294229
"integration/RetryConfig:initialDelayMs",
42304230
"integration/RetryConfig:jitter",

packages/spec/dual-source-exports.baseline.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
"HttpMethod — [./api, ./shared (type)] ≠ [./ui (type)]",
1616
"PackageDependency — [./cloud (type)] ≠ [./kernel (type)]",
1717
"PackageDependencySchema — [./cloud (const)] ≠ [./kernel (const)]",
18-
"RateLimitConfig — [./integration (type)] ≠ [./shared (type)]",
19-
"RateLimitConfigSchema — [./integration (const)] ≠ [./shared (const)]",
2018
"TenantPlan — [./cloud (type)] ≠ [./system (type)]",
2119
"TenantPlanSchema — [./cloud (const)] ≠ [./system (const)]"
2220
]

packages/spec/json-schema.manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,7 @@
879879
"integration/ConnectorInstanceBasicAuth",
880880
"integration/ConnectorInstanceBearerAuth",
881881
"integration/ConnectorInstanceNoAuth",
882+
"integration/ConnectorRateLimitConfig",
882883
"integration/ConnectorRetryStrategy",
883884
"integration/ConnectorStatus",
884885
"integration/ConnectorTrigger",
@@ -889,7 +890,6 @@
889890
"integration/ErrorMappingRule",
890891
"integration/FieldMapping",
891892
"integration/HealthCheckConfig",
892-
"integration/RateLimitConfig",
893893
"integration/RateLimitStrategy",
894894
"integration/RetryConfig",
895895
"integration/SyncStrategy",

packages/spec/scripts/build-schemas.ts

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import fs from 'fs';
99
import path from 'path';
1010
import { z } from 'zod';
1111
import { schemaNameFromExportKey } from './lib/schema-name';
12+
import { RENAMED_DEFS, carryAuthorableKey, checkRenameTable } from './lib/renamed-defs';
1213
import { CONVERSIONS_BY_MAJOR } from '../src/conversions/registry';
1314
import { MIGRATIONS_BY_MAJOR } from '../src/migrations/registry';
1415
import * as AI from '../src/ai';
@@ -301,7 +302,24 @@ try {
301302
}
302303

303304
const generatedKeys = new Set(generatedSchemas.keys());
304-
const missing = (manifest?.schemas ?? []).filter((key) => !generatedKeys.has(key));
305+
306+
// ─── Declared def renames must describe THIS build ────────────────────
307+
// Both ratchets below consult RENAMED_DEFS, so an entry that no longer matches
308+
// reality (target never emitted, or source still emitted alongside it) would
309+
// weaken them silently. Fail before either one runs. See lib/renamed-defs.ts.
310+
const renameProblems = checkRenameTable(generatedKeys);
311+
if (renameProblems.length > 0) {
312+
console.error(`\n❌ ${renameProblems.length} problem(s) in RENAMED_DEFS (scripts/lib/renamed-defs.ts):`);
313+
for (const p of renameProblems) console.error(` - ${p}`);
314+
process.exit(1);
315+
}
316+
317+
const missing = (manifest?.schemas ?? []).filter(
318+
// A def listed as renamed is not missing — it is published under the new
319+
// name, which `checkRenameTable` just proved this build emits. The manifest
320+
// rewrite below drops the old key, so the entry self-clears on regeneration.
321+
(key) => !generatedKeys.has(key) && !(key in RENAMED_DEFS),
322+
);
305323
if (missing.length > 0) {
306324
console.error(`\n❌ ${missing.length} previously published schema(s) disappeared from this build:`);
307325
for (const key of missing) {
@@ -319,7 +337,12 @@ if (missing.length > 0) {
319337
}
320338

321339
const added = [...generatedKeys].filter((key) => !(manifest?.schemas ?? []).includes(key));
322-
if (!manifest || added.length > 0) {
340+
// A renamed-away source key must be dropped from the manifest even in the (rare)
341+
// case where the new name adds nothing — e.g. a rename onto a def that already
342+
// existed. Without this the stale key would sit in the manifest forever, kept
343+
// alive only by its RENAMED_DEFS entry.
344+
const renamedAway = (manifest?.schemas ?? []).filter((key) => key in RENAMED_DEFS);
345+
if (!manifest || added.length > 0 || renamedAway.length > 0) {
323346
const updated: SchemaManifest = {
324347
description:
325348
'Ratchet manifest of every JSON Schema emitted by scripts/build-schemas.ts. ' +
@@ -401,10 +424,46 @@ if (fs.existsSync(AUTHORABLE_SURFACE_PATH)) {
401424
}
402425

403426
if (surfaceDoc) {
404-
const prev = new Map<string, boolean>(
427+
const snapshot = new Map<string, boolean>(
405428
surfaceDoc.keys.map((e) => [e.replace(RETIRED_MARK, ''), e.endsWith(RETIRED_MARK)]),
406429
);
407430

431+
// Carry the snapshot through any declared def rename FIRST, so every check
432+
// below compares like with like. A rename moves keys between defs; it must
433+
// never be able to drop one, and it must never launder a retirement past
434+
// check (b) either — which is why the carried key keeps the OLD key's
435+
// retired state. See scripts/lib/renamed-defs.ts (#4684).
436+
const prev = new Map<string, boolean>();
437+
const carriedFrom = new Map<string, string>(); // new key -> old key
438+
for (const [key, retired] of snapshot) {
439+
const carried = carryAuthorableKey(key);
440+
if (carried !== key) carriedFrom.set(carried, key);
441+
prev.set(carried, retired);
442+
}
443+
444+
// (a0) A declared rename that did not carry one of its keys. Reported apart
445+
// from (a) because the remedy is the opposite one: the key did not leave
446+
// the contract by accident of a deletion, it failed to arrive under the
447+
// new def — restore it there, or stop calling this a rename.
448+
const notCarried = [...carriedFrom.entries()].filter(([to]) => !currentKeys.has(to));
449+
if (notCarried.length > 0) {
450+
console.error(
451+
`\n❌ ${notCarried.length} authorable key(s) were lost by a declared def rename:`,
452+
);
453+
for (const [to, from] of notCarried) console.error(` - ${from}${to} (absent)`);
454+
console.error(
455+
`\n RENAMED_DEFS (scripts/lib/renamed-defs.ts) declares that these defs were renamed,\n` +
456+
` and a rename must carry EVERY key: the author-facing contract is unchanged, only\n` +
457+
` an internal schema name moved. A key missing under the new name is a real removal\n` +
458+
` wearing a rename's clothes — and these schemas are NOT .strict(), so Zod would\n` +
459+
` silently strip whatever the author kept writing (#3733, ADR-0104).\n\n` +
460+
` Either re-add the key under the new def, or — if it is genuinely being retired —\n` +
461+
` tombstone it there with \`retiredKey()\` plus its registered D2 conversion, exactly\n` +
462+
` as a retirement without a rename would require.`,
463+
);
464+
process.exit(1);
465+
}
466+
408467
// (a) A key that vanished outright. The silent-strip class — always fatal.
409468
const vanished = [...prev.keys()].filter((k) => !currentKeys.has(k));
410469
if (vanished.length > 0) {

0 commit comments

Comments
 (0)