Skip to content

Commit 4e28fe4

Browse files
HumanBean17claude
andcommitted
fix: address PR review bugs (model flag, IO error propagation, ontology version)
1. --model no longer silently ignored in non-interactive mode — provided paths are resolved; missing paths fall back to "auto" with a warning. 2. merge_mcp_config IO errors now raise RuntimeError instead of returning False, so _deploy_mcp_config correctly reports failure. 3. Ontology version bumped from 16 to 17 in both shipped and repo-root SKILL.md files. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 262ec39 commit 4e28fe4

4 files changed

Lines changed: 56 additions & 24 deletions

File tree

java_codebase_rag/install_data/skills/explore-codebase/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ Any time you need to search, locate, navigate, or explore the codebase. **Do NOT
6666

6767
## Graph Navigation Reference (java-codebase-rag MCP)
6868

69-
**Ontology: 16** — if results look structurally wrong or empty across tools, the index may be missing or stale; ask the operator to rebuild.
69+
**Ontology: 17** — if results look structurally wrong or empty across tools, the index may be missing or stale; ask the operator to rebuild.
7070
Responses may include `hints_structured` (suggested next calls) and `advisories` — advisory only; ignore when `success` is false.
7171

7272
### Forced reasoning preamble (every MCP call)

java_codebase_rag/installer.py

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -216,30 +216,41 @@ def resolve_model(model_input: str | None, *, non_interactive: bool) -> str:
216216
Returns:
217217
Resolved model string ("auto" or a valid path)
218218
"""
219-
if non_interactive or not model_input:
220-
return "auto"
219+
if model_input:
220+
# Expand ~ and $HOME
221+
expanded = os.path.expandvars(model_input.strip())
222+
expanded = os.path.expanduser(expanded)
223+
model_path = Path(expanded)
221224

222-
# Expand ~ and $HOME
223-
expanded = os.path.expandvars(model_input.strip())
224-
expanded = os.path.expanduser(expanded)
225-
model_path = Path(expanded)
225+
if model_path.exists():
226+
return str(model_path)
226227

227-
if model_path.exists():
228-
return str(model_path)
228+
# Path not found
229+
if non_interactive:
230+
print(f"Warning: Model path {model_input} not found, falling back to 'auto'.")
231+
return "auto"
229232

230-
# Path not found - prompt for confirmation in interactive mode
231-
confirmed = prompt(
232-
"confirm",
233-
f"Model path {model_input} not found. Use 'auto' instead?",
234-
)
235-
if confirmed:
236-
return "auto"
237-
else:
238-
# Re-prompt for model path
239-
new_input = prompt("text", "Enter model path (or 'auto'):", default="auto")
240-
if new_input == "auto" or not new_input:
233+
confirmed = prompt(
234+
"confirm",
235+
f"Model path {model_input} not found. Use 'auto' instead?",
236+
)
237+
if confirmed:
241238
return "auto"
242-
return resolve_model(new_input, non_interactive=non_interactive)
239+
else:
240+
# Re-prompt for model path
241+
new_input = prompt("text", "Enter model path (or 'auto'):", default="auto")
242+
if new_input == "auto" or not new_input:
243+
return "auto"
244+
return resolve_model(new_input, non_interactive=non_interactive)
245+
246+
if non_interactive:
247+
return "auto"
248+
249+
# Interactive with no CLI input: prompt for model
250+
user_input = prompt("text", "Embedding model path (or 'auto'):", default="auto")
251+
if user_input == "auto" or not user_input:
252+
return "auto"
253+
return resolve_model(user_input, non_interactive=False)
243254

244255

245256
def select_hosts(*, non_interactive: bool, cli_agents: list[str] | None) -> list[HostConfig]:
@@ -438,13 +449,12 @@ def merge_mcp_config(config_path: Path, host: HostConfig, *, mcp_command: str) -
438449
os.rename(tmp_name, config_path)
439450
return True
440451
except (IOError, OSError) as e:
441-
print(f"Error: Failed to write {config_path}: {e}")
442452
if tmp_name:
443453
try:
444454
os.unlink(tmp_name)
445455
except OSError:
446456
pass
447-
return False
457+
raise RuntimeError(f"Failed to write {config_path}: {e}") from e
448458

449459

450460
def _read_package_artifact(relative_path: str) -> str:

skills/explore-codebase/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ Any time you need to search, locate, navigate, or explore the codebase. **Do NOT
6666

6767
## Graph Navigation Reference (java-codebase-rag MCP)
6868

69-
**Ontology: 16** — if results look structurally wrong or empty across tools, the index may be missing or stale; ask the operator to rebuild.
69+
**Ontology: 17** — if results look structurally wrong or empty across tools, the index may be missing or stale; ask the operator to rebuild.
7070
Responses may include `hints_structured` (suggested next calls) and `advisories` — advisory only; ignore when `success` is false.
7171

7272
### Forced reasoning preamble (every MCP call)

tests/test_installer.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,28 @@ def mock_prompt(*args, **kwargs):
219219
result = resolve_model("/nonexistent/path", non_interactive=False)
220220
assert result == "auto"
221221

222+
def test_model_non_interactive_with_path_uses_path(self, tmp_path):
223+
"""--model /path/to/model with --non-interactive → uses the path"""
224+
model_file = tmp_path / "model.gguf"
225+
model_file.write_text("fake model")
226+
from java_codebase_rag.installer import resolve_model
227+
result = resolve_model(str(model_file), non_interactive=True)
228+
assert result == str(model_file)
229+
230+
def test_model_non_interactive_with_bad_path_falls_back(self, capsys):
231+
"""--model /bad/path with --non-interactive → warning + auto"""
232+
from java_codebase_rag.installer import resolve_model
233+
result = resolve_model("/nonexistent/model.gguf", non_interactive=True)
234+
assert result == "auto"
235+
captured = capsys.readouterr()
236+
assert "Warning" in captured.out
237+
238+
def test_model_non_interactive_no_input_returns_auto(self):
239+
"""no --model with --non-interactive → auto"""
240+
from java_codebase_rag.installer import resolve_model
241+
result = resolve_model(None, non_interactive=True)
242+
assert result == "auto"
243+
222244

223245
class TestSelectHostsAndScope:
224246
"""Test select_hosts and select_scope functions."""

0 commit comments

Comments
 (0)