Skip to content

Keep Codex users distinct in a shared Team workspace - #593

Open
gleaming9 wants to merge 1 commit into
nesszer:mainfrom
gleaming9:fix/shared-workspace-account-identity
Open

gleaming9 wants to merge 1 commit into
nesszer:mainfrom
gleaming9:fix/shared-workspace-account-identity

Conversation

@gleaming9

@gleaming9 gleaming9 commented Sep 22, 2026 •

Copy link
Copy Markdown

Summary

Fix saved-account reconciliation for different OpenAI users that belong to the same ChatGPT Team workspace.

Account matching previously accepted an equal effective workspace ID before rejecting conflicting user identity. This could collapse two independently authenticated users into one saved account even though they have separate usage quotas.

This change:

  • Rejects a match when both accounts have different authenticated subjects.
  • Uses different email hints as fallback evidence when subjects cannot both be compared.
  • Applies the same conflict rule to removed-account matching, preventing one user's managed home from being claimed or removed for another user.
  • Preserves the existing workspace, subject, email, and managed-home matching behavior when user evidence does not conflict.

No persisted account schema or migration is required.

Related issue

Fixes #592

Affected areas

Check every area this PR changes or could affect:

  • Tray panel
  • Settings UI
  • Config file / settings persistence
  • CLI
  • Provider-specific behavior
  • Installer / release packaging
  • Startup / background behavior
  • Documentation
  • Other:

The frontend is unchanged. The checked UI surfaces consume the corrected shared Rust account list.

Validation

Hosted PR check runs on Blacksmith Windows when CI_BUDGET_MODE is not off (see .github/workflows/pr-check.yml and CONTEXT.md). Still run the local slice and list commands/results below. If a check is not relevant, say why.

  • powershell.exe -ExecutionPolicy Bypass -NoProfile -File scripts\local-check.ps1 — passed.
  • Full pre-release validation is not applicable; release and packaging behavior are unchanged.
  • Installer/release validation is not applicable.
  • Thermo-nuclear code quality review completed before submitting: https://github.com/cursor/plugins/blob/main/cursor-team-kit/skills/thermo-nuclear-code-quality-review/SKILL.md
  • Other:
    • Shared Rust tests — 2,104 passed, 1 ignored (existing opt-in Claude CLI smoke test), 0 failed.
    • Tauri Rust tests — 453 passed, 0 failed.
    • Frontend tests — 393 passed, 0 failed.
    • Locale validation — 879 keys matched.
    • TypeScript and Vite production build — passed.
    • Rust formatting and Clippy gates — passed.
    • Focused account tests — 83 passed, 0 failed.
    • git diff --check — passed.

UI / tray proof

For UI, tray, settings, or visual behavior changes, use CUA Driver for visual proof. If CUA Driver cannot be used, explain why and attach equivalent manual proof.

  • Not applicable — no frontend, layout, tray interaction, or visual implementation changed.
  • CUA Driver visual proof attached
  • CUA Driver could not be used; equivalent manual proof and explanation attached

Manual runtime verification confirmed that two independently authenticated users in the same Team workspace remain visible as separate saved accounts.

Notes for reviewers

The identity precedence is intentionally narrow:

  1. Different known subjects mean different users.
  2. When subjects cannot both be compared, different known email hints mean different users.
  3. Only when user evidence does not conflict can the existing workspace and fallback identity rules identify the same account.

Regression coverage verifies subject conflicts, email fallback, email changes for a stable subject, different workspaces, discovery, removal, and managed-home materialization.

CLI --all-accounts enumeration is unchanged and outside this PR.

Summary by CodeRabbit

  • Bug Fixes
    • Improved account identity matching to prevent users with conflicting identity details from being incorrectly linked through shared workspace or home-path information.
    • Ensured users sharing a provider account ID but having different identities maintain separate discovery, removal, and managed-home records.
    • Added coverage for identity conflicts and shared-account scenarios to improve reliability.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: c5b54ef7-d1e4-4703-ab89-6975e95cebdd

📥 Commits

Reviewing files that changed from the base of the PR and between 8cdd2ca and fc54a8c.

📒 Files selected for processing (2)
  • rust/src/codex_accounts/account_manager/tests.rs
  • rust/src/codex_accounts/models.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The change prevents conflicting user identities from matching through shared workspace or home paths. It also updates account-manager test setup to use explicit subjects and verifies separate discovery, removal, and materialization for shared provider identifiers.

Changes

Team user identity separation

Layer / File(s) Summary
Identity conflict matching
rust/src/codex_accounts/models.rs
CodexAccount::matches and RemovedAccountIdentity::matches reject conflicting normalized subjects or emails before fallback matching. Tests cover conflicting and matching identity cases.
Account manager validation
rust/src/codex_accounts/account_manager/tests.rs
write_auth delegates to write_user_auth with an explicit subject. The integration test verifies separate discovery, selective removal, and materialization for users sharing a provider account identifier.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: finesssee

Merge Risk: ⚪ Minimal · up to fc54a

Account reconciliation now keeps distinct users in the same workspace separate, including their saved accounts and managed homes. The supplied validation passes, so the change is mergeable.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #592 requires separate saved accounts for users who share a Team workspace but have different identities. user_identity_conflicts rejects conflicting normalized subjects and rejects conflictin…
Out of Scope Changes check ✅ Passed The changes are limited to account identity matching and focused automated tests. The matching changes directly implement issue #592 for shared workspaces, removed-account matching, and managed-home o…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preserving distinct Codex users within a shared Team workspace.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

@Finesssee

Copy link
Copy Markdown
Collaborator

Independent thermo source review: PASS at fc54a8c81a5191959d85656c10a01598220c90dd.

The identity-conflict guard runs before workspace-ID and removed-home fallback matching. Conflicting subjects remain separate; when both subjects cannot be compared, conflicting email hints also remain separate. Temporary-home regression coverage checks discovery, selective removal, materialization, and preservation of the other user's authentication file. No blocking structural or identity-isolation issue was found in the two-file Rust diff.

There are no frontend or tray-render implementation changes. The existing local validation is author-reported, and this head has no hosted Windows gate. Its exact commit is being included in #610 for full validation against current main before landing.

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.

[Bug]: Different Codex users in the same Team workspace collapse into one saved account

2 participants