fix(harness): show one message in the canvas Render-failed state [SAP-3199]

[gwitwer]

Fixes SAP-3199.

The bug

When an agent cannot render, the canvas pane drew two error strings on top of
each other: the app's Render-failed card and the rendered document's own prose,
in the same area, unreadable.

Two things paint the same failure:

1. CanvasPane.tsx draws the Render-failed card (glyph, short claim, one-line
   reason, Ask/Retry, full reason behind Details). .canvas-render-error is
   position: absolute; inset: 0 with no background, so it is a transparent
   layer over the iframe.
2. The document in that iframe (buildErrorPanelHtml) paints its own paragraph:
   "Could not extract this agent's step graph: ... Use the workbench actions to
   ask your coding agent to fix it or retry the deterministic render."

Nothing hid the second one, so both showed through each other.

The stylesheet already claimed otherwise. The comment above
.canvas-render-error read "the document's own error text is hidden by the board
snippet". That was the intent; the mechanism was never there. This PR implements
it and corrects the comment.

The fix

The document stands its prose down while it is embedded, exactly the way the
title, badge and legend already do. canvas-template.ts has a rule that visually
hides .canvas-header and .canvas-legend because they are app chrome the SPA
draws around the iframe, with the reason spelled out: "floating a second copy
over the board would just crowd the diagram". The failure reason is the same kind
of duplicate, so it gets the same treatment, one line lower in the same block.

• buildErrorPanelHtml tags the paragraph canvas-render-error-note.
• A new :root[data-canvas-embedded] .canvas-render-error-note { display: none }
  sits beside the existing header rule.
• A four-line head script sets data-canvas-embedded when
  window.parent !== window. It is its own script, before the body, so the flag
  is set before first paint and a parse error in the much larger run-state script
  cannot leave it unset.

Opened standalone there is no card coming, so the prose stays and is the only
message. That direction is pinned by its own spec.

Nothing about the card itself changed, and no user-visible string changed.

Verification

Real harness server (node dist/cli/bin.js, --state-root of its own) against a
real agent tree, selecting a project folder with no index.ts, which is the repro
in the ticket. Light and dark, before and after, below.

Specs, in e2e/canvas-render.spec.ts:

• embedded shows one message. The reason reaches the parent exactly once, and
  no element in the document paints a second copy. "Paints" is hit-tested with
  elementFromPoint rather than inferred from the box, because the title and
  badge sit inside a clipped 1px .canvas-header while keeping full-size rects
  of their own. Every text-bearing element is walked, not just the two known
  ones, so a third string added later cannot quietly reintroduce the overlap.
• standalone keeps the prose as the only message.

Mutation-tested, three ways, each confirmed red before restoring:

Mutation	Result
Delete the stylesheet rule	embedded spec fails
Drop the class from the paragraph	embedded spec fails
Hide unconditionally (drop the window.parent check)	standalone spec fails

Local: pnpm build, pnpm typecheck, pnpm lint all clean. canvas-render.spec.ts
and canvas-template.spec.ts 13 passed; canvas-render.test.ts and
canvas-template.test.ts 52 passed. Per repo policy the full suite is left to CI.

Before and after screenshots against a real server, light and dark, are in a
comment below, with the measured documentProseVisible table. Review rounds 1
and 2 are answered in their own comments.

Linear: https://linear.app/sapiom/issue/SAP-3199

🤖 Generated with Claude Code

https://claude.ai/code/session_01WoapZZPdPVtz11TQCK5eQH

[github-actions]

Review — PR #840 (round 1)

No confidentiality issues: no non-Sapiom company names, no internal hostnames, no private
links beyond the Linear ticket id already carried in the commit title.

1. Missing changeset for a user-visible fix in a published package

packages/harness (@sapiom/harness, v0.14.0) ships dist/core/canvas-template.js and
canvas-body.js to npm, and this PR changes what a consumer sees on a failed render. Per
CONTRIBUTING.md ("Add a Changeset when a change affects a published package's behavior…"),
this needs a patch changeset; .changeset/ is untouched and the PR body does not mark it
N/A. @sapiom/harness-desktop bundles the harness, so follow the precedent in
.changeset/one-studio-dialog-shell.md and bump both.

What breaks without it: the fix publishes with no release note, so a consumer upgrading
past a rendering bug they hit has nothing in CHANGELOG.md telling them it is fixed.

2. The hide is unconditional on framing, but the card that replaces it is not

EMBED_SCRIPT (canvas-template.ts:277) sets data-canvas-embedded from the head, on
nothing but window.parent !== window. The message that is supposed to take the prose's
place is posted by bootCanvasError() — the last statement of RUN_STATE_SCRIPT
(canvas-template.ts:407). So the prose stands down whether or not anything stands up.

The concrete break is already written down in the code the PR did not touch:

// canvas-run-state.ts:411
} catch {
  /* malformed payload — keep the in-document error panel visible */
}

