Skip to content

Commit 4cf9299

Browse files
fix(accounts): preserve shared workspace account slots (Soju06#974)
* fix(accounts): preserve shared workspace account slots * fix(accounts): align shared workspace slot CI expectations * fix(accounts): preserve shared workspace slot identity semantics * style(accounts): format shared workspace fallback * fix(accounts): preserve legacy workspace upgrades * fix(accounts): keep workspace slot fallback scoped * fix(accounts): allow additional workspace slots * fix(accounts): preserve workspace identity on oauth * fix(accounts): route oauth through account slots * fix(accounts): keep oauth slots identity-keyed * fix(accounts): scope oauth slot preservation * fix(accounts): match identity merge by email slot * fix(accounts): upgrade label-only workspace slots --------- Co-authored-by: Darafei Praliaskouski <me@komzpa.net>
1 parent a7ed878 commit 4cf9299

20 files changed

Lines changed: 550 additions & 49 deletions

File tree

.all-contributorsrc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,16 @@
706706
"contributions": [
707707
"code"
708708
]
709+
},
710+
{
711+
"login": "KakatkarAkshay",
712+
"name": "Akshay Kakatkar",
713+
"avatar_url": "https://avatars.githubusercontent.com/u/49910222?v=4",
714+
"profile": "https://github.com/KakatkarAkshay",
715+
"contributions": [
716+
"code",
717+
"test"
718+
]
709719
}
710720
],
711721
"contributorsPerLine": 7,

app/core/auth/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,9 @@ def generate_unique_account_id(
171171
account_id: str | None,
172172
email: str | None,
173173
workspace_id: str | None = None,
174+
workspace_label: str | None = None,
174175
) -> str:
175-
workspace_key = clean_account_identity_part(workspace_id)
176+
workspace_key = clean_account_identity_part(workspace_id) or clean_account_identity_part(workspace_label)
176177
if account_id and workspace_key:
177178
workspace_hash = hashlib.sha256(workspace_key.encode()).hexdigest()[:8]
178179
return f"{account_id}_{workspace_hash}"

