✨ feat(auth): add logout and re-authentication functionality

[chriskyfung]

Implement logout to remove session files and add re-authentication capabilities through new CLI flags. This change enhances session management by allowing users to clear stored sessions and refresh credentials easily. Update documentation to reflect these new features and include tests to ensure functionality.

[codecov]

Codecov Report

❌ Patch coverage is 96.82540% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/instapaper_scraper/auth.py	89.65%	3 Missing ⚠️
src/instapaper_scraper/cli.py	92.10%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

[chriskyfung]

🚨 Remaining Issues

1. CLI ignores logout()'s return value, masking real failures (Medium-High)

File: src/instapaper_scraper/cli.py
Line: _handle_auth_command, `if args.logout:` branch
Issue: `authenticator.logout(purge_key=args.purge_key)` is called but its boolean 
       result is discarded — the CLI always exits with code 0 regardless of whether 
       deletion succeeded, failed (permission error), or was a no-op.
Suggestion: Since logout() currently returns False for both "nothing to remove" and 
       "deletion failed," this ambiguity needs resolving first — e.g. have logout() 
       raise a distinct exception (or return a tri-state / dataclass result) on 
       genuine OSError, and have the CLI exit(1) only on that failure path, not on 
       the harmless no-op case.
Severity: Medium

Right now a user running --logout on a permission-locked file gets a clean-looking exit 0 with only a log line buried in stderr — automation/CI scripts checking exit codes would never notice the session wasn't actually cleared.

2. --reauth can hang non-interactively without credentials (Low)

The docs correctly recommend interactive prompting, but there's no guard for --reauth invoked in a non-TTY context (e.g., CI/cron) with no --username/--password — it will block on input()/getpass() inside login(). Consider detecting sys.stdin.isatty() and failing fast with a clear error in that case.

3. Minor: test docstring typo (Cosmetic)

test_cli_logout_and_reauth_mutually_exclusive's docstring reads "--logoutand --reauth cannot be combined." — missing a space, purely cosmetic, no functional impact.

✅ Positive Observations

• The unlink error-handling pattern is applied consistently to both the session and key file paths, and is properly covered by tests.
• The --purge-key/--reauth warning closes a real UX gap without adding friction to the happy path.
• Test coverage expanded meaningfully: idempotency, cookie-clearing, and failure-injection are all now exercised, which is exactly the right response to the prior gaps.

🎯 Action Items

Must Fix: None blocking — all critical/high items from the prior review are resolved.

Should Fix: Propagate logout() failure into a non-zero CLI exit code so automation can detect deletion failures distinctly from no-op success.

Consider: Add a non-interactive/TTY guard for --reauth to avoid indefinite hangs in CI; fix the minor test docstring typo.

[chriskyfung]

All three previously identified issues are now resolved, and the fixes are implemented cleanly with no new problems introduced.

Verification of Previously Known Issues

Issue (Previous Review)	Status	Evidence
logout() failure silently ignored, CLI always exits 0	✅ Fixed	New SessionLogoutError(OSError) exception added in exceptions.py; logout() now raises it on unlink() failure instead of swallowing the error, and _handle_auth_command catches it and calls sys.exit(1)
--reauth could hang indefinitely in non-TTY contexts	✅ Fixed	New guard checks not args.username and not args.password and not sys.stdin.isatty(), logging an error and exiting 1 before ever calling force_login()
Test docstring typo (--logoutand)	✅ Fixed	test_cli_logout_and_reauth_mutually_exclusive docstring now correctly reads "--logout and --reauth cannot be combined."

Supporting test coverage confirms each fix behaves correctly: test_cli_logout_failure_exits_nonzero mocks a SessionLogoutError and asserts exit code 1 with the error logged, test_logout_survives_unlink_failure verifies the exception (not a no-op False) is raised on a real PermissionError, and test_cli_reauth_non_tty_without_credentials_fails_fast monkeypatches sys.stdin.isatty to confirm the fast-fail path and that force_login() is never invoked.

Design Quality Check

The SessionLogoutError subclassing OSError is a sensible choice — it lets logout() distinguish a genuine filesystem failure from the idempotent "nothing to remove" case (False return), and callers upstream can catch it specifically without swallowing unrelated exceptions. force_login() correctly lets this exception bubble up through its own unguarded self.logout() call, since _handle_auth_command wraps the entire force_login() invocation in the same try/except SessionLogoutError block used for --logout.

No new critical, high, or medium-severity issues surfaced in this revision.

📋 Review Summary

PR #115 — feat(auth): add logout and re-authentication functionality

This PR adds --logout and --reauth CLI flags with corresponding logout()/force_login() methods on InstapaperAuthenticator, backed by a SessionLogoutError exception for proper failure signaling, plus a DRY refactor extracting _resolve_session_paths() for shared use across --dump-session, --logout, and --reauth. Across three review passes, all identified issues — CLI password exposure in docs, unhandled filesystem exceptions, missing in-memory cookie clearing, misleading docstrings, silently-ignored --purge-key with --reauth, masked logout failures, and non-TTY hang risk — have been fully addressed with corresponding regression tests. Test coverage is thorough across both test_auth.py and test_cli.py, including idempotency, cookie-clearing, failure-injection via monkeypatched unlink, mutual-exclusivity, and non-interactive guard scenarios.

Recommendation: ✅ Approve — ready to merge.