fix: detect claude.ai's 403 session-invalid as an expired key; ChatGPT .1 cookie is optional

claude.ai answers an invalid or expired sessionKey with 403 account_session_invalid, never 401, so the refresh-your-cookie message could not fire for Claude. Auth detection is now a provider decision (_is_auth_failure); Claude matches the 403 on its error code so a real permission error still reports as itself. README and .env.example no longer claim both ChatGPT cookie chunks are required.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LbnmGHnFqDjyhPcCg1SEfF
This commit is contained in:
JesseMarkowitz
2026-10-05 07:14:01 -04:00
co-authored by Claude Opus 5.5
parent d3745e1de4
commit b6ce636891
8 changed files with 256 additions and 30 deletions
+14
View File
@@ -6,6 +6,20 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
## [Unreleased]
### Fixed
- **An expired Claude session key reported a raw JSON dump instead of how to fix it.** `_make_request` routed only **401** to the auth handler (`src/providers/base.py`), and claude.ai does not use 401 — an invalid or expired `sessionKey` comes back as `403 permission_error` with `details.error_code = account_session_invalid`. So the one message that names the cookie, its ~30-day lifetime and the DevTools path to refresh it could never fire for Claude. What the user got instead was the generic 4xx path: `HTTP 403 — error: {'type': 'permission_error', 'message': 'Invalid authorization'…}`, which reads like a permissions problem with the account and not like "your key expired, here is how to replace it."
Measured live 2026-09-20 against `GET /api/organizations`: a valid key returns 200, while an expired key, a deliberately malformed key and **no cookie at all** return byte-identical 403s carrying that code — i.e. the API treats a dead session as an absent one. This is the same mistake as the ChatGPT media 403s below: assuming 403 means "forbidden" when the service uses it for "unauthenticated."
Auth detection is now a provider decision rather than a hardcoded status. `BaseProvider._is_auth_failure(response)` defaults to 401 and `ClaudeProvider` overrides it to add 403 **matched on `account_session_invalid`**, not on the bare status — so a genuine permission error, which carries a different code, is still reported as itself rather than being mislabelled an expired key. `_handle_401` is renamed `_handle_auth_failure` and takes the response, because a handler named for one status that must handle two is how this stayed hidden; its messages now state the status actually observed instead of asserting "401 Unauthorized". ChatGPT is unaffected: it does not override the default, so the deleted-asset 403 path is untouched.
Seven regression tests cover the split (`TestAuthFailureDetection`), including the two that matter most: a Claude 403 with a *different* error code must **not** be treated as an auth failure, and a ChatGPT 403 must not either. The docs that repeated the wrong premise — `README.md`'s expiry table and "When Tokens Expire" section, the `auth` wizard's on-screen note, and the `ClaudeProvider` docstring — are corrected in the same change.
- **The docs claimed both ChatGPT cookie chunks were required; they are not.** `README.md` stated flatly that "ChatGPT splits large session tokens across two cookies to stay under the browser's 4KB cookie limit. Both are required," and `.env.example` documented only the chunked layout — so a machine whose session token happens to fit in a single `__Secure-next-auth.session-token` cookie looked broken, with the user hunting for a `.1` that does not exist. Chrome splits a cookie only above ~4KB, so the layout varies by session size and the *same account* can be chunked on one machine and not on another.
The code was already correct: `CHATGPT_SESSION_TOKEN_1` is optional (`src/providers/chatgpt.py:162`) and the `auth` wizard already told you to paste a lone cookie into `.0` and leave `.1` blank. Only the reference docs were wrong, and they are the ones read when setting up a new machine.
Measured 2026-09-20 against `/api/auth/session`, reassembling a real 4089-byte token to test each naming: chunked `.0`+`.1` → 200 with an `accessToken`; the whole value under the unchunked name → 200 with an `accessToken`; the whole value under `.0` alone → 200 with an `accessToken`. The server reassembles a complete value sent under `.0`, so both layouts authenticate as the code already assumed. A *partial* `.0` with its `.1` omitted is the one combination that fails, and it fails **silently** — HTTP 200 with no `accessToken` rather than an error — which is now documented in both files alongside the correction.
- **The test suite sent real push notifications to the developer's phone.** `TestSyncCommand` invokes the actual `sync` command, which calls `load_config()`, which calls `load_dotenv()` — so the real `.env` was loaded and its live `NTFY_TOPIC` used for the POST. Every `pytest` run fired three or four pushes, including a fabricated "codex: 3 conversation(s) failed to export" straight out of a fixture, which is worse than noise: it reports a failure that never happened. Nothing appeared in `cache/logs/exporter.log` to explain it, because every test invocation passes `--no-log-file`.
A `tests/conftest.py` autouse fixture now neutralises the environment for every test: `NTFY_TOPIC`/`NTFY_TOKEN` are emptied, and `NTFY_SERVER` and `JOPLIN_API_URL` are pointed at a closed local port, so a stray topic cannot reach the internet and a test cannot write notes into a real Joplin instance. The values are **emptied rather than deleted** — `load_dotenv(override=False)` skips only keys already present, so deleting one lets `.env` put it back. Verified by instrumenting `requests` across a full run: zero outbound requests, where the same instrumentation without the fixture records POSTs to the live ntfy topic.