diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 2a6f60b..b2eb737 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -41,10 +41,11 @@ per-run budget of N sources; the sustainable alternative to regenerating the wik upgrade), `curate [--dry-run] [--limit N] [--stale-rules] [--diff PATH] [--retry]` (the SECOND lifecycle: improve EXISTING pages — re-sort/split/re-ground/resolve contradictions/fix locators — against a recomputed findings checklist), `status` (read-only -per-source state table: ingested / failed / skipped-duplicate / ignored / pending; MCP twin +per-source state table: ingested / failed / skipped-duplicate / ignored / oversized / pending; MCP twin `wiki_status`), `doctor` (read-only setup health check — OK/WARN/FAIL lines for workspace / rules / config-parse fallbacks / -agent CLI / the inert-`CITADEL_INGEST_MODEL`-on-copilot/gemini advisory / raw roots / +agent CLI / the inert-`CITADEL_INGEST_MODEL`-on-copilot/gemini advisory / raw roots / wiki placement +(the wiki nested inside a raw root) / child paths (the UNC-vs-drive-letter cwd) / manifest / billing / the HTTP-serve posture / wiki-git state / a best-effort PyPI update check / workspace coherence; needs no workspace, exits 1 only on a FAIL), `serve [--http [--host H] [--port P] [--path /mcp] [--read-only]]` (the MCP server — stdio by default; `--http` serves the SAME surface over MCP's Streamable HTTP transport for a client that is @@ -203,7 +204,14 @@ it is itself a workspace. - **Discovery is incremental and deletion-safe**: one iterative `os.scandir` walk over every `CITADEL_RAW_DIRS` root keeps each file's stat; the **manifest doubles as the scan cache** (an entry's `size`/`mtime_ns`/`ctime_ns`/`hashed_at_ns` are a skip-hint — sha256 - stays the sole arbiter of "changed"; `--full-rescan` distrusts the cache). Deletion candidates + stays the sole arbiter of "changed"; `--full-rescan` distrusts the cache). Two things are pruned + from the walk before any of that: the **wiki dir itself** (`_is_wiki_internal`, lexical — a raw + root ABOVE the wiki, e.g. the whole of `T:\`, would otherwise fold the generated wiki back in as + sources run after run; a root that IS the wiki is refused without arming the sweep, an explicit + path inside it is refused too, and the run-start migration sweep clears any already-self-ingested + key), and files over `CITADEL_MAX_SOURCE_BYTES` (the size complement to the name-matching ignore + globs — skipped from the walk's own stat, so a 10 GB sensor dump is never opened, let alone + hashed; reported, never silent, and off by default). Deletion candidates come from the walked-seen-set diff and each is positively **confirmed with `.exists()`**; any walk error aborts the whole sweep, an unreachable root contributes no candidates, keys under no configured root are logged and never swept, and a workspace-identity mismatch whose keys do not @@ -254,7 +262,8 @@ it is itself a workspace. at run start under the lock, never per source. **`llm.py` is the ONLY place that talks to an LLM**, and it does so by shelling out to a CLI in -agentic mode (`cwd` = workspace root, autonomous file tools). The prompt is **paths-only** — it references +agentic mode (`cwd` = `config.child_cwd()` — the workspace root in its child-friendly spelling — +autonomous file tools). The prompt is **paths-only** — it references the source and rules by path, never embeds file content — which keeps argv tiny (the Windows `WinError 206` fix). One per-kind spec table (`_KIND_SPECS`) maps each `kind` to its task-rule file, whether it reads a source, and its format policy; an unknown kind fails loud. `kind` selects @@ -320,7 +329,8 @@ catalog + one stat-only walk (never re-hashes) rendered as a per-source state ta (model + rules_version, `(stale)` when it predates the current rulebook, `checked YYYY-MM-DD` from the `ingested_at` stamp, the last session's cost when recorded — with a `Recorded LLM cost` corpus total above the table), failed (reason, attempts), -skipped-duplicate, ignored (pattern), pending. +skipped-duplicate, ignored (pattern), oversized (over `CITADEL_MAX_SOURCE_BYTES`, with the size), +pending. **Other modules:** `okf.py` is the OKF format core (parse/dump, type→folder routing, link math, and the non-negotiable `safe_join` path guard — reuse it for any wiki-relative path). `grammar.py` is @@ -412,7 +422,17 @@ browser `Origin`s refused unless `CITADEL_HTTP_ALLOWED_ORIGINS` admits them — 11 readers and the advertised tool list untouched (`server.set_read_only`). No new dependency — starlette/uvicorn already ship with `mcp`. The `viewer/` subpackage builds the self-contained offline HTML viewer (build logic in `__init__.py`; `template.html`/`app.css`/`app.js` are real package-data -assets loaded via `importlib.resources`). `config.py` resolves all paths/settings. `cli.py` mirrors +assets loaded via `importlib.resources`). `config.py` resolves all paths/settings — including the +**native-form registry** (`NATIVE_FORMS`/`native_form`/`child_cwd`): every configured path stays +`resolve()`-d, which is what makes path identity unambiguous, but on Windows that rewrites a mapped +drive (`T:\team-wiki`) into its UNC form, and a UNC `cwd` is refused outright by some agent CLIs +(and read as a different repository by git). So the NON-resolved spelling citadel already holds (the +`.env` value, the launching drive) is remembered alongside the resolved one and handed to CHILD +processes only — the agent CLI's `cwd` + directory grants, git's `-C` — and every path UNDER an +aliased directory inherits it (a default `wiki/` has no `.env` value of its own to record, yet it is +what git gets as `-C`). An alias is recorded only when both spellings of one directory are known AND +resolution turned a non-UNC path into a UNC one, so POSIX and ordinary Windows paths keep exactly +one spelling. `cli.py` mirrors the MCP tools as subcommands (full parity: `define`/`read`/`raw`/`neighbors`/`index`/`sources`/`capture` twin their tools; `view` stays CLI-only and `wiki_lint`/`wiki_status` close the `lint`/`status` gaps from the MCP side). `capture.py` is the conversational-capture bridge behind `wiki_capture`/`citadel capture`: an append-only, @@ -468,7 +488,10 @@ save-the-transcript-as-a-file lane for whole conversations). `rawsource.py` back sources: continue at the segment an interrupted run died on instead of re-paying for the earlier ones; default on), `CITADEL_DEDUP_BY_BASENAME` (skip same-basename document duplicates), `CITADEL_IGNORE_PATTERNS` (OS/junk-file globs skipped at discovery — `Thumbs.db`, - `desktop.ini`, `~$` locks, …; a `+` prefix extends the built-in defaults), `CITADEL_WIKI_LANG` + `desktop.ini`, `~$` locks, …; a `+` prefix extends the built-in defaults), + `CITADEL_MAX_SOURCE_BYTES` (the SIZE complement to those globs: a raw file bigger than this many + bytes is skipped at discovery — never hashed, never tracked, but reported; 0 = no limit, the + default; an explicitly named path always wins), `CITADEL_WIKI_LANG` (target language of all wiki prose, default `en`; verbatim quotes stay original), `CITADEL_PDF_MODE` (`text` | `images` — whether the agent also reads a PDF's figures), `CITADEL_PDF_TEXT` (`auto` | `1` | `0` — the pypdf text-layer pre-pass; auto = on when pypdf diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c277e3..6dc983f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,58 @@ All notable changes to this project are documented here. The format is based on ## [Unreleased] +### Added + +- **`CITADEL_MAX_SOURCE_BYTES` — a discovery size ceiling.** Ignore patterns matched *names*; + nothing matched **size**, so a raw root that also holds machine data (the reported case: 617 + `.tdms` sensor dumps, 10.6 GB in one folder) was stream-hashed in full on every first scan just to + end up recorded as unreadable binary. A file over the ceiling is now skipped from the walk's own + `stat` — never opened, never hashed, never written to the manifest or the failures catalog — and + *reported*: a stderr NOTE, an **Oversized** section on the run report, and an **Oversized** bucket + in `citadel status` (`--json` included). Off by default (`0` = no limit), because silently + dropping a large-but-legitimate source — a 2 GB lecture recording, a scanned archive PDF — would + be worse than a slow scan. Explicitly named paths bypass it, and an already-ingested source that + later crosses the ceiling stays in the wiki; it just stops being re-checked (the deletion sweep's + positive `.exists()` confirmation is what keeps it from reading as vanished). +- **Two `citadel doctor` checks**: **wiki placement** (WARNs when the wiki dir sits inside a walked + raw root) and **child paths** (names the working directory agent sessions will actually run in, + WARNing when only a UNC spelling of the workspace is known). + +### Fixed + +- **Explicitly requested ingest paths now expand `~`.** Every other configured path already did + (`config._resolve_dir_entry`, `workspace.init`, `CITADEL_WORKSPACE`); the `citadel ingest + ` / `wiki_ingest` arguments were the outlier. A POSIX shell expands `~` before citadel + sees it, but Windows `cmd.exe` — and PowerShell, for a native binary's arguments — does not, so + `citadel ingest ~/ws/raw/notes.md` arrived as a literal `~` directory and stat'ed away to + nothing. Expansion uses `os.path.expanduser`, not `Path.expanduser`, which raises on an + unresolvable home (`~nosuchuser`) — discovery must never raise on user input. +- **The wiki can no longer become one of its own raw sources.** With a raw root *above* the wiki + (`CITADEL_RAW_DIRS=T:\` and the wiki at `T:\llmWiki\data-science\wiki` — a normal way to say "scan + this whole drive"), discovery walked the wiki's generated pages back in as sources, run after run, + each pass citing the last; the only defense was a hand-written `CITADEL_IGNORE_PATTERNS` entry. + Discovery now prunes the wiki directory out of every walk (lexically, so it costs nothing per + entry and never blocks on a dead mount), refuses a raw root that *is* the wiki without arming the + deletion sweep for it, refuses an explicitly named path inside the wiki, and announces the + exclusion once per run. The run-start migration sweep additionally clears any page an earlier run + had already self-ingested out of the manifest and the failures catalog, so `wiki/sources/index.md` + stops carrying them. +- **Agent sessions no longer run with a UNC working directory on Windows mapped drives.** + `Path.resolve()` rewrites `T:\team-wiki` into `\\fileserver\share\team-wiki` regardless of which + spelling the `.env` used, and that resolved path was what citadel passed as the agent CLI's `cwd` + — where the copilot backend refuses to work at all ("environment blocks UNC/network paths") and + git treats the spelling as a separate repository (`safe.directory` / `core.filemode`). The + resolved form remains the one identity everywhere (manifest keys, root containment, staging vs. + live); what child processes are handed — the agent CLI's `cwd` and its `--add-dir`/ + `--include-directories` grants, and git's `-C` — is now the **non-resolved** spelling citadel + already held (the `.env` value, or the drive the process was launched from), and every path + UNDER an aliased directory inherits it — which is what reaches the *derived* paths, the common + case: with a workspace on `T:\`, the wiki is just `WORKSPACE_ROOT / "wiki"` with no + `CITADEL_WIKI_DIR` to spell it out, and it is exactly what git gets as `-C`. Nothing is guessed: + an alias is recorded only where both spellings of one directory are known *and* resolution turned + a non-UNC path into a UNC one, so POSIX layouts and ordinary Windows paths are byte-for-byte + unchanged. + ## [0.5.0] - 2026-07-25 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 3c2a867..f4bb7f7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -38,10 +38,11 @@ per-run budget of N sources; the sustainable alternative to regenerating the wik upgrade), `curate [--dry-run] [--limit N] [--stale-rules] [--diff PATH] [--retry]` (the SECOND lifecycle: improve EXISTING pages — re-sort/split/re-ground/resolve contradictions/fix locators — against a recomputed findings checklist), `status` (read-only -per-source state table: ingested / failed / skipped-duplicate / ignored / pending; MCP twin +per-source state table: ingested / failed / skipped-duplicate / ignored / oversized / pending; MCP twin `wiki_status`), `doctor` (read-only setup health check — OK/WARN/FAIL lines for workspace / rules / config-parse fallbacks / -agent CLI / the inert-`CITADEL_INGEST_MODEL`-on-copilot/gemini advisory / raw roots / +agent CLI / the inert-`CITADEL_INGEST_MODEL`-on-copilot/gemini advisory / raw roots / wiki placement +(the wiki nested inside a raw root) / child paths (the UNC-vs-drive-letter cwd) / manifest / billing / the HTTP-serve posture / wiki-git state / a best-effort PyPI update check / workspace coherence; needs no workspace, exits 1 only on a FAIL), `serve [--http [--host H] [--port P] [--path /mcp] [--read-only]]` (the MCP server — stdio by default; `--http` serves the SAME surface over MCP's Streamable HTTP transport for a client that is @@ -200,7 +201,14 @@ it is itself a workspace. - **Discovery is incremental and deletion-safe**: one iterative `os.scandir` walk over every `CITADEL_RAW_DIRS` root keeps each file's stat; the **manifest doubles as the scan cache** (an entry's `size`/`mtime_ns`/`ctime_ns`/`hashed_at_ns` are a skip-hint — sha256 - stays the sole arbiter of "changed"; `--full-rescan` distrusts the cache). Deletion candidates + stays the sole arbiter of "changed"; `--full-rescan` distrusts the cache). Two things are pruned + from the walk before any of that: the **wiki dir itself** (`_is_wiki_internal`, lexical — a raw + root ABOVE the wiki, e.g. the whole of `T:\`, would otherwise fold the generated wiki back in as + sources run after run; a root that IS the wiki is refused without arming the sweep, an explicit + path inside it is refused too, and the run-start migration sweep clears any already-self-ingested + key), and files over `CITADEL_MAX_SOURCE_BYTES` (the size complement to the name-matching ignore + globs — skipped from the walk's own stat, so a 10 GB sensor dump is never opened, let alone + hashed; reported, never silent, and off by default). Deletion candidates come from the walked-seen-set diff and each is positively **confirmed with `.exists()`**; any walk error aborts the whole sweep, an unreachable root contributes no candidates, keys under no configured root are logged and never swept, and a workspace-identity mismatch whose keys do not @@ -251,7 +259,8 @@ it is itself a workspace. at run start under the lock, never per source. **`llm.py` is the ONLY place that talks to an LLM**, and it does so by shelling out to a CLI in -agentic mode (`cwd` = workspace root, autonomous file tools). The prompt is **paths-only** — it references +agentic mode (`cwd` = `config.child_cwd()` — the workspace root in its child-friendly spelling — +autonomous file tools). The prompt is **paths-only** — it references the source and rules by path, never embeds file content — which keeps argv tiny (the Windows `WinError 206` fix). One per-kind spec table (`_KIND_SPECS`) maps each `kind` to its task-rule file, whether it reads a source, and its format policy; an unknown kind fails loud. `kind` selects @@ -317,7 +326,8 @@ catalog + one stat-only walk (never re-hashes) rendered as a per-source state ta (model + rules_version, `(stale)` when it predates the current rulebook, `checked YYYY-MM-DD` from the `ingested_at` stamp, the last session's cost when recorded — with a `Recorded LLM cost` corpus total above the table), failed (reason, attempts), -skipped-duplicate, ignored (pattern), pending. +skipped-duplicate, ignored (pattern), oversized (over `CITADEL_MAX_SOURCE_BYTES`, with the size), +pending. **Other modules:** `okf.py` is the OKF format core (parse/dump, type→folder routing, link math, and the non-negotiable `safe_join` path guard — reuse it for any wiki-relative path). `grammar.py` is @@ -409,7 +419,17 @@ browser `Origin`s refused unless `CITADEL_HTTP_ALLOWED_ORIGINS` admits them — 11 readers and the advertised tool list untouched (`server.set_read_only`). No new dependency — starlette/uvicorn already ship with `mcp`. The `viewer/` subpackage builds the self-contained offline HTML viewer (build logic in `__init__.py`; `template.html`/`app.css`/`app.js` are real package-data -assets loaded via `importlib.resources`). `config.py` resolves all paths/settings. `cli.py` mirrors +assets loaded via `importlib.resources`). `config.py` resolves all paths/settings — including the +**native-form registry** (`NATIVE_FORMS`/`native_form`/`child_cwd`): every configured path stays +`resolve()`-d, which is what makes path identity unambiguous, but on Windows that rewrites a mapped +drive (`T:\team-wiki`) into its UNC form, and a UNC `cwd` is refused outright by some agent CLIs +(and read as a different repository by git). So the NON-resolved spelling citadel already holds (the +`.env` value, the launching drive) is remembered alongside the resolved one and handed to CHILD +processes only — the agent CLI's `cwd` + directory grants, git's `-C` — and every path UNDER an +aliased directory inherits it (a default `wiki/` has no `.env` value of its own to record, yet it is +what git gets as `-C`). An alias is recorded only when both spellings of one directory are known AND +resolution turned a non-UNC path into a UNC one, so POSIX and ordinary Windows paths keep exactly +one spelling. `cli.py` mirrors the MCP tools as subcommands (full parity: `define`/`read`/`raw`/`neighbors`/`index`/`sources`/`capture` twin their tools; `view` stays CLI-only and `wiki_lint`/`wiki_status` close the `lint`/`status` gaps from the MCP side). `capture.py` is the conversational-capture bridge behind `wiki_capture`/`citadel capture`: an append-only, @@ -465,7 +485,10 @@ save-the-transcript-as-a-file lane for whole conversations). `rawsource.py` back sources: continue at the segment an interrupted run died on instead of re-paying for the earlier ones; default on), `CITADEL_DEDUP_BY_BASENAME` (skip same-basename document duplicates), `CITADEL_IGNORE_PATTERNS` (OS/junk-file globs skipped at discovery — `Thumbs.db`, - `desktop.ini`, `~$` locks, …; a `+` prefix extends the built-in defaults), `CITADEL_WIKI_LANG` + `desktop.ini`, `~$` locks, …; a `+` prefix extends the built-in defaults), + `CITADEL_MAX_SOURCE_BYTES` (the SIZE complement to those globs: a raw file bigger than this many + bytes is skipped at discovery — never hashed, never tracked, but reported; 0 = no limit, the + default; an explicitly named path always wins), `CITADEL_WIKI_LANG` (target language of all wiki prose, default `en`; verbatim quotes stay original), `CITADEL_PDF_MODE` (`text` | `images` — whether the agent also reads a PDF's figures), `CITADEL_PDF_TEXT` (`auto` | `1` | `0` — the pypdf text-layer pre-pass; auto = on when pypdf diff --git a/citadel/config.py b/citadel/config.py index 494b97f..c495b6d 100644 --- a/citadel/config.py +++ b/citadel/config.py @@ -56,6 +56,93 @@ def _safe_resolve(path: Path) -> Path: return path if path.is_absolute() else path.absolute() +def _path_id(path: Path | str) -> str: + """The normalized, case-folded identity of a path for pure STRING comparison + (``normcase(normpath(...))``) — no ``resolve()``, so it never touches the filesystem and never + blocks on a dead mount. The one spelling-normalizer behind root de-duplication + (:func:`source_roots`) and the native-form registry below.""" + return os.path.normcase(os.path.normpath(str(path))) + + +# --- Native (non-resolved) path spellings: the UNC-vs-drive-letter seam ------------------------ +# Every configured path is ``resolve()``-d, which is what makes path IDENTITY unambiguous (manifest +# keys, root containment, staging vs. live). On Windows that resolution has one side effect we must +# not hand to a CHILD process: ``Path.resolve()`` rewrites a mapped network drive (``T:\team-wiki``) +# into its UNC form (``\\fileserver\share\team-wiki``) — what ``GetFinalPathNameByHandle`` reports — +# no matter whether the ``.env`` named the drive letter or the share. Some agent CLIs then refuse to +# work at all ("environment blocks UNC/network paths") because their sandbox rejects a UNC working +# directory, and git treats the two spellings as different repositories (``safe.directory`` / +# ``core.filemode`` complaints) — so a workspace that behaves perfectly when the user cds into +# ``T:\...`` breaks the moment citadel spawns the same tool with the resolved cwd. +# +# The fix is deliberately narrow: the resolved form stays the ONE identity everywhere, and the +# non-resolved spelling is remembered ALONGSIDE it purely for what child processes are handed. +# Nothing is guessed — an alias is recorded only where we already hold both spellings of the same +# directory (a ``.env`` value or the process CWD, plus its resolution), and only in the one case +# that matters: a resolved UNC path whose native spelling is not UNC. Everywhere else (all of +# POSIX, every ordinary Windows path) the registry stays EMPTY and :func:`native_form` is the +# identity function, so those invocations are byte-for-byte unchanged. +NATIVE_FORMS: dict[str, str] = {} + + +def _is_unc_path(path: Path | str) -> bool: + """True for a Windows UNC path (``\\\\server\\share\\...``, or its forward-slash spelling). + A pure string test, so it stays meaningful — and unit-testable — on any platform.""" + return str(path).replace("/", "\\").startswith("\\\\") + + +def _prefer_native(resolved: Path | str, native: Path | str) -> bool: + """Whether ``native`` is worth remembering as the child-process spelling of ``resolved``: only + when the two genuinely differ AND resolution turned a non-UNC path into a UNC one (the Windows + mapped-drive rewrite). Every other pair — all of POSIX, a workspace the user named by UNC in + the first place — keeps a single spelling.""" + return _path_id(resolved) != _path_id(native) and _is_unc_path(resolved) and not _is_unc_path(native) + + +def _record_native_form(resolved: Path | str, native: Path | str) -> None: + """Remember ``native`` as the child-process spelling of ``resolved``, but only when + :func:`_prefer_native` says it is worth it — which is what keeps :data:`NATIVE_FORMS` empty + (and :func:`native_form` an identity function) outside the Windows mapped-drive case.""" + if _prefer_native(resolved, native): + NATIVE_FORMS[_path_id(resolved)] = str(native) + + +def native_form(path: Path | str) -> Path: + """The spelling of ``path`` to hand a CHILD process — the agent CLI's ``cwd`` and its directory + grants, git's ``-C`` — namely the recorded non-resolved alias when there is one (a Windows + mapped drive letter instead of ``resolve()``'s UNC rewrite), else ``path`` unchanged. + + An alias is INHERITED down the tree: a directory under one that has an alias is rebased onto + it, so the DERIVED paths get the fix too, not just the ones a ``.env`` spelled out. That is the + common case, not an edge one — with a workspace at ``T:\\team-wiki`` the wiki is + ``WORKSPACE_ROOT / "wiki"`` (no ``CITADEL_WIKI_DIR`` at all), and it is git's ``-C`` argument. + Rebasing is sound because a drive mapping covers the WHOLE subtree: if + ``\\\\srv\\share\\team-wiki`` and ``T:\\team-wiki`` are the same directory, so are their + identically-named children. + + Read at call time (tests monkeypatch :data:`NATIVE_FORMS` like any other config attribute) and + purely lexical: it never touches the filesystem, so a dead mount cannot make it hang. The empty + registry — every POSIX layout, every ordinary Windows one — short-circuits before any of it.""" + if not NATIVE_FORMS: + return Path(path) + p = Path(path) + recorded = NATIVE_FORMS.get(_path_id(p)) + if recorded: + return Path(recorded) + for parent in p.parents: + alias = NATIVE_FORMS.get(_path_id(parent)) + if alias: + return Path(alias) / p.relative_to(parent) + return p + + +def child_cwd() -> str: + """The working directory to spawn a child process in: :data:`WORKSPACE_ROOT` in its + child-friendly spelling (:func:`native_form`). Every ``cwd=`` that citadel passes to a + subprocess goes through this one accessor, so the UNC seam is fixed in exactly one place.""" + return str(native_form(WORKSPACE_ROOT)) + + WORKSPACE_MARKER: str = "citadel.toml" @@ -144,6 +231,32 @@ def _cwd_fallback() -> Path: return Path(".") +def _native_workspace_alias(root: Path) -> Path | None: + """The non-resolved spelling of the workspace ``root`` when THIS process already holds one: + the ``CITADEL_WORKSPACE`` value as typed, or the process CWD / one of its parents (the marker + walk resolves upward, so the root is at or above the CWD). None when no candidate matches — + the normal case everywhere except a Windows mapped drive. + + ``os.path.abspath`` rather than ``resolve()`` is the whole point: it normalizes without asking + the OS for a final path, so a mapped drive letter survives it. The cheap string gate + (:func:`_prefer_native`) runs BEFORE the resolve, so no platform pays for a candidate walk it + can never use.""" + candidates: list[Path] = [] + env = os.environ.get("CITADEL_WORKSPACE", "").strip() + if env: + candidates.append(Path(os.path.abspath(os.path.expanduser(env)))) + try: + cwd = Path(os.path.abspath(os.getcwd())) + candidates.append(cwd) + candidates.extend(cwd.parents) + except OSError: + pass + for candidate in candidates: + if _prefer_native(root, candidate) and _safe_resolve(candidate) == root: + return candidate + return None + + _resolved_root = _resolve_workspace() # Whether discovery actually found a workspace. False means WORKSPACE_ROOT is only the bare CWD # fallback — cli.main fails loud on every workspace-needing subcommand. @@ -151,6 +264,14 @@ def _cwd_fallback() -> Path: WORKSPACE_ROOT: Path = _resolved_root if _resolved_root is not None else _cwd_fallback() del _resolved_root +# Remember how the user (or the CWD) actually SPELLED the workspace root, for the child processes +# citadel spawns there — see NATIVE_FORMS. A no-op unless resolution rewrote a mapped drive letter +# into a UNC path. +_ws_native = _native_workspace_alias(WORKSPACE_ROOT) +if _ws_native is not None: + _record_native_form(WORKSPACE_ROOT, _ws_native) +del _ws_native + # Load the optional workspace .env BEFORE reading the env settings below, so a bare .env # (no exported vars) also works. Only when a workspace actually resolved — the fallback # CWD is NOT a workspace, so a stray .env in some random directory is never slurped. @@ -217,11 +338,20 @@ def _resolve_dir_entry(value: str) -> Path: an ABSOLUTE value AS-IS — including a Windows mapped-drive path (``T:\\team-wiki\\wiki``) or a POSIX mount (``/mnt/share/wiki``) — and resolve a RELATIVE value against the WORKSPACE ROOT (never the process CWD). Always ``_safe_resolve``-d, so ``CITADEL_*_DIR`` and every - ``CITADEL_RAW_DIRS`` entry resolve through the identical path.""" + ``CITADEL_RAW_DIRS`` entry resolve through the identical path. + + The value's own (non-resolved) spelling is remembered for child processes on the way through + (:func:`_record_native_form`), so a ``CITADEL_WIKI_DIR=T:\\other-share\\wiki`` — an ABSOLUTE + override, the one spelling nothing else could derive — is still handed to the agent CLI / git as + a drive-letter path even though its identity here is the resolved UNC form. A default or + relative dir needs no record: it lives under the workspace root, so :func:`native_form` inherits + the root's own alias.""" path = Path(value).expanduser() if not path.is_absolute(): path = WORKSPACE_ROOT / path - return _safe_resolve(path) + resolved = _safe_resolve(path) + _record_native_form(resolved, Path(os.path.abspath(path))) + return resolved def _dir_setting(env_key: str, default: Path) -> Path: @@ -302,7 +432,7 @@ def source_roots() -> list[Path]: roots: list[Path] = [] seen: set[str] = set() for root in (*RAW_DIRS, RAW_DIR): - ident = os.path.normcase(os.path.normpath(str(root))) + ident = _path_id(root) if ident not in seen: seen.add(ident) roots.append(Path(root)) @@ -830,6 +960,37 @@ def _pdf_text_mode() -> str: # to disable) for a very large one. MAX_SOURCE_CHARS: int = _int_env("CITADEL_MAX_SOURCE_CHARS", 300000) + +def _max_source_bytes() -> int: + """Resolve ``CITADEL_MAX_SOURCE_BYTES`` to a byte ceiling >= 0. A negative value is a + misconfiguration, not "unlimited" spelled oddly: it falls back to 0 (no limit — the default) + and records a :data:`CONFIG_WARNINGS` entry, so ``citadel doctor`` names it.""" + value = _int_env("CITADEL_MAX_SOURCE_BYTES", 0) + if value < 0: + CONFIG_WARNINGS.append(f"CITADEL_MAX_SOURCE_BYTES={value} is not a byte count (>= 0) - using 0 (no limit)") + return 0 + return value + + +# Discovery SIZE ceiling, in bytes. A raw file bigger than this is skipped at discovery — before it +# is hashed, before it is sniffed — and reported as oversized; 0 (the default) means no limit, so +# the behavior is exactly what it has always been until this is deliberately set. +# +# The complement to CITADEL_IGNORE_PATTERNS: those match names, this matches SIZE, and size is what +# distinguishes a knowledge corpus from the machine-data that tends to sit beside it (a folder of +# 617 raw sensor dumps, 10 GB of `.tdms`, is useless to a wiki but expensive to scan — every +# untracked candidate is stream-hashed in full before anything can classify it as unreadable +# binary). Set it just above your largest real document to keep such a folder out of the scan. +# +# Deliberately OFF by default: silently dropping a large-but-legitimate source (a 2 GB lecture +# recording under CITADEL_AUDIO_SUPPORT, a scanned archive PDF) would be a data-loss-shaped +# surprise, and citadel's contract is that everything under a raw root is either ingested or +# reported. Skips ARE reported (the run report's "Oversized" section, `citadel status`), so the +# ceiling never hides work silently. Explicitly-named paths (`citadel ingest big.tdms`) bypass it — +# explicit always wins, exactly as it does for ignore patterns and hidden files. An already-ingested +# source that later crosses the ceiling stays in the wiki: it simply stops being re-checked. +MAX_SOURCE_BYTES: int = _max_source_bytes() + # Resume checkpoints for CHUNKED sources (citadel/resume.py). Promotion stays all-or-nothing — the # live wiki only ever holds fully-imported sources — but when ON (default) each completed segment # records the delta it produced in a dotdir sibling of the wiki (.citadel_resume/), so a run that diff --git a/citadel/doctor.py b/citadel/doctor.py index 5249447..779fe7b 100644 --- a/citadel/doctor.py +++ b/citadel/doctor.py @@ -19,6 +19,11 @@ - **raw roots** — is every raw root ingest actually walks (``CITADEL_RAW_DIRS``) reachable (a dir on disk)? Also WARNs when the primary ``raw/`` was configured OUT of the walk list while holding files — those would silently never be ingested. +- **wiki placement** — does the wiki dir sit INSIDE a walked raw root (a whole mounted drive as one + root)? Discovery excludes the wiki either way, so this is a clarity WARN, not data loss. +- **child paths** — the UNC advisory: ``resolve()`` rewrites a Windows mapped drive into its UNC + form, which some agent CLIs refuse as a working directory; names the drive-letter spelling + citadel hands child processes, or WARNs when only a UNC one exists. - **manifest** — does ``wiki/.citadel_ingested.json`` parse, with its format version, source count, and a workspace stamp matching the current root? - **failures** — a summary of the sources the failures catalog says could not be ingested. @@ -229,6 +234,54 @@ def check_raw_roots() -> Check: return Check(OK, "raw roots", f"{len(roots)} walked raw root(s) reachable") +def check_wiki_placement() -> Check: + """WARN when the wiki directory lies under a walked raw root — the self-ingest layout (a whole + mounted drive configured as one raw root, with the wiki somewhere inside it). Discovery now + excludes the wiki automatically (``ingest._is_wiki_internal``), so this is not a data-loss FAIL; + it is worth saying out loud because the nesting still costs clarity: every wiki file sits inside + a source tree, so a stray citation INTO the wiki looks like legal provenance, and any + third-party tool pointed at the raw root sees generated pages as content. OK otherwise.""" + from . import grammar + + wiki = Path(config.WIKI_DIR) + covering = [Path(r) for r in config.RAW_DIRS if grammar.is_within(wiki, r)] + if not covering: + return Check(OK, "wiki placement", "the wiki dir is outside every walked raw root") + return Check( + WARN, + "wiki placement", + f"the wiki dir ({wiki}) lies under walked raw root(s) {', '.join(str(r) for r in covering)} - " + "discovery excludes it automatically (generated pages are never sources), but prefer a wiki " + "outside the raw tree: narrow CITADEL_RAW_DIRS, or move the wiki with CITADEL_WIKI_DIR", + ) + + +def check_child_paths() -> Check: + """The UNC advisory. ``Path.resolve()`` rewrites a Windows mapped network drive (``T:\\wiki``) + into its UNC form (``\\\\server\\share\\wiki``), and that resolved path used to be what citadel + handed to the agent CLI as its working directory — where some backends refuse to run at all + ("environment blocks UNC/network paths") and git treats the spelling as a different repository. + + OK (with the drive-letter cwd named) when citadel recorded a non-UNC alias for the workspace, + OK when no UNC path is involved at all, and WARN when the workspace really is UNC-only — then + the agent genuinely runs on a UNC cwd, and the fix is to map the share to a drive letter and + point ``CITADEL_WORKSPACE`` (or run citadel from) there.""" + root = Path(config.WORKSPACE_ROOT) + native = config.native_form(root) + if config._path_id(native) != config._path_id(root): + return Check(OK, "child paths", f"agent sessions run in {native} (workspace resolves to {root})") + if config._is_unc_path(root): + return Check( + WARN, + "child paths", + f"the workspace resolves to a UNC path ({root}) and no drive-letter spelling of it is " + "known here - some agent CLIs refuse a UNC working directory and git treats it as a " + "separate repository; map the share to a drive letter and set CITADEL_WORKSPACE to it " + "(or run citadel from that drive) if sessions fail with path errors", + ) + return Check(OK, "child paths", "no UNC/network path rewriting in effect") + + def check_manifest() -> Check: """OK when there is no manifest yet (nothing ingested) or it parses with a matching workspace stamp; WARN when it is unparseable JSON (treated as empty) or its stamp names another workspace @@ -618,6 +671,8 @@ def run() -> DoctorReport: check_agent_cli(), check_ingest_model(), check_raw_roots(), + check_wiki_placement(), + check_child_paths(), check_manifest(), check_failures(), check_billing_shadow(), diff --git a/citadel/ingest.py b/citadel/ingest.py index 5e3892f..dcdffd0 100644 --- a/citadel/ingest.py +++ b/citadel/ingest.py @@ -46,6 +46,7 @@ config, extract, failures, + grammar, llm, manifest, okf, @@ -106,6 +107,10 @@ class IngestReport: # (forced_key, kept_key) for same-basename pairs a FORCED run ingested ALONGSIDE the kept # sibling (a forced run bypasses the dedup drop — nothing was skipped, both formats are in the wiki). duplicates_forced: list[tuple[str, str]] = field(default_factory=list) + # (rel_key, size_bytes) for sources skipped at discovery because they exceed + # CITADEL_MAX_SOURCE_BYTES — never hashed, never ingested, and (like an ignore-pattern match) + # never recorded in the manifest or the failures catalog. Reported so a size skip is visible. + oversized: list[tuple[str, int]] = field(default_factory=list) # rel-keys of tracked sources that VANISHED from disk (a full run only): their provenance is # reconciled out of the wiki by a cleanup agent session, then the manifest key is dropped. sources_deleted: list[str] = field(default_factory=list) @@ -179,6 +184,9 @@ def render(self) -> str: ) else: lines.append(f" - {p}") + if self.oversized: + lines.append(f"Oversized (over CITADEL_MAX_SOURCE_BYTES = {_human_bytes(config.MAX_SOURCE_BYTES)}):") + lines.extend(f" - {key} ({_human_bytes(size)})" for key, size in self.oversized) if self.duplicates: lines.append("Skipped as duplicate (same basename as another format that was ingested):") lines.extend(f" - {dropped} (kept {kept})" for dropped, kept in self.duplicates) @@ -226,6 +234,75 @@ def _is_ignored_name(name: str) -> bool: return any(fnmatch.fnmatchcase(lowered, pattern.lower()) for pattern in config.IGNORE_PATTERNS) +def _is_wiki_internal(path: Path) -> bool: + """True when ``path`` is at or under the LIVE wiki directory — the generated, LLM-owned layer, + which is never a raw source. + + This is the self-ingest guard. A layout whose raw root sits ABOVE the wiki — a whole mounted + drive walked as one root (``CITADEL_RAW_DIRS=T:\\`` with the wiki at ``T:\\llmWiki\\ds\\wiki``) + — would otherwise discover the wiki's own pages as sources and fold the wiki into itself, run + after run, each pass citing the last. Nothing else prevented it: the wiki dir is not hidden and + matches no ignore pattern, so the only workaround was a hand-written + ``CITADEL_IGNORE_PATTERNS`` entry. + + Containment is ``grammar.is_within`` — purely lexical, no ``resolve()`` and no ``abspath()`` — + so the check costs nothing per directory entry and can never block (or syscall) on a dead + mount. ``path`` must therefore already be ABSOLUTE, which every walk-built path and every + ``config.source_path_for_key`` result is; the one caller that can be handed a relative path (an + explicitly requested one, typed by the user) normalizes it itself. + + Deliberately the process-wide ``config.WIKI_DIR`` rather than ``config.wiki_dir()``: what must + never be scanned is the LIVE wiki, whichever per-source staging copy the current context is + redirected to (staging copies are hidden dotdir siblings, which discovery already skips).""" + return grammar.is_within(path, config.WIKI_DIR) + + +def _explicit_path(raw: str | os.PathLike) -> Path: + """One EXPLICITLY requested path (``citadel ingest ``, ``wiki_ingest``), with ``~`` + expanded — the same courtesy every other configured path already gets + (``config._resolve_dir_entry``, ``workspace.init``, ``CITADEL_WORKSPACE``); the ingest + arguments were the outlier. + + It matters most where this PR's other fixes do: a POSIX shell expands ``~`` before citadel ever + sees it, but Windows ``cmd.exe`` (and PowerShell, for a native binary's arguments) does not, so + ``citadel ingest ~/ws/raw/notes.md`` arrived as a literal ``~`` directory that stat'ed away to + nothing. It also closes the same gap in the wiki guard: an unexpanded path could not be + recognized as wiki-internal (it merely failed to resolve, so nothing was ingested — but the + guard must hold by construction, not by a downstream accident). + + ``os.path.expanduser`` rather than ``Path.expanduser``: the latter RAISES on an unresolvable + home (``~nosuchuser``), and discovery must never raise on user input — the stdlib function + returns such a path unchanged instead.""" + return Path(os.path.expanduser(raw)) + + +def _is_untrackable_key(key: str) -> bool: + """True for a tracked key that must not be tracked AT ALL any more — the run-start migration + sweep's predicate: an OS/junk basename (an ignore pattern added after it was recorded) or a + path inside the wiki itself (a page self-ingested before :func:`_is_wiki_internal` guarded + discovery). Both keep existing on disk, so deletion detection would never clean them up.""" + return _is_ignored_name(PurePosixPath(key).name) or _is_wiki_internal(config.source_path_for_key(key)) + + +def _exceeds_size_cap(size: int) -> bool: + """True when ``config.MAX_SOURCE_BYTES`` is set and a file of ``size`` bytes is over it — the + discovery SIZE ceiling that complements the name-matching ignore patterns. Read at call time; + 0 (the default) means no limit.""" + cap = config.MAX_SOURCE_BYTES + return cap > 0 and size > cap + + +def _human_bytes(size: int) -> str: + """A short ASCII rendering of a byte count for the report ("10.6 GB") — console output stays + ASCII-only, so no multiplication sign or non-breaking space sneaks in.""" + value = float(size) + for unit in ("B", "KB", "MB", "GB", "TB"): + if value < 1024 or unit == "TB": + return f"{value:.0f} {unit}" if unit == "B" else f"{value:.1f} {unit}" + value /= 1024 + return f"{size} B" # unreachable; keeps the return type total + + def _is_repo_source(path: Path) -> bool: """True if ``path`` should be ingested as ONE repo source: repo support is on, it is a repo dir (``.git``/``.citadelsource``), and it is NOT a configured corpus root (``RAW_DIR`` or any @@ -253,6 +330,12 @@ class _Walk: errors: list[str] = field(default_factory=list) # OSErrors below an entered root unreachable: list[Path] = field(default_factory=list) # roots that could not be entered at all entered_roots: list[Path] = field(default_factory=list) # roots whose top-level scandir succeeded + # (path, size) for files skipped by the CITADEL_MAX_SOURCE_BYTES ceiling — never hashed, never + # ingested, but reported, so a size skip is visible instead of silent. + oversized: list[tuple[Path, int]] = field(default_factory=list) + # Directories skipped because they ARE (or are inside) the wiki — a raw root that sits above + # the wiki dir. Reported once per run so the exclusion is visible, never inferred as absence. + excluded_wiki: list[Path] = field(default_factory=list) def _scan_tree(root: Path, walk: _Walk) -> None: @@ -260,13 +343,21 @@ def _scan_tree(root: Path, walk: _Walk) -> None: the two ``os.walk`` passes (files + repos) with a single traversal whose ``DirEntry.stat`` results are kept for the scan-cache quick check. - Same skip rules as before: hidden names (leading ``.``), OS/junk ignore globs - (:func:`_is_ignored_name`), and — with repo support on — no descending into a git repository - (collected as one repo source instead). Any file type in any sub-folder is picked up; - ``follow_symlinks=False`` throughout, so a symlinked directory is never recursed into (a - cycle on a share must not hang discovery). Deterministic order (names sorted per directory, - depth-first). NEVER raises: a top-level failure marks the root unreachable; a failure deeper - in records a walk error (either one disarms the deletion sweep — see :func:`ingest`).""" + Skip rules: hidden names (leading ``.``), OS/junk ignore globs (:func:`_is_ignored_name`), the + wiki directory itself (:func:`_is_wiki_internal` — the generated layer is never a source), files + over the size ceiling (:func:`_exceeds_size_cap`), and — with repo support on — no descending + into a git repository (collected as one repo source instead). Any other file type in any + sub-folder is picked up; ``follow_symlinks=False`` throughout, so a symlinked directory is never + recursed into (a cycle on a share must not hang discovery). Deterministic order (names sorted + per directory, depth-first). NEVER raises: a top-level failure marks the root unreachable; a + failure deeper in records a walk error (either one disarms the deletion sweep — see + :func:`ingest`).""" + if _is_wiki_internal(Path(root)): + # The configured raw root IS the wiki (or lives inside it). Refuse the whole walk rather + # than scan generated pages back in as sources — and, by never entering, leave this root + # out of ``entered_roots``, so the deletion sweep is not armed for a root nothing scanned. + walk.excluded_wiki.append(Path(root)) + return at_root = True stack: list[Path] = [Path(root)] while stack: @@ -291,14 +382,26 @@ def _scan_tree(root: Path, walk: _Walk) -> None: path = Path(d) / name try: if entry.is_dir(follow_symlinks=False): + if _is_wiki_internal(path): + # The wiki dir under a raw root: prune it whole. Its pages are generated, + # never sources — ingesting them would fold the wiki into itself. + walk.excluded_wiki.append(path) # Deliberately NOT _is_repo_source: its corpus-root guard resolve()s every root per call # — too costly per-directory on a network share (a subdir is never a configured root here). - if config.REPO_SUPPORT and repo.is_repo_dir(path): + elif config.REPO_SUPPORT and repo.is_repo_dir(path): walk.repos.append(path) # one repo source; the file walk stops here else: subdirs.append(path) elif entry.is_file(follow_symlinks=False): - walk.files.append((path, entry.stat(follow_symlinks=False))) + st = entry.stat(follow_symlinks=False) + # The size ceiling is applied HERE, on the stat the walk already took: an + # oversized file is never opened, never hashed, and never classified — which + # is the whole point (a folder of multi-GB machine-data dumps otherwise costs + # a full sha256 stream per file before anything can call it unreadable). + if _exceeds_size_cap(st.st_size): + walk.oversized.append((path, st.st_size)) + else: + walk.files.append((path, st)) except OSError as exc: walk.errors.append(f"{path}: {exc}") stack.extend(reversed(subdirs)) # LIFO -> depth-first in sorted order @@ -306,15 +409,23 @@ def _scan_tree(root: Path, walk: _Walk) -> None: def _discover_walk(paths: list[str] | None) -> _Walk: """Resolve requested paths (or default to every configured raw root, ``config.RAW_DIRS``) - into one :class:`_Walk`. A requested file path is stat'ed and taken as-is (even a hidden or - ignore-matched name — explicit wins, as before; one that is missing or not a regular file is - silently dropped, replacing the old per-candidate ``is_file()``); a requested directory - contributes its whole subtree — unless it is itself a repo source, which - :func:`_discover_repos` handles. Roots are de-duplicated by resolved path.""" + into one :class:`_Walk`. A requested file path is stat'ed and taken as-is (even a hidden name, + an ignore-matched one, or one over the size ceiling — explicit wins, as before; one that is + missing or not a regular file is silently dropped, replacing the old per-candidate + ``is_file()``); a requested directory contributes its whole subtree — unless it is itself a repo + source, which :func:`_discover_repos` handles. Roots are de-duplicated by resolved path. + + The ONE thing explicit does NOT win over is :func:`_is_wiki_internal`: a path inside the wiki is + generated output, not a source, so naming it directly cannot make it one either.""" walk = _Walk() if paths: for raw in paths: - p = Path(raw) + p = _explicit_path(raw) + # abspath (not resolve) so a RELATIVE argument — `citadel ingest wiki/x.md` from the + # workspace root — is still recognized as wiki-internal, without a filesystem round trip. + if _is_wiki_internal(Path(os.path.abspath(p))): + walk.excluded_wiki.append(p) + continue if p.is_dir(): if not _is_repo_source(p): _scan_tree(p, walk) @@ -397,13 +508,20 @@ def _discover_repos(paths: list[str] | None, walk: _Walk) -> list[Path]: """The repo sources to ingest: the repo dirs the walk found under the raw roots (or under an explicitly requested directory), plus an explicitly requested path that is itself a repo. De-duplicated by resolved path, sorted. Empty when repo support is off (the walk then - descended into repos file-by-file — the legacy behavior).""" + descended into repos file-by-file — the legacy behavior). + + The walk's own repo list is already wiki-free (:func:`_scan_tree` prunes the wiki before the + repo test), but an EXPLICIT path needs the same guard here: a wiki dir under + ``CITADEL_WIKI_GIT`` holds a ``.git``, so naming it would otherwise digest the whole wiki as a + repo source.""" if not config.REPO_SUPPORT: return [] found: list[Path] = list(walk.repos) if paths: for raw in paths: - p = Path(raw) + p = _explicit_path(raw) + if _is_wiki_internal(Path(os.path.abspath(p))): + continue if p.is_dir() and _is_repo_source(p): found.append(p) seen: set[Path] = set() @@ -2498,16 +2616,17 @@ def emit(event: str, **data) -> None: # Updated through the run and rewritten at the end, so it always reflects the CURRENT stuck set. failures_dict = failures.load() failures_before = {k: dict(v) if isinstance(v, dict) else v for k, v in failures_dict.items()} - # Migration sweep: drop any entry a PREVIOUS run recorded for a file that is NOW ignored - # (Thumbs.db & friends, before this feature existed). It still exists on disk, so a full run - # would never re-detect it as deleted — clean it out of the manifest AND the failures catalog - # directly so wiki/sources/index.md stops carrying the noise. Repo entries never match (their - # key basename is a folder name), so this only touches junk-file keys. + # Migration sweep: drop any entry a PREVIOUS run recorded for a source that must not be tracked + # at all — a now-ignored junk file (Thumbs.db & friends, recorded before that feature existed) + # or a path inside the WIKI (a page self-ingested by a layout whose raw root sits above the wiki, + # before the discovery guard existed). Both still exist on disk, so a full run would never + # re-detect them as deleted — clean them out of the manifest AND the failures catalog directly + # so wiki/sources/index.md stops carrying the noise. pruned_ignored = False - for key in [k for k in manifest_dict if _is_ignored_name(PurePosixPath(k).name)]: + for key in [k for k in manifest_dict if _is_untrackable_key(k)]: del manifest_dict[key] pruned_ignored = True - for key in [k for k in failures_dict if _is_ignored_name(PurePosixPath(k).name)]: + for key in [k for k in failures_dict if _is_untrackable_key(k)]: failures.clear(failures_dict, key) pruned_ignored = True if pruned_ignored: @@ -2604,6 +2723,27 @@ def emit(event: str, **data) -> None: "kept as ingested and re-checked next run:\n " + "\n ".join(sorted(scan.unreadable_tracked)), file=sys.stderr, ) + # --- Discovery exclusions: both are deliberate skips, so say so rather than let the sources + # simply not appear. The wiki note fires once per run (a raw root above the wiki prunes the + # same tree at every level); the size note lists what the ceiling kept out. --- + if walk.excluded_wiki: + print( + f"NOTE: the wiki directory ({config.WIKI_DIR}) is excluded from discovery - generated " + "pages are never raw sources. If a raw root sits above the wiki, narrow " + "CITADEL_RAW_DIRS (or move the wiki with CITADEL_WIKI_DIR) to silence this.", + file=sys.stderr, + ) + report.oversized = sorted((manifest.rel_key(p), size) for p, size in walk.oversized) + if report.oversized: + listed = [f"{key} ({_human_bytes(size)})" for key, size in report.oversized[:10]] + if len(report.oversized) > len(listed): + listed.append(f"... +{len(report.oversized) - len(listed)} more (all listed on the run report)") + print( + f"NOTE: {len(report.oversized)} file(s) skipped by CITADEL_MAX_SOURCE_BYTES " + f"({_human_bytes(config.MAX_SOURCE_BYTES)}); raise it (or name a path explicitly) to " + "ingest them:\n " + "\n ".join(listed), + file=sys.stderr, + ) # A pending source whose key is ALREADY tracked is a re-ingest of changed bytes (reconcile); # one not yet tracked is brand new. Captured before the manifest is mutated below. pending_keys = {manifest.rel_key(p) for p in scan.pending} diff --git a/citadel/llm.py b/citadel/llm.py index 09fb408..23134d1 100644 --- a/citadel/llm.py +++ b/citadel/llm.py @@ -181,7 +181,12 @@ def _external_dirs(rel_key: str, read_path: str | None = None) -> list[str]: workspace ``rules/`` overlay, which lives under cwd and therefore filters out), the source file's own parent, and — for an Office source — the temp dir holding its extracted text}. Empty for the all-under-workspace dev-checkout layout (cwd already covers the rules), so that - invocation is byte-for-byte unchanged.""" + invocation is byte-for-byte unchanged. + + Each granted directory is named in its CHILD-friendly spelling (``config.native_form``): the + grant has to match the paths the agent actually walks from its ``cwd``, and on a Windows mapped + drive that is the drive letter, not the UNC form ``resolve()`` produces. Outside that case the + two spellings are identical.""" candidates = [ config.wiki_dir(), *config.source_roots(), # every raw source root (multi-root: an out-of-workspace root needs a grant) @@ -199,7 +204,7 @@ def _external_dirs(rel_key: str, read_path: str | None = None) -> list[str]: out: dict[str, None] = {} for d in candidates: if config.is_outside_workspace(d): - out[str(Path(d).resolve())] = None + out[str(config.native_form(Path(d).resolve()))] = None return sorted(out) @@ -773,7 +778,7 @@ def _stream_subprocess(cli: str, argv: list[str], stdin_text: str | None) -> tup encoding="utf-8", errors="replace", bufsize=1, - cwd=str(config.WORKSPACE_ROOT), + cwd=config.child_cwd(), env=config.child_env(), ) if stdin_text is not None and proc.stdin is not None: @@ -867,7 +872,8 @@ def _write_transcript( def _run_session( cli: str, argv: list[str], stdin_text: str | None, *, log_label: str | None = None ) -> SessionUsage | None: - """Run the agentic CLI once in ``config.WORKSPACE_ROOT``. Success = the session completed + """Run the agentic CLI once in the workspace root (``config.child_cwd()`` — the root in the + spelling a child process can use; see ``config.NATIVE_FORMS``). Success = the session completed without error; the agent's edits are on disk. Returns the session's :class:`SessionUsage` when the backend reported one (claude's result envelope; None for copilot/gemini, whose stdout carries no cost data — gemini's stats file is read by ``run_ingest_session``). @@ -899,7 +905,7 @@ def _run_session( encoding="utf-8", errors="replace", timeout=config.LLM_TIMEOUT, - cwd=str(config.WORKSPACE_ROOT), + cwd=config.child_cwd(), # The wiki this session must edit is passed EXPLICITLY (ingest's per-source staging # copy, via config.wiki_redirect) rather than through a process-global # os.environ assignment — two concurrent sessions each get their own. diff --git a/citadel/status.py b/citadel/status.py index ed015b3..dca7c70 100644 --- a/citadel/status.py +++ b/citadel/status.py @@ -11,6 +11,8 @@ style record, its attempt count; - **skipped-duplicate** — a same-basename twin skipped in favor of another format; - **ignored** — an OS/junk file matched by ``CITADEL_IGNORE_PATTERNS``; +- **oversized** — on disk under a raw root but past the ``CITADEL_MAX_SOURCE_BYTES`` ceiling, so + discovery skips it (never hashed, never tracked); - **pending** — on disk under a raw root, not yet in the manifest or the failures catalog. Built from the manifest + the failures catalog + ONE stat-only discovery walk (reusing ingest's @@ -24,6 +26,7 @@ import os from dataclasses import asdict, dataclass, field +from pathlib import Path from . import config, failures, ingest, llm, manifest @@ -64,6 +67,8 @@ class StatusReport: failed: list[SourceState] = field(default_factory=list) skipped_duplicate: list[SourceState] = field(default_factory=list) ignored: list[str] = field(default_factory=list) + # (key, size_bytes) for files the CITADEL_MAX_SOURCE_BYTES ceiling keeps out of discovery. + oversized: list[tuple[str, int]] = field(default_factory=list) pending: list[str] = field(default_factory=list) rules_version: str = "" @@ -122,6 +127,10 @@ def render(self) -> str: for name in self.ignored: lines.append(f" {name}") + lines.append(f"Oversized ({len(self.oversized)})") + for key, size in self.oversized: + lines.append(f" {key} {ingest._human_bytes(size)}") + lines.append(f"Pending ({len(self.pending)})") for key in self.pending: lines.append(f" {key}") @@ -129,10 +138,12 @@ def render(self) -> str: return "\n".join(lines).rstrip() + "\n" def as_dict(self) -> dict: - """The report as one JSON-ready dict (``citadel status --json``): the five buckets plus + """The report as one JSON-ready dict (``citadel status --json``): the six buckets plus ``rules_version`` and ``cost_usd_total``, each source row a plain dict with only its None fields dropped — ``attempts: 0`` / ``stale_rules: false`` stay explicit, so scripts get a predictable - shape for 'which sources failed and why' without scraping :meth:`render`'s table.""" + shape for 'which sources failed and why' without scraping :meth:`render`'s table. + ``oversized`` carries ``{"key", "size_bytes"}`` objects rather than bare strings, since the + size is the reason the row exists.""" def row(s: SourceState) -> dict: return {k: v for k, v in asdict(s).items() if v is not None} @@ -147,6 +158,7 @@ def row(s: SourceState) -> dict: "failed": [row(s) for s in self.failed], "skipped_duplicate": [row(s) for s in self.skipped_duplicate], "ignored": list(self.ignored), + "oversized": [{"key": key, "size_bytes": size} for key, size in self.oversized], "pending": list(self.pending), } @@ -158,32 +170,38 @@ def _is_stale_rules(entry, current_rules_version: str) -> bool: return recorded is not None and recorded != current_rules_version -def _present_source_keys() -> set[str]: - """Every source key visible on disk under the raw roots RIGHT NOW — files plus repo dirs — - via ingest's own stat-only walk (no hashing, repo-aware, dead-mount-safe). Defensive: any walk - failure degrades to the empty set (pending simply shows nothing) rather than raising.""" +def _walk_state() -> tuple[set[str], list[tuple[str, int]]]: + """ONE stat-only discovery walk (ingest's own — no hashing, repo-aware, dead-mount-safe), read + for the two things status needs from disk: every source key visible under the raw roots RIGHT + NOW (files plus repo dirs), and the ``(key, size)`` pairs the ``CITADEL_MAX_SOURCE_BYTES`` + ceiling kept out of it. Defensive: any walk failure degrades to empty (pending/oversized simply + show nothing) rather than raising.""" try: walk = ingest._discover_walk(None) except OSError: - return set() + return set(), [] keys = {manifest.rel_key(path) for path, _st in walk.files} keys |= {manifest.rel_key(path) for path in walk.repos} - return keys + oversized = sorted((manifest.rel_key(path), size) for path, size in walk.oversized) + return keys, oversized def _ignored_names() -> list[str]: """The OS/junk basenames under the raw roots that discovery skips (``CITADEL_IGNORE_PATTERNS``) - — a light, stat-free ``os.walk`` that prunes ignored/hidden directories exactly as discovery - does (the ONE ignore predicate, :func:`ingest._is_ignored_name`). Deduped + sorted; degrades to - an empty list on any walk error.""" + — a light, stat-free ``os.walk`` that prunes ignored/hidden directories (and the wiki dir) + exactly as discovery does, through discovery's OWN predicates + (:func:`ingest._is_ignored_name` / :func:`ingest._is_wiki_internal`). Deduped + sorted; degrades + to an empty list on any walk error.""" found: set[str] = set() for root in config.source_roots(): try: - for _dirpath, dirnames, filenames in os.walk(root): + for dirpath, dirnames, filenames in os.walk(root): kept = [] for d in dirnames: if d.startswith("."): continue + if ingest._is_wiki_internal(Path(dirpath) / d): + continue # generated output, not a source tree — never walked if ingest._is_ignored_name(d): found.add(d) else: @@ -247,6 +265,8 @@ def build_status() -> StatusReport: report.failed.append(row) tracked = set(manifest_dict) | set(failures_dict) - report.pending = sorted(key for key in _present_source_keys() if key not in tracked) + present, oversized = _walk_state() + report.pending = sorted(key for key in present if key not in tracked) + report.oversized = oversized report.ignored = _ignored_names() return report diff --git a/citadel/templates/env.example b/citadel/templates/env.example index e62e4ef..9449ad8 100644 --- a/citadel/templates/env.example +++ b/citadel/templates/env.example @@ -139,10 +139,16 @@ CITADEL_INGEST_MODEL=sonnet # use CITADEL_LLM_LOG_DIR above for a full claude record). CLI flag: -v. # CITADEL_LLM_VERBOSE=0 -# --- Ignore patterns --- +# --- What discovery skips --- # OS/junk globs skipped at discovery (Thumbs.db, .DS_Store, ~$ locks, swap/backup files, ...). # A comma/newline list REPLACES the built-in defaults; a `+` prefix ADDS to them. # CITADEL_IGNORE_PATTERNS=+*.bak,~backup* +# Size ceiling in BYTES: a raw file bigger than this is skipped at discovery — never hashed, never +# ingested — and listed on the run report and in `citadel status`. The size-based complement to the +# name-based patterns above, for a raw root that also holds machine data (a folder of multi-GB +# sensor dumps costs a full sha256 stream per file before anything can call it unreadable binary). +# 0 (the default) = no limit; an explicitly named path (`citadel ingest big.tdms`) always wins. +# CITADEL_MAX_SOURCE_BYTES=52428800 # --- Style profiles --- # When 1, first-person sources also yield attributed, dated, cited opinions plus a per-person diff --git a/citadel/wikigit.py b/citadel/wikigit.py index 2de9aca..a256432 100644 --- a/citadel/wikigit.py +++ b/citadel/wikigit.py @@ -55,13 +55,18 @@ def _git(cwd: Path, *args: str, timeout: int = _GIT_TIMEOUT_S) -> tuple[int, str] | None: """Run ``git -C ``; return ``(returncode, combined output)`` or None when git is - missing or the call itself failed to run (OSError/timeout). Never raises.""" + missing or the call itself failed to run (OSError/timeout). Never raises. + + ``cwd`` is passed in its child-friendly spelling (``config.native_form``): git for Windows + treats a mapped drive letter and its UNC form as different repositories, so the drive-letter + path the user configured — and already ran ``git init`` / ``safe.directory`` against — is the + one to hand it. Identical to ``str(cwd)`` everywhere else.""" git = shutil.which("git") if not git: return None try: proc = subprocess.run( - [git, "-C", str(cwd), *args], + [git, "-C", str(config.native_form(cwd)), *args], capture_output=True, text=True, encoding="utf-8", diff --git a/docs/configuration.md b/docs/configuration.md index ac8b63d..84caa9a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -123,6 +123,7 @@ $env:CITADEL_LLM_CLI = "copilot" | `CITADEL_RESUME` | `1` | Resume checkpoints for those chunked sources: each completed segment banks the delta it produced (`.citadel_resume/` next to the wiki), so a run that dies at segment N continues there next time instead of re-buying segments 1…N-1. Promotion stays all-or-nothing — nothing partial ever reaches the wiki — and every guard (changed source/model/rules/knobs, a page changed underneath, a replay that no longer validates) falls back to a full restart. `0` turns it off; only chunked sources ever write one. | | `CITADEL_DEDUP_BY_BASENAME` | `1` | When several same-folder files share a basename and are all export formats (e.g. `report.pptx` + `report.pdf`), ingest one (PDF → modern Office → legacy) and record the rest as skipped duplicates. | | `CITADEL_IGNORE_PATTERNS` | (built-in OS/junk globs) | Case-insensitive globs skipped at discovery (`Thumbs.db`, `.DS_Store`, `~$` locks, editor swap/backup files). A comma/newline list **replaces** the defaults; a `+` prefix **extends** them. | +| `CITADEL_MAX_SOURCE_BYTES` | `0` (no limit) | Discovery **size** ceiling, in bytes — the complement to the name-matching patterns above. A raw file over it is skipped from the walk's own stat: never opened, never hashed, never ingested, and never recorded in the manifest or the failures catalog. It is *reported*, though — the run report's *Oversized* section and `citadel status`' *Oversized* bucket — so nothing is dropped silently. Set it when a raw root also holds machine data (a folder of multi-GB `.tdms` sensor dumps is useless to a wiki but expensive to scan: every untracked candidate is stream-hashed in full before anything can classify it as unreadable binary). Off by default, because silently skipping a large-but-legitimate source (a 2 GB lecture recording, a scanned archive PDF) would be worse than a slow scan. An explicitly named path (`citadel ingest big.tdms`) bypasses it — explicit always wins — and an already-ingested source that later crosses the ceiling stays in the wiki, it just stops being re-checked. | ## Audio/video sources (whisper) @@ -190,3 +191,35 @@ as `.obsidian/` are skipped at discovery, so a living vault is a clean source | `CITADEL_DOCS_DIR` | `docs` | Reference docs. | | `CITADEL_RAW_DIRS` | (single `raw/`) | Comma/newline-separated list of raw roots, each walked by ingest. **Replaces** the walk list (include `raw` to keep the workspace root — `citadel doctor` warns when the primary `raw/` holds files but is missing from the list). A page citing a source in a non-sibling root cites it by absolute posix path. Deletion detection is scoped per root — an unmounted root never reads as deleted sources. | | `CITADEL_WORKSPACE` | (walk up for `citadel.toml`) | Force the workspace root (useful for `citadel serve` launched from an arbitrary CWD). | + +### The wiki is never one of its own sources + +A raw root may sit **above** the wiki — `CITADEL_RAW_DIRS=T:\` with the wiki at +`T:\llmWiki\data-science\wiki` is a normal way to say "scan this whole drive". Discovery prunes the +wiki directory out of that walk (it is generated output, never a source), announces the exclusion +once per run, and sweeps any page an earlier run had already self-ingested out of the manifest and +the failures catalog. No `CITADEL_IGNORE_PATTERNS` entry is needed. `citadel doctor`'s **wiki +placement** check still WARNs about the nesting, because it costs clarity: with the wiki inside a +source tree, a stray citation *into* the wiki looks like legal provenance. + +### Windows mapped drives (`T:\…`) and UNC paths + +Every configured path is `resolve()`-d — that is what makes path identity (manifest keys, root +containment) unambiguous. On Windows, resolving a **mapped network drive** rewrites it into its UNC +form: `T:\team-wiki` becomes `\\fileserver\share\team-wiki`, whether your `.env` named the drive +letter or the share. That resolved path is fine as an identity, but it is a bad thing to hand a +child process: some agent CLIs refuse to run at all with a UNC working directory ("environment +blocks UNC/network paths"), and git treats the two spellings as different repositories +(`safe.directory` / `core.filemode` complaints). + +So citadel remembers the spelling you actually used — the `.env` value, or the drive your shell was +on — and hands **that** to the agent CLI (`cwd` and its directory grants) and to git, while the +resolved form stays the one identity everywhere else. The alias covers the whole **subtree**, so a +default `wiki/`, `raw/`, or a relative `CITADEL_WIKI_DIR=wiki` inherits it without needing its own +entry (if `\\fileserver\share\team-wiki` and `T:\team-wiki` are the same directory, so are their +children). Nothing is guessed: an alias is only recorded when citadel already holds both spellings +of the same directory, and only when resolution turned a non-UNC path into a UNC one — on POSIX, +and for ordinary Windows paths, nothing changes at all. +`citadel doctor`'s **child paths** check names the working directory sessions will actually use, and +WARNs when only a UNC spelling is known (map the share to a drive letter and point +`CITADEL_WORKSPACE` at it, or run citadel from that drive). diff --git a/docs/maintenance.md b/docs/maintenance.md index 296cd4c..9de02c7 100644 --- a/docs/maintenance.md +++ b/docs/maintenance.md @@ -88,8 +88,9 @@ source — **ingested** (with the importing model and rules version, plus `(stal was ingested under an older rulebook than the current one, `checked YYYY-MM-DD` — when a model last verified it, the ordering `citadel refresh` works through — and what that last verification cost when the backend reported it, e.g. `$0.05`), **failed** (with the reason and attempt -count), **skipped-duplicate**, **ignored** (which pattern matched), or **pending** (not yet -ingested — the next `citadel ingest` will pick it up). A `Recorded LLM cost` line above the table +count), **skipped-duplicate**, **ignored** (which pattern matched), **oversized** (past the +`CITADEL_MAX_SOURCE_BYTES` discovery ceiling, with the size that explains it), or **pending** (not +yet ingested — the next `citadel ingest` will pick it up). A `Recorded LLM cost` line above the table totals the per-source stamps (the maintenance-cost snapshot of the current corpus; `--json` carries it as `cost_usd_total`). It never runs an agent and never re-hashes sources, so it is always cheap to run. An MCP client gets the same table via the read-only diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 2388fc8..179f241 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -108,7 +108,8 @@ CLI (see [configuration — Audio/video sources](configuration.md#audiovideo-sou ### "Nothing got ingested" - Run `citadel status` — the read-only per-source state table shows exactly what happened to each - file: ingested, failed, skipped-duplicate, ignored (matched `CITADEL_IGNORE_PATTERNS`), or pending. + file: ingested, failed, skipped-duplicate, ignored (matched `CITADEL_IGNORE_PATTERNS`), oversized + (over `CITADEL_MAX_SOURCE_BYTES`), or pending. - Already-ingested sources are skipped by sha match — that's not a bug. To deliberately re-read one, use `citadel ingest --force `. - Watch a run live with `citadel ingest --verbose` (`-v`), or capture a full transcript per source @@ -142,6 +143,44 @@ viewer's sources lose their names/links, and `citadel lint` reports the citation citation and where it actually resolved, and prints the fix (set `CITADEL_RAW_DIR` to the `raw/` tree next to the wiki, or select the workspace with `CITADEL_WORKSPACE`). +### The scan is slow, or a raw root also holds huge machine-data files + +Discovery stream-hashes every new candidate in full before anything can classify it, so a folder of +multi-GB `.tdms` sensor dumps (or video, or database exports) costs real time on every first scan — +even though every one of them ends up recorded as unreadable binary. Set a **size ceiling**: +`CITADEL_MAX_SOURCE_BYTES=52428800` (50 MB) skips anything larger straight from the walk's own +`stat`, so those files are never opened. Skips are listed on the run report (*Oversized*) and in +`citadel status`, never dropped silently; naming a path explicitly (`citadel ingest big.tdms`) +ingests it anyway. Off by default — see +[configuration.md](configuration.md#what-gets-ingested). + +### Windows: the agent CLI fails on a mapped network drive (`T:\…`) + +Symptoms, on a workspace that works perfectly when you `cd` into it yourself: sessions fail with +*"environment blocks UNC/network paths"* or plain "file not found" for paths that plainly exist, and +`git init` / `git add` in the wiki complain about `core.filemode` or `safe.directory`. + +Cause: `Path.resolve()` rewrites a mapped drive into its UNC form (`T:\wiki` → +`\\fileserver\share\wiki`), and that resolved path used to be what citadel handed the agent CLI as +its working directory. Citadel now hands child processes the spelling you configured (the drive +letter) while keeping the resolved form as its internal identity — run `citadel doctor` and read the +**child paths** line to see which working directory sessions will use. If it WARNs that only a UNC +spelling is known, map the share to a drive letter and either run citadel from that drive or set +`CITADEL_WORKSPACE=T:\your-workspace`. + +WSL is not a workaround here: a DrvFs mount of the same SMB share (`/mnt/t/…`) does not support the +POSIX metadata operations the staging copy and `git init` need, so it fails differently (*Operation +not permitted*). Run citadel natively on Windows against the drive letter. + +### My whole wiki turned up as raw sources + +This happened when a raw root sat above the wiki (`CITADEL_RAW_DIRS=T:\` with the wiki inside it): +discovery walked the wiki's own pages back in as sources. Discovery now excludes the wiki directory +from every walk, and the next run also sweeps the self-ingested keys out of the manifest and the +failures catalog — so a plain `citadel ingest` cleans it up. The pages those sessions created are +ordinary wiki pages; delete the ones you don't want, or let `citadel curate` fold them in. +`citadel doctor`'s **wiki placement** check flags the nesting itself. + ### Where failures are recorded A source that could not be read (unreadable binary, an errored or timed-out session) is persisted diff --git a/tests/test_config_env.py b/tests/test_config_env.py index f3efc27..4f487d3 100644 --- a/tests/test_config_env.py +++ b/tests/test_config_env.py @@ -127,3 +127,32 @@ def test_wiki_git_unrecognized_value_warns_and_falls_back_to_auto(monkeypatch, r assert "CITADEL_WIKI_GIT" in config.CONFIG_WARNINGS[0] assert raw in config.CONFIG_WARNINGS[0] assert "auto" in config.CONFIG_WARNINGS[0] + + +# --- _max_source_bytes (the discovery size ceiling) ----------------------------------------- + + +@pytest.mark.parametrize("raw", ["", " ", "0"]) +def test_max_source_bytes_defaults_to_no_limit(monkeypatch, raw): + """Unset/blank/0 all mean "no ceiling" — the default behavior citadel has always had.""" + monkeypatch.setattr(config, "CONFIG_WARNINGS", []) + monkeypatch.setenv("CITADEL_MAX_SOURCE_BYTES", raw) + assert config._max_source_bytes() == 0 + assert config.CONFIG_WARNINGS == [] + + +def test_max_source_bytes_valid_value_parses(monkeypatch): + monkeypatch.setattr(config, "CONFIG_WARNINGS", []) + monkeypatch.setenv("CITADEL_MAX_SOURCE_BYTES", " 5242880 ") + assert config._max_source_bytes() == 5242880 + assert config.CONFIG_WARNINGS == [] + + +def test_max_source_bytes_negative_clamps_to_no_limit_and_warns(monkeypatch): + """A negative ceiling is a misconfiguration, not "unlimited" spelled oddly (mirroring + CITADEL_JOBS): it falls back to 0 AND records a warning doctor's config check surfaces.""" + monkeypatch.setattr(config, "CONFIG_WARNINGS", []) + monkeypatch.setenv("CITADEL_MAX_SOURCE_BYTES", "-1") + assert config._max_source_bytes() == 0 + assert len(config.CONFIG_WARNINGS) == 1 + assert "CITADEL_MAX_SOURCE_BYTES" in config.CONFIG_WARNINGS[0] diff --git a/tests/test_doctor.py b/tests/test_doctor.py index 2a63ac8..9118559 100644 --- a/tests/test_doctor.py +++ b/tests/test_doctor.py @@ -10,6 +10,7 @@ import json import socket +from pathlib import Path import pytest @@ -225,6 +226,53 @@ def test_raw_roots_ok_when_excluded_primary_raw_is_empty(tmp_citadel, monkeypatc assert "1 walked raw root(s) reachable" in c.detail +# --- wiki placement ------------------------------------------------------------------------ + + +def test_wiki_placement_ok_when_the_wiki_sits_outside_the_raw_roots(tmp_citadel): + c = doctor.check_wiki_placement() + assert c.status == doctor.OK + assert "outside every walked raw root" in c.detail + + +def test_wiki_placement_warns_when_the_wiki_is_inside_a_raw_root(make_citadel, tmp_path): + """The self-ingest layout (a whole mounted drive as one raw root, the wiki somewhere inside it). + Discovery excludes the wiki automatically, so this is a clarity WARN — but it must be said.""" + drive = tmp_path / "drive" + make_citadel(root=tmp_path / "repo", raw=drive, wiki=drive / "llmWiki" / "wiki") + c = doctor.check_wiki_placement() + assert c.status == doctor.WARN + assert "lies under walked raw root" in c.detail + assert "discovery excludes it automatically" in c.detail + + +# --- child paths (the UNC advisory) --------------------------------------------------------- + + +def test_child_paths_ok_without_any_unc_rewriting(tmp_citadel): + c = doctor.check_child_paths() + assert c.status == doctor.OK + assert "no UNC/network path rewriting" in c.detail + + +def test_child_paths_names_the_drive_letter_when_one_is_known(tmp_citadel, monkeypatch): + unc = Path(r"\\fileserver\share\team-wiki") + monkeypatch.setattr(config, "WORKSPACE_ROOT", unc) + monkeypatch.setattr(config, "NATIVE_FORMS", {config._path_id(unc): r"T:\team-wiki"}) + c = doctor.check_child_paths() + assert c.status == doctor.OK + assert r"T:\team-wiki" in c.detail + + +def test_child_paths_warns_when_only_a_unc_spelling_exists(tmp_citadel, monkeypatch): + """No drive-letter alias: the agent really does run on a UNC cwd, which some backends refuse.""" + monkeypatch.setattr(config, "WORKSPACE_ROOT", Path(r"\\fileserver\share\team-wiki")) + monkeypatch.setattr(config, "NATIVE_FORMS", {}) + c = doctor.check_child_paths() + assert c.status == doctor.WARN + assert "UNC" in c.detail and "CITADEL_WORKSPACE" in c.detail + + # --- manifest ---------------------------------------------------------------------------- @@ -719,6 +767,8 @@ def test_run_emits_the_full_check_inventory(tmp_citadel, monkeypatch): "agent CLI", "ingest model", "raw roots", + "wiki placement", + "child paths", "manifest", "failures", "billing", diff --git a/tests/test_ingest_discovery.py b/tests/test_ingest_discovery.py index 64188ec..e02070f 100644 --- a/tests/test_ingest_discovery.py +++ b/tests/test_ingest_discovery.py @@ -343,3 +343,196 @@ def test_failures_are_persisted_surfaced_and_cleared(tmp_citadel, fake_agent, tr data2 = json.loads(fpath.read_text(encoding="utf-8")) assert "raw/notes.md" not in data2 # succeeded -> dropped assert data2["raw/blob.bin"]["reason"] == "unreadable" # still stuck -> stays across runs + + +# --- the wiki is never its own raw source --------------------------------------------------- + + +@pytest.fixture +def wiki_under_raw(make_citadel, tmp_path): + """The self-ingest layout: ONE raw root (a whole mounted drive, say) with the wiki INSIDE it — + ``CITADEL_RAW_DIRS=T:\\`` and the wiki at ``T:\\llmWiki\\ds\\wiki``. Discovery must exclude the + wiki from the walk; nothing but a hand-written ignore pattern used to.""" + drive = tmp_path / "drive" + return make_citadel(root=tmp_path / "repo", raw=drive, wiki=drive / "llmWiki" / "ds" / "wiki") + + +def test_wiki_under_a_raw_root_is_never_discovered(wiki_under_raw, seed_page): + """The wiki's own pages (and its generated index) are not candidates, while real sources in the + same root are — the walk prunes the wiki directory whole.""" + seed_page("concepts/thing.md", {"type": "Concept", "title": "T", "description": "d", "tags": ["x"]}) + (wiki_under_raw.wiki / "index.md").write_text("# Index\n", encoding="utf-8") + (wiki_under_raw.raw / "notes.md").write_text("real source\n", encoding="utf-8") + (wiki_under_raw.raw / "llmWiki").mkdir(exist_ok=True) + (wiki_under_raw.raw / "llmWiki" / "readme.md").write_text("also a real source\n", encoding="utf-8") + + got = {p.relative_to(wiki_under_raw.raw).as_posix() for p in ingest._candidates(None)} + assert got == {"notes.md", "llmWiki/readme.md"} + + +def test_wiki_pages_are_not_ingested_and_the_exclusion_is_announced( + wiki_under_raw, fake_agent, seed_page, cite_page, capsys +): + """End to end: a wiki page sitting under the raw root is neither ingested nor tracked, and the + run says out loud that the wiki was excluded (silence would read as "there was nothing there").""" + seed_page("concepts/thing.md", {"type": "Concept", "title": "T", "description": "d", "tags": ["x"]}) + src = wiki_under_raw.raw / "notes.md" + src.write_text("Transformers use self-attention.\n", encoding="utf-8") + key = manifest.rel_key(src) + agent = fake_agent(side_effect=lambda *a, **k: cite_page("concepts/transformer.md", key, "A fact.")) + + report = ingest.ingest() + assert [called for called, _kind in agent.calls] == [key] + assert report.processed == [key] + tracked = wiki_under_raw.read_manifest() + assert not [k for k in tracked if "/wiki/" in k], tracked + assert "excluded from discovery" in capsys.readouterr().err + + +def test_prior_self_ingested_wiki_entries_are_pruned(wiki_under_raw, fake_agent, seed_page): + """A wiki page ingested by an EARLIER run (before the guard existed) is swept out of the + manifest and the failures catalog — it still exists on disk, so deletion detection would never + clean it up, and it would sit in wiki/sources/index.md forever.""" + seed_page("concepts/thing.md", {"type": "Concept", "title": "T", "description": "d", "tags": ["x"]}) + agent = fake_agent() + page_key = manifest.rel_key(wiki_under_raw.wiki / "concepts" / "thing.md") + seeded = manifest.load() + seeded[page_key] = manifest.make_entry("aa" * 32, "claude:sonnet") + manifest.save(seeded) + fails = failures.load() + failures.record(fails, page_key + ".bak", failures.UNREADABLE, "no extractable text") + failures.save(fails) + + ingest.ingest() + assert agent.count == 0 + assert page_key not in manifest.load() + assert page_key + ".bak" not in failures.load() + + +def test_explicit_path_inside_the_wiki_is_still_refused(wiki_under_raw, seed_page, monkeypatch): + """Explicit wins over hidden names, ignore globs and the size ceiling — but not over the wiki + guard: a generated page cannot be turned into a source by naming it, absolutely OR relatively + (the relative form is what a user actually types).""" + page = seed_page("concepts/thing.md", {"type": "Concept", "title": "T", "description": "d", "tags": ["x"]}) + assert ingest._candidates([str(page)]) == [] + assert ingest._candidates([str(wiki_under_raw.wiki)]) == [] + + monkeypatch.chdir(wiki_under_raw.wiki.parent) + assert ingest._candidates(["wiki/concepts/thing.md"]) == [] + assert ingest._candidates(["wiki"]) == [] + + +def test_a_git_backed_wiki_is_not_ingested_as_a_repo_source(wiki_under_raw, seed_page, monkeypatch): + """CITADEL_WIKI_GIT makes the wiki dir its OWN git repo — i.e. a repo source by every other + measure. Neither the walk nor an explicit path may digest it as one.""" + monkeypatch.setattr(config, "REPO_SUPPORT", True, raising=False) + seed_page("concepts/thing.md", {"type": "Concept", "title": "T", "description": "d", "tags": ["x"]}) + (wiki_under_raw.wiki / ".git").mkdir() + + assert ingest._discover_repos(None, ingest._discover_walk(None)) == [] + explicit = [str(wiki_under_raw.wiki)] + assert ingest._discover_repos(explicit, ingest._discover_walk(explicit)) == [] + + +def test_raw_root_that_is_the_wiki_walks_nothing_and_arms_no_sweep(make_citadel, tmp_path, seed_page): + """The degenerate config (a raw root that IS the wiki): the walk is refused outright, and the + root is never counted as entered — so the deletion sweep stays disarmed rather than reading the + whole corpus as vanished.""" + both = tmp_path / "both" + make_citadel(root=tmp_path / "repo", raw=both, wiki=both) + seed_page("concepts/thing.md", {"type": "Concept", "title": "T", "description": "d", "tags": ["x"]}) + + walk = ingest._discover_walk(None) + assert walk.files == [] and walk.entered_roots == [] + assert walk.excluded_wiki == [both] + + +# --- the discovery size ceiling (CITADEL_MAX_SOURCE_BYTES) ----------------------------------- + + +def test_oversized_files_are_skipped_without_being_hashed(tmp_citadel, monkeypatch): + """Over the ceiling: skipped at discovery from the walk's own stat — the file is never opened, + so no sha256 is streamed over it (the whole point for a folder of multi-GB machine data).""" + raw = tmp_citadel.raw + (raw / "small.md").write_text("x" * 100, encoding="utf-8") + (raw / "dump.tdms").write_bytes(b"\x00" * 5000) + monkeypatch.setattr(config, "MAX_SOURCE_BYTES", 1000) + monkeypatch.setattr(manifest, "file_sha256", lambda p: pytest.fail(f"hashed {p}")) + + walk = ingest._discover_walk(None) + assert [p.name for p, _st in walk.files] == ["small.md"] + assert [(p.name, size) for p, size in walk.oversized] == [("dump.tdms", 5000)] + + +def test_oversized_files_are_reported_and_never_tracked(tmp_citadel, fake_agent, transformer_page, monkeypatch, capsys): + """A size skip is visible (run report + a stderr NOTE) but, like an ignore-pattern match, is + never recorded in the manifest or the failures catalog — it is not a failure, just out of scope.""" + raw = tmp_citadel.raw + (raw / "notes.md").write_text("Transformers use self-attention.\n", encoding="utf-8") + (raw / "dump.tdms").write_bytes(b"\x00" * 4096) + monkeypatch.setattr(config, "MAX_SOURCE_BYTES", 1024) + fake_agent(transformer_page) + + report = ingest.ingest() + assert report.processed == ["raw/notes.md"] + assert report.oversized == [("raw/dump.tdms", 4096)] + assert "raw/dump.tdms (4.0 KB)" in report.render() + assert "CITADEL_MAX_SOURCE_BYTES" in capsys.readouterr().err + assert "raw/dump.tdms" not in tmp_citadel.read_manifest() + assert "raw/dump.tdms" not in failures.load() + + +def test_size_ceiling_is_off_by_default(tmp_citadel): + """0 (the default) means no limit at all — the behavior citadel has always had.""" + (tmp_citadel.raw / "big.md").write_text("y" * 20000, encoding="utf-8") + assert config.MAX_SOURCE_BYTES == 0 + assert [p.name for p in ingest._candidates(None)] == ["big.md"] + + +def test_explicitly_named_oversized_path_is_still_ingested(tmp_citadel, monkeypatch): + """The ceiling is a scan-budget policy, not a ban: naming the file explicitly ingests it.""" + big = tmp_citadel.raw / "dump.tdms" + big.write_bytes(b"x" * 5000) + monkeypatch.setattr(config, "MAX_SOURCE_BYTES", 1000) + assert ingest._candidates([str(big)]) == [big] + + +def test_a_tracked_source_growing_past_the_ceiling_is_not_swept_as_deleted(tmp_citadel, fake_agent, monkeypatch): + """An already-ingested source that later crosses the ceiling drops out of the walk. It must NOT + read as deleted (its provenance would be reconciled out of a wiki that is still correct): the + sweep's positive .exists() confirmation is what saves it — it simply stops being re-checked.""" + raw = tmp_citadel.raw + src = raw / "notes.md" + src.write_text("z" * 5000, encoding="utf-8") + tracked = manifest.load() + tracked["raw/notes.md"] = manifest.make_entry(manifest.file_sha256(src), "claude:sonnet") + manifest.save(tracked) + monkeypatch.setattr(config, "MAX_SOURCE_BYTES", 1000) + agent = fake_agent() + + report = ingest.ingest() + assert report.sources_deleted == [] + assert agent.count == 0 + assert "raw/notes.md" in manifest.load() + + +def test_explicit_paths_expand_a_leading_tilde(tmp_citadel, wiki_under_raw, monkeypatch): + """`~` is expanded for explicitly requested paths, like every other configured path already is. + A POSIX shell expands it first, but Windows cmd.exe / PowerShell hand a native binary the + literal `~` — so it used to stat away to nothing, and the wiki guard could not recognize such a + path as wiki-internal either. Both now hold by construction.""" + monkeypatch.setenv("HOME", str(wiki_under_raw.raw)) + monkeypatch.setenv("USERPROFILE", str(wiki_under_raw.raw)) # Windows' home variable + (wiki_under_raw.raw / "notes.md").write_text("real source\n", encoding="utf-8") + + assert ingest._candidates(["~/notes.md"]) == [wiki_under_raw.raw / "notes.md"] + # ...and the wiki guard sees through it: ~ pointing INTO the wiki is still refused. + (wiki_under_raw.wiki / "concepts").mkdir(parents=True, exist_ok=True) + (wiki_under_raw.wiki / "concepts" / "thing.md").write_text("generated\n", encoding="utf-8") + rel = wiki_under_raw.wiki.relative_to(wiki_under_raw.raw) + assert ingest._candidates([f"~/{rel.as_posix()}/concepts/thing.md"]) == [] + + +def test_explicit_path_with_an_unresolvable_home_never_raises(tmp_citadel): + """`~nosuchuser` must degrade, not crash: Path.expanduser() raises there, os.path's does not.""" + assert ingest._candidates(["~nosuchuser-zzz/notes.md"]) == [] diff --git a/tests/test_netdrive.py b/tests/test_netdrive.py index af95653..d1a5788 100644 --- a/tests/test_netdrive.py +++ b/tests/test_netdrive.py @@ -19,6 +19,7 @@ from __future__ import annotations import os +import subprocess from pathlib import Path from conftest import PROMPT_CHAR_BUDGET @@ -371,3 +372,152 @@ def shortened_session(rel_key, kind="ingest"): # The canonical resource matches the manifest key, so later move/delete lookups find the page. assert store.find_raw_references(abs_key) == ["concepts/internal-data-analysis.md"] assert ingest.ingest([str(source)]).processed == [] # idempotent on the abs key + + +# --- Windows mapped drives: the UNC spelling handed to child processes ------------------- + + +def test_is_unc_path_recognizes_both_separators(): + """A pure string test, so it stays meaningful on the POSIX box CI runs on.""" + assert config._is_unc_path(r"\\fileserver\share\team-wiki") + assert config._is_unc_path("//fileserver/share/team-wiki") + assert not config._is_unc_path(r"T:\team-wiki") + assert not config._is_unc_path("/mnt/share/team-wiki") + + +def test_prefer_native_only_fires_for_a_unc_rewrite(): + """An alias is worth remembering ONLY when resolution turned a non-UNC path into a UNC one. + Everything else keeps a single spelling, which is what keeps the registry empty off Windows.""" + assert config._prefer_native(r"\\srv\share\ws", r"T:\ws") + assert not config._prefer_native(r"T:\ws", r"T:\ws") # identical + assert not config._prefer_native(r"T:\ws", r"T:\other") # resolution did not produce a UNC path + assert not config._prefer_native(r"\\srv\share\ws", r"\\srv\share\ws\..\ws") # both UNC + assert not config._prefer_native("/mnt/share/ws", "/home/me/ws") # POSIX: never + + +def test_native_form_is_the_identity_function_without_a_recorded_alias(tmp_citadel): + """No registry entry (every POSIX layout, every ordinary Windows path) means every child + process is handed exactly the path it was handed before this existed.""" + assert config.NATIVE_FORMS == {} + assert config.native_form(tmp_citadel.root) == tmp_citadel.root + assert config.child_cwd() == str(tmp_citadel.root) + + +def test_child_cwd_prefers_the_recorded_drive_letter(tmp_citadel, monkeypatch): + """The reported bug: `Path.resolve()` rewrites a mapped drive (T:\\...) into its UNC form, and + that resolved path was handed to the agent CLI as cwd — where some backends refuse to run at + all ("environment blocks UNC/network paths"). The resolved form stays the identity; only what + a CHILD is handed switches back to the drive letter.""" + unc = Path(r"\\fileserver\share\team-wiki") + monkeypatch.setattr(config, "WORKSPACE_ROOT", unc) + monkeypatch.setattr(config, "NATIVE_FORMS", {config._path_id(unc): r"T:\team-wiki"}) + + assert config.child_cwd() == r"T:\team-wiki" + assert config.WORKSPACE_ROOT == unc # identity is untouched — only the child spelling changed + assert config.native_form(Path(r"\\fileserver\share\other")) == Path(r"\\fileserver\share\other") + + +def test_record_native_form_only_stores_a_unc_rewrite(monkeypatch): + monkeypatch.setattr(config, "NATIVE_FORMS", {}) + config._record_native_form(Path("/mnt/share/ws"), Path("/home/me/ws")) + assert config.NATIVE_FORMS == {} + config._record_native_form(Path(r"\\srv\share\ws"), Path(r"T:\ws")) + assert config.NATIVE_FORMS == {config._path_id(r"\\srv\share\ws"): r"T:\ws"} + + +def test_run_session_spawns_the_cli_in_the_child_friendly_cwd(monkeypatch): + """The whole point of the fix: the subprocess actually gets the drive-letter cwd.""" + unc = Path(r"\\fileserver\share\team-wiki") + monkeypatch.setattr(config, "WORKSPACE_ROOT", unc) + monkeypatch.setattr(config, "NATIVE_FORMS", {config._path_id(unc): r"T:\team-wiki"}) + seen = {} + + class _Proc: + returncode = 0 + stdout = '{"type":"result","is_error":false,"result":"done"}' + stderr = "" + + def fake_run(*a, **kwargs): + seen.update(kwargs) + return _Proc() + + monkeypatch.setattr(subprocess, "run", fake_run) + llm._run_session("claude", ["claude", "-p"], "PROMPT") + assert seen["cwd"] == r"T:\team-wiki" + + +def test_external_dir_grants_use_the_child_friendly_spelling(tmp_citadel_external, monkeypatch): + """`--add-dir` has to name the paths the agent can actually reach from its cwd: on a mapped + drive that is the drive letter, not resolve()'s UNC rewrite.""" + # Keyed off the RESOLVED path, exactly as _external_dirs looks it up. + raw_dir = Path(config.RAW_DIR).resolve() + monkeypatch.setattr(config, "NATIVE_FORMS", {config._path_id(raw_dir): r"T:\team-wiki\raw"}) + granted = llm._external_dirs(config.rel_or_abs_posix(raw_dir / "notes.md")) + assert r"T:\team-wiki\raw" in granted + assert str(raw_dir) not in granted + + +def test_wikigit_runs_git_against_the_child_friendly_path(tmp_citadel, monkeypatch): + """git for Windows treats a mapped drive and its UNC form as different repositories, so the + drive-letter path the user already ran `git init` / safe.directory against is the one to use.""" + from citadel import wikigit + + unc = Path(r"\\fileserver\share\team-wiki\wiki") + monkeypatch.setattr(config, "NATIVE_FORMS", {config._path_id(unc): r"T:\team-wiki\wiki"}) + monkeypatch.setattr(wikigit.shutil, "which", lambda name: "/usr/bin/git") + seen = {} + + class _Proc: + returncode = 0 + stdout = "" + stderr = "" + + def fake_run(argv, **kwargs): + seen["argv"] = argv + return _Proc() + + monkeypatch.setattr(wikigit.subprocess, "run", fake_run) + wikigit._git(unc, "status", "--porcelain") + assert seen["argv"][:3] == ["/usr/bin/git", "-C", r"T:\team-wiki\wiki"] + + +def test_native_form_is_inherited_by_paths_under_an_aliased_root(monkeypatch): + """The alias covers the whole SUBTREE, which is what makes the fix reach the derived paths: a + default wiki is `WORKSPACE_ROOT / "wiki"` with no CITADEL_WIKI_DIR to record a spelling for, + and it is exactly what git gets as `-C`. Sound because a drive mapping maps the whole tree.""" + unc = Path(r"\\fileserver\share\team-wiki") + monkeypatch.setattr(config, "NATIVE_FORMS", {config._path_id(unc): r"T:\team-wiki"}) + + # Built the same way on both sides: the separator is the platform's, not this file's. + drive = Path(r"T:\team-wiki") + assert config.native_form(unc / "wiki") == drive / "wiki" + assert config.native_form(unc / "raw" / "sub") == drive / "raw" / "sub" + # Unrelated trees keep their own spelling — inheritance is containment, not a global rewrite. + other = Path(r"\\fileserver\share\other\wiki") + assert config.native_form(other) == other + + +def test_wikigit_uses_the_drive_letter_for_a_default_wiki_under_an_aliased_workspace(tmp_citadel, monkeypatch): + """The reviewer's case end to end: no CITADEL_WIKI_DIR at all, so the wiki is just + `WORKSPACE_ROOT / "wiki"` — and git must still be pointed at the drive letter.""" + from citadel import wikigit + + unc = Path(r"\\fileserver\share\team-wiki") + monkeypatch.setattr(config, "WORKSPACE_ROOT", unc) + monkeypatch.setattr(config, "WIKI_DIR", unc / "wiki") + monkeypatch.setattr(config, "NATIVE_FORMS", {config._path_id(unc): r"T:\team-wiki"}) + monkeypatch.setattr(wikigit.shutil, "which", lambda name: "/usr/bin/git") + seen = {} + + class _Proc: + returncode = 0 + stdout = "" + stderr = "" + + def fake_run(argv, **kwargs): + seen["argv"] = argv + return _Proc() + + monkeypatch.setattr(wikigit.subprocess, "run", fake_run) + wikigit._git(Path(config.wiki_dir()), "status", "--porcelain") + assert seen["argv"][:3] == ["/usr/bin/git", "-C", str(Path(r"T:\team-wiki") / "wiki")] diff --git a/tests/test_status.py b/tests/test_status.py index 85e09ca..8fb6334 100644 --- a/tests/test_status.py +++ b/tests/test_status.py @@ -126,13 +126,13 @@ def test_cli_status_exits_0_with_every_section_header(tmp_citadel, capsys): assert cli.main(["status"]) == 0 out = capsys.readouterr().out assert "Corpus status" in out - for heading in ("Ingested (", "Failed (", "Skipped as duplicate (", "Ignored (", "Pending ("): + for heading in ("Ingested (", "Failed (", "Skipped as duplicate (", "Ignored (", "Oversized (", "Pending ("): assert heading in out assert "raw/good.md" in out and "raw/dup.pdf" in out and "raw/pending.md" in out def test_cli_status_json_emits_machine_readable_buckets(tmp_citadel, capsys): - """``status --json`` dumps the five buckets + rules_version as one JSON object, so a script + """``status --json`` dumps the six buckets + rules_version as one JSON object, so a script gets 'which sources failed and why' without scraping the table.""" import json @@ -147,6 +147,7 @@ def test_cli_status_json_emits_machine_readable_buckets(tmp_citadel, capsys): assert [s["key"] for s in data["ingested"]] == ["raw/good.md"] assert data["failed"][0]["key"] == "raw/bad.bin" and data["failed"][0]["reason"] == failures.UNREADABLE assert data["pending"] == ["raw/pending.md"] + assert data["oversized"] == [] def test_cli_status_exit_code_gates_on_failed_or_pending(tmp_citadel, capsys): @@ -159,3 +160,18 @@ def test_cli_status_exit_code_gates_on_failed_or_pending(tmp_citadel, capsys): _track("raw/pending.md", manifest.file_sha256(tmp_citadel.raw / "pending.md"), config.rules_version()) assert cli.main(["status", "--exit-code"]) == 0 capsys.readouterr() + + +def test_oversized_file_is_its_own_bucket_not_pending(tmp_citadel, monkeypatch): + """A file past ``CITADEL_MAX_SOURCE_BYTES`` is neither pending (ingest will never pick it up) + nor failed (nothing went wrong) — it gets its own bucket, with the size that explains it.""" + (tmp_citadel.raw / "dump.tdms").write_bytes(b"\x00" * 4096) + (tmp_citadel.raw / "real.md").write_text("real\n", encoding="utf-8") + monkeypatch.setattr(config, "MAX_SOURCE_BYTES", 1024) + + report = status.build_status() + assert report.oversized == [("raw/dump.tdms", 4096)] + assert "raw/dump.tdms" not in report.pending + assert "raw/real.md" in report.pending + assert "raw/dump.tdms 4.0 KB" in report.render() + assert report.as_dict()["oversized"] == [{"key": "raw/dump.tdms", "size_bytes": 4096}]