Skip to content

Refreshing Lanes Tabs#328

Merged
arul28 merged 4 commits into
mainfrom
ade/refreshing-lanes-tabs-2614e8e7
May 21, 2026
Merged

Refreshing Lanes Tabs#328
arul28 merged 4 commits into
mainfrom
ade/refreshing-lanes-tabs-2614e8e7

Conversation

@arul28
Copy link
Copy Markdown
Owner

@arul28 arul28 commented May 21, 2026

Summary

Describe the change.

What Changed

Key files and behaviors.

Validation

How you tested.

Risks

Anything to watch.

ADE   Open in ADE  ·  ade/refreshing-lanes-tabs-2614e8e7 branch  ·  PR #328

Summary by CodeRabbit

  • New Features

    • PR creation commands now display both GitHub and ADE PR URLs for quick navigation
    • Agent skills discovery expanded to include project, user, runtime, and bundled skill roots
    • iOS app now supports HTTPS-based PR deep-link sharing
  • Improvements

    • Project switching now retains lane and session caches across tab switches
    • Draft chat mode preserves user model/provider selections during lane hydration
    • Lanes surface optimized with keep-alive behavior during navigation

Review Change Stack

Greptile Summary

This PR delivers the "Refreshing Lanes Tabs" milestone: the Lanes surface is now kept alive (keep-alive) across tab switches, mirroring the existing Work surface pattern. Project switching restores lane/session caches on warm tab switches, draft chat mode preserves user-selected model/provider during lane hydration, and CLI PR creation now outputs both GitHub and ADE URLs.

  • Lanes keep-alive in App.tsx: a lanesSurface parallel to workSurface is mounted on first /lanes visit and kept hidden (CSS + inert attribute) rather than unmounted; a ProjectTransitionVeil overlays the tab host during cold project switches.
  • Stale-closure fix in useLaneWorkSessions: refresh callback's laneId/projectRoot captures replaced with refs updated in a useEffect, preventing stale fetch targets when lanes or projects switch mid-request; two dedicated test cases cover the mid-refresh scenarios.
  • runtimeBridge.ts trust boundary hardened: resolveAuthorizedLocalRuntimeRootPath validates renderer-supplied rootPath against the window session's authorized roots (binding, active project, open project tabs) before connecting to the local runtime pool.

Confidence Score: 5/5

Safe to merge — changes are additive, well-tested, and the runtimeBridge trust boundary is now properly enforced.

The lanes keep-alive mirrors a proven pattern already used for the Work surface, the stale-closure fix in useLaneWorkSessions is backed by two targeted mid-refresh tests, and the IPC rootPath authorization in runtimeBridge now validates against the full set of the window's authorized project roots rather than accepting an arbitrary renderer-supplied path. No functional regressions were found across the changed paths.

No files require special attention.

Important Files Changed

Filename Overview
apps/desktop/src/renderer/components/app/App.tsx Adds lanes surface keep-alive mirroring the existing work surface pattern; introduces ProjectTransitionVeil for project-switch UX; well-tested.
apps/desktop/src/renderer/state/appStore.ts Warm tab switch now restores lane cache from laneCacheByProject; cache pruning now includes openProjectTabRoots alongside active+recent roots.
apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts Stale-closure fix: refresh reads laneId/projectRoot from refs instead of closure captures; empty deps array intentional; two new mid-refresh tests validate correctness.
apps/desktop/src/renderer/components/chat/AgentChatPane.tsx draftLaunchConfigTouchedKeyRef prevents syncComposerToSession from overwriting user-selected model/provider during lane hydration; resets correctly on lane/project change.
apps/desktop/src/main/services/ipc/runtimeBridge.ts resolveAuthorizedLocalRuntimeRootPath validates renderer-supplied rootPath against session's authorized roots before connecting; addresses prior trust boundary concern.
apps/desktop/src/preload/preload.ts All IPC calls now pass explicit rootPath; switchToPath properly saves/restores binding on error instead of pre-clearing it.
apps/desktop/src/shared/agentSkillRoots.ts New getAgentSkillRootCandidates consolidates duplicate ancestorSkillRoots logic; getAdeAgentSkillRootsForPrompt still caps at 4 via slice(0, promptAgentSkillRootLimit).
apps/desktop/src/main/services/chat/agentChatService.ts filesystemBackedCommands helper unifies codex/droid/cursor slash command discovery; skill injection cap (MAX_INJECTED_PROJECT_SKILLS=20) removed for skills in the system prompt.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User navigates to /lanes] --> B{lanesMounted?}
    B -- No --> C[Mount LanesPage\nlanesMounted = true]
    B -- Yes --> D[Show cached LanesPage\nremove inert attr]
    C --> E[LanesPage visible & active]
    D --> E

    E --> F[User navigates away]
    F --> G[Set inert + aria-hidden\nopacity:0 / z-index:-1]
    G --> H[LanesPage stays mounted\nin background]

    H --> I[User returns to /lanes]
    I --> D

    J[Project tab switch] --> K{Warm switch?}
    K -- Yes --> L[Restore laneCacheByProject\nrestore laneSelectionByProject]
    K -- No --> M[Show ProjectTransitionVeil\nlanesLoading: true]
    L --> N[Lanes surface hydrated instantly]
    M --> O[Full project load\nclear caches]
