feat(webapp): add browser observability console - #216
Conversation
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughAdded a loopback-only GLaDOS observability console. The change includes configuration, CLI lifecycle integration, multi-subscriber event streaming, telemetry serializers, HTTP and SSE endpoints, browser interfaces, fallback simulation, and tests. ChangesWebapp Console
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant WebappServer
participant Serializers
participant ObservabilityBus
Browser->>WebappServer: Request snapshot or SSE stream
WebappServer->>Serializers: Build JSON telemetry
Serializers->>WebappServer: Return snapshot or state frame
WebappServer->>ObservabilityBus: Subscribe to observation events
ObservabilityBus-->>WebappServer: Replay and deliver events
WebappServer-->>Browser: Send JSON or SSE data
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
src/glados/webapp/server.py (2)
115-131: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd
X-Content-Type-Options: nosniffand a CSP.The console renders engine-derived strings through
innerHTMLinstatic/index.html. The page is inline-only and loads no external resources, so a strict Content-Security-Policy costs nothing and blocks injected<script>from executing.Suggested response headers for the HTML and JSON responses:
X-Content-Type-Options: nosniffContent-Security-Policy: default-src 'none'; style-src 'unsafe-inline'; script-src 'unsafe-inline'; connect-src 'self'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/glados/webapp/server.py` around lines 115 - 131, Add X-Content-Type-Options: nosniff and the specified strict Content-Security-Policy headers in the shared response helpers _json and _text, ensuring both JSON and HTML responses receive them while preserving existing content-type and length headers.
237-237: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the
queueimport to module scope.Line 237 imports
queueinside_stream. The module already importsthreadingandpathlibat the top. A module-levelimport queueis more idiomatic and avoids the alias.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/glados/webapp/server.py` at line 237, Move the queue import from inside _stream to module scope alongside the existing threading and pathlib imports, and use the standard queue name instead of the _queue alias throughout _stream.src/glados/cli.py (1)
285-294: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the shared config-override block.
Lines 285-294 repeat the same override logic as
startat lines 229-238. A small helper keeps the three launchers consistent when a new override is added.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/glados/cli.py` around lines 285 - 294, Extract the repeated input_mode, tts_enabled, and asr_muted override logic from the current launcher and the start launcher into a shared helper. Have the helper accept GladosConfig and the optional override values, apply only non-None updates via model_copy, and use it from both launch paths so future overrides remain consistent.src/glados/webapp/static/index.html (1)
561-561: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winEscape values passed to
kv.
kvat line 561 insertsvintoinnerHTMLwithout escaping. Line 623 passess.statusands.importancestraight from/api/slots/{id}. Every other render path in this file usesesc. Makekvescape its value so the whole file is consistent.🛡️ Proposed fix
-function kv(k,v){ return '<div class="kv"><span class="k">'+k+'</span><span class="v">'+v+'</span></div>'; } +function kv(k,v){ return '<div class="kv"><span class="k">'+esc(k)+'</span><span class="v">'+esc(v)+'</span></div>'; }Line 569 then becomes
kv("Scene", vision)becausekvhandles the escaping.Also applies to: 622-624
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/glados/webapp/static/index.html` at line 561, Update the kv function to pass v through the existing esc helper before inserting it into the generated HTML. Keep callers such as the status and importance rendering unchanged so kv centrally escapes all displayed values.tests/test_webapp.py (1)
156-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a positive
Hostheader case.
test_server_rejects_cross_origin_browser_requestcovers the reject paths. No test covers the acceptedOrigincase, whereOriginmatches the requestHost. That branch atserver.pyline 181 comparesparsed.netloc.lower() == request_host.lower(). A same-origin browser sendsOrigin: http://127.0.0.1:<port>andHost: 127.0.0.1:<port>, so the comparison must succeed. A test would lock that behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_webapp.py` around lines 156 - 198, Extend test_webapp_server_serves_snapshot_and_stream with a same-origin request that sends an Origin header matching the server’s Host and assert the request is accepted. Build the Origin from the bound port, exercise an API endpoint such as /api/snapshot, and verify the successful response to cover the server.py host-comparison branch.src/glados/webapp/serializers.py (1)
133-139: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a failure-tolerant audio read.
_emotion_stateand_mcp_summaryboth wrap their engine reads intry/except._audio_statedoes not.float(snap.rms)raises ifsnapshot()fails or returnsNoneforrms. The same SSE-loop consequence described forbuild_lanesapplies.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/glados/webapp/serializers.py` around lines 133 - 139, Make _audio_state failure-tolerant like _emotion_state and _mcp_summary: wrap the audio_state snapshot and field conversion in the established exception handling, and return the neutral {"rms": 0.0, "vad_active": False} state whenever snapshot() fails or rms is unavailable/invalid, so SSE processing continues.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/webapp.md`:
- Around line 98-99: Update the _stream method so state frames are emitted
whenever at least 0.5 seconds have elapsed since the previous state frame,
including while processing observability events rather than only in the
queue.Empty branch. Track the last state-send time and preserve the existing
state payload and event-stream behavior.
In `@examples/webapp/index.html`:
- Around line 478-480: Replace the `"debug"` level value with `"debg"` for the
seeded events and emitted events in the relevant event data and logging paths,
including the entries near `audio.vad` and the code around lines 666-667.
Preserve all other event fields and behavior.
- Line 586: Correct the `el` call in the slot-card construction near the
affected code, passing `"slot"` as the class argument and an empty string as the
initial HTML argument. Preserve the subsequent innerHTML assignment and ensure
the generated element has className `"slot"` so the existing slot styles apply.
In `@src/glados/cli.py`:
- Around line 304-313: Update the startup flow around Glados.from_config,
WebappServer.start, and the server.is_running check so a failed bind invokes the
correct Glados shutdown entry point before raising SystemExit. Ensure cleanup
runs for engines initialized before run(), while preserving normal startup
behavior when the server binds successfully.
In `@src/glados/observability/bus.py`:
- Around line 45-60: Move the self._queue.put(event) call inside the with
self._lock block in publish, after updating history and subscribers, so
legacy-queue delivery preserves the same ordering as snapshot and SSE consumers.
Keep the queue operation unbounded and retain the existing subscriber handling.
In `@src/glados/webapp/static/index.html`:
- Around line 453-456: The renderFullWire function currently reverses eventLog
before addWireRow prepends rows, producing incorrect ordering and trimming the
wrong events. In src/glados/webapp/static/index.html lines 453-456, remove the
reverse operation and iterate eventLog in insertion order; apply the identical
change in examples/webapp/index.html lines 486-489 to keep both consoles
consistent.
- Line 635: Update the priority status handling around prioInflight and prioOf
so the UI does not represent the priority queue depth as an in-flight boolean:
either rename the displayed label and associated field to indicate queued work,
preserving the full queue depth, or expose and use a genuine priority-lane
in-flight counter from build_lanes.
In `@tests/test_webapp.py`:
- Around line 258-260: Add an inline Ruff S104 suppression to the intentional
"0.0.0.0" host literal in test_server_refuses_non_loopback_bind, keeping the
test behavior unchanged and limiting the suppression to this negative-test
occurrence.
---
Nitpick comments:
In `@src/glados/cli.py`:
- Around line 285-294: Extract the repeated input_mode, tts_enabled, and
asr_muted override logic from the current launcher and the start launcher into a
shared helper. Have the helper accept GladosConfig and the optional override
values, apply only non-None updates via model_copy, and use it from both launch
paths so future overrides remain consistent.
In `@src/glados/webapp/serializers.py`:
- Around line 133-139: Make _audio_state failure-tolerant like _emotion_state
and _mcp_summary: wrap the audio_state snapshot and field conversion in the
established exception handling, and return the neutral {"rms": 0.0,
"vad_active": False} state whenever snapshot() fails or rms is
unavailable/invalid, so SSE processing continues.
In `@src/glados/webapp/server.py`:
- Around line 115-131: Add X-Content-Type-Options: nosniff and the specified
strict Content-Security-Policy headers in the shared response helpers _json and
_text, ensuring both JSON and HTML responses receive them while preserving
existing content-type and length headers.
- Line 237: Move the queue import from inside _stream to module scope alongside
the existing threading and pathlib imports, and use the standard queue name
instead of the _queue alias throughout _stream.
In `@src/glados/webapp/static/index.html`:
- Line 561: Update the kv function to pass v through the existing esc helper
before inserting it into the generated HTML. Keep callers such as the status and
importance rendering unchanged so kv centrally escapes all displayed values.
In `@tests/test_webapp.py`:
- Around line 156-198: Extend test_webapp_server_serves_snapshot_and_stream with
a same-origin request that sends an Origin header matching the server’s Host and
assert the request is accepted. Build the Origin from the bound port, exercise
an API endpoint such as /api/snapshot, and verify the successful response to
cover the server.py host-comparison branch.
🪄 Autofix
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e78608d-4b17-47e1-83dd-b4b2f84cc47f
📒 Files selected for processing (12)
configs/glados_webapp_config.yamldocs/webapp.mdexamples/webapp/index.htmlsrc/glados/cli.pysrc/glados/core/engine.pysrc/glados/observability/bus.pysrc/glados/webapp/__init__.pysrc/glados/webapp/config.pysrc/glados/webapp/serializers.pysrc/glados/webapp/server.pysrc/glados/webapp/static/index.htmltests/test_webapp.py
| - `state` events — every ~0.5 s, mirroring `/api/state`, so gauges, clock, and | ||
| lane chips stay live without re-sending the whole snapshot. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
state pings are not guaranteed every 0.5 s.
The doc states that state events arrive every ~0.5 s. In src/glados/webapp/server.py _stream, the state frame is emitted only in the _queue.Empty branch of sub.get(timeout=0.5). While observability events arrive faster than one per 0.5 s, the timeout never fires and no state frame is sent. The browser gauges then freeze exactly when the engine is busiest.
Two options:
- Track the last state-send time and emit a
stateframe when 0.5 s has elapsed, regardless of which branch ran. - Change this doc line to describe the behavior as "when the event stream is idle".
The first option matches the stated intent.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/webapp.md` around lines 98 - 99, Update the _stream method so state
frames are emitted whenever at least 0.5 seconds have elapsed since the previous
state frame, including while processing observability events rather than only in
the queue.Empty branch. Track the last state-send time and preserve the existing
state payload and event-stream behavior.
| {o:-8.0, src:"autonomy",kind:"tick", level:"debug", msg:"tick coalesced 2 events", meta:{}}, | ||
| {o:-18.9, src:"mcp", kind:"server.status", level:"ok", msg:"MCP home-assistant online", meta:{tools:14}}, | ||
| {o:-45.2, src:"engine",kind:"audio.vad", level:"debug", msg:"VAD active", meta:{rms:-31.2}}, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use debg for the debug level.
LEVELS at line 450 contains debg, and the CSS at line 73 defines .lv.debg. Lines 478 and 480 seed events with level:"debug", and lines 666-667 emit "debug". Those rows get no level color, and the debg filter never matches them.
Change "debug" to "debg" at lines 478, 480, 666, and 667.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/webapp/index.html` around lines 478 - 480, Replace the `"debug"`
level value with `"debg"` for the seeded events and emitted events in the
relevant event data and logging paths, including the entries near `audio.vad`
and the code around lines 666-667. Preserve all other event fields and behavior.
| function renderSlots(){ | ||
| $("slots").innerHTML=""; | ||
| SLOTS.forEach(s => { | ||
| const c = el("div","slot"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
el("div","slot") passes the arguments in the wrong order.
el(cls, html) sets className from the first argument and innerHTML from the second. Line 586 therefore creates <div class="div">slot</div>. Line 588 overwrites the inner HTML, but the class stays div. Every .slot style rule is then lost, so the slot cards render unstyled.
The production console uses the correct form at src/glados/webapp/static/index.html line 546.
🐛 Proposed fix
- const c = el("div","slot");
+ const c = el("slot");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const c = el("div","slot"); | |
| const c = el("slot"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/webapp/index.html` at line 586, Correct the `el` call in the
slot-card construction near the affected code, passing `"slot"` as the class
argument and an empty string as the initial HTML argument. Preserve the
subsequent innerHTML assignment and ensure the generated element has className
`"slot"` so the existing slot styles apply.
| glados = Glados.from_config(glados_config) | ||
| server = WebappServer(glados, host=webapp_config.host, port=webapp_config.port) | ||
| server.start() | ||
| if not server.is_running: | ||
| logger.error( | ||
| "Webapp console could not bind {}:{} - aborting.", | ||
| webapp_config.host, | ||
| webapp_config.port, | ||
| ) | ||
| raise SystemExit(1) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Shut the engine down when the bind fails.
Line 304 builds the full Glados engine. Glados.__init__ starts component threads and opens the audio backend. If server.start() fails to bind, line 313 raises SystemExit(1) without any engine shutdown. The audio device and worker threads are never released through the normal _graceful_shutdown path.
Move the bind check inside a try/finally, or trigger the engine shutdown before exiting.
🛠️ Proposed fix
glados = Glados.from_config(glados_config)
server = WebappServer(glados, host=webapp_config.host, port=webapp_config.port)
server.start()
if not server.is_running:
logger.error(
"Webapp console could not bind {}:{} - aborting.",
webapp_config.host,
webapp_config.port,
)
+ glados.shutdown_event.set()
raise SystemExit(1)Confirm the correct shutdown entry point for a Glados instance that has not entered run().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/glados/cli.py` around lines 304 - 313, Update the startup flow around
Glados.from_config, WebappServer.start, and the server.is_running check so a
failed bind invokes the correct Glados shutdown entry point before raising
SystemExit. Ensure cleanup runs for engines initialized before run(), while
preserving normal startup behavior when the server binds successfully.
| def publish(self, event: ObservabilityEvent) -> None: | ||
| """Publish an event to history, the legacy queue, and all subscribers.""" | ||
| with self._lock: | ||
| self._history.append(event) | ||
| for subscriber in self._subscribers: | ||
| try: | ||
| subscriber.put_nowait(event) | ||
| except queue.Full: | ||
| # A slow subscriber must not block producers. Keep its | ||
| # newest events by evicting one oldest item. | ||
| try: | ||
| subscriber.get_nowait() | ||
| subscriber.put_nowait(event) | ||
| except (queue.Empty, queue.Full): | ||
| pass | ||
| self._queue.put(event) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Publish to the legacy queue inside the lock.
Line 60 puts the event on self._queue after the lock is released. Two concurrent producers can therefore append to _history and to subscriber queues in one order, then reach line 60 in the opposite order. The TUI drain() consumer then sees events out of order relative to snapshot() and to the SSE stream.
self._queue is unbounded, so put never blocks. Move it inside the with self._lock: block to keep one total order for all consumers.
🔒 Proposed fix
with self._lock:
self._history.append(event)
for subscriber in self._subscribers:
try:
subscriber.put_nowait(event)
except queue.Full:
# A slow subscriber must not block producers. Keep its
# newest events by evicting one oldest item.
try:
subscriber.get_nowait()
subscriber.put_nowait(event)
except (queue.Empty, queue.Full):
pass
- self._queue.put(event)
+ self._queue.put(event)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def publish(self, event: ObservabilityEvent) -> None: | |
| """Publish an event to history, the legacy queue, and all subscribers.""" | |
| with self._lock: | |
| self._history.append(event) | |
| for subscriber in self._subscribers: | |
| try: | |
| subscriber.put_nowait(event) | |
| except queue.Full: | |
| # A slow subscriber must not block producers. Keep its | |
| # newest events by evicting one oldest item. | |
| try: | |
| subscriber.get_nowait() | |
| subscriber.put_nowait(event) | |
| except (queue.Empty, queue.Full): | |
| pass | |
| self._queue.put(event) | |
| def publish(self, event: ObservabilityEvent) -> None: | |
| """Publish an event to history, the legacy queue, and all subscribers.""" | |
| with self._lock: | |
| self._history.append(event) | |
| for subscriber in self._subscribers: | |
| try: | |
| subscriber.put_nowait(event) | |
| except queue.Full: | |
| # A slow subscriber must not block producers. Keep its | |
| # newest events by evicting one oldest item. | |
| try: | |
| subscriber.get_nowait() | |
| subscriber.put_nowait(event) | |
| except (queue.Empty, queue.Full): | |
| pass | |
| self._queue.put(event) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/glados/observability/bus.py` around lines 45 - 60, Move the
self._queue.put(event) call inside the with self._lock block in publish, after
updating history and subscribers, so legacy-queue delivery preserves the same
ordering as snapshot and SSE consumers. Keep the queue operation unbounded and
retain the existing subscriber handling.
| function renderFullWire(){ | ||
| const host=$("full-wire"); host.innerHTML=""; | ||
| [...eventLog].reverse().forEach(ev => { if(wirePass(ev.level)) addWireRow(host, ev, false); }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
renderFullWire inverts event order in both consoles. Both files copy the same function. It iterates [...eventLog].reverse() (newest first) and calls addWireRow, which uses host.prepend. The last row prepended is the oldest, so the rendered list is oldest-first. The cap||110 trim in addWireRow then removes from the bottom, which holds the newest rows during this re-render. Opening "The Wire" shows the 110 oldest events in reverse order.
src/glados/webapp/static/index.html#L453-L456: drop the.reverse()and iterateeventLogin insertion order.examples/webapp/index.html#L486-L489: apply the same change so the mockup keeps matching the production console.
📍 Affects 2 files
src/glados/webapp/static/index.html#L453-L456(this comment)examples/webapp/index.html#L486-L489
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/glados/webapp/static/index.html` around lines 453 - 456, The
renderFullWire function currently reverses eventLog before addWireRow prepends
rows, producing incorrect ordering and trimming the wrong events. In
src/glados/webapp/static/index.html lines 453-456, remove the reverse operation
and iterate eventLog in insertion order; apply the identical change in
examples/webapp/index.html lines 486-489 to keep both consoles consistent.
| S.mcpServers=(j.mcp&&j.mcp.servers)||[]; | ||
| if(j.emotion) S.emo=[j.emotion.pleasure,j.emotion.arousal,j.emotion.dominance]; | ||
| S.lanes = Object.assign({prioQ:0,autoQ:0,autoInflight:0,workers:0,enabled:false}, j.lanes||{}); | ||
| S.prioInflight = prioOf(j.lanes)>0 ? 1 : 0; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
prioInflight reports a queue depth as a boolean.
prioOf reads lanes.priority.queue, which is the pending queue size from build_lanes. Line 635 collapses it to 1 or 0 and the UI labels it "in-flight". A queued item is not an in-flight inference, and a depth of 5 still displays as 1.
Either rename the UI label to "queued", or expose a real priority-lane in-flight counter from build_lanes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/glados/webapp/static/index.html` at line 635, Update the priority status
handling around prioInflight and prioOf so the UI does not represent the
priority queue depth as an in-flight boolean: either rename the displayed label
and associated field to indicate queued work, preserving the full queue depth,
or expose and use a genuine priority-lane in-flight counter from build_lanes.
| def test_server_refuses_non_loopback_bind() -> None: | ||
| with pytest.raises(ValueError, match="loopback-only"): | ||
| WebappServer(_FakeEngine(), host="0.0.0.0", port=8050) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Silence the S104 finding on this negative test.
Ruff reports S104 ("possible binding to all interfaces") at line 260. The literal is intentional here: the test proves WebappServer rejects a non-loopback host. Add an inline suppression so the rule stays enforced elsewhere.
🔧 Proposed fix
def test_server_refuses_non_loopback_bind() -> None:
with pytest.raises(ValueError, match="loopback-only"):
- WebappServer(_FakeEngine(), host="0.0.0.0", port=8050)
+ WebappServer(_FakeEngine(), host="0.0.0.0", port=8050) # noqa: S104 - negative test📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_server_refuses_non_loopback_bind() -> None: | |
| with pytest.raises(ValueError, match="loopback-only"): | |
| WebappServer(_FakeEngine(), host="0.0.0.0", port=8050) | |
| def test_server_refuses_non_loopback_bind() -> None: | |
| with pytest.raises(ValueError, match="loopback-only"): | |
| WebappServer(_FakeEngine(), host="0.0.0.0", port=8050) # noqa: S104 - negative test |
🧰 Tools
🪛 Ruff (0.16.1)
[error] 260-260: Possible binding to all interfaces
(S104)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_webapp.py` around lines 258 - 260, Add an inline Ruff S104
suppression to the intentional "0.0.0.0" host literal in
test_server_refuses_non_loopback_bind, keeping the test behavior unchanged and
limiting the suppression to this negative-test occurrence.
Source: Linters/SAST tools
Fixes Applied SuccessfullyFixed 6 file(s) based on 8 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixes Applied SuccessfullyFixed 6 file(s) based on 8 unresolved review comments. A stacked PR containing fixes has been created.
Time taken: |
Fixed 6 file(s) based on 8 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
|
❌ Failed to clone repository into sandbox. Please try again. |
1 participant
Summary
glados webappObservabilityBuswith bounded independent subscriptions without changing the existing TUI drain queueSafety and correctness
127.0.0.1orlocalhost) because this demo has no authentication boundaryValidation
tests/test_webapp.py: 16 passedglados/webapp/static/index.htmlglados.autonomy.preferencesmoduleFollow-up to merged #215.
Summary by CodeRabbit
New Features
webappcommand with configurable host, port, and environment-variable overrides.Documentation
Tests