test(dashboard): give the slot-close race double the body surface read_bounded_json reads

[iamwhatever]

Problem / Motivation

test/test_slot_close_recreation_race.py fails on every PR whose merge commit carries it, on both Backend Tests shard-4 lanes, and it is blocking unrelated PRs from reaching green (#8518).

Every one of the eleven test_cleanup_* cases is broken, and only one of them says why:

FAILED test_cleanup_ordinary_archive_still_saves_and_removes - AttributeError: '_Req' object has no attribute 'can_read_body'
FAILED test_cleanup_recreate_during_save_preserves_replacement - Failed: Timeout >120.0s
FAILED test_cleanup_first_guard_hands_the_marker_to_the_replacement - worker 'gw2' crashed

api_chat_slots_cleanup reads its body through read_bounded_json, whose very first statement is if allow_absent and not request.can_read_body. The _Req test double in this file only stubs json(), so the handler raises AttributeError before it does anything else.

Ten of the eleven never report that error. They arm a live turn, launch the handler as a task, and then park on an asyncio.Event that the monkeypatched save_slot_off_loop sets — but the handler now raises before it reaches that save, so the event never fires and the test blocks until its own deadline. On Linux that is Timeout >120.0s; on Windows the xdist worker is torn down and the failure reads worker 'gwN' crashed.

Why it matters

The one-line cause was invisible and the cost was not: the file alone burned 181s locally and ~20 minutes of CI wall-clock per lane, and produced 15 failures across the two lanes in #8518's evidence — none of which named the defect. Because it fails on the merge ref rather than on a branch, it reds PRs that never touched the dashboard, and the loudest symptom (a crashed worker) points at the harness instead of at a missing attribute on a fake.

What changed (motivation → approach → change)

Symptom: eleven test_cleanup_* failures with three different shapes. Root cause: one — api_chat_slots_cleanup was migrated onto read_bounded_json (the consolidation in #5587's line of work), and the test double was left stubbing only json(). The three shapes are one AttributeError plus ten tests whose parking event is downstream of where it now raises.

The fix is to give _Req the request surface the helper actually reads, rather than the one surface the handler used to read:

• can_read_body — the first thing read_bounded_json touches, and the whole failure.
• content_length / content / charset — the capped path (max_bytes defaults to 64 KiB) streams the body off request.content.iter_chunked instead of calling request.json(), so a double that stubs only json() would read an empty body there even once the attribute exists. A small _Stream chunks at the helper's own 8 KiB step.
• json() is kept: the max_bytes=None path still calls it.

Every field is derived from the body the double was constructed with, so the streaming path sees exactly the bytes json() would have returned. body is None (what all 42 call sites in this file pass) is no body and reads as can_read_body == False; an explicitly-passed {} is a body that is present and empty, which is what aiohttp would report as readable — keying off truthiness instead would have made the double lie about that case.

Scope is deliberately the double, not the handler: read_bounded_json is correct, and api_chat_slot_delete — the other handler these tests drive, and the source of the 31 cases that were already passing — never reads the body at all.

Tests

No new tests. This repairs the 42 existing assertions in test/test_slot_close_recreation_race.py, which pin the close-vs-recreate teardown guards from #7212 and were all unreachable on the cleanup half.

Before and after, same file, same machine:

	result	wall-clock
before	10 failed (1 AttributeError, 9 Timeout >120.0s)	181.43s
after	42 passed	1.52s

That before/after is the proof the change is load-bearing; there is no production hunk to revert, so prove.py has nothing to prove here.

Manual verification

N/A — unit coverage sufficient; the repaired assertions are the verification.

Local gates run on this diff, all green: run_scoped_tests --test, leaf_test_scope --test, check_black_formatting, isort, flake8, mypy src/kiro_crew/, subprocess-encoding, agent-SDK-boundary, sync-IO-in-async, lockdown-before-publish, brand-name, harness-parity, feature-map, loop-bound-locks, builtin-skill-scope, testpaths-coverage, changelog-history, focus-cue, docs-lint, per-file-coverage, scrub-lint. Frontend/deploy gates were not run: the diff touches no frontend, template or deploy path.

The full backend suite was also run on this branch and on pristine origin/main. test_slot_close_recreation_race.py contributes zero failures on this branch. The residual reds are host-environment artifacts that reproduce identically on the base — the runner's real $HOME (test_host_isolation_floor, test_file_explorer_app), uid 65534 on /local/home (test_service), AF_UNIX path too long (test_dashboard_peer_auth), host CPU count (test_xdist_host_budget), and an absent gh binary (test_issue_radar_gh_bin). The failure sets on branch and base are identical.

Related Issues

Fixes #8518

Pattern harvest

Rule candidate: review-prompt
Pattern: migrating a handler onto a shared request-reading helper without updating the hand-rolled request doubles that drive it — the helper reads attributes (can_read_body, content, content_length, charset) the old await request.json() never touched, so a double that stubs only json() breaks, and it breaks at the top of the handler, upstream of any event a test parks on. That ordering is what turns a one-line AttributeError into a fleet of 120s timeouts and crashed workers. read_bounded_json's own docstring already warns that the capped path moves a caller off request.json() and that "that handler's unit tests must feed content/content_length rather than mocking json" — the note exists and was not applied at this call site, so the generalizable rule is to treat the docstring's warning as a checklist item on every conversion, and to look for hand-rolled _Req-style doubles in the converted handler's test files.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of f710b131caaa34ae6c042d8c1ac4f507d8630c14 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Test-only double widened to the exact surface read_bounded_json reads, following that helper's own documented testing contract — proportionate and sound.

[DESIGN-REVIEWED] f710b13

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of f710b131caaa34ae6c042d8c1ac4f507d8630c14 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] f710b13

False positive or not applicable? A repository writer can comment:
/ai-review override gpt f710b131caaa34ae6c042d8c1ac4f507d8630c14: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed f710b131caaa34ae6c042d8c1ac4f507d8630c14 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] f710b13

Verdict parsed from the review's SHA-scoped output markers for commit f710b131caaa34ae6c042d8c1ac4f507d8630c14.

False positive or not applicable? A repository writer can comment:
/ai-review override fable f710b131caaa34ae6c042d8c1ac4f507d8630c14: <one-sentence reason>