Loading

Comments Outside Diff (1)

  1. apps/desktop/src/main/services/ipc/runtimeBridge.ts, line 617-636 (link)

    P2 security Renderer can now request runtime access for any arbitrary project root

    The three IPC handlers (localRuntimeCallAction, localRuntimeCallSync, localRuntimeStreamEvents) now accept a renderer-supplied rootPath, which is used directly — after only a trim() — to connect to the local runtime pool and create event subscriptions. Previously, the root was always derived from the window session's trusted binding. A compromised or malicious renderer page can now direct IPC calls to any path on disk (e.g. "/" or a path belonging to another project). For a desktop app with controlled renderer content this is lower risk, but the trust boundary change is worth a second look before shipping.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/desktop/src/main/services/ipc/runtimeBridge.ts
    Line: 617-636
    
    Comment:
    **Renderer can now request runtime access for any arbitrary project root**
    
    The three IPC handlers (`localRuntimeCallAction`, `localRuntimeCallSync`, `localRuntimeStreamEvents`) now accept a renderer-supplied `rootPath`, which is used directly — after only a `trim()` — to connect to the local runtime pool and create event subscriptions. Previously, the root was always derived from the window session's trusted binding. A compromised or malicious renderer page can now direct IPC calls to any path on disk (e.g. `"/"` or a path belonging to another project). For a desktop app with controlled renderer content this is lower risk, but the trust boundary change is worth a second look before shipping.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Claude Code

Reviews (5): Last reviewed commit: "ship: iteration 2 - address coderabbit r..." | Re-trigger Greptile

@vercel
Copy link
Copy Markdown

vercel Bot commented May 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
ade Ignored Ignored Preview May 21, 2026 10:33pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 21, 2026

📝 Walkthrough

Walkthrough

This PR extends PR creation with both GitHub and ADE deep-link URLs, refactors agent skills root discovery to support multiple sources, improves project/lane switching with warm cache restoration and state preservation, and adds lanes routing with project transition UI.

Changes

PR Deep-Link URLs for Handoff

