diff --git a/docs/chat-interface.md b/docs/chat-interface.md index 0839185b..20c62c53 100644 --- a/docs/chat-interface.md +++ b/docs/chat-interface.md @@ -71,7 +71,7 @@ Each AI response has action buttons: ### Autosave -By default, Copilot automatically saves your conversations as markdown files in your vault. Each saved chat appears in the `copilot/copilot-conversations/` folder. +By default, Copilot automatically saves your conversations as markdown files in your vault. Each saved chat appears in the `copilot-conversations/` sub-folder of your Copilot folder (by default `copilot/copilot-conversations/`). You can turn off autosave in Settings → Basic. When you start a new chat, any unsaved conversation is saved automatically. @@ -145,7 +145,7 @@ Relevant Notes lives in its own pane. Open it from the command palette: **Open R ## Saving a Chat Manually -If autosave is off, or you want to save mid-conversation, click the **Save Chat as Note** button above the chat input box. This saves the current conversation to your configured save folder. +If autosave is off, or you want to save mid-conversation, click the **Save Chat as Note** button above the chat input box. This saves the current conversation to your Copilot conversations folder (`copilot-conversations/` under your Copilot folder). --- diff --git a/docs/copilot-plus-and-self-host.md b/docs/copilot-plus-and-self-host.md index b8787c27..62c69559 100644 --- a/docs/copilot-plus-and-self-host.md +++ b/docs/copilot-plus-and-self-host.md @@ -64,7 +64,7 @@ You can ask Copilot to explicitly remember specific facts about you: Copilot saves this to a memory file in your vault and references it in future conversations. - **Enable**: **Settings → Copilot → Plus → Reference Saved Memories** (on by default) -- **Memory folder**: **Settings → Copilot → Plus → Memory Folder Name** — default: `copilot/memory` +- **Memory folder**: memories are stored in the `memory/` sub-folder of your Copilot folder — default `copilot/memory`. It follows the Copilot folder location (**Settings → Copilot → Basic → Copilot folder location**). - **Update memory tool**: The AI can add, update, or remove memories when you ask --- diff --git a/docs/custom-commands.md b/docs/custom-commands.md index f49111d4..b5f98877 100644 --- a/docs/custom-commands.md +++ b/docs/custom-commands.md @@ -121,7 +121,7 @@ Go to **Settings → Copilot → Command** to manage all your custom commands: ### Custom Prompts Folder -Commands are stored as markdown files in your vault. The default folder is `copilot/copilot-custom-prompts/`. You can change this in **Settings → Copilot → Basic → Custom prompts folder**. +Commands are stored as markdown files in your vault, in the `copilot-custom-prompts/` sub-folder of your Copilot folder (by default `copilot/copilot-custom-prompts/`). This location is derived from your Copilot folder — change the root in **Settings → Copilot → Basic → Copilot folder location** to change where new commands are saved. There is no separate custom-prompts folder setting anymore. --- diff --git a/docs/system-prompts.md b/docs/system-prompts.md index f3a853b8..84fc7787 100644 --- a/docs/system-prompts.md +++ b/docs/system-prompts.md @@ -38,12 +38,12 @@ Custom system prompts let you add your own instructions on top of the built-in p ### Where They're Stored -Custom system prompts are stored as markdown files in your vault, in the folder: +Custom system prompts are stored as markdown files in your vault, in the `system-prompts/` sub-folder of your Copilot folder: ``` copilot/system-prompts/ ``` -You can change this folder in **Settings → Copilot → Advanced → System Prompts Folder Name**. +This location is derived from your Copilot folder. To move it, change the root in **Settings → Copilot → Basic → Copilot folder location** — every Copilot sub-folder follows that root, so it is no longer configured on its own. Changing the root affects only where new prompts are saved; existing prompt files stay where they are unless you move them yourself. ### Creating a System Prompt diff --git a/docs/troubleshooting-and-faq.md b/docs/troubleshooting-and-faq.md index 061e9f36..76d69cae 100644 --- a/docs/troubleshooting-and-faq.md +++ b/docs/troubleshooting-and-faq.md @@ -259,7 +259,7 @@ Yes. You can have API keys configured for multiple providers simultaneously and ### Where are my saved chats stored? -Chat conversations are saved as markdown files in your vault, in the folder `copilot/copilot-conversations/` by default. You can change this folder in **Settings → Copilot → Basic → Default save folder**. +Chat conversations are saved as markdown files in your vault, in the `copilot-conversations/` sub-folder of your Copilot folder (by default `copilot/copilot-conversations/`). To change where new chats are saved, change the root in **Settings → Copilot → Basic → Copilot folder location** — every Copilot sub-folder derives from it. Changing the root affects only new chats; conversations you have already saved stay in the old folder unless you move them yourself. ### How do I clear the Copilot cache? @@ -267,13 +267,15 @@ Use **Command palette → Clear Copilot cache**. This clears cached responses an ### What is the `copilot/` folder in my vault? -The `copilot/` folder is created by the plugin and stores: +The `copilot/` folder is your Copilot folder — the default root the plugin uses to store its own files: - `copilot-conversations/` — Saved chat histories - `copilot-custom-prompts/` — Your custom commands - `system-prompts/` — Your custom system prompts - `memory/` — Saved AI memories (if enabled) +- `skills/` — Agent skills +- `projects/` — Project files -This folder is automatically excluded from vault search to avoid cluttering results. +You can rename this root in **Settings → Copilot → Basic → Copilot folder location**; all of the sub-folders above derive from it. The Copilot folder is automatically excluded from vault search to avoid cluttering results. If you change the root, the old folder is not moved for you and stays excluded from search permanently — any folder that has ever been your Copilot folder is kept out of search results even after you switch away from it. ### How do I switch modes? diff --git a/docs/vault-search-and-indexing.md b/docs/vault-search-and-indexing.md index f7a6038b..5b93f668 100644 --- a/docs/vault-search-and-indexing.md +++ b/docs/vault-search-and-indexing.md @@ -114,7 +114,7 @@ Example: `private, Work/Confidential, #private` excludes the private folder, a s > **Note**: Tag matching works with tags in the note's **properties (frontmatter)**, not inline tags within the note body. -The `copilot` folder is always excluded automatically (it contains the plugin's own files). +Your Copilot folder is always excluded automatically (it holds the plugin's own files — conversations, prompts, memory, skills, and projects). This covers the default `copilot` folder, your current Copilot folder if you changed it in **Settings → Copilot → Basic → Copilot folder location**, and every folder that has previously been your Copilot folder. Past Copilot folders stay excluded permanently, so any files the plugin left behind after a folder change never leak into search results. ### Inclusions diff --git a/src/agentMode/index.ts b/src/agentMode/index.ts index 914b37ae..118e3833 100644 --- a/src/agentMode/index.ts +++ b/src/agentMode/index.ts @@ -3,8 +3,8 @@ import os from "node:os"; import * as path from "node:path"; import type CopilotPlugin from "@/main"; import { logError } from "@/logger"; -import { DEFAULT_SKILLS_FOLDER } from "@/constants"; import { getSettings, subscribeToSettingsChange, type CopilotSettings } from "@/settings/model"; +import { deriveSkillsFolder, getEffectiveSkillsFolder } from "@/settings/copilotFolder"; import { subscribeToSystemPromptChange } from "@/system-prompts/state"; import { copilotAppDataDir, getVaultId } from "@/utils/appPaths"; import { buildAgentSystemPrompt } from "./backends/shared/agentSystemPrompt"; @@ -364,8 +364,10 @@ export function createAgentSessionManager(app: App, plugin: CopilotPlugin): Agen // appear in the new folder without a reload) or when Miyo availability // flips (so the gated Miyo skill is seeded/pruned to match). Both run the // same gate-aware seed pass against the current folder. - const prevFolder = prev.agentMode?.skills?.folder ?? DEFAULT_SKILLS_FOLDER; - const nextFolder = next.agentMode?.skills?.folder ?? DEFAULT_SKILLS_FOLDER; + // Reason: the skills folder is derived from the configurable copilotFolder + // root; compare derived paths rather than the retired agentMode.skills.folder. + const prevFolder = deriveSkillsFolder(prev); + const nextFolder = deriveSkillsFolder(next); // The `miyo-search` skill now seeds/prunes off the explicit // `enableMiyoSearchSkill` toggle, so a flip must re-seed and restart (codex/ // opencode bake the steering at spawn). `enableMiyo` / `miyoServerUrl` are @@ -414,11 +416,9 @@ export function createAgentSessionManager(app: App, plugin: CopilotPlugin): Agen // Seed plugin-shipped builtin skills into the canonical folder, THEN run // discovery so the first pass picks them up and fans them out to the agent // dirs. Non-blocking for plugin load. - void seedManagedBuiltins(getSettings().agentMode?.skills?.folder ?? DEFAULT_SKILLS_FOLDER).catch( - (error) => { - logError("[Skills] Initial discovery pass failed", error); - } - ); + void seedManagedBuiltins(getEffectiveSkillsFolder()).catch((error) => { + logError("[Skills] Initial discovery pass failed", error); + }); // Non-blocking — plugin load should not wait on disk reconcile. for (const descriptor of listBackendDescriptors()) { descriptor diff --git a/src/agentMode/session/AgentChatPersistenceManager.test.ts b/src/agentMode/session/AgentChatPersistenceManager.test.ts index 89fde06f..dd8171fb 100644 --- a/src/agentMode/session/AgentChatPersistenceManager.test.ts +++ b/src/agentMode/session/AgentChatPersistenceManager.test.ts @@ -6,6 +6,7 @@ import { GLOBAL_SCOPE } from "./scope"; import type { AgentChatMessage } from "./types"; import { TFile } from "obsidian"; import type { App } from "obsidian"; +import { getSettings } from "@/settings/model"; jest.mock("obsidian", () => ({ Notice: jest.fn(), @@ -19,6 +20,9 @@ jest.mock("@/settings/model", () => ({ defaultConversationNoteName: "{$date}_{$time}__{$topic}", }), })); +jest.mock("@/settings/copilotFolder", () => ({ + getEffectiveConversationsFolder: jest.fn(() => "test-folder"), +})); jest.mock("@/utils", () => ({ ensureFolderExists: jest.fn(async () => {}), formatDateTime: jest.fn(() => ({ @@ -129,6 +133,19 @@ describe("AgentChatPersistenceManager", () => { expect(loaded.messages[1].message).toBe("hi back"); }); + it("writes the built-in conversation tag independent of the persisted setting", async () => { + // Freeze check: a custom/stale defaultConversationTag must not reach new notes. + (getSettings as jest.Mock).mockReturnValueOnce({ + defaultSaveFolder: "test-folder", + defaultConversationTag: "user-custom-tag", + defaultConversationNoteName: "{$date}_{$time}__{$topic}", + }); + const saved = await manager.saveSession([makeMessage(USER_SENDER, "hi")], "claude", {}); + const contents = app.files.get(saved!.path)!.contents!; + expect(contents).toContain("tags:\n - copilot-conversation"); + expect(contents).not.toContain("user-custom-tag"); + }); + it("serializes a mid-stream fan-out turn so an interrupted autosave isn't blank", async () => { // A long fan-out turn whose composite body has NOT been written to `message` // yet (still streaming), saved mid-turn (reload/close/crash). The live fanout diff --git a/src/agentMode/session/AgentChatPersistenceManager.ts b/src/agentMode/session/AgentChatPersistenceManager.ts index 2af404d5..6f929e55 100644 --- a/src/agentMode/session/AgentChatPersistenceManager.ts +++ b/src/agentMode/session/AgentChatPersistenceManager.ts @@ -1,7 +1,8 @@ import { serializeFanoutComposite } from "@/agentMode/session/fanout/fanoutTypes"; -import { AGENT_CHAT_MODE, AI_SENDER, USER_SENDER } from "@/constants"; +import { AGENT_CHAT_MODE, AI_SENDER, COPILOT_CONVERSATION_TAG, USER_SENDER } from "@/constants"; import { logError, logInfo, logWarn } from "@/logger"; import { getSettings } from "@/settings/model"; +import { getEffectiveConversationsFolder } from "@/settings/copilotFolder"; import { FormattedDateTime } from "@/types/message"; import { ensureFolderExists, @@ -139,11 +140,10 @@ export class AgentChatPersistenceManager { if (messages.length === 0) return null; try { - const settings = getSettings(); const chatContent = this.formatChatContent(messages); const firstMessageEpoch = messages[0].timestamp?.epoch ?? Date.now(); - await ensureFolderExists(this.app.vault, settings.defaultSaveFolder); + await ensureFolderExists(this.app.vault, getEffectiveConversationsFolder()); const existingFile = options?.existingPath ? this.resolveExistingFile(options.existingPath) @@ -196,7 +196,7 @@ export class AgentChatPersistenceManager { } if (isNameTooLongError(err)) { logWarn("[AgentChatPersistenceManager] Filename too long, falling back to minimal name"); - const fallback = `${settings.defaultSaveFolder}/${AGENT_FILENAME_PREFIX}chat-${firstMessageEpoch}.md`; + const fallback = `${getEffectiveConversationsFolder()}/${AGENT_FILENAME_PREFIX}chat-${firstMessageEpoch}.md`; try { const created = await this.app.vault.create(fallback, noteContent); return { path: created.path }; @@ -255,8 +255,7 @@ export class AgentChatPersistenceManager { * never collides with legacy or project chats. */ async getAgentChatHistoryFiles(): Promise { - const settings = getSettings(); - const files = await listMarkdownFiles(this.app, settings.defaultSaveFolder); + const files = await listMarkdownFiles(this.app, getEffectiveConversationsFolder()); return files.filter((file) => file.basename.startsWith(AGENT_FILENAME_PREFIX)); } @@ -474,10 +473,10 @@ export class AgentChatPersistenceManager { if (getUtf8ByteLength(baseNameWithPrefix) > SAFE_FILENAME_BYTE_LIMIT) { const availableForBasename = SAFE_FILENAME_BYTE_LIMIT - extensionBytes - filePrefixBytes; const truncatedBasename = truncateToByteLimit(sanitizedFileName, availableForBasename); - return `${settings.defaultSaveFolder}/${filePrefix}${truncatedBasename}.md`; + return `${getEffectiveConversationsFolder()}/${filePrefix}${truncatedBasename}.md`; } - return `${settings.defaultSaveFolder}/${baseNameWithPrefix}`; + return `${getEffectiveConversationsFolder()}/${baseNameWithPrefix}`; } private generateNoteContent(args: { @@ -492,7 +491,6 @@ export class AgentChatPersistenceManager { projectId?: string; usage?: SessionUsage; }): string { - const settings = getSettings(); const lines: string[] = [ "---", `epoch: ${args.firstMessageEpoch}`, @@ -516,7 +514,7 @@ export class AgentChatPersistenceManager { // parser (and our hand-rolled splitFrontmatter) verbatim. if (args.usage) lines.push(`usage: '${JSON.stringify(args.usage)}'`); lines.push("tags:"); - lines.push(` - ${settings.defaultConversationTag}`); + lines.push(` - ${COPILOT_CONVERSATION_TAG}`); lines.push("---"); lines.push(""); lines.push(args.chatContent); diff --git a/src/agentMode/skills/SkillManager.test.ts b/src/agentMode/skills/SkillManager.test.ts index 98154930..3c95d859 100644 --- a/src/agentMode/skills/SkillManager.test.ts +++ b/src/agentMode/skills/SkillManager.test.ts @@ -30,6 +30,10 @@ jest.mock("@/settings/model", () => ({ updateSetting: jest.fn(), })); +jest.mock("@/settings/copilotFolder", () => ({ + getEffectiveSkillsFolder: () => skillsFolder, +})); + jest.mock("./discoverManagedSkills", () => ({ discoverManagedSkills: jest.fn(), })); diff --git a/src/agentMode/skills/SkillManager.ts b/src/agentMode/skills/SkillManager.ts index 19f7f6e6..0c8d528e 100644 --- a/src/agentMode/skills/SkillManager.ts +++ b/src/agentMode/skills/SkillManager.ts @@ -1,5 +1,6 @@ import { logError, logInfo, logWarn } from "@/logger"; import { getSettings, updateSetting } from "@/settings/model"; +import { getEffectiveSkillsFolder } from "@/settings/copilotFolder"; import { atom, createStore, useAtomValue } from "jotai"; import { FileSystemAdapter, type App, type EventRef, type TAbstractFile } from "obsidian"; import { normalizeAbsPath, parentDir } from "@/utils/pathUtils"; @@ -1211,16 +1212,13 @@ function stableHash(value: string): string { } /** - * Resolve `agentMode.skills.folder` defensively. Settings validation - * normally guarantees a well-formed value, but the UI may render before - * settings hydration finishes; fall back to the default in that window. + * Resolve the effective skills folder, derived from the configurable copilotFolder + * root. Settings validation normally guarantees a well-formed root, but the UI may + * render before settings hydration finishes; the derivation falls back to the + * default in that window. */ function resolveSkillsFolder(): string { - const raw = getSettings().agentMode?.skills?.folder; - if (typeof raw !== "string" || raw.trim().length === 0) { - return DEFAULT_SKILLS_FOLDER; - } - return raw; + return getEffectiveSkillsFolder(); } /** diff --git a/src/agentMode/skills/ui/SkillsSettings.test.tsx b/src/agentMode/skills/ui/SkillsSettings.test.tsx new file mode 100644 index 00000000..74269086 --- /dev/null +++ b/src/agentMode/skills/ui/SkillsSettings.test.tsx @@ -0,0 +1,73 @@ +import { AppContext } from "@/context"; +import { DEFAULT_SETTINGS } from "@/constants"; +import { settingsAtom, settingsStore, updateSetting } from "@/settings/model"; +import { act, render, screen } from "@testing-library/react"; +import type { App } from "obsidian"; +import React from "react"; +import { SkillsSettings } from "./SkillsSettings"; + +// The manager owns filesystem discovery and a live subscription store; stub the +// singleton so the component renders in isolation while letting us observe the +// refresh triggered by a derived-path change. +const refresh = jest.fn().mockResolvedValue(undefined); +const getAgentDirsProjectRel = jest.fn().mockReturnValue({}); +jest.mock("@/agentMode/skills/SkillManager", () => ({ + SkillManager: { getInstance: () => ({ refresh, getAgentDirsProjectRel }) }, + // eslint-disable-next-line @eslint-react/hooks-extra/no-unnecessary-use-prefix -- mocks the real hook; name must match the export + useManagedSkills: () => [], + // eslint-disable-next-line @eslint-react/hooks-extra/no-unnecessary-use-prefix -- mocks the real hook; name must match the export + useEpermSeen: () => false, + dismissEpermBanner: jest.fn(), +})); + +// The registry pulls in every backend's icon/adapter chain; the row list is +// empty in these tests, so an empty descriptor set keeps the surface minimal. +jest.mock("@/agentMode/backends/registry", () => ({ + listBackendDescriptors: () => [], +})); + +function renderSettings() { + return render( + + + + ); +} + +describe("SkillsSettings", () => { + describe("SkillsSettings()", () => { + beforeEach(() => { + jest.clearAllMocks(); + settingsStore.set(settingsAtom, { ...DEFAULT_SETTINGS, copilotFolder: "copilot" }); + }); + + it("does not surface a Skills folder settings control (folder is root-derived, not user-editable)", () => { + renderSettings(); + // The dedicated "Skills folder" setting row was removed: no editable input + // bound to the retired agentMode.skills.folder field, and no setting row. + expect(screen.queryByLabelText("Skills folder")).toBeNull(); + expect(screen.queryByText("Skills folder")).toBeNull(); + }); + + it("re-derives the discovered folder and re-scans when the Copilot root changes", () => { + renderSettings(); + // Mount runs one discovery pass; the derived path surfaces in the + // empty-state hint (there is no folder-setting row). + expect(screen.getByText(/copilot\/skills/)).not.toBeNull(); + expect(refresh).toHaveBeenCalledTimes(1); + + // No manual rerender: useSettingsValue is a live jotai subscription, so + // the store update alone must re-render and re-scan. Asserting that here + // is what locks the reactive contract (a snapshot-only regression fails). + act(() => { + updateSetting("copilotFolder", "vault-tools"); + }); + + expect(screen.getByText(/vault-tools\/skills/)).not.toBeNull(); + expect(screen.queryByText(/copilot\/skills/)).toBeNull(); + // The discovery pass re-runs so the list reflects the new derived folder; + // guards against the effect regressing to a stale/retired dependency. + expect(refresh).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/src/agentMode/skills/ui/SkillsSettings.tsx b/src/agentMode/skills/ui/SkillsSettings.tsx index 93037203..21d3bf7d 100644 --- a/src/agentMode/skills/ui/SkillsSettings.tsx +++ b/src/agentMode/skills/ui/SkillsSettings.tsx @@ -1,4 +1,3 @@ -import { DEFAULT_SKILLS_FOLDER } from "@/agentMode/skills/agentPaths"; import { formatSkillDisplayName } from "@/agentMode/skills/mergeDiscovery"; import { listBackendDescriptors } from "@/agentMode/backends/registry"; import type { AgentBrand } from "@/agentMode/session/types"; @@ -17,16 +16,14 @@ import { } from "@/agentMode/skills/SkillManager"; import { SkillRow } from "./SkillRow"; import { type Skill } from "@/agentMode/skills/types"; -import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; -import { SettingItem } from "@/components/ui/setting-item"; -import { SettingSection } from "@/components/ui/setting-section"; import { cn } from "@/lib/utils"; -import { logError, logWarn } from "@/logger"; +import { logWarn } from "@/logger"; +import { deriveSkillsFolder } from "@/settings/copilotFolder"; import { openWithSystemDefault } from "@/utils/openWithSystemDefault"; import { getVaultBase, toVaultRelative } from "@/utils/vaultPath"; -import { updateSetting, useSettingsValue, validateSkillsFolder } from "@/settings/model"; -import { AlertTriangle, Folder, Search } from "lucide-react"; +import { useSettingsValue } from "@/settings/model"; +import { AlertTriangle, Search } from "lucide-react"; import { App, FileSystemAdapter, Notice, TFile, TFolder } from "obsidian"; import { useApp } from "@/context"; import React, { useCallback, useEffect, useMemo, useRef, useState } from "react"; @@ -45,9 +42,10 @@ const SYNC_BRANDS: ReadonlyArray<{ substr: string; brand: string }> = [ /** * Skills tab. * - * Renders the header copy, the Skills-folder setting row, the toolbar - * (search + count), and either the empty placeholder or the Tidy list of - * {@link SkillRow}s sourced from {@link SkillManager}. + * Renders the header copy, the toolbar (search + count), and either the + * empty placeholder or the Tidy list of {@link SkillRow}s sourced from + * {@link SkillManager}. The skills folder is root-derived and not editable + * here, so there is no folder-setting row. * * Discovery is fully automatic — the unified walker (canonical folder plus * every registered agent's project-skills directory) runs on every mount @@ -62,6 +60,10 @@ const SYNC_BRANDS: ReadonlyArray<{ substr: string; brand: string }> = [ export const SkillsSettings: React.FC = () => { const app = useApp(); const settings = useSettingsValue(); + // Skills live under the single configurable Copilot root. The derived path + // drives discovery (the effect below) and the empty-state hint; it is not + // user-editable here, so there is no folder-setting row. + const skillsFolder = deriveSkillsFolder(settings); // Brand projection of every registered backend. Sourced from the public // registry — descriptors are module-level constants so the list is stable // per session; the `useMemo` keeps the reference identity stable across @@ -75,12 +77,9 @@ export const SkillsSettings: React.FC = () => { })), [] ); - const persistedFolder = settings.agentMode.skills.folder; const skills = useManagedSkills(); const epermSeen = useEpermSeen(); - const [draft, setDraft] = useState(persistedFolder); - const [validationError, setValidationError] = useState(null); const [searchValue, setSearchValue] = useState(""); // Anchor for Radix portals on this tab (e.g. SkillRow's overflow menu). @@ -93,58 +92,13 @@ export const SkillsSettings: React.FC = () => { // EPERM banner. Neither persists across plugin reloads — by design. const [syncBannerDismissed, setSyncBannerDismissed] = useState(false); - // Keep the local draft in sync if persisted settings change underneath us - // (e.g. via Reset Settings). We don't want to clobber the user's in-flight - // typing, so only sync when the persisted value changes. - useEffect(() => { - /* eslint-disable @eslint-react/hooks-extra/no-direct-set-state-in-use-effect -- resync the local draft when persisted settings change underneath us (e.g. Reset Settings); see comment above */ - setDraft(persistedFolder); - setValidationError(null); - /* eslint-enable @eslint-react/hooks-extra/no-direct-set-state-in-use-effect */ - }, [persistedFolder]); - - // Trigger a discovery pass on mount and on persisted-folder change so - // the list reflects whatever lives at the currently configured path. - // The unified walker pulls in canonical + every agent's project-skills - // dir in one pass. + // Trigger a discovery pass on mount and whenever the derived folder changes + // (e.g. the user moves the Copilot root) so the list reflects whatever lives + // at the currently configured path. The unified walker pulls in canonical + + // every agent's project-skills dir in one pass. useEffect(() => { void SkillManager.getInstance().refresh(); - }, [persistedFolder]); - - /** Validate the draft against `validateSkillsFolder`; updates inline error state. */ - const validate = useCallback((value: string) => { - const result = validateSkillsFolder(value); - if (!result.ok) { - setValidationError(result.reason); - return null; - } - setValidationError(null); - return result.folder; - }, []); - - /** Persist the draft when it is valid; called on blur and on Enter. */ - const commit = useCallback( - (value: string) => { - const folder = validate(value); - if (folder === null) return; - if (folder === persistedFolder) return; - try { - updateSetting("agentMode", { - ...settings.agentMode, - skills: { ...settings.agentMode.skills, folder }, - }); - } catch (err) { - logError("Failed to persist skills folder", err); - new Notice("Failed to save skills folder. See console for details."); - } - }, - [persistedFolder, settings.agentMode, validate] - ); - - /** Folder-picker icon button. Not wired yet — opens a notice telling the user to type the path. */ - const handlePickFolder = useCallback(() => { - new Notice("Folder picker is not available yet — type the vault-relative path for now."); - }, []); + }, [skillsFolder]); /** * Open a SKILL.md (absolute path) for editing. Managed skills live inside @@ -189,7 +143,7 @@ export const SkillsSettings: React.FC = () => { const filteredSkills = useMemo(() => filterSkills(skills, searchValue), [skills, searchValue]); - const displayFolder = persistedFolder.length > 0 ? persistedFolder : DEFAULT_SKILLS_FOLDER; + const displayFolder = skillsFolder; /** * Open the per-skill Properties modal. The modal owns its own save and @@ -264,56 +218,6 @@ export const SkillsSettings: React.FC = () => { - - - - Where Copilot keeps the shared copy of every skill. Agent shortcuts point here. - Changing this moves the folder and rewrites all shortcuts. - - {validationError !== null && ( - {validationError} - )} - - } - > -
- { - const next = e.target.value; - setDraft(next); - validate(next); - }} - onBlur={(e) => commit(e.target.value)} - onKeyDown={(e) => { - if (e.key === "Enter") { - e.preventDefault(); - commit(draft); - (e.target as HTMLInputElement).blur(); - } - }} - placeholder="copilot/skills" - className="!tw-w-56" - aria-label="Skills folder" - aria-invalid={validationError !== null} - /> - -
- - - {/* Durable banners — stack at the top of the tab body, above the toolbar. */} {(epermSeen || (syncBrand !== null && !syncBannerDismissed)) && (
diff --git a/src/commands/customCommandRegister.test.ts b/src/commands/customCommandRegister.test.ts new file mode 100644 index 00000000..9f8516b1 --- /dev/null +++ b/src/commands/customCommandRegister.test.ts @@ -0,0 +1,243 @@ +import { App, Plugin, Vault } from "obsidian"; +import { CustomCommandRegister } from "@/commands/customCommandRegister"; +import type { CustomCommand } from "@/commands/type"; +import type { CopilotSettings } from "@/settings/model"; + +jest.mock("obsidian", () => ({ + Plugin: jest.fn(), + Vault: jest.fn(), + TFile: jest.fn(), + normalizePath: (path: string) => path.replace(/\/+/g, "/").replace(/^\/|\/$/g, ""), +})); + +jest.mock("@/logger", () => ({ + logError: jest.fn(), +})); + +jest.mock("@/commands/CustomCommandChatModal", () => ({ + CustomCommandChatModal: jest.fn().mockImplementation(() => ({ open: jest.fn() })), +})); + +jest.mock("@/commands/customCommandManager", () => ({ + CustomCommandManager: { + getInstance: jest.fn(() => ({ recordUsage: jest.fn().mockResolvedValue(undefined) })), + }, +})); + +jest.mock("@/commands/customCommandUtils", () => ({ + getCommandId: (title: string) => `copilot-command-${title}`, + isCustomCommandFile: jest.fn(() => true), + loadAllCustomCommands: jest.fn().mockResolvedValue([]), + fetchAllCustomCommands: jest.fn(), + parseCustomCommandFile: jest.fn(), + getNextCustomCommandOrder: jest.fn(() => 0), + ensureCommandFrontmatter: jest.fn(), + hasOrderFrontmatter: jest.fn(() => true), +})); + +jest.mock("@/commands/state", () => ({ + deleteCachedCommand: jest.fn(), + getCachedCustomCommands: jest.fn(() => []), + isFileWritePending: jest.fn(() => false), + updateCachedCommand: jest.fn(), + updateCachedCommands: jest.fn(), +})); + +jest.mock("@/settings/model", () => ({ + getSettings: jest.fn(() => ({ copilotFolder: "copilot" })), + subscribeToSettingsChange: jest.fn().mockReturnValue(() => {}), +})); + +function command(title: string): CustomCommand { + return { + title, + content: "", + modelKey: "", + showInContextMenu: false, + showInSlashMenu: false, + order: 0, + lastUsedMs: 0, + }; +} + +function settingsWithRoot(copilotFolder: string): CopilotSettings { + return { copilotFolder } as CopilotSettings; +} + +describe("customCommandRegister", () => { + describe("CustomCommandRegister", () => { + describe("handleSettingsChange()", () => { + let settingsChangeHandler: (prev: CopilotSettings, next: CopilotSettings) => void; + let unsubscribe: jest.Mock; + let addCommand: jest.Mock; + let removeCommand: jest.Mock; + let fetchAllCustomCommands: jest.Mock; + let getCachedCustomCommands: jest.Mock; + let updateCachedCommands: jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers(); + + ({ fetchAllCustomCommands } = jest.requireMock("@/commands/customCommandUtils")); + ({ getCachedCustomCommands, updateCachedCommands } = jest.requireMock("@/commands/state")); + + unsubscribe = jest.fn(); + const { subscribeToSettingsChange } = jest.requireMock<{ + subscribeToSettingsChange: jest.Mock; + }>("@/settings/model"); + subscribeToSettingsChange.mockReturnValue(unsubscribe); + + addCommand = jest.fn(); + removeCommand = jest.fn(); + const mockPlugin = { addCommand, removeCommand, app: {} } as unknown as Plugin; + const mockVault = { on: jest.fn(), off: jest.fn() } as unknown as Vault; + + new CustomCommandRegister(mockPlugin, { vault: mockVault } as unknown as App); + + settingsChangeHandler = subscribeToSettingsChange.mock + .calls[0][0] as typeof settingsChangeHandler; + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it("reloads registrations when the derived folder changes because the root changed", async () => { + getCachedCustomCommands.mockReturnValue([command("Old")]); + fetchAllCustomCommands.mockResolvedValue([command("New")]); + + settingsChangeHandler(settingsWithRoot("copilot"), settingsWithRoot("team/ai")); + jest.advanceTimersByTime(1000); + await Promise.resolve(); + await Promise.resolve(); + + // Stale "Old" registration removed, "New" registered, cache replaced. + expect(removeCommand).toHaveBeenCalledWith("copilot-command-Old"); + expect(addCommand).toHaveBeenCalledWith( + expect.objectContaining({ id: "copilot-command-New", name: "New" }) + ); + expect(updateCachedCommands).toHaveBeenCalledWith([command("New")]); + }); + + it("does not reload when the root — and thus the derived folder — is unchanged", () => { + settingsChangeHandler(settingsWithRoot("copilot"), settingsWithRoot("copilot")); + jest.advanceTimersByTime(1000); + + expect(fetchAllCustomCommands).not.toHaveBeenCalled(); + }); + + it("discards a superseded reload so its stale commands never reach the cache", async () => { + let resolveStale: (value: CustomCommand[]) => void = () => {}; + const stalePromise = new Promise((r) => { + resolveStale = r; + }); + fetchAllCustomCommands + .mockReturnValueOnce(stalePromise) // request A (stale) + .mockResolvedValueOnce([command("Fresh")]); // request B (latest) + + // Request A: folder change copilot -> a + settingsChangeHandler(settingsWithRoot("copilot"), settingsWithRoot("a")); + jest.advanceTimersByTime(1000); + await Promise.resolve(); + + // Request B: folder change a -> b, resolves before A + settingsChangeHandler(settingsWithRoot("a"), settingsWithRoot("b")); + jest.advanceTimersByTime(1000); + await Promise.resolve(); + await Promise.resolve(); + + expect(updateCachedCommands).toHaveBeenCalledWith([command("Fresh")]); + updateCachedCommands.mockClear(); + + // Now the stale request A resolves — it must be discarded. + resolveStale([command("Stale")]); + await Promise.resolve(); + await Promise.resolve(); + + expect(updateCachedCommands).not.toHaveBeenCalled(); + }); + }); + + describe("cleanup()", () => { + it("unsubscribes from settings changes on teardown", () => { + const unsubscribe = jest.fn(); + const { subscribeToSettingsChange } = jest.requireMock<{ + subscribeToSettingsChange: jest.Mock; + }>("@/settings/model"); + subscribeToSettingsChange.mockReturnValue(unsubscribe); + + const mockPlugin = { + addCommand: jest.fn(), + removeCommand: jest.fn(), + app: {}, + } as unknown as Plugin; + const mockVault = { on: jest.fn(), off: jest.fn() } as unknown as Vault; + const register = new CustomCommandRegister(mockPlugin, { + vault: mockVault, + } as unknown as App); + + register.cleanup(); + + expect(unsubscribe).toHaveBeenCalledTimes(1); + }); + + it("discards an in-flight reload that resolves after teardown so it never registers commands or writes the cache", async () => { + jest.clearAllMocks(); + jest.useFakeTimers(); + try { + const { fetchAllCustomCommands } = jest.requireMock<{ + fetchAllCustomCommands: jest.Mock; + }>("@/commands/customCommandUtils"); + const { getCachedCustomCommands, updateCachedCommands } = jest.requireMock<{ + getCachedCustomCommands: jest.Mock; + updateCachedCommands: jest.Mock; + }>("@/commands/state"); + getCachedCustomCommands.mockReturnValue([]); + + let resolveFetch: (value: CustomCommand[]) => void = () => {}; + fetchAllCustomCommands.mockReturnValue( + new Promise((r) => { + resolveFetch = r; + }) + ); + + const { subscribeToSettingsChange } = jest.requireMock<{ + subscribeToSettingsChange: jest.Mock; + }>("@/settings/model"); + subscribeToSettingsChange.mockReturnValue(() => {}); + + const addCommand = jest.fn(); + const removeCommand = jest.fn(); + const mockPlugin = { addCommand, removeCommand, app: {} } as unknown as Plugin; + const mockVault = { on: jest.fn(), off: jest.fn() } as unknown as Vault; + const register = new CustomCommandRegister(mockPlugin, { + vault: mockVault, + } as unknown as App); + + const settingsChangeHandler = subscribeToSettingsChange.mock.calls[0][0] as ( + prev: CopilotSettings, + next: CopilotSettings + ) => void; + + // Kick off a reload; its fetch stays pending across teardown. + settingsChangeHandler(settingsWithRoot("copilot"), settingsWithRoot("team/ai")); + jest.advanceTimersByTime(1000); + await Promise.resolve(); + + register.cleanup(); + + // The fetch resolves only after teardown; the disposed guard must drop it. + resolveFetch([command("Late")]); + await Promise.resolve(); + await Promise.resolve(); + + expect(addCommand).not.toHaveBeenCalled(); + expect(updateCachedCommands).not.toHaveBeenCalled(); + } finally { + jest.useRealTimers(); + } + }); + }); + }); +}); diff --git a/src/commands/customCommandRegister.ts b/src/commands/customCommandRegister.ts index c2833332..2a2c9c7a 100644 --- a/src/commands/customCommandRegister.ts +++ b/src/commands/customCommandRegister.ts @@ -2,6 +2,7 @@ import { getCommandId, isCustomCommandFile, loadAllCustomCommands, + fetchAllCustomCommands, parseCustomCommandFile, getNextCustomCommandOrder, ensureCommandFrontmatter, @@ -16,8 +17,11 @@ import { getCachedCustomCommands, isFileWritePending, updateCachedCommand, + updateCachedCommands, } from "@/commands/state"; import { CustomCommandManager } from "@/commands/customCommandManager"; +import { getSettings, subscribeToSettingsChange } from "@/settings/model"; +import { deriveCustomPromptsFolder } from "@/settings/copilotFolder"; import { logError } from "@/logger"; /** This manager is used to register custom commands as obsidian commands */ @@ -25,6 +29,11 @@ export class CustomCommandRegister { private plugin: Plugin; private app: App; private vault: Vault; + private settingsUnsubscriber?: () => void; + /** Monotonic request id for latest-wins semantics on folder change. */ + private folderChangeRequestId = 0; + /** Set once {@link cleanup} runs so an in-flight reload cannot touch a torn-down instance. */ + private disposed = false; constructor(plugin: Plugin, app: App) { this.plugin = plugin; @@ -53,6 +62,13 @@ export class CustomCommandRegister { * Clean up resources used by the cache */ cleanup() { + // Reason: mark disposed and bump the request id so any reload awaiting a + // fetch bails out before it can register commands or write the module-level + // cache after teardown. + this.disposed = true; + ++this.folderChangeRequestId; + this.debouncedFolderChange.cancel(); + this.settingsUnsubscriber?.(); this.vault.off("create", this.handleFileCreation); this.vault.off("delete", this.handleFileDeletion); this.vault.off("rename", this.handleFileRename); @@ -64,6 +80,92 @@ export class CustomCommandRegister { this.vault.on("delete", this.handleFileDeletion); this.vault.on("rename", this.handleFileRename); this.vault.on("modify", this.handleFileModify); + this.settingsUnsubscriber = subscribeToSettingsChange(this.handleSettingsChange); + } + + /** + * React to a change in the derived custom-commands folder (driven by the + * configurable copilotFolder root). Reloads command registrations from the + * new folder while old files stay in place. + */ + private handleSettingsChange = ( + prev: ReturnType, + next: ReturnType + ): void => { + // Reason: the folder is derived from copilotFolder; compare derived paths. + if (deriveCustomPromptsFolder(prev) !== deriveCustomPromptsFolder(next)) { + this.debouncedFolderChange(); + } + }; + + /** Debounced folder-change reload (avoid rapid-fire while the root is edited). */ + private debouncedFolderChange = debounce( + () => { + void this.handleFolderChange(); + }, + 1000, + { leading: false, trailing: true } + ); + + /** + * Reload command registrations after the folder changes, using a pure fetch + * followed by an atomic swap so a superseded reload cannot register commands + * from a stale folder or clobber a newer cache. + * + * Reason: `loadAllCustomCommands` writes the global cache before returning, so + * an in-flight stale reload would pollute the cache even behind a request-id + * guard. Fetching without touching the cache, then checking the generation + * before mutating registrations, keeps only the latest reload's effect. + * + * DESIGN NOTE (scan-race, accepted): if a `handleFileCreation` registers a new + * command file and writes the cache while this fetch is in flight, the fetch + * snapshot predates that command, so the atomic-swap phase treats it as stale, + * removes its registration, and `updateCachedCommands` overwrites it out of the + * cache. + * (a) Trigger: a command file created concurrently inside the change-root + * debounce + fetch window. + * (b) Consequence: that command is briefly cleared by this swap; the next vault + * event targeting the same file (modify/create/delete) re-registers it, so + * the state self-heals. + * (c) Why not fixed: a real fix needs a vault-mutation generation counter; that + * machinery is disproportionate to a window that is both narrow (root change + * overlapping a concurrent command-file create) and self-healing. + * (d) If a future review flags this again, point them at this note. + */ + private async handleFolderChange(): Promise { + const currentRequestId = ++this.folderChangeRequestId; + + try { + const nextCommands = await fetchAllCustomCommands(this.app); + + // Bail out if the instance was torn down while this fetch was in flight; + // registering commands or writing the cache post-teardown would leak state. + if (this.disposed) return; + + // Latest-wins: a newer folder change started while this fetch was in flight. + if (currentRequestId !== this.folderChangeRequestId) return; + + // Atomically swap registrations: drop commands no longer present, then + // (re)register the current set, then replace the cache in one step. + const nextTitles = new Set(nextCommands.map((command) => command.title)); + for (const stale of getCachedCustomCommands()) { + if (!nextTitles.has(stale.title)) { + this.removeRegisteredCommand(stale.title); + } + } + nextCommands.forEach((command) => this.registerCommand(command)); + updateCachedCommands(nextCommands); + } catch (error) { + // Latest-wins guard: ignore a stale failure resolved after a newer reload. + if (currentRequestId !== this.folderChangeRequestId) return; + logError("Error reloading custom commands after folder change", error); + } + } + + /** Remove a registered obsidian command by its command title. */ + private removeRegisteredCommand(title: string): void { + const commandId = getCommandId(title); + (this.plugin as unknown as { removeCommand: (id: string) => void }).removeCommand(commandId); } private handleFileModify = debounce( diff --git a/src/commands/customCommandUtils.ts b/src/commands/customCommandUtils.ts index 24df6539..e1abf582 100644 --- a/src/commands/customCommandUtils.ts +++ b/src/commands/customCommandUtils.ts @@ -9,8 +9,9 @@ import { } from "@/commands/constants"; import { CustomCommand } from "@/commands/type"; import { logWarn } from "@/logger"; -import { App, normalizePath, Notice, TAbstractFile, TFile, Vault } from "obsidian"; +import { App, Notice, TAbstractFile, TFile, Vault } from "obsidian"; import { getSettings } from "@/settings/model"; +import { getEffectiveCustomPromptsFolder } from "@/settings/copilotFolder"; import { updateCachedCommands, getCachedCustomCommands, @@ -76,7 +77,7 @@ export function getCommandId(commandName: string) { } export function getCustomCommandsFolder(): string { - return normalizePath(getSettings().customPromptsFolder); + return getEffectiveCustomPromptsFolder(); } export function getCommandFilePath(title: string): string { @@ -129,11 +130,18 @@ export async function parseCustomCommandFile(app: App, file: TFile): Promise { +/** + * Fetch all custom commands from the vault WITHOUT writing the global cache. + * Use this when the caller must coordinate cache writes itself (e.g. latest-wins + * folder reloads) so a superseded async reload cannot clobber a newer result. + */ +export async function fetchAllCustomCommands(app: App): Promise { const files = app.vault.getFiles().filter((file) => isCustomCommandFile(file)); - const commands: CustomCommand[] = await Promise.all( - files.map((file) => parseCustomCommandFile(app, file)) - ); + return await Promise.all(files.map((file) => parseCustomCommandFile(app, file))); +} + +export async function loadAllCustomCommands(app: App): Promise { + const commands = await fetchAllCustomCommands(app); updateCachedCommands(commands); return commands; } diff --git a/src/commands/index.ts b/src/commands/index.ts index 0f6cde9c..4b761b58 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -27,6 +27,7 @@ import { getSearchBackend } from "@/miyo/miyoUtils"; import { getAllQAMarkdownContent } from "@/search/searchUtils"; import { NoteSelectedTextContext, WebSelectedTextContext } from "@/types/message"; import { ensureFolderExists, isSourceModeOn } from "@/utils"; +import { getEffectiveCopilotFolder } from "@/settings/copilotFolder"; import { isDesktopRuntime } from "@/utils/desktopRuntime"; import { Editor, MarkdownView, Notice, TFile } from "obsidian"; import { v4 as uuidv4 } from "uuid"; @@ -395,7 +396,7 @@ export function registerCommands(plugin: CopilotPlugin) { // Create or update the file in the vault const fileName = `Copilot-Indexed-Files-${new Date().toLocaleDateString().replace(/\//g, "-")}.md`; - const folderPath = "copilot"; + const folderPath = getEffectiveCopilotFolder(); const filePath = `${folderPath}/${fileName}`; // Ensure destination folder exists (supports mobile and nested) @@ -470,7 +471,7 @@ export function registerCommands(plugin: CopilotPlugin) { // Create the debug file const fileName = `Copilot-Embedding-Debug-${activeFile.basename.replace(/[\\/:*?"<>|]/g, "_")}.md`; - const folderPath = "copilot"; + const folderPath = getEffectiveCopilotFolder(); const filePath = `${folderPath}/${fileName}`; await ensureFolderExists(plugin.app.vault, folderPath); diff --git a/src/components/chat-components/hooks/useAtMentionSearch.ts b/src/components/chat-components/hooks/useAtMentionSearch.ts index d2653b2d..cf7bc83c 100644 --- a/src/components/chat-components/hooks/useAtMentionSearch.ts +++ b/src/components/chat-components/hooks/useAtMentionSearch.ts @@ -16,7 +16,7 @@ import { CategoryOption, EMPTY_AGENT_MENTION_BRANDS, } from "./useAtMentionCategories"; -import { getSettings } from "@/settings/model"; +import { getEffectiveCustomPromptsFolder } from "@/settings/copilotFolder"; import { SelfHostCloudWarningIcon } from "@/components/ui/SelfHostCloudWarningIcon"; // Maximum number of results to show in @ mention search @@ -273,7 +273,7 @@ export function useAtMentionSearch( if (!query) { // For notes category with no query, rank custom command notes lower if (selectedCategory === "notes") { - const customPromptsFolder = getSettings().customPromptsFolder; + const customPromptsFolder = getEffectiveCustomPromptsFolder(); const regularNotes = items.filter( (item) => !( diff --git a/src/components/chat-components/hooks/useNoteSearch.ts b/src/components/chat-components/hooks/useNoteSearch.ts index 6354803b..c71c585e 100644 --- a/src/components/chat-components/hooks/useNoteSearch.ts +++ b/src/components/chat-components/hooks/useNoteSearch.ts @@ -4,7 +4,7 @@ import { FileText, FileClock } from "lucide-react"; import fuzzysort from "fuzzysort"; import { useAllNotes } from "./useAllNotes"; import { TypeaheadOption } from "@/components/chat-components/TypeaheadMenuContent"; -import { getSettings } from "@/settings/model"; +import { getEffectiveCustomPromptsFolder } from "@/settings/copilotFolder"; export interface NoteSearchOption extends TypeaheadOption { file: TFile; @@ -61,7 +61,7 @@ export function useNoteSearch( // Filter and search notes based on query (name only) const searchResults = useMemo(() => { const mergedConfig = { ...DEFAULT_CONFIG, ...config }; - const customPromptsFolder = getSettings().customPromptsFolder; + const customPromptsFolder = getEffectiveCustomPromptsFolder(); // If no query, return first N notes with custom command notes ranked lower if (!query.trim()) { diff --git a/src/constants.ts b/src/constants.ts index 191086cb..876192f8 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -46,6 +46,9 @@ export const AI_SENDER = "ai"; // Default folder names export const COPILOT_FOLDER_ROOT = "copilot"; +// Configurable root all Copilot sub-folders derive from (PR-乙). Defaults to +// the historical hardcoded root so existing vaults keep their layout. +export const DEFAULT_COPILOT_FOLDER = COPILOT_FOLDER_ROOT; const DEFAULT_CHAT_HISTORY_FOLDER = `${COPILOT_FOLDER_ROOT}/copilot-conversations`; const DEFAULT_CUSTOM_PROMPTS_FOLDER = `${COPILOT_FOLDER_ROOT}/copilot-custom-prompts`; const DEFAULT_MEMORY_FOLDER = `${COPILOT_FOLDER_ROOT}/memory`; @@ -53,6 +56,12 @@ const DEFAULT_SYSTEM_PROMPTS_FOLDER = `${COPILOT_FOLDER_ROOT}/system-prompts`; const DEFAULT_PROJECTS_FOLDER = `${COPILOT_FOLDER_ROOT}/projects`; export const DEFAULT_SKILLS_FOLDER = `${COPILOT_FOLDER_ROOT}/skills`; const DEFAULT_CONVERTED_DOC_OUTPUT_FOLDER = ""; +// Built-in tag written into every saved conversation's frontmatter. Frozen as a +// constant (no longer user-configurable): no code reads the tag to identify +// conversations — they are keyed by folder + epoch — so it only serves as a +// stable, filterable marker. The legacy `defaultConversationTag` setting is kept +// as a dead field for schema compatibility but is no longer read. +export const COPILOT_CONVERSATION_TAG = "copilot-conversation"; export const DEFAULT_QA_EXCLUSIONS_SETTING = COPILOT_FOLDER_ROOT; export const DEFAULT_SYSTEM_PROMPT = `You are Obsidian Copilot, a helpful assistant that integrates AI to Obsidian note-taking. 1. Never mention that you do not have access to something. Always rely on the user provided context. @@ -993,6 +1002,14 @@ export const DEFAULT_SETTINGS: CopilotSettings = { openAIProxyBaseUrl: "", openAIEmbeddingProxyBaseUrl: "", stream: true, + copilotFolder: DEFAULT_COPILOT_FOLDER, + // Every folder ever activated as the Copilot root (seeded with the legacy + // root in the v8 migration). Kept append-only so each historical root stays + // permanently excluded from QA indexing even after the root is changed. + copilotRootHistory: [], + // True only when a legacy (v1-v7) vault was migrated to v8; WS-D reads it to + // decide whether to show the one-time folder-relocation prompt, then clears it. + upgradedToV8FromLegacy: false, defaultSaveFolder: DEFAULT_CHAT_HISTORY_FOLDER, defaultConversationTag: "copilot-conversation", autosaveChat: true, diff --git a/src/core/ChatPersistenceManager.test.ts b/src/core/ChatPersistenceManager.test.ts index 447902c2..0c06df9e 100644 --- a/src/core/ChatPersistenceManager.test.ts +++ b/src/core/ChatPersistenceManager.test.ts @@ -4,6 +4,7 @@ import { App, Notice, TFile } from "obsidian"; import type { MessageRepository } from "./MessageRepository"; import { ChatPersistenceManager } from "./ChatPersistenceManager"; import { mockTFile } from "@/__tests__/mockObsidian"; +import { getSettings } from "@/settings/model"; const USER_SENDER = "user"; const AI_SENDER = "ai"; @@ -29,6 +30,9 @@ jest.mock("@/settings/model", () => ({ defaultConversationNoteName: "{$topic}@{$date}_{$time}", }), })); +jest.mock("@/settings/copilotFolder", () => ({ + getEffectiveConversationsFolder: jest.fn(() => "test-folder"), +})); jest.mock("@/aiParams", () => ({ getCurrentProject: jest.fn().mockReturnValue(null), })); @@ -1589,6 +1593,35 @@ tags: }); }); + describe("frozen conversation tag", () => { + it("writes the built-in tag independent of the persisted defaultConversationTag", () => { + // The tag is frozen to a constant. Feed a custom setting value: if a + // regression made generateNoteContent read the setting again, the custom + // value would leak into the note and fail the assertion below. + const gs = getSettings as jest.Mock; + gs.mockReturnValue({ + defaultSaveFolder: "test-folder", + defaultConversationTag: "user-custom-tag", + defaultConversationNoteName: "{$topic}@{$date}_{$time}", + }); + try { + const noteContent = asInternal(persistenceManager).generateNoteContent( + "**user**: hi", + 1695513480000, + "gpt-4" + ); + expect(noteContent).toContain("tags:\n - copilot-conversation"); + expect(noteContent).not.toContain("user-custom-tag"); + } finally { + gs.mockReturnValue({ + defaultSaveFolder: "test-folder", + defaultConversationTag: "copilot-conversation", + defaultConversationNoteName: "{$topic}@{$date}_{$time}", + }); + } + }); + }); + describe("lastAccessedAt preservation", () => { it("should preserve lastAccessedAt when updating existing file", async () => { const messages: ChatMessage[] = [ diff --git a/src/core/ChatPersistenceManager.ts b/src/core/ChatPersistenceManager.ts index dd741bf5..8e63fe92 100644 --- a/src/core/ChatPersistenceManager.ts +++ b/src/core/ChatPersistenceManager.ts @@ -1,11 +1,12 @@ import { getCurrentProject, ProjectConfig } from "@/aiParams"; -import { AI_SENDER, USER_SENDER } from "@/constants"; +import { AI_SENDER, COPILOT_CONVERSATION_TAG, USER_SENDER } from "@/constants"; import ChainManager from "@/LLMProviders/chainManager"; import { parseReasoningBlock } from "@/LLMProviders/chainRunner/utils/AgentReasoningState"; import { logError, logInfo, logWarn } from "@/logger"; import { sanitizeVaultPathSegment } from "@/projects/projectUtils"; import { filterChatHistoryFiles, readChatPathProjectId } from "@/utils/chatHistoryUtils"; import { getSettings } from "@/settings/model"; +import { getEffectiveConversationsFolder } from "@/settings/copilotFolder"; import { ChatMessage, MessageContext } from "@/types/message"; import { ensureFolderExists, @@ -62,12 +63,11 @@ export class ChatPersistenceManager { return; } - const settings = getSettings(); const chatContent = this.formatChatContent(messages); const firstMessageEpoch = messages[0].timestamp?.epoch || Date.now(); // Ensure the save folder exists (supports nested paths) using utility helper. - await ensureFolderExists(this.app.vault, settings.defaultSaveFolder); + await ensureFolderExists(this.app.vault, getEffectiveConversationsFolder()); // Check if a file with this epoch already exists const existingFile = await this.findFileByEpoch(firstMessageEpoch); @@ -153,7 +153,7 @@ export class ChatPersistenceManager { const currentProjectId = currentProject?.id; if (currentProjectId && conflictProjectId !== currentProjectId) { // Different project owns this file — generate a unique name instead - const rawUniqueName = `${settings.defaultSaveFolder}/${sanitizeVaultPathSegment(currentProjectId)}__chat-${firstMessageEpoch}.md`; + const rawUniqueName = `${getEffectiveConversationsFolder()}/${sanitizeVaultPathSegment(currentProjectId)}__chat-${firstMessageEpoch}.md`; const uniqueName = await this.resolveChatSavePath(rawUniqueName, currentProject); targetFile = await this.app.vault.create(uniqueName, noteContent); new Notice(`Chat saved as note: ${uniqueName}`); @@ -204,7 +204,7 @@ export class ChatPersistenceManager { const filePrefix = fallbackProject ? `${sanitizeVaultPathSegment(fallbackProject.id)}__` : ""; - const rawFallbackName = `${settings.defaultSaveFolder}/${filePrefix}chat-${firstMessageEpoch}.md`; + const rawFallbackName = `${getEffectiveConversationsFolder()}/${filePrefix}chat-${firstMessageEpoch}.md`; // Reason: check ownership to prevent cross-project collision on fallback path const fallbackName = await this.resolveChatSavePath(rawFallbackName, currentProject); @@ -325,8 +325,7 @@ export class ChatPersistenceManager { * Get all chat history files from the vault */ async getChatHistoryFiles(): Promise { - const settings = getSettings(); - const folderFiles = await listMarkdownFiles(this.app, settings.defaultSaveFolder); + const folderFiles = await listMarkdownFiles(this.app, getEffectiveConversationsFolder()); if (folderFiles.length === 0) return []; const currentProject = getCurrentProject(); @@ -776,10 +775,10 @@ ${conversationSummary}`; // If still too long, truncate the entire filename more aggressively const availableForBasename = SAFE_FILENAME_BYTE_LIMIT - extensionBytes - filePrefixBytes; const truncatedBasename = truncateToByteLimit(sanitizedFileName, availableForBasename); - return `${settings.defaultSaveFolder}/${filePrefix}${truncatedBasename}.md`; + return `${getEffectiveConversationsFolder()}/${filePrefix}${truncatedBasename}.md`; } - return `${settings.defaultSaveFolder}/${baseNameWithPrefix}`; + return `${getEffectiveConversationsFolder()}/${baseNameWithPrefix}`; } /** @@ -792,7 +791,6 @@ ${conversationSummary}`; topic?: string, lastAccessedAt?: number ): string { - const settings = getSettings(); const currentProject = getCurrentProject(); return `--- @@ -803,7 +801,7 @@ ${lastAccessedAt ? `lastAccessedAt: ${lastAccessedAt}` : ""} ${currentProject ? `projectId: "${escapeYamlString(currentProject.id)}"` : ""} ${currentProject ? `projectName: "${escapeYamlString(currentProject.name)}"` : ""} tags: - - ${settings.defaultConversationTag} + - ${COPILOT_CONVERSATION_TAG} --- ${chatContent}`; diff --git a/src/logFileManager.ts b/src/logFileManager.ts index 104f6ecf..eb9a68e0 100644 --- a/src/logFileManager.ts +++ b/src/logFileManager.ts @@ -2,13 +2,14 @@ import { err2String } from "@/errorFormat"; import { App, TFile } from "obsidian"; import { ensureFolderExists } from "@/utils"; import { getSettings } from "@/settings/model"; +import { getEffectiveCopilotFolder } from "@/settings/copilotFolder"; import { isSensitiveKey } from "@/encryptionService"; type LogLevel = "INFO" | "WARN" | "ERROR"; /** * Manages a rolling log file that keeps the last N entries and works on desktop and mobile. - * - Writes to /copilot/copilot-log.md + * - Writes to //copilot-log.md * - Maintains an in-memory ring buffer of the last 500 entries * - Debounced flush to reduce I/O; single-line entries to preserve accurate line limits */ @@ -39,7 +40,8 @@ class LogFileManager { } getLogPath(): string { - return "copilot/copilot-log.md"; // under copilot/ + // Under the configurable copilot root folder. + return `${getEffectiveCopilotFolder()}/copilot-log.md`; } /** Ensure the log manager is initialized. Always starts with an empty buffer. */ diff --git a/src/main.ts b/src/main.ts index 84d17a69..fe4aa055 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,4 +1,5 @@ import type { AgentSessionManager } from "@/agentMode"; +import React from "react"; // Deep import (not the barrel): these run on the load path for every // platform, and the barrel pulls Node-only modules that crash mobile. import { isNativeChatId, parseNativeChatId } from "@/utils/nativeChatId"; @@ -15,6 +16,7 @@ import { registerCommands } from "@/commands"; import CopilotView from "@/components/CopilotView"; import RelevantNotesView from "@/components/RelevantNotesView"; import { APPLY_VIEW_TYPE, ApplyView } from "@/components/composer/ApplyView"; +import { ConfirmModal } from "@/components/modals/ConfirmModal"; import { LoadChatHistoryModal } from "@/components/modals/LoadChatHistoryModal"; import { registerContextMenu } from "@/commands/contextMenu"; @@ -71,7 +73,11 @@ import { getSettings, setSettings, subscribeToSettingsChange, + updateSetting, } from "@/settings/model"; +import { ensureCopilotSubfolders, getEffectiveConversationsFolder } from "@/settings/copilotFolder"; +import { buildUpgradeRelocationEntries } from "@/settings/upgradeNotice"; +import { UpgradeRelocationNotice } from "@/settings/UpgradeRelocationNotice"; import { dehydrateDeviceProfile, hydrateDeviceProfile } from "@/settings/deviceProfiles"; import { getDeviceId } from "@/utils/deviceId"; import { isDesktopRuntime } from "@/utils/desktopRuntime"; @@ -416,6 +422,8 @@ export default class CopilotPlugin extends Plugin { void this.systemPromptRegister .initialize() .then(() => migrateSystemPromptsFromSettings(this.app)); + + void this.notifyLegacyUpgradeRelocation(); }); // Initialize automatic selection handler @@ -425,6 +433,38 @@ export default class CopilotPlugin extends Plugin { this.initWebSelectionWatcher(); } + /** + * One-time guidance for users upgrading a legacy (v1-v7) vault whose Copilot + * data needs relocating (a sub-folder was customized, or the root itself + * moved). v4 consolidated every data folder under a single derived root, so + * Copilot now reads and writes the derived locations while their old files + * stay put. This shows them the old→new paths + * and asks them to move files manually; per the maintainer decision it never + * moves files itself. The flag is cleared afterwards (whether or not the notice + * is shown) so the check runs once. A failed clear-write only repeats the + * one-time check on the next restart, which is idempotent. + */ + private async notifyLegacyUpgradeRelocation(): Promise { + if (!getSettings().upgradedToV8FromLegacy) return; + + const entries = buildUpgradeRelocationEntries(getSettings()); + if (entries.length > 0) { + // Pre-create the derived sub-folders so the destinations the notice points + // at already exist when the user goes to move their files there. + await ensureCopilotSubfolders(this.app.vault, getSettings()); + new ConfirmModal( + this.app, + () => {}, + React.createElement(UpgradeRelocationNotice, { entries }), + "", + "OK", + "" + ).open(); + } + + updateSetting("upgradedToV8FromLegacy", false); + } + /** * Register a view, tolerating a type that is already registered. Obsidian * throws "Attempting to register an existing view type" when a prior plugin @@ -1054,7 +1094,7 @@ export default class CopilotPlugin extends Plugin { } async getChatHistoryFiles(): Promise { - const folderFiles = await listMarkdownFiles(this.app, getSettings().defaultSaveFolder); + const folderFiles = await listMarkdownFiles(this.app, getEffectiveConversationsFolder()); if (folderFiles.length === 0) return []; const currentProject = getCurrentProject(); diff --git a/src/memory/UserMemoryManager.test.ts b/src/memory/UserMemoryManager.test.ts index 4523b37e..e88d3e35 100644 --- a/src/memory/UserMemoryManager.test.ts +++ b/src/memory/UserMemoryManager.test.ts @@ -17,6 +17,10 @@ import { UserMemoryManager } from "./UserMemoryManager"; import { App, TFile, Vault } from "obsidian"; import { ChatMessage } from "@/types/message"; import { logError, logWarn } from "@/logger"; +jest.mock("@/settings/copilotFolder", () => ({ + getEffectiveMemoryFolder: jest.fn(() => "copilot/memory"), +})); + import { CopilotSettings, getSettings } from "@/settings/model"; import { ensureFolderExists } from "@/utils"; import { BaseChatModel } from "@langchain/core/language_models/chat_models"; diff --git a/src/memory/UserMemoryManager.ts b/src/memory/UserMemoryManager.ts index c9f3114a..2a03dd9b 100644 --- a/src/memory/UserMemoryManager.ts +++ b/src/memory/UserMemoryManager.ts @@ -2,6 +2,7 @@ import { App, TFile } from "obsidian"; import { ChatMessage } from "@/types/message"; import { logInfo, logError, logWarn } from "@/logger"; import { getSettings } from "@/settings/model"; +import { getEffectiveMemoryFolder } from "@/settings/copilotFolder"; import { ensureFolderExists } from "@/utils"; import { BaseChatModel } from "@langchain/core/language_models/chat_models"; import { HumanMessage, SystemMessage } from "@langchain/core/messages"; @@ -225,20 +226,15 @@ export class UserMemoryManager { * Ensure the user memory folder exists */ private async ensureMemoryFolderExists(): Promise { - const settings = getSettings(); - const memoryFolderPath = settings.memoryFolderName; - - await ensureFolderExists(this.app.vault, memoryFolderPath); + await ensureFolderExists(this.app.vault, getEffectiveMemoryFolder()); } private getRecentConversationFilePath(): string { - const settings = getSettings(); - return `${settings.memoryFolderName}/Recent Conversations.md`; + return `${getEffectiveMemoryFolder()}/Recent Conversations.md`; } public getSavedMemoriesFilePath(): string { - const settings = getSettings(); - return `${settings.memoryFolderName}/Saved Memories.md`; + return `${getEffectiveMemoryFolder()}/Saved Memories.md`; } /** diff --git a/src/projects/ensureAgentsMirror.test.ts b/src/projects/ensureAgentsMirror.test.ts index 2895682c..f6ddb970 100644 --- a/src/projects/ensureAgentsMirror.test.ts +++ b/src/projects/ensureAgentsMirror.test.ts @@ -16,6 +16,13 @@ jest.mock("@/settings/model", () => ({ getSettings: jest.fn(() => ({ projectsFolder: "copilot-projects" })), })); +// getProjectsFolder derives from copilotFolder in production; shim the derived +// accessor to the folder these tests configure via getSettings().projectsFolder. +jest.mock("@/settings/copilotFolder", () => { + const { getSettings } = jest.requireMock("@/settings/model"); + return { getEffectiveProjectsFolder: jest.fn(() => getSettings().projectsFolder) }; +}); + jest.mock("@/projects/state", () => ({ addPendingFileWrite: jest.fn(), removePendingFileWrite: jest.fn(), diff --git a/src/projects/projectPaths.test.ts b/src/projects/projectPaths.test.ts index 3c501c03..31daace7 100644 --- a/src/projects/projectPaths.test.ts +++ b/src/projects/projectPaths.test.ts @@ -5,8 +5,8 @@ import { } from "@/projects/projectPaths"; import { mockTFile } from "@/__tests__/mockObsidian"; -jest.mock("@/settings/model", () => ({ - getSettings: jest.fn(() => ({ projectsFolder: "copilot-projects" })), +jest.mock("@/settings/copilotFolder", () => ({ + getEffectiveProjectsFolder: jest.fn(() => "copilot-projects"), })); describe("getProjectConfigFilePath — single source of truth", () => { diff --git a/src/projects/projectPaths.ts b/src/projects/projectPaths.ts index 9d3758b2..13396d1c 100644 --- a/src/projects/projectPaths.ts +++ b/src/projects/projectPaths.ts @@ -1,13 +1,13 @@ import { PROJECT_CONFIG_FILE_NAME, PROJECTS_UNSUPPORTED_FOLDER_NAME } from "@/projects/constants"; -import { getSettings } from "@/settings/model"; +import { getEffectiveProjectsFolder } from "@/settings/copilotFolder"; import { normalizePath, TAbstractFile, TFile, Vault } from "obsidian"; /** - * Get the projects root folder path from settings. + * Get the projects root folder path, derived from the configurable copilotFolder root. * @returns Normalized vault path */ export function getProjectsFolder(): string { - return normalizePath(getSettings().projectsFolder); + return getEffectiveProjectsFolder(); } /** diff --git a/src/projects/projectRegister.test.ts b/src/projects/projectRegister.test.ts new file mode 100644 index 00000000..0780a43e --- /dev/null +++ b/src/projects/projectRegister.test.ts @@ -0,0 +1,117 @@ +import { App, Vault } from "obsidian"; +import { ProjectRegister } from "@/projects/projectRegister"; +import type { CopilotSettings } from "@/settings/model"; + +jest.mock("obsidian", () => ({ + Notice: jest.fn(), + Vault: jest.fn(), + normalizePath: (path: string) => path.replace(/\/+/g, "/").replace(/^\/|\/$/g, ""), +})); + +jest.mock("@/logger", () => ({ + logInfo: jest.fn(), + logError: jest.fn(), + logWarn: jest.fn(), +})); + +jest.mock("@/projects/ProjectFileManager", () => { + const instance = { fetchProjects: jest.fn().mockResolvedValue([]), initialize: jest.fn() }; + return { ProjectFileManager: { getInstance: jest.fn(() => instance) } }; +}); + +jest.mock("@/projects/projectUtils", () => ({ + ensureProjectFrontmatter: jest.fn(), + getProjectsFolder: jest.fn(() => "copilot/projects"), + isProjectConfigFile: jest.fn(() => false), + parseProjectConfigFile: jest.fn(), + loadAllProjects: jest.fn(), +})); + +jest.mock("@/projects/state", () => ({ + deleteCachedProjectRecordByFilePath: jest.fn(), + getCachedProjectRecordByFilePath: jest.fn(), + getCachedProjectRecordById: jest.fn(), + getCachedProjectRecords: jest.fn(() => []), + isPendingFileWrite: jest.fn(() => false), + replaceCachedProjectRecordByFilePath: jest.fn(), + updateCachedProjectRecords: jest.fn(), + upsertCachedProjectRecord: jest.fn(), +})); + +jest.mock("@/aiParams", () => ({ + getCurrentProject: jest.fn(() => null), +})); + +jest.mock("@/cache/projectContextCache", () => ({ + ProjectContextCache: { + getInstance: jest.fn(() => ({ clearForProject: jest.fn().mockResolvedValue(undefined) })), + }, +})); + +jest.mock("@/settings/model", () => ({ + getSettings: jest.fn(() => ({ copilotFolder: "copilot" })), + subscribeToSettingsChange: jest.fn().mockReturnValue(() => {}), +})); + +/** Build a settings object carrying only the root the watcher reads. */ +function settingsWithRoot(copilotFolder: string): CopilotSettings { + return { copilotFolder } as CopilotSettings; +} + +describe("projectRegister", () => { + describe("ProjectRegister", () => { + describe("handleSettingsChange()", () => { + let settingsChangeHandler: (prev: CopilotSettings, next: CopilotSettings) => void; + let fetchProjects: jest.Mock; + let updateCachedProjectRecords: jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + jest.useFakeTimers(); + + const { ProjectFileManager } = jest.requireMock<{ + ProjectFileManager: { getInstance: () => { fetchProjects: jest.Mock } }; + }>("@/projects/ProjectFileManager"); + fetchProjects = ProjectFileManager.getInstance().fetchProjects; + fetchProjects.mockResolvedValue([]); + + ({ updateCachedProjectRecords } = jest.requireMock<{ + updateCachedProjectRecords: jest.Mock; + }>("@/projects/state")); + + const mockVault = { on: jest.fn(), off: jest.fn() } as unknown as Vault; + const register = new ProjectRegister({ vault: mockVault } as unknown as App); + void register.initialize(); + + const { subscribeToSettingsChange } = jest.requireMock<{ + subscribeToSettingsChange: jest.Mock; + }>("@/settings/model"); + settingsChangeHandler = subscribeToSettingsChange.mock + .calls[0][0] as typeof settingsChangeHandler; + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it("reloads projects when the derived folder changes because the root changed", async () => { + settingsChangeHandler(settingsWithRoot("copilot"), settingsWithRoot("team/ai")); + + jest.advanceTimersByTime(1000); + await Promise.resolve(); + await Promise.resolve(); + + expect(fetchProjects).toHaveBeenCalledTimes(1); + expect(updateCachedProjectRecords).toHaveBeenCalledWith([]); + }); + + it("does not reload when the root — and thus the derived folder — is unchanged", () => { + settingsChangeHandler(settingsWithRoot("copilot"), settingsWithRoot("copilot")); + + jest.advanceTimersByTime(1000); + + expect(fetchProjects).not.toHaveBeenCalled(); + }); + }); + }); +}); diff --git a/src/projects/projectRegister.ts b/src/projects/projectRegister.ts index 9700fd55..5a13c38f 100644 --- a/src/projects/projectRegister.ts +++ b/src/projects/projectRegister.ts @@ -21,6 +21,7 @@ import { import { loadAllProjects } from "@/projects/projectUtils"; import { PROJECT_CONFIG_FILE_NAME, PROJECTS_UNSUPPORTED_FOLDER_NAME } from "@/projects/constants"; import { getSettings, subscribeToSettingsChange } from "@/settings/model"; +import { deriveProjectsFolder } from "@/settings/copilotFolder"; import { debounce } from "@/utils/debounce"; import { App, Notice, TAbstractFile, Vault } from "obsidian"; @@ -100,8 +101,11 @@ export class ProjectRegister { prev: ReturnType, next: ReturnType ): void => { - if (prev.projectsFolder !== next.projectsFolder) { - this.debouncedFolderChange(next.projectsFolder); + // Reason: the folder is derived from the configurable copilotFolder root, so + // compare the derived paths rather than the retired projectsFolder field. + const nextFolder = deriveProjectsFolder(next); + if (deriveProjectsFolder(prev) !== nextFolder) { + this.debouncedFolderChange(nextFolder); } }; diff --git a/src/projects/projectUtils.test.ts b/src/projects/projectUtils.test.ts index d5c1ede2..043bc1d8 100644 --- a/src/projects/projectUtils.test.ts +++ b/src/projects/projectUtils.test.ts @@ -12,6 +12,13 @@ jest.mock("@/settings/model", () => ({ getSettings: jest.fn(() => ({ projectsFolder: "copilot-projects" })), })); +// getProjectsFolder derives from copilotFolder in production; shim the derived +// accessor to the folder these tests configure via getSettings().projectsFolder. +jest.mock("@/settings/copilotFolder", () => { + const { getSettings } = jest.requireMock("@/settings/model"); + return { getEffectiveProjectsFolder: jest.fn(() => getSettings().projectsFolder) }; +}); + jest.mock("@/projects/state", () => ({ addPendingFileWrite: jest.fn(), removePendingFileWrite: jest.fn(), diff --git a/src/search/miyo/MiyoSemanticRetriever.test.ts b/src/search/miyo/MiyoSemanticRetriever.test.ts index c283bb78..677082b6 100644 --- a/src/search/miyo/MiyoSemanticRetriever.test.ts +++ b/src/search/miyo/MiyoSemanticRetriever.test.ts @@ -11,6 +11,10 @@ const mockGetDocumentsByPath = jest.fn(); jest.mock("@/logger"); jest.mock("@/settings/model", () => ({ getSettings: jest.fn(), + // searchUtils' getSystemExcludedFolders normalizes root paths through the real + // helper; keep it faithful so the pattern filter behaves as in production. + normalizeRootFolders: + jest.requireActual("@/settings/model").normalizeRootFolders, })); jest.mock("@/miyo/miyoUtils", () => ({ getMiyoFolderName: jest.fn(), @@ -90,11 +94,14 @@ describe("MiyoSemanticRetriever", () => { const retriever = createRetriever(); const documents = await retriever.getRelevantDocuments("query with [[notes/a]] mention"); + // Over-fetch is always on now: the system root exclusion is always active + // (hasActiveCopilotPatterns is unconditionally true), so the request uses + // RETURN_ALL_LIMIT so post-filtering still leaves enough to fill finalK. expect(mockSearch).toHaveBeenCalledWith( "http://miyo.local", "/vault", "query with [[notes/a]] mention", - 10, + RETURN_ALL_LIMIT, undefined ); expect(mockGetDocumentsByPath).not.toHaveBeenCalled(); @@ -194,15 +201,23 @@ describe("MiyoSemanticRetriever", () => { ]); }); - it("bounds the request to the requested limit when no filter is active", async () => { - // With no inclusion/exclusion pattern and returnAll off, over-fetching only - // wastes transfer/processing, so the request is bounded to the cap. + it("over-fetches even with no user patterns because the system root exclusion is always active", async () => { + // The always-on system root exclusion (copilot + active + historical roots) + // can always drop a chunk, so the retriever must over-fetch to RETURN_ALL_LIMIT + // regardless of the user's QA patterns — otherwise post-filtering could leave + // fewer than finalK results. mockSearch.mockResolvedValue({ results: [] }); const retriever = createRetriever({ maxK: 3 }); await retriever.getRelevantDocuments("query"); - expect(mockSearch).toHaveBeenCalledWith("http://miyo.local", "/vault", "query", 3, undefined); + expect(mockSearch).toHaveBeenCalledWith( + "http://miyo.local", + "/vault", + "query", + RETURN_ALL_LIMIT, + undefined + ); }); it("filters chunks by Copilot inclusion/exclusion rules", async () => { diff --git a/src/search/searchUtils.test.ts b/src/search/searchUtils.test.ts index 47b57223..7cd1baf3 100644 --- a/src/search/searchUtils.test.ts +++ b/src/search/searchUtils.test.ts @@ -3,15 +3,20 @@ import * as utils from "@/utils"; import { TFile } from "obsidian"; import { categorizePatterns, + createCopilotPatternFilter, createPatternSettingsValue, getDecodedPatterns, getMatchingPatterns, + getSystemExcludedFolders, + hasActiveCopilotPatterns, + isInternalExcludedPath, previewPatternValue, shouldIndexFile, } from "./searchUtils"; // Mock Obsidian's TFile and Modal classes jest.mock("obsidian", () => ({ + normalizePath: (path: string) => path.replace(/\/+/g, "/").replace(/^\/|\/$/g, ""), TFile: class TFile { path: string; }, @@ -481,4 +486,106 @@ describe("searchUtils", () => { }); }); }); + + describe("getSystemExcludedFolders", () => { + it("always includes the historical copilot root", () => { + const folders = getSystemExcludedFolders({ + copilotFolder: "copilot", + copilotRootHistory: [], + } as unknown as Parameters[0]); + expect(folders).toContain("copilot"); + }); + + it("includes the active root and every historical root", () => { + const folders = getSystemExcludedFolders({ + copilotFolder: "team-ai", + copilotRootHistory: ["copilot", "ai"], + } as unknown as Parameters[0]); + expect(new Set(folders)).toEqual(new Set(["copilot", "ai", "team-ai"])); + }); + + it("normalizes and dedupes without lowercasing (matcher is case-sensitive)", () => { + const folders = getSystemExcludedFolders({ + copilotFolder: "Copilot/", + copilotRootHistory: ["copilot", "Copilot"], + } as unknown as Parameters[0]); + // "Copilot" (trailing slash stripped) and "copilot" stay distinct entries. + expect(new Set(folders)).toEqual(new Set(["copilot", "Copilot"])); + }); + }); + + describe("isInternalExcludedPath", () => { + it("excludes project-config files under the projects folder derived from the default root", () => { + (settingsModel.getSettings as jest.Mock).mockReturnValue({ + qaInclusions: "", + qaExclusions: "", + copilotFolder: "copilot", + }); + expect(isInternalExcludedPath("copilot/projects/my-project/project.md")).toBe(true); + }); + + it("derives the projects folder from a custom root, not the retired projectsFolder field", () => { + (settingsModel.getSettings as jest.Mock).mockReturnValue({ + qaInclusions: "", + qaExclusions: "", + copilotFolder: "team-ai", + // Retired field left pointing at the stale default path; derivation must ignore it. + projectsFolder: "copilot/projects", + }); + // Excluded under the derived custom-root path. + expect(isInternalExcludedPath("team-ai/projects/my-project/project.md")).toBe(true); + // NOT excluded under the stale retired path, proving derivation from the root. + expect(isInternalExcludedPath("copilot/projects/my-project/project.md")).toBe(false); + }); + + it("does not exclude ordinary user files that merely live under the projects folder", () => { + (settingsModel.getSettings as jest.Mock).mockReturnValue({ + qaInclusions: "", + qaExclusions: "", + copilotFolder: "team-ai", + }); + expect(isInternalExcludedPath("team-ai/projects/my-project/notes.md")).toBe(false); + }); + }); + + describe("hasActiveCopilotPatterns", () => { + it("is always true because the system root exclusion is always active", () => { + (settingsModel.getSettings as jest.Mock).mockReturnValue({ + qaInclusions: "", + qaExclusions: "", + }); + expect(hasActiveCopilotPatterns()).toBe(true); + }); + }); + + describe("createCopilotPatternFilter", () => { + it("excludes the active and historical roots even with no user patterns", () => { + (settingsModel.getSettings as jest.Mock).mockReturnValue({ + qaInclusions: "", + qaExclusions: "", + copilotFolder: "ai", + copilotRootHistory: ["copilot", "ai"], + }); + const filter = createCopilotPatternFilter(window.app); + // System roots dropped on the raw path — no TFile resolution required. + expect(filter("ai/memory/note.md")).toBe(false); + expect(filter("copilot/copilot-conversations/chat.md")).toBe(false); + expect(filter("notes/idea.md")).toBe(true); + expect(mockGetAbstractFileByPath).not.toHaveBeenCalledWith("ai/memory/note.md"); + }); + + it("does not over-match sibling or differently-cased folders", () => { + (settingsModel.getSettings as jest.Mock).mockReturnValue({ + qaInclusions: "", + qaExclusions: "", + copilotFolder: "copilot", + copilotRootHistory: ["copilot"], + }); + const filter = createCopilotPatternFilter(window.app); + // Case-sensitive: "Copilot/" is a different folder and is kept. + expect(filter("Copilot/note.md")).toBe(true); + // Segment boundary: "mycopilot/" is not the "copilot" root. + expect(filter("mycopilot/note.md")).toBe(true); + }); + }); }); diff --git a/src/search/searchUtils.ts b/src/search/searchUtils.ts index 95711d7f..d3eb0bf5 100644 --- a/src/search/searchUtils.ts +++ b/src/search/searchUtils.ts @@ -1,8 +1,10 @@ +import { COPILOT_FOLDER_ROOT } from "@/constants"; import { CustomError } from "@/error"; import { AGENTS_MIRROR_FILE, PROJECT_CONFIG_FILE_NAME } from "@/projects/constants"; import EmbeddingsManager from "@/LLMProviders/embeddingManager"; import { logError, logInfo, logWarn } from "@/logger"; -import { getSettings } from "@/settings/model"; +import { getSettings, normalizeRootFolders, type CopilotSettings } from "@/settings/model"; +import { getEffectiveProjectsFolder } from "@/settings/copilotFolder"; import { logFileManager } from "@/logFileManager"; import { getTagsFromNote, stripHash } from "@/utils"; import { Embeddings } from "@langchain/core/embeddings"; @@ -137,6 +139,25 @@ export function getMatchingPatterns(options?: { }; } +/** + * The Copilot root folders excluded from QA indexing independent of the user's + * patterns: `copilot`, the active root, and every root ever activated + * ({@link CopilotSettings.copilotRootHistory}). Always-on privacy invariant so + * content under any current OR former root never enters the index. + * + * @param settings - Current Copilot settings. + * @returns Normalized, deduped root folders to exclude (never empty). + */ +export function getSystemExcludedFolders(settings: CopilotSettings): string[] { + const history = Array.isArray(settings.copilotRootHistory) ? settings.copilotRootHistory : []; + return normalizeRootFolders([COPILOT_FOLDER_ROOT, settings.copilotFolder, ...history]); +} + +/** Whether a vault-relative path falls under any live system-excluded Copilot root. */ +function isSystemExcludedPath(filePath: string): boolean { + return matchFilePathWithFolders(filePath, getSystemExcludedFolders(getSettings())); +} + /** * Should index the file based on the inclusions and exclusions patterns. * @param file - The file to check. @@ -156,6 +177,10 @@ export function shouldIndexFile( if (isInternalExcludedFile(file)) { return false; } + // Exclude the system Copilot roots (active + historical) before user patterns. + if (isSystemExcludedPath(file.path)) { + return false; + } if (exclusions && matchFilePathWithPatterns(app, file, exclusions)) { return false; } @@ -172,20 +197,26 @@ export function shouldIndexFile( } /** - * Build a predicate that decides whether a vault-relative path passes Copilot's - * QA inclusion/exclusion rules. The active patterns are resolved once so the - * predicate can be reused across many results. Paths that don't resolve to a - * vault {@link TFile} are kept, since the rules can't be evaluated for them. + * Build a predicate deciding whether a vault-relative path passes Copilot's QA + * rules (resolved once for reuse). Unresolvable paths are kept, but the system + * root exclusion is applied to the raw path first, so a former root's content is + * dropped even with no user QA patterns configured. * * @param app - The Obsidian app instance. * @returns Predicate returning true when the path should be kept. */ export function createCopilotPatternFilter(app: App): (path: string) => boolean { + const systemExcludedFolders = getSystemExcludedFolders(getSettings()); const { inclusions, exclusions } = getMatchingPatterns(); - if (!inclusions && !exclusions) { - return () => true; - } return (path: string) => { + // System root exclusion runs first on the raw path (no TFile), holding even + // in the no-user-pattern fast path below. + if (matchFilePathWithFolders(path, systemExcludedFolders)) { + return false; + } + if (!inclusions && !exclusions) { + return true; + } const file = app.vault.getAbstractFileByPath(path); if (!(file instanceof TFile)) { return true; @@ -195,16 +226,12 @@ export function createCopilotPatternFilter(app: App): (path: string) => boolean } /** - * Whether any QA inclusion/exclusion pattern is currently configured. Callers - * use this to decide whether {@link createCopilotPatternFilter} can actually - * remove results — when nothing is configured the filter is a no-op, so there - * is no point over-fetching candidates to compensate for it. - * - * @returns True when at least one inclusion or exclusion pattern is active. + * Whether {@link createCopilotPatternFilter} can remove results, so callers know + * whether to over-fetch to compensate. Always true: the system root exclusion is + * always active, so a candidate can be dropped even with no user patterns. */ export function hasActiveCopilotPatterns(): boolean { - const { inclusions, exclusions } = getMatchingPatterns(); - return Boolean(inclusions || exclusions); + return true; } /** @@ -419,8 +446,10 @@ function getInternalExcludePaths(): string[] { * Any file whose path starts with one of these prefixes is considered internal. */ function getInternalExcludeFolderPrefixes(): string[] { - const settings = getSettings(); - const projectsFolder = (settings.projectsFolder || "").trim(); + // Reason: derive the projects folder from the configurable root instead of the + // retired `settings.projectsFolder`, so a custom root excludes its own + // project-config files rather than the stale default path. + const projectsFolder = getEffectiveProjectsFolder().trim(); if (projectsFolder) { // Reason: normalize to forward slashes, collapse duplicates, strip trailing slash, // then append exactly one "/" for prefix matching. Mirrors normalizePath behavior diff --git a/src/settings/UpgradeRelocationNotice.test.tsx b/src/settings/UpgradeRelocationNotice.test.tsx new file mode 100644 index 00000000..c0ba63dc --- /dev/null +++ b/src/settings/UpgradeRelocationNotice.test.tsx @@ -0,0 +1,33 @@ +import { UpgradeRelocationNotice } from "@/settings/UpgradeRelocationNotice"; +import type { FolderRelocationEntry } from "@/settings/upgradeNotice"; +import { render, screen } from "@testing-library/react"; +import React from "react"; + +describe("UpgradeRelocationNotice", () => { + describe("UpgradeRelocationNotice()", () => { + it("renders the heading and states that files were not moved", () => { + render(); + expect(screen.getByRole("heading", { name: "Copilot folders have moved" })).not.toBeNull(); + expect(screen.getByText(/weren.t moved/)).not.toBeNull(); + }); + + it("lists each entry as an old-to-new path pair", () => { + const entries: FolderRelocationEntry[] = [ + { + label: "Chat conversations", + oldPath: "old/chats", + newPath: "copilot/copilot-conversations", + }, + { label: "Agent skills", oldPath: "old/skills", newPath: "copilot/skills" }, + ]; + render(); + // Each entry contributes a list item carrying its old and new paths. + const items = screen.getAllByRole("listitem"); + expect(items).toHaveLength(2); + expect(items[0].textContent).toBe( + "Chat conversations: old/chats → copilot/copilot-conversations" + ); + expect(items[1].textContent).toBe("Agent skills: old/skills → copilot/skills"); + }); + }); +}); diff --git a/src/settings/UpgradeRelocationNotice.tsx b/src/settings/UpgradeRelocationNotice.tsx new file mode 100644 index 00000000..73cc459b --- /dev/null +++ b/src/settings/UpgradeRelocationNotice.tsx @@ -0,0 +1,35 @@ +import { FolderRelocationEntry } from "@/settings/upgradeNotice"; +import { FolderSync } from "lucide-react"; +import React from "react"; + +/** + * Body of the one-time "Copilot folders have moved" modal shown after a v3→v4 + * upgrade to a user whose data needs relocating (a folder was customized, or the + * root itself moved). Leads with a folder-sync icon header, states that files + * were not moved, then lists each folder that needs relocating as `old → new`. + */ +export function UpgradeRelocationNotice({ + entries, +}: { + entries: readonly FolderRelocationEntry[]; +}) { + return ( +
+
+ +

Copilot folders have moved

+
+

+ Copilot now keeps everything under one folder. Your files weren't moved — move them + over if you want Copilot to keep using them (Obsidian updates the links automatically). +

+
    + {entries.map((entry) => ( +
  • + {entry.label}: {entry.oldPath} → {entry.newPath} +
    • + ))} +
+
+ ); +} diff --git a/src/settings/copilotFolder.test.ts b/src/settings/copilotFolder.test.ts new file mode 100644 index 00000000..77a4e64a --- /dev/null +++ b/src/settings/copilotFolder.test.ts @@ -0,0 +1,217 @@ +import { + COPILOT_SUBFOLDER, + deriveConversationsFolder, + deriveCustomPromptsFolder, + deriveMemoryFolder, + deriveProjectsFolder, + deriveSkillsFolder, + deriveSystemPromptsFolder, + ensureCopilotSubfolders, + getEffectiveConversationsFolder, + getEffectiveCopilotFolder, + getEffectiveCustomPromptsFolder, + getEffectiveMemoryFolder, + getEffectiveProjectsFolder, + getEffectiveSkillsFolder, + getEffectiveSystemPromptsFolder, +} from "@/settings/copilotFolder"; +import type { CopilotSettings } from "@/settings/model"; +import { getSettings } from "@/settings/model"; +import { DEFAULT_SETTINGS } from "@/constants"; + +jest.mock("obsidian", () => ({ + // Collapse duplicate separators and trim edges the way Obsidian's helper does, + // enough for the derived-path assertions here. + normalizePath: (path: string) => path.replace(/\/+/g, "/").replace(/^\/|\/$/g, ""), +})); + +jest.mock("@/settings/model", () => ({ + getSettings: jest.fn(), +})); + +jest.mock("@/utils", () => ({ + ensureFolderExists: jest.fn().mockResolvedValue(undefined), +})); + +jest.mock("@/logger", () => ({ + logWarn: jest.fn(), +})); + +const mockedGetSettings = getSettings as jest.MockedFunction; + +/** Build the minimal settings slice the derivation helpers read. */ +function settingsWithRoot(copilotFolder: string): CopilotSettings { + return { copilotFolder } as CopilotSettings; +} + +describe("copilotFolder", () => { + describe("deriveConversationsFolder()", () => { + it("derives the historical default conversations path for the default root", () => { + expect(deriveConversationsFolder(settingsWithRoot("copilot"))).toBe( + "copilot/copilot-conversations" + ); + }); + + it("re-roots the conversations folder under a custom root", () => { + expect(deriveConversationsFolder(settingsWithRoot("team/ai"))).toBe( + "team/ai/copilot-conversations" + ); + }); + + it("falls back to the default root when the configured root is blank", () => { + expect(deriveConversationsFolder(settingsWithRoot(" "))).toBe( + "copilot/copilot-conversations" + ); + }); + }); + + describe("deriveCustomPromptsFolder()", () => { + it("derives the historical default custom-prompts path for the default root", () => { + expect(deriveCustomPromptsFolder(settingsWithRoot("copilot"))).toBe( + "copilot/copilot-custom-prompts" + ); + }); + + it("re-roots the custom-prompts folder under a custom root", () => { + expect(deriveCustomPromptsFolder(settingsWithRoot("notes/copilot"))).toBe( + "notes/copilot/copilot-custom-prompts" + ); + }); + }); + + describe("deriveSystemPromptsFolder()", () => { + it("derives the historical default system-prompts path for the default root", () => { + expect(deriveSystemPromptsFolder(settingsWithRoot("copilot"))).toBe("copilot/system-prompts"); + }); + + it("re-roots the system-prompts folder under a custom root", () => { + expect(deriveSystemPromptsFolder(settingsWithRoot("team/ai"))).toBe("team/ai/system-prompts"); + }); + }); + + describe("deriveSkillsFolder()", () => { + it("derives the historical default skills path for the default root", () => { + expect(deriveSkillsFolder(settingsWithRoot("copilot"))).toBe("copilot/skills"); + }); + + it("re-roots the skills folder under a custom root", () => { + expect(deriveSkillsFolder(settingsWithRoot("team/ai"))).toBe("team/ai/skills"); + }); + }); + + describe("deriveMemoryFolder()", () => { + it("derives the historical default memory path for the default root", () => { + expect(deriveMemoryFolder(settingsWithRoot("copilot"))).toBe("copilot/memory"); + }); + + it("re-roots the memory folder under a custom root", () => { + expect(deriveMemoryFolder(settingsWithRoot("team/ai"))).toBe("team/ai/memory"); + }); + }); + + describe("deriveProjectsFolder()", () => { + it("derives the historical default projects path for the default root", () => { + expect(deriveProjectsFolder(settingsWithRoot("copilot"))).toBe("copilot/projects"); + }); + + it("re-roots the projects folder under a custom root", () => { + expect(deriveProjectsFolder(settingsWithRoot("team/ai"))).toBe("team/ai/projects"); + }); + }); + + describe("COPILOT_SUBFOLDER", () => { + it("pins the sub-folder names to the historical hardcoded defaults", () => { + expect(COPILOT_SUBFOLDER).toEqual({ + conversations: "copilot-conversations", + customPrompts: "copilot-custom-prompts", + systemPrompts: "system-prompts", + skills: "skills", + memory: "memory", + projects: "projects", + }); + }); + }); + + describe("getEffectiveCopilotFolder()", () => { + it("returns the configured root read from global settings", () => { + mockedGetSettings.mockReturnValue(settingsWithRoot("team/ai")); + expect(getEffectiveCopilotFolder()).toBe("team/ai"); + }); + }); + + describe("effective accessors read the live global root", () => { + it("derive every sub-folder from the current global copilotFolder", () => { + mockedGetSettings.mockReturnValue(settingsWithRoot("team/ai")); + expect(getEffectiveConversationsFolder()).toBe("team/ai/copilot-conversations"); + expect(getEffectiveCustomPromptsFolder()).toBe("team/ai/copilot-custom-prompts"); + expect(getEffectiveSystemPromptsFolder()).toBe("team/ai/system-prompts"); + expect(getEffectiveSkillsFolder()).toBe("team/ai/skills"); + expect(getEffectiveMemoryFolder()).toBe("team/ai/memory"); + expect(getEffectiveProjectsFolder()).toBe("team/ai/projects"); + }); + }); + + describe("ensureCopilotSubfolders()", () => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { ensureFolderExists } = require("@/utils") as { + ensureFolderExists: jest.MockedFunction<(vault: unknown, folder: string) => Promise>; + }; + const fakeVault = {} as import("obsidian").Vault; + + beforeEach(() => ensureFolderExists.mockClear()); + + it("creates all six derived sub-folders under the given root", async () => { + await ensureCopilotSubfolders(fakeVault, settingsWithRoot("team/ai")); + const created = ensureFolderExists.mock.calls.map((call) => call[1]); + expect(created).toEqual([ + "team/ai/copilot-conversations", + "team/ai/copilot-custom-prompts", + "team/ai/system-prompts", + "team/ai/skills", + "team/ai/memory", + "team/ai/projects", + ]); + }); + + it("continues past a folder that fails so one bad path cannot block the rest", async () => { + ensureFolderExists + .mockRejectedValueOnce(new Error("path conflict")) + .mockResolvedValue(undefined); + await expect( + ensureCopilotSubfolders(fakeVault, settingsWithRoot("copilot")) + ).resolves.toBeUndefined(); + // All six are still attempted even though the first threw. + expect(ensureFolderExists).toHaveBeenCalledTimes(6); + }); + }); + + describe("byte-for-byte parity with the retired default sub-folders", () => { + // Reason: a default vault (copilotFolder === "copilot") must resolve to the + // exact paths the retired per-folder defaults used, so upgrades are a no-op. + const settings = DEFAULT_SETTINGS; + + it("matches the default conversations folder", () => { + expect(deriveConversationsFolder(settings)).toBe(DEFAULT_SETTINGS.defaultSaveFolder); + }); + + it("matches the default custom-prompts folder", () => { + expect(deriveCustomPromptsFolder(settings)).toBe(DEFAULT_SETTINGS.customPromptsFolder); + }); + + it("matches the default system-prompts folder", () => { + expect(deriveSystemPromptsFolder(settings)).toBe(DEFAULT_SETTINGS.userSystemPromptsFolder); + }); + + it("matches the default skills folder", () => { + expect(deriveSkillsFolder(settings)).toBe(DEFAULT_SETTINGS.agentMode.skills.folder); + }); + + it("matches the default memory folder", () => { + expect(deriveMemoryFolder(settings)).toBe(DEFAULT_SETTINGS.memoryFolderName); + }); + + it("matches the default projects folder", () => { + expect(deriveProjectsFolder(settings)).toBe(DEFAULT_SETTINGS.projectsFolder); + }); + }); +}); diff --git a/src/settings/copilotFolder.ts b/src/settings/copilotFolder.ts new file mode 100644 index 00000000..b86bbde3 --- /dev/null +++ b/src/settings/copilotFolder.ts @@ -0,0 +1,147 @@ +import { DEFAULT_COPILOT_FOLDER } from "@/constants"; +import { logWarn } from "@/logger"; +import { getSettings, type CopilotSettings } from "@/settings/model"; +import { ensureFolderExists } from "@/utils"; +import { normalizePath, type Vault } from "obsidian"; + +/** + * Fixed sub-folder names every Copilot data folder derives from the single + * configurable `copilotFolder` root. These names are the historical hardcoded + * defaults (byte-for-byte), so a default vault (`copilotFolder === "copilot"`) + * resolves to exactly the same paths it used before the root became configurable. + */ +export const COPILOT_SUBFOLDER = Object.freeze({ + conversations: "copilot-conversations", + customPrompts: "copilot-custom-prompts", + systemPrompts: "system-prompts", + skills: "skills", + memory: "memory", + projects: "projects", +} as const); + +/** Settings shape the derivation helpers depend on. */ +type FolderSettings = Pick; + +/** + * Resolve the configurable root all sub-folders derive from. + * + * Reason: the loader's `sanitizeSettings` guarantees a non-empty, traversal-free + * root, but the UI can render before settings hydration completes. Falling back + * to the default in that window keeps derived paths well-formed rather than + * producing a leading-slash path. Mirrors `SkillManager.resolveSkillsFolder`. + */ +function copilotRoot(settings: FolderSettings): string { + return (settings.copilotFolder || "").trim() || DEFAULT_COPILOT_FOLDER; +} + +/** Join the resolved root with a fixed sub-folder name into a normalized path. */ +function deriveSubfolder(settings: FolderSettings, subfolder: string): string { + return normalizePath(`${copilotRoot(settings)}/${subfolder}`); +} + +/** + * Derive the chat-conversations folder from a settings snapshot. + * Pure: callers (e.g. watchers comparing prev/next) pass the snapshot they hold + * instead of reading the global store mid-operation. + */ +export function deriveConversationsFolder(settings: FolderSettings): string { + return deriveSubfolder(settings, COPILOT_SUBFOLDER.conversations); +} + +/** Derive the custom-commands folder from a settings snapshot. */ +export function deriveCustomPromptsFolder(settings: FolderSettings): string { + return deriveSubfolder(settings, COPILOT_SUBFOLDER.customPrompts); +} + +/** Derive the user system-prompts folder from a settings snapshot. */ +export function deriveSystemPromptsFolder(settings: FolderSettings): string { + return deriveSubfolder(settings, COPILOT_SUBFOLDER.systemPrompts); +} + +/** Derive the agent skills folder from a settings snapshot. */ +export function deriveSkillsFolder(settings: FolderSettings): string { + return deriveSubfolder(settings, COPILOT_SUBFOLDER.skills); +} + +/** Derive the user-memory folder from a settings snapshot. */ +export function deriveMemoryFolder(settings: FolderSettings): string { + return deriveSubfolder(settings, COPILOT_SUBFOLDER.memory); +} + +/** Derive the projects folder from a settings snapshot. */ +export function deriveProjectsFolder(settings: FolderSettings): string { + return deriveSubfolder(settings, COPILOT_SUBFOLDER.projects); +} + +/** + * Effective root folder for artifacts written directly under it (e.g. the log + * file and index-inspection notes), not into one of the derived sub-folders. + */ +export function getEffectiveCopilotFolder(): string { + return normalizePath(copilotRoot(getSettings())); +} + +/** Effective chat-conversations folder derived from the current global settings. */ +export function getEffectiveConversationsFolder(): string { + return deriveConversationsFolder(getSettings()); +} + +/** Effective custom-commands folder derived from the current global settings. */ +export function getEffectiveCustomPromptsFolder(): string { + return deriveCustomPromptsFolder(getSettings()); +} + +/** Effective user system-prompts folder derived from the current global settings. */ +export function getEffectiveSystemPromptsFolder(): string { + return deriveSystemPromptsFolder(getSettings()); +} + +/** Effective agent skills folder derived from the current global settings. */ +export function getEffectiveSkillsFolder(): string { + return deriveSkillsFolder(getSettings()); +} + +/** Effective user-memory folder derived from the current global settings. */ +export function getEffectiveMemoryFolder(): string { + return deriveMemoryFolder(getSettings()); +} + +/** Effective projects folder derived from the current global settings. */ +export function getEffectiveProjectsFolder(): string { + return deriveProjectsFolder(getSettings()); +} + +/** + * Pre-create every derived Copilot sub-folder under the current root so the + * locations the relocation/change-root notices point at already exist when the + * user goes to move files into them (Obsidian's own move dialog creates missing + * targets, but a user moving files from the OS file manager needs them present). + * + * Best-effort and idempotent: {@link ensureFolderExists} skips folders that + * already exist, and a per-folder failure (e.g. a file already occupies the + * path) is logged and skipped rather than aborting the rest. + * + * @param vault - Active vault, threaded in (never the global `app`). + * @param settings - Settings snapshot whose `copilotFolder` roots the derivation. + */ +export async function ensureCopilotSubfolders( + vault: Vault, + settings: FolderSettings +): Promise { + const derivers = [ + deriveConversationsFolder, + deriveCustomPromptsFolder, + deriveSystemPromptsFolder, + deriveSkillsFolder, + deriveMemoryFolder, + deriveProjectsFolder, + ]; + for (const derive of derivers) { + const folder = derive(settings); + try { + await ensureFolderExists(vault, folder); + } catch (error) { + logWarn(`Could not pre-create Copilot sub-folder "${folder}".`, error); + } + } +} diff --git a/src/settings/copilotRootChange.test.ts b/src/settings/copilotRootChange.test.ts new file mode 100644 index 00000000..c1330951 --- /dev/null +++ b/src/settings/copilotRootChange.test.ts @@ -0,0 +1,115 @@ +import { DEFAULT_SETTINGS } from "@/constants"; +import { + applyCopilotRootChange, + copilotRootContainsNotes, + isKnownCopilotRoot, +} from "@/settings/copilotRootChange"; +import { getSettings, settingsAtom, settingsStore, type CopilotSettings } from "@/settings/model"; +import type { App } from "obsidian"; + +const garbageCollectVectorStore = jest.fn, []>(); +jest.mock("@/search/vectorStoreManager", () => ({ + __esModule: true, + default: { getInstance: () => ({ garbageCollectVectorStore }) }, +})); + +/** Minimal App whose vault reports the given Markdown file paths. */ +function appWithMarkdown(paths: string[]): App { + return { + vault: { getMarkdownFiles: () => paths.map((path) => ({ path })) }, + } as unknown as App; +} + +function seedSettings(partial: Partial): void { + settingsStore.set(settingsAtom, { ...DEFAULT_SETTINGS, ...partial }); +} + +describe("copilotRootChange", () => { + beforeEach(() => { + jest.clearAllMocks(); + garbageCollectVectorStore.mockResolvedValue(0); + settingsStore.set(settingsAtom, { ...DEFAULT_SETTINGS }); + }); + + describe("copilotRootContainsNotes()", () => { + it("returns true when a Markdown file lives directly under the target root", () => { + const app = appWithMarkdown(["ai/note.md", "other/x.md"]); + expect(copilotRootContainsNotes(app, "ai")).toBe(true); + }); + + it("returns true for a nested Markdown file under the target root", () => { + const app = appWithMarkdown(["ai/sub/deep.md"]); + expect(copilotRootContainsNotes(app, "ai")).toBe(true); + }); + + it("returns false when no Markdown file is at or under the target root", () => { + const app = appWithMarkdown(["notes/a.md", "ai-adjacent/b.md"]); + // "ai-adjacent" must not match the "ai" prefix at a segment boundary. + expect(copilotRootContainsNotes(app, "ai")).toBe(false); + }); + + it("returns false for an empty candidate root", () => { + const app = appWithMarkdown(["a.md"]); + expect(copilotRootContainsNotes(app, "")).toBe(false); + }); + }); + + describe("isKnownCopilotRoot()", () => { + it("returns true for a root recorded in the history", () => { + expect(isKnownCopilotRoot("ai", ["copilot", "ai"])).toBe(true); + }); + + it("matches a root against the history in canonical form despite trailing slashes", () => { + expect(isKnownCopilotRoot("ai/", ["copilot", "ai"])).toBe(true); + }); + + it("returns false for a root absent from the history", () => { + expect(isKnownCopilotRoot("new-root", ["copilot", "ai"])).toBe(false); + }); + + it("returns false for an empty candidate root", () => { + expect(isKnownCopilotRoot("", ["copilot", "ai"])).toBe(false); + }); + + it("returns false against an empty history", () => { + expect(isKnownCopilotRoot("ai", [])).toBe(false); + }); + }); + + describe("applyCopilotRootChange()", () => { + it("commits the new root and its protection history in one settings snapshot", async () => { + seedSettings({ copilotFolder: "ai", copilotRootHistory: ["copilot", "ai"] }); + + await applyCopilotRootChange("team-ai"); + + const after = getSettings(); + expect(after.copilotFolder).toBe("team-ai"); + // Old + new + legacy roots all survive in the append-only history. + expect(new Set(after.copilotRootHistory)).toEqual(new Set(["copilot", "ai", "team-ai"])); + }); + + it("triggers a best-effort garbage-collection pass after activating", async () => { + seedSettings({ copilotFolder: "copilot" }); + await applyCopilotRootChange("ai"); + expect(garbageCollectVectorStore).toHaveBeenCalledTimes(1); + }); + + it("still activates the new root when garbage collection fails", async () => { + seedSettings({ copilotFolder: "copilot" }); + garbageCollectVectorStore.mockRejectedValueOnce(new Error("index offline")); + + await expect(applyCopilotRootChange("ai")).resolves.toBeUndefined(); + expect(getSettings().copilotFolder).toBe("ai"); + }); + + it("does not activate an invalid root or run garbage collection", async () => { + seedSettings({ copilotFolder: "copilot", copilotRootHistory: ["copilot"] }); + + await applyCopilotRootChange("../escape"); + + expect(getSettings().copilotFolder).toBe("copilot"); + expect(getSettings().copilotRootHistory).toEqual(["copilot"]); + expect(garbageCollectVectorStore).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/src/settings/copilotRootChange.ts b/src/settings/copilotRootChange.ts new file mode 100644 index 00000000..c6e06a7f --- /dev/null +++ b/src/settings/copilotRootChange.ts @@ -0,0 +1,141 @@ +import { logInfo, logWarn } from "@/logger"; +import { normalizeRootFolders, setSettings, validateCopilotFolder } from "@/settings/model"; +import type { App } from "obsidian"; +import { normalizePath } from "obsidian"; + +/** + * Whether `folder` is a root Copilot has used before, per the recorded history. + * + * Reason: a previously-active root still holds the Copilot Markdown it wrote + * (chats, memory, prompts) because a root switch never moves files, and that + * root stays permanently QA-excluded via `copilotRootHistory`. Re-activating it + * therefore introduces no new "ordinary notes silently hidden" risk, so the + * caller uses this to skip the {@link copilotRootContainsNotes} guard that would + * otherwise reject the switch on its own leftover data. Comparison is done on + * the same canonical form the history and QA matcher use. + * + * @param folder - Candidate root, vault-root-relative. + * @param history - Recorded `copilotRootHistory` from settings. + */ +export function isKnownCopilotRoot(folder: string, history: readonly string[]): boolean { + const [normalizedFolder] = normalizeRootFolders([folder]); + if (!normalizedFolder) return false; + return normalizeRootFolders(history).includes(normalizedFolder); +} + +/** + * App-aware helpers for changing the single configurable Copilot root folder. + * + * Syntax validation lives in {@link validateCopilotFolder} (pure, in `model`). + * This module adds the two pieces that need runtime state: + * 1. vault-content validation — reject a root that already holds ordinary + * notes, because activating it would permanently exclude those notes from + * QA search (the root and its history are always system-excluded); + * 2. the atomic root switch plus an immediate best-effort garbage-collection + * pass so index docs newly excluded by the change are dropped without + * waiting for the next full index. + */ + +/** + * Whether the target root already contains ordinary Markdown notes. + * + * Reason: once a folder becomes the Copilot root it is system-excluded from QA + * indexing wholesale, so pointing the root at a folder that already holds the + * user's notes would silently drop them from search. The Apply layer calls this + * to reject that case before committing. + * + * @param app - Active Obsidian app, threaded in (never the global `app`). + * @param folder - Candidate root, vault-root-relative. + * @returns True when at least one Markdown file lives at or under `folder`. + */ +export function copilotRootContainsNotes(app: App, folder: string): boolean { + const root = normalizePath(folder).replace(/\/+$/, ""); + if (root.length === 0) return false; + const prefix = `${root}/`; + return app.vault + .getMarkdownFiles() + .some((file) => file.path === root || file.path.startsWith(prefix)); +} + +/** + * Activate a new Copilot root, persisting the QA protection set and the active + * root together and then garbage-collecting orphaned index docs. + * + * The protection set (legacy root + previous root + full history + new root) + * and the new `copilotFolder` are written in a single atomic settings update, + * so a persisted snapshot can never show the new root without a history entry + * that keeps the previous root excluded from QA — there is no half-committed + * window to activate a root whose history has not landed. Callers must have + * already run {@link validateCopilotFolder} and the content check; the guard + * here is defensive and simply refuses to activate an invalid value. + * + * DESIGN NOTE — activation is memory-first, not persist-before-activate. + * `setSettings` only updates the in-memory atom; the disk write is the + * fire-and-forget `persistSettings` in the settings subscriber (see `main.ts`). + * (a) The residual risk: if that persist fails AND the user creates new + * conversations under the new root during this same session AND then + * restarts, the reloaded snapshot still carries the old root/history, so the + * leftover new-root content is no longer system-excluded and can leak into + * QA search. + * (b) We deliberately do NOT `await persistSettings` before activating (no GC / + * no "success" until disk lands). The persistence architecture is locked to + * memory-first, no-rollback-on-failure (see the subscriber comment in + * `main.ts`): memory is the source of truth and disk reconciles on the next + * successful write. An awaited persist here could not roll the in-memory + * root back on failure without violating that invariant, so it would only + * relabel the same unavoidable window — not close it — while adding a + * module-level `suppressNextPersistOnce` race against the subscriber. + * (c) Existing mitigations bound the damage: the subscriber surfaces a Notice on + * persist failure so the user knows the save did not land; the next + * successful persist reconciles disk with memory; and `sanitizeSettings` + * idempotently unions the current root back into the protection set on every + * load, so a persisted (even if lagging) snapshot never activates a root + * whose own path is unprotected. + * (d) If a future review flags this again, point them at this note. + * + * DESIGN NOTE — content-check TOCTOU (accepted, not fixed here). + * The vault-content guard ({@link copilotRootContainsNotes}) runs in the UI + * before the confirm modal; this Apply layer deliberately does NOT re-run it + * before `setSettings`. + * (a) Trigger: within the few-second window between the UI content check passing + * and the user confirming, Obsidian Sync or another plugin creates notes + * under the target root. + * (b) Consequence: those newly-arrived notes get excluded from QA wholesale when + * the root activates. It is fully reversible — pointing the root elsewhere + * restores them; no data is lost. + * (c) Why not fixed: re-validating at the Apply layer means threading `App` + + * `Vault` into this function purely to re-scan for a few-second, reversible + * window — a cost that does not match the exposure. + * (d) If a future review flags this again, point them at this note. + * + * @param newRoot - The user-entered root; re-validated defensively. + */ +export async function applyCopilotRootChange(newRoot: string): Promise { + const validation = validateCopilotFolder(newRoot); + if (!validation.ok) { + logWarn("Copilot root change rejected an invalid folder value.", validation.reason); + return; + } + const folder = validation.folder; + + setSettings((current) => ({ + copilotRootHistory: normalizeRootFolders([ + ...current.copilotRootHistory, + current.copilotFolder, + folder, + ]), + copilotFolder: folder, + })); + + // Best-effort: drop index docs that the old/new root now excludes. GC is + // idempotent and the next full index re-runs it, so failures are logged and + // swallowed rather than retried. Dynamically imported to keep the search + // stack out of the settings bundle's eager graph. + try { + const { default: VectorStoreManager } = await import("@/search/vectorStoreManager"); + const removed = await VectorStoreManager.getInstance().garbageCollectVectorStore(); + logInfo(`Copilot root change: garbage collection removed ${removed} stale index docs.`); + } catch (error) { + logWarn("Copilot root change: garbage collection failed (will retry on next index).", error); + } +} diff --git a/src/settings/migrations/index.ts b/src/settings/migrations/index.ts index 3e37afd2..665118df 100644 --- a/src/settings/migrations/index.ts +++ b/src/settings/migrations/index.ts @@ -10,10 +10,11 @@ * for a v4 vault picking up the v5 backfill). */ +import { DEFAULT_COPILOT_FOLDER } from "@/constants"; import { logInfo } from "@/logger"; import { seedDocProcessorBackend } from "@/miyo/miyoUtils"; import type { ModelManagementApi } from "@/modelManagement"; -import { getSettings, setSettings } from "@/settings/model"; +import { getSettings, normalizeRootFolders, setSettings } from "@/settings/model"; import { executeByokMigration } from "./byokMigration"; import { planRequiresApiKeyBackfill } from "./requiresApiKeyMigration"; @@ -64,6 +65,31 @@ export async function runSettingsMigrations(api: ModelManagementApi): Promise { + it("v8: seeds copilotFolder for a v7 vault", async () => { + // A v7 vault predates the configurable root and must be stamped with the + // historical default so the derived sub-folder accessors have a base. + mockGetSettings.mockReturnValue(settings({ settingsVersion: 7 })); + const { api } = makeApi(); + + await runSettingsMigrations(api); + + expect(mockSetSettings).toHaveBeenCalledWith({ copilotFolder: DEFAULT_COPILOT_FOLDER }); + expect(mockSetSettings).toHaveBeenCalledWith({ settingsVersion: CURRENT_SETTINGS_VERSION }); + }); + + it("v8: seeds copilotFolder for a pre-versioned install", async () => { + mockGetSettings.mockReturnValue(settings({ settingsVersion: undefined })); + const { api } = makeApi(); + + await runSettingsMigrations(api); + + expect(mockSetSettings).toHaveBeenCalledWith({ copilotFolder: DEFAULT_COPILOT_FOLDER }); + }); + + it("v8: does not re-seed copilotFolder for a vault already at the current version", async () => { + mockGetSettings.mockReturnValue(settings({ settingsVersion: CURRENT_SETTINGS_VERSION })); + const { api } = makeApi(); + + await runSettingsMigrations(api); + + const copilotFolderWrite = mockSetSettings.mock.calls.find( + (call) => "copilotFolder" in call[0] + ); + expect(copilotFolderWrite).toBeUndefined(); + }); + + it("v8: seeds copilotRootHistory with the legacy and current roots", async () => { + mockGetSettings.mockReturnValue(settings({ settingsVersion: 7 })); + const { api } = makeApi(); + + await runSettingsMigrations(api); + + const historyWrite = mockSetSettings.mock.calls.find( + (call) => typeof call[0] === "object" && "copilotRootHistory" in call[0] + ); + const seededHistory = (historyWrite?.[0] as Partial | undefined) + ?.copilotRootHistory; + expect(seededHistory).toEqual([DEFAULT_COPILOT_FOLDER]); + }); + + it("v8: flags a legacy vault (v7) as upgraded so WS-D can prompt", async () => { + mockGetSettings.mockReturnValue(settings({ settingsVersion: 7 })); + const { api } = makeApi(); + + await runSettingsMigrations(api); + + expect(mockSetSettings).toHaveBeenCalledWith({ upgradedToV8FromLegacy: true }); + }); + + it("v8: flags a pre-versioned install (version 0) as upgraded so WS-D can prompt", async () => { + // A pre-versioned install (settingsVersion absent → `fromVersion === 0`) is a + // real user whose data.json predates the version field, not a fresh install: + // fresh installs are stamped to the current version at bootstrap and never + // reach this migration. So a `0` here IS a legacy vault and must be flagged. + mockGetSettings.mockReturnValue(settings({ settingsVersion: undefined })); + const { api } = makeApi(); + + await runSettingsMigrations(api); + + expect(mockSetSettings).toHaveBeenCalledWith({ upgradedToV8FromLegacy: true }); + }); + + it("v8: does not flag a vault already at the current version", async () => { + mockGetSettings.mockReturnValue(settings({ settingsVersion: CURRENT_SETTINGS_VERSION })); + const { api } = makeApi(); + + await runSettingsMigrations(api); + + const flagWrite = mockSetSettings.mock.calls.find( + (call) => "upgradedToV8FromLegacy" in call[0] + ); + expect(flagWrite).toBeUndefined(); + }); +}); diff --git a/src/settings/migrations/version.ts b/src/settings/migrations/version.ts index 6b6c63eb..ce96ff61 100644 --- a/src/settings/migrations/version.ts +++ b/src/settings/migrations/version.ts @@ -15,5 +15,6 @@ * 5 → backfill `Provider.requiresApiKey` on flagless rows. * 6 → seed `docProcessorBackend` from effective Miyo state. * 7 → seed `enableMiyoSearchSkill` from persisted `enableMiyo` intent. + * 8 → seed `copilotFolder` root so derived sub-folder accessors resolve. */ -export const CURRENT_SETTINGS_VERSION = 7; +export const CURRENT_SETTINGS_VERSION = 8; diff --git a/src/settings/model.test.ts b/src/settings/model.test.ts index eb53826a..d656080b 100644 --- a/src/settings/model.test.ts +++ b/src/settings/model.test.ts @@ -6,9 +6,14 @@ import { SEND_SHORTCUT, } from "@/constants"; import { + normalizeRootFolders, + resetSettings, sanitizeEnvOverrides, sanitizeQaExclusions, sanitizeSettings, + settingsAtom, + settingsStore, + validateCopilotFolder, CopilotSettings, } from "@/settings/model"; import { getEffectiveUserPrompt, getSystemPrompt } from "@/system-prompts/systemPromptBuilder"; @@ -51,10 +56,15 @@ describe("sanitizeQaExclusions", () => { const sanitized = sanitizeQaExclusions(rawValue); - expect(sanitized.split(",")).toEqual([ - encodeURIComponent("folder/"), - encodeURIComponent(COPILOT_FOLDER_ROOT), - ]); + expect(sanitized.split(",")).toEqual([encodeURIComponent("folder/")]); + }); + + it("no longer force-injects the copilot root (system exclusion covers it)", () => { + const rawValue = encodeURIComponent("folder"); + + const sanitized = sanitizeQaExclusions(rawValue); + + expect(sanitized.split(",")).toEqual([encodeURIComponent("folder")]); }); }); @@ -667,3 +677,208 @@ describe("sanitizeSettings - docProcessorBackend (v6 field)", () => { expect(out.docProcessorBackend).toBe("miyo"); }); }); + +describe("model", () => { + describe("sanitizeSettings()", () => { + it("defaults to the historical root when empty", () => { + const out = sanitizeSettings({ + ...DEFAULT_SETTINGS, + copilotFolder: "", + }); + expect(out.copilotFolder).toBe(DEFAULT_SETTINGS.copilotFolder); + }); + + it("defaults to the historical root when whitespace-only", () => { + const out = sanitizeSettings({ + ...DEFAULT_SETTINGS, + copilotFolder: " ", + }); + expect(out.copilotFolder).toBe(DEFAULT_SETTINGS.copilotFolder); + }); + + it("trims surrounding whitespace from a custom value", () => { + const out = sanitizeSettings({ + ...DEFAULT_SETTINGS, + copilotFolder: " my-ai ", + }); + expect(out.copilotFolder).toBe("my-ai"); + }); + + it("preserves a nested custom value", () => { + const out = sanitizeSettings({ + ...DEFAULT_SETTINGS, + copilotFolder: "notes/ai", + }); + expect(out.copilotFolder).toBe("notes/ai"); + }); + + it("rejects a parent-traversal path", () => { + const out = sanitizeSettings({ + ...DEFAULT_SETTINGS, + copilotFolder: "../escape", + }); + expect(out.copilotFolder).toBe(DEFAULT_SETTINGS.copilotFolder); + }); + + it("rejects a Windows drive-absolute path", () => { + const out = sanitizeSettings({ + ...DEFAULT_SETTINGS, + copilotFolder: "C:/Users/evil", + }); + expect(out.copilotFolder).toBe(DEFAULT_SETTINGS.copilotFolder); + }); + + it("rejects a Unix-absolute path", () => { + const out = sanitizeSettings({ + ...DEFAULT_SETTINGS, + copilotFolder: "/etc/passwd", + }); + expect(out.copilotFolder).toBe(DEFAULT_SETTINGS.copilotFolder); + }); + it("unions the active root into a normalized, deduped history", () => { + const out = sanitizeSettings({ + ...DEFAULT_SETTINGS, + copilotFolder: "team-ai", + copilotRootHistory: ["copilot", "ai", "copilot"], + }); + expect(new Set(out.copilotRootHistory)).toEqual(new Set(["copilot", "ai", "team-ai"])); + }); + + it("guarantees the active root is present even when history is missing", () => { + const raw = { ...DEFAULT_SETTINGS, copilotFolder: "ai" } as unknown as Record< + string, + unknown + >; + delete raw.copilotRootHistory; + const out = sanitizeSettings(raw as unknown as CopilotSettings); + expect(out.copilotRootHistory).toContain("ai"); + }); + + it("coerces a non-boolean upgrade flag to the default", () => { + const out = sanitizeSettings({ + ...DEFAULT_SETTINGS, + upgradedToV8FromLegacy: undefined, + } as unknown as CopilotSettings); + expect(out.upgradedToV8FromLegacy).toBe(false); + }); + + it("preserves a true upgrade flag", () => { + const out = sanitizeSettings({ + ...DEFAULT_SETTINGS, + upgradedToV8FromLegacy: true, + }); + expect(out.upgradedToV8FromLegacy).toBe(true); + }); + }); + + describe("validateCopilotFolder()", () => { + it("rejects empty and whitespace-only values", () => { + expect(validateCopilotFolder("").ok).toBe(false); + expect(validateCopilotFolder(" ").ok).toBe(false); + }); + + it("accepts a simple relative folder and trims it", () => { + expect(validateCopilotFolder(" my-ai ")).toEqual({ ok: true, folder: "my-ai" }); + }); + + it("accepts a nested relative folder and strips a trailing slash", () => { + expect(validateCopilotFolder("notes/ai/")).toEqual({ ok: true, folder: "notes/ai" }); + }); + + it("rejects parent-traversal, absolute, and drive-letter paths", () => { + expect(validateCopilotFolder("../escape").ok).toBe(false); + expect(validateCopilotFolder("a/../b").ok).toBe(false); + expect(validateCopilotFolder("/etc/passwd").ok).toBe(false); + expect(validateCopilotFolder("C:/Users/evil").ok).toBe(false); + }); + + it("rejects a lone dot segment", () => { + expect(validateCopilotFolder("a/./b").ok).toBe(false); + }); + + it("rejects the Obsidian config folder case-insensitively", () => { + // eslint-disable-next-line obsidianmd/hardcoded-config-path -- the test asserts rejection of the conventional config dir literal + expect(validateCopilotFolder(".obsidian").ok).toBe(false); + expect(validateCopilotFolder(".Obsidian/plugins").ok).toBe(false); + }); + + it("rejects Windows-illegal characters in any segment", () => { + expect(validateCopilotFolder("a/b { + // sanitizeSettings must coerce every value validateCopilotFolder rejects to + // the default; a value it accepts must survive verbatim. + for (const value of ["../escape", "/etc/passwd", "C:/x", ""]) { + const out = sanitizeSettings({ ...DEFAULT_SETTINGS, copilotFolder: value }); + expect(out.copilotFolder).toBe(DEFAULT_SETTINGS.copilotFolder); + } + const kept = sanitizeSettings({ ...DEFAULT_SETTINGS, copilotFolder: "team/ai" }); + expect(kept.copilotFolder).toBe("team/ai"); + }); + }); + + describe("normalizeRootFolders()", () => { + it("preserves case and strips trailing slashes without lowercasing", () => { + expect(normalizeRootFolders(["Copilot/", "ai//"])).toEqual(["Copilot", "ai"]); + }); + + it("dedupes case-sensitively, preserving first-seen order", () => { + expect(normalizeRootFolders(["copilot", "ai", "copilot", "Copilot"])).toEqual([ + "copilot", + "ai", + "Copilot", + ]); + }); + + it("drops empty, non-string, and vault-escaping entries", () => { + expect( + normalizeRootFolders([ + "", + " ", + undefined, + "../escape", + "a/../b", + "/etc", + "C:\\Users\\Josh", + "notes/ai", + ]) + ).toEqual(["notes/ai"]); + }); + + it("collapses interior duplicate slashes into the matcher's canonical form", () => { + expect(normalizeRootFolders(["a//b"])).toEqual(["a/b"]); + }); + + it("strips interior single-dot segments into the matcher's canonical form", () => { + expect(normalizeRootFolders(["a/./b"])).toEqual(["a/b"]); + }); + + it("leaves an already-canonical legitimate root unchanged", () => { + expect(normalizeRootFolders(["a/b"])).toEqual(["a/b"]); + }); + + it("canonicalizes before deduping and traversal filtering", () => { + expect(normalizeRootFolders(["a//b", "a/./b", "x/../y", "a/b"])).toEqual(["a/b"]); + }); + }); + + describe("resetSettings()", () => { + it("preserves historical roots and folds in the pre-reset active root", () => { + settingsStore.set(settingsAtom, { + ...DEFAULT_SETTINGS, + copilotFolder: "team-ai", + copilotRootHistory: ["copilot", "ai"], + }); + + resetSettings(); + + const after = settingsStore.get(settingsAtom); + expect(after.copilotFolder).toBe(DEFAULT_SETTINGS.copilotFolder); + // Legacy + historical + pre-reset active root all survive the reset. + expect(new Set(after.copilotRootHistory)).toEqual(new Set(["copilot", "ai", "team-ai"])); + }); + }); +}); diff --git a/src/settings/model.ts b/src/settings/model.ts index 54066378..4bbbd9ac 100644 --- a/src/settings/model.ts +++ b/src/settings/model.ts @@ -10,7 +10,6 @@ import { AGENT_MAX_ITERATIONS_LIMIT, BUILTIN_CHAT_MODELS, BUILTIN_EMBEDDING_MODELS, - COPILOT_FOLDER_ROOT, DEFAULT_OPEN_AREA, DEFAULT_QA_EXCLUSIONS_SETTING, DEFAULT_SETTINGS, @@ -85,6 +84,33 @@ export interface CopilotSettings { openAIProxyBaseUrl: string; openAIEmbeddingProxyBaseUrl: string; stream: boolean; + /** Configurable root folder all Copilot sub-folders derive from (default: "copilot"). */ + copilotFolder: string; + /** + * Every folder that has ever been the Copilot root (seeded with the legacy + * `copilot` root in the v8 migration). Append-only: once a folder has held + * Copilot data it stays here so it remains permanently excluded from QA + * indexing, even after the root is changed away from it. + * + * DESIGN NOTE — this list is best-effort under multi-device Sync. Two offline + * devices that each change to a different root can have one branch's history + * overwritten by a covering Sync merge; the startup union in + * {@link sanitizeSettings} only re-adds THIS device's current root, so a root + * that only ever existed on the overwritten device can't be recovered. + * Accepted as a residual because changing the root at all is rare, and two + * devices concurrently changing to different roots rarer still. + * + * A second residual — a local persist failure activating a new root in memory + * before its history lands on disk — is documented at the activation site in + * {@link applyCopilotRootChange}; see that note before re-litigating whether + * the switch should persist before activating. + */ + copilotRootHistory: string[]; + /** + * True only when a legacy (v1-v7) vault was migrated to v8. Consumed once by + * the v3->v4 upgrade prompt (WS-D), which clears it back to false. + */ + upgradedToV8FromLegacy: boolean; defaultSaveFolder: string; defaultConversationTag: string; autosaveChat: boolean; @@ -427,6 +453,56 @@ const EMPTY_PROVIDERS = Object.freeze({}) as unknown as Record const EMPTY_CONFIGURED_MODELS = Object.freeze([]) as unknown as ConfiguredModel[]; const EMPTY_BACKENDS = Object.freeze({}) as unknown as Partial>; +/** Frozen fallback for an empty {@link CopilotSettings.copilotRootHistory}. */ +const EMPTY_COPILOT_ROOT_HISTORY = Object.freeze([]) as unknown as string[]; + +/** + * Canonicalize Copilot root-folder paths into the form the QA folder matcher + * (`matchFilePathWithFolders`) compares against: forward slashes, collapsed + * duplicate separators, no `.` segments, no trailing slash, and case preserved + * (the matcher is case-sensitive, so lowercasing here would break exact-root + * matching). Collapsing `//` and dropping `.` segments matters because history + * entries can arrive from Obsidian Sync merges or hand-edited `data.json` in a + * non-`normalizePath` form (e.g. `a//b`, `a/./b`); real file paths are compared + * post-`normalizePath`, so an un-collapsed root would fail the prefix match and + * silently leak that root's notes into QA. Legitimate roots already pass through + * `validateCopilotFolder` free of `//`/`.` segments, so this is a no-op for them + * and only repairs anomalous entries. Entries that are empty or escape the vault + * root (parent traversal, drive-absolute, or root-absolute paths) are dropped + * rather than coerced to a default, and duplicates are removed preserving + * first-seen order. + * + * @param input - Raw root paths; non-string, empty, and unsafe entries are skipped. + * @returns A deduped, normalized list, or a frozen empty constant when none survive. + */ +export function normalizeRootFolders(input: readonly (string | undefined)[]): string[] { + const seen = new Set(); + const result: string[] = []; + for (const raw of input) { + if (typeof raw !== "string") continue; + // Reason: collapse `//` and strip `.` segments so anomalous history entries + // land in the same canonical form the QA matcher compares real paths against. + const normalized = raw + .trim() + .replace(/\\/g, "/") + .replace(/\/+/g, "/") + .split("/") + .filter((segment) => segment !== ".") + .join("/") + .replace(/\/+$/, ""); + if (normalized.length === 0) continue; + const escapesVault = + /(^|\/)\.\.(\/|$)/.test(normalized) || + /^[a-zA-Z]:/.test(normalized) || + normalized.startsWith("/"); + if (escapesVault) continue; + if (seen.has(normalized)) continue; + seen.add(normalized); + result.push(normalized); + } + return result.length > 0 ? result : EMPTY_COPILOT_ROOT_HISTORY; +} + /** * Resolve a valid embedding model key for the current settings. * @@ -464,7 +540,15 @@ export function setSettings( } /** - * Normalize QA exclusion patterns and guarantee the Copilot folder root is excluded. + * Normalize the user's QA exclusion patterns (dedupe by canonical path key, + * preserving a single trailing slash when the user wrote one). + * + * The Copilot root itself is NOT forced in here anymore: the always-on system + * exclusion ({@link getSystemExcludedFolders} in `searchUtils`) permanently + * excludes the `copilot` root plus the active and historical roots, so this + * function only has to canonicalize what the user typed. A user-supplied + * `copilot` entry is kept as-is and simply overlaps the system exclusion. + * * @param rawValue - Persisted QA exclusion setting value. * @returns Encoded QA exclusion patterns string. */ @@ -481,10 +565,6 @@ export function sanitizeQaExclusions(rawValue: unknown): string { decodedPatterns.forEach((pattern) => { const canonical = pattern.replace(/\/+$/, ""); const canonicalKey = canonical.length > 0 ? canonical : pattern; - if (canonicalKey === COPILOT_FOLDER_ROOT) { - canonicalToOriginalPattern.set(COPILOT_FOLDER_ROOT, COPILOT_FOLDER_ROOT); - return; - } if (!canonicalToOriginalPattern.has(canonicalKey)) { const normalizedValue = canonical.length > 0 && pattern.endsWith("/") ? `${canonical}/` : pattern; @@ -492,8 +572,6 @@ export function sanitizeQaExclusions(rawValue: unknown): string { } }); - canonicalToOriginalPattern.set(COPILOT_FOLDER_ROOT, COPILOT_FOLDER_ROOT); - return Array.from(canonicalToOriginalPattern.values()) .map((pattern) => encodeURIComponent(pattern)) .join(","); @@ -545,10 +623,20 @@ export function getSettings(): Readonly { * If a future review flags this again, point them at this note. */ export function resetSettings(): void { + const current = getSettings(); + // Reset is a deterministic path, not best-effort: preserve the root-exclusion + // history and fold in the pre-reset active root before it is replaced by the + // default. Otherwise a reset would drop every historical root and leave that + // still-on-disk content exposed to QA indexing. + const preservedRootHistory = normalizeRootFolders([ + ...(Array.isArray(current.copilotRootHistory) ? current.copilotRootHistory : []), + current.copilotFolder, + ]); const defaultSettingsWithBuiltIns = { ...DEFAULT_SETTINGS, activeModels: BUILTIN_CHAT_MODELS.map((model) => ({ ...model, enabled: true })), activeEmbeddingModels: BUILTIN_EMBEDDING_MODELS.map((model) => ({ ...model, enabled: true })), + copilotRootHistory: preservedRootHistory, }; setSettings(defaultSettingsWithBuiltIns); } @@ -854,6 +942,34 @@ export function sanitizeSettings(settings: CopilotSettings): CopilotSettings { sanitizedSettings.defaultSendShortcut = DEFAULT_SETTINGS.defaultSendShortcut; } + // Coerce copilotFolder to a valid vault-relative path. validateCopilotFolder + // is the single source of truth for root syntax (empty, traversal, absolute, + // .obsidian, control/Windows-illegal chars) shared with the settings UI; an + // invalid persisted value falls back to the default rather than being trusted. + const copilotFolderValidation = validateCopilotFolder( + typeof settingsToSanitize.copilotFolder === "string" ? settingsToSanitize.copilotFolder : "" + ); + sanitizedSettings.copilotFolder = copilotFolderValidation.ok + ? copilotFolderValidation.folder + : DEFAULT_SETTINGS.copilotFolder; + + // Normalize the append-only root history and idempotently union in the + // active root, so this device's current root is always present in its own + // history (the load-time self-heal referenced in the field's DESIGN NOTE). + // These roots are excluded from QA indexing permanently — see + // getSystemExcludedFolders in searchUtils. + const rawRootHistory = Array.isArray(settingsToSanitize.copilotRootHistory) + ? settingsToSanitize.copilotRootHistory + : []; + sanitizedSettings.copilotRootHistory = normalizeRootFolders([ + ...rawRootHistory, + sanitizedSettings.copilotFolder, + ]); + + if (typeof sanitizedSettings.upgradedToV8FromLegacy !== "boolean") { + sanitizedSettings.upgradedToV8FromLegacy = DEFAULT_SETTINGS.upgradedToV8FromLegacy; + } + // Ensure folder settings fall back to defaults when empty/whitespace const saveFolder = (settingsToSanitize.defaultSaveFolder || "").trim(); sanitizedSettings.defaultSaveFolder = @@ -1015,6 +1131,15 @@ function sanitizeAgentMode(raw: unknown): CopilotSettings["agentMode"] { // eslint-disable-next-line no-control-regex const CONTROL_CHAR_RE = /[\u0000-\u001f\u007f]/; +/** + * The conventional Obsidian config-folder name, rejected as a Copilot root so a + * misconfigured root can't collide with Obsidian's own config. This is a pure, + * Vault-less syntax check, so it can only guard the near-universal default — + * users who relocated their config dir get no false positive here. + */ +// eslint-disable-next-line obsidianmd/hardcoded-config-path -- pure validator has no Vault to read configDir; guard the conventional default +const CONVENTIONAL_OBSIDIAN_CONFIG_DIR = ".obsidian"; + /** * Validate a user-entered "Skills folder" value against the rules in * `designdocs/SKILLS_MANAGEMENT.md` §Skills folder setting. @@ -1083,6 +1208,69 @@ export function validateSkillsFolder( return { ok: true, folder: cleaned }; } +/** + * Validate a user-entered Copilot root folder (`copilotFolder`) for syntax + * only — the reusable check shared by `sanitizeSettings` (every load / Sync / + * hand-edited data.json passes through it) and the settings UI for early + * feedback before Apply. Vault-content checks that need App/Vault access + * (rejecting a root that already holds ordinary notes) live in + * `copilotRootChange`, not here. + * + * Unlike {@link validateSkillsFolder}, absolute and drive-letter paths are + * rejected rather than stripped: the root is the trust boundary every derived + * sub-folder inherits, so a leading-slash value is treated as a mistake to + * surface, not silently rewritten. The `.obsidian` config folder is rejected + * to keep Copilot data from colliding with Obsidian's own config. + * + * @param value Raw user input. + * @returns `{ ok: true, folder }` with the trimmed, trailing-slash-stripped + * value, or `{ ok: false, reason }` carrying a UI-ready message. + */ +export function validateCopilotFolder( + value: string +): { ok: true; folder: string } | { ok: false; reason: string } { + if (typeof value !== "string" || value.trim().length === 0) { + return { ok: false, reason: "Folder name cannot be empty." }; + } + const trimmed = value.trim(); + // Reject (do not strip) absolute and drive-letter paths so a root can never + // escape the vault; a stray leading slash is surfaced as an error instead. + if (/^[/\\]/.test(trimmed) || /^[a-zA-Z]:/.test(trimmed)) { + return { ok: false, reason: "Folder path must be relative to the vault root." }; + } + const cleaned = trimmed.replace(/\\/g, "/").replace(/\/+$/, ""); + if (cleaned.length === 0) { + return { ok: false, reason: "Folder name cannot be empty." }; + } + for (const segment of cleaned.split("/")) { + if (segment.length === 0) { + return { ok: false, reason: "Folder path cannot contain empty segments (//)." }; + } + if (segment === "..") { + return { ok: false, reason: 'Folder path cannot contain ".." segments.' }; + } + if (segment === ".") { + return { ok: false, reason: 'Folder path cannot contain "." segments.' }; + } + if (segment.toLowerCase() === CONVENTIONAL_OBSIDIAN_CONFIG_DIR) { + return { + ok: false, + reason: "Folder path cannot use the Obsidian config folder.", + }; + } + if (CONTROL_CHAR_RE.test(segment)) { + return { ok: false, reason: "Folder path contains illegal control characters." }; + } + if (/[<>:"|?*]/.test(segment)) { + return { + ok: false, + reason: 'Folder path contains characters not allowed in folder names (< > : " | ? *).', + }; + } + } + return { ok: true, folder: cleaned }; +} + /** Truthy-string coerce: keep only non-empty string values. */ function nonEmptyString(v: unknown): string | undefined { return typeof v === "string" && v ? v : undefined; diff --git a/src/settings/upgradeNotice.test.ts b/src/settings/upgradeNotice.test.ts new file mode 100644 index 00000000..4fdfee10 --- /dev/null +++ b/src/settings/upgradeNotice.test.ts @@ -0,0 +1,187 @@ +import type { CopilotSettings } from "@/settings/model"; +import { buildUpgradeRelocationEntries } from "@/settings/upgradeNotice"; + +jest.mock("obsidian", () => ({ + // Collapse duplicate separators and trim edges the way Obsidian's helper does, + // enough for the derived-path assertions here. + normalizePath: (path: string) => path.replace(/\/+/g, "/").replace(/^\/|\/$/g, ""), +})); + +// The pure helpers never read the global store, but copilotFolder.ts imports it. +jest.mock("@/settings/model", () => ({ + getSettings: jest.fn(), +})); + +/** + * Build the settings slice the upgrade notice reads. Defaults every folder to + * its historical legacy default under the seeded "copilot" root, so a test only + * overrides the fields it wants to treat as customized. + */ +function buildSettings( + overrides: Partial<{ + copilotFolder: string; + defaultSaveFolder: string; + customPromptsFolder: string; + userSystemPromptsFolder: string; + skillsFolder: string; + memoryFolderName: string; + }> = {} +): CopilotSettings { + const { + copilotFolder = "copilot", + defaultSaveFolder = "copilot/copilot-conversations", + customPromptsFolder = "copilot/copilot-custom-prompts", + userSystemPromptsFolder = "copilot/system-prompts", + skillsFolder = "copilot/skills", + memoryFolderName = "copilot/memory", + } = overrides; + return { + copilotFolder, + defaultSaveFolder, + customPromptsFolder, + userSystemPromptsFolder, + memoryFolderName, + agentMode: { skills: { folder: skillsFolder } }, + } as unknown as CopilotSettings; +} + +describe("upgradeNotice", () => { + describe("buildUpgradeRelocationEntries()", () => { + it("returns no entries when every folder is at its legacy default", () => { + expect(buildUpgradeRelocationEntries(buildSettings())).toEqual([]); + }); + + it("reports a single customized folder with its old value and derived new path", () => { + const entries = buildUpgradeRelocationEntries( + buildSettings({ defaultSaveFolder: "my-notes/chats" }) + ); + expect(entries).toEqual([ + { + label: "Chat conversations", + oldPath: "my-notes/chats", + newPath: "copilot/copilot-conversations", + }, + ]); + }); + + it("reports every customized folder while omitting the untouched ones", () => { + const entries = buildUpgradeRelocationEntries( + buildSettings({ + customPromptsFolder: "prompts", + userSystemPromptsFolder: "sys", + skillsFolder: "my-skills", + }) + ); + expect(entries).toEqual([ + { label: "Custom prompts", oldPath: "prompts", newPath: "copilot/copilot-custom-prompts" }, + { label: "System prompts", oldPath: "sys", newPath: "copilot/system-prompts" }, + { label: "Agent skills", oldPath: "my-skills", newPath: "copilot/skills" }, + ]); + }); + + it("lists every sub-folder when the root moves, since they all relocate", () => { + // Re-rooting to team/ai moves all sub-folders: each old copilot/ + // no longer equals the new team/ai/, so all five are surfaced. The + // conversations entry additionally reflects the separately-customized old + // value. + const entries = buildUpgradeRelocationEntries( + buildSettings({ copilotFolder: "team/ai", defaultSaveFolder: "old/chats" }) + ); + expect(entries).toEqual([ + { + label: "Chat conversations", + oldPath: "old/chats", + newPath: "team/ai/copilot-conversations", + }, + { + label: "Custom prompts", + oldPath: "copilot/copilot-custom-prompts", + newPath: "team/ai/copilot-custom-prompts", + }, + { + label: "System prompts", + oldPath: "copilot/system-prompts", + newPath: "team/ai/system-prompts", + }, + { label: "Agent skills", oldPath: "copilot/skills", newPath: "team/ai/skills" }, + { label: "Memory", oldPath: "copilot/memory", newPath: "team/ai/memory" }, + ]); + }); + + it("surfaces a customized memory folder, since the new manager reads the derived path", () => { + // A vault upgraded from a release whose Plus settings exposed "Memory + // Folder Name" can carry a non-default memoryFolderName; the new manager + // reads copilot/memory instead, so the old data must be flagged to move. + const entries = buildUpgradeRelocationEntries( + buildSettings({ memoryFolderName: "my-memory" }) + ); + expect(entries).toEqual([ + { label: "Memory", oldPath: "my-memory", newPath: "copilot/memory" }, + ]); + }); + + it("omits memory when its stored value is the legacy default", () => { + expect( + buildUpgradeRelocationEntries(buildSettings({ memoryFolderName: "copilot/memory" })) + ).toEqual([]); + }); + + it("omits folders whose old value already equals the derived new path", () => { + // The user re-rooted to team/ai and their stored sub-folder values already + // equal the paths that root now derives to — nothing to move, so the + // notice lists nothing. + const entries = buildUpgradeRelocationEntries( + buildSettings({ + copilotFolder: "team/ai", + defaultSaveFolder: "team/ai/copilot-conversations", + customPromptsFolder: "team/ai/copilot-custom-prompts", + userSystemPromptsFolder: "team/ai/system-prompts", + skillsFolder: "team/ai/skills", + memoryFolderName: "team/ai/memory", + }) + ); + expect(entries).toEqual([]); + }); + + it("does not flag a default folder that differs only by a trailing slash", () => { + expect( + buildUpgradeRelocationEntries( + buildSettings({ defaultSaveFolder: "copilot/copilot-conversations/" }) + ) + ).toEqual([]); + }); + + it("does not flag a default folder that differs only by a trailing slash then whitespace", () => { + expect( + buildUpgradeRelocationEntries( + buildSettings({ defaultSaveFolder: "copilot/copilot-conversations/ " }) + ) + ).toEqual([]); + }); + + it("flags a folder that differs from its default only by letter case as customized", () => { + // Reason: comparison is case-sensitive to match the QA folder matcher, so on + // a case-sensitive filesystem `Copilot/...` is a genuinely different directory + // whose migration notice must still fire. + expect( + buildUpgradeRelocationEntries( + buildSettings({ defaultSaveFolder: "Copilot/copilot-conversations" }) + ) + ).toEqual([ + { + label: "Chat conversations", + oldPath: "Copilot/copilot-conversations", + newPath: "copilot/copilot-conversations", + }, + ]); + }); + + it("does not flag a default folder that differs only by path separator", () => { + expect( + buildUpgradeRelocationEntries( + buildSettings({ userSystemPromptsFolder: "copilot\\system-prompts" }) + ) + ).toEqual([]); + }); + }); +}); diff --git a/src/settings/upgradeNotice.ts b/src/settings/upgradeNotice.ts new file mode 100644 index 00000000..d115acad --- /dev/null +++ b/src/settings/upgradeNotice.ts @@ -0,0 +1,141 @@ +import { + deriveConversationsFolder, + deriveCustomPromptsFolder, + deriveMemoryFolder, + deriveSkillsFolder, + deriveSystemPromptsFolder, +} from "@/settings/copilotFolder"; +import type { CopilotSettings } from "@/settings/model"; + +/** + * One legacy sub-folder whose data needs relocating after the v3→v4 folder + * consolidation — its stored old path differs from the path Copilot now derives + * to (because the folder was customized, or the root itself moved). `oldPath` is + * where the data physically still lives; `newPath` is where Copilot reads and + * writes now. + */ +export interface FolderRelocationEntry { + /** Human-readable name of the data class (e.g. "Chat conversations"). */ + label: string; + /** Stored path Copilot used before the upgrade; the files remain here. */ + oldPath: string; + /** Derived path Copilot reads and writes after the upgrade. */ + newPath: string; +} + +/** Settings slice the upgrade notice inspects. */ +type UpgradeNoticeSettings = Pick< + CopilotSettings, + | "copilotFolder" + | "defaultSaveFolder" + | "customPromptsFolder" + | "userSystemPromptsFolder" + | "memoryFolderName" + | "agentMode" +>; + +/** Settings slice the pure derivation helpers depend on. */ +type RootSettings = Pick; + +interface SubFolderSpec { + /** Display name for the relocation entry. */ + label: string; + /** Stored legacy field value, shown verbatim as the old path. */ + legacyValue: (settings: UpgradeNoticeSettings) => string; + /** Derive the effective sub-folder path from a settings snapshot. */ + derive: (settings: RootSettings) => string; +} + +/** + * The legacy sub-folders whose stored path is compared against the derived new + * path to build the relocation list. Each had a real writer before v4 — either a + * user-facing path input or, for memory, a "Memory Folder Name" field that + * shipped in the Copilot Plus settings from 3.1.0 through 3.3.3 and wrote + * `memoryFolderName` via `updateSetting`. A vault upgraded from any of those + * releases can therefore carry a non-default `memoryFolderName` in `data.json`, + * and the new `UserMemoryManager` reads the derived `/memory` instead, so + * memory must be surfaced when the two differ. + * + * Projects (`projectsFolder`) is deliberately absent: although it is a persisted + * field, a full history search found no product write path (no `updateSetting` + * call, no input control) — only the default seed, sanitize fallback, and read + * sites. Its stored value is therefore always the default, so it can never + * differ from the derived path at upgrade. If a future review flags projects as + * missing here, point them at this note and re-check for a real writer before + * adding it. + */ +const SUBFOLDER_SPECS: readonly SubFolderSpec[] = [ + { + label: "Chat conversations", + legacyValue: (s) => s.defaultSaveFolder, + derive: deriveConversationsFolder, + }, + { + label: "Custom prompts", + legacyValue: (s) => s.customPromptsFolder, + derive: deriveCustomPromptsFolder, + }, + { + label: "System prompts", + legacyValue: (s) => s.userSystemPromptsFolder, + derive: deriveSystemPromptsFolder, + }, + { + label: "Agent skills", + legacyValue: (s) => s.agentMode.skills.folder, + derive: deriveSkillsFolder, + }, + { + label: "Memory", + legacyValue: (s) => s.memoryFolderName, + derive: deriveMemoryFolder, + }, +]; + +/** + * Canonicalize a vault-relative folder path for equality comparison only. + * Normalizes backslashes, duplicate and surrounding slashes so a user who never + * customized a folder (but whose stored value differs by a trailing slash) is + * not mistaken for a customizer. Comparison is case-sensitive to match the QA + * folder matcher and the rest of the codebase's path handling: on a + * case-sensitive filesystem (Linux) `Copilot/...` and `copilot/...` are + * genuinely different directories, so lowercasing here would misjudge a real + * customizer as a default user and skip their migration notice, silently + * orphaning their old conversations. The trade-off is that on a case-insensitive + * filesystem (macOS/Windows) a stored value differing only by case yields one + * benign no-op prompt — an acceptable, low-risk cost for not under-detecting + * real customizers on case-sensitive filesystems. + */ +function canonicalizePath(path: string): string { + return path + .trim() + .replace(/\\/g, "/") + .replace(/\/+/g, "/") + .replace(/^\/+|\/+$/g, ""); +} + +/** + * Build the list of legacy sub-folders whose data needs relocating after the + * v3→v4 folder consolidation: every folder whose stored old path differs from + * the path it now derives to (covers both individually-customized folders and, + * when the root itself moved, folders that only shifted because of the new + * root). Pure — the caller passes the settings snapshot so the decision is + * unit-testable without the global store. Returns an empty array when nothing + * needs to move (e.g. default users), which the caller treats as "do not prompt". + */ +export function buildUpgradeRelocationEntries( + settings: UpgradeNoticeSettings +): FolderRelocationEntry[] { + return SUBFOLDER_SPECS.flatMap((spec) => { + const oldPath = spec.legacyValue(settings); + const newPath = spec.derive(settings); + // Only surface a folder whose data actually needs to move: if the old + // location already resolves to the new derived path there is nothing to + // relocate. This also covers default (never-customized) users, whose old + // value equals the derived default. + if (canonicalizePath(oldPath) === canonicalizePath(newPath)) { + return []; + } + return [{ label: spec.label, oldPath, newPath }]; + }); +} diff --git a/src/settings/v2/components/AdvancedSettings.tsx b/src/settings/v2/components/AdvancedSettings.tsx index 95cba05a..4dada646 100644 --- a/src/settings/v2/components/AdvancedSettings.tsx +++ b/src/settings/v2/components/AdvancedSettings.tsx @@ -354,15 +354,6 @@ export const AdvancedSettings: React.FC = () => {
- - updateSetting("userSystemPromptsFolder", value)} - placeholder="copilot/system-prompts" - /> {/* Others Section */} diff --git a/src/settings/v2/components/BasicSettings.test.tsx b/src/settings/v2/components/BasicSettings.test.tsx new file mode 100644 index 00000000..31231cc8 --- /dev/null +++ b/src/settings/v2/components/BasicSettings.test.tsx @@ -0,0 +1,109 @@ +import { DEFAULT_SETTINGS } from "@/constants"; +import { settingsAtom, settingsStore } from "@/settings/model"; +import { BasicSettings } from "@/settings/v2/components/BasicSettings"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { Notice } from "obsidian"; +import React from "react"; + +// Stub the Plus banner to keep its dependency chain out of the test. +jest.mock("@/settings/v2/components/PlusSettings", () => ({ PlusSettings: () => null })); + +// App is threaded via useApp; the root-change orchestration is unit-tested in +// copilotRootChange.test, so mock it here to observe the UI's decisions. +jest.mock("@/context", () => ({ + // eslint-disable-next-line @eslint-react/hooks-extra/no-unnecessary-use-prefix -- mocks the real hook; name must match the export + useApp: () => ({ vault: { getMarkdownFiles: () => [] } }), +})); + +const applyCopilotRootChange = jest.fn, unknown[]>().mockResolvedValue(undefined); +const copilotRootContainsNotes = jest.fn().mockReturnValue(false); +const isKnownCopilotRoot = jest.fn().mockReturnValue(false); +jest.mock("@/settings/copilotRootChange", () => ({ + applyCopilotRootChange: (...a: unknown[]) => applyCopilotRootChange(...a), + copilotRootContainsNotes: (...a: unknown[]) => copilotRootContainsNotes(...a), + isKnownCopilotRoot: (...a: unknown[]) => isKnownCopilotRoot(...a), +})); + +// Capture ConfirmModal construction so a test can fire its confirm callback. +let capturedOnConfirm: (() => void) | null = null; +const modalCtor = jest.fn((onConfirm: () => void) => { + capturedOnConfirm = onConfirm; +}); +jest.mock("@/components/modals/ConfirmModal", () => ({ + ConfirmModal: class { + open = jest.fn(); + constructor(_app: unknown, onConfirm: () => void) { + modalCtor(onConfirm); + } + }, +})); + +describe("BasicSettings", () => { + beforeEach(() => { + jest.clearAllMocks(); + capturedOnConfirm = null; + settingsStore.set(settingsAtom, { ...DEFAULT_SETTINGS, copilotFolder: "copilot" }); + copilotRootContainsNotes.mockReturnValue(false); + isKnownCopilotRoot.mockReturnValue(false); + }); + + it("binds the Copilot folder input to the persisted root", () => { + render(); + expect(screen.getByLabelText("Copilot folder").value).toBe("copilot"); + }); + + it("no longer renders the retired conversation folder and tag inputs", () => { + render(); + expect(screen.queryByText("Default Conversation Folder Name")).toBeNull(); + expect(screen.queryByText("Default Conversation Tag")).toBeNull(); + }); + + it("rejects an invalid root on Apply without opening the confirm modal", () => { + render(); + fireEvent.change(screen.getByLabelText("Copilot folder"), { target: { value: "../escape" } }); + fireEvent.click(screen.getByRole("button", { name: "Apply Copilot folder" })); + expect(Notice).toHaveBeenCalledTimes(1); + expect(modalCtor).not.toHaveBeenCalled(); + expect(applyCopilotRootChange).not.toHaveBeenCalled(); + }); + + it("rejects a root that already contains notes without opening the confirm modal", () => { + copilotRootContainsNotes.mockReturnValue(true); + render(); + fireEvent.change(screen.getByLabelText("Copilot folder"), { target: { value: "existing" } }); + fireEvent.click(screen.getByRole("button", { name: "Apply Copilot folder" })); + expect(Notice).toHaveBeenCalledTimes(1); + expect(modalCtor).not.toHaveBeenCalled(); + expect(applyCopilotRootChange).not.toHaveBeenCalled(); + }); + + it("opens the confirm modal for a previously-used root even though it holds Copilot notes", () => { + // Re-activating a known Copilot root: its Markdown is Copilot's own leftover + // data and stays QA-excluded via history, so the note guard is skipped and + // the user reaches the confirmation instead of being blocked. + copilotRootContainsNotes.mockReturnValue(true); + isKnownCopilotRoot.mockReturnValue(true); + render(); + fireEvent.change(screen.getByLabelText("Copilot folder"), { target: { value: "old-root" } }); + fireEvent.click(screen.getByRole("button", { name: "Apply Copilot folder" })); + expect(modalCtor).toHaveBeenCalledTimes(1); + }); + + it("opens the confirm modal for a valid new root and applies the change on confirm", () => { + render(); + fireEvent.change(screen.getByLabelText("Copilot folder"), { target: { value: "ai" } }); + fireEvent.click(screen.getByRole("button", { name: "Apply Copilot folder" })); + expect(modalCtor).toHaveBeenCalledTimes(1); + expect(applyCopilotRootChange).not.toHaveBeenCalled(); + + capturedOnConfirm?.(); + expect(applyCopilotRootChange).toHaveBeenCalledWith("ai"); + }); + + it("does nothing when Apply is pressed with the current root unchanged", () => { + render(); + fireEvent.click(screen.getByRole("button", { name: "Apply Copilot folder" })); + expect(modalCtor).not.toHaveBeenCalled(); + expect(applyCopilotRootChange).not.toHaveBeenCalled(); + }); +}); diff --git a/src/settings/v2/components/BasicSettings.tsx b/src/settings/v2/components/BasicSettings.tsx index 1dcc2ba2..3fca901e 100644 --- a/src/settings/v2/components/BasicSettings.tsx +++ b/src/settings/v2/components/BasicSettings.tsx @@ -1,29 +1,111 @@ +import { ConfirmModal } from "@/components/modals/ConfirmModal"; import { Button } from "@/components/ui/button"; import { HelpTooltip } from "@/components/ui/help-tooltip"; import { Input } from "@/components/ui/input"; import { SettingItem } from "@/components/ui/setting-item"; import { SettingSection } from "@/components/ui/setting-section"; import { DEFAULT_OPEN_AREA, SEND_SHORTCUT } from "@/constants"; +import { useApp } from "@/context"; import { cn } from "@/lib/utils"; -import { updateSetting, useSettingsValue } from "@/settings/model"; +import { ensureCopilotSubfolders } from "@/settings/copilotFolder"; +import { + applyCopilotRootChange, + copilotRootContainsNotes, + isKnownCopilotRoot, +} from "@/settings/copilotRootChange"; +import { updateSetting, useSettingsValue, validateCopilotFolder } from "@/settings/model"; import { PlusSettings } from "@/settings/v2/components/PlusSettings"; import { formatDateTime } from "@/utils"; -import { Loader2 } from "lucide-react"; +import { revealFolderInExplorer } from "@/utils/revealFolderInExplorer"; +import { Folder, FolderSync, Loader2 } from "lucide-react"; import { Notice } from "obsidian"; -import React, { useState } from "react"; +import React, { useEffect, useState } from "react"; -// Read-only preview of the unified Copilot folder (PR-3 makes it editable and -// wires the real setting). Sub-folders derive from this root. The row is hidden -// until PR-3 lands — see the commented block in the render below. -// const COPILOT_FOLDER_PLACEHOLDER = "copilot"; +/** + * Body of the "Change Copilot folder" confirmation modal. Leads with a + * folder-sync icon header, then states the two things the user must know before + * committing: files are not moved, and the old root stays permanently excluded + * from search. + */ +function CopilotFolderChangeNotice({ oldRoot, newRoot }: { oldRoot: string; newRoot: string }) { + return ( +
+
+ +

Change Copilot folder

+
+

+ Copilot will keep new chats and data under {newRoot}/. Your files aren't + moved — your old data stays in {oldRoot}/, which + stays permanently excluded from search. Move it over if you want; Obsidian updates the + links. +

+
+ ); +} export const BasicSettings: React.FC = () => { + const app = useApp(); const settings = useSettingsValue(); const [isChecking, setIsChecking] = useState(false); const [conversationNoteName, setConversationNoteName] = useState( settings.defaultConversationNoteName || "{$date}_{$time}__{$topic}" ); + // Draft + explicit Apply for the Copilot root. Reason: persisting on every + // keystroke would re-point every derived sub-folder and trigger reload + // watchers per character; a root change also needs an up-front confirmation. + const persistedRoot = settings.copilotFolder; + const [folderDraft, setFolderDraft] = useState(persistedRoot); + + useEffect(() => { + /* eslint-disable @eslint-react/hooks-extra/no-direct-set-state-in-use-effect -- resync the draft when the persisted root changes underneath us (e.g. Reset Settings or a completed change) */ + setFolderDraft(persistedRoot); + /* eslint-enable @eslint-react/hooks-extra/no-direct-set-state-in-use-effect */ + }, [persistedRoot]); + + const applyFolderChange = () => { + const validation = validateCopilotFolder(folderDraft); + if (!validation.ok) { + new Notice(`Invalid Copilot folder: ${validation.reason}`, 5000); + return; + } + const folder = validation.folder; + if (folder === persistedRoot) { + new Notice("That's already the Copilot folder.", 3000); + return; + } + // Reason: a root that already holds ordinary notes would be excluded from + // search wholesale once activated, silently hiding those notes. A root + // Copilot used before is exempt — its only Markdown is Copilot's own + // leftover data (chats/memory/prompts), and it stays QA-excluded via + // history, so re-activating it hides nothing new. + if ( + !isKnownCopilotRoot(folder, settings.copilotRootHistory) && + copilotRootContainsNotes(app, folder) + ) { + new Notice( + `"${folder}" already contains notes. Choose a folder that isn't used for regular notes, otherwise those notes would be excluded from search.`, + 8000 + ); + return; + } + new ConfirmModal( + app, + () => { + void applyCopilotRootChange(folder) + .then(() => ensureCopilotSubfolders(app.vault, { copilotFolder: folder })) + .then(() => new Notice(`Copilot folder changed to "${folder}".`, 4000)) + .catch(() => new Notice("Failed to change the Copilot folder. Check the logs.", 5000)); + }, + , + "", + "Change folder", + "Cancel", + () => setFolderDraft(persistedRoot) + ).open(); + }; + const applyCustomNoteFormat = () => { setIsChecking(true); @@ -92,38 +174,6 @@ export const BasicSettings: React.FC = () => { ]} /> - {/* Copilot folder location — hidden until the next phase (PR-3) makes it - editable and wires the real `copilotFolder` setting + migration. - Shipping a disabled read-only preview now would just confuse users, so - keep it out of the UI until it does something. Restore this block when - PR-3 lands. */} - {/* - -
- - -
- - */} - { { label: "Shift + Enter", value: SEND_SHORTCUT.SHIFT_ENTER }, ]} /> + + +
+ setFolderDraft(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + applyFolderChange(); + } + }} + placeholder="copilot" + className="tw-w-full sm:tw-w-[160px]" + aria-label="Copilot folder" + /> + + +
+ {/* Saving Conversations Section */} @@ -162,24 +251,6 @@ export const BasicSettings: React.FC = () => { onCheckedChange={(checked) => updateSetting("autosaveChat", checked)} /> - updateSetting("defaultSaveFolder", value)} - placeholder="copilot/copilot-conversations" - /> - - updateSetting("defaultConversationTag", value)} - placeholder="ai-conversations" - /> - { }, [rawCommands]); const settings = useSettingsValue(); + // Derived from the single Copilot root; the folder is no longer separately + // editable, so the banner shows where commands are actually loaded from. + const customPromptsFolder = deriveCustomPromptsFolder(settings); const containerRef = useRef(null); const sensors = useSensors( useSensor(PointerSensor, { @@ -314,33 +312,6 @@ export const CommandSettings: React.FC = () => { await CustomCommandManager.getInstance().updateCommand(newCommand, prevCommandTitle); }; - // Draft + blur-commit for the custom-prompts folder field. Reason: persisting - // on every keystroke would fire updateSetting + a full command reload per - // character. Mirrors the draft/blur pattern used by other folder inputs. - const persistedPromptsFolder = settings.customPromptsFolder; - const [promptsFolderDraft, setPromptsFolderDraft] = useState(persistedPromptsFolder); - - useEffect(() => { - /* eslint-disable @eslint-react/hooks-extra/no-direct-set-state-in-use-effect -- resync the local draft when the persisted folder changes underneath us (e.g. Reset Settings) */ - setPromptsFolderDraft(persistedPromptsFolder); - /* eslint-enable @eslint-react/hooks-extra/no-direct-set-state-in-use-effect */ - }, [persistedPromptsFolder]); - - // Persist the custom-prompts folder and reload commands from the new location. - // Shared by the blur-commit on the text input and the folder-picker button. - const commitPromptsFolder = (value: string) => { - if (value === persistedPromptsFolder) return; - updateSetting("customPromptsFolder", value); - void loadAllCustomCommands(app).catch((err) => logError("loadAllCustomCommands failed", err)); - }; - - const handlePickPromptsFolder = () => { - new FolderSearchModal(app, (folder) => { - setPromptsFolderDraft(folder); - commitPromptsFolder(folder); - }).open(); - }; - const handleCreate = async (newCommand: CustomCommand) => { await CustomCommandManager.getInstance().createCommand(newCommand); }; @@ -436,8 +407,8 @@ export const CommandSettings: React.FC = () => { return (
-
-
+
+
Custom Commands
Preset prompts you trigger from the editor right-click menu or with a /{" "} @@ -449,44 +420,12 @@ export const CommandSettings: React.FC = () => { {" "}
Commands are automatically loaded from .md files in your custom prompts folder{" "} - {settings.customPromptsFolder}. Modifying the files will also update - the command settings. + {customPromptsFolder}. Modifying the files will also update the command + settings.
- -
- setPromptsFolderDraft(e.target.value)} - onBlur={(e) => commitPromptsFolder(e.target.value)} - onKeyDown={(e) => { - if (e.key === "Enter") { - e.preventDefault(); - commitPromptsFolder(promptsFolderDraft); - (e.target as HTMLInputElement).blur(); - } - }} - placeholder="copilot/copilot-custom-prompts" - className="!tw-w-56" - aria-label="Custom prompts folder" - /> - -
- { // vault kept out of Relay can flip allow_remote_read off per folder in Miyo. allow_remote_read: true, ...getMiyoFolderInclusions(settings.qaInclusions), - ...getMiyoFolderExclusions(settings.qaExclusions, extractAppIgnoreSettings(app)), + // Project the always-on system root exclusions (active + historical + // Copilot roots) alongside Obsidian's own ignore folders, so a former + // root's content isn't uploaded for indexing. This is a registration- + // time snapshot; MiyoSemanticRetriever re-applies the LIVE scope + // (createCopilotPatternFilter, which also enforces these roots) at + // query time, so correctness never depends on the snapshot. + ...getMiyoFolderExclusions(settings.qaExclusions, [ + ...getSystemExcludedFolders(settings), + ...extractAppIgnoreSettings(app), + ]), }, customUrl || undefined ); diff --git a/src/settings/v2/components/SelfHostSettings.tsx b/src/settings/v2/components/SelfHostSettings.tsx index 9b4a2755..b54a6006 100644 --- a/src/settings/v2/components/SelfHostSettings.tsx +++ b/src/settings/v2/components/SelfHostSettings.tsx @@ -87,7 +87,11 @@ export const SelfHostSettings: React.FC = () => { disabled={isValidating || isEligible === false} /> -
+
Privacy-first. While Self-Host is on, cloud diff --git a/src/system-prompts/SystemPromptAddModal.test.tsx b/src/system-prompts/SystemPromptAddModal.test.tsx new file mode 100644 index 00000000..9e196658 --- /dev/null +++ b/src/system-prompts/SystemPromptAddModal.test.tsx @@ -0,0 +1,40 @@ +import { DEFAULT_SETTINGS } from "@/constants"; +import { settingsAtom, settingsStore } from "@/settings/model"; +import { SystemPromptAddModalContent } from "@/system-prompts/SystemPromptAddModal"; +import { render, screen } from "@testing-library/react"; +import React from "react"; + +describe("SystemPromptAddModal", () => { + // The banner under test never touches `contentEl` (only the disabled template + // popover would), so a bare stub keeps the render free of DOM-global lint. + const contentEl = {} as unknown as HTMLElement; + const renderContent = () => + render( + + ); + + describe("SystemPromptAddModalContent", () => { + beforeEach(() => { + settingsStore.set(settingsAtom, { ...DEFAULT_SETTINGS }); + }); + + it("shows the system-prompts folder derived from the default root", () => { + settingsStore.set(settingsAtom, { ...DEFAULT_SETTINGS, copilotFolder: "copilot" }); + renderContent(); + expect(screen.getByText("copilot/system-prompts")).toBeTruthy(); + }); + + it("shows the system-prompts folder derived from a custom root after a root change", () => { + settingsStore.set(settingsAtom, { ...DEFAULT_SETTINGS, copilotFolder: "team-ai" }); + renderContent(); + expect(screen.getByText("team-ai/system-prompts")).toBeTruthy(); + // Guards the F4 fix: the banner must not fall back to the retired flat field. + expect(screen.queryByText("copilot/system-prompts")).toBeNull(); + }); + }); +}); diff --git a/src/system-prompts/SystemPromptAddModal.tsx b/src/system-prompts/SystemPromptAddModal.tsx index 30a7695c..77816712 100644 --- a/src/system-prompts/SystemPromptAddModal.tsx +++ b/src/system-prompts/SystemPromptAddModal.tsx @@ -12,6 +12,7 @@ import { validatePromptName } from "@/system-prompts/systemPromptUtils"; import { SystemPromptManager } from "@/system-prompts/systemPromptManager"; import { EMPTY_SYSTEM_PROMPT } from "@/system-prompts/constants"; import { useSettingsValue } from "@/settings/model"; +import { deriveSystemPromptsFolder } from "@/settings/copilotFolder"; import { SystemPromptSyntaxInstruction } from "@/components/SystemPromptSyntaxInstruction"; import { logError } from "@/logger"; @@ -69,7 +70,9 @@ interface SystemPromptAddModalContentProps { contentEl: HTMLElement; } -function SystemPromptAddModalContent({ +/** Exported for testing: the modal's React body, so the derived-path banner can + * be asserted without mounting the Obsidian `Modal` shell. */ +export function SystemPromptAddModalContent({ prompts, onConfirm, onCancel, @@ -124,8 +127,8 @@ function SystemPromptAddModalContent({
System prompts are automatically loaded from .md files in your system prompts folder{" "} - {settings.userSystemPromptsFolder}. Modifying the files will also update - the system prompt settings. + {deriveSystemPromptsFolder(settings)}. Modifying the files will also + update the system prompt settings.
diff --git a/src/system-prompts/systemPromptRegister.test.ts b/src/system-prompts/systemPromptRegister.test.ts index 473fc6ec..afab3a04 100644 --- a/src/system-prompts/systemPromptRegister.test.ts +++ b/src/system-prompts/systemPromptRegister.test.ts @@ -10,6 +10,7 @@ jest.mock("obsidian", () => ({ Plugin: jest.fn(), TFile: jest.fn(), Vault: jest.fn(), + normalizePath: (path: string) => path, })); // Mock logger @@ -41,6 +42,7 @@ jest.mock("@/system-prompts/systemPromptUtils", () => ({ lastUsedMs: 0, }), ensurePromptFrontmatter: jest.fn().mockResolvedValue(undefined), + updatePromptDefaultFlag: jest.fn().mockResolvedValue(undefined), })); // Mock settings @@ -267,10 +269,7 @@ describe("SystemPromptRegister", () => { ]); // Trigger folder change - settingsChangeHandler( - { userSystemPromptsFolder: "OldFolder" }, - { userSystemPromptsFolder: "NewFolder" } - ); + settingsChangeHandler({ copilotFolder: "OldFolder" }, { copilotFolder: "NewFolder" }); // Fast-forward debounce timer jest.advanceTimersByTime(300); @@ -303,10 +302,7 @@ describe("SystemPromptRegister", () => { ]); // Trigger folder change - settingsChangeHandler( - { userSystemPromptsFolder: "OldFolder" }, - { userSystemPromptsFolder: "NewFolder" } - ); + settingsChangeHandler({ copilotFolder: "OldFolder" }, { copilotFolder: "NewFolder" }); // Fast-forward debounce timer jest.advanceTimersByTime(300); @@ -339,10 +335,7 @@ describe("SystemPromptRegister", () => { ]); // Trigger folder change - settingsChangeHandler( - { userSystemPromptsFolder: "OldFolder" }, - { userSystemPromptsFolder: "NewFolder" } - ); + settingsChangeHandler({ copilotFolder: "OldFolder" }, { copilotFolder: "NewFolder" }); // Fast-forward debounce timer jest.advanceTimersByTime(300); @@ -357,20 +350,41 @@ describe("SystemPromptRegister", () => { expect(Notice).not.toHaveBeenCalled(); }); + it("passes the OLD derived folder to the default flag update when the root also changes", async () => { + const { updatePromptDefaultFlag } = jest.requireMock<{ + updatePromptDefaultFlag: jest.Mock; + }>("@/system-prompts/systemPromptUtils"); + const { getSettings } = jest.requireMock<{ getSettings: jest.Mock }>("@/settings/model"); + getSettings.mockReturnValue({ + defaultSystemPromptTitle: "NewDefault", + copilotFolder: "team/ai", + }); + + // Root changes copilot -> team/ai AND the default title changes in the same tick. + settingsChangeHandler( + { copilotFolder: "copilot", defaultSystemPromptTitle: "OldDefault" }, + { copilotFolder: "team/ai", defaultSystemPromptTitle: "NewDefault" } + ); + + await Promise.resolve(); + await Promise.resolve(); + + // Old flag cleared using the OLD derived folder (copilot/system-prompts), + // new flag set in the current (new) folder. + expect(updatePromptDefaultFlag).toHaveBeenCalledWith( + expect.anything(), + "OldDefault", + false, + "copilot/system-prompts" + ); + expect(updatePromptDefaultFlag).toHaveBeenCalledWith(expect.anything(), "NewDefault", true); + }); + it("debounces rapid folder changes", async () => { // Trigger multiple rapid folder changes - settingsChangeHandler( - { userSystemPromptsFolder: "Folder1" }, - { userSystemPromptsFolder: "Folder2" } - ); - settingsChangeHandler( - { userSystemPromptsFolder: "Folder2" }, - { userSystemPromptsFolder: "Folder3" } - ); - settingsChangeHandler( - { userSystemPromptsFolder: "Folder3" }, - { userSystemPromptsFolder: "Folder4" } - ); + settingsChangeHandler({ copilotFolder: "Folder1" }, { copilotFolder: "Folder2" }); + settingsChangeHandler({ copilotFolder: "Folder2" }, { copilotFolder: "Folder3" }); + settingsChangeHandler({ copilotFolder: "Folder3" }, { copilotFolder: "Folder4" }); // Fast-forward debounce timer jest.advanceTimersByTime(300); @@ -388,10 +402,7 @@ describe("SystemPromptRegister", () => { mockManager.fetchPrompts.mockRejectedValueOnce(new Error("Network error")); // Trigger folder change - settingsChangeHandler( - { userSystemPromptsFolder: "OldFolder" }, - { userSystemPromptsFolder: "NewFolder" } - ); + settingsChangeHandler({ copilotFolder: "OldFolder" }, { copilotFolder: "NewFolder" }); // Fast-forward debounce timer jest.advanceTimersByTime(300); @@ -435,18 +446,12 @@ describe("SystemPromptRegister", () => { .mockReturnValueOnce(promiseB); // Second call (request B) // Trigger first folder change (request A) - settingsChangeHandler( - { userSystemPromptsFolder: "Original" }, - { userSystemPromptsFolder: "FolderA" } - ); + settingsChangeHandler({ copilotFolder: "Original" }, { copilotFolder: "FolderA" }); jest.advanceTimersByTime(300); await Promise.resolve(); // Trigger second folder change (request B) before A completes - settingsChangeHandler( - { userSystemPromptsFolder: "FolderA" }, - { userSystemPromptsFolder: "FolderB" } - ); + settingsChangeHandler({ copilotFolder: "FolderA" }, { copilotFolder: "FolderB" }); jest.advanceTimersByTime(300); await Promise.resolve(); diff --git a/src/system-prompts/systemPromptRegister.ts b/src/system-prompts/systemPromptRegister.ts index b207d6a4..66688ff5 100644 --- a/src/system-prompts/systemPromptRegister.ts +++ b/src/system-prompts/systemPromptRegister.ts @@ -16,6 +16,7 @@ import { setSelectedPromptTitle, } from "@/system-prompts/state"; import { getSettings, subscribeToSettingsChange, updateSetting } from "@/settings/model"; +import { deriveSystemPromptsFolder } from "@/settings/copilotFolder"; import { SystemPromptManager } from "@/system-prompts/systemPromptManager"; import { debounce } from "@/utils/debounce"; import { logError, logInfo } from "@/logger"; @@ -83,13 +84,17 @@ export class SystemPromptRegister { prev: ReturnType, next: ReturnType ): void => { - const folderChanged = prev.userSystemPromptsFolder !== next.userSystemPromptsFolder; + // Reason: the folder is derived from the configurable copilotFolder root, so + // compare the derived paths rather than the retired userSystemPromptsFolder field. + const prevFolder = deriveSystemPromptsFolder(prev); + const nextFolder = deriveSystemPromptsFolder(next); + const folderChanged = prevFolder !== nextFolder; const defaultChanged = prev.defaultSystemPromptTitle !== next.defaultSystemPromptTitle; // Handle default change first, using the old folder path if folder also changed // This ensures we can find the old default file before the folder changes if (defaultChanged) { - const oldFolder = folderChanged ? prev.userSystemPromptsFolder : undefined; + const oldFolder = folderChanged ? prevFolder : undefined; void this.handleDefaultPromptChange( prev.defaultSystemPromptTitle, next.defaultSystemPromptTitle, @@ -98,7 +103,7 @@ export class SystemPromptRegister { } if (folderChanged) { - this.debouncedFolderChange(next.userSystemPromptsFolder); + this.debouncedFolderChange(nextFolder); } }; diff --git a/src/system-prompts/systemPromptUtils.test.ts b/src/system-prompts/systemPromptUtils.test.ts index c8a16ed3..57ad256a 100644 --- a/src/system-prompts/systemPromptUtils.test.ts +++ b/src/system-prompts/systemPromptUtils.test.ts @@ -7,7 +7,7 @@ import { generateCopyPromptName, } from "@/system-prompts/systemPromptUtils"; import { UserSystemPrompt } from "@/system-prompts/type"; -import { TFile, TAbstractFile, normalizePath } from "obsidian"; +import { TFile, TAbstractFile } from "obsidian"; import * as settingsModel from "@/settings/model"; import type { CopilotSettings } from "@/settings/model"; import { mockTFile } from "@/__tests__/mockObsidian"; @@ -26,6 +26,16 @@ jest.mock("@/settings/model", () => ({ })), })); +// The folder read point derives from copilotFolder in production; these tests +// exercise path-composition/file-matching, so the derived accessor is shimmed +// to the folder each test configures via getSettings().userSystemPromptsFolder. +jest.mock("@/settings/copilotFolder", () => { + const { getSettings } = jest.requireMock("@/settings/model"); + return { + getEffectiveSystemPromptsFolder: jest.fn(() => getSettings().userSystemPromptsFolder), + }; +}); + // Mock state management jest.mock("@/system-prompts/state", () => ({ updateCachedSystemPrompts: jest.fn(), @@ -116,7 +126,7 @@ describe("validatePromptName", () => { }); describe("getSystemPromptsFolder", () => { - it("returns the system prompts folder path from settings", () => { + it("returns the effective (copilotFolder-derived) system prompts folder", () => { jest.spyOn(settingsModel, "getSettings").mockReturnValue({ userSystemPromptsFolder: "CustomFolder/SystemPrompts", } as CopilotSettings); @@ -124,16 +134,6 @@ describe("getSystemPromptsFolder", () => { const result = getSystemPromptsFolder(); expect(result).toBe("CustomFolder/SystemPrompts"); }); - - it("normalizes the path", () => { - jest.spyOn(settingsModel, "getSettings").mockReturnValue({ - userSystemPromptsFolder: "SystemPrompts", - } as CopilotSettings); - - const result = getSystemPromptsFolder(); - expect(normalizePath).toHaveBeenCalled(); - expect(result).toBe("SystemPrompts"); - }); }); describe("getPromptFilePath", () => { diff --git a/src/system-prompts/systemPromptUtils.ts b/src/system-prompts/systemPromptUtils.ts index 197cf9bc..9adcfd84 100644 --- a/src/system-prompts/systemPromptUtils.ts +++ b/src/system-prompts/systemPromptUtils.ts @@ -7,7 +7,7 @@ import { } from "@/system-prompts/constants"; import { UserSystemPrompt } from "@/system-prompts/type"; import { App, normalizePath, TAbstractFile, TFile } from "obsidian"; -import { getSettings } from "@/settings/model"; +import { getEffectiveSystemPromptsFolder } from "@/settings/copilotFolder"; import { stripFrontmatter } from "@/utils"; import { updateCachedSystemPrompts, @@ -53,10 +53,10 @@ export function validatePromptName( } /** - * Get the system prompts folder path from settings + * Get the system prompts folder path, derived from the configurable copilotFolder root. */ export function getSystemPromptsFolder(): string { - return normalizePath(getSettings().userSystemPromptsFolder); + return getEffectiveSystemPromptsFolder(); } /** diff --git a/src/utils/revealFolderInExplorer.test.ts b/src/utils/revealFolderInExplorer.test.ts new file mode 100644 index 00000000..1ec73dc8 --- /dev/null +++ b/src/utils/revealFolderInExplorer.test.ts @@ -0,0 +1,56 @@ +import { revealFolderInExplorer } from "@/utils/revealFolderInExplorer"; +import { Notice, TFolder, type App } from "obsidian"; + +/** Build an App whose vault resolves `relPath` to `resolved` and exposes an + * optionally-enabled File Explorer with a spyable `revealInFolder`. */ +function makeApp(options: { + resolved: unknown; + explorerEnabled?: boolean; + hasReveal?: boolean; + revealInFolder?: jest.Mock; +}): App { + const instance = options.hasReveal ? { revealInFolder: options.revealInFolder } : {}; + return { + vault: { getAbstractFileByPath: () => options.resolved }, + internalPlugins: { + getPluginById: (id: string) => + id === "file-explorer" ? { enabled: options.explorerEnabled ?? true, instance } : undefined, + }, + } as unknown as App; +} + +describe("revealFolderInExplorer", () => { + describe("revealFolderInExplorer()", () => { + beforeEach(() => jest.clearAllMocks()); + + it("reveals the folder when it resolves to a TFolder and the explorer is enabled", () => { + const revealInFolder = jest.fn(); + const folder = new (TFolder as unknown as new (p: string) => TFolder)("copilot"); + revealFolderInExplorer( + makeApp({ resolved: folder, explorerEnabled: true, hasReveal: true, revealInFolder }), + "copilot" + ); + expect(revealInFolder).toHaveBeenCalledWith(folder); + expect(Notice).not.toHaveBeenCalled(); + }); + + it("shows a Notice when the folder is not in the vault cache", () => { + revealFolderInExplorer(makeApp({ resolved: null }), "copilot"); + expect(Notice).toHaveBeenCalledTimes(1); + }); + + it("shows a Notice when the File Explorer plugin is disabled", () => { + const folder = new (TFolder as unknown as new (p: string) => TFolder)("copilot"); + revealFolderInExplorer( + makeApp({ + resolved: folder, + explorerEnabled: false, + hasReveal: true, + revealInFolder: jest.fn(), + }), + "copilot" + ); + expect(Notice).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/src/utils/revealFolderInExplorer.ts b/src/utils/revealFolderInExplorer.ts new file mode 100644 index 00000000..66059b38 --- /dev/null +++ b/src/utils/revealFolderInExplorer.ts @@ -0,0 +1,37 @@ +import { logWarn } from "@/logger"; +import { App, Notice, TFolder } from "obsidian"; + +/** + * Reveal a vault-root-relative folder in Obsidian's built-in File Explorer. + * + * Surfaces a Notice instead of failing silently when the folder isn't in the + * vault cache yet (e.g. it hasn't been created — Copilot creates its folders + * lazily on first write) or the File Explorer core plugin is disabled. + * + * @param app - Active Obsidian app, threaded in rather than read from global. + * @param relPath - Vault-root-relative folder path to reveal. + */ +export function revealFolderInExplorer(app: App, relPath: string): void { + const folder = app.vault.getAbstractFileByPath(relPath); + if (!(folder instanceof TFolder)) { + new Notice(`Folder "${relPath}" doesn't exist yet — it's created on first use.`, 5000); + return; + } + const fileExplorer = ( + app as unknown as { + internalPlugins?: { + getPluginById?: ( + id: string + ) => + | { enabled?: boolean; instance?: { revealInFolder?: (folder: TFolder) => void } } + | undefined; + }; + } + ).internalPlugins?.getPluginById?.("file-explorer"); + if (fileExplorer?.enabled && fileExplorer.instance?.revealInFolder) { + fileExplorer.instance.revealInFolder(folder); + return; + } + logWarn("[settings] File Explorer plugin unavailable; cannot reveal folder."); + new Notice("File Explorer isn't enabled; can't reveal the folder.", 5000); +}