Skip to content

Add Chromium console streaming to the debugger - #164

Draft
bjdodson-openai wants to merge 1 commit into
bjd/debugger-storage-providerfrom
bjd/debugger-console
Draft

Add Chromium console streaming to the debugger#164
bjdodson-openai wants to merge 1 commit into
bjd/debugger-storage-providerfrom
bjd/debugger-console

Conversation

@bjdodson-openai

Copy link
Copy Markdown
Collaborator

Description

Streams Chromium console events into the debugger with bounded formatting, lifecycle cancellation, and secret redaction.

  • Ties every stream and response to the exact target generation.
  • Cancels discovery and streaming work on abort, shutdown, and target replacement.
  • Bounds retained entries and redacts credential-shaped content before presentation.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Documentation improvement
  • Performance optimization
  • Test improvement
  • Other (new debugger capability)

Testing

  • Tests pass locally (bazel test //...)
  • Added/updated tests for changes (if applicable)
  • Tested on multiple platforms (iOS/Android/Web/macOS as applicable)
  • Manual testing performed (describe below)

Testing Details

  • Incremental branch: Focused formatter/server 71/71 and author CLI 299/299 passed; restacked CLI 301/301, production build, syntax, package, lint, and format checks passed.
  • Assembled debugger stack: npm test passed 436/436; the CLI production build passed.
  • Focused //src/valdi_modules/src/valdi/web_renderer:test passed.
  • bazel query //... passed.
  • The broad Valdi suite reproduced the established 12 unrelated failures; all new debugger specs passed.

Checklist

  • Code follows project style guidelines
  • Documentation updated (if needed)
  • No breaking changes (or documented in description)
  • Commit messages follow conventional format
  • No secrets, API keys, or internal URLs included

Related Issues

Relates to #154

Additional Context

Stack 11/22. Stacked on #163 (bjd/debugger-storage-provider). Review this PR as the single incremental commit da506903 against that base; do not merge it before its parent.

@github-actions

Copy link
Copy Markdown

📊 PR Size: size/XL

Total changes: 1981 lines (11 files)

Top files changed:

  • npm_modules/cli/src/debugger/server.spec.ts: +410 -0
  • npm_modules/cli/src/debugger/server.ts: +336 -6
  • npm_modules/cli/src/debugger/chromiumConsole.ts: +304 -0
  • npm_modules/cli/src/debugger/devtoolsPanel.spec.ts: +243 -0
  • npm_modules/cli/src/debugger/chromiumConsole.spec.ts: +181 -0
  • npm_modules/cli/src/utils/owlCdpClient.spec.ts: +98 -51
  • npm_modules/cli/debugger/devtools-panel.js: +118 -20
  • npm_modules/cli/src/debugger/consoleSseWriter.spec.ts: +91 -0
  • npm_modules/cli/src/utils/chromiumDevToolsClient.ts: +60 -1
  • npm_modules/cli/src/utils/owlCdpClient.ts: +41 -12

...and 1 more files

Size calculated as additions + deletions. Labels: XS (<10), S (<50), M (<250), L (<1000), XL (1000+)

@clholgat clholgat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review of console streaming. The bounds, backpressure (128 events / 512KB, drain listener removed on close), teardown across abort/close/pagehide/target-replacement, and getter/deep-recursion handling all look solid, and multi-arg redaction is correctly double-applied (per-fragment + whole-message), so console.log('Authorization:', token) is covered. One redaction-coverage gap inline, plus one low note below.

Note: please carry this feedback into the squashed PR when the stack is collapsed.

  • 🟢 Low — formatChromiumConsoleEvent returns null for empty formatted output, so zero-arg / empty-string console calls (console.log(), console.log('')) are silently dropped from the stream. Developers see nothing for intentional blank/separator logs. Consider emitting an empty entry instead of dropping.

return [substituted, ...formatted.slice(nextArgument)].join(' ');
}

function redactConsoleText(text: string): string {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟠 Med — redactConsoleText only matches sk-/sess- prefixes, bearer …, and key-labeled values (authorization/cookie/*token/api_key/password/secret/etc). It misses common unlabeled high-entropy credential formats.

Failure scenario: a bare console.log(jwt) or console.log(githubToken) where the value is a JWT (eyJ…), AWS AKIA…, GitHub ghp_…, Google AIza…, or Slack xox… token — with no adjacent key name — is streamed and rendered verbatim in the panel, defeating the stated "redacts credential-shaped content" guarantee. Impact is limited to the local loopback debugger UI, but matters for shared/screenshotted panels.

Suggested fix: add standalone token-shape patterns (eyJ[\w-]+\.[\w-]+\.[\w-]+, ghp_[\w]{36}, AKIA[0-9A-Z]{16}, AIza[\w-]{35}, xox[baprs]-[\w-]+) to the redactor as defense-in-depth alongside the label-based rules.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants