Skip to content

resume_session doesn't drain pending background tasks — stale extraction/comprehension results can be applied to the resumed session #314

Description

@hherb

Found during the PR #306 code review (finding 7 of the report-only list). Latent today — neither current caller triggers it — but the Arc<Mutex<DialogueManager>> API shape makes it a real footgun for any future long-lived host.

The behaviour

resume_session (src/crates/primer-pedagogy/src/dialogue_manager/lifecycle.rs) resets last_break_suggested_at but does not take()/drain classify_task / post_response_task the way close_session does. If a host calls respond_to_streaming and then resume_session on the same manager, the next turn's await_pending_background applies the old session's PostResponseResult:

  • merge_concepts_into_turn writes the old exchange's concepts into self.session.turns[child_idx/primer_idx] of the newly loaded session — the indices are in-bounds for any resumed session with ≥2 turns, so the out-of-bounds no-op guard doesn't help;
  • apply_extraction bumps learner.concepts encounter counts for an exchange belonging to a different session.

Also, last_extraction / last_comprehension are documented as "cleared on session lifecycle events" but are never cleared outside new().

Why it doesn't bite today

The CLI resumes immediately after constructing the manager; the GUI builds a fresh ActiveSession/DM per resume. Verified at both call sites during the review.

Suggested resolution

Have resume_session drain/abort both pending task slots (same shape as close_session) and clear last_extraction/last_comprehension, plus a test that spawns a post-response task, resumes, and asserts the old result is not applied.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions