Skip to content

test(worker): guard in-memory buffer reclaim#2951

Draft
anavarro-bb wants to merge 1 commit into
thedotmack:mainfrom
anavarro-bb:agent/lib284-worker-drain
Draft

test(worker): guard in-memory buffer reclaim#2951
anavarro-bb wants to merge 1 commit into
thedotmack:mainfrom
anavarro-bb:agent/lib284-worker-drain

Conversation

@anavarro-bb

Copy link
Copy Markdown

Summary

  • Add regression coverage for SessionMessageBuffer.resetClaimed() re-yielding an in-flight message without requiring a durable pending_messages.status='processing' row.
  • This guards the worker queue failure class observed in Liberty LIB-284: orphaned durable processing rows wedging the drain.

Context

The live Liberty box is running claude-mem plugin v10.6.2, where the installed generated worker bundle still has the durable pending queue behavior. Upstream main (v13.6.1) has already moved the worker to the in-memory SessionMessageBuffer path, which removes the cross-process orphaned processing row failure mode. This PR keeps that behavior reviewable and locked down with a focused test instead of proposing a stale patch against the old generated bundle.

Verification

  • /Users/alex/.bun/bin/bun test tests/services/worker/session-message-buffer.test.ts
  • git diff --check

Not done

  • Did not apply or restart the live Liberty claude-mem worker.
  • Did not mutate the live sqlite queue.
  • Did not merge this PR.

Add a focused regression test for re-yielding an in-flight SessionMessageBuffer message after resetClaimed without relying on a durable processing row. This guards the queue failure class observed in LIB-284 while current main uses the in-memory buffer path.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a single regression test to lock down the SessionMessageBuffer.resetClaimed() re-yield path — the in-memory equivalent of the orphaned pending_messages.status='processing' row failure observed in Liberty (LIB-284). No production code is changed.

  • New test (resetClaimed re-yields an in-flight message without a durable processing row): creates a live drain iterator, enqueues a message, claims it via iterator.next(), calls resetClaimed, and asserts the same message is re-yielded by a second iterator.next() call before confirming and aborting.
  • The test correctly exercises the step-by-step iterator API rather than the drainAll helper, which is appropriate for verifying mid-stream reclaim behaviour.

Confidence Score: 4/5

Safe to merge — only a test file is added, no production code changes.

The new test correctly exercises the resetClaimed re-yield path and the assertions are accurate. The one note is that idleTimeoutMs: 10_000 means a regressed resetClaimed would cause the test to block for 10 seconds before failing, whereas all other tests in the file use 25–30 ms timeouts for the same purpose.

tests/services/worker/session-message-buffer.test.ts — the idle timeout value is worth revisiting.

Important Files Changed

Filename Overview
tests/services/worker/session-message-buffer.test.ts Adds a focused regression test for resetClaimed re-yielding an in-flight message; logic is correct but the 10-second idle timeout could make failure output very slow compared to the 25–30 ms used in every other test in the file.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant T as Test
    participant B as SessionMessageBuffer
    participant G as drain generator

    T->>B: "drain sessionDbId=1"
    T->>B: enqueue Read/tool-1
    T->>G: iterator.next first
    activate G
    G->>B: "claimNext -> message claimed=true"
    G-->>T: "done=false value=messageId"
    deactivate G
    T->>B: resetClaimed
    Note over B: claimed=false signal emitted no listener
    T->>G: iterator.next second
    activate G
    Note over G: resumes from yield loops to claimNext
    G->>B: "claimNext -> same message claimed=true"
    G-->>T: "done=false value=messageId"
    deactivate G
    T->>B: confirm messageId
    Note over B: message removed getPendingCount=0
    T->>G: controller.abort
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant T as Test
    participant B as SessionMessageBuffer
    participant G as drain generator

    T->>B: "drain sessionDbId=1"
    T->>B: enqueue Read/tool-1
    T->>G: iterator.next first
    activate G
    G->>B: "claimNext -> message claimed=true"
    G-->>T: "done=false value=messageId"
    deactivate G
    T->>B: resetClaimed
    Note over B: claimed=false signal emitted no listener
    T->>G: iterator.next second
    activate G
    Note over G: resumes from yield loops to claimNext
    G->>B: "claimNext -> same message claimed=true"
    G-->>T: "done=false value=messageId"
    deactivate G
    T->>B: confirm messageId
    Note over B: message removed getPendingCount=0
    T->>G: controller.abort
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
tests/services/worker/session-message-buffer.test.ts:78-82
**Slow failure on regression**`idleTimeoutMs: 10_000` means that if `resetClaimed` ever fails to un-claim the message, `second = await iterator.next()` will block for a full 10 seconds before the idle timeout fires and the generator exits with `done: true`. The test would then fail on `expect(second.done).toBe(false)`, but only after a 10-second hang. All other tests in this file use 25–30 ms idle timeouts with a matching `onIdleTimeout` callback; adopting the same pattern here keeps CI feedback fast.

Reviews (1): Last reviewed commit: "test(worker): cover in-memory buffer rec..." | Re-trigger Greptile

Comment on lines +78 to +82
const iterator = buffer.drain({
sessionDbId: 1,
signal: controller.signal,
idleTimeoutMs: 10_000,
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Slow failure on regressionidleTimeoutMs: 10_000 means that if resetClaimed ever fails to un-claim the message, second = await iterator.next() will block for a full 10 seconds before the idle timeout fires and the generator exits with done: true. The test would then fail on expect(second.done).toBe(false), but only after a 10-second hang. All other tests in this file use 25–30 ms idle timeouts with a matching onIdleTimeout callback; adopting the same pattern here keeps CI feedback fast.

Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/services/worker/session-message-buffer.test.ts
Line: 78-82

Comment:
**Slow failure on regression**`idleTimeoutMs: 10_000` means that if `resetClaimed` ever fails to un-claim the message, `second = await iterator.next()` will block for a full 10 seconds before the idle timeout fires and the generator exits with `done: true`. The test would then fail on `expect(second.done).toBe(false)`, but only after a 10-second hang. All other tests in this file use 25–30 ms idle timeouts with a matching `onIdleTimeout` callback; adopting the same pattern here keeps CI feedback fast.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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