Skip to content

fix(schema): require integer retry_after seconds - #6165

Open
bokelley wants to merge 1 commit into
mainfrom
fix/retry-after-integer-3-2
Open

fix(schema): require integer retry_after seconds#6165
bokelley wants to merge 1 commit into
mainfrom
fix/retry-after-integer-3-2

Conversation

@bokelley

@bokelley bokelley commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Makes error.retry_after integer-only for 3.2 and pins migration behavior rather than leaving fractional values to SDK-specific truncation.

The rule is ceiling before clamping:

  1. Round a finite fractional delay up to the next whole second.
  2. Clamp the result to 1–3600.
  3. Treat non-finite values as absent.

Rounding up preserves the producer's minimum intended delay; flooring could make a client retry early. During migration, tolerant clients should apply the same rule to legacy fractional values, while 3.2 producers emit integers only.

Closes #5953.

Changes

  • Changes the JSON Schema type from number to integer.
  • Aligns error vocabulary and both error-handling references.
  • Updates the executable JavaScript example to use Math.ceil before clamping.
  • Adds a regression test that pins type, bounds, and migration prose.
  • Adds a minor protocol changeset.

Validation

  • Retry-after regression tests: 2 passed
  • Error-code and drift lints: clean
  • Error-handling copy test: passed
  • Changeset scope gate: passed
  • git diff --check

@bokelley
bokelley marked this pull request as ready for review August 3, 2026 14:37
"retry_after": {
"type": "number",
"description": "Seconds to wait before retrying the operation. Sellers MUST return values between 1 and 3600. Clients MUST clamp values outside this range.",
"type": "integer",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MUST FIX: numberinteger narrows the value domain of a field released in 3.1.x. Every payload valid under the old schema (retry_after: 30.5) is now schema-invalid. That is a backward-incompatible tightening of a published wire contract, not an additive change — a 3.2 consumer that validates a 3.1 seller's error strictly against this schema rejects it. The PR's own migration prose ("legacy finite fractional value") confirms fractional values exist on the wire, so this is not a clarification every conformant implementation already satisfies (the prior reference example did not ceil). Constitution decision class: Breaking → major, not the minor changeset shipped. Repo rule: a minor changeset carrying a breaking wire change is high. Either resize to major (and route through Breaking-class ratification) or state in the PR body why a domain narrowing on a released field is being treated as non-breaking.

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Request changes

Request changes — 1 blocking finding.

Blocking findings

  • static/schemas/source/core/error.json:27 — retry_after type narrowed from number to integer, a backward-incompatible wire tightening shipped on a minor changeset. A field released in 3.1.x is having its value domain narrowed; a producer that previously emitted a fractional retry_after now emits an invalid value against the tightened schema, breaking conformant 3.1 implementations. Per the changeset-scope rule, a breaking wire change (type narrowing / value-domain restriction on a published field) requires a major changeset, not a minor. This is also a Breaking-class protocol change under the constitution's decision classes, which is never auto-approved without human ratification.

The schema↔docs coherence, ceil-before-clamp migration prose, JS example, and regression test are all internally consistent and correct — the sole concern is the release sizing of the breaking narrowing.

Note: this PR also touches gated paths (static/schemas/source/**) with review_decision: REVIEW_REQUIRED, so it additionally requires human/CODEOWNERS approval before merge.

Row 1 of the decision table (a high finding present) governs: outcome is request-changes.

Blocking findings

  • static/schemas/source/core/error.json:27 — retry_after number -> integer is a breaking type narrowing shipped as minor; breaking wire changes require a major changeset and human ratification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec: decide whether error.retry_after permits fractional seconds

1 participant