From 48881543d0a5cbf6cabfe02bffee7d7ca2ed062b Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 4 May 2026 12:09:55 -0700 Subject: [PATCH 01/19] conformance(ts): persist operation field on wire snapshots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../runner/typescript/live-runner.test.ts | 16 ++++++++++++---- conformance/runner/typescript/wire-capture.ts | 14 ++++++++++++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/conformance/runner/typescript/live-runner.test.ts b/conformance/runner/typescript/live-runner.test.ts index 9cfce685..4ce5e857 100644 --- a/conformance/runner/typescript/live-runner.test.ts +++ b/conformance/runner/typescript/live-runner.test.ts @@ -20,7 +20,12 @@ import * as fs from "node:fs"; import * as path from "node:path"; import { fileURLToPath } from "node:url"; -import { installWireCapture, type WireSnapshot, type WirePage } from "./wire-capture.js"; +import { + installWireCapture, + type WireSnapshot, + type WirePage, + type PersistedWireSnapshot, +} from "./wire-capture.js"; import { validateResponse, type ValidationResult } from "./schema-validator.js"; import { LIVE_OPERATIONS, @@ -97,13 +102,16 @@ function recordExtras(operation: string, extras: string[]): void { for (const e of extras) bucket.add(e); } -function persistSnapshot(testName: string, snapshot: WireSnapshot): void { +function persistSnapshot(testName: string, operation: string, snapshot: WireSnapshot): void { if (!RECORD_DIR) return; const wireDir = path.join(RECORD_DIR, BACKEND, "wire"); fs.mkdirSync(wireDir, { recursive: true }); const safeName = testName.replace(/[^a-z0-9_-]+/gi, "_"); const file = path.join(wireDir, `${safeName}.json`); - fs.writeFileSync(file, JSON.stringify(snapshot, null, 2)); + // Top-level `operation` lets replay runners (PR 3) dispatch without + // re-parsing the live-my-surface fixture. + const payload: PersistedWireSnapshot = { operation, ...snapshot }; + fs.writeFileSync(file, JSON.stringify(payload, null, 2)); } function checkRequiredFields(page: WirePage, fields: string[]): string[] { @@ -214,7 +222,7 @@ LIVE_DESCRIBE("conformance live runner", () => { } const snapshot = capture.drain(); - persistSnapshot(tc.name, snapshot); + persistSnapshot(tc.name, tc.operation, snapshot); if (dispatchError) { throw new Error(`SDK dispatch threw: ${dispatchError.message}`); diff --git a/conformance/runner/typescript/wire-capture.ts b/conformance/runner/typescript/wire-capture.ts index 05ceb1ee..78847f45 100644 --- a/conformance/runner/typescript/wire-capture.ts +++ b/conformance/runner/typescript/wire-capture.ts @@ -7,8 +7,13 @@ * we can clone the response and record its raw bytes/headers without * affecting SDK behavior. * - * Snapshot format per test: - * { pages: [{status, headers, body}, ...], pages_count: N } + * Snapshot format per test (persisted by live-runner): + * { operation: "GetProject", pages: [{status, headers, body, ...}, ...], pages_count: N } + * + * The top-level `operation` field is added by the runner when serializing, + * so cross-language replay runners (PR 3) can dispatch without re-parsing + * the test fixture. Backwards compatible — existing readers ignore unknown + * top-level keys. * * Schema validation runs per page; extras-observed reporting unions extras * across all pages. @@ -31,6 +36,11 @@ export interface WireSnapshot { pages_count: number; } +/** Snapshot as persisted to disk: WireSnapshot plus the operation that produced it. */ +export interface PersistedWireSnapshot extends WireSnapshot { + operation: string; +} + export interface WireCaptureSession { /** Restore the original fetch. Idempotent. */ restore(): void; From 61f481bd8c5d73fa6949eb3fe86232522736ec75 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 4 May 2026 12:10:15 -0700 Subject: [PATCH 02/19] conformance(ruby): wire-replay runner + schema walker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 //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. --- conformance/runner/ruby/replay-runner.rb | 198 +++++++++++++++++++++++ conformance/runner/ruby/schema-walker.rb | 167 +++++++++++++++++++ 2 files changed, 365 insertions(+) create mode 100644 conformance/runner/ruby/replay-runner.rb create mode 100644 conformance/runner/ruby/schema-walker.rb diff --git a/conformance/runner/ruby/replay-runner.rb b/conformance/runner/ruby/replay-runner.rb new file mode 100644 index 00000000..32595370 --- /dev/null +++ b/conformance/runner/ruby/replay-runner.rb @@ -0,0 +1,198 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Wire-replay runner for the Ruby SDK conformance suite. +# +# Reads snapshots written by the canonical TS live runner (see +# conformance/runner/typescript/live-runner.test.ts), decodes each page +# through the Ruby SDK's decode boundary, and writes a per-test +# decode-result snapshot under //decode/ruby/. +# +# Mode-gate: this runner only does anything when WIRE_REPLAY_DIR is set. +# When unset, the existing mock runner.rb handles things and this file is a +# no-op if invoked. The make target only invokes this when WIRE_REPLAY_DIR +# is set. + +require "bundler/setup" +require "basecamp" +require "json" +require "fileutils" +require_relative "schema-walker" + +class ReplayRunner + SCHEMA_VERSION = 1 + + # Maps operation_id -> proc(body_text) -> raises on parse/decode failure. + # + # The Ruby SDK has no typed deserializers — every response method ultimately + # calls JSON.parse(body) followed by Http.normalize_person_ids(result), then + # returns the resulting Hash/Array unchanged. That pipeline (the contents + # of RequestResult#json plus the post-processing the paginators apply on + # every page) is the canonical decoder boundary, so each lambda exercises + # exactly that. + SDK_DECODE = lambda do |body_text| + return nil if body_text.nil? || body_text.empty? + + parsed = JSON.parse(body_text) + Basecamp::Http.normalize_person_ids(parsed) + parsed + end + + DECODERS = { + "ListProjects" => SDK_DECODE, + "GetProject" => SDK_DECODE, + "GetMyAssignments" => SDK_DECODE, + "GetMyCompletedAssignments" => SDK_DECODE, + "GetMyDueAssignments" => SDK_DECODE, + "GetMyNotifications" => SDK_DECODE, + "GetMyProfile" => SDK_DECODE, + "GetTodoset" => SDK_DECODE, + "ListTodolists" => SDK_DECODE, + "ListTodos" => SDK_DECODE, + }.freeze + + def initialize(replay_dir, backend, fixture_path, openapi_path) + @replay_dir = replay_dir + @backend = backend + @fixture_path = fixture_path + @walker = Basecamp::Conformance::SchemaWalker.new(openapi_path) + @fixture = JSON.parse(File.read(fixture_path)).select { |t| t["mode"] == "live" } + end + + def run + fail_messages = coverage_gate + if fail_messages.any? + warn fail_messages.join("\n") + return 1 + end + + out_dir = File.join(@replay_dir, @backend, "decode", "ruby") + FileUtils.mkdir_p(out_dir) + + failures = 0 + @fixture.each do |test| + snapshot = read_snapshot(test["name"]) + result = decode_snapshot(snapshot) + File.write(File.join(out_dir, "#{safe_name(test["name"])}.json"), JSON.pretty_generate(result)) + failures += 1 if result[:pages].any? { |p| !p[:decoded] || p[:missing_required].any? } + end + + failures.zero? ? 0 : 1 + end + + private + + def coverage_gate + msgs = [] + fixture_ops = @fixture.map { |t| t["operation"] }.uniq + + # 1. Decoder coverage: every fixture operation has a decoder. + missing_decoders = fixture_ops.reject { |op| DECODERS.key?(op) } + if missing_decoders.any? + msgs << "Ruby replay runner missing decoders for: #{missing_decoders.join(", ")}. " \ + "Add to DECODERS in replay-runner.rb." + end + + # 2. Snapshot completeness: every fixture op has a snapshot file. + wire_dir = File.join(@replay_dir, @backend, "wire") + @fixture.each do |t| + f = File.join(wire_dir, "#{safe_name(t["name"])}.json") + next if File.exist?(f) + + # Per-test skipReason files are not part of the PR2 contract today; + # treat missing snapshots as runner failure with a clear pointer. + msgs << "Snapshot missing for operation #{t["operation"]} (test #{t["name"]}); " \ + "expected at #{f}. Re-run TS live capture or check skip status." + end + + # 3. Snapshot recognition: every snapshot's operation is in the fixture. + if Dir.exist?(wire_dir) + Dir.glob(File.join(wire_dir, "*.json")).each do |f| + snap = JSON.parse(File.read(f)) + op = snap["operation"] + + if op.nil? + msgs << "Snapshot #{File.basename(f)} is missing the top-level `operation` field. " \ + "Re-run the TS live canary; pre-PR3 snapshots are no longer supported." + next + end + + unless fixture_ops.include?(op) + msgs << "Unknown operation #{op.inspect} in snapshot #{File.basename(f)}; " \ + "TS dispatch table appears to have drifted from live-my-surface.json." + end + end + end + + msgs + end + + def read_snapshot(test_name) + path = File.join(@replay_dir, @backend, "wire", "#{safe_name(test_name)}.json") + JSON.parse(File.read(path)) + end + + def decode_snapshot(snapshot) + operation = snapshot["operation"] + decoder = DECODERS[operation] + schema = @walker.find_response_schema(operation) + + pages = snapshot["pages"].map do |page| + decode_page(page, operation, decoder, schema) + end + + { schema_version: SCHEMA_VERSION, operation: operation, pages: pages } + end + + def decode_page(page, operation, decoder, schema) + body_text = page["bodyText"] || JSON.generate(page["body"]) + decoded = false + decode_error = nil + + begin + decoder.call(body_text) + decoded = true + rescue StandardError => e + decode_error = "#{e.class}: #{e.message}" + end + + missing_required = [] + extras_seen = [] + + if schema + # Per the TS validator: walk against parsed JSON, not the SDK-decoded + # structure (decoders may drop unknown fields). A page whose body was + # not parseable JSON gets empty arrays here — the decoded:false above + # already captures the failure. + body = page["body"] + body = (JSON.parse(body_text) rescue nil) if body.is_a?(String) || body.nil? + if body.is_a?(Hash) || body.is_a?(Array) + missing_required = @walker.missing_required(body, schema) + extras_seen = @walker.extras_seen(body, schema) + end + end + + { + decoded: decoded, + decode_error: decode_error, + missing_required: missing_required, + extras_seen: extras_seen, + } + end + + def safe_name(name) + name.gsub(/[^a-z0-9_-]+/i, "_") + end +end + +if __FILE__ == $PROGRAM_NAME + replay_dir = ENV["WIRE_REPLAY_DIR"] + backend = ENV["BASECAMP_BACKEND"] + abort "WIRE_REPLAY_DIR is required" if replay_dir.nil? || replay_dir.empty? + abort "BASECAMP_BACKEND is required" if backend.nil? || backend.empty? + + fixture_path = File.expand_path("../../tests/live-my-surface.json", __dir__) + openapi_path = File.expand_path("../../../openapi.json", __dir__) + + exit ReplayRunner.new(replay_dir, backend, fixture_path, openapi_path).run +end diff --git a/conformance/runner/ruby/schema-walker.rb b/conformance/runner/ruby/schema-walker.rb new file mode 100644 index 00000000..35ebcc82 --- /dev/null +++ b/conformance/runner/ruby/schema-walker.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require "json" + +# Pure-Ruby port of conformance/runner/typescript/schema-validator.ts. +# +# Walks parsed JSON against the OpenAPI response schema, surfacing: +# - missing_required: required-field paths absent from the body +# - extras_seen: field paths present on the wire but not declared in the schema +# +# Conventions match the TS walker exactly so cross-language replay output is +# directly comparable: +# - Object paths use "." (e.g. "owner.id") +# - Array element paths use "[i]" for required walk, "[]" for extras walk +# (mirrors how the TS validator's collectExtras tags item-level extras and +# how a per-index required-field check identifies the offending element). +# - "$ref" chains resolve until a non-ref schema or a cycle. Both +# "#/components/schemas/X" and "openapi.json#/components/schemas/X" are +# accepted. +# - additionalProperties:false is intentionally ignored — extras are reported +# but do not fail validation (forward-compat). +# - Bounded recursion depth (12) as a cycle guard. +module Basecamp + module Conformance + class SchemaWalker + MAX_DEPTH = 12 + + def initialize(openapi_path) + @doc = JSON.parse(File.read(openapi_path)) + end + + # Returns the response schema for operation_id, or nil when none exists. + # Prefers 200, then any explicit 2xx, then "default". + def find_response_schema(operation_id) + candidates = %w[200 201 202 203 204 default] + @doc.fetch("paths", {}).each_value do |path_item| + path_item.each_value do |op| + next unless op.is_a?(Hash) && op["operationId"] == operation_id + + responses = op["responses"] || {} + candidates.each do |code| + schema = responses.dig(code, "content", "application/json", "schema") + return schema if schema + end + responses.each do |code, response| + next unless code.is_a?(String) && code =~ /\A2\d\d\z/ + + schema = response.dig("content", "application/json", "schema") + return schema if schema + end + end + end + nil + end + + # Returns array of dotted-path strings for required fields absent from body. + def missing_required(body, schema) + walk_required("", body, schema, 0) + end + + # Returns array of dotted-path strings for fields present on the wire but + # not declared in the schema. Recurses through known properties so item- + # level extras on lists surface (e.g. "unreads[].new_field"). + def extras_seen(body, schema) + walk_extras("", body, schema, 0) + end + + private + + def walk_required(prefix, body, schema, depth) + return [] if depth > MAX_DEPTH + + resolved = resolve_ref(schema) + return [] unless resolved.is_a?(Hash) + + if body.is_a?(Array) + return [] unless resolved["type"] == "array" && resolved["items"] + + missing = [] + body.each_with_index do |item, i| + child_prefix = prefix.empty? ? "[#{i}]" : "#{prefix}[#{i}]" + missing.concat(walk_required(child_prefix, item, resolved["items"], depth + 1)) + end + return missing + end + + return [] unless body.is_a?(Hash) + return [] if resolved["type"] && resolved["type"] != "object" + + props = resolved["properties"] || {} + required = resolved["required"] || [] + missing = [] + + required.each do |name| + unless body.key?(name) + field_path = prefix.empty? ? name : "#{prefix}.#{name}" + missing << field_path + end + end + + body.each do |key, value| + next unless props.key?(key) + + field_path = prefix.empty? ? key : "#{prefix}.#{key}" + missing.concat(walk_required(field_path, value, props[key], depth + 1)) + end + + missing + end + + def walk_extras(prefix, body, schema, depth) + return [] if depth > MAX_DEPTH + return [] if body.nil? + + resolved = resolve_ref(schema) + return [] unless resolved.is_a?(Hash) + + if body.is_a?(Array) + return [] unless resolved["type"] == "array" && resolved["items"] + + seen = {} + child_prefix = prefix.empty? ? "[]" : "#{prefix}[]" + body.each do |item| + walk_extras(child_prefix, item, resolved["items"], depth + 1).each { |e| seen[e] = true } + end + return seen.keys + end + + return [] unless body.is_a?(Hash) + return [] if resolved["type"] && resolved["type"] != "object" + + props = resolved["properties"] || {} + extras = [] + body.each do |key, value| + field_path = prefix.empty? ? key : "#{prefix}.#{key}" + if props.key?(key) + extras.concat(walk_extras(field_path, value, props[key], depth + 1)) + else + extras << field_path + end + end + extras + end + + # Follows $ref chains until a non-ref schema or a cycle. + # Accepts "#/components/schemas/X" and "openapi.json#/components/schemas/X". + def resolve_ref(schema) + seen = {} + current = schema + while current.is_a?(Hash) && current["$ref"].is_a?(String) + ref = current["$ref"] + break if seen[ref] + + seen[ref] = true + match = ref.match(%r{\A(?:openapi\.json)?#/components/schemas/(.+)\z}) + break unless match + + next_schema = @doc.dig("components", "schemas", match[1]) + break unless next_schema + + current = next_schema + end + current + end + end + end +end From 95534aba3fd66eddf98344f5b12cfb70187ecc79 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 4 May 2026 12:10:28 -0700 Subject: [PATCH 03/19] conformance(python): wire-replay runner + schema walker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 //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. --- conformance/runner/python/replay_runner.py | 215 +++++++++++++++++++++ conformance/runner/python/schema_walker.py | 146 ++++++++++++++ 2 files changed, 361 insertions(+) create mode 100644 conformance/runner/python/replay_runner.py create mode 100644 conformance/runner/python/schema_walker.py diff --git a/conformance/runner/python/replay_runner.py b/conformance/runner/python/replay_runner.py new file mode 100644 index 00000000..a07f9f98 --- /dev/null +++ b/conformance/runner/python/replay_runner.py @@ -0,0 +1,215 @@ +#!/usr/bin/env python3 +"""Wire-replay runner for the Python SDK. + +Reads wire snapshots written by the TS canonical canary runner, decodes each +page through the Python SDK's parse + normalize pipeline, walks the raw JSON +for required-field and extras detection, and persists per-test decode-result +snapshots at ``//decode/python/.json``. + +Mode-gate: this script is invoked only when ``WIRE_REPLAY_DIR`` is set +(see Makefile target ``conformance-python-replay``). The existing mock +runner (``runner.py``) handles the unset case. + +Decode boundary +--------------- +The Python SDK does not use typed dataclasses for response bodies — every +generated service method returns ``dict[str, Any]`` or a ``ListResult`` of +dicts. The only post-parse transformation the SDK applies is +``_normalize_person_ids`` (see ``basecamp.generated.services._base``), +which coerces system-actor Person ids ("basecamp", "campfire") to numeric +form. Calling that function directly on a parsed body is therefore the +faithful Python "decode" — it exercises the same code path the SDK runs +on every live response, so a regression in normalize (e.g. a new +personable shape) surfaces here as a decode_error rather than silently +passing. +""" + +from __future__ import annotations + +import json +import os +import re +import sys +from pathlib import Path +from typing import Any, Callable + +# Driver import; the only SDK piece we exercise is the post-parse normalize +# function — see module docstring for why this is the right boundary. +from basecamp.generated.services._base import _normalize_person_ids + +from schema_walker import SchemaWalker + +SCHEMA_VERSION = 1 + + +def _decode(body_text: str) -> None: + """Parse a single page body and run the SDK's normalize pass. + + Raises on JSON parse failure; raises if normalize raises (it currently + only mutates dict/list, but a future regression could throw). + """ + parsed = json.loads(body_text) + _normalize_person_ids(parsed) + + +# Map operation_id -> decoder. Same boundary for every op (see module +# docstring) — the SDK has no per-op typed deserializer to invoke. Keeping +# the dict explicit (rather than a default fallback) makes the +# coverage_gate diagnostic accurate: a typo or missing op surfaces +# loudly instead of being absorbed by a default decoder. +DECODERS: dict[str, Callable[[str], None]] = { + "ListProjects": _decode, + "GetProject": _decode, + "GetMyAssignments": _decode, + "GetMyCompletedAssignments": _decode, + "GetMyDueAssignments": _decode, + "GetMyNotifications": _decode, + "GetMyProfile": _decode, + "GetTodoset": _decode, + "ListTodolists": _decode, + "ListTodos": _decode, +} + + +def _safe_name(name: str) -> str: + return re.sub(r"[^a-z0-9_-]+", "_", name, flags=re.IGNORECASE) + + +class ReplayRunner: + def __init__(self, replay_dir: Path, backend: str, fixture_path: Path, openapi_path: Path) -> None: + self._replay_dir = replay_dir + self._backend = backend + self._walker = SchemaWalker(openapi_path) + self._fixture: list[dict] = [ + t for t in json.loads(fixture_path.read_text()) + if t.get("mode") == "live" + ] + + def coverage_gate(self) -> list[str]: + msgs: list[str] = [] + fixture_ops = sorted({t["operation"] for t in self._fixture}) + + # 1. Decoder coverage — every fixture operation must dispatch. + missing = [op for op in fixture_ops if op not in DECODERS] + if missing: + msgs.append( + f"Python replay runner missing decoders for: {', '.join(missing)}. " + "Add to DECODERS in replay_runner.py." + ) + + # 2. Snapshot completeness — every fixture op needs a wire file. + wire_dir = self._replay_dir / self._backend / "wire" + for t in self._fixture: + f = wire_dir / f"{_safe_name(t['name'])}.json" + if not f.exists(): + msgs.append( + f"Snapshot missing for operation {t['operation']} " + f"(test {t['name']!r}); expected at {f}. " + "Re-run TS live capture or check skip status." + ) + + # 3. Snapshot recognition — every captured snapshot's operation + # must be in the shared fixture (catches TS-side dispatch drift). + if wire_dir.exists(): + for f in sorted(wire_dir.glob("*.json")): + snap = json.loads(f.read_text()) + op = snap.get("operation") + if op is None: + msgs.append( + f"Snapshot {f.name} is missing the top-level 'operation' field. " + "Re-run the TS live canary; pre-PR3 snapshots are no longer supported." + ) + continue + if op not in fixture_ops: + msgs.append( + f"Unknown operation {op!r} in snapshot {f.name}; " + "TS dispatch table appears to have drifted from live-my-surface.json." + ) + return msgs + + def run(self) -> int: + msgs = self.coverage_gate() + if msgs: + for m in msgs: + print(m, file=sys.stderr) + return 1 + + out_dir = self._replay_dir / self._backend / "decode" / "python" + out_dir.mkdir(parents=True, exist_ok=True) + + failures = 0 + for t in self._fixture: + snapshot = self._read_snapshot(t["name"]) + result = self._decode_snapshot(snapshot) + (out_dir / f"{_safe_name(t['name'])}.json").write_text( + json.dumps(result, indent=2) + ) + if any(not p["decoded"] or p["missing_required"] for p in result["pages"]): + failures += 1 + + return 0 if failures == 0 else 1 + + def _read_snapshot(self, test_name: str) -> dict: + path = self._replay_dir / self._backend / "wire" / f"{_safe_name(test_name)}.json" + return json.loads(path.read_text()) + + def _decode_snapshot(self, snapshot: dict) -> dict: + operation = snapshot["operation"] + decoder = DECODERS[operation] + schema = self._walker.find_response_schema(operation) + + pages = [] + for page in snapshot["pages"]: + body_text: str = page.get("bodyText") or json.dumps(page["body"]) + decoded = False + decode_error: str | None = None + missing_required: list[str] = [] + extras_seen: list[str] = [] + + try: + decoder(body_text) + decoded = True + except Exception as e: + decode_error = f"{type(e).__name__}: {e}" + + if schema is not None: + # Re-parse for the walker if body came across as a string + # (non-JSON page body). On parse failure, leave body None + # and skip the walker — the decode_error above already + # records the failure. + body: Any = page.get("body") + if isinstance(body, str): + try: + body = json.loads(body_text) + except Exception: + body = None + if body is not None: + missing_required = self._walker.missing_required(body, schema) + extras_seen = self._walker.extras_seen(body, schema) + + pages.append({ + "decoded": decoded, + "decode_error": decode_error, + "missing_required": missing_required, + "extras_seen": extras_seen, + }) + + return { + "schema_version": SCHEMA_VERSION, + "operation": operation, + "pages": pages, + } + + +if __name__ == "__main__": + replay_dir = os.environ.get("WIRE_REPLAY_DIR") + backend = os.environ.get("BASECAMP_BACKEND") + if not replay_dir: + sys.exit("WIRE_REPLAY_DIR is required") + if not backend: + sys.exit("BASECAMP_BACKEND is required") + + fixture_path = Path(__file__).parent.parent.parent / "tests" / "live-my-surface.json" + openapi_path = Path(__file__).parent.parent.parent.parent / "openapi.json" + runner = ReplayRunner(Path(replay_dir), backend, fixture_path, openapi_path) + sys.exit(runner.run()) diff --git a/conformance/runner/python/schema_walker.py b/conformance/runner/python/schema_walker.py new file mode 100644 index 00000000..54c44852 --- /dev/null +++ b/conformance/runner/python/schema_walker.py @@ -0,0 +1,146 @@ +"""OpenAPI schema walker for the wire-replay runner. + +Pure-Python port of conformance/runner/typescript/schema-validator.ts. The +TS canary's Ajv-driven validation pipeline is replaced here by two focused +walkers: + + * `missing_required` — emits ``prefix/name`` paths for declared @required + fields that are absent from the wire body. + * `extras_seen` — emits paths for fields present on the wire that the + schema does not declare. Mirrors collectExtras semantics: object keys + use ``.``, array elements use ``[]``. + +Conventions kept exactly in sync with the TS implementation so cross-language +extras parity (PR 4 verification §5e) doesn't false-diff: + * ``additionalProperties: false`` is ignored — forward-compat fields must + not break the canary. + * ``$ref`` chains resolve until non-ref or cycle. Both ``#/components/...`` + and ``openapi.json#/components/...`` ref forms are accepted. + * Recursion depth bound 12. + * For arrays: only recurse when ``schema.type == "array"`` and ``items`` + is set. + * For non-object schemas (``type`` set and != ``"object"``), do not + recurse — guards against descending into primitives. + +No new dependencies — hand-rolled keeps semantics byte-identical to the TS +walker. ~80 LoC target. +""" + +from __future__ import annotations + +import json +import re +from pathlib import Path +from typing import Any + +_REF_RE = re.compile(r"^(?:openapi\.json)?#/components/schemas/(.+)$") +_MAX_DEPTH = 12 + + +class SchemaWalker: + def __init__(self, openapi_path: Path) -> None: + self._doc: dict = json.loads(openapi_path.read_text()) + + def find_response_schema(self, operation_id: str) -> dict | None: + for path_item in self._doc.get("paths", {}).values(): + for op in path_item.values(): + if not isinstance(op, dict) or op.get("operationId") != operation_id: + continue + responses = op.get("responses") or {} + # Match TS preference order: 200 first, then any 2xx, then default. + for code in ("200", "201", "202", "203", "204", "default"): + schema = _schema_for(responses.get(code)) + if schema is not None: + return schema + for code, response in responses.items(): + if re.fullmatch(r"2\d\d", str(code)): + schema = _schema_for(response) + if schema is not None: + return schema + return None + + def missing_required(self, body: Any, schema: dict) -> list[str]: + out: list[str] = [] + self._walk_required("", body, schema, 0, out) + return out + + def extras_seen(self, body: Any, schema: dict) -> list[str]: + out: list[str] = [] + seen: set[str] = set() + self._walk_extras("", body, schema, 0, out, seen) + return out + + def _resolve_ref(self, schema: Any) -> Any: + seen: set[str] = set() + current = schema + while isinstance(current, dict): + ref = current.get("$ref") + if not isinstance(ref, str) or ref in seen: + return current + seen.add(ref) + m = _REF_RE.match(ref) + if not m: + return current + nxt = self._doc.get("components", {}).get("schemas", {}).get(m.group(1)) + if nxt is None: + return current + current = nxt + return current + + def _walk_required(self, prefix: str, body: Any, schema: Any, depth: int, out: list[str]) -> None: + if depth > _MAX_DEPTH or body is None: + return + s = self._resolve_ref(schema) + if not isinstance(s, dict): + return + if s.get("type") == "array" and isinstance(body, list) and s.get("items") is not None: + for i, item in enumerate(body): + child = f"{prefix}[{i}]" if prefix else f"[{i}]" + self._walk_required(child, item, s["items"], depth + 1, out) + return + if s.get("type") is not None and s.get("type") != "object": + return + if not isinstance(body, dict): + return + props = s.get("properties") or {} + for name in s.get("required") or []: + if name not in body: + out.append(f"{prefix}/{name}" if prefix else name) + for name, sub in props.items(): + if name in body: + child = f"{prefix}/{name}" if prefix else name + self._walk_required(child, body[name], sub, depth + 1, out) + + def _walk_extras(self, prefix: str, body: Any, schema: Any, depth: int, out: list[str], seen: set[str]) -> None: + if depth > _MAX_DEPTH or body is None: + return + s = self._resolve_ref(schema) + if not isinstance(s, dict): + return + if isinstance(body, list): + if s.get("type") != "array" or s.get("items") is None: + return + child = f"{prefix}[]" if prefix else "[]" + for item in body: + self._walk_extras(child, item, s["items"], depth + 1, out, seen) + return + if not isinstance(body, dict): + return + if s.get("type") is not None and s.get("type") != "object": + return + props = s.get("properties") or {} + for key, value in body.items(): + field_path = f"{prefix}.{key}" if prefix else key + if key not in props: + if field_path not in seen: + seen.add(field_path) + out.append(field_path) + else: + self._walk_extras(field_path, value, props[key], depth + 1, out, seen) + + +def _schema_for(response: Any) -> dict | None: + if not isinstance(response, dict): + return None + schema = (response.get("content") or {}).get("application/json", {}).get("schema") + return schema if isinstance(schema, dict) else None From bcf26322e7af54d0b18767db9c29bf0038c0ba80 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 4 May 2026 12:10:42 -0700 Subject: [PATCH 04/19] conformance(go): wire-replay runner + schema walker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 //decode/go/. Decoder boundary is generated.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. --- conformance/runner/go/main.go | 23 ++ conformance/runner/go/replay_runner.go | 340 +++++++++++++++++++++++++ conformance/runner/go/schema_walker.go | 307 ++++++++++++++++++++++ 3 files changed, 670 insertions(+) create mode 100644 conformance/runner/go/replay_runner.go create mode 100644 conformance/runner/go/schema_walker.go diff --git a/conformance/runner/go/main.go b/conformance/runner/go/main.go index a9df5cb8..09ebbc43 100644 --- a/conformance/runner/go/main.go +++ b/conformance/runner/go/main.go @@ -107,6 +107,29 @@ type TestResult struct { } func main() { + // Wire-replay mode gate: when WIRE_REPLAY_DIR is set, dispatch to + // the replay runner (replay_runner.go) and exit. The replay runner + // consumes wire snapshots written by the canonical TS live runner; + // see conformance/runner/typescript/live-runner.test.ts. Mock mode + // (the rest of this function) runs only when the gate is unset. + if dir := os.Getenv("WIRE_REPLAY_DIR"); dir != "" { + backend := os.Getenv("BASECAMP_BACKEND") + if backend == "" { + fmt.Fprintln(os.Stderr, "BASECAMP_BACKEND is required when WIRE_REPLAY_DIR is set") + os.Exit(1) + } + // Match the existing relative-path convention in this file: the + // runner is invoked with cwd = conformance/runner/go. + fixturePath := filepath.Join("..", "..", "tests", "live-my-surface.json") + openapiPath := filepath.Join("..", "..", "..", "openapi.json") + runner, err := NewReplayRunner(dir, backend, fixturePath, openapiPath) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(runner.Run()) + } + testsDir := filepath.Join("..", "..", "tests") files, err := filepath.Glob(filepath.Join(testsDir, "*.json")) diff --git a/conformance/runner/go/replay_runner.go b/conformance/runner/go/replay_runner.go new file mode 100644 index 00000000..9cde4220 --- /dev/null +++ b/conformance/runner/go/replay_runner.go @@ -0,0 +1,340 @@ +package main + +// Wire-replay runner for the Go SDK. +// +// Mode-gate: this entrypoint runs only when WIRE_REPLAY_DIR is set. The +// existing main.go handles mock-mode and remains untouched aside from a +// thin gate at the top of main() that delegates here. +// +// We import go/pkg/generated directly. The unexported mappers +// (projectFromGenerated, etc.) intentionally stay private; this runner +// consumes the same internal generated types oapi-codegen produces. Do +// not "fix" this import — it is a deliberate seam for canary purposes. +// Per the BC5-readiness plan §"Per-language additions" (PR 3): the Go +// runner uses `json.Unmarshal(bodyBytes, &generated.{})` for the +// typed-decode boundary and walks the same bytes parsed into `any` for +// extras detection. + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "regexp" + "sort" + + generated "github.com/basecamp/basecamp-sdk/go/pkg/generated" +) + +const ReplaySchemaVersion = 1 + +// Decoders map operation_id -> decoder fn. Each decoder unmarshals +// bodyText into the generated response type produced by oapi-codegen +// (the same types the SDK uses internally) and returns the unmarshal +// error, or nil on success. Keep this table in sync with +// LIVE_OPERATIONS in conformance/runner/typescript/live-dispatch.ts — +// the coverage gate enforces parity. +var decoders = map[string]func(bodyText string) error{ + "ListProjects": func(bt string) error { + var v generated.ListProjectsResponseContent + return json.Unmarshal([]byte(bt), &v) + }, + "GetProject": func(bt string) error { + var v generated.GetProjectResponseContent + return json.Unmarshal([]byte(bt), &v) + }, + "GetMyAssignments": func(bt string) error { + var v generated.GetMyAssignmentsResponseContent + return json.Unmarshal([]byte(bt), &v) + }, + "GetMyCompletedAssignments": func(bt string) error { + var v generated.GetMyCompletedAssignmentsResponseContent + return json.Unmarshal([]byte(bt), &v) + }, + "GetMyDueAssignments": func(bt string) error { + var v generated.GetMyDueAssignmentsResponseContent + return json.Unmarshal([]byte(bt), &v) + }, + "GetMyNotifications": func(bt string) error { + var v generated.GetMyNotificationsResponseContent + return json.Unmarshal([]byte(bt), &v) + }, + "GetMyProfile": func(bt string) error { + var v generated.GetMyProfileResponseContent + return json.Unmarshal([]byte(bt), &v) + }, + "GetTodoset": func(bt string) error { + var v generated.GetTodosetResponseContent + return json.Unmarshal([]byte(bt), &v) + }, + "ListTodolists": func(bt string) error { + var v generated.ListTodolistsResponseContent + return json.Unmarshal([]byte(bt), &v) + }, + "ListTodos": func(bt string) error { + var v generated.ListTodosResponseContent + return json.Unmarshal([]byte(bt), &v) + }, +} + +var safeNameRE = regexp.MustCompile(`(?i)[^a-z0-9_-]+`) + +func safeName(s string) string { + return safeNameRE.ReplaceAllString(s, "_") +} + +// ReplayPage mirrors the per-page output schema documented in the +// BC5-readiness plan §"Snapshot output". Slices are emitted as `[]` (not +// null) in the JSON output for downstream comparator stability — see +// emptyIfNil before serialization. +type ReplayPage struct { + Decoded bool `json:"decoded"` + DecodeError *string `json:"decode_error"` + MissingRequired []string `json:"missing_required"` + ExtrasSeen []string `json:"extras_seen"` +} + +type ReplayResult struct { + SchemaVersion int `json:"schema_version"` + Operation string `json:"operation"` + Pages []ReplayPage `json:"pages"` +} + +type fixtureTest struct { + Mode string `json:"mode"` + Name string `json:"name"` + Operation string `json:"operation"` +} + +type wirePage struct { + Status int `json:"status"` + Headers map[string]string `json:"headers"` + Body any `json:"body"` + BodyText string `json:"bodyText"` + URL string `json:"url"` +} + +type wireSnapshot struct { + Operation string `json:"operation"` + Pages []wirePage `json:"pages"` + PagesCount int `json:"pages_count"` +} + +type ReplayRunner struct { + replayDir string + backend string + fixturePath string + walker *SchemaWalker + fixture []fixtureTest +} + +func NewReplayRunner(replayDir, backend, fixturePath, openapiPath string) (*ReplayRunner, error) { + walker, err := NewSchemaWalker(openapiPath) + if err != nil { + return nil, err + } + raw, err := os.ReadFile(fixturePath) + if err != nil { + return nil, err + } + var all []fixtureTest + if err := json.Unmarshal(raw, &all); err != nil { + return nil, err + } + live := make([]fixtureTest, 0, len(all)) + for _, t := range all { + if t.Mode == "live" { + live = append(live, t) + } + } + return &ReplayRunner{replayDir, backend, fixturePath, walker, live}, nil +} + +// coverageGate enforces the three startup checks documented in PR 3 +// §"Coverage gate": +// 1. every fixture op has a decoder +// 2. every fixture op has a snapshot file +// 3. every snapshot's operation field is in the fixture +// +// Any failure returns a non-empty list of human-readable messages; the +// caller prints them and exits non-zero. +func (r *ReplayRunner) coverageGate() []string { + var msgs []string + fixtureOps := map[string]bool{} + for _, t := range r.fixture { + fixtureOps[t.Operation] = true + } + + // 1. Decoder coverage + var missing []string + for op := range fixtureOps { + if _, ok := decoders[op]; !ok { + missing = append(missing, op) + } + } + if len(missing) > 0 { + sort.Strings(missing) + msgs = append(msgs, fmt.Sprintf( + "Go replay runner missing decoders for: %v. Add to decoders map in replay_runner.go.", + missing, + )) + } + + // 2. Snapshot completeness + wireDir := filepath.Join(r.replayDir, r.backend, "wire") + for _, t := range r.fixture { + f := filepath.Join(wireDir, safeName(t.Name)+".json") + if _, err := os.Stat(f); err != nil { + msgs = append(msgs, fmt.Sprintf( + "Snapshot missing for operation %s (test %q); expected at %s. Re-run TS live capture or check skip status.", + t.Operation, t.Name, f, + )) + } + } + + // 3. Snapshot recognition + if entries, err := os.ReadDir(wireDir); err == nil { + for _, e := range entries { + if filepath.Ext(e.Name()) != ".json" { + continue + } + data, err := os.ReadFile(filepath.Join(wireDir, e.Name())) + if err != nil { + continue + } + var snap struct { + Operation string `json:"operation"` + } + if err := json.Unmarshal(data, &snap); err != nil { + continue + } + if snap.Operation == "" { + msgs = append(msgs, fmt.Sprintf( + "Snapshot %s is missing the top-level `operation` field. Re-run the TS live canary; pre-PR3 snapshots are no longer supported.", + e.Name(), + )) + continue + } + if !fixtureOps[snap.Operation] { + msgs = append(msgs, fmt.Sprintf( + "Unknown operation %q in snapshot %s; TS dispatch table appears to have drifted from live-my-surface.json.", + snap.Operation, e.Name(), + )) + } + } + } + + return msgs +} + +func (r *ReplayRunner) Run() int { + if msgs := r.coverageGate(); len(msgs) > 0 { + for _, m := range msgs { + fmt.Fprintln(os.Stderr, m) + } + return 1 + } + + outDir := filepath.Join(r.replayDir, r.backend, "decode", "go") + if err := os.MkdirAll(outDir, 0o755); err != nil { + fmt.Fprintln(os.Stderr, err) + return 1 + } + + failures := 0 + for _, t := range r.fixture { + snap, err := r.readSnapshot(t.Name) + if err != nil { + fmt.Fprintln(os.Stderr, err) + failures++ + continue + } + result := r.decodeSnapshot(snap) + out, err := json.MarshalIndent(result, "", " ") + if err != nil { + fmt.Fprintln(os.Stderr, err) + failures++ + continue + } + outPath := filepath.Join(outDir, safeName(t.Name)+".json") + if err := os.WriteFile(outPath, out, 0o644); err != nil { + fmt.Fprintln(os.Stderr, err) + failures++ + continue + } + for _, p := range result.Pages { + if !p.Decoded || len(p.MissingRequired) > 0 { + failures++ + break + } + } + } + + if failures > 0 { + return 1 + } + return 0 +} + +func (r *ReplayRunner) readSnapshot(testName string) (*wireSnapshot, error) { + path := filepath.Join(r.replayDir, r.backend, "wire", safeName(testName)+".json") + raw, err := os.ReadFile(path) + if err != nil { + return nil, err + } + var snap wireSnapshot + if err := json.Unmarshal(raw, &snap); err != nil { + return nil, err + } + return &snap, nil +} + +func (r *ReplayRunner) decodeSnapshot(snap *wireSnapshot) ReplayResult { + decoder := decoders[snap.Operation] + schema := r.walker.FindResponseSchema(snap.Operation) + + pages := make([]ReplayPage, 0, len(snap.Pages)) + for _, page := range snap.Pages { + bodyText := page.BodyText + if bodyText == "" && page.Body != nil { + if b, err := json.Marshal(page.Body); err == nil { + bodyText = string(b) + } + } + + rp := ReplayPage{Decoded: false} + if err := decoder(bodyText); err == nil { + rp.Decoded = true + } else { + s := err.Error() + rp.DecodeError = &s + } + + if schema != nil { + // Per the TS validator: walk against parsed JSON, not the + // SDK-decoded structure (decoders may drop unknown fields). + // A page whose body is not parseable JSON gets empty arrays + // here — the decode_error above already captures that failure. + var body any + if json.Unmarshal([]byte(bodyText), &body) == nil { + rp.MissingRequired = r.walker.MissingRequired(body, schema) + rp.ExtrasSeen = r.walker.ExtrasSeen(body, schema) + } + } + // Emit `[]` rather than `null` for stable cross-language + // comparator output (PR 4 jq diffs would otherwise false-fire). + if rp.MissingRequired == nil { + rp.MissingRequired = []string{} + } + if rp.ExtrasSeen == nil { + rp.ExtrasSeen = []string{} + } + pages = append(pages, rp) + } + + return ReplayResult{ + SchemaVersion: ReplaySchemaVersion, + Operation: snap.Operation, + Pages: pages, + } +} diff --git a/conformance/runner/go/schema_walker.go b/conformance/runner/go/schema_walker.go new file mode 100644 index 00000000..10c4dafa --- /dev/null +++ b/conformance/runner/go/schema_walker.go @@ -0,0 +1,307 @@ +package main + +// Pure-Go port of conformance/runner/typescript/schema-validator.ts. +// +// Walks parsed JSON against the OpenAPI response schema, surfacing: +// - MissingRequired: required-field paths absent from the body +// - ExtrasSeen: field paths present on the wire but not declared +// +// Conventions match the TS walker (and the Python/Ruby ports under +// conformance/runner/{python,ruby}/) so cross-language extras parity diffs +// (PR 4 §Verification) don't false-fire: +// - Object paths use "." (e.g. "owner.id") +// - Required walk uses "[i]" element segments; extras walk uses "[]" +// to dedupe item-level extras across an array +// - "$ref" chains resolve until a non-ref schema or a cycle. Both +// "#/components/schemas/X" and "openapi.json#/components/schemas/X" +// are accepted. +// - additionalProperties:false is intentionally ignored — extras are +// reported but do not fail validation (forward-compat). +// - Recursion depth bound 12 as a cycle guard. +// +// No new dependencies: hand-rolled keeps semantics identical to the TS +// walker. After json.Unmarshal into any, JSON objects become +// map[string]any and arrays become []any; both branches dispatch on that. + +import ( + "encoding/json" + "os" + "regexp" + "sort" +) + +const maxDepth = 12 + +var refRE = regexp.MustCompile(`^(?:openapi\.json)?#/components/schemas/(.+)$`) + +type SchemaWalker struct { + doc map[string]any +} + +func NewSchemaWalker(openapiPath string) (*SchemaWalker, error) { + raw, err := os.ReadFile(openapiPath) + if err != nil { + return nil, err + } + var doc map[string]any + if err := json.Unmarshal(raw, &doc); err != nil { + return nil, err + } + return &SchemaWalker{doc: doc}, nil +} + +// FindResponseSchema returns the response schema for operationID, or nil +// when none is found. Match TS preference order: 200, then any 2xx, then +// "default". +func (w *SchemaWalker) FindResponseSchema(operationID string) map[string]any { + paths, _ := w.doc["paths"].(map[string]any) + for _, pathName := range sortedKeys(paths) { + pathItem, _ := paths[pathName].(map[string]any) + for _, method := range sortedKeys(pathItem) { + op, ok := pathItem[method].(map[string]any) + if !ok { + continue + } + if id, _ := op["operationId"].(string); id != operationID { + continue + } + responses, _ := op["responses"].(map[string]any) + for _, code := range []string{"200", "201", "202", "203", "204", "default"} { + if s := schemaFor(responses[code]); s != nil { + return s + } + } + for _, code := range sortedKeys(responses) { + if len(code) == 3 && code[0] == '2' && isDigit(code[1]) && isDigit(code[2]) { + if s := schemaFor(responses[code]); s != nil { + return s + } + } + } + } + } + return nil +} + +// MissingRequired returns dotted-path strings for required fields absent +// from body. +func (w *SchemaWalker) MissingRequired(body any, schema map[string]any) []string { + return w.walkRequired("", body, schema, 0) +} + +// ExtrasSeen returns dotted-path strings for fields present on the wire +// but not declared in the schema. Recurses through known properties so +// item-level extras on lists surface (e.g. "unreads[].new_field"). +func (w *SchemaWalker) ExtrasSeen(body any, schema map[string]any) []string { + return w.walkExtras("", body, schema, 0) +} + +func (w *SchemaWalker) walkRequired(prefix string, body, schema any, depth int) []string { + if depth > maxDepth || body == nil { + return nil + } + resolved := w.resolveRef(schema) + s, ok := resolved.(map[string]any) + if !ok { + return nil + } + + if arr, isArr := body.([]any); isArr { + typ, _ := s["type"].(string) + if typ != "array" || s["items"] == nil { + return nil + } + var missing []string + for i, item := range arr { + child := joinIndex(prefix, i) + missing = append(missing, w.walkRequired(child, item, s["items"], depth+1)...) + } + return missing + } + + obj, isObj := body.(map[string]any) + if !isObj { + return nil + } + if typ, present := s["type"].(string); present && typ != "object" { + return nil + } + + props, _ := s["properties"].(map[string]any) + required, _ := s["required"].([]any) + var missing []string + // Required check: iterate the required array in declaration order. + for _, r := range required { + name, _ := r.(string) + if _, ok := obj[name]; !ok { + missing = append(missing, joinDot(prefix, name)) + } + } + // Recurse into present known props. Sort keys: cross-language + // comparable output (TS preserves JSON parse order; Go map iteration + // is randomized, so sorted keys keep diffs stable). + for _, name := range sortedKeys(props) { + if value, ok := obj[name]; ok { + missing = append(missing, w.walkRequired(joinDot(prefix, name), value, props[name], depth+1)...) + } + } + return missing +} + +func (w *SchemaWalker) walkExtras(prefix string, body, schema any, depth int) []string { + if depth > maxDepth || body == nil { + return nil + } + resolved := w.resolveRef(schema) + s, ok := resolved.(map[string]any) + if !ok { + return nil + } + + if arr, isArr := body.([]any); isArr { + typ, _ := s["type"].(string) + if typ != "array" || s["items"] == nil { + return nil + } + // Per-array dedup mirrors TS collectExtras's `new Set` for arrays. + seen := map[string]struct{}{} + var out []string + child := joinBrackets(prefix) + for _, item := range arr { + for _, e := range w.walkExtras(child, item, s["items"], depth+1) { + if _, dup := seen[e]; !dup { + seen[e] = struct{}{} + out = append(out, e) + } + } + } + return out + } + + obj, isObj := body.(map[string]any) + if !isObj { + return nil + } + if typ, present := s["type"].(string); present && typ != "object" { + return nil + } + + props, _ := s["properties"].(map[string]any) + var extras []string + // Sort body keys: stable cross-language output. TS preserves JSON parse + // order, which equals insertion order; Go map iteration is randomized, + // so we sort. PR 4 cross-lang extras parity diff compares deduped sets + // anyway, so this only affects the per-page ordering inside the snapshot. + for _, key := range sortedKeys(obj) { + fieldPath := joinDot(prefix, key) + if _, known := props[key]; known { + extras = append(extras, w.walkExtras(fieldPath, obj[key], props[key], depth+1)...) + } else { + extras = append(extras, fieldPath) + } + } + return extras +} + +// resolveRef follows $ref chains until a non-ref schema or a cycle. +// Accepts "#/components/schemas/X" and "openapi.json#/components/schemas/X". +func (w *SchemaWalker) resolveRef(schema any) any { + seen := map[string]struct{}{} + current := schema + for { + m, ok := current.(map[string]any) + if !ok { + return current + } + ref, ok := m["$ref"].(string) + if !ok { + return current + } + if _, dup := seen[ref]; dup { + return current + } + seen[ref] = struct{}{} + match := refRE.FindStringSubmatch(ref) + if match == nil { + return current + } + components, _ := w.doc["components"].(map[string]any) + schemas, _ := components["schemas"].(map[string]any) + next, ok := schemas[match[1]] + if !ok { + return current + } + current = next + } +} + +func schemaFor(response any) map[string]any { + resp, ok := response.(map[string]any) + if !ok { + return nil + } + content, _ := resp["content"].(map[string]any) + appJSON, _ := content["application/json"].(map[string]any) + schema, _ := appJSON["schema"].(map[string]any) + return schema +} + +func sortedKeys(m map[string]any) []string { + if m == nil { + return nil + } + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + return keys +} + +func isDigit(b byte) bool { return b >= '0' && b <= '9' } + +func joinDot(prefix, name string) string { + if prefix == "" { + return name + } + return prefix + "." + name +} + +func joinIndex(prefix string, i int) string { + suffix := "[" + itoa(i) + "]" + if prefix == "" { + return suffix + } + return prefix + suffix +} + +func joinBrackets(prefix string) string { + if prefix == "" { + return "[]" + } + return prefix + "[]" +} + +// itoa avoids importing strconv just for one use. +func itoa(i int) string { + if i == 0 { + return "0" + } + neg := false + if i < 0 { + neg = true + i = -i + } + var buf [20]byte + pos := len(buf) + for i > 0 { + pos-- + buf[pos] = byte('0' + i%10) + i /= 10 + } + if neg { + pos-- + buf[pos] = '-' + } + return string(buf[pos:]) +} From da711a2f4f2e7ef961da6b7fdaaf029fbaac681d Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 4 May 2026 12:11:00 -0700 Subject: [PATCH 05/19] conformance(kotlin): wire-replay runner + schema walker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 //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. --- kotlin/conformance/build.gradle.kts | 16 ++ .../basecamp/sdk/conformance/ReplayRunner.kt | 251 ++++++++++++++++++ .../basecamp/sdk/conformance/SchemaWalker.kt | 178 +++++++++++++ 3 files changed, 445 insertions(+) create mode 100644 kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt create mode 100644 kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt diff --git a/kotlin/conformance/build.gradle.kts b/kotlin/conformance/build.gradle.kts index 755247de..e7ed0d53 100644 --- a/kotlin/conformance/build.gradle.kts +++ b/kotlin/conformance/build.gradle.kts @@ -12,6 +12,22 @@ tasks.named("run") { workingDir = rootProject.projectDir } +// Wire-replay runner entry point (PR 3). The mock-mode runner above is +// untouched; this task targets ReplayRunner.kt's `main` and is invoked by +// the Makefile target `conformance-kotlin-replay` when WIRE_REPLAY_DIR is +// set. Reads canonical wire snapshots written by the TS live runner and +// writes per-test decode-result snapshots. +tasks.register("runReplay") { + group = "application" + description = "Run the wire-replay conformance runner (set WIRE_REPLAY_DIR + BASECAMP_BACKEND)." + classpath = sourceSets["main"].runtimeClasspath + mainClass.set("com.basecamp.sdk.conformance.ReplayRunnerKt") + workingDir = rootProject.projectDir + // Pass through the env vars the runner reads. + environment("WIRE_REPLAY_DIR", System.getenv("WIRE_REPLAY_DIR") ?: "") + environment("BASECAMP_BACKEND", System.getenv("BASECAMP_BACKEND") ?: "") +} + dependencies { implementation(project(":basecamp-sdk")) implementation(libs.kotlinx.serialization.json) diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt new file mode 100644 index 00000000..5d407640 --- /dev/null +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt @@ -0,0 +1,251 @@ +package com.basecamp.sdk.conformance + +import com.basecamp.sdk.generated.models.Person +import com.basecamp.sdk.generated.models.Project +import com.basecamp.sdk.generated.models.Todo +import com.basecamp.sdk.generated.models.Todolist +import com.basecamp.sdk.generated.models.Todoset +import kotlinx.serialization.Serializable +import kotlinx.serialization.builtins.ListSerializer +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.contentOrNull +import kotlinx.serialization.json.encodeToJsonElement +import kotlinx.serialization.json.jsonArray +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive +import java.io.File +import java.nio.file.Path +import java.nio.file.Paths +import kotlin.system.exitProcess + +/** + * Wire-replay runner for the Kotlin SDK conformance suite. + * + * Reads canonical wire snapshots written by the TS live runner (see + * `conformance/runner/typescript/live-runner.test.ts`), decodes each page + * through the Kotlin SDK's deserialization boundary, walks the raw JSON for + * required-field and extras detection, and persists per-test decode-result + * snapshots at `//decode/kotlin/.json`. + * + * Mode-gate: invoked only when `WIRE_REPLAY_DIR` is set (Makefile target + * `conformance-kotlin-replay`). The existing mock runner (`Main.kt`) handles + * the unset case and is unaffected by anything in this file. + * + * Decode boundary + * --------------- + * Where the Kotlin SDK has a typed model for the response payload (`Project`, + * `Person`, `Todo`, `Todolist`, `Todoset`), the decoder routes through that + * type — exactly what the generated service method does at runtime. The four + * `My*` operations (assignments, completed assignments, due assignments, + * notifications) currently return `JsonElement` from the SDK because + * `MyAssignment` and `Notification` lack typed Kotlin models; for those we + * decode as `JsonElement`, which mirrors the SDK's actual surface. + * + * The `Json` instance below intentionally mirrors mock-mode (`ignoreUnknownKeys + * = true`, `coerceInputValues = false`) — additive BC5 fields must NOT decode + * as errors here. 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`, not from the decoder. + */ +private val replayJson = Json { + ignoreUnknownKeys = true + coerceInputValues = false + isLenient = false +} + +/** Walker uses an unconfigured Json — only parses the body for tree access. */ +private val parserJson = Json + +const val REPLAY_SCHEMA_VERSION = 1 + +/** + * Decoders per operation. Each takes the raw bodyText for one page and + * either completes normally (decode succeeded) or throws (decode failed). + * + * For `JsonElement`-returning SDK methods, we decode as `JsonElement` — + * that's the SDK's actual decode boundary today. When the SDK adds typed + * models for those operations, switch the decoder to the typed model. + */ +private val decoders: Map Unit> = mapOf( + "ListProjects" to { bt -> replayJson.decodeFromString(ListSerializer(Project.serializer()), bt) }, + "GetProject" to { bt -> replayJson.decodeFromString(Project.serializer(), bt) }, + "GetMyAssignments" to { bt -> replayJson.decodeFromString(JsonElement.serializer(), bt) }, + "GetMyCompletedAssignments" to { bt -> replayJson.decodeFromString(JsonElement.serializer(), bt) }, + "GetMyDueAssignments" to { bt -> replayJson.decodeFromString(JsonElement.serializer(), bt) }, + "GetMyNotifications" to { bt -> replayJson.decodeFromString(JsonElement.serializer(), bt) }, + "GetMyProfile" to { bt -> replayJson.decodeFromString(Person.serializer(), bt) }, + "GetTodoset" to { bt -> replayJson.decodeFromString(Todoset.serializer(), bt) }, + "ListTodolists" to { bt -> replayJson.decodeFromString(ListSerializer(Todolist.serializer()), bt) }, + "ListTodos" to { bt -> replayJson.decodeFromString(ListSerializer(Todo.serializer()), bt) }, +) + +private val safeNameRegex = Regex("[^a-z0-9_-]+", RegexOption.IGNORE_CASE) +private fun safeName(s: String): String = safeNameRegex.replace(s, "_") + +@Serializable +data class ReplayPageResult( + val decoded: Boolean, + val decode_error: String?, + val missing_required: List, + val extras_seen: List, +) + +@Serializable +data class ReplayResult( + val schema_version: Int, + val operation: String, + val pages: List, +) + +class ReplayRunner( + private val replayDir: Path, + private val backend: String, + fixturePath: Path, + openapiPath: Path, +) { + private val walker = SchemaWalker(openapiPath.toString()) + private val fixture: List = run { + val all = parserJson.parseToJsonElement(File(fixturePath.toString()).readText()).jsonArray + all.mapNotNull { el -> + val obj = el as? JsonObject ?: return@mapNotNull null + obj.takeIf { it["mode"]?.jsonPrimitive?.contentOrNull == "live" } + } + } + + fun coverageGate(): List { + val msgs = mutableListOf() + val fixtureOps = fixture.mapNotNull { it["operation"]?.jsonPrimitive?.contentOrNull } + .distinct().sorted() + + // 1. Decoder coverage — every fixture operation must have a decoder. + val missing = fixtureOps.filterNot { decoders.containsKey(it) } + if (missing.isNotEmpty()) { + msgs += "Kotlin replay runner missing decoders for: ${missing.joinToString(", ")}. " + + "Add to decoders in ReplayRunner.kt." + } + + // 2. Snapshot completeness — every fixture op needs a wire file. + val wireDir = replayDir.resolve(backend).resolve("wire").toFile() + for (t in fixture) { + val name = t["name"]?.jsonPrimitive?.contentOrNull ?: continue + val op = t["operation"]?.jsonPrimitive?.contentOrNull ?: continue + val f = File(wireDir, "${safeName(name)}.json") + if (!f.exists()) { + msgs += "Snapshot missing for operation $op (test \"$name\"); expected at " + + "${f.path}. Re-run TS live capture or check skip status." + } + } + + // 3. Snapshot recognition — every captured snapshot's operation must + // be in the shared fixture (catches TS-side dispatch drift). + if (wireDir.exists()) { + wireDir.listFiles { f -> f.extension == "json" }?.forEach { f -> + val snap = parserJson.parseToJsonElement(f.readText()) as? JsonObject + ?: return@forEach + val op = snap["operation"]?.jsonPrimitive?.contentOrNull + if (op == null) { + msgs += "Snapshot ${f.name} is missing the top-level `operation` field. " + + "Re-run the TS live canary; pre-PR3 snapshots are no longer supported." + return@forEach + } + if (op !in fixtureOps) { + msgs += "Unknown operation \"$op\" in snapshot ${f.name}; TS dispatch " + + "table appears to have drifted from live-my-surface.json." + } + } + } + return msgs + } + + fun run(): Int { + val msgs = coverageGate() + if (msgs.isNotEmpty()) { + msgs.forEach { System.err.println(it) } + return 1 + } + + val outDir = replayDir.resolve(backend).resolve("decode").resolve("kotlin").toFile() + outDir.mkdirs() + + val pretty = Json { prettyPrint = true } + var failures = 0 + for (t in fixture) { + val name = t["name"]!!.jsonPrimitive.content + val snap = readSnapshot(name) + val result = decodeSnapshot(snap) + File(outDir, "${safeName(name)}.json").writeText( + pretty.encodeToString(ReplayResult.serializer(), result) + ) + if (result.pages.any { !it.decoded || it.missing_required.isNotEmpty() }) failures++ + } + return if (failures == 0) 0 else 1 + } + + private fun readSnapshot(testName: String): JsonObject { + val path = replayDir.resolve(backend).resolve("wire") + .resolve("${safeName(testName)}.json").toFile() + return parserJson.parseToJsonElement(path.readText()).jsonObject + } + + private fun decodeSnapshot(snap: JsonObject): ReplayResult { + val operation = snap["operation"]!!.jsonPrimitive.content + val decoder = decoders[operation]!! + val schema = walker.findResponseSchema(operation) + + val pages = snap["pages"]!!.jsonArray.map { p -> + val page = p.jsonObject + // Prefer bodyText (raw); fall back to serializing `body` if absent. + val bodyText = page["bodyText"]?.jsonPrimitive?.contentOrNull + ?: page["body"]?.let { parserJson.encodeToString(JsonElement.serializer(), it) } + ?: "" + + var decoded = false + var decodeError: String? = null + try { + decoder(bodyText) + decoded = true + } catch (e: Throwable) { + decodeError = "${e::class.simpleName}: ${e.message}" + } + + var missing: List = emptyList() + var extras: List = emptyList() + if (schema != null && bodyText.isNotEmpty()) { + try { + val body = parserJson.parseToJsonElement(bodyText) + missing = walker.missingRequired(body, schema) + extras = walker.extrasSeen(body, schema) + } catch (_: Throwable) { + // Body is not parseable JSON; decode_error above already + // records the failure. Leave walker output empty. + } + } + + ReplayPageResult(decoded, decodeError, missing, extras) + } + return ReplayResult(REPLAY_SCHEMA_VERSION, operation, pages) + } +} + +fun main() { + val replayDir = System.getenv("WIRE_REPLAY_DIR") + ?: run { System.err.println("WIRE_REPLAY_DIR is required"); exitProcess(1) } + val backend = System.getenv("BASECAMP_BACKEND") + ?: run { System.err.println("BASECAMP_BACKEND is required"); exitProcess(1) } + + // Resolve repo paths relative to the kotlin/ root. The Gradle `runReplay` + // task sets workingDir = rootProject.projectDir (i.e. the repo's `kotlin/` + // directory), so repoRoot is the parent. + val kotlinRoot = Paths.get("").toAbsolutePath() + val repoRoot = kotlinRoot.parent + ?: error("Cannot resolve repo root from CWD=$kotlinRoot") + val fixturePath = repoRoot.resolve("conformance/tests/live-my-surface.json") + val openapiPath = repoRoot.resolve("openapi.json") + + val runner = ReplayRunner(Paths.get(replayDir), backend, fixturePath, openapiPath) + exitProcess(runner.run()) +} diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt new file mode 100644 index 00000000..1fa46343 --- /dev/null +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt @@ -0,0 +1,178 @@ +package com.basecamp.sdk.conformance + +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonArray +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonNull +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.contentOrNull +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive +import java.io.File + +/** + * Pure-Kotlin port of `conformance/runner/typescript/schema-validator.ts`. + * + * Walks parsed JSON against the OpenAPI response schema, surfacing: + * - missing required-field paths (slash-separated, e.g. "owner/id") + * - extras-seen field paths (dot-separated for objects, "[]" for arrays, + * e.g. "unreads[].new_field") + * + * Conventions match the TS / Ruby / Python / Go ports in + * `conformance/runner/{ruby,python,go}/` so cross-language extras parity diffs + * (PR 4 §Verification) don't false-fire: + * - `additionalProperties: false` is intentionally ignored — extras are + * reported but do not fail validation (forward-compat). + * - `$ref` chains resolve until a non-ref schema or a cycle. Both + * `#/components/schemas/X` and `openapi.json#/components/schemas/X` are + * accepted. + * - Recursion depth bound 12 as a cycle guard. + * - Required walk uses `[i]` element segments; extras walk uses `[]` to + * dedupe item-level extras across an array. + * - A field whose value is `JsonNull` counts as PRESENT (matches TS + * `name in body` semantics) — `nullable` separately governs whether null + * is acceptable, but presence-checking only asks "is the key in the + * object?". + * + * No new dependencies — kotlinx.serialization.json is already on the + * conformance build classpath via the mock runner. + */ +class SchemaWalker(openapiPath: String) { + private val doc: JsonObject = Json.parseToJsonElement(File(openapiPath).readText()).jsonObject + private val schemas: JsonObject = doc["components"]?.jsonObject?.get("schemas")?.jsonObject + ?: JsonObject(emptyMap()) + + /** Returns null when no response schema is found (do not throw). */ + fun findResponseSchema(operationId: String): JsonObject? { + val paths = doc["paths"]?.jsonObject ?: return null + for ((_, pathItemEl) in paths) { + val pathItem = pathItemEl as? JsonObject ?: continue + for ((_, opEl) in pathItem) { + val op = opEl as? JsonObject ?: continue + val opId = op["operationId"]?.jsonPrimitive?.contentOrNull + if (opId != operationId) continue + val responses = op["responses"]?.jsonObject ?: continue + for (code in PREFERRED_CODES) { + schemaFor(responses[code])?.let { return it } + } + for ((code, response) in responses) { + if (code.length == 3 && code[0] == '2' && code[1].isAsciiDigit() && code[2].isAsciiDigit()) { + schemaFor(response)?.let { return it } + } + } + } + } + return null + } + + /** Slash-separated paths for required fields absent from body. */ + fun missingRequired(body: JsonElement, schema: JsonObject): List { + val out = mutableListOf() + walkRequired("", body, schema, 0, out) + return out + } + + /** Dotted-path strings for fields present on wire but not declared in schema. */ + fun extrasSeen(body: JsonElement, schema: JsonObject): List { + val out = mutableListOf() + walkExtras("", body, schema, 0, out) + return out + } + + private fun walkRequired(prefix: String, body: JsonElement, schema: JsonElement, depth: Int, out: MutableList) { + if (depth > MAX_DEPTH || body is JsonNull) return + val s = resolveRef(schema) as? JsonObject ?: return + + if (body is JsonArray) { + if (s["type"]?.jsonPrimitive?.contentOrNull != "array") return + val items = s["items"] ?: return + for ((i, item) in body.withIndex()) { + val child = if (prefix.isEmpty()) "[$i]" else "$prefix[$i]" + walkRequired(child, item, items, depth + 1, out) + } + return + } + if (body !is JsonObject) return + val type = s["type"]?.jsonPrimitive?.contentOrNull + if (type != null && type != "object") return + + val props = s["properties"]?.jsonObject ?: JsonObject(emptyMap()) + // Emit prefix/name for any required key absent from the body. JsonNull + // values count as PRESENT (TS `name in body`). + val required = s["required"] as? JsonArray + if (required != null) { + for (req in required) { + val name = req.jsonPrimitive.contentOrNull ?: continue + if (!body.containsKey(name)) { + out += if (prefix.isEmpty()) name else "$prefix/$name" + } + } + } + for ((name, sub) in props) { + val value = body[name] ?: continue + val child = if (prefix.isEmpty()) name else "$prefix/$name" + walkRequired(child, value, sub, depth + 1, out) + } + } + + private fun walkExtras(prefix: String, body: JsonElement, schema: JsonElement, depth: Int, out: MutableList) { + if (depth > MAX_DEPTH || body is JsonNull) return + val s = resolveRef(schema) as? JsonObject ?: return + + if (body is JsonArray) { + if (s["type"]?.jsonPrimitive?.contentOrNull != "array") return + val items = s["items"] ?: return + val child = if (prefix.isEmpty()) "[]" else "$prefix[]" + // Per-array dedup mirrors TS collectExtras's `new Set` for arrays. + val seen = mutableSetOf() + for (item in body) { + val tmp = mutableListOf() + walkExtras(child, item, items, depth + 1, tmp) + for (e in tmp) if (seen.add(e)) out += e + } + return + } + if (body !is JsonObject) return + val type = s["type"]?.jsonPrimitive?.contentOrNull + if (type != null && type != "object") return + + val props = s["properties"]?.jsonObject ?: JsonObject(emptyMap()) + for ((key, value) in body) { + val fieldPath = if (prefix.isEmpty()) key else "$prefix.$key" + val sub = props[key] + if (sub == null) { + out += fieldPath + } else { + walkExtras(fieldPath, value, sub, depth + 1, out) + } + } + } + + /** Follow $ref chains until a non-ref schema or a cycle. */ + private fun resolveRef(schema: JsonElement): JsonElement { + val seen = mutableSetOf() + var current: JsonElement = schema + while (current is JsonObject) { + val ref = current["\$ref"]?.jsonPrimitive?.contentOrNull ?: return current + if (!seen.add(ref)) return current + val match = REF_REGEX.matchEntire(ref) ?: return current + current = schemas[match.groupValues[1]] ?: return current + } + return current + } + + private fun schemaFor(response: JsonElement?): JsonObject? { + val resp = response as? JsonObject ?: return null + val content = resp["content"] as? JsonObject ?: return null + val appJson = content["application/json"] as? JsonObject ?: return null + return appJson["schema"] as? JsonObject + } + + private fun Char.isAsciiDigit(): Boolean = this in '0'..'9' + + companion object { + private const val MAX_DEPTH = 12 + private val PREFERRED_CODES = listOf("200", "201", "202", "203", "204", "default") + private val REF_REGEX = Regex("^(?:openapi\\.json)?#/components/schemas/(.+)$") + } +} From 335668d86774bf52104f223010cfa7744e277fc5 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 4 May 2026 12:11:15 -0700 Subject: [PATCH 06/19] conformance: wire-replay make targets + CONTRIBUTING + gitignore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .gitignore | 4 +++ CONTRIBUTING.md | 71 ++++++++++++++++++++++++++++++++++++++++++++++++- Makefile | 53 ++++++++++++++++++++++++++++++------ 3 files changed, 119 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index 92d3ce20..74e0d332 100644 --- a/.gitignore +++ b/.gitignore @@ -37,7 +37,11 @@ python/dist/ python/.pytest_cache/ python/.coverage python/src/*.egg-info/ + +# Python conformance runner artifacts conformance/runner/python/.venv/ +conformance/runner/python/**/__pycache__/ +conformance/runner/python/**/*.pyc conformance/runner/python/uv.lock # TypeScript SDK build artifacts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bd15fc71..9481719d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -293,7 +293,76 @@ gate refuses to run if any fixture operation lacks a dispatch. Because live canary fixtures live in the shared `conformance/tests/` directory, offline conformance runners must treat `mode` as part of the shared schema and execute only mock tests: omitted `mode` or `mode: "mock"`. `mode: "live"` entries -belong to the TypeScript live wire-capture runner until replay runners are added. +belong to the TypeScript live wire-capture runner and the cross-language +wire-replay runners described next. + +### Wire replay (cross-language) + +The TypeScript live runner is the single canonical wire-capturer. When invoked +with `LIVE_RECORD_DIR=`, it persists every captured response to +`//wire/.json` with the snapshot format +`{ operation, pages: [{status, headers, body, bodyText, url}], pages_count }`. + +The Ruby, Python, Go, and Kotlin runners each have a *wire-replay mode* that +reads those snapshots and decodes each page through their SDK. No HTTP calls, +no mock servers — the input is the canonical wire bytes captured by the TS +runner. Decode results land at +`//decode//.json` with the format +`{ schema_version, operation, pages: [{decoded, decode_error, missing_required, extras_seen}] }`. + +Each runner enforces three coverage gates at startup before doing any decode +work: + +1. **Decoder coverage** — every operation in `live-my-surface.json` has a + decode case in this runner. +2. **Snapshot completeness** — every operation in `live-my-surface.json` has + a corresponding snapshot file at `//wire/`. +3. **Snapshot recognition** — every snapshot's `operation` field is in + `live-my-surface.json` (catches drift between TS dispatch and the shared + fixture). + +Each gate prints which operations triggered it so the operator can fix the +right side: TS dispatch, the fixture, or the replay runner. + +Two-stage flow: + +```bash +# Step 1: TS captures canonical wire snapshots (live HTTP, requires creds). +BASECAMP_LIVE=1 \ +BASECAMP_TOKEN= \ +BASECAMP_ACCOUNT_ID= \ +BASECAMP_BACKEND=bc4 \ +LIVE_RECORD_DIR=tmp/canary \ +make conformance-typescript-live + +# Step 2: each language replays those snapshots through its SDK (offline). +for lang in ruby python go kotlin; do + WIRE_REPLAY_DIR=tmp/canary BASECAMP_BACKEND=bc4 \ + make conformance-$lang-replay +done +``` + +Step 2 needs no credentials and no network — it's pure decode + walk. The +extras-observed output across languages is a consistency check on the +hand-rolled schema walkers (which mirror the TS validator's required + extras +algorithm in each language). Any per-language divergence in `extras_seen` +points at a walker bug in the diverging language. + +`make conformance-typescript-live` is owned by PR 2; the four +`make conformance-*-replay` targets ship in PR 3. The orchestrating +`make conformance-live` and `make check-bc5-compat` targets that thread +TS capture → replay → BC4↔BC5 comparison together land in PR 4. + +When the SDK gains a new operation in `live-my-surface.json`, it must be +added to: + +- `conformance/runner/typescript/live-dispatch.ts` — TS dispatch case. +- `conformance/runner/ruby/replay-runner.rb` — Ruby decoder. +- `conformance/runner/python/replay_runner.py` — Python decoder. +- `conformance/runner/go/replay_runner.go` — Go decoder. +- `kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt` — Kotlin decoder. + +Each runner's coverage gate refuses to start until all five are in place. ## API gap registry (`spec/api-gaps/`) diff --git a/Makefile b/Makefile index 9a160333..6de28bdd 100644 --- a/Makefile +++ b/Makefile @@ -370,7 +370,7 @@ py-clean: # Conformance Test targets #------------------------------------------------------------------------------ -.PHONY: conformance conformance-go conformance-kotlin conformance-typescript conformance-typescript-live conformance-ruby conformance-python conformance-build +.PHONY: conformance conformance-go conformance-go-replay conformance-kotlin conformance-kotlin-replay conformance-typescript conformance-typescript-live conformance-ruby conformance-ruby-replay conformance-python conformance-python-replay conformance-build # Build conformance test runner conformance-build: @@ -382,11 +382,27 @@ conformance-go: conformance-build @echo "==> Running Go conformance tests..." cd conformance/runner/go && ./conformance-runner +# Run Go wire-replay against snapshots written by the TS live runner. +# Required env: WIRE_REPLAY_DIR, BASECAMP_BACKEND. Opt-in: not in `make check`. +conformance-go-replay: + @echo "==> Running Go wire-replay runner..." + @test -n "$$WIRE_REPLAY_DIR" || (echo "WIRE_REPLAY_DIR is required" >&2; exit 1) + @test -n "$$BASECAMP_BACKEND" || (echo "BASECAMP_BACKEND is required" >&2; exit 1) + cd conformance/runner/go && go run . + # Run Kotlin conformance tests conformance-kotlin: @echo "==> Running Kotlin conformance tests..." cd kotlin && ./gradlew :conformance:run +# Run Kotlin wire-replay against snapshots written by the TS live runner. +# Required env: WIRE_REPLAY_DIR, BASECAMP_BACKEND. Opt-in: not in `make check`. +conformance-kotlin-replay: + @echo "==> Running Kotlin wire-replay runner..." + @test -n "$$WIRE_REPLAY_DIR" || (echo "WIRE_REPLAY_DIR is required" >&2; exit 1) + @test -n "$$BASECAMP_BACKEND" || (echo "BASECAMP_BACKEND is required" >&2; exit 1) + cd kotlin && ./gradlew --quiet :conformance:runReplay + # Run TypeScript conformance tests conformance-typescript: @echo "==> Running TypeScript conformance tests..." @@ -408,11 +424,27 @@ conformance-ruby: @echo "==> Running Ruby conformance tests..." cd conformance/runner/ruby && bundle install --quiet && ruby runner.rb +# Run Ruby wire-replay against snapshots written by the TS live runner. +# Required env: WIRE_REPLAY_DIR, BASECAMP_BACKEND. Opt-in: not in `make check`. +conformance-ruby-replay: + @echo "==> Running Ruby wire-replay runner..." + @test -n "$$WIRE_REPLAY_DIR" || (echo "WIRE_REPLAY_DIR is required" >&2; exit 1) + @test -n "$$BASECAMP_BACKEND" || (echo "BASECAMP_BACKEND is required" >&2; exit 1) + cd conformance/runner/ruby && bundle install --quiet && ruby replay-runner.rb + # Run Python conformance tests conformance-python: @echo "==> Running Python conformance tests..." cd conformance/runner/python && uv sync && uv run python runner.py +# Run Python wire-replay against snapshots written by the TS live runner. +# Required env: WIRE_REPLAY_DIR, BASECAMP_BACKEND. Opt-in: not in `make check`. +conformance-python-replay: + @echo "==> Running Python wire-replay runner..." + @test -n "$$WIRE_REPLAY_DIR" || (echo "WIRE_REPLAY_DIR is required" >&2; exit 1) + @test -n "$$BASECAMP_BACKEND" || (echo "BASECAMP_BACKEND is required" >&2; exit 1) + cd conformance/runner/python && uv sync && uv run python replay_runner.py + # Run all conformance tests conformance: conformance-go conformance-kotlin conformance-typescript conformance-ruby conformance-python @echo "==> Conformance tests passed" @@ -678,13 +710,18 @@ help: @echo " swift-clean Remove Swift build artifacts" @echo "" @echo "Conformance:" - @echo " conformance Run all conformance tests" - @echo " conformance-go Run Go conformance tests" - @echo " conformance-kotlin Run Kotlin conformance tests" - @echo " conformance-typescript Run TypeScript conformance tests" - @echo " conformance-ruby Run Ruby conformance tests" - @echo " conformance-python Run Python conformance tests" - @echo " conformance-build Build Go conformance test runner" + @echo " conformance Run all conformance tests" + @echo " conformance-go Run Go conformance tests" + @echo " conformance-go-replay Decode TS-captured wire snapshots through Go SDK" + @echo " conformance-kotlin Run Kotlin conformance tests" + @echo " conformance-kotlin-replay Decode TS-captured wire snapshots through Kotlin SDK" + @echo " conformance-typescript Run TypeScript conformance tests" + @echo " conformance-typescript-live Run TypeScript live canary against a real backend" + @echo " conformance-ruby Run Ruby conformance tests" + @echo " conformance-ruby-replay Decode TS-captured wire snapshots through Ruby SDK" + @echo " conformance-python Run Python conformance tests" + @echo " conformance-python-replay Decode TS-captured wire snapshots through Python SDK" + @echo " conformance-build Build Go conformance test runner" @echo "" @echo "Ruby SDK:" @echo " rb-generate Generate types and metadata from OpenAPI" From 9c52e34355e194a862f032fd2ddce1bc9654d629 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 8 May 2026 14:34:59 -0700 Subject: [PATCH 07/19] conformance(ruby): align walker required-paths with `/`; fix mode-gate 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. --- conformance/runner/ruby/replay-runner.rb | 8 ++++---- conformance/runner/ruby/schema-walker.rb | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/conformance/runner/ruby/replay-runner.rb b/conformance/runner/ruby/replay-runner.rb index 32595370..7d26bba0 100644 --- a/conformance/runner/ruby/replay-runner.rb +++ b/conformance/runner/ruby/replay-runner.rb @@ -8,10 +8,10 @@ # through the Ruby SDK's decode boundary, and writes a per-test # decode-result snapshot under //decode/ruby/. # -# Mode-gate: this runner only does anything when WIRE_REPLAY_DIR is set. -# When unset, the existing mock runner.rb handles things and this file is a -# no-op if invoked. The make target only invokes this when WIRE_REPLAY_DIR -# is set. +# Mode-gate: invoking this script directly aborts unless WIRE_REPLAY_DIR +# and BASECAMP_BACKEND are set. The make target `conformance-ruby-replay` +# is the intended entrypoint and enforces both env vars in its preflight; +# the existing mock runner.rb is unaffected. require "bundler/setup" require "basecamp" diff --git a/conformance/runner/ruby/schema-walker.rb b/conformance/runner/ruby/schema-walker.rb index 35ebcc82..60bbf150 100644 --- a/conformance/runner/ruby/schema-walker.rb +++ b/conformance/runner/ruby/schema-walker.rb @@ -91,9 +91,12 @@ def walk_required(prefix, body, schema, depth) required = resolved["required"] || [] missing = [] + # Required-field paths use `/` as the separator (extras_seen uses `.`) + # so the two streams are visually distinct in tooling and consistent + # across Ruby/Python/Go/Kotlin walkers. required.each do |name| unless body.key?(name) - field_path = prefix.empty? ? name : "#{prefix}.#{name}" + field_path = prefix.empty? ? name : "#{prefix}/#{name}" missing << field_path end end @@ -101,7 +104,7 @@ def walk_required(prefix, body, schema, depth) body.each do |key, value| next unless props.key?(key) - field_path = prefix.empty? ? key : "#{prefix}.#{key}" + field_path = prefix.empty? ? key : "#{prefix}/#{key}" missing.concat(walk_required(field_path, value, props[key], depth + 1)) end From f50f22c0e6a8c51f4748c08db2198f6d70d5c849 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 8 May 2026 14:35:04 -0700 Subject: [PATCH 08/19] conformance(go): align walker required-paths with `/` 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. --- conformance/runner/go/schema_walker.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/conformance/runner/go/schema_walker.go b/conformance/runner/go/schema_walker.go index 10c4dafa..45700667 100644 --- a/conformance/runner/go/schema_walker.go +++ b/conformance/runner/go/schema_walker.go @@ -130,11 +130,13 @@ func (w *SchemaWalker) walkRequired(prefix string, body, schema any, depth int) props, _ := s["properties"].(map[string]any) required, _ := s["required"].([]any) var missing []string - // Required check: iterate the required array in declaration order. + // Required-field paths use `/` as the separator (walkExtras uses `.`) + // so the two streams are visually distinct in tooling and consistent + // across Ruby/Python/Go/Kotlin walkers. for _, r := range required { name, _ := r.(string) if _, ok := obj[name]; !ok { - missing = append(missing, joinDot(prefix, name)) + missing = append(missing, joinSlash(prefix, name)) } } // Recurse into present known props. Sort keys: cross-language @@ -142,7 +144,7 @@ func (w *SchemaWalker) walkRequired(prefix string, body, schema any, depth int) // is randomized, so sorted keys keep diffs stable). for _, name := range sortedKeys(props) { if value, ok := obj[name]; ok { - missing = append(missing, w.walkRequired(joinDot(prefix, name), value, props[name], depth+1)...) + missing = append(missing, w.walkRequired(joinSlash(prefix, name), value, props[name], depth+1)...) } } return missing @@ -267,6 +269,13 @@ func joinDot(prefix, name string) string { return prefix + "." + name } +func joinSlash(prefix, name string) string { + if prefix == "" { + return name + } + return prefix + "/" + name +} + func joinIndex(prefix string, i int) string { suffix := "[" + itoa(i) + "]" if prefix == "" { From 79727d86228856758c7d816f2be0a4c644735403 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 8 May 2026 14:35:14 -0700 Subject: [PATCH 09/19] conformance(kotlin): tighten env-var gate + narrow Throwable catches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 `//...` 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. --- kotlin/conformance/build.gradle.kts | 7 ++++--- .../com/basecamp/sdk/conformance/ReplayRunner.kt | 14 ++++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/kotlin/conformance/build.gradle.kts b/kotlin/conformance/build.gradle.kts index e7ed0d53..a5049006 100644 --- a/kotlin/conformance/build.gradle.kts +++ b/kotlin/conformance/build.gradle.kts @@ -23,9 +23,10 @@ tasks.register("runReplay") { classpath = sourceSets["main"].runtimeClasspath mainClass.set("com.basecamp.sdk.conformance.ReplayRunnerKt") workingDir = rootProject.projectDir - // Pass through the env vars the runner reads. - environment("WIRE_REPLAY_DIR", System.getenv("WIRE_REPLAY_DIR") ?: "") - environment("BASECAMP_BACKEND", System.getenv("BASECAMP_BACKEND") ?: "") + // JavaExec inherits the parent process environment by default; we don't + // explicitly set WIRE_REPLAY_DIR/BASECAMP_BACKEND because forcing them in + // (with `?: ""` fallbacks) would smuggle empty strings into the child env + // and bypass the runner's "is required" gate. } dependencies { diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt index 5d407640..4fa7d4a8 100644 --- a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt @@ -208,7 +208,10 @@ class ReplayRunner( try { decoder(bodyText) decoded = true - } catch (e: Throwable) { + } catch (e: Exception) { + // Catch Exception, not Throwable: fatal JVM errors (OOM, + // StackOverflow, LinkageError) shouldn't be silently demoted + // to a decode-failure record. decodeError = "${e::class.simpleName}: ${e.message}" } @@ -219,7 +222,7 @@ class ReplayRunner( val body = parserJson.parseToJsonElement(bodyText) missing = walker.missingRequired(body, schema) extras = walker.extrasSeen(body, schema) - } catch (_: Throwable) { + } catch (_: Exception) { // Body is not parseable JSON; decode_error above already // records the failure. Leave walker output empty. } @@ -232,9 +235,12 @@ class ReplayRunner( } fun main() { - val replayDir = System.getenv("WIRE_REPLAY_DIR") + // Treat empty/blank as missing — Gradle and shells can pass empty strings + // through, and those would otherwise slip past a `?: null` guard and let + // the runner read/write relative to CWD or `//...`. + val replayDir = System.getenv("WIRE_REPLAY_DIR")?.takeUnless { it.isBlank() } ?: run { System.err.println("WIRE_REPLAY_DIR is required"); exitProcess(1) } - val backend = System.getenv("BASECAMP_BACKEND") + val backend = System.getenv("BASECAMP_BACKEND")?.takeUnless { it.isBlank() } ?: run { System.err.println("BASECAMP_BACKEND is required"); exitProcess(1) } // Resolve repo paths relative to the kotlin/ root. The Gradle `runReplay` From 6422d34bcb6c1075b7fcd8130552f7b1cf887443 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 12 May 2026 09:06:39 -0700 Subject: [PATCH 10/19] conformance(go,ruby): fix walker docstring drift on required-path separator 3d771f08 / 04e9eea9 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. --- conformance/runner/go/schema_walker.go | 14 +++++++++----- conformance/runner/ruby/schema-walker.rb | 13 +++++++++---- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/conformance/runner/go/schema_walker.go b/conformance/runner/go/schema_walker.go index 45700667..a8da35b3 100644 --- a/conformance/runner/go/schema_walker.go +++ b/conformance/runner/go/schema_walker.go @@ -3,13 +3,17 @@ package main // Pure-Go port of conformance/runner/typescript/schema-validator.ts. // // Walks parsed JSON against the OpenAPI response schema, surfacing: -// - MissingRequired: required-field paths absent from the body -// - ExtrasSeen: field paths present on the wire but not declared +// - MissingRequired: slash-separated paths for required fields absent +// from the body (e.g. "owner/id") +// - ExtrasSeen: dotted paths for fields present on the wire but not +// declared (e.g. "unreads[].new_field") // // Conventions match the TS walker (and the Python/Ruby ports under // conformance/runner/{python,ruby}/) so cross-language extras parity diffs // (PR 4 §Verification) don't false-fire: -// - Object paths use "." (e.g. "owner.id") +// - Required-walk object paths use "/" (e.g. "owner/id"); extras-walk +// object paths use "." (e.g. "owner.new_field"). The two streams use +// distinct separators so they're visually distinguishable in tooling. // - Required walk uses "[i]" element segments; extras walk uses "[]" // to dedupe item-level extras across an array // - "$ref" chains resolve until a non-ref schema or a cycle. Both @@ -83,8 +87,8 @@ func (w *SchemaWalker) FindResponseSchema(operationID string) map[string]any { return nil } -// MissingRequired returns dotted-path strings for required fields absent -// from body. +// MissingRequired returns slash-separated path strings for required fields +// absent from body (e.g. "owner/id"). func (w *SchemaWalker) MissingRequired(body any, schema map[string]any) []string { return w.walkRequired("", body, schema, 0) } diff --git a/conformance/runner/ruby/schema-walker.rb b/conformance/runner/ruby/schema-walker.rb index 60bbf150..78a5b354 100644 --- a/conformance/runner/ruby/schema-walker.rb +++ b/conformance/runner/ruby/schema-walker.rb @@ -5,12 +5,16 @@ # Pure-Ruby port of conformance/runner/typescript/schema-validator.ts. # # Walks parsed JSON against the OpenAPI response schema, surfacing: -# - missing_required: required-field paths absent from the body -# - extras_seen: field paths present on the wire but not declared in the schema +# - missing_required: slash-separated paths for required fields absent +# from the body (e.g. "owner/id") +# - extras_seen: dotted paths for fields present on the wire but not +# declared in the schema (e.g. "unreads[].new_field") # # Conventions match the TS walker exactly so cross-language replay output is # directly comparable: -# - Object paths use "." (e.g. "owner.id") +# - Required-walk object paths use "/" (e.g. "owner/id"); extras-walk +# object paths use "." (e.g. "owner.new_field"). The two streams use +# distinct separators so they're visually distinguishable in tooling. # - Array element paths use "[i]" for required walk, "[]" for extras walk # (mirrors how the TS validator's collectExtras tags item-level extras and # how a per-index required-field check identifies the offending element). @@ -53,7 +57,8 @@ def find_response_schema(operation_id) nil end - # Returns array of dotted-path strings for required fields absent from body. + # Returns array of slash-separated path strings for required fields + # absent from body (e.g. "owner/id"). def missing_required(body, schema) walk_required("", body, schema, 0) end From 34d4faa5a51d9da61e2a89eb60b67d6c9dfd5fdb Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 13 May 2026 11:42:06 -0700 Subject: [PATCH 11/19] conformance(python,go): preserve empty bodyText on wire snapshots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- conformance/runner/go/replay_runner.go | 9 ++++++--- conformance/runner/python/replay_runner.py | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/conformance/runner/go/replay_runner.go b/conformance/runner/go/replay_runner.go index 9cde4220..e6020a45 100644 --- a/conformance/runner/go/replay_runner.go +++ b/conformance/runner/go/replay_runner.go @@ -110,7 +110,7 @@ type wirePage struct { Status int `json:"status"` Headers map[string]string `json:"headers"` Body any `json:"body"` - BodyText string `json:"bodyText"` + BodyText *string `json:"bodyText"` URL string `json:"url"` } @@ -295,8 +295,11 @@ func (r *ReplayRunner) decodeSnapshot(snap *wireSnapshot) ReplayResult { pages := make([]ReplayPage, 0, len(snap.Pages)) for _, page := range snap.Pages { - bodyText := page.BodyText - if bodyText == "" && page.Body != nil { + var bodyText string + switch { + case page.BodyText != nil: + bodyText = *page.BodyText + case page.Body != nil: if b, err := json.Marshal(page.Body); err == nil { bodyText = string(b) } diff --git a/conformance/runner/python/replay_runner.py b/conformance/runner/python/replay_runner.py index a07f9f98..0cf1ae7d 100644 --- a/conformance/runner/python/replay_runner.py +++ b/conformance/runner/python/replay_runner.py @@ -160,7 +160,8 @@ def _decode_snapshot(self, snapshot: dict) -> dict: pages = [] for page in snapshot["pages"]: - body_text: str = page.get("bodyText") or json.dumps(page["body"]) + raw = page.get("bodyText") + body_text: str = raw if raw is not None else json.dumps(page["body"]) decoded = False decode_error: str | None = None missing_required: list[str] = [] From c028679cd21c263303bb07e85e21c6a5408edfae Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 13 May 2026 12:56:49 -0700 Subject: [PATCH 12/19] conformance: align default-response precedence with documented order MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- conformance/runner/go/schema_walker.go | 5 ++++- conformance/runner/python/schema_walker.py | 5 ++++- conformance/runner/ruby/schema-walker.rb | 6 ++++-- conformance/runner/typescript/schema-validator.ts | 8 ++++++-- .../kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt | 8 ++++++-- 5 files changed, 24 insertions(+), 8 deletions(-) diff --git a/conformance/runner/go/schema_walker.go b/conformance/runner/go/schema_walker.go index a8da35b3..1b0f970e 100644 --- a/conformance/runner/go/schema_walker.go +++ b/conformance/runner/go/schema_walker.go @@ -70,7 +70,7 @@ func (w *SchemaWalker) FindResponseSchema(operationID string) map[string]any { continue } responses, _ := op["responses"].(map[string]any) - for _, code := range []string{"200", "201", "202", "203", "204", "default"} { + for _, code := range []string{"200", "201", "202", "203", "204"} { if s := schemaFor(responses[code]); s != nil { return s } @@ -82,6 +82,9 @@ func (w *SchemaWalker) FindResponseSchema(operationID string) map[string]any { } } } + if s := schemaFor(responses["default"]); s != nil { + return s + } } } return nil diff --git a/conformance/runner/python/schema_walker.py b/conformance/runner/python/schema_walker.py index 54c44852..26f0355d 100644 --- a/conformance/runner/python/schema_walker.py +++ b/conformance/runner/python/schema_walker.py @@ -48,7 +48,7 @@ def find_response_schema(self, operation_id: str) -> dict | None: continue responses = op.get("responses") or {} # Match TS preference order: 200 first, then any 2xx, then default. - for code in ("200", "201", "202", "203", "204", "default"): + for code in ("200", "201", "202", "203", "204"): schema = _schema_for(responses.get(code)) if schema is not None: return schema @@ -57,6 +57,9 @@ def find_response_schema(self, operation_id: str) -> dict | None: schema = _schema_for(response) if schema is not None: return schema + schema = _schema_for(responses.get("default")) + if schema is not None: + return schema return None def missing_required(self, body: Any, schema: dict) -> list[str]: diff --git a/conformance/runner/ruby/schema-walker.rb b/conformance/runner/ruby/schema-walker.rb index 78a5b354..843556e8 100644 --- a/conformance/runner/ruby/schema-walker.rb +++ b/conformance/runner/ruby/schema-walker.rb @@ -34,9 +34,9 @@ def initialize(openapi_path) end # Returns the response schema for operation_id, or nil when none exists. - # Prefers 200, then any explicit 2xx, then "default". + # Prefers 200, then any 2xx, then "default" — matches TS. def find_response_schema(operation_id) - candidates = %w[200 201 202 203 204 default] + candidates = %w[200 201 202 203 204] @doc.fetch("paths", {}).each_value do |path_item| path_item.each_value do |op| next unless op.is_a?(Hash) && op["operationId"] == operation_id @@ -52,6 +52,8 @@ def find_response_schema(operation_id) schema = response.dig("content", "application/json", "schema") return schema if schema end + schema = responses.dig("default", "content", "application/json", "schema") + return schema if schema end end nil diff --git a/conformance/runner/typescript/schema-validator.ts b/conformance/runner/typescript/schema-validator.ts index 1f2d13d2..baaea65b 100644 --- a/conformance/runner/typescript/schema-validator.ts +++ b/conformance/runner/typescript/schema-validator.ts @@ -72,18 +72,22 @@ function findResponseSchema(doc: OpenAPIDocument, operationId: string): unknown // last resort is "default". Operations that return 201 (Create*) // shouldn't fall back to "" because their response body still has // a schema worth validating. - const candidates = ["200", "201", "202", "203", "204", "default"]; + const candidates = ["200", "201", "202", "203", "204"]; for (const code of candidates) { if (!responses[code]) continue; const schema = responses[code].content?.["application/json"]?.schema; if (schema) return schema; } - // Last resort: any 2xx key not in the explicit list above. + // Any 2xx key not in the explicit list above (e.g. 205-299). for (const [code, response] of Object.entries(responses)) { if (!/^2\d\d$/.test(code)) continue; const schema = response.content?.["application/json"]?.schema; if (schema) return schema; } + // Last resort: "default". + const defaultSchema = + responses["default"]?.content?.["application/json"]?.schema; + if (defaultSchema) return defaultSchema; } } return null; diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt index 1fa46343..8f64147d 100644 --- a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/SchemaWalker.kt @@ -42,7 +42,10 @@ class SchemaWalker(openapiPath: String) { private val schemas: JsonObject = doc["components"]?.jsonObject?.get("schemas")?.jsonObject ?: JsonObject(emptyMap()) - /** Returns null when no response schema is found (do not throw). */ + /** + * Returns null when no response schema is found (do not throw). + * Preference order: 200, then any 2xx, then "default" — matches TS. + */ fun findResponseSchema(operationId: String): JsonObject? { val paths = doc["paths"]?.jsonObject ?: return null for ((_, pathItemEl) in paths) { @@ -60,6 +63,7 @@ class SchemaWalker(openapiPath: String) { schemaFor(response)?.let { return it } } } + schemaFor(responses["default"])?.let { return it } } } return null @@ -172,7 +176,7 @@ class SchemaWalker(openapiPath: String) { companion object { private const val MAX_DEPTH = 12 - private val PREFERRED_CODES = listOf("200", "201", "202", "203", "204", "default") + private val PREFERRED_CODES = listOf("200", "201", "202", "203", "204") private val REF_REGEX = Regex("^(?:openapi\\.json)?#/components/schemas/(.+)$") } } From 27162f10de298d8aa8864c65b34098c42750edb1 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 13 May 2026 12:56:54 -0700 Subject: [PATCH 13/19] conformance(kotlin): drop unused JsonArray and encodeToJsonElement imports --- .../main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt index 4fa7d4a8..5fa91130 100644 --- a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt @@ -8,11 +8,9 @@ import com.basecamp.sdk.generated.models.Todoset import kotlinx.serialization.Serializable import kotlinx.serialization.builtins.ListSerializer import kotlinx.serialization.json.Json -import kotlinx.serialization.json.JsonArray import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.contentOrNull -import kotlinx.serialization.json.encodeToJsonElement import kotlinx.serialization.json.jsonArray import kotlinx.serialization.json.jsonObject import kotlinx.serialization.json.jsonPrimitive From 751b72223bce45a365c2c410202a0624d67564cb Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 13 May 2026 13:12:08 -0700 Subject: [PATCH 14/19] conformance: faithful empty-body decoding + Kotlin gate hardening MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 a1bd6df1) 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. --- conformance/runner/go/replay_runner.go | 27 +++++--- conformance/runner/go/replay_runner_test.go | 62 +++++++++++++++++++ conformance/runner/python/replay_runner.py | 15 ++++- .../runner/python/test_replay_runner.py | 43 +++++++++++++ conformance/runner/ruby/Gemfile | 1 + conformance/runner/ruby/replay-runner.rb | 6 +- conformance/runner/ruby/replay_runner_test.rb | 30 +++++++++ .../basecamp/sdk/conformance/ReplayRunner.kt | 6 +- 8 files changed, 176 insertions(+), 14 deletions(-) create mode 100644 conformance/runner/go/replay_runner_test.go create mode 100644 conformance/runner/python/test_replay_runner.py create mode 100644 conformance/runner/ruby/replay_runner_test.rb diff --git a/conformance/runner/go/replay_runner.go b/conformance/runner/go/replay_runner.go index e6020a45..88b8a049 100644 --- a/conformance/runner/go/replay_runner.go +++ b/conformance/runner/go/replay_runner.go @@ -114,6 +114,23 @@ type wirePage struct { URL string `json:"url"` } +// resolveBodyText returns the bytes the decoder should see for a wire-snapshot +// page. Empty-but-present bodyText (HTTP 204 or an actually-empty 200 body) +// must flow through as "" so the decoder errors — not be silently replaced +// with a re-serialized body, which would mask a real decode failure. *string +// distinguishes a missing key (nil) from an empty value (&""). +func resolveBodyText(page wirePage) string { + switch { + case page.BodyText != nil: + return *page.BodyText + case page.Body != nil: + if b, err := json.Marshal(page.Body); err == nil { + return string(b) + } + } + return "" +} + type wireSnapshot struct { Operation string `json:"operation"` Pages []wirePage `json:"pages"` @@ -295,15 +312,7 @@ func (r *ReplayRunner) decodeSnapshot(snap *wireSnapshot) ReplayResult { pages := make([]ReplayPage, 0, len(snap.Pages)) for _, page := range snap.Pages { - var bodyText string - switch { - case page.BodyText != nil: - bodyText = *page.BodyText - case page.Body != nil: - if b, err := json.Marshal(page.Body); err == nil { - bodyText = string(b) - } - } + bodyText := resolveBodyText(page) rp := ReplayPage{Decoded: false} if err := decoder(bodyText); err == nil { diff --git a/conformance/runner/go/replay_runner_test.go b/conformance/runner/go/replay_runner_test.go new file mode 100644 index 00000000..82aa47c0 --- /dev/null +++ b/conformance/runner/go/replay_runner_test.go @@ -0,0 +1,62 @@ +// Regression test for the empty-bodyText decode-masking bug. +// +// Pre-fix, BodyText was a `string` so encoding/json zero-fills a missing +// key with "". The decode path then conflated "" (missing) with "" (empty +// HTTP body) and re-serialized `body` instead, silently green-passing an +// actually-empty wire payload. Post-fix, BodyText is `*string` and +// resolveBodyText distinguishes nil (missing) from &"" (empty), letting +// the decoder fail honestly on an empty body. + +package main + +import ( + "encoding/json" + "testing" +) + +func strPtr(s string) *string { return &s } + +func TestResolveBodyText_EmptyPassesThrough(t *testing.T) { + got := resolveBodyText(wirePage{BodyText: strPtr("")}) + if got != "" { + t.Fatalf("empty bodyText should pass through as empty string; got %q", got) + } +} + +func TestResolveBodyText_MissingFallsBackToBody(t *testing.T) { + got := resolveBodyText(wirePage{Body: map[string]any{"a": 1}}) + want := `{"a":1}` + if got != want { + t.Fatalf("missing bodyText should serialize body; got %q want %q", got, want) + } +} + +func TestResolveBodyText_NonEmptyWinsOverBody(t *testing.T) { + got := resolveBodyText(wirePage{ + BodyText: strPtr(`{"b":2}`), + Body: map[string]any{"a": 1}, + }) + if got != `{"b":2}` { + t.Fatalf("bodyText should win over body; got %q", got) + } +} + +func TestDecoder_ErrorsOnEmptyBodyText(t *testing.T) { + // Composes the regression: empty bodyText → "" → decoder errors. + // Pre-fix this path would have green-passed because "" got replaced + // by `{}` before reaching the decoder. + text := resolveBodyText(wirePage{BodyText: strPtr("")}) + dec, ok := decoders["GetProject"] + if !ok { + t.Fatal("GetProject decoder missing from decoders map") + } + if err := dec(text); err == nil { + t.Fatal("decoder should error on empty bodyText; got nil") + } + // Sanity: a syntactically valid empty object should still decode cleanly. + if err := dec(`{}`); err != nil { + var se *json.SyntaxError + _ = se + t.Fatalf("decoder should accept {}; got %v", err) + } +} diff --git a/conformance/runner/python/replay_runner.py b/conformance/runner/python/replay_runner.py index 0cf1ae7d..e820a1b7 100644 --- a/conformance/runner/python/replay_runner.py +++ b/conformance/runner/python/replay_runner.py @@ -75,6 +75,18 @@ def _safe_name(name: str) -> str: return re.sub(r"[^a-z0-9_-]+", "_", name, flags=re.IGNORECASE) +def _resolve_body_text(page: dict) -> str: + """Return the bytes the decoder should see for a wire-snapshot page. + + Empty-but-present ``bodyText`` (HTTP 204 or an actually-empty 200 body) + must flow through as ``""`` so the decoder errors — not be silently + replaced with a re-serialized ``body`` field, which would mask a real + decode failure. + """ + raw = page.get("bodyText") + return raw if raw is not None else json.dumps(page["body"]) + + class ReplayRunner: def __init__(self, replay_dir: Path, backend: str, fixture_path: Path, openapi_path: Path) -> None: self._replay_dir = replay_dir @@ -160,8 +172,7 @@ def _decode_snapshot(self, snapshot: dict) -> dict: pages = [] for page in snapshot["pages"]: - raw = page.get("bodyText") - body_text: str = raw if raw is not None else json.dumps(page["body"]) + body_text: str = _resolve_body_text(page) decoded = False decode_error: str | None = None missing_required: list[str] = [] diff --git a/conformance/runner/python/test_replay_runner.py b/conformance/runner/python/test_replay_runner.py new file mode 100644 index 00000000..9def62be --- /dev/null +++ b/conformance/runner/python/test_replay_runner.py @@ -0,0 +1,43 @@ +"""Regression test for the empty-bodyText decode-masking bug. + +Pre-fix, an empty-but-present ``bodyText`` ("" for HTTP 204 or a genuinely +empty 200 body) was treated as falsy and silently replaced with a +re-serialized ``body: {}`` → ``"{}"``, which decoded successfully. That hid +a real decode failure (the production SDK calls ``json.loads`` on the raw +bytes and would error on ``""``). Post-fix, ``_resolve_body_text`` returns +``""`` directly so the decoder errors and the page reports ``decode_error``. + +Run: ``uv run python -m unittest test_replay_runner -v`` +""" + +from __future__ import annotations + +import json +import unittest + +from replay_runner import _decode, _resolve_body_text + + +class ResolveBodyTextTest(unittest.TestCase): + def test_empty_body_text_passes_through(self) -> None: + page = {"status": 204, "headers": {}, "body": {}, "bodyText": "", "url": ""} + self.assertEqual(_resolve_body_text(page), "") + + def test_missing_body_text_falls_back_to_serialized_body(self) -> None: + page = {"status": 200, "headers": {}, "body": {"a": 1}, "url": ""} + self.assertEqual(_resolve_body_text(page), json.dumps({"a": 1})) + + def test_non_empty_body_text_wins_over_body(self) -> None: + page = {"status": 200, "headers": {}, "body": {"a": 1}, "bodyText": '{"b":2}', "url": ""} + self.assertEqual(_resolve_body_text(page), '{"b":2}') + + def test_decoder_errors_on_empty_body_text(self) -> None: + # Composes the regression: empty bodyText → "" → decoder raises. + # Pre-fix this path would have green-passed because "" got replaced + # by "{}" before reaching the decoder. + with self.assertRaises(json.JSONDecodeError): + _decode(_resolve_body_text({"body": {}, "bodyText": ""})) + + +if __name__ == "__main__": + unittest.main() diff --git a/conformance/runner/ruby/Gemfile b/conformance/runner/ruby/Gemfile index a5f5487a..dae6e2be 100644 --- a/conformance/runner/ruby/Gemfile +++ b/conformance/runner/ruby/Gemfile @@ -5,3 +5,4 @@ source "https://rubygems.org" gem "basecamp-sdk", path: "../../../ruby" gem "webmock", "~> 3.0" gem "json" +gem "minitest" diff --git a/conformance/runner/ruby/replay-runner.rb b/conformance/runner/ruby/replay-runner.rb index 7d26bba0..8869aa19 100644 --- a/conformance/runner/ruby/replay-runner.rb +++ b/conformance/runner/ruby/replay-runner.rb @@ -31,8 +31,10 @@ class ReplayRunner # every page) is the canonical decoder boundary, so each lambda exercises # exactly that. SDK_DECODE = lambda do |body_text| - return nil if body_text.nil? || body_text.empty? - + # No empty-body guard: Basecamp::Http#json calls JSON.parse(@body) + # directly, so an empty body surfaces as a JSON::ParserError in + # production. The runner mirrors that to record decode_error rather + # than silently green-passing on an empty wire payload. parsed = JSON.parse(body_text) Basecamp::Http.normalize_person_ids(parsed) parsed diff --git a/conformance/runner/ruby/replay_runner_test.rb b/conformance/runner/ruby/replay_runner_test.rb new file mode 100644 index 00000000..204b32e0 --- /dev/null +++ b/conformance/runner/ruby/replay_runner_test.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# Regression test for the empty-bodyText decode-masking bug. +# +# Pre-fix, SDK_DECODE had `return nil if body_text.nil? || body_text.empty?`, +# which silently green-passed an empty body — diverging from the production +# Ruby SDK (Basecamp::Http#json calls JSON.parse(@body) without an empty-body +# guard, so an empty body raises JSON::ParserError). Post-fix, an empty +# bodyText flows into JSON.parse and surfaces as a decode_error. +# +# Run: `bundle exec ruby replay_runner_test.rb` + +require "json" +require "minitest/autorun" +require_relative "replay-runner" + +class SdkDecodeTest < Minitest::Test + def test_empty_body_raises_parser_error + assert_raises(JSON::ParserError) { ReplayRunner::SDK_DECODE.call("") } + end + + def test_well_formed_body_decodes_cleanly + result = ReplayRunner::SDK_DECODE.call(%({"a":1})) + assert_equal({ "a" => 1 }, result) + end + + def test_malformed_body_raises_parser_error + assert_raises(JSON::ParserError) { ReplayRunner::SDK_DECODE.call("not json") } + end +end diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt index 5fa91130..9a9cbc21 100644 --- a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt @@ -10,6 +10,7 @@ import kotlinx.serialization.builtins.ListSerializer import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.contentOrNull import kotlinx.serialization.json.jsonArray import kotlinx.serialization.json.jsonObject @@ -144,7 +145,10 @@ class ReplayRunner( wireDir.listFiles { f -> f.extension == "json" }?.forEach { f -> val snap = parserJson.parseToJsonElement(f.readText()) as? JsonObject ?: return@forEach - val op = snap["operation"]?.jsonPrimitive?.contentOrNull + // Defensive: `.jsonPrimitive` on JsonNull/JsonObject/JsonArray + // throws, which would crash the gate. Cast first so a malformed + // `operation` value emits the gate message instead. + val op = (snap["operation"] as? JsonPrimitive)?.contentOrNull if (op == null) { msgs += "Snapshot ${f.name} is missing the top-level `operation` field. " + "Re-run the TS live canary; pre-PR3 snapshots are no longer supported." From 148b0216862496aba02ac8f0ff53064dde122be5 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 13 May 2026 13:12:13 -0700 Subject: [PATCH 15/19] gitignore: stray Go binary from `go build` in conformance runner 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. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 74e0d332..1d71b141 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ spec/build/ # Conformance test runner artifacts conformance/runner/go/conformance-runner +conformance/runner/go/go conformance/runner/ruby/Gemfile.lock conformance/runner/typescript/node_modules/ From da5f49929509e3c5c4649f1d18b29551479a4983 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 13 May 2026 14:04:56 -0700 Subject: [PATCH 16/19] conformance: surface snapshot read/parse errors as gate messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- conformance/runner/go/replay_runner.go | 8 +++++++ conformance/runner/python/replay_runner.py | 9 +++++++- conformance/runner/ruby/replay-runner.rb | 12 +++++++++-- .../basecamp/sdk/conformance/ReplayRunner.kt | 21 +++++++++++++++++-- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/conformance/runner/go/replay_runner.go b/conformance/runner/go/replay_runner.go index 88b8a049..bb78301e 100644 --- a/conformance/runner/go/replay_runner.go +++ b/conformance/runner/go/replay_runner.go @@ -217,12 +217,20 @@ func (r *ReplayRunner) coverageGate() []string { } data, err := os.ReadFile(filepath.Join(wireDir, e.Name())) if err != nil { + msgs = append(msgs, fmt.Sprintf( + "Snapshot %s could not be read: %v.", + e.Name(), err, + )) continue } var snap struct { Operation string `json:"operation"` } if err := json.Unmarshal(data, &snap); err != nil { + msgs = append(msgs, fmt.Sprintf( + "Snapshot %s is not valid JSON: %v.", + e.Name(), err, + )) continue } if snap.Operation == "" { diff --git a/conformance/runner/python/replay_runner.py b/conformance/runner/python/replay_runner.py index e820a1b7..c599e9e1 100644 --- a/conformance/runner/python/replay_runner.py +++ b/conformance/runner/python/replay_runner.py @@ -124,7 +124,14 @@ def coverage_gate(self) -> list[str]: # must be in the shared fixture (catches TS-side dispatch drift). if wire_dir.exists(): for f in sorted(wire_dir.glob("*.json")): - snap = json.loads(f.read_text()) + try: + snap = json.loads(f.read_text()) + except OSError as e: + msgs.append(f"Snapshot {f.name} could not be read: {type(e).__name__}: {e}.") + continue + except json.JSONDecodeError as e: + msgs.append(f"Snapshot {f.name} is not valid JSON: {e}.") + continue op = snap.get("operation") if op is None: msgs.append( diff --git a/conformance/runner/ruby/replay-runner.rb b/conformance/runner/ruby/replay-runner.rb index 8869aa19..bed08ba2 100644 --- a/conformance/runner/ruby/replay-runner.rb +++ b/conformance/runner/ruby/replay-runner.rb @@ -110,9 +110,17 @@ def coverage_gate # 3. Snapshot recognition: every snapshot's operation is in the fixture. if Dir.exist?(wire_dir) Dir.glob(File.join(wire_dir, "*.json")).each do |f| - snap = JSON.parse(File.read(f)) - op = snap["operation"] + snap = begin + JSON.parse(File.read(f)) + rescue Errno::ENOENT, IOError, SystemCallError => e + msgs << "Snapshot #{File.basename(f)} could not be read: #{e.class}: #{e.message}." + next + rescue JSON::ParserError => e + msgs << "Snapshot #{File.basename(f)} is not valid JSON: #{e.message}." + next + end + op = snap["operation"] if op.nil? msgs << "Snapshot #{File.basename(f)} is missing the top-level `operation` field. " \ "Re-run the TS live canary; pre-PR3 snapshots are no longer supported." diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt index 9a9cbc21..250f689d 100644 --- a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt @@ -6,6 +6,7 @@ import com.basecamp.sdk.generated.models.Todo import com.basecamp.sdk.generated.models.Todolist import com.basecamp.sdk.generated.models.Todoset import kotlinx.serialization.Serializable +import kotlinx.serialization.SerializationException import kotlinx.serialization.builtins.ListSerializer import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonElement @@ -16,6 +17,7 @@ import kotlinx.serialization.json.jsonArray import kotlinx.serialization.json.jsonObject import kotlinx.serialization.json.jsonPrimitive import java.io.File +import java.io.IOException import java.nio.file.Path import java.nio.file.Paths import kotlin.system.exitProcess @@ -143,8 +145,23 @@ class ReplayRunner( // be in the shared fixture (catches TS-side dispatch drift). if (wireDir.exists()) { wireDir.listFiles { f -> f.extension == "json" }?.forEach { f -> - val snap = parserJson.parseToJsonElement(f.readText()) as? JsonObject - ?: return@forEach + val text = try { + f.readText() + } catch (e: IOException) { + msgs += "Snapshot ${f.name} could not be read: ${e::class.simpleName}: ${e.message}." + return@forEach + } + val parsed = try { + parserJson.parseToJsonElement(text) + } catch (e: SerializationException) { + msgs += "Snapshot ${f.name} is not valid JSON: ${e.message}." + return@forEach + } + val snap = parsed as? JsonObject + if (snap == null) { + msgs += "Snapshot ${f.name} top-level JSON is not an object; expected the wire-snapshot envelope." + return@forEach + } // Defensive: `.jsonPrimitive` on JsonNull/JsonObject/JsonArray // throws, which would crash the gate. Cast first so a malformed // `operation` value emits the gate message instead. From ce68aa11601ddf9a8e1dcb4b64c39425069eae25 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 13 May 2026 15:57:11 -0700 Subject: [PATCH 17/19] conformance(python): catch UnicodeDecodeError on snapshot read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- conformance/runner/python/replay_runner.py | 6 ++ .../runner/python/test_replay_runner.py | 60 ++++++++++++++++--- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/conformance/runner/python/replay_runner.py b/conformance/runner/python/replay_runner.py index c599e9e1..08d3b206 100644 --- a/conformance/runner/python/replay_runner.py +++ b/conformance/runner/python/replay_runner.py @@ -129,6 +129,12 @@ def coverage_gate(self) -> list[str]: except OSError as e: msgs.append(f"Snapshot {f.name} could not be read: {type(e).__name__}: {e}.") continue + except UnicodeDecodeError as e: + # Path.read_text() decodes UTF-8 by default; malformed + # bytes raise UnicodeDecodeError (a ValueError, NOT an + # OSError), so it must be caught separately. + msgs.append(f"Snapshot {f.name} is not valid UTF-8: {e}.") + continue except json.JSONDecodeError as e: msgs.append(f"Snapshot {f.name} is not valid JSON: {e}.") continue diff --git a/conformance/runner/python/test_replay_runner.py b/conformance/runner/python/test_replay_runner.py index 9def62be..71b49c3c 100644 --- a/conformance/runner/python/test_replay_runner.py +++ b/conformance/runner/python/test_replay_runner.py @@ -1,11 +1,21 @@ -"""Regression test for the empty-bodyText decode-masking bug. +"""Regression tests for the wire-replay runner. -Pre-fix, an empty-but-present ``bodyText`` ("" for HTTP 204 or a genuinely -empty 200 body) was treated as falsy and silently replaced with a -re-serialized ``body: {}`` → ``"{}"``, which decoded successfully. That hid -a real decode failure (the production SDK calls ``json.loads`` on the raw -bytes and would error on ``""``). Post-fix, ``_resolve_body_text`` returns -``""`` directly so the decoder errors and the page reports ``decode_error``. +Covers two known bugs: + + * Empty-bodyText decode masking — pre-fix, an empty-but-present + ``bodyText`` ("" for HTTP 204 or a genuinely empty 200 body) was treated + as falsy and silently replaced with a re-serialized ``body: {}`` → + ``"{}"``, which decoded successfully. That hid a real decode failure + (the production SDK calls ``json.loads`` on the raw bytes and would + error on ``""``). Post-fix, ``_resolve_body_text`` returns ``""`` + directly so the decoder errors and the page reports ``decode_error``. + + * Malformed-UTF-8 coverage gate — pre-fix, the coverage-gate snapshot + reader caught ``OSError`` and ``json.JSONDecodeError`` but not + ``UnicodeDecodeError`` from ``Path.read_text()``. A snapshot containing + invalid UTF-8 bytes crashed the gate instead of emitting a clear + diagnostic. Post-fix, the gate appends a "not valid UTF-8" message and + keeps going. Run: ``uv run python -m unittest test_replay_runner -v`` """ @@ -13,9 +23,11 @@ from __future__ import annotations import json +import tempfile import unittest +from pathlib import Path -from replay_runner import _decode, _resolve_body_text +from replay_runner import ReplayRunner, _decode, _resolve_body_text class ResolveBodyTextTest(unittest.TestCase): @@ -39,5 +51,37 @@ def test_decoder_errors_on_empty_body_text(self) -> None: _decode(_resolve_body_text({"body": {}, "bodyText": ""})) +class CoverageGateUtf8Test(unittest.TestCase): + def test_malformed_utf8_snapshot_yields_gate_message(self) -> None: + # Pre-fix: Path.read_text() raised UnicodeDecodeError, which is a + # ValueError (not OSError) and so escaped the gate's exception + # filters and crashed the runner. Post-fix: the gate appends a + # clear "not valid UTF-8" diagnostic and continues. + with tempfile.TemporaryDirectory() as tmp: + tmpdir = Path(tmp) + backend = "bc4" + test_name = "GetProject" + + fixture_path = tmpdir / "live.json" + fixture_path.write_text(json.dumps([ + {"name": test_name, "mode": "live", "operation": "GetProject"} + ])) + + openapi_path = tmpdir / "openapi.json" + openapi_path.write_text("{}") + + wire_dir = tmpdir / "replay" / backend / "wire" + wire_dir.mkdir(parents=True) + (wire_dir / f"{test_name}.json").write_bytes(b"\xff\xfe{\"operation\":\"GetProject\"}") + + runner = ReplayRunner(tmpdir / "replay", backend, fixture_path, openapi_path) + msgs = runner.coverage_gate() + + self.assertTrue( + any("not valid UTF-8" in m for m in msgs), + f"expected 'not valid UTF-8' diagnostic in {msgs!r}", + ) + + if __name__ == "__main__": unittest.main() From 09c73f614f8c79fd09df06d76c334074b95ad2d7 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 13 May 2026 20:57:08 -0700 Subject: [PATCH 18/19] conformance: address four Copilot threads on PR #301 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- conformance/runner/go/replay_runner.go | 10 +++ conformance/runner/go/replay_runner_test.go | 87 ++++++++++++++++--- .../basecamp/sdk/conformance/ReplayRunner.kt | 13 ++- 3 files changed, 97 insertions(+), 13 deletions(-) diff --git a/conformance/runner/go/replay_runner.go b/conformance/runner/go/replay_runner.go index bb78301e..c6f85516 100644 --- a/conformance/runner/go/replay_runner.go +++ b/conformance/runner/go/replay_runner.go @@ -311,6 +311,16 @@ func (r *ReplayRunner) readSnapshot(testName string) (*wireSnapshot, error) { if err := json.Unmarshal(raw, &snap); err != nil { return nil, err } + // A snapshot like `{"operation":"GetProject"}` unmarshals cleanly with + // Pages == nil; decodeSnapshot would then loop zero times and Run() + // would record zero failures — a silent green-pass with no decode + // actually attempted. Require at least one page and a matching count. + if len(snap.Pages) == 0 { + return nil, fmt.Errorf("snapshot %s has no pages; expected at least one wire response", path) + } + if snap.PagesCount != len(snap.Pages) { + return nil, fmt.Errorf("snapshot %s pages_count (%d) does not match len(pages) (%d)", path, snap.PagesCount, len(snap.Pages)) + } return &snap, nil } diff --git a/conformance/runner/go/replay_runner_test.go b/conformance/runner/go/replay_runner_test.go index 82aa47c0..5fbc835b 100644 --- a/conformance/runner/go/replay_runner_test.go +++ b/conformance/runner/go/replay_runner_test.go @@ -1,16 +1,24 @@ -// Regression test for the empty-bodyText decode-masking bug. +// Regression tests for the wire-replay runner. Covers: // -// Pre-fix, BodyText was a `string` so encoding/json zero-fills a missing -// key with "". The decode path then conflated "" (missing) with "" (empty -// HTTP body) and re-serialized `body` instead, silently green-passing an -// actually-empty wire payload. Post-fix, BodyText is `*string` and -// resolveBodyText distinguishes nil (missing) from &"" (empty), letting -// the decoder fail honestly on an empty body. +// * The empty-bodyText decode-masking bug. Pre-fix, BodyText was a +// `string` so encoding/json zero-fills a missing key with "". The +// decode path then conflated "" (missing) with "" (empty HTTP body) +// and re-serialized `body` instead, silently green-passing an +// actually-empty wire payload. Post-fix, BodyText is `*string` and +// resolveBodyText distinguishes nil (missing) from &"" (empty), +// letting the decoder fail honestly on an empty body. +// +// * The empty-pages snapshot green-pass bug. Pre-fix, a snapshot like +// `{"operation":"GetProject"}` unmarshaled with Pages == nil; the +// per-page loop ran zero times and Run() recorded zero failures — +// a silent success without any decode attempted. Post-fix, +// readSnapshot rejects empty pages and pages_count mismatches. package main import ( - "encoding/json" + "os" + "path/filepath" "testing" ) @@ -55,8 +63,67 @@ func TestDecoder_ErrorsOnEmptyBodyText(t *testing.T) { } // Sanity: a syntactically valid empty object should still decode cleanly. if err := dec(`{}`); err != nil { - var se *json.SyntaxError - _ = se t.Fatalf("decoder should accept {}; got %v", err) } } + +// readSnapshotFixture writes a wire snapshot and a minimal openapi/fixture +// so that readSnapshot has a runner to call against. +func readSnapshotFixture(t *testing.T, testName, snapshotBody string) *ReplayRunner { + t.Helper() + dir := t.TempDir() + openapi := filepath.Join(dir, "openapi.json") + if err := os.WriteFile(openapi, []byte("{}"), 0o644); err != nil { + t.Fatal(err) + } + fixture := filepath.Join(dir, "fixture.json") + if err := os.WriteFile(fixture, []byte("[]"), 0o644); err != nil { + t.Fatal(err) + } + wireDir := filepath.Join(dir, "replay", "bc4", "wire") + if err := os.MkdirAll(wireDir, 0o755); err != nil { + t.Fatal(err) + } + snapPath := filepath.Join(wireDir, safeName(testName)+".json") + if err := os.WriteFile(snapPath, []byte(snapshotBody), 0o644); err != nil { + t.Fatal(err) + } + r, err := NewReplayRunner(filepath.Join(dir, "replay"), "bc4", fixture, openapi) + if err != nil { + t.Fatal(err) + } + return r +} + +func TestReadSnapshot_RejectsMissingPages(t *testing.T) { + // Pre-fix: `{"operation":"GetProject"}` unmarshaled with Pages == nil + // and the per-page loop ran zero times — silent green-pass with no + // decode attempted. Post-fix: readSnapshot returns an error. + r := readSnapshotFixture(t, "Test", `{"operation":"GetProject"}`) + if _, err := r.readSnapshot("Test"); err == nil { + t.Fatal("readSnapshot should error on missing pages; got nil") + } +} + +func TestReadSnapshot_RejectsEmptyPages(t *testing.T) { + r := readSnapshotFixture(t, "Test", `{"operation":"GetProject","pages":[],"pages_count":0}`) + if _, err := r.readSnapshot("Test"); err == nil { + t.Fatal("readSnapshot should error on empty pages; got nil") + } +} + +func TestReadSnapshot_RejectsMismatchedPagesCount(t *testing.T) { + r := readSnapshotFixture(t, "Test", + `{"operation":"GetProject","pages":[{"status":200,"bodyText":"{}"}],"pages_count":2}`) + if _, err := r.readSnapshot("Test"); err == nil { + t.Fatal("readSnapshot should error on mismatched pages_count; got nil") + } +} + +func TestReadSnapshot_AcceptsMatchingPagesCount(t *testing.T) { + r := readSnapshotFixture(t, "Test", + `{"operation":"GetProject","pages":[{"status":200,"bodyText":"{}"}],"pages_count":1}`) + if _, err := r.readSnapshot("Test"); err != nil { + t.Fatalf("readSnapshot should accept matching pages_count; got %v", err) + } +} diff --git a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt index 250f689d..82ee252e 100644 --- a/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt +++ b/kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/ReplayRunner.kt @@ -143,8 +143,11 @@ class ReplayRunner( // 3. Snapshot recognition — every captured snapshot's operation must // be in the shared fixture (catches TS-side dispatch drift). + // + // Sort by name so gate diagnostics are deterministic across + // filesystems (matches Python/Ruby/Go runners). if (wireDir.exists()) { - wireDir.listFiles { f -> f.extension == "json" }?.forEach { f -> + wireDir.listFiles { f -> f.extension == "json" }?.sortedBy { it.name }?.forEach { f -> val text = try { f.readText() } catch (e: IOException) { @@ -217,8 +220,12 @@ class ReplayRunner( val pages = snap["pages"]!!.jsonArray.map { p -> val page = p.jsonObject - // Prefer bodyText (raw); fall back to serializing `body` if absent. - val bodyText = page["bodyText"]?.jsonPrimitive?.contentOrNull + // Prefer bodyText (raw); fall back to serializing `body` if + // absent. Defensive cast mirrors the operation-field handling + // in coverageGate(): `.jsonPrimitive` throws if the element is + // JsonNull/JsonObject/JsonArray, which would crash the runner + // before it can record `decode_error`. + val bodyText = (page["bodyText"] as? JsonPrimitive)?.contentOrNull ?: page["body"]?.let { parserJson.encodeToString(JsonElement.serializer(), it) } ?: "" From bb855314dd25feeb5caef2cd6fe95fceecbbd7f9 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Wed, 13 May 2026 21:14:13 -0700 Subject: [PATCH 19/19] conformance(ts): keep persisted operation authoritative in snapshot spread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- conformance/runner/typescript/live-runner.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/conformance/runner/typescript/live-runner.test.ts b/conformance/runner/typescript/live-runner.test.ts index 4ce5e857..38544da7 100644 --- a/conformance/runner/typescript/live-runner.test.ts +++ b/conformance/runner/typescript/live-runner.test.ts @@ -109,8 +109,10 @@ function persistSnapshot(testName: string, operation: string, snapshot: WireSnap const safeName = testName.replace(/[^a-z0-9_-]+/gi, "_"); const file = path.join(wireDir, `${safeName}.json`); // Top-level `operation` lets replay runners (PR 3) dispatch without - // re-parsing the live-my-surface fixture. - const payload: PersistedWireSnapshot = { operation, ...snapshot }; + // re-parsing the live-my-surface fixture. Spread `snapshot` first so + // `operation` stays authoritative even if WireSnapshot ever grows a + // colliding key. + const payload: PersistedWireSnapshot = { ...snapshot, operation }; fs.writeFileSync(file, JSON.stringify(payload, null, 2)); }