feat(claude-sdk-oauth): add account manager (LAB-28) - #644
Open
eddieparc wants to merge 4 commits into
Open
Conversation
The goal continuation scheduler waits two very different amounts of time and only narrates one of them. `#schedule()` in monitor-continuation.ts emits ui.notify plus a durable goal-cache-warmup entry for the 240s monitor wait, while the 60s user-grace wait (GOAL_USER_GRACE_DELAY_MS, continuation.ts:11) emits nothing at all. A session waiting out the grace window is therefore indistinguishable from a hung one for a full minute. Add wait-progress.ts as the render half of a fix: renderGoalWaitBar draws a clamped 12-cell bar and formatGoalWaitLabel composes it with the remaining time, reusing formatWakeDuration so a countdown reads like the existing cache-warm notices. userGrace reads "goal resumes in 47s"; monitor reads "goal continues in 3m 12s - 2 monitors on duty". Nothing imports the module yet. The scheduler and footer wiring is proposed, not implemented, so this commit cannot change runtime behavior.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found and verified against the latest diff
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="packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/account-command.ts">
<violation number="1" location="packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/account-command.ts:168">
P3: `/claude-account add` can now resolve through the account manager without adding an account (remove, logout-all, pin, unpin, unblock), yet it always notifies 'Claude SDK OAuth account added.' and emits an accounts-changed event. When the manager returns a managed credential rather than a fresh OAuth login, the success message (and the emit) describe an add that never happened. Consider distinguishing the 'new account persisted' case from manager resolutions before showing the success notification, for example by having the login report which branch it took.</violation>
<violation number="2" location="packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/account-command.ts:171">
P2: Account-management selections are ambiguous when a stored account uses a synthetic environment name such as `env`: choosing the environment row maps to the first same-label option. Reserving environment names or making the displayed labels/IDs source-aware would prevent pinning or unblocking the wrong account.</violation>
</file>
<file name="packages/coding-agent/src/core/extensions/builtin/goal/wait-progress.ts">
<violation number="1" location="packages/coding-agent/src/core/extensions/builtin/goal/wait-progress.ts:32">
P3: The wait label duplicates the existing `cache-warm.ts` monitor-count formatter, so future wording changes can make these user-facing notices inconsistent. Reusing a shared/exported helper would keep the monitor status text in sync.</violation>
<violation number="2" location="packages/coding-agent/src/core/extensions/builtin/goal/wait-progress.ts:36">
P3: This new `wait-progress.ts` module is currently unreferenced by any production code — nothing calls `renderGoalWaitBar`, `formatGoalWaitLabel`, or reads `GOAL_WAIT_BAR_CELLS` outside its own test file (`goal-wait-progress.test.ts`). The change is an isolated render layer with the wiring left for a future maintainer decision, which is documented in `changes.md`, so it is not an accidental leftover. However, shipping a fully unused module that could be abandoned (the doc even asks whether the countdown should be text-only) means the exported surface has no production consumer yet. Consider keeping this behind the follow-up PR that wires it into the footer so the API doesn't land and potentially churn before it is actually used.</violation>
</file>
<file name="packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/oauth-login.ts">
<violation number="1" location="packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/oauth-login.ts:120">
P2: When only environment-backed Claude accounts exist, choosing “Log out of one account” opens an empty account picker with no way to complete the action. Hide this action unless at least one stored account exists, or handle the empty stored-account case before opening the picker.</violation>
<violation number="2" location="packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/oauth-login.ts:133">
P2: Account mutations made through the new provider-login account manager don't notify account-change listeners. The same pin/remove operations via `/claude-account` (account-management.ts/account-command.ts) always call `emitProviderAccountsChanged`, but `manageExistingAccounts` in oauth-login.ts returns the mutated credentials from `login()` without it. RPC/app-server subscribers of `subscribeProviderAccountEvents` (e.g., the app-server account menu) will silently go stale after a user pins, unpins, removes, or logs out through the manager until some other action refreshes them. Consider emitting `emitProviderAccountsChanged(CLAUDE_SDK_OAUTH_PROVIDER_ID)` (or funneling through the account-management helpers) after each mutation branch of `manageExistingAccounts`, matching the command path.</violation>
</file>
<file name="packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/index.ts">
<violation number="1" location="packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/index.ts:51">
P1: Headless/RPC Claude login can no longer start when `CLAUDE_CODE_OAUTH_TOKEN` (or a numbered variant) is present: environment-account discovery enters the interactive account manager before OAuth, but RPC cannot answer that menu. Preserve the noninteractive OAuth flow or provide an RPC-compatible add-account path when selection is unavailable.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| streamSimple: streamClaudeSdkOauth, | ||
| oauth: createOAuthConfig({ | ||
| readCurrent: async () => readStoredCredential(CLAUDE_SDK_OAUTH_PROVIDER_ID), | ||
| readEnv: (name) => process.env[name], |
There was a problem hiding this comment.
P1: Headless/RPC Claude login can no longer start when CLAUDE_CODE_OAUTH_TOKEN (or a numbered variant) is present: environment-account discovery enters the interactive account manager before OAuth, but RPC cannot answer that menu. Preserve the noninteractive OAuth flow or provide an RPC-compatible add-account path when selection is unavailable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/index.ts, line 51:
<comment>Headless/RPC Claude login can no longer start when `CLAUDE_CODE_OAUTH_TOKEN` (or a numbered variant) is present: environment-account discovery enters the interactive account manager before OAuth, but RPC cannot answer that menu. Preserve the noninteractive OAuth flow or provide an RPC-compatible add-account path when selection is unavailable.</comment>
<file context>
@@ -48,6 +48,7 @@ export default function claudeSdkOauthExtension(pi: ExtensionAPI): void {
streamSimple: streamClaudeSdkOauth,
oauth: createOAuthConfig({
readCurrent: async () => readStoredCredential(CLAUDE_SDK_OAUTH_PROVIDER_ID),
+ readEnv: (name) => process.env[name],
readAnthropicCredential: async () => {
const credential = readStoredCredential("anthropic");
</file context>
| return undefined; | ||
| case "remove": { | ||
| const name = await pickAccount(callbacks, credential.accounts ?? [], "Log out of which stored account?"); | ||
| return name ? removeAccount(credential, name) : credential; |
There was a problem hiding this comment.
P2: Account mutations made through the new provider-login account manager don't notify account-change listeners. The same pin/remove operations via /claude-account (account-management.ts/account-command.ts) always call emitProviderAccountsChanged, but manageExistingAccounts in oauth-login.ts returns the mutated credentials from login() without it. RPC/app-server subscribers of subscribeProviderAccountEvents (e.g., the app-server account menu) will silently go stale after a user pins, unpins, removes, or logs out through the manager until some other action refreshes them. Consider emitting emitProviderAccountsChanged(CLAUDE_SDK_OAUTH_PROVIDER_ID) (or funneling through the account-management helpers) after each mutation branch of manageExistingAccounts, matching the command path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/oauth-login.ts, line 133:
<comment>Account mutations made through the new provider-login account manager don't notify account-change listeners. The same pin/remove operations via `/claude-account` (account-management.ts/account-command.ts) always call `emitProviderAccountsChanged`, but `manageExistingAccounts` in oauth-login.ts returns the mutated credentials from `login()` without it. RPC/app-server subscribers of `subscribeProviderAccountEvents` (e.g., the app-server account menu) will silently go stale after a user pins, unpins, removes, or logs out through the manager until some other action refreshes them. Consider emitting `emitProviderAccountsChanged(CLAUDE_SDK_OAUTH_PROVIDER_ID)` (or funneling through the account-management helpers) after each mutation branch of `manageExistingAccounts`, matching the command path.</comment>
<file context>
@@ -48,17 +51,131 @@ async function promptAccountName(callbacks: OAuthLoginCallbacks, existing: Accou
+ return undefined;
+ case "remove": {
+ const name = await pickAccount(callbacks, credential.accounts ?? [], "Log out of which stored account?");
+ return name ? removeAccount(credential, name) : credential;
+ }
+ case "logout-all": {
</file context>
| message: `Claude accounts\n${accountSummary(credential, readEnv)}`, | ||
| options: [ | ||
| { id: "add", label: "Add an account" }, | ||
| { id: "remove", label: "Log out of one account" }, |
There was a problem hiding this comment.
P2: When only environment-backed Claude accounts exist, choosing “Log out of one account” opens an empty account picker with no way to complete the action. Hide this action unless at least one stored account exists, or handle the empty stored-account case before opening the picker.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/oauth-login.ts, line 120:
<comment>When only environment-backed Claude accounts exist, choosing “Log out of one account” opens an empty account picker with no way to complete the action. Hide this action unless at least one stored account exists, or handle the empty stored-account case before opening the picker.</comment>
<file context>
@@ -48,17 +51,131 @@ async function promptAccountName(callbacks: OAuthLoginCallbacks, existing: Accou
+ message: `Claude accounts\n${accountSummary(credential, readEnv)}`,
+ options: [
+ { id: "add", label: "Add an account" },
+ { id: "remove", label: "Log out of one account" },
+ { id: "logout-all", label: "Log out of every stored account" },
+ { id: "pin", label: "Pin an account" },
</file context>
Suggested change
| { id: "remove", label: "Log out of one account" }, | |
| ...(credential.accounts?.length ? [{ id: "remove", label: "Log out of one account" }] : []), |
| if (prompt.type === "select") { | ||
| const labels = prompt.options.map((option) => option.label); | ||
| const selected = await ctx.ui.select(prompt.message, labels); | ||
| const id = prompt.options.find((option) => option.label === selected)?.id; |
There was a problem hiding this comment.
P2: Account-management selections are ambiguous when a stored account uses a synthetic environment name such as env: choosing the environment row maps to the first same-label option. Reserving environment names or making the displayed labels/IDs source-aware would prevent pinning or unblocking the wrong account.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/account-command.ts, line 171:
<comment>Account-management selections are ambiguous when a stored account uses a synthetic environment name such as `env`: choosing the environment row maps to the first same-label option. Reserving environment names or making the displayed labels/IDs source-aware would prevent pinning or unblocking the wrong account.</comment>
<file context>
@@ -165,6 +165,13 @@ async function addAccount(ctx: ExtensionCommandContext): Promise<void> {
+ if (prompt.type === "select") {
+ const labels = prompt.options.map((option) => option.label);
+ const selected = await ctx.ui.select(prompt.message, labels);
+ const id = prompt.options.find((option) => option.label === selected)?.id;
+ if (!id) throw new Error("Login cancelled");
+ return id;
</file context>
| await ctx.modelRegistry.modelRuntime.login(CLAUDE_SDK_OAUTH_PROVIDER_ID, "oauth", { | ||
| signal: ctx.signal, | ||
| prompt: async (prompt) => { | ||
| if (prompt.type === "select") { |
There was a problem hiding this comment.
P3: /claude-account add can now resolve through the account manager without adding an account (remove, logout-all, pin, unpin, unblock), yet it always notifies 'Claude SDK OAuth account added.' and emits an accounts-changed event. When the manager returns a managed credential rather than a fresh OAuth login, the success message (and the emit) describe an add that never happened. Consider distinguishing the 'new account persisted' case from manager resolutions before showing the success notification, for example by having the login report which branch it took.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/core/extensions/builtin/claude-sdk-oauth/account-command.ts, line 168:
<comment>`/claude-account add` can now resolve through the account manager without adding an account (remove, logout-all, pin, unpin, unblock), yet it always notifies 'Claude SDK OAuth account added.' and emits an accounts-changed event. When the manager returns a managed credential rather than a fresh OAuth login, the success message (and the emit) describe an add that never happened. Consider distinguishing the 'new account persisted' case from manager resolutions before showing the success notification, for example by having the login report which branch it took.</comment>
<file context>
@@ -165,6 +165,13 @@ async function addAccount(ctx: ExtensionCommandContext): Promise<void> {
await ctx.modelRegistry.modelRuntime.login(CLAUDE_SDK_OAUTH_PROVIDER_ID, "oauth", {
signal: ctx.signal,
prompt: async (prompt) => {
+ if (prompt.type === "select") {
+ const labels = prompt.options.map((option) => option.label);
+ const selected = await ctx.ui.select(prompt.message, labels);
</file context>
| return count === 1 ? "1 monitor on duty" : `${count} monitors on duty`; | ||
| } | ||
|
|
||
| export function formatGoalWaitLabel(input: GoalWaitLabelInput): string { |
There was a problem hiding this comment.
P3: This new wait-progress.ts module is currently unreferenced by any production code — nothing calls renderGoalWaitBar, formatGoalWaitLabel, or reads GOAL_WAIT_BAR_CELLS outside its own test file (goal-wait-progress.test.ts). The change is an isolated render layer with the wiring left for a future maintainer decision, which is documented in changes.md, so it is not an accidental leftover. However, shipping a fully unused module that could be abandoned (the doc even asks whether the countdown should be text-only) means the exported surface has no production consumer yet. Consider keeping this behind the follow-up PR that wires it into the footer so the API doesn't land and potentially churn before it is actually used.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/core/extensions/builtin/goal/wait-progress.ts, line 36:
<comment>This new `wait-progress.ts` module is currently unreferenced by any production code — nothing calls `renderGoalWaitBar`, `formatGoalWaitLabel`, or reads `GOAL_WAIT_BAR_CELLS` outside its own test file (`goal-wait-progress.test.ts`). The change is an isolated render layer with the wiring left for a future maintainer decision, which is documented in `changes.md`, so it is not an accidental leftover. However, shipping a fully unused module that could be abandoned (the doc even asks whether the countdown should be text-only) means the exported surface has no production consumer yet. Consider keeping this behind the follow-up PR that wires it into the footer so the API doesn't land and potentially churn before it is actually used.</comment>
<file context>
@@ -0,0 +1,43 @@
+ return count === 1 ? "1 monitor on duty" : `${count} monitors on duty`;
+}
+
+export function formatGoalWaitLabel(input: GoalWaitLabelInput): string {
+ const bar = renderGoalWaitBar(elapsedRatioOf(input.remainingMs, input.totalMs));
+ const remaining = formatWakeDuration(Math.max(0, input.remainingMs));
</file context>
| return clampRatio((totalMs - Math.max(0, remainingMs)) / totalMs); | ||
| } | ||
|
|
||
| function monitorsOnDuty(count: number): string { |
There was a problem hiding this comment.
P3: The wait label duplicates the existing cache-warm.ts monitor-count formatter, so future wording changes can make these user-facing notices inconsistent. Reusing a shared/exported helper would keep the monitor status text in sync.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/core/extensions/builtin/goal/wait-progress.ts, line 32:
<comment>The wait label duplicates the existing `cache-warm.ts` monitor-count formatter, so future wording changes can make these user-facing notices inconsistent. Reusing a shared/exported helper would keep the monitor status text in sync.</comment>
<file context>
@@ -0,0 +1,43 @@
+ return clampRatio((totalMs - Math.max(0, remainingMs)) / totalMs);
+}
+
+function monitorsOnDuty(count: number): string {
+ return count === 1 ? "1 monitor on duty" : `${count} monitors on duty`;
+}
</file context>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Linear: https://linear.app/jgplabs/issue/LAB-28/senpi-multiaccounts-로컬-미적용으로-계정별-사용량과-관리-메뉴-누락
Depends on #638 for fail-closed behavior after the final managed Claude account is removed.
Summary
Verification
Summary by cubic
Adds an account manager for
claude-sdk-oauthso users can manage multiple Claude accounts without restarting OAuth. Also adds a wait-progress render helper and enforces fail-closed behavior when no managed accounts remain. Addresses Linear LAB-28 by restoring per-account controls and menu visibility.New Features
goal/wait-progresshelpers:renderGoalWaitBarandformatGoalWaitLabel(render layer only, not yet wired).Bug Fixes
Written for commit f367f26. Summary will update on new commits.