Find operations by what they are not, so no HTTP verb can drop one in silence by jeremy · Pull Request #931 · basecamp/basecamp-sdk

jeremy · 2026-09-18T19:36:20Z

The hole

Every per-language generator found operations by iterating its own hard-coded list of five HTTP verbs — get, post, put, patch, delete. Smithy's @http trait takes method as a free-form string that Smithy "will use literally and will perform no validation on", so a model author writing method: "HEAD" in spec/basecamp.smithy got a valid model, a valid openapi.json, and no method on any client, in six languages, with no error.

Measured rather than asserted. Taking the committed openapi.json, renaming GetProject's verb from get to head, and running the pre-change generators:

	operations generated	`GetProject` present
Ruby, unmodified spec	265	yes
Ruby, HEAD spec	264	no
Python, unmodified spec	265	yes
Python, HEAD spec	264	no

One operation gone from the SDK, and a clean exit.

The instrument, per #922

Not a longer list. #922 fixed the same shape in scripts/check-required-tags.rb by identifying an operation by what it is not, and the same inversion lands here: a Path Item Object's non-operation fields are a closed, spec-defined set ($ref, summary, description, servers, parameters) plus x- extensions, and every other field is an operation. A field that is neither a known non-operation field nor a readable operation object is named, not stepped over. A $ref path item is refused rather than read as empty, since its operations cannot be counted without resolving it.

The verb list has already fallen behind twice — OpenAPI 3.0 added trace, 3.2 adds query and additionalOperations — and @http.method means this repo's own generator can put a key in a path item that no list anticipates.

Loud refusal, not new emission — and the reason this matters beyond this PR

Chosen: the generators refuse an operation on a verb they cannot emit. They were deliberately not taught to emit new verbs.

Emitting HEAD/OPTIONS/TRACE would mean inventing transport helpers in six runtimes for a verb the Basecamp API does not serve — API surface nobody asked for, six times. Refusal costs nothing and is fail-closed: the operation cannot ship missing, because nothing ships until someone decides.

The consequence, stated because a follow-up depends on it: scripts/gen-catalog/main.go bounds the catalog to the generated verb set and fails the build on anything else. That gate stays correct and stays needed after this PR. It would have inverted into a bug only if the generators had started emitting those verbs, which they do not. Its logic is untouched here.

One declaration, not six literals

The five verbs were restated independently in each generator, in scripts/generate-url-routes, and in gen-catalog. That copying is the mechanism, not just the symptom, so it is gone:

spec/generated-verbs.json is the single declaration, read by all six service generators. It is a policy, not six per-language capabilities: check-operation-assignment-parity already requires all six SDKs to expose the same operation set, so six independently-maintained bounds could never legitimately differ — it was one fact written six times. The array is ordered, and that order is emission order, so there is no second per-generator list for ordering either.

Resolution mirrors what each generator already does for openapi.json: a path relative to the script (Ruby, Python, TypeScript) or a --verbs flag defaulting beside --openapi (Kotlin, Swift, Rust). gen-catalog is peer-owned; its comment now points at the declaration, and deriving from it is theirs to take.

The file's own header carries the reasoning, including the part a declaration cannot enforce: adding an entry does not implement it. Six runtimes need the transport helper first.

What each generator does now

Generator	Discovery	Emission bound
`ruby/scripts/generate-services.rb`	inverted	declaration; refuses by name
`python/scripts/generate_services.py`	inverted (`gen_common.iter_operations`)	declaration; refuses by name
`typescript/scripts/generate-services.ts`	inverted (`scripts/path-items.ts`)	declaration; refuses by name
`kotlin/…/OperationParser.kt`	inverted (`PathItems.kt`)	declaration; refuses by name
`swift/…/OpenAPIParser.swift`	inverted (`PathItems.swift`)	declaration; refuses by name
`rust/generator/src/model.rs`	inverted	declaration; refuses by name
`ruby/scripts/generate-metadata.rb`	inverted	none — verb-agnostic
`python/scripts/generate_metadata.py`	inverted	none — verb-agnostic
`typescript/scripts/extract-metadata.ts`	inverted	none — verb-agnostic
`typescript/scripts/extract-person-id-sites.ts`	inverted	none — verb-agnostic
`scripts/generate-url-routes` (jq)	inverted	none — verb-agnostic
`scripts/check-operation-assignment-parity`	inverted	n/a — the anchor must stay total

The four metadata extractors and the route generator key everything on operationId and represent a verb as data, so they take no bound and now cover an operation on any verb. Bounding them would have been fail-closed theatre: they can genuinely handle it.

Go is the one generator with nothing to invert. Its client comes from oapi-codegen; there is no repo-owned path-item walk. Its route table is scripts/generate-url-routes, which is in the table above.

Mutation check

scripts/test-generator-verb-inversion.rb (new, wired into make check and the Spec Gates job) drives Ruby, Python and the jq route generator with crafted specs — HEAD, 3.2's query, a non-operation field, an unreadable field, a $ref path item, a declaration missing delete. 21 cases, all passing.

The numbers are the argument:

walk restored to	cases failing (of 21)
the original five-verb list	10
the eight verbs OpenAPI 3.1 names	3

The eight-verb list repairs HEAD and still fails on 3.2's query and on the two cases where a field that is neither known nor readable is stepped over in silence. That gap is why this is an inversion rather than a longer list, and why it is a test rather than a paragraph.

Case 9 and 17 are worth naming separately: they point a generator at a crafted declaration lacking delete and assert DELETE is then refused. That is what proves the bound is read from spec/generated-verbs.json rather than from a private literal that merely happens to match.

The compiled generators carry the identical walk; the test does not drive them (they need a JVM, a Swift toolchain, cargo or an npm install, and their drift checks run in their own CI jobs). The refusal was reproduced by hand in all four against the same HEAD spec:

TypeScript  exit 1  Error: openapi.json declares HEAD /{accountId}/projects/{projectId} (GetProject), and this generator emits only GET/POST/PUT/PATCH/DELETE. …
Kotlin      exit 1  (IllegalStateException, same text)
Swift       exit 1  (same text)
Rust        exit 1  (same text)

No partial output was written in any of them.

Regeneration

Byte-identical in all seven SDKs. openapi.json declares four verbs and no non-operation path-item fields, so the inversion sees exactly what the lists saw. The ordering property is what makes that true and is pinned by case 8: the declaration's order is emission order, so a path item spelled {delete, get} still emits get first.

Verified green locally: the six per-language drift checks, go-check-generated-drift, go-check-wrapper-drift, check-operation-assignment-parity, check-service-inventory-parity, check-grouped-client-coverage, url-routes-check, catalog-check, smithy-check, bc3-route-parity, and every self-test alongside them.

Two harness fixes were needed, both because the harness stands in for the repository layout the generators resolve against:

scripts/check-typescript-service-drift.sh assembles a throwaway project outside the repo; it now copies spec/generated-verbs.json to where scripts/path-items.ts resolves it, exactly as it already did for openapi.json.
rust/generator/tests/fixtures.rs passes --verbs pointing at the repository's own declaration rather than a per-fixture copy — copying it into each fixture root would reintroduce, in the tests, the duplicated literal this removes.

The tracking claims, removed in this PR because it closes #925

Two canonical tracked in #925 claims and both spec/tracking-issues.yml rows are gone. Leaving either would have pointed the Known-defect tracking issues are open gate at a closed issue the moment this merged — red on main and, because it evaluates on the merge commit, on every open PR at once.

scripts/check-required-tags.rb — the claim described the hole this PR closes, so the paragraph is rewritten as an as-of fact: the generators now walk by the same exclusion this gate uses. The explanation survives; only the promise is gone.
scripts/gen-catalog/main.go — comment text only, no logic change. The consolidation half of that claim is obsolete (there is a shared source now, and taking it belongs to that generator's owner). The bound half is not obsolete and is not deleted: this PR made the generators refuse rather than emit, so a non-generated verb still reaches no client, and gen-catalog reads openapi.json directly rather than the SDK trees, so it holds that line on its own input even when nothing has been regenerated.

make check-known-defect-issues-open exits 0 locally, and rg 'tracked in #925' returns nothing.

What this does not do

It does not teach any SDK to serve HEAD/OPTIONS/TRACE/query. Those refuse, loudly.
It does not resolve $ref path items anywhere. Every walk refuses one by name; resolving local references is the better end state and no walk under-counts while waiting for it.
OpenAPI 3.2's additionalOperations is a map of method to Operation. Every walk here reads it as a single operation and refuses it by name. The artifact declares 3.1.0, so it is unreachable; the failure is loud and the remedy is local to each walker.
spec/generated-verbs.json declares what the generators emit; nothing mechanically verifies each runtime actually has the helper. Adding a verb there without implementing it is a compile error in TypeScript, Kotlin, Swift and Rust, and a missing-method error at call time in Ruby and Python. The file's header says to add the implementations first; that instruction is the whole of the guard, and it is stated rather than assumed.
scripts/check-projected-examples.rb, scripts/check-kotlin-optional-arrays-and-scalars.rb and scripts/sync-doc-constants.rb still spell the eight-verb list. They are checks, not generators — none of them can drop an operation from an SDK — and they were out of Generators iterate a five-verb list, so an operation on any other HTTP method is silently dropped from every SDK #925's scope. Converting them is a small, separate change.

Closes #925.

Summary by cubic

Fixes operations being silently dropped from generated SDKs when the model uses an HTTP verb outside the previous five-verb list. Generators now find every operation in a path item and refuse, by name, any operation they cannot emit.

What changed

Path items are read by exclusion: non-operation fields and x- extensions are skipped; every other field is treated as an operation.
spec/generated-verbs.json is now the single ordered declaration of which verbs the SDKs emit.
Metadata extractors and the URL route generator are verb-agnostic and now cover operations on any verb.
$ref path items stop generation rather than being silently ignored.

Side effects

Generated SDK output is byte-identical.
No SDK gains new verb support: HEAD, OPTIONS, TRACE, and query fail loudly.
Adding a verb to spec/generated-verbs.json requires the corresponding transport helper in all six runtimes first.

Closes #925.

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

@http

Every per-language generator found operations by iterating its own hard-coded list of five HTTP verbs. Smithy's @http trait takes the method as a free-form string it will use literally and perform no validation on, so method: "HEAD" in the model produced a valid openapi.json and no method on any client, in six languages, with no error. Each generator now reads a Path Item Object by exclusion — its non-operation fields are a closed, spec-defined set and its extensions are x- prefixed, so every other field is an operation — and refuses by name what it cannot emit. Discovery is total; emission is bounded by spec/generated-verbs.json, one declaration all six read rather than six literals that could only ever agree. The metadata extractors and the route generator take no bound at all: they key everything on operationId, so they are verb-agnostic and now cover an operation on any verb. Closes #925.

Both stand in for the repository layout the generators resolve against: the TypeScript drift check assembles a throwaway project outside the repo, and the Rust fixture roots are self-contained. Copy the file into the one, pass --verbs to the other — a per-fixture copy would put the duplicated literal back into the tests.

github-actions · 2026-09-18T19:36:31Z

Sensitive Change Detection (shadow mode)

This PR modifies control-plane files:

.github/workflows/test.yml

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

chatgpt-codex-connector · 2026-09-18T19:36:34Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-18T19:40:13.351173Z	`a7f1229`	Manual request

ℹ️ About Codex in GitHub

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

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

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

jeremy · 2026-09-18T19:36:43Z

@codex review

chatgpt-codex-connector · 2026-09-18T19:40:12Z

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: a7f1229db3

ℹ️ About Codex in GitHub

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

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

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

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

Copilot

🟡 Changes recommended

The shared policy permits broken PATCH emission, and several fail-closed paths remain incomplete.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Closes #925 by replacing fixed HTTP-verb discovery with fail-closed OpenAPI path-item traversal across SDK generators.

Changes:

Centralizes supported verbs in spec/generated-verbs.json.
Updates generators, metadata extractors, route generation, and parity checks.
Adds mutation coverage and CI wiring.

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

File summaries

File	Description
typescript/scripts/path-items.ts	Adds shared TypeScript path-item traversal.
typescript/scripts/generate-services.ts	Uses bounded fail-closed operation discovery.
typescript/scripts/extract-person-id-sites.ts	Makes person-ID extraction verb-agnostic.
typescript/scripts/extract-metadata.ts	Makes metadata extraction verb-agnostic.
swift/Sources/BasecampGenerator/PathItems.swift	Adds Swift path-item traversal and verb loading.
swift/Sources/BasecampGenerator/OpenAPIParser.swift	Uses fail-closed Swift discovery.
swift/Sources/BasecampGenerator/main.swift	Adds the Swift verb-policy argument.
spec/tracking-issues.yml	Removes resolved #925 tracking entries.
spec/generated-verbs.json	Defines the shared ordered verb policy.
scripts/test-generator-verb-inversion.rb	Adds cross-generator mutation tests.
scripts/test-check-operation-assignment-parity.rb	Updates parity-test operation discovery.
scripts/generate-url-routes	Makes route discovery verb-agnostic.
scripts/gen-catalog/main.go	Updates documentation for the remaining catalog bound.
scripts/check-typescript-service-drift.sh	Copies the shared policy into the test layout.
scripts/check-required-tags.rb	Documents the resolved generator gap.
scripts/check-operation-assignment-parity	Makes the parity anchor fail closed.
rust/generator/tests/fixtures.rs	Supplies the shared verb policy to fixtures.
rust/generator/src/model.rs	Adds bounded Rust operation discovery.
rust/generator/src/main.rs	Loads the Rust verb-policy input.
ruby/scripts/generate-services.rb	Adds fail-closed Ruby service discovery.
ruby/scripts/generate-metadata.rb	Makes Ruby metadata traversal verb-agnostic.
python/scripts/generate_services.py	Uses shared Python traversal and policy.
python/scripts/generate_metadata.py	Makes Python metadata traversal verb-agnostic.
python/scripts/gen_common.py	Adds shared Python path-item utilities.
Makefile	Registers the mutation test in checks.
kotlin/generator/src/test/kotlin/com/basecamp/sdk/generator/PaginationStyleTest.kt	Updates parser construction for the new policy.
kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/PathItems.kt	Adds Kotlin traversal and verb loading.
kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/OperationParser.kt	Uses bounded Kotlin operation discovery.
kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/Main.kt	Supplies the Kotlin verb policy.
.github/workflows/test.yml	Runs the mutation test in Spec Gates.

Review details

Files reviewed: 30/30 changed files
Comments generated: 8
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    val parser = OperationParser(api, PathItems.generatedVerbs(verbsPath))
    val services = parser.groupOperations()


+    "The array is ORDERED, and the order is emission order. Keeping order here rather than in a second per-generator list means there is exactly one place the verb set is written."
+  ],
+  "adding_a_verb": "Adding an entry here does not implement it. Each SDK's runtime needs the transport helper the generated call site names (Basecamp::Http#http_head, BaseService.httpHead, and so on) before the verb belongs in this list, and the operation needs a tag in spec/overlays/tags.smithy. Add the six implementations first, this entry last.",
+  "verbs": ["get", "post", "put", "patch", "delete"]


+# openapi.json's four verbs, so this drives them with crafted specs — HEAD, 3.2's
+# `query`, an unreadable field, a $ref path item — across Ruby, Python and the jq
+# route generator. Stdlib toolchains only, so it belongs in the spec-gates CI job.


+        val verbs = (Json.parseToJsonElement(file.readText()) as JsonObject)["verbs"]
+            ?.jsonArray
+            ?.map { it.jsonPrimitive.content }
+            ?: error("$path must declare a `verbs` array")
+        require(verbs.isNotEmpty() && verbs.none { it.isEmpty() }) {
+            "$path must declare a non-empty `verbs` array of strings"
+        }


+    let emittable_verbs: Vec<String> = verbs["verbs"]
+        .as_array()
+        .map(|verbs| {
+            verbs
+                .iter()
+                .filter_map(|verb| verb.as_str().map(str::to_string))
+                .collect()
+        })
+        .filter(|verbs: &Vec<String>| !verbs.is_empty())


    ($path_item | to_entries | map(
-      select(.key | IN("get","post","put","patch","delete"))
+      select((.key | IN("summary","description","servers","parameters") | not)
+             and (.key | startswith("x-") | not))
    )) as $ops |


@@ -71,8 +78,9 @@ func parseAllOperations(spec: [String: Any]) -> (operations: [ParsedOperation],
        let pathItem = paths[path]!
        guard let pathDict = pathItem as? [String: Any] else { continue }


+  return fields.map((field): [string, T] => {
+    const operation = item[field];
+    if (typeof operation !== "object" || operation === null || Array.isArray(operation)) {
+      die(
+        `Error: openapi.json path ${path} field ${JSON.stringify(field)} is neither a known ` +
+          `non-operation field nor an operation object. If a later OpenAPI version added it, add ` +
+          `it to NON_OPERATION_FIELDS with a reason.`
+      );
+    }
+    if (emittable && !emittable.includes(field)) {


jeremy · 2026-09-18T20:26:42Z

All eight findings from the review were real, and all eight are fixed in #932. This PR merged about nine minutes after opening — roughly forty seconds before the review landed — so none of them could be addressed here.

Per finding:

Kotlin cleared its output before parsing — confirmed and fixed. Main.kt deleted models/*.kt, services/*.kt, Metadata.kt and ServiceAccessors.kt before groupOperations(), so a refusal erased the committed tree. It parses first now, and the new compiled-generator test asserts an untouched output tree (seeded first — an empty directory cannot show data loss, which is why the first version of that test passed against the unfixed generator).
patch is not a shared capability — confirmed on both counts: Ruby's BaseService delegates only get post put delete post_raw put_raw, and Kotlin's ServiceEmitter renders PATCH through httpPut. Removed from spec/generated-verbs.json, with the two required fixes recorded there. No PATCH operation exists, so nothing regenerates differently; one now refuses by name instead of shipping broken.
The compiled generators had no test that could see a regression — correct, and the sharpest of the eight. scripts/test-compiled-generator-refusal runs each real binary against a HEAD spec and asserts a named refusal that writes nothing and destroys nothing; wired into ts-check, kt-check, swift-check, rs-check and each language's CI job.
Kotlin's loader coerced non-strings — fixed; every entry must now be a non-blank JSON string.
Rust's filter_map dropped non-strings — fixed the same way. Together these were a shared declaration that six generators read differently, which is worse than six literals.
The jq filter accepted unreadable fields — fixed; a path-item field must be an operation object with a non-empty operationId, so a scalar no longer errors obliquely and an unrecognised object no longer becomes a route naming null.
Swift still continued past a malformed path item — fixed; the guard is gone and operationsOf validates, so the walk is total.
additionalOperations slipped through the verb-agnostic walkers — the most substantive. The bounded walks refused it as an unemittable verb, but the four metadata extractors and the route generator read it as one operation with no operationId and skipped it, dropping every operation inside. Every walk names it now.

The self-test is up from 21 cases to 28 and make check is green; all seven SDKs still regenerate byte-identical.

11 version files and 8 lockfiles, written by scripts/bump-version.sh. MIGRATING.md's '# Unreleased' section is promoted to '# v0.20.0'. Thirteen commits since v0.19.0 (2026-09-16), the substantial ones being the #925 chain — #931 finding operations by exclusion so no HTTP verb can drop one in silence, #932 stopping a refusal from destroying generated output, and #938 deriving gen-catalog's emission bound from the shared declaration — plus #926's embedded tool Catalog, #930's Automation split, #924's event-feed body validation at the wrapper's decode, and #937's anyio bump for CVE-2026-63374. Ran the release's own preflight here rather than discovering it at tag time: promote-migrating --check passes, rs-publish-check packages and dry-run-uploads 0.20.0 clean, and all four lockfile freshness assertions hold.

jeremy added 2 commits September 18, 2026 12:13

Copilot AI balanced review requested due to automatic review settings September 18, 2026 19:36

Copilot started reviewing on behalf of jeremy September 18, 2026 19:36

View session

jeremy merged commit 6c9cf3e into main Sep 18, 2026
56 checks passed

jeremy deleted the sdk925-verb-inversion branch September 18, 2026 19:45

Copilot AI reviewed Sep 18, 2026

View reviewed changes

This was referenced Sep 18, 2026

Split the Automation catch-all into six real domains #930

Merged

A refusal must not destroy generated output: fixed in Kotlin and TypeScript, swept across the rest #932

Merged

This was referenced Sep 18, 2026

Language-independent policy is transcribed once per language #935

Open

One validator, six readers: stop the generators disagreeing about the shared declaration #933

Closed

Bump to 0.20.0 #940

Merged

Navigation Menu

Repository navigation

Find operations by what they are not, so no HTTP verb can drop one in silence - #931

Find operations by what they are not, so no HTTP verb can drop one in silence#931
jeremy merged 2 commits into
mainfrom
sdk925-verb-inversion

jeremy commented Sep 18, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

github-actions Bot commented Sep 18, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 18, 2026 •

edited

Loading

Uh oh!

jeremy commented Sep 18, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 18, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

jeremy commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		val parser = OperationParser(api, PathItems.generatedVerbs(verbsPath))
		val services = parser.groupOperations()

		@@ -71,8 +78,9 @@ func parseAllOperations(spec: [String: Any]) -> (operations: [ParsedOperation],
		let pathItem = paths[path]!
		guard let pathDict = pathItem as? [String: Any] else { continue }

Conversation

jeremy commented Sep 18, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The hole

The instrument, per #922

Loud refusal, not new emission — and the reason this matters beyond this PR

One declaration, not six literals

What each generator does now

Mutation check

Regeneration

The tracking claims, removed in this PR because it closes #925

What this does not do

Summary by cubic

Uh oh!

github-actions Bot commented Sep 18, 2026

Sensitive Change Detection (shadow mode)

Uh oh!

chatgpt-codex-connector Bot commented Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

jeremy commented Sep 18, 2026

Uh oh!

chatgpt-codex-connector Bot commented Sep 18, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

jeremy commented Sep 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jeremy commented Sep 18, 2026 •

edited by cubic-dev-ai Bot

Loading

chatgpt-codex-connector Bot commented Sep 18, 2026 •

edited

Loading