[PR #7] [MERGED] fix(installer): wire Amp via settings.json instead of unsupported CLI flags #8

New issue

Closed

opened 2026-10-26 22:27:48 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:27:48 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-cloud-mcp/pull/7
Author: @mattmillerai
Created: 5/5/2026
Status: ✅ Merged
Merged: 5/5/2026
Merged by: @mattmillerai

Base: main ← Head: mattmiller/be-712-amp-installer-fix

📝 Commits (1)

1f4ea12 fix(installer): wire Amp via settings.json instead of unsupported CLI flags

📊 Changes

2 files changed (+85 additions, -11 deletions)

View changed files

📝 install.ps1 (+58 -1)
📝 install.sh (+27 -10)

📄 Description

Summary

Amp's mcp add CLI auto-detects transport from the URL and has no flag for HTTP headers, so the previous --transport http -H "X-API-Key: ..." invocation always exited non-zero and dumped users to manual setup. &>/dev/null was hiding the actual error.
Write the server entry directly to ~/.config/amp/settings.json (macOS/Linux) or %APPDATA%\amp\settings.json (Windows) under amp.mcpServers.comfyui-cloud, where Amp does support headers. Errors from jq / JSON parsing now surface to the user instead of being swallowed.
install.ps1 previously had no Amp config at all; adds Test-Amp / Get-AmpConfigPath (with AMP_SETTINGS_FILE env override) / Install-Amp mirroring the existing Install-ClaudeDesktop hashtable-merge pattern.

Test plan

bash -n install.sh — passes
configure_amp against fresh config dir → writes correct JSON shape
configure_amp against existing settings.json with unrelated keys (editor.fontSize, an existing linear server) → preserves them, adds comfyui-cloud alongside
configure_amp against malformed JSON → refuses to clobber, surfaces jq parse error to user
Re-running on an existing comfyui-cloud entry → overwrites cleanly (jq assignment semantics)
End-to-end run on a machine with Amp installed; verify comfyui-cloud shows up in amp mcp list and the server is reachable from a fresh Amp session
install.ps1 end-to-end on Windows (mirrors known-good Install-ClaudeDesktop pattern but was not run-tested from macOS)

Notes

Adds jq as a runtime dependency of install.sh. Most Linux distros ship it; Homebrew users almost always have it. Falls back gracefully with a clear "install jq and re-run" message if missing — does not abort the rest of the installer.
The earlier commit f7955d5 fix: suppress verbose claude/amp CLI output during install is what hid the underlying error and made this hard to diagnose; this PR keeps Claude's &>/dev/null (it works) but replaces the Amp path with one whose errors are useful.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-cloud-mcp/pull/7 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 5/5/2026 **Status:** ✅ Merged **Merged:** 5/5/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `mattmiller/be-712-amp-installer-fix` --- ### 📝 Commits (1) - [`1f4ea12`](https://github.com/Comfy-Org/comfy-cloud-mcp/commit/1f4ea123169c09d24cb73bcc0a13acb90d16ef23) fix(installer): wire Amp via settings.json instead of unsupported CLI flags ### 📊 Changes **2 files changed** (+85 additions, -11 deletions) <details> <summary>View changed files</summary> 📝 `install.ps1` (+58 -1) 📝 `install.sh` (+27 -10) </details> ### 📄 Description ## Summary - Amp's `mcp add` CLI auto-detects transport from the URL and has no flag for HTTP headers, so the previous `--transport http -H "X-API-Key: ..."` invocation always exited non-zero and dumped users to manual setup. `&>/dev/null` was hiding the actual error. - Write the server entry directly to `~/.config/amp/settings.json` (macOS/Linux) or `%APPDATA%\amp\settings.json` (Windows) under `amp.mcpServers.comfyui-cloud`, where Amp does support `headers`. Errors from jq / JSON parsing now surface to the user instead of being swallowed. - `install.ps1` previously had no Amp config at all; adds `Test-Amp` / `Get-AmpConfigPath` (with `AMP_SETTINGS_FILE` env override) / `Install-Amp` mirroring the existing `Install-ClaudeDesktop` hashtable-merge pattern. ## Test plan - [x] `bash -n install.sh` — passes - [x] `configure_amp` against fresh config dir → writes correct JSON shape - [x] `configure_amp` against existing settings.json with unrelated keys (`editor.fontSize`, an existing `linear` server) → preserves them, adds `comfyui-cloud` alongside - [x] `configure_amp` against malformed JSON → refuses to clobber, surfaces jq parse error to user - [x] Re-running on an existing `comfyui-cloud` entry → overwrites cleanly (jq assignment semantics) - [ ] End-to-end run on a machine with Amp installed; verify `comfyui-cloud` shows up in `amp mcp list` and the server is reachable from a fresh Amp session - [ ] `install.ps1` end-to-end on Windows (mirrors known-good `Install-ClaudeDesktop` pattern but was not run-tested from macOS) ## Notes - Adds `jq` as a runtime dependency of `install.sh`. Most Linux distros ship it; Homebrew users almost always have it. Falls back gracefully with a clear "install jq and re-run" message if missing — does not abort the rest of the installer. - The earlier commit `f7955d5 fix: suppress verbose claude/amp CLI output during install` is what hid the underlying error and made this hard to diagnose; this PR keeps Claude's `&>/dev/null` (it works) but replaces the Amp path with one whose errors are useful. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:27:48 +00:00