From f4da806b1c61806ea7bb552515f1ab67a7fc71c5 Mon Sep 17 00:00:00 2001 From: Logan Yang Date: Mon, 22 Jun 2026 20:31:04 -0700 Subject: [PATCH] feat(agent-settings): tab the Agents settings into per-backend panels (#2632) * feat(agent-settings): tab the Agents settings into per-backend panels Convert the Agents settings tab from stacked vertical sections into a sub-tab strip (OpenCode / Claude / Codex / Quick Chat) built on the existing setting-tabs primitive. The global default-backend picker and MCP servers panel stay above the strip. Within each backend panel the default-model picker now sits above the model enable list, followed by the backend's binary/auth config. Quick Chat keeps its existing model curation. Placement-only revamp; no default-model resolution, seeding, or persistence changes. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_017Swdw8vKE3zczYJFzAboWG * fix(agent-settings): anchor tab strip on switch; add Quick Chat icon + default-model picker Switching sub-tabs no longer jumps the settings scroll: the tab strip is pinned to its pre-switch viewport position when panel heights differ. The Quick Chat tab gains a chat-bubble icon (was iconless) and a Default model picker mirroring the per-agent panels, writing the shared defaultModelKey. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_017Swdw8vKE3zczYJFzAboWG * chore(scripts): reload deploy-target vault by cwd; timestamp-only build label The Obsidian CLI resolves its target vault from the working directory, so the reload step now runs from $VAULT_PATH (was hitting the repo's own vault). The manifest label is tagged with the build timestamp only, not the branch name. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_017Swdw8vKE3zczYJFzAboWG --------- Co-authored-by: Claude Opus 4.8 --- scripts/test-vault.sh | 35 +++-- .../v2/components/AgentSettings.test.tsx | 115 +++++++++++++++ src/settings/v2/components/AgentSettings.tsx | 134 +++++++++++++++--- 3 files changed, 255 insertions(+), 29 deletions(-) create mode 100644 src/settings/v2/components/AgentSettings.test.tsx diff --git a/scripts/test-vault.sh b/scripts/test-vault.sh index 4b41a41e..f47a94ef 100755 --- a/scripts/test-vault.sh +++ b/scripts/test-vault.sh @@ -95,28 +95,45 @@ for f in main.js styles.css; do fi done -# Write a branch- and timestamp-tagged manifest.json (real file, not a symlink) -# so Obsidian's Community plugins list visibly reflects which worktree/branch -# is loaded and when this build was deployed. +# Write a timestamp-tagged manifest.json (real file, not a symlink). The plugin +# NAME (shown in Obsidian's Community plugins list / sidebar) carries the build +# timestamp ONLY — never the branch/worktree name, which is not a reliable signal +# of what code is actually loaded. The DESCRIPTION still carries `branch: ` +# because the `npm run test:vault` preflight in TESTING_GUIDE.md greps it +# (`/branch: ([^ |]+)/`) to catch deploying from the wrong worktree when several +# worktrees share one vault. BRANCH="$(git -C "$WORKTREE_ROOT" rev-parse --abbrev-ref HEAD 2>/dev/null || echo unknown)" BUILD_TS="$(date +%Y%m%d-%H%M%S)" -echo "==> Writing branch-tagged manifest.json (branch: $BRANCH, build: $BUILD_TS)" +echo "==> Writing timestamp-tagged manifest.json (name build: $BUILD_TS, branch: $BRANCH)" rm -f "$PLUGIN_DIR/manifest.json" SRC="$WORKTREE_ROOT/manifest.json" DEST="$PLUGIN_DIR/manifest.json" BRANCH="$BRANCH" BUILD_TS="$BUILD_TS" node -e ' const fs = require("fs"); const m = JSON.parse(fs.readFileSync(process.env.SRC, "utf8")); - m.name = m.name + " [" + process.env.BRANCH + " @ " + process.env.BUILD_TS + "]"; + m.name = m.name + " [" + process.env.BUILD_TS + "]"; m.description = "[branch: " + process.env.BRANCH + " | build: " + process.env.BUILD_TS + "] " + m.description; fs.writeFileSync(process.env.DEST, JSON.stringify(m, null, 2) + "\n"); ' -echo "==> Reloading plugin in Obsidian" +# Reload by toggling disable -> enable, NOT `plugin:reload`. On this setup +# `plugin:reload` returns success but does NOT re-run the plugin's onload, so the +# freshly deployed main.js never executes. A disable+enable cycle re-runs onload. +# +# CRITICAL: the Obsidian CLI picks its TARGET VAULT from the current working +# directory (it resolves the vault enclosing $PWD; `vault=` does NOT override +# this). So we MUST run the CLI from inside the target vault's directory, or the +# reload silently hits whatever vault the caller's cwd sits in (e.g. the +# repo/worktree vault) instead of the deploy target. Hence the `cd "$VAULT_PATH"`. +echo "==> Reloading plugin in Obsidian (vault dir: $VAULT_PATH)" if [[ ! -x "$OBSIDIAN_BIN" ]]; then echo "warning: Obsidian CLI not found at $OBSIDIAN_BIN; skipping reload." >&2 else - if ! "$OBSIDIAN_BIN" plugin:enable id="$PLUGIN_ID" >/dev/null 2>&1 \ - || ! "$OBSIDIAN_BIN" plugin:reload id="$PLUGIN_ID" >/dev/null 2>&1; then - echo "warning: Obsidian doesn't appear to be running. Start it and the symlinked plugin will load on next open." >&2 + ( cd "$VAULT_PATH" && "$OBSIDIAN_BIN" plugin:disable id="$PLUGIN_ID" >/dev/null 2>&1 ) || true + if ( cd "$VAULT_PATH" && "$OBSIDIAN_BIN" plugin:enable id="$PLUGIN_ID" >/dev/null 2>&1 ); then + echo " reloaded (onload re-ran). Note: the sidebar manifest label only" + echo " refreshes on a full Obsidian restart; use a dev-console marker to" + echo " confirm the loaded build, not the label." + else + echo "warning: could not reload via the CLI. Is Obsidian running with this vault open? The plugin will load on next open." >&2 fi fi diff --git a/src/settings/v2/components/AgentSettings.test.tsx b/src/settings/v2/components/AgentSettings.test.tsx new file mode 100644 index 00000000..38ddb356 --- /dev/null +++ b/src/settings/v2/components/AgentSettings.test.tsx @@ -0,0 +1,115 @@ +import { fireEvent, render, screen, within } from "@testing-library/react"; +import React from "react"; +import { AgentSettings } from "./AgentSettings"; + +jest.mock("@/logger", () => ({ logInfo: jest.fn(), logWarn: jest.fn(), logError: jest.fn() })); + +jest.mock("obsidian", () => ({ Platform: { isMobile: false } })); + +jest.mock("@/settings/model", () => ({ + // eslint-disable-next-line @eslint-react/hooks-extra/no-unnecessary-use-prefix -- mocks the real hook; name must match the export + useSettingsValue: () => ({ agentMode: { activeBackend: "opencode", backends: {} } }), + setSettings: jest.fn(), + updateSetting: jest.fn(), +})); + +// Mock the chat-backend options hook to avoid pulling the heavy @/modelManagement +// dependency chain (ByokPanel -> ConfirmModal extends Modal) into the test. +jest.mock("@/hooks/useChatBackendModelOptions", () => ({ + // eslint-disable-next-line @eslint-react/hooks-extra/no-unnecessary-use-prefix -- mocks the real hook; name must match the export + useChatBackendModelOptions: () => ({ options: [], resolveSelectionId: () => undefined }), +})); + +const Icon: React.FC<{ className?: string }> = () => ; + +function makeDescriptor(id: string, displayName: string) { + return { + id, + displayName, + Icon, + getInstallState: () => ({ kind: "ready" as const }), + getResolvedBinaryPath: () => null, + openInstallUI: jest.fn(), + SettingsPanel: () =>
settings panel
, + }; +} + +const DESCRIPTORS = [ + makeDescriptor("opencode", "OpenCode"), + makeDescriptor("claude", "Claude"), + makeDescriptor("codex", "Codex"), +]; + +jest.mock("@/agentMode", () => ({ + listBackendDescriptors: () => DESCRIPTORS, + InstallBadge: () => , + McpServersPanel: () =>
mcp
, + AgentDefaultModelSetting: ({ descriptor }: { descriptor: { id: string } }) => ( +
default model
+ ), +})); + +jest.mock("@/contexts/PluginContext", () => ({ + // eslint-disable-next-line @eslint-react/hooks-extra/no-unnecessary-use-prefix -- mocks the real `usePlugin` hook; the name must match the export + usePlugin: () => ({ + app: {}, + agentSessionManager: { + getCachedBackendState: () => ({ model: "x" }), + preloadModels: jest.fn().mockResolvedValue(undefined), + }, + }), +})); + +jest.mock("./ChatModelEnableList", () => ({ + ChatModelEnableList: () =>
chat models
, +})); + +jest.mock("./ConfiguredModelEnableList", () => ({ + ConfiguredModelEnableList: ({ descriptor }: { descriptor: { id: string } }) => ( +
model list
+ ), +})); + +describe("AgentSettings", () => { + it("renders the four sub-tabs in order: OpenCode, Claude, Codex, Quick Chat", () => { + render(); + const tabs = screen.getAllByRole("tab").map((t) => t.textContent); + expect(tabs).toEqual(["OpenCode", "Claude", "Codex", "Quick Chat"]); + }); + + it("keeps global items (Default backend + MCP) outside the tab strip", () => { + render(); + const tablist = screen.getByRole("tablist"); + expect(within(tablist).queryByText("Default backend")).toBeNull(); + expect(within(tablist).queryByTestId("mcp-panel")).toBeNull(); + expect(screen.getByText("Default backend")).not.toBeNull(); + expect(screen.getByTestId("mcp-panel")).not.toBeNull(); + }); + + it("shows the first backend's content by default and the default-model picker above the model list", () => { + render(); + const picker = screen.getByTestId("default-model-opencode"); + const list = screen.getByTestId("model-list-opencode"); + expect(picker).not.toBeNull(); + expect(list).not.toBeNull(); + // DOCUMENT_POSITION_FOLLOWING means `list` comes after `picker` in the DOM. + expect(picker.compareDocumentPosition(list) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); + // Other backends' panels are not mounted while their tab is unselected. + expect(screen.queryByTestId("default-model-codex")).toBeNull(); + }); + + it("switches to the selected backend's content", () => { + render(); + expect(screen.queryByTestId("model-list-codex")).toBeNull(); + fireEvent.click(screen.getByRole("tab", { name: "Codex" })); + expect(screen.getByTestId("model-list-codex")).not.toBeNull(); + expect(screen.queryByTestId("model-list-opencode")).toBeNull(); + }); + + it("shows the Quick Chat model list on the Quick Chat tab", () => { + render(); + expect(screen.queryByTestId("chat-model-list")).toBeNull(); + fireEvent.click(screen.getByText("Quick Chat")); + expect(screen.getByTestId("chat-model-list")).not.toBeNull(); + }); +}); diff --git a/src/settings/v2/components/AgentSettings.tsx b/src/settings/v2/components/AgentSettings.tsx index c402ccfa..e3d5e0d2 100644 --- a/src/settings/v2/components/AgentSettings.tsx +++ b/src/settings/v2/components/AgentSettings.tsx @@ -8,30 +8,71 @@ import { } from "@/agentMode"; import { Button } from "@/components/ui/button"; import { SettingItem } from "@/components/ui/setting-item"; +import { TabContent, TabItem, type TabItem as TabItemType } from "@/components/ui/setting-tabs"; import { TruncatedText } from "@/components/TruncatedText"; import { usePlugin } from "@/contexts/PluginContext"; +import { useChatBackendModelOptions } from "@/hooks/useChatBackendModelOptions"; import { logError } from "@/logger"; -import { setSettings, useSettingsValue } from "@/settings/model"; +import { setSettings, updateSetting, useSettingsValue } from "@/settings/model"; import { formatBinaryPathForDisplay } from "@/utils/binaryPath"; +import { MessageCircle } from "lucide-react"; import { Platform } from "obsidian"; import React from "react"; import { ChatModelEnableList } from "./ChatModelEnableList"; import { ConfiguredModelEnableList } from "./ConfiguredModelEnableList"; /** - * Explicit ordering for backend sections. Keeps Opencode → Claude → Codex + * Explicit ordering for backend sub-tabs. Keeps Opencode → Claude → Codex * regardless of what `listBackendDescriptors()` returns. */ const BACKEND_ORDER: BackendId[] = ["opencode", "claude", "codex"]; +/** Synthetic sub-tab id for the (non-backend) Quick Chat model curation. */ +const QUICK_CHAT_TAB_ID = "quickchat"; + +/** Nearest scrollable ancestor, used to keep the tab strip anchored on switch. */ +function getScrollableParent(el: HTMLElement): HTMLElement | null { + let node: HTMLElement | null = el.parentElement; + while (node) { + const overflowY = getComputedStyle(node).overflowY; + if (overflowY === "auto" || overflowY === "scroll") return node; + node = node.parentElement; + } + return null; +} + /** - * Top-level "Agents" settings tab. Owns the master agent-mode toggle, the - * default backend picker, the MCP server panel, and one per-backend section - * (binary path + model curation). + * Top-level "Agents" settings tab. Owns the global default-backend picker and + * the MCP server panel, then a sub-tab strip with one panel per backend plus a + * Quick Chat panel. Each backend panel curates that backend's default model, + * enabled models, and binary/auth config. */ export const AgentSettings: React.FC = () => { const settings = useSettingsValue(); const plugin = usePlugin(); + const [selectedTab, setSelectedTab] = React.useState(BACKEND_ORDER[0]); + const tabStripRef = React.useRef(null); + const pendingAnchorTop = React.useRef(null); + + // Panels vary widely in height (opencode's model list is long, Quick Chat is + // short), so switching to a shorter one clamps the settings scroll and jumps + // the view. Pin the tab strip to its pre-switch viewport position so only the + // content below it changes. + React.useLayoutEffect(() => { + const strip = tabStripRef.current; + if (!strip || pendingAnchorTop.current === null) return; + const scroller = getScrollableParent(strip); + if (scroller) { + const delta = strip.getBoundingClientRect().top - pendingAnchorTop.current; + if (delta !== 0) scroller.scrollTop += delta; + } + pendingAnchorTop.current = null; + }, [selectedTab]); + + const handleSelectTab = React.useCallback((id: string) => { + pendingAnchorTop.current = tabStripRef.current?.getBoundingClientRect().top ?? null; + setSelectedTab(id); + }, []); if (Platform.isMobile) { return ( @@ -49,6 +90,15 @@ export const AgentSettings: React.FC = () => { allDescriptors.find((d) => d.id === id) ).filter((d): d is BackendDescriptor => d !== undefined); + const tabs: TabItemType[] = [ + ...orderedDescriptors.map((d) => ({ + id: d.id, + icon: , + label: d.displayName, + })), + { id: QUICK_CHAT_TAB_ID, icon: , label: "Quick Chat" }, + ]; + return (
Agents (alpha)
@@ -66,43 +116,87 @@ export const AgentSettings: React.FC = () => { - {orderedDescriptors.map((descriptor) => ( - - ))} +
+
+ {tabs.map((tab, index) => ( + handleSelectTab(tab.id)} + isFirst={index === 0} + isLast={index === tabs.length - 1} + /> + ))} +
- + {orderedDescriptors.map((descriptor) => ( + + + + ))} + + + +
); }; /** - * Quick Chat curation card: which models appear in the (non-agent) chat model + * Quick Chat curation panel: which models appear in the (non-agent) chat model * picker. Lives under Agents per the model-management design (chat is a * first-class curation backend alongside the agents). Models come from the * BYOK / Plus registries — chat doesn't own providers. */ -const QuickChatSection: React.FC = () => { +const QuickChatPanel: React.FC = () => { + const settings = useSettingsValue(); + const { options: chatModelOptions, resolveSelectionId } = useChatBackendModelOptions(); + const resolvedDefaultModelId = resolveSelectionId(settings.defaultModelKey); + const hasDefault = resolvedDefaultModelId !== undefined; + return ( -
+
Quick Chat models Models shown in the chat model picker. Add providers on the Models (BYOK) tab.
+ { + if (value === "Select Model") return; + updateSetting("defaultModelKey", value); + }} + options={ + hasDefault + ? chatModelOptions + : [{ label: "Select Model", value: "Select Model" }, ...chatModelOptions] + } + placeholder="Model" + />
); }; /** - * One per-backend block: heading, binary install panel, and the model enable - * list. If the backend is installed but no catalog is cached yet, it kicks a - * probe so discovery enrolls the reported models, which then populate the list - * (the list reads the model-management registry, not the probe state). + * One per-backend panel: install header, then (when ready) the default-model + * picker above the model enable list, then the binary/auth config. If the + * backend is installed but no catalog is cached yet, it kicks a probe so + * discovery enrolls the reported models, which then populate the list (the + * list reads the model-management registry, not the probe state). */ -const BackendSection: React.FC<{ +const BackendPanel: React.FC<{ descriptor: BackendDescriptor; plugin: ReturnType; }> = ({ descriptor, plugin }) => { @@ -127,7 +221,7 @@ const BackendSection: React.FC<{ const Icon = descriptor.Icon; return ( -
+
@@ -153,12 +247,12 @@ const BackendSection: React.FC<{
- {installState.kind === "ready" && } - {installState.kind === "ready" && manager && ( )} + {installState.kind === "ready" && } + {Panel && }
);