Skip to content

fix(host-sweep): inject DbRw opener so in-memory tests don't fail silently#2210

Closed
robbyczgw-cla wants to merge 1 commit intoqwibitai:mainfrom
robbyczgw-cla:fix/host-sweep-test-fixture
Closed

fix(host-sweep): inject DbRw opener so in-memory tests don't fail silently#2210
robbyczgw-cla wants to merge 1 commit intoqwibitai:mainfrom
robbyczgw-cla:fix/host-sweep-test-fixture

Conversation

@robbyczgw-cla
Copy link
Copy Markdown
Contributor

Summary

resetStuckProcessingRows() (called from the regression-protected claim-stuck path) reopens outbound.db by filesystem path via openOutboundDbRw(agent_group_id, session_id) to drop orphan processing claims after killing a stuck container.

The 2 host-sweep regression tests (should clear orphan claims even when message is past process_after and still clears orphan claims even when the inbound message has already been retried (skip path), both added in 7ce9922) call _resetStuckProcessingRowsForTesting with an in-memory outDb and a fakeSession() whose path doesn't exist on disk — the reopen fails, the error is silently caught in the try/catch, the orphan claim stays in place, and the assertion expect(getProcessingClaims(outDb)).toEqual([]) fails on a clean checkout.

The bug is in the test fixture's contract with the function under test, not in the function's runtime behaviour. Production paths exist on disk, so the reopen succeeds there.

Fix

Make the rw-opener injectable. _resetStuckProcessingRowsForTesting passes () => outDb so the delete runs on the caller's handle; production keeps openOutboundDbRw as the default. Don't close the test-supplied handle in the finally — the caller still owns it.

Verification

$ git checkout main
$ pnpm test src/host-sweep.test.ts
 Test Files  1 passed (1)
      Tests  12 passed | 2 failed (14)   ← reproduction

$ git checkout fix/host-sweep-test-fixture
$ pnpm test src/host-sweep.test.ts
 Test Files  1 passed (1)
      Tests  14 passed (14)              ← fixed
$ pnpm test
      Tests  246 passed (246)            ← full suite green

Tested on upstream/main@953264e0 (v2.0.27).

Test plan

  • Reproduce 2-failing-tests on clean checkout
  • Verify fix passes all 14 host-sweep tests
  • Verify full suite (246 tests) still green
  • Typecheck clean

…ently

resetStuckProcessingRows() reopens outbound.db by filesystem path via openOutboundDbRw(agent_group_id, session_id) to drop orphan processing claims after killing a stuck container. The 2 host-sweep regression tests (added in 7ce9922) call _resetStuckProcessingRowsForTesting with an in-memory outDb and a fakeSession() whose path doesn't exist on disk — the reopen fails, the error is silently caught, the orphan claim stays in place, and the assertion 'expect(getProcessingClaims(outDb)).toEqual([])' fails on a clean checkout.

Make the rw-opener injectable. The test path passes () => outDb so the delete runs on the caller's handle; production keeps openOutboundDbRw as the default. Don't close the test-supplied handle in the finally — the caller still owns it.

Verified: all 14 host-sweep.test.ts tests pass on clean upstream/main@953264e. Full suite remains green.

Co-Authored-By: Andy (Claude Opus 4.7) <noreply@anthropic.com>
@robbyczgw-cla
Copy link
Copy Markdown
Contributor Author

Closing as duplicate of #2209 — same bug (host-sweep regression tests fail because in-memory outDb gets reopened by filesystem path that does not exist for fakeSession()), same fix concept (make the rw-DB handle injectable for the test path). #2209 was opened first (4h prior) and has a marginally cleaner API (passes the DB directly vs. an opener function); going with that one. Apologies for the duplication — should have grep-checked open PRs before starting.

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