Relax Tool.name/Tool.enabled and absorb the seven keys bc3 actually emits
Closes #650.
Ships-broken in Swift. Tool.name and Tool.enabled were @required, so swift/Sources/Basecamp/Generated/Models/Tool.swift declared public let name: String / public let enabled: Bool. bc3 emits neither key, so GetTool, CreateTool and UpdateTool threw DecodingError.keyNotFound on every real response. The other five SDKs failed silently: Go read Enabled == false for every tool, enabled or not.
Grounding
app/views/api/docks/tools/show.json.jbuilder is one line:
json.partial! "recordings/recording", recording: @recording
recordings/_recording.json.jbuilder emits id, status, visible_to_clients, created_at, updated_at, title, inherits_status, type, url, app_url, bookmark_url, subscription_url?, position?, parent?, bucket, creator — no name, no enabled. Sibling dock-tool projections (Todoset, Questionnaire) do carry a name, but from their own recordable partial (json.name recording.recordable.name), which tools have none of.
doc/api/sections/tools.md's "Get a tool" example agrees exactly, so the jbuilder and the doc do not disagree. The doc prose that mentions name/enabled describes the project dock array, which the SDK already models separately and correctly as DockItem (spec/basecamp.smithy) — confirmed, nothing is lost by relaxing Tool. No bc3 companion PR is warranted. Verified against the bc3 revision spec/api-provenance.json currently pins.
What changed
Relaxed name and enabled to optional. They stay modeled, with doc comments pointing at DockItem, rather than being removed: removal is a compile error at every reference in five SDKs, where relaxation degrades to nil.
Absorbed the seven emitted keys the spec omitted. type, visible_to_clients, inherits_status, creator are @required — the partial emits all four unconditionally, recordings.visible_to_clients and .inherits_status are null: false columns, and the five sibling dock-tool projections that render the same partial (Todoset, MessageBoard, Schedule, Questionnaire, Vault) already model exactly these four @required. bookmark_url, subscription_url, parent are optional, matching those siblings.
The three conditional keys are the partial's own branches, not guesses:
| key | branch | consequence |
|---|---|---|
subscription_url | if recording.subscribable? | absent for Vault, Message::Board, Schedule, Inbox, Questionnaire (none overrides Recordable#subscribable?, default false) |
position | if recording.positioned? | absent for a tool disabled in the dock — this is what replaces the enabled: false that never existed |
parent | if !recording.docked? && recording.parent | the dock-tool lookup scopes by recordable type (Recordable::CORE_GROUPS["dock_tools"] includes Vault, and Recording::Recordables turns each group into a bare where recordable_type: scope), so a vault nested inside another vault resolves through GET /dock/tools/:id and does carry one |
Settled, so it does not get re-opened: recordable_type is nullable in schema.rb, yet type is @required here. That is schema laxity, not a wire contract. recordings/_recording.json.jbuilder emits json.type recording.recordable_type and the json.creator do block unconditionally — @required models key presence, which is what those two lines guarantee — and all five sibling structures that decode the very same partial (Todoset, MessageBoard, Schedule, Questionnaire, Vault) already mark the identical four members @required and have shipped that way. A reachable null recordable_type would be breaking those five identically today.
Requiredness is only ever relaxed here, never tightened. status, url, app_url and bucket stay optional even though the partial emits them unconditionally and the siblings mark them @required. Promoting them would add new throw sites — the same class of risk this PR exists to remove — and belongs in its own change.
The openapi diff is exactly the intended change: required [created_at, enabled, id, name, title, updated_at] → [created_at, creator, id, inherits_status, title, type, updated_at, visible_to_clients], 7 properties added, 0 removed, paths byte-identical, Tool the only schema that moved.
Fixtures. All four spec/fixtures/tools/*.json were fabricated and unreferenced by the manifest — they carried name/enabled and matched no bc3 output. Rewritten to the real projection, plus a fifth (nested_vault.json) for the parent branch, and Tool registered in spec/fixtures/manifest.yaml: three operation entries validating the complete GetTool/CreateTool/UpdateTool response bodies, two pointer entries for the conditional-key variants. disabled.json is kept but repurposed — with no enabled key to carry, a disabled tool's real signature is an absent position (and, as a Vault, an absent subscription_url).
Tests. TypeScript and Ruby actually asserted on the fabricated keys (tools.test.ts:49, tools_service_test.rb:19-20), which is why nothing ever went red; both now source their stubs from the shared fixtures so they cannot drift back. All six SDKs gain absence coverage plus the disabled and nested-vault variants. New swift/Tests/BasecampTests/ToolDecodeTests.swift pins the projection under strict decoding. Go's wrapper gains the seven fields plus a service-level test through toolFromGenerated — the existing Unmarshal tests decode straight into the wrapper and bypass the conversion where a dropped field silently zeroes. conformance/tests/paths.json's six tool mock bodies carried the same fabrication and broke the Swift runner's decode; corrected.
Go's wrapper types Name/Enabled as *string/*bool rather than value types with omitempty (as #716 did for SearchResult): there the keys are absent on one branch, here they are absent always, so "" and false would read like real answers.
Red proofs
Proof 1 — the fixture-coverage guard against the old (still-@required) schema. Fixtures corrected and registered, openapi.json not yet regenerated:
Fixture-coverage check failed:
- target `tool-create`: tools/create.json missing required field `enabled`
- target `tool-create`: tools/create.json missing required field `name`
- target `tool-disabled`: tools/disabled.json missing required field `enabled`
- target `tool-disabled`: tools/disabled.json missing required field `name`
- target `tool-get`: tools/get.json missing required field `enabled`
- target `tool-get`: tools/get.json missing required field `name`
- target `tool-nested-vault`: tools/nested_vault.json missing required field `enabled`
- target `tool-nested-vault`: tools/nested_vault.json missing required field `name`
- target `tool-update`: tools/update.json missing required field `enabled`
- target `tool-update`: tools/update.json missing required field `name`
make: *** [check-fixture-coverage] Error 1
REAL_EXIT=2
Green after the spec change:
==> Fixture coverage clean — 33 covered schemas, 51 manifest targets, 2 tracked exclusions, 23 rich-text emitters accounted for
==> Fixture-coverage self-test passed — 1 positive + 8 negative + 20 synthetic cases
REAL_EXIT=0
Proof 2 — the Swift decode test against the pre-change model. swift/Sources/Basecamp/Generated/Models/Tool.swift restored to its 9aef1ccf3 bytes with git show (never git stash — the stash stack is shared across worktrees), and ToolDecodeTests.swift reduced to the one test whose prefix is byte-identical to the shipped file (the other four reference members that do not exist on the old model, so the target would not compile):
ToolDecodeTests.swift:70: error: -[BasecampTests.ToolDecodeTests testDecodesTheRealDockToolProjection] : failed - bc3's real dock-tool projection must decode; got DecodingError.keyNotFound: Key 'enabled' not found in keyed decoding container. Debug description: No value associated with key CodingKeys(stringValue: "enabled", intValue: nil) ("enabled").
Executed 1 test, with 1 failure (0 unexpected) in 4.016 (4.018) seconds
REAL_EXIT=1
Both files restored by cp and confirmed byte-identical with diff -q. Green after:
Executed 5 tests, with 0 failures (0 unexpected)
REAL_EXIT=0
Proof 3 (unplanned, and the useful one for the absorbed half). The @required additions are load-bearing too: with the fabricated stubs still in place, make swift-check came back REAL_EXIT=2 with five GeneratedServiceTests failures — DecodingError.keyNotFound: Key 'creator' not found — which is how the six stale conformance/tests/paths.json mock bodies and the Swift service stubs were found. Green after correcting them.
Per-SDK red proofs for the new absence tests (each mutated to re-add name/enabled, shown red, then restored by cp + diff -q): TypeScript REAL_EXIT=1, Ruby REAL_EXIT=1, Python REAL_EXIT=1, Kotlin REAL_EXIT=1.
Verification
Full serial make green. Also run individually: make go-check-drift (250/250 operations), make go-check-wrapper-drift (92 pairs, 1252 generated fields — 1245 + the 7 new), make go-check-optional-pointers, make kt-check-drift, make py-check-drift, make check-fixture-coverage, make doc-constants-check, make check-projected-examples, make bc3-route-parity, make url-routes-check, make validate-api-gaps. Test counts are deliberately not quoted from local runs — read them off the CI jobs.
Deliberately out of scope
Removing name/enabled from Tool entirely, which is where this ends up. Relaxing keeps every existing reference compiling and returns nil; removal would be a compile error at each one across five SDKs, for the same information content. Worth doing on a major, not here.
Rebased
Rebased onto origin/main after #726 (8dcc755b4) and #727 (0ddc57d49) landed. The only conflict was MIGRATING.md, where both sides insert at the top of # Unreleased — resolved additively, no entry dropped or reordered, and the three replays that touched the same heading were resolved with an asserting script rather than by hand (it fails loudly if the hunk is not the additive-plus-edit shape it expects).
Nothing else conflicted, including conformance/tests/paths.json — main touched conformance/tests/{retry,search}.json, not paths.json. The generated tree was derived, not merged: make generate re-run on the rebased base produces a tree byte-identical to the replayed commits apart from three embedded generated: timestamps, which were diffed before being restored rather than blind-checked-out. git diff between the pre- and post-rebase heads is exactly main's 30 files, so none of this branch's own content moved.
Both red proofs were re-established against the rebased base (a proof captured against the old base does not carry over): fixture coverage REAL_EXIT=2 red / 0 green, Swift decode REAL_EXIT=1 red with DecodingError.keyNotFound: Key 'enabled' not found / 0 green. Full make green.