From 8fec2239139901aa41d391e7767a3e11b880bde8 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Fri, 3 Jul 2026 23:16:41 +0300 Subject: [PATCH 1/2] fix(installer): failed index build no longer reports install success (#351) run_install discarded run_init_if_needed's return value and unconditionally returned 0, so a cocoindex/graph non-zero exit reported a successful install while the most important step failed silently. Distinguish failure from skip: run_init_if_needed now returns None when the index already exists (skip) and False only when indexing ran and failed; run_install returns 1 on that failure, mirroring run_update's `if not index_ok: return 1` check. Co-Authored-By: Claude --- java_codebase_rag/installer.py | 21 +++++++++++++------- tests/test_installer.py | 36 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/java_codebase_rag/installer.py b/java_codebase_rag/installer.py index fc948e85..9d464736 100644 --- a/java_codebase_rag/installer.py +++ b/java_codebase_rag/installer.py @@ -847,8 +847,8 @@ def run_init_if_needed( non_interactive: bool, quiet: bool, verbose: bool = False, -) -> bool: - """Run init if index directory has no artifacts. Return True if init was run. +) -> bool | None: + """Run init if index directory has no artifacts. The indexing sub-step (CocoIndex update + AST graph build) renders the unified ``Vectors → Optimize → Graph`` progress on **stderr** in default @@ -867,7 +867,10 @@ def run_init_if_needed( verbose: If True, raw-relay subprocess output (no Live region) Returns: - True if init was run, False if skipped + True if init ran and succeeded; False if it ran and failed (cocoindex or + graph build returned non-zero); None if skipped because the index already + exists. Callers must distinguish ``False`` (failure) from ``None`` (skip) + so a failed index does not report success (issue #351). """ from java_codebase_rag.config import ( index_dir_has_existing_artifacts, @@ -878,7 +881,7 @@ def run_init_if_needed( has_existing, _ = index_dir_has_existing_artifacts(index_dir) if has_existing: print("Index already exists. Run `java-codebase-rag reprocess` to rebuild.") - return False + return None # skipped, not failed cfg = resolve_operator_config( source_root=source_root, @@ -1561,9 +1564,12 @@ def run_install( if not quiet: print("Configuration written to", config_path) - # Run init if index directory is empty + # Run init if index directory is empty. run_init_if_needed returns True (ran + # OK), False (ran and failed — cocoindex/graph non-zero exit), or None + # (skipped: index already exists). A failed index must NOT report success in + # CI/automation; a skip is not a failure (issue #351). index_dir = (source_root / ".java-codebase-rag").resolve() - run_init_if_needed( + init_outcome = run_init_if_needed( source_root, index_dir, resolved_model, @@ -1571,5 +1577,6 @@ def run_install( quiet=quiet, verbose=verbose, ) - + if init_outcome is False: + return 1 return 0 diff --git a/tests/test_installer.py b/tests/test_installer.py index 15c3a426..176f9f06 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -1693,3 +1693,39 @@ def boom(env, *, full_reprocess, quiet, verbose=True, # The footer rendered the failure marker (red cross), not the green check. assert cli_format.styled_cross() in err_text assert cli_format.styled_check() not in err_text + + def test_install_indexing_failure_returns_nonzero(self, tmp_path, monkeypatch): + """A non-exception indexing failure (cocoindex exits non-zero) must NOT + report install success. Regression for issue #351: run_install discarded + run_init_if_needed's return value and unconditionally returned 0, so a + broken or empty index reported exit 0 in CI/automation while the most + important install step failed silently. (The exception path was already + covered; this covers the returncode != 0 path.)""" + import io + import contextlib + import subprocess + from java_codebase_rag.installer import run_install + + cwd = self._setup_repo(tmp_path, monkeypatch) + + def failing_coco(env, *, full_reprocess, quiet, verbose=True, + lance_project_root=None, on_progress=None, on_progress_console=None): + return subprocess.CompletedProcess(args=["stub"], returncode=1, stdout="", stderr="boom") + + monkeypatch.setattr("java_codebase_rag.pipeline.run_cocoindex_update", failing_coco) + + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + rc = run_install( + non_interactive=True, + agents=["claude-code"], + scope="project", + model="auto", + source_root=cwd, + quiet=False, + ) + assert rc == 1, ( + f"install reported success (exit {rc}) despite cocoindex failure (#351)" + ) + # The failure was surfaced on stderr, not swallowed. + assert "CocoIndex update failed" in err.getvalue() From cd1c774c88579f81c4fab980843d2edea27a5a92 Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Sat, 4 Jul 2026 08:49:55 +0300 Subject: [PATCH 2/2] =?UTF-8?q?test(installer):=20cover=20skip=E2=86=92Non?= =?UTF-8?q?e=E2=86=92exit-0;=20document=20#351=20deferred=20scope?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add test_install_over_existing_index_skips_init_and_exits_zero: a re-run of install over an existing index skips init and exits 0. Guards the None branch of run_init_if_needed (the PR's central design: `if init_outcome is False`), so a future `if not init_outcome` simplification that collapses None into the failure branch would fail this test instead of silently breaking idempotent re-runs. Verified red->green. - Document at the partial-failure classifier that issue #351's "treat skill/agent deploy failures as critical" is intentionally deferred: only MCP config failures are critical (a broken MCP config is fatal; missing skill/agent hints are recoverable), so the deferred scope reads as a deliberate product decision rather than an oversight. Co-Authored-By: Claude --- java_codebase_rag/installer.py | 7 +++++ tests/test_installer.py | 48 ++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/java_codebase_rag/installer.py b/java_codebase_rag/installer.py index 9d464736..9bef0baf 100644 --- a/java_codebase_rag/installer.py +++ b/java_codebase_rag/installer.py @@ -1534,6 +1534,13 @@ def run_install( print("Warning: Some artifacts failed to deploy:") for r in partial_failures: print(f" {r.path}: {r.error}") + # Severity model: only MCP config (.json/.yml/.yaml) deploy failures are + # critical (return 1) -- a broken MCP config means the server cannot start. + # Skill/agent (.md / dir) failures are downgraded to non-critical: the + # server still runs and the affected host simply lacks those hints. Issue + # #351's "treat skill/agent deploy failures as critical for the affected + # host" is intentionally DEFERRED here -- promoting them to critical is a + # product decision (recoverable vs. fatal) best made explicitly, not bundled. if all( r.success for r in results diff --git a/tests/test_installer.py b/tests/test_installer.py index 176f9f06..de4296a0 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -1729,3 +1729,51 @@ def failing_coco(env, *, full_reprocess, quiet, verbose=True, ) # The failure was surfaced on stderr, not swallowed. assert "CocoIndex update failed" in err.getvalue() + + def test_install_over_existing_index_skips_init_and_exits_zero(self, tmp_path, monkeypatch): + """A re-run of install over an existing index skips init (the index build) + and still exits 0. Regression guard for the None branch of + run_init_if_needed (issue #351): run_install uses ``if init_outcome is + False: return 1`` precisely so a SKIP (None) stays exit 0 -- a future + ``if not init_outcome`` simplification would collapse None into the + failure branch and break idempotent re-runs in CI/automation.""" + import io + import contextlib + import subprocess + from java_codebase_rag.installer import run_install + + cwd = self._setup_repo(tmp_path, monkeypatch) + + # Pre-create an existing index so index_dir_has_existing_artifacts is True + # -> run_init_if_needed returns None (skip), not True/False. + index_dir = cwd / ".java-codebase-rag" + index_dir.mkdir() + (index_dir / "code_graph.lbug").write_bytes(b"\x00" * 16) + + coco_called = [] + + def coco_should_not_run(env, *, full_reprocess, quiet, verbose=True, + lance_project_root=None, on_progress=None, on_progress_console=None): + coco_called.append(True) + return subprocess.CompletedProcess(args=["stub"], returncode=0, stdout="", stderr="") + + monkeypatch.setattr("java_codebase_rag.pipeline.run_cocoindex_update", coco_should_not_run) + + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + rc = run_install( + non_interactive=True, + agents=["claude-code"], + scope="project", + model="auto", + source_root=cwd, + quiet=False, + ) + assert rc == 0, ( + f"install over an existing index should skip init and exit 0, got exit {rc} " + "(#351 None branch: a skip must not be treated as failure)" + ) + # init was genuinely skipped, not silently succeeded. + assert not coco_called, "init should have been SKIPPED (index exists) but cocoindex ran" + # run_init_if_needed prints the skip notice to stdout (no file=sys.stderr). + assert "Index already exists" in out.getvalue()