MAESTRO: Close PR #899 - too broad, overlaps with merged PRs #693, #937, #940

Reviewed PR #899 (multi-session message processing fix). Identified significant
overlap with 3 recently merged PRs causing merge conflicts in 3 files. Closed
with detailed feedback requesting focused re-submission of the genuinely valuable
parts: AbortController stale-check, FK constraint protection, and queueDepth fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Alex Newman
2026-02-06 01:51:52 -05:00
parent dd753184db
commit 9fd32ad44b
+2 -1
View File
@@ -18,7 +18,8 @@ These PRs fix various session lifecycle issues — orphaned messages, provider r
- [x] Review PR #937 (`fix(worker): gracefully process orphaned pending messages after session termination` by @jayvenn21). Files: `src/services/sqlite/PendingMessageStore.ts`, `src/services/worker-service.ts`, `src/services/worker/SessionManager.ts`. Steps: (1) `gh pr checkout 937` (2) Review — orphaned messages should be processed or discarded cleanly, not stuck forever (3) Run `npm run build` (4) If clean: `gh pr merge 937 --rebase --delete-branch`
- **MERGED** (2026-02-06): Clean +125/-3 line fix addressing issue #936 (51+ orphaned messages/day). Adds `isSessionTerminatedError()` to detect SDK resume failures from closed terminals. On failure, falls back to Gemini/OpenRouter agents if available to drain the queue. If no fallback, `markAllSessionMessagesAbandoned()` marks pending/processing messages as failed and `removeSessionImmediate()` cleans up the session without deadlocking on the generator promise. Build passes. Rebased cleanly onto main.
- [ ] Review PR #899 (`fix: resolve message processing failures in multi-session scenarios` by @hahaschool). Files: 7 files including SessionStore, SDKAgent, ResponseProcessor, SessionRoutes. Steps: (1) `gh pr checkout 899` (2) This is a broader fix — review carefully for scope creep (3) Check that multi-session message routing is correct (messages go to the right session) (4) Run `npm run build` (5) If focused and correct: `gh pr merge 899 --rebase --delete-branch`. If too broad, request scope reduction.
- [x] Review PR #899 (`fix: resolve message processing failures in multi-session scenarios` by @hahaschool). Files: 7 files including SessionStore, SDKAgent, ResponseProcessor, SessionRoutes. Steps: (1) `gh pr checkout 899` (2) This is a broader fix — review carefully for scope creep (3) Check that multi-session message routing is correct (messages go to the right session) (4) Run `npm run build` (5) If focused and correct: `gh pr merge 899 --rebase --delete-branch`. If too broad, request scope reduction.
- **CLOSED** (2026-02-06): Too broad — conflicts in 3 files (`worker-service.ts`, `worker-types.ts`, `SDKAgent.ts`) due to overlap with recently merged PRs #693, #937, and #940. Crash recovery logic already addressed by #693 (restart limits with exponential backoff) and #937 (orphaned message fallback + session termination detection). Requested author re-submit as focused PRs for the genuinely valuable parts: (1) AbortController stale-check at generator start (~10 lines, real bug), (2) FK constraint protection in SDKAgent/ResponseProcessor (prevents FK violation when SDK generates new session ID after restart), (3) queueDepth logging fix using `pendingStore.getPendingCount()` instead of deprecated `session.pendingMessages.length`.
- [ ] Review PR #627 (`fix: Reset AbortController before starting generator` by @TranslateMe). File: `src/services/worker/http/routes/SessionRoutes.ts`. Steps: (1) `gh pr checkout 627` (2) Old PR (Dec 27) — check if still applicable after v8.5.2 memory leak fix (3) If the abort controller reset is still needed: rebase and merge. If already handled: close.