Seven SDKs read a person id seven ways, and Go disagreed with itself

[jorgemanrubia]

Every SDK here turns a person id that arrived as a JSON string into a number, because BC3
serializes one that way on some payloads and spells its system actors — LocalPerson,
whose id is "basecamp" or "campfire" — in the same string field. Go reads those actors
as 0.

Each SDK reached for its own language's integer parser, and none of those parsers is the
one the reference applies. Python's int() read a fullwidth "１２３" as person 123.
Kotlin's toLongOrNull did the same through Character.digit. TypeScript turned every id
past 2^53 — and every id with a leading + — into id: 0, which is to say into
LocalPerson. Swift's ICU-flavoured \d treated a fullwidth digit run as an overflow and
failed the whole response. Four ports read an oversized malformed id as the system actor
where Go raises.

And Go disagreed with itself. Its pre-decode normalizer held a ParseInt-valid string
verbatim as a json.Number, which the JSON encoder then refused — that grammar allows
neither a leading + nor leading zeros — so the marshal failed, every caller fell back to
the raw body, and the raw body still has a string where the wrapper's Person.ID is a
plain int64. An embedded creator with id "+7" or "007" failed the whole response
on the notification, gauge and bubble-up paths, while Go's own FlexibleInt64, reading the
same bytes, read 7.

Context

• 34 Go's own person-id paths disagree: FlexibleInt64 vs coercePersonID
• 35 Person-id normalizers accept ids Go reads as the system actor

One branch, because they are one surface. Follow-up filed as 42 Merge-safe writes refuse
assignee ids the reference accepts,
which is a different question — decoder coverage rather than normalizer grammar — and needs
a per-SDK, per-field audit rather than a sweep.

The rule

strconv.ParseInt(s, 10, 64), scan order included, written out once per language and
shared by that language's person-id sites. Not its documentation: ParseUint checks the
magnitude inside the scan and returns ErrRange the instant the accumulator would
overflow uint64, before it reaches the rest of the string, so "18446744073709551616x"
fails the read while "18446744073709551615x" — the same length, one smaller — is a syntax
refusal that reads 0. The pair differs by which refusal the scan reaches first: ...616
overflows uint64 mid-scan and returns ErrRange before it ever sees the x, while
...615 does not overflow, so the scan runs on into the x and returns ErrSyntax. Every port had that pair backwards, in the direction that names the system actor.

Its two refusals stay apart, because the SDK does different things with them: a syntax
refusal is the sentinel and reads 0; a range refusal leaves the value alone so the read
fails. No single "is this a number?" predicate can separate them.

The gid rule stays deliberately different, and every SDK now has a test saying so in
both directions. PersonIDFromSGID walks the bytes and refuses anything outside 0..=9
before parsing, so it rejects the leading + the other rule accepts — correct at that
site, and loosening it is the +77 defect
#886 closed. It is also not simply the
stricter of the two: leading zeros pass its walk, so a reader who "hardens" it by refusing
"007" breaks a test as surely as one who loosens it. Ruby and TypeScript had no such test
at all; both could have lost the guard silently.

Measured against a linked oracle

Expectations come from a probe linked against the real normalizeEmbeddedPeopleJSON, the
real types.FlexibleInt64 and the real PersonIDFromSGID, over a corpus built to hold the
shapes that discriminate. It is checked in as a dump mode on the Go test, so the next
reader re-derives the table rather than trusting a comment.

Divergences over the 74 rows, per site, before and after:

	normalizer	reader
Go (coercePersonID vs FlexibleInt64)	13 → 0	—
Rust	none to have	0 → 0
Ruby	3 → 0	3 → 0
Python	29 → 0	0 → 0
Kotlin	12 → 0	12 → 0
Swift	13 → 0	13 → 0
TypeScript	26 → 11	14 → 11

TypeScript's 11 are one residual, argued in the code: a JS number cannot carry an int64
past 2^53, so an id Go reads is reported unreadable rather than rounded to a different
person or collapsed to the system actor.

What adversarial review changed

A separate Opus reviewer read each head intended for merge, round after round. The findings
below are the ones that changed the code, in the order they were found; the last two are
defects this branch had itself introduced, and have their own section at the end.
The scan survived — 70 further corpus rows across five ports, 0 divergences; 26 mutations,
each guard mutated separately, all 26 caught — but the call paths did not.

