Skip to content

Commit 00b38d7

Browse files
os-billclaude
andauthored
fix(spec): correct the conversion registry's falsified retryConfig liveness claim (#18789)
Fixes #18614 Clause-②: no The `connector-rate-limit-config-removed` entry's fixture comment asserted that `retryConfig` "and the timeouts beside it are untouched — they are live." The first half is true and is kept. The second half was never measured by that entry, and is false. ## The two questions the claim comment could not answer ### 1. Is `conversions/registry.ts` generated? — measured: **hand-written** Not read off the file header (the absence of a banner proves nothing). Read off the repo's own routing ledger: - `scripts/regen-artifacts.mjs` — named in `.gitattributes` as "the single source of truth for this list" — carries `packages/spec/src/conversions/registry.ts` in `NOT_DRIVER_MANAGED` with `why: 'hand-written source.'`, and **no `gen:` key**. - **Lit control on the same ledger**: its sibling `packages/spec/src/migrations/registry.ts` sits in the same structure *with* `gen: 'gen:migration-registry'`. The ledger does distinguish a generated registry from a hand-written one, so "no gen key" is a reading, not a search that never fired. - Marker control: the generated sibling carries `⚠️ The three tables below are GENERATED (#7297)`; the same predicate reads **0** on `conversions/registry.ts`. - `pnpm --filter @objectstack/spec check:generated` — **all 15 artifacts up to date**, including `check:spec-changes` and `check:upgrade-guide`, the two artifacts derived *from* this registry. Nothing regenerates it. No generator was run against it, so no hand-edit of a generated file occurred. ### 2. What should the wording say? — measured: **declared, unimplemented, and unreachable by a host** The seat listed three candidate readings. Measured against this tree: | candidate | verdict | basis | |---|---|---| | declared but unimplemented | **true** | read-probe below | | left to the host | **false** | `ConnectorProviderContext` carries none of the three keys | | pending retirement | **not established** | ADR-0049 owes a decision; this card does not ask for a sweep | **Read-probe** — the property-access form, i.e. a consumer *reading* the key, everywhere outside `packages/spec`: ``` .retryConfig 0 hits .connectionTimeoutMs 0 hits .requestTimeoutMs 0 hits ``` **Lit control for that probe**: `providerConfig` — a sibling key on the same schema that *is* consumed — fires on the identical predicate (`connector-mcp/src/mcp-provider.ts:173`, `connector-openapi/src/openapi-provider.ts:175`, `connector-rest/src/rest-provider.ts:41`, and more). The zero is a reading. The host seam settles the "left to the host" branch: `packages/spec/src/integration/connector-provider.ts:57` declares `ConnectorProviderContext` as `name`, `label`, `description`, `icon`, `type`, `providerConfig`, `auth`, `loadPackageFile`. A provider factory is never handed `retryConfig` or either timeout, so it cannot honour them even if it wanted to. So the new text says: untouched **by this conversion** — a statement about scope, not a liveness verdict — and points at `liveness/connector.json` as the authority, without implying a retirement ADR-0049 has not decided. ## Census — paths printed, never counts The card records the method lesson that a count over a widened path set nearly produced a false refutation. Both censuses printed every path; the full listings are in the report comment. Summary: - **`retryConfig`** — every occurrence under `packages/` is inside `packages/spec` (the declaration, its own test, generated baselines, `SYNC_ARCHITECTURE.md`, the liveness ledger, and the claim itself). Outside `packages/spec`: two `content/docs/**` pages (one generated from the declaration, one prose) and, new since the card was filed, the `.changeset` entry for #18582. None is a producer or a consumer. - **`connectionTimeoutMs` / `requestTimeoutMs`** — every occurrence outside `packages/spec` is a **write** of the literal `30000` into a def so it satisfies the post-parse `Connector` type: four connector packages plus the degraded-husk literal in `service-automation/src/plugin.ts`, each with a comment saying exactly that. **Lit control** (required, so that "nothing outside" is not a search that never fired): the census reads **2** on `packages/spec/src/integration/connector.zod.ts`, a file known to declare the key. ## DARK control — and a broken predicate the control caught Required reading: the "are live" assertion about `retryConfig` must read **0** in `packages/spec/src/conversions/registry.ts` after the change, with a non-zero control proving the predicate fires. ``` predicate: retryConfig.*\b(are|is) live|\b(are|is) live.*retryConfig pre-edit (git show HEAD:...registry.ts) 1 hit, line 4493 == control: FIRES post-edit (working tree) 0 hits == DARK: required reading ``` ⚠️ **The control earned its keep.** The first spelling of this predicate used `[^\n]*` between the two halves and read **0 on the pre-edit tree** — it would have "confirmed" the fix against a file that still carried the claim. In POSIX ERE a bracket expression has no escapes, so `[^\n]` is "any character except backslash and the letter n" and the match died on the `n` in "and". Demonstrated on a string that plainly contains the assertion: v1 returns 0 hits. The corrected predicate is the one above. Second control, against a blanket wipe: the file carries 9 `are live` / `is live` assertions before the change and 8 after — exactly one moved, and the other eight (about `aria`/`data`, `batchSize`, `doc.translations`, `icon`, `object`) are untouched. ## Changeset — decided by measuring published bytes `Clause-②: no` — comment text only, no authorable key moves. The bump is **`patch`, not `skip-changeset`**, and that was measured rather than inferred from the source path: - `files[]` for `@objectstack/spec` ships `src/**/*.zod.ts`, so the edited `src/conversions/registry.ts` ships **no source bytes** — `npm pack --dry-run --json` lists 0 `src/conversions/*` entries. The inference stops here and gets it wrong. - **But `tsup` does not strip comments.** The corrected text is present in `dist/index.js`, `dist/index.mjs`, `dist/shared/index.js`, `dist/shared/index.mjs`, `dist/browser/index.js` and `dist/browser/index.mjs`, and `npm pack --dry-run --json` (re-run **after** a real build — the first run was taken against a tree with no `dist/` and silently missed all of it) lists all six in the 2021-entry tarball for `@objectstack/spec@17.4.0`. - Positive control for that grep: a string from the same conversion entry that is known to ship (`no outbound rate-limiting engine exists`) is found in those bundles plus `spec-changes.json`. Published bytes move, so `skip-changeset` is unavailable. ## Verification - `pnpm --filter @objectstack/spec build` — clean, 34/34 declaration files emitted. - `pnpm --filter @objectstack/spec test` — **486 files / 14017 tests passed**. - `pnpm --filter @objectstack/spec typecheck` — clean. - `pnpm --filter @objectstack/spec check:generated` — **15/15 up to date**. - `pnpm lint` — the full repo-wide `eslint . --no-inline-config`, exit 0. Not a narrowed run, so no scope-invariance argument is needed. - Gates derived from the real change set via `scripts/pm/dispatch-gates.mjs`, and the implicated families run green: `check:adr-0087-registration`, `check:spec-docblock-symbol-anchors`, `check:page-declaration-shape`, `check:pm-widening-tells` (`--declaration no`), `docs-audit/check-affected-docs`, `check:changeset-no-major`, `check:empty-changeset`, `check:objectui-changeset`, `check:nul-bytes`, `check:comment-mask-adoption` (+ self-test), `check:comment-mask-corpus`, `check:keyed-text-bounds`, `check:published-files`, `check:llms-txt`, `check:merge-driver`, `check:empty-state`, `check:exported-any`, `check:dual-source-exports`, `check:variant-docs`, `check:doc-authoring`. - Dependency closure (`@objectstack/spec^...`) is **empty** — `packages/spec` declares no `workspace:` dependencies — so that family is a genuine no-op here, not a skip. - The remaining derived families are repo-wide population scans that CI owns; they are declared to CI, not silently narrowed. ## Acceptance notes Out-of-scope observations, noted and deliberately **not** folded into this PR. They are carried back in the report comment for the triage seat; none is filed from here. - `packages/spec/docs/SYNC_ARCHITECTURE.md` still routes an author to `retryConfig` as the answer for a rate-limited upstream, citing its `retryableStatusCodes` default of `[408, 429, 500, 502, 503, 504]` — a default the liveness ledger records as read by nothing. `content/docs/automation/flows.mdx:1588` carries the same advice. This is the metadata-authoring-trap class, and the ledger's own note already flags it as the reason ADR-0049 owes a decision rather than a sweep. It is a docs surface this card's file surface does not cover. - The generated reference page `content/docs/references/integration/connector.mdx` advertises `retryConfig` and both timeouts with their defaults and no liveness signal. It is generated from the declaration, so the remedy belongs at the generator or the declaration, never in the page. Neither is touched here: this card is a one-comment correction and says in its own body that it does not ask for the keys to be retired. --- _Generated by [Claude Code](https://claude.ai/code/session_01JbZnqu8bt6YqfJsr9vaFb3)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 627382b commit 00b38d7

2 files changed

Lines changed: 20 additions & 1 deletion

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
`src/conversions/registry.ts` — the `connector-rate-limit-config-removed` entry no longer asserts that `retryConfig` and the connector timeouts "are live" (#18614). The assertion was measured false; the ledger seeded by #18582 had already recorded the correction on the other side.
6+
7+
The comment conflated two different statements. That the rate-limit retirement left those keys *in place* is true and is kept — it is what the fixture's single notice demonstrates. That they are *live* was never measured by that entry and is false: the read-probe for `retryConfig`, `connectionTimeoutMs` and `requestTimeoutMs` finds no consumer anywhere outside `packages/spec` (the sibling key `providerConfig`, on the same schema, fires on the identical probe), no retry loop reads a strategy or a backoff, every timeout occurrence outside the spec is a write of the literal `30000` so a def satisfies the post-parse `Connector` type, and `ConnectorProviderContext` carries none of the three — so a provider factory cannot read them either. `liveness/connector.json` classifies all ten rows `dead` and is now cited as the authority.
8+
9+
Nothing is retired here and no schema moved: ADR-0049 owes these keys a decision, which the corrected comment states rather than pre-empts. The text ships — `tsup` preserves comments, so these bytes reach `dist/index.js`, `dist/index.mjs` and the `shared`/`browser` bundles inside the published tarball, which is why this is a `patch` and not `skip-changeset`.
10+
11+
Clause-②: no

‎packages/spec/src/conversions/registry.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4490,7 +4490,15 @@ const connectorRateLimitConfigRemoved: MetadataConversion = {
44904490
],
44914491
},
44924492
// One notice per connector, not per knob: the block is what was removed.
4493-
// `retryConfig` and the timeouts beside it are untouched — they are live.
4493+
// `retryConfig` and the timeouts beside it are untouched by THIS conversion
4494+
// — a statement about its scope, not a liveness verdict. They are not live:
4495+
// declared, defaulted and documented, and read by nothing. No retry loop
4496+
// reads a strategy or a backoff; every occurrence of the timeouts outside
4497+
// `packages/spec` is a write of the literal `30000` so a def satisfies the
4498+
// post-parse type; and `ConnectorProviderContext` carries neither, so a
4499+
// provider factory cannot see them. `liveness/connector.json` classifies
4500+
// all of them `dead` and is the authority here — ADR-0049 owes them a
4501+
// decision, which this entry does not pre-empt.
44944502
after: {
44954503
connectors: [
44964504
{ name: 'billing_api', label: 'Billing API', type: 'api' },

0 commit comments

Comments
 (0)