Skip to content
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ describe("prService.getMobileSnapshot", () => {
const eligibleEntry = snapshot.createCapabilities.lanes.find((lane) => lane.laneId === "lane-feat")!;
expect(eligibleEntry.canCreate).toBe(true);
expect(eligibleEntry.blockedReason).toBeNull();
expect(eligibleEntry.commitsAheadOfBase).toBe(0);
});

it("includes queue and rebase workflow cards and skips completed queues", async () => {
Expand Down
2 changes: 2 additions & 0 deletions apps/desktop/src/main/services/prs/prService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5510,6 +5510,7 @@ export function createPrService({
primaryBranchRef: primaryLane?.branchRef ?? null,
});
const dirty = lane.status?.dirty === true;
const commitsAheadOfBase = Math.max(0, Number(lane.status?.ahead ?? 0) || 0);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[🟡 Medium] [🔵 Bug]

This field is documented and displayed as "ahead of defaultBaseBranch", but the new value is taken from lane.status.ahead, which is not guaranteed to be measured against that branch. The changed code does:

// apps/desktop/src/main/services/prs/prService.ts
const defaultBaseBranch = resolveStableLaneBaseBranch({
  lane,
  parent,
  primaryBranchRef: primaryLane?.branchRef ?? null,
});
const dirty = lane.status?.dirty === true;
const commitsAheadOfBase = Math.max(0, Number(lane.status?.ahead ?? 0) || 0);

I verified computeLaneStatus() in @apps/desktop/src/main/services/lanes/laneService.ts computes ahead from git rev-list ${baseRef}...${branchRef}, while resolveStableLaneBaseBranch() in @apps/desktop/src/shared/laneBaseResolution.ts intentionally switches child lanes with non-primary parents to the parent branch. For stacked lanes where lane.baseRef is still the stored/base branch, mobile will report commits ahead of the parent while actually counting commits ahead of the older baseRef, overstating the hint by including parent commits. Compute this count against the same branch returned by resolveStableLaneBaseBranch, or rename the field/message so it matches the underlying metric.

const hasExistingPr = existingPr !== null && (existingPr.state === "open" || existingPr.state === "draft");
const canCreate = !hasExistingPr;
const blockedReason = hasExistingPr
Expand All @@ -5524,6 +5525,7 @@ export function createPrService({
defaultBaseBranch,
defaultTitle: lane.name,
dirty,
commitsAheadOfBase,
hasExistingPr,
canCreate,
blockedReason,
Expand Down
38 changes: 38 additions & 0 deletions apps/desktop/src/renderer/components/app/AppShell.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,44 @@ describe("AppShell", () => {
expect(
screen.getByText(/No AI provider is configured yet/i),
).toBeTruthy();

fireEvent.click(screen.getByTestId("dismiss-missing-ai-banner"));

expect(
screen.queryByText(/No AI provider is configured yet/i),
).toBeNull();
} finally {
vi.useRealTimers();
}
});

it("dismisses the GitHub not connected banner for the current session", async () => {
vi.useFakeTimers();
try {
globalThis.window.ade.github.getStatus = vi.fn(async () => ({ tokenStored: false })) as any;

render(
<MemoryRouter initialEntries={["/work"]}>
<AppShell>
<div>child</div>
</AppShell>
</MemoryRouter>,
);

await act(async () => {
vi.advanceTimersByTime(1_000);
await Promise.resolve();
});

expect(
screen.getByText(/GitHub is not connected for this ADE app yet/i),
).toBeTruthy();

fireEvent.click(screen.getByTestId("dismiss-github-banner"));

expect(
screen.queryByText(/GitHub is not connected for this ADE app yet/i),
).toBeNull();
} finally {
vi.useRealTimers();
}
Expand Down
73 changes: 63 additions & 10 deletions apps/desktop/src/renderer/components/app/AppShell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,12 @@ export function AppShell({ children }: { children: React.ReactNode }) {
);
const [dismissedContextBannerRoots, setDismissedContextBannerRoots] =
useState<Record<string, true>>({});
/** Session dismiss for the “no AI provider” banner (per project root). */
const [dismissedMissingAiBannerRoots, setDismissedMissingAiBannerRoots] =
useState<Record<string, true>>({});
/** Session dismiss for the “GitHub not connected” banner (per project root). */
const [dismissedGithubBannerRoots, setDismissedGithubBannerRoots] =
useState<Record<string, true>>({});
const [projectMissing, setProjectMissing] = useState(false);
const [feedbackGenerating, setFeedbackGenerating] = useState(false);
const previousProjectRootRef = useRef<string | null | undefined>(undefined);
Expand Down Expand Up @@ -480,6 +486,11 @@ export function AppShell({ children }: { children: React.ReactNode }) {
setProjectMissing(false);
}, [project?.rootPath]);

useEffect(() => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[🟡 Medium] [🔵 Bug]

// apps/desktop/src/renderer/components/app/AppShell.tsx
useEffect(() => {
  setDismissedMissingAiBannerRoots({});
  setDismissedGithubBannerRoots({});
}, [project?.rootPath]);

This effect clears all dismissed-banner entries every time the active project.rootPath changes, so dismissing either banner in project A is forgotten as soon as the user opens project B. Because AppShell stays mounted across project changes and the state is already keyed by rootPath, switching back to project A makes the banner reappear even though the UI promises “Dismiss for this session.” Remove this reset effect (matching the existing dismissedContextBannerRoots behavior) or otherwise preserve per-root entries until the app session actually ends.

setDismissedMissingAiBannerRoots({});
setDismissedGithubBannerRoots({});
}, [project?.rootPath]);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

useEffect(() => {
const previousProjectRoot = previousProjectRootRef.current;
const nextProjectRoot = project?.rootPath ?? null;
Expand Down Expand Up @@ -603,6 +614,12 @@ export function AppShell({ children }: { children: React.ReactNode }) {
[missingContextDocs],
);
const currentProjectRoot = project?.rootPath ?? null;
const missingAiBannerDismissed = Boolean(
currentProjectRoot && dismissedMissingAiBannerRoots[currentProjectRoot],
);
const githubBannerDismissed = Boolean(
currentProjectRoot && dismissedGithubBannerRoots[currentProjectRoot],
);
const contextBannerDismissed = Boolean(
currentProjectRoot && dismissedContextBannerRoots[currentProjectRoot],
);
Expand Down Expand Up @@ -791,12 +808,30 @@ export function AppShell({ children }: { children: React.ReactNode }) {
!showWelcome &&
aiStatusLoaded &&
aiStatus !== null &&
!hasAnyAiProvider ? (
!hasAnyAiProvider &&
!missingAiBannerDismissed ? (
<div className="shrink-0 mx-2 mt-1 rounded bg-amber-500/6 px-3 py-1.5 text-[11px] font-mono text-amber-800">
No AI provider is configured yet.{" "}
<Link to="/settings?tab=ai" className="underline">
Set up AI
</Link>
<span>
No AI provider is configured yet.{" "}
<Link to="/settings?tab=ai" className="underline">
Set up AI
</Link>
</span>
<button
type="button"
data-testid="dismiss-missing-ai-banner"
className="ml-2 text-amber-900/70 hover:text-amber-900"
onClick={() => {
if (!currentProjectRoot) return;
setDismissedMissingAiBannerRoots((prev) => ({
...prev,
[currentProjectRoot]: true,
}));
}}
title="Dismiss for this session"
>
×
</button>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
</div>
) : null}

Expand All @@ -805,12 +840,30 @@ export function AppShell({ children }: { children: React.ReactNode }) {
!showWelcome &&
!isOnboardingRoute &&
githubStatus !== null &&
!githubStatus.tokenStored ? (
!githubStatus.tokenStored &&
!githubBannerDismissed ? (
<div className="shrink-0 mx-3 mt-1.5 rounded bg-amber-500/6 px-3 py-1.5 text-[11px] font-mono text-amber-800">
GitHub is not connected for this ADE app yet.{" "}
<Link to="/settings?tab=integrations" className="underline">
Connect GitHub
</Link>
<span>
GitHub is not connected for this ADE app yet.{" "}
<Link to="/settings?tab=integrations" className="underline">
Connect GitHub
</Link>
</span>
<button
type="button"
data-testid="dismiss-github-banner"
className="ml-2 text-amber-900/70 hover:text-amber-900"
onClick={() => {
if (!currentProjectRoot) return;
setDismissedGithubBannerRoots((prev) => ({
...prev,
[currentProjectRoot]: true,
}));
}}
title="Dismiss for this session"
>
×
</button>
</div>
) : null}

Expand Down
38 changes: 38 additions & 0 deletions apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ import { ClaudeCacheTtlBadge } from "../shared/ClaudeCacheTtlBadge";
import { shouldShowClaudeCacheTtl } from "../../lib/claudeCacheTtl";
import { getAgentChatModelsCached, getAiStatusCached } from "../../lib/aiDiscoveryCache";
import { invalidateSessionListCache } from "../../lib/sessionListCache";
import { playAgentTurnCompletionSound } from "../../lib/agentTurnCompletionSound";

const LAST_MODEL_ID_KEY = "ade.chat.lastModelId";
const LAST_REASONING_KEY_PREFIX = "ade.chat.lastReasoningEffort";
Expand Down Expand Up @@ -710,6 +711,7 @@ export function AgentChatPane({
onLaneChange?: (laneId: string) => void;
}) {
const projectRoot = useAppStore((s) => s.project?.rootPath ?? null);
const agentTurnCompletionSound = useAppStore((s) => s.agentTurnCompletionSound);
const navigate = useNavigate();
const openAiProvidersSettings = useCallback(() => {
navigate("/settings?tab=ai#ai-providers");
Expand Down Expand Up @@ -777,6 +779,8 @@ export function AgentChatPane({
const shellRef = useRef<HTMLElement | null>(null);
const composerMaxHeightPx = layoutVariant === "grid-tile" ? 144 : null;
const sessionsRef = useRef<AgentChatSessionSummary[]>(sessions);
const completionSoundPrevTurnActiveRef = useRef(false);
const completionSoundArmedRef = useRef(true);

const appliedInitialSessionIdRef = useRef<string | null>(initialSessionId ?? null);
const loadedHistoryRef = useRef<Set<string>>(new Set());
Expand Down Expand Up @@ -824,6 +828,40 @@ export function AgentChatPane({
const pendingInput = selectedSessionId ? (pendingInputsBySession[selectedSessionId]?.[0] ?? null) : null;
const selectedSessionAwaitingInput = Boolean(pendingInput) || selectedSession?.awaitingInput === true;
const turnActive = selectedSessionId ? (turnActiveBySession[selectedSessionId] ?? false) : false;

useEffect(() => {
completionSoundPrevTurnActiveRef.current = false;
completionSoundArmedRef.current = true;
}, [selectedSessionId]);

useEffect(() => {
if (agentTurnCompletionSound === "off") {
completionSoundPrevTurnActiveRef.current = turnActive;
return;
}
if (turnActive) {
completionSoundArmedRef.current = true;
}
const sessionEnded = selectedSession?.status === "ended";
const settled =

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[🟡 Medium] [🔵 Bug]

The new completion check treats any transition from turnActive=true to false as a successful finish as long as the session is not ended and not awaiting input:

// apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
const settled =
  Boolean(selectedSessionId)
  && !selectedSessionAwaitingInput
  && !sessionEnded;
const becameIdle = settled && prevTurn && !turnActive;

This is too broad. I verified in @apps/desktop/src/renderer/components/chat/AgentChatPane.tsx:216-250 that deriveRuntimeState() clears turnActive for every done event, and in @apps/desktop/src/main/services/chat/agentChatService.ts:5178-5199 plus the interrupt/failure paths around 7913-7919, 7966-8003, and 12493-12498 that interrupted/failed turns are left in status = "idle", not ended. That means cancelling a run or hitting a provider error satisfies settled and still calls playAgentTurnCompletionSound(...), which contradicts the setting copy in @apps/desktop/src/renderer/components/settings/AppearanceSection.tsx that says the sound plays when the assistant "finishes a turn". Gate the sound on a verified successful turn outcome (for example the latest done.status === "completed" / status.turnStatus === "completed") instead of any idle transition.

Boolean(selectedSessionId)
&& !selectedSessionAwaitingInput
&& !sessionEnded;
const prevTurn = completionSoundPrevTurnActiveRef.current;
const becameIdle = settled && prevTurn && !turnActive;
completionSoundPrevTurnActiveRef.current = turnActive;
if (becameIdle && completionSoundArmedRef.current) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[🟡 Medium] [🔵 Bug]

The new idle-transition effect treats any turnActivefalse transition as a successful completion and plays the notification immediately:

// apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
const prevTurn = completionSoundPrevTurnActiveRef.current;
const becameIdle = settled && prevTurn && !turnActive;
completionSoundPrevTurnActiveRef.current = turnActive;
if (becameIdle && completionSoundArmedRef.current) {
  completionSoundArmedRef.current = false;
  playAgentTurnCompletionSound(agentTurnCompletionSound);
}

That also matches the existing interrupt() path in this same component, which optimistically does setTurnActiveBySession((prev) => ({ ...prev, [selectedSessionId]: false })) before awaiting the backend interrupt. In practice, clicking Stop will satisfy prevTurn && !turnActive and emit the new "completion" sound for an aborted turn. Suppress the next notification when the stop path initiated the transition, or gate playback on a confirmed non-interrupted terminal event instead of any idle transition.

completionSoundArmedRef.current = false;
playAgentTurnCompletionSound(agentTurnCompletionSound);
}
}, [
agentTurnCompletionSound,
selectedSessionId,
selectedSession?.status,
selectedSessionAwaitingInput,
turnActive,
]);

const activeProviderConnection = selectedSession?.provider === "claude"
? (providerConnections?.claude ?? null)
: selectedSession?.provider === "codex"
Expand Down
42 changes: 33 additions & 9 deletions apps/desktop/src/renderer/components/chat/CodeHighlighter.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, { Suspense, useCallback, useEffect, useState, useRef } from "react";
import { CopySimple, Checks } from "@phosphor-icons/react";
import { useAppStore, type CodeBlockCopyButtonPosition } from "../../state/appStore";

/* ── LRU cache for highlighted HTML ── */

Expand Down Expand Up @@ -113,25 +114,47 @@ function DiffCodeBlock({ code }: { code: string }) {

/* ── Copy button ── */

function CodeCopyButton({ code }: { code: string }) {
function copyTextToClipboard(text: string): Promise<boolean> {
if (typeof navigator !== "undefined" && navigator.clipboard?.writeText) {
return navigator.clipboard.writeText(text).then(() => true).catch(() => false);
}
try {
const ta = document.createElement("textarea");
ta.value = text;
ta.setAttribute("readonly", "");
ta.style.position = "fixed";
ta.style.left = "-9999px";
document.body.appendChild(ta);
ta.select();
const ok = document.execCommand("copy");
document.body.removeChild(ta);
return Promise.resolve(ok);
Comment on lines +130 to +133

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[🟡 Medium] [🔵 Bug]

The new fallback branch appends a hidden <textarea> to document.body, but it only removes that node on the success path. If ta.select() or document.execCommand("copy") throws, control jumps to the outer catch and the appended element stays in the DOM forever, so repeated failed copy attempts in the unsupported browsers this fallback targets will accumulate hidden nodes. Move the cleanup into a finally so the temporary textarea is always removed.

// apps/desktop/src/renderer/components/chat/CodeHighlighter.tsx
    document.body.appendChild(ta);
    ta.select();
    const ok = document.execCommand("copy");
    document.body.removeChild(ta);
    return Promise.resolve(ok);
Suggested change
document.body.appendChild(ta);
ta.select();
const ok = document.execCommand("copy");
document.body.removeChild(ta);
return Promise.resolve(ok);
document.body.appendChild(ta);
try {
ta.select();
return Promise.resolve(document.execCommand("copy"));
} finally {
ta.remove();
}

} catch {
return Promise.resolve(false);
}
}

function CodeCopyButton({ code, position }: { code: string; position: CodeBlockCopyButtonPosition }) {
const [copied, setCopied] = useState(false);

const handleCopy = useCallback(() => {
if (typeof navigator === "undefined" || !navigator.clipboard?.writeText) return;
void navigator.clipboard.writeText(code)
.then(() => {
void copyTextToClipboard(code)
.then((ok) => {
if (!ok) {
setCopied(false);
return;
}
setCopied(true);
window.setTimeout(() => setCopied(false), 1_500);
})
.catch(() => {
setCopied(false);
});
}, [code]);

const posClass = position === "bottom" ? "bottom-2 top-auto" : "top-2";

return (
<button
type="button"
className="absolute right-2 top-2 z-10 inline-flex items-center gap-1 rounded-md border border-white/[0.08] bg-white/[0.03] px-1.5 py-0.5 font-sans text-[9px] text-fg/45 opacity-0 backdrop-blur-sm transition-all group-hover:opacity-100 hover:border-white/[0.14] hover:bg-white/[0.05] hover:text-fg/72"
className={`absolute right-2 z-10 inline-flex items-center gap-1 rounded-md border border-white/[0.08] bg-white/[0.03] px-1.5 py-0.5 font-sans text-[9px] text-fg/45 opacity-0 backdrop-blur-sm transition-all group-hover:opacity-100 [@media(hover:none)]:opacity-100 hover:border-white/[0.14] hover:bg-white/[0.05] hover:text-fg/72 ${posClass}`}
onClick={handleCopy}
title={copied ? "Copied" : "Copy code"}
aria-label={copied ? "Copied" : "Copy code"}
Expand Down Expand Up @@ -223,12 +246,13 @@ export const HighlightedCode = React.memo(function HighlightedCode({
code: string;
language: string;
}) {
const copyButtonPosition = useAppStore((s) => s.codeBlockCopyButtonPosition);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[🟡 Medium] [🔵 Bug]

This new store read only changes HighlightedCode, but semantic reference tracing shows HighlightedCode is currently used only by @apps/desktop/src/renderer/components/prs/shared/PrMarkdown.tsx:464; the actual chat renderer in @apps/desktop/src/renderer/components/chat/AgentChatMessageList.tsx:597-623 still renders fenced blocks with plain <pre>/<code> and never consults codeBlockCopyButtonPosition. That means the new Appearance setting labeled for chat code blocks has no effect where users expect it, and instead changes PR review markdown. Route chat fenced blocks through the shared highlighter/copy-button component or apply the same positioning logic in MarkdownBlock so the preference actually controls chat code blocks.

// apps/desktop/src/renderer/components/chat/CodeHighlighter.tsx
export const HighlightedCode = React.memo(function HighlightedCode({
  code,
  language,
}) {
  const copyButtonPosition = useAppStore((s) => s.codeBlockCopyButtonPosition);

const trimmedCode = code.replace(/\n$/, "");
const isDiff = language === "diff";

return (
<div className="group relative my-3 overflow-hidden rounded-[10px] border border-[color:var(--chat-code-border)] bg-[var(--chat-code-bg)]">
<CodeCopyButton code={trimmedCode} />
<CodeCopyButton code={trimmedCode} position={copyButtonPosition} />
<div className="overflow-x-auto whitespace-pre-wrap break-words px-4 py-3">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[🟡 Medium] [🔵 Bug]

In auto mode the button is rendered as position: sticky, but it is nested inside the same wrapper that still has overflow-x-auto, so CSS sticky binds to that nearest overflow ancestor instead of the transcript pane. MDN documents that any ancestor with overflow: auto/hidden/scroll becomes the sticky container even when it is not the element actually scrolling vertically, which means the new “tracks the viewport as you scroll” option will stay pinned within the code block rather than following chat scroll. Move the sticky row outside the horizontal overflow element, or split the horizontal scroller into a child wrapper under the sticky row. ```tsx
// apps/desktop/src/renderer/components/chat/CodeHighlighter.tsx

{copyButtonPosition === "auto" && ( )} ```

{isDiff ? (
<DiffCodeBlock code={trimmedCode} />
Expand Down
Loading
Loading