Skip to content

BC5 readiness: cross-language wire-replay decoders#301

Open
jeremy wants to merge 19 commits into
bc5-readiness-ts-live-canaryfrom
bc5-readiness-wire-replay-decoders
Open

BC5 readiness: cross-language wire-replay decoders#301
jeremy wants to merge 19 commits into
bc5-readiness-ts-live-canaryfrom
bc5-readiness-wire-replay-decoders

Conversation

@jeremy

@jeremy jeremy commented May 4, 2026

Copy link
Copy Markdown
Member

Summary

Companion to #294 (TS canonical wire-capture canary). Adds wire-replay mode to the Ruby, Python, Go, and Kotlin runners so they consume the canonical wire snapshots TS captures against a live backend, decode each page through their SDK, and walk the raw JSON for required-field + extras detection. Swift remains excluded from the live canary surface.

The architecture pairs with §5e/5f of the BC5-readiness plan: TS is the only live HTTP runner; everyone else replays from disk. Decode-result snapshots land at <dir>/<backend>/decode/<lang>/ with schema_version: 1.

Per the cross-SDK omnibus pattern, this is one PR with one commit per SDK plus a TS amendment commit (operation field on snapshots) and one infrastructure commit (Make targets + CONTRIBUTING + gitignore).

What ships

  • TS amendment — wire snapshots now carry operation at the top level so replay runners dispatch without re-parsing the live fixture. Backwards compatible at the type level; runtime gates fail loud if a pre-PR3 snapshot shows up.
  • Ruby replay runner — decoder boundary is the SDK's actual production path (JSON.parse + normalize_person_ids).
  • Python replay runner — decoder boundary is json.loads + _normalize_person_ids. Same code path every SDK request runs in production.
  • Go replay runner — decoder boundary is the typed generated.<Op>ResponseContent types from go/pkg/generated. Mode-gate prepended to existing main(); mock-mode body untouched.
  • Kotlin replay runner — decoder boundary is @Serializable data classes via the same Json { ignoreUnknownKeys = true } the mock runner uses. Separate :conformance:runReplay Gradle task; existing :conformance:run untouched. Four ops decode through JsonElement because typed MyAssignment / Notification models don't yet exist (TODO noted).
  • Schema walkers — hand-rolled per-language ports of conformance/runner/typescript/schema-validator.ts's required + extras algorithm. No new library dependencies in any language; using json_schemer/jsonschema/santhosh-tekuri/networknt would create divergent extras semantics across runners.
  • Three startup gates per runner: decoder coverage, snapshot completeness, snapshot recognition. Missing top-level operation is a hard failure pointing at re-running the TS canary.
  • Make targets: conformance-{ruby,python,go,kotlin}-replay, each with required-env preflight (WIRE_REPLAY_DIR, BASECAMP_BACKEND). None run as part of make check.
  • CONTRIBUTING.md gains a "Wire replay (cross-language)" subsection covering the two-stage flow and the five files to update when adding a new operation.

Verification

Cross-language walker parity confirmed end-to-end against synthetic snapshots covering all 10 ops in live-my-surface.json:

$ for lang in ruby python go kotlin; do
    jq -s '[.[].pages[].missing_required] | flatten | sort | unique' \
      tmp/canary/bc4/decode/$lang/*.json > /tmp/missing-$lang.json
  done
$ diff /tmp/missing-ruby.json /tmp/missing-python.json   # empty
$ diff /tmp/missing-ruby.json /tmp/missing-go.json       # empty
$ diff /tmp/missing-ruby.json /tmp/missing-kotlin.json   # empty

The synthetic GetProject {} body produces byte-identical missing_required ([app_url, created_at, id, name, status, updated_at, url]) across all four languages.

schema_version lock:

$ jq -s '[.[].schema_version] | unique' tmp/canary/bc4/decode/*/*.json
[1]

