Mobile sync stability + webhook relay productization + edit-protection removal#683
Merged
Merged
Conversation
…h-installe-dsimaultor-0d00d08b
iOS crash/perf hardening (verified: iOS build green, desktop sync 24/24, ade-cli mpRpc 10/10): - Fix Int64(Double) bind trap (safeInt64Value) — poison-batch launch crasher - Fix gunzip pointer-escape UB; enforce 32MiB chunk-reassembly byte cap - Fix reconnect-wedge (generation-paired reconnectConnectInFlight flag) - Evict per-session chat-event history (64-session LRU) - Clamp chat_subscribe snapshot to client maxBytes + dedupe snapshot/pump overlap - Files diff: O(n·m) LCS -> CollectionDifference, computed once into @State - Attachment thumbnails downsample via CGImageSourceCreateThumbnailAtIndex - Handoff classification via isChatToolType (cursor/droid + orphan chat rows) - fetchSessionLocked keeps lane-less sessions fetchable by id Quick wins: memoize runtime build hash; cache compiled regex in workRegexMatches; shared date parser in CTO workflows; raise highlight cache to 160; LaneDetail defer-not-drop throttle; in-place recordChatEventEnvelope (no per-event array copy); cap prDetailCache; CtoTeamScreen visibility gate. Webhook relay productization: hosted Cloudflare Worker default, repo-scoped routes authorized by the user's GitHub token (no relay token / local.secret required); legacy project-token routes preserved for self-hosted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Files are freely editable with no "Enable editing" step. Removes the FilesWorkbench read-only gate + editOverrides machinery; derives workspace isReadOnlyByDefault as constant false (laneService, iOS Database, browserMock). Keeps the is_edit_protected column and its auto-rebase exclusion filters (`and is_edit_protected = 0`) untouched — this change is scoped to file editing only. FilesWorkbench test flipped to assert edit-protected workspaces are editable immediately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- relay.test.ts: cover token-present-but-denied on both repo-scoped routes (events + status) — asserts 403/404 and no webhook-event leakage to a denied caller. Closes the /quality-flagged coverage gap. - docs: reflect edit-protection removal (files freely editable), webhook relay productization (hosted Worker default, repo-scoped token auth), the mobile chat_subscribe maxBytes clamp + 64-session history eviction, and the new all-projects Hub roster sub-protocol. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
arul28
changed the title
📝 WalkthroughWalkthroughThis PR adds mobile hub roster sync, new chat history/subagent command surfaces, hosted GitHub relay routing, files-workspace writable-by-default changes, and broad iOS Work chat, PR navigation, and core-service updates across desktop and iOS. ChangesAll-Projects Roster / Mobile Hub Sync Protocol
Estimated code review effort: 5 (Critical) | ~180 minutes Chat History, Remote Commands, and GitHub Relay Auth
Estimated code review effort: 4 (Complex) | ~90 minutes Files Workspace Read-Only Removal
Estimated code review effort: 3 (Moderate) | ~30 minutes iOS Work Chat Timeline and Composer Overhaul
Estimated code review effort: 5 (Critical) | ~240 minutes iOS PR Navigation and Work Chat PR Integration
Estimated code review effort: 3 (Moderate) | ~40 minutes iOS Core Services and Misc UI Polish
Estimated code review effort: 3 (Moderate) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Owner
Author
|
@copilot review but do not make fixes |
Copilot stopped work on behalf of
arul28 due to an error
July 1, 2026 23:46
![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
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/ade-cli/src/multiProjectRpcServer.ts (1)
460-493: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign runtime build-hash fallback across RPC servers.
apps/ade-cli/src/adeRpcServer.tsstill returnsbuildHash: nullwhenADE_RUNTIME_BUILD_HASHis unset, whileapps/ade-cli/src/multiProjectRpcServer.tshashesprocess.argv[1]instead. That makesmachineRuntimeMismatchReasondepend on which server handlesade/initializewhen the env is missing.🤖 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/ade-cli/src/multiProjectRpcServer.ts` around lines 460 - 493, Align the runtime build-hash fallback in the RPC server initialization path so both servers behave the same when ADE_RUNTIME_BUILD_HASH is missing. Update adeRpcServer’s runtime env resolution to match multiProjectRpcServer’s computeRuntimeBuildHash logic, using process.argv[1] with file checks and hashing as the fallback instead of returning null. Keep the behavior consistent through the relevant resolveRuntimeEnvInfo/initialize flow so machineRuntimeMismatchReason does not vary by server.apps/ios/ADE/Views/Settings/SettingsConnectionHeader.swift (1)
47-62: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winProvide a fallback when a saved host has no display name.
stateDetailLinecan returnnilin the disconnected state whencanReconnectToSavedHostis true but no host name is available, leaving the card blank; show a generic reconnect caption in that case.apps/ios/ADE/Views/Settings/SettingsConnectionHeader.swift:47-62, 136-152🤖 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/ios/ADE/Views/Settings/SettingsConnectionHeader.swift` around lines 47 - 62, The disconnected-state copy in SettingsConnectionHeader is missing a fallback when a saved host exists but has no display name, so the card can render blank. Update stateDetailLine and/or the disconnected branch in SettingsConnectionHeader to return a generic reconnect caption whenever canReconnectToSavedHost is true but no host name is available, using the existing pendingDescription/stateDetailLine flow as the place to preserve the connected, pending, and onboarding cases.apps/desktop/src/main/services/github/githubRelayConfig.ts (1)
169-175: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win"unconfigured" branch is now unreachable dead code.
Since
apiBaseUrlis always populated viaconfiguredApiBaseUrl || DEFAULT_GITHUB_RELAY_API_BASE_URL,config.apiBaseUrlcan never be falsy, soif (!config.apiBaseUrl)here never triggers. This meansfetchGitHubAppInstallationStatuscan no longer report the"unconfigured"state even when a project genuinely has zero relay setup — it silently falls through to the hosted-default path instead. If that's intentional (per the hosted-default design), consider removing this dead branch or replacing the check with something that reflects the real "fully unconfigured" condition (e.g.!config.remoteProjectId && !config.accessToken && config.usesHostedDefault === undefined-style explicit flag), so the code doesn't imply a reachable state that no longer exists.🤖 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/github/githubRelayConfig.ts` around lines 169 - 175, The `fetchGitHubAppInstallationStatus` check for `!config.apiBaseUrl` is dead code because `githubRelayConfig` always assigns a default API base URL, so the `"unconfigured"` status can never be returned. Remove this unreachable branch or replace it with an explicit fully-unconfigured condition based on the real config flags used by `githubRelayConfig` and `fetchGitHubAppInstallationStatus`, so the status logic matches the hosted-default behavior and the code does not advertise an unreachable state.
🟡 Minor comments (12)
apps/ios/ADE/Views/PRs/PrsRootScreen.swift-1345-1357 (1)
1345-1357: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winProgrammatic create-PR navigation skips the
canCreatePrcapability gate.The manual "+" button disables/greys out when
!canCreatePr(Lines 664-665), but therequest.createLaneId-driven path here presentscreatePrWizardSheetunconditionally, with nocanCreatePrcheck. If the host is offline or lacks create capabilities, this could surface a create wizard that can't actually create anything.🐛 Proposed fix to gate the programmatic create path
if let createLaneId = request.createLaneId?.trimmingCharacters(in: .whitespacesAndNewlines), !createLaneId.isEmpty { await reload(refreshRemote: false) + guard canCreatePr else { + errorMessage = "Pull request creation isn't available on this phone right now." + syncService.requestedPrNavigation = nil + return + } rootSurfaceRawValue = PrRootSurface.github.rawValue🤖 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/ios/ADE/Views/PRs/PrsRootScreen.swift` around lines 1345 - 1357, The programmatic create-PR navigation path in PrsRootScreen’s request.createLaneId handling bypasses the same canCreatePr gate used by the manual “+” button. Before setting createPresented and opening the createPrWizardSheet, check canCreatePr in this block and only proceed when creation is allowed; otherwise ignore the request or route back without presenting the wizard.apps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swift-697-701 (1)
697-701: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKeep PR details refresh single-flight even when forced.
Line 698 lets every forced refresh start while another refresh is in flight, so overlapping tasks can interleave and stale snapshot/error state can win after
awaits. Prefer guarding all in-flight refreshes, and keepforceonly for the refresh behavior.Proposed fix
- guard force || !prDetailsRefreshing else { return } + guard !prDetailsRefreshing else { return }🤖 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/ios/ADE/Views/Work/WorkSessionDestinationView`+Actions.swift around lines 697 - 701, The refreshChatPrDetails flow currently allows forced calls to bypass the in-flight guard, which can start overlapping refresh tasks and let stale snapshot/error state overwrite newer results after awaits. Update refreshChatPrDetails so the single-flight protection always blocks when prDetailsRefreshing is already true, and use force only to control whether the refresh should bypass cached data or trigger a reload. Keep the fix localized to refreshChatPrDetails and the prDetailsRefreshing/prDetailsError state handling.apps/ios/ADE/Views/Work/WorkChatRichCardViews.swift-1027-1060 (1)
1027-1060: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse status-specific icons and include metadata in the accessibility label.
Line 1034 shows a pause icon for every non-failed status, so completed/started statuses can look interrupted. Line 1060 also omits the metadata status when
card.bodyis empty, even though that is the visible ribbon text.Proposed fix
private var statusRibbonBody: some View { let normalized = card.metadata.first?.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() ?? "" let isFailure = normalized == "failed" let isInterrupted = normalized == "interrupted" + let isCompleted = ["completed", "complete", "succeeded", "success"].contains(normalized) + let icon = isFailure + ? "xmark.circle.fill" + : (isInterrupted ? "pause.circle.fill" : (isCompleted ? "checkmark.circle.fill" : "circle.fill")) let tint = isFailure ? ADEColor.danger : (isInterrupted ? ADEColor.warning : ADEColor.textMuted) + let statusText = normalized.isEmpty ? ribbonText.uppercased() : normalized.uppercased() return HStack(alignment: .center, spacing: 8) { - Image(systemName: isFailure ? "xmark.circle.fill" : "pause.circle.fill") + Image(systemName: icon) .font(.system(size: 11, weight: .bold)) - Text(normalized.isEmpty ? ribbonText.uppercased() : normalized.uppercased()) + Text(statusText) .font(.caption2.monospaced().weight(.semibold)) .tracking(0.8) @@ .frame(maxWidth: .infinity, alignment: .leading) .accessibilityElement(children: .combine) - .accessibilityLabel([card.title, card.body].compactMap { $0 }.joined(separator: ". ")) + .accessibilityLabel([card.title, statusText, card.body].compactMap { $0 }.joined(separator: ". ")) }🤖 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/ios/ADE/Views/Work/WorkChatRichCardViews.swift` around lines 1027 - 1060, Update statusRibbonBody in WorkChatRichCardViews so the leading SF Symbol matches the normalized metadata status instead of defaulting every non-failed case to pause.circle.fill; map completed/started/other statuses to appropriate icons while keeping failed/interrupted distinct. Also fix the accessibilityLabel on the ribbon to include the visible status text (normalized or ribbonText) alongside card.title and card.body, so when card.body is empty the status is still announced.apps/ios/ADE/Views/Work/WorkChatSessionView.swift-139-160 (1)
139-160: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winResolve the
@Statevisibility warnings.
These properties are used fromWorkChatSessionView+Actions.swiftandWorkChatSessionView+Timeline.swift, so they can’t be madeprivatewithout moving that logic into the same file. Add a narrowswiftlint:disable private_swiftui_statewith a brief rationale, or colocate the related code.🤖 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/ios/ADE/Views/Work/WorkChatSessionView.swift` around lines 139 - 160, The `WorkChatSessionView` state properties are triggering visibility warnings because they are shared with `WorkChatSessionView+Actions.swift` and `WorkChatSessionView+Timeline.swift`. Keep the cross-file access intact and either add a narrow `swiftlint:disable private_swiftui_state` with a brief rationale near the affected `@State` declarations, or move the dependent logic into the same `WorkChatSessionView` file. Focus on the state block containing `composerLayoutHeight`, `timelineSnapshot`, `timelineRebuildTask`, and the other shared `@State` members.Sources: Path instructions, Linters/SAST tools
apps/ade-cli/src/services/sync/syncHostService.ts-2807-2810 (1)
2807-2810: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not drop roster dirties that arrive during an in-flight flush.
If a debounce/max-wait timer fires while
rosterFlushInFlightis true,flushRoster()clears the timers and returns, so changes that happened after the snapshot build started can be lost until the safety poll. Track a pending flag and schedule another flush infinally.🤖 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/ade-cli/src/services/sync/syncHostService.ts` around lines 2807 - 2810, The `flushRoster()` flow in `syncHostService.ts` is dropping roster dirties that arrive while `rosterFlushInFlight` is true because it clears timers and exits without retrying. Add a pending-flush flag in the roster flush state, have `flushRoster()` set it when called during an in-flight flush, and in the existing `finally` block schedule another flush if that flag is set. Use the `flushRoster`, `clearRosterFlushTimers`, and `rosterFlushInFlight` logic to locate the fix and ensure no dirty updates are lost between snapshot build and completion.apps/ade-cli/src/services/sync/syncHostService.ts-197-205 (1)
197-205: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude all roster-visible mutations in the dirtying set.
This set misses commands that remove or restore visible rows/metadata, e.g.
chat.archive,chat.unarchive,chat.delete,lanes.unarchive,lanes.rename, andlanes.updateAppearance. Those hub actions can otherwise remain stale until the 15s safety poll.Suggested patch
const ROSTER_DIRTYING_COMMAND_ACTIONS = new Set<string>([ "chat.create", + "chat.archive", + "chat.unarchive", + "chat.delete", + "chat.updateSession", "work.startCliSession", "work.resumeCliSession", + "work.updateSessionMeta", "lanes.create", "lanes.createChild", + "lanes.rename", + "lanes.updateAppearance", "lanes.archive", + "lanes.unarchive", "lanes.delete", ]);🤖 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/ade-cli/src/services/sync/syncHostService.ts` around lines 197 - 205, The ROSTER_DIRTYING_COMMAND_ACTIONS set in syncHostService should include every roster-visible mutation, not just creation/deletion actions. Update the set to add the missing hub actions such as chat.archive, chat.unarchive, chat.delete, lanes.unarchive, lanes.rename, and lanes.updateAppearance so syncHostService marks the roster dirty immediately after those mutations.apps/ade-cli/src/services/sync/rosterBuilder.ts-64-70 (1)
64-70: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
hostProjectIdis documented but never used.
BuildRosterSnapshotArgs.hostProjectIdclaims the host project is "always booted — included with live fidelity," but this field is never read inbuildRosterSnapshot/buildRosterProject. Both callers (bootstrap.ts,cli.ts) pass it expecting that guarantee. Today it happens to work out becauseswitchSyncHostseparately boots that scope, but the explicit "always" contract isn't actually enforced by this function — e.g. a roster build racing the host project's boot won't get live fidelity despite the promise.Either wire it through (e.g. await/force-boot the host scope before falling back to disk-only) or drop the unused field and update the doc comment/callers accordingly.
Also applies to: 384-413
🤖 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/ade-cli/src/services/sync/rosterBuilder.ts` around lines 64 - 70, `BuildRosterSnapshotArgs.hostProjectId` is documented as guaranteeing the host project is always booted, but `buildRosterSnapshot`/`buildRosterProject` never read it, so the contract is not enforced. Update the roster-building flow to either use `hostProjectId` to ensure the host scope is booted and included with live fidelity before snapshotting, or remove the unused field and adjust the doc comment plus the `bootstrap.ts` and `cli.ts` callers to match the actual behavior. Use the `buildRosterSnapshot` and `buildRosterProject` symbols to locate the implementation.apps/ios/ADE/Views/Hub/HubScreen.swift-316-323 (1)
316-323: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRebuild presentations after restoring saved order.
projectOrderis restored here, buthubProjectPresentationsmay already have been built using the default alphabetical order. Rebuild immediately so the persisted layout is actually applied on reconnect.Proposed fix
collapsedLaneKeys = Set(saved.collapsedLanes) collapsedDefaultsSeededProjectIds = Set(saved.seededProjects) collapsedDefaultsSeededLaneKeys = Set(saved.seededLanes) - seedCollapsedHubDefaults(for: hubProjectPresentations) + rebuildHubProjectPresentations() }🤖 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/ios/ADE/Views/Hub/HubScreen.swift` around lines 316 - 323, After restoring layout in HubScreen.restoreSavedLayout, rebuild the presentation list immediately because hubProjectPresentations may still reflect the default alphabetical order. Update the flow so the restored projectOrder is applied before any derived presentations are used, then regenerate or refresh the presentations with the restored order. Use the existing restoreSavedLayout logic and the hubProjectPresentations source so the persisted layout takes effect on reconnect.apps/ios/ADE/Views/Hub/HubScreen.swift-27-27 (1)
27-27: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep
openChatTargetprivate.
@Stateis view-local mutable state; makeopenChatTargetprivate unless another type needs direct access.🤖 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/ios/ADE/Views/Hub/HubScreen.swift` at line 27, Make the HubScreen view’s openChatTarget state private, since it is only used as local mutable UI state. Update the property declaration for openChatTarget in HubScreen.swift to use private with `@State`, and keep any access through HubScreen’s own methods or bindings unless another type truly needs it.Source: Linters/SAST tools
apps/desktop/src/main/services/chat/agentChatService.ts-7242-7242 (1)
7242-7242: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winThread
maxBytesthroughchat.getChatEventHistory
agentChatService.getChatEventHistoryacceptsmaxBytes, but the renderer/IPC boundary still only forwardsmaxEvents, so the new limit can’t be set fromapps/desktop/src/preload/preload.tsorapps/desktop/src/main/services/ipc/registerIpc.ts. AddmaxBytesto the shared args or keep the option internal-only.🤖 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/chat/agentChatService.ts` at line 7242, `agentChatService.getChatEventHistory` now accepts `maxBytes`, but the renderer-to-main path still drops it, so the limit cannot be configured through the IPC flow. Update the shared chat history args and the IPC wiring in `preload` and `registerIpc` so `maxBytes` is forwarded alongside `maxEvents` into `getChatEventHistory`, or remove it from the public signature if it should remain internal-only. Make sure the change is applied consistently wherever the chat history options are typed or destructured.Source: Coding guidelines
apps/desktop/src/main/services/chat/agentChatService.ts-7269-7274 (1)
7269-7274: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
maxBytesshould be measured in UTF-8 bytes, notJSON.stringify(...).length. The current mobile cap is well below the 16 MiB RPC ceiling, so this won’t usually trip transport limits, but it can still exceed the caller’s requested byte budget on multi-byte transcripts. Switching the envelope estimator toBuffer.byteLength(JSON.stringify(envelope), "utf8")would make the cap match the API contract.🤖 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/chat/agentChatService.ts` around lines 7269 - 7274, The maxBytes handling in agentChatService should use UTF-8 byte اندازه rather than string length when estimating the response envelope. Update the logic around the requestedMaxBytes/responseMaxChars calculation and the envelope sizing path to measure JSON output with Buffer.byteLength(..., "utf8") instead of JSON.stringify(...).length, so the cap in chat history responses matches the API contract for multi-byte content.apps/ios/ADE/Models/RemoteModels.swift-734-780 (1)
734-780: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd the missing fields to
AgentChatSessionSummaryequality
cursorModeSnapshot,cursorConfigValues,computerUse, andcompletionare part of the model state but are ignored here, so summaries that differ only in those values still compare equal. That also flows intoCtoRosterequality viacto ==. Add them to==unless they’re intentionally excluded.🐛 Proposed fix
&& lhs.orchestrationStepId == rhs.orchestrationStepId && lhs.orchestrationBundlePath == rhs.orchestrationBundlePath + && lhs.cursorModeSnapshot == rhs.cursorModeSnapshot + && lhs.cursorConfigValues == rhs.cursorConfigValues + && lhs.computerUse == rhs.computerUse + && lhs.completion == rhs.completion }🤖 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/ios/ADE/Models/RemoteModels.swift` around lines 734 - 780, `AgentChatSessionSummary` equality is missing several state fields, so objects can compare equal even when they differ. Update the `==` implementation in `AgentChatSessionSummary` to include `cursorModeSnapshot`, `cursorConfigValues`, `computerUse`, and `completion` alongside the existing property checks, unless they are intentionally meant to be excluded from identity. This will also keep `CtoRoster` equality via `cto ==` consistent with the full model state.
🧹 Nitpick comments (6)
apps/ade-cli/src/multiProjectRpcServer.test.ts (1)
61-89: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMove
handler.dispose()into thefinallyblock.If the
expect(init).toMatchObject(...)assertion fails,handler.dispose()(line 80) is skipped, leaking the handler/scope registry resources for the rest of the test run.🧹 Proposed fix
expect(init).toMatchObject({ runtimeInfo: { buildHash: expectedHash, multiProject: true, }, }); - handler.dispose(); } finally { + handler?.dispose(); process.argv = originalArgv;🤖 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/ade-cli/src/multiProjectRpcServer.test.ts` around lines 61 - 89, Move the cleanup for createMultiProjectRpcRequestHandler into the existing try/finally flow so it always runs, even when the expect(init).toMatchObject assertion fails. Keep the dispose call next to the handler variable in the multiProjectRpcServer test, and ensure handler.dispose() is executed from the finally block after the registry/argv/env restoration logic.apps/ios/ADE/Views/Work/WorkChatPrViews.swift (1)
172-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the shared PR-state tint helper
workChatPrStateTintduplicates the same open/draft/closed/merged mapping already used bylanePullRequestTint, so this should call the shared helper to keep the badge and details sheet in sync if the palette changes.♻️ Proposed consolidation
- let stateTint = workChatPrStateTint(tag.state) + let stateTint = lanePullRequestTint(tag.state)🤖 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/ios/ADE/Views/Work/WorkChatPrViews.swift` around lines 172 - 173, The PR-state tint mapping in WorkChatPrViews is duplicated and can drift from the shared badge/details palette. Update the state tint logic in the WorkChatPrViews flow to use the shared lanePullRequestTint helper instead of the local workChatPrStateTint mapping, keeping the branch/badge colors consistent wherever tag.state is rendered.apps/ios/ADE/Views/Components/ADECodeRenderingCache.swift (1)
14-14: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a size bound to the highlighted-string cache.
countLimit = 160only caps entry count, not the memory held by largeAttributedStringvalues. Since the cache stores highlighted output for the full input text, addtotalCostLimitwith per-entry cost accounting, or keep the lower limit.🤖 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/ios/ADE/Views/Components/ADECodeRenderingCache.swift` at line 14, The highlighted-string cache in ADECodeRenderingCache only limits entry count via attributedCache.countLimit, so large AttributedString values can still consume too much memory. Update the cache setup to add a totalCostLimit and make the caching path account for per-entry cost when storing highlighted output, or reduce the existing limit if cost tracking is not feasible. Use the ADECodeRenderingCache cache initialization and the highlighted-string storage logic to apply the bound consistently.Source: Path instructions
apps/ios/ADE/Views/Files/FilesModels.swift (1)
284-301: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid the full string concatenation just to compute a byte count.
combinedText = "\(diff.original.text)\n\(diff.modified.text)"(Line 293) copies both diff sides into a new string purely to computecombinedText.utf8.count. This is exactly the kind of large-diff cost the newfilesLinePairCountExceedsLimitguard above is trying to avoid — for diffs that pass the line-pair check but are still large, this still forces a full duplicate allocation/copy. Summing the UTF-8 counts directly avoids the copy.⚡ Proposed fix to avoid the concatenation
- let combinedText = "\(diff.original.text)\n\(diff.modified.text)" return filesTextLimit( - byteCount: combinedText.utf8.count, + byteCount: diff.original.text.utf8.count + diff.modified.text.utf8.count + 1, lineCount: originalLineCount + modifiedLineCount,🤖 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/ios/ADE/Views/Files/FilesModels.swift` around lines 284 - 301, The byte-count path in FilesModels.swift still builds a full combined string in the diff preview flow, which defeats the new large-diff guard. In the logic that computes the limit check after filesLinePairCountExceedsLimit, replace the combinedText concatenation with a direct calculation using the UTF-8 counts of diff.original.text and diff.modified.text (including the separator if needed) so FilesPreviewLimit and filesTextLimit can be evaluated without duplicating the diff content.apps/ios/ADE/Views/Components/FilesCodeSupport.swift (1)
348-437: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional cleanup: centralize the unchanged-line path and broaden diff tests.
- Route the
.unchangedcase throughappendLinetoo so the ID/line-number formatting stays in one place.- Add dedicated coverage for adjacent swaps, full rewrites, empty inputs, and trailing-only insert/remove cases; the current test only exercises a simple mixed diff.
🤖 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/ios/ADE/Views/Components/FilesCodeSupport.swift` around lines 348 - 437, The diff-building logic in the inline diff routine should be cleaned up so the `.unchanged` path uses the same `appendLine` helper as added/removed lines, keeping ID and line-number formatting centralized in the same code path. Update the line handling in the diff generation flow around `appendLine` and `diffLines` so unchanged entries are appended consistently, then expand the tests for this behavior to cover adjacent swaps, full rewrites, empty inputs, and trailing-only insert/remove cases beyond the current mixed-diff scenario.apps/desktop/src/shared/types/sync.ts (1)
374-376: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAlign the
sinceSeqcontract with the host behavior.Line 375 says the host can send a delta from
sinceSeq, buthandleRosterSubscribeignores the payload and always sends a snapshot. Either remove this field/comment for now or implement resume-by-seq so clients do not rely on an unsupported protocol path. As per coding guidelines, “Keep IPC contracts, preload types, shared types, and renderer usage in sync whenever an interface changes”.🤖 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/types/sync.ts` around lines 374 - 376, The SyncRosterSubscribePayload contract is ahead of the actual host behavior: handleRosterSubscribe still ignores sinceSeq and always returns a snapshot, so clients may rely on unsupported resume-by-seq behavior. Either remove sinceSeq and its comment from SyncRosterSubscribePayload in sync.ts, or update the host-side handleRosterSubscribe path to honor sinceSeq and send a delta when possible, keeping the shared type and IPC behavior in sync.Source: Coding guidelines
🤖 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/services/sync/rosterBuilder.ts`:
- Around line 303-331: The overlay lookup in buildRosterProject can block the
entire snapshot when scopeRegistry.getIfBooted(record.projectId) is still
pending for a booting scope. Update the getIfBooted await path in
buildRosterProject to use a short timeout or fallback so a slow/stuck boot
returns null and the function continues with disk-only fidelity. Keep the
best-effort behavior around the scopeRegistry, agentChatService, and
listSessions overlay so buildRosterSnapshot cannot be stalled by one project.
- Around line 14-20: The roster DB setup in rosterBuilder still opens
`node:sqlite` databases without read-only mode, which can take locks during the
all-projects scan. Update the `DatabaseSync` construction path in
`rosterBuilder` to pass a `readOnly: true` option, and extend the
`DatabaseSyncConstructor` typing plus `apps/ade-cli/src/types/node-sqlite.d.ts`
so `readOnly?: boolean` is accepted.
In `@apps/desktop/src/main/services/github/githubRelayConfig.ts`:
- Line 8: The default GitHub relay base URL is hardcoded to a personal hosted
relay, which would route tokens from unconfigured installs through an unintended
third-party endpoint. Update DEFAULT_GITHUB_RELAY_API_BASE_URL in
githubRelayConfig to point to vendor-owned infrastructure, and ensure the status
check and background poll paths only use a hosted relay after explicit
user-facing consent or configuration.
In `@apps/ios/ADE/Models/RemoteModels.swift`:
- Around line 2827-2862: TerminalSessionSummary equality is missing the
resumeMetadata.launch field, so launch-mode-only updates can be treated as
unchanged. Update the TerminalSessionSummary == implementation to compare
lhs.resumeMetadata?.launch with rhs.resumeMetadata?.launch alongside the other
resumeMetadata properties, so reload() detects these changes and refreshes
sessions correctly.
In `@apps/ios/ADE/Views/Hub/HubComponents.swift`:
- Around line 366-386: The hub row grouping logic is only treating isChatTool
chats as top-level, so standalone CLI rows are dropped instead of being shown in
the roster. Update the grouping in HubComponents.swift around topLevelChats,
childRowsByParentId, and the lane-building logic so child rows are identified
first from visibleChats, and any non-child rows (including CLI rows) are added
back as top-level entries. Keep the existing sorting/presentation flow in
HubChatRowPresentation.make and the lane grouping, but ensure rows without a
valid parent are not excluded.
In `@apps/ios/ADE/Views/Hub/HubComposerDrawer.swift`:
- Around line 727-782: The follow-up calls are still using the active project
context instead of the newly selected project, so a chat created elsewhere can
miss its opener and leave an orphaned lane. Update the HubComposerDrawer flow to
thread targetProjectId through the post-create calls in this block, especially
syncService.sendChatMessage and syncService.deleteLane, so both operate on the
same project envelope used by createChatSession.
In `@apps/ios/ADE/Views/Hub/HubScreen.swift`:
- Line 35: `activeRoster` is being overlaid without verifying it belongs to the
current project, which can merge stale chats after a project switch. Update
`HubScreen` so the roster is keyed by project id (for example, store the
roster’s project identifier alongside `activeRoster`) and only apply the overlay
when it matches the current project. Make the check in the
`hubPresentationKey`/`rebuildKey` flow and in the overlay logic that uses
`activeRoster`, so the previous project’s local chats are ignored once the
active project changes.
In `@apps/ios/ADE/Views/Hub/HubScreen`+ChatNavigation.swift:
- Around line 57-66: The chat navigation task in HubScreen+ChatNavigation.swift
always flips ready to true after calling syncService.openProjectForHubChat, even
though that method does not confirm success; gate rendering on a verified active
project instead. Update the .task logic to check the result by re-reading
syncService.isActiveProject(target.project) after the project switch attempt (or
otherwise confirming the activation completed) and only set ready = true when
the target project is actually active, keeping the existing early return path
for the already-active case.
In `@apps/ios/ADE/Views/Work/WorkChatRichCardViews.swift`:
- Around line 2072-2075: The color selection logic in color uses
abs(workStableSubagentHash(id)), which can trap on Int.min; update this indexing
to use UInt(bitPattern:) instead. Apply the same approach anywhere the
workStableSubagentHash(id) result is used for palette or glyph bit calculations
so the palette lookup stays safe and deterministic.
In `@apps/ios/ADE/Views/Work/WorkChatSessionView`+Actions.swift:
- Around line 391-400: The duplicate-echo cleanup in
workIncrementalRemoveDuplicateEchoes(matching:from:) is too broad because
removeAll removes every pending echo with the same normalized text. Update the
logic so it removes only the single matched optimistic echo entry from timeline,
keeping any other echo entries with the same text untouched; use the existing
entry.id prefix check and normalizedWorkLocalEchoText/message.markdown match to
identify the one echo to drop.
- Around line 120-129: The incremental tail update in
WorkChatSessionView+Actions is setting the turn active without updating
transcriptHasInterruptibleActivity, so the interrupt control state can become
stale. In the incremental snapshot update path, alongside
transcriptIndicatesActiveTurn and transcriptLatestTurnEnded, recompute or set
transcriptHasInterruptibleActivity to reflect the newly streaming assistant
activity. Make sure the logic stays consistent with shouldShowInterruptControl
and the same snapshot-building flow that uses workIncrementalLatestTimestamp and
workIncrementalLatestAssistantId.
In `@apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift`:
- Around line 899-915: The text-fallback logic in WorkErrorAndMessageHelpers
should not resolve every queued steer that shares the same normalized text. In
the queued-steer scan and resolution path around
sortedWorkChatEnvelopes/transcript processing, prefer matching by steerId first,
and when falling back to text, consume only a single queued entry instead of
unioning the whole Set. If needed, update the fallback key to include turnId or
ordering so duplicate prompts do not clear multiple pending steers at once.
- Around line 1007-1022: In mergedWorkChatEnvelope, keep the original envelope
ordering stable when combining assistantText updates instead of always copying
incoming timestamp and sequence. Update the merge logic so the merged
WorkChatEnvelope preserves the existing envelope’s ordering fields (or only
advances them when explicitly required) while still merging text and ids,
preventing assistant fragments from being resorted ahead of surrounding
tool/status events. Use mergedWorkChatEnvelope and the assistantText case as the
main fix location.
In `@apps/ios/ADE/Views/Work/WorkRootScreen.swift`:
- Around line 221-228: The session presentation key is currently feeding pushed
screens with presentation-derived workOrderedLanes that can go stale when
sessionPresentationTaskKey becomes nil off the root. Update WorkRootScreen’s
sessionPresentationTaskKey and the consumers in WorkSessionDestinationView and
WorkNewChatScreen so they use live lanes from the root state, or derive a safe
fallback directly from lanes instead of relying on
sessionPresentation.workOrderedLanes.
In `@apps/ios/ADE/Views/Work/WorkRootScreen`+Actions.swift:
- Around line 383-391: The lane-navigation handler in WorkRootScreen+Actions
should not continue after cancellation, because try? await Task.sleep(for:) can
return early and still let the task scroll and clear
requestedWorkLaneNavigation. In the navigation flow around the request.id guard
and proxy.scrollTo inside the existing Task, explicitly stop when the task is
cancelled before consuming the request, then only perform the scroll and nil out
syncService.requestedWorkLaneNavigation if the task is still active and the
request still matches.
---
Outside diff comments:
In `@apps/ade-cli/src/multiProjectRpcServer.ts`:
- Around line 460-493: Align the runtime build-hash fallback in the RPC server
initialization path so both servers behave the same when ADE_RUNTIME_BUILD_HASH
is missing. Update adeRpcServer’s runtime env resolution to match
multiProjectRpcServer’s computeRuntimeBuildHash logic, using process.argv[1]
with file checks and hashing as the fallback instead of returning null. Keep the
behavior consistent through the relevant resolveRuntimeEnvInfo/initialize flow
so machineRuntimeMismatchReason does not vary by server.
In `@apps/desktop/src/main/services/github/githubRelayConfig.ts`:
- Around line 169-175: The `fetchGitHubAppInstallationStatus` check for
`!config.apiBaseUrl` is dead code because `githubRelayConfig` always assigns a
default API base URL, so the `"unconfigured"` status can never be returned.
Remove this unreachable branch or replace it with an explicit fully-unconfigured
condition based on the real config flags used by `githubRelayConfig` and
`fetchGitHubAppInstallationStatus`, so the status logic matches the
hosted-default behavior and the code does not advertise an unreachable state.
In `@apps/ios/ADE/Views/Settings/SettingsConnectionHeader.swift`:
- Around line 47-62: The disconnected-state copy in SettingsConnectionHeader is
missing a fallback when a saved host exists but has no display name, so the card
can render blank. Update stateDetailLine and/or the disconnected branch in
SettingsConnectionHeader to return a generic reconnect caption whenever
canReconnectToSavedHost is true but no host name is available, using the
existing pendingDescription/stateDetailLine flow as the place to preserve the
connected, pending, and onboarding cases.
---
Minor comments:
In `@apps/ade-cli/src/services/sync/rosterBuilder.ts`:
- Around line 64-70: `BuildRosterSnapshotArgs.hostProjectId` is documented as
guaranteeing the host project is always booted, but
`buildRosterSnapshot`/`buildRosterProject` never read it, so the contract is not
enforced. Update the roster-building flow to either use `hostProjectId` to
ensure the host scope is booted and included with live fidelity before
snapshotting, or remove the unused field and adjust the doc comment plus the
`bootstrap.ts` and `cli.ts` callers to match the actual behavior. Use the
`buildRosterSnapshot` and `buildRosterProject` symbols to locate the
implementation.
In `@apps/ade-cli/src/services/sync/syncHostService.ts`:
- Around line 2807-2810: The `flushRoster()` flow in `syncHostService.ts` is
dropping roster dirties that arrive while `rosterFlushInFlight` is true because
it clears timers and exits without retrying. Add a pending-flush flag in the
roster flush state, have `flushRoster()` set it when called during an in-flight
flush, and in the existing `finally` block schedule another flush if that flag
is set. Use the `flushRoster`, `clearRosterFlushTimers`, and
`rosterFlushInFlight` logic to locate the fix and ensure no dirty updates are
lost between snapshot build and completion.
- Around line 197-205: The ROSTER_DIRTYING_COMMAND_ACTIONS set in
syncHostService should include every roster-visible mutation, not just
creation/deletion actions. Update the set to add the missing hub actions such as
chat.archive, chat.unarchive, chat.delete, lanes.unarchive, lanes.rename, and
lanes.updateAppearance so syncHostService marks the roster dirty immediately
after those mutations.
In `@apps/desktop/src/main/services/chat/agentChatService.ts`:
- Line 7242: `agentChatService.getChatEventHistory` now accepts `maxBytes`, but
the renderer-to-main path still drops it, so the limit cannot be configured
through the IPC flow. Update the shared chat history args and the IPC wiring in
`preload` and `registerIpc` so `maxBytes` is forwarded alongside `maxEvents`
into `getChatEventHistory`, or remove it from the public signature if it should
remain internal-only. Make sure the change is applied consistently wherever the
chat history options are typed or destructured.
- Around line 7269-7274: The maxBytes handling in agentChatService should use
UTF-8 byte اندازه rather than string length when estimating the response
envelope. Update the logic around the requestedMaxBytes/responseMaxChars
calculation and the envelope sizing path to measure JSON output with
Buffer.byteLength(..., "utf8") instead of JSON.stringify(...).length, so the cap
in chat history responses matches the API contract for multi-byte content.
In `@apps/ios/ADE/Models/RemoteModels.swift`:
- Around line 734-780: `AgentChatSessionSummary` equality is missing several
state fields, so objects can compare equal even when they differ. Update the
`==` implementation in `AgentChatSessionSummary` to include
`cursorModeSnapshot`, `cursorConfigValues`, `computerUse`, and `completion`
alongside the existing property checks, unless they are intentionally meant to
be excluded from identity. This will also keep `CtoRoster` equality via `cto ==`
consistent with the full model state.
In `@apps/ios/ADE/Views/Hub/HubScreen.swift`:
- Around line 316-323: After restoring layout in HubScreen.restoreSavedLayout,
rebuild the presentation list immediately because hubProjectPresentations may
still reflect the default alphabetical order. Update the flow so the restored
projectOrder is applied before any derived presentations are used, then
regenerate or refresh the presentations with the restored order. Use the
existing restoreSavedLayout logic and the hubProjectPresentations source so the
persisted layout takes effect on reconnect.
- Line 27: Make the HubScreen view’s openChatTarget state private, since it is
only used as local mutable UI state. Update the property declaration for
openChatTarget in HubScreen.swift to use private with `@State`, and keep any
access through HubScreen’s own methods or bindings unless another type truly
needs it.
In `@apps/ios/ADE/Views/PRs/PrsRootScreen.swift`:
- Around line 1345-1357: The programmatic create-PR navigation path in
PrsRootScreen’s request.createLaneId handling bypasses the same canCreatePr gate
used by the manual “+” button. Before setting createPresented and opening the
createPrWizardSheet, check canCreatePr in this block and only proceed when
creation is allowed; otherwise ignore the request or route back without
presenting the wizard.
In `@apps/ios/ADE/Views/Work/WorkChatRichCardViews.swift`:
- Around line 1027-1060: Update statusRibbonBody in WorkChatRichCardViews so the
leading SF Symbol matches the normalized metadata status instead of defaulting
every non-failed case to pause.circle.fill; map completed/started/other statuses
to appropriate icons while keeping failed/interrupted distinct. Also fix the
accessibilityLabel on the ribbon to include the visible status text (normalized
or ribbonText) alongside card.title and card.body, so when card.body is empty
the status is still announced.
In `@apps/ios/ADE/Views/Work/WorkChatSessionView.swift`:
- Around line 139-160: The `WorkChatSessionView` state properties are triggering
visibility warnings because they are shared with
`WorkChatSessionView+Actions.swift` and `WorkChatSessionView+Timeline.swift`.
Keep the cross-file access intact and either add a narrow `swiftlint:disable
private_swiftui_state` with a brief rationale near the affected `@State`
declarations, or move the dependent logic into the same `WorkChatSessionView`
file. Focus on the state block containing `composerLayoutHeight`,
`timelineSnapshot`, `timelineRebuildTask`, and the other shared `@State`
members.
In `@apps/ios/ADE/Views/Work/WorkSessionDestinationView`+Actions.swift:
- Around line 697-701: The refreshChatPrDetails flow currently allows forced
calls to bypass the in-flight guard, which can start overlapping refresh tasks
and let stale snapshot/error state overwrite newer results after awaits. Update
refreshChatPrDetails so the single-flight protection always blocks when
prDetailsRefreshing is already true, and use force only to control whether the
refresh should bypass cached data or trigger a reload. Keep the fix localized to
refreshChatPrDetails and the prDetailsRefreshing/prDetailsError state handling.
---
Nitpick comments:
In `@apps/ade-cli/src/multiProjectRpcServer.test.ts`:
- Around line 61-89: Move the cleanup for createMultiProjectRpcRequestHandler
into the existing try/finally flow so it always runs, even when the
expect(init).toMatchObject assertion fails. Keep the dispose call next to the
handler variable in the multiProjectRpcServer test, and ensure handler.dispose()
is executed from the finally block after the registry/argv/env restoration
logic.
In `@apps/desktop/src/shared/types/sync.ts`:
- Around line 374-376: The SyncRosterSubscribePayload contract is ahead of the
actual host behavior: handleRosterSubscribe still ignores sinceSeq and always
returns a snapshot, so clients may rely on unsupported resume-by-seq behavior.
Either remove sinceSeq and its comment from SyncRosterSubscribePayload in
sync.ts, or update the host-side handleRosterSubscribe path to honor sinceSeq
and send a delta when possible, keeping the shared type and IPC behavior in
sync.
In `@apps/ios/ADE/Views/Components/ADECodeRenderingCache.swift`:
- Line 14: The highlighted-string cache in ADECodeRenderingCache only limits
entry count via attributedCache.countLimit, so large AttributedString values can
still consume too much memory. Update the cache setup to add a totalCostLimit
and make the caching path account for per-entry cost when storing highlighted
output, or reduce the existing limit if cost tracking is not feasible. Use the
ADECodeRenderingCache cache initialization and the highlighted-string storage
logic to apply the bound consistently.
In `@apps/ios/ADE/Views/Components/FilesCodeSupport.swift`:
- Around line 348-437: The diff-building logic in the inline diff routine should
be cleaned up so the `.unchanged` path uses the same `appendLine` helper as
added/removed lines, keeping ID and line-number formatting centralized in the
same code path. Update the line handling in the diff generation flow around
`appendLine` and `diffLines` so unchanged entries are appended consistently,
then expand the tests for this behavior to cover adjacent swaps, full rewrites,
empty inputs, and trailing-only insert/remove cases beyond the current
mixed-diff scenario.
In `@apps/ios/ADE/Views/Files/FilesModels.swift`:
- Around line 284-301: The byte-count path in FilesModels.swift still builds a
full combined string in the diff preview flow, which defeats the new large-diff
guard. In the logic that computes the limit check after
filesLinePairCountExceedsLimit, replace the combinedText concatenation with a
direct calculation using the UTF-8 counts of diff.original.text and
diff.modified.text (including the separator if needed) so FilesPreviewLimit and
filesTextLimit can be evaluated without duplicating the diff content.
In `@apps/ios/ADE/Views/Work/WorkChatPrViews.swift`:
- Around line 172-173: The PR-state tint mapping in WorkChatPrViews is
duplicated and can drift from the shared badge/details palette. Update the state
tint logic in the WorkChatPrViews flow to use the shared lanePullRequestTint
helper instead of the local workChatPrStateTint mapping, keeping the
branch/badge colors consistent wherever tag.state is rendered.
🪄 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: 5f5cbf68-4db2-4901-98b6-048f93eed3eb
⛔ Files ignored due to path filters (10)
apps/ios/ADE.xcodeproj/project.pbxprojis excluded by!**/*.xcodeproj/project.pbxprojdocs/features/automations/README.mdis excluded by!docs/**docs/features/chat/composer-and-ui.mdis excluded by!docs/**docs/features/files-and-editor/README.mdis excluded by!docs/**docs/features/files-and-editor/editor-surfaces.mdis excluded by!docs/**docs/features/onboarding-and-settings/README.mdis excluded by!docs/**docs/features/project-home/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/README.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**docs/features/sync-and-multi-device/remote-commands.mdis excluded by!docs/**
📒 Files selected for processing (100)
apps/ade-cli/src/bootstrap.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/headlessLinearServices.tsapps/ade-cli/src/multiProjectRpcServer.test.tsapps/ade-cli/src/multiProjectRpcServer.tsapps/ade-cli/src/services/projects/projectScope.tsapps/ade-cli/src/services/sync/rosterBuilder.test.tsapps/ade-cli/src/services/sync/rosterBuilder.tsapps/ade-cli/src/services/sync/sharedSyncListener.tsapps/ade-cli/src/services/sync/syncHostService.test.tsapps/ade-cli/src/services/sync/syncHostService.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.test.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.tsapps/ade-cli/src/services/sync/syncService.tsapps/desktop/src/main/main.tsapps/desktop/src/main/services/automations/automationIngressService.test.tsapps/desktop/src/main/services/automations/automationIngressService.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/github/githubRelayConfig.tsapps/desktop/src/main/services/github/githubService.test.tsapps/desktop/src/main/services/github/githubService.tsapps/desktop/src/main/services/lanes/laneService.tsapps/desktop/src/main/services/sync/syncHostService.test.tsapps/desktop/src/main/services/sync/syncRemoteCommandService.test.tsapps/desktop/src/main/services/sync/syncService.test.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/files/v2/FilesWorkbench.test.tsxapps/desktop/src/renderer/components/files/v2/FilesWorkbench.tsxapps/desktop/src/renderer/components/github/GitHubAppInstallPanel.tsxapps/desktop/src/shared/types/chat.tsapps/desktop/src/shared/types/sync.tsapps/ios/ADE/App/ContentView.swiftapps/ios/ADE/Debug/ADEInspectorKit/ADEInspectable.swiftapps/ios/ADE/Models/RemoteModels.swiftapps/ios/ADE/Models/RemoteRosterModels.swiftapps/ios/ADE/Resources/DatabaseBootstrap.sqlapps/ios/ADE/Services/Database.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Views/Components/ADECodeRenderingCache.swiftapps/ios/ADE/Views/Components/ADEDesignSystem.swiftapps/ios/ADE/Views/Components/FilesCodeSupport.swiftapps/ios/ADE/Views/Cto/CtoRootScreen.swiftapps/ios/ADE/Views/Cto/CtoTeamScreen.swiftapps/ios/ADE/Views/Cto/CtoWorkflowsScreen.swiftapps/ios/ADE/Views/Files/FilesDetailComponents.swiftapps/ios/ADE/Views/Files/FilesDetailScreen+Actions.swiftapps/ios/ADE/Views/Files/FilesDetailScreen.swiftapps/ios/ADE/Views/Files/FilesDirectoryContentsView.swiftapps/ios/ADE/Views/Files/FilesModels.swiftapps/ios/ADE/Views/Files/FilesRootComponents.swiftapps/ios/ADE/Views/Files/FilesSearchScreen.swiftapps/ios/ADE/Views/Hub/HubComponents.swiftapps/ios/ADE/Views/Hub/HubComposerDrawer.swiftapps/ios/ADE/Views/Hub/HubScreen+ChatNavigation.swiftapps/ios/ADE/Views/Hub/HubScreen.swiftapps/ios/ADE/Views/Lanes/LaneDetailScreen.swiftapps/ios/ADE/Views/PRs/PrHelpers.swiftapps/ios/ADE/Views/PRs/PrsRootScreen.swiftapps/ios/ADE/Views/Settings/ConnectionSettingsView.swiftapps/ios/ADE/Views/Settings/SettingsConnectionHeader.swiftapps/ios/ADE/Views/Settings/SettingsPairingSection.swiftapps/ios/ADE/Views/Work/WorkActivityIndicator.swiftapps/ios/ADE/Views/Work/WorkBrowserHelpers.swiftapps/ios/ADE/Views/Work/WorkChatAttachmentTray.swiftapps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swiftapps/ios/ADE/Views/Work/WorkChatHeaderAndMessageViews.swiftapps/ios/ADE/Views/Work/WorkChatPrViews.swiftapps/ios/ADE/Views/Work/WorkChatRichCardViews.swiftapps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swiftapps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swiftapps/ios/ADE/Views/Work/WorkChatSessionView.swiftapps/ios/ADE/Views/Work/WorkContextCompactDivider.swiftapps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swiftapps/ios/ADE/Views/Work/WorkEventMapping.swiftapps/ios/ADE/Views/Work/WorkMarkdownParsing.swiftapps/ios/ADE/Views/Work/WorkMarkdownViews.swiftapps/ios/ADE/Views/Work/WorkModelPickerSheet.swiftapps/ios/ADE/Views/Work/WorkModels.swiftapps/ios/ADE/Views/Work/WorkNavigationAndTranscriptHelpers.swiftapps/ios/ADE/Views/Work/WorkNewChatScreen.swiftapps/ios/ADE/Views/Work/WorkPlanComposerViews.swiftapps/ios/ADE/Views/Work/WorkPreviews.swiftapps/ios/ADE/Views/Work/WorkRootComponents.swiftapps/ios/ADE/Views/Work/WorkRootScreen+Actions.swiftapps/ios/ADE/Views/Work/WorkRootScreen.swiftapps/ios/ADE/Views/Work/WorkSessionDestinationView+Actions.swiftapps/ios/ADE/Views/Work/WorkSessionDestinationView.swiftapps/ios/ADE/Views/Work/WorkSessionGrouping.swiftapps/ios/ADE/Views/Work/WorkSessionSettingsSheet.swiftapps/ios/ADE/Views/Work/WorkStatusAndFormattingHelpers.swiftapps/ios/ADE/Views/Work/WorkTimelineHelpers.swiftapps/ios/ADE/Views/Work/WorkTranscriptParser.swiftapps/ios/ADETests/ADETests.swiftapps/ios/ADETests/SyncEnvelopeChunkAssemblerTests.swiftapps/ios/ADETests/WorkMarkdownStreamingParsingTests.swiftapps/webhook-relay/README.mdapps/webhook-relay/migrations/0003_github_events_repository.sqlapps/webhook-relay/src/relay.tsapps/webhook-relay/test/relay.test.ts
💤 Files with no reviewable changes (1)
- apps/ios/ADE/Resources/DatabaseBootstrap.sql
| export const ADE_GITHUB_APP_SLUG = "ade-for-github"; | ||
| export const ADE_GITHUB_APP_INSTALL_URL = `https://github.com/apps/${ADE_GITHUB_APP_SLUG}/installations/new`; | ||
| export const GITHUB_APP_INSTALLATIONS_URL = "https://github.com/settings/installations"; | ||
| export const DEFAULT_GITHUB_RELAY_API_BASE_URL = "https://ade-github-webhook-relay.arulsharma1028.workers.dev"; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether this hosted relay domain is documented/explained anywhere in the repo.
rg -n "arulsharma1028|ade-github-webhook-relay" --type=md --type=tsRepository: arul28/ADE
Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== githubRelayConfig.ts ==\n'
sed -n '1,220p' apps/desktop/src/main/services/github/githubRelayConfig.ts
printf '\n== search relay usage ==\n'
rg -n "readGitHubRelayConfig|DEFAULT_GITHUB_RELAY_API_BASE_URL|github relay|relay config|apiBaseUrl" apps/desktop/src/main/services -g '*.ts'
printf '\n== consumers referenced in comment ==\n'
sed -n '1,260p' apps/desktop/src/main/services/github/githubService.ts
printf '\n----\n'
sed -n '1,260p' apps/desktop/src/main/services/headlessLinearServices.ts
printf '\n----\n'
sed -n '1,280p' apps/desktop/src/main/services/automationIngressService.tsRepository: arul28/ADE
Length of output: 23055
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== automationIngressService.ts (relevant slice) ==\n'
sed -n '380,520p' apps/desktop/src/main/services/automations/automationIngressService.ts
printf '\n== automationIngressService.test.ts (relay-related slice) ==\n'
sed -n '150,380p' apps/desktop/src/main/services/automations/automationIngressService.test.ts
printf '\n== search for user-facing relay disclosure ==\n'
rg -n "GitHub relay|relay is not configured|ade-github-webhook-relay|GITHUB_RELAY|webhook relay|GitHub App relay" apps/desktop/src -g '*.ts' -g '*.tsx'Repository: arul28/ADE
Length of output: 22607
Don't ship a hardcoded personal relay as the default — unconfigured installs will send the user's GitHub token to ade-github-webhook-relay.arulsharma1028.workers.dev through both the status check and background poll paths. Use vendor-owned infrastructure here and require explicit user-facing consent for the hosted relay.
🤖 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/github/githubRelayConfig.ts` at line 8, The
default GitHub relay base URL is hardcoded to a personal hosted relay, which
would route tokens from unconfigured installs through an unintended third-party
endpoint. Update DEFAULT_GITHUB_RELAY_API_BASE_URL in githubRelayConfig to point
to vendor-owned infrastructure, and ensure the status check and background poll
paths only use a hosted relay after explicit user-facing consent or
configuration.
CI: - stdioRpcDaemon test: buildHash assertion was stale after the lane's `?? computeRuntimeBuildHash()` fallback; assert a computed hash instead of null. Host (CodeRabbit): - rosterBuilder: bound the getIfBooted overlay await (250ms) so a mid-boot scope can't stall the whole roster snapshot (Critical); open project DBs read-only. - githubRelayConfig: document the hosted-relay default (pre-launch item, deferred). iOS (CodeRabbit): - Hub: keep standalone CLI rows visible; key activeRoster to its project so a stale roster isn't merged post-switch; gate chat render on successful activation; thread targetProjectId through composer send/cleanup. - Work: recompute transcriptHasInterruptibleActivity on incremental tail; remove only the matched optimistic echo / resolve only the matched queued steer; preserve envelope ordering on assistant merge; UInt bucketing (no abs Int.min trap); guard cancelled lane-nav task; fall back to live lanes off-root. - RemoteModels: compare resumeMetadata.launch in session Equatable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7942ff1789
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| && lhs.codexFastMode == rhs.codexFastMode | ||
| && lhs.fastMode == rhs.fastMode | ||
| && lhs.executionMode == rhs.executionMode | ||
| && lhs.permissionMode == rhs.permissionMode |
There was a problem hiding this comment.
When only the Cursor mode snapshot/config changes, this equality still reports the fetched AgentChatSessionSummary as unchanged because cursorModeSnapshot/cursorConfigValues are not compared here. refreshChatSummaryFromHost only replaces chatSummary when chatSummary != fetchedSummary, while the Work UI derives the displayed Cursor mode from summary.cursorModeId ?? workCursorCurrentModeId(summary.cursorModeSnapshot), so hosts that refresh modes without changing cursorModeId leave the iOS session header/settings stuck on the old mode until another compared field changes.
Useful? React with 👍 / 👎.
Compare the four remaining fields the manual == skipped — cursorModeSnapshot, cursorConfigValues, computerUse, completion — so a host-side change to any of them (e.g. Cursor mode-only refresh) triggers the iOS session header/settings to update instead of latching stale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/stdioRpcDaemon.test.ts`:
- Around line 454-458: The test in stdioRpcDaemon should assert the exact
computed SHA-256 value instead of only checking truthiness; update the
runtimeInfo.buildHash expectation in stdioRpcDaemon.test.ts to match the full
64-character hex digest produced by the daemon’s entrypoint hashing logic, using
the initialize/runtimeInfo path as the locator.
🪄 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: 57ceacca-e51f-418c-a9f1-0edb4aef0677
📒 Files selected for processing (15)
apps/ade-cli/src/services/sync/rosterBuilder.tsapps/ade-cli/src/stdioRpcDaemon.test.tsapps/ade-cli/src/types/node-sqlite.d.tsapps/desktop/src/main/services/github/githubRelayConfig.tsapps/ios/ADE/Models/RemoteModels.swiftapps/ios/ADE/Services/SyncService.swiftapps/ios/ADE/Views/Hub/HubComponents.swiftapps/ios/ADE/Views/Hub/HubComposerDrawer.swiftapps/ios/ADE/Views/Hub/HubScreen+ChatNavigation.swiftapps/ios/ADE/Views/Hub/HubScreen.swiftapps/ios/ADE/Views/Work/WorkChatRichCardViews.swiftapps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swiftapps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swiftapps/ios/ADE/Views/Work/WorkRootScreen+Actions.swiftapps/ios/ADE/Views/Work/WorkRootScreen.swift
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/ios/ADE/Views/Hub/HubScreen+ChatNavigation.swift
- apps/desktop/src/main/services/github/githubRelayConfig.ts
- apps/ios/ADE/Views/Hub/HubScreen.swift
- apps/ios/ADE/Views/Work/WorkRootScreen+Actions.swift
- apps/ios/ADE/Views/Hub/HubComponents.swift
- apps/ios/ADE/Views/Work/WorkChatSessionView+Actions.swift
- apps/ios/ADE/Views/Hub/HubComposerDrawer.swift
- apps/ios/ADE/Models/RemoteModels.swift
- apps/ade-cli/src/services/sync/rosterBuilder.ts
- apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift
- apps/ios/ADE/Views/Work/WorkChatRichCardViews.swift
- apps/ios/ADE/Views/Work/WorkRootScreen.swift
Comment on lines
+454
to
+458
| // With no env-advertised build hash, the daemon computes a sha256 of its | ||
| // own entrypoint, so buildHash is a truthy string rather than null. | ||
| expect( | ||
| (initialize as { runtimeInfo?: { buildHash?: string | null } }).runtimeInfo?.buildHash, | ||
| ).toBeTruthy(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== FILE LIST ==\n'
git ls-files apps/ade-cli/src/stdioRpcDaemon.test.ts apps/ade-cli/src | sed -n '1,120p'
printf '\n== OUTLINE TEST FILE ==\n'
ast-grep outline apps/ade-cli/src/stdioRpcDaemon.test.ts --view expanded || true
printf '\n== SEARCH buildHash REFERENCES ==\n'
rg -n "buildHash|runtimeInfo" apps/ade-cli/src apps/ade-cli/test apps/ade-cli -g '!**/dist/**' -g '!**/build/**' || true
printf '\n== CONTEXT AROUND CITED LINES ==\n'
sed -n '380,480p' apps/ade-cli/src/stdioRpcDaemon.test.tsRepository: arul28/ADE
Length of output: 30108
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '430,470p' apps/ade-cli/src/stdioRpcDaemon.test.ts
printf '\n----\n'
rg -n "sha256|buildHash" apps/ade-cli/src apps/desktop/src . -g '!**/dist/**' -g '!**/build/**'Repository: arul28/ADE
Length of output: 50366
🌐 Web query:
TypeScript RegExp for SHA-256 64 hex build hash assertions best practice toBeTruthy non-empty string test weakness
💡 Result:
The recommended best practice for validating a SHA-256 hash in TypeScript is to use a strict, anchored regular expression, often combined with a type guard for runtime safety [1][2]. Recommended Regex Pattern To validate an exact SHA-256 hash (64 hexadecimal characters), use the following anchored regex [1]: const SHA256_REGEX = /^[a-f0-9]{64}$/i; - The ^ and $ anchors are critical to ensure the string contains nothing but the 64-character hash [1]. - The i flag makes it case-insensitive, accommodating both lowercase and uppercase hex representations [1][3]. Weakness of toBeTruthy and Non-empty String Tests Using generic assertions like toBeTruthy or checking that a string is simply non-empty is considered a significant testing weakness [4]. - toBeTruthy: This only checks if a value is "truthy." In JavaScript/TypeScript, this includes any non-empty string, non-zero number, or object. It provides zero validation that the string is a valid SHA-256 hash. - Non-empty check: This only confirms the string has length > 0. It would pass invalid values (e.g., "123", "abc", or 40-character SHA-1 hashes) [5]. Best Practices for Robust Validation 1. Use Type Guards: For production code, implement a type guard to ensure the string conforms to the expected hash format [2]. function isSha256(input: string): input is string { return /^[a-f0-9]{64}$/i.test(input); } 2. Prefer Dedicated Libraries: Instead of manually managing regex, use established libraries (e.g., Zod or dedicated hash validation utilities) that provide tested, reusable schemas and type safety [2][6]. 3. Positive Assertions in Tests: In your test suite, avoid "not equal" or "is truthy" assertions [4]. Instead, perform positive, explicit assertions that the generated hash matches the expected value or satisfies the format requirement (e.g., using toMatch or toHaveLength) [4][7]. 4. Security Warning: When dealing with crypto inputs, be aware that JavaScript strings can sometimes contain malformed UTF-16 sequences which may behave inconsistently across different hashing implementations [8]. Where possible, prefer working with Buffer or Uint8Array directly when using the Node.js crypto module or Web Crypto API [8][9].
Citations:
- 1: https://melvingeorge.me/blog/check-if-string-is-valid-sha256-hash-javascript
- 2: https://github.com/ehmpathy/hash-fns
- 3: fix: expand SHA regex to support SHA-256 (64-char) commit hashes actions/checkout#2413
- 4: https://glebbahmutov.com/cypress-examples/recipes/sha-256.html
- 5: fix: expand commit hash regex to support SHA-256 (64-char) hashes actions/runner#4347
- 6:
z.hash()colinhacks/zod#5049 - 7: https://github.com/CaoMeiYouRen/momei/blob/master/utils/shared/hash.test.ts
- 8: Equal hashes on non-equal JS strings are dangerous to the ecosystem nodejs/node#60267
- 9: https://dev.to/harmonyos/handling-exceptions-when-calculating-sha-256-or-md5-hash-for-empty-string-1520
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== TEST BLOCKS AROUND 340-410 ==\n'
sed -n '340,410p' apps/ade-cli/src/stdioRpcDaemon.test.ts | cat -n
printf '\n== TEST NAMES CONTAINING build hash ==\n'
rg -n 'itUnix\(|it\(|build hash' apps/ade-cli/src/stdioRpcDaemon.test.tsRepository: arul28/ADE
Length of output: 4882
Assert the full SHA-256 here. toBeTruthy() still accepts any non-empty string, so match the expected 64-character hex hash instead.
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn, type ChildProcessWithoutNullStreams } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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/ade-cli/src/stdioRpcDaemon.test.ts` around lines 454 - 458, The test in
stdioRpcDaemon should assert the exact computed SHA-256 value instead of only
checking truthiness; update the runtimeInfo.buildHash expectation in
stdioRpcDaemon.test.ts to match the full 64-character hex digest produced by the
daemon’s entrypoint hashing logic, using the initialize/runtimeInfo path as the
locator.
Source: Coding guidelines
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.
Large lane consolidating mobile (iOS) sync stability work, the GitHub webhook relay productization, and removal of file edit-protection.
iOS sync/chat stability (main focus)
Audited end-to-end (transport, chat render pipeline, SQLite/CRR, host). Crash/perf hardening, all verified (iOS build green, desktop sync 24/24, ade-cli mpRpc 10/10):
Int64(Double)bind trap (safeInt64Value) — a poison-batch launch crasher; regression-testedgunzippointer-escape UB; enforce 32 MiB chunk-reassembly byte cap (tested)reconnectConnectInFlight)chat_subscribesnapshot clamped to clientmaxBytes+ snapshot/pump dedupe (tested)isChatToolType(cursor/droid + orphan rows)CollectionDifference, computed once into@StateCGImageSourceCreateThumbnailAtIndexrecordChatEventEnvelope, cappedprDetailCache, CTO tab-visibility gate, LaneDetail defer-not-drop throttleWebhook relay productization
Hosted Cloudflare Worker is the default; ADE polls repo-scoped routes (
/github/repos/:owner/:repo/status+/events) authorized by the user's existing GitHub token — no relay token /local.secret.yamlneeded. Legacy/projects/:projectId/...token routes preserved for self-hosted. HMAC-verified ingestion, table-wide retention pruning, negative-authz tested.Edit-protection removed
Files are freely editable on desktop and mobile — no "Enable editing" step. The
is_edit_protectedcolumn and its auto-rebase exclusion are untouched; only the file-edit gate was removed.Tests / parity
Webhook 19/19, desktop sync/files/github green, TUI 744 green, ade-cli typecheck clean. Docs updated (files-and-editor, automations, sync-and-multi-device, onboarding). Mobile/CLI/TUI parity: no changes required.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Greptile Summary
This PR consolidates three workstreams: iOS/mobile sync stability hardening, webhook relay productization (hosted Cloudflare Worker as default, GitHub-token auth), and removal of the file edit-protection gate.
Int64(Double)bind trap insafeInt64Value, agunzippointer-escape UB, a reconnect-wedge via generation-pairedreconnectConnectInFlight, and adds a 64-session LRU cap on the chat event history cache.DEFAULT_GITHUB_RELAY_API_BASE_URL); desktop polls/github/repos/:owner/:repo/eventsusing the user's existing GitHub token instead of a manually configured relay token. Legacy/projects/:projectId/...routes are preserved for self-hosted deployments withRELAY_ACCESS_TOKENset.is_edit_protectedDB column and auto-rebase exclusion remain untouched.Confidence Score: 5/5
Safe to merge. All three workstreams are well-scoped, the iOS crash fixes are correct, and the relay productization is backed by 19 passing tests with explicit negative-authz coverage.
The iOS
safeInt64Valuebound uses strict< Double(Int64.max)(not<=), which correctly avoids the floating-point rounding trap. TheSyncEnvelopeChunkAssemblerbyte cap tracks decoded bytes before assembly, closing the UB window. Generation pairing onreconnectConnectInFlightis properly threaded through all clear sites. The two observations flagged are design-level and do not affect correctness.apps/webhook-relay/src/relay.ts(assertGitHubRepoAuthorized) andapps/desktop/src/main/services/github/githubRelayConfig.ts(configuredfield) are worth revisiting as usage scales up.Important Files Changed
configuredto always true for the hosted default, and addsshouldUseLegacyGitHubRelayProjectRoute.safeInt64Valuecrash fix, 32 MiB chunk cap, generation-pairedreconnectConnectInFlight, 64-session LRU chat history eviction, and all-projects roster subscription.safeInt64Valueguard (isFinite+ strict< Double(Int64.max)) to fix theInt64(Double)bind trap.mergeSubagentSnapshots, hard-cap option for mobile, andmergeAssistantTranscriptTextfor non-contiguous turn merging.(repository_full_name collate nocase, received_at desc)to support the new repo-scoped events queries.Reviews (3): Last reviewed commit: "ship: iteration 2 — sweep AgentChatSessi..." | Re-trigger Greptile