fix(client): remove obsolete device-registry cleanup task

[Bot-Dev-RPA]

The background device-registry task retained a strong Arc<Client>, which kept the client and its SQLite store handle alive after the last consumer reference was dropped. On Windows, deleting the session database could then race the delayed task teardown and fail because the file was still open.

The real registry cleanup was removed during the storage-trait simplification in #204, but its shutdown-only placeholder and detached spawn remained. Remove both instead of replacing the captured Arc with Weak: there is no cleanup work left to preserve, and deleting the task also avoids leaving an orphaned shutdown waiter when a client is dropped without signaling shutdown.

Add dropping_fresh_client_releases_it_without_shutdown as a lifecycle regression test. On the current main, it times out with one background-owned strong reference remaining; with this fix, the client and persistence handles are released promptly.

Validation:

• cargo fmt --all -- --check
• cargo clippy --all --tests
• cargo test --workspace --exclude e2e-tests
• cargo test --all was also attempted; the E2E package failed with Connection refused because its required mock server was not running.

[coderabbitai]

📝 Walkthrough

Walkthrough

The device-registry cleanup loop and its constructor-spawned task are removed. A Tokio integration test verifies that dropping the last Arc<Client> releases the client without explicit shutdown.

Changes

Client cleanup lifecycle

Layer / File(s)	Summary
Remove device-registry cleanup task src/client/device_registry.rs, src/client/lifecycle.rs	Removes the cleanup-loop stub and stops starting it from Client::new_with_cache_config.
Validate client release src/client/tests.rs	Adds a test that drops a fresh client and waits for its weak reference to reach zero strong references.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• oxidezap/whatsapp-rust#1061: Related client lifecycle refactoring for device-registry cleanup task startup.

Suggested reviewers: jlucaso1, greptile-apps

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly matches the main change: removing the obsolete device-registry cleanup task.
Description check	✅ Passed	The description is directly related to the device-registry cleanup task removal and the lifecycle regression test.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR removes an obsolete background task that retained the client during teardown. The main changes are:

• Removes the device-registry shutdown waiter.
• Stops spawning its detached background task.
• Adds a test that checks a fresh client is released after its last owner is dropped.

Confidence Score: 5/5

This looks safe to merge.

• The removed task only waited for shutdown and performed no cleanup.
• Removing its spawn eliminates the client-retention path.
• No blocking issues were found in the updated code.

Important Files Changed

Filename	Overview
src/client/device_registry.rs	Removes the shutdown-only device-registry cleanup placeholder.
src/client/lifecycle.rs	Stops spawning the detached task that retained the client.
src/client/tests.rs	Adds coverage for releasing a fresh client without an explicit shutdown.

_{Reviews (3): Last reviewed commit: "fix(client): remove obsolete registry cl..." | Re-trigger Greptile}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/client/device_registry.rs (1)

996-1002: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Ship the guardrail with a test, not just a comment.

This function's whole reason for existing (per this PR) is the fast-exit-on-dropped-client path. Right now nothing in the test module actually exercises client.upgrade() returning None. A one-line regression test locks in the behavior this PR is delivering, and it's cheap to write since device_registry_cleanup_loop is already reachable from #[cfg(test)] mod tests via use super::*;.

✅ Suggested test

#[tokio::test]
async fn cleanup_loop_exits_immediately_when_client_already_dropped() {
    let client = create_test_client().await;
    let weak = Arc::downgrade(&client);
    drop(client);

    tokio::time::timeout(
        std::time::Duration::from_millis(200),
        Client::device_registry_cleanup_loop(weak),
    )
    .await
    .expect("cleanup loop must exit immediately when the client is already gone");
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/client/device_registry.rs` around lines 996 - 1002, Add a Tokio test in
the existing tests module named
cleanup_loop_exits_immediately_when_client_already_dropped that creates a test
client, downgrades and drops it, then awaits
Client::device_registry_cleanup_loop with a short timeout and asserts
completion. This must exercise the client.upgrade() == None path and verify the
loop exits promptly.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/client/device_registry.rs`:
- Around line 996-1002: Add a Tokio test in the existing tests module named
cleanup_loop_exits_immediately_when_client_already_dropped that creates a test
client, downgrades and drops it, then awaits
Client::device_registry_cleanup_loop with a short timeout and asserts
completion. This must exercise the client.upgrade() == None path and verify the
loop exits promptly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e10add80-367f-4a5e-b9d6-546f5c351aca

📥 Commits

Reviewing files that changed from the base of the PR and between 5c02695 and b4cf497.

📒 Files selected for processing (2)

• src/client/device_registry.rs
• src/client/lifecycle.rs

[jlucaso1]

@Bot-Dev-RPA nice catch. I'll review this soon. Thank you!

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

[jlucaso1]

Reproduced the client-retention bug on the current main with a lifecycle regression test: after the last consumer Arc was dropped, the obsolete device-registry task kept one strong reference alive. The original Weak fix released the client, but retained a shutdown-only detached task whose real cleanup work was removed in #204. The follow-up removes the dead method and spawn entirely and keeps the behavioral regression test.

Validated with cargo fmt --all -- --check, cargo clippy --all --tests, and cargo test --workspace --exclude e2e-tests. The E2E-only run requires the mock server and failed locally with the expected connection-refused errors when it was absent.