feat(protocol): generate Go SDK from Codex JSON Schema

[birdmanmandbir]

Summary

• Generate typed Go bindings from the Codex app-server JSON Schema (~15K lines, 91 top-level + 248 v2 definitions)
• Custom Python codegen (cmd/codegen/main.py) produces three files:
  • protocol/types_gen.go — ~330 struct types, enums, and type aliases with correct json tags and nullable pointers
  • protocol/unions_gen.go — discriminated union types (ClientRequest, ServerNotification, ServerRequest) with typed params accessor methods
  • protocol/methods_gen.go — all method/notification/request string constants
• Add Makefile with generate, vet, test, ci targets
• Add GitHub Actions CI that verifies generated files stay in sync
• All code is regeneratable via make generate / go generate ./...

Test plan

• go vet ./... passes
• go test ./... passes (marshal round-trips, params unmarshaling, enum values, nullable field omission)
• CI workflow runs on PR

[birdmanmandbir]

PR Review: feat: Generate Go SDK from Codex JSON Schema

Summary

Solid codegen foundation. Python script reads the Codex JSON Schema and emits typed Go structs, discriminated union wrappers with typed *Params() getters, method/notification constants, and a CI pipeline to enforce freshness. Tests pass, go vet clean.

---

Critical

1. Inconsistent Go field naming for snake_case vs camelCase JSON keys

The to_go_field_name function handles snake_case and camelCase differently, producing inconsistent Go field names for the same concept:

• thread_id (snake_case) → ThreadId (no acronym fixup applied)
• threadId (camelCase) → ThreadID (regex fixup applied)

Same for SessionId vs SessionID, TurnId vs TurnID, etc.

Root cause: The snake_case branch at line 86-88 of cmd/codegen/main.py returns early without applying the re.sub acronym fixups that the camelCase branch uses (lines 103-110).

# Snake_case path (line 86-88) - NO acronym fixup
if "_" in name:
    parts = name.split("_")
    name = "".join(p.capitalize() for p in parts if p)
    return name  # <-- returns "ThreadId" not "ThreadID"

# camelCase path (lines 100-110) - HAS acronym fixup
result = re.sub(r'Id$', 'ID', result)  # <-- fixes to "ThreadID"

Fix: Apply the same acronym regex fixups after the snake_case join before returning.

---

Minor

2. Dead code in to_go_field_name (cmd/codegen/main.py:94-98)

Lines 94-96 compute aggressive replacements that are immediately overwritten at line 100 (result = name[0].upper() + name[1:]). This is leftover from a refactor — the comment even says "Revert and do it more carefully". Should be cleaned up.

3. var _ json.RawMessage blank identifier (protocol/types_gen.go:10)

This is a hack to keep the encoding/json import alive. More idiomatic to emit it only when json.RawMessage is actually used in the file (which it always is here, so the guard is unnecessary).

4. CI doesn't cache Python — The GitHub Actions workflow sets up Python 3.12 but doesn't cache pip. Not needed now since there are no pip dependencies, but worth noting if dependencies are added later.

---

Nits

• go.mod targets go 1.25.5 — very new; confirm this is intentional and won't block downstream consumers on older Go versions.
• Makefile has a clean phony target declared but no clean recipe.

---

VERDICT: NEEDS_WORK