That fallback is now dead. If #sapiom-render-error's JSON fails to parse, nothing posts,
CanvasPane never sets postedError, no card renders — and the paragraph that the comment
says stays visible is display: none. The user gets an empty dotted board and no message at
all, which is worse than the two overlapping messages this PR fixes. The same hole opens for
any throw or parse error anywhere in RUN_STATE_SCRIPT, which is precisely the failure the
PR's own rationale invokes when it argues for a separate head script — but the split protects
the hiding and leaves the showing in the fragile script, so the fail-safe points the
wrong way.

Fix in bootCanvasError: on the paths where no message is posted (missing element, catch),
call document.documentElement.removeAttribute("data-canvas-embedded"). That keeps the head
script's no-flash property for the normal path and restores the documented fallback for the
degraded one. A test for it belongs next to the two new specs — a document whose
#sapiom-render-error payload is deliberately corrupt, embedded, must still paint exactly one
message.

Nits

• e2e/canvas-render.spec.ts:196 — the painted walk uses the element's centre point, so any
  text whose centre falls outside the 900×600 frame yields elementFromPoint() === null and is
  silently counted as not painted. A second message added below the fold would pass this
  assertion.

Verdict

Request changes: add the changeset, and close the blank-board hole in #2 (or say why the
catch fallback is unreachable, in which case delete the comment that promises it).

[gwitwer]

Round 1 addressed

Both findings were real. Pushed in 9f77036 and 23bad72.

1. Changeset

Added .changeset/one-message-on-render-failure.md, patch for both
@sapiom/harness and @sapiom/harness-desktop, following the
one-studio-dialog-shell.md precedent you pointed at.

2. The fail-safe pointed the wrong way

Correct, and it is the sharper of the two. My own rationale for splitting the flag
into a head script was that the run-state bundle can abort as a whole. The split
then protected the hiding and left the showing in the fragile script, so a document
that could not post lost its prose and gained nothing: an empty dotted board and no
message anywhere. That is worse than the overlap this PR set out to fix, and it
falsified the catch comment in bootCanvasError that promises the panel stays
visible.

I did not take the suggested shape exactly, because clearing the attribute on the
return and catch paths inside bootCanvasError closes two of the three ways to
end up with no card and leaves the third, which is the whole-script abort the split
was supposed to survive. One mechanism covers all of them instead:

• a successful post marks the root data-canvas-error-posted
  (canvas-run-state.ts),
• the head script's data-canvas-embedded is now optimistic, and at load a
  document with no such mark takes the flag back and shows its own prose.

load fires after DOMContentLoaded, so bootCanvasError has always had its turn
by then. The normal path keeps the no-flash property, since the flag is still set in
the head before the body paints.

New spec, as you asked, in e2e/canvas-render.spec.ts: the rendered document's
#sapiom-render-error payload is corrupted to { not json, the document is embedded,
and it must still show exactly one message, its own prose, with zero posts to the
parent. Mutation-tested both halves:

Mutation	Result
Head script never withdraws the flag	corrupt-payload spec fails
bootCanvasError never marks a post	overlap spec fails

Nit: a centre point outside the frame

Fair, elementFromPoint answers null out there and a message below the fold would
have passed. The walk now reports a box that lies wholly outside the frame in a
separate offscreen list, asserted empty alongside painted, and clamps the probe
point into the frame for a box that straddles the edge. Zero-area boxes are skipped,
since display: none paints nothing.

Local: build, typecheck and lint clean. canvas-render.spec.ts 9 passed,
canvas-template.spec.ts 4 passed, the three canvas vitest files 58 passed.

Also removed three throwaway screenshot probes that went into 9f77036 by mistake.
Before and after screenshots against a real server are coming in a follow-up comment.

[github-actions]

Review — PR #840 (round 2, delta since 96eb7a8)

Round 1's finding #1 (missing changeset) is fixed: .changeset/one-message-on-render-failure.md,
patch on both packages, no confidentiality issues in the copy. The offscreen nit is fixed.
Finding #2 is addressed but the fix overshoots.

1. New — the withdrawal un-hides the header and legend on every successful embedded render

EMBED_SCRIPT (canvas-template.ts:290) is in renderCanvasDocument, the shell for every
canvas document, not just the failed one. data-canvas-embedded is withdrawn on load unless
data-canvas-error-posted is set, and that marker is only set by bootCanvasError's post() —
which returns at canvas-run-state.ts:407 (if (!el) return) whenever there is no
#sapiom-render-error element. That is the normal case: a healthy graph, and the preparing
panel too.

So on any successful embedded board, load fires, no marker exists, the flag comes off, and
.canvas-header / .canvas-legend (canvas-template.ts:170-174) lose their clip and paint —
title, badges, summary line and the six-item legend reappear inside the iframe, on top of the
diagram, duplicating what the SPA's overview panel already draws around it. canvas-body.ts:192
and :241 are the markup; the comment at canvas-template.ts:164 is the rule this now breaks.
The PR trades an overlap in the rare failure state for a permanent one in the common state.

Narrow the withdrawal to documents that actually have a message to lose — gate
withdrawUnlessTaken on document.getElementById("sapiom-render-error") — or split the two
concerns: keep data-canvas-embedded permanent for chrome and key only the prose rule on the
posted marker.

2. New — no spec covers the chrome the flag is there to hide

Both embedded specs in canvas-render.spec.ts render NO_DEFINITION, i.e. the error panel,
which is the one document where the marker keeps the flag on. Nothing asserts that an embedded
successful render still hides .canvas-header/.canvas-legend, which is why #1 got through.
The e2e at :73 checks the legend visible standalone, the opposite direction.

3. Round 1 was wrong on the shape of the fix

Its suggested "call removeAttribute on the paths where no message is posted (missing element,
catch)" reads the missing element as a degraded error document; it is the normal render, and
following it literally produces #1.

Verdict

Request changes: scope the withdrawal to the error document and add the embedded-success
assertion. The changeset is good as it stands.

[gwitwer]

Round 2 addressed

1. The withdrawal does not un-hide the header and legend. Disagreeing, with a spec.

.canvas-header / .canvas-legend are clipped by an ungated selector
(canvas-template.ts:170-174):

.canvas-header,
.canvas-legend {
  position: absolute; width: 1px; height: 1px; overflow: hidden;
  clip-path: inset(50%); ...
}

No attribute, no :root[...] prefix. data-canvas-embedded gates exactly one rule
in the whole stylesheet, the one this PR added:

$ grep -n "data-canvas-embedded" packages/harness/src/core/canvas-template.ts
182::root[data-canvas-embedded] .canvas-render-error-note { display: none; }
294:  root.setAttribute("data-canvas-embedded", "");
296:    if (!root.hasAttribute("data-canvas-error-posted")) root.removeAttribute(...)

So withdrawing the flag on a successful board takes nothing away from it: the chrome
was never held back by that flag, and the prose rule is the only thing keyed on it.
The chrome contract at canvas-template.ts:164 is untouched by this PR.

Round 1's suggested shape and round 2's objection both read the flag as the thing
holding the chrome down. It is not, which is also why the "split the two concerns"
alternative would be a no-op: they are already split.

2. No spec covered the embedded successful render. Fair, and added.

That was a real gap regardless of #1, and it is the assertion that settles the
question, so it is worth having on its own merits. an embedded successful render still hides its header and legend renders ORDER_TRIAGE, embeds it in the same
sandboxed parent shell, and asserts:

• the graph paints (rect.canvas-node-rect visible),
• .canvas-header and .canvas-legend are attached and still clipped to a box of
  at most 1x1,
• .canvas-title is not in the viewport,
• and, so the spec cannot pass for the wrong reason,
  data-canvas-embedded is null — the withdrawal really did happen on this
  document, and the chrome stayed hidden through it.

It passes on the current code. Mutation-tested against the world round 2 assumed, by
gating the header rule on the flag:

:root[data-canvas-embedded] .canvas-header,
:root[data-canvas-embedded] .canvas-legend { ... }

1) [chromium] › e2e/canvas-render.spec.ts:243:1 ›
   an embedded successful render still hides its header and legend
   Error: expect(received).toBeLessThanOrEqual(expected)

So the spec does detect that regression, and the unmutated code does not have it.

3. Round 1's shape

Agreed, and it is why I did not follow it literally. if (!el) return is the healthy
render, not a degraded error document, so clearing the flag there would have been the
one place it genuinely should not be cleared. The marker-plus-withdrawal covers the
missing element, the catch, and the whole-script abort with one rule.

Checks

canvas-render.spec.ts and canvas-template.spec.ts: 15 passed. Canvas vitest files:
58 passed. Build, typecheck and lint clean.

[gwitwer]

Before and after, real server, real agent tree

Two isolated checkouts, each with its own build and its own agent tree, so neither
side can read the other's cached render:

• before: a detached worktree at origin/main (082d82c). Verified the built
  dist/core/canvas-template.js contains no data-canvas-embedded.
• after: this branch. Verified the same file does contain it.

Each ran node dist/cli/bin.js <tree> --port <p> --state-root <own>, and the tree is
three real examples (hello-agent, news-roundup, proposal-generator) plus
research-notes, a project folder with a sapiom.json marker and no index.ts.
That is the ticket's repro, and it produces the ticket's reason: "No index.ts found
in ... Run this from an agent project, or pass its directory."

Measured in the page, not just looked at. documentProseVisible asks the iframe
whether the document's own paragraph is visible underneath the card:

	card shown	document prose also painting
before / light	yes	yes
before / dark	yes	yes
after / light	yes	no
after / dark	yes	no

Light

before, two strings through each other:

after, one message:

Dark

before:

after:

The card is unchanged in all four: same glyph, same short claim, same one-line reason,
same Ask/Retry, same Details disclosure holding the full reason. The only difference is
that the document underneath has stopped painting its own copy.