Skip to content

feat: event-driven thread wait coordinator (BM-52) - #1

Closed
jonolee-kr wants to merge 3 commits into
mainfrom
bm-52-wait-coordinator
Closed

feat: event-driven thread wait coordinator (BM-52)#1
jonolee-kr wants to merge 3 commits into
mainfrom
bm-52-wait-coordinator

Conversation

@jonolee-kr

Copy link
Copy Markdown
Owner

Summary

Fixes excessive memory and request load from bb thread wait (BM-52).

  • Server: new reference-counted wait coordinator keyed by (threadId, target, cursor). Identical waits share one NotificationHub subscription and one coalesced DB re-check. Per-caller deadlines, abort cleanup, caller limits, pino observability with an unref()ed summary timer, and a stats accessor.
  • Route: new GET /api/v1/threads/:id/wait?status=... long-poll (404 thread_not_found for missing threads). The existing events/wait route is refactored onto the coordinator with its response contract preserved.
  • SDK: status waits long-poll the new route. Fallback to the legacy 250 ms polling loop triggers only on BbHttpError status 404 + code not_found (route absent on older servers). The wait sleep is now abort-aware. pollIntervalMs governs only the fallback path.
  • CLI: bb thread wait --output (one blocking follow returning terminal status + final output), bb thread wait-many, and lazy-loaded thread subcommands. Existing wait messages, JSON shapes, and exit codes unchanged.
  • Guidance/docs: builtin skills no longer teach log/wait alternation; changelog documents rollout compatibility; QA runbook adds PSS measurement.

Measured impact

  • Old CLI waiter: 150–174 MiB RSS / 99–122 MiB PSS each, 4 status GETs/s per waiter (~6 DB ops each).
  • New CLI waiter: 48 MiB RSS / 12 MiB PSS (standalone).
  • Live demo against this branch's server: 20 concurrent CLI waiters → 1 coordinator entry, 20 total HTTP requests for the whole wait, one shared DB re-check on the status flip, all resolved within 77–133 ms, zero leaks. Old CLI in the same harness: 3.8 GETs/s per waiter.

Testing

  • apps/server: 2083 passed / packages/sdk: 102 passed / apps/cli: 513 passed, 0 failed, no new skips (independently re-run by a second reviewer).
  • Five deliberate mutations (broken dedup, shared abort, broad fallback, non-abortable sleep, wait-after-output) are each caught by the new tests.
  • 100-duplicate-wait stress tests assert one entry, bounded checks, and full cleanup.

Plan, review artifacts, and demo report are attached to task BM-52.

🤖 Generated with Claude Code

@jonolee-kr

Copy link
Copy Markdown
Owner Author

Superseded by upstream PR get-bb#2688.

@jonolee-kr jonolee-kr closed this Aug 29, 2026
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.

1 participant