api: support channel archiving and channel list filters

[txchen]

Summary

This PR adds non-destructive channel management for large deployments with many threads/channels.

What’s added

1. GET /api/channels now supports optional query filters:

• include_inactive (bool): include archived/inactive channels
• agent_id (string): restrict results to a single agent
• is_active (bool): return only active or only inactive channels

2. New endpoint to archive/unarchive without deleting history:

• PUT /api/channels/archive
• body:
  • agent_id (string)
  • channel_id (string)
  • archived (bool)

3. ChannelStore enhancements:

• list(include_inactive: bool)
• set_active(channel_id, active)

list_active() remains and now delegates to list(false) for backward compatibility.

Why

Deleting channels currently removes message history, which is too destructive for UI/thread-management use cases.
This PR enables hiding old channels from normal listings while retaining history.

Behavior notes

• Archiving sets channels.is_active = 0; unarchiving sets it back to 1.
• Existing delete behavior is unchanged (DELETE /api/channels still deletes channel + history).
• Existing channel list behavior is unchanged when no query params are provided.

Tests

Added unit tests in src/conversation/channels.rs:

• list_include_inactive_controls_visibility
• set_active_toggles_channel_state_without_deleting

Also verified:

• cargo check --lib
• cargo fmt --all --check
• cargo test conversation::channels::tests -- --nocapture

[coderabbitai]

Walkthrough

Adds channel archiving and queryable channel listing. Introduces ListChannelsQuery and SetChannelArchiveRequest types, a PUT /channels/archive endpoint, updates list_channels to accept query params, and extends ChannelStore with list(is_active_filter) and set_active; includes unit tests for new behaviors.

Changes

Cohort / File(s)	Summary
API Channel Operations src/api/channels.rs	Adds ListChannelsQuery (include_inactive, agent_id, is_active), resolve_is_active_filter, updates list_channels to accept query params and apply per-agent/is_active filtering, adds SetChannelArchiveRequest and set_channel_archive endpoint, and archive_update_response_payload. Adds unit tests.
API Route Registration src/api/server.rs	Registers new route PUT /channels/archive -> channels::set_channel_archive.
Channel Store Data Layer src/conversation/channels.rs	Replaces single-purpose list_active with list(is_active_filter: Option<bool>) (SQL conditional on is_active), keeps list_active delegating to list(Some(true)), and adds set_active(&self, channel_id, active) to update is_active. Adds tests covering list filtering and set_active behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: adding channel archiving capability and list filtering support to the API.
Description check	✅ Passed	The description is clearly related to the changeset, providing detailed context about the new features, endpoints, behavioral notes, and testing performed.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/api/channels.rs (1)

69-111: Push is_active filtering into SQL to avoid over-fetching.

On Line 82, store.list(include_inactive) can fetch extra rows and then filter in memory on Line 85. For larger channel tables, this adds avoidable DB/network/memory overhead. Consider extending ChannelStore with an optional is_active predicate so filtering happens at query time.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/channels.rs` around lines 69 - 111, The current list_channels
function calls ChannelStore::list(include_inactive) which fetches extra rows and
then filters by query.is_active in-memory; change ChannelStore::list to accept
an optional is_active filter (e.g., Option<bool> or a tri-state) so the database
query can apply the active/inactive predicate. Update the call in list_channels
to pass query.is_active (or convert the existing include_inactive logic into the
new parameter) instead of include_inactive, remove the in-memory is_active
filtering block, and ensure ChannelStore::list and its SQL/query builder use the
new parameter to add a WHERE clause on is_active. Use the symbols
ChannelStore::list, list_channels, include_inactive, and query.is_active to
locate the edits.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/api/channels.rs`:
- Around line 69-111: The current list_channels function calls
ChannelStore::list(include_inactive) which fetches extra rows and then filters
by query.is_active in-memory; change ChannelStore::list to accept an optional
is_active filter (e.g., Option<bool> or a tri-state) so the database query can
apply the active/inactive predicate. Update the call in list_channels to pass
query.is_active (or convert the existing include_inactive logic into the new
parameter) instead of include_inactive, remove the in-memory is_active filtering
block, and ensure ChannelStore::list and its SQL/query builder use the new
parameter to add a WHERE clause on is_active. Use the symbols
ChannelStore::list, list_channels, include_inactive, and query.is_active to
locate the edits.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7ebd1e1 and 0202cb1.

