From 426de1fc29293fbd09dcf892b9d485b847d99e72 Mon Sep 17 00:00:00 2001 From: Rick Hightower Date: Sat, 19 Sep 2026 17:27:43 -0500 Subject: [PATCH] fix: resolve the search bundle so a symlinked root works (#85) rg prints resolved paths. search() built each hit's display path against the bundle the caller passed, so when that bundle still held a symlink the two did not match and relative_to raised. Search failed outright. Root cause is the bundle, not the line that raised. Every engine derives its paths from the bundle it is handed, so resolving once at the top of search() makes candidate_files and the display loop agree. One syscall per call, not one per file, which is the property _filter_rg_hits already protects. candidate_files keeps taking the bundle as given. Tests call it directly and expect paths under the bundle they passed, and _filter_rg_hits already resolves for its own comparison, so that path was never broken. Not macOS-only. /var -> /private/var makes every temporary bundle reproduce it there; a symlinked checkout or a container bind mount is the same shape on Linux. CI has neither, which is why this shipped. Adds test_symlinked_bundle_agrees_across_engines, which searches through a real symlink on all three engines. Verified it fails with the one-line fix reverted and passes with it. tools/ci-local.sh reports 27 passed, 0 failed. Closes #85. Item 01M2XWFX063T5QMJ7SC5ZE2R6Q. Co-Authored-By: Claude Opus 5 --- .work/todo.jsonl | 2 ++ docs/roadmap.md | 4 ++-- scripts/pkc_search.py | 7 +++++++ tests/test_pkc.py | 23 +++++++++++++++++++++++ 4 files changed, 34 insertions(+), 2 deletions(-) diff --git a/.work/todo.jsonl b/.work/todo.jsonl index 019752e..6f80be9 100644 --- a/.work/todo.jsonl +++ b/.work/todo.jsonl @@ -132,3 +132,5 @@ {"actor":"richardhightower","ev":"01M2XSTB8K77AKXWD160QM4XTJ","git":"f7134f8","item":"01M2XSTB8K5DMTPHVX3WR8QXSM","op":"create","set":{"body":"CHANGELOG.md carries two '## Unreleased' headings: the intentional placeholder at the top and a leftover from the 0.7.2 era sitting between the 0.7.3 and 0.7.2 sections. Its two bullets describe host manifests reaching 0.7.2, which shipped in 0.7.3, so they belong in the 0.7.3 section above them. A second Unreleased heading makes a release-notes reader think unreleased work is pending.","kind":"bug","level":"task","milestone":"v0.9.6","priority":"P3","status":"todo","title":"Remove the stray Unreleased heading from CHANGELOG"},"ts":"2026-09-19T21:40:35Z"} {"actor":"richardhightower","ev":"01M2XSY20ZHBEVCZTTGZ90W0BH","item":"01M2XSTB63SP54A482P7GF95VD","op":"close","set":{"resolution":"tools/ci-local.sh parses every .github/workflows/*.yml; negative-tested against the #82 breakage","status":"done"},"ts":"2026-09-19T21:42:36Z"} {"actor":"richardhightower","ev":"01M2XSY22P112RB524KRYMRTY6","item":"01M2XSTB8K5DMTPHVX3WR8QXSM","op":"close","set":{"resolution":"stray Unreleased heading removed; its bullets folded into 0.7.3","status":"done"},"ts":"2026-09-19T21:42:36Z"} +{"actor":"richardhightower","ev":"01M2XWFX068C9YAT53QRT3MZ8D","item":"01M2XWFX063T5QMJ7SC5ZE2R6Q","op":"create","set":{"body":"rg prints resolved paths. search() built each hit's display path against the bundle the caller passed, so when that bundle still contained a symlink the two did not match and relative_to raised. Search failed outright rather than returning a wrong answer. On macOS /var is a symlink to /private/var, so every temporary bundle reproduces it; a symlinked checkout or a container bind mount is the same shape on Linux. CI has neither, so the defect shipped. Resolve the bundle once at the top of search() and every engine agrees.","kind":"bug","level":"task","milestone":"v0.9.6","priority":"P1","status":"todo","title":"search dies on a symlinked bundle root"},"ts":"2026-09-19T22:27:18Z"} +{"actor":"richardhightower","ev":"01M2XWG1S5HFP2QQ5MHKHNHKAA","item":"01M2XWFX063T5QMJ7SC5ZE2R6Q","op":"close","set":{"resolution":"search() resolves the bundle once; regression test searches through a real symlink","status":"done"},"ts":"2026-09-19T22:27:23Z"} diff --git a/docs/roadmap.md b/docs/roadmap.md index 3bbb37f..bf569c6 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -2,8 +2,8 @@ wiki_key: roadmap doc_type: roadmap truth_state: current -source_hash: eb13176e -generated_at: 2026-09-19T21:42:36Z +source_hash: 81b0e6dc +generated_at: 2026-09-19T22:27:23Z --- diff --git a/scripts/pkc_search.py b/scripts/pkc_search.py index 37821f3..5850210 100755 --- a/scripts/pkc_search.py +++ b/scripts/pkc_search.py @@ -126,6 +126,13 @@ def search( if not terms: return [], "scan" + # rg yields resolved paths, so on a symlinked root the rel path built for + # each hit below raised ValueError against the caller's bundle (#85). + # /var -> /private/var on macOS, a symlinked checkout, a bind mount. Every + # engine derives its paths from the bundle it is handed, so resolving once + # here makes candidate_files and the loop below agree. One syscall, not one + # per file: _filter_rg_hits keeps its own guard for direct callers. + bundle = bundle.resolve() type_filter = {t.lower() for t in (types or []) if t} results: list[dict[str, Any]] = [] files, engine = candidate_files( diff --git a/tests/test_pkc.py b/tests/test_pkc.py index 8c2fd14..8509da1 100644 --- a/tests/test_pkc.py +++ b/tests/test_pkc.py @@ -1676,6 +1676,29 @@ def test_titleless_stem_match_is_invisible_on_every_engine(self): self.assertEqual([h["path"] for h in hits], [h["path"] for h in scan]) self.assertEqual([h["score"] for h in hits], [h["score"] for h in scan]) + def test_symlinked_bundle_agrees_across_engines(self): + # Issue #85. rg prints resolved paths, so building the rel path against + # an unresolved bundle raised ValueError and search() died outright. + # /var -> /private/var on macOS does this to every tempfile bundle; a + # symlinked checkout or a bind mount is the same shape on Linux, which + # is why CI never saw it. + real = Path(tempfile.mkdtemp()) + (real / "index.md").write_text('---\nokf_version: "0.2"\ntitle: t\n---\n', encoding="utf-8") + (real / "features").mkdir() + (real / "features" / "other.md").write_text( + "---\ntype: Feature\ntitle: Zebra crossing\n---\n\nplain\n", encoding="utf-8" + ) + link = Path(tempfile.mkdtemp()) / "via-symlink" + link.symlink_to(real, target_is_directory=True) + + scan, scan_engine = search_bundle(link, "zebra", use_rg=False, use_index=False) + accel, rg_engine = search_bundle(link, "zebra", use_rg=True, use_index=False) + indexed, idx_engine = search_bundle(link, "zebra", use_rg=False, use_index=True) + self.assertEqual((scan_engine, rg_engine, idx_engine), ("scan", "rg", "index")) + self.assertEqual([h["path"] for h in scan], ["/features/other.md"]) + for hits in (accel, indexed): + self.assertEqual([h["path"] for h in hits], [h["path"] for h in scan]) + def test_search_rg_matches_scan_ranking(self): bundle = ROOT / "sample-knowledge" scan, scan_engine = search_bundle(bundle, "JWT", limit=10, use_rg=False, use_index=False)