Account event feed, inbox, and stream tickets: spec operations and generated clients - #898
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
github-actions
Bot
added
typescript
There was a problem hiding this comment.
🟡 Changes recommended
Nullable response fields are modeled incorrectly, Kotlin responses remain untyped, and delegated-performer coverage is incomplete.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds the account event-feed wire API across the specification, generated SDKs, Go wrapper, conformance suite, and supporting documentation.
Changes:
- Models polling, inbox, stream-ticket, error, and delegated-performer contracts.
- Regenerates and wires services across all seven SDKs.
- Adds service tests, conformance coverage, metadata, and route/gap updates.
[!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 |
|---|---|
typescript/tests/services/event-feed.test.ts |
Tests TypeScript event-feed operations. |
typescript/src/index.ts |
Exports event-feed APIs. |
typescript/src/generated/services/index.ts |
Registers generated service. |
typescript/src/generated/services/event-feed.ts |
Implements generated service methods. |
typescript/src/generated/path-mapping.ts |
Maps new routes. |
typescript/src/generated/metadata.ts |
Adds retry metadata. |
typescript/src/client.ts |
Wires the service accessor. |
typescript/scripts/generate-services.ts |
Adds resource-type overrides. |
swift/Sources/BasecampGenerator/Utilities.swift |
Adds resource-type overrides. |
swift/Sources/Basecamp/Generated/Services/EventFeedService.swift |
Implements generated Swift service. |
swift/Sources/Basecamp/Generated/Models/WebhookEvent.swift |
Adds delegated performer. |
swift/Sources/Basecamp/Generated/Models/PollInboxResponseContent.swift |
Adds inbox envelope. |
swift/Sources/Basecamp/Generated/Models/PollEventsResponseContent.swift |
Adds feed envelope. |
swift/Sources/Basecamp/Generated/Models/InboxItem.swift |
Adds inbox item model. |
swift/Sources/Basecamp/Generated/Models/FeedEventDetails.swift |
Adds event details model. |
swift/Sources/Basecamp/Generated/Models/FeedEvent.swift |
Adds feed event model. |
swift/Sources/Basecamp/Generated/Models/Event.swift |
Adds delegated performer. |
swift/Sources/Basecamp/Generated/Models/CreateStreamTicketResponseContent.swift |
Adds ticket response model. |
swift/Sources/Basecamp/Generated/Metadata.swift |
Adds operation metadata. |
swift/Sources/Basecamp/Generated/AccountClient+Services.swift |
Exposes event-feed service. |
swift/README.md |
Updates service counts. |
spec/overlays/tags.smithy |
Assigns EventFeed tags. |
spec/bc3-route-allowlist.yml |
Temporarily waives new routes. |
spec/basecamp.smithy |
Defines operations and schemas. |
spec/api-gaps/README.md |
Marks gaps absorbed. |
spec/api-gaps/event-feed.md |
Records event-feed absorption. |
spec/api-gaps/delegated-events-performed-by.md |
Records performer-field absorption. |
SECURITY.md |
Updates operation-count documentation. |
scripts/check-idempotency-parity |
Updates parity expectations. |
rust/generator/names.toml |
Adds resource-type mappings. |
rust/basecamp-sdk/tests/services.rs |
Tests Rust service operations. |
rust/basecamp-sdk/tests/guarantees.rs |
Updates generated guarantees. |
rust/basecamp-sdk/src/generated/types.rs |
Adds generated Rust types. |
rust/basecamp-sdk/src/generated/services/mod.rs |
Registers the service module. |
rust/basecamp-sdk/src/generated/services/event_feed.rs |
Implements generated Rust service. |
rust/basecamp-sdk/src/generated/routes.rs |
Adds generated routes. |
rust/basecamp-sdk/src/generated/mod.rs |
Updates operation count. |
rust/basecamp-sdk/src/generated/metadata.rs |
Adds operation metadata. |
rust/basecamp-sdk/src/generated/accessors.rs |
Exposes event-feed accessor. |
ruby/test/basecamp/services/event_feed_service_test.rb |
Tests Ruby service operations. |
ruby/lib/basecamp/generated/types.rb |
Adds generated Ruby types. |
ruby/lib/basecamp/generated/services/event_feed_service.rb |
Implements generated Ruby service. |
ruby/lib/basecamp/generated/metadata.json |
Adds operation metadata. |
ruby/lib/basecamp/client.rb |
Exposes event-feed service. |
python/tests/services/test_event_feed_service.py |
Tests Python service operations. |
python/src/basecamp/generated/types.py |
Adds generated Python types. |
python/src/basecamp/generated/services/event_feed.py |
Implements sync/async services. |
python/src/basecamp/generated/services/__init__.py |
Exports generated services. |
python/src/basecamp/generated/metadata.json |
Adds operation metadata. |
python/src/basecamp/client.py |
Exposes synchronous service. |
python/src/basecamp/async_client.py |
Exposes asynchronous service. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/Types.kt |
Adds query option types. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/services/event-feed.kt |
Implements generated Kotlin service. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/ServiceAccessors.kt |
Exposes event-feed accessor. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/options-param-order.json |
Records option ordering. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/WebhookEvent.kt |
Adds delegated performer. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Event.kt |
Adds delegated performer. |
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/Metadata.kt |
Adds operation metadata. |
kotlin/README.md |
Updates service counts. |
kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/Config.kt |
Adds resource-type overrides. |
kotlin/conformance/src/main/kotlin/com/basecamp/sdk/conformance/Main.kt |
Dispatches conformance cases. |
go/pkg/basecamp/webhooks.go |
Maps webhook performers. |
go/pkg/basecamp/webhook_event.go |
Exposes webhook performer. |
go/pkg/basecamp/url-routes.json |
Adds route metadata. |
go/pkg/basecamp/events.go |
Exposes and maps event performer. |
go/pkg/basecamp/event_feed_test.go |
Tests Go wrapper behavior. |
go/pkg/basecamp/client.go |
Adds event-feed accessor. |
go/grouped-client-inventory.yml |
Registers ungrouped operations. |
conformance/tests/event_feed.json |
Defines cross-SDK cases. |
conformance/runner/typescript/runner.test.ts |
Dispatches TypeScript cases. |
conformance/runner/swift/Sources/ConformanceRunner/Dispatch.swift |
Dispatches Swift cases. |
conformance/runner/rust/src/operations.rs |
Dispatches Rust cases. |
conformance/runner/ruby/runner.rb |
Dispatches Ruby cases. |
conformance/runner/python/runner.py |
Dispatches Python cases. |
conformance/runner/go/main.go |
Dispatches Go cases. |
behavior-model.json |
Adds retry and sensitivity metadata. |
AGENTS.md |
Updates operation counts. |
Review details
Suppressed comments (2)
spec/basecamp.smithy:7490
- These two members are documented as JSON
nullfor boosts on a recording (andboosted_event_typemay also be null for an uncataloged kind), but the generated schemas type them only as absent-or-non-null. Mark both properties nullable in the OpenAPI enhancement pass so TypeScript/Python consumers can represent valid responses, then regenerate.
spec/basecamp.smithy:7961 - No fixture or service test supplies a non-null
Event.performed_by; the added generated fields and Go wrapper mapping are therefore never exercised. Add a delegated recording-event fixture/assertion so this changed response field is verified across the SDK generation paths.
- Files reviewed: 44/83 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![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: dda6401a1d
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9295baf93f
ℹ️ 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.
🟡 Changes recommended
Kotlin responses remain untyped, and the stated non-breaking classification overlooks Kotlin positional-constructor and Go unkeyed-literal source breaks.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
conformance/tests/event_feed.json:4
- This description says six SDKs, but the fixture is dispatched by all seven runners (including the new Rust SDK), as the PR and
spec/api-gaps/event-feed.mdstate. Use “seven SDKs” to keep the conformance documentation accurate.
- Files reviewed: 52/91 changed files
- Comments generated: 3
- Review effort level: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe2cf75e5d
ℹ️ 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.
🟡 Changes recommended
Kotlin responses remain untyped, malformed Go continuation queries can silently discard cursors, and the retry documentation retains a stale count.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 53/92 changed files
- Comments generated: 2
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Kotlin currently loses the modeled response types and introduces avoidable constructor-order compatibility breaks.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 53/92 changed files
- Comments generated: 5
- Review effort level: Balanced
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0be238ef73
ℹ️ 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".
Member
|
🤖 Our comments crossed — I pushed The one thing worth reading before you write your decision 3: dropping Also worth knowing: Codex reviewed — @jorge Manrubia |
|
To use Codex here, create a Codex account and connect to github. |
Member
|
🤖 I put 1. "A body the server never sends" is too strong —
|
Closes an adversarial-review finding against the commit before this one:
"a decodable body the server never sends" is too strong a claim about the
route. It holds for `Events::InboxesController#require_agent_principal`,
which answers `head :forbidden` with no JSON. It does not hold for
/{accountId}/inbox.json as a whole: ApplicationController includes
TwoFactorAuthenticationEnforcement, whose `require_two_factor_authentication`
renders a flat JSON body at 403, and whose guard — `signed_in? &&
Current.user.identified?` — is true for a human API principal and false for
an agent. Being registered on the superclass it runs BEFORE the subclass's
agent guard, so for exactly the human-principal population this operation's
403 targets, the body-carrying 403 fires first. There is no API carve-out:
the only `bypass_two_factor_authentication_requirement` callers are
accounts/two_factor_authentication_requirements, my/login_options and three
internal/ controllers.
The modeling stands — that 403 is reachable on every operation in this spec
and is modeled on none of them, its {error, status} shape matches neither
ForbiddenError nor anything else here, and every SDK maps 403 by status and
parses whatever body arrives opportunistically. What changes is the claim:
the shape now says it describes what an operation's own guard answers, not
what the route can never send.
Also drops the roadmap line ("until registrations open it to people"), which
would have rotted on its own schedule, and the per-operation detail that
already lives on PollInbox's own doc comment — restoring the deliberately
operation-agnostic register of the BareNotFoundError this shape copies.
Spec-only: Smithy emits no components schema for an empty error structure,
so openapi.json is byte-identical (verified by rebuilding).
Closes the second adversarial-review finding against the BareForbiddenError
commit. Giving PollInbox a bare 403 made its own doc comment wrong:
**Errors** follow PollEvents, except that 410 (FeedPositionGoneError) here
means the position fell behind the retention window…
403 no longer follows PollEvents either. I corrected the parallel sentence in
SPEC.md §23's wire-operations table and missed this one — and this is the
sentence that ships. Smithy emits it verbatim as the operation `description`
in openapi.json, and the generators carry it into three SDKs' public API
docs. The SPEC.md row a contributor reads and the docstring a user reads had
drifted apart in the direction that matters.
Regenerated artifacts are doc strings only, on PollInbox alone:
openapi.json, typescript/src/generated/{schema.d.ts,openapi-stripped.json},
rust/…/generated/services/event_feed.rs, and both the sync and async
docstrings in python/…/generated/services/event_feed.py. Go, Ruby, Kotlin
and Swift do not carry operation descriptions into generated code, so their
trees are untouched.
Gates: smithy-check, behavior-model-check, doc-constants-check,
check-idempotency-parity, go-check-generated-drift, kt-check-drift,
check-fixture-coverage, ts-check and py-check all pass.
Member
|
🤖 I said nothing else from me was going onto this branch, and then pushed two more commits. Here is why, and then I am done: both are corrections to claims in my own commit that are checkably wrong, and leaving them for you to inherit seemed worse than one more push. Jorge made the call.
Gates run locally before pushing: The branch is yours from here. No more commits from me, and I am not going to touch the four you said you are writing. The one thing I would still read before writing decision 3 is the evidence two comments up: the mint's retry in Go and TypeScript hangs off Jorge is holding the merge until you have made your call. — @jorge Manrubia |
Member
Author
|
Correction on item 3 above: Status: items 1, 2 and 4 are landing on this branch now (the |
jorgemanrubia
added a commit
that referenced
this pull request
Sep 16, 2026
Three conflicts, all in generated output, all on the embedded generation timestamp alone: ruby/lib/basecamp/generated/metadata.json, types.rb and typescript/src/generated/metadata.ts. main's event feed operations (#898) and this branch's PaginationConfig narrowing merged cleanly around them. Resolved by taking main's files whole and regenerating from the merged inputs (make rb-generate, make ts-generate) rather than picking hunks, so the committed artifacts are what the generator actually produces. The result is main's content plus this branch's only real generated change: PaginationConfig.style loses "page" and pageParam goes with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
The declared 400/410 error contracts are not implemented, their conformance cases are missing, and Kotlin exposes the new responses only as raw JSON.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
conformance/tests/event_feed.json:183
- This is the suite’s only 400 case, and it covers only the body without
reason; the file has 10 cases rather than the 12 described by the PR, with noinvalid_positionorinvalid_filtercase. Add reason-bearing cases so the revised error contract and Go typed mapping are exercised across all runners.
spec/basecamp.smithy:7376 - The 400 contract still uses the generic
BadRequestError, so the generated schema has no optionalreasonand the Go wrapper exposes only a plain*Error. This contradicts the PR’sFeedRequestErrorAPI and prevents consumers from distinguishinginvalid_positionfrominvalid_filter; model that dedicated error on both poll operations and regenerate the SDKs.
This issue also appears on line 7537 of the same file.
spec/basecamp.smithy:7537
PollInboxstill sharesFeedPositionGoneErrorwithPollEvents. The PR promises a distinctInboxPositionGoneErrorspecifically so anerrors.Ashandler for the feed’s required epoch cannot accept an inbox retention gap; the current Go wrapper does return the same*FeedPositionGoneErrorfor both lanes. Give this operation its separate 410 shape and regenerate/update the wrapper.
- Files reviewed: 56/94 changed files
- Comments generated: 0 new
- Review effort level: Balanced
jorgemanrubia
added a commit
that referenced
this pull request
Sep 16, 2026
…use a mention it cannot name Two defects from the adversarial review, and the first says the earlier two-pass fix copied the wrong mechanism. THE KEY SET. Go never needs the normalizer for a typed read: `generated.Person.Id` is `types.FlexibleInt64`, so EVERY Person-typed field converts at decode. `normalizeEmbeddedPeopleJSON` exists only for the three wrapper paths whose `basecamp.Person.ID` is a plain int64 — so `creator`/`participants` is that wrapper's list, not the model's. TypeScript, which has no decoder at all, was still handing back `"007"` for `assignees`, `subscribers` and `completion_subscribers`, and `writableIdList` then refused it — a merge-safe update blocked by data BC3 controls. `PERSON_VALUED_KEYS` is now derived from the spec rather than typed out: every property whose schema `$ref`s `Person`, or an array of it. Twelve keys. A test recomputes the set from `openapi-stripped.json` and fails on any disagreement. THE GUARD EARNED ITSELF WITHIN THE HOUR. `performed_by` arrived on `Event` and `WebhookEvent` with #898 while this branch was being rebased — 450 schemas became 457 — and the check named it before anyone read the diff. A hand-maintained list would have rotted silently on its first day, which is precisely the failure this finding was. That is now the comment's stated reason for the guard. Three things the reference forced beyond a list of names. Arity is honoured, because Go's wrapper asserts `map[string]any` for `creator` and `[]any` for `participants` while `Assignees []Person` refuses a non-array — an object under `assignees` is a malformed body, not a person to coerce. The derivation asserts non-vacuity before comparing, since a walk that silently stops matching would otherwise "agree" with any set. And only `$ref: Person` counts: `OutOfOfficePerson` and its kin are person-SHAPED but plain int64 in Go. Divergences per position, 74 rows each, through the real pipeline: the five new object positions and the five new array positions were 62 of 74 and are now 11, every one of those 62 being the string reaching the caller untouched. The 11 are the unrepresentable-value residual, identical at every position. A PRE-EXISTING TEST PINNED THE DEFECT here too: an `it.each` row asserting `update` REFUSES a string assignee id. Removed — float, NaN, null and boolean still refused — and replaced with the positive case: a GET carrying `assignees: [{id: "007"}]` now PUTs `assignee_ids: [7]`. THE DROPPED MENTIONS. `mentionedPersonIds` silently skipped an id past 2^53: Go returned 5 for a six-mention text, Ruby 5, this 2, with no error. It now throws, naming the id. The argument, which is in the doc: unlike the normalizer, this returns `number[]` — there is no string to leave in place and no room to say "and one more I could not name". Under-reporting a set is the one failure a caller cannot detect, because the evidence it would need is the thing that was dropped, and it feeds the list deciding who gets notified. `mentions.ts`'s "every `<bc-attachment>` counts" now says so, with `@throws`. An id past INT64 is deliberately not that case: `ParseInt` raises, the reference answers "not a person", and so does this — skipped silently, as it skips it. Both boundaries pinned. One refactor beyond the brief, flagged by its author rather than slipped in: telling those two cases apart needs information `number | undefined` cannot carry, so the gid parse body moved to an internal `parsePersonSGID` and `personIdFromSGID` is a wrapper over it. Every answer is unchanged, and the tail is now literally the reference's shape — `scanPersonId` plus `id <= 0`, compared as a bigint — where `Number()` plus `isSafeInteger` stood, so the safe-integer boundary no longer decides identity. I re-ran the mutation check myself after the refactor: removing the `[0-9]` walk still fails with "+7: expected 7 to be undefined". Two gaps reported rather than hidden, both documented in the code: a response whose ROOT is a Person is unreached when the body omits `personable_type`, since the normalizer cannot know the operation's response schema; and on the new decoder-only positions TS writes a `system_label` where Go's generated path has nowhere to put one, and leaves the string on a range refusal where `FlexibleInt64` fails the read. Both additive or residual, neither a different id. 93 files, 1913 tests passed. No drift. Found by adversarial review (Opus) of the head intended for merge.
This was referenced Sep 16, 2026
jorgemanrubia
added a commit
that referenced
this pull request
Sep 16, 2026
…use a mention it cannot name Two defects from the adversarial review, and the first says the earlier two-pass fix copied the wrong mechanism. THE KEY SET. Go never needs the normalizer for a typed read: `generated.Person.Id` is `types.FlexibleInt64`, so EVERY Person-typed field converts at decode. `normalizeEmbeddedPeopleJSON` exists only for the three wrapper paths whose `basecamp.Person.ID` is a plain int64 — so `creator`/`participants` is that wrapper's list, not the model's. TypeScript, which has no decoder at all, was still handing back `"007"` for `assignees`, `subscribers` and `completion_subscribers`, and `writableIdList` then refused it — a merge-safe update blocked by data BC3 controls. `PERSON_VALUED_KEYS` is now derived from the spec rather than typed out: every property whose schema `$ref`s `Person`, or an array of it. Twelve keys. A test recomputes the set from `openapi-stripped.json` and fails on any disagreement. THE GUARD EARNED ITSELF WITHIN THE HOUR. `performed_by` arrived on `Event` and `WebhookEvent` with #898 while this branch was being rebased — 450 schemas became 457 — and the check named it before anyone read the diff. A hand-maintained list would have rotted silently on its first day, which is precisely the failure this finding was. That is now the comment's stated reason for the guard. Three things the reference forced beyond a list of names. Arity is honoured, because Go's wrapper asserts `map[string]any` for `creator` and `[]any` for `participants` while `Assignees []Person` refuses a non-array — an object under `assignees` is a malformed body, not a person to coerce. The derivation asserts non-vacuity before comparing, since a walk that silently stops matching would otherwise "agree" with any set. And only `$ref: Person` counts: `OutOfOfficePerson` and its kin are person-SHAPED but plain int64 in Go. Divergences per position, 74 rows each, through the real pipeline: the five new object positions and the five new array positions were 62 of 74 and are now 11, every one of those 62 being the string reaching the caller untouched. The 11 are the unrepresentable-value residual, identical at every position. A PRE-EXISTING TEST PINNED THE DEFECT here too: an `it.each` row asserting `update` REFUSES a string assignee id. Removed — float, NaN, null and boolean still refused — and replaced with the positive case: a GET carrying `assignees: [{id: "007"}]` now PUTs `assignee_ids: [7]`. THE DROPPED MENTIONS. `mentionedPersonIds` silently skipped an id past 2^53: Go returned 5 for a six-mention text, Ruby 5, this 2, with no error. It now throws, naming the id. The argument, which is in the doc: unlike the normalizer, this returns `number[]` — there is no string to leave in place and no room to say "and one more I could not name". Under-reporting a set is the one failure a caller cannot detect, because the evidence it would need is the thing that was dropped, and it feeds the list deciding who gets notified. `mentions.ts`'s "every `<bc-attachment>` counts" now says so, with `@throws`. An id past INT64 is deliberately not that case: `ParseInt` raises, the reference answers "not a person", and so does this — skipped silently, as it skips it. Both boundaries pinned. One refactor beyond the brief, flagged by its author rather than slipped in: telling those two cases apart needs information `number | undefined` cannot carry, so the gid parse body moved to an internal `parsePersonSGID` and `personIdFromSGID` is a wrapper over it. Every answer is unchanged, and the tail is now literally the reference's shape — `scanPersonId` plus `id <= 0`, compared as a bigint — where `Number()` plus `isSafeInteger` stood, so the safe-integer boundary no longer decides identity. I re-ran the mutation check myself after the refactor: removing the `[0-9]` walk still fails with "+7: expected 7 to be undefined". Two gaps reported rather than hidden, both documented in the code: a response whose ROOT is a Person is unreached when the body omits `personable_type`, since the normalizer cannot know the operation's response schema; and on the new decoder-only positions TS writes a `system_label` where Go's generated path has nowhere to put one, and leaves the string on a range refusal where `FlexibleInt64` fails the read. Both additive or residual, neither a different id. 93 files, 1913 tests passed. No drift. Found by adversarial review (Opus) of the head intended for merge.
Member
Author
|
Review threads (for the record, post-merge): 29 resolved (24 fixed, 5 declined with the reasoning in each thread). Declined:
The two findings decided after this merged — the per-lane 410 split and the reasoned 400 — are in #912. |
jeremy
added a commit
that referenced
this pull request
Sep 16, 2026
) Follow-up to #898 carrying the two review findings decided after it merged (the third, BareForbiddenError on PollInbox, landed in #898 itself). 1. The poll lanes' 410 is two shapes on two operations. PollEvents answers FeedPositionGoneError {error, epoch_after_id, resume}: epoch_after_id is now required and resume re-enters at since=<epoch_after_id>, so the servable history above the fence is not skipped. PollInbox answers a distinct InboxPositionGoneError {error, resume}: no epoch, resume re-entering at since=0, the earliest retained item. They were one shape with an optional epoch_after_id, so a consumer could write one errors.As arm that was silently wrong on one lane. The recoveries are not interchangeable, and the spec now says so. 2. A dedicated 400 with a reason. FeedRequestError {error, reason?} on both poll operations replaces the shared BadRequestError there. bc3 adds reason: "invalid_position" | "invalid_filter" beside error; reason is optional, and when it is absent the 400 is undifferentiated: a consumer surfaces it rather than guessing between re-entering with since= and stopping. Fixtures cover the 400 with reason on both lanes and the reason-less fallback. CreateStreamTicket keeps @basecampIdempotent(natural: true): Ruby, TypeScript and Go classify retry off that flag, so dropping it fails check-idempotency-parity. The cursor pagination style from #905 is not applied to these operations here. What changed: - Spec: FeedRequestError (400), FeedPositionGoneError (410, PollEvents, epoch_after_id required), InboxPositionGoneError (410, PollInbox); the two poll operations' error lists and docs. openapi.json regenerated. - Generated code, all seven SDKs. Generated Go: PollEventsResponse.JSON400 / PollInboxResponse.JSON400 are *FeedRequestErrorResponseContent; PollInboxResponse.JSON410 is *InboxPositionGoneErrorResponseContent; FeedPositionGoneErrorResponseContent.EpochAfterId is int64 (was *int64). - Go wrapper: *FeedRequestError{Err, Reason} with FeedReasonInvalidPosition / FeedReasonInvalidFilter (Reason == "" is the undifferentiated case); *FeedPositionGoneError{Err, EpochAfterID int64, Resume} returned by PollEvents only; *InboxPositionGoneError{Err, Resume} returned by PollInbox only; each unwraps to the canonical *Error. Tests cover the reasoned and reason-less 400, the feed 410, and that an inbox 410 never types as the feed's. - Conformance: event_feed.json gains the 400-with-reason case on each lane (12 cases, all seven runners); the 410 and 403 descriptions name the shapes. - Docs / registry: SPEC section 23 "Wire Operations" rows and typed-error paragraph; spec/api-gaps/event-feed.md smithy_refs. Breaking: generated Go types change shape (EpochAfterId pointer to value; the poll responses' JSON400/JSON410 types; PollInboxResponse loses its JSON403 body). The typed feed surface shipped in #898 is unreleased, so MIGRATING.md carries no entry. Verification: make generate (no diff), all 46 static gates, go/ts/py/rb/rs/kt/swift checks, and check-fixture-execution across all seven runners, rebased on main at e58b582. Review: 3 threads resolved (2 fixed, 1 declined with reasoning in the thread); Copilot and Codex reported on the merged head.
jorgemanrubia
added a commit
that referenced
this pull request
Sep 16, 2026
…use a mention it cannot name Two defects from the adversarial review, and the first says the earlier two-pass fix copied the wrong mechanism. THE KEY SET. Go never needs the normalizer for a typed read: `generated.Person.Id` is `types.FlexibleInt64`, so EVERY Person-typed field converts at decode. `normalizeEmbeddedPeopleJSON` exists only for the three wrapper paths whose `basecamp.Person.ID` is a plain int64 — so `creator`/`participants` is that wrapper's list, not the model's. TypeScript, which has no decoder at all, was still handing back `"007"` for `assignees`, `subscribers` and `completion_subscribers`, and `writableIdList` then refused it — a merge-safe update blocked by data BC3 controls. `PERSON_VALUED_KEYS` is now derived from the spec rather than typed out: every property whose schema `$ref`s `Person`, or an array of it. Twelve keys. A test recomputes the set from `openapi-stripped.json` and fails on any disagreement. THE GUARD EARNED ITSELF WITHIN THE HOUR. `performed_by` arrived on `Event` and `WebhookEvent` with #898 while this branch was being rebased — 450 schemas became 457 — and the check named it before anyone read the diff. A hand-maintained list would have rotted silently on its first day, which is precisely the failure this finding was. That is now the comment's stated reason for the guard. Three things the reference forced beyond a list of names. Arity is honoured, because Go's wrapper asserts `map[string]any` for `creator` and `[]any` for `participants` while `Assignees []Person` refuses a non-array — an object under `assignees` is a malformed body, not a person to coerce. The derivation asserts non-vacuity before comparing, since a walk that silently stops matching would otherwise "agree" with any set. And only `$ref: Person` counts: `OutOfOfficePerson` and its kin are person-SHAPED but plain int64 in Go. Divergences per position, 74 rows each, through the real pipeline: the five new object positions and the five new array positions were 62 of 74 and are now 11, every one of those 62 being the string reaching the caller untouched. The 11 are the unrepresentable-value residual, identical at every position. A PRE-EXISTING TEST PINNED THE DEFECT here too: an `it.each` row asserting `update` REFUSES a string assignee id. Removed — float, NaN, null and boolean still refused — and replaced with the positive case: a GET carrying `assignees: [{id: "007"}]` now PUTs `assignee_ids: [7]`. THE DROPPED MENTIONS. `mentionedPersonIds` silently skipped an id past 2^53: Go returned 5 for a six-mention text, Ruby 5, this 2, with no error. It now throws, naming the id. The argument, which is in the doc: unlike the normalizer, this returns `number[]` — there is no string to leave in place and no room to say "and one more I could not name". Under-reporting a set is the one failure a caller cannot detect, because the evidence it would need is the thing that was dropped, and it feeds the list deciding who gets notified. `mentions.ts`'s "every `<bc-attachment>` counts" now says so, with `@throws`. An id past INT64 is deliberately not that case: `ParseInt` raises, the reference answers "not a person", and so does this — skipped silently, as it skips it. Both boundaries pinned. One refactor beyond the brief, flagged by its author rather than slipped in: telling those two cases apart needs information `number | undefined` cannot carry, so the gid parse body moved to an internal `parsePersonSGID` and `personIdFromSGID` is a wrapper over it. Every answer is unchanged, and the tail is now literally the reference's shape — `scanPersonId` plus `id <= 0`, compared as a bigint — where `Number()` plus `isSafeInteger` stood, so the safe-integer boundary no longer decides identity. I re-ran the mutation check myself after the refactor: removing the `[0-9]` walk still fails with "+7: expected 7 to be undefined". Two gaps reported rather than hidden, both documented in the code: a response whose ROOT is a Person is unreached when the body omits `personable_type`, since the normalizer cannot know the operation's response schema; and on the new decoder-only positions TS writes a `system_label` where Go's generated path has nowhere to put one, and leaves the string on a range refusal where `FlexibleInt64` fails the read. Both additive or residual, neither a different id. 93 files, 1913 tests passed. No drift. Found by adversarial review (Opus) of the head intended for merge.
jorgemanrubia
added a commit
that referenced
this pull request
Sep 16, 2026
…use a mention it cannot name Two defects from the adversarial review, and the first says the earlier two-pass fix copied the wrong mechanism. THE KEY SET. Go never needs the normalizer for a typed read: `generated.Person.Id` is `types.FlexibleInt64`, so EVERY Person-typed field converts at decode. `normalizeEmbeddedPeopleJSON` exists only for the three wrapper paths whose `basecamp.Person.ID` is a plain int64 — so `creator`/`participants` is that wrapper's list, not the model's. TypeScript, which has no decoder at all, was still handing back `"007"` for `assignees`, `subscribers` and `completion_subscribers`, and `writableIdList` then refused it — a merge-safe update blocked by data BC3 controls. `PERSON_VALUED_KEYS` is now derived from the spec rather than typed out: every property whose schema `$ref`s `Person`, or an array of it. Twelve keys. A test recomputes the set from `openapi-stripped.json` and fails on any disagreement. THE GUARD EARNED ITSELF WITHIN THE HOUR. `performed_by` arrived on `Event` and `WebhookEvent` with #898 while this branch was being rebased — 450 schemas became 457 — and the check named it before anyone read the diff. A hand-maintained list would have rotted silently on its first day, which is precisely the failure this finding was. That is now the comment's stated reason for the guard. Three things the reference forced beyond a list of names. Arity is honoured, because Go's wrapper asserts `map[string]any` for `creator` and `[]any` for `participants` while `Assignees []Person` refuses a non-array — an object under `assignees` is a malformed body, not a person to coerce. The derivation asserts non-vacuity before comparing, since a walk that silently stops matching would otherwise "agree" with any set. And only `$ref: Person` counts: `OutOfOfficePerson` and its kin are person-SHAPED but plain int64 in Go. Divergences per position, 74 rows each, through the real pipeline: the five new object positions and the five new array positions were 62 of 74 and are now 11, every one of those 62 being the string reaching the caller untouched. The 11 are the unrepresentable-value residual, identical at every position. A PRE-EXISTING TEST PINNED THE DEFECT here too: an `it.each` row asserting `update` REFUSES a string assignee id. Removed — float, NaN, null and boolean still refused — and replaced with the positive case: a GET carrying `assignees: [{id: "007"}]` now PUTs `assignee_ids: [7]`. THE DROPPED MENTIONS. `mentionedPersonIds` silently skipped an id past 2^53: Go returned 5 for a six-mention text, Ruby 5, this 2, with no error. It now throws, naming the id. The argument, which is in the doc: unlike the normalizer, this returns `number[]` — there is no string to leave in place and no room to say "and one more I could not name". Under-reporting a set is the one failure a caller cannot detect, because the evidence it would need is the thing that was dropped, and it feeds the list deciding who gets notified. `mentions.ts`'s "every `<bc-attachment>` counts" now says so, with `@throws`. An id past INT64 is deliberately not that case: `ParseInt` raises, the reference answers "not a person", and so does this — skipped silently, as it skips it. Both boundaries pinned. One refactor beyond the brief, flagged by its author rather than slipped in: telling those two cases apart needs information `number | undefined` cannot carry, so the gid parse body moved to an internal `parsePersonSGID` and `personIdFromSGID` is a wrapper over it. Every answer is unchanged, and the tail is now literally the reference's shape — `scanPersonId` plus `id <= 0`, compared as a bigint — where `Number()` plus `isSafeInteger` stood, so the safe-integer boundary no longer decides identity. I re-ran the mutation check myself after the refactor: removing the `[0-9]` walk still fails with "+7: expected 7 to be undefined". Two gaps reported rather than hidden, both documented in the code: a response whose ROOT is a Person is unreached when the body omits `personable_type`, since the normalizer cannot know the operation's response schema; and on the new decoder-only positions TS writes a `system_label` where Go's generated path has nowhere to put one, and leaves the string on a range refusal where `FlexibleInt64` fails the read. Both additive or residual, neither a different id. 93 files, 1913 tests passed. No drift. Found by adversarial review (Opus) of the head intended for merge.
jorgemanrubia
added a commit
that referenced
this pull request
Sep 16, 2026
…use a mention it cannot name Two defects from the adversarial review, and the first says the earlier two-pass fix copied the wrong mechanism. THE KEY SET. Go never needs the normalizer for a typed read: `generated.Person.Id` is `types.FlexibleInt64`, so EVERY Person-typed field converts at decode. `normalizeEmbeddedPeopleJSON` exists only for the three wrapper paths whose `basecamp.Person.ID` is a plain int64 — so `creator`/`participants` is that wrapper's list, not the model's. TypeScript, which has no decoder at all, was still handing back `"007"` for `assignees`, `subscribers` and `completion_subscribers`, and `writableIdList` then refused it — a merge-safe update blocked by data BC3 controls. `PERSON_VALUED_KEYS` is now derived from the spec rather than typed out: every property whose schema `$ref`s `Person`, or an array of it. Twelve keys. A test recomputes the set from `openapi-stripped.json` and fails on any disagreement. THE GUARD EARNED ITSELF WITHIN THE HOUR. `performed_by` arrived on `Event` and `WebhookEvent` with #898 while this branch was being rebased — 450 schemas became 457 — and the check named it before anyone read the diff. A hand-maintained list would have rotted silently on its first day, which is precisely the failure this finding was. That is now the comment's stated reason for the guard. Three things the reference forced beyond a list of names. Arity is honoured, because Go's wrapper asserts `map[string]any` for `creator` and `[]any` for `participants` while `Assignees []Person` refuses a non-array — an object under `assignees` is a malformed body, not a person to coerce. The derivation asserts non-vacuity before comparing, since a walk that silently stops matching would otherwise "agree" with any set. And only `$ref: Person` counts: `OutOfOfficePerson` and its kin are person-SHAPED but plain int64 in Go. Divergences per position, 74 rows each, through the real pipeline: the five new object positions and the five new array positions were 62 of 74 and are now 11, every one of those 62 being the string reaching the caller untouched. The 11 are the unrepresentable-value residual, identical at every position. A PRE-EXISTING TEST PINNED THE DEFECT here too: an `it.each` row asserting `update` REFUSES a string assignee id. Removed — float, NaN, null and boolean still refused — and replaced with the positive case: a GET carrying `assignees: [{id: "007"}]` now PUTs `assignee_ids: [7]`. THE DROPPED MENTIONS. `mentionedPersonIds` silently skipped an id past 2^53: Go returned 5 for a six-mention text, Ruby 5, this 2, with no error. It now throws, naming the id. The argument, which is in the doc: unlike the normalizer, this returns `number[]` — there is no string to leave in place and no room to say "and one more I could not name". Under-reporting a set is the one failure a caller cannot detect, because the evidence it would need is the thing that was dropped, and it feeds the list deciding who gets notified. `mentions.ts`'s "every `<bc-attachment>` counts" now says so, with `@throws`. An id past INT64 is deliberately not that case: `ParseInt` raises, the reference answers "not a person", and so does this — skipped silently, as it skips it. Both boundaries pinned. One refactor beyond the brief, flagged by its author rather than slipped in: telling those two cases apart needs information `number | undefined` cannot carry, so the gid parse body moved to an internal `parsePersonSGID` and `personIdFromSGID` is a wrapper over it. Every answer is unchanged, and the tail is now literally the reference's shape — `scanPersonId` plus `id <= 0`, compared as a bigint — where `Number()` plus `isSafeInteger` stood, so the safe-integer boundary no longer decides identity. I re-ran the mutation check myself after the refactor: removing the `[0-9]` walk still fails with "+7: expected 7 to be undefined". Two gaps reported rather than hidden, both documented in the code: a response whose ROOT is a Person is unreached when the body omits `personable_type`, since the normalizer cannot know the operation's response schema; and on the new decoder-only positions TS writes a `system_label` where Go's generated path has nowhere to put one, and leaves the string on a range refusal where `FlexibleInt64` fails the read. Both additive or residual, neither a different id. 93 files, 1913 tests passed. No drift. Found by adversarial review (Opus) of the head intended for merge.
jorgemanrubia
added a commit
that referenced
this pull request
Sep 16, 2026
…908) * fix(go): normalize a person id to a number the JSON encoder can write `coercePersonID` stored a `ParseInt`-valid string VERBATIM as a `json.Number`, and `json.Marshal` validates that against the JSON number grammar, which is narrower than `ParseInt`'s: no leading `+`, no leading zeros. So the marshal at the end of `normalizeEmbeddedPeopleJSON` failed, every caller fell back to the RAW body as designed, and the raw body still carries a JSON string where the wrapper `Person.ID` is a plain `int64` — an embedded creator whose id is `"+7"`, `"007"` or `"0009223372036854775807"` therefore failed the WHOLE response on the notification, gauge and bubble-up paths, while Go's own `FlexibleInt64`, reading the same bytes, reads 7, 7 and 9223372036854775807 without complaint. Storing `strconv.FormatInt` of what `ParseInt` already returned is the same value in a form the encoder can write. Nothing else moves: an out-of-range id is still left as a string for the decoder to refuse, and a non-numeric sentinel still collapses to 0 with the label preserved. Measured, not reasoned. A probe linked against the real `normalizeEmbeddedPeopleJSON`, the real `types.FlexibleInt64` and the real `PersonIDFromSGID` ran a 74-row corpus built to hold the shapes that discriminate between the three rules. Divergences of the normalizer against `FlexibleInt64`: 13 before, 0 after. Reverting the fix fails `TestPersonIDNormalizeMatchesFlexible` on exactly those 13 rows. `person_id_grammar_test.go` carries the corpus, both properties, and the probe itself as a dump mode (`ORACLE_OUT=... go test -run TestPersonIDOracleDump`), so the six ports held to this rule can re-derive the table rather than copy one out of a comment. And the gid person-id rule stays where it is, with a test that says so in both directions. `PersonIDFromSGID` walks the bytes and refuses anything outside `0..=9` BEFORE parsing, so it rejects the leading `+` this path accepts; loosening it is the defect #886 closed, where `gid://bc3/Person/+77` began naming person 77. `TestPersonIDGidRuleStaysApart` pins the five rows where the two genuinely disagree about a real person AND the three leading-zero rows where they agree, so tightening the walk breaks a test as surely as loosening it. Deleting the digit walk fails it on `+7`, `+007` and `+9223372036854775807`. * test(rust): pin "010" as ten, the row the cross-SDK trap turns on Rust already implements Go's `ParseInt` scan exactly — measured, not assumed: a probe running the 74-row person-id corpus through the real `generated::types::Person` diverges on 0 rows, and Rust is the only SDK with no pre-decode normalizer to get wrong in the first place. Its corpus was thin in one place. `"07"` was pinned but `"010"` was not, and `"010"` is the row that separates "reads base ten" from "detects a base from the literal": Ruby's `Integer()` read it as EIGHT. That is not a refusal against an acceptance but two different PEOPLE from one wire value, both valid ids, with nothing downstream able to tell which was meant. * fix(ruby): decide a person id's refusal by Go's scan order, not by its shape `coerce_person_id` and `person_from_wire` both asked `bounded_decimal` "is the whole string a decimal, and does it fit?" — a lexical test, then a bound. Go never asks that. `ParseUint` (strconv/atoi.go) checks the magnitude INSIDE the scan and returns `ErrRange` the instant the accumulator would overflow uint64, before it ever reaches the rest of the string, so the first disqualifying byte wins and the boundary is u64's rather than int64's: "18446744073709551615x" digits still fit, the scan reaches the x -> syntax, reads 0 "18446744073709551616x" the overflow fires first -> range, fails the read Ruby read the second as the "basecamp" system actor — an oversized malformed id silently naming the actor that posts on nobody's behalf, in the accepting direction, which is the direction that matters. Three rows of the corpus: `"18446744073709551616x"`, `"-18446744073709551616x"`, `"99999999999999999999999x"`. Divergences against the Go table: 3 before, 0 after, at BOTH sites. `Ids.parse_int` is now that scan written out, byte by byte, with the two refusals kept apart as the thing the caller branches on — `:syntax` is the sentinel that reads 0, `:range` fails the read, and no single "is this a number?" predicate can tell them apart. Both person-id sites share it, so there is one rule rather than two that drift. `bounded_decimal` is UNCHANGED and keeps its four other callers. Its rule is lexical-then-bounded, which is the right rule for a caller's argument, a header count, a Retry-After and — this is the one that matters — the sgid's person id, where the reference walks the bytes and refuses anything outside 0..=9 BEFORE parsing (mentions.go:252-256) and so rejects a leading `+`. Unifying the two would accept `+77` as a mentioned person again, which is the defect #886 closed. Both docs now say so, in both directions. Every other `bounded_decimal` caller was compared by ROUTE as well as by verdict, over all 74 corpus rows plus boundary extras, because two rules that agree on today's inputs diverge the moment a case is added between them: `parse_global_id` differs on neither (its digit-run pre-gate admits nothing else), `person_identity` and `parse_total_count` differ in route on the same three rows but map both refusals to one answer by construction, and `parse_retry_after` is gated to digit runs. `Ids.integer` differs on 18 rows and must, since a caller's argument is unsigned digits by contract. Reading bytes rather than matching a regexp also keeps the two properties the old call had: a String carrying invalid UTF-8 does not raise, and nothing builds an arbitrarily large Integer out of a 50 MB body — the accumulator passes u64 within 20 digits and every other byte ends the scan, so the scan is bounded by construction and needs no length gate. Pinned on the clock at 5,000,000 digits. The 74 rows are measured verdicts of the reference's own reader, dumped by `TestPersonIDOracleDump` and generated into `GoPersonIds::CORPUS` mechanically rather than retyped. 1744 runs, 32956 assertions, 0 failures. * fix(python): read a person id with Go's ParseInt, not Python's int() `_normalize_person_ids` runs over EVERY response body in this SDK, and it turned an integer-shaped string into a number with `int()`. That is the wrong tool in four separate directions, each of them ACCEPTING — and Go's 0 is the system-actor sentinel (`LocalPerson`, `"basecamp"`, `"campfire"`), so each acceptance either mints an id BC3 never wrote or collapses a real person onto an actor that posts on nobody's behalf: " 7", "7 ", "\n7", "\t7" int() strips ASCII whitespace; Go trims nothing "1_0", "1_2" int() honours PEP 515; `_` is a digit only at base 0 "１２３", "٠١٢", "৭", "۷" int() takes every Unicode decimal digit; Go tests bytes "18446744073709551616" int() is arbitrary-precision; the wire id is an int64 and three rows where the scan order decides which refusal you get — `"18446744073709551616x"`, `"-18446744073709551616x"`, `"99999999999999999999999x"` were labelled as the system actor where Go raises a range error and fails the read. Divergences against the Go table, measured with a probe linked against the real functions rather than read off a document: 29 of 74 before, 0 after, at both the sync and async normalizer. (28 by the id number alone; the 29th is `"٠"`, where `int()` happened to produce Go's 0 but wrote no `system_label`, so the actor's own spelling was lost.) There were THREE copies of this rule in the Python SDK, not two, and they disagreed. `services/_campfire_index.py` already carried `_go_parse_int64` — the Go-correct one, scan order included — feeding the `FlexibleInt64` port the recording-summary composite reads through, measured at 0 of 74 before and after. The rule now lives once, in `basecamp._person_id`, and all three call it; a test asserts the sync and async modules hold the SAME function object rather than merely equal behaviour, since equal behaviour is what they had before they drifted. `mentions.person_id_from_sgid` is untouched, and that was checked rather than assumed: an oracle built from `PersonIDFromSGID` (`go/pkg/basecamp/mentions.go: 252-258` — the digit pre-walk, then ParseInt, then `id <= 0`) agrees with it on 74 of 74, and it differs from the ParseInt table on 62 of 74 deliberately. Unifying the two would have moved 62 rows and reintroduced the `+77` defect #886 closed. Both sites now carry a comment pointing at the other and saying so in both directions. 2593 passed, 4 skipped. * fix(kotlin): read a person id with Go's ParseInt scan, not toLongOrNull Both person-id sites — the pre-decode `normalizePersonIds` and `FlexibleLongSerializer` — shared one predicate, `toLongOrNull()` with a `^-?\d+$` regex behind it for the overflow case, and it was wrong in both directions at once. Divergences against the Go table: 12 of 74 at each site, 0 after. Accepting, which is the direction that matters, 7 rows: `toLongOrNull` goes through `digitOf`, which on JVM is `Character.digit` and therefore Unicode-aware. `"１２３"` read 123, `"٠١٢"` read 12, `"৭7"` and `"7৭"` read 77 — real person ids where Go reads its non-numeric sentinel `0`, which is the SYSTEM ACTOR (`LocalPerson`, `"basecamp"`, `"campfire"`). Verified against the real stdlib rather than assumed: `javap` on kotlin-stdlib 2.4.20 shows `digitOf` is a one-line `invokestatic Character.digit`, and jshell answers 123, 12 and 77 for those three. Sentinel-instead-of-refusal, 5 rows: the regex refuses the leading `+` Go accepts, so `"+9223372036854775808"` collapsed to the system actor rather than failing the read; and the regex gets the scan-order boundary backwards. `ParseUint` checks the magnitude INSIDE the scan against `UInt64.MAX` and returns `ErrRange` before it ever reaches the rest of the string, so `"18446744073709551616x"` is a range refusal that fails the read while `"18446744073709551615x"`, one digit shorter, is a syntax refusal that reads 0. Both sides of that pair are now pinned. `parseInt64` is that scan written out, with the two refusals kept apart as a sealed `ParsedInt64` the caller branches on — no single "is this a number?" predicate can tell them apart, since which refusal comes first depends on where the disqualifying byte sits. Depending on no platform parser also makes the `expect`/`actual` question moot: `digitOf` is per-target, and this module declares only a JVM target today, but the code is in commonMain and the common `digitToInt` contract is Unicode Nd on every target. The gid rule stays apart. `personIdFromSgid` walks the bytes and refuses anything outside `0..9` before parsing, so it rejects the leading `+` this scan accepts; loosening it is the defect #886 closed. `MentionsTest` now pins that in both directions — it refuses `+7`, `+007`, `+9223372036854775807`, `-7` and `-9223372036854775808`, and still accepts `007`, `010` and `0009223372036854775807`, so tightening the walk breaks a test as surely as loosening it. `Mentions.kt` needed no functional change and got none: it compares `personIdFromSgid`'s answer against a `Long` the wire already produced through `FlexibleLongSerializer`, which is the site above. The 74 rows are measured verdicts of the reference's own reader, asserted at both sites — the reader on `{"id":"<raw>"}`, and the normalizer by normalizing then decoding onto the real generated `Person` exactly as `BaseService` runs it, `system_label` included. `./gradlew :basecamp-sdk:check` and `make kt-test` both BUILD SUCCESSFUL. One coverage gap noted rather than closed, because it is about WHICH objects are normalized rather than the grammar: Go runs two passes, the `personable_type`-keyed one and a `creator`/`participants`-keyed one, and Kotlin has only the first. An embedded creator that omits `personable_type` still reads id 0 here, because the reader handles the sentinel where Go's plain int64 field cannot, but it gets no `system_label`. * fix(typescript): read a person id with Go's ParseInt, and stop naming the system actor `normalizePersonIds` runs over every response body, and its `^-?\d+$` plus `Number.isSafeInteger` pair was wrong at both ends. It refused the leading `+` that `ParseInt` takes, so `"+7"` — a real person Go reads as 7 — became `id: 0`, which is Go's NON-NUMERIC SENTINEL: the id of a `LocalPerson`, `"basecamp"`, `"campfire"`. And it collapsed every id past `Number.MAX_SAFE_INTEGER` to that same `0` plus a `system_label` — range errors and genuine large int64 ids alike. A real person handed back as the system actor, silently, in the exact shape a caller is meant to trust. `personIdValue` in `mentions.ts` had the third defect, the scan-order one: `ParseUint` checks the magnitude INSIDE the scan against u64 and returns `ErrRange` before reaching the rest of the string, so `"18446744073709551616x"` fails the read in Go while `"18446744073709551615x"`, one digit shorter, is a syntax refusal that reads 0. It answered 0 to both. Both sites now go through one scan, `scanPersonId` in `src/person-id.ts`, which accumulates into a `bigint` so Go's answer is never rounded before a caller decides what to do with it, and keeps the two refusals apart as the thing the caller branches on. Divergences against the measured Go table: 26 of 74 before at the normalizer, 14 of 74 at the reader; 11 at each after, all of them the one residual below. (25 by the id alone at the normalizer; the 26th is `"+0"`, which agreed on `0` while attaching a spurious label.) THE RESIDUAL, argued rather than papered over. `"9007199254740993"` is a real person and a JS `number` cannot carry it — past `MAX_SAFE_INTEGER` two distinct int64s land on the same double — while the SDK types every id as `number` (`conformance/tests/integer-precision.json` records the same constraint for the JSON-number path, and waiver 1B.6 retains it). TypeScript also has no runtime decoder downstream to perform Go's RANGE refusal on its behalf, so whatever is chosen, a caller sees it. Writing `0` is what it did before and is the worst of the three: a wrong id that looks right is the one outcome nothing downstream can defend against. Throwing refuses a response Go reads fine, and refuses ALL of it — one unrepresentable id would discard the other ninety-nine readable people in the body, from inside a normalizer every response passes through. So the string is LEFT IN PLACE: nothing rounded, nothing invented, the digits verbatim for a caller that can hold them, and `typeof person.id === "string"` is a check a caller can actually make. That is the treatment the measured table already assigns to RANGE, so one rule covers both unrepresentable cases, and `personIdValue` already answered this way — the normalizer joins the reader rather than the reverse. Eleven rows, named in the test that pins the count. `personIdValue` is now exported at module level because the scan-order defect is unobservable through `mentionMarkup` — `0` and `undefined` both refuse the mention — so only the corpus can pin it. `index.ts` re-exports explicitly and has no `export *`, so the package's public API is unchanged. The gid rule stays apart: `personIdFromSGID` walks the bytes and refuses anything outside `0..=9` before parsing, which is correct at that site and is the defect #886 closed. Noted at both, in both directions. `make ts-check`: no drift, typecheck clean, 92 files, 1895 tests passed. * fix(swift): read a person id with Go's ParseInt scan, not Int() plus an ICU regex Both person-id sites — `BaseService.normalizeWalk` and `FlexibleInt` — paired `Int(s)`, which is ASCII-strict, with `s.range(of: #"^-?\d+$"#, options: .regularExpression)` to tell an overflow from a sentinel. That pairing was wrong three separate ways, in both directions at once. Divergences against the Go table: 13 of 74 at each site, 0 after. REFUSED WHERE GO LABELS, 9 rows. `NSRegularExpression` is ICU and ICU's `\d` is `\p{Nd}` — every Unicode decimal digit — while `Int(_:radix:)` is ASCII-only. So `"１２３"`, `"٠١٢"`, `"৭"`, `"۷"`, `"７"` and their mixed forms failed `Int()`, matched the regex, and were therefore reported as numeric OVERFLOW, which throws and fails the whole response, where Go quietly reads its non-numeric sentinel 0. And `"7\n"`, which neither the card nor the brief predicted: ICU's `$` matches before a final newline, AND `range(of:options:)` asks for a match SOMEWHERE rather than over the whole string. Two anchors that look like they pin both ends pin neither. Found by calling the actual Swift API — a first ICU probe using `uregex_matches`, which has full-string semantics, missed it. NAMED THE SYSTEM ACTOR WHERE GO RAISES, 4 rows. The regex refuses the leading `+` that `ParseInt` accepts, so `"+9223372036854775808"` fell through to the sentinel branch; and it gets the scan-order boundary backwards — `ParseUint` checks the magnitude INSIDE the scan against `UInt64.max` and returns `ErrRange` before reaching the junk, so `"18446744073709551616x"` fails the read in Go while `"18446744073709551615x"`, one digit shorter, is a syntax refusal that reads 0. `parsePersonID` walks the UTF-8 bytes, which settles all three permanently: there is no character class, anchor or match-mode semantics left to depend on a library version. It keeps the two refusals apart as a `PersonIDReading` the caller branches on, and both sites share it, because `coercePersonID` and `FlexibleInt64` are two call sites of one `ParseInt` in the reference. `"+7"` was NOT a divergence and the brief was wrong to say so: Swift's `Int(_:radix:)` takes a leading `+`, so the regex was never reached for it. The row stays in the corpus — it discriminates for the other ports. The gid rule stays apart. `Mentions`' global-id parser keeps its digit walk and its `id > 0`, untouched, with a comment facing the opposite way from the one on `parsePersonID`: dropping the walk to share this scan would name person 77 for `gid://bc3/Person/+77`, which is the defect #886 closed. MEASURED, with what this machine has. There is no Swift toolchain here, so the corpus was run under `swift:6.0-jammy` in Docker, against a standalone package holding `FlexibleInt.swift` verbatim and the three normalizer statics lifted verbatim: 74 rows through the real decode path and through the walk's real decision logic, before and after, plus ICU's behaviour confirmed twice — through `uregex_*` in C and through Swift's own `String.range(of:options:)`. Build complete, 0 warnings, 0 errors under language mode v6. Two things are reasoned rather than measured, and both are about the platform rather than the rule. The whole package does not build on Linux for pre-existing reasons (it is Darwin-only, and the generator has a trailing comma Swift 6.0 rejects), so macOS CI is the first place the full compile is exercised. And the two new JSON-level normalizer tests fail on Linux — as do the three pre-existing ones, unmodified — because iterating an `NSMutableDictionary` there yields a Swift `Dictionary`, so `normalizeWalk`'s `as? NSMutableDictionary` misses nested values and the walk is a no-op. Cause established rather than guessed; pre-existing platform property, untouched here. * docs(spec): state the person-id rule, and which disagreement is deliberate "Match Go" was not sufficient guidance for a person id, because Go has two rules for one and they disagree on purpose. §10 now says which is which, where a port implementer meets it: the `ParseInt` grammar with its scan order, the three outcomes each site owes to the two refusals, the gid walk that must stay apart in both directions, and the shapes a corpus has to contain before a clean sweep over it means anything. It points at the oracle rather than restating a table. Every port that reasoned from `ParseInt`'s documentation instead of probing it got something wrong, and the probe is checked in as a dump mode on the Go test, so the numbers here can be re-derived rather than trusted. Per-SDK state included, with TypeScript's one residual named as a residual: a JS `number` cannot carry an `int64` past 2^53, the same constraint waiver 1B.6 and `conformance/tests/integer-precision.json` already record, so an id Go reads is reported unreadable rather than rounded or turned into the system actor. * fix(kotlin): a sentinel id's system_label must not lose to the body's own The normalizer rebuilds a Person-shaped object by replaying its members in order, and a JSON object's members ARE ordered. So when the sentinel branch wrote `system_label` from the raw id and the incoming body happened to spell its own `system_label` AFTER `id`, the loop reached that key afterwards and copied the wire value straight over the one just written. The value being overwritten came off the wire, which is what makes this worth a branch rather than a tidy-up: a response carrying its own `system_label` could choose the label this SDK reports for the system actor — the one field a caller reads to find out which actor it was handed. The reference cannot have the bug and so has no branch for it. `coercePersonID` assigns into a map (`go/pkg/basecamp/normalize.go:66-67`), so the label it writes wins however the body was spelled; only a builder that replays members in order can lose. Kotlin is the only port that rebuilds rather than assigns — Swift, TypeScript, Python and Ruby all overwrite a key — so this was Kotlin's alone. Dropped only for the sentinel outcome. A value or a range refusal never assigns `system_label` in the reference, so an incoming one is preserved here too, and a test pins that in case the fix is ever widened into dropping the key outright. Both orders are pinned, because only one of them was ever wrong and a test holding the other proves nothing. Restoring the old loop fails `aSentinelIdOverwritesAnyIncomingSystemLabelInEitherOrder`. Found by Copilot on #908. * test(ruby): pin the gid rule's refusal of a sign, and its acceptance of zeros Every other gid case in this file is refused twice over — by the digit pre-walk and by the unsigned parse behind it — so none of them holds the walk on its own, and none of them carries a sign at all. `gid://bc3/Person/+7` was untested, which is the one shape this rule and `Ids.parse_int` answer differently about a real person: the reference walks the bytes and refuses anything outside 0..9 BEFORE parsing (`go/pkg/basecamp/mentions.go:252-256`), while `strconv.ParseInt` taken whole reads `"+7"` as 7. The acceptance half is pinned with it. Leading zeros carry no magnitude and pass the walk, so `"007"`, `"010"` and `"0009223372036854775807"` resolve — without those rows the refusal test could be satisfied by refusing too much, and a reader who "hardens" the rule into rejecting zero-padding diverges from the reference just as surely as one who loosens it. The mutation check taught me something I had wrong. My first attempt mutated `bounded_decimal`'s `signed:` flag and the suite stayed green — because the `/\A\d+\z/n` pre-walk is the guard, and the unsigned parse is a second, independent one. Either alone refuses `"+7"`; both have to go before the tests fail. So the verification is: drop the walk AND unsign the parse, and `test_refuses_a_signed_person_id_that_parse_int_would_accept` fails on `+7`. Ruby is defended twice here, which is worth knowing rather than assuming. Prompted by Copilot's finding of the same gap in TypeScript on #908; checked across all seven SDKs, and Ruby was the other one missing it. * fix(typescript): normalize an embedded person that omits personable_type Go's wrapper runs TWO passes over a response body, not one. `normalizeEmbeddedPeopleJSON` calls the generic `personable_type`-keyed pass AND `normalizeEmbeddedPersonIds`, which finds people by their known structural position — the `creator` object and each `participants` element, at any depth, whether or not they carry a `personable_type`. Its comment says exactly why: embedded creator/participants people frequently omit that key, so the first pass skips precisely the payloads the second exists to fix. TypeScript had only the first pass, and it is the one SDK where that is observable. Every other port has a runtime decoder behind the normalizer — Kotlin's `FlexibleLongSerializer`, Swift's `FlexibleInt`, Rust's `flexible_i64`, Python's `_decoded_flexible_int64`, Ruby's `person_from_wire` — which converts the string at read time whichever pass did or did not touch it. TypeScript has none, so an un-normalized `creator.id` reached the caller as the STRING `"007"` in a field typed `number`, where the reference and the other five give `7`. Divergences over the 74 rows, per shape: before after the ParseInt fix now personable_type person 26/74 11/74 11/74 bare creator 62/74 62/74 11/74 participants element 62/74 62/74 11/74 nested creator 62/74 62/74 11/74 The 62 is every row but the 12 range ones, and those agreed only by accident: "leave the string in place" is what doing nothing looks like. That accident is also what made the honest string this normalizer still leaves behind — the id past 2^53 that a JS `number` cannot carry — unreadable as a signal. It now means one thing: Go read an int64 this platform cannot hold, and nothing else. One walk here where Go runs two, which is safe only because `coercePersonId` is idempotent and both passes apply it unchanged: after `JSON.parse` the body is a tree, the set of coerced nodes is the same union either way, and a node in both sets is coerced twice under both schemes — a no-op the second time, since its id is no longer a string. Only the order differs. Argued in the comment rather than asserted, and pinned by an idempotence test on a `creator` that also carries `personable_type`. Go's type assertions are mirrored, so a `creator: "me"` or a non-array `participants` is skipped rather than coerced, and that is pinned too. One test depended on the old behaviour — an hour-old one of my own asserting a bare `creator: {id: "+7"}` came back as `"+7"` — and it was wrong for the reason above. Every other TS test and every `spec/fixtures` JSON was scanned for a string id under `creator`/`participants`: no other hits. 92 files, 1897 tests passed. * docs(spec): the reference finds an embedded person two ways, not one The §10 rule covered the grammar and not the coverage, and the coverage is where a port silently differs: `normalizeEmbeddedPeopleJSON` runs a second pass that finds people by structural position — `creator` and each `participants` element, at any depth, regardless of `personable_type` — because embedded people frequently omit that key. Worth stating because it is invisible from inside most ports. An id field with a decoder behind it converts the string at read time whichever pass touched it, so the missing pass shows up only in a port with no runtime decoder, which is how it reached a caller in TypeScript and nowhere else. * test(typescript): pin the gid rule directly, in both directions Nothing in this suite held Rule A. Every other `personIdFromSGID` case is refused for reasons unrelated to the digit walk and none of them carries a sign, so the `[0-9]` guard could be deleted and the suite would stay green while `gid://bc3/Person/+7` began naming person 7 — the `+77` defect #886 closed, on the field that decides WHO a mention names, so the failure is a tag pointing at the wrong person rather than a dropped read. Refused: `+7`, `-7`, `+007`, `+9223372036854775807`, `-9223372036854775808`, `0` and `-0`. Accepted: `007`, `010`, and `0009007199254740991` — the acceptance half matters as much, since without it the refusals could be satisfied by a walk that refuses too much, and "hardening" the rule into rejecting zero-padding diverges from the reference exactly as far as loosening it does. The nineteen-character padded row is there so the accepted set is not three digits wide: it fails the moment someone bounds the walk by length. Mutation-verified per guard, not per file, which is the lesson from getting it wrong in Ruby an hour earlier. This site has two independent guards, so mutating one leaves the rows the other catches green: removing the digit walk fails on `+7` (1 failed, 56 passed), removing `id <= 0` fails on `0` (2 failed, 55 passed). Each half is genuinely held, and the remaining signed rows are belt-and-braces. One expected value was corrected rather than asserted. `0009223372036854775807` is 9223372036854775807 in Go and `undefined` here: `personIdFromSGID` ends in a safe-integer check, so an id past 2^53 is refused rather than rounded into a neighbouring person. That is the same residual `personIdNumber` argues, reached at rule A's site, and refusing beats misattributing on this field — so it is pinned as the residual it is, with Go's real value named beside it, and SPEC §10 now records that the limit reaches this site too. `personIdFromSGID` itself is byte-identical. 92 files, 1898 tests passed. * fix(swift): the number path has no sentinel either, so stop answering the system actor `FlexibleInt` spent this branch getting its STRING path exactly right while the other half quietly answered `0` — the system actor, `LocalPerson` / `"basecamp"` / `"campfire"` — to everything that was neither an `Int` nor a `String`. Twelve rows of a 22-row number corpus diverged from the reference; 0 now. Go runs a JSON number through `json.Decoder` with `UseNumber()` and then calls `json.Number.Int64()` (`go/pkg/types/flexible_int64.go:52-62`), which IS `strconv.ParseInt` over the literal's text — the same scan the string path runs. What the number path does NOT have is the `ErrSyntax`-becomes-`0` branch: `:60` returns an error for either refusal. So `null`, `true`, `[7]`, `{"a":7}`, `7.5` and an out-of-range magnitude all fail the read there, and now fail it here. The row that matters most is the one the brief did not name. `-9223372036854775809` as a bare JSON number read `0`: the NEGATIVE int64 boundary, the exact mirror of the refusal this branch spent its whole length pinning on the string side, answering with the id of the actor that posts on nobody's behalf. Three more the brief missed with it — `1e30`, `0.0`, `-0.0`. Tests were written first and confirmed failing against the unfixed code. `null` fails the read and an absent key does not, and here that falls out of the model rather than needing a branch: `Person.id` is a non-optional `FlexibleInt`, the SDK's only use site, so a served `null` reaches this initializer and throws while a missing key is `keyNotFound` and never arrives. That matches `encoding/json`, which calls `UnmarshalJSON` for a `null` — leaving the `json.Number` empty, so `ParseInt("")` is a syntax error — while an absent field keeps its zero value. Ruby's `person_from_wire` documents the same asymmetry. Both halves are pinned, and so is an `OptionalWrapper` case, because making `Person.id` optional is a one-character generator change that would silently turn a served `null` into `nil` without this type ever seeing it. That is now a failing test rather than a surprise. SPEC §10's float-tolerant rule is deliberately NOT applied here and the comment says so: a rich-text `width`/`height` is a bare `Int32?` where BC3 really does serialize `1024.0` and `null` means "not an image". A pixel count may be lenient; an identity may not. ONE RESIDUAL, pinned as a test rather than left implicit: a float-spelled but integral number in range — `7.0`, `1e3`, `7e0`, `0.0`, `-0.0` — reads as that integer where Go refuses it. Unreachable rather than unfixed: measured under Swift 6.0, `7` and `7.0` are indistinguishable through every accessor a `SingleValueDecodingContainer` offers, because `JSONDecoder` unboxes via `Int(exactly: Double)`. Closing it needs the raw bytes. The direction is benign in the way that matters — it reads the CORRECT id for a spelling Go refuses, and can never produce the system actor or name a different person, unlike the twelve rows that did. Two claims are reasoned, not measured, and are marked as such: Go's verdict on the twelve rows was read off `flexible_int64.go:52-62` rather than run, and full compilation on macos-15 is inferred, since the package is Darwin-only and there is no Swift toolchain on this machine. The corpus itself was measured under `swift:6.0-jammy` against the verbatim source. Found by adversarial review (Opus) of the head intended for merge. * fix(python): normalize an embedded person that omits personable_type Python implemented one of the reference's two normalizer passes, and my reason for thinking that was safe — "the flexible reader converts the string at read time anyway" — was wrong for Python. That reader runs only inside the recording-summary composite; every generated service returns a plain dict, so for everything else nothing converts. `normalizeEmbeddedPeopleJSON` also finds people by structural position: the `creator` object and each `participants` element, at any depth, REGARDLESS of `personable_type`. Go's comment says why — embedded creator and participant people frequently omit that key, so the first pass skips exactly the payloads the second exists to fix. Divergences against the Go table, driven through the real request path rather than by calling the walk: before after personable_type person 0/74 0/74 bare creator 62/74 0/74 participants element 62/74 0/74 nested creator 62/74 0/74 The same 62 as TypeScript, and the twelve that "agreed" agreed only by accident: they are the RANGE rows, where leaving the string is what doing nothing looks like. IT IS REACHABLE ON A WRITE, which is the half a caller feels. Measured through the real composite both ways: old walk ApiError: ScheduleEntry field 'participants'[0].id is not an integer: str '1049715915' shipped walk OK, seeded participant_ids=[1049715915] A merge-safe update refusing a body the reference accepts is the vanishing direction, on a write, from data BC3 controls. The WALK moved into `basecamp._person_id`, not just the id rule, so the sync and async base files now import the same function object — pinned by a test asserting identity rather than agreement, because agreement is what they had before they drifted. One walk for Go's two, with the idempotence argument in the docstring and pinned twice: a `creator` that also carries `personable_type`, and idempotence asserted over all 74 rows. Go's type assertions are mirrored, so `creator: "me"`, `creator: 7`, `participants: {}` and `participants: ["7", 7, None]` are skipped. TWO EXISTING TESTS ASSERTED THE DEFECT and both are corrected rather than deleted. `test_edit_refuses_malformed_participants_before_writing[string-id]` pinned the raise above; it is replaced by a `range-string-id` row (`"9223372036854775808"`) which still raises, because the normalizer leaves a RANGE string exactly as it arrived and this guard is the reader that refuses it. The other four rows stay. And the recording-summary creator table now expects a `system_label` beside the id on its sentinel rows, because that creator sits at a `creator` key and the second pass reaches it first; Go has no label there, which is the pre-existing Python breadth difference the file already documents as Python-only — additive, an extra key beside an id Go agrees on, never a different id. Verified independently that this cannot reach the cross-SDK comparison: every one of the 68 creator ids in `conformance/tests/recording_summary.json` is an integer, so no label is ever produced there. Scope pinned so it cannot drift: the second pass is keyed on two names, not on "anything person-shaped". `assignees` is not one of them, and Go does not widen it either, so a string id there still needs `personable_type`. 2918 passed, 4 skipped. ruff, mypy and the service-drift check all clean. Found by adversarial review (Opus) of the head intended for merge. * fix(ruby): normalize an embedded person that omits personable_type, and stop to_i minting a bignum id Three defects from the adversarial review, all on the same surface. THE SECOND PASS. Ruby implemented one of the reference's two normalizer passes, and my reason for thinking that was safe — "person_from_wire converts the string at read time anyway" — was wrong for Ruby: every generated service returns a plain Hash, so nothing converts. `normalizeEmbeddedPeopleJSON` also finds people by structural position — the `creator` object and each `participants` element, at any depth, REGARDLESS of the tag — because embedded creator and participant people frequently omit it, so the tag-keyed pass skips exactly the payloads the second exists to fix. Divergences against the 74-row table: bare `creator` 62 → 0, `participants` element 62 → 0, `creator` nested three deep 62 → 0, tagged control 0 → 0. The twelve rows that "agreed" were the range rows, where leaving the string is what doing nothing looks like. One walk for the reference's two, and the equivalence was PROVEN rather than asserted: `coerce_person_id` now carries the reference's own guard (`normalize.go:41-44` — return unless the id is a String), so a second visit is a no-op, and the single walk was run against a literal two-walk transcription of `normalize.go` over 19 document shapes x 74 wires = 1406 documents: 0 mismatches, and 0 documents changed by normalizing twice. Both are pinned. Deliberately not widened past `creator`/`participants`, with a negative control asserting `assignees` and a bare `person` key stay untouched — the reference reaches those by tag or not at all. THE WRITE PATH, which is the half a caller feels. Measured through `MergeSafe.writable_id_list` on a participants element with a string id and no tag: before, all 74 rows raised; after, 62 accept and 12 raise — and those 12 are exactly the out-of-range rows, where the reference also leaves the string and its own decoder fails the READ, so refusing the WRITE is the matching direction. A PRE-EXISTING TEST PINNED THE DEFECT: the schedules guard table refused `{"id" => "1049715914"}` as "a non-integer id", which is precisely the merge-safe update BC3's own data was blocking. Replaced with an out-of-range id string that is still malformed after normalization, plus a float id, so the guard stays held for the right reason rather than the wrong one. THE BIGNUM, which this branch introduced. `Types::Person` read its id with `to_i`, so `"18446744073709551616x"` — now correctly left as a string for the reader to refuse — became the bignum 18446744073709551616, an id that is not even int64, where the reference fails the read. Divergences at `Types::Person#id`: 29 → 0 raw, 12 → 0 after normalization; that row is now nil. Fixed in the GENERATOR, not its output: `ruby/scripts/generate-types.rb` already keyed on `x-go-type: ...FlexibleInt64` to emit the `system_label` accessor, and exactly one field in `openapi.json` carries that marker, so the flexible-id branch goes beside it and `types.rb` was regenerated. Nothing under `generated/` was hand-edited; `make rb-check-drift` confirms it. 1770 runs, 33625 assertions, 0 failures. Drift clean, rubocop clean. Found by adversarial review (Opus) of the head intended for merge. * fix(typescript): derive the Person-valued keys from the spec, and refuse a mention it cannot name Two defects from the adversarial review, and the first says the earlier two-pass fix copied the wrong mechanism. THE KEY SET. Go never needs the normalizer for a typed read: `generated.Person.Id` is `types.FlexibleInt64`, so EVERY Person-typed field converts at decode. `normalizeEmbeddedPeopleJSON` exists only for the three wrapper paths whose `basecamp.Person.ID` is a plain int64 — so `creator`/`participants` is that wrapper's list, not the model's. TypeScript, which has no decoder at all, was still handing back `"007"` for `assignees`, `subscribers` and `completion_subscribers`, and `writableIdList` then refused it — a merge-safe update blocked by data BC3 controls. `PERSON_VALUED_KEYS` is now derived from the spec rather than typed out: every property whose schema `$ref`s `Person`, or an array of it. Twelve keys. A test recomputes the set from `openapi-stripped.json` and fails on any disagreement. THE GUARD EARNED ITSELF WITHIN THE HOUR. `performed_by` arrived on `Event` and `WebhookEvent` with #898 while this branch was being rebased — 450 schemas became 457 — and the check named it before anyone read the diff. A hand-maintained list would have rotted silently on its first day, which is precisely the failure this finding was. That is now the comment's stated reason for the guard. Three things the reference forced beyond a list of names. Arity is honoured, because Go's wrapper asserts `map[string]any` for `creator` and `[]any` for `participants` while `Assignees []Person` refuses a non-array — an object under `assignees` is a malformed body, not a person to coerce. The derivation asserts non-vacuity before comparing, since a walk that silently stops matching would otherwise "agree" with any set. And only `$ref: Person` counts: `OutOfOfficePerson` and its kin are person-SHAPED but plain int64 in Go. Divergences per position, 74 rows each, through the real pipeline: the five new object positions and the five new array positions were 62 of 74 and are now 11, every one of those 62 being the string reaching the caller untouched. The 11 are the unrepresentable-value residual, identical at every position. A PRE-EXISTING TEST PINNED THE DEFECT here too: an `it.each` row asserting `update` REFUSES a string assignee id. Removed — float, NaN, null and boolean still refused — and replaced with the positive case: a GET carrying `assignees: [{id: "007"}]` now PUTs `assignee_ids: [7]`. THE DROPPED MENTIONS. `mentionedPersonIds` silently skipped an id past 2^53: Go returned 5 for a six-mention text, Ruby 5, this 2, with no error. It now throws, naming the id. The argument, which is in the doc: unlike the normalizer, this returns `number[]` — there is no string to leave in place and no room to say "and one more I could not name". Under-reporting a set is the one failure a caller cannot detect, because the evidence it would need is the thing that was dropped, and it feeds the list deciding who gets notified. `mentions.ts`'s "every `<bc-attachment>` counts" now says so, with `@throws`. An id past INT64 is deliberately not that case: `ParseInt` raises, the reference answers "not a person", and so does this — skipped silently, as it skips it. Both boundaries pinned. One refactor beyond the brief, flagged by its author rather than slipped in: telling those two cases apart needs information `number | undefined` cannot carry, so the gid parse body moved to an internal `parsePersonSGID` and `personIdFromSGID` is a wrapper over it. Every answer is unchanged, and the tail is now literally the reference's shape — `scanPersonId` plus `id <= 0`, compared as a bigint — where `Number()` plus `isSafeInteger` stood, so the safe-integer boundary no longer decides identity. I re-ran the mutation check myself after the refactor: removing the `[0-9]` walk still fails with "+7: expected 7 to be undefined". Two gaps reported rather than hidden, both documented in the code: a response whose ROOT is a Person is unreached when the body omits `personable_type`, since the normalizer cannot know the operation's response schema; and on the new decoder-only positions TS writes a `system_label` where Go's generated path has nowhere to put one, and leaves the string on a range refusal where `FlexibleInt64` fails the read. Both additive or residual, neither a different id. 93 files, 1913 tests passed. No drift. Found by adversarial review (Opus) of the head intended for merge. * docs: two comments claimed the reference leaves an assignee id alone; it does not Ruby's `coerce_embedded_person_ids` said widening to `"assignees"` "would coerce ids the reference leaves alone", and Python's scope test said "Go does not widen it either". Both are true of Go's NORMALIZER and false of Go. The reference spells this rule twice. `normalizeEmbeddedPersonIds` covers `creator` and `participants` for the wrapper types whose `Person.ID` is a plain int64, and the generated DECODER covers every other person-valued field, because `generated.Person.Id` is a `types.FlexibleInt64`. So Go's observable answer at `assignees` is the number, reached by the other route — and a reader who took these comments at face value would conclude the string is correct there. It is not. Ruby and Python have no decoder on the generated path, so that second route does not exist for them and an `assignees` id that arrives as a string stays one — measured on a write, where merge-safe refuses a body the reference completes. Both comments now say that plainly, and say why the gap is not closed by widening the list: the faithful site is the reader, field by field, because some person ids in the model really are plain int64 there (`TemplateLibraryConfirmationPerson`) and a blanket sweep would break them. No behaviour changes. This is the comment-accuracy half of the same finding, and it matters because the branch's own review caught two other comments stating measured facts that were not measured. A comment that quietly licenses the defect is worse than no comment. * fix(kotlin): the number path has no sentinel either, and 007 is not a JSON integer `FlexibleLongSerializer`'s NUMBER path — the half the string-path work did not touch — answered the system actor where Go fails the read. Seven of 25 bare literals diverged; 0 now. [7] [] {"a":7} {} read 0 Go fails the read 1e3 1E3 read 1000 Go fails the read 007 read 7 Go fails the read The arrays and objects fell off the end of `deserialize` to a trailing `return 0L`, and 0 is `LocalPerson` / `"basecamp"` / `"campfire"` — the system actor, on the one field that says who acted. `1e3` came through `JsonPrimitive.long`, which in kotlinx 1.11.0 accepts an exponent; the comment claiming it is `content.toLong()` was stale. `007` came through kotlinx's lenient number lexer. THE FIX IS TWO STAGES, BECAUSE THE REFERENCE IS TWO STAGES. `encoding/json` validates the token before it ever calls `UnmarshalJSON`, and only then does `json.Number.Int64()` run `ParseInt(text, 10, 64)`. So: refuse a non-primitive outright, then apply a JSON-integer token grammar, then hand the survivor to the existing `parseInt64`. Both stages are load-bearing — reusing `parseInt64` alone would have REGRESSED `+7` from a refusal to 7, since `ParseInt` takes a leading sign and the lexer hands `+7` through as a literal. It is also what fixes `007` for free: that is not valid JSON, and Go scans the document for validity before dispatching. There is no sentinel on this path at all. `"basecamp"` is a string, so only the quoted path can answer 0 without having read a number. `null` is decided rather than inherited: Go decodes it into an empty `json.Number` and `ParseInt("")` fails, so `{"id": null}` fails the read while an absent id is the zero value. Kotlin reaches the same verdict by a different route — the literal's text is "null" — and both the route and the asymmetry are written down at the guard, crediting Ruby's `person_from_wire`, which measured it. MUTATIONS, one guard at a time, and M2 is the one that earns its keep: dropping the whole grammar check fails only at `007`, NOT at `1e3`/`null`/`true`, because `parseInt64`'s syntax branch already catches those. That is exactly the overlapping-guard trap this branch hit twice before, so each guard's unique contribution is pinned separately. M6 kills nothing and is reported as such rather than given an invented row: the `Syntax -> 0L` arm is unreachable behind the grammar check, and the code says so. ONE ASSERTION REMOVED FROM ANOTHER TEST, deliberately and after checking who depends on it. `DecodeIsolationTest` asserted the inner cause beneath the mapped error is a `NumberFormatException`. Nothing reaches a numeric conversion that can raise any more, so there is no numeric original to carry. Verified independently that `cause.cause` is read in that one test and nowhere in main, the generator or the conformance runner, while the composites and the runner read the OUTER type through `decodeFailure`, which is untouched and still asserted. The guarantee moved from a catch to the shape of the code, and the KDoc now says that rather than describing a leak that no longer exists. RESIDUAL, pinned as measured rather than fixed: an absent `id` is a `MissingFieldException` where Go reads 0. That is model-layer required-field handling for a spec-required field — the serializer is never called — and it errs in the REFUSING direction, declining to invent a person rather than naming actor 0. Noted for the other ports, not fixed here: kotlinx's lenient lexer means `007` reads as 7 for EVERY numeric field in this SDK, not only this one. Go fails the read on all of them. `:basecamp-sdk:check` BUILD SUCCESSFUL, 717 tests, 0 failures. `make kt-test` BUILD SUCCESSFUL. Found by adversarial review (Opus) of the head intended for merge. * fix(typescript): skip a mention no number can carry, and say so on the summary `mentionedPersonIds` threw on a mention whose person id is a valid int64 past 2^53, so that a short list could not pass for a short text. That reasoning is sound and the throw was still the wrong instrument, because of where it runs. `recordings.summarize()` calls it on server-returned `content`, and no sgid signature is verified on that path: `globalIDFromSGID` takes a bare unsigned envelope, so the id inside it is chosen by whoever wrote the comment. One crafted `<bc-attachment>` therefore failed the whole summary -- the content, the title, the creator and every other mention went with the one that could not be named. A denial of read, reachable by any Basecamp user who can comment. It now skips, which is also the reference's own failure mode: `MentionedPersonIDs` (go/pkg/basecamp/mentions.go:82-92) continues past every sgid `PersonIDFromSGID` declines and never fails the text. Go simply has fewer to decline, because an int64 fits its return type. The skip is not silent. `readMentions` returns the same list alongside the ids it could not name, as the decimal strings they arrived as -- strings, because these are exactly the ids a number would round into a neighbouring person. The summary carries them as `unnameable_mention_ids`, a key present only when the mention list is actually short, so no existing payload changes shape. Only TypeScript threw. Ruby and Python ints are arbitrary-precision and both already skipped a declined sgid. Proven to fail: restoring the throw turns three tests red for the stated reason, the summarize one among them -- still reads a recording whose rich text carries an unnameable mention BasecampError: rich text mentions person 9007199254740993, whose id does not fit a JavaScript number Decided by Jorge on card 35 (comment 10309557349): keep the refusal off the summarize path, skip with a distinguishable signal there. * fix: run the positional person-id pass only where the reference runs it Go's positional pass -- `normalizeEmbeddedPersonIds`, which finds a person under `creator` and each `participants` element whether or not it carries `personable_type` -- runs only where `normalizeEmbeddedPeopleJSON` is called: `decodeGaugePayload` (gauges.go:170) and the notification decoders (my_notifications.go:171, 281, 296). Nowhere else. Every other person id in Go converts at DECODE, because `generated.Person.Id` is `types.FlexibleInt64`. Ruby, Python and TypeScript ran it on every response body. TypeScript went further and matched twelve spec-derived keys. Neither was safe, because the keys are matched by name and are not unique to the wrapper types. They reach schemas whose person id is a plain int64 in the reference, where a string is a decode error -- Go: json: cannot unmarshal string into Go struct field UpcomingScheduleEntry.creator.id of type int64 here: {"creator": {"id": 0, "system_label": "basecamp"}} -- the SYSTEM ACTOR, on the field that says who acted, for a body the reference refuses outright. A divergence in the accepting direction on an identity field, which is the class this branch exists to remove. Six sites, now strict: UpcomingSchedulePerson UpcomingScheduleEntry.creator, .participants, UpcomingAssignable.assignees, UpcomingAssignableCompletion.creator MyAssignmentAssignee MyAssignment.assignees OutOfOfficePerson DisableOutOfOfficeOutput.person Latent -- BC3 sends integers at all six in spec/fixtures/schedules/upcoming.json -- and still wrong. None of the operations serving them carries a genuine Person field at any depth, so leaving them strict costs no coverage. Each port now gates the pass on the reference's two surfaces: TypeScript on the operation's service ("Gauges", "MyNotifications"), Ruby and Python on the request path, because Go's own boundary is the call site and Ruby's update_gauge_needle passes no operation id. The personable_type pass keeps its reach: an object declaring that key IS the Person projection. THIS WALKS BACK A MEASURED CLAIM, and the cost is stated rather than hidden. The wider reach was covering a real gap: with no decoder, these three ports now leave a string id in `assignees`, `subscribers`, `completion_subscribers` and schedule `participants` as a string, where Go's decoder reads the number, and the merge-safe composites refuse such a body before writing. That is the refusing direction -- no id invented, no partial update sent -- and it is decoder coverage, field by field, not normalizer reach. PR #913 owns it. The tests that pinned the wider behaviour now pin the refusal, each saying so, as the ones to flip when #913 lands. One summarize expectation moves the other way, toward the reference. Python's summary creator for a sentinel id carried a `system_label`, and only because the positional pass wrote one onto the recording's untagged creator first. Go's summary reads through `personFromGenerated` (people.go:920), which never sets `SystemLabel`, so the reference's answer is `{"id": 0}`. It is now. The negative test in each port is proven to fail on the previous head -- with the gate removed, a strict site becomes the system actor: typescript AssertionError: expected +0 to be 'basecamp' ruby Expected: "basecamp" Actual: 0 python AssertionError: assert 0 == 'basecamp' and each converted refusal test goes red the same way (ApiError expected but nothing was raised), because unscoped the participant string was normalized. Decided by Jorge on card 35 (comment 10309557349): narrow to Go's two surfaces. * perf(python): scan a signed person id from an offset instead of copying it `parse_int64` sliced `text[1:]` for a signed id, copying the whole remaining string before a scan that reaches its verdict within about 20 digits -- the per-digit `magnitude > _UINT64_MAX` check refuses there. So a long malformed id cost a copy of itself for nothing, bounded only by the response body cap, which limits the body rather than any one id inside it. It now takes an offset and walks the string lazily with `islice`, which keeps the ASCII byte test and the per-digit overflow check exactly where they were. Behaviour-neutral, so it adds no test: the 74-row corpus in every position is the regression test, and all 630 rows pass unchanged. Raised by Copilot on #908. Swift's sibling finding (`Array(text.utf8)`) is not taken here: `String.UTF8View` is not Int-indexable, so it is a loop rewrite rather than a line, and Swift does not build on this machine to prove it. * docs(spec): state where the positional pass runs, and the mention trade-off §10 said a person is found two ways and did not say where the second way runs, which is how three ports came to run it everywhere. It now states the rule -- the reference's call sites, gauges and notifications, and no wider -- names the six strict sites the wider reach turned into the system actor, and says what that reach had been covering and where that gap is tracked (#913), so a port implementer meets the boundary and the reason for it in the same place. The TypeScript row records the mentionedPersonIds skip as the availability trade-off it is: an under-report, deliberately chosen over a throw that let one crafted comment make a recording unreadable, and made visible through readMentions and the summary's unnameable_mention_ids. * fix(typescript): count each unnameable mention once, and none on a plain-text line Two defects in the distinguishable signal added for skipped mentions, both found by adversarial review (Opus) of the head intended for merge. `unnameable` was deduplicated on the raw digits. The reference deduplicates on the int64 (go/pkg/basecamp/mentions.go:87), so `9223372036854775807` and `0009223372036854775807` are one person -- and the signal exists precisely so a caller can see how short the mention list is. On the branch's own six-mention text Go names 5 people; `ids` plus `unnameable` came to 6. The unrepresentable case now carries the CANONICAL decimal, formatted from the bigint the scan already produced, and is deduplicated on that. The count matches the reference, and a leading-zero spelling reports digits a caller can match against a person id. A plain-text chat line kept `unnameable_mention_ids`. The projection runs the rich-text walk over `content` and only then learns the line is not rich text, where it resets `mentioned_person_ids` to `[]` -- but not the new key. So a line BC3 never read as markup reported an unnameable mention of someone it does not mention, in Go or here. The key is now dropped with the list it belongs to. The `MentionedPerson` doc still said `mentionedPersonIds` "has to refuse rather than drop" such a mention; it now says what the code does. Proven to fail on the previous head: dedup expected [ '9007199254740992', ...(3) ] to deeply equal [ '9007199254740992', ...(2) ] chat line expected true to be false ("unnameable_mention_ids" in text) * test: pin both halves of the positional-pass gate, and anchor it to the path The gate added to run the positional pass only on Go's two surfaces was correct on reading and half-unpinned in every port. Adversarial review (Opus) mutated each piece and the suites stayed green: typescript drop "Gauges" from the gate 1935 pass wrong service on followed pages 1935 pass ruby embedded_people: false in Response#json+parse_page 1780 pass python async gate always False 2939 pass every paginated sync site forced False 2939 pass The only pinned positive case anywhere was the first page of a notifications read. Each hole now has a test driven through the real client, and each is proven to go red under the exact mutation that used to pass: typescript gaugeNeedle, untagged creator expected '007' to be 7 listGaugeNeedles, page 2 expected '+8' to be 8 ruby get_gauge_needle / my notifications Expected: 7 Actual: "007" list_gauge_needles, both pages Expected: [7, 8] Actual: ["007", "+8"] upcoming report, through HTTP (fails when the gate always matches) python async get_gauge_needle, async and 2 failed sync list_gauge_needles on 2 pages 1 failed Two gate fixes ride with them: ANCHORED TO THE END OF THE PATH. Ruby and Python matched unanchored patterns against the whole URL, host included, so a base URL whose own path contained `/gauge_needles/` would have switched the pass on for every request. Each pattern now matches only the URL's path, anchored at its end, per surface (`/my/readings.json`, `/my/readings/bubble_ups.json`, `/gauge_needles/<id>`, `/projects/<id>/gauge/needles.json`, `/reports/gauges.json`). Proven: under the old patterns, a proxy prefix, a trailing segment and a nested path all switch the pass on (python: 4 of 5 cases fail). `httpx.Response.request` RAISES RuntimeError when unset; it is never merely absent, so `getattr(response, "request", None)` let the error out of the gate instead of defaulting. Proven: `RuntimeError: The request instance has not been set on this response.` Also corrects `_person_id.py`'s docstring, which still presented the pass as the fix for `schedules.edit_entry` two paragraphs before the section that walks that back, and an islice comment that said the scan decides "within ~20 digits" without saying leading zeros are walked in full, as Go walks them. * docs: say what Go's summary actually does, and list the whole decoder gap Two statements this round made were wrong, and adversarial review (Opus) checked both against a probe of the reference. The Python summarize table said `{"id": 0}` is "what Go's summary carries" for a sentinel creator. Not for this fixture, which sends a creator with no name: `commentFromGenerated` sets the creator only when `Id != 0 || Name != ""` (go/pkg/basecamp/comments.go:364), so Go's summary has no `creator` key at all. With a name, Go gives `{"id": 0, "name": ...}` -- which is what dropping the label now matches. The comment now says both, and names the presence rule as a pre-existing projection divergence (Python keeps `{"id": 0}`; Ruby omits the creator, as Go does) left untouched rather than folded into a person-id change. The contradicting comment block above it, which still explained a label that is no longer there, is gone. SPEC §10's list of what the narrowed pass leaves as a string was incomplete. It named `assignees`, `subscribers`, `completion_subscribers` and schedule `participants`. It now also names an untagged `creator` on every recording read in all three decoder-less ports, and TypeScript's further seven keys (`approver`, `booster`, `completer`, `performed_by`, `granted`, `revoked`, `person`). All of it is decoder coverage and belongs to #913. It also says where the gap is VISIBLE, and that was measured on the rebased code, not taken from the review: a plain generated read. `comments.get` returns an untagged `creator.id` of "7" as the string "7" in TypeScript, Ruby and Python alike, where Go gives 7. The recording-summary composites do not show it -- since #911 all three decode a nested person's id as a FlexibleInt64, so `recordings.summarize()` gives 7 in every port. (Before rebasing onto #911 this line claimed the opposite for TypeScript; it was true of that base and is not of this one.) * test(typescript): a tagged person id past 2^53 is refused, not the system actor #911 landed a summarize test whose last assertion pinned the behaviour this branch removes. For a creator carrying `personable_type` and the id "9223372036854775807" it expected person 0, because `main`'s pre-pass collapsed every id past Number.MAX_SAFE_INTEGER into 0-plus-system_label -- the system actor, for a real person Go reads exactly. Its comment called that the behaviour "a caller actually sees on a real person", and at the time it was. That collapse is one of the defects #908 fixes. The pre-pass now reads the id Go reads and, unable to hold it in a number, leaves the string for the reader, which refuses it with the same "exactly" message it gives the untagged spelling a few lines above. So tagged and untagged now agree, and neither names the system actor. The two PRs were each correct against their own base; this is the merge of them, found by running the suite after rebasing onto #911. The assertion now expects the refusal. Proven to discriminate: restoring `main`'s collapse in `coercePersonId` makes it fail, because a summary comes back instead -- expected { id: 1, status: '', type: '', ...(7) } to be an instance of BasecampError * docs(python): TypeScript also keeps a nameless id-0 creator that Go omits The summarize table's note on the pre-existing presence divergence said "Python keeps {"id": 0} ... Ruby omits it". TypeScript keeps it too: the convergence review probed `recordings.summarize` and a nameless sentinel creator comes back as {"id": 0} there, where Go's `commentFromGenerated` drops the creator entirely. So the sentence under-counted which ports diverge. It now names both. Comment only.
This was referenced Sep 16, 2026
Merged
jeremy
added a commit
that referenced
this pull request
Sep 18, 2026
…on every operation (#922) * Give the recordings operations their own tag so MCP catalogs it as its own domain basecamp/mcp's catalog.Load groups SDK operations into MCP domain tools by their OpenAPI tag, one tag per operation. The six recording-lifecycle operations — ListRecordings, SpotlightRecording, UnspotlightRecording, TrashRecording, ArchiveRecording, UnarchiveRecording — carried the Automation tag, so adopting the SDK's native tag mapping would fold basecamp_recordings into basecamp_admin and silently drop the deliberately-designed recordings tool surface. (The account event-feed surface already got its own EventFeed tag in #898; this closes the matching gap for recordings.) Retag the six under a new Recordings tag, mirroring the Recordings service every SDK generator already emits for exactly these operations. Recording boosts stay under Boosts and the recording timesheet stays under Schedule -> Timesheets, matching the SDK service groupings. Each generator assigns services from its own split table rather than from the OpenAPI tag, so all seven SDKs regenerate byte-identical; the artifacts that copy the tag verbatim change: openapi.json, the go url-routes resource field (which now reads "Recordings" for these routes instead of the stale "Automation"), and the TypeScript vendored openapi-stripped.json. Regenerated via make smithy-build, make url-routes, and the TypeScript generator. behavior-model.json is unchanged: tags live only in the OpenAPI projection. * Fail closed when an operation reaches openapi.json without exactly one tag The recordings-into-Automation fold happened because nothing enforced that every operation carries its own tag: an operation added to the Smithy model without a matching entry in spec/overlays/tags.smithy reaches openapi.json untagged, and catalog.Load then drops it or folds it into an unrelated domain. This is an accident-class guard — anyone who can add an operation can add a tag; the check exists so they cannot do the first and forget the second. check-required-tags.rb reads the committed openapi.json — the same artifact catalog.Load consumes — and fails if any operation has zero tags or more than one. The allowlist is empty and documented: every Basecamp API operation is a catalog operation and must carry a domain tag; a genuinely tag-less operation would be added by operationId with a reason rather than by weakening the check. It fails closed on a spec that yields no operations, so a truncated openapi.json cannot pass vacuously. The smithy-verify "OpenAPI is up to date" step already proves openapi.json is regenerated from the model, so a dropped tag reaches this gate rather than hiding behind a stale file. test-check-required-tags.rb drives the check with crafted openapi documents (untagged, empty-tagged, multi-tagged, allowlisted, empty, and path-level parameters) since the live run only ever exercises the passing case. Both are wired into make check and the spec-gates CI job (bash/ruby, no toolchain). * Check every HTTP method a path item can carry, not just the five in use The tag gate only visited get/post/put/patch/delete, so an operation on options, head or trace reached openapi.json unvisited: a tagged GET beside an untagged HEAD passed and reported one operation. Visit all eight methods an OpenAPI path item may carry, matching check-projected-examples.rb. The self-test drives the tagged-GET-plus-untagged-HEAD document that used to pass, and asserts an untagged operation is caught on each of the eight. * Record what the tag gate sees that the generators do not * Identify operations by what they are not, so a new verb cannot slip past Enumerating the operation verbs made a fail-closed gate fail open in the one dimension it exists to hold: the list is only ever as complete as its last edit. It missed options/head/trace, and lengthening it would not have been a fix. OpenAPI 3.2 adds query and additionalOperations, and Smithy's @http trait takes the method as a free-form string it performs no validation on, so this repo's own generator can put a key in a path item that no list anticipates. A Path Item Object's non-operation fields are a closed, spec-defined set and its extensions are x- prefixed. Skip those; everything else is an operation. A field that is neither is now reported by name instead of stepped over. With the verb list restored to eight, the self-test still fails five cases. * Require the one tag to name a domain, not merely to be present An operation tagged [""] satisfied exactly-one while naming nothing, and the generators do not agree what that means: TypeScript reads "" as falsy and files the operation under Miscellaneous, while Ruby and Python read it as truthy and derive a service from the empty string. One blank tag would split the SDKs rather than just mis-file an operation. The sole tag must now be a non-blank string. A non-array tags value counts as untagged instead of raising. Surrounding whitespace still passes, so the rule is about naming a domain and not about trimming. * Retire the Automation split entries the retag made unreachable, and pin the router's resource Each tag-keyed generator looked the recording operations up under the Automation split; they carry the Recordings tag now, so the lookup misses and the tag-derived service name produces the same output. The entries are dead configuration describing an assignment that no longer happens. Regenerating without them is byte-identical in all five: ruby, python, typescript, swift and kotlin drift checks all clean, as are the operation-assignment and service-inventory parity gates. Rust's names.toml keys off operationId rather than the tag, so its entries stay live and unchanged. The route table's resource label for recordings went Automation -> Recordings in the same retag, and that field is public on Match while no test asserted it. TestRouterMatch now carries a wantResource column, filled in for the two existing bucket cases and for three recording routes. Reverting the five route labels fails those three. * Refuse a path item behind a $ref, and read webhooks as well as paths Two places an operation could sit unseen, both the same shape as the verb list: the check read paths alone, and it skipped $ref as a non-operation field. It is not an operation, but it points at a path item whose operations cannot be counted without resolving the reference, so skipping it under-counts silently — a $ref beside one real operation kept seen nonzero and passed. An unresolved $ref is now refused by name rather than skipped, and webhooks are scanned through the same routine as paths, so a tagged webhook operation counts and an untagged one fails. That leaves components.pathItems reachable only through a $ref, which now fails, so the enumerable places an operation can hide are all either read or refused. * Report a malformed webhooks value instead of ignoring it The webhooks scan coerced a non-object to an empty hash, which stepped over the one construct the rest of this check refuses to step over. Say what could not be read, the same way a non-object path item is already reported. * Describe the gap that is actually left, not the one that was closed The header still said the check reads paths only and would miss webhooks and components.pathItems. It reads both places path items live now; what remains is that it refuses a $ref rather than following one. Say that, and point the generator hole at its issue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Models the account event feed's wire surface, now merged on bc3
master, and regenerates all seven SDKs from it. Three operations on a newEventFeedtag (serviceeventFeed):PollEventsGET /{accountId}/events.json@readonly;since(decimal int64 id,0, ornow),position, and the comma-joined filterstypes/buckets/creators/performers/exclude_performers/actor_typesPollInboxGET /{accountId}/inbox.json@readonly;since,position,reasons/types/buckets; 403 for every non-agent principalCreateStreamTicketPOST /{accountId}/events/stream_ticket.json@idempotentbecause the mint is a stateless signed capability (safe to retry, not a claim of an identical response)Shapes:
FeedEvent(with a nullableperformed_by_idand adetailsdocument, carried verbatim — only forboost.created/card.movedtoday; a typed projection would drop the explicit nulls a recording-level boost sends and any member a newly cataloged type adds, so the poll lane would disagree with the connector's push lane),PollEventsOutput/PollInboxOutputenvelopes (events|items,position,next?),InboxItem, and the mint's{ticket, expires_in, url}withticketandurlmarked sensitive (recorded inbehavior-model.json's redaction map and rendered as Rust'sSensitiveString; no SDK logs response bodies, and the §23 connector never renders either value). Typed error bodies (revised after peer review):FeedRequestError(400 on both poll lanes:errorplus an optionalreasonofinvalid_position|invalid_filterfrom bc3 #13362 — absent means an undifferentiated 400 the consumer surfaces rather than guessing between recovering and stopping),FeedFilterMismatchError(409:position_digest,filters_digest), and two deliberately distinct 410 shapes on two operations:FeedPositionGoneErroronPollEvents(epoch_after_idrequired;resumere-enters at the epoch) andInboxPositionGoneErroronPollInbox(resumere-enters atsince=0; no epoch) — so no singleerrors.Asarm can silently handle the wrong lane.PollInbox's 403 isBareForbiddenError(bodyless, followingBareNotFoundError).CreateStreamTicketkeeps@basecampIdempotent(natural: true): Ruby, TypeScript and Go classify retry off that flag, and dropping it failscheck-idempotency-parity.Pagination is deliberately not the Link-header walk: the body envelope's
positionis the durable cursor andnextthe continuation URL, so the operations carry no pagination trait and are wired into no SDK's paginator — one call is one page. The operation docs and SPEC §23 "Wire Operations" say so.Also absorbed from the same bc3 range: the optional
performed_by: Personmember onEvent(recording events) andWebhookEvent(bc3 #13040).Person.personable_typewas already an open string in every SDK, so the newAgent/Tombstonevalues decode without a closed enum to widen.Contract sources (bc3
master):What changed
spec/basecamp.smithy(operations, shapes, error shapes,@sensitiveticket types,performed_by),spec/overlays/tags.smithy;openapi.json,behavior-model.json,url-routes.jsonregenerated.make generate), plus theeventFeedservice wired into the TypeScript, Ruby and Python clients;RESOURCE_TYPE_OVERRIDESgainPollEvents → feed_eventandPollInbox → inbox_itemin the four generators that emit resource types (Pollis not a verb pattern, so inference fell through toresource).AccountClient.EventFeed()→EventFeedServicewithPollEvents/PollInbox/CreateStreamTicket, typed*FeedRequestError(withReason),*FeedFilterMismatchError,*FeedPositionGoneError(feed only) and*InboxPositionGoneError(inbox only), each unwrapping to the canonical*Error, andPollEventsOptionsFromURL/PollInboxOptionsFromURLto follow anextorresumeURL through the generated operation (query-only parse; origin validation stays with the SPEC §23 connector).Event.PerformedByandWebhookEvent.PerformedByon the existing wrappers.performed_by_idand bothdetailsvariants, 400/409/410 mapping, the inbox's agents-only 403, the bodyless mint and its 401); Go wrapper tests for the same plus the typed errors and URL parsers.conformance/tests/event_feed.json(12 cases, including the 400 with and withoutreason) dispatched from all seven runners with flat per-runner summaries; registered in SPEC §19's categories table and Appendix D.spec/bc3-route-allowlist.ymlwaives the three routes with routes.rb / controller / test evidence (they are documented past the current provenance pin — see out of scope);scripts/check-idempotency-paritycounts +1 idempotent / +3 union;go/grouped-client-inventory.ymllists the three as not grouped; Rustguarantees.rspins moved to 265 operations.detailsper language: Gojson.RawMessage(the bytes, untouched; the wrapper'sFeedEvent.Detailstoo), TypeScriptunknown, PythonAny, Ruby a Hash, KotlinJsonElement, Rustserde_json::Value, Swift the SDK'sJSONValue?— the Swift generator used to render every Smithy document asString?, which cannot decode an object, so it now maps documents toJSONValue(which gainedEncodable);WebhookEvent.detailschanges type with it, noted inMIGRATING.md, and numbers ride asDoublethere.spec/api-gaps/event-feed.mdanddelegated-events-performed-by.mdclose asabsorbed-in-sdk. AGENTS/SECURITY/README operation and service counts were rewritten bymake generate.Verification
Run locally on macOS (every exit code 0 unless noted):
make generate(clean re-run produces no diff)make check-targetsindividually (smithy-check, behavior-model-check, provenance-check, sync-api-version-check, doc-constants-check, url-routes-check, bc3-route-parity + its self-test, go-check-drift / go-check-wrapper-drift / go-check-generated-drift, check-grouped-client-coverage, auth-routable-check, check-service-inventory-parity, check-operation-assignment-parity, the five per-SDK drift checks, check-bucket-flat-parity, validate-api-gaps, check-deprecation-parity, check-fixture-coverage, kt-check-optional-arrays-and-scalars, go-check-optional-pointers, check-idempotency-parity, check-write-semantics-parity, check-retry-metadata-parity, check-runner-test-reachability, check-replay-decoder-parity, check-readme-env-vars, lint-npm-lockfile-writes, check-projected-examples, the OAuth / event-feed / conformance fixture checks, …)make go-check,make ts-check(1899 tests),make py-check(2094 tests),make rb-check(1745 runs),make rs-check,make kt-check,make swift-check(563 tests)make check-fixture-execution(runsmake conformance: all seven runners execute the 284 cases; the new family passes in every language)Live canary against a real account with the Go SDK on this branch, using a person-principal token:
PollEvents since=now→ 200, empty page with a position;PollEvents since=0withtypesandexclude_performers=self→ 200; a malformedposition→ the documented 400 (validation, "Unrecognized position…");PollInbox→ 403forbidden(agents only, as documented); oneCreateStreamTicket→ 200 withexpires_in: 120, a ticket, and awss://URL carrying it (nothing beyond lengths was recorded; the URL was not connected to — that is the connector's job).Out of scope, deliberately
TicketMinter/PollSourceadapters over these operations, thesrv2rename, the 410 re-entry semantics, and the new filter dimensions land in the event-feed connector PRs; the symbols above are what they bind to.spec/api-gaps/triage, so this PR follows the existing convention of evidenced route waivers marked "delete at the next repin".api_error, non-retryable) with the server's message; readingposition_digest/resumeoff a typed value is the connector adapter's concern in each language.TimelineEventalso rendersperformed_byupstream; onlyEventandWebhookEvent(the two the gap registry names) are modeled here.Release notes: Features (
enhancement). The API Compatibility gate flags the new generated Go interface methods (ClientInterface.PollEventsetc.), as it does for every added operation (#836, #833, #827, #847 all carriedbreaking), andEvent/WebhookEventgain an exportedPerformedByfield, which breaks unkeyed Go composite literals —MIGRATING.mdcarries the note under Unreleased. Applying thebreakinglabel needs triage rights, so a maintainer will have to add it for the gate to pass.