feat(s-claim-filter): policy-aware timer-sweep claim#296
Merged
Conversation
Make _due_task_ids policy-aware so a task is claimed only when it has a configured, unstamped timer step. Closes the recurring Codex P2 (the remind_2_sent_at IS NULL / DOC_ACK-escalate tautology that re-claimed every open task on every sweep forever). Query-only, no migration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…claimed Three integration cases for _due_task_ids: a fully-fired OPEN APPROVE task and a fully-fired OPEN DOC_ACK task must NOT be claimed (the remind_2 / escalate tautologies), and pending-step tasks must still be claimed (over-tightening guard). Red on the current 4-way claim; green after the policy-aware predicate lands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_due_task_ids now claims a task only when it has a configured, unstamped timer step (each stamp-NULL clause gated on its SlaPolicy offset), mirroring due_steps' gating. Closes the remind_2_sent_at / DOC_ACK-escalate tautology that re-claimed every open task on every sweep forever. Query-only; no migration, no new key, no WORM/authz/web change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…in control Final-review Minors: reword the loose "only <X> is NULL" comments (remind_2 is also NULL but inert), and add a pending-overdue DOC_ACK positive control so the DOC_ACK exclusion test can't silently pass-for-wrong-reason if the seed ever drops DOC_ACK's SLA policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CoJoA13
added a commit
that referenced
this pull request
Jun 26, 2026
Record the S-claim-filter slice (policy-aware timer-sweep claim, Codex P2 close-out): slice-history narrative + CLAUDE.md Recent-learnings bullet (dropping the oldest S-notify-4 to hold the ~8 cap) + Current-status pointer. No code change; migration head stays 0067. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
S-claim-filter — policy-aware timer-sweep claim
Makes the escalation
timer_sweep's claim query (_due_task_ids) policy-aware so itstops re-claiming fully-fired tasks on every 5-minute sweep forever. Closes the recurring
deferred Codex P2 (the "claim-threshold filter" residual, Option A).
The bug
The old claim OR-ed four stamp-NULL clauses, two of which are tautologies given the seed
(migration 0065):
remind_2_sent_at IS NULL— always true (remind_2_before=Nonefor every seeded policy →REMIND_2never fires/stamps).escalated_1_at IS NULL— always true for DOC_ACK / PERIODIC_REVIEW (escalate_1_after=None).So every open task with an active SLA policy + a
due_atwas re-claimed each sweep, forever —each re-claim running the full
process_task_timersno-op (advisory lock → locking SELECT →policy SELECT → instance GET → calendar resolve → commit) only for
due_stepsto return[].The fix (query-only)
Gate each stamp-NULL clause on its
SlaPolicyoffset being configured, mirroringtimer.due_steps'policy.<offset> is not None and stamp is Nonegating (OVERDUE has no offset →stamp-only).
remind_2stays policy-gated and inert (not a tautology —remind_2_before IS NOT NULLis false everywhere today), so the future "distinct remind_2" slice is a one-line seed flip.A fully-fired task now evaluates all four disjuncts to false → not claimed. The claim only ever
gets tighter;
due_stepsinside the locked txn remains the authoritative per-step decision, sono notification can be dropped or mis-fired — verified by both final reviewers as a strict subset of
the old claim and a superset of the
due_stepsfiring condition.Out of scope (named residuals, unchanged): pre-due claim churn (the conservative threshold-gate,
Option C) and narrowing
ix_task_timer_pending(Option B — a join-qual planner subtlety made it notworth a migration).
Scope
0067), no new permission key, no WORM / authz / web / openapi change.due_steps/process_task_timers/ stamping / audit / recipients are byte-identical.Tests
apps/api/tests/integration/test_notification_timer_sweep.py:open DOC_ACK task must not be claimed (RED on the old 4-way claim, GREEN after this change);
pending-overdue DOC_ACK (proves the DOC_ACK policy join resolves) must still be claimed.
All assertions are run-scoped membership on open-state tasks; no shared-DB pollution.
Gates
Local:
ruffclean ·ruff formatclean ·mypy src(412 files) clean ·pytest -m unit1062 passed, 1 skipped. The new tests are
@pytest.mark.integration→ CI'sintegrationjobis their authoritative gate (Docker-less dev box).
Review
Subagent-driven build (per-task spec+quality review) → final whole-branch review: opus senior
reviewer (Ready=Yes) + diff-critic (CLEAN) converged; the two Minors they raised (loose
positive-control comments; a DOC_ACK join positive control) are folded in.
🤖 Generated with Claude Code