From d2ceecd2552e1d24e07aea3f4da56bde0694c4ee Mon Sep 17 00:00:00 2001 From: Erik van der Boom Date: Sun, 3 May 2026 08:59:38 +0200 Subject: [PATCH] feat: add review marker commands and enhance editor functionality in Obsidian plugin Co-authored-by: Copilot --- .github/copilot-instructions.md | 2 +- CONTRIBUTING.md | 22 +++++- README.md | 40 +++++----- mcp-ts/src/aisetup.ts | 4 +- obsidian-plugin/src/main.ts | 64 +++++++++++++++- obsidian-plugin/src/obsidian.d.ts | 25 ++++++- obsidian-plugin/test/main.test.ts | 115 +++++++++++++++++++++++++++-- obsidian-plugin/test/tsconfig.json | 9 +++ vscode-ext/package.json | 10 +++ 9 files changed, 256 insertions(+), 35 deletions(-) create mode 100644 obsidian-plugin/test/tsconfig.json diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 37a2404..d617077 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -123,7 +123,7 @@ Skills: `review`, `brainstorm`, `memory`, `translate`, `translation-review`, `st The **memory skill** uses `memory_list` → `memory_append` → `memory_compact`. Do not fall back to `get_text` + Edit tool for memory writes. ### AI setup versioning -`FILE_VERSION_INFO` in `mcp-ts/src/templates.ts` is a per-file version table/map (a Record keyed by output path) that controls staleness detection. +`FILE_VERSION_INFO` in `bindery-core/src/templates.ts` is a per-file version table/map (a Record keyed by output path) that controls staleness detection. - `setupAiFiles()` stamps `.bindery/ai-version.json` with the current version of each file after every run. - On extension activation, if `.bindery/settings.json` exists and a stamped version is older than the one in `FILE_VERSION_INFO`, the user is notified and offered an "Update now" button that opens `bindery.setupAI`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 237bba5..e8eb3b8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -63,6 +63,20 @@ lives: --- +## Host Feature Parity Policy + +`vscode-ext/` and `obsidian-plugin/` are two host implementations of the same +authoring feature set. Unless a feature is explicitly host-specific, any +functional change added to one host should be implemented in the other host in +the same PR (or in a clearly linked follow-up PR). + +When adding or changing commands: +- update command wiring in both hosts +- add or update tests in both hosts +- document intentional host-specific exceptions in the PR description + +--- + ## MCP stdio integration tests (`mcp-ts/test/integration-stdio.test.ts`) These tests spawn `node out/index.js` as a real child process and drive it over @@ -95,16 +109,18 @@ Tests cover: The workflow runs on every push and pull request on all branches. -A single `test` job runs the steps sequentially (the template-sync step must -occur between the mcp-ts and vscode-ext builds): +A single `test` job runs the steps sequentially: | Step | What it does | |---|---| +| Install + compile bindery-core | `npm ci` → `npm run compile` | +| Run bindery-core tests | `npm run test:ci` → uploads `test-results.json` artifact | | Install + compile mcp-ts | `npm ci` → `npm run compile` | | Run mcp-ts tests | `npm run test:ci` → uploads `test-results.json` artifact | -| Sync templates | Copies `mcp-ts/src/templates.ts` → `vscode-ext/src/ai-setup-templates.ts` | | Install + compile vscode-ext | `npm ci` → `npm run compile` | | Run vscode-ext tests | `npm run test:ci` → uploads `test-results.json` artifact | +| Install + compile obsidian-plugin | `npm ci` → `npm run compile` | +| Run obsidian-plugin tests | `npm run test:ci` → uploads `test-results.json` artifact | PRs **cannot be merged** unless the `test` job passes. diff --git a/README.md b/README.md index 04444b4..3cc0553 100644 --- a/README.md +++ b/README.md @@ -144,46 +144,50 @@ MIT — see [LICENSE](LICENSE). ## Contributing — template source of truth +## Host Parity Reminder + +`vscode-ext/` and `obsidian-plugin/` are two host implementations of the same +Bindery authoring feature set. Unless a change is intentionally host-specific, +functional changes in one host should be mirrored in the other in the same PR +or in a clearly linked follow-up PR. + The AI instruction file templates are maintained in **one place only**: ``` -mcp-ts/src/templates.ts ← SINGLE SOURCE OF TRUTH +bindery-core/src/templates.ts ← SINGLE SOURCE OF TRUTH ``` -`vscode-ext/src/ai-setup-templates.ts` is a generated copy and **must never be edited directly**. -CI syncs it automatically before every publish step. +`mcp-ts/src/templates.ts` is now a thin re-export shim to keep existing imports +stable. Consumers should continue importing from their local package entrypoint, +but template edits belong in `bindery-core/src/templates.ts`. ### Syncing locally -After changing `mcp-ts/src/templates.ts`, copy it to the VS Code extension: +After changing `bindery-core/src/templates.ts`, rebuild and test the workspace +(no template copy step is required): ```bash -cp mcp-ts/src/templates.ts vscode-ext/src/ai-setup-templates.ts +npm run build --workspace=bindery-core +npm test --workspace=bindery-core +npm test --workspace=mcp-ts +npm test --workspace=vscode-ext +npm test --workspace=obsidian-plugin ``` ### Running tests ```bash -# MCP server (includes template contract tests + copy-parity check) +# MCP server cd mcp-ts && npm test # VS Code extension cd vscode-ext && npm test ``` -The copy-parity test (`mcp-ts/test/templates-parity.test.ts`) skips gracefully when -`ai-setup-templates.ts` is absent (normal in fresh checkouts) and **fails** when it exists -but differs from the source — this is what CI catches. - ### What CI does The CI workflow (`.github/workflows/ci.yml`) runs on every push and pull request: -1. Builds and tests the MCP server (Ubuntu, Windows, macOS). -2. Copies `mcp-ts/src/templates.ts` → `vscode-ext/src/ai-setup-templates.ts`. -3. Verifies the copy is identical to the source (fails with a clear remediation message if not). -4. Builds and tests the VS Code extension. -5. Runs the **tool parity guard** (`scripts/check-tool-parity.mjs`) — verifies all MCP tools are registered consistently across all 5 surfaces (server, VS Code LM tools, package.json, mcpb manifest, implementation). -6. Enforces **coverage thresholds** (statements 80%, branches 65%, functions 90%, lines 80%) for both packages. - -If CI fails on the sync check, fix it by running the copy command above and committing the result. +1. Builds and tests `bindery-core`, `mcp-ts`, `vscode-ext`, and `obsidian-plugin` (Ubuntu, Windows, macOS). +2. Runs the **tool parity guard** (`scripts/check-tool-parity.mjs`) on coverage job. +3. Enforces **coverage thresholds** (statements 80%, branches 65%, functions 90%, lines 80%) across packages. diff --git a/mcp-ts/src/aisetup.ts b/mcp-ts/src/aisetup.ts index d413c47..b625431 100644 --- a/mcp-ts/src/aisetup.ts +++ b/mcp-ts/src/aisetup.ts @@ -5,8 +5,8 @@ * AGENTS.md, and .claude/skills//SKILL.md from the book's * .bindery/settings.json. * - * Templates live in templates.ts — the single source of truth. - * vscode-ext/src/ai-setup.ts imports its copy via ai-setup-templates.ts. + * Templates are owned by bindery-core/src/templates.ts (single source of truth). + * mcp-ts/src/templates.ts is a local re-export shim for stable imports. */ import * as fs from 'node:fs'; diff --git a/obsidian-plugin/src/main.ts b/obsidian-plugin/src/main.ts index e5c08f0..7939ba7 100644 --- a/obsidian-plugin/src/main.ts +++ b/obsidian-plugin/src/main.ts @@ -9,15 +9,18 @@ * inside the Obsidian app itself. */ -import { readWorkspaceSettings, BINDERY_FOLDER, SETTINGS_FILENAME } from '@bindery/core'; +import { BINDERY_FOLDER, SETTINGS_FILENAME } from '@bindery/core'; import { formatFile } from './formatter'; import { exportBook, resolveBookRoot } from './exporter'; import { BinderySettingsTab, DEFAULT_SETTINGS, type BinderySettings } from './settings-tab'; import { Plugin } from 'obsidian'; -import type { TFile } from 'obsidian'; +import type { Editor, TFile } from 'obsidian'; import * as fs from 'node:fs'; import * as path from 'node:path'; +const REVIEW_START_MARKER = ''; +const REVIEW_STOP_MARKER = ''; + export default class BinderyPlugin extends Plugin { settings: BinderySettings = { ...DEFAULT_SETTINGS }; @@ -63,6 +66,18 @@ export default class BinderyPlugin extends Plugin { callback: () => void this.formatActive(), }); + // Review marker commands + this.addCommand({ + id: 'start-review-marker', + name: 'Bindery: Insert Review Start Marker (or wrap selection)', + editorCallback: (editor) => this.insertStartReviewMarker(editor), + }); + this.addCommand({ + id: 'stop-review-marker', + name: 'Bindery: Insert Review Stop Marker', + editorCallback: (editor) => this.insertStopReviewMarker(editor), + }); + // Export commands for (const fmt of ['md', 'docx', 'epub', 'pdf'] as const) { const label = fmt.toUpperCase(); @@ -95,6 +110,45 @@ export default class BinderyPlugin extends Plugin { // This is a placeholder; full implementation requires the Obsidian runtime API. } + private insertStartReviewMarker(editor: Editor): void { + if (!this.isMarkdownActive()) { + return; + } + + const selection = editor.getSelection(); + if (selection.length > 0) { + const start = editor.getCursor('from'); + const end = editor.getCursor('to'); + const startWrap = this.lineBoundaryWrap(editor, start.line, start.ch, REVIEW_START_MARKER); + const stopWrap = this.lineBoundaryWrap(editor, end.line, end.ch, REVIEW_STOP_MARKER); + editor.replaceSelection(`${startWrap}${selection}${stopWrap}`); + return; + } + + const pos = editor.getCursor(); + editor.replaceRange(this.lineBoundaryWrap(editor, pos.line, pos.ch, REVIEW_START_MARKER), pos); + } + + private insertStopReviewMarker(editor: Editor): void { + if (!this.isMarkdownActive()) { + return; + } + const pos = editor.getCursor(); + editor.replaceRange(this.lineBoundaryWrap(editor, pos.line, pos.ch, REVIEW_STOP_MARKER), pos); + } + + private lineBoundaryWrap(_editor: Editor, _line: number, ch: number, marker: string): string { + const atLineStart = ch === 0; + const prefix = atLineStart ? '' : '\n'; + const suffix = '\n'; + return `${prefix}${marker}${suffix}`; + } + + private isMarkdownActive(): boolean { + const active = this.app.workspace?.getActiveFile(); + return active?.extension === 'md'; + } + private async initWorkspace(): Promise { const vaultPath = this.getVaultBasePath(); const trimmedBookRoot = this.settings.bookRoot.trim(); @@ -157,7 +211,9 @@ export default class BinderyPlugin extends Plugin { async loadSettings(): Promise { const loaded = await this.loadData() as Partial | null; - this.settings = Object.assign({}, DEFAULT_SETTINGS, loaded ?? {}); + this.settings = loaded + ? { ...DEFAULT_SETTINGS, ...loaded } + : { ...DEFAULT_SETTINGS }; } async saveSettings(): Promise { @@ -166,4 +222,4 @@ export default class BinderyPlugin extends Plugin { } // Re-export for consumers (e.g. settings tab, tests) -export { readWorkspaceSettings }; +export { readWorkspaceSettings } from '@bindery/core'; diff --git a/obsidian-plugin/src/obsidian.d.ts b/obsidian-plugin/src/obsidian.d.ts index 219bdf9..573fd00 100644 --- a/obsidian-plugin/src/obsidian.d.ts +++ b/obsidian-plugin/src/obsidian.d.ts @@ -1,4 +1,17 @@ declare module 'obsidian' { + export interface EditorPosition { + line: number; + ch: number; + } + + export interface Editor { + getCursor(which?: 'from' | 'to' | 'head' | 'anchor'): EditorPosition; + getLine(line: number): string; + getSelection(): string; + replaceSelection(text: string): void; + replaceRange(text: string, from: EditorPosition, to?: EditorPosition): void; + } + export interface EventRef { [key: string]: unknown; } @@ -20,6 +33,16 @@ declare module 'obsidian' { export interface App { vault: Vault; + workspace?: { + getActiveFile(): TFile | null; + }; + } + + export interface Command { + id: string; + name: string; + callback?: () => void | Promise; + editorCallback?: (editor: Editor) => void; } export class Plugin { @@ -27,7 +50,7 @@ declare module 'obsidian' { constructor(app: App); loadData(): Promise; saveData(data: unknown): Promise; - addCommand(command: { id: string; name: string; callback: () => void | Promise }): void; + addCommand(command: Command): void; addSettingTab(tab: unknown): void; registerEvent(eventRef: EventRef): void; } diff --git a/obsidian-plugin/test/main.test.ts b/obsidian-plugin/test/main.test.ts index d062079..04a48c9 100644 --- a/obsidian-plugin/test/main.test.ts +++ b/obsidian-plugin/test/main.test.ts @@ -1,3 +1,6 @@ +/// +/// + import { describe, it, expect, vi, beforeEach } from 'vitest'; import * as os from 'node:os'; import * as path from 'node:path'; @@ -22,18 +25,23 @@ vi.mock('obsidian', () => { return Promise.resolve(); } - addCommand(_command: { id: string; name: string; callback: () => void | Promise }): void {} + addCommand(_command: { + id: string; + name: string; + callback?: () => void | Promise; + editorCallback?: (editor: unknown) => void; + }): void { return; } - addSettingTab(_tab: unknown): void {} + addSettingTab(_tab: unknown): void { return; } - registerEvent(_eventRef: unknown): void {} + registerEvent(_eventRef: unknown): void { return; } } return { Plugin }; }); import BinderyPlugin from '../src/main'; -import type { App, Vault } from 'obsidian'; +import type { App, Vault, Command, Editor } from 'obsidian'; // ─── Mock helpers ───────────────────────────────────────────────────────────── @@ -48,6 +56,21 @@ function makeApp(vaultPath: string, vaultName = 'TestVault'): App { return { vault }; } +function makeEditor(lineText: string, cursorCh: number, selection = ''): Editor { + return { + getCursor: vi.fn((which?: 'from' | 'to' | 'head' | 'anchor') => { + if (which === 'to') { + return { line: 0, ch: cursorCh + selection.length }; + } + return { line: 0, ch: cursorCh }; + }), + getLine: vi.fn(() => lineText), + getSelection: vi.fn(() => selection), + replaceSelection: vi.fn(), + replaceRange: vi.fn(), + }; +} + // ─── showMcpSnippet ─────────────────────────────────────────────────────────── describe('showMcpSnippet', () => { @@ -160,7 +183,7 @@ describe('formatOnSave bookRoot scoping', () => { // Capture the vault.on callback before onload let savedCallback: ((...args: unknown[]) => void) | undefined; - vi.spyOn(app.vault, 'on').mockImplementation((_event, cb) => { + vi.spyOn(app.vault, 'on').mockImplementation((_event: string, cb: (...args: unknown[]) => unknown) => { savedCallback = cb as (...args: unknown[]) => void; return {}; }); @@ -184,7 +207,7 @@ describe('formatOnSave bookRoot scoping', () => { const bp = new BinderyPlugin(app); let savedCallback: ((...args: unknown[]) => void) | undefined; - vi.spyOn(app.vault, 'on').mockImplementation((_event, cb) => { + vi.spyOn(app.vault, 'on').mockImplementation((_event: string, cb: (...args: unknown[]) => unknown) => { savedCallback = cb as (...args: unknown[]) => void; return {}; }); @@ -209,3 +232,83 @@ describe('formatOnSave bookRoot scoping', () => { fs.rmSync(vaultPath, { recursive: true, force: true }); }); }); + +describe('review marker commands', () => { + it('registers start/stop review commands', async () => { + const app = makeApp('/vault', 'MyVault'); + app.workspace = { + getActiveFile: () => ({ path: 'Story/ch1.md', extension: 'md', name: 'ch1.md', basename: 'ch1' }), + }; + const bp = new BinderyPlugin(app); + + const addCommandSpy = vi.spyOn(bp, 'addCommand'); + await bp.onload(); + + const commands = addCommandSpy.mock.calls.map((call) => call[0] as Command); + const ids = commands.map((c) => c.id); + expect(ids).toContain('start-review-marker'); + expect(ids).toContain('stop-review-marker'); + }); + + it('start-review-marker inserts review start marker at cursor', async () => { + const app = makeApp('/vault', 'MyVault'); + app.workspace = { + getActiveFile: () => ({ path: 'Story/ch1.md', extension: 'md', name: 'ch1.md', basename: 'ch1' }), + }; + const bp = new BinderyPlugin(app); + const addCommandSpy = vi.spyOn(bp, 'addCommand'); + await bp.onload(); + + const commands = addCommandSpy.mock.calls.map((call) => call[0] as Command); + const start = commands.find((c) => c.id === 'start-review-marker'); + const editor = makeEditor('abc', 0, ''); + + start?.editorCallback?.(editor); + + expect(editor.replaceRange).toHaveBeenCalledWith( + '\n', + { line: 0, ch: 0 }, + ); + }); + + it('start-review-marker wraps selection with start/stop markers', async () => { + const app = makeApp('/vault', 'MyVault'); + app.workspace = { + getActiveFile: () => ({ path: 'Story/ch1.md', extension: 'md', name: 'ch1.md', basename: 'ch1' }), + }; + const bp = new BinderyPlugin(app); + const addCommandSpy = vi.spyOn(bp, 'addCommand'); + await bp.onload(); + + const commands = addCommandSpy.mock.calls.map((call) => call[0] as Command); + const start = commands.find((c) => c.id === 'start-review-marker'); + const editor = makeEditor('abc', 0, 'X'); + + start?.editorCallback?.(editor); + + expect(editor.replaceSelection).toHaveBeenCalledWith( + '\nX\n\n', + ); + }); + + it('stop-review-marker inserts review stop marker at cursor', async () => { + const app = makeApp('/vault', 'MyVault'); + app.workspace = { + getActiveFile: () => ({ path: 'Story/ch1.md', extension: 'md', name: 'ch1.md', basename: 'ch1' }), + }; + const bp = new BinderyPlugin(app); + const addCommandSpy = vi.spyOn(bp, 'addCommand'); + await bp.onload(); + + const commands = addCommandSpy.mock.calls.map((call) => call[0] as Command); + const stop = commands.find((c) => c.id === 'stop-review-marker'); + const editor = makeEditor('abc', 0, ''); + + stop?.editorCallback?.(editor); + + expect(editor.replaceRange).toHaveBeenCalledWith( + '\n', + { line: 0, ch: 0 }, + ); + }); +}); diff --git a/obsidian-plugin/test/tsconfig.json b/obsidian-plugin/test/tsconfig.json new file mode 100644 index 0000000..0c13deb --- /dev/null +++ b/obsidian-plugin/test/tsconfig.json @@ -0,0 +1,9 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "rootDir": "..", + "noEmit": true, + "types": ["node", "vitest/globals"] + }, + "include": ["**/*.ts", "../src/**/*.ts"] +} diff --git a/vscode-ext/package.json b/vscode-ext/package.json index f1d0f8e..dcef63d 100644 --- a/vscode-ext/package.json +++ b/vscode-ext/package.json @@ -1033,6 +1033,16 @@ "when": "resourceLangId == markdown", "group": "2_dialect@3" }, + { + "command": "bindery.startReviewMarker", + "when": "resourceLangId == markdown", + "group": "2_review@1" + }, + { + "command": "bindery.stopReviewMarker", + "when": "resourceLangId == markdown", + "group": "2_review@2" + }, { "command": "bindery.mergeMarkdown", "group": "3_export@1"