Address review comments on the schema-derived data model

* flowToBlueprint dropped scope_end when it was an empty string, contradicting
  the round-trip guarantee the module documents. Preserve it whenever defined,
  matching the scope_start handling beside it.
* Move getLayoutedElements out of BlueprintCanvas into a UI-free
  utils/flow-layout, so the serialization layer no longer pulls React, CSS and
  browser globals into its module graph.
* update-blueprint-schema opened (and truncated) the destination before
  checking the status code, then unlinked it on failure, destroying the
  committed schema the fallback path depends on. Download to a temp file and
  rename into place only once the transfer completes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: chenkasirer

scripts/update-blueprint-schema.js

@@ -31,31 +31,51 @@ if (!fs.existsSync(SCHEMA_DIR)) {
     fs.mkdirSync(SCHEMA_DIR, { recursive: true });
 }
 
+// Downloads to a temporary file and only moves it into place once the transfer
+// has completed. A failed download must never clobber the committed schema —
+// that copy is the fallback the build relies on when the remote is unreachable.
 const downloadUrl = (url, destPath) => {
     return new Promise((resolve, reject) => {
-        const file = fs.createWriteStream(destPath);
+        const tmpPath = `${destPath}.download`;
         const options = { headers: { 'User-Agent': 'Node.js' } };
         if (process.env.GITHUB_TOKEN) {
             options.headers['Authorization'] = `token ${process.env.GITHUB_TOKEN}`;
         }
+
+        const failWith = (err, file) => {
+            if (file) {
+                file.destroy();
+                fs.unlink(tmpPath, () => { });
+            }
+            reject(err);
+        };
+
         https
             .get(url, options, (response) => {
                 if (response.statusCode !== 200) {
-                    fs.unlink(destPath, () => { });
-                    reject(new Error(`Failed to download ${url}: ${response.statusCode}`));
+                    response.resume();
+                    failWith(new Error(`Failed to download ${url}: ${response.statusCode}`));
                     return;
                 }
+
+                const file = fs.createWriteStream(tmpPath);
+                file.on('error', (err) => failWith(err, file));
+                response.on('error', (err) => failWith(err, file));
+
                 response.pipe(file);
                 file.on('finish', () => {
-                    file.close();
-                    console.log(`Downloaded ${path.basename(destPath)}`);
-                    resolve();
+                    file.close((err) => {
+                        if (err) {
+                            failWith(err, file);
+                            return;
+                        }
+                        fs.renameSync(tmpPath, destPath);
+                        console.log(`Downloaded ${path.basename(destPath)}`);
+                        resolve();
+                    });
                 });
             })
-            .on('error', (err) => {
-                fs.unlink(destPath, () => { });
-                reject(err);
-            });
+            .on('error', (err) => failWith(err));
     });
 };
 

src/components/author/BlueprintCanvas.tsx

@@ -22,12 +22,9 @@ import type {
   OnSelectionChangeParams,
   EdgeProps,
 } from '@xyflow/react';
-import dagre from '@dagrejs/dagre';
 import '@xyflow/react/dist/style.css';
 import { AuthorTaskNode } from './AuthorTaskNode';
-
-export const NODE_WIDTH = 240;
-export const NODE_HEIGHT = 100;
+import { NODE_WIDTH } from '../../utils/flow-layout';
 
 const nodeTypes = { authorTask: AuthorTaskNode };
 
@@ -108,32 +105,6 @@ function DeletableEdge({
 
 const edgeTypes = { deletable: DeletableEdge };
 
-export function getLayoutedElements(nodes: Node[], edges: Edge[]) {
-  const g = new dagre.graphlib.Graph();
-  g.setDefaultEdgeLabel(() => ({}));
-  g.setGraph({ rankdir: 'LR', ranksep: 160, nodesep: 60 });
-
-  nodes.forEach((n) => g.setNode(n.id, { width: NODE_WIDTH, height: NODE_HEIGHT }));
-  edges.forEach((e) => g.setEdge(e.source, e.target));
-  dagre.layout(g);
-
-  return {
-    nodes: nodes.map((n) => {
-      const pos = g.node(n.id);
-      return {
-        ...n,
-        targetPosition: Position.Left,
-        sourcePosition: Position.Right,
-        position: {
-          x: pos.x - NODE_WIDTH / 2,
-          y: pos.y - NODE_HEIGHT / 2,
-        },
-      };
-    }),
-    edges,
-  };
-}
-
 interface BlueprintCanvasProps {
   nodes: Node[];
   edges: Edge[];

src/utils/blueprint-flow.ts

@@ -1,6 +1,6 @@
 import { Position, MarkerType } from '@xyflow/react';
 import type { Node, Edge } from '@xyflow/react';
-import { getLayoutedElements } from '../components/author/BlueprintCanvas';
+import { getLayoutedElements } from './flow-layout';
 import type {
   AuthorNodeData,
   Blueprint,
@@ -87,10 +87,12 @@ export function flowToBlueprint(
     if (params.length) task.params = params;
     if (depends_on.length) task.depends_on = depends_on;
 
-    // Round-trip scope boundaries. scope_start may legitimately be an empty
-    // object (a skip-policy scope), so preserve it whenever it is defined.
+    // Round-trip scope boundaries. Both are preserved whenever defined rather
+    // than when truthy: scope_start may legitimately be an empty object (a
+    // skip-policy scope), and an empty scope_end is a value the schema accepts,
+    // so dropping either would break the round-trip guarantee above.
     if (d.scopeStart !== undefined) task.scope_start = d.scopeStart;
-    if (d.scopeEnd) task.scope_end = d.scopeEnd;
+    if (d.scopeEnd !== undefined) task.scope_end = d.scopeEnd;
 
     return task;
   });

src/utils/flow-layout.ts

@@ -0,0 +1,38 @@
+import { Position } from '@xyflow/react';
+import type { Node, Edge } from '@xyflow/react';
+import dagre from '@dagrejs/dagre';
+
+/**
+ * Dagre layout for the author canvas, kept UI-free so the serialization layer
+ * (and its tests) can lay out a graph without pulling in React, CSS or the
+ * browser globals that the canvas component needs.
+ */
+
+export const NODE_WIDTH = 240;
+export const NODE_HEIGHT = 100;
+
+export function getLayoutedElements(nodes: Node[], edges: Edge[]) {
+  const g = new dagre.graphlib.Graph();
+  g.setDefaultEdgeLabel(() => ({}));
+  g.setGraph({ rankdir: 'LR', ranksep: 160, nodesep: 60 });
+
+  nodes.forEach((n) => g.setNode(n.id, { width: NODE_WIDTH, height: NODE_HEIGHT }));
+  edges.forEach((e) => g.setEdge(e.source, e.target));
+  dagre.layout(g);
+
+  return {
+    nodes: nodes.map((n) => {
+      const pos = g.node(n.id);
+      return {
+        ...n,
+        targetPosition: Position.Left,
+        sourcePosition: Position.Right,
+        position: {
+          x: pos.x - NODE_WIDTH / 2,
+          y: pos.y - NODE_HEIGHT / 2,
+        },
+      };
+    }),
+    edges,
+  };
+}