diff --git a/specter/CHANGELOG.md b/specter/CHANGELOG.md index 4a90823..3816603 100644 --- a/specter/CHANGELOG.md +++ b/specter/CHANGELOG.md @@ -8,6 +8,10 @@ Unreleased changes accumulate under `## Unreleased`. Every user-visible change a ## Unreleased +### Fixed + +- The VS Code extension now preserves independently installed Specter CLIs, including newer supported versions. Update the extension to receive the fix. Existing CLI installations need no reinstall. If you have no shell CLI, run Specter: Add CLI to Shell PATH. + --- ## v0.15.1 - 2026-09-13 diff --git a/specter/README.md b/specter/README.md index fbd77f5..137e783 100644 --- a/specter/README.md +++ b/specter/README.md @@ -43,7 +43,7 @@ Specter validates the artifacts in front of it, not the order you wrote them in. ### VS Code extension (recommended for most users) -Search **Specter SDD** in the Extensions panel. The extension auto-downloads the CLI binary matching the host's OS and architecture, installs it under `~/.specter/bin/`, and wires up the integrated terminal so `specter` works without further setup. To call `specter` from external terminals, run **Specter: Add CLI to Shell PATH** from the command palette once. +Search **Specter SDD** in the Extensions panel. The extension uses a CLI already on your PATH when its version is one the extension supports, and otherwise downloads its own copy under `~/.specter/cli/`. It never replaces a CLI you installed. To call `specter` from a terminal, run **Specter: Add CLI to Shell PATH** from the command palette once; it puts a copy at `~/.specter/bin/specter` when nothing is there. ### CLI, Linux / macOS (tar.gz) diff --git a/specter/docs/CLI_REFERENCE.md b/specter/docs/CLI_REFERENCE.md index dc1b6ff..d236ee5 100644 --- a/specter/docs/CLI_REFERENCE.md +++ b/specter/docs/CLI_REFERENCE.md @@ -6,7 +6,7 @@ Specter is a spec compiler toolchain, "a type system for specs." It validates, l ## Installation -Install the VS Code extension for the smoothest path: it auto-downloads the CLI and sets PATH. For CLI-only installs (tar.gz, `.deb`, `.rpm`, Windows zip, or build from source), see the [Install section in the Specter README](../README.md#install). Asset naming pattern: `specter___.` with lowercase `linux`/`darwin`/`windows` and `amd64`/`arm64`. +Install the VS Code extension for the smoothest path: it uses a supported CLI already on PATH, or downloads its own copy, and can set PATH for you. For CLI-only installs (tar.gz, `.deb`, `.rpm`, Windows zip, or build from source), see the [Install section in the Specter README](../README.md#install). Asset naming pattern: `specter___.` with lowercase `linux`/`darwin`/`windows` and `amd64`/`arm64`. --- diff --git a/specter/docs/QUICKSTART.md b/specter/docs/QUICKSTART.md index 5fe09c7..8bc8f1f 100644 --- a/specter/docs/QUICKSTART.md +++ b/specter/docs/QUICKSTART.md @@ -6,7 +6,7 @@ Get from zero to a working spec pipeline in under 5 minutes. ## 1. Install -**Fastest path — VS Code extension:** search `Specter SDD` in the Extensions panel, install, then run **Specter: Add CLI to Shell PATH** from the command palette once. The extension auto-downloads the CLI binary for your OS and architecture. +**Fastest path, the VS Code extension:** search `Specter SDD` in the Extensions panel, install, then run **Specter: Add CLI to Shell PATH** from the command palette once. The extension uses a CLI already on your PATH when its version is one it supports, and otherwise downloads its own copy; the shell PATH command puts a copy at `~/.specter/bin/specter` for your terminal. **CLI-only, macOS / Linux:** ```bash @@ -32,7 +32,7 @@ For `.deb`, `.rpm`, and other install methods see the [Specter README](../README ## 2. Bootstrap specs from your code -Point Specter at your source directory — it generates draft specs automatically: +Point Specter at your source directory, and it generates draft specs automatically: ```bash specter reverse src/ # TypeScript / JavaScript @@ -54,7 +54,7 @@ This creates a `specs/` directory with one `.spec.yaml` per file group. specter init ``` -Creates `specter.yaml` — the manifest that tells Specter (and the VS Code extension) where your specs and tests live. +Creates `specter.yaml`, the manifest that tells Specter (and the VS Code extension) where your specs and tests live. --- @@ -163,7 +163,7 @@ Add this to CI and you're protected. ## What's next? -- **[Getting Started](GETTING_STARTED.md)** — full walkthrough from zero specs to 100% coverage, with AI prompts for every step and VS Code workspace guide -- **[CLI Reference](CLI_REFERENCE.md)** — every command and flag -- **[AI Prompts](AI_PROMPTS.md)** — ready-to-use prompts for the full SDD loop -- **[FAQ](FAQ.md)** — "Do I need to migrate my existing specs?" +- **[Getting Started](GETTING_STARTED.md)**: full walkthrough from zero specs to 100% coverage, with AI prompts for every step and VS Code workspace guide +- **[CLI Reference](CLI_REFERENCE.md)**: every command and flag +- **[AI Prompts](AI_PROMPTS.md)**: ready-to-use prompts for the full SDD loop +- **[FAQ](FAQ.md)**: "Do I need to migrate my existing specs?" diff --git a/specter/specs/spec-vscode.spec.yaml b/specter/specs/spec-vscode.spec.yaml index c78f66d..43627f4 100644 --- a/specter/specs/spec-vscode.spec.yaml +++ b/specter/specs/spec-vscode.spec.yaml @@ -1,6 +1,6 @@ spec: id: spec-vscode - version: "5.1.1" + version: "6.0.0" status: draft tier: 2 @@ -71,7 +71,7 @@ spec: constraints: - id: C-01 - description: "MUST discover the specter binary in this resolution order: workspace setting specter.binaryPath → PATH → ~/.specter/bin/specter → auto-download from GitHub Releases. MUST verify SHA256 checksum on auto-download. MUST validate the resolved binary (magic-byte check and `--version` probe) regardless of resolution source; a corrupt file on PATH must not slip through as a valid binary. MUST expose a `Specter: Re-download CLI` command and MUST surface CLI invocation failures via an output channel reachable from the status bar (never leave the UI stuck on an indefinite loading state)." + description: "MUST discover the specter binary in this resolution order: workspace setting specter.binaryPath → PATH → ~/.specter/bin/specter → the extension's private copy under ~/.specter/cli, downloaded from GitHub Releases when absent. A PATH or ~/.specter/bin candidate is used only when its version satisfies the range C-34 declares; one outside the range is skipped and never modified. MUST verify SHA256 checksum on auto-download. MUST validate the resolved binary (magic-byte check and `--version` probe) regardless of resolution source; a corrupt file on PATH must not slip through as a valid binary. MUST expose a `Specter: Re-download CLI` command and MUST surface CLI invocation failures via an output channel reachable from the status bar (never leave the UI stuck on an indefinite loading state)." type: technical enforcement: error @@ -201,7 +201,7 @@ spec: enforcement: error - id: C-27 - description: "The CLI auto-download MUST default to the CLI version matching the extension's own version — a v0.10.0 extension fetches v0.10.0 CLI, not whatever GitHub's /releases/latest currently returns. The extension reads its own version via ctx.extension.packageJSON.version at download time. Users MAY override via specter.version: set to 'latest' to track GitHub's newest release, or pin a specific semver (e.g. '0.9.2'). Default-to-latest is prohibited because the CLI release and extension Marketplace publish are decoupled — a GoReleaser-produced CLI release fires on tag push, but the matching extension may publish days later (or not at all), creating split-brain installs where users run an older extension against a newer CLI." + description: "The extension's private copy (C-34) MUST default to the CLI version matching the extension's own version — a v0.10.0 extension fetches v0.10.0 CLI, not whatever GitHub's /releases/latest currently returns. The extension reads its own version via ctx.extension.packageJSON.version at download time. Users MAY override via specter.version: set to 'latest' to track GitHub's newest release, or pin a specific semver (e.g. '0.9.2'). Default-to-latest is prohibited because the CLI release and extension Marketplace publish are decoupled — a GoReleaser-produced CLI release fires on tag push, but the matching extension may publish days later (or not at all), creating split-brain installs where users run an older extension against a newer CLI. The private copy is how the extension guarantees itself a compatible CLI. It is not how the user's shell gets one: a newer CLI on PATH is used as is while it satisfies the C-34 range, and is never replaced." type: business enforcement: error @@ -511,6 +511,11 @@ spec: type: technical enforcement: error + - id: C-34 + description: "The extension MUST keep the CLI it runs separate from the CLI the user's shell and git hooks run. Its own copy lives at ~/.specter/cli/specter- (with .exe on Windows), one file per version, and the extension MAY download, replace, or delete files there. The extension MUST NOT write, replace, or delete ~/.specter/bin/specter, any binary found on PATH, or the file named by specter.binaryPath: not at activation, not on a version mismatch, and not from the Re-download command. The one permitted write to ~/.specter/bin/specter is the Add CLI to Shell PATH command copying the private binary there when no file exists, as an explicit user action; when a file exists the command MUST leave it alone and say so. The extension MUST declare, in package.json under `specterCli.range`, the CLI version range it supports, in the form `>=A.B.C =0.15.0 <0.16.0`: a PATH binary reporting 0.15.1 resolves with source `path`, no download is planned, and no write is planned. A PATH binary reporting 0.16.0 is skipped, the plan names it with its version and the range for the Output channel, the private copy for 0.15.0 is resolved with a download planned if it is absent, and the PATH file is not in any write. A ~/.specter/bin/specter reporting 0.14.1 is skipped the same way. A candidate whose version cannot be read is not a candidate." + inputs: + extension_version: "0.15.0" + declared_range: ">=0.15.0 <0.16.0" + path_candidate_in_range: "0.15.1" + path_candidate_above_range: "0.16.0" + user_dir_candidate_below_range: "0.14.1" + expected_output: + in_range_source: "path" + in_range_download_planned: false + in_range_writes: [] + out_of_range_source: "private" + out_of_range_skipped_names_version_and_range: true + out_of_range_writes_touching_candidate: [] + references_constraints: ["C-34", "C-01"] + priority: critical + + - id: AC-81 + description: "Nothing automatic writes ~/.specter/bin/specter. With no candidate anywhere, the plan downloads to ~/.specter/cli/specter-0.15.0 (with .exe on Windows) and to nothing else. The Re-download command's plan targets the same private path and no other. The Add CLI to Shell PATH command copies the private binary to ~/.specter/bin/specter only when no file exists there; when one exists, it leaves the bytes unchanged and reports that it left the file alone." + inputs: + no_candidates: "no PATH binary, no ~/.specter/bin/specter, no private copy" + redownload: "the Re-download command with a private copy present" + shell_path_absent: "the shell PATH command with no ~/.specter/bin/specter" + shell_path_present: "the shell PATH command with an existing ~/.specter/bin/specter of different bytes" + expected_output: + download_target: "~/.specter/cli/specter-0.15.0" + user_dir_in_any_automatic_write: false + shell_path_absent_writes_user_copy: true + shell_path_present_bytes_unchanged: true + shell_path_present_reports_left_alone: true + references_constraints: ["C-34"] + priority: critical + + - id: AC-82 + description: "package.json declares `specterCli.range` in the form `>=A.B.C =0.15.0 <0.16.0" + candidates: "0.15.0, 0.15.1, 0.15.10, 0.16.0, 0.14.9, 0.15.2-rc.1" + malformed: "^0.15.0, >=0.15.0, 0.15.x" + expected_output: + satisfies: "0.15.0, 0.15.1, 0.15.10, 0.15.2-rc.1" + does_not_satisfy: "0.16.0, 0.14.9" + malformed_rejected: true + repository_version_satisfies_declared_range: true + references_constraints: ["C-34"] + priority: high + depends_on: - spec_id: spec-parse version_range: "^1.1.0" @@ -1629,6 +1682,11 @@ spec: relationship: requires changelog: + - version: "6.0.0" + date: "2026-09-14" + author: "specter-team" + type: major + description: "C-34 separates the CLI the extension runs from the CLI the user's shell runs. The extension keeps a private copy per version under ~/.specter/cli and never writes ~/.specter/bin/specter, a PATH binary, or specter.binaryPath; the one exception is the shell PATH command creating the user copy when none exists. It declares a supported CLI range in package.json and uses any PATH or user-dir binary inside it as is, skipping and never modifying one outside it. C-01's order now ends at the private copy, and C-27's default-version rule now governs that copy. AC-80 binds the range gate on both sides and that a skipped candidate is untouched, AC-81 binds that nothing automatic writes the user's copy, and AC-82 binds the range form and that the repository's VERSION satisfies it. Major: the auto-update on version mismatch that a conforming implementation performed since v0.6.5 is now forbidden. It downgraded a 0.15.1 in ~/.specter/bin to 0.15.0 on every activation after the CLI-only v0.15.1 release, with no error, and because the shell PATH command put that directory first on PATH, the pre-push hook ran the downgraded binary." - version: "5.1.1" date: "2026-09-01" author: "specter-team" diff --git a/specter/vscode-extension/README.md b/specter/vscode-extension/README.md index 8778b93..e55bc20 100644 --- a/specter/vscode-extension/README.md +++ b/specter/vscode-extension/README.md @@ -131,9 +131,9 @@ The annotations are plain comments, no build step, no framework, works in any la | Setting | Default | Description | |---|---|---| -| `specter.binaryPath` | `""` | Path to the specter binary. Leave empty to auto-resolve. Machine-scoped; workspace settings are ignored. | -| `specter.autoDownload` | `true` | Download specter automatically if not found. | -| `specter.version` | `""` | Binary version to download. Empty means "match the extension version"; `latest` tracks the newest GitHub release. Machine-scoped; workspace settings are ignored. | +| `specter.binaryPath` | `""` | Path to the specter binary. Leave empty to auto-resolve: a CLI on PATH or at `~/.specter/bin/specter` is used when its version is one the extension supports, and otherwise the extension uses its own copy under `~/.specter/cli`. A path set here must be a working binary that answers `--version`, but it is not checked against the supported range, because it is your explicit choice. A path that does not exist is ignored and resolution continues. Machine-scoped; workspace settings are ignored. | +| `specter.autoDownload` | `true` | Download the extension's own CLI copy when no supported CLI is found. | +| `specter.version` | `""` | Version of the extension's own CLI copy. Empty means "match the extension version"; `latest` tracks the newest GitHub release. A supported CLI on PATH is used regardless. Machine-scoped; workspace settings are ignored. | | `specter.showInsightsOnFailure` | `true` | Open Insights panel automatically when a spec fails threshold. | --- @@ -146,8 +146,8 @@ The annotations are plain comments, no build step, no framework, works in any la | `Specter: Copy Spec Context for AI` | Copy current spec as a structured AI prompt preamble | | `Specter: Run Sync` | Re-run the full coverage pipeline manually | | `Specter: Run Reverse Compiler` | Generate draft specs from your source code | -| `Specter: Add CLI to Shell PATH` | Append `~/.specter/bin` to your shell rc file so `specter` works in external terminals | -| `Specter: Re-download CLI` | Force a fresh download of the CLI binary (recovery if the cached one is broken) | +| `Specter: Add CLI to Shell PATH` | Install a copy of the CLI at `~/.specter/bin/specter` if none is there, and append `~/.specter/bin` to your shell rc file so `specter` works in external terminals. An existing file there is left alone. When you already have a working CLI on PATH, in the supported range or not, the command does nothing, so it never puts a copy ahead of yours. | +| `Specter: Re-download CLI` | Force a fresh download of the extension's own CLI copy (recovery if it is broken). Your `~/.specter/bin/specter` is not touched. | | `Specter: Show Output Log` | Open the Specter output channel with download/coverage error details | | `Specter: Reveal in Tree View` | Jump to the current spec in the Coverage sidebar | @@ -155,7 +155,7 @@ The annotations are plain comments, no build step, no framework, works in any la ## Using `specter` from external terminals -When the extension auto-downloads the CLI, it lands at `~/.specter/bin/specter`. VS Code's integrated terminal gets this path prepended automatically. External terminals (iTerm, Windows Terminal, tmux, etc.) don't, you'd need to type the full path. +The extension keeps its own CLI copy under `~/.specter/cli`, one file per version, and never changes a CLI you installed yourself. `~/.specter/bin/specter` is yours: the extension creates it only when you run the shell PATH command and nothing is there, and it never replaces it afterward. VS Code's integrated terminal gets `~/.specter/bin` prepended automatically. External terminals (iTerm, Windows Terminal, tmux, etc.) don't, you'd need to type the full path. If you have no CLI of your own on PATH, typing `specter` there yourself needs that copy to exist, so run the shell PATH command once. The extension's own commands that open a terminal, such as Run Reverse Compiler and View Diff, use the CLI the extension resolved and need no setup. Run `Specter: Add CLI to Shell PATH` from the command palette once, and the extension will append an idempotent export to your shell's rc file (`.bashrc` on Linux, `.bash_profile` on macOS, `.zshrc` for zsh, `config.fish` for fish). Restart your terminal and `specter` works from anywhere. diff --git a/specter/vscode-extension/package.json b/specter/vscode-extension/package.json index 6d7fdb7..1646456 100644 --- a/specter/vscode-extension/package.json +++ b/specter/vscode-extension/package.json @@ -8,6 +8,9 @@ "engines": { "vscode": "^1.85.0" }, + "specterCli": { + "range": ">=0.15.0 <0.16.0" + }, "categories": [ "AI", "Linters", @@ -48,18 +51,18 @@ "type": "string", "default": "", "scope": "machine", - "description": "Path to the specter binary. If empty, Specter will search PATH then ~/.specter/bin/specter, then auto-download. Machine-scoped: ignored by workspace and folder settings." + "description": "Path to the specter binary. If empty, Specter uses a CLI on PATH or at ~/.specter/bin/specter when its version is one this extension supports, and otherwise its own private copy under ~/.specter/cli. The extension never modifies a binary it did not put there. Machine-scoped: ignored by workspace and folder settings." }, "specter.autoDownload": { "type": "boolean", "default": true, - "description": "Automatically download the specter binary if not found." + "description": "Download the extension's own CLI copy when no supported CLI is found." }, "specter.version": { "type": "string", "default": "", "scope": "machine", - "description": "Specter CLI version to auto-download. Empty (default) matches the extension version. Set 'latest' to always track the newest GitHub release, or pin a specific version (e.g. '0.9.2'). Machine-scoped: ignored by workspace and folder settings." + "description": "Version of the extension's own CLI copy under ~/.specter/cli. Empty (default) matches the extension version. Set 'latest' to always track the newest GitHub release, or pin a specific version (e.g. '0.9.2'). A CLI on PATH inside the supported range is used regardless of this setting. Machine-scoped: ignored by workspace and folder settings." }, "specter.showInsightsOnFailure": { "type": "boolean", diff --git a/specter/vscode-extension/src/__tests__/binary.test.ts b/specter/vscode-extension/src/__tests__/binary.test.ts index 623deab..9e21959 100644 --- a/specter/vscode-extension/src/__tests__/binary.test.ts +++ b/specter/vscode-extension/src/__tests__/binary.test.ts @@ -3,7 +3,7 @@ // Tests for binary discovery and auto-download logic. // All functions under test are pure or injectable — no VS Code runtime required. -import { resolveBinaryPath, verifyChecksum, buildDownloadUrl, isBinaryFile, validateVersion } from '../binaryDiscovery'; +import { planBinaryResolution, verifyChecksum, buildDownloadUrl, isBinaryFile, validateVersion } from '../binaryDiscovery'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -24,61 +24,49 @@ const mockWhich = (name: string): string | null => null; // --------------------------------------------------------------------------- // @ac AC-02 -describe('[spec-vscode/AC-02] resolveBinaryPath', () => { +describe('[spec-vscode/AC-02] planBinaryResolution: setting, then PATH, then user dir, then the private copy', () => { + const RANGE = '>=0.15.0 <0.16.0'; + const base = { + which: mockWhich, + fs: mockFs, + probeVersion: (_: string) => '0.15.1', + userBinPath: '/home/user/.specter/bin/specter', + privateDir: '/home/user/.specter/cli', + privateVersion: '0.15.0', + range: RANGE, + platform: 'linux', + }; + it('returns workspace setting path when specter.binaryPath is set and file exists', () => { const fs = { ...mockFs, exists: (p: string) => p === '/custom/specter' }; - const result = resolveBinaryPath({ - workspaceSetting: '/custom/specter', - which: mockWhich, - fs, - cachePath: '~/.specter/bin/specter', - }); + const result = planBinaryResolution({ ...base, workspaceSetting: '/custom/specter', fs }); expect(result.resolved).toBe('/custom/specter'); expect(result.source).toBe('workspace-setting'); }); it('falls through to PATH when workspace setting is absent', () => { const which = (name: string) => name === 'specter' ? '/usr/local/bin/specter' : null; - const result = resolveBinaryPath({ - workspaceSetting: null, - which, - fs: { ...mockFs, exists: () => true }, - cachePath: '~/.specter/bin/specter', - }); + const result = planBinaryResolution({ ...base, workspaceSetting: null, which, fs: { ...mockFs, exists: () => true } }); expect(result.resolved).toBe('/usr/local/bin/specter'); expect(result.source).toBe('path'); }); - it('falls through to cache path when PATH lookup fails', () => { + it('falls through to the user dir when PATH lookup fails', () => { const fs = { ...mockFs, exists: (p: string) => p === '/home/user/.specter/bin/specter', isExecutable: () => true }; - const result = resolveBinaryPath({ - workspaceSetting: null, - which: () => null, - fs, - cachePath: '/home/user/.specter/bin/specter', - }); + const result = planBinaryResolution({ ...base, workspaceSetting: null, which: () => null, fs }); expect(result.resolved).toBe('/home/user/.specter/bin/specter'); - expect(result.source).toBe('cache'); + expect(result.source).toBe('user-dir'); }); - it('returns needs-download when all resolution strategies fail', () => { - const result = resolveBinaryPath({ - workspaceSetting: null, - which: () => null, - fs: { ...mockFs, exists: () => false }, - cachePath: '/home/user/.specter/bin/specter', - }); - expect(result.resolved).toBeNull(); - expect(result.source).toBe('needs-download'); + it('plans a download of the private copy when every other source fails', () => { + const result = planBinaryResolution({ ...base, workspaceSetting: null, which: () => null, fs: { ...mockFs, exists: () => false } }); + expect(result.source).toBe('private'); + expect(result.download).not.toBeNull(); + expect(result.resolved).toBe(path.join('/home/user/.specter/cli', 'specter-0.15.0')); }); it('rejects workspace setting path that does not exist on disk', () => { - const result = resolveBinaryPath({ - workspaceSetting: '/nonexistent/specter', - which: () => null, - fs: { ...mockFs, exists: () => false }, - cachePath: '~/.specter/bin/specter', - }); + const result = planBinaryResolution({ ...base, workspaceSetting: '/nonexistent/specter', which: () => null, fs: { ...mockFs, exists: () => false } }); // Must not return the non-existent setting; must fall through expect(result.source).not.toBe('workspace-setting'); }); diff --git a/specter/vscode-extension/src/__tests__/extract.test.ts b/specter/vscode-extension/src/__tests__/extract.test.ts new file mode 100644 index 0000000..980300d --- /dev/null +++ b/specter/vscode-extension/src/__tests__/extract.test.ts @@ -0,0 +1,63 @@ +// @spec spec-vscode +// +// C-34 names the private copy ~/.specter/cli/specter-. The plan +// targets that path, and the plan's tests prove it. This test proves the +// download actually lands there: extractBinary is fed a real tar.gz whose +// member is named "specter", as goreleaser produces, and asked to place it +// at a versioned target. An independent review found the tar branch left +// the file as /specter and then chmod'ed a path that did not exist, so +// every private download on Linux and macOS failed with ENOENT. + +import { execFileSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { extractBinary } from '../binaryDiscovery'; + +const describeWithTar = process.platform === 'win32' ? describe.skip : describe; + +/** Builds a tar.gz in tmp whose single member is an executable named "specter". */ +function archiveWithSpecter(tmp: string, body: string): Buffer { + const src = path.join(tmp, 'src'); + fs.mkdirSync(src, { recursive: true }); + fs.writeFileSync(path.join(src, 'specter'), body, { mode: 0o755 }); + const archive = path.join(tmp, 'specter_0.15.0_linux_amd64.tar.gz'); + execFileSync('tar', ['czf', archive, '-C', src, 'specter']); + return fs.readFileSync(archive); +} + +// @ac AC-81 +describeWithTar('[spec-vscode/AC-81] extractBinary places the archive member at the versioned private path', () => { + it('lands the binary at ~/.specter/cli/specter-, executable, and leaves no stray "specter"', async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'specter-extract-')); + try { + const data = archiveWithSpecter(tmp, '#!/bin/sh\necho specter version 0.15.0\n'); + const target = path.join(tmp, 'home', '.specter', 'cli', 'specter-0.15.0'); + + await extractBinary(data, 'tar.gz', target); + + expect(fs.existsSync(target)).toBe(true); + expect(fs.readFileSync(target, 'utf8')).toContain('specter version 0.15.0'); + expect(fs.statSync(target).mode & 0o111).not.toBe(0); + // The member name must not survive beside the versioned file, or a + // later plan would find a stale unversioned binary the extension + // does not track. + expect(fs.existsSync(path.join(path.dirname(target), 'specter'))).toBe(false); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); + + it('a second version extracts beside the first without disturbing it', async () => { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'specter-extract-')); + try { + const dir = path.join(tmp, 'cli'); + await extractBinary(archiveWithSpecter(path.join(tmp, 'a'), 'A'), 'tar.gz', path.join(dir, 'specter-0.15.0')); + await extractBinary(archiveWithSpecter(path.join(tmp, 'b'), 'B'), 'tar.gz', path.join(dir, 'specter-0.15.1')); + expect(fs.readFileSync(path.join(dir, 'specter-0.15.0'), 'utf8')).toBe('A'); + expect(fs.readFileSync(path.join(dir, 'specter-0.15.1'), 'utf8')).toBe('B'); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + }); +}); diff --git a/specter/vscode-extension/src/__tests__/privateCli.test.ts b/specter/vscode-extension/src/__tests__/privateCli.test.ts new file mode 100644 index 0000000..686df54 --- /dev/null +++ b/specter/vscode-extension/src/__tests__/privateCli.test.ts @@ -0,0 +1,220 @@ +// @spec spec-vscode +// +// C-34: the CLI the extension runs is separate from the CLI the user's shell +// runs. These tests bind the pure resolution plan, the range gate, and the +// one permitted write to the user's copy. +// +// The new functions are reached through require() rather than a typed +// import. ts-jest runs strict, so a typed import of a symbol that does not +// exist yet is a build failure, and a build failure is not a red test. Each +// test asserts the function exists first, so the red names what is missing. + +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +// eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-explicit-any +const mod: any = require('../binaryDiscovery'); + +const USER_BIN = '/home/u/.specter/bin/specter'; +const PRIVATE_DIR = '/home/u/.specter/cli'; +const RANGE = '>=0.15.0 <0.16.0'; + +/** A filesystem double keyed by path: which files exist and are executable. */ +function fsWith(paths: string[]) { + return { + exists: (p: string) => paths.includes(p), + isExecutable: (p: string) => paths.includes(p), + }; +} + +/** Builds plan options for one scenario. versions maps a path to what --version reports. */ +function planOpts(overrides: { + which?: string | null; + present?: string[]; + versions?: Record; + workspaceSetting?: string | null; + privateVersion?: string; +}) { + const versions = overrides.versions ?? {}; + return { + workspaceSetting: overrides.workspaceSetting ?? null, + which: (_: string) => overrides.which ?? null, + fs: fsWith(overrides.present ?? []), + probeVersion: (p: string) => (p in versions ? versions[p] : null), + userBinPath: USER_BIN, + privateDir: PRIVATE_DIR, + privateVersion: overrides.privateVersion ?? '0.15.0', + range: RANGE, + platform: 'linux', + }; +} + +// @ac AC-80 +describe('[spec-vscode/AC-80] planBinaryResolution gates PATH and user-dir candidates by the declared range', () => { + it('exports planBinaryResolution', () => { + expect(typeof mod.planBinaryResolution).toBe('function'); + }); + + it('uses a PATH binary inside the range as is: no download, no write', () => { + const plan = mod.planBinaryResolution(planOpts({ + which: '/usr/local/bin/specter', + present: ['/usr/local/bin/specter'], + versions: { '/usr/local/bin/specter': '0.15.1' }, + })); + expect(plan.resolved).toBe('/usr/local/bin/specter'); + expect(plan.source).toBe('path'); + expect(plan.download).toBeNull(); + expect(plan.writes).toEqual([]); + expect(plan.skipped).toEqual([]); + }); + + it('skips a PATH binary above the range, names it, uses the private copy, and never writes the candidate', () => { + const plan = mod.planBinaryResolution(planOpts({ + which: '/usr/local/bin/specter', + present: ['/usr/local/bin/specter'], + versions: { '/usr/local/bin/specter': '0.16.0' }, + })); + expect(plan.source).toBe('private'); + expect(plan.resolved).toBe(path.join(PRIVATE_DIR, 'specter-0.15.0')); + expect(plan.download).toEqual({ version: '0.15.0', target: path.join(PRIVATE_DIR, 'specter-0.15.0') }); + expect(plan.skipped).toEqual([{ path: '/usr/local/bin/specter', version: '0.16.0', range: RANGE }]); + expect(plan.writes).not.toContain('/usr/local/bin/specter'); + expect(plan.writes).not.toContain(USER_BIN); + }); + + it('skips a user-dir binary below the range the same way', () => { + const plan = mod.planBinaryResolution(planOpts({ + present: [USER_BIN], + versions: { [USER_BIN]: '0.14.1' }, + })); + expect(plan.source).toBe('private'); + expect(plan.skipped).toEqual([{ path: USER_BIN, version: '0.14.1', range: RANGE }]); + expect(plan.writes).not.toContain(USER_BIN); + }); + + it('uses a user-dir binary inside the range as is', () => { + const plan = mod.planBinaryResolution(planOpts({ + present: [USER_BIN], + versions: { [USER_BIN]: '0.15.1' }, + })); + expect(plan.resolved).toBe(USER_BIN); + expect(plan.source).toBe('user-dir'); + expect(plan.download).toBeNull(); + expect(plan.writes).toEqual([]); + }); + + it('treats the user dir found through PATH as one candidate, not two', () => { + // On a machine where the shell PATH command ran, which() returns the + // user-dir path itself. It must be considered once. + const plan = mod.planBinaryResolution(planOpts({ + which: USER_BIN, + present: [USER_BIN], + versions: { [USER_BIN]: '0.16.0' }, + })); + expect(plan.skipped).toHaveLength(1); + }); + + it('a candidate whose version cannot be read is not a candidate', () => { + const plan = mod.planBinaryResolution(planOpts({ + which: '/usr/local/bin/specter', + present: ['/usr/local/bin/specter'], + versions: { '/usr/local/bin/specter': null }, + })); + expect(plan.source).toBe('private'); + expect(plan.skipped).toEqual([]); + expect(plan.writes).not.toContain('/usr/local/bin/specter'); + }); + + it('uses an existing private copy without planning a download', () => { + const priv = path.join(PRIVATE_DIR, 'specter-0.15.0'); + const plan = mod.planBinaryResolution(planOpts({ + present: [priv], + versions: { [priv]: '0.15.0' }, + })); + expect(plan.resolved).toBe(priv); + expect(plan.source).toBe('private'); + expect(plan.download).toBeNull(); + }); +}); + +// @ac AC-81 +describe('[spec-vscode/AC-81] nothing automatic writes the user copy', () => { + it('with no candidate anywhere, the download targets the private path and nothing else', () => { + const plan = mod.planBinaryResolution(planOpts({})); + const target = path.join(PRIVATE_DIR, 'specter-0.15.0'); + expect(plan.download).toEqual({ version: '0.15.0', target }); + expect(plan.writes).toEqual([target]); + }); + + it('names the private copy with .exe on Windows', () => { + expect(typeof mod.privateBinaryPath).toBe('function'); + expect(mod.privateBinaryPath(PRIVATE_DIR, '0.15.0', 'win32')).toBe(path.join(PRIVATE_DIR, 'specter-0.15.0.exe')); + expect(mod.privateBinaryPath(PRIVATE_DIR, '0.15.0', 'linux')).toBe(path.join(PRIVATE_DIR, 'specter-0.15.0')); + }); + + it('the Re-download plan targets the private path and no other', () => { + expect(typeof mod.planRedownload).toBe('function'); + const plan = mod.planRedownload({ privateDir: PRIVATE_DIR, version: '0.15.0', platform: 'linux' }); + const target = path.join(PRIVATE_DIR, 'specter-0.15.0'); + expect(plan.download).toEqual({ version: '0.15.0', target }); + expect(plan.writes).toEqual([target]); + }); + + it('the shell PATH command creates the user copy only when absent, and leaves an existing one alone', () => { + expect(typeof mod.installUserCopy).toBe('function'); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'specter-c34-')); + try { + const priv = path.join(dir, 'cli', 'specter-0.15.0'); + fs.mkdirSync(path.dirname(priv), { recursive: true }); + fs.writeFileSync(priv, 'PRIVATE', { mode: 0o755 }); + const user = path.join(dir, 'bin', 'specter'); + + const first = mod.installUserCopy(priv, user); + expect(first.wrote).toBe(true); + expect(fs.readFileSync(user, 'utf8')).toBe('PRIVATE'); + + fs.writeFileSync(user, 'USER OWNED', { mode: 0o755 }); + const second = mod.installUserCopy(priv, user); + expect(second.wrote).toBe(false); + expect(second.message).toMatch(/left/i); + expect(fs.readFileSync(user, 'utf8')).toBe('USER OWNED'); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +// @ac AC-82 +describe('[spec-vscode/AC-82] the declared range, its form, and the shipped CLI', () => { + it('exports satisfiesRange', () => { + expect(typeof mod.satisfiesRange).toBe('function'); + }); + + it('lower bound inclusive, upper bound exclusive, numeric compare, pre-release suffix ignored', () => { + for (const v of ['0.15.0', '0.15.1', '0.15.10', '0.15.2-rc.1']) { + expect({ v, ok: mod.satisfiesRange(v, RANGE) }).toEqual({ v, ok: true }); + } + for (const v of ['0.16.0', '0.14.9']) { + expect({ v, ok: mod.satisfiesRange(v, RANGE) }).toEqual({ v, ok: false }); + } + }); + + it('rejects any range not in the form >=A.B.C { + for (const r of ['^0.15.0', '>=0.15.0', '0.15.x', '']) { + expect(() => mod.satisfiesRange('0.15.1', r)).toThrow(); + } + }); + + it('package.json declares specterCli.range in that form', () => { + const pkg = JSON.parse(fs.readFileSync(path.join(__dirname, '..', '..', 'package.json'), 'utf8')); + expect(pkg.specterCli).toBeDefined(); + expect(pkg.specterCli.range).toMatch(/^>=\d+\.\d+\.\d+ <\d+\.\d+\.\d+$/); + }); + + it('the repository VERSION satisfies the declared range, so the shipped extension accepts the shipped CLI', () => { + const pkg = JSON.parse(fs.readFileSync(path.join(__dirname, '..', '..', 'package.json'), 'utf8')); + const version = fs.readFileSync(path.join(__dirname, '..', '..', '..', 'VERSION'), 'utf8').trim(); + expect(mod.satisfiesRange(version, pkg.specterCli.range)).toBe(true); + }); +}); diff --git a/specter/vscode-extension/src/__tests__/shellInstall.test.ts b/specter/vscode-extension/src/__tests__/shellInstall.test.ts new file mode 100644 index 0000000..3b5894d --- /dev/null +++ b/specter/vscode-extension/src/__tests__/shellInstall.test.ts @@ -0,0 +1,85 @@ +// @spec spec-vscode +// +// The two decisions the wrapper makes on top of the resolution plan: what +// the shell PATH command may do, and how the extension names the binary +// when it types a command into a terminal for the user. + +import { shellInstallDecision, terminalInvocation, BinaryPlan } from '../binaryDiscovery'; + +const USER_BIN = '/home/u/.specter/bin/specter'; +const RANGE = '>=0.15.0 <0.16.0'; + +function plan(p: Partial): BinaryPlan { + return { resolved: '/home/u/.specter/cli/specter-0.15.0', source: 'private', download: null, skipped: [], writes: [], ...p }; +} + +// @ac AC-81 +describe('[spec-vscode/AC-81] shellInstallDecision: the user copy is created only when the extension runs its own', () => { + it('installs and adds PATH when the extension is on its private copy and nothing of the user\'s is on PATH', () => { + const d = shellInstallDecision(plan({}), USER_BIN); + expect(d).toMatchObject({ install: true, addPath: true }); + }); + + it('adds PATH but does not install when the user copy already exists and is in use', () => { + const d = shellInstallDecision(plan({ resolved: USER_BIN, source: 'user-dir' }), USER_BIN); + expect(d).toMatchObject({ install: false, addPath: true }); + }); + + it('does nothing when the extension is using a CLI on PATH', () => { + const d = shellInstallDecision(plan({ resolved: '/usr/local/bin/specter', source: 'path' }), USER_BIN); + expect(d).toMatchObject({ install: false, addPath: false }); + expect(d.reason).toContain('/usr/local/bin/specter'); + }); + + it('does nothing when the extension is using specter.binaryPath', () => { + const d = shellInstallDecision(plan({ resolved: '/opt/specter', source: 'workspace-setting' }), USER_BIN); + expect(d).toMatchObject({ install: false, addPath: false }); + }); + + it('does nothing when a PATH CLI outside the range was skipped, so the shell keeps the user\'s CLI', () => { + // The review found this hole: the skipped PATH CLI set the source to + // private, and the command would then have put ~/.specter/bin ahead of + // the user's 0.16.0, changing what their shell runs. + const d = shellInstallDecision(plan({ skipped: [{ path: '/usr/local/bin/specter', version: '0.16.0', range: RANGE }] }), USER_BIN); + expect(d).toMatchObject({ install: false, addPath: false }); + expect(d.reason).toContain('0.16.0'); + expect(d.reason).toContain(RANGE); + }); + + it('a skipped user-dir copy does not count as a PATH CLI', () => { + // An out-of-range ~/.specter/bin/specter is the user's and is left + // alone, but it is not a reason to refuse the PATH edit. + const d = shellInstallDecision(plan({ skipped: [{ path: USER_BIN, version: '0.14.1', range: RANGE }] }), USER_BIN); + expect(d).toMatchObject({ install: false, addPath: true }); + }); +}); + +// @ac AC-45 +describe('[spec-vscode/AC-45] terminalInvocation names the resolved binary, so terminal commands work without a shell PATH entry', () => { + it('uses a plain resolved path bare', () => { + expect(terminalInvocation('/home/u/.specter/cli/specter-0.15.0', 'reverse ', 'linux')).toBe('/home/u/.specter/cli/specter-0.15.0 reverse '); + }); + + it('single-quotes a path with spaces on POSIX', () => { + expect(terminalInvocation('/Users/a b/.specter/cli/specter-0.15.0', 'diff x', 'darwin')).toBe("'/Users/a b/.specter/cli/specter-0.15.0' diff x"); + }); + + it('makes every shell-active character literal on POSIX: backslash, dollar, backtick, double quote', () => { + const hostile = '/home/u\\x/$HOME/`id`/"q"/specter-0.15.0'; + expect(terminalInvocation(hostile, 'reverse ', 'linux')).toBe(`'${hostile}' reverse `); + }); + + it("writes an embedded single quote as '\\'' on POSIX", () => { + expect(terminalInvocation("/home/o'brien/specter-0.15.0", 'reverse ', 'linux')).toBe("'/home/o'\\''brien/specter-0.15.0' reverse "); + }); + + it('uses the PowerShell call operator and doubled quotes on Windows', () => { + expect(terminalInvocation('C:\\Users\\a b\\specter-0.15.0.exe', 'reverse ', 'win32')).toBe("& 'C:\\Users\\a b\\specter-0.15.0.exe' reverse "); + expect(terminalInvocation("C:\\o'b\\specter.exe", 'reverse ', 'win32')).toBe("& 'C:\\o''b\\specter.exe' reverse "); + expect(terminalInvocation('C:\\Users\\ab\\specter-0.15.0.exe', 'reverse ', 'win32')).toBe('C:\\Users\\ab\\specter-0.15.0.exe reverse '); + }); + + it('falls back to the bare name when nothing is resolved', () => { + expect(terminalInvocation(null, 'reverse ', 'linux')).toBe('specter reverse '); + }); +}); diff --git a/specter/vscode-extension/src/binaryDiscovery.ts b/specter/vscode-extension/src/binaryDiscovery.ts index a73a886..51b0236 100644 --- a/specter/vscode-extension/src/binaryDiscovery.ts +++ b/specter/vscode-extension/src/binaryDiscovery.ts @@ -16,57 +16,12 @@ export interface FsAdapter { isExecutable: (path: string) => boolean; } -export interface ResolveBinaryOptions { - workspaceSetting: string | null; - which: (name: string) => string | null; - fs: FsAdapter; - cachePath: string; -} - -export type BinarySource = 'workspace-setting' | 'path' | 'cache' | 'needs-download'; - -export interface BinaryResolution { - resolved: string | null; - source: BinarySource; -} - export interface DownloadUrlOptions { version: string; os: string; arch: string; } -// --------------------------------------------------------------------------- -// AC-02: Binary discovery — workspace setting → PATH → cache → auto-download -// --------------------------------------------------------------------------- - -/** - * Resolves the specter binary path using the documented priority order: - * 1. Workspace setting (specter.binaryPath) — if file exists on disk - * 2. PATH lookup via which() - * 3. Cache path (~/.specter/bin/specter) — if file exists on disk - * 4. Needs download - */ -export function resolveBinaryPath(opts: ResolveBinaryOptions): BinaryResolution { - // 1. Workspace setting - if (opts.workspaceSetting && opts.fs.exists(opts.workspaceSetting)) { - return { resolved: opts.workspaceSetting, source: 'workspace-setting' }; - } - - // 2. PATH - const fromPath = opts.which('specter'); - if (fromPath && opts.fs.exists(fromPath)) { - return { resolved: fromPath, source: 'path' }; - } - - // 3. Cache - if (opts.fs.exists(opts.cachePath) && opts.fs.isExecutable(opts.cachePath)) { - return { resolved: opts.cachePath, source: 'cache' }; - } - - // 4. Needs download - return { resolved: null, source: 'needs-download' }; -} /** * Returns true if the file at `filePath` looks like a compiled binary @@ -269,8 +224,16 @@ export async function extractBinary( try { if (format === 'tar.gz') { - // Extract only the 'specter' binary from the archive - execFileSync('tar', ['xzf', tmpArchive, '-C', dir, 'specter'], { timeout: 30000 }); + // Extract only the 'specter' member, into a scratch directory, then + // move it to the versioned target. Extracting straight into dir left + // the file as /specter and the chmod below hit a path that did + // not exist, so every private download failed with ENOENT. + const tmpDir = path.join(dir, 'specter-extract'); + fs.rmSync(tmpDir, { recursive: true, force: true }); + fs.mkdirSync(tmpDir, { recursive: true }); + execFileSync('tar', ['xzf', tmpArchive, '-C', tmpDir, 'specter'], { timeout: 30000 }); + fs.renameSync(path.join(tmpDir, 'specter'), targetPath); + fs.rmSync(tmpDir, { recursive: true, force: true }); } else { // Windows: extract zip then move binary const tmpDir = path.join(dir, 'specter-extract'); @@ -323,3 +286,209 @@ export async function downloadChecksums(version: string): Promise[.exe]. */ +export function privateBinaryPath(privateDir: string, version: string, platform: string): string { + const ext = platform === 'win32' ? '.exe' : ''; + return path.join(privateDir, `specter-${version}${ext}`); +} + +const RANGE_RE = /^>=(\d+)\.(\d+)\.(\d+) <(\d+)\.(\d+)\.(\d+)$/; + +function versionTriple(v: string): [number, number, number] | null { + const m = /^(\d+)\.(\d+)\.(\d+)/.exec(v); + if (!m) return null; + return [Number(m[1]), Number(m[2]), Number(m[3])]; +} + +function cmp(a: [number, number, number], b: [number, number, number]): number { + for (let i = 0; i < 3; i++) { + if (a[i] !== b[i]) return a[i] < b[i] ? -1 : 1; + } + return 0; +} + +/** + * Reports whether a CLI version satisfies a declared range of the form + * `>=A.B.C =A.B.C = 0 && cmp(v, hi) < 0; +} + +export type PlanSource = 'workspace-setting' | 'path' | 'user-dir' | 'private'; + +export interface SkippedCandidate { + path: string; + version: string; + range: string; +} + +export interface BinaryPlan { + /** The path to run. When a download is planned, it does not exist yet. */ + resolved: string; + source: PlanSource; + /** The one download this plan performs, or null. */ + download: { version: string; target: string } | null; + /** Candidates left untouched because their version is outside the range. */ + skipped: SkippedCandidate[]; + /** Every path this plan writes. Never the user's copy, PATH, or the setting. */ + writes: string[]; +} + +export interface PlanOptions { + workspaceSetting: string | null; + which: (name: string) => string | null; + fs: FsAdapter; + /** Runs `--version` on a path; null when the file is not a valid CLI. */ + probeVersion: (p: string) => string | null; + /** ~/.specter/bin/specter, the user's copy. Read, never written here. */ + userBinPath: string; + /** ~/.specter/cli, the extension's own directory. */ + privateDir: string; + /** The version the private copy should be, per C-27. */ + privateVersion: string; + /** package.json specterCli.range. */ + range: string; + platform: string; +} + +/** + * The C-34 resolution decision, pure. Order: the workspace setting, used as + * is because it is the user's explicit choice; then PATH, then the user's + * copy, each used only when its version satisfies the range and otherwise + * skipped and left alone; then the private copy, downloaded when absent. + * The plan lists every write it will make, and that list can only ever + * name the private copy. + */ +export function planBinaryResolution(opts: PlanOptions): BinaryPlan { + const skipped: SkippedCandidate[] = []; + + if (opts.workspaceSetting && opts.fs.exists(opts.workspaceSetting)) { + return { resolved: opts.workspaceSetting, source: 'workspace-setting', download: null, skipped, writes: [] }; + } + + const candidates: Array<{ p: string; source: PlanSource }> = []; + const fromPath = opts.which('specter'); + if (fromPath && opts.fs.exists(fromPath)) { + candidates.push({ p: fromPath, source: fromPath === opts.userBinPath ? 'user-dir' : 'path' }); + } + if (fromPath !== opts.userBinPath && opts.fs.exists(opts.userBinPath) && opts.fs.isExecutable(opts.userBinPath)) { + candidates.push({ p: opts.userBinPath, source: 'user-dir' }); + } + for (const c of candidates) { + const v = opts.probeVersion(c.p); + if (!v) continue; + if (satisfiesRange(v, opts.range)) { + return { resolved: c.p, source: c.source, download: null, skipped, writes: [] }; + } + skipped.push({ path: c.p, version: v, range: opts.range }); + } + + const target = privateBinaryPath(opts.privateDir, opts.privateVersion, opts.platform); + if (opts.fs.exists(target) && opts.fs.isExecutable(target) && opts.probeVersion(target)) { + return { resolved: target, source: 'private', download: null, skipped, writes: [] }; + } + return { + resolved: target, + source: 'private', + download: { version: opts.privateVersion, target }, + skipped, + writes: [target], + }; +} + +/** The Re-download command's plan: refresh the private copy and nothing else. */ +export function planRedownload(opts: { privateDir: string; version: string; platform: string }): { download: { version: string; target: string }; writes: string[] } { + const target = privateBinaryPath(opts.privateDir, opts.version, opts.platform); + return { download: { version: opts.version, target }, writes: [target] }; +} + +/** + * The one permitted write to the user's copy: the shell PATH command copying + * the private binary there when nothing is there. An existing file is the + * user's, whatever its version, and is left byte for byte. + */ +export function installUserCopy(privatePath: string, userBinPath: string): { wrote: boolean; message: string } { + if (fs.existsSync(userBinPath)) { + return { wrote: false, message: `${userBinPath} already exists and was left alone. Replace it yourself if you want a different version there.` }; + } + fs.mkdirSync(path.dirname(userBinPath), { recursive: true }); + fs.copyFileSync(privatePath, userBinPath); + fs.chmodSync(userBinPath, 0o755); + return { wrote: true, message: `Installed the CLI at ${userBinPath}.` }; +} + +/** + * What the shell PATH command may do, decided from the resolution plan. + * It installs the user's copy only when the extension is running its own + * copy and no CLI of the user's sits on PATH, in range or out of it. + * Prepending ~/.specter/bin ahead of a CLI the user installed would change + * what their shell runs, which is the very thing C-34 forbids by another + * route. + */ +export function shellInstallDecision(plan: BinaryPlan, userBinPath: string): { install: boolean; addPath: boolean; reason: string } { + if (plan.source === 'path' || plan.source === 'workspace-setting') { + return { install: false, addPath: false, reason: `a CLI is already available at ${plan.resolved}. Nothing was installed.` }; + } + const onPath = plan.skipped.find(s => s.path !== userBinPath); + if (onPath) { + return { + install: false, addPath: false, + reason: `a CLI is already on your PATH at ${onPath.path} (version ${onPath.version}), outside the range this extension supports (${onPath.range}). Nothing was installed, so your shell keeps it.`, + }; + } + // The user copy exists, in use or skipped for its version: it is theirs, + // and only the PATH entry is worth offering. + if (plan.source === 'user-dir' || plan.skipped.some(s => s.path === userBinPath)) { + return { install: false, addPath: true, reason: `${userBinPath} is yours and was left alone.` }; + } + return { install: true, addPath: true, reason: 'installing a copy of the CLI for your shell.' }; +} + +/** + * The command line the extension types into a terminal for the user. It + * names the binary the extension resolved, so the command works whether or + * not `specter` is on the shell PATH. A path made only of characters no + * shell interprets is used bare. Anything else is single-quoted, which is + * the one form that makes every character literal: spaces, backslashes, + * `$`, backticks, and double quotes alike. An embedded single quote is + * written the way each shell reads it, `'\''` on POSIX and `''` in + * PowerShell. Escaping inside double quotes was the previous approach, and + * CodeQL rightly flagged it as incomplete. The arguments are the caller's + * and are not touched. + */ +export function terminalInvocation(binaryPath: string | null, args: string, platform: string = process.platform): string { + const bin = binaryPath ?? 'specter'; + const posix = platform !== 'win32'; + const safe = posix ? /^[A-Za-z0-9._/-]+$/ : /^[A-Za-z0-9._:\\-]+$/; + if (safe.test(bin)) { + return `${bin} ${args}`; + } + if (posix) { + return `'${bin.replace(/'/g, "'\\''")}' ${args}`; + } + // PowerShell needs the call operator to run a quoted path. + return `& '${bin.replace(/'/g, "''")}' ${args}`; +} diff --git a/specter/vscode-extension/src/extension.ts b/specter/vscode-extension/src/extension.ts index a0673b2..2e0dbbe 100644 --- a/specter/vscode-extension/src/extension.ts +++ b/specter/vscode-extension/src/extension.ts @@ -5,7 +5,13 @@ import * as path from 'path'; import { shouldActivate, resolveManifestPath, createClientKey, isSpecFilePath } from './activation'; import { - resolveBinaryPath, + planBinaryResolution, + planRedownload, + privateCliDir, + installUserCopy, + BinaryPlan, + shellInstallDecision, + terminalInvocation, buildDownloadUrl, defaultCachePath, resolveLatestVersion, @@ -84,6 +90,9 @@ const coverageReports = new CoverageReportStore({ const coverageErrorFolders = new Set(); let statusBarItem: vscode.StatusBarItem | null = null; let binaryPath: string | null = null; +// C-34: where the running CLI came from, so the shell PATH command knows +// whether the user already has a CLI of their own. +let lastPlan: BinaryPlan | null = null; const rateLimiter = new NotificationRateLimiter({ windowMs: 60_000 }); let treeProvider: SpecterTreeProvider | null = null; let specterTreeView: vscode.TreeView | null = null; @@ -143,7 +152,14 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { // One-time prompt for existing users (and anyone whose rc file doesn't // include ~/.specter/bin): offer to run the shell-path command so the // CLI works from external terminals. Non-blocking — fire and forget. - void maybePromptAddCliToShellPath(ctx, specterBinDir); + // C-34: only when the shell PATH command would have something to do. + // A user with their own CLI on PATH, in range or out, has nothing to add. + const shellDecision = lastPlan ? shellInstallDecision(lastPlan, defaultCachePath()) : null; + if (shellDecision?.addPath) { + // The prompt offers what the command will do: install only when the + // decision says so, not merely because the extension runs its own copy. + void maybePromptAddCliToShellPath(ctx, specterBinDir, shellDecision.install); + } // If the workspace has no specs or manifest, we're done. Commands are // registered, the binary is available, the walkthrough fired if needed. @@ -284,8 +300,21 @@ function teardownFolder(folder: vscode.WorkspaceFolder): void { async function resolveBinary(ctx: vscode.ExtensionContext): Promise { const cfg = vscode.workspace.getConfiguration('specter'); const workspaceSetting = cfg.get('binaryPath') || null; + const nodeFs = require('fs'); - const { resolved, source } = resolveBinaryPath({ + const range = ctx.extension.packageJSON?.specterCli?.range as string | undefined; + if (!range) { + vscode.window.showErrorMessage( + 'Specter: this extension build declares no supported CLI range (package.json specterCli.range). Reinstall the extension.', + { modal: true }, + ); + return null; + } + const privateVersion = await resolvePrivateVersion(ctx); + if (!privateVersion) return null; + + // C-34: the decision is a pure plan. Everything below only carries it out. + const plan = planBinaryResolution({ workspaceSetting, which: name => { try { @@ -294,94 +323,91 @@ async function resolveBinary(ctx: vscode.ExtensionContext): Promise { try { require('fs').accessSync(p); return true; } catch { return false; } }, + exists: p => { try { nodeFs.accessSync(p); return true; } catch { return false; } }, isExecutable: p => { - try { require('fs').accessSync(p, require('fs').constants.X_OK); return true; } + try { nodeFs.accessSync(p, nodeFs.constants.X_OK); return true; } catch { return false; } }, }, - cachePath: defaultCachePath(), + // C-01: a candidate counts only if it is a real binary that answers + // --version. A corrupt file on PATH is not a candidate, wherever it is. + probeVersion: p => (isBinaryFile(p) ? getCachedBinaryVersion(p) : null), + userBinPath: defaultCachePath(), + privateDir: privateCliDir(), + privateVersion, + range, + platform: process.platform, }); + lastPlan = plan; - const cachePath = defaultCachePath(); - - if (resolved) { - // Always validate the resolved binary — regardless of source. A corrupt - // file in ~/.specter/bin that also happens to be on the shell PATH would - // otherwise slip through as source='path' and every specter invocation - // would fail silently. See issue: https://github.com/Hanalyx/specter/issues - if (!isBinaryFile(resolved) || !getCachedBinaryVersion(resolved)) { - // If the corrupt file is the cache path we own, delete it and fall - // through to auto-download. Otherwise it's user-provided (workspace - // setting or something else on PATH) — don't touch it, just prompt. - if (resolved === cachePath) { - try { require('fs').unlinkSync(resolved); } catch { /* ignore */ } - // fall through to auto-download - } else { - const pick = await vscode.window.showErrorMessage( - `Specter binary at ${resolved} (via ${source}) is not a valid executable. ` + - `It may be a corrupt download or a stale file. Re-download to ${cachePath}?`, - 'Re-download', 'Cancel', - ); - if (pick === 'Re-download') { - return downloadBinary(ctx); - } - return null; - } - } else { - // Valid binary. Auto-update if CLI version != extension version. - const cliVersion = getCachedBinaryVersion(resolved); - const extVersion = vscode.extensions.getExtension('Hanalyx.specter-vscode')?.packageJSON?.version as string | undefined; - if (cliVersion && extVersion && cliVersion !== extVersion) { - const autoDownload = cfg.get('autoDownload', true); - if (autoDownload) { - const updated = await downloadBinary(ctx); - if (updated) return updated; - } - } - return resolved; + for (const sk of plan.skipped) { + outputChannel?.appendLine( + `Specter: not using ${sk.path} (version ${sk.version}); this extension supports ${sk.range}. ` + + 'The file was left unchanged. The extension uses its own copy instead.', + ); + } + + if (plan.source === 'workspace-setting') { + // C-01: validated regardless of source. A path the user named is never + // modified; an invalid one is reported and the run stops. + if (!isBinaryFile(plan.resolved) || !getCachedBinaryVersion(plan.resolved)) { + vscode.window.showErrorMessage( + `Specter binary at ${plan.resolved} (via specter.binaryPath) is not a valid executable. Fix or clear the setting.`, + { modal: true }, + ); + return null; } + return plan.resolved; } - // Auto-download - const autoDownload = cfg.get('autoDownload', true); - if (!autoDownload) { + if (!plan.download) return plan.resolved; + + if (!cfg.get('autoDownload', true)) { vscode.window.showErrorMessage( 'Specter binary not found. Set specter.binaryPath or enable specter.autoDownload.', { modal: true }, ); return null; } + // The private copy is the extension's own: a stale or corrupt file there + // is replaced. Nothing else is ever unlinked from here. + try { nodeFs.unlinkSync(plan.download.target); } catch { /* absent */ } + return downloadBinary(ctx, plan.download); +} - return downloadBinary(ctx); +/** + * C-27: the version the private copy should be. The extension's own version + * unless specter.version names another or asks for the latest release. + */ +async function resolvePrivateVersion(ctx: vscode.ExtensionContext): Promise { + const versionSetting = vscode.workspace.getConfiguration('specter').get('version', ''); + if (versionSetting === 'latest') { + try { + return await resolveLatestVersion(); + } catch (e) { + vscode.window.showErrorMessage(`Specter: could not resolve the latest release: ${e}`, { modal: true }); + return null; + } + } + if (versionSetting) return versionSetting; + return ctx.extension.packageJSON.version as string; } -async function downloadBinary(ctx: vscode.ExtensionContext): Promise { - const cfg = vscode.workspace.getConfiguration('specter'); - const versionSetting = cfg.get('version', ''); +async function downloadBinary(ctx: vscode.ExtensionContext, target: { version: string; target: string }): Promise { return vscode.window.withProgress( { location: vscode.ProgressLocation.Notification, title: 'Downloading Specter CLI…', cancellable: false }, async (progress) => { try { - // 1. Resolve version — default pins the CLI to the extension's own - // version, so v0.10.0 VSIX always fetches v0.10.0 CLI. 'latest' opts - // in to whatever GitHub's /releases/latest points at, and any other - // string is treated as a pinned semver. + // 1. The version and the target come from the plan (C-34): always + // the private copy, never the user's. progress.report({ message: 'resolving version…' }); - let version: string; - if (versionSetting === 'latest') { - version = await resolveLatestVersion(); - } else if (versionSetting) { - version = versionSetting; - } else { - version = ctx.extension.packageJSON.version as string; - } + const version = target.version; // 2. Build download URL const dlOpts = { version, os: process.platform, arch: process.arch }; const url = buildDownloadUrl(dlOpts); - const targetPath = defaultCachePath(); + const targetPath = target.target; const archiveName = assetName(dlOpts); const format: 'tar.gz' | 'zip' = process.platform === 'win32' ? 'zip' : 'tar.gz'; @@ -470,6 +496,7 @@ const ADD_PATH_PROMPT_DISMISSED_KEY = 'specter.addPathPromptDismissed'; async function maybePromptAddCliToShellPath( ctx: vscode.ExtensionContext, binDir: string, + installFirst: boolean, ): Promise { const fs = require('fs'); @@ -484,8 +511,10 @@ async function maybePromptAddCliToShellPath( if (!shouldPromptAddPath(rcContents, binDir, dismissed)) return; const pick = await vscode.window.showInformationMessage( - `Specter CLI is installed at ${binDir} but not on your shell PATH. ` + - `Run \`specter\` from external terminals by adding it to ${cfg.rcFile}.`, + installFirst + ? `The Specter CLI is available to VS Code but not to your shell. Install a copy at ${binDir} and add it to ${cfg.rcFile}?` + : `Specter CLI is installed at ${binDir} but not on your shell PATH. ` + + `Run \`specter\` from external terminals by adding it to ${cfg.rcFile}.`, 'Add to PATH', "Don't show again", ); @@ -1130,7 +1159,7 @@ function registerDiagnosticHooks(ctx: vscode.ExtensionContext): void { ); } else { const terminal = vscode.window.createTerminal('Specter Diff'); - terminal.sendText(`specter diff ${fsPath}@HEAD ${fsPath}`); + terminal.sendText(terminalInvocation(binaryPath, `diff ${fsPath}@HEAD ${fsPath}`)); terminal.show(); } } @@ -1352,7 +1381,7 @@ function registerCommands(ctx: vscode.ExtensionContext): void { }); terminal.show(); // Don't execute — let the user pick the source directory. - terminal.sendText('specter reverse ', false); + terminal.sendText(terminalInvocation(binaryPath, 'reverse '), false); }), ); @@ -1395,6 +1424,20 @@ function registerCommands(ctx: vscode.ExtensionContext): void { vscode.commands.registerCommand('specter.addCliToShellPath', async () => { const fs = require('fs'); const binDir = path.dirname(defaultCachePath()); + + // C-34: the one permitted write to the user's copy, and only when the + // extension is running its own copy. A CLI already on PATH is the + // user's; putting ~/.specter/bin ahead of it would shadow it. + const decision = lastPlan ? shellInstallDecision(lastPlan, defaultCachePath()) : null; + if (!decision || !decision.addPath) { + vscode.window.showInformationMessage(`Specter: ${decision?.reason ?? 'no CLI is resolved yet. Nothing was installed.'}`); + return; + } + if (decision.install && binaryPath) { + const install = installUserCopy(binaryPath, defaultCachePath()); + outputChannel?.appendLine(`Specter: ${install.message}`); + } + const shell = process.env.SHELL || ''; const cfg = detectShellConfig({ shell, platform: process.platform, home: os.homedir() }, binDir); @@ -1441,9 +1484,13 @@ function registerCommands(ctx: vscode.ExtensionContext): void { // downloadBinary always writes a new copy, then re-runs activation wiring. ctx.subscriptions.push( vscode.commands.registerCommand('specter.redownloadCli', async () => { - const cachePath = defaultCachePath(); - try { require('fs').unlinkSync(cachePath); } catch { /* ignore */ } - const resolved = await downloadBinary(ctx); + // C-34: refreshes the extension's own copy and nothing else. The + // user's ~/.specter/bin/specter is not touched. + const version = await resolvePrivateVersion(ctx); + if (!version) return; + const plan = planRedownload({ privateDir: privateCliDir(), version, platform: process.platform }); + try { require('fs').unlinkSync(plan.download.target); } catch { /* ignore */ } + const resolved = await downloadBinary(ctx, plan.download); if (!resolved) return; binaryPath = resolved; for (const folder of (vscode.workspace.workspaceFolders ?? [])) {