feat: headless Mongo session restore for remote AI chat - #1215
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds remote AI session resumption by introducing ChangesRemote AI Session Resume
Sequence Diagram(s)sequenceDiagram
rect rgba(100, 149, 237, 0.5)
Note over ai_task,_run_loop: Remote Respawn Path
ai_task->>_maybe_resume_remote: interactive == "remote", session_id
_maybe_resume_remote->>_get_query_engine: workspace config
_get_query_engine-->>_maybe_resume_remote: QueryEngine
_maybe_resume_remote->>QueryEngine: search(_type="ai", session_id=session_id)
QueryEngine-->>_maybe_resume_remote: prior_docs[]
end
alt No prior docs
_maybe_resume_remote-->>ai_task: restored=False (fresh start)
else Non-Mongo backend
_maybe_resume_remote-->>ai_task: yield Warning("remote")
else Prior docs found (MongoDB)
_maybe_resume_remote->>restore_history_from_db: session_id, engine, model, encryptor, system_prompt
restore_history_from_db->>ChatHistory: rebuild turns from sorted docs
restore_history_from_db-->>_maybe_resume_remote: ChatHistory
_maybe_resume_remote->>_run_loop: continue with restored history + new prompt
_run_loop->>_save_history: persist on every exit path
_maybe_resume_remote-->>ai_task: restored=True
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@secator/tasks/ai.py`:
- Around line 561-565: The remote correlation fallback in the task session setup
is too broad because `session_name` can collide across unrelated conversations.
Update the session initialization logic in the AI task flow so
`_maybe_resume_remote()` and the `self.session_id` assignment rely only on
`run_opts.context.session_id` for remote/web resumes, and otherwise fall back to
a non-remote unique identifier such as `self.id` instead of `session_name`. Keep
the stable remote session behavior for respawns, but ensure `session_name` is
not used as the Mongo query key in the `AI` task/resume path.
- Around line 276-279: The prompt handling in ai.py is treating user-supplied
prompt text as a worker-local file path, which can leak local files when the
remote resume flow passes a path-like string. Update the prompt resolution logic
in the code that assigns self.prompt from run_opts so it only uses the provided
prompt as literal text, and remove the Path(...).is_file() / read_text()
expansion for remote/UI-originated prompts. Keep any file-loading behavior
confined to explicitly trusted/local paths elsewhere, and make sure the prompt
passed into the transcript/LLM is exactly the user input.
- Around line 286-293: Skip restoring text-only history for non-chat modes,
since restore_history_from_db() drops tool calls/results and breaks resumed
remote sessions. In ai.py, update the initialization path around _detect_mode(),
get_system_prompt(), and restore_history_from_db() so history is rebuilt only
when self.mode is chat (or equivalent text-only mode), and leave remote modes
like attack to start with a fresh tool context. Ensure _run_loop() receives a
complete context for any mode that relies on tools.
In `@tests/unit/test_ai_session.py`:
- Line 126: The tests in test_ai_session are unpacking task and engine from
_make_task without using both values, triggering RUF059 and weakening the
assertion intent. Update the affected test cases to only bind the values that
are actually needed, or use the engine variable to assert the
restore/query-engine call where applicable, especially in the restore-related
test methods. Keep the assertions aligned with the behavior under test by
checking the query engine argument in the relevant restore calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a0b44ab-24bf-4d04-9039-73d5ac61f9da
📒 Files selected for processing (3)
secator/ai/session.pysecator/tasks/ai.pytests/unit/test_ai_session.py
Adds the two secator-core gaps for the Workspace AI Assistant (Mongo-channel chat), repo 1/3: - `restore_history_from_db(session_id, query_engine, model, encryptor, system_prompt)` in `secator/ai/session.py`: rebuilds a `ChatHistory` from the workspace `_type:"ai"` docs (queried by session_id, ordered by `_timestamp`) — `prompt`→user, `response`→assistant, system prompt set, re-encrypted when an encryptor is active. Headless: no local files, no TUI. - Wire a remote-resume branch in `ai.py:yielder`: when `interactive="remote"` and the session has prior `_type:"ai"` docs, restore from Mongo and continue; fresh conversations (no docs) start as before. The local CLI `replay_session`/`show_session_picker` path is untouched. - `session_id` now prefers `run_opts.context.session_id` so a respawned task finds its prior docs. - `save_history` (local `history.json`) is skipped on the remote path via a `_save_history()` helper — the Mongo docs are the source of truth. - Query-engine guard: warn when `interactive="remote"` but the resolved query backend is not mongodb/api (the web answer channel can't work otherwise). History-fidelity finding: persisted `_type:"ai"` docs capture only text turns (prompt/response) plus action *display* records — not the litellm assistant `tool_calls` messages or their `tool` results. Restore is therefore text-only. This is valid and sufficient for `mode="chat"` continuation; fabricating partial tool-call messages would produce a malformed transcript providers reject, so tool activity is deliberately collapsed. Richer assistant persistence for `mode="attack"` replay is a documented follow-up. Tests: `tests/unit/test_ai_session.py` — restore rebuilds equivalent History (order/roles/system/encryption/empty-docs/search-failure), and the remote-resume branch picks Mongo restore for prior docs / fresh otherwise / warns on non-Mongo backend. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
…nscript
The web UI correlates an AI chat conversation by session_id (across respawns),
but only the resume-prompt and follow_up items set it — the prompt/response/
token_usage/chat_compacted message items did not, so they persisted to Mongo
without session_id and the UI's {_type:"ai", session_id} query returned nothing
(empty transcript despite the task running fine). Wrap yielder to stamp
session_id on every Ai item centrally (self.session_id is set in _init_options
before the first yield).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
…ontext) The web UI's session_id arrives on the runner context, but the Task dispatcher sends self.context (not run_opts['context']) to the worker and pops run_opts['context'] — so in the worker run_opts.context is empty and session_id fell back to the prompt label, never matching the UI's UUID (empty transcript). Prefer self.context for session_id. Pairs with secator-api adding session_id to the RunnerContext model so it survives validation into self.context. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
… empty) Persisted _type:"ai" docs had session_id="" but _context.session_id=<uuid>: the runner auto-stamps item._context = self.context, so _context.session_id is reliably present, while the top-level session_id field never landed. Query _context.session_id in _poll_for_answer, the timeout update, restore_history_from_db and the resume check; drop the now-pointless yielder session_id stamp. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
…evel choices) In the web AI chat, when the worker hit a follow_up the persisted `_type:"ai"` doc had `status:""` and empty top-level `choices`, so the UI (which gates on `status=="pending"` and reads `m.choices`) stayed stuck on "thinking" with no question/buttons. Two root causes: 1. `_handle_follow_up` (ai/actions.py) stored choices ONLY in `extra_data["choices"]`, never on the top-level `Ai.choices` field the UI reads -> persisted `choices: []`. Now populate both. 2. `_dispatch_and_collect` (tasks/ai.py) persisted the follow_up Ai via `add_result()` (status="") BEFORE the main loop mutated it to `status="pending"`. Since `add_result` dedupes by `_uuid`, the later re-yield could never re-persist the pending state. Now, for a RemoteBackend run, stamp `status="pending"` + top-level `choices` + `session_id` on the single Ai BEFORE the one `add_result`, so the one persisted doc is renderable. The redundant re-stamp/yield in the main loop is removed. Local/CLI follow-up is untouched (remote-only branch). No secator-ui change needed: the doc now carries top-level `choices` and `status=="pending"`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
The ai task dispatches task/workflow sub-runners in-process and runs them synchronously. The runner framework only re-registers driver hooks (mongodb/api) from context['drivers'] on the pickle path (__setstate__, used by Celery workers) — a sync sub-runner never hits that path. So the sub-runner inherited the ai task's workspace_id/drivers in its context but registered no driver hooks: its update_runner/update_finding hooks never fired, its runner doc + findings were never persisted, and the sub-runs were absent from the workspace History. Build the hooks dict from context['drivers'] (mirroring the CLI entrypoint in cli_helper) and pass hooks= to each dispatched sub-runner, so its results are workspace-scoped and appear in History exactly like a normal runner. Also emit the created runner's id on the action Ai item (extra_data.runner_id + extra_data.runner_type) so the UI can link the action to a RunnerCard. The Ai item is now emitted after the runner is constructed (its on_init hook stamps the id into context), and is emitted even in batch/silent mode so the action doc is always persisted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
…a.finding) So the web UI can render the finding's FindingCard (VulnerabilityCard/etc.) for an add_finding action. The finding is serialized (toDict, includes _type for routing). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
…lidation LLMs frequently emit wrong-typed scalars in add_finding (a bool field as the string "true", an int as "3"), which validate_fields then rejected, dropping the finding. Add _coerce_finding_fields(cls, data), called before validate_fields, that fixes obvious type mismatches (bool/int/float/list) while leaving valid values, unknown keys, and unparseable values untouched so real errors still surface. Field type resolution is robust to both actual-type and string annotations (from __future__ import annotations), mirroring validate_fields. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
…nner.id
The UI's getRunner queries the persisted runner doc by its _id, which equals
context.{type}_id (stamped by the on_init mongodb hook) — not runner.id (secator's
internal id). So the RunnerCard showed "Runner not found" for ai-dispatched
sub-runners even though they appear in History. Prefer the context id.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
…n loop
RemoteBackend._poll_for_answer matched ANY answered follow_up doc in the
session ({_type:"ai", ai_type:"follow_up", _context.session_id, status:
"answered"}, limit:1, no sort). Across a multi-turn chat, previously answered
follow_up docs accumulate, so the poll for a NEW follow_up immediately matched
a STALE answered doc from a prior turn and returned its old answer. The loop
then set that old answer as self.prompt, re-yielded Ai(ai_type="prompt") (the
original prompt reappears), re-ran the whole turn, asked the follow_up again,
re-matched the same stale doc -> an infinite respawn that re-runs scans and
burns tokens. (On the very first turn with no prior answered docs it instead
timed out cleanly, masking the deeper stale-match bug.)
Fix: correlate the poll AND the timeout update to the SPECIFIC pending doc the
worker is blocked on. A unique prompt_uuid is stamped into the pending
follow_up's extra_data before persist and threaded
_dispatch_and_collect -> _run_loop -> _prompt_and_redetect -> ask_user ->
_poll_for_answer, which now filters on extra_data.prompt_uuid. A timeout flips
only that doc to timed_out. The turn ends cleanly and nothing re-dispatches
until the user explicitly sends a new message.
The secator-ui AiChatPanel side was investigated and is clean: spawn() is only
called from the explicit user send(); there is no watch/effect that re-spawns
on done/timed_out.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
AI-spawned sub-runners (task/workflow/scan) need context.session_id set so their persisted runner docs are queryable by conversation. The ai task's session_id is often derived (from session_name / the runner id) and is not guaranteed to live in self.context, so sub-runners did NOT carry it. Stamp it in _get_result_context from ActionContext.session_id (without overwriting an existing one). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
A Python error during an iteration (e.g. TypeError: 'str' object is not a mapping from a malformed LLM action/opts) previously propagated out of _dispatch_and_collect, was caught by the loop's broad except Exception, and killed the task. Now each action's dispatch is wrapped so the failure becomes that tool call's result fed back to the LLM, and the loop continues. - Add safe_dispatch_action(): wraps dispatch_action and, on Exception, yields an Error carrying the action's tool_call_id/tool_call_name in _context. Only Exception is caught — KeyboardInterrupt/SystemExit/GeneratorExit propagate. - The Error groups into a tool result via the existing format_tool_result / add_tool_result path, so the model sees "Action failed with error: <type>: <msg>\n<short traceback>. Fix the issue and try again." next turn. - Use safe_dispatch_action for the single-action path in _dispatch_and_collect and inside _run_batch's run_single, so one action's failure no longer aborts the turn or the other batch actions. - max_iterations still bounds a persistently-erroring model: each failed turn increments the iteration counter as before. - Drop a pre-existing unused follow_up_ai assignment to keep flake8 green. - Tests: a raising handler yields an Error, appends the error to history (LLM-visible), and continues without raising; KeyboardInterrupt propagates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
Add cooperative mid-flight steering to the Workspace AI Assistant: a user can send a message WHILE the agent is running, and the worker picks it up at the next loop checkpoint to redirect the next turn. Distinct from the hard Stop button (which revokes the Celery task). - RemoteBackend.poll_steers(session_id): drains pending `ai_type:"steer"` channel docs, returns their content oldest-first, marks them consumed so each injects exactly once. Robust — backend errors return [] (never crash). - _poll_for_answer: a steer breaks a blocked follow-up wait (returns the steer content as the answer) so the loop redirects instead of stalling; follow-up semantics intact for the no-steer case. - _run_loop: _drain_steers() at the top of each iteration appends each steer to history as `[User interjected]: …` and echoes a steer Ai item (with session_id so it persists in the transcript). - output_types/ai.py: render `steer` ai_type in the CLI transcript. - Tests: poll_steers drain/consume/robustness, steer-breaks-wait, _drain_steers inject-into-history, no-steer no-op, non-remote no-op. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
…spawn Drop the worker's redundant `Ai(ai_type="steer")` echo: the API's pending steer doc already carries `_context.session_id` and is itself the persisted transcript entry, so a second echo would double-render in the UI. Keep `_drain_steers` a generator (no items yielded) so the loop call site is unchanged and future echoes can be added without churn. Also restore steers as user turns in `restore_history_from_db` (framed `[User interjected]: …`) so a mid-flight redirect survives a respawn/history restore. Update the drain test to assert no echo doc is yielded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
…oks_from_context) Runner.__init__ now auto-registers driver hooks from context['drivers'] (via _apply_context_drivers, added on main), so the ai task's manual hook-building for sub-runners is redundant. Delete the helper and the hooks= kwarg it fed into Task/Workflow construction. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NNjPggRSVZ2xnLb7ZxWP5H
Replace the hand-rolled _format_action_error formatter with the framework's Error.from_exception, already used elsewhere in this file, so action-dispatch failures build their LLM-facing message the same way as every other error path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NNjPggRSVZ2xnLb7ZxWP5H
…eld_types Extract the field-name -> concrete-type resolution duplicated between OutputType.validate_fields and actions._resolve_field_type onto a single OutputType.field_types() classmethod, reused by both validate_fields and _coerce_finding_fields. Behavior unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NNjPggRSVZ2xnLb7ZxWP5H
Delete comments that restate the obvious or describe UI behavior; reduce the rest to one terse line capturing the non-obvious why, per reviewer request. No logic changes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NNjPggRSVZ2xnLb7ZxWP5H
Repo 1/3 of Workspace AI Assistant (secator → secator-api → secator-ui).
The Mongo-channel chat feature spawns the
aitask inmode="chat",interactive="remote"; it talks back and forth through the workspace MongoDB (_type:"ai"findings), and on respawn (after a wait-timeout) the next user message restores the conversation from Mongo. The channel (RemoteBackend), theAioutput type, and the timeout already existed. This PR closes the two remaining core gaps: headless Mongo session restore, and verifying the dispatched worker's query-engine resolves to Mongo.History-fidelity finding (the Step-0 gate)
Verdict: text-only restore — sufficient for
mode="chat"v1 (proceeded).The litellm
ChatHistorycarriessystem/user/assistant/ assistant-with-tool_calls/tool-result messages. But the persisted_type:"ai"docs only carry text turns —ai_type="prompt"→user,ai_type="response"→assistant — plus action display records (task/shell/query/…). The assistanttool_callsmessages and theirtoolresults are added to the in-memory history but are never yielded as_type:"ai"docs, so they are not in Mongo.Therefore a literal restore can only rebuild text turns. Crucially, fabricating an assistant
tool_callsmessage without its matchingtoolresult yields a malformed transcript that most providers reject — so we deliberately collapse tool activity into the surrounding text turns rather than reconstruct it. This is:mode="chat"(workspace Q&A — the feature's primary mode): the assistant's response text already narrates what it did, so the LLM continues coherently.mode="attack"(tool-heavy): intermediate tool I/O isn't replayed. Documented follow-up: persist a richer assistant record (the fulltool_calls/toolmessages) to Mongo if attack-mode replay is needed. This matches the build-spec's flagged risk.Per the task's own guidance ("a text-only restore that works for
mode=\"chat\"is an acceptable v1 if you document the limitation"), I proceeded.Restore approach
restore_history_from_db(session_id, query_engine, model, encryptor, system_prompt)(secator/ai/session.py):query_engine.search({"_type":"ai","session_id":session_id}), sorted by_timestamp.prompt→add_user,response→add_assistant; all otherai_types (action displays, follow_up/permission, shell_output, summaries) are channel/UX artifacts and skipped.maybe_encryptwhen an encryptor is active (persisted docs hold plaintext, since response content is decrypted before it's yielded).Wired in
ai.py:yieldervia_maybe_resume_remote(): wheninteractive=="remote"and the session has prior_type:"ai"docs, restore from Mongo, append the new user message, and run the loop; fresh conversations (no docs) start as before. The local CLIreplay_session/show_session_pickerpath is untouched.session_idnow prefersrun_opts.context.session_id(the UI's stable id, reused on respawn). Localhistory.jsonwrites are skipped on the remote path via a_save_history()helper (Mongo is the source of truth).Query-engine finding (Task 2)
ctx.get_query_engine()buildsQueryEngine(workspace_id, context=self.context);QueryEngine._select_backendresolves the backend fromcontext['drivers'](mongodb > api > local). The API appends themongodbdriver on every dispatch, so for an async-dispatchedaitask withcontext.workspace_idthis correctly resolves to the Mongo backend — no wiring fix needed. The gap was the absence of a guard, so_maybe_resume_remotenow emits aWarningwheninteractive="remote"but the resolved backend is not mongodb/api (the answer channel can't work otherwise). The existing_run_loopRemoteBackend wiring (self.backend.query_engine = ctx.get_query_engine()) uses the same context and is consistent.Tests (
tests/unit/test_ai_session.py, 9 passing)restore_history_from_db: rebuilds equivalent History (timestamp order, roles, system prompt, non-turn docs skipped); no-prior-docs → system-only / empty; empty-content docs skipped; backend search failure → system-only (no crash); encryptor re-encrypts restored turns.Verification
flake8(max-line-length 120) clean on all changed files.test_ai_session,test_ai_interactivity,test_ai_history,test_ai_task_opts): 73 passed. Imports clean.test_ai_loop.py(guardrails "command not approved") reproduce on cleanorigin/main— not caused by this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01P5vSjfkBuGAAHdKxHS3ySm
Summary by CodeRabbit