Skip to content

Upgrade to Go 1.27 and repair the shell e2e sync harness - #208

Merged
marcus merged 5 commits into
mainfrom
go-upgrade
Aug 22, 2026
Merged

Upgrade to Go 1.27 and repair the shell e2e sync harness#208
marcus merged 5 commits into
mainfrom
go-upgrade

Conversation

@marcus

@marcus marcus commented Aug 22, 2026

Copy link
Copy Markdown
Owner

Lands the Go 1.27 toolchain upgrade, and fixes the failing sync e2e tests that surfaced while verifying it. The two are independent problems that happened to meet in the verification step, so they land together.

Go 1.27 upgrade

go.mod moves 1.25.81.27.0, plus a go mod tidy that reclassified atotto/clipboard from indirect to direct. Both CI workflows resolve the toolchain from go-version-file: go.mod, so the directive is the only lever — no workflow change needed.

The risk this plan called out was encoding/json/v2 becoming the default implementation, since td leans on JSON for sync payloads, the serve API, JSONL logs, and SQLite JSON columns. Nothing failed for a JSON reason, so the planned GOEXPERIMENT=nojsonv2 attribution pass was never needed.

Note for downstream: Go 1.26 raised the floor for darwin builds to macOS 13+. Homebrew users are unaffected in practice.

The failing tests — root cause

All 16 scripts/e2e tests failed, but not from 16 bugs. They all died in shared harness setup, and had been failing on main since 2026-06-19.

Commit fb1327a migrated test/e2e/harness.go and scripts/e2e-sync-test.sh to the device PKCE login flow. It missed scripts/e2e/harness.sh, which kept authenticating through /v1/auth/login/start + /auth/verify. Those endpoints now answer 410 Gone unless SYNC_LEGACY_DEVICE_AUTH is set.

The break was invisible: curl -sf under set -e exited the script with no assertion, no error text, and no diagnostic. That is why it read as a wholesale broken suite rather than a moved endpoint.

Sync behavior itself was never at fault. This is test rot, not a sync regression.

The fix

The harness now drives the same flow the real td auth login uses, matching the two already-migrated implementations rather than inventing a third shape: PKCE pair → device/start → read the magic link from the in-memory mailbox via the dev-only /internal/dev/last-email → GET the approve URL → device/poll.

Supporting changes:

  • Provision users via td-sync admin create-user before the server opens the DB — device/start is non-enumerating, so an unprovisioned actor is silently never mailed a link.
  • Server gains SYNC_EMAIL_PROVIDER=memory, SYNC_DEV_EMAIL_INSPECT=1, SYNC_EMAIL_BASE_URL, and lifted rate limits (the default auth limit is 10/min per IP, which a full suite trips).
  • setup() now calls start_server() instead of duplicating the launch block, so restart scenarios cannot drift from initial start.
  • Every failure path reports why instead of exiting silently — the specific thing that cost the most time here.

deploy/deploy.sh also calls the legacy endpoint, but deliberately: it asserts a 410/404 to confirm the secure-auth cutover took effect. Left alone.

Verification

All at Go 1.27.0 (darwin/arm64):

  • go test -count=1 ./... with the release-safe env — clean, cache busted
  • scripts/e2e/run-all.sh16 passed, 3 skipped (--full only), 0 failed (was 0/16)
  • scripts/e2e-sync-test.sh — green
  • td serve smoke — GET /health, GET /v1/issues, POST /v1/issues round-trip; a malformed body returns a precise json/v2 validation error
  • goreleaser release --snapshot --clean — all four targets build and archive

Follow-ups, deliberately not in this PR

  1. This suite is not run by CI, and is not part of go test ./.... That is exactly why it rotted unnoticed for two months, and it is the only coverage for real multi-client sync convergence. Wiring it in is worth doing, but it is slow (each script builds binaries and starts a server), so the CI-time tradeoff is a separate call.
  2. syncclient.LoginStart / LoginPoll (internal/syncclient/client.go:176-189) target the retired endpoints and have no non-test callers — the same rot, not yet swept.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ABM7FViu3tTWDEcGpm3qcY

marcus and others added 5 commits August 22, 2026 09:50
Crosses 1.26 and 1.27 in one step per docs/plans/active/go-1-27-upgrade.md.
Full suite, e2e sync test, and serve smoke all green at 1.27.0.
All 16 scripts/e2e tests failed during shared setup, and had been failing
on main since fb1327a (2026-06-19). That commit moved test/e2e/harness.go
and scripts/e2e-sync-test.sh onto the device PKCE flow but missed
scripts/e2e/harness.sh, which kept authenticating through
/v1/auth/login/start + /auth/verify. Those endpoints now answer 410 Gone
unless SYNC_LEGACY_DEVICE_AUTH is set, so every test died in setup.

The failure was invisible: `curl -sf` under `set -e` exited the script
with no assertion, no error text, and no diagnostic — which is why this
read as a broken suite rather than a moved endpoint. Sync itself was
never at fault.

Migrate the harness to the same flow the real `td auth login` uses,
matching the two already-migrated implementations rather than inventing a
third shape: PKCE pair -> device/start -> read the magic link from the
in-memory mailbox via the dev-only /internal/dev/last-email -> GET the
approve URL -> device/poll.

Supporting changes:

- Provision users via `td-sync admin create-user` before the server opens
  the DB. device/start is non-enumerating, so an unprovisioned actor is
  silently never mailed a link.
- Give the server SYNC_EMAIL_PROVIDER=memory, SYNC_DEV_EMAIL_INSPECT=1 and
  SYNC_EMAIL_BASE_URL so the magic link is readable, and lift the rate
  limits — the default auth limit is 10/min per IP, which a full suite
  trips.
- Have setup() call start_server() instead of duplicating the launch
  block, so restart scenarios cannot drift from initial start.
- Report why on every failure path instead of exiting silently.

scripts/e2e/run-all.sh: 16 passed, 3 skipped (--full only), 0 failed.
scripts/e2e-sync-test.sh passes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ABM7FViu3tTWDEcGpm3qcY
Fill in the upgrade plan's acceptance section with what was actually run,
and note that step 3 surfaced a pre-existing harness break rather than a
1.27 regression — no json/v2 attribution pass was needed because nothing
failed for a JSON reason.

Also flag for a future plan that the shell sync suite is neither part of
`go test ./...` nor run by CI, which is why it rotted unnoticed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ABM7FViu3tTWDEcGpm3qcY
@marcus
marcus merged commit d99e7e0 into main Aug 22, 2026
1 check passed
@marcus
marcus deleted the go-upgrade branch August 25, 2026 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant