Skip to content

fix(spec): correct the conversion registry's falsified retryConfig liveness claim - #18789

Merged
os-bill merged 2 commits into
mainfrom
claude/issue-18614-retryconfig-live-claim
Sep 17, 2026
Merged

os-bill merged 2 commits into
mainfrom
claude/issue-18614-retryconfig-live-claim

Conversation

@os-bill

@os-bill os-bill commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

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 spec/liveness: seed ledgers for the three PENDING_GOVERNANCE debts #18133 declared — connector, sharing_rule, analytics_cube #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

… liveness claim

The `connector-rate-limit-config-removed` entry's fixture comment asserted that
`retryConfig` and the connector timeouts beside it "are live". Measured false:
the read-probe for all three keys reads 0 outside `packages/spec` (control:
the sibling key `providerConfig` fires on the same probe), no retry loop reads
a strategy or a backoff, every timeout occurrence outside the spec is a write
of the literal 30000 to satisfy the post-parse type, and
`ConnectorProviderContext` carries none of them, so a provider factory cannot
see them either. `packages/spec/liveness/connector.json` (#18582) classifies
all of these rows `dead`.

The true half of the claim is kept and narrowed to what it actually measured —
this conversion's scope — and the liveness verdict now points at the ledger.
No retirement is implied: ADR-0049 owes these keys a decision.

Claude-Session: https://claude.ai/code/session_01JbZnqu8bt6YqfJsr9vaFb3
Co-authored-by: Claude <noreply@anthropic.com>
…claim

The corrected comment ships: tsup preserves comments, so the text reaches
dist/index.js, dist/index.mjs and the shared/browser bundles, all six of which
npm pack lists in the published tarball. Published bytes move, so this is a
patch rather than skip-changeset.

Claude-Session: https://claude.ai/code/session_01JbZnqu8bt6YqfJsr9vaFb3
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation tooling labels Sep 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 136 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 2085be2b2d8769c6227167bb3525f4fa72b9a486 → packageMentionDocs.

@os-bill os-bill added domain:spec priority:p2 Medium: important, M3 labels Sep 17, 2026 — with Claude
@os-bill
os-bill marked this pull request as ready for review September 17, 2026 21:34
@os-bill
os-bill added this pull request to the merge queue Sep 17, 2026
Merged via the queue into main with commit 00b38d7 Sep 17, 2026
45 checks passed
@os-bill
os-bill deleted the claude/issue-18614-retryconfig-live-claim branch September 17, 2026 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation domain:spec priority:p2 Medium: important, M3 size/s tooling

Projects

None yet

2 participants