feat(fork): build with official T3 Connect public config - #35
Conversation
Fork builds compiled T3 Connect out because upstream injects its four public values (Clerk publishable key, JWT template, CLI OAuth client id, relay URL) from release-time CI variables this fork does not have. Track those values — read from the published t3@0.0.30 CLI, public identifiers by upstream's own docs — in a repository-root .env, force-added past .gitignore so every clone and worktree builds at parity with official releases against the hosted relay. .env.local stays gitignored and outranks it for personal or self-hosted-relay overrides. Registered as t3-connect-official-config in the fork manifest, with a guard pinning the .env's key set to exactly the four public values (so secret creep fails CI), asserting the file stays tracked, and exercising upstream's public-config loader against the repo root. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018u3xDVxSAq2CyaDVhhB2sg
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
REQUEST_CHANGES — wrong ownership boundary for Connect public config
The intent is right: fork clones/worktrees should build with the same four T3 Connect public values as official artifacts, and reusing loadRepoEnv is the right instinct.
The implementation co-opts repository-root .env — a path upstream treats as a gitignored, mutable scratchpad. infra/relay/scripts/deploy.ts (reconcileRootEnvPublicConfig) rewrites that file and appends OTLP keys. Force-tracking it and pinning “exactly four keys” turns a normal relay deploy / local reconcile into a dirty tree, a guard failure, and an accidental-commit hazard. The guard then has to compensate for gitignore magic that !.env.example already shows how to avoid.
Refactor: put the four values in a fork-owned file (e.g. .fork/t3-connect.public.env), teach loadRepoEnv a small fenced lowest-precedence read, leave .env / .env.local alone as override/scratchpad. Tighten the loader guard with a temp repoRoot and toMatchObject projections from one OFFICIAL_VALUES constant — same pattern as scripts/lib/public-config.test.ts.
Do not land the force-tracked .env as the durable design. Avoiding a Tier-4 loader hunk is not worth co-opting the mutable scratchpad.
Sent by Cursor Automation: Thermo-nuclear PR review
| # Official T3 Connect public configuration, identical to what upstream bakes | ||
| # into every released artifact (read from the published npm CLI, t3@0.0.30). | ||
| # These are public identifiers, not secrets — see docs/cloud/t3-connect-clerk.md. | ||
| # Tracked on purpose (force-added past .gitignore's .env* rule) so every clone |
There was a problem hiding this comment.
blocker: tracking repo-root .env fights upstream’s contract for this path. .gitignore ignores it on purpose, docs treat it as optional local config, and infra/relay/scripts/deploy.ts (reconcileRootEnvPublicConfig) mutates it — rewriting T3CODE_RELAY_URL and appending T3CODE_MOBILE_OTLP_* / T3CODE_RELAY_CLIENT_OTLP_* keys (including ingest tokens). A force-tracked “exactly four keys” pin means a normal relay deploy dirties git, fails the guard, and risks committing stage URLs/tokens.
code judo: put these four values in a fork-owned file (e.g. .fork/t3-connect.public.env) and add a small fenced lowest-precedence read in scripts/lib/public-config.ts. Keep .env / .env.local as the override/scratchpad layer deploy already owns. Avoiding a Tier-4 loader hunk is not worth co-opting the mutable scratchpad.
| service. | ||
| tier: 1 | ||
| files: | ||
| - .env |
There was a problem hiding this comment.
important: files: [.env] encodes the wrong ownership boundary. Manifest this as a fork-owned path under .fork/, with watch: [scripts/lib/public-config.ts] only for the small fenced read — not for “we force-added a gitignored upstream scratchpad.” Intent prose should also name the reconcileRootEnvPublicConfig collision so the next sync reviewer does not re-learn it.
| never conflict with a path it has never shipped, and `.env.local` (still ignored, higher | ||
| precedence in `scripts/lib/public-config.ts`) remains the escape hatch for anyone pointing a | ||
| checkout at a staging or self-hosted relay. The alternative — carrying the values as code-level | ||
| fallbacks — would have meant a Tier-4 fence in an upstream loader that syncs would fight over. |
There was a problem hiding this comment.
This decision frames force-adding .env as avoiding a Tier-4 loader fence, but it never accounts for deploy.ts rewriting that same file. Please revisit: a tiny fenced read of a .fork/* public-env file is the smaller structural cost. Document why .env was rejected once that collision is in scope.
| // baseEnv: {} isolates from ambient CI variables; a developer's local | ||
| // .env.local may override individual values, so this asserts presence | ||
| // and projection rather than exact contents. | ||
| const env = loadRepoEnv({ baseEnv: {}, repoRoot }); |
There was a problem hiding this comment.
important: baseEnv: {} does not isolate .env.local — loadRepoEnv still merges it from this repoRoot. Presence-only checks against a hand-maintained projected list can pass on the wrong source and will not catch value drift through the loader.
Prefer the existing scripts/lib/public-config.test.ts pattern: write official values into a temp dir, loadRepoEnv({ baseEnv: {}, repoRoot: temp }), and toMatchObject the canonical + VITE_* / EXPO_PUBLIC_* projections from one OFFICIAL_VALUES constant. Assert the real fork-owned file’s exact contents in a separate test.
| } as const; | ||
|
|
||
| describe("fork guard: t3-connect-official-config", () => { | ||
| it("keeps .env tracked, not merely present on disk", () => { |
There was a problem hiding this comment.
If we stay on a force-tracked .env (not recommended), this test exists because an untracked copy is invisible under .env*. The repo already has the honest pattern (!.env.example). Either add a fenced !.env exception, or — better — stop tracking .env so this silent-untracked class of failure goes away with the ownership fix.
Review:
|
The tracked .env broke upstream's connectCliAuth tests, which model an unconfigured clone by leaving VITE_CLERK_CLI_OAUTH_CLIENT_ID unstubbed — no longer the ambient default once every build bakes the official id. Two fenced stubs blank it to restore the case the tests assert. Review follow-ups from #35: correct fork-clerk-launch-resilience's now-false premise (fork builds are keyed; the keyless skip remains the net for unconfigured checkouts, and keyed coverage already lives in DesktopClerk.test.ts's fenced key bake); fence a warning into .env.example against `cp .env.example .env` clobbering the tracked file; make the guard docblock honest about what it detects and record upstream value rotation as a known gap; distinguish a missing git binary from the tracking regression in the guard; and record the accepted residual risks (committable .env, production defaults in dev) in the decisions note. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018u3xDVxSAq2CyaDVhhB2sg
|
Addressed in 04c72b5, plus the CI failure that predated the review. Point by point: CI (Test job): §1 — keyed launch path. Agreed on the manifest: the One correction to the review, though: "zero coverage on the path now shipped" isn't quite right. Upstream's I tried to dispatch that dry run on this branch ( On risk framing: a keyed fork build is the exact configuration every official desktop release ships, and §2 — §3 — §4 — guard honesty. Docblock rewritten to say what test 2 actually is: a secret-creep and copy-drift fence between two hand-maintained copies. Upstream rotation is recorded as a known gap in the decisions note — both copies keep agreeing while the relay handshake dies. A live probe was considered and declined; guards don't do network. §5 — production defaults in dev. Recorded in the decisions note: dev runs are now keyed against production Clerk/relay by default, and the CLI consent flow round-trips through upstream's hosted app under upstream's application name. Both are what parity means; both overridable via §6 — nits. The tracking assertion now uses Remaining before merge: the human-dispatched Generated by Claude Code |
Code review — 4 findingsRead the loader ( Checks that came back clean, so they don't need re-litigating: no test outside 1. Keyed builds re-enter the launch race
|
Guard the .env.example fork note (an upstream sync could drop the only warning against `cp .env.example .env`, and watch: only reports drift); give the key-set assertion a failure message naming the two legitimate writers that trip it (relay deploy's reconcileRootEnv, optional OTLP values) and pointing them at .env.local; correct every claim that .env.local can turn T3 Connect off — the loader's firstNonEmpty treats blank as unset and falls through, so disabling means temporarily deleting the tracked .env; and record the reconcileRootEnv token-write hazard in the decisions note with deploy.ts drift-watched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018u3xDVxSAq2CyaDVhhB2sg
|
All four verified against source; three fixed in 19d7c57, the fourth remains the gate dispatch. 1 — keyed launch race. No dispute on the mechanism, and agreed the dry-run gate is what settles it — before merge, not before the next tag; my earlier phrasing was looser than it should have been. The dispatch is still blocked for me ( 2 — 3 — 4 — nothing guards the Verified locally: 186 guard tests green, fixed Generated by Claude Code |
|
Re the Cursor automation review (pinned to 0ee39d2, pre-dating both fix commits): declining the proposed refactor, with reasons — its one factual finding is already addressed. What it got right is the same mechanism the second review's finding 2 identified: Why not On " The loader-guard tightening suggestion (temp Status on the latest commit (19d7c57): all CI green — Test, Check, Release Smoke, Mobile Native Static Analysis. Outstanding before merge remain the two human steps: the Generated by Claude Code |
…ork-compatibility-ik9szb
This branch configures fork builds with the official T3 Connect public values, which made the release body's 'cloud features are not configured and will be unavailable' paragraph false. Say what is now true: sign-in and remote access work against upstream's hosted relay, and local use still needs no account. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tration The v0.1.7 dry run's launch isolation gate caught the keyed packaged build crashing at launch: @clerk/electron's createClerkBridge unconditionally calls protocol.registerSchemesAsPrivileged, which throws once Electron is ready, and Effect layer construction trails the ready event on a packaged boot. main.ts already registers the identical privilege set for both schemes at module load, so make it the sole registrar on every path: createDesktopClerkBridge no-ops the registrar for the duration of the bridge call and restores it after. Covered by a new fork-owned unit test, pinned by the launch-resilience guard, recorded in the manifest. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The dry-run gate did its job: run 30508846869 failed at the launch isolation check. The keyed packaged build crashed at launch with Fixed in 7eedbbb by finishing what Also in this push: merged A fresh dry-run is dispatched from the branch head; merging only on green. 🤖 Generated with Claude Code |
…le claim The decisions note still argued a keyed fork build was "no worse-positioned in the ready race than an official build" — true about registration order, useless about the crash, and disproven by the first keyed dry-run gate (run 30508846869). Correct that bullet and add the fork-clerk-launch-resilience incident section: what failed, why the registrar no-op in 7eedbbb was chosen over an isReady() gate or catching the initialization error, and the known tradeoff if @clerk/electron ever changes its registered privilege set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018u3xDVxSAq2CyaDVhhB2sg


What Changed
Fork builds now carry the same T3 Connect public configuration as official releases, so the cloud UI, the
t3 connectCLI group, and the managed tunnel work against upstream's hosted relay out of the box..env(repo root, tracked): the four official public values — Clerk publishable key,t3-relayJWT template, CLI OAuth client ID, andhttps://relay.t3.codes— read from the publishedt3@0.0.30npm package, where upstream's release build inlines them. Force-added past.gitignore's.env*rule on purpose: tracked files are exempt from ignore rules, upstream never ships a.env, so syncs can never conflict with it..env.localstays gitignored and higher-precedence for personal or self-hosted-relay overrides..fork/customizations.yaml: newt3-connect-official-configentry (Tier 1) with the loaderscripts/lib/public-config.tsdrift-watched.apps/web/src/__fork_guards__/t3ConnectOfficialConfig.test.ts: guard asserting the.envstays tracked (an untracked copy looks identical on disk but vanishes from fresh clones), pins its key set to exactly the four public values so secret creep fails CI, and exercises upstream's loader against the real repo root so a sync that renames the variables or stops reading.envturns CI red..fork/notes/FORK-CUSTOMIZATION-DECISIONS.md: where the values came from, rejected alternatives (self-hosting the relay, untracked per-clone.env), and how to refresh on rotation.No code changes were needed:
fork-clerk-launch-resilience's keyless skip only engages when no publishable key is baked (keyed builds keep upstream's Clerk bridge behavior exactly), andfork-app-identitydeliberately kept the sharedt3code://scheme that Clerk's desktop OAuth redirect allowlist expects. The values are public identifiers, not secrets, perdocs/cloud/t3-connect-clerk.md; relay access itself remains gated by the signed-in user's Clerk account.Why
Upstream injects these values at release time from CI variables this fork doesn't have (
release.yml'srelay_public_configjob), so every fork build silently compiled T3 Connect out. The goal is parity with official releases on top of the fork's changes; tracking the public config in the repo gives every clone and worktree that parity with zero per-machine setup, while the existing loader precedence keeps overrides possible.Verified: all 181 fork guard tests pass, web typecheck clean, fork-owned lint gate reports zero warnings.
Checklist
Built by Claude (Fable) via Claude Code on the web.
🤖 Generated with Claude Code
https://claude.ai/code/session_018u3xDVxSAq2CyaDVhhB2sg
Generated by Claude Code