Feature/ai agent second brain longterm memory - #361
Conversation
- add second brain settings and shared backend contracts - bootstrap user-owned Markdown memory with memory.md - implement safe paths, atomic writes, hashes, and conflicts - add revisions, archive, restore, and state recovery - enforce page and total storage budgets - add filesystem security and settings migration tests
…tion - add scoped Second Brain context injection across agent sessions - implement bounded wiki search, page reads, backlinks, and context expansion - integrate Second Brain tools into SQL, project, notebook, and analytics agents - enforce conversation scope, authorization, and runtime isolation - consolidate scope, search, and context handling in SecondBrainRuntimeService - add context usage reporting and comprehensive integration tests
- collect bounded session, Analytics, and dbt project evidence - redact credentials, sensitive values, and absolute user paths - add deterministic source hashes and incremental refresh cursors - generate and validate structured memory operations using the configured model - support dry runs, cancellation, provenance, and revision-backed updates - skip model calls and filesystem writes when source evidence is unchanged - add atomic refresh-state commits and focused backend tests
- add typed IPC contracts for page, revision, archive, and refresh operations - enforce page-ID validation, operation ownership, cancellation, and safe folder access - add renderer service and React Query controller with progress subscriptions - add the Second Brain settings workspace with searchable page navigation - support Monaco Markdown edit, preview, and split modes - preserve drafts during conflicts and save with expected revision hashes - add archive, restore, revision history, initialization, and refresh controls - add backend, IPC, and renderer service tests
- harden page validation against encoded paths, Unicode controls, symlinks, and hard links - expand secret detection and exclude sensitive refresh sources - enforce disabled-state, cancellation, and renderer ownership protections - add bounded search, prompt, storage, refresh, and performance regression gates - normalize wiki references and expose clear scoped creation targets - distinguish existing scoped pages from suggested pages that are not yet created - improve progressive discovery and cross-session context usage reporting - fix Monaco saved-draft synchronization and false unsaved warnings - finalize failed tool calls and persist terminal error states - display tool arguments and structured error results correctly - add security, lifecycle, performance, context usage, and UI regression tests
- store long-term memory as a portable OKF Markdown wiki bundle - add required concept metadata and generated read-only indexes - keep memory.md as the progressive-discovery entry point - separate portable knowledge from internal state, revisions, sources, and logs - update retrieval, refresh, validation, and agent tools for OKF concepts - expose one secure wiki-folder flow across backend, IPC, controller, and UI - remove legacy layout compatibility for the unmerged feature - add OKF storage, security, IPC, and renderer test coverage
…cs, and UI - add local OKF v0.1 Markdown memory storage with deterministic indexes - add progressive memory retrieval and agent-facing wiki tools - collect bounded evidence from chats, Analytics, notebooks, dbt projects, application metadata, connections, local Git status, and existing concepts - add refresh preview, apply, cancellation, no-change detection, and partial failure recovery with safe cursor handling - add versioned source provenance manifests without storing raw source content - add bounded privacy-safe JSONL diagnostic logs with rotation and retention - add support-data status, clear, export preview, and path-free export IPC - ensure support-data failures never fail or roll back memory refreshes - protect support storage against malformed files, symlinks, and hard links - add Monaco Markdown editing with sanitized preview and revision support - add archive, restore, search, folder-opening, and OS terminal actions - rename the AI Settings tab from Wiki Memory to Agent Memory - update consent, progress, context usage, and tool-result UI - add regression tests for storage, refresh, IPC, provenance, diagnostics, privacy, retention, recovery, renderer services, and Markdown sanitization - update Plan 51 documentation with the implemented architecture and remaining manual release gates
…an 51g) - feat: `ChatWindow.tsx` automatically reads `<dbt-project>/agent.md` on the Project screen and injects its contents into every chat request as `projectAiContext`, giving the Project Agent project-specific rules and context. - feat: a lightweight inline banner prompts the user to generate `agent.md` if it is missing. The banner delegates directly to the Project Agent via a chat prompt; the agent uses its native `writeFile` tool to create the file. The banner is permanently dismissible per-project via localStorage. - feat: the Project Agent's system prompt now dynamically injects the active database connection, session context, and `agent.md` content in a strict, ordered block. - feat: the Project Agent gains access to `studio_sql_schema_extract` to understand the backing database schema in addition to dbt project files. - feat: added `EnrichedConnectionMeta` in `agentTypes.ts` and implemented `AgentService.resolveEnrichedConnectionMeta()` to resolve `database`, `schema`, and `linkedDbtProject` for the active connection. - feat: SQL, Notebooks, and Analytics agents now inject `Database`, `Schema`, and `Linked dbt Project` metadata into their system prompts. - feat: SQL, Notebooks, and Analytics agents safely inject pure NodeJS read-only filesystem tools (`readFile`, `listDirectory`, `readDbtModel`, etc.) when a dbt project is linked, without relying on UI bridges. - feat: connection dropdown in SQL and Notebooks screens now shows a project-link icon for any connection associated with a dbt project, colored green when it matches the currently selected project and grey otherwise. - fix: stripped destructive and IDE-bridge tools from non-project agents to prevent cross-boundary IPC errors while preserving all native UI tools.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a filesystem-backed Second Brain with scoped runtime context, refresh orchestration, agent tools, IPC handlers, renderer controllers, settings UI, consent flow, context accounting, and expanded tests. ChangesSecond Brain storage and refresh
Agent integration
Renderer experience
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
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 (1)
src/main/services/ai/agents/notebooksAgent.ts (1)
227-231: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
safeEnabledToolsis now dead — the Monaco-bridge crash guard is bypassed.The gating loop reads
enabledTools(Lines 284 and 286), notsafeEnabledTools, so the deletions ofstudio_ducklake_query/studio_sql_queryat Lines 230-231 have no effect. Those tools are back in the Notebooks toolset, which the comment above them says will crash/hang because the Notebooks screen has no SQL Editor bridge.🐛 Proposed fix
- Object.entries(allAvailableTools).forEach(([name, toolDef]) => { - const isUI = name in studioNotebookTools; - const isAllowedProjectTool = enabledTools && enabledTools[name]; - - if ((isUI && enabledTools?.[name] !== false) || isAllowedProjectTool) { + Object.entries(allAvailableTools).forEach(([name, toolDef]) => { + const isUI = name in studioNotebookTools; + const isAllowedProjectTool = Boolean(safeEnabledTools[name]); + + if ( + (isUI && name in safeEnabledTools && safeEnabledTools[name] !== false) || + isAllowedProjectTool + ) {If UI tools should stay default-on when absent from the map, keep the
!== falseform but gate onsafeEnabledToolsand explicitly exclude the two removed names.Also applies to: 282-293
🤖 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/main/services/ai/agents/notebooksAgent.ts` around lines 227 - 231, Update the tool-gating loop to read from safeEnabledTools instead of enabledTools, preserving the existing default-on behavior for tools absent from the map. Ensure studio_ducklake_query and studio_sql_query remain excluded from the Notebooks toolset after their deletion.
🟡 Minor comments (11)
src/main/ipcHandlers/secondBrain.ipcHandlers.ts-42-50 (1)
42-50: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse the configured page size limit for
second-brain:write.
createService()passessettings.secondBrain.maxPageBytesintoSecondBrainService, butsecond-brain:writerejects Markdown content over128 * 1024beforewritePage()can enforce the same configured limit. Content accepted between the configured limit and this IPC cap would then fail asINVALID_CONTENT; content that is only rejected by the IPC cap would bypass the service’s consistent budget error path. Passsettings.secondBrain.maxPageBytesintorequireStringfor this handler.🤖 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/main/ipcHandlers/secondBrain.ipcHandlers.ts` around lines 42 - 50, Update the second-brain:write handler to pass settings.secondBrain.maxPageBytes as the max argument when calling requireString for Markdown content, replacing the hardcoded 128 * 1024 limit. Keep createService() and SecondBrainService aligned with this same configured page-size limit.src/renderer/components/settings/SecondBrainTab.tsx-744-841 (1)
744-841: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winArchive/restore failures are silent unhandled rejections.
handleSave,handleRefresh, and the support-data handlers all wrapmutateAsyncin try/catch, but these three inline handlers (Lines 748-757, 769-781, 821-834) do not. A rejected write (conflict, permission, missing page) produces an unhandled promise rejection with no toast, and the optimisticsetSelected(...)right after simply never runs, so the UI silently stays stale.Wrap each in try/catch with
toast.error((error as Error).message), or passonErrorwhen constructing the mutations so all call sites are covered.🤖 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/renderer/components/settings/SecondBrainTab.tsx` around lines 744 - 841, Handle errors for the three inline archive/restore mutation handlers in the selected-page controls and revision restore flow by wrapping each await mutateAsync call and its subsequent state update in try/catch, reporting failures with toast.error((error as Error).message). Ensure rejected archive, archived-page restore, and historical-revision restore operations no longer become unhandled rejections while preserving successful state updates.src/renderer/components/chat/ToolCallFormatters.tsx-7-25 (1)
7-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
seenis never released, so repeated (non-circular) references render as[Circular].The
WeakSetaccumulates every visited object for the whole traversal instead of tracking only the current ancestor chain, so{ a: shared, b: shared }rendersbas[Circular]even though there is no cycle. Tool args with shared sub-objects (e.g. the same schema/config referenced twice) will display misleading output.Using the replacer's
this(the parent holder) to walk ancestors, or a stack-based approach, avoids the false positive:🐛 Ancestor-only cycle detection
const stringifyToolValue = (value: unknown): string => { try { - const seen = new WeakSet<object>(); + const ancestors: unknown[] = []; return JSON.stringify( value, - (_key, item) => { + function replacer(_key, item) { if (typeof item === 'bigint') return item.toString(); - if (typeof item === 'object' && item !== null) { - if (seen.has(item)) return '[Circular]'; - seen.add(item); - } + if (typeof item === 'object' && item !== null) { + while (ancestors.length > 0 && ancestors.at(-1) !== this) { + ancestors.pop(); + } + if (ancestors.includes(item)) return '[Circular]'; + ancestors.push(item); + } return item; }, 2, );🤖 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/renderer/components/chat/ToolCallFormatters.tsx` around lines 7 - 25, Update stringifyToolValue so cycle detection tracks only the current ancestor chain rather than every object visited during serialization. Replace the WeakSet-based seen tracking with replacer-parent or stack-based ancestor checks, preserving BigInt stringification, circular markers, formatting, and the existing error fallback; repeated non-circular references such as shared sibling objects must serialize normally.src/renderer/components/settings/SecondBrainTab.tsx-149-158 (1)
149-158: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winResolve the existing Monaco model instead of re-creating and unconditionally disposing it.
monacoNs.editor.createModel(...)throws when a model already exists forinmemory://second-brain/active-page.md, and this component disposes that memoized model on unmount. UsemonacoNs.editor.getModel(...)first, then only dispose the model for any editor instance owned by this mount.🤖 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/renderer/components/settings/SecondBrainTab.tsx` around lines 149 - 158, Update the model initialization in SecondBrainTab to resolve the existing Monaco model via editor.getModel for the active-page URI before creating one, avoiding duplicate-model errors. Track whether this mount created the model, and in the cleanup effect dispose it only when owned by this component; preserve shared models otherwise.src/renderer/components/editor/markdownPreview/index.tsx-6-6 (1)
6-6: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAllow alert classes after sanitization.
> [!NOTE]alerts render throughrehypeRaw, and the default GitHub schema won’t preserve.markdown-alerton theblockquotethey wrap, so the styles at line 156 can be stripped. Add a small custom schema merging the default, whitelisting the alert classes onblockquoteafter sanitization.🤖 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/renderer/components/editor/markdownPreview/index.tsx` at line 6, Update the sanitization setup in the markdown preview component to create a custom schema by merging the default GitHub schema and whitelist the required markdown-alert classes on blockquote elements. Pass this schema to rehypeSanitize after rehypeRaw so alert styling classes are preserved.src/renderer/components/chat/ChatWindow.tsx-208-212 (1)
208-212: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate dismissal state immediately.
Writing to
localStoragedoes not re-renderChatWindow, so “No thanks” leaves the banner visible until another state change occurs. Track the dismissed project key in React state when handling the click.Also applies to: 1136-1143
🤖 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/renderer/components/chat/ChatWindow.tsx` around lines 208 - 212, Update the ChatWindow dismissal flow around handleDismissAgentMd to track the dismissed project key in React state immediately when the handler runs, while preserving the existing localStorage write. Use that state when rendering the banner so “No thanks” hides it without requiring another state change, including the corresponding dismissal logic at the other referenced location.src/main/services/ai/secondBrain/secondBrain.service.ts-691-723 (1)
691-723: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRestore paths reinstate page bytes without enforcing
maxTotalBytes.writePageguards every growth withassertTotalBudget(newBytes - previousBytes), but both restore paths add content back into the active wiki with no equivalent check, so the configured total ceiling can be exceeded. Archived pages are also excluded fromcalculateManagedPageBytes(it only walkswikiRoot), which makes the archive→active direction the easiest way to overshoot.
src/main/services/ai/secondBrain/secondBrain.service.ts#L691-L723: inrestoreArchivedPage, callawait this.assertTotalBudget(archived.sizeBytes)before the rename, and runSecondBrainService.assertValidOkfConcept(pageId, archived.content)so a hand-edited archive file can't be reinstated with invalid frontmatter.src/main/services/ai/secondBrain/secondBrain.service.ts#L781-L825: inrestoreRevision, callawait this.assertTotalBudget(Buffer.byteLength(normalized, 'utf8') - Buffer.byteLength(currentPage.content, 'utf8'))alongside the existingassertPageBudget.🤖 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/main/services/ai/secondBrain/secondBrain.service.ts` around lines 691 - 723, Update restoreArchivedPage in src/main/services/ai/secondBrain/secondBrain.service.ts (lines 691-723) to validate archived content with SecondBrainService.assertValidOkfConcept and call assertTotalBudget(archived.sizeBytes) before renaming the archive. Update restoreRevision in the same file (lines 781-825) to call assertTotalBudget with the UTF-8 byte-size delta between normalized and current page content alongside assertPageBudget.tests/unit/main/services/ai/secondBrain/secondBrainProgressive.service.test.ts-282-291 (1)
282-291: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReconcile
INBOUND_LINKSwith the shared error-code type.
wiki_archivereturnserror.code: 'INBOUND_LINKS', butSecondBrainErrorCodeonly includes the enumerated Second Brain error codes, such as'INVALID_CONTENT'; this string does not belong to that union, so tool-layer or renderer error handling should not treat it as a shared Second Brain error code unlessINBOUND_LINKSis added to the shared type.🤖 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 `@tests/unit/main/services/ai/secondBrain/secondBrainProgressive.service.test.ts` around lines 282 - 291, Reconcile the wiki_archive assertion with the shared SecondBrainErrorCode used by tool-layer and renderer error handling: either add INBOUND_LINKS to that shared error-code union if it is an intended Second Brain code, or represent the archive-specific error without typing it as SecondBrainErrorCode. Update the test and related handling consistently while preserving the existing inboundLinks payload.src/main/services/ai/tools/studio/secondBrain.tools.ts-144-152 (1)
144-152: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winUnknown error messages can leak absolute filesystem paths into model context.
Non-
SecondBrainErrorfailures here are almost always Node fs errors, whose messages embed absolute paths (ENOENT ... open '/Users/<name>/...'). That string is returned to the model and surfaced in chat, whileredactSecondBrainEvidencein the refresh pipeline deliberately scrubs home paths. Return a fixed message for unclassified errors and log the detail in the main process instead.🛡️ Proposed fix
return { ok: false as const, error: { code: 'UNKNOWN', - message: - error instanceof Error ? error.message : 'Wiki Memory tool failed.', + message: 'Wiki Memory tool failed.', }, };🤖 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/main/services/ai/tools/studio/secondBrain.tools.ts` around lines 144 - 152, Update the unknown-error handling in the surrounding tool function to stop returning raw Error messages: preserve the existing message for SecondBrainError instances, but return a fixed generic message for all other failures. Log the original error details through the main process logging path so diagnostics remain available without exposing filesystem paths to the model.src/main/services/ai/agents/projectAgent.ts-76-76 (1)
76-76: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
projectPathis optional, so these paths can render asundefined/agent.md.Both branches already guard the "Active dbt Project" section on
projectPath, but theagent.mdchecklist interpolates it unconditionally.🐛 Proposed fix
-1. Use \`pathExists\` to check if \`agent.md\` exists at \`${projectPath}/agent.md\`. +1. Use \`pathExists\` to check if \`agent.md\` exists at the project root${projectPath ? ` (\`${projectPath}/agent.md\`)` : ''}.Apply the same shape to the Code-mode copy on Line 108.
Also applies to: 108-108
🤖 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/main/services/ai/agents/projectAgent.ts` at line 76, Guard both `agent.md` checklist path checks in the project agent, including the Code-mode copy, with the existing `projectPath` availability condition before invoking `pathExists` or interpolating the path. Preserve the checklist behavior when `projectPath` is defined and avoid generating `undefined/agent.md` when it is absent.src/main/services/agent.service.ts-1735-1755 (1)
1735-1755: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNon-streaming path never marks failed tool results as errors.
The streaming branch derives
status/errorfromgetToolFailureMessage, but herestatusis hardcoded to'done'. WithstreamResponsesdisabled, a tool returning{ ok: false, error: ... }is persisted ascompletedwitherrorMessage: null, so history and the UI show it as a success.🐛 Proposed fix
const persisted = sanitizeWikiToolCallForPersistence( tr.toolName, (tr as any).input ?? (tr as any).args, (tr as any).output ?? (tr as any).result, ); + const failureMessage = getToolFailureMessage(persisted.output); collectedToolCalls.push({ toolName: tr.toolName, toolCallId: tr.toolCallId, input: persisted.input, output: persisted.output, stepNumber: idx, - status: 'done', + status: failureMessage ? 'error' : 'done', + error: failureMessage, }); collectedParts.push({ type: 'tool-call', toolCallId: tr.toolCallId, toolName: tr.toolName, args: persisted.input, result: persisted.output, - status: 'done', + error: failureMessage, + status: failureMessage ? 'error' : 'done', });🤖 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/main/services/agent.service.ts` around lines 1735 - 1755, Update the non-streaming tool-result handling around sanitizeWikiToolCallForPersistence to derive failure state using getToolFailureMessage, matching the streaming branch. Set the collected tool call and collected part status to error and include the failure message when the tool result is unsuccessful; otherwise preserve the existing done status and output behavior.
🧹 Nitpick comments (18)
tests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.ts (1)
162-327: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing wiring tests for write/archive/restore handlers.
Coverage is strong for read/status/progress/disabled paths, but
second-brain:write,second-brain:archive, andsecond-brain:restore(which forwardexpectedHash/actorfor optimistic-concurrency control) have no wiring tests here.🤖 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 `@tests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.ts` around lines 162 - 327, Add wiring tests alongside the existing second-brain IPC handler tests for second-brain:write, second-brain:archive, and second-brain:restore. Invoke each handler with representative page or archive data plus expectedHash and actor, then assert the corresponding service mock is called once with those values forwarded unchanged and verify the returned result as appropriate.src/renderer/controllers/secondBrain.controller.ts (1)
39-44: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider debouncing the search key.
SecondBrainTabfeeds rawTextFieldstate into this hook, so every keystroke creates a new query key and fires asecond-brain:searchIPC round-trip that hits the filesystem index.keepPreviousDatahides the flicker but not the load. A debounced value (oruseDeferredValue) would cut this significantly.🤖 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/renderer/controllers/secondBrain.controller.ts` around lines 39 - 44, Debounce the raw query used by useSecondBrainSearch before constructing SECOND_BRAIN_KEYS.search and invoking secondBrainService.searchPages, while preserving the existing enabled check and keepPreviousData behavior. Ensure rapid TextField updates produce only the settled search request rather than one IPC round-trip per keystroke.tests/unit/renderer/components/ContextUsageRing.test.tsx (1)
4-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider covering the band boundary and non-finite input.
The cases skip the switchover between the one-decimal band and whole percentages (e.g.
9.95/10) and any guard behavior forNaN, negatives, or values above 100 — exactly where a formatter like this tends to regress.🤖 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 `@tests/unit/renderer/components/ContextUsageRing.test.tsx` around lines 4 - 14, Extend the formatContextPercentage tests to cover the precision boundary around 10%, including values such as 9.95 and 10, and verify guard behavior for NaN, negative values, and values above 100. Assert the intended formatted output for each edge case while preserving the existing precision and whole-percentage expectations.tests/unit/renderer/components/MarkdownPreview.test.tsx (1)
29-31: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssertion doesn't pin sanitize between raw and highlight.
indexOf(sanitizePlugin) < length - 1also passes for[sanitize, raw, highlight], i.e. the insecure ordering where raw HTML is injected after sanitization. Capture the raw/highlight mocks the same way assanitizePluginand assert the full order.💚 Pin the plugin order
const reactMarkdown = jest.fn(({ children }) => <div>{children}</div>); +const rawPlugin = jest.fn(); +const highlightPlugin = jest.fn(); const sanitizePlugin = jest.fn(); @@ -jest.mock('rehype-raw', () => jest.fn()); -jest.mock('rehype-highlight', () => jest.fn()); +jest.mock('rehype-raw', () => rawPlugin); +jest.mock('rehype-highlight', () => highlightPlugin); jest.mock('rehype-sanitize', () => sanitizePlugin);const { rehypePlugins } = reactMarkdown.mock.calls[0][0]; - expect(rehypePlugins.indexOf(sanitizePlugin)).toBeLessThan( - rehypePlugins.length - 1, - ); + expect(rehypePlugins).toEqual([rawPlugin, sanitizePlugin, highlightPlugin]);🤖 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 `@tests/unit/renderer/components/MarkdownPreview.test.tsx` around lines 29 - 31, Update the plugin-order assertion in the MarkdownPreview test to capture the raw and highlight plugin mocks alongside sanitizePlugin, then assert their exact relative order: raw before highlight and sanitize between them. Replace the length-based check so the test rejects sanitize-first ordering.tests/unit/main/services/ai/secondBrain/secondBrainPerformance.service.test.ts (2)
32-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHeavy
beforeEachruns under Jest's default 5 s timeout.40 sequential
writePagecalls each re-walk the wiki for the total-byte budget and regenerate everyindex.md, so setup cost grows with each iteration.secondBrainProgressive.service.test.tsraises the limit viajest.setTimeout(30_000); this file doesn't, which makes it the most likely file to flake on loaded CI.💚 Proposed fix
import type { SecondBrainScope, SecondBrainSettings, } from '../../../../../../src/types/backend'; + +jest.setTimeout(30_000);🤖 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 `@tests/unit/main/services/ai/secondBrain/secondBrainPerformance.service.test.ts` around lines 32 - 47, Increase the Jest timeout for this performance test file, following the existing 30-second timeout used by secondBrainProgressive.service.test.ts, so the heavy beforeEach setup containing repeated service.writePage calls can complete reliably under loaded CI.
86-96: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThese wall-clock gates are near-tautological and the "p95" is actually the max.
Math.ceil(5 * 0.95) - 1indexes the largest of five samples, and 2 s for a single 300-byte file read / 5 s for a 40-page search will only catch catastrophic regressions while still being timing-sensitive on shared CI. The genuinely useful assertions here are the bounded-result ones (Lines 83-84, 97). Consider keeping the bound assertions and dropping or widening the duration gates.🤖 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 `@tests/unit/main/services/ai/secondBrain/secondBrainPerformance.service.test.ts` around lines 86 - 96, Remove the wall-clock threshold assertions for readDuration, p95Search, and contextDuration from this performance test, while preserving the bounded-result assertions around them. Leave the duration measurements and search-duration calculation only if still needed by remaining assertions; otherwise remove the now-unused timing logic from the test.tests/unit/main/services/ai/secondBrain/secondBrain.service.test.ts (1)
325-354: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe 200-line assertion can't fail independently of the byte cap.
limitedServiceusesmaxPageBytes: 1024, and the 201-line body is ~1.6 KB, soassertPageBudgetrejects on size before reaching the line check — both paths throwBUDGET_EXCEEDED, so the line rule is effectively untested. Assert the error details (or use a service without the reduced byte cap) so the two rules are distinguishable.💚 Proposed tightening
- ).rejects.toMatchObject({ code: 'BUDGET_EXCEEDED' }); + ).rejects.toMatchObject({ + code: 'BUDGET_EXCEEDED', + details: { maxLines: 200 }, + });🤖 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 `@tests/unit/main/services/ai/secondBrain/secondBrain.service.test.ts` around lines 325 - 354, Update the “enforces page and memory.md line budgets” test to isolate the line-budget assertion from the byte-budget assertion: use a service configuration whose maxPageBytes permits the 201-line content, or assert the rejected error’s distinguishing details so the failure is confirmed to come from the line limit rather than the 1024-byte cap. Keep the existing large-page case covering the byte budget.src/main/services/ai/secondBrain/secondBrain.service.ts (2)
562-567: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winEvery page is read and hashed more times than necessary.
listPagesbuilds summaries by fully reading, parsing, and SHA-256 hashing each page, and callers that need the body then read the same files again. An internalreadAllPages()(returning fullSecondBrainPage[]) withlistPagesprojecting summaries from it would remove the duplication.
src/main/services/ai/secondBrain/secondBrain.service.ts#L562-L567: insearchManagedPages, drop the per-hitreadPage(summary.pageId)call and search over pages already materialized by the listing pass.src/main/services/ai/secondBrain/secondBrain.service.ts#L415-L432: havelistPagesproject from a sharedreadAllPages()helper, or servehashfromstate.pageHashesplusfs.statwhen only summaries are needed (getStatusnever needs bodies).🤖 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/main/services/ai/secondBrain/secondBrain.service.ts` around lines 562 - 567, Eliminate duplicate page reads by introducing or reusing a shared readAllPages() helper that materializes full SecondBrainPage values; update listPages to project summaries from it or use state.pageHashes with fs.stat for summary-only callers such as getStatus. In searchManagedPages, iterate over the already materialized pages and remove the per-hit readPage(summary.pageId) call. Apply these changes at src/main/services/ai/secondBrain/secondBrain.service.ts lines 562-567 and 415-432.
415-432: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
listPagesmaterializes every page to produce summaries.Each summary needs only
title/hash/sizeBytes/frontmatter, butreadPagereads and hashes the full file for every page, andgetStatus/listManagedPages/searchManagedPagesall call this. Consider a lighter summary path (stat + frontmatter-only parse) or caching hashes fromstate.pageHashes.🤖 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/main/services/ai/secondBrain/secondBrain.service.ts` around lines 415 - 432, Optimize listPages so it does not call the full-content readPage for every page; introduce or reuse a lightweight summary path that obtains metadata via stat, parses only frontmatter, and reuses state.pageHashes when available for hashes. Preserve the existing SecondBrainPageSummary fields and pageId sorting, and ensure getStatus, listManagedPages, and searchManagedPages continue using the optimized listPages behavior.tests/unit/main/services/ai/secondBrain/wikiMemorySupport.service.test.ts (1)
70-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVacuous assertion at Line 75.
'chat body'never appears in any input to this test, sonot.toContain('chat body')can never fail. If the intent is "no raw chat content is persisted", assert against a value actually fed intorecordSource; otherwise drop the line. Thenot.toContain('secret.md')check above it is the meaningful one.🤖 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 `@tests/unit/main/services/ai/secondBrain/wikiMemorySupport.service.test.ts` around lines 70 - 76, The assertion against “chat body” in the test should not be vacuous: either replace it with raw chat content that is actually passed to recordSource, or remove that assertion if no such input exists. Keep the meaningful “secret.md” persistence check unchanged.tests/unit/main/services/ai/secondBrain/secondBrainRefresh.service.test.ts (1)
53-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShared module-level
jest.fn()s accumulate state across tests.
emptyAdditionalSourcesis created once and spread into most suites, so call history and any futuremockResolvedValueOnceleak between tests. Build it per test (factory) or addjest.restoreAllMocks()/jest.clearAllMocks()inafterEach— the latter also cleans up the un-restoredjest.spyOn(secondBrain, 'writePage')at Line 286.♻️ Proposed refactor
-const emptyAdditionalSources = { - loadConnections: jest.fn(async () => []), - listNotebooks: jest.fn(async () => []), - collectGitStatus: jest.fn(async () => ({})), -}; +const createEmptyAdditionalSources = () => ({ + loadConnections: jest.fn(async () => []), + listNotebooks: jest.fn(async () => []), + collectGitStatus: jest.fn(async () => ({})), +});and in
afterEach:afterEach(async () => { + jest.restoreAllMocks(); await fs.remove(temporaryDirectory); });🤖 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 `@tests/unit/main/services/ai/secondBrain/secondBrainRefresh.service.test.ts` around lines 53 - 57, Make emptyAdditionalSources a per-test factory so each test receives fresh jest.fn() mocks and no call history or one-time mock values leak between tests. Also add appropriate afterEach cleanup for mocks, including restoring the secondBrain.writePage spy, while preserving the existing default mock behavior.tests/unit/main/services/ai/secondBrain/secondBrainRuntime.service.test.ts (1)
47-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the remaining
resolveScopefailure branches.Two guard paths in
resolveScopeare untested: a missing conversation (getConversationScope→null) and an unsupported storedscreenKey(normalizeStoredScreenKey). Both should reject withSCOPE_MISMATCHand are cheap to assert with the existing mock.🤖 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 `@tests/unit/main/services/ai/secondBrain/secondBrainRuntime.service.test.ts` around lines 47 - 55, Extend the resolveScope tests around the existing runtime scope conflict case to cover both remaining failure branches: mock getConversationScope to return null for a missing conversation, and provide an unsupported stored screenKey that exercises normalizeStoredScreenKey. Assert that each case rejects with code SCOPE_MISMATCH, reusing the existing mock setup.src/main/services/ai/secondBrain/secondBrainRefresh.service.ts (1)
1497-1497: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist
listPageIds()out of the validation loop.Validation performs no writes, so the page set is stable for the whole loop; calling
listPageIds()per operation costs up toOPERATION_LIMITfull listings.♻️ Proposed refactor
- const exists = (await this.secondBrain.listPageIds()).includes(pageId); + const exists = existingPageIds.has(pageId);Add before the loop:
const existingPageIds = new Set(await this.secondBrain.listPageIds());🤖 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/main/services/ai/secondBrain/secondBrainRefresh.service.ts` at line 1497, In the validation flow containing the `exists` check, call `this.secondBrain.listPageIds()` once before the validation loop and store the result in a Set named `existingPageIds`. Replace each per-operation listing and array `includes` check with membership lookup against that Set, preserving the existing validation behavior.src/main/services/ai/secondBrain/secondBrainRuntime.service.ts (1)
386-421: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winSearch reads up to 500 pages serially on every
wiki_searchcall.Each candidate page is fully read before the byte budget is evaluated, so a large wiki costs 500 sequential file reads per tool invocation (and
findInboundLinksrepeats the same pattern). Consider using thelistPages()summaries (size/hash) to pre-trim candidates againstMAX_SCANNED_BYTES, and reading the remainder with a bounded concurrency pool.🤖 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/main/services/ai/secondBrain/secondBrainRuntime.service.ts` around lines 386 - 421, Update the candidate-page scanning flow around the loop using secondBrain.readPage so it does not serially fully read every candidate before enforcing MAX_SCANNED_BYTES. Use listPages() summary metadata to pre-trim candidates by size/hash, then read the retained pages through a bounded-concurrency pool while preserving byte-budget accounting and result scoring; apply the same approach to findInboundLinks if it duplicates this pattern.src/main/services/agent.service.ts (2)
1314-1368: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated connection-meta resolution; also resolves against the unresolved
request.connectionId.
resolveEnrichedConnectionMetaalready does exactly what thescreenKey === 'project'block does (getConnectionById→ name/type/database/schema), plus the dbt-project link. Two divergent code paths for the same data will drift. Note also that Line 1315 passesrequest.connectionIdwhile the project branch uses the resolvedconnectionIdfrom Lines 1233-1239.♻️ Proposed consolidation
- // Resolve enriched connection meta - const connectionMeta = await AgentService.resolveEnrichedConnectionMeta( - request.connectionId, - ); + // Resolve enriched connection meta (uses the resolved connectionId) + const connectionMeta = + await AgentService.resolveEnrichedConnectionMeta(connectionId);Then derive the project block from the same object:
- let projectConnectionMeta: { - name?: string; - type?: string; - database?: string; - schema?: string; - } = {}; - - if (screenKey === 'project') { - try { - if (connectionId) { - ... - } else { - ... - } - } catch (err) { ... } - } + let projectConnectionMeta: { + name?: string; + type?: string; + database?: string; + schema?: string; + } = {}; + + if (screenKey === 'project') { + if (connectionId && connectionMeta.type !== 'unknown') { + projectConnectionMeta = { + name: connectionMeta.name, + type: connectionMeta.type, + database: connectionMeta.database, + schema: connectionMeta.schema, + }; + } else { + // keep the existing dbtConnection fallback for legacy projects + } + }🤖 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/main/services/agent.service.ts` around lines 1314 - 1368, Remove the duplicated connection lookup inside the screenKey === 'project' branch and derive projectConnectionMeta from the existing connectionMeta returned by resolveEnrichedConnectionMeta. Pass the already-resolved connectionId, rather than request.connectionId, to resolveEnrichedConnectionMeta, preserving the existing name/type/database/schema mapping and fallback behavior supplied by that resolver.
1132-1143: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
buildMCPToolset()anddiscoverSkills()now run twice per agent run.
buildFixedPromptContextperforms MCP toolset construction and skill discovery, andbuildBaseAgentConfig(src/main/services/ai/agents/baseAgentConfig.tsLines 58-60) repeats both a few lines later inrunAgent. That doubles MCP round-trips and skills-directory I/O on every message.♻️ Suggested direction
Return
mcpTools/skills/skillsPromptfrombuildFixedPromptContextand pass them intobuildBaseAgentConfig(as optional pre-resolved inputs) instead of recomputing.🤖 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/main/services/agent.service.ts` around lines 1132 - 1143, Update buildFixedPromptContext to return the resolved mcpTools, skills, and skillsPrompt alongside the existing context and token data, then pass these values into buildBaseAgentConfig as optional pre-resolved inputs from runAgent. Make buildBaseAgentConfig reuse supplied values and only call buildMCPToolset or discoverSkills when inputs are absent, ensuring each agent run performs MCP construction and skill discovery once.src/main/services/ai/agents/projectAgent.ts (1)
235-237: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
studio_sql_schema_extractis force-enabled twice, overriding both the registry flag and the user's tool toggles.
forceSchemaExtract: truealready bypassesstudio.sql.schema_extractin the registry, and Line 273 additionally admits the tool even when it is absent fromenabledTools. A user who disables it in AI settings has no way to turn it off for the project agent. Consider forcing only the registry flag and lettingenabledToolsremain authoritative.Also applies to: 273-273
🤖 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/main/services/ai/agents/projectAgent.ts` around lines 235 - 237, Update the createStudioSqlTools call in projectAgent so schema extraction only bypasses the registry flag; remove the additional unconditional admission of studio_sql_schema_extract from the enabledTools handling around line 273, allowing user tool toggles to remain authoritative.src/main/services/ai/agents/sqlAgent.ts (1)
242-290: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExtract the linked-dbt-project tool construction and enablement gating into one shared helper. All three screen agents now build
projectToolsfromconnectionMeta.linkedDbtProject?.pathand run the sameisUI/isAllowedProjectToolgate over{ ...uiTools, ...projectTools }. The copies have already diverged (onlyanalyticsAgent.tskeeps theTOOL_FLAGScheck;notebooksAgent.tsgates on the wrong map), and each new screen agent will repeat the mistake.
src/main/services/ai/agents/sqlAgent.ts#L242-L290: replace the inline block with a call to a shared helper (e.g.buildScreenAgentTools({ base, connectionMeta, uiTools: studioSqlTools, enabledTools, readOnlyTools: READ_ONLY_TOOLS, isAskMode })).src/main/services/ai/agents/analyticsAgent.ts#L257-L290: call the same helper withstudioAnalyticsTools, keeping theTOOL_FLAGScheck inside the helper so all agents apply it.src/main/services/ai/agents/notebooksAgent.ts#L282-L293: call the helper withstudioNotebookToolsand passsafeEnabledToolsso the Monaco-bridge exclusions survive the refactor.🤖 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/main/services/ai/agents/sqlAgent.ts` around lines 242 - 290, Extract the shared linked-dbt project tool construction and UI/project enablement gating into a reusable buildScreenAgentTools helper, including the TOOL_FLAGS check and Ask-mode handling. In src/main/services/ai/agents/sqlAgent.ts:242-290, replace the inline logic with the helper using studioSqlTools, enabledTools, READ_ONLY_TOOLS, and isAskMode; in src/main/services/ai/agents/analyticsAgent.ts:257-290, use it with studioAnalyticsTools while preserving TOOL_FLAGS through the helper; in src/main/services/ai/agents/notebooksAgent.ts:282-293, use it with studioNotebookTools and safeEnabledTools so Monaco-bridge exclusions remain applied.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4545aa5a-eaf2-406a-8253-d9c775e4a85a
⛔ Files ignored due to path filters (5)
src/renderer/assets/icons/lucide/blocks.svgis excluded by!**/*.svgsrc/renderer/assets/icons/lucide/bot.svgis excluded by!**/*.svgsrc/renderer/assets/icons/lucide/brain-circuit.svgis excluded by!**/*.svgsrc/renderer/assets/icons/lucide/network.svgis excluded by!**/*.svgsrc/renderer/assets/icons/lucide/settings-2.svgis excluded by!**/*.svg
📒 Files selected for processing (57)
src/main/ipcHandlers/agent.ipcHandlers.tssrc/main/ipcHandlers/index.tssrc/main/ipcHandlers/secondBrain.ipcHandlers.tssrc/main/ipcSetup.tssrc/main/services/agent.service.tssrc/main/services/ai/agents/agentTypes.tssrc/main/services/ai/agents/analyticsAgent.tssrc/main/services/ai/agents/baseAgentConfig.tssrc/main/services/ai/agents/composeAgentRuntime.tssrc/main/services/ai/agents/notebooksAgent.tssrc/main/services/ai/agents/projectAgent.tssrc/main/services/ai/agents/sqlAgent.tssrc/main/services/ai/secondBrain/secondBrain.service.tssrc/main/services/ai/secondBrain/secondBrain.types.tssrc/main/services/ai/secondBrain/secondBrainPolicy.tssrc/main/services/ai/secondBrain/secondBrainRefresh.service.tssrc/main/services/ai/secondBrain/secondBrainRefreshCoordinator.service.tssrc/main/services/ai/secondBrain/secondBrainRuntime.service.tssrc/main/services/ai/secondBrain/wikiMemorySupport.service.tssrc/main/services/ai/tools/studio/secondBrain.tools.tssrc/main/services/ai/tools/studio/sql.tools.tssrc/main/services/mainDatabase.service.tssrc/renderer/assets/icons/lucide/LICENSE.txtsrc/renderer/components/chat/ChatWindow.tsxsrc/renderer/components/chat/ContextUsageRing.tsxsrc/renderer/components/chat/MemoryConsentDialog.tsxsrc/renderer/components/chat/MessageRenderer.tsxsrc/renderer/components/chat/ToolCallFormatters.tsxsrc/renderer/components/editor/markdownPreview/index.tsxsrc/renderer/components/settings/AIProvidersSettings.tsxsrc/renderer/components/settings/SecondBrainTab.tsxsrc/renderer/components/settings/index.tssrc/renderer/controllers/agent.controller.tssrc/renderer/controllers/index.tssrc/renderer/controllers/secondBrain.controller.tssrc/renderer/hooks/useAgentStream.tssrc/renderer/screens/notebooks/index.tsxsrc/renderer/screens/sql/index.tsxsrc/renderer/services/agent.service.tssrc/renderer/services/secondBrain.service.tssrc/types/agentEvents.tssrc/types/backend.tssrc/types/ipc.tssrc/types/secondBrain.tstests/unit/__setup__/electron.mock.tstests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.tstests/unit/main/services/agent.service.test.tstests/unit/main/services/ai/secondBrain/secondBrain.service.test.tstests/unit/main/services/ai/secondBrain/secondBrainPerformance.service.test.tstests/unit/main/services/ai/secondBrain/secondBrainProgressive.service.test.tstests/unit/main/services/ai/secondBrain/secondBrainRefresh.service.test.tstests/unit/main/services/ai/secondBrain/secondBrainRuntime.service.test.tstests/unit/main/services/ai/secondBrain/wikiMemorySupport.service.test.tstests/unit/renderer/components/ContextUsageRing.test.tsxtests/unit/renderer/components/MarkdownPreview.test.tsxtests/unit/renderer/components/ToolCallFormatters.test.tsxtests/unit/renderer/services/secondBrain.service.test.ts
- add pause and clear-and-disable Agent Memory IPC operations - cancel active refreshes before destructive memory clearing - keep filesystem deletion inside SecondBrainService.clearAll - reset enabled and initialized state after clearing memory - add two-stage pause or destructive-clear confirmation UX - organize Agent Memory settings into Global and Project tabs - move global lifecycle controls and wiki workspace into Global - add a per-project agent.md context inclusion switch - keep agent.md creation in the Project Agent ChatWindow flow - remove agent.md status, absolute path, and creation controls from settings - use Agent Memory and Project Memory in user-facing copy - fix enabled switch styling with MUI success colors - strengthen the Initialize memory call to action with MUI theme tokens - remove obsolete Source Health and support diagnostics UI - remove unused diagnostics IPC, types, service, persistence hooks, and tests - preserve development refresh logging without diagnostic persistence - accept OKF concepts with a non-empty type and optional title - prevent source hashes and cursors advancing when all proposals are rejected - update focused refresh, IPC, and renderer tests
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.ts (1)
16-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMock lacks
saveAISettings/clearAll; new pause and clear-and-disable channels are untested.The
agent.servicemock only exportsloadAISettings, and the service mock has noclearAll, so the two new destructive/lifecycle handlers can't be exercised at all. Givenclear-and-disablepermanently deletes memory, add coverage asserting cancel → clear → settings persisted asenabled: false, initialized: false, and that pause leavesinitializeduntouched.💚 Mock additions needed
archivePage: jest.fn(), + clearAll: jest.fn(async () => undefined), listRevisions: jest.fn(async () => []),jest.doMock('../../../../src/main/services/agent.service', () => ({ loadAISettings: jest.fn(async () => ({ secondBrain: { enabled, + initialized: true, maxPageBytes: 65536, maxTotalBytes: 10485760, }, })), + saveAISettings: jest.fn(async () => undefined), }));🤖 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 `@tests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.ts` around lines 16 - 66, Extend the test setup mocks for the agent service and Second Brain service to include saveAISettings and clearAll, then add coverage for the new pause and clear-and-disable IPC handlers. Assert cancel performs no destructive action, while clear-and-disable calls clearAll and persists settings with enabled: false and initialized: false; also assert pause preserves the existing initialized value.src/renderer/components/settings/SecondBrainTab.tsx (1)
86-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated storage-key helper across files.
projectMemoryEnabledKeyis redefined verbatim insrc/renderer/components/chat/ChatWindow.tsx(lines 86-87). Two independent copies of a persisted-key format will silently diverge; extract it (plus the!== 'false'default-on read semantics) into a shared module both import.🤖 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/renderer/components/settings/SecondBrainTab.tsx` around lines 86 - 87, Extract projectMemoryEnabledKey and the associated default-on read semantics (treat stored values other than "false" as enabled) into a shared module, then update SecondBrainTab and ChatWindow to import and use that shared implementation. Remove both local helper definitions and preserve the existing persisted-key format and behavior.
🤖 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 `@src/main/ipcHandlers/secondBrain.ipcHandlers.ts`:
- Around line 174-190: Update the second-brain:clear-and-disable handler to use
an awaitable cancellation/drain operation on refreshCoordinator instead of
synchronous cancelActive(). Ensure the handler waits until all active refresh
work and cleanup are idle before invoking createService().clearAll(), then
preserve the existing settings update and result.
- Around line 158-172: Update the second-brain:pause handler to cancel the
currently active refresh before saving disabled settings, using the same
cancellation mechanism as second-brain:clear-and-disable. Preserve the existing
SecondBrainDisableResult values and settings update behavior.
In `@src/main/services/ai/secondBrain/secondBrainRefresh.service.ts`:
- Around line 539-563: Update the zero-validation branch in the refresh flow
around the `operations.length > 0 && validated.length === 0` check to commit
refresh state for changed sources not implicated by rejected operations,
withholding only source IDs identified through `evidenceByProvenance` and the
rejected-operation provenance. Move or reuse the existing `evidenceByProvenance`
declaration so this filtering occurs before the early return, while preserving
the current completed result and event behavior.
In `@tests/unit/main/services/ai/secondBrain/secondBrainRefresh.service.test.ts`:
- Around line 188-192: Reformat the `.replace(...)` call in the `accepts OKF
documents with type frontmatter and no title` test to match the repository’s
Prettier style, without changing its replacement behavior or surrounding test
logic.
---
Nitpick comments:
In `@src/renderer/components/settings/SecondBrainTab.tsx`:
- Around line 86-87: Extract projectMemoryEnabledKey and the associated
default-on read semantics (treat stored values other than "false" as enabled)
into a shared module, then update SecondBrainTab and ChatWindow to import and
use that shared implementation. Remove both local helper definitions and
preserve the existing persisted-key format and behavior.
In `@tests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.ts`:
- Around line 16-66: Extend the test setup mocks for the agent service and
Second Brain service to include saveAISettings and clearAll, then add coverage
for the new pause and clear-and-disable IPC handlers. Assert cancel performs no
destructive action, while clear-and-disable calls clearAll and persists settings
with enabled: false and initialized: false; also assert pause preserves the
existing initialized value.
🪄 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: a2072109-0b87-458d-8dbe-6b4c2a9f27f7
📒 Files selected for processing (14)
src/main/ipcHandlers/secondBrain.ipcHandlers.tssrc/main/services/ai/agents/projectAgent.tssrc/main/services/ai/secondBrain/secondBrain.service.tssrc/main/services/ai/secondBrain/secondBrainRefresh.service.tssrc/main/services/ai/secondBrain/secondBrainRefreshCoordinator.service.tssrc/renderer/components/chat/ChatWindow.tsxsrc/renderer/components/settings/SecondBrainTab.tsxsrc/renderer/controllers/secondBrain.controller.tssrc/renderer/services/secondBrain.service.tssrc/types/ipc.tssrc/types/secondBrain.tstests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.tstests/unit/main/services/ai/secondBrain/secondBrainRefresh.service.test.tstests/unit/renderer/services/secondBrain.service.test.ts
💤 Files with no reviewable changes (1)
- tests/unit/renderer/services/secondBrain.service.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/services/ai/agents/projectAgent.ts
- src/renderer/components/chat/ChatWindow.tsx
- src/main/services/ai/secondBrain/secondBrain.service.ts
- redact Wiki Memory update and archive rationale, source references, headings, search queries, and page content before tool-call persistence - centralize strict secret detection for refresh operations and agent write tools, including tokens, private keys, and connection strings - serialize generated index updates with a dedicated global lock - budget evidence items before prompt serialization and mark truncated evidence without producing malformed JSON - commit refresh cursors for unaffected sources while withholding sources implicated by rejected or failed operations - use coalesced created/updated timestamps consistently for evidence keyset pagination - add an awaitable refresh cancellation path before pausing or clearing Agent Memory - prevent concurrent agent.md generation streams - keep the memory consent dialog open until settings can be persisted and provide a retry action when loading fails - replace unsupported window.prompt page creation with an MUI dialog - add regression coverage for redaction, secret detection, concurrent index generation, prompt budgeting, selective cursor commits, and refresh draining
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/unit/renderer/components/MarkdownPreview.security.test.ts (1)
3-46: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftExercise the production MarkdownPreview pipeline.
This test sanitizes a hand-built HAST tree in a separate Node process, so it never covers
ReactMarkdown → rehypeRaw → rehypeSanitize → rehypeHighlightfromsrc/renderer/components/editor/markdownPreview/index.tsx. A regression in raw-HTML parsing, plugin ordering, or component wiring could therefore pass. Move this case intotests/unit/renderer/components/MarkdownPreview.test.tsx(or run the same pipeline) and assert the rendered DOM/tree structurally rather than relying on serialized substring checks.rehype-sanitizeis the production wrapper aroundhast-util-sanitize, and its documented security contract includes placement after the last unsafe transform. (github.com)🤖 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 `@tests/unit/renderer/components/MarkdownPreview.security.test.ts` around lines 3 - 46, Move the security case from the standalone hast-util-sanitize subprocess test into MarkdownPreview.test.tsx and exercise the production MarkdownPreview render pipeline, including raw HTML, sanitization, and highlighting plugin wiring. Render malicious script, event-handler, and javascript-link markdown through MarkdownPreview, then assert the resulting DOM/tree structurally excludes executable elements, event-handler attributes, and unsafe URL protocols; preserve sanitize placement after unsafe transforms.
🤖 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 `@src/main/services/ai/secondBrain/secondBrainSecrets.ts`:
- Around line 2-5: Update the authorization-matching regex in the
secondBrainSecrets redaction helper so scheme-prefixed values such as
“Authorization: Bearer <token>” and Basic credentials are matched and fully
redacted, rather than stopping at the scheme. Add regression tests covering both
Bearer and Basic authorization headers, while preserving existing secret-pattern
behavior.
In `@tests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.ts`:
- Line 59: Update the test’s cancelActiveAndWait mock and related assertions to
use a deferred promise instead of resolving immediately. In the cancellation
test around cancelActiveAndWait and clearAll, assert clearAll has not been
called while cancellation remains pending, then resolve the deferred
cancellation and await completion before asserting clearAll was invoked after
cancellation.
---
Nitpick comments:
In `@tests/unit/renderer/components/MarkdownPreview.security.test.ts`:
- Around line 3-46: Move the security case from the standalone
hast-util-sanitize subprocess test into MarkdownPreview.test.tsx and exercise
the production MarkdownPreview render pipeline, including raw HTML,
sanitization, and highlighting plugin wiring. Render malicious script,
event-handler, and javascript-link markdown through MarkdownPreview, then assert
the resulting DOM/tree structurally excludes executable elements, event-handler
attributes, and unsafe URL protocols; preserve sanitize placement after unsafe
transforms.
🪄 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: f3a02184-9fa8-4f65-8aae-b83deacc2dd5
📒 Files selected for processing (18)
src/main/ipcHandlers/secondBrain.ipcHandlers.tssrc/main/services/agent.service.tssrc/main/services/ai/secondBrain/secondBrain.service.tssrc/main/services/ai/secondBrain/secondBrainRefresh.service.tssrc/main/services/ai/secondBrain/secondBrainRefreshCoordinator.service.tssrc/main/services/ai/secondBrain/secondBrainSecrets.tssrc/main/services/ai/tools/studio/secondBrain.tools.tssrc/main/services/mainDatabase.service.tssrc/renderer/components/chat/ChatWindow.tsxsrc/renderer/components/chat/MemoryConsentDialog.tsxsrc/renderer/components/settings/SecondBrainTab.tsxsrc/types/backend.tstests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.tstests/unit/main/services/agent.service.test.tstests/unit/main/services/ai/secondBrain/secondBrain.service.test.tstests/unit/main/services/ai/secondBrain/secondBrainProgressive.service.test.tstests/unit/main/services/ai/secondBrain/secondBrainRefresh.service.test.tstests/unit/renderer/components/MarkdownPreview.security.test.ts
💤 Files with no reviewable changes (1)
- src/types/backend.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- src/main/services/ai/secondBrain/secondBrainRefreshCoordinator.service.ts
- src/renderer/components/chat/MemoryConsentDialog.tsx
- src/renderer/components/settings/SecondBrainTab.tsx
- tests/unit/main/services/agent.service.test.ts
- tests/unit/main/services/ai/secondBrain/secondBrainProgressive.service.test.ts
- src/main/services/ai/tools/studio/secondBrain.tools.ts
- src/main/services/mainDatabase.service.ts
- src/main/ipcHandlers/secondBrain.ipcHandlers.ts
- tests/unit/main/services/ai/secondBrain/secondBrainRefresh.service.test.ts
- tests/unit/main/services/ai/secondBrain/secondBrain.service.test.ts
- src/renderer/components/chat/ChatWindow.tsx
- src/main/services/ai/secondBrain/secondBrain.service.ts
- src/main/services/ai/secondBrain/secondBrainRefresh.service.ts
- src/main/services/agent.service.ts
…-agent-second-brain-longterm-memory
- fully redact scheme-prefixed Authorization credentials - cover Bearer and Basic authorization header redaction - preserve regression coverage for generic secret assignments - use a deferred cancellation promise in IPC handler tests - verify memory clearing waits for active refresh cancellation - remove the weaker invocation-order-only assertion
…-agent-second-brain-longterm-memory
…eAgentStream hook
- replace global memory.md with canonical MEMORY.md - replace project agent.md context with canonical AGENTS.md - update Agent Memory initialization, retrieval, tools, UI, and prompts - validate OKF v0.2 provenance, trust, lifecycle, and date metadata - ground refresh sources and producer metadata in trusted backend evidence - support safe verified Apache dbt runtime evidence - replace wiki-link aliases with standard Markdown links - validate AGENTS.md size, symlink safety, and project-root containment - update tests and AI-context documentation for the direct cutover - require existing development memory to be cleared and reinitialized
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/renderer/components/chat/ChatWindow.tsx (2)
199-220: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRun memory creation in agent mode.
Line 214 forwards
currentMode; when chat mode excludes write tools, accepting this prompt cannot createAGENTS.md. Force'agent'for this explicit creation action.Proposed fix
prompt, [], undefined, - currentMode, + 'agent', screenKey,🤖 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/renderer/components/chat/ChatWindow.tsx` around lines 199 - 220, Update handleGenerateProjectContext so this explicit project-context file creation starts the stream with agent mode instead of forwarding currentMode. Keep the existing prompt and other startStream arguments unchanged, ensuring write tools are available for the generation action.
269-293: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInclude project memory in overhead requests.
Actual runs send
projectMemoryEnabled(Line 1171), but this request omitsincludeProjectAiContext. The context ring therefore undercountsAGENTS.mdwhenever it is included in the prompt.Proposed fix
connectionId, notebookId, pageId, + includeProjectAiContext: projectMemoryEnabled, }🤖 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/renderer/components/chat/ChatWindow.tsx` around lines 269 - 293, Update the contextOverheadRequest useMemo to include the project-memory flag as includeProjectAiContext, matching the projectMemoryEnabled value used by the actual request flow near line 1171, and add that value to the memo dependency list so overhead calculations stay synchronized.src/renderer/hooks/useAgentStream.ts (1)
181-196: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winFinalize running tool calls on every terminal path.
This cleanup only runs for
data.done. The rejection handler at Lines 411-419 stops streaming but preserves running tool calls, leaving their UI permanently spinning whenrunAgentfails before a done event. Reuse this mapping in rejection and cancellation cleanup too.🤖 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/renderer/hooks/useAgentStream.ts` around lines 181 - 196, Extract or reuse the running tool-call finalization mapping currently inside the data.done branch of the stream handling logic. Apply the same cleanup when runAgent rejects and during cancellation cleanup, alongside setting isStreaming to false, so every terminal path converts running tool-call parts to error status with the existing message.
🧹 Nitpick comments (2)
tests/unit/main/services/ai/secondBrain/secondBrainRefreshCoordinator.service.test.ts (1)
40-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSolid coverage of busy/owner/operation-id checks and abort propagation. Correctly matches
run/cancelsemantics insecondBrainRefreshCoordinator.service.ts.One minor observation: synchronizing on the busy state via a fixed
await Promise.resolve()x2 (lines 57-58) is brittle if an extra microtask is later inserted beforerun()setsthis.activeOperation; a small helper (e.g. resolving oncemockRefreshhas been called) would be more robust to future refactors. Not a blocker.🤖 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 `@tests/unit/main/services/ai/secondBrain/secondBrainRefreshCoordinator.service.test.ts` around lines 40 - 81, Replace the fixed pair of Promise.resolve waits in the “preserves operation and owner checks while an operation is active” test with a synchronization helper that resolves when mockRefresh has been invoked and the active operation is established. Keep the existing status, owner/operation-id validation, abort propagation, and cleanup assertions unchanged.src/main/services/ai/secondBrain/secondBrainRefresh.service.ts (1)
943-952: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated runtime-classification expression.
installed.version?.startsWith('2.') ? 'v2' : 'v1'is repeated verbatim incontentHashandprojection.runtime; extracting a localconst runtime = ...avoids the two copies drifting apart.♻️ Proposed fix
const items: SecondBrainEvidenceItem[] = verified ? [ { sourceId: 'dbt-runtime', sourceKind: 'application', stableId: 'installed-apache-dbt-core', updatedAt: '', - contentHash: hashValue({ - version: installed.version, - runtime: installed.version?.startsWith('2.') ? 'v2' : 'v1', - }), + contentHash: hashValue({ version: installed.version, runtime }), scope: {}, provenance: 'rosetta:dbt-core:installed-package', projection: { package: 'apache-dbt-core', version: installed.version, - runtime: installed.version?.startsWith('2.') ? 'v2' : 'v1', + runtime, executableVerified: true, }, truncated: false, }, ] : [];(add
const runtime = installed.version?.startsWith('2.') ? 'v2' : 'v1';above the ternary chain.)🤖 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/main/services/ai/secondBrain/secondBrainRefresh.service.ts` around lines 943 - 952, In the installed-package mapping, extract the repeated version classification into a local runtime constant before constructing the object, then reuse it for both contentHash and projection.runtime. Keep the existing v2/v1 classification unchanged.
🤖 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 `@src/main/services/ai/projectAgentContext.ts`:
- Line 1: Add fs-extra to the application manifest’s dependencies and regenerate
the checked-in lockfiles so the package and resolved metadata are included for
production installs. Keep the existing imports and usage unchanged.
In `@src/main/services/ai/secondBrain/secondBrainRefresh.service.ts`:
- Around line 326-332: Update groundRefreshContent’s frontmatter.sources
construction to sort the resolved evidence deterministically by each item’s
stable identifier before mapping to source entries. Do not preserve the
LLM-controlled operation.provenanceIds order; match the stableId.localeCompare
ordering used by the other evidence lists so identical evidence produces
identical output.
- Around line 321-325: Update groundRefreshContent() to preserve verified and
usage_window from the existing page frontmatter when handling replace
operations, while continuing to reset sources and generated. In
validateOperations, pass the page’s existing frontmatter alongside expectedHash
into groundRefreshContent() so those fields can be rehydrated before the
refreshed content is returned.
---
Outside diff comments:
In `@src/renderer/components/chat/ChatWindow.tsx`:
- Around line 199-220: Update handleGenerateProjectContext so this explicit
project-context file creation starts the stream with agent mode instead of
forwarding currentMode. Keep the existing prompt and other startStream arguments
unchanged, ensuring write tools are available for the generation action.
- Around line 269-293: Update the contextOverheadRequest useMemo to include the
project-memory flag as includeProjectAiContext, matching the
projectMemoryEnabled value used by the actual request flow near line 1171, and
add that value to the memo dependency list so overhead calculations stay
synchronized.
In `@src/renderer/hooks/useAgentStream.ts`:
- Around line 181-196: Extract or reuse the running tool-call finalization
mapping currently inside the data.done branch of the stream handling logic.
Apply the same cleanup when runAgent rejects and during cancellation cleanup,
alongside setting isStreaming to false, so every terminal path converts running
tool-call parts to error status with the existing message.
---
Nitpick comments:
In `@src/main/services/ai/secondBrain/secondBrainRefresh.service.ts`:
- Around line 943-952: In the installed-package mapping, extract the repeated
version classification into a local runtime constant before constructing the
object, then reuse it for both contentHash and projection.runtime. Keep the
existing v2/v1 classification unchanged.
In
`@tests/unit/main/services/ai/secondBrain/secondBrainRefreshCoordinator.service.test.ts`:
- Around line 40-81: Replace the fixed pair of Promise.resolve waits in the
“preserves operation and owner checks while an operation is active” test with a
synchronization helper that resolves when mockRefresh has been invoked and the
active operation is established. Keep the existing status, owner/operation-id
validation, abort propagation, and cleanup assertions unchanged.
🪄 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: 23f8f9e1-0d56-4d67-8bf4-b9b3690dcc6f
📒 Files selected for processing (30)
src/main/services/agent.service.tssrc/main/services/ai/agents/projectAgent.tssrc/main/services/ai/projectAgentContext.tssrc/main/services/ai/secondBrain/secondBrain.service.tssrc/main/services/ai/secondBrain/secondBrain.types.tssrc/main/services/ai/secondBrain/secondBrainPolicy.tssrc/main/services/ai/secondBrain/secondBrainRefresh.service.tssrc/main/services/ai/secondBrain/secondBrainRefreshCoordinator.service.tssrc/main/services/ai/secondBrain/secondBrainRuntime.service.tssrc/main/services/ai/tools/studio/secondBrain.tools.tssrc/renderer/components/chat/ChatMessageList.tsxsrc/renderer/components/chat/ChatWindow.tsxsrc/renderer/components/settings/SecondBrainTab.tsxsrc/renderer/components/settings/secondBrainOperationUi.tssrc/renderer/controllers/secondBrain.controller.tssrc/renderer/hooks/useAgentStream.tssrc/renderer/services/agent.service.tssrc/shared/agentMemoryConstants.tssrc/types/backend.tssrc/types/secondBrain.tstests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.tstests/unit/main/services/agent.service.test.tstests/unit/main/services/ai/projectAgentContext.test.tstests/unit/main/services/ai/secondBrain/secondBrain.service.test.tstests/unit/main/services/ai/secondBrain/secondBrainProgressive.service.test.tstests/unit/main/services/ai/secondBrain/secondBrainRefresh.service.test.tstests/unit/main/services/ai/secondBrain/secondBrainRefreshCoordinator.service.test.tstests/unit/main/services/ai/secondBrain/secondBrainRuntime.service.test.tstests/unit/renderer/components/secondBrainOperationUi.test.tstests/unit/renderer/services/secondBrain.service.test.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- tests/unit/renderer/services/secondBrain.service.test.ts
- src/types/backend.ts
- src/main/services/ai/secondBrain/secondBrainPolicy.ts
- tests/unit/main/services/ai/secondBrain/secondBrainRuntime.service.test.ts
- src/types/secondBrain.ts
- tests/unit/main/ipcHandlers/secondBrain.ipcHandlers.test.ts
- src/main/services/ai/secondBrain/secondBrain.types.ts
- tests/unit/main/services/agent.service.test.ts
- tests/unit/main/services/ai/secondBrain/secondBrainProgressive.service.test.ts
- src/renderer/controllers/secondBrain.controller.ts
- src/main/services/ai/secondBrain/secondBrainRefreshCoordinator.service.ts
- tests/unit/main/services/ai/secondBrain/secondBrainRefresh.service.test.ts
- src/main/services/ai/agents/projectAgent.ts
- src/renderer/components/settings/SecondBrainTab.tsx
- src/main/services/ai/secondBrain/secondBrainRuntime.service.ts
- src/main/services/ai/secondBrain/secondBrain.service.ts
- src/main/services/agent.service.ts
- declare fs-extra in application dependencies - regenerate root and production app lockfiles - preserve verified and usage_window metadata on replace - prevent refreshes from forging preserved metadata - sort grounded sources deterministically by stable ID - add regression coverage for replace metadata and source ordering
…ructured output schema error In @ai-sdk/openai v3+, calling the provider directly (provider(model)) routes to the Responses API (/v1/responses), which enforces strict JSON schema validation requiring all properties to be listed in `required`. This caused the second-brain refresh to fail with: "Invalid schema for response_format 'second_brain_refresh_operations': Missing 'expectedHash' in required array." Fix by using createOpenAI().chat(model) to explicitly target the Chat Completions API, which does not enforce strict schema mode. Also annotate `model` as LanguageModel in secondBrainRefresh.service.ts to prevent TS2589 deep type instantiation triggered by the provider type change.
2 participants
Summary by CodeRabbit
feat(ai): implement project-scoped AI context and agent.md memory
ChatWindow.tsxautomatically reads<dbt-project>/agent.mdon the Project screen and injects its contents into every chat request asprojectAiContext, giving the Project Agent project-specific rules and context.agent.mdif it is missing. The banner delegates directly to the Project Agent via a chat prompt; the agent uses its nativewriteFiletool to create the file. The banner is permanently dismissible per-project via localStorage.agent.mdcontent in a strict, ordered block.studio_sql_schema_extractto understand the backing database schema in addition to dbt project files.EnrichedConnectionMetainagentTypes.tsand implementedAgentService.resolveEnrichedConnectionMeta()to resolvedatabase,schema, andlinkedDbtProjectfor the active connection.Database,Schema, andLinked dbt Projectmetadata into their system prompts.readFile,listDirectory,readDbtModel, etc.) when a dbt project is linked, without relying on UI bridges.