The claim it broke was mine: that the other ports' decoders made a missing normalizer
pass unobservable. Go runs two passes, finding people by personable_type and by
structural position, because embedded creator and participant people frequently omit the
tag. Every port implemented one. Ruby and Python have no decoder at all, so both carried the
same 62 of 74 divergence at an untagged creator or participants element. Both ports
now run both passes, and Ruby proved its single walk equivalent to a literal two-walk
transcription over 1406 documents. Where that second pass runs turned out to matter as
much as whether it runs — see the last section.

Swift's number path answered the system actor for 9223372036854775808 — the exact
refusal this branch spent its length pinning on the string side — along with null, true,
[7], {"a":7} and 7.5. Twelve of a 22-row number corpus diverged; 0 now. Four rows
worse than the brief reported, and the one that matters most is
-9223372036854775809: the negative int64 boundary, read as 0, which is the id of the
actor that posts on nobody's behalf. 1e30, 0.0 and -0.0 came with it.

Kotlin's number path had the same hole, found the same way and fixed last: seven of 25
bare literals diverged, 0 now. [7], [], {"a":7} and {} fell off the end of
deserialize to a trailing return 0L; 1e3 came through JsonPrimitive.long, which
accepts an exponent in kotlinx 1.11.0; 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 UnmarshalJSON ever runs — and both are load-bearing: reusing parseInt64
alone would have regressed +7 from a refusal to 7. Each guard's unique contribution is
mutated separately, because dropping the whole grammar check fails only at 007, which is
the overlapping-guard trap this branch hit twice before.

And mentionedPersonIds silently dropped mentions: Go returned 5 for a six-mention
text, Ruby 5, TypeScript 2, with no error. It briefly threw instead — and the last round
showed the throw was worse. See below.

What the last review round walked back

The final round — Copilot's re-review and a fresh adversarial Opus review, both on
32ce521f — found two defects this branch introduced. Both are fixed, and both fixes
were decided by Jorge on
card 35:
"1) keep the refusal off the summarize path, skip with a distinguishable signal there
2) narrow to Go's two surfaces".

1. A crafted comment could make a recording unreadable. To stop mentionedPersonIds
under-reporting, TypeScript made it throw on a mention whose id is a valid int64 past
2^53. But recordings.summarize() calls it on server-returned content, and no sgid
signature is verified on that path — the id is chosen by whoever wrote the comment. One
crafted <bc-attachment> failed the whole summary: a denial of read, reachable by any
user who can comment, on the one input in the SDK that is untrusted by construction.

It now skips, which is also the reference's own failure mode — MentionedPersonIDs
continues past every sgid it declines. The skip is not silent: readMentions returns the
ids it could not name, as decimal strings, and 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. The under-report is real and is now an availability
trade-off stated in SPEC §10 rather than a throw.

2. The positional pass reached schemas the reference refuses — so the measured write
fix is walked back. Go runs its positional pass only where normalizeEmbeddedPeopleJSON
is called: decodeGaugePayload (gauges.go:170) and the notification decoders
(my_notifications.go:171,281,296). Everywhere else Go converts at decode, because
generated.Person.Id is FlexibleInt64. This branch ran the pass on every response in
Ruby, Python and TypeScript, and TypeScript widened it to twelve spec-derived keys to
stand in for the decoder it lacks.

That was a divergence in the accepting direction on an identity field. The keys are
matched by name and are not unique to the wrapper types, so the pass reached schemas whose
person id is a plain int64 in the reference — where a string is a decode error — and
wrote person 0 with a system_label: the system actor, for a body Go refuses
outright. Measured latent (BC3 sends integers at all of them today in
spec/fixtures/schedules/upcoming.json), and still exactly the class this PR exists to
remove. The pass now runs only on Go's two surfaces, and these six sites are left
strict:

type (plain int64 id in Go)	field
UpcomingSchedulePerson	UpcomingScheduleEntry.creator
UpcomingSchedulePerson	UpcomingScheduleEntry.participants
UpcomingSchedulePerson	UpcomingAssignable.assignees
UpcomingSchedulePerson	UpcomingAssignableCompletion.creator
MyAssignmentAssignee	MyAssignment.assignees
OutOfOfficePerson	DisableOutOfOfficeOutput.person

None of the operations serving them carries a genuine Person field at any depth, so
leaving them strict costs nothing there.

What it does cost, stated plainly, because earlier versions of this description claimed
the opposite:

• The merge-safe write fix is gone. This description said Python's
  schedules.edit_entry raised on a body the reference accepts, and that running both
  passes everywhere fixed it. The wider reach was what fixed it. With no decoder, Ruby,
  Python and TypeScript now refuse a string id in assignees, subscribers,
  completion_subscribers and schedule participants, where Go's decoder reads the number.
• The live fixture gap is open again. The $.completer person in
  spec/fixtures/cards/step.json — a real id, no personable_type — was normalized only by
  the twelve-key set, and is not normalized now.
• TypeScript's spec-derived key set is gone. It was the right derivation applied the
  wrong way: by name, at any depth, on every body. It is now the reference's two keys.
• Two tests this description called "holding the wrong behaviour in place" — Ruby's
  schedules guard and Python's [string-id] row, both asserting a string person id raises —
  describe what these ports do again. The refusal is now pinned deliberately, as a named
  divergence, rather than by accident.

All four are the refusing direction: no id is invented, and every merge-safe composite
refuses before its PUT, so no partial update is sent. That is decoder coverage, field
by field against the reference, rather than normalizer reach, and it belongs to
#913 (card 42). The tests that pinned
the wider behaviour now pin the refusal, each named as the one to flip when #913 lands.

Each port has a negative test — a strict site keeps refusing a string id — proven to
fail on 32ce521f for the stated reason, which is the system actor appearing:

typescript  AssertionError: expected +0 to be 'basecamp'
ruby        Expected: "basecamp"  Actual: 0
python      AssertionError: assert 0 == 'basecamp'

One expectation moved toward the reference. Python's summary creator for a sentinel id
carried a system_label, only because the pass wrote one onto the recording's untagged
creator first. Go's summary reads through personFromGenerated (people.go:920), which
never sets SystemLabel.

Every residual that remains is pinned as a test with its reason, and SPEC §10 states the
rule, the deliberate disagreements, and — new in this round — where the positional pass runs
and why no wider.

[jorgemanrubia]

@codex review

[chatgpt-codex-connector]

To use Codex here, create a Codex account and connect to github.

[Copilot]

🟡 Changes recommended

Kotlin can overwrite the normalized label, and TypeScript lacks direct coverage for the distinct SGID rule.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Standardizes person-ID parsing across seven SDKs against Go’s strconv.ParseInt behavior while preserving the stricter SGID rule.

Changes:

• Adds shared parsers distinguishing syntax and range failures.
• Fixes Go normalization of signed and zero-padded IDs.
• Adds cross-SDK corpus tests and specification guidance.

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

File summaries

File	Description
SPEC.md	Defines person-ID parsing rules.
go/pkg/basecamp/normalize.go	Emits canonical JSON numbers.
go/pkg/basecamp/person_id_grammar_test.go	Adds the Go oracle corpus.
typescript/src/person-id.ts	Implements shared parsing.
typescript/src/services/base.ts	Uses shared normalization.
typescript/src/services/mentions.ts	Uses shared flexible parsing.
typescript/tests/helpers/person-id-corpus.ts	Adds the measured corpus.
typescript/tests/services/person-id-normalization.test.ts	Tests response normalization.
typescript/tests/services/my-notifications.test.ts	Updates overflow expectations.
typescript/tests/services/mentions.test.ts	Tests reader outcomes.
swift/Sources/Basecamp/FlexibleInt.swift	Implements shared parsing.
swift/Sources/Basecamp/Services/BaseService.swift	Updates normalization.
swift/Sources/Basecamp/Mentions.swift	Documents SGID divergence.
swift/Tests/BasecampTests/FlexibleIntTests.swift	Adds corpus coverage.
rust/basecamp-sdk/src/types.rs	Pins decimal interpretation.
ruby/lib/basecamp/ids.rb	Adds ParseInt-compatible scanning.
ruby/lib/basecamp/http.rb	Shares the new parser.
ruby/test/test_helper.rb	Defines the Ruby corpus.
ruby/test/basecamp/ids_test.rb	Tests parser and reader.
ruby/test/basecamp/normalize_person_ids_test.rb	Tests normalization.
python/src/basecamp/_person_id.py	Adds shared parsing.
python/src/basecamp/generated/services/_base.py	Updates synchronous normalization.
python/src/basecamp/generated/services/_async_base.py	Updates asynchronous normalization.
python/src/basecamp/services/_campfire_index.py	Shares flexible parsing.
python/src/basecamp/mentions.py	Documents the SGID rule.
python/tests/person_id_corpus.py	Defines the Python corpus.
python/tests/test_person_id.py	Tests all parsing sites.
python/tests/services/test_notifications.py	Tests response integration.
python/tests/services/test_recordings_summarize.py	Expands flexible-ID coverage.
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/serialization/ParseInt64.kt	Implements shared parsing.
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/serialization/NormalizePersonIds.kt	Updates normalization.
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/serialization/FlexibleLongSerializer.kt	Shares parser outcomes.
kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/Mentions.kt	Documents SGID validation.
kotlin/sdk/src/commonTest/kotlin/com/basecamp/sdk/FlexibleLongSerializerTest.kt	Adds corpus coverage.
kotlin/sdk/src/commonTest/kotlin/com/basecamp/sdk/MentionsTest.kt	Tests SGID divergence.

