Repository navigation
[Bug] Atomically deliver S15 background results - #525
Bill-Billion merged 2 commits into
Conversation
Bill-Billion
left a comment
Bill-Billion
left a comment
There was a problem hiding this comment.
Thanks for fixing the two atomicity gaps: moving ID allocation plus registration into one critical section and claiming completed results under the same lock is the right approach. Before merging, please run cd web && npm run extract and commit the updated web/src/data/generated/versions.json (the full suite currently has one stale-snapshot failure and becomes 208 passed, 84 subtests passed after regeneration), and add a deterministic concurrent-start regression test proving two callers cannot receive the same background ID, since the new test only covers concurrent consumers.
Please also make the background-task lifecycle fail-safe: a PostToolUse hook exception must not bypass the terminal status/result write, and a Thread.start() exception should roll back or mark the registered task failed, as S11 already does; both paths currently leave a permanent running entry with no deliverable result and can indefinitely block worktree removal.
Submitted by mistake; withdrawing this review.
Fixes #524
Problem
Concurrent consumers can race while collecting completed S15 background tasks, allowing duplicate or inconsistent result delivery. Concurrent task starts also allocate IDs separately from task registration.
Changes
Verification
python -m py_compile s15_integrated_harness/code.py tests/test_agent_teams_runtime.pyScope
Only
s15_integrated_harness/code.pyand its focused test are included. Windows-only compatibility is outside scope.