Issue #506: feat: Make logout summarization non-blocking

[dereck-symmetry]

Description of Change

These change fix issue #506. We add a asynch io call and return immediately from a logout call.

Closes #506

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality
  to not work as expected)
• Documentation update
• Infrastructure/deployment change
• Performance improvement
• Code refactoring

Project Area(s) Affected

• bases/
• components/
• projects/
• orchestrators/
• frontends/
• deployments/
• cloudformation/ or sam/ templates
• reference_data/
• scripts/
• test/ or e2e/
• Database schema (migrations)
• API endpoints
• Documentation (docs/, READMEs, ARCHITECTURE.md, CLAUDE.md)

Checklist

• commit message follows commit guidelines (see commitlint.config.mjs)
• tests are included (unit and/or integration tests)
• documentation is changed or added (in /docs directory)
• code passes linting checks (uv run ruff check)
• code passes formatting checks (uv run ruff format)
• code passes type checking (uv run ty check)
• pre-commit hooks have been run successfully
• database schema changes: migration files created and CHANGELOG.md updated
• API changes: base (Python code) documentation in docs/
  and project README updated
• configuration changes: relevant folder README updated
• breaking changes: added to MIGRATION.md with upgrade instructions
  and CHANGELOG.md entry

Testing

• Manual testing performed
• Automated tests added/updated
• Integration testing completed

Additional Notes

Risks are very low that the logout will not complete. That would only happen if the logout background task is incomplete when there is a server restart.

Testing and code checks done with Claude.

[bjagg]

Overview

Makes /logout return immediately by moving interaction summarization into a background task, fixing #506 ("kick off background task"). Small and focused, and the error handling is right: _safe_summarize catches broadly but uses logger.exception rather than a bare pass, so failures are still visible. test_logout_succeeds_even_when_summarization_fails is exactly the test I'd want here — it proves the user-facing path is genuinely decoupled from LLM availability.

Two things I'd like changed before merge, and a couple of notes.

1. The task reference is discarded

asyncio.create_task(_safe_summarize(agent, task, username))

The return value isn't kept. The event loop holds only a weak reference to a running task, so this can be garbage-collected mid-execution — the CPython docs call this out directly and recommend saving a reference. The failure mode is nasty precisely because it's rare and load-dependent: summaries silently vanish some fraction of the time, with nothing in the logs.

Minimal fix:

_background_tasks: set[asyncio.Task] = set()

task_ref = asyncio.create_task(_safe_summarize(agent, task, username))
_background_tasks.add(task_ref)
task_ref.add_done_callback(_background_tasks.discard)

2. FastAPI's BackgroundTasks is the better fit here

FastAPI has a first-class mechanism for "do this after the response is sent":

async def logout(background_tasks: BackgroundTasks, username: str = Depends(get_current_user)):
    ...
    background_tasks.add_task(_safe_summarize, agent, task, username)

It solves the reference problem for you, and — the part I care more about — it makes the tests deterministic. TestClient runs background tasks to completion before returning, so the two await asyncio.sleep(0.1) calls disappear.

Those sleeps are the second thing I'd change. A fixed 100ms as a synchronization barrier is timing-dependent and will flake under CI load; grep says these would be the only such sleeps in the whole test suite. If BackgroundTasks isn't wanted for some reason, the alternative is to capture the task and await it in the test rather than sleeping.

Worth noting the repo doesn't use BackgroundTasks anywhere yet, so either way this establishes a pattern — which is an argument for establishing the framework-idiomatic one.

3. Nothing survives shutdown — and this weakens #1118

bases/lif/advisor_restapi/core.py has no on_event("shutdown") or lifespan handler. When an ECS task stops (a deploy, a scale-in, a health-check failure), any in-flight summarization is killed and the interaction summary is lost with no trace beyond an absent log line.

That matters because #1118 already tracks interaction-summary capture being best-effort and logout-only. This change makes the window wider: previously the summary completed before the response returned, so a user who logged out successfully had their summary saved. Now a successful logout no longer implies that.

Not asking you to solve durability here — that's #1118's job. But it's worth a line in the PR description or a Refs #1118, because this trades a latency win for a durability loss and that trade should be visible rather than discovered later.

4. The TODO(#986) marker was dropped

The prompt string moved into _safe_summarize, but the comment went with it:

# TODO(#986): move this hard-coded query prompt to env/config

#986 is still open. That comment was its only in-code anchor — worth carrying it over to the new location.

Verdict

Approve once items 1 and 2 are addressed — the unretained task is a real (if intermittent) correctness issue, and the sleeps will cost someone a flaky-CI investigation. Items 3 and 4 are notes, not blockers.

The core change is right and the failure-path test is a good addition.