fix: prevent story detection device auth in non-tty

[prompt-driven-github]

Summary

pdd detect --stories (and the other commands that run story validation through the shared runner — pdd change, agentic change, and drift fixing) could block forever on an interactive GitHub device-login prompt when run from an agent or CI-style non-interactive shell. This PR makes story validation non-interactive by default when there is no TTY, so it never opens a browser/device-login flow; instead it uses available credentials or fails fast with clear guidance.

Fixes #1923.

The bug (before)

When an agent/CI runs story validation with cloud enabled but no cached credentials, the story-validation LLM call reaches cloud authentication and prints a GitHub device code, then waits for a human who will never come:

To authenticate, visit: https://github.com/login/device
Enter code: XXXX-XXXX
Opening browser for authentication...
Waiting for authentication...      # <-- hangs forever; must be interrupted

Required story validation can never complete, so the whole run is stuck.

After

In a non-interactive (non-TTY) shell, story validation no longer starts device login. It either uses existing credentials, or fails fast and non-zero with actionable guidance:

FAIL user_stories/story__example.md
Story validation failed for user_stories/story__example.md: <error>. If this is
an authentication/credential error, non-interactive CI/agent runs must provide
model API credentials, set PDD_JWT_TOKEN for PDD Cloud, or run `pdd auth login`
before validation so no browser/device login is required.

• Interactive terminals are unchanged — a real user still gets the device-login flow.
• pdd auth login (explicit authentication) is unaffected, even with piped stdin.
• To deliberately allow device login during story validation from a non-TTY shell, set PDD_ALLOW_INTERACTIVE=1.

What changed

This is a prompt-first repo, so the behavior is specified in the .prompt sources and the generated code follows:

• run_user_story_tests is the single choke point for story validation (detect --stories, pdd change, agentic change, and drift fixing all call it). The non-interactive guard now lives there: when stdin is not a TTY (and the caller hasn't set PDD_ALLOW_INTERACTIVE), it sets PDD_NO_INTERACTIVE=1 around each auth-sensitive detect_change call and restores the caller's prior value afterward. This protects every caller, not just detect.
• The downstream device-flow guard (get_jwt_token._is_noninteractive) and interactive API-key acquisition (llm_invoke) both honor PDD_NO_INTERACTIVE, so neither a GitHub device flow nor an API-key input() prompt can start.
• Story validation now fails closed: if detect_change raises (auth/model/credential/network), the run reports a failed story and exits non-zero instead of crashing or hanging (preserves the Regression: pdd detect --stories exits 0 despite failed stories #1872 non-zero-exit contract).
• The non-interactive "truthy" set is unified to 1/true/yes/on across the guards.
• Docs updated (docs/generating_user_stories.md) with the supported credential paths and the PDD_ALLOW_INTERACTIVE opt-in.

Prompt/code/test/example/doc are all in sync (prompts: user_story_tests_python.prompt, commands/analysis_python.prompt, get_jwt_token_python.prompt, llm_invoke_python.prompt).

Testing

End-to-end / high-level scenarios covered by tests/test_issue_1923_detect_stories_noninteractive.py and updated existing tests:

• No device login in a non-TTY run (the exact bug): with cloud enabled and no cached credentials, run_user_story_tests drives the real detect_change → llm_invoke → get_jwt_token path with DeviceFlow trapped; the trap is never hit and the run fails closed. This is red on main (device flow starts) and green here.
• Falsy/empty presets can't reopen the hang: PDD_NO_INTERACTIVE="", "0", "false", "on", "1" are all forced to a guard-honored "1" during validation, then restored.
• Opt-in and interactive terminals still work: PDD_ALLOW_INTERACTIVE=1 and a real TTY do not force non-interactive mode.
• Fails closed / exits non-zero on a validation error, with the conditional credential guidance.
• Regression: pdd detect --stories exits 0 despite failed stories #1872 not regressed: the existing Regression: pdd detect --stories exits 0 despite failed stories #1872 reproduction tests still pass (fatal story failures exit non-zero).

Validation / mergeability

• Full local test sweep of the affected suites passes (detect/analysis, user story tests, cloud non-interactive auth, get_jwt_token, llm_invoke, auth commands).
• Edge cases considered: falsy env presets, the on truthy value, per-story env restore across fail_fast/continue/exception, opt-in, and real-TTY. Story validation is single-threaded per command, so the scoped os.environ set/restore is safe.
• Residual (documented, out of scope): a system keyring lookup still occurs before the device-flow guard; it is timeout-bounded and cannot hang indefinitely.
• Scope note: two sibling story-validation paths that call detect_change directly (run_user_story_fix, used by pdd fix <story>, and story prompt-link caching) are pre-existing and outside this issue's acceptance criteria; they are tracked in a separate follow-up so this PR stays scoped to pdd detect --stories blocks on interactive GitHub device auth in non-interactive runs #1923.

[niti-go]

Independent verification — PR #1959 (fixes #1923)

Ran an isolated worktree off main with a fresh end-to-end reproducer plus the existing suite. Summary: the fix correctly resolves #1923, but it is not mergeable as-is — it introduces two regressions that fail required CI (unit-tests.yml runs the full pytest tests/).

✅ What works

• pdd detect --stories blocks on interactive GitHub device auth in non-interactive runs #1923 is fixed at the CLI level. A hermetic reproducer that drives pdd detect --stories in a non‑interactive shell (non‑TTY streams, CI/PDD_NO_INTERACTIVE unset, cloud creds present, no cached/injected token) and mocks only the device‑code network boundary goes RED on main → GREEN on this branch: DeviceFlow.request_device_code is never called, no https://github.com/login/device banner, and the command exits non‑zero instead of blocking on poll_for_token.
• Regression: pdd detect --stories exits 0 despite failed stories #1872 is preserved. tests/test_issue_1872_reproduction.py passes (4/4). The new fail‑closed try/except around detect_change in user_story_tests.py correctly marks stories failed with credential guidance and keeps the non‑zero exit.
• Docs addition in generating_user_stories.md is clear and satisfies the "documented credential path" criterion.
• The PR's own new tests pass.

❌ Blocking — two CI regressions (both PASS on main, FAIL on this branch)

The full pytest tests/ that CI runs will fail on:

1. tests/test_cloud_noninteractive_auth.py::test_is_interactive_when_no_env_set
   • Root cause: the global change to get_jwt_token._is_noninteractive() (adding if not sys.stdin.isatty(): return True) contradicts this test, which explicitly asserts _is_noninteractive() is False when no env vars are set (even under a non‑TTY stdin). The test was not updated.
2. tests/test_llm_invoke.py::test_llm_invoke_auth_error_new_key_retry
   • Root cause: the global change to _interactive_credential_acquisition_allowed() (returning sys.stdin.isatty() as the default) makes it return False under pytest's non‑TTY stdin, so the interactive new‑key retry path is skipped and all candidates fail.

⚠️ Design concern — global change over‑reaches and regresses pdd auth login

• The scoped guard in commands/analysis.py (_story_detection_noninteractive() → set PDD_NO_INTERACTIVE=1 around run_user_story_tests) alone fixes pdd detect --stories blocks on interactive GitHub device auth in non-interactive runs #1923 via the existing env‑var guard. The two global non‑TTY changes are broader than the issue requires and are what break CI.
• pdd auth login calls get_jwt_token(...) (commands/auth.py) with no interactive opt‑in. With the global _is_noninteractive() change, an explicit pdd auth login run in a non‑TTY shell is now refused with a confusing "Refusing interactive device-flow auth … run pdd auth login" message, and it undermines the existing should_skip_browser remote/headless login flow. The PDD_ALLOW_INTERACTIVE escape hatch is undiscoverable to that user.

✅ Recommended fix (Path A — scoped; verified locally)

Keep the parts that matter and drop the global blast radius:

• Keep commands/analysis.py scoped guard, user_story_tests.py fail‑closed, and the docs.
• Revert the non‑TTY additions in get_jwt_token._is_noninteractive() (back to env‑only) and in _interactive_credential_acquisition_allowed() (drop return sys.stdin.isatty(); keep the PDD_ALLOW_INTERACTIVE/PDD_NO_INTERACTIVE gates with default True).

I applied exactly this locally and re‑ran the matrix: both regressions clear (9/9), the #1923 CLI reproducer stays GREEN, and #1872 + all of this PR's new tests pass (105/105).

Alternative (Path B — keep global): if the broader non‑TTY default is intended, then (a) update test_is_interactive_when_no_env_set (+ rationale) to the new semantics with a PDD_ALLOW_INTERACTIVE case, (b) fix test_llm_invoke_auth_error_new_key_retry, and (c) make pdd auth login opt back into interactive (set PDD_ALLOW_INTERACTIVE / pass an explicit allow_interactive=True) so explicit login is not refused in non‑TTY.

Minor

• The PR description is malformed — it contains raw dumped command output (device‑login repro text + preprocessing warnings) instead of a clean summary. Please tidy before merge.
• test_detect_stories_non_tty_disables_interactive_auth relies on pytest's ambient stdin capture (it will fail under pytest -s). Consider patching sys.stdin.isatty/os.isatty explicitly so it is deterministic.
• Neither new test exercises the real llm_invoke → cloud → get_jwt_token path; they mock run_user_story_tests/detect_change. A stronger regression test patches only the device‑code network boundary and asserts request_device_code is never called — happy to contribute the reproducer I used.

Verification method: RED baseline on main, GREEN on this branch for the CLI reproducer; existing tests run on both branches to isolate the two regressions.

[niti-go]

/gcbrun

[niti-go]

/gcbrun

[niti-go]

✅ Independent verification — APPROVED (head 052a67f2)

Re-verified the current head in an isolated worktree. This revision (v4 hardening + a clean merge of upstream/main) addresses every item from my earlier reviews. Recommending merge once required checks finish.

Verification results (head 052a67f2)

• pdd detect --stories blocks on interactive GitHub device auth in non-interactive runs #1923 reproducer: RED on main → GREEN here. An end-to-end pdd detect --stories run in a non-interactive shell (non-TTY, CI/PDD_NO_INTERACTIVE unset, cloud creds present, no cached token) issues no DeviceFlow.request_device_code, prints no login/device banner, and exits non-zero instead of blocking on poll_for_token.
• Regression sweep across auth/cloud/llm/story/detect: 0 failures. The two regressions I reported on the first revision (test_is_interactive_when_no_env_set, test_llm_invoke_auth_error_new_key_retry) are fixed.
• Regression: pdd detect --stories exits 0 despite failed stories #1872 preserved — fatal story-mode failures still exit non-zero (tests/test_issue_1872_reproduction.py green).
• Realistic red-green test: tests/test_issue_1923_detect_stories_noninteractive.py is genuinely RED on main (device-flow trap fires) and GREEN here; it covers the decision matrix, real-TTY behavior, force-over-preset, and env restoration.
• Prompts/code/tests/docs synchronized.

Design review — shared choke point is the right call

• The guard lives in run_user_story_tests (_story_validation_noninteractive() → scope PDD_NO_INTERACTIVE=1 around each detect_change, restored in finally), so all four callers are protected uniformly (detect --stories, change, agentic change, drift), not just the detect command.
• get_jwt_token._is_noninteractive() stays env-only, so an explicit pdd auth login (even with piped stdin) is unaffected.
• v4 hardening is correct and worthwhile: .strip() on env checks (whitespace consistency) and rich.markup.escape on the fail-closed print — the latter is a real fix, since an exception string containing [...] would otherwise raise MarkupError and turn a clean non-zero exit into a traceback.
• Env set/restore verified leak-free (handles the fail-fast break/continue paths; forces "1" so a falsy preset can't leave the downstream guard unset).

Resolved since earlier reviews

• ✔ Branch no longer behind main (merged; mergeable_state is now gated only on required checks/reviews).
• ✔ PR description rewritten to a clean summary.
• ✔ Sibling-entry-point gap closed by moving the guard to the shared runner.

CI (head 052a67f2)

All checks green: Run Unit Tests ✅, Public CLI Regression ✅, CodeQL ✅, Story Regression ✅, Package Preprocess Smoke ✅, Analyze (python/js/actions) ✅. (mergeable_state may flip to behind as main advances — that's just churn, not a conflict; give the branch a final update-to-main right before merge.)

From a correctness / test / prompt-code-doc-sync standpoint this is approved to merge. Nice iteration on the choke-point design and the fail-closed hardening.

[niti-go]

How this PR was reviewed and hardened

This PR was taken through two rounds of adversarial review plus an independent reproducer. Summary of what was found and how it was addressed:

Design correction (prompt-first): The initial version edited generated code without touching the owning .prompt files (so pdd sync would have reverted the fix) and made a global change to the device-flow guard that broke pdd auth login with piped stdin and an existing test. Reworked so the source-of-truth prompts describe the behavior and the fix is scoped to story validation.

Findings addressed:

• Falsy env preset hole: a stray PDD_NO_INTERACTIVE=""/"0" used to silently reopen the hang. Now story validation forces PDD_NO_INTERACTIVE=1 around each auth-sensitive call and restores the caller's value afterward, so no falsy preset can defeat it.
• Sibling commands still hung: the guard was moved into the shared run_user_story_tests choke point, so pdd detect --stories, pdd change, agentic change, and drift are all protected uniformly (not just detect).
• Truthy-set inconsistency: unified to 1/true/yes/on (whitespace-stripped, case-insensitive) across the guards.
• Error misattribution: the fail-closed story error is now conditional ("If this is an authentication/credential error …") so genuine bugs aren't reported as missing credentials; the error text is markup-escaped so it can't crash the fail-closed path.
• Test quality: the regression test now drives the real run_user_story_tests → detect_change → llm_invoke → get_jwt_token path with DeviceFlow trapped (red if the fix is removed, green here), plus parametrized coverage of the falsy/on presets, the opt-in, and the real-TTY case.

Validation: #1872 non-zero-exit contract preserved (4/4 reproduction tests pass); pdd auth login unaffected; prompts/code/tests/docs synchronized. Two pre-existing sibling detect_change paths (pdd fix <story>, story prompt-link caching) are outside this issue's scope and tracked in a separate follow-up so this PR stays focused on #1923.

[niti-go]

/gcbrun

[niti-go]

✅ Re-verified at head 76cd595e2 — approval stands

Confirmed the new head is the previously-verified fix + an unrelated upstream merge: the diff vs the green 052a67f2 across every fix-relevant file (get_jwt_token.py, user_story_tests.py, commands/analysis.py, llm_invoke.py, all touched prompts, and every #1923/#1872/noninteractive test) is empty. The only changes are upstream ci/cloud-batch/* + unrelated tests — none touch the fix.

• pdd detect --stories blocks on interactive GitHub device auth in non-interactive runs #1923 reproducer: still RED on main → GREEN here (no DeviceFlow.request_device_code, no login/device banner, non-zero exit).
• Reproducer + test_issue_1923_detect_stories_noninteractive.py + test_issue_1872_reproduction.py + the two previously-regressed tests: all green on this head.
• Branch is up to date with main.

My full verification review still applies (see the earlier comment). This confirms the main merge introduced no regression. Approved to merge once Run Unit Tests is green on 76cd595e2. 🚢