WIP: Code preview #4

2026-08-09T23:40:18Z

Athena commented

2026-08-09 23:40:18 +00:00

No description provided.

Athena added 2 commits 2026-08-09 23:40:19 +00:00

feat(preview): add sandboxed live code previews a6e2ec1136

Render validated HTML, SVG, JSX, and TSX fences locally while preserving tool context and preventing explanatory JSON from triggering actions.

Co-authored-by: Cursor <cursoragent@cursor.com>

fix(runtime): improve local service reliability 193e91b234

Close SQLite handles safely on Windows, clean orphaned vectors, normalize Ollama endpoints, and surface model errors without leaking reasoning tags.

Co-authored-by: Cursor <cursoragent@cursor.com>

enderofwings marked the pull request as work in progress 2026-08-25 14:16:13 +00:00

enderofwings reviewed 2026-08-25 14:23:39 +00:00

enderofwings left a comment

Reviewed at 193e91b. The feature is good and the sandboxing is right; the branch just needs to come forward before any of it can land.

Please pull and rebase first

This branched from b5541d1, which was main before the 2026-08-25 sync (42eaed6). main has moved a long way since: the Projects workspace replaced the Documents page, modules/ (mail, network) landed, memory curation moved in-process into synapse/memory/curator.py, and the separate memory service on :8001 was deleted.

A merge from here conflicts in six files (bin/sync.py, synapse/main.py, synapse/memory/store.py, synapse/tools.py, tests/test_smoke.py, tests/test_tools.py), and resolving them the wrong way would revert that work — the diff against current main shows Projects.jsx, modules/ and curator.py being deleted and Documents.jsx plus synapse/memory/service.py coming back.

git fetch origin
git rebase origin/main

synapse/tools.py will be the fiddly one: main reworked it, and the preview additions here sit on top of the older shape.

Suggestion: split this into two PRs

The second commit, "fix(runtime): improve local service reliability", is four unrelated fixes that have nothing to do with previews, and every one of them is worth having on its own:

contextlib.closing on the sqlite handles in bin/sync.py. Real bug: sqlite3's context manager commits without closing, and restore_db() unlinks the file immediately afterwards, which Windows refuses while a handle is open.
_normalize_ollama_host. OLLAMA_HOST=0.0.0.0:11434 is the normal way to expose Ollama on a LAN and it silently produced an empty model list. Good catch, and the docstring explains exactly why.
<think> stripping for models that emit reasoning inline in content.
_raise_for_ollama, surfacing the error body instead of a bare status.

Landing those first, rebased and separate, gets them in without waiting on the preview review. The commit title should say what they are — "improve local service reliability" doesn't survive a git log search.

One overlap to drop while rebasing: _delete_message_vectors duplicates a fix already on main in delete_conversation, which does the same thing in one statement, without the 500-id batching, and checks sqlite_master first because vec_messages is created lazily on the first semantic search. Keep your _sweep_orphan_msg_vectors though — databases that predate that fix still carry orphans, and nothing else cleans them up.

The sandbox model is correct

sandbox="allow-scripts" deliberately without allow-same-origin, plus default-src 'none' / connect-src 'none' and base-uri named explicitly because it does not fall back to default-src. Identifying the postMessage sender by window object rather than e.origin is right — an opaque origin reports the string "null", which proves nothing. Height clamped, error text rendered as a string.

That matters more here than it would in most apps: the backend REST API is unauthenticated on loopback, so model-authored code running same-origin would have had the whole /memory and /conversations surface. Keeping the backend validation read-only is the right boundary.

The JSX transform is the part that will generate bugs

686 hand-written lines of lexer and parser is a lot to own. The stated justification is payload size, but the transform runs in the parent window, not inside the frame — it is bundle weight once, not per-preview bytes. Only the ~15 KB preact UMD is per-preview, and that argument stands on its own.

A dynamically imported sucrase (or @babel/standalone) would delete the file, load only when someone actually opens a preview, and bring a real parser's edge cases with it. The file's own comments already record hitting the classic ambiguity — Math.sin(t) / 6 scanning as an unterminated regex — which is exactly the failure mode a hand-rolled scanner keeps producing.

In its favour: the scope really is narrow, failures throw rather than emit silently wrong code, and the 32 node --test cases pass. If the dependency was measured and rejected, say so in the header comment and I'll stop asking.

Consider cutting the critique heuristics

The _critique_* family in synapse/tools.py — roughly 350 lines deciding whether a model's SVG "looks like a plot rather than decoration", whether prose was requested, whether a canvas is used — plus the scaffold/retry path and _should_nudge_render / _last_ok_render_fence in synapse/chat.py, is the part I would leave out. It is string heuristics grading model output, it sits in the chat hot path, and there is no test that can tell you it got the judgement wrong rather than merely different. The preview works without any of it: validate that the fence parses, render it, let the reader look.

If you want to keep it, a third PR of its own would make it reviewable on its own merits.

Summary

Rebase onto origin/main, split the runtime fixes out and land those first, and the preview is close. The security work on the frame is the part I would not change.

