Status changes: applied_at and closed_at are decided from the caller's stale copy, not the locked row #543

New issue

Closed

opened 2026-10-01 10:47:31 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:47:31 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading change_status and replaying two moves made from copies fetched before either ran.

Checked again on 2026-10-01 against main at 0a5cad587: still present.

Since #231, change_status re-reads the application with select_for_update() so that the status it moves from is the one another request has just written. It copies only status from the locked row. Whether to stamp applied_at and whether to set or clear closed_at are still decided from the object the caller fetched before the lock, and both are then written with update_fields, so a concurrent move can leave the two timestamps contradicting the status.

Where: applications/services.py:149-151 copies locked.status onto the caller's object; :162 tests application.applied_at is None and :167-171 test application.closed_at on the caller's object.
What happens (replayed on SQLite with two copies of one application in Screening, fetched before either move):
- Copy A moves it to Rejected, which sets closed_at. Copy B then moves it to Interviewing: B finds Rejected under the lock, but its own closed_at is None, so it clears nothing. The result is an Interviewing application with closed_at set.
- With a Draft, copy A moves it to Applied and stamps applied_at; copy B then moves it to Screening, sees applied_at is None on its own copy and stamps it again with a later time, although the docstring says it is "never moved afterwards".
- The reverse also holds: A reopens a rejected application (clearing closed_at) while B withdraws it from a copy that still has closed_at, and the withdrawn application is left with no closed_at.
- On a file SQLite database every request is one immediate transaction, so the page's own read already happens after the other request committed. The race needs PostgreSQL, where two tabs, or the board and an API client, can move one application at the same moment.
Why it matters: closed_at decides whether an application counts as open in lists, reports and exports, and applied_at is what response times are measured from (#222). The event log stays right; the derived columns disagree with it.
Fix: before deciding anything, copy applied_at and closed_at from locked onto the caller's object (as status already is), or decide on locked and mirror the result onto application.

Done when

tests/test_application_swaps.py: two copies of one application fetched before either move; after change_status(a, "rejected") and change_status(b, "interviewing"), the row has closed_at None; after change_status(a, "applied") and change_status(b, "screening") from a draft, applied_at is the first stamp.

Related to #231 (closed), whose point 11 introduced the lock and left these two columns reading the caller's copy.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading `change_status` and replaying two moves made from copies fetched before either ran. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. Since #231, `change_status` re-reads the application with `select_for_update()` so that the status it moves *from* is the one another request has just written. It copies only `status` from the locked row. Whether to stamp `applied_at` and whether to set or clear `closed_at` are still decided from the object the caller fetched before the lock, and both are then written with `update_fields`, so a concurrent move can leave the two timestamps contradicting the status. - **Where:** `applications/services.py:149-151` copies `locked.status` onto the caller's object; `:162` tests `application.applied_at is None` and `:167-171` test `application.closed_at` on the caller's object. - **What happens** (replayed on SQLite with two copies of one application in *Screening*, fetched before either move): - Copy A moves it to *Rejected*, which sets `closed_at`. Copy B then moves it to *Interviewing*: B finds *Rejected* under the lock, but its own `closed_at` is `None`, so it clears nothing. The result is an *Interviewing* application with `closed_at` set. - With a *Draft*, copy A moves it to *Applied* and stamps `applied_at`; copy B then moves it to *Screening*, sees `applied_at is None` on its own copy and stamps it again with a later time, although the docstring says it is "never moved afterwards". - The reverse also holds: A reopens a rejected application (clearing `closed_at`) while B withdraws it from a copy that still has `closed_at`, and the withdrawn application is left with no `closed_at`. - On a file SQLite database every request is one immediate transaction, so the page's own read already happens after the other request committed. The race needs PostgreSQL, where two tabs, or the board and an API client, can move one application at the same moment. - **Why it matters:** `closed_at` decides whether an application counts as open in lists, reports and exports, and `applied_at` is what response times are measured from (#222). The event log stays right; the derived columns disagree with it. - **Fix:** before deciding anything, copy `applied_at` and `closed_at` from `locked` onto the caller's object (as `status` already is), or decide on `locked` and mirror the result onto `application`. ## Done when - `tests/test_application_swaps.py`: two copies of one application fetched before either move; after `change_status(a, "rejected")` and `change_status(b, "interviewing")`, the row has `closed_at` `None`; after `change_status(a, "applied")` and `change_status(b, "screening")` from a draft, `applied_at` is the first stamp. Related to #231 (closed), whose point 11 introduced the lock and left these two columns reading the caller's copy.

tiagoagueda added this to the 0.6.0 milestone

2026-10-01 10:47:31 +00:00

tiagoagueda added the

tier

bug

labels

2026-10-01 10:47:31 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 02:35:51 +00:00

Decide applied_at and closed_at from the locked row in change_status

tiagoagueda closed this issue

2026-10-04 02:35:51 +00:00