Fix review findings: importer DoS, sync data loss, views/profile rendering, docs
Source importers: lstat before open so FIFOs no longer hang the directory walk; replace the quadratic Rust macro regex with a linear scan. Three-way sync: report page add/add and delete-vs-modify as conflicts instead of silently overwriting or resurrecting; canonical comparison so key order is not a change; the sync action withholds output and fails on conflicts unless --force. Story publishing escapes <, >, & and U+2028/9 inside the embedded JSON. Views: drop parentId of unselected containers, re-layout instead of manual geometry, validate before serialising; flatten() merges identical nodes repeated across pages so C4 output works with every analysis action. Dark theme edge labels get a background; tube-map corridors sit above the stations; C4 containers keep the swimlane style and orphan relationships land on the matching page; router keeps container header bands as obstacles; sequence self-messages loop on one side. Docs: SKILL.md lists all 23 CLI actions and how each capability family is invoked, task wrappers for query/test/what-if/doctor, capability tables and --page scope corrected, maintenance snippet uses the real synchronous action signature, duplicated rule bullets moved to the rule references, coverage matrix wording made verifiable. Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
Claude Code
@@ -1,10 +1,14 @@
|
||||
import { diagramIRToDrawio } from "../../authoring/ir-to-drawio.js";
|
||||
import { assertOutputSafe, atomicWrite, loadIR, structuredText, type LifecycleActionOptions } from "../../services/semantic-lifecycle/lifecycle-io.js";
|
||||
import { syncDiagramIR } from "../../services/semantic-lifecycle/sync.js";
|
||||
export function run(filePath: string, _page = 0, outputPath?: string, options: LifecycleActionOptions = {}): Record<string, unknown> {
|
||||
export type SyncActionOptions = LifecycleActionOptions & { force?: boolean };
|
||||
export function run(filePath: string, _page = 0, outputPath?: string, options: SyncActionOptions = {}): Record<string, unknown> {
|
||||
if (!options.base || !options.spec) throw new Error("sync requires --base <base> and --spec <incoming>"); if (!options.dryRun && !outputPath) throw new Error("sync requires --output unless --dry-run is used");
|
||||
if (outputPath) assertOutputSafe(outputPath, [filePath, options.base, options.spec]);
|
||||
const result = syncDiagramIR(loadIR(options.base), loadIR(filePath), loadIR(options.spec), { prune: options.prune });
|
||||
let output: string | undefined; if (!options.dryRun && outputPath) output = atomicWrite(outputPath, /\.(drawio|xml)$/i.test(outputPath) ? diagramIRToDrawio(result.ir) : structuredText(result.ir, outputPath), [filePath, options.base, options.spec]);
|
||||
return { action: "sync", output, dryRun: options.dryRun === true, added: result.added, removed: result.removed, conflicts: result.conflicts, preview: options.dryRun ? result.ir : undefined };
|
||||
// Unresolved conflicts fail the action (exit code 1) and block writing unless --force accepts the manual-preferred merge.
|
||||
const failed = result.conflicts.length > 0 && options.force !== true;
|
||||
const summary = result.conflicts.length === 0 ? "sync merged without conflicts" : failed ? `${result.conflicts.length} unresolved conflict(s); output not written (resolve them or pass --force to write the manual-preferred merge)` : `${result.conflicts.length} conflict(s) overridden by --force; manual values kept`;
|
||||
let output: string | undefined; if (!options.dryRun && outputPath && !failed) output = atomicWrite(outputPath, /\.(drawio|xml)$/i.test(outputPath) ? diagramIRToDrawio(result.ir) : structuredText(result.ir, outputPath), [filePath, options.base, options.spec]);
|
||||
return { action: "sync", output, dryRun: options.dryRun === true, failed, summary, added: result.added, removed: result.removed, conflicts: result.conflicts, preview: options.dryRun ? result.ir : undefined };
|
||||
}
|
||||
|
||||
@@ -0,0 +1,80 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import test from "node:test";
|
||||
|
||||
import { loadGraphStates } from "../../services/maxgraph-loader/graph-loader.js";
|
||||
import { parseAllPages } from "../../services/drawio-parser/parser.js";
|
||||
import { projectC4 } from "../../services/profiles/c4.js";
|
||||
import { run as validate } from "../validate/action.js";
|
||||
import { run as views } from "./action.js";
|
||||
|
||||
const SPEC = `version: 2
|
||||
pages:
|
||||
- id: main
|
||||
title: Main
|
||||
layout: { type: layered }
|
||||
nodes:
|
||||
- { id: zone, label: Zone, kind: container }
|
||||
- { id: api, label: API, kind: service, parentId: zone, properties: { importance: 9 } }
|
||||
- { id: worker, label: Worker, kind: service, parentId: zone, properties: { importance: 8 } }
|
||||
- { id: db, label: DB, kind: database, parentId: zone, properties: { importance: 7 } }
|
||||
- { id: user, label: User, kind: actor, properties: { importance: 10 } }
|
||||
edges:
|
||||
- { id: e1, source: user, target: api }
|
||||
- { id: e2, source: api, target: worker }
|
||||
- { id: e3, source: api, target: db, kind: write }
|
||||
`;
|
||||
|
||||
function pageVertexIds(file: string, pageName: string): Set<string> {
|
||||
const page = parseAllPages(file).find((item) => item.pageName === pageName)!;
|
||||
return new Set(loadGraphStates(page.graphModelXml).vertexBounds.keys());
|
||||
}
|
||||
|
||||
test("views drop parents that were projected away so every selected vertex renders", () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "drawio-views-test-"));
|
||||
const spec = join(dir, "spec.yaml");
|
||||
const output = join(dir, "views.drawio");
|
||||
try {
|
||||
writeFileSync(spec, SPEC, "utf8");
|
||||
// The security view selects user (actor) and db (database) but not their container "zone".
|
||||
const result = views(spec, 0, output, { views: "security,dataflow" }) as { views: Array<{ id: string; fallback: boolean }> };
|
||||
assert.deepEqual(result.views.map((view) => view.id), ["security", "dataflow"]);
|
||||
assert.deepEqual(validate(output).summary, { pages: 2, valid: true, invalidPages: 0 });
|
||||
assert.deepEqual([...pageVertexIds(output, "Security")].sort(), ["db", "user"]);
|
||||
assert.deepEqual([...pageVertexIds(output, "Dataflow")].sort(), ["api", "db"]);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("views render C4 profile output whose elements span several pages", () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "drawio-views-c4-test-"));
|
||||
const source = join(dir, "c4.yaml");
|
||||
const output = join(dir, "views.drawio");
|
||||
try {
|
||||
const c4 = projectC4({
|
||||
title: "Shop",
|
||||
elements: [
|
||||
{ id: "customer", label: "Customer", type: "person" },
|
||||
{ id: "shop", label: "Shop System", type: "system" },
|
||||
{ id: "web", label: "Web App", type: "container", parentId: "shop" },
|
||||
{ id: "api", label: "API", type: "container", parentId: "shop" },
|
||||
{ id: "ctrl", label: "Controller", type: "component", parentId: "api" },
|
||||
],
|
||||
relationships: [
|
||||
{ id: "r1", source: "customer", target: "web", label: "uses" },
|
||||
{ id: "r2", source: "web", target: "api", label: "calls" },
|
||||
{ id: "r3", source: "customer", target: "shop", label: "shops" },
|
||||
],
|
||||
});
|
||||
writeFileSync(source, JSON.stringify(c4), "utf8");
|
||||
views(source, 0, output, { views: "system,executive" });
|
||||
assert.deepEqual(validate(output).summary, { pages: 2, valid: true, invalidPages: 0 });
|
||||
assert.deepEqual([...pageVertexIds(output, "System")].sort(), ["api", "ctrl", "customer", "shop", "web"]);
|
||||
assert.deepEqual([...pageVertexIds(output, "Executive")].sort(), ["api", "ctrl", "customer", "shop", "web"]);
|
||||
} finally {
|
||||
rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
@@ -1,5 +1,6 @@
|
||||
import { diagramIRToDrawio } from "../../authoring/ir-to-drawio.js";
|
||||
import type { DiagramIRV2 } from "../../model/diagram-ir.js";
|
||||
import type { DiagramIRV2, DiagramNode } from "../../model/diagram-ir.js";
|
||||
import { validateDiagramIR } from "../../model/diagram-ir.js";
|
||||
import { projectLinkedViews, type ViewName } from "../../services/semantic-lifecycle/analysis.js";
|
||||
import { atomicWrite, loadIR, type LifecycleActionOptions } from "../../services/semantic-lifecycle/lifecycle-io.js";
|
||||
export function run(filePath: string, _page = 0, outputPath?: string, options: LifecycleActionOptions = {}): Record<string, unknown> {
|
||||
@@ -10,7 +11,14 @@ export function run(filePath: string, _page = 0, outputPath?: string, options: L
|
||||
const unknown = names?.filter((name) => !allowed.has(name)) ?? [];
|
||||
if (unknown.length) throw new Error(`Unknown linked view: ${unknown.join(", ")}`);
|
||||
const views = projectLinkedViews(source, names);
|
||||
const ir: DiagramIRV2 = { version: 2, title: source.title, provenance: source.provenance, pages: views.map((view) => ({ id: view.id, title: view.title, nodes: view.nodes, edges: view.edges, layout: { type: "manual" }, properties: { linkedView: true, sourcePageIds: view.sourcePageIds, fallback: view.fallback, fallbackReason: view.fallbackReason, hint: view.hint } })) };
|
||||
// A view unions nodes from several source pages, so their page-relative positions cannot coexist: keep only
|
||||
// the sizes and let the layered layout place everything (waypoints are dropped for the same reason).
|
||||
const relayout = (node: DiagramNode): DiagramNode => {
|
||||
const { geometry, ...rest } = node;
|
||||
return geometry ? { ...rest, width: geometry.width, height: geometry.height } : rest;
|
||||
};
|
||||
const ir: DiagramIRV2 = { version: 2, title: source.title, provenance: source.provenance, pages: views.map((view) => ({ id: view.id, title: view.title, nodes: view.nodes.map(relayout), edges: view.edges.map(({ waypoints: _waypoints, ...edge }) => edge), layout: { type: "layered" }, properties: { linkedView: true, sourcePageIds: view.sourcePageIds, fallback: view.fallback, fallbackReason: view.fallbackReason, hint: view.hint } })) };
|
||||
validateDiagramIR(ir);
|
||||
const output = atomicWrite(outputPath, diagramIRToDrawio(ir), [filePath]);
|
||||
return { action: "views", output, views: views.map(({ id, fallback, fallbackReason, hint }) => ({ id, fallback, fallbackReason, hint })) };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user