Skip to content

fix: separate reset and new conversation semantics - #10118

Open
w31r4 wants to merge 3 commits into
masterfrom
codex/separate-reset-new-conversation
Open

w31r4 wants to merge 3 commits into
masterfrom
codex/separate-reset-new-conversation

Conversation

@w31r4

@w31r4 w31r4 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Restore distinct reset and new conversation semantics for local and third-party runners.
  • Preserve conversation metadata and old local records according to each command contract.
  • Update command descriptions, user documentation, and regression coverage.

Validation

  • 163 related tests passed.
  • Ruff format check passed.
  • Ruff lint check passed.
  • No OpenSpec or other spec documents are included.

Summary by Sourcery

Separate /reset context clearing from /new conversation creation while preserving the appropriate local and remote conversation state.

Bug Fixes:

  • Restore distinct /reset and /new conversation behavior across built-in and third-party Agent Runners.

Enhancements:

  • Make /reset clear the active conversation context while preserving its metadata, and make /new create a separate conversation while retaining previous local records.
  • Ensure both commands stop active tasks and clean up temporary group context consistently.

Documentation:

  • Update English and Chinese command documentation to describe the separate /reset and /new semantics.

Tests:

  • Expand regression coverage for command permissions, context clearing, metadata preservation, third-party runners, command descriptions, and localized documentation.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/builtin_stars/builtin_commands/commands/conversation.py" line_range="120-128" />
<code_context>
+        cfg = self.context.get_config(umo=umo)
+        agent_runner_type = cfg["agent_runner"]["runner_type"]
+
+        active_event_registry.stop_all(umo, exclude=message)
+        cid = await self.context.conversation_manager.get_curr_conversation_id(umo)
+        if agent_runner_type in THIRD_PARTY_AGENT_RUNNER_KEY:
+            await _clear_third_party_agent_runner_state(
+                self.context,
+                umo,
+                agent_runner_type,
+            )
+        else:
+            if cid:
+                await self.context.conversation_manager.update_conversation(
</code_context>
<issue_to_address>
**issue (bug_risk):** `reset` removes the third-party runner's stored conversation/thread ID immediately after `active_event_registry.stop_all`, but `stop_all` does not wait for already-running requests to finish. A request that completes afterward writes its newly received Dify/Coze/DashScope/DeerFlow ID back to session storage, so the next user message continues the supposedly reset remote conversation.

**Triggers:** When a third-party runner request is still completing while `/reset` is processed.

**Suggested fix:** Invalidate the runner state with a generation/token or await/cancel the relevant tasks before removing the stored remote conversation ID, and ignore late writes from the invalidated request.
</issue_to_address>

Sourcery assessment

Needs a human reviewer. 1 finding to address first, and if the new /reset behavior is wrong, it permanently clears the current conversation's stored history and may clear remote runner context; reverting the code will not restore that data. The impact is bounded to the session being reset, but the deleted context cannot be recovered by rerunning the command.

Blocking findings: astrbot/builtin_stars/builtin_commands/commands/conversation.py:128


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment on lines +120 to +128
active_event_registry.stop_all(umo, exclude=message)
cid = await self.context.conversation_manager.get_curr_conversation_id(umo)
if agent_runner_type in THIRD_PARTY_AGENT_RUNNER_KEY:
await _clear_third_party_agent_runner_state(
self.context,
umo,
agent_runner_type,
)
else:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): reset removes the third-party runner's stored conversation/thread ID immediately after active_event_registry.stop_all, but stop_all does not wait for already-running requests to finish. A request that completes afterward writes its newly received Dify/Coze/DashScope/DeerFlow ID back to session storage, so the next user message continues the supposedly reset remote conversation.

Triggers: When a third-party runner request is still completing while /reset is processed.

Suggested fix: Invalidate the runner state with a generation/token or await/cancel the relevant tasks before removing the stored remote conversation ID, and ignore late writes from the invalidated request.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 17, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
astrbot-docs 11540df Commit Preview URL

Branch Preview URL
Sep 19 2026, 07:52 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant