feat(mcp): qsv installer refactor#3609
Merged
jqnatividad merged 10 commits intomasterfrom Mar 13, 2026
Merged
Conversation
…irect download The qsv_setup installer previously tried Homebrew/Scoop which fails in GUI environments (Claude Desktop) due to PATH issues. Now downloads the qsvmcp binary directly from GitHub Releases API, extracts it, and installs to a discoverable location (/usr/local/bin or ~/.local/bin on Unix, %LOCALAPPDATA%\Programs\qsv on Windows). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add missing ASSET_SUFFIXES for linux-x64, linux-arm64, and darwin-x64 so direct download works on Linux and Intel Mac (not just macOS ARM) - Replace write-then-delete writability check with fs.accessSync(W_OK) to avoid leaving stale temp files on crash - Fix Unix zip extraction to handle nested directory structures by extracting full archive then searching for binary (matching Windows behavior) - Use test.skip with reason messages instead of silent platform guards so CI reports clearly show skipped vs passed tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Guard against empty-string result from /usr/bin/find when no binary is found in the extracted zip, and add a test verifying that unsupported platform keys (freebsd, sunos, aix) return null from the asset suffix lookup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…uard Export ASSET_SUFFIXES from installer.ts so the unsupported-platform test validates the actual map instead of a duplicated local copy. Also remove the redundant `&& findResult.length > 0` check since an empty string after .trim() is already falsy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… does not support
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reworks the MCP “qsv setup” installer flow to use direct binary download/extraction (instead of package managers), and updates tool hints and tests accordingly.
Changes:
- Replace package-manager detection/installation with a GitHub Releases “latest” downloader that installs
qsvmcp. - Update manual installation instructions to remove Homebrew/Scoop references.
- Expand installer unit tests to cover asset suffix selection and install directory logic.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| .claude/skills/src/installer.ts | Implements direct-download install path, platform suffix mapping, and updated manual instructions. |
| .claude/skills/src/mcp-tools.ts | Updates qsv_setup tool description to reflect a new installation approach. |
| .claude/skills/tests/installer.test.ts | Replaces package-manager tests with suffix/install-dir/manual-instructions assertions. |
…" default SkillExecutor and FilesystemResourceProvider both hardcoded "qsv" as the default binary name, causing ENOENT failures when only qsvmcp is installed. Now both fall back to config.qsvBinPath which auto-detects qsvmcp or qsv. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The FilesystemConfig.qsvBinPath comment still said "default: 'qsv'" after the previous commit switched to auto-detected paths via appConfig. Also verified the two review findings: - maxFilesPerListing: FilesystemConfig never declared this field, so the rename to appConfig.maxFilesPerListing is correct (no regression). - qsvBinPath fallback chain: config.qsvBinPath || appConfig.qsvBinPath is necessary since mcp-server.ts passes qsvBinPath in the constructor. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests for macOS x64, Linux x64, and Linux ARM64 still expected asset suffixes after those platforms were removed from ASSET_SUFFIXES. Now they correctly expect null, fixing the CI failure on ubuntu-latest. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…and paths - Add User-Agent header and AbortSignal timeout to GitHub API and download fetch calls, matching the pattern in update-checker.ts - Escape single quotes in PowerShell string interpolation to prevent injection from paths containing apostrophes (e.g. O'Connor) - Use os.homedir() instead of env var fallbacks (USERPROFILE/"", HOME/"/tmp") for reliable install directory resolution - Fix macOS manual instructions to specify aarch64-apple-darwin asset name - Fix setup tool hint from "qsv.dathere.com" to "GitHub Releases" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…g of fixed binaryName Moves psEscape helper from inside the try block to module scope (avoids unnecessary allocation on non-Windows platforms) and drops the redundant psEscape(binaryName) call since binaryName is always "qsvmcp"/"qsvmcp.exe". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.