Refs #1 (Partial)

[fzoll]

What this does

Implements Option 2 from #1 ("version handshake so drift fails loudly"), which the issue calls the best value-per-effort direction even if the rest is deferred.

Root cause recap: on source-mode nodes (e.g. the RPi), apps/web/dist is a static bundle that's only rebuilt when someone explicitly runs the web build — git pull + server restart doesn't touch it. serverVersion/APP_VERSION are both derived from package.json's semver, which doesn't bump per commit on this fork, so a stale bundle can silently share the exact same version string as a server that has since moved on to a schema-incompatible commit. The existing client/server version-mismatch banner (versionSkew.ts — it already existed, wired into Sidebar/ChatView/ConnectionsSettings) never fires, and the WS RPC connection just dies with a generic SchemaError on every attempt.

Changes:

• apps/server/src/environment/ServerBuildSha.ts (new): resolves the server's own git HEAD SHA at runtime via the existing ProcessRunner service, falling back to null on any failure (no .git, no git binary, timeout, non-repo).
• apps/server/src/environment/ServerEnvironment.ts: suffixes that SHA onto serverVersion (0.0.28+git.<sha12>) when resolvable. Packaged builds without a .git directory (desktop DMG, HA add-on) are unaffected — they already build server+web together per the issue's own deploy matrix.
• apps/web/vite.config.ts: stamps APP_VERSION with the checkout SHA the build ran from, unless an explicit APP_VERSION env var is set (release builds keep their existing behavior). Uses effect/unstable/process/ChildProcess rather than raw node:child_process per this repo's nodeBuiltinImport lint rule.
• apps/web/src/connection/platform.ts: logs a warning via the existing resolveVersionMismatch check as soon as the pre-flight GET /.well-known/t3/environment descriptor is fetched over plain HTTP — before the WS RPC session is attempted. This matters because the WS handshake's first call (server.getConfig) is exactly what fails to decode when client/server drift, so a mismatch signal has to come from somewhere that resolves independently of that fragile channel to be useful for this specific failure mode.

This reuses all the existing version-mismatch UI (banners, dismissal/localStorage, hints) with zero new schema fields — serverVersion itself now just carries more signal.

Why "Refs #1 (Partial)" and not "Fixes #1"

The issue opens with three directions and explicitly says "opening for design discussion," not "please implement X." I only implemented Option 2, on purpose:

• Not implemented (deliberately, per the issue's own framing as deferred): Option 1 (scripts/deploy-node.sh full pull→install→build→restart sequence) and Option 3 (CI-built artifacts distributed to nodes). Both are bigger, riskier changes that the issue itself flags as "larger effort" / "deferred," not something to unilaterally decide in this PR.
• Auto-deploy design questions (trigger, ordering, drain, rollback) raised at the end of the issue are untouched — genuinely open questions for a maintainer, not something I should guess at.
• Known residual gap: in the exact worst case the issue reports — the WS connection fails to establish on literally every attempt because the very first RPC call can't decode — primaryServerConfigAtom (which the existing UI banners read from) never populates, since it's only set after a successful server.getConfig round-trip. In that scenario the new pre-flight HTTP-descriptor check still fires and logs a clear, correlated warning (visible in console/observability), but it won't produce a UI banner on its own. Wiring the pre-flight descriptor's version into UI state independent of the WS session would close this gap but touches connection-state plumbing shared across web/mobile/desktop; I judged that out of scope for one focused PR and didn't want to guess at the right cross-platform design under limited time.

Testing

• pnpm --filter t3 test / pnpm --filter @t3tools/web test — full suites pass (one MessagesTimeline.test.tsx timeout is pre-existing flakiness under load; verified it passes standalone and is unrelated to this diff — untouched code path).
• pnpm --filter t3 --filter @t3tools/web --filter @t3tools/contracts typecheck — clean.
• vp lint on all changed files — clean.
• Verified apps/web/vite.config.ts actually loads and builds under the new top-level await (ran vite build --mode development to the transform stage).
• New unit tests: apps/server/src/environment/ServerBuildSha.test.ts (5 cases: success, non-zero exit, timeout, malformed output, spawn failure).

[fzoll]

VERDICT:CHANGES_NEEDED

Summary

Well-scoped, well-tested implementation of Option 2 from #1, and the PR description's self-assessment is unusually honest. The Effect idioms are correct, the graceful null fallback for packaged/non-git builds is right, and the ServerBuildSha unit tests cover the meaningful failure modes (non-zero exit, timeout, malformed output, spawn ENOENT). The core single-node fix works: on a source-mode RPi serving its own bundle, client APP_VERSION and server serverVersion both carry the checkout SHA, so a stale apps/web/dist now produces a real mismatch signal instead of a bare SchemaError. That is the intended win and it lands.

The blocker is a false-positive regression that the PR's otherwise-thorough analysis doesn't mention.

Main issue: SHA-granularity comparison produces false "version drift" on healthy remote environments

resolveVersionMismatch (apps/web/src/versionSkew.ts:26) is a raw string equality check, and the existing banners apply it per connected environment, not just the local/primary one:

• apps/web/src/components/settings/ConnectionsSettings.tsx:1390 — resolveServerConfigVersionMismatch(environment.serverConfig) per environment row → "Version drift" banner.
• apps/web/src/components/ChatView.tsx:1771 — resolveServerConfigVersionMismatch(serverConfig) for the active thread's environment (which can be a remote secondary).

Before this PR both sides derived from package.json semver, so an entire fleet on the same release compared equal and no banner fired. After this PR the source-mode client is stamped 0.0.28+git.<clientSha> and compared by exact string against each peer server:

• Packaged desktop (Mac DMG): no .git, so serverVersion stays plain 0.0.28. 0.0.28+git.<sha> != 0.0.28 → mismatch banner fires on a perfectly healthy connection. The Mac is one of the three first-class nodes in the issue's own deploy matrix, so this is not a corner case.
• Any peer on a different commit: different SHA → mismatch, even though a remote peer being on another commit is normal operation, not client staleness.

This matters because the issue's failure surfaced specifically "while adding a second environment" — the multi-environment path is exactly where the new false positives concentrate. The feature's value is a signal you trust; routinely firing it for healthy remotes invites banner fatigue and erodes the real local-drift signal it's meant to sharpen. The change conflates two distinct conditions — "my local web bundle is stale relative to the server serving it" (the actual bug) vs. "a remote peer is on a different commit" (benign) — and the SHA suffix makes the second trip the same UI as the first.

Suggested direction (maintainer's call)

Restrict the SHA-granularity comparison to the same-origin / primary connection where local bundle drift is the real risk, and keep remote-peer comparison at semver granularity (or suppress the SHA component for non-primary environments). That preserves the fix for the reported case without lighting up every cross-node connection.

Minor notes (non-blocking)

• apps/server/src/environment/ServerBuildSha.ts:4 — the regex already runs after .toLowerCase(), so the /i flag is redundant. Harmless.
• apps/web/vite.config.ts — the top-level await now spawns git rev-parse on every vite invocation (dev/test included), not just production builds. orElseSucceed(null) keeps it safe, but consider skipping it outside build mode to avoid the per-run spawn.
• The author already flags the residual gap where a never-establishing WS primary produces no UI banner (only a logged warning). That is a reasonable scope cut and separate from the issue above.

Net: the mechanism is sound and the code quality is high, but the per-environment false positive needs to be addressed (or explicitly accepted by a maintainer) before merge, since it degrades a shared web/mobile/desktop feature in the very scenario the issue describes.

[fzoll]

Addressed review feedback (commit e8f275f)

Main issue — SHA-granularity false positive on healthy remotes: fixed. resolveVersionMismatch/resolveServerConfigVersionMismatch (apps/web/src/versionSkew.ts) now strip +git.<sha> build metadata and compare at release granularity by default. Exact-SHA comparison is now opt-in via { compareBuildMetadata: true }, and I only pass it at call sites that are provably the primary/same-origin connection:

• ConnectionsSettings.tsx's primary-environment banner — opts in.
• ChatView.tsx's per-thread banner — opts in only when the active thread's environment id equals the primary environment id; falls back to release-only comparison for threads pinned to a remote peer.
• connection/platform.ts's pre-flight descriptor check — opts in. Both its callers (loadPrimaryConnectionRegistration, loadSecondaryConnectionRegistration) bootstrap a same-machine connection built from this client's own checkout (primary origin, or a desktop-local WSL backend), so SHA drift is still a meaningful signal there.
• ConnectionsSettings.tsx's saved/remote environment rows (SavedBackendListRow) pass no option, so they get the new semver-only default — this is the exact spot the review flagged, and it's also structurally guaranteed to never be the primary (savedEnvironments filters out PrimaryConnectionTarget entries).

Added test coverage in versionSkew.test.ts for both the default (strips metadata) and compareBuildMetadata: true (exact) paths, plus confirming a genuine release-version mismatch still fires either way.

Minor notes: both addressed.

• Dropped the redundant /i flag on the SHA regex in both ServerBuildSha.ts and vite.config.ts (input is already lowercased before the test).
• vite.config.ts now only spawns git rev-parse when command === "build", so vite dev/vitest no longer pay that spawn on every invocation.

Verified:

• pnpm --filter @t3tools/web test — full suite, 160 files / 1380 tests, all pass.
• pnpm --filter t3 test — full suite, 170 files / 1530 tests pass (2 skipped); one unrelated transient failure on the first run that passed clean on a targeted rerun, consistent with the pre-existing flakiness already noted in the PR description.
• pnpm --filter t3 --filter @t3tools/web --filter @t3tools/contracts typecheck — clean.
• vp lint on all changed files — clean.

[fzoll]

VERDICT:APPROVED

Summary

This is a well-scoped, well-tested implementation of Option 2 from #1, and the prior-review blocker has been correctly resolved in commit e8f275fa. I re-checked the fix against the PR head rather than taking the addressed-feedback comment at face value, and the claims hold up.

The prior blocker is genuinely fixed

The earlier review flagged that raw SHA-granularity comparison would fire false "version drift" banners on healthy remote/desktop connections — precisely in the multi-environment path the issue describes ("while adding a second environment"). The current diff resolves this correctly:

• versionSkew.ts now strips +git.<sha> and compares at release granularity by default; exact-SHA comparison is opt-in via { compareBuildMetadata: true }. This is the right default — a fleet on the same release but different commits no longer trips the banner.
• Packaged Mac desktop case (the one that would have regressed): the local Mac has no .git, so both client APP_VERSION and server serverVersion stay plain 0.0.28; the primary comparison is 0.0.28 vs 0.0.28 → equal, no false positive. Confirmed the logic, not just the description.
• Opt-in is restricted to provably-primary/same-origin call sites:
  • ConnectionsSettings.tsx:1830 primary banner opts in; SavedBackendListRow (:1390) passes no option → semver-only. And savedEnvironments structurally excludes primary (.filter(... target._tag !== "PrimaryConnectionTarget") at :1723), so a remote row can never accidentally get SHA comparison. Verified in the head file.
  • ChatView.tsx:1775 opts in only when activeThread.environmentId === primaryEnvironmentId (primaryEnvironmentId defined at :1563), falling back to release-only for threads pinned to a remote peer. Verified.

Correctness of the core mechanism

• Server stamps the checkout SHA at runtime via ProcessRunner (ServerBuildSha.ts), falling back to null on any failure (no .git, no git binary, timeout, non-repo) → plain semver. Correct for packaged builds.
• Web build stamps the same SHA in vite.config.ts, gated on command === "build" and an explicit APP_VERSION override for releases. Both sides use a 12-char slice of the 40-char SHA — consistent, so a matched checkout compares equal and a stale bundle compares unequal. This is exactly the intended win for the RPi case.
• Graceful degradation is right throughout; no new schema fields, reuses existing mismatch UI.

Tests

ServerBuildSha.test.ts covers the meaningful failure modes (success, non-zero exit, timeout, malformed output, spawn ENOENT). versionSkew.test.ts adds coverage for both the strip-by-default and compareBuildMetadata: true paths plus a genuine release mismatch. Adequate.

Minor, non-blocking

• platform.ts's loadSecondaryConnectionRegistration opts into compareBuildMetadata: true on the reasoning that both callers bootstrap a same-machine connection. If a secondary can ever be a genuinely remote peer, this would emit a spurious log warning — but it's log-only (no UI banner), so impact is negligible. Worth a glance if secondary targets broaden later.
• The author already documented the residual gap where a WS primary that never establishes yields only a logged warning, not a UI banner. Reasonable scope cut and orthogonal to this change.

Net: the mechanism is sound, the earlier false-positive regression is properly fixed and scoped, code quality is high, and the deliberate scope (Option 2 only) matches the issue's own framing. Approving.