diff --git a/.gitignore b/.gitignore index ae0ac10..069e6b8 100644 --- a/.gitignore +++ b/.gitignore @@ -216,3 +216,17 @@ __marimo__/ # per-run copy of the operator's OAuth credential plus claude-stream logs and a # clone of the fixture target — local-only, must never be committed. .e2e-runs/ + +# Bonfire's own knowledge-backend store. .bonfire/vault is the documented +# default path for BOTH on-disk shapes that store can take, and one pattern +# has to cover both: the LanceDB backend makes it a DIRECTORY holding a vector +# index built from this repo's own source, while the SQLite backend hands the +# same path to sqlite3.connect and gets a REGULAR FILE whose +# vault_entries.content column holds that source as cleartext. Hence NO +# trailing slash — a directory-only pattern cannot reach the file shape, which +# is the worse of the two to publish. The laws and the mechanism travel; the +# operator's contents never do. No other .bonfire-specific pattern lives here: +# sessions/ handoffs, context.json and costs.jsonl stay committable, which is +# why there is no bare .bonfire/ line. (The broad patterns elsewhere in this +# file — *.log, .env*, .cache — reach into .bonfire/ as they reach everywhere.) +.bonfire/vault diff --git a/CHANGELOG.md b/CHANGELOG.md index eabb6f4..2f3e875 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,21 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Changed +- `bonfire init` now seeds two `.gitignore` entries instead of one: + `.bonfire/tools.local.toml`, the per-machine tool inventory, and + `.bonfire/vault`, where the knowledge backend keeps its store once an + operator enables a persistent one. The store entry carries no trailing + slash because `.bonfire/vault` is the documented default path for both + on-disk shapes that store can take, and one pattern has to cover both: + the LanceDB backend makes it a directory holding a vector index built + from the operator's own source, while the SQLite backend hands the same + path to `sqlite3.connect` and gets a regular file whose row content + holds that source as cleartext. A directory-only pattern would leave + the file shape stageable, which is the worse of the two to publish. + Nothing else under `.bonfire/` is covered: `sessions/` handoffs, + `context.json` and `costs.jsonl` stay committable. Re-running `init` + still duplicates neither line, and the success output names both + entries. The repo's own `.gitignore` moved to the same pattern. - README: the "What's Not There Yet" list is re-derived against the current tree. `bonfire run` exists and is documented; `status`, `resume` and `handoff` are implementations rather than stubs; four diff --git a/README.md b/README.md index 8698394..f2ae2f2 100644 --- a/README.md +++ b/README.md @@ -117,8 +117,9 @@ Claude Code session. # - .bonfire/ (per-project state directory) # - agents/ (role-local prompt + identity-block overrides; see # Extension Points) -# - .gitignore entry: `.bonfire/tools.local.toml` (appended if -# missing; idempotent — re-running does not duplicate the line) +# - .gitignore carrying two entries, appended if missing (idempotent; +# re-running duplicates neither line): `.bonfire/tools.local.toml` +# and `.bonfire/vault`, the knowledge-backend store bonfire init . # Drive a prompt through a workflow and the pipeline engine. Needs a diff --git a/file-budget.json b/file-budget.json index a0e9ed4..222fe93 100644 --- a/file-budget.json +++ b/file-budget.json @@ -25,6 +25,9 @@ "tests/unit/test_events.py": 675, "tests/unit/test_git.py": 754, "tests/unit/test_github.py": 846, + "tests/unit/test_init_gitignore_width.py": { + "purpose": "the two-direction width contract for the .gitignore that bonfire init seeds: the operator-local paths under .bonfire/ are covered in BOTH on-disk shapes the knowledge store's single default path can take (a LanceDB directory and a SQLite regular file), while every committable sub-path (sessions, context.json, the cost ledger) stays stageable. Split out of test_tools_section_is_local.py, which owns the tools-local-file pins and was at its frozen size: this is one contract with its own module, and the split returns that file BELOW its frozen number rather than raising it. Graded by real git check-ignore against a real repository with git's own global and system config neutralised, so a contributor's excludes file cannot make it pass or fail." + }, "tests/unit/test_merge_preflight_handler.py": 1139, "tests/unit/test_onboard_scanner_claude_memory.py": 562, "tests/unit/test_onboard_scanner_cli_toolchain.py": 504, diff --git a/src/bonfire/cli/commands/init.py b/src/bonfire/cli/commands/init.py index b378beb..a4382db 100644 --- a/src/bonfire/cli/commands/init.py +++ b/src/bonfire/cli/commands/init.py @@ -34,20 +34,40 @@ # sites stay consistent. _INIT_READ_MAX_BYTES: int = 1 * 1024 * 1024 -# ``.bonfire/`` carries a MIX of operator-local state (the per-machine -# ``tools.local.toml`` written by ``bonfire scan``) AND artefacts that -# ARE committable: ``.bonfire/sessions`` (handoff history), -# ``.bonfire/context.json`` (project config), ``.bonfire/vault`` -# (knowledge backend seed), ``.bonfire/costs.jsonl`` (cost ledger, when -# operator opts in to commit). A broad ``.bonfire/`` ignore would -# silently exclude those committable sub-paths and break workflows that -# depend on them landing in git. The narrower entry names the single -# operator-local file the W8.G work introduced so other sub-paths under -# ``.bonfire/`` remain stageable by default — a contract pinned by the -# gitignore-narrowness test in ``test_tools_section_is_local.py``. The -# operator can still add broader patterns to ``.gitignore`` by hand if -# they want; ``bonfire init`` does not assume that policy. +# ``.bonfire/`` carries a MIX of state that must never leave the +# operator's machine AND state that is committable by design. The seeded +# entries name ONLY the former, one line each. +# +# Committable — deliberately NOT ignored: ``.bonfire/sessions/`` (handoff +# history operators commit so the next session picks up the thread), +# ``.bonfire/context.json`` (project config, portable by design), +# ``.bonfire/costs.jsonl`` (cost ledger, committed when the operator opts +# in to keeping the spend record in the repo). +# +# Never committable — seeded here: +# * ``.bonfire/tools.local.toml`` — the host tool inventory and version +# footprint ``bonfire scan`` stamps; per-machine, and a privacy leak +# in a public repo. (Introduced by the W8.G migration.) +# * ``.bonfire/vault`` — the knowledge backend's store, once an +# operator enables a persistent one. NO trailing slash: one default +# path, TWO on-disk shapes, and one pattern must cover both. LanceDB +# makes it a DIRECTORY (a vector index of the operator's own source); +# SQLite hands it to ``sqlite3.connect``, making it a REGULAR FILE +# holding the indexed source as cleartext — the worse leak, and the +# shape a directory-only pattern cannot reach. Slash-less covers both. +# +# A broad ``.bonfire/`` ignore would be simpler and is deliberately NOT +# used: it would silently exclude the committable sub-paths above and +# break the workflows that depend on them landing in git. BOTH +# directions — the vault IS covered, no other ``.bonfire`` sub-path is — +# are pinned by ``tests/unit/test_init_gitignore_width.py``. An operator +# may still add broader patterns by hand; ``bonfire init`` does not +# assume that policy. ``_GITIGNORE_LINE`` keeps its name and +# single-string shape (the tools-file entry, imported by name from the +# hardening tests); ``_GITIGNORE_LINES`` is the full seeded sequence. _GITIGNORE_LINE = ".bonfire/tools.local.toml" +_GITIGNORE_VAULT_LINE = ".bonfire/vault" +_GITIGNORE_LINES: tuple[str, ...] = (_GITIGNORE_LINE, _GITIGNORE_VAULT_LINE) # --------------------------------------------------------------------------- @@ -184,13 +204,26 @@ def _make_directory_or_refuse(path: Path, label: str) -> None: def _ensure_gitignore_entry(target: Path, line: str) -> None: - """Append ``line`` to ``target/.gitignore`` iff not already present. + """Seed exactly one entry — the single-line primitive. + + Kept as the narrow entry point (one line in, one write out): that is + the shape the symlink/TOCTOU hardening tests drive directly. + """ + _ensure_gitignore_entries(target, (line,)) + + +def _ensure_gitignore_entries(target: Path, lines: tuple[str, ...]) -> None: + """Append each of ``lines`` to ``target/.gitignore`` iff absent. Idempotent: re-running ``bonfire init`` MUST NOT duplicate an entry (the no-duplicate canary pins this). The presence check matches a - stripped/non-comment line against the requested entry; existing - comments and blank lines are preserved. Creates ``.gitignore`` if - absent. + stripped line against each requested entry; existing comments and + blank lines are preserved. Creates ``.gitignore`` if absent. + + All missing entries land in ONE write — the fresh-file branch emits + the header plus every line, the extend branch appends the missing + lines as a single payload — so the number of writes is independent + of the number of entries. Uses :func:`safe_write_text` (W7.M) when creating the file fresh and :func:`safe_append_text` (W7.M append helper) when extending @@ -219,19 +252,21 @@ def _ensure_gitignore_entry(target: Path, line: str) -> None: _require_regular_file_slot(gitignore_path, ".gitignore") if not gitignore_path.exists(): - # Fresh file — create with the entry and a brief header so a + # Fresh file — create with the entries and a brief header so a # future contributor reading ``.gitignore`` understands why the - # operator-local file is excluded. - body = f"# Bonfire — operator-local state (do not commit).\n{line}\n" - _write_or_refuse(gitignore_path, body, label=".gitignore") + # operator-local paths are excluded. + header = "# Bonfire — operator-local state (do not commit).\n" + _write_or_refuse(gitignore_path, header + _as_entry_block(lines), label=".gitignore") return # W11 M2: route through ``safe_read_capped_text`` so the read uses # ``O_NOFOLLOW`` defense-in-depth against a race-planted symlink # between the ``is_symlink`` pre-check above and this read. existing = _read_or_refuse(gitignore_path, ".gitignore") - if line in [ln.strip() for ln in existing.splitlines()]: - # Already covered — idempotent no-op. + present = {ln.strip() for ln in existing.splitlines()} + missing = tuple(line for line in lines if line not in present) + if not missing: + # Every entry already covered — idempotent no-op. return # Append on a fresh line. Ensure exactly one trailing newline before @@ -251,7 +286,16 @@ def _ensure_gitignore_entry(target: Path, line: str) -> None: # is refused at ``open(2)`` time by the kernel rather than slipping # through to an attacker-controlled target. suffix = "" if existing.endswith("\n") else "\n" - _append_or_refuse(gitignore_path, suffix + f"{line}\n", label=".gitignore") + _append_or_refuse(gitignore_path, suffix + _as_entry_block(missing), label=".gitignore") + + +def _as_entry_block(lines: tuple[str, ...]) -> str: + """Render ``lines`` as newline-terminated ``.gitignore`` entries. + + One place decides that each seeded entry owns its line and ends in + ``\\n``, so the fresh-file body and the append payload cannot drift. + """ + return "".join(f"{line}\n" for line in lines) def _existing_gitignore_entries(gitignore_path: Path) -> set[str]: @@ -268,7 +312,7 @@ def _existing_gitignore_entries(gitignore_path: Path) -> set[str]: body = safe_read_capped_text(gitignore_path, max_bytes=_INIT_READ_MAX_BYTES) except (OSError, ValueError): return set() - return {ln.strip() for ln in body.splitlines()} & {_GITIGNORE_LINE} + return {ln.strip() for ln in body.splitlines()} & set(_GITIGNORE_LINES) def _has_legacy_tools_section(toml_path: Path) -> bool: @@ -348,9 +392,9 @@ def _report(target: Path, pre_existed: dict[str, bool], seeded_before: set[str]) Quick Start enumerated only the subset (``bonfire.toml`` + ``.bonfire/``) and the prior success message hid the rest: the ``agents/`` scaffold the prompt compiler reads from, and the - operator-local-state line appended to ``.gitignore``. A README - reconciliation test pins this list against the README so the two - cannot drift. + operator-local-state lines appended to ``.gitignore``. The README + reconciliation test only checks that four artefact tokens appear near + the README's ``bonfire init`` example, so it cannot notice a NEW one. Per-artefact verb prefix: ``Created:`` when the artefact was created this run, ``Already present:`` when it pre-existed. The artefact-name @@ -370,12 +414,15 @@ def _verb(existed: bool) -> str: f" - {_verb(pre_existed['agents_dir'])}: agents/ " "(role-local prompt + identity-block overrides)" ) - # The .gitignore entry is reported per-entry, not per-file, because - # the file may pre-exist with unrelated user content while the entry - # is freshly appended. Reporting "Already present" only when BOTH the - # file and the line existed before this run keeps the truth honest. - entry_existed = pre_existed["gitignore"] and _GITIGNORE_LINE in seeded_before - typer.echo(f" - {_verb(entry_existed)}: .gitignore entry: {_GITIGNORE_LINE}") + # The .gitignore entries are reported per-entry, not per-file: the + # file may pre-exist with user content while one entry is freshly + # appended and the other was already there. Each line's verb follows + # whether THAT entry was in the file before this run, so a fresh + # append never reads "Already present" and neither is a pre-existing + # entry announced as "Created". + for line in _GITIGNORE_LINES: + entry_existed = pre_existed["gitignore"] and line in seeded_before + typer.echo(f" - {_verb(entry_existed)}: .gitignore entry: {line}") def init( @@ -444,11 +491,9 @@ def init( _make_directory_or_refuse(bonfire_dir, ".bonfire/") _make_directory_or_refuse(agents_dir, "agents/") - # W8.G — seed .gitignore so ``.bonfire/tools.local.toml`` (and any - # future operator-local file under ``.bonfire/``) is never staged - # for commit. Idempotent: re-running ``bonfire init`` does not - # duplicate the entry. - _ensure_gitignore_entry(target, _GITIGNORE_LINE) + # Seed .gitignore for the operator-local paths (which ones, and why + # each, at ``_GITIGNORE_LINES``). Idempotent on re-run. + _ensure_gitignore_entries(target, _GITIGNORE_LINES) _report(target, pre_existed, seeded_before) raise typer.Exit(0) diff --git a/tests/unit/test_init_gitignore_width.py b/tests/unit/test_init_gitignore_width.py new file mode 100644 index 0000000..9cd5677 --- /dev/null +++ b/tests/unit/test_init_gitignore_width.py @@ -0,0 +1,284 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 BonfireAI + +"""Contract: the WIDTH of the ``.gitignore`` that ``bonfire init`` seeds. + +This module owns ONE contract, graded in both directions. + + * No UNDER-coverage (the control rod). Every operator-local path the + seed exists to cover MUST be ignored by the seeded ``.gitignore`` + itself: the per-machine ``.bonfire/tools.local.toml`` written by + ``bonfire scan``, and the knowledge-backend store at + ``.bonfire/vault`` in BOTH on-disk shapes that one path can take. + Without this direction the whole class is satisfied by a seed that + covers nothing at all. + * No OVER-coverage. The committable sub-paths under ``.bonfire/`` MUST + stay stageable: ``sessions/`` handoffs (operators commit these so the + next session picks up the thread), ``context.json`` (portable project + config) and the opt-in ``costs.jsonl`` ledger. A bare ``.bonfire/`` + line is the obvious way to get this wrong, so it is also named + directly, and the seeded ``.bonfire`` entries are pinned as a closed + set — a THIRD distinct path would fail here even if it were narrow. + +Why the store needs two probes. ``bonfire.knowledge.get_vault_backend`` +documents one default ``vault_path`` for every backend, +``.bonfire/vault``. The LanceDB backend turns that path into a DIRECTORY +(a vector index built from the operator's own source). The SQLite backend +hands the same string to ``sqlite3.connect``, which creates a REGULAR +FILE whose ``vault_entries.content`` column holds that source as +cleartext — strictly worse to leak than embeddings. A trailing-slash +(directory-only) pattern covers the first shape and NOT the second, and +``git check-ignore``'s verdict on such a pattern depends on what is on +disk, so this module materialises the file shape rather than trusting a +name. + +Preventive, not the closing of a live leak. ``get_vault_backend`` has no +production caller today: ``engine/composition.py`` documents the ingest +consumer as deliberately not wired, and ``VaultConfig`` carries only +``session_dir`` and ``context_file``, so no ``bonfire.toml`` key selects +a backend. This pin is what makes the store safe to enable — it does not +report that anything is leaking now. + +Split out of ``test_tools_section_is_local.py`` (which keeps Pins #1-#8, +including the init-coverage and idempotence pins) so that one contract +owns one file. +""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +# The exact set of non-comment ``.bonfire`` entries ``bonfire init`` is +# allowed to seed. Written as LITERAL strings on purpose: importing +# ``_GITIGNORE_LINES`` from ``init.py`` would make the closed-world +# assertion below true by construction — the gate would then grade +# nothing at all, no matter what init seeds. +_EXPECTED_SEEDED_BONFIRE_ENTRIES = {".bonfire/tools.local.toml", ".bonfire/vault"} + +# Blanket spellings that would cover every committable sub-path at once. +_BLANKET_SPELLINGS = {".bonfire", ".bonfire/", ".bonfire/*", ".bonfire/**"} + +# Neutralise the contributor's and the CI runner's own git configuration +# for EVERY git subprocess this module runs. ``git check-ignore`` reports +# matches from ``core.excludesFile`` as well as from the repo's +# ``.gitignore``, so a global excludes carrying ``*.json``, ``*.md`` or +# ``.bonfire/`` would otherwise decide this test's verdict on a file +# ``bonfire init`` never wrote. ``tests/conftest.py`` scrubs ``BONFIRE_*`` +# only; nothing else does this. Residual: git's XDG fallback +# (``~/.config/git/ignore``) is not disabled by these two variables, which +# is why every assertion below attributes the match to a SOURCE rather +# than merely asking whether one exists. +_GIT_ENV = { + **os.environ, + "GIT_CONFIG_GLOBAL": os.devnull, + "GIT_CONFIG_SYSTEM": os.devnull, +} + + +def _run_init(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Run ``bonfire init .`` inside *tmp_path* and require a clean exit. + + So no pin can mistake a crashed init for a narrow gitignore. This is a + verbatim twin of the helper in ``test_tools_section_is_local.py``: + sharing it would need either a third helper module or a + ``tests/conftest.py`` fixture, and a new import surface is a worse + trade than six duplicated lines whose meaning is fixed by the + assertion text they carry. + """ + from typer.testing import CliRunner + + from bonfire.cli.app import app + + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke(app, ["init", "."]) + assert result.exit_code == 0, ( + f"bonfire init must succeed in {tmp_path}; got exit_code={result.exit_code}, " + f"output={result.output!r}" + ) + + +def _ignored_by(repo: Path, rel_path: str) -> str: + """Return the ``source:line:pattern`` that ignores *rel_path*, or ``""``. + + ``git check-ignore -v`` rather than ``-q``: the quiet form answers only + "something matched", which cannot tell the seeded ``.gitignore`` from an + unrelated global excludes file, and both directions below need the + source. + + Exit statuses: 0 one or more patterns matched, 1 none did, 128 a fatal + error (not a repo, bad option, unreadable index). Folding 128 into + ``""`` — "not ignored" — would make every must-NOT-be-ignored assertion + pass on a void while looking green, so the status is asserted first and + a fatal git error is a loud red instead of an alibi. + """ + check = subprocess.run( + ["git", "check-ignore", "-v", rel_path], + cwd=repo, + capture_output=True, + text=True, + env=_GIT_ENV, + ) + assert check.returncode in (0, 1), ( + f"git check-ignore could not answer for {rel_path!r} in {repo}: " + f"returncode={check.returncode} (0=ignored, 1=not ignored; anything " + f"else is a fatal git error, never a verdict). " + f"stderr={check.stderr!r} stdout={check.stdout!r}" + ) + return check.stdout.strip() if check.returncode == 0 else "" + + +class TestInitGitignoreCoversOperatorStateAndNothingElse: + """``bonfire init``'s seeded ``.gitignore`` is exactly as wide as it needs. + + Graded by real ``git check-ignore`` against a real repository, so the + contract is git's own pattern semantics rather than this test's idea of + them. + """ + + def test_seeded_gitignore_covers_operator_state_without_over_covering( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """``git check-ignore`` grades both directions against a real repo.""" + _run_init(tmp_path, monkeypatch) + + gitignore_path = tmp_path / ".gitignore" + + # The skip below fires ONLY when git is absent or cannot create a + # repo at all — never as a way to absorb a real gitignore failure, + # and the reason names which of the two it was. + try: + subprocess.run( + ["git", "init", "-q"], + cwd=tmp_path, + check=True, + capture_output=True, + env=_GIT_ENV, + ) + except (FileNotFoundError, subprocess.CalledProcessError) as exc: + pytest.skip(f"git unavailable, cannot grade gitignore semantics: {exc!r}") + + # Materialise the SQLite shape: a REGULAR FILE at .bonfire/vault, + # exactly what ``sqlite3.connect(".bonfire/vault")`` produces. This + # is not decoration — a directory-only pattern's verdict DEPENDS on + # the on-disk shape (git will not apply a trailing-slash pattern to + # something it cannot see is a directory), so probing the name alone + # would grade a different question than the one operators face. + (tmp_path / ".bonfire" / "vault").write_bytes(b"") + + committable_paths = [ + ".bonfire/sessions/handoff.md", + ".bonfire/context.json", + ".bonfire/costs.jsonl", + # ``.bonfire/vault/seed.md`` was asserted committable here until + # it was measured: nothing under ``src/`` ever writes a + # ``seed.md`` and the string appeared in exactly one place in the + # whole repository — this assertion. It guarded a phantom. The + # vault is operator content either way, so the path moved to + # ``ignored_paths`` below. + ] + ignored_paths = [ + # Per-machine tool inventory written by ``bonfire scan``. + ".bonfire/tools.local.toml", + # The store's TWO shapes. The bare path is the SQLite shape + # (materialised above as a regular file); the nested paths are + # the LanceDB directory shape. Together they separate FILE from + # DIRECTORY, which is the distinction a trailing slash gets + # wrong. ``some-index.idx`` is deliberately synthetic — no code + # in this repo writes that name; it is here so the pattern is + # shown to cover ARBITRARY nesting under the directory, not one + # known filename. + ".bonfire/vault", + ".bonfire/vault/vault_v2.lance/data/0.lance", + ".bonfire/vault/some-index.idx", + ] + # Non-vacuity: an empty list makes its loop below pass while grading + # nothing, which is how this whole class goes quiet. + assert committable_paths, "committable_paths is empty: over-coverage check is vacuous" + assert ignored_paths, "ignored_paths is empty: the coverage control rod is vacuous" + + body = gitignore_path.read_text() + + # Direction 2 first: the control rod. Attribution to ``.gitignore:`` + # is what keeps an unrelated pattern in someone's global excludes + # (say ``*.lance``) from satisfying it over a seed covering nothing. + for rel_path in ignored_paths: + match = _ignored_by(tmp_path, rel_path) + assert match.startswith(".gitignore:"), ( + f"bonfire init's .gitignore at {gitignore_path} UNDER-covers: " + f"{rel_path!r} is not ignored by the SEEDED .gitignore, so " + f"`git add` would stage the operator's own content. " + f"check-ignore said {match!r} (empty = no pattern matched; a " + f"non-.gitignore source means some other excludes file, not " + f"the seed, would have to do the job). Gitignore body:\n{body}" + ) + + # Direction 1: no over-coverage. The verdict is about the SEEDED + # file, so only a ``.gitignore:``-sourced match is an accusation + # against ``bonfire init``. A foreign source is collected and + # reported separately, at the very end, so that a contributor's own + # excludes can never (a) be mistaken for bonfire over-covering, nor + # (b) pre-empt bonfire's verdict. + foreign_matches: list[str] = [] + for rel_path in committable_paths: + match = _ignored_by(tmp_path, rel_path) + assert not match.startswith(".gitignore:"), ( + f"bonfire init's .gitignore at {gitignore_path} OVER-covers: " + f"{rel_path!r} is matched by {match!r} but must remain " + f"stageable. Gitignore body:\n{body}" + ) + if match: + foreign_matches.append(f"{rel_path} <- {match}") + + seeded = [ + line.strip() + for line in body.splitlines() + if line.strip() and not line.strip().startswith("#") + ] + + # Blanket cover, named directly. The committable loop already rules + # it out, but asserting it by name makes the FAILURE readable + # instead of leaving the reader to infer which pattern was too wide. + blanket = _BLANKET_SPELLINGS & set(seeded) + assert not blanket, ( + f"bonfire init seeded a blanket .bonfire cover {sorted(blanket)!r} in " + f"{gitignore_path}, which ignores every committable sub-path under it. " + f"Seeded entries: {seeded!r}. Gitignore body:\n{body}" + ) + + # Closed-world width. The probe lists above are finite samples, so + # on their own they cannot see a THIRD seeded path (``.bonfire/cache/``, + # ``.bonfire/tmp*``) that no sample happens to touch. Set equality + # against a literal expectation is the width bound: a new seeded + # entry fails here and has to be argued for, and a dropped entry + # fails here too. + seeded_bonfire = {line for line in seeded if ".bonfire" in line} + assert seeded_bonfire == _EXPECTED_SEEDED_BONFIRE_ENTRIES, ( + f"the set of .bonfire entries bonfire init seeds into " + f"{gitignore_path} changed. Expected exactly " + f"{sorted(_EXPECTED_SEEDED_BONFIRE_ENTRIES)!r}; got " + f"{sorted(seeded_bonfire)!r} (added " + f"{sorted(seeded_bonfire - _EXPECTED_SEEDED_BONFIRE_ENTRIES)!r}, " + f"missing {sorted(_EXPECTED_SEEDED_BONFIRE_ENTRIES - seeded_bonfire)!r}). " + f"Every seeded path must be operator-local AND must not cover a " + f"committable sibling; if this change is correct, update the " + f"expectation here deliberately. Gitignore body:\n{body}" + ) + + # Environment signal, deliberately last and deliberately worded so + # it can never be read as an accusation against ``bonfire init``. + assert not foreign_matches, ( + f"THIS IS NOT A BONFIRE DEFECT: bonfire init's seed graded clean " + f"above. Some OTHER excludes source on this machine ignores a " + f"path bonfire deliberately leaves committable: {foreign_matches!r}. " + f"GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM are already pointed at " + f"{os.devnull} for this test, so the likely source is git's XDG " + f"fallback (~/.config/git/ignore) or .git/info/exclude. Fix the " + f"environment, or neutralise that source here too — do not widen " + f"or narrow the seed to satisfy it." + ) diff --git a/tests/unit/test_no_persona_names_in_public_docs.py b/tests/unit/test_no_persona_names_in_public_docs.py index b7fac80..699bb9c 100644 --- a/tests/unit/test_no_persona_names_in_public_docs.py +++ b/tests/unit/test_no_persona_names_in_public_docs.py @@ -163,12 +163,12 @@ # `standard_build` bullet was corrected against the tree. ( "README.md", - 377, + 378, "predecessor named Passelewe. History is sacred — see", ), ( "README.md", - 378, + 379, "`docs/_lore/passelewe.md` if you want the lineage.", ), # --- CHANGELOG.md — predecessor-persona historical entries ----- @@ -194,22 +194,22 @@ # under [Unreleased]. Anchors only; expected text is unchanged. ( "CHANGELOG.md", - 662, + 677, "predecessor persona (Passelewe, the Chamberlain) was retired; the", ), ( "CHANGELOG.md", - 664, + 679, "`docs/_lore/passelewe.md`. The persona builtins directory", ), ( "CHANGELOG.md", - 665, + 680, "`src/bonfire/persona/builtins/passelewe/` was deleted; a new", ), ( "CHANGELOG.md", - 682, + 697, 'to ban `"passelewe"` in src/ (the predecessor persona is gone, so', ), # --- CLAUDE.md — constellation-pointer breadcrumbs -------------- diff --git a/tests/unit/test_tools_section_is_local.py b/tests/unit/test_tools_section_is_local.py index 8b58189..e8665fc 100644 --- a/tests/unit/test_tools_section_is_local.py +++ b/tests/unit/test_tools_section_is_local.py @@ -23,7 +23,13 @@ ``bonfire init`` time. ``bonfire.toml`` stays project-portable; ``.bonfire/tools.local.toml`` stays per-machine, never committed. -This file pins six contracts: +This file pins the six numbered contracts below, plus two +defense-in-depth pins documented at their own classes (#7, the reader's +symlink refusal; #8, the sentinel label whitelist). A ninth pin — the +WIDTH of the ``.gitignore`` ``bonfire init`` seeds, in both directions — +lives in ``tests/unit/test_init_gitignore_width.py``. + +The six: 1. ``generate_config`` MUST NOT include a ``[bonfire.tools]`` section in the main ``config_toml`` string. @@ -34,9 +40,10 @@ ``[bonfire.tools]`` table. 3. ``bonfire init`` MUST add a ``.gitignore`` entry covering - ``.bonfire/tools.local.toml`` (the simplest sufficient cover is the - directory itself, ``.bonfire/``, but a narrower entry that names - the file directly is equally acceptable). + ``.bonfire/tools.local.toml``. Any pattern that matches the file + under standard git semantics satisfies this pin — but the width + module independently forbids a blanket ``.bonfire/`` cover, so the + seed has to name the operator-local paths rather than the dir. 4. The reader API ``load_tools_config(project_path)`` (new module surface owned by the Warrior) MUST prefer @@ -107,6 +114,25 @@ def _fake_tool_scans() -> list[ScanUpdate]: return [_scan("cli_toolchain", name, ver) for name, ver in _FAKE_TOOLS] +def _run_init(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Run ``bonfire init .`` inside *tmp_path* and require a clean exit. + + Shared by every init-facing pin below so there is one assertion text + for "init did not even succeed" — no pin can mistake a crashed init + for a narrow gitignore. + """ + from typer.testing import CliRunner + + from bonfire.cli.app import app + + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke(app, ["init", "."]) + assert result.exit_code == 0, ( + f"bonfire init must succeed in {tmp_path}; got exit_code={result.exit_code}, " + f"output={result.output!r}" + ) + + def _load_tools_reader(): """Lazy-import the (new) reader so its absence fails per-test cleanly. @@ -305,11 +331,14 @@ def test_local_tools_file_not_created_when_no_tools_scans( class TestInitGitignoresLocalToolsFile: """``bonfire init`` MUST seed a ``.gitignore`` entry that prevents - ``.bonfire/tools.local.toml`` from ever being staged. The simplest - sufficient cover is the directory ``.bonfire/`` itself; a narrower - entry naming the file directly is equally acceptable. The Knight + ``.bonfire/tools.local.toml`` from ever being staged. This class accepts any pattern that matches the operator-local file under - standard git semantics. + standard git semantics, including the blanket ``.bonfire/`` cover — + it grades coverage, not width. Width is graded in + ``tests/unit/test_init_gitignore_width.py``, which asserts set + equality against the two entries the seed is allowed to hold and so + rules the blanket cover out; the two together admit only a seed that + names the operator-local paths. """ def test_init_creates_gitignore_covering_tools_local_file( @@ -320,16 +349,7 @@ def test_init_creates_gitignore_covering_tools_local_file( """``bonfire init`` adds an entry to ``.gitignore`` matching ``.bonfire/tools.local.toml``. """ - from typer.testing import CliRunner - - from bonfire.cli.app import app - - runner = CliRunner() - monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["init", "."]) - assert result.exit_code == 0, ( - f"init must succeed; got exit_code={result.exit_code}, output={result.output!r}" - ) + _run_init(tmp_path, monkeypatch) gitignore_path = tmp_path / ".gitignore" assert gitignore_path.exists(), ( @@ -372,36 +392,51 @@ def test_init_gitignore_idempotent_does_not_duplicate( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """Running ``bonfire init`` twice (re-init) MUST NOT duplicate the - ``.bonfire/`` entry in ``.gitignore``. ``bonfire init`` is + """Running ``bonfire init`` twice (re-init) MUST NOT duplicate any + seeded ``.bonfire/`` entry in ``.gitignore``. ``bonfire init`` is idempotent (per ``init.py``'s ``if not toml_path.exists()`` and ``mkdir(exist_ok=True)``); the gitignore seeding must follow the - same rule — re-running must not append a second copy of the - same line. + same rule — re-running must not append a second copy of a line, + nor accumulate blank lines. + + The check is byte-level: the whole file must come back identical, + so a duplicate of ANY seeded entry fails and so does stray + whitespace. That is strictly stronger than the entry-count cap it + replaces ON IDEMPOTENCE and strictly weaker ON WIDTH — the old cap + ("at most one line mentioning ``.bonfire``") incidentally bounded + how many paths could be seeded, while forbidding a SECOND + legitimate operator-local entry, so it conflated the two + properties. Width is bounded on its own terms in + ``tests/unit/test_init_gitignore_width.py``: set equality against + a literal expectation, which a third seeded path fails. """ - from typer.testing import CliRunner - - from bonfire.cli.app import app - - runner = CliRunner() - monkeypatch.chdir(tmp_path) - - result1 = runner.invoke(app, ["init", "."]) - assert result1.exit_code == 0 - result2 = runner.invoke(app, ["init", "."]) - assert result2.exit_code == 0 - gitignore_path = tmp_path / ".gitignore" - assert gitignore_path.exists() - body = gitignore_path.read_text() - lines = [line.strip() for line in body.splitlines()] - # Count how many gitignore lines mention the .bonfire token. - bonfire_lines = [line for line in lines if ".bonfire" in line and not line.startswith("#")] - assert len(bonfire_lines) <= 1, ( - f"bonfire init duplicated .bonfire-related .gitignore entries on " - f"re-init. Got {len(bonfire_lines)} matching lines: " - f"{bonfire_lines!r}. Full body:\n{body}" + _run_init(tmp_path, monkeypatch) + after_first = gitignore_path.read_text() + _run_init(tmp_path, monkeypatch) + after_second = gitignore_path.read_text() + + assert after_second == after_first, ( + f"re-running bonfire init rewrote {gitignore_path}. After first run:\n" + f"{after_first}\n--- After second run:\n{after_second}" + ) + bonfire_lines = [ + line.strip() + for line in after_second.splitlines() + if ".bonfire" in line and not line.strip().startswith("#") + ] + # Non-vacuity: with no seeded entries at all, the duplicate check + # below would pass over an empty list. + assert bonfire_lines, ( + f"bonfire init seeded no .bonfire entry at all into {gitignore_path}, " + f"so the no-duplicate check has nothing to grade. Full body:\n{after_second}" + ) + duplicated = sorted({ln for ln in bonfire_lines if bonfire_lines.count(ln) > 1}) + assert not duplicated, ( + f"bonfire init duplicated a .bonfire .gitignore entry in " + f"{gitignore_path} on re-init: {duplicated!r}. All .bonfire lines: " + f"{bonfire_lines!r}. Full body:\n{after_second}" ) @@ -817,77 +852,17 @@ def test_sentinel_drops_uppercase_and_oversize_labels(self) -> None: # --------------------------------------------------------------------------- -# Pin #9 — Gitignore narrowness: committable sub-paths under .bonfire/ -# must remain stageable by default. +# Pin #9 — MOVED. The width of the ``.gitignore`` ``bonfire init`` seeds — +# both directions, plus the closed set of seeded ``.bonfire`` +# entries — now lives in its own module: +# +# tests/unit/test_init_gitignore_width.py +# +# It did not vanish and it did not weaken: the move added the +# SQLite file shape of ``.bonfire/vault`` to the covered paths, +# made a fatal ``git check-ignore`` exit a red instead of a +# silent "not ignored", attributed both directions to a source so +# a contributor's global excludes cannot decide the verdict, and +# replaced the sampled width check with set equality against a +# literal expectation. One contract, one file. # --------------------------------------------------------------------------- - - -class TestInitGitignoreDoesNotOverCover: - """``bonfire init`` must NOT seed a gitignore entry that excludes - committable sub-paths under ``.bonfire/`` (sessions, context.json, - vault seed, opt-in cost ledger). Over-broad coverage silently - breaks workflows that depend on those paths landing in git. - """ - - def test_gitignore_entry_does_not_cover_bonfire_sessions( - self, - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, - ) -> None: - """After init, the gitignore must NOT match - ``.bonfire/sessions/2026-05-15-handoff.md`` — operators commit - session handoffs. - """ - import subprocess - - from typer.testing import CliRunner - - from bonfire.cli.app import app - - runner = CliRunner() - monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["init", "."]) - assert result.exit_code == 0 - - # Use git itself to evaluate the gitignore (most authoritative). - # ``git check-ignore`` returns 0 when path IS ignored, 1 when - # NOT ignored. We want NOT ignored for these committable paths. - try: - subprocess.run( - ["git", "init", "-q"], - cwd=tmp_path, - check=True, - capture_output=True, - ) - except (FileNotFoundError, subprocess.CalledProcessError): - pytest.skip("git not available") - - committable_paths = [ - ".bonfire/sessions/handoff.md", - ".bonfire/context.json", - ".bonfire/vault/seed.md", - ".bonfire/costs.jsonl", - ] - for rel_path in committable_paths: - check = subprocess.run( - ["git", "check-ignore", "-q", rel_path], - cwd=tmp_path, - capture_output=True, - ) - # Exit code 1 = NOT ignored (good). - assert check.returncode == 1, ( - f"bonfire init's .gitignore over-covers: {rel_path!r} is " - f"matched by the seeded entry but should remain stageable. " - f"Gitignore body:\n{(tmp_path / '.gitignore').read_text()}" - ) - - # Sanity: the operator-local file IS ignored. - check = subprocess.run( - ["git", "check-ignore", "-q", ".bonfire/tools.local.toml"], - cwd=tmp_path, - capture_output=True, - ) - assert check.returncode == 0, ( - f"bonfire init's .gitignore did NOT cover the operator-local " - f"tools.local.toml file. Body:\n{(tmp_path / '.gitignore').read_text()}" - )