[yordis]

Problem

An older client that saves or removes any source control token wipes every other saved token. It sends the whole token map, the server replaced the stored map with it, and the result drops every other host's literal token and secret reference. An older desktop app (which bundles its own web client) connected to a newer remote server hits this.

Change

The server now merges tokens per host when applying a source control host settings patch. A host missing from the patch keeps its saved value. Removing a token still works: the current client sends "" for it, and that is stored as before. An empty tokens map still clears every token for the host kind.

The sign-in text in Settings said a saved token is used "until you remove it". A saved reference falls back to the sign-in when the vault can't be read, so it now says "while it can be read".

Scope and approval

Follow-up to #27756, which noted this as "fix before the flag goes on". It is one server-side merge in packages/shared/src/serverSettings.ts, plus one line of sign-in text in the web settings. No contract change.

Verification

• Added a case to packages/shared/src/serverSettings.test.ts that patches tokens: { "ghe.example": "x" } over a saved github.com secret reference and expects both to remain. It fails on main and passes with the fix.
• The server settings secret-lifecycle test (an empty tokens map clears all) caught a first version that merged an empty map as a no-op; the merge now applies only to a non-empty map. apps/server serverSettings and sourceControlHostSecrets tests pass.
• vp run check:changed passes (format, lint, knip, typecheck, tests).
• Ran the real applyServerSettingsPatch through save, older-client save, and "" clear: the reference and literal tokens survive the older-client save, and "" still clears a host.
• Not checked: an older client against a live server.

[macroscopeapp]

Approvability

Verdict: Approved at 65d2ad1

Macroscope's review found this PR approvable — Saving a source-control token now preserves other hosts' credentials, while explicit per-host removal and clear-all behavior remain intact. The change is limited to settings merging and explanatory text, without changing permissions or secret-storage protections.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 60aec28
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: e5656b7a-f3ac-4a7b-b645-3fac6514276c

Commits

Reviewing files that changed from the base of the PR and between 8b437c4 and 65d2ad1.

Files selected for processing: 3

• packages/contracts/src/settings.ts
• packages/shared/src/serverSettings.test.ts
• packages/shared/src/serverSettings.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

Source-control host patches now normalize and merge nonempty token maps with saved tokens. Empty token maps clear tokens, and omitted token fields preserve saved tokens. The settings contract comment and sign-in subtitle describe these behaviors.

Changes: Source-control token handling

Layer / File(s)	Summary
Merge source-control host token patches packages/shared/src/serverSettings.ts, packages/contracts/src/settings.ts, packages/shared/src/serverSettings.test.ts	Nonempty token maps merge with saved tokens. Host keys are trimmed and lowercased. Empty maps clear tokens, while omitted token fields preserve saved tokens. The contract comment documents these rules, and tests cover host-token retention and normalized duplicate keys.
Update saved-token subtitle apps/web/src/components/settings/SourceControlAppSignInSettings.tsx	The subtitle says the saved token is used while it can be read.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge, maria-rcks

Merge Risk: Minimal · up to 65d2a

The token patch behavior and sign-in guidance match the intended contract, with no merge-blocking risk identified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title is concise, uses the conventional format, and clearly identifies the main fix: preserving tokens for other source control hosts.
Description check		The description includes all required sections and clearly explains the problem, implementation, scope, approval reference, verification results, and the untested live older-client scenario. It does n…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description includes all required sections and clearly explains the problem, implementation, scope, approval reference, verification results, and the untested live older-client scenario. It does not include before-and-after screenshots for the UI text change, but the description is otherwise complete and focused.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

[coderabbitai]

• Fix comments: Fix CodeRabbit comments on this PR

Dismissing prior approval to re-evaluate 65d2ad1

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

Verified unchanged head 65d2ad1996210b163bd5c825ea0864b4ec7fb562 on a Linux Boat VM with the repo's Playwright and /usr/bin/google-chrome. Ran main and the PR sequentially, with separate dev state and one dev server at a time. No mocks or demo data.

On github/main (71646e031a), saved a real GitHub credential in Settings → Source control. Sent an authenticated server.updateSettings WebSocket request containing only a second host's token, reproducing the older client's partial token map. Main dropped the saved github.com token. The same request on the PR retained both hosts, visible after reloading the web client.

Also exercised current-client Save and Remove, trimmed/mixed-case host keys, redaction-marker round trips, a host settings patch without tokens, and clear-all with tokens: {}. GitHub discovery authenticated with the saved settings token; after clear-all it authenticated through the server's GH_TOKEN fallback. The second hostname checks storage isolation; it is not an enterprise GitHub service.

Created a project for the real checkout through the app. A real Codex gpt-6.1-sol turn read the code and ran git rev-parse; a follow-up completed after restarting the dev server.

Short runtime transcript, with credential values omitted:

main: partial save -> saved hosts [www.github.com], github.com lost
PR: partial save -> saved hosts [github.com, www.github.com]
PR: trimmed / recased host -> one github.com key, neighbor retained
PR: redaction marker round trip -> both hosts retained
PR: GitHub discovery -> authenticated using saved settings token
PR: explicit empty-string removal -> github.com retained
PR: unrelated host setting -> github.com retained
PR: empty token map -> all host tokens cleared
PR: GitHub discovery after clear -> authenticated using GH_TOKEN fallback
PR: browser reload -> both saved hosts visible
Codex follow-up -> git rev-parse --short HEAD returned 65d2ad1996

Blacksmith passed all 113 focused shared/settings/secret-lifecycle tests. vp run check:changed 6f41364805e31a1f8d59c18c7cc451630cd9265d passed format, lint, knip, affected typechecks, tests, and React compiler checks. CI has no failing or pending checks on this head. No code changes or pushes were needed.

Before, on main after the partial save:

After, on the PR with the same hosts:

Save, remove, and partial-save interaction. Inspected the capture and verified uploaded playback and seeking in T3's browser:

Not verified

• An actual older desktop binary against a remote server. Its partial-map request was exercised through the real authenticated WebSocket API; no old desktop binary was installed.
• Native iOS/Android, macOS/Windows desktop, relay/tunnel, and multi-device sessions.
• External vault resolution and unavailable-vault fallback, because this VM has no configured vault. Reference retention is covered by the focused tests, not a live vault integration.
• The changed app-sign-in subtitle in its signed-in state. GitHub app OAuth sign-in is not configured on this dev server, so that sentence was not rendered. No synthetic sign-in state was injected.