Conversation
…er routing - Add `create-lane` automation action plus per-action `targetLaneId`, `modelConfig`, and `permissionConfig` overrides; planner and executor merge them on top of rule-level config. - Add `forceHostRole` to syncService so the desktop instance hosting phone sync cannot be demoted to viewer by stale on-disk state. - Extend ade-cli with `proof attach` / `proof capture` and explicit `ownerKind` / `ownerId` routing on RPC proof tools (with `chat` / `pr` aliases). Capture-style commands run headless by default. - iOS Work surface refactor: model catalog/picker, session grouping helpers, per-host token shelf in keychain, paired auth payloads. - Tests: add coverage for the new planner/service branches, sync forceHostRole, and explicit proof owner resolution. - Docs: refresh automations, sync, proof, and ARCHITECTURE for the above. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR extends computer-use artifact ownership, adds create-lane automation actions with dynamic model/permission configuration, implements multi-host sync capabilities, enhances Claude V2 session resilience, and optimizes iOS session-presentation rendering via state caching. Changes span ADE CLI, desktop services, renderer components, type definitions, and iOS client. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
April 26, 2026 07:53
![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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/ios/ADE/Views/Work/WorkModelCatalog.swift (1)
488-507:⚠️ Potential issue | 🟡 MinorDon’t inject the current model into the wrong provider bucket.
providerKeyis derived straight fromcurrentProvider, and Line 500 falls back toproviders.startIndexwhen that bucket is missing. That misroutes active Cursor/OpenCode models into the runtime bucket or even the first curated provider instead of the upstream provider thatworkModelProviderKey(...)would choose.🛠️ Suggested fix
- let providerKey = providerLower.isEmpty ? "other" : providerLower + let providerKey = workCurrentModelProviderKey( + currentModelId: currentModelId, + currentProvider: currentProvider, + targetGroupKey: targetGroupKey + ) ... - let providerIndex = providers.firstIndex(where: { $0.key == providerKey }) ?? providers.startIndex - if !providers.isEmpty { + if let providerIndex = providers.firstIndex(where: { $0.key == providerKey }) { var rebuilt = providers let targetProvider = rebuilt[providerIndex] rebuilt[providerIndex] = WorkModelProvider( key: targetProvider.key, displayName: targetProvider.displayName, models: [injected] + targetProvider.models ) groups[groupIndex] = WorkModelCatalogGroup( key: groups[groupIndex].key, displayName: groups[groupIndex].displayName, providers: rebuilt ) + } else { + groups[groupIndex] = WorkModelCatalogGroup( + key: groups[groupIndex].key, + displayName: groups[groupIndex].displayName, + providers: providers + [ + WorkModelProvider( + key: providerKey, + displayName: workProviderDisplayName( + groupKey: targetGroupKey, + providerKey: providerKey, + curatedGroups: workCuratedModelCatalogGroups() + ), + models: [injected] + ) + ] + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Views/Work/WorkModelCatalog.swift` around lines 488 - 507, The code is incorrectly falling back to providers.startIndex when providerKey isn't found, which can insert the current model into the wrong bucket; update the logic in the block that handles groups[groupIndex] so that if providers.firstIndex(where: { $0.key == providerKey }) returns nil you create a new WorkModelProvider (using the correct key/displayName via workModelBrandKey or the same logic used elsewhere) with models: [injected] and insert it into groups[groupIndex].providers, otherwise preserve the existing behavior of prepending injected into the found provider's models; reference symbols: providerKey, injected (WorkModelOption), WorkModelProvider, groups, workModelCatalogGroupKey, workModelBrandKey.apps/ios/ADE/Views/Settings/SettingsPairingSection.swift (1)
225-257:⚠️ Potential issue | 🟠 MajorDon't hide the live discovery row unless the saved row is synchronized with current discovery.
reconnectToSavedHostreconnects via the stored profile/token path, not the tappedDiscoveredSyncHostitself. The saved rows displayed in the sheet are reconstructed from profiles loaded vialoadKnownProfiles()(which reloads from disk), while in-memory discovery updates flow only toactiveHostProfile. If a saved profile'sdiscoveredLanAddresseson disk are stale and this filter removes the matching live discovery row, the user is left with only a reconnect action that will retry obsolete endpoints. Either synchronize saved rows with live discovery data before rendering, or keep the live row visible as a fallback route.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Views/Settings/SettingsPairingSection.swift` around lines 225 - 257, The current filter that builds liveHosts from syncService.discoveredHosts removes a discovered row whenever a savedReconnectHost matches by id or hostIdentity, which can hide a usable discovery route if the saved profile on disk has stale discoveredLanAddresses; update the logic in the view that computes liveHosts (where syncService.savedReconnectHosts and syncService.discoveredHosts are used) to either (A) merge/synchronize discovery data into the corresponding savedReconnectHost before filtering — e.g., update savedHost.discoveredLanAddresses from the matching DiscoveredSyncHost or from activeHostProfile if available — and then exclude the discovered row only when the saved profile actually contains the current discovered endpoints, or (B) keep the discovered row visible as a fallback whenever the saved profile does not contain the discovered host’s current endpoints; ensure reconnectToSavedHost behavior remains unchanged but the UI preserves the DiscoveredSyncHost route unless the saved profile is verified to include the discovered endpoints.apps/desktop/src/main/services/chat/agentChatService.ts (1)
13980-13987:⚠️ Potential issue | 🟠 MajorDon't clear a queued continuity invalidation here.
If a busy Claude turn already queued
pendingSessionResetClearSdkSessionId = truebecausesetPermissionMode()failed, Line 13985 overwrites that back tofalse. A later reasoning-effort tweak in the same turn then makes the deferred reset reuse the stale resumed session id.💡 Proposed fix
if (prev !== next && managed.runtime?.kind === "claude" && (managed.runtime.v2Session || managed.runtime.v2WarmupDone)) { if (managed.runtime.busy) { // Defer session reset until the current turn completes — tearing down // a live session mid-turn would force the stream down the failure path. managed.runtime.pendingSessionReset = true; - managed.runtime.pendingSessionResetClearSdkSessionId = false; } else { resetClaudeV2Session(managed, managed.runtime, "session_reset"); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/chat/agentChatService.ts` around lines 13980 - 13987, The code is overwriting an existing pendingSessionResetClearSdkSessionId=true and thereby losing a queued continuity invalidation; change the branch that sets managed.runtime.pendingSessionResetClearSdkSessionId = false so it only clears the flag when it is not already true (e.g. check managed.runtime.pendingSessionResetClearSdkSessionId !== true before assigning), leaving any previously queued true value intact; this touches the busy-branch where you set managed.runtime.pendingSessionReset = true and should ensure interaction with setPermissionMode() and the deferred reset that calls resetClaudeV2Session preserves a previously queued clear request.apps/desktop/src/main/services/automations/automationService.ts (1)
1788-1812:⚠️ Potential issue | 🟡 MinorApply the publish-gate permission override to built-in
agent-sessionactions for consistency.
dispatchAgentSessionRun(lines 2103-2107) forcespermissionMode = "plan"whenrequiresPublishGate(rule)is true orverification.mode === "dry-run". However, the built-in pipeline branch foragent-sessionactions (lines 1788-1812) only callsresolveProviderPermissionMode(...)without applying this gate, allowing full edit/auto permissions even whenverifyBeforePublishis enabled and the rule uses publish-capable tools.🛡️ Suggested gate parity fix
const permissionConfig = buildPermissionConfig(rule, { publishPhase: false }, action); - const permissionMode = resolveProviderPermissionMode(providerGroup, permissionConfig); + const verificationRequired = requiresPublishGate(rule); + const dryRun = rule.verification.mode === "dry-run"; + const permissionMode = verificationRequired || dryRun + ? "plan" + : resolveProviderPermissionMode(providerGroup, permissionConfig);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/automations/automationService.ts` around lines 1788 - 1812, The agent-session branch sets permissionMode via resolveProviderPermissionMode(...) but doesn't apply the publish-gate override used elsewhere; after computing permissionMode (the variable assigned from resolveProviderPermissionMode in automationService.ts), enforce the same override logic as dispatchAgentSessionRun by forcing permissionMode = "plan" when requiresPublishGate(rule) returns true or when rule.execution?.verification?.mode === "dry-run"; update any related variables (permissionConfig/permissionMode) in the agent-session path so built-in agent sessions respect verifyBeforePublish/publish-capable tool restrictions.
🧹 Nitpick comments (14)
apps/ios/ADE/Views/Work/WorkTimelineHelpers.swift (1)
513-519: Dead"activity"branch left inworkToolGroupSoftBreak.Now that
.activityenvelopes never produce aWorkEventCardModel, the"activity"case on line 339 insideworkToolGroupSoftBreak(_:)is unreachable —eventCard(for:)is the only producer ofWorkEventCardModel, and soft-break dispatch only sees.eventCardpayloads. Consider dropping"activity"from that switch to keep the soft-break list honest about what can actually flow through the timeline.♻️ Proposed cleanup
switch card.kind { - case "status", "activity", "todo", "notice", + case "status", "todo", "notice", "autoApproval", "pendingInputResolved", "promptSuggestion", "toolUseSummary": return true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Views/Work/WorkTimelineHelpers.swift` around lines 513 - 519, The switch in workToolGroupSoftBreak(_:) contains an unreachable "activity" case because only eventCard(for:) produces WorkEventCardModel payloads for soft-breaks; remove the "activity" case from the switch and any associated comments so the switch only handles actual payload variants (e.g., .eventCard) and does not claim to handle activity envelopes, referencing workToolGroupSoftBreak(_:) and eventCard(for:) to locate the logic; ensure no behavior changes by running existing timeline tests after removal.apps/desktop/src/renderer/components/automations/GitHubTriggerFilters.tsx (1)
40-70: Consider debouncing repo lookups ontrigger.repokeystrokes.Adding
trigger.repoto the dependency array re-runs this effect on every keystroke in the Repository input (line 148 patchestrigger.repoon each character). WhileparseRepoSlugreturnsnullfor partial input (so the in-progress slug falls back toapi.detectRepo()), each keystroke still kicks off a freshdetectRepo+listRepoLabels+listRepoCollaboratorsround-trip. Thecancelledflag only discards stale results client-side; the network requests still hit GitHub and can burn through rate limits on a long-edited field.Consider debouncing the input (e.g., 250–400ms) before triggering the lookup, or only firing when
parseRepoSlugsucceeds (and usingdetectRepoonce on mount for the empty-field case).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/automations/GitHubTriggerFilters.tsx` around lines 40 - 70, The effect that calls load() on every change of trigger.repo (inside the useEffect that defines load, uses getGithubApi, parseRepoSlug, api.detectRepo, api.listRepoLabels, api.listRepoCollaborators, and the cancelled flag) should be debounced or gated so network requests are not fired on every keystroke; update the effect to either (a) debounce trigger.repo changes (250–400ms) before invoking load() (or use a debounced hook like useDebouncedValue) or (b) only call detectRepo when parseRepoSlug returns null on mount and thereafter only run lookups when parseRepoSlug returns a valid slug, ensuring you still respect the cancelled flag for stale results; apply the change around the existing load() invocation and dependency on trigger.repo so listRepoLabels/listRepoCollaborators/detectRepo calls are minimized.apps/desktop/src/shared/types/sync.ts (1)
242-249: Consider a discriminated union to encode auth-mode invariants.The current type allows unsafe combinations like
authKind: "bootstrap"with a missing/nulltoken. In practice, bootstrap always supplies a string token (persyncProtocol.test.tsandsyncHostService.test.ts), while paired always passestoken: null(permain.tslines 4102–4104). Runtime validation insyncPeerService.ts(line 343) checksdraft.authKind === "paired" && draft.pairedDeviceIdbefore constructing paired auth, falling back to bootstrap if either condition fails. A discriminated union would enforce these invariants at compile time:♻️ Suggested refactor
-export type SyncProjectConnectionPayload = { - authKind: "bootstrap" | "paired"; - token?: string | null; - pairedDeviceId?: string | null; - hostIdentity: SyncPairingQrPayload["hostIdentity"]; - port: number; - addressCandidates: SyncAddressCandidate[]; -}; +type SyncProjectConnectionBase = { + hostIdentity: SyncPairingQrPayload["hostIdentity"]; + port: number; + addressCandidates: SyncAddressCandidate[]; +}; +export type SyncProjectConnectionPayload = + | (SyncProjectConnectionBase & { authKind: "bootstrap"; token: string }) + | (SyncProjectConnectionBase & { + authKind: "paired"; + token?: null; + pairedDeviceId: string | null; + });The
pairedDeviceId: nullinmain.ts(line 4104) is intentional—it's the initial handoff before the iOS client identifies itself; the device ID is populated later bysyncHostService.ts(line 1712) during the hello handshake. The iOSMobileProjectConnectionPayloadalready tolerates optionaltoken, so the type change is compatible.🤖 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 242 - 249, The current SyncProjectConnectionPayload type allows invalid combinations (e.g., authKind: "bootstrap" with token null), so replace it with a discriminated union to enforce invariants: define two variants keyed by authKind—one { authKind: "bootstrap"; token: string; pairedDeviceId?: null; ... } and one { authKind: "paired"; token?: null; pairedDeviceId: string; ... }—so callers and callers like syncPeerService (checks around draft.authKind === "paired" and draft.pairedDeviceId) and places constructing payloads (e.g., main.ts where pairedDeviceId is initially null) get type-safe guarantees matching runtime behavior tested in syncProtocol.test.ts and syncHostService.test.ts. Ensure other code that expects optional token (MobileProjectConnectionPayload) remains compatible by adjusting usage sites to the new union.apps/desktop/src/main/services/ipc/registerIpc.ts (1)
2359-2426: Consider removing the repeated nestedawaitpattern in sync handlers.This is correct as-is, but
return await (await requireSyncService()).x()is hard to scan repeatedly. A small helper/local variable pattern would improve readability.Refactor sketch
+ const withSyncService = async <T>( + run: (service: ReturnType<typeof createSyncService>) => Promise<T> | T, + ): Promise<T> => { + const service = await requireSyncService(); + return await run(service); + }; + - ipcMain.handle(IPC.syncGetStatus, async (): Promise<SyncRoleSnapshot> => { - return await (await requireSyncService()).getStatus(); - }); + ipcMain.handle(IPC.syncGetStatus, async (): Promise<SyncRoleSnapshot> => { + return withSyncService((service) => service.getStatus()); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/ipc/registerIpc.ts` around lines 2359 - 2426, The handlers repeatedly use nested awaits like return await (await requireSyncService()).getStatus(); — simplify by awaiting the service once into a local (e.g., const svc = await requireSyncService()) inside each ipcMain.handle and then call the relevant method on svc (e.g., return await svc.getStatus() or simply return svc.getStatus() for direct returns). Update all handlers referencing requireSyncService (handlers for IPC.syncGetStatus, IPC.syncRefreshDiscovery, IPC.syncListDevices, IPC.syncUpdateLocalDevice, IPC.syncConnectToBrain, IPC.syncDisconnectFromBrain, IPC.syncForgetDevice, IPC.syncGetTransferReadiness, IPC.syncTransferBrainToLocal, IPC.syncGetPin, IPC.syncSetPin, IPC.syncClearPin, IPC.syncSetActiveLanePresence) to use this pattern to improve readability and remove the double-await noise.apps/desktop/src/main/services/sync/syncService.test.ts (2)
1025-1027: Minor: prefertoBeInstanceOf(Promise)over a structuralthen-check.- const result = service.setHostStartupEnabled(true); - expect(typeof (result as any)?.then).toBe("function"); - await result; + const result = service.setHostStartupEnabled(true); + expect(result).toBeInstanceOf(Promise); + await result;Stronger assertion and avoids the
as anycast. Functionally equivalent for this test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/sync/syncService.test.ts` around lines 1025 - 1027, Replace the structural then-check on the returned value from service.setHostStartupEnabled(true) with a stronger instance assertion: assert that the result is a Promise using expect(result).toBeInstanceOf(Promise) (remove the "as any" cast), then continue to await result; this change targets the test around the call to setHostStartupEnabled in syncService.test.ts.
614-618: AwaitsetHostStartupEnablednow that it's async.Per the PR objectives,
setHostStartupEnabledis nowPromise<void>, and the new test at lines 1025–1027 explicitly verifies the awaitable contract. This pre-existing call site at line 614 still invokes it as fire-and-forget, leaving a floating promise thatvi.waitForhappens to absorb. Worth tightening for consistency and to surface any future error fromrefreshRoleState:♻️ Suggested change
- service.setHostStartupEnabled(true); - - await vi.waitFor(() => { - expect(createSyncHostServiceMock).toHaveBeenCalled(); - }); + await service.setHostStartupEnabled(true); + expect(createSyncHostServiceMock).toHaveBeenCalled();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/sync/syncService.test.ts` around lines 614 - 618, The test currently calls setHostStartupEnabled(true) without awaiting it, leaving a floating Promise; change the call to await setHostStartupEnabled(true) so the test waits for refreshRoleState and any errors to surface before vi.waitFor, ensuring createSyncHostServiceMock invocation is observed reliably by the assertion; update the invocation in the test to await the Promise returned by setHostStartupEnabled.apps/ios/ADE/Services/KeychainService.swift (1)
9-11: Consider documenting thehostKeycontract.
tokenAccount(forHostKey:)interpolateshostKeydirectly into the keychain account string. Two minor things worth doing:
- Document (or assert) that
hostKeymust be non-empty — an emptyhostKeywould produce account"connection-token.", which silently shadows neither the legacy account nor a real host token but is still a footgun.- Decide on case/whitespace normalization at one chokepoint (e.g., where
SyncServicederives the host key) so a host likeMac-Studio.localandmac-studio.localcannot end up with two distinct keychain entries on different code paths.Otherwise the host-scoped token API mirrors the existing pattern cleanly. No security concerns — items are added with
kSecAttrAccessibleAfterFirstUnlockvia the sharedsaveStringhelper.Also applies to: 57-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ios/ADE/Services/KeychainService.swift` around lines 9 - 11, tokenAccount(forHostKey:) currently interpolates hostKey directly which can produce ambiguous or duplicate keychain accounts; update the function and related host-scoped keychain helpers to assert/guard that hostKey is non-empty (or document the contract) and normalize the hostKey (e.g., trim whitespace and lowercase) at a single chokepoint (either inside tokenAccount(forHostKey:) or where SyncService derives the host key) so hosts like "Mac-Studio.local" and "mac-studio.local" map to the same account; apply the same non-empty check and normalization to the other host-scoped token functions referenced in the same region (lines 57-75) and update callers or add unit tests to reflect the normalized contract.apps/desktop/src/main/services/chat/agentChatService.test.ts (1)
2402-2411: Decouple this recovery test from lane-directive persistence.
lastLaneDirectiveKeyis orthogonal to thebypassPermissionsfallback path, so this assertion makes the test fail on unrelated directive/persistence changes. Seed a fixed sentinel value here instead of depending on the earlier turn to have produced one.Suggested change
const persistedAfterPrime = readPersistedChatState(session.id); - expect(persistedAfterPrime.lastLaneDirectiveKey).toBeTruthy(); + const laneDirectiveKey = "test-lane-directive"; writePersistedChatState(session.id, { ...persistedAfterPrime, sdkSessionId: "sdk-stale", - lastLaneDirectiveKey: persistedAfterPrime.lastLaneDirectiveKey, + lastLaneDirectiveKey: laneDirectiveKey, claudePermissionMode: "bypassPermissions", permissionMode: "full-auto", });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/chat/agentChatService.test.ts` around lines 2402 - 2411, The test should not rely on a previously persisted lastLaneDirectiveKey; instead of asserting expect(persistedAfterPrime.lastLaneDirectiveKey) and reusing persistedAfterPrime.lastLaneDirectiveKey when calling writePersistedChatState, seed a fixed sentinel value for lastLaneDirectiveKey (e.g., "test-sentinel-lane-key") and remove the dependency on persistedAfterPrime for that field; update the writePersistedChatState call so it spreads persistedAfterPrime but overrides lastLaneDirectiveKey with the sentinel and keeps sdkSessionId, claudePermissionMode, and permissionMode as intended, using the existing readPersistedChatState, writePersistedChatState, persistedAfterPrime, and session.id identifiers to locate the change.apps/desktop/src/main/services/automations/automationPlannerService.ts (1)
641-654: Per-actionmodelConfigspread leaks unsanitized fields.
actionModelConfigspreads...rawModelConfigand then overrides onlymodelId(and optionallythinkingLevel). Any other properties in the caller-supplied object — including a non-stringthinkingLevelwhensafeTrimreturns""and the conditional skips the override — pass through verbatim into the normalized rule and end up persisted to project config. Consider whitelisting fields explicitly, e.g.:♻️ Suggested narrowing
- const actionModelConfig = rawModelConfig && typeof rawModelConfig === "object" && modelConfigModelId - ? { - ...rawModelConfig, - modelId: modelConfigModelId, - ...(modelConfigThinkingLevel ? { thinkingLevel: modelConfigThinkingLevel } : {}), - } - : null; + const actionModelConfig = rawModelConfig && typeof rawModelConfig === "object" && modelConfigModelId + ? { + modelId: modelConfigModelId, + ...(modelConfigThinkingLevel ? { thinkingLevel: modelConfigThinkingLevel } : {}), + } + : null;Same idea applies to
actionPermissionConfig, which currently passes the object through with only atypeof === "object"check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/automations/automationPlannerService.ts` around lines 641 - 654, The current creation of actionModelConfig and actionPermissionConfig leaks arbitrary caller-supplied fields by spreading rawModelConfig and action.permissionConfig; instead, build sanitized objects by whitelisting and validating fields: read rawModelConfig and use safeTrim(modelId) and safeTrim(thinkingLevel) (or explicit type checks) to set modelId and thinkingLevel only when valid, and copy only allowed fields (e.g., modelId, thinkingLevel, temperature, maxTokens — whatever your schema permits) into the new actionModelConfig object; similarly, replace the passthrough actionPermissionConfig with a new object that only includes explicitly allowed permission keys (e.g., role, users, level) after validating their types, and set to null if validation fails.apps/desktop/src/renderer/components/automations/ActionList.tsx (1)
27-48: Note: blank-action templates assume an issue trigger.The defaults reference
{{trigger.issue.title}}/{{trigger.issue.number}}/{{trigger.issue.url}}/{{trigger.issue.body}}, which only resolve forgithub.issue_*triggers. For other trigger types these render empty and the planner correctly falls back torule.name(verified by theautomationService.test.ts"fallback to rule.name when create-lane template renders empty" test), but users addingcreate-laneto non-issue triggers may be surprised. Consider tailoring the seed template to the rule's trigger type, or showing a hint inActionRowwhen the placeholders won't resolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/automations/ActionList.tsx` around lines 27 - 48, The createBlankAction function currently seeds create-lane templates with issue placeholders that only resolve for github.issue triggers; update the implementation (createBlankAction) to accept the rule/trigger type (e.g., add a parameter triggerType or rule) and choose a seed template accordingly (use "{{trigger.issue.title}}"/"#{{trigger.issue.number}}" only for github.issue triggers, otherwise use a neutral template like "{{rule.name}}" or an empty sensible default), and update the UI component (ActionRow) to display a small hint when kind === "create-lane" explaining which placeholders will resolve for the selected trigger type so users aren’t surprised. Ensure references to ActionRowKind/ActionRowValue remain compatible and adjust any call sites to pass the new triggerType parameter.apps/desktop/src/main/services/automations/automationService.ts (1)
1160-1168:cursorprovider lacks a default — inconsistent with siblings.
claude,codex, andopencodeall default to a sensible mode via?? "edit"/?? "default", butcursoris left asproviders.cursor(potentiallyundefined).resolveProviderPermissionModepapers over this with its own?? "edit"fallback at line 1178, but the storedpermissionConfig.providers.cursorends upundefined(vs other providers carrying a concrete value). Either drop the explicitcursorkey when undefined, or apply a default symmetric to its siblings:♻️ Suggested symmetry
- cursor: rule.verification.mode === "dry-run" ? "plan" : providers.cursor, + cursor: rule.verification.mode === "dry-run" ? "plan" : (providers.cursor ?? "edit"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/services/automations/automationService.ts` around lines 1160 - 1168, The providers object sets defaults for claude/codex/opencode but leaves cursor as providers.cursor (possibly undefined), causing permissionConfig.providers.cursor to be undefined; change the cursor entry in the providers object inside the automationService.ts provider-building block to mirror the others (i.e., cursor: rule.verification.mode === "dry-run" ? "plan" : (providers.cursor ?? "edit")) so cursor always gets a concrete default (or alternatively omit the cursor key when undefined), ensuring symmetry with claude/codex/opencode and matching resolveProviderPermissionMode's expectations.apps/desktop/src/renderer/components/automations/permissionControls.ts (1)
18-22: Nit:config-tomlfalls into the default branch.
codexSandboxForModereturns"workspace-write"for any mode that is notfull-auto/plan, includingconfig-toml. The caller already gates thecodexSandboxpatch withmode !== "config-toml"(line 38), so this is harmless today, but it makes the helper a slightly leaky abstraction. A short comment or an explicitif (mode === "config-toml") return "workspace-write";(or returningnull) would document the assumption.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/automations/permissionControls.ts` around lines 18 - 22, codexSandboxForMode currently maps any non-"full-auto"/"plan" mode (including "config-toml") to "workspace-write", which leaks the caller's gating assumption; make the mapping explicit by adding an explicit branch for "config-toml" (e.g., if (mode === "config-toml") return "workspace-write";) or return null for unsupported modes and update callers (e.g., the code that currently checks mode !== "config-toml") to handle a null return, so the helper's behavior is documented and not implicit.apps/desktop/src/renderer/components/automations/ActionRow.tsx (1)
210-229: "Use rule" button leavespermissionConfigoverride in place.Clearing
modelConfigresets the model to the rule default, but the action-levelpermissionConfig(which was likely picked for the previously-selected model) remains. After resetting, the permission select to the right will derive its key from the newactiveModel.modelId(the rule default), so the previously-saved per-action mode keyed under a different provider becomes orphaned invalue.permissionConfig.providers— silently shipped on save.♻️ Suggested reset
<button type="button" className="text-[10px] text-[`#8FA1B8`] hover:text-[`#F5FAFF`]" - onClick={() => onChange({ ...value, modelConfig: undefined })} + onClick={() => onChange({ ...value, modelConfig: undefined, permissionConfig: undefined })} > Use rule </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/automations/ActionRow.tsx` around lines 210 - 229, The "Use rule" button clears modelConfig but leaves action-level permissionConfig entries keyed to the previously-selected provider, causing orphaned provider entries; in the onClick handler where you call onChange({ ...value, modelConfig: undefined }) (in the same block that renders ModelSelector and the "Use rule" button), also remove or reset any per-action overrides in value.permissionConfig.providers that reference the old model/provider (or clear permissionConfig entirely) so the permissions will derive from the new activeModel.modelId; update the onChange payload to include the cleaned/cleared permissionConfig (e.g., remove entries in value.permissionConfig.providers not matching the resulting activeModel.modelId or set permissionConfig to undefined) to avoid shipping stale provider keys.apps/desktop/src/renderer/components/automations/components/RuleEditorPanel.tsx (1)
150-196: Soloagent-sessiondraft-to-row mapping drops rule-levelmodelConfig/permissionConfig/targetLaneId.The
built-inbranch (lines 181-189) faithfully threadstargetLaneId,modelConfig, andpermissionConfigfrom the action into the row, but theagent-sessionexecution branch (lines 153-158) only carriespromptandsessionTitle. Result: when a rule's only action is an agent-session, the action-row UI inActionRowwill show a blanktargetLaneId/ "Use rule" model / "Rule permissions" even though those values are configured on the rule itself.This isn't a correctness bug —
applyActionRowsToDraftonly overwritesdraft.modelConfig/draft.permissionConfigwhen the row carries truthy values, so existing draft state is preserved on save. But the UX is misleading and the asymmetry will keep biting future readers. Consider populating the row from the draft so the rule-level "Execution" section and the row stay in sync:♻️ Suggested symmetric mapping
if (execution?.kind === "agent-session") { rows.push({ kind: "agent-session", prompt: draft.prompt ?? "", sessionTitle: execution.session?.title ?? "", + targetLaneId: execution.targetLaneId ?? null, + modelConfig: draft.modelConfig?.orchestratorModel, + permissionConfig: draft.permissionConfig, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/automations/components/RuleEditorPanel.tsx` around lines 150 - 196, The agent-session branch in draftToActionRows omits rule-level targetLaneId/modelConfig/permissionConfig so the ActionRow UI looks out-of-sync; update the agent-session row construction in draftToActionRows to include targetLaneId (use execution.targetLaneId ?? draft.targetLaneId ?? null) and include modelConfig and permissionConfig (use execution.modelConfig ?? draft.modelConfig and execution.permissionConfig ?? draft.permissionConfig) so the row mirrors the rule-level execution settings (refer to function draftToActionRows and the agent-session branch).
🤖 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/ade-cli/src/adeRpcServer.ts`:
- Around line 2076-2078: The current check uses asOptionalTrimmedString on
toolArgs.ownerKind and toolArgs.ownerId and silently returns if either is
missing (rawKind or ownerId), which allows half-specified explicit owners to be
ignored; change this to explicitly reject half-specified owner input: detect if
either ownerKind or ownerId is provided but the other is missing and return an
error (or throw a BadRequest/ClientError) rather than early-returning, otherwise
proceed when both are present; update the validation around rawKind/ownerId in
the same block where asOptionalTrimmedString(toolArgs.ownerKind) and
asOptionalTrimmedString(toolArgs.ownerId) are computed.
- Around line 2080-2095: The new ownerKind/ownerId path always calls
add(normalizedKind, ownerId) which uses the default relation (attached_to),
differing from the prUrl/linearIssueId legacy paths that create
github_pr/linear_issue links with relation published_to; update the switch so
when normalizedKind is "github_pr" or "linear_issue" you pass the published_to
relation to add (or call the equivalent helper) instead of the default, and keep
attached_to for other kinds—i.e., use add(normalizedKind, ownerId,
"published_to") for those two cases, leaving ownerKind/ownerId, normalizedKind
and add as the referenced symbols to locate the change.
In `@apps/ade-cli/src/cli.ts`:
- Around line 1576-1583: VALUE_CARRIER_FLAGS is missing the new value-taking
flags, causing hasHelpFlag to mis-detect --help when flags like --owner,
--owner-kind, --owner-id, --caption are used; update the VALUE_CARRIER_FLAGS set
(the constant near the existing alphabetical groupings) to include "--owner",
"--owner-kind", "--owner-id", and "--caption" so that readValue calls in
proofOwnerBase and in attach/capture/record are correctly recognized as
value-carrying options and hasHelpFlag behaves consistently.
In `@apps/desktop/src/main/main.ts`:
- Around line 935-951: reconcileSyncHostContexts can run concurrently causing
stale invocations to flip the wrong host; serialize all calls by introducing a
module-scoped serial Promise and enqueue reconciles through it instead of firing
reconcileSyncHostContexts() directly. Concretely, add a top-level Promise<void>
(e.g., reconcileSyncSerial = Promise.resolve()) and change setActiveProject to
do reconcileSyncSerial = reconcileSyncSerial.then(() =>
reconcileSyncHostContexts()).then(() =>
notifyMobileSyncProjectCatalogChanged()); ensure any other callers of
reconcileSyncHostContexts also enqueue through reconcileSyncSerial so
projectContexts/getMobileSyncHostRoot/activeProjectRoot changes are applied in
order.
In `@apps/desktop/src/main/services/automations/automationService.ts`:
- Around line 1736-1741: The lane name can be a literal unresolved template like
"{{trigger.issue.title}}" when resolveTemplateString/resolvePlaceholders returns
the original template for missing paths; update the create-lane branch (where
fallbackName, laneName are computed) to detect unresolved template residue and
treat it as empty before using fallbackName: after computing laneName =
resolveTemplateString(action.laneNameTemplate, trigger) || fallbackName, check
if laneName matches a leftover placeholder pattern (e.g., contains "{{" and "}}"
or matches /^\s*\{\{.*\}\}\s*$/) or is only template tokens, and if so set
laneName = fallbackName (or fallbackName.trim()) so unresolved templates fall
back to rule.name/trigger values; reference resolveTemplateString,
resolvePlaceholders, laneName and fallbackName in the change.
In `@apps/desktop/src/main/services/chat/agentChatService.test.ts`:
- Around line 4132-4188: The test is asserting a generic `status` property but
the resumed start notification sets `turnStatus` on status envelopes, so update
the event mapping and assertion to check `turnStatus` instead of `status`: in
the onEvent handler (collecting into events) map the status value from
`event.event.turnStatus` (i.e., use "turnStatus" in event.event) into the
captured object (replace the current `status` mapping), and update the
expectation for the resumed start to include { type: "status", turnId:
"resumed-turn", turnStatus: "inProgress" } so the `turn/started` notification
emitted via `mockState.emitCodexPayload` is actually verified; references:
AgentChatEventEnvelope, events array, service.createSession /
service.resumeSession, and mockState.emitCodexPayload.
In `@apps/desktop/src/main/services/chat/agentChatService.ts`:
- Around line 10156-10176: In resetClaudeV2Session, when clearing
runtime.sdkSessionId you must also refresh the chat reconstruction buffer so
context isn’t lost; update resetClaudeV2Session to rebuild or repopulate
managed.pendingReconstructionContext (e.g., by invoking an existing helper or a
small new helper like rebuildPendingReconstructionContext(managed) that
reconstructs context from managed.session/history) before or after setting
managed.runtimeInvalidated = true so that runClaudeTurn() and permission-mode
recovery retain prior chat context; reference resetClaudeV2Session,
runtime.sdkSessionId, managed.pendingReconstructionContext and runClaudeTurn in
your change.
In `@apps/desktop/src/main/services/sync/syncService.ts`:
- Around line 860-864: The method setHostStartupEnabled was changed to async and
now awaits refreshRoleState(), so callers must await its Promise to avoid
floating promises/unhandled rejections; update the test that calls
service.setHostStartupEnabled(true) (in syncService.test.ts) to await
service.setHostStartupEnabled(true) (or explicitly attach .catch(err => {/*fail
or noop*/}) ) so any rejection from refreshRoleState/startHostIfNeeded is
handled.
In `@apps/desktop/src/renderer/components/automations/permissionControls.ts`:
- Around line 24-41: Currently patchPermissionConfig returns undefined when
rawMode is empty, which wipes the entire permissionConfig; change it to only
remove the specific provider override instead: if !rawMode and permissionConfig
is undefined return undefined; otherwise clone permissionConfig, clone
permissionConfig.providers (or start {}), delete the entry at [meta.key], and if
meta.key === "codex" also remove codexSandbox from providers; return the updated
permissionConfig object preserving all other top-level fields (including
deprecated shapes, writablePaths, allowedTools). Use the existing symbols
permissionConfig, providers, meta.key, and codexSandboxForMode to locate and
implement this logic in patchPermissionConfig.
In `@apps/ios/ADE/Services/SyncService.swift`:
- Around line 1070-1076: The issue is that saveProfile(profile) (which upserts
into profilesKey) is called before the connection is confirmed, causing stale
host entries if connectUsingProfile fails; instead, persist only the global
token (keychain.saveToken) and keep teardownSocket/connection logic as-is, then
call saveProfile(profile) and any host-scoped persistence only after
connectUsingProfile returns successfully; on the failure path ensure you do not
leave a profilesKey entry (remove/avoid creating it) and preserve the existing
profileKey/global token restore behavior so savedReconnectHosts does not contain
hosts for failed switches.
- Around line 1410-1445: upsertKnownProfile currently only filters out entries
whose profileStorageKey equals the new key, so when a profile gains a
hostIdentity older entries stored under legacy keys (route:/name:) remain;
update upsertKnownProfile to also prune any saved profiles that represent the
same host even if their storage key differs by: compute key =
profileStorageKey(profile), load existingProfiles = loadKnownProfiles(), then
build filtered = existingProfiles.filter { profileStorageKey($0) != key &&
!(profile.hostIdentity != nil && $0.hostIdentity == profile.hostIdentity) }
(i.e., remove any profile whose profileStorageKey matches the new key OR whose
hostIdentity equals the new profile's hostIdentity when hostIdentity is
present), then saveKnownProfiles([profile] + filtered) and preserve token
handling via keychain.saveToken as before.
- Around line 4349-4350: The disconnect call on host-identity mismatch currently
uses the default suspendAutoReconnect (true), which erroneously pauses
auto-reconnect; update the call in the if block that checks expectedHostIdentity
vs remoteHostIdentity to pass suspendAutoReconnect: false (e.g.,
disconnect(clearCredentials: false, suspendAutoReconnect: false)) so the service
does not mark reconnect as user-paused when a wrong-host hello is received.
In `@apps/ios/ADE/Views/PRs/PrsRootScreen.swift`:
- Line 358: Remove the now-unused animated background by deleting the
PrsLiquidBackdrop view and its supporting PrsBackdropPalette type from the
codebase (they are referenced together in PrListRowModifier.swift); update any
imports or modifiers that referenced PrsLiquidBackdrop and run a project-wide
search to ensure no remaining references to PrsLiquidBackdrop or
PrsBackdropPalette exist, then run a build to confirm nothing else depended on
them.
In `@apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift`:
- Around line 108-117: The observation Task bound to artifactObservationKey
relies on localStateRevision but SyncService never increments localStateRevision
on artifact/proof/computer-use syncs, so artifact updates don't trigger the
.task refreshArtifacts call; modify SyncService to increment (or bump)
localStateRevision whenever artifact-table or proof/computer-use writes are
applied (the same place where project selection/connection/auth/chat
subscription changes currently bump it) so the task observing localStateRevision
will re-run and call refreshArtifacts(force: false) when those CRDT-backed
artifact updates arrive.
In `@apps/ios/ADE/Views/Work/WorkSessionGrouping.swift`:
- Around line 113-127: The awaiting-input branch sets firstGlobalLiveSessionId
but never increments globalLiveSessionCount, causing a mismatch; inside the
status == "awaiting-input" block increment globalLiveSessionCount
(globalLiveSessionCount += 1) alongside the existing globalNeedsInputCount and
firstGlobalLiveSessionId assignment so that firstGlobalLiveSessionId and
globalLiveSessionCount remain consistent; locate and update the guarded loop
that uses isRunOwnedSession(session), isArchived, status, globalNeedsInputCount,
firstGlobalAttentionSessionId, firstGlobalLiveSessionId, and
globalLiveSessionCount.
---
Outside diff comments:
In `@apps/desktop/src/main/services/automations/automationService.ts`:
- Around line 1788-1812: The agent-session branch sets permissionMode via
resolveProviderPermissionMode(...) but doesn't apply the publish-gate override
used elsewhere; after computing permissionMode (the variable assigned from
resolveProviderPermissionMode in automationService.ts), enforce the same
override logic as dispatchAgentSessionRun by forcing permissionMode = "plan"
when requiresPublishGate(rule) returns true or when
rule.execution?.verification?.mode === "dry-run"; update any related variables
(permissionConfig/permissionMode) in the agent-session path so built-in agent
sessions respect verifyBeforePublish/publish-capable tool restrictions.
In `@apps/desktop/src/main/services/chat/agentChatService.ts`:
- Around line 13980-13987: The code is overwriting an existing
pendingSessionResetClearSdkSessionId=true and thereby losing a queued continuity
invalidation; change the branch that sets
managed.runtime.pendingSessionResetClearSdkSessionId = false so it only clears
the flag when it is not already true (e.g. check
managed.runtime.pendingSessionResetClearSdkSessionId !== true before assigning),
leaving any previously queued true value intact; this touches the busy-branch
where you set managed.runtime.pendingSessionReset = true and should ensure
interaction with setPermissionMode() and the deferred reset that calls
resetClaudeV2Session preserves a previously queued clear request.
In `@apps/ios/ADE/Views/Settings/SettingsPairingSection.swift`:
- Around line 225-257: The current filter that builds liveHosts from
syncService.discoveredHosts removes a discovered row whenever a
savedReconnectHost matches by id or hostIdentity, which can hide a usable
discovery route if the saved profile on disk has stale discoveredLanAddresses;
update the logic in the view that computes liveHosts (where
syncService.savedReconnectHosts and syncService.discoveredHosts are used) to
either (A) merge/synchronize discovery data into the corresponding
savedReconnectHost before filtering — e.g., update
savedHost.discoveredLanAddresses from the matching DiscoveredSyncHost or from
activeHostProfile if available — and then exclude the discovered row only when
the saved profile actually contains the current discovered endpoints, or (B)
keep the discovered row visible as a fallback whenever the saved profile does
not contain the discovered host’s current endpoints; ensure reconnectToSavedHost
behavior remains unchanged but the UI preserves the DiscoveredSyncHost route
unless the saved profile is verified to include the discovered endpoints.
In `@apps/ios/ADE/Views/Work/WorkModelCatalog.swift`:
- Around line 488-507: The code is incorrectly falling back to
providers.startIndex when providerKey isn't found, which can insert the current
model into the wrong bucket; update the logic in the block that handles
groups[groupIndex] so that if providers.firstIndex(where: { $0.key ==
providerKey }) returns nil you create a new WorkModelProvider (using the correct
key/displayName via workModelBrandKey or the same logic used elsewhere) with
models: [injected] and insert it into groups[groupIndex].providers, otherwise
preserve the existing behavior of prepending injected into the found provider's
models; reference symbols: providerKey, injected (WorkModelOption),
WorkModelProvider, groups, workModelCatalogGroupKey, workModelBrandKey.
---
Nitpick comments:
In `@apps/desktop/src/main/services/automations/automationPlannerService.ts`:
- Around line 641-654: The current creation of actionModelConfig and
actionPermissionConfig leaks arbitrary caller-supplied fields by spreading
rawModelConfig and action.permissionConfig; instead, build sanitized objects by
whitelisting and validating fields: read rawModelConfig and use
safeTrim(modelId) and safeTrim(thinkingLevel) (or explicit type checks) to set
modelId and thinkingLevel only when valid, and copy only allowed fields (e.g.,
modelId, thinkingLevel, temperature, maxTokens — whatever your schema permits)
into the new actionModelConfig object; similarly, replace the passthrough
actionPermissionConfig with a new object that only includes explicitly allowed
permission keys (e.g., role, users, level) after validating their types, and set
to null if validation fails.
In `@apps/desktop/src/main/services/automations/automationService.ts`:
- Around line 1160-1168: The providers object sets defaults for
claude/codex/opencode but leaves cursor as providers.cursor (possibly
undefined), causing permissionConfig.providers.cursor to be undefined; change
the cursor entry in the providers object inside the automationService.ts
provider-building block to mirror the others (i.e., cursor:
rule.verification.mode === "dry-run" ? "plan" : (providers.cursor ?? "edit")) so
cursor always gets a concrete default (or alternatively omit the cursor key when
undefined), ensuring symmetry with claude/codex/opencode and matching
resolveProviderPermissionMode's expectations.
In `@apps/desktop/src/main/services/chat/agentChatService.test.ts`:
- Around line 2402-2411: The test should not rely on a previously persisted
lastLaneDirectiveKey; instead of asserting
expect(persistedAfterPrime.lastLaneDirectiveKey) and reusing
persistedAfterPrime.lastLaneDirectiveKey when calling writePersistedChatState,
seed a fixed sentinel value for lastLaneDirectiveKey (e.g.,
"test-sentinel-lane-key") and remove the dependency on persistedAfterPrime for
that field; update the writePersistedChatState call so it spreads
persistedAfterPrime but overrides lastLaneDirectiveKey with the sentinel and
keeps sdkSessionId, claudePermissionMode, and permissionMode as intended, using
the existing readPersistedChatState, writePersistedChatState,
persistedAfterPrime, and session.id identifiers to locate the change.
In `@apps/desktop/src/main/services/ipc/registerIpc.ts`:
- Around line 2359-2426: The handlers repeatedly use nested awaits like return
await (await requireSyncService()).getStatus(); — simplify by awaiting the
service once into a local (e.g., const svc = await requireSyncService()) inside
each ipcMain.handle and then call the relevant method on svc (e.g., return await
svc.getStatus() or simply return svc.getStatus() for direct returns). Update all
handlers referencing requireSyncService (handlers for IPC.syncGetStatus,
IPC.syncRefreshDiscovery, IPC.syncListDevices, IPC.syncUpdateLocalDevice,
IPC.syncConnectToBrain, IPC.syncDisconnectFromBrain, IPC.syncForgetDevice,
IPC.syncGetTransferReadiness, IPC.syncTransferBrainToLocal, IPC.syncGetPin,
IPC.syncSetPin, IPC.syncClearPin, IPC.syncSetActiveLanePresence) to use this
pattern to improve readability and remove the double-await noise.
In `@apps/desktop/src/main/services/sync/syncService.test.ts`:
- Around line 1025-1027: Replace the structural then-check on the returned value
from service.setHostStartupEnabled(true) with a stronger instance assertion:
assert that the result is a Promise using expect(result).toBeInstanceOf(Promise)
(remove the "as any" cast), then continue to await result; this change targets
the test around the call to setHostStartupEnabled in syncService.test.ts.
- Around line 614-618: The test currently calls setHostStartupEnabled(true)
without awaiting it, leaving a floating Promise; change the call to await
setHostStartupEnabled(true) so the test waits for refreshRoleState and any
errors to surface before vi.waitFor, ensuring createSyncHostServiceMock
invocation is observed reliably by the assertion; update the invocation in the
test to await the Promise returned by setHostStartupEnabled.
In `@apps/desktop/src/renderer/components/automations/ActionList.tsx`:
- Around line 27-48: The createBlankAction function currently seeds create-lane
templates with issue placeholders that only resolve for github.issue triggers;
update the implementation (createBlankAction) to accept the rule/trigger type
(e.g., add a parameter triggerType or rule) and choose a seed template
accordingly (use "{{trigger.issue.title}}"/"#{{trigger.issue.number}}" only for
github.issue triggers, otherwise use a neutral template like "{{rule.name}}" or
an empty sensible default), and update the UI component (ActionRow) to display a
small hint when kind === "create-lane" explaining which placeholders will
resolve for the selected trigger type so users aren’t surprised. Ensure
references to ActionRowKind/ActionRowValue remain compatible and adjust any call
sites to pass the new triggerType parameter.
In `@apps/desktop/src/renderer/components/automations/ActionRow.tsx`:
- Around line 210-229: The "Use rule" button clears modelConfig but leaves
action-level permissionConfig entries keyed to the previously-selected provider,
causing orphaned provider entries; in the onClick handler where you call
onChange({ ...value, modelConfig: undefined }) (in the same block that renders
ModelSelector and the "Use rule" button), also remove or reset any per-action
overrides in value.permissionConfig.providers that reference the old
model/provider (or clear permissionConfig entirely) so the permissions will
derive from the new activeModel.modelId; update the onChange payload to include
the cleaned/cleared permissionConfig (e.g., remove entries in
value.permissionConfig.providers not matching the resulting activeModel.modelId
or set permissionConfig to undefined) to avoid shipping stale provider keys.
In
`@apps/desktop/src/renderer/components/automations/components/RuleEditorPanel.tsx`:
- Around line 150-196: The agent-session branch in draftToActionRows omits
rule-level targetLaneId/modelConfig/permissionConfig so the ActionRow UI looks
out-of-sync; update the agent-session row construction in draftToActionRows to
include targetLaneId (use execution.targetLaneId ?? draft.targetLaneId ?? null)
and include modelConfig and permissionConfig (use execution.modelConfig ??
draft.modelConfig and execution.permissionConfig ?? draft.permissionConfig) so
the row mirrors the rule-level execution settings (refer to function
draftToActionRows and the agent-session branch).
In `@apps/desktop/src/renderer/components/automations/GitHubTriggerFilters.tsx`:
- Around line 40-70: The effect that calls load() on every change of
trigger.repo (inside the useEffect that defines load, uses getGithubApi,
parseRepoSlug, api.detectRepo, api.listRepoLabels, api.listRepoCollaborators,
and the cancelled flag) should be debounced or gated so network requests are not
fired on every keystroke; update the effect to either (a) debounce trigger.repo
changes (250–400ms) before invoking load() (or use a debounced hook like
useDebouncedValue) or (b) only call detectRepo when parseRepoSlug returns null
on mount and thereafter only run lookups when parseRepoSlug returns a valid
slug, ensuring you still respect the cancelled flag for stale results; apply the
change around the existing load() invocation and dependency on trigger.repo so
listRepoLabels/listRepoCollaborators/detectRepo calls are minimized.
In `@apps/desktop/src/renderer/components/automations/permissionControls.ts`:
- Around line 18-22: codexSandboxForMode currently maps any
non-"full-auto"/"plan" mode (including "config-toml") to "workspace-write",
which leaks the caller's gating assumption; make the mapping explicit by adding
an explicit branch for "config-toml" (e.g., if (mode === "config-toml") return
"workspace-write";) or return null for unsupported modes and update callers
(e.g., the code that currently checks mode !== "config-toml") to handle a null
return, so the helper's behavior is documented and not implicit.
In `@apps/desktop/src/shared/types/sync.ts`:
- Around line 242-249: The current SyncProjectConnectionPayload type allows
invalid combinations (e.g., authKind: "bootstrap" with token null), so replace
it with a discriminated union to enforce invariants: define two variants keyed
by authKind—one { authKind: "bootstrap"; token: string; pairedDeviceId?: null;
... } and one { authKind: "paired"; token?: null; pairedDeviceId: string; ...
}—so callers and callers like syncPeerService (checks around draft.authKind ===
"paired" and draft.pairedDeviceId) and places constructing payloads (e.g.,
main.ts where pairedDeviceId is initially null) get type-safe guarantees
matching runtime behavior tested in syncProtocol.test.ts and
syncHostService.test.ts. Ensure other code that expects optional token
(MobileProjectConnectionPayload) remains compatible by adjusting usage sites to
the new union.
In `@apps/ios/ADE/Services/KeychainService.swift`:
- Around line 9-11: tokenAccount(forHostKey:) currently interpolates hostKey
directly which can produce ambiguous or duplicate keychain accounts; update the
function and related host-scoped keychain helpers to assert/guard that hostKey
is non-empty (or document the contract) and normalize the hostKey (e.g., trim
whitespace and lowercase) at a single chokepoint (either inside
tokenAccount(forHostKey:) or where SyncService derives the host key) so hosts
like "Mac-Studio.local" and "mac-studio.local" map to the same account; apply
the same non-empty check and normalization to the other host-scoped token
functions referenced in the same region (lines 57-75) and update callers or add
unit tests to reflect the normalized contract.
In `@apps/ios/ADE/Views/Work/WorkTimelineHelpers.swift`:
- Around line 513-519: The switch in workToolGroupSoftBreak(_:) contains an
unreachable "activity" case because only eventCard(for:) produces
WorkEventCardModel payloads for soft-breaks; remove the "activity" case from the
switch and any associated comments so the switch only handles actual payload
variants (e.g., .eventCard) and does not claim to handle activity envelopes,
referencing workToolGroupSoftBreak(_:) and eventCard(for:) to locate the logic;
ensure no behavior changes by running existing timeline tests after 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
Comment on lines
+935
to
+951
| const reconcileSyncHostContexts = async (): Promise<void> => { | ||
| const hostRoot = getMobileSyncHostRoot(); | ||
| for (const [root, ctx] of projectContexts) { | ||
| const isActive = activeProjectRoot != null && root === activeProjectRoot; | ||
| ctx.syncService?.setHostStartupEnabled?.(isActive); | ||
| ctx.syncService?.setHostDiscoveryEnabled?.(isActive); | ||
| const isSyncHost = hostRoot != null && root === hostRoot; | ||
| ctx.syncService?.setHostDiscoveryEnabled?.(isSyncHost); | ||
| } | ||
| for (const [root, ctx] of projectContexts) { | ||
| const isSyncHost = hostRoot != null && root === hostRoot; | ||
| await ctx.syncService?.setHostStartupEnabled?.(isSyncHost); | ||
| } | ||
| }; | ||
|
|
||
| const setActiveProject = (projectRoot: string | null): void => { | ||
| activeProjectRoot = projectRoot ? normalizeProjectRoot(projectRoot) : null; | ||
| void reconcileSyncHostContexts().then(() => { | ||
| notifyMobileSyncProjectCatalogChanged(); | ||
| }); |
There was a problem hiding this comment.
Serialize host reconciliation; older async runs can win and flip the wrong host back on.
reconcileSyncHostContexts() now performs awaited host-startup mutations, but setActiveProject() fires it without waiting and other paths call it concurrently. A stale invocation can therefore finish after a newer one and reapply discovery/startup flags for the old host root. This is exactly the kind of race the new app-level host selection is trying to avoid.
⚙️ One way to serialize it
+ let reconcileSyncHostContextsPromise: Promise<void> = Promise.resolve();
+
const reconcileSyncHostContexts = async (): Promise<void> => {
- const hostRoot = getMobileSyncHostRoot();
- for (const [root, ctx] of projectContexts) {
- const isSyncHost = hostRoot != null && root === hostRoot;
- ctx.syncService?.setHostDiscoveryEnabled?.(isSyncHost);
- }
- for (const [root, ctx] of projectContexts) {
- const isSyncHost = hostRoot != null && root === hostRoot;
- await ctx.syncService?.setHostStartupEnabled?.(isSyncHost);
- }
+ reconcileSyncHostContextsPromise = reconcileSyncHostContextsPromise.then(async () => {
+ const hostRoot = getMobileSyncHostRoot();
+ for (const [root, ctx] of projectContexts) {
+ const isSyncHost = hostRoot != null && root === hostRoot;
+ ctx.syncService?.setHostDiscoveryEnabled?.(isSyncHost);
+ }
+ for (const [root, ctx] of projectContexts) {
+ const isSyncHost = hostRoot != null && root === hostRoot;
+ await ctx.syncService?.setHostStartupEnabled?.(isSyncHost);
+ }
+ });
+ return reconcileSyncHostContextsPromise;
};
const setActiveProject = (projectRoot: string | null): void => {
activeProjectRoot = projectRoot ? normalizeProjectRoot(projectRoot) : null;
- void reconcileSyncHostContexts().then(() => {
- notifyMobileSyncProjectCatalogChanged();
- });
+ void reconcileSyncHostContexts()
+ .then(() => {
+ notifyMobileSyncProjectCatalogChanged();
+ })
+ .catch((error) => {
+ dormantContext.logger.warn("sync.mobile_host_reconcile_failed", {
+ error: error instanceof Error ? error.message : String(error),
+ });
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/main/main.ts` around lines 935 - 951,
reconcileSyncHostContexts can run concurrently causing stale invocations to flip
the wrong host; serialize all calls by introducing a module-scoped serial
Promise and enqueue reconciles through it instead of firing
reconcileSyncHostContexts() directly. Concretely, add a top-level Promise<void>
(e.g., reconcileSyncSerial = Promise.resolve()) and change setActiveProject to
do reconcileSyncSerial = reconcileSyncSerial.then(() =>
reconcileSyncHostContexts()).then(() =>
notifyMobileSyncProjectCatalogChanged()); ensure any other callers of
reconcileSyncHostContexts also enqueue through reconcileSyncSerial so
projectContexts/getMobileSyncHostRoot/activeProjectRoot changes are applied in
order.
| .listRowSpacing(10) | ||
| .scrollContentBackground(.hidden) | ||
| .background { PrsLiquidBackdrop() } | ||
| .adeScreenBackground() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether PrsLiquidBackdrop / PrsBackdropPalette are still referenced anywhere.
rg -nP --type=swift -C2 '\bPrsLiquidBackdrop\b'
rg -nP --type=swift -C2 '\bPrsBackdropPalette\b'Repository: arul28/ADE
Length of output: 3387
🏁 Script executed:
# Check for any string references, type aliases, or indirect usages of PrsLiquidBackdrop
rg -nP --type=swift 'PrsLiquidBackdrop|PrsBackdropPalette' --iglob '!**/PrListRowModifier.swift'Repository: arul28/ADE
Length of output: 36
Remove dead code PrsLiquidBackdrop and PrsBackdropPalette.
The switch from PrsLiquidBackdrop() to .adeScreenBackground() removes the only usage of this component. PrsLiquidBackdrop and its supporting PrsBackdropPalette in PrListRowModifier.swift are now unreferenced and should be deleted. If this background simplification was intentional (e.g., aligning all tabs to a consistent flat theme), the cleanup is straightforward; otherwise, reconsider whether the animated gradient backdrop should be preserved.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ios/ADE/Views/PRs/PrsRootScreen.swift` at line 358, Remove the
now-unused animated background by deleting the PrsLiquidBackdrop view and its
supporting PrsBackdropPalette type from the codebase (they are referenced
together in PrListRowModifier.swift); update any imports or modifiers that
referenced PrsLiquidBackdrop and run a project-wide search to ensure no
remaining references to PrsLiquidBackdrop or PrsBackdropPalette exist, then run
a build to confirm nothing else depended on them.
Comment on lines
+108
to
+117
| .task(id: artifactObservationKey) { | ||
| // Proof rows arrive through CRDT-backed local DB updates, not chat | ||
| // event streams, so observe the synced DB revision directly. | ||
| try? await Task.sleep(nanoseconds: 320_000_000) | ||
| guard !Task.isCancelled else { return } | ||
| // Local sync can tick rapidly while a turn is streaming. Coalesce | ||
| // refreshes here so we do not refetch artifact lists for every | ||
| // unrelated revision burst while the user is reading the chat. | ||
| await refreshArtifacts(force: false) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== localStateRevision writes =="
rg -n -C2 'localStateRevision\s*\+\=' apps/ios/ADE/Services/SyncService.swift
echo
echo "== artifact/proof/computer-use sync paths =="
rg -n -C3 'artifact|proof|computerUse' apps/ios/ADE/Services/SyncService.swiftRepository: arul28/ADE
Length of output: 219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== localStateRevision assignments =="
rg -n 'localStateRevision\s*=' apps/ios/ADE/Services/SyncService.swift | head -20
echo
echo "== artifact/proof/computerUse mentions =="
rg -n -i 'artifact\|proof\|computeruse' apps/ios/ADE/Services/SyncService.swift | head -20
echo
echo "== File size and structure =="
wc -l apps/ios/ADE/Services/SyncService.swiftRepository: arul28/ADE
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Where is localStateRevision incremented? =="
rg -n 'localStateRevision' apps/ios/ADE/Services/SyncService.swift
echo
echo "== Where are artifacts synced? =="
fd -t f '\.swift$' apps/ios/ADE | xargs rg -l 'artifact.*sync\|sync.*artifact' -i | head -10
echo
echo "== Check WorkSessionDestinationView for task dependency =="
rg -n 'task\|artifactObservationKey\|refreshArtifacts' apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift | head -20Repository: arul28/ADE
Length of output: 785
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Context around localStateRevision += 1 increments =="
for line in 809 1007 1083 1100 1320 2555 2566 5176; do
echo "--- Line $line context ---"
sed -n "$((line-5)),$((line+2))p" apps/ios/ADE/Services/SyncService.swift
doneRepository: arul28/ADE
Length of output: 3171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== artifactObservationKey definition and task binding =="
rg -n -B5 -A10 'artifactObservationKey' apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift
echo
echo "== refreshArtifacts function =="
rg -n -B2 -A8 'func refreshArtifacts' apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift
echo
echo "== Where artifacts are actually synced =="
fd -t f '\.swift$' apps/ios/ADE | xargs rg -l 'refreshArtifacts\|artifact.*apply\|proof.*apply' | head -10Repository: arul28/ADE
Length of output: 1953
The artifact observation task depends on localStateRevision, which is never incremented on artifact/proof/computer-use syncs.
The task binding is correctly configured (line 204-206), but artifact-table updates through the CRDT-backed local DB do not trigger localStateRevision increments in SyncService. That counter only changes for project selection, connection state, authentication, and chat subscriptions. Artifact rows will remain stale until an unrelated local-state change occurs. Ensure artifact sync writes also increment localStateRevision so this observation task responds to the data it needs to monitor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/ios/ADE/Views/Work/WorkSessionDestinationView.swift` around lines 108 -
117, The observation Task bound to artifactObservationKey relies on
localStateRevision but SyncService never increments localStateRevision on
artifact/proof/computer-use syncs, so artifact updates don't trigger the .task
refreshArtifacts call; modify SyncService to increment (or bump)
localStateRevision whenever artifact-table or proof/computer-use writes are
applied (the same place where project selection/connection/auth/chat
subscription changes currently bump it) so the task observing localStateRevision
will re-run and call refreshArtifacts(force: false) when those CRDT-backed
artifact updates arrive.
- syncHostService: drop silent CONNECTING fall-through in sendAndWait - automationService: keep cursor provider key undefined-free in returned config - automationPlannerService: sanitize per-action permissionConfig pass-through - adeRpcServer: reject half-specified explicit owners; preserve PR/issue relations - cli: include new owner/caption flags in VALUE_CARRIER_FLAGS - main: serialize reconcileSyncHostContexts so async runs land in order - automationService: fall back to default lane name when template stays unresolved - agentChatService: refresh reconstruction context on sdkSessionId clear - agentChatService.test: assert turnStatus on resumed start event - syncService.test: tighten setHostStartupEnabled await assertion - permissionControls: per-key clear instead of wiping permissionConfig - iOS SyncService: failure path removes stale saved profile; prune legacy host keys; drop suspendAutoReconnect on host-identity mismatch - iOS WorkSessionGrouping: count awaiting-input sessions in globalLiveSessionCount Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
April 26, 2026 08:19
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
create-laneaction and per-actiontargetLaneId/modelConfig/permissionConfigoverrides that merge over the rule-level config in both planner and executor.forceHostRoleflag prevents the desktop instance hosting phone sync from being demoted to viewer by stale on-disk state.setHostStartupEnabledis now async;setHostDiscoveryEnabledno-ops on no-op.proof attachandproof capturesubcommands, explicitownerKind/ownerIdrouting on proof RPC tools (withchat/praliases), and headless-by-default execution for capture-style commands.Test plan
apps/desktoptypecheck + lintapps/ade-clitypecheck + tests (192/192)apps/webtypecheck + buildapps/desktopsharded vitest (8/8) — all pass except one pre-existing failure onagentChatService.test.ts:1131(omits the project slash commands section) that reproduces on cleanmain(commitf8becb18)node scripts/validate-docs.mjs🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Greptile Summary
This PR delivers per-action
modelConfig/permissionConfig/targetLaneIdoverrides in the automation engine, acreate-laneaction type, and aforceHostRoleflag in sync that prevents stale disk state from demoting the phone-sync host to viewer. ThesendAndWaitfix insyncHostService(awaiting WebSocket flush before callingcompleteProjectConnection) correctly addresses the race condition flagged in the previous review.writablePathsarray inbuildPermissionConfig's providers merge is replaced rather than merged when an action-level override exists, so rule-level path grants are silently dropped (see inline comment).launch-missionstill reads onlyrule.execution?.targetLaneId—action.targetLaneIdis ignored — while all other updated action types now usegetConfiguredTargetLaneId(rule, action)(previously flagged, still unresolved).Confidence Score: 4/5
Safe to merge with awareness of the two unresolved P2s; no data-loss or security regressions introduced.
All P1-level race conditions from the previous review are resolved (sendAndWait, forceHostRole). Remaining findings are P2: writablePaths merge semantics, trigger mutation in create-lane, and mobileSyncSelectedRoot set without reconciling. The pre-existing launch-mission targetLaneId gap is also still open. None of these block shipping.
apps/desktop/src/main/services/automations/automationService.ts (writablePaths merge + launch-mission targetLaneId), apps/desktop/src/main/main.ts (mobileSyncSelectedRoot reconciliation gap)
Important Files Changed
Sequence Diagram
sequenceDiagram participant iOS participant SyncHostService participant prepareMobileSyncProjectConnection participant completeMobileSyncProjectConnection participant reconcileSyncHostContexts iOS->>SyncHostService: project_switch_request SyncHostService->>prepareMobileSyncProjectConnection: prepareProjectConnection(payload) prepareMobileSyncProjectConnection->>prepareMobileSyncProjectConnection: ensureProjectContextForMobileSync prepareMobileSyncProjectConnection->>prepareMobileSyncProjectConnection: setHostStartupEnabled(true) + initialize() prepareMobileSyncProjectConnection-->>SyncHostService: SyncProjectSwitchResultPayload + connection info SyncHostService->>iOS: sendAndWait("project_switch_result") [waits for flush] SyncHostService->>completeMobileSyncProjectConnection: completeProjectConnection(payload, result) completeMobileSyncProjectConnection->>completeMobileSyncProjectConnection: mobileSyncSelectedRoot = targetRoot completeMobileSyncProjectConnection->>reconcileSyncHostContexts: reconcile (disable old host, keep new) completeMobileSyncProjectConnection->>completeMobileSyncProjectConnection: scheduleProjectContextRebalance + broadcastCatalog iOS->>SyncHostService: reconnects to new host (paired auth)Comments Outside Diff (1)
apps/desktop/src/main/services/automations/automationService.ts, line 1849-1863 (link)launch-missionignores per-actiontargetLaneIdoverrideThe
launch-missionexecutor constructsmissionRule.executionusing onlyrule.execution?.targetLaneId, silently discarding any per-actiontargetLaneIdset on the action itself. Every other action type that was updated in this PR (run-tests,agent-session,run-command) usesgetConfiguredTargetLaneId(rule, action)orresolveExecutionLaneId(rule, trigger, action)to prefer the action-level value first. Thelaunch-missioncase was missed.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "ship: iter 1 — address Greptile/CodeRabb..." | Re-trigger Greptile