A self-served URL app composes, and a cross-origin target stays spatial-only

review.r2 contract audit — the last two Important findings.

I2 — the installable-app surface.url branch had no composition path. The app
contract defines surface.url for a self-served app, mutually exclusive with path
and with no entry to copy. Composition unconditionally joined m.surface.entry, so
a schema-valid URL app could not compose — it was reported missing or threw. It
now branches: a url app emits a URL row and copies nothing (it serves itself),
and may still declare a provider and calls. Control: a surface.url app composes
to a row carrying the url and no path.

I3 — a cross-origin annotation target produced a FALSE anchor. On an unreadable
cross-origin nested target the shell fell back to the outer surface document, and
the annotation layer resolved the record's selector against it — so a host and
its nested doc both carrying `#publish` would anchor to the wrong one, the exact
false anchor the contract says it prevents. The shell now returns a spatial-only
view for a cross-origin declared target (a rect, no readable doc) instead of
falling back to the surface document, and the layer's view() honours spatial-only
without falling back while elementAnchor refuses a doc-less view. A cross-origin
surface stays spatial-only, as the contract requires. Structural controls updated
(the fallback rename, the spatialOnly branch, the doc guard) with the positive
control proving they can fail; the pixel-level behaviour remains browser-verified,
as this repo has no browser harness.

365 -> 366 tests, 366/366 green. Audit round complete: all Criticals, Importants,
and Minors fixed (I4 excluded — PM's lane).

app/annotations.js

@@ -124,7 +124,14 @@ export function attach(shell, { store, persistence = null } = {}) {
   // frame against a shell that predates the seam.
   const view = () => {
     if (typeof shell.target === "function") {
-      try { const v = shell.target(); if (v?.doc && v?.rect) return v; } catch {}
+      let v = null;
+      try { v = shell.target(); } catch { v = null; }
+      // A cross-origin declared target is SPATIAL-ONLY: a rect but no readable
+      // doc. Return it as-is and do NOT fall back to the surface document — the
+      // record's selector was meant for the cross-origin frame, and matching it
+      // against the outer host is the false anchor the contract forbids.
+      if (v?.spatialOnly && v?.rect) return { doc: null, rect: v.rect };
+      if (v?.doc && v?.rect) return v;
     }
     const frame = active().frame;
     try { return frame?.contentDocument ? { doc: frame.contentDocument, rect: frame.getBoundingClientRect() } : null; }
@@ -142,7 +149,10 @@ export function attach(shell, { store, persistence = null } = {}) {
   };
   const elementAnchor = (selector) => {
     const v = view();
-    if (!v || !selector) return null;
+    // No readable doc (a cross-origin spatial-only target) means NO element
+    // anchoring: the selector stays unresolved and the mark is spatial/missing,
+    // never falsely anchored to the wrong document.
+    if (!v || !v.doc || !selector) return null;
     let element;
     try { element = v.doc.querySelector(selector); } catch { return null; }
     if (!element) return null;

app/shell.html

@@ -781,21 +781,33 @@
       const outer = frames.get(activeId);
       if (!outer) return null;
       const outerRect = outer.getBoundingClientRect();
-      const fallback = () => {
+      // The surface's OWN document, element-anchorable. Used when no nested
+      // target is declared, or the declared one is not a frame.
+      const surfaceDoc = () => {
         try { return outer.contentDocument ? { doc: outer.contentDocument, rect: outerRect } : null; }
         catch { return null; }
       };
       const frameId = declaredTargets.get(activeId) ?? null;
-      if (!frameId) return fallback();
+      if (!frameId) return surfaceDoc();
       try {
         const nested = outer.contentDocument?.getElementById(frameId);
-        const doc = nested && nested.tagName === "IFRAME" ? nested.contentDocument : null;
-        if (!doc) return fallback();
-        const innerRect = nested.getBoundingClientRect();
-        return { doc, rect: {
-          left: outerRect.left + innerRect.left, top: outerRect.top + innerRect.top,
-          width: innerRect.width, height: innerRect.height } };
-      } catch { return fallback(); }
+        if (!nested || nested.tagName !== "IFRAME") return surfaceDoc();
+        let doc = null;
+        try { doc = nested.contentDocument; } catch { doc = null; }
+        if (doc) {
+          const innerRect = nested.getBoundingClientRect();
+          return { doc, rect: {
+            left: outerRect.left + innerRect.left, top: outerRect.top + innerRect.top,
+            width: innerRect.width, height: innerRect.height } };
+        }
+        // CROSS-ORIGIN declared target: SPATIAL-ONLY. Its document cannot be
+        // read for element identity, so it must NOT fall back to the surface
+        // document — a record's selector was meant for the cross-origin frame,
+        // and resolving it against the outer host is the exact false anchor the
+        // contract forbids (a host and its nested doc both carrying `#publish`
+        // would anchor to the wrong one). A rect for spatial marks, no doc.
+        return { spatialOnly: true, rect: outerRect };
+      } catch { return surfaceDoc(); }
     },
     scope: () => {
       const context = declaredContexts.get(activeId) ?? null;

test/annotations.test.mjs

@@ -232,8 +232,21 @@ test("the nested-frame target seam is wired, guarded, and falls back — STRUCTU
     "target() does not add the outer frame offset — nested anchors will be off by the surface frame's position");
   assert.match(target[1], /catch/,
     "target() does not guard the cross-origin read; a cross-origin nested frame would throw instead of degrading");
-  assert.match(target[1], /fallback\(\)/,
-    "target() has no fallback to the surface frame");
+  assert.match(target[1], /surfaceDoc\(\)/,
+    "target() has no fallback to the surface frame for the no-target / not-a-frame cases");
+  // review.r2 IMPORTANT — the cross-origin FALSE ANCHOR. A cross-origin declared
+  // target must degrade SPATIAL-ONLY (a rect, no readable doc), NOT fall back to
+  // the surface document: resolving a record's selector against the outer host
+  // when it was meant for the cross-origin frame is a false anchor (host and
+  // nested doc both carrying `#publish`). target() returns spatialOnly for that
+  // case, and the layer refuses to element-anchor a doc-less view.
+  assert.match(target[1], /spatialOnly:\s*true/,
+    "a cross-origin declared target still falls back to the surface document — the false anchor the contract forbids");
+  const layer = fs.readFileSync(path.join(REPO, "app", "annotations.js"), "utf8");
+  assert.match(layer, /spatialOnly[\s\S]*return \{ doc: null/,
+    "the layer's view() does not honour spatialOnly — it would fall back to the surface document");
+  assert.match(layer, /if \(!v \|\| !v\.doc \|\| !selector\) return null/,
+    "elementAnchor no longer refuses a doc-less (spatial-only) view — a cross-origin selector could resolve");
 });
 
 test("the layer anchors through the shell's view, not the surface frame directly", () => {
@@ -265,12 +278,12 @@ test("positive control: the two checks above can fail", () => {
     "the offset survived being removed, so that assertion would pass with the chain walk gone");
 
   const layer = fs.readFileSync(path.join(REPO, "app", "annotations.js"), "utf8");
-  const regressed = layer.replace(/const v = view\(\);\n    if \(!v \|\| !selector\) return null;/,
-    "const v = { doc: active().frame.contentDocument, rect: active().frame.getBoundingClientRect() };\n    if (!v || !selector) return null;");
-  const a = regressed.match(/const elementAnchor = \(selector\) => \{([\s\S]*?)\n  \};/);
-  assert.ok(a, "the mutation broke the elementAnchor extractor");
-  assert.match(a[1], /active\(\)\.frame/,
-    "the regression could not be planted, so the doesNotMatch assertion proves nothing");
+  // Regress the cross-origin doc-guard back to the pre-fix form (dropping the
+  // v.doc check), and prove the assertion that requires it would then fail.
+  const regressed = layer.replace("if (!v || !v.doc || !selector) return null;", "if (!v || !selector) return null;");
+  assert.notEqual(regressed, layer, "the v.doc guard could not be located to mutate — the assertion proves nothing");
+  assert.doesNotMatch(regressed, /if \(!v \|\| !v\.doc \|\| !selector\) return null/,
+    "the v.doc guard survived removal, so the doc-guard assertion above cannot fail");
 });
 
 test("the refresh hook re-measures WITHOUT reloading — STRUCTURAL", () => {

test/compose-rail.test.mjs

@@ -277,6 +277,22 @@ test("a malformed provider.json REFUSES rather than silently falling back", () =
     "a broken provider declaration quietly fell back to the app's copy");
 });
 
+test("I2: a self-served surface.url app composes as a URL row, with no entry to copy", () => {
+  // review.r2 IMPORTANT: the app contract defines surface.url for a self-served
+  // app, mutually exclusive with path and with no entry. Composition
+  // unconditionally joined m.surface.entry, so a schema-valid URL app could not
+  // compose at all — it either threw or was reported missing. It must produce a
+  // URL row and copy nothing.
+  const t = tree({ apps: { remote: { surface: { url: "https://example.test/app", glyph: "◆", hint: "remote" } } } });
+  const out = t.run(["remote"]);
+
+  assert.deepEqual(out.missing, [], `a schema-valid URL app failed to compose: ${out.missing.join(", ")}`);
+  const row = out.rows.find((r) => r.id === "remote");
+  assert.ok(row, "the URL app produced no rail row");
+  assert.equal(row.url, "https://example.test/app", "the URL row does not carry the self-served url");
+  assert.ok(!("path" in row), "a URL app emitted a path row — url and path are mutually exclusive");
+});
+
 test("positive control: this suite is capable of failing", () => {
   assert.throws(() => assert.equal(1, 2));
 });

tools/compose-rail.mjs

@@ -112,28 +112,38 @@ export function composeRail({ appsRoot, enabled, doors = [], runtimeDir, studioR
     if (!fs.existsSync(manifestPath)) { missing.push(`${id}: no app.json at ${appDir}`); continue; }
 
     const m = readJson(manifestPath);
-    const entry = path.join(appDir, m.surface.entry);
-    if (!fs.existsSync(entry)) { missing.push(`${id}: surface entry missing (${m.surface.entry})`); continue; }
+    // A SELF-SERVED app (`surface.url`) has no `entry` to copy and no overlay
+    // `path` — something else already serves it, and the contract makes `url` and
+    // `path` mutually exclusive (app-manifest.md). Composition emitted only a
+    // path row and unconditionally joined `m.surface.entry`, so a schema-valid
+    // URL app could not compose at all (review.r2). It gets a URL row and skips
+    // the copy; it may still declare a provider and calls below.
+    if (typeof m.surface.url === "string" && m.surface.url) {
+      rows.push({ id: m.id, name: m.name, glyph: m.surface.glyph, url: m.surface.url, hint: m.surface.hint });
+    } else {
+      const entry = path.join(appDir, m.surface.entry);
+      if (!fs.existsSync(entry)) { missing.push(`${id}: surface entry missing (${m.surface.entry})`); continue; }
 
-    // The seam resolves /surfaces/<file> to <overlay>/<file>, flat.
-    fs.copyFileSync(entry, path.join(surfacesOut, path.basename(m.surface.path)));
+      // The seam resolves /surfaces/<file> to <overlay>/<file>, flat.
+      fs.copyFileSync(entry, path.join(surfacesOut, path.basename(m.surface.path)));
 
-    // An app's vendored assets travel with it. canvas ships 3.4MB of tldraw
-    // deliberately so the box never calls a CDN.
-    const appVendor = path.join(appDir, "vendor");
-    if (fs.existsSync(appVendor)) {
-      for (const entryName of fs.readdirSync(appVendor)) {
-        fs.cpSync(path.join(appVendor, entryName), path.join(vendorOut, entryName), { recursive: true });
+      // An app's vendored assets travel with it. canvas ships 3.4MB of tldraw
+      // deliberately so the box never calls a CDN.
+      const appVendor = path.join(appDir, "vendor");
+      if (fs.existsSync(appVendor)) {
+        for (const entryName of fs.readdirSync(appVendor)) {
+          fs.cpSync(path.join(appVendor, entryName), path.join(vendorOut, entryName), { recursive: true });
+        }
       }
-    }
 
-    rows.push({
-      id: m.id,
-      name: m.name,
-      glyph: m.surface.glyph,
-      path: m.surface.path,
-      hint: m.surface.hint,
-    });
+      rows.push({
+        id: m.id,
+        name: m.name,
+        glyph: m.surface.glyph,
+        path: m.surface.path,
+        hint: m.surface.hint,
+      });
+    }
     // Carry the provider's RUN SPEC, not just its name. The box used to know
     // each provider by name and hardcode how to start it, which meant a sixth
     // app could not run without editing box code — the opposite of installable.