Skip to content

Add isolated Codex and Claude accounts with safe session handoff - #1

Closed
slashdevcorpse wants to merge 1 commit into
mainfrom
feat/provider-account-profiles
Closed

slashdevcorpse wants to merge 1 commit into
mainfrom
feat/provider-account-profiles

Conversation

@slashdevcorpse

@slashdevcorpse slashdevcorpse commented Sep 6, 2026 •

Copy link
Copy Markdown
Owner

Related issue

Owner-directed change in this fork; no upstream issue or upstream maintainer agreement is claimed.

Problem / pressure

Codex and Claude sessions currently share the machine's default provider authentication. Users need independent additional accounts and a manual account handoff inside an existing Lody session without changing the workspace, worktree, transcript, or other running sessions.

Summary

  • Add machine-local Codex and Claude account profiles with native isolated authentication homes, status/login controls, process-lifetime use leases, and a backward-compatible System Default profile.
  • Persist each session's profile binding and implement an idle-only, checkpointed handoff with target validation, native resume when supported, transcript continuation otherwise, and rollback on failure.
  • Add capability-gated local/remote protocol support, settings controls, a session switcher, profile-scoped usage, and English/Chinese labels.
  • Fix safety defects exposed by Windows regression testing: malformed shutdown responses, upload symlink/replacement validation, SQLite recovery handles, hidden worktree metadata rewrites, and unfinished Git probes during non-Git scans. Make existing fixtures portable and isolate their temporary homes.

Before / after

Before After
Provider sessions use the default machine login. Existing sessions retain System Default behavior; additional profiles have isolated native homes.
Changing accounts requires an external global login change. A user can manually switch an idle session with a durable binding and failure recovery.
A failed native resume has no account-aware continuation path. Known resume rejection can continue from the existing Lody history; generic startup errors roll back.

Test plan

  • Workspace typechecks passed using pnpm -r --workspace-concurrency=1 run typecheck; this avoids the root script's Windows single-quote filter issue.
  • pnpm check:quick passed: type-aware lint, translations, Code Collab imports, platform boundaries, and public repository boundaries.
  • Shared suite: 984 passed, 3 skipped; RPC suite: 99 passed, 3 skipped. Electron: 43 passed; code-review-helper: 34 passed; site-docs: 3 passed.
  • Full components suite passed with vitest run --pool=forks --maxWorkers=2: 2,801 tests. The default worker run encountered a Windows native process crash; the fork-pool rerun completed without assertion failures.
  • Final full CLI suite: 243 files passed, 2 skipped; 2,433 tests passed, 13 skipped (vitest run --maxWorkers=2). The desktop build passed; the final CLI change was rebuilt, its published bundle imports checked, and the embedded CLI synchronized successfully.
  • Failure-path tests cover busy sessions/tools, missing/deleted profiles, login/use conflicts, exhausted targets, unsupported native resume, generic launch failures, unavailable models, failed commit persistence, rollback, restart/replay, partial output, malformed protocol responses, UI timeouts/retry, and stale profile lists.
  • Synthetic profile-home checks passed on Windows and WSL; desktop/mobile Chromium inspection passed. Real provider login and cross-account native resume were not exercised with live credentials.
  • GitHub Linux CI static checks passed, including full formatting, typechecks, lint, and repository boundaries. Locally, scoped formatting passed; the Windows full-format invocation reported unchanged dsh submodule formatting and the root script has the quoting issue described above.

Context handoff

Instructions for reviewing agents

  • Review focus: Review account-profiles.ts process isolation/leases, session-account-handoff.ts persistence and rollback, and the UI's durable binding behavior after RPC success.
  • Decisions to challenge: Known native resume rejection falls back to a history continuation in the same Lody session; other startup failures preserve the old binding.
  • Plausible failures / evidence gaps: Live provider authentication and cross-account native resume remain unverified; process-global leases coordinate one CLI host, not arbitrary external provider processes.

Authoring context

  • User goal / directives: Implement independent Codex/Claude accounts, preserve normal session behavior, test unhappy paths, repair unsafe failures, and open a PR directly.
  • Constraints / non-goals: Preserve System Default authentication behavior, workspace identity, transcript, worktrees, and concurrent sessions; no automatic account rotation or credential copying.
  • Risk-bearing decisions: Additional profiles use provider-native isolated homes; handoff validates first, checkpoints intent, stops the old process, and commits profile/native-session metadata together.
  • Destructive or irreversible behavior: A handoff stops the selected idle provider process; failed candidates are stopped and metadata is rolled back. No account migration, default logout, deployment, or merge is included.
  • Deliberately not done or tested: No live credential-based authentication or cross-account provider resume test; synthetic processes validate isolation and recovery contracts.
  • Unknowns / confidence: Local regression evidence is extensive, but native provider behavior and independent external processes remain integration boundaries.

Copilot AI lite review requested due to automatic review settings September 6, 2026 04:14
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: f3919308-a81d-49ab-937e-8d99e8391693


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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

21 issues found across 83 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/cli/src/session/AGENTS.md">

<violation number="1" location="apps/cli/src/session/AGENTS.md:199">
P3: The marker is cleared when the prompt produces visible output, not on success: a prompt that fails after emitting output clears it ("even if the request later fails"), and a successful but silent turn does not. Reword to describe output-based consumption so future agents don't reintroduce the bounded replay.</violation>
</file>

<file name="locales/zh_CN.json">

<violation number="1" location="locales/zh_CN.json:7">
P3: “跟随您通常使用的 CLI 登录”是英文 “follows your normal CLI login” 的直译,中文读起来不自然(“跟随…登录”搭配别扭)。建议改为更自然的表达,如“系统默认账户会沿用您常用的 CLI 登录。”</violation>
</file>

<file name="apps/cli/src/agent/claude-env-conflict.ts">

<violation number="1" location="apps/cli/src/agent/claude-env-conflict.ts:50">
P2: When the host exports `AWS_BEARER_TOKEN_BEDROCK`, a managed Claude profile still inherits that Bedrock credential because this matcher never removes it. Scrub `AWS_BEARER_TOKEN_BEDROCK` as well so the managed subscription cannot retain another provider's authentication material.</violation>
</file>

<file name="apps/cli/src/session/session-fork-service.ts">

<violation number="1" location="apps/cli/src/session/session-fork-service.ts:694">
P1: When a managed-profile worktree fork crashes before its final metadata write, recovery drops the source account binding and the next prompt runs under System Default. Add the account profile to the durable fork marker and restore it in `republishedMeta` so recovery preserves the isolated account.</violation>
</file>

<file name="packages/shared/src/schema.ts">

<violation number="1" location="packages/shared/src/schema.ts:799">
P2: Repeated account switches append every receipt to durable session metadata without a retention bound. Cap or compact the idempotency receipts, otherwise long-lived sessions eventually incur growing sync/storage and lookup costs and may fail metadata writes.</violation>
</file>

<file name="packages/components/src/components/settings/agent-config-dialog.tsx">

<violation number="1" location="packages/components/src/components/settings/agent-config-dialog.tsx:2143">
P2: After a successful System Default sign-in, the account row keeps its pre-login status because this node's callback never refreshes `AccountProfilesPanel`'s profile snapshot. Pass an authentication-completed callback through the panel/list and refresh the profiles after the System Default login succeeds.</violation>
</file>

<file name="packages/shared/src/node/local-session-control.ts">

<violation number="1" location="packages/shared/src/node/local-session-control.ts:498">
P2: When an account profile uses the nil or max UUID, this new check rejects the request in the TypeScript validator while the CJS validator accepts it, so the two local transports disagree and valid account messages are dropped on the Electron path. Use the same account-ID predicate in both validators, or widen `AccountProfileIdSchema` to include the IDs that the protocol accepts.</violation>
</file>

<file name="apps/cli/src/agent/title-generator.ts">

<violation number="1" location="apps/cli/src/agent/title-generator.ts:310">
P2: For a managed Codex account profile, passing accountProfileId here makes resolveAccountProfileEnv set CODEX_HOME to the durable profile home, which flips shouldUseWorkdirCodexHome in acp-runner.ts to false. The title agent then writes its rollout/history into the shared profile home instead of the temporary repo-local home its isolation design guarantees, and can run a second codex process against that home while the session's own codex is active. Confirm this is intended for managed profiles, or preserve isolation by running the title prompt against a temp copy of the profile home.</violation>
</file>

<file name="apps/cli/src/agent/account-profiles.ts">

<violation number="1" location="apps/cli/src/agent/account-profiles.ts:175">
P2: If profile initialization fails after the directory is created, the deterministic operation ID becomes permanently poisoned because retries treat the partial directory as an existing profile. Remove the newly created directory on initialization failure or complete creation transactionally.</violation>

<violation number="2" location="apps/cli/src/agent/account-profiles.ts:230">
P1: When switching a Claude session back to System Default with an API-key or provider-auth environment, validation rejects the target because its status is intentionally `unknown`. Skip native status validation for System Default so its existing environment remains usable.</violation>
</file>

<file name="apps/cli/src/lib/code-collab/code-collab-v2-service.test.ts">

<violation number="1" location="apps/cli/src/lib/code-collab/code-collab-v2-service.test.ts:265">
P2: If `saveText` rejects before the file-index publish starts, the finally's `await publishFinished` never resolves because `publishFileIndexSignal` is only called after the publish completes, so the test hangs until the vitest timeout and masks the actual failure. Guard the wait (e.g., race `publishFinished` with a short timeout, or only await it when the publish actually started) so a broken save fails the test with the real error instead of a hang.</violation>
</file>

<file name="apps/cli/src/session/session-manager.ts">

<violation number="1" location="apps/cli/src/session/session-manager.ts:656">
P2: When a new session is created while this account is beginning authentication, the reservation is installed only after two awaits. The sign-in can therefore win, causing `createSession` to fail after publishing an initializing session document and leaving an orphaned durable session; reserve the account before creating durable metadata or roll back the created session on this path.</violation>
</file>

<file name="packages/components/tests/create-workspace-runtime-meta-recovery.test.ts">

<violation number="1" location="packages/components/tests/create-workspace-runtime-meta-recovery.test.ts:577">
P3: The new narrowing test never supplies configId or label, yet asserts they arrive as undefined — which vitest's deep equality treats as identical to the keys being absent. As written, a regression that stops forwarding configId or label across the remote boundary would still pass. Pass real values (e.g. configId: 'cfg-1', label: 'work') for at least one of the two requests so the forwarding is actually exercised.</violation>
</file>

<file name="apps/cli/tests/account-profiles-platform-check.ts">

<violation number="1" location="apps/cli/tests/account-profiles-platform-check.ts:1">
P2: This platform check is never executed: vitest's include patterns only match `tests/**/*.test.ts` and `src/**/*.{test,spec}.ts`, and nothing in package.json scripts or CI references `account-profiles-platform-check`. The PR relies on it for Windows/WSL profile-home coverage, but it only runs if a developer manually invokes `npx tsx tests/account-profiles-platform-check.ts`, which nothing documents. Wire it in (rename to `*.test.ts`, add an npm script, or run it in the Windows CI step) so the isolation checks actually run.</violation>
</file>

<file name="apps/cli/tests/login-shell-env.test.ts">

<violation number="1" location="apps/cli/tests/login-shell-env.test.ts:34">
P2: `vi.spyOn(process, 'platform', 'get')` throws because `process.platform` is a plain data property in Node 22, not an accessor. In vitest 3.2.4, spying with accessType 'get' requires an own getter, so this line fails in beforeEach and errors every test in the describe block (including the pre-existing slow-probe test). Override the property with `Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true })` and restore it in afterEach (vi.restoreAllMocks will not undo defineProperty).</violation>
</file>

<file name="packages/components/src/components/settings/account-profiles.tsx">

<violation number="1" location="packages/components/src/components/settings/account-profiles.tsx:56">
P2: After a successful `requestSessionAccountSwitch`, `change` never calls `refresh()` nor updates the select locally. The `<select>` value comes from the `accountProfileId` prop, so the UI keeps showing the previous binding/status until unrelated re-renders or session-doc propagation catch up. Call `refresh()` after success (and re-evaluate `currentId` from the response) so the newly bound profile's label/status is reflected immediately.</violation>
</file>

<file name="packages/components/tests/account-profiles.test.tsx">

<violation number="1" location="packages/components/tests/account-profiles.test.tsx:160">
P3: This test's title claims the UI waits for durable metadata after a successful switch, but it only verifies the select stays unchanged before any parent update. It never simulates the durable binding arriving (re-rendering with the new accountProfileId) or asserts account-b becomes selected, and the returned accountProfileId:'account-b' is never asserted. Since the PR lists durable binding after RPC success as a focus area, add a step that re-renders with accountProfileId='account-b' and asserts the select reflects it, so the full handoff path is covered.</violation>
</file>

<file name="apps/cli/src/session/session-account-handoff.ts">

<violation number="1" location="apps/cli/src/session/session-account-handoff.ts:142">
P2: If the rollback `deps.stop()` or `deps.checkpoint(...)` throws, the original switch error is masked and the durable `accountHandoff` intent is left un-rolled-back. Persist the rollback failure as a log/side effect instead of letting it replace the original error, and ensure a failed rollback does not leave the committed meta in the in-flight intent state.</violation>
</file>

<file name="packages/shared/src/message.ts">

<violation number="1" location="packages/shared/src/message.ts:379">
P3: `SessionAccountSwitchRequest.accountProfileId` is typed as plain `string`, but the matching `SessionAccountSwitchRequestSchema` validates it with `AccountProfileIdSchema` (`z.union([z.literal('system-default'), z.uuid()])`). Because `LoroStreamsRpcRequestSchema` is `.strict()` and `LoroMachineRpcRequestSchema.safeParse` failure is handled by silently logging and returning without a response, any non-conforming id (e.g. a stale/deleted profile id) causes the request to be dropped and the client to hang until the 300s `requestSessionAccountSwitch` timeout instead of receiving an error. The same widening applies to `MachineAcpAuthenticateRequest.accountProfileId?: string`. Type the interface to the schema union (or relax the schema) so invalid values fail fast on the caller side.</violation>
</file>

<file name="packages/loro-streams-rpc/src/rpc.ts">

<violation number="1" location="packages/loro-streams-rpc/src/rpc.ts:1559">
P2: The new error responses for `machine/account-profiles` and `session/account-switch` in `toLegacyRpcErrorResponse` set `error: error.message`, dropping the `${error.code}: ` prefix that every other method in this function uses. The file's own contract comment states clients pattern-match the error `code` to render localized messages and decide retry behavior, so these two methods lose that signal (e.g. `method_unavailable` vs `internal_error` are indistinguishable and never surfaced).</violation>
</file>

<file name="apps/cli/src/lib/code-collab/file-index-scan-core.ts">

<violation number="1" location="apps/cli/src/lib/code-collab/file-index-scan-core.ts:80">
P3: `deletedResult` can never be rejected because `runGit` catches all errors and resolves `{ ok: false }` instead. The `|| deletedResult.status === 'rejected'` half of this guard is dead code and implies the deleted subprocess can reject, which it cannot.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

cliType: source.cliType,
agentType: source.agentType,
agentConfigId: source.agentConfigId,
accountProfileId: source.accountProfileId ?? 'system-default',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When a managed-profile worktree fork crashes before its final metadata write, recovery drops the source account binding and the next prompt runs under System Default. Add the account profile to the durable fork marker and restore it in republishedMeta so recovery preserves the isolated account.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/cli/src/session/session-fork-service.ts, line 694:

<comment>When a managed-profile worktree fork crashes before its final metadata write, recovery drops the source account binding and the next prompt runs under System Default. Add the account profile to the durable fork marker and restore it in `republishedMeta` so recovery preserves the isolated account.</comment>

