fix(catalog): flag name conflicts, fail an empty root, keep spec errors out of routing

dokimi/changelog.d/20261021_130000_napetrov_routing-catalog-manifest.md

@@ -5,5 +5,5 @@
   does for `catalog scan`, so a `SKILL.md` it does not list is no longer a sibling. Without a
   manifest, every `SKILL.md` under the directory counts as before. A declared skill that cannot
   be loaded is skipped with a warning on stderr instead of making the whole field unreadable.
-  Two catalog skills with the same name and different content are refused, naming both paths,
-  rather than the run failing on a duplicate competitor name.
+  Two catalog skills with the same name and different content leave the field with a warning
+  naming every path, rather than the run being refused.

dokimi/changelog.d/20261031_120000_napetrov_rescan-catalog.md

@@ -0,0 +1,30 @@
+### Added
+
+- **`novelty routing --include-invalid`** keeps a catalog skill with a spec error in the `--catalog`
+  field, and the saved result records the flag under `field`. A name that conflicts with another
+  catalog skill still leaves the field.
+
+### Changed
+
+- **`novelty routing --catalog` leaves out a skill with a spec error by default** (for example a
+  description over 1024 characters, or a name that is not kebab-case or not its directory name), with
+  a warning on stderr naming the error. Before, such a skill was in the field. `--include-invalid`
+  keeps it. A `--competitors skill:<path>` with a spec error is skipped the same way. A name with two
+  different copies also changes the field, so a routing result saved before this change is not
+  comparable with one saved now for the same `--catalog`: rerun it to compare. With
+  `--include-disabled`, a copy that is `disable-model-invocation` counts as one of the copies.
+- **`novelty routing --catalog` skips a name that two different skills share, with a warning, instead of
+  refusing the run** (exit 2). Every copy of that name leaves the field, the rest of it stays, and stderr
+  names every path. Under `--all-vs-all`, a query file that names such a skill, or one left out
+  for a spec error, is still refused (exit 2), and the message names both reasons.
+- **`catalog scan` reports a same-name skill with different content as a `name-conflict` warning**
+  (among listed skills: no `disable-model-invocation`, no spec error), naming the other path and that
+  `novelty routing --catalog` leaves the name out of the field. Before, each of the two skills was
+  reported `ok`. It is a warning, not an error, so `--strict` does not fail on it.
+
+### Fixed
+
+- **`catalog scan` on a root with no `SKILL.md` exits 1 with `no SKILL.md found under <root>`**
+  instead of exiting 0 silently. With `--kind all`, a tasks-only root only warns, and `--strict`
+  makes that a failure. A root with a `skills.yaml` that has no skill that can be scanned says
+  `skills.yaml under <root> has no skill that can be scanned` instead.

dokimi/docs/external-content.md

@@ -113,21 +113,25 @@ Read that output as three outcomes, not two:
 
 Only load errors change the exit code, so `catalog scan` exiting 0 means
 "everything declared as runnable is runnable". A spec error (below) changes it
-only under `--strict`. Note the asymmetry above: a `planned` *task*
+only under `--strict`. A root with no `SKILL.md` at all (and no skills.yaml entry)
+prints `no SKILL.md found under <root>` to stderr and exits 1. With `--kind all`, a
+tasks-only root only warns, and exits 1 under `--strict`. Note the asymmetry above: a `planned` *task*
 with no directory is skipped, while a `planned` *skill* with no directory is an
 error — the skill reader has no notion of a not-yet-written entry. Either drop
 such a skill from `skills.yaml` or expect the non-zero exit.
 
-A loaded skill is also checked against the Agent Skills spec. Its `name` must be
-its directory name and kebab-case (at most 64 characters; a `SKILL.md` directly in the scanned root is not compared with the root directory's name), and its `description`
-at most 1024 characters: a violation is an `error`, printed under the skill. It
-exits 1 only with `--strict`; without it, the scan reports the error and still
-exits 0. A `name` missing from the frontmatter, or a `SKILL.md` byte-identical to
-another skill's, is a `warn`. Of identical copies, the one nearest the root (fewest
-path parts, ties to path order) is the original, the copy the `skill-catalog:` arm
-offers; each other copy is the `warn` and names it. Warnings never change
-the exit code. `disable-model-invocation: true` is printed as a `note`, because
-the model is never offered that skill.
+A loaded skill is also checked against the Agent Skills spec. Its `name` must be its directory name
+and kebab-case (at most 64 characters; a `SKILL.md` directly in the scanned root is not compared
+with the root directory's name), and its `description` at most 1024 characters: a violation is an
+`error`, printed under the skill. It exits 1 only with `--strict`; without it, the scan reports the
+error and still exits 0. A `name` missing from the frontmatter, or a `SKILL.md` byte-identical to
+another skill's, is a `warn`. So is a skill whose `name` another skill shares with different content
+(`name-conflict`), when both are listed: neither is `disable-model-invocation` or has a spec error.
+The finding names the other path, and `dokimi novelty routing --catalog` skips that name rather than
+the whole catalog. Of identical copies, the one nearest the root (fewest path parts, ties to path
+order) is the original, the copy the `skill-catalog:` arm offers; each other copy is the `warn` and
+names it. Warnings never change the exit code. `disable-model-invocation: true` is printed as a
+`note`, because the model is never offered that skill.
 
 Frontmatter that is not valid YAML (a bare colon in `description:`, say) stops
 the skill from loading, since the loader reads it as having no description. The
@@ -392,6 +396,7 @@ Worth knowing before you read a suspiciously clean result:
 | Command | Root missing | Exit |
 |---|---|---|
 | `catalog scan --root <missing>` | `--root is not a directory: <path>` | 1 |
+| `catalog scan --root <no SKILL.md> --kind skills` | `no SKILL.md found under <path>` | 1 |
 | `tasks run --tasks-root <missing>` | `FileNotFoundError: Task directory not found: demo-widget` | 1 |
 | `tasks list --tasks-root <missing>` | prints nothing | 0 |
 | `explore list --tasks-root <missing>` | `--tasks-root is not a directory: <path>` | 2 |

dokimi/docs/reference/cli-interface.json

@@ -8533,6 +8533,22 @@
               "required": false,
               "type": null
             },
+            {
+              "action": "_StoreTrueAction",
+              "choices": null,
+              "const": true,
+              "default": false,
+              "group_key": null,
+              "group_required": null,
+              "hidden": false,
+              "is_flag": true,
+              "key": "--include-invalid",
+              "kind": "option",
+              "nargs": 0,
+              "position": null,
+              "required": false,
+              "type": null
+            },
             {
               "action": "_StoreAction",
               "choices": null,

dokimi/docs/skill-novelty.md

@@ -605,8 +605,22 @@ lists except the artifact itself. A `skills.yaml` at the root decides what the c
 without one, every `SKILL.md` under it counts, except a `template/` copy and vendored or VCS
 directories such as `.git` and `node_modules`, which are not walked. A byte-identical copy under
 another path counts once. A declared skill that cannot be loaded is skipped with a warning. Two
-catalog skills with the same name and different content are refused, with both paths named:
-rename one, or delete the copy you do not mean to route against.
+catalog skills with the same name and different content are both skipped, with a warning that
+names every path of that name: the rest of the field stays. Only the copies the field lists count:
+a disabled copy (unless `--include-disabled`) or a spec-error copy (unless `--include-invalid`) is
+left out first, so it does not make its name a conflict. Rename one, or delete the copy you do
+not mean to route against, to route the name. `dokimi catalog scan` lists the same names as
+`name-conflict` warnings, for the copies it lists. A skill with a spec error (for example a description over 1024 characters, or a
+name that is not kebab-case or not its directory name) is also left out, with a warning naming the error; pass
+`--include-invalid` to keep it, as a harness that refuses to list such a skill would not. A `skill:`
+competitor with a spec error is skipped with the same warning.
+The directory-name check applies to a `skill:` competitor under a `skills/` directory
+(`skills/<dir>/SKILL.md`, `plugins/x/skills/<dir>/SKILL.md`), as it does in a catalog. A `skill:`
+competitor anywhere else is read as a root skill, and its name is not checked against its
+directory: `tools/database/SKILL.md` named `db` is routed when passed explicitly, while a
+`--catalog` scan of its tree leaves it out with a name error. Keep the directory name and the
+`name` field equal to get the same field from both.
+A name that conflicts with another catalog skill leaves the field whatever the flag says.
 A `skill:` competitor that is not a `SKILL.md`, or a directory without one, is skipped with a
 warning. A shell glob must be expanded first, with the prefix on each path
 (`skill:dir/a/SKILL.md skill:dir/b/SKILL.md`): an unexpanded `skill:dir/*` is refused as a missing
@@ -673,7 +687,8 @@ is no attractor, and a tied skill's own rate is lower by the picks inside its gr
 `n_scored` and `n_unscored` per skill say how many, a query with no content word or a failed
 model call is unscored, and a skill with none scored has an own rate of `null`, not 0.0. A run
 with any failed model call writes the result and exits non-zero. The saved file records the
-field rule under `field` (`include_disabled`, the `competitors` and what was `left_out`); a
+field rule under `field` (`include_disabled`, `include_invalid`, the `competitors` and what was `left_out`, which
+also names each skill left out for a conflicting name or a spec error); a
 single-artifact run records it too. It takes no `--artifact`, `--queries`,
 `--queries-from-headings` or `--repeats` (one model call per query). The model arm also records the selector `model`, so two runs with
 different models can be told apart. It is refused with fewer than two skills, a name repeated

dokimi/dokimi/commands/catalog.py

@@ -131,6 +131,15 @@ def cmd_catalog_scan(args: argparse.Namespace) -> None:
         any_failure = _scan_skills(args, root, report, as_json)
     if args.kind in ("tasks", "all"):
         any_failure = _scan_tasks(args, root, report, as_json) or any_failure
+    if args.kind in ("skills", "all") and not report["skills"]:
+        # Nothing to scan is a wrong root or an empty checkout, so a skills scan fails on it. A
+        # --kind all scan of a tasks-only catalog is a warning, which --strict makes a failure.
+        if (root / "skills.yaml").is_file():
+            print(f"skills.yaml under {root} has no skill that can be scanned", file=sys.stderr)
+        else:
+            print(f"no SKILL.md found under {root}", file=sys.stderr)
+        if args.kind == "skills" or args.strict or not report["tasks"]:
+            any_failure = True
     if as_json:
         print(json.dumps(report, indent=2))
     if any_failure:
@@ -335,7 +344,8 @@ def register(sub, positive_int) -> None:
         "--strict",
         action="store_true",
         help="Exit 1 on a skill spec error (name not the directory name or not kebab-case, "
-        "description over 1024 characters) as well as on a skill that does not load",
+        "description over 1024 characters) as well as on a skill that does not load; with "
+        "--kind all, also on a tasks-only root with no SKILL.md",
     )
     scan_p.add_argument(
         "--format",

dokimi/dokimi/commands/novelty.py

@@ -304,9 +304,8 @@ def _routing_field(args: argparse.Namespace, exclude: set[str],
     """
     from dokimi.novelty.routing import (
         catalog_competitors,
-        disables_model_invocation,
+        competitor_left_out,
         non_skill_reason,
-        skill_file,
     )
 
     competitors = []
@@ -318,20 +317,19 @@ def _routing_field(args: argparse.Namespace, exclude: set[str],
                 left_out.append(f"{ref}: {reason}")
             continue
         candidate = _routing_card(ref)[0]
-        ref_file = skill_file(ref)
-        if (ref_file and not args.include_disabled
-                and disables_model_invocation(ref_file)):
-            print(f"WARNING: skipped competitor {ref}: disable-model-invocation "
-                  "(--include-disabled keeps it)", file=sys.stderr)
+        reason = competitor_left_out(ref, include_disabled=args.include_disabled,
+                                     include_invalid=args.include_invalid)
+        if reason:
+            print(f"WARNING: skipped competitor {ref}: {reason}", file=sys.stderr)
             if left_out is not None:
-                left_out.append(f"{ref}: disable-model-invocation")
+                left_out.append(f"{ref}: {reason}")
             continue
         competitors.append(candidate)
     if args.catalog:
         notes: list[str] = []
-        catalog = catalog_competitors(args.catalog,
-                                      exclude | {c.name for c in competitors}, notes,
-                                      include_disabled=args.include_disabled)
+        catalog = catalog_competitors(args.catalog, exclude | {c.name for c in competitors},
+                                      notes, include_disabled=args.include_disabled,
+                                      include_invalid=args.include_invalid)
         for note in notes:
             print(f"WARNING: {note}", file=sys.stderr)
         if left_out is not None:
@@ -408,7 +406,7 @@ def _cmd_routing_all_vs_all(args: argparse.Namespace) -> None:
 
 def _field_record(args: argparse.Namespace, competitors: list, left_out: list[str]) -> dict:
     """The field rule a routing result was measured under, saved next to the result."""
-    return {"include_disabled": bool(args.include_disabled),
+    return {**{name: bool(getattr(args, name)) for name in ("include_disabled", "include_invalid")},
             "competitors": [c.name for c in competitors], "left_out": left_out}
 
 
@@ -855,16 +853,16 @@ def register(sub, positive_int) -> None:
                            help="Directory of skills to take the competitors from: every "
                                 "skill the catalog lists (skills.yaml when present, else every "
                                 "SKILL.md under it; a template/ copy and vendored or VCS "
-                                "directories like .git are left out, and a byte-identical "
-                                "copy under another path counts once) except "
-                                "the artifact itself, i.e. the "
-                                "siblings a harness lists next to it. Combines with "
-                                "--competitors")
+                                "directories like .git are left out, and a byte-identical copy "
+                                "under another path counts once) except the artifact itself, the "
+                                "siblings a harness lists next to it. Combines with --competitors")
     routing_p.add_argument("--include-disabled", action="store_true", dest="include_disabled",
                            help="Keep skills that set disable-model-invocation in the field. "
                                 "Off by default: no harness lists them, so the model cannot "
                                 "select them and a card for one measures a choice that does "
                                 "not exist")
+    routing_p.add_argument("--include-invalid", action="store_true", dest="include_invalid",
+                           help="Keep catalog and skill: competitors with a spec error in the field")
     routing_p.add_argument("--queries", default=None,
                            help="JSON list (or one per line) of user-shaped requests that "
                                 "should select this artifact, written independently of it")

dokimi/dokimi/external/validate.py

@@ -27,6 +27,10 @@
 Mirrors are common (one skill checked in under two plugin directories), and
 counting them twice inflates routing and coverage numbers.
 
+Same name, different content: a skill whose ``name`` another listed skill shares, with other bytes, is a
+``warn`` (``name-conflict``) naming the other path. A listed skill is one a harness offers (no
+``disable-model-invocation``, no spec error). Routing leaves that name out of the field rather than aborting.
+
 ``disable-model-invocation: true`` is not a finding: the model is never offered
 that skill, so it is reported as a flag (the same reading as the skill arm's
 check in :mod:`dokimi.harnesses.skill_treatment`).
@@ -193,6 +197,7 @@ def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillRe
     """
     reviews: list[Optional[SkillReview]] = []
     copies: list[tuple[SkillReview, str, str]] = []  # (review, skill directory, digest) per loaded skill
+    by_name: dict[str, list[tuple[SkillReview, str, str]]] = {}
     for entry in entries:
         if not entry.loaded or entry.skill is None:
             reviews.append(_unloaded_review(entry))
@@ -205,6 +210,7 @@ def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillRe
         review.content_sha256 = _skill_digest(skill)
         where = _relative_to_root(skill.path.parent, entry.source_root)
         copies.append((review, where, review.content_sha256))
+        by_name.setdefault(skill.name, []).append((review, where, review.content_sha256))
         reviews.append(review)
 
     original_of: dict[str, tuple[SkillReview, str]] = {}
@@ -222,4 +228,31 @@ def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillRe
                 f"SKILL.md is identical to {where}; the same skill is counted twice",
             )
         )
