Merge upstream main (fc8d15d779): freshen before PR push

scripts/ci/check_public_surface.py

@@ -0,0 +1,180 @@
+#!/usr/bin/env python3
+"""Public-surface diff against a base ref: dropped public names and dropped test functions.
+
+The Sep 2026 whole-codebase refactor (PR #102117) opened with 1,703 public top-level names dropped
+across 341 modules, 1,000 public methods in 166, and 130 ``def test_`` deleted in 54 files.
+Reviewers found ~30 of the names by hand; the rest surfaced as post-merge fixes (10 commits
+restoring symbols/re-exports, 6 restoring tests, plus a qwen OAuth break that went past import
+smoke because the caller used ``module.attr``). Every one of those was catchable in seconds with
+this script; nothing ran it because it did not exist.
+
+Rules (deliberately narrow, no allowlist file to maintain):
+- A PUBLIC top-level name (function, class, module-level assignment, re-exported import) or a
+  public/dunder method of a module present on BOTH sides may be moved and re-exported or
+  deprecated, never silently removed. A deleted MODULE is not a drop here (that is a visible
+  decision; ``check_doc_paths`` territory).
+- A ``tests/**`` file present on both sides must not end with fewer ``def test_`` than it started
+  with. Deleting a whole test file is, again, a visible decision and not flagged.
+
+Advisory by default (prints the report, exit 0). ``--strict`` exits 1 on any finding so a
+refactor brief or a CI lane can make it a hard gate. Runs in ~30 s over the whole tree.
+
+Usage:
+    python scripts/ci/check_public_surface.py [--base origin/main] [--head HEAD] [--strict] [--json out.json]
+"""
+from __future__ import annotations
+
+import argparse
+import ast
+import json
+import re
+import subprocess
+import sys
+
+SRC_DIRS = ("agent", "gateway", "hermes_cli", "tools", "tui_gateway", "cron", "acp_adapter", "plugins")
+_TEST_DEF_RE = re.compile(r"^\s*(?:async\s+)?def test_", re.M)
+
+
+def _git(*args: str) -> str:
+    return subprocess.run(["git", *args], capture_output=True, text=True, encoding="utf-8", errors="replace").stdout
+
+
+def _changed_py(base: str, head: str) -> list[str]:
+    out = _git("diff", "--name-only", "--diff-filter=M", base, head)
+    return [f for f in out.split() if f.endswith(".py")]
+
+
+def _show(rev: str, path: str) -> str | None:
+    r = subprocess.run(["git", "show", f"{rev}:{path}"], capture_output=True, text=True, encoding="utf-8", errors="replace")
+    return r.stdout if r.returncode == 0 else None
+
+
+def public_toplevel(src: str) -> set[str] | None:
+    """Public top-level names: defs, classes, assigned names, and imported/re-exported names."""
+    try:
+        tree = ast.parse(src)
+    except SyntaxError:
+        return None
+    names: set[str] = set()
+    for node in tree.body:
+        if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)):
+            names.add(node.name)
+        elif isinstance(node, ast.Assign):
+            for target in node.targets:
+                names.update(x.id for x in ast.walk(target) if isinstance(x, ast.Name))
+        elif isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name):
+            names.add(node.target.id)
+        elif isinstance(node, (ast.Import, ast.ImportFrom)):
+            names.update((a.asname or a.name).split(".")[0] for a in node.names)
+    return {n for n in names if not n.startswith("_")}
+
+
+def _is_public_method(name: str) -> bool:
+    return not name.startswith("_") or name.startswith("__")
+
+
+def public_methods(src: str) -> set[str]:
+    """``Class.method`` for public and dunder methods REACHABLE on each top-level class: defined on it
+    or inherited from a base class defined in the same module. Extracting a method into a mixin/base
+    that the class still derives from is not a removal (the attribute still resolves)."""
+    try:
+        tree = ast.parse(src)
+    except SyntaxError:
+        return set()
+    classes = {n.name: n for n in tree.body if isinstance(n, ast.ClassDef)}
+
+    def own(cls: ast.ClassDef) -> set[str]:
+        return {m.name for m in cls.body if isinstance(m, (ast.FunctionDef, ast.AsyncFunctionDef)) and _is_public_method(m.name)}
+
+    def reachable(cls: ast.ClassDef, seen: set[str]) -> set[str]:
+        names = own(cls)
+        for base in cls.bases:
+            base_name = base.id if isinstance(base, ast.Name) else (base.attr if isinstance(base, ast.Attribute) else None)
+            if base_name in classes and base_name not in seen:
+                names |= reachable(classes[base_name], seen | {base_name})
+        return names
+
+    out: set[str] = set()
+    for name, cls in classes.items():
+        out.update(f"{name}.{m}" for m in reachable(cls, {name}))
+    return out
+
+
+def _is_source(path: str) -> bool:
+    return "/" not in path or path.split("/", 1)[0] in SRC_DIRS
+
+
+def diff_surface(base: str, head: str) -> dict:
+    dropped_names: dict[str, list[str]] = {}
+    dropped_methods: dict[str, list[str]] = {}
+    test_drops: dict[str, tuple[int, int]] = {}
+    for path in _changed_py(base, head):
+        before, after = _show(base, path), _show(head, path)
+        if before is None or after is None:
+            continue
+        if path.startswith("tests/"):
+            nb, na = len(_TEST_DEF_RE.findall(before)), len(_TEST_DEF_RE.findall(after))
+            if na < nb:
+                test_drops[path] = (nb, na)
+            continue
+        if not _is_source(path) or path.startswith("tests"):
+            continue
+        pb, pa = public_toplevel(before), public_toplevel(after)
+        if pb is not None and pa is not None and (gone := sorted(pb - pa)):
+            dropped_names[path] = gone
+        if gone_m := sorted(public_methods(before) - public_methods(after)):
+            dropped_methods[path] = gone_m
+    return {"dropped_names": dropped_names, "dropped_methods": dropped_methods, "test_drops": test_drops}
+
+
+def render(report: dict) -> str:
+    lines: list[str] = []
+    n_names = sum(len(v) for v in report["dropped_names"].values())
+    n_meth = sum(len(v) for v in report["dropped_methods"].values())
+    n_tests = sum(b - a for b, a in report["test_drops"].values())
+    lines.append(
+        f"public-surface: {n_names} public top-level name(s) dropped in {len(report['dropped_names'])} module(s); "
+        f"{n_meth} public/dunder method(s) dropped in {len(report['dropped_methods'])}; "
+        f"{n_tests} test def(s) dropped in {len(report['test_drops'])} file(s)"
+    )
+    for path, names in sorted(report["dropped_names"].items()):
+        lines.append(f"  {path}: -{', '.join(names[:12])}{' …' if len(names) > 12 else ''}")
+    for path, names in sorted(report["dropped_methods"].items()):
+        lines.append(f"  {path}: -{', '.join(names[:8])}{' …' if len(names) > 8 else ''}")
+    for path, (b, a) in sorted(report["test_drops"].items()):
+        lines.append(f"  {path}: {b} -> {a} test defs")
+    if n_names or n_meth or n_tests:
+        lines.append(
+            "  A public name may be moved and re-exported or deprecated, never silently removed (in-tree refs say "
+            "nothing about plugins); a test file must not lose test defs unless the commit names the private symbol "
+            "they pinned."
+        )
+    return "\n".join(lines)
+
+
+def main(argv: list[str] | None = None) -> int:
+    ap = argparse.ArgumentParser(description=(__doc__ or "").split("\n\n")[0])
+    ap.add_argument("--base", default="origin/main")
+    ap.add_argument("--head", default="HEAD")
+    ap.add_argument("--strict", action="store_true", help="exit 1 on any finding")
+    ap.add_argument("--json", dest="json_out", help="also write the report as JSON")
+    args = ap.parse_args(argv)
+    for ref in (args.base, args.head):
+        if subprocess.run(["git", "rev-parse", "--verify", "--quiet", f"{ref}^{{commit}}"], capture_output=True).returncode != 0:
+            print(f"public-surface: cannot resolve ref {ref!r} (fetch it first); refusing to report a clean diff", file=sys.stderr)
+            return 2
+    base = _git("merge-base", args.base, args.head).strip()
+    if not base:
+        print(f"public-surface: no merge-base between {args.base!r} and {args.head!r}", file=sys.stderr)
+        return 2
+    report = diff_surface(base, args.head)
+    print(render(report))
+    if args.json_out:
+        with open(args.json_out, "w", encoding="utf-8") as fh:
+            json.dump(report, fh, indent=1)
+    findings = any(report[k] for k in ("dropped_names", "dropped_methods", "test_drops"))
+    return 1 if (args.strict and findings) else 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

