refactor: complete Modern Go adoption across module boundaries - #706

Closes #482.

Summary

This PR completes the Modern Go migration across every Go module and build-tagged tooling surface in the repository, and makes compliance continuous so a regression fails the gate regardless of which module it lands in.

It is a behavior-preserving refactor. Ordering, nil-vs-empty collections, JSON omission, error matching, goroutine ownership, cancellation lifetimes and wire contracts are treated as contracts. Every migration was justified by reading the surrounding code; nothing was a blanket search-and-replace. Sites that can't be migrated without changing behavior are kept and documented, never silently skipped.

¹ A test fake whose comparator returns true for nil/nil; a valid three-way comparator would change its observable order, so it keeps sort.Slice behind a site-specific lint exception.
² All remaining uses are sort.Search upper-bound cursor/insertion searches, which slices.BinarySearch (lower bound) can't express.
³ Four targets are interfaces that don't implement error (errorPayload, ExitCode() int), so errors.AsType[E error] can't express them. The fifth match is a string inside a t.Fatal message.
⁴ Each remaining use falls in a documented category: cleanup callbacks (where t.Context() is already canceled), TestMain, helpers with no testing.TB, or process/server lifetimes that cleanup stops.
⁵ Only scalar and pointer fields whose encoding is byte-identical. See "JSON".

Context: what changed since the issue was filed

• The root module is now Go 1.27.1, not 1.26.4. Root code was migrated against the 54 guideline IDs the Modern Go Guidelines CLI resolves for 1.27.1. That adds stdlib_uuid, strings_bytes_cut_last, promoted_field_literals, url_clone, generic_methods and json_v2. The five nested modules (sdk/go, sdk/examples/notes-commands, sdk/examples/clarify-tool, internal/extension/testdata/{command,palette}-fixture-go) still declare 1.26.4 and were migrated against its 48 IDs, with no 1.27 APIs. Their go directives were left alone, because sdk/go is a public SDK and raising its minimum Go version is a separate decision.
• The Daytona sidecar no longer exists. internal/sandbox was removed in the packages cleanup. The HTTP API is served by gin, so http_servemux_patterns only applied to hand-written net/http handlers. The ones that remain were evaluated and kept, because exact parity on status codes and Allow behavior wasn't guaranteed.
• json_v2 (encoding/json/v2) is out of scope: the guideline says not to migrate existing encoding/json code unless explicitly requested.

What changed

1. Continuous compliance (tooling, gates, CI)

• Every module and build-tagged surface is linted. make go-lint, make lint and the CI go-lint job now run golangci-lint with the root .golangci.yml on the root module, magefiles (mage tag), sdk/go, both SDK examples and both extension fixture modules. The fixture modules are resolved through a disposable go.work pinned to the local SDK, the same way the authoring tests build them, and the workspace is removed afterwards. Scoped runs (COMPOZY_GO_LINT_SCOPES) translate nested-module scopes and stay narrow for root scopes. The first failing module exits non-zero.
• Explicit checks for guideline IDs that modernize doesn't enforce. The bundled modernize suite (x/tools v0.49.0 in golangci-lint v2.13.1) was inspected analyzer by analyzer. The gaps are now enforced by:
  • type-aware forbidigo on sort.Strings|Ints|Float64s|Slice|SliceStable|Sort|Stable and on errors.As;
  • depguard, which bans github.com/google/uuid in the root module only (the nested modules can't use the 1.27 stdlib uuid);
  • copyloopvar.
• errcheck accepts the errors.AsType idiom. errcheck.exclude-functions: [errors.AsType] lets the if _, ok := errors.AsType[*T](err); ok idiom pass check-blank, which is still enabled. A probe showed that a genuinely ignored error is still reported.
• scripts/gate.sh classifies nested modules correctly. Go files and go.mod/go.sum under sdk/examples/<name>/ select that module's Go lint and test lanes; before this PR they only selected a JS filter. sdk/go selects lint as well as tests. Fixture-module changes select fixture lint. magefiles selects tagged lint. Bash 3.2 compatibility is kept, and the scripts integration suite covers the new classifications.
• CI change detection includes nested module manifests. The changes filter now treats nested go.mod/go.sum as backend changes, and the golangci cache key hashes every module manifest.
• New runbook: docs/runbooks/modern-go.md. It covers the module/Go-version table, a rule → enforcement map for all 54/48 IDs (modernize analyzer, other linter, explicit check, or review-enforced), local commands, gate/CI lane selection, and the consolidated retained-exclusion inventory.

2. Mechanical migrations (all modules)

Every rule below was applied only where the rewrite is exactly equivalent:

• Sorting:
  • sort.* → slices.Sort / SortFunc / SortStableFunc. Comparators were translated key by key with cmp.Compare and cmp.Or, and stable sorts stay stable.
  • Map-key collection now uses slices.Sorted(maps.Keys(...)). Where the empty result can reach JSON, a DTO, an equality assertion or a nil check, it uses slices.AppendSeq(make(...), maps.Keys(...)) so the result stays non-nil.
• Errors: errors.As → errors.AsType[T], keeping the same pointer/value target, lazy evaluation order (switch cases became ordered if chains), and typed-nil semantics.
• Loop variables: redundant x := x copies removed.
• UUIDs: github.com/google/uuid → stdlib uuid.NewV4().String(), which keeps random v4 and the same canonical format. The dependency is gone from go.mod.
• Other idioms: new(value) instead of pointer helpers and temporaries (preserving aliasing), strings/bytes.CutLast, CutPrefix, SplitSeq, range-over-int, min/max, slices.Contains/Index/Compact, and maps.Clone/Copy/DeleteFunc. Each is applied only where nil/empty identity and no-match defaults are preserved.
• Promoted field literals (Go 1.27): keyed literals only. They are never mixed with the embedded field and never go through pointer embedding.

3. Semantic migrations, reviewed by owning layer

• Test contexts (testing_t_context).
  • About 3,700 test contexts now use t.Context() / b.Context() / tb.Context().
  • The shared helper internal/testutil.Context(t) now inherits the owning test's context, as the issue asked. When it is called during cleanup, after t.Context() has been canceled, it detaches and applies a fresh 45-second bound.
  • Contexts created in a test body and later used inside a t.Cleanup callback would now arrive canceled. An AST audit over the whole merged tree, build-tagged files included, found every such capture. Each one now gets a fresh bounded cleanup context (testutil.Context(t) inside the callback). A reviewer had reproduced the failure before the fix (TestMCPAuthOwnerIsolation, workspacedb TestOpen, TestManagerBusyInput*).
  • Contexts that drive goroutines or processes whose lifetime ends in cleanup keep an independent context, so teardown order doesn't change. Examples: the terminal reaper, MCP server, process-group kill, and ACP process host.
• sync.WaitGroup.Go / sync.OnceFunc / sync.OnceValue(s) are used only for locally paired equivalents. An Add performed under an admission lock, an Add(n) batch, or a struct-owned lifecycle gate (reset, per-call state, re-panic semantics) is kept.
• Clone/search/iterator helpers. append([]T(nil), s...) normalizes an empty input to nil, but slices.Clone doesn't. It was only replaced behind existing length guards.
• Pointer allocation (new(expr)) was only used where the temporary was allocation-only. Behavioral helpers (defaulting, trimming, UTC conversion, deep copy) are kept.

4. JSON (json_omitzero), audited field by field with codegen parity

• Tag consumers inventoried before any change: encoding/json v1, OpenAPI schema reflection, the TS SDK generator, the Go SDK generator, MCP jsonschema-go, goccy go-yaml (which falls back to json tags), gin binding, mapstructure, and the TOML encoders.
• What was migrated: 1,238 omitempty options on bool, numeric and pointer fields, which encode identically under omitzero in encoding/json v1. This excludes types with an IsZero method, which omitzero would call. The rewrite was done by an AST tool that resolves named types, aliases and promoted IsZero methods, and touched tag text only.
• What was kept:
  • omitempty on strings, slices and maps, as the guideline says;
  • types with IsZero, including one promoted through store.EventCorrelation, which the parity probes caught;
  • interfaces (tri-state);
  • struct no-ops whose zero object is on the wire (sdk/go/types.go:136, which now has a justified nolint);
  • Go SDK generator source tags, which are copied verbatim into generated code;
  • all generated files.
• Parity evidence:
  • make codegen produced no changes to generated artifacts: OpenAPI, the TS/Go SDKs, web types, native tool descriptors, schemas and digests.
  • make codegen-check passes.
  • 1,238 field types were probed in four states (zero, pointer-to-zero, populated, negative), and 592 whole struct shapes were compared old vs new, including JSON and YAML fallback. All produced byte-identical output.

Retained exclusions

Every retained prohibited call has a site-specific //nolint:<linter> // <reason>. Review-enforced patterns are inventoried by guideline ID in docs/runbooks/modern-go.md under "Retained exclusions", each classified as semantic, generated, or justified exclusion. No blanket suppression was added.

How this was produced and reviewed

The work was split into twelve file-disjoint slices, each in its own git worktree: daemon, api, store, cli/config, session/acp/terminal, loop, task/automation, extension/hooks, tools/mcp/skills, platform/testutil, tooling and nested modules, and JSON tags. Each slice was executed under one shared contract, with per-rule policy, ownership boundaries and capped scoped verification. The slices were merged into this branch, followed by one integration pass for cross-slice concerns: the errors.AsType errcheck fix and the cleanup-context sweep. Every slice diff then got an independent adversarial review for behavior changes (comparator translation, nil/empty, context lifetimes, WaitGroup/Once ownership, aliasing, Cut edge cases, scope). Confirmed findings were fixed before this PR was opened.

Final pre-merge review (Codex GPT-6 Astra, xhigh)

Before merge, an independent reviewer (gpt-6-astra, reasoning xhigh, six parallel area sub-reviewers) read every non-tag hunk and every changed test hunk of the full diff. It also re-audited all JSON tag changes with go/types: 1,238 of 1,238 fields are pointers (769) or bool/numeric (469), none has IsZero, and 117 JSON/YAML-fallback/JSON-Schema before/after comparisons showed zero differences. The SDK generator --check reproduced every generated byte.

• Business logic changed? No unintended production business-rule or public-wire change was found.
• Bugs introduced? Two, both fixed in this PR and re-verified by the same reviewer on the new head:
  • F1 (major, test-only). Two E2E tests (TestDaemonE2EMCPServeProjectsWorkspaceBoundHostAPI, TestDaemonNativeWorktreeJourneyE2E004) derived their timeout context from t.Context(). That context drives exec.CommandContext for compozy mcp serve clients, which are closed in t.Cleanup. Because t.Context() is canceled before cleanups, the subprocess was killed first (MCP client.Close() error = signal: killed in the E2E runtime lane). Both tests are back to their original context.Background() lifetime (782f514). The reviewer then traced 29 other hosted-MCP client call sites and cleared each one.
  • F2 (minor, tooling). The gate's module and Mage lint lanes could inherit evidence from a narrower cached root lint run. Root evidence is now reused only when the recorded command actually covered ./... (e1f2049), and the gate integration suite has a regression case for it.
• CI flake. packages/site marketplace-catalog.test.tsx ("Should filter the current catalog…") takes about 4s in jsdom because of accessible-name role queries. That exceeded the 5s default when this broad change invalidated the turbo cache and all workspace suites ran concurrently (on main that run was a cache replay). The test now has the same 15_000 per-test budget the site suite already uses for heavy renders. Its assertions are unchanged (33d65d5).

Verification

• Per slice: go build ./..., go vet on the slice packages, race tests on every changed package (go test -race, -tags=integration where the slice has integration tests), scoped make go-lint with 0 issues, and Windows cross-vet for terminal/pty and procutil.
• JSON: make codegen produced no generated diff, make codegen-check passes, the marshal/YAML parity probes passed, and contract, spec, codegen and SDK race suites pass.
• Tooling: the magefiles (-tags=mage) and scripts (-tags=integration) suites pass, along with SDK/example/fixture build and vet. Probes for each new lint rule fire in every module.
• Integration branch: the reproduced cleanup failures (TestMCPAuthOwnerIsolation, TestOpen, TestManagerBusyInput*) now pass, plus the touched integration tests.
• Integration branch, full make go-lint (whole config, every target): 0 issues on root ("" tags), root (integration Modern Go pass), sdk/go, both SDK examples, both fixture modules, and magefiles (mage tag). The fmt lane passed on all 1,479 changed Go files.
• Full verification (the whole race matrix, E2E, and the make verify-equivalent gates) is delegated to this PR's CI, per the maintainer's direction to keep heavy gates off the local machine. CI therefore stands in for the issue's "one current make gate-full record" criterion.

Acceptance criteria (#482)

• Every supported Go module and build-tagged Go tooling surface has an explicit Modern Go check.
• The guideline baseline (Go 1.27.1 root / Go 1.26.4 nested) has no unclassified findings; see the runbook inventory.
• Safe mechanical migrations are complete, with scoped race-enabled tests.
• Semantic candidates are either migrated under owning contract tests or retained with a concrete justification.
• JSON changes preserve HTTP/UDS/CLI/SDK wire behavior and pass codegen parity.
• Daytona sidecar routing: N/A, the sidecar was removed before this work. The remaining hand-written net/http fixtures were evaluated and kept.
• scripts/gate.sh classifies nested Go module changes and runs the relevant lint/test lanes.
• make go-lint reports zero warnings and errors on every target (run locally on the merged branch). make bun-lint (no JS changes) is verified by this PR's CI.
• Full gate: this PR's CI run on head 33d65d533 (35/35 required checks green: Go lint, all 8 race shards, Windows terminal, Darwin peer, build/boundaries, SDK race, preflight codegen, frontend, desktop, E2E runtime/web/desktop). It replaces the local make gate-full record per maintainer direction.

Compozy impact audit

• Native tools: no change. compozy__* descriptors, schemas, digests, risk flags and capability gates are byte-identical (make codegen shows no diff).
• Extensibility and hooks: no change. Hook dispatch order and registry ordering were reviewed as contracts. The extension fixtures and SDK are now covered by the nested-module lint lanes.
• Workspace data isolation: no data ownership, workspace_id propagation, or cache/event/SSE isolation changes.
• Official skill (skills/compozy/): unchanged. No public behavior or tool-ID change.
• Web/Docs: no Web or public wire change. The new developer runbook is docs/runbooks/modern-go.md. No QA scenario changes, since nothing user-visible changed.
• Config: no config.toml key changes.

🤖 Generated with Claude Code