📒 Files selected for processing (3)

• src/api/channels.rs
• src/api/server.rs
• src/conversation/channels.rs

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

src/api/channels.rs (1)

76-113: ⚠️ Potential issue | 🟡 Minor

Global channel ordering is currently non-deterministic across agents.

Each agent’s SQL result is sorted, but the merged response order depends on map iteration order. If clients expect newest-first globally, this can produce unstable ordering.

💡 Proposed fix

 pub(super) async fn list_channels(
     State(state): State<Arc<ApiState>>,
     Query(query): Query<ListChannelsQuery>,
 ) -> Json<ChannelsResponse> {
@@
     for (agent_id, pool) in pools.iter() {
@@
     }
+
+    // Ensure deterministic newest-first ordering across all agents.
+    all_channels.sort_by(|left, right| right.last_activity_at.cmp(&left.last_activity_at));

     Json(ChannelsResponse {
         channels: all_channels,
     })
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/api/channels.rs` around lines 76 - 113, The merged channel list in
list_channels is non-deterministic because you append per-agent results in map
iteration order; instead collect channels into a temporary vector of (agent_id,
Channel) from each ChannelStore::list call, then sort that vector globally by
channel.last_activity_at (descending, newest-first) with a secondary sort by
channel.created_at (descending) to break ties, and only after sorting map items
into ChannelResponse (formatting timestamps with to_rfc3339()). Update
list_channels to build the temporary collection, perform the sort, then populate
all_channels from the sorted collection so the response order is stable and
newest-first.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/api/channels.rs`:
- Around line 76-113: The merged channel list in list_channels is
non-deterministic because you append per-agent results in map iteration order;
instead collect channels into a temporary vector of (agent_id, Channel) from
each ChannelStore::list call, then sort that vector globally by
channel.last_activity_at (descending, newest-first) with a secondary sort by
channel.created_at (descending) to break ties, and only after sorting map items
into ChannelResponse (formatting timestamps with to_rfc3339()). Update
list_channels to build the temporary collection, perform the sort, then populate
all_channels from the sorted collection so the response order is stable and
newest-first.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0202cb1 and 79913dc.

📒 Files selected for processing (3)

• src/api/channels.rs
• src/api/server.rs
• src/conversation/channels.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/api/server.rs

[coderabbitai]

🧹 Nitpick comments (1)

src/conversation/channels.rs (1)

228-228: Use a non-abbreviated error variable name.

At Line 228, prefer |error| over |e| for readability and guideline consistency.

As per coding guidelines "Do not abbreviate variable names; use queue not q, message not msg, channel not ch; common abbreviations like config are acceptable".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/conversation/channels.rs` at line 228, Replace the single-letter closure
variable in the map_err call with a descriptive name: change the closure pattern
in the expression that currently uses |e| (the map_err closure invoking
anyhow::anyhow!(e)) to use |error| and pass that into anyhow::anyhow!(error) so
the map_err invocation becomes more readable; locate the map_err on the
type-conversion/propagation chain in channels.rs (the expression ending with
.map_err(|e| anyhow::anyhow!(e))?) and update the closure variable and its usage
accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/conversation/channels.rs`:
- Line 228: Replace the single-letter closure variable in the map_err call with
a descriptive name: change the closure pattern in the expression that currently
uses |e| (the map_err closure invoking anyhow::anyhow!(e)) to use |error| and
pass that into anyhow::anyhow!(error) so the map_err invocation becomes more
readable; locate the map_err on the type-conversion/propagation chain in
channels.rs (the expression ending with .map_err(|e| anyhow::anyhow!(e))?) and
update the closure variable and its usage accordingly.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 79913dc and 4c5d720.

📒 Files selected for processing (3)

• src/api/channels.rs
• src/api/server.rs
• src/conversation/channels.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/api/server.rs