Skip to content

feat(fork): build with official T3 Connect public config - #35

Merged
NoahHendrickson merged 7 commits into
customfrom
claude/t3-connect-fork-compatibility-ik9szb
Jul 30, 2026
Merged

feat(fork): build with official T3 Connect public config#35
NoahHendrickson merged 7 commits into
customfrom
claude/t3-connect-fork-compatibility-ik9szb

Conversation

@NoahHendrickson

Copy link
Copy Markdown
Owner

What Changed

Fork builds now carry the same T3 Connect public configuration as official releases, so the cloud UI, the t3 connect CLI 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-relay JWT template, CLI OAuth client ID, and https://relay.t3.codes — read from the published t3@0.0.30 npm 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.local stays gitignored and higher-precedence for personal or self-hosted-relay overrides.
  • .fork/customizations.yaml: new t3-connect-official-config entry (Tier 1) with the loader scripts/lib/public-config.ts drift-watched.
  • apps/web/src/__fork_guards__/t3ConnectOfficialConfig.test.ts: guard asserting the .env stays 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 .env turns 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), and fork-app-identity deliberately kept the shared t3code:// scheme that Clerk's desktop OAuth redirect allowlist expects. The values are public identifiers, not secrets, per docs/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's relay_public_config job), 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

  • This PR is small and focused
  • I explained what changed and why
  • I included before/after screenshots for any UI changes (no UI changes)
  • I included a video for animation/interaction changes (not applicable)

Built by Claude (Fable) via Claude Code on the web.

🤖 Generated with Claude Code

https://claude.ai/code/session_018u3xDVxSAq2CyaDVhhB2sg


Generated by Claude Code

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
@cursor

cursor Bot commented Jul 29, 2026

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M labels Jul 29, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Open in Web View Automation 

Sent by Cursor Automation: Thermo-nuclear PR review

Comment thread .env
# 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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .fork/customizations.yaml
service.
tier: 1
files:
- .env

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

important: baseEnv: {} does not isolate .env.localloadRepoEnv 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", () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Owner Author

Review: feat(fork): build with official T3 Connect public config

Read the whole diff against the branch and traced each claim into the code. The core reasoning is sound and unusually well-documented — but there is one consequence this PR causes and does not account for, and it is the kind that only shows up at release time.

What checks out

  • Precedence is exactly as claimed. loadRepoEnv resolves baseEnv.env.local.env (scripts/lib/public-config.ts:33-40), so process env and .env.local both outrank the tracked file. The escape hatch is real.
  • The values are genuinely public. docs/cloud/t3-connect-clerk.md:29-30 says so explicitly, and the CLI is a PKCE public client with no secret (:60). The "these aren't secrets" framing is correct, not hand-waving.
  • Tracked-past-.gitignore reasoning is correct. Ignore rules bind only untracked paths; git check-ignore .env returns nothing now that it is in the index, and upstream has never shipped a .env to conflict with.
  • No collateral test breakage. Upstream's own scripts/lib/public-config.test.ts builds every fixture in mkdtemp dirs, so a real repo-root .env cannot perturb it. And apps/web/tsconfig.json already carries "../../scripts/lib/public-config.ts" in include (pre-existing, not added here), so the guard's cross-package import typechecks legitimately rather than by luck.
  • util.parseEnv on the actual file yields exactly the four expected pairs — the comment header parses clean.

1. This flips fork desktop builds onto the launch path fork-clerk-launch-resilience exists to avoid — and nothing in this PR tests it

This is the blocking one.

DesktopClerk.ts:99 skips the Clerk bridge only when desktopClerkFrontendApiHostname === undefined, and that value is derived from the baked publishable key (:68-70). As of this commit every fork desktop build bakes pk_live_…, so the hostname is defined, the skip never fires, and createClerkBridge runs during Effect layer construction — the thing the code comment at DesktopClerk.ts:89 calls "a live hazard."

