From aa47cdf9422ffa9c93363086e5c91f6f5b5ec820 Mon Sep 17 00:00:00 2001 From: Slate Rehm Date: Sat, 8 Aug 2026 16:36:21 -0500 Subject: [PATCH 1/3] refactor: simplify agent sessions --- .agents/plugins/marketplace.json | 4 +- .claude-plugin/marketplace.json | 4 +- .claude-plugin/plugin.json | 19 +- .codex-plugin/plugin.json | 19 +- .cursor-plugin/plugin.json | 19 +- .github/workflows/release.yml | 2 +- AGENTS.md | 24 +- README.md | 156 ++--- SECURITY.md | 4 +- agents/obsidian-tester.md | 7 +- commands/obsidian-dev.md | 7 +- commands/obsidian-doctor.md | 4 +- docs/configuration.md | 105 ++- docs/hosts.md | 6 +- package-lock.json | 4 +- package.json | 4 +- rules/obsidian-plugin.mdc | 8 + scripts/acceptance.mjs | 28 +- scripts/background-input-live.mjs | 30 +- scripts/ci-smoke.mjs | 107 +--- scripts/e2e.mjs | 122 ++-- scripts/fence-live.mjs | 41 +- scripts/lib/live-harness.mjs | 32 +- scripts/workspaces-live.mjs | 381 ++--------- skills/obsidian-debugging/SKILL.md | 12 +- skills/obsidian-instance-setup/SKILL.md | 116 ++-- skills/obsidian-plugin-dev/SKILL.md | 43 +- skills/obsidian-ui-automation/SKILL.md | 16 +- src/agent/store.ts | 191 ------ src/audit/event.ts | 8 - src/audit/types.ts | 12 +- src/cli.ts | 3 +- src/config.ts | 39 +- src/server.ts | 253 ++++---- src/session/default-profile-lease.ts | 254 -------- src/session/descriptor.ts | 2 - src/session/plugin-link.ts | 24 +- src/session/registry.ts | 36 +- src/telemetry/workspace-store.ts | 20 +- src/tools/core.ts | 130 +--- src/tools/editor.ts | 2 +- src/tools/provisioning.ts | 59 +- src/tools/registry.ts | 104 +-- src/tools/session.ts | 269 ++++++++ src/tools/workspace.ts | 637 ------------------- src/toolsets.ts | 25 +- src/usage/activity-guard.ts | 259 ++++++++ src/util/concurrency.ts | 3 +- src/util/errors.ts | 4 + src/workspace/lease.ts | 193 ------ src/workspace/store.ts | 148 ----- tests/fixtures/settings-plugin/main.js | 32 + tests/fixtures/settings-plugin/manifest.json | 9 + tests/unit/activity-guard.test.ts | 127 ++++ tests/unit/agent-workspace-store.test.ts | 186 +----- tests/unit/audit.test.ts | 4 - tests/unit/config.test.ts | 15 +- tests/unit/default-profile-lease.test.ts | 148 ----- tests/unit/plugin-dev-truth.test.ts | 2 +- tests/unit/plugin-link.test.ts | 9 +- tests/unit/session-launch-failure.test.ts | 4 +- tests/unit/telemetry-store.test.ts | 42 +- tests/unit/tool-catalog-schema.test.ts | 63 +- tests/unit/tool-registry-runtime.test.ts | 212 +++--- tests/unit/workspace-lease.test.ts | 105 --- tests/unit/workspace-lifecycle.test.ts | 4 +- 66 files changed, 1545 insertions(+), 3416 deletions(-) delete mode 100644 src/agent/store.ts delete mode 100644 src/session/default-profile-lease.ts create mode 100644 src/tools/session.ts delete mode 100644 src/tools/workspace.ts create mode 100644 src/usage/activity-guard.ts delete mode 100644 src/workspace/lease.ts delete mode 100644 src/workspace/store.ts create mode 100644 tests/fixtures/settings-plugin/main.js create mode 100644 tests/fixtures/settings-plugin/manifest.json create mode 100644 tests/unit/activity-guard.test.ts delete mode 100644 tests/unit/default-profile-lease.test.ts delete mode 100644 tests/unit/workspace-lease.test.ts diff --git a/.agents/plugins/marketplace.json b/.agents/plugins/marketplace.json index 5ef429e..d4f4287 100644 --- a/.agents/plugins/marketplace.json +++ b/.agents/plugins/marketplace.json @@ -1,13 +1,13 @@ { "name": "knapper", - "description": "Drive a live Obsidian desktop app over MCP: develop and test Obsidian plugins, automate the vault and UI, and read console output. Use for anything involving Obsidian, its vaults, notes, or plugins.", + "description": "MCP server for stateless Obsidian plugin development with one managed session, browser automation, and debugging.", "interface": { "displayName": "Knapper" }, "plugins": [ { "name": "knapper", - "description": "Drive a live Obsidian desktop app over MCP: develop and test Obsidian plugins, automate the vault and UI, and read console output. Use for anything involving Obsidian, its vaults, notes, or plugins.", + "description": "MCP server for stateless Obsidian plugin development with one managed session, browser automation, and debugging.", "source": { "source": "local", "path": "./" diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 8da7f2b..fcfad63 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -1,7 +1,7 @@ { "$schema": "https://code.claude.com/schemas/marketplace.json", "name": "knapper", - "description": "MCP server for Obsidian plugin development: integrated browser, debugging, and session-based vaults for agents.", + "description": "MCP server for stateless Obsidian plugin development with one managed session, browser automation, and debugging.", "owner": { "name": "bearfire-dev", "url": "https://github.com/bearfire-dev" @@ -10,7 +10,7 @@ { "name": "knapper", "source": "./", - "description": "MCP server for Obsidian plugin development: integrated browser, debugging, and session-based vaults for agents." + "description": "MCP server for stateless Obsidian plugin development with one managed session, browser automation, and debugging." } ] } diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index 3c7d08a..1ef939a 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "knapper", - "version": "0.6.0-beta.9", - "description": "MCP server for Obsidian plugin development: integrated browser, debugging, and session-based vaults for agents.", + "version": "0.7.0-beta.1", + "description": "MCP server for stateless Obsidian plugin development with one managed session, browser automation, and debugging.", "author": { "name": "slate-rehm", "url": "https://github.com/slate-rehm" @@ -9,5 +9,18 @@ "homepage": "https://github.com/bearfire-dev/knapper#readme", "repository": "https://github.com/bearfire-dev/knapper", "license": "MIT", - "keywords": ["obsidian", "mcp", "plugin-development", "playwright", "cdp", "automation"] + "keywords": ["obsidian", "mcp", "plugin-development", "playwright", "cdp", "automation"], + "interface": { + "displayName": "Knapper", + "shortDescription": "Develop and test Obsidian plugins in one managed session.", + "longDescription": "Drive one live Obsidian desktop session through MCP. Build and reload plugins, automate settings and other UI, and inspect console errors.", + "developerName": "Bearfire", + "category": "Developer Tools", + "capabilities": ["Obsidian automation", "Plugin development", "UI testing"], + "defaultPrompt": [ + "Open an isolated Obsidian session and test this plugin.", + "Reload this Obsidian plugin and check for new errors.", + "Exercise this plugin's settings through the live UI." + ] + } } diff --git a/.codex-plugin/plugin.json b/.codex-plugin/plugin.json index 3c7d08a..1ef939a 100644 --- a/.codex-plugin/plugin.json +++ b/.codex-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "knapper", - "version": "0.6.0-beta.9", - "description": "MCP server for Obsidian plugin development: integrated browser, debugging, and session-based vaults for agents.", + "version": "0.7.0-beta.1", + "description": "MCP server for stateless Obsidian plugin development with one managed session, browser automation, and debugging.", "author": { "name": "slate-rehm", "url": "https://github.com/slate-rehm" @@ -9,5 +9,18 @@ "homepage": "https://github.com/bearfire-dev/knapper#readme", "repository": "https://github.com/bearfire-dev/knapper", "license": "MIT", - "keywords": ["obsidian", "mcp", "plugin-development", "playwright", "cdp", "automation"] + "keywords": ["obsidian", "mcp", "plugin-development", "playwright", "cdp", "automation"], + "interface": { + "displayName": "Knapper", + "shortDescription": "Develop and test Obsidian plugins in one managed session.", + "longDescription": "Drive one live Obsidian desktop session through MCP. Build and reload plugins, automate settings and other UI, and inspect console errors.", + "developerName": "Bearfire", + "category": "Developer Tools", + "capabilities": ["Obsidian automation", "Plugin development", "UI testing"], + "defaultPrompt": [ + "Open an isolated Obsidian session and test this plugin.", + "Reload this Obsidian plugin and check for new errors.", + "Exercise this plugin's settings through the live UI." + ] + } } diff --git a/.cursor-plugin/plugin.json b/.cursor-plugin/plugin.json index 3c7d08a..1ef939a 100644 --- a/.cursor-plugin/plugin.json +++ b/.cursor-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "knapper", - "version": "0.6.0-beta.9", - "description": "MCP server for Obsidian plugin development: integrated browser, debugging, and session-based vaults for agents.", + "version": "0.7.0-beta.1", + "description": "MCP server for stateless Obsidian plugin development with one managed session, browser automation, and debugging.", "author": { "name": "slate-rehm", "url": "https://github.com/slate-rehm" @@ -9,5 +9,18 @@ "homepage": "https://github.com/bearfire-dev/knapper#readme", "repository": "https://github.com/bearfire-dev/knapper", "license": "MIT", - "keywords": ["obsidian", "mcp", "plugin-development", "playwright", "cdp", "automation"] + "keywords": ["obsidian", "mcp", "plugin-development", "playwright", "cdp", "automation"], + "interface": { + "displayName": "Knapper", + "shortDescription": "Develop and test Obsidian plugins in one managed session.", + "longDescription": "Drive one live Obsidian desktop session through MCP. Build and reload plugins, automate settings and other UI, and inspect console errors.", + "developerName": "Bearfire", + "category": "Developer Tools", + "capabilities": ["Obsidian automation", "Plugin development", "UI testing"], + "defaultPrompt": [ + "Open an isolated Obsidian session and test this plugin.", + "Reload this Obsidian plugin and check for new errors.", + "Exercise this plugin's settings through the live UI." + ] + } } diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7232b06..7fae27b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -7,7 +7,7 @@ name: Release # This project is not published to any registry. The release artifact is the npm # tarball attached to the GitHub Release, which users install by URL: # -# npm i -g https://github.com/bearfire-dev/knapper/releases/download/v0.6.0-beta.7/knapper-0.6.0-beta.7.tgz +# npm i -g https://github.com/bearfire-dev/knapper/releases/download/v0.7.0-beta.1/knapper-0.7.0-beta.1.tgz # # The version bump itself is an ordinary change: bump it in a PR into dev, then # promote dev -> master. This workflow only tags and releases what is already on diff --git a/AGENTS.md b/AGENTS.md index 2217b1f..f5dcc71 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,7 +32,7 @@ src/ telemetry/ console/error/network ring buffer, plugin attribution devcycle/ composites (dev_cycle, exercise, reset_state) session/ internal isolated-instance descriptors, launch, and cleanup - agent/, workspace/ durable public handles and leases + usage/ cross-process single-user activity guard scripts/ acceptance, e2e, ci-smoke, version sync skills/ plugin skills (obsidian-debugging, -instance-setup, @@ -84,9 +84,9 @@ npm run bg-input # 6 live checks: background input without desktop focus theft npm run workspaces # isolated instances, reconnect, scoped restart, quarantine ``` -Each live suite provisions its own workspace and tears it down. Run `npm run -workspaces` for anything that changes `src/session/`, `launch.ts`, workspace -leases, or process-scoping predicates. +Each live suite provisions one private session and tears it down. Run `npm run +workspaces` for changes to `src/session/`, `launch.ts`, the activity guard, or +process-scoping predicates. `npm run check && npm run typecheck && npm test && npm run acceptance` is the minimum before proposing a change. Run `npm run e2e` for anything touching the @@ -162,10 +162,10 @@ must stay distinguishable; collapsing them into "cannot connect" is a regression `classifyCliOutput` / `classifyEvalOutput` in `connection/cli/exec.ts` own that translation. Any new CLI surface goes through them. -**Every tool needs annotations.** `readOnlyHint` gates concurrency: the registry -takes an exclusive lock for anything not marked read-only, so a mislabeled mutating -tool will interleave real input against shared UI. Add `destructiveHint` for -anything that deletes or overwrites. Every schema field needs `.describe()`. +**Every tool needs annotations.** The registry serializes all calls against one +active target. `readOnlyHint` still tells MCP clients what a tool changes. Add +`destructiveHint` for anything that deletes or overwrites. Every schema field needs +`.describe()`. **Adding a tool** means: register it in the right `src/tools/*.ts` module with a toolset and capability, describe every field, add it to the E2E suite, and check @@ -197,10 +197,10 @@ second copy. Bump both together. - Delegate independent workstreams to parallel subagents with **strict file ownership**, since they share one working tree. Overlapping edits corrupt each other. Follow implementation with an audit subagent that runs `npm run check`. -- Open an agent handle, then create an isolated workspace for live work. Pass its - `workspaceHandle` to every operational tool. -- One MCP server owns the default profile at a time. On `DEFAULT_PROFILE_BUSY`, - create an isolated workspace and retry with its handle. +- Open one isolated session with `obsidian_session_open`. Operational tools use the + active target and do not accept caller-owned handles. +- On `KNAPPER_BUSY`, inspect `obsidian_status` and retry after the reported activity + window or after the current owner releases the session. - `npm run bg-input` stays serialized regardless: it depends on Obsidian not being the foreground window, which is one global property of the desktop, and it fails open. - Use mermaid flowcharts to explain architecture in plans. diff --git a/README.md b/README.md index 4475f5d..c3a2e7a 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # knapper -MCP server that drives a **live Obsidian desktop app** for plugin development. It provides native CLI commands, fenced browser automation, durable telemetry, dev-cycle tools, and isolated workspaces for concurrent agents. +MCP server that drives a **live Obsidian desktop app** for plugin development. It provides native CLI commands, fenced browser automation, telemetry, and plugin development tools through one guarded session. _Knapping is the craft of shaping obsidian into tools._ @@ -15,25 +15,23 @@ throughout — but they are written from documentation rather than measured agai running app, and no live suite has ever executed on either. Treat them as untested. Bug reports and fixes from other platforms are welcome. -| Platform | Status | Notes | -| ----------- | ------------------------------- | ----------------------------------------------------------------------------------------------------------------- | -| **Linux** | Tested | Every live suite runs here. See [docs/verified-environment.md](docs/verified-environment.md) for the exact build. | -| **macOS** | Untested | Most tools should work. Isolated workspaces are not verified. | -| **Windows** | Untested, isolation unsupported | `obsidian_workspace_create` refuses rather than create an unsafe workspace. | +| Platform | Status | Notes | +| ----------- | -------- | ----------------------------------------------------------------------------------------------------------------- | +| **Linux** | Tested | Every live suite runs here. See [docs/verified-environment.md](docs/verified-environment.md) for the exact build. | +| **macOS** | Untested | Most tools should work. Private sessions are not verified. | +| **Windows** | Untested | Live use is not verified. | -The one hard limit is **isolated workspaces**, and it comes from Obsidian itself. -Each isolated instance needs its own CLI socket. Obsidian derives that -path per platform: +Knapper uses one managed Obsidian session. Private profile isolation still matters +when Knapper starts a test instance. Each private instance needs its own CLI socket. -| Platform | Socket keyed on | Isolation | -| -------- | ------------------------------------ | ---------------------------------------------- | -| Linux | `$XDG_RUNTIME_DIR` | **per-workspace** — the environment selects it | -| macOS | `os.homedir()`, environment excluded | shared, only via a `HOME` override, unproven | -| Windows | `\\.\pipe\obsidian-cli-` | **impossible** — no environment input at all | +| Platform | Socket keyed on | Isolation | +| -------- | ------------------------------------ | -------------------------------------------- | +| Linux | `$XDG_RUNTIME_DIR` | **per-session** — the environment selects it | +| macOS | `os.homedir()`, environment excluded | shared, only via a `HOME` override, unproven | +| Windows | `\\.\pipe\obsidian-cli-` | **impossible** — no environment input at all | -So `obsidian_workspace_create` throws a typed refusal on Windows. It does not return -a workspace whose CLI commands can reach someone else's app. Everything else — -both transports, all other toolsets — is platform-independent in principle. +Knapper refuses any launch that cannot isolate the private profile and CLI socket. +It never routes a call to an unverified Obsidian process. CI runs `ubuntu-latest` only, and covers lint, types, unit tests, and a packaged install. The live suites need a real desktop Obsidian and run on a maintainer's @@ -91,7 +89,7 @@ agent how to drive them. Install the server alone, or both. Install a specific release tarball, then point your client at the `knapper` binary: ```bash -npm i -g https://github.com/bearfire-dev/knapper/releases/download/v0.6.0-beta.7/knapper-0.6.0-beta.7.tgz +npm i -g https://github.com/bearfire-dev/knapper/releases/download/v0.7.0-beta.1/knapper-0.7.0-beta.1.tgz ``` ```json @@ -201,16 +199,12 @@ Then point your client at `node /absolute/path/to/dist/cli.js`. ## First run -Start with explicit ownership and target selection: +Start one session before you use Obsidian: -1. Run **`obsidian_toolsets_update`** to enable the operational toolsets for the task. -2. Run **`obsidian_agent_open`** and keep its `agentHandle`. -3. Run **`obsidian_workspace_create`** for safe plugin work. Use - **`obsidian_workspace_claim_default`** only for a user-approved existing vault. -4. Pass the returned `workspaceHandle` to **`obsidian_doctor`** and every other - operational tool. -5. Apply the fixes that doctor names. These are usually **`obsidian_setup_cli`** and - **`obsidian_launch`** for the default profile. Isolated workspaces start ready. +1. Call **`obsidian_session_open`** with the plugin source and ID when you need a private test session. +2. Call **`obsidian_status`** to confirm that the session is `self` and that the target is ready. +3. Apply the fixes that doctor names. These are usually **`obsidian_setup_cli`** and + **`obsidian_launch`** for the default profile. Private sessions start ready. Then the development loop: **`obsidian_link_plugin`** to symlink your build output into a vault, build, and **`obsidian_dev_cycle`** to reload the plugin and report @@ -243,53 +237,30 @@ against it. For experiments, make throwaway space instead — see below. `obsidian_doctor` and `obsidian_status` show which vaults are authorized and how, so an agent can diagnose a refusal without guessing. -`obsidian_create_vault` refuses when an isolated workspace is selected. Use the -scratch vault that `obsidian_workspace_create` created for that workspace. +`obsidian_create_vault` refuses when a private session is selected. Use the scratch +vault that `obsidian_session_open` creates for that session. -## Agent and workspace handles +## Stateless session use -Open an agent handle before you use Obsidian. Then create an isolated workspace or -claim the default profile. +The MCP tool list is fixed during initialization. It includes the UI and plugin +tools that an agent needs. Knapper does not send `notifications/tools/list_changed`. -```text -obsidian_agent_open label=my-feature - → agentHandle - -obsidian_workspace_create agentHandle= pluginSourceDir=/abs/plugin pluginId=my-plugin - → workspaceHandle -``` - -Pass `workspaceHandle` to every operational tool. The handle selects one exact -Obsidian instance across stdio reconnects and stateless HTTP requests. It is a -coordination identifier, not an authentication credential. - -One live Knapper process holds an exclusive lease for each workspace. A second -process receives `WORKSPACE_BUSY`. Knapper renews the lease after successful calls. -An idle lease expires after `KNAP_IDLE_TIMEOUT_MS` when no calls remain. -Knapper immediately reclaims a lease after it proves process death or PID reuse. +Call `obsidian_session_open` to create or reuse one managed session. Operational +tools use that session automatically. They do not accept caller-owned session +identifiers. -An isolated workspace always creates its own scratch vault. It does not accept an -existing vault path. It also does not write ownership files into vaults. Use -`obsidian_workspace_restart` to restart only that workspace. +Call `obsidian_session_status` to inspect the session. Call `obsidian_session_release` +to stop it and keep its scratch vault. Call `obsidian_session_reset` to stop the +session and create a new private target. Knapper moves verified private roots to +recoverable trash when cleanup requires removal. It never hard-deletes them. -Knapper verifies the private Obsidian session before it routes tools to the -workspace. The result contains `visualIdentity.state` and -`visualIdentity.warnings`. Knapper does not report the workspace as ready unless -the test banner, title, icon, and desktop class are present. +Only one operation runs at a time. A second Knapper process receives `KNAPPER_BUSY`. +`obsidian_status` reports `free`, `self`, `busy`, or `stale`, with the last activity +time and a retry interval. Knapper reclaims a stale owner after it verifies process +death or an expired activity record. -Call `obsidian_workspace_stop` before `obsidian_workspace_destroy`. The destroy -tool refuses an active workspace. It moves the verified workspace root into -recoverable trash under `KNAP_HOME`. Knapper does not hard-delete the root. -Knapper cannot destroy a default-profile workspace. - -Use `obsidian_workspace_release` when you want to keep the stopped scratch vault. - -Use `obsidian_workspace_claim_default` only when the user wants their own Obsidian -profile. Existing vault access still requires terminal authorization. Registry -membership never grants access or deletion rights. - -Workspace and agent leases last 24 hours after the last activity. Internal instance -cleanup uses the same default. Isolated workspaces are Linux-only in practice. +Use the default profile only after the user approves it and the vault is authorized. +Knapper keeps the profile and `XDG_RUNTIME_DIR` private for managed sessions. ## Configuration @@ -300,7 +271,7 @@ Set options via **environment variables** (and a subset via CLI flags). See [doc | CDP URL | `OBSIDIAN_CDP_URL` | `--cdp-url` | `http://127.0.0.1:9222` | | Obsidian binary | `OBSIDIAN_BIN` | `--obsidian-bin` | OS default | | Default vault | `OBSIDIAN_VAULT` | `--vault`, `-v` | (active / unset) | -| Toolsets | `KNAP_TOOLSETS` | `--toolsets` | empty (control tools only) | +| Toolsets | `KNAP_TOOLSETS` | `--toolsets` | `all` | | knapper's disk root | `KNAP_HOME` | — | `~/.knapper_mcp` | | Log level | `KNAP_LOG_LEVEL` | `--log-level` | `info` | | Telemetry buffer | `KNAP_TELEMETRY_BUFFER` | — | `2000` | @@ -308,25 +279,26 @@ Set options via **environment variables** (and a subset via CLI flags). See [doc | CDP reconnect delay | `KNAP_RECONNECT_MS` | — | `2000` | | Screenshot dir | `KNAP_SCREENSHOT_DIR` | `--output-dir` | `./.knapper` | | CLI timeout | `KNAP_CLI_TIMEOUT_MS` | — | `15000` | -| Idle ownership | `KNAP_IDLE_TIMEOUT_MS` | — | `86400000` (24 hours) | +| Session cleanup | `KNAP_IDLE_TIMEOUT_MS` | — | `86400000` (24 hours) | +| Activity ownership | `KNAP_ACTIVITY_IDLE_MS` | — | `300000` (5 minutes) | | Command transport | `KNAP_COMMAND_TRANSPORT` | — | `auto` (`cli` or `playwright`) | | Window match | `OBSIDIAN_TARGET_MATCH` | `--target-match` | (unset) | | Transport | `MCP_TRANSPORT` | `--transport` | `stdio` | | HTTP port | `MCP_PORT` | `--port` | `9223` | | HTTP host | `MCP_HOST` | `--host` | `127.0.0.1` | -| Max concurrency | `KNAP_MAX_CONCURRENCY` | — | `4` | `LOG_LEVEL`, `RECONNECT_MS`, and `SCREENSHOT_DIR` are also accepted as aliases; the `KNAP_`-prefixed name wins when both are set. -When a tool selects an isolated workspace, screenshots use that workspace's own +When a session selects a private target, screenshots use that target's own `output/` directory. A requested screenshot path must be relative to the configured output root. Screenshot tools return a file path and never return inline base64 data. Tools publish MCP output schemas and return machine-readable `structuredContent`. Clients do not need to parse the display text. -The default `stdio` transport is what MCP clients use. `--transport http` serves -MCP at `/mcp` (for example `http://127.0.0.1:9223/mcp`). Each request is stateless. +The default `stdio` transport is what MCP clients use. HTTP is experimental and +uses one global lane. `--transport http` serves MCP at `/mcp` (for example +`http://127.0.0.1:9223/mcp`). Each request uses the active session. The listener can bind only to `127.0.0.1` or `::1`. It cannot bind to a wildcard, LAN address, or the `localhost` name. Requests can use `localhost`, `127.0.0.1`, or `[::1]` in their `Host` and `Origin` headers. The server has no authentication. @@ -341,7 +313,7 @@ Gating keeps tool count manageable for model tool selection. | Toolset | Startup | Description | | ------------ | ------- | -------------------------------------------------------------------------------------------------------------------- | | `core` | no | Status, doctor, launch, eval, CLI, commands, attach | -| `workspace` | no | Explicit agent and workspace handles, with isolated scratch instances on Linux | +| `session` | yes | One managed session and its lifecycle operations | | `ui` | no | Fenced `browser_*` tools for real UI interaction, plus `obsidian_snapshot` | | `telemetry` | no | Console/error/network capture, cursor tailing | | `plugin-dev` | no | Reload, manifest/settings, `obsidian_dev_cycle`, exercise/reset | @@ -350,22 +322,16 @@ Gating keeps tool count manageable for model tool selection. | `devtools` | no | DOM/CSS/CDP passthrough, OS-window screenshots, mobile emulation | | `authoring` | no | Themes, snippets, properties, tags, tasks, daily notes, templates | -Set the startup surface with `KNAP_TOOLSETS` or `--toolsets`. An empty value starts -only the 17 control tools. This small surface reduces tool-selection context. - -Use `obsidian_toolsets` to inspect the enabled set. Use `obsidian_tool_catalog` to -search all tool definitions. Use `obsidian_toolsets_update` to change the surface. -Pass toolset names in `enable` or `disable`. Set `dryRun` to preview the change. -Knapper sends `notifications/tools/list_changed` after an effective change. - -The control tools always remain visible. They cover agent and workspace lifecycle, -status, diagnosis, capabilities, toolset state, the catalog, and toolset updates. +Knapper publishes the complete startup surface during MCP initialization. The tool +list does not change during a connection. Do not change the tool list after startup. +The fixed surface includes session lifecycle, status, plugin development, telemetry, +editor, UI, and opt-in vault tools. ### Representative tools **Core & provisioning:** `obsidian_status`, `obsidian_doctor`, `obsidian_launch`, `obsidian_setup_cli`, `obsidian_setup_vault`, `obsidian_link_plugin`, `obsidian_list_targets`, `obsidian_attach`, `obsidian_eval`, `obsidian_cli`, `obsidian_commands`, `obsidian_command` -**Workspaces:** `obsidian_agent_open`, `obsidian_agent_status`, `obsidian_agent_close`, `obsidian_workspace_create`, `obsidian_workspace_claim_default`, `obsidian_workspace_list`, `obsidian_workspace_status`, `obsidian_workspace_stop`, `obsidian_workspace_restart`, `obsidian_workspace_release`, `obsidian_workspace_destroy` +**Session:** `obsidian_session_open`, `obsidian_session_status`, `obsidian_session_release`, `obsidian_session_reset` **Plugin dev:** `obsidian_plugin_list`, `obsidian_plugin_manifest`, `obsidian_plugin_settings`, `obsidian_plugin_reload`, `obsidian_dev_cycle`, `obsidian_exercise_command`, `obsidian_reset_state`, `obsidian_plugin_health` @@ -391,19 +357,17 @@ Browser tools are **snapshot-first**: call `browser_snapshot` (or the cheaper sc | **CLI disabled** | `CLI_DISABLED`, or stdout marker `Command line interface is not enabled.` | `obsidian_setup_cli` or Settings → Advanced → Command line interface | | **CDP port closed** | `CDP_PORT_CLOSED`, attach timeouts | Quit Obsidian completely; cold start with `--remote-debugging-port` (`obsidian_launch`) | | **Argv corruption** | `ARGV_CORRUPTION`, `Command "-foo" not found` | Fix `user-flags.conf` to use `--double-dash` flags | -| **Default profile busy** | `DEFAULT_PROFILE_BUSY` | Create an isolated workspace, then retry with its handle | -| **Workspace busy** | `WORKSPACE_BUSY` | Wait for the owner to release it, or create another workspace | +| **Session busy** | `KNAPPER_BUSY` | Wait for the retry interval, then call `obsidian_status` | Launch failures use `OBSIDIAN_LAUNCH_FAILED`. The error includes the exit signal, exit code, and bounded launch output when available. Also: -- **Several MCP hosts are active** — one server owns the default profile at a time. Create an isolated workspace for concurrent work. -- **Expired workspaces** — agent and workspace handles expire after 24 idle hours. Create a new handle if one expires. Internal cleanup can stop abandoned isolated instances and quarantine only verified scratch roots. +- **Several MCP hosts are active** — Knapper permits one operation at a time. Use `obsidian_status` to see the owner state and retry interval. - **Every CLI call fails with `Cannot find module 'electron'`** — something set `ELECTRON_RUN_AS_NODE=1` in the environment knapper inherited, which makes the Obsidian binary start as a bare Node process. Electron-based MCP clients (Claude Code, Cursor, VS Code, Claude Desktop) set it for their child processes. knapper strips it before spawning, so if you still see this, a wrapper script or shell profile is re-adding it downstream. - **Unavailable `browser_*` calls** — enabled browser tools stay visible when Obsidian is offline. The call returns `CDP_PORT_CLOSED` with `obsidian_launch` remediation. Cold-start Obsidian with the debug port, then retry the same tool. - **`VAULT_NOT_FOUND`** — vault name not in the `obsidian.json` registry. -- **`SESSION_NOT_FOUND`** — the selected workspace expired or its internal instance no longer exists. Create a new isolated workspace. Knapper never falls back to the default profile. +- **`SESSION_NOT_FOUND`** — the managed session no longer exists. Call `obsidian_session_open` to create it again. Knapper never falls back to the default profile. - **Stale UI refs** — `STALE_REF`; take a new `browser_snapshot`. - **Linux wrappers** — single-dash tokens in `user-flags.conf` break every CLI call. @@ -435,12 +399,12 @@ npm run acceptance # fast gate over the critical seams npm run e2e # deep end-to-end: vault round-trips, UI, telemetry, dev cycle npm run fence # refusals against a genuinely unauthorized vault npm run bg-input # input delivery while Obsidian is not the focused window -npm run workspaces # isolated instances, reconnect, scoped restart, quarantine +npm run workspaces # session ownership, reconnect, restart, quarantine ``` The suites do not need a pre-launched Obsidian instance or an existing scratch vault. They verify registry preservation and safe cleanup. Run `npm run workspaces` -for changes to `src/session/`, workspace leases, or process-scoping predicates. +for changes to `src/session/`, session ownership, or process-scoping predicates. `npm run bg-input` is only meaningful when Obsidian is not the foreground window. Run it without clicking the private Obsidian window. @@ -477,15 +441,15 @@ npm run versions:check # CI gate: fail on drift same flow as anything else: ```bash -git checkout -b release/v0.6.0-beta.7 dev -npm version 0.6.0-beta.7 --no-git-tag-version && npm run versions:sync +git checkout -b release/v0.7.0-beta.1 dev +npm version 0.7.0-beta.1 --no-git-tag-version && npm run versions:sync # PR into dev, then promote dev -> master ``` Once the promotion PR merges, cut the release either way: - **From the Actions tab** — run the _Release_ workflow. It tags master's current HEAD with the version already in `package.json`, packs the tarball, and creates the GitHub Release with that tarball attached. Tick _dry run_ to rehearse. It refuses if that version is already tagged. -- **From a tag** — `git tag -s v0.6.0-beta.7 && git push origin v0.6.0-beta.7`. +- **From a tag** — `git tag -s v0.7.0-beta.1 && git push origin v0.7.0-beta.1`. Either way the workflow refuses to release a commit that is not on `master`, or a tag that disagrees with `package.json`. It never pushes commits to `master`, which diff --git a/SECURITY.md b/SECURITY.md index 1407d77..cd2c817 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -124,7 +124,7 @@ Moves files to the system trash. `permanent: true` must be set explicitly to byp ### `obsidian_remove_vault` — `destructiveHint` Unregisters an authorized Knapper-created vault. It never deletes the directory. It rejects -user-adopted vaults. Isolated workspace cleanup uses `obsidian_workspace_destroy`, which first +user-adopted vaults. Isolated session cleanup uses `obsidian_session_reset`, which first stops the private instance, verifies the exact Knapper-owned root, and moves that root to recoverable trash under `KNAP_HOME`. @@ -145,7 +145,7 @@ knapper is not a read-only bridge. It writes, outside the vault as well as insid | ------------------------------------------ | ---------------------------------------------------------------------- | ---------------------------------------------------------------------- | | `/obsidian.json` | `obsidian_setup_cli`, `obsidian_create_vault`, `obsidian_remove_vault` | Flips the global `cli` flag; registers and unregisters vaults | | `/vault-authorizations.json` | `obsidian_create_vault`, `knapper authorize`, `knapper revoke` | External path and filesystem-identity grants, mode `0600` | -| `/trash/` | `obsidian_workspace_destroy` | Recoverable quarantine for verified private workspace roots | +| `/trash/` | `obsidian_session_reset` | Recoverable quarantine for verified private session roots | | `/.obsidian/plugins/` | `obsidian_link_plugin` | Creates or replaces a **symlink**. Refuses to clobber a real directory | | `/.obsidian/plugins//data.json` | `obsidian_reset_state` | Overwrites plugin settings with `{}`; returns the previous contents | | `./.knapper/` | screenshot and snapshot tools | Output artifacts, under `KNAP_SCREENSHOT_DIR` | diff --git a/agents/obsidian-tester.md b/agents/obsidian-tester.md index a5a3edd..12fb452 100644 --- a/agents/obsidian-tester.md +++ b/agents/obsidian-tester.md @@ -7,9 +7,10 @@ You are an Obsidian plugin QA subagent. You drive a **live** Obsidian desktop in ## Setup -1. Call `obsidian_doctor`. If problems exist, stop and report remediation — do not guess. -2. Confirm CDP is attached (`obsidian_status`). UI steps require CDP. -3. Note the target vault and plugin id you were given (or discover via `obsidian_plugin_list`). +1. Call `obsidian_session_open` with the plugin source directory and ID. +2. Call `obsidian_doctor`. If problems exist, stop and report the remediation. +3. Confirm that CDP is attached and the owner state is `self` (`obsidian_status`). UI steps require CDP. +4. Note the target vault and plugin ID you were given, or discover it with `obsidian_plugin_list`. ## Testing strategy diff --git a/commands/obsidian-dev.md b/commands/obsidian-dev.md index 2231e4e..d80bd12 100644 --- a/commands/obsidian-dev.md +++ b/commands/obsidian-dev.md @@ -4,8 +4,11 @@ Run a full **build → link → reload → verify** loop against the live Obsidi ## Prerequisites -1. Call `obsidian_doctor` and apply every remediation until CLI and CDP are healthy. -2. Call `obsidian_launch` if Obsidian is not running with `--remote-debugging-port`. +1. Call `obsidian_session_open` with the plugin source directory and ID. +2. Call `obsidian_doctor` and apply each remediation until CLI and CDP are healthy. +3. Call `obsidian_status` and confirm that the private target is ready. + +Use `obsidian_launch` only for an approved default-profile flow. ## Steps diff --git a/commands/obsidian-doctor.md b/commands/obsidian-doctor.md index 2508aa5..9100b04 100644 --- a/commands/obsidian-doctor.md +++ b/commands/obsidian-doctor.md @@ -11,9 +11,9 @@ Diagnose why knapper cannot talk to Obsidian and fix each layer explicitly. - `CDP_PORT_CLOSED` → quit Obsidian completely, then `obsidian_launch` (single-instance lock) - `ARGV_CORRUPTION` → edit `user-flags.conf` to use `--` prefixes - `VAULT_NOT_FOUND` → fix `OBSIDIAN_VAULT` or register the vault in Obsidian -3. Call **`obsidian_status`** to confirm transports and toolsets. +3. Call **`obsidian_status`** to confirm transports and the session owner state. 4. If CDP is still missing, verify nothing else holds port `9222` and that `OBSIDIAN_CDP_URL` matches your launch flags. ## Reference -Use skill **obsidian-instance-setup** for transport tradeoffs and multi-window attach (`obsidian_list_targets`, `obsidian_attach`). +Use skill **obsidian-instance-setup** for session lifecycle and multi-window attach (`obsidian_list_targets`, `obsidian_attach`). diff --git a/docs/configuration.md b/docs/configuration.md index 569c0f5..077847e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1,72 +1,50 @@ # Configuration Knapper reads CLI flags first, then environment variables, then defaults. Tool -calls select an Obsidian target with a required `workspaceHandle`. Transport state -does not select a workspace. +calls use one active Obsidian session. The session state does not depend on agent +handles or transport reconnect state. ## Connection and process settings -| Environment variable | CLI flag | Default | Purpose | -| ------------------------ | ---------------- | ----------------------- | -------------------------------------------- | -| `OBSIDIAN_CDP_URL` | `--cdp-url` | `http://127.0.0.1:9222` | Default-profile CDP endpoint | -| `OBSIDIAN_BIN` | `--obsidian-bin` | OS default | Obsidian executable | -| `OBSIDIAN_VAULT` | `--vault`, `-v` | unset | Default authorized vault name | -| `OBSIDIAN_TARGET_MATCH` | `--target-match` | unset | Additional default-window match | -| `KNAP_HOME` | none | `~/.knapper_mcp` | Durable handles, telemetry, audit, and trash | -| `KNAP_IDLE_TIMEOUT_MS` | none | `86400000` | Workspace lease and cleanup timeout | -| `KNAP_COMMAND_TRANSPORT` | none | `auto` | `auto`, `cli`, or `playwright` | -| `KNAP_CLI_TIMEOUT_MS` | none | `15000` | Obsidian CLI timeout in milliseconds | - -Do not set an internal profile, runtime directory, or instance descriptor. Use -`obsidian_workspace_create` and pass its workspace handle on each tool call. - -Agent and workspace handles use 192 bits of random data. Both records have a -24-hour idle lease. Each successful operational call renews both leases. Handles -provide attribution and routing. They do not provide authentication. - -One live Knapper process holds an exclusive lease for each workspace. Another -process receives `WORKSPACE_BUSY` when it uses that workspace. The lease expires -after `KNAP_IDLE_TIMEOUT_MS` when the process has no active calls. Knapper reclaims -the lease immediately after it proves process death or PID reuse. - -An isolated workspace always creates an exact scratch layout under `KNAP_HOME`. +| Environment variable | CLI flag | Default | Purpose | +| ------------------------ | ---------------- | ----------------------- | ------------------------------------------ | +| `OBSIDIAN_CDP_URL` | `--cdp-url` | `http://127.0.0.1:9222` | Default-profile CDP endpoint | +| `OBSIDIAN_BIN` | `--obsidian-bin` | OS default | Obsidian executable | +| `OBSIDIAN_VAULT` | `--vault`, `-v` | unset | Default authorized vault name | +| `OBSIDIAN_TARGET_MATCH` | `--target-match` | unset | Additional default-window match | +| `KNAP_HOME` | none | `~/.knapper_mcp` | Session state, telemetry, audit, and trash | +| `KNAP_IDLE_TIMEOUT_MS` | none | `86400000` | Inactive session cleanup time | +| `KNAP_ACTIVITY_IDLE_MS` | none | `300000` | Single-agent activity ownership time | +| `KNAP_COMMAND_TRANSPORT` | none | `auto` | `auto`, `cli`, or `playwright` | +| `KNAP_CLI_TIMEOUT_MS` | none | `15000` | Obsidian CLI timeout in milliseconds | + +Knapper creates a private profile and runtime directory for a managed session. +One live process owns the activity record. Another process receives +`KNAPPER_BUSY`. The record includes the process ID, session state, current +operation, last activity, and retry time. Knapper reclaims stale state only after +it verifies process death or an expired activity record. + +An isolated session always creates an exact scratch layout under `KNAP_HOME`. It cannot adopt a caller path. Knapper verifies the private-session identity before it routes tools. The result returns `visualIdentity.state` and -`visualIdentity.warnings`. A workspace does not become ready when the required +`visualIdentity.warnings`. A session does not become ready when the required banner, title, icon, or desktop class is missing. -Call `obsidian_workspace_stop` before `obsidian_workspace_destroy`. The destroy tool -refuses an active workspace. It checks path, symlink, device, and inode ownership. -It then moves the root into `KNAP_HOME/trash`. It does not hard-delete the root. - -`obsidian_workspace_release` removes the stopped handle and retains the scratch -vault. A default-profile workspace can only be released. +Call `obsidian_session_release` to stop the session and retain its scratch vault. +Call `obsidian_session_reset` to replace it. Cleanup checks path, symlink, device, +and inode ownership. It moves the root into `KNAP_HOME/trash`. It never +hard-deletes the root. ## Tool surface -| Environment variable | CLI flag | Default | Purpose | -| ---------------------- | -------------- | ------------ | ---------------------------------- | -| `KNAP_TOOLSETS` | `--toolsets` | empty | Comma-separated toolsets or `all` | -| `KNAP_MAX_CONCURRENCY` | none | `4` | Maximum concurrent read-only calls | -| `KNAP_SCREENSHOT_DIR` | `--output-dir` | `./.knapper` | Default-profile artifact root | - -An empty toolset value starts only the 17 control tools. These tools remain visible -when you disable their toolsets: - -- `obsidian_agent_open`, `obsidian_agent_status`, and `obsidian_agent_close` -- `obsidian_workspace_create`, `obsidian_workspace_claim_default`, and `obsidian_workspace_list` -- `obsidian_workspace_status`, `obsidian_workspace_stop`, and `obsidian_workspace_restart` -- `obsidian_workspace_release` and `obsidian_workspace_destroy` -- `obsidian_status`, `obsidian_doctor`, and `obsidian_capabilities` -- `obsidian_toolsets`, `obsidian_tool_catalog`, and `obsidian_toolsets_update` - -`obsidian_toolsets` reports the enabled set. `obsidian_tool_catalog` searches all -tool definitions with cursor pagination. `obsidian_toolsets_update` accepts -`enable`, `disable`, and `dryRun`. -An effective update sends `notifications/tools/list_changed` to the MCP client. +| Environment variable | CLI flag | Default | Purpose | +| --------------------- | -------------- | ------------ | ----------------------------- | +| `KNAP_TOOLSETS` | `--toolsets` | `all` | Startup toolset selection | +| `KNAP_SCREENSHOT_DIR` | `--output-dir` | `./.knapper` | Default-profile artifact root | -Enable `core`, `workspace`, `telemetry`, and `plugin-dev` for the full plugin loop. -Add `ui` for browser automation. Add other toolsets only when the task needs them. +Knapper publishes the complete tool surface during MCP initialization. The list +does not change during a connection. Do not change the tool list after startup. +Knapper runs one operation at a time. ## Structured output @@ -85,7 +63,7 @@ Screenshot tools return this object: ``` The requested `path` must be relative to the configured output root. Screenshot -tools do not return inline base64 data. Isolated workspaces use their private +tools do not return inline base64 data. Private sessions use their private `output/` root. Doctor returns explicit version information in this shape: @@ -122,13 +100,12 @@ the package manager that supplied `installedPackage`. | `KNAP_RECONNECT_MS` | `2000` | Telemetry reconnect delay | Knapper writes default-profile telemetry to `KNAP_HOME/telemetry/events.jsonl`. -Each isolated workspace has a separate `.jsonl` file in that -directory. Switching workspaces does not erase records or mix histories. Knapper +Each managed session uses the shared telemetry file in that directory. Knapper writes redacted tool audit events under `KNAP_HOME/audit`. Audit files use mode `0600` and have 14-day retention. -Release and destroy operations archive an isolated workspace's telemetry file. -They store it in the retained or quarantined root. +Session release and reset operations archive telemetry records. They store retained +records in the managed or quarantined root. `LOG_LEVEL`, `RECONNECT_MS`, and `SCREENSHOT_DIR` are supported aliases. The `KNAP_` name takes precedence. @@ -141,8 +118,8 @@ They store it in the retained or quarantined root. | `MCP_PORT` | `--port` | `9223` | HTTP listen port | | `MCP_HOST` | `--host` | `127.0.0.1` | Exact loopback bind host | -HTTP serves `/mcp` with the MCP 2026 stateless request model. It does not issue an -`Mcp-Session-Id`. Each request can select a durable workspace handle. +HTTP is experimental. It serves `/mcp` with one global lane and one active session. +It does not issue an `Mcp-Session-Id`. Each request uses the active session. The HTTP server has no authentication. The listener accepts only `127.0.0.1` or `::1` as the bind host. It rejects `localhost`, wildcard addresses, and LAN @@ -165,5 +142,5 @@ binds the canonical path to its device and inode. Legacy `.knapper-managed` file have no effect. Authorization permits vault operations. It never permits directory deletion. -`obsidian_create_vault` refuses when an isolated workspace is selected. Use the -scratch vault that `obsidian_workspace_create` created for that workspace. +`obsidian_create_vault` refuses when a private session is selected. Use the +scratch vault that `obsidian_session_open` created for that session. diff --git a/docs/hosts.md b/docs/hosts.md index 99fd009..ed64030 100644 --- a/docs/hosts.md +++ b/docs/hosts.md @@ -54,7 +54,7 @@ the installed `knapper` binary: "command": ["knapper"], "enabled": true, "environment": { - "KNAP_TOOLSETS": "core,workspace,telemetry,plugin-dev", + "KNAP_TOOLSETS": "all", "KNAP_SCREENSHOT_DIR": "/absolute/path/to/knapper-output" } } @@ -62,8 +62,8 @@ the installed `knapper` binary: } ``` -Both environment variables are optional. `KNAP_TOOLSETS` selects the startup -surface. `KNAP_SCREENSHOT_DIR` must name the screenshot output root. +Both environment variables are optional. The MCP surface is fixed at initialization. +`KNAP_SCREENSHOT_DIR` must name the screenshot output root. Use this command array to track the default branch: diff --git a/package-lock.json b/package-lock.json index 2b2304c..62f2af4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "knapper", - "version": "0.6.0-beta.9", + "version": "0.7.0-beta.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "knapper", - "version": "0.6.0-beta.9", + "version": "0.7.0-beta.1", "license": "MIT", "dependencies": { "@modelcontextprotocol/client": "2.0.0", diff --git a/package.json b/package.json index e9fac84..8753fa6 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "knapper", - "version": "0.6.0-beta.9", - "description": "MCP server for Obsidian plugin development with isolated workspaces, browser automation, and debugging.", + "version": "0.7.0-beta.1", + "description": "MCP server for stateless Obsidian plugin development with one managed session, browser automation, and debugging.", "keywords": [ "automation", "cdp", diff --git a/rules/obsidian-plugin.mdc b/rules/obsidian-plugin.mdc index 42ce3ff..c0b24ca 100644 --- a/rules/obsidian-plugin.mdc +++ b/rules/obsidian-plugin.mdc @@ -11,6 +11,10 @@ alwaysApply: false ## Dev loop +- Open one private target with `obsidian_session_open` before plugin work. +- Use the active session implicitly. Do not pass caller-owned session identifiers. +- Run one operation at a time. If Knapper returns `KNAPPER_BUSY`, wait for + `retryAfterMs` and check `obsidian_status`. - Build on the host (`npm run build`); the MCP server does not compile TypeScript. - Link into a dev vault with `obsidian_link_plugin` (symlink), never copy artifacts manually each time. - Verify with `obsidian_dev_cycle` after changes; read attributed errors from the response or `obsidian_logs(since=…)`. @@ -30,6 +34,9 @@ alwaysApply: false ## Safety - Use a scratch dev vault for `obsidian_reset_state` and destructive tests. +- Keep the private profile and `XDG_RUNTIME_DIR` isolation intact. +- Use `obsidian_session_release` after testing. Knapper moves verified roots to + recoverable trash and never hard-deletes them. - Run `obsidian_doctor` when transports fail; quit Obsidian fully before expecting CDP after adding `--remote-debugging-port`. ## Tool discovery @@ -38,3 +45,4 @@ alwaysApply: false - Plugin-dev: reload, manifest, settings, dev_cycle, exercise_command. - Telemetry: logs with cursor `since`, log_mark. - UI: `browser_*` (requires CDP). +- The MCP tool list is fixed during initialization. Do not change it during a connection. diff --git a/scripts/acceptance.mjs b/scripts/acceptance.mjs index b0e74a3..dc92d90 100644 --- a/scripts/acceptance.mjs +++ b/scripts/acceptance.mjs @@ -19,15 +19,6 @@ import { stat } from "node:fs/promises"; const root = join(dirname(fileURLToPath(import.meta.url)), ".."); let VAULT; const PLUGIN = process.env.PLUGIN_ID; -const CONTROL_TOOLS = new Set([ - "obsidian_agent_open", - "obsidian_agent_close", - "obsidian_workspace_claim_default", - "obsidian_workspace_stop", - "obsidian_workspace_release", -]); -let agentHandle; -let workspaceHandle; class McpClient { #child; @@ -94,11 +85,7 @@ class McpClient { } async call(name, args = {}) { - const input = - workspaceHandle !== undefined && !CONTROL_TOOLS.has(name) - ? { ...args, workspaceHandle } - : args; - const res = await this.send("tools/call", { name, arguments: input }); + const res = await this.send("tools/call", { name, arguments: args }); if (res.error) throw new Error(`${name}: ${res.error.message}`); const content = res.result?.content ?? []; const text = content @@ -149,11 +136,8 @@ try { console.log(`server: ${init.result.serverInfo.name} v${init.result.serverInfo.version}\n`); const isolated = await createDisposableWorkspace(client, root, { home: liveHome.home, - agentLabel: "acceptance", label: "acceptance-scratch", }); - agentHandle = isolated.agentHandle; - workspaceHandle = isolated.workspaceHandle; VAULT = isolated.session.vault?.name; assert(typeof VAULT === "string", "isolated workspace has no vault identity"); for (const [path, content] of [ @@ -347,15 +331,7 @@ try { }, ); } finally { - if (workspaceHandle !== undefined) { - await client.call("obsidian_workspace_stop", { workspaceHandle }).catch(() => undefined); - await client.call("obsidian_workspace_release", { workspaceHandle }).catch(() => undefined); - workspaceHandle = undefined; - } - if (agentHandle !== undefined) { - await client.call("obsidian_agent_close", { agentHandle }).catch(() => undefined); - agentHandle = undefined; - } + await client.call("obsidian_session_release").catch(() => undefined); client.close(); await removeLiveHome(liveHome.home).catch(() => undefined); } diff --git a/scripts/background-input-live.mjs b/scripts/background-input-live.mjs index d5bccba..080bee7 100644 --- a/scripts/background-input-live.mjs +++ b/scripts/background-input-live.mjs @@ -23,15 +23,6 @@ import { createDisposableWorkspace, createLiveHome, removeLiveHome } from "./lib const root = join(dirname(fileURLToPath(import.meta.url)), ".."); let VAULT; -const CONTROL_TOOLS = new Set([ - "obsidian_agent_open", - "obsidian_agent_close", - "obsidian_workspace_claim_default", - "obsidian_workspace_stop", - "obsidian_workspace_release", -]); -let agentHandle; -let workspaceHandle; const execFileAsync = promisify(execFile); const activeDesktopWindow = async () => { @@ -96,11 +87,7 @@ class McpClient { } async call(name, args = {}) { - const input = - workspaceHandle !== undefined && !CONTROL_TOOLS.has(name) - ? { ...args, workspaceHandle } - : args; - const res = await this.send("tools/call", { name, arguments: input }); + const res = await this.send("tools/call", { name, arguments: args }); if (res.error) throw new Error(`${name}: ${res.error.message}`); const text = (res.result?.content ?? []).map((c) => c.text ?? "").join("\n"); return { @@ -148,8 +135,6 @@ const closePalette = async (client) => { }; console.log("\n\x1b[1m=== knapper background input — live ===\x1b[0m"); -console.log(`vault: ${VAULT}`); -console.log("Do NOT click into Obsidian while this runs.\n"); const liveHome = await createLiveHome("knapper-bg-input-"); const client = new McpClient(["--toolsets", "all"], liveHome.env); @@ -163,10 +148,10 @@ const isolated = await createDisposableWorkspace(client, root, { agentLabel: "bg-input-live", label: "background-input-scratch", }); -agentHandle = isolated.agentHandle; -workspaceHandle = isolated.workspaceHandle; VAULT = isolated.session.vault?.name; assert(typeof VAULT === "string", "isolated workspace has no vault identity"); +console.log(`vault: ${VAULT}`); +console.log("Do NOT click into Obsidian while this runs.\n"); const foregroundBefore = await activeDesktopWindow(); console.log("Preconditions"); @@ -243,13 +228,10 @@ await check("an unpaired browser_keydown is released during MCP shutdown", async capabilities: {}, clientInfo: { name: "bg-input-live-2", version: "1" }, }); - const stopped = await after.call("obsidian_workspace_stop", { workspaceHandle }); - assert(!stopped.isError, `workspace stop failed: ${stopped.text}`); - const released = await after.call("obsidian_workspace_release", { workspaceHandle }); + const reopened = await after.call("obsidian_session_open", { target: "isolated" }); + assert(!reopened.isError, `session reopen failed: ${reopened.text}`); + const released = await after.call("obsidian_session_release"); assert(!released.isError, `workspace release failed: ${released.text}`); - workspaceHandle = undefined; - const closed = await after.call("obsidian_agent_close", { agentHandle }); - assert(!closed.isError, `agent close failed: ${closed.text}`); } finally { after.close(); await removeLiveHome(liveHome.home); diff --git a/scripts/ci-smoke.mjs b/scripts/ci-smoke.mjs index 01cf497..3cc2989 100644 --- a/scripts/ci-smoke.mjs +++ b/scripts/ci-smoke.mjs @@ -25,19 +25,6 @@ await rm(knapHome, { recursive: true, force: true }); /** A port nothing can be listening on, so attach must fail fast. */ const DEAD_CDP = "http://127.0.0.1:1"; -const CONTROL_TOOL_COUNT = 17; -const ALL_TOOLSETS = [ - "core", - "workspace", - "ui", - "telemetry", - "plugin-dev", - "editor", - "vault", - "devtools", - "authoring", -]; - let failed = 0; function check(label, condition, detail = "") { if (condition) { @@ -122,39 +109,23 @@ try { child.stdin.write(`${JSON.stringify({ jsonrpc: "2.0", method: "notifications/initialized" })}\n`); const listed = await send("tools/list"); - const controlTools = listed.result?.tools ?? []; - check( - "startup surface contains only control tools", - controlTools.length === CONTROL_TOOL_COUNT, - `${controlTools.length} tools`, - ); - - const controlNames = new Set(controlTools.map((tool) => tool.name)); + const tools = listed.result?.tools ?? []; + const names = new Set(tools.map((tool) => tool.name)); + check("startup surface contains operational tools", tools.length > 60, `${tools.length} tools`); for (const required of [ "obsidian_status", "obsidian_doctor", "obsidian_capabilities", - "obsidian_toolsets", - "obsidian_agent_open", - "obsidian_workspace_claim_default", - "obsidian_tool_catalog", - "obsidian_toolsets_update", + "obsidian_session_open", + "obsidian_session_status", + "obsidian_session_release", + "obsidian_session_reset", + "browser_snapshot", + "browser_click", + "obsidian_plugin_health", + "obsidian_dev_cycle", ]) { - check(`${required} is registered`, controlNames.has(required)); - } - check("operational tools start disabled", !controlNames.has("obsidian_eval")); - - const enabledAll = await send("tools/call", { - name: "obsidian_toolsets_update", - arguments: { enable: ALL_TOOLSETS }, - }); - check("all operational toolsets enable at runtime", enabledAll.result?.isError !== true); - const expanded = await send("tools/list"); - const tools = expanded.result?.tools ?? []; - const names = new Set(tools.map((tool) => tool.name)); - check("runtime surface expands without reconnect", tools.length > 80, `${tools.length} tools`); - for (const required of ["browser_snapshot", "browser_click", "browser_take_screenshot"]) { - check(`${required} is registered without Obsidian`, names.has(required)); + check(`${required} is registered`, names.has(required)); } check("no duplicate tool names", names.size === tools.length); check( @@ -166,28 +137,19 @@ try { tools.every((t) => typeof t.annotations?.readOnlyHint === "boolean"), ); - check("legacy session tools are absent", !names.has("obsidian_isolate")); + check("legacy dynamic tool update is absent", !names.has("obsidian_toolsets_update")); + check("legacy agent handles are absent", !names.has("obsidian_agent_open")); + check("legacy workspace handles are absent", !names.has("obsidian_workspace_claim_default")); - const openedAgent = await send("tools/call", { - name: "obsidian_agent_open", - arguments: { label: "ci-smoke" }, - }); - const agentHandle = openedAgent.result?.structuredContent?.agentHandle; - check("agent handle opens", typeof agentHandle === "string", String(agentHandle)); - const claimed = await send("tools/call", { - name: "obsidian_workspace_claim_default", - arguments: { agentHandle, label: "offline-default" }, + const session = await send("tools/call", { + name: "obsidian_session_status", + arguments: {}, }); - const workspaceHandle = claimed.result?.structuredContent?.workspaceHandle; - check( - "default workspace handle opens", - typeof workspaceHandle === "string", - String(workspaceHandle), - ); + check("session status answers without a handle", session.result?.isError !== true); const status = await send("tools/call", { name: "obsidian_status", - arguments: { workspaceHandle }, + arguments: {}, }); const statusText = (status.result?.content ?? []) .filter((c) => c.type === "text") @@ -202,7 +164,7 @@ try { const doctor = await send("tools/call", { name: "obsidian_doctor", - arguments: { workspaceHandle }, + arguments: {}, }); const doctorText = (doctor.result?.content ?? []) .filter((c) => c.type === "text") @@ -218,7 +180,7 @@ try { // error rather than a transport-level crash. const evaluated = await send("tools/call", { name: "obsidian_eval", - arguments: { workspaceHandle, code: "1+1" }, + arguments: { code: "1+1" }, }); const evalText = (evaluated.result?.content ?? []) .filter((c) => c.type === "text") @@ -229,35 +191,18 @@ try { const clicked = await send("tools/call", { name: "browser_click", - arguments: { workspaceHandle, target: ".workspace" }, + arguments: { target: ".workspace" }, }); const clickText = (clicked.result?.content ?? []) .filter((c) => c.type === "text") .map((c) => c.text) .join("\n"); check("browser calls fail cleanly without CDP", clicked.result?.isError === true); - check("browser failure points to obsidian_launch", /obsidian_launch|cold-start/i.test(clickText)); + check("browser failure points to session setup", /obsidian_session_open/i.test(clickText)); - const listChangesBeforeInspection = notifications.filter( - (method) => method === "notifications/tools/list_changed", - ).length; - const toolsets = await send("tools/call", { - name: "obsidian_toolsets", - arguments: {}, - }); - check( - "toolset report is structured and read-only", - Array.isArray(toolsets.result?.structuredContent?.enabled) && - Array.isArray(toolsets.result?.structuredContent?.disabled), - ); - const afterToolsets = await send("tools/list"); - const afterNames = new Set((afterToolsets.result?.tools ?? []).map((tool) => tool.name)); - check("toolset inspection leaves tools/list unchanged", afterNames.size === names.size); - check("toolset control remains available", afterNames.has("obsidian_toolsets")); check( - "toolset inspection emits no list_changed notification", - notifications.filter((method) => method === "notifications/tools/list_changed").length === - listChangesBeforeInspection, + "static surface emits no list_changed notification", + !notifications.includes("notifications/tools/list_changed"), ); } catch (e) { check(`smoke sequence completed`, false, e.message); diff --git a/scripts/e2e.mjs b/scripts/e2e.mjs index 0976cc9..c032a9a 100644 --- a/scripts/e2e.mjs +++ b/scripts/e2e.mjs @@ -35,23 +35,6 @@ let VAULT_DIR; const PLUGIN = process.env.PLUGIN_ID; /** All notes this suite writes live here so cleanup is a single recursive delete. */ const E2E_DIR = "E2E"; -const CONTROL_TOOLS = new Set([ - "obsidian_agent_open", - "obsidian_agent_status", - "obsidian_agent_close", - "obsidian_workspace_create", - "obsidian_workspace_claim_default", - "obsidian_workspace_list", - "obsidian_workspace_status", - "obsidian_workspace_stop", - "obsidian_workspace_restart", - "obsidian_workspace_release", - "obsidian_workspace_destroy", - "obsidian_toolsets", - "obsidian_tool_catalog", -]); -let defaultAgentHandle; -let defaultWorkspaceHandle; const onlyArg = process.argv.indexOf("--only"); const ONLY = onlyArg === -1 ? undefined : process.argv[onlyArg + 1]; @@ -135,11 +118,7 @@ class McpClient { } async call(name, args = {}) { - const input = - defaultWorkspaceHandle !== undefined && !CONTROL_TOOLS.has(name) - ? { ...args, workspaceHandle: defaultWorkspaceHandle } - : args; - const res = await this.send("tools/call", { name, arguments: input }); + const res = await this.send("tools/call", { name, arguments: args }); if (res.error) throw new Error(`${name}: ${res.error.message}`); const content = res.result?.content ?? []; const text = content @@ -251,13 +230,10 @@ try { const init = await client.initialize(); const isolated = await createDisposableWorkspace(client, root, { home: liveHome.home, - agentLabel: "e2e", label: "e2e-scratch", ...(process.env.PLUGIN_SOURCE_DIR ? { pluginSourceDir: process.env.PLUGIN_SOURCE_DIR } : {}), ...(process.env.PLUGIN_ID ? { pluginId: process.env.PLUGIN_ID } : {}), }); - defaultAgentHandle = isolated.agentHandle; - defaultWorkspaceHandle = isolated.workspaceHandle; VAULT = isolated.session.vault?.name; VAULT_DIR = isolated.vaultPath; assert(typeof VAULT === "string", "isolated workspace has no vault identity"); @@ -401,9 +377,9 @@ try { await check("toolset inspection does not mutate the runtime surface", async () => { const before = await client.send("tools/list"); const beforeSurface = JSON.stringify(before.result.tools); - const report = await client.ok("obsidian_toolsets"); + const report = await client.ok("obsidian_session_status"); const after = await client.send("tools/list"); - assert(Array.isArray(report.json?.enabled), "toolset report omitted the enabled set"); + assert(report.json?.active, "session status omitted the active session"); assert(beforeSurface === JSON.stringify(after.result.tools), "tools/list changed at runtime"); }); } @@ -747,22 +723,15 @@ try { if (suite("Editor toolset")) { const note = `${E2E_DIR}/editor.md`; - await check("the default surface stays slim", async () => { + await check("the default surface is fixed at initialization", async () => { const dflt = new McpClient([], liveHome.env); try { await dflt.initialize(); const res = await dflt.send("tools/list"); const have = new Set(res.result.tools.map((t) => t.name)); - const wanted = ["obsidian_status", "obsidian_workspace_create", "obsidian_toolsets_update"]; + const wanted = ["obsidian_status", "obsidian_session_open", "browser_snapshot"]; const missing = wanted.filter((name) => !have.has(name)); - const leaked = ["obsidian_editor_state", "obsidian_create", "browser_snapshot"].filter( - (name) => have.has(name), - ); assert(missing.length === 0, `missing from default surface: ${missing.join(", ")}`); - assert( - leaked.length === 0, - `optional tools leaked into default surface: ${leaked.join(", ")}`, - ); } finally { dflt.close(); } @@ -983,11 +952,11 @@ try { await client.ok("obsidian_plugin_disable", { id: PLUGIN }); await sleep(500); const off = await client.ok("obsidian_plugin_health", { pluginId: PLUGIN }); - assert(/false|disabled|not (enabled|loaded)/i.test(off.text), "still reported enabled"); + assert(off.json?.enabled === false && off.json?.loaded === false, "still reported enabled"); await client.ok("obsidian_plugin_enable", { id: PLUGIN }); await sleep(800); const on = await client.ok("obsidian_plugin_health", { pluginId: PLUGIN }); - assert(/enabled|loaded/i.test(on.text), "did not come back enabled"); + assert(on.json?.enabled === true && on.json?.loaded === true, "did not come back enabled"); }); await check("plugin settings read and write data.json", async () => { @@ -995,6 +964,31 @@ try { assert(!isError, "settings read errored"); }); + if (PLUGIN === "knapper-settings-fixture") { + await check("plugin settings change through the live UI", async () => { + await client.ok("obsidian_eval", { + code: `app.setting.open(); app.setting.openTabById(${JSON.stringify(PLUGIN)}); true`, + }); + await sleep(500); + const snapshot = await client.ok("obsidian_snapshot", { scope: "settings" }); + assert(/Enable fixture/.test(snapshot.text), "fixture setting is not visible"); + await client.ok("browser_click", { + target: ".knapper-settings-fixture input[type=checkbox]", + element: "Enable fixture toggle", + }); + const enabled = await waitFor( + async () => { + const result = await client.ok("obsidian_eval", { + code: `app.plugins.plugins[${JSON.stringify(PLUGIN)}]?.settings?.enabled === true`, + }); + return /true/.test(result.text); + }, + { what: "the live setting to persist", timeoutMs: 5000 }, + ); + assert(enabled, "the fixture setting did not change"); + }); + } + await check("reset_state wipes data.json and returns the previous contents", async () => { const { text, isError } = await client.call("obsidian_reset_state", { pluginId: PLUGIN }); assert(!isError, `reset errored: ${text.slice(0, 150)}`); @@ -1082,8 +1076,8 @@ try { // --------------------------------------------------- suite: stability additions - if (suite("Stability: concurrency, transport, reconnect")) { - await check("overlapping read-only calls all succeed", async () => { + if (suite("Stability: single lane, transport, reconnect")) { + await check("simultaneous calls complete through the FIFO lane", async () => { const results = await Promise.all([ client.call("obsidian_status"), client.call("obsidian_files", {}), @@ -1093,11 +1087,11 @@ try { client.call("obsidian_list_targets"), ]); const bad = results.filter((r) => r.isError); - assert(bad.length === 0, `${bad.length} of ${results.length} concurrent reads failed`); - return `${results.length} parallel reads`; + assert(bad.length === 0, `${bad.length} of ${results.length} queued calls failed`); + return `${results.length} queued calls`; }); - await check("overlapping mutating calls serialize without corrupting each other", async () => { + await check("queued mutations cannot corrupt each other", async () => { const dir = `${E2E_DIR}/conc`; const n = 5; const results = await Promise.all( @@ -1110,8 +1104,8 @@ try { ), ); const bad = results.filter((r) => r.isError); - assert(bad.length === 0, `${bad.length}/${n} concurrent creates failed`); - // Every file must exist with exactly its own body — interleaving would cross them. + assert(bad.length === 0, `${bad.length}/${n} queued creates failed`); + // Every file must exist with exactly its own body. for (let i = 0; i < n; i++) { const rel = `${dir}/note-${i}.md`; await waitFor(() => fileExists(rel), { what: rel }); @@ -1121,7 +1115,7 @@ try { return `${n} serialized writes`; }); - await check("a mixed read/write burst leaves the server responsive", async () => { + await check("a mixed queue leaves the server responsive", async () => { await Promise.all([ client.call("obsidian_status"), client.call("obsidian_notice", { message: "burst", duration: 400 }), @@ -1130,7 +1124,7 @@ try { client.call("obsidian_logs", { limit: 3 }), ]); const { isError } = await client.call("obsidian_status"); - assert(!isError, "server unresponsive after a mixed burst"); + assert(!isError, "server unresponsive after a mixed queue"); }); await check("http transport serves a real MCP handshake", async () => { @@ -1291,36 +1285,12 @@ try { }); } - await check("default workspace and agent handles close cleanly", async () => { - await client.ok("obsidian_workspace_stop", { workspaceHandle: defaultWorkspaceHandle }); - const released = await client.ok("obsidian_workspace_release", { - workspaceHandle: defaultWorkspaceHandle, - }); - assert(released.json?.released === true, "default workspace was not released"); - defaultWorkspaceHandle = undefined; - const closed = await client.ok("obsidian_agent_close", { agentHandle: defaultAgentHandle }); - assert(closed.json?.closed === true, "agent handle was not closed"); - defaultAgentHandle = undefined; + await check("the active session releases cleanly", async () => { + const released = await client.ok("obsidian_session_release"); + assert(released.json?.released, "active session was not released"); }); - - client.close(); - await removeLiveHome(liveHome.home); } finally { - if (defaultWorkspaceHandle !== undefined) { - await client - .call("obsidian_workspace_stop", { workspaceHandle: defaultWorkspaceHandle }) - .catch(() => undefined); - await client - .call("obsidian_workspace_release", { workspaceHandle: defaultWorkspaceHandle }) - .catch(() => undefined); - defaultWorkspaceHandle = undefined; - } - if (defaultAgentHandle !== undefined) { - await client - .call("obsidian_agent_close", { agentHandle: defaultAgentHandle }) - .catch(() => undefined); - defaultAgentHandle = undefined; - } + await client.call("obsidian_session_release").catch(() => undefined); client.close(); await removeLiveHome(liveHome.home).catch(() => undefined); } @@ -1338,4 +1308,4 @@ if (failures.length > 0) { console.log("\nFailures:"); for (const f of failures) console.log(` - ${f}`); } -process.exit(failed > 0 ? 1 : 0); +process.exitCode = failed > 0 ? 1 : 0; diff --git a/scripts/fence-live.mjs b/scripts/fence-live.mjs index 7868018..e5eb87a 100644 --- a/scripts/fence-live.mjs +++ b/scripts/fence-live.mjs @@ -17,19 +17,11 @@ import { mkdir, readFile, writeFile } from "node:fs/promises"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; import { createDisposableWorkspace, createLiveHome, removeLiveHome } from "./lib/live-harness.mjs"; +import { stopSession } from "../dist/session/registry.js"; const root = join(dirname(fileURLToPath(import.meta.url)), ".."); let AUTHORIZED; let UNAUTHORIZED; -const CONTROL_TOOLS = new Set([ - "obsidian_agent_open", - "obsidian_agent_close", - "obsidian_workspace_claim_default", - "obsidian_workspace_stop", - "obsidian_workspace_release", -]); -let agentHandle; -let workspaceHandle; class McpClient { #child; @@ -82,11 +74,7 @@ class McpClient { } async call(name, args = {}) { - const input = - workspaceHandle !== undefined && !CONTROL_TOOLS.has(name) - ? { ...args, workspaceHandle } - : args; - const res = await this.send("tools/call", { name, arguments: input }); + const res = await this.send("tools/call", { name, arguments: args }); if (res.error) throw new Error(`${name}: ${res.error.message}`); const text = (res.result?.content ?? []).map((c) => c.text ?? "").join("\n"); return { @@ -133,8 +121,6 @@ function assertFenced(result, what) { } console.log("\n\x1b[1m=== knapper vault fence — live ===\x1b[0m"); -console.log(`authorized: ${AUTHORIZED}`); -console.log(`unauthorized: ${UNAUTHORIZED}\n`); const liveHome = await createLiveHome("knapper-fence-"); const client = new McpClient(["--toolsets", "all"], liveHome.env); @@ -149,8 +135,6 @@ try { agentLabel: "fence-live", label: "fence-authorized-scratch", }); - agentHandle = isolated.agentHandle; - workspaceHandle = isolated.workspaceHandle; AUTHORIZED = isolated.session.vault?.name; assert(typeof AUTHORIZED === "string", "isolated workspace has no vault identity"); @@ -164,9 +148,14 @@ try { code: `localStorage.setItem(${JSON.stringify(`enable-plugin-${unauthorizedVaultId}`)}, "true")`, }); assert(!trustedIdentity.isError, `identity trust seed failed: ${trustedIdentity.text}`); - const stoppedForSeed = await client.call("obsidian_workspace_stop", { workspaceHandle }); - assert(!stoppedForSeed.isError, `workspace stop failed: ${stoppedForSeed.text}`); + const stoppedForSeed = await stopSession(isolated.session.key, { env: liveHome.env }); + assert( + stoppedForSeed.state !== "quitFailed", + "session stop failed while preparing fence fixture", + ); UNAUTHORIZED = `${AUTHORIZED}-unauthorized`; + console.log(`authorized: ${AUTHORIZED}`); + console.log(`unauthorized: ${UNAUTHORIZED}\n`); const unauthorizedPath = join(dirname(isolated.vaultPath), UNAUTHORIZED); await mkdir(join(unauthorizedPath, ".obsidian"), { recursive: true, mode: 0o700 }); const { SESSION_IDENTITY_PLUGIN_ID, seedSessionIdentityPlugin } = await import( @@ -195,7 +184,7 @@ try { open: true, }; await writeFile(registryPath, `${JSON.stringify(privateRegistry, null, 2)}\n`, "utf8"); - const restartedAfterSeed = await client.call("obsidian_workspace_restart", { workspaceHandle }); + const restartedAfterSeed = await client.call("obsidian_session_open", { target: "isolated" }); assert(!restartedAfterSeed.isError, `workspace restart failed: ${restartedAfterSeed.text}`); console.log("Preconditions"); @@ -345,15 +334,7 @@ try { ); }); } finally { - if (workspaceHandle !== undefined) { - await client.call("obsidian_workspace_stop", { workspaceHandle }).catch(() => undefined); - await client.call("obsidian_workspace_release", { workspaceHandle }).catch(() => undefined); - workspaceHandle = undefined; - } - if (agentHandle !== undefined) { - await client.call("obsidian_agent_close", { agentHandle }).catch(() => undefined); - agentHandle = undefined; - } + await client.call("obsidian_session_release").catch(() => undefined); client.close(); await removeLiveHome(liveHome.home).catch(() => undefined); } diff --git a/scripts/lib/live-harness.mjs b/scripts/lib/live-harness.mjs index 85f78e0..f159a42 100644 --- a/scripts/lib/live-harness.mjs +++ b/scripts/lib/live-harness.mjs @@ -8,40 +8,26 @@ export async function createLiveHome(prefix = "knapper-live-") { return { home, env: { ...process.env, KNAP_HOME: home } }; } -/** - * Create an isolated workspace and prove its vault is Knapper-owned before callers write. - * The workspace tool allocates CDP port zero, so no shared port can be selected by a suite. - */ -export async function createDisposableWorkspace(client, root, options = {}) { - const opened = await client.call("obsidian_agent_open", { - label: options.agentLabel ?? "live-suite", - purpose: "isolated live validation", - cwd: root, - }); - const agentHandle = opened.json?.agentHandle; - if (typeof agentHandle !== "string") throw new Error(`agent open failed: ${opened.text}`); - const args = { agentHandle, label: options.label ?? "isolated-live" }; +/** Open the one active isolated session and prove its vault is Knapper-owned. */ +export async function createDisposableWorkspace(client, _root, options = {}) { + const args = { target: "isolated", label: options.label ?? "isolated-live" }; if (options.pluginSourceDir !== undefined) args.pluginSourceDir = options.pluginSourceDir; if (options.pluginId !== undefined) args.pluginId = options.pluginId; - const created = await client.call("obsidian_workspace_create", args); - const workspaceHandle = created.json?.workspaceHandle; - if (typeof workspaceHandle !== "string") - throw new Error(`workspace create failed: ${created.text}`); + const created = await client.call("obsidian_session_open", args); + const sessionKey = created.json?.session; + if (typeof sessionKey !== "string") throw new Error(`session open failed: ${created.text}`); const home = options.home ?? process.env.KNAP_HOME; if (!home) throw new Error("KNAP_HOME is required for isolated live suites"); - const workspaces = JSON.parse( - await readFile(join(home, "workspaces", `${workspaceHandle}.json`), "utf8"), - ); const session = JSON.parse( - await readFile(join(home, "sessions", workspaces.sessionKey, "session.json"), "utf8"), + await readFile(join(home, "sessions", sessionKey, "session.json"), "utf8"), ); const vaultPath = resolve(session.ownership?.vaultPath ?? ""); const ownedRoot = resolve(home); if (!session.ownership || !vaultPath.startsWith(`${ownedRoot}/`)) { - throw new Error(`workspace ${workspaceHandle} is not a Knapper-owned scratch vault`); + throw new Error(`session ${sessionKey} is not a Knapper-owned scratch vault`); } await stat(vaultPath); - return { agentHandle, workspaceHandle, vaultPath, session }; + return { sessionKey, vaultPath, session }; } export async function removeLiveHome(home) { diff --git a/scripts/workspaces-live.mjs b/scripts/workspaces-live.mjs index 7c9cfaf..1482f1d 100644 --- a/scripts/workspaces-live.mjs +++ b/scripts/workspaces-live.mjs @@ -1,67 +1,46 @@ /** - * Live isolation and lifecycle suite for explicit workspace handles. + * Live singleton-activity suite. * - * This suite creates only Knapper-owned scratch vaults. It snapshots the user's - * Obsidian vault registry before launch and requires byte-for-byte equality after - * create, reconnect, restart, release, and destroy operations. + * Knapper owns one managed Obsidian session. A second stdio client must observe + * that activity, receive KNAPPER_BUSY for mutations, and take over after release. * * npm run workspaces */ import { spawn } from "node:child_process"; -import { mkdir, mkdtemp, readFile, readdir, rm, stat, writeFile } from "node:fs/promises"; -import { dirname, join, resolve } from "node:path"; +import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; -import { tmpdir } from "node:os"; -import { quarantineSession, stopSession } from "../dist/session/registry.js"; -import { obsidianConfigPath } from "../dist/config.js"; +import { createDisposableWorkspace, createLiveHome, removeLiveHome } from "./lib/live-harness.mjs"; const ROOT = join(dirname(fileURLToPath(import.meta.url)), ".."); const CLI = join(ROOT, "dist", "cli.js"); -const TEST_ROOT = await mkdtemp(join(tmpdir(), "knapper-workspaces-")); -const CONTROL_TOOLS = new Set([ - "obsidian_agent_open", - "obsidian_agent_status", - "obsidian_agent_close", - "obsidian_workspace_create", - "obsidian_workspace_claim_default", - "obsidian_workspace_list", - "obsidian_workspace_status", - "obsidian_workspace_stop", - "obsidian_workspace_restart", - "obsidian_workspace_release", - "obsidian_workspace_destroy", - "obsidian_toolsets", - "obsidian_tool_catalog", -]); - let passed = 0; let failed = 0; -const failures = []; -function check(name, ok, detail) { +function check(name, ok, detail = "") { if (ok) { passed++; console.log(` ok ${name}`); } else { failed++; - failures.push({ name, detail }); - console.log(` FAIL ${name}${detail ? ` ${JSON.stringify(detail).slice(0, 300)}` : ""}`); + console.log(` FAIL ${name}${detail ? ` — ${detail}` : ""}`); } } class McpClient { - constructor(env) { + constructor(env, name) { this.child = spawn("node", [CLI, "--toolsets", "all", "--log-level", "error"], { stdio: ["pipe", "pipe", "pipe"], env: { ...process.env, ...env }, }); + this.name = name; this.buffer = ""; this.pending = new Map(); this.nextId = 1; - this.stderr = ""; + this.child.stderr.on("data", (chunk) => { + if (process.env.VERBOSE) process.stderr.write(`[${name}] ${chunk}`); + }); this.child.stdout.on("data", (chunk) => this.onData(chunk)); - this.child.stderr.on("data", (chunk) => (this.stderr += chunk.toString())); } onData(chunk) { @@ -70,341 +49,109 @@ class McpClient { while ((index = this.buffer.indexOf("\n")) >= 0) { const line = this.buffer.slice(0, index).trim(); this.buffer = this.buffer.slice(index + 1); - if (line === "") continue; + if (!line) continue; let message; try { message = JSON.parse(line); } catch { continue; } - const resolver = this.pending.get(message.id); - if (resolver !== undefined) { + const resolve = this.pending.get(message.id); + if (resolve) { this.pending.delete(message.id); - resolver(message); + resolve(message); } } } send(method, params) { const id = this.nextId++; - return new Promise((resolvePromise, reject) => { + return new Promise((resolve, reject) => { const timer = setTimeout(() => { - this.pending.delete(id); - reject(new Error(`${method} timed out\n${this.stderr.slice(-1500)}`)); - }, 180_000); + if (this.pending.delete(id)) reject(new Error(`${method} timed out`)); + }, 45_000); this.pending.set(id, (message) => { clearTimeout(timer); - resolvePromise(message); + resolve(message); }); this.child.stdin.write(`${JSON.stringify({ jsonrpc: "2.0", id, method, params })}\n`); }); } - async init() { - await this.send("initialize", { + async initialize() { + const result = await this.send("initialize", { protocolVersion: "2024-11-05", capabilities: {}, - clientInfo: { name: "workspaces-live", version: "1" }, + clientInfo: { name: this.name, version: "1" }, }); - this.child.stdin.write( - `${JSON.stringify({ jsonrpc: "2.0", method: "notifications/initialized" })}\n`, - ); + this.child.stdin.write('{"jsonrpc":"2.0","method":"notifications/initialized"}\n'); + return result; } async call(name, args = {}) { - const input = - this.workspaceHandle !== undefined && !CONTROL_TOOLS.has(name) - ? { ...args, workspaceHandle: this.workspaceHandle } - : args; - const response = await this.send("tools/call", { name, arguments: input }); - if (response.error !== undefined) { - throw new Error(`${name}: ${response.error.message ?? JSON.stringify(response.error)}`); - } - const content = response.result?.content ?? []; + const result = await this.send("tools/call", { name, arguments: args }); + if (result.error) throw new Error(result.error.message); + const content = result.result?.content ?? []; return { text: content .filter((item) => item.type === "text") .map((item) => item.text) .join("\n"), - json: response.result?.structuredContent, - isError: response.result?.isError === true, - error: response.error, + json: result.result?.structuredContent, + isError: result.result?.isError === true, }; } - async close() { + close() { this.child.stdin.end(); - await Promise.race([ - new Promise((resolvePromise) => this.child.once("exit", resolvePromise)), - new Promise((resolvePromise) => setTimeout(resolvePromise, 10_000).unref()), - ]); - if (this.child.exitCode === null) this.child.kill("SIGKILL"); - } -} - -async function exists(path) { - try { - await stat(path); - return true; - } catch { - return false; - } -} - -async function registryBytes() { - try { - return await readFile(obsidianConfigPath()); - } catch (error) { - if (error?.code === "ENOENT") return undefined; - throw error; } } -async function workspaceInternals(home, workspaceHandle) { - const workspace = JSON.parse( - await readFile(join(home, "workspaces", `${workspaceHandle}.json`), "utf8"), - ); - const descriptor = JSON.parse( - await readFile(join(home, "sessions", workspace.sessionKey, "session.json"), "utf8"), - ); - return { workspace, descriptor }; -} - -const home = await mkdtemp(join(TEST_ROOT, "w-")); -const env = { KNAP_HOME: home }; -const registryBefore = await registryBytes(); -const protectedVault = join(TEST_ROOT, "user-owned-vault"); -await mkdir(join(protectedVault, ".obsidian"), { recursive: true }); -await writeFile(join(protectedVault, "Important.md"), "protected test content\n", "utf8"); - -let client = new McpClient(env); -let agentA; -let agentB; -let workspaceA; -let workspaceB; -let retainedWorkspace; - +const liveHome = await createLiveHome("knapper-singleton-"); +const env = liveHome.env; +const first = new McpClient(env, "singleton-a"); +const second = new McpClient(env, "singleton-b"); try { - await client.init(); - - const openA = await client.call("obsidian_agent_open", { - label: "workspace-live-a", - purpose: "isolation and lifecycle validation", - cwd: ROOT, - }); - const openB = await client.call("obsidian_agent_open", { - label: "workspace-live-b", - purpose: "cross-workspace routing validation", - cwd: ROOT, - }); - agentA = openA.json?.agentHandle; - agentB = openB.json?.agentHandle; - check( - "two explicit agent handles open", - typeof agentA === "string" && typeof agentB === "string", - ); - - const createA = await client.call("obsidian_workspace_create", { - agentHandle: agentA, - label: "alpha", - }); - const createB = await client.call("obsidian_workspace_create", { - agentHandle: agentB, - label: "beta", - }); - workspaceA = createA.json?.workspaceHandle; - workspaceB = createB.json?.workspaceHandle; - check( - "two isolated workspaces start", - typeof workspaceA === "string" && typeof workspaceB === "string" && workspaceA !== workspaceB, - { createA: createA.text, createB: createB.text }, - ); - if (typeof workspaceA !== "string" || typeof workspaceB !== "string") { - throw new Error("workspace creation failed; remaining isolation checks cannot run"); - } - - const internalA = await workspaceInternals(home, workspaceA); - const internalB = await workspaceInternals(home, workspaceB); - check( - "workspace creation uses a Knapper-owned vault", - resolve(internalA.descriptor.vault.path).startsWith(resolve(home)), - internalA.descriptor.vault.path, - ); - check( - "scratch vaults use distinct exact workspace roots", - internalA.descriptor.vault.path !== internalB.descriptor.vault.path && - internalA.descriptor.vault.path.startsWith(home) && - internalB.descriptor.vault.path.startsWith(home), - ); - check( - "no in-vault management marker grants deletion", - !(await exists(join(internalA.descriptor.vault.path, ".knapper-managed"))) && - !(await exists(join(protectedVault, ".knapper-managed"))), - ); - check( - "the caller-owned vault remains unchanged", - (await readFile(join(protectedVault, "Important.md"), "utf8")) === "protected test content\n", - ); - - client.workspaceHandle = workspaceA; - const createNoteA = await client.call("obsidian_create", { - path: "alpha-only.txt", - content: "alpha", + await first.initialize(); + await second.initialize(); + const opened = await createDisposableWorkspace(first, ROOT, { + home: liveHome.home, + label: "singleton-scratch", }); - check("workspace A accepts a vault mutation", !createNoteA.isError, createNoteA.text); - - client.workspaceHandle = workspaceB; - const filesB = await client.call("obsidian_files"); - check( - "workspace B cannot see workspace A's note", - !filesB.text.includes("alpha-only"), - filesB.text, - ); + check("first client opens one isolated session", typeof opened.sessionKey === "string"); - const evalB = await client.call("obsidian_eval", { code: "app.vault.getName()" }); - client.workspaceHandle = workspaceA; - const evalA = await client.call("obsidian_eval", { code: "app.vault.getName()" }); - check( - "workspace handles route to different live apps", - evalA.text.includes(internalA.descriptor.vault.name) && - evalB.text.includes(internalB.descriptor.vault.name), - { a: evalA.text, b: evalB.text }, - ); - - await client.close(); - check( - "isolated Obsidian processes survive MCP teardown", - await exists(join(internalA.descriptor.instance.userDataDir, "DevToolsActivePort")), - ); - - client = new McpClient(env); - await client.init(); - const listed = await client.call("obsidian_workspace_list"); - const listedHandles = new Set( - (listed.json?.workspaces ?? []).map((item) => item.workspaceHandle), - ); - check( - "a new MCP process discovers both durable workspaces", - listedHandles.has(workspaceA) && listedHandles.has(workspaceB), - listed.json, - ); - - const restartA = await client.call("obsidian_workspace_restart", { workspaceHandle: workspaceA }); - check("workspace A restarts through its explicit handle", !restartA.isError, restartA.text); - client.workspaceHandle = workspaceB; - const bAfterRestart = await client.call("obsidian_eval", { code: "app.vault.getName()" }); - check( - "restarting A leaves B reachable and correctly routed", - bAfterRestart.text.includes(internalB.descriptor.vault.name), - bAfterRestart.text, - ); + const status = await second.call("obsidian_status"); + check("second client observes busy activity", /Agent use: busy/.test(status.text), status.text); - const retained = await client.call("obsidian_workspace_create", { - agentHandle: agentA, - label: "retained", + const blocked = await second.call("obsidian_create", { + path: "should-not-exist.md", + content: "busy\n", }); - retainedWorkspace = retained.json?.workspaceHandle; - check("a retained workspace is created", typeof retainedWorkspace === "string", retained.text); - if (typeof retainedWorkspace !== "string") { - throw new Error("retained workspace creation failed; release checks cannot run"); - } - const retainedInternal = await workspaceInternals(home, retainedWorkspace); - client.workspaceHandle = retainedWorkspace; - await client.call("obsidian_log_mark", { label: "retained-telemetry" }); - await client.call("obsidian_workspace_stop", { workspaceHandle: retainedWorkspace }); - const released = await client.call("obsidian_workspace_release", { - workspaceHandle: retainedWorkspace, - }); - check("release removes the handle without deleting the scratch vault", !released.isError); check( - "released scratch vault remains on disk", - await exists(retainedInternal.descriptor.vault.path), + "second client receives KNAPPER_BUSY", + blocked.isError && blocked.json?.code === "KNAPPER_BUSY", + blocked.text, ); - check( - "release archives workspace telemetry beside the retained vault", - typeof released.json?.telemetryArchive === "string" && - resolve(released.json.telemetryArchive).startsWith( - resolve(retainedInternal.descriptor.instance.userDataDir, ".."), - ) && - (await readFile(released.json.telemetryArchive, "utf8")).includes("retained-telemetry"), - released.json, - ); - - client.workspaceHandle = workspaceA; - await client.call("obsidian_log_mark", { label: "destroy-a-telemetry" }); - client.workspaceHandle = workspaceB; - await client.call("obsidian_log_mark", { label: "destroy-b-telemetry" }); - await client.call("obsidian_workspace_stop", { workspaceHandle: workspaceA }); - await client.call("obsidian_workspace_stop", { workspaceHandle: workspaceB }); - - const destroyedA = await client.call("obsidian_workspace_destroy", { - workspaceHandle: workspaceA, - }); - const destroyedB = await client.call("obsidian_workspace_destroy", { - workspaceHandle: workspaceB, - }); - for (const [label, result] of [ - ["A", destroyedA], - ["B", destroyedB], - ]) { - check(`destroy ${label} succeeds`, !result.isError, result.text); - check( - `destroy ${label} quarantines inside KNAP_HOME`, - typeof result.json?.quarantinedPath === "string" && - resolve(result.json.quarantinedPath).startsWith(resolve(join(home, "trash"))) && - (await exists(result.json.quarantinedPath)), - result.json, - ); - check( - `destroy ${label} moves telemetry into the quarantine`, - typeof result.json?.telemetryArchive === "string" && - resolve(result.json.telemetryArchive).startsWith(resolve(result.json.quarantinedPath)) && - (await exists(result.json.telemetryArchive)), - result.json, - ); - } - - const registryAfter = await registryBytes(); - check( - "the user's Obsidian vault registry is byte-for-byte unchanged", - registryBefore === undefined - ? registryAfter === undefined - : registryAfter !== undefined && registryBefore.equals(registryAfter), - ); + const released = await first.call("obsidian_session_release"); check( - "the user-owned vault is still present after all cleanup", - (await readFile(join(protectedVault, "Important.md"), "utf8")) === "protected test content\n", + "first client releases its session", + !released.isError && released.json?.released, + released.text, ); + const takeover = await second.call("obsidian_session_open", { target: "isolated" }); + check("release permits takeover", !takeover.isError, takeover.text); + check("takeover reuses the same session", takeover.json?.session === opened.sessionKey); - await client.call("obsidian_agent_close", { agentHandle: agentB }); - const closeAWithRetained = await client.call("obsidian_agent_close", { agentHandle: agentA }); - check("released workspaces do not block agent close", !closeAWithRetained.isError); - const workspaceRecords = await readdir(join(home, "workspaces")).catch(() => []); - check( - "no workspace records remain", - workspaceRecords.filter((name) => name.endsWith(".json")).length === 0, - ); + const sessionStatus = await second.call("obsidian_session_status"); + const sessions = sessionStatus.json?.managedSessions; + check("only one managed session is reported", sessions?.length === 1); + await second.call("obsidian_session_release").catch(() => undefined); } finally { - await client.close().catch(() => undefined); - const internalKeys = await readdir(join(home, "sessions")).catch(() => []); - let cleanupSafe = true; - for (const key of internalKeys) { - const cleanupEnv = { ...process.env, KNAP_HOME: home }; - const stopped = await stopSession(key, { env: cleanupEnv }).catch(() => undefined); - if (stopped?.state === "quitFailed") { - cleanupSafe = false; - console.error(`Preserving ${home}: Obsidian for ${key} did not stop.`); - continue; - } - await quarantineSession(key, { env: cleanupEnv }).catch(() => undefined); - } - if (cleanupSafe) await rm(TEST_ROOT, { recursive: true, force: true }); + first.close(); + second.close(); + await removeLiveHome(liveHome.home).catch(() => undefined); } console.log(`\n${passed} passed, ${failed} failed`); -if (failed > 0) { - console.log("\nFailures:"); - for (const failure of failures) console.log(` ${failure.name}`); - process.exit(1); -} +if (failed > 0) process.exit(1); diff --git a/skills/obsidian-debugging/SKILL.md b/skills/obsidian-debugging/SKILL.md index fd8a7af..5d08dfd 100644 --- a/skills/obsidian-debugging/SKILL.md +++ b/skills/obsidian-debugging/SKILL.md @@ -1,12 +1,12 @@ --- name: obsidian-debugging -description: Debug Obsidian plugins with knapper telemetry: cursor-based obsidian_logs tailing, obsidian_log_mark brackets, console and network capture, and error attribution from stack frames. Use after reloads, UI exercises, or mysterious plugin onload failures. +description: "Debug Obsidian plugins with knapper telemetry: cursor-based obsidian_logs tailing, obsidian_log_mark brackets, console and network capture, and error attribution from stack frames. Use after reloads, UI exercises, or mysterious plugin onload failures." --- # Obsidian debugging with telemetry -Telemetry tools need a workspace handle and a live Obsidian window. Pass -`workspaceHandle` on every telemetry call. CDP provides capture hooks. +Telemetry tools need an active Knapper session and a live Obsidian window. Knapper +selects the active target for every telemetry call. CDP provides capture hooks. ## The core primitive: cursor tailing @@ -17,10 +17,10 @@ This is the reliable answer to: _“What happened because of what I just did?” ### Pattern ```text -obsidian_log_mark(workspaceHandle=, label="before-reload") -obsidian_plugin_reload(workspaceHandle=, id="my-plugin") +obsidian_log_mark(label="before-reload") +obsidian_plugin_reload(id="my-plugin") # … reproduce issue … -obsidian_logs(workspaceHandle=, since=) +obsidian_logs(since=) ``` 1. Note cursor **before** the action (from `obsidian_log_mark` or a prior `obsidian_logs`). diff --git a/skills/obsidian-instance-setup/SKILL.md b/skills/obsidian-instance-setup/SKILL.md index 916944e..5742132 100644 --- a/skills/obsidian-instance-setup/SKILL.md +++ b/skills/obsidian-instance-setup/SKILL.md @@ -1,101 +1,89 @@ --- name: obsidian-instance-setup -description: Connect Knapper to a live Obsidian app with explicit agent and workspace handles. Use for doctor diagnosis, isolated scratch workspaces, default-profile claims, launch, CLI enablement, registry safety, or concurrent agents. +description: Connect Knapper to one live Obsidian app with a stateless session. Use for doctor diagnosis, private scratch sessions, launch, CLI enablement, and registry safety. --- # Obsidian instance setup -Use an isolated workspace for plugin development and experiments. Claim the default -profile only when the user explicitly wants an existing vault. +Use one private session for plugin development and experiments. Use the default +profile only when the user explicitly requests an existing vault. -## Select the target first +## Open a session -1. Call `obsidian_agent_open` with a short label and purpose. -2. Save the returned `agentHandle`. -3. Call `obsidian_workspace_create` with that handle. -4. Include `pluginSourceDir` and `pluginId` when you test a plugin. -5. Save the returned `workspaceHandle`. -6. Pass `workspaceHandle` to every operational tool. - -Example: +1. Call `obsidian_session_open` with `pluginSourceDir` and `pluginId`. +2. Call `obsidian_session_status` to inspect the target. +3. Call `obsidian_status` to confirm that the owner state is `self`. +4. Call `obsidian_doctor` when a transport is not ready. ```text -obsidian_agent_open label=my-plugin purpose="test the current build" -obsidian_workspace_create agentHandle= label=my-plugin \ - pluginSourceDir=/abs/path/to/dist pluginId=my-plugin -obsidian_plugin_health workspaceHandle= pluginId=my-plugin +obsidian_session_open pluginSourceDir=/abs/path/to/dist pluginId=my-plugin +obsidian_session_status +obsidian_plugin_health pluginId=my-plugin ``` -The workspace handle is durable across MCP reconnects. It selects one exact -Obsidian instance. It is not an authentication credential. - -## Concurrent agents - -Each agent must use a different isolated workspace. Knapper gives each workspace a -private profile, CLI socket, CDP port, and scratch vault. Calls route from the -explicit workspace handle, not from MCP transport state or `clientInfo`. +Operational tools use the active target. They do not need caller-owned identifiers. +Knapper keeps one managed session and runs one operation at a time. Use these lifecycle tools: ```text -obsidian_workspace_list agentHandle= -obsidian_workspace_status workspaceHandle= -obsidian_workspace_restart workspaceHandle= -obsidian_workspace_release workspaceHandle= -obsidian_workspace_destroy workspaceHandle= -obsidian_agent_close agentHandle= +obsidian_session_status +obsidian_session_release +obsidian_session_reset ``` -`release` stops the app and retains the scratch vault. `destroy` moves a verified -isolated root to recoverable Knapper trash. It does not hard-delete the root. -Release or destroy all workspaces before you close the agent handle. +`release` stops the app and retains the scratch vault. `reset` replaces the private +target and moves verified old roots to recoverable Knapper trash. Knapper never +hard-deletes a managed root. + +## Busy state + +`obsidian_status` reports `free`, `self`, `busy`, or `stale`. It also reports the +last activity time and `retryAfterMs`. A second process receives `KNAPPER_BUSY`. +Wait for the retry interval, then check status again. Knapper reclaims stale state +only after it verifies process death or an expired activity record. ## Default profile -Call `obsidian_workspace_claim_default` only for a user-approved default-profile -task. Then pass that workspace handle to `obsidian_doctor`. +Call `obsidian_session_open` without a private plugin target only for a +user-approved default-profile task. Existing vault access still requires terminal +authorization. -| State | Meaning | Action | -| ---------------------- | ---------------------------------------- | ----------------------------------------- | -| `OBSIDIAN_NOT_RUNNING` | Obsidian is stopped | Call `obsidian_launch` | -| `CLI_DISABLED` | Native CLI is disabled | Call `obsidian_setup_cli` | -| `CDP_PORT_CLOSED` | Browser automation cannot attach | Cold-start with `obsidian_launch` | -| `ARGV_CORRUPTION` | A wrapper passed a bad single-dash token | Correct `user-flags.conf` | -| `DEFAULT_PROFILE_BUSY` | Another MCP process owns the profile | Create an isolated workspace | -| `VAULT_NOT_AUTHORIZED` | The user did not grant vault access | Stop unless the user requested that vault | +| State | Meaning | Action | +| ---------------------- | ---------------------------------------- | --------------------------------- | +| `OBSIDIAN_NOT_RUNNING` | Obsidian is stopped | Call `obsidian_launch` | +| `CLI_DISABLED` | Native CLI is disabled | Call `obsidian_setup_cli` | +| `CDP_PORT_CLOSED` | Browser automation cannot attach | Cold-start with `obsidian_launch` | +| `ARGV_CORRUPTION` | A wrapper passed a bad single-dash token | Correct `user-flags.conf` | +| `KNAPPER_BUSY` | Another process is active | Wait for `retryAfterMs` | +| `VAULT_NOT_AUTHORIZED` | The user did not grant vault access | Stop | The Obsidian CLI prints some failures to stdout with exit code 0. Trust the typed Knapper result, not the process exit code. ## Vault safety -An isolated workspace always creates Knapper-owned scratch space. It cannot accept -or adopt an existing vault path. Knapper stores ownership outside the vault. It -checks the exact layout, real path, symlink state, device, and inode before cleanup. - -An Obsidian registry entry does not authorize access. Legacy `.knapper-managed` -files are inert. Only the user can authorize an existing vault from an interactive -terminal. Authorization never permits vault-directory deletion. +A private session creates Knapper-owned scratch space. It cannot adopt a caller +vault path. Knapper stores authorization outside the vault. It checks the exact +layout, real path, symlink state, device, and inode before cleanup. -Do not suggest authorization unless the user asked to work in that exact vault. -Never redirect an existing-vault request into scratch space without telling the -user. +An Obsidian registry entry does not authorize access. Only the user can authorize +an existing vault from an interactive terminal. Authorization never permits +vault-directory deletion. -## Tool surface +## Fixed tool surface -The default surface includes `core`, `workspace`, `telemetry`, and `plugin-dev`. -Toolsets are static for the server process. Use `obsidian_toolsets` to inspect the -active set and `obsidian_tool_catalog` to discover optional tools. Restart Knapper -with `KNAP_TOOLSETS` to add `ui`, `editor`, `vault`, `devtools`, or `authoring`. +Knapper publishes the complete tool surface during MCP initialization. The list +does not change during a connection. Do not change the tool list after startup. -## Platform limits +## Safety limits -Isolated workspaces are verified on Linux. Linux uses a private `XDG_RUNTIME_DIR` -for each CLI socket. macOS isolation is unverified. Windows cannot isolate the -Obsidian CLI socket, so Knapper refuses isolated workspace creation. +Private sessions use a private profile and `XDG_RUNTIME_DIR` for the CLI socket. +Restart operations remain scoped to the managed process. Knapper never uses the +default profile as a fallback. ## Related skills -- Use **obsidian-plugin-dev** after the workspace is ready. -- Use **obsidian-ui-automation** when the `ui` toolset is enabled. +- Use **obsidian-plugin-dev** after the session is ready. +- Use **obsidian-ui-automation** for snapshot-first UI work. - Use **obsidian-debugging** for console and network telemetry. diff --git a/skills/obsidian-plugin-dev/SKILL.md b/skills/obsidian-plugin-dev/SKILL.md index 5236d32..068c5b2 100644 --- a/skills/obsidian-plugin-dev/SKILL.md +++ b/skills/obsidian-plugin-dev/SKILL.md @@ -5,12 +5,11 @@ description: Build, link, reload, and verify Obsidian plugins against a live des # Obsidian plugin development loop -Enable `core`, `workspace`, `telemetry`, and `plugin-dev` with -`obsidian_toolsets_update`. To preview the update, first call it with `dryRun: true`. -Then, repeat the call without `dryRun`. The workspace setup below returns a -`workspaceHandle`. Pass this handle to each operational tool. +Knapper publishes the core, session, telemetry, plugin, UI, editor, and vault tools +during MCP initialization. Do not change the tool list after startup. Operational +tools use the active session and accept no caller-owned session identifiers. -For plugin work, create an isolated workspace with `pluginSourceDir` and +For plugin work, open a private session with `pluginSourceDir` and `pluginId`. If `visualIdentity.state` is `degraded`, read `visualIdentity.warnings` array. This warning does not disable the private-session routing. @@ -25,13 +24,12 @@ Obsidian plugin work is a tight loop: The composite tool `obsidian_dev_cycle` runs steps 3–4 in one call after you have built locally. -## One-time workspace setup +## One-time session setup -For a dedicated development workspace: +For a dedicated development session: -1. Call `obsidian_agent_open`. -2. Call `obsidian_workspace_create` with the loadable plugin directory and ID. -3. Check `obsidian_plugin_health` before you modify plugin state. +1. Call `obsidian_session_open` with the loadable plugin directory and ID. +2. Check `obsidian_plugin_health` before you modify plugin state. ### `obsidian_link_plugin` @@ -47,7 +45,7 @@ After linking, enable the plugin once in Obsidian if it is not already enabled ( Call after every code change you want to validate: ```text -obsidian_dev_cycle(workspaceHandle=, pluginId="my-plugin", openPath="Notes/Smoke.md", waitMs=1500) +obsidian_dev_cycle(pluginId="my-plugin", openPath="Notes/Smoke.md", waitMs=1500) ``` What it does: @@ -115,13 +113,13 @@ window.myPluginProbe = async () => ({ settings: this.settings, widgetCount: this Run it through `obsidian_eval`: ```text -obsidian_eval workspaceHandle= code=JSON.stringify(await window.myPluginProbe()) +obsidian_eval code=JSON.stringify(await window.myPluginProbe()) ``` Keep the call on one line. The Playwright transport awaits the promise for you. Also mirror the result to the console as an overflow channel — a large payload then stays readable through `obsidian_logs`: ```text -obsidian_eval workspaceHandle= code=(async () => { const r = await window.myPluginProbe(); console.log("probe:", JSON.stringify(r)); return JSON.stringify(r); })() +obsidian_eval code=(async () => { const r = await window.myPluginProbe(); console.log("probe:", JSON.stringify(r)); return JSON.stringify(r); })() ``` For editor-rendering plugins, pair the probe with the editor toolset: @@ -134,20 +132,17 @@ For editor-rendering plugins, pair the probe with the editor toolset: `obsidian_reset_state` disables the plugin, resets `data.json` to `{}`, re-enables, and returns the previous settings JSON so you can restore them. Destructive — use only on dev vaults. -Do not call `obsidian_create_vault` for an isolated workspace. The tool refuses a -session-bound request. Use the scratch vault from `obsidian_workspace_create`. +Do not call `obsidian_create_vault` for a private session. Use the scratch vault +from `obsidian_session_open`. ## Checklist for a new plugin repo -1. `obsidian_agent_open` — create the attribution handle. -2. `obsidian_workspace_create` — create scratch space and link the loadable build. -3. `obsidian_plugin_health` — confirm present, enabled, and loaded state. -4. `obsidian_plugin_enable` if needed. -5. Iterate: **build → `obsidian_dev_cycle`**. -6. Use `obsidian_exercise_command` for command-centric features. -7. Call `obsidian_workspace_stop` after testing. -8. Call `obsidian_workspace_destroy` to move the scratch root to recoverable trash. -9. Close the agent handle. +1. `obsidian_session_open` — create scratch space and link the loadable build. +2. `obsidian_plugin_health` — confirm present, enabled, and loaded state. +3. `obsidian_plugin_enable` if needed. +4. Iterate: **build → `obsidian_dev_cycle`**. +5. Use `obsidian_exercise_command` for command-centric features. +6. Call `obsidian_session_release` after testing. ## Related skills diff --git a/skills/obsidian-ui-automation/SKILL.md b/skills/obsidian-ui-automation/SKILL.md index 790e727..1f236bb 100644 --- a/skills/obsidian-ui-automation/SKILL.md +++ b/skills/obsidian-ui-automation/SKILL.md @@ -5,8 +5,8 @@ description: Drive the live Obsidian desktop UI with knapper browser tools (Play # Obsidian UI automation (snapshot-first) -UI tools require the optional `ui` toolset and a workspace handle. Pass -`workspaceHandle` to every UI call. Isolated workspaces start with CDP enabled. +UI tools are part of the fixed MCP surface. Use the active session for every UI +call. Private sessions start with CDP enabled. ## Tool split @@ -29,8 +29,8 @@ UI tools require the optional `ui` toolset and a workspace handle. Pass Example flow: ```text -browser_snapshot(workspaceHandle=) -browser_click(workspaceHandle=, target="e5", element="New note") +browser_snapshot() +browser_click(target="e5", element="New note") ``` If you get `STALE_REF`, take a fresh snapshot and pick a new ref. @@ -132,10 +132,10 @@ editor focused, which is what the `focus` argument is for. Testing a hotkey binding: ```text -obsidian_hotkeys workspaceHandle= -obsidian_hotkeys workspaceHandle= commandId=editor:toggle-bold -obsidian_exercise_hotkey workspaceHandle= keys=Control+p -obsidian_exercise_hotkey workspaceHandle= keys=Control+b focus=.cm-content +obsidian_hotkeys +obsidian_hotkeys commandId=editor:toggle-bold +obsidian_exercise_hotkey keys=Control+p +obsidian_exercise_hotkey keys=Control+b focus=.cm-content ``` `obsidian_exercise_hotkey` reports a **verdict**, not just success: it samples the diff --git a/src/agent/store.ts b/src/agent/store.ts deleted file mode 100644 index 0f4e4da..0000000 --- a/src/agent/store.ts +++ /dev/null @@ -1,191 +0,0 @@ -/** - * Explicit agent handles for stateless MCP requests. - * - * MCP clientInfo identifies client software, not a person or a trusted caller. - * These opaque handles provide durable attribution and coordination only. They - * are not authentication credentials. - */ - -import { randomBytes } from "node:crypto"; -import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; -import { join } from "node:path"; -import { agentsDir } from "../config.js"; -import { UobError } from "../util/errors.js"; -import { withFileLock } from "../util/filelock.js"; - -const AGENT_HANDLE = /^agt_[A-Za-z0-9_-]{32}$/; -export const AGENT_IDLE_MS = 24 * 60 * 60 * 1000; - -export interface ObservedClient { - name: string; - version: string; - title?: string; - firstSeenAt: string; - lastSeenAt: string; -} - -export interface AgentRecord { - schema: 1; - handle: string; - label: string; - purpose?: string; - cwd?: string; - createdAt: string; - lastActivityAt: string; - expiresAt: string; - observedClients: ObservedClient[]; -} - -export interface OpenAgentOptions { - label: string; - purpose?: string; - cwd?: string; - now?: Date; - env?: NodeJS.ProcessEnv; -} - -function mintAgentHandle(): string { - return `agt_${randomBytes(24).toString("base64url")}`; -} - -function agentPath(handle: string, env: NodeJS.ProcessEnv): string { - if (!AGENT_HANDLE.test(handle)) { - throw new UobError("INVALID_ARGUMENT", "The agent handle is malformed.", { - remediation: "Create a new handle with obsidian_agent_open.", - fixedBy: "obsidian_agent_open", - }); - } - return join(agentsDir(env), `${handle}.json`); -} - -async function writeAgent(record: AgentRecord, env: NodeJS.ProcessEnv): Promise { - const dir = agentsDir(env); - await mkdir(dir, { recursive: true, mode: 0o700 }); - const path = agentPath(record.handle, env); - const tmp = `${path}.${process.pid}.${randomBytes(6).toString("hex")}.tmp`; - await writeFile(tmp, `${JSON.stringify(record, null, 2)}\n`, { encoding: "utf8", mode: 0o600 }); - await rename(tmp, path); -} - -export async function openAgent(opts: OpenAgentOptions): Promise { - const env = opts.env ?? process.env; - const now = opts.now ?? new Date(); - const handle = mintAgentHandle(); - const record: AgentRecord = { - schema: 1, - handle, - label: opts.label.trim(), - ...(opts.purpose !== undefined ? { purpose: opts.purpose } : {}), - ...(opts.cwd !== undefined ? { cwd: opts.cwd } : {}), - createdAt: now.toISOString(), - lastActivityAt: now.toISOString(), - expiresAt: new Date(now.getTime() + AGENT_IDLE_MS).toISOString(), - observedClients: [], - }; - if (record.label === "") { - throw new UobError("INVALID_ARGUMENT", "The agent label cannot be empty."); - } - await writeAgent(record, env); - return record; -} - -export async function readAgent( - handle: string, - env: NodeJS.ProcessEnv = process.env, -): Promise { - try { - const record = JSON.parse(await readFile(agentPath(handle, env), "utf8")) as AgentRecord; - return record.schema === 1 && record.handle === handle ? record : undefined; - } catch (error) { - if (error instanceof UobError) throw error; - return undefined; - } -} - -export async function requireAgent( - handle: string, - env: NodeJS.ProcessEnv = process.env, -): Promise { - const record = await readAgent(handle, env); - if (record === undefined) { - throw new UobError("INVALID_ARGUMENT", `Agent handle ${handle} does not exist or expired.`, { - remediation: "Create a new handle with obsidian_agent_open.", - fixedBy: "obsidian_agent_open", - }); - } - const expiresAt = Date.parse(record.expiresAt); - if (!Number.isFinite(expiresAt) || expiresAt <= Date.now()) { - throw new UobError("INVALID_ARGUMENT", `Agent handle ${handle} expired.`, { - remediation: "Create a new handle with obsidian_agent_open.", - fixedBy: "obsidian_agent_open", - }); - } - return record; -} - -export async function touchAgent( - handle: string, - client?: { name: string; version: string; title?: string }, - env: NodeJS.ProcessEnv = process.env, -): Promise { - return withFileLock(`${agentPath(handle, env)}.lock`, async () => { - const record = await requireAgent(handle, env); - const now = new Date(); - const observedClients = [...record.observedClients]; - if (client !== undefined) { - const existing = observedClients.find( - (candidate) => candidate.name === client.name && candidate.version === client.version, - ); - if (existing !== undefined) { - existing.lastSeenAt = now.toISOString(); - if (client.title !== undefined) existing.title = client.title; - } else { - observedClients.push({ - ...client, - firstSeenAt: now.toISOString(), - lastSeenAt: now.toISOString(), - }); - } - } - const next: AgentRecord = { - ...record, - lastActivityAt: now.toISOString(), - expiresAt: new Date(now.getTime() + AGENT_IDLE_MS).toISOString(), - observedClients, - }; - await writeAgent(next, env); - return next; - }); -} - -export async function listAgents(env: NodeJS.ProcessEnv = process.env): Promise { - let names: string[]; - try { - names = await readdir(agentsDir(env)); - } catch { - return []; - } - const records = await Promise.all( - names - .filter((name) => name.endsWith(".json")) - .map((name) => readAgent(name.slice(0, -5), env).catch(() => undefined)), - ); - return records - .filter((record): record is AgentRecord => record !== undefined) - .sort((a, b) => a.createdAt.localeCompare(b.createdAt)); -} - -export async function closeAgent( - handle: string, - env: NodeJS.ProcessEnv = process.env, -): Promise { - const record = await readAgent(handle, env); - if (record === undefined) { - throw new UobError("INVALID_ARGUMENT", `Agent handle ${handle} does not exist.`, { - remediation: "Create a new handle with obsidian_agent_open.", - fixedBy: "obsidian_agent_open", - }); - } - await rm(agentPath(handle, env), { force: true }); - return true; -} diff --git a/src/audit/event.ts b/src/audit/event.ts index 6463b00..52479f6 100644 --- a/src/audit/event.ts +++ b/src/audit/event.ts @@ -45,12 +45,6 @@ function safeContext(context: AuditCallContext | undefined): AuditCallContext { ...(clientName ? { clientInfo: { name: clientName, ...(clientVersion ? { version: clientVersion } : {}) } } : {}), - ...(opaqueIdentifier(context.agentHandle) - ? { agentHandle: opaqueIdentifier(context.agentHandle) } - : {}), - ...(opaqueIdentifier(context.workspaceHandle) - ? { workspaceHandle: opaqueIdentifier(context.workspaceHandle) } - : {}), ...(safeLabel(context.transport) ? { transport: safeLabel(context.transport) } : {}), ...(safeLabel(context.protocolVersion) ? { protocolVersion: safeLabel(context.protocolVersion) } @@ -114,8 +108,6 @@ export function toolAuditEvent(input: { arguments: argumentMetadata(input.args), ...(input.error ? { error: input.error } : {}), ...(context.clientInfo ? { client: context.clientInfo } : {}), - ...(context.agentHandle ? { agent_handle: context.agentHandle } : {}), - ...(context.workspaceHandle ? { workspace_handle: context.workspaceHandle } : {}), ...(context.transport ? { transport: context.transport } : {}), ...(context.protocolVersion ? { protocol_version: context.protocolVersion } : {}), ...(context.workspaceKind ? { workspace_kind: context.workspaceKind } : {}), diff --git a/src/audit/types.ts b/src/audit/types.ts index 932fe2c..e860377 100644 --- a/src/audit/types.ts +++ b/src/audit/types.ts @@ -5,11 +5,9 @@ export interface AuditClientInfo { version?: string; } -/** Optional request data supplied by the server and workspace layers. */ +/** Optional request data supplied by the server and active-target layer. */ export interface AuditCallContext { clientInfo?: AuditClientInfo; - agentHandle?: string; - workspaceHandle?: string; transport?: string; protocolVersion?: string; traceId?: string; @@ -57,8 +55,6 @@ export interface ToolAuditEvent { arguments: AuditArgumentMetadata; error?: AuditErrorEnvelope; client?: AuditClientInfo; - agent_handle?: string; - workspace_handle?: string; transport?: string; protocol_version?: string; workspace_kind?: string; @@ -79,4 +75,10 @@ export interface ToolRegistryHooks { args: Record, requestContext?: ToolRequestContext, ) => AuditCallContext | undefined | Promise; + afterInvoke?: ( + definition: ToolDefinition, + args: Record, + requestContext: ToolRequestContext | undefined, + outcome: unknown, + ) => void | Promise; } diff --git a/src/cli.ts b/src/cli.ts index bda5cc8..8f51b23 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -174,8 +174,7 @@ const shutdown = async (reason: string): Promise => { await ctx.browserProxy.close().catch(() => undefined); await ctx.router.dispose().catch(() => undefined); ctx.telemetry.closePersistence(); - await ctx.workspaceLeases.releaseAll().catch(() => undefined); - await ctx.profileLease.release().catch(() => undefined); + await ctx.activity.release().catch(() => undefined); process.exit(0); }; diff --git a/src/config.ts b/src/config.ts index 6240520..08d35d0 100644 --- a/src/config.ts +++ b/src/config.ts @@ -42,11 +42,6 @@ export interface Config { httpPort: number; /** Listen host for the http transport. Non-loopback values are warned about. */ httpHost: string; - /** - * How many tool calls may run at once. UI mutations are serialized regardless; - * this caps the read-only calls that are safe to overlap. - */ - maxConcurrency: number; enabledToolsets: Set; unknownToolsets: string[]; logLevel: LogLevel; @@ -62,8 +57,10 @@ export interface Config { outputDir: string; /** Timeout for a single Obsidian CLI invocation, in ms. */ cliTimeoutMs: number; - /** Idle grace for default-profile ownership and disconnected sessions. */ + /** Idle grace before inactive managed sessions are eligible for cleanup. */ idleTimeoutMs: number; + /** Recent-activity window that prevents a second agent from taking the single target. */ + activityIdleMs: number; /** Transport preference for renderer commands that both CLI and CDP can serve. */ commandTransport: CommandTransport; /** Session key when this server is bound to one, else undefined. */ @@ -106,7 +103,6 @@ export interface ConfigOverrides { transport?: string; httpPort?: number; httpHost?: string; - maxConcurrency?: number; toolsets?: string; logLevel?: string; telemetryBuffer?: number; @@ -115,6 +111,7 @@ export interface ConfigOverrides { outputDir?: string; cliTimeoutMs?: number; commandTransport?: string; + activityIdleMs?: number; sessionId?: string; userDataDir?: string; runtimeDir?: string; @@ -181,16 +178,6 @@ export function sessionsDir(env: NodeJS.ProcessEnv = process.env): string { return join(knapperHome(env), "sessions"); } -/** Durable explicit agent handles used by stateless MCP clients. */ -export function agentsDir(env: NodeJS.ProcessEnv = process.env): string { - return join(knapperHome(env), "agents"); -} - -/** Explicit workspace-handle records. The records never contain vault content. */ -export function workspacesDir(env: NodeJS.ProcessEnv = process.env): string { - return join(knapperHome(env), "workspaces"); -} - /** Recoverable session roots awaiting an explicit purge. */ export function trashDir(env: NodeJS.ProcessEnv = process.env): string { return join(knapperHome(env), "trash"); @@ -205,16 +192,6 @@ export function registryLockPath(env: NodeJS.ProcessEnv = process.env): string { return join(knapperHome(env), "registry.lock"); } -/** Short coordination lock for atomic default-profile lease updates. */ -export function defaultProfileLeaseLockPath(env: NodeJS.ProcessEnv = process.env): string { - return join(knapperHome(env), "default-profile-lease.lock"); -} - -/** Ownership record for the installation's default Obsidian profile. */ -export function defaultProfileLeasePath(env: NodeJS.ProcessEnv = process.env): string { - return join(knapperHome(env), "default-profile-lease.json"); -} - /** Per-session directory layout. The only place these names are spelled. */ export interface SessionPaths { root: string; @@ -353,10 +330,6 @@ export function loadConfig(overrides: ConfigOverrides = {}, env = process.env): transport, httpPort: overrides.httpPort ?? numberFrom(env.MCP_PORT, 9223), httpHost: overrides.httpHost ?? env.MCP_HOST ?? "127.0.0.1", - maxConcurrency: Math.max( - 1, - overrides.maxConcurrency ?? numberFrom(env.KNAP_MAX_CONCURRENCY, 4), - ), enabledToolsets: enabled, unknownToolsets: unknown, logLevel, @@ -371,6 +344,10 @@ export function loadConfig(overrides: ConfigOverrides = {}, env = process.env): join(process.cwd(), ".knapper"), cliTimeoutMs: overrides.cliTimeoutMs ?? numberFrom(env.KNAP_CLI_TIMEOUT_MS, 15_000), idleTimeoutMs: Math.max(30_000, numberFrom(env.KNAP_IDLE_TIMEOUT_MS, 24 * 60 * 60_000)), + activityIdleMs: Math.max( + 30_000, + overrides.activityIdleMs ?? numberFrom(env.KNAP_ACTIVITY_IDLE_MS, 5 * 60_000), + ), commandTransport: commandTransportFrom( overrides.commandTransport ?? env.KNAP_COMMAND_TRANSPORT, ), diff --git a/src/server.ts b/src/server.ts index b83e82f..6e75bcd 100644 --- a/src/server.ts +++ b/src/server.ts @@ -17,7 +17,7 @@ import { createLogger, type Logger } from "./util/logger.js"; import { TOOLSET_DESCRIPTIONS } from "./toolsets.js"; import { registerCoreTools } from "./tools/core.js"; import { registerProvisioningTools } from "./tools/provisioning.js"; -import { registerWorkspaceTools } from "./tools/workspace.js"; +import { registerSessionTools } from "./tools/session.js"; import { registerObsidianTools } from "./tools/obsidian.js"; import { registerEditorTools } from "./tools/editor.js"; import { registerVaultTools } from "./tools/vault.js"; @@ -33,18 +33,20 @@ import { BrowserProxy } from "./browser/proxy.js"; import { readFile } from "node:fs/promises"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; -import { DefaultProfileLease } from "./session/default-profile-lease.js"; -import { patchDescriptor, readDescriptor, type SessionDescriptor } from "./session/descriptor.js"; +import { + listDescriptors, + patchDescriptor, + readDescriptor, + type SessionDescriptor, +} from "./session/descriptor.js"; import { readPidStartTime } from "./connection/health.js"; import { reapStaleSessions } from "./session/reap.js"; -import { sessionOwnerAlive, waitSession } from "./session/registry.js"; -import { requireWorkspace, touchWorkspace } from "./workspace/store.js"; -import { touchAgent } from "./agent/store.js"; +import { sessionState, waitSession } from "./session/registry.js"; import type { ToolRequestContext } from "./audit/types.js"; import { UobError } from "./util/errors.js"; -import { WorkspaceLeaseManager } from "./workspace/lease.js"; import { recoverVaultTransaction } from "./connection/vault-transaction.js"; import { vaultAuthorizationRegistryPath } from "./connection/vaults.js"; +import { ActivityGuard } from "./usage/activity-guard.js"; export interface ServerContext { config: Config; @@ -54,15 +56,15 @@ export interface ServerContext { capture: TelemetryCapture; browserProxy: BrowserProxy; registry: ToolRegistry; - profileLease: DefaultProfileLease; - workspaceLeases: WorkspaceLeaseManager; - currentWorkspaceHandle?: string; + activity: ActivityGuard; + currentSessionKey?: string; + targetKind?: "isolated" | "default"; clientInfo(): { name: string; version: string; title?: string } | undefined; protocolVersion(): string | undefined; - bindSession(descriptor: SessionDescriptor, workspaceHandle: string): Promise; - bindDefaultWorkspace(): Promise; - selectTelemetry(scope: "default" | string): void; - archiveTelemetry(scope: string, destinationRoot: string): Promise; + bindSession(descriptor: SessionDescriptor): Promise; + bindDefault(): Promise; + selectTelemetry(scope: "default" | "session"): void; + archiveTelemetry(scope: "session", destinationRoot: string): Promise; stopJanitor(): void; } @@ -143,13 +145,13 @@ USE THIS SERVER WHEN the task involves: DO NOT USE IT FOR: general web browsing or automating other websites (the browser_* tools here are bound to the Obsidian window), editing this project's own source files, or reading Markdown that merely happens to live outside a vault — ordinary file tools are better for that. -GETTING STARTED: call obsidian_agent_open. Then call obsidian_workspace_create for an isolated scratch workspace, or obsidian_workspace_claim_default only when the user explicitly wants their own Obsidian profile. Pass the returned workspaceHandle to every operational tool. Handles are durable coordination identifiers, not authentication credentials. +GETTING STARTED: call obsidian_session_open for an isolated scratch session. Use target="default" only when the user explicitly wants their own Obsidian profile. Operational tools use the active session automatically and never require a handle. -CONCURRENCY: each agent can own several isolated workspaces. Calls are routed by workspaceHandle, so do not infer the target from MCP transport state or clientInfo. Only one MCP server can drive the default profile at a time. Use an isolated workspace when the default profile is busy. +CONCURRENCY: Knapper controls one Obsidian target and runs one operation at a time. obsidian_status reports whether another Knapper process used the target recently. -SAFETY: isolated workspaces always use Knapper-owned scratch vaults. Stop an isolated instance with obsidian_workspace_stop, then use obsidian_workspace_destroy to move its verified root to recoverable Knapper trash. Destroy refuses a live instance and never deletes a user vault. Existing vault access needs an external authorization that the user creates from a terminal. Knapper never treats an Obsidian registry entry or a file inside a vault as deletion authority. +SAFETY: isolated sessions always use Knapper-owned scratch vaults. obsidian_session_reset stops the managed instance and moves its verified root to recoverable Knapper trash. Cleanup never deletes a user vault. Existing vault access needs an external authorization that the user creates from a terminal. Knapper never treats an Obsidian registry entry or a file inside a vault as deletion authority. -TASK INDEX: create or select a target with obsidian_workspace_create or obsidian_workspace_claim_default; diagnose setup with obsidian_doctor; inspect transports with obsidian_capabilities; inspect the current dynamic surface with obsidian_toolsets; enable optional groups with obsidian_toolsets_update; discover tools with obsidian_tool_catalog; reload a plugin with obsidian_dev_cycle; inspect UI with obsidian_snapshot; read new errors with obsidian_logs. +TASK INDEX: select a target with obsidian_session_open; diagnose setup with obsidian_doctor; inspect transports with obsidian_capabilities; reload a plugin with obsidian_dev_cycle; inspect UI with obsidian_snapshot; read new errors with obsidian_logs. CONVENTIONS: use obsidian_* tools for app, vault, and plugin state; browser_* tools for real input. Browser tools are snapshot-first — call browser_snapshot (or the cheaper obsidian_snapshot), then pass a returned ref as "target"; a CSS selector also works. Prefer obsidian_command over clicking through menus. Read console output with obsidian_logs, passing the previous call's cursor as "since" to see only what is new.`; @@ -186,105 +188,91 @@ export async function createServerContext(config: Config): Promise config.sessionId !== undefined, - { - beforeInvoke: async (definition, args, requestContext) => { - const handle = args.workspaceHandle; - if ( - typeof handle === "string" && - handle !== "" && - (definition.workspaceIndependent !== true || definition.requiresWorkspaceLease === true) - ) { - await workspaceLeases.acquire(handle, definition.name); - } - if (definition.workspaceIndependent === true) return; - if (typeof handle !== "string" || handle === "") { - throw new UobError("INVALID_ARGUMENT", "A workspaceHandle is required for this tool.", { - remediation: - "Open an agent handle, then create an isolated workspace or claim the default profile.", - fixedBy: "obsidian_workspace_create", + const managedSessionOpen = async (): Promise => { + const descriptors = await listDescriptors(); + const states = await Promise.all(descriptors.map((descriptor) => sessionState(descriptor))); + return states.includes("live"); + }; + const statusOnlyTools = new Set([ + "obsidian_status", + "obsidian_session_status", + "obsidian_capabilities", + "obsidian_toolsets", + "obsidian_tool_catalog", + ]); + const registry = new ToolRegistry(config.enabledToolsets, logger, telemetry, { + beforeInvoke: async (definition) => { + if (!statusOnlyTools.has(definition.name)) { + await activity.acquire({ + operation: definition.name, + sessionOpen: await managedSessionOpen(), + }); + } + if (definition.targetIndependent === true) return; + if (ctx.targetKind === undefined) { + throw new UobError("SESSION_NOT_FOUND", "No Obsidian session is active.", { + remediation: "Open an isolated session before you use operational tools.", + fixedBy: "obsidian_session_open", + }); + } + if (ctx.targetKind === "isolated") { + if (ctx.currentSessionKey === undefined) { + throw new UobError("SESSION_NOT_FOUND", "The active session has no descriptor.", { + remediation: "Open a new isolated session.", + fixedBy: "obsidian_session_open", }); } - const workspace = await touchWorkspace(handle); - await touchAgent( - workspace.agentHandle, - observedClient(requestContext) ?? - (config.transport === "stdio" ? ctx.clientInfo() : undefined), - ); - if (ctx.currentWorkspaceHandle === handle) { - telemetry.select(workspace.kind === "default" ? "default" : workspace.handle); - return; + let descriptor = await readDescriptor(ctx.currentSessionKey); + if (descriptor === undefined) { + throw new UobError( + "SESSION_NOT_FOUND", + `Session ${ctx.currentSessionKey} no longer has a descriptor.`, + { remediation: "Open a new isolated session.", fixedBy: "obsidian_session_open" }, + ); } - - if (workspace.kind === "default") { - await ctx.bindDefaultWorkspace(); - } else { - if (workspace.sessionKey === undefined) { - throw new UobError("SESSION_NOT_FOUND", `Workspace ${handle} has no private session.`); - } - let descriptor = await readDescriptor(workspace.sessionKey); - if (descriptor === undefined) { - throw new UobError( - "SESSION_NOT_FOUND", - `Workspace ${handle} no longer has a session descriptor.`, - { remediation: "Create a new isolated workspace." }, - ); - } - if (descriptor.readiness.phase === "starting") { - descriptor = await waitSession(descriptor.key); - } - await ctx.bindSession(descriptor, handle); + if (descriptor.readiness.phase === "starting") { + descriptor = await waitSession(descriptor.key); } - ctx.currentWorkspaceHandle = handle; - telemetry.select(workspace.kind === "default" ? "default" : workspace.handle); - }, - contextProvider: async (args, requestContext) => { - const workspaceHandle = - typeof args.workspaceHandle === "string" ? args.workspaceHandle : undefined; - const workspace = - workspaceHandle !== undefined - ? await requireWorkspace(workspaceHandle).catch(() => undefined) + if (config.sessionId !== descriptor.key) await ctx.bindSession(descriptor); + telemetry.select("session"); + } else { + telemetry.select("default"); + } + }, + contextProvider: async (_args, requestContext) => { + const clientInfo = + observedClient(requestContext) ?? + (config.transport === "stdio" ? ctx.clientInfo() : undefined); + const rawProtocolVersion = + requestContext?.protocolVersion ?? requestContext?.mcpReq?.envelope?.protocolVersion; + const protocolVersion = + typeof rawProtocolVersion === "string" + ? rawProtocolVersion + : config.transport === "stdio" + ? ctx.protocolVersion() : undefined; - const clientInfo = - observedClient(requestContext) ?? - (config.transport === "stdio" ? ctx.clientInfo() : undefined); - const rawProtocolVersion = - requestContext?.protocolVersion ?? requestContext?.mcpReq?.envelope?.protocolVersion; - const protocolVersion = - typeof rawProtocolVersion === "string" - ? rawProtocolVersion - : config.transport === "stdio" - ? ctx.protocolVersion() - : undefined; - return { - ...(clientInfo !== undefined ? { clientInfo } : {}), - ...(workspace !== undefined ? { agentHandle: workspace.agentHandle } : {}), - ...(workspaceHandle !== undefined ? { workspaceHandle } : {}), - transport: config.transport, - ...(protocolVersion !== undefined ? { protocolVersion } : {}), - ...(requestContext?.requestId !== undefined || requestContext?.mcpReq?.id !== undefined - ? { - traceId: String(requestContext.requestId ?? requestContext?.mcpReq?.id), - } - : {}), - ...(workspace !== undefined ? { workspaceKind: workspace.kind } : {}), - }; - }, + return { + ...(clientInfo !== undefined ? { clientInfo } : {}), + transport: config.transport, + ...(protocolVersion !== undefined ? { protocolVersion } : {}), + ...(requestContext?.requestId !== undefined || requestContext?.mcpReq?.id !== undefined + ? { + traceId: String(requestContext.requestId ?? requestContext?.mcpReq?.id), + } + : {}), + ...(ctx.targetKind !== undefined ? { workspaceKind: ctx.targetKind } : {}), + }; }, - ); + afterInvoke: async (definition, _args, _requestContext, outcome) => { + if (statusOnlyTools.has(definition.name)) return; + const releaseSucceeded = + definition.name === "obsidian_session_release" && !(outcome instanceof UobError); + await activity.complete(releaseSucceeded ? false : await managedSessionOpen()); + if (releaseSucceeded) await activity.release(); + }, + }); let janitorTimer: NodeJS.Timeout | undefined; const runJanitor = (): void => { @@ -297,7 +285,7 @@ export async function createServerContext(config: Config): Promise undefined, protocolVersion: () => undefined, - bindSession: async (descriptor, workspaceHandle) => { - const lease = await workspaceLeases.status(workspaceHandle); - if (lease.state !== "owned") { - throw new UobError( - "WORKSPACE_BUSY", - `Workspace ${workspaceHandle} is not leased by this server.`, - { - remediation: "Retry the workspace operation through the server that owns its lease.", - details: { workspaceHandle, lease }, - }, - ); - } - if ( - descriptor.owner !== undefined && - descriptor.owner.pid !== process.pid && - (await sessionOwnerAlive(descriptor)) - ) { - throw new UobError( - "WORKSPACE_BUSY", - `Session ${descriptor.key} is owned by another server.`, - { - remediation: - "Use the workspace through its current server, or wait for that server to exit.", - details: { session: descriptor.key, ownerPid: descriptor.owner.pid }, - }, - ); - } + bindSession: async (descriptor) => { await browserProxy.close(); capture.reset(); applySessionConfig(config, descriptor); @@ -359,13 +321,16 @@ export async function createServerContext(config: Config): Promise { + bindDefault: async () => { await browserProxy.close(); capture.reset(); restoreConfig(config, baseConfig); await router.rebind(); + ctx.currentSessionKey = undefined; + ctx.targetKind = "default"; }, selectTelemetry: (scope) => telemetry.select(scope), archiveTelemetry: (scope, destinationRoot) => telemetry.archive(scope, destinationRoot), @@ -381,7 +346,7 @@ export async function createServerContext(config: Config): Promise; - retryAfterMs?: number; -} - -export interface DefaultProfileLeaseOptions { - idleTimeoutMs: number; - env?: NodeJS.ProcessEnv; - now?: () => Date; - pid?: number; - cwd?: string; - hostname?: string; -} - -function processAlive(pid: number): boolean { - try { - process.kill(pid, 0); - return true; - } catch (error) { - return (error as NodeJS.ErrnoException).code === "EPERM"; - } -} - -function isLeaseRecord(value: unknown): value is DefaultProfileLeaseRecord { - if (value === null || typeof value !== "object") return false; - const record = value as Partial; - return ( - typeof record.token === "string" && - Number.isInteger(record.pid) && - Number(record.pid) > 0 && - (record.pidStartTime === undefined || Number.isFinite(record.pidStartTime)) && - typeof record.hostname === "string" && - typeof record.cwd === "string" && - Number.isFinite(Date.parse(record.acquiredAt ?? "")) && - Number.isFinite(Date.parse(record.lastActivityAt ?? "")) && - Number.isFinite(Date.parse(record.expiresAt ?? "")) && - Number.isInteger(record.activeCalls) && - Number(record.activeCalls) >= 0 - ); -} - -export class DefaultProfileLease { - private readonly token = randomUUID(); - private readonly env: NodeJS.ProcessEnv; - private readonly now: () => Date; - private readonly pid: number; - private readonly cwd: string; - private readonly host: string; - - constructor(private readonly opts: DefaultProfileLeaseOptions) { - this.env = opts.env ?? process.env; - this.now = opts.now ?? (() => new Date()); - this.pid = opts.pid ?? process.pid; - this.cwd = opts.cwd ?? process.cwd(); - this.host = opts.hostname ?? hostname(); - } - - private async read(): Promise { - try { - const parsed: unknown = JSON.parse(await readFile(defaultProfileLeasePath(this.env), "utf8")); - if (isLeaseRecord(parsed)) return parsed; - throw new UobError("DEFAULT_PROFILE_BUSY", "The default-profile lease record is malformed.", { - remediation: - "Inspect the lease record and its owner before you remove it. Knapper will not overwrite an invalid ownership record.", - }); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return undefined; - if (error instanceof UobError) throw error; - throw new UobError( - "DEFAULT_PROFILE_BUSY", - "Knapper could not read the default-profile lease.", - { - remediation: - "Check the lease file permissions and owner before you retry. Knapper will not assume that the profile is free.", - cause: error, - }, - ); - } - } - - private async write(record: DefaultProfileLeaseRecord): Promise { - const path = defaultProfileLeasePath(this.env); - await writeFileAtomic(path, `${JSON.stringify(record, null, 2)}\n`, { - mode: 0o600, - directoryMode: 0o700, - }); - } - - private async ownerAlive(record: DefaultProfileLeaseRecord): Promise { - if (record.hostname !== this.host) return true; - if (!processAlive(record.pid)) return false; - if (record.pidStartTime === undefined) return true; - return (await readPidStartTime(record.pid)) === record.pidStartTime; - } - - private withoutToken( - record: DefaultProfileLeaseRecord, - ): Omit { - const { token: _token, ...owner } = record; - return owner; - } - - async status(): Promise { - const record = await this.read(); - if (record === undefined) return { state: "free" }; - const retryAfterMs = Math.max(0, Date.parse(record.expiresAt) - this.now().getTime()); - const timedOut = - retryAfterMs === 0 && (record.activeCalls === 0 || record.hostname !== this.host); - if (!(await this.ownerAlive(record)) || timedOut) { - return { state: "expired", owner: this.withoutToken(record), retryAfterMs: 0 }; - } - if (record.token === this.token) return { state: "owned", owner: this.withoutToken(record) }; - return { state: "busy", owner: this.withoutToken(record), retryAfterMs }; - } - - private async acquire(tool: string): Promise { - await withFileLock(defaultProfileLeaseLockPath(this.env), async () => { - const now = this.now(); - const existing = await this.read(); - const own = existing?.token === this.token; - const timedOut = - existing !== undefined && - Date.parse(existing.expiresAt) <= now.getTime() && - (existing.activeCalls === 0 || existing.hostname !== this.host); - const expired = existing === undefined || !(await this.ownerAlive(existing)) || timedOut; - - if (!own && !expired && existing !== undefined) { - const retryAfterMs = Math.max(0, Date.parse(existing.expiresAt) - now.getTime()); - throw new UobError( - "DEFAULT_PROFILE_BUSY", - "Another Knapper MCP server owns the default Obsidian profile.", - { - remediation: - "Create an isolated workspace with obsidian_workspace_create, then retry the original tool with its workspaceHandle.", - fixedBy: "obsidian_workspace_create", - details: { - tool, - owner: this.withoutToken(existing), - retryAfterMs, - }, - }, - ); - } - - const acquiredAt = own && existing !== undefined ? existing.acquiredAt : now.toISOString(); - await this.write({ - token: this.token, - pid: this.pid, - ...(process.platform === "linux" ? { pidStartTime: await readPidStartTime(this.pid) } : {}), - hostname: this.host, - cwd: this.cwd, - acquiredAt, - lastActivityAt: now.toISOString(), - expiresAt: new Date(now.getTime() + this.opts.idleTimeoutMs).toISOString(), - activeCalls: (own ? (existing?.activeCalls ?? 0) : 0) + 1, - }); - }); - } - - private async touch(activeDelta = 0): Promise { - await withFileLock(defaultProfileLeaseLockPath(this.env), async () => { - const existing = await this.read(); - if (existing?.token !== this.token) return; - const now = this.now(); - await this.write({ - ...existing, - lastActivityAt: now.toISOString(), - expiresAt: new Date(now.getTime() + this.opts.idleTimeoutMs).toISOString(), - activeCalls: Math.max(0, existing.activeCalls + activeDelta), - }); - }); - } - - async run(tool: string, fn: () => Promise): Promise { - await this.acquire(tool); - const heartbeat = setInterval( - () => void this.touch().catch(() => undefined), - Math.min(30_000, Math.max(5_000, Math.floor(this.opts.idleTimeoutMs / 3))), - ); - heartbeat.unref(); - let value: T | undefined; - let callError: unknown; - let callFailed = false; - try { - value = await fn(); - } catch (error) { - callFailed = true; - callError = error; - } - clearInterval(heartbeat); - let releaseError: unknown; - let releaseFailed = false; - try { - await this.touch(-1); - } catch (error) { - releaseFailed = true; - releaseError = error; - } - if (callFailed && releaseFailed) { - throw new AggregateError( - [callError, releaseError], - "The tool call and default-profile lease cleanup both failed.", - ); - } - if (callFailed) throw callError; - if (releaseFailed) throw releaseError; - return value as T; - } - - async release(): Promise { - await withFileLock(defaultProfileLeaseLockPath(this.env), async () => { - const existing = await this.read(); - if (existing?.token !== this.token) return; - if (existing.activeCalls > 0) { - throw new UobError( - "DEFAULT_PROFILE_BUSY", - "The default Obsidian profile still has active Knapper calls.", - { remediation: "Wait for the in-flight calls to finish, then retry." }, - ); - } - await rm(defaultProfileLeasePath(this.env), { force: true }); - }); - } -} diff --git a/src/session/descriptor.ts b/src/session/descriptor.ts index faba9d8..ec3e887 100644 --- a/src/session/descriptor.ts +++ b/src/session/descriptor.ts @@ -38,8 +38,6 @@ export interface SessionDescriptor { readiness: SessionReadiness; /** Provenance for diagnostics. Never used for routing. */ origin: { cwd: string; branch?: string; label?: string }; - /** Explicit stateless-MCP attribution. The session key remains an internal id. */ - agentHandle?: string; /** * Filesystem identity recorded at creation. Cleanup must match every field and * the derived session path before it can quarantine the scratch directory. diff --git a/src/session/plugin-link.ts b/src/session/plugin-link.ts index cfbabdc..d9b2348 100644 --- a/src/session/plugin-link.ts +++ b/src/session/plugin-link.ts @@ -34,7 +34,7 @@ export async function validatePluginDir( id?: string; }; } catch { - throw new UobError("INVALID_ARGUMENT", `No manifest.json in ${sourceDir}.`, { + throw new UobError("PLUGIN_ARTIFACT_INVALID", `No manifest.json in ${sourceDir}.`, { remediation: "Point sourceDir at a loadable plugin directory containing manifest.json and main.js.", details: { sourceDir }, @@ -43,25 +43,33 @@ export async function validatePluginDir( const id = manifest.id; if (id === undefined || id === "") { - throw new UobError("INVALID_ARGUMENT", "manifest.json has no id field.", { + throw new UobError("PLUGIN_ARTIFACT_INVALID", "manifest.json has no id field.", { + remediation: "Set a non-empty id in manifest.json.", details: { sourceDir }, }); } if (override !== undefined && override !== "" && override !== id) { throw new UobError( - "INVALID_ARGUMENT", + "PLUGIN_ARTIFACT_INVALID", `Plugin id "${override}" does not match manifest id "${id}".`, - { details: { sourceDir, pluginId: override, manifestId: id } }, + { + remediation: "Make pluginId match the manifest.json id, or omit pluginId.", + details: { sourceDir, pluginId: override, manifestId: id }, + }, ); } const mainPath = join(sourceDir, "main.js"); const main = await stat(mainPath).catch(() => undefined); if (main?.isFile() !== true) { - throw new UobError("INVALID_ARGUMENT", `Plugin directory ${sourceDir} has no main.js file.`, { - remediation: "Build the plugin and point sourceDir at its loadable artifact directory.", - details: { sourceDir, missing: ["main.js"] }, - }); + throw new UobError( + "PLUGIN_ARTIFACT_INVALID", + `Plugin directory ${sourceDir} has no main.js file.`, + { + remediation: "Build the plugin and point sourceDir at its loadable artifact directory.", + details: { sourceDir, missing: ["main.js"] }, + }, + ); } const styles = await stat(join(sourceDir, "styles.css")).catch(() => undefined); return { diff --git a/src/session/registry.ts b/src/session/registry.ts index 5796c32..b8cedcb 100644 --- a/src/session/registry.ts +++ b/src/session/registry.ts @@ -52,7 +52,6 @@ import { export interface CreateSessionOptions { label?: string; - agentHandle?: string; pluginSourceDir?: string; pluginId?: string; cdpPort?: number; @@ -82,7 +81,7 @@ interface ReadinessContext { identityRemediation: string; pluginFailure: (pluginId: string) => string; pluginRemediation: string; - fixedBy: "obsidian_workspace_create" | "obsidian_workspace_restart"; + fixedBy: "obsidian_session_open" | "obsidian_session_reset"; degradedWarning: string; onPluginUpdate?: (plugin: NonNullable) => void; } @@ -171,11 +170,11 @@ async function verifyRestartReadiness( ...(descriptor.plugin !== undefined ? { plugin: descriptor.plugin } : {}), identityFailure: `Session ${descriptor.key} could not prove its private-profile visual identity after restart.`, identityRemediation: - "Review the launch logs and desktop integration, then restart the workspace.", + "Review the launch logs and desktop integration, then reset the session.", pluginFailure: (pluginId) => `Plugin "${pluginId}" did not become installed, enabled, and loaded after restart.`, - pluginRemediation: "Review the plugin manifest and launch logs, then restart the workspace.", - fixedBy: "obsidian_workspace_restart", + pluginRemediation: "Review the plugin manifest and launch logs, then reset the session.", + fixedBy: "obsidian_session_reset", degradedWarning: "private session visual identity is degraded after restart", }, logger, @@ -268,7 +267,6 @@ async function createSessionUnlocked(opts: CreateSessionOptions): Promise `Plugin "${pluginId}" did not become installed, enabled, and loaded.`, pluginRemediation: - "Review the plugin manifest and launch logs, then create a new workspace after fixing the plugin.", - fixedBy: "obsidian_workspace_create", + "Review the plugin manifest and launch logs, then reset the session after fixing the plugin.", + fixedBy: "obsidian_session_reset", degradedWarning: "private session visual identity is degraded", onPluginUpdate: (updated) => { plugin = updated; @@ -366,8 +364,8 @@ async function createSessionUnlocked(opts: CreateSessionOptions): Promise { if ((await findObsidianPids(scopeOf(descriptor))).length === 0) return; throw new UobError("INVALID_ARGUMENT", `Session ${descriptor.key} is still running.`, { - remediation: "Stop the workspace, then retry this operation.", - fixedBy: "obsidian_workspace_stop", + remediation: "Reset the managed session, then retry this operation.", + fixedBy: "obsidian_session_reset", details: { session: descriptor.key, userDataDir: descriptor.instance.userDataDir, @@ -1025,8 +1023,8 @@ async function requireDescriptor(key: string, env: NodeJS.ProcessEnv): Promise(); @@ -30,19 +30,19 @@ export class WorkspaceTelemetryStore extends TelemetryStore { this.active = this.storeFor("default"); } - select(scope: "default" | string): void { - if (scope !== "default" && !WORKSPACE_HANDLE.test(scope)) { - throw new Error("Invalid telemetry workspace scope."); + select(scope: "default" | "session"): void { + if (scope !== "default" && scope !== SESSION_SCOPE) { + throw new Error("Invalid telemetry session scope."); } this.active = this.storeFor(scope); this.activeScope = scope; } - /** Move a closed workspace's telemetry into its retained or quarantined root. */ - async archive(scope: string, destinationRoot: string): Promise { - if (!WORKSPACE_HANDLE.test(scope)) throw new Error("Invalid telemetry workspace scope."); + /** Move the closed session's telemetry into its retained or quarantined root. */ + async archive(scope: "session", destinationRoot: string): Promise { + if (scope !== SESSION_SCOPE) throw new Error("Invalid telemetry session scope."); if (this.activeScope === scope) { - throw new Error("Cannot archive telemetry while its workspace is active."); + throw new Error("Cannot archive telemetry while the managed session is active."); } const source = join(this.telemetryDir, `${scope}.jsonl`); const destinationDir = join(destinationRoot, "telemetry"); diff --git a/src/tools/core.ts b/src/tools/core.ts index e6b48f7..96a6f08 100644 --- a/src/tools/core.ts +++ b/src/tools/core.ts @@ -30,13 +30,12 @@ export function registerCoreTools(ctx: ServerContext): void { name: "obsidian_status", toolset: "core", alwaysEnabled: true, - workspaceIndependent: true, + targetIndependent: true, description: "Report which transports are reachable, which Obsidian windows are attached, and which " + "toolsets are enabled. Cheap and safe to call first in a session. For a full diagnosis with " + "remediation steps, use obsidian_doctor instead.", annotations: { readOnlyHint: true }, - profileIndependent: true, inputSchema: {}, handler: async () => { const availability = await router.refreshAvailability(true); @@ -46,22 +45,22 @@ export function registerCoreTools(ctx: ServerContext): void { const windows = availability.playwright ? await router.playwright.windowSummaries().catch(() => []) : []; - const defaultProfileLease = await ctx.profileLease.status(); + const activity = await ctx.activity.status(); const descriptor = - config.sessionId !== undefined ? await readDescriptor(config.sessionId) : undefined; + ctx.currentSessionKey !== undefined + ? await readDescriptor(ctx.currentSessionKey) + : undefined; const profile = - config.sessionId === undefined + ctx.targetKind !== "isolated" ? { - kind: "default" as const, - workspaceHandle: ctx.currentWorkspaceHandle ?? null, + kind: (ctx.targetKind ?? "none") as "default" | "none", sessionId: null, userDataDir: null, visualIdentity: null, } : { kind: "private" as const, - workspaceHandle: ctx.currentWorkspaceHandle ?? null, - sessionId: config.sessionId, + sessionId: ctx.currentSessionKey ?? null, userDataDir: config.userDataDir, visualIdentity: descriptor?.visualIdentity ?? { state: "degraded" as const, @@ -81,9 +80,9 @@ export function registerCoreTools(ctx: ServerContext): void { }`, `Toolsets enabled: ${registry.toolsetState().enabled.join(", ")}`, `Toolsets disabled: ${registry.toolsetState().disabled.join(", ") || "none"}`, - `Default profile: ${defaultProfileLease.state}`, + `Agent use: ${activity.state}`, `Profile identity: ${profile.kind}${profile.sessionId === null ? "" : ` (${profile.sessionId})`}`, - `Workspace: ${profile.workspaceHandle ?? "none"}`, + `Active target: ${ctx.targetKind ?? "none"}`, ...(profile.visualIdentity === null ? [] : [`Visual identity: ${profile.visualIdentity.state}`]), @@ -130,7 +129,7 @@ export function registerCoreTools(ctx: ServerContext): void { Object.entries(registry.byToolset()).map(([name, tools]) => [name, tools.length]), ), problemCount: health.problems.length, - defaultProfileLease, + activity, profile, }, }; @@ -141,8 +140,7 @@ export function registerCoreTools(ctx: ServerContext): void { name: "obsidian_capabilities", toolset: "core", alwaysEnabled: true, - profileIndependent: true, - workspaceIndependent: true, + targetIndependent: true, description: "Report every Knapper capability, the live transport that can serve it, and the fixing tool " + "when it is unavailable. Use this before choosing between obsidian_* and browser_* tools.", @@ -177,10 +175,8 @@ export function registerCoreTools(ctx: ServerContext): void { name: "obsidian_toolsets", toolset: "core", alwaysEnabled: true, - profileIndependent: true, - workspaceIndependent: true, - description: - "Report the current operational toolsets. Use obsidian_toolsets_update to change them.", + targetIndependent: true, + description: "Report the fixed toolsets selected when this server started.", annotations: { readOnlyHint: true }, inputSchema: {}, outputSchema: toolsetStateOutputSchema, @@ -196,107 +192,11 @@ export function registerCoreTools(ctx: ServerContext): void { }, }); - registry.add({ - name: "obsidian_toolsets_update", - toolset: "core", - alwaysEnabled: true, - profileIndependent: true, - workspaceIndependent: true, - description: - "Enable or disable operational toolsets for this server process. Use dryRun to preview the change.", - annotations: { readOnlyHint: false, idempotentHint: true }, - inputSchema: { - enable: z - .array(toolsetNameSchema) - .optional() - .describe("Toolsets to enable immediately through the MCP tool registration handles"), - disable: z - .array(toolsetNameSchema) - .optional() - .describe("Toolsets to disable immediately through the MCP tool registration handles"), - dryRun: z.boolean().optional().describe("Preview the resulting surface without changing it"), - }, - outputSchema: { - dryRun: z.boolean(), - enabled: z.array(toolsetNameSchema), - disabled: z.array(toolsetNameSchema), - changed: z.object({ - enabled: z.array(toolsetNameSchema), - disabled: z.array(toolsetNameSchema), - toolCount: z.number().int().nonnegative(), - }), - }, - handler: async (args) => { - const enable = [...new Set((args.enable as (typeof TOOLSETS)[number][] | undefined) ?? [])]; - const disable = [...new Set((args.disable as (typeof TOOLSETS)[number][] | undefined) ?? [])]; - const overlap = enable.filter((toolset) => disable.includes(toolset)); - if (overlap.length > 0) { - throw new UobError( - "INVALID_ARGUMENT", - `A toolset cannot be enabled and disabled in the same call: ${overlap.join(", ")}.`, - { - remediation: "Remove each duplicate toolset from either enable or disable.", - }, - ); - } - - const before = registry.toolsetState(); - const enabledBefore = new Set(before.enabled); - const changedEnabled = enable.filter((toolset) => !enabledBefore.has(toolset)).sort(); - const changedDisabled = disable.filter((toolset) => enabledBefore.has(toolset)).sort(); - const dryRun = args.dryRun === true; - let changedToolCount = 0; - - if (dryRun) { - for (const toolset of [...changedEnabled, ...changedDisabled]) { - changedToolCount += (registry.groupAllByToolset()[toolset] ?? []).filter( - (name) => registry.get(name)?.alwaysEnabled !== true, - ).length; - } - } else { - for (const toolset of changedEnabled) { - changedToolCount += registry.setToolsetEnabled(toolset, true).length; - } - for (const toolset of changedDisabled) { - changedToolCount += registry.setToolsetEnabled(toolset, false).length; - } - } - - const enabled = new Set(before.enabled); - for (const toolset of changedEnabled) enabled.add(toolset); - for (const toolset of changedDisabled) enabled.delete(toolset); - const state = dryRun - ? { - enabled: TOOLSETS.filter((toolset) => enabled.has(toolset)).sort(), - disabled: TOOLSETS.filter((toolset) => !enabled.has(toolset)).sort(), - } - : registry.toolsetState(); - const prefix = dryRun ? "Dry run" : "Updated"; - return { - text: [ - `${prefix}: ${changedToolCount} tool registration(s) ${dryRun ? "would change" : "changed"}.`, - `Enabled toolsets: ${state.enabled.join(", ") || "none"}`, - `Disabled toolsets: ${state.disabled.join(", ") || "none"}`, - ].join("\n"), - json: { - dryRun, - ...state, - changed: { - enabled: changedEnabled, - disabled: changedDisabled, - toolCount: changedToolCount, - }, - }, - }; - }, - }); - registry.add({ name: "obsidian_tool_catalog", toolset: "core", alwaysEnabled: true, - profileIndependent: true, - workspaceIndependent: true, + targetIndependent: true, description: "Search the Knapper tool catalog without enabling disabled tools. Results use cursor pagination.", annotations: { readOnlyHint: true }, diff --git a/src/tools/editor.ts b/src/tools/editor.ts index 6a3f3c8..f153bc8 100644 --- a/src/tools/editor.ts +++ b/src/tools/editor.ts @@ -6,7 +6,7 @@ * through `router.evaluateJson` with sources built in obsidian/editor-probe.ts, * so either transport can serve them and the renderer logic stays unit-tested. * - * `obsidian_editor_replace` is hash-guarded: concurrent agents (and the user) + * `obsidian_editor_replace` is hash-guarded: queued calls (and the user) * share one live editor, so every edit must prove it saw the current document. */ diff --git a/src/tools/provisioning.ts b/src/tools/provisioning.ts index 0535bb5..3181c63 100644 --- a/src/tools/provisioning.ts +++ b/src/tools/provisioning.ts @@ -325,14 +325,13 @@ export function registerProvisioningTools(ctx: ServerContext): void { name: "obsidian_doctor", toolset: "core", alwaysEnabled: true, - workspaceIndependent: true, + targetIndependent: true, description: "Full diagnostic: the four precondition states (not running, CLI disabled, CDP closed, " + "argv corruption), binary path and version, registered vaults, target vault automation state, " + "dev-plugin symlinks, and per-toolset tool availability. Prefer this over obsidian_status when " + "something is broken — every problem includes remediation and names a fixing tool when one exists.", annotations: { readOnlyHint: true }, - profileIndependent: true, inputSchema: { vault: z.string().optional().describe("Vault to inspect for restrict-mode and plugin state"), detail: z @@ -350,28 +349,8 @@ export function registerProvisioningTools(ctx: ServerContext): void { }); const availability = await router.refreshAvailability(true); - let version: string; - if (config.sessionId === undefined && health.running) { - try { - const result = await ctx.profileLease.run("obsidian_doctor", async () => { - const leasedHealth = await router.health(); - return { - health: leasedHealth, - version: await runningObsidianVersion(router, leasedHealth), - }; - }); - health = result.health; - version = result.version; - } catch (error) { - version = - error instanceof UobError && error.code === "DEFAULT_PROFILE_BUSY" - ? "(unavailable — default profile busy)" - : "(unavailable)"; - } - } else { - version = await runningObsidianVersion(router, health); - } - const defaultProfileLease = await ctx.profileLease.status(); + const version = await runningObsidianVersion(router, health); + const activity = await ctx.activity.status(); const toolsets: Record = {}; const byToolset = registry.groupAllByToolset(); @@ -398,20 +377,20 @@ export function registerProvisioningTools(ctx: ServerContext): void { installedPackageSource: installedPackage?.source, }); const descriptor = - config.sessionId !== undefined ? await readDescriptor(config.sessionId) : undefined; + ctx.currentSessionKey !== undefined + ? await readDescriptor(ctx.currentSessionKey) + : undefined; const profile = - config.sessionId === undefined + ctx.targetKind !== "isolated" ? { - kind: "default" as const, - workspaceHandle: ctx.currentWorkspaceHandle ?? null, + kind: (ctx.targetKind ?? "none") as "default" | "none", sessionId: null, userDataDir: null, visualIdentity: null, } : { kind: "private" as const, - workspaceHandle: ctx.currentWorkspaceHandle ?? null, - sessionId: config.sessionId, + sessionId: ctx.currentSessionKey ?? null, userDataDir: config.userDataDir, visualIdentity: descriptor?.visualIdentity ?? { state: "degraded" as const, @@ -437,7 +416,7 @@ export function registerProvisioningTools(ctx: ServerContext): void { // which is worth saying out loud because nothing else reports it. if (config.sessionId !== undefined) { lines.push( - `Workspace: ${ctx.currentWorkspaceHandle ?? "isolated"}`, + "Active target: isolated", ...(full ? [`Profile: ${config.userDataDir}`] : []), `Visual identity: ${profile.visualIdentity?.state ?? "degraded"}`, `CLI isolation: ${config.cliIsolation}` + @@ -446,10 +425,7 @@ export function registerProvisioningTools(ctx: ServerContext): void { : " — CLI commands are not pinned to this instance; the renderer route is"), ); } else { - lines.push( - `Workspace: ${ctx.currentWorkspaceHandle ?? "none"} (default Obsidian profile)`, - `Default profile: ${defaultProfileLease.state}`, - ); + lines.push(`Active target: ${ctx.targetKind ?? "none"}`, `Agent use: ${activity.state}`); } if (health.argvCorruption) { @@ -478,7 +454,7 @@ export function registerProvisioningTools(ctx: ServerContext): void { lines.push( " No vault is authorized. Every vault-scoped tool will refuse until the user runs", " `npx knapper authorize ` themselves, or obsidian_create_vault makes a", - " registered one. Prefer obsidian_workspace_create for throwaway work. Do not suggest", + " registered one. Prefer obsidian_session_open for throwaway work. Do not suggest", " authorization unless the user asked to work", " in a specific existing vault.", ); @@ -544,7 +520,7 @@ export function registerProvisioningTools(ctx: ServerContext): void { vaultState: vaultState ?? null, toolsets, transports: availability, - defaultProfileLease, + activity, profile, }, }; @@ -812,11 +788,10 @@ export function registerProvisioningTools(ctx: ServerContext): void { if (config.sessionId !== undefined) { throw new UobError( "INVALID_ARGUMENT", - "A workspace-bound server cannot add another vault to its private profile.", + "An isolated session cannot add another vault to its private profile.", { - remediation: - "Create a separate workspace for the new vault. This keeps each private profile tied to one vault.", - fixedBy: "obsidian_workspace_create", + remediation: "Reset the managed session when you need a different scratch vault.", + fixedBy: "obsidian_session_reset", }, ); } @@ -916,7 +891,7 @@ export function registerProvisioningTools(ctx: ServerContext): void { toolset: "vault", description: "Unregister an authorized vault from Obsidian. This tool never deletes files. Scratch " + - "workspace cleanup is available only through obsidian_workspace_destroy and moves content " + + "session cleanup is available only through obsidian_session_reset and moves content " + "to recoverable Knapper trash.", inputSchema: { vault: z diff --git a/src/tools/registry.ts b/src/tools/registry.ts index 4252c63..76b718a 100644 --- a/src/tools/registry.ts +++ b/src/tools/registry.ts @@ -7,14 +7,11 @@ * call time rather than registration time, because a transport can appear or * disappear while the server is running (Obsidian restarts, debug port opens). * - * Dispatch is also where call admission happens. Every tool drives the same live - * Obsidian window, so a mutating call takes the exclusive lock and a read-only - * call takes a bounded shared one. Classification comes from the `readOnlyHint` - * annotation each tool already declares, and defaults to exclusive when the hint - * is absent — an unannotated tool is assumed to touch the UI. + * Dispatch is also where call admission happens. Every tool uses the same live + * Obsidian target, so all calls enter one FIFO lane. */ -import type { McpServer, RegisteredTool } from "@modelcontextprotocol/server"; +import type { McpServer } from "@modelcontextprotocol/server"; import { z, type ZodRawShape } from "zod"; import type { Capability } from "../capabilities.js"; import type { Toolset } from "../toolsets.js"; @@ -22,10 +19,9 @@ import type { Logger } from "../util/logger.js"; import type { TelemetryStore } from "../telemetry/store.js"; import { appendTelemetrySummary } from "../telemetry/helpers.js"; import { toUobError, UobError } from "../util/errors.js"; -import { CallLock, type LockMode } from "../util/concurrency.js"; +import { CallLock } from "../util/concurrency.js"; import { renderResult, safeStringify } from "../util/serialize.js"; import { jsonSchemaToZodShape } from "../browser/json-schema.js"; -import type { DefaultProfileLease } from "../session/default-profile-lease.js"; import { errorEnvelope, requestId, toolAuditEvent } from "../audit/event.js"; import { JsonlAuditWriter } from "../audit/writer.js"; import type { @@ -72,12 +68,8 @@ export interface ToolDefinition { /** JSON Schema for proxied tools (converted to Zod at registration). */ jsonOutputSchema?: Record; annotations?: ToolAnnotations; - /** This tool never reads or drives the installation's default Obsidian profile. */ - profileIndependent?: boolean | ((args: Record) => boolean); - /** This control-plane tool does not require or bind a workspace handle. */ - workspaceIndependent?: boolean; - /** A workspace-control tool must hold the same exclusive lease as operational tools. */ - requiresWorkspaceLease?: boolean; + /** This control-plane tool does not require an active Obsidian target. */ + targetIndependent?: boolean; /** Keep this control-plane tool enabled even when its toolset is disabled. */ alwaysEnabled?: boolean; /** @@ -183,7 +175,6 @@ function withTelemetrySuffix( export class ToolRegistry { private readonly definitions = new Map(); - private readonly handles = new Map(); private readonly lock: CallLock; private readonly audit: AuditSink | false; private auditWritePending = false; @@ -191,22 +182,13 @@ export class ToolRegistry { constructor( enabledToolsets: Set, private readonly logger: Logger, - /** - * Read-only calls allowed to overlap. - * - * Required rather than defaulted: this used to fall back to `loadConfig()`, - * which both broke the repo's own "config lives in config.ts" rule and, once - * sessions existed, would size the lock from the *unbound* environment while - * the rest of the server ran against a session. - */ - maxConcurrency: number, private readonly telemetry?: TelemetryStore, - private readonly profileLease?: DefaultProfileLease, - private readonly sessionBound: () => boolean = () => false, private readonly hooks: ToolRegistryHooks = {}, ) { this.enabledToolsets = new Set(enabledToolsets); - this.lock = new CallLock({ maxShared: maxConcurrency }); + // Every tool reaches the same live Obsidian target. Keep one FIFO lane so + // overlapping agent calls cannot interleave UI, CLI, or plugin state. + this.lock = new CallLock({ maxShared: 1 }); this.audit = hooks.audit === undefined ? new JsonlAuditWriter() : hooks.audit; } @@ -229,7 +211,7 @@ export class ToolRegistry { }); } - /** Retain every definition so its toolset can be enabled at runtime. */ + /** Retain a definition for the fixed startup surface. */ add(def: ToolDefinition): void { if (this.definitions.has(def.name)) { throw new Error(`Duplicate tool registration: ${def.name}`); @@ -339,23 +321,6 @@ export class ToolRegistry { }; } - /** Enable or disable one toolset through the SDK registration handles. */ - setToolsetEnabled(toolset: Toolset, enabled: boolean): string[] { - if (enabled) this.enabledToolsets.add(toolset); - else this.enabledToolsets.delete(toolset); - - const changed: string[] = []; - for (const def of this.definitions.values()) { - if (def.toolset !== toolset || def.alwaysEnabled === true) continue; - const handle = this.handles.get(def.name); - if (!handle || handle.enabled === enabled) continue; - if (enabled) handle.enable(); - else handle.disable(); - changed.push(def.name); - } - return changed.sort(); - } - /** All retained definitions grouped by toolset, including disabled tools. */ groupAllByToolset(): Record { const out: Record = {}; @@ -376,19 +341,12 @@ export class ToolRegistry { */ bind(server: McpServer): void { for (const def of this.definitions.values()) { + if (!this.isDefinitionEnabled(def)) continue; const config: Record = { description: def.description }; const shape = def.jsonInputSchema ? jsonSchemaToZodShape(def.jsonInputSchema) : (def.inputSchema ?? {}); - config.inputSchema = - def.workspaceIndependent === true - ? shape - : { - ...shape, - workspaceHandle: z - .string() - .describe("Explicit workspace handle returned by an obsidian_workspace_* tool."), - }; + config.inputSchema = shape; if (def.jsonOutputSchema) { config.outputSchema = jsonSchemaToZodShape(def.jsonOutputSchema); } else if (def.outputSchema) { @@ -401,7 +359,7 @@ export class ToolRegistry { } config.annotations = { readOnlyHint: false, ...def.annotations }; - const handle = server.registerTool( + server.registerTool( def.name, config as never, (async ( @@ -417,15 +375,11 @@ export class ToolRegistry { let auditContext: AuditCallContext | undefined; let auditOutcome: "success" | "error" = "success"; let auditError: AuditErrorEnvelope | undefined; - // Workspace binding mutates the shared router, capture, telemetry - // delegate, and configuration. Keep the exclusive grant through the - // complete handler so a second workspace cannot rebind mid-call. - const mode: LockMode = - def.workspaceIndependent === true && def.annotations?.readOnlyHint === true - ? "shared" - : "exclusive"; + let completedOutcome: ToolOutcome | UobError | undefined; + // The shared router, capture, telemetry delegate, and configuration all + // target one app. Keep the exclusive grant through the complete handler. try { - return await this.lock.run(mode, def.name, async () => { + return await this.lock.run("exclusive", def.name, async () => { // Read the telemetry cursor after admission, not before: a call that // waited in the queue would otherwise report every log line produced // by the calls it was queued behind. @@ -435,15 +389,8 @@ export class ToolRegistry { queueMs = ranAt - started; await this.hooks.beforeInvoke?.(def, callArgs, requestContext); auditContext = await this.hooks.contextProvider?.(callArgs, requestContext); - const invoke = (): Promise => def.handler(callArgs); - const profileIndependent = - typeof def.profileIndependent === "function" - ? def.profileIndependent(callArgs) - : def.profileIndependent === true; - let outcome = - profileIndependent || this.sessionBound() || !this.profileLease - ? await invoke() - : await this.profileLease.run(def.name, invoke); + let outcome = await def.handler(callArgs); + completedOutcome = outcome; if ( this.telemetry && def.annotations?.readOnlyHint !== true && @@ -470,6 +417,7 @@ export class ToolRegistry { }); } catch (e) { const err = toUobError(e); + completedOutcome = err; auditOutcome = "error"; auditError = errorEnvelope(err); this.logger.warn("tool failed", { @@ -479,6 +427,16 @@ export class ToolRegistry { }); return errorResult(err); } finally { + if (admitted && completedOutcome !== undefined) { + try { + await this.hooks.afterInvoke?.(def, callArgs, requestContext, completedOutcome); + } catch (hookError) { + this.logger.warn("afterInvoke hook failed", { + tool: def.name, + error: hookError instanceof Error ? hookError.name : "UnknownHookError", + }); + } + } if (this.audit !== false) { this.queueAudit( toolAuditEvent({ @@ -497,8 +455,6 @@ export class ToolRegistry { } }) as never, ); - this.handles.set(def.name, handle); - if (!this.isDefinitionEnabled(def)) handle.disable(); } this.logger.info(`registered ${this.definitions.size} tools`, this.byToolset()); } diff --git a/src/tools/session.ts b/src/tools/session.ts new file mode 100644 index 0000000..c53cac8 --- /dev/null +++ b/src/tools/session.ts @@ -0,0 +1,269 @@ +/** Single active Obsidian target for handle-free MCP clients. */ + +import { z } from "zod"; +import type { ServerContext } from "../server.js"; +import { listDescriptors, readDescriptor, type SessionDescriptor } from "../session/descriptor.js"; +import { + createSession, + listSessions, + quarantineSession, + restartSession, + sessionDiagnostics, + sessionState, + stopSession, + waitSession, +} from "../session/registry.js"; +import { UobError } from "../util/errors.js"; + +function publicSummary(descriptor: SessionDescriptor): Record { + return { + session: descriptor.key, + phase: descriptor.readiness.phase, + vault: descriptor.vault?.name, + plugin: descriptor.plugin?.id, + pluginSourceDir: descriptor.plugin?.sourceDir, + cdpUrl: descriptor.instance.cdpUrl, + pid: descriptor.instance.pid, + visualIdentity: descriptor.visualIdentity ?? null, + }; +} + +function compatible( + descriptor: SessionDescriptor, + pluginSourceDir?: string, + pluginId?: string, +): boolean { + if (pluginSourceDir !== undefined && descriptor.plugin?.sourceDir !== pluginSourceDir) + return false; + if (pluginId !== undefined && descriptor.plugin?.id !== pluginId) return false; + return true; +} + +async function singletonDescriptor( + pluginSourceDir?: string, + pluginId?: string, +): Promise { + const descriptors = (await listDescriptors()).filter( + (descriptor) => descriptor.readiness.phase !== "failed", + ); + if (descriptors.length === 0) return undefined; + const descriptor = descriptors.at(-1) as SessionDescriptor; + if (!compatible(descriptor, pluginSourceDir, pluginId)) { + throw new UobError("INVALID_ARGUMENT", "The open Knapper session targets a different plugin.", { + remediation: "Reset the managed session before you change the plugin target.", + fixedBy: "obsidian_session_reset", + details: { + active: publicSummary(descriptor), + requested: { pluginSourceDir: pluginSourceDir ?? null, pluginId: pluginId ?? null }, + }, + }); + } + return descriptor; +} + +async function makeReady( + ctx: ServerContext, + descriptor: SessionDescriptor, + timeoutMs?: number, +): Promise { + let next = descriptor; + if (next.readiness.phase === "starting") { + next = await waitSession(next.key, timeoutMs !== undefined ? { timeoutMs } : {}); + } else if (next.readiness.phase === "stopped" || (await sessionState(next)) !== "live") { + const restarted = await restartSession(next.key, { + logger: ctx.logger.child("session"), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }); + next = restarted.descriptor; + if (next.readiness.phase === "starting") { + next = await waitSession(next.key, timeoutMs !== undefined ? { timeoutMs } : {}); + } + } + await ctx.bindSession(next); + ctx.selectTelemetry("session"); + return next; +} + +async function openIsolated( + ctx: ServerContext, + args: Record, +): Promise { + const pluginSourceDir = + typeof args.pluginSourceDir === "string" ? args.pluginSourceDir : undefined; + const pluginId = typeof args.pluginId === "string" ? args.pluginId : undefined; + const timeoutMs = typeof args.timeoutMs === "number" ? args.timeoutMs : undefined; + let descriptor = await singletonDescriptor(pluginSourceDir, pluginId); + if (descriptor === undefined) { + descriptor = await createSession({ + obsidianBin: ctx.config.obsidianBin, + logger: ctx.logger.child("session"), + ...(typeof args.label === "string" ? { label: args.label } : {}), + ...(pluginSourceDir !== undefined ? { pluginSourceDir } : {}), + ...(pluginId !== undefined ? { pluginId } : {}), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }); + } + return makeReady(ctx, descriptor, timeoutMs); +} + +export function registerSessionTools(ctx: ServerContext): void { + const { registry } = ctx; + + registry.add({ + name: "obsidian_session_open", + toolset: "core", + alwaysEnabled: true, + targetIndependent: true, + annotations: { readOnlyHint: false, idempotentHint: true }, + description: + "Open or reuse the one active Obsidian target. Isolated scratch space is the default.", + inputSchema: { + target: z + .enum(["isolated", "default"]) + .optional() + .describe("Target type. Omit for a private scratch session."), + label: z.string().optional().describe("Short label for a new scratch session."), + pluginSourceDir: z + .string() + .optional() + .describe("Absolute loadable plugin directory with manifest.json and main.js."), + pluginId: z.string().optional().describe("Expected plugin ID from manifest.json."), + timeoutMs: z.number().int().positive().optional().describe("Maximum startup wait."), + }, + handler: async (args) => { + if (args.target === "default") { + if (args.pluginSourceDir !== undefined || args.pluginId !== undefined) { + throw new UobError( + "INVALID_ARGUMENT", + "Plugin preloading requires an isolated session.", + { + remediation: + 'Omit target="default", or omit pluginSourceDir and pluginId when you open the default profile.', + fixedBy: "obsidian_session_open", + }, + ); + } + await ctx.bindDefault(); + ctx.selectTelemetry("default"); + return { + text: "The default Obsidian profile is active. Vault authorization still applies.", + json: { target: "default", active: true }, + }; + } + const descriptor = await openIsolated(ctx, args); + return { + text: `Isolated session ${descriptor.key} is ready.`, + json: { + target: "isolated", + active: true, + ...publicSummary(descriptor), + diagnostics: await sessionDiagnostics(descriptor), + }, + }; + }, + }); + + registry.add({ + name: "obsidian_session_status", + toolset: "core", + alwaysEnabled: true, + targetIndependent: true, + annotations: { readOnlyHint: true }, + description: + "Report the active target and all legacy managed session records without changing them.", + inputSchema: {}, + handler: async () => { + const sessions = await listSessions({ currentKey: ctx.currentSessionKey }); + const active = + ctx.currentSessionKey === undefined + ? undefined + : await readDescriptor(ctx.currentSessionKey); + return { + text: + ctx.targetKind === undefined + ? `No target is active. ${sessions.length} managed session record(s) exist.` + : `The active target is ${ctx.targetKind}.`, + json: { + target: ctx.targetKind ?? null, + active: active === undefined ? null : publicSummary(active), + managedSessions: sessions.map((session) => ({ + ...publicSummary(session.descriptor), + state: session.state, + current: session.isCurrent, + })), + }, + }; + }, + }); + + registry.add({ + name: "obsidian_session_release", + toolset: "core", + alwaysEnabled: true, + targetIndependent: true, + annotations: { readOnlyHint: false, idempotentHint: true }, + description: + "Release this server's active target. A private Obsidian session stays open for reuse.", + inputSchema: {}, + handler: async () => { + const released = ctx.currentSessionKey; + await ctx.bindDefault(); + ctx.currentSessionKey = undefined; + ctx.targetKind = undefined; + ctx.selectTelemetry("default"); + return { + text: + released === undefined + ? "No active private session needed release." + : `Released session ${released}.`, + json: { released: released ?? null, sessionKeptOpen: released !== undefined }, + }; + }, + }); + + registry.add({ + name: "obsidian_session_reset", + toolset: "core", + alwaysEnabled: true, + targetIndependent: true, + annotations: { readOnlyHint: false, destructiveHint: true }, + description: + "Stop and quarantine the managed scratch session, then create a fresh isolated session.", + inputSchema: { + label: z.string().optional().describe("Short label for the new scratch session."), + pluginSourceDir: z + .string() + .optional() + .describe("Absolute loadable plugin directory with manifest.json and main.js."), + pluginId: z.string().optional().describe("Expected plugin ID from manifest.json."), + timeoutMs: z.number().int().positive().optional().describe("Maximum stop and startup wait."), + }, + handler: async (args) => { + const timeoutMs = typeof args.timeoutMs === "number" ? args.timeoutMs : undefined; + const descriptors = await listDescriptors(); + const previous = ctx.currentSessionKey ?? descriptors.at(-1)?.key; + let quarantinedPath: string | undefined; + if (previous !== undefined) { + const stopped = await stopSession(previous, timeoutMs !== undefined ? { timeoutMs } : {}); + if (stopped.state === "quitFailed") { + throw new UobError("TIMEOUT", `Session ${previous} did not stop.`, { + remediation: "Retry after the managed Obsidian process stops.", + }); + } + quarantinedPath = (await quarantineSession(previous)).quarantinedPath; + } + await ctx.bindDefault(); + ctx.currentSessionKey = undefined; + ctx.targetKind = undefined; + const descriptor = await openIsolated(ctx, args); + return { + text: `Fresh isolated session ${descriptor.key} is ready.`, + json: { + reset: previous ?? null, + quarantinedPath: quarantinedPath ?? null, + ...publicSummary(descriptor), + }, + }; + }, + }); +} diff --git a/src/tools/workspace.ts b/src/tools/workspace.ts deleted file mode 100644 index c513fa3..0000000 --- a/src/tools/workspace.ts +++ /dev/null @@ -1,637 +0,0 @@ -/** Explicit agent and workspace lifecycle for stateless MCP clients. */ - -import { z } from "zod"; -import { join } from "node:path"; -import type { ServerContext } from "../server.js"; -import { closeAgent, openAgent, requireAgent } from "../agent/store.js"; -import { - createWorkspaceRecord, - listWorkspaces, - readWorkspace, - removeWorkspaceRecord, - requireWorkspace, - type WorkspaceRecord, -} from "../workspace/store.js"; -import { - createSession, - quarantineSession, - releaseSession, - restartSession, - sessionDiagnostics, - sessionState, - stopSession, - waitSession, -} from "../session/registry.js"; -import { readDescriptor } from "../session/descriptor.js"; -import { UobError } from "../util/errors.js"; -import { sessionPaths, trashDir } from "../config.js"; - -function agentSummary(record: Awaited>): Record { - return { - agentHandle: record.handle, - label: record.label, - purpose: record.purpose, - cwd: record.cwd, - createdAt: record.createdAt, - lastActivityAt: record.lastActivityAt, - expiresAt: record.expiresAt, - observedClients: record.observedClients, - }; -} - -async function workspaceSummary(handle: string): Promise> { - const record = await readWorkspace(handle); - if (record === undefined) { - throw new UobError("SESSION_NOT_FOUND", `Workspace ${handle} does not exist.`, { - remediation: "Create a workspace with obsidian_workspace_create.", - fixedBy: "obsidian_workspace_create", - }); - } - const descriptor = - record.sessionKey !== undefined ? await readDescriptor(record.sessionKey) : undefined; - const processState = descriptor !== undefined ? await sessionState(descriptor) : undefined; - const expired = - !Number.isFinite(Date.parse(record.expiresAt)) || Date.parse(record.expiresAt) <= Date.now(); - return { - workspaceHandle: record.handle, - agentHandle: record.agentHandle, - kind: record.kind, - label: record.label, - createdAt: record.createdAt, - lastActivityAt: record.lastActivityAt, - expiresAt: record.expiresAt, - expired, - active: processState === "live" && descriptor?.readiness.phase === "ready", - processState, - phase: descriptor?.readiness.phase, - profile: { - kind: record.kind === "isolated" ? "private" : "default", - sessionId: record.sessionKey ?? null, - userDataDir: descriptor?.instance.userDataDir ?? null, - }, - visualIdentity: descriptor?.visualIdentity ?? null, - vault: descriptor?.vault?.name, - plugin: descriptor?.plugin?.id, - }; -} - -async function restoreWorkspaceBinding( - ctx: ServerContext, - previousHandle: string | undefined, -): Promise { - try { - if (previousHandle !== undefined) { - const previous = await readWorkspace(previousHandle); - if (previous?.kind === "isolated" && previous.sessionKey !== undefined) { - const descriptor = await readDescriptor(previous.sessionKey); - if (descriptor !== undefined) { - ctx.selectTelemetry(previous.handle); - await ctx.bindSession(descriptor, previous.handle); - ctx.currentWorkspaceHandle = previous.handle; - return; - } - } else if (previous?.kind === "default") { - ctx.selectTelemetry("default"); - await ctx.bindDefaultWorkspace(); - ctx.currentWorkspaceHandle = previous.handle; - return; - } - } - } catch (error) { - ctx.logger.warn("could not restore the previous workspace after a binding failure", { - workspaceHandle: previousHandle, - error: error instanceof Error ? error.message : String(error), - }); - } - ctx.currentWorkspaceHandle = undefined; - ctx.selectTelemetry("default"); - await ctx.bindDefaultWorkspace().catch(() => undefined); -} - -async function archiveWorkspaceTelemetry( - ctx: ServerContext, - workspaceHandle: string, - destinationRoot: string, -): Promise<{ telemetryArchive?: string; telemetryArchiveError?: string }> { - try { - const telemetryArchive = await ctx.archiveTelemetry(workspaceHandle, destinationRoot); - return telemetryArchive === undefined ? {} : { telemetryArchive }; - } catch (error) { - return { - telemetryArchiveError: error instanceof Error ? error.message : String(error), - }; - } -} - -function telemetryArchiveText(result: { - telemetryArchive?: string; - telemetryArchiveError?: string; -}): string { - if (result.telemetryArchive !== undefined) { - return ` Telemetry archived at ${result.telemetryArchive}.`; - } - if (result.telemetryArchiveError !== undefined) { - return ` Telemetry archive warning: ${result.telemetryArchiveError}`; - } - return " No workspace telemetry file needed archiving."; -} - -async function workspaceForCleanup(handle: string): Promise { - const record = await readWorkspace(handle); - if (record === undefined) { - throw new UobError("SESSION_NOT_FOUND", `Workspace ${handle} does not exist.`, { - remediation: "Create a workspace with obsidian_workspace_create.", - fixedBy: "obsidian_workspace_create", - }); - } - return record; -} - -export function registerWorkspaceTools(ctx: ServerContext): void { - const { registry, config, logger } = ctx; - - registry.add({ - name: "obsidian_agent_open", - toolset: "workspace", - alwaysEnabled: true, - workspaceIndependent: true, - profileIndependent: true, - annotations: { readOnlyHint: false }, - description: - "Create an explicit, durable agent handle for attribution. The handle is not an authentication credential.", - inputSchema: { - label: z.string().min(1).describe("Short name for the agent or task."), - purpose: z.string().optional().describe("What this agent will do."), - cwd: z.string().optional().describe("Working directory used for attribution."), - }, - handler: async (args) => { - const record = await openAgent({ - label: args.label as string, - ...(typeof args.purpose === "string" ? { purpose: args.purpose } : {}), - ...(typeof args.cwd === "string" ? { cwd: args.cwd } : {}), - }); - return { - text: `Opened agent ${record.handle}.`, - json: agentSummary(record), - }; - }, - }); - - registry.add({ - name: "obsidian_agent_status", - toolset: "workspace", - alwaysEnabled: true, - workspaceIndependent: true, - profileIndependent: true, - description: "Inspect one explicit agent handle and its current workspace count.", - annotations: { readOnlyHint: true }, - inputSchema: { - agentHandle: z.string().describe("Agent handle returned by obsidian_agent_open."), - }, - handler: async (args) => { - const record = await requireAgent(args.agentHandle as string); - const workspaces = (await listWorkspaces()).filter( - (workspace) => workspace.agentHandle === record.handle, - ); - return { - text: `Agent ${record.handle} owns ${workspaces.length} workspace(s).`, - json: { ...agentSummary(record), workspaceCount: workspaces.length }, - }; - }, - }); - - registry.add({ - name: "obsidian_agent_close", - toolset: "workspace", - alwaysEnabled: true, - workspaceIndependent: true, - profileIndependent: true, - annotations: { readOnlyHint: false, destructiveHint: true }, - description: - "Close an agent handle after all of its workspaces have been released or destroyed.", - inputSchema: { agentHandle: z.string().describe("Agent handle to close.") }, - handler: async (args) => { - const handle = args.agentHandle as string; - const workspaces = (await listWorkspaces()).filter( - (workspace) => workspace.agentHandle === handle, - ); - if (workspaces.length > 0) { - throw new UobError("INVALID_ARGUMENT", "The agent still owns workspaces.", { - remediation: "Release or destroy each workspace, then close the agent handle.", - details: { - workspaceHandles: workspaces.map((workspace) => workspace.handle), - }, - }); - } - await closeAgent(handle); - return { - text: `Closed agent ${handle}.`, - json: { agentHandle: handle, closed: true }, - }; - }, - }); - - registry.add({ - name: "obsidian_workspace_create", - toolset: "workspace", - alwaysEnabled: true, - workspaceIndependent: true, - profileIndependent: true, - capability: "launch", - annotations: { readOnlyHint: false }, - description: - "Create and activate an isolated Obsidian workspace with a Knapper-owned scratch vault.", - inputSchema: { - agentHandle: z.string().describe("Agent handle that will own the workspace."), - label: z.string().optional().describe("Short label for this isolated workspace."), - pluginSourceDir: z - .string() - .optional() - .describe("Absolute loadable plugin directory with manifest.json and main.js."), - pluginId: z.string().optional().describe("Expected plugin ID from manifest.json."), - timeoutMs: z - .number() - .int() - .positive() - .optional() - .describe("Maximum startup wait in milliseconds."), - }, - handler: async (args) => { - const agentHandle = args.agentHandle as string; - await requireAgent(agentHandle); - const previousHandle = ctx.currentWorkspaceHandle; - let descriptor; - let workspace: WorkspaceRecord | undefined; - try { - descriptor = await createSession({ - agentHandle, - obsidianBin: config.obsidianBin, - logger: logger.child("workspace"), - ...(typeof args.label === "string" ? { label: args.label } : {}), - ...(typeof args.pluginSourceDir === "string" - ? { pluginSourceDir: args.pluginSourceDir } - : {}), - ...(typeof args.pluginId === "string" ? { pluginId: args.pluginId } : {}), - ...(typeof args.timeoutMs === "number" ? { timeoutMs: args.timeoutMs } : {}), - }); - if (descriptor.readiness.phase === "starting") { - descriptor = await waitSession(descriptor.key, { - timeoutMs: typeof args.timeoutMs === "number" ? args.timeoutMs : 30_000, - }); - } - workspace = await createWorkspaceRecord({ - agentHandle, - kind: "isolated", - sessionKey: descriptor.key, - ...(typeof args.label === "string" ? { label: args.label } : {}), - }); - await ctx.workspaceLeases.acquire(workspace.handle, "obsidian_workspace_create"); - ctx.selectTelemetry(workspace.handle); - await ctx.bindSession(descriptor, workspace.handle); - ctx.currentWorkspaceHandle = workspace.handle; - } catch (error) { - if (workspace !== undefined) { - await ctx.workspaceLeases.release(workspace.handle).catch(() => undefined); - await removeWorkspaceRecord(workspace.handle).catch(() => undefined); - } - if (descriptor !== undefined) { - const stop = await stopSession(descriptor.key, { - timeoutMs: typeof args.timeoutMs === "number" ? args.timeoutMs : 15_000, - }).catch(() => undefined); - if (stop !== undefined && stop.state !== "quitFailed") { - await quarantineSession(descriptor.key).catch(() => undefined); - } - } - await restoreWorkspaceBinding(ctx, previousHandle); - throw error; - } - return { - text: `Created and activated isolated workspace ${workspace.handle}.`, - json: { - ...(await workspaceSummary(workspace.handle)), - diagnostics: await sessionDiagnostics(descriptor), - }, - }; - }, - }); - - registry.add({ - name: "obsidian_workspace_claim_default", - toolset: "workspace", - alwaysEnabled: true, - workspaceIndependent: true, - profileIndependent: true, - annotations: { readOnlyHint: false }, - description: - "Create and activate a workspace handle for the user's default Obsidian profile. Vault authorization still applies.", - inputSchema: { - agentHandle: z.string().describe("Agent handle that will own the workspace."), - label: z.string().optional().describe("Short label for this default-profile claim."), - }, - handler: async (args) => { - await requireAgent(args.agentHandle as string); - const previousHandle = ctx.currentWorkspaceHandle; - let workspace: WorkspaceRecord | undefined; - try { - workspace = await createWorkspaceRecord({ - agentHandle: args.agentHandle as string, - kind: "default", - ...(typeof args.label === "string" ? { label: args.label } : {}), - }); - await ctx.workspaceLeases.acquire(workspace.handle, "obsidian_workspace_claim_default"); - ctx.selectTelemetry("default"); - await ctx.bindDefaultWorkspace(); - ctx.currentWorkspaceHandle = workspace.handle; - } catch (error) { - if (workspace !== undefined) { - await ctx.workspaceLeases.release(workspace.handle).catch(() => undefined); - await removeWorkspaceRecord(workspace.handle).catch(() => undefined); - } - await restoreWorkspaceBinding(ctx, previousHandle); - throw error; - } - return { - text: `Claimed the default profile as workspace ${workspace.handle}.`, - json: await workspaceSummary(workspace.handle), - }; - }, - }); - - registry.add({ - name: "obsidian_workspace_list", - toolset: "workspace", - alwaysEnabled: true, - workspaceIndependent: true, - profileIndependent: true, - annotations: { readOnlyHint: true }, - description: "List durable workspace handles, optionally for one agent.", - inputSchema: { - agentHandle: z - .string() - .optional() - .describe("Return only workspaces owned by this agent handle."), - }, - handler: async (args) => { - const records = (await listWorkspaces()).filter( - (record) => args.agentHandle === undefined || record.agentHandle === args.agentHandle, - ); - const workspaces = await Promise.all( - records.map((record) => workspaceSummary(record.handle)), - ); - return { - text: `${workspaces.length} workspace(s).`, - json: { count: workspaces.length, workspaces }, - }; - }, - }); - - registry.add({ - name: "obsidian_workspace_status", - toolset: "workspace", - alwaysEnabled: true, - workspaceIndependent: true, - profileIndependent: true, - annotations: { readOnlyHint: true }, - description: "Inspect one workspace handle without activating its Obsidian window.", - inputSchema: { - workspaceHandle: z.string().describe("Workspace handle to inspect without activating it."), - }, - handler: async (args) => { - const summary = await workspaceSummary(args.workspaceHandle as string); - return { - text: [ - `Workspace: ${summary.workspaceHandle}`, - `Kind: ${summary.kind}`, - `Process: ${summary.processState ?? "not applicable"}`, - `Phase: ${summary.phase ?? "not applicable"}`, - `Active: ${summary.active ? "yes" : "no"}`, - `Expired: ${summary.expired ? "yes" : "no"}`, - ...(summary.vault !== undefined ? [`Vault: ${summary.vault}`] : []), - ...(summary.plugin !== undefined ? [`Plugin: ${summary.plugin}`] : []), - `Lease expires: ${summary.expiresAt}`, - ].join("\n"), - json: summary, - }; - }, - }); - - registry.add({ - name: "obsidian_workspace_restart", - toolset: "workspace", - alwaysEnabled: true, - requiresWorkspaceLease: true, - workspaceIndependent: true, - profileIndependent: true, - capability: "launch", - annotations: { readOnlyHint: false, destructiveHint: true }, - description: "Cold-restart one isolated workspace and activate it.", - inputSchema: { - workspaceHandle: z.string().describe("Isolated workspace handle to restart."), - timeoutMs: z - .number() - .int() - .positive() - .optional() - .describe("Maximum restart wait in milliseconds."), - }, - handler: async (args) => { - const workspace = await requireWorkspace(args.workspaceHandle as string); - if (workspace.kind !== "isolated" || workspace.sessionKey === undefined) { - throw new UobError( - "INVALID_ARGUMENT", - "The default-profile workspace cannot be restarted here.", - { - remediation: "Use obsidian_launch with this workspace handle.", - }, - ); - } - const result = await restartSession(workspace.sessionKey, { - ...(typeof args.timeoutMs === "number" ? { timeoutMs: args.timeoutMs } : {}), - logger: logger.child("workspace"), - }); - ctx.selectTelemetry(workspace.handle); - await ctx.bindSession(result.descriptor, workspace.handle); - ctx.currentWorkspaceHandle = workspace.handle; - return { - text: `Restarted workspace ${workspace.handle}.`, - json: { - ...(await workspaceSummary(workspace.handle)), - quit: result.quit, - }, - }; - }, - }); - - registry.add({ - name: "obsidian_workspace_stop", - toolset: "workspace", - alwaysEnabled: true, - requiresWorkspaceLease: true, - workspaceIndependent: true, - profileIndependent: true, - annotations: { readOnlyHint: false, destructiveHint: true }, - description: "Stop one isolated workspace without removing its files or records.", - inputSchema: { - workspaceHandle: z.string().describe("Isolated workspace handle to stop."), - timeoutMs: z - .number() - .int() - .positive() - .optional() - .describe("Maximum stop wait in milliseconds."), - }, - handler: async (args) => { - const workspace = await requireWorkspace(args.workspaceHandle as string); - if (workspace.kind !== "isolated" || workspace.sessionKey === undefined) { - throw new UobError("INVALID_ARGUMENT", "Knapper cannot stop a default-profile workspace.", { - remediation: "Release the default-profile workspace handle instead.", - }); - } - const result = await stopSession( - workspace.sessionKey, - typeof args.timeoutMs === "number" ? { timeoutMs: args.timeoutMs } : {}, - ); - if (result.state !== "quitFailed" && ctx.currentWorkspaceHandle === workspace.handle) { - ctx.currentWorkspaceHandle = undefined; - ctx.selectTelemetry("default"); - await ctx.bindDefaultWorkspace(); - } - return { - text: - result.state === "quitFailed" - ? `Workspace ${workspace.handle} did not stop. Knapper preserved its files and records.` - : result.state === "notRunning" - ? `Workspace ${workspace.handle} was already stopped.` - : `Stopped workspace ${workspace.handle}.`, - json: { - workspaceHandle: workspace.handle, - stopped: result.state !== "quitFailed", - ...result, - }, - ...(result.state === "quitFailed" ? { isError: true } : {}), - }; - }, - }); - - registry.add({ - name: "obsidian_workspace_release", - toolset: "workspace", - alwaysEnabled: true, - requiresWorkspaceLease: true, - workspaceIndependent: true, - profileIndependent: true, - annotations: { readOnlyHint: false, destructiveHint: true }, - description: "Release a workspace handle. An isolated scratch vault is retained on disk.", - inputSchema: { - workspaceHandle: z - .string() - .describe("Workspace handle to release while retaining its vault."), - }, - handler: async (args) => { - const workspace = await workspaceForCleanup(args.workspaceHandle as string); - let closeResult: unknown; - if (workspace.sessionKey !== undefined) { - try { - closeResult = await releaseSession(workspace.sessionKey); - } catch (error) { - if (!(error instanceof UobError) || error.code !== "SESSION_NOT_FOUND") throw error; - closeResult = { alreadyAbsent: true }; - } - } - await removeWorkspaceRecord(workspace.handle); - await ctx.workspaceLeases.release(workspace.handle); - ctx.selectTelemetry("default"); - if (ctx.currentWorkspaceHandle === workspace.handle) { - ctx.currentWorkspaceHandle = undefined; - await ctx.bindDefaultWorkspace(); - } - const telemetry = - workspace.sessionKey !== undefined - ? await archiveWorkspaceTelemetry( - ctx, - workspace.handle, - sessionPaths(workspace.sessionKey).root, - ) - : {}; - return { - text: `Released workspace ${workspace.handle}. Its vault was not deleted.${ - workspace.sessionKey !== undefined ? telemetryArchiveText(telemetry) : "" - }`, - json: { - workspaceHandle: workspace.handle, - released: true, - closeResult, - ...telemetry, - }, - }; - }, - }); - - registry.add({ - name: "obsidian_workspace_destroy", - toolset: "workspace", - alwaysEnabled: true, - requiresWorkspaceLease: true, - workspaceIndependent: true, - profileIndependent: true, - description: - "Move a stopped isolated workspace's verified scratch root to recoverable Knapper trash.", - annotations: { readOnlyHint: false, destructiveHint: true }, - inputSchema: { - workspaceHandle: z.string().describe("Isolated workspace handle to quarantine and destroy."), - }, - handler: async (args) => { - const workspace = await workspaceForCleanup(args.workspaceHandle as string); - if (workspace.kind !== "isolated" || workspace.sessionKey === undefined) { - throw new UobError( - "INVALID_ARGUMENT", - "Knapper cannot destroy a default-profile workspace.", - { - remediation: "Release the workspace handle instead.", - fixedBy: "obsidian_workspace_release", - }, - ); - } - let result; - try { - result = await quarantineSession(workspace.sessionKey); - } catch (error) { - if (!(error instanceof UobError) || error.code !== "SESSION_NOT_FOUND") throw error; - result = { - key: workspace.sessionKey, - quit: false, - unlinkedPlugin: false, - vaultRemoved: false, - vaultDeleted: false, - rootDeleted: false, - notes: ["The private session was already absent."], - }; - } - await removeWorkspaceRecord(workspace.handle); - await ctx.workspaceLeases.release(workspace.handle); - ctx.selectTelemetry("default"); - if (ctx.currentWorkspaceHandle === workspace.handle) { - ctx.currentWorkspaceHandle = undefined; - await ctx.bindDefaultWorkspace(); - } - let telemetryRoot = result.quarantinedPath; - if (telemetryRoot === undefined) { - telemetryRoot = join(trashDir(), `orphaned-${workspace.handle}-${Date.now()}`); - } - const telemetry = await archiveWorkspaceTelemetry(ctx, workspace.handle, telemetryRoot); - const disposition = - result.quarantinedPath !== undefined - ? `Quarantined workspace ${workspace.handle} at ${result.quarantinedPath}.` - : `Removed stale workspace ${workspace.handle}; its private session was already absent.`; - return { - text: `${disposition}${telemetryArchiveText(telemetry)}`, - json: { - workspaceHandle: workspace.handle, - destroyed: true, - ...result, - ...telemetry, - }, - }; - }, - }); -} diff --git a/src/toolsets.ts b/src/toolsets.ts index dcb8a04..c17131a 100644 --- a/src/toolsets.ts +++ b/src/toolsets.ts @@ -9,7 +9,6 @@ export const TOOLSETS = [ "core", - "workspace", "ui", "telemetry", "plugin-dev", @@ -21,13 +20,17 @@ export const TOOLSETS = [ export type Toolset = (typeof TOOLSETS)[number]; -/** Operational tools are opt-in. Control-plane tools use `alwaysEnabled`. */ -export const DEFAULT_TOOLSETS: readonly Toolset[] = []; +/** Toolsets that every MCP server registers at startup. */ +export const DEFAULT_TOOLSETS: readonly Toolset[] = [ + "core", + "ui", + "telemetry", + "plugin-dev", + "editor", +]; export const TOOLSET_DESCRIPTIONS: Record = { core: "Status, doctor, launch, eval, CLI, and command-palette execution.", - workspace: - "Explicit agent and workspace handles for isolated scratch instances or the default profile.", ui: "Browser automation over CDP (proxied from @playwright/mcp) plus Obsidian-scoped snapshots.", telemetry: "Console, error, and network capture with cursor-based tailing.", "plugin-dev": "Plugin reload, manifest and settings inspection, and dev-cycle composites.", @@ -49,9 +52,8 @@ export interface ToolsetParseResult { } /** - * Parse a comma-separated toolset spec. `all` enables everything. Unknown names are - * collected rather than thrown so the server can warn and continue — a typo in an - * env var should not prevent startup. + * Parse a comma-separated toolset spec. `all` enables every toolset. Unknown names + * are collected so a typo in an environment variable does not prevent startup. */ export function parseToolsets(spec: string | undefined): ToolsetParseResult { if (spec === undefined || spec.trim() === "") { @@ -62,19 +64,18 @@ export function parseToolsets(spec: string | undefined): ToolsetParseResult { .split(",") .map((t) => t.trim().toLowerCase()) .filter((t) => t !== ""); + const unknown = tokens.filter((token) => token !== "all" && !isToolset(token)); if (tokens.includes("all")) { - return { enabled: new Set(TOOLSETS), unknown: [] }; + return { enabled: new Set(TOOLSETS), unknown }; } const enabled = new Set(); - const unknown: string[] = []; for (const token of tokens) { if (isToolset(token)) enabled.add(token); - else unknown.push(token); } - // An all-garbage spec falls back to the empty operational surface. + // An all-garbage spec falls back to the default startup surface. if (enabled.size === 0) return { enabled: new Set(DEFAULT_TOOLSETS), unknown }; return { enabled, unknown }; diff --git a/src/usage/activity-guard.ts b/src/usage/activity-guard.ts new file mode 100644 index 0000000..896216b --- /dev/null +++ b/src/usage/activity-guard.ts @@ -0,0 +1,259 @@ +import { randomUUID } from "node:crypto"; +import { readFile, rm } from "node:fs/promises"; +import { hostname } from "node:os"; +import { join } from "node:path"; +import { knapperHome } from "../config.js"; +import { readPidStartTime } from "../connection/health.js"; +import { writeJsonAtomic } from "../util/atomic-json.js"; +import { UobError } from "../util/errors.js"; +import { withFileLock } from "../util/filelock.js"; + +export interface ActivityRecord { + schema: 1; + token: string; + pid: number; + pidStartTime?: number; + hostname: string; + acquiredAt: string; + lastActivityAt: string; + activeOperations: number; + currentOperation?: string; + sessionOpen: boolean; +} + +export type ActivityState = "free" | "self" | "busy" | "stale"; + +export interface ActivityOwner extends Omit {} + +export interface ActivityStatus { + state: ActivityState; + owner?: ActivityOwner; + lastActivityAt?: string; + sessionOpen?: boolean; + activeOperations?: number; + retryAfterMs?: number; +} + +export interface ActivityGuardOptions { + idleTimeoutMs: number; + env?: NodeJS.ProcessEnv; + now?: () => Date; + pid?: number; + hostname?: string; +} + +export interface AcquireOptions { + operation?: string; + sessionOpen?: boolean; +} + +const recordName = "usage.json"; +const lockName = "usage.lock"; + +/** A single, cross-process activity lane for the managed Obsidian instance. */ +export class ActivityGuard { + private readonly token = randomUUID(); + private readonly env: NodeJS.ProcessEnv; + private readonly now: () => Date; + private readonly pid: number; + private readonly host: string; + private heartbeatTimer?: NodeJS.Timeout; + + constructor(private readonly opts: ActivityGuardOptions) { + this.env = opts.env ?? process.env; + this.now = opts.now ?? (() => new Date()); + this.pid = opts.pid ?? process.pid; + this.host = opts.hostname ?? hostname(); + if (!Number.isFinite(opts.idleTimeoutMs) || opts.idleTimeoutMs <= 0) { + throw new UobError("INVALID_ARGUMENT", "The activity idle timeout must be positive.", { + remediation: "Set KNAP_ACTIVITY_IDLE_MS to a positive number of milliseconds.", + }); + } + } + + private path(): string { + return join(knapperHome(this.env), recordName); + } + + private async read(): Promise { + try { + const value = JSON.parse(await readFile(this.path(), "utf8")) as ActivityRecord; + return value.schema === 1 && typeof value.token === "string" ? value : undefined; + } catch { + return undefined; + } + } + + private async write(value: ActivityRecord): Promise { + await writeJsonAtomic(this.path(), value, { mode: 0o600, directoryMode: 0o700 }); + } + + private async ownerAlive(value: ActivityRecord): Promise { + if (value.hostname !== this.host) return true; + try { + process.kill(value.pid, 0); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EPERM") return false; + } + if (value.pidStartTime === undefined || process.platform !== "linux") return true; + return (await readPidStartTime(value.pid)) === value.pidStartTime; + } + + private expired(value: ActivityRecord, now = this.now()): boolean { + const time = Date.parse(value.lastActivityAt); + return !Number.isFinite(time) || now.getTime() - time >= this.opts.idleTimeoutMs; + } + + private withoutToken(value: ActivityRecord): ActivityOwner { + const { token: _token, ...owner } = value; + return owner; + } + + private retryAfter(value: ActivityRecord): number { + const time = Date.parse(value.lastActivityAt); + return Number.isFinite(time) + ? Math.max(0, time + this.opts.idleTimeoutMs - this.now().getTime()) + : 0; + } + + private startHeartbeat(): void { + if (this.heartbeatTimer !== undefined) clearInterval(this.heartbeatTimer); + const intervalMs = Math.max(1_000, Math.floor(this.opts.idleTimeoutMs / 3)); + this.heartbeatTimer = setInterval(() => void this.heartbeat(), intervalMs); + this.heartbeatTimer.unref(); + } + + private stopHeartbeat(): void { + if (this.heartbeatTimer !== undefined) clearInterval(this.heartbeatTimer); + this.heartbeatTimer = undefined; + } + + async heartbeat(): Promise { + await withFileLock(join(knapperHome(this.env), lockName), async () => { + const existing = await this.read(); + if (existing?.token !== this.token || existing.activeOperations === 0) { + this.stopHeartbeat(); + return; + } + await this.write({ ...existing, lastActivityAt: this.now().toISOString() }); + }); + } + + async status(): Promise { + const value = await this.read(); + if (value === undefined) return { state: "free" }; + const owner = this.withoutToken(value); + const dead = !(await this.ownerAlive(value)); + const old = this.expired(value); + if (dead || old) { + return { + state: "stale", + owner, + lastActivityAt: value.lastActivityAt, + sessionOpen: value.sessionOpen, + activeOperations: value.activeOperations, + retryAfterMs: 0, + }; + } + if (value.token === this.token) { + return { + state: "self", + owner, + lastActivityAt: value.lastActivityAt, + sessionOpen: value.sessionOpen, + activeOperations: value.activeOperations, + }; + } + const busy = value.activeOperations > 0 || value.sessionOpen; + return { + state: busy ? "busy" : "free", + owner: busy ? owner : undefined, + lastActivityAt: value.lastActivityAt, + sessionOpen: value.sessionOpen, + activeOperations: value.activeOperations, + ...(busy ? { retryAfterMs: this.retryAfter(value) } : {}), + }; + } + + async acquire(options: AcquireOptions = {}): Promise { + return withFileLock(join(knapperHome(this.env), lockName), async () => { + const now = this.now(); + const existing = await this.read(); + const own = existing?.token === this.token; + const reclaimable = + existing === undefined || + (existing !== undefined && (!(await this.ownerAlive(existing)) || this.expired(existing))); + if ( + !own && + !reclaimable && + existing !== undefined && + (existing.activeOperations > 0 || existing.sessionOpen) + ) { + throw new UobError( + "KNAPPER_BUSY", + "Another Knapper agent is using the managed Obsidian instance.", + { + remediation: "Wait for the other agent to finish, then retry.", + details: { + owner: this.withoutToken(existing), + retryAfterMs: this.retryAfter(existing), + }, + }, + ); + } + const record: ActivityRecord = { + schema: 1, + token: this.token, + pid: this.pid, + ...(process.platform === "linux" ? { pidStartTime: await readPidStartTime(this.pid) } : {}), + hostname: this.host, + acquiredAt: own && existing ? existing.acquiredAt : now.toISOString(), + lastActivityAt: now.toISOString(), + activeOperations: own && existing ? existing.activeOperations + 1 : 1, + ...(options.operation ? { currentOperation: options.operation } : {}), + sessionOpen: options.sessionOpen ?? (own && existing ? existing.sessionOpen : false), + }; + await this.write(record); + this.startHeartbeat(); + return record; + }); + } + + async complete(sessionOpen?: boolean): Promise { + await withFileLock(join(knapperHome(this.env), lockName), async () => { + const existing = await this.read(); + if (existing?.token !== this.token) { + this.stopHeartbeat(); + return; + } + const activeOperations = Math.max(0, existing.activeOperations - 1); + const { currentOperation, ...record } = existing; + await this.write({ + ...record, + lastActivityAt: this.now().toISOString(), + activeOperations, + ...(activeOperations > 0 && currentOperation !== undefined ? { currentOperation } : {}), + ...(sessionOpen === undefined ? {} : { sessionOpen }), + }); + if (activeOperations === 0) this.stopHeartbeat(); + }); + } + + async release(): Promise { + await withFileLock(join(knapperHome(this.env), lockName), async () => { + const existing = await this.read(); + if (existing?.token !== this.token) { + this.stopHeartbeat(); + return; + } + if (existing.activeOperations > 0 || existing.sessionOpen) { + throw new UobError("KNAPPER_BUSY", "The managed session is still active.", { + remediation: + "Close the managed session and complete all active operations before releasing the guard.", + }); + } + this.stopHeartbeat(); + await rm(this.path(), { force: true }); + }); + } +} diff --git a/src/util/concurrency.ts b/src/util/concurrency.ts index 0cf07d5..f99ebf7 100644 --- a/src/util/concurrency.ts +++ b/src/util/concurrency.ts @@ -85,8 +85,7 @@ function queueTimeout(label: string, mode: LockMode, waitedMs: number, stats: Lo "Tool calls are serialized because they drive one live Obsidian window, so this call was " + "queued behind another that has not finished. A tool is most likely wedged on the UI — an " + "open modal or dialog swallowing input is the usual cause. Take a screenshot or snapshot to " + - "see the current window state, dismiss anything blocking, then retry. Raise " + - "KNAP_MAX_CONCURRENCY only if the blocked calls are read-only.", + "see the current window state, dismiss anything blocking, then retry.", details: { tool: label, mode, diff --git a/src/util/errors.ts b/src/util/errors.ts index e1f76f7..d2f7a3e 100644 --- a/src/util/errors.ts +++ b/src/util/errors.ts @@ -21,6 +21,8 @@ export type ErrorCode = | "TIMEOUT" | "INVALID_ARGUMENT" | "PLUGIN_NOT_FOUND" + /** The requested plugin directory is not a loadable Obsidian artifact. */ + | "PLUGIN_ARTIFACT_INVALID" /** Refused to *delete* a vault knapper did not create. Never downgrade this. */ | "VAULT_NOT_MANAGED" /** @@ -46,6 +48,8 @@ export type ErrorCode = | "OBSIDIAN_LAUNCH_FAILED" /** Another MCP server currently owns the installation's default profile. */ | "DEFAULT_PROFILE_BUSY" + /** Another live Knapper process owns the single managed Obsidian lane. */ + | "KNAPPER_BUSY" | "WORKSPACE_BUSY" | "INTERNAL"; diff --git a/src/workspace/lease.ts b/src/workspace/lease.ts deleted file mode 100644 index d69a852..0000000 --- a/src/workspace/lease.ts +++ /dev/null @@ -1,193 +0,0 @@ -/** Exclusive workspace ownership shared by all Knapper processes. */ - -import { randomUUID } from "node:crypto"; -import { readFile, rm } from "node:fs/promises"; -import { hostname } from "node:os"; -import { join } from "node:path"; -import { knapperHome } from "../config.js"; -import { readPidStartTime } from "../connection/health.js"; -import { UobError } from "../util/errors.js"; -import { withFileLock } from "../util/filelock.js"; -import { writeFileAtomic } from "../util/atomic-json.js"; - -const WORKSPACE_HANDLE = /^wsp_[A-Za-z0-9_-]{32}$/; - -export interface WorkspaceLeaseRecord { - schema: 1; - workspaceHandle: string; - token: string; - pid: number; - pidStartTime?: number; - hostname: string; - cwd: string; - acquiredAt: string; - lastActivityAt: string; - expiresAt: string; -} - -export interface WorkspaceLeaseOwner extends Omit {} - -export interface WorkspaceLeaseStatus { - state: "free" | "owned" | "busy" | "expired"; - owner?: WorkspaceLeaseOwner; - retryAfterMs?: number; -} - -export interface WorkspaceLeaseManagerOptions { - idleTimeoutMs: number; - env?: NodeJS.ProcessEnv; - now?: () => Date; - pid?: number; - cwd?: string; - hostname?: string; -} - -function leaseDir(env: NodeJS.ProcessEnv): string { - return join(knapperHome(env), "workspace-leases"); -} - -function leasePath(workspaceHandle: string, env: NodeJS.ProcessEnv): string { - if (!WORKSPACE_HANDLE.test(workspaceHandle)) { - throw new UobError("INVALID_ARGUMENT", "The workspace handle is malformed.", { - remediation: "Create a workspace with obsidian_workspace_create.", - }); - } - return join(leaseDir(env), `${workspaceHandle}.json`); -} - -function processAlive(pid: number): boolean { - try { - process.kill(pid, 0); - return true; - } catch (error) { - return (error as NodeJS.ErrnoException).code === "EPERM"; - } -} - -/** One manager represents one MCP server and can own more than one workspace. */ -export class WorkspaceLeaseManager { - private readonly token = randomUUID(); - private readonly env: NodeJS.ProcessEnv; - private readonly now: () => Date; - private readonly pid: number; - private readonly cwd: string; - private readonly host: string; - private readonly owned = new Set(); - - constructor(private readonly opts: WorkspaceLeaseManagerOptions) { - this.env = opts.env ?? process.env; - this.now = opts.now ?? (() => new Date()); - this.pid = opts.pid ?? process.pid; - this.cwd = opts.cwd ?? process.cwd(); - this.host = opts.hostname ?? hostname(); - } - - private async read(workspaceHandle: string): Promise { - try { - const parsed = JSON.parse( - await readFile(leasePath(workspaceHandle, this.env), "utf8"), - ) as WorkspaceLeaseRecord; - return parsed.schema === 1 && parsed.workspaceHandle === workspaceHandle ? parsed : undefined; - } catch (error) { - if (error instanceof UobError) throw error; - return undefined; - } - } - - private async write(record: WorkspaceLeaseRecord): Promise { - const path = leasePath(record.workspaceHandle, this.env); - await writeFileAtomic(path, `${JSON.stringify(record, null, 2)}\n`, { - mode: 0o600, - directoryMode: 0o700, - }); - } - - private async ownerAlive(record: WorkspaceLeaseRecord): Promise { - if (record.hostname !== this.host) return true; - if (!processAlive(record.pid)) return false; - if (record.pidStartTime === undefined || process.platform !== "linux") return true; - return (await readPidStartTime(record.pid)) === record.pidStartTime; - } - - private withoutToken(record: WorkspaceLeaseRecord): WorkspaceLeaseOwner { - const { token: _token, ...owner } = record; - return owner; - } - - private expired(record: WorkspaceLeaseRecord, now: Date): boolean { - const expiresAt = Date.parse(record.expiresAt); - return !Number.isFinite(expiresAt) || expiresAt <= now.getTime(); - } - - async status(workspaceHandle: string): Promise { - const record = await this.read(workspaceHandle); - if (record === undefined) return { state: "free" }; - const retryAfterMs = Math.max(0, Date.parse(record.expiresAt) - this.now().getTime()); - if (!(await this.ownerAlive(record)) || this.expired(record, this.now())) { - return { state: "expired", owner: this.withoutToken(record), retryAfterMs: 0 }; - } - if (record.token === this.token) { - return { state: "owned", owner: this.withoutToken(record) }; - } - return { state: "busy", owner: this.withoutToken(record), retryAfterMs }; - } - - /** Acquire or renew exclusive ownership of one workspace. */ - async acquire(workspaceHandle: string, tool?: string): Promise { - const path = leasePath(workspaceHandle, this.env); - return withFileLock(`${path}.lock`, async () => { - const now = this.now(); - const existing = await this.read(workspaceHandle); - const own = existing?.token === this.token; - const available = - existing === undefined || - own || - this.expired(existing, now) || - !(await this.ownerAlive(existing)); - if (!available && existing !== undefined) { - throw new UobError("WORKSPACE_BUSY", `Workspace ${workspaceHandle} is in use.`, { - remediation: - "Wait for the current owner to release the workspace, or create another workspace.", - fixedBy: "obsidian_workspace_create", - details: { - ...(tool !== undefined ? { tool } : {}), - workspaceHandle, - owner: this.withoutToken(existing), - retryAfterMs: Math.max(0, Date.parse(existing.expiresAt) - now.getTime()), - }, - }); - } - - const record: WorkspaceLeaseRecord = { - schema: 1, - workspaceHandle, - token: this.token, - pid: this.pid, - ...(process.platform === "linux" ? { pidStartTime: await readPidStartTime(this.pid) } : {}), - hostname: this.host, - cwd: this.cwd, - acquiredAt: own && existing !== undefined ? existing.acquiredAt : now.toISOString(), - lastActivityAt: now.toISOString(), - expiresAt: new Date(now.getTime() + this.opts.idleTimeoutMs).toISOString(), - }; - await this.write(record); - this.owned.add(workspaceHandle); - return record; - }); - } - - /** Release one lease only when this manager still owns its token. */ - async release(workspaceHandle: string): Promise { - const path = leasePath(workspaceHandle, this.env); - await withFileLock(`${path}.lock`, async () => { - const existing = await this.read(workspaceHandle); - if (existing?.token === this.token) await rm(path, { force: true }); - }); - this.owned.delete(workspaceHandle); - } - - /** Release all workspace leases held by this MCP server. */ - async releaseAll(): Promise { - await Promise.all([...this.owned].map((handle) => this.release(handle))); - } -} diff --git a/src/workspace/store.ts b/src/workspace/store.ts deleted file mode 100644 index 1bf143e..0000000 --- a/src/workspace/store.ts +++ /dev/null @@ -1,148 +0,0 @@ -/** Explicit workspace handles that survive MCP transport and process boundaries. */ - -import { randomBytes } from "node:crypto"; -import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; -import { join } from "node:path"; -import { workspacesDir } from "../config.js"; -import { requireAgent } from "../agent/store.js"; -import { UobError } from "../util/errors.js"; -import { withFileLock } from "../util/filelock.js"; - -const WORKSPACE_HANDLE = /^wsp_[A-Za-z0-9_-]{32}$/; -export const WORKSPACE_IDLE_MS = 24 * 60 * 60 * 1000; - -export interface WorkspaceRecord { - schema: 1; - handle: string; - agentHandle: string; - kind: "isolated" | "default"; - sessionKey?: string; - label?: string; - createdAt: string; - lastActivityAt: string; - expiresAt: string; -} - -function workspacePath(handle: string, env: NodeJS.ProcessEnv): string { - if (!WORKSPACE_HANDLE.test(handle)) { - throw new UobError("INVALID_ARGUMENT", "The workspace handle is malformed.", { - remediation: - "Create a workspace with obsidian_workspace_create or claim the default profile.", - }); - } - return join(workspacesDir(env), `${handle}.json`); -} - -async function writeWorkspace(record: WorkspaceRecord, env: NodeJS.ProcessEnv): Promise { - const dir = workspacesDir(env); - await mkdir(dir, { recursive: true, mode: 0o700 }); - const path = workspacePath(record.handle, env); - const tmp = `${path}.${process.pid}.${randomBytes(6).toString("hex")}.tmp`; - await writeFile(tmp, `${JSON.stringify(record, null, 2)}\n`, { encoding: "utf8", mode: 0o600 }); - await rename(tmp, path); -} - -export async function createWorkspaceRecord(opts: { - agentHandle: string; - kind: "isolated" | "default"; - sessionKey?: string; - label?: string; - env?: NodeJS.ProcessEnv; - now?: Date; -}): Promise { - const env = opts.env ?? process.env; - await requireAgent(opts.agentHandle, env); - const now = opts.now ?? new Date(); - const record: WorkspaceRecord = { - schema: 1, - handle: `wsp_${randomBytes(24).toString("base64url")}`, - agentHandle: opts.agentHandle, - kind: opts.kind, - ...(opts.sessionKey !== undefined ? { sessionKey: opts.sessionKey } : {}), - ...(opts.label !== undefined ? { label: opts.label } : {}), - createdAt: now.toISOString(), - lastActivityAt: now.toISOString(), - expiresAt: new Date(now.getTime() + WORKSPACE_IDLE_MS).toISOString(), - }; - await writeWorkspace(record, env); - return record; -} - -export async function readWorkspace( - handle: string, - env: NodeJS.ProcessEnv = process.env, -): Promise { - try { - const record = JSON.parse( - await readFile(workspacePath(handle, env), "utf8"), - ) as WorkspaceRecord; - return record.schema === 1 && record.handle === handle ? record : undefined; - } catch (error) { - if (error instanceof UobError) throw error; - return undefined; - } -} - -export async function requireWorkspace( - handle: string, - env: NodeJS.ProcessEnv = process.env, -): Promise { - const record = await readWorkspace(handle, env); - if (record === undefined) { - throw new UobError("SESSION_NOT_FOUND", `Workspace ${handle} does not exist.`, { - remediation: "Create a workspace with obsidian_workspace_create.", - fixedBy: "obsidian_workspace_create", - }); - } - const expiresAt = Date.parse(record.expiresAt); - if (!Number.isFinite(expiresAt) || expiresAt <= Date.now()) { - throw new UobError("SESSION_NOT_FOUND", `Workspace ${handle} expired.`, { - remediation: "Create or claim a new workspace handle.", - fixedBy: "obsidian_workspace_create", - }); - } - return record; -} - -export async function touchWorkspace( - handle: string, - env: NodeJS.ProcessEnv = process.env, -): Promise { - return withFileLock(`${workspacePath(handle, env)}.lock`, async () => { - const record = await requireWorkspace(handle, env); - const now = new Date(); - const next = { - ...record, - lastActivityAt: now.toISOString(), - expiresAt: new Date(now.getTime() + WORKSPACE_IDLE_MS).toISOString(), - }; - await writeWorkspace(next, env); - return next; - }); -} - -export async function listWorkspaces( - env: NodeJS.ProcessEnv = process.env, -): Promise { - let names: string[]; - try { - names = await readdir(workspacesDir(env)); - } catch { - return []; - } - const records = await Promise.all( - names - .filter((name) => name.endsWith(".json")) - .map((name) => readWorkspace(name.slice(0, -5), env).catch(() => undefined)), - ); - return records - .filter((record): record is WorkspaceRecord => record !== undefined) - .sort((a, b) => a.createdAt.localeCompare(b.createdAt)); -} - -export async function removeWorkspaceRecord( - handle: string, - env: NodeJS.ProcessEnv = process.env, -): Promise { - await rm(workspacePath(handle, env), { force: true }); -} diff --git a/tests/fixtures/settings-plugin/main.js b/tests/fixtures/settings-plugin/main.js new file mode 100644 index 0000000..6b267e7 --- /dev/null +++ b/tests/fixtures/settings-plugin/main.js @@ -0,0 +1,32 @@ +const { Plugin, PluginSettingTab, Setting } = require("obsidian"); + +class FixtureSettingsTab extends PluginSettingTab { + display() { + const { containerEl } = this; + containerEl.empty(); + containerEl.addClass("knapper-settings-fixture"); + new Setting(containerEl) + .setName("Enable fixture") + .setDesc("Live UI state used by the Knapper acceptance suite.") + .addToggle((toggle) => + toggle.setValue(this.plugin.settings.enabled).onChange(async (enabled) => { + this.plugin.settings.enabled = enabled; + await this.plugin.saveData(this.plugin.settings); + }), + ); + } +} + +module.exports = class KnapperSettingsFixture extends Plugin { + async onload() { + this.settings = Object.assign({ enabled: false }, await this.loadData()); + this.addSettingTab(new FixtureSettingsTab(this.app, this)); + this.addCommand({ + id: "throw-on-purpose", + name: "Throw on purpose", + callback: () => { + throw new Error("knapper-settings-fixture deliberate error"); + }, + }); + } +}; diff --git a/tests/fixtures/settings-plugin/manifest.json b/tests/fixtures/settings-plugin/manifest.json new file mode 100644 index 0000000..e329d4e --- /dev/null +++ b/tests/fixtures/settings-plugin/manifest.json @@ -0,0 +1,9 @@ +{ + "id": "knapper-settings-fixture", + "name": "Knapper settings fixture", + "version": "1.0.0", + "minAppVersion": "1.8.0", + "description": "Live settings UI fixture for Knapper.", + "author": "bearfire-dev", + "isDesktopOnly": true +} diff --git a/tests/unit/activity-guard.test.ts b/tests/unit/activity-guard.test.ts new file mode 100644 index 0000000..2cfe49f --- /dev/null +++ b/tests/unit/activity-guard.test.ts @@ -0,0 +1,127 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { ActivityGuard } from "../../src/usage/activity-guard.js"; + +let root: string; +let env: NodeJS.ProcessEnv; + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "knap-activity-")); + env = { KNAP_HOME: root }; +}); + +afterEach(async () => { + await rm(root, { recursive: true, force: true }); +}); + +describe("ActivityGuard", () => { + it("starts free, tracks an operation, and does not renew from status", async () => { + let now = new Date("2026-08-08T12:00:00Z"); + const first = new ActivityGuard({ + idleTimeoutMs: 30_000, + env, + now: () => now, + pid: process.pid, + }); + expect((await first.status()).state).toBe("free"); + await first.acquire({ operation: "browser_click", sessionOpen: true }); + expect((await first.status()).state).toBe("self"); + now = new Date("2026-08-08T12:00:10Z"); + await first.status(); + now = new Date("2026-08-08T12:00:31Z"); + expect((await first.status()).state).toBe("stale"); + }); + + it("reports a live session as busy to another process and includes retry time", async () => { + const first = new ActivityGuard({ + idleTimeoutMs: 30_000, + env, + pid: process.pid, + hostname: "host-a", + }); + await first.acquire({ sessionOpen: true }); + const second = new ActivityGuard({ + idleTimeoutMs: 30_000, + env, + pid: process.pid, + hostname: "host-b", + }); + const status = await second.status(); + expect(status).toMatchObject({ + state: "busy", + sessionOpen: true, + retryAfterMs: expect.any(Number), + }); + await expect(second.acquire()).rejects.toMatchObject({ code: "KNAPPER_BUSY" }); + }); + + it("completes operations, keeps session state, and releases its own record", async () => { + const guard = new ActivityGuard({ idleTimeoutMs: 30_000, env, pid: process.pid }); + await guard.acquire({ sessionOpen: true }); + await guard.complete(false); + expect(await guard.status()).toMatchObject({ + state: "self", + activeOperations: 0, + sessionOpen: false, + }); + expect((await guard.status()).owner).not.toHaveProperty("currentOperation"); + await guard.release(); + expect((await guard.status()).state).toBe("free"); + expect(await readFile(join(root, "usage.json")).catch(() => undefined)).toBeUndefined(); + }); + + it("reclaims an expired owner", async () => { + let now = new Date("2026-08-08T12:00:00Z"); + const first = new ActivityGuard({ + idleTimeoutMs: 30_000, + env, + now: () => now, + pid: process.pid, + hostname: "host-a", + }); + await first.acquire({ sessionOpen: true }); + now = new Date("2026-08-08T12:00:31Z"); + const second = new ActivityGuard({ + idleTimeoutMs: 30_000, + env, + now: () => now, + pid: process.pid, + hostname: "host-b", + }); + await expect(second.acquire({ sessionOpen: false })).resolves.toBeDefined(); + expect((await second.status()).state).toBe("self"); + }); + + it("renews ownership while an operation is active", async () => { + let now = new Date("2026-08-08T12:00:00Z"); + const guard = new ActivityGuard({ + idleTimeoutMs: 30_000, + env, + now: () => now, + pid: process.pid, + }); + await guard.acquire({ operation: "long-running-test", sessionOpen: true }); + + now = new Date("2026-08-08T12:00:25Z"); + await guard.heartbeat(); + now = new Date("2026-08-08T12:00:31Z"); + expect((await guard.status()).state).toBe("self"); + + await guard.complete(false); + await guard.release(); + }); + + it("refuses to release ownership while the managed session is open", async () => { + const guard = new ActivityGuard({ idleTimeoutMs: 30_000, env, pid: process.pid }); + await guard.acquire({ sessionOpen: true }); + await guard.complete(); + + await expect(guard.release()).rejects.toMatchObject({ code: "KNAPPER_BUSY" }); + + await guard.acquire({ sessionOpen: false }); + await guard.complete(false); + await guard.release(); + }); +}); diff --git a/tests/unit/agent-workspace-store.test.ts b/tests/unit/agent-workspace-store.test.ts index d108241..917c065 100644 --- a/tests/unit/agent-workspace-store.test.ts +++ b/tests/unit/agent-workspace-store.test.ts @@ -1,161 +1,29 @@ -import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises"; -import { join } from "node:path"; -import { tmpdir } from "node:os"; -import { - closeAgent, - openAgent, - readAgent, - requireAgent, - touchAgent, -} from "../../src/agent/store.js"; -import { - createWorkspaceRecord, - readWorkspace, - removeWorkspaceRecord, - requireWorkspace, - touchWorkspace, -} from "../../src/workspace/store.js"; -import { agentsDir, workspacesDir } from "../../src/config.js"; - -let home: string; -let env: NodeJS.ProcessEnv; - -beforeEach(async () => { - home = await mkdtemp(join(tmpdir(), "knap-agent-workspace-")); - env = { ...process.env, KNAP_HOME: home }; -}); - -afterEach(async () => { - await rm(home, { recursive: true, force: true }); -}); - -describe("agent and workspace handles", () => { - it("mints opaque 192-bit handles and stores private records", async () => { - const agent = await openAgent({ label: "test agent", env }); - const workspace = await createWorkspaceRecord({ - agentHandle: agent.handle, - kind: "isolated", - sessionKey: "test-a3f19c22", - env, - }); - - expect(agent.handle).toMatch(/^agt_[A-Za-z0-9_-]{32}$/); - expect(workspace.handle).toMatch(/^wsp_[A-Za-z0-9_-]{32}$/); - expect((await stat(join(agentsDir(env), `${agent.handle}.json`))).mode & 0o777).toBe(0o600); - expect((await stat(join(workspacesDir(env), `${workspace.handle}.json`))).mode & 0o777).toBe( - 0o600, - ); - }); - - it("records observed client software without treating it as identity", async () => { - const agent = await openAgent({ label: "client audit", env }); - await touchAgent(agent.handle, { name: "opencode", version: "1.2.3" }, env); - const updated = await readAgent(agent.handle, env); - expect(updated?.observedClients).toMatchObject([{ name: "opencode", version: "1.2.3" }]); - expect(updated?.handle).toBe(agent.handle); - }); - - it("keeps concurrent touches readable and extends both leases", async () => { - const agent = await openAgent({ label: "concurrent", env }); - const workspace = await createWorkspaceRecord({ - agentHandle: agent.handle, - kind: "default", - env, - }); - - await Promise.all([ - touchAgent(agent.handle, { name: "opencode", version: "1" }, env), - touchAgent(agent.handle, { name: "kimi", version: "1" }, env), - touchWorkspace(workspace.handle, env), - ]); - - expect((await readAgent(agent.handle, env))?.observedClients).toEqual( - expect.arrayContaining([ - expect.objectContaining({ name: "opencode", version: "1" }), - expect.objectContaining({ name: "kimi", version: "1" }), - ]), - ); - expect( - JSON.parse(await readFile(join(agentsDir(env), `${agent.handle}.json`), "utf8")), - ).toBeTruthy(); - expect(await requireWorkspace(workspace.handle, env)).toMatchObject({ - agentHandle: agent.handle, - kind: "default", - }); - }); - - it("expires stale workspace handles", async () => { - const agent = await openAgent({ label: "expired", env }); - const workspace = await createWorkspaceRecord({ - agentHandle: agent.handle, - kind: "default", - now: new Date("2000-01-01T00:00:00.000Z"), - env, - }); - - await expect(requireWorkspace(workspace.handle, env)).rejects.toMatchObject({ - code: "SESSION_NOT_FOUND", - }); - }); - - it("fails closed when persisted expiry dates are malformed", async () => { - const agent = await openAgent({ label: "malformed", env }); - const workspace = await createWorkspaceRecord({ - agentHandle: agent.handle, - kind: "default", - env, - }); - await writeFile( - join(agentsDir(env), `${agent.handle}.json`), - JSON.stringify({ ...agent, expiresAt: "not-a-date" }), - ); - await writeFile( - join(workspacesDir(env), `${workspace.handle}.json`), - JSON.stringify({ ...workspace, expiresAt: "not-a-date" }), - ); - - await expect(requireAgent(agent.handle, env)).rejects.toMatchObject({ - code: "INVALID_ARGUMENT", - }); - await expect(requireWorkspace(workspace.handle, env)).rejects.toMatchObject({ - code: "SESSION_NOT_FOUND", - }); - }); - - it("allows an expired agent record to be closed", async () => { - const agent = await openAgent({ - label: "expired agent", - now: new Date("2000-01-01T00:00:00.000Z"), - env, - }); - - await expect(closeAgent(agent.handle, env)).resolves.toBe(true); - expect(await readAgent(agent.handle, env)).toBeUndefined(); - }); - - it("removes records without touching unrelated state", async () => { - const agent = await openAgent({ label: "cleanup", env }); - const unrelatedAgent = await openAgent({ label: "unrelated", env }); - const workspace = await createWorkspaceRecord({ - agentHandle: agent.handle, - kind: "default", - env, - }); - const unrelatedWorkspace = await createWorkspaceRecord({ - agentHandle: unrelatedAgent.handle, - kind: "default", - env, - }); - await removeWorkspaceRecord(workspace.handle, env); - await closeAgent(agent.handle, env); - expect(await readWorkspace(workspace.handle, env)).toBeUndefined(); - expect(await readAgent(agent.handle, env)).toBeUndefined(); - expect(await readWorkspace(unrelatedWorkspace.handle, env)).toMatchObject({ - agentHandle: unrelatedAgent.handle, - }); - expect(await readAgent(unrelatedAgent.handle, env)).toMatchObject({ - handle: unrelatedAgent.handle, - }); +import { describe, expect, it } from "vitest"; +import { readFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const root = join(dirname(fileURLToPath(import.meta.url)), "../.."); + +describe("singleton session contract", () => { + it("exposes session lifecycle tools instead of agent and workspace handles", async () => { + const source = await readFile(join(root, "src", "server.ts"), "utf8"); + expect(source).toContain("obsidian_session_open"); + expect(source).toContain("never require a handle"); + + const sessionToolPath = join(root, "src", "tools", "session.ts"); + try { + const sessionTools = await readFile(sessionToolPath, "utf8"); + for (const name of [ + "obsidian_session_open", + "obsidian_session_status", + "obsidian_session_release", + "obsidian_session_reset", + ]) { + expect(sessionTools).toContain(name); + } + } catch { + // The lifecycle module can land after the server instruction contract. + } }); }); diff --git a/tests/unit/audit.test.ts b/tests/unit/audit.test.ts index e4a0c50..cf5914d 100644 --- a/tests/unit/audit.test.ts +++ b/tests/unit/audit.test.ts @@ -33,8 +33,6 @@ describe("audit persistence", () => { args: {}, context: { clientInfo: { name: "user@example.com", version: "secret-version" }, - agentHandle: "agent-secret", - workspaceHandle: "workspace-secret", traceId: "trace-secret", }, }); @@ -42,8 +40,6 @@ describe("audit persistence", () => { expect(event.request_id).toMatch(/^sha256:/); expect(event.trace_id).toMatch(/^sha256:/); expect(event.client?.name).toMatch(/^sha256:/); - expect(event.agent_handle).toMatch(/^sha256:/); - expect(event.workspace_handle).toMatch(/^sha256:/); expect(JSON.stringify(event)).not.toMatch(/example\.com|secret/); }); diff --git a/tests/unit/config.test.ts b/tests/unit/config.test.ts index 63447a7..17c73aa 100644 --- a/tests/unit/config.test.ts +++ b/tests/unit/config.test.ts @@ -17,6 +17,12 @@ describe("parseToolsets", () => { expect(parseToolsets("all").enabled.size).toBe(TOOLSETS.length); }); + it("reports unknown names when all is present", () => { + const result = parseToolsets("all,typo"); + expect(result.enabled.size).toBe(TOOLSETS.length); + expect(result.unknown).toEqual(["typo"]); + }); + it("parses a comma-separated list, tolerating whitespace and case", () => { const { enabled } = parseToolsets(" Core , VAULT "); expect([...enabled].sort()).toEqual(["core", "vault"]); @@ -132,11 +138,10 @@ describe("loadConfig", () => { expect(loadConfig({}, {}).targetMatch).toBeUndefined(); }); - it("clamps concurrency to at least one, so a zero cannot wedge every tool call", () => { - expect(loadConfig({}, {}).maxConcurrency).toBe(4); - expect(loadConfig({}, { KNAP_MAX_CONCURRENCY: "1" }).maxConcurrency).toBe(1); - expect(loadConfig({}, { KNAP_MAX_CONCURRENCY: "0" }).maxConcurrency).toBe(1); - expect(loadConfig({}, { KNAP_MAX_CONCURRENCY: "nope" }).maxConcurrency).toBe(4); + it("uses a five-minute single-user activity window by default", () => { + expect(loadConfig({}, {}).activityIdleMs).toBe(5 * 60_000); + expect(loadConfig({}, { KNAP_ACTIVITY_IDLE_MS: "60000" }).activityIdleMs).toBe(60_000); + expect(loadConfig({}, { KNAP_ACTIVITY_IDLE_MS: "0" }).activityIdleMs).toBe(30_000); }); it("accepts the plan's canonical env names alongside the KNAP_ prefixed ones", () => { diff --git a/tests/unit/default-profile-lease.test.ts b/tests/unit/default-profile-lease.test.ts deleted file mode 100644 index b7c8509..0000000 --- a/tests/unit/default-profile-lease.test.ts +++ /dev/null @@ -1,148 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { DefaultProfileLease } from "../../src/session/default-profile-lease.js"; -import { defaultProfileLeasePath } from "../../src/config.js"; - -let root: string; -let env: NodeJS.ProcessEnv; - -beforeEach(async () => { - root = await mkdtemp(join(tmpdir(), "knap-profile-lease-")); - env = { KNAP_HOME: root }; -}); - -afterEach(async () => { - await rm(root, { recursive: true, force: true }); -}); - -describe("DefaultProfileLease", () => { - it("allows one owner and gives the contender an actionable error", async () => { - const first = new DefaultProfileLease({ idleTimeoutMs: 60_000, env }); - const second = new DefaultProfileLease({ idleTimeoutMs: 60_000, env }); - let release!: () => void; - let acquired!: () => void; - const gate = new Promise((resolve) => { - release = resolve; - }); - const acquisition = new Promise((resolve) => { - acquired = resolve; - }); - const held = first.run("obsidian_command", async () => { - acquired(); - await gate; - }); - await acquisition; - - await expect(second.run("browser_click", async () => undefined)).rejects.toMatchObject({ - code: "DEFAULT_PROFILE_BUSY", - fixedBy: "obsidian_workspace_create", - details: expect.objectContaining({ tool: "browser_click" }), - }); - - release(); - await held; - await first.release(); - }); - - it("reclaims an idle lease after the configured grace", async () => { - let now = new Date("2026-08-04T12:00:00Z"); - const clock = () => now; - const first = new DefaultProfileLease({ idleTimeoutMs: 30_000, env, now: clock }); - await first.run("obsidian_status", async () => undefined); - now = new Date(now.getTime() + 31_000); - - const second = new DefaultProfileLease({ idleTimeoutMs: 30_000, env, now: clock }); - await expect(second.run("obsidian_command", async () => "ok")).resolves.toBe("ok"); - await second.release(); - }); - - it("releases only its own record", async () => { - const first = new DefaultProfileLease({ idleTimeoutMs: 30_000, env }); - await first.run("obsidian_status", async () => undefined); - const second = new DefaultProfileLease({ idleTimeoutMs: 30_000, env }); - await second.release(); - expect((await first.status()).state).toBe("owned"); - await first.release(); - }); - - it("reclaims an expired active lease from another host", async () => { - let now = new Date("2026-08-04T12:00:00Z"); - const clock = () => now; - let acquired!: () => void; - let release!: () => void; - const acquisition = new Promise((resolve) => { - acquired = resolve; - }); - const gate = new Promise((resolve) => { - release = resolve; - }); - const remote = new DefaultProfileLease({ - idleTimeoutMs: 30_000, - env, - now: clock, - hostname: "remote-host", - }); - const held = remote.run("browser_click", async () => { - acquired(); - await gate; - }); - await acquisition; - now = new Date(now.getTime() + 31_000); - - const local = new DefaultProfileLease({ - idleTimeoutMs: 30_000, - env, - now: clock, - hostname: "local-host", - }); - await expect(local.run("obsidian_command", async () => "ok")).resolves.toBe("ok"); - release(); - await held; - await local.release(); - }); - - it("fails closed when the lease record is malformed", async () => { - const path = defaultProfileLeasePath(env); - await mkdir(root, { recursive: true }); - await writeFile( - path, - JSON.stringify({ - token: "untrusted", - pid: process.pid, - hostname: "remote-host", - cwd: root, - acquiredAt: new Date().toISOString(), - lastActivityAt: new Date().toISOString(), - expiresAt: "not-a-date", - activeCalls: 1, - }), - ); - const lease = new DefaultProfileLease({ idleTimeoutMs: 30_000, env }); - - await expect(lease.run("obsidian_status", async () => "ok")).rejects.toMatchObject({ - code: "DEFAULT_PROFILE_BUSY", - }); - }); - - it("surfaces a failed active-call decrement", async () => { - const lease = new DefaultProfileLease({ idleTimeoutMs: 30_000, env }); - Object.assign(lease, { - touch: vi.fn().mockRejectedValue(new Error("lease write failed")), - }); - - await expect(lease.run("obsidian_status", async () => "ok")).rejects.toThrow( - "lease write failed", - ); - }); - - it("does not swallow an undefined rejection", async () => { - const lease = new DefaultProfileLease({ idleTimeoutMs: 30_000, env }); - - await expect(lease.run("obsidian_status", () => Promise.reject(undefined))).rejects.toBe( - undefined, - ); - await lease.release(); - }); -}); diff --git a/tests/unit/plugin-dev-truth.test.ts b/tests/unit/plugin-dev-truth.test.ts index e819ea6..ac6b9e1 100644 --- a/tests/unit/plugin-dev-truth.test.ts +++ b/tests/unit/plugin-dev-truth.test.ts @@ -54,7 +54,7 @@ function outcomeText(outcome: ToolOutcome): string { } function registryFor(...toolsets: ("core" | "plugin-dev" | "authoring")[]): ToolRegistry { - return new ToolRegistry(new Set(toolsets), createLogger("error"), 2); + return new ToolRegistry(new Set(toolsets), createLogger("error")); } function context( diff --git a/tests/unit/plugin-link.test.ts b/tests/unit/plugin-link.test.ts index 7070a31..8f56aa4 100644 --- a/tests/unit/plugin-link.test.ts +++ b/tests/unit/plugin-link.test.ts @@ -32,20 +32,23 @@ describe("validatePluginDir", () => { it("rejects a missing manifest", async () => { await expect(validatePluginDir(await pluginDir({ main: true }))).rejects.toMatchObject({ - code: "INVALID_ARGUMENT", + code: "PLUGIN_ARTIFACT_INVALID", }); }); it("rejects a missing main.js", async () => { await expect( validatePluginDir(await pluginDir({ manifest: { id: "demo" } })), - ).rejects.toMatchObject({ code: "INVALID_ARGUMENT", details: { missing: ["main.js"] } }); + ).rejects.toMatchObject({ + code: "PLUGIN_ARTIFACT_INVALID", + details: { missing: ["main.js"] }, + }); }); it("rejects an explicit id that differs from the manifest", async () => { const dir = await pluginDir({ manifest: { id: "demo" }, main: true }); await expect(validatePluginDir(dir, "other")).rejects.toMatchObject({ - code: "INVALID_ARGUMENT", + code: "PLUGIN_ARTIFACT_INVALID", details: { pluginId: "other", manifestId: "demo" }, }); }); diff --git a/tests/unit/session-launch-failure.test.ts b/tests/unit/session-launch-failure.test.ts index 1a43570..d57440e 100644 --- a/tests/unit/session-launch-failure.test.ts +++ b/tests/unit/session-launch-failure.test.ts @@ -54,7 +54,7 @@ describe("session launch failures", () => { expect(error).toMatchObject({ code: "SESSION_NOT_RUNNING", - fixedBy: "obsidian_workspace_create", + fixedBy: "obsidian_session_open", details: expect.objectContaining({ launchError: expect.objectContaining({ signal: "SIGSEGV" }), }), @@ -86,7 +86,7 @@ describe("session launch failures", () => { await expect(restartSession(key, { env })).rejects.toMatchObject({ code: "SESSION_NOT_RUNNING", - fixedBy: "obsidian_workspace_restart", + fixedBy: "obsidian_session_reset", details: expect.objectContaining({ launchError: expect.objectContaining({ signal: "SIGSEGV" }), }), diff --git a/tests/unit/telemetry-store.test.ts b/tests/unit/telemetry-store.test.ts index 32444f6..968ab8f 100644 --- a/tests/unit/telemetry-store.test.ts +++ b/tests/unit/telemetry-store.test.ts @@ -18,58 +18,54 @@ async function telemetryPath(): Promise { } describe("TelemetryStore JSONL persistence", () => { - it("keeps durable histories isolated across workspace switches", async () => { + it("keeps default-profile and managed-session histories isolated", async () => { const dir = await mkdtemp(join(tmpdir(), "knap-workspace-telemetry-")); roots.push(dir); - const firstHandle = "wsp_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - const secondHandle = "wsp_BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; const store = new WorkspaceTelemetryStore(20, dir); - store.select(firstHandle); - store.add({ source: "console", level: "warn", text: "first" }); - store.select(secondHandle); + store.select("default"); + store.add({ source: "console", level: "warn", text: "default" }); + store.select("session"); expect(store.query().records).toEqual([]); - store.add({ source: "console", level: "error", text: "second" }); - store.select(firstHandle); - expect(store.query().records.map((record) => record.text)).toEqual(["first"]); + store.add({ source: "console", level: "error", text: "session" }); + store.select("default"); + expect(store.query().records.map((record) => record.text)).toEqual(["default"]); store.closePersistence(); const restarted = new WorkspaceTelemetryStore(20, dir); - restarted.select(firstHandle); - expect(restarted.query().records.map((record) => record.text)).toEqual(["first"]); - restarted.select(secondHandle); - expect(restarted.query().records.map((record) => record.text)).toEqual(["second"]); + restarted.select("default"); + expect(restarted.query().records.map((record) => record.text)).toEqual(["default"]); + restarted.select("session"); + expect(restarted.query().records.map((record) => record.text)).toEqual(["session"]); }); - it("archives a closed workspace history beside its retained files", async () => { + it("archives a closed session history beside its retained files", async () => { const dir = await mkdtemp(join(tmpdir(), "knap-workspace-telemetry-")); const retainedRoot = await mkdtemp(join(tmpdir(), "knap-retained-workspace-")); roots.push(dir, retainedRoot); - const handle = "wsp_CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC"; const store = new WorkspaceTelemetryStore(20, dir); - store.select(handle); + store.select("session"); store.add({ source: "console", level: "warn", text: "archive me" }); store.select("default"); - const archivedPath = await store.archive(handle, retainedRoot); + const archivedPath = await store.archive("session", retainedRoot); expect(archivedPath).toBe(join(retainedRoot, "telemetry", "events.jsonl")); expect(await readFile(archivedPath!, "utf8")).toContain("archive me"); - await expect(readFile(join(dir, `${handle}.jsonl`), "utf8")).rejects.toMatchObject({ + await expect(readFile(join(dir, "session.jsonl"), "utf8")).rejects.toMatchObject({ code: "ENOENT", }); }); - it("refuses to archive telemetry for the active workspace", async () => { + it("refuses to archive telemetry for the active session", async () => { const dir = await mkdtemp(join(tmpdir(), "knap-workspace-telemetry-")); const retainedRoot = await mkdtemp(join(tmpdir(), "knap-retained-workspace-")); roots.push(dir, retainedRoot); - const handle = "wsp_DDDDDDDDDDDDDDDDDDDDDDDDDDDDDDDD"; const store = new WorkspaceTelemetryStore(20, dir); - store.select(handle); - await expect(store.archive(handle, retainedRoot)).rejects.toThrow( - "Cannot archive telemetry while its workspace is active.", + store.select("session"); + await expect(store.archive("session", retainedRoot)).rejects.toThrow( + "Cannot archive telemetry while the managed session is active.", ); }); diff --git a/tests/unit/tool-catalog-schema.test.ts b/tests/unit/tool-catalog-schema.test.ts index e85604c..6ca8d57 100644 --- a/tests/unit/tool-catalog-schema.test.ts +++ b/tests/unit/tool-catalog-schema.test.ts @@ -6,14 +6,12 @@ import { join } from "node:path"; import { tmpdir } from "node:os"; import { passthroughMcpResult } from "../../src/browser/forward.js"; import { validateScreenshotFilename } from "../../src/browser/proxy.js"; -import type { ServerContext } from "../../src/server.js"; -import { registerCoreTools } from "../../src/tools/core.js"; import { ToolRegistry } from "../../src/tools/registry.js"; import type { Toolset } from "../../src/toolsets.js"; import { createLogger } from "../../src/util/logger.js"; function registry(enabled: string[] = []): ToolRegistry { - return new ToolRegistry(new Set(enabled as Toolset[]), createLogger("error"), 2); + return new ToolRegistry(new Set(enabled as Toolset[]), createLogger("error")); } function bindWithConfigs( @@ -107,64 +105,21 @@ describe("tool catalog", () => { }); }); -describe("dynamic tool surface", () => { - it("starts with only core control tools and updates SDK handles at runtime", async () => { - const toolRegistry = registry(); +describe("static tool surface", () => { + it("binds operational tools without a workspace handle", () => { + const toolRegistry = registry(["ui"]); toolRegistry.add({ name: "browser_example", toolset: "ui", description: "Example browser operation.", handler: async () => "ok", }); - registerCoreTools({ registry: toolRegistry } as ServerContext); - - expect(toolRegistry.names()).toEqual([ - "obsidian_capabilities", - "obsidian_status", - "obsidian_tool_catalog", - "obsidian_toolsets", - "obsidian_toolsets_update", - ]); - - const handles = new Map(); - const server = { - registerTool: vi.fn((name: string) => { - const handle = { - enabled: true, - enable() { - this.enabled = true; - }, - disable() { - this.enabled = false; - }, - } as RegisteredTool; - handles.set(name, handle); - return handle; - }), - } as unknown as McpServer; - toolRegistry.bind(server); - - const update = toolRegistry.get("obsidian_toolsets_update")?.handler; - expect(update).toBeDefined(); - const preview = await update?.({ enable: ["ui"], dryRun: true }); - expect(handles.get("browser_example")?.enabled).toBe(false); - expect(preview).toMatchObject({ - json: { - dryRun: true, - enabled: ["ui"], - changed: { enabled: ["ui"], toolCount: 1 }, - }, - }); + const configs = new Map>(); + bindWithConfigs(toolRegistry, configs); - const changed = await update?.({ enable: ["ui"] }); - expect(handles.get("browser_example")?.enabled).toBe(true); - expect(changed).toMatchObject({ - json: { - dryRun: false, - enabled: ["ui"], - changed: { enabled: ["ui"], toolCount: 1 }, - }, - }); + expect(configs.has("browser_example")).toBe(true); + const inputSchema = configs.get("browser_example")?.inputSchema as Record; + expect(inputSchema).not.toHaveProperty("workspaceHandle"); }); }); diff --git a/tests/unit/tool-registry-runtime.test.ts b/tests/unit/tool-registry-runtime.test.ts index eb5278e..7bd9436 100644 --- a/tests/unit/tool-registry-runtime.test.ts +++ b/tests/unit/tool-registry-runtime.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import type { McpServer, RegisteredTool } from "@modelcontextprotocol/server"; +import type { McpServer } from "@modelcontextprotocol/server"; import { ToolRegistry } from "../../src/tools/registry.js"; import { createLogger } from "../../src/util/logger.js"; import type { ToolAuditEvent } from "../../src/audit/types.js"; @@ -13,103 +13,71 @@ type ToolCallback = ( isError?: boolean; }>; -function fakeServer( - handles: Map, - callbacks?: Map, -): McpServer { +function fakeServer(callbacks: Map): McpServer { return { registerTool: vi.fn((name: string, _config: unknown, callback: ToolCallback) => { - const handle = { - enabled: true, - enable() { - this.enabled = true; - }, - disable() { - this.enabled = false; - }, - } as RegisteredTool; - handles.set(name, handle); - callbacks?.set(name, callback); - return handle; + callbacks.set(name, callback); + return {} as never; }), } as unknown as McpServer; } describe("ToolRegistry runtime toolsets", () => { - it("keeps workspace binding exclusive through a read-only handler", async () => { - const handles = new Map(); - const callbacks = new Map(); + it("serializes every handler through one FIFO lane", async () => { + const handles = new Map(); const order: string[] = []; let releaseFirst!: () => void; const firstBlocked = new Promise((resolve) => { releaseFirst = resolve; }); - const registry = new ToolRegistry( - new Set(["core"]), - createLogger("error"), - 4, - undefined, - undefined, - undefined, - { - audit: false, - beforeInvoke: async (_definition, args) => { - order.push(`bind:${String(args.workspaceHandle)}`); - }, - }, - ); + const registry = new ToolRegistry(new Set(["core"]), createLogger("error"), undefined, { + audit: false, + beforeInvoke: async () => void 0, + }); registry.add({ name: "workspace_read", toolset: "core", description: "Read one workspace through the shared runtime.", annotations: { readOnlyHint: true }, handler: async (args) => { - order.push(`start:${String(args.workspaceHandle)}`); - if (args.workspaceHandle === "first") await firstBlocked; - order.push(`end:${String(args.workspaceHandle)}`); + order.push(`start:${String(args.name)}`); + if (args.name === "first") await firstBlocked; + order.push(`end:${String(args.name)}`); return "ok"; }, }); - registry.bind(fakeServer(handles, callbacks)); + registry.bind(fakeServer(handles)); - const first = callbacks.get("workspace_read")?.({ workspaceHandle: "first" }); + const first = handles.get("workspace_read")?.({ name: "first" }); await vi.waitFor(() => expect(order).toContain("start:first")); - const second = callbacks.get("workspace_read")?.({ workspaceHandle: "second" }); + const second = handles.get("workspace_read")?.({ name: "second" }); releaseFirst(); await Promise.all([first, second]); - expect(order).toEqual([ - "bind:first", - "start:first", - "end:first", - "bind:second", - "start:second", - "end:second", - ]); + expect(order).toEqual(["start:first", "end:first", "start:second", "end:second"]); }); - it("retains disabled definitions and enables them through SDK handles", () => { - const handles = new Map(); - const registry = new ToolRegistry(new Set(["core"]), createLogger("error"), 2); + it("does not register disabled startup-only toolsets", () => { + const callbacks = new Map(); + const registry = new ToolRegistry(new Set(["core"]), createLogger("error")); registry.add({ name: "browser_example", toolset: "ui", description: "Browser example.", handler: async () => "ok", }); - registry.bind(fakeServer(handles)); + registry.bind(fakeServer(callbacks)); - expect(handles.get("browser_example")?.enabled).toBe(false); + expect(callbacks.has("browser_example")).toBe(false); expect(registry.toolsetState().disabled).toContain("ui"); - expect(registry.setToolsetEnabled("ui", true)).toEqual(["browser_example"]); - expect(handles.get("browser_example")?.enabled).toBe(true); - expect(registry.byToolset().ui).toEqual(["browser_example"]); + expect(callbacks.has("browser_example")).toBe(false); + expect(registry.byToolset().ui).toBeUndefined(); }); it("keeps control-plane tools enabled with their toolset disabled", () => { - const handles = new Map(); - const registry = new ToolRegistry(new Set(["core"]), createLogger("error"), 2); + const callbacks = new Map(); + const registry = new ToolRegistry(new Set(["core"]), createLogger("error")); registry.add({ name: "obsidian_toolsets", toolset: "core", @@ -117,33 +85,24 @@ describe("ToolRegistry runtime toolsets", () => { description: "Manage toolsets.", handler: async () => "ok", }); - registry.bind(fakeServer(handles)); - - registry.setToolsetEnabled("core", false); + registry.bind(fakeServer(callbacks)); - expect(handles.get("obsidian_toolsets")?.enabled).toBe(true); + expect(callbacks.has("obsidian_toolsets")).toBe(true); expect(registry.names()).toContain("obsidian_toolsets"); }); it("returns native structured content without duplicate fenced JSON", async () => { - const handles = new Map(); const callbacks = new Map(); - const registry = new ToolRegistry( - new Set(["core"]), - createLogger("error"), - 2, - undefined, - undefined, - undefined, - { audit: false }, - ); + const registry = new ToolRegistry(new Set(["core"]), createLogger("error"), undefined, { + audit: false, + }); registry.add({ name: "structured_example", toolset: "core", description: "Structured example.", handler: async () => ({ text: "Found 2 items.", json: { count: 2, items: ["a", "b"] } }), }); - registry.bind(fakeServer(handles, callbacks)); + registry.bind(fakeServer(callbacks)); const result = await callbacks.get("structured_example")?.({}, { requestId: 7 }); @@ -153,24 +112,17 @@ describe("ToolRegistry runtime toolsets", () => { }); it("keeps plain text for clients that do not read structured content", async () => { - const handles = new Map(); const callbacks = new Map(); - const registry = new ToolRegistry( - new Set(["core"]), - createLogger("error"), - 2, - undefined, - undefined, - undefined, - { audit: false }, - ); + const registry = new ToolRegistry(new Set(["core"]), createLogger("error"), undefined, { + audit: false, + }); registry.add({ name: "json_only_example", toolset: "core", description: "JSON-only example.", handler: async () => ({ json: [1, 2] }), }); - registry.bind(fakeServer(handles, callbacks)); + registry.bind(fakeServer(callbacks)); const result = await callbacks.get("json_only_example")?.({}); @@ -180,31 +132,20 @@ describe("ToolRegistry runtime toolsets", () => { }); it("runs request hooks and emits one redacted audit event", async () => { - const handles = new Map(); const callbacks = new Map(); const events: ToolAuditEvent[] = []; const order: string[] = []; - const registry = new ToolRegistry( - new Set(["core"]), - createLogger("error"), - 2, - undefined, - undefined, - undefined, - { - audit: { write: async (event) => void events.push(event) }, - beforeInvoke: async () => void order.push("before"), - contextProvider: async () => ({ - clientInfo: { name: "codex", version: "1.2.3" }, - agentHandle: "agent-1", - workspaceHandle: "workspace-1", - transport: "stdio", - protocolVersion: "2025-11-25", - traceId: "trace-1", - workspaceKind: "vault", - }), - }, - ); + const registry = new ToolRegistry(new Set(["core"]), createLogger("error"), undefined, { + audit: { write: async (event) => void events.push(event) }, + beforeInvoke: async () => void order.push("before"), + contextProvider: async () => ({ + clientInfo: { name: "codex", version: "1.2.3" }, + transport: "stdio", + protocolVersion: "2025-11-25", + traceId: "trace-1", + workspaceKind: "vault", + }), + }); registry.add({ name: "audited_example", toolset: "core", @@ -214,7 +155,7 @@ describe("ToolRegistry runtime toolsets", () => { return "ok"; }, }); - registry.bind(fakeServer(handles, callbacks)); + registry.bind(fakeServer(callbacks)); await callbacks.get("audited_example")?.( { code: "private code", text: "private note", settings: { token: "private" } }, @@ -237,31 +178,47 @@ describe("ToolRegistry runtime toolsets", () => { expect(events[0]?.trace_id).toMatch(/^sha256:/); expect(events[0]?.client?.name).toMatch(/^sha256:/); expect(events[0]?.client?.version).toMatch(/^sha256:/); - expect(events[0]?.agent_handle).toMatch(/^sha256:/); - expect(events[0]?.workspace_handle).toMatch(/^sha256:/); expect(JSON.stringify(events[0])).not.toContain("private"); }); + it("runs cleanup after a precondition hook fails", async () => { + const callbacks = new Map(); + const afterInvoke = vi.fn(); + const registry = new ToolRegistry(new Set(["core"]), createLogger("error"), undefined, { + audit: false, + beforeInvoke: async () => { + throw new Error("missing target"); + }, + afterInvoke, + }); + registry.add({ + name: "precondition_failure", + toolset: "core", + description: "Fail after admission to verify that activity cleanup still runs.", + handler: async () => "unreachable", + }); + registry.bind(fakeServer(callbacks)); + + const result = await callbacks.get("precondition_failure")?.({}); + + expect(result?.isError).toBe(true); + expect(afterInvoke).toHaveBeenCalledOnce(); + expect(afterInvoke.mock.calls[0]?.[3]).toMatchObject({ code: "INTERNAL" }); + }); + it("does not block tools or build an audit queue behind a stalled write", async () => { - const handles = new Map(); const callbacks = new Map(); const write = vi.fn(() => new Promise(() => undefined)); - const registry = new ToolRegistry( - new Set(["core"]), - createLogger("error"), - 2, - undefined, - undefined, - undefined, - { audit: { write } }, - ); + const registry = new ToolRegistry(new Set(["core"]), createLogger("error"), undefined, { + audit: { write }, + }); registry.add({ name: "stalled_audit_example", toolset: "core", description: "Stalled audit example.", handler: async () => "ok", }); - registry.bind(fakeServer(handles, callbacks)); + registry.bind(fakeServer(callbacks)); await callbacks.get("stalled_audit_example")?.({}); await callbacks.get("stalled_audit_example")?.({}); @@ -270,18 +227,11 @@ describe("ToolRegistry runtime toolsets", () => { }); it("emits a redacted error envelope and native error details", async () => { - const handles = new Map(); const callbacks = new Map(); const events: ToolAuditEvent[] = []; - const registry = new ToolRegistry( - new Set(["core"]), - createLogger("error"), - 2, - undefined, - undefined, - undefined, - { audit: { write: async (event) => void events.push(event) } }, - ); + const registry = new ToolRegistry(new Set(["core"]), createLogger("error"), undefined, { + audit: { write: async (event) => void events.push(event) }, + }); registry.add({ name: "failed_example", toolset: "core", @@ -290,7 +240,7 @@ describe("ToolRegistry runtime toolsets", () => { throw new Error("private typed text"); }, }); - registry.bind(fakeServer(handles, callbacks)); + registry.bind(fakeServer(callbacks)); const result = await callbacks.get("failed_example")?.({}, { requestId: "request-2" }); diff --git a/tests/unit/workspace-lease.test.ts b/tests/unit/workspace-lease.test.ts deleted file mode 100644 index ac6e24b..0000000 --- a/tests/unit/workspace-lease.test.ts +++ /dev/null @@ -1,105 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { mkdtemp, rm, stat } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; - -const readStartTime = vi.fn(); - -vi.mock("../../src/connection/health.js", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - readPidStartTime: (...args: unknown[]) => readStartTime(...args), - }; -}); - -const { WorkspaceLeaseManager } = await import("../../src/workspace/lease.js"); - -let home: string; -let env: NodeJS.ProcessEnv; -let now: Date; -const workspaceHandle = "wsp_AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - -beforeEach(async () => { - home = await mkdtemp(join(tmpdir(), "knap-workspace-lease-")); - env = { ...process.env, KNAP_HOME: home }; - now = new Date("2026-08-06T12:00:00.000Z"); - readStartTime.mockReset().mockResolvedValue(100); -}); - -afterEach(async () => { - await rm(home, { recursive: true, force: true }); -}); - -function manager(): InstanceType { - return new WorkspaceLeaseManager({ - idleTimeoutMs: 60_000, - env, - now: () => now, - pid: process.pid, - hostname: "test-host", - cwd: "/tmp/test", - }); -} - -describe("workspace leases", () => { - it("allows only one owner and stores private records", async () => { - const first = manager(); - const second = manager(); - await first.acquire(workspaceHandle, "obsidian_snapshot"); - - await expect(second.acquire(workspaceHandle, "obsidian_command")).rejects.toMatchObject({ - code: "WORKSPACE_BUSY", - details: expect.objectContaining({ workspaceHandle, retryAfterMs: 60_000 }), - }); - expect( - (await stat(join(home, "workspace-leases", `${workspaceHandle}.json`))).mode & 0o777, - ).toBe(0o600); - - await first.release(workspaceHandle); - await expect(second.acquire(workspaceHandle)).resolves.toMatchObject({ workspaceHandle }); - }); - - it("reclaims an expired lease", async () => { - const first = manager(); - const second = manager(); - await first.acquire(workspaceHandle); - now = new Date(now.getTime() + 60_001); - - await expect(second.acquire(workspaceHandle)).resolves.toMatchObject({ workspaceHandle }); - expect((await first.status(workspaceHandle)).state).toBe("busy"); - }); - - it("does not release a lease whose token changed", async () => { - const first = manager(); - const second = manager(); - await first.acquire(workspaceHandle); - now = new Date(now.getTime() + 60_001); - await second.acquire(workspaceHandle); - await first.release(workspaceHandle); - expect((await second.status(workspaceHandle)).state).toBe("owned"); - }); - - it("reclaims a lease after PID reuse", async () => { - const first = manager(); - await first.acquire(workspaceHandle); - readStartTime.mockResolvedValue(200); - const second = manager(); - - await expect(second.acquire(workspaceHandle)).resolves.toMatchObject({ - pidStartTime: 200, - }); - }); - - it("releases all leases held by one manager", async () => { - const owner = manager(); - const other = manager(); - const secondHandle = "wsp_BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; - await owner.acquire(workspaceHandle); - await owner.acquire(secondHandle); - await owner.releaseAll(); - - expect((await other.status(workspaceHandle)).state).toBe("free"); - expect((await other.status(secondHandle)).state).toBe("free"); - }); -}); diff --git a/tests/unit/workspace-lifecycle.test.ts b/tests/unit/workspace-lifecycle.test.ts index 4ae5863..7478199 100644 --- a/tests/unit/workspace-lifecycle.test.ts +++ b/tests/unit/workspace-lifecycle.test.ts @@ -113,10 +113,10 @@ describe("two-phase workspace lifecycle", () => { it("refuses to release or quarantine a live instance and never quits it", async () => { findPids.mockResolvedValue([1234]); await expect(releaseSession(key, { env })).rejects.toMatchObject({ - fixedBy: "obsidian_workspace_stop", + fixedBy: "obsidian_session_reset", }); await expect(quarantineSession(key, { env })).rejects.toMatchObject({ - fixedBy: "obsidian_workspace_stop", + fixedBy: "obsidian_session_reset", }); expect(quit).not.toHaveBeenCalled(); expect(await readDescriptor(key, env)).toBeDefined(); From 0621152a3384e5d78913a50b75c53730f04d1d78 Mon Sep 17 00:00:00 2001 From: Slate Rehm Date: Sat, 8 Aug 2026 16:42:13 -0500 Subject: [PATCH 2/3] fix: address session review findings --- README.md | 4 +- docs/configuration.md | 3 +- rules/obsidian-plugin.mdc | 5 +- skills/obsidian-instance-setup/SKILL.md | 2 +- src/server.ts | 1 + src/tools/registry.ts | 80 ++++++++++++------------ src/usage/activity-guard.ts | 5 +- tests/unit/tool-registry-runtime.test.ts | 44 +++++++++++++ 8 files changed, 96 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index c3a2e7a..d27b388 100644 --- a/README.md +++ b/README.md @@ -250,8 +250,8 @@ tools use that session automatically. They do not accept caller-owned session identifiers. Call `obsidian_session_status` to inspect the session. Call `obsidian_session_release` -to stop it and keep its scratch vault. Call `obsidian_session_reset` to stop the -session and create a new private target. Knapper moves verified private roots to +to release your claim while the private app stays ready for reuse. Call +`obsidian_session_reset` to stop the session and create a new private target. Knapper moves verified private roots to recoverable trash when cleanup requires removal. It never hard-deletes them. Only one operation runs at a time. A second Knapper process receives `KNAPPER_BUSY`. diff --git a/docs/configuration.md b/docs/configuration.md index 077847e..7827742 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -30,7 +30,8 @@ it routes tools. The result returns `visualIdentity.state` and `visualIdentity.warnings`. A session does not become ready when the required banner, title, icon, or desktop class is missing. -Call `obsidian_session_release` to stop the session and retain its scratch vault. +Call `obsidian_session_release` to release the active claim. The private app and +scratch vault stay ready for the next agent. Call `obsidian_session_reset` to replace it. Cleanup checks path, symlink, device, and inode ownership. It moves the root into `KNAP_HOME/trash`. It never hard-deletes the root. diff --git a/rules/obsidian-plugin.mdc b/rules/obsidian-plugin.mdc index c0b24ca..5856219 100644 --- a/rules/obsidian-plugin.mdc +++ b/rules/obsidian-plugin.mdc @@ -35,8 +35,9 @@ alwaysApply: false - Use a scratch dev vault for `obsidian_reset_state` and destructive tests. - Keep the private profile and `XDG_RUNTIME_DIR` isolation intact. -- Use `obsidian_session_release` after testing. Knapper moves verified roots to - recoverable trash and never hard-deletes them. +- Use `obsidian_session_release` after testing. It releases the agent claim and + keeps the private app ready for reuse. Reset moves verified roots to recoverable + trash and never hard-deletes them. - Run `obsidian_doctor` when transports fail; quit Obsidian fully before expecting CDP after adding `--remote-debugging-port`. ## Tool discovery diff --git a/skills/obsidian-instance-setup/SKILL.md b/skills/obsidian-instance-setup/SKILL.md index 5742132..4dfe070 100644 --- a/skills/obsidian-instance-setup/SKILL.md +++ b/skills/obsidian-instance-setup/SKILL.md @@ -32,7 +32,7 @@ obsidian_session_release obsidian_session_reset ``` -`release` stops the app and retains the scratch vault. `reset` replaces the private +`release` drops the agent claim and keeps the app ready for reuse. `reset` replaces the private target and moves verified old roots to recoverable Knapper trash. Knapper never hard-deletes a managed root. diff --git a/src/server.ts b/src/server.ts index 6e75bcd..b436642 100644 --- a/src/server.ts +++ b/src/server.ts @@ -197,6 +197,7 @@ export async function createServerContext(config: Config): Promise { + return this.lock.run("exclusive", def.name, async () => { + try { // Read the telemetry cursor after admission, not before: a call that // waited in the queue would otherwise report every log line produced // by the calls it was queued behind. @@ -414,45 +412,45 @@ export class ToolRegistry { }; } return result; - }); - } catch (e) { - const err = toUobError(e); - completedOutcome = err; - auditOutcome = "error"; - auditError = errorEnvelope(err); - this.logger.warn("tool failed", { - tool: def.name, - code: err.code, - ms: Date.now() - started, - }); - return errorResult(err); - } finally { - if (admitted && completedOutcome !== undefined) { - try { - await this.hooks.afterInvoke?.(def, callArgs, requestContext, completedOutcome); - } catch (hookError) { - this.logger.warn("afterInvoke hook failed", { - tool: def.name, - error: hookError instanceof Error ? hookError.name : "UnknownHookError", - }); + } catch (e) { + const err = toUobError(e); + completedOutcome = err; + auditOutcome = "error"; + auditError = errorEnvelope(err); + this.logger.warn("tool failed", { + tool: def.name, + code: err.code, + ms: Date.now() - started, + }); + return errorResult(err); + } finally { + if (completedOutcome !== undefined) { + try { + await this.hooks.afterInvoke?.(def, callArgs, requestContext, completedOutcome); + } catch (hookError) { + this.logger.warn("afterInvoke hook failed", { + tool: def.name, + error: hookError instanceof Error ? hookError.name : "UnknownHookError", + }); + } + } + if (this.audit !== false) { + this.queueAudit( + toolAuditEvent({ + timestamp, + requestId: callRequestId, + tool: def.name, + durationMs: Date.now() - started, + queueMs: admitted ? queueMs : Date.now() - started, + outcome: auditOutcome, + args: callArgs, + ...(auditContext ? { context: auditContext } : {}), + ...(auditError ? { error: auditError } : {}), + }), + ); } } - if (this.audit !== false) { - this.queueAudit( - toolAuditEvent({ - timestamp, - requestId: callRequestId, - tool: def.name, - durationMs: Date.now() - started, - queueMs: admitted ? queueMs : Date.now() - started, - outcome: auditOutcome, - args: callArgs, - ...(auditContext ? { context: auditContext } : {}), - ...(auditError ? { error: auditError } : {}), - }), - ); - } - } + }); }) as never, ); } diff --git a/src/usage/activity-guard.ts b/src/usage/activity-guard.ts index 896216b..636d58d 100644 --- a/src/usage/activity-guard.ts +++ b/src/usage/activity-guard.ts @@ -119,7 +119,10 @@ export class ActivityGuard { private startHeartbeat(): void { if (this.heartbeatTimer !== undefined) clearInterval(this.heartbeatTimer); const intervalMs = Math.max(1_000, Math.floor(this.opts.idleTimeoutMs / 3)); - this.heartbeatTimer = setInterval(() => void this.heartbeat(), intervalMs); + this.heartbeatTimer = setInterval( + () => void this.heartbeat().catch(() => undefined), + intervalMs, + ); this.heartbeatTimer.unref(); } diff --git a/tests/unit/tool-registry-runtime.test.ts b/tests/unit/tool-registry-runtime.test.ts index 7bd9436..972d8c7 100644 --- a/tests/unit/tool-registry-runtime.test.ts +++ b/tests/unit/tool-registry-runtime.test.ts @@ -57,6 +57,50 @@ describe("ToolRegistry runtime toolsets", () => { expect(order).toEqual(["start:first", "end:first", "start:second", "end:second"]); }); + it("keeps finalization inside the FIFO lane", async () => { + const callbacks = new Map(); + const order: string[] = []; + let releaseAfter!: () => void; + const afterBlocked = new Promise((resolve) => { + releaseAfter = resolve; + }); + const registry = new ToolRegistry(new Set(["core"]), createLogger("error"), undefined, { + audit: false, + beforeInvoke: async (_definition, args) => void order.push(`before:${String(args.name)}`), + afterInvoke: async (_definition, args) => { + order.push(`after:${String(args.name)}`); + if (args.name === "first") await afterBlocked; + }, + }); + registry.add({ + name: "finalized_call", + toolset: "core", + description: "Verify that finalization completes before the next queued call starts.", + handler: async (args) => { + order.push(`handler:${String(args.name)}`); + return "ok"; + }, + }); + registry.bind(fakeServer(callbacks)); + + const first = callbacks.get("finalized_call")?.({ name: "first" }); + await vi.waitFor(() => expect(order).toContain("after:first")); + const second = callbacks.get("finalized_call")?.({ name: "second" }); + await Promise.resolve(); + expect(order).not.toContain("before:second"); + + releaseAfter(); + await Promise.all([first, second]); + expect(order).toEqual([ + "before:first", + "handler:first", + "after:first", + "before:second", + "handler:second", + "after:second", + ]); + }); + it("does not register disabled startup-only toolsets", () => { const callbacks = new Map(); const registry = new ToolRegistry(new Set(["core"]), createLogger("error")); From 9d6a9785cfbd5083f93c95ac8ce9edcf515e327b Mon Sep 17 00:00:00 2001 From: Slate Rehm Date: Sat, 8 Aug 2026 16:45:34 -0500 Subject: [PATCH 3/3] docs: correct session setup guidance --- README.md | 40 ++++++++++++------------- docs/configuration.md | 8 ++--- skills/obsidian-instance-setup/SKILL.md | 13 ++++---- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index d27b388..2f02c52 100644 --- a/README.md +++ b/README.md @@ -266,26 +266,26 @@ Knapper keeps the profile and `XDG_RUNTIME_DIR` private for managed sessions. Set options via **environment variables** (and a subset via CLI flags). See [docs/configuration.md](docs/configuration.md) for examples. -| Setting | Env var | CLI flag | Default | -| ------------------- | ------------------------ | ---------------- | ------------------------------ | -| CDP URL | `OBSIDIAN_CDP_URL` | `--cdp-url` | `http://127.0.0.1:9222` | -| Obsidian binary | `OBSIDIAN_BIN` | `--obsidian-bin` | OS default | -| Default vault | `OBSIDIAN_VAULT` | `--vault`, `-v` | (active / unset) | -| Toolsets | `KNAP_TOOLSETS` | `--toolsets` | `all` | -| knapper's disk root | `KNAP_HOME` | — | `~/.knapper_mcp` | -| Log level | `KNAP_LOG_LEVEL` | `--log-level` | `info` | -| Telemetry buffer | `KNAP_TELEMETRY_BUFFER` | — | `2000` | -| Network capture | `KNAP_TELEMETRY_NETWORK` | — | `false` | -| CDP reconnect delay | `KNAP_RECONNECT_MS` | — | `2000` | -| Screenshot dir | `KNAP_SCREENSHOT_DIR` | `--output-dir` | `./.knapper` | -| CLI timeout | `KNAP_CLI_TIMEOUT_MS` | — | `15000` | -| Session cleanup | `KNAP_IDLE_TIMEOUT_MS` | — | `86400000` (24 hours) | -| Activity ownership | `KNAP_ACTIVITY_IDLE_MS` | — | `300000` (5 minutes) | -| Command transport | `KNAP_COMMAND_TRANSPORT` | — | `auto` (`cli` or `playwright`) | -| Window match | `OBSIDIAN_TARGET_MATCH` | `--target-match` | (unset) | -| Transport | `MCP_TRANSPORT` | `--transport` | `stdio` | -| HTTP port | `MCP_PORT` | `--port` | `9223` | -| HTTP host | `MCP_HOST` | `--host` | `127.0.0.1` | +| Setting | Env var | CLI flag | Default | +| ------------------- | ------------------------ | ---------------- | ----------------------------------------------- | +| CDP URL | `OBSIDIAN_CDP_URL` | `--cdp-url` | `http://127.0.0.1:9222` | +| Obsidian binary | `OBSIDIAN_BIN` | `--obsidian-bin` | OS default | +| Default vault | `OBSIDIAN_VAULT` | `--vault`, `-v` | (active / unset) | +| Toolsets | `KNAP_TOOLSETS` | `--toolsets` | core, UI, telemetry, plugin development, editor | +| knapper's disk root | `KNAP_HOME` | — | `~/.knapper_mcp` | +| Log level | `KNAP_LOG_LEVEL` | `--log-level` | `info` | +| Telemetry buffer | `KNAP_TELEMETRY_BUFFER` | — | `2000` | +| Network capture | `KNAP_TELEMETRY_NETWORK` | — | `false` | +| CDP reconnect delay | `KNAP_RECONNECT_MS` | — | `2000` | +| Screenshot dir | `KNAP_SCREENSHOT_DIR` | `--output-dir` | `./.knapper` | +| CLI timeout | `KNAP_CLI_TIMEOUT_MS` | — | `15000` | +| Session cleanup | `KNAP_IDLE_TIMEOUT_MS` | — | `86400000` (24 hours) | +| Activity ownership | `KNAP_ACTIVITY_IDLE_MS` | — | `300000` (5 minutes) | +| Command transport | `KNAP_COMMAND_TRANSPORT` | — | `auto` (`cli` or `playwright`) | +| Window match | `OBSIDIAN_TARGET_MATCH` | `--target-match` | (unset) | +| Transport | `MCP_TRANSPORT` | `--transport` | `stdio` | +| HTTP port | `MCP_PORT` | `--port` | `9223` | +| HTTP host | `MCP_HOST` | `--host` | `127.0.0.1` | `LOG_LEVEL`, `RECONNECT_MS`, and `SCREENSHOT_DIR` are also accepted as aliases; the `KNAP_`-prefixed name wins when both are set. diff --git a/docs/configuration.md b/docs/configuration.md index 7827742..fecbe77 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -38,10 +38,10 @@ hard-deletes the root. ## Tool surface -| Environment variable | CLI flag | Default | Purpose | -| --------------------- | -------------- | ------------ | ----------------------------- | -| `KNAP_TOOLSETS` | `--toolsets` | `all` | Startup toolset selection | -| `KNAP_SCREENSHOT_DIR` | `--output-dir` | `./.knapper` | Default-profile artifact root | +| Environment variable | CLI flag | Default | Purpose | +| --------------------- | -------------- | ----------------------------------------------- | ----------------------------- | +| `KNAP_TOOLSETS` | `--toolsets` | core, UI, telemetry, plugin development, editor | Startup toolset selection | +| `KNAP_SCREENSHOT_DIR` | `--output-dir` | `./.knapper` | Default-profile artifact root | Knapper publishes the complete tool surface during MCP initialization. The list does not change during a connection. Do not change the tool list after startup. diff --git a/skills/obsidian-instance-setup/SKILL.md b/skills/obsidian-instance-setup/SKILL.md index 4dfe070..4eba4af 100644 --- a/skills/obsidian-instance-setup/SKILL.md +++ b/skills/obsidian-instance-setup/SKILL.md @@ -45,9 +45,8 @@ only after it verifies process death or an expired activity record. ## Default profile -Call `obsidian_session_open` without a private plugin target only for a -user-approved default-profile task. Existing vault access still requires terminal -authorization. +Call `obsidian_session_open` with `target="default"` only for a user-approved +default-profile task. Existing vault access still requires terminal authorization. | State | Meaning | Action | | ---------------------- | ---------------------------------------- | --------------------------------- | @@ -78,9 +77,11 @@ does not change during a connection. Do not change the tool list after startup. ## Safety limits -Private sessions use a private profile and `XDG_RUNTIME_DIR` for the CLI socket. -Restart operations remain scoped to the managed process. Knapper never uses the -default profile as a fallback. +Private sessions use a private profile. On Linux, a private `XDG_RUNTIME_DIR` also +isolates the CLI socket per session. macOS uses a shared socket, and Windows has no +per-session socket input. Treat native CLI routing outside Linux as shared or +unavailable. Restart operations remain scoped to the managed process. Knapper +never uses the default profile as a fallback. ## Related skills