Refuse a malformed GET field instead of writing it back (#576)

[jeremy]

Closes #576.

The shipped Todos and Cards merge-safe composites in Python, Ruby and TypeScript GET a record, read each writable field, and PUT the full representation back. Every value read is therefore a value written — on a call that never mentioned the field — and none of the three validated what they read.

Two failure modes, the same defect wearing different clothes. Erasure: a falsey non-string coalesced away, wiping the field. Corruption: a non-string forwarded verbatim, writing a number, boolean, array or object where a string belongs. Testing only for the first is what produced #576's original wrong verdict on TypeScript.

Red proof — what main actually puts on the wire

Probed against the unfixed composites, one call each: update(content: "New title") and update(title: "Renamed"). Nothing in these calls mentions the corrupted field.

Python Todos (update(content=…) with a malformed GET description)

  GET description=False        -> PUT description=''
  GET description=0            -> PUT description=''
  GET description=[]           -> PUT description=''
  GET description={}           -> PUT description=''
  GET description=42           -> PUT description=42
  GET description=True         -> PUT description=True
  GET description=['x']        -> PUT description=['x']
  GET description={'a': 1}     -> PUT description={'a': 1}

Python Todos — the ID lists, resent in full as the complete assignee set:

  GET assignees[0].id='100'    -> PUT assignee_ids=['100']
  GET assignees[0].id=10.5     -> PUT assignee_ids=[10.5]
  GET assignees[0].id=True     -> PUT assignee_ids=[True]
  GET assignees[0].id=None     -> PUT assignee_ids=[None]

Python Cards (update(title=…) with a malformed GET due_on)

  GET due_on=False        -> PUT due_on='<omitted -> BC3 ERASES the due date>'
  GET due_on=0            -> PUT due_on='<omitted -> BC3 ERASES the due date>'
  GET due_on=[]           -> PUT due_on='<omitted -> BC3 ERASES the due date>'
  GET due_on={}           -> PUT due_on='<omitted -> BC3 ERASES the due date>'
  GET due_on=42           -> PUT due_on=42
  GET due_on=True         -> PUT due_on=True
  GET due_on=['x']        -> PUT due_on=['x']
  GET due_on={'a': 1}     -> PUT due_on={'a': 1}

Ruby Todos / Ruby Cards

  GET description=false        -> PUT description=""
  GET description=0            -> PUT description=0
  GET description=[]           -> PUT description=[]
  GET description={}           -> PUT description={}
  GET description=42           -> PUT description=42
  GET description=true         -> PUT description=true
  GET description=["x"]        -> PUT description=["x"]
  GET description={"a" => 1}   -> PUT description={"a" => 1}

  GET due_on=false        -> PUT due_on=false
  GET due_on=0            -> PUT due_on=0
  GET due_on=[]           -> PUT due_on=[]
  GET due_on={}           -> PUT due_on={}
  GET due_on=42           -> PUT due_on=42
  GET due_on=true         -> PUT due_on=true
  GET due_on=["x"]        -> PUT due_on=["x"]
  GET due_on={"a" => 1}   -> PUT due_on={"a" => 1}

TypeScript Todos / TypeScript Cards

  GET description=false     -> PUT description=false
  GET description=0         -> PUT description=0
  GET description=[]        -> PUT description=[]
  GET description={}        -> PUT description={}
  GET description=42        -> PUT description=42
  GET description=true      -> PUT description=true
  GET description=["x"]     -> PUT description=["x"]
  GET description={"a":1}   -> PUT description={"a":1}

  GET due_on=false     -> PUT sent with due_on OMITTED -> BC3 ERASES the due date
  GET due_on=0         -> PUT sent with due_on OMITTED -> BC3 ERASES the due date
  GET due_on=[]        -> BasecampError: Due on must be in YYYY-MM-DD format
  GET due_on={}        -> BasecampError: Due on must be in YYYY-MM-DD format
  GET due_on=42        -> BasecampError: Due on must be in YYYY-MM-DD format
  ...

One correction to the issue's TypeScript Cards table: the truthy shapes do not reach the wire on main — the generated updateVerbatim's YYYY-MM-DD check intercepts them first (misclassified as a caller validation error, but no PUT). The falsey shapes are the ones that do real damage, and they are the worst case in the issue: if (current.due_on) drops them, and an omitted due_on is precisely how BC3 erases a card's due date.

Red proof — the tests

Every new test was run against the unfixed composites (git show HEAD:<path> swap, no stash) before the fix landed:

suite	failures against unfixed code	after
python/tests/services/{test_todos,test_cards}.py	85 failed, 41 passed, 4 skipped	126 passed, 4 skipped
ruby full suite (make rb-test)	1171 runs, 2643 assertions, 81 failures — exit 2	1171 runs, 0 failures
typescript/tests/services/{todos,cards}.test.ts	78 failed, 43 passed	121 passed
conformance kill cases (python / ruby / typescript)	5 / 5 / 3 failed	all pass

Exit codes are the ones those exact commands produce: a failing make <target> exits 2 (make's own code for a failed recipe) while the bare runner underneath exits 1 — make: *** [rb-test] Error 1 on the line above REAL_EXIT=2. An earlier revision of this write-up reported 1 for the Ruby row; the counts were right, the number was not.

Representative messages, all from the unfixed code:

Failed: DID NOT RAISE <class 'basecamp.errors.ApiError'>            (x69, Python)
TodosServiceTest#test_update_refuses_a_non_object_response_body_42:
  [Basecamp::ApiError] exception expected, not
  Class: <TypeError>  Message: <"no implicit conversion of String into Integer">
TodosServiceTest#test_update_refuses_a_non_array_completion_subscribers_0:
  Class: <NoMethodError>  Message: <"undefined method 'map' for an instance of Integer">
AssertionError: expected TypeError: Cannot read properties of null… to be an instance of BasecampError
FAIL: update-kill: an array description is refused before the full-replace PUT
      Expected the call to fail, but it succeeded; Expected 1 requests, got 2

The fix

Absent or explicit null is genuinely empty; an actual string passes verbatim; anything else raises before the PUT, naming the field. The ID-list fields get the analogous check — an array, of objects, each carrying an integer id. One level up, the response body itself must be an object: on main a scalar or null body produced a raw TypeError/AttributeError rather than the documented statusless api_error.

Classification is api_error, not usage: the value arrived in a successful API response, so nothing the caller passed is at fault. Non-retryable — re-requesting cannot repair a malformed body.

The rule underneath, from #576: a composite is safe exactly when a decoder rejects a wrong-typed field at runtime — not when a type merely claims one. Go (json.Unmarshal) and Swift (Codable) genuinely refuse. TypeScript's schema.d.ts is erased at build time and the generated Python and Ruby services return an untyped dict/Hash, so those three do it by hand — once per language in a shared helper (_merge_safe.py, merge_safe.rb, merge-safe.ts) rather than six copies.

Those three helper files are no longer part of this diff. #601 (the Documents triad) landed byte-identical copies of all three while this PR was in review, so rebasing onto main collapsed them to a zero diff — confirmed by matching SHA-256 on each before the rebase, and by their absence from git diff --name-only origin/main...HEAD after it. They are main's files now, unchanged. What remains here is the Todos and Cards consumers plus the conformance work.

Todolists keeps its own copy from #574. It is deliberately untouched here: #544 owns those files, and #578's generated validating layer is the intended end state for all of them.

Shared kill fixtures, and the fourth language they found

Per #576, kill coverage belongs in the shared conformance fixtures, not per language — the defect survived five consecutive review passes precisely because each pass fixed one instance. Five cases across todos_write.json and cards_write.json assert errorRaised + requestCount: 1: the guard must fire before the PUT, because a guard that fires after has already lost the field. A second mock response is queued in each so a runner cannot pass by exhausting the queue instead of refusing the field.

errorRaised is a new assertion type, the code-agnostic inverse of noError. The six SDKs refuse the same body by two different mechanisms — a hand-written guard versus a model decoder — and those share no canonical error code, so errorType would make the fixture unwritable. Declaring it also tells the Kotlin and Swift runners that a decoder rejection is the point of the case rather than an under-specified fixture body (their #555 policy otherwise fails it).

Writing the fixture immediately earned its keep: it found a fourth affected language. #576 lists Kotlin Todos and Cards as structurally safe on the grounds that "kotlinx.serialization rejects a wrong-typed field at decode". That is only true for structural mismatches. Kotlin's client-wide Json { isLenient = true; coerceInputValues = true } (BasecampClient.kt:156) coerces a JSON scalar into a String field, so the composite decodes it and writes it back. Proven on the wire by temporarily flipping the kill fixtures to requestCount: 2 with a requestBody pin, all four PASSING:

PASS: update-kill … (assertions: requestCount 2, requestBody description == "42")
PASS: update-kill … (assertions: requestCount 2, requestBody description == "false")
PASS: update-kill … (assertions: requestCount 2, requestBody due_on == "false")
PASS: update-kill … (assertions: requestCount 2, requestBody due_on == "42")

i.e. GET "description": 42 → PUT "description": "42", and GET "due_on": false → PUT "due_on": "false".

This cannot be fixed by this PR's pattern: the coercion happens at decode, so by the time the composite runs it only ever sees a String. The shipped fixtures therefore use array/object shapes, which kotlinx.serialization does reject, and the scalar hole is filed separately as a follow-up. The fixture descriptions say so, in the fixtures, so the limit of what they prove travels with them.

The fifth case, and the TypeScript Cards hole it closes

The first two Cards kill cases (["x"], {}) do not discriminate in TypeScript. The generated updateVerbatim guards the date with /^\d{4}-\d{2}-\d{2}$/.test(req.dueOn), and RegExp.test coerces its argument to a string first: String(["x"]) is "x" and String({}) is "[object Object]", so both are rejected before the PUT with or without this PR's guard. Measured, not reasoned — against the unfixed composites TypeScript conformance failed 2 cases, not 4, and both were Todos. TypeScript Cards had no regression protection from the shared fixture at all.

A fifth case fixes that. ["2024-02-01"] is the shape the format check is blind to: String(["2024-02-01"]) is exactly "2024-02-01", so the regex matches and the array rides through untouched. With the case temporarily flipped to requestCount: 2 + requestMethod PUT + a requestBody pin — the same probe used for Kotlin above — against the unfixed TypeScript composite:

 ✓ runner.test.ts > conformance/cards_write.json > update-kill: a date-shaped array due_on is refused where the format check is blind 25ms
 Test Files  1 passed | 6 skipped (7)
      Tests  1 passed | 182 skipped (183)
REAL_EXIT=0

That is main issuing a real second request whose body carries "due_on": ["2024-02-01"] — a JSON array written onto a field the API stores as a date, on a call that only renamed the card.

Restored to its shipped errorRaised + requestCount: 1 form, it is red against the unfixed composites in all three hand-written-guard languages. TypeScript (make conformance-typescript, REAL_EXIT=2):

 FAIL  runner.test.ts > conformance/cards_write.json > update-kill: a date-shaped array due_on is refused where the format check is blind
AssertionError: [update-kill: a date-shaped array due_on is refused where the format check is blind] Expected the call to fail, but it succeeded: expected 'Expected the call to fail, but it suc…' to be undefined

Python and Ruby (make conformance-python / make conformance-ruby, both REAL_EXIT=2) print the same two lines, and name the PUT that the TypeScript runner stops short of reporting because expect throws on the first failing assertion:

  FAIL: update-kill: a date-shaped array due_on is refused where the format check is blind
        Expected the call to fail, but it succeeded; Expected 1 requests, got 2

So the shared fixture is now non-vacuous in every language it can be: TypeScript fails 3 cases against the unfixed composites rather than 2, Python and Ruby 5 rather than 4.

It also keeps the property that makes a shared fixture worth writing. Go, Kotlin and Swift decode due_on into a String, and json.Unmarshal, kotlinx.serialization and Codable reject a JSON array there structurally, exactly as they reject ["x"]. The date-shaped element is invisible to a decoder — it is a string inside an array, and the array is the mismatch. A scalar still cannot be used, for the Kotlin coercion reason above.

The errorRaised handler is unit-tested in all six runners

errorRaised's failing branch is unreachable from conformance/tests/: every case declaring it is one the SDK does refuse, so a handler that accepted everything would report green in all six runners at once. That is precisely the #563 shape — a delayBetweenRequests check that passed vacuously because no fixture supplied a gap it could fail on.

So the predicate is split out per runner — error_raised.go, error_raised_failure in runner.py, ErrorRaised.check in runner.rb, error-raised.ts, ErrorRaised.kt, ErrorRaised.swift — and unit-tested on both directions, next to the existing delayGaps tests. Go additionally asserts the wiring, that checkAssertion routes the type to that branch: a typo'd case label would fall through to the default and assert nothing.

Non-vacuity proved by mutation rather than by inspection. Each handler was temporarily changed to return the pass value unconditionally, and every runner's unit test went red:

runner	command	under mutation
Go	go test ./...	--- FAIL: TestErrorRaisedFailure, --- FAIL: TestCheckAssertionRoutesErrorRaised — exit 1
Python	uv run python -m pytest -q test_error_raised.py	1 failed, 1 passed — exit 1
Ruby	bundle exec ruby error_raised_test.rb	2 runs, 2 assertions, 1 failures — exit 1
Kotlin	./gradlew --quiet :conformance:test	12 tests completed, 1 failed — exit 1
Swift	swift test	ErrorRaisedTests failed — exit 1
TypeScript	npx vitest run error-raised.test.ts	Tests 1 failed | 1 passed (2) — exit 1

Verbatim, for the three that had previously only been checked by inspection:

--- FAIL: TestErrorRaisedFailure (0.00s)
    error_raised_test.go:23: expected "Expected the call to fail, but it succeeded" for a successful dispatch, got ""
--- FAIL: TestCheckAssertionRoutesErrorRaised (0.00s)
    error_raised_test.go:40: expected checkAssertion to fail when the dispatch succeeded, got a pass

FAILED: com.basecamp.sdk.conformance.ErrorRaisedTest.a successful dispatch fails the assertion()
   org.opentest4j.AssertionFailedError: expected: <Expected the call to fail, but it succeeded> but was: <null>

ErrorRaisedTests.swift:23: error: -[ConformanceSupportTests.ErrorRaisedTests testSuccessfulDispatchFailsTheAssertion] : XCTAssertEqual failed: ("Optional("Expected the call to fail, but it succeeded")") is not equal to ("nil")

The message is pinned verbatim in all six, so a fixture debugged in one language does not read differently in another. Go, Python, Ruby, Kotlin and Swift run under make conformance-runner-tests (and in CI, which this PR extends to cover the two new Ruby and Python files); the TypeScript case runs under make conformance-typescript, where delay-gaps.test.ts already lives.

One behaviour change beyond pure containment

Ruby Cards.update, in the case where the server returns due_on: "".

• Before: get(card_id:)["due_on"] handed "" straight to update_verbatim, and compact_params is kwargs.compact, which strips only nil — so the PUT went out carrying due_on: "".
• After: writable_string passes the "" through (it is a String, so the guard does not fire), current_due_on then normalises "" to nil, and compact_params strips it — so the PUT omits due_on.

An omitted due_on is exactly how BC3 clears the date ({ due_on: nil }.merge(card_params)), so this changes what goes on the wire, not merely what gets validated. It is called out here rather than left inside the containment story. Three reasons it is nonetheless the right change:

1. It restores parity with Python, which has always omitted in this case: ... or None collapses "" to None and the generated _compact strips it. Ruby was the outlier, before and after.
2. due_on: "" is not a value BC3 accepts. The composite's own comment on the sibling branch already says sending "" "risks a date-format error", which is why an explicit caller-requested clear is encoded as an omission. The nil branch now agrees with the "" branch instead of contradicting it.
3. The record ends up in the same state. The server told us the card has no due date; omitting due_on leaves it with no due date. The bytes change; the card does not.

The trigger is narrow — BC3 returns null, not "", for a card with no due date — so this is an edge-shaped-response path, which is the path this PR is about.

Verification

Re-run in full after the rebase onto main at dc5f17ee6 — #592, #601, #590, #605 and three dependency bumps all landed under this branch, so every number below is measured at the current head, not carried forward. Every conformance total is unchanged by the dependency bumps. Every command was run with REAL_EXIT=$? written into a log and grepped back; a failing make <target> exits 2, the bare runner underneath exits 1.

command	result
make py-check	1046 passed, 4 skipped; mypy no issues found in 38 source files; ruff clean — exit 0
make rb-check	1244 runs, 29713 assertions, 0 failures; rubocop 150 files, no offenses — exit 0
make ts-check	79 files, 1282 tests passed; tsc clean — exit 0
make kt-check	BUILD SUCCESSFUL — exit 0. Locally this was a Gradle allTests UP-TO-DATE hit, so the load-bearing evidence is CI's Kotlin Tests job at this head, which builds cold. make conformance-kotlin did execute (155/0/1).
make swift-check	Executed 333 tests, with 0 failures (genuinely ran, not the macOS SKIP line) — exit 0
make conformance-go	154 passed, 0 failed, 2 skipped — exit 0
make conformance-python	156 passed, 0 failed, 0 skipped — exit 0
make conformance-ruby	145 passed, 0 failed, 11 skipped — exit 0
make conformance-typescript	189 passed, 2 skipped (7 files) — exit 0
make conformance-kotlin	155 passed, 0 failed, 1 skipped — exit 0
make conformance-swift	155 passed, 0 failed, 1 skipped — exit 0
make conformance-runner-tests	Go/Python/Ruby/Kotlin/Swift runner units, 0 failures; Swift's 41 XCTest cases include both ErrorRaisedTests — exit 0
make conformance-fixtures-check	metaschema ok; fixture validation ok; all 5 errorRaised fixtures have a body-pinning control sibling; gate self-test 25/25 — exit 0

The conformance totals are higher than the previous revision because the rebase brought in #601's Documents fixtures, not because of anything here.

All five kill cases were confirmed to actually run in every runner — a fixture whose operation has no dispatch case silently skips, which would look identical to a pass in the totals. Verbatim PASS: lines in Go, Python, Ruby, Kotlin and Swift, and a --reporter=verbose re-run for TypeScript, which does not name passing cases by default:

 ✓ runner.test.ts > conformance/cards_write.json > update-kill: an array due_on is refused before the replacement PUT 18ms
 ✓ runner.test.ts > conformance/cards_write.json > update-kill: an empty-object due_on is refused, not coerced or dropped 3ms
 ✓ runner.test.ts > conformance/cards_write.json > update-kill: a date-shaped array due_on is refused where the format check is blind 2ms
 ✓ runner.test.ts > conformance/todos_write.json > update-kill: an array description is refused before the full-replace PUT 1ms
 ✓ runner.test.ts > conformance/todos_write.json > update-kill: an empty-object description is refused, not coalesced to empty 1ms
      Tests  5 passed | 178 skipped (183)
REAL_EXIT=0

The Go runner-unit result above is from a -count=1 re-run, because go test had reported a cached ok after the mutation probe restored the file.

Review follow-ups

Codex P2, Assertions.swift — "mark an enforced HTTPS crash as a dispatch failure". Real, and fixed. The .enforced branch recorded caughtError but never set dispatchFailed, so an errorRaised fixture with an http:// configOverrides.baseUrl would have read a trapped child process as a call that succeeded. Fixed at both ends rather than one: Runner.swift now sets dispatchFailed = true in that branch (the child died as required, so the dispatch did fail — by a trap rather than a throw), and the assertion reads dispatchFailed || caughtError != nil, so the union holds by construction instead of by call-site discipline. Kotlin's Main.kt takes the same union for the same reason.

Codex P2, Runner.swift — "require the intended decoding error". Also real, also fixed, by a different route than proposed. Declaring errorRaised switches the #555 stop-on-mismatch policy off wholesale in the decoder-backed runners: Swift's DecodingError branch and Kotlin's MissingFieldException/SerializationException branches accept any decode failure. So if a model gains a required field, or an unrelated field in one of these 27-key bodies drifts, the decode fails for an unrelated reason while errorRaised + requestCount: 1 + requestMethod + requestPath all still hold — the case passes and has stopped testing the field it names. Nothing else covered it: conformance-fixtures-check validates fixture format, not that mock bodies still decode into the generated models.

The proposed fix (pin the expected field per decode path) is structured and cheap in Swift, but in Kotlin it means matching SerializationException's message prose for at path: $.due_on — the type-mismatch case has no structured field accessor — and a cross-language invariant encoded as a string match against one serializer's error text is a worse thing to own than the hole.

What the investigation turned up instead: the protection already exists structurally and only needed enforcing. Every kill body is a passing case's body with exactly one field perturbed, and that sibling does not declare errorRaised, so it keeps the full #555 policy and fails loudly on precisely that drift — Cards kill cases pair with update-preserves-due-on (differing only in due_on), Todos with update-merge (differing only in description). But nothing enforced the coupling: edit one body and not the other and it silently breaks, which is the real failure mode. So conformance/check_kill_case_controls.py now asserts the invariant — for every case declaring errorRaised, some case in the same file that does not declare it must have a mock body with the identical key set, differing in exactly one field — and that one differing field is the field under test. It runs inside make conformance-fixtures-check, which CI already invokes, so no new job. Enforcing the claim rather than asserting it in a comment is #576's own lesson applied to #576's fixtures.

Non-vacuous, not assumed. Perturbing a kill body in a second field fails it:

FAIL: errorRaised fixtures without a control sibling:

  - cards_write.json: 'update-kill: a date-shaped array due_on is refused where the format check is blind' declares errorRaised but no non-errorRaised case in this file has a mock body with the same key set differing in exactly one field.
      Without one, a decode failure caused by unrelated model drift would satisfy this case and it would stop testing the field it names.
      Add (or repair) the control case so the two bodies differ only in the field under test.
REAL_EXIT=1

A follow-up Codex P2 then found the gate had the same vacuity one level up: it matched a kill body against any queued response of any control. Each kill case queues two object-shaped responses — response 0 is the GET whose decoder rejection is under test, response 1 is a decoy queued so a runner cannot pass by exhausting the queue — and the decoy is never consumed. Reachable, not theoretical; the case that separates the two versions drifts the consumed body by a second field while making the decoy differ from its control by exactly one:

consumed[0]: 2 fields differ from control (due_on + title)
decoy[1]   : 1 field differs from control (due_on)

########## OLD gate (matches ANY queued response) ##########
  ok  'update-kill: a date-shaped array due_on is refused where the for'
REAL_EXIT=0

########## NEW gate (consumed response only) ##########
FAIL: errorRaised fixtures without a control sibling:
  - cards_write.json: 'update-kill: a date-shaped array due_on is refused where the format check is blind' declares errorRaised but no non-errorRaised case for operation 'UpdateCard' in this file has a FIRST mock response body with the same key set differing in exactly one field.
REAL_EXIT=1

The gate is now restricted on both sides — the first response only, and a control exercising the same operation — and on the shipped fixtures it names each pairing, at REAL_EXIT=0:

  ok  'update-kill: a date-shaped array due_on is refused where the for'
      field 'due_on' controlled by 'update-preserves-due-on: composite refetches and resends'
  ok  'update-kill: an array description is refused before the full-rep'
      field 'description' controlled by 'update-merge: content-only update preserves every unset '

All errorRaised fixtures have a body-pinning control sibling.

Codex P2, conformance/schema.json — "move $comment out of the properties map". Correct, and the interesting half is why it shipped. The errorRaised annotation sat inside properties.assertions.items.properties, declaring a property literally named $comment whose schema was a string; Draft 2020-12 requires every value under properties to be a schema object or boolean, so schema.json was not itself a valid schema — and tests.schema.json $refs it, so a validator that meta-validates would reject the conformance schema before reading a single fixture.

The gap underneath is that conformance-fixtures-check validated fixtures against the schema and never validated the schema itself, so an invalid schema sailed through a green gate. Same shape as everything else here, so it got the same treatment: the target now runs --check-metaschema over schema.json and tests.schema.json first. Red proof through the make target rather than the bare validator — putting the annotation back inside properties gives REAL_EXIT=2 and '...' is not of type 'object', 'boolean'.

evaluateAssertions(dispatchFailed:) lost its default. It was dispatchFailed: Bool = false. There is exactly one call site and it does pass the argument, so nothing was broken — but a defaulted parameter on a new assertion path is an invitation, and this default fails closed: a future call site that omitted it would report "the call succeeded" on a call that did not, turning every errorRaised fixture red for a reason nowhere near the actual bug. The parameter is now required, so omitting it is a compile error.

Codex P2, check_kill_case_controls.py — "reject 204 responses before accepting kill cases". Correct, and it is this PR's own false-green shape one layer further down. Both sides of the gate accepted any 2xx, so a kill case answering 204 passed: TypeScript returns undefined for a 204 (base.ts), Kotlin returns Unit without calling parse (BaseService.kt), Go rewrites the body to JSON null (client.go). The malformed field is never decoded; the composite fails because the record came back absent; and errorRaised + requestCount: 1 + requestMethod + requestPath are all satisfied by that unrelated failure while the control, still a 200, stays green. Strictly worse than the 500 case above it — a 500 at least fails the call, so errorRaised is satisfied by something about the response.

Reachable, not merely structural. The shipped Todos kill case and its shipped control, with only the kill's first response changed 200 -> 204, against the gate at c348acf7b:

  ok  'update-kill (204): the malformed description never reaches a dec'
      field 'description' controlled by 'update-merge: content-only update preserves every unset '

All errorRaised fixtures have a body-pinning control sibling.
REAL_EXIT=0

Fixed as an allowlist, {200, 201}, rather than a 204 exclusion — the review offered both, and excluding 204 by name only closes the status somebody happened to think of. Two constraints meet there: Go's success arm is exactly {200, 201, 204} (case http.StatusOK, http.StatusCreated, http.StatusNoContent), so 202, 203, 205 and 206 are never decoded there at all; and 204/205 forbid a body outright. A 202 is now rejected too, with a message that names why. not_a_success becomes not_decoded, because a 204 does not fail the call — it bypasses the decode, and the old name asserted the wrong property.

The gate itself is now self-tested

This was the fifth defect found in a gate every one of whose rejections was correct by inspection alone — the standard that let #576 through five review passes. So it gets #576's own treatment. conformance/test_check_kill_case_controls.py crafts 25 inputs — 21 claimed rejections (both sides' 204 / 205 / undecoded-2xx / HTTP-error / networkError, the same-operation, key-set and exactly-one-field rules, the first-response-only restriction on both sides, and the entry point's own failure modes) plus 4 positive controls, the real fixture set among them — and drives each through the real entry point, via a new optional FIXTURE_DIR argument added for exactly that purpose.

Non-vacuous by measurement rather than by claim. Reverting only the two new status branches, leaving the renames, the argument and every other rejection intact, turns exactly four cases red and nothing else:

check_kill_case_controls self-test: 4 case(s) failed.

kill case answers 204: expected FAILURE, gate exited 0:
  ok  'update-kill: an array description is refused before the full-rep'
      field 'description' controlled by 'update-merge: content-only update preserves every unset '

All errorRaised fixtures have a body-pinning control sibling.
[...three further cases: 205, an undecoded 2xx, and the control-side 204...]

It runs inside make conformance-fixtures-check, which CI already invokes, so no new job.

The doc-constants gate the rebase brought into range

#590 landed make doc-constants-check on main after this branch was cut, and it holds SPEC.md §19's marked table against the conformance/schema.json assertion enum: "defines 22 assertion types, the table documents 21; missing: errorRaised". This branch is what added the 22nd, so §19 now carries a row for it — what the type is for, and what declaring it costs (declaring it switches the stop-on-mismatch policy off for that case, which is why every fixture declaring it needs a control sibling).

That one row is the entire SPEC.md diff of this PR, and spec/doc-constants.json is untouched (byte-identical to main).

The same red run also flagged SPEC.md §Documents for restating the current pin, which was #601's line, not this branch's. An earlier revision of this branch carried a fix for it — at the time main was red on it and nothing was open against it. #605 has since fixed it on main, with a count-2 grant that keeps the prose as written, so that commit is dropped rather than rebased: it would have contradicted a merged decision inside an unrelated PR. If the by-reference rewrite AGENTS.md prefers is still worth making, it belongs in its own PR.

Deliberately out of scope

• The caller-side mirror. edit hands the caller a mutable view and a closure assigning 42 walks into the same PUT. That is caller misuse of a value the caller chose, not a value silently substituted for one they asked to preserve, and Python, Ruby and TypeScript merge-safe composites forward a non-string GET field into the full-replace PUT #576 is scoped to the GET-field path.
• Kotlin's lenient decoder. Same defect class, different layer; the only fixes are strict client-wide decoding (large blast radius) or per-field serializers. Filed separately.
• TypeScript, Python and Ruby have no runtime response validation on any route #578. No guard here is written in anticipation of it, and none is deleted for it. When the generated validating layer lands, these guards go and the shared kill fixtures stay — they are what proves the replacement actually replaced something.

---

Summary by cubic

Refuse malformed GET fields in merge‑safe Todos and Cards (Python, Ruby, TypeScript) and fail before any PUT; strengthen cross‑SDK checks with errorRaised fixtures, gates, and runner unit tests.

• Bug Fixes

  • Require the GET response body to be an object; raise api_error if not.
  • Writable strings (content, description, due_on, etc.): allow absent/null or a string; otherwise raise before PUT.
  • ID lists: require an array of objects with integer id; otherwise raise before PUT.
  • Cards.update: validate due_on; reject non‑strings and, in Ruby, omit "" instead of sending it.

• Refactors

  • Implement errorRaised in Go, Python, Ruby, TypeScript, Kotlin, and Swift with unit tests; add CI hooks.
  • Add conformance/check_kill_case_controls.py gate: same operation, first response only, identical key set with exactly one differing field; require decoded 2xx on both kill/control; forbid 204/205 and networkError; add a self‑test suite.
  • Run metaschema checks first and move $comment out of assertion properties; validate fixtures after.
  • Add a TypeScript‑discriminating Cards kill case (["2024‑02‑01"]).
  • Kotlin/Swift runners: treat decoder rejections and HTTPS traps as dispatch failures; require explicit dispatchFailed (no default).
  • Document errorRaised in SPEC §19 and update Makefile/workflows to run the new checks.

^{Written for commit f3377f8. Summary will update on new commits.}

[jeremy]

Kotlin follow-up from the shared kill fixtures filed as #598 — Json { isLenient = true } coerces a wrong-typed scalar into a String, so GET "description": 42 becomes PUT "description": "42". Not fixable by this PR's per-composite pattern (the coercion happens at decode; the composite only ever sees a String), which is why the fixtures here use array/object shapes that kotlinx.serialization does reject.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d60369fe5c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

This PR closes #576 by hardening the merge-safe Todos.update/edit and Cards.update composites in the three SDKs that lack a runtime decoder (Python, Ruby, TypeScript). These composites GET a record, read each writable field, and PUT the full representation back — so any value read is a value written, on a call that may never have mentioned the field. Previously a malformed field read off the wire was either coalesced away (erasure) or forwarded verbatim (corruption). The fix validates each fetched field and raises a statusless, non-retryable api_error before the PUT, naming the offending field. It also adds a code-agnostic errorRaised conformance assertion and shared "kill" fixtures so the defect class is caught once, across all six runners, rather than per-language.

Tip

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

Changes:

• New per-language shared guard helpers (_merge_safe.py, merge_safe.rb, merge-safe.ts) providing require*/writable_string/writable_id_list that reject non-object bodies, non-string writable fields, and malformed ID lists.
• Todos/Cards composites in Python, Ruby, and TypeScript rewired to read fields through the guards instead of or "" / || "" / ?? "" / if (current.due_on).
• New code-agnostic errorRaised assertion added to conformance/schema.json and all six conformance runners, plus four shared kill fixtures across todos_write.json/cards_write.json.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
python/src/basecamp/services/_merge_safe.py	New shared guard helpers raising ApiError before the PUT.
python/src/basecamp/services/todos.py	Todos composite reads fields via guards.
python/src/basecamp/services/cards.py	_resolve_due_on validates the fetched due_on.
ruby/lib/basecamp/services/merge_safe.rb	New shared guard module (Zeitwerk-autoloaded).
ruby/lib/basecamp/services/todos_extensions.rb	Todos composite uses MergeSafe.
ruby/lib/basecamp/services/cards_extensions.rb	Cards current_due_on validates the fetched date.
typescript/src/services/merge-safe.ts	New shared guard helpers returning BasecampError (api_error).
typescript/src/services/todos-extensions.ts	Todos composite uses guards.
typescript/src/services/cards-extensions.ts	Cards composite validates fetched due_on.
conformance/schema.json	Adds errorRaised to the assertion enum.
conformance/runner/{go,python,ruby,swift,typescript}, kotlin/conformance/.../Main.kt	Implement errorRaised; Kotlin/Swift treat decoder rejection as the point of the case.
conformance/tests/{todos_write,cards_write}.json	Four kill fixtures asserting refusal before the PUT (requestCount: 1).
{python,ruby,typescript} test files	Comprehensive erasure + corruption coverage for all writable fields, ID lists, and non-object bodies.

---

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

[github-actions]

Sensitive Change Detection (shadow mode)

This PR modifies control-plane files:

• .github/workflows/test.yml

Shadow mode — this check is informational only. When activated, changes to these paths will require approval from a maintainer.

[jeremy]

Review round pushed as a9c1814ca. Four things, three of them about the new errorRaised assertion proving less than it claimed.

1. The shared kill fixture had a hole in TypeScript. The two Cards cases (["x"], {}) pass in TypeScript with or without this PR's guard: the generated updateVerbatim checks /^\d{4}-\d{2}-\d{2}$/.test(req.dueOn) and RegExp.test stringifies first, so both were already rejected before the PUT. Against the unfixed composites TypeScript conformance failed 2 cases, not 4 — TypeScript Cards had no regression protection from the fixture at all. A fifth case uses ["2024-02-01"], which String() renders as exactly "2024-02-01": the format check waves it through and only the guard stops it. Proven on the wire — flipped to requestCount: 2 + a requestBody pin, unfixed TypeScript passes it, i.e. it really does PUT "due_on": ["2024-02-01"]. It still discriminates in Python and Ruby, and Go/Kotlin/Swift reject it structurally like any JSON array in a String field, so the fixture stays genuinely shared. Kill-case failures against unfixed code are now 5 / 5 / 3 (py / rb / ts), up from 4 / 4 / 2.

2. The errorRaised handler had no unit test in any runner. Its failing branch is unreachable from conformance/tests/, so a handler that accepted everything would look green in all six runners at once — the #563 shape exactly. The predicate is now split out per runner and tested on both directions, message pinned verbatim in all six; Go also asserts the wiring, since a typo'd case label would fall through to the default. Non-vacuity proved by mutation rather than inspection: each handler was temporarily made to accept everything and every runner's test went red (all exit 1), including the three that had previously only been checked by eye — Go, Kotlin and Swift.

3. Disclosed a behaviour change beyond pure containment. Ruby Cards.update with a server-returned due_on: "": main forwarded "" on the wire, this PR normalizes to nil and compact_params strips it, and an omitted due_on is how BC3 clears the date. It restores parity with Python (which always omitted here) and agrees with the sibling branch's own "sending \"\" risks a date-format error" reasoning, and the record ends in the same state — but it is a wire change, so it now has its own section in the description rather than being folded into the containment story.

4. Corrected the write-up. The Ruby red proof's exit code is 2, not 1 — make rb-test exits 2 (make's recipe-failure code) while the bare rake underneath exits 1; make: *** [rb-test] Error 1 sits directly above REAL_EXIT=2. Counts were right (1171 runs, 2643 assertions, 81 failures) and are unchanged. Also tightened evaluateAssertions(dispatchFailed:) by removing its default (see the resolved thread).

Full suite re-run at this head, every command with REAL_EXIT written into a log and grepped back: 13/13 targets exit 0. Conformance counts each rise by one for the fifth case (TypeScript by three, adding the two new runner units).

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[jeremy]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a9c1814cae

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ec7cec5c0e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb8cd466d9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[jeremy]

Ready for merge at f3377f81b. Rebased twice under review — onto c441c235b, then onto dc5f17ee6 — so this is a fresh base, not a carried-forward one.

Review state. All 8 threads resolved, each with a posted, argued reply rather than a silent resolve. Every review body was audited for a <details>Comments suppressed due to low confidence</details> block across all 28 reviews: none. The last two Codex findings are fixed in code, not declined:

• P2, "reject 204 responses before accepting kill cases" — cb8cd466d. Fixed as a {200, 201} allowlist rather than a 204 exclusion, applied to the control response too, plus a 25-case self-test for the gate that is measured non-vacuous (reverting only the two new status branches turns exactly four cases red).
• P1, "document errorRaised in the assertion table" — f3377f81b. That row is this PR's entire SPEC.md diff; spec/doc-constants.json is byte-identical to main.

Copilot could not review this revision. It has returned "Copilot encountered an error and was unable to review this pull request" four times, including once after an explicit re-request. Its last successful pass was at c348acf7b — "reviewed 38 out of 38 changed files ... generated no new comments" — which predates the 204 fix and the SPEC.md row. Per the usual convention this proceeds on Codex's review plus green CI, with the gap recorded here rather than left implicit.

CI is green at this head: 42 success, 2 neutral (CodeQL, cubic), 2 skipped, 0 failures.

Independently re-verified locally at this base, each with REAL_EXIT written to a log and grepped back, all exit 0 — py-check, rb-check, ts-check, kt-check, swift-check, the six conformance-* suites, conformance-runner-tests, and conformance-fixtures-check. Two caveats stated rather than buried: local kt-check was a Gradle allTests UP-TO-DATE hit, so CI's cold Kotlin Tests job is the load-bearing evidence there (local conformance-kotlin did execute, 155/0/1); and swift-check genuinely ran — Executed 333 tests, with 0 failures — rather than printing its macOS SKIP line.

One item is a behaviour change beyond pure containment and is called out in the description rather than folded into the containment story: Ruby Cards.update now normalises a server-returned due_on: "" to nil, which compact_params strips, so the PUT omits due_on — and omission is how BC3 clears the date. It restores parity with Python, which has always omitted here.

[jeremy]

Rebased onto main (dc5f17ee6) — merge-ready. Head f3377f81b.

#601 landing changed what this PR needs to carry, in two ways:

1. The three shared guard files are gone from the diff. _merge_safe.py, merge_safe.rb and merge-safe.ts shipped byte-identical in Stop the sparse document PUT from erasing the content (#543) #601, so the rebase resolves them to no-ops. Nothing here re-adds or re-edits them; the Todos/Cards composites simply import what is now already on main.
2. The spec/doc-constants.json conflict is gone rather than merged. It was an add/add on the same SPEC.md key — this branch wanted count: 1, main (via Unbreak doc-constants-check: grant SPEC.md's two as-of pin citations #605) had count: 2. The conflict existed because an earlier revision of this branch also rewrote Stop the sparse document PUT from erasing the content (#543) #601's §Documents pin sentence, which would have dropped one of the two citations Unbreak doc-constants-check: grant SPEC.md's two as-of pin citations #605 grants. That rewrite was never this PR's business — it was Stop the sparse document PUT from erasing the content (#543) #601's line, and Unbreak doc-constants-check: grant SPEC.md's two as-of pin citations #605 has since granted it on main — so it is dropped, and this branch no longer touches spec/doc-constants.json at all. main's count: 2 stays correct because the line it counts is untouched.

The entire remaining SPEC.md footprint is one line: the errorRaised row in §19's gated assertion table.

$ git diff origin/main --stat -- SPEC.md spec/doc-constants.json
 SPEC.md | 1 +
 1 file changed, 1 insertion(+)

Gates, run with the SHA pinned before and after so the result provably applies to that exact tree — a precaution worth taking here because several agents are working this repo concurrently and a symbolic HEAD moved under an earlier run:

PRE_SHA=f3377f81bfb7b3e6dd7fdc2d47eb09563a8fc598
==> All checks passed
REAL_EXIT=0
POST_SHA=f3377f81bfb7b3e6dd7fdc2d47eb09563a8fc598

$ ruby scripts/sync-doc-constants.rb --check
Doc constants match their sources (5 marked spans across 3 files).
  api-version      2026-08-02
  bc3-pin          2c0dafba (2026-08-02)
  assertion-types  22
REAL_EXIT=0

Review state: 8 threads, 0 unresolved. The last one — the errorRaised §19 P1 — was accurate when filed against cb8cd466d and was fixed by the very next commit; answered with the before/after evidence rather than declined, and no duplicate row added. No <details>Comments suppressed due to low confidence</details> block in any review body.

One red check, and it is not a finding. copilot-pull-request-reviewer is failing, but Copilot has a completed clean pass on this content — "Copilot reviewed 38 out of 38 changed files in this pull request and generated no new comments" (2026-08-03T10:27:28Z). Every other Copilot entry on this PR is the infrastructure error "Copilot encountered an error and was unable to review this pull request", eleven times today, and the same bot errored the same way on #601. It cannot be re-requested through the API (422 Reviews may only be requested from collaborators), and main has no branch protection, so it is not a required check. Flagging rather than silently ignoring it.

Checks otherwise: 42 success, 0 pending, 0 other failures.