scripts/ci/classify_changes.py

@@ -67,6 +67,8 @@ import subprocess
 import sys
 
 _FRONTEND = ("ui-tui/", "web/", "apps/")  # TS typecheck-matrix packages
+# Shipped page outside those packages, exercised by the desktop Electron suite.
+_FRONTEND_FILES = {"scripts/desktop-update/ui.html"}
 _ROOT_NPM = {"package.json", "package-lock.json"}  # shifts every package's tree
 _DOCKER_META = ("docker/", ".hadolint.yml", "Dockerfile") # docker setup
 _NIX_PATHS = ("nix/",) # nix files
@@ -214,7 +216,10 @@ def classify(files: list[str]) -> dict[str, bool]:
     files = [f.strip() for f in files if f.strip()]
     python = any(not _py_irrelevant(f) for f in files)
     python_prod = any(not _py_irrelevant(f) and not _py_test_only(f) for f in files)
-    frontend = any(f.startswith(_FRONTEND) or f in _ROOT_NPM for f in files)
+    frontend = any(
+        f.startswith(_FRONTEND) or f in _ROOT_NPM or f in _FRONTEND_FILES
+        for f in files
+    )
     deps = any(f == "pyproject.toml" for f in files)
     npm_lock = any(f.split("/")[-1] == "package-lock.json" for f in files)
     docker_meta = any(f.startswith(_DOCKER_META) for f in files)

scripts/desktop-update/ui.html

@@ -99,8 +99,8 @@
     font-size: 11px;
     color: var(--foreground);
   }
-  body.done #loader, body.error #loader { display: none; }
-  body.done #glyph, body.error #glyph { display: flex; }
+  body.done #loader, body.error #loader, body.disconnected #loader { display: none; }
+  body.done #glyph, body.error #glyph, body.disconnected #glyph { display: flex; }
 </style>
 </head>
 <body>
@@ -205,6 +205,7 @@
   const glyphEl = document.getElementById('glyph')
   const defaultLine = lineEl.textContent   /* what a stage-less run says */
   let settled = false
+  let failures = 0
 
   const elapsedText = s =>
     s < 60 ? `${s}s elapsed` : `${Math.floor(s / 60)}m ${s % 60}s elapsed`
@@ -226,7 +227,8 @@
     } else if (state.status === 'done') {
       settle('done')
       glyphEl.textContent = '\u2713'
-      lineEl.textContent = 'Opening Hermes\u2026'
+      titleEl.textContent = 'Update complete'
+      lineEl.textContent = 'Opening Hermes\u2026\nYou can close this window.'
     } else if (state.status === 'manual') {
       // Update landed but Hermes will NOT reopen itself (package skew,
       // sandbox helper, launch rejected). The orchestrator leaves this
@@ -243,13 +245,43 @@
     }
   }
 
+  async function request(url, options = {}) {
+    const controller = new AbortController()
+    const timeout = setTimeout(() => controller.abort(), 5000)
+    try {
+      const response = await fetch(url, { ...options, cache: 'no-store', signal: controller.signal })
+      if (!response.ok) throw new Error('Progress request failed')
+      // Keep the deadline active while reading the body too: receiving
+      // headers alone does not prove that the progress server is responsive.
+      return options.method === 'POST' ? null : await response.json()
+    } finally {
+      clearTimeout(timeout)
+    }
+  }
+
   async function poll() {
     try {
-      const res = await fetch('/progress', { cache: 'no-store' })
-      if (res.ok) apply(await res.json())
+      const state = await request('/progress')
+      if (!state || !['running', 'done', 'manual', 'error'].includes(state.status)) {
+        throw new Error('Invalid progress response')
+      }
+      failures = 0
+      apply(state)
+      // The Windows server can now wait for delivery instead of guessing
+      // that one polling interval was enough. Older/POSIX servers omit this
+      // receipt. Apply first so a failed acknowledgement cannot hide a result.
+      if (settled && typeof state.receipt === 'string' && state.receipt) {
+        await request(`/ack/${encodeURIComponent(state.receipt)}`, { method: 'POST' })
+      }
     } catch {
-      // Server gone: hold the last known state. The orchestrator owns
-      // closing this window; the relaunched Desktop owns the result.
+      if (!settled && ++failures >= 3) {
+        // A vanished server is not evidence of success OR failure. Leave an
+        // honest, finite state even if the browser refuses the close request.
+        settle('disconnected')
+        glyphEl.textContent = '!'
+        titleEl.textContent = 'Update status unavailable'
+        lineEl.textContent = 'The progress connection was lost.\nCheck Hermes for the update result. You can close this window.'
+      }
     }
     if (!settled) setTimeout(poll, 400)
   }

