From 14e99bb8a9f6551382071a238d228743f79685a8 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 11:49:21 +0530 Subject: [PATCH 01/11] feat: add safe Relay setup and agent configuration --- .prettierignore | 3 + README.md | 2 + docs/agent-integration.md | 8 +- docs/distribution/npm-package.md | 5 + docs/setup-and-configuration.md | 49 +++++ ...e-41-safe-setup-and-agent-configuration.md | 26 ++- docs/troubleshooting-agent-integration.md | 8 + integrations/claude-code/.mcp.json.example | 7 +- integrations/claude-code/README.md | 2 +- integrations/codex/README.md | 2 +- integrations/codex/config.toml.example | 7 +- integrations/generic-mcp/README.md | 4 +- .../generic-mcp/server-config.json.example | 7 +- package.json | 3 +- pnpm-lock.yaml | 14 +- scripts/package/smoke-installed-package.ts | 82 ++++++++- scripts/validate-agent-integration-assets.ts | 60 ++----- scripts/validate-repository-assets.ts | 40 ++++- .../setup/apply-integration-change.ts | 81 +++++++++ .../setup/backup-and-atomic-write.ts | 167 ++++++++++++++++++ .../setup/clients/claude-json-adapter.ts | 92 ++++++++++ .../setup/clients/client-adapter.ts | 18 ++ .../setup/clients/codex-toml-adapter.ts | 92 ++++++++++ src/distribution/setup/initialize-relay.ts | 50 ++++++ src/distribution/setup/ownership-store.ts | 130 ++++++++++++++ .../setup/plan-integration-change.ts | 114 ++++++++++++ src/distribution/setup/setup-errors.ts | 6 + src/distribution/setup/setup-types.ts | 43 +++++ src/distribution/setup/snippets.ts | 13 ++ src/interfaces/cli/main.ts | 18 +- src/interfaces/cli/operational-output.ts | 42 +++++ .../cli/parse-operational-command.ts | 144 +++++++++++++++ src/interfaces/cli/run-operational-command.ts | 138 +++++++++++++++ src/interfaces/cli/run-relay.ts | 8 + src/interfaces/production-dependencies.ts | 26 +++ .../claude-code/.mcp.json.example | 2 +- .../integrations/codex/config.toml.example | 7 +- .../valid/integrations/generic-mcp/README.md | 2 +- .../generic-mcp/server-config.json.example | 5 +- .../setup/claude-code/conflicting.json | 8 + tests/fixtures/setup/claude-code/crlf.json | 3 + tests/fixtures/setup/claude-code/empty.json | 3 + .../fixtures/setup/claude-code/formatted.json | 6 + .../fixtures/setup/claude-code/malformed.json | 2 + .../fixtures/setup/claude-code/matching.json | 8 + .../fixtures/setup/claude-code/unrelated.json | 13 ++ tests/fixtures/setup/codex/conflicting.toml | 3 + tests/fixtures/setup/codex/crlf.toml | 2 + tests/fixtures/setup/codex/empty.toml | 1 + tests/fixtures/setup/codex/formatted.toml | 4 + tests/fixtures/setup/codex/malformed.toml | 2 + tests/fixtures/setup/codex/matching.toml | 3 + tests/fixtures/setup/codex/unrelated.toml | 8 + tests/fixtures/setup/metadata/disabled.json | 15 ++ tests/fixtures/setup/metadata/duplicate.json | 25 +++ tests/fixtures/setup/metadata/empty.json | 1 + tests/fixtures/setup/metadata/enabled.json | 15 ++ tests/fixtures/setup/metadata/malformed.json | 1 + .../setup/metadata/unsupported-schema.json | 1 + .../setup/metadata/wrong-command.json | 15 ++ tests/integration/setup-workflow.test.ts | 96 ++++++++++ .../setup/apply-integration-change.test.ts | 80 +++++++++ .../setup/backup-and-atomic-write.test.ts | 58 ++++++ .../setup/claude-json-adapter.test.ts | 30 ++++ .../setup/codex-toml-adapter.test.ts | 31 ++++ .../setup/initialize-relay.test.ts | 85 +++++++++ .../setup/ownership-store.test.ts | 58 ++++++ .../setup/plan-integration-change.test.ts | 62 +++++++ .../distribution/setup/setup-types.test.ts | 37 ++++ .../unit/distribution/setup/snippets.test.ts | 24 +++ .../cli/operational-commands.test.ts | 48 +++++ .../cli/run-operational-command.test.ts | 65 +++++++ .../validate-agent-integration-assets.test.ts | 21 +-- tsup.config.ts | 1 + 74 files changed, 2258 insertions(+), 104 deletions(-) create mode 100644 docs/setup-and-configuration.md create mode 100644 src/distribution/setup/apply-integration-change.ts create mode 100644 src/distribution/setup/backup-and-atomic-write.ts create mode 100644 src/distribution/setup/clients/claude-json-adapter.ts create mode 100644 src/distribution/setup/clients/client-adapter.ts create mode 100644 src/distribution/setup/clients/codex-toml-adapter.ts create mode 100644 src/distribution/setup/initialize-relay.ts create mode 100644 src/distribution/setup/ownership-store.ts create mode 100644 src/distribution/setup/plan-integration-change.ts create mode 100644 src/distribution/setup/setup-errors.ts create mode 100644 src/distribution/setup/setup-types.ts create mode 100644 src/distribution/setup/snippets.ts create mode 100644 src/interfaces/cli/operational-output.ts create mode 100644 src/interfaces/cli/parse-operational-command.ts create mode 100644 src/interfaces/cli/run-operational-command.ts create mode 100644 tests/fixtures/setup/claude-code/conflicting.json create mode 100644 tests/fixtures/setup/claude-code/crlf.json create mode 100644 tests/fixtures/setup/claude-code/empty.json create mode 100644 tests/fixtures/setup/claude-code/formatted.json create mode 100644 tests/fixtures/setup/claude-code/malformed.json create mode 100644 tests/fixtures/setup/claude-code/matching.json create mode 100644 tests/fixtures/setup/claude-code/unrelated.json create mode 100644 tests/fixtures/setup/codex/conflicting.toml create mode 100644 tests/fixtures/setup/codex/crlf.toml create mode 100644 tests/fixtures/setup/codex/empty.toml create mode 100644 tests/fixtures/setup/codex/formatted.toml create mode 100644 tests/fixtures/setup/codex/malformed.toml create mode 100644 tests/fixtures/setup/codex/matching.toml create mode 100644 tests/fixtures/setup/codex/unrelated.toml create mode 100644 tests/fixtures/setup/metadata/disabled.json create mode 100644 tests/fixtures/setup/metadata/duplicate.json create mode 100644 tests/fixtures/setup/metadata/empty.json create mode 100644 tests/fixtures/setup/metadata/enabled.json create mode 100644 tests/fixtures/setup/metadata/malformed.json create mode 100644 tests/fixtures/setup/metadata/unsupported-schema.json create mode 100644 tests/fixtures/setup/metadata/wrong-command.json create mode 100644 tests/integration/setup-workflow.test.ts create mode 100644 tests/unit/distribution/setup/apply-integration-change.test.ts create mode 100644 tests/unit/distribution/setup/backup-and-atomic-write.test.ts create mode 100644 tests/unit/distribution/setup/claude-json-adapter.test.ts create mode 100644 tests/unit/distribution/setup/codex-toml-adapter.test.ts create mode 100644 tests/unit/distribution/setup/initialize-relay.test.ts create mode 100644 tests/unit/distribution/setup/ownership-store.test.ts create mode 100644 tests/unit/distribution/setup/plan-integration-change.test.ts create mode 100644 tests/unit/distribution/setup/setup-types.test.ts create mode 100644 tests/unit/distribution/setup/snippets.test.ts create mode 100644 tests/unit/interfaces/cli/operational-commands.test.ts create mode 100644 tests/unit/interfaces/cli/run-operational-command.test.ts diff --git a/.prettierignore b/.prettierignore index bb02667..69df30b 100644 --- a/.prettierignore +++ b/.prettierignore @@ -4,3 +4,6 @@ coverage/ pnpm-lock.yaml .superpowers/ .codegraph/ +*.toml +tests/fixtures/setup/claude-code/malformed.json +tests/fixtures/setup/metadata/malformed.json diff --git a/README.md b/README.md index f9cf90d..92f3f65 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,7 @@ # Relay +Safe installed setup is documented in [setup and configuration](docs/setup-and-configuration.md). Use an explicit absolute `--config-file` and preview before `--apply`; generic MCP is snippet-only. + For Linux-only Claude Desktop MCPB evaluation, see [the MCPB guide](integrations/claude-desktop/README.md) and [verification record](docs/claude-desktop-mcpb-verification.md). **Testing Relay for the first time?** Follow the [source-checkout installation and usage guide](docs/source-checkout-guide.md) to clone, run, connect an AI client, and complete a safe smoke test. diff --git a/docs/agent-integration.md b/docs/agent-integration.md index 0c9f100..eadeeb7 100644 --- a/docs/agent-integration.md +++ b/docs/agent-integration.md @@ -1,8 +1,10 @@ # Agent Integration -## Supported source-checkout model +## Supported installed model -Relay integrations run the built entries from an absolute source checkout: `node __RELAY_CHECKOUT__/dist/mcp/main.js` or `node __RELAY_CHECKOUT__/dist/cli/main.js`. `relay mcp` is a future packaged command owned by Epic #18 and is not available. +Installed integrations invoke the stable command `relay mcp`. Use `relay setup --client codex --config-file ` or the equivalent Claude Code command to preview a change, then add `--apply` only after reviewing the exact target, operation, and snippet. Generic MCP remains snippet-only. + +For source development only, run the built entries from an absolute source checkout: `node __RELAY_CHECKOUT__/dist/mcp/main.js` or `node __RELAY_CHECKOUT__/dist/cli/main.js`. These source-checkout examples are intentionally separate from installed templates. For source development only, run `pnpm dev:mcp` from the checkout; vendor configuration should use the built Node entry so it is independent of the current working directory. @@ -25,7 +27,7 @@ Set `RELAY_DB_PATH` to `__RELAY_CHECKOUT__/.relay-validation/relay.db` for first ## Canonical MCP and CLI entry points -See [generic MCP](../integrations/generic-mcp/README.md) and [generic CLI](../integrations/generic-cli/README.md). The canonical MCP server is `node __RELAY_CHECKOUT__/dist/mcp/main.js`; `relay mcp` remains future-only and unavailable until Epic #18. +See [generic MCP](../integrations/generic-mcp/README.md) and [generic CLI](../integrations/generic-cli/README.md). Installed configuration uses `relay mcp`; source-checkout validation uses the explicit built Node entry above. ## Session and provenance example diff --git a/docs/distribution/npm-package.md b/docs/distribution/npm-package.md index 75f4308..1d04e6d 100644 --- a/docs/distribution/npm-package.md +++ b/docs/distribution/npm-package.md @@ -38,6 +38,11 @@ configuration, and cache paths are independent of the current directory. platform default, and whitespace or relative values fail with usage/validation exit code 2. CLI, MCP, and UI use the same effective database path. +After installation, initialize Relay with `relay setup`. To configure Codex or +Claude Code, preview with an explicit absolute `--config-file` and add +`--apply` only after reviewing the exact entry; see [safe setup and +configuration](../setup-and-configuration.md). Generic MCP is snippet-only. + ## Supported runtime The initial release claim is Node.js 24 on Windows x64, macOS arm64, and diff --git a/docs/setup-and-configuration.md b/docs/setup-and-configuration.md new file mode 100644 index 0000000..e56215f --- /dev/null +++ b/docs/setup-and-configuration.md @@ -0,0 +1,49 @@ +# Safe setup and agent configuration + +`relay setup` initializes Relay's data and configuration roots and opens the canonical database runtime so forward migrations run. It never replaces, resets, or deletes an existing database. Re-running it is safe. + +Mutable client setup is preview-first: + +```text +relay setup --client codex --config-file +relay setup --client codex --config-file --apply +relay setup --client claude-code --config-file [--apply] +relay setup --client generic-mcp +``` + +Codex and Claude Code require an explicit absolute configuration path. Relay never scans home directories or infers a client file. Generic MCP produces a reviewed snippet only. + +Before applying, inspect the target, operation, entry identifier, and snippet. Relay proves ownership using the exact `relay` entry, the `relay` command, `['mcp']` arguments, client, and normalized path. Unknown or conflicting entries fail closed. A changed client file receives a collision-safe sibling backup and a validated atomic replacement; Relay ownership metadata is updated only after the replacement is reparsed successfully. + +Use `relay config paths` and `relay config integrations` to inspect effective paths and Relay-owned records. `relay config disable` removes an exact owned entry while retaining disabled ownership; setup can safely re-enable it. `relay config remove` removes only the exact owned entry and ownership record. These operations retain the database, tasks, backups, and unrelated configuration. + +The complete inspection and mutation surface is: + +```text +relay config paths +relay config paths --output json +relay config integrations +relay config integrations --output json +relay config snippet --client codex +relay config snippet --client claude-code +relay config snippet --client generic-mcp +relay config disable --client codex --config-file +relay config disable --client codex --config-file --apply +relay config remove --client codex --config-file +relay config remove --client codex --config-file --apply +``` + +The client configuration path is always explicit and absolute. `--apply` is required for a mutation; without it, setup, disable, and remove return a preview. Generic MCP is snippet-only and has no mutation mode. + +For the human safety gate, use this checklist with a disposable absolute path and an isolated `RELAY_DB_PATH`: + +1. Copy a real Codex or Claude Code configuration to a disposable file; never start with the only live configuration copy. +2. Run the preview and record the reported operation and exact snippet. +3. Apply the setup and verify the configured entry is exact. +4. Compare unrelated configuration bytes before and after. +5. Compare the backup with the original bytes. +6. Rerun setup and verify it reports unchanged and creates no new backup. +7. Disable the owned entry and verify unrelated content remains. +8. Re-enable it with setup and verify the entry returns. +9. Remove it and verify the ownership record is gone while the database remains. +10. Query a task created before setup and confirm it still exists. diff --git a/docs/superpowers/plans/2026-08-02-issue-41-safe-setup-and-agent-configuration.md b/docs/superpowers/plans/2026-08-02-issue-41-safe-setup-and-agent-configuration.md index 23b9c84..671ec08 100644 --- a/docs/superpowers/plans/2026-08-02-issue-41-safe-setup-and-agent-configuration.md +++ b/docs/superpowers/plans/2026-08-02-issue-41-safe-setup-and-agent-configuration.md @@ -132,12 +132,7 @@ export interface RelayIntegrationOwnership { readonly lastBackupPath?: string; } -export type IntegrationOperation = - | 'created' - | 'updated' - | 'unchanged' - | 'disabled' - | 'removed'; +export type IntegrationOperation = 'created' | 'updated' | 'unchanged' | 'disabled' | 'removed'; export interface IntegrationChangePlan { readonly client: MutableIntegrationClient; @@ -584,14 +579,27 @@ git commit -m "feat: apply owned configuration changes atomically" ```ts export type OperationalCommand = - | { readonly kind: 'setup'; readonly client?: IntegrationClient; readonly configFile?: string; readonly apply: boolean } + | { + readonly kind: 'setup'; + readonly client?: IntegrationClient; + readonly configFile?: string; + readonly apply: boolean; + } | { readonly kind: 'config-paths' } | { readonly kind: 'config-integrations' } | { readonly kind: 'config-snippet'; readonly client: IntegrationClient } - | { readonly kind: 'config-disable' | 'config-remove'; readonly client: MutableIntegrationClient; readonly configFile: string; readonly apply: true }; + | { + readonly kind: 'config-disable' | 'config-remove'; + readonly client: MutableIntegrationClient; + readonly configFile: string; + readonly apply: true; + }; export function parseOperationalCommand(argv: readonly string[]): OperationalCommand; -export async function runOperationalCommand(command: OperationalCommand, dependencies: OperationalDependencies): Promise; +export async function runOperationalCommand( + command: OperationalCommand, + dependencies: OperationalDependencies, +): Promise; ``` - [ ] **Step 1: Write failing parser tests for the locked command grammar.** diff --git a/docs/troubleshooting-agent-integration.md b/docs/troubleshooting-agent-integration.md index 5f7149f..6a451f0 100644 --- a/docs/troubleshooting-agent-integration.md +++ b/docs/troubleshooting-agent-integration.md @@ -4,6 +4,14 @@ **Symptom:** build fails. **Check:** `node --version` and `pnpm --version`. **Resolution:** use the documented versions. +## Setup preview or conflict failure + +**Symptom:** setup refuses to apply. **Check:** run the same command without `--apply` and inspect the exact target, operation, and `relay` entry. **Resolution:** use an explicit absolute `--config-file`, resolve any conflicting or unowned `relay` entry manually, and keep the original file and Relay backup intact. + +## Configuration backup or race failure + +**Symptom:** an apply reports a backup, write, or concurrent-change error. **Check:** inspect the named target and sibling `.relay-backup-...` file. **Resolution:** do not delete the backup; restore or review the original, then retry after the client file is stable. + ## Missing dist/mcp/main.js or dist/cli/main.js **Symptom:** process cannot start. **Check:** run `pnpm build:node`. **Resolution:** rebuild before configuring the client. diff --git a/integrations/claude-code/.mcp.json.example b/integrations/claude-code/.mcp.json.example index eac3081..4710f43 100644 --- a/integrations/claude-code/.mcp.json.example +++ b/integrations/claude-code/.mcp.json.example @@ -1,11 +1,8 @@ { "mcpServers": { "relay": { - "command": "node", - "args": ["__RELAY_CHECKOUT__/dist/mcp/main.js"], - "env": { - "RELAY_DB_PATH": "__RELAY_CHECKOUT__/.relay-validation/relay.db" - } + "command": "relay", + "args": ["mcp"] } } } diff --git a/integrations/claude-code/README.md b/integrations/claude-code/README.md index 31f7e9c..4e9950b 100644 --- a/integrations/claude-code/README.md +++ b/integrations/claude-code/README.md @@ -1,6 +1,6 @@ # Claude Code integration -Build Relay and substitute an absolute checkout path. Add the stdio server with `claude mcp add --transport stdio --scope project --env RELAY_DB_PATH=ABSOLUTE_CHECKOUT/.relay-validation/relay.db relay -- node ABSOLUTE_CHECKOUT/dist/mcp/main.js`, or copy the template to the project root as `.mcp.json`. `local` is private to the current project, `project` is shared through `.mcp.json`, and `user` applies across projects; local takes priority. Set the same isolated `RELAY_DB_PATH` in the selected configuration. +Use `relay setup --client claude-code --config-file ` to preview the exact JSON entry, then add `--apply` only after reviewing a disposable or reviewed file. The generated entry invokes `relay mcp`; no client-file discovery occurs. The source-checkout command remains documented separately for repository development. `local` is private to the current project, `project` is shared through `.mcp.json`, and `user` applies across projects; local takes priority. Install the canonical [Relay Capture](../../skills/relay-capture/SKILL.md) and [Relay Session Review](../../skills/relay-session-review/SKILL.md) skill directories by copying or symlinking them unchanged to `.claude/skills/relay-capture/` and `.claude/skills/relay-session-review/`. For a personal installation across projects, use the client’s documented user-scoped skills directory. Do not copy the policy text into Claude-specific documentation or use instruction-file imports as skill discovery. diff --git a/integrations/codex/README.md b/integrations/codex/README.md index 310c19d..300534a 100644 --- a/integrations/codex/README.md +++ b/integrations/codex/README.md @@ -1,5 +1,5 @@ # Codex integration -Build Relay, replace `__RELAY_CHECKOUT__` with an absolute path, create `.relay-validation`, then add the template to trusted project or user-scoped Codex configuration. Restart Codex and use `/mcp` or `codex mcp list` to confirm Relay. Verify `relay_health`, the five read/capture tools, a disposable capture, and exact-session retrieval. The JSON CLI fallback is in [generic CLI](../generic-cli/README.md). +Use `relay setup --client codex --config-file ` to preview the exact Relay entry, then add `--apply` to mutate a disposable or reviewed Codex file. The generated entry is `command = "relay"` with `args = ["mcp"]`; no client-file discovery occurs. Restart Codex and use `/mcp` or `codex mcp list` to confirm Relay. The source-checkout fallback and JSON CLI are documented in [generic CLI](../generic-cli/README.md). Install the canonical [Relay Capture](../../skills/relay-capture/SKILL.md) and [Relay Session Review](../../skills/relay-session-review/SKILL.md) as repository skills by copying their complete directories unchanged to `.agents/skills/relay-capture/` and `.agents/skills/relay-session-review/`; Codex discovers repository skills from `.agents/skills` after a new session. Do not copy their policy into this README. Remove the Relay configuration and those skill directories to disable it. The SQLite database remains untouched. diff --git a/integrations/codex/config.toml.example b/integrations/codex/config.toml.example index 2bca1bd..02b047e 100644 --- a/integrations/codex/config.toml.example +++ b/integrations/codex/config.toml.example @@ -1,6 +1,3 @@ [mcp_servers.relay] -command = "node" -args = ["__RELAY_CHECKOUT__/dist/mcp/main.js"] - -[mcp_servers.relay.env] -RELAY_DB_PATH = "__RELAY_CHECKOUT__/.relay-validation/relay.db" +command = "relay" +args = ["mcp"] diff --git a/integrations/generic-mcp/README.md b/integrations/generic-mcp/README.md index 26f9797..bef50f5 100644 --- a/integrations/generic-mcp/README.md +++ b/integrations/generic-mcp/README.md @@ -1,6 +1,8 @@ # Generic MCP integration -Build Relay with `pnpm build:node`, replace `__RELAY_CHECKOUT__` in [server-config.json.example](server-config.json.example) with an absolute checkout path, then copy the command, arguments, and optional environment map into the client configuration. Keep command and arguments separate: do not use a shell or interpolation. Validation flows must set the isolated `RELAY_DB_PATH` shown in the template; omitting `RELAY_DB_PATH` is permitted only for non-validation use and then selects Relay's platform default. +Run `relay setup --client generic-mcp` or `relay config snippet --client generic-mcp` to print the reviewed snippet. Generic MCP is snippet-only in this issue and never mutates a client file. The installed snippet invokes `relay mcp`; source-checkout validation remains documented separately and must use an explicitly isolated `RELAY_DB_PATH`. + +Validation RELAY_DB_PATH must be explicit and isolated; omitting RELAY_DB_PATH is permitted only for non-validation use and selects Relay's platform default. The stdio protocol requires clean stdout. Relay exposes exactly these MCP tools: `relay_health`, `task_capture`, `task_list`, `task_get`, `task_find_similar`, `session_captures_list`, `task_edit`, `task_triage`, `task_start`, `task_complete`, and `task_archive`. Restart or reload the client, capture one disposable task, and retrieve it by the same exact session ID. diff --git a/integrations/generic-mcp/server-config.json.example b/integrations/generic-mcp/server-config.json.example index a39d47c..ffffc2e 100644 --- a/integrations/generic-mcp/server-config.json.example +++ b/integrations/generic-mcp/server-config.json.example @@ -1,7 +1,4 @@ { - "command": "node", - "args": ["__RELAY_CHECKOUT__/dist/mcp/main.js"], - "env": { - "RELAY_DB_PATH": "__RELAY_CHECKOUT__/.relay-validation/relay.db" - } + "command": "relay", + "args": ["mcp"] } diff --git a/package.json b/package.json index 535be43..1da2bd8 100644 --- a/package.json +++ b/package.json @@ -73,7 +73,6 @@ }, "devDependencies": { "@anthropic-ai/mcpb": "2.1.2", - "@iarna/toml": "2.2.5", "@testing-library/jest-dom": "^7.0.0", "@testing-library/react": "^16.3.2", "@types/better-sqlite3": "^7.6.13", @@ -97,8 +96,10 @@ "vitest": "^4.1.10" }, "dependencies": { + "@iarna/toml": "2.2.5", "@modelcontextprotocol/sdk": "^1.29.0", "better-sqlite3": "^13.0.1", + "jsonc-parser": "3.3.1", "react": "^19.2.8", "react-dom": "^19.2.8", "zod": "^4.4.3" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 899556c..67ce628 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -11,12 +11,18 @@ importers: .: dependencies: + '@iarna/toml': + specifier: 2.2.5 + version: 2.2.5 '@modelcontextprotocol/sdk': specifier: ^1.29.0 version: 1.29.0(zod@4.4.3) better-sqlite3: specifier: ^13.0.1 version: 13.0.1 + jsonc-parser: + specifier: 3.3.1 + version: 3.3.1 react: specifier: ^19.2.8 version: 19.2.8 @@ -30,9 +36,6 @@ importers: '@anthropic-ai/mcpb': specifier: 2.1.2 version: 2.1.2 - '@iarna/toml': - specifier: 2.2.5 - version: 2.2.5 '@testing-library/jest-dom': specifier: ^7.0.0 version: 7.0.0(@testing-library/dom@10.4.1) @@ -1765,6 +1768,9 @@ packages: engines: {node: '>=6'} hasBin: true + jsonc-parser@3.3.1: + resolution: {integrity: sha512-HUgH65KyejrUFPvHFPbqOY0rsFip3Bo5wb4ngvdi1EpCYWUQDC5V+Y7mZws+DLkr4M//zQJoanu1SP+87Dv1oQ==} + jsonfile@6.2.1: resolution: {integrity: sha512-zwOTdL3rFQ/lRdBnntKVOX6k5cKJwEc1HdilT71BWEu7J41gXIB2MRp+vxduPSwZJPWBxEzv4yH1wYLJGUHX4Q==} @@ -4139,6 +4145,8 @@ snapshots: json5@2.2.3: {} + jsonc-parser@3.3.1: {} + jsonfile@6.2.1: dependencies: universalify: 2.0.1 diff --git a/scripts/package/smoke-installed-package.ts b/scripts/package/smoke-installed-package.ts index 062e097..f78a132 100644 --- a/scripts/package/smoke-installed-package.ts +++ b/scripts/package/smoke-installed-package.ts @@ -1,5 +1,5 @@ import { execFileSync, spawn, spawnSync } from 'node:child_process'; -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { createServer } from 'node:net'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -90,6 +90,7 @@ function runCli( cwd: string, databasePath: string, args: readonly string[], + environment: Readonly> = {}, ): CliRun { const result = process.platform === 'win32' @@ -101,11 +102,15 @@ function runCli( '/c', [commandPath, ...args].map((arg) => (/\s/.test(arg) ? `"${arg}"` : arg)).join(' '), ], - { cwd, env: { ...process.env, RELAY_DB_PATH: databasePath }, encoding: 'utf8' }, + { + cwd, + env: { ...process.env, ...environment, RELAY_DB_PATH: databasePath }, + encoding: 'utf8', + }, ) : spawnSync(commandPath, args, { cwd, - env: { ...process.env, RELAY_DB_PATH: databasePath }, + env: { ...process.env, ...environment, RELAY_DB_PATH: databasePath }, encoding: 'utf8', }); return { status: result.status, stdout: result.stdout ?? '', stderr: result.stderr ?? '' }; @@ -233,6 +238,77 @@ export async function verifyInstalledPackage(rootDir = process.cwd()): Promise; + mcpServers?: Record>; }; const server = parsed.mcpServers?.relay ?? parsed; - if (server.command !== 'node' || !Array.isArray(server.args) || server.args.length !== 1) { - fail(`${path} must separate command and arguments for a stdio server.`); - } - if (server.args[0] !== expectedMcpPath) { - fail(`${path} must use the canonical dist/mcp/main.js entry path.`); - } - if (typeof server.command !== 'string' || /[\\/\s]/.test(server.command)) { - fail(`${path} must not embed a shell command in command.`); - } + if ( + JSON.stringify(Object.keys(server).sort()) !== JSON.stringify(['args', 'command']) || + server.command !== 'relay' || + JSON.stringify(server.args) !== JSON.stringify(['mcp']) + ) + fail(`${path} must use separate command and arguments for the installed relay mcp command.`); } const tomlSource = readAsset(rootDir, 'integrations/codex/config.toml.example'); if (/(?:[A-Z]:[\\/]Users[\\/]|\/Users\/|\/home\/|~\/)/i.test(tomlSource)) { fail('integrations/codex/config.toml.example must not contain a machine-specific home path.'); } - const toml = parse(tomlSource.replaceAll('__RELAY_CHECKOUT__', '/tmp/relay-checkout')) as { + const toml = parse(tomlSource) as { mcp_servers?: { - relay?: { - command?: unknown; - args?: unknown; - env?: { RELAY_DB_PATH?: unknown }; - }; + relay?: Record; }; }; const codexServer = toml.mcp_servers?.relay; if ( - codexServer?.command !== 'node' || + codexServer === undefined || + JSON.stringify(Object.keys(codexServer).sort()) !== JSON.stringify(['args', 'command']) || + codexServer?.command !== 'relay' || !Array.isArray(codexServer.args) || codexServer.args.length !== 1 || - codexServer.args[0] !== expectedMcpPath + codexServer.args[0] !== 'mcp' ) { - fail( - 'integrations/codex/config.toml.example must use node plus the canonical dist/mcp/main.js entry as separate fields.', - ); - } - if (codexServer.env?.RELAY_DB_PATH !== '/tmp/relay-checkout/.relay-validation/relay.db') { - fail('integrations/codex/config.toml.example must configure an isolated RELAY_DB_PATH.'); + fail('integrations/codex/config.toml.example must invoke the installed relay mcp command.'); } } @@ -286,21 +275,8 @@ export function validateAgentIntegrationAssets( if (/^## Autonomy boundaries$/m.test(contents)) fail('Vendor assets must not copy behavioural policy.'); validateRemovalGuidance(rootDir); - for (const match of all.matchAll(/relay mcp/gi)) { - const context = all.slice(Math.max(0, match.index! - 80), match.index! + 100); - if (!/(future|not available|Epic #18)/i.test(context)) - fail('relay mcp must be marked as future-only.'); - } validateTemplateShape(rootDir); - const replaceCheckout = (text: string) => - text.replaceAll('__RELAY_CHECKOUT__', '/tmp/relay-checkout'); - JSON.parse( - replaceCheckout( - readFileSync(join(integrationRoot, 'generic-mcp/server-config.json.example'), 'utf8'), - ), - ); - JSON.parse( - replaceCheckout(readFileSync(join(integrationRoot, 'claude-code/.mcp.json.example'), 'utf8')), - ); - parse(replaceCheckout(readFileSync(join(integrationRoot, 'codex/config.toml.example'), 'utf8'))); + JSON.parse(readFileSync(join(integrationRoot, 'generic-mcp/server-config.json.example'), 'utf8')); + JSON.parse(readFileSync(join(integrationRoot, 'claude-code/.mcp.json.example'), 'utf8')); + parse(readFileSync(join(integrationRoot, 'codex/config.toml.example'), 'utf8')); } diff --git a/scripts/validate-repository-assets.ts b/scripts/validate-repository-assets.ts index ac4a514..4c9bcb7 100644 --- a/scripts/validate-repository-assets.ts +++ b/scripts/validate-repository-assets.ts @@ -2,6 +2,7 @@ import { existsSync, readFileSync, readdirSync } from 'node:fs'; import { isAbsolute, join, relative, resolve } from 'node:path'; import { pathToFileURL } from 'node:url'; import { parse as parseToml } from '@iarna/toml'; +import { parse as parseJsonc, type ParseError } from 'jsonc-parser'; import { validateSkillAssets } from './validate-skill-assets.js'; import { validateAgentIntegrationAssets } from './validate-agent-integration-assets.js'; import { validateMcpbAssets } from './validate-mcpb-assets.js'; @@ -36,6 +37,27 @@ export const requiredDistributionAssets = [ 'tests/fixtures/distribution/config-examples/claude-code-conflict.json', 'tests/fixtures/distribution/lifecycle-policy.json', 'tests/fixtures/distribution/version-compatibility.json', + 'tests/fixtures/setup/codex/empty.toml', + 'tests/fixtures/setup/codex/unrelated.toml', + 'tests/fixtures/setup/codex/matching.toml', + 'tests/fixtures/setup/codex/conflicting.toml', + 'tests/fixtures/setup/codex/malformed.toml', + 'tests/fixtures/setup/codex/formatted.toml', + 'tests/fixtures/setup/codex/crlf.toml', + 'tests/fixtures/setup/claude-code/empty.json', + 'tests/fixtures/setup/claude-code/unrelated.json', + 'tests/fixtures/setup/claude-code/matching.json', + 'tests/fixtures/setup/claude-code/conflicting.json', + 'tests/fixtures/setup/claude-code/malformed.json', + 'tests/fixtures/setup/claude-code/formatted.json', + 'tests/fixtures/setup/claude-code/crlf.json', + 'tests/fixtures/setup/metadata/empty.json', + 'tests/fixtures/setup/metadata/enabled.json', + 'tests/fixtures/setup/metadata/disabled.json', + 'tests/fixtures/setup/metadata/malformed.json', + 'tests/fixtures/setup/metadata/unsupported-schema.json', + 'tests/fixtures/setup/metadata/duplicate.json', + 'tests/fixtures/setup/metadata/wrong-command.json', ] as const; function walkFiles(rootDir: string, startDir = rootDir): string[] { @@ -107,8 +129,18 @@ function validateJsonFiles(files: readonly string[]): void { if (!filePath.endsWith('.json')) { continue; } + const normalizedPath = filePath.replaceAll('\\', '/'); + if ( + normalizedPath.endsWith('tests/fixtures/setup/metadata/malformed.json') || + normalizedPath.endsWith('tests/fixtures/setup/claude-code/malformed.json') || + normalizedPath.endsWith('tests/fixtures/setup/claude-code/formatted.json') + ) { + continue; + } - JSON.parse(readFileSync(filePath, 'utf-8')); + const errors: ParseError[] = []; + parseJsonc(readFileSync(filePath, 'utf-8'), errors); + if (errors.length > 0) throw new Error(`Malformed JSON/JSONC asset: ${filePath}`); } } @@ -368,6 +400,12 @@ function validateDistributionContract( if (compatibility.releaseTrigger !== 'manual-maintainer-action') { fail('Distribution version fixture must require a manual maintainer release action.'); } + + for (const path of requiredDistributionAssets.filter((asset) => + asset.startsWith('tests/fixtures/setup/'), + )) { + if (!existsSync(join(rootDir, path))) fail(`Setup fixture is missing: ${path}`); + } } export function validateRepositoryAssets(options: ValidateRepositoryAssetsOptions = {}): void { diff --git a/src/distribution/setup/apply-integration-change.ts b/src/distribution/setup/apply-integration-change.ts new file mode 100644 index 0000000..ebd9516 --- /dev/null +++ b/src/distribution/setup/apply-integration-change.ts @@ -0,0 +1,81 @@ +import type { ClientConfigAdapter } from './clients/client-adapter.js'; +import { backupAndAtomicWrite, restoreFile } from './backup-and-atomic-write.js'; +import { readFile } from 'node:fs/promises'; +import { fingerprint } from './plan-integration-change.js'; +import { SetupStorageError } from './setup-errors.js'; +import type { IntegrationChangePlan, IntegrationChangeResult } from './setup-types.js'; +import type { OwnershipStore } from './ownership-store.js'; + +export async function applyIntegrationChange(input: { + readonly plan: IntegrationChangePlan; + readonly adapter: ClientConfigAdapter; + readonly ownershipStore: OwnershipStore; + readonly applicationVersion: string; + readonly now: Date; +}): Promise { + if (!input.plan.changed) { + return { + client: input.plan.client, + configPath: input.plan.configPath, + entryId: 'relay', + operation: 'unchanged', + changed: false, + }; + } + const clientChanged = fingerprint(input.plan.nextContent) !== input.plan.beforeFingerprint; + const backup = clientChanged + ? await backupAndAtomicWrite({ + targetPath: input.plan.configPath, + expectedFingerprint: input.plan.beforeFingerprint, + nextContent: input.plan.nextContent, + validate: (content) => input.adapter.parse(content), + now: input.now, + }) + : undefined; + try { + const ownership = await input.ownershipStore.read(); + const existing = ownership.integrations.filter( + (record) => + !(record.client === input.plan.client && record.configPath === input.plan.configPath), + ); + const nextRecord = { + client: input.plan.client, + configPath: input.plan.configPath, + entryId: 'relay' as const, + command: 'relay' as const, + args: ['mcp'] as const, + status: input.plan.operation === 'disabled' ? ('disabled' as const) : ('enabled' as const), + applicationVersion: input.applicationVersion, + lastSuccessfulSetupAt: input.now.toISOString(), + ...(backup === undefined ? {} : { lastBackupPath: backup.backupPath }), + }; + await input.ownershipStore.write({ + schemaVersion: 1, + integrations: input.plan.operation === 'removed' ? existing : [...existing, nextRecord], + }); + } catch (error) { + try { + if (backup !== undefined) { + await restoreFile(backup.backupPath, input.plan.configPath); + input.adapter.parse(await readFile(input.plan.configPath, 'utf8')); + } + } catch (restoreError) { + throw new SetupStorageError( + `Client configuration was replaced but could not be restored from ${backup?.backupPath ?? input.plan.configPath}.`, + restoreError, + ); + } + throw new SetupStorageError( + `Client configuration was restored after metadata persistence failed: ${input.plan.configPath}.`, + error, + ); + } + return { + client: input.plan.client, + configPath: input.plan.configPath, + entryId: 'relay', + operation: input.plan.operation, + changed: true, + ...(backup === undefined ? {} : { backupPath: backup.backupPath }), + }; +} diff --git a/src/distribution/setup/backup-and-atomic-write.ts b/src/distribution/setup/backup-and-atomic-write.ts new file mode 100644 index 0000000..e661c41 --- /dev/null +++ b/src/distribution/setup/backup-and-atomic-write.ts @@ -0,0 +1,167 @@ +import { randomUUID } from 'node:crypto'; +import { chmod, open, readFile, rename, stat, unlink, writeFile } from 'node:fs/promises'; +import { basename, dirname, join } from 'node:path'; +import { fingerprint } from './plan-integration-change.js'; +import { SetupConflictError, SetupStorageError } from './setup-errors.js'; + +export async function backupAndAtomicWrite(input: { + readonly targetPath: string; + readonly expectedFingerprint: string; + readonly nextContent: string; + readonly validate: (content: string) => void; + readonly now: Date; +}): Promise<{ readonly backupPath: string }> { + const original = await readFile(input.targetPath).catch((error: unknown) => { + if (isMissing(error)) return Buffer.from(''); + throw storageError(input.targetPath, error); + }); + if (fingerprint(original) !== input.expectedFingerprint) + throw new SetupConflictError(`Configuration changed before replacement: ${input.targetPath}`); + const mode = await fileMode(input.targetPath); + const backupPath = await createExclusiveBackup(input.targetPath, input.now, original, mode); + const tempPath = join( + dirname(input.targetPath), + `.${basename(input.targetPath)}.${process.pid}.${randomUUID()}.tmp`, + ); + let replaced = false; + try { + const handle = await open(tempPath, 'wx', mode); + try { + await handle.writeFile(input.nextContent, 'utf8'); + await handle.sync(); + } finally { + await handle.close(); + } + await chmod(tempPath, mode); + input.validate(await readFile(tempPath, 'utf8')); + const current = await readFile(input.targetPath).catch((error: unknown) => { + if (isMissing(error)) return Buffer.from(''); + throw storageError(input.targetPath, error); + }); + if (fingerprint(current) !== input.expectedFingerprint) + throw new SetupConflictError(`Configuration changed before replacement: ${input.targetPath}`); + await replaceFile(tempPath, input.targetPath); + replaced = true; + input.validate(await readFile(input.targetPath, 'utf8')); + return { backupPath }; + } catch (error) { + if (replaced) { + try { + await restoreFile(backupPath, input.targetPath, mode); + input.validate(await readFile(input.targetPath, 'utf8')); + } catch (restoreError) { + throw new SetupStorageError( + `Failed to restore ${input.targetPath} from ${backupPath}.`, + restoreError, + ); + } + } + throw error; + } finally { + await unlink(tempPath).catch(() => undefined); + } +} + +export async function restoreFile( + sourcePath: string, + targetPath: string, + mode?: number, +): Promise { + const source = await readFile(sourcePath); + const preservedMode = mode ?? (await fileMode(sourcePath)); + const temporaryPath = join( + dirname(targetPath), + `.${basename(targetPath)}.${process.pid}.${randomUUID()}.restore.tmp`, + ); + await writeFile(temporaryPath, source, { flag: 'wx', mode: preservedMode }); + try { + await chmod(temporaryPath, preservedMode); + await replaceFile(temporaryPath, targetPath); + } finally { + await unlink(temporaryPath).catch(() => undefined); + } +} + +export async function replaceFile(sourcePath: string, targetPath: string): Promise { + try { + await rename(sourcePath, targetPath); + } catch (error) { + if (!isWindowsReplacementError(error)) throw storageError(targetPath, error); + const displaced = `${targetPath}.${process.pid}.${randomUUID()}.displaced`; + try { + await rename(targetPath, displaced); + } catch (displaceError) { + throw storageError(targetPath, displaceError); + } + try { + await rename(sourcePath, targetPath); + } catch (replaceError) { + try { + await rename(displaced, targetPath); + } catch (restoreError) { + throw new SetupStorageError( + `Could not replace ${targetPath} and could not restore its original file.`, + restoreError, + ); + } + throw storageError(targetPath, replaceError); + } + await unlink(displaced).catch(() => undefined); + } +} + +async function createExclusiveBackup( + targetPath: string, + now: Date, + contents: Buffer, + mode: number, +): Promise { + const stamp = now.toISOString().replaceAll('-', '').replaceAll(':', ''); + const base = `${targetPath}.relay-backup-${stamp}`; + for (let suffix = 0; suffix < 10_000; suffix += 1) { + const candidate = suffix === 0 ? base : `${base}-${suffix}`; + try { + await writeFile(candidate, contents, { flag: 'wx', mode }); + await chmod(candidate, mode); + return candidate; + } catch (error) { + if (isExists(error)) continue; + throw storageError(candidate, error); + } + } + throw new SetupStorageError(`Could not allocate a collision-safe backup for ${targetPath}.`); +} + +async function fileMode(path: string): Promise { + try { + return (await stat(path)).mode & 0o777; + } catch (error) { + if (isMissing(error)) return 0o600; + throw storageError(path, error); + } +} + +function storageError(path: string, cause: unknown): SetupStorageError { + return new SetupStorageError( + `Could not safely update ${path}. Check permissions and retry.`, + cause, + ); +} + +function isMissing(error: unknown): boolean { + return typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT'; +} + +function isExists(error: unknown): boolean { + return typeof error === 'object' && error !== null && 'code' in error && error.code === 'EEXIST'; +} + +function isWindowsReplacementError(error: unknown): boolean { + return ( + process.platform === 'win32' && + typeof error === 'object' && + error !== null && + 'code' in error && + (error.code === 'EEXIST' || error.code === 'EPERM') + ); +} diff --git a/src/distribution/setup/clients/claude-json-adapter.ts b/src/distribution/setup/clients/claude-json-adapter.ts new file mode 100644 index 0000000..a05e564 --- /dev/null +++ b/src/distribution/setup/clients/claude-json-adapter.ts @@ -0,0 +1,92 @@ +import { modify, parse, type ParseError } from 'jsonc-parser'; +import type { ClientConfigAdapter, ClientEntryState } from './client-adapter.js'; +import type { MutableIntegrationClient } from '../setup-types.js'; +import { renderIntegrationSnippet } from '../snippets.js'; +import { SetupUsageError } from '../setup-errors.js'; + +const relayEntry = { command: 'relay', args: ['mcp'] } as const; + +export function createClaudeJsonAdapter(): ClientConfigAdapter { + return { + client: 'claude-code', + parse: (content) => readDocument(content), + inspect: (content) => inspect(readDocument(content)), + upsertRelayEntry: (content) => { + const document = readDocument(content); + if (inspect(document).kind === 'matching') return content; + const edits = modify(content, ['mcpServers', 'relay'], relayEntry, { + formattingOptions: { insertSpaces: true, tabSize: 2, eol: newlineFor(content) }, + }); + return applyEdits(content, edits); + }, + removeRelayEntry: (content) => { + readDocument(content); + const edits = modify(content, ['mcpServers', 'relay'], undefined, { + formattingOptions: { insertSpaces: true, tabSize: 2, eol: newlineFor(content) }, + }); + return applyEdits(content, edits); + }, + renderSnippet: () => renderIntegrationSnippet('claude-code'), + }; +} + +function readDocument(content: string): Record { + if (content.trim() === '') return {}; + const errors: ParseError[] = []; + const value = parse(content, errors, { allowTrailingComma: true }); + if ( + errors.length > 0 || + !isRecord(value) || + (value.mcpServers !== undefined && !isRecord(value.mcpServers)) + ) { + throw new SetupUsageError('Claude Code configuration is malformed.'); + } + return value; +} + +function inspect(document: Record): ClientEntryState { + const servers = document.mcpServers; + if (servers === undefined) return { kind: 'absent' }; + if (!isRecord(servers)) + throw new SetupUsageError('Claude Code mcpServers configuration is malformed.'); + const relay = servers.relay; + if (relay === undefined) return { kind: 'absent' }; + if (!isRecord(relay)) return { kind: 'conflicting' }; + const command = typeof relay.command === 'string' ? relay.command : undefined; + const args = + Array.isArray(relay.args) && relay.args.every((arg): arg is string => typeof arg === 'string') + ? relay.args + : undefined; + const keys = Object.keys(relay); + if (keys.length === 2 && command === 'relay' && args?.length === 1 && args[0] === 'mcp') + return { kind: 'matching', command, args }; + return { + kind: 'conflicting', + ...(command === undefined ? {} : { command }), + ...(args === undefined ? {} : { args }), + }; +} + +function applyEdits( + content: string, + edits: readonly { offset: number; length: number; content: string }[], +): string { + return edits + .slice() + .sort((left, right) => right.offset - left.offset) + .reduce( + (current, edit) => + `${current.slice(0, edit.offset)}${edit.content}${current.slice(edit.offset + edit.length)}`, + content, + ); +} + +function newlineFor(content: string): string { + return content.includes('\r\n') ? '\r\n' : '\n'; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +export const claudeClient: MutableIntegrationClient = 'claude-code'; diff --git a/src/distribution/setup/clients/client-adapter.ts b/src/distribution/setup/clients/client-adapter.ts new file mode 100644 index 0000000..b95b19b --- /dev/null +++ b/src/distribution/setup/clients/client-adapter.ts @@ -0,0 +1,18 @@ +import type { MutableIntegrationClient } from '../setup-types.js'; + +export type { MutableIntegrationClient } from '../setup-types.js'; + +export interface ClientEntryState { + readonly kind: 'absent' | 'matching' | 'conflicting'; + readonly command?: string; + readonly args?: readonly string[]; +} + +export interface ClientConfigAdapter { + readonly client: MutableIntegrationClient; + parse(content: string): void; + inspect(content: string): ClientEntryState; + upsertRelayEntry(content: string): string; + removeRelayEntry(content: string): string; + renderSnippet(): string; +} diff --git a/src/distribution/setup/clients/codex-toml-adapter.ts b/src/distribution/setup/clients/codex-toml-adapter.ts new file mode 100644 index 0000000..b3ba5f2 --- /dev/null +++ b/src/distribution/setup/clients/codex-toml-adapter.ts @@ -0,0 +1,92 @@ +import { parse as parseToml } from '@iarna/toml'; +import type { ClientConfigAdapter, ClientEntryState } from './client-adapter.js'; +import { renderIntegrationSnippet } from '../snippets.js'; +import { SetupConflictError, SetupUsageError } from '../setup-errors.js'; + +const headerPattern = /^\s*\[mcp_servers\.relay\][^\r\n]*(?:\r?\n|$)/gm; + +export function createCodexTomlAdapter(): ClientConfigAdapter { + return { + client: 'codex', + parse: (content) => parseDocument(content), + inspect: (content) => inspect(content), + upsertRelayEntry: (content) => { + const state = inspect(content); + if (state.kind === 'matching') return content; + if (state.kind === 'conflicting') + throw new SetupConflictError('Codex configuration contains a conflicting relay entry.'); + const newline = newlineFor(content); + const snippet = renderIntegrationSnippet('codex').replaceAll('\n', newline); + if (content.length === 0) return snippet; + return `${content}${content.endsWith(newline) ? newline : `${newline}${newline}`}${snippet}`; + }, + removeRelayEntry: (content) => { + const match = singleRelayHeader(content); + if (match === undefined) return content; + return `${content.slice(0, match.start)}${content.slice(match.end)}`; + }, + renderSnippet: () => renderIntegrationSnippet('codex'), + }; +} + +function parseDocument(content: string): Record { + try { + return parseToml(content) as Record; + } catch { + throw new SetupUsageError('Codex configuration is malformed.'); + } +} + +function inspect(content: string): ClientEntryState { + const document = parseDocument(content); + const servers = document.mcp_servers; + if (servers === undefined) return { kind: 'absent' }; + if (!isRecord(servers)) + throw new SetupUsageError('Codex mcp_servers configuration is malformed.'); + if (servers.relay === undefined) return { kind: 'absent' }; + if (!isRecord(servers.relay)) return { kind: 'conflicting' }; + const command = typeof servers.relay.command === 'string' ? servers.relay.command : undefined; + const args = + Array.isArray(servers.relay.args) && + servers.relay.args.every((arg): arg is string => typeof arg === 'string') + ? servers.relay.args + : undefined; + const keys = Object.keys(servers.relay); + if ( + keys.length === 2 && + command === 'relay' && + args?.length === 1 && + args[0] === 'mcp' && + singleRelayHeader(content) !== undefined + ) + return { kind: 'matching', command, args }; + return { + kind: 'conflicting', + ...(command === undefined ? {} : { command }), + ...(args === undefined ? {} : { args }), + }; +} + +function singleRelayHeader(content: string): { start: number; end: number } | undefined { + const matches = [...content.matchAll(headerPattern)]; + if (matches.length !== 1) { + if (matches.length > 1) + throw new SetupConflictError('Codex configuration contains duplicate relay tables.'); + return undefined; + } + const match = matches[0]; + if (match === undefined) return undefined; + if (match.index === undefined) return undefined; + const nextHeader = /\r?\n\s*\[[^\r\n\]]+\]/g; + nextHeader.lastIndex = match.index + match[0].length; + const next = nextHeader.exec(content); + return { start: match.index, end: next?.index ?? content.length }; +} + +function newlineFor(content: string): string { + return content.includes('\r\n') ? '\r\n' : '\n'; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} diff --git a/src/distribution/setup/initialize-relay.ts b/src/distribution/setup/initialize-relay.ts new file mode 100644 index 0000000..7c0e3c5 --- /dev/null +++ b/src/distribution/setup/initialize-relay.ts @@ -0,0 +1,50 @@ +import type { mkdir } from 'node:fs/promises'; +import type { RuntimePaths } from '../resolve-runtime-paths.js'; +import { SetupStorageError } from './setup-errors.js'; + +export interface InitializeRelayDependencies { + readonly runtimePaths: RuntimePaths; + readonly openRuntime: (databasePath: string) => { close(): void }; + readonly mkdir: typeof mkdir; +} + +export interface InitializeRelayResult { + readonly dataRoot: string; + readonly configRoot: string; + readonly databasePath: string; + readonly createdDirectories: readonly string[]; +} + +export async function initializeRelay( + dependencies: InitializeRelayDependencies, +): Promise { + const createdDirectories: string[] = []; + try { + for (const directory of [ + dependencies.runtimePaths.dataRoot, + dependencies.runtimePaths.configRoot, + ]) { + const created = await dependencies.mkdir(directory, { recursive: true }); + if (created !== undefined) createdDirectories.push(created); + } + } catch (error) { + throw new SetupStorageError('Relay directories could not be initialized.', error); + } + + let runtime: { close(): void }; + try { + runtime = dependencies.openRuntime(dependencies.runtimePaths.databasePath); + } catch (error) { + throw new SetupStorageError('Relay database could not be initialized.', error); + } + try { + return { + dataRoot: dependencies.runtimePaths.dataRoot, + configRoot: dependencies.runtimePaths.configRoot, + databasePath: dependencies.runtimePaths.databasePath, + createdDirectories, + }; + } finally { + runtime.close(); + } +} diff --git a/src/distribution/setup/ownership-store.ts b/src/distribution/setup/ownership-store.ts new file mode 100644 index 0000000..e472916 --- /dev/null +++ b/src/distribution/setup/ownership-store.ts @@ -0,0 +1,130 @@ +import { randomUUID } from 'node:crypto'; +import { mkdir, readFile, unlink, writeFile } from 'node:fs/promises'; +import { dirname, normalize, resolve } from 'node:path'; +import { replaceFile } from './backup-and-atomic-write.js'; +import type { RelayIntegrationOwnership, RelayOwnershipFile } from './setup-types.js'; +import { SetupStorageError } from './setup-errors.js'; + +export interface OwnershipStore { + read(): Promise; + write(next: RelayOwnershipFile): Promise; +} + +export function createOwnershipStore(input: { + readonly metadataPath: string; + readonly applicationVersion: string; +}): OwnershipStore { + return { + read: async () => { + let source: string; + try { + source = await readFile(input.metadataPath, 'utf8'); + } catch (error) { + if (isMissingFile(error)) return { schemaVersion: 1, integrations: [] }; + throw new SetupStorageError( + `Relay ownership metadata could not be read at ${input.metadataPath}.`, + error, + ); + } + try { + return validateOwnership(JSON.parse(source), input.applicationVersion); + } catch (error) { + throw new SetupStorageError( + `Relay ownership metadata schema is invalid at ${input.metadataPath}.`, + { + cause: error, + }, + ); + } + }, + write: async (next) => { + let validated: RelayOwnershipFile; + try { + validated = validateOwnership(next, input.applicationVersion); + await mkdir(dirname(input.metadataPath), { recursive: true }); + } catch (error) { + throw new SetupStorageError( + `Relay ownership metadata could not be prepared at ${input.metadataPath}.`, + error, + ); + } + const temporaryPath = `${input.metadataPath}.${process.pid}.${randomUUID()}.tmp`; + try { + await writeFile(temporaryPath, `${JSON.stringify(validated, null, 2)}\n`, 'utf8'); + await replaceFile(temporaryPath, input.metadataPath); + } catch (error) { + await unlink(temporaryPath).catch(() => undefined); + throw new SetupStorageError( + `Relay ownership metadata could not be written at ${input.metadataPath}.`, + error, + ); + } + }, + }; +} + +function validateOwnership(value: unknown, _applicationVersion: string): RelayOwnershipFile { + if (!isRecord(value) || value.schemaVersion !== 1 || !Array.isArray(value.integrations)) { + throw new Error('Relay ownership metadata has an unsupported schema.'); + } + const integrations = value.integrations.map((record) => validateRecord(record)); + const seen = new Set(); + for (const record of integrations) { + const key = `${record.client}:${pathKey(record.configPath)}`; + if (seen.has(key)) throw new Error('Relay ownership metadata contains duplicate integrations.'); + seen.add(key); + } + integrations.sort((left, right) => + `${left.client}:${pathKey(left.configPath)}`.localeCompare( + `${right.client}:${pathKey(right.configPath)}`, + ), + ); + return { schemaVersion: 1, integrations }; +} + +function validateRecord(value: unknown): RelayIntegrationOwnership { + if (!isRecord(value)) throw new Error('Relay ownership record is invalid.'); + if ( + (value.client !== 'codex' && value.client !== 'claude-code') || + value.entryId !== 'relay' || + value.command !== 'relay' || + !Array.isArray(value.args) || + value.args.length !== 1 || + value.args[0] !== 'mcp' || + (value.status !== 'enabled' && value.status !== 'disabled') || + typeof value.applicationVersion !== 'string' || + typeof value.lastSuccessfulSetupAt !== 'string' || + typeof value.configPath !== 'string' || + !isAbsolutePath(value.configPath) + ) { + throw new Error('Relay ownership record is invalid.'); + } + return { + client: value.client, + configPath: normalize(resolve(value.configPath)), + entryId: 'relay', + command: 'relay', + args: ['mcp'], + status: value.status, + applicationVersion: value.applicationVersion, + lastSuccessfulSetupAt: value.lastSuccessfulSetupAt, + ...(typeof value.lastBackupPath === 'string' ? { lastBackupPath: value.lastBackupPath } : {}), + }; +} + +function isAbsolutePath(value: string): boolean { + return /^[A-Za-z]:[\\/]/.test(value) || value.startsWith('\\\\') || value.startsWith('/'); +} + +function pathKey(value: string): string { + const normalized = normalize(resolve(value)); + return process.platform === 'win32' ? normalized.toLowerCase() : normalized; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isMissingFile(error: unknown): boolean { + return isRecord(error) && error.code === 'ENOENT'; +} diff --git a/src/distribution/setup/plan-integration-change.ts b/src/distribution/setup/plan-integration-change.ts new file mode 100644 index 0000000..e8d27db --- /dev/null +++ b/src/distribution/setup/plan-integration-change.ts @@ -0,0 +1,114 @@ +import { createHash } from 'node:crypto'; +import { readFile } from 'node:fs/promises'; +import { isAbsolute, normalize, resolve } from 'node:path'; +import type { ClientConfigAdapter } from './clients/client-adapter.js'; +import { + SetupConflictError, + SetupNotFoundError, + SetupStorageError, + SetupUsageError, +} from './setup-errors.js'; +import type { + IntegrationChangePlan, + MutableIntegrationClient, + RelayOwnershipFile, +} from './setup-types.js'; + +export async function planIntegrationChange(input: { + readonly action: 'setup' | 'disable' | 'remove'; + readonly client: MutableIntegrationClient; + readonly configPath: string; + readonly adapter: ClientConfigAdapter; + readonly ownership: RelayOwnershipFile; +}): Promise { + assertAbsoluteConfigPath(input.configPath); + const configPath = normalize(resolve(input.configPath)); + const content = await readClientFile(configPath); + const state = input.adapter.inspect(content); + const ownership = input.ownership.integrations.find( + (record) => record.client === input.client && samePath(record.configPath, configPath), + ); + const otherOwnership = input.ownership.integrations.find( + (record) => record.client !== input.client && samePath(record.configPath, configPath), + ); + if (otherOwnership !== undefined) + throw new SetupConflictError('Relay ownership belongs to another client.'); + if (state.kind === 'conflicting') + throw new SetupConflictError('The configuration contains a conflicting relay entry.'); + + let operation: IntegrationChangePlan['operation']; + let nextContent = content; + if (input.action === 'setup') { + if (state.kind === 'matching' && ownership?.status === 'enabled') operation = 'unchanged'; + else if (state.kind === 'matching' && ownership?.status === 'disabled') operation = 'updated'; + else if (state.kind === 'matching') + throw new SetupConflictError('A matching relay entry is not Relay-owned.'); + else if (ownership?.status === 'enabled') + throw new SetupConflictError('Relay ownership exists but its entry is missing.'); + else { + operation = 'created'; + nextContent = input.adapter.upsertRelayEntry(content); + } + } else if (input.action === 'disable') { + if (ownership?.status !== 'enabled') + throw new SetupNotFoundError('No enabled Relay integration owns this entry.'); + if (state.kind !== 'matching') + throw new SetupConflictError('The owned relay entry is not present or no longer matches.'); + operation = 'disabled'; + nextContent = input.adapter.removeRelayEntry(content); + } else { + if (ownership === undefined) + throw new SetupNotFoundError('No Relay ownership record exists for this entry.'); + if (state.kind === 'matching') { + operation = 'removed'; + nextContent = input.adapter.removeRelayEntry(content); + } else if (ownership.status === 'disabled') { + operation = 'removed'; + } else { + throw new SetupConflictError('The owned relay entry is not present or no longer matches.'); + } + } + + return { + client: input.client, + configPath, + entryId: 'relay', + operation, + changed: + nextContent !== content || + (operation === 'updated' && ownership?.status === 'disabled') || + (operation === 'removed' && ownership?.status === 'disabled'), + beforeFingerprint: fingerprint(content), + nextContent, + snippet: input.adapter.renderSnippet(), + }; +} + +async function readClientFile(path: string): Promise { + try { + return await readFile(path, 'utf8'); + } catch (error) { + if (isMissing(error)) return ''; + throw new SetupStorageError(`Could not read configuration at ${path}.`, error); + } +} + +export function fingerprint(content: string | Buffer): string { + return createHash('sha256').update(content).digest('hex'); +} + +function samePath(left: string, right: string): boolean { + const normalizedLeft = normalize(resolve(left)); + const normalizedRight = normalize(resolve(right)); + return process.platform === 'win32' + ? normalizedLeft.toLowerCase() === normalizedRight.toLowerCase() + : normalizedLeft === normalizedRight; +} + +function isMissing(error: unknown): boolean { + return typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT'; +} + +export function assertAbsoluteConfigPath(configPath: string): void { + if (!isAbsolute(configPath)) throw new SetupUsageError('Configuration path must be absolute.'); +} diff --git a/src/distribution/setup/setup-errors.ts b/src/distribution/setup/setup-errors.ts new file mode 100644 index 0000000..f6c6782 --- /dev/null +++ b/src/distribution/setup/setup-errors.ts @@ -0,0 +1,6 @@ +import { RelayError } from '../../shared/errors.js'; + +export class SetupUsageError extends Error {} +export class SetupNotFoundError extends Error {} +export class SetupConflictError extends Error {} +export class SetupStorageError extends RelayError {} diff --git a/src/distribution/setup/setup-types.ts b/src/distribution/setup/setup-types.ts new file mode 100644 index 0000000..03a10d1 --- /dev/null +++ b/src/distribution/setup/setup-types.ts @@ -0,0 +1,43 @@ +export type MutableIntegrationClient = 'codex' | 'claude-code'; +export type IntegrationClient = MutableIntegrationClient | 'generic-mcp'; + +export type IntegrationStatus = 'enabled' | 'disabled'; + +export interface RelayOwnershipFile { + readonly schemaVersion: 1; + readonly integrations: readonly RelayIntegrationOwnership[]; +} + +export interface RelayIntegrationOwnership { + readonly client: MutableIntegrationClient; + readonly configPath: string; + readonly entryId: 'relay'; + readonly command: 'relay'; + readonly args: readonly ['mcp']; + readonly status: IntegrationStatus; + readonly applicationVersion: string; + readonly lastSuccessfulSetupAt: string; + readonly lastBackupPath?: string; +} + +export type IntegrationOperation = 'created' | 'updated' | 'unchanged' | 'disabled' | 'removed'; + +export interface IntegrationChangePlan { + readonly client: MutableIntegrationClient; + readonly configPath: string; + readonly entryId: 'relay'; + readonly operation: IntegrationOperation; + readonly changed: boolean; + readonly beforeFingerprint: string; + readonly nextContent: string; + readonly snippet: string; +} + +export interface IntegrationChangeResult { + readonly client: MutableIntegrationClient; + readonly configPath: string; + readonly entryId: 'relay'; + readonly operation: IntegrationOperation; + readonly changed: boolean; + readonly backupPath?: string; +} diff --git a/src/distribution/setup/snippets.ts b/src/distribution/setup/snippets.ts new file mode 100644 index 0000000..d7c6fb0 --- /dev/null +++ b/src/distribution/setup/snippets.ts @@ -0,0 +1,13 @@ +import type { IntegrationClient } from './setup-types.js'; + +const installedServer = { command: 'relay', args: ['mcp'] as const }; + +export function renderIntegrationSnippet(client: IntegrationClient): string { + if (client === 'codex') { + return '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n'; + } + if (client === 'claude-code') { + return `${JSON.stringify({ mcpServers: { relay: installedServer } }, null, 2)}\n`; + } + return `${JSON.stringify(installedServer, null, 2)}\n`; +} diff --git a/src/interfaces/cli/main.ts b/src/interfaces/cli/main.ts index 6fe5e70..3108b23 100644 --- a/src/interfaces/cli/main.ts +++ b/src/interfaces/cli/main.ts @@ -2,7 +2,13 @@ import { runCli } from './run-cli.js'; import { runRelay } from './run-relay.js'; import { createTaskRuntime } from '../shared/create-task-runtime.js'; -import { runMcpServer, runUiServer } from '../production-dependencies.js'; +import { + createOperationalDependencies, + runMcpServer, + runUiServer, +} from '../production-dependencies.js'; +import { runOperationalCommand } from './run-operational-command.js'; +import { writeOperationalError } from './operational-output.js'; void runRelay(process.argv.slice(2), { runTaskCommand: (argv) => @@ -13,6 +19,16 @@ void runRelay(process.argv.slice(2), { }), runMcp: runMcpServer, runUi: runUiServer, + runOperationalCommand: async (argv) => { + try { + return await runOperationalCommand( + argv, + createOperationalDependencies({ stdout: process.stdout, stderr: process.stderr }), + ); + } catch (error) { + return writeOperationalError(process.stdout, process.stderr, error); + } + }, stderr: process.stderr, }).then((exitCode) => { process.exitCode = exitCode; diff --git a/src/interfaces/cli/operational-output.ts b/src/interfaces/cli/operational-output.ts new file mode 100644 index 0000000..ad56c4f --- /dev/null +++ b/src/interfaces/cli/operational-output.ts @@ -0,0 +1,42 @@ +import { cliFailure, cliSuccess } from './output/cli-result.js'; +import { + SetupConflictError, + SetupNotFoundError, + SetupStorageError, + SetupUsageError, +} from '../../distribution/setup/setup-errors.js'; +import { CliUsageError } from './output/cli-errors.js'; +import { RelayError } from '../../shared/errors.js'; + +type Writer = { write(text: string): unknown }; + +export function writeOperationalSuccess( + stdout: Writer, + command: string, + data: Record, +): void { + stdout.write(`${JSON.stringify(cliSuccess({ command, ...data }))}\n`); +} + +export function writeOperationalError(stdout: Writer, stderr: Writer, error: unknown): number { + const mapped = mapOperationalError(error); + stdout.write(`${JSON.stringify(cliFailure(mapped.code, mapped.message))}\n`); + stderr.write(`${mapped.message}\n`); + return mapped.exitCode; +} + +function mapOperationalError(error: unknown): { code: string; message: string; exitCode: number } { + if (error instanceof SetupNotFoundError) + return { code: 'NOT_FOUND', message: error.message, exitCode: 3 }; + if (error instanceof SetupConflictError) + return { code: 'CONFLICT', message: error.message, exitCode: 4 }; + if (error instanceof SetupStorageError) + return { code: 'STORAGE_ERROR', message: error.message, exitCode: 5 }; + if ( + error instanceof CliUsageError || + error instanceof SetupUsageError || + error instanceof RelayError + ) + return { code: 'VALIDATION_ERROR', message: error.message, exitCode: 2 }; + return { code: 'INTERNAL_ERROR', message: 'An unexpected internal error occurred.', exitCode: 1 }; +} diff --git a/src/interfaces/cli/parse-operational-command.ts b/src/interfaces/cli/parse-operational-command.ts new file mode 100644 index 0000000..b0dda24 --- /dev/null +++ b/src/interfaces/cli/parse-operational-command.ts @@ -0,0 +1,144 @@ +import { isAbsolute } from 'node:path'; +import type { + IntegrationClient, + MutableIntegrationClient, +} from '../../distribution/setup/setup-types.js'; +import { CliUsageError } from './output/cli-errors.js'; + +export type OperationalCommand = + | { + readonly kind: 'setup'; + readonly client?: IntegrationClient; + readonly configFile?: string; + readonly apply: boolean; + } + | { readonly kind: 'config-paths' } + | { readonly kind: 'config-integrations' } + | { readonly kind: 'config-snippet'; readonly client: IntegrationClient } + | { + readonly kind: 'config-disable' | 'config-remove'; + readonly client: MutableIntegrationClient; + readonly configFile: string; + readonly apply: true; + }; + +export function parseOperationalCommand(argv: readonly string[]): OperationalCommand { + const [group, ...rest] = argv; + if (group === 'setup') return parseSetup(rest); + const [action, ...tokens] = rest; + if (group !== 'config') throw new CliUsageError('Unknown or missing command.'); + if (action === 'paths') return exact('config paths', tokens, { kind: 'config-paths' }); + if (action === 'integrations') + return exact('config integrations', tokens, { kind: 'config-integrations' }); + if (action === 'snippet') { + const options = parseOptions(tokens, { client: true, output: true }); + const client = clientValue(options.client); + if (client === undefined) throw new CliUsageError('Missing required option --client.'); + return { kind: 'config-snippet', client }; + } + if (action === 'disable' || action === 'remove') { + const options = parseOptions(tokens, { + client: true, + 'config-file': true, + apply: false, + output: true, + }); + const client = mutableClient(options.client); + const configFile = absoluteConfigFile(options['config-file']); + if (!options.apply) throw new CliUsageError(`${action} requires --apply.`); + return { + kind: action === 'disable' ? 'config-disable' : 'config-remove', + client, + configFile, + apply: true, + }; + } + throw new CliUsageError(`Unknown command: config ${action ?? ''}`.trim()); +} + +function parseSetup(tokens: readonly string[]): OperationalCommand { + const options = parseOptions(tokens, { + client: true, + 'config-file': true, + apply: false, + output: true, + }); + const client = clientValue(options.client); + const configFile = options['config-file']; + if (client === undefined) { + if (configFile !== undefined || options.apply) + throw new CliUsageError('--client is required for client setup.'); + return { kind: 'setup', apply: false }; + } + if (client === 'generic-mcp') { + if (configFile !== undefined || options.apply) + throw new CliUsageError('generic-mcp setup is snippet-only.'); + return { kind: 'setup', client, apply: false }; + } + return { + kind: 'setup', + client, + configFile: absoluteConfigFile(configFile), + apply: Boolean(options.apply), + }; +} + +function parseOptions( + tokens: readonly string[], + specs: Readonly>, +): Record { + const result: Record = {}; + for (let index = 0; index < tokens.length; index += 1) { + const token = tokens[index]; + if (token === undefined || !token.startsWith('--')) + throw new CliUsageError(`Unexpected argument: ${token ?? ''}`.trim()); + const key = token.slice(2); + if (!(key in specs)) throw new CliUsageError(`Unknown option --${key}.`); + if (result[key] !== undefined) + throw new CliUsageError(`Option --${key} may be supplied only once.`); + if (!specs[key]) { + result[key] = true; + continue; + } + const value = tokens[index + 1]; + if (value === undefined || value.startsWith('--')) + throw new CliUsageError(`Missing value for --${key}.`); + result[key] = value; + if (key === 'output' && value !== 'json') + throw new CliUsageError('Operational commands support only --output json.'); + index += 1; + } + return result; +} + +function clientValue(value: string | true | undefined): IntegrationClient | undefined { + if (value === undefined) return undefined; + if (value === 'codex' || value === 'claude-code' || value === 'generic-mcp') return value; + throw new CliUsageError('Unsupported integration client.'); +} + +function mutableClient(value: string | true | undefined): MutableIntegrationClient { + const client = clientValue(value); + if (client !== 'codex' && client !== 'claude-code') + throw new CliUsageError('This operation supports only Codex or Claude Code.'); + return client; +} + +function absoluteConfigFile(value: string | true | undefined): string { + if (typeof value !== 'string' || value.trim() === '' || !isAbsolute(value)) + throw new CliUsageError('--config-file must be an absolute path.'); + return value; +} + +function exact( + name: string, + tokens: readonly string[], + command: T, +): T { + if ( + tokens.length > 0 && + !(tokens.length === 2 && tokens[0] === '--output' && tokens[1] === 'json') + ) + throw new CliUsageError(`${name} does not accept options.`); + return command; +} diff --git a/src/interfaces/cli/run-operational-command.ts b/src/interfaces/cli/run-operational-command.ts new file mode 100644 index 0000000..122f5c5 --- /dev/null +++ b/src/interfaces/cli/run-operational-command.ts @@ -0,0 +1,138 @@ +import { mkdir } from 'node:fs/promises'; +import { join } from 'node:path'; +import type { RuntimePaths } from '../../distribution/resolve-runtime-paths.js'; +import { initializeRelay } from '../../distribution/setup/initialize-relay.js'; +import { + createOwnershipStore, + type OwnershipStore, +} from '../../distribution/setup/ownership-store.js'; +import { applyIntegrationChange } from '../../distribution/setup/apply-integration-change.js'; +import { createClaudeJsonAdapter } from '../../distribution/setup/clients/claude-json-adapter.js'; +import { createCodexTomlAdapter } from '../../distribution/setup/clients/codex-toml-adapter.js'; +import { planIntegrationChange } from '../../distribution/setup/plan-integration-change.js'; +import { renderIntegrationSnippet } from '../../distribution/setup/snippets.js'; +import type { MutableIntegrationClient } from '../../distribution/setup/setup-types.js'; +import { parseOperationalCommand, type OperationalCommand } from './parse-operational-command.js'; +import { writeOperationalError, writeOperationalSuccess } from './operational-output.js'; + +export interface OperationalDependencies { + readonly runtimePaths: RuntimePaths; + readonly openRuntime: (databasePath: string) => { close(): void }; + readonly applicationVersion: string; + readonly ownershipStore?: OwnershipStore; + readonly stdout: { write(text: string): unknown }; + readonly stderr: { write(text: string): unknown }; + readonly now?: () => Date; +} + +export async function runOperationalCommand( + argv: readonly string[], + dependencies: OperationalDependencies, +): Promise { + let command: OperationalCommand; + try { + command = parseOperationalCommand(argv); + } catch (error) { + return writeOperationalError(dependencies.stdout, dependencies.stderr, error); + } + try { + const store = + dependencies.ownershipStore ?? + createOwnershipStore({ + metadataPath: join(dependencies.runtimePaths.configRoot, 'config.json'), + applicationVersion: dependencies.applicationVersion, + }); + const needsInitialization = + command.kind === 'setup' || + command.kind === 'config-disable' || + command.kind === 'config-remove'; + const initialized = needsInitialization + ? await initializeRelay({ + runtimePaths: dependencies.runtimePaths, + openRuntime: dependencies.openRuntime, + mkdir, + }) + : undefined; + if (command.kind === 'setup' && command.client === undefined && initialized !== undefined) { + writeOperationalSuccess(dependencies.stdout, 'setup', { + changed: initialized.createdDirectories.length > 0, + createdDirectories: initialized.createdDirectories, + paths: initialized, + }); + return 0; + } + if (command.kind === 'config-paths') { + writeOperationalSuccess(dependencies.stdout, 'config paths', { + paths: { + ...dependencies.runtimePaths, + metadataPath: join(dependencies.runtimePaths.configRoot, 'config.json'), + }, + }); + return 0; + } + if (command.kind === 'config-integrations') { + const ownership = await store.read(); + writeOperationalSuccess(dependencies.stdout, 'config integrations', { + integrations: ownership.integrations, + }); + return 0; + } + if (command.kind === 'config-snippet') { + writeOperationalSuccess(dependencies.stdout, 'config snippet', { + client: command.client, + snippet: renderIntegrationSnippet(command.client), + changed: false, + }); + return 0; + } + if (command.kind === 'setup' && command.client === 'generic-mcp') { + writeOperationalSuccess(dependencies.stdout, 'setup', { + client: command.client, + changed: false, + operation: 'unchanged', + snippet: renderIntegrationSnippet(command.client), + }); + return 0; + } + const client = command.client as MutableIntegrationClient; + const configPath = command.configFile!; + const adapter = client === 'codex' ? createCodexTomlAdapter() : createClaudeJsonAdapter(); + const ownership = await store.read(); + const action = + command.kind === 'config-disable' + ? 'disable' + : command.kind === 'config-remove' + ? 'remove' + : 'setup'; + const plan = await planIntegrationChange({ action, client, configPath, adapter, ownership }); + if (command.kind === 'setup' && !command.apply) { + writeOperationalSuccess(dependencies.stdout, 'setup', { + client, + changed: plan.changed, + operation: plan.operation, + path: plan.configPath, + entryId: plan.entryId, + snippet: plan.snippet, + }); + return 0; + } + const result = await applyIntegrationChange({ + plan, + adapter, + ownershipStore: store, + applicationVersion: dependencies.applicationVersion, + now: dependencies.now?.() ?? new Date(), + }); + writeOperationalSuccess(dependencies.stdout, action, { + client: result.client, + changed: result.changed, + operation: result.operation, + path: result.configPath, + entryId: result.entryId, + ...(result.backupPath === undefined ? {} : { backupPath: result.backupPath }), + }); + return 0; + } catch (error) { + return writeOperationalError(dependencies.stdout, dependencies.stderr, error); + } +} diff --git a/src/interfaces/cli/run-relay.ts b/src/interfaces/cli/run-relay.ts index 2600770..ec15530 100644 --- a/src/interfaces/cli/run-relay.ts +++ b/src/interfaces/cli/run-relay.ts @@ -2,6 +2,7 @@ export interface RelayCommandDependencies { readonly runTaskCommand: (argv: readonly string[]) => Promise; readonly runMcp: () => Promise; readonly runUi: () => Promise; + readonly runOperationalCommand?: (argv: readonly string[]) => Promise; readonly stderr: { write(text: string): unknown }; } @@ -12,6 +13,13 @@ export async function runRelay( const command = argv[0]; if (command === 'mcp') return (await dependencies.runMcp()) ?? 0; if (command === 'ui') return (await dependencies.runUi()) ?? 0; + if (command === 'setup' || command === 'config') { + if (dependencies.runOperationalCommand === undefined) { + dependencies.stderr.write('Operational command runner is unavailable.\n'); + return 1; + } + return dependencies.runOperationalCommand(argv); + } if (command === 'task' || command === 'session') return dependencies.runTaskCommand(argv); dependencies.stderr.write('Unknown or missing command.\n'); return 2; diff --git a/src/interfaces/production-dependencies.ts b/src/interfaces/production-dependencies.ts index d15b324..d0e3673 100644 --- a/src/interfaces/production-dependencies.ts +++ b/src/interfaces/production-dependencies.ts @@ -1,4 +1,30 @@ import { runMcpServer } from './mcp/main.js'; import { runUiServer } from './http/main.js'; +import { join } from 'node:path'; +import { resolveRuntimePaths } from '../distribution/resolve-runtime-paths.js'; +import { readPackageVersion } from '../distribution/package-version.js'; +import { createTaskRuntime } from './shared/create-task-runtime.js'; +import { createOwnershipStore } from '../distribution/setup/ownership-store.js'; +import type { OperationalDependencies } from './cli/run-operational-command.js'; export { runMcpServer, runUiServer }; + +export function createOperationalDependencies(output: { + stdout: { write(text: string): unknown }; + stderr: { write(text: string): unknown }; +}): OperationalDependencies { + const runtimePaths = resolveRuntimePaths(); + const applicationVersion = readPackageVersion(); + return { + runtimePaths, + applicationVersion, + openRuntime: (databasePath) => createTaskRuntime({ databasePath }), + ownershipStore: createOwnershipStore({ + metadataPath: join(runtimePaths.configRoot, 'config.json'), + applicationVersion, + }), + stdout: output.stdout, + stderr: output.stderr, + now: () => new Date(), + }; +} diff --git a/tests/fixtures/agent-integrations/valid/integrations/claude-code/.mcp.json.example b/tests/fixtures/agent-integrations/valid/integrations/claude-code/.mcp.json.example index 8e8c34e..161617e 100644 --- a/tests/fixtures/agent-integrations/valid/integrations/claude-code/.mcp.json.example +++ b/tests/fixtures/agent-integrations/valid/integrations/claude-code/.mcp.json.example @@ -1,3 +1,3 @@ { - "mcpServers": { "relay": { "command": "node", "args": ["__RELAY_CHECKOUT__/dist/mcp/main.js"] } } + "mcpServers": { "relay": { "command": "relay", "args": ["mcp"] } } } diff --git a/tests/fixtures/agent-integrations/valid/integrations/codex/config.toml.example b/tests/fixtures/agent-integrations/valid/integrations/codex/config.toml.example index 2bca1bd..02b047e 100644 --- a/tests/fixtures/agent-integrations/valid/integrations/codex/config.toml.example +++ b/tests/fixtures/agent-integrations/valid/integrations/codex/config.toml.example @@ -1,6 +1,3 @@ [mcp_servers.relay] -command = "node" -args = ["__RELAY_CHECKOUT__/dist/mcp/main.js"] - -[mcp_servers.relay.env] -RELAY_DB_PATH = "__RELAY_CHECKOUT__/.relay-validation/relay.db" +command = "relay" +args = ["mcp"] diff --git a/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/README.md b/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/README.md index 9f7a815..3c69289 100644 --- a/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/README.md +++ b/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/README.md @@ -1 +1 @@ -skills/relay-capture/SKILL.md skills/relay-session-review/SKILL.md Validation requires explicit isolated RELAY_DB_PATH; omission is permitted only for non-validation use. Remove only the client configuration; the SQLite database remains untouched. relay_health task_capture task_list task_get task_find_similar session_captures_list SQLite database remains untouched. +skills/relay-capture/SKILL.md skills/relay-session-review/SKILL.md Validation RELAY_DB_PATH must be explicit and isolated; omission is permitted only for non-validation use. Remove only the client configuration; the SQLite database remains untouched. relay_health task_capture task_list task_get task_find_similar session_captures_list SQLite database remains untouched. diff --git a/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/server-config.json.example b/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/server-config.json.example index dd781d0..ffffc2e 100644 --- a/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/server-config.json.example +++ b/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/server-config.json.example @@ -1,5 +1,4 @@ { - "command": "node", - "args": ["__RELAY_CHECKOUT__/dist/mcp/main.js"], - "env": { "RELAY_DB_PATH": "__RELAY_CHECKOUT__/.relay-validation/relay.db" } + "command": "relay", + "args": ["mcp"] } diff --git a/tests/fixtures/setup/claude-code/conflicting.json b/tests/fixtures/setup/claude-code/conflicting.json new file mode 100644 index 0000000..0295477 --- /dev/null +++ b/tests/fixtures/setup/claude-code/conflicting.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "relay": { + "command": "other-agent", + "args": ["serve"] + } + } +} diff --git a/tests/fixtures/setup/claude-code/crlf.json b/tests/fixtures/setup/claude-code/crlf.json new file mode 100644 index 0000000..da39e4f --- /dev/null +++ b/tests/fixtures/setup/claude-code/crlf.json @@ -0,0 +1,3 @@ +{ + "mcpServers": {} +} diff --git a/tests/fixtures/setup/claude-code/empty.json b/tests/fixtures/setup/claude-code/empty.json new file mode 100644 index 0000000..da39e4f --- /dev/null +++ b/tests/fixtures/setup/claude-code/empty.json @@ -0,0 +1,3 @@ +{ + "mcpServers": {} +} diff --git a/tests/fixtures/setup/claude-code/formatted.json b/tests/fixtures/setup/claude-code/formatted.json new file mode 100644 index 0000000..b99b6e1 --- /dev/null +++ b/tests/fixtures/setup/claude-code/formatted.json @@ -0,0 +1,6 @@ +{ + // Preserve comments and formatting. + "mcpServers": { + "other": { "command": "other-agent", "args": ["serve"] } + } +} diff --git a/tests/fixtures/setup/claude-code/malformed.json b/tests/fixtures/setup/claude-code/malformed.json new file mode 100644 index 0000000..fffb765 --- /dev/null +++ b/tests/fixtures/setup/claude-code/malformed.json @@ -0,0 +1,2 @@ +{ + "mcpServers": { diff --git a/tests/fixtures/setup/claude-code/matching.json b/tests/fixtures/setup/claude-code/matching.json new file mode 100644 index 0000000..4710f43 --- /dev/null +++ b/tests/fixtures/setup/claude-code/matching.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "relay": { + "command": "relay", + "args": ["mcp"] + } + } +} diff --git a/tests/fixtures/setup/claude-code/unrelated.json b/tests/fixtures/setup/claude-code/unrelated.json new file mode 100644 index 0000000..0e0d618 --- /dev/null +++ b/tests/fixtures/setup/claude-code/unrelated.json @@ -0,0 +1,13 @@ +{ + // Keep this comment and unrelated values. + "profile": { + "name": "local", + "secret_like": "fixture-secret-never-print" + }, + "mcpServers": { + "other": { + "command": "other-agent", + "args": ["serve"] + } + } +} diff --git a/tests/fixtures/setup/codex/conflicting.toml b/tests/fixtures/setup/codex/conflicting.toml new file mode 100644 index 0000000..3212e8c --- /dev/null +++ b/tests/fixtures/setup/codex/conflicting.toml @@ -0,0 +1,3 @@ +[mcp_servers.relay] +command = "other-agent" +args = ["serve"] diff --git a/tests/fixtures/setup/codex/crlf.toml b/tests/fixtures/setup/codex/crlf.toml new file mode 100644 index 0000000..5d8f102 --- /dev/null +++ b/tests/fixtures/setup/codex/crlf.toml @@ -0,0 +1,2 @@ +[profile] +name = "crlf" diff --git a/tests/fixtures/setup/codex/empty.toml b/tests/fixtures/setup/codex/empty.toml new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/fixtures/setup/codex/empty.toml @@ -0,0 +1 @@ + diff --git a/tests/fixtures/setup/codex/formatted.toml b/tests/fixtures/setup/codex/formatted.toml new file mode 100644 index 0000000..2acf1ca --- /dev/null +++ b/tests/fixtures/setup/codex/formatted.toml @@ -0,0 +1,4 @@ +# Preserve comments, spacing, and order. +[mcp_servers.other] +command = "other-agent" +args = [ "serve" ] diff --git a/tests/fixtures/setup/codex/malformed.toml b/tests/fixtures/setup/codex/malformed.toml new file mode 100644 index 0000000..9619f68 --- /dev/null +++ b/tests/fixtures/setup/codex/malformed.toml @@ -0,0 +1,2 @@ +[mcp_servers.relay +command = "relay" diff --git a/tests/fixtures/setup/codex/matching.toml b/tests/fixtures/setup/codex/matching.toml new file mode 100644 index 0000000..02b047e --- /dev/null +++ b/tests/fixtures/setup/codex/matching.toml @@ -0,0 +1,3 @@ +[mcp_servers.relay] +command = "relay" +args = ["mcp"] diff --git a/tests/fixtures/setup/codex/unrelated.toml b/tests/fixtures/setup/codex/unrelated.toml new file mode 100644 index 0000000..29607e5 --- /dev/null +++ b/tests/fixtures/setup/codex/unrelated.toml @@ -0,0 +1,8 @@ +# Keep this comment and unrelated values. +[profile] +name = "local" +secret_like = "fixture-secret-never-print" + +[mcp_servers.other] +command = "other-agent" +args = ["serve"] diff --git a/tests/fixtures/setup/metadata/disabled.json b/tests/fixtures/setup/metadata/disabled.json new file mode 100644 index 0000000..2bcfd10 --- /dev/null +++ b/tests/fixtures/setup/metadata/disabled.json @@ -0,0 +1,15 @@ +{ + "schemaVersion": 1, + "integrations": [ + { + "client": "claude-code", + "configPath": "C:/tmp/claude.json", + "entryId": "relay", + "command": "relay", + "args": ["mcp"], + "status": "disabled", + "applicationVersion": "0.1.0", + "lastSuccessfulSetupAt": "2026-08-02T00:00:00.000Z" + } + ] +} diff --git a/tests/fixtures/setup/metadata/duplicate.json b/tests/fixtures/setup/metadata/duplicate.json new file mode 100644 index 0000000..da9e7ae --- /dev/null +++ b/tests/fixtures/setup/metadata/duplicate.json @@ -0,0 +1,25 @@ +{ + "schemaVersion": 1, + "integrations": [ + { + "client": "codex", + "configPath": "C:/tmp/codex.toml", + "entryId": "relay", + "command": "relay", + "args": ["mcp"], + "status": "enabled", + "applicationVersion": "0.1.0", + "lastSuccessfulSetupAt": "2026-08-02T00:00:00.000Z" + }, + { + "client": "codex", + "configPath": "C:/tmp/./codex.toml", + "entryId": "relay", + "command": "relay", + "args": ["mcp"], + "status": "disabled", + "applicationVersion": "0.1.0", + "lastSuccessfulSetupAt": "2026-08-02T00:00:00.000Z" + } + ] +} diff --git a/tests/fixtures/setup/metadata/empty.json b/tests/fixtures/setup/metadata/empty.json new file mode 100644 index 0000000..ce58b79 --- /dev/null +++ b/tests/fixtures/setup/metadata/empty.json @@ -0,0 +1 @@ +{ "schemaVersion": 1, "integrations": [] } diff --git a/tests/fixtures/setup/metadata/enabled.json b/tests/fixtures/setup/metadata/enabled.json new file mode 100644 index 0000000..fa0307b --- /dev/null +++ b/tests/fixtures/setup/metadata/enabled.json @@ -0,0 +1,15 @@ +{ + "schemaVersion": 1, + "integrations": [ + { + "client": "codex", + "configPath": "C:/tmp/codex.toml", + "entryId": "relay", + "command": "relay", + "args": ["mcp"], + "status": "enabled", + "applicationVersion": "0.1.0", + "lastSuccessfulSetupAt": "2026-08-02T00:00:00.000Z" + } + ] +} diff --git a/tests/fixtures/setup/metadata/malformed.json b/tests/fixtures/setup/metadata/malformed.json new file mode 100644 index 0000000..40c3e59 --- /dev/null +++ b/tests/fixtures/setup/metadata/malformed.json @@ -0,0 +1 @@ +{"schemaVersion":1,"integrations": diff --git a/tests/fixtures/setup/metadata/unsupported-schema.json b/tests/fixtures/setup/metadata/unsupported-schema.json new file mode 100644 index 0000000..ce1bc8d --- /dev/null +++ b/tests/fixtures/setup/metadata/unsupported-schema.json @@ -0,0 +1 @@ +{ "schemaVersion": 2, "integrations": [] } diff --git a/tests/fixtures/setup/metadata/wrong-command.json b/tests/fixtures/setup/metadata/wrong-command.json new file mode 100644 index 0000000..9e688be --- /dev/null +++ b/tests/fixtures/setup/metadata/wrong-command.json @@ -0,0 +1,15 @@ +{ + "schemaVersion": 1, + "integrations": [ + { + "client": "codex", + "configPath": "C:/tmp/codex.toml", + "entryId": "relay", + "command": "other", + "args": ["serve"], + "status": "enabled", + "applicationVersion": "0.1.0", + "lastSuccessfulSetupAt": "2026-08-02T00:00:00.000Z" + } + ] +} diff --git a/tests/integration/setup-workflow.test.ts b/tests/integration/setup-workflow.test.ts new file mode 100644 index 0000000..c276ea8 --- /dev/null +++ b/tests/integration/setup-workflow.test.ts @@ -0,0 +1,96 @@ +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { spawnSync } from 'node:child_process'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; + +interface CliRun { + readonly status: number | null; + readonly stdout: string; + readonly stderr: string; +} + +describe('installed setup workflow', () => { + const roots: string[] = []; + afterEach(() => + roots.splice(0).forEach((root) => rmSync(root, { recursive: true, force: true })), + ); + + it('preserves unrelated Codex content and task data through idempotency and removal', () => { + const root = mkdtempSync(join(tmpdir(), 'relay-setup-workflow-')); + roots.push(root); + const databasePath = join(root, 'data', 'relay.db'); + const configPath = join(root, 'codex.toml'); + const before = '[profile]\nname = "workflow"\n'; + writeFileSync(configPath, before); + const environment = { + ...process.env, + RELAY_DB_PATH: databasePath, + APPDATA: join(root, 'appdata'), + LOCALAPPDATA: join(root, 'localappdata'), + }; + const run = (...args: readonly string[]): CliRun => { + const result = spawnSync( + process.execPath, + [join(process.cwd(), 'dist/cli/main.js'), ...args], + { cwd: root, env: environment, encoding: 'utf8' }, + ); + return { status: result.status, stdout: result.stdout ?? '', stderr: result.stderr ?? '' }; + }; + expect(run('setup').status).toBe(0); + const capture = run( + 'task', + 'capture', + '--title', + 'Before setup', + '--agent', + 'test', + '--session', + 'setup-session', + '--output', + 'json', + ); + expect(capture.status).toBe(0); + const taskId = (JSON.parse(capture.stdout) as { data?: { task?: { id?: string } } }).data?.task + ?.id; + expect(taskId).toBeTruthy(); + expect(run('setup', '--client', 'codex', '--config-file', configPath, '--apply').status).toBe( + 0, + ); + expect(readFileSync(configPath, 'utf8')).toContain('command = "relay"'); + const backup = run('setup', '--client', 'codex', '--config-file', configPath, '--apply'); + expect(backup.status).toBe(0); + expect(backup.stdout).toContain('"changed":false'); + expect( + run('config', 'disable', '--client', 'codex', '--config-file', configPath, '--apply').status, + ).toBe(0); + expect(run('setup', '--client', 'codex', '--config-file', configPath, '--apply').status).toBe( + 0, + ); + expect( + run('config', 'remove', '--client', 'codex', '--config-file', configPath, '--apply').status, + ).toBe(0); + expect(existsSync(databasePath)).toBe(true); + expect(run('task', 'get', taskId!, '--output', 'json').stdout).toContain(taskId!); + }); + + it('returns a JSON storage error when the database path is unusable', () => { + const root = mkdtempSync(join(tmpdir(), 'relay-setup-error-')); + roots.push(root); + const databasePath = join(root, 'database-dir'); + mkdirSync(databasePath); + const result = spawnSync(process.execPath, [join(process.cwd(), 'dist/cli/main.js'), 'setup'], { + cwd: root, + env: { + ...process.env, + RELAY_DB_PATH: databasePath, + APPDATA: join(root, 'appdata'), + LOCALAPPDATA: join(root, 'localappdata'), + }, + encoding: 'utf8', + }); + expect(result.status).toBe(5); + expect(JSON.parse(result.stdout) as { ok?: boolean }).toMatchObject({ ok: false }); + expect(result.stderr).toMatch(/database|path/i); + }); +}); diff --git a/tests/unit/distribution/setup/apply-integration-change.test.ts b/tests/unit/distribution/setup/apply-integration-change.test.ts new file mode 100644 index 0000000..5a7dc38 --- /dev/null +++ b/tests/unit/distribution/setup/apply-integration-change.test.ts @@ -0,0 +1,80 @@ +import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { createCodexTomlAdapter } from '../../../../src/distribution/setup/clients/codex-toml-adapter.js'; +import { applyIntegrationChange } from '../../../../src/distribution/setup/apply-integration-change.js'; +import { planIntegrationChange } from '../../../../src/distribution/setup/plan-integration-change.js'; +import { createOwnershipStore } from '../../../../src/distribution/setup/ownership-store.js'; + +describe('applyIntegrationChange', () => { + it('updates the client before writing enabled ownership metadata', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-apply-')); + const path = join(root, 'codex.toml'); + const metadataPath = join(root, 'config.json'); + writeFileSync(path, ''); + const adapter = createCodexTomlAdapter(); + const ownershipStore = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); + const plan = await planIntegrationChange({ + action: 'setup', + client: 'codex', + configPath: path, + adapter, + ownership: await ownershipStore.read(), + }); + const result = await applyIntegrationChange({ + plan, + adapter, + ownershipStore, + applicationVersion: '0.1.0', + now: new Date('2026-08-02T01:02:03.004Z'), + }); + expect(result.operation).toBe('created'); + expect(readFileSync(path, 'utf8')).toContain('command = "relay"'); + await expect(ownershipStore.read()).resolves.toMatchObject({ + integrations: [{ status: 'enabled', client: 'codex' }], + }); + }); + + it('removes stale disabled ownership without rewriting an already absent client file', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-apply-')); + const path = join(root, 'codex.toml'); + const metadataPath = join(root, 'config.json'); + writeFileSync(path, ''); + const adapter = createCodexTomlAdapter(); + const ownershipStore = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); + await ownershipStore.write({ + schemaVersion: 1, + integrations: [ + { + client: 'codex', + configPath: path, + entryId: 'relay', + command: 'relay', + args: ['mcp'], + status: 'disabled', + applicationVersion: '0.1.0', + lastSuccessfulSetupAt: '2026-08-02T01:02:03.004Z', + }, + ], + }); + const plan = await planIntegrationChange({ + action: 'remove', + client: 'codex', + configPath: path, + adapter, + ownership: await ownershipStore.read(), + }); + expect(plan.changed).toBe(true); + const result = await applyIntegrationChange({ + plan, + adapter, + ownershipStore, + applicationVersion: '0.1.0', + now: new Date('2026-08-02T01:02:03.004Z'), + }); + expect(result.operation).toBe('removed'); + expect(result.backupPath).toBeUndefined(); + await expect(ownershipStore.read()).resolves.toEqual({ schemaVersion: 1, integrations: [] }); + }); +}); diff --git a/tests/unit/distribution/setup/backup-and-atomic-write.test.ts b/tests/unit/distribution/setup/backup-and-atomic-write.test.ts new file mode 100644 index 0000000..1477734 --- /dev/null +++ b/tests/unit/distribution/setup/backup-and-atomic-write.test.ts @@ -0,0 +1,58 @@ +import { readFileSync, mkdtempSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { backupAndAtomicWrite } from '../../../../src/distribution/setup/backup-and-atomic-write.js'; +import { fingerprint } from '../../../../src/distribution/setup/plan-integration-change.js'; + +describe('backupAndAtomicWrite', () => { + it('backs up exact bytes and replaces the file with a validated result', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-write-')); + const path = join(root, 'config.json'); + const original = '{\n "other": true\n}\n'; + writeFileSync(path, original); + const result = await backupAndAtomicWrite({ + targetPath: path, + expectedFingerprint: fingerprint(original), + nextContent: '{\n "relay": true\n}\n', + validate: (content) => JSON.parse(content) as unknown, + now: new Date('2026-08-02T01:02:03.004Z'), + }); + expect(readFileSync(result.backupPath, 'utf8')).toBe(original); + expect(readFileSync(path, 'utf8')).toContain('relay'); + expect(result.backupPath).toContain('.relay-backup-20260802T010203.004Z'); + }); + + it('rejects a pre-replacement race without changing the target', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-write-')); + const path = join(root, 'config.json'); + writeFileSync(path, 'before'); + await expect( + backupAndAtomicWrite({ + targetPath: path, + expectedFingerprint: fingerprint('different'), + nextContent: 'after', + validate: () => undefined, + now: new Date('2026-08-02T01:02:03.004Z'), + }), + ).rejects.toThrow(/changed/i); + expect(readFileSync(path, 'utf8')).toBe('before'); + }); + + it('uses a collision-safe suffix when the timestamped backup already exists', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-write-')); + const path = join(root, 'config.json'); + const original = 'before'; + writeFileSync(path, original); + writeFileSync(`${path}.relay-backup-20260802T010203.004Z`, 'previous'); + const result = await backupAndAtomicWrite({ + targetPath: path, + expectedFingerprint: fingerprint(original), + nextContent: 'after', + validate: () => undefined, + now: new Date('2026-08-02T01:02:03.004Z'), + }); + expect(result.backupPath).toContain('.relay-backup-20260802T010203.004Z-1'); + expect(readFileSync(result.backupPath, 'utf8')).toBe(original); + }); +}); diff --git a/tests/unit/distribution/setup/claude-json-adapter.test.ts b/tests/unit/distribution/setup/claude-json-adapter.test.ts new file mode 100644 index 0000000..7f94e56 --- /dev/null +++ b/tests/unit/distribution/setup/claude-json-adapter.test.ts @@ -0,0 +1,30 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { createClaudeJsonAdapter } from '../../../../src/distribution/setup/clients/claude-json-adapter.js'; + +const fixture = (name: string) => + readFileSync(join(process.cwd(), 'tests/fixtures/setup/claude-code', name), 'utf8'); + +describe('Claude JSON adapter', () => { + const adapter = createClaudeJsonAdapter(); + it('inserts and removes only the Relay entry', () => { + const source = fixture('unrelated.json'); + const edited = adapter.upsertRelayEntry(source); + expect(adapter.inspect(edited).kind).toBe('matching'); + expect(edited).toContain('fixture-secret-never-print'); + expect(adapter.removeRelayEntry(edited)).toContain('other-agent'); + expect(adapter.inspect(adapter.removeRelayEntry(edited)).kind).toBe('absent'); + }); + it('fails closed for malformed and conflicting entries', () => { + expect(() => adapter.parse(fixture('malformed.json'))).toThrow(/malformed/i); + expect(adapter.inspect(fixture('conflicting.json')).kind).toBe('conflicting'); + }); + it('preserves matching bytes and CRLF line endings', () => { + const source = fixture('matching.json'); + expect(adapter.upsertRelayEntry(source)).toBe(source); + expect(adapter.upsertRelayEntry(fixture('crlf.json').replaceAll('\n', '\r\n'))).toContain( + '\r\n', + ); + }); +}); diff --git a/tests/unit/distribution/setup/codex-toml-adapter.test.ts b/tests/unit/distribution/setup/codex-toml-adapter.test.ts new file mode 100644 index 0000000..363159b --- /dev/null +++ b/tests/unit/distribution/setup/codex-toml-adapter.test.ts @@ -0,0 +1,31 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { createCodexTomlAdapter } from '../../../../src/distribution/setup/clients/codex-toml-adapter.js'; + +const fixture = (name: string) => + readFileSync(join(process.cwd(), 'tests/fixtures/setup/codex', name), 'utf8'); + +describe('Codex TOML adapter', () => { + const adapter = createCodexTomlAdapter(); + it('inserts and removes only the Relay table', () => { + const source = fixture('unrelated.toml'); + const edited = adapter.upsertRelayEntry(source); + expect(adapter.inspect(edited).kind).toBe('matching'); + expect(edited).toContain('fixture-secret-never-print'); + const removed = adapter.removeRelayEntry(edited); + expect(adapter.inspect(removed).kind).toBe('absent'); + expect(removed).toContain('other-agent'); + }); + it('fails closed for malformed and conflicting entries', () => { + expect(() => adapter.parse(fixture('malformed.toml'))).toThrow(/malformed/i); + expect(adapter.inspect(fixture('conflicting.toml')).kind).toBe('conflicting'); + }); + it('preserves matching bytes and CRLF line endings', () => { + const source = fixture('matching.toml'); + expect(adapter.upsertRelayEntry(source)).toBe(source); + expect(adapter.upsertRelayEntry(fixture('crlf.toml').replaceAll('\n', '\r\n'))).toContain( + '\r\n', + ); + }); +}); diff --git a/tests/unit/distribution/setup/initialize-relay.test.ts b/tests/unit/distribution/setup/initialize-relay.test.ts new file mode 100644 index 0000000..0620f91 --- /dev/null +++ b/tests/unit/distribution/setup/initialize-relay.test.ts @@ -0,0 +1,85 @@ +import { existsSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs'; +import type { mkdir as mkdirFunction } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { initializeRelay } from '../../../../src/distribution/setup/initialize-relay.js'; +import { SetupStorageError } from '../../../../src/distribution/setup/setup-errors.js'; + +describe('initializeRelay', () => { + const roots: string[] = []; + afterEach(() => + roots.splice(0).forEach((root) => rmSync(root, { recursive: true, force: true })), + ); + + it('creates Relay roots and opens the canonical runtime exactly once', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-init-')); + roots.push(root); + const dataRoot = join(root, 'data'); + const configRoot = join(root, 'config'); + const calls: string[] = []; + const result = await initializeRelay({ + runtimePaths: { + dataRoot, + configRoot, + cacheRoot: join(root, 'cache'), + databasePath: join(dataRoot, 'relay.db'), + }, + mkdir: (async (path, options) => { + const textPath = path.toString(); + mkdirSync(textPath, options); + calls.push(textPath); + return textPath; + }) as typeof mkdirFunction, + openRuntime: (databasePath) => { + calls.push(databasePath); + return { close: () => calls.push('closed') }; + }, + }); + + expect(result.createdDirectories).toEqual([dataRoot, configRoot]); + expect(calls).toEqual([dataRoot, configRoot, join(dataRoot, 'relay.db'), 'closed']); + }); + + it('closes the runtime when initialization returns', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-init-')); + roots.push(root); + let closed = 0; + await initializeRelay({ + runtimePaths: { + dataRoot: join(root, 'data'), + configRoot: join(root, 'config'), + cacheRoot: join(root, 'cache'), + databasePath: join(root, 'relay.db'), + }, + mkdir: async () => undefined, + openRuntime: () => ({ + close: () => { + closed += 1; + }, + }), + }); + expect(closed).toBe(1); + }); + + it('does not create metadata as a side effect of a runtime failure', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-init-')); + roots.push(root); + const metadataPath = join(root, 'config', 'config.json'); + const error = await initializeRelay({ + runtimePaths: { + dataRoot: join(root, 'data'), + configRoot: join(root, 'config'), + cacheRoot: join(root, 'cache'), + databasePath: join(root, 'relay.db'), + }, + mkdir: async () => undefined, + openRuntime: () => { + throw new Error('migration failed'); + }, + }).catch((caught: unknown) => caught); + expect(error).toBeInstanceOf(SetupStorageError); + expect((error as SetupStorageError).cause).toMatchObject({ message: 'migration failed' }); + expect(existsSync(metadataPath)).toBe(false); + }); +}); diff --git a/tests/unit/distribution/setup/ownership-store.test.ts b/tests/unit/distribution/setup/ownership-store.test.ts new file mode 100644 index 0000000..0d8c7ac --- /dev/null +++ b/tests/unit/distribution/setup/ownership-store.test.ts @@ -0,0 +1,58 @@ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { createOwnershipStore } from '../../../../src/distribution/setup/ownership-store.js'; + +describe('ownership store', () => { + const roots: string[] = []; + afterEach(() => + roots.splice(0).forEach((root) => rmSync(root, { recursive: true, force: true })), + ); + + it('reads missing metadata as an empty schema', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-ownership-')); + roots.push(root); + await expect( + createOwnershipStore({ + metadataPath: join(root, 'config.json'), + applicationVersion: '0.1.0', + }).read(), + ).resolves.toEqual({ schemaVersion: 1, integrations: [] }); + }); + + it('rejects malformed and unsafe ownership records', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-ownership-')); + roots.push(root); + const metadataPath = join(root, 'config.json'); + writeFileSync(metadataPath, JSON.stringify({ schemaVersion: 2, integrations: [] })); + await expect( + createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }).read(), + ).rejects.toThrow(/schema/i); + }); + + it('normalizes and sorts valid records on read and writes atomically', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-ownership-')); + roots.push(root); + const metadataPath = join(root, 'config.json'); + const store = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); + await store.write({ + schemaVersion: 1, + integrations: [ + { + client: 'codex', + configPath: join(root, 'nested', '..', 'codex.toml'), + entryId: 'relay', + command: 'relay', + args: ['mcp'], + status: 'enabled', + applicationVersion: '0.1.0', + lastSuccessfulSetupAt: '2026-08-02T00:00:00.000Z', + }, + ], + }); + expect(JSON.parse(readFileSync(metadataPath, 'utf8')).integrations[0].configPath).toBe( + join(root, 'codex.toml'), + ); + }); +}); diff --git a/tests/unit/distribution/setup/plan-integration-change.test.ts b/tests/unit/distribution/setup/plan-integration-change.test.ts new file mode 100644 index 0000000..c04ac28 --- /dev/null +++ b/tests/unit/distribution/setup/plan-integration-change.test.ts @@ -0,0 +1,62 @@ +import { readFileSync } from 'node:fs'; +import { mkdtempSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { createCodexTomlAdapter } from '../../../../src/distribution/setup/clients/codex-toml-adapter.js'; +import { planIntegrationChange } from '../../../../src/distribution/setup/plan-integration-change.js'; +import { + SetupConflictError, + SetupNotFoundError, +} from '../../../../src/distribution/setup/setup-errors.js'; + +describe('planIntegrationChange', () => { + it('plans an absent Relay entry as created without writing', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-plan-')); + const path = join(root, 'codex.toml'); + writeFileSync( + path, + readFileSync(join(process.cwd(), 'tests/fixtures/setup/codex/unrelated.toml')), + ); + const before = readFileSync(path, 'utf8'); + const plan = await planIntegrationChange({ + action: 'setup', + client: 'codex', + configPath: path, + adapter: createCodexTomlAdapter(), + ownership: { schemaVersion: 1, integrations: [] }, + }); + expect(plan.operation).toBe('created'); + expect(readFileSync(path, 'utf8')).toBe(before); + }); + + it('requires ownership even when the installed entry matches', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-plan-')); + const path = join(root, 'codex.toml'); + writeFileSync(path, '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n'); + await expect( + planIntegrationChange({ + action: 'setup', + client: 'codex', + configPath: path, + adapter: createCodexTomlAdapter(), + ownership: { schemaVersion: 1, integrations: [] }, + }), + ).rejects.toBeInstanceOf(SetupConflictError); + }); + + it('requires enabled ownership for disable', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-plan-')); + const path = join(root, 'codex.toml'); + writeFileSync(path, '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n'); + await expect( + planIntegrationChange({ + action: 'disable', + client: 'codex', + configPath: path, + adapter: createCodexTomlAdapter(), + ownership: { schemaVersion: 1, integrations: [] }, + }), + ).rejects.toBeInstanceOf(SetupNotFoundError); + }); +}); diff --git a/tests/unit/distribution/setup/setup-types.test.ts b/tests/unit/distribution/setup/setup-types.test.ts new file mode 100644 index 0000000..9b02c5d --- /dev/null +++ b/tests/unit/distribution/setup/setup-types.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from 'vitest'; +import type { + IntegrationChangePlan, + IntegrationClient, + RelayOwnershipFile, +} from '../../../../src/distribution/setup/setup-types.js'; +import type { ClientConfigAdapter } from '../../../../src/distribution/setup/clients/client-adapter.js'; + +describe('setup contracts', () => { + it('freezes the supported clients and installed Relay entry identity', () => { + const client: IntegrationClient = 'generic-mcp'; + const ownership: RelayOwnershipFile = { schemaVersion: 1, integrations: [] }; + const plan: IntegrationChangePlan = { + client: 'codex', + configPath: 'C:/tmp/codex.toml', + entryId: 'relay', + operation: 'created', + changed: true, + beforeFingerprint: 'digest', + nextContent: '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n', + snippet: '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n', + }; + const adapter: ClientConfigAdapter = { + client: 'claude-code', + parse: () => undefined, + inspect: () => ({ kind: 'absent' }), + upsertRelayEntry: (content) => content, + removeRelayEntry: (content) => content, + renderSnippet: () => '{}\n', + }; + + expect(client).toBe('generic-mcp'); + expect(ownership).toEqual({ schemaVersion: 1, integrations: [] }); + expect(plan.entryId).toBe('relay'); + expect(adapter.client).toBe('claude-code'); + }); +}); diff --git a/tests/unit/distribution/setup/snippets.test.ts b/tests/unit/distribution/setup/snippets.test.ts new file mode 100644 index 0000000..60005a4 --- /dev/null +++ b/tests/unit/distribution/setup/snippets.test.ts @@ -0,0 +1,24 @@ +import { parse as parseToml } from '@iarna/toml'; +import { describe, expect, it } from 'vitest'; +import { renderIntegrationSnippet } from '../../../../src/distribution/setup/snippets.js'; + +describe('renderIntegrationSnippet', () => { + it.each(['codex', 'claude-code', 'generic-mcp'] as const)( + 'renders the installed command for %s', + (client) => { + const snippet = renderIntegrationSnippet(client); + expect(snippet.endsWith('\n')).toBe(true); + expect(snippet).not.toMatch(/__RELAY_CHECKOUT__|RELAY_DB_PATH|node\s/); + if (client === 'codex') { + expect(parseToml(snippet)).toEqual({ + mcp_servers: { relay: { command: 'relay', args: ['mcp'] } }, + }); + } else { + const parsed = JSON.parse(snippet) as Record; + const server = + client === 'claude-code' ? (parsed.mcpServers as Record).relay : parsed; + expect(server).toEqual({ command: 'relay', args: ['mcp'] }); + } + }, + ); +}); diff --git a/tests/unit/interfaces/cli/operational-commands.test.ts b/tests/unit/interfaces/cli/operational-commands.test.ts new file mode 100644 index 0000000..80767b2 --- /dev/null +++ b/tests/unit/interfaces/cli/operational-commands.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from 'vitest'; +import { parseOperationalCommand } from '../../../../src/interfaces/cli/parse-operational-command.js'; +import { CliUsageError } from '../../../../src/interfaces/cli/output/cli-errors.js'; + +describe('parseOperationalCommand', () => { + it('parses setup preview and explicit apply', () => { + expect(parseOperationalCommand(['setup'])).toEqual({ kind: 'setup', apply: false }); + expect( + parseOperationalCommand([ + 'setup', + '--client', + 'codex', + '--config-file', + 'C:/tmp/codex.toml', + '--apply', + ]), + ).toEqual({ kind: 'setup', client: 'codex', configFile: 'C:/tmp/codex.toml', apply: true }); + }); + it('rejects unsafe or unsupported mutation grammar', () => { + for (const argv of [ + ['setup', '--client', 'codex', '--config-file', 'relative.toml'], + ['setup', '--client', 'generic-mcp', '--apply'], + ['setup', '--client', 'generic-mcp', '--config-file', 'C:/tmp/mcp.json'], + ['config', 'disable', '--client', 'codex', '--config-file', 'C:/tmp/codex.toml'], + [ + 'config', + 'remove', + '--client', + 'codex', + '--config-file', + 'C:/tmp/codex.toml', + '--apply', + '--apply', + ], + ]) + expect(() => parseOperationalCommand(argv)).toThrow(CliUsageError); + }); + it('parses snippet and inspection commands', () => { + expect(parseOperationalCommand(['config', 'paths'])).toEqual({ kind: 'config-paths' }); + expect(parseOperationalCommand(['config', 'integrations'])).toEqual({ + kind: 'config-integrations', + }); + expect(parseOperationalCommand(['config', 'snippet', '--client', 'generic-mcp'])).toEqual({ + kind: 'config-snippet', + client: 'generic-mcp', + }); + }); +}); diff --git a/tests/unit/interfaces/cli/run-operational-command.test.ts b/tests/unit/interfaces/cli/run-operational-command.test.ts new file mode 100644 index 0000000..41f11d4 --- /dev/null +++ b/tests/unit/interfaces/cli/run-operational-command.test.ts @@ -0,0 +1,65 @@ +import { existsSync, mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { runOperationalCommand } from '../../../../src/interfaces/cli/run-operational-command.js'; + +describe('runOperationalCommand', () => { + const roots: string[] = []; + afterEach(() => + roots.splice(0).forEach((root) => rmSync(root, { recursive: true, force: true })), + ); + + it('initializes before previewing a mutable client and does not write the client', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-operational-')); + roots.push(root); + const output: string[] = []; + const configPath = join(root, 'codex.toml'); + const code = await runOperationalCommand( + ['setup', '--client', 'codex', '--config-file', configPath], + { + runtimePaths: { + dataRoot: join(root, 'data'), + configRoot: join(root, 'config'), + cacheRoot: join(root, 'cache'), + databasePath: join(root, 'data', 'relay.db'), + }, + openRuntime: () => ({ close: () => undefined }), + applicationVersion: '0.1.0', + stdout: { + write: (text) => { + output.push(text); + }, + }, + stderr: { write: () => undefined }, + }, + ); + expect(code).toBe(0); + expect(existsSync(join(root, 'config', 'config.json'))).toBe(false); + expect(JSON.parse(output[0] ?? '{}').data.snippet).toContain('command = "relay"'); + }); + + it('reports effective paths without opening client configuration files', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-operational-')); + roots.push(root); + const output: string[] = []; + const code = await runOperationalCommand(['config', 'paths'], { + runtimePaths: { + dataRoot: join(root, 'data'), + configRoot: join(root, 'config'), + cacheRoot: join(root, 'cache'), + databasePath: join(root, 'data', 'relay.db'), + }, + openRuntime: () => ({ close: () => undefined }), + applicationVersion: '0.1.0', + stdout: { + write: (text) => { + output.push(text); + }, + }, + stderr: { write: () => undefined }, + }); + expect(code).toBe(0); + expect(JSON.parse(output[0] ?? '{}').data.paths.metadataPath).toContain('config.json'); + }); +}); diff --git a/tests/unit/scripts/validate-agent-integration-assets.test.ts b/tests/unit/scripts/validate-agent-integration-assets.test.ts index 618a318..5e11b44 100644 --- a/tests/unit/scripts/validate-agent-integration-assets.test.ts +++ b/tests/unit/scripts/validate-agent-integration-assets.test.ts @@ -106,18 +106,15 @@ describe('validateAgentIntegrationAssets', () => { expect(() => validateAgentIntegrationAssets({ rootDir })).toThrow(/task_archive/i); }); - it('rejects a Codex config that falls back to the default database', () => { + it('rejects a Codex config that does not use the installed command', () => { const rootDir = createRoot(); const path = join(rootDir, 'integrations/codex/config.toml.example'); writeFileSync( path, - readFileSync(path, 'utf8').replace( - 'RELAY_DB_PATH = "__RELAY_CHECKOUT__/.relay-validation/relay.db"', - 'RELAY_DB_PATH = "/default/relay.db"', - ), + readFileSync(path, 'utf8').replace('command = "relay"', 'command = "node"'), ); - expect(() => validateAgentIntegrationAssets({ rootDir })).toThrow(/isolated.*RELAY_DB_PATH/i); + expect(() => validateAgentIntegrationAssets({ rootDir })).toThrow(/installed relay mcp/i); }); it('rejects generic MCP guidance without explicit validation database isolation', () => { @@ -126,7 +123,7 @@ describe('validateAgentIntegrationAssets', () => { writeFileSync( path, readFileSync(path, 'utf8').replace( - 'Validation requires explicit isolated RELAY_DB_PATH; omission is permitted only for non-validation use.', + 'Validation RELAY_DB_PATH must be explicit and isolated; omission is permitted only for non-validation use.', 'The database is available.', ), ); @@ -220,12 +217,12 @@ describe('validateAgentIntegrationAssets', () => { expect(() => validateAgentIntegrationAssets({ rootDir })).toThrow(); }); - it('rejects an unqualified packaged relay mcp command', () => { + it('accepts the now-available packaged relay mcp command', () => { const rootDir = createRoot(); const path = join(rootDir, 'docs/agent-integration.md'); writeFileSync(path, `${readFileSync(path, 'utf8')} Use relay mcp now.`); - expect(() => validateAgentIntegrationAssets({ rootDir })).toThrow(/future-only/i); + expect(() => validateAgentIntegrationAssets({ rootDir })).not.toThrow(); }); it('rejects a canonical capture skill without autonomous-create permission', () => { @@ -319,12 +316,10 @@ describe('validateAgentIntegrationAssets', () => { const path = join(rootDir, 'integrations/generic-mcp/server-config.json.example'); writeFileSync( path, - readFileSync(path, 'utf8').replace('dist/mcp/main.js', 'dist/other-mcp.js'), + readFileSync(path, 'utf8').replace('"command": "relay"', '"command": "other"'), ); - expect(() => validateAgentIntegrationAssets({ rootDir })).toThrow( - /canonical.*dist\/mcp\/main\.js/i, - ); + expect(() => validateAgentIntegrationAssets({ rootDir })).toThrow(/installed relay mcp/i); }); it('rejects removal guidance that deletes the SQLite database', () => { diff --git a/tsup.config.ts b/tsup.config.ts index 194e2ca..916d9c7 100644 --- a/tsup.config.ts +++ b/tsup.config.ts @@ -13,5 +13,6 @@ export default defineConfig({ sourcemap: false, splitting: false, bundle: true, + external: ['@iarna/toml', 'jsonc-parser'], shims: true, }); From ce18aa9166b8a0ece331f71289631686b36b403b Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 11:59:20 +0530 Subject: [PATCH 02/11] docs: add PR 48 review remediation plan --- .../2026-08-02-pr-48-review-remediation.md | 609 ++++++++++++++++++ 1 file changed, 609 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-02-pr-48-review-remediation.md diff --git a/docs/superpowers/plans/2026-08-02-pr-48-review-remediation.md b/docs/superpowers/plans/2026-08-02-pr-48-review-remediation.md new file mode 100644 index 0000000..05b2042 --- /dev/null +++ b/docs/superpowers/plans/2026-08-02-pr-48-review-remediation.md @@ -0,0 +1,609 @@ +# PR #48 Review Remediation Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Correct PR #48 so concurrent setup operations cannot lose ownership records, rollback restores a previously absent client configuration to absence, and the operational command tests pass on every supported host platform. + +**Architecture:** Keep the existing setup adapters, client-file fingerprint checks, backup pipeline, ownership JSON format, CLI contract, and public commands unchanged. Add one process-safe ownership-metadata mutation boundary that serializes the read-modify-write sequence, carry original-file existence through the backup result so rollback can restore the exact pre-operation state, and make CLI parser tests generate host-native absolute paths instead of embedding Windows-only paths. + +**Tech Stack:** Node.js 24 (`>=24 <25`), TypeScript/ESM, Node filesystem primitives, Vitest, existing Relay setup/configuration modules. + +## Global Constraints + +- Do not change the public `relay setup` or `relay config` command grammar. +- Do not change ownership metadata schema version `1` or add speculative metadata fields. +- Do not move ownership state into SQLite; `config.json` remains authoritative. +- Do not weaken client-file fingerprint checks, backup-before-mutation, parse validation, atomic replacement, or task-data retention. +- Concurrent operations must either serialize safely or fail with an actionable conflict; they must never silently overwrite another integration record. +- The ownership lock must be package-independent and use only Node.js filesystem primitives; do not add a locking dependency. +- Lock acquisition must not scan the filesystem or infer other client paths. +- A failed metadata write after creating a previously absent client file must remove that new file during rollback. +- A failed metadata write after modifying an existing client file must restore the exact backup and preserve its mode. +- Backups remain retained after failure. +- Tests must use temporary directories and must not touch real user configuration or default Relay paths. +- `pnpm verify` and `RELAY_RUN_PACKAGE_SMOKE=1 pnpm verify:package` must pass before the PR is ready. + +--- + +## Locked Remediation Design + +### Ownership mutation serialization + +Add a lock file beside Relay ownership metadata: + +```text +/config.json.relay-lock +``` + +The lock is acquired with `open(lockPath, 'wx', 0o600)`. `wx` is the cross-process exclusion primitive: exactly one process may create the lock file. + +Use these exact constants: + +```ts +const OWNERSHIP_LOCK_RETRY_DELAY_MS = 25; +const OWNERSHIP_LOCK_MAX_ATTEMPTS = 40; +``` + +This gives other short-lived Relay operations up to approximately one second to finish. On every `EEXIST`, wait 25 ms and retry. After 40 unsuccessful attempts, throw `SetupConflictError` with an actionable message that another Relay configuration operation is in progress and the user should retry. Do not automatically delete or break an existing lock because Relay cannot prove that its owner is dead. + +The process that acquires the lock must close and unlink it in `finally`. The lock must cover the complete ownership read-modify-write operation, not merely the final rename. + +Expose one mutation API so callers cannot accidentally reintroduce an unlocked read/write pair: + +```ts +export interface OwnershipStore { + read(): Promise; + update( + mutate: (current: RelayOwnershipFile) => RelayOwnershipFile, + ): Promise; +} +``` + +`update()` must: + +1. acquire the lock; +2. read and validate the latest metadata while holding the lock; +3. call `mutate(current)` exactly once; +4. validate and atomically write the returned value; +5. return the persisted value; +6. release the lock in `finally`. + +Remove the public `write()` method after all tests and callers migrate. Tests may use `update(() => fixture)` to seed metadata. + +### Exact rollback state + +Change the backup result to record whether the target existed before mutation: + +```ts +export interface BackupAndAtomicWriteResult { + readonly backupPath: string; + readonly originalExisted: boolean; + readonly originalMode: number; +} +``` + +Add one restoration function: + +```ts +export async function restoreOriginalFile(input: { + readonly backupPath: string; + readonly targetPath: string; + readonly originalExisted: boolean; + readonly originalMode: number; +}): Promise; +``` + +Behavior: + +- `originalExisted: true`: restore backup contents atomically and apply `originalMode`. +- `originalExisted: false`: unlink the newly created target; treat `ENOENT` as success. +- Never delete the backup. + +Use this function both for post-replacement validation failure inside `backupAndAtomicWrite()` and metadata-persistence failure inside `applyIntegrationChange()`. + +--- + +### Task 1: Make operational parser tests host-independent and restore green CI + +**Files:** + +- Modify: `tests/unit/interfaces/cli/operational-commands.test.ts` + +**Interfaces:** + +- Consumes: existing `parseOperationalCommand(argv)` behavior. +- Produces: the same assertions using an absolute path valid on the current test host. + +- [ ] **Step 1: Replace hard-coded Windows paths with a host-native absolute fixture path.** + +At the top of the test file import `resolve`: + +```ts +import { resolve } from 'node:path'; +``` + +Inside the suite define: + +```ts +const absoluteConfigPath = resolve('tmp', 'codex.toml'); +``` + +Use `absoluteConfigPath` in every test case intended to represent a valid absolute path. Keep `relative.toml` for the negative relative-path case. + +The successful assertion must be: + +```ts +expect( + parseOperationalCommand([ + 'setup', + '--client', + 'codex', + '--config-file', + absoluteConfigPath, + '--apply', + ]), +).toEqual({ + kind: 'setup', + client: 'codex', + configFile: absoluteConfigPath, + apply: true, +}); +``` + +- [ ] **Step 2: Run the previously failing test.** + +```bash +pnpm test -- tests/unit/interfaces/cli/operational-commands.test.ts +``` + +Expected: all three tests pass on the current host. + +- [ ] **Step 3: Commit the portable test fix.** + +```bash +git add tests/unit/interfaces/cli/operational-commands.test.ts +git commit -m "test: use host-native setup config paths" +``` + +--- + +### Task 2: Serialize ownership metadata read-modify-write operations + +**Files:** + +- Modify: `src/distribution/setup/ownership-store.ts` +- Modify: `src/distribution/setup/apply-integration-change.ts` +- Modify: `tests/unit/distribution/setup/ownership-store.test.ts` +- Modify: `tests/unit/distribution/setup/apply-integration-change.test.ts` +- Modify: any test currently calling `ownershipStore.write(...)` + +**Interfaces:** + +- Produces: + +```ts +export interface OwnershipStore { + read(): Promise; + update( + mutate: (current: RelayOwnershipFile) => RelayOwnershipFile, + ): Promise; +} +``` + +- Internal helper: + +```ts +async function withOwnershipLock( + metadataPath: string, + action: () => Promise, +): Promise; +``` + +- [ ] **Step 1: Write a failing concurrent-update test.** + +Add a test to `ownership-store.test.ts` using two store instances pointing to the same temporary `config.json`: + +```ts +it('preserves both records when independent stores update concurrently', async () => { + const first = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); + const second = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); + + await Promise.all([ + first.update((current) => ({ + schemaVersion: 1, + integrations: [...current.integrations, codexRecord], + })), + second.update((current) => ({ + schemaVersion: 1, + integrations: [...current.integrations, claudeRecord], + })), + ]); + + await expect(first.read()).resolves.toMatchObject({ + integrations: [ + expect.objectContaining({ client: 'claude-code' }), + expect.objectContaining({ client: 'codex' }), + ], + }); +}); +``` + +Use different absolute config paths for the two records. Do not mock the lock; this test must exercise real cross-instance filesystem exclusion. + +- [ ] **Step 2: Write a failing lock-timeout test.** + +Create `.relay-lock` before calling `update()`. Assert rejection is `SetupConflictError` and the message includes both `in progress` and `retry`. Remove the fixture lock in `finally`. + +Use fake timers only if necessary. Prefer injecting these optional internal timing dependencies into `createOwnershipStore`: + +```ts +readonly lockRetryDelayMs?: number; +readonly lockMaxAttempts?: number; +readonly sleep?: (milliseconds: number) => Promise; +``` + +Production defaults must remain 25 ms and 40 attempts. The timeout test may use `lockRetryDelayMs: 0`, `lockMaxAttempts: 2`, and `sleep: async () => undefined`. + +- [ ] **Step 3: Run ownership tests and confirm the new tests fail.** + +```bash +pnpm test -- tests/unit/distribution/setup/ownership-store.test.ts +``` + +Expected: FAIL because `update()` and locking do not exist. + +- [ ] **Step 4: Implement `withOwnershipLock()`.** + +Use `open(lockPath, 'wx', 0o600)`. After acquisition, write a minimal diagnostic payload containing only PID and acquisition timestamp: + +```ts +await handle.writeFile( + `${JSON.stringify({ pid: process.pid, acquiredAt: new Date().toISOString() })}\n`, + 'utf8', +); +await handle.sync(); +``` + +Do not print the payload. In `finally`, close the handle and unlink the lock path. If lock cleanup fails, wrap it as `SetupStorageError`; do not report success while the lock remains unexpectedly. + +On `EEXIST`, retry. After the maximum attempts, throw: + +```ts +new SetupConflictError( + `Another Relay configuration operation is in progress for ${metadataPath}. Retry after it completes.`, +); +``` + +Other open/write/close/unlink errors map to `SetupStorageError` and name only the metadata or lock path. + +- [ ] **Step 5: Implement `OwnershipStore.update()` and make raw writing private.** + +Refactor existing write logic into a private `writeValidatedOwnership(next)` closure. `update()` executes this exact sequence while holding the lock: + +```ts +return withOwnershipLock(input.metadataPath, async () => { + const current = await readOwnership(); + const next = mutate(current); + await writeValidatedOwnership(next); + return validateOwnership(next, input.applicationVersion); +}); +``` + +Keep `read()` lock-free because inspection is read-only and atomic replacement prevents partial JSON reads. + +Remove `write()` from the exported interface. Do not expose the lock helper. + +- [ ] **Step 6: Update `applyIntegrationChange()` to merge against the latest metadata under lock.** + +Replace this unsafe sequence: + +```ts +const ownership = await input.ownershipStore.read(); +// calculate existing +await input.ownershipStore.write(next); +``` + +with: + +```ts +await input.ownershipStore.update((ownership) => { + const existing = ownership.integrations.filter( + (record) => + !( + record.client === input.plan.client && + sameOwnedPath(record.configPath, input.plan.configPath) + ), + ); + return { + schemaVersion: 1, + integrations: + input.plan.operation === 'removed' + ? existing + : [...existing, nextRecord], + }; +}); +``` + +Add or reuse one path-comparison helper with native Windows case-insensitive behavior. Do not compare ownership paths using raw string equality. + +- [ ] **Step 7: Migrate test setup from `write()` to `update()`.** + +For fixture seeding use: + +```ts +await ownershipStore.update(() => fixtureOwnership); +``` + +Do not add a test-only public write bypass. + +- [ ] **Step 8: Add an `applyIntegrationChange` concurrency regression test.** + +Create two client files and two plans from the same initial empty ownership snapshot. Run the two `applyIntegrationChange()` calls concurrently with separate adapters and the same metadata path. Assert: + +- both client files contain their Relay entry; +- final ownership metadata contains both records; +- neither operation reports success with a missing ownership record. + +Use Codex for one operation and Claude Code for the other so the regression matches the real failure mode. + +- [ ] **Step 9: Run focused setup tests.** + +```bash +pnpm test -- \ + tests/unit/distribution/setup/ownership-store.test.ts \ + tests/unit/distribution/setup/apply-integration-change.test.ts \ + tests/integration/setup-workflow.test.ts +pnpm typecheck +``` + +Expected: all pass. + +- [ ] **Step 10: Commit ownership serialization.** + +```bash +git add \ + src/distribution/setup/ownership-store.ts \ + src/distribution/setup/apply-integration-change.ts \ + tests/unit/distribution/setup \ + tests/integration/setup-workflow.test.ts +git commit -m "fix: serialize Relay ownership metadata updates" +``` + +--- + +### Task 3: Restore exact pre-write absence after metadata failure + +**Files:** + +- Modify: `src/distribution/setup/backup-and-atomic-write.ts` +- Modify: `src/distribution/setup/apply-integration-change.ts` +- Modify: `tests/unit/distribution/setup/backup-and-atomic-write.test.ts` +- Modify: `tests/unit/distribution/setup/apply-integration-change.test.ts` + +**Interfaces:** + +- Produces: + +```ts +export interface BackupAndAtomicWriteResult { + readonly backupPath: string; + readonly originalExisted: boolean; + readonly originalMode: number; +} + +export async function restoreOriginalFile(input: { + readonly backupPath: string; + readonly targetPath: string; + readonly originalExisted: boolean; + readonly originalMode: number; +}): Promise; +``` + +- [ ] **Step 1: Write a failing low-level rollback test for a missing original file.** + +In `backup-and-atomic-write.test.ts`: + +1. choose a target path that does not exist; +2. call `backupAndAtomicWrite()` with the empty-content fingerprint and valid next content; +3. assert the target now exists; +4. call `restoreOriginalFile(result)`; +5. assert the target does not exist; +6. assert the backup still exists and contains zero bytes. + +Use `existsSync()` only for assertions; production remains async. + +- [ ] **Step 2: Write a failing application-level metadata failure test.** + +In `apply-integration-change.test.ts`, create a plan for a nonexistent Codex config. Supply an `OwnershipStore` whose `read()` returns empty ownership and whose `update()` throws `SetupStorageError('forced metadata failure')`. + +After rejection, assert: + +```ts +expect(existsSync(configPath)).toBe(false); +expect(readdirSync(root).some((name) => name.includes('.relay-backup-'))).toBe(true); +``` + +Also assert the thrown error states that client configuration was restored after metadata persistence failed. + +- [ ] **Step 3: Run the two focused files and confirm failure.** + +```bash +pnpm test -- \ + tests/unit/distribution/setup/backup-and-atomic-write.test.ts \ + tests/unit/distribution/setup/apply-integration-change.test.ts +``` + +Expected: FAIL because current rollback restores an empty target file. + +- [ ] **Step 4: Record exact original state in `backupAndAtomicWrite()`.** + +Read the target with a helper returning: + +```ts +interface OriginalFileState { + readonly existed: boolean; + readonly contents: Buffer; + readonly mode: number; +} +``` + +For `ENOENT`, return `{ existed: false, contents: Buffer.alloc(0), mode: 0o600 }`. For an existing file, read bytes and stat mode. + +Return: + +```ts +{ + backupPath, + originalExisted: original.existed, + originalMode: original.mode, +} +``` + +- [ ] **Step 5: Implement `restoreOriginalFile()`.** + +For an absent original: + +```ts +if (!input.originalExisted) { + await unlink(input.targetPath).catch((error: unknown) => { + if (!isMissing(error)) throw storageError(input.targetPath, error); + }); + return; +} +``` + +For an existing original, delegate to the existing atomic restore implementation using `originalMode`. + +Do not unlink or rewrite the backup. + +- [ ] **Step 6: Use `restoreOriginalFile()` in both rollback locations.** + +Inside `backupAndAtomicWrite()` after a post-replacement failure, call `restoreOriginalFile()` with the returned original state. + +Inside `applyIntegrationChange()` after ownership update failure, call `restoreOriginalFile()` using the backup result. Parse the restored target only when `originalExisted` is true. When it was false, verify absence with `stat()` and accept only `ENOENT`. + +- [ ] **Step 7: Retain existing-file rollback coverage.** + +Add or preserve an assertion that an existing file is restored byte-for-byte and keeps its original mode after forced metadata failure. This prevents the new absence branch from weakening the existing branch. + +- [ ] **Step 8: Run focused tests.** + +```bash +pnpm test -- \ + tests/unit/distribution/setup/backup-and-atomic-write.test.ts \ + tests/unit/distribution/setup/apply-integration-change.test.ts \ + tests/integration/setup-workflow.test.ts +pnpm typecheck +``` + +Expected: all pass. + +- [ ] **Step 9: Commit exact-state rollback.** + +```bash +git add \ + src/distribution/setup/backup-and-atomic-write.ts \ + src/distribution/setup/apply-integration-change.ts \ + tests/unit/distribution/setup/backup-and-atomic-write.test.ts \ + tests/unit/distribution/setup/apply-integration-change.test.ts +git commit -m "fix: restore absent client configs after setup failure" +``` + +--- + +### Task 4: Run full regression, package smoke, and update PR evidence + +**Files:** + +- Modify only if results require it: PR #48 description +- Do not modify issue #41 acceptance criteria or the accepted ADR. + +- [ ] **Step 1: Run the complete repository gate from a clean generated-output state.** + +```bash +rm -rf dist coverage .relay-package +pnpm install --frozen-lockfile +pnpm verify +``` + +On Windows PowerShell use the repository's existing Windows-safe cleanup equivalent rather than Unix `rm`. + +Expected: + +- formatting passes; +- lint passes with zero warnings; +- typecheck passes; +- every test passes; +- coverage thresholds pass; +- build passes; +- metadata and asset validation pass; +- high-severity audit gate passes. + +- [ ] **Step 2: Run installed-package smoke with setup/configuration enabled.** + +```bash +RELAY_RUN_PACKAGE_SMOKE=1 pnpm verify:package +``` + +On Windows PowerShell: + +```powershell +$env:RELAY_RUN_PACKAGE_SMOKE = '1' +pnpm verify:package +``` + +Expected: installed tarball setup, preview, apply, idempotency, disable, re-enable, remove, MCP startup, UI startup, and task-data retention all pass from an unrelated working directory. + +- [ ] **Step 3: Add one manual concurrency smoke check.** + +Using two disposable config files and one disposable Relay config root, start Codex and Claude setup applies at approximately the same time. After both commands complete, run: + +```bash +relay config integrations --output json +``` + +Confirm both records are present. If one operation reports an in-progress conflict, retry it and confirm both records are present afterward. Do not use real client files for this check. + +- [ ] **Step 4: Update the PR description verification section.** + +Record exact final counts and commands. Include: + +- the previous Linux path-test failure is fixed; +- concurrent ownership updates are serialized and regression-tested; +- rollback of a newly created config restores absence; +- `pnpm verify` result; +- package smoke result; +- manual real Codex/Claude restart validation remains the human acceptance gate. + +Do not claim CodeRabbit review coverage because it skipped this draft PR. + +- [ ] **Step 5: Push all remediation commits and wait for GitHub CI.** + +The PR remains draft until GitHub Actions is green. Do not merge based only on local Windows verification. + +- [ ] **Step 6: Human review checkpoint.** + +Before marking ready, inspect: + +1. `ownership-store.ts` to ensure every mutation is inside `update()` and lock release is in `finally`; +2. `apply-integration-change.ts` to ensure it never performs unlocked read-then-write metadata mutation; +3. rollback tests proving absence and existing-file restoration; +4. CI logs proving the Linux parser test passes; +5. final `config.json` after concurrent disposable setup proving both records survive. + +--- + +## Completion Criteria + +The remediation is complete only when all are true: + +- Concurrent Codex and Claude setup cannot silently lose either ownership record. +- Lock contention returns exit code `4` through the existing `SetupConflictError` mapping. +- No public raw ownership `write()` method remains. +- A metadata failure after creating a new client config removes that new file. +- A metadata failure after changing an existing config restores exact contents and mode. +- Backups remain retained in both rollback paths. +- Operational parser tests use host-native absolute paths. +- GitHub Actions passes on Ubuntu. +- `pnpm verify` passes. +- `RELAY_RUN_PACKAGE_SMOKE=1 pnpm verify:package` passes. From 278a5b727b4ab1cb00a26a8bfc0be46ff059d432 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 12:14:19 +0530 Subject: [PATCH 03/11] test: use host-native setup config paths --- .../interfaces/cli/operational-commands.test.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tests/unit/interfaces/cli/operational-commands.test.ts b/tests/unit/interfaces/cli/operational-commands.test.ts index 80767b2..cf194af 100644 --- a/tests/unit/interfaces/cli/operational-commands.test.ts +++ b/tests/unit/interfaces/cli/operational-commands.test.ts @@ -1,8 +1,11 @@ +import { resolve } from 'node:path'; import { describe, expect, it } from 'vitest'; import { parseOperationalCommand } from '../../../../src/interfaces/cli/parse-operational-command.js'; import { CliUsageError } from '../../../../src/interfaces/cli/output/cli-errors.js'; describe('parseOperationalCommand', () => { + const absoluteConfigPath = resolve('tmp', 'codex.toml'); + it('parses setup preview and explicit apply', () => { expect(parseOperationalCommand(['setup'])).toEqual({ kind: 'setup', apply: false }); expect( @@ -11,24 +14,24 @@ describe('parseOperationalCommand', () => { '--client', 'codex', '--config-file', - 'C:/tmp/codex.toml', + absoluteConfigPath, '--apply', ]), - ).toEqual({ kind: 'setup', client: 'codex', configFile: 'C:/tmp/codex.toml', apply: true }); + ).toEqual({ kind: 'setup', client: 'codex', configFile: absoluteConfigPath, apply: true }); }); it('rejects unsafe or unsupported mutation grammar', () => { for (const argv of [ ['setup', '--client', 'codex', '--config-file', 'relative.toml'], ['setup', '--client', 'generic-mcp', '--apply'], - ['setup', '--client', 'generic-mcp', '--config-file', 'C:/tmp/mcp.json'], - ['config', 'disable', '--client', 'codex', '--config-file', 'C:/tmp/codex.toml'], + ['setup', '--client', 'generic-mcp', '--config-file', absoluteConfigPath], + ['config', 'disable', '--client', 'codex', '--config-file', absoluteConfigPath], [ 'config', 'remove', '--client', 'codex', '--config-file', - 'C:/tmp/codex.toml', + absoluteConfigPath, '--apply', '--apply', ], From 58a5f52363f2c38e2d69291fb9d5c5edd501e117 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 12:19:44 +0530 Subject: [PATCH 04/11] fix: serialize Relay ownership metadata updates --- .../setup/apply-integration-change.ts | 29 ++- src/distribution/setup/ownership-store.ts | 207 ++++++++++++++---- .../setup/apply-integration-change.test.ts | 59 ++++- .../setup/ownership-store.test.ts | 68 +++++- 4 files changed, 306 insertions(+), 57 deletions(-) diff --git a/src/distribution/setup/apply-integration-change.ts b/src/distribution/setup/apply-integration-change.ts index ebd9516..3f18920 100644 --- a/src/distribution/setup/apply-integration-change.ts +++ b/src/distribution/setup/apply-integration-change.ts @@ -1,6 +1,7 @@ import type { ClientConfigAdapter } from './clients/client-adapter.js'; import { backupAndAtomicWrite, restoreFile } from './backup-and-atomic-write.js'; import { readFile } from 'node:fs/promises'; +import { normalize, resolve } from 'node:path'; import { fingerprint } from './plan-integration-change.js'; import { SetupStorageError } from './setup-errors.js'; import type { IntegrationChangePlan, IntegrationChangeResult } from './setup-types.js'; @@ -33,11 +34,6 @@ export async function applyIntegrationChange(input: { }) : undefined; try { - const ownership = await input.ownershipStore.read(); - const existing = ownership.integrations.filter( - (record) => - !(record.client === input.plan.client && record.configPath === input.plan.configPath), - ); const nextRecord = { client: input.plan.client, configPath: input.plan.configPath, @@ -49,9 +45,18 @@ export async function applyIntegrationChange(input: { lastSuccessfulSetupAt: input.now.toISOString(), ...(backup === undefined ? {} : { lastBackupPath: backup.backupPath }), }; - await input.ownershipStore.write({ - schemaVersion: 1, - integrations: input.plan.operation === 'removed' ? existing : [...existing, nextRecord], + await input.ownershipStore.update((ownership) => { + const existing = ownership.integrations.filter( + (record) => + !( + record.client === input.plan.client && + sameOwnedPath(record.configPath, input.plan.configPath) + ), + ); + return { + schemaVersion: 1, + integrations: input.plan.operation === 'removed' ? existing : [...existing, nextRecord], + }; }); } catch (error) { try { @@ -79,3 +84,11 @@ export async function applyIntegrationChange(input: { ...(backup === undefined ? {} : { backupPath: backup.backupPath }), }; } + +function sameOwnedPath(left: string, right: string): boolean { + const normalizedLeft = normalize(resolve(left)); + const normalizedRight = normalize(resolve(right)); + return process.platform === 'win32' + ? normalizedLeft.toLowerCase() === normalizedRight.toLowerCase() + : normalizedLeft === normalizedRight; +} diff --git a/src/distribution/setup/ownership-store.ts b/src/distribution/setup/ownership-store.ts index e472916..888e24a 100644 --- a/src/distribution/setup/ownership-store.ts +++ b/src/distribution/setup/ownership-store.ts @@ -1,66 +1,181 @@ import { randomUUID } from 'node:crypto'; -import { mkdir, readFile, unlink, writeFile } from 'node:fs/promises'; +import { mkdir, open, readFile, unlink, writeFile, type FileHandle } from 'node:fs/promises'; import { dirname, normalize, resolve } from 'node:path'; import { replaceFile } from './backup-and-atomic-write.js'; import type { RelayIntegrationOwnership, RelayOwnershipFile } from './setup-types.js'; -import { SetupStorageError } from './setup-errors.js'; +import { SetupConflictError, SetupStorageError } from './setup-errors.js'; + +const OWNERSHIP_LOCK_RETRY_DELAY_MS = 25; +const OWNERSHIP_LOCK_MAX_ATTEMPTS = 40; export interface OwnershipStore { read(): Promise; - write(next: RelayOwnershipFile): Promise; + update(mutate: (current: RelayOwnershipFile) => RelayOwnershipFile): Promise; } export function createOwnershipStore(input: { readonly metadataPath: string; readonly applicationVersion: string; + readonly lockRetryDelayMs?: number; + readonly lockMaxAttempts?: number; + readonly sleep?: (milliseconds: number) => Promise; }): OwnershipStore { + const readOwnership = async (): Promise => { + let source: string; + try { + source = await readFile(input.metadataPath, 'utf8'); + } catch (error) { + if (isMissingFile(error)) return { schemaVersion: 1, integrations: [] }; + throw new SetupStorageError( + `Relay ownership metadata could not be read at ${input.metadataPath}.`, + error, + ); + } + try { + return validateOwnership(JSON.parse(source), input.applicationVersion); + } catch (error) { + throw new SetupStorageError( + `Relay ownership metadata schema is invalid at ${input.metadataPath}.`, + { + cause: error, + }, + ); + } + }; + + const writeValidatedOwnership = async (next: RelayOwnershipFile): Promise => { + try { + await mkdir(dirname(input.metadataPath), { recursive: true }); + } catch (error) { + throw new SetupStorageError( + `Relay ownership metadata could not be prepared at ${input.metadataPath}.`, + error, + ); + } + const temporaryPath = `${input.metadataPath}.${process.pid}.${randomUUID()}.tmp`; + try { + await writeFile(temporaryPath, `${JSON.stringify(next, null, 2)}\n`, 'utf8'); + await replaceFile(temporaryPath, input.metadataPath); + } catch (error) { + await unlink(temporaryPath).catch(() => undefined); + throw new SetupStorageError( + `Relay ownership metadata could not be written at ${input.metadataPath}.`, + error, + ); + } + }; + return { - read: async () => { - let source: string; - try { - source = await readFile(input.metadataPath, 'utf8'); - } catch (error) { - if (isMissingFile(error)) return { schemaVersion: 1, integrations: [] }; - throw new SetupStorageError( - `Relay ownership metadata could not be read at ${input.metadataPath}.`, - error, - ); - } - try { - return validateOwnership(JSON.parse(source), input.applicationVersion); - } catch (error) { - throw new SetupStorageError( - `Relay ownership metadata schema is invalid at ${input.metadataPath}.`, - { - cause: error, - }, - ); - } + read: readOwnership, + update: async (mutate) => { + return withOwnershipLock( + input.metadataPath, + async () => { + const current = await readOwnership(); + const next = validateOwnership(mutate(current), input.applicationVersion); + await writeValidatedOwnership(next); + return next; + }, + { + ...(input.lockRetryDelayMs === undefined + ? {} + : { lockRetryDelayMs: input.lockRetryDelayMs }), + ...(input.lockMaxAttempts === undefined + ? {} + : { lockMaxAttempts: input.lockMaxAttempts }), + ...(input.sleep === undefined ? {} : { sleep: input.sleep }), + }, + ); }, - write: async (next) => { - let validated: RelayOwnershipFile; - try { - validated = validateOwnership(next, input.applicationVersion); - await mkdir(dirname(input.metadataPath), { recursive: true }); - } catch (error) { + }; +} + +async function withOwnershipLock( + metadataPath: string, + action: () => Promise, + options: { + readonly lockRetryDelayMs?: number; + readonly lockMaxAttempts?: number; + readonly sleep?: (milliseconds: number) => Promise; + } = {}, +): Promise { + const lockPath = `${metadataPath}.relay-lock`; + const retryDelayMs = options.lockRetryDelayMs ?? OWNERSHIP_LOCK_RETRY_DELAY_MS; + const maxAttempts = options.lockMaxAttempts ?? OWNERSHIP_LOCK_MAX_ATTEMPTS; + const sleep = options.sleep ?? ((milliseconds: number) => delay(milliseconds)); + try { + await mkdir(dirname(metadataPath), { recursive: true }); + } catch (error) { + throw new SetupStorageError( + `Relay ownership metadata could not be prepared at ${metadataPath}.`, + error, + ); + } + + let handle: FileHandle | undefined; + let attempts = 0; + while (handle === undefined) { + try { + handle = await open(lockPath, 'wx', 0o600); + } catch (error) { + if (!isExists(error)) throw new SetupStorageError( - `Relay ownership metadata could not be prepared at ${input.metadataPath}.`, + `Relay ownership lock could not be opened at ${lockPath}.`, error, ); - } - const temporaryPath = `${input.metadataPath}.${process.pid}.${randomUUID()}.tmp`; - try { - await writeFile(temporaryPath, `${JSON.stringify(validated, null, 2)}\n`, 'utf8'); - await replaceFile(temporaryPath, input.metadataPath); - } catch (error) { - await unlink(temporaryPath).catch(() => undefined); - throw new SetupStorageError( - `Relay ownership metadata could not be written at ${input.metadataPath}.`, - error, + attempts += 1; + if (attempts >= maxAttempts) + throw new SetupConflictError( + `Another Relay configuration operation is in progress for ${metadataPath}. Retry after it completes.`, ); - } - }, - }; + await sleep(retryDelayMs); + } + } + + let result: T | undefined; + let actionFailed = false; + let actionError: unknown; + try { + try { + await handle.writeFile( + `${JSON.stringify({ pid: process.pid, acquiredAt: new Date().toISOString() })}\n`, + 'utf8', + ); + await handle.sync(); + } catch (error) { + throw new SetupStorageError( + `Relay ownership lock could not be written at ${lockPath}.`, + error, + ); + } + result = await action(); + } catch (error) { + actionFailed = true; + actionError = error; + } + + let cleanupError: unknown; + try { + await handle.close(); + } catch (error) { + cleanupError = error; + } + try { + await unlink(lockPath); + } catch (error) { + if (!isMissingFile(error)) cleanupError ??= error; + } + if (cleanupError !== undefined) + throw new SetupStorageError( + `Relay ownership lock could not be released at ${lockPath}.`, + cleanupError, + ); + if (actionFailed) throw actionError; + return result as T; +} + +async function delay(milliseconds: number): Promise { + await new Promise((resolveDelay) => setTimeout(resolveDelay, milliseconds)); } function validateOwnership(value: unknown, _applicationVersion: string): RelayOwnershipFile { @@ -128,3 +243,7 @@ function isRecord(value: unknown): value is Record { function isMissingFile(error: unknown): boolean { return isRecord(error) && error.code === 'ENOENT'; } + +function isExists(error: unknown): boolean { + return isRecord(error) && error.code === 'EEXIST'; +} diff --git a/tests/unit/distribution/setup/apply-integration-change.test.ts b/tests/unit/distribution/setup/apply-integration-change.test.ts index 5a7dc38..6386185 100644 --- a/tests/unit/distribution/setup/apply-integration-change.test.ts +++ b/tests/unit/distribution/setup/apply-integration-change.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; import { createCodexTomlAdapter } from '../../../../src/distribution/setup/clients/codex-toml-adapter.js'; +import { createClaudeJsonAdapter } from '../../../../src/distribution/setup/clients/claude-json-adapter.js'; import { applyIntegrationChange } from '../../../../src/distribution/setup/apply-integration-change.js'; import { planIntegrationChange } from '../../../../src/distribution/setup/plan-integration-change.js'; import { createOwnershipStore } from '../../../../src/distribution/setup/ownership-store.js'; @@ -43,7 +44,7 @@ describe('applyIntegrationChange', () => { writeFileSync(path, ''); const adapter = createCodexTomlAdapter(); const ownershipStore = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); - await ownershipStore.write({ + await ownershipStore.update(() => ({ schemaVersion: 1, integrations: [ { @@ -57,7 +58,7 @@ describe('applyIntegrationChange', () => { lastSuccessfulSetupAt: '2026-08-02T01:02:03.004Z', }, ], - }); + })); const plan = await planIntegrationChange({ action: 'remove', client: 'codex', @@ -77,4 +78,58 @@ describe('applyIntegrationChange', () => { expect(result.backupPath).toBeUndefined(); await expect(ownershipStore.read()).resolves.toEqual({ schemaVersion: 1, integrations: [] }); }); + + it('preserves both ownership records when Codex and Claude apply concurrently', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-apply-')); + const codexPath = join(root, 'codex.toml'); + const claudePath = join(root, 'claude.json'); + const metadataPath = join(root, 'config.json'); + writeFileSync(codexPath, ''); + writeFileSync(claudePath, ''); + const codexAdapter = createCodexTomlAdapter(); + const claudeAdapter = createClaudeJsonAdapter(); + const codexStore = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); + const claudeStore = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); + const ownership = { schemaVersion: 1 as const, integrations: [] }; + const codexPlan = await planIntegrationChange({ + action: 'setup', + client: 'codex', + configPath: codexPath, + adapter: codexAdapter, + ownership, + }); + const claudePlan = await planIntegrationChange({ + action: 'setup', + client: 'claude-code', + configPath: claudePath, + adapter: claudeAdapter, + ownership, + }); + + await Promise.all([ + applyIntegrationChange({ + plan: codexPlan, + adapter: codexAdapter, + ownershipStore: codexStore, + applicationVersion: '0.1.0', + now: new Date('2026-08-02T01:02:03.004Z'), + }), + applyIntegrationChange({ + plan: claudePlan, + adapter: claudeAdapter, + ownershipStore: claudeStore, + applicationVersion: '0.1.0', + now: new Date('2026-08-02T01:02:03.004Z'), + }), + ]); + + expect(readFileSync(codexPath, 'utf8')).toContain('command = "relay"'); + expect(readFileSync(claudePath, 'utf8')).toContain('"relay"'); + await expect(codexStore.read()).resolves.toMatchObject({ + integrations: [ + expect.objectContaining({ client: 'claude-code' }), + expect.objectContaining({ client: 'codex' }), + ], + }); + }); }); diff --git a/tests/unit/distribution/setup/ownership-store.test.ts b/tests/unit/distribution/setup/ownership-store.test.ts index 0d8c7ac..f84ada7 100644 --- a/tests/unit/distribution/setup/ownership-store.test.ts +++ b/tests/unit/distribution/setup/ownership-store.test.ts @@ -1,8 +1,9 @@ -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtempSync, readFileSync, rmSync, unlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; import { createOwnershipStore } from '../../../../src/distribution/setup/ownership-store.js'; +import { SetupConflictError } from '../../../../src/distribution/setup/setup-errors.js'; describe('ownership store', () => { const roots: string[] = []; @@ -36,7 +37,7 @@ describe('ownership store', () => { roots.push(root); const metadataPath = join(root, 'config.json'); const store = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); - await store.write({ + await store.update(() => ({ schemaVersion: 1, integrations: [ { @@ -50,9 +51,70 @@ describe('ownership store', () => { lastSuccessfulSetupAt: '2026-08-02T00:00:00.000Z', }, ], - }); + })); expect(JSON.parse(readFileSync(metadataPath, 'utf8')).integrations[0].configPath).toBe( join(root, 'codex.toml'), ); }); + + it('preserves both records when independent stores update concurrently', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-ownership-')); + roots.push(root); + const metadataPath = join(root, 'config.json'); + const codexPath = join(root, 'codex.toml'); + const claudePath = join(root, 'claude.json'); + const first = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); + const second = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); + const record = (client: 'codex' | 'claude-code', configPath: string) => ({ + client, + configPath, + entryId: 'relay' as const, + command: 'relay' as const, + args: ['mcp'] as const, + status: 'enabled' as const, + applicationVersion: '0.1.0', + lastSuccessfulSetupAt: '2026-08-02T00:00:00.000Z', + }); + + await Promise.all([ + first.update((current) => ({ + schemaVersion: 1, + integrations: [...current.integrations, record('codex', codexPath)], + })), + second.update((current) => ({ + schemaVersion: 1, + integrations: [...current.integrations, record('claude-code', claudePath)], + })), + ]); + + await expect(first.read()).resolves.toMatchObject({ + integrations: [ + expect.objectContaining({ client: 'claude-code', configPath: claudePath }), + expect.objectContaining({ client: 'codex', configPath: codexPath }), + ], + }); + }); + + it('fails with an actionable conflict when the ownership lock remains held', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-ownership-')); + roots.push(root); + const metadataPath = join(root, 'config.json'); + const lockPath = `${metadataPath}.relay-lock`; + writeFileSync(lockPath, 'held'); + const store = createOwnershipStore({ + metadataPath, + applicationVersion: '0.1.0', + lockRetryDelayMs: 0, + lockMaxAttempts: 2, + sleep: async () => undefined, + }); + try { + await expect(store.update((current) => current)).rejects.toMatchObject({ + constructor: SetupConflictError, + message: expect.stringMatching(/in progress.*retry/i), + }); + } finally { + unlinkSync(lockPath); + } + }); }); From 5979f5a925cdb66ce0de0ece289bf2a2949127a5 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 12:24:09 +0530 Subject: [PATCH 05/11] fix: restore absent client configs after setup failure --- .../setup/apply-integration-change.ts | 31 +++++- .../setup/backup-and-atomic-write.ts | 96 +++++++++++++++---- .../setup/apply-integration-change.test.ts | 76 ++++++++++++++- .../setup/backup-and-atomic-write.test.ts | 26 ++++- 4 files changed, 204 insertions(+), 25 deletions(-) diff --git a/src/distribution/setup/apply-integration-change.ts b/src/distribution/setup/apply-integration-change.ts index 3f18920..c292119 100644 --- a/src/distribution/setup/apply-integration-change.ts +++ b/src/distribution/setup/apply-integration-change.ts @@ -1,6 +1,6 @@ import type { ClientConfigAdapter } from './clients/client-adapter.js'; -import { backupAndAtomicWrite, restoreFile } from './backup-and-atomic-write.js'; -import { readFile } from 'node:fs/promises'; +import { backupAndAtomicWrite, restoreOriginalFile } from './backup-and-atomic-write.js'; +import { readFile, stat } from 'node:fs/promises'; import { normalize, resolve } from 'node:path'; import { fingerprint } from './plan-integration-change.js'; import { SetupStorageError } from './setup-errors.js'; @@ -61,8 +61,17 @@ export async function applyIntegrationChange(input: { } catch (error) { try { if (backup !== undefined) { - await restoreFile(backup.backupPath, input.plan.configPath); - input.adapter.parse(await readFile(input.plan.configPath, 'utf8')); + await restoreOriginalFile({ + backupPath: backup.backupPath, + targetPath: input.plan.configPath, + originalExisted: backup.originalExisted, + originalMode: backup.originalMode, + }); + if (backup.originalExisted) { + input.adapter.parse(await readFile(input.plan.configPath, 'utf8')); + } else { + await assertAbsent(input.plan.configPath); + } } } catch (restoreError) { throw new SetupStorageError( @@ -92,3 +101,17 @@ function sameOwnedPath(left: string, right: string): boolean { ? normalizedLeft.toLowerCase() === normalizedRight.toLowerCase() : normalizedLeft === normalizedRight; } + +async function assertAbsent(path: string): Promise { + try { + await stat(path); + } catch (error) { + if (isMissing(error)) return; + throw error; + } + throw new SetupStorageError(`Previously absent configuration was not removed: ${path}.`); +} + +function isMissing(error: unknown): boolean { + return typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT'; +} diff --git a/src/distribution/setup/backup-and-atomic-write.ts b/src/distribution/setup/backup-and-atomic-write.ts index e661c41..0f50079 100644 --- a/src/distribution/setup/backup-and-atomic-write.ts +++ b/src/distribution/setup/backup-and-atomic-write.ts @@ -4,35 +4,48 @@ import { basename, dirname, join } from 'node:path'; import { fingerprint } from './plan-integration-change.js'; import { SetupConflictError, SetupStorageError } from './setup-errors.js'; +export interface BackupAndAtomicWriteResult { + readonly backupPath: string; + readonly originalExisted: boolean; + readonly originalMode: number; +} + +interface OriginalFileState { + readonly existed: boolean; + readonly contents: Buffer; + readonly mode: number; +} + export async function backupAndAtomicWrite(input: { readonly targetPath: string; readonly expectedFingerprint: string; readonly nextContent: string; readonly validate: (content: string) => void; readonly now: Date; -}): Promise<{ readonly backupPath: string }> { - const original = await readFile(input.targetPath).catch((error: unknown) => { - if (isMissing(error)) return Buffer.from(''); - throw storageError(input.targetPath, error); - }); - if (fingerprint(original) !== input.expectedFingerprint) +}): Promise { + const original = await readOriginalFile(input.targetPath); + if (fingerprint(original.contents) !== input.expectedFingerprint) throw new SetupConflictError(`Configuration changed before replacement: ${input.targetPath}`); - const mode = await fileMode(input.targetPath); - const backupPath = await createExclusiveBackup(input.targetPath, input.now, original, mode); + const backupPath = await createExclusiveBackup( + input.targetPath, + input.now, + original.contents, + original.mode, + ); const tempPath = join( dirname(input.targetPath), `.${basename(input.targetPath)}.${process.pid}.${randomUUID()}.tmp`, ); let replaced = false; try { - const handle = await open(tempPath, 'wx', mode); + const handle = await open(tempPath, 'wx', original.mode); try { await handle.writeFile(input.nextContent, 'utf8'); await handle.sync(); } finally { await handle.close(); } - await chmod(tempPath, mode); + await chmod(tempPath, original.mode); input.validate(await readFile(tempPath, 'utf8')); const current = await readFile(input.targetPath).catch((error: unknown) => { if (isMissing(error)) return Buffer.from(''); @@ -43,12 +56,21 @@ export async function backupAndAtomicWrite(input: { await replaceFile(tempPath, input.targetPath); replaced = true; input.validate(await readFile(input.targetPath, 'utf8')); - return { backupPath }; + return { + backupPath, + originalExisted: original.existed, + originalMode: original.mode, + }; } catch (error) { if (replaced) { try { - await restoreFile(backupPath, input.targetPath, mode); - input.validate(await readFile(input.targetPath, 'utf8')); + await restoreOriginalFile({ + backupPath, + targetPath: input.targetPath, + originalExisted: original.existed, + originalMode: original.mode, + }); + await validateRestoredTarget(input.validate, input.targetPath, original.existed); } catch (restoreError) { throw new SetupStorageError( `Failed to restore ${input.targetPath} from ${backupPath}.`, @@ -62,11 +84,22 @@ export async function backupAndAtomicWrite(input: { } } -export async function restoreFile( - sourcePath: string, - targetPath: string, - mode?: number, -): Promise { +export async function restoreOriginalFile(input: { + readonly backupPath: string; + readonly targetPath: string; + readonly originalExisted: boolean; + readonly originalMode: number; +}): Promise { + if (!input.originalExisted) { + await unlink(input.targetPath).catch((error: unknown) => { + if (!isMissing(error)) throw storageError(input.targetPath, error); + }); + return; + } + await restoreFile(input.backupPath, input.targetPath, input.originalMode); +} + +async function restoreFile(sourcePath: string, targetPath: string, mode?: number): Promise { const source = await readFile(sourcePath); const preservedMode = mode ?? (await fileMode(sourcePath)); const temporaryPath = join( @@ -141,6 +174,33 @@ async function fileMode(path: string): Promise { } } +async function readOriginalFile(path: string): Promise { + try { + const contents = await readFile(path); + return { existed: true, contents, mode: await fileMode(path) }; + } catch (error) { + if (isMissing(error)) return { existed: false, contents: Buffer.alloc(0), mode: 0o600 }; + throw storageError(path, error); + } +} + +async function validateRestoredTarget( + validate: (content: string) => void, + targetPath: string, + existed: boolean, +): Promise { + if (!existed) { + try { + await readFile(targetPath); + } catch (error) { + if (isMissing(error)) return; + throw storageError(targetPath, error); + } + throw new SetupStorageError(`Previously absent configuration was not removed: ${targetPath}.`); + } + validate(await readFile(targetPath, 'utf8')); +} + function storageError(path: string, cause: unknown): SetupStorageError { return new SetupStorageError( `Could not safely update ${path}. Check permissions and retry.`, diff --git a/tests/unit/distribution/setup/apply-integration-change.test.ts b/tests/unit/distribution/setup/apply-integration-change.test.ts index 6386185..55fffb2 100644 --- a/tests/unit/distribution/setup/apply-integration-change.test.ts +++ b/tests/unit/distribution/setup/apply-integration-change.test.ts @@ -1,4 +1,11 @@ -import { mkdtempSync, readFileSync, writeFileSync } from 'node:fs'; +import { + existsSync, + mkdtempSync, + readFileSync, + readdirSync, + statSync, + writeFileSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; @@ -7,6 +14,8 @@ import { createClaudeJsonAdapter } from '../../../../src/distribution/setup/clie import { applyIntegrationChange } from '../../../../src/distribution/setup/apply-integration-change.js'; import { planIntegrationChange } from '../../../../src/distribution/setup/plan-integration-change.js'; import { createOwnershipStore } from '../../../../src/distribution/setup/ownership-store.js'; +import type { OwnershipStore } from '../../../../src/distribution/setup/ownership-store.js'; +import { SetupStorageError } from '../../../../src/distribution/setup/setup-errors.js'; describe('applyIntegrationChange', () => { it('updates the client before writing enabled ownership metadata', async () => { @@ -132,4 +141,69 @@ describe('applyIntegrationChange', () => { ], }); }); + + it('restores an absent config after metadata persistence fails and retains the backup', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-apply-')); + const path = join(root, 'codex.toml'); + const adapter = createCodexTomlAdapter(); + const plan = await planIntegrationChange({ + action: 'setup', + client: 'codex', + configPath: path, + adapter, + ownership: { schemaVersion: 1, integrations: [] }, + }); + const ownershipStore: OwnershipStore = { + read: async () => ({ schemaVersion: 1, integrations: [] }), + update: async () => { + throw new SetupStorageError('forced metadata failure'); + }, + }; + + await expect( + applyIntegrationChange({ + plan, + adapter, + ownershipStore, + applicationVersion: '0.1.0', + now: new Date('2026-08-02T01:02:03.004Z'), + }), + ).rejects.toThrow(/restored after metadata persistence failed/i); + expect(existsSync(path)).toBe(false); + expect(readdirSync(root).some((name) => name.includes('.relay-backup-'))).toBe(true); + }); + + it('restores existing config bytes and mode after metadata persistence fails', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-apply-')); + const path = join(root, 'codex.toml'); + const original = '[profile]\nname = "existing"\n'; + writeFileSync(path, original); + const originalMode = statSync(path).mode & 0o777; + const adapter = createCodexTomlAdapter(); + const plan = await planIntegrationChange({ + action: 'setup', + client: 'codex', + configPath: path, + adapter, + ownership: { schemaVersion: 1, integrations: [] }, + }); + const ownershipStore: OwnershipStore = { + read: async () => ({ schemaVersion: 1, integrations: [] }), + update: async () => { + throw new SetupStorageError('forced metadata failure'); + }, + }; + + await expect( + applyIntegrationChange({ + plan, + adapter, + ownershipStore, + applicationVersion: '0.1.0', + now: new Date('2026-08-02T01:02:03.004Z'), + }), + ).rejects.toThrow(/restored after metadata persistence failed/i); + expect(readFileSync(path, 'utf8')).toBe(original); + expect(statSync(path).mode & 0o777).toBe(originalMode); + }); }); diff --git a/tests/unit/distribution/setup/backup-and-atomic-write.test.ts b/tests/unit/distribution/setup/backup-and-atomic-write.test.ts index 1477734..2749954 100644 --- a/tests/unit/distribution/setup/backup-and-atomic-write.test.ts +++ b/tests/unit/distribution/setup/backup-and-atomic-write.test.ts @@ -1,8 +1,11 @@ -import { readFileSync, mkdtempSync, writeFileSync } from 'node:fs'; +import { existsSync, readFileSync, mkdtempSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; -import { backupAndAtomicWrite } from '../../../../src/distribution/setup/backup-and-atomic-write.js'; +import { + backupAndAtomicWrite, + restoreOriginalFile, +} from '../../../../src/distribution/setup/backup-and-atomic-write.js'; import { fingerprint } from '../../../../src/distribution/setup/plan-integration-change.js'; describe('backupAndAtomicWrite', () => { @@ -55,4 +58,23 @@ describe('backupAndAtomicWrite', () => { expect(result.backupPath).toContain('.relay-backup-20260802T010203.004Z-1'); expect(readFileSync(result.backupPath, 'utf8')).toBe(original); }); + + it('restores a missing original file to absence while retaining an empty backup', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-write-')); + const path = join(root, 'new-config.toml'); + const result = await backupAndAtomicWrite({ + targetPath: path, + expectedFingerprint: fingerprint(''), + nextContent: 'created', + validate: () => undefined, + now: new Date('2026-08-02T01:02:03.004Z'), + }); + + expect(result.originalExisted).toBe(false); + expect(existsSync(path)).toBe(true); + await restoreOriginalFile({ ...result, targetPath: path }); + expect(existsSync(path)).toBe(false); + expect(existsSync(result.backupPath)).toBe(true); + expect(readFileSync(result.backupPath)).toHaveLength(0); + }); }); From e3a221535fe782a9ce3cc1e7a24c8e747fb048c5 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 12:30:25 +0530 Subject: [PATCH 06/11] chore: format PR 48 remediation plan --- .../2026-08-02-pr-48-review-remediation.md | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/docs/superpowers/plans/2026-08-02-pr-48-review-remediation.md b/docs/superpowers/plans/2026-08-02-pr-48-review-remediation.md index 05b2042..9a35c02 100644 --- a/docs/superpowers/plans/2026-08-02-pr-48-review-remediation.md +++ b/docs/superpowers/plans/2026-08-02-pr-48-review-remediation.md @@ -53,9 +53,7 @@ Expose one mutation API so callers cannot accidentally reintroduce an unlocked r ```ts export interface OwnershipStore { read(): Promise; - update( - mutate: (current: RelayOwnershipFile) => RelayOwnershipFile, - ): Promise; + update(mutate: (current: RelayOwnershipFile) => RelayOwnershipFile): Promise; } ``` @@ -184,19 +182,14 @@ git commit -m "test: use host-native setup config paths" ```ts export interface OwnershipStore { read(): Promise; - update( - mutate: (current: RelayOwnershipFile) => RelayOwnershipFile, - ): Promise; + update(mutate: (current: RelayOwnershipFile) => RelayOwnershipFile): Promise; } ``` - Internal helper: ```ts -async function withOwnershipLock( - metadataPath: string, - action: () => Promise, -): Promise; +async function withOwnershipLock(metadataPath: string, action: () => Promise): Promise; ``` - [ ] **Step 1: Write a failing concurrent-update test.** @@ -316,10 +309,7 @@ await input.ownershipStore.update((ownership) => { ); return { schemaVersion: 1, - integrations: - input.plan.operation === 'removed' - ? existing - : [...existing, nextRecord], + integrations: input.plan.operation === 'removed' ? existing : [...existing, nextRecord], }; }); ``` From e2e49f0428efae6b62a13d10fbde91f5ffe5fb12 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 13:14:04 +0530 Subject: [PATCH 07/11] docs: correct config mutation apply requirements --- docs/setup-and-configuration.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/docs/setup-and-configuration.md b/docs/setup-and-configuration.md index e56215f..6625e24 100644 --- a/docs/setup-and-configuration.md +++ b/docs/setup-and-configuration.md @@ -2,7 +2,7 @@ `relay setup` initializes Relay's data and configuration roots and opens the canonical database runtime so forward migrations run. It never replaces, resets, or deletes an existing database. Re-running it is safe. -Mutable client setup is preview-first: +Mutable client setup is preview-first: it mutates only when `--apply` is supplied. ```text relay setup --client codex --config-file @@ -15,7 +15,7 @@ Codex and Claude Code require an explicit absolute configuration path. Relay nev Before applying, inspect the target, operation, entry identifier, and snippet. Relay proves ownership using the exact `relay` entry, the `relay` command, `['mcp']` arguments, client, and normalized path. Unknown or conflicting entries fail closed. A changed client file receives a collision-safe sibling backup and a validated atomic replacement; Relay ownership metadata is updated only after the replacement is reparsed successfully. -Use `relay config paths` and `relay config integrations` to inspect effective paths and Relay-owned records. `relay config disable` removes an exact owned entry while retaining disabled ownership; setup can safely re-enable it. `relay config remove` removes only the exact owned entry and ownership record. These operations retain the database, tasks, backups, and unrelated configuration. +Use `relay config paths` to inspect effective paths. Before destructive configuration actions, run `relay config integrations` to inspect Relay-owned records. `relay config disable` removes an exact owned entry while retaining disabled ownership; setup can safely re-enable it. `relay config remove` removes only the exact owned entry and ownership record. Both commands require `--apply`. These operations retain the database, tasks, backups, and unrelated configuration. The complete inspection and mutation surface is: @@ -27,18 +27,16 @@ relay config integrations --output json relay config snippet --client codex relay config snippet --client claude-code relay config snippet --client generic-mcp -relay config disable --client codex --config-file relay config disable --client codex --config-file --apply -relay config remove --client codex --config-file relay config remove --client codex --config-file --apply ``` -The client configuration path is always explicit and absolute. `--apply` is required for a mutation; without it, setup, disable, and remove return a preview. Generic MCP is snippet-only and has no mutation mode. +The client configuration path is always explicit and absolute. Setup is preview-first and mutates only when `--apply` is supplied. `relay config disable` and `relay config remove` always require `--apply`; omitting it is a usage error with exit code 2, not a preview. Generic MCP is snippet-only and has no mutation mode. For the human safety gate, use this checklist with a disposable absolute path and an isolated `RELAY_DB_PATH`: 1. Copy a real Codex or Claude Code configuration to a disposable file; never start with the only live configuration copy. -2. Run the preview and record the reported operation and exact snippet. +2. Run the setup preview and record the reported operation and exact snippet. 3. Apply the setup and verify the configured entry is exact. 4. Compare unrelated configuration bytes before and after. 5. Compare the backup with the original bytes. From c45390ad42ca61d702f505658b743ea05e1a4da8 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 14:31:35 +0530 Subject: [PATCH 08/11] fix: address remaining review findings --- package.json | 2 +- pnpm-lock.yaml | 81 ++++++++- scripts/package/smoke-installed-package.ts | 9 +- scripts/validate-agent-integration-assets.ts | 38 ++--- scripts/validate-repository-assets.ts | 29 ++-- .../setup/apply-integration-change.ts | 155 +++++++++++------- .../setup/backup-and-atomic-write.ts | 19 +-- .../setup/clients/claude-json-adapter.ts | 12 +- .../setup/clients/codex-toml-adapter.ts | 14 +- src/distribution/setup/file-lock.ts | 98 +++++++++++ src/distribution/setup/initialize-relay.ts | 2 +- src/distribution/setup/ownership-store.ts | 135 +++------------ .../setup/plan-integration-change.ts | 3 +- src/distribution/setup/relay-entry.ts | 4 + src/distribution/setup/snippets.ts | 7 +- src/interfaces/cli/operational-output.ts | 16 +- src/interfaces/cli/run-operational-command.ts | 29 +++- src/interfaces/production-dependencies.ts | 8 +- .../valid/integrations/generic-mcp/README.md | 2 +- .../setup/claude-code/no-mcp-servers.json | 3 + tests/integration/cli.test.ts | 2 +- tests/integration/setup-workflow.test.ts | 2 +- .../contracts/distribution-contract.test.ts | 2 +- .../setup/apply-integration-change.test.ts | 136 ++++++++++++++- .../setup/backup-and-atomic-write.test.ts | 42 ++++- .../setup/claude-json-adapter.test.ts | 3 + .../setup/initialize-relay.test.ts | 2 +- .../setup/ownership-store.test.ts | 22 ++- .../setup/plan-integration-change.test.ts | 112 +++++++++++++ .../unit/distribution/setup/snippets.test.ts | 2 +- .../cli/operational-commands.test.ts | 32 ++++ .../interfaces/cli/operational-output.test.ts | 20 +++ .../cli/run-operational-command.test.ts | 61 ++++--- .../validate-agent-integration-assets.test.ts | 2 +- 34 files changed, 811 insertions(+), 295 deletions(-) create mode 100644 src/distribution/setup/file-lock.ts create mode 100644 src/distribution/setup/relay-entry.ts create mode 100644 tests/fixtures/setup/claude-code/no-mcp-servers.json create mode 100644 tests/unit/interfaces/cli/operational-output.test.ts diff --git a/package.json b/package.json index 1da2bd8..55c9690 100644 --- a/package.json +++ b/package.json @@ -96,9 +96,9 @@ "vitest": "^4.1.10" }, "dependencies": { - "@iarna/toml": "2.2.5", "@modelcontextprotocol/sdk": "^1.29.0", "better-sqlite3": "^13.0.1", + "js-toml": "^1.2.1", "jsonc-parser": "3.3.1", "react": "^19.2.8", "react-dom": "^19.2.8", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 67ce628..6a1cdf4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -11,15 +11,15 @@ importers: .: dependencies: - '@iarna/toml': - specifier: 2.2.5 - version: 2.2.5 '@modelcontextprotocol/sdk': specifier: ^1.29.0 version: 1.29.0(zod@4.4.3) better-sqlite3: specifier: ^13.0.1 version: 13.0.1 + js-toml: + specifier: ^1.2.1 + version: 1.2.1 jsonc-parser: specifier: 3.3.1 version: 3.3.1 @@ -179,6 +179,10 @@ packages: engines: {node: '>=6.0.0'} hasBin: true + '@babel/runtime-corejs3@7.29.7': + resolution: {integrity: sha512-ppj9ouYku+RX0ljtgZd+KMO5mkM2bCqg8H2PYAFWnLsHEIKIdRojqbJ2i3eVHrisuxy7nOFCmngTDdWtUCdXUQ==} + engines: {node: '>=6.9.0'} + '@babel/runtime@7.29.7': resolution: {integrity: sha512-Nq8OhGWiZIZGV6hLHoyAKLLcJihP/xFeBMGJoUrxTX2psI8dCifzLhZISFb+VWS3wFMRDmCGw5R+dOySCqPLhw==} engines: {node: '>=6.9.0'} @@ -203,6 +207,21 @@ packages: resolution: {integrity: sha512-ctxtJ/eA+t+6q2++vj5j7FYX3nRu311q1wfYH3xjlLOsczhlhxAg2FWNUXhpGvAw3BWo1xBcvOV6/YLc2r5FJw==} hasBin: true + '@chevrotain/cst-dts-gen@12.0.0': + resolution: {integrity: sha512-fSL4KXjTl7cDgf0B5Rip9Q05BOrYvkJV/RrBTE/bKDN096E4hN/ySpcBK5B24T76dlQ2i32Zc3PAE27jFnFrKg==} + + '@chevrotain/gast@12.0.0': + resolution: {integrity: sha512-1ne/m3XsIT8aEdrvT33so0GUC+wkctpUPK6zU9IlOyJLUbR0rg4G7ZiApiJbggpgPir9ERy3FRjT6T7lpgetnQ==} + + '@chevrotain/regexp-to-ast@12.0.0': + resolution: {integrity: sha512-p+EW9MaJwgaHguhoqwOtx/FwuGr+DnNn857sXWOi/mClXIkPGl3rn7hGNWvo31HA3vyeQxjqe+H36yZJwYU8cA==} + + '@chevrotain/types@12.0.0': + resolution: {integrity: sha512-S+04vjFQKeuYw0/eW3U52LkAHQsB1ASxsPGsLPUyQgrZ2iNNibQrsidruDzjEX2JYfespXMG0eZmXlhA6z7nWA==} + + '@chevrotain/utils@12.0.0': + resolution: {integrity: sha512-lB59uJoaGIfOOL9knQqQRfhl9g7x8/wqFkp13zTdkRu1huG9kg6IJs1O8hqj9rs6h7orGxHJUKb+mX3rPbWGhA==} + '@csstools/color-helpers@6.1.0': resolution: {integrity: sha512-064IFJdjTfUqnjpCVpMOdbr8FLQBhinbZj6yRv2An2E41O/pLEXqfFRWqGq/SxlE5PEUYTlvWsG2r8MswAVvkg==} engines: {node: '>=20.19.0'} @@ -625,9 +644,6 @@ packages: resolution: {integrity: sha512-bV0Tgo9K4hfPCek+aMAn81RppFKv2ySDQeMoSZuvTASywNTnVJCArCZE2FWqpvIatKu7VMRLWlR1EazvVhDyhQ==} engines: {node: '>=18.18'} - '@iarna/toml@2.2.5': - resolution: {integrity: sha512-trnsAYxU3xnS1gPHPyU961coFyLkh4gAD/0zQ5mymY4yOZ+CYvsPqUbOFSw0aDM4y0tV7tiFxL/1XfXPNC6IPg==} - '@inquirer/checkbox@3.0.1': resolution: {integrity: sha512-0hm2nrToWUdD6/UHnel/UKGdk1//ke5zGUpHIvk5ZWmaKezlGxZkOJXNSWsdxO/rEqTkbB3lNC2J6nBElV2aAQ==} engines: {node: '>=18'} @@ -1252,6 +1268,10 @@ packages: chardet@0.7.0: resolution: {integrity: sha512-mT8iDcrh03qDGRRmoA2hmBJnxpllMR+0/0qlzjqZES6NdiWDcZkCNAk4rPFZ9Q85r27unkiNNg8ZOiwZXBHwcA==} + chevrotain@12.0.0: + resolution: {integrity: sha512-csJvb+6kEiQaqo1woTdSAuOWdN0WTLIydkKrBnS+V5gZz0oqBrp4kQ35519QgK6TpBThiG3V1vNSHlIkv4AglQ==} + engines: {node: '>=22.0.0'} + chokidar@4.0.3: resolution: {integrity: sha512-Qgzu8kfBvo+cA4962jnP1KkS6Dop5NS6g7R5LFYJr4b8Ub94PPQXUksCw9PvXoeXPRRddRNC5C1JQUR2SMGtnA==} engines: {node: '>= 14.16.0'} @@ -1314,6 +1334,9 @@ packages: resolution: {integrity: sha512-yki5XnKuf750l50uGTllt6kKILY4nQ1eNIQatoXEByZ5dWgnKqbnqmTrBE5B4N7lrMJKQ2ytWMiTO2o0v6Ew/w==} engines: {node: '>= 0.6'} + core-js-pure@3.49.0: + resolution: {integrity: sha512-XM4RFka59xATyJv/cS3O3Kml72hQXUeGRuuTmMYFxwzc9/7C8OYTaIR/Ji+Yt8DXzsFLNhat15cE/JP15HrCgw==} + cors@2.8.6: resolution: {integrity: sha512-tJtZBBHA6vjIAaF6EnIaq6laBBP9aq/Y3ouVJjEfoHbRBcHBAHYcMh/w8LDrk2PvIMMq8gmopa5D4V8RmbrxGw==} engines: {node: '>= 0.10'} @@ -1734,6 +1757,9 @@ packages: js-tokens@4.0.0: resolution: {integrity: sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==} + js-toml@1.2.1: + resolution: {integrity: sha512-alpDea3b3mKroupsayNq96hMkqQg1gdRTq55TPRLI+cqwhBbm6/0y8plFCZyZrAJhFZDgWwu56JB/HiYP/QRlg==} + jsdom@29.1.1: resolution: {integrity: sha512-ECi4Fi2f7BdJtUKTflYRTiaMxIB0O6zfR1fX0GXpUrf6flp8QIYn1UT20YQqdSOfk2dfkCwS8LAFoJDEppNK5Q==} engines: {node: ^20.19.0 || ^22.13.0 || >=24.0.0} @@ -2519,6 +2545,9 @@ packages: xmlchars@2.2.0: resolution: {integrity: sha512-JZnDKK8B0RCDw84FNdDAIpZK+JuJw+s7Lz8nksI7SIuU3UXJJslUthsi+uWBUYOwPFwW7W7PRLRfUKpxjtjFCw==} + xregexp@5.1.2: + resolution: {integrity: sha512-6hGgEMCGhqCTFEJbqmWrNIPqfpdirdGWkqshu7fFZddmTSfgv5Sn9D2SaKloR79s5VUiUlpwzg3CM3G6D3VIlw==} + y18n@5.0.8: resolution: {integrity: sha512-0pfFzegeDWJHJIAmTLRP2DwHjdF5s7jo9tuztdQxAhINCdvS+3nGINqPd00AphqJR/0LhANUS6/+7SCb98YOfA==} engines: {node: '>=10'} @@ -2674,6 +2703,10 @@ snapshots: dependencies: '@babel/types': 7.29.7 + '@babel/runtime-corejs3@7.29.7': + dependencies: + core-js-pure: 3.49.0 + '@babel/runtime@7.29.7': {} '@babel/template@7.29.7': @@ -2705,6 +2738,21 @@ snapshots: dependencies: css-tree: 3.2.1 + '@chevrotain/cst-dts-gen@12.0.0': + dependencies: + '@chevrotain/gast': 12.0.0 + '@chevrotain/types': 12.0.0 + + '@chevrotain/gast@12.0.0': + dependencies: + '@chevrotain/types': 12.0.0 + + '@chevrotain/regexp-to-ast@12.0.0': {} + + '@chevrotain/types@12.0.0': {} + + '@chevrotain/utils@12.0.0': {} + '@csstools/color-helpers@6.1.0': {} '@csstools/css-calc@3.3.0(@csstools/css-parser-algorithms@4.0.0(@csstools/css-tokenizer@4.0.0))(@csstools/css-tokenizer@4.0.0)': @@ -2953,8 +3001,6 @@ snapshots: '@humanwhocodes/retry@0.4.3': {} - '@iarna/toml@2.2.5': {} - '@inquirer/checkbox@3.0.1': dependencies: '@inquirer/core': 9.2.1 @@ -3583,6 +3629,14 @@ snapshots: chardet@0.7.0: {} + chevrotain@12.0.0: + dependencies: + '@chevrotain/cst-dts-gen': 12.0.0 + '@chevrotain/gast': 12.0.0 + '@chevrotain/regexp-to-ast': 12.0.0 + '@chevrotain/types': 12.0.0 + '@chevrotain/utils': 12.0.0 + chokidar@4.0.3: dependencies: readdirp: 4.1.2 @@ -3630,6 +3684,8 @@ snapshots: cookie@0.7.2: {} + core-js-pure@3.49.0: {} + cors@2.8.6: dependencies: object-assign: 4.1.1 @@ -4105,6 +4161,11 @@ snapshots: js-tokens@4.0.0: {} + js-toml@1.2.1: + dependencies: + chevrotain: 12.0.0 + xregexp: 5.1.2 + jsdom@29.1.1: dependencies: '@asamuzakjp/css-color': 5.1.11 @@ -4822,6 +4883,10 @@ snapshots: xmlchars@2.2.0: {} + xregexp@5.1.2: + dependencies: + '@babel/runtime-corejs3': 7.29.7 + y18n@5.0.8: {} yallist@3.1.1: {} diff --git a/scripts/package/smoke-installed-package.ts b/scripts/package/smoke-installed-package.ts index f78a132..bdf601d 100644 --- a/scripts/package/smoke-installed-package.ts +++ b/scripts/package/smoke-installed-package.ts @@ -118,6 +118,7 @@ function runCli( interface CliEnvelope { readonly data?: { + readonly changed?: boolean; readonly task?: { readonly id?: string }; readonly change?: { readonly to?: string }; }; @@ -238,9 +239,14 @@ export async function verifyInstalledPackage(rootDir = process.cwd()): Promise>; }; - const server = parsed.mcpServers?.relay ?? parsed; - if ( - JSON.stringify(Object.keys(server).sort()) !== JSON.stringify(['args', 'command']) || - server.command !== 'relay' || - JSON.stringify(server.args) !== JSON.stringify(['mcp']) - ) - fail(`${path} must use separate command and arguments for the installed relay mcp command.`); + validateInstalledMcpServer(parsed.mcpServers?.relay ?? parsed, path); } const tomlSource = readAsset(rootDir, 'integrations/codex/config.toml.example'); if (/(?:[A-Z]:[\\/]Users[\\/]|\/Users\/|\/home\/|~\/)/i.test(tomlSource)) { fail('integrations/codex/config.toml.example must not contain a machine-specific home path.'); } - const toml = parse(tomlSource) as { + const toml = parseToml(tomlSource) as { mcp_servers?: { relay?: Record; }; }; const codexServer = toml.mcp_servers?.relay; - if ( - codexServer === undefined || - JSON.stringify(Object.keys(codexServer).sort()) !== JSON.stringify(['args', 'command']) || - codexServer?.command !== 'relay' || - !Array.isArray(codexServer.args) || - codexServer.args.length !== 1 || - codexServer.args[0] !== 'mcp' - ) { + if (codexServer === undefined) fail('integrations/codex/config.toml.example must invoke the installed relay mcp command.'); - } + validateInstalledMcpServer(codexServer, 'integrations/codex/config.toml.example'); +} + +function validateInstalledMcpServer(value: unknown, label: string): void { + if (typeof value !== 'object' || value === null || Array.isArray(value)) + fail(`${label} must define one installed relay server object.`); + const server = value as Record; + if ( + JSON.stringify(Object.keys(server).sort()) !== JSON.stringify(['args', 'command']) || + server.command !== 'relay' || + JSON.stringify(server.args) !== JSON.stringify(['mcp']) + ) + fail(`${label} must use separate command and arguments for the installed relay mcp command.`); } export function validateAgentIntegrationAssets( @@ -276,7 +275,4 @@ export function validateAgentIntegrationAssets( fail('Vendor assets must not copy behavioural policy.'); validateRemovalGuidance(rootDir); validateTemplateShape(rootDir); - JSON.parse(readFileSync(join(integrationRoot, 'generic-mcp/server-config.json.example'), 'utf8')); - JSON.parse(readFileSync(join(integrationRoot, 'claude-code/.mcp.json.example'), 'utf8')); - parse(readFileSync(join(integrationRoot, 'codex/config.toml.example'), 'utf8')); } diff --git a/scripts/validate-repository-assets.ts b/scripts/validate-repository-assets.ts index 4c9bcb7..b70d93c 100644 --- a/scripts/validate-repository-assets.ts +++ b/scripts/validate-repository-assets.ts @@ -1,7 +1,7 @@ import { existsSync, readFileSync, readdirSync } from 'node:fs'; import { isAbsolute, join, relative, resolve } from 'node:path'; import { pathToFileURL } from 'node:url'; -import { parse as parseToml } from '@iarna/toml'; +import { load as parseToml } from 'js-toml'; import { parse as parseJsonc, type ParseError } from 'jsonc-parser'; import { validateSkillAssets } from './validate-skill-assets.js'; import { validateAgentIntegrationAssets } from './validate-agent-integration-assets.js'; @@ -132,15 +132,26 @@ function validateJsonFiles(files: readonly string[]): void { const normalizedPath = filePath.replaceAll('\\', '/'); if ( normalizedPath.endsWith('tests/fixtures/setup/metadata/malformed.json') || - normalizedPath.endsWith('tests/fixtures/setup/claude-code/malformed.json') || - normalizedPath.endsWith('tests/fixtures/setup/claude-code/formatted.json') + normalizedPath.endsWith('tests/fixtures/setup/claude-code/malformed.json') ) { continue; } - const errors: ParseError[] = []; - parseJsonc(readFileSync(filePath, 'utf-8'), errors); - if (errors.length > 0) throw new Error(`Malformed JSON/JSONC asset: ${filePath}`); + const source = readFileSync(filePath, 'utf-8'); + const isClaudeJsoncFixture = + normalizedPath.endsWith('tests/fixtures/setup/claude-code/unrelated.json') || + normalizedPath.endsWith('tests/fixtures/setup/claude-code/formatted.json'); + if (isClaudeJsoncFixture) { + const errors: ParseError[] = []; + parseJsonc(source, errors); + if (errors.length > 0) throw new Error(`Malformed JSON/JSONC asset: ${filePath}`); + continue; + } + try { + JSON.parse(source); + } catch (error) { + throw new Error(`Malformed JSON asset: ${filePath}`, { cause: error }); + } } } @@ -400,12 +411,6 @@ function validateDistributionContract( if (compatibility.releaseTrigger !== 'manual-maintainer-action') { fail('Distribution version fixture must require a manual maintainer release action.'); } - - for (const path of requiredDistributionAssets.filter((asset) => - asset.startsWith('tests/fixtures/setup/'), - )) { - if (!existsSync(join(rootDir, path))) fail(`Setup fixture is missing: ${path}`); - } } export function validateRepositoryAssets(options: ValidateRepositoryAssetsOptions = {}): void { diff --git a/src/distribution/setup/apply-integration-change.ts b/src/distribution/setup/apply-integration-change.ts index c292119..641518a 100644 --- a/src/distribution/setup/apply-integration-change.ts +++ b/src/distribution/setup/apply-integration-change.ts @@ -1,9 +1,15 @@ import type { ClientConfigAdapter } from './clients/client-adapter.js'; -import { backupAndAtomicWrite, restoreOriginalFile } from './backup-and-atomic-write.js'; +import { + backupAndAtomicWrite, + restoreOriginalFile, + type BackupAndAtomicWriteResult, +} from './backup-and-atomic-write.js'; import { readFile, stat } from 'node:fs/promises'; import { normalize, resolve } from 'node:path'; +import { withExclusiveFileLock } from './file-lock.js'; +import { RELAY_ARGS, RELAY_COMMAND, RELAY_ENTRY_ID } from './relay-entry.js'; import { fingerprint } from './plan-integration-change.js'; -import { SetupStorageError } from './setup-errors.js'; +import { SetupConflictError, SetupStorageError } from './setup-errors.js'; import type { IntegrationChangePlan, IntegrationChangeResult } from './setup-types.js'; import type { OwnershipStore } from './ownership-store.js'; @@ -18,80 +24,103 @@ export async function applyIntegrationChange(input: { return { client: input.plan.client, configPath: input.plan.configPath, - entryId: 'relay', + entryId: RELAY_ENTRY_ID, operation: 'unchanged', changed: false, }; } - const clientChanged = fingerprint(input.plan.nextContent) !== input.plan.beforeFingerprint; - const backup = clientChanged - ? await backupAndAtomicWrite({ - targetPath: input.plan.configPath, - expectedFingerprint: input.plan.beforeFingerprint, - nextContent: input.plan.nextContent, - validate: (content) => input.adapter.parse(content), - now: input.now, - }) - : undefined; - try { - const nextRecord = { - client: input.plan.client, - configPath: input.plan.configPath, - entryId: 'relay' as const, - command: 'relay' as const, - args: ['mcp'] as const, - status: input.plan.operation === 'disabled' ? ('disabled' as const) : ('enabled' as const), - applicationVersion: input.applicationVersion, - lastSuccessfulSetupAt: input.now.toISOString(), - ...(backup === undefined ? {} : { lastBackupPath: backup.backupPath }), - }; - await input.ownershipStore.update((ownership) => { - const existing = ownership.integrations.filter( - (record) => - !( - record.client === input.plan.client && - sameOwnedPath(record.configPath, input.plan.configPath) - ), - ); - return { - schemaVersion: 1, - integrations: input.plan.operation === 'removed' ? existing : [...existing, nextRecord], - }; - }); - } catch (error) { + return withExclusiveFileLock(`${input.plan.configPath}.relay-lock`, async () => { + const clientChanged = fingerprint(input.plan.nextContent) !== input.plan.beforeFingerprint; + let backup: BackupAndAtomicWriteResult | undefined; try { - if (backup !== undefined) { - await restoreOriginalFile({ - backupPath: backup.backupPath, + if (clientChanged) { + backup = await backupAndAtomicWrite({ targetPath: input.plan.configPath, - originalExisted: backup.originalExisted, - originalMode: backup.originalMode, + expectedFingerprint: input.plan.beforeFingerprint, + nextContent: input.plan.nextContent, + validate: (content) => input.adapter.parse(content), + now: input.now, }); - if (backup.originalExisted) { - input.adapter.parse(await readFile(input.plan.configPath, 'utf8')); - } else { - await assertAbsent(input.plan.configPath); + } + + const nextRecord = { + client: input.plan.client, + configPath: input.plan.configPath, + entryId: RELAY_ENTRY_ID, + command: RELAY_COMMAND, + args: RELAY_ARGS, + status: input.plan.operation === 'disabled' ? ('disabled' as const) : ('enabled' as const), + applicationVersion: input.applicationVersion, + lastSuccessfulSetupAt: input.now.toISOString(), + ...(backup?.backupPath === undefined ? {} : { lastBackupPath: backup.backupPath }), + }; + await input.ownershipStore.update(async (ownership) => { + if ( + !clientChanged && + fingerprint(await readCurrentContent(input.plan.configPath)) !== + input.plan.beforeFingerprint + ) { + throw new SetupConflictError( + `Configuration changed before ownership metadata update: ${input.plan.configPath}`, + ); } + const existing = ownership.integrations.filter( + (record) => + !( + record.client === input.plan.client && + sameOwnedPath(record.configPath, input.plan.configPath) + ), + ); + return { + schemaVersion: 1, + integrations: input.plan.operation === 'removed' ? existing : [...existing, nextRecord], + }; + }); + } catch (error) { + try { + if (backup !== undefined) { + await restoreOriginalFile({ + ...(backup.backupPath === undefined ? {} : { backupPath: backup.backupPath }), + targetPath: input.plan.configPath, + originalExisted: backup.originalExisted, + originalMode: backup.originalMode, + }); + if (backup.originalExisted) { + input.adapter.parse(await readFile(input.plan.configPath, 'utf8')); + } else { + await assertAbsent(input.plan.configPath); + } + } + } catch (restoreError) { + throw new SetupStorageError( + `Client configuration was replaced but could not be restored from ${backup?.backupPath ?? input.plan.configPath}.`, + new AggregateError([error, restoreError]), + ); } - } catch (restoreError) { + if (error instanceof SetupConflictError) throw error; throw new SetupStorageError( - `Client configuration was replaced but could not be restored from ${backup?.backupPath ?? input.plan.configPath}.`, - restoreError, + `Client configuration was restored after metadata persistence failed: ${input.plan.configPath}.`, + error, ); } - throw new SetupStorageError( - `Client configuration was restored after metadata persistence failed: ${input.plan.configPath}.`, - error, - ); + return { + client: input.plan.client, + configPath: input.plan.configPath, + entryId: RELAY_ENTRY_ID, + operation: input.plan.operation, + changed: true, + ...(backup?.backupPath === undefined ? {} : { backupPath: backup.backupPath }), + }; + }); +} + +async function readCurrentContent(path: string): Promise { + try { + return await readFile(path, 'utf8'); + } catch (error) { + if (isMissing(error)) return ''; + throw new SetupStorageError(`Client configuration could not be reread: ${path}.`, error); } - return { - client: input.plan.client, - configPath: input.plan.configPath, - entryId: 'relay', - operation: input.plan.operation, - changed: true, - ...(backup === undefined ? {} : { backupPath: backup.backupPath }), - }; } function sameOwnedPath(left: string, right: string): boolean { diff --git a/src/distribution/setup/backup-and-atomic-write.ts b/src/distribution/setup/backup-and-atomic-write.ts index 0f50079..fe9a20f 100644 --- a/src/distribution/setup/backup-and-atomic-write.ts +++ b/src/distribution/setup/backup-and-atomic-write.ts @@ -5,7 +5,7 @@ import { fingerprint } from './plan-integration-change.js'; import { SetupConflictError, SetupStorageError } from './setup-errors.js'; export interface BackupAndAtomicWriteResult { - readonly backupPath: string; + readonly backupPath?: string; readonly originalExisted: boolean; readonly originalMode: number; } @@ -26,12 +26,9 @@ export async function backupAndAtomicWrite(input: { const original = await readOriginalFile(input.targetPath); if (fingerprint(original.contents) !== input.expectedFingerprint) throw new SetupConflictError(`Configuration changed before replacement: ${input.targetPath}`); - const backupPath = await createExclusiveBackup( - input.targetPath, - input.now, - original.contents, - original.mode, - ); + const backupPath = original.existed + ? await createExclusiveBackup(input.targetPath, input.now, original.contents, original.mode) + : undefined; const tempPath = join( dirname(input.targetPath), `.${basename(input.targetPath)}.${process.pid}.${randomUUID()}.tmp`, @@ -57,7 +54,7 @@ export async function backupAndAtomicWrite(input: { replaced = true; input.validate(await readFile(input.targetPath, 'utf8')); return { - backupPath, + ...(backupPath === undefined ? {} : { backupPath }), originalExisted: original.existed, originalMode: original.mode, }; @@ -65,7 +62,7 @@ export async function backupAndAtomicWrite(input: { if (replaced) { try { await restoreOriginalFile({ - backupPath, + ...(backupPath === undefined ? {} : { backupPath }), targetPath: input.targetPath, originalExisted: original.existed, originalMode: original.mode, @@ -85,7 +82,7 @@ export async function backupAndAtomicWrite(input: { } export async function restoreOriginalFile(input: { - readonly backupPath: string; + readonly backupPath?: string; readonly targetPath: string; readonly originalExisted: boolean; readonly originalMode: number; @@ -96,6 +93,8 @@ export async function restoreOriginalFile(input: { }); return; } + if (input.backupPath === undefined) + throw new SetupStorageError(`No backup is available to restore ${input.targetPath}.`); await restoreFile(input.backupPath, input.targetPath, input.originalMode); } diff --git a/src/distribution/setup/clients/claude-json-adapter.ts b/src/distribution/setup/clients/claude-json-adapter.ts index a05e564..e6a5612 100644 --- a/src/distribution/setup/clients/claude-json-adapter.ts +++ b/src/distribution/setup/clients/claude-json-adapter.ts @@ -2,10 +2,9 @@ import { modify, parse, type ParseError } from 'jsonc-parser'; import type { ClientConfigAdapter, ClientEntryState } from './client-adapter.js'; import type { MutableIntegrationClient } from '../setup-types.js'; import { renderIntegrationSnippet } from '../snippets.js'; +import { RELAY_ENTRY, RELAY_ENTRY_ID } from '../relay-entry.js'; import { SetupUsageError } from '../setup-errors.js'; -const relayEntry = { command: 'relay', args: ['mcp'] } as const; - export function createClaudeJsonAdapter(): ClientConfigAdapter { return { client: 'claude-code', @@ -14,7 +13,7 @@ export function createClaudeJsonAdapter(): ClientConfigAdapter { upsertRelayEntry: (content) => { const document = readDocument(content); if (inspect(document).kind === 'matching') return content; - const edits = modify(content, ['mcpServers', 'relay'], relayEntry, { + const edits = modify(content, ['mcpServers', RELAY_ENTRY_ID], RELAY_ENTRY, { formattingOptions: { insertSpaces: true, tabSize: 2, eol: newlineFor(content) }, }); return applyEdits(content, edits); @@ -58,7 +57,12 @@ function inspect(document: Record): ClientEntryState { ? relay.args : undefined; const keys = Object.keys(relay); - if (keys.length === 2 && command === 'relay' && args?.length === 1 && args[0] === 'mcp') + if ( + keys.length === 2 && + command === RELAY_ENTRY.command && + args?.length === RELAY_ENTRY.args.length && + args[0] === RELAY_ENTRY.args[0] + ) return { kind: 'matching', command, args }; return { kind: 'conflicting', diff --git a/src/distribution/setup/clients/codex-toml-adapter.ts b/src/distribution/setup/clients/codex-toml-adapter.ts index b3ba5f2..ff11db2 100644 --- a/src/distribution/setup/clients/codex-toml-adapter.ts +++ b/src/distribution/setup/clients/codex-toml-adapter.ts @@ -1,9 +1,13 @@ -import { parse as parseToml } from '@iarna/toml'; +import { load as parseToml } from 'js-toml'; import type { ClientConfigAdapter, ClientEntryState } from './client-adapter.js'; import { renderIntegrationSnippet } from '../snippets.js'; +import { RELAY_ARGS, RELAY_COMMAND, RELAY_ENTRY_ID } from '../relay-entry.js'; import { SetupConflictError, SetupUsageError } from '../setup-errors.js'; -const headerPattern = /^\s*\[mcp_servers\.relay\][^\r\n]*(?:\r?\n|$)/gm; +const headerPattern = new RegExp( + `^\\s*\\[mcp_servers\\.${RELAY_ENTRY_ID}\\][^\\r\\n]*(?:\\r?\\n|$)`, + 'gm', +); export function createCodexTomlAdapter(): ClientConfigAdapter { return { @@ -54,9 +58,9 @@ function inspect(content: string): ClientEntryState { const keys = Object.keys(servers.relay); if ( keys.length === 2 && - command === 'relay' && - args?.length === 1 && - args[0] === 'mcp' && + command === RELAY_COMMAND && + args?.length === RELAY_ARGS.length && + args[0] === RELAY_ARGS[0] && singleRelayHeader(content) !== undefined ) return { kind: 'matching', command, args }; diff --git a/src/distribution/setup/file-lock.ts b/src/distribution/setup/file-lock.ts new file mode 100644 index 0000000..6f42a04 --- /dev/null +++ b/src/distribution/setup/file-lock.ts @@ -0,0 +1,98 @@ +import { mkdir, open, unlink, type FileHandle } from 'node:fs/promises'; +import { dirname } from 'node:path'; +import { SetupConflictError, SetupStorageError } from './setup-errors.js'; + +export const SETUP_LOCK_RETRY_DELAY_MS = 25; +export const SETUP_LOCK_MAX_ATTEMPTS = 40; + +export interface ExclusiveFileLockOptions { + readonly retryDelayMs?: number; + readonly maxAttempts?: number; + readonly sleep?: (milliseconds: number) => Promise; +} + +export async function withExclusiveFileLock( + lockPath: string, + action: () => Promise, + options: ExclusiveFileLockOptions = {}, +): Promise { + try { + await mkdir(dirname(lockPath), { recursive: true }); + } catch (error) { + throw new SetupStorageError( + `Setup lock directory could not be prepared at ${lockPath}.`, + error, + ); + } + + const retryDelayMs = options.retryDelayMs ?? SETUP_LOCK_RETRY_DELAY_MS; + const maxAttempts = options.maxAttempts ?? SETUP_LOCK_MAX_ATTEMPTS; + const sleep = options.sleep ?? delay; + let handle: FileHandle | undefined; + let attempts = 0; + while (handle === undefined) { + try { + handle = await open(lockPath, 'wx', 0o600); + } catch (error) { + if (!isExists(error)) + throw new SetupStorageError(`Setup lock could not be opened at ${lockPath}.`, error); + attempts += 1; + if (attempts >= maxAttempts) + throw new SetupConflictError( + `Another Relay configuration operation is in progress. Retry after it completes. Lock: ${lockPath}`, + ); + await sleep(retryDelayMs); + } + } + + let result: T | undefined; + let actionFailed = false; + let actionError: unknown; + try { + try { + await handle.writeFile( + `${JSON.stringify({ pid: process.pid, acquiredAt: new Date().toISOString() })}\n`, + 'utf8', + ); + await handle.sync(); + } catch (error) { + throw new SetupStorageError(`Setup lock could not be written at ${lockPath}.`, error); + } + result = await action(); + } catch (error) { + actionFailed = true; + actionError = error; + } + + let cleanupError: unknown; + try { + await handle.close(); + } catch (error) { + cleanupError = error; + } + try { + await unlink(lockPath); + } catch (error) { + if (!isMissing(error)) cleanupError ??= error; + } + if (cleanupError !== undefined) + throw new SetupStorageError(`Setup lock could not be released at ${lockPath}.`, cleanupError); + if (actionFailed) throw actionError; + return result as T; +} + +async function delay(milliseconds: number): Promise { + await new Promise((resolveDelay) => setTimeout(resolveDelay, milliseconds)); +} + +function isExists(error: unknown): boolean { + return isRecord(error) && error.code === 'EEXIST'; +} + +function isMissing(error: unknown): boolean { + return isRecord(error) && error.code === 'ENOENT'; +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null; +} diff --git a/src/distribution/setup/initialize-relay.ts b/src/distribution/setup/initialize-relay.ts index 7c0e3c5..b1263af 100644 --- a/src/distribution/setup/initialize-relay.ts +++ b/src/distribution/setup/initialize-relay.ts @@ -25,7 +25,7 @@ export async function initializeRelay( dependencies.runtimePaths.configRoot, ]) { const created = await dependencies.mkdir(directory, { recursive: true }); - if (created !== undefined) createdDirectories.push(created); + if (created !== undefined) createdDirectories.push(directory); } } catch (error) { throw new SetupStorageError('Relay directories could not be initialized.', error); diff --git a/src/distribution/setup/ownership-store.ts b/src/distribution/setup/ownership-store.ts index 888e24a..14a11d0 100644 --- a/src/distribution/setup/ownership-store.ts +++ b/src/distribution/setup/ownership-store.ts @@ -1,16 +1,17 @@ import { randomUUID } from 'node:crypto'; -import { mkdir, open, readFile, unlink, writeFile, type FileHandle } from 'node:fs/promises'; +import { mkdir, readFile, unlink, writeFile } from 'node:fs/promises'; import { dirname, normalize, resolve } from 'node:path'; import { replaceFile } from './backup-and-atomic-write.js'; +import { withExclusiveFileLock } from './file-lock.js'; +import { RELAY_ARGS, RELAY_COMMAND, RELAY_ENTRY_ID } from './relay-entry.js'; import type { RelayIntegrationOwnership, RelayOwnershipFile } from './setup-types.js'; -import { SetupConflictError, SetupStorageError } from './setup-errors.js'; - -const OWNERSHIP_LOCK_RETRY_DELAY_MS = 25; -const OWNERSHIP_LOCK_MAX_ATTEMPTS = 40; +import { SetupStorageError } from './setup-errors.js'; export interface OwnershipStore { read(): Promise; - update(mutate: (current: RelayOwnershipFile) => RelayOwnershipFile): Promise; + update( + mutate: (current: RelayOwnershipFile) => RelayOwnershipFile | Promise, + ): Promise; } export function createOwnershipStore(input: { @@ -36,9 +37,7 @@ export function createOwnershipStore(input: { } catch (error) { throw new SetupStorageError( `Relay ownership metadata schema is invalid at ${input.metadataPath}.`, - { - cause: error, - }, + error, ); } }; @@ -68,21 +67,17 @@ export function createOwnershipStore(input: { return { read: readOwnership, update: async (mutate) => { - return withOwnershipLock( - input.metadataPath, + return withExclusiveFileLock( + `${input.metadataPath}.relay-lock`, async () => { const current = await readOwnership(); - const next = validateOwnership(mutate(current), input.applicationVersion); + const next = validateOwnership(await mutate(current), input.applicationVersion); await writeValidatedOwnership(next); return next; }, { - ...(input.lockRetryDelayMs === undefined - ? {} - : { lockRetryDelayMs: input.lockRetryDelayMs }), - ...(input.lockMaxAttempts === undefined - ? {} - : { lockMaxAttempts: input.lockMaxAttempts }), + ...(input.lockRetryDelayMs === undefined ? {} : { retryDelayMs: input.lockRetryDelayMs }), + ...(input.lockMaxAttempts === undefined ? {} : { maxAttempts: input.lockMaxAttempts }), ...(input.sleep === undefined ? {} : { sleep: input.sleep }), }, ); @@ -90,94 +85,6 @@ export function createOwnershipStore(input: { }; } -async function withOwnershipLock( - metadataPath: string, - action: () => Promise, - options: { - readonly lockRetryDelayMs?: number; - readonly lockMaxAttempts?: number; - readonly sleep?: (milliseconds: number) => Promise; - } = {}, -): Promise { - const lockPath = `${metadataPath}.relay-lock`; - const retryDelayMs = options.lockRetryDelayMs ?? OWNERSHIP_LOCK_RETRY_DELAY_MS; - const maxAttempts = options.lockMaxAttempts ?? OWNERSHIP_LOCK_MAX_ATTEMPTS; - const sleep = options.sleep ?? ((milliseconds: number) => delay(milliseconds)); - try { - await mkdir(dirname(metadataPath), { recursive: true }); - } catch (error) { - throw new SetupStorageError( - `Relay ownership metadata could not be prepared at ${metadataPath}.`, - error, - ); - } - - let handle: FileHandle | undefined; - let attempts = 0; - while (handle === undefined) { - try { - handle = await open(lockPath, 'wx', 0o600); - } catch (error) { - if (!isExists(error)) - throw new SetupStorageError( - `Relay ownership lock could not be opened at ${lockPath}.`, - error, - ); - attempts += 1; - if (attempts >= maxAttempts) - throw new SetupConflictError( - `Another Relay configuration operation is in progress for ${metadataPath}. Retry after it completes.`, - ); - await sleep(retryDelayMs); - } - } - - let result: T | undefined; - let actionFailed = false; - let actionError: unknown; - try { - try { - await handle.writeFile( - `${JSON.stringify({ pid: process.pid, acquiredAt: new Date().toISOString() })}\n`, - 'utf8', - ); - await handle.sync(); - } catch (error) { - throw new SetupStorageError( - `Relay ownership lock could not be written at ${lockPath}.`, - error, - ); - } - result = await action(); - } catch (error) { - actionFailed = true; - actionError = error; - } - - let cleanupError: unknown; - try { - await handle.close(); - } catch (error) { - cleanupError = error; - } - try { - await unlink(lockPath); - } catch (error) { - if (!isMissingFile(error)) cleanupError ??= error; - } - if (cleanupError !== undefined) - throw new SetupStorageError( - `Relay ownership lock could not be released at ${lockPath}.`, - cleanupError, - ); - if (actionFailed) throw actionError; - return result as T; -} - -async function delay(milliseconds: number): Promise { - await new Promise((resolveDelay) => setTimeout(resolveDelay, milliseconds)); -} - function validateOwnership(value: unknown, _applicationVersion: string): RelayOwnershipFile { if (!isRecord(value) || value.schemaVersion !== 1 || !Array.isArray(value.integrations)) { throw new Error('Relay ownership metadata has an unsupported schema.'); @@ -201,11 +108,11 @@ function validateRecord(value: unknown): RelayIntegrationOwnership { if (!isRecord(value)) throw new Error('Relay ownership record is invalid.'); if ( (value.client !== 'codex' && value.client !== 'claude-code') || - value.entryId !== 'relay' || - value.command !== 'relay' || + value.entryId !== RELAY_ENTRY_ID || + value.command !== RELAY_COMMAND || !Array.isArray(value.args) || value.args.length !== 1 || - value.args[0] !== 'mcp' || + value.args[0] !== RELAY_ARGS[0] || (value.status !== 'enabled' && value.status !== 'disabled') || typeof value.applicationVersion !== 'string' || typeof value.lastSuccessfulSetupAt !== 'string' || @@ -217,9 +124,9 @@ function validateRecord(value: unknown): RelayIntegrationOwnership { return { client: value.client, configPath: normalize(resolve(value.configPath)), - entryId: 'relay', - command: 'relay', - args: ['mcp'], + entryId: RELAY_ENTRY_ID, + command: RELAY_COMMAND, + args: RELAY_ARGS, status: value.status, applicationVersion: value.applicationVersion, lastSuccessfulSetupAt: value.lastSuccessfulSetupAt, @@ -243,7 +150,3 @@ function isRecord(value: unknown): value is Record { function isMissingFile(error: unknown): boolean { return isRecord(error) && error.code === 'ENOENT'; } - -function isExists(error: unknown): boolean { - return isRecord(error) && error.code === 'EEXIST'; -} diff --git a/src/distribution/setup/plan-integration-change.ts b/src/distribution/setup/plan-integration-change.ts index e8d27db..a935239 100644 --- a/src/distribution/setup/plan-integration-change.ts +++ b/src/distribution/setup/plan-integration-change.ts @@ -13,6 +13,7 @@ import type { MutableIntegrationClient, RelayOwnershipFile, } from './setup-types.js'; +import { RELAY_ENTRY_ID } from './relay-entry.js'; export async function planIntegrationChange(input: { readonly action: 'setup' | 'disable' | 'remove'; @@ -72,7 +73,7 @@ export async function planIntegrationChange(input: { return { client: input.client, configPath, - entryId: 'relay', + entryId: RELAY_ENTRY_ID, operation, changed: nextContent !== content || diff --git a/src/distribution/setup/relay-entry.ts b/src/distribution/setup/relay-entry.ts new file mode 100644 index 0000000..fe6eae7 --- /dev/null +++ b/src/distribution/setup/relay-entry.ts @@ -0,0 +1,4 @@ +export const RELAY_ENTRY_ID = 'relay' as const; +export const RELAY_COMMAND = 'relay' as const; +export const RELAY_ARGS = ['mcp'] as const; +export const RELAY_ENTRY = { command: RELAY_COMMAND, args: RELAY_ARGS } as const; diff --git a/src/distribution/setup/snippets.ts b/src/distribution/setup/snippets.ts index d7c6fb0..5ca6616 100644 --- a/src/distribution/setup/snippets.ts +++ b/src/distribution/setup/snippets.ts @@ -1,13 +1,12 @@ import type { IntegrationClient } from './setup-types.js'; - -const installedServer = { command: 'relay', args: ['mcp'] as const }; +import { RELAY_ENTRY } from './relay-entry.js'; export function renderIntegrationSnippet(client: IntegrationClient): string { if (client === 'codex') { return '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n'; } if (client === 'claude-code') { - return `${JSON.stringify({ mcpServers: { relay: installedServer } }, null, 2)}\n`; + return `${JSON.stringify({ mcpServers: { relay: RELAY_ENTRY } }, null, 2)}\n`; } - return `${JSON.stringify(installedServer, null, 2)}\n`; + return `${JSON.stringify(RELAY_ENTRY, null, 2)}\n`; } diff --git a/src/interfaces/cli/operational-output.ts b/src/interfaces/cli/operational-output.ts index ad56c4f..9cb03da 100644 --- a/src/interfaces/cli/operational-output.ts +++ b/src/interfaces/cli/operational-output.ts @@ -21,10 +21,22 @@ export function writeOperationalSuccess( export function writeOperationalError(stdout: Writer, stderr: Writer, error: unknown): number { const mapped = mapOperationalError(error); stdout.write(`${JSON.stringify(cliFailure(mapped.code, mapped.message))}\n`); - stderr.write(`${mapped.message}\n`); + stderr.write( + `${mapped.code === 'INTERNAL_ERROR' ? formatInternalError(error) : mapped.message}\n`, + ); return mapped.exitCode; } +function formatInternalError(error: unknown): string { + if (!(error instanceof Error)) + return `An unexpected internal error occurred. Details: ${String(error)}`; + const detail = error.stack ?? error.message; + if (error.cause === undefined) return detail; + const cause = + error.cause instanceof Error ? (error.cause.stack ?? error.cause.message) : String(error.cause); + return `${detail}\nCaused by: ${cause}`; +} + function mapOperationalError(error: unknown): { code: string; message: string; exitCode: number } { if (error instanceof SetupNotFoundError) return { code: 'NOT_FOUND', message: error.message, exitCode: 3 }; @@ -35,7 +47,7 @@ function mapOperationalError(error: unknown): { code: string; message: string; e if ( error instanceof CliUsageError || error instanceof SetupUsageError || - error instanceof RelayError + (error instanceof RelayError && error.constructor !== RelayError) ) return { code: 'VALIDATION_ERROR', message: error.message, exitCode: 2 }; return { code: 'INTERNAL_ERROR', message: 'An unexpected internal error occurred.', exitCode: 1 }; diff --git a/src/interfaces/cli/run-operational-command.ts b/src/interfaces/cli/run-operational-command.ts index 122f5c5..b5cbec2 100644 --- a/src/interfaces/cli/run-operational-command.ts +++ b/src/interfaces/cli/run-operational-command.ts @@ -1,5 +1,4 @@ import { mkdir } from 'node:fs/promises'; -import { join } from 'node:path'; import type { RuntimePaths } from '../../distribution/resolve-runtime-paths.js'; import { initializeRelay } from '../../distribution/setup/initialize-relay.js'; import { @@ -12,8 +11,10 @@ import { createCodexTomlAdapter } from '../../distribution/setup/clients/codex-t import { planIntegrationChange } from '../../distribution/setup/plan-integration-change.js'; import { renderIntegrationSnippet } from '../../distribution/setup/snippets.js'; import type { MutableIntegrationClient } from '../../distribution/setup/setup-types.js'; +import { CliUsageError } from './output/cli-errors.js'; import { parseOperationalCommand, type OperationalCommand } from './parse-operational-command.js'; import { writeOperationalError, writeOperationalSuccess } from './operational-output.js'; +import { resolveOwnershipMetadataPath } from '../production-dependencies.js'; export interface OperationalDependencies { readonly runtimePaths: RuntimePaths; @@ -39,7 +40,7 @@ export async function runOperationalCommand( const store = dependencies.ownershipStore ?? createOwnershipStore({ - metadataPath: join(dependencies.runtimePaths.configRoot, 'config.json'), + metadataPath: resolveOwnershipMetadataPath(dependencies.runtimePaths), applicationVersion: dependencies.applicationVersion, }); const needsInitialization = @@ -65,7 +66,7 @@ export async function runOperationalCommand( writeOperationalSuccess(dependencies.stdout, 'config paths', { paths: { ...dependencies.runtimePaths, - metadataPath: join(dependencies.runtimePaths.configRoot, 'config.json'), + metadataPath: resolveOwnershipMetadataPath(dependencies.runtimePaths), }, }); return 0; @@ -94,8 +95,10 @@ export async function runOperationalCommand( }); return 0; } - const client = command.client as MutableIntegrationClient; - const configPath = command.configFile!; + if (!isMutableOperationalCommand(command)) + throw new CliUsageError('This command does not target a mutable client configuration.'); + const client: MutableIntegrationClient = command.client; + const configPath = command.configFile; const adapter = client === 'codex' ? createCodexTomlAdapter() : createClaudeJsonAdapter(); const ownership = await store.read(); const action = @@ -136,3 +139,19 @@ export async function runOperationalCommand( return writeOperationalError(dependencies.stdout, dependencies.stderr, error); } } + +function isMutableOperationalCommand(command: OperationalCommand): command is + | Extract + | (Extract & { + readonly client: MutableIntegrationClient; + readonly configFile: string; + }) { + return ( + (command.kind === 'setup' || + command.kind === 'config-disable' || + command.kind === 'config-remove') && + command.client !== undefined && + command.client !== 'generic-mcp' && + command.configFile !== undefined + ); +} diff --git a/src/interfaces/production-dependencies.ts b/src/interfaces/production-dependencies.ts index d0e3673..bec2c5b 100644 --- a/src/interfaces/production-dependencies.ts +++ b/src/interfaces/production-dependencies.ts @@ -1,7 +1,7 @@ import { runMcpServer } from './mcp/main.js'; import { runUiServer } from './http/main.js'; import { join } from 'node:path'; -import { resolveRuntimePaths } from '../distribution/resolve-runtime-paths.js'; +import { resolveRuntimePaths, type RuntimePaths } from '../distribution/resolve-runtime-paths.js'; import { readPackageVersion } from '../distribution/package-version.js'; import { createTaskRuntime } from './shared/create-task-runtime.js'; import { createOwnershipStore } from '../distribution/setup/ownership-store.js'; @@ -9,6 +9,10 @@ import type { OperationalDependencies } from './cli/run-operational-command.js'; export { runMcpServer, runUiServer }; +export function resolveOwnershipMetadataPath(runtimePaths: RuntimePaths): string { + return join(runtimePaths.configRoot, 'config.json'); +} + export function createOperationalDependencies(output: { stdout: { write(text: string): unknown }; stderr: { write(text: string): unknown }; @@ -20,7 +24,7 @@ export function createOperationalDependencies(output: { applicationVersion, openRuntime: (databasePath) => createTaskRuntime({ databasePath }), ownershipStore: createOwnershipStore({ - metadataPath: join(runtimePaths.configRoot, 'config.json'), + metadataPath: resolveOwnershipMetadataPath(runtimePaths), applicationVersion, }), stdout: output.stdout, diff --git a/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/README.md b/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/README.md index 3c69289..e5b522a 100644 --- a/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/README.md +++ b/tests/fixtures/agent-integrations/valid/integrations/generic-mcp/README.md @@ -1 +1 @@ -skills/relay-capture/SKILL.md skills/relay-session-review/SKILL.md Validation RELAY_DB_PATH must be explicit and isolated; omission is permitted only for non-validation use. Remove only the client configuration; the SQLite database remains untouched. relay_health task_capture task_list task_get task_find_similar session_captures_list SQLite database remains untouched. +skills/relay-capture/SKILL.md skills/relay-session-review/SKILL.md Validation RELAY_DB_PATH must be a non-empty absolute path in an isolated location; omission is permitted only for non-validation use. Remove only the client configuration; the SQLite database remains untouched. relay_health task_capture task_list task_get task_find_similar session_captures_list SQLite database remains untouched. diff --git a/tests/fixtures/setup/claude-code/no-mcp-servers.json b/tests/fixtures/setup/claude-code/no-mcp-servers.json new file mode 100644 index 0000000..d2537df --- /dev/null +++ b/tests/fixtures/setup/claude-code/no-mcp-servers.json @@ -0,0 +1,3 @@ +{ + "profile": "empty" +} diff --git a/tests/integration/cli.test.ts b/tests/integration/cli.test.ts index 826959e..7827638 100644 --- a/tests/integration/cli.test.ts +++ b/tests/integration/cli.test.ts @@ -152,7 +152,7 @@ describe('built CLI', () => { } finally { rmSync(workspace, { recursive: true, force: true }); } - }); + }, 15_000); it('validates before runtime creation and maps an isolated storage failure', () => { const workspace = mkdtempSync(join(tmpdir(), 'relay-cli-validation-')); diff --git a/tests/integration/setup-workflow.test.ts b/tests/integration/setup-workflow.test.ts index c276ea8..b96db8a 100644 --- a/tests/integration/setup-workflow.test.ts +++ b/tests/integration/setup-workflow.test.ts @@ -72,7 +72,7 @@ describe('installed setup workflow', () => { ).toBe(0); expect(existsSync(databasePath)).toBe(true); expect(run('task', 'get', taskId!, '--output', 'json').stdout).toContain(taskId!); - }); + }, 15_000); it('returns a JSON storage error when the database path is unusable', () => { const root = mkdtempSync(join(tmpdir(), 'relay-setup-error-')); diff --git a/tests/unit/contracts/distribution-contract.test.ts b/tests/unit/contracts/distribution-contract.test.ts index 2d1bd39..c1cdeaa 100644 --- a/tests/unit/contracts/distribution-contract.test.ts +++ b/tests/unit/contracts/distribution-contract.test.ts @@ -1,7 +1,7 @@ import { readFileSync } from 'node:fs'; import { homedir } from 'node:os'; import { resolve } from 'node:path'; -import { parse as parseToml } from '@iarna/toml'; +import { load as parseToml } from 'js-toml'; import { describe, expect, it } from 'vitest'; import { z } from 'zod'; diff --git a/tests/unit/distribution/setup/apply-integration-change.test.ts b/tests/unit/distribution/setup/apply-integration-change.test.ts index 55fffb2..7a5ec3b 100644 --- a/tests/unit/distribution/setup/apply-integration-change.test.ts +++ b/tests/unit/distribution/setup/apply-integration-change.test.ts @@ -4,6 +4,7 @@ import { readFileSync, readdirSync, statSync, + unlinkSync, writeFileSync, } from 'node:fs'; import { tmpdir } from 'node:os'; @@ -15,7 +16,10 @@ import { applyIntegrationChange } from '../../../../src/distribution/setup/apply import { planIntegrationChange } from '../../../../src/distribution/setup/plan-integration-change.js'; import { createOwnershipStore } from '../../../../src/distribution/setup/ownership-store.js'; import type { OwnershipStore } from '../../../../src/distribution/setup/ownership-store.js'; -import { SetupStorageError } from '../../../../src/distribution/setup/setup-errors.js'; +import { + SetupConflictError, + SetupStorageError, +} from '../../../../src/distribution/setup/setup-errors.js'; describe('applyIntegrationChange', () => { it('updates the client before writing enabled ownership metadata', async () => { @@ -142,7 +146,7 @@ describe('applyIntegrationChange', () => { }); }); - it('restores an absent config after metadata persistence fails and retains the backup', async () => { + it('restores an absent config after metadata persistence fails without a backup', async () => { const root = mkdtempSync(join(tmpdir(), 'relay-apply-')); const path = join(root, 'codex.toml'); const adapter = createCodexTomlAdapter(); @@ -170,7 +174,7 @@ describe('applyIntegrationChange', () => { }), ).rejects.toThrow(/restored after metadata persistence failed/i); expect(existsSync(path)).toBe(false); - expect(readdirSync(root).some((name) => name.includes('.relay-backup-'))).toBe(true); + expect(readdirSync(root).some((name) => name.includes('.relay-backup-'))).toBe(false); }); it('restores existing config bytes and mode after metadata persistence fails', async () => { @@ -206,4 +210,130 @@ describe('applyIntegrationChange', () => { expect(readFileSync(path, 'utf8')).toBe(original); expect(statSync(path).mode & 0o777).toBe(originalMode); }); + + it('rejects a no-rewrite plan when the configuration changes before metadata persistence', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-apply-')); + const path = join(root, 'codex.toml'); + const metadataPath = join(root, 'config.json'); + const content = '[profile]\nname = "unrelated"\n'; + writeFileSync(path, content); + const adapter = createCodexTomlAdapter(); + const seedStore = createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }); + await seedStore.update(() => ({ + schemaVersion: 1, + integrations: [ + { + client: 'codex', + configPath: path, + entryId: 'relay', + command: 'relay', + args: ['mcp'], + status: 'disabled', + applicationVersion: '0.1.0', + lastSuccessfulSetupAt: '2026-08-02T01:02:03.004Z', + }, + ], + })); + const plan = await planIntegrationChange({ + action: 'remove', + client: 'codex', + configPath: path, + adapter, + ownership: await seedStore.read(), + }); + const ownershipStore: OwnershipStore = { + read: () => seedStore.read(), + update: async (mutate) => { + writeFileSync(path, 'changed externally'); + return mutate({ schemaVersion: 1, integrations: [] }); + }, + }; + + await expect( + applyIntegrationChange({ + plan, + adapter, + ownershipStore, + applicationVersion: '0.1.0', + now: new Date('2026-08-02T01:02:03.004Z'), + }), + ).rejects.toBeInstanceOf(SetupConflictError); + }); + + it('preserves metadata and restore failures together', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-apply-')); + const path = join(root, 'codex.toml'); + const original = '[profile]\nname = "existing"\n'; + writeFileSync(path, original); + const adapter = createCodexTomlAdapter(); + const plan = await planIntegrationChange({ + action: 'setup', + client: 'codex', + configPath: path, + adapter, + ownership: { schemaVersion: 1, integrations: [] }, + }); + const metadataError = new SetupStorageError('forced metadata failure'); + const ownershipStore: OwnershipStore = { + read: async () => ({ schemaVersion: 1, integrations: [] }), + update: async () => { + const backupName = readdirSync(root).find((name) => name.includes('.relay-backup-')); + if (backupName === undefined) throw new Error('Expected backup fixture.'); + unlinkSync(join(root, backupName)); + throw metadataError; + }, + }; + + const error = await applyIntegrationChange({ + plan, + adapter, + ownershipStore, + applicationVersion: '0.1.0', + now: new Date('2026-08-02T01:02:03.004Z'), + }).catch((caught: unknown) => caught); + expect(error).toBeInstanceOf(SetupStorageError); + expect((error as SetupStorageError).cause).toBeInstanceOf(AggregateError); + expect((error as SetupStorageError).cause).toMatchObject({ + errors: expect.arrayContaining([metadataError]), + }); + }); + + it('rejects an apply while the target configuration lock is held', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-apply-')); + const path = join(root, 'codex.toml'); + writeFileSync(path, ''); + const adapter = createCodexTomlAdapter(); + const plan = await planIntegrationChange({ + action: 'setup', + client: 'codex', + configPath: path, + adapter, + ownership: { schemaVersion: 1, integrations: [] }, + }); + const lockPath = `${path}.relay-lock`; + writeFileSync(lockPath, 'held'); + + try { + await expect( + applyIntegrationChange({ + plan, + adapter, + ownershipStore: createOwnershipStore({ + metadataPath: join(root, 'config.json'), + applicationVersion: '0.1.0', + lockRetryDelayMs: 0, + lockMaxAttempts: 2, + sleep: async () => undefined, + }), + applicationVersion: '0.1.0', + now: new Date('2026-08-02T01:02:03.004Z'), + }), + ).rejects.toMatchObject({ + constructor: SetupConflictError, + message: expect.stringMatching(/relay-lock/i), + }); + } finally { + unlinkSync(lockPath); + } + }); }); diff --git a/tests/unit/distribution/setup/backup-and-atomic-write.test.ts b/tests/unit/distribution/setup/backup-and-atomic-write.test.ts index 2749954..5741fe3 100644 --- a/tests/unit/distribution/setup/backup-and-atomic-write.test.ts +++ b/tests/unit/distribution/setup/backup-and-atomic-write.test.ts @@ -1,4 +1,12 @@ -import { existsSync, readFileSync, mkdtempSync, writeFileSync } from 'node:fs'; +import { + chmodSync, + existsSync, + mkdtempSync, + readFileSync, + readdirSync, + statSync, + writeFileSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; @@ -21,7 +29,8 @@ describe('backupAndAtomicWrite', () => { validate: (content) => JSON.parse(content) as unknown, now: new Date('2026-08-02T01:02:03.004Z'), }); - expect(readFileSync(result.backupPath, 'utf8')).toBe(original); + expect(result.backupPath).toBeDefined(); + expect(readFileSync(result.backupPath!, 'utf8')).toBe(original); expect(readFileSync(path, 'utf8')).toContain('relay'); expect(result.backupPath).toContain('.relay-backup-20260802T010203.004Z'); }); @@ -56,10 +65,10 @@ describe('backupAndAtomicWrite', () => { now: new Date('2026-08-02T01:02:03.004Z'), }); expect(result.backupPath).toContain('.relay-backup-20260802T010203.004Z-1'); - expect(readFileSync(result.backupPath, 'utf8')).toBe(original); + expect(readFileSync(result.backupPath!, 'utf8')).toBe(original); }); - it('restores a missing original file to absence while retaining an empty backup', async () => { + it('restores a missing original file to absence without creating a backup', async () => { const root = mkdtempSync(join(tmpdir(), 'relay-write-')); const path = join(root, 'new-config.toml'); const result = await backupAndAtomicWrite({ @@ -71,10 +80,31 @@ describe('backupAndAtomicWrite', () => { }); expect(result.originalExisted).toBe(false); + expect(result.backupPath).toBeUndefined(); expect(existsSync(path)).toBe(true); await restoreOriginalFile({ ...result, targetPath: path }); expect(existsSync(path)).toBe(false); - expect(existsSync(result.backupPath)).toBe(true); - expect(readFileSync(result.backupPath)).toHaveLength(0); + expect(readdirSync(root).some((name) => name.includes('.relay-backup-'))).toBe(false); + }); + + it('restores an existing target with exact bytes and mode', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-write-')); + const path = join(root, 'existing-config.toml'); + const original = '[profile]\nname = "existing"\n'; + writeFileSync(path, original); + chmodSync(path, 0o640); + const originalMode = statSync(path).mode & 0o777; + const result = await backupAndAtomicWrite({ + targetPath: path, + expectedFingerprint: fingerprint(original), + nextContent: '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n', + validate: () => undefined, + now: new Date('2026-08-02T01:02:03.004Z'), + }); + + await restoreOriginalFile({ ...result, targetPath: path }); + + expect(readFileSync(path, 'utf8')).toBe(original); + expect(statSync(path).mode & 0o777).toBe(originalMode); }); }); diff --git a/tests/unit/distribution/setup/claude-json-adapter.test.ts b/tests/unit/distribution/setup/claude-json-adapter.test.ts index 7f94e56..0d7a15e 100644 --- a/tests/unit/distribution/setup/claude-json-adapter.test.ts +++ b/tests/unit/distribution/setup/claude-json-adapter.test.ts @@ -16,6 +16,9 @@ describe('Claude JSON adapter', () => { expect(adapter.removeRelayEntry(edited)).toContain('other-agent'); expect(adapter.inspect(adapter.removeRelayEntry(edited)).kind).toBe('absent'); }); + it('recognizes an absent entry when mcpServers is omitted', () => { + expect(adapter.inspect(fixture('no-mcp-servers.json')).kind).toBe('absent'); + }); it('fails closed for malformed and conflicting entries', () => { expect(() => adapter.parse(fixture('malformed.json'))).toThrow(/malformed/i); expect(adapter.inspect(fixture('conflicting.json')).kind).toBe('conflicting'); diff --git a/tests/unit/distribution/setup/initialize-relay.test.ts b/tests/unit/distribution/setup/initialize-relay.test.ts index 0620f91..27ace7e 100644 --- a/tests/unit/distribution/setup/initialize-relay.test.ts +++ b/tests/unit/distribution/setup/initialize-relay.test.ts @@ -29,7 +29,7 @@ describe('initializeRelay', () => { const textPath = path.toString(); mkdirSync(textPath, options); calls.push(textPath); - return textPath; + return join(root, 'shared'); }) as typeof mkdirFunction, openRuntime: (databasePath) => { calls.push(databasePath); diff --git a/tests/unit/distribution/setup/ownership-store.test.ts b/tests/unit/distribution/setup/ownership-store.test.ts index f84ada7..858d71a 100644 --- a/tests/unit/distribution/setup/ownership-store.test.ts +++ b/tests/unit/distribution/setup/ownership-store.test.ts @@ -3,7 +3,10 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; import { createOwnershipStore } from '../../../../src/distribution/setup/ownership-store.js'; -import { SetupConflictError } from '../../../../src/distribution/setup/setup-errors.js'; +import { + SetupConflictError, + SetupStorageError, +} from '../../../../src/distribution/setup/setup-errors.js'; describe('ownership store', () => { const roots: string[] = []; @@ -32,6 +35,21 @@ describe('ownership store', () => { ).rejects.toThrow(/schema/i); }); + it('preserves the metadata parse error as the storage cause', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-ownership-')); + roots.push(root); + const metadataPath = join(root, 'config.json'); + writeFileSync(metadataPath, '{'); + + try { + await createOwnershipStore({ metadataPath, applicationVersion: '0.1.0' }).read(); + throw new Error('Expected malformed metadata to fail.'); + } catch (error) { + expect(error).toBeInstanceOf(SetupStorageError); + expect((error as SetupStorageError).cause).toBeInstanceOf(SyntaxError); + } + }); + it('normalizes and sorts valid records on read and writes atomically', async () => { const root = mkdtempSync(join(tmpdir(), 'relay-ownership-')); roots.push(root); @@ -111,7 +129,7 @@ describe('ownership store', () => { try { await expect(store.update((current) => current)).rejects.toMatchObject({ constructor: SetupConflictError, - message: expect.stringMatching(/in progress.*retry/i), + message: expect.stringMatching(/in progress.*retry.*relay-lock/i), }); } finally { unlinkSync(lockPath); diff --git a/tests/unit/distribution/setup/plan-integration-change.test.ts b/tests/unit/distribution/setup/plan-integration-change.test.ts index c04ac28..7f35ca2 100644 --- a/tests/unit/distribution/setup/plan-integration-change.test.ts +++ b/tests/unit/distribution/setup/plan-integration-change.test.ts @@ -59,4 +59,116 @@ describe('planIntegrationChange', () => { }), ).rejects.toBeInstanceOf(SetupNotFoundError); }); + + it('plans a missing configuration file as created', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-plan-')); + const path = join(root, 'missing.toml'); + const plan = await planIntegrationChange({ + action: 'setup', + client: 'codex', + configPath: path, + adapter: createCodexTomlAdapter(), + ownership: { schemaVersion: 1, integrations: [] }, + }); + + expect(plan.operation).toBe('created'); + expect(plan.changed).toBe(true); + expect(plan.nextContent).toContain('[mcp_servers.relay]'); + }); + + it('plans disabling an enabled Relay entry', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-plan-')); + const path = join(root, 'codex.toml'); + const content = '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n'; + writeFileSync(path, content); + const plan = await planIntegrationChange({ + action: 'disable', + client: 'codex', + configPath: path, + adapter: createCodexTomlAdapter(), + ownership: { + schemaVersion: 1, + integrations: [ + { + client: 'codex', + configPath: path, + entryId: 'relay', + command: 'relay', + args: ['mcp'], + status: 'enabled', + applicationVersion: '0.1.0', + lastSuccessfulSetupAt: '2026-08-02T00:00:00.000Z', + }, + ], + }, + }); + + expect(plan.operation).toBe('disabled'); + expect(plan.changed).toBe(true); + expect(plan.nextContent).not.toContain('mcp_servers.relay'); + }); + + it('plans removing a disabled Relay entry without rewriting the client file', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-plan-')); + const path = join(root, 'codex.toml'); + const content = '[profile]\nname = "unrelated"\n'; + writeFileSync(path, content); + const plan = await planIntegrationChange({ + action: 'remove', + client: 'codex', + configPath: path, + adapter: createCodexTomlAdapter(), + ownership: { + schemaVersion: 1, + integrations: [ + { + client: 'codex', + configPath: path, + entryId: 'relay', + command: 'relay', + args: ['mcp'], + status: 'disabled', + applicationVersion: '0.1.0', + lastSuccessfulSetupAt: '2026-08-02T00:00:00.000Z', + }, + ], + }, + }); + + expect(plan.operation).toBe('removed'); + expect(plan.changed).toBe(true); + expect(plan.nextContent).toBe(content); + }); + + it('plans re-enabling a disabled matching Relay entry', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-plan-')); + const path = join(root, 'codex.toml'); + const content = '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n'; + writeFileSync(path, content); + const plan = await planIntegrationChange({ + action: 'setup', + client: 'codex', + configPath: path, + adapter: createCodexTomlAdapter(), + ownership: { + schemaVersion: 1, + integrations: [ + { + client: 'codex', + configPath: path, + entryId: 'relay', + command: 'relay', + args: ['mcp'], + status: 'disabled', + applicationVersion: '0.1.0', + lastSuccessfulSetupAt: '2026-08-02T00:00:00.000Z', + }, + ], + }, + }); + + expect(plan.operation).toBe('updated'); + expect(plan.changed).toBe(true); + expect(plan.nextContent).toBe(content); + }); }); diff --git a/tests/unit/distribution/setup/snippets.test.ts b/tests/unit/distribution/setup/snippets.test.ts index 60005a4..cda35b7 100644 --- a/tests/unit/distribution/setup/snippets.test.ts +++ b/tests/unit/distribution/setup/snippets.test.ts @@ -1,4 +1,4 @@ -import { parse as parseToml } from '@iarna/toml'; +import { load as parseToml } from 'js-toml'; import { describe, expect, it } from 'vitest'; import { renderIntegrationSnippet } from '../../../../src/distribution/setup/snippets.js'; diff --git a/tests/unit/interfaces/cli/operational-commands.test.ts b/tests/unit/interfaces/cli/operational-commands.test.ts index cf194af..5db5cfa 100644 --- a/tests/unit/interfaces/cli/operational-commands.test.ts +++ b/tests/unit/interfaces/cli/operational-commands.test.ts @@ -47,5 +47,37 @@ describe('parseOperationalCommand', () => { kind: 'config-snippet', client: 'generic-mcp', }); + expect( + parseOperationalCommand([ + 'config', + 'disable', + '--client', + 'codex', + '--config-file', + absoluteConfigPath, + '--apply', + ]), + ).toEqual({ + kind: 'config-disable', + client: 'codex', + configFile: absoluteConfigPath, + apply: true, + }); + expect( + parseOperationalCommand([ + 'config', + 'remove', + '--client', + 'codex', + '--config-file', + absoluteConfigPath, + '--apply', + ]), + ).toEqual({ + kind: 'config-remove', + client: 'codex', + configFile: absoluteConfigPath, + apply: true, + }); }); }); diff --git a/tests/unit/interfaces/cli/operational-output.test.ts b/tests/unit/interfaces/cli/operational-output.test.ts new file mode 100644 index 0000000..26c5732 --- /dev/null +++ b/tests/unit/interfaces/cli/operational-output.test.ts @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest'; +import { RelayError } from '../../../../src/shared/errors.js'; +import { writeOperationalError } from '../../../../src/interfaces/cli/operational-output.js'; + +describe('operational output errors', () => { + it('keeps base RelayError failures internal and writes details only to stderr', () => { + const stdout: string[] = []; + const stderr: string[] = []; + const code = writeOperationalError( + { write: (text) => stdout.push(text) }, + { write: (text) => stderr.push(text) }, + new RelayError('private implementation detail'), + ); + + expect(code).toBe(1); + expect(stdout[0]).toContain('"code":"INTERNAL_ERROR"'); + expect(stdout[0]).not.toContain('private implementation detail'); + expect(stderr.join('')).toContain('private implementation detail'); + }); +}); diff --git a/tests/unit/interfaces/cli/run-operational-command.test.ts b/tests/unit/interfaces/cli/run-operational-command.test.ts index 41f11d4..588bcf0 100644 --- a/tests/unit/interfaces/cli/run-operational-command.test.ts +++ b/tests/unit/interfaces/cli/run-operational-command.test.ts @@ -3,6 +3,26 @@ import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; import { runOperationalCommand } from '../../../../src/interfaces/cli/run-operational-command.js'; +import type { OperationalDependencies } from '../../../../src/interfaces/cli/run-operational-command.js'; + +function createDependencies(root: string, output: string[]): OperationalDependencies { + return { + runtimePaths: { + dataRoot: join(root, 'data'), + configRoot: join(root, 'config'), + cacheRoot: join(root, 'cache'), + databasePath: join(root, 'data', 'relay.db'), + }, + openRuntime: () => ({ close: () => undefined }), + applicationVersion: '0.1.0', + stdout: { + write: (text) => { + output.push(text); + }, + }, + stderr: { write: () => undefined }, + }; +} describe('runOperationalCommand', () => { const roots: string[] = []; @@ -17,25 +37,11 @@ describe('runOperationalCommand', () => { const configPath = join(root, 'codex.toml'); const code = await runOperationalCommand( ['setup', '--client', 'codex', '--config-file', configPath], - { - runtimePaths: { - dataRoot: join(root, 'data'), - configRoot: join(root, 'config'), - cacheRoot: join(root, 'cache'), - databasePath: join(root, 'data', 'relay.db'), - }, - openRuntime: () => ({ close: () => undefined }), - applicationVersion: '0.1.0', - stdout: { - write: (text) => { - output.push(text); - }, - }, - stderr: { write: () => undefined }, - }, + createDependencies(root, output), ); expect(code).toBe(0); expect(existsSync(join(root, 'config', 'config.json'))).toBe(false); + expect(output).toHaveLength(1); expect(JSON.parse(output[0] ?? '{}').data.snippet).toContain('command = "relay"'); }); @@ -43,12 +49,24 @@ describe('runOperationalCommand', () => { const root = mkdtempSync(join(tmpdir(), 'relay-operational-')); roots.push(root); const output: string[] = []; - const code = await runOperationalCommand(['config', 'paths'], { + const code = await runOperationalCommand(['config', 'paths'], createDependencies(root, output)); + expect(code).toBe(0); + expect(output).toHaveLength(1); + expect(JSON.parse(output[0] ?? '{}').data.paths.metadataPath).toContain('config.json'); + }); + + it('reports both requested roots when their shared parent is created', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-operational-')); + roots.push(root); + const output: string[] = []; + const dataRoot = join(root, 'shared', 'data'); + const configRoot = join(root, 'shared', 'config'); + const code = await runOperationalCommand(['setup'], { runtimePaths: { - dataRoot: join(root, 'data'), - configRoot: join(root, 'config'), + dataRoot, + configRoot, cacheRoot: join(root, 'cache'), - databasePath: join(root, 'data', 'relay.db'), + databasePath: join(dataRoot, 'relay.db'), }, openRuntime: () => ({ close: () => undefined }), applicationVersion: '0.1.0', @@ -59,7 +77,8 @@ describe('runOperationalCommand', () => { }, stderr: { write: () => undefined }, }); + expect(code).toBe(0); - expect(JSON.parse(output[0] ?? '{}').data.paths.metadataPath).toContain('config.json'); + expect(JSON.parse(output[0] ?? '{}').data.createdDirectories).toEqual([dataRoot, configRoot]); }); }); diff --git a/tests/unit/scripts/validate-agent-integration-assets.test.ts b/tests/unit/scripts/validate-agent-integration-assets.test.ts index 5e11b44..39a920a 100644 --- a/tests/unit/scripts/validate-agent-integration-assets.test.ts +++ b/tests/unit/scripts/validate-agent-integration-assets.test.ts @@ -123,7 +123,7 @@ describe('validateAgentIntegrationAssets', () => { writeFileSync( path, readFileSync(path, 'utf8').replace( - 'Validation RELAY_DB_PATH must be explicit and isolated; omission is permitted only for non-validation use.', + 'Validation RELAY_DB_PATH must be a non-empty absolute path in an isolated location; omission is permitted only for non-validation use.', 'The database is available.', ), ); From ece4b3b74a5d1877280b9af06794366ab3c22fa2 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 15:16:44 +0530 Subject: [PATCH 09/11] fix: recover interrupted integration transactions --- .../setup/apply-integration-change.ts | 105 +++++- .../setup/backup-and-atomic-write.ts | 13 +- src/distribution/setup/file-lock.ts | 23 +- .../setup/integration-transaction-journal.ts | 248 +++++++++++++ src/interfaces/cli/run-operational-command.ts | 67 +++- tests/integration/setup-workflow.test.ts | 144 +++++++- .../setup/apply-integration-change.test.ts | 10 +- .../integration-transaction-journal.test.ts | 337 ++++++++++++++++++ .../setup/ownership-store.test.ts | 2 +- .../cli/run-operational-command.test.ts | 37 +- 10 files changed, 935 insertions(+), 51 deletions(-) create mode 100644 src/distribution/setup/integration-transaction-journal.ts create mode 100644 tests/unit/distribution/setup/integration-transaction-journal.test.ts diff --git a/src/distribution/setup/apply-integration-change.ts b/src/distribution/setup/apply-integration-change.ts index 641518a..a7d4ae8 100644 --- a/src/distribution/setup/apply-integration-change.ts +++ b/src/distribution/setup/apply-integration-change.ts @@ -1,11 +1,16 @@ +import { readFile, stat } from 'node:fs/promises'; +import { normalize, resolve } from 'node:path'; import type { ClientConfigAdapter } from './clients/client-adapter.js'; import { backupAndAtomicWrite, restoreOriginalFile, type BackupAndAtomicWriteResult, } from './backup-and-atomic-write.js'; -import { readFile, stat } from 'node:fs/promises'; -import { normalize, resolve } from 'node:path'; +import { + deleteIntegrationTransactionJournal, + writeIntegrationTransactionJournal, + type RelayIntegrationTransactionJournal, +} from './integration-transaction-journal.js'; import { withExclusiveFileLock } from './file-lock.js'; import { RELAY_ARGS, RELAY_COMMAND, RELAY_ENTRY_ID } from './relay-entry.js'; import { fingerprint } from './plan-integration-change.js'; @@ -19,6 +24,10 @@ export async function applyIntegrationChange(input: { readonly ownershipStore: OwnershipStore; readonly applicationVersion: string; readonly now: Date; + readonly clientLockHeld?: boolean; + readonly lockRetryDelayMs?: number; + readonly lockMaxAttempts?: number; + readonly sleep?: (milliseconds: number) => Promise; }): Promise { if (!input.plan.changed) { return { @@ -29,9 +38,12 @@ export async function applyIntegrationChange(input: { changed: false, }; } - return withExclusiveFileLock(`${input.plan.configPath}.relay-lock`, async () => { + const journalPath = `${input.plan.configPath}.relay-transaction.json`; + const action = async (): Promise => { const clientChanged = fingerprint(input.plan.nextContent) !== input.plan.beforeFingerprint; let backup: BackupAndAtomicWriteResult | undefined; + let journal: RelayIntegrationTransactionJournal | undefined; + let ownershipPersisted = false; try { if (clientChanged) { backup = await backupAndAtomicWrite({ @@ -40,9 +52,24 @@ export async function applyIntegrationChange(input: { nextContent: input.plan.nextContent, validate: (content) => input.adapter.parse(content), now: input.now, + beforeReplace: async (result) => { + journal = createJournal(input, result); + await writeIntegrationTransactionJournal(journalPath, journal); + }, }); + } else { + const original = await readOriginalState(input.plan.configPath); + journal = createJournal(input, original); + await writeIntegrationTransactionJournal(journalPath, journal); } + if (journal === undefined) + throw new SetupStorageError('Transaction journal was not prepared.'); + await writeIntegrationTransactionJournal(journalPath, { + ...journal, + phase: 'client-written', + }); + const nextRecord = { client: input.plan.client, configPath: input.plan.configPath, @@ -76,24 +103,27 @@ export async function applyIntegrationChange(input: { integrations: input.plan.operation === 'removed' ? existing : [...existing, nextRecord], }; }); + ownershipPersisted = true; + await deleteIntegrationTransactionJournal(journalPath); } catch (error) { + if (ownershipPersisted) throw error; + if (journal === undefined || backup === undefined) throw error; try { - if (backup !== undefined) { - await restoreOriginalFile({ - ...(backup.backupPath === undefined ? {} : { backupPath: backup.backupPath }), - targetPath: input.plan.configPath, - originalExisted: backup.originalExisted, - originalMode: backup.originalMode, - }); - if (backup.originalExisted) { - input.adapter.parse(await readFile(input.plan.configPath, 'utf8')); - } else { - await assertAbsent(input.plan.configPath); - } + await restoreOriginalFile({ + ...(backup.backupPath === undefined ? {} : { backupPath: backup.backupPath }), + targetPath: input.plan.configPath, + originalExisted: backup.originalExisted, + originalMode: backup.originalMode, + }); + if (backup.originalExisted) { + input.adapter.parse(await readFile(input.plan.configPath, 'utf8')); + } else { + await assertAbsent(input.plan.configPath); } + await deleteIntegrationTransactionJournal(journalPath); } catch (restoreError) { throw new SetupStorageError( - `Client configuration was replaced but could not be restored from ${backup?.backupPath ?? input.plan.configPath}.`, + `Client configuration was replaced but could not be restored from ${backup.backupPath ?? input.plan.configPath}.`, new AggregateError([error, restoreError]), ); } @@ -111,9 +141,52 @@ export async function applyIntegrationChange(input: { changed: true, ...(backup?.backupPath === undefined ? {} : { backupPath: backup.backupPath }), }; + }; + + if (input.clientLockHeld) return action(); + return withExclusiveFileLock(`${input.plan.configPath}.relay-lock`, action, { + ...(input.lockRetryDelayMs === undefined ? {} : { retryDelayMs: input.lockRetryDelayMs }), + ...(input.lockMaxAttempts === undefined ? {} : { maxAttempts: input.lockMaxAttempts }), + ...(input.sleep === undefined ? {} : { sleep: input.sleep }), + recoveryJournalPath: journalPath, }); } +function createJournal( + input: Parameters[0], + original: BackupAndAtomicWriteResult, +): RelayIntegrationTransactionJournal { + return { + schemaVersion: 1, + client: input.plan.client, + configPath: input.plan.configPath, + entryId: RELAY_ENTRY_ID, + action: + input.plan.operation === 'disabled' + ? 'disable' + : input.plan.operation === 'removed' + ? 'remove' + : 'setup', + phase: 'prepared', + beforeFingerprint: input.plan.beforeFingerprint, + nextFingerprint: fingerprint(input.plan.nextContent), + originalExisted: original.originalExisted, + originalMode: original.originalMode, + ...(original.backupPath === undefined ? {} : { backupPath: original.backupPath }), + applicationVersion: input.applicationVersion, + startedAt: input.now.toISOString(), + }; +} + +async function readOriginalState(path: string): Promise { + try { + return { originalExisted: true, originalMode: (await stat(path)).mode & 0o777 }; + } catch (error) { + if (isMissing(error)) return { originalExisted: false, originalMode: 0o600 }; + throw new SetupStorageError(`Could not inspect configuration at ${path}.`, error); + } +} + async function readCurrentContent(path: string): Promise { try { return await readFile(path, 'utf8'); diff --git a/src/distribution/setup/backup-and-atomic-write.ts b/src/distribution/setup/backup-and-atomic-write.ts index fe9a20f..c797af3 100644 --- a/src/distribution/setup/backup-and-atomic-write.ts +++ b/src/distribution/setup/backup-and-atomic-write.ts @@ -22,6 +22,7 @@ export async function backupAndAtomicWrite(input: { readonly nextContent: string; readonly validate: (content: string) => void; readonly now: Date; + readonly beforeReplace?: (result: BackupAndAtomicWriteResult) => Promise; }): Promise { const original = await readOriginalFile(input.targetPath); if (fingerprint(original.contents) !== input.expectedFingerprint) @@ -50,14 +51,16 @@ export async function backupAndAtomicWrite(input: { }); if (fingerprint(current) !== input.expectedFingerprint) throw new SetupConflictError(`Configuration changed before replacement: ${input.targetPath}`); - await replaceFile(tempPath, input.targetPath); - replaced = true; - input.validate(await readFile(input.targetPath, 'utf8')); - return { + const result = { ...(backupPath === undefined ? {} : { backupPath }), originalExisted: original.existed, originalMode: original.mode, - }; + } satisfies BackupAndAtomicWriteResult; + await input.beforeReplace?.(result); + await replaceFile(tempPath, input.targetPath); + replaced = true; + input.validate(await readFile(input.targetPath, 'utf8')); + return result; } catch (error) { if (replaced) { try { diff --git a/src/distribution/setup/file-lock.ts b/src/distribution/setup/file-lock.ts index 6f42a04..35e5a54 100644 --- a/src/distribution/setup/file-lock.ts +++ b/src/distribution/setup/file-lock.ts @@ -9,6 +9,7 @@ export interface ExclusiveFileLockOptions { readonly retryDelayMs?: number; readonly maxAttempts?: number; readonly sleep?: (milliseconds: number) => Promise; + readonly recoveryJournalPath?: string; } export async function withExclusiveFileLock( @@ -38,9 +39,7 @@ export async function withExclusiveFileLock( throw new SetupStorageError(`Setup lock could not be opened at ${lockPath}.`, error); attempts += 1; if (attempts >= maxAttempts) - throw new SetupConflictError( - `Another Relay configuration operation is in progress. Retry after it completes. Lock: ${lockPath}`, - ); + throw new SetupConflictError(formatConflictMessage(lockPath, options.recoveryJournalPath)); await sleep(retryDelayMs); } } @@ -75,12 +74,26 @@ export async function withExclusiveFileLock( } catch (error) { if (!isMissing(error)) cleanupError ??= error; } - if (cleanupError !== undefined) - throw new SetupStorageError(`Setup lock could not be released at ${lockPath}.`, cleanupError); + if (cleanupError !== undefined) { + const releaseError = new SetupStorageError( + `Setup lock could not be released at ${lockPath}.`, + cleanupError, + ); + if (actionFailed) throw new AggregateError([actionError, releaseError]); + throw releaseError; + } if (actionFailed) throw actionError; return result as T; } +function formatConflictMessage(lockPath: string, recoveryJournalPath?: string): string { + const journal = + recoveryJournalPath === undefined + ? 'Preserve any associated transaction journal and backup.' + : `Transaction journal: ${recoveryJournalPath}. Preserve the journal and backup`; + return `Another Relay configuration operation is in progress. Lock: ${lockPath}. ${journal} If no Relay process is active, remove only the stale lock and rerun the same command so Relay can execute journal recovery.`; +} + async function delay(milliseconds: number): Promise { await new Promise((resolveDelay) => setTimeout(resolveDelay, milliseconds)); } diff --git a/src/distribution/setup/integration-transaction-journal.ts b/src/distribution/setup/integration-transaction-journal.ts new file mode 100644 index 0000000..91951df --- /dev/null +++ b/src/distribution/setup/integration-transaction-journal.ts @@ -0,0 +1,248 @@ +import { readFile, stat, unlink, writeFile } from 'node:fs/promises'; +import { basename, dirname, join, normalize, resolve } from 'node:path'; +import { randomUUID } from 'node:crypto'; +import type { ClientConfigAdapter } from './clients/client-adapter.js'; +import { replaceFile, restoreOriginalFile } from './backup-and-atomic-write.js'; +import { fingerprint } from './plan-integration-change.js'; +import type { OwnershipStore } from './ownership-store.js'; +import type { MutableIntegrationClient } from './setup-types.js'; +import { SetupConflictError, SetupStorageError } from './setup-errors.js'; + +export interface RelayIntegrationTransactionJournal { + readonly schemaVersion: 1; + readonly client: MutableIntegrationClient; + readonly configPath: string; + readonly entryId: 'relay'; + readonly action: 'setup' | 'disable' | 'remove'; + readonly phase: 'prepared' | 'client-written'; + readonly beforeFingerprint: string; + readonly nextFingerprint: string; + readonly originalExisted: boolean; + readonly originalMode: number; + readonly backupPath?: string; + readonly applicationVersion: string; + readonly startedAt: string; +} + +export async function writeIntegrationTransactionJournal( + journalPath: string, + journal: RelayIntegrationTransactionJournal, +): Promise { + const temporaryPath = joinTemporaryPath(journalPath); + try { + await writeFile(temporaryPath, `${JSON.stringify(journal, null, 2)}\n`, { + flag: 'wx', + mode: 0o600, + }); + await replaceFile(temporaryPath, journalPath); + } catch (error) { + await unlink(temporaryPath).catch(() => undefined); + throw journalStorageError(journalPath, error); + } +} + +export async function deleteIntegrationTransactionJournal(journalPath: string): Promise { + try { + await unlink(journalPath); + } catch (error) { + if (!isMissing(error)) throw journalStorageError(journalPath, error); + } +} + +export async function recoverIntegrationTransaction(input: { + readonly journalPath: string; + readonly configPath: string; + readonly adapter: ClientConfigAdapter; + readonly ownershipStore: OwnershipStore; + readonly applicationVersion: string; +}): Promise<'none' | 'rolled-back' | 'completed'> { + const journal = await readIntegrationTransactionJournal(input.journalPath); + if (journal === undefined) return 'none'; + if (!samePath(journal.configPath, input.configPath)) + throw malformedJournalError( + input.journalPath, + new Error('Journal configuration path mismatch.'), + ); + + if (journal.phase === 'prepared') { + const currentFingerprint = await readConfigFingerprint(input.configPath); + if (currentFingerprint === journal.beforeFingerprint) { + await deleteIntegrationTransactionJournal(input.journalPath); + return 'rolled-back'; + } + if (currentFingerprint !== journal.nextFingerprint) + throw new SetupConflictError( + `The client configuration changed during an interrupted Relay transaction. Preserve the transaction journal and backup: ${input.journalPath}`, + ); + } + + const ownership = await input.ownershipStore.read(); + const record = ownership.integrations.find( + (candidate) => + candidate.client === journal.client && samePath(candidate.configPath, journal.configPath), + ); + const ownershipMatches = + journal.action === 'remove' + ? record === undefined + : record?.status === (journal.action === 'setup' ? 'enabled' : 'disabled'); + if (ownershipMatches) { + await deleteIntegrationTransactionJournal(input.journalPath); + return 'completed'; + } + + try { + if (journal.beforeFingerprint === journal.nextFingerprint) { + if ((await readConfigFingerprint(input.configPath)) !== journal.beforeFingerprint) + throw new SetupConflictError( + `The client configuration changed during an interrupted Relay transaction. Preserve the transaction journal and backup: ${input.journalPath}`, + ); + await deleteIntegrationTransactionJournal(input.journalPath); + return 'rolled-back'; + } + await restoreOriginalFile({ + ...(journal.backupPath === undefined ? {} : { backupPath: journal.backupPath }), + targetPath: input.configPath, + originalExisted: journal.originalExisted, + originalMode: journal.originalMode, + }); + await validateRestoredConfiguration(input.adapter, input.configPath, journal.originalExisted); + await deleteIntegrationTransactionJournal(input.journalPath); + return 'rolled-back'; + } catch (error) { + throw new SetupStorageError( + `Interrupted Relay transaction could not be recovered. The journal and backup were retained. Journal: ${input.journalPath}. Do not edit the client configuration until the journal is inspected or restored manually.`, + new AggregateError([error, new Error(`Original transaction journal: ${input.journalPath}`)]), + ); + } +} + +async function readIntegrationTransactionJournal( + journalPath: string, +): Promise { + let source: string; + try { + source = await readFile(journalPath, 'utf8'); + } catch (error) { + if (isMissing(error)) return undefined; + throw malformedJournalError(journalPath, error); + } + try { + return validateJournal(JSON.parse(source)); + } catch (error) { + throw malformedJournalError(journalPath, error); + } +} + +function validateJournal(value: unknown): RelayIntegrationTransactionJournal { + if (!isRecord(value)) throw new Error('Journal must be an object.'); + const allowedKeys = new Set([ + 'schemaVersion', + 'client', + 'configPath', + 'entryId', + 'action', + 'phase', + 'beforeFingerprint', + 'nextFingerprint', + 'originalExisted', + 'originalMode', + 'backupPath', + 'applicationVersion', + 'startedAt', + ]); + if (Object.keys(value).some((key) => !allowedKeys.has(key))) + throw new Error('Journal contains unsupported fields.'); + if ( + value.schemaVersion !== 1 || + (value.client !== 'codex' && value.client !== 'claude-code') || + typeof value.configPath !== 'string' || + value.entryId !== 'relay' || + (value.action !== 'setup' && value.action !== 'disable' && value.action !== 'remove') || + (value.phase !== 'prepared' && value.phase !== 'client-written') || + typeof value.beforeFingerprint !== 'string' || + typeof value.nextFingerprint !== 'string' || + typeof value.originalExisted !== 'boolean' || + typeof value.originalMode !== 'number' || + (value.backupPath !== undefined && typeof value.backupPath !== 'string') || + typeof value.applicationVersion !== 'string' || + typeof value.startedAt !== 'string' || + !isAbsolutePath(value.configPath) + ) + throw new Error('Journal has an unsupported schema.'); + return { + schemaVersion: 1, + client: value.client, + configPath: normalize(resolve(value.configPath)), + entryId: 'relay', + action: value.action, + phase: value.phase, + beforeFingerprint: value.beforeFingerprint, + nextFingerprint: value.nextFingerprint, + originalExisted: value.originalExisted, + originalMode: value.originalMode, + ...(typeof value.backupPath === 'string' ? { backupPath: value.backupPath } : {}), + applicationVersion: value.applicationVersion, + startedAt: value.startedAt, + }; +} + +async function validateRestoredConfiguration( + adapter: ClientConfigAdapter, + configPath: string, + existed: boolean, +): Promise { + if (!existed) { + try { + await stat(configPath); + } catch (error) { + if (isMissing(error)) return; + throw error; + } + throw new SetupStorageError(`Previously absent configuration was not removed: ${configPath}.`); + } + adapter.parse(await readFile(configPath, 'utf8')); +} + +async function readConfigFingerprint(configPath: string): Promise { + try { + return fingerprint(await readFile(configPath)); + } catch (error) { + if (isMissing(error)) return fingerprint(''); + throw journalStorageError(configPath, error); + } +} + +function malformedJournalError(journalPath: string, cause: unknown): SetupStorageError { + return new SetupStorageError( + `Transaction journal is malformed or unsupported at ${journalPath}. Do not edit the client configuration until the journal is inspected or restored manually.`, + cause, + ); +} + +function journalStorageError(path: string, cause: unknown): SetupStorageError { + return new SetupStorageError(`Transaction journal could not be updated at ${path}.`, cause); +} + +function joinTemporaryPath(path: string): string { + return join(dirname(path), `.${basename(path)}.${process.pid}.${randomUUID()}.tmp`); +} + +function samePath(left: string, right: string): boolean { + const normalizedLeft = normalize(resolve(left)); + const normalizedRight = normalize(resolve(right)); + return process.platform === 'win32' + ? normalizedLeft.toLowerCase() === normalizedRight.toLowerCase() + : normalizedLeft === normalizedRight; +} + +function isAbsolutePath(value: string): boolean { + return /^[A-Za-z]:[\\/]/.test(value) || value.startsWith('\\\\') || value.startsWith('/'); +} + +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isMissing(error: unknown): boolean { + return isRecord(error) && error.code === 'ENOENT'; +} diff --git a/src/interfaces/cli/run-operational-command.ts b/src/interfaces/cli/run-operational-command.ts index b5cbec2..44f318a 100644 --- a/src/interfaces/cli/run-operational-command.ts +++ b/src/interfaces/cli/run-operational-command.ts @@ -8,9 +8,12 @@ import { import { applyIntegrationChange } from '../../distribution/setup/apply-integration-change.js'; import { createClaudeJsonAdapter } from '../../distribution/setup/clients/claude-json-adapter.js'; import { createCodexTomlAdapter } from '../../distribution/setup/clients/codex-toml-adapter.js'; +import { withExclusiveFileLock } from '../../distribution/setup/file-lock.js'; +import { recoverIntegrationTransaction } from '../../distribution/setup/integration-transaction-journal.js'; import { planIntegrationChange } from '../../distribution/setup/plan-integration-change.js'; import { renderIntegrationSnippet } from '../../distribution/setup/snippets.js'; import type { MutableIntegrationClient } from '../../distribution/setup/setup-types.js'; +import { normalize, resolve } from 'node:path'; import { CliUsageError } from './output/cli-errors.js'; import { parseOperationalCommand, type OperationalCommand } from './parse-operational-command.js'; import { writeOperationalError, writeOperationalSuccess } from './operational-output.js'; @@ -100,15 +103,20 @@ export async function runOperationalCommand( const client: MutableIntegrationClient = command.client; const configPath = command.configFile; const adapter = client === 'codex' ? createCodexTomlAdapter() : createClaudeJsonAdapter(); - const ownership = await store.read(); const action = command.kind === 'config-disable' ? 'disable' : command.kind === 'config-remove' ? 'remove' : 'setup'; - const plan = await planIntegrationChange({ action, client, configPath, adapter, ownership }); if (command.kind === 'setup' && !command.apply) { + const plan = await planIntegrationChange({ + action, + client, + configPath, + adapter, + ownership: await store.read(), + }); writeOperationalSuccess(dependencies.stdout, 'setup', { client, changed: plan.changed, @@ -119,22 +127,45 @@ export async function runOperationalCommand( }); return 0; } - const result = await applyIntegrationChange({ - plan, - adapter, - ownershipStore: store, - applicationVersion: dependencies.applicationVersion, - now: dependencies.now?.() ?? new Date(), - }); - writeOperationalSuccess(dependencies.stdout, action, { - client: result.client, - changed: result.changed, - operation: result.operation, - path: result.configPath, - entryId: result.entryId, - ...(result.backupPath === undefined ? {} : { backupPath: result.backupPath }), - }); - return 0; + const normalizedConfigPath = normalize(resolve(configPath)); + const journalPath = `${normalizedConfigPath}.relay-transaction.json`; + return withExclusiveFileLock( + `${normalizedConfigPath}.relay-lock`, + async () => { + await recoverIntegrationTransaction({ + journalPath, + configPath: normalizedConfigPath, + adapter, + ownershipStore: store, + applicationVersion: dependencies.applicationVersion, + }); + const plan = await planIntegrationChange({ + action, + client, + configPath: normalizedConfigPath, + adapter, + ownership: await store.read(), + }); + const result = await applyIntegrationChange({ + plan, + adapter, + ownershipStore: store, + applicationVersion: dependencies.applicationVersion, + now: dependencies.now?.() ?? new Date(), + clientLockHeld: true, + }); + writeOperationalSuccess(dependencies.stdout, action, { + client: result.client, + changed: result.changed, + operation: result.operation, + path: result.configPath, + entryId: result.entryId, + ...(result.backupPath === undefined ? {} : { backupPath: result.backupPath }), + }); + return 0; + }, + { recoveryJournalPath: journalPath }, + ); } catch (error) { return writeOperationalError(dependencies.stdout, dependencies.stderr, error); } diff --git a/tests/integration/setup-workflow.test.ts b/tests/integration/setup-workflow.test.ts index b96db8a..3ac966b 100644 --- a/tests/integration/setup-workflow.test.ts +++ b/tests/integration/setup-workflow.test.ts @@ -1,7 +1,17 @@ -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + readdirSync, + rmSync, + unlinkSync, + writeFileSync, +} from 'node:fs'; import { spawnSync } from 'node:child_process'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { pathToFileURL } from 'node:url'; import { afterEach, describe, expect, it } from 'vitest'; interface CliRun { @@ -93,4 +103,136 @@ describe('installed setup workflow', () => { expect(JSON.parse(result.stdout) as { ok?: boolean }).toMatchObject({ ok: false }); expect(result.stderr).toMatch(/database|path/i); }); + + it('recovers a cross-process transaction interrupted after client write', () => { + const root = mkdtempSync(join(tmpdir(), 'relay-setup-crash-recovery-')); + roots.push(root); + const configPath = join(root, 'codex.toml'); + const journalPath = `${configPath}.relay-transaction.json`; + const lockPath = `${configPath}.relay-lock`; + const original = '[profile]\nname = "before crash"\n'; + writeFileSync(configPath, original); + const caseData = JSON.stringify({ root, configPath, journalPath, lockPath }); + const adapterModule = pathToFileURL( + join(process.cwd(), 'src/distribution/setup/clients/codex-toml-adapter.ts'), + ).href; + const atomicWriteModule = pathToFileURL( + join(process.cwd(), 'src/distribution/setup/backup-and-atomic-write.ts'), + ).href; + const journalModule = pathToFileURL( + join(process.cwd(), 'src/distribution/setup/integration-transaction-journal.ts'), + ).href; + const lockModule = pathToFileURL( + join(process.cwd(), 'src/distribution/setup/file-lock.ts'), + ).href; + const planModule = pathToFileURL( + join(process.cwd(), 'src/distribution/setup/plan-integration-change.ts'), + ).href; + const crashScript = ` + import { createCodexTomlAdapter } from ${JSON.stringify(adapterModule)}; + import { backupAndAtomicWrite } from ${JSON.stringify(atomicWriteModule)}; + import { writeIntegrationTransactionJournal } from ${JSON.stringify(journalModule)}; + import { withExclusiveFileLock } from ${JSON.stringify(lockModule)}; + import { fingerprint } from ${JSON.stringify(planModule)}; + const value = JSON.parse(process.env.RELAY_TRANSACTION_CASE); + const adapter = createCodexTomlAdapter(); + const before = ${JSON.stringify(original)}; + const next = adapter.upsertRelayEntry(before); + await withExclusiveFileLock(value.lockPath, async () => { + let prepared; + const backup = await backupAndAtomicWrite({ + targetPath: value.configPath, + expectedFingerprint: fingerprint(before), + nextContent: next, + validate: (content) => adapter.parse(content), + now: new Date('2026-08-02T01:02:03.004Z'), + beforeReplace: async (result) => { + prepared = { + schemaVersion: 1, + client: 'codex', + configPath: value.configPath, + entryId: 'relay', + action: 'setup', + phase: 'prepared', + beforeFingerprint: fingerprint(before), + nextFingerprint: fingerprint(next), + originalExisted: result.originalExisted, + originalMode: result.originalMode, + ...(result.backupPath === undefined ? {} : { backupPath: result.backupPath }), + applicationVersion: '0.1.0', + startedAt: '2026-08-02T01:02:03.004Z', + }; + await writeIntegrationTransactionJournal(value.journalPath, prepared); + }, + }); + await writeIntegrationTransactionJournal(value.journalPath, { ...prepared, phase: 'client-written' }); + process.exit(0); + }); + `; + const crashed = spawnSync( + process.execPath, + ['--import', 'tsx/esm', '--input-type=module', '--eval', crashScript], + { + cwd: process.cwd(), + env: { ...process.env, RELAY_TRANSACTION_CASE: caseData }, + encoding: 'utf8', + timeout: 30_000, + }, + ); + expect(crashed.status).toBe(0); + expect(existsSync(lockPath)).toBe(true); + expect(existsSync(journalPath)).toBe(true); + expect(readFileSync(configPath, 'utf8')).toContain('command = "relay"'); + + unlinkSync(lockPath); + + const runModule = pathToFileURL( + join(process.cwd(), 'src/interfaces/cli/run-operational-command.ts'), + ).href; + const recoveryScript = ` + import { runOperationalCommand } from ${JSON.stringify(runModule)}; + const value = JSON.parse(process.env.RELAY_TRANSACTION_CASE); + const output = []; + const code = await runOperationalCommand( + ['setup', '--client', 'codex', '--config-file', value.configPath, '--apply'], + { + runtimePaths: { + dataRoot: value.root + '/data', + configRoot: value.root + '/config', + cacheRoot: value.root + '/cache', + databasePath: value.root + '/data/relay.db', + }, + openRuntime: () => ({ close: () => undefined }), + applicationVersion: '0.1.0', + stdout: { write: (text) => output.push(text) }, + stderr: { write: () => undefined }, + }, + ); + process.stdout.write(JSON.stringify({ code, output })); + `; + const recovered = spawnSync( + process.execPath, + ['--import', 'tsx/esm', '--input-type=module', '--eval', recoveryScript], + { + cwd: process.cwd(), + env: { ...process.env, RELAY_TRANSACTION_CASE: caseData }, + encoding: 'utf8', + timeout: 30_000, + }, + ); + expect(recovered.status).toBe(0); + const result = JSON.parse(recovered.stdout) as { code: number; output: string[] }; + expect(result.code).toBe(0); + expect(readFileSync(configPath, 'utf8')).toContain('command = "relay"'); + expect(existsSync(journalPath)).toBe(false); + const ownership = JSON.parse(readFileSync(join(root, 'config', 'config.json'), 'utf8')) as { + integrations: Array<{ client: string; configPath: string; status: string }>; + }; + expect(ownership.integrations).toEqual([ + expect.objectContaining({ client: 'codex', configPath, status: 'enabled' }), + ]); + const backups = readdirSync(root).filter((name) => name.includes('.relay-backup-')); + expect(backups.length).toBeGreaterThanOrEqual(2); + expect(backups.map((name) => readFileSync(join(root, name), 'utf8'))).toContain(original); + }); }); diff --git a/tests/unit/distribution/setup/apply-integration-change.test.ts b/tests/unit/distribution/setup/apply-integration-change.test.ts index 7a5ec3b..1491e62 100644 --- a/tests/unit/distribution/setup/apply-integration-change.test.ts +++ b/tests/unit/distribution/setup/apply-integration-change.test.ts @@ -321,16 +321,18 @@ describe('applyIntegrationChange', () => { ownershipStore: createOwnershipStore({ metadataPath: join(root, 'config.json'), applicationVersion: '0.1.0', - lockRetryDelayMs: 0, - lockMaxAttempts: 2, - sleep: async () => undefined, }), applicationVersion: '0.1.0', now: new Date('2026-08-02T01:02:03.004Z'), + lockRetryDelayMs: 0, + lockMaxAttempts: 2, + sleep: async () => undefined, }), ).rejects.toMatchObject({ constructor: SetupConflictError, - message: expect.stringMatching(/relay-lock/i), + message: expect.stringMatching( + /relay-lock.*relay-transaction\.json.*remove only the stale lock/i, + ), }); } finally { unlinkSync(lockPath); diff --git a/tests/unit/distribution/setup/integration-transaction-journal.test.ts b/tests/unit/distribution/setup/integration-transaction-journal.test.ts new file mode 100644 index 0000000..65fbdc4 --- /dev/null +++ b/tests/unit/distribution/setup/integration-transaction-journal.test.ts @@ -0,0 +1,337 @@ +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + statSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { createCodexTomlAdapter } from '../../../../src/distribution/setup/clients/codex-toml-adapter.js'; +import { + deleteIntegrationTransactionJournal, + recoverIntegrationTransaction, + writeIntegrationTransactionJournal, + type RelayIntegrationTransactionJournal, +} from '../../../../src/distribution/setup/integration-transaction-journal.js'; +import type { OwnershipStore } from '../../../../src/distribution/setup/ownership-store.js'; +import { fingerprint } from '../../../../src/distribution/setup/plan-integration-change.js'; +import type { RelayOwnershipFile } from '../../../../src/distribution/setup/setup-types.js'; +import { + SetupConflictError, + SetupStorageError, +} from '../../../../src/distribution/setup/setup-errors.js'; + +function createStore(initial: RelayOwnershipFile): OwnershipStore { + let current = initial; + return { + read: async () => current, + update: async (mutate) => { + current = await mutate(current); + return current; + }, + }; +} + +function journalFor(input: { + configPath: string; + action?: RelayIntegrationTransactionJournal['action']; + phase?: RelayIntegrationTransactionJournal['phase']; + beforeFingerprint?: string; + nextFingerprint?: string; + originalExisted?: boolean; + originalMode?: number; + backupPath?: string; +}): RelayIntegrationTransactionJournal { + return { + schemaVersion: 1, + client: 'codex', + configPath: input.configPath, + entryId: 'relay', + action: input.action ?? 'setup', + phase: input.phase ?? 'client-written', + beforeFingerprint: input.beforeFingerprint ?? fingerprint('before'), + nextFingerprint: input.nextFingerprint ?? fingerprint('next'), + originalExisted: input.originalExisted ?? true, + originalMode: input.originalMode ?? 0o640, + ...(input.backupPath === undefined ? {} : { backupPath: input.backupPath }), + applicationVersion: '0.1.0', + startedAt: '2026-08-02T01:02:03.004Z', + }; +} + +describe('integration transaction journal recovery', () => { + const roots: string[] = []; + afterEach(() => + roots.splice(0).forEach((root) => rmSync(root, { recursive: true, force: true })), + ); + + function createCase(): { root: string; configPath: string; journalPath: string } { + const root = mkdtempSync(join(tmpdir(), 'relay-transaction-')); + roots.push(root); + const configPath = join(root, 'codex.toml'); + return { root, configPath, journalPath: `${configPath}.relay-transaction.json` }; + } + + it('returns none when no journal exists', async () => { + const { configPath, journalPath } = createCase(); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).resolves.toBe('none'); + }); + + it('fails closed and retains a malformed journal', async () => { + const { configPath, journalPath } = createCase(); + writeFileSync(journalPath, '{"schemaVersion":99}'); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).rejects.toMatchObject({ + message: expect.stringContaining(journalPath), + }); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).rejects.toThrow(/do not edit/i); + expect(existsSync(journalPath)).toBe(true); + }); + + it('deletes a prepared journal when the client still has the before fingerprint', async () => { + const { configPath, journalPath } = createCase(); + const before = '[profile]\nname = "before"\n'; + writeFileSync(configPath, before); + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ + configPath, + phase: 'prepared', + beforeFingerprint: fingerprint(before), + }), + ); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).resolves.toBe('rolled-back'); + expect(readFileSync(configPath, 'utf8')).toBe(before); + expect(existsSync(journalPath)).toBe(false); + }); + + it('treats a prepared journal with the next fingerprint as client-written recovery', async () => { + const { configPath, journalPath } = createCase(); + const before = '[profile]\nname = "before"\n'; + const next = `${before}\n[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n`; + const backupPath = join(createCase().root, 'backup.toml'); + writeFileSync(configPath, next); + writeFileSync(backupPath, before); + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ + configPath, + phase: 'prepared', + beforeFingerprint: fingerprint(before), + nextFingerprint: fingerprint(next), + backupPath, + }), + ); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).resolves.toBe('rolled-back'); + expect(readFileSync(configPath, 'utf8')).toBe(before); + }); + + it('returns conflict for an unrelated prepared fingerprint and preserves state', async () => { + const { configPath, journalPath } = createCase(); + const unrelated = '[profile]\nname = "unrelated"\n'; + writeFileSync(configPath, unrelated); + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ configPath, phase: 'prepared' }), + ); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).rejects.toBeInstanceOf(SetupConflictError); + expect(existsSync(journalPath)).toBe(true); + expect(readFileSync(configPath, 'utf8')).toBe(unrelated); + }); + + it.each([ + ['setup', 'enabled'], + ['disable', 'disabled'], + ] as const)( + 'completes a client-written %s journal when ownership matches', + async (action, status) => { + const { configPath, journalPath } = createCase(); + writeFileSync(configPath, ''); + await writeIntegrationTransactionJournal(journalPath, journalFor({ configPath, action })); + const store = createStore({ + schemaVersion: 1, + integrations: [ + { + client: 'codex', + configPath, + entryId: 'relay', + command: 'relay', + args: ['mcp'], + status, + applicationVersion: '0.1.0', + lastSuccessfulSetupAt: '2026-08-02T01:02:03.004Z', + }, + ], + }); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: store, + applicationVersion: '0.1.0', + }), + ).resolves.toBe('completed'); + expect(existsSync(journalPath)).toBe(false); + }, + ); + + it('completes a client-written remove journal when ownership is absent', async () => { + const { configPath, journalPath } = createCase(); + writeFileSync(configPath, ''); + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ configPath, action: 'remove' }), + ); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).resolves.toBe('completed'); + expect(existsSync(journalPath)).toBe(false); + }); + + it('restores an existing client file byte-for-byte and preserves its mode', async () => { + const { configPath, journalPath } = createCase(); + const before = '[profile]\r\nname = "before"\r\n'; + const next = `${before}\r\n[mcp_servers.relay]\r\ncommand = "relay"\r\nargs = ["mcp"]\r\n`; + const backupPath = join(journalPath, '..', 'codex.toml.relay-backup'); + writeFileSync(configPath, next); + writeFileSync(backupPath, before); + chmodSync(backupPath, 0o640); + const originalMode = statSync(backupPath).mode & 0o777; + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ + configPath, + beforeFingerprint: fingerprint(before), + nextFingerprint: fingerprint(next), + backupPath, + originalMode, + }), + ); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).resolves.toBe('rolled-back'); + expect(readFileSync(configPath)).toEqual(Buffer.from(before)); + expect(statSync(configPath).mode & 0o777).toBe(originalMode); + expect(existsSync(backupPath)).toBe(true); + }); + + it('removes an originally absent client file during rollback', async () => { + const { configPath, journalPath } = createCase(); + const next = '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n'; + writeFileSync(configPath, next); + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ + configPath, + originalExisted: false, + originalMode: 0o600, + beforeFingerprint: fingerprint(''), + nextFingerprint: fingerprint(next), + }), + ); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).resolves.toBe('rolled-back'); + expect(existsSync(configPath)).toBe(false); + }); + + it('retains the journal and backup when restoration fails', async () => { + const { configPath, journalPath } = createCase(); + const backupPath = join(journalPath, '..', 'codex.toml.relay-backup'); + mkdirSync(configPath); + writeFileSync(backupPath, 'before'); + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ configPath, backupPath, originalExisted: true }), + ); + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).rejects.toMatchObject({ constructor: SetupStorageError }); + expect(existsSync(journalPath)).toBe(true); + expect(existsSync(backupPath)).toBe(true); + }); + + it('deletes a journal atomically when explicitly requested', async () => { + const { configPath, journalPath } = createCase(); + await writeIntegrationTransactionJournal(journalPath, journalFor({ configPath })); + await deleteIntegrationTransactionJournal(journalPath); + expect(existsSync(journalPath)).toBe(false); + }); +}); diff --git a/tests/unit/distribution/setup/ownership-store.test.ts b/tests/unit/distribution/setup/ownership-store.test.ts index 858d71a..bb89a05 100644 --- a/tests/unit/distribution/setup/ownership-store.test.ts +++ b/tests/unit/distribution/setup/ownership-store.test.ts @@ -129,7 +129,7 @@ describe('ownership store', () => { try { await expect(store.update((current) => current)).rejects.toMatchObject({ constructor: SetupConflictError, - message: expect.stringMatching(/in progress.*retry.*relay-lock/i), + message: expect.stringMatching(/in progress.*relay-lock.*remove only the stale lock/i), }); } finally { unlinkSync(lockPath); diff --git a/tests/unit/interfaces/cli/run-operational-command.test.ts b/tests/unit/interfaces/cli/run-operational-command.test.ts index 588bcf0..18cb037 100644 --- a/tests/unit/interfaces/cli/run-operational-command.test.ts +++ b/tests/unit/interfaces/cli/run-operational-command.test.ts @@ -1,9 +1,11 @@ -import { existsSync, mkdtempSync, rmSync } from 'node:fs'; +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, describe, expect, it } from 'vitest'; import { runOperationalCommand } from '../../../../src/interfaces/cli/run-operational-command.js'; import type { OperationalDependencies } from '../../../../src/interfaces/cli/run-operational-command.js'; +import { writeIntegrationTransactionJournal } from '../../../../src/distribution/setup/integration-transaction-journal.js'; +import { fingerprint } from '../../../../src/distribution/setup/plan-integration-change.js'; function createDependencies(root: string, output: string[]): OperationalDependencies { return { @@ -81,4 +83,37 @@ describe('runOperationalCommand', () => { expect(code).toBe(0); expect(JSON.parse(output[0] ?? '{}').data.createdDirectories).toEqual([dataRoot, configRoot]); }); + + it('recovers an interrupted transaction before replanning setup', async () => { + const root = mkdtempSync(join(tmpdir(), 'relay-operational-')); + roots.push(root); + const output: string[] = []; + const configPath = join(root, 'codex.toml'); + const nextContent = '[mcp_servers.relay]\ncommand = "relay"\nargs = ["mcp"]\n'; + writeFileSync(configPath, nextContent); + await writeIntegrationTransactionJournal(`${configPath}.relay-transaction.json`, { + schemaVersion: 1, + client: 'codex', + configPath, + entryId: 'relay', + action: 'setup', + phase: 'client-written', + beforeFingerprint: fingerprint(''), + nextFingerprint: fingerprint(nextContent), + originalExisted: false, + originalMode: 0o600, + applicationVersion: '0.1.0', + startedAt: '2026-08-02T01:02:03.004Z', + }); + + const code = await runOperationalCommand( + ['setup', '--client', 'codex', '--config-file', configPath, '--apply'], + createDependencies(root, output), + ); + + expect(code).toBe(0); + expect(readFileSync(configPath, 'utf8')).toContain('command = "relay"'); + expect(existsSync(`${configPath}.relay-transaction.json`)).toBe(false); + expect(JSON.parse(output.at(-1) ?? '{}').data.operation).toBe('created'); + }); }); From b33f7cc3a92943d695f99767c2285f3e27c2fd37 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 19:54:19 +0530 Subject: [PATCH 10/11] fix: fail closed on changed recovery target --- .../setup/integration-transaction-journal.ts | 20 ++-- .../integration-transaction-journal.test.ts | 109 +++++++++++++++++- 2 files changed, 118 insertions(+), 11 deletions(-) diff --git a/src/distribution/setup/integration-transaction-journal.ts b/src/distribution/setup/integration-transaction-journal.ts index 91951df..ae17be4 100644 --- a/src/distribution/setup/integration-transaction-journal.ts +++ b/src/distribution/setup/integration-transaction-journal.ts @@ -64,17 +64,14 @@ export async function recoverIntegrationTransaction(input: { new Error('Journal configuration path mismatch.'), ); + const currentFingerprint = await readConfigFingerprint(input.configPath); if (journal.phase === 'prepared') { - const currentFingerprint = await readConfigFingerprint(input.configPath); if (currentFingerprint === journal.beforeFingerprint) { await deleteIntegrationTransactionJournal(input.journalPath); return 'rolled-back'; } - if (currentFingerprint !== journal.nextFingerprint) - throw new SetupConflictError( - `The client configuration changed during an interrupted Relay transaction. Preserve the transaction journal and backup: ${input.journalPath}`, - ); } + if (currentFingerprint !== journal.nextFingerprint) throw transactionConflict(input); const ownership = await input.ownershipStore.read(); const record = ownership.integrations.find( @@ -92,10 +89,6 @@ export async function recoverIntegrationTransaction(input: { try { if (journal.beforeFingerprint === journal.nextFingerprint) { - if ((await readConfigFingerprint(input.configPath)) !== journal.beforeFingerprint) - throw new SetupConflictError( - `The client configuration changed during an interrupted Relay transaction. Preserve the transaction journal and backup: ${input.journalPath}`, - ); await deleteIntegrationTransactionJournal(input.journalPath); return 'rolled-back'; } @@ -116,6 +109,15 @@ export async function recoverIntegrationTransaction(input: { } } +function transactionConflict(input: { + readonly journalPath: string; + readonly configPath: string; +}): SetupConflictError { + return new SetupConflictError( + `The client configuration changed after the interrupted Relay transaction. Relay preserved the client configuration, transaction journal, and backup for manual inspection. Config: ${input.configPath}. Journal: ${input.journalPath}.`, + ); +} + async function readIntegrationTransactionJournal( journalPath: string, ): Promise { diff --git a/tests/unit/distribution/setup/integration-transaction-journal.test.ts b/tests/unit/distribution/setup/integration-transaction-journal.test.ts index 65fbdc4..aa44040 100644 --- a/tests/unit/distribution/setup/integration-transaction-journal.test.ts +++ b/tests/unit/distribution/setup/integration-transaction-journal.test.ts @@ -199,7 +199,10 @@ describe('integration transaction journal recovery', () => { async (action, status) => { const { configPath, journalPath } = createCase(); writeFileSync(configPath, ''); - await writeIntegrationTransactionJournal(journalPath, journalFor({ configPath, action })); + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ configPath, action, nextFingerprint: fingerprint('') }), + ); const store = createStore({ schemaVersion: 1, integrations: [ @@ -233,7 +236,7 @@ describe('integration transaction journal recovery', () => { writeFileSync(configPath, ''); await writeIntegrationTransactionJournal( journalPath, - journalFor({ configPath, action: 'remove' }), + journalFor({ configPath, action: 'remove', nextFingerprint: fingerprint('') }), ); await expect( recoverIntegrationTransaction({ @@ -247,6 +250,108 @@ describe('integration transaction journal recovery', () => { expect(existsSync(journalPath)).toBe(false); }); + it('fails closed when matching ownership exists but the client file changed externally', async () => { + const { root, configPath, journalPath } = createCase(); + const external = '[profile]\nname = "external"\n'; + const backupPath = join(root, 'codex.toml.relay-backup'); + writeFileSync(configPath, external); + writeFileSync(backupPath, '[profile]\nname = "before"\n'); + const ownership = { + schemaVersion: 1 as const, + integrations: [ + { + client: 'codex' as const, + configPath, + entryId: 'relay' as const, + command: 'relay' as const, + args: ['mcp'] as const, + status: 'enabled' as const, + applicationVersion: '0.1.0', + lastSuccessfulSetupAt: '2026-08-02T01:02:03.004Z', + }, + ], + }; + const store = createStore(ownership); + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ configPath, backupPath, nextFingerprint: fingerprint('next') }), + ); + + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: store, + applicationVersion: '0.1.0', + }), + ).rejects.toBeInstanceOf(SetupConflictError); + expect(readFileSync(configPath, 'utf8')).toBe(external); + expect(existsSync(journalPath)).toBe(true); + expect(existsSync(backupPath)).toBe(true); + await expect(store.read()).resolves.toEqual(ownership); + }); + + it('fails closed when ownership is missing and the client file changed externally', async () => { + const { root, configPath, journalPath } = createCase(); + const external = '[profile]\nname = "external"\n'; + const backup = '[profile]\nname = "before"\n'; + const backupPath = join(root, 'codex.toml.relay-backup'); + writeFileSync(configPath, external); + writeFileSync(backupPath, backup); + const store = createStore({ schemaVersion: 1, integrations: [] }); + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ configPath, backupPath, nextFingerprint: fingerprint('next') }), + ); + + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: store, + applicationVersion: '0.1.0', + }), + ).rejects.toMatchObject({ + constructor: SetupConflictError, + message: expect.stringContaining(configPath), + }); + expect(readFileSync(configPath, 'utf8')).toBe(external); + expect(existsSync(journalPath)).toBe(true); + expect(readFileSync(backupPath, 'utf8')).toBe(backup); + await expect(store.read()).resolves.toEqual({ schemaVersion: 1, integrations: [] }); + }); + + it('fails closed when a no-content-change transaction target changed externally', async () => { + const { configPath, journalPath } = createCase(); + const external = '[profile]\nname = "external"\n'; + writeFileSync(configPath, external); + await writeIntegrationTransactionJournal( + journalPath, + journalFor({ + configPath, + beforeFingerprint: fingerprint('same'), + nextFingerprint: fingerprint('same'), + }), + ); + + await expect( + recoverIntegrationTransaction({ + journalPath, + configPath, + adapter: createCodexTomlAdapter(), + ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), + applicationVersion: '0.1.0', + }), + ).rejects.toMatchObject({ + constructor: SetupConflictError, + message: expect.stringMatching(/journal|config/i), + }); + expect(readFileSync(configPath, 'utf8')).toBe(external); + expect(existsSync(journalPath)).toBe(true); + }); + it('restores an existing client file byte-for-byte and preserves its mode', async () => { const { configPath, journalPath } = createCase(); const before = '[profile]\r\nname = "before"\r\n'; From 08fe6c4f60867a35fd1261c87f1a4e6e82c218a9 Mon Sep 17 00:00:00 2001 From: Krishnamurti Pandaram Date: Sun, 2 Aug 2026 20:12:05 +0530 Subject: [PATCH 11/11] fix: address remaining review findings --- .../setup/apply-integration-change.ts | 13 ++++++++++- src/distribution/setup/file-lock.ts | 2 +- .../setup/integration-transaction-journal.ts | 1 - src/interfaces/cli/run-operational-command.ts | 4 ++-- .../setup/apply-integration-change.test.ts | 1 + .../integration-transaction-journal.test.ts | 22 ++++--------------- .../cli/run-operational-command.test.ts | 1 + 7 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/distribution/setup/apply-integration-change.ts b/src/distribution/setup/apply-integration-change.ts index a7d4ae8..6eb967b 100644 --- a/src/distribution/setup/apply-integration-change.ts +++ b/src/distribution/setup/apply-integration-change.ts @@ -107,7 +107,18 @@ export async function applyIntegrationChange(input: { await deleteIntegrationTransactionJournal(journalPath); } catch (error) { if (ownershipPersisted) throw error; - if (journal === undefined || backup === undefined) throw error; + if (journal === undefined) throw error; + if (backup === undefined) { + try { + await deleteIntegrationTransactionJournal(journalPath); + } catch (journalError) { + throw new SetupStorageError( + `No-content-change transaction journal could not be removed at ${journalPath}.`, + new AggregateError([error, journalError]), + ); + } + throw error; + } try { await restoreOriginalFile({ ...(backup.backupPath === undefined ? {} : { backupPath: backup.backupPath }), diff --git a/src/distribution/setup/file-lock.ts b/src/distribution/setup/file-lock.ts index 35e5a54..649c86a 100644 --- a/src/distribution/setup/file-lock.ts +++ b/src/distribution/setup/file-lock.ts @@ -90,7 +90,7 @@ function formatConflictMessage(lockPath: string, recoveryJournalPath?: string): const journal = recoveryJournalPath === undefined ? 'Preserve any associated transaction journal and backup.' - : `Transaction journal: ${recoveryJournalPath}. Preserve the journal and backup`; + : `Transaction journal: ${recoveryJournalPath}. Preserve the journal and backup.`; return `Another Relay configuration operation is in progress. Lock: ${lockPath}. ${journal} If no Relay process is active, remove only the stale lock and rerun the same command so Relay can execute journal recovery.`; } diff --git a/src/distribution/setup/integration-transaction-journal.ts b/src/distribution/setup/integration-transaction-journal.ts index ae17be4..f019b89 100644 --- a/src/distribution/setup/integration-transaction-journal.ts +++ b/src/distribution/setup/integration-transaction-journal.ts @@ -54,7 +54,6 @@ export async function recoverIntegrationTransaction(input: { readonly configPath: string; readonly adapter: ClientConfigAdapter; readonly ownershipStore: OwnershipStore; - readonly applicationVersion: string; }): Promise<'none' | 'rolled-back' | 'completed'> { const journal = await readIntegrationTransactionJournal(input.journalPath); if (journal === undefined) return 'none'; diff --git a/src/interfaces/cli/run-operational-command.ts b/src/interfaces/cli/run-operational-command.ts index 44f318a..aed8ed2 100644 --- a/src/interfaces/cli/run-operational-command.ts +++ b/src/interfaces/cli/run-operational-command.ts @@ -132,12 +132,11 @@ export async function runOperationalCommand( return withExclusiveFileLock( `${normalizedConfigPath}.relay-lock`, async () => { - await recoverIntegrationTransaction({ + const recovery = await recoverIntegrationTransaction({ journalPath, configPath: normalizedConfigPath, adapter, ownershipStore: store, - applicationVersion: dependencies.applicationVersion, }); const plan = await planIntegrationChange({ action, @@ -160,6 +159,7 @@ export async function runOperationalCommand( operation: result.operation, path: result.configPath, entryId: result.entryId, + recovery, ...(result.backupPath === undefined ? {} : { backupPath: result.backupPath }), }); return 0; diff --git a/tests/unit/distribution/setup/apply-integration-change.test.ts b/tests/unit/distribution/setup/apply-integration-change.test.ts index 1491e62..63c3e5c 100644 --- a/tests/unit/distribution/setup/apply-integration-change.test.ts +++ b/tests/unit/distribution/setup/apply-integration-change.test.ts @@ -258,6 +258,7 @@ describe('applyIntegrationChange', () => { now: new Date('2026-08-02T01:02:03.004Z'), }), ).rejects.toBeInstanceOf(SetupConflictError); + expect(existsSync(`${path}.relay-transaction.json`)).toBe(false); }); it('preserves metadata and restore failures together', async () => { diff --git a/tests/unit/distribution/setup/integration-transaction-journal.test.ts b/tests/unit/distribution/setup/integration-transaction-journal.test.ts index aa44040..8303e54 100644 --- a/tests/unit/distribution/setup/integration-transaction-journal.test.ts +++ b/tests/unit/distribution/setup/integration-transaction-journal.test.ts @@ -85,7 +85,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).resolves.toBe('none'); }); @@ -99,7 +98,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).rejects.toMatchObject({ message: expect.stringContaining(journalPath), @@ -110,7 +108,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).rejects.toThrow(/do not edit/i); expect(existsSync(journalPath)).toBe(true); @@ -134,7 +131,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).resolves.toBe('rolled-back'); expect(readFileSync(configPath, 'utf8')).toBe(before); @@ -164,7 +160,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).resolves.toBe('rolled-back'); expect(readFileSync(configPath, 'utf8')).toBe(before); @@ -184,7 +179,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).rejects.toBeInstanceOf(SetupConflictError); expect(existsSync(journalPath)).toBe(true); @@ -224,7 +218,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: store, - applicationVersion: '0.1.0', }), ).resolves.toBe('completed'); expect(existsSync(journalPath)).toBe(false); @@ -244,7 +237,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).resolves.toBe('completed'); expect(existsSync(journalPath)).toBe(false); @@ -283,7 +275,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: store, - applicationVersion: '0.1.0', }), ).rejects.toBeInstanceOf(SetupConflictError); expect(readFileSync(configPath, 'utf8')).toBe(external); @@ -311,7 +302,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: store, - applicationVersion: '0.1.0', }), ).rejects.toMatchObject({ constructor: SetupConflictError, @@ -342,7 +332,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).rejects.toMatchObject({ constructor: SetupConflictError, @@ -353,10 +342,10 @@ describe('integration transaction journal recovery', () => { }); it('restores an existing client file byte-for-byte and preserves its mode', async () => { - const { configPath, journalPath } = createCase(); + const { root, configPath, journalPath } = createCase(); const before = '[profile]\r\nname = "before"\r\n'; const next = `${before}\r\n[mcp_servers.relay]\r\ncommand = "relay"\r\nargs = ["mcp"]\r\n`; - const backupPath = join(journalPath, '..', 'codex.toml.relay-backup'); + const backupPath = join(root, 'codex.toml.relay-backup'); writeFileSync(configPath, next); writeFileSync(backupPath, before); chmodSync(backupPath, 0o640); @@ -377,7 +366,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).resolves.toBe('rolled-back'); expect(readFileSync(configPath)).toEqual(Buffer.from(before)); @@ -405,15 +393,14 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).resolves.toBe('rolled-back'); expect(existsSync(configPath)).toBe(false); }); it('retains the journal and backup when restoration fails', async () => { - const { configPath, journalPath } = createCase(); - const backupPath = join(journalPath, '..', 'codex.toml.relay-backup'); + const { root, configPath, journalPath } = createCase(); + const backupPath = join(root, 'codex.toml.relay-backup'); mkdirSync(configPath); writeFileSync(backupPath, 'before'); await writeIntegrationTransactionJournal( @@ -426,7 +413,6 @@ describe('integration transaction journal recovery', () => { configPath, adapter: createCodexTomlAdapter(), ownershipStore: createStore({ schemaVersion: 1, integrations: [] }), - applicationVersion: '0.1.0', }), ).rejects.toMatchObject({ constructor: SetupStorageError }); expect(existsSync(journalPath)).toBe(true); diff --git a/tests/unit/interfaces/cli/run-operational-command.test.ts b/tests/unit/interfaces/cli/run-operational-command.test.ts index 18cb037..15b1b02 100644 --- a/tests/unit/interfaces/cli/run-operational-command.test.ts +++ b/tests/unit/interfaces/cli/run-operational-command.test.ts @@ -115,5 +115,6 @@ describe('runOperationalCommand', () => { expect(readFileSync(configPath, 'utf8')).toContain('command = "relay"'); expect(existsSync(`${configPath}.relay-transaction.json`)).toBe(false); expect(JSON.parse(output.at(-1) ?? '{}').data.operation).toBe('created'); + expect(JSON.parse(output.at(-1) ?? '{}').data.recovery).toBe('rolled-back'); }); });