The PR body frames this as a no-op: "the keyless skip only engages when no publishable key is baked (keyed builds keep upstream's Clerk bridge behavior exactly)." True as stated, but read it the other way round — it means this PR moves the fork off the hardened path and onto the one whose race the v0.1.2 launch isolation gate caught, where the app exited before its server ever started and, per the manifest's own words, "CI runners lose it deterministically."

The manifest entry did anticipate part of this: it records that pre-ready re-registration by a keyed build's bridge "replaces the list harmlessly" on the bundled Electron 41. That covers the case where the bridge wins the race. It does not cover the original failure, which was registerSchemesAsPrivileged throwing once the app is readymain.ts's synchronous module-load registration guarantees the privileges are set, but it cannot stop the bridge from making its own late call on a slow boot, and the same entry states keyed builds keep initialization failures "loud."

Three things follow, and I'd want all three before merge:

  • The manifest now contains a false statement. fork-clerk-launch-resilience's intent still opens with "Fork builds bake no Clerk publishable key, so the bridge is guaranteed dead weight." That is untrue as of this commit, and the entry wasn't touched. The manifest is meant to be the fork's source of truth; a Tier-4 entry whose premise silently inverted is worse than no entry.
  • Zero coverage on the path now shipped. DesktopClerkForkSkip.test.ts contains exactly one describe"DesktopClerk keyless-build skip". Everything the fork actually ships after this PR is untested by it, so CI stays green precisely because it is still testing the configuration you just stopped building.
  • No gate on this change. launch-isolation-check.sh runs only in fork-release.yml:122; ci.yml:134 merely shellchecks the script. So the flip is not exercised by anything on this PR — it gets its first real test during a fork release, which is the worst place to discover it.

2. The real cost of tracking .env is not the four values — it is that .env stopped being un-committable

You are right that these identifiers are public. That is not the exposure. The exposure is the file's new status.

Before this commit, .gitignore:11,36 made it structurally impossible to commit .env. That property is now gone: git commit -am and git add -A will stage any edit to it. And .env is simultaneously the single file a developer or agent is most likely to drop a CLERK_SECRET_KEY into — both .env.example and docs/cloud/t3-connect-clerk.md:150 warn against exactly that, which tells you it is a live temptation, not a hypothetical.

The exact-key-set assertion is the right instinct, but it is CI-only and post-hoc: by the time it goes red, the secret is already in the branch history and pushed to GitHub, where the only remediation is a history rewrite plus rotation. There is no pre-commit tooling in this repo to catch it earlier (no husky, no lefthook, no simple-git-hooks).

I don't think this sinks the approach — the tradeoff is defensible and the alternatives really are worse. But it should be named as accepted residual risk in the decision note rather than left implicit, and it is worth confirming push protection / secret scanning is enabled on the fork, since that is the control actually positioned to catch this before the push lands.

3. .env.example still tells developers the opposite, and isn't watched

.env.example opens with "Leave these unset to disable optional T3 Connect features in local source builds" and lists all four as commented-out placeholders. For this fork that is now false.

The concrete failure: the documented onboarding move is cp .env.example .env, which silently clobbers the tracked file, destroys parity, and leaves a dirty tracked file with nothing explaining why. Worse, .env.example is not in the customization's watch: list, so an upstream sync that edits it will never surface the coupling.

Cheap fix: a fork-fenced note at the top of .env.example pointing at the tracked .env, and add the path under watch:.

4. The guard's headline assertion is tautological on the failure mode that actually matters

The docblock claims three silent-death modes, but look at what test 2 compares: .env on disk against OFFICIAL_VALUES hardcoded in the same commit. Those are two hand-maintained copies of one fact, and the decision note confirms they are updated together — "the fix is re-extracting from the current release and updating .env and the guard's OFFICIAL_VALUES together."

So the assertion can only catch "somebody edited one copy and not the other," which is not a failure anyone was going to have. The failure that actually breaks T3 Connect is upstream rotating the values, after which both copies agree perfectly, the guard stays green, and the fork silently talks to a dead relay. Nothing in this PR detects that — and since it is the one failure mode with real user impact, leaving it undetected while the test reads as comprehensive is the part I'd push back on.

