fix: distinguish install/dual-solve/crystallize failures from success

CHANGELOG.md

@@ -19,6 +19,32 @@ All notable changes to vouch are documented here. Format follows
   `vouch triage [proposal-id...]` mirrors it on the CLI with `--json` and
   `--reverse` (#322).
 
+### Fixed
+- `vouch install-mcp <host>` (codex `toml_merge`): a `config.toml` the minimal
+  serializer couldn't faithfully re-emit (a non-BMP string value, a `nan`/`inf`
+  float) was bucketed as `skipped` and printed as `(already present)` with a
+  clean `Done`, so the user believed vouch was wired into codex when it wasn't.
+  serializer-failure now lands in a distinct `failed` bucket, is reported as
+  such, and the command exits non-zero — "already installed" and "install
+  failed" no longer look the same.
+- `vouch install-mcp <host>`: a manifest `dst` that escaped the target tree
+  (via `..` or an absolute path) is now refused with an `AdapterError` instead
+  of writing outside `target` (defense in depth for the manifest file writer;
+  shipped adapters are unaffected).
+- dual-solve review-ui: the recommendation hint rendered "neither engine
+  produced a usable diff" for the entire duration of a still-running job — the
+  hint was computed over the not-yet-populated candidate list on every poll.
+  it is now omitted until candidates exist.
+- `session.crystallize`: retrying on a session that hasn't been ended rewrote
+  the `session-<id>` summary page with a fresh wall-clock `Ended:` stamp each
+  time (and needlessly re-embedded it), so the "idempotent retry" wasn't. an
+  open session now renders a stable marker, so retries produce an identical
+  body.
+- `vouch capture ingest-codex`: rollout parsing had no size cap and could read
+  an oversized (or newline-free blob) rollout whole into memory. the file is
+  now bounded to 64 MiB up front, mirroring the byte caps on other untrusted
+  reads.
+
 ## [1.2.1] — 2026-07-06
 
 ### Fixed

src/vouch/cli.py

