Skip to content

t3x: shrink the upstream conflict surface and fix the Claude multi-account docs - #31

Merged
radroid merged 3 commits into
mainfrom
t3x/seam-hygiene
Jul 28, 2026
Merged

t3x: shrink the upstream conflict surface and fix the Claude multi-account docs#31
radroid merged 3 commits into
mainfrom
t3x/seam-hygiene

Conversation

@radroid

@radroid radroid commented Jul 28, 2026

Copy link
Copy Markdown
Owner

Groundwork for issue #29 (recurring daily-sync conflict) and the docs half of issue #30
(multiple Claude accounts). No behaviour change — one test moved, one settings string, three docs.

1. Removes the file that caused #29

packages/contracts/src/settings.test.ts is upstream-owned and churns hard (8 commits/30d). The
fork's notifyOnNeedsInput block sat at the same describe anchor upstream keeps appending to —
a plain add/add conflict (upstream 362f1279 vs fork a2ea115b). The other two "culprit commits"
in the escalation JSON are false positives; sync-upstream.sh lists every commit that touched
the file, not the ones that conflict.

Moved to packages/contracts/src/t3x/settings.t3x.test.ts. settings.test.ts is now
byte-identical to upstream, so this conflict cannot recur there.

Landing this before resolving #29 gives the resolver strictly less to do.

2. Fixes the broken Claude multi-account recipe

docs/providers/claude.md still documented the pre-eb733c10f HOME-based setup:

HOME=~/.claude_personal_home claude auth login   # wrong

Overriding HOME relocates the macOS keychain lookup — exactly what ClaudeHome.ts's own comment
says not to do. Anyone following the shipped guide logs in somewhere the provider never reads.

Verified against the installed CLI (2.1.220) rather than assumed:

Check Result
claude auth login/logout/status exist yes — the commands were never wrong, only the HOME= prefix
CLAUDE_CONFIG_DIR=<fresh dir> claude auth status loggedIn: false while default reports the real account → isolation confirmed
CLAUDE_CONFIG_DIR=~/.claude claude auth status loggedIn: true but email: null

That last row is why the settings placeholder changed: the form suggested ~/.claude, i.e. the one
value that stops the account being identified. Now an example isolated path, plus a warning in the
field description.

Also documents how to switch accounts manually (per thread, via the model picker's provider rail)
and why an existing thread cannot switch — the question the guide's own heading promises to answer.

3. Replaces the fictional seam ledger

docs/t3x/SEAMS.md claimed 2 upstream files and "Contracts / persistence: None." Measured: 34
files, +1466/-112
, including a persisted schema change and +151 in ChatView.tsx — a file the doc
named as deliberately avoided. That gap is why #29 was a surprise: the conflict landed in a file the
ledger said the fork did not touch, so its own "re-isolate rather than accept more daily sync pain"
tripwire could never fire.

Rewritten as a measured ledger with a risk column, plus the shell command that regenerates it.

Runbook corrections (each verified against the workflow it documents):

  • "No independent CI gates the PR" was stale — but the fix isn't "CI covers it". The resolver pushes
    with GITHUB_TOKEN, which fires no workflow runs, so CI must be dispatched by hand.
  • The @claude resolve comment path is hard-wired to claude-sonnet-5; only workflow_dispatch
    can pick a model. Documents the 45min/150-turn budget and that the one successful resolve used 81%
    of it for a smaller range than the one now pending.
  • In-flight branches need rebase --onto <old-main>, not a plain rebase.
  • rerere only helps a local clone, never the CI resolver.

Verification

vp run test 14/14 packages green · vp run typecheck exit 0 · vp run lint exit 0
(remaining output is pre-existing upstream warnings/suggestions).

Not included

The ClaudeHome.ts blank-homePath normalisation from the review. Testing turned up evidence
against it — see the PR discussion.

🤖 Generated with Claude Code

radroid and others added 3 commits July 28, 2026 13:05
`packages/contracts/src/settings.test.ts` is upstream-owned and churns hard
(8 commits in 30 days). The fork's `notifyOnNeedsInput` block was inserted at
the same `describe("ClientSettings glass opacity")` anchor that upstream keeps
appending to, which produced a plain add/add rebase conflict on the daily
upstream sync — the exact failure reported in issue #29 (upstream 362f127 vs
fork a2ea115).

Moving the three cases into a fork-owned file removes that file from the
conflict surface permanently. `settings.test.ts` is now byte-identical to
upstream again, so this class of conflict cannot recur there.

The two decode helpers are re-declared rather than imported because the
upstream test file keeps them module-local.

No behaviour change; 27 tests still pass across both files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`docs/providers/claude.md` was written when T3 Code isolated Claude instances
by overriding HOME. Upstream eb733c1 switched to CLAUDE_CONFIG_DIR and
updated the code and the settings label, but never updated this guide, so the
shipped second-account instructions have been wrong since then:

    HOME=~/.claude_personal_home claude auth login

Overriding HOME also relocates the macOS login-keychain lookup, which is
exactly what ClaudeHome.ts's own comment says not to do — the CLI cannot find
its stored OAuth credentials and reports "Not logged in". Anyone following the
guide logs in to a location the provider never reads.

Verified against the installed CLI (2.1.220) rather than assumed:

  - `claude auth login` / `logout` / `status` all exist, so the commands were
    never the problem — only the `HOME=` prefix was.
  - `CLAUDE_CONFIG_DIR=<dir> claude auth status` in a fresh dir reports
    `loggedIn: false` while the default reports the real account, confirming
    the corrected recipe genuinely isolates accounts.
  - `CLAUDE_CONFIG_DIR=~/.claude claude auth status` reports `loggedIn: true`
    but `email: null`, where leaving it unset reports the real email. So the
    `~/.claude` placeholder in the settings form invited users to type the one
    value that stops the account being identified — replaced with an example
    isolated path and a warning in the field description.

Also documents how to switch accounts manually (per thread, via the model
picker's provider rail) and why an existing thread cannot switch, since that
is the question the guide's title promises to answer.

Remaining `Claude HOME path` labels, `_home` directory names and the second
`HOME=` recipe in the router section are corrected the same way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SEAMS.md claimed the fork's upstream conflict surface was 2 files and that
"Contracts / persistence: _None._". Measured against the merge-base it is 34
files, +1466/-112, including a persisted ClientSettingsSchema change and a
+151 edit to ChatView.tsx — a file the doc explicitly named as deliberately
avoided.

That gap is why issue #29 was a surprise: the conflict landed in a file the
ledger said the fork did not touch, so the doc's own "re-isolate rather than
accept more daily sync pain" tripwire could never fire.

Rewritten as a measured ledger with a risk column (fork lines x upstream
churn), the top offenders called out, and — most importantly — the shell
command that regenerates it, so the next person can check rather than trust.
Records that settings.test.ts has been removed from the surface and that the
t3x/ sibling-file pattern is how to keep fork tests out of upstream specs.

Also corrects the resolver runbook against the workflows it documents:

  - "No independent CI gates the PR" was stale (t3x-ci.yml exists) but the
    replacement is not "CI covers it": the resolver pushes with GITHUB_TOKEN,
    which fires no workflow runs, so CI must be dispatched by hand.
  - The comment path is hard-wired to claude-sonnet-5; only workflow_dispatch
    can pick a model. Documents the budget (45min/150 turns), the one data
    point we have (36 commits used 81% of it), and how to raise the caps.
  - In-flight branches need `rebase --onto <old-main>`, not a plain rebase,
    because main's history is replaced rather than extended.
  - rerere only helps a local clone, never the CI resolver.
  - The daily job cannot self-trigger the resolver, and a weekly-build
    escalation shares the same label and issue.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bdddba9c-b025-4b14-b280-457b6f6bb7ec

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch t3x/seam-hygiene

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@radroid

radroid commented Jul 28, 2026

Copy link
Copy Markdown
Owner Author

Why the ClaudeHome.ts blank-homePath normalisation is not in this PR

The review recommended normalising a blank homePath to <homedir>/.claude, so that the default
instance and one explicitly set to ~/.claude stop being treated as different accounts. Two
investigation agents independently rated it SAFE / no migration — the continuation group key is
never persisted, and session bindings key on threadId + providerInstanceId. That part holds up.

But the empirical test run while fixing the docs argues against shipping it as specified:

$ claude auth status                              # blank field
  "loggedIn": true,  "email": "…@gmail.com"

$ CLAUDE_CONFIG_DIR=~/.claude claude auth status  # explicit ~/.claude
  "loggedIn": true,  "email": null

The two configurations are not equivalent at the credential layer, even though both point at the
same config directory. Merging their continuation keys would let the picker offer continuation
between a properly-identified instance and one the app cannot identify — trading a cosmetic identity
split for a real behavioural one.

The user-facing problem the normalisation was meant to solve — people typing the placeholder — is
fully addressed in this PR by changing the placeholder and warning in the field description. That is
zero-risk and needs no upstream behaviour change.

If we do want the code fix later, the better shape is the inverse: treat an explicit path that
resolves to the CLI's default as equivalent to blank (i.e. don't set CLAUDE_CONFIG_DIR at all),
so both the key and the environment match. That needs care around an ambient CLAUDE_CONFIG_DIR
on the server process, which ClaudeSkills.ts documents as taking precedence. Worth its own issue
rather than smuggling into a docs PR.

@radroid
radroid merged commit 2c59e5c into main Jul 28, 2026
2 checks passed
@radroid
radroid deleted the t3x/seam-hygiene branch July 28, 2026 17:47
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