Skip to content

fix(channels,kernel): scope /new to the calling channel + purge JSONL on delete (#4868)#4905

Merged
houko merged 2 commits into
mainfrom
fix/4868-channel-new-scope
May 11, 2026
Merged

fix(channels,kernel): scope /new to the calling channel + purge JSONL on delete (#4868)#4905
houko merged 2 commits into
mainfrom
fix/4868-channel-new-scope

Conversation

@houko

@houko houko commented May 11, 2026

Copy link
Copy Markdown
Contributor

Closes #4868.

Summary

  • Channel /new, /reboot, /compact previously deleted every session for the agent (default + every per-channel sid + cron-spawned). Typing /new in Telegram silently nuked the dashboard transcript and every other channel's transcript.
  • Same path also leaked the on-disk JSONL mirror — delete_agent_sessions clears SQL rows but never touched <workspace>/sessions/{sid}.jsonl, so every reset accumulated orphan transcripts indefinitely.
  • This PR scopes channel resets to the per-channel sid via a new ResetScope { Agent, Session(SessionId) } arg on reset_session / reboot_session, and adds best-effort JSONL cleanup to all three session-delete paths (reset_session, reboot_session, clear_agent_history).

Behaviour change matrix

Surface Before After
Channel /new, /reboot, /compact Wiped every session for the agent Resets only SessionId::for_channel(agent, "<ch>:<chat>"); siblings untouched
POST /api/agents/{id}/session/reset, …/reboot Agent-wide wipe Agent-wide wipe (passes ResetScope::Agent) — preserved
Dashboard WS reset / reboot commands Agent-wide wipe Agent-wide wipe — preserved
DELETE /api/agents/{id}/history (clear_agent_history) Agent-wide wipe of sessions + canonical Same + JSONL cleanup

Subtleties

  • Per-session reset eagerly recreates an empty session at the same deterministic sid so external SessionStart / SessionReset hooks fire symmetrically (lazy creation in the agent loop only fires the internal SessionLifecycleEvent, not the external hook).
  • Per-session reset does NOT clear agent-wide token quota (no grief vector for one channel user to clear an agent-wide quota-exceeded state by typing /new).
  • Cross-agent sids are rejected with InvalidInput instead of silently deleted (defence-in-depth — callers compute the sid from a trusted (channel, chat) pair, but the typed argument itself isn't).
  • JSONL cleanup is best-effort: SQL is the source of truth, a leftover file is hygiene not privacy. Failures log the orphan path with a warn! so admins can clean up.

API changes

  • New: librefang_types::agent::ResetScope { Agent, Session(SessionId) }.
  • New: KernelHandle::compact_agent_session_with_id(agent, Option<SessionId>) — promoted from inherent method.
  • New: ChannelBridgeHandle::{reset,reboot,compact}_channel_session(agent, channel, chat_id). Default impls return Err("Not implemented") matching the trait's existing pattern.
  • Changed: KernelHandle::{reset,reboot}_session now takes a ResetScope arg.

Verification

  • cargo check --workspace --lib — clean.
  • cargo clippy -p librefang-types -p librefang-kernel -p librefang-channels -p librefang-api --all-targets -- -D warnings — clean.
  • cargo test -p librefang-kernel --test session_reset_scope_test — 4 new tests:
    • per_session_reset_only_deletes_targeted_sid — telegram sid wiped, dashboard + slack survive (SQL + JSONL).
    • agent_wide_reset_purges_all_sessions_and_jsonl — every sid + every JSONL gone.
    • per_session_reset_rejects_cross_agent_sid — defence-in-depth.
    • per_session_reset_on_unmaterialised_sid_is_ok/new typed in a fresh chat that has never received a message.
  • cargo test -p librefang-api --lib channel_session_id_matches_inbound_resolver_formula — pins the channel-side sid derivation to the inbound resolver's formula so they cannot drift.
  • cargo test -p librefang-api --test agents_routes_integration — 19 existing agent-route tests still pass.
  • cargo test -p librefang-types -p librefang-kernel -p librefang-channels --lib — 2086 existing lib tests still pass.

Out of scope

Per the issue's own #3 follow-up: a librefang sessions gc admin tool to reconcile already-orphaned JSONL files on existing deployments. The new code stops accumulating orphans going forward; the historical backfill is a separate environmental cleanup that would expand scope into librefang-cli and is best tracked separately.

Test plan

  • Per-channel /new deletes only the targeted sid (kernel test)
  • JSONL mirror is purged for the deleted sid (kernel test)
  • Agent-wide reset still purges everything + every JSONL (kernel test)
  • Cross-agent sid rejected (kernel test)
  • Unmaterialised sid /new works (kernel test)
  • Channel-side sid derivation matches inbound resolver formula (api unit test)
  • Existing agent-routes integration tests still green

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions github-actions Bot added area/channels Messaging channel adapters area/kernel Core kernel (scheduling, RBAC, workflows) size/L 250-999 lines changed labels May 11, 2026
@houko
houko force-pushed the fix/4868-channel-new-scope branch from fb3e4d2 to 84bd626 Compare May 11, 2026 09:18
@github-actions github-actions Bot added area/sdk JavaScript and Python SDKs size/XL 1000+ lines changed and removed size/L 250-999 lines changed labels May 11, 2026
houko added a commit that referenced this pull request May 11, 2026
…4868)

Address review feedback on #4905:

- `reset_all_sessions` previously held only `agent_msg_locks[agent_id]`,
  but `send_message_full` / streaming acquire EITHER that or
  `session_msg_locks[sid]` depending on `session_id_override` (#2959).
  A concurrent multi-tab WS or scoped HTTP turn could finish after the
  delete and UPSERT its session row back into existence. Snapshot the
  live sids under the agent lock, then acquire every per-session lock
  (sorted) and hold all guards through the delete + recreate.
- Tighten the lock-order comment in `reset_one_session`: send_message_full
  is XOR (one lock or the other), not both — so the agent-then-session
  order here cannot cycle.
- Map `LibreFangError::InvalidInput` from `reset_session` /
  `reboot_session` route handlers to HTTP 400 instead of the catch-all
  500, matching the parallel branch in `update_schedule` (#4732). Today
  only forward-compat (callers always pass `ResetScope::Agent`); arms
  ready for a scoped admin endpoint.
- info!() in both `reset_all_sessions` and `reset_one_session` now
  includes `op = "reset" | "reboot"` so log readers can tell the two
  paths apart without inspecting `save_summary`.
houko added 2 commits May 11, 2026 18:50
…NL on delete (#4868)

Channel `/new`, `/reboot`, `/compact` previously called
`kernel.reset_session(agent)` without a scope, which deleted EVERY session
the agent owned (default + every per-channel sid + cron-spawned ones).
Typing `/new` in Telegram silently nuked the dashboard transcript and
every other channel's transcript too.

Same code path also leaked the on-disk transcript: `delete_agent_sessions`
clears `sessions` + `sessions_fts` rows transactionally but never touches
the `<workspace>/sessions/{sid}.jsonl` mirror, so every reset accumulated
orphan files indefinitely.

This change introduces a `ResetScope { Agent, Session(SessionId) }` arg on
`reset_session` / `reboot_session` so the channel layer can ask the kernel
to delete exactly one sid (the one derived from `(channel, chat_id)` via
`SessionId::for_sender_scope`), and adds best-effort JSONL cleanup to all
three session-delete paths (`reset_session`, `reboot_session`,
`clear_agent_history`).

Behaviour summary:
  - Channel `/new` now resets only the per-channel session; sibling
    sessions on other surfaces are untouched. The session is recreated
    eagerly at the same deterministic sid so external `SessionStart` /
    `SessionReset` hooks fire symmetrically and the next inbound message
    lands on a fresh empty row.
  - Per-session reset does NOT clear agent-wide quota state (no grief
    vector for one channel to clear an agent-wide token-quota cap).
  - Cross-agent sids are rejected with `InvalidInput` instead of being
    silently deleted (defence-in-depth on the kernel boundary).
  - `POST /api/agents/{id}/session/reset`, `…/reboot`, the dashboard
    WebSocket `reset` / `reboot` commands, and `clear_agent_history` keep
    the historical agent-wide semantics by passing `ResetScope::Agent`.
  - All three SQL-delete paths now also remove the
    `<workspace>/sessions/{sid}.jsonl` mirror. Failures are logged with
    the orphan path but not propagated — SQL is the source of truth,
    leaving a ghost file is hygiene not privacy.

Concurrency (review fix):
  - All three reset paths (reset / reboot / clear_agent_history) are now
    `async fn` and acquire the same per-agent / per-session message lock
    that `send_message_full` holds. Without this, an in-flight turn's
    later `save_session` UPSERT would silently revive the just-deleted
    row, defeating the entire fix. `reset_one_session` takes
    `agent_msg_locks[agent]` then `session_msg_locks[sid]` (same order
    as `send_message_full`'s lock acquisition — no deadlock can form).

Single source of truth for the scope formula (review fix):
  - The channel-scope formula previously lived inline in four places:
    `kernel/messaging.rs::send_message_full`, `kernel/agent_execution.rs`,
    `kernel/mod.rs::resolve_dispatch_session_id`, and the channel-bridge
    reset helper. Any one drifting from the others silently regresses
    #4868. Extracted to `SessionId::for_sender_scope(agent, channel,
    chat_id)` in `librefang-types` and pinned via a unit test.

Tests:
  - `crates/librefang-kernel/tests/session_reset_scope_test.rs` —
    7 tokio integration tests:
      * per_session_reset_only_deletes_targeted_sid
      * agent_wide_reset_purges_all_sessions_and_jsonl
      * per_session_reset_rejects_cross_agent_sid
      * per_session_reset_on_unmaterialised_sid_is_ok
      * per_session_reboot_skips_summary_save
      * clear_agent_history_purges_jsonl_too
      * compact_with_id_targets_the_requested_session
  - Unit test `for_sender_scope_matches_legacy_inline_formula` in
    `librefang-api/src/channel_bridge.rs` pins the helper's output
    against the pre-refactor inline formula so the four kernel call
    sites can't drift.

Out of scope (deferred per issue's own #3 follow-up): a `librefang sessions
gc` admin tool to reconcile already-orphaned JSONL files on existing
deployments. The new code stops accumulating orphans going forward; the
backfill is a separate environmental cleanup.
…4868)

Address review feedback on #4905:

- `reset_all_sessions` previously held only `agent_msg_locks[agent_id]`,
  but `send_message_full` / streaming acquire EITHER that or
  `session_msg_locks[sid]` depending on `session_id_override` (#2959).
  A concurrent multi-tab WS or scoped HTTP turn could finish after the
  delete and UPSERT its session row back into existence. Snapshot the
  live sids under the agent lock, then acquire every per-session lock
  (sorted) and hold all guards through the delete + recreate.
- Tighten the lock-order comment in `reset_one_session`: send_message_full
  is XOR (one lock or the other), not both — so the agent-then-session
  order here cannot cycle.
- Map `LibreFangError::InvalidInput` from `reset_session` /
  `reboot_session` route handlers to HTTP 400 instead of the catch-all
  500, matching the parallel branch in `update_schedule` (#4732). Today
  only forward-compat (callers always pass `ResetScope::Agent`); arms
  ready for a scoped admin endpoint.
- info!() in both `reset_all_sessions` and `reset_one_session` now
  includes `op = "reset" | "reboot"` so log readers can tell the two
  paths apart without inspecting `save_summary`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/channels Messaging channel adapters area/kernel Core kernel (scheduling, RBAC, workflows) size/XL 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Channel /new wipes ALL agent sessions, not just the channel-derived one

1 participant