@@ -3505,12 +3505,21 @@ def install_mcp(
         click.echo(f"  ~ {f}  (merged into existing)")
     for f in result.skipped:
         click.echo(f"  · {f}  (already present)")
+    for f in result.failed:
+        click.echo(f"  ✗ {f}  (could not install — left unchanged)")
     click.echo(
         f"Done — {len(result.written)} written, "
         f"{len(result.appended)} appended, {len(result.merged)} merged, "
-        f"{len(result.skipped)} skipped "
+        f"{len(result.skipped)} skipped, {len(result.failed)} failed "
         f"under {target}"
     )
+    if result.failed:
+        # a failed install is not a no-op: exit non-zero so scripts (and the
+        # user) notice vouch was NOT wired into these files.
+        raise click.ClickException(
+            f"{len(result.failed)} file(s) could not be installed: "
+            f"{', '.join(result.failed)}"
+        )
 
 
 # --- sync: bidirectional vouch <-> Obsidian-style vault -------------------

src/vouch/codex_rollout.py

@@ -42,6 +42,11 @@
 
 _ZSTD_MAGIC = b"\x28\xb5\x2f\xfd"
 _MAX_PROMPT_CHARS = 240
+# Upper bound on a rollout file we'll parse. Rollouts carry large tool
+# outputs, so this is generous, but an unbounded read (a huge file, or a
+# newline-free blob that the streaming reader would slurp whole) must not be
+# able to exhaust memory. Mirrors the byte caps elsewhere (fetch, http_server).
+_MAX_ROLLOUT_BYTES = 64 * 1024 * 1024
 _PATCH_FILE_RE = re.compile(r"^\*\*\* (Add|Update|Delete) File: (.+)$", re.MULTILINE)
 _EXIT_CODE_RE = re.compile(r"exited with code (\d+)")
 
@@ -151,14 +156,23 @@ def _iter_rollout_lines(path: Path) -> Iterator[str]:
     """Yield the rollout's lines one at a time, decoded as UTF-8.
 
     Streams from the file handle rather than slurping the whole file into
-    memory — codex rollouts can carry large tool outputs. The zstd magic
-    header is checked up front (compressed rollouts aren't parsed here).
+    memory — codex rollouts can carry large tool outputs. The size is checked
+    up front so an oversized rollout (or a newline-free blob that iterating the
+    handle would otherwise read whole) is refused instead of exhausting memory.
+    The zstd magic header is checked up front too (compressed rollouts aren't
+    parsed here).
     """
     try:
         fh = path.open("rb")
     except OSError as e:
         raise CodexRolloutError(f"cannot read rollout file {path}: {e}") from e
     with fh:
+        size = os.fstat(fh.fileno()).st_size
+        if size > _MAX_ROLLOUT_BYTES:
+            raise CodexRolloutError(
+                f"{path.name} is too large to parse "
+                f"({size} bytes > {_MAX_ROLLOUT_BYTES} byte limit)"
+            )
         if fh.read(4) == _ZSTD_MAGIC:
             raise CodexRolloutError(
                 f"{path.name} is zstd-compressed; decompress it first "

src/vouch/install_adapter.py

@@ -73,6 +73,11 @@ class InstallResult:
     appended: list[str] = field(default_factory=list)
     skipped: list[str] = field(default_factory=list)
     merged: list[str] = field(default_factory=list)
+    # files vouch wanted to install but could not (e.g. a config it can't
+    # faithfully re-serialize, or a malformed existing file). Kept distinct
+    # from ``skipped`` so "already installed" and "install failed" never look
+    # the same to the caller / CLI.
+    failed: list[str] = field(default_factory=list)
 
 
 @dataclass(frozen=True)
@@ -235,6 +240,16 @@ def install(adapter: str, *, target: Path, tier: str = "T4") -> InstallResult:
         for entry in entries:
             src = src_root / entry.src
             dst = target / entry.dst
+            # Defense in depth: `dst` comes from the manifest. Adapters ship
+            # with vouch (trusted), but a `..`/absolute `dst` must never let a
+            # manifest write outside the target tree. `target` is already
+            # resolved above; resolve `dst` (normalizes `..`, even when the
+            # file doesn't exist yet) and require containment.
+            if not dst.resolve().is_relative_to(target):
+                raise AdapterError(
+                    f"{adapter}: install.yaml dst {entry.dst!r} escapes the "
+                    f"target tree ({target})"
+                )
             if not src.is_file():
                 # Manifest declares a template that doesn't exist in the
                 # adapter directory: a contributor-time bug, but surface it
@@ -588,12 +603,17 @@ def _install_toml_merge(
     * dst exists, merge adds keys -> merge + rewrite (``merged``)
     * dst exists, nothing to add  -> skip (``skipped``); already installed
     * dst exists, unparseable     -> skip (``skipped``); never clobber the user
+    * dst exists, can't re-emit   -> ``failed``; the minimal serializer can't
+                                     round-trip the merged data, so vouch is
+                                     left unwired — reported, not silently
+                                     bucketed as "already present"
 
     Rewriting re-serializes the whole file (comments and formatting are not
     preserved — same trade-off ``_install_json_merge`` already makes). The
     serialized result must survive a tomllib round-trip back to the merged
     data; anything the minimal serializer can't faithfully re-emit degrades
-    to ``skipped`` rather than risking the user's config.
+    to ``failed`` (the user's config is left untouched) rather than being
+    reported as a successful no-op.
     """
     if not dst.exists():
         dst.parent.mkdir(parents=True, exist_ok=True)
@@ -605,7 +625,8 @@ def _install_toml_merge(
         dst_data = tomllib.loads(dst.read_text(encoding="utf-8"))
         src_data = tomllib.loads(src.read_text(encoding="utf-8"))
     except (OSError, UnicodeDecodeError, tomllib.TOMLDecodeError):
-        # Malformed or unreadable user file — leave it untouched.
+        # Malformed or unreadable user file — leave it untouched (existing
+        # "never clobber the user" contract; see the skipped test).
         result.skipped.append(rel_dst)
         return
 
@@ -618,7 +639,10 @@ def _install_toml_merge(
         if tomllib.loads(text) != dst_data:
             raise ValueError("serializer round-trip mismatch")
     except (ValueError, tomllib.TOMLDecodeError):
-        result.skipped.append(rel_dst)
+        # The minimal serializer can't faithfully re-emit the merged config
+        # (e.g. a non-BMP string value, or a nan/inf float). Leave the user's
+        # file alone, but report it as a failure — not "already present".
+        result.failed.append(rel_dst)
         return
 
     dst.write_text(text, encoding="utf-8")

src/vouch/sessions.py

@@ -149,11 +149,16 @@ def _build_summary_body(sess: Session, ids: list[str]) -> str:
     # through the review gate. Anything agent-controlled (sess.task,
     # sess.note, sess.agent) is omitted to keep the review-gate guarantee
     # intact for the Page artifact kind. See #76.
+    # crystallize does not end the session, so ended_at is often None here.
+    # A wall-clock fallback would make every retry write a different body,
+    # breaking crystallize idempotency (#256) and re-embedding needlessly, so
+    # emit a stable marker instead of datetime.now() for a still-open session.
+    ended = sess.ended_at.isoformat() if sess.ended_at else "(session still open)"
     lines = [
         f"# Session {sess.id}",
         "",
         f"**Started:** {sess.started_at.isoformat()}",
-        f"**Ended:** {(sess.ended_at or datetime.now(UTC)).isoformat()}",
+        f"**Ended:** {ended}",
         "",
         "## Crystallized artifacts",
         "",

src/vouch/web/dual_solve_api.py

@@ -71,7 +71,13 @@ def _serialize(job: DualSolveJob) -> dict[str, Any]:
              "log": c.log, "diff": c.diff}
             for c in job.candidates
         ],
-        "recommendation": ds.recommendation(job.candidates),
+        # No candidates yet (still running) -> no recommendation. Computing it
+        # over [] returns the "neither engine produced a usable diff" hint,
+        # which the SPA would otherwise render as a false failure for the whole
+        # duration of a run that is still in progress.
+        "recommendation": (
+            ds.recommendation(job.candidates) if job.candidates else None
+        ),
         "proposed_ids": list(job.proposed_ids),
         "kept_branch": job.kept_branch,
         "changed_files": ds.changed_files(kept.diff) if kept is not None else [],

tests/test_codex_rollout.py

@@ -297,3 +297,17 @@ def test_fixtures_use_placeholder_data_only() -> None:
         text = fixture.read_text(encoding="utf-8")
         assert "alice-example" in text or "session_meta" not in text
         assert "/home/a/" not in text
+
+
+def test_parse_rejects_oversized_rollout(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    # The parser streams line-by-line, but a rollout with no interior newlines
+    # (or a huge one) would still be read whole into memory. Guard on file size
+    # so an oversized/blob rollout is refused up front, mirroring the byte caps
+    # the codebase applies to other untrusted reads.
+    monkeypatch.setattr(cr, "_MAX_ROLLOUT_BYTES", 64, raising=False)
+    big = tmp_path / "rollout-2026-07-01T09-00-00-x.jsonl"
+    big.write_text("x" * 512, encoding="utf-8")
+    with pytest.raises(cr.CodexRolloutError, match="too large"):
+        cr.parse_rollout(big)

tests/test_install_adapter.py

@@ -867,3 +867,75 @@ def test_installed_wheel_resolves_adapters(tmp_path: Path) -> None:
     assert "claude-code" in result["hosts"], (
         f"installed copy can't resolve adapters: {result}"
     )
+
+
+# --- toml_merge failure is not silent success -----------------------------
+
+
+def test_toml_merge_unserializable_config_is_failed_not_skipped(tmp_path: Path) -> None:
+    # A user config.toml carrying a non-BMP char in a string value can't
+    # survive the minimal serializer's json.dumps -> tomllib round-trip, so
+    # vouch cannot install. That must be reported as a distinct failure, not
+    # bucketed as "already present" (which would tell the user vouch is wired
+    # when it is not).
+    from vouch.install_adapter import _install_toml_merge
+
+    dst = tmp_path / "config.toml"
+    dst.write_text('model = "gpt-5"\nnote = "hi \U0001f600 there"\n', encoding="utf-8")
+    src = tmp_path / "src.toml"
+    src.write_text('[mcp_servers.vouch]\ncommand = "vouch"\n', encoding="utf-8")
+
+    result = InstallResult()
+    _install_toml_merge(src, dst, result, ".codex/config.toml")
+
+    assert result.failed == [".codex/config.toml"]
+    assert result.skipped == []
+    assert "[mcp_servers.vouch]" not in dst.read_text(encoding="utf-8")
+
+
+def test_install_mcp_reports_failure_and_exits_nonzero(tmp_path: Path) -> None:
+    # End-to-end: an existing codex config.toml with an emoji makes the T1
+    # toml_merge fail. The CLI must NOT print "already present" / a clean
+    # "Done" for it, and must exit non-zero so scripts notice.
+    cfg = tmp_path / ".codex" / "config.toml"
+    cfg.parent.mkdir(parents=True)
+    cfg.write_text('model = "gpt-5"\nnote = "\U0001f600"\n', encoding="utf-8")
+
+    result = CliRunner().invoke(
+        cli, ["install-mcp", "codex", "--tier", "T1", "--path", str(tmp_path)]
+    )
+
+    assert result.exit_code != 0, result.output
+    assert "already present" not in result.output
+    assert "[mcp_servers.vouch]" not in cfg.read_text(encoding="utf-8")
+
+
+# --- manifest dst containment (path traversal) ----------------------------
+
+
+def test_install_rejects_dst_escaping_target(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    # A manifest whose `dst` escapes the target tree (via `..` or an absolute
+    # path) must be refused, not written outside `target`.
+    import vouch.install_adapter as ia
+
+    adapters = tmp_path / "adapters"
+    (adapters / "evil").mkdir(parents=True)
+    (adapters / "evil" / "payload.txt").write_text("pwned", encoding="utf-8")
+    (adapters / "evil" / "install.yaml").write_text(
+        "host: evil\n"
+        "pretty: Evil\n"
+        "fence: { begin: x, end: y }\n"
+        "tiers:\n"
+        "  T1:\n"
+        "    - { src: payload.txt, dst: ../../escape.txt }\n",
+        encoding="utf-8",
+    )
+    monkeypatch.setattr(ia, "ADAPTERS_DIR", adapters)
+
+    target = tmp_path / "project"
+    target.mkdir()
+    with pytest.raises(AdapterError, match=r"escape|outside|traversal"):
+        install("evil", target=target, tier="T1")
+    assert not (tmp_path / "escape.txt").exists()

tests/test_sessions.py

@@ -204,3 +204,45 @@ def flaky_approve(*args, **kwargs):
 
     summary = store.get_page(second["summary_page_id"])
     assert sorted(summary.claims) == sorted([c.id for c in store.list_claims()])
+
+
+def test_crystallize_open_session_summary_body_is_idempotent(
+    store: KBStore, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    # crystallize never ends the session, so sess.ended_at stays None. #256
+    # made the retry stop crashing, but the summary body still embedded a
+    # wall-clock 'Ended' stamp, so each rewrite produced a different body (and
+    # re-embedded). Retrying on an unchanged, still-open session must yield an
+    # identical page body.
+    import datetime as _dt
+
+    from vouch import sessions as s
+
+    src = store.put_source(b"e")
+    sess = sess_mod.session_start(store, agent="a", task="t")
+    propose_claim(
+        store, text="finding", evidence=[src.id], proposed_by="a",
+        session_id=sess.id,
+    )
+
+    # a clock that advances on every read, so any wall-clock stamp in the body
+    # would differ between the two crystallize calls.
+    ticks = iter([
+        _dt.datetime(2026, 1, 1, tzinfo=_dt.UTC),
+        _dt.datetime(2026, 6, 30, 12, tzinfo=_dt.UTC),
+        _dt.datetime(2026, 12, 31, tzinfo=_dt.UTC),
+    ])
+
+    class _Clock:
+        @staticmethod
+        def now(tz: object = None) -> _dt.datetime:
+            return next(ticks)
+
+    monkeypatch.setattr(s, "datetime", _Clock)
+
+    first = sess_mod.crystallize(store, sess.id, approver="u")
+    body1 = store.get_page(first["summary_page_id"]).body
+    second = sess_mod.crystallize(store, sess.id, approver="u")
+    body2 = store.get_page(second["summary_page_id"]).body
+
+    assert body1 == body2

tests/test_web_dual_solve.py

@@ -272,3 +272,37 @@ def test_spa_assets_are_served(git_kb):
         assert c.get(path).status_code == 200, path
     page = c.get("/dual-solve").text
     assert "/static/dual_solve.js" in page and "createApp" in page
+
+
+def _job(**kw):
+    from vouch.web.dual_solve_api import DualSolveJob
+
+    base = dict(id="j", issue_url="o/n#1", claude_effort="high",
+                codex_effort="high")
+    base.update(kw)
+    return DualSolveJob(**base)
+
+
+def test_serialize_omits_recommendation_while_running():
+    # While the job is still running there are no candidates yet, so
+    # recommendation([]) would say "neither engine produced a usable diff".
+    # That false negative must not be surfaced during a running job.
+    from vouch.web.dual_solve_api import _serialize
+
+    data = _serialize(_job())  # status defaults to "running", candidates=[]
+    assert data["status"] == "running"
+    assert data["recommendation"] is None
+
+
+def test_serialize_includes_recommendation_once_candidates_exist():
+    from vouch.dual_solve import Candidate
+    from vouch.web.dual_solve_api import _serialize
+
+    job = _job(status="ready")
+    job.candidates = [
+        Candidate(engine="claude", branch="ds/claude", worktree=Path("/tmp/x"),
+                  diff="+a\n+b\n", ok=True),
+    ]
+    data = _serialize(job)
+    assert data["recommendation"] is not None
+    assert data["recommendation"]["engine"] == "claude"