Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"@hono/node-server": "^1.14.1",
"@hookform/resolvers": "^5.2.2",
"@lezer/highlight": "^1.2.3",
"@mastra/core": "1.25.0",
"@parcel/watcher": "^2.5.6",
"@pierre/diffs": "1.1.3",
"@radix-ui/react-dialog": "^1.1.15",
Expand Down Expand Up @@ -200,7 +201,7 @@
"lowdb": "^7.0.1",
"lowlight": "^3.3.0",
"lucide-react": "^0.563.0",
"mastracode": "0.9.2",
"mastracode": "0.14.0",
"nanoid": "^5.1.6",
"node-addon-api": "^7.1.0",
"node-pty": "1.1.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# Manual Testing Plan — PR #3517

## Prerequisites
- Desktop dev running (`bun dev` from apps/desktop, or full `bun dev` from root)
- At least one project configured with a git repo

## 1. v1 AI Branch Naming (API key path)

**Setup**: `ANTHROPIC_API_KEY` or `OPENAI_API_KEY` set in env (or stored via Settings > Models).

| Step | Expected |
|---|---|
| Open v1 new-workspace modal (Cmd+N) | Modal opens |
| Type a prompt: "fix dropdown alignment bug" | Text entered |
| Submit (Enter or click Create) | Modal closes, pending workspace shows "Generating branch…" briefly |
| Wait for workspace to initialize | Branch name is AI-generated kebab-case (e.g. `fix-dropdown-alignment`), not random words |
| Check worktree | Branch exists locally |

## 2. v1 AI Branch Naming (no credentials)

**Setup**: unset `ANTHROPIC_API_KEY` and `OPENAI_API_KEY` from env. No stored API keys in Settings > Models.

| Step | Expected |
|---|---|
| Create workspace with prompt | Branch name falls back to random friendly name (e.g. `pickle-streetcar`) or prompt-derived slug |
| No error toast | Degradation is silent |

## 3. v1 Workspace Auto-Rename

**Setup**: API key available.

| Step | Expected |
|---|---|
| Create workspace with prompt "refactor auth middleware" | Workspace title updates to AI-generated name (e.g. "Refactor Auth Middleware") after a few seconds |
| If no API key available | Title falls back to prompt text or friendly name |

## 4. Anthropic OAuth Auto-Refresh (from #3510)

**Setup**: Anthropic OAuth configured (Claude Max). Requires waiting for token expiry or manual simulation.

| Step | Expected |
|---|---|
| Sign in to Anthropic via OAuth in Settings > Models | "Active" badge appears |
| Force-expire: edit `~/Library/Application Support/mastracode/auth.json`, set `anthropic.expires` to a past timestamp | — |
| Send a chat message | Chat succeeds silently (token auto-refreshed via `authStorage.getApiKey`). No "Reconnect" prompt. |
| If refresh token is also invalid | Falls to expired state, "Expired" badge + "Reconnect" button appears |
| Check terminal for `[chat-service] Anthropic OAuth refresh failed` | Logged if refresh fails |

## 5. Settings > Models Page

| Step | Expected |
|---|---|
| Navigate to Settings > Models | Page loads with Anthropic + OpenAI sections, each with provider icon in header |
| Each provider shows a single card with OAuth row + API Key row | OAuth row: label + badge + action. API Key row: input + contextual buttons |
| **Disconnected state** | "Not connected" badge, primary "Connect" button, no Save/Clear buttons |
| **API key flow**: type key → Save appears → click Save | "API key updated" toast, "Active" badge, "Logout" button appears |
| **API key flow**: click Clear | Key removed, badge reverts to "Not connected" |
| **OAuth flow**: click Connect → complete in browser | "Active" badge, "Logout" button |
| **OAuth flow**: click Logout | Badge reverts, Connect button returns |
| **API key + OAuth**: set API key, then connect OAuth, then disconnect OAuth | API key should survive the OAuth cycle (backup/restore workaround) |
| **OpenAI dialog** auto-opens browser on Connect | No manual "Open browser" step needed |
| **Copy URL** button shows "Copied!" feedback for 2s | — |

