feat(cli): add moss playground command with WASM-based query UI - #496
Open
msranjana wants to merge 29 commits into
Open
feat(cli): add moss playground command with WASM-based query UI#496msranjana wants to merge 29 commits into
msranjana wants to merge 29 commits into
Conversation
msranjana
requested review from
HarshaNalluru,
ashvathsureshkumar and
r4ghu
as code owners
July 29, 2026 03:18
Codex reviewThe PR adds a useful local playground, but the current implementation has a credential-trust issue and stale client state when switching connections. |
msranjana
force-pushed
the
feat/local-playground-ui
branch
from
July 29, 2026 03:22
627bea6 to
ce2f2ff
Compare
- New playground.py command that starts a local HTTP server on :8765 - Serves index.html with importmap pointing to unpkg CDN (avoids esm.sh WASM loading issues with module.require shims) - Includes /api/indexes and /api/index endpoints backed by MossClient - Browser loads @moss-dev/moss-web via WASM; queries run locally with adjustable Top-K and Alpha controls - Fixes: favicon.ico returns 204, log_message handles variable args
msranjana
force-pushed
the
feat/local-playground-ui
branch
from
July 29, 2026 03:24
ce2f2ff to
24e8ba3
Compare
Contributor
|
@msranjana address these comments |
…er, fix stale results, fix alpha-zero, fix load-btn state, remove --index-dir
…al API from cross-origin abuse
…+ select check, disable select during load
…calhost and 127.0.0.1 origins
…idate topK/alpha server-side
…t-loop worker thread
…d starts I have updated the documentation (if applicable).
msranjana
force-pushed
the
feat/local-playground-ui
branch
from
July 30, 2026 18:22
f1c55ee to
72d167c
Compare
I have updated the documentation (if applicable).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe CLI adds a local browser playground for browsing indexes, loading an index, and running configurable searches through a credentialed Moss client. ChangesLocal Playground
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant PlaygroundHandler
participant AsyncWorker
participant MossClient
Browser->>PlaygroundHandler: POST /api/query with query options
PlaygroundHandler->>AsyncWorker: Submit query operation
AsyncWorker->>MossClient: Execute query through MossClient
MossClient-->>AsyncWorker: Return matching results
AsyncWorker-->>PlaygroundHandler: Return serialized query data
PlaygroundHandler-->>Browser: JSON results and timing
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 9
🧹 Nitpick comments (8)
packages/moss-cli/src/moss_cli/commands/playground.py (4)
132-135: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winCompare the token with
secrets.compare_digest.The
!=comparison onstrshort-circuits on the first differing byte. Use a constant-time comparison for the shared secret. The change is small and removes a timing side channel.🔒 Proposed fix
- if self.headers.get("X-Moss-Token") != self._token: + provided = self.headers.get("X-Moss-Token") or "" + if not self._token or not secrets.compare_digest(provided, self._token): self._send_json(403, {"error": "Forbidden: invalid or missing token"}) return False🤖 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 `@packages/moss-cli/src/moss_cli/commands/playground.py` around lines 132 - 135, Update _check_api_request to compare the X-Moss-Token header with self._token using secrets.compare_digest instead of !=, preserving the existing 403 response and False return for invalid or missing tokens.
387-392: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unreachable credential check.
resolve_credentialsnever returns empty values. It raisestyper.BadParameterwhen credentials are missing, as shown inpackages/moss-cli/src/moss_cli/config.pyLines 168-173. The block below therefore never runs, and its message duplicates the error text thatresolve_credentialsalready produces.♻️ Proposed refactor
- if not pid or not pkey: - console.print( - "[red]No credentials found.[/red] Run [bold]moss init[/bold] first " - "or set MOSS_PROJECT_ID and MOSS_PROJECT_KEY." - ) - raise typer.Exit(1) -🤖 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 `@packages/moss-cli/src/moss_cli/commands/playground.py` around lines 387 - 392, Remove the unreachable `if not pid or not pkey` credential check and its associated console message and `typer.Exit` from the command flow. Rely on `resolve_credentials` to raise `typer.BadParameter` when credentials are missing, while leaving valid credential handling unchanged.
82-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the return value of
stopto state its meaning.
stopreturnsTruewhen the thread is still alive, so the caller at Line 432 readsif worker.stop():as success. The inverted meaning is easy to misread in later changes. Return a named result or invert the value.♻️ Proposed refactor
- def stop(self, timeout: float | None = 5.0) -> bool: + def stop(self, timeout: float | None = 5.0) -> bool: + """Stop the loop and join the thread. Return True if the thread stopped.""" self._loop.call_soon_threadsafe(self._loop.stop) self._thread.join(timeout) - return self._thread.is_alive() + return not self._thread.is_alive()Then update the caller:
if not worker.stop(): console.print("[yellow]Worker thread did not shut down gracefully within timeout[/yellow]")🤖 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 `@packages/moss-cli/src/moss_cli/commands/playground.py` around lines 82 - 85, Update the stop method’s return contract and its caller so the result clearly represents whether shutdown completed successfully: invert the current self._thread.is_alive() result in stop, then change the worker.stop() check to handle failure with the existing warning message when shutdown does not complete within the timeout.
285-289: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBoth POST handlers accept non-string values for request-body fields.
topK,alpha, andrequestIdreceive strictisinstancechecks, butnameandqueryusedata.get(field, "")and only a truthiness test. A JSON number, list, or object is truthy, reaches the PyO3 boundary, and surfaces as an opaque 500 rather than a 400. This replaces the two per-site comments.
packages/moss-cli/src/moss_cli/commands/playground.py#L285-L289: in_handle_post_query, requireisinstance(name, str)andisinstance(query, str), each non-empty afterstrip().packages/moss-cli/src/moss_cli/commands/playground.py#L269-L272: in_handle_post_load_index, requireisinstance(name, str)and non-empty afterstrip().🛡️ Proposed fix: shared field extractor
def _require_str(self, data: dict, field: str) -> str | None: """Return the field value, or send a 400 response and return None.""" value = data.get(field) if not isinstance(value, str) or not value.strip(): self._send_json(400, {"error": f"Missing or invalid '{field}' in request body"}) return None return value- name = data.get("name", "") - query = data.get("query", "") - if not name or not query: - self._send_json(400, {"error": "Missing 'name' or 'query' in request body"}) - return + name = self._require_str(data, "name") + if name is None: + return + query = self._require_str(data, "query") + if query is None: + return🤖 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 `@packages/moss-cli/src/moss_cli/commands/playground.py` around lines 285 - 289, Update _handle_post_query at packages/moss-cli/src/moss_cli/commands/playground.py#L285-L289 to require name and query to be strings containing non-whitespace characters, returning HTTP 400 for missing or invalid values. Update _handle_post_load_index at packages/moss-cli/src/moss_cli/commands/playground.py#L269-L272 with the same validation for name. Reuse a shared string-field validation helper if appropriate, while preserving the existing error-response behavior.packages/moss-cli/src/moss_cli/playground/index.html (3)
249-252: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the
searchTimeoutdeclaration next to the other state variables.
loadIndexreadssearchTimeoutat Line 250, butlet searchTimeout = null;appears later at Line 368. The code works becauseloadIndexruns only from the click listener after module evaluation completes. Any future call during module evaluation would raise a temporal dead zoneReferenceError. DeclaresearchTimeoutwithindexes,currentIndex,requestId, andsearchAbortat Lines 148-151.♻️ Proposed refactor
let requestId = 0; let searchAbort = null; +let searchTimeout = null;-let searchTimeout = null; - function handleSearch(e) {🤖 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 `@packages/moss-cli/src/moss_cli/playground/index.html` around lines 249 - 252, Move the existing searchTimeout declaration from its later location to the state-variable declarations alongside indexes, currentIndex, requestId, and searchAbort; remove the original declaration and preserve its initialization and all existing timeout behavior.
89-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd accessible names to the form controls.
Three controls have no accessible name:
#index-selectat Line 89 relies on the.panel-titlediv, which is not a label.#search-inputat Line 111 has only aplaceholder. A placeholder is not an accessible name.#alpha-sliderat Line 121 sits between two<span>elements, which do not label it.A screen reader announces these as unlabeled controls. Add
aria-labelattributes.♿ Proposed fix
- <select id="index-select"> + <select id="index-select" aria-label="Select an index">- <input type="text" id="search-input" placeholder="Type to search..." disabled /> + <input type="text" id="search-input" placeholder="Type to search..." aria-label="Search query" disabled />- <input type="range" id="alpha-slider" min="0" max="1" step="0.05" value="0.5" disabled /> + <input type="range" id="alpha-slider" min="0" max="1" step="0.05" value="0.5" + aria-label="Alpha: keyword to semantic balance" disabled />Also consider
aria-live="polite"on#load-statusand#resultsso status changes are announced.Also applies to: 111-111, 121-121
🤖 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 `@packages/moss-cli/src/moss_cli/playground/index.html` around lines 89 - 91, Add accessible names to the form controls in the playground markup: add descriptive aria-label attributes to `#index-select`, `#search-input`, and `#alpha-slider`. Do not rely on the surrounding .panel-title, placeholder text, or adjacent spans as labels; leave the optional aria-live enhancement for `#load-status` and `#results` out of scope.
418-422: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winBuild the error box with DOM APIs instead of
innerHTML.This is the only place where dynamic data reaches
innerHTML.escapeHtmlat Lines 362-366 escapes<,>, and&, but it does not escape quotes, so the helper is unsafe if it is ever reused for an attribute value. The rest of the file already usestextContent. Construct the node directly and deleteescapeHtml.Note that the
catchblock also swallowsAbortError. ThethisAbort.signal.abortedcheck at Line 419 covers it, so no extra guard is needed.♻️ Proposed refactor
} catch (e) { if (thisRequestId !== requestId || thisAbort.signal.aborted) return; console.error('Query failed:', e); - results.innerHTML = `<div class="status-box status-error">Query failed: ${escapeHtml(e.message || e)}</div>`; + results.innerHTML = ''; + const box = document.createElement('div'); + box.className = 'status-box status-error'; + box.textContent = `Query failed: ${e.message || e}`; + results.appendChild(box); }Then remove the now-unused helper:
-function escapeHtml(text) { - const div = document.createElement('div'); - div.textContent = text; - return div.innerHTML; -} -🤖 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 `@packages/moss-cli/src/moss_cli/playground/index.html` around lines 418 - 422, Update the catch block in the request handling flow to build the error status box with DOM APIs and assign the dynamic error text via textContent, while preserving the existing status-box/status-error structure and abort/request guards. Remove the now-unused escapeHtml helper; do not add an extra AbortError guard.Source: Linters/SAST tools
packages/moss-cli/tests/test_playground.py (1)
4-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the request validation and token check.
This smoke test covers only asset presence. The PR checklist records that feature tests were not completed. The highest-value untested logic is deterministic and needs no network:
_check_api_requestrejects a missing token, a wrong token, a foreignHost, and a foreignOrigin._handle_post_queryrejects a non-integertopK, a non-finitealpha, an out-of-rangetopK, an out-of-rangealpha, and a missingrequestId.- The
_latest_request_idcomparison marks an older request as superseded.You can drive these through
http.serverwith a stubclientassigned toPlaygroundHandler.client, or by startingDaemonThreadingHTTPServeron an ephemeral port inside a fixture.Do you want me to generate these tests?
🤖 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 `@packages/moss-cli/tests/test_playground.py` around lines 4 - 8, Expand the playground tests beyond test_playground_html_asset_exists by exercising PlaygroundHandler request validation and token checks with a stub client and local HTTP server or direct handler setup. Cover _check_api_request for missing/wrong tokens and foreign Host/Origin, and _handle_post_query for invalid topK/alpha values and missing requestId. Also verify _latest_request_id marks an older request as superseded, keeping all tests network-independent.
🤖 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 `@packages/moss-cli/README.md`:
- Around line 153-170: Update the playground documentation examples and
description: state that port 8765 is used when available and the next free port
otherwise, add the --profile staging example, instruct users with --no-open to
use the exact URL printed by the command including its #<token> fragment, and
move the API-proxy statement outside the capabilities list.
In `@packages/moss-cli/src/moss_cli/commands/playground.py`:
- Around line 243-244: Update the request-body handling around Content-Length in
the authenticated handler to safely parse the header, return a 400 response for
malformed or negative values, and reject values above the permitted body-size
limit before calling self.rfile.read. Preserve the existing empty-body fallback
for a zero length.
- Around line 324-328: Update PlaygroundHandler request staleness tracking so
_latest_request_id is scoped per browser session instead of shared server-wide,
using the session identifier supplied by the client; apply the same per-session
lookup in the in-worker recheck around the query execution path, and include
sessionId in /api/query requests from index.html so page reloads start a fresh
session.
- Line 350: Update packages/moss-cli/src/moss_cli/commands/playground.py at
lines 350-350, 208-208, 228-228, and 277-277 in _handle_post_query,
_handle_list_indexes, _handle_get_index, and _handle_post_load_index so
responses use a fixed safe error message instead of str(e), while logging the
exception detail locally; centralize this behavior in a shared helper if
appropriate.
- Around line 230-236: Update log_message to escape each client-controlled value
with rich.markup.escape before interpolating it into any console.print message,
preserving the existing argument-count formatting while preventing request data
from being interpreted as Rich markup.
In `@packages/moss-cli/src/moss_cli/playground/index.html`:
- Around line 237-242: Update the `.index-info` styling to use `white-space:
pre-wrap` so the newline separators appended by `showIndexInfo` render as line
breaks; leave the existing row-building logic unchanged.
- Around line 388-390: Update the topK parsing near rawTopK, parsedTopK, and
topK to round the numeric value and clamp it to the inclusive range 1–50 before
sending the request. Preserve the existing default of 5 for blank or non-numeric
input so the server always receives a valid integer.
- Around line 140-146: Update the token initialization in the module script to
read the hash once, then remove it from the address bar using
history.replaceState so it is not retained in browser history. Add startup
handling around the existing initialization flow to detect an empty mossToken
and display a direct missing-token message instead of attempting API calls and
showing a generic list failure.
- Around line 411-414: Replace direct res.json() parsing for non-OK responses
with one shared errorFromResponse helper that reads the body as text, attempts
JSON.parse to reuse a server-provided error, and falls back to an error
containing the HTTP status. Apply the helper in the /api/query handler at
packages/moss-cli/src/moss_cli/playground/index.html#L411-L414, refreshIndexList
at `#L170-L173`, and loadIndex at `#L267-L270`.
---
Nitpick comments:
In `@packages/moss-cli/src/moss_cli/commands/playground.py`:
- Around line 132-135: Update _check_api_request to compare the X-Moss-Token
header with self._token using secrets.compare_digest instead of !=, preserving
the existing 403 response and False return for invalid or missing tokens.
- Around line 387-392: Remove the unreachable `if not pid or not pkey`
credential check and its associated console message and `typer.Exit` from the
command flow. Rely on `resolve_credentials` to raise `typer.BadParameter` when
credentials are missing, while leaving valid credential handling unchanged.
- Around line 82-85: Update the stop method’s return contract and its caller so
the result clearly represents whether shutdown completed successfully: invert
the current self._thread.is_alive() result in stop, then change the
worker.stop() check to handle failure with the existing warning message when
shutdown does not complete within the timeout.
- Around line 285-289: Update _handle_post_query at
packages/moss-cli/src/moss_cli/commands/playground.py#L285-L289 to require name
and query to be strings containing non-whitespace characters, returning HTTP 400
for missing or invalid values. Update _handle_post_load_index at
packages/moss-cli/src/moss_cli/commands/playground.py#L269-L272 with the same
validation for name. Reuse a shared string-field validation helper if
appropriate, while preserving the existing error-response behavior.
In `@packages/moss-cli/src/moss_cli/playground/index.html`:
- Around line 249-252: Move the existing searchTimeout declaration from its
later location to the state-variable declarations alongside indexes,
currentIndex, requestId, and searchAbort; remove the original declaration and
preserve its initialization and all existing timeout behavior.
- Around line 89-91: Add accessible names to the form controls in the playground
markup: add descriptive aria-label attributes to `#index-select`, `#search-input`,
and `#alpha-slider`. Do not rely on the surrounding .panel-title, placeholder
text, or adjacent spans as labels; leave the optional aria-live enhancement for
`#load-status` and `#results` out of scope.
- Around line 418-422: Update the catch block in the request handling flow to
build the error status box with DOM APIs and assign the dynamic error text via
textContent, while preserving the existing status-box/status-error structure and
abort/request guards. Remove the now-unused escapeHtml helper; do not add an
extra AbortError guard.
In `@packages/moss-cli/tests/test_playground.py`:
- Around line 4-8: Expand the playground tests beyond
test_playground_html_asset_exists by exercising PlaygroundHandler request
validation and token checks with a stub client and local HTTP server or direct
handler setup. Cover _check_api_request for missing/wrong tokens and foreign
Host/Origin, and _handle_post_query for invalid topK/alpha values and missing
requestId. Also verify _latest_request_id marks an older request as superseded,
keeping all tests network-independent.
🪄 Autofix (Beta)
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: c2fbf5d6-7a44-41a9-9f10-efd55bd23ff0
📒 Files selected for processing (6)
packages/moss-cli/README.mdpackages/moss-cli/pyproject.tomlpackages/moss-cli/src/moss_cli/commands/playground.pypackages/moss-cli/src/moss_cli/main.pypackages/moss-cli/src/moss_cli/playground/index.htmlpackages/moss-cli/tests/test_playground.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/moss-cli/tests/test_playground.py`:
- Around line 181-201: Scope stale-query tracking in PlaygroundHandler to
individual browser sessions instead of the shared _latest_request_id class
value. Add or reuse a browser session identifier and maintain the latest request
ID per session, ensuring reloads and concurrent tabs can submit lower IDs
without being marked superseded. Update the stale-request tests to cover both
scenarios and verify valid queries are processed.
🪄 Autofix (Beta)
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: 810982fd-d14d-415a-ab53-0fbb2edcbd2f
📒 Files selected for processing (1)
packages/moss-cli/tests/test_playground.py
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/moss-cli/src/moss_cli/commands/playground.py">
<violation number="1" location="packages/moss-cli/src/moss_cli/commands/playground.py:86">
P2: Ctrl+C can still block process exit during an in-flight index load or query because stopping the event loop does not stop the `asyncio.to_thread` executor work. Track/cancel or drain outstanding operations and shut down the executor before returning from the command.</violation>
<violation number="2" location="packages/moss-cli/src/moss_cli/commands/playground.py:303">
P2: Loading several indexes during one playground session retains all of them in server memory, so exploring large indexes can grow the process until it is killed. Track the active index and unload the previous one before loading a replacement, or provide an explicit unload/eviction policy.</violation>
<violation number="3" location="packages/moss-cli/src/moss_cli/commands/playground.py:345">
P2: An oversized integer `alpha` crashes the request handler instead of returning a validation error; catch conversion overflow before the existing finite/range checks.</violation>
<violation number="4" location="packages/moss-cli/src/moss_cli/commands/playground.py:394">
P2: Automatic port selection has a time-of-check/time-of-use race: a port reported free can be occupied before `ThreadingHTTPServer` binds it, making `moss playground` fail even though later ports may be available. Binding the actual HTTP server while probing, or retrying server construction across the candidate range, would make the next-free-port guarantee reliable.</violation>
<violation number="5" location="packages/moss-cli/src/moss_cli/commands/playground.py:419">
P1: Running `moss playground` without CLI, environment, or profile credentials exits before the browser can open, so the advertised manual connection fallback is unreachable. The command would need to start a credential-free UI/server path and provide a manual connection form (with corresponding credential handling) instead of unconditionally calling `resolve_credentials` first.</violation>
</file>
<file name="packages/moss-cli/src/moss_cli/playground/index.html">
<violation number="1" location="packages/moss-cli/src/moss_cli/playground/index.html:419">
P0: Searches are executed by the credentialed Python server rather than by the browser WASM UI. As a result, this command does not deliver the advertised WASM-based/local-browser query behavior and adds a server-side query endpoint that the feature contract explicitly excludes; wiring the page to `@moss-dev/moss-web` and keeping the server limited to index retrieval/loading would preserve the intended architecture.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/moss-cli/src/moss_cli/commands/playground.py`:
- Around line 303-313: Update the nested _call function to set
PlaygroundHandler._loaded_index to None immediately after a successful
unload_index(previous) completes, before calling load_index(name). Add a
regression test covering successful unload followed by load_index raising, and
verify the tracked index remains None.
🪄 Autofix (Beta)
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: 8c90bf2d-e167-4d02-b7d3-c7aa96b29f16
📒 Files selected for processing (3)
packages/moss-cli/src/moss_cli/commands/playground.pypackages/moss-cli/src/moss_cli/playground/index.htmlpackages/moss-cli/tests/test_playground.py
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/moss-cli/src/moss_cli/playground/index.html
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new moss playground subcommand to the Moss CLI that starts a local HTTP server and serves a browser UI for browsing indexes, loading an index, and running queries interactively.
Changes:
- Added
moss playgroundTyper command backed by a localThreadingHTTPServerwith token-gated API endpoints. - Added a static single-page playground UI (
index.html) plus packaging config to ship the HTML asset. - Added unit tests covering request validation, token/host/origin checks, and stale-query supersession behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/moss-cli/src/moss_cli/commands/playground.py | Implements the playground server, API endpoints, token checks, and CLI command wiring. |
| packages/moss-cli/src/moss_cli/playground/index.html | Adds the browser-based UI that calls the local playground API. |
| packages/moss-cli/src/moss_cli/main.py | Registers the new playground command in the CLI. |
| packages/moss-cli/tests/test_playground.py | Adds tests for API validation, auth checks, and concurrency/supersession logic. |
| packages/moss-cli/README.md | Documents usage for moss playground and how the tokenized URL works. |
| packages/moss-cli/pyproject.toml | Includes the playground HTML file in package data for distribution. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def playground_command( | ||
| ctx: typer.Context, | ||
| port: int = typer.Option(0, "--port", "-p", help="Port for the HTTP server (0 = auto)"), |
| if not isinstance(value, str) or not value.strip(): | ||
| self._send_json(400, {"error": message}) | ||
| return None | ||
| return value |
Comment on lines
+451
to
+454
| worker = AsyncWorker() | ||
| client = worker.submit(lambda: MossClient(pid, pkey)) | ||
|
|
||
| # Start server |
Comment on lines
+419
to
+423
| const res = await apiFetch('/api/query', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| signal: thisAbort.signal, | ||
| body: JSON.stringify({ name: idx, query, topK, alpha, requestId: thisRequestId, sessionId }), |
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/moss-cli/src/moss_cli/commands/playground.py">
<violation number="1" location="packages/moss-cli/src/moss_cli/commands/playground.py:306">
P2: When loading an index fails after the previous index was already unloaded, `_loaded_index` still names the unloaded previous index. The next switch to a different index will try to `unload_index` an index that is no longer loaded, which can raise in the SDK and turn a user typo (bad index name) into a working-index teardown plus a confusing 500 on the next load. Consider resetting `_loaded_index` (or restoring the previous value) when `load_index` fails inside `_call`, so the unload bookkeeping stays consistent even after a failed switch.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| async def _call(): | ||
| previous = PlaygroundHandler._loaded_index | ||
| if previous is not None and previous != name: | ||
| unload = client.unload_index(previous) |
Contributor
There was a problem hiding this comment.
P2: When loading an index fails after the previous index was already unloaded, _loaded_index still names the unloaded previous index. The next switch to a different index will try to unload_index an index that is no longer loaded, which can raise in the SDK and turn a user typo (bad index name) into a working-index teardown plus a confusing 500 on the next load. Consider resetting _loaded_index (or restoring the previous value) when load_index fails inside _call, so the unload bookkeeping stays consistent even after a failed switch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/moss-cli/src/moss_cli/commands/playground.py, line 306:
<comment>When loading an index fails after the previous index was already unloaded, `_loaded_index` still names the unloaded previous index. The next switch to a different index will try to `unload_index` an index that is no longer loaded, which can raise in the SDK and turn a user typo (bad index name) into a working-index teardown plus a confusing 500 on the next load. Consider resetting `_loaded_index` (or restoring the previous value) when `load_index` fails inside `_call`, so the unload bookkeeping stays consistent even after a failed switch.</comment>
<file context>
@@ -300,7 +300,18 @@ def _handle_post_load_index(self, data: dict) -> None:
+ async def _call():
+ previous = PlaygroundHandler._loaded_index
+ if previous is not None and previous != name:
+ unload = client.unload_index(previous)
+ if inspect.isawaitable(unload):
+ await unload
</file context>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| <script type="importmap"> | ||
| { | ||
| "imports": { | ||
| "@moss-dev/moss-web": "https://unpkg.com/@moss-dev/moss-web@1.0.0/dist/index.js", |
There was a problem hiding this comment.
BLOCKING ```html
"@moss-dev/moss-web": "https://unpkg.com/@moss-dev/moss-web@1.0.0/dist/index.js",
The page executes remote CDN modules and then receives the configured `projectKey` from `/api/config`, so every auto-connected playground session trusts those CDN responses with the user's credentials. Fix by bundling/serving the JS/WASM assets from the CLI package with `script-src 'self'`, or keep the key on the local server and proxy the Moss calls instead of exposing it to remotely loaded code.
| connect(pid, pkey); | ||
| }); | ||
|
|
||
| changeConnBtn.addEventListener('click', () => { |
There was a problem hiding this comment.
CONSIDER ```js
changeConnBtn.addEventListener('click', () => {
mainPanel.classList.add('hidden');
Switching connections hides the main panel but leaves `currentIndex`, pending searches, old results, and `searchTimeout` intact; a previous query can still render after reconnect, and an index with the same name in the new project can be treated as already loaded. Add a `resetSessionState()` that clears/increments `requestId`, cancels the timeout, sets `currentIndex = null`, clears results/search fields/status, and call it before showing the connection form or before assigning the new client.
Contributor
There was a problem hiding this comment.
4 issues found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/moss-cli/src/moss_cli/playground/index.html">
<violation number="1" location="packages/moss-cli/src/moss_cli/playground/index.html:277">
P1: Reconnecting can falsely reuse the previous client's loaded-index state: selecting the same index after Change connection enables search without loading it into the new client. Reset `currentIndex` and invalidate pending searches when changing connections.</violation>
<violation number="2" location="packages/moss-cli/src/moss_cli/playground/index.html:283">
P0: Every connection attempt fails because `@moss-dev/moss-web` exposes a constructor, not `MossClient.create`; instantiate it with `new MossClient(pid, pkey)` so the playground can reach the index UI.</violation>
<violation number="3" location="packages/moss-cli/src/moss_cli/playground/index.html:521">
P2: Query latency is always missing with the WASM SDK because its result field is `timeTakenInMs` while the renderer still reads `timeTakenMs`; update the renderer to use the SDK field.</violation>
</file>
<file name="packages/moss-cli/src/moss_cli/commands/playground.py">
<violation number="1" location="packages/moss-cli/src/moss_cli/commands/playground.py:119">
P1: The automatic connection exposes the CLI's project key to an untrusted CDN-loaded JavaScript module, turning a CDN/package compromise into project credential theft. Keeping cloud calls behind the local server, or issuing a scoped short-lived browser token and self-hosting/integrity-pinning the client bundle, would preserve the playground flow without exposing the long-lived key.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| connectBtn.disabled = true; | ||
| try { | ||
| await loadSdk(); | ||
| client = await MossClient.create(pid, pkey); |
Contributor
There was a problem hiding this comment.
P0: Every connection attempt fails because @moss-dev/moss-web exposes a constructor, not MossClient.create; instantiate it with new MossClient(pid, pkey) so the playground can reach the index UI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/moss-cli/src/moss_cli/playground/index.html, line 283:
<comment>Every connection attempt fails because `@moss-dev/moss-web` exposes a constructor, not `MossClient.create`; instantiate it with `new MossClient(pid, pkey)` so the playground can reach the index UI.</comment>
<file context>
@@ -166,36 +230,92 @@ <h1>Moss <span>Playground</span></h1>
+ connectBtn.disabled = true;
+ try {
+ await loadSdk();
+ client = await MossClient.create(pid, pkey);
+ connectPanel.classList.add('hidden');
+ mainPanel.classList.remove('hidden');
</file context>
Suggested change
| client = await MossClient.create(pid, pkey); | |
| client = new MossClient(pid, pkey); |
|
|
||
| async function connect(pid, pkey) { | ||
| connectPanel.classList.remove('hidden'); | ||
| mainPanel.classList.add('hidden'); |
Contributor
There was a problem hiding this comment.
P1: Reconnecting can falsely reuse the previous client's loaded-index state: selecting the same index after Change connection enables search without loading it into the new client. Reset currentIndex and invalidate pending searches when changing connections.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/moss-cli/src/moss_cli/playground/index.html, line 277:
<comment>Reconnecting can falsely reuse the previous client's loaded-index state: selecting the same index after Change connection enables search without loading it into the new client. Reset `currentIndex` and invalidate pending searches when changing connections.</comment>
<file context>
@@ -166,36 +230,92 @@ <h1>Moss <span>Playground</span></h1>
+
+async function connect(pid, pkey) {
+ connectPanel.classList.remove('hidden');
+ mainPanel.classList.add('hidden');
+ showStatus(connectStatus, '',
+ 'Loading the Moss WASM engine (first load downloads the embedding model)...');
</file context>
Suggested change
| mainPanel.classList.add('hidden'); | |
| ++requestId; | |
| if (searchTimeout) clearTimeout(searchTimeout); | |
| currentIndex = null; | |
| mainPanel.classList.add('hidden'); |
| 200, | ||
| { | ||
| "projectId": self._project_id, | ||
| "projectKey": self._project_key, |
Contributor
There was a problem hiding this comment.
P1: The automatic connection exposes the CLI's project key to an untrusted CDN-loaded JavaScript module, turning a CDN/package compromise into project credential theft. Keeping cloud calls behind the local server, or issuing a scoped short-lived browser token and self-hosting/integrity-pinning the client bundle, would preserve the playground flow without exposing the long-lived key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/moss-cli/src/moss_cli/commands/playground.py, line 119:
<comment>The automatic connection exposes the CLI's project key to an untrusted CDN-loaded JavaScript module, turning a CDN/package compromise into project credential theft. Keeping cloud calls behind the local server, or issuing a scoped short-lived browser token and self-hosting/integrity-pinning the client bundle, would preserve the playground flow without exposing the long-lived key.</comment>
<file context>
@@ -205,42 +111,14 @@ def _serve_index(self) -> None:
+ 200,
+ {
+ "projectId": self._project_id,
+ "projectKey": self._project_key,
+ },
+ )
</file context>
| if (placeholder) placeholder.innerHTML = '<p>Searching...</p>'; | ||
|
|
||
| try { | ||
| const searchResults = await client.query(idx, query, { topK, alpha }); |
Contributor
There was a problem hiding this comment.
P2: Query latency is always missing with the WASM SDK because its result field is timeTakenInMs while the renderer still reads timeTakenMs; update the renderer to use the SDK field.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/moss-cli/src/moss_cli/playground/index.html, line 521:
<comment>Query latency is always missing with the WASM SDK because its result field is `timeTakenInMs` while the renderer still reads `timeTakenMs`; update the renderer to use the SDK field.</comment>
<file context>
@@ -407,34 +512,22 @@ <h1>Moss <span>Playground</span></h1>
- }
- const searchResults = await res.json();
- if (thisRequestId !== requestId || thisAbort.signal.aborted) return;
+ const searchResults = await client.query(idx, query, { topK, alpha });
+ if (thisRequestId !== requestId) return;
renderResults(searchResults);
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist
Please ensure that your PR meets the following requirements:
Description
Adds a local playground for interactively exploring Moss indexes from the browser.
The new
moss playgroundcommand starts a lightweight local HTTP server that serves a browser-based playground. The UI uses@moss-dev/moss-web(WASM) to execute searches entirely in the browser after an index is loaded.Usage
The server starts on
127.0.0.1:8765(or the next available port if occupied).What's Included
Added
playground.pycommand to start a local HTTP server.Added a browser-based playground UI (
playground/index.html).Added API endpoints:
GET /api/indexes– lists available cloud indexes.GET /api/index?name=...– returns index metadata.Supports automatic credential resolution from CLI flags, environment variables, or configuration.
Loads
@moss-dev/moss-webvia WASM using animportmapwith the unpkg CDN.Allows users to:
Falls back to a manual connection form when credentials are not injected by the server.
Fixes #434
Type of Change
Summary by CodeRabbit
New Features
moss playgroundcommand with configurable ports, credential profiles, and optional automatic browser launch.Documentation
Tests