feat(preview): shareable preview mode — share, comment sync, parity#626
Merged
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add TestShareSyncPreview (integration-tagged) that publishes the testdata/preview fixture to a live crit-web via `crit share --preview` and asserts each asset round-trips through `/r/<token>/raw/<path>`: style.css and app.js match the fixtures verbatim, logo.png decodes back to its original PNG bytes, and index.html carries the injected preview agent. Named TestShareSync* so the e2e-share.sh `-run TestShareSync` filter picks it up; it uses the existing critWebURL/critBinary helpers. Add TestCrawlPreviewFixtureShape (no server required) that asserts the crawlPreview seam produces the same payload shape the relay sends: text assets inlined with no encoding, the binary asset base64-encoded and decodable to PNG magic. Add testdata/preview fixtures (index.html, style.css, app.js, and a valid 16x16 logo.png). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rawler crawlPreviewFromURL + its helper chain (collectHTMLAssetsFromURL, collectCSSAssetsFromURL, cleanRelPathForURL, resolveURL, fetchAsset, the cssEntry type, and crawlHTTPClient) had zero production callers and zero tests — every path uses the filesystem crawlPreview. The `unused` linter is not enabled, so the hook would never have flagged it. Also drops the now-unused net/http, net/url, time, and io imports. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The Share button is part of CRI-78's design ("Share button in design-mode UI
triggers crawl + upload"), but it never appeared in `crit preview`.
- preview.go: default the share URL via resolveShareURL(..., defaultShareURL),
the same as code review. Previously preview used an empty default, so the
Share button stayed hidden unless --share-url/config was set (files review got
it via the crit.md default; preview did not — an inconsistency).
- frontend/app.js: gate the Share button on reviewType !== 'live' so it shows
in files + preview reviews but not design/live mode (a live preview proxies a
running app — nothing self-contained to publish) or vcs/diff (git).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…+ preview The Share flow lived entirely in app.js (code-review only). Preview mode runs live-mode.js chrome, so the Share button — though shown — was wired to nothing. Extract the whole flow into frontend/crit-share.js (IIFE + dual-export window.crit.share), a factory that owns all share state and wires the #shareBtn click handler. Both modes consume it via injected adapters/options: - app.js builds options and calls window.crit.share.create(...).reveal() - live-mode.js installShareController() (preview only) reads /api/config and creates the controller with reviewType:'preview', canShare:true. Transport, not protocol: every endpoint is byte-for-byte the pre-extraction app.js code; the proxy_auth relay still hits the same crit-web APIs. Preview's only branch is the payload-builder endpoint (/api/share/preview-payload). Also: - index.html: add crit-share.js + crit-icons.js to liveDeps; mark <html> with crit-mode-preview so style-live.css un-hides #shareBtn in preview (design/live stay hidden — nothing to publish behind a running app). - live-mode.js: use canonical window.crit.icons.ICON_CLIPBOARD/ICON_CHECK_SMALL for the share modal copy buttons instead of a hand-rolled filled-clipboard glyph, so preview matches code-review exactly. - frontend/__tests__/crit-share.test.js: 8 node --test cases for the module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two bugs in the in-UI preview Share flow, both rooted in a key mismatch between the session's identity and the crawled snapshot: 1. Shared status was lost on restart, so re-sharing duplicated the review. handleShare persisted the share scope over the *crawled* asset paths (index.html, style.css, ...), but restoreShareStateLocked recomputes scope from the session's single previewed file — so it never matched and the share pointer was dropped. Now the scope is always derived from the session's file identity (FilePathsSnapshot), which both sides agree on. handleShareURL (proxy-auth persist path) now sets the scope too. 2. Existing comments were not shared. The crawler keys the HTML as "index.html" (previewMainHTMLKey), but comments are stored under the session's previewed-file path, so the per-file filter dropped them — in the direct path (handleShare) and the proxy path (handlePreviewPayload passed nil comments). Both paths now load the previewed file's comments and re-key them to the crawl entry via remapPreviewCommentFiles. Tests: remap primitive, direct-path payload re-key, proxy-payload comment inclusion, and scope restore-on-restart (with a pre-fix regression guard). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…c comment Review follow-ups: assert a resolved preview comment is dropped from the shared payload (locks the filter-then-remap ordering), and correct the crit-share.js header comment which documented a nested `config` object — options are flat. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two more preview-share gaps, both from the same realisation: preview comments are DOM-anchored pins stored in a *separate* "live-route" FileEntry keyed by the iframe pathname (/preview-content), not under the previewed HTML's FileEntry. 1. Re-share failed with "no files in session". handleUpsertPayload used LoadShareFilesFromDisk, which is empty for a preview session (its FileEntry has no on-disk AbsPath). Now it crawls via shareFilesForSession (same source as the initial share) and re-keys pins. 2. The proxy preview-payload only loaded comments for the first session path, missing pins (which live on a later live-route entry). Both it and the upsert builder now load across ALL session paths (FilePathsSnapshot) and collapse onto the crawl entry via the new loadPreviewShareComments helper. handleShare was already correct (it loads all session paths). Verified end-to-end against a live daemon + stub: a /preview-content pin ships in the POST /api/reviews payload re-keyed to index.html. Tests now model the real storage (pin in a live-route entry); the upsert test guards "no files in session". Earlier unit tests used the wrong storage model and missed this. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The vendoring instruction (re-sync agent-*.js into crit-web, run the drift-guard test) only lived in crit-web's AGENTS.md, but its trigger targets someone editing those files in crit/ — who reads this file, not crit-web's. Add a direction-flipped block so the guidance reaches its intended reader. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Companion PR (hosted crit-web side): tomasz-tomczyk/crit-web#241 |
3 tasks
The HTML/CSS preview crawler added golang.org/x/net to go.mod; the flake's Go module vendorHash was never updated, failing the Nix build in CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #626 +/- ##
==========================================
- Coverage 70.64% 70.62% -0.03%
==========================================
Files 54 55 +1
Lines 14248 14534 +286
==========================================
+ Hits 10066 10264 +198
- Misses 3412 3476 +64
- Partials 770 794 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cleanRelPath/resolveCSSRef computed shareFile path keys with filepath.Clean/ Join, which yields backslashes on Windows — breaking the asset keys (they must match HTML/CSS refs and crit-web's served paths) and failing TestCleanRelPath on windows CI. Use the `path` package for web-path keys; disk reads keep filepath. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Local-CLI side of CRI-78 — makes
crit previewshareable to crit-web and fixes the preview comment round-trip.review_type+ per-file encoding.crit share --previewuploads a standalone snapshot from the CLI.frontend/crit-share.js(IIFE + dual-export), consumed by both code-review (app.js) and preview (live-mode.js) — the button was previously wired to nothing in preview.POST /api/share, proxy relay, re-share upsert) now load comments across every session path and re-key them to the crawl entry — previously they were dropped, and re-share failed with "no files in session".Review
/crit-review): SHIP IT, no blockers (go-expert + frontend-expert, validated)go test -race,golangci-lint,gofmt: cleanmake e2e-share: allTestShareSync*pass against crit-webTest plan
/preview-contentpin ships inPOST /api/reviewsre-keyed toindex.html.make e2e-sharegreen.🤖 Generated with Claude Code