Skip to content

feat: adopt Go 1.27 JSON v2 semantics - #1475

Open
mariusvniekerk wants to merge 4 commits into
mainfrom
t3code/upgrade-go-1-27
Open

feat: adopt Go 1.27 JSON v2 semantics#1475
mariusvniekerk wants to merge 4 commits into
mainfrom
t3code/upgrade-go-1-27

Conversation

@mariusvniekerk

@mariusvniekerk mariusvniekerk commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Moves AgentsView to Go 1.27 and adopts native encoding/json/v2 behavior
throughout. This is a clean migration with no JSON v1 compatibility or
checkpoint fallback paths.

Typed Huma routes now share JSON v2 request and response semantics with the
rest of the application. OpenAPI continues to describe token usage as
arbitrary JSON. JSONL rewrite paths retain untouched values as raw JSON, so
large integers stay exact, while stored map output is deterministic where bytes
define artifact identity.

The Go 1.27 audit also adopts generic methods, assignment-context inference,
promoted fields in struct literals, errors.AsType, and related
standard-library simplifications. Build images, local tooling, documentation,
and reviewer guidance now require Go 1.27. golangci-lint is pinned to v2.13.0,
which can analyze the new language level.

@mariusvniekerk
mariusvniekerk force-pushed the t3code/upgrade-go-1-27 branch from 3731e3a to a0389b2 Compare August 20, 2026 18:05
@roborev-ci

roborev-ci Bot commented Aug 20, 2026

Copy link
Copy Markdown

roborev: Combined Review (a0389b2)

High-severity compilation failure and medium-severity nondeterministic persisted JSON need fixing before merge.

High

  • internal/db/messages.go:112 — Changing Message.TokenUsage to jsontext.Value breaks test fixtures that still assign the distinct type encoding/json.RawMessage, including internal/db/usage_cache_fill_test.go:29 and internal/postgres/usage_facts_parity_pgtest_test.go:188. The test packages will not compile. Convert all remaining assignments and imports to jsontext.Value, or retain a source-compatible field type.

Medium

  • internal/parser/workbuddy.go:302 — JSON v2 does not sort map keys by default, so persisting map-backed JSON in TokenUsage or InputJSON can produce different byte sequences when identical input is reparsed. Byte-for-byte parsed-diff and fingerprint comparisons may then report false changes and trigger unnecessary synchronization. Similar sites include gptme.go:204, vscode_copilot.go:593, visualstudio_copilot.go:1316, and roocode.go:1335. Use json.Deterministic(true) or otherwise canonicalize map-backed JSON before persistence.

Reviewers: 2 done | Synthesis: codex, 14s | Total: 12m45s

@mariusvniekerk
mariusvniekerk marked this pull request as ready for review August 20, 2026 19:33
@roborev-ci

roborev-ci Bot commented Aug 20, 2026

Copy link
Copy Markdown

roborev: Combined Review (f3044da)

High-severity persisted-hash compatibility regression and three medium-severity determinism, compatibility, and memory issues remain.

High

  • Persisted hashes change without a version bumpinternal/db/recall_evidence_window.go:507, internal/artifact/canonical_json.go:12, internal/rawsync/manifest.go:185

    Migrating from the previous JSON representation to JSON v2/JCS changes hashes for strings containing characters such as <, >, or &, while format and digest versions remain unchanged. This can revoke provenance_ok for unchanged evidence, cause raw-ingest retry conflicts, and assign new artifact identities to unchanged content.

    Fix: Preserve legacy encoding for existing versions, or introduce new versions with dual-version validation and migration. Add compatibility vectors containing HTML-sensitive characters.

Medium

  • Zero numeric fields now alter pinned segment bytesinternal/artifact/wire.go:218, internal/artifact/wire.go:231

    These fields still use omitempty, but JSON v2 no longer omits zero numbers. Empty tool-call or result events can therefore change content hashes without a segment-version bump.

    Fix: Change both tags to omitzero and add a compatibility test for zero-length nested tool results.

  • Persisted parser JSON is nondeterministicinternal/parser/kilo_legacy.go:1755, internal/parser/kilo_legacy.go:1767, internal/parser/poolside.go:358, internal/parser/zed_helpers.go:163

    Maps are serialized into persisted InputJSON without deterministic marshaling. Reprocessing identical data may produce different bytes and ToolCallFingerprint values, causing spurious session changes and mirror pushes.

    Fix: Use deterministic marshaling for all parser-generated or normalized JSON, and test repeated parsing for identical input and fingerprints.

  • Checkpoint bootstrap can consume excessive memoryinternal/artifact/export_checkpoint.go:173

    The implementation reads and clones up to 64 MiB, unmarshals the complete sessions map, and serializes it again, potentially requiring several hundred megabytes and risking OOM.

    Fix: Restore streaming validation and hashing with jsontext.Decoder, retaining one session entry at a time while preserving the decoded-size limit.


Reviewers: 2 done | Synthesis: codex, 19s | Total: 20m51s

Move the project to Go 1.27 so all JSON encoding and decoding uses the new
standard-library semantics without legacy compatibility paths. This removes
custom checkpoint machinery and makes the JSONL readers easier to audit.

Use the new generic methods, inference, promoted struct-literal fields, and
error helpers where they remove repeated adapter code. Pin golangci-lint
v2.13.0 because earlier releases cannot analyze the Go 1.27 language level.
The usage rollup work added a benchmark seed after this branch migrated
message token data to JSON v2. Use the same v2 value type so the rebased test
package compiles and measures the production representation.
Use JSON v2 consistently at typed HTTP and session rewrite boundaries. This
rejects duplicate object names and keeps untouched large integers exact instead
of converting them through float64.

Deterministic derived JSON preserves byte-level artifact identities.
The large-number regression inserted a native temporary path directly into
JSON. Windows backslashes made that input malformed, so the resolver returned
the original line before the test reached the behavior it was meant to cover.

Encode path-bearing strings as JSON so every platform exercises the same
persisted-result rewrite contract.
@mariusvniekerk
mariusvniekerk force-pushed the t3code/upgrade-go-1-27 branch from f3044da to 27d14b6 Compare August 20, 2026 20:02
@roborev-ci

roborev-ci Bot commented Aug 20, 2026

Copy link
Copy Markdown

roborev: Combined Review (27d14b6)

Migration is generally sound, but two medium-severity issues remain.

Medium

  • Non-deterministic persisted tool inputsinternal/parser/kilo_legacy.go:1755, internal/parser/poolside.go:358
    Map-backed tool inputs are marshaled without json.Deterministic(true). JSON v2 does not sort map keys by default, so reparsing unchanged sessions can produce different InputJSON, causing spurious revisions and inconsistent exports. Enable deterministic marshaling and test repeated and full-versus-incremental parsing stability.

  • Excessive checkpoint bootstrap memory useinternal/artifact/export_checkpoint.go:173
    Bootstrap now reads, clones, canonicalizes, fully unmarshals, and re-marshals checkpoints up to 64 MiB. Multiple full-size buffers and the decoded session map can coexist, regressing streaming behavior and risking excessive memory use or OOM for valid large archives. Restore streaming validation and incremental hashing to avoid full-body copies and map materialization.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 15m29s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant