Skip to content

fix(ui): persist chat messages across sessions + fix restrict_to_workspace#711

Open
ducphamhoang wants to merge 2 commits intonextlevelbuilder:devfrom
ducphamhoang:dev
Open

fix(ui): persist chat messages across sessions + fix restrict_to_workspace#711
ducphamhoang wants to merge 2 commits intonextlevelbuilder:devfrom
ducphamhoang:dev

Conversation

@ducphamhoang
Copy link
Copy Markdown

Summary

  • New Zustand store (use-chat-messages-store) for chat message state so messages persist when switching sessions and navigating back
  • Fixed infinite re-render loop (React Mise env management, module composability, and podman support #185) — inline ?? [] fallback in Zustand selector created a new array reference on every render; Object.is always detected a "change". Fixed with a stable EMPTY_MESSAGES module constant
  • Fixed first message disappearing in new chataddLocalMessage checked the component-level sessionKey which is still "" when send fires (navigation is async). Optimistic user message was silently dropped. Fixed by threading the explicit sessionKey through onMessageAdded in useChatSend
  • Fixed restrict_to_workspace always forced to true — HTTP agent update handler unconditionally overwrote the field on every update; WS handler had no support for it at all. Fixed both and updated effectiveRestrict() to read the per-agent setting instead of hardcoding true

Test plan

  • Open a new chat, send a message — optimistic message appears immediately and does not disappear
  • Switch between sessions — messages from previously visited sessions are preserved without re-fetching
  • Navigate back to a session — correct message history shown instantly from store
  • Update an agent's restrict_to_workspace via HTTP API — value is saved correctly (not always overwritten to true)
  • Update an agent's restrict_to_workspace via WS agents.update — field is accepted and persisted

🤖 Generated with Claude Code

…space

Migrate chat message state from component-local useState to a Zustand
store so messages survive session switches and back-navigation.

Three bugs fixed:

- Infinite re-render loop (React nextlevelbuilder#185): inline `?? []` fallback in
  Zustand selector created a new array reference every render; Zustand's
  Object.is check always detected a change. Fixed with a stable
  EMPTY_MESSAGES module constant.

- First message disappearing in new chat: addLocalMessage guarded on the
  component-level sessionKey which is still "" when send fires (navigate
  is async). Optimistic user message was silently dropped. Fixed by
  threading the explicit sessionKey through onMessageAdded in useChatSend.

- restrict_to_workspace always forced to true: HTTP agent update handler
  unconditionally overwrote the field; WS handler had no support for it
  at all. Fixed both and updated effectiveRestrict() to read the
  per-agent setting instead of hardcoding true.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ducphamhoang
Copy link
Copy Markdown
Author

Note on restrict_to_workspace changes

I want to clarify the intent behind the 3 backend changes to restrict_to_workspace, since they diverge from the current hardcoded return true approach.

What the current code does

effectiveRestrict() always returns true, which means the restrict_to_workspace column in the DB is silently ignored — whatever value is stored there has no effect. On top of that, the HTTP update handler overwrites it to true on every agent update, so even if someone sets it in the DB directly, the next UI save resets it.

What our change does

func effectiveRestrict(ctx context.Context, toolDefault bool) bool {
    if restrict, ok := RestrictFromCtx(ctx); ok {
        return restrict  // explicit per-agent override
    }
    return toolDefault   // falls back to tool default (true for filesystem tools)
}

Security is not weakened by default. Agents without an explicit restrict_to_workspace setting still resolve to true via toolDefault. The only difference is that an admin can consciously set restrict_to_workspace = false on a specific agent when the environment warrants it (local dev, air-gapped setups, controlled single-tenant deployments, mounting external host directories into the workspace, etc.).

Why this is better than hardcoding

  • The DB column already exists — it was clearly intended to be configurable at some point
  • Hardcoding true and silently ignoring the stored value is misleading and makes the schema lie
  • The HTTP handler bug (always overwriting to true on every save) means the field is currently broken even if you try to set it manually
  • Setting restrict_to_workspace = false already requires admin access via RBAC — there's no new privilege escalation path

Suggested compromise (if full flexibility is too broad)

If the concern is accidental misconfiguration in multi-tenant production, a middle ground would be to gate the toggle on an env/config flag like GOCLAW_ALLOW_WORKSPACE_OVERRIDE=true, defaulting to off. That way the hardcoded behavior is preserved in standard deployments while giving operators an explicit escape hatch.

Happy to adjust the approach — just want to make the existing DB column actually functional rather than decorative.

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