fix: stop dropping current-trimester tasks on the ingest ordering race - #20
Merged
Merged
Conversation
project_tasks and projects land as independent, concurrent /ingest requests from the extension. The project_tasks handler rejected any project_id not yet present in `projects` as an ended unit, but a returning student's current-trimester project_tasks push routinely beat its own projects ingest to the DB — misclassifying it as inactive and silently dropping the tasks (compounded by the extension's ingest dedup caching the "skipped" response as if it had been stored, so it never retried). Remove the eager active_ids rejection — the existing prune step already cleans up stale-trimester data once its own projects ingest lands, so the check isn't needed for correctness and only introduced the race. Also harden the extension's dedup cache to never treat a skipped/ non-stored response as sent, as defense in depth.
There was a problem hiding this comment.
Pull request overview
This PR fixes a race between concurrent /ingest requests where project_tasks can arrive before projects, causing current-trimester tasks to be misclassified as inactive and dropped. It removes the server-side eager “active project” rejection and adds client-side dedup hardening, plus a regression test to pin the behavior.
Changes:
- Server: remove the
project_tasks“active_ids” rejection so tasks are stored even if the correspondingprojectsrow hasn’t landed yet. - Extension: update ingest dedup caching to avoid treating
skippedresponses as successfully sent. - Tests: add a regression test covering both the “no projects row yet” and “stale old projects present” race scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/test_ingest_project_tasks_race.py | Adds regression coverage for the ingest ordering race using a throwaway DB-backed Flask client. |
| routes/main.py | Removes eager inactive-project rejection to eliminate ingest ordering race behavior. |
| extension/public/background.js | Hardens ingest dedup caching behavior to avoid caching skipped ingests. |
| core/db.py | Removes the now-unneeded get_active_project_ids helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+49
to
+52
| monkeypatch.setenv("DB_PATH", db_path) | ||
| monkeypatch.setenv("SECRET_KEY", "test-secret") | ||
| monkeypatch.setenv("RESEND_API_KEY", "test-key") | ||
| monkeypatch.setenv("RESEND_FROM_EMAIL", "test@example.com") |
Comment on lines
+69
to
+72
| flask_app = create_app() | ||
| flask_app.config["TESTING"] = True | ||
| with flask_app.test_client() as c: | ||
| yield c |
Comment on lines
117
to
+121
| .then((r) => r.json()) | ||
| .then(() => { | ||
| lastIngestHash.set(dedupKey, hash); // only cache once server has accepted it | ||
| .then((d) => { | ||
| // Only cache once the server actually stored it — a `skipped` response | ||
| // (e.g. rejected as an inactive project) must not be treated as sent, or | ||
| // this dedup would permanently suppress a push that never landed. |
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
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.
project_tasks and projects land as independent, concurrent /ingest
requests from the extension. The project_tasks handler rejected any
project_id not yet present in
projectsas an ended unit, but areturning student's current-trimester project_tasks push routinely beat
its own projects ingest to the DB — misclassifying it as inactive and
silently dropping the tasks (compounded by the extension's ingest dedup
caching the "skipped" response as if it had been stored, so it never
retried).
Remove the eager active_ids rejection — the existing prune step already
cleans up stale-trimester data once its own projects ingest lands, so
the check isn't needed for correctness and only introduced the race.
Also harden the extension's dedup cache to never treat a skipped/
non-stored response as sent, as defense in depth.