Implements all reviewer comments (code-review, red-team, and test-strategy
audit), accepting the recommended variants.
Server — realtime service (ai-realtime.service.ts):
- SSRF: pin the validated IP via a WebSocket `lookup` hook that re-checks every
resolved address with isIpAllowed (mirrors external-mcp buildPinnedDispatcher),
closing the TOCTOU/DNS-rebinding window; fix the misleading comment.
- no-silent-loss: on Stop, drain the in-flight segment (bounded 2.5s) and deliver
the final via onFinal before closing instead of dropping the tail.
- fail-closed deriveRealtimeUrl: a non-empty unparseable base now THROWS (no
silent api.openai.com fallback that would leak a self-hosted key); http://ws://
bases rejected (plaintext key). Path normalization preserved.
- parseUpstreamEvent keys the accumulator by item_id+content_index so GA segments
don't concatenate.
- inject a wsFactory seam for testing; also fix a latent bug — `import WebSocket
from 'ws'` resolved to undefined at runtime (no esModuleInterop) -> import=require.
- unref idle/max/drain timers.
Server — realtime gateway (ai-realtime.gateway.ts, session-limits.ts):
- reject revoked/disabled users and inactive sessions (mirror jwt.strategy:
findById+isUserDisabled + findActiveById) with NO counter increment.
- CSWSH: Origin allowlist (matching APP_URL, or no Origin for native clients)
before auth, no increment.
- extract SessionCounters (delete-at-zero, never negative) + pure canConnect
(both caps >= checked before any increment); document the per-process/in-memory
cap caveat (single-replica only).
Client:
- dictation-group: realtime final now inserts at the captured rangeRef SNAPSHOT
(not the live caret) and guards editor.isEditable; single-space separator.
- use-realtime-dictation/realtime-dictation-client: stop-during-acquisition tears
down the mic (no leak / button reset); reconnect re-emits start (double-start
guarded); interim ghost cleared on teardown; io() options de-duplicated.
- pcm16-worklet: flush the partial sub-frame tail on stop; one-pole anti-aliasing
low-pass before 48k->24k.
- extract shared mic-capture (acquireMicStream/mapGetUserMediaError, used by batch
+ realtime), pure DSP (pcm16-dsp.ts), and the session reducer/baseLanguageSubtag;
extract applyInterimMeta/clampRange/resolveUrl/appendFinalToDraft.
Tests + infra: +~150 server tests (deriveRealtimeUrl, parseUpstreamEvent branches,
openSession/lifecycle/timers/testConnection via fake ws, gateway auth/caps/no-leak,
realtime-test admin contract, AiSettings update/resolve, DTO boolean, SSRF deny)
and +~140 client tests (DSP property/edge, resampler continuity, framing, reducer,
mic-capture, RealtimeDictationClient/MicButton, ProseMirror interim regression +
history guards, appendFinalToDraft, resolveKeyField, route contract). Added
@vitest/coverage-v8. CHANGELOG [Unreleased] entry incl. the single-replica caveat.
Review: APPROVE WITH SUGGESTIONS (no critical/regression); applied the drain-timer
unref. Server tsc clean + 358 tests; client tsc clean + 201 tests; vite build ok.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-ups from the multi-aspect review of the e5bc82c7..d4658d4c range.
- CHANGELOG: document under [Unreleased] that the default per-workspace
hourly public-share assistant cap was lowered 300 -> 100 after the
v0.93.0 tag (#62). v0.93.0 shipped 300, so existing deployments that
never set SHARE_AI_WORKSPACE_MAX_PER_HOUR drop to 100 on upgrade.
- Recreate the still-open Section 3 (AiChatService.stream integration
coverage) of the deleted feature-test-coverage-deferred.md as a focused
backlog doc so the test debt stays tracked; Sections 1-2 are already
closed by the integration harness (PR #115).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address the non-test code-review findings on the htmlEmbed sandbox change
(test-coverage gaps are tracked in issue #99):
- html-embed-view: track the iframe's reported content height even while a
fixed height is set, so clearing the height (fixed -> auto) without editing
the source no longer leaves the frame pinned to the stale value. Derive the
fixed-height predicate once; seed autoHeight to the default.
- html-embed-view: drop width/border from the iframe inline style (the
.htmlEmbedFrame CSS class already provides them).
- html-embed-sandbox: coalesce height reports via requestAnimationFrame and
skip <=1px deltas to damp the self-measure feedback loop; fix the misleading
bootstrap comment.
- tracker-settings: add an aria-label to the snippet Textarea (a11y).
- CHANGELOG: note the removal of server-side role-based HTML-embed stripping.
The shared MCP_TOKEN guard moved from 'Authorization: Bearer <MCP_TOKEN>' to the
X-MCP-Token header (Authorization is now per-user Basic/Bearer), silently breaking
existing /mcp clients. Document it as a Breaking Change in CHANGELOG (reconfigure
to X-MCP-Token). Add a once-per-process migration warning: when MCP_TOKEN is set,
no x-mcp-token is present, and Authorization carries the old 'Bearer <MCP_TOKEN>',
log a hint to migrate — without changing the auth decision (still rejected) or
logging the token value.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up fixes to the htmlEmbed-sandbox / trackerHead change:
- share-seo: inject trackerHead via a function replacer so `$`-sequences
($&, $', $`, $$) in the admin snippet are inserted literally instead of
being treated as String.replace substitution patterns; warn when the
</head> marker is absent instead of silently skipping injection.
- mcp: register a passthrough `htmlEmbed` node in the schema mirror so an
AI/MCP edit of a page containing an embed no longer throws
"Unknown node type: htmlEmbed" in TiptapTransformer.toYdoc.
- editor-ext + client: treat a non-finite `data-height` as auto (null) so a
crafted/corrupted height cannot disable auto-resize or yield a NaN iframe
height; extract a shared clampHeight helper.
- client: rename render-raw-html.{ts,test.ts} -> html-embed-sandbox.{...} and
shouldExecute -> shouldRender so the seam name matches the sandbox model.
- client: i18n the iframe title; surface the real error reason in
tracker-settings (console.error + err.response.data.message).
- docs: note hasHtmlEmbedNode is now a test-only helper; add an Unreleased
CHANGELOG entry; drop the dangling "arbitrary HTML embed" planning-doc ref.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
