fix(auth): a transient gh probe failure no longer looks like being signed out (ext 0.19.10)

README.md

@@ -578,7 +578,9 @@ Every write verb the VS Code extension drives runs **without a TTY** — explici
 
 `list-open-issues --repo=<owner/name> [--exclude=<csv>]` is a second viewer read surface: it emits a repo's **open** issues as JSON (`{repo, issues:[…]}`, the same per-issue shape as `export`). The extension's **Slot** command uses it to offer a pick-list instead of a typed number; `--exclude` drops the track's current issues so they don't reappear. Unlike `export`'s `untracked` (open issues in *no* track), this includes issues tracked by *other* tracks — they're valid slot targets. Read-only.
 
-`auth-status [--json]` is the viewer's **auth probe**: it runs `gh auth status` and emits `{gh_present, authenticated, user, error}` (exit `0` authenticated / `1` gh present but not signed in / `2` gh not found). Because every GitHub read goes through `gh` and the fetch helpers return empty rather than erroring, an unauthenticated session would otherwise look like an empty-but-working one. The extension calls this at activation (and after every refresh) to **fast-fail with a clear "Not signed in to GitHub" banner + a Sign in path** instead of rendering a misleadingly empty tree — and distinguishes "not signed in" (`gh auth login`) from "gh not installed" (a different fix). Read-only.
+`auth-status [--json]` is the viewer's **auth probe**: it runs `gh auth status` and emits `{gh_present, authenticated, probe_ok, user, error}` (exit `0` authenticated / `1` gh present but not signed in / `2` gh not found / `3` the probe couldn't reach a verdict). Because every GitHub read goes through `gh` and the fetch helpers return empty rather than erroring, an unauthenticated session would otherwise look like an empty-but-working one. The extension calls this at activation (and after every refresh) to **fast-fail with a clear "Not signed in to GitHub" banner + a Sign in path** instead of rendering a misleadingly empty tree — and distinguishes "not signed in" (`gh auth login`) from "gh not installed" (a different fix). Read-only.
+
+**`probe_ok` is the trust flag** — and the reason this is more than an exit-code check. `gh auth status` performs a *live* token-validation request, so a momentary network failure (a waking laptop, a reconnecting VPN) exits non-zero and actively misreports the cause as `The token in keyring is invalid.` It also exits non-zero when *any* configured account fails validation, so one stale second account would otherwise read as a logout of a healthy active one. Only two verdicts are therefore treated as authoritative (`probe_ok: true`): a clean exit `0`, and the explicit "not logged into any GitHub hosts" message. Everything else — a validation failure, a timeout, an unrecognised exit — is `probe_ok: false`, meaning *unverified, not signed out*; callers keep whatever state they last had rather than prompting for a sign-in that isn't needed.
 
 `plan-status --json` is the viewer's **Plans view** read surface: alongside each doc's verdict it now also emits `manifest_last_touched` (the most recent commit date across the plan's declared files), `stalled`, `lie_gap`, `unchecked_items`, and `stall_days`. The staleness window honors `stall_days:` in `~/.claude/work-plan/config.yml` and a `--stall-days=<n>` flag (precedence: flag → config → default 14). The viewer consumes these to flag plans whose declared-file build has gone cold — a `partial` plan with no recent commit on its manifest ("stalled") — and plans scored shipped whose own phase checkboxes are mostly unticked ("lie-gap"). It also emits `override` (the human `verdict_override`, `shipped`/`partial`/`dead` or `null`, set via `plan-confirm` — when present the CLI pins the verdict to it and forces `lie_gap` false), `acknowledged` (the durable frontmatter ack set via `plan-ack`), and `verdict_baseline` + `verdict_drift` (the drift tripwire set via `plan-baseline` — `verdict_drift` is true when the live verdict no longer matches the stamped baseline, suppressed under an override). It also emits `offtree_paths`: declared manifest paths that resolve **outside** the repo (absolute, `~`, `..`-escape, junk `/`) — a read-only flag for a typo or misfiled plan that would otherwise silently drag the file score down (the 🧳 foreign verdict only fires when *most* paths are off-tree; this surfaces the sub-threshold ones too). Never auto-fixed — surfacing only.
 

skills/work-plan/commands/auth_status.py

@@ -8,7 +8,9 @@
 
 Read-only; never mutates anything. Exit code mirrors auth state so a shell caller
 can gate on it: 0 = authenticated, 1 = gh present but not logged in, 2 = gh not
-found.
+found, 3 = the probe couldn't reach a verdict (#485 — a live-network validation
+failure or a timeout, which must NOT be reported as a logout). `rc == 0` remains
+the "am I usable" gate; 3 is additive detail for callers that care why not.
 """
 import json
 
@@ -20,16 +22,30 @@ def run(args: list) -> int:
     flags, _ = parse_flags(args, {"--json"})
     status = github_state.gh_auth_status()
 
+    # Absent probe_ok (an older/hand-built status dict) means "trust the verdict",
+    # preserving the historical two-way split.
+    probe_ok = status.get("probe_ok", True)
+
     if flags.get("--json"):
         print(json.dumps(status))
     elif status["authenticated"]:
         who = f" as {status['user']}" if status.get("user") else ""
         print(f"✓ Authenticated to GitHub{who}.")
     elif not status["gh_present"]:
         print("✗ GitHub CLI (gh) not found on PATH. Install it: https://cli.github.com")
+    elif not probe_ok:
+        # The probe failed to reach a verdict — do NOT send a signed-in user
+        # through a pointless sign-in flow (#485). Relay gh's own words; they
+        # carry the real remediation (wait for the network, or `gh auth refresh`).
+        print("? Couldn't verify GitHub sign-in — the probe didn't reach a verdict.")
+        print("  This is usually a transient network failure, not a logout.")
+        if status.get("error"):
+            print(f"  gh said: {status['error']}")
     else:
         print("✗ Not logged in to GitHub. Run: gh auth login")
 
     if status["authenticated"]:
         return 0
-    return 2 if not status["gh_present"] else 1
+    if not status["gh_present"]:
+        return 2
+    return 1 if probe_ok else 3

skills/work-plan/lib/github_state.py

@@ -87,41 +87,67 @@ def set_issue_in_progress(repo: str, number: int, clear: bool = False) -> tuple:
     return (True, (proc.stdout or f"{verb} #{number} in-progress").strip())
 
 
+# `gh auth status` says this — and only this — when no credentials are
+# configured at all. Every OTHER non-zero exit means "gh has credentials but
+# couldn't confirm them", which is NOT a logout (#485). Both the current and the
+# legacy phrasings begin the same way.
+_NO_CREDENTIALS_RE = re.compile(r"not logged in ?to any (?:GitHub )?hosts?", re.I)
+
+
 def gh_auth_status() -> dict:
     """Probe `gh` authentication so callers can fast-fail instead of silently
     degrading (#auth). Returns:
 
-        {"gh_present": bool, "authenticated": bool,
+        {"gh_present": bool, "authenticated": bool, "probe_ok": bool,
          "user": str | None, "error": str | None}
 
-    Distinguishes the two failure modes the UI must handle differently:
+    Distinguishes the failure modes the UI must handle differently:
     `gh` not installed (`gh_present` False — fix is "install gh") vs installed
-    but not logged in (`authenticated` False — fix is "gh auth login").
-
-    Never raises. `gh auth status` exits 0 when at least one host is logged in,
-    non-zero otherwise; it prints the human status to STDERR. We parse a
-    best-effort `user` from that text but treat the EXIT CODE as authoritative."""
+    but not logged in (`authenticated` False — fix is "gh auth login") vs the
+    probe never reaching a verdict at all (`probe_ok` False — fix is "wait /
+    retry", NOT a sign-in prompt).
+
+    `probe_ok` is the trust flag, and it is the whole reason this function is
+    more than an exit-code check (#485). `gh auth status` performs a LIVE
+    token-validation request, so a momentary network failure — waking from
+    sleep, a reconnecting VPN — exits non-zero and actively misreports the cause
+    as "The token in keyring is invalid." Treating that as a logout wipes the
+    viewer's tree and tells an already-signed-in user to sign in again. It also
+    exits non-zero when ANY configured account fails validation, so one stale
+    second account would otherwise read as a logout of a healthy active one.
+
+    So only two verdicts are authoritative: exit 0 (signed in), and the explicit
+    "not logged into any hosts" message (signed out). Everything else — a
+    validation failure, a timeout, an OS error, an exit code we don't recognise
+    — is indeterminate, and callers should keep whatever state they last had.
+
+    Never raises. `gh` prints its human status to STDERR; we parse a best-effort
+    `user` from that text."""
     try:
         proc = subprocess.run(
             ["gh", "auth", "status"],
             capture_output=True, text=True, timeout=GH_TIMEOUT,
         )
     except FileNotFoundError:
-        return {"gh_present": False, "authenticated": False,
+        # Authoritative: the binary genuinely isn't there.
+        return {"gh_present": False, "authenticated": False, "probe_ok": True,
                 "user": None, "error": "gh CLI not found on PATH"}
     except Exception as e:  # timeout / OS error — gh present but unusable now
-        return {"gh_present": True, "authenticated": False,
+        return {"gh_present": True, "authenticated": False, "probe_ok": False,
                 "user": None, "error": f"gh auth status failed: {e}"}
 
     blob = f"{proc.stdout}\n{proc.stderr}"
     authenticated = proc.returncode == 0
+    # Authoritative only for a clean success or an explicit no-credentials
+    # message. Anything else is a probe that failed to reach a verdict.
+    probe_ok = authenticated or bool(_NO_CREDENTIALS_RE.search(blob))
     # `gh auth status` prints e.g. "✓ Logged in to github.com account USER" or
     # the older "Logged in to github.com as USER". Match either phrasing.
     m = re.search(r"Logged in to \S+ (?:account|as) (\S+)", blob)
     user = m.group(1) if (authenticated and m) else None
     error = None if authenticated else (blob.strip() or "not logged in to GitHub")
     return {"gh_present": True, "authenticated": authenticated,
-            "user": user, "error": error}
+            "probe_ok": probe_ok, "user": user, "error": error}
 
 
 def fetch_issue(repo: str, number: int) -> Optional[dict]:

skills/work-plan/tests/test_auth_status.py

@@ -27,6 +27,7 @@ def test_authenticated_parses_user(self):
             s = github_state.gh_auth_status()
         self.assertTrue(s["authenticated"])
         self.assertTrue(s["gh_present"])
+        self.assertTrue(s["probe_ok"])
         self.assertEqual(s["user"], "evemcgivern")
         self.assertIsNone(s["error"])
 
@@ -43,6 +44,7 @@ def test_not_logged_in(self):
             s = github_state.gh_auth_status()
         self.assertFalse(s["authenticated"])
         self.assertTrue(s["gh_present"])      # gh ran, just not logged in
+        self.assertTrue(s["probe_ok"])        # authoritative: no credentials at all
         self.assertIsNone(s["user"])
         self.assertIn("not logged", s["error"].lower())
 
@@ -51,14 +53,74 @@ def test_gh_not_installed(self):
             s = github_state.gh_auth_status()
         self.assertFalse(s["gh_present"])
         self.assertFalse(s["authenticated"])
+        self.assertTrue(s["probe_ok"])        # authoritative: gh is genuinely absent
         self.assertIn("not found", s["error"].lower())
 
-    def test_timeout_is_present_but_unauthenticated(self):
+
+class GhAuthStatusIndeterminateTest(unittest.TestCase):
+    """#485 — a probe that could not REACH a verdict must never be reported as a
+    logout. `gh auth status` makes a live token-validation call, so a network
+    blip (waking from sleep, VPN reconnecting) exits non-zero and claims the
+    keyring token is invalid. Treating that as "signed out" wipes the viewer's
+    tree and shows a sign-in banner to an already-signed-in user."""
+
+    # Verbatim `gh auth status` output with the network unreachable, while the
+    # keyring token is in fact perfectly valid.
+    NETWORK_BLIP = (
+        "github.com\n"
+        "  X Failed to log in to github.com account evemcgivern (keyring)\n"
+        "  - Active account: true\n"
+        "  - The token in keyring is invalid.\n"
+        "  - To re-authenticate, run: gh auth refresh -h github.com\n"
+    )
+
+    def test_validation_failure_is_indeterminate_not_logged_out(self):
+        out = _proc(1, stderr=self.NETWORK_BLIP)
+        with mock.patch("lib.github_state.subprocess.run", return_value=out):
+            s = github_state.gh_auth_status()
+        self.assertFalse(s["authenticated"])
+        self.assertTrue(s["gh_present"])
+        self.assertFalse(s["probe_ok"])       # the whole point: NOT authoritative
+        self.assertIn("token in keyring is invalid", s["error"])
+
+    def test_timeout_is_indeterminate(self):
         with mock.patch("lib.github_state.subprocess.run",
                         side_effect=subprocess.TimeoutExpired("gh", 30)):
             s = github_state.gh_auth_status()
         self.assertTrue(s["gh_present"])
         self.assertFalse(s["authenticated"])
+        self.assertFalse(s["probe_ok"])
+
+    def test_os_error_is_indeterminate(self):
+        with mock.patch("lib.github_state.subprocess.run",
+                        side_effect=OSError("resource temporarily unavailable")):
+            s = github_state.gh_auth_status()
+        self.assertTrue(s["gh_present"])
+        self.assertFalse(s["authenticated"])
+        self.assertFalse(s["probe_ok"])
+
+    def test_unrecognised_failure_defaults_to_indeterminate(self):
+        """Unknown non-zero exits stay indeterminate. Keeping a stale tree is a
+        far cheaper error than falsely telling a signed-in user to sign in."""
+        out = _proc(1, stderr="error connecting to api.github.com")
+        with mock.patch("lib.github_state.subprocess.run", return_value=out):
+            s = github_state.gh_auth_status()
+        self.assertFalse(s["authenticated"])
+        self.assertFalse(s["probe_ok"])
+
+    def test_one_broken_account_alongside_a_healthy_one_is_indeterminate(self):
+        """gh exits non-zero if ANY configured account fails validation, so a
+        stale second account must not read as a logout of the active one."""
+        out = _proc(1, stderr=(
+            "github.com\n"
+            "  ✓ Logged in to github.com account evemcgivern (keyring)\n"
+            "  - Active account: true\n"
+            "  X Failed to log in to github.com account eve-mcgivern (keyring)\n"
+            "  - The token in keyring is invalid.\n"
+        ))
+        with mock.patch("lib.github_state.subprocess.run", return_value=out):
+            s = github_state.gh_auth_status()
+        self.assertFalse(s["probe_ok"])
 
 
 class AuthStatusCommandTest(unittest.TestCase):
@@ -70,29 +132,58 @@ def _run(self, status, args):
         return rc, buf.getvalue()
 
     def test_json_authenticated_exit_0(self):
-        status = {"gh_present": True, "authenticated": True, "user": "eve", "error": None}
+        status = {"gh_present": True, "authenticated": True, "probe_ok": True,
+                  "user": "eve", "error": None}
         rc, out = self._run(status, ["--json"])
         self.assertEqual(rc, 0)
         self.assertEqual(json.loads(out), status)
 
     def test_not_logged_in_exit_1(self):
-        status = {"gh_present": True, "authenticated": False, "user": None, "error": "x"}
+        status = {"gh_present": True, "authenticated": False, "probe_ok": True,
+                  "user": None, "error": "x"}
         rc, out = self._run(status, [])
         self.assertEqual(rc, 1)
         self.assertIn("gh auth login", out)
 
     def test_gh_missing_exit_2(self):
-        status = {"gh_present": False, "authenticated": False, "user": None, "error": "x"}
+        status = {"gh_present": False, "authenticated": False, "probe_ok": True,
+                  "user": None, "error": "x"}
         rc, out = self._run(status, [])
         self.assertEqual(rc, 2)
         self.assertIn("not found", out.lower())
 
     def test_human_authenticated_names_user(self):
-        status = {"gh_present": True, "authenticated": True, "user": "eve", "error": None}
+        status = {"gh_present": True, "authenticated": True, "probe_ok": True,
+                  "user": "eve", "error": None}
         rc, out = self._run(status, [])
         self.assertEqual(rc, 0)
         self.assertIn("eve", out)
 
+    def test_indeterminate_exit_3_does_not_say_not_logged_in(self):
+        """#485 — the terminal output must not tell a signed-in user to run
+        `gh auth login` when the probe merely failed to reach a verdict."""
+        status = {"gh_present": True, "authenticated": False, "probe_ok": False,
+                  "user": None, "error": "The token in keyring is invalid."}
+        rc, out = self._run(status, [])
+        self.assertEqual(rc, 3)
+        self.assertNotIn("gh auth login", out)
+        self.assertIn("couldn't verify", out.lower())
+        self.assertIn("token in keyring is invalid", out)
+
+    def test_indeterminate_json_still_exits_3(self):
+        status = {"gh_present": True, "authenticated": False, "probe_ok": False,
+                  "user": None, "error": "boom"}
+        rc, out = self._run(status, ["--json"])
+        self.assertEqual(rc, 3)
+        self.assertEqual(json.loads(out), status)
+
+    def test_missing_probe_ok_key_is_treated_as_authoritative(self):
+        """Back-compat: a status dict from an older code path (no probe_ok) keeps
+        the historical exit-1 behaviour rather than becoming indeterminate."""
+        status = {"gh_present": True, "authenticated": False, "user": None, "error": "x"}
+        rc, _ = self._run(status, [])
+        self.assertEqual(rc, 1)
+
 
 if __name__ == "__main__":
     unittest.main()