Implement mobile project switching and sync catalog#172
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughCaller-aware local computer-use gating added; mobile project catalog and switch protocol implemented across desktop and iOS; mission preflight now enforces phase-level proof gating and capability fallbacks; onboarding/tour lifecycle and context threading added; editorial homepage redesign with many new components; numerous tests and iOS DB project-scoping changes. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![capy-ai[bot]](https://avatars.githubusercontent.com/in/1915919?s=60&v=4){kind=small}
| .map(toRecentProjectSummary) | ||
| .find((entry) => normalizeProjectRoot(entry.rootPath) === targetRoot) ?? null; | ||
| const project = await mobileProjectSummaryForContext(ctx, recent, { useProjectRowId: true }); | ||
| mobileSyncHandoffLeases.set(targetRoot, Date.now() + MOBILE_SYNC_HANDOFF_LEASE_MS); |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
This handoff lease is only consulted inside hasActiveProjectWorkloads() during a rebalance pass, but the new switch path only queues a rebalance immediately after setting the lease. If the phone cancels the switch or never reconnects, nothing schedules another rebalance when the 60s lease expires, so the inactive project context can stay resident indefinitely with its sync host, DB, and watchers still alive until some unrelated project switch happens. ts // apps/desktop/src/main/main.ts const project = await mobileProjectSummaryForContext(ctx, recent, { useProjectRowId: true }); mobileSyncHandoffLeases.set(targetRoot, Date.now() + MOBILE_SYNC_HANDOFF_LEASE_MS); projectLastActivatedAt.set(targetRoot, Date.now()); scheduleProjectContextRebalance(); Schedule a follow-up rebalance for leaseExpiresAt (or explicitly clear/rebalance when the handoff finishes/fails) so abandoned mobile switches do not pin warm contexts forever.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/desktop/src/renderer/components/onboarding/tour/TourOverlay.tsx (1)
157-196:⚠️ Potential issue | 🟠 MajorDisable the overlay Escape handler during act intro steps.
ActIntroalready handles Escape as skip viaonSkip, but the overlay-level keydown handler still treats the same Escape as tour dismissal. Pressing Escape on an intro step can therefore advance and dismiss the tour in the same key event.Proposed fix
useEffect(() => { + if (step.actIntro) return; const onKeyDown = (e: KeyboardEvent) => { if (e.key === "Escape") { e.preventDefault(); handleDismiss(); return; @@ }; window.addEventListener("keydown", onKeyDown); return () => window.removeEventListener("keydown", onKeyDown); - }, [handleNext, handlePrev, handleDismiss]); + }, [step.actIntro, handleNext, handlePrev, handleDismiss]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/onboarding/tour/TourOverlay.tsx` around lines 157 - 196, The overlay-level keydown handler (useEffect -> onKeyDown) is intercepting Escape during ActIntro and causing both skip and dismiss; update onKeyDown to no-op when step.actIntro is truthy so ActIntro can handle Escape itself (i.e., return early if step.actIntro), and add step.actIntro (or step) to the effect dependency array so the handler updates when the step changes; reference the useEffect/onKeyDown block, the step.actIntro check, and the handleDismiss/ActIntro onSkip handlers when making the change.apps/ade-cli/src/adeRpcServer.ts (1)
4136-4145:⚠️ Potential issue | 🟠 MajorAdd policy guard to close direct-call bypass for
get_environment_info.
get_environment_infois hidden from the tool list viaLOCAL_COMPUTER_USE_TOOL_NAMES, but a direct RPC invocation bypasses this because the handler never callsensureLocalComputerUse. This allows callers to access sensitive environment details (frontmost app, display configuration) without authorization.Add the policy guard before reading system state:
Fix
if (name === "get_environment_info") { + ensureLocalComputerUse(name, "environmentInfo"); const includeDisplays = asBoolean(toolArgs.includeDisplays, false); const capabilities = getLocalComputerUseCapabilities();Also add a test case asserting that
get_environment_infois denied for external callers (similar to the existingscreenshot_environmentdenial test). Currently only the success case with orchestrator role is tested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ade-cli/src/adeRpcServer.ts` around lines 4136 - 4145, The get_environment_info RPC handler currently reads sensitive system state without calling the policy guard; update the handler to call ensureLocalComputerUse(toolName, "environmentInfo") (or simply ensureLocalComputerUse with appropriate toolName) before any system access so external callers are denied when isLocalComputerUseAllowed(callerCtx) is false; reference the existing ensureLocalComputerUse function and LOCAL_COMPUTER_USE_TOOL_NAMES to pick the same capability key ("environmentInfo") and mirror the placement used in other handlers (e.g., screenshot handler). Also add a unit/integration test that asserts get_environment_info is denied for non-orchestrator/external callers (pattern it after the existing screenshot_environment denial test) to prevent future regressions.apps/ios/ADE/Services/Database.swift (3)
720-739:⚠️ Potential issue | 🟠 MajorFilter lane list snapshots by
currentProjectId().
fetchLaneListSnapshotsjoinslanesbut only filters archive state, so cached snapshots from inactive projects can still appear in the mobile lane list after switching projects.🐛 Proposed fix
func fetchLaneListSnapshots(includeArchived: Bool) -> [LaneListSnapshot] { + guard let projectId = currentProjectId() else { return [] } let sql = """ select s.lane_id, s.snapshot_json, s.updated_at from lane_list_snapshots s join lanes l on l.id = s.lane_id - where (? = 1 or l.archived_at is null) + where l.project_id = ? + and (? = 1 or l.archived_at is null) order by l.created_at desc """ - return query(sql, bind: { statement in - sqlite3_bind_int(statement, 1, includeArchived ? 1 : 0) + return query(sql, bind: { [self] statement in + try self.bindText(projectId, to: statement, index: 1) + sqlite3_bind_int(statement, 2, includeArchived ? 1 : 0) }) { statement in🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/Database.swift` around lines 720 - 739, fetchLaneListSnapshots currently only filters archived lanes and can return snapshots from other projects; update the SQL and binding in fetchLaneListSnapshots to also restrict to the active project by joining/filtering on l.project_id = ? (or equivalent) using currentProjectId(). Modify the SQL (in the function fetchLaneListSnapshots) to add "and l.project_id = ?" to the WHERE clause, add a second sqlite3_bind_* call in the bind closure to bind currentProjectId(), and ensure the rest of the function (the query call and decoding) remains unchanged.
1559-1688:⚠️ Potential issue | 🟠 MajorScope PR list reads to the active project.
fetchPullRequests()andfetchPullRequestListItems(forLane:)still read all projects. This can leak PRs from the previously active desktop project into the current mobile project view.🐛 Proposed direction
func fetchPullRequests() -> [PrSummary] { + guard let projectId = currentProjectId() else { return [] } let sql = """ select id, lane_id, project_id, repo_owner, repo_name, github_pr_number, github_url, github_node_id, title, state, base_branch, head_branch, checks_status, review_status, additions, deletions, last_synced_at, created_at, updated_at from pull_requests + where project_id = ? order by updated_at desc """ - return query(sql) { statement in + return query(sql, bind: { [self] statement in + try self.bindText(projectId, to: statement, index: 1) + }) { statement inFor
fetchPullRequestListItems(forLane:), addpr.project_id = ?to the generatedwhereclause and bindlaneIdas the second parameter when present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/Database.swift` around lines 1559 - 1688, Both fetchPullRequests() and fetchPullRequestListItems(forLane:) are missing a filter by the active project, causing PRs from other projects to leak; update both SQL queries to include "pr.project_id = ?" in their WHERE clauses and bind the active project id when preparing the statement (for fetchPullRequestListItems(forLane:) ensure that when laneId is present you use "where pr.project_id = ? and pr.lane_id = ?" or when laneId is nil use "where pr.project_id = ?" and bind the project id as the first parameter and laneId as the second parameter when applicable), and adjust the parameter binding order in the query execution code for the functions fetchPullRequests() and fetchPullRequestListItems(forLane:) accordingly.
696-708:⚠️ Potential issue | 🟠 MajorScope lane-detail snapshot deletion to the active project.
Line 698 deletes every
lane_detail_snapshotsrow when the active project hydrates with no lanes, and Lines 700-708 delete details for any lane not in the current hydration set, including lanes from other projects. That can wipe cached lane details for inactive projects when switching.🐛 Proposed fix
let laneIds = orderedLanes.map(\.id) if laneIds.isEmpty { - try exec("delete from lane_detail_snapshots") + _ = try execute(""" + delete from lane_detail_snapshots + where lane_id in ( + select id + from lanes + where project_id = ? + ) + """) { statement in + try bindText(projectId, to: statement, index: 1) + } } else { - try exec(""" + _ = try execute(""" delete from lane_detail_snapshots - where not exists ( + where lane_id in ( + select id + from lanes + where project_id = ? + ) + and not exists ( select 1 from temp_hydrated_lane_ids hydrated where hydrated.id = lane_detail_snapshots.lane_id ) - """) + """) { statement in + try bindText(projectId, to: statement, index: 1) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/Database.swift` around lines 696 - 708, The deletion currently removes snapshots across all projects; scope it to the active project by including the project's id in the WHERE clause. When laneIds.isEmpty, change the exec call to delete only rows with project_id = <activeProjectId> (use the variable that holds the current project id), and in the non-empty branch add "and lane_detail_snapshots.project_id = <activeProjectId>" to the delete ... where not exists (...) query; ensure temp_hydrated_lane_ids only contains IDs for that same project or join/filter by project_id to avoid removing snapshots from other projects.
🧹 Nitpick comments (11)
apps/desktop/src/main/services/computerUse/computerUseArtifactBrokerService.ts (1)
485-537: Cache external-backend detection to avoid spawningsh -lcsubprocesses on every call.
commandExists("ghost")andcommandExists("agent-browser")runspawnSync("sh", ["-lc", ...])on POSIX platforms (login shell sources rc files, i.e., tens–hundreds of ms each).getBackendStatusis invoked on hot paths —buildComputerUseOwnerSnapshotfor every owner snapshot andrunPreflight— so every preflight/snapshot read does 2 synchronous subprocess launches that block the main process. The installed state of these CLIs changes rarely; memoize (ideally in service closure, optionally with a short TTL or an invalidate hook when capabilities are re-checked).♻️ Suggested memoization inside the service closure
const allowedImportRoots = Array.from(new Set([ layout.artifactsDir, layout.tmpDir, os.tmpdir(), path.join(os.homedir(), ".agent-browser"), ])); + + type BackendProbe = { installed: boolean; at: number }; + const BACKEND_PROBE_TTL_MS = 60_000; + const backendProbeCache = new Map<string, BackendProbe>(); + const probeBackend = (command: string): boolean => { + const cached = backendProbeCache.get(command); + const now = Date.now(); + if (cached && now - cached.at < BACKEND_PROBE_TTL_MS) return cached.installed; + const installed = commandExists(command); + backendProbeCache.set(command, { installed, at: now }); + return installed; + }; @@ - const ghostInstalled = commandExists("ghost"); + const ghostInstalled = probeBackend("ghost"); @@ - const agentBrowserInstalled = commandExists("agent-browser"); + const agentBrowserInstalled = probeBackend("agent-browser");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/computerUse/computerUseArtifactBrokerService.ts` around lines 485 - 537, getBackendStatus currently calls commandExists("ghost") and commandExists("agent-browser") every invocation, spawning costly sh -lc subprocesses; memoize those checks in the service closure and use the cached values in getBackendStatus (or add a short TTL/invalidate hook) so you don't spawn on every snapshot/preflight. Concretely: add closure-scoped variables (e.g., ghostInstalledCached, agentBrowserInstalledCached and ghostCheckedAt/agentCheckedAt if TTL desired) initialized once (or on first use), replace direct commandExists calls in getBackendStatus with the cached booleans, and add an invalidate or refresh call in the existing capability re-check flow (the function that updates local capabilities, e.g., getLocalComputerUseCapabilities or its re-check handler) to reset or refresh the cached values when a capability scan runs. Ensure getBackendStatus still returns correct supportedKinds/state/detail based on the cached booleans.apps/web/src/components/editorial/FadeBand.tsx (1)
3-6: Update the stale height comment.The component documents a
40vhband, but the rendered class usesh-[18vh] min-h-[110px].📝 Proposed comment fix
- * 40vh gradient band for dark↔cream transitions. + * Responsive gradient band for dark↔cream transitions.Also applies to: 22-22
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/editorial/FadeBand.tsx` around lines 3 - 6, The top comment in the FadeBand component is stale: it says "40vh gradient band" but the JSX uses class names h-[18vh] and min-h-[110px]; update the comment in FadeBand (component FadeBand) to reflect the actual rendered height (18vh with a 110px min-height) or make it generic (e.g., "18vh / min 110px gradient band") so the doc matches the class names h-[18vh] and min-h-[110px].apps/desktop/src/renderer/onboarding/stepBuilders/stepBuilders.test.ts (1)
65-69: Use the shared docs catalog as the allowed URL set.Prefix validation still permits stale hardcoded paths. Checking against
docsLinkskeeps builder steps aligned with the centralized docs contract.🧪 Proposed assertion update
import type { TourCtx } from "../registry"; +import { docs } from "../docsLinks"; @@ it("every docUrl points at ade-app.dev", () => { + const allowed = new Set<string>(Object.values(docs)); for (const step of steps) { if (!step.docUrl) continue; - expect(step.docUrl.startsWith(VALID_DOCS_PREFIX)).toBe(true); + expect(allowed.has(step.docUrl), `unknown docUrl: ${step.docUrl}`).toBe(true); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/onboarding/stepBuilders/stepBuilders.test.ts` around lines 65 - 69, The test currently asserts docUrl prefix by checking step.docUrl.startsWith(VALID_DOCS_PREFIX), which allows stale hardcoded paths; change the assertion to validate each step.docUrl against the shared docs catalog instead—import or reference the docsLinks collection and assert docsLinks.has(step.docUrl) (or docsLinks.includes(step.docUrl) depending on its type) for every step that has a docUrl, replacing the VALID_DOCS_PREFIX check in the test that iterates over steps and uses the step.docUrl symbol.apps/desktop/src/renderer/onboarding/tours/highlights.test.ts (1)
62-67: Assert highlight doc URLs come fromdocsLinks.This domain-only check won’t catch a hardcoded or stale highlight URL. Mirror the
lanesTourcoverage by validating againstObject.values(docs).🧪 Proposed tighter assertion
import { describe, it, expect, beforeAll } from "vitest"; import "./index"; import { getTour, listTours } from "../registry"; +import { docs } from "../docsLinks"; @@ it("every docUrl on highlight steps points to ade-app.dev", () => { + const allowed = new Set<string>(Object.values(docs)); for (const tour of listTours("highlights")) { for (const step of tour.steps) { if (!step.docUrl) continue; - expect(step.docUrl).toMatch(/^https:\/\/www\.ade-app\.dev/); + expect(allowed.has(step.docUrl), `unknown docUrl: ${step.docUrl}`).toBe(true); } } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/onboarding/tours/highlights.test.ts` around lines 62 - 67, The test currently only asserts docUrl starts with the ade-app.dev domain; instead, change the assertion to validate that every step.docUrl (from listTours("highlights") / variable step) is one of the canonical links in the docs object by checking inclusion in Object.values(docs). Locate the test in highlights.test.ts where listTours("highlights") is iterated and replace the expect(step.docUrl).toMatch(...) with an assertion that step.docUrl is in Object.values(docs) (mirroring the lanesTour coverage) and skip steps without docUrl as before.apps/desktop/src/renderer/components/settings/SyncDevicesSection.tsx (1)
171-184: Consider gating the 5s poll on document visibility.
refreshWhenVisiblefires every 5 seconds for as long as the settings pane is mounted, even when the Electron window is hidden or the OS has backgrounded it. Each tick runs two IPC round-trips (getStatus+listDevices). SinceonEvent("sync-status")already pushes updates, you could skip polling whendocument.visibilityState !== "visible"and rely on the existing focus listener to catch up on re-show.Also note
refresh()itself does not checkcancelledbefore callingsetStatus/setDevices/setError, so a tick that resolves after unmount will still update state. The currentcancelledguard only fires before the call.♻️ Proposed fix
const refreshWhenVisible = () => { - if (!cancelled) { - void refresh().catch(() => {}); - } + if (cancelled) return; + if (typeof document !== "undefined" && document.visibilityState !== "visible") return; + void refresh().catch(() => {}); }; const interval = window.setInterval(refreshWhenVisible, 5_000); window.addEventListener("focus", refreshWhenVisible); + document.addEventListener("visibilitychange", refreshWhenVisible); return () => { cancelled = true; window.clearInterval(interval); window.removeEventListener("focus", refreshWhenVisible); + document.removeEventListener("visibilitychange", refreshWhenVisible); dispose(); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/settings/SyncDevicesSection.tsx` around lines 171 - 184, The polling callback refreshWhenVisible should skip calling refresh() when the document is not visible and the refresh function should stop applying state updates if the component has been unmounted: update refreshWhenVisible to first check document.visibilityState === "visible" (in addition to the existing cancelled check) before calling refresh(), and modify refresh (or wrap its internal promises) to consult the cancelled flag before calling setStatus, setDevices, or setError so resolved/errored ticks after unmount do not update state; reference the existing refreshWhenVisible, refresh, and cancelled symbols and ensure the useEffect cleanup still sets cancelled and clears the interval/listener as before.apps/desktop/src/renderer/components/app/TopBar.test.tsx (1)
37-79: Type the sync fixture againstSyncRoleSnapshot.The test fixture mirrors an IPC/shared sync payload, but
Record<string, unknown>lets contract drift slip through. Typing the helper keeps these mocks checked when sync fields change.Proposed type tightening
import React from "react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { TopBar } from "./TopBar"; import { useAppStore } from "../../state/appStore"; +import type { SyncRoleSnapshot } from "../../../shared/types"; @@ -function makeSyncSnapshot(overrides: Record<string, unknown> = {}) { +function makeSyncSnapshot(overrides: Partial<SyncRoleSnapshot> = {}): SyncRoleSnapshot { return { mode: "standalone", role: "brain",As per coding guidelines,
**/*.{ts,tsx}: Keep IPC contracts, preload types, shared types, and renderer usage in sync whenever an interface changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/app/TopBar.test.tsx` around lines 37 - 79, The helper makeSyncSnapshot should be typed to the shared IPC contract to prevent drift: change its signature to accept overrides: Partial<SyncRoleSnapshot> and return SyncRoleSnapshot, and import the SyncRoleSnapshot type (from the shared IPC/preload types used by the renderer). Update any usage that passes overrides to satisfy Partial<SyncRoleSnapshot> and ensure the returned object matches SyncRoleSnapshot (keep the current object shape but let TypeScript validate it).apps/ios/ADE/App/ContentView.swift (1)
23-30: Keep the tab subtree mounted when showing project home.Swapping
rootTabsout destroys the tab view hierarchy while the project picker is open, which can reset nested navigation/scroll/input state when returning to the active app surface. Consider overlayingProjectHomeViewwhen an active project exists, and only replacing tabs when there is no active project.♻️ Suggested SwiftUI structure
- Group { - if syncService.shouldShowProjectHome { - ProjectHomeView() - } else { - rootTabs - } - } + ZStack { + if syncService.activeProjectId != nil { + rootTabs + } + + if syncService.shouldShowProjectHome { + ProjectHomeView() + .transition(.opacity) + } + }As per coding guidelines,
apps/ios/**/*.swift: “iOS Swift app — check for memory management, Swift conventions, and proper SwiftUI patterns.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/App/ContentView.swift` around lines 23 - 30, The current if/else replaces the tab subtree (rootTabs) with ProjectHomeView causing the tab view hierarchy to unmount; change the layout to keep rootTabs mounted and overlay ProjectHomeView when syncService.shouldShowProjectHome is true and there is an active project, e.g. use a ZStack or .overlay to always render rootTabs and conditionally present ProjectHomeView above it, and only fully replace rootTabs when there is no active project; update logic around syncService.shouldShowProjectHome / ProjectHomeView so the tab hierarchy (rootTabs) is never destroyed while the picker is shown.apps/ios/ADETests/ADETests.swift (1)
578-610: Cover lane-less project workspaces to catch cross-project leakage.This only verifies lane-backed workspaces. If root/project workspaces use
laneId == nil, alistWorkspaces()implementation that scopes vialanescan still leak stale workspaces across projects without this test failing.🧪 Suggested test coverage expansion
try database.replaceFilesWorkspaces([ + FilesWorkspace( + id: "workspace-root-one", + kind: "project", + laneId: nil, + name: "Project One", + rootPath: "/tmp/project-one", + isReadOnlyByDefault: true, + mobileReadOnly: true + ), FilesWorkspace( id: "workspace-one", kind: "worktree", @@ try database.replaceFilesWorkspaces([ + FilesWorkspace( + id: "workspace-root-two", + kind: "project", + laneId: nil, + name: "Project Two", + rootPath: "/tmp/project-two", + isReadOnlyByDefault: true, + mobileReadOnly: true + ), FilesWorkspace( id: "workspace-two", kind: "worktree", @@ - XCTAssertEqual(database.listWorkspaces().map(\.id), ["workspace-two"]) + XCTAssertEqual(Set(database.listWorkspaces().map(\.id)), Set(["workspace-root-two", "workspace-two"])) @@ - XCTAssertEqual(database.listWorkspaces().map(\.id), ["workspace-one"]) + XCTAssertEqual(Set(database.listWorkspaces().map(\.id)), Set(["workspace-root-one", "workspace-one"]))Also applies to: 612-619
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADETests/ADETests.swift` around lines 578 - 610, Add test cases that include lane-less (laneId: nil) project workspaces to the replaceFilesWorkspaces calls so listWorkspaces() can't leak across projects; specifically, in the existing setup that calls database.replaceFilesWorkspaces(...) (the two occurrences around the current blocks) add an additional FilesWorkspace entry with laneId: nil (e.g., a root/project workspace for "project-one" and another for "project-two") and then assert via the same listWorkspaces() checks that only workspaces for the active project are returned; update the two blocks where replaceFilesWorkspaces(...) is invoked to include these laneId: nil workspaces to cover lane-less workspace behavior.apps/desktop/src/shared/types/sync.ts (1)
249-254: Make successful switch results requireprojectandconnection.The current type permits
{ ok: true }, which would compile but leave iOS without the connection payload needed to complete a switch.♻️ Proposed type tightening
-export type SyncProjectSwitchResultPayload = { - ok: boolean; - message?: string | null; - project?: SyncMobileProjectSummary | null; - connection?: SyncProjectConnectionPayload | null; -}; +export type SyncProjectSwitchResultPayload = + | { + ok: true; + message?: string | null; + project: SyncMobileProjectSummary; + connection: SyncProjectConnectionPayload; + } + | { + ok: false; + message?: string | null; + project?: null; + connection?: null; + };As per coding guidelines,
**/*.{ts,tsx}: Keep IPC contracts, preload types, shared types, and renderer usage in sync whenever an interface changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/shared/types/sync.ts` around lines 249 - 254, The current SyncProjectSwitchResultPayload type allows ok: true without required project/connection; change it to a discriminated union so that when ok is true the payload requires project: SyncMobileProjectSummary and connection: SyncProjectConnectionPayload, and when ok is false keep message?: string | null (and optional project/connection if needed). Update the type alias SyncProjectSwitchResultPayload to a union of { ok: true; project: SyncMobileProjectSummary; connection: SyncProjectConnectionPayload; message?: string | null } | { ok: false; message?: string | null; project?: SyncMobileProjectSummary | null; connection?: SyncProjectConnectionPayload | null } so callers can rely on project/connection when ok === true.apps/ios/ADE/Services/SyncService.swift (2)
3600-3602: Don't blockhelloon the remote catalog refresh.
refreshRemoteProjectCatalog()goes throughawaitResponsewith the default 30s timeout and intentionally usesdisconnectOnTimeout: false, so a slow or silent desktop response will stall this continuation for up to 30s. Sincehellois awaited fromconnectUsingProfile, the reconnect path keepsreconnectConnectInFlight = trueand cannot service network-path / foreground-triggered retries for the whole window. The catalog is already delivered inline viahello_ok.projects(applied at L3663-3666) and/or pushed as an unsolicitedproject_catalog(L3824-3827), andshowProjectHome()fires the same refresh as a detached Task — mirroring that here keeps behavior consistent without holding hello open.♻️ Proposed fix
await restoreTrackedOpenLanesAfterReconnect() - await refreshRemoteProjectCatalog() + Task { `@MainActor` [weak self] in + await self?.refreshRemoteProjectCatalog() + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/SyncService.swift` around lines 3600 - 3602, Currently the code awaits refreshRemoteProjectCatalog() which blocks the hello continuation (called from connectUsingProfile) and leaves reconnectConnectInFlight true for the awaitResponse timeout; change the call so the refresh runs asynchronously without awaiting it (e.g., spawn a Task or Task.detached that calls await refreshRemoteProjectCatalog()), but keep await restoreTrackedOpenLanesAfterReconnect() as-is so tracked lanes are restored before continuing; mirror the same detached-refresh approach used by showProjectHome to avoid blocking hello and allow retries to proceed.
641-645: Normalize rootPaths before matching to avoid duplicate catalog entries.
isActiveProject(L568-572) and the active-project reconcile branch (L663-665) both match root paths vianormalizedProjectRoot(...), but this fallback comparesremote.rootPathandcachedProject.rootPathverbatim. Cached rows come out of SQLite with trailing slashes/whitespace already trimmed, whileremoteProjectCatalogrows come straight from the desktop JSON and can carry a trailing/or padding. When they diverge only in that way the cached and remote descriptors of the same project won't merge, soprojectsends up with two entries for the same path and the active-project sort/selection becomes ambiguous.♻️ Proposed fix
} else if let match = mergedById.first(where: { entry in let remote = entry.value - guard let left = remote.rootPath, let right = cachedProject.rootPath else { return false } - return left == right + guard let left = normalizedProjectRoot(remote.rootPath), + let right = normalizedProjectRoot(cachedProject.rootPath) + else { return false } + return left == right }) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/SyncService.swift` around lines 641 - 645, The fallback match against mergedById compares remote.rootPath and cachedProject.rootPath verbatim which allows mismatches when remote paths have trailing slashes/whitespace; update this branch to normalize both sides using the existing normalizedProjectRoot(...) helper (the same normalization used by isActiveProject and the active-project reconcile branch) before comparing—i.e., compute normalizedProjectRoot(remote.rootPath) and normalizedProjectRoot(cachedProject.rootPath) (ensuring you handle nils) and compare those so cached and remote descriptors for the same project merge correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/main.ts`:
- Around line 3486-3501: mobileProjectSummaryForContext currently can return
either ctx.projectId or a stable root id
(`root:${normalizeProjectRoot(ctx.project.rootPath)}`), which causes mismatch
between catalog entries (which use the normalized root id) and switch-success
payloads that return ctx.projectId; fix by ensuring switch-success responses use
the stable root id: update the code that builds the switch success payload to
call mobileProjectSummaryForContext with options.useProjectRowId set to false
(or remove the useProjectRowId branch so the function always returns
`root:${normalizeProjectRoot(...)}`), and ensure any places that previously
returned ctx.projectId instead use the mobileProjectSummaryForContext result so
IDs remain stable across catalog and switch results (referenced symbols:
mobileProjectSummaryForContext, ctx.projectId, normalizeProjectRoot).
In `@apps/desktop/src/main/services/sync/syncHostService.ts`:
- Around line 1637-1653: The hello_ok payload always advertises
projectCatalog.enabled: true even when no projectCatalogProvider exists; update
the code that builds/sends the hello_ok response (the send call that includes
buildProjectCatalogPayload and projectCatalog.projects) to set
features.projectCatalog.enabled based on whether projectCatalogProvider is
available (e.g., whether buildProjectCatalogPayload returns available or a
provider exists) instead of hardcoding true, and update the
SyncFeatureFlags.projectCatalog.enabled type in shared types (SyncFeatureFlags /
projectCatalog) to be a plain boolean so the flag can reflect actual
availability.
In `@apps/desktop/src/renderer/components/app/TopBar.tsx`:
- Around line 571-607: The backdrop currently uses onMouseDown to close the
phone sync panel which allows click-through; change the outer backdrop handler
to onClick (keep the inner onMouseDown stopPropagation), and add proper
modal/dialog semantics to the panel: add role="dialog" and aria-modal="true" on
the panel container, give the header title element (the "Phone sync" text in the
inner header) an id and set aria-labelledby to that id, and make the panel
focusable (tabIndex={-1}) and move focus to it or the close button when
phoneSyncOpen becomes true; also add an onKeyDown on the panel to close on
Escape by calling setPhoneSyncOpen(false) so keyboard users can dismiss it.
Ensure references to phoneSyncOpen, setPhoneSyncOpen, and SyncDevicesSection are
preserved.
In `@apps/desktop/src/renderer/components/onboarding/HelpMenu.tsx`:
- Around line 234-236: The code treats a whitespace-only
firstStep.waitForSelector as valid because it isn't trimmed; update the selector
computation so waitForSelector is trimmed and treated as falsy when empty before
falling back to target (i.e., use firstStep?.waitForSelector?.trim() or
equivalent), referencing the symbols firstStep, waitForSelector, target, and
selector so the trimmed/empty waitForSelector won't block tour startup.
In `@apps/desktop/src/renderer/onboarding/docsLinks.test.ts`:
- Around line 5-8: The test "every URL is under ade-app.dev" only asserts
host-level coverage and misses path-level mismatches; update the test in
docsLinks.test.ts to assert canonical routes for the centralized docs mapping
(variable docs) by defining a small dictionary of expected paths for known keys
(e.g., keys used by tours/glossary) and then iterating over Object.entries(docs)
to check both the host (https://www.ade-app.dev) and that the pathname contains
the expected canonical route for that key; keep the original host regex but add
an assertion that url.includes(expectedPath) for keys present in the
expected-path map and fall back to the host-only check for others.
In `@apps/desktop/src/renderer/onboarding/docsLinks.ts`:
- Around line 17-27: Update the doc URL mappings so each key points to its
specific Mintlify page instead of generic overviews: change lanesCreating →
`${DOCS_BASE}/lanes/creating`, lanesStacks → `${DOCS_BASE}/lanes/stacks`,
lanesPacks → `${DOCS_BASE}/lanes/packs`, lanesEnvironment →
`${DOCS_BASE}/lanes/environment`; change chatOverview →
`${DOCS_BASE}/missions/agent-chat`, chatContext →
`${DOCS_BASE}/missions/context`, chatCapabilities →
`${DOCS_BASE}/missions/capabilities`; and change terminals →
`${DOCS_BASE}/tools/terminals`, filesEditor → `${DOCS_BASE}/tools/files-editor`
so the keys (lanesCreating, lanesStacks, lanesPacks, lanesEnvironment,
chatOverview, chatContext, chatCapabilities, terminals, filesEditor) point to
their matching Mintlify routes.
In `@apps/desktop/src/renderer/state/onboardingStore.ts`:
- Around line 194-207: prevStep currently changes activeStepIndex without
invoking the current step's afterLeave hook, so call runAfterLeave for the
current step before mutating state: inside prevStep (use activeStepIndex from
get() and tour from getTour(activeTourId)), construct ctx as you already do
(createTourCtx if needed), then await
runAfterLeave(tour?.steps[activeStepIndex], ctx) before calling set({
activeStepIndex: nextIndex, activeTourCtx: ctx }), then continue with the
existing onboarding update and finally await
runBeforeEnter(tour?.steps[nextIndex], ctx); ensure you handle missing
tour/step/null ctx the same way other paths do.
In `@apps/ios/ADE/App/ContentView.swift`:
- Around line 248-255: The UI allows selecting other projects while a switch is
pending because only the matching row is blocked; wrap ProjectHomeRow's action
and state with a global guard using syncService.projectSwitchInFlightRootPath
(or a helper like syncService.isProjectSwitchInFlight) so that when
projectSwitchInFlightRootPath != nil all rows are gated: change the trailing
closure passed to ProjectHomeRow from always calling
syncService.selectProject(project) to conditionally no-op when
projectSwitchInFlightRootPath != nil, and pass a derived isSwitching/isDisabled
flag (e.g., isSwitching: syncService.projectSwitchInFlightRootPath != nil) to
the row so it visually disables all rows while a switch is in flight.
In `@apps/web/public/mockup.html`:
- Around line 728-731: Replace the placeholder href="#" links in the public
mockup HTML so they aren’t exposed as dead links: locate the <nav class="nav">
anchors (including the <a class="download"> element) and the similar anchors
around lines 811-814 and update each href to the real target URLs (Docs, GitHub,
Download) or remove/disable the href and use a non-navigable element or data
attribute if the mockup shouldn’t be live; ensure the changes only affect the
public mockup file and keep link text and classes intact.
In `@apps/web/src/app/layout/SiteLayout.tsx`:
- Around line 14-18: Currently SiteFooter is conditionally rendered only when
!isHome, which hides legal/privacy/terms links on the homepage; update the JSX
in SiteLayout so SiteFooter is always rendered (replace the conditional {!isHome
&& <SiteFooter />} with an unconditional <SiteFooter />) to ensure footer/legal
links remain reachable on "/". Keep the existing conditional for SiteHeader
as-is.
In `@apps/web/src/app/pages/HomePage.tsx`:
- Around line 64-81: The animated arrow currently uses an inline style
("animation: 'nudge 2.4s...'") on the span and a local <style> keyframes block;
this ignores users who prefer reduced motion. Move the animation into a named
CSS rule (e.g., .turn-arrow) applied to the span (the span with className
"font-serif italic ..." and inline style) and add a media query `@media`
(prefers-reduced-motion: reduce) { .turn-arrow { animation: none !important; } }
(keep the `@keyframes` nudge in the same stylesheet). This ensures the arrow
animation is disabled for reduced-motion users while keeping SSR/react patterns
intact.
In `@apps/web/src/components/editorial/AnnotatedFigure.tsx`:
- Around line 93-123: The callout label elements rendered in the callouts.map
(the motion.div / inner span that displays c.label) must be hidden from
assistive tech; add aria-hidden="true" (and ensure no tabindex) to the label
element (preferably the span containing {c.label} or the surrounding motion.div)
so these visual-only annotations aren’t exposed; if any label content is
essential, move that text into the figcaption or image alt instead.
In `@apps/web/src/components/editorial/BackCover.tsx`:
- Around line 75-89: The CTA in BackCover.tsx uses a hover translate
(`hover:-translate-y-[1px]`) which ignores users' reduced motion settings;
update the primary CTA anchor in the BackCover component to only move when
motion is allowed by replacing the class with a motion-safe variant (e.g.,
`motion-safe:hover:-translate-y-[1px]`) or remove the translation entirely and
rely on color-only hover feedback (keep other classes like Download,
LINKS.releases, and the anchor props unchanged). Ensure the same change is
applied to any other CTA anchor in this file (e.g., the GitHub link) so hover
movement is gated by motion-safe for accessibility.
In `@apps/web/src/components/editorial/CompetitorEquation.tsx`:
- Around line 60-77: The plus separators and abbreviated labels in
CompetitorEquation are decorative and causing redundant screen-reader output;
mark the motion.span that renders "+" and the short-label span (the element
rendering {app.short}) as presentation-only for ATs (e.g., add
aria-hidden="true" or role="presentation") so screen readers only read the img
alt with the full competitor name; keep the img alt as-is and do not remove
visible text styling — only change accessibility attributes on the decorative
motion.span and the short label span.
In `@apps/web/src/components/editorial/Cutout.tsx`:
- Around line 25-38: The reduceMotion variable in Cutout.tsx should default to
the safe "reduced" state when the hook returns null on SSR; change the
declaration to use the nullish fallback (useReducedMotion() ?? true) and update
any checks that depend on reduceMotion (the motion.div initial and whileInView
props) to continue using that variable so animations are disabled by default
during the indeterminate SSR render.
In `@apps/web/src/components/editorial/FeatureGrid.tsx`:
- Around line 135-141: The hover translate/scale must respect reduced-motion:
use the same reduced-motion signal used elsewhere (e.g., useReducedMotion()) or
Tailwind's motion-safe/motion-reduce utilities to disable transforms; modify the
container div class and the img class in FeatureGrid (the element with class
"group-hover:-translate-y-[3px]" and the img with "group-hover:scale-[1.04]") so
that the transform classes are only applied when motion is allowed (either by
prepending motion-safe: to the hover classes or by conditionally applying those
class names when useReducedMotion() is false), ensuring hover still works
without motion for reduced-motion users.
In `@apps/web/src/components/editorial/Masthead.tsx`:
- Around line 10-51: The Masthead component's header (the outer div, the center
volume span, and the nav anchors in nav) currently forces a single-row layout
and can overflow on narrow screens; update the container and nav to be
responsive by allowing wrapping (e.g., add flex-wrap to the outer flex container
and/or nav), reduce spacing and font size at small breakpoints (use responsive
utility classes or media queries for the span and nav items), and change
alignment so items stack vertically or center when space is limited; ensure
links keep their aria-labels and retain SSR/React patterns used in Masthead so
accessibility and server rendering are preserved.
In `@apps/web/src/styles/globals.css`:
- Around line 31-32: Update the CSS variable declarations for --font-sans and
--font-serif to quote the font family names that Stylelint flags (e.g.,
"BlinkMacSystemFont" and "Georgia") so they pass the value-keyword-case rule;
specifically edit the lines that define --font-sans and --font-serif to wrap the
flagged family names in quotes while preserving the existing fallbacks and
order.
---
Outside diff comments:
In `@apps/ade-cli/src/adeRpcServer.ts`:
- Around line 4136-4145: The get_environment_info RPC handler currently reads
sensitive system state without calling the policy guard; update the handler to
call ensureLocalComputerUse(toolName, "environmentInfo") (or simply
ensureLocalComputerUse with appropriate toolName) before any system access so
external callers are denied when isLocalComputerUseAllowed(callerCtx) is false;
reference the existing ensureLocalComputerUse function and
LOCAL_COMPUTER_USE_TOOL_NAMES to pick the same capability key
("environmentInfo") and mirror the placement used in other handlers (e.g.,
screenshot handler). Also add a unit/integration test that asserts
get_environment_info is denied for non-orchestrator/external callers (pattern it
after the existing screenshot_environment denial test) to prevent future
regressions.
In `@apps/desktop/src/renderer/components/onboarding/tour/TourOverlay.tsx`:
- Around line 157-196: The overlay-level keydown handler (useEffect ->
onKeyDown) is intercepting Escape during ActIntro and causing both skip and
dismiss; update onKeyDown to no-op when step.actIntro is truthy so ActIntro can
handle Escape itself (i.e., return early if step.actIntro), and add
step.actIntro (or step) to the effect dependency array so the handler updates
when the step changes; reference the useEffect/onKeyDown block, the
step.actIntro check, and the handleDismiss/ActIntro onSkip handlers when making
the change.
In `@apps/ios/ADE/Services/Database.swift`:
- Around line 720-739: fetchLaneListSnapshots currently only filters archived
lanes and can return snapshots from other projects; update the SQL and binding
in fetchLaneListSnapshots to also restrict to the active project by
joining/filtering on l.project_id = ? (or equivalent) using currentProjectId().
Modify the SQL (in the function fetchLaneListSnapshots) to add "and l.project_id
= ?" to the WHERE clause, add a second sqlite3_bind_* call in the bind closure
to bind currentProjectId(), and ensure the rest of the function (the query call
and decoding) remains unchanged.
- Around line 1559-1688: Both fetchPullRequests() and
fetchPullRequestListItems(forLane:) are missing a filter by the active project,
causing PRs from other projects to leak; update both SQL queries to include
"pr.project_id = ?" in their WHERE clauses and bind the active project id when
preparing the statement (for fetchPullRequestListItems(forLane:) ensure that
when laneId is present you use "where pr.project_id = ? and pr.lane_id = ?" or
when laneId is nil use "where pr.project_id = ?" and bind the project id as the
first parameter and laneId as the second parameter when applicable), and adjust
the parameter binding order in the query execution code for the functions
fetchPullRequests() and fetchPullRequestListItems(forLane:) accordingly.
- Around line 696-708: The deletion currently removes snapshots across all
projects; scope it to the active project by including the project's id in the
WHERE clause. When laneIds.isEmpty, change the exec call to delete only rows
with project_id = <activeProjectId> (use the variable that holds the current
project id), and in the non-empty branch add "and
lane_detail_snapshots.project_id = <activeProjectId>" to the delete ... where
not exists (...) query; ensure temp_hydrated_lane_ids only contains IDs for that
same project or join/filter by project_id to avoid removing snapshots from other
projects.
---
Nitpick comments:
In
`@apps/desktop/src/main/services/computerUse/computerUseArtifactBrokerService.ts`:
- Around line 485-537: getBackendStatus currently calls commandExists("ghost")
and commandExists("agent-browser") every invocation, spawning costly sh -lc
subprocesses; memoize those checks in the service closure and use the cached
values in getBackendStatus (or add a short TTL/invalidate hook) so you don't
spawn on every snapshot/preflight. Concretely: add closure-scoped variables
(e.g., ghostInstalledCached, agentBrowserInstalledCached and
ghostCheckedAt/agentCheckedAt if TTL desired) initialized once (or on first
use), replace direct commandExists calls in getBackendStatus with the cached
booleans, and add an invalidate or refresh call in the existing capability
re-check flow (the function that updates local capabilities, e.g.,
getLocalComputerUseCapabilities or its re-check handler) to reset or refresh the
cached values when a capability scan runs. Ensure getBackendStatus still returns
correct supportedKinds/state/detail based on the cached booleans.
In `@apps/desktop/src/renderer/components/app/TopBar.test.tsx`:
- Around line 37-79: The helper makeSyncSnapshot should be typed to the shared
IPC contract to prevent drift: change its signature to accept overrides:
Partial<SyncRoleSnapshot> and return SyncRoleSnapshot, and import the
SyncRoleSnapshot type (from the shared IPC/preload types used by the renderer).
Update any usage that passes overrides to satisfy Partial<SyncRoleSnapshot> and
ensure the returned object matches SyncRoleSnapshot (keep the current object
shape but let TypeScript validate it).
In `@apps/desktop/src/renderer/components/settings/SyncDevicesSection.tsx`:
- Around line 171-184: The polling callback refreshWhenVisible should skip
calling refresh() when the document is not visible and the refresh function
should stop applying state updates if the component has been unmounted: update
refreshWhenVisible to first check document.visibilityState === "visible" (in
addition to the existing cancelled check) before calling refresh(), and modify
refresh (or wrap its internal promises) to consult the cancelled flag before
calling setStatus, setDevices, or setError so resolved/errored ticks after
unmount do not update state; reference the existing refreshWhenVisible, refresh,
and cancelled symbols and ensure the useEffect cleanup still sets cancelled and
clears the interval/listener as before.
In `@apps/desktop/src/renderer/onboarding/stepBuilders/stepBuilders.test.ts`:
- Around line 65-69: The test currently asserts docUrl prefix by checking
step.docUrl.startsWith(VALID_DOCS_PREFIX), which allows stale hardcoded paths;
change the assertion to validate each step.docUrl against the shared docs
catalog instead—import or reference the docsLinks collection and assert
docsLinks.has(step.docUrl) (or docsLinks.includes(step.docUrl) depending on its
type) for every step that has a docUrl, replacing the VALID_DOCS_PREFIX check in
the test that iterates over steps and uses the step.docUrl symbol.
In `@apps/desktop/src/renderer/onboarding/tours/highlights.test.ts`:
- Around line 62-67: The test currently only asserts docUrl starts with the
ade-app.dev domain; instead, change the assertion to validate that every
step.docUrl (from listTours("highlights") / variable step) is one of the
canonical links in the docs object by checking inclusion in Object.values(docs).
Locate the test in highlights.test.ts where listTours("highlights") is iterated
and replace the expect(step.docUrl).toMatch(...) with an assertion that
step.docUrl is in Object.values(docs) (mirroring the lanesTour coverage) and
skip steps without docUrl as before.
In `@apps/desktop/src/shared/types/sync.ts`:
- Around line 249-254: The current SyncProjectSwitchResultPayload type allows
ok: true without required project/connection; change it to a discriminated union
so that when ok is true the payload requires project: SyncMobileProjectSummary
and connection: SyncProjectConnectionPayload, and when ok is false keep
message?: string | null (and optional project/connection if needed). Update the
type alias SyncProjectSwitchResultPayload to a union of { ok: true; project:
SyncMobileProjectSummary; connection: SyncProjectConnectionPayload; message?:
string | null } | { ok: false; message?: string | null; project?:
SyncMobileProjectSummary | null; connection?: SyncProjectConnectionPayload |
null } so callers can rely on project/connection when ok === true.
In `@apps/ios/ADE/App/ContentView.swift`:
- Around line 23-30: The current if/else replaces the tab subtree (rootTabs)
with ProjectHomeView causing the tab view hierarchy to unmount; change the
layout to keep rootTabs mounted and overlay ProjectHomeView when
syncService.shouldShowProjectHome is true and there is an active project, e.g.
use a ZStack or .overlay to always render rootTabs and conditionally present
ProjectHomeView above it, and only fully replace rootTabs when there is no
active project; update logic around syncService.shouldShowProjectHome /
ProjectHomeView so the tab hierarchy (rootTabs) is never destroyed while the
picker is shown.
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 3600-3602: Currently the code awaits refreshRemoteProjectCatalog()
which blocks the hello continuation (called from connectUsingProfile) and leaves
reconnectConnectInFlight true for the awaitResponse timeout; change the call so
the refresh runs asynchronously without awaiting it (e.g., spawn a Task or
Task.detached that calls await refreshRemoteProjectCatalog()), but keep await
restoreTrackedOpenLanesAfterReconnect() as-is so tracked lanes are restored
before continuing; mirror the same detached-refresh approach used by
showProjectHome to avoid blocking hello and allow retries to proceed.
- Around line 641-645: The fallback match against mergedById compares
remote.rootPath and cachedProject.rootPath verbatim which allows mismatches when
remote paths have trailing slashes/whitespace; update this branch to normalize
both sides using the existing normalizedProjectRoot(...) helper (the same
normalization used by isActiveProject and the active-project reconcile branch)
before comparing—i.e., compute normalizedProjectRoot(remote.rootPath) and
normalizedProjectRoot(cachedProject.rootPath) (ensuring you handle nils) and
compare those so cached and remote descriptors for the same project merge
correctly.
In `@apps/ios/ADETests/ADETests.swift`:
- Around line 578-610: Add test cases that include lane-less (laneId: nil)
project workspaces to the replaceFilesWorkspaces calls so listWorkspaces() can't
leak across projects; specifically, in the existing setup that calls
database.replaceFilesWorkspaces(...) (the two occurrences around the current
blocks) add an additional FilesWorkspace entry with laneId: nil (e.g., a
root/project workspace for "project-one" and another for "project-two") and then
assert via the same listWorkspaces() checks that only workspaces for the active
project are returned; update the two blocks where replaceFilesWorkspaces(...) is
invoked to include these laneId: nil workspaces to cover lane-less workspace
behavior.
In `@apps/web/src/components/editorial/FadeBand.tsx`:
- Around line 3-6: The top comment in the FadeBand component is stale: it says
"40vh gradient band" but the JSX uses class names h-[18vh] and min-h-[110px];
update the comment in FadeBand (component FadeBand) to reflect the actual
rendered height (18vh with a 110px min-height) or make it generic (e.g., "18vh /
min 110px gradient band") so the doc matches the class names h-[18vh] and
min-h-[110px].
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f86a1b42-6b75-4627-b2cf-be55cb0cc020
⛔ Files ignored due to path filters (4)
apps/web/public/images/features/git-history.pngis excluded by!**/*.pngdocs/ARCHITECTURE.mdis excluded by!docs/**docs/features/sync-and-multi-device/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**
📒 Files selected for processing (61)
apps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/desktop/src/main/main.tsapps/desktop/src/main/services/computerUse/computerUseArtifactBrokerService.tsapps/desktop/src/main/services/computerUse/controlPlane.test.tsapps/desktop/src/main/services/computerUse/controlPlane.tsapps/desktop/src/main/services/missions/missionPreflightService.test.tsapps/desktop/src/main/services/missions/missionPreflightService.tsapps/desktop/src/main/services/sync/syncHostService.test.tsapps/desktop/src/main/services/sync/syncHostService.tsapps/desktop/src/main/services/sync/syncProtocol.test.tsapps/desktop/src/main/services/sync/syncService.tsapps/desktop/src/renderer/components/app/TopBar.test.tsxapps/desktop/src/renderer/components/app/TopBar.tsxapps/desktop/src/renderer/components/onboarding/HelpMenu.tsxapps/desktop/src/renderer/components/onboarding/tour/TourHost.tsxapps/desktop/src/renderer/components/onboarding/tour/TourOverlay.tsxapps/desktop/src/renderer/components/onboarding/tour/TourStep.tsxapps/desktop/src/renderer/components/settings/SyncDevicesSection.tsxapps/desktop/src/renderer/onboarding/docsLinks.test.tsapps/desktop/src/renderer/onboarding/docsLinks.tsapps/desktop/src/renderer/onboarding/stepBuilders/stepBuilders.test.tsapps/desktop/src/renderer/onboarding/tours/firstJourneyTour.test.tsapps/desktop/src/renderer/onboarding/tours/highlights.test.tsapps/desktop/src/renderer/onboarding/tours/lanesTour.test.tsapps/desktop/src/renderer/state/onboardingStore.test.tsapps/desktop/src/renderer/state/onboardingStore.tsapps/desktop/src/shared/types/sync.tsapps/ios/ADE/App/ContentView.swiftapps/ios/ADE/Assets.xcassets/BrandMark.imageset/Contents.jsonapps/ios/ADE/Models/RemoteModels.swiftapps/ios/ADE/Services/Database.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Views/AttentionDrawer/AttentionDrawerButton.swiftapps/ios/ADE/Views/Components/ADEDesignSystem.swiftapps/ios/ADE/Views/Work/WorkRootScreen.swiftapps/ios/ADETests/ADETests.swiftapps/web/index.htmlapps/web/public/mockup.htmlapps/web/src/app/layout/SiteLayout.tsxapps/web/src/app/pages/HomePage.tsxapps/web/src/components/FeatureGallery.tsxapps/web/src/components/MultiDeviceShowcase.tsxapps/web/src/components/ProductShowcase.tsxapps/web/src/components/ProviderOrbit.tsxapps/web/src/components/editorial/AnnotatedFigure.tsxapps/web/src/components/editorial/BackCover.tsxapps/web/src/components/editorial/Chapter.tsxapps/web/src/components/editorial/ChapterHeadline.tsxapps/web/src/components/editorial/CompetitorEquation.tsxapps/web/src/components/editorial/Cutout.tsxapps/web/src/components/editorial/DeviceComposition.tsxapps/web/src/components/editorial/FadeBand.tsxapps/web/src/components/editorial/FeatureGrid.tsxapps/web/src/components/editorial/IPhoneFrame.tsxapps/web/src/components/editorial/IndexPage.tsxapps/web/src/components/editorial/Lede.tsxapps/web/src/components/editorial/Masthead.tsxapps/web/src/components/editorial/PullQuote.tsxapps/web/src/components/ui/ImageAutoSlider.tsxapps/web/src/styles/globals.css
💤 Files with no reviewable changes (6)
- apps/web/src/components/ui/ImageAutoSlider.tsx
- apps/web/src/components/ProductShowcase.tsx
- apps/web/src/components/ProviderOrbit.tsx
- apps/web/src/components/MultiDeviceShowcase.tsx
- apps/desktop/src/renderer/onboarding/tours/firstJourneyTour.test.ts
- apps/web/src/components/FeatureGallery.tsx
Comment on lines
+10
to
+51
| <div className="mx-auto flex max-w-[1240px] items-center gap-6 px-[clamp(20px,3vw,40px)] py-[11px]"> | ||
| <a href="/" className="flex items-center gap-2" aria-label="ADE home"> | ||
| <img | ||
| src="/images/ade-wordmark.png" | ||
| alt="ADE" | ||
| className="h-[22px] w-auto" | ||
| style={{ filter: "brightness(1.05)" }} | ||
| /> | ||
| </a> | ||
|
|
||
| <div className="flex-1 text-center"> | ||
| <span className="text-[11px] uppercase tracking-[0.22em] text-[color:var(--color-cream-muted)]"> | ||
| Vol. 1 · v1.1.0 · Apr 2026 | ||
| </span> | ||
| </div> | ||
|
|
||
| <nav className="flex items-center gap-5 text-[12px] uppercase tracking-[0.14em]"> | ||
| <a | ||
| href={LINKS.docs} | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| className="text-[color:var(--color-cream-muted)] transition-colors hover:text-[color:var(--color-cream)]" | ||
| > | ||
| Docs | ||
| </a> | ||
| <a | ||
| href={LINKS.github} | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| className="text-[color:var(--color-cream-muted)] transition-colors hover:text-[color:var(--color-cream)]" | ||
| > | ||
| GitHub | ||
| </a> | ||
| <a | ||
| href={LINKS.releases} | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| className="text-[color:var(--color-violet-bright)] transition-colors hover:text-[color:var(--color-cream)]" | ||
| > | ||
| Download <span aria-hidden>↓</span> | ||
| </a> | ||
| </nav> |
There was a problem hiding this comment.
Make the replacement masthead responsive.
Because the global mobile header is hidden on /, this masthead becomes the only home navigation. The issue label plus three nav links stay in one row and can overflow on narrow screens.
📱 Proposed responsive adjustment
- <div className="mx-auto flex max-w-[1240px] items-center gap-6 px-[clamp(20px,3vw,40px)] py-[11px]">
+ <div className="mx-auto flex max-w-[1240px] items-center gap-4 px-[clamp(20px,3vw,40px)] py-[11px] sm:gap-6">
<a href="/" className="flex items-center gap-2" aria-label="ADE home">
@@
- <div className="flex-1 text-center">
+ <div className="hidden flex-1 text-center md:block">
@@
- <nav className="flex items-center gap-5 text-[12px] uppercase tracking-[0.14em]">
+ <nav className="ml-auto flex shrink-0 items-center gap-3 text-[11px] uppercase tracking-[0.12em] sm:gap-5 sm:text-[12px] sm:tracking-[0.14em]">As per coding guidelines, apps/web/**: Web app — check for accessibility, SSR compatibility, and proper React patterns.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div className="mx-auto flex max-w-[1240px] items-center gap-6 px-[clamp(20px,3vw,40px)] py-[11px]"> | |
| <a href="/" className="flex items-center gap-2" aria-label="ADE home"> | |
| <img | |
| src="/images/ade-wordmark.png" | |
| alt="ADE" | |
| className="h-[22px] w-auto" | |
| style={{ filter: "brightness(1.05)" }} | |
| /> | |
| </a> | |
| <div className="flex-1 text-center"> | |
| <span className="text-[11px] uppercase tracking-[0.22em] text-[color:var(--color-cream-muted)]"> | |
| Vol. 1 · v1.1.0 · Apr 2026 | |
| </span> | |
| </div> | |
| <nav className="flex items-center gap-5 text-[12px] uppercase tracking-[0.14em]"> | |
| <a | |
| href={LINKS.docs} | |
| target="_blank" | |
| rel="noreferrer" | |
| className="text-[color:var(--color-cream-muted)] transition-colors hover:text-[color:var(--color-cream)]" | |
| > | |
| Docs | |
| </a> | |
| <a | |
| href={LINKS.github} | |
| target="_blank" | |
| rel="noreferrer" | |
| className="text-[color:var(--color-cream-muted)] transition-colors hover:text-[color:var(--color-cream)]" | |
| > | |
| GitHub | |
| </a> | |
| <a | |
| href={LINKS.releases} | |
| target="_blank" | |
| rel="noreferrer" | |
| className="text-[color:var(--color-violet-bright)] transition-colors hover:text-[color:var(--color-cream)]" | |
| > | |
| Download <span aria-hidden>↓</span> | |
| </a> | |
| </nav> | |
| <div className="mx-auto flex max-w-[1240px] items-center gap-4 px-[clamp(20px,3vw,40px)] py-[11px] sm:gap-6"> | |
| <a href="/" className="flex items-center gap-2" aria-label="ADE home"> | |
| <img | |
| src="/images/ade-wordmark.png" | |
| alt="ADE" | |
| className="h-[22px] w-auto" | |
| style={{ filter: "brightness(1.05)" }} | |
| /> | |
| </a> | |
| <div className="hidden flex-1 text-center md:block"> | |
| <span className="text-[11px] uppercase tracking-[0.22em] text-[color:var(--color-cream-muted)]"> | |
| Vol. 1 · v1.1.0 · Apr 2026 | |
| </span> | |
| </div> | |
| <nav className="ml-auto flex shrink-0 items-center gap-3 text-[11px] uppercase tracking-[0.12em] sm:gap-5 sm:text-[12px] sm:tracking-[0.14em]"> | |
| <a | |
| href={LINKS.docs} | |
| target="_blank" | |
| rel="noreferrer" | |
| className="text-[color:var(--color-cream-muted)] transition-colors hover:text-[color:var(--color-cream)]" | |
| > | |
| Docs | |
| </a> | |
| <a | |
| href={LINKS.github} | |
| target="_blank" | |
| rel="noreferrer" | |
| className="text-[color:var(--color-cream-muted)] transition-colors hover:text-[color:var(--color-cream)]" | |
| > | |
| GitHub | |
| </a> | |
| <a | |
| href={LINKS.releases} | |
| target="_blank" | |
| rel="noreferrer" | |
| className="text-[color:var(--color-violet-bright)] transition-colors hover:text-[color:var(--color-cream)]" | |
| > | |
| Download <span aria-hidden>↓</span> | |
| </a> | |
| </nav> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/editorial/Masthead.tsx` around lines 10 - 51, The
Masthead component's header (the outer div, the center volume span, and the nav
anchors in nav) currently forces a single-row layout and can overflow on narrow
screens; update the container and nav to be responsive by allowing wrapping
(e.g., add flex-wrap to the outer flex container and/or nav), reduce spacing and
font size at small breakpoints (use responsive utility classes or media queries
for the span and nav items), and change alignment so items stack vertically or
center when space is limited; ensure links keep their aria-labels and retain
SSR/React patterns used in Masthead so accessibility and server rendering are
preserved.
| export const docs = { | ||
| // Root / welcome | ||
| home: DOCS_BASE, | ||
| home: SITE_BASE, |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
This change redefines docs.home to https://www.ade-app.dev, but docs.home is the target used by the desktop Help menu and onboarding settings when the user clicks “ADE docs”. The repo’s own web config still treats https://www.ade-app.dev/docs as the docs entrypoint (apps/web/src/lib/links.ts, apps/web/vercel.json), so this regresses those entrypoints from the docs index to the marketing homepage. Point docs.home back to the Mintlify base (DOCS_BASE) and update the companion test that now asserts the site root.
// apps/desktop/src/renderer/onboarding/docsLinks.ts
const SITE_BASE = "https://www.ade-app.dev";
const DOCS_BASE = `${SITE_BASE}/docs`;
export const docs = {
home: SITE_BASE,
| || requirement === "video_recording" | ||
| ); | ||
| if (requiresBrowserEvidence && descriptor) { | ||
| const likelyBrowserCapable = descriptor.capabilities.tools && descriptor.capabilities.vision; |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
This new capability gate hard-codes every browser/screenshot proof requirement to tools && vision, which makes preflight fail otherwise valid mission configs that rely on external proof capture instead of model vision. For example, the registry includes selectable tool-capable/non-vision models like openai/gpt-5.3-codex-spark, and dynamic OpenCode descriptors default to vision: false; those models can still drive agent-browser/other external backends, and the runtime prompt already tells workers to prefer external backends for screenshot, browser_trace, and related artifacts. Turning that into a hard preflight failure blocks launches that the rest of ADE would allow. Fix by basing the check on actual proof backend/tool availability, or at minimum avoid requiring vision for proof kinds that can be satisfied by external automation.
// apps/desktop/src/main/services/missions/missionPreflightService.ts
if (requiresBrowserEvidence && descriptor) {
const likelyBrowserCapable = descriptor.capabilities.tools && descriptor.capabilities.vision;
if (!likelyBrowserCapable) {
const message = `${phase.name}: requires browser/screenshot evidence, but ${descriptor.displayName} does not advertise both tools and vision support.`;
| } else if let match = mergedById.first(where: { entry in | ||
| let remote = entry.value | ||
| guard let left = remote.rootPath, let right = cachedProject.rootPath else { return false } | ||
| return left == right |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
refreshProjectCatalog() re-keys a cached project onto a remote catalog entry only when the raw rootPath strings are identical, even though this same service already treats project roots as normalized elsewhere (activeProjectRootPath, normalizedProjectRoot). That means a cached row like /tmp/project-two/ and a live desktop catalog entry /tmp/project-two survive as two separate projects entries after connect, so the project home can show duplicates for the same desktop project and the active/switching state can attach to the wrong row. Normalize both sides before comparing (and keep the other raw root-path checks in this flow consistent with that rule).
// apps/ios/ADE/Services/SyncService.swift
} else if let match = mergedById.first(where: { entry in
let remote = entry.value
guard let left = remote.rootPath, let right = cachedProject.rootPath else { return false }
return left == right
}) {
| label: "Workspace graph", | ||
| blurb: "See the shape of your repo — files, imports, and open lanes.", | ||
| src: "/images/features/workspacegraph.png", | ||
| docs: "/tools/workspace", |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
The new editorial homepage hard-codes several docs slugs that do not exist in the current docs site. For example, https://www.ade-app.dev/docs/tools/workspace now returns the live docs 404 page, while both the repo content and production nav expose tools/workspace-graph.mdx at /docs/tools/workspace-graph; the same mismatch pattern is repeated in @apps/web/src/components/editorial/IndexPage.tsx for routes like /tools/files, /tools/git, /tools/prs, /configuration/models, and /cto/linear-sync, while the repo still contains /tools/files-editor, /tools/history, /tools/pull-requests, /configuration/ai-providers, and /cto/linear. That means users who click the redesigned landing page's docs CTAs are sent to dead pages instead of the referenced feature docs. Update these entries to the existing slugs or add matching redirects before shipping.
// apps/web/src/components/editorial/FeatureGrid.tsx
{
label: "Workspace graph",
blurb: "See the shape of your repo — files, imports, and open lanes.",
docs: "/tools/workspace",
}
| { name: "CLI · ade", page: "12", href: "/tools/cli" }, | ||
| { name: "Dispatch", page: "17", href: "/missions/dispatch" }, | ||
| { name: "Figma integration", page: "32", href: "/tools/figma" }, | ||
| { name: "File viewer", page: "05", href: "/tools/files" }, |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
Several of the new homepage docs links were copied to slugs that do not exist in the repo’s docs map, so the landing page now sends users to missing docs pages instead of the feature docs it advertises. For example, docs.json and the checked-in docs files contain tools/files-editor, tools/history, tools/pull-requests, and tools/workspace-graph, but the new editorial components link to /tools/files, /tools/git, /tools/prs, and /tools/workspace instead. The same stale slugs also appear in the feature grid. Update these hardcoded href values to the current docs routes before shipping.
// apps/web/src/components/editorial/IndexPage.tsx
{ name: "File viewer", page: "05", href: "/tools/files" },
{ name: "Git history", page: "07", href: "/tools/git" },
...
{ name: "PR review", page: "20", href: "/tools/prs" },
{ name: "Workspace graph", page: "06", href: "/tools/workspace" },// apps/web/src/components/editorial/FeatureGrid.tsx
docs: "/tools/files",
...
docs: "/tools/git",
...
docs: "/tools/workspace",
| else capabilityWarnings.push(message); | ||
| } | ||
| } | ||
| if (requiresBrowserEvidence && !descriptor) { |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
runPreflight() already accepts models that are present in getAvailabilityAsync() even when they are not resolvable from modelRegistry (availableDirect at lines 267-277), but this new branch turns the same case back into a hard capability failure as soon as a phase requires screenshot/browser evidence. That means a mission using a runtime-discovered model ID (for example a local/OpenCode model that is available at runtime but has no registry descriptor) will still be blocked even when agent-browser or another proof backend can satisfy the evidence contract. This reintroduces the exact false-negative launch block the preflight change is meant to avoid. Skip the unresolved-model failure when backend/local proof coverage already exists, or derive capability checks from the runtime availability record instead.
// apps/desktop/src/main/services/missions/missionPreflightService.ts
if (requiresBrowserEvidence && !descriptor) {
const message = `${phase.name}: requires browser/screenshot evidence, but model ${phase.model.modelId} could not be resolved for capability checks.`;
if ((phase.validationGate.capabilityFallback ?? "block") === "block") capabilityIssues.push(message);
else capabilityWarnings.push(message);
}
Suggested change
| if (requiresBrowserEvidence && !descriptor) { | |
| if (requiresBrowserEvidence && !descriptor && !browserEvidenceCoveredByBackend) { |
| if (name === "get_environment_info") { | ||
| const includeDisplays = asBoolean(toolArgs.includeDisplays, false); | ||
| const capabilities = getLocalComputerUseCapabilities(); | ||
| const capabilities = ensureLocalComputerUse(name, "environmentInfo"); |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
get_environment_info now goes through ensureLocalComputerUse, which throws whenever environmentInfo.available is false: ts // apps/ade-cli/src/adeRpcServer.ts if (name === "get_environment_info") { const includeDisplays = asBoolean(toolArgs.includeDisplays, false); const capabilities = ensureLocalComputerUse(name, "environmentInfo"); const frontmostApp = capabilities.environmentInfo.available That changes this read-only diagnostic tool from returning structured blocked_by_capability / missing state to hard-failing on Linux or on macOS hosts where osascript is unavailable, even though @AGENTS.md says computer-use features should gracefully degrade off macOS. The regression is user-facing because ade proof environment can no longer inspect why local computer use is unavailable for an authorized session. Keep the new policyDenied check for unauthorized callers, but do not treat missing local capability as fatal for this inspection endpoint.
Suggested change
| const capabilities = ensureLocalComputerUse(name, "environmentInfo"); | |
| const capabilities = isLocalComputerUseAllowed(callerCtx) ? getLocalComputerUseCapabilities() : ensureLocalComputerUse(name, "environmentInfo"); |
| if let activeProjectIdOverride { | ||
| return activeProjectIdOverride | ||
| } | ||
| return queryString("select id from projects order by created_at asc limit 1") |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
When activeProjectIdOverride is nil, this new fallback silently selects the oldest project row instead of the most recently used one, which makes the app derive cached state from the wrong project before the user finishes project selection. In this PR, SyncService.init() now restores any saved selection and then unconditionally calls refreshActiveSessionsAndSnapshot(), and that path feeds database.fetchSessions() plus writeWorkspaceSnapshotNow()/database.fetchPullRequestListItems() into the shared widget/live-activity snapshot. On a fresh install or cleared selection state with multiple cached projects, the phone can therefore show sessions/PRs from an unrelated older desktop project immediately on launch. swift // apps/ios/ADE/Services/Database.swift func currentProjectId() -> String? { if let activeProjectIdOverride { return activeProjectIdOverride } return queryString("select id from projects order by created_at asc limit 1") } Order the fallback by recency (last_opened_at, then created_at) so the implicit selection matches the project-home MRU behavior.
Suggested change
| return queryString("select id from projects order by created_at asc limit 1") | |
| return queryString("select id from projects order by last_opened_at desc, created_at desc limit 1") |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/ios/ADE/Services/Database.swift (6)
1830-1858:⚠️ Potential issue | 🟠 MajorFilter integration proposals by
currentProjectId().
integration_proposalshasproject_id, but this reader has no active-project guard orwhere project_id = ?. After switching projects, the mobile UI can surface proposals from another cached project.Proposed project filter
func fetchIntegrationProposals() -> [IntegrationProposal] { + guard let projectId = currentProjectId() else { return [] } let sql = """ @@ from integration_proposals + where project_id = ? order by created_at desc """ - return query(sql) { statement in + return query(sql, bind: { [self] statement in + try self.bindText(projectId, to: statement, index: 1) + }) { statement in🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/Database.swift` around lines 1830 - 1858, The query in fetchIntegrationProposals reads from integration_proposals without scoping to the active project, causing proposals from other projects to appear; update fetchIntegrationProposals to guard on currentProjectId() and add a WHERE project_id = ? clause (or equivalent parameter binding) to the SQL and pass currentProjectId() as the bound parameter before executing the query so only proposals for the active project are returned.
696-708:⚠️ Potential issue | 🟠 MajorKeep lane detail snapshot cleanup scoped to the active project.
Line 698 and Line 701 delete
lane_detail_snapshotsglobally. Hydrating project B can drop cached lane details for project A, which breaks the multi-project cache isolation this PR adds.Proposed scoped cleanup
- let laneIds = orderedLanes.map(\.id) - if laneIds.isEmpty { - try exec("delete from lane_detail_snapshots") + if orderedLaneIds.isEmpty { + _ = try execute(""" + delete from lane_detail_snapshots + where lane_id in (select id from lanes where project_id = ?) + """) { statement in + try bindText(projectId, to: statement, index: 1) + } } else { - try exec(""" + _ = try execute(""" delete from lane_detail_snapshots - where not exists ( - select 1 - from temp_hydrated_lane_ids hydrated - where hydrated.id = lane_detail_snapshots.lane_id - ) - """) + where lane_id in (select id from lanes where project_id = ?) + and not exists ( + select 1 + from temp_hydrated_lane_ids hydrated + where hydrated.id = lane_detail_snapshots.lane_id + ) + """) { statement in + try bindText(projectId, to: statement, index: 1) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/Database.swift` around lines 696 - 708, The delete currently removes lane_detail_snapshots globally; restrict it to the active project by scoping the DELETE with the project identifier used for hydration. Update the exec SQL (the branches that delete from lane_detail_snapshots and the branch using temp_hydrated_lane_ids) to include a condition that matches the current project (e.g., lane_detail_snapshots.project_id = :currentProjectId or join/filter by hydrated.project_id), and pass the project id from the same context that produced orderedLanes so deletions only affect snapshots for that project.
1917-1936:⚠️ Potential issue | 🟠 MajorFilter queue landing state by the active project.
queue_landing_stateincludesproject_idand even has a project index, but this fetch reads every queue. Project switching can therefore show queue state from another desktop project.Proposed project-scoped queue query
func fetchQueueStates() -> [QueueLandingState] { + guard let projectId = currentProjectId() else { return [] } let sql = """ @@ from queue_landing_state q - left join pr_groups g on g.id = q.group_id + left join pr_groups g on g.id = q.group_id and g.project_id = q.project_id + where q.project_id = ? order by q.updated_at desc, q.started_at desc """ - return query(sql) { statement in + return query(sql, bind: { [self] statement in + try self.bindText(projectId, to: statement, index: 1) + }) { statement in🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/Database.swift` around lines 1917 - 1936, The fetchQueueStates function currently selects every row from queue_landing_state causing cross-project leakage; modify the SQL in fetchQueueStates to include a WHERE q.project_id = ? (or named parameter) and pass the current project's id into the query so results are scoped to the active project; update the fetchQueueStates call sites if needed to provide the project id or read the active project id from the same context used elsewhere (refer to function fetchQueueStates and the queue_landing_state.project_id column and its index).
1648-1662:⚠️ Potential issue | 🟡 MinorScope PR auxiliary joins to the active PR’s project.
The base PR rows are filtered by
pr.project_id, but Lines 1650-1661 can still attach group or integration metadata from projection rows whoseproject_iddiffers. Keep those joins project-constrained so stale projection rows cannot leak workflow state across projects.Proposed join constraints
- left join pr_group_members gm on gm.pr_id = pr.id - left join pr_groups g on g.id = gm.group_id + left join pr_group_members gm on gm.pr_id = pr.id + left join pr_groups g on g.id = gm.group_id and g.project_id = pr.project_id @@ - ) group_counts on group_counts.group_id = gm.group_id + ) group_counts on group_counts.group_id = g.id @@ - ? "left join integration_proposals ip on ip.linked_pr_id = pr.id" + ? "left join integration_proposals ip on ip.linked_pr_id = pr.id and ip.project_id = pr.project_id"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/Database.swift` around lines 1648 - 1662, The prGroupJoins and integrationJoin blocks can attach rows from other projects; modify the join predicates in the prGroupJoins string (references: pr_group_members gm, pr_groups g, group_counts) to include a project constraint matching pr.project_id (e.g. join on gm.pr_id = pr.id AND gm.project_id = pr.project_id and likewise ensure g and group_counts are scoped to gm.project_id or pr.project_id), and update integrationJoin (integration_proposals ip) to include ip.project_id = pr.project_id (or ip.linked_pr_id = pr.id AND ip.project_id = pr.project_id) so both hasPrGroupContext and hasIntegrationWorkflowContext paths only pull rows from the same project as pr.
1008-1092:⚠️ Potential issue | 🟠 MajorReject PR hydration payloads for a different project before deleting stale rows.
Line 1036 can insert PRs under
pr.projectId, but Line 1092 deletes stale rows using the activeprojectId. If a delayed project-A hydration lands after switching to project B, it can insert A’s PRs while deleting B’s PRs.Proposed mismatch guard
+ let scopedPrs = payload.prs.filter { pr in + pr.projectId.isEmpty || pr.projectId == projectId + } + guard scopedPrs.count == payload.prs.count else { + throw sqliteError("Pull request hydration payload does not match the active project.") + } + let scopedPrIds = Set(scopedPrs.map(\.id)) + - for pr in payload.prs { + for pr in scopedPrs { _ = try execute(""" insert into pull_requests( @@ - try bindText(pr.projectId.isEmpty ? projectId : pr.projectId, to: statement, index: 2) + try bindText(projectId, to: statement, index: 2) @@ - for snapshot in payload.snapshots { + for snapshot in payload.snapshots where scopedPrIds.contains(snapshot.prId) { @@ - try deleteStalePullRequestRows(projectId: projectId, keeping: payload.prs.map(\.id)) + try deleteStalePullRequestRows(projectId: projectId, keeping: scopedPrs.map(\.id))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/Database.swift` around lines 1008 - 1092, This code can insert PRs using pr.projectId while later calling deleteStalePullRequestRows(projectId: projectId), risking cross-project deletion; add a guard at the start of the hydration handler to reject the payload when payload.projectId (or any non-empty pr.projectId values) does not match the active projectId: check payload.projectId != "" && payload.projectId != projectId and/or ensure all payload.prs[].projectId (if set) equal projectId, and if any mismatch throw or return an error before performing inserts into pull_requests, pull_request_snapshots, or calling deleteStalePullRequestRows; update the handler surrounding payload.prs, payload.snapshots, and the deleteStalePullRequestRows call to enforce this guard.
1161-1245:⚠️ Potential issue | 🟠 MajorRequire an active project and scope incoming workspaces before writing.
Line 1163 allows
projectIdto benil, and Lines 1216-1245 upsert every incoming workspace even when its lane/root does not belong to the active project. The stale-delete path is scoped, but the write path is not.Proposed active-project guard and scoped writes
func replaceFilesWorkspaces(_ workspaces: [FilesWorkspace]) throws { guard tableExists("files_workspaces") else { return } - let projectId = currentProjectId() - let projectRoot = projectId.flatMap { id in + guard let projectId = currentProjectId() else { + throw sqliteError(SyncHydrationMessaging.waitingForProjectData) + } + let projectRoot = queryString("select root_path from projects where id = ? limit 1", bind: { [self] statement in + try self.bindText(projectId, to: statement, index: 1) + }) - queryString("select root_path from projects where id = ? limit 1", bind: { [self] statement in - try self.bindText(id, to: statement, index: 1) - }) - } let activeLaneIds = Set(query("select id from lanes where project_id = ?", bind: { [self] statement in - if let projectId { - try self.bindText(projectId, to: statement, index: 1) - } else { - sqlite3_bind_null(statement, 1) - } + try self.bindText(projectId, to: statement, index: 1) }) { statement in stringValue(statement, index: 0) ?? "" }) + let scopedWorkspaces = workspaces.filter { workspace in + if let laneId = workspace.laneId { + return activeLaneIds.contains(laneId) + } + guard workspace.kind == "primary", let projectRoot else { return false } + return workspace.rootPath == projectRoot + } try exec("begin immediate") do { - let incomingIds = Set(workspaces.map(\.id)) + let incomingIds = Set(scopedWorkspaces.map(\.id)) @@ - for workspace in workspaces { + for workspace in scopedWorkspaces {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/Database.swift` around lines 1161 - 1245, The replaceFilesWorkspaces function currently allows a nil projectId and unconditionally upserts all incoming workspaces; change it to require an active project (guard let projectId / projectRoot) and before the upsert loop filter the incoming workspaces to a scopedWorkspaces set that only includes workspaces whose laneId is in activeLaneIds or whose kind == "primary" and rootPath == projectRoot, then iterate over scopedWorkspaces for the insert/update and only delete staleIds computed against scopedExistingIds as already done; reference replaceFilesWorkspaces, projectId, projectRoot, activeLaneIds, scopedExistingIds, staleIds and the workspaces insertion loop.
🧹 Nitpick comments (4)
apps/desktop/src/main/services/missions/missionPreflightService.ts (2)
754-768: Optional: flatten thesummarynested ternary for readability.The indentation on lines 762-767 drops a level between the
warningMissingComputerUseKindsandfallbackOnlyKindsbranches, which reads like a parse-level fall-through even though the semantics are correct. Extracting this into a small helper (or a sequence ofif/early-return statements building the string) would make the precedence ofblocked > warning-only > fallback-only > external-backendobvious at a glance and keep it in sync with the matching branching at line 331-339 in the checklist item.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/missions/missionPreflightService.ts` around lines 754 - 768, The nested ternary in the computerUse.summary field is hard to read; replace it with a small helper or sequential if/return logic that enforces the precedence blocked > warning-only > fallback-only > external-backend. Locate the computerUse object (the summary property) and implement a function like computeComputerUseSummary(requiredComputerUseKinds, computerUseBlocked, blockingMissingComputerUseKinds, warningMissingComputerUseKinds, fallbackOnlyKinds, availableExternalBackends) or inline if/else that returns the same strings: first return the no-requirement message when requiredComputerUseKinds.length === 0, then if computerUseBlocked return the blocked message using blockingMissingComputerUseKinds (or fall back to requiredComputerUseKinds), then if warningMissingComputerUseKinds.length > 0 return the warning message, then if fallbackOnlyKinds.length > 0 return the fallback-only message, otherwise return the external-backends message using availableExternalBackends.join(", ").
367-400: Capability gate now correctly honors backend coverage and fallback policy — LGTM.
browserEvidenceCoveredByBackendgates both the tools/vision descriptor check (line 373-374) and the unresolved-descriptor check (line 381), resolving the two earlier false-negatives for external-backend-satisfied proof and runtime-discovered models. Per-requirement messages at line 386-399 route viacapabilityFallbackcorrectly.Minor: at line 391,
localCapability = backendStatus ? null : getCapabilityForRequirement(requirementKind). WhenbackendStatusis present but itslocalFallbackdoesn't cover the kind,localCapabilityis forced tonull, solocalStateReason(lines 393-395) always resolves to"missing required tooling"— the"blocked by platform support"reason is effectively unreachable whenevercomputerUseArtifactBrokerServiceis wired. If the finer-grained signal matters in the broker-present path, consider also consultinggetCapabilityForRequirement(requirementKind)as a fallback for the reason string only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/missions/missionPreflightService.ts` around lines 367 - 400, The localCapability assignment currently sets localCapability = backendStatus ? null : getCapabilityForRequirement(requirementKind) which makes the "blocked_by_capability" reason unreachable when backendStatus exists; change this so when backendStatus exists but its localFallback does not cover the requirementKind you still consult getCapabilityForRequirement(requirementKind) (or at least use its .state for determining localStateReason) — i.e., compute localCapability = backendStatus ? (backendStatus.localFallback?.coversRequirement ? null : getCapabilityForRequirement(requirementKind)) : getCapabilityForRequirement(requirementKind), then keep the existing localDetail/localStateReason logic that checks localCapability?.state === "blocked_by_capability" to produce the correct reason string.apps/desktop/src/main/services/sync/syncHostService.ts (1)
1056-1079: Protocol-level validation for project switch requests is optional defensive programming, not required.
SyncProjectSwitchRequestPayloadfields are optional, andprepareMobileSyncProjectConnectionsafely handles empty or missing payloads. When bothprojectIdandrootPathare null/undefined, the provider correctly returns an error response ("That project is not available from this desktop.") rather than causing undefined behavior. Adding explicit validation at the handler boundary would be a defensive improvement, but it is not required to prevent a security issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/sync/syncHostService.ts` around lines 1056 - 1079, Remove any protocol-level validation in handleProjectSwitchRequest that rejects requests when SyncProjectSwitchRequestPayload fields (e.g., projectId/rootPath) are missing; instead, forward the raw payload to args.projectCatalogProvider.prepareProjectConnection (as currently done) and let that provider return the appropriate error response. Ensure no added checks on payload properties remain in handleProjectSwitchRequest so prepareProjectConnection handles invalid or empty payloads.apps/ios/ADE/Services/SyncService.swift (1)
3644-3655: Redundantproject_catalog_requestright after hello.
applyHelloPayloadalready decodesprojectsfrom thehello_okenvelope (lines 3716‑3719) and feeds them throughapplyRemoteProjectCatalog. Unconditionally callingrefreshRemoteProjectCatalog()here then fires a secondproject_catalog_requestround‑trip for the same data on every successful connect/reconnect. Consider gating this on the hello payload not having suppliedprojects, or dropping it and relying on the hello‑provided catalog plus the on‑demand refresh inshowProjectHome().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/SyncService.swift` around lines 3644 - 3655, applyHelloPayload already decodes projects from the hello_ok envelope and calls applyRemoteProjectCatalog, so refreshRemoteProjectCatalog() right after connect causes a redundant project_catalog_request; update the connect flow to skip calling refreshRemoteProjectCatalog() when applyHelloPayload provided projects (e.g. detect presence of projects in the hello payload or have applyHelloPayload return a flag/enum indicating whether it applied a catalog) and only call refreshRemoteProjectCatalog() when no projects were included, or remove the unconditional refresh and rely on the hello-provided catalog plus on-demand refresh in showProjectHome(); refer to applyHelloPayload, applyRemoteProjectCatalog, refreshRemoteProjectCatalog, showProjectHome and the hello_ok/projects handling to implement the gate or removal.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/main.ts`:
- Around line 3628-3655: If ensureProjectContextForMobileSync(targetRoot) may
create a new (cold) context that should be cleaned up on failure, wrap the
initialization and status checks so failures clean up newly-created contexts:
record whether the context was newly created (e.g., a boolean newContext =
ctx.isNew || set immediately after calling ensureProjectContextForMobileSync),
then if ctx.syncService.initialize(), ctx.syncService.getStatus(), or readiness
checks throw, if newContext then either call the context teardown method (e.g.,
ctx.close() or ctx.dispose() if present) or remove any references (delete
mobileSyncHandoffLeases/mobileSyncHandoffLeaseTimers and call
scheduleProjectContextRebalance()) before rethrowing the error; ensure you
reference ensureProjectContextForMobileSync, ctx.syncService.initialize,
getStatus, mobileSyncHandoffLeases, mobileSyncHandoffLeaseTimers,
scheduleProjectContextRebalance, and projectLastActivatedAt when implementing
this cleanup.
In `@apps/desktop/src/renderer/components/app/TopBar.tsx`:
- Around line 116-138: The polling currently swallows errors from
window.ade.sync.getStatus() so stale sync UI can remain; update the
refreshSyncStatus handler used in the useEffect (the function that calls
window.ade.sync.getStatus()) to setSyncSnapshot(null) when the promise rejects
(or otherwise on catch), ensuring the UI is cleared on failure; keep the
existing interval, focus listener and dispose logic (onEvent "sync-status"
handler) intact and only add the snapshot-clear in the catch path of
getStatus().
In `@apps/desktop/src/renderer/state/onboardingStore.ts`:
- Around line 154-165: startTour currently proceeds even if getTour(id) returns
undefined, causing activeTourId/activeStepIndex/activeTourCtx to be set and
updateTourStep called for a non-existent tour; change startTour to check the
result of getTour(id) immediately after trimming and return early (or handle
error) if tour is falsy before calling createTourCtx, set(...),
onboarding.updateTourStep, or runBeforeEnter, and only then proceed to
createTourCtx, set active state, call onboarding.updateTourStep, and
runBeforeEnter with tour.steps[0]; reference the startTour function, getTour,
createTourCtx, runBeforeEnter, and onboarding.updateTourStep when making this
guard.
- Around line 194-207: The prevStep handler (prevStep) runs runAfterLeave and
runBeforeEnter even when activeStepIndex is 0, causing no-op navigations to
re-run step teardown/setup; modify prevStep to early-return when activeStepIndex
<= 0 (or when nextIndex === activeStepIndex) before calling
runAfterLeave/runBeforeEnter and before updating API progress, so no lifecycle
hooks run if there is no actual step change (refer to prevStep, runAfterLeave,
runBeforeEnter, activeStepIndex, nextIndex).
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 837-859: When rolling back in the catch block after restoring
activeProjectId/profile/token/catalog, also perform the same local-selection
cleanup that selectProject does: increment the localStateRevision (or call the
helper that bumps it), call refreshActiveSessionsAndSnapshot() to update the
Published activeSessions, and call scheduleWorkspaceSnapshotWrite() to refresh
the App Group/widget snapshot so the UI (Work tab, widgets, attention drawer)
immediately reflects the previous project; place these calls after
refreshProjectCatalog() and before changing connectionState/currentAddress so
the restored state is fully applied before reconnect logic runs.
In `@apps/ios/ADETests/ADETests.swift`:
- Around line 530-606: The fetchIntegrationProposals method in Database.swift
does not filter by currentProjectId() and will return proposals from all
projects; to fix add regression coverage in
testDatabaseScopesPullRequestReadsByActiveProject by inserting rows into the
integration_proposals table for both 'project-1' and 'project-2' (matching the
seeded PR ids), then after database.setActiveProjectId("project-1") assert that
database.fetchIntegrationProposals() returns only proposals for project-1, and
after setActiveProjectId("project-2") assert it returns only project-2
proposals; alternatively ensure fetchIntegrationProposals itself is updated to
include "where project_id = ?" using currentProjectId() to enforce project
scoping.
In `@apps/web/src/components/editorial/BackCover.tsx`:
- Around line 75-91: The two CTA anchor elements that use target="_blank" (the
anchor linking to LINKS.releases with the Download icon and the anchor linking
to LINKS.github with the Github icon) must expose the new-tab behavior to
assistive tech; update each anchor to include either an explicit aria-label that
appends "opens in a new tab" (e.g., aria-label="Download DMG, opens in a new
tab") or add an inline visually-hidden span (screen-reader-only text) such as
<span className="sr-only"> (or your project's existing hidden utility)
containing "opens in a new tab" after the visible text; keep the existing
target/rel attributes and ensure the change is plain JSX (SSR-safe) and applied
to both the LINKS.releases and LINKS.github anchors so screen readers receive
the cue.
In `@apps/web/src/components/editorial/CompetitorEquation.tsx`:
- Around line 26-28: The rowEnd calculation only multiplies stagger by
COMPETITORS.length but ignores separators/chips between competitors; update
rowEnd to use the total child count (competitors plus separators) i.e.
(COMPETITORS.length * 2 - 1) so it reads something like rowEnd = 0.1 +
((COMPETITORS.length * 2 - 1) * stagger) to ensure Row 2 starts after all Row 1
children animate.
---
Outside diff comments:
In `@apps/ios/ADE/Services/Database.swift`:
- Around line 1830-1858: The query in fetchIntegrationProposals reads from
integration_proposals without scoping to the active project, causing proposals
from other projects to appear; update fetchIntegrationProposals to guard on
currentProjectId() and add a WHERE project_id = ? clause (or equivalent
parameter binding) to the SQL and pass currentProjectId() as the bound parameter
before executing the query so only proposals for the active project are
returned.
- Around line 696-708: The delete currently removes lane_detail_snapshots
globally; restrict it to the active project by scoping the DELETE with the
project identifier used for hydration. Update the exec SQL (the branches that
delete from lane_detail_snapshots and the branch using temp_hydrated_lane_ids)
to include a condition that matches the current project (e.g.,
lane_detail_snapshots.project_id = :currentProjectId or join/filter by
hydrated.project_id), and pass the project id from the same context that
produced orderedLanes so deletions only affect snapshots for that project.
- Around line 1917-1936: The fetchQueueStates function currently selects every
row from queue_landing_state causing cross-project leakage; modify the SQL in
fetchQueueStates to include a WHERE q.project_id = ? (or named parameter) and
pass the current project's id into the query so results are scoped to the active
project; update the fetchQueueStates call sites if needed to provide the project
id or read the active project id from the same context used elsewhere (refer to
function fetchQueueStates and the queue_landing_state.project_id column and its
index).
- Around line 1648-1662: The prGroupJoins and integrationJoin blocks can attach
rows from other projects; modify the join predicates in the prGroupJoins string
(references: pr_group_members gm, pr_groups g, group_counts) to include a
project constraint matching pr.project_id (e.g. join on gm.pr_id = pr.id AND
gm.project_id = pr.project_id and likewise ensure g and group_counts are scoped
to gm.project_id or pr.project_id), and update integrationJoin
(integration_proposals ip) to include ip.project_id = pr.project_id (or
ip.linked_pr_id = pr.id AND ip.project_id = pr.project_id) so both
hasPrGroupContext and hasIntegrationWorkflowContext paths only pull rows from
the same project as pr.
- Around line 1008-1092: This code can insert PRs using pr.projectId while later
calling deleteStalePullRequestRows(projectId: projectId), risking cross-project
deletion; add a guard at the start of the hydration handler to reject the
payload when payload.projectId (or any non-empty pr.projectId values) does not
match the active projectId: check payload.projectId != "" && payload.projectId
!= projectId and/or ensure all payload.prs[].projectId (if set) equal projectId,
and if any mismatch throw or return an error before performing inserts into
pull_requests, pull_request_snapshots, or calling deleteStalePullRequestRows;
update the handler surrounding payload.prs, payload.snapshots, and the
deleteStalePullRequestRows call to enforce this guard.
- Around line 1161-1245: The replaceFilesWorkspaces function currently allows a
nil projectId and unconditionally upserts all incoming workspaces; change it to
require an active project (guard let projectId / projectRoot) and before the
upsert loop filter the incoming workspaces to a scopedWorkspaces set that only
includes workspaces whose laneId is in activeLaneIds or whose kind == "primary"
and rootPath == projectRoot, then iterate over scopedWorkspaces for the
insert/update and only delete staleIds computed against scopedExistingIds as
already done; reference replaceFilesWorkspaces, projectId, projectRoot,
activeLaneIds, scopedExistingIds, staleIds and the workspaces insertion loop.
---
Nitpick comments:
In `@apps/desktop/src/main/services/missions/missionPreflightService.ts`:
- Around line 754-768: The nested ternary in the computerUse.summary field is
hard to read; replace it with a small helper or sequential if/return logic that
enforces the precedence blocked > warning-only > fallback-only >
external-backend. Locate the computerUse object (the summary property) and
implement a function like computeComputerUseSummary(requiredComputerUseKinds,
computerUseBlocked, blockingMissingComputerUseKinds,
warningMissingComputerUseKinds, fallbackOnlyKinds, availableExternalBackends) or
inline if/else that returns the same strings: first return the no-requirement
message when requiredComputerUseKinds.length === 0, then if computerUseBlocked
return the blocked message using blockingMissingComputerUseKinds (or fall back
to requiredComputerUseKinds), then if warningMissingComputerUseKinds.length > 0
return the warning message, then if fallbackOnlyKinds.length > 0 return the
fallback-only message, otherwise return the external-backends message using
availableExternalBackends.join(", ").
- Around line 367-400: The localCapability assignment currently sets
localCapability = backendStatus ? null :
getCapabilityForRequirement(requirementKind) which makes the
"blocked_by_capability" reason unreachable when backendStatus exists; change
this so when backendStatus exists but its localFallback does not cover the
requirementKind you still consult getCapabilityForRequirement(requirementKind)
(or at least use its .state for determining localStateReason) — i.e., compute
localCapability = backendStatus ?
(backendStatus.localFallback?.coversRequirement ? null :
getCapabilityForRequirement(requirementKind)) :
getCapabilityForRequirement(requirementKind), then keep the existing
localDetail/localStateReason logic that checks localCapability?.state ===
"blocked_by_capability" to produce the correct reason string.
In `@apps/desktop/src/main/services/sync/syncHostService.ts`:
- Around line 1056-1079: Remove any protocol-level validation in
handleProjectSwitchRequest that rejects requests when
SyncProjectSwitchRequestPayload fields (e.g., projectId/rootPath) are missing;
instead, forward the raw payload to
args.projectCatalogProvider.prepareProjectConnection (as currently done) and let
that provider return the appropriate error response. Ensure no added checks on
payload properties remain in handleProjectSwitchRequest so
prepareProjectConnection handles invalid or empty payloads.
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 3644-3655: applyHelloPayload already decodes projects from the
hello_ok envelope and calls applyRemoteProjectCatalog, so
refreshRemoteProjectCatalog() right after connect causes a redundant
project_catalog_request; update the connect flow to skip calling
refreshRemoteProjectCatalog() when applyHelloPayload provided projects (e.g.
detect presence of projects in the hello payload or have applyHelloPayload
return a flag/enum indicating whether it applied a catalog) and only call
refreshRemoteProjectCatalog() when no projects were included, or remove the
unconditional refresh and rely on the hello-provided catalog plus on-demand
refresh in showProjectHome(); refer to applyHelloPayload,
applyRemoteProjectCatalog, refreshRemoteProjectCatalog, showProjectHome and the
hello_ok/projects handling to implement the gate or removal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 56ebdaee-ddcc-4722-9816-82afe91dd593
📒 Files selected for processing (32)
apps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/desktop/src/main/main.tsapps/desktop/src/main/services/computerUse/computerUseArtifactBrokerService.tsapps/desktop/src/main/services/missions/missionPreflightService.test.tsapps/desktop/src/main/services/missions/missionPreflightService.tsapps/desktop/src/main/services/missions/missionService.test.tsapps/desktop/src/main/services/missions/missionService.tsapps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.tsapps/desktop/src/main/services/sync/syncHostService.tsapps/desktop/src/renderer/components/app/TopBar.tsxapps/desktop/src/renderer/components/onboarding/HelpMenu.tsxapps/desktop/src/renderer/components/onboarding/tour/TourOverlay.tsxapps/desktop/src/renderer/onboarding/docsLinks.test.tsapps/desktop/src/renderer/onboarding/docsLinks.tsapps/desktop/src/renderer/state/onboardingStore.tsapps/desktop/src/shared/types/sync.tsapps/ios/ADE/App/ContentView.swiftapps/ios/ADE/Services/Database.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADETests/ADETests.swiftapps/web/public/mockup.htmlapps/web/src/app/layout/SiteLayout.tsxapps/web/src/app/pages/HomePage.tsxapps/web/src/components/editorial/AnnotatedFigure.tsxapps/web/src/components/editorial/BackCover.tsxapps/web/src/components/editorial/CompetitorEquation.tsxapps/web/src/components/editorial/Cutout.tsxapps/web/src/components/editorial/FeatureGrid.tsxapps/web/src/components/editorial/IndexPage.tsxapps/web/src/components/editorial/Masthead.tsxapps/web/src/styles/globals.css
✅ Files skipped from review due to trivial changes (6)
- apps/desktop/src/main/services/missions/missionService.ts
- apps/web/public/mockup.html
- apps/web/src/styles/globals.css
- apps/web/src/components/editorial/Cutout.tsx
- apps/ade-cli/src/adeRpcServer.ts
- apps/desktop/src/renderer/onboarding/docsLinks.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/web/src/app/layout/SiteLayout.tsx
- apps/desktop/src/renderer/onboarding/docsLinks.test.ts
- apps/ade-cli/src/adeRpcServer.test.ts
- apps/desktop/src/renderer/components/onboarding/HelpMenu.tsx
- apps/web/src/components/editorial/Masthead.tsx
- apps/desktop/src/main/services/computerUse/computerUseArtifactBrokerService.ts
- apps/web/src/components/editorial/IndexPage.tsx
- apps/web/src/components/editorial/AnnotatedFigure.tsx
Comment on lines
+162
to
+163
| if (step.actIntro) handleNext(); | ||
| else handleDismiss(); |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
For actIntro steps this new branch makes TourOverlay call handleNext() on Escape, but the rendered ActIntro already installs its own Escape listener and also calls onSkip={handleNext}. One keypress therefore advances twice, which skips the first real step after each intro (for example the many act*.intro beats in @apps/desktop/src/renderer/onboarding/tours/firstJourneyTour.ts) or can finish the tour early when the intro is last. Let only one layer own Escape for act-intro mode.
// apps/desktop/src/renderer/components/onboarding/tour/TourOverlay.tsx
if (e.key === "Escape") {
e.preventDefault();
if (step.actIntro) handleNext();
else handleDismiss();
}
Suggested change
| if (step.actIntro) handleNext(); | |
| else handleDismiss(); | |
| if (!step.actIntro) handleDismiss(); |
| { name: "CTO (chief technical officer)", page: "15", href: "/cto/overview" }, | ||
| { name: "CLI · ade", page: "12", href: "/tools/project-home" }, | ||
| { name: "Dispatch", page: "17", href: "/missions/creating" }, | ||
| { name: "Figma integration", page: "32", href: "/ai-tools/cursor" }, |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
This new index entry sends users to https://www.ade-app.dev/docs/ai-tools/cursor, and that URL currently resolves to Mintlify's 404 Page Not Found page rather than feature documentation. The rest of this redesign uses the index as a navigation surface, so shipping a dead entry creates a broken outbound link directly from the homepage.
// apps/web/src/components/editorial/IndexPage.tsx
const ENTRIES: Entry[] = [
{ name: "Figma integration", page: "32", href: "/ai-tools/cursor" },
// ...
];
| } | ||
| return (try? decode(actions, as: [SyncRemoteCommandDescriptor].self)) ?? [] | ||
| }() | ||
| if let projects = payload["projects"], |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
remoteProjectCatalog is new persistent state, but hello_ok only replaces it when the payload happens to include projects. If the phone first connects to a desktop that advertises a catalog and then reconnects to a desktop that does not (or an older host on the same pairing path), the old remote-only entries stay merged into projects, so Project Home shows projects from the wrong computer and lets the user tap stale targets. The changed code here is the gap:
// apps/ios/ADE/Services/SyncService.swift
if let projects = payload["projects"],
let catalog = try? decode(["projects": projects], as: MobileProjectCatalogPayload.self) {
applyRemoteProjectCatalog(catalog)
}Clear the remote catalog when hello_ok has no projects field so host changes cannot leak the previous desktop's catalog into the current session.
Suggested change
| if let projects = payload["projects"], | |
| if let projects = payload["projects"], | |
| let catalog = try? decode(["projects": projects], as: MobileProjectCatalogPayload.self) { | |
| applyRemoteProjectCatalog(catalog) | |
| } else { | |
| applyRemoteProjectCatalog(MobileProjectCatalogPayload(projects: [])) | |
| } |
| if (hasLocalComputerUseCoverage(requirementKind)) continue; | ||
| const localCapability = backendStatus ? null : getCapabilityForRequirement(requirementKind); | ||
| const localDetail = backendStatus?.localFallback?.detail ?? localCapability?.detail; | ||
| const localStateReason = localCapability?.state === "blocked_by_capability" |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
When the desktop runs through the normal production path, createMissionPreflightService is instantiated with a real computerUseArtifactBrokerService, so this branch always forces localCapability to null. That means the new capability error can never classify a failure as blocked_by_capability, even though @apps/desktop/src/main/services/computerUse/localComputerUse.ts reports that exact state on non-macOS platforms. ts // apps/desktop/src/main/services/missions/missionPreflightService.ts const localCapability = backendStatus ? null : getCapabilityForRequirement(requirementKind); const localDetail = backendStatus?.localFallback?.detail ?? localCapability?.detail; const localStateReason = localCapability?.state === "blocked_by_capability" ? "blocked by platform support" : "missing required tooling"; On Linux, required proof failures will therefore tell operators that the machine is missing tooling instead of explaining that ADE local capture is unsupported on the platform, which sends them toward the wrong remediation. Preserve the local fallback state in ComputerUseBackendStatus (or re-read getLocalComputerUseCapabilities() here) and derive the message from that state before building the checklist entry.
Comment on lines
+391
to
+395
| const localCapability = backendStatus ? null : getCapabilityForRequirement(requirementKind); | ||
| const localDetail = backendStatus?.localFallback?.detail ?? localCapability?.detail; | ||
| const localStateReason = localCapability?.state === "blocked_by_capability" | ||
| ? "blocked by platform support" | ||
| : "missing required tooling"; |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
When computerUseArtifactBrokerService is wired in production, this branch forces localCapability to null, so the newly added capability diagnostics can never emit the blocked by platform support path even on platforms where getLocalComputerUseCapabilities() returns blocked_by_capability (for example Linux, per @apps/desktop/src/main/services/computerUse/localComputerUse.ts). That means a blocked mission is reported as if the user is merely missing tools, which sends operators toward the wrong remediation for a hard failure.
// apps/desktop/src/main/services/missions/missionPreflightService.ts
const localCapability = backendStatus ? null : getCapabilityForRequirement(requirementKind);
const localDetail = backendStatus?.localFallback?.detail ?? localCapability?.detail;
const localStateReason = localCapability?.state === "blocked_by_capability"
? "blocked by platform support"
: "missing required tooling";Always consulting getCapabilityForRequirement() for the message path preserves the platform-blocked vs tooling-missing distinction without changing the broker coverage logic.
Suggested change
| const localCapability = backendStatus ? null : getCapabilityForRequirement(requirementKind); | |
| const localDetail = backendStatus?.localFallback?.detail ?? localCapability?.detail; | |
| const localStateReason = localCapability?.state === "blocked_by_capability" | |
| ? "blocked by platform support" | |
| : "missing required tooling"; | |
| const localCapability = getCapabilityForRequirement(requirementKind); | |
| const localDetail = localCapability?.detail ?? backendStatus?.localFallback?.detail; | |
| const localStateReason = localCapability?.state === "blocked_by_capability" | |
| ? "blocked by platform support" | |
| : "missing required tooling"; |
| connectionState == .connected || connectionState == .syncing | ||
| else { return } | ||
|
|
||
| guard activeProjectId != nil else { |
There was a problem hiding this comment.
[🟠 High] [🔵 Bug]
On a fresh phone or after clearing the saved selection, connecting to an older desktop that does not advertise projectCatalog now strands the app on Project Home. applyHelloPayload() falls back to refreshProjectCatalog(), which normalizes with allowSingleProjectFallback: false, so no project is auto-selected; then performInitialHydration() exits before requesting any data, and selectProject() refuses manual selection while the legacy host is already connected. The result is a successful socket connection that can never hydrate lanes/work/PRs until the user reconnects to a newer desktop.
// apps/ios/ADE/Services/SyncService.swift
guard activeProjectId != nil else {
refreshProjectCatalog()
return
}
There was a problem hiding this comment.
Actionable comments posted: 13
♻️ Duplicate comments (1)
apps/ios/ADE/Services/SyncService.swift (1)
4507-4518:⚠️ Potential issue | 🟠 MajorDon’t hydrate no-catalog hosts under a stale persisted project ID.
When
activeProjectIdis restored fromUserDefaults, this closure accepts it if the project exists locally, even if the connected host did not advertise a catalog to validate it. The subsequent live refreshes can then write the current host’s data into an old project scope. Make persisted selections host-scoped or clear/rederive the active project before initial hydration whensupportsProjectCatalog == false.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/SyncService.swift` around lines 4507 - 4518, The current closure passed to InitialHydrationGate.waitForProjectRow accepts a persisted activeProjectId from UserDefaults even when the connected host did not advertise a catalog, allowing stale project scopes to be used; change the logic so that when supportsProjectCatalog == false you do not accept a persisted activeProjectId — either clear/derive self.activeProjectId (e.g., set to nil or rederive from database.currentProjectId) before calling InitialHydrationGate.waitForProjectRow or update the closure to also check supportsProjectCatalog and refuse the persisted id if the host has no catalog; ensure you reference activeProjectId, supportsProjectCatalog, InitialHydrationGate.waitForProjectRow, and database.hasProject/database.currentProjectId in the update.
🧹 Nitpick comments (2)
apps/web/src/components/editorial/BackCover.tsx (1)
4-4: Unused import:IPhoneFrame.
IPhoneFrameis imported but never referenced in this file (the iPhone mockup at lines 151–173 is rendered inline with raw divs). Drop the import to avoid dead code / potential lint failures.-import { IPhoneFrame } from "./IPhoneFrame";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/editorial/BackCover.tsx` at line 4, Remove the unused import IPhoneFrame from BackCover.tsx: delete the line importing IPhoneFrame since the component is not referenced (the iPhone mockup uses inline divs at the current render) and ensure no other references to the IPhoneFrame symbol remain in this file so lint errors are resolved.apps/ios/ADETests/ADETests.swift (1)
666-702: Cover project-level workspaces withoutlaneId.This scoping regression only seeds lane-backed workspaces, so it won’t catch stale root/project workspaces leaking across active projects. Add one
laneId: nilworkspace per project and assert only the active project’s root workspace is returned after switching.🧪 Proposed coverage expansion
try database.replaceFilesWorkspaces([ + FilesWorkspace( + id: "workspace-project-one", + kind: "project", + laneId: nil, + name: "Project One", + rootPath: "/tmp/project-one", + isReadOnlyByDefault: true, + mobileReadOnly: true + ), FilesWorkspace( id: "workspace-one", kind: "worktree", laneId: "lane-one", @@ try database.replaceFilesWorkspaces([ + FilesWorkspace( + id: "workspace-project-two", + kind: "project", + laneId: nil, + name: "Project Two", + rootPath: "/tmp/project-two", + isReadOnlyByDefault: true, + mobileReadOnly: true + ), FilesWorkspace( id: "workspace-two", kind: "worktree", laneId: "lane-two", @@ XCTAssertEqual(database.fetchLanes(includeArchived: true).map(\.id), ["lane-two"]) XCTAssertEqual(database.fetchSessions().map(\.id), ["session-two"]) - XCTAssertEqual(database.listWorkspaces().map(\.id), ["workspace-two"]) + XCTAssertEqual(Set(database.listWorkspaces().map(\.id)), Set(["workspace-project-two", "workspace-two"])) database.setActiveProjectId("project-1") XCTAssertEqual(database.fetchLanes(includeArchived: true).map(\.id), ["lane-one"]) XCTAssertEqual(database.fetchSessions().map(\.id), ["session-one"]) - XCTAssertEqual(database.listWorkspaces().map(\.id), ["workspace-one"]) + XCTAssertEqual(Set(database.listWorkspaces().map(\.id)), Set(["workspace-project-one", "workspace-one"]))As per coding guidelines,
apps/ios/**/*.swift: “iOS Swift app — check for memory management, Swift conventions, and proper SwiftUI patterns.”Also applies to: 688-707
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADETests/ADETests.swift` around lines 666 - 702, Add coverage for project-level (root) workspaces missing a laneId by seeding one FilesWorkspace with laneId: nil for each project before switching active project; update the calls around replaceFilesWorkspaces and setActiveProjectId (referencing FilesWorkspace and database.replaceFilesWorkspaces / database.setActiveProjectId) so that after switching you assert database.listWorkspaces() only includes the root workspace for the active project (in addition to existing lane-backed workspace assertions using fetchLanes and fetchSessions); ensure one nil laneId workspace per project is inserted and the expected IDs are asserted after the active project change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/main/main.ts`:
- Around line 3599-3613: The current find uses an OR between requestedRoot and
requestedProjectId so a stale root or id can incorrectly win; change the
matching logic in the catalogEntry lookup (the catalog.projects.find that uses
normalizeProjectRoot, requestedRoot, and requestedProjectId) to require both
identifiers when both are provided (i.e., if requestedRoot != null &&
requestedProjectId != null then require entryRoot === requestedRoot AND entry.id
=== requestedProjectId), otherwise fall back to single-constraint matching as
before; also update the projectContexts fallback loop that sets catalogEntry
(the loop using projectContexts, ctx.projectId and normalizeProjectRoot and
mobileProjectSummaryForContext) to respect requestedRoot when provided so the
chosen context/root agrees with requestedProjectId and requestedRoot.
- Around line 3628-3679: The failure path can clear a lease or close a context
created by a concurrent successful request because hadExistingContext is
captured before awaiting ensureProjectContextForMobileSync; to fix, generate a
unique owner token (e.g., const leaseOwner = Symbol() or random id) before
calling ensureProjectContextForMobileSync, store the owner alongside the lease
in mobileSyncHandoffLeases (e.g., {owner, expiresAt}), set the corresponding
timer entry in mobileSyncHandoffLeaseTimers with that owner, and in the catch
cleanup only remove timers/leases or call closeProjectContext(targetRoot) if the
stored owner matches leaseOwner; alternatively (or additionally) serialize
prepareMobileSyncProjectConnection per targetRoot (a per-target mutex around
ensureProjectContextForMobileSync / ctx.syncService.initialize / getStatus) to
avoid concurrent races.
- Around line 3517-3528: The function mobileProjectSummaryForRecent currently
returns persisted summaries with isCached: false; change it to isCached: true so
persisted "recent" rows are marked as cached and remain distinguishable from
live AppContext summaries; update the return in mobileProjectSummaryForRecent
(which builds a SyncMobileProjectSummary) to set isCached: true (leave other
fields as-is so live context can still overwrite).
In `@apps/desktop/src/main/services/missions/missionPreflightService.ts`:
- Around line 367-385: The predicate computing browserEvidenceCoveredByBackend
is currently checking all evidenceRequirements; narrow it to only the
browser/screenshot-flavored requirements before calling
hasExternalComputerUseCoverage/hasLocalComputerUseCoverage (e.g. filter
evidenceRequirements where kind === "screenshot" or maps to a
browser/computer-use kind), so browserEvidenceCoveredByBackend reflects only
browser-related coverage; update the logic that sets
browserEvidenceCoveredByBackend (and any helpers it uses:
hasExternalComputerUseCoverage, hasLocalComputerUseCoverage) and keep the rest
of the checks that reference requiresBrowserEvidence, descriptor,
likelyBrowserCapable, phase, capabilityWarnings/capabilityIssues and
validationGate.capabilityFallback unchanged.
- Around line 391-400: The substring check using
backendStatus?.localFallback?.detail.includes("blocked_by_capability") is
fragile; remove localFallbackBlocked and stop probing localFallback.detail.
Instead determine platform-blocked solely from the structured
localCapability?.state === "blocked_by_capability" (i.e. use localCapability for
the canonical state) and adjust the localStateReason computation and resulting
message accordingly; if broker support for a structured state is later added to
backendStatus.localFallback (e.g. localFallback.state), switch to that field
rather than any detail string.
- Around line 755-769: Update the MissionPreflightResult type definition to
document the "blocking-only" semantics for computerUse.missingKinds: add a JSDoc
comment on the missingKinds field stating it contains only
blockingMissingComputerUseKinds (not warning-only kinds), and mention that the
UI/renderer consumes checklist.details for separate blocking vs warning strings
(see blockingMissingComputerUseKinds and checklist.details for context) so
consumers and external callers/telemetry don't assume missingKinds includes
non-blocking items.
In `@apps/desktop/src/renderer/components/app/TopBar.tsx`:
- Around line 28-47: deriveSyncLabel can return a “ready” message while
syncDotClass shows an error; fix by giving the error state priority: in
deriveSyncLabel (the function handling SyncRoleSnapshot), after the null check
add an early check like if (snapshot.client?.state === "error") return "Phone
sync error" so any snapshot whose client is in error returns the error label
before evaluating brain/standalone ready labels; this aligns deriveSyncLabel
with syncDotClass.
- Around line 582-625: The phone sync panel is declared aria-modal but does not
trap focus; when phoneSyncOpen is true Tab can reach background controls. Fix by
implementing a focus trap around the dialog (or use a shared dialog/focus-trap
primitive) tied to phoneSyncPanelRef and phoneSyncOpen: on open move focus into
the panel (e.g., to the first focusable inside SyncDevicesSection or a focusable
sentinel), on Tab/Shift+Tab detect when focus would leave and wrap it to the
last/first focusable, and on close restore focus. Update the dialog container
code that uses phoneSyncPanelRef, setPhoneSyncOpen, and SyncDevicesSection to
include the trap logic or replace the markup with the shared dialog component
that enforces modal focus-trapping.
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 606-611: projectSwitchInFlightRootPath is being set to only a
whitespace-trimmed rootPath, but isSwitchingProject compares against a
normalized root (so trailing slashes cause mismatches); normalize the path
before storing by trimming whitespace and removing any trailing path
separator(s) (e.g., strip trailing "/" or platform-specific separator) and store
that value in projectSwitchInFlightRootPath whenever assigning from
project.rootPath (including the assignment inside projectSelectionTask and the
other occurrence that sets projectSwitchInFlightRootPath), so isSwitchingProject
and stored in-flight root use the same normalized representation.
- Around line 848-856: After switching the active remote project (the block that
mutates remoteProjectCatalog, calls setActiveProjectId(...) and
refreshProjectCatalog() and sets latestRemoteDbVersion = 0), also advance the
local state revision and rebuild derived local state so the UI reflects the new
project's sessions/PR snapshot immediately; e.g. increment localStateRevision
(or call the existing helper that bumps it) and invoke the code-path that
rebuilds activeSessions and the widget snapshot (use your project's helpers such
as rebuildActiveSessions()/refreshWidgetSnapshot() or
refreshDerivedLocalState()) right after setting latestRemoteDbVersion so the app
doesn't keep showing the previous project's data.
In `@apps/web/src/components/editorial/IndexPage.tsx`:
- Around line 7-40: The index contains multiple entries that share the same page
numbers likely due to copy/paste drift (e.g., "Automations", "iOS app", "Mobile
sync" all page "28"; "Byok", "Model configuration", "Providers", "Settings" all
page "22"; "Missions", "Orchestrator", "Planning phase", "Testing phase" all
page "14"; "Context packs" & "Merge conflicts" page "26"; "Lanes" & "Worktrees
(lanes)" page "04"). Verify the intended page numbers for each entry and either
(a) consolidate entries intentionally grouped (e.g., keep "Lanes" and "Worktrees
(lanes)" on the same page) or (b) correct the mistaken page values so each
distinct doc has the correct page; update the array entries for "Automations",
"iOS app", "Mobile sync", "Byok", "Model configuration", "Providers",
"Settings", "Missions", "Orchestrator", "Planning phase", "Testing phase",
"Context packs", and "Merge conflicts" accordingly.
- Around line 6-41: Update the ENTRIES constant's entry for "Team (CTO org)"
inside the Entry[] array: change its href from "/cto/team" to the correct slug
"/cto/workers" (look for the object with name "Team (CTO org)" in the ENTRIES
array). Ensure no other fields are altered; optionally re-evaluate the "CLI ·
ade" href if you want to update that separately.
- Line 48: The reduce-motion default currently forces reduced motion during
SSR/first paint by using useReducedMotion() ?? true; decide whether you want the
conservative SSR-safe behavior or the typical framer-motion idiom and update
both occurrences in IndexPage.tsx accordingly: if you want SSR/hydration safety
keep or explicitly document useReducedMotion() ?? true; if you want animations
enabled by default on client-first renders change both to useReducedMotion() ??
false so the variable reduceMotion (and its second occurrence) will treat null
as “no preference” rather than “reduce motion.” Ensure you update the two lines
where reduceMotion is assigned.
---
Duplicate comments:
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 4507-4518: The current closure passed to
InitialHydrationGate.waitForProjectRow accepts a persisted activeProjectId from
UserDefaults even when the connected host did not advertise a catalog, allowing
stale project scopes to be used; change the logic so that when
supportsProjectCatalog == false you do not accept a persisted activeProjectId —
either clear/derive self.activeProjectId (e.g., set to nil or rederive from
database.currentProjectId) before calling InitialHydrationGate.waitForProjectRow
or update the closure to also check supportsProjectCatalog and refuse the
persisted id if the host has no catalog; ensure you reference activeProjectId,
supportsProjectCatalog, InitialHydrationGate.waitForProjectRow, and
database.hasProject/database.currentProjectId in the update.
---
Nitpick comments:
In `@apps/ios/ADETests/ADETests.swift`:
- Around line 666-702: Add coverage for project-level (root) workspaces missing
a laneId by seeding one FilesWorkspace with laneId: nil for each project before
switching active project; update the calls around replaceFilesWorkspaces and
setActiveProjectId (referencing FilesWorkspace and
database.replaceFilesWorkspaces / database.setActiveProjectId) so that after
switching you assert database.listWorkspaces() only includes the root workspace
for the active project (in addition to existing lane-backed workspace assertions
using fetchLanes and fetchSessions); ensure one nil laneId workspace per project
is inserted and the expected IDs are asserted after the active project change.
In `@apps/web/src/components/editorial/BackCover.tsx`:
- Line 4: Remove the unused import IPhoneFrame from BackCover.tsx: delete the
line importing IPhoneFrame since the component is not referenced (the iPhone
mockup uses inline divs at the current render) and ensure no other references to
the IPhoneFrame symbol remain in this file so lint errors are resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6549794-5c1f-4322-9a46-5626f75fac8b
📒 Files selected for processing (18)
apps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/desktop/src/main/main.tsapps/desktop/src/main/services/missions/missionPreflightService.test.tsapps/desktop/src/main/services/missions/missionPreflightService.tsapps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.tsapps/desktop/src/renderer/components/app/TopBar.tsxapps/desktop/src/renderer/components/onboarding/tour/TourOverlay.test.tsxapps/desktop/src/renderer/components/onboarding/tour/TourOverlay.tsxapps/desktop/src/renderer/state/onboardingStore.test.tsapps/desktop/src/renderer/state/onboardingStore.tsapps/ios/ADE/Services/Database.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADETests/ADETests.swiftapps/web/src/app/layout/SiteLayout.tsxapps/web/src/components/editorial/BackCover.tsxapps/web/src/components/editorial/CompetitorEquation.tsxapps/web/src/components/editorial/IndexPage.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- apps/web/src/app/layout/SiteLayout.tsx
- apps/desktop/src/renderer/state/onboardingStore.test.ts
- apps/ade-cli/src/adeRpcServer.test.ts
- apps/web/src/components/editorial/CompetitorEquation.tsx
- apps/desktop/src/renderer/components/onboarding/tour/TourOverlay.tsx
- apps/ade-cli/src/adeRpcServer.ts
- apps/desktop/src/renderer/state/onboardingStore.ts
- apps/ios/ADE/Services/Database.swift
- apps/desktop/src/main/services/missions/missionPreflightService.test.ts
| try await InitialHydrationGate.waitForProjectRow( | ||
| currentProjectId: { self.database.currentProjectId() }, | ||
| currentProjectId: { | ||
| guard let activeProjectId = self.activeProjectId else { return self.database.currentProjectId() } |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
When a paired desktop does not advertise projectCatalog, activeProjectId can still be nil or point at an older cached project when the socket reconnects. This new gate unblocks on database.currentProjectId(), which only returns whichever cached project is currently selected / most recently opened, not the project row for the connected host. I verified that the first hydration pass immediately calls refreshLaneSnapshots(), refreshWorkSessions(), and refreshPullRequestSnapshots(), and the new DatabaseService.replace* implementations now scope those destructive refreshes by currentProjectId(). On legacy hosts with multiple cached projects, the phone can therefore archive/write the connected host's lanes, sessions, and PR snapshots into the wrong local project before the real project identity is known, breaking backward compatibility and cross-project isolation. The fix is to wait for the connected host's project row (or skip scoped snapshot refreshes until host/project identity is resolved) instead of falling back to an arbitrary cached project.
// apps/ios/ADE/Services/SyncService.swift
try await InitialHydrationGate.waitForProjectRow(
currentProjectId: {
guard let activeProjectId = self.activeProjectId else { return self.database.currentProjectId() }
return self.database.hasProject(id: activeProjectId) ? activeProjectId : nil
},
)| await runActions(result); | ||
| } | ||
| if (step.waitForSelector) { | ||
| await waitForSelector(step.waitForSelector, { |
There was a problem hiding this comment.
[🟡 Medium] [🔵 Bug]
runBeforeEnter() now wires every waitForSelector step through the shared helper, but it never passes the helper's optional signal, so dismissing the tour or superseding the step leaves that helper's MutationObserver, interval, and timeout running until the fallback timeout expires (many of these steps use 30s timeouts in the registered tours). That creates stacked background polling on document during repeated skip/replay flows and is exactly the lifecycle leak this change was meant to avoid. Abort the in-flight wait when nextStep, prevStep, dismissCurrentTour, or completeCurrentTour invalidates the pending step.
// apps/desktop/src/renderer/state/onboardingStore.ts
if (step.waitForSelector) {
await waitForSelector(step.waitForSelector, {
timeoutMs: step.fallbackAfterMs,
});
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the mobile multi-project home and desktop sync catalog flow. Desktop now advertises a project catalog to paired phones, handles project switch requests, and keeps sync host status fresh when switching projects. The iOS app now launches through an ADE-style project home, can open connection settings from that surface, lists desktop projects, switches between them, and can return to the active app surface. The mobile runtime scopes cached lanes, sessions, files, PRs, and snapshots to the active project so stale data from another desktop project does not bleed into the current mobile view.
This also includes the branch’s desktop onboarding/computer-use and web editorial updates that were already part of this work branch, plus the CodeRabbit follow-up fixes for project switching rollback/cancellation, onboarding tour cleanup, mission preflight fallback messaging, and web accessibility/reduced-motion details.
Validation
mobile-lanes-tab-2d82c012, verified project-scoped Work data, and checked the simulator SQLite database for project/lane/session counts and foreign-key health.git diff --checkxcrun swiftc -parse apps/ios/ADE/App/ContentView.swift apps/ios/ADE/Models/RemoteModels.swift apps/ios/ADE/Services/Database.swift apps/ios/ADE/Services/SyncService.swift apps/ios/ADE/Views/Components/ADEDesignSystem.swift apps/ios/ADE/Views/AttentionDrawer/AttentionDrawerButton.swift apps/ios/ADE/Views/Work/WorkRootScreen.swift apps/ios/ADETests/ADETests.swiftplutil -lint apps/ios/ADE/Info.plistxcodebuild test -quiet -project apps/ios/ADE.xcodeproj -scheme ADE -destination 'platform=iOS Simulator,name=iPhone 17 Pro,OS=26.2' -only-testing:ADETests/ADETests/testDatabaseListsMobileProjectsAndScopesCachedRuntimeByActiveProject -only-testing:ADETests/ADETests/testSyncServiceProjectHomeUsesCachedProjectsAndLocalSelection -only-testing:ADETests/ADETests/testDatabaseFetchSessionsHidesSessionsWhenLaneRowIsMissingasdf exec npm --prefix apps/desktop run typecheckasdf exec npm --prefix apps/desktop run buildasdf exec npm --prefix apps/desktop run lintexits with 0 errors and the existing warning backlogTopBar, sync host/protocol, computer-use control plane, onboarding, queue landing, and rerun failed shardsasdf exec npm --prefix apps/ade-cli run typecheckasdf exec npm --prefix apps/ade-cli run testasdf exec npm --prefix apps/ade-cli run buildasdf exec npm --prefix apps/web run typecheckasdf exec npm --prefix apps/web run buildnode scripts/validate-docs.mjscoderabbit review --agent --type uncommitted; valid findings were fixed. A final rerun hit the service rate limit after the fix pass.Notes
The full
coderabbit --agentbranch review cannot run on this PR shape because the service rejects diffs over 150 files; the uncommitted scoped reviews were used before committing and valid findings from those passes were addressed.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Greptile Summary
This PR implements end-to-end mobile multi-project switching: the desktop now builds and advertises a project catalog to paired phones, handles
project_switch_requestmessages, and manages per-project sync host lifecycle; the iOS app gains a Project Home screen, catalog merging with dedup, and project-scoped DB reads for lanes, sessions, files, PRs, snapshots, and integration proposals. Previous review concerns (sequentialPromise.all,fetchIntegrationProposalsscoping, rollback/cancellation guard) are all resolved.handleDropstale-closure inTopBar.tsx: iffetchRecentfires on window focus mid-drag, the drop handler splices from a stalerecentProjectssnapshot and persists a wrong order viareorderRecent.Confidence Score: 4/5
Safe to merge after addressing the stale-closure drag-reorder bug in TopBar; iOS offline fallback race is low-probability and non-data-corrupting.
One P1 bug (drag reorder overwrites fresh server state with stale closure data on mid-drag focus event) keeps the score at 4. All prior P1 concerns from previous review rounds are resolved. The P2 iOS offline fallback race is a narrow edge case that only produces a user-visible error message, not data loss.
apps/desktop/src/renderer/components/app/TopBar.tsx — handleDrop stale recentProjects closure.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Phone as iOS App participant Desktop as Desktop (main.ts) participant SyncHost as SyncHostService Phone->>Desktop: project_catalog_request Desktop->>Desktop: listMobileSyncProjects() Promise.all over contexts Desktop-->>Phone: project_catalog_response { projects[] } Phone->>Phone: refreshProjectCatalog() merge remote + cached, dedup by rootPath Phone->>Desktop: project_switch_request { projectId, rootPath } Desktop->>Desktop: prepareMobileSyncProjectConnection() ensureProjectContextForMobileSync() Desktop->>SyncHost: initialize() + getStatus() Desktop-->>Phone: project_switch_result { ok, connection, project } Phone->>Phone: switchToDesktopProject() save rollback state, setActiveProjectId Phone->>SyncHost: connectUsingProfile(new token/port) alt success Phone->>Phone: projectHomePresented = false else error and still current selection Phone->>Phone: rollback activeProjectId, token, profile then reconnectIfPossible else error and superseded by newer selection Phone->>Phone: rethrow no rollback endComments Outside Diff (3)
apps/ios/ADE/Services/Database.swift, line 1817-1845 (link)fetchIntegrationProposalsnot scoped to active projectUnlike
fetchLanes,fetchTerminalSessions, andfetchPullRequests, this function omits any project-id filter and returns all rows across every project in the local database. After a project switch, proposals whosesource_lane_ids_jsoncontains lanes from the previous project will still appear in the integration-proposals tab for the newly selected project. Theintegration_proposalstable has noproject_idcolumn, so a join through lanes is needed:Alternatively, add a
project_idcolumn at the schema layer if per-project isolation is desired here.Prompt To Fix With AI
apps/ios/ADE/Services/Database.swift, line 1830-1857 (link)fetchIntegrationProposalsnot scoped to active projectThe query returns every row in
integration_proposalsacross all projects. The table has aproject_id NOT NULLcolumn with an index (idx_integration_proposals_project), so a scoped filter is straightforward. After a project switch, proposals whosesource_lane_ids_jsonreferences lanes from the previous project will still appear in the integration-proposals tab for the newly selected project.Add a
WHERE project_id = ?filter directly — the join through lanes is not needed since the column exists:Prompt To Fix With AI
apps/desktop/src/renderer/components/app/TopBar.tsx, line 269-281 (link)handleDropcloses over stalerecentProjectshandleDropis wrapped inuseCallbackwith[dragIdx, recentProjects]as deps, which means on every render whererecentProjectschanges a new callback is created. However, if a focus-triggeredfetchRecentfires betweendragstartanddrop(e.g. the window re-focuses mid-drag),recentProjectsin state is replaced with a fresh server response, but thehandleDropclosure still holds the old snapshot. Thesplicethen produces a reordered list from the stale array, overwriting the just-refreshed data. ThereorderRecentIPC call will persist the stale order too.Consider using a
refto always read the latestrecentProjectsinsidehandleDrop, or suppressfetchRecentwhile a drag is in progress (dragIdx !== null).Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (10): Last reviewed commit: "Allow cached mobile project ids during s..." | Re-trigger Greptile