Skip to content

Commit 2b41ae6

Browse files
arul28claude
andauthored
Mobile Ui Fixes (#212)
* Add mobile project icons and work controls * Update ship-lane force-finalize guidance * ship: iter 1 — address PR #212 review (greptile P2 + coderabbit + capy + copilot) Greptile P2: - registerIpc: validate projectResolveIcon rootPath via resolveAllowedRendererPath - TopBar: bound projectIconCache with LRU(24); honor disabled before cache lookup - ChatAttachmentTray: focus-trap lightbox (focus close on mount, Tab cycle, restore on unmount, Escape to close) CodeRabbit: - registerIpc: return realpath from resolveAllowedRendererPath (TOCTOU defense); switch image data URL handler to async fs.promises with size pre-check - ptyService: restore Codex storage fallback for session-list hydration (regression flagged); only resume-launch keeps the live-capture exclusion - App.tsx: gate PersistentWorkSurface reveal on project-hydrated state, not just `active` - TopBar: don't reuse cached icons for newly-disabled projects - ChatAttachmentTray: split attachmentName on /\\ for Windows paths; aria-label on file remove button - browserMock: add project.resolveIcon mock - cliLaunch/useLaneWorkSessions/useWorkSessions: extract resolveLaunchFields() so caller overrides are atomic — defaults only fire when no field supplied - SyncService: refresh active-session snapshot when chatSummary cache mutates; fall back to `now` for running sessions with no activity timestamp - LaneAdvancedScreen: deny branch-switch for any mission lane (incl. nil role) - LaneCommitSheet: TODO + broader needles for AI-setup error detection (proper structured error code is a follow-up) - LaneDetailContentSections: Pull/Fetch label + a11y reflect actual action - LaneDetailScreen: drop divider when footer renders no content - ADELiveActivityViews: trim modelId before isEmpty check - ADETests: shortcut-vs-saved-host test uses fresh discovery timestamp capy-ai (High Bug): - SyncService: long-lived running sessions no longer fall through the recency cutoff via stale `started` fallback Copilot (selected, high-impact): - ptyService: shrink readFilePrefix scan to 16 KB when scanning for required marker (session meta is at the top) - LaneCommitSheet: restore .onAppear focus on commit message field Skipped: BrandMark retina assets (no source); Copilot ANSI-renderer limitations (known/experimental, not regressions); Copilot design nits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ship: iter 2 — Copilot followup - shell.ts: ANSI-C quote ($'...') args with embedded newlines/tabs/etc so the PTY shell-fallback doesn't fire PS2 continuation prompts mid-startup-command. Adds tests covering the codex preamble case. - ChatAttachmentTray: lock body scroll while ImageLightbox is open so the wheel doesn't move content behind the overlay; restore on close. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ship: iter 3 — address PR #212 iter-2 review Greptile P1 / security: - projectIconResolver: use realpath-based resolvePathWithinRoot for both the source-file scan and the candidate-icon stat path so a `public -> /etc` symlink in a checked-out repo can't escape the project root. Defensive try/catch on stat/read returns "no icon" rather than crashing. - Added a regression test that symlinks `<root>/public` to an outside dir and asserts no icon is returned. CodeRabbit Critical: - registerIpc.appGetImageDataUrl no longer derives MIME from the path extension. Magic-byte sniffer (PNG, JPEG, GIF87/89, WebP, BMP, SVG with optional <?xml prelude / BOM / leading whitespace) plus stat preflight is the only path now; non-images throw "Path is not an image." CodeRabbit Major: - registerIpc.appWriteClipboardImage now runs the same readImageFileAndSniffMime preflight before nativeImage.createFromPath, so the renderer can't make the main process sync-read an arbitrarily large or non-image file. - App.tsx PersistentWorkSurface inert effect deps extended to [active, projectHydrated, hasActiveProject, showWelcome] so the effect re-runs once the early-return guards settle and the workSurfaceRef is a real node (fix for first cold /work mount). CodeRabbit Minor: - ChatAttachmentTray chip remove button: title now uses attachmentName(...) to match aria-label, so sighted hover and AT both see the basename. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ship: iter 4 — Copilot iter-3 followup - sniffImageMimeType: recognize ICO (00 00 01 00) so .ico chat attachments render their preview instead of failing through to "Path is not an image." - appWriteClipboardImage: hand the already-read buffer from readImageFileAndSniffMime to nativeImage.createFromBuffer so the file isn't re-read off the main thread (avoids a 10 MB sync read pair for the largest allowed images). Skipped: resolveLaunchFields permission-mode injection — verified via git history that this was a deliberate part of the lane's main feature commit (9d35235), not a review-loop regression. LiveActivityCoordinator focused-lane flicker — pre-existing behavior, out of scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ship: iter 5 — bump aiOrchestratorService overlap test budget for CI flake The "does not enter a run body while another health sweep owns it" test polls reconcileCalls in a 200 * 25ms = 5s window before asserting it reached the gated onTrackedSessionEnded callback. That window has flaked twice on the test-desktop(8) shard for this PR's pushes (and once on the shard rerun after the same failure) while passing 3/3 locally — the gated callback simply doesn't fire within 5s on a heavily loaded runner. Bump the wait window to 600 * 25ms = 15s and raise the test timeout from 15s to 30s. Test exits immediately when the callback fires, so the larger ceiling only kicks in for slow runners. No semantic change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * ship: iter 6 (force-finalize) — fix iter-1 projectResolveIcon regression Iter 1's "validate rootPath against allowlist" fix used the generic file allowlist (project root + Downloads/Documents/Temp), which broke tab/catalog icon resolution for any project living outside those folders (e.g. ~/code/*). Switch to a project-root allowlist sourced from globalState.recentProjects + current project root, so the access boundary is the legitimate set of known projects. CI stays green; Greptile + CodeRabbit + Copilot all reported terminal on iter 5. This pass addresses the one new diff-line comment (coderabbitai #3146852140) inline because it's a clear regression I introduced earlier in the loop. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1418b0c commit 2b41ae6

70 files changed

Lines changed: 3248 additions & 1243 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/scheduled_tasks.lock

Lines changed: 0 additions & 1 deletion
This file was deleted.

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ node_modules
1010
*.png
1111
!docs/**/*.png
1212
!apps/web/public/**/*.png
13+
!apps/ios/**/*.png
1314

1415
# Python cache
1516
__pycache__/
@@ -45,6 +46,7 @@ release-stable/
4546
# Xcode user data & derived data
4647
xcuserdata/
4748
*.xcuserstate
49+
/.derived-data/
4850
apps/ios/.dry-run-derived-data/
4951
apps/ios/build/
5052
ios-signing/

apps/ade-cli/package-lock.json

Lines changed: 2 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

apps/desktop/src/main/main.ts

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
upsertProjectRow,
5151
} from "./services/projects/projectService";
5252
import { inspectRecentProject, type RecentProjectInspection } from "./services/projects/recentProjectSummary";
53+
import { resolveProjectIcon } from "./services/projects/projectIconResolver";
5354
import { createAdeProjectService } from "./services/projects/adeProjectService";
5455
import { createConfigReloadService } from "./services/projects/configReloadService";
5556
import { IPC } from "../shared/ipc";
@@ -955,6 +956,10 @@ app.whenReady().then(async () => {
955956
});
956957
if (activeProjectRoot) {
957958
projectLastActivatedAt.set(activeProjectRoot, Date.now());
959+
const activeCtx = projectContexts.get(activeProjectRoot);
960+
if (activeCtx) {
961+
persistRecentProject(activeCtx.project, { recordLastProject: false });
962+
}
958963
try {
959964
adeArtifactAllowedDir =
960965
resolveAdeLayout(activeProjectRoot).artifactsDir;
@@ -3938,6 +3943,7 @@ app.whenReady().then(async () => {
39383943
rootPath: ctx.project.rootPath,
39393944
defaultBaseRef: ctx.project.baseRef,
39403945
lastOpenedAt: recent?.summary.lastOpenedAt ?? null,
3946+
iconDataUrl: mobileProjectIconDataUrl(ctx.project.rootPath),
39413947
laneCount,
39423948
isAvailable: fs.existsSync(ctx.project.rootPath),
39433949
isCached: false,
@@ -3953,13 +3959,30 @@ app.whenReady().then(async () => {
39533959
rootPath: recent.summary.rootPath,
39543960
defaultBaseRef: recent.defaultBaseRef,
39553961
lastOpenedAt: recent.summary.lastOpenedAt,
3962+
iconDataUrl: mobileProjectIconDataUrl(recent.summary.rootPath),
39563963
laneCount: recent.summary.laneCount ?? 0,
39573964
isAvailable: recent.summary.exists,
39583965
isCached: false,
39593966
isOpen: false,
39603967
};
39613968
}
39623969

3970+
function mobileProjectIconDataUrl(projectRoot: string): string | null {
3971+
try {
3972+
const icon = resolveProjectIcon(projectRoot);
3973+
if (!icon.sourcePath) return null;
3974+
3975+
const image = nativeImage.createFromPath(icon.sourcePath);
3976+
if (!image.isEmpty()) {
3977+
return image.resize({ width: 64, height: 64, quality: "best" }).toDataURL();
3978+
}
3979+
3980+
return icon.mimeType === "image/png" ? icon.dataUrl : null;
3981+
} catch {
3982+
return null;
3983+
}
3984+
}
3985+
39633986
async function listMobileSyncProjects(): Promise<{ projects: SyncMobileProjectSummary[] }> {
39643987
const recentProjects = (readGlobalState(globalStatePath).recentProjects ?? [])
39653988
.map(inspectRecentProject);
@@ -3971,9 +3994,11 @@ app.whenReady().then(async () => {
39713994
byRoot.set(normalizeProjectRoot(recent.summary.rootPath), mobileProjectSummaryForRecent(recent));
39723995
}
39733996
const contextSummaries = await Promise.all(
3974-
[...projectContexts.entries()].map(async ([root, ctx]) =>
3975-
[root, await mobileProjectSummaryForContext(ctx, recentByRoot.get(root) ?? null)] as const
3976-
),
3997+
[...projectContexts.entries()]
3998+
.filter(([root]) => recentByRoot.has(root))
3999+
.map(async ([root, ctx]) =>
4000+
[root, await mobileProjectSummaryForContext(ctx, recentByRoot.get(root) ?? null)] as const
4001+
),
39774002
);
39784003
for (const [root, summary] of contextSummaries) {
39794004
byRoot.set(root, summary);

apps/desktop/src/main/services/ipc/registerIpc.ts

Lines changed: 181 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { app, BrowserWindow, clipboard, dialog, ipcMain, shell } from "electron";
1+
import { app, BrowserWindow, clipboard, dialog, ipcMain, nativeImage, shell } from "electron";
22
import { createEmptyAutoUpdateSnapshot, type createAutoUpdateService } from "../updates/autoUpdateService";
33
import { spawn } from "node:child_process";
44
import { randomUUID } from "node:crypto";
@@ -13,6 +13,7 @@ import { launchPrIssueResolutionChat, previewPrIssueResolutionPrompt } from "../
1313
import { launchRebaseResolutionChat } from "../prs/prRebaseResolver";
1414
import { browseProjectDirectories } from "../projects/projectBrowserService";
1515
import { getProjectDetail } from "../projects/projectDetailService";
16+
import { resolveProjectIcon } from "../projects/projectIconResolver";
1617
import { runGit } from "../git/git";
1718
import type { AdeCleanupResult, AdeProjectSnapshot } from "../../../shared/types";
1819
import { toRecentProjectSummary } from "../projects/recentProjectSummary";
@@ -258,6 +259,7 @@ import type {
258259
ProjectBrowseInput,
259260
ProjectBrowseResult,
260261
ProjectDetail,
262+
ProjectIcon,
261263
ProjectInfo,
262264
RecentProjectSummary,
263265
PtyCreateArgs,
@@ -1890,6 +1892,107 @@ export function registerIpc({
18901892
return path.resolve(path.isAbsolute(inputPath) ? inputPath : path.join(projectRoot, inputPath));
18911893
};
18921894

1895+
const resolveAllowedRendererPath = (rawPath: string): string => {
1896+
const raw = typeof rawPath === "string" ? rawPath.trim() : "";
1897+
if (!raw) throw new Error("Missing path.");
1898+
const ctx = getCtx();
1899+
const normalized = resolveRendererSuppliedPath(raw, ctx.project.rootPath);
1900+
const allowedDirs = getAllowedDirs(getCtx);
1901+
// resolvePathWithinRoot follows symlinks via fs.realpath while validating
1902+
// containment, so we both reject symlinks pointing outside the allowlist
1903+
// *and* return the canonical real path for callers to read from. Returning
1904+
// the lexical path would still be safe because the check resolved real
1905+
// paths, but handing back the realpath avoids any TOCTOU-adjacent surprises
1906+
// and keeps file I/O pinned to the validated target.
1907+
let resolved: string | null = null;
1908+
for (const dir of allowedDirs) {
1909+
try {
1910+
resolved = resolvePathWithinRoot(dir, normalized);
1911+
break;
1912+
} catch {
1913+
// try next allowed dir
1914+
}
1915+
}
1916+
if (!resolved) {
1917+
throw new Error("Path is outside allowed directories.");
1918+
}
1919+
return resolved;
1920+
};
1921+
1922+
/**
1923+
* Sniff the first bytes of a buffer for known image magic numbers and
1924+
* return the corresponding MIME type. Returns null if the buffer doesn't
1925+
* match any supported image format.
1926+
*
1927+
* We deliberately do NOT trust the file extension here — extension-only
1928+
* inference would let a renderer hand us any allow-listed file (text,
1929+
* binary, etc.) and get it back as a base64 `image/png` data URL.
1930+
*/
1931+
const sniffImageMimeType = (buffer: Buffer): string | null => {
1932+
if (buffer.length >= 8
1933+
&& buffer[0] === 0x89 && buffer[1] === 0x50 && buffer[2] === 0x4E && buffer[3] === 0x47
1934+
&& buffer[4] === 0x0D && buffer[5] === 0x0A && buffer[6] === 0x1A && buffer[7] === 0x0A) {
1935+
return "image/png";
1936+
}
1937+
if (buffer.length >= 3
1938+
&& buffer[0] === 0xFF && buffer[1] === 0xD8 && buffer[2] === 0xFF) {
1939+
return "image/jpeg";
1940+
}
1941+
if (buffer.length >= 6
1942+
&& buffer[0] === 0x47 && buffer[1] === 0x49 && buffer[2] === 0x46 && buffer[3] === 0x38
1943+
&& (buffer[4] === 0x37 || buffer[4] === 0x39) && buffer[5] === 0x61) {
1944+
return "image/gif";
1945+
}
1946+
if (buffer.length >= 12
1947+
&& buffer[0] === 0x52 && buffer[1] === 0x49 && buffer[2] === 0x46 && buffer[3] === 0x46
1948+
&& buffer[8] === 0x57 && buffer[9] === 0x45 && buffer[10] === 0x42 && buffer[11] === 0x50) {
1949+
return "image/webp";
1950+
}
1951+
if (buffer.length >= 2 && buffer[0] === 0x42 && buffer[1] === 0x4D) {
1952+
return "image/bmp";
1953+
}
1954+
if (buffer.length >= 4
1955+
&& buffer[0] === 0x00 && buffer[1] === 0x00
1956+
&& buffer[2] === 0x01 && buffer[3] === 0x00) {
1957+
return "image/x-icon";
1958+
}
1959+
// SVG/XML: scan a small prefix as text so leading whitespace, BOM, or an
1960+
// <?xml ... ?> declaration before <svg ...> are tolerated.
1961+
const head = buffer.slice(0, Math.min(buffer.length, 1024)).toString("utf8");
1962+
const stripped = head.replace(/^/, "").trimStart();
1963+
if (/^<\?xml\b/i.test(stripped) && /<svg\b/i.test(head)) {
1964+
return "image/svg+xml";
1965+
}
1966+
if (/^<svg\b/i.test(stripped)) {
1967+
return "image/svg+xml";
1968+
}
1969+
return null;
1970+
};
1971+
1972+
const MAX_IMAGE_BYTES = 10 * 1024 * 1024;
1973+
1974+
/**
1975+
* Read an allow-listed image file from disk after a stat-based size check,
1976+
* sniff its bytes for a known image magic, and return both the bytes and
1977+
* the sniffed MIME type. Throws if the file is too large, isn't a regular
1978+
* file, or doesn't look like a supported image.
1979+
*/
1980+
const readImageFileAndSniffMime = async (filePath: string): Promise<{ data: Buffer; mimeType: string }> => {
1981+
const stat = await fs.promises.stat(filePath);
1982+
if (!stat.isFile()) {
1983+
throw new Error("Path is not a file.");
1984+
}
1985+
if (stat.size > MAX_IMAGE_BYTES) {
1986+
throw new Error("Image must be 10 MB or smaller.");
1987+
}
1988+
const data = await fs.promises.readFile(filePath);
1989+
const mimeType = sniffImageMimeType(data);
1990+
if (!mimeType) {
1991+
throw new Error("Path is not an image.");
1992+
}
1993+
return { data, mimeType };
1994+
};
1995+
18931996
ipcMain.handle(IPC.appRevealPath, async (_event, arg: { path: string }): Promise<void> => {
18941997
const raw = typeof arg?.path === "string" ? arg.path.trim() : "";
18951998
if (!raw) return;
@@ -1940,6 +2043,35 @@ export function registerIpc({
19402043
clipboard.writeText(text);
19412044
});
19422045

2046+
ipcMain.handle(IPC.appGetImageDataUrl, async (_event, arg: { path: string }): Promise<{ dataUrl: string }> => {
2047+
const filePath = resolveAllowedRendererPath(arg?.path);
2048+
// Use async fs APIs and a size pre-check so a 10 MB image read never
2049+
// blocks the main process event loop (input dispatch, IPC, window
2050+
// animations all share that loop). The MIME type is derived from the
2051+
// file's *bytes*, not its extension, so a renderer can't smuggle
2052+
// arbitrary text/binary back as a base64 `image/png` data URL.
2053+
const { data, mimeType } = await readImageFileAndSniffMime(filePath);
2054+
return {
2055+
dataUrl: `data:${mimeType};base64,${data.toString("base64")}`,
2056+
};
2057+
});
2058+
2059+
ipcMain.handle(IPC.appWriteClipboardImage, async (_event, arg: { path: string }): Promise<void> => {
2060+
const filePath = resolveAllowedRendererPath(arg?.path);
2061+
// Apply the same size + magic-byte preflight as `appGetImageDataUrl` so
2062+
// we can't hand `nativeImage.createFromPath` a giant or non-image file
2063+
// (which would otherwise silently produce an empty image, or worse,
2064+
// attempt a sync read of a 100 MB binary on the main process). We then
2065+
// hand the already-read buffer to `nativeImage.createFromBuffer` so the
2066+
// file isn't read a second time off the main thread.
2067+
const { data } = await readImageFileAndSniffMime(filePath);
2068+
const image = nativeImage.createFromBuffer(data);
2069+
if (image.isEmpty()) {
2070+
throw new Error("Unable to read image.");
2071+
}
2072+
clipboard.writeImage(image);
2073+
});
2074+
19432075
ipcMain.handle(
19442076
IPC.appOpenPathInEditor,
19452077
async (
@@ -2146,6 +2278,54 @@ export function registerIpc({
21462278
}
21472279
);
21482280

2281+
// Project-root allowlist for icon resolution. Tab/catalog icons are
2282+
// resolved for the *current* project root and any *recently opened*
2283+
// project root — including ones that live outside Downloads/Documents/Temp
2284+
// (the generic `getAllowedDirs` set). Using `resolveAllowedRendererPath`
2285+
// here would silently strip icons for any project in `~/code/*` etc.
2286+
const getAllowedProjectRoots = (): string[] => {
2287+
const state = readGlobalState(globalStatePath);
2288+
return Array.from(new Set([
2289+
getCtx().project.rootPath,
2290+
...(state.recentProjects ?? [])
2291+
.map((entry) => entry.rootPath)
2292+
.filter((root): root is string => typeof root === "string" && root.trim().length > 0),
2293+
]));
2294+
};
2295+
2296+
const resolveAllowedProjectRoot = (rawPath: string): string => {
2297+
const raw = typeof rawPath === "string" ? rawPath.trim() : "";
2298+
if (!raw) throw new Error("Missing root path.");
2299+
const normalized = resolveRendererSuppliedPath(raw, getCtx().project.rootPath);
2300+
for (const dir of getAllowedProjectRoots()) {
2301+
try {
2302+
return resolvePathWithinRoot(dir, normalized);
2303+
} catch {
2304+
// try next known project root
2305+
}
2306+
}
2307+
throw new Error("rootPath is outside known project roots.");
2308+
};
2309+
2310+
ipcMain.handle(
2311+
IPC.projectResolveIcon,
2312+
async (_event, args: { rootPath: string }): Promise<ProjectIcon> => {
2313+
const rootPath = typeof args?.rootPath === "string" ? args.rootPath.trim() : "";
2314+
if (!rootPath) return { dataUrl: null, sourcePath: null, mimeType: null };
2315+
// Validate the renderer-supplied root against the project-root
2316+
// allowlist (current + recent projects) so a compromised renderer
2317+
// can't probe arbitrary directories for icons, while still serving
2318+
// icons for projects that live outside the generic file allowlist.
2319+
let validatedRoot: string;
2320+
try {
2321+
validatedRoot = resolveAllowedProjectRoot(rootPath);
2322+
} catch {
2323+
return { dataUrl: null, sourcePath: null, mimeType: null };
2324+
}
2325+
return resolveProjectIcon(validatedRoot);
2326+
},
2327+
);
2328+
21492329
ipcMain.handle(IPC.projectOpenAdeFolder, async (): Promise<void> => {
21502330
const ctx = getCtx();
21512331
await shell.openPath(ctx.adeDir);

apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3278,9 +3278,11 @@ describe("aiOrchestratorService", () => {
32783278
}) as typeof fixture.orchestratorService.onTrackedSessionEnded;
32793279

32803280
const firstSweep = fixture.aiOrchestratorService.runHealthSweep("overlap-owner");
3281-
// CI runners can be heavily loaded; give the first sweep up to ~5s to reach
3282-
// the gated `onTrackedSessionEnded` call before we assert it was invoked.
3283-
for (let tries = 0; tries < 200 && reconcileCalls === 0; tries += 1) {
3281+
// CI runners can be heavily loaded; give the first sweep up to ~15s to
3282+
// reach the gated `onTrackedSessionEnded` call before we assert it was
3283+
// invoked. The test passes immediately when the callback fires, so the
3284+
// generous ceiling only kicks in for slow/contended runners.
3285+
for (let tries = 0; tries < 600 && reconcileCalls === 0; tries += 1) {
32843286
await new Promise((resolve) => setTimeout(resolve, 25));
32853287
}
32863288
expect(reconcileCalls).toBe(1);
@@ -3295,7 +3297,7 @@ describe("aiOrchestratorService", () => {
32953297
releaseFirstSweep();
32963298
fixture.dispose();
32973299
}
3298-
}, 15_000);
3300+
}, 30_000);
32993301

33003302
it("skips background health sweeps for runs blocked on open interventions", async () => {
33013303
const fixture = await createFixture();

0 commit comments

Comments
 (0)