fix(backend): reject expired task and workstream suggestions on accept

Merged
AR
17d ago
+299-9

Summary

Addresses only the expired-suggestion acceptance slice (criterion 2) of #19566. This is not a close-out of that umbrella issue and does not overlap the floating-bar Execute work proposed there.

An old chat card could accept a pending Candidate after its two-day lifetime had ended. Suggested-list reads hid it, but the acceptance transactions checked only status == pending. They could create a task/workstream or apply an old task mutation anyway.

Both canonical acceptance transactions now use the existing candidate_has_lapsed predicate on the stored Candidate before any write. They return HTTP 409, Candidate suggestion has expired, at or past the deadline. The clock is sampled inside each transaction attempt after reading the Candidate, so a retried transaction cannot reuse the first attempt's eligibility time.

  • Covers task create/update/complete/cancel/supersede and workstream-plus-anchor-task acceptance.
  • Preserves account-generation checks and existing accepted receipts. A late retry of an already successful acceptance still returns the original receipt without another write.
  • Keeps the current two-day fallback for old rows without expires_at.
  • Uses the existing conflict-to-HTTP mapping; no router/schema changes, which also avoids overlapping #18851's router error-sanitization work.
  • Refreshes stale lifecycle documentation and fixes historical test fixtures that unintentionally attempted to accept July suggestions using today's clock.

Expiry policy and limits

Reject late acceptance; keep derived expiry. This change does not rewrite a lapsed row's stored pending status, delete history, run a backfill, or enable Firestore TTL. The existing explicit /expire operation remains available. Storage reclamation is a separate operational/product decision, not an incidental consequence of an old card being clicked.

Eligibility is evaluated at the resolution check inside each attempt, not against a promised Firestore commit timestamp. Legacy staged-promotion claims/recovery are unchanged. No mobile, hardware, summary matching, floating-bar UI, production-data inventory, or deployment is included. This backend fix does not claim a physical desktop test or improved client-side expired-card presentation.

Reproduction and regression proof

Base: e7dfeee5b63d63ea8d7b450023a7042c1d097288.

The final regression suite was run with both production files restored exactly to that base (verified with git diff --exit-code), then rerun with the fix:

Before: 38 failed, 8 passed — all 38 failures were HTTP 200 where 409 was required
After:  46 passed (30 hermetic cases + 16 real Firestore-emulator cases)

The matrix covers all six proposal types, stored/derived deadlines, exact deadline and long-expired suggestions, untouched stored rows and delivery mocks after rejection, generation mismatch, live acceptance one microsecond before expiry, and accepted-receipt replay 30 days later.

The emulator suite runs the production router/service/resolvers with the real Firestore SDK. Two tests inject an aborted commit, release its emulator transaction locks, advance the clock across the deadline, and let the SDK retry. The retry now rejects before a second commit. This is controlled retry fault injection, not a claim of naturally occurring contention or a full production end-to-end test.

cd backend
FIRESTORE_EMULATOR_HOST=127.0.0.1:10285 .venv/bin/pytest -q \
  tests/integration/test_candidate_accept_expiry_emulator.py \
  tests/unit/test_candidate_accept_expiry.py

The integration proof requires a loopback emulator and synthetic demo-candidate-expiry project. Authentication and downstream delivery are stubbed; the real persistence and HTTP error mapping are exercised. Hermetic coverage uses the shared strict read-before-write fixture and is selected by the normal CI test runner.

Local HTTP smoke

Served the real Candidate router with Uvicorn on 127.0.0.1:10286, synthetic auth/records, and the loopback Firestore emulator. Called the actual acceptance route with curl:

curl -X POST -H 'X-Account-Generation: 3' \
  http://127.0.0.1:10286/v1/candidates/expired-task/accept

Before calls:           tasks=0, workstreams=0, outbox=0
expired-task:          409 {"detail":"Candidate suggestion has expired"}
expired-workstream:    409 {"detail":"Candidate suggestion has expired"}
After rejected calls:   tasks=0, workstreams=0, outbox=0
live-task:             200, newly_resolved=true
live-task retry:       200, newly_resolved=false, same receipt/task/resolved_at
After live + retry:     tasks=1, workstreams=0, outbox=1

Verification

  • 312 hermetic tests across 16 focused files passed, using the repository's file-isolated backend/test.sh runner. Covers Candidate lifecycle/readers/routes/capture, staged migration, workstream core/association/readers/routes, and frozen task contracts. The automatic selector expands this shared dependency to 1,344 files; this local run deliberately targets the affected contract instead of claiming the entire suite.
  • 16 real-SDK/emulator cases passed, including both transaction-retry deadline crossings.
  • make preflight passed 31 checks; PR-body validation passed four metadata checks. The history-dependent failure-class recurrence check explicitly skips on this shallow clone; it is not claimed as fully verified locally.
  • Normal commit hooks and bounded pre-push passed, including type-checking (0 errors, 8,413 existing warnings), formatting, source closure, and changed-file unit tests. Final committed-source preflight also passed all 31 checks with this PR body. Full CI remains authoritative; the full 1,344-file backend suite was not run locally.
  • Both owned local proof servers were stopped after verification. No production records or services were used.

Product invariants affected

  • INV-TASK-2 — unused suggestions expire; acceptance cannot revive a lapsed pending suggestion into work.

Failure class

Failure-Class: FC-record-lifetime-enforced-by-one-reader

Why isn't this a shared primitive instead? It already is: both transaction readers now use the same candidate_has_lapsed predicate as the Suggested projection, including legacy fallback semantics. No separate TTL constant, policy checker, or new expiry implementation is added. The real incident is #19566; the read-side lifetime was introduced in #11974 and covered by the existing class.

Line-Count-Exception: backend/database/candidates.py | 1286 -> 1288 | Two net lines enforce the shared lifetime predicate at the existing atomic acceptance boundary; extracting this transaction would add unrelated risk.
Line-Count-Exception: backend/database/workstreams.py | 1404 -> 1407 | One import and two net lines apply the same shared lifetime predicate to the existing workstream/anchor-task transaction.