fix(npm): let npm run its 2FA prompt when reading trust state

npm/README.md

@@ -136,6 +136,13 @@ What it does per name, and why:
    expired session must never be read as "unconfigured", or the script ends up
    advising you to revoke a perfectly good entry.
 
+   On `unknown` the script retries once with an UNCAPTURED `npm trust list`.
+   npm's trust commands need an interactive 2FA challenge — it prints
+   "Authenticate your account at: …" and waits on the terminal — and a captured
+   stdout swallows that prompt, so npm fails with `EOTP`. Follow the prompt when
+   it appears; a session already warm from an earlier command prompts for
+   nothing.
+
    `--allow-publish` requires npm >= 11.15. An OLDER npm accepts the command
    without it and creates an entry carrying no publish permission — which looks
    configured and still 404s on release day. The script detects that and routes

npm/bootstrap-names.sh

@@ -87,15 +87,45 @@ fi
 # only ever be tested against fixtures invented to match it — that proves the
 # parser self-consistent and nothing about npm. The text path remains as a
 # fallback for an npm whose --json is missing or empty.
+# `trust_state` with one uncaptured retry, which is what makes it work at all on a
+# cold session.
+#
+# `trust_state` captures npm's output, and npm's trust commands need an
+# INTERACTIVE 2FA handshake: npm wants to print "Authenticate your account at:
+# <url>" and wait on the TERMINAL. A captured stdout swallows that, so npm fails
+# with EOTP and `trust_state` correctly reports `unknown`. One UNCAPTURED call
+# lets the handshake happen with stdio attached; the session then covers the
+# captured reads. Only done when needed, so a warm session prompts for nothing.
+#
+# A separate function ONLY so the test harness can drive it: the fixtures stub npm
+# out, so nothing about this retry was observable while it sat inline in the
+# caller — and a mutation removing it passed all 28 of them.
+#
+# The answer comes back in TRUST_STATE, NOT on stdout, and that is the whole
+# point. A caller writing `state="$(trust_state_interactive …)"` would recapture
+# the stdout this function exists to leave attached to the terminal: npm's prompt
+# would be swallowed again, and `$state` would arrive as several lines that match
+# no arm of the caller's `case`. The first version of this function did exactly
+# that. The test asserts the call site is not wrapped.
+trust_state_interactive() {
+  local pkg="$1"
+  TRUST_STATE="$(trust_state "$pkg")"
+  if [ "$TRUST_STATE" = unknown ]; then
+    echo "  auth:     npm needs an interactive 2FA challenge — follow its prompt"
+    $TRUST_NPM trust list "$pkg" || true
+    TRUST_STATE="$(trust_state "$pkg")"
+  fi
+}
+
 trust_state() {
-  local pkg="$1" out status
+  local pkg="$1" out rc
 
   out="$($TRUST_NPM trust list "$pkg" --json 2>/dev/null)"
-  status=$?
-  if [ "$status" -ne 0 ] || [ -z "$out" ]; then
+  rc=$?
+  if [ "$rc" -ne 0 ] || [ -z "$out" ]; then
     out="$($TRUST_NPM trust list "$pkg" 2>/dev/null)"
-    status=$?
-    [ "$status" -ne 0 ] && { echo unknown; return 0; }
+    rc=$?
+    [ "$rc" -ne 0 ] && { echo unknown; return 0; }
     printf '%s' "$out" | awk -v WORKFLOW="$WORKFLOW" -v REPO="$REPO" '
       # LINE-oriented, with an entry boundary at a REPEATED key. Do not assume
       # npm separates entries with a blank line: if it does not, a paragraph
@@ -241,7 +271,12 @@ JSON
   fi
 
   # 2. Attach the trusted publisher, so npm-publish.yml's OIDC can publish it.
-  case "$(trust_state "$pkg")" in
+  # NOT `state="$(trust_state_interactive …)"`: that would recapture the stdout
+  # npm's 2FA prompt needs. The state arrives in TRUST_STATE instead.
+  trust_state_interactive "$pkg"
+  state="$TRUST_STATE"
+
+  case "$state" in
     granted)
       echo "  trust:    already grants publish for $WORKFLOW"
       ;;

npm/test-bootstrap-names.sh

@@ -45,6 +45,8 @@ TRUST_NPM="fake_npm"
 
 # shellcheck disable=SC1090
 eval "$(awk '/^trust_state\(\) \{/,/^\}/' "$SCRIPT_UNDER_TEST")"
+# shellcheck disable=SC1090
+eval "$(awk '/^trust_state_interactive\(\) \{/,/^\}/' "$SCRIPT_UNDER_TEST")"
 
 fails=0
 t() {
@@ -59,6 +61,20 @@ t() {
   fi
 }
 
+# ---- CAPTURED FROM A REAL REGISTRY -----------------------------------------
+# Verbatim `npx npm@latest trust list @bugsee/cli-win32-arm64` output, npm
+# 12.x, 2026-09-16. Every other fixture here was written to match the parser,
+# which proves the parser self-consistent and nothing about npm; this one is
+# the other way round. Do not "tidy" it — the leading blank line and the exact
+# labels are the point.
+t "real npm output is classified granted" granted text 0 '
+type: github
+id: 6827c0f2-26cd-49fc-9cd3-374f7bb1d3b2
+file: npm-publish.yml
+repository: bugsee/bugsee-cli
+permissions: publish, stage publish
+'
+
 # ---- the state that must never be guessed --------------------------------
 # npm errored. Previously this returned "not configured", and the script then
 # advised revoking a possibly-correct entry.
@@ -185,6 +201,87 @@ t "JSON: unrelated metadata does not disarm the guard" unknown both 0 '{"meta":{
 t "JSON: npm supporting --json with no entries is 'absent'" absent both 0 '[]'
 t "JSON: a non-zero --json call falls back, not 'absent'" unknown json 1 '[]'
 
+# ---- the uncaptured retry ---------------------------------------------------
+# This is the half the 28 fixtures above CANNOT see: they stub npm out, so the
+# interaction between command substitution and npm's 2FA prompt is invisible to
+# them. The retry lived inline in the caller, and a mutation deleting it passed
+# every one of them.
+#
+# A fake npm that fails its first two (CAPTURED) reads and answers afterwards
+# stands in for the real failure mode: captured stdout -> EOTP -> `unknown`, then
+# an uncaptured call that authenticates, then a re-read that succeeds. It also has
+# no `--json` support, like the npm that produced the text fixture above.
+REAL_ENTRY='
+type: github
+id: 6827c0f2-26cd-49fc-9cd3-374f7bb1d3b2
+file: npm-publish.yml
+repository: bugsee/bugsee-cli
+permissions: publish, stage publish
+'
+# The call count lives in a FILE, not a variable: `trust_state` is invoked through
+# `$(...)`, and a subshell's increments die with it — which is why a first attempt
+# at this test saw the retry happen and still read `unknown`.
+# shellcheck disable=SC2329,SC2317
+flaky_npm() {
+  local json=0 n
+  for a in "$@"; do [ "$a" = "--json" ] && json=1; done
+  n=$(( $(cat "$CALL_FILE") + 1 ))
+  echo "$n" > "$CALL_FILE"
+  [ "$json" = 1 ] && return 1
+  [ "$n" -le "$FAIL_FIRST" ] && return 1
+  printf '%s' "$REAL_ENTRY"
+}
+
+# Asserts the classification AND whether the interactive retry was used, because
+# "granted either way" cannot tell a working retry from one that always runs.
+#
+# The state is read from TRUST_STATE and compared EXACTLY — no `tail -1`. An
+# earlier version of this test took the function's stdout and stripped the last
+# line off it, which quietly accommodated the bug the reviewer found: the state
+# was arriving with npm's output glued to it, and a real caller has no `tail`.
+# Stdout goes to a FILE here for the same reason the real call site leaves it
+# attached to the terminal — capturing it with `$(...)` would put the function in
+# a subshell and lose TRUST_STATE with it.
+ti() {
+  local name="$1" expected="$2" want_retry="$3" out retried
+  CALL_FILE="$(mktemp)"; echo 0 > "$CALL_FILE"
+  local out_file; out_file="$(mktemp)"
+  TRUST_STATE=""
+  TRUST_NPM=flaky_npm trust_state_interactive "@bugsee/x" > "$out_file" 2>/dev/null
+  out="$(cat "$out_file")"
+  case "$out" in *"needs an interactive 2FA challenge"*) retried=yes ;; *) retried=no ;; esac
+  rm -f "$CALL_FILE" "$out_file"
+  if [ "$TRUST_STATE" = "$expected" ] && [ "$retried" = "$want_retry" ]; then
+    printf '  ok    %s\n' "$name"
+  else
+    printf '  FAIL  %s (expected %s/retry=%s, got %s/retry=%s)\n' \
+      "$name" "$expected" "$want_retry" "$TRUST_STATE" "$retried"
+    fails=1
+  fi
+}
+
+# The call site itself, which no amount of driving the function can check: wrapping
+# it in a command substitution recaptures the stdout npm's prompt needs, and the
+# multi-line value then matches no arm of the caller's `case`. That is precisely
+# the regression the reviewer caught, so it is pinned structurally.
+# Comments stripped first: this file's own explanation of the hazard, and the
+# script's, both contain the very shape being searched for.
+if grep -vE '^[[:space:]]*#' "$SCRIPT_UNDER_TEST" | grep -qE '[$][(][[:space:]]*trust_state_interactive'; then
+  printf '  FAIL  trust_state_interactive is called in a command substitution — recaptures the 2FA prompt\n'
+  fails=1
+else
+  printf '  ok    the interactive read is not wrapped in a command substitution\n'
+fi
+
+# A cold session: the captured reads fail, so the retry is what rescues it.
+FAIL_FIRST=2
+ti "an EOTP-failing captured read is retried uncaptured, then classified" granted yes
+
+# A warm session answers immediately, and nothing is retried — so a session
+# already authenticated is never prompted.
+FAIL_FIRST=0
+ti "a session that already answers is classified without a retry" granted no
+
 if [ "$fails" -eq 0 ]; then
   echo "all trust_state cases pass"
 else