feat(per-session-stop): per-(agent, session) liveness tracking and session-scoped stop#3195
Conversation
…ssion-scoped stop Implements issue #3172. Pre-change the kernel keyed `running_tasks` and `session_interrupts` by `AgentId` only, so spawning a second concurrent loop for the same agent (parallel `session_mode = "new"` triggers, `agent_send` fan-out, parallel channel chats) silently overwrote the prior abort handle and left the earlier loop un-stoppable. Kernel - Rekey `running_tasks` to `DashMap<(AgentId, SessionId), RunningTask>` where `RunningTask { abort, started_at }`. - Rekey `session_interrupts` to `DashMap<(AgentId, SessionId), ...>`. - Update every insert/remove site (streaming + non-streaming entry, cleanup on success/failure, dead-agent reaper). - New methods: - `stop_session_run(agent, session)` — abort one entry. - `list_running_sessions(agent)` — snapshot for introspection. - `agent_has_active_session(agent)` — cheap "any in-flight" check used by the WS state-event gate. - `any_session_interrupt_for_agent(agent)` — fork lookup helper that picks any active session for the agent (forks chain off the parent's interrupt; choice is best-effort but cancellation cascades correctly because `stop_agent_run` fans out across all sessions). - `stop_agent_run` now fans out across every `(agent, *)` entry, preserving the existing public semantics of `POST /api/agents/{id}/stop` ("abort all sessions for this agent"). - `suspend_agent` reuses `stop_agent_run` for the same fan-out. Types - `RunningSessionSnapshot { session_id, started_at, state }` and `RunningSessionState::Running` for the wire shape. The state enum is single-variant today; finer-grained sub-states (WaitingLLM, ExecutingTool) are a separate follow-up that requires the agent loop to write back its current step. API - `GET /api/agents/{id}/runtime` — snapshot of in-flight loops. - `POST /api/agents/{id}/sessions/{session_id}/stop` — abort one `(agent, session)` loop without affecting the agent's other concurrent sessions. Scoping under `/agents/{id}/...` rather than the issue's suggested `/api/sessions/{id}/stop` removes the need for a session-to-agent reverse lookup and prevents cross-agent confusion. - WS `queue` command migrated from `running_tasks_ref().contains_key` to `agent_has_active_session`. - OpenAPI registers both new handler paths. CLI - `librefang sessions` gains a STATE column showing `running` / `idle` per session, populated by calling the new runtime endpoint once per unique agent in the listing. - `--active` flag filters to currently-executing sessions only. - JSON output annotates each entry with `state` so scripts see the same signal as the table renderer. Tests - Three new kernel tests covering: two concurrent sessions on one agent are independently abortable, `stop_agent_run` fans out and doesn't leak across agents, `stop_agent_run` on an idle agent returns false. Existing cascade tests updated to the new map shape via `any_session_interrupt_for_agent`. Refs #3172
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Three small fixes from the PR review: 1. **i18n the invalid-session-id error.** stop_session was returning a hardcoded English `"invalid session id"` while every adjacent error went through the FTL translator. Added `api-error-session-invalid-id` to all six locales (en, zh-CN, ja, de, es, fr) and routed the handler through `t.t(...)`. 2. **Document the snapshot semantics on `stop_agent_run`.** The function collects session keys into a `Vec`, then iterates to remove. A session that finishes between the snapshot and the removal is silently absent from the count; a session inserted *after* the snapshot is not aborted by this call. Added a `Snapshot semantics` paragraph to the doc comment so future callers and reviewers know this is best-effort against the observed instant, with a pointer to `suspend_agent` for freeze-then-abort needs. 3. **Add a fork-registration-skip test.** The production fork path deliberately skips `running_tasks` / `session_interrupts` inserts so it doesn't clobber the parent's `(agent, session)` entry (forks reuse the parent's session id for prompt-cache alignment). The skip was only comment-asserted before; new test `test_fork_does_not_overwrite_parent_registration` registers the parent, simulates the fork's deliberate no-op, and asserts the parent's entry — including its `started_at` timestamp and the shared `Arc<AtomicBool>` cancellation handle — survives unchanged. `cargo test -p librefang-kernel --lib` → 567 passed (was 566).
Deploying librefang-docs with
|
| Latest commit: |
ff3a461
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://80b5e77c.librefang-docs-web.pages.dev |
| Branch Preview URL: | https://feat-per-session-stop-3172-i.librefang-docs-web.pages.dev |
Root cause of the 3-platform Test job failureCI is red on Both Fluent rejects duplicate identifiers as a parse error. The Fluent silent-fallback masking the real bug is also a small footgun worth a follow-up TODO — at minimum logging the parse errors at warn level would have made this obvious — but that's outside this PR's scope. FixDrop the duplicate line in Verified locally on macOS: diff --git a/crates/librefang-types/locales/en/errors.ftl b/crates/librefang-types/locales/en/errors.ftl
index 2703a915..4151ede6 100644
--- a/crates/librefang-types/locales/en/errors.ftl
+++ b/crates/librefang-types/locales/en/errors.ftl
@@ -4,7 +4,6 @@
api-error-agent-not-found = Agent not found
api-error-agent-spawn-failed = Agent spawn failed
api-error-agent-invalid-id = Invalid agent ID
-api-error-session-invalid-id = Invalid session ID
api-error-agent-already-exists = Agent already exists
api-error-agent-no-workspace = Agent has no workspace
api-error-agent-not-found-or-terminated = Agent not found or already terminated
diff --git a/crates/librefang-types/locales/ja/errors.ftl b/crates/librefang-types/locales/ja/errors.ftl
index 9d213df3..e1d960ae 100644
--- a/crates/librefang-types/locales/ja/errors.ftl
+++ b/crates/librefang-types/locales/ja/errors.ftl
@@ -4,7 +4,6 @@
api-error-agent-not-found = エージェントが見つかりません
api-error-agent-spawn-failed = エージェントの作成に失敗しました
api-error-agent-invalid-id = 無効なエージェント ID
-api-error-session-invalid-id = 無効なセッション ID
api-error-agent-already-exists = エージェントは既に存在します
api-error-agent-no-workspace = エージェントにワークスペースがありません
api-error-agent-not-found-or-terminated = エージェントが見つからないか、既に終了しています |
The followup commit added api-error-session-invalid-id under the
"Agent errors" section in all six locales, but the key already lived
in the "Session errors" section in en/errors.ftl (line 52) and
ja/errors.ftl (line 51). Fluent rejects duplicate identifiers as a
parse error; ErrorTranslator silently falls back to EN_FTL for the
broken bundle, which is why ja.t("api-error-agent-not-found") was
returning "Agent not found" even though the source file had the
Japanese translation. CI failed on three platforms with that exact
panic.
Drop the duplicate. The original entries in the Session errors
section stay; de/es/fr/zh-CN are unaffected.
Closes #3172.
Summary
Implements per-
(agent, session)liveness tracking so concurrent loops on the same agent can be observed and stopped independently. Pre-change, the kernel keyedrunning_tasksandsession_interruptsbyAgentIdonly, which silently overwrote the abort handle of any earlier loop the moment a second one spawned (parallelsession_mode = "new"triggers,agent_sendfan-out, parallel channel chats). The earlier loop was un-stoppable through the public API.What changed
Kernel (
librefang-kernel/src/kernel/mod.rs)running_taskstoDashMap<(AgentId, SessionId), RunningTask>whereRunningTask { abort, started_at }.session_interruptsto the same shape.stop_session_run(agent, session)— abort one entry.list_running_sessions(agent)— snapshot of in-flight loops.agent_has_active_session(agent)— cheap "any in-flight" check.any_session_interrupt_for_agent(agent)— fork lookup helper.stop_agent_runfans out across every(agent, *)entry. Public semantics ofPOST /api/agents/{id}/stopare preserved ("abort all sessions for this agent").suspend_agentreusesstop_agent_runfor the same fan-out.Types (
librefang-types/src/agent.rs)RunningSessionSnapshot { session_id, started_at, state }andRunningSessionState::Runningfor the wire shape. State is intentionally a single variant today; finer-grained sub-states (WaitingLLM/ExecutingTool) are a follow-up that needs the agent loop to write back its current step.API (
librefang-api)GET /api/agents/{id}/runtime— snapshot of in-flight loops (empty array when idle).POST /api/agents/{id}/sessions/{session_id}/stop— abort one(agent, session)loop. Scoping under/agents/{id}/...rather than the issue's suggested/api/sessions/{id}/stopremoves the need for a session-to-agent reverse lookup and prevents cross-agent confusion.queuecommand migrated toagent_has_active_session.CLI (
librefang-cli)librefang sessionsadds a STATE column showingrunning/idle.--activeflag filters to currently-executing sessions only.state.Tests
stop_agent_runfans out and doesn't leak across agents;stop_agent_runon an idle agent returnsfalse.any_session_interrupt_for_agent.Branch note
The implementation lives on
feat/per-session-stop-3172-implrather thanfeat/per-session-stop-3172because the latter remote ref existed at a stale commit and creating a fresh branch was cleaner than force-pushing.Test plan
cargo check -p librefang-kernel -p librefang-types -p librefang-api -p librefang-clicargo test -p librefang-kernel --lib— 562 passedcargo clippy -p librefang-kernel -p librefang-api -- -D warnings— cleanGET /api/agents/{id}/runtimereturns empty when idle, populated mid-turnPOST /api/agents/{id}/sessions/{sid}/stopaborts only the targeted looplibrefang sessions --activelists only running sessions; STATE column matches the runtime endpointOut of scope
RunningSessionStatevariants (WaitingLLM,ExecutingTool(name)) — wire format already leaves room.