fix(framework): close the architecture guard's three recorded gaps

plugins/aidd-context/skills/04-skill-generate/references/skill-authoring.md

@@ -14,7 +14,7 @@ The contract every generated skill satisfies. `skill-generate` obeys it too.
 
 - **R6.** The router holds the flow, the action table, and the transversal rules. Nothing else.
 - **R7.** The mermaid flow shows every path a run can take: the nominal chain, one entry node per case, a back-edge per loop, a terminal node per outcome. A branch stated in prose is a branch missing from the flow.
-- **R8.** The action table is `| Action | Does |`, one row per action file, in run order. `Action` is the bare slug, no backticks and no number. `Does` is a lowercase imperative half-line with no final period. Above the table, one sentence: what to read next, nothing more.
+- **R8.** The action table is `| Action | Does |`, one row per action file, in run order. `Action` is the bare slug, no backticks and no number; two actions sharing a slug keep their number (`01-plan`, `04-plan`). `Does` is a lowercase imperative half-line with no final period. Above the table, one sentence: what to read next, nothing more.
 - **R9.** `## Transversal rules` holds the rules no single action or reference owns. A rule stated there is stated nowhere else.
 - **R10.** The router is loaded on every call, an action only when its turn comes: the router carries nothing an action or a reference could carry.
 

scripts/__tests__/architecture-rules.property.test.js