Review details

• Files reviewed: 33/35 changed files
• Comments generated: 2
• Review effort level: Balanced

---

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

[Copilot]

🟡 Changes recommended

Global key-based normalization now coerces string IDs in schemas whose IDs are intentionally strict plain integers.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 41/44 changed files
• Comments generated: 5
• Review effort level: Balanced

[jorgemanrubia]

🤖 Holding this PR. Copilot's global-normalization finding is correct, and it is a regression this branch introduces. Not merging until the fix is settled.

Posting the full site inventory here because it is the part that took the work, and the card 42 audit can build on it directly rather than re-deriving it.

What the reference actually reaches

Go's positional creator/participants pass (normalizeEmbeddedPersonIds) runs only where normalizeEmbeddedPeopleJSON is called — gauges.go:171, my_notifications.go:171,281,296. Nowhere else. todos.go:68-77 mentions it in prose only. Every other person id in Go converts at decode, because generated.Person.Id is types.FlexibleInt64 (client.gen.go:2484).

Ruby, Python and TypeScript instead run the positional walk on every response (ruby/lib/basecamp/http.rb:55 via Response#json and parse_page; python/.../generated/services/_base.py:71,106,199,299,362,420,452; typescript/src/services/base.ts PERSON_VALUED_KEYS). Kotlin, Swift and Rust have no positional pass and are unaffected.

Every strict site the walk now reaches

Person.Id is FlexibleInt64, so Go converts a string there. These have a plain int64 id, so Go rejects a string:

type	Go id	reached by the walk at
UpcomingSchedulePerson	int64 (:4196)	UpcomingScheduleEntry.creator, .participants, UpcomingAssignable.assignees, UpcomingAssignableCompletion.creator
MyAssignmentAssignee	int64 (:2323)	MyAssignment.assignees
OutOfOfficePerson	int64 (:2452)	DisableOutOfOfficeOutput.person
PreferencesPayload	no id member	UpdateMyPreferencesInput.person — an input shape; inert
TemplateLibraryConfirmationPerson	int64 (:3487)	not reached. It sits under PeopleConfirmationRequiredError.people, and people is not a key in this walk

That last row is the one card 42 should note: the strict shape card 42 documents is not affected by this branch either way, so the two pieces of work do not collide at that site.

Operations affected: GetUpcomingSchedule, GetMyAssignments, GetMyDueAssignments, GetMyCompletedAssignments, PrioritizeAssignment / DeprioritizeAssignment / ReorderUpNext, DisableOutOfOffice. Each response schema walked to full depth: none contains a genuine Person-typed field, so scoping the pass away from these operations costs nothing.

Severity, measured

The pass fires only on a string id. In spec/fixtures/schedules/upcoming.json all six upcoming-schedule sites arrive as JSON integers, and those objects carry no personable_type. So the divergence is latent today, not a live defect — the same standard this branch applied to the root-Person case. Still a divergence from the reference, and still this branch's to fix.

Why it is not already fixed

The fix is a fork, and the two branches differ in what they take from card 42:

1. Narrow to Go's two wrapper surfaces. Faithful and small, but it hands assignees / subscribers / completion_subscribers back to card 42 — and walks back the measured merge-safe-write fix this PR currently claims.
2. Keep the wider reach, make it operation-aware from the spec. Keeps the measured improvement and satisfies "prove the reach is identical", but it keeps this PR standing in card 42's territory.

Put to the operator rather than decided here, since it is a scope call across two cards. Whichever way it goes, the negative test lands with it — a strict site staying strict, proven to fail on the current head.

cc the card 42 audit: the inventory above is stable regardless of which option is chosen.

[jorgemanrubia]

🤖 Adversarial review of 32ce521f (a separate Opus reviewer, read-only, briefed to break the change). It holed it in three places, one of which I had not seen and which I think outranks the normalization finding.

1. recordings.summarize() now throws on untrusted rich text — new denial-of-read

mentionedPersonIds (typescript/src/services/mentions.ts:118) used to skip an sgid whose person id exceeds 2^53; this branch makes it throw, discarding every other mention in the text. projectRecording calls it unguarded on server-returned content (typescript/src/services/recordings-extensions.ts:1753).

The sgid's signature is not verified — globalIDFromSGID accepts a bare unsigned envelope — so the id is attacker-controlled by anyone who can write a <bc-attachment> into a comment. One crafted comment makes the whole summary unreadable:

THREW: BasecampError api_error
  rich text mentions person 9007199254740993, whose id does not fit a JavaScript number

The response was otherwise well-formed. Before this branch, summarize returned the summary with that mention skipped.

What makes this sharp is that person-id.ts argues against exactly this shape for the normalizer — "One unrepresentable id in one embedded person would discard every other record in the body… The id is safe; everything beside it pays." — and then this site does it, on the one input in the SDK that is untrusted by construction. The under-reporting argument in the docstring is sound and I still believe it; the throw is the wrong instrument at a site whose input an adversary writes. Severity: blocker in my read.

2. The positional pass reaches schemas the reference decodes as plain int64

Independently reproduced what Copilot found, with the reference's own refusal alongside:

Go:     json.Unmarshal(`{"creator":{"id":"basecamp",…}}`, &generated.UpcomingScheduleEntry{})
        -> cannot unmarshal string into field UpcomingScheduleEntry.creator.id of type int64
Ruby:   {"creator"=>{"id"=>0, …, "system_label"=>"basecamp"}}
Python: {'creator': {'id': 0, …, 'system_label': 'basecamp'}}
TS:     {"creator":{"id":0,…,"system_label":"basecamp"}}

So a body the reference refuses outright now reads as the system actor — in the accepting direction, on an identity field. These three sites were untouched before this diff, so it is new.

It also contradicts a rationale already written into this branch's own drift check (person-id-normalization.test.ts:186-190): "OutOfOfficePerson and friends are person-SHAPED but are plain int64 ids in the reference." The derivation excludes those schemas; the application is by key name, so the exclusion never takes effect for a name that a Person-typed field elsewhere already claims. creator and participants are both such names. Full site inventory in my previous comment.

3. Ruby and Python still refuse merge-safe writes that Go — and now TypeScript — accept

TypeScript closes assignees in this diff; Ruby and Python do not. Ruby documents the deferral (http.rb:71-86, "Tracked as its own unit of work"); Python does not mention it at all, and normalize_person_ids's docstring claims it "owns BOTH of the passes that function runs" — true of Go's normalizer, but it misses Go's second mechanism, the flexible decoder. The same write-path failure this PR fixes for participants, left standing on the sibling keys, now inconsistent across ports within one change.

4. Two doc errors, one of them mine

• base.ts says twice that Go runs normalizeEmbeddedPeopleJSON on "notifications, gauges, todos". It does not — the only call sites are gauges.go:171 and my_notifications.go:171,281,296. Todo person ids reach Go through the generated decoder instead, which is the very distinction that comment is drawing.
• The PR description called "18446744073709551615x" "one digit shorter" than "18446744073709551616x". Same length, one smaller. Fixed in the description; the pair differs by which refusal the scan reaches first, which is the whole point of it.

5. A wall-clock assertion

ids_test.rb, test_the_scan_bounds_itself_without_a_length_gate scans 10M digits in interpreted Ruby and asserts elapsed < 1.0. It pins something real, but it is the only timing assertion in this work and a flake candidate on a loaded runner.

What it attacked and could not break

Worth recording, since it bounds what the above does and does not cast doubt on. Grammar equivalence to strconv.ParseInt re-derived by hand in all six ports — ParseUint's in-loop u64 bound and ParseInt's asymmetric signed bound reproduce exactly everywhere, Kotlin's algebraic rearrangement included. It regenerated the Go oracle and mechanically diffed all five language corpora against it: 74 rows each, identical strings, identical verdicts, zero disagreements. Rule A / Rule B separation holds in every port, in both directions. Idempotence of the positional pass confirmed. Unicode digits (fullwidth, Arabic-Indic, Extended Arabic-Indic, Bengali, mixed) refused everywhere, none via a \p{Nd}-aware path. The replaced Ruby and Python tests are non-vacuous against the current code. All normalizer call sites are response-side only, so the injected system_label never reaches a PUT body. Line references in SPEC.md and the port comments all check out.

Not merging. Findings 1 and 2 both need a decision about scope before code: see my previous comment for the fork on 2, and 1 needs a call on whether the refusal moves, softens, or gets an opt-in reader. Both are with the operator.