diff --git a/src/forge_loop/manifestos.py b/src/forge_loop/manifestos.py new file mode 100644 index 0000000..1caaa73 --- /dev/null +++ b/src/forge_loop/manifestos.py @@ -0,0 +1,176 @@ +"""Quality + testing manifesto discovery and worker-prompt injection. + +This module is the consumer-side of the manifesto contract introduced in +issues #130 (sibling discovery seed) and #132 (this ticket: inject into +worker system prompt). + +A "manifesto" is a free-form markdown file under ``/.forge/`` that +the operator uses to nail down house rules every worker MUST follow: + + .forge/quality-manifesto.md → "QUALITY RULES YOU MUST FOLLOW" + .forge/testing-manifesto.md → "TESTING RULES YOU MUST FOLLOW" + +Both files are OPTIONAL. A repo with neither file behaves byte-identically +to the pre-feature baseline (back-compat is an acceptance criterion). + +Public surface: + +- :func:`load_manifestos` — discovery; returns a :class:`ManifestoBundle`. +- :func:`render_manifesto_block` — turns a bundle into the prompt prefix. +- :func:`inject_into_brief` — convenience: prepend block to an existing + brief string, in front of all task-specific content. + +The git-blob-style sha for each rendered manifesto is recorded so the +worker outcome telemetry can audit which manifesto version a given PR was +built against (acceptance criterion: ``manifesto_sha`` dict in outcome). +""" + +from __future__ import annotations + +import hashlib +from dataclasses import dataclass +from pathlib import Path + +QUALITY_REL = ".forge/quality-manifesto.md" +TESTING_REL = ".forge/testing-manifesto.md" + +QUALITY_HEADER = "QUALITY RULES YOU MUST FOLLOW:" +TESTING_HEADER = "TESTING RULES YOU MUST FOLLOW:" +BLOCK_OPEN = "===== MANIFESTO =====" +BLOCK_CLOSE = "===== END MANIFESTO =====" + + +@dataclass(frozen=True) +class ManifestoSide: + """A single manifesto (quality OR testing) after discovery. + + ``content`` is the verbatim file body, or ``None`` if the file was + absent, whitespace-only, or unreadable. ``sha`` is the git-blob-style + sha1 of the file body when content is present; ``None`` otherwise. + Keeping the two fields locked together (both set, or both None) is + enforced in :func:`_load_one`. + """ + + content: str | None + sha: str | None + + @property + def present(self) -> bool: + return self.content is not None + + +@dataclass(frozen=True) +class ManifestoBundle: + """Quality + testing manifestos, post-discovery.""" + + quality: ManifestoSide + testing: ManifestoSide + + @property + def any_present(self) -> bool: + return self.quality.present or self.testing.present + + def sha_payload(self) -> dict[str, str | None]: + """Return the ``manifesto_sha`` field for outcome telemetry. + + Always a 2-key dict so downstream consumers can rely on the shape; + missing sides are ``None`` (not absent keys). + """ + return {"quality": self.quality.sha, "testing": self.testing.sha} + + +def _git_blob_sha(text: str) -> str: + """Compute the git-blob sha1 of ``text``. + + Matches what ``git hash-object `` returns, so an operator can + cross-reference the recorded telemetry against the live file: + + $ git hash-object .forge/quality-manifesto.md + """ + blob = text.encode("utf-8") + h = hashlib.sha1() + h.update(f"blob {len(blob)}\0".encode("ascii")) + h.update(blob) + return h.hexdigest() + + +def _load_one(path: Path) -> ManifestoSide: + """Discover one manifesto file. Never raises. + + - File missing → both fields None. + - File unreadable (permission, binary garbage that isn't decodable as + utf-8) → both None. We deliberately swallow because the acceptance + criterion says a corrupt manifesto MUST NOT crash the worker. + - Whitespace-only body → treated as missing (no empty header rendered). + - Otherwise → content + blob sha. + """ + try: + if not path.is_file(): + return ManifestoSide(None, None) + except OSError: + return ManifestoSide(None, None) + try: + text = path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + return ManifestoSide(None, None) + if not text.strip(): + return ManifestoSide(None, None) + return ManifestoSide(text, _git_blob_sha(text)) + + +def load_manifestos(repo: Path | str) -> ManifestoBundle: + """Discover the quality + testing manifestos under ``/.forge/``. + + Pure I/O; no formatting, no injection. Always returns a bundle — + consumers check ``bundle.any_present`` to decide whether to render the + MANIFESTO block at all. + """ + root = Path(repo) + return ManifestoBundle( + quality=_load_one(root / QUALITY_REL), + testing=_load_one(root / TESTING_REL), + ) + + +def render_manifesto_block(bundle: ManifestoBundle) -> str: + """Format the MANIFESTO prefix for a worker brief. + + Returns the empty string when BOTH sides are absent (back-compat: a + repo with no manifestos sees a byte-identical prompt). When only one + side is present, that subsection is rendered and the other is + silently omitted. + + Order is fixed: QUALITY first, then TESTING — operators reading the + brief should see quality framing before test framing because tests + enforce quality, not the other way around. + """ + if not bundle.any_present: + return "" + parts: list[str] = [BLOCK_OPEN] + if bundle.quality.present: + parts.append(QUALITY_HEADER) + # body kept verbatim — no trimming, no wrapping; the operator + # authored it deliberately. + parts.append((bundle.quality.content or "").rstrip("\n")) + if bundle.testing.present: + if bundle.quality.present: + parts.append("") # blank line between subsections + parts.append(TESTING_HEADER) + parts.append((bundle.testing.content or "").rstrip("\n")) + parts.append(BLOCK_CLOSE) + parts.append("") # trailing newline before downstream brief content + return "\n".join(parts) + "\n" + + +def inject_into_brief(brief: str, bundle: ManifestoBundle) -> str: + """Prepend the MANIFESTO block to a rendered worker brief. + + Injection happens BEFORE any task-specific content so the rules frame + everything the worker reads after (acceptance criterion). When the + bundle is empty, returns ``brief`` unchanged (no leading newline, no + sentinel) so back-compat tests pass byte-for-byte. + """ + block = render_manifesto_block(bundle) + if not block: + return brief + return block + brief diff --git a/src/forge_loop/worker.py b/src/forge_loop/worker.py index 92ed477..7ba08e5 100644 --- a/src/forge_loop/worker.py +++ b/src/forge_loop/worker.py @@ -71,6 +71,11 @@ class WorkerOutcome: cost_usd: float = 0.0 usage: dict[str, Any] | None = None model: str = "" + # Issue #132: which manifesto versions were prepended to the worker + # system prompt for this run. Always a 2-key dict ({"quality": ..., + # "testing": ...}) when at least one side was present; ``None`` when + # the repo had no manifestos at all (back-compat baseline). + manifesto_sha: dict[str, str | None] | None = None def make_brief( @@ -83,6 +88,7 @@ def make_brief( lumen_test_pattern: str = "**/*Test.*", coauthor: str = "", dry_run: bool = False, + manifesto_bundle: Any | None = None, ) -> str: """Render the worker brief for an issue. @@ -150,6 +156,16 @@ def make_brief( from forge_loop.replay import apply_dry_run_to_brief rendered = apply_dry_run_to_brief(rendered) + # Issue #132 — manifesto injection. The MANIFESTO block goes in FRONT + # of every other brief line so the worker reads the house rules before + # it sees the issue body, the contract, or the exit checklist. When + # the bundle is empty (no manifestos in this repo), inject_into_brief + # is a no-op and the rendered brief is byte-identical to the + # pre-feature baseline (back-compat acceptance criterion). + if manifesto_bundle is not None: + from forge_loop.manifestos import inject_into_brief + + rendered = inject_into_brief(rendered, manifesto_bundle) return rendered @@ -496,11 +512,26 @@ def run_worker( logs_dir.mkdir(parents=True, exist_ok=True) log_path = logs_dir / f"worker-{n}-{int(time.time())}.log" + # Issue #132 — discover the active manifestos once at dispatch time. + # The bundle threads into BOTH the brief renderer (prepends MANIFESTO + # block) AND the outcome telemetry (``manifesto_sha`` audit field). + # Discovery sources from the repo checkout, NOT the worktree — the + # worktree's .forge/ exists post-branch but the manifestos live on + # the canonical checkout that controls house rules. + from forge_loop.manifestos import load_manifestos + + manifesto_bundle = load_manifestos(repo) + manifesto_sha = manifesto_bundle.sha_payload() if manifesto_bundle.any_present else None + # Iteration loop (issue #78) passes a focused follow-up brief that # short-circuits ``make_brief`` — the follow-up session reuses the same # worktree + branch and just gets told "your ONLY job is X". if brief_override is not None: - brief = brief_override + # Even on follow-up runs, prepend the manifesto block so iteration 2 + # is held to the same house rules as iteration 1. + from forge_loop.manifestos import inject_into_brief + + brief = inject_into_brief(brief_override, manifesto_bundle) else: brief = make_brief( issue, @@ -510,10 +541,11 @@ def run_worker( lumen_top_k=lumen_top_k, lumen_test_pattern=lumen_test_pattern, coauthor=coauthor, + manifesto_bundle=manifesto_bundle, ) if provider == "codex": - return _run_worker_codex( + outcome = _run_worker_codex( issue=issue, worktree=worktree, log_path=log_path, @@ -521,8 +553,10 @@ def run_worker( timeout_s=timeout_s, model=model, ) + outcome.manifesto_sha = manifesto_sha + return outcome - return _run_worker_sdk( + outcome = _run_worker_sdk( issue=issue, worktree=worktree, log_path=log_path, @@ -537,6 +571,8 @@ def run_worker( strict_mcp_config=strict_mcp_config, mcp_servers=mcp_servers, ) + outcome.manifesto_sha = manifesto_sha + return outcome def run_repair_worker( diff --git a/tests/test_worker_manifesto_injection.py b/tests/test_worker_manifesto_injection.py new file mode 100644 index 0000000..f50ea9d --- /dev/null +++ b/tests/test_worker_manifesto_injection.py @@ -0,0 +1,378 @@ +"""Tests for worker manifesto injection (issue #132). + +Covers the acceptance criteria's test matrix: + +- Both manifestos present → both headers + bodies in order. +- Both missing → byte-identical pre-feature baseline (no MANIFESTO block). +- Only one side present → that subsection only, no empty header. +- Whitespace-only file → treated as missing. +- ``manifesto_sha`` recorded on the WorkerOutcome / telemetry. +- Block precedes any task-specific brief markers. +- Integration: SDK system prompt + outcome both round-trip the bundle. +- Adversarial: unreadable/binary manifesto does NOT crash the worker. +""" + +from __future__ import annotations + +import hashlib +import os +import subprocess +from pathlib import Path +from typing import Any +from unittest.mock import patch + +import pytest + +from forge_loop.manifestos import ( + BLOCK_CLOSE, + BLOCK_OPEN, + QUALITY_HEADER, + QUALITY_REL, + TESTING_HEADER, + TESTING_REL, + ManifestoBundle, + ManifestoSide, + inject_into_brief, + load_manifestos, + render_manifesto_block, +) + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +def _write_manifesto(repo: Path, rel: str, content: str) -> None: + path = repo / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content, encoding="utf-8") + + +def _git_blob_sha(text: str) -> str: + blob = text.encode("utf-8") + h = hashlib.sha1() + h.update(f"blob {len(blob)}\0".encode("ascii")) + h.update(blob) + return h.hexdigest() + + +@pytest.fixture +def issue() -> dict[str, Any]: + return {"number": 999, "title": "test ticket", "body": "do the thing"} + + +# --------------------------------------------------------------------------- +# Discovery +# --------------------------------------------------------------------------- + + +def test_load_manifestos_empty_repo_returns_empty_bundle(tmp_path: Path) -> None: + bundle = load_manifestos(tmp_path) + assert not bundle.any_present + assert bundle.quality == ManifestoSide(None, None) + assert bundle.testing == ManifestoSide(None, None) + assert bundle.sha_payload() == {"quality": None, "testing": None} + + +def test_load_manifestos_both_present(tmp_path: Path) -> None: + _write_manifesto(tmp_path, QUALITY_REL, "be excellent") + _write_manifesto(tmp_path, TESTING_REL, "test everything") + bundle = load_manifestos(tmp_path) + assert bundle.quality.content == "be excellent" + assert bundle.testing.content == "test everything" + assert bundle.quality.sha == _git_blob_sha("be excellent") + assert bundle.testing.sha == _git_blob_sha("test everything") + + +def test_whitespace_only_manifesto_treated_as_missing(tmp_path: Path) -> None: + _write_manifesto(tmp_path, QUALITY_REL, "\n\n \n") + bundle = load_manifestos(tmp_path) + assert bundle.quality.content is None + assert bundle.quality.sha is None + + +def test_corrupt_manifesto_does_not_crash_loader(tmp_path: Path) -> None: + # Binary garbage that's invalid utf-8. + bad_path = tmp_path / QUALITY_REL + bad_path.parent.mkdir(parents=True, exist_ok=True) + bad_path.write_bytes(b"\xff\xfe\x00\x01garbage\xc3\x28") + bundle = load_manifestos(tmp_path) + assert bundle.quality.content is None + assert bundle.quality.sha is None + + +# --------------------------------------------------------------------------- +# Rendering +# --------------------------------------------------------------------------- + + +def test_renders_both_manifestos_when_present(tmp_path: Path) -> None: + _write_manifesto(tmp_path, QUALITY_REL, "Q1\nQ2") + _write_manifesto(tmp_path, TESTING_REL, "T1\nT2") + bundle = load_manifestos(tmp_path) + block = render_manifesto_block(bundle) + # Both headers present. + assert QUALITY_HEADER in block + assert TESTING_HEADER in block + # Quality precedes testing. + assert block.index(QUALITY_HEADER) < block.index(TESTING_HEADER) + # Bodies verbatim. + assert "Q1\nQ2" in block + assert "T1\nT2" in block + # Sentinel markers wrap. + assert block.startswith(BLOCK_OPEN) + assert BLOCK_CLOSE in block + + +def test_silent_skip_when_both_missing(tmp_path: Path, issue: dict[str, Any]) -> None: + """Pre-feature baseline: empty bundle → brief is byte-identical.""" + from forge_loop.worker import make_brief + + bundle_empty = load_manifestos(tmp_path) + baseline = make_brief(issue, tmp_path) + with_bundle = make_brief(issue, tmp_path, manifesto_bundle=bundle_empty) + assert baseline == with_bundle + assert BLOCK_OPEN not in with_bundle + + +def test_partial_render_only_quality(tmp_path: Path) -> None: + _write_manifesto(tmp_path, QUALITY_REL, "only quality here") + bundle = load_manifestos(tmp_path) + block = render_manifesto_block(bundle) + assert QUALITY_HEADER in block + assert TESTING_HEADER not in block + assert "only quality here" in block + + +def test_partial_render_only_testing(tmp_path: Path) -> None: + _write_manifesto(tmp_path, TESTING_REL, "only testing here") + bundle = load_manifestos(tmp_path) + block = render_manifesto_block(bundle) + assert TESTING_HEADER in block + assert QUALITY_HEADER not in block + + +def test_whitespace_only_manifesto_not_rendered_as_empty_header(tmp_path: Path) -> None: + _write_manifesto(tmp_path, QUALITY_REL, "\n \n") + _write_manifesto(tmp_path, TESTING_REL, "real testing rules") + bundle = load_manifestos(tmp_path) + block = render_manifesto_block(bundle) + assert QUALITY_HEADER not in block + assert TESTING_HEADER in block + + +# --------------------------------------------------------------------------- +# Injection into worker brief +# --------------------------------------------------------------------------- + + +def test_manifesto_block_precedes_task_brief(tmp_path: Path, issue: dict[str, Any]) -> None: + from forge_loop.worker import make_brief + + _write_manifesto(tmp_path, QUALITY_REL, "QQ rules") + _write_manifesto(tmp_path, TESTING_REL, "TT rules") + bundle = load_manifestos(tmp_path) + brief = make_brief(issue, tmp_path, manifesto_bundle=bundle) + # The MANIFESTO block must appear at offset 0 (or near zero) — before + # the "You are an autonomous worker" task header. + task_marker = "autonomous worker" + assert task_marker in brief + assert brief.index(BLOCK_OPEN) < brief.index(task_marker) + assert brief.index(QUALITY_HEADER) < brief.index(task_marker) + assert brief.index(TESTING_HEADER) < brief.index(task_marker) + + +def test_inject_no_op_when_bundle_empty() -> None: + empty = ManifestoBundle(ManifestoSide(None, None), ManifestoSide(None, None)) + assert inject_into_brief("hello world", empty) == "hello world" + + +# --------------------------------------------------------------------------- +# Telemetry (manifesto_sha on WorkerOutcome) +# --------------------------------------------------------------------------- + + +def test_manifesto_sha_in_telemetry_both_present( + tmp_path: Path, issue: dict[str, Any] +) -> None: + """End-to-end: run_worker records manifesto_sha on the outcome. + + Uses a fake _prep_worktree + _run_worker_sdk so we exercise the wiring + without launching the real SDK or git. + """ + from forge_loop import worker as worker_mod + from forge_loop.worker import WorkerOutcome + + repo = tmp_path / "repo" + repo.mkdir() + _write_manifesto(repo, QUALITY_REL, "QUAL") + _write_manifesto(repo, TESTING_REL, "TEST") + logs = tmp_path / "logs" + + fake_outcome = WorkerOutcome( + issue=999, title="t", pr_url=None, status="open", + duration_s=0.1, stdout_tail="", error=None, + ) + captured: dict[str, Any] = {} + + def _fake_prep(*a: Any, **k: Any) -> tuple[Path, str | None]: + wt = tmp_path / "wt" + wt.mkdir(exist_ok=True) + return wt, None + + def _fake_sdk(**kwargs: Any) -> WorkerOutcome: + captured["brief"] = kwargs["brief"] + return fake_outcome + + with patch.object(worker_mod, "_prep_worktree", _fake_prep), patch.object( + worker_mod, "_run_worker_sdk", _fake_sdk + ): + outcome = worker_mod.run_worker(issue, repo, logs, timeout_s=10) + + assert outcome.manifesto_sha == { + "quality": _git_blob_sha("QUAL"), + "testing": _git_blob_sha("TEST"), + } + # Brief that was sent to the SDK must contain the rendered block. + assert BLOCK_OPEN in captured["brief"] + assert QUALITY_HEADER in captured["brief"] + assert TESTING_HEADER in captured["brief"] + + +def test_manifesto_sha_null_when_repo_has_no_manifestos( + tmp_path: Path, issue: dict[str, Any] +) -> None: + from forge_loop import worker as worker_mod + from forge_loop.worker import WorkerOutcome + + repo = tmp_path / "repo" + repo.mkdir() + logs = tmp_path / "logs" + fake_outcome = WorkerOutcome( + issue=999, title="t", pr_url=None, status="open", + duration_s=0.1, stdout_tail="", error=None, + ) + captured: dict[str, Any] = {} + + def _fake_prep(*a: Any, **k: Any) -> tuple[Path, str | None]: + wt = tmp_path / "wt" + wt.mkdir(exist_ok=True) + return wt, None + + def _fake_sdk(**kwargs: Any) -> WorkerOutcome: + captured["brief"] = kwargs["brief"] + return fake_outcome + + with patch.object(worker_mod, "_prep_worktree", _fake_prep), patch.object( + worker_mod, "_run_worker_sdk", _fake_sdk + ): + outcome = worker_mod.run_worker(issue, repo, logs, timeout_s=10) + + assert outcome.manifesto_sha is None + assert BLOCK_OPEN not in captured["brief"] + + +def test_manifesto_sha_partial_when_only_one_side_present( + tmp_path: Path, issue: dict[str, Any] +) -> None: + from forge_loop import worker as worker_mod + from forge_loop.worker import WorkerOutcome + + repo = tmp_path / "repo" + repo.mkdir() + _write_manifesto(repo, QUALITY_REL, "only quality") + logs = tmp_path / "logs" + fake_outcome = WorkerOutcome( + issue=999, title="t", pr_url=None, status="open", + duration_s=0.1, stdout_tail="", error=None, + ) + + def _fake_prep(*a: Any, **k: Any) -> tuple[Path, str | None]: + wt = tmp_path / "wt" + wt.mkdir(exist_ok=True) + return wt, None + + def _fake_sdk(**kwargs: Any) -> WorkerOutcome: + return fake_outcome + + with patch.object(worker_mod, "_prep_worktree", _fake_prep), patch.object( + worker_mod, "_run_worker_sdk", _fake_sdk + ): + outcome = worker_mod.run_worker(issue, repo, logs, timeout_s=10) + + assert outcome.manifesto_sha == { + "quality": _git_blob_sha("only quality"), + "testing": None, + } + + +def test_corrupt_manifesto_does_not_crash_worker( + tmp_path: Path, issue: dict[str, Any] +) -> None: + """Adversarial: binary garbage in manifesto file → worker still runs.""" + from forge_loop import worker as worker_mod + from forge_loop.worker import WorkerOutcome + + repo = tmp_path / "repo" + repo.mkdir() + bad = repo / QUALITY_REL + bad.parent.mkdir(parents=True, exist_ok=True) + bad.write_bytes(b"\xff\xfe\x00garbage\xc3\x28") + # Testing side is fine. + _write_manifesto(repo, TESTING_REL, "valid testing") + logs = tmp_path / "logs" + fake_outcome = WorkerOutcome( + issue=999, title="t", pr_url=None, status="open", + duration_s=0.1, stdout_tail="", error=None, + ) + + def _fake_prep(*a: Any, **k: Any) -> tuple[Path, str | None]: + wt = tmp_path / "wt" + wt.mkdir(exist_ok=True) + return wt, None + + def _fake_sdk(**kwargs: Any) -> WorkerOutcome: + return fake_outcome + + with patch.object(worker_mod, "_prep_worktree", _fake_prep), patch.object( + worker_mod, "_run_worker_sdk", _fake_sdk + ): + outcome = worker_mod.run_worker(issue, repo, logs, timeout_s=10) + + # Corrupt side → null sha; clean side → real sha. + assert outcome.manifesto_sha == { + "quality": None, + "testing": _git_blob_sha("valid testing"), + } + + +# --------------------------------------------------------------------------- +# Git blob sha parity +# --------------------------------------------------------------------------- + + +def test_sha_matches_git_hash_object(tmp_path: Path) -> None: + """Our blob-sha implementation must match ``git hash-object`` byte-for-byte. + + Operators audit ``manifesto_sha`` by running ``git hash-object`` on the + live file; a divergence would break that workflow. + """ + if not _which("git"): # pragma: no cover + pytest.skip("git not available") + content = "rule 1\nrule 2\n" + path = tmp_path / "f.md" + path.write_text(content, encoding="utf-8") + expected = subprocess.check_output( + ["git", "hash-object", str(path)], text=True + ).strip() + from forge_loop.manifestos import _git_blob_sha as impl + + assert impl(content) == expected + + +def _which(cmd: str) -> str | None: + for d in os.environ.get("PATH", "").split(os.pathsep): + p = Path(d) / cmd + if p.is_file() and os.access(p, os.X_OK): + return str(p) + return None