Layer / File(s) Summary
ADE and GitHub URL derivation for PR creation
apps/ade-cli/src/adeRpcServer.ts, apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts, apps/desktop/src/main/services/ai/tools/workflowTools.ts
Tools add buildDeeplink imports and helpers to derive githubUrl/adeUrl from PR-like objects (owner, repo, PR number), returning enriched PR creation responses. Tests verify fixture expansions and assertion updates.
CLI formatting and summarization of PR creation output
apps/ade-cli/src/cli.ts, apps/ade-cli/src/cli.test.ts
CLI adds pr-create formatter id, helpers (positiveInteger, getPrCreateLinks, summarizePrCreateResult), and formatPrCreate renderer that outputs PR data with both GitHub and ADE URLs. Summarization special-cases the PR create plan label, formatter inference routes to pr-create, and text output dispatches to formatPrCreate.
TUI PR summary formatter with URL rows
apps/ade-cli/src/tuiClient/rightPaneFormatters.ts, apps/ade-cli/src/tuiClient/__tests__/rightPaneFormatters.test.ts
TUI formatter adds pickPositiveInteger and unwrapPrValue helpers, updates formatPrSummary to resolve multiple URL fields (github, fallback, ade), and emits separate rows for each link. Tests verify formatting with action envelopes and repo metadata derivation.
iOS deeplink parsing and routing for PR links
apps/ios/ADE/App/DeepLinkRouter.swift, apps/ios/ADE/App/DeepLinkURLParsing.swift, apps/ios/ADE/Views/Deeplinks/SendToMacCard.swift, apps/ios/ADETests/ADETests.swift
iOS adds ADEDeepLinkURLParsing with URL-parsing helpers, DeepLinkRouter recognizes HTTPS mirror links (https://ade.app/open), and SendToMacCard.Kind gains .pr(owner:repo:number) case with parsing and UI rendering for PR-specific deeplinks.
Skill documentation on PR handoff with ADE links
apps/desktop/resources/agent-skills/ade-deeplinks/SKILL.md, apps/desktop/resources/agent-skills/ade-pr-workflows/SKILL.md
Agent skill docs now include guidance for including both GitHub and ADE PR URLs during handoff, with instructions on using ade prs create output and alternative ade link pr commands.

Agent Skills Discovery and Organization

Layer / File(s) Summary
Core skill roots discovery and bundled skills constant
apps/desktop/src/shared/agentSkillRoots.ts, apps/desktop/src/shared/adeCliGuidance.ts
New exported getAgentSkillRootCandidates combines ancestor-derived roots, home-based roots, and ADE-specific roots via path helpers and bounded ancestor traversal. ADE_BUNDLED_AGENT_SKILLS constant lists bundled skill names, with test coverage verifying multi-source aggregation and deduplication.
CLI guidance with bundled skills list
apps/desktop/src/shared/adeCliGuidance.ts, apps/desktop/src/shared/adeCliGuidance.test.ts
CLI guidance now dynamically renders the bundled skills list via ADE_BUNDLED_AGENT_SKILLS, with updated wording for skill roots availability and test coverage.
Slash-command discovery refactoring for multi-root skills
apps/desktop/src/main/services/chat/claudeSlashCommandDiscovery.ts, apps/desktop/src/main/services/chat/claudeSlashCommandDiscovery.test.ts
Slash-command discovery now uses getAgentSkillRootCandidates, removes bespoke ancestor traversal, and enables discovery across .agents, .ade, and .codex directories with corresponding test coverage.
Slash-command aggregation and provider-agnostic skills
apps/desktop/src/main/services/chat/agentChatService.ts, apps/desktop/src/main/services/chat/agentChatService.test.ts
Chat service removes injected-skill caps, introduces filesystemBackedCommands() helper, updates command discovery and prompt formatting to list all skills without capping/hiding counts, and refactors provider-based command aggregation. Tests verify filesystem-backed skill command sourcing.
TUI skills command made provider-agnostic
apps/ade-cli/src/tuiClient/commands.ts, apps/ade-cli/src/tuiClient/app.tsx, apps/ade-cli/src/tuiClient/__tests__/commands.test.ts
Built-in /skills command now references "agent skills", removes Claude-only restriction, and is available across providers. TUI app.tsx enumerates skills via getAgentSkillRootCandidates with distinct paths for agents vs. skills. Tests verify command visibility across providers.
CLI bootstrap and launch with multi-root skills
apps/ade-cli/src/bootstrap.ts, apps/desktop/src/shared/cliLaunch.ts
CLI bootstrap and launch compute skill roots via getAdeAgentSkillRootsForPrompt and getAgentSkillRootCandidates with updated logic for lane worktree paths.
Skill discovery test coverage and artifacts validation
apps/desktop/src/shared/agentSkillRoots.test.ts, apps/desktop/scripts/validate-mac-artifacts.mjs, apps/desktop/scripts/validate-win-artifacts.mjs, apps/desktop/src/renderer/components/terminals/cliLaunch.test.ts
Skill discovery tests verify multi-root candidate generation and capping. Artifact validation scripts define BUNDLED_AGENT_SKILLS and assert SKILL.md files for all bundled skills. Launch tests verify skill-root seeding for lane, Codex, Cursor, and Droid.

Project Switching and Warm State Restoration

Layer / File(s) Summary
IPC handlers with explicit rootPath parameter
apps/desktop/src/main/services/ipc/runtimeBridge.ts, apps/desktop/src/main/services/ipc/runtimeBridge.test.ts
IPC handlers accept optional rootPath in payloads, preferring caller-provided roots before falling back to window-session binding. Local binding is inferred when rootPath is explicitly requested, enabling multi-project action routing.
Preload IPC call updates with rootPath payload
apps/desktop/src/preload/preload.ts, apps/desktop/src/preload/preload.test.ts
Preload adds localProjectBindingForRoot helper, updates all local runtime IPC invocations to include rootPath, and reworks project.switchToPath to snapshot/restore bindings around the switch. Tests verify rootPath-aware routing and in-flight project switches.
Warm project switch with lane cache and session restoration
apps/desktop/src/renderer/state/appStore.ts, apps/desktop/src/renderer/state/appStore.test.ts
App store's switchProjectToPath captures cached lane data and restores laneSnapshots/lanes/lanesLoading from cache for warm tabs. Retained state roots expand to include openProjectTabRoots alongside active/recent roots. Tests verify cache restoration and multi-tab state preservation.
Session refresh replay during lane/project changes
apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts, apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.test.ts
Lanes work sessions hook tracks laneId and projectRoot in refs, allowing refresh to replay queued results against the latest context without stale closure values. Tests verify replay behavior during lane and project switches.
Draft launch config touched suppression for user edits
apps/desktop/src/renderer/components/chat/AgentChatPane.tsx, apps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsx
Chat pane computes draftLaunchConfigScopeKey per scope and tracks touched state via draftLaunchConfigTouchedKeyRef to suppress hydration when users manually edit composer controls, preventing late hydration from overwriting selections. Tests verify suppression during lane switching.

App Routing, Lanes Surface, and Project Transition UI

Layer / File(s) Summary
Lanes route and surface infrastructure
apps/desktop/src/renderer/components/app/App.tsx
App.tsx adds isLanesRoutePath helper and expands ProjectRouteContent to manage a separately mounted lanes surface with route state tracking, inert attributes, error boundary, and Suspense fallback.
Project transition veil overlay component
apps/desktop/src/renderer/components/app/App.tsx
App.tsx introduces ProjectTransitionVeil component and wires it into ProjectTabHost via projectTransition store selector, rendering veil with transition-kind-based label (switching/opening/closing) when active.
Lanes route keep-alive and transition veil tests
apps/desktop/src/renderer/components/app/App.workKeepAlive.test.tsx
Tests verify lanes surface mount/unmount, keep-alive state after navigation, project-transition-veil rendering with expected text, and lazy mounting (not mounted on initial work route).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

  • arul28/ADE#185: Both PRs change apps/ade-cli/src/adeRpcServer.ts's create_pr_from_lane tool/handler logic (one to auto-draft title/body and handle duplicate-lane PRs, the other to enrich the returned payload with githubUrl/adeUrl derived deep links).
  • arul28/ADE#297: Main PR's updates to the Claude slash-command/skill discovery and injected context (e.g., agentChatService.ts injection/capping logic and claudeSlashCommandDiscovery.ts skill-root–based traversal) overlap with the retrieved PR's corresponding changes in the same areas of code.

Suggested labels

desktop, ios

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.20% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title 'Refreshing Lanes Tabs' is vague and does not convey meaningful information about the changeset's primary objectives or the substantial work completed. Consider using a more descriptive title that reflects the main achievements, such as 'Add keep-alive and warm switching for Lanes tab' or 'Implement Lanes persistence and PR URL deep-linking'.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ade/refreshing-lanes-tabs-2614e8e7

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 21, 2026

@copilot review but do not make fixes

@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 21, 2026

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

PR Review

Scope: 60 file(s), +1596 / −220
Verdict: Minor issues

This PR makes Lanes tab state stick across tab switches and project changes (keep-alive routing, warm-tab lane cache, explicit rootPath on local-runtime IPC during switches), and broadens agent-skill / PR-deeplink discovery across desktop, CLI, TUI, and iOS. The race fixes and tests look solid; the main risks are prompt/env bloat from unbounded skill discovery.


⚡ Performance

[Medium] All discovered skills are injected into every Claude session startup prompt

File: apps/desktop/src/main/services/chat/agentChatService.ts:L14426-L14451
Issue: The prior MAX_INJECTED_PROJECT_SKILLS cap and “N more hidden” note were removed. Every discovered skill (name, description, and full filePath) is now appended to the Claude Code preset systemPrompt on session start.
Impact: Repos with many .agents/.ade/.claude/.codex skills can add thousands of tokens to every new Work chat cold start, increasing latency and cost with no user-visible benefit until a skill is actually relevant.
Fix: Restore a bounded slice for prompt injection (keep full discovery for slash-command UI), e.g. reintroduce MAX_INJECTED_PROJECT_SKILLS and the hidden-count line for overflow.

[Medium] Prompt/env skill-root lists are no longer capped

File: apps/desktop/src/shared/agentSkillRoots.ts:L48-L59 and L153-L158
Issue: getAdeAgentSkillRootsForPrompt now returns the full getAgentSkillRootCandidates set. That walks up to 25 ancestors × four roots (.claude, .agents, .ade, .codex) per cwd, plus home and bundled fallbacks—often dozens to 100+ directories in monorepos.
Impact: ADE_AGENT_SKILLS_DIRS and the “Agent skill roots for this session” guidance string can grow very large for CLI launches and CTO/mission prompts, slowing startup and making env blocks harder for runtimes to parse.
Fix: Keep broad discovery for filesystem scanning, but cap prompt/env emission (the previous .slice(0, 4) on prompt roots was a reasonable default).


Notes

  • Good patterns: Lanes keep-alive passes active={active && isLanesRoute} so existing if (!active) return guards in LanesPage should keep background lane/PR polling off while parked; useLaneWorkSessions ref-scoping fixes mid-flight lane/project switches; preload rootPath + binding rollback on failed switchToPath is well tested.
  • Security: No exploitable issues found in the diff; explicit rootPath on local-runtime IPC is appropriate for trusted Electron preload during project transitions (not a web attacker boundary).
  • Verification gap: iOS HTTPS ade.app/open routing was reviewed from code/tests only; not exercised on a device in this run.
Open in Web View Automation 

Sent by Cursor Automation: BUGBOT in Versic

Comment thread apps/desktop/src/shared/agentSkillRoots.ts
Comment thread apps/ios/ADE/Views/Deeplinks/SendToMacCard.swift Outdated
@arul28 arul28 force-pushed the ade/refreshing-lanes-tabs-2614e8e7 branch from 9901b85 to 6ff3dbc Compare May 21, 2026 21:03
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 21, 2026

@copilot review but do not make fixes

Copy link
Copy Markdown
Contributor

Copilot AI commented May 21, 2026

@copilot review but do not make fixes

Reviewed 6ff3dbc as requested (review-only, no fixes applied). I did a focused pass on the lane refresh changes (useLaneWorkSessions, appStore, and App keep-alive paths) and did not find a blocking regression in the updated logic/tests. No UI changes were made in this pass, so there is no new screenshot to attach.

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 21, 2026

@copilot review but do not make fixes

@arul28 arul28 force-pushed the ade/refreshing-lanes-tabs-2614e8e7 branch from 6724ead to 03e26f1 Compare May 21, 2026 21:39
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 21, 2026

@copilot review but do not make fixes

@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 21, 2026

@copilot review but do not make fixes

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (7)
apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts (1)

192-199: ⚡ Quick win

Consolidate duplicated ADE PR deeplink builder.

Line 192 duplicates the same buildAdePrUrl logic already added in apps/desktop/src/main/services/ai/tools/workflowTools.ts. Extracting this into a shared helper avoids contract drift.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts` around lines 192
- 199, The buildAdePrUrl function duplicates logic already present in
workflowTools.ts; extract the deeplink construction into a shared helper (e.g.,
a new exported function like createAdePrDeeplink or reuse an existing shared
module) and replace both buildAdePrUrl implementations to call that helper;
locate usages of buildAdePrUrl in ctoOperatorTools.ts and workflowTools.ts and
update imports to reference the new shared helper, ensuring the helper takes a
PrSummary (or the same repoOwner/repoName/prNumber fields) and internally calls
buildDeeplink.
apps/desktop/src/main/services/ipc/runtimeBridge.test.ts (1)

197-239: ⚡ Quick win

Add matching explicit-rootPath tests for sync/events handlers.

This new test protects IPC.localRuntimeCallAction, but the same override logic was added for IPC.localRuntimeCallSync and IPC.localRuntimeStreamEvents. Adding parallel assertions would close a high-risk regression gap for project-switch routing.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/main/services/ipc/runtimeBridge.test.ts` around lines 197 -
239, Add tests mirroring the existing IPC.localRuntimeCallAction case for the
other two handlers: IPC.localRuntimeCallSync and IPC.localRuntimeStreamEvents.
Reuse the same registerRuntimeBridge setup (including localRuntimeConnectionPool
and getWindowSession with binding "/old-repo"), invoke
ipcHandlers.get(IPC.localRuntimeCallSync) and
ipcHandlers.get(IPC.localRuntimeStreamEvents) with an eventForSender and a
payload containing rootPath: "/new-repo", then assert the returned values
resolve/match as appropriate and that localRuntimeConnectionPool was invoked
with "/new-repo" (e.g., expect(localRuntimeConnectionPool.<sync/stream
method>).toHaveBeenCalledWith("/new-repo", expect.objectContaining({ domain:
"lane", action: "list" }) ) ). Ensure tests reference IPC.localRuntimeCallSync,
IPC.localRuntimeStreamEvents and localRuntimeConnectionPool to mirror the
existing test structure.
apps/desktop/src/renderer/components/app/App.workKeepAlive.test.tsx (1)

291-313: 💤 Low value

Consider adding round-trip assertion for lanes keep-alive.

The work surface test (lines 183-212) verifies both directions: work → files → work, confirming the surface becomes active again. This test only verifies lanes → files. Adding the return navigation assertion would provide parity and catch any issues with lanes re-activation.

🧪 Suggested addition to complete round-trip coverage
     expect(lanesLifecycle.mounts).toBe(1);
     expect(lanesLifecycle.unmounts).toBe(0);
+
+    // Navigate back to lanes and verify reactivation
+    // (requires adding a "Return to lanes" button in FilesPage mock or using navigate directly)
   });

