From c9ab42cb9ebcb6e7aa8d6306113866d435b47872 Mon Sep 17 00:00:00 2001 From: drknowhow Date: Wed, 9 Sep 2026 12:01:15 -0400 Subject: [PATCH 1/4] fix(read): read outside the project root instead of crashing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A path outside the project root did not get a refusal. It got an uncaught ValueError out of Path.relative_to in handle_read, surfaced to the caller as "Error calling tool 'c3_read': '...' is not in the subpath of '...'". Nothing in the tool ever checked containment. validate_file_path is a ghost-path heuristic (type names, heredoc markers, redirect artifacts), and access_guard.canonicalize returns rel="" with NO denial for an outside path — tests/test_access_guard.py has always asserted that. The accidental boundary was worth nothing as a control: - Access Guard rules match the absolute canonical path as well as the project-relative one (Rule.matches tests `canon`), so the builtin dotenv and vault denies, and user deny globs, already bound on outside paths. That is where the protection lives, and it is untouched here. - The PreToolUse hook deliberately stands down outside the root, so native Read went there unimpeded. Refusing only pushed the agent off the budgeted, guarded, telemetered tool onto the one with none of it. It also leaked. full.exists() was checked before the throw, so an existing outside file produced the subpath error while a missing one produced "File not found" — a distinguishable-response probe over the whole filesystem, which is what the R2 rule quoted three lines above that code exists to prevent. What the root still decides is INDEXING. file_memory is keyed by project-relative path and feeds the text index, the map cache and prune_stale, so an outside file is served from a transient record (FileMemoryStore.build_transient_record): same parser, same schema, never persisted, never looked up. It is built lazily — a lines=[a,b] slice must not pay for a full parse it never uses. Directory maps outside the root extract symbols the same transient way, under the same file/deadline/byte bounds. Every outside response opens with a [c3-read:external] banner. Related facts are suppressed there (project knowledge keyed by a key the file does not have), and telemetry carries external: true. c3_compress had the identical crash at the same expression and is fixed the same way; project_key() and external_banner() are shared so the two map surfaces cannot drift apart on where a file may live. A hard root boundary remains available as an Access Guard rule. It is not a property of the tool. Claude-Session: https://claude.ai/code/session_01H2xJVonLH3ZKjDUQa4h3eS --- cli/mcp_server.py | 5 +- cli/tools/_helpers.py | 34 ++++ cli/tools/compress.py | 68 ++++++-- cli/tools/read.py | 112 ++++++++++--- docs/file-map.md | 45 +++++ services/dir_map.py | 54 ++++-- services/file_memory.py | 59 +++++-- tests/test_read_outside_root.py | 289 ++++++++++++++++++++++++++++++++ 8 files changed, 601 insertions(+), 65 deletions(-) create mode 100644 tests/test_read_outside_root.py diff --git a/cli/mcp_server.py b/cli/mcp_server.py index 66a758d..5b908c1 100644 --- a/cli/mcp_server.py +++ b/cli/mcp_server.py @@ -734,7 +734,10 @@ async def c3_read(file_path: str, symbols: Any = None, lines: Any = None, """READ source, or MAP a file when called with only file_path (read-only, plan-mode safe). file_path: single or comma-separated. symbols: names as the map prints them (Class.method ok). lines: int, [start,end], or list of ranges. No symbols/lines = one line per symbol with - signature and [La-Lb] range at ~10% of the file's tokens; then read the symbols you need.""" + signature and [La-Lb] range at ~10% of the file's tokens; then read the symbols you need. + Paths OUTSIDE the project root are read too (sibling repo, extra working dir, installed + package source): tagged [c3-read:external], served from a transient record, never added + to this project's index or related facts. Access Guard rules still apply there.""" path_err = validate_file_path(file_path) if path_err: return f"[c3_read:error] {path_err}" diff --git a/cli/tools/_helpers.py b/cli/tools/_helpers.py index d2d0f76..edd7440 100644 --- a/cli/tools/_helpers.py +++ b/cli/tools/_helpers.py @@ -147,6 +147,40 @@ def finalize_with_tokens(finalize, svc, tool_name: str, args: dict, _GHOST_NAMES = _PYTHON_TYPE_NAMES | _HEREDOC_MARKERS +EXTERNAL_TAG = "[c3-read:external]" + + +def external_banner(path: str) -> str: + """The one line every outside-the-root response opens with. + + One definition so c3_read and c3_compress cannot drift apart on it. + """ + return (f"{EXTERNAL_TAG} {path} — outside the project root; served but " + f"not indexed, and absent from this project's search, map cache " + f"and related facts.\n") + + +def project_key(full, project_path) -> tuple[str, bool]: + """``(key, external)`` for an already-resolved-or-not path. + + Inside the root the key is the project-relative posix path — how + file_memory, the edit ledger and related facts all address a file. + Outside it there is no such key, so the absolute posix path stands in and + ``external`` tells the caller to serve from a transient record rather than + the index (docs/file-map.md § Outside the root). + + This used to be a bare ``relative_to`` whose ValueError escaped as a tool + crash. It is not a containment check and must not become one: Access Guard + already rules on the absolute canonical path, and it runs before this. + """ + resolved = Path(full).resolve() + try: + rel = resolved.relative_to(Path(project_path).resolve()) + return str(rel).replace("\\", "/"), False + except ValueError: + return resolved.as_posix(), True + + def validate_file_path(file_path: str) -> str | None: """Return an error message if file_path looks like a ghost-file path, else None. diff --git a/cli/tools/compress.py b/cli/tools/compress.py index 98c2b82..1582463 100644 --- a/cli/tools/compress.py +++ b/cli/tools/compress.py @@ -11,9 +11,10 @@ from pathlib import Path from cli.tools import _grants -from cli.tools._helpers import finalize_with_tokens, show_token_ratios +from cli.tools._helpers import external_banner, finalize_with_tokens, project_key, show_token_ratios from core import count_tokens from services import access_guard +from services.file_map import render_map #: Every mode name c3_compress ever accepted, and what to say about it now. RETIRED_MODES = { @@ -35,7 +36,8 @@ def deprecation_line(requested: str) -> str: def map_detail(svc, rel: str, requested_mode: str, *, cache_hit: bool, - symbols: int = 0, fallback: str = "") -> dict: + symbols: int = 0, fallback: str = "", record: dict = None, + external: bool = False) -> dict: """Telemetry `detail` for a map served from file_memory (C0, 2.120.0). backend names the code path, parser the extractor that built the record @@ -43,13 +45,15 @@ def map_detail(svc, rel: str, requested_mode: str, *, cache_hit: bool, whether the record was already fresh. `symbols` is how many the caller asked for and `fallback` why a read served a map instead of source. `deprecated_mode` (2.122.0) is set when a retired mode name was asked for. + `record` is a caller-supplied transient record (an outside-the-root read, + which is deliberately absent from file_memory) and `external` flags it. Flat so telemetry.aggregate_tool_telemetry can fold it without a schema. """ - record = None - try: - record = svc.file_memory.get(rel) - except Exception: - record = None + if record is None: + try: + record = svc.file_memory.get(rel) + except Exception: + record = None if not isinstance(record, dict): record = None detail = { @@ -65,6 +69,9 @@ def map_detail(svc, rel: str, requested_mode: str, *, cache_hit: bool, detail["fallback"] = fallback if requested_mode not in ("map", "read"): detail["deprecated_mode"] = requested_mode + if external: + detail["backend"] = "transient" + detail["external"] = True return detail @@ -136,20 +143,37 @@ def _compress_single(file_path: str, requested: str, note: str, svc, finalize) - if not full.exists(): return ("[file_map:error] not found. To create a new file use " "c3_edit(file_path=..., old_string='', new_string=).") - rel = str(full.resolve().relative_to(Path(svc.project_path).resolve())).replace("\\", "/") - # C0 measurement: was the record already fresh before this call? - cache_hit = not svc.file_memory.needs_update(rel) - res = _build_map(svc, rel) + resolved = full.resolve() + rel, external = project_key(resolved, svc.project_path) + # Outside the root the map comes from a transient record — same parser, + # never written to an index keyed by project-relative path + # (docs/file-map.md § Outside the root). c3_read does the same thing; the + # two map surfaces must not disagree about where a file may live. + banner = external_banner(rel) if external else "" + if external: + ext_record = svc.file_memory.build_transient_record(resolved, key=rel) + cache_hit = False + body = (render_map(ext_record, max_tokens=svc.file_memory.MAP_TOKEN_BUDGET) + if ext_record + else f"[file_map] Could not build map for {rel} — unreadable.") + else: + ext_record = None + # C0 measurement: was the record already fresh before this call? + cache_hit = not svc.file_memory.needs_update(rel) + body = _build_map(svc, rel) + res = banner + body raw_tokens = None map_tokens = 0 try: raw_text = full.read_text(encoding="utf-8", errors="replace") raw_tokens = count_tokens(raw_text) map_tokens = count_tokens(res) - if map_tokens >= raw_tokens and not res.startswith("[file_map]"): + if map_tokens >= raw_tokens and not body.startswith("[file_map]"): # A map that costs more than the file is worth nothing # (docs/file-map.md § Small files). - res = f"[compress:{file_path}] whole file — smaller than its map\n" + raw_text + res = (banner + + f"[compress:{file_path}] whole file — smaller than its map\n" + + raw_text) map_tokens = count_tokens(res) summary = "map" except Exception: @@ -162,7 +186,8 @@ def _compress_single(file_path: str, requested: str, note: str, svc, finalize) - res, summary, raw_tokens=raw_tokens, optimized_tokens=map_tokens or None, response_tokens=map_tokens, - detail=map_detail(svc, rel, requested, cache_hit=cache_hit)) + detail=map_detail(svc, rel, requested, cache_hit=cache_hit, + record=ext_record, external=external)) def _compress_batch(paths: list, requested: str, note: str, svc, finalize) -> str: @@ -195,10 +220,17 @@ def _do_one(fp): full = Path(fp) if not full.exists(): return fp, None, None, None, "not found" - rel = str(full.resolve().relative_to( - Path(svc.project_path).resolve())).replace("\\", "/") - res = svc.file_memory.get_or_build_map( - rel, max_tokens=svc.file_memory.MAP_TOKEN_BUDGET) + resolved = full.resolve() + rel, external = project_key(resolved, svc.project_path) + if external: + record = svc.file_memory.build_transient_record(resolved, key=rel) + res = external_banner(rel) + ( + render_map(record, max_tokens=svc.file_memory.MAP_TOKEN_BUDGET) + if record + else f"[file_map] Could not build map for {rel} — unreadable.") + else: + res = svc.file_memory.get_or_build_map( + rel, max_tokens=svc.file_memory.MAP_TOKEN_BUDGET) try: raw_tok = count_tokens(full.read_text(encoding="utf-8", errors="replace")) map_tok = count_tokens(res) diff --git a/cli/tools/read.py b/cli/tools/read.py index 300dce0..61b6bf0 100644 --- a/cli/tools/read.py +++ b/cli/tools/read.py @@ -7,10 +7,11 @@ from typing import Any from cli.tools import _grants -from cli.tools._helpers import finalize_with_tokens, maybe_related_facts +from cli.tools._helpers import external_banner, finalize_with_tokens, maybe_related_facts, project_key from cli.tools.compress import map_detail from core import count_tokens from services import access_guard +from services.file_map import render_map def _coerce_list(val: Any) -> list[str] | None: @@ -175,7 +176,21 @@ def _worker_finalize(name, args, resp, summ, **kw): if not full.exists(): return f"[read:error] File not found: {file_path}" - rel_path = str(full.resolve().relative_to(Path(svc.project_path).resolve())).replace("\\", "/") + # A path outside the project root is READ, not refused (docs/file-map.md + # § Outside the root). The root was never the control here: Access Guard + # ruled above and its rules match the absolute canonical path as well as + # the project-relative one, so the builtin denies still cover an outside + # file; and the PreToolUse hook stands down outside the root, so native + # Read already went there unimpeded. Refusing only pushed the agent off + # the budgeted, guarded tool onto the unbudgeted one. + # + # What the root DOES decide is indexing. file_memory is keyed by + # project-relative path and feeds the text index, the map cache and + # prune_stale, so an outside file is served from a transient record and + # never written into this project's store. + resolved = full.resolve() + rel_path, external = project_key(resolved, svc.project_path) + banner = external_banner(rel_path) if external else "" if full.is_dir(): # A directory maps to one line per file, budgeted @@ -184,7 +199,9 @@ def _worker_finalize(name, args, resp, summ, **kw): budget = DEFAULT_MAX_TOKENS if isinstance(lines, int) and not isinstance(lines, bool) and lines > 0: budget = lines - text, dir_detail = render_directory_map(svc, rel_path, max_tokens=budget) + text, dir_detail = render_directory_map(svc, rel_path, max_tokens=budget, + external=external) + text = banner + text tok = count_tokens(text) if finalize is None: return text @@ -228,15 +245,56 @@ def _add_range(start: int, end: int) -> None: # Ensure file_memory index is fresh. # When the watcher is running, it handles updates in the background — # only force-update if file_memory has no record at all (first access). - try: - watcher_active = (hasattr(svc, "watcher") and svc.watcher._observer.is_alive()) - if watcher_active: - if not svc.file_memory.get(rel_path): + # An outside-the-root file is never indexed; it gets a transient record + # instead, built lazily below. + if not external: + try: + watcher_active = (hasattr(svc, "watcher") and svc.watcher._observer.is_alive()) + if watcher_active: + if not svc.file_memory.get(rel_path): + svc.file_memory.update(rel_path) + elif svc.file_memory.needs_update(rel_path): svc.file_memory.update(rel_path) - elif svc.file_memory.needs_update(rel_path): - svc.file_memory.update(rel_path) - except Exception: - pass + except Exception: + pass + + _ext_cache = [] + + def _ext_record(): + """The transient record for an outside file — same parser and schema + as an indexed one, never persisted (docs/file-map.md § Outside the + root). + + Built at most once, and only where a map or a symbol lookup actually + needs it: a `lines=[a,b]` read must not pay for a full parse of the + file it is slicing two lines out of, which is the whole reason the + large-file bounds exist. + """ + if not external: + return None + if not _ext_cache: + _ext_cache.append( + svc.file_memory.build_transient_record(resolved, key=rel_path)) + return _ext_cache[0] + + def _file_map(max_tokens=None) -> str: + """The map, from the project index or from the transient record.""" + if not external: + return svc.file_memory.get_or_build_map(rel_path, max_tokens=max_tokens) + record = _ext_record() + if not record: + return f"[file_map] Could not build map for {rel_path} — unreadable." + return render_map(record, max_tokens=max_tokens) + + def _map_cached() -> bool: + return False if external else not svc.file_memory.needs_update(rel_path) + + def _facts() -> str: + # Facts are project knowledge keyed by project-relative path; an + # outside file has none, and matching one by basename would be a lie. + if external: + return "" + return maybe_related_facts(svc, rel_path, top_k=3, context="read") raw_text = full.read_text(encoding="utf-8", errors="replace") # EOL-normalize exactly the way c3_edit's matcher does (\r\n and \r → \n), @@ -256,7 +314,9 @@ def full_file_tokens(): return _full_tok_cache[0] if symbols: - matches = svc.file_memory.get_symbol_ranges(rel_path, symbols, return_matches=True) + matches = svc.file_memory.get_symbol_ranges(rel_path, symbols, + return_matches=True, + record=_ext_record()) # Check for ambiguity disambiguation_msgs = [] @@ -292,7 +352,7 @@ def full_file_tokens(): ranges.append(m["range"]) if '
' in symbols or '' in symbols: - record = svc.file_memory.get(rel_path) + record = _ext_record() or svc.file_memory.get(rel_path) if record and "sections" in record: covered = set() @@ -318,29 +378,31 @@ def _mark(secs): ranges.extend(main_ranges) if not ranges and symbols: - map_cached = not svc.file_memory.needs_update(rel_path) - file_map = svc.file_memory.get_or_build_map(rel_path) - resp = f"[read:{file_path}] symbols not found: {symbols}. Showing file map:\n{file_map}" + map_cached = _map_cached() + file_map = _file_map() + resp = (banner + + f"[read:{file_path}] symbols not found: {symbols}. " + f"Showing file map:\n{file_map}") map_tok = count_tokens(file_map) return finalize_with_tokens( finalize, svc, "c3_read", {"file": file_path, "symbols": symbols}, resp, f"{full_file_tokens()}->{map_tok}tok", raw_tokens=full_file_tokens(), optimized_tokens=map_tok, detail=map_detail(svc, rel_path, "read", cache_hit=map_cached, - symbols=len(symbols), fallback="symbols_not_found")) + symbols=len(symbols), fallback="symbols_not_found", + record=_ext_record(), external=external)) if not ranges: - map_cached = not svc.file_memory.needs_update(rel_path) - file_map = svc.file_memory.get_or_build_map( - rel_path, max_tokens=svc.file_memory.MAP_TOKEN_BUDGET) + map_cached = _map_cached() + file_map = _file_map(max_tokens=svc.file_memory.MAP_TOKEN_BUDGET) if count_tokens(file_map) >= full_file_tokens(): # A map that costs more than the file is worth nothing: serve the # file (docs/file-map.md § Small files). file_map = (f"[read:{file_path}] whole file — smaller than its map\n" + raw_text) - resp = (file_map + resp = (banner + file_map + "\n[map only — pass lines=[start,end] or symbols=[...] for exact source]" - + maybe_related_facts(svc, rel_path, top_k=3, context="read")) + + _facts()) map_tok = count_tokens(resp) if finalize is None: return resp @@ -350,7 +412,8 @@ def _mark(secs): raw_tokens=full_file_tokens(), optimized_tokens=map_tok, response_tokens=map_tok, detail=map_detail(svc, rel_path, "read", cache_hit=map_cached, - fallback="map_only")) + fallback="map_only", record=_ext_record(), + external=external)) else: # Sort and merge overlapping ranges ranges.sort() @@ -387,7 +450,7 @@ def _mark(secs): prev_end = end final_content = "\n".join(parts) - resp = final_content + maybe_related_facts(svc, rel_path, top_k=3, context="read") + resp = banner + final_content + _facts() tokens = count_tokens(resp) summary = f"{full_file_tokens()}->{tokens}tok" if tokens < full_file_tokens() else f"{tokens}tok" # C0 measurement: what the read asked for and how much source it took. @@ -398,6 +461,7 @@ def _mark(secs): "ranges": len(ranges), "lines_served": sum(e - s_ + 1 for s_, e in ranges), "file_lines": len(content_lines), + "external": external, } return finalize_with_tokens( finalize, svc, "c3_read", {"file": file_path, "symbols": symbols}, resp, summary, diff --git a/docs/file-map.md b/docs/file-map.md index 7705f26..8f4649e 100644 --- a/docs/file-map.md +++ b/docs/file-map.md @@ -159,3 +159,48 @@ symbol's signature, then rendering every symbol. lines and ends with `… N more symbols (map budget …)`; ask for the part you need with `symbols=[…]` or `lines=[a,b]`. (For a directory map, `lines=` sets the budget — a directory has no line numbers.) + +## Outside the root (2.129.0) + +`c3_read` reads a path outside the project root — a sibling repo, an +additional working directory, `~/.claude/CLAUDE.md`, installed package +source. Source, maps, symbol reads and directory maps all work there. + +Until 2.129.0 it did not: `relative_to` raised `ValueError` and the tool +call died with *"is not in the subpath of"*. That was never a control. Two +things make the boundary fictional as security: + +- Access Guard rules match the **absolute canonical path** as well as the + project-relative one (`Rule.matches` tests `canon`), so `**/.env*`, + `**/.c3/secrets.enc` and any user `deny` glob bind on an outside path + exactly as they do inside. The guard runs before path resolution and is + unchanged. +- The PreToolUse hook deliberately stands down outside the root (*"none of + this hook's business"*), so native `Read` already went there unimpeded. + +So the refusal protected nothing and only pushed the agent off the +budgeted, guarded, telemetered tool onto the one with none of that. It +also leaked: an existing outside file produced the subpath error while a +missing one produced `File not found`, making `c3_read` an existence +oracle for the whole filesystem — the exact distinguishable-response probe +Access Guard's R2 exists to prevent. + +What the root **does** decide is indexing. `file_memory` is keyed by +project-relative path and feeds the text index, the map cache and +`prune_stale`, so an outside file is served from a transient record +(`FileMemoryStore.build_transient_record`) — same parser, same schema, +never persisted, never looked up. A directory outside the root extracts +symbols the same transient way, under the same file/deadline/byte bounds. + +Every outside response opens with a banner: + +``` +[c3-read:external] C:/Users/me/other-repo/lib.py — outside the project root; served but not indexed, and absent from this project's search, map cache and related facts. +``` + +Related facts are suppressed there (they are project knowledge keyed by +project-relative path; matching one by basename would be a lie), and +telemetry carries `external: true`. + +Want a hard boundary anyway? That is an Access Guard rule, not a property +of the tool — the rest of the policy surface already lives there. diff --git a/services/dir_map.py b/services/dir_map.py index 2fc7943..14a786b 100644 --- a/services/dir_map.py +++ b/services/dir_map.py @@ -87,12 +87,44 @@ def _line_count(path: Path) -> int: return 0 -def render_directory_map(svc, rel_dir: str, *, max_tokens: int = DEFAULT_MAX_TOKENS) -> tuple: - """Return (text, detail) for the directory `rel_dir` (project-relative, posix).""" - root = Path(svc.project_path).resolve() - rel_dir = str(rel_dir).replace("\\", "/").strip("/") - target = (root / rel_dir).resolve() if rel_dir else root - label = (rel_dir + "/") if rel_dir else "./" +def render_directory_map(svc, rel_dir: str, *, max_tokens: int = DEFAULT_MAX_TOKENS, + external: bool = False) -> tuple: + """Return (text, detail) for the directory `rel_dir` (project-relative, posix). + + `external=True` maps a directory OUTSIDE the project root: `rel_dir` is an + absolute path and names are relative to it. Symbols are still extracted, + under the same file/deadline/byte bounds — but through transient records + that are never persisted or looked up, because an outside file has no + project-relative key and indexing it would poison a store the rest of C3 + addresses by that key. The edit ledger, which supplies the recency + ranking, likewise has nothing to say about a file outside the project. + """ + store = getattr(svc, "file_memory", None) + if external: + root = Path(rel_dir).resolve() + target = root + name_base = "" + label = root.as_posix().rstrip("/") + "/" + recent = {} + + def _cached(rel): + return None # nothing outside the root is ever in the store + + def _extract(rel): + return store.build_transient_record(root / rel, key=rel) + else: + root = Path(svc.project_path).resolve() + rel_dir = str(rel_dir).replace("\\", "/").strip("/") + target = (root / rel_dir).resolve() if rel_dir else root + name_base = rel_dir + label = (rel_dir + "/") if rel_dir else "./" + recent = _recent_edit_ranks(svc) + + def _cached(rel): + return store.get(rel) + + def _extract(rel): + return store.update(rel) files = [] capped = False @@ -104,9 +136,6 @@ def render_directory_map(svc, rel_dir: str, *, max_tokens: int = DEFAULT_MAX_TOK files.append(fp.resolve().relative_to(root).as_posix()) except ValueError: continue - - store = getattr(svc, "file_memory", None) - recent = _recent_edit_ranks(svc) started = time.monotonic() extracted = 0 rows = [] @@ -115,7 +144,7 @@ def render_directory_map(svc, rel_dir: str, *, max_tokens: int = DEFAULT_MAX_TOK record = None if store is not None: try: - record = store.get(rel) + record = _cached(rel) except Exception: record = None ext = Path(rel).suffix.lower() @@ -124,7 +153,7 @@ def render_directory_map(svc, rel_dir: str, *, max_tokens: int = DEFAULT_MAX_TOK and time.monotonic() - started < EXTRACT_DEADLINE_S): try: if (root / rel).stat().st_size <= MAX_EXTRACT_BYTES: - record = store.update(rel) + record = _extract(rel) extracted += 1 except Exception: record = None @@ -135,7 +164,7 @@ def render_directory_map(svc, rel_dir: str, *, max_tokens: int = DEFAULT_MAX_TOK symbols = _top_symbols(record) rows.append({ "rel": rel, - "name": rel[len(rel_dir) + 1:] if rel_dir and rel.startswith(rel_dir + "/") else rel, + "name": rel[len(name_base) + 1:] if name_base and rel.startswith(name_base + "/") else rel, "lines": lines, "lang": lang, "symbols": symbols, @@ -172,5 +201,6 @@ def render_directory_map(svc, rel_dir: str, *, max_tokens: int = DEFAULT_MAX_TOK "listed": len(kept), "extracted": extracted, "capped": capped, + "external": bool(external), } return text, detail diff --git a/services/file_memory.py b/services/file_memory.py index 77a7d54..c2da06a 100644 --- a/services/file_memory.py +++ b/services/file_memory.py @@ -179,12 +179,28 @@ def update(self, rel_path: str, ai_summary: str = None) -> Optional[dict]: # If we are here, we are forcing a fresh extraction sections, parser = self._extract_sections_with_parser(full_path, content) + record = self._record_dict(rel_path, ext, content_hash, len(lines), + stat, sections, parser) + self._save(rel_path, record) + self._cache_map(rel_path, record) + with self._search_lock: + if not self._search_dirty: + self._search_index.add_or_update(rel_path, self._search_doc(record)) + return record + + def _record_dict(self, key: str, ext: str, content_hash: str, line_count: int, + stat, sections: list, parser: str) -> dict: + """The record schema, in one place. + + Shared by update() and build_transient_record() so a field added to + one can never be missing from the other. + """ record = { - "path": rel_path, + "path": str(key).replace("\\", "/"), "content_hash": content_hash, "updated_at": time.strftime("%Y-%m-%dT%H:%M:%S"), - "lines": len(lines), + "lines": line_count, "size_bytes": stat.st_size, "mtime_ns": getattr(stat, "st_mtime_ns", int(stat.st_mtime * 1_000_000_000)), "language": LANG_MAP.get(ext, ext.lstrip('.')), @@ -199,14 +215,31 @@ def update(self, rel_path: str, ai_summary: str = None) -> Optional[dict]: if shape.get("minified") or shape.get("generated") or shape.get("oversized"): record["shape"] = {k: shape[k] for k in ("minified", "generated", "oversized", "longest_line", "bytes") if k in shape} - - self._save(rel_path, record) - self._cache_map(rel_path, record) - with self._search_lock: - if not self._search_dirty: - self._search_index.add_or_update(rel_path, self._search_doc(record)) return record + def build_transient_record(self, full_path, key: str = "") -> Optional[dict]: + """Extract a structural record WITHOUT persisting or indexing it. + + For a file outside the project root (docs/file-map.md § Outside the + root): this store is keyed by project-relative path, and its records + feed the text index, the map cache and prune_stale. An absolute key + would poison all three, and prune_stale would churn on a path it can + never resolve. Same extractor and same schema as update(), minus the + lifecycle: built, used once, thrown away. + """ + full_path = Path(full_path) + try: + stat = full_path.stat() + content = full_path.read_text(encoding="utf-8", errors="replace") + except Exception: + return None + sections, parser = self._extract_sections_with_parser(full_path, content) + return self._record_dict(key or full_path.as_posix(), + full_path.suffix.lower(), + hashlib.md5(content.encode()).hexdigest(), + len(content.splitlines()), + stat, sections, parser) + def get_map(self, rel_path: str) -> Optional[str]: """Return a formatted structural map for Claude consumption. @@ -274,12 +307,18 @@ def needs_update(self, rel_path: str) -> bool: except Exception: return True - def get_symbol_ranges(self, rel_path: str, symbol_names: list[str], return_matches: bool = False) -> list: + def get_symbol_ranges(self, rel_path: str, symbol_names: list[str], + return_matches: bool = False, + record: Optional[dict] = None) -> list: """Resolve symbol names to line ranges (1-indexed). Supports exact match and substring/partial match (e.g. 'handle_req' matches 'handle_request_data'). Supports exact regex if anchored (e.g. '^cmd_benchmark$'). + + `record` supplies a transient record for a file that is not in this + store and must not be — an outside-the-root read builds one in memory. """ - record = self.get(rel_path) + if record is None: + record = self.get(rel_path) if not record or "sections" not in record: return [] diff --git a/tests/test_read_outside_root.py b/tests/test_read_outside_root.py new file mode 100644 index 0000000..0d94898 --- /dev/null +++ b/tests/test_read_outside_root.py @@ -0,0 +1,289 @@ +"""c3_read outside the project root — served, not refused. + +Before this, an outside path raised ValueError out of `relative_to` and the +whole tool call died with "is not in the subpath of". That was never a +security control: Access Guard already rules on the ABSOLUTE canonical path +(Rule.matches tests `canon` as well as `rel`), and the PreToolUse hook +deliberately stands down outside the root, so native Read went there +unimpeded. The crash only pushed the agent off the budgeted, guarded tool. + +What the root still decides is INDEXING. file_memory is keyed by +project-relative path and feeds the text index, the map cache and +prune_stale, so an outside file is served from a transient record and must +never land in this project's store. These tests pin both halves. +""" +from __future__ import annotations + +import json +import sys +import tempfile +import unittest +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import MagicMock, patch + +REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT)) + +from cli.tools._helpers import project_key # noqa: E402 +from cli.tools.compress import handle_compress # noqa: E402 +from cli.tools.read import handle_read # noqa: E402 +from services import access_guard as ag # noqa: E402 +from services.file_memory import FileMemoryStore # noqa: E402 + +BANNER = "[c3-read:external]" +DENIED = "[c3-access:denied]" + + +def _finalize(name, args, resp, summ="", **kw): + return resp + + +def _py(n_funcs: int, prefix: str) -> str: + parts = [f"class {prefix.title()}Thing:\n def run(self):\n return 1\n\n"] + for i in range(n_funcs): + parts.append(f"def {prefix}_{i}(x):\n return x + {i}\n\n") + return "".join(parts) + + +class _Base(unittest.TestCase): + """A project and a sibling directory that is NOT inside it.""" + + def setUp(self): + self._proj_tmp = tempfile.TemporaryDirectory() + self._out_tmp = tempfile.TemporaryDirectory() + self.proj = Path(self._proj_tmp.name) + self.outside = Path(self._out_tmp.name) + (self.proj / ".c3").mkdir() + # Isolate from the developer's real ~/.c3 rules — an outside path is + # exactly what a personal global rule is most likely to cover. + self._home = patch.object(ag, "_global_base", return_value=None) + self._home.start() + self.svc = SimpleNamespace( + project_path=str(self.proj), file_memory=FileMemoryStore(str(self.proj)), + edit_ledger=None, session_mgr=None, memory=None, hybrid_config={}) + + def tearDown(self): + self._home.stop() + self._proj_tmp.cleanup() + self._out_tmp.cleanup() + + def _read(self, path, symbols=None, lines=None): + return handle_read(str(path), symbols, lines, True, self.svc, _finalize) + + def _indexed(self): + """Records actually persisted in the project's file_memory store.""" + store = self.proj / ".c3" / "file_memory" + return sorted(p.name for p in store.glob("*.json") + if not p.name.startswith("_")) + + +class TestOutsideFileIsServed(_Base): + def test_source_read_by_lines(self): + target = self.outside / "lib.py" + target.write_text("alpha = 1\nbeta = 2\ngamma = 3\n", encoding="utf-8") + out = self._read(target, lines=[2, 3]) + self.assertIn("beta = 2", out) + self.assertIn("gamma = 3", out) + self.assertNotIn("alpha = 1", out) + + def test_map_lists_symbols(self): + target = self.outside / "wide.py" + target.write_text(_py(12, "out"), encoding="utf-8") + out = self._read(target) + self.assertIn("OutThing", out) + self.assertIn("out_0", out) + self.assertIn("[map only", out) + + def test_symbol_read_returns_that_symbol(self): + target = self.outside / "wide.py" + target.write_text(_py(12, "out"), encoding="utf-8") + out = self._read(target, symbols=["out_7"]) + self.assertIn("def out_7(x):", out) + self.assertNotIn("def out_3(x):", out) + + def test_unknown_symbol_falls_back_to_the_map(self): + target = self.outside / "wide.py" + target.write_text(_py(12, "out"), encoding="utf-8") + out = self._read(target, symbols=["nope_not_here"]) + self.assertIn("symbols not found", out) + self.assertIn("out_0", out) + + def test_directory_map(self): + (self.outside / "a.py").write_text(_py(4, "aa"), encoding="utf-8") + (self.outside / "b.py").write_text(_py(4, "bb"), encoding="utf-8") + out = self._read(self.outside) + self.assertIn("a.py", out) + self.assertIn("b.py", out) + self.assertIn("AaThing", out) + + def test_missing_outside_file_still_reports_not_found(self): + out = self._read(self.outside / "nope.py") + self.assertIn("File not found", out) + + def test_batch_mixes_inside_and_outside(self): + (self.proj / "inside.py").write_text(_py(3, "ins"), encoding="utf-8") + (self.outside / "out.py").write_text(_py(3, "out"), encoding="utf-8") + out = handle_read(f"inside.py,{self.outside / 'out.py'}", + None, None, True, self.svc, _finalize) + self.assertIn("InsThing", out) + self.assertIn("OutThing", out) + + +class TestOutsideReadIsLabelled(_Base): + def test_banner_names_the_absolute_path(self): + target = self.outside / "lib.py" + target.write_text("x = 1\n", encoding="utf-8") + out = self._read(target, lines=[1, 1]) + self.assertTrue(out.startswith(BANNER), out[:120]) + self.assertIn(target.resolve().as_posix(), out.splitlines()[0]) + + def test_banner_on_map_and_directory_too(self): + target = self.outside / "wide.py" + target.write_text(_py(12, "out"), encoding="utf-8") + self.assertTrue(self._read(target).startswith(BANNER)) + self.assertTrue(self._read(self.outside).startswith(BANNER)) + + def test_inside_reads_carry_no_banner(self): + target = self.proj / "inside.py" + target.write_text(_py(12, "ins"), encoding="utf-8") + self.assertNotIn(BANNER, self._read(target)) + self.assertNotIn(BANNER, self._read(target, symbols=["ins_2"])) + self.assertNotIn(BANNER, self._read(self.proj)) + + +class TestOutsideReadNeverIndexes(_Base): + """The whole reason outside files get a transient record.""" + + def test_file_reads_write_nothing_to_the_store(self): + target = self.outside / "wide.py" + target.write_text(_py(12, "out"), encoding="utf-8") + self._read(target) + self._read(target, symbols=["out_1"]) + self._read(target, lines=[1, 3]) + self.assertEqual(self._indexed(), []) + self.assertEqual(self.svc.file_memory.list_tracked(), []) + + def test_directory_map_writes_nothing_to_the_store(self): + (self.outside / "a.py").write_text(_py(6, "aa"), encoding="utf-8") + (self.outside / "b.py").write_text(_py(6, "bb"), encoding="utf-8") + self._read(self.outside) + self.assertEqual(self._indexed(), []) + + def _spy_on_extraction(self): + calls = [] + real = self.svc.file_memory.build_transient_record + + def spy(*a, **kw): + calls.append(a) + return real(*a, **kw) + + self.svc.file_memory.build_transient_record = spy + return calls + + def test_a_line_read_never_parses_the_file(self): + """A slice must not pay for a full parse it never uses — the whole + reason the large-file bounds exist.""" + target = self.outside / "wide.py" + target.write_text(_py(12, "out"), encoding="utf-8") + calls = self._spy_on_extraction() + out = self._read(target, lines=[1, 2]) + self.assertEqual(calls, []) + self.assertIn("OutThing", out) + + def test_a_map_read_parses_exactly_once(self): + target = self.outside / "wide.py" + target.write_text(_py(12, "out"), encoding="utf-8") + calls = self._spy_on_extraction() + self._read(target) + self.assertEqual(len(calls), 1) + + def test_a_symbol_read_parses_exactly_once(self): + target = self.outside / "wide.py" + target.write_text(_py(12, "out"), encoding="utf-8") + calls = self._spy_on_extraction() + self._read(target, symbols=["out_3"]) + self.assertEqual(len(calls), 1) + + def test_inside_reads_still_index(self): + target = self.proj / "inside.py" + target.write_text(_py(12, "ins"), encoding="utf-8") + self._read(target) + self.assertEqual(len(self._indexed()), 1) + self.assertEqual(self.svc.file_memory.list_tracked(), ["inside.py"]) + + +class TestCompressAgrees(_Base): + """c3_compress serves the same map; the two surfaces must not disagree + about where a file is allowed to live — it had the identical crash.""" + + def test_single_file_map(self): + target = self.outside / "wide.py" + target.write_text(_py(12, "out"), encoding="utf-8") + out = handle_compress(str(target), "map", self.svc, _finalize, None) + self.assertTrue(out.startswith(BANNER), out[:120]) + self.assertIn("out_0", out) + self.assertEqual(self._indexed(), []) + + def test_batch_mixes_inside_and_outside(self): + (self.proj / "inside.py").write_text(_py(12, "ins"), encoding="utf-8") + (self.outside / "out.py").write_text(_py(12, "out"), encoding="utf-8") + out = handle_compress(f"inside.py,{self.outside / 'out.py'}", "map", + self.svc, _finalize, None) + self.assertIn("ins_0", out) + self.assertIn("out_0", out) + self.assertNotIn("not in the subpath", out) + self.assertEqual(self.svc.file_memory.list_tracked(), ["inside.py"]) + + +class TestProjectKey(unittest.TestCase): + def test_inside_is_relative_and_not_external(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + (root / "pkg").mkdir() + key, external = project_key(root / "pkg" / "a.py", str(root)) + self.assertEqual(key, "pkg/a.py") + self.assertFalse(external) + + def test_dotdot_escape_is_external_and_absolute(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) / "proj" + root.mkdir() + key, external = project_key(root / ".." / "sibling.py", str(root)) + self.assertTrue(external) + self.assertTrue(key.endswith("/sibling.py")) + self.assertNotIn("..", key) + + +class TestGuardStillRulesOutsideTheRoot(_Base): + """Serving outside paths must not become a way around Access Guard.""" + + def _mock_svc(self): + svc = MagicMock() + svc.project_path = str(self.proj) + svc.hybrid_config = {} + svc.session_mgr = None + svc.edit_ledger = None + svc.memory = None + return svc + + def test_builtin_env_deny_binds_on_an_outside_path(self): + secret = self.outside / ".env" + secret.write_text("TOPSECRET=1\n", encoding="utf-8") + resp = handle_read(str(secret), svc=self._mock_svc(), finalize=_finalize) + self.assertIn(DENIED, resp) + self.assertNotIn("TOPSECRET", resp) + + def test_project_deny_rule_binds_on_an_outside_path(self): + (self.proj / ".c3" / "config.json").write_text( + json.dumps({"access": {"deny": ["**/keys/**"]}}), encoding="utf-8") + (self.outside / "keys").mkdir() + target = self.outside / "keys" / "id.txt" + target.write_text("TOPSECRET\n", encoding="utf-8") + resp = handle_read(str(target), svc=self._mock_svc(), finalize=_finalize) + self.assertIn(DENIED, resp) + self.assertNotIn("TOPSECRET", resp) + + +if __name__ == "__main__": + unittest.main() From b5a1f0fda53f6e7436368d30442a84e2ce1826fa Mon Sep 17 00:00:00 2001 From: drknowhow Date: Wed, 9 Sep 2026 12:01:28 -0400 Subject: [PATCH 2/4] fix(ci): report an act setup failure as one, not as "0 parsed failures" (#173) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit act can fail BEFORE any workflow step runs: the runner image cannot be pulled, the daemon refuses, the container never starts. ci_runner handed that log to ci_failures.parse like any other failure, so it came back parser: unparsed with a log tail — and c3_ci(action='failures') answered "0 parsed failures" for what was really "the runner image could not be obtained". ci_act.setup_failure() recognises it from act's own "Failure - Set up job" marker and names the cause: registry auth (with the docker logout fix), missing image, unreachable daemon, out of disk, or the daemon's own words. The job is reported with parser: container-setup and that reason. Status stays FAILED — a job that could not run is never a green light, and demoting it to a not-run status would change what PARTIAL_PASS means. A step failure is explicitly NOT a setup failure: the marker gates it, so a job that reached its steps still goes to the parsers and pytest or ruff output inside a container still yields {file,line,message}. c3_ci(action='doctor') now reports whether the runner image is present locally, without pulling ~1 GB to find out. Note what a pre-flight probe cannot do, and why the classification has to live in the runner: in the reported case the image WAS present and `docker pull` succeeded, while act's own pull failed because act sends the stored registry credential where the CLI sends none for a public image. Only a real act run shows that. tests/test_ci_act.py's one container test now skips with that reason instead of failing on `parser != 'ruff'` when no step ever ran, which is the reported symptom. The classification itself is pinned by unit tests against the exact log from the issue; the live symptom does not currently reproduce on this machine, so the fix is verified against that log rather than against the failure. Claude-Session: https://claude.ai/code/session_01H2xJVonLH3ZKjDUQa4h3eS --- cli/tools/ci.py | 9 +++++ services/ci_act.py | 60 +++++++++++++++++++++++++++++++++ services/ci_runner.py | 11 ++++++ tests/test_ci_act.py | 78 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 158 insertions(+) diff --git a/cli/tools/ci.py b/cli/tools/ci.py index 698e445..355d3ae 100644 --- a/cli/tools/ci.py +++ b/cli/tools/ci.py @@ -385,6 +385,15 @@ def handle_ci(action: str, job: str, run_id: str, allow_foreign: bool, f" runs Linux jobs in a container " f"({ci_act.DEFAULT_IMAGE}), real `uses:` actions included", ] + if state.get("warning"): + lines.append(f" WARNING: {state['warning']}") + # An image that is present locally is still not proof act can pull + # it: act sends the stored registry credential where the CLI sends + # none, so an expired token fails a public pull (issue #173). Say + # so rather than let `available` read as a guarantee. + lines.append(" a runner failure before any step runs is " + "reported as a container-setup failure, not as 0 " + "parsed failures") else: lines += [" act UNAVAILABLE", f" {state['reason']}"] lines += [ diff --git a/services/ci_act.py b/services/ci_act.py index 78def5e..c6ca185 100644 --- a/services/ci_act.py +++ b/services/ci_act.py @@ -116,6 +116,19 @@ def availability() -> dict: info["docker"] = True info["docker_version"] = out.strip().splitlines()[-1] if out.strip() else "" info["ok"] = True + + # Is the default runner image already local? Absent, the first act run + # pulls ~1 GB, which is worth warning about but is not unreadiness. This + # probe deliberately does NOT pull: it cannot, in any case, predict the + # failure in issue #173 — there the image is present and `docker pull` + # succeeds, while act's own pull fails because act sends the stored + # registry credential and the CLI does not. Only a real act run shows + # that, which is why the runner classifies it (see setup_failure). + rc, _ = _probe(["docker", "image", "inspect", DEFAULT_IMAGE], timeout=20) + info["image_present"] = rc == 0 + if rc != 0: + info["warning"] = (f"runner image {DEFAULT_IMAGE} is not present " + "locally; the first act run pulls ~1 GB.") return info @@ -243,6 +256,53 @@ def build_command(inst, project, act_path: str, event: str = "", _ACT_OUTPUT_LINE = re.compile(r"^\[[^\]]*\]\s{0,3}\|\s?(.*)$") +# ── Setup-phase failures ──────────────────────────────────────────────────── +# act can fail BEFORE any workflow step runs: the runner image cannot be +# pulled, the daemon refuses, the container never starts. Nothing the failure +# parsers look for was ever printed, so parsing that log yields `unparsed` and +# `c3_ci(action='failures')` answers "0 parsed failures" — for what is really +# "the runner image could not be obtained" (issue #173). Classify it instead. +SETUP_PARSER = "container-setup" + +_SETUP_FAILED = re.compile(r"(?:❌\s*)?Failure\s+-\s+Set up job", re.I) +_DAEMON_ERROR = re.compile(r"Error response from daemon:\s*(.+)", re.I) +_SETUP_REASONS = ( + (re.compile(r"authentication required|incorrect username or password", re.I), + "the Docker registry rejected the runner image pull with an " + "authentication error. act sends the stored Docker credential where the " + "CLI sends none for a public image, so an expired token turns a public " + "pull into an auth failure — `docker logout` clears it."), + (re.compile(r"pull access denied|repository does not exist|manifest unknown", re.I), + "the runner image could not be found in the registry."), + (re.compile(r"Cannot connect to the Docker daemon|daemon is not running|" + r"docker daemon is not running", re.I), + "the Docker daemon is not reachable — start Docker and retry."), + (re.compile(r"no space left on device", re.I), + "the Docker host ran out of disk space pulling the runner image."), +) + + +def setup_failure(text: str) -> str: + """Why act failed before any workflow step ran, or '' if it got that far. + + The verdict marker is act's own `Failure - Set up job`; the patterns after + it only supply a reason a human can act on. A job that reached its steps + and failed there is NOT a setup failure and must still go to the parsers. + """ + text = text or "" + if not _SETUP_FAILED.search(text): + return "" + for pattern, reason in _SETUP_REASONS: + if pattern.search(text): + return f"act could not start the runner container: {reason}" + daemon = _DAEMON_ERROR.search(text) + if daemon: + return ("act could not start the runner container — the Docker daemon " + f"said: {daemon.group(1).strip()[:200]}") + return ("act failed during 'Set up job': the runner container never " + "started, so no workflow step ran.") + + def program_output(text: str) -> str: """Just what the commands printed, with act's prefixes removed. diff --git a/services/ci_runner.py b/services/ci_runner.py index 7a55194..6fc0601 100644 --- a/services/ci_runner.py +++ b/services/ci_runner.py @@ -413,6 +413,17 @@ def _run_job_act(inst, project: Path, run_dir: Path, event: str, pass if result.status in (FAILED, TIMEOUT): + # A setup-phase failure has no program output to parse: the container + # never started. Parsing it anyway reported `unparsed` with a log + # tail, so `c3_ci(action='failures')` answered "0 parsed failures" for + # a broken runner (issue #173). Say what actually happened. The status + # stays FAILED — a job that could not run is never a green light. + setup_reason = ci_act.setup_failure(outcome.get("output") or "") + if setup_reason: + result.reason = setup_reason + result.parser = ci_act.SETUP_PARSER + result.failures = [] + return result # Parse what the commands printed, not act's narration around it — # otherwise every file path arrives wearing a `[CI/lint] |` prefix. parsed = ci_failures.parse( diff --git a/tests/test_ci_act.py b/tests/test_ci_act.py index 57e31d8..f3ef0b5 100644 --- a/tests/test_ci_act.py +++ b/tests/test_ci_act.py @@ -455,6 +455,77 @@ def test_cross_os_fidelity_cannot(self): self.assertEqual(res.verdict, cr.PARTIAL_PASS) +SETUP_FAIL_LOG = ( + "[CI/lint] \U0001f680 Start image=catthehacker/ubuntu:act-latest\n" + "[CI/lint] \U0001f433 docker pull image=catthehacker/ubuntu:act-latest " + "platform= username= forcePull=true\n" + "[CI/lint] using DockerAuthConfig authentication for docker pull\n" + "[CI/lint] Error response from daemon: authentication required - " + "incorrect username or password\n" + "[CI/lint] \u274c Failure - Set up job\n" + "[CI/lint] \U0001f3c1 Job failed\n" +) + + +class TestSetupFailureClassification(unittest.TestCase): + """Issue #173. A failure before any step ran has no program output, so + parsing it reported `unparsed` and c3_ci(action='failures') answered + "0 parsed failures" for a runner that never started.""" + + def test_auth_failure_names_the_cause_and_the_fix(self): + reason = ci_act.setup_failure(SETUP_FAIL_LOG) + self.assertIn("authentication", reason.lower()) + self.assertIn("docker logout", reason) + + def test_daemon_error_without_a_known_reason_echoes_the_daemon(self): + log = ("[CI/x] Error response from daemon: no such host\n" + "[CI/x] \u274c Failure - Set up job\n") + self.assertIn("no such host", ci_act.setup_failure(log)) + + def test_dead_daemon_is_named(self): + log = ("[CI/x] Cannot connect to the Docker daemon at " + "npipe:////./pipe/docker_engine\n" + "[CI/x] \u274c Failure - Set up job\n") + self.assertIn("daemon is not reachable", ci_act.setup_failure(log)) + + def test_a_step_failure_is_not_a_setup_failure(self): + """The job reached its steps; its output must still go to the parsers.""" + log = ("[CI/lint] \u2705 Success - Set up job\n" + "[CI/lint] | app/thing.py:14:5: F401 unused import\n" + "[CI/lint] \u274c Failure - Main run\n") + self.assertEqual(ci_act.setup_failure(log), "") + + def test_empty_log_is_not_a_setup_failure(self): + self.assertEqual(ci_act.setup_failure(""), "") + + +class TestSetupFailureReporting(ActTempProject): + def test_the_runner_reports_it_instead_of_unparsed(self): + workflow(self.tmp, """ +name: CI +on: [push] +jobs: + lint: + runs-on: ubuntu-latest + steps: + - run: echo hi +""") + with mock.patch.object(ci_act, "availability", return_value={"ok": True}), \ + mock.patch.object(ci_act, "run_job", + return_value={"exit_code": 1, + "output": SETUP_FAIL_LOG, + "timed_out": False, + "duration_ms": 9, "command": "act"}): + res = cr.run_ci(self.tmp, engine="act", event="push") + job = res.jobs[0] + self.assertEqual(job.parser, ci_act.SETUP_PARSER) + self.assertEqual(job.failures, []) + self.assertIn("authentication", job.reason.lower()) + # A runner that could not start is never a green light. + self.assertEqual(job.status, cr.FAILED) + self.assertEqual(res.verdict, cr.FAIL) + + @unittest.skipUnless(ACT_READY, f"act+Docker unavailable: {ACT_STATE.get('reason')}") class TestActIntegration(ActTempProject): """The one test that really starts a container.""" @@ -475,6 +546,13 @@ def test_runs_a_linux_job_and_reports_a_clean_failure(self): capture_output=True) res = cr.run_ci(self.tmp, engine="act", event="push") job = res.jobs[0] + if job.parser == ci_act.SETUP_PARSER: + # Nothing this test asserts could have been produced: no step ran. + # ACT_READY cannot predict this — on the box in issue #173 the + # image is present and `docker pull` succeeds, while act's own + # pull carries the stored registry credential and fails. Skipping + # here is the honest answer; failing on `parser != 'ruff'` was not. + self.skipTest(f"act could not start a runner container: {job.reason}") self.assertEqual(job.status, cr.FAILED) self.assertEqual(job.engine, "act") self.assertEqual(job.fidelity, cr.FIDELITY_CONTAINER) From 91de876a1945d8997055800de6e8fc0a8a731c94 Mon Sep 17 00:00:00 2001 From: drknowhow Date: Wed, 9 Sep 2026 12:01:37 -0400 Subject: [PATCH 3/4] chore(release): 2.129.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Version bumped in BOTH sites (pyproject.toml and cli/c3.py __version__) — test_version_sync exists because v2.56.0 shipped with them apart. Claude-Session: https://claude.ai/code/session_01H2xJVonLH3ZKjDUQa4h3eS --- CHANGELOG.md | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++ cli/c3.py | 2 +- pyproject.toml | 2 +- 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1daa903..9f3a9d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,78 @@ All notable changes to Code Context Control (C3) are documented here. The format is loosely based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.129.0] - 2026-09-09 + +### Fixed — a broken act runner no longer reports "0 parsed failures" (#173) + +`act` can fail *before any workflow step runs*: the runner image cannot be +pulled, the daemon refuses, the container never starts. `ci_runner` handed +that log to `ci_failures.parse` like any other failure, so it came back +`parser: unparsed` with a log tail — and `c3_ci(action='failures')` answered +"0 parsed failures" for what was really "the runner image could not be +obtained". + +`ci_act.setup_failure()` now recognises it from act's own +`Failure - Set up job` marker and names the cause: registry auth, missing +image, dead daemon, or the daemon's own words. The job is reported with +`parser: container-setup` and that reason. Status stays `FAILED` — a job +that could not run is never a green light. + +`c3_ci(action='doctor')` reports whether the runner image is present locally +(without pulling ~1 GB to find out) and says what a setup failure looks like. +Note what a pre-flight probe **cannot** do: in the reported case the image +was present and `docker pull` succeeded, while act's own pull failed because +act sends the stored registry credential where the CLI sends none. Only a +real act run surfaces that, which is why the classification lives in the +runner. `tests/test_ci_act.py`'s one container test now skips with that +reason instead of failing on `parser != 'ruff'` when no step ever ran. + +### Fixed — `c3_read` reads outside the project root instead of crashing + +A path outside the project root did not get a refusal. It got an uncaught +`ValueError` out of `Path.relative_to` in `handle_read`, surfaced as +`Error calling tool 'c3_read': '…' is not in the subpath of '…'`. Nothing +in the tool ever checked containment: `validate_file_path` is a ghost-path +heuristic (type names, heredoc markers, redirect artifacts) and +`access_guard.canonicalize` returns `rel=""` with **no denial** for an +outside path, which `tests/test_access_guard.py` has always asserted. + +The accidental boundary was worth nothing as a control: + +- Access Guard rules match the **absolute canonical path** as well as the + project-relative one (`Rule.matches` tests `canon`), so `**/.env*`, + `**/.c3/secrets.enc` and user `deny` globs already bound on outside + paths. That is where the protection lives, and it is unchanged. +- The PreToolUse hook deliberately stands down outside the root, so native + `Read` went there unimpeded. Refusing only pushed the agent off the + budgeted, guarded, telemetered tool onto the one with none of that. + +It also leaked. `full.exists()` was checked *before* the throw, so an +existing outside file produced the subpath error and a missing one produced +`File not found` — a distinguishable-response probe over the whole +filesystem, which is exactly what the R2 rule quoted three lines above that +code exists to prevent. + +Outside paths are now read: source, maps, symbol reads and directory maps. +Access Guard still runs first, unchanged. What the root still decides is +**indexing** — `file_memory` is keyed by project-relative path and feeds the +text index, the map cache and `prune_stale`, so an outside file is served +from a transient record (`FileMemoryStore.build_transient_record`): same +parser, same schema, never persisted, never looked up. Directory maps +outside the root extract symbols the same transient way, under the same +file/deadline/byte bounds. + +Every outside response opens with +`[c3-read:external] — outside the project root; served but not +indexed, …`. Related facts are suppressed there (they are project knowledge +keyed by project-relative path), and telemetry carries `external: true`. + +A hard root boundary is available as an Access Guard rule; it is not a +property of the tool. See `docs/file-map.md` § Outside the root. + +`c3_edit` already tolerated outside paths (it falls back to `rel = file_path` +for the ledger) and is unchanged here. + ## [2.128.1] - 2026-09-08 Both of these were found by shipping 2.128.0 and watching the runtime, not diff --git a/cli/c3.py b/cli/c3.py index 7858f8e..d44e3d1 100644 --- a/cli/c3.py +++ b/cli/c3.py @@ -91,7 +91,7 @@ # Config CONFIG_DIR = ".c3" CONFIG_FILE = ".c3/config.json" -__version__ = "2.128.1" +__version__ = "2.129.0" def _compress_file_cli(compressor, path, mode="smart", **kw): diff --git a/pyproject.toml b/pyproject.toml index d0e66b7..998177a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "code-context-control" -version = "2.128.1" +version = "2.129.0" description = "Local MCP code-intelligence for AI coding tools: surgical search/read/edit, agent-config version history, path-level access + masking guards, and a multi-project hub." readme = "README.md" requires-python = ">=3.10" From c25d4c17d10a4016e1c4b4d6f3a7de44c4cb3ac0 Mon Sep 17 00:00:00 2001 From: drknowhow Date: Wed, 9 Sep 2026 12:01:38 -0400 Subject: [PATCH 4/4] docs: research note comparing repository mapping systems Landscape survey of file/repo mapping approaches for coding agents (Aider's tree-sitter + PageRank repo map, MCP graph servers, IDE symbol indexers, static ARCHITECTURE.md generators, plain tree dumps) with a feature matrix. Background reading for C3's own file-map work; it describes other systems and asserts nothing about C3's behaviour. Claude-Session: https://claude.ai/code/session_01H2xJVonLH3ZKjDUQa4h3eS --- agent_file_map_systems.md | 76 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 agent_file_map_systems.md diff --git a/agent_file_map_systems.md b/agent_file_map_systems.md new file mode 100644 index 0000000..e447dcc --- /dev/null +++ b/agent_file_map_systems.md @@ -0,0 +1,76 @@ +# Repository Mapping Systems for AI Coding Agents: Summary & Comparison + +> **Purpose:** Evaluation and ranking of file-folder and repository mapping systems to optimize agent contextual awareness, token efficiency, and codebase navigation. + +--- + +## 🏆 Summary Ranking & Architectural Tiers + +``` +[ Tier 1: AST & Graph-Driven (Gold Standard) ] + ├── 1. Aider Repo Map (Tree-sitter + PageRank) + └── 2. MCP Graph Servers (e.g., repo-graph, Codebase-Memory) + +[ Tier 2: IDE & Subagent Indexing Engines ] + ├── 3. Claude Code Native Indexer (Subagent Context Architecture) + └── 4. Cursor / Copilot Workspace Symbol Maps + +[ Tier 3: Static & Baseline Mapping ] + ├── 5. Static Generator CLIs (e.g., repo-map / Annotated Markdown) + └── 6. Standard Directory Trees (`tree` / Shell Script Piping) +``` + +--- + +## 📊 Feature & Performance Matrix + +| Tool / System | Architecture Type | Semantic Depth | Token Efficiency | Execution Model | Best Use Case | +| :--- | :--- | :--- | :--- | :--- | :--- | +| **Aider Repo Map** | Native AST + Graph | **Highest** (Signatures + Call Graphs) | **Extreme** (~1k token cap) | Pre-prompt Graph Truncation | CLI coding agent workflows | +| **MCP Graph Servers** | MCP Server Daemon | **High** (Dynamic Graph Queries) | **Very High** (On-demand querying) | Tool-call based AST inspection | Cross-tool / Custom agent stacks | +| **Claude Code Indexer** | Subagent Orchestration | **High** (Subagent Summaries) | **High** (Isolated context) | Dynamic subagent exploration | Large monorepos & complex tasks | +| **Cursor / Copilot** | IDE Background Index | **Medium-High** (Vector + Symbol) | **Medium** (Automated windowing) | Continuous workspace sync | IDE-native development | +| **Static Markdown Maps** | Pre-computed LLM Docs | **Medium** (Annotated Folders) | **Medium** (Static overhead) | Session boot context injection | Architecture docs & `.cursorrules` | +| **Raw Shell `tree`** | Directory Paths Only | **Very Low** (Paths only) | **Poor** (High guesswork) | Pure shell dump | Simple scripts / Tiny codebases | + +--- + +## 🔍 Detailed Analysis of Top Mapping Systems + +### 1. Aider's Repo Map +* **Mechanism:** Leverages `tree-sitter` to parse source code into Abstract Syntax Trees (ASTs) and extract classes, function signatures, and variable definitions. Uses a **Personalized PageRank algorithm** over the directed dependency graph to surface only the relevant code symbols within a strict token budget (~1k tokens). +* **Key Advantages:** Unmatched context-to-token ratio; provides precise function signatures without sending full file implementations. +* **Limitations:** Coupled to Aider's CLI runtime environment. + +### 2. MCP Structural Graph Servers (e.g., `repo-graph`, `Codebase-Memory`) +* **Mechanism:** Operates as a background Model Context Protocol (MCP) server. Indexes codebase entities using AST parsers and exposes explicit query endpoints (e.g., `get_caller_hierarchy`, `trace_route`, `get_symbol_definition`). +* **Key Advantages:** Reduces token bloat by 70–90%; standardized protocol compatible with Claude Code, Cursor, Windsurf, or custom agent frameworks. +* **Limitations:** Requires maintaining an active background daemon process during development sessions. + +### 3. Claude Code Native Indexer +* **Mechanism:** Implements a **subagent architecture** to decouple discovery from reasoning. Specialized background subagents explore target directory branches and return concise structural summaries to the main orchestrator agent. +* **Key Advantages:** Scales effortlessly to massive monorepos where single static maps exceed context limits. +* **Limitations:** Introduces additional API tool-call latency during initial exploration phases. + +### 4. Cursor / Copilot Workspace Symbol Indexers +* **Mechanism:** Proprietary background indexing engines combining semantic vector embeddings with AST symbol graphs integrated directly into the editor UI. +* **Key Advantages:** Completely friction-free developer experience with automated real-time index synchronization. +* **Limitations:** Proprietary black-box implementation with limited customization of token allocation and parsing rules. + +### 5. Static Generator CLIs (Annotated `ARCHITECTURE.md` Maps) +* **Mechanism:** Traverses repo structures and utilizes fast LLM passes to generate human- and agent-readable architectural summaries stored in root project files (e.g., `CLAUDE.md`, `ARCHITECTURE.md`). +* **Key Advantages:** Instant session cold-starts with minimal operational overhead. +* **Limitations:** Prone to drift as code changes unless tied to git pre-commit triggers. + +### 6. Standard `tree` Command Dumps +* **Mechanism:** Generates plain directory hierarchy text via native CLI commands (`tree -I "node_modules"`). +* **Key Advantages:** Zero dependencies or setup required. +* **Limitations:** Lacks semantic understanding; forces the agent into high-cost file exploration loops. + +--- + +## 🎯 Implementation Guidance for AI Agents + +1. **For Multi-File Refactoring / High Precision:** Prefer **AST/Graph-based options** (Aider Repo Map or MCP Graph Servers) to ensure type and import safety across module boundaries. +2. **For Monorepo Management:** Utilize **Subagent Context Isolation** (Claude Code pattern) to avoid blowing context windows on single-pass map injection. +3. **For Agent Context Injection:** Maintain a concise `ARCHITECTURE.md` map in the root directory for instant cold-start baseline orientation. \ No newline at end of file