All four mock baselines preserved exactly: Ruby 59, Python 64, Go 68, Kotlin 58 (counts match those documented in #294's plan).

Coverage gate failure modes verified by injection (missing decoder, missing snapshot, unknown operation, pre-PR3 snapshot lacking operation field) — each emits an actionable message pointing at the right side to fix (TS dispatch, fixture, or this runner).

Test plan

  • make conformance-ruby shows 59/0/9
  • make conformance-python shows 64/0/4
  • make conformance-go shows 68/0/0
  • make conformance-kotlin shows 58/0/10
  • make conformance-typescript passes (TS amendment is back-compat)
  • WIRE_REPLAY_DIR=<dir> BASECAMP_BACKEND=bc4 make conformance-<lang>-replay for each language fails preflight when env unset, succeeds against TS-captured snapshots
  • All four runners reject pre-PR3 snapshots (no operation field) with a clear "Re-run the TS live canary" message

Sequencing

PR 4 (pairwise BC4↔BC5 comparison + check-bc5-compat orchestrator + scheduled CI) consumes both the TS canary's <backend>/wire/ snapshots and these decode-result snapshots; it lands separately.


Summary by cubic

Adds cross-language wire-replay for Ruby, Python, Go, and Kotlin using TS-captured snapshots. TS now persists operation; runners decode via production code paths, report missing required fields and extras, and write per-test results for BC5 readiness.

  • New Features

    • TS live runner persists operation on wire snapshots for replay dispatch.
    • Ruby/Python/Go/Kotlin replay runners read <dir>/<backend>/wire/*.json, decode via SDK production paths, and write to <dir>/<backend>/decode/<lang>/ with schema_version: 1. Walkers emit missing_required (slash paths) and extras_seen (dot paths). Kotlin decodes four My*/notifications ops as JsonElement for now.
    • Startup gates: decoder coverage, snapshot completeness, and snapshot recognition (requires top-level operation). Make targets: conformance-{ruby,python,go,kotlin}-replay (Kotlin uses :conformance:runReplay). CONTRIBUTING and .gitignore updated.
  • Bug Fixes

    • Faithful empty-body decoding: Ruby now errors on empty bodyText; Python/Go distinguish missing vs empty bodyText so empty bodies trigger decode errors.
    • Gates hardened and diagnostics improved: all runners surface snapshot read/parse errors; Python reports malformed UTF-8; Kotlin treats blank env vars as missing, defensively reads malformed/non-primitive operation and bodyText, narrows decoder catch to Exception, and sorts snapshot files for stable messages.
    • Go snapshot validation: reject missing/empty pages and pages_count mismatches to prevent silent green passes.
    • Response schema lookup precedence aligned across TS and all walkers: prefer 200–204, then any other 2xx, then default.
    • TS snapshot persistence: keep operation authoritative when serializing snapshots.

Written for commit 0d64a65. Summary will update on new commits.

Review in cubic

Copilot AI review requested due to automatic review settings May 4, 2026 23:50
@github-actions github-actions Bot added documentation Improvements or additions to documentation dependencies Pull requests that update a dependency file kotlin conformance Conformance test suite enhancement New feature or request and removed documentation Improvements or additions to documentation labels May 4, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 15 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt">

<violation number="1" location="kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt:211">
P2: Catch `Exception` instead of `Throwable` so fatal JVM errors are not swallowed as decode failures.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the BC5-readiness conformance suite with cross-language wire-replay decoders: TypeScript remains the sole live HTTP runner that captures canonical wire snapshots, while Ruby/Python/Go/Kotlin add opt-in replay modes that decode those snapshots through their SDK boundaries and emit per-test decode-result snapshots for required-field + extras detection.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Changes:

  • Add wire-replay runners + OpenAPI schema walkers for Ruby, Python, Go, and Kotlin, producing <WIRE_REPLAY_DIR>/<backend>/decode/<lang>/... outputs.
  • Amend TS live snapshot persistence to include a top-level operation for replay dispatch.
  • Add Makefile replay targets and contributor documentation for the two-stage flow.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
Makefile Adds conformance-*-replay targets with env-var preflight and help text updates.
kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt Kotlin OpenAPI schema walker for missing_required + extras_seen.
kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt Kotlin wire-replay runner that decodes snapshots and writes decode-result JSON.
kotlin/conformance/build.gradle.kts Adds Gradle :conformance:runReplay JavaExec task.
CONTRIBUTING.md Documents the cross-language wire replay flow and required update points when adding operations.
conformance/runner/typescript/wire-capture.ts Documents and types the persisted snapshot format (operation + pages).
conformance/runner/typescript/live-runner.test.ts Persists snapshots with top-level operation for replay dispatch.
conformance/runner/ruby/schema-walker.rb Ruby OpenAPI schema walker used by the replay runner.
conformance/runner/ruby/replay-runner.rb Ruby wire-replay runner that decodes snapshots and emits decode-result JSON.
conformance/runner/python/schema_walker.py Python OpenAPI schema walker used by the replay runner.
conformance/runner/python/replay_runner.py Python wire-replay runner that decodes snapshots and emits decode-result JSON.
conformance/runner/go/schema_walker.go Go OpenAPI schema walker used by the replay runner.
conformance/runner/go/replay_runner.go Go wire-replay runner that decodes snapshots and emits decode-result JSON.
conformance/runner/go/main.go Adds a WIRE_REPLAY_DIR mode-gate to dispatch to the replay runner.
.gitignore Ignores Python conformance runner virtualenv + bytecode artifacts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread kotlin/conformance/build.gradle.kts Outdated
Comment thread kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt Outdated
Comment thread conformance/runner/ruby/schema-walker.rb
Comment thread conformance/runner/go/schema_walker.go Outdated
Comment thread conformance/runner/ruby/replay-runner.rb Outdated
@jeremy
jeremy requested a review from Copilot May 8, 2026 21:35
@github-actions github-actions Bot added documentation Improvements or additions to documentation and removed documentation Improvements or additions to documentation labels May 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.

Comment thread conformance/runner/ruby/schema-walker.rb Outdated
Comment thread conformance/runner/go/schema_walker.go Outdated
@jeremy
jeremy requested a review from Copilot May 12, 2026 16:06
@github-actions github-actions Bot added documentation Improvements or additions to documentation and removed documentation Improvements or additions to documentation labels May 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Comment thread conformance/runner/python/replay_runner.py Outdated
@jeremy
jeremy requested a review from Copilot May 13, 2026 18:42
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label May 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.

Comment thread conformance/runner/ruby/schema-walker.rb Outdated
Comment thread conformance/runner/python/schema_walker.py
Comment thread conformance/runner/go/schema_walker.go
Comment thread kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt Outdated
@jeremy
jeremy requested a review from Copilot May 13, 2026 19:57
@github-actions github-actions Bot removed the documentation Improvements or additions to documentation label May 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 16 changed files in this pull request and generated 2 comments.

Comment thread conformance/runner/ruby/replay-runner.rb
Comment thread kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt">

<violation number="1" location="kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt:66">
P2: `default` is checked after generic `2xx`, which violates the expected response-schema precedence and can select the wrong schema when both exist.

(Based on your team's feedback about OpenAPI response schema resolution order.) [FEEDBACK_USED]</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.

@jeremy
jeremy requested a review from Copilot May 13, 2026 20:12
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label May 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.

jeremy added a commit that referenced this pull request Jul 14, 2026
Both come in transitively via openapi-typescript → @redocly/openapi-core
(fast-uri through @redocly/ajv, brace-expansion through minimatch).
npm audit on the affected versions reports:

  fast-uri ≤3.1.1 — high — path traversal + host confusion
  (GHSA-q3j6-qgpj-74h6 / GHSA-v39h-62p7-jpjc)
  brace-expansion 4.0.0–5.0.4 — moderate — zero-step sequence
  hangs the process (GHSA-f886-m6hf-6m8v)

Lift both to the patched majors via overrides (alongside the existing
minimatch pin). The override path is preferred over plain dependency
bumps because the SDK doesn't depend on either package directly —
the responsibility for forwarding the fix sits with this repo until
the upstream packages cut new releases that pull the patches in
transitively.

Lands at the bottom of the BC5 stack so the wire-replay PRs above
(#294, #301) inherit the fix on rebase.
@jeremy
jeremy force-pushed the bc5-readiness-ts-live-canary branch from 6e78222 to 1a40114 Compare July 14, 2026 02:36
jeremy added 19 commits July 13, 2026 19:37
Replay runners (PR 3) need the operation name to dispatch a decoder; without
it on the snapshot top level, every replayer would have to re-parse the live
fixture and rebuild the slug→operation map. Adding it once at capture time
keeps the contract single-sourced.
Reads canonical wire snapshots written by the TS live canary, decodes each
page through the Ruby SDK, and walks the raw JSON for required-field +
extras detection. Output: per-test decode-result snapshots under
<dir>/<backend>/decode/ruby/.

Decoder boundary is the SDK's actual production path (JSON.parse +
normalize_person_ids) — same code every paginator runs on every page, so
normalize-layer regressions surface as decode_error here.

The schema walker is a hand-rolled port of the TS validator's required +
extras algorithm. No new gem dependencies; using json_schemer would create
divergent "what counts as extra" semantics across languages against an
OpenAPI doc that uses JSON Reference, not pure JSON Schema.

Three startup gates: decoder coverage, snapshot completeness, snapshot
recognition. Each prints actionable messages to stderr and exits 1 before
any decode work; a missing top-level operation field is treated as a
hard failure pointing at re-running the TS canary, not silently skipped.

Mock conformance unchanged: 59 passed, 0 failed, 9 skipped.
Reads canonical wire snapshots from the TS live canary, decodes each page
through the Python SDK's deserialize-and-normalize path, and walks the
raw JSON for required-field + extras detection. Output: per-test
decode-result snapshots under <dir>/<backend>/decode/python/.

Decoder boundary is json.loads + basecamp.generated.services._base.
_normalize_person_ids — the same code path every SDK request runs in
production. Any normalize-layer regression (new personable shape, etc.)
surfaces here as decode_error.

The schema walker is a hand-rolled port of the TS validator's required +
extras algorithm. No new dependencies; pyproject.toml untouched.

Three startup gates: decoder coverage, snapshot completeness, snapshot
recognition. Missing top-level operation is a hard failure pointing at
re-running the TS canary.

Mock conformance unchanged: 64 passed, 0 failed, 4 skipped.
Reads canonical wire snapshots from the TS live canary, decodes each page
through the typed go/pkg/generated response types, and walks the raw JSON
for required-field + extras detection. Output: per-test decode-result
snapshots under <dir>/<backend>/decode/go/.

Decoder boundary is generated.<Op>ResponseContent across all 10 ops —
the same internal types oapi-codegen produces. Unexported mappers
(projectFromGenerated, etc.) intentionally stay private; the replay runner
imports the generated package directly so future maintainers don't try to
"fix" the import.

main()'s mode gate: when WIRE_REPLAY_DIR is set, delegate to
ReplayRunner.Run() and exit; otherwise fall through to the existing
mock-mode logic, which is untouched. Go's one-main()-per-package
constraint makes a separate package's worth of cross-wiring a worse
option than this small prefix.

Walker is a hand-rolled port of the TS algorithm; map-iteration order is
sorted alphabetically for stable cross-language extras-parity diffing.

Three startup gates: decoder coverage, snapshot completeness, snapshot
recognition. Missing top-level operation is a hard failure pointing at
re-running the TS canary.

Mock conformance unchanged: 68 passed, 0 failed, 0 skipped.
Reads canonical wire snapshots from the TS live canary, decodes each page
through the Kotlin SDK's @serializable data classes, and walks the parsed
JsonElement for required-field + extras detection. Output: per-test
decode-result snapshots under <dir>/<backend>/decode/kotlin/.

The replay-mode Json mirrors the existing mock-mode Json
(ignoreUnknownKeys = true, coerceInputValues = false): additive BC5
fields decode without error. This is a forward-compat canary, not a
strictness audit. Type-mismatches and missing required (non-nullable)
fields still throw and surface as decode_error. Extras detection comes
from the schema walker on the parsed JsonElement, so unknown wire
fields surface even though the decoder ignores them.

Four ops (assignment families + notifications) currently decode through
JsonElement because the SDK doesn't yet expose typed MyAssignment /
Notification models — TODO(pr3-followup) to swap to typed serializers
when those models land. The walker output is the cross-language signal
either way.

Entry point is a separate :conformance:runReplay Gradle task, not a
mode-gate in Main.kt — leaves the existing :conformance:run path
untouched. No new dependencies in build.gradle.kts.

Three startup gates: decoder coverage, snapshot completeness, snapshot
recognition. Missing top-level operation is a hard failure pointing at
re-running the TS canary.

Mock conformance unchanged: 58 passed, 0 failed, 10 skipped.
Adds conformance-{ruby,python,go,kotlin}-replay targets, each with
required-env preflight (WIRE_REPLAY_DIR, BASECAMP_BACKEND). None of these
run as part of make check — they consume snapshots from the TS live
canary and are opt-in.

CONTRIBUTING gains a "Wire replay (cross-language)" subsection that
describes the two-stage flow (TS captures → other languages decode), the
three startup gates each runner enforces, and the five files to update
when adding a new operation to live-my-surface.json.

gitignore: cover conformance/runner/python/{.venv,__pycache__,*.pyc,uv.lock}
so test runs don't leave untracked files behind.

The orchestrating make conformance-live and make check-bc5-compat targets
that thread TS capture → all four replays → BC4↔BC5 comparison together
land in PR 4.
…e doc

Required-field paths now use `/` as the separator (extras_seen still uses
`.`) so the two streams are visually distinct in tooling and consistent
across Ruby/Python/Go/Kotlin walkers. Verified end-to-end against a
nested-required synthetic (Project.dock[0] missing all required fields):
all four languages now emit byte-identical paths like `dock[0]/id`,
`dock[0]/title`, etc.

Header-comment fix: the script aborts when WIRE_REPLAY_DIR/BASECAMP_BACKEND
are missing; it is not a no-op. Updated the comment to reflect the
fail-fast behavior so future readers don't try to invoke it bare.
Required-field paths now use `/` as the separator (walkExtras still uses
`.`) so the two streams are visually distinct in tooling and consistent
across Ruby/Python/Go/Kotlin walkers. New joinSlash helper sits beside
joinDot; walkRequired switches to it; walkExtras is unchanged.
Three reviewer-noted hardenings on ReplayRunner + its Gradle task:

build.gradle.kts: drop the explicit environment("WIRE_REPLAY_DIR" / ...)
calls. JavaExec inherits the parent process env by default; the previous
`?: ""` fallbacks were smuggling empty strings into the child env, which
let the runner read/write relative to CWD or `<dir>/<empty>/...` instead
of failing fast on the missing-env gate.

ReplayRunner.kt main(): treat blank values as missing too (`takeUnless
{ it.isBlank() }`). Defense in depth — covers shells/launchers that pass
empty strings through even when the build script doesn't.

ReplayRunner.kt decode loop: catch Exception, not Throwable. Fatal JVM
errors (OOM, StackOverflow, LinkageError) shouldn't be silently demoted
to a decode-failure record; they should propagate. Same change applied to
the body-not-parseable-JSON catch for consistency.
…arator

3d771f0 / 04e9eea switched the required-walk path separator to `/`
(matching Python/Kotlin) but left both walkers' module + function
docstrings saying "dotted-path strings" for required output. Update the
docs to match the code: required walks emit "owner/id"; extras walks
still emit "owner.new_field". Adds an explicit conventions line calling
out the two-separator split so future maintainers don't re-introduce the
drift.

No code changes.
The Python and Go replay runners were using truthiness/zero-value checks
to detect a missing `bodyText` field on wire snapshots, which conflated
a missing field with an empty-but-present one. A genuinely-empty body
(HTTP 204, or a 200 with `""`) would be silently replaced with a
re-serialized `body: null` → `"null"`, which `json.loads`/`json.Unmarshal`
parse successfully, turning a real decode failure into a silent green.

Python: distinguish None from empty string explicitly.

  raw = page.get("bodyText")
  body_text: str = raw if raw is not None else json.dumps(page["body"])

Go: change `BodyText string` to `BodyText *string` on `wirePage` so
encoding/json distinguishes a missing key (nil) from an empty value
(&""). The body-serialization fallback survives only for legacy
snapshots that lack `bodyText` entirely; new TS-canary captures always
set the field (even to `""`).

Ruby and Kotlin already handle this correctly (Ruby `""` is truthy;
Kotlin `contentOrNull` returns `""` for empty primitives, only nulling
on `JsonNull`), so they need no change.
The response-schema lookup in all five walkers (TS, Ruby, Python, Go,
Kotlin) put "default" inside the explicit candidate list, which meant
the actual precedence was "200, 201, 202, 203, 204, default, then any
other 2xx" — but every comment described it as "200, then any 2xx,
then default" (Copilot called this out on the three new walkers).

Behavior now matches the comments: check the common explicit 2xx codes
first, then scan for any other 2xx key, then fall back to "default" as
the absolute last resort. Zero behavior change for any current OpenAPI
operation in this repo — no operation combines "default" with a 2xx
key outside 200..204 — but the lookup is now unambiguous if one is
ever added.
Three related fixes prompted by review feedback on the wire-replay
runners, plus regression tests pinning the boundary.

Ruby — `SDK_DECODE` had `return nil if body_text.empty?` that silently
green-passed empty wire payloads, diverging from the production SDK
(`Basecamp::Http#json` calls `JSON.parse(@Body)` without an empty-body
guard, so empty bodies raise `JSON::ParserError`). Drop the guard so
the runner mirrors production.

Python / Go — extract `_resolve_body_text` / `resolveBodyText` helpers
so the empty-vs-missing distinction (already fixed inline in a1bd6df)
is testable without standing up a full ReplayRunner. Regression tests
assert: empty bodyText passes through as `""`, missing bodyText falls
back to a serialized body, a non-empty bodyText wins over body, and
the decoder errors on an empty bodyText.

Ruby — minimal Minitest regression test against `SDK_DECODE` directly,
adding `minitest` to the Gemfile so `bundle exec ruby` can run it.

Kotlin — `coverageGate()` extracted the snapshot's `operation` via
`snap["operation"]?.jsonPrimitive?.contentOrNull`, which throws if the
value is non-primitive (JsonNull, JsonObject, JsonArray) and crashes
the gate with a stack trace instead of emitting the intended
"snapshot missing operation field" message. Cast defensively with
`(snap["operation"] as? JsonPrimitive)?.contentOrNull` so a malformed
snapshot fails the gate cleanly.
The conformance/runner/go module builds to a binary named `go` (after
the module's last path component) when developers run `go build`
locally without `-o`. The existing entry covers `conformance-runner`
(a previous build-target name); add the current default so the binary
doesn't show up as untracked in working trees.
Copilot flagged that the Go replay runner's coverage-gate
snapshot-recognition pass silently `continue`'d on `ReadFile` and
`json.Unmarshal` errors, so a corrupted snapshot would slip past
the gate and produce a confusing later failure. Audit revealed
parallel patterns across the four runners:

  Go     — silent-continue on read AND parse errors.
  Kotlin — silent skip on non-JsonObject root (`as? JsonObject ?: return`);
           uncaught throw on parse failure.
  Ruby   — uncaught throw on parse/read failure.
  Python — uncaught throw on parse/read failure.

Bring all four into the same shape: emit a gate message naming the
offending snapshot file and continue to the next, so the runner exits
with `Run() => 1` and a complete list of malformed inputs rather than
crashing on the first one or — worse — silently shrinking the verified
set.
Cubic flagged that the Python coverage-gate snapshot reader at
replay_runner.py:128 caught `OSError` and `json.JSONDecodeError`
but not `UnicodeDecodeError`. `Path.read_text()` decodes UTF-8 by
default, and malformed bytes raise `UnicodeDecodeError` — a
`ValueError`, not an `OSError` — so a corrupt snapshot crashed
the gate instead of emitting a clear diagnostic.

Widen the `except` chain to record "Snapshot X is not valid
UTF-8: …" alongside the existing read/parse messages, and add a
regression test that drives `coverage_gate()` against a tempdir
snapshot starting with `\xff\xfe`.

Cross-language audit for symmetry (verified empirically):

  Ruby   — `File.read` does not enforce UTF-8 at read time;
           `JSON.parse` raises `JSON::ParserError` on invalid
           bytes. Already caught.
  Go     — `os.ReadFile` returns raw bytes; `json.Unmarshal`
           raises `*json.SyntaxError` on invalid bytes. Already
           caught.
  Kotlin — JVM `InputStreamReader` (under `File.readText()`)
           silently replaces invalid bytes with U+FFFD; any
           resulting non-JSON surfaces as `SerializationException`.
           Already caught.

Only Python needed the fix; the other three runners reach the
same outcome via their existing handlers.
Same review pass, four small hardening fixes:

  Kotlin ReplayRunner.kt:221
    `decodeSnapshot` reads `page["bodyText"]?.jsonPrimitive` directly,
    which throws if the underlying `JsonElement` is `JsonNull`,
    `JsonObject`, or `JsonArray` — crashing the runner before it can
    record `decode_error`. Mirror the operation-field defensive cast
    in `coverageGate()`: `as? JsonPrimitive` first, then `contentOrNull`.

  Kotlin ReplayRunner.kt:147
    `wireDir.listFiles(...)` order is filesystem-dependent, making gate
    diagnostics nondeterministic across machines/CI shards. Sort by
    name to match the Python/Ruby/Go runners.

  Go replay_runner.go:313
    A snapshot like `{"operation":"GetProject"}` unmarshals cleanly
    with `Pages == nil`; `decodeSnapshot` loops zero times and `Run()`
    records zero failures — a silent green-pass with no decode
    actually attempted. Reject missing/empty `pages` and
    pages_count-vs-len(pages) mismatches in `readSnapshot`. Adds four
    table-style regression tests (missing pages, empty pages,
    mismatched count, matching count).

  Go replay_runner_test.go:58
    Dead `*json.SyntaxError` variable (`var se ...; _ = se`) discarded
    without use. Drop the line and the now-unused `encoding/json`
    import.

Verified locally:
  go test ./conformance/runner/go/...        → 8 pass
  make conformance-go                        → 68 / 0 / 0
  make conformance-kotlin                    → 58 / 0 / 10 (skips pre-existing)
…pread

Copilot flagged that `persistSnapshot` built the payload as
`{ operation, ...snapshot }`. `WireSnapshot` doesn't currently
carry an `operation` key, so the behavior is correct today —
but the spread order means a future addition to `WireSnapshot`
with a same-named field would silently override the parameter
the caller passed in. Flip to `{ ...snapshot, operation }` so
the explicit argument wins.

Single one-liner; no behavior change for any existing snapshot.
Copilot AI review requested due to automatic review settings July 14, 2026 02:41
@jeremy
jeremy force-pushed the bc5-readiness-wire-replay-decoders branch from bb85531 to 0d64a65 Compare July 14, 2026 02:41
@jeremy

jeremy commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

Rebased onto the rebased #294 (1a401145, which sits on the rebased #293 → current main). Clean replay, 19 commits preserved, no content change. Mock conformance re-verified at baseline across all runners: TS 84 passed / 6 skipped, Ruby 59/0/9, Python 64/0/4, Go 68/0/0, Kotlin 58/0/10. The rebase picks up main's CI fixes (#363: Ruby-head bundler pin, govulncheck go1.26.5, webrick removal) and #294's fast-uri 3.1.3 bump, so the previously repo-wide red checks should now be green here.

@github-actions github-actions Bot removed the documentation Improvements or additions to documentation label Jul 14, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.

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

Labels

conformance Conformance test suite dependencies Pull requests that update a dependency file enhancement New feature or request kotlin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants