Repin bc3 to 2c0dafba13 and absorb the Folders API it brings into range - #593
Conversation
The repin and the absorption are one change on purpose. `doc/api/sections/ folders.md` does not exist at the old pin `d0edc1283b`, so the five `/stacks` routes are absent from `spec/bc3-routes.json` today. Repinning alone would make them appear in the route table with no operation modelling them, and the #586 parity gate would then demand a disposition for five routes we have every reason to model. Absorbing alone would model a contract the pin says isn't there. 238 -> 243 operations. ## The repin: d0edc1283b -> 2c0dafba13 (11 commits) Exactly one touches API surface: - `dc6cd10714` #12384, the Folders API — the absorption half, below. Two more are behaviour fixes to endpoints already modelled, with the wire shape unchanged, so they need the repin and nothing else: `344581a379` (#12494) and `2c0dafba13` (#12501) stop a draft's subscribers being recomputed when an update doesn't address them. They add prose to five `doc/api` sections but no route and no payload field — proven by regenerating the route table, whose only delta is the five `/stacks` routes. `19956c5579` (#12488) deserves its own sentence because it *deletes* a `request.format.json?` branch, which reads like an API change. It is not. The removed branch returned `nil` for JSON when the recording's own assignment wasn't prioritized; the replacement returns that same assignment unconditionally and `&.unprioritize` is a no-op when there is no priority. Both spellings are exact-target on the id in the URL and both answer `204`. What actually changed is that the *web* path stopped cascading to a sibling step — it adopted the contract the JSON API has had since bc3#12483 (absorbed in SDK #528). The remaining seven are four dev-tooling (`112c1664bf`, `63a34abe30`, `5f889d8365`, `faf5fe9dfc`), two Turbo-morph web-only (`1fad1ef608`, `c664cdc9de`), and one push-notification backend swap (`845df62b6b`). `spec/bc3-routes.json` is regenerated at the new pin (369 routes, 64 sections); `make provenance-sync` keeps the go:embed copy in step. The API version tracks the pin date, so every SDK's `API_VERSION` moves to 2026-08-02. ## The absorption: FoldersService, five operations Folders group projects on one person's home screen. They are per-user, so the collection is flat rather than bucket-scoped. **The wire type stays `Stack`** — the product was renamed, the payload was not — and so does the URI segment. The operations, structures and generated methods use `Folder`. ListFolders GET /{accountId}/stacks.json 200, bare array GetFolder GET /{accountId}/stacks/{folderId} 200 CreateFolder POST /{accountId}/stacks.json 201 UpdateFolder PUT /{accountId}/stacks/{folderId} 200 DeleteFolder DELETE /{accountId}/stacks/{folderId} 204 Two output structures, not one. `Folder` is what the list returns; only `FolderWithProjects` carries the expanded `projects`. A single shape with an optional member would make every generated list-item type declare a field the list response never populates. `gauges_url`, `color` and `image_url` are **required-and-nullable** (SPEC §"A third wire state"): the jbuilder emits all three unconditionally and all three are commonly `null`, so `@required` models the presence and `smithy-build.json` `jsonAdd` layers `type: ["string","null"]` on top. Go types them `*string` because the value is nullable, not because the key is optional. Same treatment `Wormhole.color`/`destination_url` already get. `project_ids` does not round-trip: the create input takes it, and the response reports the same ids as `bucket_ids` *and* expands them as `projects`. Three names, one relationship. An unreachable id fails the whole request with 404 and writes nothing, so `CreateFolder` lists `NotFoundError`; `UpdateFolder` lists `ValidationError` because a blank name is a real 422 (create defaults it to "New folder" instead). `UpdateFolder` and `DeleteFolder` are naturally idempotent; `CreateFolder` is not, and gets the inert retry block every non-idempotent POST carries. The `Folders` tag resolves to `FoldersService` through every generator's default fallback — zero service-group overrides, and `ListFolders` infers the `folder` resource type cleanly, so no override there either. One deliberate generator addition: `GetFolder` -> `getFolder` in TS/Kotlin/Swift, because the bare rule collapses it to `folder(id)`, a noun with no verb, while Ruby and Python already emit `get_folder`. Same shape as the `GetBookmark` override; all six SDKs now agree on listFolders/getFolder/createFolder/updateFolder/deleteFolder. Go's `FolderWithProjects` repeats the base fields rather than embedding `Folder`. Embedding promotes them correctly at runtime but hides them from `check-wrapper-drift`, which walks declared fields and reported all twelve as missing. Repeating them keeps the guard able to verify every field. ## Registry and coordination `spec/api-gaps/folders-api.md` moves to `absorbed-in-sdk` with seven `smithy_refs`. `spec/api-gaps/stack-doc-and-smithy.md` deliberately keeps its `confirmed-not-api-resource` status: it records a decision that was correct when made, and the supersession note in `folders-api.md` exists precisely to preserve that history. `COORDINATION.md` named pin `338b7a11`, three repins stale; it now names `2c0dafba13` and says the provenance file is the only authority. ## Verified `make check` exit 0. Swift is not skipped on this host: `swift build`/`swift test` ran 306 tests with 0 failures, and `swift run ConformanceRunner` ran the conformance suite. Ruby 1092 runs / 0 failures. Conformance 147 passed, 1 skipped (a pre-existing Go-runner origin case), and the five new `paths.json` cases PASS in **all six** runners — Go, Kotlin, TypeScript, Ruby, Python and Swift each have a dispatch branch, so none of them silently skips. `BC3_REPO_PATH=... make bc3-routes-check` exit 0. Counts updated deliberately: `check-idempotency-parity` 77/199 -> 79/203, SPEC/SECURITY/AGENTS 238 -> 243 operations, 122 -> 124 GETs, 47 -> 48 PUTs, 23 -> 24 DELETEs, 39 -> 40 single-attempt POSTs, 188 -> 192 retry-eligible ops at max 3.
github-actions
Bot
added
typescript
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7548dab83
ℹ️ 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".
There was a problem hiding this comment.
🟡 Human review recommended
It is a broad cross-cutting change (provenance repin plus a new service generated across all six SDKs, conformance, and hand-maintained spec tallies) that warrants human verification, and it already contains one stale op-count in SPEC.md that should be corrected.
Pull request overview
This PR repins bc3 provenance from d0edc1283b → 2c0dafba13 and, as one coupled change, absorbs the Folders API (/stacks routes, wire type Stack) that the repin brings into range. It adds a FoldersService (List/Get/Create/Update/Delete) across all six SDKs — starting from the Smithy spec, cascading through openapi.json and every generated service/model layer, plus conformance dispatch and per-SDK metadata. Operation count moves 238 → 243 and the API version to 2026-08-02.
Changes:
- Adds
ListFolders/GetFolder/CreateFolder/UpdateFolder/DeleteFolderwith two shapes (Folderfor lists,FolderWithProjectsfor singular ops);gauges_url/color/image_urlare required-and-nullable,project_idsis input-only. - Repins provenance + regenerates
bc3-routes.json, and updates the hand-maintained op-count tallies in SPEC.md/SECURITY.md/AGENTS.md,check-idempotency-parity(79/203), and API-version constants in all six SDKs. - Wires the new service into every client, adds hand-written TS/Ruby/Python unit tests and 5 conformance path cases dispatched by all six runners.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
| spec/basecamp.smithy | Adds 5 Folders operations + Folder/FolderWithProjects structures (wire type Stack) |
| spec/overlays/tags.smithy, spec/smithy-build.json | Tags new ops to Folders; layers nullable types on 3 fields |
| openapi.json, typescript/src/generated/{schema.d.ts,openapi-stripped.json,path-mapping.ts,metadata.ts} | Regenerated OpenAPI + TS artifacts |
| typescript/src/generated/services/folders.ts, index.ts, client.ts, scripts/generate-services.ts | New TS service, wiring, naming/type-alias overrides |
| go/pkg/basecamp/folders.go, client.go, url-routes.json | Hand-written Go service over generated client + route table |
| ruby/.../folders_service.rb, types.rb, client.rb, metadata.json | Ruby service, types, wiring |
| python/.../services/folders.py, types.py, client.py, async_client.py, metadata.json | Sync+async Python service, types, wiring |
| kotlin/.../services/folders.kt, models/Folder(WithProjects).kt, Types.kt, Metadata.kt, Config.kt, ServiceAccessors.kt | Kotlin service, models, generator config |
| swift/.../FoldersService.swift, Models/*.swift, Metadata.swift, AccountClient+Services.swift, MethodNaming.swift | Swift service, models, wiring |
| conformance/tests/paths.json + 6 runners | 5 path cases + dispatch branches in all runners |
| SPEC.md, SECURITY.md, AGENTS.md, scripts/check-idempotency-parity | Operation-count/retry/idempotency tally updates |
| spec/api-provenance.json, go/.../api-provenance.json, spec/bc3-routes.json, COORDINATION.md, spec/api-gaps/* | Provenance repin, route table, registry status |
| */version constants (go/ruby/python/kotlin/swift/ts) | API_VERSION → 2026-08-02 |
| tests: folders.test.ts, folders_service_test.rb, test_folders_service.py | Hand-written unit tests |
Review details
- Files reviewed: 41/68 changed files
- Comments generated: 1
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…nder
CreateFolder and UpdateFolder declared `ValidationError`, whose required wire
member is a singular `error` string. bc3 renders neither create nor update that
way. `stacks_controller.rb:51` answers a rejected rename with
`render json: { errors: @stack.errors }`, and `:27` answers a rejected create
with the same rendering from its `RecordInvalid` rescue. `ActiveModel::Errors`
serializes field-keyed, so the wire body is:
{"errors":{"name":["can't be blank"]}}
`FieldValidationError` already exists for exactly this shape and is already
carried by `UpdateCalendar`, `UpdateMyNote` and `UpdateMyPreferences`. Both
folder writes now use it. No other operation is touched.
The failure this fixes is silent, which is why it earns a test. While the
operations declared `ValidationError`, Go's typed `JSON422` was a
`*ValidationErrorResponseContent`; decoding the field-keyed body into it
succeeds — `encoding/json` does not care that `error` is absent — and leaves
`JSON422.Error` as `""`. A caller reading the typed field got an empty string
and no error of any kind.
`TestFoldersGenerated_JSON422_DecodesFieldKeyedBody` goes at the generated
client directly and pins `JSON422.Errors` for both operations. It was shown to
fail before the fix: against the previous commit's `client.gen.go` it does not
compile, because `*ValidationErrorResponseContent` has no `Errors` field.
The two wrapper-level tests beside it (`TestFoldersService_{Update,Create}_
FieldKeyed422`) are deliberately not the proof. They were run against the
un-fixed generated client and **passed**, because `checkResponse` parses the raw
body itself and never reads the typed field. A wrapper-only assertion here would
have been vacuous.
The hand-written TS/Python/Ruby tests already modelled the field-keyed body, so
they needed no change — they were right about the server and the spec was the
wrong half.
make check: REAL_EXIT=0. Swift ran rather than skipped (306 tests, 0 failures;
`swift run ConformanceRunner` executed). Ruby 1092 runs, 0 failures. Conformance
147 passed / 1 skipped (pre-existing).
Copilot caught this on #593. All five Folders operations carry `max: 3` in behavior-model.json, so the `max:3` bucket is 200, not 195 — and 200 + 43 = 243 matches the new operation total. The neighbouring derived figures in the same section (192 retry-eligible at max 3, 203 retry-eligible total) were updated with the absorption, so leaving 195 made the section internally inconsistent. Re-derived from behavior-model.json rather than incremented by hand. This prose carries no CI gate.
There was a problem hiding this comment.
🟡 Human review recommended
It repins bc3 provenance and spans generated code, spec, and count metadata across all six SDKs with a large blast radius, so it warrants final human verification despite only one minor optional nit being found.
Review details
Suppressed comments (1)
conformance/runner/swift/Sources/ConformanceRunner/Dispatch.swift:356
- Minor consistency nit: this hand-rolls
project_idsparsing withcompactMap, which silently drops any element that isn't an integer or that overflowsInt. That's the exact "one step quieter" silent-drop fault the helper docstring at the top of this file warns about (lines 19–24), and every other array/scalar fixture read here goes through the throwing helpers (intArray,longParam,optString,stringParam).intArray(defined at lines 77–90) already returns[Int]?and throwsbadParameteron a malformed element, so the request body assertion can't be pinned against a quietly truncated array. Consider using it here for consistency and fail-closed behavior. Optional.
projectIds: rb?["project_ids"]?.arrayValue?.compactMap {
$0.intValue.flatMap { Int(exactly: $0) }
}))
- Files reviewed: 42/69 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Copilot flagged this as a suppressed low-confidence comment on #593, and it is right. The Folders dispatch hand-rolled project_ids parsing with compactMap, which silently drops any element that is not an integer or that overflows Int — the exact "one step quieter" fault Dispatch.swift's own helper docstring (lines 19-24) exists to warn about. A quietly truncated array would still have satisfied the requestBody assertion. intArray (lines 77-90) already returns [Int]? and throws badParameter on a malformed element, and every other array read in this file goes through it. Mine was the only exception.
There was a problem hiding this comment.
🟡 Human review recommended
It combines a provenance repin with a new API surface spanning generated code, hand-written wrappers, conformance runners, and count metadata across all six SDKs, which warrants final human verification even though no objective defects were found.
Review details
- Files reviewed: 42/69 changed files
- Comments generated: 0 new
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Three review findings addressed. Two were on threads (both replied to and resolved); the third was a suppressed comment with no thread, so recording it here. 1. Codex P2 — folder writes declared the wrong 422 shape (
|
Repins bc3 provenance
d0edc1283b→2c0dafba13and absorbs the Folders APIthat repin brings into range, as one change. 238 → 243 operations.
Why these are coupled
doc/api/sections/folders.mddoes not exist at the old pin, so the five/stacksroutes are absent fromspec/bc3-routes.jsontoday. Repinning alonemakes them appear in the route table with no operation modelling them, and the
#586 parity gate (direction 2) then demands a disposition for five routes we
have every reason to model. Absorbing alone would model a contract the pin says
isn't there. Doing both is the only coherent order.
The repin — 11 commits, one API-surface change
dc6cd10714#12384344581a379#124942c0dafba13#1250119956c5579#12488845df62b6b#12487action_push_nativestructured errors112c1664bf63a34abe305f889d8365faf5fe9dfc1fad1ef608c664cdc9de#12494 / #12501 are behaviour fixes to endpoints the SDK already models.
They add prose to five
doc/apisections but no route and no payload field —proven, not assumed: regenerating
spec/bc3-routes.jsonat the new pin yieldsexactly five new entries, all
/stacks.#12488 verified independently, and the brief's read of it holds. It deletes
a
request.format.json?branch, which looks like an API change. It isn't. Theremoved branch returned
nilfor JSON when the recording's own assignmentwasn't prioritized; the replacement returns that same assignment unconditionally
and
&.unprioritizeis a no-op when there's no priority to remove. Bothspellings target exactly the id in the URL and both answer
204. What changedis that the web path stopped cascading to a sibling step — it adopted the
contract the JSON API has had since bc3#12483 (absorbed in SDK #528).
The absorption —
FoldersServiceThe wire type stays
Stack. The product was renamed, the payload was not.The operations, structures and generated methods use
Folder; the URI segmentand the
typediscriminator keepstack/Stack. Anything matching ontypemust match
"Stack".Two output structures, not one.
Folderis what the list returns;only
FolderWithProjectscarries the expandedprojects. A single shape withan optional member would make every generated list-item type declare a field the
list response never populates.
Three required-and-nullable fields.
gauges_url,colorandimage_urlare emitted unconditionally by
app/views/api/stacks/_stack.json.jbuilderandare commonly
null, so@requiredmodels the presence andsmithy-build.jsonjsonAddlayerstype: ["string","null"]on top — SPEC §"A third wire state",the same treatment
Wormhole.color/destination_urlalready get. Go types them*stringbecause the value is nullable, not because the key is optional.project_idsdoesn't round-trip. Create takes it; the response reports thesame ids as
bucket_idsand expands them asprojects. Three names, onerelationship. An unreachable id fails the whole request with 404 and writes
nothing, so
CreateFolderlistsNotFoundError.UpdateFolderlistsValidationErrorbecause a blank name is a real 422 — create defaults it toNew folderinstead.UpdateFolder/DeleteFolderare naturally idempotent;CreateFolderis not.Generator overrides: one. The
Folderstag resolves toFoldersServicethrough every generator's default fallback (zero service-group overrides), and
ListFoldersinfers thefolderresource type cleanly. The one addition isGetFolder→getFolderin TS/Kotlin/Swift: the bare rule collapses it tofolder(id), a noun with no verb, while Ruby and Python already emitget_folder. Same shape as theGetBookmarkoverride — all six SDKs now agree.Go note.
FolderWithProjectsrepeats the base fields rather than embeddingFolder. Embedding promotes them correctly at runtime but hides them fromcheck-wrapper-drift, which walks declared fields and reported all twelve asmissing. Repeating them keeps the guard able to verify every field.
Registry and coordination
spec/api-gaps/folders-api.md→absorbed-in-sdk, sevensmithy_refs.spec/api-gaps/stack-doc-and-smithy.mdkeepsconfirmed-not-api-resourceon purpose. It records a decision that was correct when made; the supersession
note in
folders-api.mdexists to preserve that history, not to be erased byit. Registration issue Register the Folders API gap (bc3 #12384) and supersede stack-doc-and-smithy #559 is already closed.
spec/bc3-route-allowlist.ymlneeds no change — no/stacksentry existed(the routes weren't in the table), and no entry became redundant.
COORDINATION.mdnamed pin338b7a11, three repins stale. Now names2c0dafba13and says the provenance file is the only authority.Verification, with real exit codes
make checkREAL_EXIT=0BC3_REPO_PATH=… make bc3-routes-checkREAL_EXIT=0make bc3-route-parityREAL_EXIT=0— five/stacksroutes matched both directionsswift build+swift test: 306 tests, 0 failures;swift run ConformanceRunnerexecuted1092 runs, 2572 assertions, 0 failures(make rb-check)867 passed, mypy clean, ruff clean147 passed, 0 failed, 1 skipped(pre-existing Go-runner origin case)The five new
paths.jsoncases PASS in all six runners — Go, Kotlin,TypeScript, Ruby, Python and Swift each got a dispatch branch, so none silently
skips. Confirmed per-runner in the log, and for TypeScript (whose summary is
compact) by a
--reporter=verboserun naming all five.Counts bumped deliberately, each derived from
behavior-model.jsonrather thanincremented by hand:
check-idempotency-parity77/199 → 79/203;SPEC/SECURITY/AGENTS 238 → 243 operations, 122 → 124 GETs, 47 → 48
PUTs, 23 → 24 DELETEs, 39 → 40 single-attempt POSTs, 188 → 192
retry-eligible ops at
max 3.New hand-written tests:
typescript/tests/services/folders.test.ts(11),python/tests/services/test_folders_service.py(11),ruby/test/basecamp/services/folders_service_test.rb(14). Each covers thelist-has-no-
projects/ get-has-projectssplit and the always-present-nulldecode, plus a 4xx per operation.
The API version tracks the pin date, so every SDK's
API_VERSIONmoves to2026-08-02.Summary by cubic
Repins BC3 to
2c0dafba13and adds the Folders API (/stacks) across all SDKs, introducing 5 operations and moving the API version to 2026-08-02. Operation count increases 238 → 243 with updated retry/idempotency metadata.New Features
FoldersServicewith List/Get/Create/Update/Delete at/stacks.jsonand/stacks/{folderId}.json.Folder(list; noprojects) andFolderWithProjects(get/create/update; includesprojects). Wiretypestays"Stack".project_idsis input-only on create; responses returnbucket_idsand expandedprojects.gauges_url,color,image_urlare required-and-nullable.UpdateFolder/DeleteFolderidempotent;CreateFoldernon-idempotent with standard retry. Naming overrideGetFolder→getFolderin TS/Kotlin/Swift.Bug Fixes
FieldValidationError(field-keyed 422) instead ofValidationErrorfor create/update. Verified with new Go tests that assert JSON422 decoding.max:3bucket 195 → 200; totals now match 243 ops).project_idsvia the throwingintArrayhelper to avoid silent truncation.Written for commit 5fda7c2. Summary will update on new commits.
Post-review correction to this description
Two statements above were written before review and are superseded by what shipped:
FieldValidationError, not justUpdateFolder. bc3'sstacks_controller.rbrenders{ errors: <ActiveModel::Errors> }on create (:27) as well as update (:51), via theRecordInvalidrescue — andActiveModel::Errorsserializes field-keyed, so the wire is{"errors":{"name":["can't be blank"]}}for both. The review only namedUpdateFolder;CreateFolderhad the identical defect. The TS/Python/Ruby tests already modelled the field-keyed body correctly — the spec was the wrong half. Verified by a Go test that pins the typedJSON422field and is red-proofed by failing to compile against the previous generated client (*ValidationErrorResponseContent has no field or method Errors). Wrapper-level assertions here are vacuous, becausecheckResponseparses the raw body and never reads the typed field.