app/modules/accounts/mappers.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ def build_account_summaries(
5959

6060

6161
def _duplicate_detection_keys_appearing_more_than_once(accounts: list[Account]) -> set[tuple[str, str, str | None]]:
62-
"""Return duplicate (email, ChatGPT account id, workspace id) keys in this list.
62+
"""Return duplicate (email, ChatGPT account id, workspace slot key) keys in this list.
6363
6464
Emails are compared case-sensitively to match the storage normalization
6565
already performed at OAuth-import time. Blank/None emails, the legacy
@@ -81,7 +81,7 @@ def _duplicate_detection_key(account: Account) -> tuple[str, str, str | None] |
8181
chatgpt_account_id = account.chatgpt_account_id
8282
if not _is_duplicate_detection_email(email) or not chatgpt_account_id:
8383
return None
84-
return email, chatgpt_account_id, account.workspace_id
84+
return email, chatgpt_account_id, account.workspace_id or account.workspace_label
8585

8686

8787
def _is_duplicate_detection_email(email: str | None) -> bool:
@@ -222,6 +222,7 @@ def _account_to_summary(
222222
)
223223
return AccountSummary(
224224
account_id=account.id,
225+
chatgpt_account_id=account.chatgpt_account_id,
225226
email=account.email,
226227
alias=account.alias,
227228
display_name=account.alias or account.email,

app/modules/accounts/repository.py

Lines changed: 83 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -202,13 +202,15 @@ async def upsert(
202202
canonical = await self._account_by_chatgpt_identity(
203203
account.chatgpt_account_id,
204204
workspace_id=account.workspace_id,
205+
email=account.email,
205206
)
206207
if canonical is not None:
207208
_apply_account_updates(canonical, account)
208209
await self._reconcile_chatgpt_identity_duplicates(
209210
canonical=canonical,
210211
chatgpt_account_id=account.chatgpt_account_id,
211212
workspace_id=account.workspace_id,
213+
email=account.email,
212214
)
213215
await self._session.commit()
214216
await self._session.refresh(canonical)
@@ -241,15 +243,14 @@ async def upsert(
241243
return account
242244

243245
async def upsert_reauthorized(self, account: Account) -> Account:
244-
if account.chatgpt_account_id:
245-
return await self.upsert(account, merge_by_email=False, merge_by_chatgpt_identity=True)
246246
return await self.upsert_account_slot(account, preserve_unknown_workspace_duplicates=False)
247247

248248
async def upsert_account_slot(
249249
self,
250250
account: Account,
251251
*,
252252
preserve_unknown_workspace_duplicates: bool | None = None,
253+
preserve_identity_slots: bool = False,
253254
) -> Account:
254255
if preserve_unknown_workspace_duplicates is None:
255256
preserve_unknown_workspace_duplicates = not await self._merge_by_email_enabled()
@@ -281,13 +282,12 @@ async def upsert_account_slot(
281282
return existing_by_id
282283
account.id = await self._next_available_account_id(account.id)
283284
elif not preserve_unknown_workspace_duplicates:
284-
existing_by_email = (
285-
await self._single_unknown_workspace_account_by_email(account.email)
286-
if account.workspace_id
287-
else await self._single_account_by_email(account.email)
288-
if not account.workspace_id
289-
else None
290-
)
285+
if _workspace_slot_key(account):
286+
existing_by_email = await self._single_unknown_workspace_account_by_email(account.email)
287+
elif preserve_identity_slots and account.chatgpt_account_id:
288+
existing_by_email = None
289+
else:
290+
existing_by_email = await self._single_account_by_email(account.email)
291291
if existing_by_email and not _can_reuse_email_fallback(existing_by_email, account):
292292
existing_by_email = None
293293
if existing_by_email:
@@ -306,15 +306,16 @@ async def _account_by_chatgpt_identity(
306306
chatgpt_account_id: str,
307307
*,
308308
workspace_id: str | None,
309+
email: str | None,
309310
) -> Account | None:
310311
"""Return the canonical local account row for a ChatGPT identity.
311312
312-
Order of preference, so that reauth reuses the row that already
313-
carries the historical usage and audit trail:
313+
Order of preference, so that reauth targets the matching real-email
314+
slot when one exists, while still allowing an upstream email change
315+
to reuse a single unambiguous identity row:
314316
315-
1. The oldest row by ``created_at`` (deterministic tie-break on
316-
``id``) — this is almost always the original row, before any
317-
``__copyN`` rows were created.
317+
1. The oldest row with the incoming email.
318+
2. The only identity row when no email match exists.
318319
"""
319320

320321
stmt = select(Account).where(Account.chatgpt_account_id == chatgpt_account_id)
@@ -325,19 +326,34 @@ async def _account_by_chatgpt_identity(
325326
else:
326327
stmt = stmt.where(Account.workspace_id.is_(None))
327328

328-
result = await self._session.execute(stmt.order_by(*order_by).limit(1))
329-
return result.scalar_one_or_none()
329+
result = await self._session.execute(stmt.order_by(*order_by))
330+
candidates = list(result.scalars().all())
331+
if not candidates:
332+
return None
333+
if not email:
334+
return candidates[0]
335+
336+
for candidate in candidates:
337+
if candidate.email == email:
338+
return candidate
339+
340+
if len(candidates) == 1:
341+
return candidates[0]
342+
return None
330343

331344
async def _reconcile_chatgpt_identity_duplicates(
332345
self,
333346
canonical: Account,
334347
chatgpt_account_id: str,
335348
workspace_id: str | None,
349+
email: str | None,
336350
) -> None:
337351
duplicate_stmt = select(Account.id).where(
338352
Account.chatgpt_account_id == chatgpt_account_id,
339353
Account.id != canonical.id,
340354
)
355+
if email:
356+
duplicate_stmt = duplicate_stmt.where(Account.email == email)
341357
if workspace_id is None:
342358
duplicate_stmt = duplicate_stmt.where(Account.workspace_id.is_(None))
343359
else:
@@ -642,6 +658,7 @@ async def _single_unknown_workspace_account_by_email(self, email: str) -> Accoun
642658
select(Account)
643659
.where(Account.email == email)
644660
.where(Account.workspace_id.is_(None))
661+
.where(Account.workspace_label.is_(None))
645662
.order_by(Account.created_at.asc(), Account.id.asc())
646663
.limit(2)
647664
)
@@ -653,21 +670,37 @@ async def _single_unknown_workspace_account_by_email(self, email: str) -> Accoun
653670
return matches[0]
654671

655672
async def _account_by_slot_identity(self, account: Account) -> Account | None:
656-
if account.chatgpt_account_id and account.workspace_id:
673+
workspace_slot = _workspace_slot_identity(account)
674+
if account.chatgpt_account_id and account.email and workspace_slot:
675+
column, value = workspace_slot
676+
result = await self._session.execute(
677+
select(Account)
678+
.where(Account.chatgpt_account_id == account.chatgpt_account_id)
679+
.where(Account.email == account.email)
680+
.where(column == value)
681+
.order_by(Account.created_at.asc(), Account.id.asc())
682+
.limit(1)
683+
)
684+
if matched := result.scalar_one_or_none():
685+
return matched
686+
if account.chatgpt_account_id and account.email and account.workspace_id and account.workspace_label:
657687
result = await self._session.execute(
658688
select(Account)
659689
.where(Account.chatgpt_account_id == account.chatgpt_account_id)
660-
.where(Account.workspace_id == account.workspace_id)
690+
.where(Account.email == account.email)
691+
.where(Account.workspace_id.is_(None))
692+
.where(Account.workspace_label == account.workspace_label)
661693
.order_by(Account.created_at.asc(), Account.id.asc())
662694
.limit(1)
663695
)
664696
if matched := result.scalar_one_or_none():
665697
return matched
666-
if account.workspace_id and account.email:
698+
if workspace_slot and account.email:
699+
column, value = workspace_slot
667700
result = await self._session.execute(
668701
select(Account)
669702
.where(Account.email == account.email)
670-
.where(Account.workspace_id == account.workspace_id)
703+
.where(column == value)
671704
.order_by(Account.created_at.asc(), Account.id.asc())
672705
.limit(1)
673706
)
@@ -733,10 +766,14 @@ def _slot_lock_key(account: Account, *, preserve_unknown_workspace_duplicates: b
733766

734767
def _slot_lock_keys(account: Account, *, preserve_unknown_workspace_duplicates: bool = True) -> tuple[str, ...]:
735768
keys: list[str] = []
736-
if account.chatgpt_account_id and account.workspace_id:
737-
keys.append(f"slot:{account.chatgpt_account_id}:{account.workspace_id}")
738-
if account.email and account.workspace_id:
739-
keys.append(f"slot-email:{account.email}:{account.workspace_id}")
769+
workspace_key = _workspace_slot_key(account)
770+
if account.chatgpt_account_id:
771+
if workspace_key:
772+
keys.append(f"slot:{account.chatgpt_account_id}:{workspace_key}")
773+
elif account.email:
774+
keys.append(f"slot:{account.chatgpt_account_id}:{account.email}")
775+
if account.email and workspace_key:
776+
keys.append(f"slot-email:{account.email}:{workspace_key}")
740777
if not preserve_unknown_workspace_duplicates:
741778
keys.append(f"slot-email-unknown:{account.email}")
742779
if keys:
@@ -748,13 +785,29 @@ def _slot_lock_keys(account: Account, *, preserve_unknown_workspace_duplicates:
748785

749786
def _same_unknown_workspace_identity(existing: Account, incoming: Account) -> bool:
750787
return (
751-
not existing.workspace_id
752-
and not incoming.workspace_id
788+
_workspace_slot_key(existing) is None
789+
and _workspace_slot_key(incoming) is None
753790
and existing.chatgpt_account_id == incoming.chatgpt_account_id
754791
and existing.email == incoming.email
755792
)
756793

757794

795+
def _workspace_slot_identity(account: Account) -> tuple[Any, str] | None:
796+
if account.workspace_id:
797+
return Account.workspace_id, account.workspace_id
798+
if account.workspace_label:
799+
return Account.workspace_label, account.workspace_label
800+
return None
801+
802+
803+
def _workspace_slot_key(account: Account) -> str | None:
804+
if account.workspace_id:
805+
return account.workspace_id
806+
if account.workspace_label:
807+
return account.workspace_label
808+
return None
809+
810+
758811
def _is_workspace_less_reauth_for_known_slot(
759812
existing: Account,
760813
incoming: Account,
@@ -772,6 +825,10 @@ def _is_workspace_less_reauth_for_known_slot(
772825

773826

774827
def _can_reuse_email_fallback(existing: Account, incoming: Account) -> bool:
828+
existing_workspace_key = _workspace_slot_key(existing)
829+
incoming_workspace_key = _workspace_slot_key(incoming)
830+
if existing_workspace_key and incoming_workspace_key and existing_workspace_key != incoming_workspace_key:
831+
return False
775832
return (
776833
not incoming.chatgpt_account_id
777834
or not existing.chatgpt_account_id

app/modules/accounts/schemas.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ class AccountAdditionalQuota(DashboardModel):
7272

7373
class AccountSummary(DashboardModel):
7474
account_id: str
75+
chatgpt_account_id: str | None = None
7576
email: str
7677
alias: str | None = None
7778
display_name: str

app/modules/accounts/service.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ async def import_account(self, raw: bytes) -> AccountImportResponse:
281281

282282
email = claims.email or DEFAULT_EMAIL
283283
raw_account_id = claims.account_id
284-
account_id = generate_unique_account_id(raw_account_id, email, claims.workspace_id)
284+
account_id = generate_unique_account_id(raw_account_id, email, claims.workspace_id, claims.workspace_label)
285285
plan_type = coerce_account_plan_type(claims.plan_type, DEFAULT_PLAN)
286286
last_refresh = to_utc_naive(auth.last_refresh_at) if auth.last_refresh_at else utcnow()
287287

app/modules/oauth/service.py

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ async def _persist_tokens(self, tokens: OAuthTokens) -> None:
573573
workspace_id = clean_account_identity_part(auth_claims.workspace_id or claims.workspace_id)
574574
workspace_label = clean_account_identity_part(auth_claims.workspace_label or claims.workspace_label)
575575
seat_type = normalize_seat_type(auth_claims.seat_type or claims.seat_type)
576-
account_id = generate_unique_account_id(raw_account_id, email, workspace_id)
576+
account_id = generate_unique_account_id(raw_account_id, email, workspace_id, workspace_label)
577577
plan_type = coerce_account_plan_type(
578578
auth_claims.chatgpt_plan_type or claims.chatgpt_plan_type,
579579
DEFAULT_PLAN,
@@ -596,19 +596,17 @@ async def _persist_tokens(self, tokens: OAuthTokens) -> None:
596596
)
597597
if self._repo_factory:
598598
async with self._repo_factory() as repo:
599-
if raw_account_id and workspace_id is None:
600-
await repo.upsert(account, merge_by_email=False, merge_by_chatgpt_identity=True)
601-
else:
602-
await repo.upsert_account_slot(account, preserve_unknown_workspace_duplicates=False)
603-
else:
604-
if raw_account_id and workspace_id is None:
605-
await self._accounts_repo.upsert(
599+
await repo.upsert_account_slot(
606600
account,
607-
merge_by_email=False,
608-
merge_by_chatgpt_identity=True,
601+
preserve_unknown_workspace_duplicates=False,
602+
preserve_identity_slots=True,
609603
)
610-
else:
611-
await self._accounts_repo.upsert_account_slot(account, preserve_unknown_workspace_duplicates=False)
604+
else:
605+
await self._accounts_repo.upsert_account_slot(
606+
account,
607+
preserve_unknown_workspace_duplicates=False,
608+
preserve_identity_slots=True,
609+
)
612610

613611
await self._invalidate_account_routing_caches()
614612

frontend/src/features/accounts/components/account-detail.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ export function AccountDetail({
8585
? account.email
8686
: null;
8787
const idSuffix = showAccountId ? ` (${compactId})` : "";
88-
const workspaceLabel = account.workspaceLabel || account.workspaceId || "Personal / unknown workspace";
88+
const workspaceLabel = account.chatgptAccountId || account.workspaceLabel || account.workspaceId || "Personal / unknown workspace";
8989
const seatLabel = account.seatType ? ` | ${formatSlug(account.seatType)}` : "";
9090

9191
return (

frontend/src/features/accounts/components/account-list-item.test.tsx

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ describe("AccountListItem", () => {
196196
displayName: "Work seat",
197197
email: "work@example.com",
198198
planType: "team",
199+
chatgptAccountId: null,
199200
workspaceLabel: "Design Workspace",
200201
seatType: "member",
201202
});
@@ -207,4 +208,19 @@ describe("AccountListItem", () => {
207208
screen.getByText((_, element) => element?.textContent === "work@example.com | Team | Design Workspace | Member"),
208209
).toBeInTheDocument();
209210
});
211+
212+
it("uses ChatGPT account id before workspace metadata or unknown fallback", () => {
213+
const account = createAccountSummary({
214+
planType: "team",
215+
workspaceId: "legacy-workspace-id",
216+
workspaceLabel: "Legacy Workspace",
217+
chatgptAccountId: "chatgpt-workspace-123",
218+
});
219+
220+
render(<AccountListItem account={account} selected={false} onSelect={vi.fn()} />);
221+
222+
expect(screen.getByText("Team | chatgpt-workspace-123")).toBeInTheDocument();
223+
expect(screen.queryByText(/Legacy Workspace/)).not.toBeInTheDocument();
224+
expect(screen.queryByText(/Personal \/ unknown workspace/)).not.toBeInTheDocument();
225+
});
210226
});

frontend/src/features/accounts/components/account-list-item.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export function AccountListItem({
4141
const emailSubtitle = account.displayName && account.displayName !== account.email
4242
? account.email
4343
: null;
44-
const workspaceLabel = account.workspaceLabel || account.workspaceId || "Personal / unknown workspace";
44+
const workspaceLabel = account.chatgptAccountId || account.workspaceLabel || account.workspaceId || "Personal / unknown workspace";
4545
const seatLabel = account.seatType ? ` | ${formatSlug(account.seatType)}` : "";
4646
const slotSubtitle = `${formatSlug(account.planType)} | ${workspaceLabel}${seatLabel}`;
4747
const idSuffix = showAccountId ? ` | ID ${formatCompactAccountId(account.accountId)}` : "";

0 commit comments

Comments
 (0)