feat: support multiple workspaces for the same email (#491) #493
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| import { | ||
| formatAccountLabel, | ||
| formatWorkspaceLines, | ||
| } from "../../accounts.js"; | ||
| import type { AccountStorageV3 } from "../../storage.js"; | ||
| import { saveAccountsWithRetry } from "../forecast-report-shared.js"; | ||
|
|
||
| type LoadedStorage = AccountStorageV3 | null; | ||
|
|
||
| export interface WorkspaceCommandDeps { | ||
| setStoragePath: (path: string | null) => void; | ||
| loadAccounts: () => Promise<LoadedStorage>; | ||
| saveAccounts: (storage: AccountStorageV3) => Promise<void>; | ||
| logError?: (message: string) => void; | ||
| logInfo?: (message: string) => void; | ||
| } | ||
|
|
||
| /** | ||
| * `codex-multi-auth workspace <account> [workspace]` | ||
| * | ||
| * With only an account index, lists the workspaces that account can rotate | ||
| * between (personal Plus vs business/team under one email, issue #491). With a | ||
| * workspace index too, sets it as the active workspace for that account. | ||
| */ | ||
| export async function runWorkspaceCommand( | ||
| args: string[], | ||
| deps: WorkspaceCommandDeps, | ||
| ): Promise<number> { | ||
| deps.setStoragePath(null); | ||
| const logError = deps.logError ?? console.error; | ||
| const logInfo = deps.logInfo ?? console.log; | ||
|
|
||
| const storage = await deps.loadAccounts(); | ||
| if (!storage || storage.accounts.length === 0) { | ||
| logError("No accounts configured."); | ||
| return 1; | ||
| } | ||
|
|
||
| const accountArg = args[0]; | ||
| if (!accountArg) { | ||
| logError( | ||
| "Missing account index. Usage: codex-multi-auth workspace <account> [workspace]", | ||
| ); | ||
| return 1; | ||
| } | ||
|
|
||
| const parsedAccount = Number.parseInt(accountArg, 10); | ||
| if (!Number.isFinite(parsedAccount) || parsedAccount < 1) { | ||
| logError(`Invalid account index: ${accountArg}`); | ||
| return 1; | ||
| } | ||
|
|
||
| const accountIndex = parsedAccount - 1; | ||
| if (accountIndex >= storage.accounts.length) { | ||
| logError( | ||
| `Account index out of range. Valid range: 1-${storage.accounts.length}`, | ||
| ); | ||
| return 1; | ||
| } | ||
|
|
||
| const account = storage.accounts[accountIndex]; | ||
| if (!account) { | ||
| logError(`Account ${parsedAccount} not found.`); | ||
| return 1; | ||
| } | ||
|
|
||
| const workspaces = account.workspaces ?? []; | ||
| if (workspaces.length === 0) { | ||
| logInfo( | ||
| `Account ${parsedAccount} (${formatAccountLabel(account, accountIndex)}) has no tracked workspaces.`, | ||
| ); | ||
| return 0; | ||
| } | ||
|
|
||
| const workspaceArg = args[1]; | ||
| if (!workspaceArg) { | ||
| logInfo(`Account ${parsedAccount}: ${formatAccountLabel(account, accountIndex)}`); | ||
| for (const line of formatWorkspaceLines(account, " ")) { | ||
| logInfo(line); | ||
| } | ||
| logInfo(""); | ||
| logInfo( | ||
| `Switch with: codex-multi-auth workspace ${parsedAccount} <workspace-number>`, | ||
| ); | ||
| return 0; | ||
| } | ||
|
|
||
| const parsedWorkspace = Number.parseInt(workspaceArg, 10); | ||
| if ( | ||
| !Number.isFinite(parsedWorkspace) || | ||
| parsedWorkspace < 1 || | ||
| parsedWorkspace > workspaces.length | ||
| ) { | ||
| logError( | ||
| `Invalid workspace index. Valid range: 1-${workspaces.length}`, | ||
| ); | ||
| return 1; | ||
| } | ||
|
|
||
| const workspaceIndex = parsedWorkspace - 1; | ||
| const target = workspaces[workspaceIndex]; | ||
| if (!target) { | ||
| logError(`Workspace ${parsedWorkspace} not found.`); | ||
| return 1; | ||
| } | ||
|
|
||
| const targetName = target.name?.trim() || "(unnamed)"; | ||
| if (target.enabled === false) { | ||
| logError( | ||
| `Workspace ${parsedWorkspace} ([${targetName}]) is disabled and cannot be selected.`, | ||
| ); | ||
| return 1; | ||
| } | ||
|
|
||
| if (account.currentWorkspaceIndex === workspaceIndex) { | ||
| logInfo( | ||
| `Account ${parsedAccount} is already using workspace ${parsedWorkspace}: [${targetName}].`, | ||
| ); | ||
| return 0; | ||
| } | ||
|
|
||
| account.currentWorkspaceIndex = workspaceIndex; | ||
| await saveAccountsWithRetry(storage, deps.saveAccounts); | ||
|
Comment on lines
+122
to
+123
There was a problem hiding this comment.
this follows the same load-mutate-save pattern as Prompt To Fix With AIThis is a comment left during a code review.
Path: lib/codex-manager/commands/workspace.ts
Line: 122-123
Comment:
**concurrent write race (TOCTOU)**
this follows the same load-mutate-save pattern as `switch`/`unpin`, but worth calling out explicitly: the runtime-rotation-proxy writes to the same accounts file (e.g. `disableCurrentWorkspace`, `lastUsed` updates). if the proxy writes between `loadAccounts()` and `saveAccountsWithRetry()` here, this save silently overwrites those changes — most critically, a workspace `enabled: false` written by the proxy could be restored to `true`. no file-level lock or re-read-before-write guards against this. low probability in practice, but concurrency tests for this path are absent.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| // Guard a possibly-undefined id the same way formatWorkspaceLines does, in | ||
| // case on-disk data does not conform to the Workspace interface. | ||
| const id = target.id?.trim() ?? ""; | ||
| const idSuffix = id.length > 6 ? id.slice(-6) : id; | ||
| logInfo( | ||
| `Account ${parsedAccount} now using workspace ${parsedWorkspace}: [${targetName}]${idSuffix ? ` (id:${idSuffix})` : ""}.`, | ||
| ); | ||
| return 0; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.