@@ -134,24 +134,41 @@ function describe(shape, omit) {
   }`;
 }
 
-test("every shape that cites all its actions is silent", () => {
+/** A blank line inside the router table is built only when the row it precedes is kept. */
+function isCut(shape, omit) {
+  return shape.splitAfter !== undefined && omit !== NAMES[shape.splitAfter];
+}
+
+function refusedOnceAtTheCut(violations) {
+  return violations.length === 1 && /no separator/.test(violations[0].message);
+}
+
+test("every unsplit shape that cites all its actions is silent, and every split one is refused at the cut", () => {
   const failures = [];
   for (const shape of shapes()) {
     const content = buildSkill({ ...shape, names: NAMES, omit: null });
     const violations = checkRouterCoherence(FILE, content, NAMES);
+    if (isCut(shape, null)) {
+      if (!refusedOnceAtTheCut(violations)) failures.push(`${describe(shape)} => ${violations.length} violations`);
+      continue;
+    }
     if (violations.length !== 0) {
       failures.push(`${describe(shape)} => ${violations.map((v) => v.message).join(" | ")}`);
     }
   }
   assert.deepEqual(failures, [], `a router shape was refused although it cites every action:\n${failures.join("\n")}`);
 });
 
-test("every shape that drops one citation yields exactly that one violation", () => {
+test("every unsplit shape that drops one citation yields exactly that one violation", () => {
   const failures = [];
   for (const shape of shapes()) {
     for (const omit of NAMES) {
       const content = buildSkill({ ...shape, names: NAMES, omit });
       const violations = checkRouterCoherence(FILE, content, NAMES);
+      if (isCut(shape, omit)) {
+        if (!refusedOnceAtTheCut(violations)) failures.push(`${describe(shape, omit)} => ${violations.length} violations`);
+        continue;
+      }
       if (violations.length !== 1) {
         failures.push(`${describe(shape, omit)} => ${violations.length} violations`);
         continue;

scripts/__tests__/architecture-rules.test.js

@@ -303,7 +303,7 @@ test("a glossary table before the router does not blind the router's own column"
   assert.deepEqual(violations, []);
 });
 
-test("a blank line splitting the router table does not unname the rows below it", () => {
+test("a blank line splitting the router table is refused once, at the rows it cut off", () => {
   const content = [
     "# Split table skill",
     "",
@@ -324,7 +324,61 @@ test("a blank line splitting the router table does not unname the rows below it"
     content,
     ["01-first.md", "02-second.md"]
   );
-  assert.deepEqual(violations, []);
+  assert.equal(violations.length, 1);
+  assert.match(violations[0].message, /:9 .*no separator/);
+});
+
+test("a glossary with no separator under the router cites nothing", () => {
+  const content = [
+    "# Glossary skill",
+    "",
+    "## Actions",
+    "",
+    "| Action | When |",
+    "| --- | --- |",
+    "| 01-keep | a |",
+    "",
+    "| role | the caller |",
+    "| scope | the ask |",
+    "",
+    "## Transversal rules",
+    "",
+    "- Nothing.",
+  ].join("\n");
+  const violations = checkArchitecture(
+    "plugins/aidd-fixture-a/skills/01-glossary/SKILL.md",
+    content,
+    ["01-keep.md", "02-role.md"]
+  );
+  assert.equal(violations.length, 1);
+  assert.match(violations[0].message, /:9 .*no separator/);
+});
+
+test("a glossary under the router with its own separator leaves a deleted row unnamed", () => {
+  const content = [
+    "# Glossary skill",
+    "",
+    "## Actions",
+    "",
+    "| Action | When |",
+    "| --- | --- |",
+    "| 01-keep | a |",
+    "",
+    "| Term | Meaning |",
+    "| --- | --- |",
+    "| role | the caller |",
+    "",
+    "## Transversal rules",
+    "",
+    "- Nothing.",
+  ].join("\n");
+  const violations = checkArchitecture(
+    "plugins/aidd-fixture-a/skills/01-glossary-table/SKILL.md",
+    content,
+    ["01-keep.md", "02-role.md"]
+  );
+  assert.equal(violations.length, 1);
+  assert.match(violations[0].message, /never names action file "02-role\.md"/);
 });
 
 test("a two-dash separator row still marks the header it follows", () => {
@@ -395,6 +449,42 @@ test("two action files sharing a stem are not both covered by one citation", ()
   assert.match(violations[1].message, /never names action file "04-plan\.md"/);
 });
 
+test("two action files sharing a stem are each cited by their numbered name", () => {
+  const content = [
+    "# Shared stem skill",
+    "",
+    "## Actions",
+    "",
+    "| Action | Does |",
+    "| --- | --- |",
+    "| 01-plan | draft it |",
+    "| 04-plan | revise it |",
+    "",
+    "## Transversal rules",
+    "",
+    "- Nothing.",
+  ].join("\n");
+  const violations = checkArchitecture(
+    "plugins/aidd-fixture-a/skills/01-shared-stem-numbered/SKILL.md",
+    content,
+    ["01-plan.md", "04-plan.md"]
+  );
+  assert.deepEqual(violations, []);
+});
+
+test("a sibling address inside a fence is still refused, unlike a fenced router table", () => {
+  const content = [
+    "# Fenced address",
+    "",
+    "```shell",
+    "/aidd-fixture-b:01-thing",
+    "```",
+  ].join("\n");
+  const violations = checkArchitecture("plugins/aidd-fixture-a/skills/01-x/references/teach.md", content, []);
+  assert.equal(violations.length, 1);
+  assert.match(violations[0].message, /:4 addresses sibling plugin "aidd-fixture-b"/);
+});
+
 test("a fenced example table inside the Actions section cites nothing", () => {
   const content = [
     "# Fenced example skill",

scripts/lib/architecture-rules.js

@@ -102,6 +102,7 @@ function checkOrthogonality(filePath, content) {
   const lines = toLines(content);
   const exemptLines = surface.kind === "agent" ? permissionListLines(lines) : new Set();
 
+  // Unlike router coherence, read through fences: a fenced command is still one the model runs.
   return addressesIn(content)
     .filter(({ plugin, line }) => plugin !== surface.owner && !exemptLines.has(line))
     .map(({ plugin, line, text }) =>
@@ -166,22 +167,24 @@ function tableCells(line) {
   return line.trim().replace(/^\|/, "").replace(/\|$/, "").split("|").map((cell) => cell.trim());
 }
 
-/** Contiguous runs of table rows: one table's header never speaks for the next one's columns. */
+/** Contiguous runs of table rows with the index of their first line: one table's header never speaks for the next one's columns. */
 function tableBlocks(lines) {
   const blocks = [];
   let current = [];
+  let start = 0;
 
-  for (const line of lines) {
+  lines.forEach((line, index) => {
     if (TABLE_ROW.test(line)) {
+      if (current.length === 0) start = index;
       current.push(line);
-      continue;
+      return;
     }
     if (current.length > 0) {
-      blocks.push(current);
+      blocks.push({ start, lines: current });
       current = [];
     }
-  }
-  if (current.length > 0) blocks.push(current);
+  });
+  if (current.length > 0) blocks.push({ start, lines: current });
 
   return blocks;
 }
@@ -197,27 +200,33 @@ function declaredActionColumn(headerCells) {
 // --- Rule two: router coherence -------------------------------------------------------------
 
 /**
- * Names cited from the column a table calls `Action`. A run of rows with no separator beneath it
- * is that table resumed after a blank line, so it keeps the column its header declared.
+ * Names cited from the column a table calls `Action`, and the first line of each run of rows with
+ * no separator that follows such a table. A blank line ends a table, so that run is no table at all:
+ * read as one, a glossary below the router would donate its cells as citations.
  */
 function citationsFromActionColumns(sectionLines) {
   const cited = new Set();
+  const cutOff = [];
   let actionColumn = -1;
 
   for (const block of tableBlocks(sectionLines)) {
-    const rows = block.map(tableCells);
+    const rows = block.lines.map(tableCells);
     const declaresItsOwnColumns = rows.length > 1 && isSeparatorRow(rows[1]);
 
-    if (declaresItsOwnColumns) actionColumn = declaredActionColumn(rows[0]);
+    if (!declaresItsOwnColumns) {
+      if (actionColumn !== -1) cutOff.push(block.start);
+      continue;
+    }
+    actionColumn = declaredActionColumn(rows[0]);
     if (actionColumn === -1) continue;
 
-    for (const cells of declaresItsOwnColumns ? rows.slice(1) : rows) {
+    for (const cells of rows.slice(1)) {
       if (isSeparatorRow(cells)) continue;
       const cell = stripBackticks(cells[actionColumn] ?? "");
       if (CITATION_TOKEN.test(cell)) cited.add(cell.toLowerCase());
     }
   }
-  return cited;
+  return { cited, cutOff };
 }
 
 function matchesOf(pattern, text) {
@@ -233,9 +242,9 @@ function matchesOf(pattern, text) {
  * `aidd-dev:10-todo` cites its one action that way; a backticked file name inside a fence is an
  * example. A word loose in prose is never a citation.
  */
-function citationsIn(blankedLines, rawLines) {
+function citationsIn(cited, blankedLines, rawLines) {
   return new Set([
-    ...citationsFromActionColumns(blankedLines),
+    ...cited,
     ...matchesOf(ACTION_PATH, rawLines.join("\n")),
     ...matchesOf(BACKTICKED_FILE_NAME, blankedLines.join("\n")),
   ]);
@@ -279,7 +288,17 @@ function checkRouterCoherence(filePath, content, actionFileNames) {
   }
 
   const { startIndex, endIndex, headingLine } = section;
-  const citations = citationsIn(lines.slice(startIndex, endIndex), rawLines.slice(startIndex, endIndex));
+  const sectionLines = lines.slice(startIndex, endIndex);
+  const { cited, cutOff } = citationsFromActionColumns(sectionLines);
+
+  // One refusal at the cut, not one per action the rows below it would have named.
+  if (cutOff.length > 0) {
+    const line = startIndex + cutOff[0] + 1;
+    const message = `${filePath}:${line} table rows with no separator follow the action table; a blank line ends a table, so remove it or give these rows their own header`;
+    return [violation("router-coherence", filePath, line, surface.owner, message)];
+  }
+
+  const citations = citationsIn(cited, sectionLines, rawLines.slice(startIndex, endIndex));
   const sharedStems = ambiguousStems(names);
 
   return names