Keep the test — the exact-key-set equality is a genuinely good secret-creep fence and I'd not want it removed. But the docblock should describe what it does (fence + copy-drift), and rotation-detection should be recorded as a known gap in the decision note rather than implied as covered.

5. Local dev now points at production Clerk and the production relay by default

pk_live_Y2xlcmsudDMuY29kZXMk base64-decodes to clerk.t3.codes$ — upstream's live production instance. So every vp run dev, including throwaway agent worktrees spun up for test-t3-app, now boots with cloud UI live against upstream production, where previously T3 Connect was compiled out entirely. .env.local overrides it, but nothing tells a developer the default changed.

Related and worth stating explicitly rather than leaving to be discovered: T3CODE_HOSTED_APP_URL is unset, so it falls back to https://app.t3.codes (apps/server/src/cloud/publicConfig.ts:110-111). Combined with upstream's CLI OAuth client ID, the fork's t3 connect login flow sends users to upstream's hosted app, and their consent screen names upstream's application. That is exactly what "parity" means and I'm not arguing against it — but it is a user-visible identity consequence, and the decision note is the right place to say so out loud.

6. Nits

  • execSync("git ls-files --error-unmatch .env") fails identically whether .env is untracked or git simply is not on PATH, so an environment problem reports itself as the regression. Minor, but the failure message will mislead whoever hits it.
  • The guard covers repo-root build configuration but lives in apps/web — consistent with the fork's convention, so I'd leave it, just noting it only runs when the web project runs, while the config it guards feeds server, desktop and mobile equally.

Verification I did not do: I read and traced rather than executed — node_modules isn't installed in this environment, so I did not re-run the 181 guard tests, typecheck, or lint. Everything above is from source, with the one runtime check I could do standalone (util.parseEnv against the real .env) confirming the parse is clean.

Bottom line: §2–§6 are all fixable with documentation and comment edits and needn't hold the PR up long. §1 is different in kind — this PR changes which desktop launch path the fork ships, that path is the one with the known race, and no gate on this PR exercises it. I'd want the manifest corrected and the launch isolation check run against a keyed build before this lands.


Generated by Claude Code

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
@github-actions github-actions Bot added size:L and removed size:M labels Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Addressed in 04c72b5, plus the CI failure that predated the review. Point by point:

CI (Test job): connectCliAuth.test.ts models an unconfigured clone by not stubbing VITE_CLERK_CLI_OAUTH_CLIENT_ID — which stopped being the ambient default once the tracked .env keyed every build. Two fenced stubs now blank it in the two tests asserting the unconfigured case, and the file is claimed under watch:. Verified locally: all 4 tests pass, full guard suite (185 tests) green, web typecheck and the fork lint gate clean.

§1 — keyed launch path. Agreed on the manifest: the fork-clerk-launch-resilience intent's premise was false as of this PR and is now rewritten — keyless-skip is the net for unconfigured checkouts; packaged fork builds are keyed and take upstream's bridge path.

One correction to the review, though: "zero coverage on the path now shipped" isn't quite right. Upstream's DesktopClerk.test.ts covers the keyed bridge — its existing fork fence bakes __T3CODE_BUILD_CLERK_PUBLISHABLE_KEY__ in via vi.hoisted precisely so upstream's bridge tests keep running against a keyed module. DesktopClerkForkSkip.test.ts was always only the keyless half. What has no unit coverage is the ready-race itself, which is only observable in a real packaged boot — and that's what fork-release.yml's launch isolation gate is for. It already supports exactly what you asked for: dry_run=true builds and runs the gate without publishing.

I tried to dispatch that dry run on this branch (version=0.0.0-fork.35, dry_run=true) and got 403 Resource not accessible by integration — the app token doesn't hold actions: write. It needs a human dispatch before merge: Actions → Fork Release → run from claude/t3-connect-fork-compatibility-ik9szb with dry_run checked.