<file context>
@@ -691,6 +691,7 @@ export class SessionForkService {
         cliType: source.cliType,
         agentType: source.agentType,
         agentConfigId: source.agentConfigId,
+        accountProfileId: source.accountProfileId ?? 'system-default',
         project: targetProject,
         repoFullName: targetRepoFullName,
</file context>

input: AccountProfileInput & ProbeOptions
): Promise<void> {
const { probeBuiltinAuthentication } = await import('./acp-authentication');
const result = await probeBuiltinAuthentication({ ...input, accountStatusOnly: true });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When switching a Claude session back to System Default with an API-key or provider-auth environment, validation rejects the target because its status is intentionally unknown. Skip native status validation for System Default so its existing environment remains usable.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/cli/src/agent/account-profiles.ts, line 230:

<comment>When switching a Claude session back to System Default with an API-key or provider-auth environment, validation rejects the target because its status is intentionally `unknown`. Skip native status validation for System Default so its existing environment remains usable.</comment>

<file context>
@@ -0,0 +1,284 @@
+  input: AccountProfileInput & ProbeOptions
+): Promise<void> {
+  const { probeBuiltinAuthentication } = await import('./acp-authentication');
+  const result = await probeBuiltinAuthentication({ ...input, accountStatusOnly: true });
+  if (result.status !== 'authenticated')
+    throw new Error('Target account authentication could not be verified. Sign in and retry.');
</file context>

Comment on lines +50 to +51
/^(ANTHROPIC_|CLAUDE_CODE_(USE_|SKIP_))/.test(key.toUpperCase()) ||
key.toUpperCase() === 'CLAUDE_CODE_OAUTH_TOKEN'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When the host exports AWS_BEARER_TOKEN_BEDROCK, a managed Claude profile still inherits that Bedrock credential because this matcher never removes it. Scrub AWS_BEARER_TOKEN_BEDROCK as well so the managed subscription cannot retain another provider's authentication material.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/cli/src/agent/claude-env-conflict.ts, line 50:

<comment>When the host exports `AWS_BEARER_TOKEN_BEDROCK`, a managed Claude profile still inherits that Bedrock credential because this matcher never removes it. Scrub `AWS_BEARER_TOKEN_BEDROCK` as well so the managed subscription cannot retain another provider's authentication material.</comment>

<file context>
@@ -42,6 +42,20 @@ const CLAUDE_AUTH_ROUTING_KEYS = [
+  const result = { ...env };
+  for (const key of Object.keys(result)) {
+    if (
+      /^(ANTHROPIC_|CLAUDE_CODE_(USE_|SKIP_))/.test(key.toUpperCase()) ||
+      key.toUpperCase() === 'CLAUDE_CODE_OAUTH_TOKEN'
+    ) {
</file context>
Suggested change
/^(ANTHROPIC_|CLAUDE_CODE_(USE_|SKIP_))/.test(key.toUpperCase()) ||
key.toUpperCase() === 'CLAUDE_CODE_OAUTH_TOKEN'
/^(ANTHROPIC_|CLAUDE_CODE_(USE_|SKIP_))/.test(key.toUpperCase()) ||
key.toUpperCase() === 'CLAUDE_CODE_OAUTH_TOKEN' ||
key.toUpperCase() === 'AWS_BEARER_TOKEN_BEDROCK'

sourceAcpSessionId?: ACPSessionId;
targetAccountProfileId: string;
} | null;
accountTransitions?: Array<{

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Repeated account switches append every receipt to durable session metadata without a retention bound. Cap or compact the idempotency receipts, otherwise long-lived sessions eventually incur growing sync/storage and lookup costs and may fail metadata writes.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/shared/src/schema.ts, line 799:

<comment>Repeated account switches append every receipt to durable session metadata without a retention bound. Cap or compact the idempotency receipts, otherwise long-lived sessions eventually incur growing sync/storage and lookup costs and may fail metadata writes.</comment>

<file context>
@@ -787,6 +787,28 @@ export type SessionMeta = {
+    sourceAcpSessionId?: ACPSessionId;
+    targetAccountProfileId: string;
+  } | null;
+  accountTransitions?: Array<{
+    requestId: string;
+    fromAccountProfileId: string;
</file context>

machineId={machine.id}
agentType={formData.agentType}
configId={agentConfigId}
systemDefaultAuthentication={systemDefaultAuthentication}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: After a successful System Default sign-in, the account row keeps its pre-login status because this node's callback never refreshes AccountProfilesPanel's profile snapshot. Pass an authentication-completed callback through the panel/list and refresh the profiles after the System Default login succeeds.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/components/src/components/settings/agent-config-dialog.tsx, line 2143:

<comment>After a successful System Default sign-in, the account row keeps its pre-login status because this node's callback never refreshes `AccountProfilesPanel`'s profile snapshot. Pass an authentication-completed callback through the panel/list and refresh the profiles after the System Default login succeeds.</comment>

<file context>
@@ -2107,24 +2130,23 @@ export function AgentConfigDialog(props: AgentConfigDialogProps) {
+                    machineId={machine.id}
+                    agentType={formData.agentType}
+                    configId={agentConfigId}
+                    systemDefaultAuthentication={systemDefaultAuthentication}
+                    runtimeOverrides={formData.runtimeOverrides}
+                    env={formData.env}
</file context>

Comment thread locales/zh_CN.json
"agents.accounts.additional": "账户 {{number}}",
"agents.accounts.authenticated": "已登录",
"agents.accounts.createFailed": "无法添加账户",
"agents.accounts.defaultHint": "系统默认账户跟随您通常使用的 CLI 登录。",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: “跟随您通常使用的 CLI 登录”是英文 “follows your normal CLI login” 的直译,中文读起来不自然(“跟随…登录”搭配别扭)。建议改为更自然的表达,如“系统默认账户会沿用您常用的 CLI 登录。”

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At locales/zh_CN.json, line 7:

<comment>“跟随您通常使用的 CLI 登录”是英文 “follows your normal CLI login” 的直译,中文读起来不自然(“跟随…登录”搭配别扭)。建议改为更自然的表达,如“系统默认账户会沿用您常用的 CLI 登录。”</comment>

<file context>
@@ -1,4 +1,18 @@
+  "agents.accounts.additional": "账户 {{number}}",
+  "agents.accounts.authenticated": "已登录",
+  "agents.accounts.createFailed": "无法添加账户",
+  "agents.accounts.defaultHint": "系统默认账户跟随您通常使用的 CLI 登录。",
+  "agents.accounts.signIn": "登录",
+  "agents.accounts.statusUnavailable": "无法获取账户状态",
</file context>

sessionId: 'session-1' as SessionId,
accountProfileId: 'system-default',
});
expect(mocks.accountProfiles).toHaveBeenCalledWith({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The new narrowing test never supplies configId or label, yet asserts they arrive as undefined — which vitest's deep equality treats as identical to the keys being absent. As written, a regression that stops forwarding configId or label across the remote boundary would still pass. Pass real values (e.g. configId: 'cfg-1', label: 'work') for at least one of the two requests so the forwarding is actually exercised.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/components/tests/create-workspace-runtime-meta-recovery.test.ts, line 577:

<comment>The new narrowing test never supplies configId or label, yet asserts they arrive as undefined — which vitest's deep equality treats as identical to the keys being absent. As written, a regression that stops forwarding configId or label across the remote boundary would still pass. Pass real values (e.g. configId: 'cfg-1', label: 'work') for at least one of the two requests so the forwarding is actually exercised.</comment>

<file context>
@@ -537,6 +545,50 @@ describe('createWorkspaceRuntime meta recovery lifecycle', () => {
+      sessionId: 'session-1' as SessionId,
+      accountProfileId: 'system-default',
+    });
+    expect(mocks.accountProfiles).toHaveBeenCalledWith({
+      requestId: 'request-1',
+      configId: undefined,
</file context>


it('waits for durable metadata even when the switch response succeeds', async () => {
mocks.list.mockResolvedValue({ success: true, profiles });
mocks.switch.mockResolvedValue({ success: true, accountProfileId: 'account-b' });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This test's title claims the UI waits for durable metadata after a successful switch, but it only verifies the select stays unchanged before any parent update. It never simulates the durable binding arriving (re-rendering with the new accountProfileId) or asserts account-b becomes selected, and the returned accountProfileId:'account-b' is never asserted. Since the PR lists durable binding after RPC success as a focus area, add a step that re-renders with accountProfileId='account-b' and asserts the select reflects it, so the full handoff path is covered.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/components/tests/account-profiles.test.tsx, line 160:

<comment>This test's title claims the UI waits for durable metadata after a successful switch, but it only verifies the select stays unchanged before any parent update. It never simulates the durable binding arriving (re-rendering with the new accountProfileId) or asserts account-b becomes selected, and the returned accountProfileId:'account-b' is never asserted. Since the PR lists durable binding after RPC success as a focus area, add a step that re-renders with accountProfileId='account-b' and asserts the select reflects it, so the full handoff path is covered.</comment>

<file context>
@@ -0,0 +1,233 @@
+
+  it('waits for durable metadata even when the switch response succeeds', async () => {
+    mocks.list.mockResolvedValue({ success: true, profiles });
+    mocks.switch.mockResolvedValue({ success: true, accountProfileId: 'account-b' });
+    const element = await render(<SessionAccountSelector {...target} />);
+    const select = element.querySelector('select')!;
</file context>

workspaceId: WorkspaceId;
requestId: string;
sessionId: SessionId;
accountProfileId: string;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: SessionAccountSwitchRequest.accountProfileId is typed as plain string, but the matching SessionAccountSwitchRequestSchema validates it with AccountProfileIdSchema (z.union([z.literal('system-default'), z.uuid()])). Because LoroStreamsRpcRequestSchema is .strict() and LoroMachineRpcRequestSchema.safeParse failure is handled by silently logging and returning without a response, any non-conforming id (e.g. a stale/deleted profile id) causes the request to be dropped and the client to hang until the 300s requestSessionAccountSwitch timeout instead of receiving an error. The same widening applies to MachineAcpAuthenticateRequest.accountProfileId?: string. Type the interface to the schema union (or relax the schema) so invalid values fail fast on the caller side.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/shared/src/message.ts, line 379:

<comment>`SessionAccountSwitchRequest.accountProfileId` is typed as plain `string`, but the matching `SessionAccountSwitchRequestSchema` validates it with `AccountProfileIdSchema` (`z.union([z.literal('system-default'), z.uuid()])`). Because `LoroStreamsRpcRequestSchema` is `.strict()` and `LoroMachineRpcRequestSchema.safeParse` failure is handled by silently logging and returning without a response, any non-conforming id (e.g. a stale/deleted profile id) causes the request to be dropped and the client to hang until the 300s `requestSessionAccountSwitch` timeout instead of receiving an error. The same widening applies to `MachineAcpAuthenticateRequest.accountProfileId?: string`. Type the interface to the schema union (or relax the schema) so invalid values fail fast on the caller side.</comment>

<file context>
@@ -348,7 +349,49 @@ export interface MachineAcpAuthMethodSummary {
+  workspaceId: WorkspaceId;
+  requestId: string;
+  sessionId: SessionId;
+  accountProfileId: string;
+}
+
</file context>

),
runGit(cwd, ['ls-files', '--deleted', '-z', '--', '.']),
]);
if (listingResult.status === 'rejected' || deletedResult.status === 'rejected') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: deletedResult can never be rejected because runGit catches all errors and resolves { ok: false } instead. The || deletedResult.status === 'rejected' half of this guard is dead code and implies the deleted subprocess can reject, which it cannot.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/cli/src/lib/code-collab/file-index-scan-core.ts, line 80:

<comment>`deletedResult` can never be rejected because `runGit` catches all errors and resolves `{ ok: false }` instead. The `|| deletedResult.status === 'rejected'` half of this guard is dead code and implies the deleted subprocess can reject, which it cannot.</comment>

<file context>
@@ -75,6 +77,11 @@ async function runGitLsFiles(
       ),
       runGit(cwd, ['ls-files', '--deleted', '-z', '--', '.']),
     ]);
+    if (listingResult.status === 'rejected' || deletedResult.status === 'rejected') {
+      return { ok: false };
+    }
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants