Compare commits
36 Commits
8309e42dd8
..
v1.3.0
| Author | SHA1 | Date | |
|---|---|---|---|
| e6b60e94e7 | |||
| 4afe21666d | |||
| 06e74d4d2c | |||
| f92b851127 | |||
| 47c95ac006 | |||
| 6b22ad26f0 | |||
| 874948506c | |||
| ac2e73e9d7 | |||
| 82ff149373 | |||
| 31ef4a0e85 | |||
| 520b1b2ffd | |||
| 9f535fb77f | |||
| 8cf19d43c4 | |||
| 0ef4586698 | |||
| ed7c40cac9 | |||
| 1384ed9703 | |||
| 41891ddad4 | |||
| 408f517c5d | |||
| 9036729cd8 | |||
| 29a08e9532 | |||
| 87303809b8 | |||
| 42456f25d2 | |||
| 2d274b9e03 | |||
| 5c476bb13f | |||
| 62c8a2ea65 | |||
| b485357cd5 | |||
| 6a91f830dc | |||
| 6dacc26057 | |||
| 1087fc84d1 | |||
| 5df364fb2e | |||
| c7b2c90518 | |||
| c493aa0c84 | |||
| bb355dac31 | |||
| d95db2b026 | |||
| 42963f98b4 | |||
| f5c9780948 |
@@ -29,21 +29,37 @@ jobs:
|
||||
run: ruff format --check .
|
||||
|
||||
test:
|
||||
runs-on: ubuntu-latest
|
||||
name: Tests (py${{ matrix.python }})
|
||||
runs-on: ${{ matrix.os }}
|
||||
name: Tests (py${{ matrix.python }} / ${{ matrix.os }})
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
python: ["3.10", "3.12"]
|
||||
os: [ubuntu-latest]
|
||||
python: ["3.10", "3.12", "3.13"]
|
||||
include:
|
||||
# Windows tests on 3.12 only — the version the release binaries ship
|
||||
# with. The self-hosted Windows runner blocks setup-python's install
|
||||
# script (PowerShell execution policy), so it uses the host's `py`
|
||||
# launcher + venv, same as release.yml.
|
||||
- os: windows-latest
|
||||
python: "3.12"
|
||||
steps:
|
||||
- name: Checkout
|
||||
uses: actions/checkout@v4
|
||||
|
||||
- name: Set up Python ${{ matrix.python }}
|
||||
- name: Set up Python ${{ matrix.python }} (Linux)
|
||||
if: runner.os == 'Linux'
|
||||
uses: actions/setup-python@v5
|
||||
with:
|
||||
python-version: ${{ matrix.python }}
|
||||
|
||||
- name: Set up Python venv (Windows)
|
||||
if: runner.os == 'Windows'
|
||||
shell: pwsh
|
||||
run: |
|
||||
py -${{ matrix.python }} -m venv .venv
|
||||
Add-Content -Path $env:GITHUB_PATH -Value "$env:GITHUB_WORKSPACE\.venv\Scripts"
|
||||
|
||||
# bcc_core has no GUI imports, so the test suite needs no PySide6 —
|
||||
# keeps CI fast and avoids Qt system-library headaches on the runner.
|
||||
- name: Install test dependencies
|
||||
|
||||
@@ -1,112 +0,0 @@
|
||||
# BCC Roadmap — v1.2.0 readiness + what's next (Cowork planning, 2026-07-07)
|
||||
|
||||
Written by the Cowork supervisor session. Captures (1) what's ready for the next
|
||||
release, (2) the remaining tracked backlog, and (3) genuinely-missing features
|
||||
worth planning. Priorities are suggestions for AJ, not commitments.
|
||||
|
||||
---
|
||||
|
||||
## 1. v1.2.0 — ready to ship
|
||||
|
||||
Five feature branches landed as green, isolated, reviewed PRs (all approved by the
|
||||
supervisor session; each touches only `bcc.py` / `bcc_core.py` / `tests/test_core.py`
|
||||
and passes CI):
|
||||
|
||||
| PR | Issue | Feature | CI |
|
||||
|----|-------|---------|----|
|
||||
| #25 | #18 + #19 | About dialog (Help menu, repo/issues/license links) + notify-only update checker (`__version__`, numeric version-compare, fail-quiet Gitea releases fetch, throttled once/day startup check) | run 160 ✓ |
|
||||
| #24 | #9 | Restart Claude Desktop button (post-save, Desktop-profile-only, platform-abstracted `restart_claude_desktop()`) | run 159 ✓ |
|
||||
| #23 | #6 | In-app MCP server log viewer (`server_log_path()`, read-only auto-tailing panel per server) | run 158 ✓ |
|
||||
| #22 | #17 | Stale-file detection hardened with size (`ConfigStat` mtime+size fingerprint) | run 157 ✓ |
|
||||
| #21 | #8 | Duplicate-name collision fix — closes a real silent-overwrite on drag-and-drop import | run 164 ✓ |
|
||||
|
||||
Combined they take the suite from 92 tests toward ~130+ (each PR adds 3–20 tests).
|
||||
|
||||
### Merge / release checklist (needs AJ — this is the open decision)
|
||||
|
||||
The five branches all edit overlapping regions of `bcc.py`/`bcc_core.py`, so they
|
||||
will **not** merge cleanly in sequence without conflict resolution:
|
||||
|
||||
- **Shared conflict:** #22/#23/#24/#25 each rewrote the one `_normalize_unicode`
|
||||
non-breaking-space line (all behavior-identical). Merge one, then take that line
|
||||
from any branch for the rest.
|
||||
- **Save-flow region:** #21 (import prompt), #22 (stale fingerprint), #24 (restart
|
||||
button) all touch the save path in `bcc.py` — review the merged result once.
|
||||
- **Version bump:** at cut, set both `__version__` (bcc_core.py) and
|
||||
`pyproject.toml` to `1.2.0` and update `test_dunder_version_matches_pyproject`.
|
||||
- **Release is tag-triggered:** push a `v1.2.0` tag → `.github/workflows/release.yml`
|
||||
builds macOS/Windows/Linux and publishes. Don't hand-create the release.
|
||||
|
||||
Suggested merge order (least- to most-conflict-prone): **#21 → #22 → #23 → #24 → #25**.
|
||||
Recommend AJ does the merges + conflict resolution locally (cleaner than the Gitea
|
||||
API for overlapping diffs), or greenlights the supervisor to merge sequentially and
|
||||
patch conflicts on each branch via the API.
|
||||
|
||||
---
|
||||
|
||||
## 2. Remaining tracked backlog (open issues)
|
||||
|
||||
**Windows-specific — deferred (can't build/test in the Linux/mac sandbox):**
|
||||
|
||||
- **#20** Windows release shows no app icon (P?) — build-config fix in `bcc.spec` /
|
||||
`release.yml`; needs a Windows build to verify. Low risk, cosmetic.
|
||||
- **#7** Windows MSIX virtualized-path detection (P1) — config edits silently ignored
|
||||
on MSIX installs. Core path-detection is unit-testable; the runtime behavior needs
|
||||
a real MSIX Windows box. Genuinely valuable (silent failure class).
|
||||
- **#13** Windows process-tree kill is best-effort (P1) — follow-up refinement to #9;
|
||||
hard to verify without Windows.
|
||||
|
||||
**Deferred by design:**
|
||||
|
||||
- **#5** Cross-client support (Cursor `.cursor/mcp.json`, Windsurf
|
||||
`~/.codeium/windsurf/mcp_config.json`, VS Code `servers` key) — P1 but a behemoth;
|
||||
its own branch, revisit after single-client UX is solid. Not a v1.2 item.
|
||||
- **#10** Server catalog / one-click add — P2, design-first. See §3 overlap with search.
|
||||
|
||||
---
|
||||
|
||||
## 3. Genuinely-missing features (not yet tracked) — planning candidates
|
||||
|
||||
Grounded against what already exists (enable/disable toggle, duplicate-server,
|
||||
`_disabledMcpServers` round-trip, backup/restore, spawn-test, secret masking, JSON
|
||||
repair pipeline — all shipped, so not repeated here).
|
||||
|
||||
**P1 candidates**
|
||||
|
||||
1. **Server search / filter box.** Once a user has many servers (and especially once
|
||||
#10's catalog lands), there's no way to filter the list. Small, high-utility:
|
||||
a `QLineEdit` above the table filtering by name/command. Pure-ish; testable filter
|
||||
fn in core.
|
||||
2. **"Test all" / health column.** Extends spawn-test (#2) + log viewer (#6): a status
|
||||
dot per server (untested / ok / crashed) and a one-click "test all". Turns three
|
||||
separate features into an at-a-glance dashboard — the single biggest "is my config
|
||||
actually working?" answer.
|
||||
3. **Project-level `.mcp.json` discovery (Claude Code).** Claude Code reads project
|
||||
`.mcp.json` files; BCC only sees the global config. Detect and offer to edit the
|
||||
project file when relevant. Previously listed "not in backlog" — worth reconsidering
|
||||
now that Claude Code support is solid.
|
||||
|
||||
**P2 candidates**
|
||||
|
||||
4. **`${VAR}` / `.env` environment-variable expansion.** Previously declined (AJ's
|
||||
call). Revisit only if secret-handling demand grows — pairs with keychain below.
|
||||
5. **OS keychain integration for secrets.** Previously declined. Bigger lift; would
|
||||
let env secrets live outside the JSON entirely. Design-first if pursued.
|
||||
6. **Config schema/lint for well-known servers.** Warn on obvious mistakes (missing
|
||||
`command`, `args` not a list, unknown top-level keys) beyond the JSON-repair layer.
|
||||
7. **Theming / dark-mode follow-through.** Style constants (`MUTED`/`ACCENT`/`GOOD`)
|
||||
already exist; a proper light/dark toggle is a polish item.
|
||||
|
||||
**Explicitly out (recorded decisions):** renaming BCC (deferred until cross-client
|
||||
actually ships — do not raise before then).
|
||||
|
||||
---
|
||||
|
||||
## 4. Suggested next sprint (after v1.2.0 ships)
|
||||
|
||||
1. Merge + release v1.2.0 (decision above).
|
||||
2. **#7 MSIX detection** — highest-value untracked-runtime P1; land the testable core
|
||||
path-detection now, gate the runtime behavior behind a Windows verification.
|
||||
3. **Server search/filter (§3.1)** + **Test-all health column (§3.2)** — small, high
|
||||
daily-utility, and they compound with the log viewer/spawn-test already in 1.2.
|
||||
4. Design pass on **#10 catalog** and **#5 cross-client** (both need design before code).
|
||||
@@ -13,14 +13,17 @@ from __future__ import annotations
|
||||
import sys
|
||||
import time
|
||||
from pathlib import Path
|
||||
from typing import ClassVar
|
||||
|
||||
from PySide6.QtCore import QRect, QSettings, QSize, Qt, QThread, QTimer, QUrl, Signal
|
||||
from PySide6.QtGui import (
|
||||
QAction,
|
||||
QColor,
|
||||
QCursor,
|
||||
QDesktopServices,
|
||||
QGuiApplication,
|
||||
QIcon,
|
||||
QKeySequence,
|
||||
QPainter,
|
||||
QPixmap,
|
||||
)
|
||||
@@ -36,6 +39,7 @@ from PySide6.QtWidgets import (
|
||||
QGridLayout,
|
||||
QHBoxLayout,
|
||||
QHeaderView,
|
||||
QInputDialog,
|
||||
QLabel,
|
||||
QLineEdit,
|
||||
QListWidget,
|
||||
@@ -50,12 +54,17 @@ from PySide6.QtWidgets import (
|
||||
QStyledItemDelegate,
|
||||
QTableWidget,
|
||||
QTableWidgetItem,
|
||||
QToolTip,
|
||||
QVBoxLayout,
|
||||
QWidget,
|
||||
)
|
||||
|
||||
import bcc_core as core
|
||||
|
||||
# A full ~/.claude.json with history can be huge; parsing happens on the UI
|
||||
# thread during drag-and-drop import, so skip anything larger than this.
|
||||
MAX_DROP_IMPORT_BYTES = 5 * 1024 * 1024 # 5 MB
|
||||
|
||||
# --- One-line rebrand: change this to recolor the whole app --------------- #
|
||||
ACCENT = "#f97316" # warm orange
|
||||
ACCENT_DIM = "#c2570b"
|
||||
@@ -120,6 +129,7 @@ QScrollBar:vertical {{ background: transparent; width: 10px; margin: 2px; }}
|
||||
QScrollBar::handle:vertical {{ background: {BORDER}; border-radius: 5px; min-height: 24px; }}
|
||||
QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }}
|
||||
QLabel#statusbar {{ color: {MUTED}; padding: 4px 2px; }}
|
||||
QLabel#warnBanner {{ color: #1a1205; background: {WARN}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
|
||||
QLabel#section {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
|
||||
QLabel#sectionDisabled {{ color: {MUTED}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
|
||||
QLabel#placeholder {{ color: {MUTED}; padding: 12px; background: {PANEL_2}; border: 1px dashed {BORDER}; border-radius: 8px; }}
|
||||
@@ -172,10 +182,30 @@ class UpdateCheckWorker(QThread):
|
||||
running binary. Fails quiet — emits None on any network problem — so
|
||||
it's safe to fire unattended from a silent startup check as well as from
|
||||
the About dialog's "Check for updates" button.
|
||||
|
||||
Lifetime: the class keeps every instance alive in `_live` until its
|
||||
thread has finished. Without this, a caller whose own reference dies
|
||||
early (the About dialog is a temporary — closing it mid-check used to
|
||||
GC the dialog and the running QThread with it) crashes the process with
|
||||
"QThread: Destroyed while thread is still running". Callers may drop
|
||||
their reference at any time; signal connections to a destroyed receiver
|
||||
are disconnected by Qt, so a late result is simply discarded.
|
||||
"""
|
||||
|
||||
done = Signal(object) # dict | None
|
||||
|
||||
_live: ClassVar[set[UpdateCheckWorker]] = set()
|
||||
|
||||
def __init__(self):
|
||||
super().__init__()
|
||||
UpdateCheckWorker._live.add(self)
|
||||
self.finished.connect(self._release_keepalive)
|
||||
|
||||
def _release_keepalive(self):
|
||||
# Delivered on the main thread after run() has returned; only now is
|
||||
# it safe for the last reference to drop.
|
||||
UpdateCheckWorker._live.discard(self)
|
||||
|
||||
def run(self):
|
||||
self.done.emit(core.fetch_latest_release())
|
||||
|
||||
@@ -245,10 +275,31 @@ class KeyValueTable(QWidget):
|
||||
self.reveal_btn.setText("Hide secrets" if on else "Show secrets")
|
||||
self.table.viewport().update()
|
||||
|
||||
def _changed(self, *_):
|
||||
def _changed(self, item=None, *_):
|
||||
if item is not None and item.column() == 0:
|
||||
new_key = item.text().strip()
|
||||
row = item.row()
|
||||
if new_key and self._is_duplicate_key(new_key, row):
|
||||
prev_key = item.data(Qt.ItemDataRole.UserRole)
|
||||
prev_key = prev_key if prev_key is not None else ""
|
||||
self.table.blockSignals(True)
|
||||
item.setText(prev_key)
|
||||
self.table.blockSignals(False)
|
||||
QToolTip.showText(QCursor.pos(), f"Duplicate key '{new_key}' — reverted.")
|
||||
return
|
||||
item.setData(Qt.ItemDataRole.UserRole, new_key)
|
||||
if self._on_change:
|
||||
self._on_change()
|
||||
|
||||
def _is_duplicate_key(self, key: str, ignore_row: int) -> bool:
|
||||
for r in range(self.table.rowCount()):
|
||||
if r == ignore_row:
|
||||
continue
|
||||
other = self.table.item(r, 0)
|
||||
if other and other.text().strip() == key:
|
||||
return True
|
||||
return False
|
||||
|
||||
def _add_row(self):
|
||||
dlg = QDialog(self.window())
|
||||
dlg.setWindowTitle(f"Add {self._key_label}")
|
||||
@@ -275,6 +326,13 @@ class KeyValueTable(QWidget):
|
||||
grid.addWidget(val_edit, 1, 1)
|
||||
v.addLayout(grid)
|
||||
|
||||
# Shown when the typed key already exists in the table.
|
||||
dup_warn = QLabel("")
|
||||
dup_warn.setStyleSheet(f"color: {WARN};")
|
||||
dup_warn.setWordWrap(True)
|
||||
dup_warn.hide()
|
||||
v.addWidget(dup_warn)
|
||||
|
||||
# Type an API_KEY/TOKEN-style name and the value field masks itself.
|
||||
def _sync_echo(text):
|
||||
secret = core.is_secret_key(text)
|
||||
@@ -290,7 +348,15 @@ class KeyValueTable(QWidget):
|
||||
ok_btn = btns.button(QDialogButtonBox.StandardButton.Ok)
|
||||
ok_btn.setObjectName("primary")
|
||||
ok_btn.setEnabled(False)
|
||||
key_edit.textChanged.connect(lambda t: ok_btn.setEnabled(bool(t.strip())))
|
||||
|
||||
def _validate(text):
|
||||
k = text.strip()
|
||||
dup = bool(k) and self._is_duplicate_key(k, ignore_row=-1)
|
||||
ok_btn.setEnabled(bool(k) and not dup)
|
||||
dup_warn.setText(f"'{k}' already exists" if dup else "")
|
||||
dup_warn.setVisible(dup)
|
||||
|
||||
key_edit.textChanged.connect(_validate)
|
||||
btns.accepted.connect(dlg.accept)
|
||||
btns.rejected.connect(dlg.reject)
|
||||
v.addWidget(btns)
|
||||
@@ -304,13 +370,15 @@ class KeyValueTable(QWidget):
|
||||
return
|
||||
k = key_edit.text().strip()
|
||||
val = val_edit.text()
|
||||
if not k:
|
||||
if not k or self._is_duplicate_key(k, ignore_row=-1):
|
||||
return
|
||||
if self._before_change:
|
||||
self._before_change()
|
||||
r = self.table.rowCount()
|
||||
self.table.insertRow(r)
|
||||
self.table.setItem(r, 0, QTableWidgetItem(k))
|
||||
key_item = QTableWidgetItem(k)
|
||||
key_item.setData(Qt.ItemDataRole.UserRole, k)
|
||||
self.table.setItem(r, 0, key_item)
|
||||
self.table.setItem(r, 1, QTableWidgetItem(val))
|
||||
self._changed()
|
||||
|
||||
@@ -329,7 +397,9 @@ class KeyValueTable(QWidget):
|
||||
for k, v in (d or {}).items():
|
||||
r = self.table.rowCount()
|
||||
self.table.insertRow(r)
|
||||
self.table.setItem(r, 0, QTableWidgetItem(str(k)))
|
||||
key_item = QTableWidgetItem(str(k))
|
||||
key_item.setData(Qt.ItemDataRole.UserRole, str(k))
|
||||
self.table.setItem(r, 0, key_item)
|
||||
self.table.setItem(r, 1, QTableWidgetItem(str(v)))
|
||||
self.table.blockSignals(False)
|
||||
|
||||
@@ -1423,6 +1493,17 @@ class AboutDialog(QDialog):
|
||||
QDesktopServices.openUrl(QUrl(self._release_url or core.RELEASES_URL))
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# 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.
|
||||
# --------------------------------------------------------------------------- #
|
||||
class RestartWorker(QThread):
|
||||
done = Signal(object) # core.RestartResult
|
||||
|
||||
def run(self):
|
||||
self.done.emit(core.restart_claude_desktop())
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Main window
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -1460,6 +1541,15 @@ class MainWindow(QMainWindow):
|
||||
|
||||
root.addLayout(self._build_topbar())
|
||||
|
||||
# Persistent warning banner (MSIX-virtualized config, etc.). Lives in
|
||||
# its own widget because the status bar is rewritten on every action,
|
||||
# which used to wipe the warning before the user could read it.
|
||||
self.warn_banner = QLabel("")
|
||||
self.warn_banner.setObjectName("warnBanner")
|
||||
self.warn_banner.setWordWrap(True)
|
||||
self.warn_banner.hide()
|
||||
root.addWidget(self.warn_banner)
|
||||
|
||||
# User-draggable divider between the server list and the editor.
|
||||
split = QSplitter(Qt.Orientation.Horizontal)
|
||||
split.setChildrenCollapsible(False)
|
||||
@@ -1511,14 +1601,16 @@ class MainWindow(QMainWindow):
|
||||
last = float(st.value("update/lastCheck", 0.0, type=float) or 0.0)
|
||||
if (time.time() - last) < 86400: # at most once/day
|
||||
return
|
||||
st.setValue("update/lastCheck", time.time())
|
||||
self._startup_update_worker = UpdateCheckWorker()
|
||||
self._startup_update_worker.done.connect(self._on_startup_update_checked)
|
||||
self._startup_update_worker.start()
|
||||
|
||||
def _on_startup_update_checked(self, release: dict | None):
|
||||
self._startup_update_worker = None
|
||||
if release and core.is_newer_version(core.__version__, release["version"]):
|
||||
if release is None:
|
||||
return # offline/failed check: don't advance lastCheck, allow retry
|
||||
QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
|
||||
if core.is_newer_version(core.__version__, release["version"]):
|
||||
self.status.setText(
|
||||
f"Update available: {release['version']} · Help ▸ About to view it."
|
||||
)
|
||||
@@ -1596,11 +1688,45 @@ class MainWindow(QMainWindow):
|
||||
head.setObjectName("h1")
|
||||
v.addWidget(head)
|
||||
|
||||
search_row = QHBoxLayout()
|
||||
self.search_box = QLineEdit()
|
||||
self.search_box.setPlaceholderText("Search servers by name, command, or url…")
|
||||
self.search_box.setClearButtonEnabled(True)
|
||||
self.search_box.textChanged.connect(self._on_search_changed)
|
||||
v.addWidget(self.search_box)
|
||||
search_row.addWidget(self.search_box, 1)
|
||||
self.enable_all_btn = QPushButton("All on")
|
||||
self.enable_all_btn.setToolTip("Enable every server")
|
||||
self.enable_all_btn.clicked.connect(lambda: self._set_all_enabled(True))
|
||||
search_row.addWidget(self.enable_all_btn)
|
||||
self.disable_all_btn = QPushButton("All off")
|
||||
self.disable_all_btn.setToolTip("Disable every server")
|
||||
self.disable_all_btn.clicked.connect(lambda: self._set_all_enabled(False))
|
||||
search_row.addWidget(self.disable_all_btn)
|
||||
v.addLayout(search_row)
|
||||
|
||||
# Named server sets (issue #52): apply a saved Active/Disabled split
|
||||
# in one click. Sets live in the config file under _bccServerSets.
|
||||
sets_row = QHBoxLayout()
|
||||
sets_lbl = QLabel("Set")
|
||||
sets_lbl.setObjectName("muted")
|
||||
sets_row.addWidget(sets_lbl)
|
||||
self.sets_combo = QComboBox()
|
||||
self.sets_combo.setMinimumWidth(120)
|
||||
sets_row.addWidget(self.sets_combo, 1)
|
||||
self.apply_set_btn = QPushButton("Apply")
|
||||
self.apply_set_btn.setToolTip("Enable exactly this set's servers; disable the rest")
|
||||
self.apply_set_btn.clicked.connect(self._apply_selected_set)
|
||||
sets_row.addWidget(self.apply_set_btn)
|
||||
self.save_set_btn = QPushButton("Save set…")
|
||||
self.save_set_btn.setToolTip("Save the current Active/Disabled split as a named set")
|
||||
self.save_set_btn.clicked.connect(self._save_set)
|
||||
sets_row.addWidget(self.save_set_btn)
|
||||
self.del_set_btn = QPushButton("−")
|
||||
self.del_set_btn.setToolTip("Delete the selected set")
|
||||
self.del_set_btn.setMaximumWidth(32)
|
||||
self.del_set_btn.clicked.connect(self._delete_set)
|
||||
sets_row.addWidget(self.del_set_btn)
|
||||
v.addLayout(sets_row)
|
||||
|
||||
# Active and Disabled sections live in a vertical splitter so the user
|
||||
# can drag the divider instead of being stuck with a fixed-height
|
||||
@@ -1684,6 +1810,12 @@ class MainWindow(QMainWindow):
|
||||
undo_action.setShortcut("Ctrl+Z")
|
||||
undo_action.triggered.connect(self._undo)
|
||||
self.addAction(undo_action)
|
||||
# Ctrl+S / Cmd+S shortcut — routed through a guard so it respects
|
||||
# the same dirty/validation gating as the Save button.
|
||||
save_action = QAction(self)
|
||||
save_action.setShortcut(QKeySequence.StandardKey.Save)
|
||||
save_action.triggered.connect(self._save_shortcut)
|
||||
self.addAction(save_action)
|
||||
bar.addStretch()
|
||||
self.validation_lbl = QLabel("")
|
||||
bar.addWidget(self.validation_lbl)
|
||||
@@ -1710,6 +1842,19 @@ class MainWindow(QMainWindow):
|
||||
self._mark_dirty()
|
||||
self.status.setText("Undone.")
|
||||
|
||||
def _set_all_enabled(self, enabled: bool):
|
||||
"""Flip every server's enabled flag in one step (one undo snapshot)."""
|
||||
if not self.servers or all(s.enabled == enabled for s in self.servers):
|
||||
return # nothing to change
|
||||
cur = self._current_index()
|
||||
self._push_undo()
|
||||
for s in self.servers:
|
||||
s.enabled = enabled
|
||||
sel = cur if 0 <= cur < len(self.servers) else None
|
||||
self._refresh_tables(select_index=sel)
|
||||
self._mark_dirty()
|
||||
self.status.setText("All servers enabled." if enabled else "All servers disabled.")
|
||||
|
||||
# --- profiles -------------------------------------------------------- #
|
||||
def reload_profiles(self):
|
||||
discovered = core.discover_profiles()
|
||||
@@ -1732,8 +1877,9 @@ class MainWindow(QMainWindow):
|
||||
def _maybe_warn_msix(self):
|
||||
"""
|
||||
Windows-only, no-op everywhere else: if Claude Desktop looks like an
|
||||
MSIX/Store install with a virtualized config, append a warning to the
|
||||
status bar so edits to the plain %APPDATA% path aren't silently lost.
|
||||
MSIX/Store install with a virtualized config, show a persistent banner
|
||||
so edits to the plain %APPDATA% path aren't silently lost. (The status
|
||||
bar is the wrong home for this: it's rewritten on every action.)
|
||||
Defensive on purpose -- this must never block startup or profile load.
|
||||
"""
|
||||
try:
|
||||
@@ -1741,8 +1887,11 @@ class MainWindow(QMainWindow):
|
||||
except Exception:
|
||||
return
|
||||
if warning:
|
||||
self.status.setText(f"{self.status.text()} ⚠ {warning}")
|
||||
self.status.setToolTip(warning)
|
||||
self.warn_banner.setText(f"⚠ {warning}")
|
||||
self.warn_banner.setToolTip(warning)
|
||||
self.warn_banner.show()
|
||||
else:
|
||||
self.warn_banner.hide()
|
||||
|
||||
def add_custom_config(self):
|
||||
start = str(core.app_support_base())
|
||||
@@ -1798,6 +1947,7 @@ class MainWindow(QMainWindow):
|
||||
self._undo_stack.clear()
|
||||
self.undo_btn.setEnabled(False)
|
||||
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_tables(select_index=0 if self.servers else -1)
|
||||
self._update_status(saved=False)
|
||||
if repaired:
|
||||
@@ -1897,6 +2047,73 @@ class MainWindow(QMainWindow):
|
||||
cur = self._current_index()
|
||||
self._refresh_tables(select_index=cur if cur >= 0 else None)
|
||||
|
||||
# --- named server sets (issue #52) ------------------------------------ #
|
||||
def _refresh_sets_combo(self, select: str | None = None):
|
||||
sets = core.list_server_sets(self.full_config)
|
||||
self.sets_combo.blockSignals(True)
|
||||
self.sets_combo.clear()
|
||||
for name in sorted(sets):
|
||||
self.sets_combo.addItem(name)
|
||||
if select is not None:
|
||||
idx = self.sets_combo.findText(select)
|
||||
if idx >= 0:
|
||||
self.sets_combo.setCurrentIndex(idx)
|
||||
self.sets_combo.blockSignals(False)
|
||||
has_sets = bool(sets)
|
||||
self.apply_set_btn.setEnabled(has_sets)
|
||||
self.del_set_btn.setEnabled(has_sets)
|
||||
|
||||
def _apply_selected_set(self):
|
||||
name = self.sets_combo.currentText()
|
||||
sets = core.list_server_sets(self.full_config)
|
||||
if name not in sets:
|
||||
return
|
||||
self._push_undo()
|
||||
missing = core.apply_server_set(self.servers, sets[name])
|
||||
self._refresh_tables(select_index=self._current_index() if self.servers else None)
|
||||
self._mark_dirty()
|
||||
on = sum(1 for s in self.servers if s.enabled)
|
||||
msg = f"Applied set “{name}” · {on} enabled. Review and Save."
|
||||
if missing:
|
||||
msg += f" ⚠ no longer in this config: {', '.join(missing)}"
|
||||
self.status.setText(msg)
|
||||
|
||||
def _save_set(self):
|
||||
name, ok = QInputDialog.getText(
|
||||
self,
|
||||
"Save server set",
|
||||
"Set name (saves which servers are currently Active):",
|
||||
text=self.sets_combo.currentText(),
|
||||
)
|
||||
name = name.strip()
|
||||
if not ok or not name:
|
||||
return
|
||||
if name in core.list_server_sets(self.full_config) and (
|
||||
QMessageBox.question(self, "Set exists", f"Replace set “{name}”?")
|
||||
!= QMessageBox.StandardButton.Yes
|
||||
):
|
||||
return
|
||||
members = core.save_server_set(self.full_config, name, self.servers)
|
||||
self._refresh_sets_combo(select=name)
|
||||
self._mark_dirty() # the set is written on the next Save
|
||||
self.status.setText(
|
||||
f"Set “{name}” saved ({len(members)} server(s)). Press Save to write it."
|
||||
)
|
||||
|
||||
def _delete_set(self):
|
||||
name = self.sets_combo.currentText()
|
||||
if not name:
|
||||
return
|
||||
if (
|
||||
QMessageBox.question(self, "Delete set", f"Delete set “{name}”?")
|
||||
!= QMessageBox.StandardButton.Yes
|
||||
):
|
||||
return
|
||||
if core.delete_server_set(self.full_config, name):
|
||||
self._refresh_sets_combo()
|
||||
self._mark_dirty()
|
||||
self.status.setText(f"Set “{name}” deleted. Press Save to write the change.")
|
||||
|
||||
# --- test all (spawn-test every enabled local server) ---------------- #
|
||||
def _test_all_servers(self):
|
||||
targets = [s for s in self.servers if s.enabled and s.kind == "stdio"]
|
||||
@@ -2045,12 +2262,17 @@ class MainWindow(QMainWindow):
|
||||
if not (0 <= idx < len(self.servers)):
|
||||
return
|
||||
entry = self.servers[idx]
|
||||
old_name = entry.name
|
||||
entry.name = self.editor.current_name()
|
||||
entry.data = self.editor.dump_data()
|
||||
# The server stays in its section (enable state unchanged), so update
|
||||
# its existing row in place rather than re-rendering.
|
||||
# An edit invalidates any cached "Test all" result -- the server that
|
||||
# was spawn-tested no longer matches what's on disk once saved.
|
||||
# was spawn-tested no longer matches what's on disk once saved. Pop
|
||||
# both names: the old one (so a rename doesn't leave a stale entry
|
||||
# for whoever takes that name next) and the new one (so we don't
|
||||
# inherit a stale result cached under the name being renamed to).
|
||||
self._health.pop(old_name, None)
|
||||
self._health.pop(entry.name, None)
|
||||
loc = self._row_of_index.get(idx)
|
||||
if loc:
|
||||
@@ -2210,11 +2432,23 @@ class MainWindow(QMainWindow):
|
||||
self.validation_lbl.setStyleSheet(f"color: {WARN};")
|
||||
self.save_btn.setEnabled(False)
|
||||
return False
|
||||
self.validation_lbl.setText("✓ valid")
|
||||
self.validation_lbl.setStyleSheet(f"color: {GOOD};")
|
||||
lint_warnings = core.lint_servers(self.servers)
|
||||
if lint_warnings:
|
||||
self.validation_lbl.setText(f"⚠ {lint_warnings[0]}")
|
||||
self.validation_lbl.setStyleSheet(f"color: {WARN};")
|
||||
else:
|
||||
self.validation_lbl.setText("✓ valid")
|
||||
self.validation_lbl.setStyleSheet(f"color: {GOOD};")
|
||||
self.save_btn.setEnabled(self.dirty)
|
||||
return True
|
||||
|
||||
def _save_shortcut(self):
|
||||
"""Ctrl+S / Cmd+S handler — only fires when the Save button itself
|
||||
would accept a click, so the shortcut can't bypass validation/dirty
|
||||
gating."""
|
||||
if self.save_btn.isEnabled():
|
||||
self.save()
|
||||
|
||||
def save(self):
|
||||
if not self.current_profile:
|
||||
return
|
||||
@@ -2284,18 +2518,30 @@ class MainWindow(QMainWindow):
|
||||
def _offer_restart_button(self):
|
||||
"""Show the 'Restart Claude Desktop' button after a successful save,
|
||||
but only when the just-saved profile is Claude Desktop -- restarting
|
||||
makes no sense for Claude Code, which has no GUI process to bounce."""
|
||||
if self.current_profile and core.profile_targets_claude_desktop(self.current_profile):
|
||||
makes no sense for Claude Code, which has no GUI process to bounce --
|
||||
and only on platforms where Claude Desktop exists (never Linux, where
|
||||
'claude' is the Claude Code CLI)."""
|
||||
if (
|
||||
self.current_profile
|
||||
and core.profile_targets_claude_desktop(self.current_profile)
|
||||
and core.restart_supported()
|
||||
):
|
||||
self.restart_btn.show()
|
||||
else:
|
||||
self.restart_btn.hide()
|
||||
|
||||
def _restart_claude_desktop(self):
|
||||
self.restart_btn.setEnabled(False)
|
||||
try:
|
||||
result = core.restart_claude_desktop()
|
||||
finally:
|
||||
self.restart_btn.setEnabled(True)
|
||||
self.restart_btn.setText("Restarting…")
|
||||
# Held on self (MainWindow outlives the worker); replaced only after
|
||||
# done re-enables the button, so a running thread is never dropped.
|
||||
self._restart_worker = RestartWorker()
|
||||
self._restart_worker.done.connect(self._on_restart_done)
|
||||
self._restart_worker.start()
|
||||
|
||||
def _on_restart_done(self, result):
|
||||
self.restart_btn.setEnabled(True)
|
||||
self.restart_btn.setText("Restart Claude Desktop")
|
||||
self.restart_btn.hide()
|
||||
if result.success:
|
||||
self.status.setText(f"{self.status.text()} · {result.detail}")
|
||||
@@ -2356,29 +2602,44 @@ class MainWindow(QMainWindow):
|
||||
e.acceptProposedAction()
|
||||
|
||||
def dropEvent(self, e):
|
||||
total_added, total_replaced, files_imported = 0, 0, 0
|
||||
undo_pushed = False
|
||||
for u in e.mimeData().urls():
|
||||
path = u.toLocalFile()
|
||||
if not path.endswith(".json"):
|
||||
continue
|
||||
text = Path(path).read_text(encoding="utf-8")
|
||||
p = Path(path)
|
||||
if p.stat().st_size > MAX_DROP_IMPORT_BYTES:
|
||||
QMessageBox.warning(
|
||||
self,
|
||||
"File too large",
|
||||
f"{p.name}:\nFile exceeds the 5 MB import limit and was skipped.",
|
||||
)
|
||||
continue
|
||||
text = p.read_text(encoding="utf-8")
|
||||
try:
|
||||
servers = core.parse_pasted_json(text)
|
||||
except Exception as ex:
|
||||
QMessageBox.warning(self, "Couldn't import", f"{Path(path).name}:\n{ex}")
|
||||
QMessageBox.warning(self, "Couldn't import", f"{p.name}:\n{ex}")
|
||||
continue
|
||||
self._push_undo()
|
||||
if not undo_pushed:
|
||||
self._push_undo()
|
||||
undo_pushed = True
|
||||
added, replaced = 0, 0
|
||||
for name, data in servers.items():
|
||||
a, r = self._import_server(name, data)
|
||||
added += int(a)
|
||||
replaced += int(r)
|
||||
total_added += added
|
||||
total_replaced += replaced
|
||||
files_imported += 1
|
||||
if files_imported:
|
||||
self._refresh_tables(select_index=len(self.servers) - 1)
|
||||
self._mark_dirty()
|
||||
self.status.setText(
|
||||
f"Imported {added} added, {replaced} replaced from {Path(path).name}. "
|
||||
"Review and Save."
|
||||
f"Imported {total_added} added, {total_replaced} replaced from "
|
||||
f"{files_imported} file(s). Review and Save."
|
||||
)
|
||||
break
|
||||
|
||||
def closeEvent(self, e):
|
||||
if self.dirty and not self._confirm_discard():
|
||||
|
||||
@@ -78,8 +78,8 @@ if sys.platform == "darwin":
|
||||
info_plist={
|
||||
"CFBundleName": "Better Claude Config",
|
||||
"CFBundleDisplayName": "Better Claude Config",
|
||||
"CFBundleShortVersionString": "1.0.0",
|
||||
"CFBundleVersion": "1.0.0",
|
||||
"CFBundleShortVersionString": "1.3.0",
|
||||
"CFBundleVersion": "1.3.0",
|
||||
"NSHighResolutionCapable": True,
|
||||
"NSRequiresAquaSystemAppearance": False, # supports dark mode
|
||||
"LSMinimumSystemVersion": "11.0",
|
||||
|
||||
+243
-22
@@ -39,6 +39,12 @@ CONFIG_FILENAME = "claude_desktop_config.json"
|
||||
# we can toggle it back on without losing the definition.
|
||||
DISABLED_KEY = "_disabledMcpServers"
|
||||
|
||||
# Named server sets: {set_name: [enabled server names]}. Same pattern as
|
||||
# DISABLED_KEY — a bcc-owned key Claude ignores, stored in the config file so
|
||||
# sets travel with it. Applying a set enables exactly the listed servers and
|
||||
# parks the rest under DISABLED_KEY.
|
||||
SETS_KEY = "_bccServerSets"
|
||||
|
||||
BACKUP_DIRNAME = ".bcc_backups"
|
||||
MAX_BACKUPS = 15
|
||||
|
||||
@@ -59,7 +65,7 @@ KNOWN_FIELDS = {"command", "args", "env", "url", "type", "headers"}
|
||||
# binary. All network I/O here is fail-quiet (returns None on any problem)
|
||||
# so it's safe to run unattended, off the UI thread, at startup.
|
||||
# --------------------------------------------------------------------------- #
|
||||
__version__ = "1.2.0"
|
||||
__version__ = "1.3.0"
|
||||
|
||||
REPO_URL = "https://git.avezzano.io/the_og/better-claude-config"
|
||||
ISSUES_URL = f"{REPO_URL}/issues"
|
||||
@@ -282,10 +288,43 @@ def msix_warning_text(
|
||||
)
|
||||
|
||||
|
||||
def discover_project_configs(claude_json_path: str | os.PathLike) -> list[Profile]:
|
||||
"""
|
||||
Find project-scope `.mcp.json` configs known to Claude Code.
|
||||
|
||||
`~/.claude.json` keeps a `projects` map keyed by absolute project
|
||||
directory path (that's what the CLI writes as it's used in each repo).
|
||||
Any project whose directory has a `.mcp.json` file next to it -- a
|
||||
standalone file with a top-level `mcpServers` object, same shape BCC
|
||||
already edits -- is surfaced here as its own profile so it can be opened
|
||||
via 'Add config...' without hunting for the path by hand.
|
||||
|
||||
Fails quiet: a missing/unreadable/malformed `claude_json_path`, or a
|
||||
`projects` value that isn't a dict, just yields an empty list rather than
|
||||
raising -- this is best-effort discovery, not a required config load.
|
||||
"""
|
||||
try:
|
||||
cfg = load_config(claude_json_path)
|
||||
except Exception:
|
||||
return []
|
||||
projects = cfg.get("projects")
|
||||
if not isinstance(projects, dict):
|
||||
return []
|
||||
out: list[Profile] = []
|
||||
for key in sorted(k for k in projects if isinstance(k, str)):
|
||||
mcp_path = Path(key) / ".mcp.json"
|
||||
if mcp_path.is_file():
|
||||
out.append(
|
||||
Profile(label=f"Project: {Path(key).name}", path=mcp_path, config_exists=True)
|
||||
)
|
||||
return out
|
||||
|
||||
|
||||
def discover_profiles() -> list[Profile]:
|
||||
"""
|
||||
Find every `Claude*` data directory in the platform's app-support base
|
||||
(Claude Desktop installs), then also check for a Claude Code global config.
|
||||
(Claude Desktop installs), then also check for a Claude Code global config
|
||||
and any project-scope `.mcp.json` configs it knows about.
|
||||
|
||||
Claude Desktop: scans the platform app-support folder for any `Claude*`
|
||||
directory (catches `Claude`, `Claude-Work`, etc.).
|
||||
@@ -298,6 +337,10 @@ def discover_profiles() -> list[Profile]:
|
||||
`claude mcp add` writes; project scope is a per-repo .mcp.json, which can
|
||||
be opened via 'Add config…'). NOT ~/.claude/settings.json — that file is
|
||||
for permissions/hooks and rejects an mcpServers key with a schema error.
|
||||
Project scope: ~/.claude.json also tracks a `projects` map, one entry per
|
||||
directory Claude Code has been run in; any of those with a `.mcp.json`
|
||||
file are surfaced as their own profiles too (see
|
||||
`discover_project_configs`).
|
||||
"""
|
||||
base = app_support_base()
|
||||
out: list[Profile] = []
|
||||
@@ -319,6 +362,12 @@ def discover_profiles() -> list[Profile]:
|
||||
cc_cfg = home / ".claude.json"
|
||||
out.append(Profile(label="Claude Code", path=cc_cfg, config_exists=cc_cfg.is_file()))
|
||||
|
||||
existing_paths = {str(p.path) for p in out}
|
||||
for proj in discover_project_configs(cc_cfg):
|
||||
if str(proj.path) not in existing_paths:
|
||||
existing_paths.add(str(proj.path))
|
||||
out.append(proj)
|
||||
|
||||
# Legacy: earlier BCC versions (and hand-edits) may have parked servers in
|
||||
# ~/.claude/settings.json, where Claude Code ignores them. Surface that
|
||||
# file only when it actually contains an mcpServers block, so the user can
|
||||
@@ -409,6 +458,70 @@ def extract_servers(cfg: dict) -> list[ServerEntry]:
|
||||
return out
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Named server sets (issue #52)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def list_server_sets(cfg: dict) -> dict[str, list[str]]:
|
||||
"""
|
||||
Return {set_name: [enabled server names]} from cfg's SETS_KEY.
|
||||
|
||||
Fail-soft: entries whose value isn't a list of strings (hand-edited or
|
||||
corrupted) are skipped rather than raising, so one bad set never hides
|
||||
the rest.
|
||||
"""
|
||||
raw = cfg.get(SETS_KEY)
|
||||
if not isinstance(raw, dict):
|
||||
return {}
|
||||
out: dict[str, list[str]] = {}
|
||||
for name, members in raw.items():
|
||||
if isinstance(members, list) and all(isinstance(m, str) for m in members):
|
||||
out[str(name)] = list(members)
|
||||
return out
|
||||
|
||||
|
||||
def save_server_set(cfg: dict, name: str, servers: list[ServerEntry]) -> list[str]:
|
||||
"""
|
||||
Snapshot the current enabled-server names into cfg under SETS_KEY as
|
||||
`name` (overwriting an existing set of that name). Returns the saved
|
||||
member list. The caller decides when cfg reaches disk (normal Save flow).
|
||||
"""
|
||||
members = [s.name for s in servers if s.enabled]
|
||||
sets = cfg.get(SETS_KEY)
|
||||
if not isinstance(sets, dict):
|
||||
sets = {}
|
||||
cfg[SETS_KEY] = sets
|
||||
sets[name] = members
|
||||
return members
|
||||
|
||||
|
||||
def delete_server_set(cfg: dict, name: str) -> bool:
|
||||
"""Remove set `name` from cfg. Drops SETS_KEY entirely when the last set
|
||||
goes, so untouched configs don't grow an empty bcc key. Returns True if
|
||||
something was deleted."""
|
||||
sets = cfg.get(SETS_KEY)
|
||||
if not isinstance(sets, dict) or name not in sets:
|
||||
return False
|
||||
del sets[name]
|
||||
if not sets:
|
||||
cfg.pop(SETS_KEY, None)
|
||||
return True
|
||||
|
||||
|
||||
def apply_server_set(servers: list[ServerEntry], enabled_names: list[str]) -> list[str]:
|
||||
"""
|
||||
Enable exactly the servers named in `enabled_names`; disable every other
|
||||
entry (in place). Returns the set members that no longer exist in
|
||||
`servers` — the caller surfaces those as a warning, and the rest of the
|
||||
set still applies.
|
||||
"""
|
||||
wanted = set(enabled_names)
|
||||
present: set[str] = set()
|
||||
for s in servers:
|
||||
s.enabled = s.name in wanted
|
||||
present.add(s.name)
|
||||
return sorted(wanted - present)
|
||||
|
||||
|
||||
def resolve_name_collision(name: str, existing: set[str]) -> str:
|
||||
"""
|
||||
Return a name guaranteed not to collide with `existing`.
|
||||
@@ -1213,6 +1326,63 @@ def validate_servers(servers: list[ServerEntry]) -> list[str]:
|
||||
return problems
|
||||
|
||||
|
||||
def lint_server(name: str, data: dict) -> list[str]:
|
||||
"""Return non-blocking structural warnings for a single server definition.
|
||||
|
||||
Unlike validate_servers, nothing here blocks Save -- these are advisory
|
||||
notes about shapes that will round-trip through JSON fine but are
|
||||
probably not what the user intended (args given as a plain string
|
||||
instead of a list, an env value that isn't a string, an unrecognized
|
||||
`type`, unknown top-level fields, etc.).
|
||||
"""
|
||||
nm = name.strip() or "(unnamed)"
|
||||
warnings: list[str] = []
|
||||
|
||||
if "command" in data and not isinstance(data["command"], str):
|
||||
warnings.append(f"'{nm}': 'command' should be a string")
|
||||
|
||||
if "args" in data:
|
||||
args = data["args"]
|
||||
if not isinstance(args, list):
|
||||
warnings.append(f"'{nm}': 'args' should be a list (one argument per item)")
|
||||
elif any(not isinstance(a, str) for a in args):
|
||||
warnings.append(
|
||||
f"'{nm}': 'args' contains non-string values "
|
||||
"(they will be saved as-is; Claude expects strings)"
|
||||
)
|
||||
|
||||
for field in ("env", "headers"):
|
||||
if field not in data:
|
||||
continue
|
||||
val = data[field]
|
||||
if not isinstance(val, dict):
|
||||
warnings.append(f"'{nm}': '{field}' should be an object of string key/value pairs")
|
||||
elif any(not isinstance(v, str) for v in val.values()):
|
||||
warnings.append(
|
||||
f"'{nm}': '{field}' contains non-string values "
|
||||
"(they will be saved as-is; Claude expects strings)"
|
||||
)
|
||||
|
||||
if "type" in data:
|
||||
t = data["type"]
|
||||
if t not in ("http", "sse", "stdio"):
|
||||
warnings.append(f"'{nm}': 'type' should be one of http, sse, stdio (found {t!r})")
|
||||
|
||||
extra = sorted(k for k in data if k not in KNOWN_FIELDS)
|
||||
if extra:
|
||||
warnings.append(f"'{nm}': extra fields preserved as-is: {', '.join(extra)}")
|
||||
|
||||
return warnings
|
||||
|
||||
|
||||
def lint_servers(servers: list[ServerEntry]) -> list[str]:
|
||||
"""Concatenate lint_server warnings across every entry, in order."""
|
||||
out: list[str] = []
|
||||
for s in servers:
|
||||
out.extend(lint_server(s.name, s.data))
|
||||
return out
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Search / filter
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -1581,6 +1751,23 @@ def server_log_path(name: str) -> Path | None:
|
||||
_STDERR_CAP = 4096 # bytes
|
||||
|
||||
|
||||
def _kill_process_tree_windows(pid: int) -> None:
|
||||
"""
|
||||
Kill `pid` and its whole descendant tree via `taskkill /T /F` (issue #13).
|
||||
|
||||
Popen.kill() only terminates the direct child; runner-style commands
|
||||
(npx → node → server, cmd → real process) leave the actual server alive,
|
||||
leaking a process on every Windows spawn test. taskkill walks the tree.
|
||||
"""
|
||||
flags = getattr(subprocess, "CREATE_NO_WINDOW", 0) # no console flash from the GUI exe
|
||||
with contextlib.suppress(OSError):
|
||||
subprocess.run(
|
||||
["taskkill", "/PID", str(pid), "/T", "/F"],
|
||||
capture_output=True,
|
||||
creationflags=flags,
|
||||
)
|
||||
|
||||
|
||||
def spawn_test(data: dict, timeout: float = 3.0) -> dict:
|
||||
"""
|
||||
Attempt to start a stdio server and observe it for `timeout` seconds.
|
||||
@@ -1675,6 +1862,10 @@ def _spawn_test_impl(data: dict, timeout: float) -> dict:
|
||||
)
|
||||
if os.name != "nt":
|
||||
popen_kwargs["start_new_session"] = True # own process group → clean kill
|
||||
else:
|
||||
# The packaged app is windowed (console=False); without this every
|
||||
# spawn test of a console server flashes a console window.
|
||||
popen_kwargs["creationflags"] = getattr(subprocess, "CREATE_NO_WINDOW", 0)
|
||||
|
||||
try:
|
||||
proc = subprocess.Popen(args_list, **popen_kwargs)
|
||||
@@ -1696,7 +1887,7 @@ def _spawn_test_impl(data: dict, timeout: float) -> dict:
|
||||
if os.name != "nt":
|
||||
os.killpg(os.getpgid(proc.pid), _signal.SIGKILL)
|
||||
else:
|
||||
proc.kill() # best-effort on Windows
|
||||
_kill_process_tree_windows(proc.pid)
|
||||
except OSError:
|
||||
pass
|
||||
with contextlib.suppress(subprocess.TimeoutExpired):
|
||||
@@ -1857,8 +2048,41 @@ def _run_quiet(cmd: list[str]) -> None:
|
||||
subprocess.run(cmd, capture_output=True)
|
||||
|
||||
|
||||
def restart_supported() -> bool:
|
||||
"""
|
||||
True only where restarting Claude Desktop makes sense (macOS, Windows).
|
||||
There is no official Claude Desktop for Linux, and the obvious binary
|
||||
name there ("claude") is the Claude Code CLI — killing or spawning it
|
||||
would be actively harmful. GUI callers gate the Restart button on this.
|
||||
"""
|
||||
return sys.platform == "darwin" or sys.platform.startswith("win")
|
||||
|
||||
|
||||
_MACOS_QUIT_WAIT_S = 5.0
|
||||
|
||||
|
||||
def _macos_claude_running() -> bool:
|
||||
try:
|
||||
return subprocess.run(["pgrep", "-x", "Claude"], capture_output=True).returncode == 0
|
||||
except OSError:
|
||||
return False
|
||||
|
||||
|
||||
def _restart_claude_desktop_macos() -> RestartResult:
|
||||
_run_quiet(["pkill", "-x", "Claude"])
|
||||
# Wait for the old instance to actually exit: `open -a` against a dying
|
||||
# process can merely re-activate it, and the config is only re-read on a
|
||||
# true relaunch. Blocks up to _MACOS_QUIT_WAIT_S — callers run this off
|
||||
# the UI thread (see RestartWorker in bcc.py).
|
||||
deadline = time.monotonic() + _MACOS_QUIT_WAIT_S
|
||||
while _macos_claude_running():
|
||||
if time.monotonic() > deadline:
|
||||
return RestartResult(
|
||||
False,
|
||||
f"Claude Desktop didn't quit within {_MACOS_QUIT_WAIT_S:.0f}s — "
|
||||
"quit it manually, then reopen it.",
|
||||
)
|
||||
time.sleep(0.15)
|
||||
try:
|
||||
result = subprocess.run(["open", "-a", "Claude"], capture_output=True, text=True)
|
||||
except OSError as e:
|
||||
@@ -1875,8 +2099,18 @@ def _claude_windows_start_menu_shortcut() -> Path:
|
||||
|
||||
|
||||
def _restart_claude_desktop_windows() -> RestartResult:
|
||||
_run_quiet(["taskkill", "/IM", "Claude.exe", "/F"])
|
||||
shortcut = _claude_windows_start_menu_shortcut()
|
||||
if not shortcut.is_file():
|
||||
# Checked BEFORE killing: an MSIX/Store install has no Start-menu .lnk
|
||||
# at this path, and killing without a relaunch path would leave the
|
||||
# user with no running Claude at all.
|
||||
return RestartResult(
|
||||
False,
|
||||
f"Claude's Start-menu shortcut wasn't found ({shortcut}). "
|
||||
"If Claude Desktop is installed from the Microsoft Store, "
|
||||
"quit and reopen it manually.",
|
||||
)
|
||||
_run_quiet(["taskkill", "/IM", "Claude.exe", "/F"])
|
||||
try:
|
||||
# `cmd /c start "" <target>` launches detached, the same as double-clicking
|
||||
# the Start-menu shortcut, and returns immediately.
|
||||
@@ -1893,32 +2127,19 @@ def _restart_claude_desktop_windows() -> RestartResult:
|
||||
return RestartResult(True, "Claude Desktop restarted.")
|
||||
|
||||
|
||||
def _restart_claude_desktop_linux() -> RestartResult:
|
||||
_run_quiet(["pkill", "claude"])
|
||||
try:
|
||||
# The Linux launcher is the app itself (no "open"-style helper), so it
|
||||
# has to be started detached rather than waited on.
|
||||
subprocess.Popen(
|
||||
["claude"],
|
||||
stdin=subprocess.DEVNULL,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
start_new_session=True,
|
||||
)
|
||||
except OSError as e:
|
||||
return RestartResult(False, f"Couldn't launch Claude Desktop: {e}")
|
||||
return RestartResult(True, "Claude Desktop restarted.")
|
||||
|
||||
|
||||
def restart_claude_desktop() -> RestartResult:
|
||||
"""
|
||||
Kill and relaunch the Claude Desktop app so a freshly saved config takes
|
||||
effect. The app not currently running is NOT a failure -- pkill/taskkill
|
||||
exiting non-zero just means "nothing to kill", and we go straight to
|
||||
relaunching. Only a failed relaunch is reported as success=False.
|
||||
|
||||
macOS blocks for up to _MACOS_QUIT_WAIT_S while the old instance exits —
|
||||
run off the UI thread. Unsupported platforms (see restart_supported())
|
||||
refuse without touching any process.
|
||||
"""
|
||||
if sys.platform == "darwin":
|
||||
return _restart_claude_desktop_macos()
|
||||
if sys.platform.startswith("win"):
|
||||
return _restart_claude_desktop_windows()
|
||||
return _restart_claude_desktop_linux()
|
||||
return RestartResult(False, "Restarting Claude Desktop isn't supported on this platform.")
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
[project]
|
||||
name = "better-claude-config"
|
||||
version = "1.2.0"
|
||||
version = "1.3.0"
|
||||
description = "Cross-platform GUI for editing the mcpServers block of Claude Desktop and Claude Code configs"
|
||||
readme = "README.md"
|
||||
license = { file = "LICENSE" }
|
||||
|
||||
+335
-44
@@ -3,9 +3,11 @@ proper test functions with tmp_path/monkeypatch fixtures)."""
|
||||
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -65,6 +67,64 @@ def test_legacy_settings_json_surfaced_only_with_servers(fake_home):
|
||||
assert any("legacy" in p.label for p in c.discover_profiles())
|
||||
|
||||
|
||||
def test_project_with_mcp_json_is_discovered(tmp_path):
|
||||
proj = tmp_path / "my-project"
|
||||
proj.mkdir()
|
||||
(proj / ".mcp.json").write_text('{"mcpServers": {"x": {"command": "npx"}}}')
|
||||
claude_json = tmp_path / ".claude.json"
|
||||
claude_json.write_text(json.dumps({"projects": {str(proj): {}}}))
|
||||
|
||||
profs = c.discover_project_configs(claude_json)
|
||||
assert len(profs) == 1
|
||||
assert profs[0].label == f"Project: {proj.name}"
|
||||
assert profs[0].path == proj / ".mcp.json"
|
||||
assert profs[0].config_exists
|
||||
|
||||
|
||||
def test_project_without_mcp_json_not_listed(tmp_path):
|
||||
proj = tmp_path / "no-mcp-project"
|
||||
proj.mkdir()
|
||||
claude_json = tmp_path / ".claude.json"
|
||||
claude_json.write_text(json.dumps({"projects": {str(proj): {}}}))
|
||||
|
||||
assert c.discover_project_configs(claude_json) == []
|
||||
|
||||
|
||||
def test_projects_missing_or_not_dict_yields_no_profiles(tmp_path):
|
||||
claude_json = tmp_path / ".claude.json"
|
||||
|
||||
claude_json.write_text(json.dumps({}))
|
||||
assert c.discover_project_configs(claude_json) == []
|
||||
|
||||
claude_json.write_text(json.dumps({"projects": ["not", "a", "dict"]}))
|
||||
assert c.discover_project_configs(claude_json) == []
|
||||
|
||||
|
||||
def test_malformed_claude_json_fails_quiet(fake_home):
|
||||
(fake_home / ".claude.json").write_text("{not valid json")
|
||||
assert c.discover_project_configs(fake_home / ".claude.json") == []
|
||||
# discover_profiles as a whole must still work and just skip project profiles
|
||||
profs = c.discover_profiles()
|
||||
assert not any(p.label.startswith("Project:") for p in profs)
|
||||
|
||||
|
||||
def test_project_configs_deduped_against_existing_profiles(fake_home):
|
||||
# Point a project directly at the Claude Code profile's own .mcp.json-shaped
|
||||
# path to prove discover_profiles() won't duplicate an already-listed path.
|
||||
proj = fake_home / "dup-project"
|
||||
proj.mkdir()
|
||||
mcp_path = proj / ".mcp.json"
|
||||
mcp_path.write_text('{"mcpServers": {"x": {"command": "npx"}}}')
|
||||
(fake_home / ".claude.json").write_text(json.dumps({"projects": {str(proj): {}}}))
|
||||
|
||||
direct = c.discover_project_configs(fake_home / ".claude.json")
|
||||
assert len(direct) == 1
|
||||
|
||||
profs = c.discover_profiles()
|
||||
project_profiles = [p for p in profs if str(p.path) == str(mcp_path)]
|
||||
assert len(project_profiles) == 1
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# 2. Write pipeline: preserves other keys + order, only touches mcpServers
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -179,6 +239,85 @@ def test_valid_set_passes_clean():
|
||||
assert c.validate_servers([c.ServerEntry("good", {"command": "node"}, True)]) == []
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# 4b. Lint: non-blocking structural warnings
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_lint_flags_non_string_command():
|
||||
warnings = c.lint_server("x", {"command": 5})
|
||||
assert any("'command' should be a string" in w for w in warnings)
|
||||
|
||||
|
||||
def test_lint_flags_args_not_a_list():
|
||||
warnings = c.lint_server("x", {"command": "node", "args": "-y foo"})
|
||||
assert any("'args' should be a list" in w for w in warnings)
|
||||
|
||||
|
||||
def test_lint_flags_args_with_non_string_items():
|
||||
warnings = c.lint_server("x", {"command": "node", "args": ["-y", 5]})
|
||||
assert any("'args' contains non-string values" in w for w in warnings)
|
||||
|
||||
|
||||
def test_lint_flags_env_not_a_dict():
|
||||
warnings = c.lint_server("x", {"command": "node", "env": ["FOO=bar"]})
|
||||
assert any("'env' should be an object" in w for w in warnings)
|
||||
|
||||
|
||||
def test_lint_flags_env_with_non_string_values():
|
||||
warnings = c.lint_server("x", {"command": "node", "env": {"FOO": 5}})
|
||||
assert any("'env' contains non-string values" in w for w in warnings)
|
||||
|
||||
|
||||
def test_lint_flags_headers_not_a_dict():
|
||||
warnings = c.lint_server("x", {"url": "https://x", "headers": ["Authorization: x"]})
|
||||
assert any("'headers' should be an object" in w for w in warnings)
|
||||
|
||||
|
||||
def test_lint_flags_headers_with_non_string_values():
|
||||
warnings = c.lint_server("x", {"url": "https://x", "headers": {"Authorization": 5}})
|
||||
assert any("'headers' contains non-string values" in w for w in warnings)
|
||||
|
||||
|
||||
def test_lint_flags_bad_type_value():
|
||||
warnings = c.lint_server("x", {"url": "https://x", "type": "websocket"})
|
||||
assert any("'type'" in w and "websocket" in w for w in warnings)
|
||||
|
||||
|
||||
def test_lint_flags_extra_unknown_fields():
|
||||
warnings = c.lint_server("x", {"command": "node", "cwd": "/tmp", "timeout": 30})
|
||||
matches = [w for w in warnings if "extra fields preserved as-is" in w]
|
||||
assert len(matches) == 1
|
||||
assert "cwd" in matches[0]
|
||||
assert "timeout" in matches[0]
|
||||
|
||||
|
||||
def test_lint_clean_server_yields_no_warnings():
|
||||
assert c.lint_server("good", {"command": "node", "args": ["a.js"]}) == []
|
||||
assert (
|
||||
c.lint_server(
|
||||
"remote", {"url": "https://x", "type": "http", "headers": {"Authorization": "x"}}
|
||||
)
|
||||
== []
|
||||
)
|
||||
|
||||
|
||||
def test_lint_servers_aggregates_across_entries():
|
||||
entries = [
|
||||
c.ServerEntry("a", {"command": 5}, True),
|
||||
c.ServerEntry("b", {"url": "https://x", "type": "bogus"}, True),
|
||||
]
|
||||
warnings = c.lint_servers(entries)
|
||||
assert any("'a'" in w and "'command' should be a string" in w for w in warnings)
|
||||
assert any("'b'" in w and "'type'" in w for w in warnings)
|
||||
|
||||
|
||||
def test_lint_warning_is_not_a_blocking_problem():
|
||||
# args given as a single string isn't caught by validate_servers (a
|
||||
# command is still present), but it's a lint warning.
|
||||
entry = c.ServerEntry("x", {"command": "node", "args": "-y foo"}, True)
|
||||
assert c.validate_servers([entry]) == []
|
||||
assert c.lint_servers([entry]) != []
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# 5. Secrets: detection + redaction
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -227,8 +366,11 @@ def test_diagnostics_redacts_token_args():
|
||||
# --------------------------------------------------------------------------- #
|
||||
# 6. Dependency / PATH checking
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_dep_check_finds_python3():
|
||||
assert c.check_dependency({"command": "python3"})["status"] == "ok"
|
||||
def test_dep_check_finds_python():
|
||||
# "python3" does not exist on a stock Windows install; "warn" (found only
|
||||
# on an augmented PATH) still proves resolution works on a real machine.
|
||||
cmd = "python" if os.name == "nt" else "python3"
|
||||
assert c.check_dependency({"command": cmd})["status"] in ("ok", "warn")
|
||||
|
||||
|
||||
def test_dep_check_flags_missing():
|
||||
@@ -326,6 +468,55 @@ def test_spawn_test_large_stderr_does_not_deadlock():
|
||||
assert len(r["stderr"]) <= c._STDERR_CAP
|
||||
|
||||
|
||||
def test_windows_tree_kill_uses_taskkill(monkeypatch):
|
||||
"""The Windows kill path must walk the process tree (taskkill /T /F);
|
||||
Popen.kill() alone orphans grandchildren (issue #13)."""
|
||||
calls = []
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
calls.append(cmd)
|
||||
return _FakeRC()
|
||||
|
||||
class _FakeRC:
|
||||
returncode = 0
|
||||
|
||||
monkeypatch.setattr(c.subprocess, "run", fake_run)
|
||||
c._kill_process_tree_windows(1234)
|
||||
assert calls == [["taskkill", "/PID", "1234", "/T", "/F"]]
|
||||
|
||||
|
||||
@pytest.mark.skipif(os.name != "nt", reason="Windows-only: real process-tree kill")
|
||||
def test_spawn_test_windows_kills_child_process_tree(tmp_path):
|
||||
"""A spawn-tested parent that has its own child (npx -> node style) must
|
||||
not leave the child running after the timeout kill (issue #13)."""
|
||||
import subprocess as sp
|
||||
import time as _t
|
||||
|
||||
pid_file = tmp_path / "child.pid"
|
||||
parent_script = (
|
||||
"import subprocess, sys, time\n"
|
||||
"p = subprocess.Popen([sys.executable, '-c', 'import time; time.sleep(60)'])\n"
|
||||
f"open({str(pid_file)!r}, 'w').write(str(p.pid))\n"
|
||||
"time.sleep(60)\n"
|
||||
)
|
||||
r = c.spawn_test({"command": sys.executable, "args": ["-c", parent_script]}, timeout=2.0)
|
||||
assert r["outcome"] == "ok" # parent was still running at timeout, then tree-killed
|
||||
assert pid_file.is_file(), "parent never spawned its child"
|
||||
child_pid = int(pid_file.read_text())
|
||||
|
||||
deadline = _t.time() + 5.0
|
||||
alive = True
|
||||
while _t.time() < deadline:
|
||||
out = sp.run(
|
||||
["tasklist", "/FI", f"PID eq {child_pid}"], capture_output=True, text=True
|
||||
).stdout
|
||||
alive = str(child_pid) in out
|
||||
if not alive:
|
||||
break
|
||||
_t.sleep(0.25)
|
||||
assert not alive, f"child pid {child_pid} survived the spawn-test kill"
|
||||
|
||||
|
||||
def test_spawn_test_non_string_env_value():
|
||||
# Pasted JSON can carry numeric env values ("env": {"PORT": 8080}) that never
|
||||
# round-trip through the editor. Popen rejects non-str env; spawn_test must
|
||||
@@ -715,6 +906,68 @@ def test_args_secret_warning_empty():
|
||||
assert c.args_secret_warning({"args": []}) is None
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Named server sets (issue #52)
|
||||
# --------------------------------------------------------------------------- #
|
||||
def _three_servers():
|
||||
return [
|
||||
c.ServerEntry("alpha", {"command": "npx"}, True),
|
||||
c.ServerEntry("beta", {"command": "uvx"}, True),
|
||||
c.ServerEntry("gamma", {"url": "https://x.example/mcp"}, False),
|
||||
]
|
||||
|
||||
|
||||
def test_save_and_list_server_sets_roundtrip():
|
||||
cfg: dict = {}
|
||||
members = c.save_server_set(cfg, "webdev", _three_servers())
|
||||
assert members == ["alpha", "beta"] # only the enabled ones
|
||||
assert c.list_server_sets(cfg) == {"webdev": ["alpha", "beta"]}
|
||||
|
||||
|
||||
def test_apply_server_set_enables_exactly_the_members():
|
||||
servers = _three_servers()
|
||||
missing = c.apply_server_set(servers, ["gamma"])
|
||||
assert missing == []
|
||||
assert [s.enabled for s in servers] == [False, False, True]
|
||||
|
||||
|
||||
def test_apply_server_set_reports_missing_members():
|
||||
servers = _three_servers()
|
||||
missing = c.apply_server_set(servers, ["alpha", "vanished", "gone"])
|
||||
assert missing == ["gone", "vanished"]
|
||||
assert [s.enabled for s in servers] == [True, False, False] # rest still applied
|
||||
|
||||
|
||||
def test_delete_server_set_drops_empty_key():
|
||||
cfg: dict = {}
|
||||
c.save_server_set(cfg, "only", _three_servers())
|
||||
assert c.delete_server_set(cfg, "only") is True
|
||||
assert c.SETS_KEY not in cfg # no empty bcc key left behind
|
||||
assert c.delete_server_set(cfg, "only") is False
|
||||
|
||||
|
||||
def test_server_sets_survive_write_and_reload(tmp_path):
|
||||
cfgpath = tmp_path / "cfg.json"
|
||||
cfg = {"mcpServers": {"alpha": {"command": "npx"}}}
|
||||
c.save_server_set(cfg, "webdev", [c.ServerEntry("alpha", {"command": "npx"}, True)])
|
||||
c.write_config(cfgpath, cfg)
|
||||
reloaded = c.load_config(cfgpath)
|
||||
assert c.list_server_sets(reloaded) == {"webdev": ["alpha"]}
|
||||
|
||||
|
||||
def test_apply_servers_preserves_sets_key():
|
||||
cfg: dict = {"mcpServers": {}}
|
||||
c.save_server_set(cfg, "s", [c.ServerEntry("alpha", {"command": "npx"}, True)])
|
||||
c.apply_servers(cfg, _three_servers())
|
||||
assert c.SETS_KEY in cfg # the server writer never touches sets
|
||||
|
||||
|
||||
def test_list_server_sets_skips_malformed_entries():
|
||||
cfg = {c.SETS_KEY: {"good": ["a"], "bad-not-list": "a", "bad-items": ["a", 3]}}
|
||||
assert c.list_server_sets(cfg) == {"good": ["a"]}
|
||||
assert c.list_server_sets({c.SETS_KEY: "junk"}) == {}
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# resolve_name_collision (paste/import duplicate-name handling — issue #8)
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -970,11 +1223,13 @@ class _FakeCompletedProcess:
|
||||
|
||||
|
||||
def test_restart_claude_desktop_macos_commands(monkeypatch):
|
||||
"""macOS: pkill -x "Claude" then open -a Claude, in that order."""
|
||||
"""macOS: pkill -x "Claude", wait for exit (pgrep), then open -a Claude."""
|
||||
calls = []
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
calls.append(cmd)
|
||||
if cmd[0] == "pgrep":
|
||||
return _FakeCompletedProcess(returncode=1) # already exited
|
||||
return _FakeCompletedProcess(returncode=0)
|
||||
|
||||
monkeypatch.setattr(c.sys, "platform", "darwin")
|
||||
@@ -982,14 +1237,14 @@ def test_restart_claude_desktop_macos_commands(monkeypatch):
|
||||
result = c.restart_claude_desktop()
|
||||
assert result.success
|
||||
assert calls[0] == ["pkill", "-x", "Claude"]
|
||||
assert calls[1] == ["open", "-a", "Claude"]
|
||||
assert calls[-1] == ["open", "-a", "Claude"]
|
||||
|
||||
|
||||
def test_restart_claude_desktop_macos_pkill_not_running_is_fine(monkeypatch):
|
||||
"""pkill exiting non-zero (nothing to kill) must NOT be treated as failure."""
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
if cmd[0] == "pkill":
|
||||
if cmd[0] in ("pkill", "pgrep"):
|
||||
return _FakeCompletedProcess(returncode=1) # no matching process
|
||||
return _FakeCompletedProcess(returncode=0)
|
||||
|
||||
@@ -1013,12 +1268,12 @@ def test_restart_claude_desktop_macos_relaunch_failure_reported(monkeypatch):
|
||||
|
||||
|
||||
def test_restart_claude_desktop_macos_pkill_binary_missing_does_not_raise(monkeypatch):
|
||||
"""pkill raising OSError (binary missing) must be swallowed, not propagated --
|
||||
relaunch is still attempted."""
|
||||
"""pkill/pgrep raising OSError (binary missing) must be swallowed, not
|
||||
propagated -- relaunch is still attempted."""
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
if cmd[0] == "pkill":
|
||||
raise OSError("pkill not found")
|
||||
if cmd[0] in ("pkill", "pgrep"):
|
||||
raise OSError("binary not found")
|
||||
return _FakeCompletedProcess(returncode=0)
|
||||
|
||||
monkeypatch.setattr(c.sys, "platform", "darwin")
|
||||
@@ -1027,7 +1282,45 @@ def test_restart_claude_desktop_macos_pkill_binary_missing_does_not_raise(monkey
|
||||
assert result.success
|
||||
|
||||
|
||||
def test_restart_claude_desktop_windows_commands(monkeypatch):
|
||||
def test_restart_claude_desktop_macos_gives_up_if_app_wont_quit(monkeypatch):
|
||||
monkeypatch.setattr(c.sys, "platform", "darwin")
|
||||
monkeypatch.setattr(c, "_run_quiet", lambda cmd: None)
|
||||
monkeypatch.setattr(c, "_macos_claude_running", lambda: True) # never exits
|
||||
monkeypatch.setattr(c, "_MACOS_QUIT_WAIT_S", 0.2)
|
||||
res = c.restart_claude_desktop()
|
||||
assert res.success is False
|
||||
assert "quit" in res.detail.lower()
|
||||
|
||||
|
||||
def test_restart_claude_desktop_macos_waits_for_exit_then_relaunches(monkeypatch):
|
||||
monkeypatch.setattr(c.sys, "platform", "darwin")
|
||||
monkeypatch.setattr(c, "_run_quiet", lambda cmd: None)
|
||||
alive = iter([True, True, False]) # exits on the third poll
|
||||
monkeypatch.setattr(c, "_macos_claude_running", lambda: next(alive))
|
||||
|
||||
launched = []
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
launched.append(cmd)
|
||||
return _FakeCompletedProcess(returncode=0)
|
||||
|
||||
monkeypatch.setattr(c.subprocess, "run", fake_run)
|
||||
res = c.restart_claude_desktop()
|
||||
assert res.success is True
|
||||
assert launched == [["open", "-a", "Claude"]]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def windows_shortcut(tmp_path, monkeypatch):
|
||||
"""A fake %APPDATA% containing the Claude Start-menu shortcut."""
|
||||
monkeypatch.setenv("APPDATA", str(tmp_path))
|
||||
lnk = tmp_path / "Microsoft" / "Windows" / "Start Menu" / "Programs" / "Claude.lnk"
|
||||
lnk.parent.mkdir(parents=True)
|
||||
lnk.write_bytes(b"")
|
||||
return lnk
|
||||
|
||||
|
||||
def test_restart_claude_desktop_windows_commands(monkeypatch, windows_shortcut):
|
||||
"""Windows: taskkill the process, then relaunch via the Start-menu shortcut."""
|
||||
calls = []
|
||||
|
||||
@@ -1044,7 +1337,7 @@ def test_restart_claude_desktop_windows_commands(monkeypatch):
|
||||
assert calls[1][-1].endswith("Claude.lnk")
|
||||
|
||||
|
||||
def test_restart_claude_desktop_windows_relaunch_failure_reported(monkeypatch):
|
||||
def test_restart_claude_desktop_windows_relaunch_failure_reported(monkeypatch, windows_shortcut):
|
||||
def fake_run(cmd, **kwargs):
|
||||
if cmd[0] == "cmd":
|
||||
return _FakeCompletedProcess(returncode=1, stderr="not found")
|
||||
@@ -1057,43 +1350,37 @@ def test_restart_claude_desktop_windows_relaunch_failure_reported(monkeypatch):
|
||||
assert "not found" in result.detail
|
||||
|
||||
|
||||
def test_restart_claude_desktop_linux_commands(monkeypatch):
|
||||
"""Linux: pkill claude, then relaunch via a detached Popen (no waiting)."""
|
||||
run_calls = []
|
||||
popen_calls = []
|
||||
def test_restart_claude_desktop_windows_missing_shortcut_refuses_before_killing(
|
||||
monkeypatch, tmp_path
|
||||
):
|
||||
killed = []
|
||||
monkeypatch.setattr(c, "_run_quiet", lambda cmd: killed.append(cmd))
|
||||
monkeypatch.setattr(c.sys, "platform", "win32")
|
||||
monkeypatch.setenv("APPDATA", str(tmp_path)) # no Claude.lnk under here (MSIX case)
|
||||
res = c.restart_claude_desktop()
|
||||
assert res.success is False
|
||||
assert "shortcut" in res.detail.lower()
|
||||
assert killed == [] # Claude must NOT be killed when it can't be relaunched
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
run_calls.append(cmd)
|
||||
return _FakeCompletedProcess(returncode=0)
|
||||
|
||||
class _FakePopen:
|
||||
def __init__(self, cmd, **kwargs):
|
||||
popen_calls.append((cmd, kwargs))
|
||||
|
||||
def test_restart_supported_only_on_desktop_platforms(monkeypatch):
|
||||
monkeypatch.setattr(c.sys, "platform", "darwin")
|
||||
assert c.restart_supported()
|
||||
monkeypatch.setattr(c.sys, "platform", "win32")
|
||||
assert c.restart_supported()
|
||||
monkeypatch.setattr(c.sys, "platform", "linux")
|
||||
monkeypatch.setattr(c.subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(c.subprocess, "Popen", _FakePopen)
|
||||
result = c.restart_claude_desktop()
|
||||
assert result.success
|
||||
assert run_calls == [["pkill", "claude"]]
|
||||
cmd, kwargs = popen_calls[0]
|
||||
assert cmd == ["claude"]
|
||||
assert kwargs.get("start_new_session") is True
|
||||
assert not c.restart_supported()
|
||||
|
||||
|
||||
def test_restart_claude_desktop_linux_popen_failure_reported(monkeypatch):
|
||||
def fake_run(cmd, **kwargs):
|
||||
return _FakeCompletedProcess(returncode=0)
|
||||
|
||||
def fake_popen(cmd, **kwargs):
|
||||
raise OSError("no such file or directory")
|
||||
|
||||
def test_restart_claude_desktop_linux_refuses_without_touching_processes(monkeypatch):
|
||||
"""There is no Claude Desktop on Linux; 'pkill claude' would substring-match
|
||||
running Claude Code CLI sessions. The restart must refuse outright."""
|
||||
killed = []
|
||||
monkeypatch.setattr(c, "_run_quiet", lambda cmd: killed.append(cmd))
|
||||
monkeypatch.setattr(c.sys, "platform", "linux")
|
||||
monkeypatch.setattr(c.subprocess, "run", fake_run)
|
||||
monkeypatch.setattr(c.subprocess, "Popen", fake_popen)
|
||||
result = c.restart_claude_desktop()
|
||||
assert not result.success
|
||||
assert "Claude Desktop" in result.detail
|
||||
res = c.restart_claude_desktop()
|
||||
assert res.success is False
|
||||
assert killed == [] # nothing killed, nothing spawned
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
@@ -1101,8 +1388,12 @@ def test_restart_claude_desktop_linux_popen_failure_reported(monkeypatch):
|
||||
# --------------------------------------------------------------------------- #
|
||||
def test_dunder_version_matches_pyproject():
|
||||
# Guards against the version drifting out of sync between the two places
|
||||
# a human might bump it.
|
||||
assert c.__version__ == "1.2.0"
|
||||
# a human might bump it. Reads pyproject.toml rather than hard-coding a
|
||||
# version here (which would make this a third place to bump).
|
||||
text = (Path(__file__).parent.parent / "pyproject.toml").read_text(encoding="utf-8")
|
||||
m = re.search(r'^version\s*=\s*"([^"]+)"', text, re.MULTILINE)
|
||||
assert m, "no [project] version found in pyproject.toml"
|
||||
assert c.__version__ == m.group(1)
|
||||
|
||||
|
||||
def test_parse_version_basic():
|
||||
|
||||
Reference in New Issue
Block a user