The snake_case acronym inconsistency (#1) is a real API surface bug — consumers will see ThreadId on some structs and ThreadID on others for the same concept. Fix the codegen, regenerate, and the rest looks good to ship.

[birdmanmandbir]

Triage Update

Fixed

• feat(protocol): generate Go SDK from Codex JSON Schema #1 Snake_case acronym inconsistency — toGoFieldName now applies acronym fixups (Id→ID, Url→URL, etc.) to both snake_case and camelCase paths. All snake_case fields like thread_id, turn_id, call_id, session_id now correctly produce ThreadID, TurnID, CallID, SessionID.
• feat(protocol): generate Go SDK from Codex JSON Schema #2 Dead code — N/A, the Python script has been replaced with a Go port (cmd/codegen/main.go). The dead code no longer exists.
• fix(codegen): extend discriminated union detection to support 'type' field #3 var _ json.RawMessage hack — Removed. The encoding/json import is used directly by json.RawMessage fields throughout the file.
• chore(repo): add lefthook pre-commit hooks #4 CI doesn't cache Python — Python dependency removed entirely. Codegen is now pure Go (go run ./cmd/codegen). actions/setup-python step removed from CI.
• Nit: clean phony without recipe — Makefile rewritten with proper targets (help, fmt, vet, lint, test, build, ci, check-generate).

Additional improvements

• Added gofmt -d -s formatting check to CI
• Added golangci-lint via golangci/golangci-lint-action@v6
• Added go build verification step to CI

Not addressed

• Nit: go 1.25.5 in go.mod — Intentional. This is the target Go version for the project.

[birdmanmandbir]

Re-Review: Post-Fix Assessment

Original Issues - Status

#	Issue	Status
1	Snake_case acronym inconsistency	Fixed - toGoFieldName now applies acronym fixups to both snake_case and camelCase paths. Verified: all thread_id, turn_id, call_id, session_id fields produce ThreadID, TurnID, CallID, SessionID.
2	Dead code in Python codegen	Fixed - Python script deleted entirely; codegen ported to Go.
3	var _ json.RawMessage hack	Fixed - Removed. The encoding/json import is used directly by json.RawMessage fields.
4	CI doesn't cache Python	Fixed - Python removed from CI. go-version-file: go.mod used instead.
5	clean phony without recipe	Fixed - Makefile rewritten with proper targets and help command.
6	go 1.25.5 in go.mod	Acknowledged - Intentional per coder.

Additional Improvements Verified

• gofmt formatting check added to CI
• golangci-lint via golangci/golangci-lint-action@v6 added
• go build ./... verification step added
• go vet ./... and go build ./... both pass locally

New Minor Issue

Dead code in toGoName (cmd/codegen/main.go:36-48)

Lines 36-47 build into a strings.Builder, then line 49 calls result.Reset() and redoes the work correctly. The first loop is dead code with an // Oops, wrote two bytes above. Let me redo this properly. comment. Should be cleaned up - just remove lines 36-48.

// Current (dead code + redo):
var result strings.Builder
for _, part := range parts {  // <-- this loop output gets discarded
    if part != "" {
        result.WriteByte(part[0] - 32 + 32)
        // ...
    }
}
// Oops, wrote two bytes above. Let me redo this properly.
result.Reset()
for _, part := range parts {  // <-- this is the real loop
    // ...
}

Nit: regexp.MustCompile in function bodies

toGoFieldName and toGoName compile regexes on every call. For a codegen tool this doesn't matter for performance, but idiomatic Go would hoist these to package-level var declarations.

---

VERDICT: NEEDS_WORK

The dead code with "Oops" comment in toGoName is a cleanup that should be done before merge - it's clearly leftover from the porting process and reads poorly in a shipped codebase. The regex nit is optional.

[birdmanmandbir]

Triage Update

Fixed

• Dead code in toGoName — Removed the dead first loop and "Oops" comment at cmd/codegen/main.go:36-48
• Inline regexp.MustCompile in function bodies — Hoisted all regexes to package-level var declarations for idiomatic Go

Remaining

None — all review items addressed.

[birdmanmandbir]

Re-Review: Post-Fix Assessment (Round 2)

Original Issues — Final Status

#	Issue	Status
1	Snake_case acronym inconsistency	Fixed — toGoFieldName now applies regex acronym fixups after both snake_case and camelCase paths. All thread_id, turn_id, call_id, session_id fields correctly produce ThreadID, TurnID, CallID, SessionID in generated output.
2	Dead code in Python codegen	Fixed — Python script deleted. Codegen ported to Go (cmd/codegen/main.go).
3	var _ json.RawMessage hack	Fixed — Removed from generated output.
4	CI doesn't cache Python	Fixed — Python removed entirely. CI uses go-version-file: go.mod.
5	clean phony without recipe	Fixed — Makefile rewritten with help, fmt, vet, lint, test, build, ci, check-generate targets.
6	Dead code in toGoName (Oops comment)	Fixed — Single clean loop remains, no dead code.
7	Inline regexp.MustCompile in functions	Fixed — All regexes hoisted to package-level var block (lines 26-37).

Verified

• go build ./... passes
• go vet ./... passes
• make generate produces output consistent with committed codegen source
• Acronym fixups verified: 76 old-style names (ThreadId, CallId, etc.) replaced with 83 corrected names (ThreadID, CallID, etc.) across generated output

Warning: Changes are uncommitted

All the fixes exist in the working tree but have not been committed. The cmd/codegen/main.go file is untracked, and all other changes (CI, Makefile, generated output, Python deletion) are unstaged. The coder needs to stage and commit before this can be merged.

VERDICT: LGTM

All review items are addressed. Code quality is good — the Go port is clean, idiomatic, and well-structured. Just needs a commit.

[birdmanmandbir]

Triage Update

Status: All items addressed, committed, and pushed

Commit 658a175 contains all fixes from this review cycle.

Fixed (all rounds)

• Snake_case acronym inconsistency — toGoFieldName applies acronym fixups to both paths
• Dead code in Python codegen — Python deleted, codegen ported to Go
• var _ json.RawMessage hack — removed
• Python CI dependency — removed entirely
• clean phony without recipe — Makefile rewritten
• Dead code in toGoName — removed dead loop + "Oops" comment
• Inline regexp.MustCompile — hoisted to package-level vars
• CI improvements — added gofmt check, golangci-lint, go build step

Remaining

None — merging.