Skip to content

fix(agent-monitor): do not call addEventListener where window has none - #430

Merged
omercnet merged 2 commits into
omercnet:mainfrom
andresantoniuk:fix/agent-monitor-android-window
Oct 29, 2026
Merged

omercnet merged 2 commits into
omercnet:mainfrom
andresantoniuk:fix/agent-monitor-android-window

Conversation

@andresantoniuk

@andresantoniuk andresantoniuk commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

On Android the whole agent-monitor plugin failed to load. React Native defines a window object, but it has no addEventListener. useKeyDown only checked that globalThis.window was an object, so it called a missing function and threw undefined is not a function during the contribution, which took the plugin down on Android.

Change

Use the window object only when it actually exposes both addEventListener and removeEventListener, so React Native's bare window is skipped (the keyboard listener is a web and desktop feature anyway) and cleanup cannot throw either.

Testing

  • Two regression tests in tests/web-keys.test.ts: a window object with no listener methods does not throw, and one with only addEventListener is not used.
  • bun run test: 117 green. bun run test:coverage: 96.53 lines / 93.33 functions. tsc and biome clean.
  • Verified on a real Android client against a 0.14.0 daemon: the plugin loads and the monitor, views and popover work.

Note: this text was produced with AI assistance, and the fix was reproduced, applied and verified on the device by the author.

Summary by CodeRabbit

  • Bug Fixes
    • Improved keyboard shortcut handling in environments where the browser window does not support the required event-listener methods. In these cases, shortcut setup now avoids attempting to register unsupported listeners, preventing errors when using the feature. Existing behavior remains unchanged in environments that support the required methods.

…e (Android)

React Native defines a window object without addEventListener. useKeyDown assumed any window object could listen and
threw 'undefined is not a function', which failed the whole plugin on Android. Use window only if it has
addEventListener and removeEventListener.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Walkthrough

webWindow now requires callable addEventListener and removeEventListener methods. Tests cover useKeyDown with an empty window object and one that has only addEventListener.

Changes: Web window validation

Layer / File(s) Summary
Validate web window and test incomplete objects
agent-monitor/client/web-keys.ts, agent-monitor/tests/web-keys.test.ts
webWindow rejects objects without callable event listener methods. Tests check useKeyDown with an empty window object and one that has only addEventListener.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: Minimal · up to 4f545

The change prevents the Android plugin failure without a demonstrated remaining merge-blocking risk.

Architecture risk: Low · up to 4f545

The change affects 1 system.

Changed systems: agent-monitor

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — agent-monitor (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in agent-monitor/client/web-keys.ts: webWindow now rejects candidates without callable addEventListener or removeEventListener methods; previously, any non-null object was cast to WebWindow.
  • observed — Modified behavior in agent-monitor/tests/web-keys.test.ts: Added a test that stubs window as an empty object and expects useKeyDown not to throw with or without the modal flag. The accompanying comment describes the React Native case and a reported error.
  • observed — Modified behavior in agent-monitor/tests/web-keys.test.ts: Added a test that stubs window with only a mocked addEventListener, calls useKeyDown, and expects the method not to be called; the test name also refers to cleanup.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the change: webWindow and useKeyDown now skip a window that lacks addEventListener and removeEventListener. The raw summary and PR objectives both describe this same guard, so the titl…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the change: webWindow and useKeyDown now skip a window that lacks addEventListener and removeEventListener. The raw summary and PR objectives both describe this same guard, so the title matches the main change. The title uses the conventional fix(scope) prefix and is 70 characters.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

Biome (2.5.15)
agent-monitor/tests/web-keys.test.ts

Biome could not lint this file: nested root configuration. Check the repository's Biome configuration and plugins.

Comment @coderabbitai help to get the list of available commands.

@coderabbitai
coderabbitai Bot requested a review from omercnet October 29, 2026 21:02
@omercnet
omercnet merged commit 404656f into omercnet:main Oct 29, 2026
11 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 29, 2026
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