On risk framing: a keyed fork build is the exact configuration every official desktop release ships, and main.ts's synchronous module-load registration still runs on keyed builds — strictly earlier than the bridge's own call — so the fork is no worse-positioned in the race than the official app. The residual hazard (bridge's late re-registration on a slow boot throwing loudly) is upstream's shipped behavior, kept loud on purpose per the entry's own intent.

§2 — .env now committable. Named as accepted residual risk in the decisions note, including that the guard's key-set assertion is CI-only and post-push. Push protection / secret scanning is the control positioned to catch it pre-push; I can't verify repo settings from a checkout, so please confirm it's enabled — noted in the decisions entry as an owner action.

§3 — .env.example. Fenced note added at the top (don't cp .env.example .env; use .env.local), and the file is now under watch: so an upstream rewrite surfaces the coupling.

§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 .env.local.

§6 — nits. The tracking assertion now uses spawnSync and reports a missing git binary as an environment problem instead of the regression. Guard placement left as-is per your note.

Remaining before merge: the human-dispatched dry_run fork release on this branch, and confirming push protection in repo settings.


Generated by Claude Code

@NoahHendrickson

Copy link
Copy Markdown
Owner Author

Code review — 4 findings

Read the loader (scripts/lib/public-config.ts), every consumer of the four variables across web/desktop/server/mobile, and the fork guard suite.

Checks that came back clean, so they don't need re-litigating: no test outside apps/web/src/cloud/ depends on these variables being unset (the one that did is patched here); desktop and server tests drive __T3CODE_BUILD_* through globals or explicit env, so they're unaffected by the tracked .env; detect-drift.mjs's line parser handles the comments interleaved in the new watch: list; and the t3code:// scheme claim holds, so the Clerk desktop redirect-allowlist reasoning is sound.


1. Keyed builds re-enter the launch race fork-clerk-launch-resilience exists to remove — .env:11

apps/desktop/vite.config.ts bakes T3CODE_CLERK_PUBLISHABLE_KEY from the tracked .env into __T3CODE_BUILD_CLERK_PUBLISHABLE_KEY__, so desktopClerkFrontendApiHostname is now defined for every fork build — packaged and dev:desktop. DesktopClerk.make therefore takes the else branch and calls createDesktopClerkBridge, whose own fenced comment states the hazard:

createClerkBridge calls protocol.registerSchemesAsPrivileged, which throws once Electron is "ready", and layer construction races that event (CI runners lose deterministically; a cold local boot rolls the same dice).

On a cold boot where ready fires before the Clerk layer constructs, the bridge throws, DesktopClerkBridgeInitializationError propagates (deliberately loud, uncaught), and the app exits before its server starts — the exact v0.1.2 failure.

main.ts's synchronous pre-ready registration does not mitigate this. It prevents the missing-privileges case; it does nothing about the bridge's own post-ready call throwing. The manifest rewrite reasons only about pre-ready re-registration being harmless, which is a different question.

The PR ships no code change here and no gate run — its own note says to "run fork-release.yml with dry_run=true to prove a keyed build before tagging a release", which is an accurate description of the keyed launch path being untested on this fork. That gate run is the thing that would settle this, and it should happen before merge rather than before the next tag.

2. The relay deploy script writes tracing tokens into the now-tracked .env.env:10

reconcileRootEnv (infra/relay/scripts/deploy.ts:208-218) runs on every relayDeploy invocation whose outcome carries public config — including the --read-state path — and rewrites <repoRoot>/.env via reconcileRootEnvPublicConfig, adding T3CODE_MOBILE_OTLP_TRACES_TOKEN and T3CODE_RELAY_CLIENT_OTLP_TRACES_TOKEN. The client one is ::add-mask::-ed in CI, i.e. the repo already treats it as a secret.

Before this PR that write landed in an untracked, ignored file and could not be committed. Now it lands in a tracked file, shows up as an ordinary modification, and a routine git commit -am publishes the tokens.

The "Never add server-side secrets to this file" comment and the guard's exact-key-set assertion are both after the fact: the guard turns red only once the tokens are already in the working tree. Self-hosting the relay was explicitly rejected in the decisions note, so this is a narrow path — but it's an in-repo script that automates precisely the failure the guard is fencing, and the notes don't mention it. Worth either a line in the decisions note or teaching reconcileRootEnv to write .env.local.

Same mechanism produces a false positive: anyone who legitimately uses the repo-root .env for the optional values .env.example documents (mobile OTLP tracing) now fails the guard locally with a "secret creep" message that doesn't explain the real rule.

3. .env.local cannot blank a value it claims to override — .env:9

Four places claim .env.local is the per-checkout escape hatch: this line, the .env.example fence note, the manifest intent, and FORK-CUSTOMIZATION-DECISIONS.md ("both are overridable per-checkout via .env.local"). That holds for replacing a value with a different one. It does not hold for turning one off, which is the natural override for the identity consequence the notes call out.

In loadRepoEnv, resolvePublicConfig(baseEnv, localEnv, rootEnv) resolves through firstNonEmpty, which treats an empty string as absent and falls through to the tracked .env. The returned object then spreads the resolved T3CODE_*/VITE_*/EXPO_PUBLIC_* projection last, overwriting the empty .env.local entry. So:

# .env.local
T3CODE_CLERK_PUBLISHABLE_KEY=

leaves the build keyed. Setting it empty in the process env fails the same way. The only way out is deleting the tracked .env, which then fails the new guard's readFileSync assertion.

Concretely: a contributor who wants their dev runs off upstream's production Clerk instance, or who wants to reproduce the keyless desktop skip path from finding 1, cannot get there by the documented mechanism. Either the docs should say "replace, not disable", or the loader needs an explicit opt-out.

4. Nothing guards the .env.example fork note — apps/web/src/__fork_guards__/t3ConnectOfficialConfig.test.ts:43

That fenced note is the only thing telling contributors not to run the upstream-documented cp .env.example .env. Nothing enforces its survival: customizationsManifest.test.ts's "claims every file that carries a fork fence" test filters candidates with /\.(?:ts|tsx|mts|cts|js|mjs|cjs|css|sh|ya?ml)$/, so .env.example is skipped entirely, and the three tests in the new guard assert only tracking, key set, and loader projection.

An upstream rewrite of .env.example during a sync drops the note with CI green — watch: reports drift, it doesn't fail the build. The next contributor follows upstream's onboarding, runs cp .env.example .env, replaces the four live values with upstream's commented-out placeholders, and because .env is tracked, a git commit -a ships that regression to every clone. A one-line expect(readEnvExample()).toContain("fork:begin t3-connect-official-config") in the new guard closes it.


Reviewed by Claude (Opus 5) via Claude Code.

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

Copy link
Copy Markdown
Owner Author

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 (actions: write not granted to the integration), so it stays a human step: Actions → Fork Release → run from this branch with dry_run checked. One scope note: dev:desktop was already keyed for anyone with a configured .env before this PR — what changed is that the tracked file makes that every checkout, packaged builds included.

2 — reconcileRootEnv writes tokens into the tracked .env. Confirmed — it reconciles all seven relay public-config keys including both OTLP tokens, and the client one is masked in upstream's own release workflow. Recorded in the decisions note as the sharpest catch of this review: in-repo automation of exactly the failure the key-set guard fences. Left un-fenced deliberately — this fork never runs relay deploys (self-hosting was rejected), and a fenced redirect to .env.local would also need fenced edits to deploy.test.ts, which asserts the .env target — but deploy.ts is now drift-watched under the customization, and the note names the redirect as the first change to make if the fork ever self-hosts. The guard's failure message now names both legitimate writers and points them at .env.local, so tripping it reads as "wrong file", not "you leaked a secret".

3 — .env.local can't blank a value. Confirmed — firstNonEmpty treats empty as absent and the resolved projection spreads last, so a blank override leaves the build keyed. All four places that claimed otherwise now say "replace, not disable" and document the actual off-switch: temporarily delete the tracked .env in that checkout (the guard suite failing locally is the reminder not to commit it). A fenced loader opt-out was considered and declined — a fork hunk in upstream's build bootstrap to serve a local-experiment case that deletion already covers.

4 — nothing guards the .env.example note. Fixed with the one-liner you suggested (plus a .env.local содержание check): the guard now fails the build if the fence disappears, closing the cp .env.example .envgit commit -a path a green sync would otherwise leave open.

Verified locally: 186 guard tests green, fixed connectCliAuth suite green, web typecheck and fork lint gate clean.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

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: reconcileRootEnv treats repo-root .env as a writable target, so tracking the file creates a dirty-tree/accidental-commit hazard on a relay deploy. That landed in 19d7c57 the way the follow-up review accepted: recorded in the decisions note, deploy.ts drift-watched under the customization, and the guard's failure message rewritten to name the legitimate writers and point them at .env.local. The path is also narrow by construction — this fork explicitly rejected self-hosting the relay, so nothing here ever invokes relayDeploy.

Why not .fork/t3-connect.public.env + a fenced read in loadRepoEnv: that trades a documented, deploy-only hazard for a permanent Tier-4 hunk in scripts/lib/public-config.ts — upstream's build bootstrap, consumed by web, desktop, server, and mobile builds, re-fought on every sync that touches the loader. The fork's own placement ladder puts inline edits to upstream files last for exactly this reason, and a tracked .env is the opposite case: a file upstream has never shipped and can never conflict with. The first review weighed this same tradeoff and called it defensible with the alternatives worse; two review rounds since have hardened the chosen design rather than replaced it.

On "!.env.example shows how to avoid the gitignore magic": un-ignoring .env via a !.env negation would edit upstream's .gitignore and make every contributor's local .env stageable by git add -A — strictly more accidental-commit surface than one tracked file with a key-set guard, not less.

The loader-guard tightening suggestion (temp repoRoot, toMatchObject) is moot in its stated form — the guard's third test intentionally runs against the real repo root, because "the tracked file resolves through the real loader in this tree" is the property being guarded; upstream's own public-config.test.ts already covers loader semantics against temp fixtures.

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 dry_run Fork Release dispatch from this branch, and confirming push protection in repo settings.


Generated by Claude Code

NoahHendrickson and others added 3 commits July 29, 2026 22:42
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>
@NoahHendrickson

Copy link
Copy Markdown
Owner Author

The dry-run gate did its job: run 30508846869 failed at the launch isolation check. The keyed packaged build crashed at launch with DesktopClerkBridgeInitializationError, caused by protocol.registerSchemesAsPrivileged should be called before app is ready inside @clerk/electron's createClerkBridge — the exact keyed launch race finding 1 predicted. On the runner, Effect layer construction trails Electron's ready event, and the bridge's unconditional scheme re-registration throws post-ready.

Fixed in 7eedbbb by finishing what fork-clerk-launch-resilience started: main.ts already registers the identical privilege set for both schemes at module load, so it is now the sole registrar on every path — createDesktopClerkBridge no-ops the registrar for the duration of the createClerkBridge call and restores it in a finally. Everything else the bridge does (token persistence IPC, OAuth transport, passkeys) is untouched, and keyed builds keep upstream's loud init/cleanup failures. Covered by a new fork-owned unit test (DesktopClerkForkRegistrarSuppression.test.ts), pinned in the launch-resilience guard, recorded in the manifest (including a correction: the old note that pre-ready re-registration is harmless was true but answered the wrong case).

Also in this push: merged custom (PRs #33's successors #34/#36 landed there) and corrected the release-body template, which still claimed T3 Connect is unavailable in fork builds — this PR is what makes that false.

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
@NoahHendrickson
NoahHendrickson merged commit cf8c815 into custom Jul 30, 2026
10 checks passed
@NoahHendrickson
NoahHendrickson deleted the claude/t3-connect-fork-compatibility-ik9szb branch July 30, 2026 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants