Compare commits
27
Commits
8fdbe90b37
..
main
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f73b49f17c | ||
|
|
451e04cd96 | ||
|
|
675490e3c3 | ||
|
|
3768113938 | ||
|
|
583e4e4af0 | ||
|
|
38e9cf2b26 | ||
|
|
d4ce2647ae | ||
|
|
ece8c99f79 | ||
|
|
66b0101dea | ||
|
|
c5a6bdd1d1 | ||
|
|
ded1eef2dd | ||
|
|
cdda60da1b | ||
|
|
0920846c2c | ||
|
|
ce82f7b5c3 | ||
|
|
d2c126a60c | ||
|
|
603d24566d | ||
|
|
0b2827e6b8 | ||
|
|
2e5d0351b4 | ||
|
|
2cd8e0fb3b | ||
|
|
8c51c25211 | ||
|
|
e087107710 | ||
|
|
436524bf00 | ||
|
|
e542ff6e8f | ||
|
|
dc9e035781 | ||
|
|
7368dcdbff | ||
|
|
694439b6f3 | ||
|
|
4743c4a995 |
@@ -3,13 +3,16 @@ name: CI
|
|||||||
on:
|
on:
|
||||||
push:
|
push:
|
||||||
branches: [main]
|
branches: [main]
|
||||||
|
paths-ignore: ["**/*.md"]
|
||||||
pull_request:
|
pull_request:
|
||||||
|
paths-ignore: ["**/*.md"]
|
||||||
workflow_dispatch:
|
workflow_dispatch:
|
||||||
|
|
||||||
jobs:
|
jobs:
|
||||||
lint:
|
lint:
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
name: Lint (ruff)
|
name: Lint (ruff)
|
||||||
|
timeout-minutes: 10
|
||||||
steps:
|
steps:
|
||||||
- name: Checkout
|
- name: Checkout
|
||||||
uses: actions/checkout@v4
|
uses: actions/checkout@v4
|
||||||
@@ -31,6 +34,11 @@ jobs:
|
|||||||
test:
|
test:
|
||||||
runs-on: ${{ matrix.os }}
|
runs-on: ${{ matrix.os }}
|
||||||
name: Tests (py${{ matrix.python }} / ${{ matrix.os }})
|
name: Tests (py${{ matrix.python }} / ${{ matrix.os }})
|
||||||
|
# Guards against a job that hangs mid-run (e.g. a wedged test). Note: this
|
||||||
|
# counts from when a runner PICKS UP the job, so it does not rescue a job
|
||||||
|
# stuck "Waiting to run" because the self-hosted Windows runner is offline —
|
||||||
|
# for that, bring the runner back or skip via paths-ignore (docs).
|
||||||
|
timeout-minutes: 15
|
||||||
strategy:
|
strategy:
|
||||||
fail-fast: false
|
fail-fast: false
|
||||||
matrix:
|
matrix:
|
||||||
@@ -85,6 +93,7 @@ jobs:
|
|||||||
catalog-signature:
|
catalog-signature:
|
||||||
name: Catalog signature
|
name: Catalog signature
|
||||||
runs-on: ubuntu-latest
|
runs-on: ubuntu-latest
|
||||||
|
timeout-minutes: 10
|
||||||
steps:
|
steps:
|
||||||
- uses: actions/checkout@v4
|
- uses: actions/checkout@v4
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,91 @@
|
|||||||
|
# Changelog
|
||||||
|
|
||||||
|
All notable changes to **BetterClaudeConfig** are recorded here. The format is
|
||||||
|
based on [Keep a Changelog](https://keepachangelog.com/), and this project
|
||||||
|
follows [Semantic Versioning](https://semver.org/): breaking changes bump the
|
||||||
|
major, new features bump the minor, fixes bump the patch.
|
||||||
|
|
||||||
|
**When you open a PR, add a line under `[Unreleased]`.** At release time, that
|
||||||
|
section is renamed to the new version + date and a fresh `[Unreleased]` is
|
||||||
|
started.
|
||||||
|
|
||||||
|
## [Unreleased]
|
||||||
|
|
||||||
|
## [1.4.0] — 2026-08-13
|
||||||
|
|
||||||
|
> ⚠️ **Using the `ssh-mcp` server?** Upstream `ssh-mcp` shipped a **breaking
|
||||||
|
> v2**: it removed the `--password`, `--sudoPassword`, `--suPassword` and
|
||||||
|
> `--disableSudo` command-line flags, and it now reads a `config.toml` sidecar
|
||||||
|
> file that overrides your command-line arguments. A config written for v1
|
||||||
|
> crashes on v2's startup. This release adds tools to detect and fix that.
|
||||||
|
> **BetterClaudeConfig itself has no breaking changes** — your existing configs
|
||||||
|
> keep working; nothing is changed without your click.
|
||||||
|
|
||||||
|
### Added
|
||||||
|
- **ssh-mcp v2 flag migration.** Detects the credential flags v2 removed and
|
||||||
|
one-click-moves each into the environment variable ssh-mcp now reads
|
||||||
|
(`--password` → `SSH_MCP_PASSWORD`; `--sudoPassword` / `--suPassword` →
|
||||||
|
`SSH_MCP_SUDO_PASSWORD`). Also flags `--maxChars=none`, whose meaning changed
|
||||||
|
between versions.
|
||||||
|
- **Sidecar-config awareness.** When a server reads a separate config file
|
||||||
|
(ssh-mcp's `config.toml`), BCC shows where that file actually lives on your
|
||||||
|
platform and warns when your command-line args are **inert** because the file
|
||||||
|
takes precedence — including when a file sits at the wrong, documented-but-
|
||||||
|
unused path.
|
||||||
|
- **In-app sidecar editor.** Edit that external config from inside BCC —
|
||||||
|
pick-lists for known fields, a raw-text fallback — written atomically with a
|
||||||
|
timestamped backup and locked to `0600`. No dropping to a terminal.
|
||||||
|
- **Live hot-reload.** External changes to a sidecar (or to your Claude config)
|
||||||
|
now surface without restarting BCC.
|
||||||
|
- **Version pinning for `npx` servers.** Spots servers launched with `-y` /
|
||||||
|
`@latest` that resolve a fresh version every run, shows the version currently
|
||||||
|
resolved, and offers a one-click **Pin to this version** — with a drift note
|
||||||
|
when a pin has fallen behind.
|
||||||
|
- **Permission pre-flight.** Warns when a credential-bearing config is readable
|
||||||
|
by other users on the machine and offers a one-click fix to `0600`/`0700`.
|
||||||
|
- **Light theme + system-following** (the dark theme is preserved exactly).
|
||||||
|
- **"Move to environment variable."** Convert a plaintext secret in a config
|
||||||
|
into a `${VAR}` reference in place — offered only on clients that actually
|
||||||
|
expand references, so it can't silently break a Claude Desktop config.
|
||||||
|
- **Cross-client foundation.** Claude Desktop and Claude Code now flow through a
|
||||||
|
single adapter — groundwork for supporting more clients.
|
||||||
|
|
||||||
|
### Changed
|
||||||
|
- The update checker is now **visible** — a persistent banner plus a Help-menu
|
||||||
|
item — instead of being buried in the About dialog.
|
||||||
|
- Config writes share one atomic-write path; the temp file is created `0600`, so
|
||||||
|
a secret is never briefly world-readable mid-write.
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
- Loading a config with a non-object server value no longer crashes, and named
|
||||||
|
server sets survive an external-change merge.
|
||||||
|
- Project profiles that share a directory basename are disambiguated, so you
|
||||||
|
can't accidentally edit the wrong `.mcp.json`.
|
||||||
|
|
||||||
|
### Internal / maintainer
|
||||||
|
- Signed-catalog core and a maintainer-only **Catalog Console** (review + sign),
|
||||||
|
with hardening of the review gate and a split of the signing keys. No
|
||||||
|
user-facing catalog browser ships yet.
|
||||||
|
|
||||||
|
## [1.3.0] — 2026-07-12
|
||||||
|
Named server sets, Claude Code project `.mcp.json` discovery, structural schema
|
||||||
|
lint, and UX polish (Ctrl+S to save, enable-all / disable-all).
|
||||||
|
|
||||||
|
## [1.2.1] — 2026-07-12
|
||||||
|
First shipped binaries: app-icon fix, a batch of audit fixes, and Windows
|
||||||
|
process-tree cleanup on spawn-tests.
|
||||||
|
|
||||||
|
## [1.2.0] — 2026-07-08
|
||||||
|
Server log viewer, duplicate-name conflict handling, a Restart-Claude button,
|
||||||
|
stale-file protection, an About dialog, and a notify-only update checker.
|
||||||
|
|
||||||
|
## [1.1.0] — 2026-07-04
|
||||||
|
Backup / restore UI and secret masking.
|
||||||
|
|
||||||
|
## [1.0.1] — 2026-06-29
|
||||||
|
## [1.0.0] — 2026-06-29
|
||||||
|
Initial releases: the core `mcpServers` editor with the lenient paste/repair
|
||||||
|
pipeline that tolerates malformed JSON.
|
||||||
|
|
||||||
|
<!-- Backfill for 1.0.0–1.3.0 is summarised from release notes; the Unreleased
|
||||||
|
section onward is maintained per-PR. -->
|
||||||
@@ -0,0 +1,50 @@
|
|||||||
|
# PR #87 — "Move to environment variable" (#83): manual test checklist
|
||||||
|
|
||||||
|
The logic is covered by 15 unit tests in CI; what CI **can't** exercise is the GUI (no PySide6). This checklist is only the parts a human needs to click. Should take ~10 minutes.
|
||||||
|
|
||||||
|
## Setup
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd ~/Documents/Claude/Projects/BetterClaudeConfig/better-claude-config
|
||||||
|
git fetch origin
|
||||||
|
git checkout feat/83-move-to-env-var
|
||||||
|
git pull # ensure you're on 8fdbe90 or later
|
||||||
|
source .venv/bin/activate # or recreate: python3 -m venv .venv && source .venv/bin/activate && pip install -r requirements.txt
|
||||||
|
python bcc.py
|
||||||
|
```
|
||||||
|
|
||||||
|
Pick a **Claude Code** profile (e.g. `~/.claude.json`) that has, or add, a server with an env value that looks like a secret — e.g. `env: { "API_KEY": "ghp_test123" }`. (You can use a throwaway value; nothing is sent anywhere.)
|
||||||
|
|
||||||
|
## The checklist
|
||||||
|
|
||||||
|
### Gating — where the action appears
|
||||||
|
- [ ] Right-click the **value cell** of a secret env row (`API_KEY`) on a **Claude Code** profile → a **"Move to environment variable…"** item appears.
|
||||||
|
- [ ] Right-click a **non-secret** row (e.g. `REGION` = `us-east-1`) → the item does **not** appear.
|
||||||
|
- [ ] Right-click a row whose value is already a reference (`${API_KEY}`) → the item does **not** appear.
|
||||||
|
- [ ] Switch to a **Claude Desktop** profile (a `claude_desktop_config.json`), right-click the same kind of secret row → the item does **not** appear. (Desktop doesn't expand `${VAR}`, so offering it would break the config — this is the important gate.)
|
||||||
|
|
||||||
|
### The dialog
|
||||||
|
- [ ] Trigger the action → dialog opens with **Variable** pre-filled from the key, sanitized to a legal shell name (e.g. `api-key` → `API_KEY`).
|
||||||
|
- [ ] Edit the variable name → the shown **shell line updates live** and matches your platform (`export VAR='…'` on macOS/Linux, `setx VAR "…"` on Windows), with the other platform shown in parentheses.
|
||||||
|
- [ ] If you type a variable name that **is already set** in your shell environment, the green "already looks set" note appears; if not, it's hidden.
|
||||||
|
- [ ] **Cancel** → nothing changes (value still the raw secret, no dirty state).
|
||||||
|
|
||||||
|
### The conversion
|
||||||
|
- [ ] **Move && copy secret** → the cell now shows the reference `${VAR}` (visible, **not** masked to dots), and the window goes dirty (Save enabled).
|
||||||
|
- [ ] Paste from your clipboard somewhere → it's the **original secret value** (handed back before removal).
|
||||||
|
- [ ] The reference value is **not** flagged as a secret warning anymore (it's the recommended state).
|
||||||
|
|
||||||
|
### Headers + persistence
|
||||||
|
- [ ] Repeat on a **remote server's Headers** table (e.g. an `Authorization` header) → same behavior.
|
||||||
|
- [ ] **Save**, then open the config file on disk in a text editor → the servers block holds `${VAR}`, and the **plaintext secret is gone** from the file.
|
||||||
|
- [ ] Re-open the profile in BCC → the row still shows `${VAR}` (round-trips).
|
||||||
|
|
||||||
|
### Undo (nice-to-have)
|
||||||
|
- [ ] After a conversion, **Ctrl+Z / Cmd+Z** restores the previous value.
|
||||||
|
|
||||||
|
## Known scope (not bugs)
|
||||||
|
- **Args rows** are out of scope for this PR — the core supports them, but the args editor is a free-text widget, so wiring that UI is a deliberate follow-up. Right-clicking args won't offer the action yet.
|
||||||
|
- The "already set" check reads **BCC's** environment, which may differ from the client's — it's advisory, worded that way.
|
||||||
|
|
||||||
|
## If anything's off
|
||||||
|
Tell me which checkbox failed and what you saw; I'll fix on the branch and re-push. If everything passes, approve/merge #87 (or tell me to merge it).
|
||||||
@@ -16,7 +16,18 @@ import time
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import ClassVar
|
from typing import ClassVar
|
||||||
|
|
||||||
from PySide6.QtCore import QRect, QSettings, QSize, Qt, QThread, QTimer, QUrl, Signal
|
from PySide6.QtCore import (
|
||||||
|
QEvent,
|
||||||
|
QFileSystemWatcher,
|
||||||
|
QRect,
|
||||||
|
QSettings,
|
||||||
|
QSize,
|
||||||
|
Qt,
|
||||||
|
QThread,
|
||||||
|
QTimer,
|
||||||
|
QUrl,
|
||||||
|
Signal,
|
||||||
|
)
|
||||||
from PySide6.QtGui import (
|
from PySide6.QtGui import (
|
||||||
QAction,
|
QAction,
|
||||||
QActionGroup,
|
QActionGroup,
|
||||||
@@ -38,8 +49,10 @@ from PySide6.QtWidgets import (
|
|||||||
QDialog,
|
QDialog,
|
||||||
QDialogButtonBox,
|
QDialogButtonBox,
|
||||||
QFileDialog,
|
QFileDialog,
|
||||||
|
QFormLayout,
|
||||||
QFrame,
|
QFrame,
|
||||||
QGridLayout,
|
QGridLayout,
|
||||||
|
QGroupBox,
|
||||||
QHBoxLayout,
|
QHBoxLayout,
|
||||||
QHeaderView,
|
QHeaderView,
|
||||||
QInputDialog,
|
QInputDialog,
|
||||||
@@ -52,6 +65,7 @@ from PySide6.QtWidgets import (
|
|||||||
QMessageBox,
|
QMessageBox,
|
||||||
QPlainTextEdit,
|
QPlainTextEdit,
|
||||||
QPushButton,
|
QPushButton,
|
||||||
|
QSpinBox,
|
||||||
QSplitter,
|
QSplitter,
|
||||||
QStackedWidget,
|
QStackedWidget,
|
||||||
QStyledItemDelegate,
|
QStyledItemDelegate,
|
||||||
@@ -291,9 +305,10 @@ class MoveToEnvDialog(QDialog):
|
|||||||
v.setSpacing(10)
|
v.setSpacing(10)
|
||||||
|
|
||||||
intro = QLabel(
|
intro = QLabel(
|
||||||
"The value will be removed from the config and replaced with a "
|
"This replaces the value in place with a ${VAR} reference. The secret "
|
||||||
"reference. Set the variable in your environment first, or the "
|
"moves to your shell/OS environment — not this config file, and not the "
|
||||||
"server won't authenticate."
|
"Environment variables table below. Run the line below to set it there, "
|
||||||
|
"or the server won't authenticate."
|
||||||
)
|
)
|
||||||
intro.setWordWrap(True)
|
intro.setWordWrap(True)
|
||||||
v.addWidget(intro)
|
v.addWidget(intro)
|
||||||
@@ -356,6 +371,141 @@ class MoveToEnvDialog(QDialog):
|
|||||||
self._already.hide()
|
self._already.hide()
|
||||||
|
|
||||||
|
|
||||||
|
class MoveArgToEnvDialog(QDialog):
|
||||||
|
"""Confirm relocating a secret arg into the env block (#83, kept in file).
|
||||||
|
|
||||||
|
Unlike the reference move, this keeps the value in the config -- it just
|
||||||
|
moves it out of the argument list (visible in process listings) and into
|
||||||
|
the Environment variables table, where the user can see and edit it. It
|
||||||
|
changes how the server is launched, so it says so plainly.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def __init__(self, parent, key: str, value: str):
|
||||||
|
super().__init__(parent)
|
||||||
|
self.setWindowTitle("Move into environment variables")
|
||||||
|
self.setMinimumWidth(460)
|
||||||
|
v = QVBoxLayout(self)
|
||||||
|
v.setSpacing(10)
|
||||||
|
|
||||||
|
intro = QLabel(
|
||||||
|
"This moves the secret out of the arguments and into the Environment "
|
||||||
|
"variables table below, where you can see and edit its value. The value "
|
||||||
|
"stays in this config file."
|
||||||
|
)
|
||||||
|
intro.setWordWrap(True)
|
||||||
|
v.addWidget(intro)
|
||||||
|
|
||||||
|
warn = QLabel(
|
||||||
|
"⚠ This changes how the server is launched: the flag is dropped and the "
|
||||||
|
"value is set as an environment variable instead. It only works if the "
|
||||||
|
"server reads this secret from that variable."
|
||||||
|
)
|
||||||
|
warn.setWordWrap(True)
|
||||||
|
warn.setStyleSheet(f"color: {WARN};")
|
||||||
|
v.addWidget(warn)
|
||||||
|
|
||||||
|
grid = QGridLayout()
|
||||||
|
grid.setSpacing(8)
|
||||||
|
lbl = QLabel("Variable:")
|
||||||
|
lbl.setObjectName("muted")
|
||||||
|
grid.addWidget(lbl, 0, 0)
|
||||||
|
self._name_edit = QLineEdit(core.sanitize_env_var_name(key))
|
||||||
|
grid.addWidget(self._name_edit, 0, 1)
|
||||||
|
v.addLayout(grid)
|
||||||
|
|
||||||
|
btns = QDialogButtonBox(
|
||||||
|
QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel
|
||||||
|
)
|
||||||
|
ok = btns.button(QDialogButtonBox.StandardButton.Ok)
|
||||||
|
ok.setText("Move into env")
|
||||||
|
ok.setObjectName("primary")
|
||||||
|
btns.accepted.connect(self.accept)
|
||||||
|
btns.rejected.connect(self.reject)
|
||||||
|
v.addWidget(btns)
|
||||||
|
|
||||||
|
def var_name(self) -> str:
|
||||||
|
return core.sanitize_env_var_name(self._name_edit.text())
|
||||||
|
|
||||||
|
|
||||||
|
class ReferencedVarsDialog(QDialog):
|
||||||
|
"""Show every ${VAR} the loaded server references and whether it's set (#83).
|
||||||
|
|
||||||
|
After a secret becomes a reference, the variable lives in the user's
|
||||||
|
environment, not the config -- so this is where they confirm it exists and
|
||||||
|
get the command to set it. Read-only; BCC can't (and shouldn't) store the
|
||||||
|
value.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def __init__(self, parent, data: dict):
|
||||||
|
super().__init__(parent)
|
||||||
|
self.setWindowTitle("Referenced variables")
|
||||||
|
self.setMinimumWidth(560)
|
||||||
|
v = QVBoxLayout(self)
|
||||||
|
v.setSpacing(10)
|
||||||
|
|
||||||
|
self._usages = core.referenced_env_vars(data)
|
||||||
|
if not self._usages:
|
||||||
|
v.addWidget(QLabel("This server references no ${VAR} variables."))
|
||||||
|
btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
|
||||||
|
btns.rejected.connect(self.reject)
|
||||||
|
btns.accepted.connect(self.accept)
|
||||||
|
v.addWidget(btns)
|
||||||
|
return
|
||||||
|
|
||||||
|
intro = QLabel(
|
||||||
|
"These references are read from your shell/OS environment when the client "
|
||||||
|
"runs. ✓ means it's set in BCC's environment (which may differ from the "
|
||||||
|
"client's) or has a default; ✗ means nothing would fill it."
|
||||||
|
)
|
||||||
|
intro.setWordWrap(True)
|
||||||
|
v.addWidget(intro)
|
||||||
|
|
||||||
|
self._table = QTableWidget(len(self._usages), 3)
|
||||||
|
self._table.setHorizontalHeaderLabels(["Variable", "Status", "Used in"])
|
||||||
|
self._table.horizontalHeader().setSectionResizeMode(0, QHeaderView.ResizeMode.Stretch)
|
||||||
|
self._table.horizontalHeader().setSectionResizeMode(2, QHeaderView.ResizeMode.Stretch)
|
||||||
|
self._table.verticalHeader().setVisible(False)
|
||||||
|
self._table.setSelectionBehavior(QAbstractItemView.SelectionBehavior.SelectRows)
|
||||||
|
self._table.setEditTriggers(QAbstractItemView.EditTrigger.NoEditTriggers)
|
||||||
|
for r, u in enumerate(self._usages):
|
||||||
|
is_set = core.is_env_var_set(u.name)
|
||||||
|
status = "✓ set" if is_set else ("✓ default" if u.has_default else "✗ not set")
|
||||||
|
self._table.setItem(r, 0, QTableWidgetItem(u.name))
|
||||||
|
self._table.setItem(r, 1, QTableWidgetItem(status))
|
||||||
|
self._table.setItem(r, 2, QTableWidgetItem(", ".join(u.fields)))
|
||||||
|
self._table.selectionModel().selectionChanged.connect(self._refresh_cmd)
|
||||||
|
v.addWidget(self._table, 1)
|
||||||
|
|
||||||
|
set_lbl = QLabel("Set the selected variable with:")
|
||||||
|
set_lbl.setObjectName("muted")
|
||||||
|
v.addWidget(set_lbl)
|
||||||
|
self._cmd = QLabel("")
|
||||||
|
self._cmd.setWordWrap(True)
|
||||||
|
self._cmd.setTextInteractionFlags(Qt.TextInteractionFlag.TextSelectableByMouse)
|
||||||
|
self._cmd.setStyleSheet("font-family: monospace;")
|
||||||
|
v.addWidget(self._cmd)
|
||||||
|
|
||||||
|
btns = QDialogButtonBox(QDialogButtonBox.StandardButton.Close)
|
||||||
|
btns.rejected.connect(self.reject)
|
||||||
|
btns.accepted.connect(self.accept)
|
||||||
|
v.addWidget(btns)
|
||||||
|
|
||||||
|
self._table.selectRow(0)
|
||||||
|
|
||||||
|
def _refresh_cmd(self, *_):
|
||||||
|
rows = self._table.selectionModel().selectedRows()
|
||||||
|
if not rows:
|
||||||
|
self._cmd.setText("")
|
||||||
|
return
|
||||||
|
name = self._usages[rows[0].row()].name
|
||||||
|
# A placeholder value -- BCC doesn't hold the secret, this shows the shape.
|
||||||
|
lines = core.shell_export_lines(name, "<value>")
|
||||||
|
if sys.platform == "win32":
|
||||||
|
self._cmd.setText(f"{lines['windows']}\n\n(macOS/Linux: {lines['posix']})")
|
||||||
|
else:
|
||||||
|
self._cmd.setText(f"{lines['posix']}\n\n(Windows: {lines['windows']})")
|
||||||
|
|
||||||
|
|
||||||
class KeyValueTable(QWidget):
|
class KeyValueTable(QWidget):
|
||||||
def __init__(self, key_label="Key", val_label="Value", on_change=None, before_change=None):
|
def __init__(self, key_label="Key", val_label="Value", on_change=None, before_change=None):
|
||||||
super().__init__()
|
super().__init__()
|
||||||
@@ -418,12 +568,23 @@ class KeyValueTable(QWidget):
|
|||||||
return
|
return
|
||||||
row = item.row()
|
row = item.row()
|
||||||
key, value = self._row_key_value(row)
|
key, value = self._row_key_value(row)
|
||||||
profile = self.profile_provider() if self.profile_provider else None
|
# Only a real stored secret is worth moving; nothing to offer otherwise.
|
||||||
if not core.can_move_value_to_env_ref(key, value, profile):
|
if not core.should_mask_value(key, value):
|
||||||
return
|
return
|
||||||
|
profile = self.profile_provider() if self.profile_provider else None
|
||||||
menu = QMenu(self)
|
menu = QMenu(self)
|
||||||
act = QAction("Move to environment variable…", self)
|
act = QAction("Replace with a ${VAR} reference (out of file)…", self)
|
||||||
act.triggered.connect(lambda: self._move_row_to_env(row))
|
if core.can_move_value_to_env_ref(key, value, profile):
|
||||||
|
act.triggered.connect(lambda: self._move_row_to_env(row))
|
||||||
|
else:
|
||||||
|
# Show it disabled with the reason rather than an empty menu, so the
|
||||||
|
# feature is discoverable and Claude Desktop's gating is explained.
|
||||||
|
act.setEnabled(False)
|
||||||
|
act.setText("Replace with ${VAR} reference — unavailable for Claude Desktop")
|
||||||
|
act.setToolTip(
|
||||||
|
"Claude Desktop doesn't expand ${VAR}, so a reference would reach "
|
||||||
|
"the server as literal text."
|
||||||
|
)
|
||||||
menu.addAction(act)
|
menu.addAction(act)
|
||||||
menu.exec(self.table.viewport().mapToGlobal(pos))
|
menu.exec(self.table.viewport().mapToGlobal(pos))
|
||||||
|
|
||||||
@@ -650,6 +811,12 @@ class ServerEditor(QFrame):
|
|||||||
self.logs_btn = QPushButton("View logs")
|
self.logs_btn = QPushButton("View logs")
|
||||||
self.logs_btn.setToolTip("Open this server's MCP log in a read-only, auto-tailing viewer")
|
self.logs_btn.setToolTip("Open this server's MCP log in a read-only, auto-tailing viewer")
|
||||||
self.logs_btn.clicked.connect(self._view_logs)
|
self.logs_btn.clicked.connect(self._view_logs)
|
||||||
|
self.vars_btn = QPushButton("Variables…")
|
||||||
|
self.vars_btn.setToolTip(
|
||||||
|
"Show the ${VAR} references this server uses and whether each is set in "
|
||||||
|
"your environment"
|
||||||
|
)
|
||||||
|
self.vars_btn.clicked.connect(self._show_referenced_vars)
|
||||||
self.details_btn = QPushButton("Details ▸")
|
self.details_btn = QPushButton("Details ▸")
|
||||||
self.details_btn.setCheckable(True)
|
self.details_btn.setCheckable(True)
|
||||||
self.details_btn.toggled.connect(self._toggle_diag)
|
self.details_btn.toggled.connect(self._toggle_diag)
|
||||||
@@ -661,10 +828,32 @@ class ServerEditor(QFrame):
|
|||||||
dep.addWidget(self.test_btn)
|
dep.addWidget(self.test_btn)
|
||||||
dep.addWidget(self.spawn_btn)
|
dep.addWidget(self.spawn_btn)
|
||||||
dep.addWidget(self.logs_btn)
|
dep.addWidget(self.logs_btn)
|
||||||
|
dep.addWidget(self.vars_btn)
|
||||||
dep.addWidget(self.details_btn)
|
dep.addWidget(self.details_btn)
|
||||||
dep.addWidget(recheck)
|
dep.addWidget(recheck)
|
||||||
outer.addLayout(dep)
|
outer.addLayout(dep)
|
||||||
|
|
||||||
|
# Version status row (#92): for an npx-style server, show the currently
|
||||||
|
# resolved version and, when the spec is unpinned, a one-click "pin".
|
||||||
|
# Mirrors the dependency-status surface above. Hidden for everything else.
|
||||||
|
ver = QHBoxLayout()
|
||||||
|
self.ver_dot = QLabel("○")
|
||||||
|
self.ver_label = QLabel("—")
|
||||||
|
self.ver_label.setObjectName("muted")
|
||||||
|
self.ver_label.setWordWrap(True)
|
||||||
|
self.pin_btn = QPushButton("Pin")
|
||||||
|
self.pin_btn.setToolTip(
|
||||||
|
"Pin the package spec to the currently-resolved version so it can't "
|
||||||
|
"change under you on the next launch"
|
||||||
|
)
|
||||||
|
self.pin_btn.clicked.connect(self._pin_version)
|
||||||
|
self.pin_btn.setVisible(False)
|
||||||
|
ver.addWidget(self.ver_dot)
|
||||||
|
ver.addWidget(self.ver_label, 1)
|
||||||
|
ver.addWidget(self.pin_btn)
|
||||||
|
self.ver_row_widgets = (self.ver_dot, self.ver_label, self.pin_btn)
|
||||||
|
outer.addLayout(ver)
|
||||||
|
|
||||||
# Collapsible diagnostics panel
|
# Collapsible diagnostics panel
|
||||||
self.diag_card = QFrame()
|
self.diag_card = QFrame()
|
||||||
self.diag_card.setObjectName("diagCard")
|
self.diag_card.setObjectName("diagCard")
|
||||||
@@ -732,6 +921,53 @@ class ServerEditor(QFrame):
|
|||||||
self.secret_warn.setWordWrap(True)
|
self.secret_warn.setWordWrap(True)
|
||||||
self.secret_warn.hide()
|
self.secret_warn.hide()
|
||||||
v.addWidget(self.secret_warn)
|
v.addWidget(self.secret_warn)
|
||||||
|
# Shown when args carry a CLI flag a package removed in a major version
|
||||||
|
# (e.g. ssh-mcp v2's --password), with a one-click move into env.
|
||||||
|
self.removed_flag_warn = QLabel("")
|
||||||
|
self.removed_flag_warn.setStyleSheet(f"color: {WARN};")
|
||||||
|
self.removed_flag_warn.setWordWrap(True)
|
||||||
|
self.removed_flag_warn.hide()
|
||||||
|
self.removed_flag_fix_btn = QPushButton("Fix: move to environment variables")
|
||||||
|
self.removed_flag_fix_btn.clicked.connect(self._fix_removed_flags)
|
||||||
|
self.removed_flag_fix_btn.hide()
|
||||||
|
rf_row = QHBoxLayout()
|
||||||
|
rf_row.addWidget(self.removed_flag_warn, 1)
|
||||||
|
rf_row.addWidget(self.removed_flag_fix_btn)
|
||||||
|
v.addLayout(rf_row)
|
||||||
|
# Shown when a server reads a config sidecar (ssh-mcp's TOML): the args
|
||||||
|
# may be inert, or a file may sit at the README path the server never
|
||||||
|
# reads. Advisory (#91); #102 adds an "Edit config…" button that opens
|
||||||
|
# the in-app sidecar editor whenever the server has a resolvable sidecar.
|
||||||
|
self.sidecar_warn = QLabel("")
|
||||||
|
self.sidecar_warn.setStyleSheet(f"color: {WARN};")
|
||||||
|
self.sidecar_warn.setWordWrap(True)
|
||||||
|
self.sidecar_warn.hide()
|
||||||
|
self.sidecar_edit_btn = QPushButton("Edit config…")
|
||||||
|
self.sidecar_edit_btn.setToolTip(
|
||||||
|
"Open this server's external config file in BCC (edit → atomic save "
|
||||||
|
"with backup → the file is tightened to 0600)"
|
||||||
|
)
|
||||||
|
self.sidecar_edit_btn.clicked.connect(self._edit_sidecar)
|
||||||
|
self.sidecar_edit_btn.hide()
|
||||||
|
sc_row = QHBoxLayout()
|
||||||
|
sc_row.addWidget(self.sidecar_warn, 1)
|
||||||
|
sc_row.addWidget(self.sidecar_edit_btn, 0, Qt.AlignmentFlag.AlignTop)
|
||||||
|
v.addLayout(sc_row)
|
||||||
|
# Shown when the sidecar config is group/other-accessible (#93): ssh-mcp
|
||||||
|
# refuses to start unless it's 0600 / its dir 0700. One-click chmod fix.
|
||||||
|
# POSIX only — hidden on Windows where modes don't apply.
|
||||||
|
self.perm_warn = QLabel("")
|
||||||
|
self.perm_warn.setStyleSheet(f"color: {WARN};")
|
||||||
|
self.perm_warn.setWordWrap(True)
|
||||||
|
self.perm_warn.hide()
|
||||||
|
self.perm_fix_btn = QPushButton("Fix permissions")
|
||||||
|
self.perm_fix_btn.setToolTip("chmod the config file to 0600 and its directory to 0700")
|
||||||
|
self.perm_fix_btn.clicked.connect(self._fix_permissions)
|
||||||
|
self.perm_fix_btn.hide()
|
||||||
|
perm_row = QHBoxLayout()
|
||||||
|
perm_row.addWidget(self.perm_warn, 1)
|
||||||
|
perm_row.addWidget(self.perm_fix_btn)
|
||||||
|
v.addLayout(perm_row)
|
||||||
v.addWidget(self._lbl("Environment variables"))
|
v.addWidget(self._lbl("Environment variables"))
|
||||||
self.env = KeyValueTable(
|
self.env = KeyValueTable(
|
||||||
"Variable", "Value", on_change=self._emit, before_change=self._before_change
|
"Variable", "Value", on_change=self._emit, before_change=self._before_change
|
||||||
@@ -762,10 +998,56 @@ class ServerEditor(QFrame):
|
|||||||
return w
|
return w
|
||||||
|
|
||||||
def set_profile_provider(self, provider):
|
def set_profile_provider(self, provider):
|
||||||
"""Let the env/headers tables gate "move to environment variable" on
|
"""Let the env/headers tables and the args editor gate the secret-move
|
||||||
which client the loaded profile targets (#83)."""
|
actions on which client the loaded profile targets (#83), and wire the
|
||||||
|
args editor's two move actions back to this editor (which owns the whole
|
||||||
|
form, since moving an arg into env touches both fields)."""
|
||||||
self.env.profile_provider = provider
|
self.env.profile_provider = provider
|
||||||
self.headers.profile_provider = provider
|
self.headers.profile_provider = provider
|
||||||
|
self.args.profile_provider = provider
|
||||||
|
self.args.on_move_to_ref = self.move_arg_to_reference
|
||||||
|
self.args.on_move_to_env = self.move_arg_into_env
|
||||||
|
|
||||||
|
# --- secret moves from args (#83) ------------------------------------ #
|
||||||
|
def _reload_from_data(self, new_data: dict):
|
||||||
|
"""Repopulate the form from a transformed data dict and mark dirty."""
|
||||||
|
self.load_entry(core.ServerEntry(self.current_name(), new_data, True))
|
||||||
|
self._emit()
|
||||||
|
|
||||||
|
def move_arg_to_reference(self, index: int):
|
||||||
|
"""Args secret -> ${VAR} reference in place (secret leaves the file)."""
|
||||||
|
data = self.dump_data()
|
||||||
|
args = data.get("args") or []
|
||||||
|
if not (0 <= index < len(args)):
|
||||||
|
return
|
||||||
|
dlg = MoveToEnvDialog(
|
||||||
|
self.window(), core.suggested_env_var_for_arg(args, index), args[index]
|
||||||
|
)
|
||||||
|
if not dlg.exec():
|
||||||
|
return
|
||||||
|
conv = core.move_value_to_env_ref(data, field="args", index=index, var_name=dlg.var_name())
|
||||||
|
if conv is None:
|
||||||
|
return
|
||||||
|
QGuiApplication.clipboard().setText(conv.secret)
|
||||||
|
self._reload_from_data(conv.data)
|
||||||
|
|
||||||
|
def move_arg_into_env(self, index: int):
|
||||||
|
"""Args secret -> env block, kept in this config (visible/editable)."""
|
||||||
|
data = self.dump_data()
|
||||||
|
args = data.get("args") or []
|
||||||
|
if not (0 <= index < len(args)):
|
||||||
|
return
|
||||||
|
default_name = core.suggested_env_var_for_arg(args, index)
|
||||||
|
dlg = MoveArgToEnvDialog(self.window(), default_name, args[index])
|
||||||
|
if not dlg.exec():
|
||||||
|
return
|
||||||
|
new = core.move_arg_to_env_block(data, index, var_name=dlg.var_name())
|
||||||
|
if new is None:
|
||||||
|
return
|
||||||
|
self._reload_from_data(new)
|
||||||
|
|
||||||
|
def _show_referenced_vars(self):
|
||||||
|
ReferencedVarsDialog(self.window(), self.dump_data()).exec()
|
||||||
|
|
||||||
# --- model <-> form -------------------------------------------------- #
|
# --- model <-> form -------------------------------------------------- #
|
||||||
def load_entry(self, entry: core.ServerEntry | None):
|
def load_entry(self, entry: core.ServerEntry | None):
|
||||||
@@ -785,6 +1067,11 @@ class ServerEditor(QFrame):
|
|||||||
self.args_warn.hide()
|
self.args_warn.hide()
|
||||||
self.args_fix_btn.hide()
|
self.args_fix_btn.hide()
|
||||||
self.secret_warn.hide()
|
self.secret_warn.hide()
|
||||||
|
self.removed_flag_warn.hide()
|
||||||
|
self.removed_flag_fix_btn.hide()
|
||||||
|
self.sidecar_warn.hide()
|
||||||
|
self.perm_warn.hide()
|
||||||
|
self.perm_fix_btn.hide()
|
||||||
self._loading = False
|
self._loading = False
|
||||||
return
|
return
|
||||||
self.setEnabled(True)
|
self.setEnabled(True)
|
||||||
@@ -862,6 +1149,17 @@ class ServerEditor(QFrame):
|
|||||||
self.refresh_dependency(auto_open=False)
|
self.refresh_dependency(auto_open=False)
|
||||||
self._check_args()
|
self._check_args()
|
||||||
|
|
||||||
|
def recheck_advisories(self):
|
||||||
|
"""Re-run the read-only sidecar / permission advisories (#101 hot-reload).
|
||||||
|
|
||||||
|
Public entry point for the file-watcher and focus-in fallback: it only
|
||||||
|
recomputes the warning labels from the current form + the filesystem — it
|
||||||
|
never touches the form's field values, so it is safe to call regardless of
|
||||||
|
unsaved edits (it can't clobber them). Delegates to the same recompute the
|
||||||
|
editor runs on every field change.
|
||||||
|
"""
|
||||||
|
self._check_args()
|
||||||
|
|
||||||
# --- args sanity check ------------------------------------------------ #
|
# --- args sanity check ------------------------------------------------ #
|
||||||
def _current_arg_lines(self) -> list[str]:
|
def _current_arg_lines(self) -> list[str]:
|
||||||
return [ln for ln in self.args.toPlainText().splitlines() if ln.strip() != ""]
|
return [ln for ln in self.args.toPlainText().splitlines() if ln.strip() != ""]
|
||||||
@@ -872,6 +1170,12 @@ class ServerEditor(QFrame):
|
|||||||
self.args_warn.hide()
|
self.args_warn.hide()
|
||||||
self.args_fix_btn.hide()
|
self.args_fix_btn.hide()
|
||||||
self.secret_warn.hide()
|
self.secret_warn.hide()
|
||||||
|
self.removed_flag_warn.hide()
|
||||||
|
self.removed_flag_fix_btn.hide()
|
||||||
|
self.sidecar_warn.hide()
|
||||||
|
self.sidecar_edit_btn.hide()
|
||||||
|
self.perm_warn.hide()
|
||||||
|
self.perm_fix_btn.hide()
|
||||||
return
|
return
|
||||||
_, notes = core.split_suspicious_args(self._current_arg_lines())
|
_, notes = core.split_suspicious_args(self._current_arg_lines())
|
||||||
if notes:
|
if notes:
|
||||||
@@ -887,6 +1191,43 @@ class ServerEditor(QFrame):
|
|||||||
self.secret_warn.show()
|
self.secret_warn.show()
|
||||||
else:
|
else:
|
||||||
self.secret_warn.hide()
|
self.secret_warn.hide()
|
||||||
|
# Flags a package removed across a major version (e.g. ssh-mcp v2's
|
||||||
|
# --password). Needs command + env too, so build from the live form.
|
||||||
|
stdio = {
|
||||||
|
"command": self.command.text().strip(),
|
||||||
|
"args": self._current_arg_lines(),
|
||||||
|
"env": self.env.dump(),
|
||||||
|
}
|
||||||
|
rf_warnings = core.removed_flag_warnings(stdio)
|
||||||
|
if rf_warnings:
|
||||||
|
self.removed_flag_warn.setText("⚠ " + "\n".join(rf_warnings))
|
||||||
|
self.removed_flag_warn.show()
|
||||||
|
# Only offer the button when something is actually auto-migratable.
|
||||||
|
migrated, _ = core.migrate_removed_flags(stdio)
|
||||||
|
self.removed_flag_fix_btn.setVisible(migrated is not stdio)
|
||||||
|
else:
|
||||||
|
self.removed_flag_warn.hide()
|
||||||
|
self.removed_flag_fix_btn.hide()
|
||||||
|
# Sidecar precedence / wrong-path / credential-scoping (#91). Uses the
|
||||||
|
# real platform + environment + filesystem so it reflects this machine.
|
||||||
|
sc_warnings = core.sidecar_warnings(stdio)
|
||||||
|
if sc_warnings:
|
||||||
|
self.sidecar_warn.setText("⚠ " + "\n\n".join(sc_warnings))
|
||||||
|
self.sidecar_warn.show()
|
||||||
|
else:
|
||||||
|
self.sidecar_warn.hide()
|
||||||
|
# Offer the in-app editor (#102) whenever this server has a resolvable
|
||||||
|
# sidecar — even before the file exists, so it can be created from BCC.
|
||||||
|
self.sidecar_edit_btn.setVisible(core.sidecar_status(stdio) is not None)
|
||||||
|
# Sidecar filesystem permissions (#93). Real platform/fs; no-op on Windows.
|
||||||
|
perm_warnings = core.sidecar_permission_warnings(stdio)
|
||||||
|
if perm_warnings:
|
||||||
|
self.perm_warn.setText("⚠ " + "\n".join(perm_warnings))
|
||||||
|
self.perm_warn.show()
|
||||||
|
self.perm_fix_btn.show()
|
||||||
|
else:
|
||||||
|
self.perm_warn.hide()
|
||||||
|
self.perm_fix_btn.hide()
|
||||||
|
|
||||||
def _fix_args(self):
|
def _fix_args(self):
|
||||||
if self._before_change:
|
if self._before_change:
|
||||||
@@ -894,11 +1235,72 @@ class ServerEditor(QFrame):
|
|||||||
fixed, _ = core.split_suspicious_args(self._current_arg_lines())
|
fixed, _ = core.split_suspicious_args(self._current_arg_lines())
|
||||||
self.args.setPlainText("\n".join(fixed)) # triggers _emit -> recheck
|
self.args.setPlainText("\n".join(fixed)) # triggers _emit -> recheck
|
||||||
|
|
||||||
|
def _fix_removed_flags(self):
|
||||||
|
if self._before_change:
|
||||||
|
self._before_change()
|
||||||
|
stdio = {
|
||||||
|
"command": self.command.text().strip(),
|
||||||
|
"args": self._current_arg_lines(),
|
||||||
|
"env": self.env.dump(),
|
||||||
|
}
|
||||||
|
migrated, _ = core.migrate_removed_flags(stdio)
|
||||||
|
# Reload env first, then args; setting args text triggers _emit -> recheck.
|
||||||
|
self.env.load(migrated.get("env", {}))
|
||||||
|
self.args.setPlainText("\n".join(migrated.get("args", [])))
|
||||||
|
|
||||||
|
def _fix_permissions(self):
|
||||||
|
"""chmod the sidecar config to 0600 / its dir to 0700 (#93)."""
|
||||||
|
stdio = {
|
||||||
|
"command": self.command.text().strip(),
|
||||||
|
"args": self._current_arg_lines(),
|
||||||
|
"env": self.env.dump(),
|
||||||
|
}
|
||||||
|
target = core.sidecar_permission_fix_target(stdio)
|
||||||
|
if target is None:
|
||||||
|
return
|
||||||
|
changed, note = core.fix_permissions(target)
|
||||||
|
if not changed:
|
||||||
|
# Surface the failure in-place rather than silently doing nothing.
|
||||||
|
self.perm_warn.setText("⚠ " + (note or "could not change permissions"))
|
||||||
|
return
|
||||||
|
self._check_args() # re-check; the warning clears when perms are now tight
|
||||||
|
|
||||||
|
def _edit_sidecar(self):
|
||||||
|
"""Open the in-app sidecar editor for this server's external config (#102)."""
|
||||||
|
stdio = {
|
||||||
|
"command": self.command.text().strip(),
|
||||||
|
"args": self._current_arg_lines(),
|
||||||
|
"env": self.env.dump(),
|
||||||
|
}
|
||||||
|
status = core.sidecar_status(stdio)
|
||||||
|
spec = core.resolve_server_spec(stdio)
|
||||||
|
if status is None or spec is None:
|
||||||
|
return
|
||||||
|
path = status["path"]
|
||||||
|
try:
|
||||||
|
text = path.read_text(encoding="utf-8") if status["exists"] else ""
|
||||||
|
except OSError as e:
|
||||||
|
QMessageBox.warning(self.window(), "Can't open config", f"{path}\n\n{e}")
|
||||||
|
return
|
||||||
|
dlg = SidecarEditorDialog(self.window(), spec.package, path, spec.schema, text)
|
||||||
|
if dlg.exec() and dlg.saved:
|
||||||
|
# Re-run detection through the same read-only path the #101 watcher
|
||||||
|
# uses, so "args inert" / permission advisories update live.
|
||||||
|
self._check_args()
|
||||||
|
bnote = f" · backup: {dlg.backup_path.name}" if dlg.backup_path else " · (new file)"
|
||||||
|
QMessageBox.information(
|
||||||
|
self.window(),
|
||||||
|
"Config saved",
|
||||||
|
f"Saved {path}{bnote}\nThe file was tightened to 0600.",
|
||||||
|
)
|
||||||
|
|
||||||
# --- dependency ------------------------------------------------------ #
|
# --- dependency ------------------------------------------------------ #
|
||||||
def refresh_dependency(self, auto_open=False):
|
def refresh_dependency(self, auto_open=False):
|
||||||
if not self.isEnabled():
|
if not self.isEnabled():
|
||||||
self._set_dep({"status": "unknown", "label": "—"})
|
self._set_dep({"status": "unknown", "label": "—"})
|
||||||
self.diag_text.clear()
|
self.diag_text.clear()
|
||||||
|
for wdg in self.ver_row_widgets:
|
||||||
|
wdg.hide()
|
||||||
return
|
return
|
||||||
data = self.dump_data()
|
data = self.dump_data()
|
||||||
res = core.check_dependency(data)
|
res = core.check_dependency(data)
|
||||||
@@ -912,6 +1314,49 @@ class ServerEditor(QFrame):
|
|||||||
self.details_btn.setChecked(True) # opens panel (fills text via _toggle_diag)
|
self.details_btn.setChecked(True) # opens panel (fills text via _toggle_diag)
|
||||||
if self.diag_card.isVisible():
|
if self.diag_card.isVisible():
|
||||||
self.diag_text.setPlainText(self._full_diag_text())
|
self.diag_text.setPlainText(self._full_diag_text())
|
||||||
|
self._refresh_version_badge(data)
|
||||||
|
|
||||||
|
def _refresh_version_badge(self, data: dict):
|
||||||
|
"""Show the resolved version / pin state for an npx-style server (#92)."""
|
||||||
|
st = core.version_status(data) if self.type.currentIndex() == 0 else None
|
||||||
|
if st is None:
|
||||||
|
for wdg in self.ver_row_widgets:
|
||||||
|
wdg.hide()
|
||||||
|
return
|
||||||
|
for wdg in self.ver_row_widgets:
|
||||||
|
wdg.show()
|
||||||
|
resolved = st["resolved_version"] or "unknown"
|
||||||
|
if st["drift"]:
|
||||||
|
glyph, color, text = "●", WARN, f"{st['package']} {st['drift']}"
|
||||||
|
elif st["unpinned"]:
|
||||||
|
glyph, color = "●", WARN
|
||||||
|
text = f"{st['package']} · resolved {resolved} · unpinned (resolves latest each launch)"
|
||||||
|
else:
|
||||||
|
glyph, color = "●", GOOD
|
||||||
|
text = f"{st['package']} · pinned {st['pinned_version']}"
|
||||||
|
self.ver_dot.setText(glyph)
|
||||||
|
self.ver_dot.setStyleSheet(f"color: {color}; font-size: 14px;")
|
||||||
|
self.ver_label.setText(text)
|
||||||
|
self.ver_label.setStyleSheet(f"color: {color};")
|
||||||
|
# Offer the pin only when unpinned AND we know what to pin to.
|
||||||
|
self.pin_btn.setVisible(st["can_pin"])
|
||||||
|
if st["can_pin"]:
|
||||||
|
self.pin_btn.setText(f"Pin to {resolved}")
|
||||||
|
self._version_resolved = st["resolved_version"]
|
||||||
|
|
||||||
|
def _pin_version(self):
|
||||||
|
if self._before_change:
|
||||||
|
self._before_change()
|
||||||
|
resolved = getattr(self, "_version_resolved", None)
|
||||||
|
if not resolved:
|
||||||
|
return
|
||||||
|
new_data, note = core.pin_spec_transform(self.dump_data(), resolved)
|
||||||
|
if not note:
|
||||||
|
return
|
||||||
|
self._loading = True
|
||||||
|
self.args.setPlainText("\n".join(str(a) for a in (new_data.get("args") or [])))
|
||||||
|
self._loading = False
|
||||||
|
self._emit() # writes back to the model and re-checks (badge now "pinned")
|
||||||
|
|
||||||
def _set_dep(self, res: dict):
|
def _set_dep(self, res: dict):
|
||||||
status = res.get("status", "unknown")
|
status = res.get("status", "unknown")
|
||||||
@@ -1044,6 +1489,52 @@ class ArgsEdit(QPlainTextEdit):
|
|||||||
self.blockCountChanged.connect(self._update_gutter_width)
|
self.blockCountChanged.connect(self._update_gutter_width)
|
||||||
self.updateRequest.connect(self._on_update_request)
|
self.updateRequest.connect(self._on_update_request)
|
||||||
self._update_gutter_width()
|
self._update_gutter_width()
|
||||||
|
# Wired by ServerEditor: gate on the loaded client, and the two move
|
||||||
|
# actions (which the editor performs, since moving an arg into env
|
||||||
|
# touches both the args and the env table). Indices are into the
|
||||||
|
# non-blank arg list, matching dump_data()'s args.
|
||||||
|
self.profile_provider = None
|
||||||
|
self.on_move_to_ref = None
|
||||||
|
self.on_move_to_env = None
|
||||||
|
|
||||||
|
def contextMenuEvent(self, event):
|
||||||
|
menu = self.createStandardContextMenu() # keep cut/copy/paste
|
||||||
|
lines = self.toPlainText().splitlines()
|
||||||
|
block = self.cursorForPosition(event.pos()).blockNumber()
|
||||||
|
if 0 <= block < len(lines) and lines[block].strip():
|
||||||
|
# This editor is one arg per line; map the clicked block to its
|
||||||
|
# index among the non-blank args the model actually sees.
|
||||||
|
cleaned = [ln for ln in lines if ln.strip() != ""]
|
||||||
|
idx = sum(1 for ln in lines[:block] if ln.strip() != "")
|
||||||
|
if idx in set(core.secret_arg_indices(cleaned)):
|
||||||
|
profile = self.profile_provider() if self.profile_provider else None
|
||||||
|
expands = profile is None or core.client_expands_env_refs(profile)
|
||||||
|
first = menu.actions()[0] if menu.actions() else None
|
||||||
|
|
||||||
|
# Reference (secret leaves the file) -- needs an expanding client.
|
||||||
|
ref_act = QAction("Replace with a ${VAR} reference (out of file)…", self)
|
||||||
|
if expands and self.on_move_to_ref:
|
||||||
|
ref_act.triggered.connect(lambda: self.on_move_to_ref(idx))
|
||||||
|
else:
|
||||||
|
ref_act.setEnabled(False)
|
||||||
|
ref_act.setText(
|
||||||
|
"Replace with ${VAR} reference — unavailable for Claude Desktop"
|
||||||
|
)
|
||||||
|
ref_act.setToolTip(
|
||||||
|
"Claude Desktop doesn't expand ${VAR}, so a reference would "
|
||||||
|
"reach the server as literal text."
|
||||||
|
)
|
||||||
|
|
||||||
|
# Move into the env block (kept in file) -- works on any client.
|
||||||
|
env_act = QAction("Move into Environment variables (kept in this config)…", self)
|
||||||
|
if self.on_move_to_env:
|
||||||
|
env_act.triggered.connect(lambda: self.on_move_to_env(idx))
|
||||||
|
|
||||||
|
menu.insertAction(first, ref_act)
|
||||||
|
menu.insertAction(first, env_act)
|
||||||
|
if first is not None:
|
||||||
|
menu.insertSeparator(first)
|
||||||
|
menu.exec(event.globalPos())
|
||||||
|
|
||||||
def gutter_width(self) -> int:
|
def gutter_width(self) -> int:
|
||||||
digits = max(1, len(str(self.blockCount())))
|
digits = max(1, len(str(self.blockCount())))
|
||||||
@@ -1717,6 +2208,176 @@ class NoticeBanner(QFrame):
|
|||||||
self.show()
|
self.show()
|
||||||
|
|
||||||
|
|
||||||
|
_UNSET_CHOICE = "— (unset) —" # sentinel entry for an enum pick-list
|
||||||
|
|
||||||
|
|
||||||
|
class SidecarEditorDialog(QDialog):
|
||||||
|
"""Edit an external sidecar config (ssh-mcp's config.toml) from inside BCC (#102).
|
||||||
|
|
||||||
|
A minimal, reviewable first pass: pick-lists for the schema enum fields and a
|
||||||
|
numeric spinner for the port, plus a raw-text editor that is the full,
|
||||||
|
always-available fallback. On Save the changed managed fields are applied
|
||||||
|
surgically on top of the raw text (so comments/unknown keys survive), the
|
||||||
|
managed values are validated against the ServerSpec schema, and the file is
|
||||||
|
written through core.write_sidecar (atomic + timestamped backup + chmod
|
||||||
|
0600). All decision logic lives in bcc_core; this class is wiring.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def __init__(self, parent, package: str, path: Path, schema: dict, initial_text: str):
|
||||||
|
super().__init__(parent)
|
||||||
|
self.setWindowTitle(f"Edit {package} config")
|
||||||
|
self.resize(620, 620)
|
||||||
|
self._path = Path(path)
|
||||||
|
self._schema = schema or {}
|
||||||
|
self.backup_path: Path | None = None # set on a successful save
|
||||||
|
self.saved = False
|
||||||
|
|
||||||
|
v = QVBoxLayout(self)
|
||||||
|
info = QLabel(
|
||||||
|
f"Editing <code>{path}</code><br>"
|
||||||
|
"Pick-lists cover the known settings; the raw editor below is the full "
|
||||||
|
"file. Saving writes atomically, keeps a timestamped backup, and tightens "
|
||||||
|
"the file to <code>0600</code>."
|
||||||
|
)
|
||||||
|
info.setObjectName("muted")
|
||||||
|
info.setWordWrap(True)
|
||||||
|
info.setTextFormat(Qt.TextFormat.RichText)
|
||||||
|
v.addWidget(info)
|
||||||
|
|
||||||
|
# Which section the pick-lists target. ssh-mcp configs are section-based
|
||||||
|
# ([server] / per-profile); default to the first section, else top-level.
|
||||||
|
sec_row = QHBoxLayout()
|
||||||
|
sec_row.addWidget(QLabel("Section:"))
|
||||||
|
self.section_combo = QComboBox()
|
||||||
|
sections = core.toml_sections(initial_text) or [""]
|
||||||
|
for s in sections:
|
||||||
|
self.section_combo.addItem("(top level)" if s == "" else s, s)
|
||||||
|
# Prefer a section literally called "server" if present.
|
||||||
|
if "server" in sections:
|
||||||
|
self.section_combo.setCurrentIndex(sections.index("server"))
|
||||||
|
self.section_combo.currentIndexChanged.connect(self._reload_fields_from_raw)
|
||||||
|
sec_row.addWidget(self.section_combo, 1)
|
||||||
|
v.addLayout(sec_row)
|
||||||
|
|
||||||
|
# Managed schema fields.
|
||||||
|
self._fields_box = QGroupBox("Known settings")
|
||||||
|
self._form = QFormLayout(self._fields_box)
|
||||||
|
self._enum_widgets: dict[str, QComboBox] = {}
|
||||||
|
self._port_widget: QSpinBox | None = None
|
||||||
|
self._port_present = QCheckBox("set") # gate for whether port is written
|
||||||
|
self._build_fields()
|
||||||
|
v.addWidget(self._fields_box)
|
||||||
|
|
||||||
|
v.addWidget(QLabel("Raw TOML (full file — edit anything here):"))
|
||||||
|
self.raw = QPlainTextEdit()
|
||||||
|
self.raw.setPlainText(initial_text)
|
||||||
|
mono = self.raw.font()
|
||||||
|
mono.setFamily("Menlo, Consolas, monospace")
|
||||||
|
self.raw.setFont(mono)
|
||||||
|
v.addWidget(self.raw, 1)
|
||||||
|
|
||||||
|
self.err = QLabel("")
|
||||||
|
self.err.setStyleSheet(f"color: {BAD};")
|
||||||
|
self.err.setWordWrap(True)
|
||||||
|
self.err.hide()
|
||||||
|
v.addWidget(self.err)
|
||||||
|
|
||||||
|
btns = QDialogButtonBox()
|
||||||
|
self.save_btn = btns.addButton("Save", QDialogButtonBox.ButtonRole.AcceptRole)
|
||||||
|
self.save_btn.setObjectName("primary")
|
||||||
|
btns.addButton(QDialogButtonBox.StandardButton.Cancel)
|
||||||
|
btns.accepted.connect(self._save)
|
||||||
|
btns.rejected.connect(self.reject)
|
||||||
|
v.addWidget(btns)
|
||||||
|
|
||||||
|
self._reload_fields_from_raw()
|
||||||
|
|
||||||
|
def _current_section(self) -> str | None:
|
||||||
|
s = self.section_combo.currentData()
|
||||||
|
return None if s == "" else s
|
||||||
|
|
||||||
|
def _build_fields(self):
|
||||||
|
"""Create a widget per schema key (enums -> combo, port range -> spin)."""
|
||||||
|
for key, rule in self._schema.items():
|
||||||
|
if isinstance(rule, list): # enum
|
||||||
|
combo = QComboBox()
|
||||||
|
combo.addItem(_UNSET_CHOICE, None)
|
||||||
|
for opt in rule:
|
||||||
|
combo.addItem(str(opt), opt)
|
||||||
|
self._enum_widgets[key] = combo
|
||||||
|
self._form.addRow(f"{key}:", combo)
|
||||||
|
elif isinstance(rule, dict) and "min" in rule and "max" in rule: # numeric range
|
||||||
|
row = QHBoxLayout()
|
||||||
|
spin = QSpinBox()
|
||||||
|
spin.setRange(int(rule["min"]), int(rule["max"]))
|
||||||
|
spin.setEnabled(False)
|
||||||
|
self._port_present.toggled.connect(spin.setEnabled)
|
||||||
|
row.addWidget(self._port_present)
|
||||||
|
row.addWidget(spin, 1)
|
||||||
|
holder = QWidget()
|
||||||
|
holder.setLayout(row)
|
||||||
|
self._port_widget = spin
|
||||||
|
self._form.addRow(f"{key}:", holder)
|
||||||
|
|
||||||
|
def _reload_fields_from_raw(self):
|
||||||
|
"""Populate the pick-lists from the raw text for the chosen section, and
|
||||||
|
remember the loaded state so Save only applies fields the user changed."""
|
||||||
|
values = core.read_toml_section(self.raw.toPlainText(), self._current_section())
|
||||||
|
self._loaded: dict = {}
|
||||||
|
for key, combo in self._enum_widgets.items():
|
||||||
|
val = values.get(key)
|
||||||
|
idx = combo.findData(val) if val is not None else 0
|
||||||
|
combo.setCurrentIndex(idx if idx >= 0 else 0)
|
||||||
|
# An out-of-enum current value can't be shown; leave it at (unset) but
|
||||||
|
# DON'T record it as loaded so we never silently overwrite it on save.
|
||||||
|
self._loaded[key] = combo.currentData()
|
||||||
|
if self._port_widget is not None:
|
||||||
|
pv = values.get("port")
|
||||||
|
has = isinstance(pv, int) and not isinstance(pv, bool)
|
||||||
|
self._port_present.setChecked(has)
|
||||||
|
if has:
|
||||||
|
self._port_widget.setValue(pv)
|
||||||
|
self._loaded["port"] = pv if has else None
|
||||||
|
|
||||||
|
def _managed_updates(self) -> dict:
|
||||||
|
"""The managed keys the user actually changed -> new value (None = delete).
|
||||||
|
|
||||||
|
Only changed fields are applied, so untouched keys (including any the
|
||||||
|
pick-list can't represent) are left exactly as the raw text has them.
|
||||||
|
"""
|
||||||
|
updates: dict = {}
|
||||||
|
for key, combo in self._enum_widgets.items():
|
||||||
|
cur = combo.currentData()
|
||||||
|
if cur != self._loaded.get(key):
|
||||||
|
updates[key] = cur # None here means "delete the key"
|
||||||
|
if self._port_widget is not None:
|
||||||
|
cur = self._port_widget.value() if self._port_present.isChecked() else None
|
||||||
|
if cur != self._loaded.get("port"):
|
||||||
|
updates["port"] = cur
|
||||||
|
return updates
|
||||||
|
|
||||||
|
def _save(self):
|
||||||
|
section = self._current_section()
|
||||||
|
updates = self._managed_updates()
|
||||||
|
# Validate only the managed values the user is actually setting (a None =
|
||||||
|
# delete needs no enum check).
|
||||||
|
to_check = {k: v for k, v in updates.items() if v is not None}
|
||||||
|
problems = core.validate_sidecar_values(to_check, self._schema)
|
||||||
|
if problems:
|
||||||
|
self.err.setText("⚠ " + "\n".join(problems))
|
||||||
|
self.err.show()
|
||||||
|
return
|
||||||
|
try:
|
||||||
|
new_text = core.update_toml(self.raw.toPlainText(), updates, section=section)
|
||||||
|
self.backup_path = core.write_sidecar(self._path, new_text)
|
||||||
|
except Exception as e: # surface any write/permission failure in-place
|
||||||
|
self.err.setText(f"⚠ Could not save: {e}")
|
||||||
|
self.err.show()
|
||||||
|
return
|
||||||
|
self.saved = True
|
||||||
|
self.accept()
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# Restart worker: core.restart_claude_desktop() blocks up to ~5 s on macOS
|
# Restart worker: core.restart_claude_desktop() blocks up to ~5 s on macOS
|
||||||
# waiting for the old instance to exit, so it must run off the UI thread.
|
# waiting for the old instance to exit, so it must run off the UI thread.
|
||||||
@@ -1757,6 +2418,18 @@ class MainWindow(QMainWindow):
|
|||||||
self._test_all_done = 0
|
self._test_all_done = 0
|
||||||
self._health_tester: SpawnTester | None = None
|
self._health_tester: SpawnTester | None = None
|
||||||
|
|
||||||
|
# Hot-reload (#101): watch the selected server's external sidecar path(s)
|
||||||
|
# and BCC's own loaded config, so changes made outside BCC surface without
|
||||||
|
# a restart. The watcher fires the debounce timer; the timer re-checks.
|
||||||
|
self._fs_watcher = QFileSystemWatcher(self)
|
||||||
|
self._fs_watcher.fileChanged.connect(self._on_fs_signal)
|
||||||
|
self._fs_watcher.directoryChanged.connect(self._on_fs_signal)
|
||||||
|
self._fs_debounce = QTimer(self)
|
||||||
|
self._fs_debounce.setSingleShot(True)
|
||||||
|
self._fs_debounce.setInterval(300) # coalesce a burst of writes
|
||||||
|
self._fs_debounce.timeout.connect(self._recheck_external_state)
|
||||||
|
self._sidecar_fp: tuple | None = None # last observed sidecar state
|
||||||
|
|
||||||
central = QWidget()
|
central = QWidget()
|
||||||
self.setCentralWidget(central)
|
self.setCentralWidget(central)
|
||||||
root = QVBoxLayout(central)
|
root = QVBoxLayout(central)
|
||||||
@@ -1779,6 +2452,12 @@ class MainWindow(QMainWindow):
|
|||||||
self.update_banner = NoticeBanner(self)
|
self.update_banner = NoticeBanner(self)
|
||||||
root.addWidget(self.update_banner)
|
root.addWidget(self.update_banner)
|
||||||
|
|
||||||
|
# External-change notice (#101): when BCC's own loaded config is edited
|
||||||
|
# outside BCC, surface it here with a one-click Reload rather than
|
||||||
|
# silently overwriting — a live reload must never clobber unsaved edits.
|
||||||
|
self.reload_banner = NoticeBanner(self)
|
||||||
|
root.addWidget(self.reload_banner)
|
||||||
|
|
||||||
# User-draggable divider between the server list and the editor.
|
# User-draggable divider between the server list and the editor.
|
||||||
split = QSplitter(Qt.Orientation.Horizontal)
|
split = QSplitter(Qt.Orientation.Horizontal)
|
||||||
split.setChildrenCollapsible(False)
|
split.setChildrenCollapsible(False)
|
||||||
@@ -2280,6 +2959,8 @@ class MainWindow(QMainWindow):
|
|||||||
self._health.clear() # health results are per-profile; a fresh load invalidates them
|
self._health.clear() # health results are per-profile; a fresh load invalidates them
|
||||||
self._refresh_sets_combo() # sets are per-config; repopulate from the loaded file
|
self._refresh_sets_combo() # sets are per-config; repopulate from the loaded file
|
||||||
self._refresh_tables(select_index=0 if self.servers else -1)
|
self._refresh_tables(select_index=0 if self.servers else -1)
|
||||||
|
self.reload_banner.hide() # loaded fresh: any external-change notice is now stale
|
||||||
|
self._rewatch_paths() # watch this profile's config + the selection's sidecar (#101)
|
||||||
self._update_status(saved=False)
|
self._update_status(saved=False)
|
||||||
if repaired:
|
if repaired:
|
||||||
self._mark_dirty()
|
self._mark_dirty()
|
||||||
@@ -2570,6 +3251,120 @@ class MainWindow(QMainWindow):
|
|||||||
self.copy_btn.setEnabled(entry is not None and len(self.profiles) > 1)
|
self.copy_btn.setEnabled(entry is not None and len(self.profiles) > 1)
|
||||||
self.dup_btn.setEnabled(entry is not None)
|
self.dup_btn.setEnabled(entry is not None)
|
||||||
self.del_btn.setEnabled(entry is not None)
|
self.del_btn.setEnabled(entry is not None)
|
||||||
|
self._rewatch_paths()
|
||||||
|
|
||||||
|
# --- hot-reload (#101) ------------------------------------------------- #
|
||||||
|
def _watch_targets(self) -> list[Path]:
|
||||||
|
"""Every external path worth watching for the current view.
|
||||||
|
|
||||||
|
BCC's own loaded config (+ its directory), plus the selected server's
|
||||||
|
sidecar path(s) resolved from its ServerSpec. Directories are watched too
|
||||||
|
so a file appearing/disappearing — which a watch on a not-yet-existent
|
||||||
|
file would miss, and which an atomic-rename replace looks like — is still
|
||||||
|
observed. All entries must exist for QFileSystemWatcher to accept them.
|
||||||
|
"""
|
||||||
|
targets: list[Path] = []
|
||||||
|
if self.current_profile is not None:
|
||||||
|
cfg = Path(self.current_profile.path)
|
||||||
|
targets.append(cfg)
|
||||||
|
targets.append(cfg.parent)
|
||||||
|
if self.editor.isEnabled():
|
||||||
|
data = self.editor.dump_data()
|
||||||
|
targets.extend(core.sidecar_watch_paths(data))
|
||||||
|
# De-dup, keep only paths that currently exist (the watcher rejects the
|
||||||
|
# rest; the parent dir covers a not-yet-created file).
|
||||||
|
seen: set[str] = set()
|
||||||
|
out: list[Path] = []
|
||||||
|
for p in targets:
|
||||||
|
key = str(p)
|
||||||
|
if key in seen:
|
||||||
|
continue
|
||||||
|
seen.add(key)
|
||||||
|
if p.exists():
|
||||||
|
out.append(p)
|
||||||
|
return out
|
||||||
|
|
||||||
|
def _rewatch_paths(self):
|
||||||
|
"""Point the watcher at the current targets and snapshot sidecar state.
|
||||||
|
|
||||||
|
Re-applied whenever the selection or the edited command/args change (which
|
||||||
|
server is selected decides which sidecar to watch) and after a reload.
|
||||||
|
"""
|
||||||
|
watcher = self._fs_watcher
|
||||||
|
existing = watcher.files() + watcher.directories()
|
||||||
|
if existing:
|
||||||
|
watcher.removePaths(existing)
|
||||||
|
wanted = [str(p) for p in self._watch_targets()]
|
||||||
|
if wanted:
|
||||||
|
watcher.addPaths(wanted)
|
||||||
|
self._sidecar_fp = self._current_sidecar_fp()
|
||||||
|
|
||||||
|
def _current_sidecar_fp(self) -> tuple | None:
|
||||||
|
if not self.editor.isEnabled():
|
||||||
|
return None
|
||||||
|
return core.sidecar_state_fingerprint(self.editor.dump_data())
|
||||||
|
|
||||||
|
def _on_fs_signal(self, _path=None):
|
||||||
|
"""A watched path changed — coalesce a burst of writes via the debounce."""
|
||||||
|
self._fs_debounce.start()
|
||||||
|
|
||||||
|
def _recheck_external_state(self):
|
||||||
|
"""Debounced re-check: re-arm the watcher, refresh advisories if state
|
||||||
|
actually changed, and surface an external edit to BCC's own config.
|
||||||
|
|
||||||
|
Runs from both the file-watcher and the focus-in fallback. Never mutates
|
||||||
|
the editor form or the loaded config — the only content reload is the
|
||||||
|
user pressing Reload on the banner, so unsaved edits are safe.
|
||||||
|
"""
|
||||||
|
# Re-arm: an atomic-rename replace drops the old inode from the watcher,
|
||||||
|
# so paths must be re-added or the next change goes unseen.
|
||||||
|
self._rewatch_after_event()
|
||||||
|
|
||||||
|
# Sidecar/permission advisories for the selected server (#91/#93). Only
|
||||||
|
# refresh when the observable state changed, so an unrelated write in the
|
||||||
|
# watched directory doesn't thrash the panel.
|
||||||
|
new_fp = self._current_sidecar_fp()
|
||||||
|
if core.sidecar_state_changed(self._sidecar_fp, new_fp):
|
||||||
|
self._sidecar_fp = new_fp
|
||||||
|
self.editor.recheck_advisories()
|
||||||
|
|
||||||
|
# BCC's own loaded config edited outside BCC: surface it (non-destructive).
|
||||||
|
self._check_config_changed_on_disk()
|
||||||
|
|
||||||
|
def _rewatch_after_event(self):
|
||||||
|
"""Re-add any wanted paths the watcher dropped, without disturbing the
|
||||||
|
sidecar fingerprint (which _recheck_external_state compares itself)."""
|
||||||
|
watcher = self._fs_watcher
|
||||||
|
current = set(watcher.files()) | set(watcher.directories())
|
||||||
|
readd = [str(p) for p in self._watch_targets() if str(p) not in current]
|
||||||
|
if readd:
|
||||||
|
watcher.addPaths(readd)
|
||||||
|
|
||||||
|
def _check_config_changed_on_disk(self):
|
||||||
|
if self.current_profile is None:
|
||||||
|
return
|
||||||
|
disk = core.config_fingerprint(self.current_profile.path)
|
||||||
|
if disk is not None and self._loaded_stat is not None and disk != self._loaded_stat:
|
||||||
|
self.reload_banner.show_notice(
|
||||||
|
f"{self.current_profile.path} changed on disk (edited outside BCC).",
|
||||||
|
action_label="Reload from disk",
|
||||||
|
on_action=lambda: self.load_profile(self.current_profile, confirm=True),
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
# Back in sync (e.g. the user reloaded, or the change was reverted).
|
||||||
|
self.reload_banner.hide()
|
||||||
|
|
||||||
|
def changeEvent(self, event):
|
||||||
|
"""Re-check external state when the window regains focus (#101).
|
||||||
|
|
||||||
|
A cheap, always-works fallback: QFileSystemWatcher can miss changes
|
||||||
|
(notably atomic-rename replaces, and files that didn't exist when the
|
||||||
|
watch was set), so a re-check on activation covers the gap.
|
||||||
|
"""
|
||||||
|
if event.type() == QEvent.Type.ActivationChange and self.isActiveWindow():
|
||||||
|
# Go through the debounce so activation + a watcher signal coalesce.
|
||||||
|
self._fs_debounce.start()
|
||||||
|
super().changeEvent(event)
|
||||||
|
|
||||||
def _table_item_changed(self, item: QTableWidgetItem):
|
def _table_item_changed(self, item: QTableWidgetItem):
|
||||||
if self._suppress_table or item.column() != 0:
|
if self._suppress_table or item.column() != 0:
|
||||||
@@ -2624,6 +3419,9 @@ class MainWindow(QMainWindow):
|
|||||||
health_item.setToolTip("Not tested since last edit.")
|
health_item.setToolTip("Not tested since last edit.")
|
||||||
self._suppress_table = False
|
self._suppress_table = False
|
||||||
self._refresh_badges()
|
self._refresh_badges()
|
||||||
|
# The edited command/args may change which package (and thus which
|
||||||
|
# sidecar) this server resolves to — re-point the watcher (#101).
|
||||||
|
self._rewatch_paths()
|
||||||
self._mark_dirty()
|
self._mark_dirty()
|
||||||
|
|
||||||
# --- server actions -------------------------------------------------- #
|
# --- server actions -------------------------------------------------- #
|
||||||
@@ -2770,6 +3568,8 @@ class MainWindow(QMainWindow):
|
|||||||
for entry in self.servers:
|
for entry in self.servers:
|
||||||
for warning in core.env_ref_warnings(entry.data, self.current_profile):
|
for warning in core.env_ref_warnings(entry.data, self.current_profile):
|
||||||
lint_warnings.append(f"'{entry.name}': {warning}")
|
lint_warnings.append(f"'{entry.name}': {warning}")
|
||||||
|
for warning in core.removed_flag_warnings(entry.data):
|
||||||
|
lint_warnings.append(f"'{entry.name}': {warning}")
|
||||||
if lint_warnings:
|
if lint_warnings:
|
||||||
self.validation_lbl.setText(f"⚠ {lint_warnings[0]}")
|
self.validation_lbl.setText(f"⚠ {lint_warnings[0]}")
|
||||||
self.validation_lbl.setStyleSheet(f"color: {WARN};")
|
self.validation_lbl.setStyleSheet(f"color: {WARN};")
|
||||||
|
|||||||
+1479
-13
File diff suppressed because it is too large
Load Diff
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
[project]
|
[project]
|
||||||
name = "better-claude-config"
|
name = "better-claude-config"
|
||||||
version = "1.3.0"
|
version = "1.4.0"
|
||||||
description = "Cross-platform GUI for editing the mcpServers block of Claude Desktop and Claude Code configs"
|
description = "Cross-platform GUI for editing the mcpServers block of Claude Desktop and Claude Code configs"
|
||||||
readme = "README.md"
|
readme = "README.md"
|
||||||
license = { file = "LICENSE" }
|
license = { file = "LICENSE" }
|
||||||
|
|||||||
@@ -3061,6 +3061,861 @@ def test_discover_project_configs_skips_non_object_and_garbage(tmp_path):
|
|||||||
assert str(missing / ".mcp.json") not in paths
|
assert str(missing / ".mcp.json") not in paths
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# Removed-flag -> env migration (ssh-mcp v2 and the general mechanism)
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_detect_migratable_package_via_npx_args():
|
||||||
|
data = {"command": "npx", "args": ["-y", "ssh-mcp", "--host=h", "--password=p"]}
|
||||||
|
assert c.detect_migratable_package(data) == "ssh-mcp"
|
||||||
|
|
||||||
|
|
||||||
|
def test_detect_migratable_package_direct_command_and_path_and_version():
|
||||||
|
assert c.detect_migratable_package({"command": "ssh-mcp", "args": []}) == "ssh-mcp"
|
||||||
|
assert (
|
||||||
|
c.detect_migratable_package({"command": "/usr/local/bin/ssh-mcp", "args": []}) == "ssh-mcp"
|
||||||
|
)
|
||||||
|
assert (
|
||||||
|
c.detect_migratable_package({"command": "npx", "args": ["-y", "ssh-mcp@2.0.1"]})
|
||||||
|
== "ssh-mcp"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_detect_migratable_package_unknown_returns_none():
|
||||||
|
assert c.detect_migratable_package({"command": "npx", "args": ["some-other"]}) is None
|
||||||
|
assert c.detect_migratable_package({}) is None
|
||||||
|
assert c.detect_migratable_package({"url": "https://x"}) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_migrate_removed_flags_inline_form():
|
||||||
|
data = {
|
||||||
|
"command": "npx",
|
||||||
|
"args": ["-y", "ssh-mcp", "--", "--host=1.2.3.4", "--password=hunter2"],
|
||||||
|
}
|
||||||
|
new, notes = c.migrate_removed_flags(data)
|
||||||
|
assert new["args"] == ["-y", "ssh-mcp", "--", "--host=1.2.3.4"]
|
||||||
|
assert new["env"] == {"SSH_MCP_PASSWORD": "hunter2"}
|
||||||
|
assert any("SSH_MCP_PASSWORD" in n for n in notes)
|
||||||
|
# Original untouched (pure function).
|
||||||
|
assert "env" not in data
|
||||||
|
|
||||||
|
|
||||||
|
def test_migrate_removed_flags_separate_form():
|
||||||
|
data = {
|
||||||
|
"command": "npx",
|
||||||
|
"args": ["-y", "ssh-mcp", "--user", "root", "--password", "s3cret"],
|
||||||
|
}
|
||||||
|
new, _ = c.migrate_removed_flags(data)
|
||||||
|
assert new["args"] == ["-y", "ssh-mcp", "--user", "root"]
|
||||||
|
assert new["env"] == {"SSH_MCP_PASSWORD": "s3cret"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_migrate_removed_flags_preserves_existing_env_and_merges():
|
||||||
|
data = {
|
||||||
|
"command": "npx",
|
||||||
|
"args": ["ssh-mcp", "--password=p"],
|
||||||
|
"env": {"OTHER": "keep"},
|
||||||
|
}
|
||||||
|
new, _ = c.migrate_removed_flags(data)
|
||||||
|
assert new["env"] == {"OTHER": "keep", "SSH_MCP_PASSWORD": "p"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_migrate_removed_flags_does_not_clobber_existing_secret():
|
||||||
|
data = {
|
||||||
|
"command": "npx",
|
||||||
|
"args": ["ssh-mcp", "--password=fromargs"],
|
||||||
|
"env": {"SSH_MCP_PASSWORD": "fromenv"},
|
||||||
|
}
|
||||||
|
new, notes = c.migrate_removed_flags(data)
|
||||||
|
# env value wins; the redundant flag is still stripped so v2 can start.
|
||||||
|
assert new["env"] == {"SSH_MCP_PASSWORD": "fromenv"}
|
||||||
|
assert "--password=fromargs" not in new["args"]
|
||||||
|
assert any("already set" in n for n in notes)
|
||||||
|
|
||||||
|
|
||||||
|
def test_migrate_removed_flags_moves_env_ref_verbatim():
|
||||||
|
data = {"command": "npx", "args": ["ssh-mcp", "--password", "${MY_PW}"]}
|
||||||
|
new, _ = c.migrate_removed_flags(data)
|
||||||
|
assert new["env"] == {"SSH_MCP_PASSWORD": "${MY_PW}"}
|
||||||
|
assert "${MY_PW}" not in new["args"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_migrate_removed_flags_noop_returns_same_object():
|
||||||
|
data = {"command": "npx", "args": ["ssh-mcp", "--host=h", "--user=u"]}
|
||||||
|
new, notes = c.migrate_removed_flags(data)
|
||||||
|
assert new is data
|
||||||
|
assert notes == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_migrate_removed_flags_unknown_package_noop():
|
||||||
|
data = {"command": "npx", "args": ["mystery", "--password=p"]}
|
||||||
|
new, notes = c.migrate_removed_flags(data)
|
||||||
|
assert new is data and notes == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_migrate_removed_flags_only_args_left_drops_args_key():
|
||||||
|
data = {"command": "ssh-mcp", "args": ["--password=p"]}
|
||||||
|
new, _ = c.migrate_removed_flags(data)
|
||||||
|
assert "args" not in new
|
||||||
|
assert new["env"] == {"SSH_MCP_PASSWORD": "p"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_removed_flag_warnings_covers_migratable_and_manual():
|
||||||
|
data = {
|
||||||
|
"command": "npx",
|
||||||
|
"args": ["ssh-mcp", "--password=p", "--sudoPassword=s", "--disableSudo"],
|
||||||
|
}
|
||||||
|
warnings = c.removed_flag_warnings(data)
|
||||||
|
text = " ".join(warnings)
|
||||||
|
assert "--password" in text
|
||||||
|
assert "sudoPassword" in text
|
||||||
|
assert "disableSudo" in text
|
||||||
|
# #90: --password AND --sudoPassword are now both auto-migratable (the
|
||||||
|
# verified ssh-mcp facts map sudo/su → SSH_MCP_SUDO_PASSWORD). The migration
|
||||||
|
# LOGIC is unchanged — only the registry data grew — so both move into env.
|
||||||
|
# --disableSudo has no env replacement (sudo is now a role/policy) → warn-only.
|
||||||
|
new, _ = c.migrate_removed_flags(data)
|
||||||
|
assert new["env"] == {"SSH_MCP_PASSWORD": "p", "SSH_MCP_SUDO_PASSWORD": "s"}
|
||||||
|
assert "--sudoPassword=s" not in new["args"]
|
||||||
|
assert "--disableSudo" in new["args"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_migrate_su_password_maps_to_sudo_env():
|
||||||
|
# --suPassword is the second flag that maps to the same SSH_MCP_SUDO_PASSWORD var.
|
||||||
|
data = {"command": "ssh-mcp", "args": ["--suPassword", "rootpw"]}
|
||||||
|
new, notes = c.migrate_removed_flags(data)
|
||||||
|
assert new["env"] == {"SSH_MCP_SUDO_PASSWORD": "rootpw"}
|
||||||
|
assert "args" not in new
|
||||||
|
assert notes
|
||||||
|
|
||||||
|
|
||||||
|
def test_migrate_sudo_and_su_two_flags_one_var_no_clobber():
|
||||||
|
# Both sudo flags map to one var; the no-clobber path keeps the first, drops
|
||||||
|
# the second (different value) with a note rather than silently overwriting.
|
||||||
|
data = {
|
||||||
|
"command": "ssh-mcp",
|
||||||
|
"args": ["--sudoPassword=first", "--suPassword=second"],
|
||||||
|
}
|
||||||
|
new, notes = c.migrate_removed_flags(data)
|
||||||
|
assert new["env"] == {"SSH_MCP_SUDO_PASSWORD": "first"}
|
||||||
|
assert "args" not in new
|
||||||
|
assert any("already set" in n for n in notes)
|
||||||
|
|
||||||
|
|
||||||
|
def test_removed_flag_warnings_empty_when_clean():
|
||||||
|
assert c.removed_flag_warnings({"command": "ssh-mcp", "args": ["--host=h"]}) == []
|
||||||
|
assert c.removed_flag_warning({"command": "ssh-mcp", "args": ["--host=h"]}) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_ssh_membermatters_end_to_end():
|
||||||
|
# The exact shape from AJ's failing server.
|
||||||
|
data = {
|
||||||
|
"command": "npx",
|
||||||
|
"args": [
|
||||||
|
"-y",
|
||||||
|
"ssh-mcp",
|
||||||
|
"--",
|
||||||
|
"--host=member.example.io",
|
||||||
|
"--user=deploy",
|
||||||
|
"--port=22",
|
||||||
|
"--password=topsecret",
|
||||||
|
],
|
||||||
|
}
|
||||||
|
new, notes = c.migrate_removed_flags(data)
|
||||||
|
assert new["args"] == [
|
||||||
|
"-y",
|
||||||
|
"ssh-mcp",
|
||||||
|
"--",
|
||||||
|
"--host=member.example.io",
|
||||||
|
"--user=deploy",
|
||||||
|
"--port=22",
|
||||||
|
]
|
||||||
|
assert new["env"] == {"SSH_MCP_PASSWORD": "topsecret"}
|
||||||
|
assert notes
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# ServerSpec spine (issue #90)
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_server_spec_registry_seeds_ssh_mcp():
|
||||||
|
spec = c.SERVER_SPECS["ssh-mcp"]
|
||||||
|
assert spec.package == "ssh-mcp"
|
||||||
|
# sudo/su both map to the one sudo env var (verified facts).
|
||||||
|
assert spec.env_flags["--sudoPassword"] == "SSH_MCP_SUDO_PASSWORD"
|
||||||
|
assert spec.env_flags["--suPassword"] == "SSH_MCP_SUDO_PASSWORD"
|
||||||
|
assert spec.env_flags["--password"] == "SSH_MCP_PASSWORD"
|
||||||
|
# disableSudo stays warn-only (no env replacement).
|
||||||
|
assert "--disableSudo" in spec.removed_flags
|
||||||
|
assert "--disableSudo" not in spec.env_flags
|
||||||
|
# Verified sidecar paths seeded per platform (README path differs → doc_path).
|
||||||
|
assert "ssh-mcp/config.toml" in spec.sidecar_paths["darwin"]
|
||||||
|
assert "Application Support" in spec.sidecar_paths["darwin"]
|
||||||
|
assert "APPDATA" in spec.sidecar_paths["win32"]
|
||||||
|
assert spec.sidecar_doc_path == "~/.config/ssh-mcp/config.toml"
|
||||||
|
# zod enums seeded as data (pick-lists later — #7).
|
||||||
|
assert spec.schema["auth"] == ["agent", "key", "password", "keychain"]
|
||||||
|
assert spec.schema["approvalMode"] == ["auto", "ask-destructive", "ask-all", "deny"]
|
||||||
|
assert spec.schema["role"] == ["viewer", "operator", "admin"]
|
||||||
|
assert spec.schema["port"] == {"min": 1, "max": 65535}
|
||||||
|
|
||||||
|
|
||||||
|
def test_flag_env_migrations_is_derived_from_server_specs():
|
||||||
|
# The legacy constant is now a derived view; it must mirror the registry.
|
||||||
|
assert set(c.FLAG_ENV_MIGRATIONS) == set(c.SERVER_SPECS)
|
||||||
|
assert c.FLAG_ENV_MIGRATIONS["ssh-mcp"]["env"] == c.SERVER_SPECS["ssh-mcp"].env_flags
|
||||||
|
assert c.FLAG_ENV_MIGRATIONS["ssh-mcp"]["removed"] == c.SERVER_SPECS["ssh-mcp"].removed_flags
|
||||||
|
|
||||||
|
|
||||||
|
def test_resolve_server_spec_matches_detect_migratable_package():
|
||||||
|
for data in (
|
||||||
|
{"command": "npx", "args": ["-y", "ssh-mcp"]},
|
||||||
|
{"command": "ssh-mcp", "args": []},
|
||||||
|
{"command": "/usr/local/bin/ssh-mcp", "args": []},
|
||||||
|
{"command": "npx", "args": ["some-other"]},
|
||||||
|
{},
|
||||||
|
{"url": "https://x"},
|
||||||
|
):
|
||||||
|
spec = c.resolve_server_spec(data)
|
||||||
|
pkg = c.detect_migratable_package(data)
|
||||||
|
assert (spec.package if spec else None) == pkg
|
||||||
|
|
||||||
|
|
||||||
|
def test_drift_warning_maxchars_none_inline_and_separate():
|
||||||
|
inline = {"command": "ssh-mcp", "args": ["--maxChars=none"]}
|
||||||
|
separate = {"command": "ssh-mcp", "args": ["--maxChars", "none"]}
|
||||||
|
for data in (inline, separate):
|
||||||
|
warnings = c.drift_warnings(data)
|
||||||
|
assert len(warnings) == 1
|
||||||
|
assert "--maxChars=none" in warnings[0]
|
||||||
|
assert "5000" in warnings[0]
|
||||||
|
|
||||||
|
|
||||||
|
def test_drift_warning_quiet_for_other_values_and_unknown_pkg():
|
||||||
|
# A real numeric cap is fine; only "none" drifts.
|
||||||
|
assert c.drift_warnings({"command": "ssh-mcp", "args": ["--maxChars=8000"]}) == []
|
||||||
|
assert c.drift_warnings({"command": "ssh-mcp", "args": ["--host=h"]}) == []
|
||||||
|
# Unknown package: nothing to say.
|
||||||
|
assert c.drift_warnings({"command": "npx", "args": ["other", "--maxChars=none"]}) == []
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# Sidecar config detection + precedence (issue #91)
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
_SSH = {"command": "npx", "args": ["-y", "ssh-mcp", "--host=h", "--user=u"]}
|
||||||
|
_HOME = "/Users/tester"
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_path_verified_per_platform():
|
||||||
|
# NOTE: assert on .as_posix() — the CI matrix includes a Windows runner where
|
||||||
|
# str(Path("/Users/…")) would render with backslashes. as_posix() normalises
|
||||||
|
# separators so these platform-parameterised checks are portable.
|
||||||
|
spec = c.SERVER_SPECS["ssh-mcp"]
|
||||||
|
mac = c.sidecar_path(spec, platform="darwin", environ={}, home=_HOME)
|
||||||
|
assert mac.as_posix() == "/Users/tester/Library/Application Support/ssh-mcp/config.toml"
|
||||||
|
# Windows resolves under %APPDATA%, not ~/.config.
|
||||||
|
win = c.sidecar_path(
|
||||||
|
spec, platform="win32", environ={"APPDATA": "C:/Users/t/AppData/Roaming"}, home=_HOME
|
||||||
|
)
|
||||||
|
assert "ssh-mcp/config.toml" in win.as_posix()
|
||||||
|
assert "AppData/Roaming" in win.as_posix()
|
||||||
|
# POSIX honours XDG_CONFIG_HOME, else ~/.config.
|
||||||
|
xdg = c.sidecar_path(spec, platform="linux", environ={"XDG_CONFIG_HOME": "/cfg"}, home=_HOME)
|
||||||
|
assert xdg.as_posix() == "/cfg/ssh-mcp/config.toml"
|
||||||
|
default = c.sidecar_path(spec, platform="linux", environ={}, home=_HOME)
|
||||||
|
assert default.as_posix() == "/Users/tester/.config/ssh-mcp/config.toml"
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_path_none_for_non_sidecar_package():
|
||||||
|
assert c.sidecar_path(None) is None
|
||||||
|
assert c.sidecar_path(c.ServerSpec(package="nope")) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_args_inert_when_file_exists():
|
||||||
|
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="darwin", environ={}, home=_HOME)
|
||||||
|
st = c.sidecar_status(
|
||||||
|
_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: p == real
|
||||||
|
)
|
||||||
|
assert st["exists"] is True
|
||||||
|
assert st["has_managed_args"] is True
|
||||||
|
assert st["args_inert"] is True
|
||||||
|
assert st["wrong_path"] is False
|
||||||
|
warns = c.sidecar_warnings(
|
||||||
|
_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: p == real
|
||||||
|
)
|
||||||
|
assert warns and "inert" in warns[0]
|
||||||
|
assert str(real) in warns[0]
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_args_live_when_file_absent():
|
||||||
|
st = c.sidecar_status(_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: False)
|
||||||
|
assert st["exists"] is False
|
||||||
|
assert st["args_inert"] is False
|
||||||
|
assert (
|
||||||
|
c.sidecar_warnings(_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: False)
|
||||||
|
== []
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_wrong_path_flag_on_macos():
|
||||||
|
# A TOML written at the README's ~/.config path is never read on macOS.
|
||||||
|
doc = c.sidecar_doc_path(c.SERVER_SPECS["ssh-mcp"], home=_HOME)
|
||||||
|
assert doc.as_posix() == "/Users/tester/.config/ssh-mcp/config.toml"
|
||||||
|
st = c.sidecar_status(
|
||||||
|
_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: p == doc
|
||||||
|
)
|
||||||
|
assert st["wrong_path"] is True
|
||||||
|
assert st["args_inert"] is False # the real file doesn't exist, so args still apply
|
||||||
|
warns = c.sidecar_warnings(
|
||||||
|
_SSH, platform="darwin", environ={}, home=_HOME, exists=lambda p: p == doc
|
||||||
|
)
|
||||||
|
assert warns and "never loaded" in warns[0]
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_no_wrong_path_on_linux_where_paths_coincide():
|
||||||
|
# On Linux the real path and the doc path are the same, so a file there is
|
||||||
|
# correctly loaded — no wrong-path warning.
|
||||||
|
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="linux", environ={}, home=_HOME)
|
||||||
|
st = c.sidecar_status(
|
||||||
|
_SSH, platform="linux", environ={}, home=_HOME, exists=lambda p: p == real
|
||||||
|
)
|
||||||
|
assert st["wrong_path"] is False
|
||||||
|
assert st["args_inert"] is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_count_toml_profiles():
|
||||||
|
assert c.count_toml_profiles("") == 0
|
||||||
|
assert c.count_toml_profiles("[server]\nhost='h'\n") == 1
|
||||||
|
text = "# comment\n[[hosts]]\nname='a'\n[[hosts]]\nname='b'\n[settings]\nx=1\n"
|
||||||
|
# two [[hosts]] share a top-level name -> one profile group; [settings] -> another.
|
||||||
|
assert c.count_toml_profiles(text) == 2
|
||||||
|
multi = "[prod]\nhost='p'\n[prod.auth]\nkey='k'\n[staging]\nhost='s'\n"
|
||||||
|
assert c.count_toml_profiles(multi) == 2
|
||||||
|
|
||||||
|
|
||||||
|
def test_unscoped_credential_warning_11():
|
||||||
|
data = dict(_SSH, env={"SSH_MCP_PASSWORD": "shared"})
|
||||||
|
# Single profile: no sharing concern.
|
||||||
|
assert c.unscoped_credential_warning(data, profile_count=1) is None
|
||||||
|
# Unknown count: claim nothing.
|
||||||
|
assert c.unscoped_credential_warning(data, profile_count=None) is None
|
||||||
|
# Multiple profiles + bare credential: warn.
|
||||||
|
w = c.unscoped_credential_warning(data, profile_count=3)
|
||||||
|
assert w and "every one of the 3 profiles" in w
|
||||||
|
# No bare credential: nothing to warn.
|
||||||
|
assert c.unscoped_credential_warning(_SSH, profile_count=3) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_warnings_includes_credential_scoping():
|
||||||
|
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="darwin", environ={}, home=_HOME)
|
||||||
|
data = dict(_SSH, env={"SSH_MCP_PASSWORD": "shared"})
|
||||||
|
toml = "[prod]\nhost='p'\n[staging]\nhost='s'\n"
|
||||||
|
warns = c.sidecar_warnings(
|
||||||
|
data,
|
||||||
|
platform="darwin",
|
||||||
|
environ={},
|
||||||
|
home=_HOME,
|
||||||
|
exists=lambda p: p == real,
|
||||||
|
read_text=lambda p: toml,
|
||||||
|
)
|
||||||
|
assert any("profiles" in w for w in warns)
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_status_none_for_unknown_package():
|
||||||
|
assert c.sidecar_status({"command": "npx", "args": ["other"]}) is None
|
||||||
|
assert c.sidecar_warnings({"command": "npx", "args": ["other"]}) == []
|
||||||
|
|
||||||
|
|
||||||
|
# Version resolve + pin + drift (issue #92)
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_server_package_spec_and_name():
|
||||||
|
assert c.server_package_spec({"command": "npx", "args": ["-y", "ssh-mcp"]}) == "ssh-mcp"
|
||||||
|
assert c.server_package_spec({"command": "npx", "args": ["ssh-mcp@2.1.0"]}) == "ssh-mcp@2.1.0"
|
||||||
|
assert c.server_package_name({"command": "npx", "args": ["ssh-mcp@2.1.0"]}) == "ssh-mcp"
|
||||||
|
# A direct binary launch has no run-time spec to pin.
|
||||||
|
assert c.server_package_spec({"command": "ssh-mcp", "args": []}) is None
|
||||||
|
assert c.server_package_spec({"url": "https://x"}) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_is_unpinned_spec():
|
||||||
|
assert c.is_unpinned_spec({"command": "npx", "args": ["-y", "ssh-mcp"]}) is True
|
||||||
|
assert c.is_unpinned_spec({"command": "npx", "args": ["ssh-mcp@latest"]}) is True
|
||||||
|
assert c.is_unpinned_spec({"command": "npx", "args": ["ssh-mcp@next"]}) is True
|
||||||
|
assert c.is_unpinned_spec({"command": "npx", "args": ["ssh-mcp@2.1.0"]}) is False
|
||||||
|
assert c.is_unpinned_spec({"command": "ssh-mcp", "args": []}) is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_parse_package_json_version():
|
||||||
|
assert c.parse_package_json_version('{"name":"ssh-mcp","version":"2.1.0"}') == "2.1.0"
|
||||||
|
assert c.parse_package_json_version('{"name":"x"}') is None
|
||||||
|
assert c.parse_package_json_version("not json") is None
|
||||||
|
assert c.parse_package_json_version('{"version":""}') is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_resolved_npx_version_reads_cache_highest_wins():
|
||||||
|
# Two cache entries for ssh-mcp; the highest version wins. No real filesystem.
|
||||||
|
files = {
|
||||||
|
"/h/.npm/_npx/aaa/node_modules/ssh-mcp/package.json": '{"version":"1.9.0"}',
|
||||||
|
"/h/.npm/_npx/bbb/node_modules/ssh-mcp/package.json": '{"version":"2.1.0"}',
|
||||||
|
}
|
||||||
|
got = c.resolved_npx_version(
|
||||||
|
"ssh-mcp",
|
||||||
|
home="/h",
|
||||||
|
find=lambda pat: list(files),
|
||||||
|
read=lambda p: files[p],
|
||||||
|
)
|
||||||
|
assert got == "2.1.0"
|
||||||
|
|
||||||
|
|
||||||
|
def test_resolved_npx_version_unknown_degrades_to_none():
|
||||||
|
assert (
|
||||||
|
c.resolved_npx_version("ssh-mcp", home="/h", find=lambda pat: [], read=lambda p: "") is None
|
||||||
|
)
|
||||||
|
assert c.resolved_npx_version("", home="/h") is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_pin_spec_transform():
|
||||||
|
data = {"command": "npx", "args": ["-y", "ssh-mcp", "--host=h"]}
|
||||||
|
new, note = c.pin_spec_transform(data, "2.1.0")
|
||||||
|
assert new["args"] == ["-y", "ssh-mcp@2.1.0", "--host=h"]
|
||||||
|
assert note and "2.1.0" in note
|
||||||
|
# Already pinned to that exact version -> no-op.
|
||||||
|
again, note2 = c.pin_spec_transform(new, "2.1.0")
|
||||||
|
assert again == new
|
||||||
|
assert note2 is None
|
||||||
|
# Bad / empty version -> no-op.
|
||||||
|
assert c.pin_spec_transform(data, "")[1] is None
|
||||||
|
assert c.pin_spec_transform(data, "latest")[1] is None
|
||||||
|
# Not an npx server -> no-op.
|
||||||
|
assert c.pin_spec_transform({"command": "ssh-mcp", "args": []}, "2.1.0")[1] is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_version_drift_note():
|
||||||
|
assert c.version_drift_note("2.1.0", "3.0.0") == "moved 2.1.0 → 3.0.0 since you pinned"
|
||||||
|
assert c.version_drift_note("3.0.0", "3.0.0") is None
|
||||||
|
assert c.version_drift_note("3.0.0", "2.1.0") is None # never a backwards "drift"
|
||||||
|
assert c.version_drift_note(None, "3.0.0") is None
|
||||||
|
assert c.version_drift_note("2.1.0", None) is None
|
||||||
|
# Numeric, not lexical: 2 < 10.
|
||||||
|
assert c.version_drift_note("2.0.0", "10.0.0") is not None
|
||||||
|
|
||||||
|
|
||||||
|
def test_version_status_unpinned_offers_pin():
|
||||||
|
st = c.version_status({"command": "npx", "args": ["-y", "ssh-mcp"]}, resolved="2.1.0")
|
||||||
|
assert st["package"] == "ssh-mcp"
|
||||||
|
assert st["unpinned"] is True
|
||||||
|
assert st["pinned_version"] is None
|
||||||
|
assert st["resolved_version"] == "2.1.0"
|
||||||
|
assert st["can_pin"] is True
|
||||||
|
assert st["drift"] is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_version_status_pinned_reports_drift():
|
||||||
|
st = c.version_status({"command": "npx", "args": ["ssh-mcp@2.1.0"]}, resolved="3.0.0")
|
||||||
|
assert st["unpinned"] is False
|
||||||
|
assert st["pinned_version"] == "2.1.0"
|
||||||
|
assert st["can_pin"] is False # already pinned
|
||||||
|
assert st["drift"] == "moved 2.1.0 → 3.0.0 since you pinned"
|
||||||
|
|
||||||
|
|
||||||
|
def test_version_status_unknown_resolved_cannot_pin():
|
||||||
|
st = c.version_status({"command": "npx", "args": ["-y", "ssh-mcp"]}, resolved=None)
|
||||||
|
assert st["unpinned"] is True
|
||||||
|
assert st["resolved_version"] is None
|
||||||
|
assert st["can_pin"] is False # nothing to pin TO
|
||||||
|
assert st["drift"] is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_version_status_none_for_non_npx():
|
||||||
|
assert c.version_status({"command": "ssh-mcp", "args": []}) is None
|
||||||
|
assert c.version_status({"url": "https://x"}) is None
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# Filesystem permission pre-flight (issue #93)
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_permission_status_ok_when_0600():
|
||||||
|
# NOTE: key the injected mode map on Path(p).as_posix() — on the Windows CI
|
||||||
|
# runner permission_status wraps the path in a WindowsPath, so str(p) would
|
||||||
|
# use backslashes and miss the lookup (the same portability trap as #91).
|
||||||
|
st = c.permission_status(
|
||||||
|
"/cfg/config.toml",
|
||||||
|
platform="darwin",
|
||||||
|
stat_mode=lambda p: {"/cfg/config.toml": 0o600, "/cfg": 0o700}.get(Path(p).as_posix()),
|
||||||
|
)
|
||||||
|
assert st["ok"] is True
|
||||||
|
assert st["mode"] == 0o600
|
||||||
|
assert st["problems"] == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_permission_status_blocks_group_world_readable_file():
|
||||||
|
st = c.permission_status(
|
||||||
|
"/cfg/config.toml",
|
||||||
|
platform="linux",
|
||||||
|
stat_mode=lambda p: {"/cfg/config.toml": 0o644, "/cfg": 0o755}.get(Path(p).as_posix()),
|
||||||
|
)
|
||||||
|
assert st["ok"] is False
|
||||||
|
assert st["file_ok"] is False
|
||||||
|
assert st["dir_ok"] is False
|
||||||
|
# Two plain-language problems, naming the offending octal modes.
|
||||||
|
text = " ".join(st["problems"])
|
||||||
|
assert "0644" in text and "0600" in text
|
||||||
|
assert "0755" in text and "0700" in text
|
||||||
|
|
||||||
|
|
||||||
|
def test_permission_status_file_bad_dir_ok():
|
||||||
|
st = c.permission_status(
|
||||||
|
"/cfg/config.toml",
|
||||||
|
platform="linux",
|
||||||
|
stat_mode=lambda p: {"/cfg/config.toml": 0o640, "/cfg": 0o700}.get(Path(p).as_posix()),
|
||||||
|
)
|
||||||
|
assert st["file_ok"] is False
|
||||||
|
assert st["dir_ok"] is True
|
||||||
|
assert len(st["problems"]) == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_permission_status_none_on_windows_and_missing_file():
|
||||||
|
# Windows: POSIX modes don't apply -> None (clean no-op).
|
||||||
|
assert c.permission_status("/cfg/config.toml", platform="win32") is None
|
||||||
|
# Absent file -> nothing to pre-flight.
|
||||||
|
assert c.permission_status("/cfg/gone.toml", platform="linux", stat_mode=lambda p: None) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_fix_permissions_chmods_file_and_dir():
|
||||||
|
# as_posix() so the recorded paths compare equal on the Windows CI runner too.
|
||||||
|
calls = []
|
||||||
|
changed, note = c.fix_permissions(
|
||||||
|
"/cfg/config.toml",
|
||||||
|
platform="linux",
|
||||||
|
chmod=lambda p, m: calls.append((Path(p).as_posix(), m)),
|
||||||
|
)
|
||||||
|
assert changed is True
|
||||||
|
assert ("/cfg/config.toml", 0o600) in calls
|
||||||
|
assert ("/cfg", 0o700) in calls
|
||||||
|
assert note and "0600" in note
|
||||||
|
|
||||||
|
|
||||||
|
def test_fix_permissions_noop_on_windows():
|
||||||
|
calls = []
|
||||||
|
changed, note = c.fix_permissions(
|
||||||
|
"/cfg/config.toml", platform="win32", chmod=lambda p, m: calls.append((p, m))
|
||||||
|
)
|
||||||
|
assert changed is False
|
||||||
|
assert note is None
|
||||||
|
assert calls == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_fix_permissions_reports_oserror():
|
||||||
|
def boom(p, m):
|
||||||
|
raise OSError("nope")
|
||||||
|
|
||||||
|
changed, note = c.fix_permissions("/cfg/config.toml", platform="linux", chmod=boom)
|
||||||
|
assert changed is False
|
||||||
|
assert "could not change permissions" in note
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_permission_warnings_over_real_path():
|
||||||
|
data = {"command": "npx", "args": ["-y", "ssh-mcp", "--host=h"]}
|
||||||
|
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="darwin", environ={}, home="/Users/t")
|
||||||
|
|
||||||
|
def stat_mode(p):
|
||||||
|
return 0o644 if Path(p) == real else 0o700
|
||||||
|
|
||||||
|
warns = c.sidecar_permission_warnings(
|
||||||
|
data, platform="darwin", environ={}, home="/Users/t", stat_mode=stat_mode
|
||||||
|
)
|
||||||
|
assert warns and warns[0].startswith("ssh-mcp:")
|
||||||
|
assert "0600" in warns[0]
|
||||||
|
# Windows / unknown package -> nothing.
|
||||||
|
assert c.sidecar_permission_warnings(data, platform="win32") == []
|
||||||
|
assert c.sidecar_permission_warnings({"command": "npx", "args": ["other"]}) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_permission_warnings_quiet_when_tight():
|
||||||
|
data = {"command": "npx", "args": ["-y", "ssh-mcp", "--host=h"]}
|
||||||
|
warns = c.sidecar_permission_warnings(
|
||||||
|
data, platform="darwin", environ={}, home="/Users/t", stat_mode=lambda p: 0o600
|
||||||
|
)
|
||||||
|
assert warns == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_permission_fix_target():
|
||||||
|
data = {"command": "npx", "args": ["-y", "ssh-mcp"]}
|
||||||
|
tgt = c.sidecar_permission_fix_target(data, platform="darwin", environ={}, home="/Users/t")
|
||||||
|
assert tgt is not None and tgt.name == "config.toml"
|
||||||
|
assert c.sidecar_permission_fix_target(data, platform="win32") is None
|
||||||
|
assert c.sidecar_permission_fix_target({"command": "npx", "args": ["other"]}) is None
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# Hot-reload — live external-change detection (issue #101)
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_sidecar_watch_paths_covers_file_dir_and_doc():
|
||||||
|
data = dict(_SSH)
|
||||||
|
paths = c.sidecar_watch_paths(data, platform="darwin", environ={}, home=_HOME)
|
||||||
|
posix = [p.as_posix() for p in paths]
|
||||||
|
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="darwin", environ={}, home=_HOME)
|
||||||
|
# The resolved sidecar file AND its directory (so create/delete registers).
|
||||||
|
assert real.as_posix() in posix
|
||||||
|
assert real.parent.as_posix() in posix
|
||||||
|
# The README/doc path is distinct on macOS -> also watched.
|
||||||
|
doc = c.sidecar_doc_path(c.SERVER_SPECS["ssh-mcp"], home=_HOME)
|
||||||
|
assert doc.as_posix() in posix
|
||||||
|
# No duplicates.
|
||||||
|
assert len(posix) == len(set(posix))
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_watch_paths_dedupes_on_linux():
|
||||||
|
# On Linux the doc path coincides with the real path, so the file + dir are
|
||||||
|
# each listed once, not twice.
|
||||||
|
data = dict(_SSH)
|
||||||
|
paths = c.sidecar_watch_paths(
|
||||||
|
data, platform="linux", environ={"XDG_CONFIG_HOME": "/cfg"}, home=_HOME
|
||||||
|
)
|
||||||
|
posix = [p.as_posix() for p in paths]
|
||||||
|
assert posix == list(dict.fromkeys(posix)) # order-preserving de-dup is a no-op
|
||||||
|
assert "/cfg/ssh-mcp/config.toml" in posix
|
||||||
|
assert "/cfg/ssh-mcp" in posix
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_watch_paths_empty_for_non_sidecar():
|
||||||
|
assert c.sidecar_watch_paths({"command": "npx", "args": ["other"]}) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_state_fingerprint_reflects_appearance_and_perms():
|
||||||
|
data = dict(_SSH)
|
||||||
|
real = c.sidecar_path(c.SERVER_SPECS["ssh-mcp"], platform="darwin", environ={}, home=_HOME)
|
||||||
|
|
||||||
|
# File absent -> a fingerprint that encodes "not there, no perms".
|
||||||
|
absent = c.sidecar_state_fingerprint(
|
||||||
|
data, platform="darwin", environ={}, home=_HOME, exists=lambda p: False
|
||||||
|
)
|
||||||
|
# File present but world-readable.
|
||||||
|
bad = c.sidecar_state_fingerprint(
|
||||||
|
data,
|
||||||
|
platform="darwin",
|
||||||
|
environ={},
|
||||||
|
home=_HOME,
|
||||||
|
exists=lambda p: Path(p) == real,
|
||||||
|
stat_mode=lambda p: 0o644 if Path(p) == real else 0o700,
|
||||||
|
)
|
||||||
|
# File present and tight.
|
||||||
|
good = c.sidecar_state_fingerprint(
|
||||||
|
data,
|
||||||
|
platform="darwin",
|
||||||
|
environ={},
|
||||||
|
home=_HOME,
|
||||||
|
exists=lambda p: Path(p) == real,
|
||||||
|
stat_mode=lambda p: 0o600 if Path(p) == real else 0o700,
|
||||||
|
)
|
||||||
|
assert absent is not None
|
||||||
|
# Each observable transition changes the fingerprint.
|
||||||
|
assert c.sidecar_state_changed(absent, bad)
|
||||||
|
assert c.sidecar_state_changed(bad, good)
|
||||||
|
assert c.sidecar_state_changed(absent, good)
|
||||||
|
# Stable when nothing changed.
|
||||||
|
assert not c.sidecar_state_changed(good, good)
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_state_fingerprint_none_for_non_sidecar():
|
||||||
|
assert c.sidecar_state_fingerprint({"command": "npx", "args": ["other"]}) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_sidecar_state_fingerprint_tracks_args_inert_flip():
|
||||||
|
# args_inert only becomes true once the sidecar file exists AND managed args
|
||||||
|
# are present; the fingerprint must capture that precedence flip.
|
||||||
|
data = dict(_SSH) # carries --host/--user (managed args)
|
||||||
|
real = c.sidecar_path(
|
||||||
|
c.SERVER_SPECS["ssh-mcp"], platform="linux", environ={"XDG_CONFIG_HOME": "/cfg"}, home=_HOME
|
||||||
|
)
|
||||||
|
before = c.sidecar_state_fingerprint(
|
||||||
|
data,
|
||||||
|
platform="linux",
|
||||||
|
environ={"XDG_CONFIG_HOME": "/cfg"},
|
||||||
|
home=_HOME,
|
||||||
|
exists=lambda p: False,
|
||||||
|
stat_mode=lambda p: None,
|
||||||
|
)
|
||||||
|
after = c.sidecar_state_fingerprint(
|
||||||
|
data,
|
||||||
|
platform="linux",
|
||||||
|
environ={"XDG_CONFIG_HOME": "/cfg"},
|
||||||
|
home=_HOME,
|
||||||
|
exists=lambda p: Path(p) == real,
|
||||||
|
stat_mode=lambda p: 0o600,
|
||||||
|
)
|
||||||
|
assert c.sidecar_state_changed(before, after)
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# Sidecar TOML editing (issue #102)
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_read_toml_section_top_level_and_named():
|
||||||
|
text = (
|
||||||
|
"# a comment\n"
|
||||||
|
'title = "root"\n'
|
||||||
|
"port = 22\n"
|
||||||
|
"enabled = true\n"
|
||||||
|
"\n"
|
||||||
|
"[server]\n"
|
||||||
|
'host = "h"\n'
|
||||||
|
"auth = 'key'\n" # literal string
|
||||||
|
"weight = 1.5\n"
|
||||||
|
)
|
||||||
|
top = c.read_toml_section(text)
|
||||||
|
assert top == {"title": "root", "port": 22, "enabled": True}
|
||||||
|
srv = c.read_toml_section(text, "server")
|
||||||
|
assert srv == {"host": "h", "auth": "key", "weight": 1.5}
|
||||||
|
# An absent section reads as empty.
|
||||||
|
assert c.read_toml_section(text, "nope") == {}
|
||||||
|
|
||||||
|
|
||||||
|
def test_read_toml_section_skips_unparseable_values():
|
||||||
|
text = '[server]\nhosts = [1, 2, 3]\ninline = {a = 1}\nname = "ok"\n'
|
||||||
|
# Arrays / inline tables are omitted (surgical writer preserves them); the
|
||||||
|
# simple scalar is read.
|
||||||
|
assert c.read_toml_section(text, "server") == {"name": "ok"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_toml_sections_lists_groups_in_order():
|
||||||
|
text = "top = 1\n[server]\nx=1\n[prod]\na=1\n[prod.auth]\nk=1\n[[hosts]]\nn=1\n"
|
||||||
|
# top-level present -> "" first; [prod] and [prod.auth] collapse to one group.
|
||||||
|
assert c.toml_sections(text) == ["", "server", "prod", "hosts"]
|
||||||
|
assert c.toml_sections("[only]\nx=1\n") == ["only"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_toml_value_replaces_and_preserves_comment():
|
||||||
|
text = '[server]\nport = 22 # ssh\nhost = "h"\n'
|
||||||
|
out = c.set_toml_value(text, "port", 2222, section="server")
|
||||||
|
assert "port = 2222 # ssh" in out
|
||||||
|
# Nothing else changed.
|
||||||
|
assert 'host = "h"' in out
|
||||||
|
assert out.startswith("[server]\n")
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_toml_value_appends_missing_key_within_section():
|
||||||
|
text = '[server]\nhost = "h"\n\n[other]\nx = 1\n'
|
||||||
|
out = c.set_toml_value(text, "auth", "key", section="server")
|
||||||
|
lines = out.splitlines()
|
||||||
|
# The new key lands inside [server], before the blank line and [other].
|
||||||
|
assert lines.index('auth = "key"') < lines.index("[other]")
|
||||||
|
assert lines.index('auth = "key"') > lines.index("[server]")
|
||||||
|
assert "[other]" in out and "x = 1" in out
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_toml_value_creates_missing_section():
|
||||||
|
text = 'title = "root"\n'
|
||||||
|
out = c.set_toml_value(text, "role", "admin", section="server")
|
||||||
|
assert 'title = "root"' in out
|
||||||
|
assert "[server]" in out
|
||||||
|
assert 'role = "admin"' in out
|
||||||
|
# And it round-trips back through the reader.
|
||||||
|
assert c.read_toml_section(out, "server") == {"role": "admin"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_toml_value_top_level_key():
|
||||||
|
text = "a = 1\n[server]\nb = 2\n"
|
||||||
|
out = c.set_toml_value(text, "a", 5) # section=None -> top-level
|
||||||
|
assert out.startswith("a = 5\n")
|
||||||
|
# The [server] b is untouched.
|
||||||
|
assert c.read_toml_section(out, "server") == {"b": 2}
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_toml_value_delete_key():
|
||||||
|
text = '[server]\nhost = "h"\nport = 22\n'
|
||||||
|
out = c.set_toml_value(text, "port", None, section="server")
|
||||||
|
assert "port" not in out
|
||||||
|
assert 'host = "h"' in out
|
||||||
|
# Deleting an absent key is a no-op.
|
||||||
|
assert c.set_toml_value(text, "ghost", None, section="server") == text
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_toml_value_preserves_crlf():
|
||||||
|
text = "[server]\r\nport = 22\r\n"
|
||||||
|
out = c.set_toml_value(text, "port", 23, section="server")
|
||||||
|
assert "port = 23\r\n" in out
|
||||||
|
assert "\r\n" in out
|
||||||
|
|
||||||
|
|
||||||
|
def test_set_toml_value_escapes_strings():
|
||||||
|
out = c.set_toml_value("", "path", 'C:\\a\\"b"', section="win")
|
||||||
|
# Backslashes and quotes are escaped; it reads back to the exact original.
|
||||||
|
assert c.read_toml_section(out, "win") == {"path": 'C:\\a\\"b"'}
|
||||||
|
|
||||||
|
|
||||||
|
def test_update_toml_batch_and_roundtrip():
|
||||||
|
text = '[server]\nhost = "h" # keep me\n'
|
||||||
|
out = c.update_toml(text, {"host": "newhost", "port": 22, "auth": "key"}, section="server")
|
||||||
|
assert c.read_toml_section(out, "server") == {"host": "newhost", "port": 22, "auth": "key"}
|
||||||
|
# The comment on the pre-existing host line survives the value change.
|
||||||
|
assert "# keep me" in out
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_sidecar_values_enums_and_port():
|
||||||
|
schema = c.SERVER_SPECS["ssh-mcp"].schema
|
||||||
|
# All valid -> no problems.
|
||||||
|
assert (
|
||||||
|
c.validate_sidecar_values(
|
||||||
|
{"auth": "key", "approvalMode": "ask-all", "role": "admin", "port": 22}, schema
|
||||||
|
)
|
||||||
|
== []
|
||||||
|
)
|
||||||
|
# Bad enum value.
|
||||||
|
probs = c.validate_sidecar_values({"auth": "sshkey"}, schema)
|
||||||
|
assert probs and "auth" in probs[0] and "sshkey" in probs[0]
|
||||||
|
# Out-of-range port, and a bool is not a valid int port.
|
||||||
|
assert c.validate_sidecar_values({"port": 70000}, schema)
|
||||||
|
assert c.validate_sidecar_values({"port": True}, schema)
|
||||||
|
assert c.validate_sidecar_values({"port": 1}, schema) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_validate_sidecar_values_passes_through_unknown_keys():
|
||||||
|
schema = c.SERVER_SPECS["ssh-mcp"].schema
|
||||||
|
# An unmanaged key is preserved, never a save-blocker.
|
||||||
|
assert c.validate_sidecar_values({"customThing": "whatever"}, schema) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_write_sidecar_atomic_backup_and_chmod(tmp_path):
|
||||||
|
p = tmp_path / "ssh-mcp" / "config.toml"
|
||||||
|
p.parent.mkdir()
|
||||||
|
p.write_text("[server]\nport = 22\n", encoding="utf-8")
|
||||||
|
|
||||||
|
chmods: list[tuple[str, int]] = []
|
||||||
|
new_text = c.set_toml_value(p.read_text(encoding="utf-8"), "port", 2222, section="server")
|
||||||
|
backup = c.write_sidecar(
|
||||||
|
p,
|
||||||
|
new_text,
|
||||||
|
platform="linux",
|
||||||
|
chmod=lambda path, mode: chmods.append((Path(path).as_posix(), mode)),
|
||||||
|
)
|
||||||
|
|
||||||
|
# File updated atomically with the new content.
|
||||||
|
assert c.read_toml_section(p.read_text(encoding="utf-8"), "server") == {"port": 2222}
|
||||||
|
# A timestamped backup of the prior content was made, under .toml.
|
||||||
|
assert backup is not None and backup.suffix == ".toml"
|
||||||
|
assert c.read_toml_section(backup.read_text(encoding="utf-8"), "server") == {"port": 22}
|
||||||
|
# Permissions tightened: file 0600, dir 0700 (via #93's fix_permissions).
|
||||||
|
assert (p.as_posix(), 0o600) in chmods
|
||||||
|
assert (p.parent.as_posix(), 0o700) in chmods
|
||||||
|
|
||||||
|
|
||||||
|
def test_write_sidecar_new_file_no_backup(tmp_path):
|
||||||
|
p = tmp_path / "config.toml"
|
||||||
|
backup = c.write_sidecar(
|
||||||
|
p, '[server]\nrole = "viewer"\n', platform="linux", chmod=lambda *_: None
|
||||||
|
)
|
||||||
|
assert backup is None # nothing pre-existing to back up
|
||||||
|
assert p.read_text(encoding="utf-8").endswith("\n")
|
||||||
|
assert c.read_toml_section(p.read_text(encoding="utf-8"), "server") == {"role": "viewer"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_write_sidecar_does_not_touch_apply_servers(tmp_path):
|
||||||
|
# Guard the cardinal rule: write_sidecar writes ONLY the text it is given —
|
||||||
|
# no mcpServers / _disabledMcpServers keys are introduced.
|
||||||
|
p = tmp_path / "config.toml"
|
||||||
|
c.write_sidecar(p, '[server]\nhost = "h"\n', platform="linux", chmod=lambda *_: None)
|
||||||
|
body = p.read_text(encoding="utf-8")
|
||||||
|
assert "mcpServers" not in body and "_disabledMcpServers" not in body
|
||||||
|
|
||||||
|
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
# Move to environment variable (issue #83)
|
# Move to environment variable (issue #83)
|
||||||
# --------------------------------------------------------------------------- #
|
# --------------------------------------------------------------------------- #
|
||||||
@@ -3148,3 +4003,86 @@ def test_is_env_var_set():
|
|||||||
assert c.is_env_var_set("FOO", {"FOO": "x"}) is True
|
assert c.is_env_var_set("FOO", {"FOO": "x"}) is True
|
||||||
assert c.is_env_var_set("FOO", {"FOO": ""}) is False
|
assert c.is_env_var_set("FOO", {"FOO": ""}) is False
|
||||||
assert c.is_env_var_set("FOO", {}) is False
|
assert c.is_env_var_set("FOO", {}) is False
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# Args secret indices + suggested var name (issue #83, args surface)
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_secret_arg_indices_flags_token_and_flag_value():
|
||||||
|
args = ["--port", "8080", "ghp_deadbeef", "--token", "sk-abc", "--flag=val"]
|
||||||
|
idxs = c.secret_arg_indices(args)
|
||||||
|
assert 2 in idxs # ghp_ token prefix
|
||||||
|
assert 4 in idxs # value following --token
|
||||||
|
assert 1 not in idxs # 8080
|
||||||
|
assert 5 not in idxs # --flag=val inline pair
|
||||||
|
|
||||||
|
|
||||||
|
def test_secret_arg_indices_flags_embedded_url_credentials():
|
||||||
|
args = ["postgres://user:pass@host/db"]
|
||||||
|
assert c.secret_arg_indices(args) == [0]
|
||||||
|
|
||||||
|
|
||||||
|
def test_secret_arg_indices_excludes_existing_references():
|
||||||
|
args = ["--token", "${GH_TOKEN}"]
|
||||||
|
assert c.secret_arg_indices(args) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_suggested_env_var_for_arg_uses_preceding_flag():
|
||||||
|
args = ["--api-key", "sk-secret"]
|
||||||
|
assert c.suggested_env_var_for_arg(args, 1) == "API_KEY"
|
||||||
|
|
||||||
|
|
||||||
|
def test_suggested_env_var_for_arg_falls_back_when_no_flag():
|
||||||
|
args = ["ghp_secret"]
|
||||||
|
assert c.suggested_env_var_for_arg(args, 0) == "SECRET"
|
||||||
|
|
||||||
|
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
# Referenced-variables readout + args->env relocation (issue #83, "both")
|
||||||
|
# --------------------------------------------------------------------------- #
|
||||||
|
def test_referenced_env_vars_dedupes_and_reports_status():
|
||||||
|
data = {
|
||||||
|
"command": "npx",
|
||||||
|
"args": ["--token", "${GH_TOKEN}", "${GH_TOKEN}"],
|
||||||
|
"env": {"API_KEY": "${API_KEY}", "REGION": "${REGION:-us-east-1}"},
|
||||||
|
}
|
||||||
|
usages = c.referenced_env_vars(data, environ={"GH_TOKEN": "x"})
|
||||||
|
by = {u.name: u for u in usages}
|
||||||
|
assert set(by) == {"GH_TOKEN", "API_KEY", "REGION"}
|
||||||
|
# GH_TOKEN appears only in args, deduped to one entry, set in env -> resolved
|
||||||
|
assert by["GH_TOKEN"].fields == ("args",)
|
||||||
|
assert by["GH_TOKEN"].resolved is True
|
||||||
|
# API_KEY not set, no default -> unresolved
|
||||||
|
assert by["API_KEY"].resolved is False
|
||||||
|
# REGION has a default -> resolved regardless of environment
|
||||||
|
assert by["REGION"].has_default is True
|
||||||
|
assert by["REGION"].resolved is True
|
||||||
|
# sorted by name
|
||||||
|
assert [u.name for u in usages] == sorted(u.name for u in usages)
|
||||||
|
|
||||||
|
|
||||||
|
def test_referenced_env_vars_empty_when_no_refs():
|
||||||
|
assert c.referenced_env_vars({"command": "npx", "args": ["-y", "pkg"]}) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_move_arg_to_env_block_removes_flag_and_value():
|
||||||
|
data = {"command": "x", "args": ["--api-key", "sk-secret", "run"], "env": {"KEEP": "1"}}
|
||||||
|
out = c.move_arg_to_env_block(data, 1)
|
||||||
|
assert out["args"] == ["run"] # flag + value both gone
|
||||||
|
assert out["env"]["API_KEY"] == "sk-secret"
|
||||||
|
assert out["env"]["KEEP"] == "1" # existing env preserved
|
||||||
|
# input not mutated
|
||||||
|
assert data["args"] == ["--api-key", "sk-secret", "run"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_move_arg_to_env_block_bare_positional_keeps_no_flag():
|
||||||
|
data = {"command": "x", "args": ["ghp_secret", "serve"]}
|
||||||
|
out = c.move_arg_to_env_block(data, 0, var_name="GITHUB_TOKEN")
|
||||||
|
assert out["args"] == ["serve"]
|
||||||
|
assert out["env"] == {"GITHUB_TOKEN": "ghp_secret"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_move_arg_to_env_block_none_on_bad_target():
|
||||||
|
assert c.move_arg_to_env_block({"args": ["a"]}, 5) is None
|
||||||
|
assert c.move_arg_to_env_block({"args": ["${REF}"]}, 0) is None # already a ref
|
||||||
|
assert c.move_arg_to_env_block({}, 0) is None
|
||||||
|
|||||||
Reference in New Issue
Block a user