Skip to content

Prevent duplicate chapter-generation workers per session - #73

Merged
CyberSecDef merged 3 commits into
mainfrom
copilot/prevent-duplicate-chapter-workers
Apr 3, 2026
Merged

Prevent duplicate chapter-generation workers per session#73
CyberSecDef merged 3 commits into
mainfrom
copilot/prevent-duplicate-chapter-workers

Conversation

Copilot AI commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

/generate_chapters had no guard against launching multiple background workers for the same session — repeated clicks or multiple tabs could spawn competing threads writing to the same session state.

Backend — session-scoped guard (novelforge/routes/generation.py)

Before spawning a worker, the route now checks whether session["progress_token"] already points to a "running" entry in _progress_store. If so, it returns HTTP 409 with the existing token:

{
  "error": "A chapter generation is already in progress for this session.",
  "error_code": "generation_in_progress",
  "token": "<existing-token>"
}

Frontend — button lifecycle (static/js/script.js)

  • btn-approve-outline is disabled on first click and stays disabled for the full duration of generation. Removed the blanket re-enable from the approve_outline complete handler.
  • Re-enabled only on: approve_outline failure, generate_chapters non-409 failure, generation done (showDoneStep), or generation error (poll error branch).
  • 409 is handled gracefully: client silently attaches to the existing token and begins polling — no user-visible error.

Tests (tests/test_concurrency.py)

Replaced test_two_generation_requests_get_different_tokens (which incorrectly expected both requests to succeed) with four targeted tests:

Test Behaviour
test_first_generation_request_succeeds 200 + token stored as running
test_duplicate_generation_request_blocked_with_409 409 + existing token returned, single store entry
test_new_generation_allowed_after_previous_completes 200 once prior token is done
test_rapid_repeat_calls_all_blocked_after_first 5 calls → [200, 409, 409, 409, 409]

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • api.openai.com
    • Triggering command: /usr/bin/python python -m pytest tests/ -v (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI and others added 2 commits April 3, 2026 17:18
- Backend: guard in /generate_chapters returns 409 (error_code:
  generation_in_progress) when a running worker already exists for the
  session; returns the existing token so the client can attach to it
- Frontend: btn-approve-outline stays disabled for the full duration of
  generation; re-enabled only on approve error, generation error, or
  generation done
- Frontend: 409 response is handled gracefully — client attaches to the
  existing in-progress token and resumes polling instead of erroring out
- Tests: replaced test_two_generation_requests_get_different_tokens with
  four focused tests covering: first request succeeds, duplicate blocked
  with 409, new request allowed after completion, rapid-fire calls all
  blocked after first

Agent-Logs-Url: https://github.com/CyberSecDef/NovelForge/sessions/419ef6c7-f0ca-4371-a42a-163df2591924

Co-authored-by: CyberSecDef <[email protected]>
Copilot AI changed the title [WIP] Prevent duplicate chapter-generation workers for the same session Prevent duplicate chapter-generation workers per session Apr 3, 2026
Copilot AI requested a review from CyberSecDef April 3, 2026 17:21
@CyberSecDef
CyberSecDef marked this pull request as ready for review April 3, 2026 17:49
Copilot AI review requested due to automatic review settings April 3, 2026 17:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Prevents multiple /generate_chapters background workers from being launched for the same user session (e.g., repeated clicks / multiple tabs), and updates the UI + tests to support the new behavior.

Changes:

  • Added a session-scoped guard in /generate_chapters to return HTTP 409 with the existing progress token when generation is already running.
  • Updated frontend button enable/disable lifecycle to avoid re-enabling “Approve & Write Chapters” while generation is in progress, and to attach to an existing token on 409.
  • Reworked concurrency tests to validate first-call success, duplicate-call 409 behavior, and “new run allowed after done”.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
novelforge/routes/generation.py Adds a guard intended to prevent starting duplicate generation workers per session.
static/js/script.js Keeps the approve button disabled during generation; handles 409 by attaching to the existing token and polling.
tests/test_concurrency.py Replaces prior token-expectations test with targeted tests for 200/409 behavior and post-completion regeneration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +72 to +82
# Guard against duplicate workers for the same session
existing_token = session.get("progress_token")
if existing_token:
with _progress_lock:
existing = _progress_store.get(existing_token)
if existing and existing.get("status") == "running":
return jsonify({
"error": "A chapter generation is already in progress for this session. Please wait for it to complete.",
"error_code": "generation_in_progress",
"token": existing_token,
}), 409

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The duplicate-worker guard relies on reading session["progress_token"], which is only updated at the end of the request. Two truly concurrent /generate_chapters requests from the same session can both observe no progress_token (or a stale one) and each start a new worker, so this doesn’t fully prevent duplicates under real concurrency.

Consider making the check-and-reserve atomic using a server-side key (e.g., session_id = get_session_id()), protected by _progress_lock: maintain a session_id -> active_token mapping (or store session_id in each progress record and look it up) and set it before spawning the thread. Also keep the status check within the same lock to avoid TOCTOU edge cases where status flips between read and decision.

Copilot uses AI. Check for mistakes.
@CyberSecDef
CyberSecDef merged commit c93fa85 into main Apr 3, 2026
6 checks passed
@CyberSecDef
CyberSecDef deleted the copilot/prevent-duplicate-chapter-workers branch April 3, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prevent duplicate chapter-generation workers for the same session or outline state

3 participants