## 6. Production Build

| Step | Expected |
|---|---|
| `bun run compile:app` (from apps/desktop) | Succeeds. `get-small-model` chunk ~1.2 MB, no 20 MB chunk. |
| `bun run copy:native-modules` | Succeeds |
| `bun run validate:native-runtime` | All checks pass |
| `npx electron dist/main/index.js` | Main process boots (renderer 404 expected in non-packaged mode). No onnxruntime error. |

## 7. Host-Service Procedure (dormant — future v2)

Not yet wired to UI. Verify via tRPC playground or direct call if available:

| Step | Expected |
|---|---|
| Call `workspaceCreation.generateBranchName({ projectId, prompt: "fix auth bug" })` | Returns `{ branchName: "fix-auth-bug" }` or similar (requires API key in host-service env) |
| Call with empty prompt | Returns `{ branchName: null }` |
| Call with no API key in env | Returns `{ branchName: null }` (graceful fallback) |

## Known Regressions (documented, accepted)

- **OAuth-only users** (Claude Max / OpenAI Codex without stored API key) get random branch names and prompt-derived workspace titles for small-model tasks. Main chat retains full OAuth.
- **Upstream dependency**: API key storage slot collision with OAuth is worked around via backup/restore. Proper fix tracked at mastra-ai/mastra#15483.
168 changes: 168 additions & 0 deletions apps/desktop/plans/done/20260415-v2-host-service-ai-branch-naming.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
# V2 Workspace Modal — Host-Service AI Branch Naming

Port v1's AI branch-name generation into v2's workspace modal, routed through host-service. Approach: **use upstream `mastracode`'s `resolveModel`** via a lightweight `createMastraCode({ disableMcp: true, disableHooks: true })` singleton. Delete our small-model abstraction; keep OAuth parity (Claude Max + Codex) because mastracode handles it internally.

## Completed

- ✅ Bumped `mastracode` 0.9.2 → **0.14.0** (+ transitive `@mastra/core` 1.16 → 1.25). Typecheck + tests green. Removed `minimumReleaseAge` from `bunfig.toml`.

## Target architecture

```
v2 useSubmitWorkspace
└─> client.workspaceCreation.generateBranchName.mutate({ projectId, prompt })
└─> generateBranchNameFromPrompt(...) [host-service]
└─> getSmallModel() [shared helper]
└─> resolveModel(modelId) from mastracode
(full auth: API-key + keychain + OAuth middleware)
```

Desktop v1's existing `ai-branch-name.ts` migrates to the same `getSmallModel` helper — single implementation, two consumers.

## Shared helper

`packages/chat/src/server/shared/small-model/get-small-model.ts`:

```ts
import { createAuthStorage, createMastraCode } from "mastracode";
import type { MastraLanguageModel } from "@mastra/core/llm";

const ANTHROPIC_SMALL = "anthropic/claude-haiku-4-5-20251001";
const OPENAI_SMALL = "openai/gpt-4o-mini";

type Resolver = Awaited<ReturnType<typeof createMastraCode>>["resolveModel"];
let initPromise: Promise<Resolver> | null = null;

function getResolver(): Promise<Resolver> {
if (!initPromise) {
initPromise = createMastraCode({ disableMcp: true, disableHooks: true })
.then((r) => r.resolveModel);
}
return initPromise;
}

function pickSmallModelId(): string | null {
const auth = createAuthStorage();
auth.reload();
if (auth.has("anthropic")) return ANTHROPIC_SMALL;
if (auth.has("openai")) return OPENAI_SMALL;
return null;
}

export async function getSmallModel(): Promise<MastraLanguageModel | null> {
const modelId = pickSmallModelId();
if (!modelId) return null;
const resolveModel = await getResolver();
return resolveModel(modelId) as MastraLanguageModel;
}
```

Module-level promise caches the mastracode init (one-time cost per process). Credential check is per-call (cheap, in-memory).

## Code-removal budget

| File | LOC | Fate |
|---|---|---|
| `apps/desktop/src/lib/ai/call-small-model.ts` | 184 | delete |
| `apps/desktop/src/lib/ai/call-small-model.test.ts` | 399 | delete |
| `apps/desktop/src/lib/ai/provider-diagnostics.ts` | 89 | delete if no other consumer |
| `packages/chat/src/server/desktop/small-model/small-model.ts` | 146 | delete |
| `packages/chat/src/server/desktop/small-model/small-model.test.ts` | 391 | delete |
| `packages/chat/src/server/desktop/title-generation/title-generation.ts` | 99 | trim (~50, drop streaming variant) |
| `packages/chat/src/server/desktop/auth/anthropic/anthropic.ts` | 232 | trim (~50, keep OAuth login helpers chat-service uses) |
| `packages/chat/src/server/desktop/auth/openai/openai.ts` | 99 | trim (~30) |
| `apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-branch-name.ts` | 117 | rewrite → ~60 |
| New `shared/small-model/get-small-model.ts` | — | +50 |

Net: **~1200 lines removed**.

---

## Step 1 — Shared helper + migrate v1 branch naming

### Actionable tasks
1. Create `packages/chat/src/server/shared/small-model/{get-small-model.ts, index.ts}` with the helper above.
2. Update `packages/chat/src/server/desktop/index.ts` barrel if needed; new helper lives in `shared/` and is imported directly from `@superset/chat/server/shared/small-model` — no re-export from desktop.
3. Rewrite `apps/desktop/src/lib/trpc/routers/workspaces/utils/ai-branch-name.ts`:
- Replace `callSmallModel` + provider branching with `getSmallModel()` + `generateText({ model, system, prompt })`.
- Keep `BRANCH_NAME_INSTRUCTIONS`, `resolveConflict`, `sanitizeBranchNameWithMaxLength`.
4. Grep for `callSmallModel`, `SmallModelProvider`, `getDefaultSmallModelProviders`, `generateTitleFromMessageWithStreamingModel`:
- Rewrite each consumer to `getSmallModel` + `generateText` (or Mastra Agent if the caller wants tracing).
5. Delete:
- `apps/desktop/src/lib/ai/call-small-model.ts` + test.
- `packages/chat/src/server/desktop/small-model/small-model.ts` + test + `index.ts`.
- `generateTitleFromMessageWithStreamingModel` from `title-generation.ts`.
6. `apps/desktop/src/lib/ai/provider-diagnostics.ts` — grep for consumers; delete if only `call-small-model.ts` uses it. Otherwise leave.
7. Audit `auth/anthropic` and `auth/openai`: keep exports chat-service uses for OAuth login UI; delete any credential-resolution helpers used only for small-model.
8. Run `bun run typecheck` + focused tests (chat-service, ai-branch-name). Fix breaks.
9. Smoke: launch desktop, create v1 workspace with a prompt, verify AI branch naming still works (both API key and OAuth paths).

### Risks (step 1)
- **mastracode init side effects**: `createMastraCode` with disabled MCP/hooks still initializes storage, auto-detects project, etc. Confirm startup stays under ~200ms and doesn't create unwanted files. If it tries to touch a DB/libsql, pass an explicit `storage` config.
- **Second init conflict**: chat service already calls `createMastraCode` for its runtime. Running a second one for small-model might duplicate auth-storage singletons or compete for files. Mitigation: verify `createMastraCode` is side-effect-safe when called twice; if not, share the existing chat runtime's resolver.
- **Credential regression**: `authStorage.has("anthropic")` must cover all the "credential present" cases our current `getAnthropicCredentialsFromAnySource` covers (env vars, stored API keys, OAuth). Audit before replacing.

---

## Step 2 — Host-service procedure

### Actionable tasks
1. Port `sanitizeBranchNameWithMaxLength` (`apps/desktop/src/shared/utils/branch.ts`) and `resolveBranchPrefix` (`apps/desktop/src/lib/trpc/routers/workspaces/utils/branch-prefix.ts`) into `packages/host-service/src/trpc/router/workspace-creation/utils/`.
2. Create `packages/host-service/src/trpc/router/workspace-creation/utils/ai-branch-name.ts` — same helper as desktop's rewritten v1, imports `getSmallModel` from `@superset/chat/server/shared/small-model`.
3. Add to `workspace-creation.ts`:
```ts
generateBranchName: publicProcedure
.input(z.object({ projectId: z.string(), prompt: z.string() }))
.mutation(async ({ input }) => {
const trimmed = input.prompt.trim();
if (!trimmed) return { branchName: null };
const project = /* existing project lookup */;
const existingBranches = /* existing branch listing */;
const prefix = await resolveBranchPrefix(project, existingBranches);
const branchName = await generateBranchNameFromPrompt(trimmed, existingBranches, prefix);
return { branchName };
}),
```
4. Delete `packages/host-service/src/providers/model-providers/LocalModelProvider/utils/resolveAnthropicCredential.ts` + `resolveOpenAICredential.ts` if unused after step (LocalModelProvider no longer needs them since auth flows through mastracode).
5. Run typecheck + host-service tests.

---

## Step 3 — Wire v2

### Actionable tasks
1. Update `apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/hooks/useSubmitWorkspace/useSubmitWorkspace.ts`:
- Compute `willGenerateAIName = !draft.branchNameEdited && !!trimmedPrompt && !draft.linkedPR`.
- Fallback via `resolveNames(draft)` (unchanged).
- Insert pending row with status `"generating-branch"` if `willGenerateAIName`.
- Close + navigate (unchanged).
- If `willGenerateAIName`, race `client.workspaceCreation.generateBranchName.mutate(...)` vs 30s timeout:
- success → update pending row `branchName` + status `"creating"`.
- auth error → toast + abort + remove pending row.
- other/timeout → toast `"Using random branch name..."`, keep fallback name.
- Call `client.workspaceCreation.create(...)` with resolved `branchName`.
2. Add `"generating-branch"` to `pendingWorkspaces` status union (`packages/local-db/src/schema/schema.ts`). Drizzle migration.
3. Update pending page UI (`apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx`) to render "Naming your branch…" for that status.

---

## Effort

| Step | Effort |
|---|---|
| 0. mastracode upgrade | ✅ done |
| 1. Shared helper + v1 migration + deletions | 2–3 hrs |
| 2. Host-service procedure | 1–1.5 hrs |
| 3. v2 wiring + pending UI | 1–2 hrs |
| **Remaining** | **~4–6.5 hrs** |

## Risks

- **mastracode init side effects** at singleton init (see step 1).
- **Remote host-service API-key availability**: remote hosts need `ANTHROPIC_API_KEY` / `OPENAI_API_KEY` set; otherwise v2 on remote hosts falls back to random-name. Document.
- **OAuth parity in host-service**: host-service can't do an interactive OAuth flow. `createAuthStorage().loadStoredApiKeysIntoEnv(...)` loads stored API keys but NOT OAuth tokens into env. For host-service, OAuth-only users get random names.
- **Diagnostics UI**: removing `provider-diagnostics.ts` removes mid-call `reportProviderIssue` signals. Audit settings UI for providers; they may source signals from chat-service regardless.

## Out of scope
- Live/debounced ghost suggestion in v2 branch-name input.
- Retiring v1's desktop-tRPC `generateBranchName` procedure (it becomes a proxy over the shared helper; deleting it is a follow-up).
57 changes: 57 additions & 0 deletions apps/desktop/plans/done/20260417-fix-api-key-storage-slot.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Fix: API keys overwritten by OAuth connect/disconnect cycle