Reviewed at 193e91b. The feature is good and the sandboxing is right; the branch just needs to come forward before any of it can land. ## Please pull and rebase first This branched from `b5541d1`, which was `main` before the 2026-08-25 sync (`42eaed6`). `main` has moved a long way since: the Projects workspace replaced the Documents page, `modules/` (mail, network) landed, memory curation moved in-process into `synapse/memory/curator.py`, and the separate memory service on :8001 was deleted. A merge from here conflicts in six files (`bin/sync.py`, `synapse/main.py`, `synapse/memory/store.py`, `synapse/tools.py`, `tests/test_smoke.py`, `tests/test_tools.py`), and resolving them the wrong way would revert that work — the diff against current `main` shows `Projects.jsx`, `modules/` and `curator.py` being deleted and `Documents.jsx` plus `synapse/memory/service.py` coming back. ``` git fetch origin git rebase origin/main ``` `synapse/tools.py` will be the fiddly one: `main` reworked it, and the preview additions here sit on top of the older shape. ## Suggestion: split this into two PRs The second commit, "fix(runtime): improve local service reliability", is four unrelated fixes that have nothing to do with previews, and every one of them is worth having on its own: - `contextlib.closing` on the sqlite handles in `bin/sync.py`. Real bug: sqlite3's context manager commits without closing, and `restore_db()` unlinks the file immediately afterwards, which Windows refuses while a handle is open. - `_normalize_ollama_host`. `OLLAMA_HOST=0.0.0.0:11434` is the normal way to expose Ollama on a LAN and it silently produced an empty model list. Good catch, and the docstring explains exactly why. - `<think>` stripping for models that emit reasoning inline in `content`. - `_raise_for_ollama`, surfacing the error body instead of a bare status. Landing those first, rebased and separate, gets them in without waiting on the preview review. The commit title should say what they are — "improve local service reliability" doesn't survive a `git log` search. One overlap to drop while rebasing: `_delete_message_vectors` duplicates a fix already on `main` in `delete_conversation`, which does the same thing in one statement, without the 500-id batching, and checks `sqlite_master` first because `vec_messages` is created lazily on the first semantic search. Keep your `_sweep_orphan_msg_vectors` though — databases that predate that fix still carry orphans, and nothing else cleans them up. ## The sandbox model is correct `sandbox="allow-scripts"` deliberately without `allow-same-origin`, plus `default-src 'none'` / `connect-src 'none'` and `base-uri` named explicitly because it does not fall back to `default-src`. Identifying the postMessage sender by window object rather than `e.origin` is right — an opaque origin reports the string `"null"`, which proves nothing. Height clamped, error text rendered as a string. That matters more here than it would in most apps: the backend REST API is unauthenticated on loopback, so model-authored code running same-origin would have had the whole `/memory` and `/conversations` surface. Keeping the backend validation read-only is the right boundary. ## The JSX transform is the part that will generate bugs 686 hand-written lines of lexer and parser is a lot to own. The stated justification is payload size, but the transform runs in the parent window, not inside the frame — it is bundle weight once, not per-preview bytes. Only the ~15 KB preact UMD is per-preview, and that argument stands on its own. A dynamically imported `sucrase` (or `@babel/standalone`) would delete the file, load only when someone actually opens a preview, and bring a real parser's edge cases with it. The file's own comments already record hitting the classic ambiguity — `Math.sin(t) / 6` scanning as an unterminated regex — which is exactly the failure mode a hand-rolled scanner keeps producing. In its favour: the scope really is narrow, failures throw rather than emit silently wrong code, and the 32 `node --test` cases pass. If the dependency was measured and rejected, say so in the header comment and I'll stop asking. ## Consider cutting the critique heuristics The `_critique_*` family in `synapse/tools.py` — roughly 350 lines deciding whether a model's SVG "looks like a plot rather than decoration", whether prose was requested, whether a canvas is used — plus the scaffold/retry path and `_should_nudge_render` / `_last_ok_render_fence` in `synapse/chat.py`, is the part I would leave out. It is string heuristics grading model output, it sits in the chat hot path, and there is no test that can tell you it got the judgement wrong rather than merely different. The preview works without any of it: validate that the fence parses, render it, let the reader look. If you want to keep it, a third PR of its own would make it reviewable on its own merits. ## Summary Rebase onto `origin/main`, split the runtime fixes out and land those first, and the preview is close. The security work on the frame is the part I would not change.

Athena commented

2026-08-26 07:49:03 +00:00

Closing as superseded. This branch's two commits (a6e2ec1 sandboxed previews, 193e91b runtime reliability) are already carried forward into feat/macos-support, which is now merged onto current main (v1.2.0) in #[PR number].

Specifically checked against the concerns raised in the review here:

_sweep_orphan_msg_vectors — present (synapse/memory/store.py), survived the v1.2.0 merge intact.
contextlib.closing() on the sqlite handles in bin/sync.py — present, four call sites.
_normalize_ollama_host, -stripping, _raise_for_ollama error surfacing — all present and covered by tests carried through the merge.
The critique-heuristics and JSX-parser feedback still stands as open follow-up — raising as a separate issue rather than letting it block this close.

Closing as superseded. This branch's two commits (a6e2ec1 sandboxed previews, 193e91b runtime reliability) are already carried forward into feat/macos-support, which is now merged onto current main (v1.2.0) in #[PR number]. Specifically checked against the concerns raised in the review here: _sweep_orphan_msg_vectors — present (synapse/memory/store.py), survived the v1.2.0 merge intact. contextlib.closing() on the sqlite handles in bin/sync.py — present, four call sites. _normalize_ollama_host, <think>-stripping, _raise_for_ollama error surfacing — all present and covered by tests carried through the merge. The critique-heuristics and JSX-parser feedback still stands as open follow-up — raising as a separate issue rather than letting it block this close.

Athena closed this pull request

2026-08-26 07:49:03 +00:00

Athena referenced this pull request

2026-08-26 08:40:50 +00:00

fix(runtime): close SQLite handles and harden Ollama I/O #5

Athena referenced this pull request

2026-08-26 08:41:12 +00:00

feat(preview): add sandboxed previews with lazy JSX compilation #6