From d433a9f4c0a506b2ae27151d3f60b9d1e8ebc4fb Mon Sep 17 00:00:00 2001 From: Rick Hightower Date: Mon, 31 Aug 2026 13:43:18 -0500 Subject: [PATCH 1/2] fix: render scalar catalog titles and canonicalize rg inbound paths (#72) (#73) Two defects the SAC v0.5.4 fixes exposed in the shared renderer and the shared rg accelerator. #72: `_escape_link_label()` assumed a string, so a concept with `title: 421` aborted catalog rendering after capture had already written concepts. It now normalizes to text at the boundary. `fm_c.get("title") or p.stem` also sent a falsy-but-real title (`0`, `false`) to the file stem. Both renderers now share one `_catalog_label()` helper, so the fallback rule cannot drift between them. PKC has two renderers, so patching only `refresh_catalog_index` would leave first-time catalog creation broken. #73: `rg_list_files()` resolves its hits, but `_inbound_via_rg()` derived `src` with `path.relative_to(bundle)`. A bundle reached through a symlink alias made that raise, and the handler silently dropped a real inbound edge while the pack still reported `reverse_index: rg`. `resolve_knowledge_root()` returns an absolute `--bundle` verbatim, so this was reachable from the CLI. Both regression tests are red without the corresponding fix. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TZ1sCoZonCJw2oPbPcPioW --- scripts/pkc_common.py | 25 ++++++++++++++++---- scripts/pkc_pack.py | 5 +++- tests/test_pkc.py | 54 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 5 deletions(-) diff --git a/scripts/pkc_common.py b/scripts/pkc_common.py index 773a52a..b0a5844 100644 --- a/scripts/pkc_common.py +++ b/scripts/pkc_common.py @@ -506,8 +506,25 @@ def write_concept( return path, "created" -def _escape_link_label(label: str) -> str: - return label.replace("[", "\\[").replace("]", "\\]") +def _escape_link_label(label: Any) -> str: + """Make a concept title safe to use as a Markdown link label. + + YAML titles may be typed scalars (for example integers or booleans), so + normalize to text at the rendering boundary. Kept as one function so the + catalog renderers cannot drift apart on escaping. + """ + return str(label).replace("[", "\\[").replace("]", "\\]") + + +def _catalog_label(fm_c: dict[str, Any], path: Path) -> str: + """Pick and escape the label for one catalog entry. + + Both renderers below call this, so the fallback rule cannot drift between + them. `or` alone would send a falsy-but-real title (`0`, `false`) to the + stem, so only a missing or empty title falls back. + """ + title = fm_c.get("title") + return _escape_link_label(path.stem if title is None or title == "" else title) def ensure_catalog_index(bundle: Path, catalog: str, title: str | None = None) -> Path: @@ -529,7 +546,7 @@ def ensure_catalog_index(bundle: Path, catalog: str, title: str | None = None) - if p.name == "index.md": continue fm_c, _ = parse_frontmatter(p.read_text(encoding="utf-8")) - label = _escape_link_label(fm_c.get("title") or p.stem) + label = _catalog_label(fm_c, p) body += f"- [{label}](/{catalog}/{p.name})\n" if body.endswith(":\n\n"): body += "_None yet._\n" @@ -554,7 +571,7 @@ def refresh_catalog_index(bundle: Path, catalog: str) -> None: if p.name == "index.md": continue fm_c, _ = parse_frontmatter(p.read_text(encoding="utf-8")) - label = _escape_link_label(fm_c.get("title") or p.stem) + label = _catalog_label(fm_c, p) entries.append(f"- [{label}](/{catalog}/{p.name})") body += "\n".join(entries) + ("\n" if entries else "_None yet._\n") index.write_text(dump_frontmatter(fm) + "\n" + body, encoding="utf-8") diff --git a/scripts/pkc_pack.py b/scripts/pkc_pack.py index 9aa07d9..87eabed 100755 --- a/scripts/pkc_pack.py +++ b/scripts/pkc_pack.py @@ -107,7 +107,10 @@ def _inbound_via_rg( if not is_concept_path(bundle, path): continue try: - src = "/" + path.relative_to(bundle).as_posix() + # rg_list_files resolves its hits, so a bundle reached through a + # symlink alias (macOS /var -> /private/var) makes relative_to + # raise and silently drop a real inbound edge. Canonicalize both. + src = "/" + path.resolve().relative_to(bundle.resolve()).as_posix() except ValueError: continue if src == target: diff --git a/tests/test_pkc.py b/tests/test_pkc.py index 37f2301..33979e8 100644 --- a/tests/test_pkc.py +++ b/tests/test_pkc.py @@ -158,6 +158,33 @@ def test_ensure_escapes_too(self): if lines: # ensure_catalog_index lists entries here self.assertIn(r"\[AREA\]", lines[0], f"label not escaped: {lines[0]!r}") + def test_yaml_scalar_titles_render_as_text(self): + """`title: 421` parses as an int. Both renderers used to call + str.replace on it and abort the whole catalog refresh.""" + cases = { + "integer": ("421", "421"), + "boolean": ("false", "False"), + "date-like": ("2026-08-31", "2026-08-31"), + "zero": ("0", "0"), + } + for renderer in (refresh_catalog_index, ensure_catalog_index): + with self.subTest(renderer=renderer.__name__): + with tempfile.TemporaryDirectory() as td: + bundle = Path(td) + ensure_bundle(bundle) + decisions = bundle / "decisions" + decisions.mkdir(exist_ok=True) + for slug, (yaml_title, _expected) in cases.items(): + (decisions / f"{slug}.md").write_text( + f"---\ntype: Decision\ntitle: {yaml_title}\n---\n\n# {yaml_title}\n", + encoding="utf-8", + ) + (decisions / "index.md").unlink(missing_ok=True) + renderer(bundle, "decisions") + index = (decisions / "index.md").read_text(encoding="utf-8") + for slug, (_yaml_title, expected) in cases.items(): + self.assertIn(f"- [{expected}](/decisions/{slug}.md)", index) + def test_refuses_a_catalog_this_plugin_does_not_declare(self): self.assertNotIn("lakehouses", CATALOGS) with tempfile.TemporaryDirectory() as td: @@ -1489,6 +1516,33 @@ def test_pack_rg_matches_scan_graph(self): self.assertEqual(accel["reverse_index"], "rg") self.assertEqual(scan["reverse_index"], "scan") + def test_inbound_pack_survives_a_symlink_aliased_bundle(self): + """rg_list_files resolves its hits. mkdtemp hands back the /var alias + on macOS, so relative_to raised and the inbound edge was dropped — + while the pack still reported `reverse_index: rg`.""" + tmp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, tmp, True) + self.assertNotEqual(tmp, tmp.resolve(), "no path alias here; test is inert") + ensure_bundle(tmp) + features = tmp / "features" + features.mkdir(exist_ok=True) + (features / "root.md").write_text( + "---\ntype: Feature\ntitle: Root\n---\n\n# Root\n", encoding="utf-8" + ) + (features / "caller.md").write_text( + "---\ntype: Feature\ntitle: Caller\nlinks:\n" + " - target: /features/root.md\n rel: relates_to\n---\n\n# Caller\n", + encoding="utf-8", + ) + seed = features / "root.md" + scan = pack_bundle(tmp, seed, hops=1, max_nodes=8, use_rg=False, use_index=False) + accel = pack_bundle(tmp, seed, hops=1, max_nodes=8, use_rg=True, use_index=False) + scan_paths = {n["path"] for n in scan["nodes"]} + accel_paths = {n["path"] for n in accel["nodes"]} + self.assertIn("/features/caller.md", scan_paths) + self.assertEqual(scan_paths, accel_paths) + self.assertEqual(accel["reverse_index"], "rg") + def test_doctor_reports_toolchain(self): report = doctor_bundle(ROOT / "sample-knowledge") self.assertIn("toolchain", report) From ac0d9fe5d4f4f1a2ea26da99924c195178b7baf2 Mon Sep 17 00:00:00 2001 From: Rick Hightower Date: Mon, 31 Aug 2026 13:46:13 -0500 Subject: [PATCH 2/2] test: build the symlink instead of leaning on the /var alias (#73) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mkdtemp yields the /var alias on macOS but a plain /tmp path on Linux, so the regression assertion was inert on CI — the platform it most needs to cover. Create the symlink explicitly and address the bundle through it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TZ1sCoZonCJw2oPbPcPioW --- tests/test_pkc.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/test_pkc.py b/tests/test_pkc.py index 33979e8..3c19624 100644 --- a/tests/test_pkc.py +++ b/tests/test_pkc.py @@ -1517,14 +1517,23 @@ def test_pack_rg_matches_scan_graph(self): self.assertEqual(scan["reverse_index"], "scan") def test_inbound_pack_survives_a_symlink_aliased_bundle(self): - """rg_list_files resolves its hits. mkdtemp hands back the /var alias - on macOS, so relative_to raised and the inbound edge was dropped — - while the pack still reported `reverse_index: rg`.""" + """rg_list_files resolves its hits, so relative_to raised on an aliased + bundle and the inbound edge was dropped — while the pack still reported + `reverse_index: rg`. + + The symlink is built here rather than leaned on: mkdtemp yields the + /var alias on macOS but a plain /tmp path on Linux, which made an + alias-dependent test inert on CI — the platform this must not regress on. + """ tmp = Path(tempfile.mkdtemp()) self.addCleanup(shutil.rmtree, tmp, True) - self.assertNotEqual(tmp, tmp.resolve(), "no path alias here; test is inert") - ensure_bundle(tmp) - features = tmp / "features" + real = tmp / "real" + real.mkdir() + alias = tmp / "alias" + alias.symlink_to(real, target_is_directory=True) + ensure_bundle(real) + self.assertNotEqual(alias.resolve(), alias) + features = alias / "features" features.mkdir(exist_ok=True) (features / "root.md").write_text( "---\ntype: Feature\ntitle: Root\n---\n\n# Root\n", encoding="utf-8" @@ -1535,8 +1544,8 @@ def test_inbound_pack_survives_a_symlink_aliased_bundle(self): encoding="utf-8", ) seed = features / "root.md" - scan = pack_bundle(tmp, seed, hops=1, max_nodes=8, use_rg=False, use_index=False) - accel = pack_bundle(tmp, seed, hops=1, max_nodes=8, use_rg=True, use_index=False) + scan = pack_bundle(alias, seed, hops=1, max_nodes=8, use_rg=False, use_index=False) + accel = pack_bundle(alias, seed, hops=1, max_nodes=8, use_rg=True, use_index=False) scan_paths = {n["path"] for n in scan["nodes"]} accel_paths = {n["path"] for n in accel["nodes"]} self.assertIn("/features/caller.md", scan_paths)