Repository navigation
v2.5.6 — Live Preview editor, community tool registry, note-delete + OAuth-cancel fixes - #92
Merged
Merged
Conversation
…at and Coding Agent - Add Temperature control to AI & Chat settings (matching Coding Agent) - Add Context window auto-detection via models.dev for both AI & Chat (Cloud/Local API) and Coding Agent settings, with fuzzy id matching for proxy/gateway model ids and a persisted contextAuto override flag - Auto-apply detected context on model change; manual value/preset disables Auto, tapping Auto re-enables - Layer localStorage under backend agent config on hydrate so contextAuto survives restarts - Correct AI Provider description (drop mobile-only Apple Intelligence) - Reflow settings rows on narrow windows via container queries - Allow outbound https: in dev CSP so the models.dev fetch succeeds
Write mode now hides markdown syntax markers on every line except the one the cursor is on, and renders formatting inline — headings at true size, bold/italic/code, bullets as •, links as display text, ==highlight==, [[wikilink]] chips, and blockquote borders — matched to Read mode's .prose-cairn styles. Raw markdown is unchanged on disk. Also fixes in-editor formatting that never worked: the theme referenced .tok-* classes that CodeMirror never emitted. Added a HighlightStyle mapping Lezer tags to those classes. - src/lib/livePreview.ts: marker-hiding + inline decoration extension - markdown-editor.tsx: HighlightStyle + livePreview compartment/prop - note-editor.tsx: enable Live Preview in Write mode - livePreview.component.test.tsx: 6 tests (ordering, classes, active-line reveal)
Obsidian-style callouts (> [!note] …) now render as their coloured box inline in Write mode, reusing the Read-mode <Callout> component and NoteMarkdownPreview for the body. Cursor outside the callout → widget; cursor inside → raw source for editing (click-to-edit). Block-level replace decorations can't be provided via a ViewPlugin, so callouts use a dedicated StateField (calloutField); inline decorations stay in the ViewPlugin. Both share findCalloutBlocks() so inline passes skip lines a widget covers (avoiding overlapping decorations). React roots mount in WidgetType.toDOM() and unmount via queueMicrotask. - src/lib/callout-widget.tsx: CalloutWidget + parseCalloutSource - src/lib/livePreview.ts: calloutField StateField + inline-pass guards - livePreview.component.test.tsx: 11 tests (widget in/out, parse)
Inline-SVG brand glyphs keyed by the cairn-community manifest 'logo' id (jira, confluence, linear, notion, monday, github, sentry, brave, hackernews) with a generic plug fallback. Scaffolding for Registry 2's Browse Community picker; not yet wired into the UI.
Fetches the cairn-community catalog (manifest.json) with a conditional GET (stored ETag), caches the last good manifest in userData for offline/instant reads, and Zod-validates the payload (https-only URLs, required fields) mirroring cairn-community/schema.json. Fail-soft: any network/parse error serves the cache; only a failure with no cache surfaces an error. - electron/lib/community-registry.ts (fetch + cache + validate) - electron/ipc/community-registry-handlers.ts (registry:fetch / :refresh) - preload registry namespace (auto-typed via ElectronAPI) - registry manifest types in src/types/index.ts - 9 unit tests (200/304/offline/no-cache/cache-first/refresh) Read/cache layer only; browse + install UI is Registry 2.
Settings -> Tools -> Browse opens the cairn-community catalog: search + tag-filtered MCP servers and HTTP services, each with brand logo (ConnectorLogo), blurb, and the exact endpoint shown before install. - BrowseCommunityModal.tsx: fetch (cache-first) + refresh, search, tag chips, install/update/installed states, secret prompt for <API_KEY> placeholder headers, OAuth badge - tools slice: installCommunityMcp / installCommunityService — install DISABLED with source=community + communityId + version; secrets stored in the keychain (secret:// refs), never persisted raw; re-install onto the same id preserves keychain secrets (update path) - ToolsSettings: Browse Community card + button, refetch on close - Update-available badge when installed version != manifest version Builds on Registry 1 (window.electron.registry). type-check:all + compile clean; 14 registry tests green.
- oauth-loopback: parse OAuth error responses (RFC 6749) — access_denied now rejects with an OAuthDeniedError + friendly 'cancelled' page instead of a misleading 'missing/invalid' 400; generic errors surface their description; the 10-min TTL rejects with a clear 'timed out' message - mcp-oauth: track active loopback listeners per server; cancelServerAuth(id) tears one down (rejects the pending callback -> renderer gets a cancelled completion) - IPC tools:cancelMcpAuth + preload cancelMcpAuth - McpAuthButton: while waiting, show a Cancel button (was a dead 'Waiting for browser…' state) - 4 new loopback tests (deny / generic error / cancel reason) Also expands ConnectorLogo with glyphs for the 7 new cairn-community connectors (asana, stripe, hubspot, canva, intercom, cloudflare, vercel).
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughWalkthroughVersion 2.5.6 adds community tool discovery and installation, Markdown live preview rendering, automatic model context detection, improved MCP OAuth cancellation, tombstone-based note deletion, safer note-link writes, responsive settings layout changes, and development HTTPS connectivity. ChangesCommunity registry and tool installation
Markdown editor live preview
Model context detection and settings
MCP sign-in cancellation and status
Note deletion, linking, and runtime fixes
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/lib/livePreview.ts (1)
173-272: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: collapse the repeated full-tree traversals in the rebuild path.
buildDecorationsruns on every doc change, selection move, and viewport scroll, and each rebuild performs four separatesyntaxTree(...).iterate(...)passes per visible range pluscalloutWidgetLineSet, which itself walks the entire tree (as doesbuildCalloutDecorationsin the sibling field). For large notes this repeated traversal is avoidable. Consider a singleiteratewith aswitch (node.name)for the mark/blockquote/link/list passes, and computing the callout block set once per rebuild and threading it through. Not required for correctness.🤖 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 `@src/lib/livePreview.ts` around lines 173 - 272, Optionally optimize buildDecorations by consolidating the separate syntaxTree(...).iterate passes for hidden marks, blockquotes, links, and list markers into one traversal using node-name branching. Compute calloutWidgetLineSet once per rebuild and reuse it across the traversal and scanInlinePatterns, preserving all existing filtering and decoration behavior.src/lib/models-dev.ts (1)
44-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider unit tests for the fuzzy-matching logic.
normalizeId/getNormIndex/lookupimplement a fairly intricate regex-based normalization chain (region/vendor prefixes, separator swapping, suffix stripping) with no accompanying test file in this cohort. Given how easily proxy/gateway id formats can regress this logic silently, a small table-driven test (e.g.playground-claude-opus-4-8,us.anthropic.claude-3-opus-20240229-v1:0,gpt-4.1↔gpt-4-1) would guard against future regressions.🤖 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 `@src/lib/models-dev.ts` around lines 44 - 93, The fuzzy model-ID matching logic lacks regression coverage. Add a focused, table-driven unit test for normalizeId and lookup that verifies gateway/vendor and region prefix removal, date/version and variant suffix stripping, and separator-swapping matches such as playground-claude-opus-4-8, us.anthropic.claude-3-opus-20240229-v1:0, and gpt-4.1/gpt-4-1.src/components/settings/AISettings.tsx (1)
45-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated models.dev auto-detect logic into a shared hook.
AISettings.tsxandAgentSettings.tsxeach implement the same ~25-line effect (detect → cancellation guard → stale-closure-safe auto-apply viauseCairnStore.getState()), the same"idle" | "loading" | "detected" | "not_found"union (also re-declared a third time inshared.tsx), and the same hardcoded128000fallback that duplicatesDEFAULT_CONTEXT_LIMITalready exported fromsrc/lib/models-dev.ts. A shared hook (e.g.useModelContextAutoDetect(modelId, contextAuto, contextLimit, applyLimit)) would remove the duplication and centralize the constant/type.
src/components/settings/AISettings.tsx#L45-L76: replace this effect + localdetectedContext/autoStatestate with the shared hook; also update the128000fallback at lines 209/222 to importDEFAULT_CONTEXT_LIMITfrom@/lib/models-dev.src/components/settings/AgentSettings.tsx#L435-L463: replace this effect + localdetectedContextAgent/autoStateAgentstate with the same shared hook; also update the128000fallback at lines 548/562 to importDEFAULT_CONTEXT_LIMITfrom@/lib/models-dev.♻️ Sketch of a shared hook
// src/lib/use-model-context-auto-detect.ts import { useEffect, useState } from "react"; import { contextLimitForModel } from "`@/lib/models-dev`"; export type AutoDetectState = "idle" | "loading" | "detected" | "not_found"; export function useModelContextAutoDetect( modelId: string | undefined, enabled: boolean, contextAuto: boolean, currentContextLimit: number | undefined, applyDetected: (n: number) => void, ) { const [detectedContext, setDetectedContext] = useState<number | null>(null); const [autoState, setAutoState] = useState<AutoDetectState>("idle"); useEffect(() => { if (!enabled) return; let cancelled = false; const id = (modelId ?? "").trim(); if (!id) { setDetectedContext(null); setAutoState("idle"); return; } setAutoState("loading"); contextLimitForModel(id, 0).then((n) => { if (cancelled) return; const found = n > 0 ? n : null; setDetectedContext(found); setAutoState(found ? "detected" : "not_found"); if (found && contextAuto && currentContextLimit !== found) applyDetected(found); }); return () => { cancelled = true; }; // eslint-disable-next-line react-hooks/exhaustive-deps }, [modelId, enabled, contextAuto]); return { detectedContext, autoState }; }🤖 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 `@src/components/settings/AISettings.tsx` around lines 45 - 76, Extract the duplicated model context detection into a shared hook, such as useModelContextAutoDetect, centralizing the AutoDetectState type and cancellation-safe detection/auto-apply behavior. In src/components/settings/AISettings.tsx lines 45-76 and src/components/settings/AgentSettings.tsx lines 435-463, replace each local effect and detection state with the hook, preserving cloud-only enablement and each component’s apply callback. In both files, replace the hardcoded 128000 fallbacks at the specified locations with the imported DEFAULT_CONTEXT_LIMIT from `@/lib/models-dev`.electron/lib/community-registry.ts (1)
86-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMigrate deprecated Zod v4 schemas.
z.string().url()and.passthrough()are deprecated in Zod 4. Move the URL checks toz.string().url()->z.url()where appropriate, and replace.passthrough()here withz.looseObject(...)or one of.loose()to avoid deprecation warnings/future breakage.🤖 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 `@electron/lib/community-registry.ts` around lines 86 - 104, The schemas use deprecated Zod v4 APIs. Update URL validations in the visible registry schemas, including baseUrl and apiUrl, to use z.url() while preserving the existing HTTPS restriction; replace the related .passthrough() usage in the surrounding schema definitions with z.looseObject(...) or the equivalent .loose() form.
🤖 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 `@electron/lib/community-registry.ts`:
- Around line 116-123: Update manifestSchema and parseManifest so invalid
individual mcpServers and services entries are filtered out instead of causing
whole-manifest parsing to fail. Use element-level validation with safe parsing,
preserve valid entries and manifest-level validation, and update the existing
malformed-entry test to assert the bad entry is dropped rather than rejected.
- Line 113: Update the community manifest schema’s homepage field to validate
only HTTP(S) URLs rather than arbitrary strings, preventing unsafe schemes from
reaching BrowseCommunityModal.tsx; apply the same URL-scheme validation to the
service definition’s apiKeyUrl field.
In `@electron/lib/mcp-oauth.ts`:
- Around line 353-368: Update cancelServerAuth so it records whether a loopback
listener was removed, captures the result of cancelPendingForServer(serverId),
and returns true when either cancellation succeeds. Preserve the existing
cleanup for both active loopbacks and pending deep-link attempts.
- Around line 245-248: Update the loopback registration flow around
activeLoopbacks to close and cancel any existing listener for the same server id
before replacing its map entry. Ensure the prior flow’s cleanup cannot remove or
invalidate the newly registered listener, while preserving cancellation behavior
for the current browser sign-in.
In `@src/lib/livePreview.ts`:
- Around line 104-125: Update findCalloutBlocks so it only processes Blockquote
nodes whose parent syntax node is not another Blockquote, skipping nested
callout blockquotes before parsing and emitting ranges. Preserve the existing
parseCalloutSource and decoration data behavior for eligible outer blockquotes.
---
Nitpick comments:
In `@electron/lib/community-registry.ts`:
- Around line 86-104: The schemas use deprecated Zod v4 APIs. Update URL
validations in the visible registry schemas, including baseUrl and apiUrl, to
use z.url() while preserving the existing HTTPS restriction; replace the related
.passthrough() usage in the surrounding schema definitions with
z.looseObject(...) or the equivalent .loose() form.
In `@src/components/settings/AISettings.tsx`:
- Around line 45-76: Extract the duplicated model context detection into a
shared hook, such as useModelContextAutoDetect, centralizing the AutoDetectState
type and cancellation-safe detection/auto-apply behavior. In
src/components/settings/AISettings.tsx lines 45-76 and
src/components/settings/AgentSettings.tsx lines 435-463, replace each local
effect and detection state with the hook, preserving cloud-only enablement and
each component’s apply callback. In both files, replace the hardcoded 128000
fallbacks at the specified locations with the imported DEFAULT_CONTEXT_LIMIT
from `@/lib/models-dev`.
In `@src/lib/livePreview.ts`:
- Around line 173-272: Optionally optimize buildDecorations by consolidating the
separate syntaxTree(...).iterate passes for hidden marks, blockquotes, links,
and list markers into one traversal using node-name branching. Compute
calloutWidgetLineSet once per rebuild and reuse it across the traversal and
scanInlinePatterns, preserving all existing filtering and decoration behavior.
In `@src/lib/models-dev.ts`:
- Around line 44-93: The fuzzy model-ID matching logic lacks regression
coverage. Add a focused, table-driven unit test for normalizeId and lookup that
verifies gateway/vendor and region prefix removal, date/version and variant
suffix stripping, and separator-swapping matches such as
playground-claude-opus-4-8, us.anthropic.claude-3-opus-20240229-v1:0, and
gpt-4.1/gpt-4-1.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 35ff6d6e-c085-444e-96ef-8e02b39e377d
📒 Files selected for processing (32)
changelogs/v2.5.6.mdelectron/ipc/community-registry-handlers.tselectron/ipc/tools.tselectron/lib/community-registry.test.tselectron/lib/community-registry.tselectron/lib/mcp-oauth.tselectron/lib/oauth-loopback.test.tselectron/lib/oauth-loopback.tselectron/lib/protocol.tselectron/main.tselectron/mcp/tools/link-note-task.test.tselectron/mcp/tools/tasks.tselectron/preload.tssrc/components/notes/markdown-editor.tsxsrc/components/notes/note-editor.tsxsrc/components/settings/AISettings.tsxsrc/components/settings/AgentSettings.tsxsrc/components/settings/ToolsSettings.tsxsrc/components/settings/settings-view.tsxsrc/components/settings/shared.tsxsrc/components/settings/tools/BrowseCommunityModal.tsxsrc/components/settings/tools/ConnectorLogo.tsxsrc/components/settings/tools/McpAuthButton.tsxsrc/lib/callout-widget.tsxsrc/lib/constants.tssrc/lib/livePreview.component.test.tsxsrc/lib/livePreview.tssrc/lib/models-dev.tssrc/store/index.tssrc/store/slices/tools.tssrc/store/slices/ui.tssrc/types/index.ts
- registry: validate homepage + apiKeyUrl as https URLs (they render as
anchor hrefs — block javascript:/data: from a compromised manifest)
- registry: drop malformed manifest entries individually instead of
failing the whole parse (an insecure/invalid entry no longer blanks the
catalog for everyone); tests updated to assert drop-not-throw
- mcp-oauth: startServerAuth now cancels a prior loopback listener (via
cancelServerAuth) before registering a new one, and the finally-cleanup
only deletes the map entry if it still points at its own listener — a
re-initiated sign-in can no longer be orphaned/uncancellable
- mcp-oauth: cancelServerAuth returns true when a deep-link pending attempt
was cancelled (not just a loopback), so tools:cancelMcpAuth reports
{ cancelled: true } accurately
- livePreview: findCalloutBlocks skips a blockquote nested inside another
blockquote, preventing overlapping block-replace decoration ranges that
throw during measurement
CI runs eslint with --max-warnings 0. These three effects reset state synchronously on purpose (model-id/context detection reset, community catalog load), so add targeted eslint-disable-line comments matching the existing convention in the file.
Inline callout block widgets cause cursor drift (cursor lands lines below the click) and unreliable click-to-enter, due to CodeMirror measuring the React-mounted widget at ~0 height before it paints. Attempted fixes (rAF requestMeasure, estimatedHeight=-1, ignoreEvent false) did not fully resolve it in live testing. Gate behind CALLOUTS_ENABLED=false so callout blockquotes fall back to ordinary Tier 1 blockquote rendering. The widget implementation stays intact for a future fix. Tracked on the Cairn board. - livePreview.ts: CALLOUTS_ENABLED flag; calloutField/theme only added when enabled; calloutWidgetLineSet returns empty when disabled - callout-widget.tsx: keep the attempted measurement fixes for later - test: replace widget-behaviour tests with a disabled-state assertion; keep parseCalloutSource unit tests - changelog: drop the callout entry (not shipping)
Deleting a note on desktop physically removed the row (DELETE FROM notes) and relied on the sync engine reconstructing a throwaway tombstone shell so peers wouldn't resurrect it. Combined with a stale device's oplog this could let a deleted note reappear or a freshly-created note vanish. deleteNote now soft-deletes: UPDATE notes SET deleted_at = COALESCE(...) so the row survives as a durable tombstone, matching mobile. The AFTER UPDATE capture trigger already stages a 'delete' op (keys off NEW.deleted_at), so peers tombstone via the normal drainPending delete branch — the shell hack is no longer needed for the desktop-delete path. - getNoteById now filters deleted_at IS NULL so MCP append/patch/tag and the disk projector can't resurrect a tombstoned note; added getNoteByIdIncludingTombstoned for projectNoteToDisk, which must still see the tombstone to remove the orphan .md file. - toNote mapper exposes deletedAt. - Live list/search reads already filtered deleted_at IS NULL (the old comment claiming otherwise was stale), so no ghost rows leak. Groundwork for delete-wins reconciliation (docs/plans/sync-lifecycle-hardening.md). All 1298 tests pass; type-check:all + compile clean.
…lyphs Connector logos now come from the compiled cairn-community manifest as inline iconSvg (allowlist-sanitized upstream at CI build time), rendered inline and tinted via brandColor. Deletes the 17 hardcoded brand-glyph cases — new connectors get their logo from the registry without an app release. Falls back to a generic glyph (MCP mark for MCP servers, plug for HTTP services) when iconSvg is absent. Defense-in-depth: looksSafeSvg() re-checks the markup in the renderer before inlining, so a compromised manifest still can't inject script. - iconSvg?: string added to registry entry types (src/types, electron lib, preload); dropped the stale logo id field - BrowseCommunityModal passes iconSvg + kind - registry test fixture updated to the compiled entry shape (id + iconSvg) type-check:all + lint + compile clean; 33 registry/oauth tests pass.
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)
electron/lib/community-registry.test.ts (1)
77-84: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRestore the stubbed global
fetch.
vi.restoreAllMocks()restores spies but does not undovi.stubGlobal("fetch", ...). Addvi.unstubAllGlobals()to the teardown, or enabletest.unstubGlobalsinvitest.config.ts, sofetchdoes not stay stubbed between tests.🤖 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 `@electron/lib/community-registry.test.ts` around lines 77 - 84, Update the test teardown in afterEach to call vi.unstubAllGlobals() alongside vi.restoreAllMocks(), ensuring any fetch stub created by vi.stubGlobal is removed between tests. Keep the existing temporary-directory cleanup unchanged.
🧹 Nitpick comments (3)
electron/lib/community-registry.test.ts (3)
153-164: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the cached manifest on the 304 path.
Verify that
res.manifestcontains the previously cached entries and thatres.erroris undefined;fromCache: truealone does not prove the cached payload was served.🤖 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 `@electron/lib/community-registry.test.ts` around lines 153 - 164, Extend the 304 Not Modified test for fetchManifest to assert that res.manifest contains the previously cached VALID entries and that res.error is undefined, while preserving the existing fromCache and conditional If-None-Match assertions.
175-181: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSeed the cache before testing
refreshManifest().Because this test starts with an empty cache, a broken non-forced implementation would still call the network and pass. First populate the cache, then call
refreshManifest()and assert that it fetches again withfromCache: false.🤖 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 `@electron/lib/community-registry.test.ts` around lines 175 - 181, Update the “refreshManifest forces the network path” test to populate the manifest cache before invoking refreshManifest, using the existing cache population mechanism and valid manifest data. Then retain the fetch invocation and fromCache false assertions to verify refreshManifest bypasses the seeded cache and fetches again.
93-113: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a regression case for invalid SVG logos.
These tests cover URL and required-field validation, but not the new
iconSvgsecurity contract. Add a malformed or unsafe SVG entry and assert it is dropped while valid entries remain.🤖 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 `@electron/lib/community-registry.test.ts` around lines 93 - 113, Add a regression test alongside the existing parseManifest filtering tests that clones VALID, makes one service’s iconSvg malformed or unsafe, and verifies parseManifest removes that service while retaining the valid service. Assert the resulting services length and surviving valid service identity, matching the existing URL and malformed-entry test style.
🤖 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 `@docs/plans/sync-lifecycle-hardening.md`:
- Around line 103-110: Expand the durable-tombstone work plan to include desktop
and mobile schema migrations, ownership of the new engine table, and the
read/write adapters used by existing databases. Reference the migration and
adapter components that must create, access, and preserve sync_row_base so Phase
1 has a deployable persistence path.
- Around line 82-93: Define an explicit causal-evidence mechanism for the
delete-wins protocol in docs/plans/sync-lifecycle-hardening.md: specify the
acknowledgement frontier, version vector, or observation token carried by puts
and tombstones, and require reconcileOne to use it to prove a put observed the
delete before resurrecting. At docs/plans/sync-lifecycle-hardening.md lines
134-139, preserve existing HLC values and explicitly prohibit treating
updated_at-derived backfill stamps as authoritative causal evidence.
- Around line 166-171: Update the tombstone-table retention/GC policy to prevent
deletion when a peer’s acknowledged-HLC watermark is missing or no longer
trustworthy after reinstall or app-data loss. Treat newly discovered or
re-registered peers as unsafe until an install-independent identity is
validated, requiring full resync or retaining affected tombstones beyond the
grace period; document this protocol alongside the per-peer watermark and
wall-clock checks.
In `@electron/db/queries.ts`:
- Line 292: Make the deletion update idempotent by restricting the query in
electron/db/queries.ts:292-292 to rows where deleted_at IS NULL, so retries do
not change updated_at or version; update the related test in
electron/db/queries.test.ts:699-705 to verify the second deletion preserves
deleted_at, updated_at, and version.
In `@electron/lib/mcp-oauth.ts`:
- Around line 439-450: Update the server-lifecycle handlers in
electron/ipc/tools.ts that currently call cancelPendingForServer during MCP
server save, deletion, or sign-out to call cancelServerAuth instead, ensuring
both pending deep-link attempts and activeLoopbacks are cancelled. Leave
cancelPendingForServer unchanged for callers that only need to remove deep-link
entries.
- Around line 290-293: Update startServerAuth around cancelServerAuth to guard
concurrent starts per server with a generation token or lock established before
any asynchronous setup. Ensure a newer invocation invalidates or serializes
older attempts, preventing both calls from registering listeners and leaving an
orphaned loopback listener; preserve cancellation of already-registered loopback
and deep-link flows.
In `@src/components/settings/tools/ConnectorLogo.tsx`:
- Around line 63-74: Update the SVG opening-tag replacement in ConnectorLogo’s
icon rendering branch to match every opening form accepted by looksSafeSvg,
including whitespace, tabs, newlines, and no attributes, so width="100%" and
height="100%" are always injected while preserving the existing safety guard.
- Around line 37-45: Update looksSafeSvg’s inline event-handler validation so
handlers are rejected regardless of whether they follow whitespace, a quoted
attribute value, or another attribute delimiter; replace the current
whitespace-dependent pattern with a boundary-aware check such as the suggested
event-attribute detection, while preserving the existing SVG safety checks.
---
Outside diff comments:
In `@electron/lib/community-registry.test.ts`:
- Around line 77-84: Update the test teardown in afterEach to call
vi.unstubAllGlobals() alongside vi.restoreAllMocks(), ensuring any fetch stub
created by vi.stubGlobal is removed between tests. Keep the existing
temporary-directory cleanup unchanged.
---
Nitpick comments:
In `@electron/lib/community-registry.test.ts`:
- Around line 153-164: Extend the 304 Not Modified test for fetchManifest to
assert that res.manifest contains the previously cached VALID entries and that
res.error is undefined, while preserving the existing fromCache and conditional
If-None-Match assertions.
- Around line 175-181: Update the “refreshManifest forces the network path” test
to populate the manifest cache before invoking refreshManifest, using the
existing cache population mechanism and valid manifest data. Then retain the
fetch invocation and fromCache false assertions to verify refreshManifest
bypasses the seeded cache and fetches again.
- Around line 93-113: Add a regression test alongside the existing parseManifest
filtering tests that clones VALID, makes one service’s iconSvg malformed or
unsafe, and verifies parseManifest removes that service while retaining the
valid service. Assert the resulting services length and surviving valid service
identity, matching the existing URL and malformed-entry test style.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e965d915-8db3-4e9d-9e41-759b3f9e7429
📒 Files selected for processing (19)
changelogs/v2.5.6.mddocs/plans/sync-lifecycle-hardening.mdelectron/db/queries.test.tselectron/db/queries.tselectron/lib/community-registry.test.tselectron/lib/community-registry.tselectron/lib/mcp-oauth.tselectron/main.tselectron/preload.tselectron/shared/db-mappers.tsshared/sync/engine.test.tssrc/components/settings/AISettings.tsxsrc/components/settings/AgentSettings.tsxsrc/components/settings/tools/BrowseCommunityModal.tsxsrc/components/settings/tools/ConnectorLogo.tsxsrc/lib/callout-widget.tsxsrc/lib/livePreview.component.test.tsxsrc/lib/livePreview.tssrc/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/components/settings/AgentSettings.tsx
- src/types/index.ts
- src/components/settings/AISettings.tsx
- electron/preload.ts
- electron/lib/community-registry.ts
- src/components/settings/tools/BrowseCommunityModal.tsx
- changelogs/v2.5.6.md
… guard) - deleteNote: guard UPDATE with `deleted_at IS NULL` so retries preserve deleted_at/updated_at/version; test asserts all three - tools.ts: MCP save/delete/sign-out now cancelServerAuth (loopback + deep-link) instead of cancelPendingForServer (deep-link only) - mcp-oauth: per-server generation token guards the concurrent-start race so a superseded attempt closes its listener instead of orphaning it - ConnectorLogo: inject width/height for every <svg> form looksSafeSvg accepts; boundary-aware inline-event-handler rejection - community-registry.test: unstubAllGlobals in teardown; stronger 304 + refreshManifest cache-bypass assertions - sync-lifecycle-hardening plan: expand tombstone migrations/adapters, causal-evidence mechanism, backfill causality, and stale-peer GC policy
The Configured Servers rows used a generic Server/Globe icon while Browse Community showed real brand logos. ToolsSettings now fetches the registry once (cache-first) and maps communityId (== definition name) -> iconSvg + brandColor, rendering ConnectorLogo for community-installed MCP servers and services. Manually-added tools (no registry match) keep the lucide fallback; offline keeps the fallback too. type-check + lint clean.
19 connectors produced 60 tag chips (50 singletons) — the chip row was longer than the results. Chips are now the connector's category (fixed 9-item vocabulary from the registry); freeform tags still feed the search box. Adds category to registry entry types (electron lib + zod, preload, src/types). type-check:all + lint + compile clean; 9 registry tests pass.
This was referenced Jul 24, 2026
This was referenced Aug 1, 2026
15 of 18 tasks
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.
What does this PR do?
Ships everything in the v2.5.6 changelog: the Live Preview note editor, AI/Chat settings additions (temperature + models.dev context detection), the community tool registry (fetch/cache + Browse & 1-click install, backed by the new cairn-community repo), plus fixes for a note-deletion race and MCP OAuth sign-in cancellation.
Type of change
Summary of changes (from
changelogs/v2.5.6.md)Added
==highlights==,[[wikilinks]](accent chips) and> blockquotes, and renders callouts (> [!note], …) inline.electron/lib/community-registry.ts) for an ETag-cached, offline-resilientmanifest.json, Zod-validated.Changed
Fixed
https:(was blocking models.dev lookups).link_note_to_task/unlink_note_from_tasknow write the.mdfile inside themcp_active_writeslock, so the separate-process file-watcher no longer misreads the relocation as a delete and purges the row. Regression test added.Screenshots / recording
Checklist
npm run type-check:allpassesnpm run lintpasses (0 errors; 3 pre-existingset-state-in-effectwarnings consistent with existing Settings panels)npm testpasses — targeted suites green (registry, oauth-loopback, link-note-task: 21 tests); fullnpm testnot run in this environmentnpm run test:e2e— not run in this environmenttext-[Npx]pixel font classes — rem equivalents onlyhandle()and returnIpcResult<T>(registry:fetch/refresh,tools:cancelMcpAuth)saveMcpServer/saveCustomService)dependencies(zod already present)--external:compile-flag changesNotes for reviewer
manifest.json+ JSON Schema + CI validator). Cairn fetches it at runtime; 15 MCP connectors + 3 HTTP services seeded, all endpoints verified live.link_note_to_taskfix requires an MCP-server restart to take effect for the standalone binary.Summary by CodeRabbit