Skip to content

[zrm] enable scribe zrm in packages#680

Open
AngeloGiacco wants to merge 1 commit into
mainfrom
angelo/enable-zrm-scribe
Open

[zrm] enable scribe zrm in packages#680
AngeloGiacco wants to merge 1 commit into
mainfrom
angelo/enable-zrm-scribe

Conversation

@AngeloGiacco

@AngeloGiacco AngeloGiacco commented Apr 15, 2026

Copy link
Copy Markdown
Collaborator

description

threads through the enable logging param, see #545

testing

added test

Note

Medium Risk
Adds a new enable_logging query parameter that changes server-side logging behavior for Scribe sessions; risk is limited but could affect observability and request compatibility if the backend expects different defaults.

Overview
Adds an enableLogging option to Scribe realtime connections and propagates it through the React useScribe hook so callers can explicitly enable/disable session logging.

Updates WebSocket URI construction to include enable_logging=true|false only when specified, and adds unit tests to cover true/false/omitted cases.

Reviewed by Cursor Bugbot for commit 6f07261. Bugbot is set up for automated code reviews on this repo. Configure here.

@kraenhansen

kraenhansen commented Apr 23, 2026

Copy link
Copy Markdown
Member

Still think we should consider using affirmative language (disableLogging: true) - was this considered and rejected?

Comment on lines +109 to +110
connection.close();
server.close();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This just extends an existing pattern, but closing of test servers should really be a part of an after each hook to prevent cross-talking between tests (in case the test throws midway).

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.

2 participants