scripts/desktop-update/windows.ps1

@@ -108,6 +108,8 @@ $script:UiState = [hashtable]::Synchronized(@{
     status     = "running"      # running | done | manual | error
     message    = $script:UiStage
     clock      = $script:UiStopwatch
+    receipt    = $null
+    acknowledged_receipt = $null
 })
 $script:UiServer = $null     # @{ Listener; Runspace; PowerShell; Port; BrowserProc; Profile }
 
@@ -190,15 +192,26 @@ function Start-UiServer([string]$HtmlPath) {
                     $request = $reader.ReadLine()
                     # Drain headers so the client doesn't see a reset mid-send.
                     while ($true) { $h = $reader.ReadLine(); if ($null -eq $h -or $h -eq "") { break } }
-                    if ($request -match "^GET /progress") {
+                    if ($request -match "^GET /progress HTTP/1\.[01]$") {
                         $elapsed = [Math]::Floor($State.clock.Elapsed.TotalSeconds)
                         $snapshot = @{
                             status          = $State.status
                             message         = $State.message
                             elapsed_seconds = $elapsed
+                            receipt         = $State.receipt
                         } | ConvertTo-Json -Compress
                         Send-Response $stream "200 OK" "application/json; charset=utf-8" ([System.Text.Encoding]::UTF8.GetBytes($snapshot))
-                    } elseif ($request -match "^GET / ") {
+                    } elseif ($request -match "^POST /ack/([^ /?]+) HTTP/1\.[01]$") {
+                        $receipt = $Matches[1]
+                        if ($State.status -in @("done", "manual", "error") -and $State.receipt -and $receipt -ceq $State.receipt) {
+                            # Flush acceptance before waking the owner that will
+                            # close the listener. No request body is needed.
+                            Send-Response $stream "204 No Content" "text/plain" ([byte[]]@())
+                            $State.acknowledged_receipt = $receipt
+                        } else {
+                            Send-Response $stream "409 Conflict" "text/plain" ([System.Text.Encoding]::ASCII.GetBytes("unknown terminal receipt"))
+                        }
+                    } elseif ($request -match "^GET / HTTP/1\.[01]$") {
                         Send-Response $stream "200 OK" "text/html; charset=utf-8" $HtmlBytes
                     } else {
                         Send-Response $stream "404 Not Found" "text/plain" ([System.Text.Encoding]::ASCII.GetBytes("not found"))
@@ -283,11 +296,25 @@ function Stop-UiServer([switch]$LeaveWindow) {
 }
 
 function Publish-UiEvent([string]$Status, [string]$Message) {
-    # The event the shim listens for. One beat of poll latency (400ms) before
-    # teardown so the page actually renders the terminal state.
+    # A background browser can miss a fixed 900ms delivery window. Retain the
+    # terminal event until the page acknowledges applying this exact receipt.
+    # Older/headless clients cannot acknowledge, so teardown remains bounded.
+    $receipt = [Guid]::NewGuid().ToString('N')
+    $script:UiState.receipt = $receipt
+    $script:UiState.acknowledged_receipt = $null
     $script:UiState.message = $Message
     $script:UiState.status = $Status
-    if ($script:UiServer) { Start-Sleep -Milliseconds 900 }
+    if ($script:UiServer) {
+        $deliveryWait = [System.Diagnostics.Stopwatch]::StartNew()
+        while ($script:UiState.acknowledged_receipt -cne $receipt -and $deliveryWait.Elapsed.TotalSeconds -lt 10) {
+            Start-Sleep -Milliseconds 50
+        }
+        if ($script:UiState.acknowledged_receipt -ceq $receipt) {
+            Write-HandoffLog "shim: terminal state '$Status' acknowledged by the window"
+        } else {
+            Write-HandoffLog "shim: terminal state '$Status' was not acknowledged within 10s; closing the progress server"
+        }
+    }
 }
 
 function Get-UiElapsedText {
@@ -309,6 +336,8 @@ function Publish-UiProgress([string]$Message) {
     $script:UiStage = $Message
     $script:UiState.message = $Message
     $script:UiState.status = "running"
+    $script:UiState.receipt = $null
+    $script:UiState.acknowledged_receipt = $null
     if ($script:Ui) {
         try {
             $script:Ui.Sub.Text = Get-UiProgressLine