Conversation
Cycle 1 Half A of LOO-1186 (live-iframe rendering). The OD daemon now
accepts a NEW manifest kind `frontend-snapshot-live` alongside the
existing `frontend-snapshot`, and persists the `liveUrl` + `viewport`
fields into project metadata so the OD viewer (cycle 2) can render
the live dev-server iframe.
Changes:
- `frontend-snapshot-import.ts`: replace single-kind constant with
`ACCEPTED_MANIFEST_KINDS` set; new `validateLiveMetadata()`
enforces http/https-only `liveUrl` (rejects javascript:, file:,
data:, etc.) and `viewport.{width,height}` inside 320-3840 x
480-2160. Static `frontend-snapshot` path is unchanged.
- `server.ts`: when manifest kind is `frontend-snapshot-live`,
persist `liveUrl` + `viewport` into the project metadata block.
Static kind path is unchanged.
- `tests/frontend-snapshot-import.test.ts`: 12 new tests covering
positive live ingest, http+https acceptance, and rejection paths
for missing/malformed liveUrl, malicious URL schemes
(javascript:, file:), missing/non-numeric/out-of-bounds viewport,
plus an updated bogus-kind test that asserts the error lists the
accepted set.
Test plan: 23 frontend-snapshot-import tests pass; full daemon
suite 341 tests pass. Stub-ZIP curl smoke verified locally:
- live ZIP -> HTTP 200, project.metadata.liveUrl + viewport persisted
- liveUrl=javascript:alert(1) -> HTTP 400 with descriptive error
- static frontend-snapshot regression -> HTTP 200, no behavior change
Half B (snapshot-package `--live-url` CLI flag) is blocked on
`looptech-ai/frontend-snapshot@v0.3.0` from P1 cycle 2.
Linear: https://linear.app/looptech/issue/LOO-1186
Contract: lune/.claude/harness/contracts/frontend-snapshot-live-iframe.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe pull request extends the manifest importer to support a new ChangesFrontend Snapshot Live Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/daemon/src/server.ts`:
- Around line 855-881: The project persists liveUrl and viewport for
'frontend-snapshot-live' via the projectMetadata object (see projectMetadata and
insertProject) but the TypeScript contracts (ProjectKind and ProjectMetadata)
are still out of sync; add a TODO comment near the projectMetadata construction
referencing LOO-1186 cycle 2 to record that the contracts must be updated to
include 'frontend-snapshot-live' in ProjectKind and add liveUrl/viewport fields
to ProjectMetadata so the mismatch is tracked and not silently ignored by
`@ts-nocheck`.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 22203c92-aa4e-45f2-987f-54b3bccfc1f2
📒 Files selected for processing (3)
apps/daemon/src/frontend-snapshot-import.tsapps/daemon/src/server.tsapps/daemon/tests/frontend-snapshot-import.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: Validate workspace
🔇 Additional comments (5)
apps/daemon/src/frontend-snapshot-import.ts (2)
39-57: LGTM — Clean expansion of accepted manifest kinds with well-documented viewport bounds.The Set-based allowlist approach is cleaner than a switch statement and makes it easy to add future kinds. The viewport bounds (320–3840 × 480–2160) covering phone-portrait through 4K are sensible, and the inline comment explains the rationale clearly.
154-206: Solid security validation for live iframe metadata.The protocol allowlist (
http:/https:only) correctly blocks dangerous schemes that could be injected into an iframe src. UsingNumber.isFinite()handles edge cases likeInfinityandNaNcleanly.apps/daemon/src/server.ts (1)
858-873: Clean conditional metadata construction.The branching correctly relies on the upstream validation in
importFrontendSnapshotZip, soliveUrlandviewportare guaranteed valid by the time they're persisted here.apps/daemon/tests/frontend-snapshot-import.test.ts (2)
253-471: Comprehensive test coverage for the new live manifest kind.The tests thoroughly cover the validation paths: valid http/https URLs, rejection of dangerous schemes (
javascript:,file://), malformed URLs, and viewport bounds/type checking.One minor gap: there's no test for
viewport.heightexceedingMAX_VIEWPORT_HEIGHT(2160), though the symmetric width test exists. This is low priority since the bounds logic is identical for both dimensions.
155-165: LGTM — Test updated to reflect expanded accepted kinds.The assertion now correctly expects the error message to list both
"frontend-snapshot"and"frontend-snapshot-live".
| // For `frontend-snapshot-live` carry liveUrl + viewport into the | ||
| // project metadata so the OD viewer can render the live iframe (cycle | ||
| // 2 of LOO-1186). The static `frontend-snapshot` path is unchanged. | ||
| const isLive = manifestMetadata.kind === 'frontend-snapshot-live'; | ||
| const projectMetadata = isLive | ||
| ? { | ||
| kind: 'frontend-snapshot-live', | ||
| importedFrom: 'frontend-snapshot', | ||
| entryFile: imported.entryFile, | ||
| sourceFileName: originalName, | ||
| liveUrl: manifestMetadata.liveUrl, | ||
| viewport: manifestMetadata.viewport, | ||
| } | ||
| : { | ||
| kind: 'frontend-snapshot', | ||
| importedFrom: 'frontend-snapshot', | ||
| entryFile: imported.entryFile, | ||
| sourceFileName: originalName, | ||
| }; | ||
|
|
||
| const project = insertProject(db, { | ||
| id, | ||
| name: declaredName, | ||
| skillId: null, | ||
| designSystemId: null, | ||
| pendingPrompt: `Imported from frontend snapshot: ${originalName}. Continue editing ${imported.entryFile}.`, | ||
| metadata: { | ||
| kind: 'frontend-snapshot', | ||
| importedFrom: 'frontend-snapshot', | ||
| entryFile: imported.entryFile, | ||
| sourceFileName: originalName, | ||
| }, | ||
| metadata: projectMetadata, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if ProjectKind includes 'frontend-snapshot-live' and if ProjectMetadata has liveUrl/viewport
echo "=== Checking ProjectKind for 'frontend-snapshot-live' ==="
rg -n "frontend-snapshot-live" --type=ts packages/contracts/
echo ""
echo "=== Checking ProjectMetadata for liveUrl and viewport fields ==="
rg -n -A 30 "export interface ProjectMetadata" packages/contracts/src/api/projects.ts | head -40Repository: looptech-ai/open-design
Length of output: 1198
🏁 Script executed:
#!/bin/bash
# Get the ProjectKind definition
echo "=== ProjectKind type definition ==="
rg -n -B 5 "type ProjectKind" packages/contracts/src/api/projects.ts | head -30
echo ""
echo "=== Checking for LOO-1186 references ==="
rg -n "LOO-1186" --type=ts .
echo ""
echo "=== Check for TODO comments related to snapshot types in contracts ==="
rg -n -B 2 -A 2 "snapshot" packages/contracts/src/api/projects.ts | head -40Repository: looptech-ai/open-design
Length of output: 649
🏁 Script executed:
#!/bin/bash
# Get the actual code at lines 855-881 in server.ts to verify it matches the review snippet
echo "=== Actual server.ts lines 855-881 ==="
sed -n '855,881p' apps/daemon/src/server.ts | cat -nRepository: looptech-ai/open-design
Length of output: 1408
🏁 Script executed:
#!/bin/bash
# Get the full ProjectKind type definition
echo "=== Full ProjectKind type definition ==="
rg -A 15 "^export type ProjectKind =" packages/contracts/src/api/projects.ts
echo ""
echo "=== Search for Cycle 2 or contracts update references ==="
rg -n "Cycle 2|cycle 2|contracts.*update|manifest.*update" --type=ts apps/daemon/src/server.tsRepository: looptech-ai/open-design
Length of output: 483
🏁 Script executed:
#!/bin/bash
# Verify the mismatch: check what insertProject expects
echo "=== Checking insertProject signature ==="
ast-grep --pattern 'function insertProject($_) {
$$$
}'Repository: looptech-ai/open-design
Length of output: 108
Add a TODO comment to document the deferred contract update for 'frontend-snapshot-live'.
The code correctly persists liveUrl and viewport for live snapshots, but the type contracts are out of sync:
ProjectKinddoes not include'frontend-snapshot-live'ProjectMetadatadoes not includeliveUrlorviewportfields
The @ts-nocheck directive hides this mismatch at compile time, but it creates a type contract gap. Since the comment references "cycle 2 of LOO-1186" and the test file mentions "cycle 1", add an explicit TODO referencing LOO-1186 to track the deferred contract update:
// TODO: Update contracts in LOO-1186 cycle 2 to include 'frontend-snapshot-live'
// in ProjectKind and add liveUrl/viewport to ProjectMetadata
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/daemon/src/server.ts` around lines 855 - 881, The project persists
liveUrl and viewport for 'frontend-snapshot-live' via the projectMetadata object
(see projectMetadata and insertProject) but the TypeScript contracts
(ProjectKind and ProjectMetadata) are still out of sync; add a TODO comment near
the projectMetadata construction referencing LOO-1186 cycle 2 to record that the
contracts must be updated to include 'frontend-snapshot-live' in ProjectKind and
add liveUrl/viewport fields to ProjectMetadata so the mismatch is tracked and
not silently ignored by `@ts-nocheck`.
There was a problem hiding this comment.
Pull request overview
Extends the OD daemon’s frontend-snapshot ZIP ingest to accept a new manifest kind (frontend-snapshot-live) and (for that kind) persist liveUrl + viewport into the created project’s metadata so the viewer can later render a live dev-server iframe.
Changes:
- Accept
frontend-snapshot-livein the frontend snapshot manifest parser and validatemetadata.liveUrl(http/https only) plus boundedmetadata.viewport. - Persist
liveUrlandviewportintoproject.metadatawhen importing a live manifest kind. - Add importer tests covering the new kind’s validation rules and update the “bogus kind” error assertion.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/daemon/src/frontend-snapshot-import.ts | Accepts frontend-snapshot-live and validates liveUrl + viewport when present. |
| apps/daemon/src/server.ts | Persists live import fields into project metadata when importing a live manifest. |
| apps/daemon/tests/frontend-snapshot-import.test.ts | Adds importer unit tests for live-kind success and validation failures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // For `frontend-snapshot-live` carry liveUrl + viewport into the | ||
| // project metadata so the OD viewer can render the live iframe (cycle | ||
| // 2 of LOO-1186). The static `frontend-snapshot` path is unchanged. | ||
| const isLive = manifestMetadata.kind === 'frontend-snapshot-live'; | ||
| const projectMetadata = isLive | ||
| ? { | ||
| kind: 'frontend-snapshot-live', | ||
| importedFrom: 'frontend-snapshot', | ||
| entryFile: imported.entryFile, | ||
| sourceFileName: originalName, | ||
| liveUrl: manifestMetadata.liveUrl, | ||
| viewport: manifestMetadata.viewport, | ||
| } | ||
| : { | ||
| kind: 'frontend-snapshot', | ||
| importedFrom: 'frontend-snapshot', | ||
| entryFile: imported.entryFile, | ||
| sourceFileName: originalName, | ||
| }; |
| // For `frontend-snapshot-live` carry liveUrl + viewport into the | ||
| // project metadata so the OD viewer can render the live iframe (cycle | ||
| // 2 of LOO-1186). The static `frontend-snapshot` path is unchanged. | ||
| const isLive = manifestMetadata.kind === 'frontend-snapshot-live'; | ||
| const projectMetadata = isLive | ||
| ? { | ||
| kind: 'frontend-snapshot-live', | ||
| importedFrom: 'frontend-snapshot', | ||
| entryFile: imported.entryFile, | ||
| sourceFileName: originalName, | ||
| liveUrl: manifestMetadata.liveUrl, | ||
| viewport: manifestMetadata.viewport, | ||
| } |
Summary
Cycle 1 Half A of LOO-1186 (live-iframe rendering). Extends the OD daemon to accept a NEW manifest kind
frontend-snapshot-livealongside the existingfrontend-snapshot, and persists theliveUrl+viewportfields into project metadata so the OD viewer (cycle 2) can render the live dev-server iframe.lune/.claude/harness/contracts/frontend-snapshot-live-iframe.md--live-urlCLI flag) is blocked onlooptech-ai/frontend-snapshot@v0.3.0from P1 cycle 2.Changes
apps/daemon/src/frontend-snapshot-import.tsEXPECTED_MANIFEST_KINDconstant withACCEPTED_MANIFEST_KINDSset (frontend-snapshot,frontend-snapshot-live).validateLiveMetadata()enforces:liveUrlis a string and parses withnew URL()http:orhttps:only (rejectsjavascript:,file:,data:,ftp:, etc.)viewport.widthis finite number in[320, 3840]viewport.heightis finite number in[480, 2160]frontend-snapshotpath is unchanged (no behavior diff).apps/daemon/src/server.tsmanifest.metadata.kind === 'frontend-snapshot-live', persistliveUrl+viewportinto the project metadata block.apps/daemon/tests/frontend-snapshot-import.test.tsliveUrl,javascript:URL,file://URL, malformed URL, missing viewport, negative width, non-number width, missing height, out-of-bounds width and height. Plus an updated bogus-kind test that asserts the error lists the accepted set.Test plan
pnpm --filter @open-design/daemon test— 341 tests pass (was 329; +12 new), including 23 infrontend-snapshot-import.test.ts(was 11).pnpm tools-dev+ manualnode -escript that builds a zero-dep ZIP, nofrontend-snapshotexporter dependency):http://localhost:3000, viewport 1440x900) -> HTTP 200, response payload includesproject.metadata.liveUrlandproject.metadata.viewport.liveUrl=javascript:alert(1)-> HTTP 400 with errormanifest.json metadata.liveUrl must use http: or https: (got javascript:).frontend-snapshotregression -> HTTP 200, metadata kind=frontend-snapshot, noliveUrl/viewportfields (unchanged shape).Notes for reviewer
validateLiveMetadata()is deliberately strict on URL protocol because the value flows directly into an<iframe src>in cycle 2; allowingjavascript:ordata:would create a stored-XSS vector via the manifest.🤖 Generated with Claude Code
Summary by CodeRabbit