Interview Prep Application of Moss - #391
Open
samanyugoyal2010 wants to merge 17 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds two new developer-facing applications to the Moss repo: (1) a local-first, voice-based interview coach grounded by Moss retrieval, and (2) a VS Code extension that provides semantic code search over the active workspace (plus packaging/CI and a Remotion promo project).
Changes:
- Introduces
apps/moss-interview-coach/: FastAPI + Pipecat SmallWebRTC voice pipeline (Whisper/Ollama/Piper) with Moss-backed rubric retrieval and subprocess-based grading, plus a Next.js assist UI. - Introduces
apps/moss-vscode/: a VS Code extension for local semantic code search with persisted indexes and optional cloud sync, including publishing docs and CI workflow. - Updates top-level docs and repo tooling files (README, .gitignore, VS Code launch/tasks) to support the new apps.
Reviewed changes
Copilot reviewed 92 out of 131 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents the new interview coach and updates the apps tree (needs to reflect VS Code extension too). |
| apps/moss-vscode/tsconfig.json | TypeScript compiler settings for the VS Code extension. |
| apps/moss-vscode/src/extension.ts | VS Code activation wiring, indexing workflow, persistence, and cloud sync commands. |
| apps/moss-vscode/src/ui/sidebar.ts | Webview UI provider for search/settings and message bridge. |
| apps/moss-vscode/src/worker/mossWorker.ts | Worker process that hosts Moss runtime operations off the extension host. |
| apps/moss-vscode/src/search/search.ts | Query mapping + small helper utilities for semantic search results. |
| apps/moss-vscode/src/moss/persistence.ts | Local disk cache + meta.json persistence helpers for indexes. |
| apps/moss-vscode/src/moss/config.ts | Credential resolution (settings/secrets/env) + search/index settings helpers. |
| apps/moss-vscode/src/moss/client.ts | Worker-backed session manager and Node binary resolution logic. |
| apps/moss-vscode/src/indexer/scanner.ts | Workspace scanning and safe file loading for indexing. |
| apps/moss-vscode/src/indexer/indexer.ts | Chunking + indexing orchestration and incremental update watchers. |
| apps/moss-vscode/src/indexer/excludes.ts | Hard + configurable exclusion logic for indexing paths. |
| apps/moss-vscode/src/indexer/chunker.ts | Line-based chunking and metadata for file navigation. |
| apps/moss-vscode/scripts/verify-package.mjs | Verifies VSIX contains expected artifacts and moss-core bundle. |
| apps/moss-vscode/scripts/prepackage.mjs | Installs cross-platform native moss-core binaries before packaging. |
| apps/moss-vscode/README.md | Extension usage docs (setup, commands, settings, architecture). |
| apps/moss-vscode/PUBLISHING.md | VS Code Marketplace publishing steps and CI notes. |
| apps/moss-vscode/PRIVACY.md | Privacy policy for local indexing + optional cloud sync. |
| apps/moss-vscode/package.json | VS Code extension manifest (commands, settings, engines, deps). |
| apps/moss-vscode/esbuild.mjs | Esbuild bundling config for extension + worker. |
| apps/moss-vscode/media/sidebar.css | Styling for the extension’s webview sidebar UI. |
| apps/moss-vscode/media/icon.svg | Activity bar icon for the extension. |
| apps/moss-vscode/LICENSE | License file for the extension package. |
| apps/moss-vscode/CHANGELOG.md | Initial extension changelog entry. |
| apps/moss-vscode/.vscodeignore | Packaging ignore rules for VSIX output. |
| apps/moss-vscode/.vscode/tasks.json | Local dev tasks for building/watching the extension. |
| apps/moss-vscode/.vscode/launch.json | Local debug configuration to run the extension host. |
| apps/moss-vscode/.gitignore | App-local ignores for build artifacts and env files. |
| apps/moss-vscode/.env.example | Example Moss credentials env file for the extension. |
| apps/moss-vscode/promo/package.json | Remotion promo project dependencies and scripts. |
| apps/moss-vscode/promo/tsconfig.json | TS config for promo rendering code. |
| apps/moss-vscode/promo/eslint.config.mjs | ESLint config for the promo project. |
| apps/moss-vscode/promo/.prettierrc | Prettier config for the promo project. |
| apps/moss-vscode/promo/.gitignore | Promo-local ignores (node_modules, out/ render output, etc.). |
| apps/moss-vscode/promo/remotion.config.ts | Remotion config (image format, Tailwind v4 integration). |
| apps/moss-vscode/promo/README.md | Promo storyboard and render commands. |
| apps/moss-vscode/promo/src/index.ts | Remotion root registration entrypoint. |
| apps/moss-vscode/promo/src/Root.tsx | Declares the Remotion composition for the promo. |
| apps/moss-vscode/promo/src/index.css | Tailwind import for Remotion UI styling. |
| apps/moss-vscode/promo/src/compositions/MossVscodePromo.tsx | Main promo composition, scenes, and audio/SFX timeline. |
| apps/moss-vscode/promo/src/scenes/HeroScene.tsx | Promo hero scene. |
| apps/moss-vscode/promo/src/scenes/GrepFailureScene.tsx | Promo “grep noise” scene. |
| apps/moss-vscode/promo/src/scenes/HowItWorksScene.tsx | Promo pipeline explanation scene. |
| apps/moss-vscode/promo/src/scenes/ProductDemoScene.tsx | Promo product demo scene (indexing + semantic query + jump). |
| apps/moss-vscode/promo/src/scenes/EditorSuperpowersScene.tsx | Promo features montage scene. |
| apps/moss-vscode/promo/src/scenes/OutroScene.tsx | Promo outro scene. |
| apps/moss-vscode/promo/src/components/VfxLayer.tsx | Vignette/grain/flash VFX overlay components. |
| apps/moss-vscode/promo/src/components/SceneBridge.tsx | Scene enter/exit transitions (opacity/scale/blur). |
| apps/moss-vscode/promo/src/components/SequoiaBackdrop.tsx | Backdrop image + scrim component. |
| apps/moss-vscode/promo/src/components/PipelineBeat.tsx | Reusable beat + scan/chunk/embed/query visuals. |
| apps/moss-vscode/promo/src/components/DropText.tsx | Headline animation component. |
| apps/moss-vscode/promo/src/components/SuperpowerCard.tsx | Feature card component. |
| apps/moss-vscode/promo/src/components/MossSidebarPanel.tsx | Promo mock Moss sidebar panel component. |
| apps/moss-vscode/promo/src/components/MossWordmark.tsx | Wordmark/symbol/avatar image helpers. |
| apps/moss-vscode/promo/src/components/CrashTogetherLogos.tsx | VS Code + Moss logo collision/morph animation. |
| apps/moss-vscode/promo/src/components/CrashToMossOutro.tsx | Outro lockup animation composition. |
| apps/moss-vscode/promo/src/components/vscode/VSCodeWindow.tsx | Promo mock VS Code window layout component. |
| apps/moss-vscode/promo/src/components/vscode/TitleBar.tsx | Promo mock title bar. |
| apps/moss-vscode/promo/src/components/vscode/ActivityBar.tsx | Promo mock activity bar. |
| apps/moss-vscode/promo/src/components/vscode/TabBar.tsx | Promo mock tab bar. |
| apps/moss-vscode/promo/src/components/vscode/StatusBar.tsx | Promo mock status bar. |
| apps/moss-vscode/promo/src/components/vscode/CodeEditorPane.tsx | Promo mock editor pane with simple token coloring. |
| apps/moss-vscode/promo/src/components/vscode/FindInFilesPanel.tsx | Promo mock “Find in Files” sidebar panel. |
| apps/moss-vscode/promo/src/lib/colors.ts | Promo color tokens (VS Code + brand palette). |
| apps/moss-vscode/promo/src/lib/demo.ts | Demo script data (queries, hits, code lines). |
| apps/moss-vscode/promo/src/lib/easing.ts | Easing curves for animation. |
| apps/moss-vscode/promo/src/lib/music.ts | Music bed section volume automation. |
| apps/moss-vscode/promo/src/lib/sfx.ts | SFX timing constants and event frames. |
| apps/moss-vscode/promo/src/lib/timing.ts | Scene timing constants and cut frames. |
| apps/moss-vscode/promo/src/lib/typing.ts | Variable-speed “typing” frame simulation helper. |
| apps/moss-vscode/promo/src/lib/typography.ts | Typography size tokens. |
| apps/moss-vscode/promo/public/branding/vscode/icon.svg | Promo VS Code icon asset. |
| apps/moss-vscode/promo/public/branding/moss/icon.svg | Promo Moss icon asset. |
| apps/moss-interview-coach/README.md | End-to-end setup + architecture notes for interview coach. |
| apps/moss-interview-coach/.gitignore | Ignores local model downloads and env/build outputs. |
| apps/moss-interview-coach/backend/requirements.txt | Backend dependencies (FastAPI, Moss, Pipecat, etc.). |
| apps/moss-interview-coach/backend/.env.example | Backend env template (Moss creds + local model settings). |
| apps/moss-interview-coach/backend/tracks.py | Track definitions (prompts, index names, knowledge files). |
| apps/moss-interview-coach/backend/ingest_knowledge.py | Ingest script to create/load/query per-track Moss indexes. |
| apps/moss-interview-coach/backend/server.py | FastAPI server exposing SmallWebRTC offer/patch and running the Pipecat pipeline with Moss injection + grading. |
| apps/moss-interview-coach/backend/grader_worker.py | Subprocess grader that calls Ollama and returns structured JSON grade. |
| apps/moss-interview-coach/backend/knowledge/system_design_rubrics.json | System design rubric knowledge source. |
| apps/moss-interview-coach/backend/knowledge/agent_native_rubrics.json | Agent-infra rubric knowledge source. |
| apps/moss-interview-coach/backend/knowledge/ml_concepts_rubrics.json | ML concepts rubric knowledge source. |
| apps/moss-interview-coach/frontend/package.json | Next.js frontend dependencies/scripts for assist UI. |
| apps/moss-interview-coach/frontend/tsconfig.json | TypeScript config for Next.js frontend. |
| apps/moss-interview-coach/frontend/next.config.ts | Next.js configuration. |
| apps/moss-interview-coach/frontend/next-env.d.ts | Next.js TS environment declarations. |
| apps/moss-interview-coach/frontend/postcss.config.mjs | Tailwind v4 PostCSS plugin configuration. |
| apps/moss-interview-coach/frontend/.env.example | Frontend env template (backend URL). |
| apps/moss-interview-coach/frontend/app/layout.tsx | Next.js app layout and metadata. |
| apps/moss-interview-coach/frontend/app/globals.css | Tailwind + global styling for the interview coach UI. |
| .github/workflows/moss-vscode-ci.yml | CI workflow to build/typecheck/package/verify the VS Code extension. |
| .gitignore | Adds ignores for local agent skill artifacts. |
| .vscode/tasks.json | Root tasks to build/watch the VS Code extension. |
| .vscode/launch.json | Root launch config to run the VS Code extension host. |
Files not reviewed (1)
- apps/moss-interview-coach/frontend/package-lock.json: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
@cubic-dev-ai review this pull request |
Contributor
@HarshaNalluru I have started the AI code review. It will take a few minutes to complete. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 27 files
Tip: instead of fixing issues one by one fix them all with cubic
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 17 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
HarshaNalluru
added a commit
that referenced
this pull request
Jul 23, 2026
## What Makes the Codex PR reviewer run on **all pull requests, including forks/contributors** — today fork PRs are silently skipped. ## Why forks were skipped A fork's `pull_request` run gets a read-only token and **no secrets** (GitHub's fork-safety default), so the reviewer can't get `OPENAI_API_KEY`. The old workflow acknowledged this by gating itself to same-repo branches (`sameRepo` check), so every contributor PR got `Skipping Codex review: … is from a fork`. You can't fix this by dropping the gate — the secret still isn't there on fork runs. You need a trigger that *has* secrets, without ever running fork code next to them. ## How — two-stage split | Stage | File | Trigger | Secrets? | Touches PR code? | |---|---|---|---|---| | 1. Collect diff | `codex-review.yml` | `pull_request` (all PRs) | ❌ none | checkout + `git diff` only (no build/install) | | 2. Review + post | `codex-review-post.yml` | `workflow_run` (base context) | ✅ | ❌ — reads the diff artifact only | Stage 1 runs on every PR with no secrets, computes the diff, and uploads it as an artifact. Stage 2 is triggered by Stage 1's completion, runs in the **base-repo** context (so it has secrets even for forks), downloads the diff, runs Codex over the **diff text only**, and posts the inline review. **Fork code never executes in a job that holds the key** — that's the structural guarantee that makes reviewing fork PRs safe, and it's more robust than `pull_request_target` (which would put fork code on disk beside the secret and rely on "no one ever adds a build step"). ## Security notes - **Anti-spoofing:** the artifact carries the PR number; Stage 2 requires `pr.head.sha === workflow_run.head_sha` (a value GitHub sets, not the fork) before posting, so a tampered artifact can't redirect the review onto another PR. PR number is validated as digits-only. - **Least privilege:** Stage 1 is `contents: read`. Stage 2 is `contents: read`, `actions: read` (to fetch the artifact), `pull-requests/issues: write` (to post). - **Residual risk — please use a scoped key.** Codex still reads the untrusted diff as *data*, and per the action's docs the API key "flows through the proxy, so Codex could read it if it can reach process memory." A prompt-injection payload in a diff could in principle try to coax the key into a review comment. The two-stage split does **not** remove this (it removes code *execution*), so use a low-limit / restricted `OPENAI_API_KEY` for this workflow. ## Behavior changes - Auto-reviews **all non-draft, non-bot PRs** (previously same-repo only). - **Drops** the `@codex review` comment trigger and the trusted-commenter gate (no longer needed to keep forks out). Easy to re-add as an opt-in gate if you'd prefer maintainer-triggered reviews on forks to control cost. ## Activation caveat `workflow_run` only fires for the copy of these workflows **on the default branch**, so Codex review activates once this is merged to `main` — **this PR will not review itself.** After merge, opening any fork PR (or re-syncing an existing one like #391/#393) will exercise it end-to-end. ## Not tested in CI here The `openai/codex-action` step reviews a pre-computed `pr.diff` in an empty `git init`'d working dir (rather than a full checkout). YAML + embedded JS are validated locally; the first live run after merge is the real end-to-end test. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/usemoss/moss/pull/437?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
Hendrixx-RE
pushed a commit
to Hendrixx-RE/moss
that referenced
this pull request
Jul 28, 2026
…oss#437) ## What Makes the Codex PR reviewer run on **all pull requests, including forks/contributors** — today fork PRs are silently skipped. ## Why forks were skipped A fork's `pull_request` run gets a read-only token and **no secrets** (GitHub's fork-safety default), so the reviewer can't get `OPENAI_API_KEY`. The old workflow acknowledged this by gating itself to same-repo branches (`sameRepo` check), so every contributor PR got `Skipping Codex review: … is from a fork`. You can't fix this by dropping the gate — the secret still isn't there on fork runs. You need a trigger that *has* secrets, without ever running fork code next to them. ## How — two-stage split | Stage | File | Trigger | Secrets? | Touches PR code? | |---|---|---|---|---| | 1. Collect diff | `codex-review.yml` | `pull_request` (all PRs) | ❌ none | checkout + `git diff` only (no build/install) | | 2. Review + post | `codex-review-post.yml` | `workflow_run` (base context) | ✅ | ❌ — reads the diff artifact only | Stage 1 runs on every PR with no secrets, computes the diff, and uploads it as an artifact. Stage 2 is triggered by Stage 1's completion, runs in the **base-repo** context (so it has secrets even for forks), downloads the diff, runs Codex over the **diff text only**, and posts the inline review. **Fork code never executes in a job that holds the key** — that's the structural guarantee that makes reviewing fork PRs safe, and it's more robust than `pull_request_target` (which would put fork code on disk beside the secret and rely on "no one ever adds a build step"). ## Security notes - **Anti-spoofing:** the artifact carries the PR number; Stage 2 requires `pr.head.sha === workflow_run.head_sha` (a value GitHub sets, not the fork) before posting, so a tampered artifact can't redirect the review onto another PR. PR number is validated as digits-only. - **Least privilege:** Stage 1 is `contents: read`. Stage 2 is `contents: read`, `actions: read` (to fetch the artifact), `pull-requests/issues: write` (to post). - **Residual risk — please use a scoped key.** Codex still reads the untrusted diff as *data*, and per the action's docs the API key "flows through the proxy, so Codex could read it if it can reach process memory." A prompt-injection payload in a diff could in principle try to coax the key into a review comment. The two-stage split does **not** remove this (it removes code *execution*), so use a low-limit / restricted `OPENAI_API_KEY` for this workflow. ## Behavior changes - Auto-reviews **all non-draft, non-bot PRs** (previously same-repo only). - **Drops** the `@codex review` comment trigger and the trusted-commenter gate (no longer needed to keep forks out). Easy to re-add as an opt-in gate if you'd prefer maintainer-triggered reviews on forks to control cost. ## Activation caveat `workflow_run` only fires for the copy of these workflows **on the default branch**, so Codex review activates once this is merged to `main` — **this PR will not review itself.** After merge, opening any fork PR (or re-syncing an existing one like usemoss#391/usemoss#393) will exercise it end-to-end. ## Not tested in CI here The `openai/codex-action` step reviews a pre-computed `pr.diff` in an empty `git init`'d working dir (rather than a full checkout). YAML + embedded JS are validated locally; the first live run after merge is the real end-to-end test. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/usemoss/moss/pull/437?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
Codex reviewThe PR is mostly careful about lifecycle and cleanup, but the VS Code cache state still has a correctness hole around rebuild failures. The durable cache can remain restorable after the previous index has already been mutated. |
samanyugoyal2010
added a commit
to samanyugoyal2010/moss
that referenced
this pull request
Jul 31, 2026
…meout Three findings from review on usemoss#391: - Split bot-speech tracking out of InterruptionBridge into a new BotSpeechTracker placed downstream of transport.output(), which is what emits Bot{Started,Stopped}SpeakingFrame. Note the bridge itself must stay *upstream* of the output transport: the RTVIServerMessageFrame it pushes only reaches the client by flowing into transport.output(), so moving the whole processor downstream would have silenced barge-in messages. Splitting keeps both correct. (For the record, bot_speaking was not actually stuck false today — pipecat 1.6.0 BaseOutputTransport broadcasts these frames upstream as well as downstream, so the bridge did observe them. But that is an implementation detail; reading them downstream of the emitter is the right dependency.) - _grade_in_subprocess only killed the worker on TimeoutError. on_client_disconnected cancels every in-flight grade, and that CancelledError propagates straight out of communicate(), leaving grader_worker.py and its Ollama request running past the session. Added _terminate_grader (kill + shielded reap) on the cancel, timeout and generic-BaseException paths. Verified against the old code: the child survives cancellation before the fix (returncode=None) and is reaped after it (returncode=-9). - The 30s connect timeout only aborted the health-check fetch; the signal was never applied to initDevices() or connect(), so a stalled WebRTC negotiation left the UI on "connecting" forever. Added withAbort() to race them against the existing AbortController. Verified a hanging promise now rejects when the timeout fires, the message still contains "timed out" so the existing user-facing copy applies, and a late rejection from the abandoned promise does not surface as an unhandled rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
samanyugoyal2010
added a commit
to samanyugoyal2010/moss
that referenced
this pull request
Jul 31, 2026
…t paths Four findings from review on usemoss#391: - uvicorn defaulted to 0.0.0.0, exposing the unauthenticated /api/offer on every interface. CORS does not stop non-browser callers, and each call starts Whisper/Ollama/Piper work plus grader subprocesses, so anyone on the network could burn the host's CPU/GPU and the project's Moss quota. Default is now 127.0.0.1, with wider binding explicitly opt-in. Also updated .env.example and the README, which both shipped BACKEND_HOST=0.0.0.0 — dotenv would have loaded that over the new default and undone the fix for anyone following the setup steps. - When Moss returned zero docs, the injector returned while leaving the previous last_rubric_id / last_rubric_text cached and the previous rubric still in the system prompt, so the turn and the grader could score against a topic no longer being asked about. Now mirrors the query-failure branch exactly. - The post-await `if (abort.signal.aborted) return;` paths skipped the catch branch that tears down the client and resets the UI, stranding the page on "connecting" if the timeout landed in that window. They now throw abortReason(), routing through the existing cleanup. Verified the old shape ends on "connecting" while the new one reaches "idle" with the timeout message (DOMException passes the `instanceof Error` check the handler makes). - onServerMessage was the only Pipecat callback not scoped to the current client, so a late data-channel message from a disconnected session could mutate a newer interview. Added the same guard the other callbacks use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
samanyugoyal2010
added a commit
to samanyugoyal2010/moss
that referenced
this pull request
Jul 31, 2026
Two findings from review on usemoss#391: - The welcome messages were queued from on_client_connected after a fixed asyncio.sleep(0.6). A transport-level WebRTC connection does not mean the RTVI client is listening, so on a slow connect the opening RTVIServerMessageFrames and TTSSpeakFrame could be sent before the client was ready and the first question/audio missed. Moved to worker.rtvi.event_handler("on_client_ready") and dropped the sleep. Verified against pipecat 1.6.0 before relying on it: PipelineWorker takes enable_rtvi=True by default so worker.rtvi does not raise, and add_event_handler appends to a list, so this coexists with pipecat's own on_client_ready handler rather than replacing set_bot_ready(). Handlers for async events are dispatched as separate tasks, so no ordering is assumed between the two. Added a `greeted` latch so a client that re-sends ready cannot replay the welcome over a running interview. - /api/offer and its PATCH wrapped everything in `except Exception` and re-raised as 500, which flattened HTTPExceptions and reported caller mistakes as server faults. Both now let HTTPException through, map KeyError/TypeError/ValueError to 422, and keep 500 for genuine faults. Body parsing moved into a helper so malformed or non-object JSON is a 400 — previously it sat outside the try and surfaced as a 500. pc_id and candidates are validated explicitly. Covered by 15 TestClient cases: valid, malformed JSON, non-object body, missing/mistyped pc_id, non-list and malformed candidates, HTTPException passthrough (409/404 preserved), and internal errors still 500. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
|
Adds two Moss-powered developer apps: - apps/moss-interview-coach/ — local-first voice interview coach (Pipecat SmallWebRTC + Whisper/Ollama/Piper) with Moss rubric retrieval, multi-track topic selection, Assist feedback panel, and subprocess-isolated grading. - apps/moss-vscode/ — VS Code extension for local semantic code search over the active workspace (worker-backed Moss runtime, persisted indexes, optional cloud sync), plus packaging/CI and a Remotion promo. Includes follow-up hardening: grade-task cancellation on barge-in, RTVI-ready greeting, connect cleanup scoping, loopback binding, stale rubric clearing, and offer/patch error classification.
cursor
Bot
force-pushed
the
interview-prep
branch
from
July 31, 2026 09:43
7189dbf to
cba76e4
Compare
…el docs - Add track blurbs to backend tracks.py and expose them via /api/tracks - Remove hardcoded INTERVIEW_TRACKS from the frontend; load tracks from API - Clarify README: OLLAMA_GRADE_MODEL is unset by default and follows OLLAMA_MODEL
Three findings from review on usemoss#391: - Tracks now come only from the backend, so a failed one-shot fetch left the picker permanently empty with no recovery short of a page reload. Added a loading/error state with a short retry backoff (1s/2s/4s) to ride out the frontend starting first, then an explicit error panel with a Retry button. The empty <ul> no longer renders, so the states do not stack. - CodebaseIndexer marked a cancelled full-workspace index as "ready" once the flushed prefix was stored. isReady() gates search on that, and the watchers only fire on *changes*, so files never scanned would stay missing and searches would silently return partial results. Cancellation now deletes the chunks that were actually flushed, clears the counts, leaves watching off and the status "unindexed". Replayed a 5-file scan cancelled after 2: previously ready with 1 file and isReady() true; now unindexed with the 3 flushed chunks removed and the still-pending ones (never added) left alone. - README claimed Node.js 20+, but @pipecat-ai/small-webrtc-transport pulls @daily-co/daily-js@0.90.0, which declares node >=22.14.0 — the only hard floor in the lockfile (everything else still allows ^20.x). Raised the documented prerequisite and added an engines field to the frontend package so it is machine-checked rather than only written down. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three findings from review on usemoss#391, plus one same-class gap found while checking: - endInterview cleared clientRef but never touched connectAbortRef, so ownsSession() still reported the in-flight startInterview as the owner. Since onConnected/onBotReady flip the UI to "active" before connect() resolves, End could be pressed mid-connect and the resumed call would then run setSession("active") and attach audio after the user had ended the session. endInterview now aborts and clears the controller, and the success path additionally requires clientRef.current === client. Replayed the interleaving: previously ended {session:"active", audioAttached:true}; now {session:"idle", audioAttached:false}. Same class, unflagged: component unmount disconnected the client but did not abort an in-flight connect, so it ran to completion against an already disconnected client. The unmount cleanup now aborts first. - /health only proved the Ollama daemon answered. An unpulled OLLAMA_MODEL or OLLAMA_GRADE_MODEL still reported ready, so the failure surfaced later inside the background pipeline once WebRTC was already negotiated. It now parses /api/tags and reports the missing model(s) with the pull command, tolerating the usual `:latest` omission. Covered by 7 cases including untagged config, explicit tags, a wrong explicit tag, and an empty daemon. - _wait_until_coach_quiet treated the pre-speech gap as silence. Grading is launched right after the tool ack, before the follow-up has been generated, so bot_speaking was still False and the wait returned in ~450ms — putting the grader's Ollama request in competition with the coach's own response. Added a bot_speech_turns counter so a waiter can tell "not started yet" from "already finished", and the wait now requires speech to start (bounded by start_timeout_secs, since the coach may not speak) before waiting it out. Measured with a follow-up starting at 0.8s and lasting 1.5s: previously returned at 0.36s mid-speech, now 2.78s after it finished; with no speech at all it still returns, at 4.36s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 9 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- ingest_knowledge: --track is repeatable but --source writes to exactly one index, so taking args.tracks[0] let argument order silently decide the destination. That combination is now an argparse error. The --source help also claimed it "overrides --track" while --index-name documented the default as the track index; rewritten so the precedence (--index-name, else the single --track, else the default track) is stated once. Multiple --track without --source is unaffected. - moss-vscode: clearIndexCache() only ran in the else-branch, so a rebuild() that threw after deleting the previous documents — from readFileForIndex() or addDocs() — jumped to catch and left the on-disk cache describing documents that no longer exist. Moved into a finally keyed on indexer.isIndexed(), which the throw path cannot skip. Verified: argparse across 6 invocations (multi-track rejected with exit 2, single track honoured, --index-name precedence intact, multi-track still fine without --source); and the try/catch/finally shape across cancelled, thrown and successful runs — the throw path now clears, the success path still does not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- page.tsx: /api/tracks returns per-track `ready` and a top-level `default`, but mapApiTracks dropped both — so nothing was preselected and a track whose Moss index is not loaded stayed selectable, failing later at /health. Both fields are carried through now: selection keeps a still-valid choice, else adopts the backend default, else the first ready track; unready tracks are disabled and labelled. A track without `ready` is treated as usable so an older backend does not grey out everything. - moss-vscode: the finally added in e733c8f cleared the persisted cache on any non-ready outcome, including a failure *before* rebuild() deleted anything (session setup, workspace scan) — where the previous documents are still intact and dropping the cache just forces a needless full re-index. The indexer now exposes hasDiscardedPreviousIndex(), set when deletion begins and reset on a ready finish, and the caller gates on it. - tracks.py: "optimising" -> "optimizing", matching US spelling elsewhere. Verified: track selection over 7 shapes (default unready, none ready, current gone unready, legacy backend without the field); and cache invalidation across fail-before-deletion, fail-after-deletion, cancelled and successful runs — only the first is now preserved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- moss-vscode: discardedPreviousIndex was set *before* deleteInBatches, so a delete that threw partway still told the caller to drop the persisted cache. Some old documents would survive while the cache — the only record of their ids — was gone, leaving them answering searches for deleted or renamed files with no way for a later rebuild to find them. The flag now flips only after the stale delete has fully succeeded, so a partial delete keeps the cache and the next rebuild retries the cleanup. - server.py: /api/offer accepted an SDP and started the pipeline immediately, with nothing to end a session whose client never completed the WebRTC/RTVI handshake — Whisper, Piper and Ollama stayed loaded until the transport noticed or the process exited. Added a watchdog that shuts the session down if on_client_ready has not fired within SESSION_HANDSHAKE_TIMEOUT_SECS (default 45s, generous against the frontend's own 30s connect timeout so a slow but genuine client is never cut off), cancelled once the session ends. Also bounded concurrency with MAX_ACTIVE_BOTS (default 2), checked before spawning so excess offers get a 503 rather than degrading live sessions. Both documented in the README table and .env.example. Verified: the delete-flag placement across partial-delete, delete-then-throw and success — only flag-after avoids stranding documents; and the watchdog for a client that never connects vs ready at 0.1s and 0.35s, plus the capacity gate at the boundary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/moss-vscode/src/indexer/indexer.ts (1)
213-231: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftSerialize watcher writes with rebuild cleanup.
upsertFile()andremoveFile()checkthis.indexingonly before their firstawait. A watcher operation that started beforerebuild()can finish after the cancellation block clearspathChunkCountsand setsunindexed. It can then calladdDocs(), updatepathChunkCounts, and callrefreshReadyStatus()orrequestPersist().A cancelled rebuild can become ready again or leave remote documents outside
partialIds. Stop and await in-flight watcher operations before destructive cleanup. A generation check must also remove late remote writes.🤖 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 `@apps/moss-vscode/src/indexer/indexer.ts` around lines 213 - 231, Serialize watcher operations from upsertFile and removeFile with rebuild cleanup by tracking and awaiting all in-flight operations before deleting partialIds and clearing pathChunkCounts. Add a rebuild generation/token check after each await and before addDocs, metadata updates, refreshReadyStatus, or requestPersist; stale watcher operations must not write or update readiness. Ensure any late remote writes from an obsolete generation are removed or otherwise discarded before the cancelled rebuild returns unindexed.
🧹 Nitpick comments (1)
apps/moss-interview-coach/backend/.env.example (1)
26-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOrder the dotenv keys to satisfy the linter.
Move
MAX_ACTIVE_BOTSand its comment beforeSESSION_HANDSHAKE_TIMEOUT_SECS.dotenv-lintercurrently reports this ordering warning.🤖 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 `@apps/moss-interview-coach/backend/.env.example` around lines 26 - 28, Reorder the dotenv entries so the MAX_ACTIVE_BOTS setting and its associated comment appear before SESSION_HANDSHAKE_TIMEOUT_SECS, without changing either value or comment.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In `@apps/moss-vscode/src/indexer/indexer.ts`:
- Around line 213-231: Serialize watcher operations from upsertFile and
removeFile with rebuild cleanup by tracking and awaiting all in-flight
operations before deleting partialIds and clearing pathChunkCounts. Add a
rebuild generation/token check after each await and before addDocs, metadata
updates, refreshReadyStatus, or requestPersist; stale watcher operations must
not write or update readiness. Ensure any late remote writes from an obsolete
generation are removed or otherwise discarded before the cancelled rebuild
returns unindexed.
---
Nitpick comments:
In `@apps/moss-interview-coach/backend/.env.example`:
- Around line 26-28: Reorder the dotenv entries so the MAX_ACTIVE_BOTS setting
and its associated comment appear before SESSION_HANDSHAKE_TIMEOUT_SECS, without
changing either value or comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dfcdd9cb-6664-44a2-a659-3d83fe6d9a62
📒 Files selected for processing (8)
apps/moss-interview-coach/README.mdapps/moss-interview-coach/backend/.env.exampleapps/moss-interview-coach/backend/ingest_knowledge.pyapps/moss-interview-coach/backend/server.pyapps/moss-interview-coach/backend/tracks.pyapps/moss-interview-coach/frontend/app/page.tsxapps/moss-vscode/src/extension.tsapps/moss-vscode/src/indexer/indexer.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/moss-vscode/src/extension.ts
- apps/moss-interview-coach/README.md
- apps/moss-interview-coach/backend/tracks.py
- apps/moss-interview-coach/backend/ingest_knowledge.py
- apps/moss-interview-coach/frontend/app/page.tsx
- apps/moss-interview-coach/backend/server.py
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- server.py: the MAX_ACTIVE_BOTS check ran in offer() while the increment happened inside the detached bot task, several awaits later, so concurrent offers all passed the guard together. Capacity is now reserved atomically under a lock before the offer is handled; ownership transfers to the bot task once it is created, and offer() releases the slot on every path where it was not. run_interview_bot wraps its whole body so even the early readiness raise releases. Measured with 10 simultaneous offers: peak active went 10 -> 2, with 8 x 503 and no leaked slots. - indexer.ts: a rebuild that threw after the stale delete left the documents it had already upserted in the index, while the caller dropped the persisted cache — the only record of their ids — stranding them in search results. Extracted discardPartialIndex(), now used by the cancel path and the catch. If that cleanup itself fails it clears discardedPreviousIndex so the cache is kept and a later rebuild can retry, rather than losing the ids entirely. - indexer.ts: the `indexing` guard only stopped watcher work from starting, so an upsert/remove already awaiting a file read could write during a rebuild's cleanup. Added a generation counter bumped per rebuild and re-checked after every await before any write, plus in-flight op tracking that rebuild drains before it mutates anything. - .env.example: MAX_ACTIVE_BOTS now precedes SESSION_HANDSHAKE_TIMEOUT_SECS. Verified: 10-way offer burst and the reserve-then-fail path; rebuild throwing mid-scan with cleanup succeeding and failing; and a watcher write racing a rebuild with and without the generation guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/moss-vscode/src/indexer/indexer.ts (1)
247-262: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep
discardedPreviousIndextrue when partial cleanup fails.
discardPartialIndex()currently resets the flag whendeleteInBatches()throws. The extension then skipsclearIndexCache(), leaving stalemeta.json. On restart,restoreFromMeta()can report a ready index for documents deleted during the rebuild. Return without resetting the flag; preservepathChunkCountsfor the retry path.🤖 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 `@apps/moss-vscode/src/indexer/indexer.ts` around lines 247 - 262, Update discardPartialIndex so a deleteInBatches failure leaves discardedPreviousIndex true and preserves pathChunkCounts for retry; ensure the catch path in the indexing flow still reaches clearIndexCache rather than skipping cleanup due to a reset flag.
🤖 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.
Outside diff comments:
In `@apps/moss-vscode/src/indexer/indexer.ts`:
- Around line 247-262: Update discardPartialIndex so a deleteInBatches failure
leaves discardedPreviousIndex true and preserves pathChunkCounts for retry;
ensure the catch path in the indexing flow still reaches clearIndexCache rather
than skipping cleanup due to a reset flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8681bcf1-6ba9-4232-899c-c4438df2be30
📒 Files selected for processing (3)
apps/moss-interview-coach/backend/.env.exampleapps/moss-interview-coach/backend/server.pyapps/moss-vscode/src/indexer/indexer.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/moss-interview-coach/backend/.env.example
- apps/moss-interview-coach/backend/server.py
…leanup - indexer.ts: the generation bump ran before drainWatcherOps(), so a watcher that had already completed addDocs() would see itself stale and return without recording those chunks in pathChunkCounts. The rebuild then computed staleIds from counts that omitted them and they survived in the index. `indexing` already blocks new watcher work, so the drain now happens first — letting in-flight operations record what they wrote — and the generation is bumped afterwards to cover anything that slips through. - indexer.ts: discardPartialIndex() reset discardedPreviousIndex when the delete failed, which made the caller skip clearIndexCache() and leave a meta.json describing the previous documents — already deleted by this rebuild. restoreFromMeta() would then report a ready index for documents that no longer exist on the next launch, which is worse than the leftovers it was protecting. The flag now stays true so the stale cache is dropped, while pathChunkCounts is kept for a retry in the same session. This reverses the choice made in bc906da. - server.py: runner.run() can end on a transport or LLM error, reaching none of the disconnect / watchdog / lifespan paths, and the finally discarded the only session handle without cancelling grade tasks — leaving a grader subprocess and its Ollama request running until timeout, then queueing into a dead worker. The finally now awaits session.shutdown() first, suppressed so teardown cannot mask the original error. Verified: a watcher writing two chunks as a rebuild starts is stranded under bump-before-drain and cleaned up under drain-then-bump; a failed partial cleanup now still clears the cache while keeping the counts; and a runner error leaves the grader alive without the shutdown call and terminated with it, with shutdown() confirmed safe to call twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- server.py: the capacity slot was reserved before the request body was read or validated, so a client trickling a body — or sending a malformed offer — occupied one of the MAX_ACTIVE_BOTS slots without ever starting a bot. The body is now parsed and SmallWebRTCRequest.from_dict() validated first (400 / 422 as before), and the slot is reserved immediately before handle_web_request(), still released on every path where the bot task was never created. Ordering in offer() is now read -> validate -> reserve -> handle -> release. - indexer.ts: discardPartialIndex()'s JSDoc still described the contract from bc906da, which b3d9cd2 reversed — it claimed a failed delete clears discardedPreviousIndex to preserve the cache, while the code deliberately keeps the flag true so the stale cache is dropped and retains pathChunkCounts for an in-session retry. Doc now matches the code. Verified: two slow malformed offers arriving before a genuine one produced 503 under reserve-first and 200 under parse-first; the 10-way concurrent burst still caps at 2 with no leaked slots; ordering asserted structurally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…arguments grade_candidate_answer took `answer` and `question` from the coach LLM, falling back to the server-captured values only when the tool arguments were empty. The model produces those arguments after reading the candidate's own transcript, so they sit downstream of untrusted input: a candidate can talk the coach into calling the tool with a forged answer and have that graded instead of what they said. The same ordering also let an ordinary paraphrase replace the real turn, so grading ran against a reconstruction rather than the transcript. moss.last_user_answer (set from the transcribed turn in _inject_rubric) and assist.last_question (what the coach was recorded as asking) are now the source of truth, with the tool arguments used only when nothing was captured. A material mismatch is logged so the substitution is visible. Verified across five shapes: a forged answer and question are both replaced by the captured ones, a paraphrase likewise, the fallback still applies when nothing was captured, an empty pair is still rejected, and the pre-fix ordering demonstrably graded the forged text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
… failures - server.py: grade_candidate_answer sets cancel_on_interruption=False, so a call issued for one turn can execute after the candidate has spoken again. Preferring moss.last_user_answer (65ae934) therefore risked grading the *next* answer against the earlier turn's question. Added user_turn_seq, advanced whenever a user turn is captured, and response_turn_seq, snapshotted when the LLM response carrying the call begins. The captured transcript and question are used only while those match; otherwise the supplied arguments — produced from the correct turn — are kept. Replayed four orderings: an interrupted turn graded the following answer before the fix and the right one after, while the prompt-injection and fallback cases are unchanged. - persistence.ts / extension.ts: clearIndexCache swallowed its own rm failure and returned void, so the caller's .catch was dead and it logged a success that had not happened — leaving metadata on disk that the next activation would restore for documents the rebuild had deleted. It now propagates, and the rebuild path logs the real outcome, clears the workspace-indexed marker as a second line of defence, and warns the user. The restore path stays best-effort (its marker is already cleared) but no longer discards the reason. Not changed: the request to make partial-cleanup failure atomic via a tombstone or index drop. Partial documents cannot reach disk — persistIndex is the only saveToDisk caller and returns early unless isIndexed() — so they cannot survive a restart, and search gates on canSearch() which is false in that state. See the thread reply for the trace. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ared state The user_turn_seq / response_turn_seq guard from 98b31fc only held until the *next* response started: that overwrote response_turn_seq, the equality became true again, and a delayed call from the earlier turn was rebound to the newer transcript. Comparing two mutable globals could never express "this call's turn" — it only described the pipeline's current position. Replaced with an immutable per-invocation snapshot. pipecat's on_function_calls_started fires when the LLM announces its calls, before the runner executes them and while the originating turn is still current, and each FunctionCallFromLLM carries a unique tool_call_id. The handler freezes the answer, question, rubric id and rubric text under that id; the tool consumes its own entry via params.tool_call_id. Nothing later can rebind it. The store is capped at MAX_CALL_SNAPSHOTS so a call whose handler never runs cannot leak. The rubric now comes from the same snapshot as the answer, so a graded turn cannot be scored against a rubric fetched for a later one. Note the event dispatches handlers as tasks rather than awaiting them, so the snapshot is not strictly guaranteed to be recorded before the handler runs. The fallback is safe by construction: with no snapshot the tool uses the supplied arguments, which are themselves bound to the invocation — never shared state that may have moved on. Verified with the reported sequence — turn 1 call, barge-in to turn 2, next response starts, then the delayed call runs: the old guard resolved to turn 2's answer and question, the snapshot stays on turn 1 with its matching rubric. Normal, missing-snapshot and prompt-injection cases all unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
Author
|
@HarshaNalluru @ashvathsureshkumar this PR is ready to be merged |
6 tasks
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
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img](https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img){kind=link}
![https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a](https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a){kind=link}
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 two Moss-powered developer apps:
apps/moss-interview-coach/— local-first voice interview coach (Pipecat SmallWebRTC + Whisper/Ollama/Piper) with Moss rubric retrieval, multi-track topic selection, Assist feedback panel, and subprocess-isolated grading.apps/moss-vscode/— VS Code extension for local semantic code search over the active workspace (worker-backed Moss runtime, persisted indexes, optional cloud sync), plus packaging/CI and a Remotion promo.Type of Change
Summary by CodeRabbit