fix(host-sweep): orphan-claim delete missed in tests (regression from #2183)#2209
Merged
gavrielc merged 4 commits intoqwibitai:mainfrom May 5, 2026
Merged
Conversation
github-actions
Bot
added
follows-guidelines
cfis
force-pushed
the
fix/host-sweep-test-uses-in-memory-db
branch
from
b84700f to
df00e08
Compare
cfis
changed the title
…ssed in tests qwibitai#2183 added orphan-claim cleanup that reopens `outbound.db` by session path (`openOutboundDbRw(session.agent_group_id, session.id)`) so the delete runs against a writable handle even when callers pass a readonly one. That works for the production caller — there's a real on-disk session DB at the expected path. The test wrapper `_resetStuckProcessingRowsForTesting` (introduced in the same series, qwibitai#2151) is called with in-memory DBs that have no on-disk path. The reopen creates a fresh empty file at `<DATA_DIR>/v2-sessions/ag-test/sess-test/outbound.db`, runs the delete against that, and leaves the in-memory `outDb` (which the test reads afterward) untouched. The two `resetStuckProcessingRows — orphan claim cleanup` tests assert `getProcessingClaims(outDb).toEqual([])` after the call and fail on the row that's still there. Fix: drop the `_…ForTesting` wrapper, export `resetStuckProcessingRows` directly with an optional `writableOutDb` parameter. When omitted (production), the function reopens `outbound.db` RW by session path — existing behavior, existing safety guarantee. When provided (tests, or any future caller that already holds a writable handle), the function uses it directly and skips the reopen. The optional parameter has a real meaning, not a "for tests" hack. Public API surface change: `_resetStuckProcessingRowsForTesting` is gone, `resetStuckProcessingRows` is now exported. No other callers inside the repo besides the test.
cfis
force-pushed
the
fix/host-sweep-test-uses-in-memory-db
branch
from
df00e08 to
e4181f5
Compare
4 tasks
This was referenced May 4, 2026
The test wrapper forwards the in-memory outDb as the writable handle, avoiding the filesystem reopen that fails in CI. The function stays private — the optional writableOutDb param is an internal detail, not a public API. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
|
@cfis Thank you for the fix!! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type of Change
.claude/skills/<name>/, no source changes)Description
#2183 added orphan-claim cleanup that reopens
outbound.dbby session path (openOutboundDbRw(session.agent_group_id, session.id)) so the delete runs against a writable handle even when callers pass a readonly one. That works for the production caller — there's a real on-disk session DB at the expected path.The test wrapper
_resetStuckProcessingRowsForTesting(added in #2151) is called with in-memory DBs that have no on-disk path. After #2183 the reopen creates a fresh empty file at<DATA_DIR>/v2-sessions/ag-test/sess-test/outbound.db, runs the delete against that, and leaves the in-memoryoutDb(which the test reads afterward) untouched. The tworesetStuckProcessingRows — orphan claim cleanuptests assertgetProcessingClaims(outDb).toEqual([])after the call and fail on the row that's still there.Fix
Drop the
_…ForTestingwrapper, exportresetStuckProcessingRowsdirectly with an optionalwritableOutDbparameter:outbound.dbRW by session path — existing behavior, existing safety guarantee.The optional parameter has a real semantic meaning, not a "for tests" hack — a future caller who already has a writable handle can avoid the reopen too.
Verification
All 26 host test files / 246 tests pass; previously the two
host-sweeptests were failing on every run.Public API
_resetStuckProcessingRowsForTestingis removed;resetStuckProcessingRowsis now exported. No other in-repo callers besides the test.