Skip to content

P2-T5: Warn when --web-ui requested but running broker lacks it#128

Merged
SoundBlaster merged 8 commits into
mainfrom
feature/P2-T5-webui-mismatch-warning
Mar 1, 2026
Merged

P2-T5: Warn when --web-ui requested but running broker lacks it#128
SoundBlaster merged 8 commits into
mainfrom
feature/P2-T5-webui-mismatch-warning

Conversation

@SoundBlaster
Copy link
Copy Markdown
Owner

Description

When the user starts a proxy with --broker --web-ui and a broker daemon is already running without the web UI, the proxy connects silently and --web-ui has no effect — the user sees no dashboard and no explanation.

Fix: After connecting to an existing broker (not one we just spawned), BrokerProxy probes 127.0.0.1:{web_ui_port} with a 0.5 s TCP timeout. If the port is not accepting connections, an actionable warning is printed to stderr:

Warning: broker is running without --web-ui on port 8080. Restart the broker to enable the dashboard.
  Hint: stop the running broker (rm ~/.mcpbridge_wrapper/broker.sock ~/.mcpbridge_wrapper/broker.pid) then reconnect with --broker --web-ui.

The MCP session continues normally — the warning is stderr-only.

Changes:

  • src/mcpbridge_wrapper/broker/proxy.py — added web_ui_port param, _new_broker_spawned flag, _warn_web_ui_mismatch() helper, and mismatch check in run() after connect; set _new_broker_spawned=True before subprocess.Popen
  • src/mcpbridge_wrapper/__main__.py — passes effective web UI port (web_ui_port or 8080 when web_ui_enabled) to BrokerProxy
  • tests/unit/test_broker_proxy.py — added TestBrokerProxyWebUIMismatch (5 tests)

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Refactoring
  • CI/CD improvement

Quality Gates

  • make test - All tests pass with ≥90% coverage (737 passed, 91.66% coverage)
  • make lint - No linting errors (ruff check src/ passed)
  • make format - Code is properly formatted (ruff format --check passed)
  • make typecheck - Type checking passes
  • make doccheck - Documentation is synced with DocC (if docs changed)

Documentation Sync

  • Documentation changes are synced with DocC catalog (or N/A) — no docs changed

Testing

  • Added/updated tests for new functionality (5 new tests in TestBrokerProxyWebUIMismatch)
  • All tests pass locally (737 passed, 0 failures)
  • Manually tested the changes

Checklist

  • Code follows the project's style guidelines
  • Self-review completed
  • Comments added for complex code
  • Documentation updated (if needed)
  • No new warnings generated
  • PR title is descriptive

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a user-facing warning for the “proxy requested --web-ui but connected to an already-running broker without the dashboard” scenario, improving UX without impacting the MCP JSON-RPC stream.

Changes:

  • Add a post-connect TCP probe in BrokerProxy to detect missing Web UI on the expected port and print an actionable stderr warning.
  • Thread the effective web_ui_port from CLI into BrokerProxy.
  • Add unit tests and archive/spec documentation updates for P2-T5.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/mcpbridge_wrapper/broker/proxy.py Adds web_ui_port, _new_broker_spawned, and _warn_web_ui_mismatch() probe + warning logic.
src/mcpbridge_wrapper/__main__.py Passes web_ui_port into BrokerProxy in broker proxy mode.
tests/unit/test_broker_proxy.py Adds TestBrokerProxyWebUIMismatch coverage for warning/probe behavior.
SPECS/Workplan.md Marks P2-T5 as completed and updates acceptance criteria checklist.
SPECS/INPROGRESS/next.md Archives P2-T5 and updates idle status/recently archived list.
SPECS/ARCHIVE/_Historical/REVIEW_P2-T5_webui_mismatch_warning.md Adds historical review report for P2-T5.
SPECS/ARCHIVE/P2-T5_Warn_when_broker_lacks_web_ui/P2-T5_Warn_when_broker_lacks_web_ui.md Adds PRD/spec for the implemented behavior.
SPECS/ARCHIVE/P2-T5_Warn_when_broker_lacks_web_ui/P2-T5_Validation_Report.md Adds validation report and quality gate results.
SPECS/ARCHIVE/INDEX.md Adds P2-T5 to archive index and updates last-updated metadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/unit/test_broker_proxy.py
Comment thread tests/unit/test_broker_proxy.py Outdated
Comment thread src/mcpbridge_wrapper/__main__.py Outdated
Comment thread src/mcpbridge_wrapper/broker/proxy.py Outdated
Comment thread src/mcpbridge_wrapper/broker/proxy.py
@SoundBlaster SoundBlaster merged commit de084ed into main Mar 1, 2026
10 checks passed
@SoundBlaster SoundBlaster deleted the feature/P2-T5-webui-mismatch-warning branch March 1, 2026 09:48
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