fix(external-wait): normalize short head SHAs at registration, heal legacy records (#961)

bridge/core/external_wait.py

@@ -301,6 +301,33 @@ def _do(records):
 
         self._mutate(_do)
 
+    def correct_head_sha(self, wait_id: str, full_sha: str) -> bool:
+        """Heal a legacy short-SHA registration to the full 40-char head.
+
+        Registration now normalizes short SHAs to the full head via ``gh``
+        (#961), but records written before that fix may hold 7-39 hex chars
+        that can never equal GitHub's 40-char ``headRefOid``. The monitor
+        calls this when the live head *starts with* the recorded short form;
+        a genuine head change still supersedes. Returns True only when the
+        record was monitoring and actually healed.
+        """
+        full_sha = validate_head_sha(full_sha)
+        if len(full_sha) != 40:
+            return False
+
+        def _do(records):
+            rec = records.get(wait_id)
+            if rec is None or rec.get("state") != STATE_MONITORING:
+                return False
+            recorded = str(rec.get("head_sha") or "")
+            if not recorded or len(recorded) >= 40 or not full_sha.startswith(recorded):
+                return False
+            rec["head_sha"] = full_sha
+            rec["updated_at"] = _utc_now_iso()
+            return True
+
+        return bool(self._mutate(_do))
+
     def finish(self, wait_id: str, terminal_status: str, *, now: Optional[float] = None) -> bool:
         """Terminal transition, journaled before any wake. First write wins.
 

bridge/core/external_wait_cli.py

@@ -23,6 +23,7 @@
 
 import json
 import os
+import subprocess
 import sys
 from pathlib import Path
 from typing import Any, Optional, Sequence
@@ -59,6 +60,47 @@ def _emit(payload: dict[str, Any]) -> None:
     print(json.dumps(payload, ensure_ascii=False, separators=(",", ":")))
 
 
+def resolve_full_head_sha(repo: str, head_sha: str) -> str:
+    """Normalize a 7-39 char short SHA to the full 40-char head (#961).
+
+    The monitor compares the recorded SHA against GitHub's 40-char
+    ``headRefOid`` with exact equality, so a short SHA that passes format
+    validation can never match — registration would return ``ok`` for a wait
+    that supersedes on the first poll (the #949 silent promise loss). Resolve
+    through ``gh`` at registration; when the short SHA does not resolve to a
+    commit in the repo, registration fails closed instead of promising a
+    watch that can never fire. Full 40-char SHAs pass through untouched.
+    """
+    if len(head_sha) == 40:
+        return head_sha
+    try:
+        proc = subprocess.run(
+            ["gh", "api", f"repos/{repo}/commits/{head_sha}", "--jq", ".sha"],
+            capture_output=True,
+            timeout=30,
+        )
+    except (OSError, subprocess.TimeoutExpired):
+        raise ExternalWaitValidationError(
+            "short head SHA could not be resolved via gh (unavailable or timeout); "
+            "pass the full 40-char SHA"
+        )
+    if proc.returncode != 0:
+        raise ExternalWaitValidationError(
+            "short head SHA does not resolve to a commit in the repo; "
+            "pass the full 40-char SHA from the PR head"
+        )
+    full = proc.stdout.decode("utf-8", "replace").strip().lower()
+    try:
+        full = validate_head_sha(full)
+    except ExternalWaitValidationError:
+        full = ""
+    if len(full) != 40 or not full.startswith(head_sha):
+        raise ExternalWaitValidationError(
+            "head SHA resolution returned an unexpected value; registration refused"
+        )
+    return full
+
+
 def _parse_args(argv: Sequence[str]) -> dict[str, Any]:
     """Tiny flag parser (no argparse dependency games in hook contexts)."""
     args: dict[str, Any] = {"_": []}
@@ -92,7 +134,7 @@ def _cmd_register(home: Path, args: dict[str, Any]) -> int:
     try:
         repo = validate_repo(str(args.get("repo", "")))
         pr_number = validate_pr_number(args.get("pr"))
-        head_sha = validate_head_sha(str(args.get("head_sha", "")))
+        head_sha = resolve_full_head_sha(repo, validate_head_sha(str(args.get("head_sha", ""))))
         summary = validate_summary(args.get("summary"))
         timeout = float(args.get("timeout_seconds", DEFAULT_TIMEOUT_SECONDS))
     except ExternalWaitValidationError as exc:

bridge/core/external_wait_monitor.py

@@ -301,10 +301,18 @@ async def _poll_one(self, record: Dict[str, Any]) -> None:
                 self._reschedule(record)
             return
         self._transport_errors.pop(wait_id, None)
-        if state.head_sha != record.get("head_sha"):
-            # The PR moved on: never report the stale run as the watched one.
-            self._registry.finish(wait_id, TERMINAL_SUPERSEDED, now=now)
-            return
+        recorded = str(record.get("head_sha") or "")
+        if state.head_sha != recorded:
+            if recorded and len(recorded) < 40 and state.head_sha.startswith(recorded):
+                # Legacy short-SHA registration watching the same head: heal
+                # the record to the full SHA instead of dropping the promise
+                # as superseded (#961). A genuinely moved head still ends the
+                # wait below.
+                self._registry.correct_head_sha(wait_id, state.head_sha)
+            else:
+                # The PR moved on: never report the stale run as the watched one.
+                self._registry.finish(wait_id, TERMINAL_SUPERSEDED, now=now)
+                return
         terminal = _TERMINAL_BY_ROLLUP.get(state.rollup)
         if terminal is not None:
             self._registry.finish(wait_id, terminal, now=now)

bridge/tests/test_external_wait_cli.py

@@ -16,11 +16,25 @@
 from telegram_bot.core import external_wait_cli
 from telegram_bot.core.external_wait import (
     ExternalWaitRegistry,
+    ExternalWaitValidationError,
     default_registry_path,
     publish_active_turn,
 )
 
 
+_REAL_RESOLVE = external_wait_cli.resolve_full_head_sha
+
+
+@pytest.fixture(autouse=True)
+def _stub_sha_resolution(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Keep ``gh`` out of unit tests: expand short SHAs deterministically."""
+
+    def _fake(repo: str, head_sha: str) -> str:
+        return head_sha if len(head_sha) == 40 else head_sha.ljust(40, "0")
+
+    monkeypatch.setattr(external_wait_cli, "resolve_full_head_sha", _fake)
+
+
 @pytest.fixture
 def home(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
     monkeypatch.setenv("CCC_EXTERNAL_WAIT_HOME", str(tmp_path))
@@ -81,6 +95,55 @@ def test_register_is_idempotent_for_the_same_natural_key(
     assert first["wait_id"] == second["wait_id"]
 
 
+def test_register_normalizes_a_short_sha_to_the_full_head(
+    home: Path, capsys: pytest.CaptureFixture
+) -> None:
+    _publish(home)
+
+    rc = external_wait_cli.main(_register_args(head_sha="a8ac2475"))
+
+    assert rc == 0
+    payload = json.loads(capsys.readouterr().out.strip())
+    # Regression (#961/#949): a short SHA must never be stored as-is — the
+    # monitor compares against GitHub's 40-char headRefOid with exact
+    # equality, so a raw short SHA supersedes on the first poll and the
+    # "CI finishes -> auto-resume" promise is silently dropped.
+    assert len(payload["head_sha"]) == 40
+    record = ExternalWaitRegistry(default_registry_path(home)).get(payload["wait_id"])
+    assert record["head_sha"] == payload["head_sha"]
+    assert record["head_sha"].startswith("a8ac2475")
+
+
+def test_register_fails_closed_when_the_short_sha_does_not_resolve(
+    home: Path, capsys: pytest.CaptureFixture, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    _publish(home)
+
+    def _unresolvable(repo: str, head_sha: str) -> str:
+        raise ExternalWaitValidationError("short head SHA does not resolve to a commit")
+
+    monkeypatch.setattr(external_wait_cli, "resolve_full_head_sha", _unresolvable)
+
+    rc = external_wait_cli.main(_register_args(head_sha="deadbeef"))
+
+    assert rc == 2
+    payload = json.loads(capsys.readouterr().out.strip())
+    assert payload["ok"] is False
+    assert payload["code"] == "validation"
+    # The honest failure must not leave a watch that can never fire.
+    assert ExternalWaitRegistry(default_registry_path(home)).records() == []
+
+
+def test_full_sha_passes_through_without_gh(monkeypatch: pytest.MonkeyPatch) -> None:
+    full = "a8ac2475" * 5
+
+    def _boom(*args, **kwargs):
+        raise AssertionError("gh must not run for an already-full SHA")
+
+    monkeypatch.setattr(external_wait_cli.subprocess, "run", _boom)
+    assert _REAL_RESOLVE("jinwon-int/ccc-node", full) == full
+
+
 def test_register_fails_closed_without_an_active_route(
     home: Path, capsys: pytest.CaptureFixture
 ) -> None:

bridge/tests/test_external_wait_monitor.py

@@ -178,6 +178,47 @@ async def test_failure_and_cancelled_are_terminal_results(tmp_path: Path) -> Non
         assert headline in recorder.notifications[0][1]
 
 
+@pytest.mark.anyio
+async def test_legacy_short_sha_heals_instead_of_superseding(tmp_path: Path) -> None:
+    # Regression (#961): a record written with a 7-char SHA watches the same
+    # head whose headRefOid merely *extends* it — that is not a moved head.
+    clock = Clock()
+    registry = _registry(tmp_path, clock)  # seeds head_sha "abc1234" (7 chars)
+    full = "abc1234" + "f" * 33
+    transport = FakeTransport([PrState(full, "pending"), PrState(full, "success")])
+    recorder = Recorder()
+    monitor = _monitor(registry, transport, recorder, clock)
+
+    await monitor._tick()
+
+    record = registry.records()[0]
+    assert record["state"] == "monitoring"
+    assert record["head_sha"] == full
+
+    clock.advance(120)
+    await monitor._tick()
+
+    record = registry.records()[0]
+    assert record["terminal_status"] == TERMINAL_SUCCESS
+    assert len(recorder.notifications) == 1
+
+
+@pytest.mark.anyio
+async def test_short_sha_prefix_of_a_different_head_still_supersedes(tmp_path: Path) -> None:
+    # Same length, different content: not the watched head, so the wait ends.
+    clock = Clock()
+    registry = _registry(tmp_path, clock)
+    transport = FakeTransport([PrState("abc9999" + "f" * 33, "success")])
+    recorder = Recorder()
+    monitor = _monitor(registry, transport, recorder, clock)
+
+    await monitor._tick()
+
+    record = registry.records()[0]
+    assert record["terminal_status"] == TERMINAL_SUPERSEDED
+    assert recorder.resumes == []
+
+
 @pytest.mark.anyio
 async def test_head_sha_mismatch_is_superseded_never_success(tmp_path: Path) -> None:
     clock = Clock()