+    _name_conflicts(by_name)
     return reviews
+
+
+def _name_conflicts(by_name: dict[str, list[tuple[SkillReview, str, str]]]) -> None:
+    """A warning on each listed skill that shares its name with a listed skill of different content.
+
+    Listed means a harness offers it: a skill with ``disable-model-invocation`` or a spec error is
+    left out of the routing field by default, so its copy is no conflict. Identical copies are not
+    a conflict (they are the duplicate above). The routing field skips such a name, so the scan
+    names both paths to say which skills are affected.
+    """
+    for name, group in by_name.items():
+        group = [entry for entry in group
+                 if not entry[0].disable_model_invocation and entry[0].state != "error"]
+        for review, _, digest in group:
+            others = sorted({other for _, other, other_digest in group if other_digest != digest})
+            if not others:
+                continue
+            review.findings.append(
+                Finding(
+                    "warn",
+                    "name-conflict",
+                    f"name {name!r} is also used by {', '.join(others)}, with different content; "
+                    "`novelty routing --catalog` leaves this name out of the field "
+                    "until one is renamed or removed",
+                )
+            )

dokimi/dokimi/novelty/all_vs_all.py

@@ -131,8 +131,10 @@ def _check_all_vs_all_input(names: list[str], queries_by_skill: dict[str, list[s
     if unknown:
         raise RoutingInputError("the queries name skills that are not in the field: "
                                 + ", ".join(f"`{n}`" for n in unknown)
-                                + "; a skill that sets disable-model-invocation is left out "
-                                "unless you pass --include-disabled")
+                                + "; the field leaves out a skill that sets "
+                                "disable-model-invocation (--include-disabled keeps it), one "
+                                "with a spec error (--include-invalid keeps it) and every copy "
+                                "of a name that two different skills share")
     if not queries_by_skill:
         raise RoutingInputError("no skill in the field has queries to route")