diff --git a/.claude/commands/finalize.md b/.claude/commands/finalize.md index c7c218ece..d49150e80 100644 --- a/.claude/commands/finalize.md +++ b/.claude/commands/finalize.md @@ -410,7 +410,14 @@ cd apps/ade-cli && npm test The desktop workspace has 3 projects (`unit-main`, `unit-renderer`, `unit-shared`); sharding distributes across all three automatically. -If a shard fails, re-run **only that shard** (or only the failing test file). Never re-run all 9 to verify a one-file fix. +If a shard fails, **re-run ONLY the failing test file(s)** — not the whole shard, and never the full sharded run. Sharding is a wall-clock optimization for the *initial* gate; once you've isolated which file failed, the cheapest signal is `npx vitest run `. A 90-second shard rerun to verify a 5-second one-file fix is wasted time and burns the prompt cache. + +Anti-pattern (do NOT do this): +- "Shard 1 had failures, let me rerun shard 1" → wrong; rerun just the failing file. +- "I fixed the failing file, let me rerun the whole shard to be sure" → wrong; rerun just that file. The other tests in the shard already passed and didn't change. +- "Let me rerun all 8 shards after a one-file fix" → wrong; this is the worst option. + +Only run the full sharded suite once at the start of Phase 3e. After that, narrow scope to failing files until they pass, then move on. Workspace-project subsets exist for debugging only; they are NOT a substitute for the sharded run: @@ -463,7 +470,7 @@ git diff --name-only | sort > /tmp/finalize-session-files.txt comm -13 /tmp/finalize-branch-files.txt /tmp/finalize-session-files.txt ``` -Revert the simplifier's edits to the offending files and re-run only the failed shard. Do NOT rewrite the test suite in Phase 3 — tests that drift because the feature branch refactored UI are a separate follow-up. +Revert the simplifier's edits to the offending files and re-run **only the failing test file** (not the full shard). Do NOT rewrite the test suite in Phase 3 unless the user explicitly asks — tests that drift because the feature branch refactored UI are a separate follow-up by default. ### 3i. Cleanup lingering processes diff --git a/apps/desktop/src/main/services/github/githubService.test.ts b/apps/desktop/src/main/services/github/githubService.test.ts index ee3000113..4b2f47dc6 100644 --- a/apps/desktop/src/main/services/github/githubService.test.ts +++ b/apps/desktop/src/main/services/github/githubService.test.ts @@ -39,8 +39,10 @@ vi.mock("node:fs", async (importOriginal) => { }; }); +const runGitMock = vi.hoisted(() => vi.fn()); + vi.mock("../git/git", () => ({ - runGit: vi.fn(), + runGit: runGitMock, })); // Replace global fetch @@ -474,3 +476,98 @@ describe("githubService issue-domain helpers", () => { expect(url).toContain("/repos/scary%20owner/name%20with%20space/issues"); }); }); + +// --------------------------------------------------------------------------- +// getStatus — connection probing (regression for false-CONNECTED bug) +// --------------------------------------------------------------------------- + +describe("githubService.getStatus", () => { + beforeEach(() => { + vi.clearAllMocks(); + delete process.env.GITHUB_TOKEN; + delete process.env.ADE_GITHUB_TOKEN; + }); + + // Mocks `git remote get-url origin` so detectRepo returns acme/ade. + function stubOriginRemote() { + runGitMock.mockResolvedValue({ + exitCode: 0, + stdout: "git@github.com:acme/ade.git\n", + stderr: "", + }); + } + + it("classic token with required scopes is connected (no repo probe needed)", async () => { + stubOriginRemote(); + process.env.GITHUB_TOKEN = "ghp_classic"; + mockFetch.mockResolvedValueOnce( + jsonResponse(200, { login: "alice" }, { "x-oauth-scopes": "repo, workflow" }), + ); + // Repo probe still runs and we still mock it; the response just decorates the status. + mockFetch.mockResolvedValueOnce(jsonResponse(200, { full_name: "acme/ade" })); + + const status = await makeService().getStatus(); + + expect(status.tokenStored).toBe(true); + expect(status.tokenType).toBe("classic"); + expect(status.userLogin).toBe("alice"); + expect(status.scopes).toEqual(["repo", "workflow"]); + expect(status.connected).toBe(true); + }); + + it("fine-grained token that authenticates but cannot read the repo is NOT connected", async () => { + // This is the original bug: /user works, so userLogin is set, but the + // active repo isn't included in the token's selected repositories. Every + // PR-tab call would 404. Status must reflect that with connected=false. + stubOriginRemote(); + process.env.GITHUB_TOKEN = "github_pat_finegrained"; + mockFetch.mockResolvedValueOnce(jsonResponse(200, { login: "alice" })); + mockFetch.mockResolvedValueOnce(jsonResponse(404, { message: "Not Found" })); + + const status = await makeService().getStatus(); + + expect(status.tokenType).toBe("fine-grained"); + expect(status.userLogin).toBe("alice"); + expect(status.repoAccessOk).toBe(false); + expect(status.repoAccessError).toContain("404"); + expect(status.connected).toBe(false); + }); + + it("fine-grained token with successful repo probe is connected", async () => { + stubOriginRemote(); + process.env.GITHUB_TOKEN = "github_pat_finegrained"; + mockFetch.mockResolvedValueOnce(jsonResponse(200, { login: "alice" })); + mockFetch.mockResolvedValueOnce(jsonResponse(200, { full_name: "acme/ade" })); + + const status = await makeService().getStatus(); + + expect(status.tokenType).toBe("fine-grained"); + expect(status.repoAccessOk).toBe(true); + expect(status.connected).toBe(true); + }); + + it("classic token without required scopes is NOT connected", async () => { + stubOriginRemote(); + process.env.GITHUB_TOKEN = "ghp_classic"; + // Token has only `read:user` (insufficient). + mockFetch.mockResolvedValueOnce( + jsonResponse(200, { login: "alice" }, { "x-oauth-scopes": "read:user" }), + ); + mockFetch.mockResolvedValueOnce(jsonResponse(200, { full_name: "acme/ade" })); + + const status = await makeService().getStatus(); + + expect(status.scopes).toEqual(["read:user"]); + expect(status.connected).toBe(false); + }); + + it("missing token returns not-connected status with all the new fields populated", async () => { + stubOriginRemote(); + const status = await makeService().getStatus(); + + expect(status.tokenStored).toBe(false); + expect(status.connected).toBe(false); + expect(status.repoAccessOk).toBeNull(); + expect(status.repoAccessError).toBeNull(); + }); +}); diff --git a/apps/desktop/src/main/services/github/githubService.ts b/apps/desktop/src/main/services/github/githubService.ts index a06dee12c..db62f62ea 100644 --- a/apps/desktop/src/main/services/github/githubService.ts +++ b/apps/desktop/src/main/services/github/githubService.ts @@ -5,7 +5,7 @@ import type { Logger } from "../logging/logger"; import { runGit } from "../git/git"; import type { GitHubRepoRef, GitHubStatus } from "../../../shared/types"; import { resolveAdeLayout } from "../../../shared/adeLayout"; -import { parseGitHubScopeHeaders } from "../../../shared/githubScopes"; +import { getGitHubTokenAccessState, parseGitHubScopeHeaders } from "../../../shared/githubScopes"; import { nowIso, asString } from "../shared/utils"; @@ -207,6 +207,36 @@ export function createGithubService({ }; }; + // Verifies the active token actually has access to the active repo by hitting + // /repos/{owner}/{name} (cheapest endpoint that requires Metadata: Read on + // fine-grained tokens; classic tokens with `repo` scope also pass). This is + // the only reliable connectivity check for fine-grained tokens, which never + // return x-oauth-scopes and so cannot be introspected via headers. + const probeRepoAccess = async ( + token: string, + repo: GitHubRepoRef, + ): Promise<{ ok: boolean; error: string | null }> => { + try { + const response = await fetch( + `https://api.github.com/repos/${encodeURIComponent(repo.owner)}/${encodeURIComponent(repo.name)}`, + { + method: "GET", + headers: { + accept: "application/vnd.github+json", + authorization: `Bearer ${token}`, + "user-agent": "ade-desktop", + }, + }, + ); + if (response.ok) return { ok: true, error: null }; + const payload = (await response.json().catch(() => ({}))) as Record; + const message = asString(payload.message) || `HTTP ${response.status}`; + return { ok: false, error: `${response.status}: ${message}` }; + } catch (error) { + return { ok: false, error: error instanceof Error ? error.message : String(error) }; + } + }; + // ETag cache for conditional GET requests. Responses that return 304 Not Modified // don't count against GitHub's rate limit, so this dramatically reduces API usage. const etagCache = new Map(); @@ -337,7 +367,37 @@ export function createGithubService({ let cachedStatus: GitHubStatus | null = null; let cachedAt = 0; - const getStatus = async (): Promise => { + // Decides whether the saved token is actually usable for the project. Classic + // tokens require both required scopes; fine-grained tokens require a successful + // repo probe (since their permissions are not introspectable via headers). + const computeConnected = (args: { + tokenStored: boolean; + userLogin: string | null; + tokenType: GitHubStatus["tokenType"]; + scopes: string[]; + repo: GitHubRepoRef | null; + repoAccessOk: boolean | null; + }): boolean => { + if (!args.tokenStored || !args.userLogin) return false; + if (args.tokenType === "fine-grained") { + // No repo to probe (e.g. project without a GitHub remote): a fine-grained + // token that authenticates as a user is the best signal we have. + if (!args.repo) return true; + return args.repoAccessOk === true; + } + if (args.tokenType === "classic") { + const access = getGitHubTokenAccessState(args.scopes); + return access.hasRequiredAccess; + } + // Unknown prefix — fall back to "user lookup worked" (best-effort). + return Boolean(args.userLogin); + }; + + const getStatus = async (opts: { forceRefresh?: boolean } = {}): Promise => { + if (opts.forceRefresh) { + cachedStatus = null; + cachedAt = 0; + } const token = readStoredToken(); const repo = await detectRepo().catch(() => null); if (!token) { @@ -349,7 +409,10 @@ export function createGithubService({ repo, userLogin: null, scopes: [], - checkedAt: null + checkedAt: null, + repoAccessOk: null, + repoAccessError: null, + connected: false, }; cachedAt = Date.now(); return cachedStatus; @@ -357,12 +420,48 @@ export function createGithubService({ const now = Date.now(); if (cachedStatus && now - cachedAt < 30_000 && cachedStatus.tokenStored) { - // Still re-detect repo, it is cheap and reflects changed remotes. - return { ...cachedStatus, repo }; + // Still re-detect repo and re-evaluate `connected` so a remote change is reflected. + const repoChanged = + (cachedStatus.repo?.owner ?? null) !== (repo?.owner ?? null) || + (cachedStatus.repo?.name ?? null) !== (repo?.name ?? null); + // If the repo just changed we can't trust the cached probe result. + const repoAccessOk = repoChanged ? null : cachedStatus.repoAccessOk; + const repoAccessError = repoChanged ? null : cachedStatus.repoAccessError; + const connected = computeConnected({ + tokenStored: true, + userLogin: cachedStatus.userLogin, + tokenType: cachedStatus.tokenType, + scopes: cachedStatus.scopes, + repo, + repoAccessOk, + }); + return { ...cachedStatus, repo, repoAccessOk, repoAccessError, connected }; } try { const validated = await validateToken(token); + let repoAccessOk: boolean | null = null; + let repoAccessError: string | null = null; + if (repo) { + const probe = await probeRepoAccess(token, repo); + repoAccessOk = probe.ok; + repoAccessError = probe.error; + if (!probe.ok) { + logger.warn("github.repo_probe_failed", { + repo: `${repo.owner}/${repo.name}`, + tokenType: validated.tokenType, + error: probe.error, + }); + } + } + const connected = computeConnected({ + tokenStored: true, + userLogin: validated.userLogin, + tokenType: validated.tokenType, + scopes: validated.scopes, + repo, + repoAccessOk, + }); cachedStatus = { tokenStored: true, tokenDecryptionFailed: false, @@ -371,7 +470,10 @@ export function createGithubService({ repo, userLogin: validated.userLogin, scopes: validated.scopes, - checkedAt: nowIso() + checkedAt: nowIso(), + repoAccessOk, + repoAccessError, + connected, }; cachedAt = now; return cachedStatus; @@ -385,7 +487,10 @@ export function createGithubService({ repo, userLogin: null, scopes: [], - checkedAt: nowIso() + checkedAt: nowIso(), + repoAccessOk: null, + repoAccessError: null, + connected: false, }; cachedAt = now; return cachedStatus; diff --git a/apps/desktop/src/main/services/ipc/registerIpc.ts b/apps/desktop/src/main/services/ipc/registerIpc.ts index 9fb6a34d1..5c138d938 100644 --- a/apps/desktop/src/main/services/ipc/registerIpc.ts +++ b/apps/desktop/src/main/services/ipc/registerIpc.ts @@ -5072,21 +5072,36 @@ export function registerIpc({ ipcMain.handle(IPC.conflictsSuggestResolverTarget, async (_event, arg) => getCtx().conflictService.suggestResolverTarget(arg)); - ipcMain.handle(IPC.githubGetStatus, async (): Promise => { + const broadcastGithubStatus = (status: GitHubStatus): void => { + for (const win of BrowserWindow.getAllWindows()) { + if (win.isDestroyed()) continue; + try { + win.webContents.send(IPC.githubStatusChanged, status); + } catch { + // ignore broadcast failures + } + } + }; + + ipcMain.handle(IPC.githubGetStatus, async (_event, arg?: { forceRefresh?: boolean }): Promise => { const ctx = getCtx(); - return await ctx.githubService.getStatus(); + return await ctx.githubService.getStatus({ forceRefresh: Boolean(arg?.forceRefresh) }); }); ipcMain.handle(IPC.githubSetToken, async (_event, arg: { token: string }): Promise => { const ctx = getCtx(); ctx.githubService.setToken(arg.token); - return await ctx.githubService.getStatus(); + const status = await ctx.githubService.getStatus(); + broadcastGithubStatus(status); + return status; }); ipcMain.handle(IPC.githubClearToken, async (): Promise => { const ctx = getCtx(); ctx.githubService.clearToken(); - return await ctx.githubService.getStatus(); + const status = await ctx.githubService.getStatus(); + broadcastGithubStatus(status); + return status; }); const resolveGithubRepoRef = async ( diff --git a/apps/desktop/src/main/services/lanes/laneService.test.ts b/apps/desktop/src/main/services/lanes/laneService.test.ts index 4c5e028bc..24079c1ee 100644 --- a/apps/desktop/src/main/services/lanes/laneService.test.ts +++ b/apps/desktop/src/main/services/lanes/laneService.test.ts @@ -2238,3 +2238,89 @@ describe("laneService missionId and laneRole", () => { expect(lane?.laneRole).toBe("integration"); }); }); + +describe("laneService updateAppearance color uniqueness", () => { + beforeEach(() => { + vi.mocked(getHeadSha).mockReset(); + vi.mocked(runGit).mockReset(); + vi.mocked(runGitOrThrow).mockReset(); + }); + + async function setup() { + const repoRoot = fs.mkdtempSync(path.join(os.tmpdir(), "ade-lane-color-uniqueness-")); + const db = await openKvDb(path.join(repoRoot, "kv.sqlite"), createLogger()); + await seedProjectAndStack(db, { projectId: "proj-color-uniqueness", repoRoot }); + const service = createLaneService({ + db, + projectRoot: repoRoot, + projectId: "proj-color-uniqueness", + defaultBaseRef: "main", + worktreesDir: path.join(repoRoot, "worktrees"), + }); + return { db, service }; + } + + it("rejects an update when another active lane already uses the color", async () => { + const { db, service } = await setup(); + db.run("update lanes set color = ? where id = ?", ["#a78bfa", "lane-parent"]); + + expect(() => + service.updateAppearance({ laneId: "lane-child", color: "#a78bfa" }), + ).toThrow(/already in use by lane "Parent"/); + + const child = db.get<{ color: string | null }>("select color from lanes where id = ?", ["lane-child"]); + expect(child?.color).toBeNull(); + }); + + it("treats hex comparison case-insensitively when detecting conflicts", async () => { + const { service, db } = await setup(); + db.run("update lanes set color = ? where id = ?", ["#a78bfa", "lane-parent"]); + + expect(() => + service.updateAppearance({ laneId: "lane-child", color: "#A78BFA" }), + ).toThrow(/already in use/i); + }); + + it("allows assigning a color that no other active lane is using", async () => { + const { db, service } = await setup(); + db.run("update lanes set color = ? where id = ?", ["#a78bfa", "lane-parent"]); + + service.updateAppearance({ laneId: "lane-child", color: "#60a5fa" }); + + const child = db.get<{ color: string | null }>("select color from lanes where id = ?", ["lane-child"]); + expect(child?.color).toBe("#60a5fa"); + }); + + it("ignores archived lanes when checking for color conflicts", async () => { + const { db, service } = await setup(); + db.run( + "update lanes set color = ?, status = 'archived', archived_at = ? where id = ?", + ["#a78bfa", "2026-04-01T00:00:00.000Z", "lane-parent"], + ); + + service.updateAppearance({ laneId: "lane-child", color: "#a78bfa" }); + + const child = db.get<{ color: string | null }>("select color from lanes where id = ?", ["lane-child"]); + expect(child?.color).toBe("#a78bfa"); + }); + + it("does not run the conflict probe when the color is unchanged (idempotent appearance updates)", async () => { + const { db, service } = await setup(); + // Two active lanes share the same stored color (e.g. legacy data before + // the uniqueness check existed). Updating one of them with the SAME color + // should succeed because the new value equals lane.color. + db.run("update lanes set color = ? where id = ?", ["#a78bfa", "lane-parent"]); + db.run("update lanes set color = ? where id = ?", ["#a78bfa", "lane-child"]); + + expect(() => + service.updateAppearance({ laneId: "lane-child", color: "#a78bfa", icon: "star" }), + ).not.toThrow(); + + const child = db.get<{ color: string | null; icon: string | null }>( + "select color, icon from lanes where id = ?", + ["lane-child"], + ); + expect(child?.color).toBe("#a78bfa"); + expect(child?.icon).toBe("star"); + }); +}); diff --git a/apps/desktop/src/main/services/lanes/laneService.ts b/apps/desktop/src/main/services/lanes/laneService.ts index e58bccd87..4e1054709 100644 --- a/apps/desktop/src/main/services/lanes/laneService.ts +++ b/apps/desktop/src/main/services/lanes/laneService.ts @@ -2880,6 +2880,21 @@ export function createLaneService({ const normalizedColor = color === undefined ? lane.color : color; const normalizedIcon = icon === undefined ? parseLaneIcon(lane.icon) : icon; + if (normalizedColor && normalizedColor !== lane.color) { + const conflict = db.get<{ name: string }>( + `select name from lanes + where project_id = ? + and id != ? + and archived_at is null + and lower(color) = lower(?) + limit 1`, + [projectId, laneId, normalizedColor] + ); + if (conflict) { + throw new Error(`Color already in use by lane "${conflict.name}"`); + } + } + db.run( ` update lanes diff --git a/apps/desktop/src/preload/global.d.ts b/apps/desktop/src/preload/global.d.ts index 7c90ffd07..c60f30f7e 100644 --- a/apps/desktop/src/preload/global.d.ts +++ b/apps/desktop/src/preload/global.d.ts @@ -1357,12 +1357,13 @@ declare global { onUpdate: (cb: (event: FeedbackSubmissionEvent) => void) => () => void; }; github: { - getStatus: () => Promise; + getStatus: (opts?: { forceRefresh?: boolean }) => Promise; setToken: (token: string) => Promise; clearToken: () => Promise; detectRepo: () => Promise<{ owner: string; name: string } | null>; listRepoLabels: (args: { owner: string; name: string }) => Promise>; listRepoCollaborators: (args: { owner: string; name: string }) => Promise>; + onStatusChanged: (cb: (status: GitHubStatus) => void) => () => void; }; prs: { createFromLane: (args: CreatePrFromLaneArgs) => Promise; diff --git a/apps/desktop/src/preload/preload.ts b/apps/desktop/src/preload/preload.ts index 246cdc2ea..10ba40d0c 100644 --- a/apps/desktop/src/preload/preload.ts +++ b/apps/desktop/src/preload/preload.ts @@ -1983,8 +1983,8 @@ contextBridge.exposeInMainWorld("ade", { }, }, github: { - getStatus: async (): Promise => - ipcRenderer.invoke(IPC.githubGetStatus), + getStatus: async (opts?: { forceRefresh?: boolean }): Promise => + ipcRenderer.invoke(IPC.githubGetStatus, opts ?? {}), setToken: async (token: string): Promise => ipcRenderer.invoke(IPC.githubSetToken, { token }), clearToken: async (): Promise => @@ -1997,6 +1997,12 @@ contextBridge.exposeInMainWorld("ade", { ipcRenderer.invoke(IPC.githubListRepoLabels, args), listRepoCollaborators: async (args: { owner: string; name: string }): Promise> => ipcRenderer.invoke(IPC.githubListRepoCollaborators, args), + onStatusChanged: (cb: (status: GitHubStatus) => void): (() => void) => { + const listener = (_event: Electron.IpcRendererEvent, payload: GitHubStatus) => + cb(payload); + ipcRenderer.on(IPC.githubStatusChanged, listener); + return () => ipcRenderer.removeListener(IPC.githubStatusChanged, listener); + }, }, prs: { createFromLane: async (args: CreatePrFromLaneArgs): Promise => diff --git a/apps/desktop/src/renderer/browserMock.ts b/apps/desktop/src/renderer/browserMock.ts index e4b3dacc4..af8269058 100644 --- a/apps/desktop/src/renderer/browserMock.ts +++ b/apps/desktop/src/renderer/browserMock.ts @@ -3056,12 +3056,49 @@ if (typeof window !== "undefined" && !(window as any).ade) { onUpdate: () => () => {}, }, github: { - getStatus: resolved({ authenticated: true, user: "arul" }), - setToken: resolvedArg({ authenticated: true, user: "arul" }), - clearToken: resolved({ authenticated: false, user: null }), + getStatus: resolved({ + tokenStored: true, + tokenDecryptionFailed: false, + storageScope: "app", + tokenType: "classic", + repo: { owner: "arul28", name: "ADE" }, + userLogin: "arul", + scopes: ["repo", "workflow"], + checkedAt: new Date().toISOString(), + repoAccessOk: true, + repoAccessError: null, + connected: true, + }), + setToken: resolvedArg({ + tokenStored: true, + tokenDecryptionFailed: false, + storageScope: "app", + tokenType: "classic", + repo: { owner: "arul28", name: "ADE" }, + userLogin: "arul", + scopes: ["repo", "workflow"], + checkedAt: new Date().toISOString(), + repoAccessOk: true, + repoAccessError: null, + connected: true, + }), + clearToken: resolved({ + tokenStored: false, + tokenDecryptionFailed: false, + storageScope: "app", + tokenType: "unknown", + repo: { owner: "arul28", name: "ADE" }, + userLogin: null, + scopes: [], + checkedAt: null, + repoAccessOk: null, + repoAccessError: null, + connected: false, + }), detectRepo: resolved({ owner: "arul28", name: "ADE" }), listRepoLabels: resolved([]), listRepoCollaborators: resolved([]), + onStatusChanged: noop, }, prs: { createFromLane: resolvedArg(NORMAL_PRS[0]), diff --git a/apps/desktop/src/renderer/components/app/AppShell.tsx b/apps/desktop/src/renderer/components/app/AppShell.tsx index 955a0f741..f3899b71d 100644 --- a/apps/desktop/src/renderer/components/app/AppShell.tsx +++ b/apps/desktop/src/renderer/components/app/AppShell.tsx @@ -21,6 +21,7 @@ import { type PrToastTone, } from "./prToastPresentation"; import { TabBackground } from "../ui/TabBackground"; +import { LaneAccentDot } from "../lanes/LaneAccentDot"; import { useAppStore } from "../../state/appStore"; import { useOnboardingStore } from "../../state/onboardingStore"; import { Button } from "../ui/Button"; @@ -138,6 +139,56 @@ function shortId(id: string): string { return trimmed.length <= 8 ? trimmed : trimmed.slice(0, 8); } +function describeGithubBanner(status: GitHubStatus): { message: string; linkLabel: string } { + if (!status.tokenStored) { + return { + message: "GitHub is not connected for this ADE app yet. ", + linkLabel: "Connect GitHub", + }; + } + if (status.tokenType === "fine-grained" && status.repoAccessOk === false) { + const repoLabel = status.repo ? `${status.repo.owner}/${status.repo.name}` : "this repo"; + return { + message: `GitHub token saved, but it cannot access ${repoLabel}. `, + linkLabel: "Fix GitHub token", + }; + } + return { + message: "GitHub token saved, but it does not have the required permissions. ", + linkLabel: "Fix GitHub token", + }; +} + +function GithubBanner({ + status, + onDismiss, +}: { + status: GitHubStatus; + onDismiss: () => void; +}): JSX.Element { + const { message, linkLabel } = describeGithubBanner(status); + return ( +
+ + {message} + + {linkLabel} + + + +
+ ); +} + function getPrToastToneClasses(tone: PrToastTone): { panel: string; badge: string; @@ -668,6 +719,17 @@ export function AppShell({ children }: { children: React.ReactNode }) { }; }, [project?.rootPath]); + // Refresh the GitHub banner the moment Settings saves/clears a token, so the + // shell does not lag behind the Settings UI (the original "banner stays up + // even though Settings says CONNECTED" bug). + useEffect(() => { + return ( + window.ade.github?.onStatusChanged?.((status) => { + setGithubStatus(status); + }) ?? (() => {}) + ); + }, []); + useEffect(() => { if (!window.ade.feedback?.onUpdate) return; const dispose = window.ade.feedback.onUpdate((event) => { @@ -936,29 +998,15 @@ export function AppShell({ children }: { children: React.ReactNode }) { !showWelcome && !isOnboardingRoute && githubStatus !== null && - !githubStatus.tokenStored && + !githubStatus.connected && !githubBannerDismissed ? ( -
- - GitHub is not connected for this ADE app yet.{" "} - - Connect GitHub - - - -
+ { + if (!currentProjectRoot) return; + dismissGithubBanner(currentProjectRoot); + }} + /> ) : null} {!hideSidebar && providerMode === "subscription" && aiMockProvider ? ( @@ -1093,9 +1141,9 @@ export function AppShell({ children }: { children: React.ReactNode }) { ) : null} {prToasts.map((toast) => { - const laneName = - lanes.find((lane) => lane.id === toast.event.laneId)?.name ?? - toast.event.laneId; + const toastLane = lanes.find((lane) => lane.id === toast.event.laneId) ?? null; + const laneName = toastLane?.name ?? toast.event.laneId; + const laneColor = toastLane?.color ?? null; const tone = getPrToastTone(toast.event.kind); const toneClasses = getPrToastToneClasses(tone); const Icon = getPrToastIcon(toast.event.kind); @@ -1155,23 +1203,29 @@ export function AppShell({ children }: { children: React.ReactNode }) { {meta.length > 0 ? (
- {meta.map((item, index) => ( - - {item.includes("/") ? ( - - ) : item.includes("#") || - (toast.event.repoOwner && - item.includes(toast.event.repoOwner)) ? ( - - ) : ( - - )} - {item} - - ))} + {meta.map((item, index) => { + const isLane = laneName != null && item === laneName; + return ( + + {isLane && laneColor ? ( + + ) : item.includes("/") ? ( + + ) : item.includes("#") || + (toast.event.repoOwner && + item.includes(toast.event.repoOwner)) ? ( + + ) : ( + + )} + {item} + + ); + })}
) : null}
diff --git a/apps/desktop/src/renderer/components/automations/ActionList.tsx b/apps/desktop/src/renderer/components/automations/ActionList.tsx index 151b73d98..1eff2a567 100644 --- a/apps/desktop/src/renderer/components/automations/ActionList.tsx +++ b/apps/desktop/src/renderer/components/automations/ActionList.tsx @@ -1,5 +1,6 @@ -import { useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { + CaretDown, Code, Lightning, Plus, @@ -10,7 +11,7 @@ import { } from "@phosphor-icons/react"; import type { ElementType } from "react"; import type { ModelConfig, TestSuiteDefinition } from "../../../shared/types"; -import { Button } from "../ui/Button"; +import { useClickOutside } from "../../hooks/useClickOutside"; import { cn } from "../ui/cn"; import { ActionRow, type ActionRowKind, type ActionRowValue } from "./ActionRow"; @@ -18,13 +19,61 @@ import { ActionRow, type ActionRowKind, type ActionRowValue } from "./ActionRow" // first-class EXECUTION setting (`laneMode: "create"`) rather than an action a // user has to chain manually. Legacy rules carrying a leading `create-lane` // action are migrated server-side on read. -const ADD_OPTIONS: Array<{ kind: ActionRowKind; label: string; icon: ElementType; disabled?: boolean; hint?: string }> = [ - { kind: "agent-session", label: "Agent session", icon: Lightning }, - { kind: "ade-action", label: "Run ADE action", icon: Code }, - { kind: "run-tests", label: "Run tests", icon: TestTube }, - { kind: "run-command", label: "Run command", icon: TerminalWindow }, - { kind: "predict-conflicts", label: "Predict conflicts", icon: Warning }, - { kind: "launch-mission", label: "Mission", icon: Rocket, disabled: true, hint: "Coming soon" }, +type AddOption = { + kind: ActionRowKind; + label: string; + icon: ElementType; + accent: string; + description: string; + disabled?: boolean; + hint?: string; +}; + +const ADD_OPTIONS: readonly AddOption[] = [ + { + kind: "agent-session", + label: "Agent session", + icon: Lightning, + accent: "#38BDF8", + description: "Send a prompt to an agent and let it work in a chat thread.", + }, + { + kind: "ade-action", + label: "ADE action", + icon: Code, + accent: "#A78BFA", + description: "Call any ADE CLI domain — git, lane, PR, tests, memory, and more.", + }, + { + kind: "run-tests", + label: "Run tests", + icon: TestTube, + accent: "#22C55E", + description: "Run a configured test suite and report the result.", + }, + { + kind: "run-command", + label: "Run command", + icon: TerminalWindow, + accent: "#F59E0B", + description: "Execute a shell command in the project workspace.", + }, + { + kind: "predict-conflicts", + label: "Predict conflicts", + icon: Warning, + accent: "#F97316", + description: "Run the conflict prediction pass against recent lanes.", + }, + { + kind: "launch-mission", + label: "Launch mission", + icon: Rocket, + accent: "#94A3B8", + description: "Spin up a multi-step mission for this rule.", + disabled: true, + hint: "Soon", + }, ]; function createBlankAction(kind: ActionRowKind, suites: TestSuiteDefinition[]): ActionRowValue { @@ -72,7 +121,6 @@ export function ActionList({ onChange: (next: ActionRowValue[]) => void; onOpenAiSettings?: () => void; }) { - const [menuOpen, setMenuOpen] = useState(false); // Stable per-row keys that survive reorders so React preserves focus/DOM // identity when the user clicks up/down arrows. Keys are regenerated only // when the row count changes (backfilling appended rows or trimming). @@ -84,13 +132,11 @@ export function ActionList({ } const addAction = (kind: ActionRowKind) => { - setMenuOpen(false); keysRef.current = [...keysRef.current, newKey()]; onChange([...actions, createBlankAction(kind, suites)]); }; const updateAction = (index: number, next: ActionRowValue) => { - // Key at `index` stays the same — only the value mutates. onChange(actions.map((action, i) => (i === index ? next : action))); }; @@ -110,13 +156,14 @@ export function ActionList({ onChange(nextActions); }; - return (
+ {/* Always-visible add bar at the TOP — its menu opens downward into the + surrounding column without ever being clipped by overflowing rows. */} + + {actions.length === 0 ? ( -
- No actions yet. Add at least one step below. -
+ ) : (
{actions.map((action, index) => ( @@ -136,45 +183,139 @@ export function ActionList({ ))}
)} +
+ ); +} + +function AddStepBar({ onAdd }: { onAdd: (kind: ActionRowKind) => void }) { + const [open, setOpen] = useState(false); + const wrapRef = useRef(null); + + useClickOutside(wrapRef, () => setOpen(false), open); + useEffect(() => { + if (!open) return; + const escape = (event: KeyboardEvent) => { + if (event.key === "Escape") setOpen(false); + }; + document.addEventListener("keydown", escape); + return () => document.removeEventListener("keydown", escape); + }, [open]); -
- - {menuOpen ? ( -
setMenuOpen(false)} - > - {ADD_OPTIONS.map((option) => { - const Icon = option.icon; - return ( - + + {open ? ( +
+ {ADD_OPTIONS.map((option) => { + const Icon = option.icon; + return ( + - ); - })} -
- ) : null} + + + + + {option.label} + {option.hint ? ( + + {option.hint} + + ) : null} + + + {option.description} + + + + ); + })} +
+ ) : null} +
+ ); +} + +function EmptyPicker({ onAdd }: { onAdd: (kind: ActionRowKind) => void }) { + return ( +
+
+ Pick a starting step +
+
+ {ADD_OPTIONS.filter((option) => !option.disabled).map((option) => { + const Icon = option.icon; + return ( + + ); + })}
); diff --git a/apps/desktop/src/renderer/components/automations/ActionRow.tsx b/apps/desktop/src/renderer/components/automations/ActionRow.tsx index b234c8d3d..e745310e7 100644 --- a/apps/desktop/src/renderer/components/automations/ActionRow.tsx +++ b/apps/desktop/src/renderer/components/automations/ActionRow.tsx @@ -2,7 +2,6 @@ import { ArrowDown, ArrowUp, Code, - Gear, GitBranch, Lightning, Rocket, @@ -59,11 +58,11 @@ export type ActionRowValue = { const KIND_META: Record = { "create-lane": { label: "Create lane", icon: GitBranch, accent: "#2DD4BF" }, "agent-session": { label: "Agent session", icon: Lightning, accent: "#38BDF8" }, - "ade-action": { label: "Run ADE action", icon: Code, accent: "#A78BFA" }, + "ade-action": { label: "ADE action", icon: Code, accent: "#A78BFA" }, "run-tests": { label: "Run tests", icon: TestTube, accent: "#22C55E" }, "run-command": { label: "Run command", icon: TerminalWindow, accent: "#F59E0B" }, "predict-conflicts": { label: "Predict conflicts", icon: Warning, accent: "#F97316" }, - "launch-mission": { label: "Mission", icon: Rocket, accent: "#94A3B8" }, + "launch-mission": { label: "Launch mission", icon: Rocket, accent: "#94A3B8" }, }; export function ActionRow({ @@ -99,15 +98,28 @@ export function ActionRow({ return (
-
+
- - - {index + 1}. {meta.label} + + + + + STEP {index + 1} + {meta.label} {value.kind === "create-lane" ? ( legacy · now in Execution ) : null} @@ -115,37 +127,37 @@ export function ActionRow({ custom model ) : null}
-
+
-
+
{value.kind === "create-lane" ? ( // Read-only legacy view. Lane creation is now an EXECUTION setting, // but old rules still on disk render their stored template here so @@ -249,8 +261,8 @@ export function ActionRow({ ) : null} {value.kind === "run-tests" ? ( -
+ onChange({ ...value, domain: next.domain, action: next.action })} + /> {registryError ? (
- {registryError} Picker will show free-text only; you can still save with a manual domain/action. + {registryError} You can still type a domain and action manually.
) : null} -
-
- Arguments (JSON) -
- {TRIGGER_PLACEHOLDERS.map((placeholder) => ( + {value.domain && value.action ? ( + setShowJson((current) => !current)} + /> + ) : ( +
+ Pick a domain and action to fill in its parameters. +
+ )} +
+ ); +} + +// --- Domain + action picker (searchable combobox) --- + +function ActionPicker({ + value, + registry, + onChange, +}: { + value: AdeActionValue; + registry: AdeActionRegistryEntry[]; + onChange: (next: { domain: string; action: string }) => void; +}) { + const [open, setOpen] = useState(false); + const [search, setSearch] = useState(""); + const wrapRef = useRef(null); + + useClickOutside(wrapRef, () => setOpen(false), open); + useEffect(() => { + if (!open) return; + const escape = (event: KeyboardEvent) => { + if (event.key === "Escape") setOpen(false); + }; + document.addEventListener("keydown", escape); + return () => document.removeEventListener("keydown", escape); + }, [open]); + + const matches = useMemo(() => { + const query = search.trim().toLowerCase(); + const items: Array<{ + domain: string; + action: string; + label: string; + description: string; + }> = []; + for (const entry of registry) { + for (const action of entry.actions) { + const schema = findAdeActionSchema(entry.domain, action.name); + items.push({ + domain: entry.domain, + action: action.name, + label: schema?.label ?? action.name, + description: schema?.description ?? action.description ?? "", + }); + } + } + if (!query) return items.slice(0, 200); + return items + .filter((item) => { + const haystack = `${item.domain} ${item.action} ${item.label} ${item.description}`.toLowerCase(); + return haystack.includes(query); + }) + .slice(0, 200); + }, [registry, search]); + + const grouped = useMemo(() => { + const map = new Map(); + for (const item of matches) { + const list = map.get(item.domain) ?? []; + list.push(item); + map.set(item.domain, list); + } + return [...map.entries()].sort(([a], [b]) => a.localeCompare(b)); + }, [matches]); + + const currentSchema = findAdeActionSchema(value.domain, value.action); + const currentLabel = currentSchema?.label ?? value.action; + + const summary = value.domain && value.action ? ( + + + {currentLabel} + + {value.domain}.{value.action} + + + ) : ( + + + Search ADE actions… + + ); + + return ( +
+ + + {open ? ( +
+
+ + setSearch(event.target.value)} + placeholder="Search by domain, action, or description" + className="flex-1 bg-transparent text-[12px] text-[#F5FAFF] placeholder:text-[#7E8A9A] focus:outline-none" + /> + {search ? ( + ) : null} +
+
+ {grouped.length === 0 ? ( +
+ No actions match "{search}". +
+ ) : ( + grouped.map(([domain, items]) => ( +
+
+ {domain} +
+ {items.map((item) => { + const active = item.domain === value.domain && item.action === value.action; + return ( + + ); + })} +
+ )) + )} +
+
+ ) : null} +
+ ); +} + +// --- Per-action parameter editor --- + +function ActionParamsEditor({ + schema, + value, + onChange, + showJson, + onToggleJson, +}: { + schema: AdeActionSchema | undefined; + value: AdeActionValue; + onChange: (next: AdeActionValue) => void; + showJson: boolean; + onToggleJson: () => void; +}) { + const args = (value.args && typeof value.args === "object" && !Array.isArray(value.args)) + ? (value.args as Record) + : {}; + + const setArg = (name: string, next: unknown) => { + const nextArgs = { ...args }; + if (next === undefined || next === "" || next === null) { + delete nextArgs[name]; + } else { + nextArgs[name] = next; + } + onChange({ + ...value, + args: Object.keys(nextArgs).length === 0 ? undefined : nextArgs, + }); + }; + + return ( +
+ {schema && schema.params.length > 0 ? ( + <> +
+
+ + Parameters +
+ +
+
+ {schema.params.map((param) => ( + setArg(param.name, next)} + /> ))}
+ + ) : ( +
+ + {schema?.description ?? "No structured form for this action yet."} + + Pass arguments as JSON below. +
+ )} + + + + {(showJson || !schema || schema.params.length === 0) ? ( + + ) : null} + + {schema ? ( +
+ + Maps to ade {value.domain} {value.action}
+ ) : null} +
+ ); +} + +function ParamField({ + param, + value, + onChange, +}: { + param: AdeActionParam; + value: unknown; + onChange: (next: unknown) => void; +}) { + const labelEl = ( +
+ + {param.name} + {param.required ? * : null} + + {param.description ? ( + + {param.description} + + ) : null} +
+ ); + + if (param.type === "boolean") { + const checked = value === true; + return ( + + ); + } + + if (param.type === "enum" && param.enumValues) { + const current = typeof value === "string" ? value : (param.defaultValue as string | undefined) ?? ""; + return ( + + ); + } + + if (param.type === "number") { + const current = typeof value === "number" ? String(value) : ""; + return ( + + ); + } + + if (param.type === "string-array") { + const current = Array.isArray(value) ? (value as string[]).join(", ") : ""; + return ( + + ); + } + + if (param.type === "json") { + const current = typeof value === "string" + ? value + : value === undefined + ? "" + : JSON.stringify(value, null, 2); + return ( +