From 646d74dbc2ba7fb2e0be6042bef1bcabb9b50055 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 16 Aug 2026 22:00:38 -0700 Subject: [PATCH 1/4] Compare what the six generators emitted, not what two of them agree on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The service split is one mapping hand-transcribed into five generator configs, and nothing compared them. The per-SDK `check-*-service-drift` scripts compare operationIds, never service names, so a service added to three tables and forgotten in the other two produced five different SDK surfaces and a fully green `make`. #745 made SPEC section 5's roster derived, but from the generated Kotlin and Swift accessors only, because those two encode account-scoping directly. Codex named the residue: those are two independent transcriptions, not two renderings of one root, so a service omitted from BOTH leaves them agreeing and certifies the old roster and the old count. Agreement between two transcriptions is not confirmation against a root. `scripts/check-service-inventory-parity` reads what the generators ALREADY EMITTED and asserts the emissions agree — five generated service directories, the two accessor files, and Go's hand-written accessors. Reading output rather than reimplementing the mapping is what keeps it from being the sixth hand-copy #745 rejected. Committed generated files only, so it needs no toolchain and lands in the spec-gates job beside kt-check-drift. All eight agree on 53 today. It also checks each accessor file against its own services directory, which is the other half: section 5's roster reads accessors, so a generator that emitted a service file and no accessor would leave the roster short with every directory agreeing the service exists. Normalization is one rule per source, expressed as fields on a SOURCES entry rather than a per-SDK branch: files to drop, one suffix to strip, one identifier casing. Go's three divergences are data too, and are checked for STALENESS rather than merely applied — a fold that closes or a spelling that changes fails the gate instead of quietly ceasing to mean anything. The self-test drives the real checker over synthetic trees built by inverting that normalization, so the builder round-tripping 53 names is itself a positive control. Ten negative cases, and the guard-to-case mapping in its header is measured, not reasoned: each guard was neutered in a copy and the red set recorded. Neutering the parity comparison turns 1, 2, 3 and 9 red; the duplicate check 4; the floor 5; the existence check 6; the two staleness checks 7 and 8; the both-spellings check 10. Neutering either carve-out APPLICATION turns both positive controls red, which is the only way a rule that prevents false positives can be pinned. SPEC.md's derivation rule said no gate compares the tables. It does now, and the sentence says so; the `@service-count` marker beside it is untouched. Appendix F's preamble and sync-doc-constants.rb's note that the residue was open are updated for the same reason. Closes #747 --- .github/workflows/test.yml | 24 ++ Makefile | 23 +- SPEC.md | 10 +- scripts/check-service-inventory-parity | 320 +++++++++++++++++ scripts/sync-doc-constants.rb | 23 +- .../test-check-service-inventory-parity.rb | 337 ++++++++++++++++++ 6 files changed, 721 insertions(+), 16 deletions(-) create mode 100755 scripts/check-service-inventory-parity create mode 100644 scripts/test-check-service-inventory-parity.rb diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index bb8ba4bb7..51d75486e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -185,6 +185,30 @@ jobs: - name: Kotlin service drift (static; jq/grep, no JVM) run: make kt-check-drift + # Cross-SDK, and reads only committed generated files, so it needs no + # toolchain and belongs here rather than in any one SDK's job. The five + # split tables are hand-maintained copies of one mapping and nothing + # compared them; doc-constants-check derives SPEC section 5's roster from + # the Kotlin and Swift accessors alone, which two omissions in the same + # place would leave agreeing. + # + # Under LC_ALL=C so CI exercises the non-UTF-8-locale path (the reads are + # pinned to UTF-8; this proves it stays that way). The accessor and Go + # client files carry non-ASCII text, so an unpinned read raises + # InvalidByteSequenceError before anything is compared. + - name: Six SDKs agree on which services exist + run: make check-service-inventory-parity + env: + LC_ALL: C + + # The run above only ever exercises the PASSING case. This drives + # synthetic repository trees through the same gate, including the + # omitted-from-both-accessor-files case that is the whole reason it exists. + - name: Service inventory parity gate self-test (synthetic trees) + run: make test-check-service-inventory-parity + env: + LC_ALL: C + # The third Go surface. go-check-drift compares generated operations # against the hand-written go/pkg/basecamp wrappers; nothing watched the # GROUPED client until this gate, which is how ArchiveProject and diff --git a/Makefile b/Makefile index 4d20e8214..c3cadd959 100644 --- a/Makefile +++ b/Makefile @@ -1252,7 +1252,7 @@ tools: # Spec-shape lints #------------------------------------------------------------------------------ -.PHONY: check-gradle-serialization test-check-gradle-serialization check-bucket-flat-parity validate-api-gaps check-deprecation-parity kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-fixture-coverage check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability check-fixture-execution check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged check-projected-examples +.PHONY: check-gradle-serialization test-check-gradle-serialization check-bucket-flat-parity check-service-inventory-parity test-check-service-inventory-parity validate-api-gaps check-deprecation-parity kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-fixture-coverage check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability check-fixture-execution check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged check-projected-examples # Verify every bucket-scoped GET list operation has a flat-path counterpart # (or is justified in spec/bucket-scoped-allowlist.txt). Cross-project SDK @@ -1260,6 +1260,25 @@ tools: check-bucket-flat-parity: @./scripts/check-bucket-flat-parity.sh +# Verify the six SDKs agree on WHICH services exist. The service split is one +# mapping hand-transcribed into five generator configs, and the per-SDK +# check-*-service-drift scripts compare operationIds, never service names — so a +# service added to three tables and forgotten in the other two produced five +# different surfaces and a green `make`. This reads what the generators EMITTED +# rather than reimplementing the mapping, which is what keeps it from being a +# sixth copy. Committed generated files only, so no toolchain: it belongs in the +# spec-gates CI job beside kt-check-drift. +check-service-inventory-parity: + @echo "==> Checking cross-SDK service inventory parity..." + @./scripts/check-service-inventory-parity + +# Drive that gate from outside with synthetic repository trees. Its live run only +# ever exercises the PASSING case, so nothing there proves it rejects anything. +# Each case builds all eight renderings in a tmpdir by inverting the gate's own +# normalization, then mutates one; the tracked tree is never written to. +test-check-service-inventory-parity: + @ruby ./scripts/test-check-service-inventory-parity.rb + # Verify @deprecated propagates to all six SDKs in the right signal class # (compiler=Kotlin; editor=TS/Go; doc-only=Ruby/Python/Swift), that the clean # controls stay unmarked, and that no doubled "Deprecated: Deprecated:" slips in. @@ -1520,7 +1539,7 @@ check: if [ $$rc -ne 0 ]; then exit $$rc; fi; \ echo "==> All checks passed" -check-targets: check-gradle-serialization test-check-gradle-serialization lint-actions sync-spec-version-check smithy-check smithy-mapper-test behavior-model-check provenance-check sync-api-version-check doc-constants-check url-routes-check bc3-route-parity test-bc3-route-parity go-check-drift go-check-wrapper-drift go-check-generated-drift check-grouped-client-coverage test-check-grouped-client-coverage auth-routable-check kt-check-drift swift-check-drift go-check ts-check rb-check kt-check swift-check py-check check-bucket-flat-parity validate-api-gaps check-deprecation-parity check-fixture-coverage kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability conformance check-fixture-execution check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged check-projected-examples +check-targets: check-gradle-serialization test-check-gradle-serialization lint-actions sync-spec-version-check smithy-check smithy-mapper-test behavior-model-check provenance-check sync-api-version-check doc-constants-check url-routes-check bc3-route-parity test-bc3-route-parity go-check-drift go-check-wrapper-drift go-check-generated-drift check-grouped-client-coverage test-check-grouped-client-coverage auth-routable-check check-service-inventory-parity test-check-service-inventory-parity kt-check-drift swift-check-drift go-check ts-check rb-check kt-check swift-check py-check check-bucket-flat-parity validate-api-gaps check-deprecation-parity check-fixture-coverage kt-check-optional-arrays-and-scalars go-check-optional-pointers test-enhance-request-reachability check-idempotency-parity check-write-semantics-parity check-retry-metadata-parity check-runner-test-reachability conformance check-fixture-execution check-replay-decoder-parity check-readme-env-vars test-check-readme-env-vars lint-npm-lockfile-writes test-lint-npm-lockfile-writes test-assert-sdk-built test-assert-lockfiles-unchanged check-projected-examples @: # Clean all build artifacts diff --git a/SPEC.md b/SPEC.md index c36560b5c..8f48aca53 100644 --- a/SPEC.md +++ b/SPEC.md @@ -307,7 +307,7 @@ That roster is the canonical surface, not a per-SDK inventory. Accessor counts v ### Derivation Rule `[static]` -The OpenAPI spec groups operations under coarse tags (e.g., `Automation`, `Todos`, `Files`). The service generators split those tags into the `53` fine-grained services above using a two-table mapping: `TAG_TO_SERVICE` (tag → default service name) and `SERVICE_SPLITS` (tag → {service → [operationIds]}). For example, the `Todos` tag splits into `Todos`, `Todolists`, `Todosets`, `TodolistGroups`, `HillCharts`; the `Files` tag splits into `Attachments`, `Uploads`, `Vaults`, `Documents`, `CloudFiles`, `GoogleDocuments`. Both examples are exhaustive on purpose: an abridged one is how `cloudFiles` and `googleDocuments` stayed invisible to this section for so long — a service that arrives through a split rather than a tag of its own is named nowhere a reader would look. These mappings are defined in each language's generator script. They are five hand-maintained copies of one table with no gate comparing them, so they are expected to produce identical service sets and are not guaranteed to: Appendix F records where they currently do not. +The OpenAPI spec groups operations under coarse tags (e.g., `Automation`, `Todos`, `Files`). The service generators split those tags into the `53` fine-grained services above using a two-table mapping: `TAG_TO_SERVICE` (tag → default service name) and `SERVICE_SPLITS` (tag → {service → [operationIds]}). For example, the `Todos` tag splits into `Todos`, `Todolists`, `Todosets`, `TodolistGroups`, `HillCharts`; the `Files` tag splits into `Attachments`, `Uploads`, `Vaults`, `Documents`, `CloudFiles`, `GoogleDocuments`. Both examples are exhaustive on purpose: an abridged one is how `cloudFiles` and `googleDocuments` stayed invisible to this section for so long — a service that arrives through a split rather than a tag of its own is named nowhere a reader would look. These mappings are defined in each language's generator script. They are five hand-maintained copies of one table, and `make check-service-inventory-parity` compares what those copies **emitted** — the five generated service directories, the two generated accessor files this section's roster is derived from, and Go's hand-written accessors — so identical service sets are enforced rather than merely expected. It reads generator output, not the mappings, which is what keeps it from being a sixth copy. Go's three divergences (it folds `automation` and `clientVisibility` into other services and spells `timesheets` singular) are stated as data in that gate and fail it if they ever stop applying; Appendix F records them. ### Merge-Safe Write Surface (Cards) @@ -3649,9 +3649,11 @@ roster. The Kotlin and Swift rows are marked, because §5's roster is derived fr exactly those two files and a restatement of a gated value has to be gated too. Of the other four, Python's is held by its own accessor-inventory test, which derives the roster from `python/src/basecamp/generated/services/` and fails when -an accessor is missing. The remaining three are hand-verified and dated below — -no gate derives them, and building one means a sixth hand-copy of each -generator's split tables. +an accessor is missing, and Go's is read by `make check-service-inventory-parity` +— including the three carve-outs its row states, which that gate fails if they +stop applying. Ruby's and TypeScript's client wiring is hand-verified and dated +below; the parity gate reads their generated service directories, which is what +each generator emitted rather than what the client exposes. | SDK | Account-scoped services | |-----|------------------------| diff --git a/scripts/check-service-inventory-parity b/scripts/check-service-inventory-parity new file mode 100755 index 000000000..06f4880ea --- /dev/null +++ b/scripts/check-service-inventory-parity @@ -0,0 +1,320 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Guard: the six SDKs agree on WHICH services exist. +# +# The service split is one mapping — `TAG_TO_SERVICE` plus `SERVICE_SPLITS` — +# transcribed by hand into five generator configs (typescript/scripts, +# ruby/scripts, kotlin/generator/.../Config.kt, python/scripts, swift's +# ServiceGrouper). Until this gate, nothing compared them. The per-SDK +# `check-*-service-drift` scripts compare operationIds, never service names, so a +# service added to three tables and forgotten in the other two produced five +# different SDK surfaces and a fully green `make`. +# +# THIS IS NOT A SIXTH HAND-COPY, which is the thing #745 rejected. It does not +# reimplement the mapping from openapi.json; it reads what the generators +# ALREADY EMITTED and asserts the emissions agree. The inputs are committed +# generated artefacts, so this needs no toolchain and lives in the spec-gates CI +# job beside kt-check-drift. +# +# WHAT IT CLOSES. `make doc-constants-check` derives SPEC §5's account-scoped +# roster from two files: the generated Kotlin and Swift accessors. Those two are +# independent transcriptions, not two renderings of one root, so a service +# omitted from BOTH generators' tables leaves them agreeing and certifies the +# old roster and the old count. That residue was named on #745 and left open. +# Comparing all eight renderings closes it: the other six have no way to agree +# with a mistake made in those two. +# +# The two accessor files are checked against their OWN generated service +# directories as well, which is the other half. §5's derivation reads accessors; +# a Kotlin generator that emitted a service file but no accessor would leave the +# roster short with every services directory agreeing that the service exists. +# +# NORMALIZATION IS DATA, NOT BRANCHES. Each SDK spells the same service its own +# way, and the entire rule set for a source is the fields on its SOURCES entry: +# which files to drop, one suffix to strip, one identifier casing. Go additionally +# carries three named carve-outs. Nothing here is a per-SDK code path, because the +# failure mode this gate exists to catch is exactly what accumulating per-SDK +# special cases produces. If a new SDK needs a rule that is not one of these +# fields, that is a finding about the generator's naming, not a place to add an +# `if`. +# +# Every read is pinned to UTF-8. Under a non-UTF-8 locale (LC_ALL=C) Ruby tags +# file contents US-ASCII, and the accessor and client files all carry non-ASCII +# text — so an unpinned scan raises InvalidByteSequenceError and the gate fails +# before comparing anything. Both CI steps run under LC_ALL=C so that stays +# proven rather than asserted, matching check-grouped-client-coverage. +# +# Env seams (for scripts/test-check-service-inventory-parity.rb — the tracked +# tree is never written to): +# SERVICE_INVENTORY_ROOT repository root the sources are resolved against +# +# Usage: scripts/check-service-inventory-parity + +require "set" + +REPO_ROOT = File.expand_path("..", __dir__) +ROOT = ENV["SERVICE_INVENTORY_ROOT"] || REPO_ROOT +UTF8 = "UTF-8" + +# Extraction floor. Every source is read with a glob or a regex, and a regex that +# stops matching returns nothing rather than an error — which reads as "all +# clear" and is how a gate silently stops gating. Deliberately far below the real +# value (53): it catches total extraction collapse, not drift, which the parity +# comparison below catches on its own. +MIN_SERVICES = 40 + +# The eight renderings. `spelling` names the identifier casing the raw name +# arrives in; every source is normalized to snake_case, which is the spelling +# three of them already use. +# +# :snake already snake_case my_notifications +# :kebab hyphenated my-notifications +# :camel camelCase or PascalCase myNotifications / MyNotifications +SOURCES = [ + { + id: "typescript", + path: "typescript/src/generated/services", + kind: :directory, + ext: ".ts", + drop: ["index"], + spelling: :kebab, + }, + { + id: "ruby", + path: "ruby/lib/basecamp/generated/services", + kind: :directory, + ext: ".rb", + drop: ["base_service"], + strip_suffix: "_service", + spelling: :snake, + }, + { + id: "python", + path: "python/src/basecamp/generated/services", + kind: :directory, + ext: ".py", + # `_base.py` and `_async_base.py` are hand-written infrastructure that lives + # under generated/ by exception (AGENTS.md Hard Rule 1); `__init__.py` is the + # generated re-export barrel. + drop: ["__init__", "_base", "_async_base"], + # The generator emits `webhooks_service.py` rather than `webhooks.py` to + # avoid clashing with the `basecamp.webhooks` package (generate_services.py's + # `service_filename`). Ruby suffixes every file, so one rule covers both. + strip_suffix: "_service", + spelling: :snake, + }, + { + id: "kotlin", + path: "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services", + kind: :directory, + ext: ".kt", + drop: ["Types"], + spelling: :kebab, + }, + { + id: "swift", + path: "swift/Sources/Basecamp/Generated/Services", + kind: :directory, + ext: ".swift", + strip_suffix: "Service", + spelling: :camel, + }, + { + id: "kotlin-accessors", + path: "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/ServiceAccessors.kt", + kind: :file, + # Same patterns scripts/sync-doc-constants.rb reads SPEC §5's roster with. + # Deliberately identical: this gate's job is to anchor that derivation, which + # it cannot do while reading a different set of lines out of the same file. + pattern: /^val AccountClient\.([A-Za-z][A-Za-z0-9_]*)\s*:/, + spelling: :camel, + }, + { + id: "swift-accessors", + path: "swift/Sources/Basecamp/Generated/AccountClient+Services.swift", + kind: :file, + pattern: /^\s*public var ([A-Za-z][A-Za-z0-9_]*)\s*:\s*[A-Za-z0-9_]+Service\b/, + spelling: :camel, + }, + { + id: "go-accessors", + # Go has no generated per-service files — the hand-written wrappers in + # go/pkg/basecamp ARE its service layer (Appendix F), so its accessors on + # AccountClient are its only rendering of the split. + path: "go/pkg/basecamp/client.go", + kind: :file, + pattern: /^func \(ac \*AccountClient\) (\w+)\(\) \*\w+Service \{/, + spelling: :camel, + carve_outs: :go, + }, +].freeze + +# Go's three divergences from the canonical set, recorded in SPEC Appendix F. +# They are stated here as data for the same reason the spelling rules are: a +# carve-out expressed as a branch is indistinguishable from a bug, and a +# carve-out nobody re-reads is how a surface silently stays behind. +# +# Each is CHECKED FOR STALENESS, not merely applied. A `:folded` name that turns +# up in Go, or a `:spelling` rule whose `from` is gone, fails the gate — so +# closing one of these divergences forces the carve-out to be deleted rather than +# leaving a rule that quietly stops meaning anything. Same device as +# spec/doc-constants.json's committed marker counts. +GO_CARVE_OUTS = [ + { + kind: :folded, + name: "automation", + reason: "its sole operation, ListMarkers, is exposed on LineupService", + }, + { + kind: :folded, + name: "client_visibility", + reason: "its sole operation, SetClientVisibility, is exposed on RecordingsService", + }, + { + kind: :spelling, + from: "timesheet", + to: "timesheets", + reason: "Go spells the accessor Timesheet(), singular", + }, +].freeze + +failures = [] + +def die(failures) + warn "FAIL: cross-SDK service inventory parity" + warn "" + failures.each { |f| warn " - #{f}" } + warn "" + warn " The service split is one mapping transcribed into five generator configs." + warn " This gate compares what they EMITTED. A name that appears in some SDKs and" + warn " not others means a tag mapping was updated in some tables and not the rest." + exit 1 +end + +# PascalCase and camelCase both land on snake_case. The second substitution +# handles an acronym run followed by a word (`HTTPClient` -> `http_client`); +# without it the first leaves the run glued to the word that follows. +def snake_case(name) + name.gsub(/([a-z0-9])([A-Z])/, '\1_\2').gsub(/([A-Z]+)([A-Z][a-z])/, '\1_\2').downcase +end + +def canonicalize(raw, spelling) + case spelling + when :snake then raw + when :kebab then raw.tr("-", "_") + when :camel then snake_case(raw) + else raise "unknown spelling #{spelling.inspect}" + end +end + +def extract(source, path) + raw = if source[:kind] == :directory + Dir.children(path).select { |f| f.end_with?(source[:ext]) }.map { |f| File.basename(f, source[:ext]) } + else + File.read(path, encoding: UTF8).scan(source[:pattern]).flatten + end + + raw = raw.reject { |n| Array(source[:drop]).include?(n) } + if (suffix = source[:strip_suffix]) + raw = raw.map { |n| n.end_with?(suffix) ? n[0...-suffix.length] : n } + end + raw.map { |n| canonicalize(n, source[:spelling]) }.sort +end + +# Go's carve-outs, applied to Go's extracted names and checked for staleness. +def apply_go_carve_outs(names, failures) + present = names.to_set + result = names.dup + + GO_CARVE_OUTS.each do |carve_out| + case carve_out[:kind] + when :folded + name = carve_out[:name] + if present.include?(name) + failures << "go/pkg/basecamp/client.go now exposes `#{name}`, but it is recorded as a " \ + "fold carve-out (#{carve_out[:reason]}). The divergence is closed — delete " \ + "the carve-out from GO_CARVE_OUTS and from SPEC Appendix F's Go row." + else + result << name + end + when :spelling + from = carve_out[:from] + to = carve_out[:to] + if !present.include?(from) + failures << "Go's `#{from}` spelling carve-out (#{carve_out[:reason]}) no longer matches " \ + "anything in go/pkg/basecamp/client.go — either the accessor was renamed, in " \ + "which case delete the carve-out, or it was removed, in which case Go is short " \ + "a service." + elsif present.include?(to) + failures << "Go exposes BOTH `#{from}` and `#{to}`; the spelling carve-out would map one " \ + "onto the other and hide a duplicate surface." + else + result = result.map { |n| n == from ? to : n } + end + end + end + + result.sort +end + +# ------------------------------------------------------------------ extraction + +inventories = {} + +SOURCES.each do |source| + path = File.join(ROOT, source[:path]) + + exists = source[:kind] == :directory ? File.directory?(path) : File.file?(path) + unless exists + die(["#{source[:path]} not found — this gate compares generated output, so a missing " \ + "rendering is a build problem, not a parity verdict"]) + end + + names = extract(source, path) + + # DUPLICATES FIRST, before any set comparison sees them. A name emitted twice + # is invisible downstream — Array#- drops every occurrence, so a doubled entry + # leaves both diffs empty and the gate passes while enumerating something the + # file does not say. Deduplicating instead of refusing would hide a generator + # bug behind a green tick. + dupes = names.tally.select { |_, n| n > 1 }.keys.sort + unless dupes.empty? + die(["#{source[:path]} yields #{dupes.join(', ')} more than once; a repeated name is " \ + "invisible to a set comparison, so nothing could be checked against this"]) + end + + names = apply_go_carve_outs(names, failures) if source[:carve_outs] == :go + + if names.length < MIN_SERVICES + die(["only #{names.length} services read from #{source[:path]} (floor #{MIN_SERVICES}) — " \ + "extraction is probably broken, not the SDK"]) + end + + inventories[source[:id]] = names +end + +# Stale carve-outs are reported before the parity diff they would otherwise +# explain away — a fold carve-out for a service Go now exposes shows up as +# nothing at all in the comparison, since the name is present either way. +die(failures) unless failures.empty? + +# --------------------------------------------------------------------- parity + +union = inventories.values.flatten.uniq.sort + +union.each do |name| + absent = inventories.select { |_id, names| !names.include?(name) }.keys + next if absent.empty? + + present = inventories.keys - absent + failures << "`#{name}` is emitted by #{present.join(', ')} but NOT by #{absent.join(', ')} — " \ + "the split tables disagree about whether this service exists." +end + +die(failures) unless failures.empty? + +count = union.length +puts "OK: cross-SDK service inventory parity — #{count} services agree across " \ + "#{inventories.size} renderings (#{inventories.keys.join(', ')}); " \ + "#{GO_CARVE_OUTS.length} Go carve-outs applied and still current" diff --git a/scripts/sync-doc-constants.rb b/scripts/sync-doc-constants.rb index 886ce0f67..162e073ce 100755 --- a/scripts/sync-doc-constants.rb +++ b/scripts/sync-doc-constants.rb @@ -397,16 +397,19 @@ def read_accessors(relative, pattern) # TypeScript, Ruby and Python tables but omitted from BOTH of these two leaves # them agreeing, and this gate certifies the old roster and the old count. # -# That residue is real and is NOT closed here. The instrument that would close -# it is comparing the GENERATED service inventories of all five SDKs — which is -# not the sixth hand-copy rejected above, since it reads generator OUTPUT rather -# than reimplementing the mapping. Verified viable: the TypeScript, Ruby and -# Python generated service sets already agree exactly with these 53 today, once -# each SDK's index/base files and Python's `_service` filename suffix are -# accounted for. It is left to its own change because that normalization is four -# per-SDK spelling rules, and bolting them onto the gate whose own argument is -# "stop accreting spellings" is the wrong place to introduce them. Codex raised -# this on #745; tracked as the cross-generator comparison follow-up. +# That residue is real and is closed ELSEWHERE, not here: +# scripts/check-service-inventory-parity compares the GENERATED service +# inventories of all six SDKs — which is not the sixth hand-copy rejected above, +# since it reads generator OUTPUT rather than reimplementing the mapping. It also +# checks these two accessor files against their own generated services +# directories, so a Kotlin or Swift generator that emitted a service file but no +# accessor cannot leave this gate certifying a short roster. Codex raised the +# residue on #745; it landed as its own change (#747) because the normalization +# is a per-SDK spelling rule each, and bolting them onto the gate whose own +# argument is "stop accreting spellings" is the wrong place to introduce them. +# +# What is still true here: this gate alone cannot detect a service both +# generators missed. It is sound because the other one runs beside it. # # Failures here are exit 2, not exit 1: a broken source of truth is not # documentation drift, and "the roster disagrees with a file that is itself diff --git a/scripts/test-check-service-inventory-parity.rb b/scripts/test-check-service-inventory-parity.rb new file mode 100644 index 000000000..000d8098c --- /dev/null +++ b/scripts/test-check-service-inventory-parity.rb @@ -0,0 +1,337 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Negative-case self-test for scripts/check-service-inventory-parity. +# +# The gate's own `make check` run only ever exercises the PASSING case, so +# nothing there proves it rejects anything. This repo treats an ungated gate as +# no gate, and a parity gate that cannot fail is worse than none: it converts +# "nobody compared them" into "the gate says they agree". +# +# Every case drives the real checker through its one env seam +# (SERVICE_INVENTORY_ROOT) against a synthetic repository tree in a tmpdir. The +# tracked tree is never written to. +# +# THE SYNTHETIC TREE IS ITSELF UNDER TEST. It is built by INVERTING each source's +# normalization — snake_case back to kebab, camelCase, PascalCase, `_service` +# suffixes, Go's three carve-outs — from one canonical list read out of the real +# Kotlin accessors. A builder that spelled anything differently from the real +# generators would make every negative case below fail for a reason unrelated to +# its mutation, so the synthetic positive control runs second and is as +# load-bearing as the real one. It is also the round-trip proof for the casing +# helpers: 53 names survive canonical -> per-SDK spelling -> canonical. +# +# SERVICE_INVENTORY_CHECKER names the checker under test (default +# scripts/check-service-inventory-parity), so a deliberately-mutated copy can be +# driven through the same suite to prove the suite is not vacuous: +# +# cp scripts/check-service-inventory-parity /tmp/m # mutate the COPY +# SERVICE_INVENTORY_CHECKER=/tmp/m ruby scripts/test-check-service-inventory-parity.rb +# +# The copy resolves its sources from SERVICE_INVENTORY_ROOT, which every case +# sets, so it needs nothing beside it. Never mutate the tracked file. +# +# Every guard in the checker is pinned by at least one case, and the mapping +# below is MEASURED — mutate the guard in a copy, record which cases go red — +# rather than reasoned about. Removing a guard from the copy must turn exactly +# this red: +# +# guard removed in the copy case that must go red +# ------------------------------------------- --------------------- +# the parity comparison 1, 2, 3, 9 +# per-source duplicate check 4 +# extraction floor 5 +# source-exists check 6 +# fold carve-out staleness check 7 +# spelling carve-out staleness check 8 +# spelling carve-out both-spellings check 10 +# APPLYING the fold carve-out both positive controls +# APPLYING the spelling carve-out both positive controls +# +# The last two rows are the measured result, not a tidy one. Applying a carve-out +# is what makes Go's 51 accessors line up with the canonical 53; delete either +# and the real tree stops passing. A rule that exists to prevent false positives +# can only be pinned by a case that is supposed to pass — the same shape as the +# `explained` filter in test-check-grouped-client-coverage.rb. +# +# Cases 1, 2, 3 and 9 share the parity comparison and are not redundant: they are +# four different shapes of disagreement (a service in three SDKs only, a service +# missing from one, a generated file with no accessor beside it, and an SDK short +# a service its carve-outs do not cover), and the messages a reader gets differ. +# Case 3 is the one that anchors SPEC §5 — its roster is derived from the two +# accessor files, and nothing else checks those against their own services +# directories. +# +# WHAT THIS SUITE IS NOT. The suite passing means these ten things are checked. +# It has never meant the list is complete. +# +# Run directly (`ruby scripts/test-check-service-inventory-parity.rb`) or via +# `make test-check-service-inventory-parity`. + +require "fileutils" +require "open3" +require "tmpdir" + +# Per-case lines go to stdout and the failure report to stderr. Unsynced, stdout +# block-buffers when redirected to a file and the report lands ahead of the cases +# it summarizes — which is exactly when someone is reading the log. +$stdout.sync = true + +ROOT = File.expand_path("..", __dir__) +CHECKER = File.expand_path( + ENV.fetch("SERVICE_INVENTORY_CHECKER", "scripts/check-service-inventory-parity"), ROOT +) + +TS_DIR = "typescript/src/generated/services" +RB_DIR = "ruby/lib/basecamp/generated/services" +PY_DIR = "python/src/basecamp/generated/services" +KT_DIR = "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services" +SW_DIR = "swift/Sources/Basecamp/Generated/Services" +KT_ACCESS = "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/ServiceAccessors.kt" +SW_ACCESS = "swift/Sources/Basecamp/Generated/AccountClient+Services.swift" +GO_CLIENT = "go/pkg/basecamp/client.go" + +def read_utf8(path) = File.read(path, encoding: "UTF-8") + +def snake_case(name) + name.gsub(/([a-z0-9])([A-Z])/, '\1_\2').gsub(/([A-Z]+)([A-Z][a-z])/, '\1_\2').downcase +end + +def kebab(name) = name.tr("_", "-") +def pascal(name) = name.split("_").map(&:capitalize).join +def camel(name) = name.split("_").each_with_index.map { |p, i| i.zero? ? p : p.capitalize }.join + +# The canonical roster, read from the real Kotlin accessors rather than typed out +# here — a literal would be a ninth hand-copy of the very table this gate exists +# because there are already five of. +CANONICAL = read_utf8(File.join(ROOT, KT_ACCESS)) + .scan(/^val AccountClient\.([A-Za-z][A-Za-z0-9_]*)\s*:/).flatten.map { |n| snake_case(n) }.sort.freeze + +raise "canonical roster came back with #{CANONICAL.length} services; the real accessors changed shape" \ + if CANONICAL.length < 40 + +# --- Synthetic tree builder ---------------------------------------------------- +# +# One writer per source, each inverting that source's normalization. `names` is +# per-source so a case can give one SDK a different roster from the rest. + +def write_dir(root, rel, filenames) + dir = File.join(root, rel) + FileUtils.mkdir_p(dir) + filenames.each { |f| File.write(File.join(dir, f), "// synthetic\n") } +end + +def write_file(root, rel, body) + path = File.join(root, rel) + FileUtils.mkdir_p(File.dirname(path)) + File.write(path, body) +end + +# Go's carve-outs, inverted: the two folded services have no accessor at all, and +# `timesheets` is spelled singular. +GO_FOLDED = %w[automation client_visibility].freeze + +def go_accessor_names(names) + names.reject { |n| GO_FOLDED.include?(n) }.map { |n| n == "timesheets" ? "timesheet" : n } +end + +def build_root(dir, names) + ts = names.fetch("typescript", CANONICAL) + rb = names.fetch("ruby", CANONICAL) + py = names.fetch("python", CANONICAL) + kt = names.fetch("kotlin", CANONICAL) + sw = names.fetch("swift", CANONICAL) + kta = names.fetch("kotlin-accessors", CANONICAL) + swa = names.fetch("swift-accessors", CANONICAL) + go = names.fetch("go-accessors", go_accessor_names(CANONICAL)) + + write_dir(dir, TS_DIR, ts.map { |n| "#{kebab(n)}.ts" } + ["index.ts"]) + write_dir(dir, RB_DIR, rb.map { |n| "#{n}_service.rb" } + ["base_service.rb"]) + # Python suffixes only `webhooks`; the checker's `strip_suffix` covers both, so + # spelling it faithfully here is what proves that. + write_dir(dir, PY_DIR, py.map { |n| n == "webhooks" ? "webhooks_service.py" : "#{n}.py" } + + ["__init__.py", "_base.py", "_async_base.py"]) + write_dir(dir, KT_DIR, kt.map { |n| "#{kebab(n)}.kt" } + ["Types.kt"]) + write_dir(dir, SW_DIR, sw.map { |n| "#{pascal(n)}Service.swift" }) + + write_file(dir, KT_ACCESS, <<~KOTLIN) + package com.basecamp.sdk.generated + #{kta.map { |n| "val AccountClient.#{camel(n)}: #{pascal(n)}Service\n get() = service(\"#{pascal(n)}\") { #{pascal(n)}Service(this) }" }.join("\n")} + KOTLIN + + write_file(dir, SW_ACCESS, <<~SWIFT) + extension AccountClient { + #{swa.map { |n| " public var #{camel(n)}: #{pascal(n)}Service { service(\"#{camel(n)}\") { #{pascal(n)}Service(accountClient: self) } }" }.join("\n")} + } + SWIFT + + write_file(dir, GO_CLIENT, <<~GO) + package basecamp + #{go.map { |n| "func (ac *AccountClient) #{pascal(n)}() *#{pascal(n)}Service {\n\treturn nil\n}" }.join("\n")} + GO +end + +def run_checker(root) + out, status = Open3.capture2e({ "SERVICE_INVENTORY_ROOT" => root }, "ruby", CHECKER) + # Under LC_ALL=C the captured output comes back tagged US-ASCII, and every + # expected fragment below contains UTF-8 punctuation — so `out.include?` raises + # Encoding::CompatibilityError before it can compare anything, and every case + # dies for a reason unrelated to what it tests. The bytes are UTF-8 either way; + # only the tag is wrong. + [out.dup.force_encoding("UTF-8"), status] +end + +def with_root(names: {}) + Dir.mktmpdir("service-inventory-parity-test") do |dir| + build_root(dir, names) + yield dir if block_given? + run_checker(dir) + end +end + +failures = [] + +def expect_pass(failures, label, out, status) + if status.success? + puts " PASS #{label}" + else + puts " FAIL #{label}" + failures << "#{label}: expected PASS but checker failed:\n#{out}" + end +end + +def expect_fail(failures, label, out, status, fragment) + if status.success? + puts " FAIL #{label}" + failures << "#{label}: expected FAILURE but checker passed:\n#{out}" + elsif !out.include?(fragment) + puts " FAIL #{label}" + failures << "#{label}: failed as expected but message missing #{fragment.inspect}:\n#{out}" + else + puts " PASS #{label}" + end +end + +puts "==> service inventory parity self-test (checker: #{CHECKER.sub("#{ROOT}/", '')})" + +# --- Positive controls --------------------------------------------------------- +# +# Both load-bearing. The first says the real tree agrees; the second says the +# synthetic builder spells all eight renderings the way the real generators do, +# without which no negative case below means anything. + +out, status = run_checker(ROOT) +expect_pass(failures, "positive control: the real tree passes", out, status) + +out, status = with_root +expect_pass(failures, "positive control: the synthetic tree passes (builder round-trips #{CANONICAL.length} names)", + out, status) + +# --- 1. THE #745 RESIDUE ------------------------------------------------------- +# +# A service added to the TypeScript, Ruby and Python tables and omitted from both +# Kotlin and Swift. `make doc-constants-check` derives SPEC §5's roster from the +# Kotlin and Swift accessors alone, so those two agree with each other, certify +# the old roster and the old count, and see nothing. This is the case that gate +# structurally cannot catch and the reason this one exists. + +extra = CANONICAL + ["fanfares"] +out, status = with_root(names: { "typescript" => extra, "ruby" => extra, "python" => extra }) +expect_fail(failures, "1. service in the TS/Ruby/Python tables only (#745 residue)", out, status, + "`fanfares` is emitted by typescript, ruby, python but NOT by " \ + "kotlin, swift, kotlin-accessors, swift-accessors, go-accessors") + +# --- 2. One SDK short ---------------------------------------------------------- + +out, status = with_root(names: { "ruby" => CANONICAL - ["gauges"] }) +expect_fail(failures, "2. one SDK missing a service the other seven emit", out, status, + "but NOT by ruby") + +# --- 3. A generated service with no accessor beside it ------------------------- +# +# Kotlin emits the service file but no accessor. SPEC §5's roster reads the +# ACCESSORS, so the roster comes up short while every services directory agrees +# the service exists — and nothing but this checked the two against each other. + +out, status = with_root(names: { "kotlin-accessors" => CANONICAL - ["wormholes"] }) +expect_fail(failures, "3. Kotlin service file with no accessor (anchors SPEC section 5)", out, status, + "but NOT by kotlin-accessors") + +# --- 4. A name emitted twice --------------------------------------------------- +# +# Invisible to every set comparison downstream: Array#- drops all occurrences, so +# both diffs come back empty and the gate would pass while enumerating something +# the file does not say. + +out, status = with_root do |dir| + path = File.join(dir, SW_ACCESS) + body = read_utf8(path) + line = body.lines.find { |l| l.include?(" public var gauges:") } + raise "no gauges accessor in the synthetic Swift file" if line.nil? + File.write(path, body.sub(line, line + line)) +end +expect_fail(failures, "4. duplicate accessor in one source", out, status, + "yields gauges more than once") + +# --- 5. Extraction collapse ---------------------------------------------------- +# +# A glob or regex that stops matching returns nothing rather than an error, which +# reads as "all clear". The floor turns that into a failure instead. + +out, status = with_root(names: { "typescript" => CANONICAL.first(3) }) +expect_fail(failures, "5. extraction floor catches a collapsed source", out, status, + "extraction is probably broken, not the SDK") + +# --- 6. A rendering that is not there ------------------------------------------ + +out, status = with_root { |dir| FileUtils.rm(File.join(dir, GO_CLIENT)) } +expect_fail(failures, "6. missing input is a build problem, not a parity verdict", out, status, + "go/pkg/basecamp/client.go not found") + +# --- 7. A fold carve-out that closed ------------------------------------------- +# +# Go grows an Automation() accessor. Without the staleness check this is INVISIBLE +# — the carve-out adds `automation` to Go's set either way, so the parity diff is +# empty and SPEC Appendix F keeps claiming a fold that no longer exists. + +out, status = with_root(names: { "go-accessors" => go_accessor_names(CANONICAL) + ["automation"] }) +expect_fail(failures, "7. Go closed the automation fold, carve-out now stale", out, status, + "now exposes `automation`, but it is recorded as a fold carve-out") + +# --- 8. A spelling carve-out that closed --------------------------------------- + +out, status = with_root(names: { "go-accessors" => go_accessor_names(CANONICAL) - ["timesheet"] + ["timesheets"] }) +expect_fail(failures, "8. Go renamed Timesheet to Timesheets, spelling carve-out now stale", out, status, + "spelling carve-out (Go spells the accessor Timesheet(), singular) no longer matches") + +# --- 9. Go short a service its carve-outs do not cover -------------------------- +# +# The carve-outs are three named divergences, not a blanket exemption. A fourth +# service missing from Go has to fail like any other SDK's would. + +out, status = with_root(names: { "go-accessors" => go_accessor_names(CANONICAL) - ["gauges"] }) +expect_fail(failures, "9. Go missing an uncarved service still fails", out, status, + "but NOT by go-accessors") + +# --- 10. Both spellings at once ------------------------------------------------ +# +# The rename would map one onto the other, and the duplicate it creates is +# reported by the check that runs before any set comparison — but only because +# the carve-out refuses instead of renaming blind. + +out, status = with_root(names: { "go-accessors" => go_accessor_names(CANONICAL) + ["timesheets"] }) +expect_fail(failures, "10. Go exposing both Timesheet and Timesheets", out, status, + "Go exposes BOTH `timesheet` and `timesheets`") + +# --- Report -------------------------------------------------------------------- + +puts +if failures.empty? + puts "==> service inventory parity self-test: all cases passed" + exit 0 +else + warn "==> service inventory parity self-test: #{failures.length} case(s) failed" + warn "" + failures.each { |f| warn " #{f}\n\n" } + exit 1 +end From 1ba479489e2a6d3d4a9112357191df5644daa841 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 16 Aug 2026 22:28:25 -0700 Subject: [PATCH 2/4] Read Python from its barrel, and state the motivation accurately MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings, both correct. CODEX: a stale Python module counted as still emitted. Of the five generators, four sweep outputs the current mapping no longer produces (generate-services.ts:1736, generate-services.rb:344, Main.kt:65, and Swift regenerates whole); `python/scripts/generate_services.py` does not. So dropping a Python service leaves its module on disk, and a directory listing counts the corpse — the exact divergence this gate exists to catch, passing it. It reaches CI, which I had assumed it did not. `check-python-service- drift.sh` regenerates to a tmpdir and `diff -rq`s, so it reports `Only in : fanfares.py` and fails — verified. But the Python CI job does not run that script. It regenerates IN PLACE and tests `git status --porcelain`, which is blind to a stale file that is already committed: nothing rewrites it, nothing deletes it, so there is no diff. Verified in a throwaway repo: 0 porcelain lines, corpse still present. Python is now read from its generated `__init__.py`, which is rewritten whole every run and so cannot name a service that is no longer emitted. It also does not import `_base.py`/`_async_base.py`, so the hand-written infrastructure drops out of the reading instead of being filtered from it. The generator defect itself is #757; this only stops one gate believing a corpse. Two cases, both red-proven against the pre-fix checker, which fails 11 with "`fanfares` is emitted by python but NOT by ..." and passes 12: 11. stale module on disk, absent from the barrel -> must PASS 12. barrel short a service the directory still has -> must FAIL Case 11 is pass-shaped on purpose: nothing else can pin a fix whose whole effect is to stop a false failure. Reverting the source to a directory listing turns 11 and 12 red and nothing else, measured. COPILOT: the motivation prose was false. It said the per-SDK drift scripts compare operationIds and never service names. TypeScript, Ruby, Python and Swift regenerate-and-diff whole trees, so they do see service filenames; only Kotlin's fast check is operationId-only. The real gap is the AXIS, not the granularity: each validates one SDK against its own generator and can see no other SDK. Reworded in the script header, the Makefile and the workflow, and SPEC's derivation rule now names the barrel rather than claiming five directories. A gate whose stated motivation is false is one nobody can size later, which is the whole reason this is worth a commit rather than a footnote. Refs #757 --- .github/workflows/test.yml | 8 +- Makefile | 18 ++-- SPEC.md | 2 +- scripts/check-service-inventory-parity | 51 ++++++++--- .../test-check-service-inventory-parity.rb | 87 ++++++++++++++----- 5 files changed, 123 insertions(+), 43 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 51d75486e..eb4c0dd93 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -188,9 +188,11 @@ jobs: # Cross-SDK, and reads only committed generated files, so it needs no # toolchain and belongs here rather than in any one SDK's job. The five # split tables are hand-maintained copies of one mapping and nothing - # compared them; doc-constants-check derives SPEC section 5's roster from - # the Kotlin and Swift accessors alone, which two omissions in the same - # place would leave agreeing. + # compared them TO EACH OTHER — every check-*-service-drift script + # validates one SDK against its own generator and can see no other SDK. + # doc-constants-check derives SPEC section 5's roster from the Kotlin and + # Swift accessors alone, which two omissions in the same place would leave + # agreeing. # # Under LC_ALL=C so CI exercises the non-UTF-8-locale path (the reads are # pinned to UTF-8; this proves it stays that way). The accessor and Go diff --git a/Makefile b/Makefile index c3cadd959..993429f1a 100644 --- a/Makefile +++ b/Makefile @@ -1261,13 +1261,17 @@ check-bucket-flat-parity: @./scripts/check-bucket-flat-parity.sh # Verify the six SDKs agree on WHICH services exist. The service split is one -# mapping hand-transcribed into five generator configs, and the per-SDK -# check-*-service-drift scripts compare operationIds, never service names — so a -# service added to three tables and forgotten in the other two produced five -# different surfaces and a green `make`. This reads what the generators EMITTED -# rather than reimplementing the mapping, which is what keeps it from being a -# sixth copy. Committed generated files only, so no toolchain: it belongs in the -# spec-gates CI job beside kt-check-drift. +# mapping hand-transcribed into five generator configs, and every per-SDK +# check-*-service-drift script validates ONE SDK against ITS OWN generator and +# config — the TypeScript/Ruby/Python/Swift ones by regenerate-and-diff (which +# does see service filenames), Kotlin's fast one by operationId. What none of +# them can do is look at another SDK. So a service added to three tables and +# forgotten in the other two is invisible to all of them: each SDK is faithful to +# its own table and the tables disagree. This is the cross-SDK axis, not a +# freshness check, and it does not replace them. It reads what the generators +# EMITTED rather than reimplementing the mapping, which is what keeps it from +# being a sixth copy. Committed generated files only, so no toolchain: it belongs +# in the spec-gates CI job beside kt-check-drift. check-service-inventory-parity: @echo "==> Checking cross-SDK service inventory parity..." @./scripts/check-service-inventory-parity diff --git a/SPEC.md b/SPEC.md index 8f48aca53..39a5824d7 100644 --- a/SPEC.md +++ b/SPEC.md @@ -307,7 +307,7 @@ That roster is the canonical surface, not a per-SDK inventory. Accessor counts v ### Derivation Rule `[static]` -The OpenAPI spec groups operations under coarse tags (e.g., `Automation`, `Todos`, `Files`). The service generators split those tags into the `53` fine-grained services above using a two-table mapping: `TAG_TO_SERVICE` (tag → default service name) and `SERVICE_SPLITS` (tag → {service → [operationIds]}). For example, the `Todos` tag splits into `Todos`, `Todolists`, `Todosets`, `TodolistGroups`, `HillCharts`; the `Files` tag splits into `Attachments`, `Uploads`, `Vaults`, `Documents`, `CloudFiles`, `GoogleDocuments`. Both examples are exhaustive on purpose: an abridged one is how `cloudFiles` and `googleDocuments` stayed invisible to this section for so long — a service that arrives through a split rather than a tag of its own is named nowhere a reader would look. These mappings are defined in each language's generator script. They are five hand-maintained copies of one table, and `make check-service-inventory-parity` compares what those copies **emitted** — the five generated service directories, the two generated accessor files this section's roster is derived from, and Go's hand-written accessors — so identical service sets are enforced rather than merely expected. It reads generator output, not the mappings, which is what keeps it from being a sixth copy. Go's three divergences (it folds `automation` and `clientVisibility` into other services and spells `timesheets` singular) are stated as data in that gate and fail it if they ever stop applying; Appendix F records them. +The OpenAPI spec groups operations under coarse tags (e.g., `Automation`, `Todos`, `Files`). The service generators split those tags into the `53` fine-grained services above using a two-table mapping: `TAG_TO_SERVICE` (tag → default service name) and `SERVICE_SPLITS` (tag → {service → [operationIds]}). For example, the `Todos` tag splits into `Todos`, `Todolists`, `Todosets`, `TodolistGroups`, `HillCharts`; the `Files` tag splits into `Attachments`, `Uploads`, `Vaults`, `Documents`, `CloudFiles`, `GoogleDocuments`. Both examples are exhaustive on purpose: an abridged one is how `cloudFiles` and `googleDocuments` stayed invisible to this section for so long — a service that arrives through a split rather than a tag of its own is named nowhere a reader would look. These mappings are defined in each language's generator script. They are five hand-maintained copies of one table, and `make check-service-inventory-parity` compares what those copies **emitted** — the TypeScript, Ruby, Kotlin and Swift generated service directories, Python's generated `__init__.py` barrel, the two generated accessor files this section's roster is derived from, and Go's hand-written accessors — so identical service sets are enforced rather than merely expected. It reads generator output, not the mappings, which is what keeps it from being a sixth copy. (Python is read from its barrel because its generator, alone among the five, does not delete outputs a mapping stopped producing; the barrel is rewritten whole every run and so cannot name a service that is no longer emitted.) Each per-SDK `check-*-service-drift` script remains the freshness gate for its own SDK; none of them can see another SDK, which is the axis this one adds. Go's three divergences (it folds `automation` and `clientVisibility` into other services and spells `timesheets` singular) are stated as data in that gate and fail it if they ever stop applying; Appendix F records them. ### Merge-Safe Write Surface (Cards) diff --git a/scripts/check-service-inventory-parity b/scripts/check-service-inventory-parity index 06f4880ea..becb52deb 100755 --- a/scripts/check-service-inventory-parity +++ b/scripts/check-service-inventory-parity @@ -6,10 +6,22 @@ # The service split is one mapping — `TAG_TO_SERVICE` plus `SERVICE_SPLITS` — # transcribed by hand into five generator configs (typescript/scripts, # ruby/scripts, kotlin/generator/.../Config.kt, python/scripts, swift's -# ServiceGrouper). Until this gate, nothing compared them. The per-SDK -# `check-*-service-drift` scripts compare operationIds, never service names, so a -# service added to three tables and forgotten in the other two produced five -# different SDK surfaces and a fully green `make`. +# ServiceGrouper). Until this gate, nothing compared them TO EACH OTHER. +# +# Be precise about what the existing checks do, because the loose version of this +# sentence ("they only compare operationIds") is false and a gate whose stated +# motivation is false is one nobody can size later. The TypeScript, Ruby, Python +# and Swift `check-*-service-drift` scripts are regenerate-and-diff gates: they +# re-run the generator into a tmpdir and `diff -rq` the whole tree, so they DO see +# service filenames. Kotlin's fast drift check is the operationId-only one. What +# every one of them has in common is the axis, not the granularity: each validates +# ONE SDK against ITS OWN generator and config. None of them can see another SDK. +# +# So a service added to three split tables and forgotten in the other two is +# invisible to all of them — every SDK is faithful to its own table, and the +# tables disagree. That produced different SDK surfaces under a fully green +# `make`. This gate is the cross-SDK axis, not a freshness check; the per-SDK +# scripts remain the freshness checks and this one does not replace them. # # THIS IS NOT A SIXTH HAND-COPY, which is the thing #745 rejected. It does not # reimplement the mapping from openapi.json; it reads what the generators @@ -91,13 +103,30 @@ SOURCES = [ }, { id: "python", - path: "python/src/basecamp/generated/services", - kind: :directory, - ext: ".py", - # `_base.py` and `_async_base.py` are hand-written infrastructure that lives - # under generated/ by exception (AGENTS.md Hard Rule 1); `__init__.py` is the - # generated re-export barrel. - drop: ["__init__", "_base", "_async_base"], + # PYTHON IS READ FROM ITS BARREL, NOT ITS DIRECTORY, and it is the only + # source that needs to be. The TypeScript, Ruby and Kotlin generators all + # delete outputs the current mapping no longer produces + # (generate-services.ts:1736, generate-services.rb:344, Main.kt:65); + # `python/scripts/generate_services.py` does not. So a mapping that DROPS a + # Python service leaves its module on disk, and a directory listing counts + # that corpse as still emitted — the exact divergence this gate exists to + # catch, passing it. + # + # It reaches CI. The local `check-python-service-drift.sh` regenerates to a + # tmpdir and `diff -rq`s, which reports `Only in : stale.py` and + # fails — but the Python CI job does not run that script. It regenerates IN + # PLACE and tests `git status --porcelain`, which is blind to a stale file + # that is already committed: nothing rewrites it, nothing deletes it, so + # there is no diff to report. + # + # `__init__.py` is rewritten whole on every run and names exactly the + # services the mapping produced, so it cannot carry a corpse. `_base.py` and + # `_async_base.py` are hand-written infrastructure living under generated/ by + # exception (AGENTS.md Hard Rule 1) and the barrel does not import them, + # which is a second reason to read it rather than filter the directory. + path: "python/src/basecamp/generated/services/__init__.py", + kind: :file, + pattern: /^from basecamp\.generated\.services\.(\w+) import/, # The generator emits `webhooks_service.py` rather than `webhooks.py` to # avoid clashing with the `basecamp.webhooks` package (generate_services.py's # `service_filename`). Ruby suffixes every file, so one rule covers both. diff --git a/scripts/test-check-service-inventory-parity.rb b/scripts/test-check-service-inventory-parity.rb index 000d8098c..6788643c9 100644 --- a/scripts/test-check-service-inventory-parity.rb +++ b/scripts/test-check-service-inventory-parity.rb @@ -36,33 +36,41 @@ # rather than reasoned about. Removing a guard from the copy must turn exactly # this red: # -# guard removed in the copy case that must go red -# ------------------------------------------- --------------------- -# the parity comparison 1, 2, 3, 9 +# guard removed in the copy cases that must go red +# ------------------------------------------- ---------------------- +# the parity comparison 1, 2, 3, 9, 12 # per-source duplicate check 4 # extraction floor 5 # source-exists check 6 # fold carve-out staleness check 7 # spelling carve-out staleness check 8 # spelling carve-out both-spellings check 10 -# APPLYING the fold carve-out both positive controls -# APPLYING the spelling carve-out both positive controls +# reading Python from its BARREL 11, 12 +# APPLYING the fold carve-out both positive controls, 11 +# APPLYING the spelling carve-out both positive controls, 11 # -# The last two rows are the measured result, not a tidy one. Applying a carve-out -# is what makes Go's 51 accessors line up with the canonical 53; delete either -# and the real tree stops passing. A rule that exists to prevent false positives -# can only be pinned by a case that is supposed to pass — the same shape as the -# `explained` filter in test-check-grouped-client-coverage.rb. +# The last three rows are the measured result, not a tidy one. # -# Cases 1, 2, 3 and 9 share the parity comparison and are not redundant: they are -# four different shapes of disagreement (a service in three SDKs only, a service -# missing from one, a generated file with no accessor beside it, and an SDK short -# a service its carve-outs do not cover), and the messages a reader gets differ. -# Case 3 is the one that anchors SPEC §5 — its roster is derived from the two -# accessor files, and nothing else checks those against their own services -# directories. +# Applying a carve-out is what makes Go's 51 accessors line up with the canonical +# 53; delete either and the real tree stops passing. A rule that exists to prevent +# false positives can only be pinned by a case that is supposed to pass — the same +# shape as the `explained` filter in test-check-grouped-client-coverage.rb. # -# WHAT THIS SUITE IS NOT. The suite passing means these ten things are checked. +# Case 11 is the other pass-shaped case, and it shows up in those two rows for the +# same structural reason: a case asserting PASS goes red for ANY breakage that +# stops the checker passing at all, so it is not a specific pin for the carve-outs. +# It is a specific pin for the barrel reading, because reverting Python to a +# directory listing turns 11 and 12 red and nothing else. +# +# Cases 1, 2, 3, 9 and 12 share the parity comparison and are not redundant: they +# are five different shapes of disagreement (a service in three SDKs only, a +# service missing from one, a generated file with no accessor beside it, an SDK +# short a service its carve-outs do not cover, and a barrel short one while the +# directory beside it is complete), and the messages a reader gets differ. Case 3 +# is the one that anchors SPEC §5 — its roster is derived from the two accessor +# files, and nothing else checks those against their own services directories. +# +# WHAT THIS SUITE IS NOT. The suite passing means these twelve things are checked. # It has never meant the list is complete. # # Run directly (`ruby scripts/test-check-service-inventory-parity.rb`) or via @@ -87,6 +95,7 @@ PY_DIR = "python/src/basecamp/generated/services" KT_DIR = "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services" SW_DIR = "swift/Sources/Basecamp/Generated/Services" +PY_BARREL = "python/src/basecamp/generated/services/__init__.py" KT_ACCESS = "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/ServiceAccessors.kt" SW_ACCESS = "swift/Sources/Basecamp/Generated/AccountClient+Services.swift" GO_CLIENT = "go/pkg/basecamp/client.go" @@ -98,6 +107,9 @@ def snake_case(name) end def kebab(name) = name.tr("_", "-") +# The generator's one filename rename (generate_services.py's `service_filename`). +def py_module(name) = name == "webhooks" ? "webhooks_service" : name +def py_module_filename(name) = "#{py_module(name)}.py" def pascal(name) = name.split("_").map(&:capitalize).join def camel(name) = name.split("_").each_with_index.map { |p, i| i.zero? ? p : p.capitalize }.join @@ -147,10 +159,16 @@ def build_root(dir, names) write_dir(dir, TS_DIR, ts.map { |n| "#{kebab(n)}.ts" } + ["index.ts"]) write_dir(dir, RB_DIR, rb.map { |n| "#{n}_service.rb" } + ["base_service.rb"]) - # Python suffixes only `webhooks`; the checker's `strip_suffix` covers both, so - # spelling it faithfully here is what proves that. - write_dir(dir, PY_DIR, py.map { |n| n == "webhooks" ? "webhooks_service.py" : "#{n}.py" } + + # Python is read from its BARREL, so the directory is written from + # `python_modules` (which a case can leave stale) while the barrel is written + # from `py`. They are the same list unless a case separates them. + py_modules = names.fetch("python-modules", py) + write_dir(dir, PY_DIR, py_modules.map { |n| py_module_filename(n) } + ["__init__.py", "_base.py", "_async_base.py"]) + write_file(dir, PY_BARREL, <<~PYTHON) + # @generated from OpenAPI spec — do not edit manually + #{py.map { |n| "from basecamp.generated.services.#{py_module(n)} import #{pascal(n)}Service, Async#{pascal(n)}Service" }.join("\n")} + PYTHON write_dir(dir, KT_DIR, kt.map { |n| "#{kebab(n)}.kt" } + ["Types.kt"]) write_dir(dir, SW_DIR, sw.map { |n| "#{pascal(n)}Service.swift" }) @@ -323,6 +341,33 @@ def expect_fail(failures, label, out, status, fragment) expect_fail(failures, "10. Go exposing both Timesheet and Timesheets", out, status, "Go exposes BOTH `timesheet` and `timesheets`") +# --- 11. A stale Python module on disk, absent from the barrel ----------------- +# +# The one source read from a barrel rather than a directory, and the reason why. +# `python/scripts/generate_services.py` does not delete outputs the current +# mapping stopped producing — unlike generate-services.ts:1736, +# generate-services.rb:344 and Main.kt:65, which all do. So a mapping that DROPS +# a Python service leaves `fanfares.py` on disk, and a directory listing counts +# the corpse as still emitted. +# +# This case EXPECTS A PASS, which is the only shape that can pin the fix: the +# stale module must not make Python look like it emits a service the other seven +# do not. Run against the pre-fix checker (which enumerated the directory) the +# same input fails with "`fanfares` is emitted by python but NOT by ...". + +out, status = with_root(names: { "python-modules" => CANONICAL + ["fanfares"] }) +expect_pass(failures, "11. stale Python module on disk is not counted as emitted", out, status) + +# --- 12. The Python barrel itself short ---------------------------------------- +# +# Case 11 must not have made Python's rendering vacuous: a service genuinely +# absent from the barrel still has to fail. Note the directory here keeps ALL the +# modules, so only the barrel reading can catch it. + +out, status = with_root(names: { "python" => CANONICAL - ["gauges"], "python-modules" => CANONICAL }) +expect_fail(failures, "12. Python barrel omitting a service the other seven emit", out, status, + "but NOT by python") + # --- Report -------------------------------------------------------------------- puts From d3dfab58e252f4c4747e65241028bd3ca0131356 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 16 Aug 2026 23:00:25 -0700 Subject: [PATCH 3/4] Mirror Python's rename exactly, instead of stripping any _service suffix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex, correctly. Python's entry borrowed Ruby's `strip_suffix: "_service"`, and the comment said "Ruby suffixes every file, so one rule covers both". Sharing the rule is exactly the bug: it is right for Ruby and wrong for Python. `generate_services.py`'s `service_filename` renames one module and tests for it by name: snake = to_snake_case(name) if snake == "webhooks": return "webhooks_service.py" return f"{snake}.py" Every other service keeps its snake name verbatim — including one whose canonical name legitimately ends in `_service`. A blind suffix strip took such a name apart, so Python reported a service the other seven did not have and the gate failed on a mapping where every generator agreed. That direction matters. This was a FALSE POSITIVE, not a miss: a red build blocking a correct change, and an invitation to "fix" a mapping that was never wrong. Ruby keeps `strip_suffix:` because suffixing every file genuinely is its rule; Python gets `rename:`, an exact-match map applied before any suffix logic. Two instruments, because the two generators do two different things. Case 13 adds a service whose canonical name is `notification_service` across all eight renderings and requires a PASS. The roster still holds `webhooks`, so one case pins both halves: the module that IS renamed and one that merely looks like it. Red-proven against the pre-fix checker, which fails it with both sides of the split it invents: `notification` is emitted by python but NOT by typescript, ruby, ... `notification_service` is emitted by typescript, ruby, ... but NOT by python Kill matrix re-measured. Respelling the rename as `strip_suffix` turns 13 red and nothing else, which is what makes it a pin rather than a smoke alarm. Deleting the rename outright, or never applying it, turns both positive controls plus 11 and 13 red — 11 and 13 are pass-shaped and go red on any global breakage, and the header now says so rather than implying they are specific. --- scripts/check-service-inventory-parity | 30 ++++++-- .../test-check-service-inventory-parity.rb | 71 +++++++++++++++---- 2 files changed, 83 insertions(+), 18 deletions(-) diff --git a/scripts/check-service-inventory-parity b/scripts/check-service-inventory-parity index becb52deb..5985a2f47 100755 --- a/scripts/check-service-inventory-parity +++ b/scripts/check-service-inventory-parity @@ -127,10 +127,24 @@ SOURCES = [ path: "python/src/basecamp/generated/services/__init__.py", kind: :file, pattern: /^from basecamp\.generated\.services\.(\w+) import/, - # The generator emits `webhooks_service.py` rather than `webhooks.py` to - # avoid clashing with the `basecamp.webhooks` package (generate_services.py's - # `service_filename`). Ruby suffixes every file, so one rule covers both. - strip_suffix: "_service", + # AN EXACT RENAME, NOT A SUFFIX STRIP, and the difference is not cosmetic. + # `generate_services.py`'s `service_filename` renames exactly one module: + # + # snake = to_snake_case(name) + # if snake == "webhooks": return "webhooks_service.py" + # return f"{snake}.py" + # + # The test is `== "webhooks"`, so every other service keeps its snake name + # verbatim — including one whose canonical name legitimately ENDS in + # `_service`. Stripping the suffix unconditionally (as Ruby correctly does, + # because Ruby suffixes every file) would take that name apart, report a + # Python service the other seven do not have, and fail a build where every + # generator mapping agreed. A false positive here is worse than a miss: it + # blocks a correct change and invites someone to "fix" a mapping that was + # never wrong. + # + # So the rule mirrors the generator exactly: one module, renamed by name. + rename: { "webhooks_service" => "webhooks" }, spelling: :snake, }, { @@ -245,6 +259,14 @@ def extract(source, path) end raw = raw.reject { |n| Array(source[:drop]).include?(n) } + # `rename:` is exact-match and runs BEFORE `strip_suffix:`. The two are + # deliberately different instruments: a suffix strip encodes "this generator + # suffixes every file" (Ruby, Swift), a rename encodes "this generator + # special-cases exactly these names" (Python). Using the first where the second + # is true mangles any service whose real name happens to end in the suffix. + if (renames = source[:rename]) + raw = raw.map { |n| renames.fetch(n, n) } + end if (suffix = source[:strip_suffix]) raw = raw.map { |n| n.end_with?(suffix) ? n[0...-suffix.length] : n } end diff --git a/scripts/test-check-service-inventory-parity.rb b/scripts/test-check-service-inventory-parity.rb index 6788643c9..c18544ffb 100644 --- a/scripts/test-check-service-inventory-parity.rb +++ b/scripts/test-check-service-inventory-parity.rb @@ -46,21 +46,28 @@ # spelling carve-out staleness check 8 # spelling carve-out both-spellings check 10 # reading Python from its BARREL 11, 12 -# APPLYING the fold carve-out both positive controls, 11 -# APPLYING the spelling carve-out both positive controls, 11 +# Python's rename respelled as a SUFFIX STRIP 13 +# APPLYING the fold carve-out positive controls, 11, 13 +# APPLYING the spelling carve-out positive controls, 11, 13 +# Python's rename rule deleted, or never applied positive controls, 11, 13 # -# The last three rows are the measured result, not a tidy one. +# THE PASS-SHAPED CASES, and why the bottom rows look blunt. 11 and 13 both assert +# a PASS, so they go red for ANY breakage that stops the checker passing at all — +# they are not specific pins for the carve-outs or for the rename's existence. +# Each is a specific pin for exactly one thing, and that is the row above the +# blunt ones: # -# Applying a carve-out is what makes Go's 51 accessors line up with the canonical -# 53; delete either and the real tree stops passing. A rule that exists to prevent -# false positives can only be pinned by a case that is supposed to pass — the same -# shape as the `explained` filter in test-check-grouped-client-coverage.rb. +# - 11 pins the barrel reading: reverting Python to a directory listing turns +# 11 and 12 red and nothing else. +# - 13 pins the SHAPE of Python's rename: respelling it as `strip_suffix: +# "_service"` — the pre-fix code — turns 13 red AND NOTHING ELSE, which is +# what makes it a real pin rather than a smoke alarm. # -# Case 11 is the other pass-shaped case, and it shows up in those two rows for the -# same structural reason: a case asserting PASS goes red for ANY breakage that -# stops the checker passing at all, so it is not a specific pin for the carve-outs. -# It is a specific pin for the barrel reading, because reverting Python to a -# directory listing turns 11 and 12 red and nothing else. +# A rule that exists to prevent false positives can only be pinned by a case that +# is supposed to pass; the same shape as the `explained` filter in +# test-check-grouped-client-coverage.rb. Applying a Go carve-out is likewise what +# makes its 51 accessors line up with the canonical 53, so deleting either stops +# the real tree passing. # # Cases 1, 2, 3, 9 and 12 share the parity comparison and are not redundant: they # are five different shapes of disagreement (a service in three SDKs only, a @@ -70,8 +77,9 @@ # is the one that anchors SPEC §5 — its roster is derived from the two accessor # files, and nothing else checks those against their own services directories. # -# WHAT THIS SUITE IS NOT. The suite passing means these twelve things are checked. -# It has never meant the list is complete. +# WHAT THIS SUITE IS NOT. The suite passing means these thirteen things are +# checked. It has never meant the list is complete — 13 exists because a reviewer +# found a false positive the other twelve could not see. # # Run directly (`ruby scripts/test-check-service-inventory-parity.rb`) or via # `make test-check-service-inventory-parity`. @@ -147,6 +155,17 @@ def go_accessor_names(names) names.reject { |n| GO_FOLDED.include?(n) }.map { |n| n == "timesheets" ? "timesheet" : n } end +# Give EVERY source the same roster, each spelled by its own generator's rules. +# Used by cases that add or remove a service across the board rather than +# creating a disagreement between SDKs. +def every_source(list) + { + "typescript" => list, "ruby" => list, "python" => list, "kotlin" => list, + "swift" => list, "kotlin-accessors" => list, "swift-accessors" => list, + "python-modules" => list, "go-accessors" => go_accessor_names(list), + } +end + def build_root(dir, names) ts = names.fetch("typescript", CANONICAL) rb = names.fetch("ruby", CANONICAL) @@ -368,6 +387,30 @@ def expect_fail(failures, label, out, status, fragment) expect_fail(failures, "12. Python barrel omitting a service the other seven emit", out, status, "but NOT by python") +# --- 13. A service whose canonical name legitimately ends in `_service` -------- +# +# Python's generator renames exactly one module. `service_filename` tests +# `snake == "webhooks"`, so a service group whose canonical snake name is +# `notification_service` is emitted as `notification_service.py`, unchanged. A +# gate that stripped `_service` unconditionally would read that back as +# `notification`, report a Python service the other seven do not have, and fail +# a build where every generator mapping agreed. +# +# That is a FALSE POSITIVE, which is why this case expects a PASS: the failure +# mode is a red build blocking a correct change, and someone then "fixing" a +# mapping that was never wrong. +# +# The roster still contains `webhooks`, so this single case holds both halves at +# once — the one module that IS renamed, and one that merely looks like it. +# Ruby keeps its unconditional strip and must stay correct here too: it suffixes +# every file, so it emits `notification_service_service.rb`. + +roster = (CANONICAL + ["notification_service"]).sort +raise "case 13 needs `webhooks` in the roster to hold both halves" unless roster.include?("webhooks") +out, status = with_root(names: every_source(roster)) +expect_pass(failures, "13. service whose canonical name ends in `_service` (webhooks still renamed)", + out, status) + # --- Report -------------------------------------------------------------------- puts From b43d7aaed5f09af197aa34a9687247417a09e5de Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sun, 16 Aug 2026 23:16:19 -0700 Subject: [PATCH 4/4] Say what disagreed and where to look; stop naming a cause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four Copilot findings, all correct, and one class: the gate was precise about what it MEASURES and loose about what it CLAIMS. Three of the four arrived as SUPPRESSED comments inside the review body, which never become threads and so never move the unresolved count. Two sweeps read clean past them. The fourth appeared only in the newest review, since the block regenerates each time. The per-failure message asserted "the split tables disagree about whether this service exists". Self-test case 3 is a standing counterexample: it constructs a Kotlin service file with no accessor beside it, where every split table agrees and a generator disagrees with ITSELF. And go-accessors is hand-written, so a Go-only absence is missing wiring, not a mapping difference. It now reports which renderings lack the service and where to look, and asserts nothing about why. The failure footer had the same defect at summary level: it printed for EVERY failure class — missing input, duplicate extraction, collapsed extraction, stale carve-out — and told all of them a tag mapping had drifted. Five of this suite's own cases would have sent a maintainer to the wrong file. Now neutral. sync-doc-constants.rb said this gate compares "the GENERATED service inventories of all six SDKs". Go has no generated per-service files, so that was wrong in the one place the argument turns on it: "not a sixth hand-copy, because it reads generator OUTPUT" is weaker if a rendering is hand-written. Seven of eight are generated; Go is the exception, which is exactly why it carries three carve-outs and now the type check below. The same over-claim is corrected in the script header and SPEC. Its earlier paragraph also still said "no gate compares them to each other", which this PR falsifies; it now points at the gate without weakening the argument for not writing a sixth copy. Separately, an open thread: the Go rendering recorded only accessor NAMES, so `Gauges() *ReportsService` contributed `gauges` and the gate reported parity while Go handed callers the wrong service. Both types exist, so the compiler is silent, and Go's accessors ARE its inventory here — nothing else would catch it. Now checked as a pair. All 51 satisfy name == type stem today. A backreference in the pattern would have made a mismatch fail to EXTRACT, reported as "go-accessors is missing gauges" — true, and a bad description of a line spelling `gauges` right there. Three new cases, all red-proven against the pre-fix checker: 14. Go accessor returning another service type -> pre-fix: PASSED, "53 services agree across 8 renderings", the miss exactly 15. stale carve-out failure must not say "tag mapping" -> pre-fix: "failed correctly but MISDIAGNOSED" 3. now also asserts the parity message does NOT blame the split tables 15 and 3 use a new `expect_fail_without`: the only way to hold a diagnostic honest is to assert what it must NOT say. Kill matrix re-measured — the type check pins 14 alone, the footer pins 15 alone, the parity message pins 3 alone. --- SPEC.md | 2 +- scripts/check-service-inventory-parity | 71 +++++++++++++-- scripts/sync-doc-constants.rb | 31 +++++-- .../test-check-service-inventory-parity.rb | 91 +++++++++++++++++-- 4 files changed, 169 insertions(+), 26 deletions(-) diff --git a/SPEC.md b/SPEC.md index 39a5824d7..bcdde174f 100644 --- a/SPEC.md +++ b/SPEC.md @@ -307,7 +307,7 @@ That roster is the canonical surface, not a per-SDK inventory. Accessor counts v ### Derivation Rule `[static]` -The OpenAPI spec groups operations under coarse tags (e.g., `Automation`, `Todos`, `Files`). The service generators split those tags into the `53` fine-grained services above using a two-table mapping: `TAG_TO_SERVICE` (tag → default service name) and `SERVICE_SPLITS` (tag → {service → [operationIds]}). For example, the `Todos` tag splits into `Todos`, `Todolists`, `Todosets`, `TodolistGroups`, `HillCharts`; the `Files` tag splits into `Attachments`, `Uploads`, `Vaults`, `Documents`, `CloudFiles`, `GoogleDocuments`. Both examples are exhaustive on purpose: an abridged one is how `cloudFiles` and `googleDocuments` stayed invisible to this section for so long — a service that arrives through a split rather than a tag of its own is named nowhere a reader would look. These mappings are defined in each language's generator script. They are five hand-maintained copies of one table, and `make check-service-inventory-parity` compares what those copies **emitted** — the TypeScript, Ruby, Kotlin and Swift generated service directories, Python's generated `__init__.py` barrel, the two generated accessor files this section's roster is derived from, and Go's hand-written accessors — so identical service sets are enforced rather than merely expected. It reads generator output, not the mappings, which is what keeps it from being a sixth copy. (Python is read from its barrel because its generator, alone among the five, does not delete outputs a mapping stopped producing; the barrel is rewritten whole every run and so cannot name a service that is no longer emitted.) Each per-SDK `check-*-service-drift` script remains the freshness gate for its own SDK; none of them can see another SDK, which is the axis this one adds. Go's three divergences (it folds `automation` and `clientVisibility` into other services and spells `timesheets` singular) are stated as data in that gate and fail it if they ever stop applying; Appendix F records them. +The OpenAPI spec groups operations under coarse tags (e.g., `Automation`, `Todos`, `Files`). The service generators split those tags into the `53` fine-grained services above using a two-table mapping: `TAG_TO_SERVICE` (tag → default service name) and `SERVICE_SPLITS` (tag → {service → [operationIds]}). For example, the `Todos` tag splits into `Todos`, `Todolists`, `Todosets`, `TodolistGroups`, `HillCharts`; the `Files` tag splits into `Attachments`, `Uploads`, `Vaults`, `Documents`, `CloudFiles`, `GoogleDocuments`. Both examples are exhaustive on purpose: an abridged one is how `cloudFiles` and `googleDocuments` stayed invisible to this section for so long — a service that arrives through a split rather than a tag of its own is named nowhere a reader would look. These mappings are defined in each language's generator script. They are five hand-maintained copies of one table, and `make check-service-inventory-parity` compares what those copies **emitted** — the TypeScript, Ruby, Kotlin and Swift generated service directories, Python's generated `__init__.py` barrel, the two generated accessor files this section's roster is derived from, and Go's hand-written accessors — so identical service sets are enforced rather than merely expected. It reads what each generator already emitted rather than reimplementing the mappings, which is what keeps it from being a sixth copy — with Go the one exception, having no generated per-service files, so its hand-written accessors are compared against the others' generated output and carry the carve-outs noted below. (Python is read from its barrel because its generator, alone among the five, does not delete outputs a mapping stopped producing; the barrel is rewritten whole every run and so cannot name a service that is no longer emitted.) Each per-SDK `check-*-service-drift` script remains the freshness gate for its own SDK; none of them can see another SDK, which is the axis this one adds. Go's three divergences (it folds `automation` and `clientVisibility` into other services and spells `timesheets` singular) are stated as data in that gate and fail it if they ever stop applying; Appendix F records them. ### Merge-Safe Write Surface (Cards) diff --git a/scripts/check-service-inventory-parity b/scripts/check-service-inventory-parity index 5985a2f47..3b80e7eac 100755 --- a/scripts/check-service-inventory-parity +++ b/scripts/check-service-inventory-parity @@ -24,10 +24,19 @@ # scripts remain the freshness checks and this one does not replace them. # # THIS IS NOT A SIXTH HAND-COPY, which is the thing #745 rejected. It does not -# reimplement the mapping from openapi.json; it reads what the generators -# ALREADY EMITTED and asserts the emissions agree. The inputs are committed -# generated artefacts, so this needs no toolchain and lives in the spec-gates CI -# job beside kt-check-drift. +# reimplement the mapping from openapi.json; it reads what each generator ALREADY +# EMITTED and asserts the emissions agree. The inputs are committed artefacts, so +# this needs no toolchain and lives in the spec-gates CI job beside +# kt-check-drift. +# +# GO IS THE ONE EXCEPTION, and saying "it reads generator output" flatly is an +# over-claim worth not making. Go has no generated per-service files, so its +# rendering here is the hand-written AccountClient accessors in +# go/pkg/basecamp/client.go — a hand-written surface compared against seven +# generated ones. That is not a flaw in the comparison (Go's accessors are still +# an independent statement of which services exist) but it is why Go, alone, +# needs three recorded carve-outs, and why it gets the accessor-vs-returned-type +# check that a generated file would not need. # # WHAT IT CLOSES. `make doc-constants-check` derives SPEC §5's account-scoped # roster from two files: the generated Kotlin and Swift accessors. Those two are @@ -184,10 +193,24 @@ SOURCES = [ id: "go-accessors", # Go has no generated per-service files — the hand-written wrappers in # go/pkg/basecamp ARE its service layer (Appendix F), so its accessors on - # AccountClient are its only rendering of the split. + # AccountClient are its only rendering of the split. It is also the ONLY + # hand-written rendering here, which is why it gets the extra check below + # that no generated source needs. path: "go/pkg/basecamp/client.go", kind: :file, pattern: /^func \(ac \*AccountClient\) (\w+)\(\) \*\w+Service \{/, + # The accessor name and its returned type are two independently typed tokens + # in a hand-written file, and `Gauges() *ReportsService` compiles: both types + # exist. Reading only the name, this gate would record `gauges` and report + # parity while Go handed callers the wrong service. Nothing else would catch + # it — Go's "generated inventory" is this file. + # + # A backreference in `pattern` would make a mismatched accessor simply fail + # to extract, which surfaces as "go-accessors is missing gauges" — true, and + # a bad description of a line that is right there spelling `gauges`. So the + # pair is checked separately and reported for what it is. All 51 accessors + # satisfy name == type stem today. + pair_pattern: /^func \(ac \*AccountClient\) (\w+)\(\) \*(\w+)Service \{/, spelling: :camel, carve_outs: :go, }, @@ -229,9 +252,17 @@ def die(failures) warn "" failures.each { |f| warn " - #{f}" } warn "" - warn " The service split is one mapping transcribed into five generator configs." - warn " This gate compares what they EMITTED. A name that appears in some SDKs and" - warn " not others means a tag mapping was updated in some tables and not the rest." + # NEUTRAL ON PURPOSE. This footer prints for EVERY failure class — a missing + # input, a duplicate extraction, a collapsed extraction, a stale Go carve-out, + # a Go accessor returning the wrong type — and most of those have nothing to do + # with the split tables. It used to assert that a tag mapping had drifted, + # which was a confident diagnosis of a cause this gate cannot observe, and it + # would have sent maintainers to the wrong file for five of its own self-test + # cases. Each failure above names its own area; this says only what the gate + # measured. + warn " This gate compares the service inventories the six SDKs emit against each" + warn " other. It can report WHICH renderings disagree; it cannot see WHY, so read" + warn " the specific failures above for the area to look at." exit 1 end @@ -324,6 +355,18 @@ SOURCES.each do |source| names = extract(source, path) + # An accessor whose declared name and returned type disagree. Only Go declares + # both, and only Go is hand-written; reported here rather than folded into the + # parity diff, which would describe it as an absence instead of a mismatch. + if (pair = source[:pair_pattern]) + File.read(path, encoding: UTF8).scan(pair).each do |accessor, stem| + next if accessor == stem + failures << "#{source[:path]} declares `#{accessor}()` returning `*#{stem}Service` — " \ + "the accessor name and its service type disagree, so callers of " \ + "`#{accessor}()` get #{stem}. Both types exist, so the compiler is silent." + end + end + # DUPLICATES FIRST, before any set comparison sees them. A name emitted twice # is invisible downstream — Array#- drops every occurrence, so a doubled entry # leaves both diffs empty and the gate passes while enumerating something the @@ -359,8 +402,16 @@ union.each do |name| next if absent.empty? present = inventories.keys - absent - failures << "`#{name}` is emitted by #{present.join(', ')} but NOT by #{absent.join(', ')} — " \ - "the split tables disagree about whether this service exists." + # Reports the disagreement and where to look, and stops there. "The split + # tables disagree" was wrong for at least two reachable cases: a generator that + # emits a service file but no accessor beside it leaves every split table + # agreeing (self-test case 3 constructs exactly that), and `go-accessors` is + # hand-written, so a Go-only absence is missing wiring rather than a mapping + # difference. Naming a cause the gate cannot observe is how a green-or-red + # signal turns into a wrong instruction. + failures << "`#{name}` is emitted by #{present.join(', ')} but NOT by #{absent.join(', ')}. " \ + "Look at the renderings that lack it — a split table, a generated accessor " \ + "beside its own services directory, or hand-written wiring for go-accessors." end die(failures) unless failures.empty? diff --git a/scripts/sync-doc-constants.rb b/scripts/sync-doc-constants.rb index 162e073ce..90ead9325 100755 --- a/scripts/sync-doc-constants.rb +++ b/scripts/sync-doc-constants.rb @@ -353,10 +353,14 @@ def tracked_fixtures # openapi.json plus the generators' TAG_TO_SERVICE / SERVICE_SPLITS tables is the # true root, and is deliberately NOT the source here: reproducing that split in # Ruby means a SIXTH hand-copy of those tables. Five already exist, one per -# language, and no gate compares them to each other — so the derivation would -# itself be the drift surface this check exists to remove. These two files are -# regenerated FROM that root and committed, which is close enough to it to be -# checkable and far enough from it to need no second implementation. +# language — so the derivation would itself be the drift surface this check +# exists to remove. These two files are regenerated FROM that root and committed, +# which is close enough to it to be checkable and far enough from it to need no +# second implementation. +# +# (Those five copies ARE now compared to each other, by +# scripts/check-service-inventory-parity — see the note further down. That does +# not change the argument here, which is about not writing a sixth.) # # They are also the only two artefacts that encode ACCOUNT-SCOPING directly # rather than leaving it to be inferred: their entire reason to exist is hanging @@ -398,10 +402,21 @@ def read_accessors(relative, pattern) # them agreeing, and this gate certifies the old roster and the old count. # # That residue is real and is closed ELSEWHERE, not here: -# scripts/check-service-inventory-parity compares the GENERATED service -# inventories of all six SDKs — which is not the sixth hand-copy rejected above, -# since it reads generator OUTPUT rather than reimplementing the mapping. It also -# checks these two accessor files against their own generated services +# scripts/check-service-inventory-parity compares the service inventories of all +# six SDKs — which is not the sixth hand-copy rejected above, since it reads what +# the generators already emitted rather than reimplementing the mapping. +# +# SEVEN of its eight renderings are generated output — five service inventories +# plus these two accessor files. GO IS THE EXCEPTION and the argument above does +# not cover it: Go has no generated per-service files, so that gate reads its +# hand-written AccountClient accessors, a hand-written surface compared against +# generated ones. That is precisely why Go needs three recorded carve-outs there +# (two folded services and a singular spelling) where no generated rendering +# needs any, and why that gate additionally checks each Go accessor against the +# service type it returns — a check meaningless for a generated file and +# necessary for a hand-written one. +# +# It also checks these two accessor files against their own generated services # directories, so a Kotlin or Swift generator that emitted a service file but no # accessor cannot leave this gate certifying a short roster. Codex raised the # residue on #745; it landed as its own change (#747) because the normalization diff --git a/scripts/test-check-service-inventory-parity.rb b/scripts/test-check-service-inventory-parity.rb index c18544ffb..36194b89e 100644 --- a/scripts/test-check-service-inventory-parity.rb +++ b/scripts/test-check-service-inventory-parity.rb @@ -42,14 +42,28 @@ # per-source duplicate check 4 # extraction floor 5 # source-exists check 6 -# fold carve-out staleness check 7 +# fold carve-out staleness check 7, 15 # spelling carve-out staleness check 8 # spelling carve-out both-spellings check 10 # reading Python from its BARREL 11, 12 # Python's rename respelled as a SUFFIX STRIP 13 +# Go accessor-vs-returned-type check 14 +# footer that asserts a CAUSE 15 +# parity message that asserts a CAUSE 3 # APPLYING the fold carve-out positive controls, 11, 13 # APPLYING the spelling carve-out positive controls, 11, 13 -# Python's rename rule deleted, or never applied positive controls, 11, 13 +# Python's rename deleted, or never applied positive controls, 11, 13 +# +# THREE OF THESE PIN A MESSAGE RATHER THAN A VERDICT, which is unusual and +# deliberate. A gate that fails for the right reason while naming a cause it +# cannot observe still sends the reader to the wrong file, and that had happened +# in two places here: the footer told every failure class that a tag mapping had +# drifted, and the parity message asserted "the split tables disagree" even for +# case 3, where every split table agrees and a generator disagrees with itself. +# Both are pinned as ABSENCES (`expect_fail_without`), because the only way to +# hold a diagnostic honest is to assert what it must NOT say. Case 15 also goes +# red if the carve-out staleness check itself is removed — it needs that failure +# to exist before it can inspect how it is described. # # THE PASS-SHAPED CASES, and why the bottom rows look blunt. 11 and 13 both assert # a PASS, so they go red for ANY breakage that stops the checker passing at all — @@ -77,9 +91,11 @@ # is the one that anchors SPEC §5 — its roster is derived from the two accessor # files, and nothing else checks those against their own services directories. # -# WHAT THIS SUITE IS NOT. The suite passing means these thirteen things are -# checked. It has never meant the list is complete — 13 exists because a reviewer -# found a false positive the other twelve could not see. +# WHAT THIS SUITE IS NOT. The suite passing means these fifteen things are +# checked. It has never meant the list is complete — 13, 14 and 15 all exist +# because a reviewer found something the earlier cases could not see, and 14 in +# particular was a straightforward miss: the gate read Go's accessor names and +# never looked at what they returned. # # Run directly (`ruby scripts/test-check-service-inventory-parity.rb`) or via # `make test-check-service-inventory-parity`. @@ -249,6 +265,25 @@ def expect_fail(failures, label, out, status, fragment) end end +# Fails, AND the message must NOT contain `forbidden`. For diagnostics: a gate +# that fails for the right reason while telling the reader the wrong place to +# look is still a defect, and only an absence assertion can pin that. +def expect_fail_without(failures, label, out, status, fragment, forbidden) + if status.success? + puts " FAIL #{label}" + failures << "#{label}: expected FAILURE but checker passed:\n#{out}" + elsif !out.include?(fragment) + puts " FAIL #{label}" + failures << "#{label}: failed as expected but message missing #{fragment.inspect}:\n#{out}" + elsif out.include?(forbidden) + puts " FAIL #{label}" + failures << "#{label}: failed correctly but MISDIAGNOSED — output contains " \ + "#{forbidden.inspect}, which names a cause this failure does not have:\n#{out}" + else + puts " PASS #{label}" + end +end + puts "==> service inventory parity self-test (checker: #{CHECKER.sub("#{ROOT}/", '')})" # --- Positive controls --------------------------------------------------------- @@ -289,10 +324,19 @@ def expect_fail(failures, label, out, status, fragment) # Kotlin emits the service file but no accessor. SPEC §5's roster reads the # ACCESSORS, so the roster comes up short while every services directory agrees # the service exists — and nothing but this checked the two against each other. +# +# It is also the standing counterexample to blaming the split tables: EVERY split +# table agrees here, and the disagreement is between a generator's own two +# outputs. So the message must report which renderings disagree without asserting +# that a mapping caused it — pinned as an absence, because a gate that fails for +# the right reason while naming the wrong cause is still sending someone to the +# wrong file. out, status = with_root(names: { "kotlin-accessors" => CANONICAL - ["wormholes"] }) -expect_fail(failures, "3. Kotlin service file with no accessor (anchors SPEC section 5)", out, status, - "but NOT by kotlin-accessors") +expect_fail_without(failures, "3. Kotlin service file with no accessor (anchors SPEC section 5)", + out, status, + "but NOT by kotlin-accessors", + "split tables disagree") # --- 4. A name emitted twice --------------------------------------------------- # @@ -411,6 +455,39 @@ def expect_fail(failures, label, out, status, fragment) expect_pass(failures, "13. service whose canonical name ends in `_service` (webhooks still renamed)", out, status) +# --- 14. A Go accessor returning the wrong service type ------------------------ +# +# `Gauges() *ReportsService` compiles — both types exist — and reading only the +# accessor name records `gauges`, so every inventory agrees and the gate reports +# parity while Go hands callers the wrong service. Go's accessors ARE its +# generated inventory here, so nothing else would catch it. + +out, status = with_root do |dir| + path = File.join(dir, GO_CLIENT) + body = read_utf8(path) + line = "func (ac *AccountClient) Gauges() *GaugesService {" + raise "no Gauges accessor in the synthetic Go client" unless body.include?(line) + File.write(path, body.sub(line, "func (ac *AccountClient) Gauges() *ReportsService {")) +end +expect_fail(failures, "14. Go accessor returning a different service type", out, status, + "declares `Gauges()` returning `*ReportsService`") + +# --- 15. A non-mapping failure must not blame the split tables ------------------ +# +# The failure footer prints for EVERY failure class, and used to assert that a +# tag mapping had drifted. For a stale Go carve-out — case 7's mutation — that is +# a confident diagnosis of a cause the gate cannot observe, and it sends the +# reader to five generator configs when the thing to edit is GO_CARVE_OUTS. +# +# Pinned as an ABSENCE, which is the only way to hold a diagnostic honest: the +# run must fail, name the carve-out, and NOT say a tag mapping drifted. + +out, status = with_root(names: { "go-accessors" => go_accessor_names(CANONICAL) + ["automation"] }) +expect_fail_without(failures, "15. stale carve-out failure does not misdiagnose a tag mapping", + out, status, + "recorded as a fold carve-out", + "tag mapping") + # --- Report -------------------------------------------------------------------- puts