Alternatively, extend the FilesPage mock to include a "Return to lanes" button, then verify:

  • lanes-page no longer has aria-hidden='true' ancestor
  • data-active returns to "true"
  • Mount count remains at 1
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/renderer/components/app/App.workKeepAlive.test.tsx` around
lines 291 - 313, Extend this test ("keeps the Lanes page mounted after visiting
it and leaving for another tab") to perform the round-trip back to lanes: after
navigating to the files page (using the existing fireEvent.click on the "Lanes
open files" button and awaiting screen.findByTestId("files-page")), simulate a
user action that returns to lanes (add a "Return to lanes" button to the
FilesPage mock if needed), then assert that the lanes surface no longer has an
ancestor with aria-hidden='true', that
screen.getByTestId("lanes-page").getAttribute("data-active") === "true", and
that lanesLifecycle.mounts remains 1 and lanesLifecycle.unmounts remains 0 to
confirm it was not remounted.
apps/desktop/scripts/validate-win-artifacts.mjs (1)

20-31: ⚡ Quick win

Use camelCase for the new bundled skills variable name.

Rename this constant to camelCase and propagate the rename to usages.

As per coding guidelines, **/*.{ts,tsx,js,jsx}: Use camelCase for variable names in TypeScript/JavaScript files.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/scripts/validate-win-artifacts.mjs` around lines 20 - 31, The
constant BUNDLED_AGENT_SKILLS should be renamed to camelCase (e.g.,
bundledAgentSkills) and all references updated accordingly; locate the
declaration of BUNDLED_AGENT_SKILLS and replace its name with
bundledAgentSkills, then update every usage call site (imports, destructuring,
or direct references) to the new identifier to ensure consistent naming across
the codebase.
apps/desktop/src/shared/adeCliGuidance.ts (1)

3-15: ⚡ Quick win

Use camelCase for the new exported skill list constant.

Rename the constant to camelCase and update imports/usages accordingly.

As per coding guidelines, **/*.{ts,tsx,js,jsx}: Use camelCase for variable names in TypeScript/JavaScript files.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/shared/adeCliGuidance.ts` around lines 3 - 15, Rename the
exported constant ADE_BUNDLED_AGENT_SKILLS to camelCase (e.g.,
adeBundledAgentSkills) and update every import and internal usage to that new
identifier; ensure the exported type remains "as const" and preserve the array
contents, and run a project-wide search/replace for ADE_BUNDLED_AGENT_SKILLS to
avoid unresolved references (update any tests or consumers that import it as
well).
apps/desktop/src/shared/agentSkillRoots.ts (1)

45-47: ⚡ Quick win

Rename new constants to camelCase to match repo naming rule.

Please rename these new constants to camelCase and update references to keep TS naming consistent.

As per coding guidelines, **/*.{ts,tsx,js,jsx}: Use camelCase for variable names in TypeScript/JavaScript files.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/src/shared/agentSkillRoots.ts` around lines 45 - 47, Rename the
exported constants to camelCase to follow repo naming conventions: change
ANCESTOR_SKILL_DIRS to ancestorSkillDirs and PROMPT_AGENT_SKILL_ROOT_LIMIT to
promptAgentSkillRootLimit, and update the related type name or usages
referencing AncestorSkillDir if you choose to adjust it (or keep
AncestorSkillDir as-is if type naming policy allows); then update all references
throughout the codebase to use the new names (search for ANCESTOR_SKILL_DIRS and
PROMPT_AGENT_SKILL_ROOT_LIMIT and replace with ancestorSkillDirs and
promptAgentSkillRootLimit) and run TypeScript checks to ensure no broken imports
or usages remain.
apps/desktop/scripts/validate-mac-artifacts.mjs (1)

18-29: ⚡ Quick win

Use camelCase for the new bundled skills variable name.

Please rename this new constant to camelCase and update call sites.

As per coding guidelines, **/*.{ts,tsx,js,jsx}: Use camelCase for variable names in TypeScript/JavaScript files.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/desktop/scripts/validate-mac-artifacts.mjs` around lines 18 - 29, The
constant BUNDLED_AGENT_SKILLS should be renamed to camelCase (e.g.,
bundledAgentSkills) and every reference updated accordingly; change the
declaration of BUNDLED_AGENT_SKILLS to bundledAgentSkills and update all call
sites/uses in this module and any imports/exports that reference that symbol
(ensure exported names or destructured uses are adjusted too) so the variable
follows the project's camelCase naming guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/ade-cli/src/adeRpcServer.ts`:
- Around line 2398-2413: prLinkUrls currently only forwards pr.githubUrl and may
omit a GitHub link even when repoOwner/repoName/prNumber are present; update
prLinkUrls (function prLinkUrls) to synthesize githubUrl when githubUrl is falsy
but repoOwner, repoName and prNumber exist (e.g. construct
"https://github.com/{repoOwner}/{repoName}/pull/{prNumber}" or use an existing
helper) and include it in the returned object alongside adeUrl so callers always
get a GitHub URL when coordinates are available.

In `@apps/desktop/resources/agent-skills/ade-deeplinks/SKILL.md`:
- Around line 124-129: The sentence fragment "for the ADE link." should be
merged into the surrounding text so the instruction flows; update the SKILL.md
section after the code block (the ade link pr example) to form a complete
sentence such as "Then include the printed ADE link. Prefer the default HTTPS
form in chat and terminal output because it is clickable, shareable, and
upgrades into the ADE PRs tab." — edit the text immediately following the code
block in the ade-deeplinks SKILL.md to replace the fragment with that complete
sentence.

In `@apps/desktop/src/main/services/ipc/runtimeBridge.ts`:
- Around line 620-626: The code currently trusts renderer-supplied arg.rootPath
(requestedRootPath → rootPath) and may route local runtime actions to arbitrary
paths; add an authorization check that resolves and validates any
renderer-provided rootPath against the window/session-authorized project scope
before using it. Implement or call a shared resolver like
verifyRendererRootPath(requestedRootPath, session, binding, windowId) that
returns either the authorized root (e.g., session.project.rootPath or
binding.rootPath) or null, and replace the direct assignment of rootPath with
the verified value; ensure this check is applied wherever requestedRootPath is
used (variables/functions referenced: requestedRootPath, rootPath, binding,
session, arg.rootPath).

In `@apps/desktop/src/preload/preload.ts`:
- Around line 1143-1152: localProjectBindingForRoot currently trims and returns
a routable OpenProjectBinding immediately from untrusted renderer input; instead
validate rootPath is non-empty/non-whitespace and do not promote it to a
routable binding until the main process confirms the switch. Update
localProjectBindingForRoot to reject empty/whitespace inputs and to return a
non-routable "pending" binding shape (e.g., include a confirmed: false or kind:
"local-pending") so callers like project.switchToPath won't be used for
privileged routing, and change project.switchToPath to only replace
currentProjectBinding with the confirmed binding after the main process
acknowledgement (or explicitly mark bindings as non-routable while pending).
Ensure all runtime consumers check the confirmed flag or binding kind before
using the binding for privileged routes.

In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 5847-5849: The code currently sets
draftLaunchConfigTouchedKeyRef.current = draftLaunchConfigScopeKey only for some
draft edits but not when the executionMode field is changed, so late hydration
can overwrite a user-edited executionMode; update every handler or effect that
updates executionMode (references: executionMode, setExecutionMode or the event
handler that assigns executionMode) to also set
draftLaunchConfigTouchedKeyRef.current = draftLaunchConfigScopeKey when there is
no selectedSessionId (use the same conditional used in the existing draft touch
logic that checks selectedSessionId), and apply the same change to the other
occurrences noted (the other blocks where draftLaunchConfigTouchedKeyRef is set)
so executionMode edits mark the draft as touched consistently.

In `@apps/ios/ADE/App/DeepLinkURLParsing.swift`:
- Around line 6-12: splitRepo currently allows extra path segments because it
uses split(separator: "/", maxSplits: 1) which treats "owner/repo/extra" as
owner="owner" and repo="repo/extra"; change splitRepo to only accept exactly one
slash and reject any additional slashes by ensuring the input contains exactly
one "/" (or that the resulting repo string contains no "/" characters). Locate
the splitRepo function and replace the current split logic (value.split(...))
with a check that the string has exactly two components (owner and repo) and
that neither component is empty and repo contains no "/" characters before
returning (owner, repo).

---

Nitpick comments:
In `@apps/desktop/scripts/validate-mac-artifacts.mjs`:
- Around line 18-29: The constant BUNDLED_AGENT_SKILLS should be renamed to
camelCase (e.g., bundledAgentSkills) and every reference updated accordingly;
change the declaration of BUNDLED_AGENT_SKILLS to bundledAgentSkills and update
all call sites/uses in this module and any imports/exports that reference that
symbol (ensure exported names or destructured uses are adjusted too) so the
variable follows the project's camelCase naming guideline.

In `@apps/desktop/scripts/validate-win-artifacts.mjs`:
- Around line 20-31: The constant BUNDLED_AGENT_SKILLS should be renamed to
camelCase (e.g., bundledAgentSkills) and all references updated accordingly;
locate the declaration of BUNDLED_AGENT_SKILLS and replace its name with
bundledAgentSkills, then update every usage call site (imports, destructuring,
or direct references) to the new identifier to ensure consistent naming across
the codebase.

In `@apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts`:
- Around line 192-199: The buildAdePrUrl function duplicates logic already
present in workflowTools.ts; extract the deeplink construction into a shared
helper (e.g., a new exported function like createAdePrDeeplink or reuse an
existing shared module) and replace both buildAdePrUrl implementations to call
that helper; locate usages of buildAdePrUrl in ctoOperatorTools.ts and
workflowTools.ts and update imports to reference the new shared helper, ensuring
the helper takes a PrSummary (or the same repoOwner/repoName/prNumber fields)
and internally calls buildDeeplink.

In `@apps/desktop/src/main/services/ipc/runtimeBridge.test.ts`:
- Around line 197-239: Add tests mirroring the existing
IPC.localRuntimeCallAction case for the other two handlers:
IPC.localRuntimeCallSync and IPC.localRuntimeStreamEvents. Reuse the same
registerRuntimeBridge setup (including localRuntimeConnectionPool and
getWindowSession with binding "/old-repo"), invoke
ipcHandlers.get(IPC.localRuntimeCallSync) and
ipcHandlers.get(IPC.localRuntimeStreamEvents) with an eventForSender and a
payload containing rootPath: "/new-repo", then assert the returned values
resolve/match as appropriate and that localRuntimeConnectionPool was invoked
with "/new-repo" (e.g., expect(localRuntimeConnectionPool.<sync/stream
method>).toHaveBeenCalledWith("/new-repo", expect.objectContaining({ domain:
"lane", action: "list" }) ) ). Ensure tests reference IPC.localRuntimeCallSync,
IPC.localRuntimeStreamEvents and localRuntimeConnectionPool to mirror the
existing test structure.

In `@apps/desktop/src/renderer/components/app/App.workKeepAlive.test.tsx`:
- Around line 291-313: Extend this test ("keeps the Lanes page mounted after
visiting it and leaving for another tab") to perform the round-trip back to
lanes: after navigating to the files page (using the existing fireEvent.click on
the "Lanes open files" button and awaiting screen.findByTestId("files-page")),
simulate a user action that returns to lanes (add a "Return to lanes" button to
the FilesPage mock if needed), then assert that the lanes surface no longer has
an ancestor with aria-hidden='true', that
screen.getByTestId("lanes-page").getAttribute("data-active") === "true", and
that lanesLifecycle.mounts remains 1 and lanesLifecycle.unmounts remains 0 to
confirm it was not remounted.

In `@apps/desktop/src/shared/adeCliGuidance.ts`:
- Around line 3-15: Rename the exported constant ADE_BUNDLED_AGENT_SKILLS to
camelCase (e.g., adeBundledAgentSkills) and update every import and internal
usage to that new identifier; ensure the exported type remains "as const" and
preserve the array contents, and run a project-wide search/replace for
ADE_BUNDLED_AGENT_SKILLS to avoid unresolved references (update any tests or
consumers that import it as well).

In `@apps/desktop/src/shared/agentSkillRoots.ts`:
- Around line 45-47: Rename the exported constants to camelCase to follow repo
naming conventions: change ANCESTOR_SKILL_DIRS to ancestorSkillDirs and
PROMPT_AGENT_SKILL_ROOT_LIMIT to promptAgentSkillRootLimit, and update the
related type name or usages referencing AncestorSkillDir if you choose to adjust
it (or keep AncestorSkillDir as-is if type naming policy allows); then update
all references throughout the codebase to use the new names (search for
ANCESTOR_SKILL_DIRS and PROMPT_AGENT_SKILL_ROOT_LIMIT and replace with
ancestorSkillDirs and promptAgentSkillRootLimit) and run TypeScript checks to
ensure no broken imports or usages remain.
🪄 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: 6981b48b-cf5b-48b9-a717-e5da76fc3a74

📥 Commits

Reviewing files that changed from the base of the PR and between 2a49971 and 7a86591.

⛔ Files ignored due to path filters (19)
  • apps/ios/ADE.xcodeproj/project.pbxproj is excluded by !**/*.xcodeproj/project.pbxproj
  • docs/ARCHITECTURE.md is excluded by !docs/**
  • docs/features/ade-code/README.md is excluded by !docs/**
  • docs/features/agents/README.md is excluded by !docs/**
  • docs/features/agents/tool-registration.md is excluded by !docs/**
  • docs/features/chat/README.md is excluded by !docs/**
  • docs/features/chat/composer-and-ui.md is excluded by !docs/**
  • docs/features/computer-use/app-control.md is excluded by !docs/**
  • docs/features/deeplinks/README.md is excluded by !docs/**
  • docs/features/ios-simulator/README.md is excluded by !docs/**
  • docs/features/lanes/README.md is excluded by !docs/**
  • docs/features/project-home/README.md is excluded by !docs/**
  • docs/features/pull-requests/README.md is excluded by !docs/**
  • docs/features/remote-runtime/README.md is excluded by !docs/**
  • docs/features/remote-runtime/internal-architecture.md is excluded by !docs/**
  • docs/features/sync-and-multi-device/README.md is excluded by !docs/**
  • docs/features/sync-and-multi-device/remote-commands.md is excluded by !docs/**
  • docs/features/terminals-and-sessions/README.md is excluded by !docs/**
  • docs/playbooks/ship-lane.md is excluded by !docs/**
📒 Files selected for processing (44)
  • apps/ade-cli/README.md
  • apps/ade-cli/src/adeRpcServer.test.ts
  • apps/ade-cli/src/adeRpcServer.ts
  • apps/ade-cli/src/bootstrap.ts
  • apps/ade-cli/src/cli.test.ts
  • apps/ade-cli/src/cli.ts
  • apps/ade-cli/src/tuiClient/__tests__/commands.test.ts
  • apps/ade-cli/src/tuiClient/__tests__/rightPaneFormatters.test.ts
  • apps/ade-cli/src/tuiClient/app.tsx
  • apps/ade-cli/src/tuiClient/commands.ts
  • apps/ade-cli/src/tuiClient/rightPaneFormatters.ts
  • apps/desktop/resources/agent-skills/ade-deeplinks/SKILL.md
  • apps/desktop/resources/agent-skills/ade-pr-workflows/SKILL.md
  • apps/desktop/scripts/validate-mac-artifacts.mjs
  • apps/desktop/scripts/validate-win-artifacts.mjs
  • apps/desktop/src/main/services/ai/tools/ctoOperatorTools.ts
  • apps/desktop/src/main/services/ai/tools/systemPrompt.test.ts
  • apps/desktop/src/main/services/ai/tools/workflowTools.ts
  • apps/desktop/src/main/services/chat/agentChatService.test.ts
  • apps/desktop/src/main/services/chat/agentChatService.ts
  • apps/desktop/src/main/services/chat/claudeSlashCommandDiscovery.test.ts
  • apps/desktop/src/main/services/chat/claudeSlashCommandDiscovery.ts
  • apps/desktop/src/main/services/ipc/runtimeBridge.test.ts
  • apps/desktop/src/main/services/ipc/runtimeBridge.ts
  • apps/desktop/src/preload/preload.test.ts
  • apps/desktop/src/preload/preload.ts
  • apps/desktop/src/renderer/components/app/App.tsx
  • apps/desktop/src/renderer/components/app/App.workKeepAlive.test.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsx
  • apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
  • apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.test.ts
  • apps/desktop/src/renderer/components/lanes/useLaneWorkSessions.ts
  • apps/desktop/src/renderer/components/terminals/cliLaunch.test.ts
  • apps/desktop/src/renderer/state/appStore.test.ts
  • apps/desktop/src/renderer/state/appStore.ts
  • apps/desktop/src/shared/adeCliGuidance.test.ts
  • apps/desktop/src/shared/adeCliGuidance.ts
  • apps/desktop/src/shared/agentSkillRoots.test.ts
  • apps/desktop/src/shared/agentSkillRoots.ts
  • apps/desktop/src/shared/cliLaunch.ts
  • apps/ios/ADE/App/DeepLinkRouter.swift
  • apps/ios/ADE/App/DeepLinkURLParsing.swift
  • apps/ios/ADE/Views/Deeplinks/SendToMacCard.swift
  • apps/ios/ADETests/ADETests.swift

Comment thread apps/ade-cli/src/adeRpcServer.ts
Comment thread apps/desktop/resources/agent-skills/ade-deeplinks/SKILL.md
Comment thread apps/desktop/src/main/services/ipc/runtimeBridge.ts Outdated
Comment thread apps/desktop/src/preload/preload.ts
Comment thread apps/desktop/src/renderer/components/chat/AgentChatPane.tsx
Comment thread apps/ios/ADE/App/DeepLinkURLParsing.swift Outdated
@arul28
Copy link
Copy Markdown
Owner Author

arul28 commented May 21, 2026

@copilot review but do not make fixes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants