feat: author ${VAR} references, gated on whether the client expands them (#76)

The blocker on this issue was whether BCC or the client does the expanding.
Answer, from Anthropic's docs: Claude Code expands ${VAR} and
${VAR:-default} itself, in command, args, env, url and headers, for both
project .mcp.json and user-scope ~/.claude.json. Claude Desktop has no
documented support.

So this is a per-client capability, not a global one, and BCC does NOT
expand on write: resolving a reference into the file would put the secret
back on disk -- the whole thing the user is avoiding -- and would defeat a
feature the client already implements correctly. BCC authors, validates and
warns; expand_env_refs exists to preview what the client will do.

Semantics mirror the documented ones exactly, including the unusual bit:
an unset variable with no default is left as literal ${VAR} text rather
than blanked, because that is what Claude Code passes through.

Gating uses the existing profile_targets_claude_desktop(), so a config that
is correct under Claude Code and broken under Desktop is reported against
whichever profile is actually loaded. The two warnings are worded
differently on purpose -- 'this client will never expand these' is a
different problem from 'this variable looks unset here'.

Two existing behaviours were backwards for this feature and are fixed:

- Secret masking hid placeholders. is_secret_key('API_KEY') is true, so
  ${API_KEY} rendered as dots -- making a reference indistinguishable from
  a stored credential, which is the one distinction that makes the feature
  worth adopting. should_mask_value() now skips references, in the table
  delegate, _redact_server_data and redact_args alike.
- args_secret_warning fired on placeholders. Moving a token into ${VAR} is
  the recommended fix for that warning; continuing to warn punished the
  fix. It now skips references while still flagging a real secret that
  follows one.

Real secrets are still masked everywhere they were before -- asserted, not
assumed.

Refs #76

bcc.py

@@ -10,6 +10,7 @@ Run:  python mcp_manager.py
 
 from __future__ import annotations
 
+import contextlib
 import sys
 import time
 from pathlib import Path
@@ -141,6 +142,10 @@ QScrollBar::handle:vertical {{ background: {p.border}; border-radius: 5px; min-h
 QScrollBar::add-line, QScrollBar::sub-line {{ height: 0; }}
 QLabel#statusbar {{ color: {p.muted}; padding: 4px 2px; }}
 QLabel#warnBanner {{ color: {p.on_accent}; background: {p.warn}; border-radius: 8px; padding: 8px 10px; font-weight: 600; }}
+QFrame#noticeBanner {{ background: {p.panel_2}; border: 1px solid {p.accent}; border-radius: 8px; }}
+QLabel#noticeText {{ color: {p.text}; }}
+QPushButton#noticeClose {{ background: transparent; border: none; color: {p.muted}; font-size: 14px; padding: 2px; }}
+QPushButton#noticeClose:hover {{ color: {p.text}; }}
 QLabel#section {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
 QLabel#sectionDisabled {{ color: {p.muted}; font-weight: 600; font-size: 12px; padding: 2px 2px; }}
 QLabel#placeholder {{ color: {p.muted}; padding: 12px; background: {p.panel_2}; border: 1px dashed {p.border}; border-radius: 8px; }}
@@ -263,7 +268,7 @@ class _SecretMaskDelegate(QStyledItemDelegate):
         if self.revealed or not option.text:
             return
         key_item = self._table.item(index.row(), 0)
-        if key_item and core.is_secret_key(key_item.text()):
+        if key_item and core.should_mask_value(key_item.text(), option.text):
             option.text = core.MASK
 
 
@@ -1528,6 +1533,54 @@ class AboutDialog(QDialog):
         QDesktopServices.openUrl(QUrl(self._release_url or core.RELEASES_URL))
 
 
+class NoticeBanner(QFrame):
+    """A persistent, dismissible notice with an optional action button.
+
+    The status bar is the wrong home for anything the user needs to act on --
+    21 call sites rewrite it, so a message posted there is gone by the next
+    click. That wiped the MSIX warning (#35) and then the update notice (#78).
+    This is the shared mechanism so it doesn't happen a third time.
+    """
+
+    def __init__(self, parent=None):
+        super().__init__(parent)
+        self.setObjectName("noticeBanner")
+        row = QHBoxLayout(self)
+        row.setContentsMargins(10, 8, 8, 8)
+        row.setSpacing(8)
+        self._label = QLabel("")
+        self._label.setObjectName("noticeText")
+        self._label.setWordWrap(True)
+        row.addWidget(self._label, 1)
+        self._action_btn = QPushButton("")
+        self._action_btn.setCursor(Qt.CursorShape.PointingHandCursor)
+        self._action_btn.hide()
+        row.addWidget(self._action_btn)
+        self._close_btn = QPushButton("\u2715")
+        self._close_btn.setObjectName("noticeClose")
+        self._close_btn.setCursor(Qt.CursorShape.PointingHandCursor)
+        self._close_btn.setFixedWidth(26)
+        self._close_btn.setToolTip("Dismiss")
+        self._close_btn.clicked.connect(self.hide)
+        row.addWidget(self._close_btn)
+        self.hide()
+
+    def show_notice(self, text: str, action_label: str = "", on_action=None):
+        self._label.setText(text)
+        self._label.setToolTip(text)
+        # Reconnect cleanly: a banner reused for a second notice would
+        # otherwise fire the previous notice's action too.
+        with contextlib.suppress(RuntimeError, TypeError):
+            self._action_btn.clicked.disconnect()
+        if action_label and on_action is not None:
+            self._action_btn.setText(action_label)
+            self._action_btn.clicked.connect(lambda _=False: on_action())
+            self._action_btn.show()
+        else:
+            self._action_btn.hide()
+        self.show()
+
+
 # --------------------------------------------------------------------------- #
 # 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.
@@ -1585,6 +1638,11 @@ class MainWindow(QMainWindow):
         self.warn_banner.hide()
         root.addWidget(self.warn_banner)
 
+        # Update availability gets its own persistent banner rather than a
+        # status-line write, which the next UI action overwrites (#78).
+        self.update_banner = NoticeBanner(self)
+        root.addWidget(self.update_banner)
+
         # User-draggable divider between the server list and the editor.
         split = QSplitter(Qt.Orientation.Horizontal)
         split.setChildrenCollapsible(False)
@@ -1637,10 +1695,59 @@ class MainWindow(QMainWindow):
             theme_menu.addAction(act)
 
         help_menu = self.menuBar().addMenu("&Help")
+
+        # "Check for updates" used to exist only as a button inside the About
+        # dialog, which is not somewhere anyone looks for it (#79).
+        update_action = QAction("Check for updates…", self)
+        # Explicit role: macOS relocates actions it recognises by text, and
+        # some Qt versions treat "update" as application-menu material. Pin it
+        # so the item stays where the menu says it is on every platform.
+        update_action.setMenuRole(QAction.MenuRole.ApplicationSpecificRole)
+        update_action.triggered.connect(self.check_for_updates)
+        help_menu.addAction(update_action)
+        help_menu.addSeparator()
+
         about_action = QAction("About Better Claude Config…", self)
+        # Qt auto-assigns AboutRole to actions whose text starts with "About",
+        # which moves this into the application menu on macOS. That is the
+        # right home there -- state it explicitly rather than inheriting it by
+        # accident, since the behaviour is invisible from this call site.
+        about_action.setMenuRole(QAction.MenuRole.AboutRole)
         about_action.triggered.connect(self._show_about)
         help_menu.addAction(about_action)
 
+    def _show_update_notice(self, notice: dict):
+        """Surface an available update where it survives the next click."""
+        url = notice["url"]
+        self.update_banner.show_notice(
+            notice["text"],
+            action_label="Open releases page",
+            on_action=lambda: QDesktopServices.openUrl(QUrl(url)),
+        )
+
+    def check_for_updates(self):
+        """Menu-driven check. Unlike the startup check this is never throttled
+        and always reports back -- the user asked, so silence would read as a
+        broken button."""
+        self.status.setText("Checking for updates…")
+        self._menu_update_worker = UpdateCheckWorker()
+        self._menu_update_worker.done.connect(self._on_menu_update_checked)
+        self._menu_update_worker.start()
+
+    def _on_menu_update_checked(self, release: dict | None):
+        self._menu_update_worker = None
+        if release is None:
+            self.status.setText("Couldn't check for updates (offline?).")
+            return
+        QSettings("BCC", "BetterClaudeConfig").setValue("update/lastCheck", time.time())
+        notice = core.update_notice(core.__version__, release)
+        if notice:
+            self._show_update_notice(notice)
+            self.status.setText(f"Update available: {notice['version']}")
+        else:
+            self.update_banner.hide()
+            self.status.setText(f"You're up to date ({core.__version__}).")
+
     def _set_theme(self, setting: str):
         """Persist the theme choice and repaint the running window."""
         QSettings("BCC", "BetterClaudeConfig").setValue("ui/theme", setting)
@@ -1681,10 +1788,9 @@ class MainWindow(QMainWindow):
         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."
-            )
+        notice = core.update_notice(core.__version__, release)
+        if notice:
+            self._show_update_notice(notice)
 
     # --- layout persistence ---------------------------------------------- #
     def _restore_layout(self):
@@ -2516,6 +2622,12 @@ class MainWindow(QMainWindow):
             self.save_btn.setEnabled(False)
             return False
         lint_warnings = core.lint_servers(self.servers)
+        # ${VAR} references are only meaningful if the target client expands
+        # them -- Claude Desktop doesn't, so the same config is fine in one
+        # profile and broken in another (#76). Report against the loaded one.
+        for entry in self.servers:
+            for warning in core.env_ref_warnings(entry.data, self.current_profile):
+                lint_warnings.append(f"'{entry.name}': {warning}")
         if lint_warnings:
             self.validation_lbl.setText(f"⚠ {lint_warnings[0]}")
             self.validation_lbl.setStyleSheet(f"color: {WARN};")

bcc_core.py

@@ -170,6 +170,39 @@ def fetch_latest_release(timeout: float = 4.0) -> dict | None:
     return {"version": tag, "url": payload.get("html_url") or RELEASES_URL}
 
 
+def update_notice(
+    current: str, release: dict | None, url_fallback: str = RELEASES_URL
+) -> dict | None:
+    """Decide whether to tell the user about a release, and what to say.
+
+    Returns {"version", "text", "url"} when `release` is newer than `current`,
+    or None when it isn't, when the check failed, or when the payload is
+    malformed. Kept here rather than in the GUI so the wording and the
+    should-we-notify decision are testable -- bcc.py can't be imported by the
+    test suite, which has no PySide6.
+
+    The text deliberately names no menu path. The old status-line notice read
+    "Help > About to view it", which is wrong on macOS: Qt relocates the About
+    action into the application menu (#79). A notice that carries its own
+    action can't drift out of sync with the platform.
+    """
+    if not isinstance(release, dict):
+        return None
+    version = release.get("version")
+    if not version or not isinstance(version, str):
+        return None
+    if not is_newer_version(current, version):
+        return None
+    # Tags carry a "v" prefix and __version__ doesn't; render both the same way
+    # so the notice doesn't read "Version v1.3.0 ... you're running 1.2.0".
+    shown = version.lstrip("vV")
+    return {
+        "version": version,
+        "text": f"Version {shown} is available. You're running {current.lstrip('vV')}.",
+        "url": release.get("url") or url_fallback,
+    }
+
+
 # --------------------------------------------------------------------------- #
 # Data model
 # --------------------------------------------------------------------------- #
@@ -860,13 +893,28 @@ def backup_label(backup_path: Path | str) -> str:
     return f"{ts[:4]}-{ts[4:6]}-{ts[6:8]}  {ts[9:11]}:{ts[11:13]}:{ts[13:]}"
 
 
+def should_mask_value(key: str, value) -> bool:
+    """Whether an env/header value should be masked for display.
+
+    A ${VAR} reference is NOT a secret -- it's a pointer to one, and it's the
+    thing we want users to adopt. Masking it to dots would make a reference
+    indistinguishable from a stored credential, hiding exactly the distinction
+    that makes the feature worth using (#76).
+    """
+    if not is_secret_key(key):
+        return False
+    return not is_env_ref(value) if isinstance(value, str) else True
+
+
 def _redact_server_data(data: dict) -> dict:
     """Return a copy of a server definition with secrets masked for display."""
     out = dict(data)
     if "args" in out:
         out["args"] = redact_args(list(out["args"] or []))
     if "env" in out:
-        out["env"] = {k: (MASK if is_secret_key(k) else v) for k, v in (out["env"] or {}).items()}
+        out["env"] = {
+            k: (MASK if should_mask_value(k, v) else v) for k, v in (out["env"] or {}).items()
+        }
     return out
 
 
@@ -1417,6 +1465,155 @@ MASK = "••••••••"
 _EMBEDDED_CRED_RE = re.compile(r"://[^:@/\s]+:[^:@/\s]+@")
 
 
+# --------------------------------------------------------------------------- #
+# Environment-variable references (issue #76)
+#
+# Claude Code expands ${VAR} and ${VAR:-default} itself, in command, args, env,
+# url and headers. So BCC does NOT expand these on write -- resolving them into
+# the file would put the secret back on disk, which is the whole thing the user
+# is avoiding, and would defeat a feature the client already implements. BCC
+# authors, validates and warns.
+#
+# Claude Desktop has no documented support, so the same text there is passed to
+# the server literally. That makes this a per-client capability, not a global
+# one -- see client_expands_env_refs().
+# --------------------------------------------------------------------------- #
+# ${NAME} or ${NAME:-default}. Names follow the shell convention (letter or
+# underscore first) so a bare "${}" or "${1}" isn't mistaken for a reference.
+_ENV_REF_RE = re.compile(r"\$\{([A-Za-z_][A-Za-z0-9_]*)(?::-([^}]*))?\}")
+
+# The five fields Claude Code documents as expansion sites.
+ENV_REF_FIELDS = ("command", "args", "env", "url", "headers")
+
+
+class EnvRef(NamedTuple):
+    """One ${VAR} / ${VAR:-default} occurrence found in a server definition."""
+
+    name: str
+    default: str | None
+    field: str  # which of ENV_REF_FIELDS it was found in
+
+    @property
+    def has_default(self) -> bool:
+        return self.default is not None
+
+
+def find_env_refs(text: str, field: str = "") -> list[EnvRef]:
+    """Every ${VAR} / ${VAR:-default} reference in a single string."""
+    if not isinstance(text, str):
+        return []
+    return [EnvRef(m.group(1), m.group(2), field) for m in _ENV_REF_RE.finditer(text)]
+
+
+def is_env_ref(value: str) -> bool:
+    """True when the value contains at least one ${VAR} reference.
+
+    Used to keep placeholders OUT of secret masking: `${API_KEY}` under a
+    secret-looking key is a reference, not a secret, and masking it to dots
+    would hide the one distinction the user needs to see.
+    """
+    return bool(find_env_refs(value))
+
+
+def server_env_refs(data: dict) -> list[EnvRef]:
+    """Every env reference in a server definition, tagged with its field.
+
+    Only inspects the fields Claude Code actually expands; a ${VAR} written
+    into some other key is not a reference and shouldn't be reported as one.
+    """
+    out: list[EnvRef] = []
+    if not isinstance(data, dict):
+        return out
+    for field in ENV_REF_FIELDS:
+        value = data.get(field)
+        if isinstance(value, str):
+            out.extend(find_env_refs(value, field))
+        elif isinstance(value, list):
+            for item in value:
+                out.extend(find_env_refs(item, field))
+        elif isinstance(value, dict):
+            for v in value.values():
+                out.extend(find_env_refs(v, field))
+    return out
+
+
+def expand_env_refs(text: str, environ: dict | None = None) -> str:
+    """Expand ${VAR} / ${VAR:-default} the way Claude Code documents it.
+
+    Provided for previewing what the client will do -- BCC never writes the
+    expanded form back to the config. Unset with no default is left as the
+    literal ${VAR} text, matching Claude Code: the config still loads and the
+    unexpanded text is passed through.
+    """
+    if not isinstance(text, str):
+        return text
+    env = os.environ if environ is None else environ
+
+    def repl(m: re.Match) -> str:
+        name, default = m.group(1), m.group(2)
+        if name in env:
+            return env[name]
+        return default if default is not None else m.group(0)
+
+    return _ENV_REF_RE.sub(repl, text)
+
+
+def unresolved_env_refs(data: dict, environ: dict | None = None) -> list[EnvRef]:
+    """References that would not resolve: variable unset AND no default.
+
+    Best-effort by nature -- BCC's environment isn't necessarily the client's,
+    so this warns rather than blocks, and the warning text says so.
+    """
+    env = os.environ if environ is None else environ
+    return [r for r in server_env_refs(data) if not r.has_default and r.name not in env]
+
+
+def client_expands_env_refs(profile: Profile) -> bool:
+    """Whether the client behind `profile` expands ${VAR} itself.
+
+    Claude Code does, in command/args/env/url/headers, for both project
+    `.mcp.json` and user-scope `~/.claude.json`. Claude Desktop has no
+    documented support, so a reference there reaches the server as literal
+    text -- which surfaces as a confusing auth failure rather than an obvious
+    config error, hence the warning.
+    """
+    return not profile_targets_claude_desktop(profile)
+
+
+def env_ref_warnings(
+    data: dict, profile: Profile | None = None, environ: dict | None = None
+) -> list[str]:
+    """Advisory warnings about env references in one server definition.
+
+    Two distinct problems, deliberately worded differently:
+      - the target client won't expand them at all (Claude Desktop)
+      - the client will expand them, but a variable looks unset here
+    """
+    refs = server_env_refs(data)
+    if not refs:
+        return []
+
+    if profile is not None and not client_expands_env_refs(profile):
+        names = ", ".join(sorted({f"${{{r.name}}}" for r in refs}))
+        return [
+            f"{names} will NOT be expanded by Claude Desktop -- it has no "
+            f"documented support for variable references, so the server "
+            f"receives the literal text. Use a real value here, or move this "
+            f"server to a Claude Code config."
+        ]
+
+    missing = unresolved_env_refs(data, environ)
+    if not missing:
+        return []
+    names = ", ".join(sorted({r.name for r in missing}))
+    return [
+        f"{names} is not set in this environment and has no ':-default'. "
+        f"Claude Code will pass the reference through unexpanded. "
+        f"(Checked against BCC's environment, which may differ from the "
+        f"client's.)"
+    ]
+
+
 def is_secret_key(name: str) -> bool:
     """Does this env-var / header / flag name look like it holds a secret?"""
     return bool(_SECRET_KEY_RE.search(name or ""))
@@ -1433,17 +1630,22 @@ def redact_args(args: list[str]) -> list[str]:
       --api-key=abc123      ->  --api-key=••••••••   (inline flag=value)
       ghp_abc123            ->  ••••••••             (well-known token prefix)
     Everything else passes through untouched.
+
+    ${VAR} references are left visible: they name a secret rather than being
+    one, and hiding them would obscure the difference between "this config
+    leaks a token" and "this config points at one" (#76).
     """
     out: list[str] = []
     mask_next = False
     for a in args:
         s = str(a)
         if mask_next:
-            out.append(MASK)
             mask_next = False
+            out.append(s if is_env_ref(s) else MASK)
             continue
         if s.startswith("-") and "=" in s and is_secret_key(s.split("=", 1)[0]):
-            out.append(s.split("=", 1)[0] + "=" + MASK)
+            flag, value = s.split("=", 1)
+            out.append(f"{flag}={value}" if is_env_ref(value) else f"{flag}={MASK}")
             continue
         if s.startswith("-") and is_secret_key(s):
             out.append(s)
@@ -1470,6 +1672,11 @@ def args_secret_warning(data: dict) -> str | None:
     args = [str(a) for a in (data.get("args") or [])]
     mask_next = False
     for a in args:
+        # A ${VAR} reference is the recommended fix for this very warning --
+        # continuing to warn after the user adopts it punishes the fix (#76).
+        if is_env_ref(a):
+            mask_next = False
+            continue
         if mask_next:
             mask_next = False
             if not a.startswith("-"):

tests/test_core.py

@@ -2626,3 +2626,192 @@ def test_every_palette_slot_is_consumed():
         if f.name == "name":
             continue
         assert f"p.{f.name}" in src, f"palette slot {f.name!r} is never consumed"
+
+
+# --------------------------------------------------------------------------- #
+# #78/#79 -- update notice: when to show it, and what it says
+# --------------------------------------------------------------------------- #
+def test_update_notice_when_a_newer_release_exists():
+    n = c.update_notice("1.2.0", {"version": "v1.3.0", "url": "https://example.test/rel"})
+    assert n is not None
+    assert n["version"] == "v1.3.0"
+    assert n["url"] == "https://example.test/rel"
+    assert "1.3.0" in n["text"] and "1.2.0" in n["text"]
+
+
+def test_update_notice_is_silent_when_current():
+    assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None
+    assert c.update_notice("1.4.0", {"version": "v1.3.0"}) is None
+
+
+@pytest.mark.parametrize("bad", [None, {}, {"version": ""}, {"version": None}, {"version": 3}, []])
+def test_update_notice_is_silent_on_a_failed_or_malformed_check(bad):
+    """fetch_latest_release returns None on any failure; a half-formed payload
+    must not produce a notice pointing at nothing."""
+    assert c.update_notice("1.0.0", bad) is None
+
+
+def test_update_notice_falls_back_to_the_releases_page_without_a_url():
+    n = c.update_notice("1.0.0", {"version": "v2.0.0"})
+    assert n["url"] == c.RELEASES_URL
+
+
+def test_update_notice_names_no_menu_path():
+    """The old status-line text said 'Help > About to view it', which is wrong
+    on macOS -- Qt moves the About action into the application menu (#79). The
+    notice carries its own action, so it must not describe a menu path."""
+    n = c.update_notice("1.0.0", {"version": "v2.0.0"})
+    lowered = n["text"].lower()
+    for phrase in ("help", "about", "menu", "▸", ">"):
+        assert phrase not in lowered, f"notice text should not reference {phrase!r}"
+
+
+def test_update_notice_handles_the_v_prefix_consistently():
+    assert c.update_notice("1.2.0", {"version": "1.3.0"}) is not None
+    assert c.update_notice("v1.2.0", {"version": "v1.3.0"}) is not None
+    assert c.update_notice("1.3.0", {"version": "v1.3.0"}) is None
+
+
+def test_update_notice_renders_both_versions_the_same_way():
+    """Tags carry a 'v' prefix, __version__ doesn't -- don't show both forms
+    in one sentence."""
+    n = c.update_notice("1.2.0", {"version": "v1.3.0"})
+    assert "v1.3.0" not in n["text"]
+    assert "1.3.0" in n["text"] and "1.2.0" in n["text"]
+    # the machine-readable field keeps the real tag
+    assert n["version"] == "v1.3.0"
+
+
+# --------------------------------------------------------------------------- #
+# #76 -- ${VAR} references. Semantics mirror Claude Code's documented
+# behaviour: ${VAR} and ${VAR:-default}, expanded in command/args/env/url/
+# headers, and an unset variable with no default left as literal text.
+# --------------------------------------------------------------------------- #
+def test_find_env_refs_plain_and_defaulted():
+    refs = c.find_env_refs("${A} and ${B:-fallback}")
+    assert [(r.name, r.default) for r in refs] == [("A", None), ("B", "fallback")]
+
+
+@pytest.mark.parametrize("text", ["${}", "${1BAD}", "$NOTBRACED", "{NOPE}", "plain", "$${X"])
+def test_find_env_refs_ignores_non_references(text):
+    assert c.find_env_refs(text) == []
+
+
+def test_find_env_refs_allows_an_empty_default():
+    """`${VAR:-}` is a documented way to say 'blank if unset'."""
+    refs = c.find_env_refs("${A:-}")
+    assert refs[0].default == ""
+    assert refs[0].has_default is True
+
+
+def test_server_env_refs_covers_all_five_documented_fields():
+    data = {
+        "command": "${BIN}",
+        "args": ["--x", "${ARG}"],
+        "env": {"K": "${ENVV}"},
+        "url": "${URL}/mcp",
+        "headers": {"Authorization": "Bearer ${HDR}"},
+    }
+    found = {(r.name, r.field) for r in c.server_env_refs(data)}
+    assert found == {
+        ("BIN", "command"),
+        ("ARG", "args"),
+        ("ENVV", "env"),
+        ("URL", "url"),
+        ("HDR", "headers"),
+    }
+
+
+def test_server_env_refs_ignores_unexpanded_fields():
+    """Claude Code expands five fields; a ${VAR} elsewhere isn't a reference."""
+    assert c.server_env_refs({"description": "${NOPE}", "timeout": "${ALSO_NO}"}) == []
+
+
+def test_expand_env_refs_matches_documented_semantics():
+    env = {"SET": "value"}
+    assert c.expand_env_refs("${SET}", env) == "value"
+    assert c.expand_env_refs("${MISSING:-dflt}", env) == "dflt"
+    assert c.expand_env_refs("${SET:-dflt}", env) == "value"
+    # unset with no default: left as literal text, exactly as Claude Code does
+    assert c.expand_env_refs("${MISSING}", env) == "${MISSING}"
+
+
+def test_expand_env_refs_handles_several_in_one_string():
+    assert c.expand_env_refs("${A}/${B:-two}/${C}", {"A": "one"}) == "one/two/${C}"
+
+
+def test_unresolved_env_refs_only_flags_unset_without_default():
+    data = {"env": {"A": "${SET}", "B": "${UNSET}", "C": "${OTHER:-has_default}"}}
+    assert [r.name for r in c.unresolved_env_refs(data, {"SET": "x"})] == ["UNSET"]
+
+
+# --- the two interactions that were backwards for this feature ------------
+def test_placeholder_under_a_secret_key_is_not_masked():
+    """A ${VAR} names a secret rather than being one. Masking it would make a
+    reference indistinguishable from a stored credential."""
+    assert c.should_mask_value("API_KEY", "${API_KEY}") is False
+    assert c.should_mask_value("API_KEY", "ghp_realsecret") is True
+    assert c.should_mask_value("NOT_SECRET", "${API_KEY}") is False
+
+
+def test_redacted_display_keeps_placeholders_but_masks_real_secrets():
+    out = c._redact_server_data({"env": {"API_KEY": "${API_KEY}", "TOKEN": "ghp_real"}})
+    assert out["env"]["API_KEY"] == "${API_KEY}"
+    assert out["env"]["TOKEN"] == c.MASK
+
+
+def test_redact_args_keeps_placeholders_visible():
+    assert c.redact_args(["--token", "${GH_TOKEN}"]) == ["--token", "${GH_TOKEN}"]
+    assert c.redact_args(["--api-key=${K}"]) == ["--api-key=${K}"]
+    # real secrets still masked
+    assert c.redact_args(["--token", "ghp_real"]) == ["--token", c.MASK]
+    assert c.redact_args(["--api-key=sk-real"]) == [f"--api-key={c.MASK}"]
+
+
+def test_args_secret_warning_is_silenced_by_a_placeholder():
+    """Moving a token into ${VAR} is the recommended fix for this warning --
+    still warning afterwards would punish the fix."""
+    assert c.args_secret_warning({"args": ["--token", "ghp_real"]}) is not None
+    assert c.args_secret_warning({"args": ["--token", "${GH_TOKEN}"]}) is None
+
+
+def test_args_secret_warning_still_fires_on_the_arg_after_a_placeholder():
+    """A placeholder must clear the pending-flag state, not blanket-suppress."""
+    assert c.args_secret_warning({"args": ["${SAFE}", "--token", "ghp_real"]}) is not None
+
+
+# --- per-client gating ----------------------------------------------------
+def _profile(path):
+    return c.Profile(label="p", path=Path(path), config_exists=True)
+
+
+def test_claude_code_profiles_expand_references():
+    assert c.client_expands_env_refs(_profile(Path.home() / ".claude.json")) is True
+    assert c.client_expands_env_refs(_profile("/repo/.mcp.json")) is True
+
+
+def test_claude_desktop_profile_does_not_expand_references():
+    desktop = _profile(c.app_support_base() / "Claude" / c.CONFIG_FILENAME)
+    assert c.client_expands_env_refs(desktop) is False
+
+
+def test_desktop_profile_warns_that_references_are_literal():
+    data = {"env": {"API_KEY": "${API_KEY}"}}
+    desktop = _profile(c.app_support_base() / "Claude" / c.CONFIG_FILENAME)
+    warnings = c.env_ref_warnings(data, desktop, {"API_KEY": "set"})
+    assert len(warnings) == 1
+    assert "NOT be expanded" in warnings[0]
+    assert "${API_KEY}" in warnings[0]
+
+
+def test_claude_code_profile_warns_only_about_unset_variables():
+    code = _profile(Path.home() / ".claude.json")
+    data = {"env": {"A": "${UNSET_ONE}"}}
+    assert c.env_ref_warnings(data, code, {}) != []
+    assert c.env_ref_warnings(data, code, {"UNSET_ONE": "x"}) == []
+    # a default means it always resolves
+    assert c.env_ref_warnings({"env": {"A": "${X:-d}"}}, code, {}) == []
+
+
+def test_no_references_means_no_warnings():
+    assert c.env_ref_warnings({"command": "npx", "args": ["-y", "pkg"]}, None) == []