fix: clear expired websocket cookies - #144

Reviewed at 4f1b98a. The direction is right, and I confirmed the underlying bug is real: the store replays a stale affinity cookie after the server has explicitly cleared it. Your two new tests pass here (tests/http/test_cookies.py → 8 passed). Three behavioural gaps, all inside _is_deletion_cookie, each measured by running the store against this PR's own src/acp/_cookies.py.

1. Max-Age is compared as the literal string "0"

if key == "max-age" and value == "0":

RFC 6265 §5.2.2 defines expiry as delta-seconds <= 0, not == 0:

Max-Age=-1 is not exotic — it is a common "expire now" idiom. (I deliberately left Max-Age=+0 out of the claim: the §5.2.2 parse algorithm does not describe a leading +, so I don't think it is fair to require it.)

2. Expires matches one exact string

if key == "expires" and value in {"thu, 01 jan 1970 00:00:00 gmt", "0"}:

Every other valid cookie-date in the past is missed — including one second later on the same day:

All five are well-formed cookie-dates per §5.1.1 and all are in the past, so all five must expire the cookie.

3. The two attributes are OR'ed, but §5.3 gives Max-Age precedence

RFC 6265 §5.3 step 3 reads "if … 'Max-Age' … Otherwise, if … 'Expires'". So Max-Age is consulted first and Expires only matters when Max-Age is absent. This header therefore describes a cookie the server intends to keep for an hour:

affinity=keepme; Max-Age=3600; Expires=Thu, 01 Jan 1970 00:00:00 GMT

This is the one direction of error that makes the change worse than the status quo, for any server that sets both.

Why the new tests don't catch any of it

Both added tests assert the exact spellings the implementation special-cases (Max-Age=0, Expires=Thu, 01 Jan 1970 00:00:00 GMT), so they pin the implementation rather than the requirement. A table-driven test over the rows above covers the same ground and goes red on any regression in them.

A shape that handles all three

Parse instead of string-matching, and consult Max-Age first so it wins outright:

def _expiry_decision(attributes: list[str]) -> bool | None:
    """True = expire now, False = persist, None = no usable expiry attribute."""
    verdict = None
    for attribute in attributes:
        key, separator, value = attribute.partition("=")
        if not separator:
            continue
        key = key.strip().lower()
        value = value.strip()
        if key == "max-age":
            try:
                return int(value) <= 0          # Max-Age wins, per 5.3
            except ValueError:
                continue                        # malformed: fall through to Expires
        if key == "expires" and verdict is None:
            try:
                when = parsedate_to_datetime(value)
            except (TypeError, ValueError):
                continue
            if when.tzinfo is None:
                when = when.replace(tzinfo=timezone.utc)
            verdict = when <= datetime.now(timezone.utc)
    return verdict

Caveat: I have not implemented or run this — it is a sketch, and int() here is more permissive than §5.2.2 (which only allows an optional leading - and digits). Happy to send it as a tested patch if reviewing a diff is easier than reviewing a snippet.

One scope question, which is genuinely yours to call

The module docstring says the store is "intentionally minimal … matching the affinity-only use case in the RFD". If the intent is really "handle exactly the two literals the known server sends", that is a defensible scope decision — but then the docstring should say that, because as written ("an epoch Expires value") it promises the general case that (2) does not deliver. Either resolution works; the mismatch between the promise and the behaviour is the part I would fix.