## Problem

Settings > Models "API key" field writes to the same auth.json slot as OAuth. When a user:
1. Saves an API key → `authStorage.set("anthropic", { type: "api_key", key: "sk-..." })`
2. Connects OAuth → `authStorage.login("anthropic", ...)` overwrites with `{ type: "oauth", ... }`
3. Disconnects OAuth → `authStorage.remove("anthropic")` deletes everything

The API key is lost. The model picker shows "disabled" even though the user saved a key.

Chat still works because `createMastraCode`'s model resolution reads from env vars / external config independently of this status check.

## Root cause

`setApiKeyForProvider` uses `authStorage.set(providerId, credential)` which writes to the main provider slot. OAuth also writes to the same slot. They collide.

mastracode's `AuthStorage` has **two separate storage mechanisms**:
- `set(providerId, credential)` / `get(providerId)` → main slot (`"anthropic"` in auth.json)
- `setStoredApiKey(providerId, key)` / `getStoredApiKey(providerId)` → dedicated API key slot (`"apikey:anthropic"` in auth.json)

We're using the wrong one for API keys.

## Fix

### `auth-storage-utils.ts`

**`setApiKeyForProvider`**: switch from `authStorage.set()` to `authStorage.setStoredApiKey()`.

**`clearApiKeyForProvider`**: clear the `apikey:` slot. Use `authStorage.set("apikey:<providerId>", ...)` with a removal, or check `hasStoredApiKey` and handle accordingly. Since mastracode doesn't expose `removeStoredApiKey`, use `authStorage.remove("apikey:<providerId>")`.

**`resolveAuthMethodForProvider`**: after checking the main slot, also check `authStorage.hasStoredApiKey(providerId)` as a fallback → return `"api_key"`.

### `chat-service.ts`

No changes needed — `getAnthropicAuthStatus` and `getOpenAIAuthStatus` already delegate to `resolveAuthMethodForProvider` which will now find stored API keys.

The `setStoredAnthropicApiKeyFromEnvVariables` helper in `disconnectAnthropicOAuth` should also use `setStoredApiKey` for consistency, but it's less critical since it reads from the env config file.

## Behavior after fix

| Action | `"anthropic"` (main) | `"apikey:anthropic"` (dedicated) |
|---|---|---|
| Save API key (Settings) | unchanged | written |
| Connect OAuth | overwritten with OAuth | survives |
| Disconnect OAuth | removed | survives |
| Auth status check | reads both | ← |

## Side effect: small-model tasks

`getSmallModel` reads `apikey:anthropic` from auth.json directly. Currently, API keys saved via Settings go to the main `"anthropic"` slot, so `getSmallModel` doesn't find them. After this fix, saved API keys land in `apikey:anthropic` where `getSmallModel` already looks → branch naming works for Settings-saved keys without any additional change.

## Scope

- `packages/chat/src/server/desktop/chat-service/auth-storage-utils.ts` (~15 LOC changed)
- `packages/chat/src/server/desktop/chat-service/chat-service.ts` — `setStoredAnthropicApiKeyFromEnvVariables` updated for consistency (~2 LOC)
- Tests in `chat-service.test.ts` if any mock `setApiKeyForProvider` behavior
5 changes: 5 additions & 0 deletions apps/desktop/runtime-dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ const packagedSupportModules = [
export const mainExternalizedDependencies = [
...externalizedRuntimeModules.map((module) => module.specifier),
"pg-native",
// mastracode transitively loads @mastra/fastembed → onnxruntime-node, whose
// native binding is loaded via a dynamic `require` that @rollup/plugin-commonjs
// can't resolve at bundle time. Externalizing lets Node handle the require at
// runtime from node_modules. Also keeps the bundle size sane (~20 MB chunk).
"mastracode",
];

export const packagedNodeModuleCopies = [
Expand Down
Loading
Loading