Repository navigation
Conversation
jeromevdl
added
area/frontend
|
Thanks for your contribution @POLAARK, good idea, we'll review this. |
Complexity and test coverageAutomated quality report using code complexity and test coverage to highlight potential hot spots in this PR.
🟢 Improving · 🟠 Needs attention · 🔴 High risk · 🟣 Highest risk — = unavailable · n/a = not applicable · * = incomplete tests |
github-actions
Bot
added
area/backend
jeromevdl
left a comment
jeromevdl
left a comment
There was a problem hiding this comment.
@POLAARK we don't have Playwright in the project, and we cannot introduce it like that.
Please remove it and eventually propose an RFC to add playwright in the project, we are evaluating Devops Agent right now so Playwright is not (at least yet) welcome. I didn't review the rest.
|
Makes sense, I agree. |
|
Thanks for the review everything is fixed and Playwright is removed. |
|
Cool feature - something our product team have enquired about so super interested in this one, nice work 👍 |
jeromevdl
left a comment
jeromevdl
left a comment
There was a problem hiding this comment.
Thanks for the taking the comments into consideration, sorry for the time lost on Playwright.
One small finding before merge, and maybe adjust the PR description (iframe sizing, same-origin, quorum limitation)
| csp.setAttribute('http-equiv', 'Content-Security-Policy'); | ||
| csp.setAttribute('content', ARTIFACT_HTML_CSP); | ||
| doc.head.prepend(csp); | ||
| doc.querySelectorAll('a, area').forEach((link) => { |
There was a problem hiding this comment.
P2 — SVG animation can restore external links
The cleanup removes href, but SVG <set> or <animate> can restore it afterward, without JavaScript. I reproduced this in Chromium using the updated source helper: clicking the link issued an external request and replaced the iframe content.
Fix: Exclude these SVG animation elements under the static-preview policy and add a browser regression test. This is the previous navigation finding, partially fixed.
![collaborative-ai-dlc[bot]](https://avatars.githubusercontent.com/u/8931462?s=60&v=4){kind=small}
| export function ArtifactContent({ title, ...props }: Props) { | ||
| return isHtmlDocument(props.content) ? ( | ||
| <ArtifactHtmlFrame content={props.content} title={title} /> |
There was a problem hiding this comment.
[P2] Gate HTML rendering by artifact type
Renderer selection checks only the content and ignores artifactType. Persisted nonvisual artifacts created before these new backend guards, or by an older writer, therefore switch from Markdown handling to iframe rendering even though the feature contract permits HTML only for three visual types.
Suggested fix: Require an allowed artifact type as well as HTML-document detection in ArtifactContent, pass the type from every caller including history, apply the same condition to ArtifactViewer collapse behavior, and cover a legacy nonvisual HTML record.
Evidence: The base MCP update and manual-edit paths accepted unrestricted artifact content. The exact head still renders any HTML-looking content without an artifact type, and every production ArtifactContent caller omits that type. IntentArtifact and ArtifactVersionSummary already expose artifactType, so the policy can be enforced at renderer selection. This is distinct from the reported navigation issue because it controls which artifact categories enter the HTML renderer.
5548c40 to
515c2f9
Compare
An artifact whose content is a complete HTML document renders in an iframe instead of the markdown renderer. The sandbox has no allow-scripts, and an injected Content-Security-Policy blocks every network request, so the page can only display inline CSS, SVG and data: images. A Rendered/Source toggle shows the markup.
wireframes, user-flow and mockups may be stored as a complete HTML document. HTML for any other type is refused by create_artifact and update_artifact, by the Quorum rewrite guard, and by the artifact edit API (400 html_not_allowed). required-sections counts <h2> elements in an HTML artifact. The execution annex tells agents when HTML is allowed and that it must be static and self-contained.
Build the frame document with DOMParser so the CSP meta comes first in <head>, and remove everything that can navigate the frame: links become span/g, area, base, link, nested frames, plugins, meta refresh and DNS prefetch control go, shadow-root templates are made inert and cleaned. The frame renders only a srcdoc that re-parses to itself with nothing that can navigate; otherwise it shows the source. The frame has a fixed height and sandbox="", and ArtifactContent picks the HTML or markdown renderer for every artifact view.
Use one sectionHeadings rule for the sensor and the rewrite check, and end leading HTML comments at --!> as browsers do. A visual artifact stored as HTML stays HTML: the prompts ask for an HTML document with <h2> structure rules, and an HTML to markdown answer is refused. Each side's sections are counted by its own format, and a legacy HTML document of a non-visual type converts to markdown keeping each <h2> as a ## heading.
515c2f9 to
a727943
Compare
Summary
Visual artifacts (
wireframes,user-flow,mockups) can now be stored as a complete HTML page and are shown to the reviewer as that page, instead of a markdown description of a screen. All other artifact types stay markdown, and HTML is refused for them.Source view
Why
The rough-mockups and refined-mockups stages produce screens, but the only format available was markdown, so reviewers got prose and ASCII layouts. HTML lets the design agent produce a layout the reviewer can actually look at during the review gate.
Changes
Rendering (frontend). When an artifact's content starts with
<!doctype html>or<html>,ArtifactMarkdownandArtifactViewerrender it in an iframe through the newArtifactHtmlFrame. This covers the work products list, the stage review panel, the artifact history drawer, and the activity panel. The card has a Rendered / Source toggle, and long pages collapse by height rather than by character count, since cutting markup would break the page.Isolation. Artifact content is written by agents and can contain text taken from tickets or repositories, so the page is treated as untrusted:
sandbox="allow-same-origin"withoutallow-scripts. No script, event handler, form submission, popup, or top-level navigation runs. Same-origin is only granted so the app can read the page height; a comment marks that it must never be combined withallow-scripts.Content-Security-Policymeta tag is inserted at the top of the page withdefault-src 'none', allowing only inline styles anddata:images, fonts, and media. Viewing an artifact makes no network request.Allowed types (backend).
lambda/shared/html-artifact.jsdefines the allowed types and the refusal message. HTML for any other type is refused at every write path:create_artifact/update_artifact(MCP)GraphWriteErrorthe agent can act on; nothing is writtenPUT .../artifacts/{id}/content400withcode: html_not_allowedSensors.
required-sectionscounts distinct<h2>elements in an HTML artifact the way it counts##lines in markdown, so the two-section floor applies to both formats.upstream-coveragealready works on the raw text and needed no change.Agent guidance. The execution annex names the three types and states that the page must be static and self-contained (inline CSS, inline SVG,
data:images, at least two<h2>sections, upstream slugs cited in the text).Docs. New "HTML artifacts" section in
docs/using-the-platform/intent-observability.md.Compatibility
No data migration. Existing artifacts are markdown and render as before. The stored
content_typeis unchanged; HTML is detected from the content. Markdown that contains inline HTML tags is not detected as a page.Out of scope: Quorum's rewrite prompt still asks for markdown, so a Quorum edit of an HTML wireframe may return it as markdown (which is allowed). The section-level read tools (
get_artifact_toc,get_section) do not split HTML artifacts, so downstream stages read them whole.Testing
npm run lint: 0 errors (40 warnings, same asmain)npm run format:check,npm run secretlint: passnpm run dep:check: 10 warnings, same asmainnpx vitest run --project=shared --project=agentcore --project=intents --project=v2-orchestrator: 4809 passednpm run test:aidlc-compatibility: 242 passednpm --prefix frontend test: 1035 passed;npm --prefix frontend run build: pass<script>, anonerrorhandler,fetch, an external stylesheet, a tracking pixel, a form submit, and atarget=_toplink left the host page untouched and produced no network requests, while inline CSS and SVG rendered. Without the sandbox and CSP, the same page changed the host title and made 4 requests. Firefox and WebKit were not tested.