Settings Refresh#390
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
📝 WalkthroughWalkthroughThis PR adds Claude thinking transcript auto-repair logic, implements app release version checking with UI integration, redesigns the onboarding flow from a multi-step stepper to a single-page card layout, restructures settings navigation, and fixes a plan-mode state synchronization race condition in the chat pane. ChangesClaude Thinking Transcript Repair and Session Improvements
Release Version Checking Infrastructure
Onboarding Redesign: Theme, Cards, and Page Layout
Settings Navigation and Component Restructuring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review
Scope: 32 file(s), +2945 / −802
Verdict: Needs changes
This PR reshapes first-run onboarding into a dashboard of setup cards, splits Settings into clearer tabs (AI Connections, Background Jobs, About), and adds Claude thinking-transcript self-repair plus plan-mode UI fixes. The onboarding and chat work look solid, but the settings consolidation removes user-facing project maintenance controls without relocating them.
🐛 Functionality
[High] Project repair and integrity UI removed with no replacement
File: apps/desktop/src/renderer/components/settings/ProjectSection.tsx (deleted); apps/desktop/src/renderer/components/app/SettingsPage.tsx:L26-L28,L171-L172
Issue: ProjectSection (project snapshot, structure repair, integrity check) and the Workspace settings tab are deleted. Deep links ?tab=workspace / ?tab=project now alias to General, which only shows setup re-entry, CLI, and About — there is no remaining renderer call to window.ade.project.getSnapshot, initializeOrRepair, or runIntegrityCheck.
Repro: Open Settings → General (or command palette / bookmark ?tab=workspace). Previously available repair/integrity actions are gone.
Fix: Restore project health controls (e.g. reintroduce ProjectSection under General or a dedicated subsection) or mount the same actions on another settings surface before merging.
[Medium] Finish setup stays disabled when essentials scan fails
File: apps/desktop/src/renderer/components/onboarding/DevToolsRow.tsx:L20-L34; apps/desktop/src/renderer/components/onboarding/ProjectSetupPage.tsx:L63-L64
Issue: gitInstalled starts as null and is only updated inside the try of refresh(). If window.ade.devTools.detect() rejects, onGitStatusChange never runs, so gitInstalled stays null and Finish setup remains disabled (gitInstalled !== true) even though Skip still works.
Repro: Simulate or trigger a failed devTools.detect IPC (runtime unavailable) on the onboarding page.
Fix: Call onGitStatusChange(false) in a catch (or finally when tools is missing), and/or treat null as “unknown” with an explicit error state instead of locking Finish.
[Medium] Linear OAuth polling can stack on repeated Sign in
File: apps/desktop/src/renderer/components/onboarding/LinearCard.tsx:L49-L82
Issue: Each beginOAuth() starts a new setInterval without clearing an existing pollRef. A second click while oauthBusy leaves two pollers running until one completes, causing duplicate getLinearOAuthSession calls and possible duplicate state updates.
Repro: Click Sign in twice quickly on the Linear card.
Fix: Clear any existing interval at the start of beginOAuth, and disable the button while oauthBusy (already partially done — also guard against re-entry).
⚡ Performance
[Medium] Worktree import rescans reset manual selection
File: apps/desktop/src/renderer/components/onboarding/WorktreesCard.tsx:L104
Issue: Rescan always calls load({ preselectAll: true }), which replaces selected with every worktree path. Users who unchecked items lose their selection on every rescan.
Impact: Extra accidental bulk imports after adjusting checkboxes and clicking Rescan.
Fix: Use preselectAll only on initial mount; on rescan call load({ keepSelected: selected }) or preserve the current Set.
🎨 UI/UX
[Medium] AI runtimes grid does not adapt on narrow widths
File: apps/desktop/src/renderer/components/onboarding/AiRuntimesBand.tsx:L184-L187
Issue: Runtime cards use gridTemplateColumns: "repeat(6, 1fr)" with fixed column spans. On viewports under ~640px (and inside the setup page’s wrapping flex layout), cards compress to very narrow columns with no minWidth, media query, or single-column fallback.
Fix: Switch to repeat(auto-fit, minmax(…)) or a @media (max-width: 640px) rule that stacks cards in one column (matching the PR’s mobile layout checklist).
[Low] Disconnect actions have no confirmation
File: apps/desktop/src/renderer/components/onboarding/LinearCard.tsx:L155-L157; apps/desktop/src/renderer/components/onboarding/GitHubCard.tsx:L88-L91
Issue: Disconnect clears tokens immediately with no confirm or undo, easy to mis-click during setup.
Fix: Add a lightweight confirm dialog or an undo toast consistent with other settings integrations.
[Low] About block renders nothing while app info loads
File: apps/desktop/src/renderer/components/settings/AboutSection.tsx:L82
Issue: if (!info) return null leaves the About section empty with no skeleton or “Loading…” for the duration of getInfo() / getLatestRelease() (often >300ms on cold open).
Fix: Show a compact loading placeholder in the card until info is available.
Notes
- Positive: Claude thinking-transcript repair (
claudeThinkingTranscriptRepair.ts) is well bounded (session id allowlist, in-place JSONL repair, tests), and the plan-mode composer fix plussystem_noticeon auto-exit address a real stuck-UI bug. AboutSection+fetchAdeLatestReleasedegrade gracefully when GitHub is unreachable; token use reuses existing encrypted store patterns.- General settings no longer surfaces local runtime service install/health (removed from
GeneralSection); if that was intentional, consider whether operators still need it elsewhere (top bar, diagnostics). - Command palette Go to Usage still targets
?tab=usage→ General; that predates this PR, but Background Jobs is now a separate tab — worth a follow-up palette entry.
Sent by Cursor Automation: BUGBOT in Versic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/components/onboarding/ProjectSetupPage.tsx (1)
19-39:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle IPC failures in
finish/skip.Neither handler catches errors from
window.ade.onboarding.complete()/setDismissed(). Combined withonClick={() => void finish()}, a rejected IPC call becomes an unhandled promise rejection and the user sees the button silently revert to "Finish setup" with no indication the action failed.Separately,
busyis shared, so clicking Skip also flips the Finish button to "Saving...". Consider a dedicated flag or only showing the saving label for the finish action.🛡️ Proposed error handling
const finish = async () => { setBusy(true); try { const next = await window.ade.onboarding.complete(); publishOnboardingStatusUpdated(next); navigate("/work", { replace: true }); + } catch (err) { + console.error("Failed to complete onboarding", err); + // surface a user-visible error (toast/inline message) here } finally { setBusy(false); } };(apply the same
catchtoskip)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/onboarding/ProjectSetupPage.tsx` around lines 19 - 39, The finish and skip handlers (finish and skip) do not catch errors from the IPC calls window.ade.onboarding.complete() and window.ade.onboarding.setDismissed(), causing unhandled promise rejections and no user feedback; update both functions to wrap the await calls in try/catch (keeping the existing finally to setBusy(false)), log or surface the error to the user (e.g., via a toast/error banner or process logger) inside the catch, and return/short-circuit as appropriate; also separate the shared busy state into two flags (e.g., finishBusy vs skipBusy or a dedicated isFinishing flag used for the Finish button label) so clicking Skip does not change the Finish button label.
🧹 Nitpick comments (1)
apps/desktop/src/main/services/chat/agentChatService.ts (1)
15289-15300: ⚡ Quick winExtract the repair-and-log into a shared helper.
This
repairClaudeResumeTranscript(...)+logger.warn("agent_chat.claude_thinking_transcript_repaired", {...})block is now repeated three times (here, hunk 3 self-heal, and hunk 5 prewarm) with only theatvalue differing. A small helper removes the drift risk and centralizes the log shape.♻️ Suggested helper (types illustrative)
function repairAndLogClaudeTranscript( managed: ManagedSession, sdkSessionId: string, worktreePath: string, at: "self_heal" | "resume" | "prewarm", ) { const repair = repairClaudeResumeTranscript(sdkSessionId, worktreePath); if (repair.repaired) { logger.warn("agent_chat.claude_thinking_transcript_repaired", { sessionId: managed.session.id, sdkSessionId, responsesRekeyed: repair.responsesRekeyed, reusedMessageIds: repair.reusedMessageIds, at, }); } return repair; }Then each site collapses to e.g.:
- if (!runtime.warmQuery && options.resume) { - const repair = repairClaudeResumeTranscript(options.resume, managed.laneWorktreePath); - if (repair.repaired) { - logger.warn("agent_chat.claude_thinking_transcript_repaired", { - sessionId: managed.session.id, - sdkSessionId: options.resume, - responsesRekeyed: repair.responsesRekeyed, - reusedMessageIds: repair.reusedMessageIds, - at: "resume", - }); - } - } + if (!runtime.warmQuery && options.resume) { + repairAndLogClaudeTranscript(managed, options.resume, managed.laneWorktreePath, "resume"); + }🤖 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 15289 - 15300, The repeated pattern calling repairClaudeResumeTranscript(...) followed by logger.warn("agent_chat.claude_thinking_transcript_repaired", {...}) should be extracted into a single helper to avoid duplication and keep the log shape consistent; add a function (e.g., repairAndLogClaudeTranscript(managed: ManagedSession, sdkSessionId: string, worktreePath: string, at: "self_heal" | "resume" | "prewarm")) that calls repairClaudeResumeTranscript(sdkSessionId, worktreePath), checks repair.repaired, emits the logger.warn with sessionId: managed.session.id, sdkSessionId, responsesRekeyed, reusedMessageIds and the provided at value, returns the repair object, and replace the three inline blocks with calls to this helper.
🤖 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/desktop/src/main/services/chat/agentChatService.ts`:
- Around line 11004-11011: The warn call currently logs
"agent_chat.claude_thinking_transcript_repaired" even when
repairClaudeResumeTranscript(...) did not actually repair anything; update the
code to only emit this event when the repair result indicates a real fix (check
repair.repaired === true) by moving the logger.warn call inside an if
(repair.repaired) block (or alternatively change the event name to reflect
"detected" instead of "repaired"); reference the repairClaudeResumeTranscript
return value and the logger.warn invocation to implement this conditional
logging.
In `@apps/desktop/src/main/services/chat/claudeThinkingTranscriptRepair.ts`:
- Around line 164-172: The current atomic rewrite uses a deterministic temp name
`${filePath}.repair-${process.pid}.tmp` (tmp) which can collide across
concurrent repairs; change tmp generation in claudeThinkingTranscriptRepair.ts
to a collision-safe name (for example include a random/unique suffix such as
crypto.randomUUID() or use fs.mkdtemp) so each operation writes to a unique temp
file before calling fs.writeFileSync(tmp, ...) and fs.renameSync(tmp, filePath);
keep the existing cleanup code paths that check fs.existsSync(tmp) and
fs.unlinkSync(tmp) but operate on the new unique tmp variable.
In `@apps/desktop/src/renderer/browserMock.ts`:
- Around line 5113-5125: The mock IPC contract is inconsistent: getStatus()
returns the extended shape but setToken() and clearToken() still return the old
shape; update setToken and clearToken to return the same object properties as
getStatus (include tokenStored, patTokenStored, tokenDecryptionFailed,
storageScope, authSource, tokenType, repo, hasOrigin, userLogin, scopes,
ghCliPath, ghAuthError) so renderer flows see a consistent response; locate the
mock handlers named getStatus, setToken, and clearToken in browserMock.ts and
make the mutation return values match the getStatus response shape exactly.
In `@apps/desktop/src/renderer/components/onboarding/AiRuntimesBand.tsx`:
- Around line 227-230: Update the stale helper text in the AiRuntimesBand
component: replace both occurrences of the hardcoded "Settings · AI" string
inside the JSX (in AiRuntimesBand.tsx) with the new settings tab label "Settings
· Integrations" so the helper links point to the renamed Settings tab.
In `@apps/desktop/src/renderer/components/onboarding/LinearCard.tsx`:
- Around line 10-19: The openExternal function currently bypasses the typed
preload contract by casting window.ade to unknown and falls back to window.open;
replace this with a call to the typed preload IPC API (use the existing preload
contract type for window.ade) and invoke the secure opener method
(opener.openExternal) without any unknown casts, and remove the raw window.open
fallback; instead, handle a missing opener by returning an error or logging via
the renderer logger and ensure callers know the promise rejection/void behavior;
refer to the openExternal function, window.ade, and opener.openExternal to
locate the code to update.
- Around line 61-82: The interval callback does not stop polling when
getLinearOAuthSession throws, so inside the catch block of the poll (the
function using pollRef and calling cto.getLinearOAuthSession({ sessionId }))
clear the interval (window.clearInterval(pollRef.current)), set pollRef.current
= null, setOauthBusy(false) and setOauthError(...) with the caught error (e.g.,
e instanceof Error ? e.message : String(e)) before returning; ensure this
mirrors the cleanup done for completed/failed/expired branches and keeps
refresh() behavior unchanged.
In `@apps/desktop/src/renderer/components/onboarding/WorktreesCard.tsx`:
- Around line 78-79: The fallback lane name currently derives from wt.path using
wt.path.split("/").pop(), which breaks on Windows; update WorktreesCard.tsx to
use a cross-platform basename (e.g., path.basename) instead: import the Node
path module and replace wt.path.split("/").pop() with path.basename(wt.path) so
the line that sets const name = wt.branch || wt.path.split("/").pop() ||
"worktree"; becomes const name = wt.branch || path.basename(wt.path) ||
"worktree"; (leave the subsequent await window.ade.lanes.attach({ name,
attachedPath: wt.path }) unchanged).
In `@apps/desktop/src/renderer/components/settings/AboutSection.tsx`:
- Around line 66-76: The manual update flow in AboutSection.tsx only toggles
checking via setChecking and never refreshes the latest release data, so after
updateCheckForUpdates completes the UI can still show stale info; update the
logic to re-fetch or update the latest-release state when the check finishes by
calling window.ade.app.getLatestRelease() (or using the AutoUpdateSnapshot
payload) either in the .catch/.finally of window.ade.updateCheckForUpdates()
inside checkForUpdates or inside the window.ade.onUpdateEvent handler that
receives AutoUpdateSnapshot, and then update the component state that holds the
latest release so the version/pill/release-notes link reflect the new release.
---
Outside diff comments:
In `@apps/desktop/src/renderer/components/onboarding/ProjectSetupPage.tsx`:
- Around line 19-39: The finish and skip handlers (finish and skip) do not catch
errors from the IPC calls window.ade.onboarding.complete() and
window.ade.onboarding.setDismissed(), causing unhandled promise rejections and
no user feedback; update both functions to wrap the await calls in try/catch
(keeping the existing finally to setBusy(false)), log or surface the error to
the user (e.g., via a toast/error banner or process logger) inside the catch,
and return/short-circuit as appropriate; also separate the shared busy state
into two flags (e.g., finishBusy vs skipBusy or a dedicated isFinishing flag
used for the Finish button label) so clicking Skip does not change the Finish
button label.
---
Nitpick comments:
In `@apps/desktop/src/main/services/chat/agentChatService.ts`:
- Around line 15289-15300: The repeated pattern calling
repairClaudeResumeTranscript(...) followed by
logger.warn("agent_chat.claude_thinking_transcript_repaired", {...}) should be
extracted into a single helper to avoid duplication and keep the log shape
consistent; add a function (e.g., repairAndLogClaudeTranscript(managed:
ManagedSession, sdkSessionId: string, worktreePath: string, at: "self_heal" |
"resume" | "prewarm")) that calls repairClaudeResumeTranscript(sdkSessionId,
worktreePath), checks repair.repaired, emits the logger.warn with sessionId:
managed.session.id, sdkSessionId, responsesRekeyed, reusedMessageIds and the
provided at value, returns the repair object, and replace the three inline
blocks with calls to this helper.
🪄 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: 079ba577-3d60-46ff-a500-b84ac6ec1340
⛔ Files ignored due to path filters (2)
docs/features/chat/README.mdis excluded by!docs/**docs/features/onboarding-and-settings/README.mdis excluded by!docs/**
📒 Files selected for processing (30)
apps/desktop/src/main/services/chat/agentChatService.tsapps/desktop/src/main/services/chat/claudeThinkingTranscriptRepair.test.tsapps/desktop/src/main/services/chat/claudeThinkingTranscriptRepair.tsapps/desktop/src/main/services/github/githubService.test.tsapps/desktop/src/main/services/github/githubService.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/browserMock.tsapps/desktop/src/renderer/components/app/CommandPalette.tsxapps/desktop/src/renderer/components/app/SettingsPage.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/onboarding/AiRuntimesBand.tsxapps/desktop/src/renderer/components/onboarding/DevToolsRow.tsxapps/desktop/src/renderer/components/onboarding/GitHubCard.tsxapps/desktop/src/renderer/components/onboarding/InputPopover.tsxapps/desktop/src/renderer/components/onboarding/LinearCard.tsxapps/desktop/src/renderer/components/onboarding/ProjectSetupPage.tsxapps/desktop/src/renderer/components/onboarding/RescanButton.tsxapps/desktop/src/renderer/components/onboarding/WorktreesCard.tsxapps/desktop/src/renderer/components/onboarding/onboardingTheme.tsapps/desktop/src/renderer/components/settings/AboutSection.tsxapps/desktop/src/renderer/components/settings/AiSettingsSection.tsxapps/desktop/src/renderer/components/settings/GeneralSection.tsxapps/desktop/src/renderer/components/settings/IntegrationsSettingsSection.tsxapps/desktop/src/renderer/components/settings/ProjectSection.tsxapps/desktop/src/renderer/components/settings/WorkspaceSettingsSection.tsxapps/desktop/src/shared/ipc.tsapps/desktop/src/shared/types/core.ts
💤 Files with no reviewable changes (3)
- apps/desktop/src/renderer/components/settings/WorkspaceSettingsSection.tsx
- apps/desktop/src/renderer/components/settings/ProjectSection.tsx
- apps/desktop/src/renderer/components/settings/AiSettingsSection.tsx
arul28
force-pushed
the
ade/settings-refresh-2db507ca
branch
from
55d6bb3 to
4f4e060
Compare
1 participant
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Greptile Summary
This PR is a Settings Refresh that reorganizes and expands the settings UI, adds self-healing transcript repair for corrupted Claude SDK sessions, and introduces a GitHub release-check endpoint for update notifications.
AboutSectioncomponent surfaces app version, latest release, and update-check controls, extracting that logic fromGeneralSection; new dedicated onboarding cards (GitHubCard,LinearCard,WorktreesCard,AiRuntimesBand) are added.claudeThinkingTranscriptRepair.tsdetects reusedmessage.idcollisions in Claude SDK JSONL transcripts and rewrites them atomically;agentChatService.tsinvokes the repair at pre-warm, cold-resume, and error-recovery sites.fetchAdeLatestReleasefetches GitHub Releases API and compares against the running version to surface an "Update available" badge in the About panel.Confidence Score: 4/5
Safe to merge with one small fix: the LinearCard error message when a token is rejected is misleading and should be corrected first.
The transcript repair, settings restructure, and update-check plumbing are all well-implemented with proper guards. The one concrete defect is in LinearCard: when
setLinearTokenreturns a non-connected status the error toast says "Linear connected", which actively misinforms the user about what went wrong.apps/desktop/src/renderer/components/onboarding/LinearCard.tsx — the
saveApiKeyreturn value for the connection-failed case uses the wrong message string.Important Files Changed
saveApiKeyreturns the success string "Linear connected" as its error message when the API call succeeds butconnectedis false, causing the InputPopover to render "Linear connected" in red.system_noticeevent on full-auto plan-mode exit to fix a stale UI chip; changes look correct and well-commented.fetchAdeLatestReleaseusing the GitHub Releases API; gracefully returns null on any failure, handles token optionally, and normalises the version string. Looks correct.formatReleasedAgohandles future timestamps gracefully.opencodeBinaryInstalled !== falseover-counts ready runtimes when the field is undefined (previously flagged outside diff).savePatis correct (returnsghAuthErrorwhenconnectedis false).Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[AgentChatPane receives system_notice] -->|exited_plan_mode| B[setInteractionMode default] B --> C[setClaudePermissionMode default if was plan] A -->|entered_plan_mode| D[setInteractionMode plan] E[Chat error handler] -->|isCorruptThinkingTranscriptError| F[repairClaudeResumeTranscript] F -->|repaired| G[refreshReconstructionContext + prewarm] F -->|no-op| H[emit error event as-is] I[prewarmClaudeQuery] --> J{options.resume?} J -->|yes| K[repairClaudeResumeTranscript at prewarm] J -->|no| L[skip repair] M[cold start path] --> N{options.resume && !warmQuery?} N -->|yes| O[repairClaudeResumeTranscript at resume] N -->|no| P[skip repair] Q[AboutSection mounts] --> R[window.ade.app.getLatestRelease] R --> S{updateAvailable?} S -->|yes| T[Show warning badge + release notes button] S -->|no| U[Show up-to-date badge]Comments Outside Diff (2)
apps/desktop/src/main/services/chat/claudeThinkingTranscriptRepair.ts, line 525-527 (link)catchreturnsNO_OPwhenwriteFileSyncorrenameSyncthrows, so a disk-full or permission error is silently ignored and the next call also returnsNO_OP(because the backup already exists, skipping the backup step, but the file was never actually repaired). Consider at least logging the failure so it shows up in the warning trail alongside theagent_chat.claude_thinking_transcript_repairedlog.Prompt To Fix With AI
apps/desktop/src/renderer/components/onboarding/AiRuntimesBand.tsx, line 1222-1224 (link)readyCountheuristic for OpenCode treats any non-falsevalue ofopencodeBinaryInstalledas ready, includingundefined(when the field is absent from an older status snapshot). If the backend hasn't populated that field yet, the count will over-report one ready runtime. Guard with a strict=== truecheck to match the pattern used for every other runtime.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "fix: address settings refresh review fin..." | Re-trigger Greptile