feat(okf): navigate hierarchy — escape, not depth, is forbidden
The pulled method-spec (commons 9801d35) retires the "a target containing a path separator is out-of-bundle" heuristic, which conflated depth with escape and forbade valid hierarchy. Triage of the pull found FIVE contradictions in okf.py, not the two STATE had measured on line 127 alone: 1. the separator ban skipped every legal nested target; 2. de-duplication keyed on the RAW target (`resolved` was computed a line later), not on the resolved path; 3. navigation never recursed — only the root index's links were read; 4. a leading `/` became filesystem-absolute via pathlib rather than denoting the bundle root (safe, because the boundary check caught it, but the right outcome for the wrong reason — and wrong the moment `/a/index.md` must be FOLLOWED); 5. rendering excluded only `verdict`, so a nested index body would render as content. navigate_bundle is now depth-first in first-seen link order, de-duplicating on the resolved path (so `./a.md` and `a.md` are one entry and cycles terminate); resolution and the fail-closed boundary check move to _resolve_target, the sole in-/out-of-bundle test. The missing-index rule binds the bundle root alone. bundle_context renders flat regardless of depth and drops nested index bodies: only the root index is the summary. The gate is the commons-owned nav-golden pair that arrived with the same pull — bundle in, expected-read-context out. Its negative case exists so the gate can go red at all, and carries a real decoy one level up plus a `/etc/passwd` trap. Detach-proved (mutate, run, restore from copy) — each new seam goes RED: D1 reinstate the separator heuristic -> RED D2 re-key dedup on the raw target -> RED D3 read a leading `/` as absolute -> RED D4 render nested index bodies -> RED Control after restore: 24 passed. Suite 631 -> 637, ruff + mypy --strict clean. Comments asserting the retired doctrine were corrected rather than left to document a rule the code no longer follows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M1zp3BxCuzRnUtJPzvEFTQ
This commit is contained in:
parent
d024a236da
commit
7d8de32543
2 changed files with 157 additions and 42 deletions
|
|
@ -2,11 +2,15 @@
|
||||||
|
|
||||||
The read-context is built by NAVIGATING the bundle with progressive disclosure —
|
The read-context is built by NAVIGATING the bundle with progressive disclosure —
|
||||||
never by stuffing the whole bundle (or keyword-retrieved chunks) into the prompt.
|
never by stuffing the whole bundle (or keyword-retrieved chunks) into the prompt.
|
||||||
Navigation starts at ``index.md`` and follows its intra-bundle cross-links; broken
|
Navigation starts at the root ``index.md`` and follows intra-bundle cross-links
|
||||||
or bundle-escaping links are tolerated (skipped, never raised — the OKF robustness
|
depth-first; a bundle MAY be hierarchical, since it is ESCAPE and not DEPTH that is
|
||||||
rule), while a missing ``index.md`` is an error (no entry point). ``type: verdict``
|
forbidden (the resolve-and-boundary-check REPLACES the retired "a path separator
|
||||||
files are EXCLUDED from rendering: prior verdicts reach the hypothesis prompt ONLY
|
means out-of-bundle" heuristic, which conflated the two and forbade valid
|
||||||
via the gated experience fold (see ``experience``), never via context rendering.
|
hierarchy). Broken or bundle-escaping links are tolerated (skipped, never raised —
|
||||||
|
the OKF robustness rule), while a missing ``index.md`` is an error at the bundle
|
||||||
|
ROOT alone. ``type: verdict`` files are EXCLUDED from rendering: prior verdicts
|
||||||
|
reach the hypothesis prompt ONLY via the gated experience fold (see ``experience``),
|
||||||
|
never via context rendering.
|
||||||
|
|
||||||
Pure stdlib by design — the context seam imports no agent toolkit (§11).
|
Pure stdlib by design — the context seam imports no agent toolkit (§11).
|
||||||
"""
|
"""
|
||||||
|
|
@ -105,13 +109,54 @@ def _parse_index_entry(index_path: Path) -> ConceptFile:
|
||||||
return ConceptFile(path=index_path, frontmatter=frontmatter, body=body)
|
return ConceptFile(path=index_path, frontmatter=frontmatter, body=body)
|
||||||
|
|
||||||
|
|
||||||
def navigate_bundle(bundle_dir: Path) -> list[ConceptFile]:
|
def _resolve_target(bundle_root: Path, linking_dir: Path, target: str) -> Path | None:
|
||||||
"""Navigate from ``index.md`` — deterministic order: index first, links first-seen.
|
"""Resolve one cross-link per §3 Step 1, or return ``None`` if it must be skipped.
|
||||||
|
|
||||||
Targets containing a path separator are out-of-bundle and skipped; resolution is
|
A leading ``/`` denotes the BUNDLE ROOT — never a filesystem-absolute path; any
|
||||||
boundary-checked against the bundle directory (fail-closed); broken links are
|
other form is relative to the linking file's own directory. The resolved path is
|
||||||
skipped, never raised. Repeated links are de-duplicated. The index entry point
|
then boundary-checked against the bundle directory, fail-closed: that check is
|
||||||
does not require ``type`` — with or without a frontmatter block
|
the SOLE in-/out-of-bundle test. It is escape, not depth, that is forbidden.
|
||||||
|
|
||||||
|
A target that fails to resolve for ANY reason — missing file, escape, or an
|
||||||
|
invalid path component such as an embedded NUL byte (which makes ``resolve`` /
|
||||||
|
``is_file`` raise ``ValueError``) — is skipped, never raised (OKF robustness).
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
candidate = (
|
||||||
|
bundle_root / target.lstrip("/") if target.startswith("/") else linking_dir / target
|
||||||
|
)
|
||||||
|
resolved = candidate.resolve()
|
||||||
|
if not resolved.is_relative_to(bundle_root) or not resolved.is_file():
|
||||||
|
return None
|
||||||
|
except ValueError:
|
||||||
|
return None
|
||||||
|
return resolved
|
||||||
|
|
||||||
|
|
||||||
|
def _descend(source: Path, bundle_root: Path, seen: set[Path], concepts: list[ConceptFile]) -> None:
|
||||||
|
"""Follow ``source``'s cross-links depth-first, in first-seen link order."""
|
||||||
|
for target in _CROSSLINK_PATTERN.findall(source.read_text(encoding="utf-8")):
|
||||||
|
resolved = _resolve_target(bundle_root, source.parent, target)
|
||||||
|
# De-duplication keys on the RESOLVED path, so `./a.md` and `a.md` are one
|
||||||
|
# entry and link cycles terminate.
|
||||||
|
if resolved is None or resolved in seen:
|
||||||
|
continue
|
||||||
|
seen.add(resolved)
|
||||||
|
is_index = resolved.name == _INDEX_FILENAME
|
||||||
|
concepts.append(_parse_index_entry(resolved) if is_index else parse_concept_file(resolved))
|
||||||
|
_descend(resolved, bundle_root, seen, concepts)
|
||||||
|
|
||||||
|
|
||||||
|
def navigate_bundle(bundle_dir: Path) -> list[ConceptFile]:
|
||||||
|
"""Navigate from ``index.md`` — depth-first, in first-seen link order.
|
||||||
|
|
||||||
|
Hierarchy is legal: a target MAY address a nested directory, and each index links
|
||||||
|
its immediate children (one path segment per level). Resolution and the
|
||||||
|
boundary check live in ``_resolve_target``; de-duplication keys on the resolved
|
||||||
|
path. A missing ``index.md`` is an error AT THE BUNDLE ROOT ALONE — an
|
||||||
|
intermediate directory is navigated only through the links its own files carry,
|
||||||
|
so a nested directory without an index is unreachable, not fatal. The index entry
|
||||||
|
point does not require ``type`` — with or without a frontmatter block
|
||||||
(``_parse_index_entry``); non-index concept files still require it.
|
(``_parse_index_entry``); non-index concept files still require it.
|
||||||
"""
|
"""
|
||||||
index_path = bundle_dir / _INDEX_FILENAME
|
index_path = bundle_dir / _INDEX_FILENAME
|
||||||
|
|
@ -119,38 +164,31 @@ def navigate_bundle(bundle_dir: Path) -> list[ConceptFile]:
|
||||||
raise FileNotFoundError(
|
raise FileNotFoundError(
|
||||||
f"bundle has no entry point: missing {_INDEX_FILENAME} in {bundle_dir}"
|
f"bundle has no entry point: missing {_INDEX_FILENAME} in {bundle_dir}"
|
||||||
)
|
)
|
||||||
index = _parse_index_entry(index_path)
|
|
||||||
bundle_root = bundle_dir.resolve()
|
bundle_root = bundle_dir.resolve()
|
||||||
concepts = [index]
|
concepts = [_parse_index_entry(index_path)]
|
||||||
seen = {_INDEX_FILENAME}
|
seen = {index_path.resolve()}
|
||||||
for target in _CROSSLINK_PATTERN.findall(index_path.read_text(encoding="utf-8")):
|
_descend(index_path.resolve(), bundle_root, seen, concepts)
|
||||||
if "/" in target or "\\" in target or target in seen:
|
|
||||||
continue
|
|
||||||
seen.add(target)
|
|
||||||
try:
|
|
||||||
resolved = (bundle_dir / target).resolve()
|
|
||||||
if not resolved.is_relative_to(bundle_root) or not resolved.is_file():
|
|
||||||
continue
|
|
||||||
except ValueError:
|
|
||||||
# An unrepresentable target (e.g. an embedded NUL byte, which carries no
|
|
||||||
# path separator and so slips past the out-of-bundle filter) makes
|
|
||||||
# ``resolve``/``is_file`` raise ``ValueError``. That is a broken link, not
|
|
||||||
# a fatal error — skip it, never raise (method-spec §72, OKF robustness).
|
|
||||||
continue
|
|
||||||
concepts.append(parse_concept_file(resolved))
|
|
||||||
return concepts
|
return concepts
|
||||||
|
|
||||||
|
|
||||||
def bundle_context(bundle_dir: Path) -> str:
|
def bundle_context(bundle_dir: Path) -> str:
|
||||||
"""Render the read-context: index body, then ``## {type}: {title}`` sections.
|
"""Render the read-context: index body, then ``## {type}: {title}`` sections.
|
||||||
|
|
||||||
Empty sections are dropped. ``type: verdict`` files are excluded — the verdict
|
Rendering is FLAT regardless of nesting depth — directory structure is
|
||||||
layer must never leak into the read-context (§3 Step 1, load-bearing §11).
|
navigation, not presentation, so a nested concept renders as the same section a
|
||||||
|
root one would and there is no level heading. Only the ROOT index body is the
|
||||||
|
summary: a nested ``index.md`` is navigation, not content, and is not rendered.
|
||||||
|
Empty sections are dropped. ``type: verdict`` files are excluded by a TYPE CHECK
|
||||||
|
on each file as it is reached — applied at every level, never a property of the
|
||||||
|
link graph, so a mislabelled or injected edge cannot smuggle a verdict into the
|
||||||
|
context (§3 Step 1, load-bearing §11).
|
||||||
"""
|
"""
|
||||||
index, *concepts = navigate_bundle(bundle_dir)
|
index, *concepts = navigate_bundle(bundle_dir)
|
||||||
sections = [index.body] if index.body else []
|
sections = [index.body] if index.body else []
|
||||||
for concept in concepts:
|
for concept in concepts:
|
||||||
if concept.type == _VERDICT_TYPE or not concept.body:
|
if concept.path.name == _INDEX_FILENAME or concept.type == _VERDICT_TYPE:
|
||||||
|
continue
|
||||||
|
if not concept.body:
|
||||||
continue
|
continue
|
||||||
sections.append(f"## {concept.type}: {concept.title}\n\n{concept.body}")
|
sections.append(f"## {concept.type}: {concept.title}\n\n{concept.body}")
|
||||||
return "\n\n".join(sections)
|
return "\n\n".join(sections)
|
||||||
|
|
|
||||||
|
|
@ -20,6 +20,7 @@ import pytest
|
||||||
from portfolio_optimiser_claude.okf import ConceptFile, bundle_context, navigate_bundle
|
from portfolio_optimiser_claude.okf import ConceptFile, bundle_context, navigate_bundle
|
||||||
|
|
||||||
BUNDLE = Path(__file__).resolve().parents[1] / "shared" / "examples" / "bygg-energi-mikro"
|
BUNDLE = Path(__file__).resolve().parents[1] / "shared" / "examples" / "bygg-energi-mikro"
|
||||||
|
NAV_GOLDENS = Path(__file__).resolve().parents[1] / "shared" / "examples"
|
||||||
SRC_PKG = Path(__file__).resolve().parents[1] / "src" / "portfolio_optimiser_claude"
|
SRC_PKG = Path(__file__).resolve().parents[1] / "src" / "portfolio_optimiser_claude"
|
||||||
|
|
||||||
|
|
||||||
|
|
@ -99,8 +100,10 @@ class TestNavigation:
|
||||||
navigate_bundle(tmp_path)
|
navigate_bundle(tmp_path)
|
||||||
|
|
||||||
def test_out_of_bundle_and_escaping_targets_are_skipped(self, tmp_path: Path) -> None:
|
def test_out_of_bundle_and_escaping_targets_are_skipped(self, tmp_path: Path) -> None:
|
||||||
# A target containing a path separator is out-of-bundle — skipped, never
|
# ../-escapes never resolve outside the bundle (fail-closed), and are skipped
|
||||||
# raised; ../-escapes never resolve outside the bundle (fail-closed).
|
# rather than raised. `sub/inner.md` is skipped here because it does not
|
||||||
|
# EXIST, not because it carries a separator — the separator heuristic is
|
||||||
|
# retired (see TestNavigationBoundary); depth is legal, escape is not.
|
||||||
outside = tmp_path / "outside.md"
|
outside = tmp_path / "outside.md"
|
||||||
_write(outside, "---\ntype: project\ntitle: Outside\n---\nSecret.")
|
_write(outside, "---\ntype: project\ntitle: Outside\n---\nSecret.")
|
||||||
bundle_dir = tmp_path / "bundle"
|
bundle_dir = tmp_path / "bundle"
|
||||||
|
|
@ -123,14 +126,13 @@ class TestNavigation:
|
||||||
assert names == ["index.md", "a.md"]
|
assert names == ["index.md", "a.md"]
|
||||||
|
|
||||||
def test_null_byte_target_is_tolerated_never_raised(self, tmp_path: Path) -> None:
|
def test_null_byte_target_is_tolerated_never_raised(self, tmp_path: Path) -> None:
|
||||||
# LOAD-BEARING (§11, method-spec §72: a broken cross-link MUST be "skipped,
|
# LOAD-BEARING (§11 "Navigation boundary": a malformed target MUST be
|
||||||
# never raised"). A NUL-byte target carries no path separator, so it slips
|
# skipped, not raised). A NUL byte is an INVALID PATH COMPONENT, so
|
||||||
# past the out-of-bundle filter and reaches resolution, where
|
# ``resolve``/``is_file`` raises ``ValueError: embedded null byte`` inside
|
||||||
# ``(bundle_dir / target).resolve()`` raises ``ValueError: embedded null
|
# ``_resolve_target``. That ValueError MUST be caught and the link skipped —
|
||||||
# byte``. That ValueError MUST be caught and the link skipped — a NUL target
|
# a NUL target is a broken link, not a fatal error. It is the malformed
|
||||||
# is a broken link, not a fatal error. This is the FIRST test that drives a
|
# sub-class the escape golden's README delegates here, precisely because a
|
||||||
# dangerous target THROUGH the filter into resolution (the ``/``-filter
|
# literal NUL byte does not belong in a committed text fixture.
|
||||||
# short-circuits every ``/``-bearing and scheme-bearing case before it).
|
|
||||||
# RED (ValueError propagates) if the resolve guard is detached.
|
# RED (ValueError propagates) if the resolve guard is detached.
|
||||||
bundle = _make_bundle(
|
bundle = _make_bundle(
|
||||||
tmp_path,
|
tmp_path,
|
||||||
|
|
@ -156,6 +158,81 @@ class TestNavigation:
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
class TestNavigationBoundary:
|
||||||
|
"""LOAD-BEARING (§11 seam "Navigation boundary"): escape, not depth, is forbidden.
|
||||||
|
|
||||||
|
Gated by the COMMONS-OWNED nav-goldens, whose shape is ``bundle/`` in →
|
||||||
|
``expected-read-context.md`` out — the only shape that can express §3 Step 1's
|
||||||
|
property "two conformant implementations produce an identical read-context from
|
||||||
|
the same bundle". The positive case pins hierarchy, both link forms, depth-first
|
||||||
|
first-seen order, resolved-path dedup, cycle termination, recursive verdict
|
||||||
|
exclusion and flat rendering; the negative case pins escape, and exists so the
|
||||||
|
gate can go RED at all (a gate that can only pass proves nothing).
|
||||||
|
|
||||||
|
RED if the retired "a path separator means out-of-bundle" heuristic returns (the
|
||||||
|
hierarchy case loses every nested concept), if dedup re-keys on the raw target,
|
||||||
|
if a leading ``/`` is read as filesystem-absolute, or if nested index bodies
|
||||||
|
start rendering as content.
|
||||||
|
"""
|
||||||
|
|
||||||
|
def _read_context(self, case: str) -> tuple[str, str]:
|
||||||
|
root = NAV_GOLDENS / case
|
||||||
|
expected = (root / "expected-read-context.md").read_text(encoding="utf-8")
|
||||||
|
# The fixture README pins the shape and permits trailing-whitespace
|
||||||
|
# normalization; only the trailing newline is normalized here.
|
||||||
|
return bundle_context(root / "bundle").rstrip("\n"), expected.rstrip("\n")
|
||||||
|
|
||||||
|
def test_hierarchy_golden_renders_the_expected_read_context(self) -> None:
|
||||||
|
rendered, expected = self._read_context("nav-golden-hierarchy")
|
||||||
|
assert rendered == expected
|
||||||
|
|
||||||
|
def test_escape_golden_renders_the_expected_read_context(self) -> None:
|
||||||
|
rendered, expected = self._read_context("nav-golden-escape")
|
||||||
|
assert rendered == expected
|
||||||
|
|
||||||
|
def test_escape_golden_never_reads_the_out_of_bundle_decoy(self) -> None:
|
||||||
|
# Stated separately from the golden compare because the decoy REALLY exists
|
||||||
|
# one level above ``bundle/``: a ``..`` escape would succeed, so its absence
|
||||||
|
# is a boundary proof rather than a missing-file accident.
|
||||||
|
rendered, _ = self._read_context("nav-golden-escape")
|
||||||
|
assert "MUST never be reached" not in rendered
|
||||||
|
assert "Decoy" not in rendered
|
||||||
|
|
||||||
|
def test_leading_slash_resolves_to_the_bundle_root_and_is_followed(
|
||||||
|
self, tmp_path: Path
|
||||||
|
) -> None:
|
||||||
|
# The root-relative form must be FOLLOWED, not merely survived. The escape
|
||||||
|
# golden's `/etc/passwd` trap is skipped either way (as a bundle-root miss
|
||||||
|
# under the rule, as a boundary violation under a filesystem reading), so it
|
||||||
|
# cannot distinguish the two; a root-relative target that EXISTS can.
|
||||||
|
bundle = _make_bundle(tmp_path, "Root-relative: [T](/deep/target.md).", {})
|
||||||
|
(bundle / "deep").mkdir()
|
||||||
|
_write(
|
||||||
|
bundle / "deep" / "target.md",
|
||||||
|
"---\ntype: project\ntitle: T\n---\nRoot-relative body.",
|
||||||
|
)
|
||||||
|
assert [c.path.name for c in navigate_bundle(bundle)] == ["index.md", "target.md"]
|
||||||
|
|
||||||
|
def test_dedup_is_keyed_on_the_resolved_path_not_the_raw_target(self, tmp_path: Path) -> None:
|
||||||
|
# `./a.md` and `a.md` are two raw strings and one resolved path — RED if the
|
||||||
|
# dedup key regresses to the raw target.
|
||||||
|
bundle = _make_bundle(
|
||||||
|
tmp_path,
|
||||||
|
"See [a](a.md) then [a again](./a.md).",
|
||||||
|
{"a.md": "---\ntype: project\ntitle: A\n---\nBody A."},
|
||||||
|
)
|
||||||
|
assert [c.path.name for c in navigate_bundle(bundle)] == ["index.md", "a.md"]
|
||||||
|
|
||||||
|
def test_a_nested_directory_without_its_own_index_is_not_an_error(self, tmp_path: Path) -> None:
|
||||||
|
# The missing-index rule binds the bundle ROOT alone: an intermediate
|
||||||
|
# directory is navigated only through the links its files carry, so a nested
|
||||||
|
# directory lacking index.md is unreachable, never fatal.
|
||||||
|
bundle = _make_bundle(tmp_path, "Down: [d](sub/doc.md).", {})
|
||||||
|
(bundle / "sub").mkdir()
|
||||||
|
_write(bundle / "sub" / "doc.md", "---\ntype: project\ntitle: D\n---\nNested body.")
|
||||||
|
assert [c.path.name for c in navigate_bundle(bundle)] == ["index.md", "doc.md"]
|
||||||
|
|
||||||
|
|
||||||
class TestFrontmatter:
|
class TestFrontmatter:
|
||||||
"""§3 Step 1: leading ``---`` block, line-oriented ``key: value``; ``type`` required."""
|
"""§3 Step 1: leading ``---`` block, line-oriented ``key: value``; ``type`` required."""
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue