Skip to content

fix(server): rebuild agent session when chat/agent mode changes (#1767)#1854

Merged
Dani Akash (DaniAkash) merged 4 commits into
browseros-ai:mainfrom
ZhYGuoL:fix/newtab-agent-mode
Jul 23, 2026
Merged

fix(server): rebuild agent session when chat/agent mode changes (#1767)#1854
Dani Akash (DaniAkash) merged 4 commits into
browseros-ai:mainfrom
ZhYGuoL:fix/newtab-agent-mode

Conversation

@ZhYGuoL

Copy link
Copy Markdown
Contributor

Switching between Chat and Agent mode mid-conversation had no effect. The assistant kept answering that it was in Chat mode and kept refusing browser automation.

The client sends the mode correctly on every turn. The problem is server side: an agent's toolset and system prompt are built once, when the session is created, and the session is cached per conversation. ChatService already rebuilds that session when MCP servers or the workspace change mid-conversation, but not when the mode changes. So a conversation that started in Chat mode kept its read-only toolset and its "you are in read-only chat mode" system prompt (prompt.ts) for the rest of the conversation, and the toggle did nothing.

This is not new-tab specific, despite the report. The sidepanel behaves the same way if you send a message before switching. The new tab page just makes it easy to hit, since its first turn is usually a plain question.

What changed

  • session-store.ts: track chatMode on AgentSession, alongside the existing mcpServerKey and workingDir change-detection fields. Declared required rather than optional so the compiler enforces the stamp: an unstamped session would compare undefined !== false and fake a mode change on the first turn.
  • chat-service.ts: rebuild the session when chatMode changes mid-conversation, matching the existing MCP and workspace rebuild paths.

Scope, and why it is drawn here

  • Both directions rebuild. Only chat -> agent was reported, and it is the safe one: agent's toolset is a superset of chat's, so that rebuild re-admits tools and strips nothing. But rebuilding only that direction would leave chat -> agent -> chat holding full write tools while the toggle reads chat, which is a worse failure than the one being fixed.
  • Known tradeoff. agent -> chat narrows the toolset, so sanitizeMessagesForToolset drops the agent's record of tool calls it already made, and switching back does not restore them. The MCP and workspace rebuilds already behave this way. It costs the agent context, not the user's chat history, which is stored client-side.
  • ACP-backed providers are excluded on purpose. A rebuild cannot deliver a mode change to them: their instructions live in the workspace instruction file, and ensureWorkspaceInstructionFile() skips with skipped-not-new-conversation whenever isNewConversation is false, which it is on every rebuild since it comes from !session evaluated beforehand. Rebuilding would hand the agent a fresh in-band prompt contradicting the stale on-disk block. Mode switching for ACP needs that file refreshed mid-conversation, which seems worth its own change and its own decision. Happy to take direction.

How I verified

The regression test fails on main before the fix:

$ bun test tests/api/services/chat-service.test.ts -t "switches from chat to agent"
expect(createCalls).toHaveLength(2)
Expected length: 2
Received length: 1
(fail) rebuilds the session when the user switches from chat to agent

and passes after:

$ cd apps/server && bun test tests/api/services/chat-service.test.ts
 15 pass  0 fail

$ cd apps/app && bun run test
 311 pass  0 fail

$ cd apps/server && bun run typecheck
 Exited with code 0

$ bunx @biomejs/biome check <changed files>
 Checked 3 files. No fixes applied.

Tests added: rebuilds the session when the user switches from chat to agent, re-restricts the session when switching back to chat mode, leaves ACP sessions alone on a mode switch, does not rebuild the session when the mode is unchanged.

Notes for reviewers

While tracing this I found two adjacent things that are out of scope here but may be worth separate issues:

  • Read-only chat mode is not enforced for ACP providers (claude-code, codex, acp-custom), only suggested via the prompt. buildAcpMcpServers / buildBrowserOsSelfMcpEntry send no mode to /mcp, so that surface stays fully writable. An agent that ignores the instruction can still click and navigate while the user believes they have restricted it.
  • autoAttachActiveTab (chat-actions.hooks.ts) is dead config: useChatActions has one caller and it never passes the flag, and the sidepanel does not use the hook at all.

Fixes #1767.

Switching between Chat and Agent mode mid-conversation had no effect. The
assistant kept answering that it was in Chat mode and kept refusing browser
automation.

The client sends the mode correctly on every turn. The problem is server
side: an agent's toolset and system prompt are built once, when the session
is created, and the session is cached per conversation. ChatService already
rebuilds that session when MCP servers or the workspace change
mid-conversation, but not when the mode changes. So a conversation that
started in Chat mode kept its read-only toolset and its "you are in
read-only chat mode" system prompt for the rest of the conversation, and
the toggle did nothing.

This is not new-tab specific despite the report. The sidepanel behaves the
same way if you send a message before switching; the new tab page just
makes it easy to hit, since its first turn is usually a plain question.

Track chatMode on the session alongside the existing mcpServerKey and
workingDir change-detection fields, and rebuild when it changes. chatMode
is required rather than optional so the compiler enforces the stamp: an
unstamped session would compare undefined !== false and fake a mode change
on the first turn.

Both directions rebuild. Only chat -> agent was reported and it is the safe
one, since agent's toolset is a superset of chat's, but rebuilding only
that direction would leave chat -> agent -> chat holding full write tools
while the toggle reads chat. Known tradeoff: agent -> chat narrows the
toolset, so sanitizeMessagesForToolset drops the agent's record of tool
calls it already made and switching back does not restore them. The MCP and
workspace rebuilds already behave this way, and it costs the agent context
rather than the user's chat history, which is stored client-side.

ACP-backed providers are excluded. A rebuild cannot deliver a mode change
to them: their instructions live in the workspace instruction file, and
ensureWorkspaceInstructionFile() skips with 'skipped-not-new-conversation'
whenever isNewConversation is false, which it is on every rebuild since it
comes from `!session` evaluated beforehand. Rebuilding would hand the agent
a fresh in-band prompt contradicting the stale on-disk block. Mode
switching for ACP needs that file refreshed, and is left for a separate
change.

Fixes browseros-ai#1767.

Signed-off-by: ZhYGuoL <zguoliau@andrew.cmu.edu>
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA. Thank you!
Posted by the CLA Assistant Lite bot.

@ZhYGuoL

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Resolve the chat-service test import conflict: keep beforeEach for the
mode-switch tests alongside main's ai import. Source auto-merged; the
mode-change rebuild logic applies unchanged against current main
(typecheck clean, 15/15 chat-service tests pass, biome clean).
@DaniAkash

Copy link
Copy Markdown
Contributor

Greptile (@greptileai)

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR rebuilds cached non-ACP agent sessions when chat/agent mode changes.

  • Stamps each session with its creation-time chat mode.
  • Reconciles mode, workspace, and MCP changes through a single rebuild.
  • Adds model-visible transition notices and regression coverage for mode switching.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains within the scope of the previous review thread.

Important Files Changed

Filename Overview
packages/browseros-agent/apps/server/src/agent/session-store.ts Adds the required chat-mode stamp used to detect configuration drift in cached sessions.
packages/browseros-agent/apps/server/src/api/services/chat-service.helpers.ts Extracts pure builders for MCP, workspace, and mode-change notices.
packages/browseros-agent/apps/server/src/api/services/chat-service.ts Snapshots cached session inputs, performs one rebuild for relevant non-ACP changes, and emits each applicable notice.
packages/browseros-agent/apps/server/tests/api/services/chat-service.helpers.test.ts Covers all notice-builder variants.
packages/browseros-agent/apps/server/tests/api/services/chat-service.test.ts Covers bidirectional mode changes, unchanged modes, ACP exclusion, and simultaneous session-input changes.

Reviews (2): Last reviewed commit: "refactor(server): replace rebuiltThisTur..." | Re-trigger Greptile

Comment thread packages/browseros-agent/apps/server/src/api/services/chat-service.ts Outdated
…conciliation

The three mid-conversation change detectors (MCP, workspace, mode) each
rebuilt the session in place and relied on a mutable rebuiltThisTurn flag
to avoid a second rebuild. Because rebuildSession restamps the session,
later detectors read already-mutated state, which is why the mode block
had to pre-compute its flag and why the workspace notice was silently
dropped when MCP also changed in the same turn.

Snapshot the session inputs once, derive mcpChanged/workspaceChanged/
modeChanged from that snapshot, rebuild at most once if any changed, then
emit each notice independently. Removes the flag and makes every detector
read consistent pre-rebuild state. Extracts the notice strings into pure
describeMcpChange/describeWorkspaceChange/describeModeChange helpers with
direct unit tests, plus integration tests asserting a single rebuild and
all notices when several inputs change together.
@DaniAkash

Copy link
Copy Markdown
Contributor

Greptile (@greptileai)

ACP conversations are always agent mode (read-only chat mode is not
enforced for those providers). agentConfig.chatMode was still set from the
current request mode, so a rebuild triggered by an unrelated change (an MCP
server becoming ready, a workspace switch) adopted a mid-conversation chat
toggle and handed the ACP agent a fresh in-band chat-mode prompt that
contradicts its on-disk instruction file. The modeChanged exclusion only
blocked a mode-triggered rebuild, not this adoption.

Pin chatMode to false for ACP providers. Adds tests that an MCP-driven
rebuild keeps the ACP session in agent mode, and that an ACP request asking
for chat mode still builds in agent mode.
@DaniAkash
Dani Akash (DaniAkash) merged commit a6520f2 into browseros-ai:main Jul 23, 2026
22 checks passed
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.

[BUG] New tab page chat box does not respect Agent mode switch - always defaults to Chat mode

2 participants