Add router mode support to ramalama sandbox#2822
Conversation
GitHub Actions
Inactive
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds non-blocking router startup and model discovery in ChangesMulti-model router sandbox
Sequence Diagram(s)sequenceDiagram
participant run_sandbox
participant LlamaCppPlugin
participant RouterServer
participant Agent
run_sandbox->>LlamaCppPlugin: start router runtime
run_sandbox->>RouterServer: wait_for_healthy()
run_sandbox->>RouterServer: GET /v1/models
RouterServer-->>run_sandbox: router_model_ids
run_sandbox->>Agent: run selected agent
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for multi-model router mode in the sandbox environment using the llama.cpp runtime, allowing sandboxes to run with zero or multiple models. It adds CLI arguments like --prompt and --models-max, and updates the Goose, OpenCode, and Pi agents to handle router-mode configurations. The review feedback highlights two important improvements: first, initializing the connection variable to None in _query_router_models to prevent an UnboundLocalError in the finally block if connection setup fails; second, appending engine.relabel() to the bind mount options in llama_cpp.py to ensure compatibility with SELinux-enabled systems.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def _query_router_models(port) -> list[str]: | ||
| """Query the llama.cpp router server for available model IDs.""" | ||
| try: | ||
| conn = HTTPConnection("127.0.0.1", int(port), timeout=5) | ||
| conn.request("GET", "/v1/models") | ||
| resp = conn.getresponse() | ||
| if resp.status != 200: | ||
| return [] | ||
| body = json.loads(resp.read()) | ||
| return [m.get("id", "") for m in body.get("data", []) if m.get("id")] | ||
| except Exception: | ||
| return [] | ||
| finally: | ||
| conn.close() |
There was a problem hiding this comment.
In _query_router_models, if HTTPConnection initialization raises an exception (for example, if int(port) raises a ValueError), the local variable conn will not be bound. When the finally block executes, calling conn.close() will raise an UnboundLocalError, which propagates and masks the original exception. To prevent this, initialize conn = None before the try block and check if conn is not None before calling conn.close() in the finally block.
| def _query_router_models(port) -> list[str]: | |
| """Query the llama.cpp router server for available model IDs.""" | |
| try: | |
| conn = HTTPConnection("127.0.0.1", int(port), timeout=5) | |
| conn.request("GET", "/v1/models") | |
| resp = conn.getresponse() | |
| if resp.status != 200: | |
| return [] | |
| body = json.loads(resp.read()) | |
| return [m.get("id", "") for m in body.get("data", []) if m.get("id")] | |
| except Exception: | |
| return [] | |
| finally: | |
| conn.close() | |
| def _query_router_models(port) -> list[str]: | |
| """Query the llama.cpp router server for available model IDs.""" | |
| conn = None | |
| try: | |
| conn = HTTPConnection("127.0.0.1", int(port), timeout=5) | |
| conn.request("GET", "/v1/models") | |
| resp = conn.getresponse() | |
| if resp.status != 200: | |
| return [] | |
| body = json.loads(resp.read()) | |
| return [m.get("id", "") for m in body.get("data", []) if m.get("id")] | |
| except Exception: | |
| return [] | |
| finally: | |
| if conn: | |
| conn.close() |
References
- An HTTPConnection object is not a context manager and cannot be used in a with statement. Use a try...finally block to ensure the connection is closed.
| @@ -510,12 +512,29 @@ def _serve_router(self, args: argparse.Namespace) -> None: | |||
| engine.add([f"--mount=type=bind,src={container_host_path},destination={mount_path},ro"]) | |||
There was a problem hiding this comment.
When mounting models in _build_router_engine, the bind mount options do not include engine.relabel(). On systems with SELinux enabled (such as Fedora or RHEL) and when args.selinux is set to True, this will lead to permission denied errors when the container tries to access the mounted models. Append engine.relabel() to the mount options to ensure proper SELinux labeling, consistent with other bind mounts in the codebase.
| engine.add([f"--mount=type=bind,src={container_host_path},destination={mount_path},ro"]) | |
| engine.add([f"--mount=type=bind,src={container_host_path},destination={mount_path},ro{engine.relabel()}"]) |
|
@mikebonnet had to change ARGS to --prompt for sandbox wondering if thats ok open to suggestion how how to better fix the 2 greedy arguments which are now serve then args |
0008790 to
f15cfa9
Compare
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ramalama/sandbox.py (1)
317-357: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd missing type hints on the new helpers.
These new helper signatures are missing return/parameter annotations, which weakens mypy coverage on the router path. As per coding guidelines, use type hints in Python code and ensure mypy compatibility.
Proposed fix
-def _run_sandbox_single_model(args: SandboxEngineArgsType, agent_cls: type[Agent]): +def _run_sandbox_single_model(args: SandboxEngineArgsType, agent_cls: type[Agent]) -> None: """Run sandbox with a single model (original behavior).""" @@ -def _query_router_models(port) -> list[str]: +def _query_router_models(port: int | str) -> list[str]: """Query the llama.cpp router server for available model IDs.""" @@ -def _run_sandbox_router(args: SandboxEngineArgsType, agent_cls: type[Agent]): +def _run_sandbox_router(args: SandboxEngineArgsType, agent_cls: type[Agent]) -> None: """Run sandbox in router mode (zero or multiple models)."""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ramalama/sandbox.py` around lines 317 - 357, The new helper signatures in ramalama.sandbox are missing explicit annotations, which weakens mypy coverage on the router path. Add type hints to _run_sandbox_single_model, _query_router_models, and _run_sandbox_router by annotating all parameters and return types, and keep the existing Agent/SandboxEngineArgsType references so the helpers remain mypy-compatible and consistent with the surrounding code.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ramalama/plugins/runtimes/inference/llama_cpp.py`:
- Around line 498-501: The router-mode image resolution in llama_cpp.py is
overwriting user CLI/runtime overrides by always using ActiveConfig values.
Update the logic around ActiveConfig, accel_image(), and ensure_image() so it
preserves args.image and args.engine when they are provided, and only falls back
to the config-derived image/engine when args.image is absent. Keep the existing
pattern used elsewhere in this file so Engine(args) continues to receive the
intended runtime selection.
In `@ramalama/sandbox.py`:
- Around line 50-55: The `--models-max` argument help in `sandbox.py` is
repeating the default value even though `ramalama/cli.py.add_argument` already
appends it automatically from `default=4`. Update the `parser.add_argument` help
text for `models_max` to remove the manual “(default: 4)” wording so the
rendered help only shows the default once.
- Around line 365-370: Handle the `--dryrun` path before invoking
`serve_router(...)` in `sandbox.py` so the router is never started for dry runs.
Move the dry-run check ahead of the router startup in the main flow around
`serve_router`, `agent_cls`, and `agent.engine.dryrun()`, and keep the router
startup only for non-dryrun execution. This ensures dry-run exits without
leaving any router process/container running.
- Around line 341-354: The _query_router_models helper can raise
UnboundLocalError in its finally block if int(port) or HTTPConnection(...) fails
before conn is assigned. Update _query_router_models to initialize conn before
the try or otherwise guard the conn.close() call in the finally block so
failures still return an empty list without masking the original error path.
In `@test/unit/test_sandbox_cmd.py`:
- Around line 430-445: The current test only exercises the empty-model fallback
by constructing OpenCode without router model IDs, so it does not verify the
real router flow. Update test_opencode_router_mode to pass args.router_model_ids
and assert the OpenCode.add_env_options path emits a provider.ramalama.models
map for those discovered models, while still omitting a top-level model entry.
Use the OpenCode and add_env_options symbols to locate the router-specific
config assembly and make the assertions match the actual /v1/models discovery
contract.
---
Outside diff comments:
In `@ramalama/sandbox.py`:
- Around line 317-357: The new helper signatures in ramalama.sandbox are missing
explicit annotations, which weakens mypy coverage on the router path. Add type
hints to _run_sandbox_single_model, _query_router_models, and
_run_sandbox_router by annotating all parameters and return types, and keep the
existing Agent/SandboxEngineArgsType references so the helpers remain
mypy-compatible and consistent with the surrounding code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f8a62c54-3915-4b09-9fcf-fb46f1210816
📒 Files selected for processing (3)
ramalama/plugins/runtimes/inference/llama_cpp.pyramalama/sandbox.pytest/unit/test_sandbox_cmd.py
| serve_router(cast(argparse.Namespace, args)) | ||
|
|
||
| if args.dryrun: | ||
| agent = agent_cls(args, "") | ||
| agent.engine.dryrun() | ||
| return |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not start the router before handling --dryrun.
Router mode calls serve_router(...) before the dry-run branch, then returns without the cleanup finally; if serve_router_nonblocking starts the server, --dryrun mutates local state and can leave the router container/process running.
Proposed fix
- serve_router(cast(argparse.Namespace, args))
-
if args.dryrun:
agent = agent_cls(args, "")
agent.engine.dryrun()
return
+ serve_router(cast(argparse.Namespace, args))
+
try:
wait_for_healthy(args, partial(is_healthy))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| serve_router(cast(argparse.Namespace, args)) | |
| if args.dryrun: | |
| agent = agent_cls(args, "") | |
| agent.engine.dryrun() | |
| return | |
| if args.dryrun: | |
| agent = agent_cls(args, "") | |
| agent.engine.dryrun() | |
| return | |
| serve_router(cast(argparse.Namespace, args)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ramalama/sandbox.py` around lines 365 - 370, Handle the `--dryrun` path
before invoking `serve_router(...)` in `sandbox.py` so the router is never
started for dry runs. Move the dry-run check ahead of the router startup in the
main flow around `serve_router`, `agent_cls`, and `agent.engine.dryrun()`, and
keep the router startup only for non-dryrun execution. This ensures dry-run
exits without leaving any router process/container running.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ramalama/sandbox.py (1)
317-341: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd complete annotations for the new helpers.
The new/changed helpers are missing return annotations, and
_query_router_models()leavesportuntyped. As per coding guidelines,**/*.py: “Use type hints in Python code and ensure mypy compatibility”.Proposed fix
-from typing import Optional, cast +from typing import Optional, Union, cast @@ -def _run_sandbox_single_model(args: SandboxEngineArgsType, agent_cls: type[Agent]): +def _run_sandbox_single_model(args: SandboxEngineArgsType, agent_cls: type[Agent]) -> None: @@ -def _query_router_models(port) -> list[str]: +def _query_router_models(port: Union[int, str]) -> list[str]: @@ -def _run_sandbox_router(args: SandboxEngineArgsType, agent_cls: type[Agent]): +def _run_sandbox_router(args: SandboxEngineArgsType, agent_cls: type[Agent]) -> None:Also applies to: 359-359
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ramalama/sandbox.py` around lines 317 - 341, Add explicit type hints to the helper definitions to match the mypy/type-hinting guidelines. In _run_sandbox_single_model, annotate the return type as None since it only performs side effects, and in _query_router_models, add a concrete type annotation for port (for example, the integer port value it accepts). Keep the existing behavior unchanged while updating the function signatures of _run_sandbox_single_model and _query_router_models.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ramalama/sandbox.py`:
- Line 53: The sandbox argument parsing for --models-max currently allows 0 or
negative values, unlike the positive validation used by the llama.cpp serve
parser. Update the option handling in the sandbox parser path around the
models-max argument to validate that the value is strictly positive before
router mode uses it, and reject invalid input early with the same behavior as
the existing serve-side validator.
- Around line 377-381: The router startup path in _build_router_engine() should
not continue when _query_router_models() returns no IDs, because that indicates
model discovery failed rather than a valid empty state. Add an early failure
after assigning args.router_model_ids, before creating the agent_cls instance,
so startup exits consistently when model_ids is empty instead of passing an
unusable first_model.
---
Outside diff comments:
In `@ramalama/sandbox.py`:
- Around line 317-341: Add explicit type hints to the helper definitions to
match the mypy/type-hinting guidelines. In _run_sandbox_single_model, annotate
the return type as None since it only performs side effects, and in
_query_router_models, add a concrete type annotation for port (for example, the
integer port value it accepts). Keep the existing behavior unchanged while
updating the function signatures of _run_sandbox_single_model and
_query_router_models.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 03308acd-fa38-4cab-a933-f955c6d1da01
📒 Files selected for processing (8)
docs/options/models-max.mddocs/options/prompt.mddocs/ramalama-sandbox-goose.1.md.indocs/ramalama-sandbox-opencode.1.md.indocs/ramalama-sandbox-pi.1.md.inramalama/plugins/runtimes/inference/llama_cpp.pyramalama/sandbox.pytest/unit/test_sandbox_cmd.py
✅ Files skipped from review due to trivial changes (4)
- docs/options/prompt.md
- docs/options/models-max.md
- docs/ramalama-sandbox-goose.1.md.in
- docs/ramalama-sandbox-pi.1.md.in
🚧 Files skipped from review as they are similar to previous changes (1)
- test/unit/test_sandbox_cmd.py
| parser.add_argument( | ||
| "--models-max", | ||
| dest="models_max", | ||
| type=int, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate --models-max as positive.
Sandbox currently accepts 0 or negative values, unlike the llama.cpp serve parser’s positive validator. Reject invalid values before passing them into router mode.
Proposed fix
+def _positive_int(value: str) -> int:
+ parsed = int(value)
+ if parsed <= 0:
+ raise argparse.ArgumentTypeError("--models-max must be a positive integer")
+ return parsed
+
+
def _add_router_args(parser: argparse.ArgumentParser) -> None:
@@
- type=int,
+ type=_positive_int,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type=int, | |
| def _positive_int(value: str) -> int: | |
| parsed = int(value) | |
| if parsed <= 0: | |
| raise argparse.ArgumentTypeError("--models-max must be a positive integer") | |
| return parsed | |
| def _add_router_args(parser: argparse.ArgumentParser) -> None: | |
| ... | |
| type=_positive_int, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ramalama/sandbox.py` at line 53, The sandbox argument parsing for
--models-max currently allows 0 or negative values, unlike the positive
validation used by the llama.cpp serve parser. Update the option handling in the
sandbox parser path around the models-max argument to validate that the value is
strictly positive before router mode uses it, and reject invalid input early
with the same behavior as the existing serve-side validator.
f15cfa9 to
d9eb1eb
Compare
GitHub Actions
Inactive
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ramalama/plugins/runtimes/inference/llama_cpp.py (1)
275-288: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRequire usable model IDs before marking router mode ready.
If
/modelsreturns entries withoutnameorid,model_namesbecomes[""], and router mode is marked ready because the list is non-empty. Filter invalid entries first.Proposed fix
if models_list is None: logger.debug(f"{self.name} {container_name} /models does not include a model list in the response") return False - model_names = [m.get("name") or m.get("id", "") for m in models_list] + if not isinstance(models_list, list): + logger.debug(f"{self.name} {container_name} /models model list is not an array") + return False + + model_names = [] + for model in models_list: + if not isinstance(model, dict): + continue + model_id = model.get("name") or model.get("id") + if model_id: + model_names.append(model_id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ramalama/plugins/runtimes/inference/llama_cpp.py` around lines 275 - 288, The readiness check in the llama_cpp model listing logic is treating `/models` entries with missing `name` and `id` as usable, which lets router mode become ready with invalid model data. Update the model collection in `llama_cpp.py` around the `models_list` and `model_names` handling so invalid entries are filtered out before readiness is evaluated, and make the `self.name` router-mode branch return False unless at least one non-empty model identifier is present. Keep the fix localized to the existing model discovery path used by the `New(...).model_alias` and router-ready logging logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ramalama/plugins/runtimes/inference/llama_cpp.py`:
- Around line 275-288: The readiness check in the llama_cpp model listing logic
is treating `/models` entries with missing `name` and `id` as usable, which lets
router mode become ready with invalid model data. Update the model collection in
`llama_cpp.py` around the `models_list` and `model_names` handling so invalid
entries are filtered out before readiness is evaluated, and make the `self.name`
router-mode branch return False unless at least one non-empty model identifier
is present. Keep the fix localized to the existing model discovery path used by
the `New(...).model_alias` and router-ready logging logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7061b73d-75ec-4ee0-bfa9-e0091b2abfbb
📒 Files selected for processing (8)
docs/options/models-max.mddocs/options/prompt.mddocs/ramalama-sandbox-goose.1.md.indocs/ramalama-sandbox-opencode.1.md.indocs/ramalama-sandbox-pi.1.md.inramalama/plugins/runtimes/inference/llama_cpp.pyramalama/sandbox.pytest/unit/test_sandbox_cmd.py
✅ Files skipped from review due to trivial changes (4)
- docs/options/prompt.md
- docs/ramalama-sandbox-goose.1.md.in
- docs/ramalama-sandbox-opencode.1.md.in
- docs/ramalama-sandbox-pi.1.md.in
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/options/models-max.md
- test/unit/test_sandbox_cmd.py
d9eb1eb to
31394cd
Compare
GitHub Actions
Inactive
Sure, that sounds reasonable. It's probably not necessary to make You'll need to update |
31394cd to
863b4d7
Compare
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ramalama/plugins/runtimes/inference/llama_cpp.py`:
- Line 536: The sandbox startup path in llama_cpp.py is leaking detached
container IDs because subprocess.Popen(engine.exec_args) inherits stdout; update
this call so stdout is redirected away while stderr remains visible. Keep the
behavior inside the runtime launch path unchanged otherwise, and use the
existing engine.exec_args invocation in the sandbox startup flow to locate the
fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d2bfd553-decc-48e2-81a3-6a75cc036dc1
📒 Files selected for processing (9)
docs/options/models-max.mddocs/options/prompt.mddocs/ramalama-sandbox-goose.1.md.indocs/ramalama-sandbox-opencode.1.md.indocs/ramalama-sandbox-pi.1.md.inramalama/plugins/runtimes/inference/llama_cpp.pyramalama/sandbox.pytest/e2e/test_sandbox.pytest/unit/test_sandbox_cmd.py
✅ Files skipped from review due to trivial changes (3)
- docs/ramalama-sandbox-goose.1.md.in
- docs/ramalama-sandbox-opencode.1.md.in
- docs/ramalama-sandbox-pi.1.md.in
| engine.dryrun() | ||
| return | ||
|
|
||
| subprocess.Popen(engine.exec_args) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Suppress detached container IDs from sandbox startup.
podman/docker run -d writes the container ID to stdout; because Popen inherits stdout here, ramalama sandbox can print that ID before the agent UI. Redirect stdout while keeping stderr visible.
Proposed fix
- subprocess.Popen(engine.exec_args)
+ subprocess.Popen(engine.exec_args, stdout=subprocess.DEVNULL)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| subprocess.Popen(engine.exec_args) | |
| subprocess.Popen(engine.exec_args, stdout=subprocess.DEVNULL) |
🧰 Tools
🪛 Ruff (0.15.20)
[error] 536-536: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ramalama/plugins/runtimes/inference/llama_cpp.py` at line 536, The sandbox
startup path in llama_cpp.py is leaking detached container IDs because
subprocess.Popen(engine.exec_args) inherits stdout; update this call so stdout
is redirected away while stderr remains visible. Keep the behavior inside the
runtime launch path unchanged otherwise, and use the existing engine.exec_args
invocation in the sandbox startup flow to locate the fix.
|
@mikebonnet @olliewalsh ready-to-test if you have the time! @rhatdan too I think this one is pretty cool cause it has the baked in model spawing loading + pi already does the chat summarizations as well |
863b4d7 to
8c5a9b7
Compare
GitHub Actions
Inactive
|
look at am17an/pi-llama-server#2 resolved so this pr is ready-to-test @mikebonnet @olliewalsh! Thanks @am17an for the changes on the pi extension |
8c5a9b7 to
ea6565d
Compare
GitHub Actions
Inactive
ea6565d to
dc18618
Compare
GitHub Actions
Inactive
|
|
||
| RUN npm install -g --ignore-scripts @earendil-works/pi-coding-agent@${PI_VERSION} \ | ||
| && pi install npm:pi-llama-cpp@${PI_LLAMA_CPP_VERSION} \ | ||
| && pi install npm:pi-llama-server@1.1.0 \ |
There was a problem hiding this comment.
It would be nice to define the pi-llama-server version as an ARG for consistency with the rest of the component versions.
| self.engine.add_env_option(f"OPENAI_HOST=http://localhost:{args.port}") | ||
| self.engine.add_env_option("OPENAI_API_KEY=ramalama") | ||
| self.engine.add_env_option(f"GOOSE_MODEL={self.model_name}") | ||
| if self.model_name: |
There was a problem hiding this comment.
model_name is always defined and non-empty (except in the dryrun case). I'd suggest removing the conditionals around model_name to avoid confusion.
| serve_router(cast(argparse.Namespace, args)) | ||
|
|
||
| if args.dryrun: | ||
| agent = agent_cls(args, "") |
There was a problem hiding this comment.
The second arg here should be a non-empty string (dummy or something) to match the non-dryrun behavior.
| raise ValueError("Router mode requires a resolved serving port") | ||
| model_ids = _query_router_models(args.port) | ||
| args.router_model_ids = model_ids # type: ignore[attr-defined] | ||
| first_model = model_ids[0] if model_ids else "" |
There was a problem hiding this comment.
This could be an assert that model_ids is non-empty (as enforced by _build_router_engine()), then an unconditional reference to model_ids[0].
dc18618 to
6249811
Compare
GitHub Actions
Inactive
|
@mikebonnet updated! |
mikebonnet
left a comment
mikebonnet
left a comment
There was a problem hiding this comment.
This is working great for me, really nice work!
Allow sandbox subcommands (pi, goose, opencode) to run without a model or with multiple models, using llama.cpp's router mode to dynamically load/unload models via --models-dir. - ramalama sandbox pi (no model) serves all store GGUF models - ramalama sandbox pi model1 model2 serves specified models - ramalama sandbox pi model1 works as before (single model) Changes: - Refactor _serve_router into _build_router_engine + serve_router_nonblocking for reuse from sandbox - Change sandbox MODEL arg from single positional to nargs="*" - Convert sandbox ARGS positional to --prompt named option - Add --models-max option to sandbox subparsers - Update agent classes (Pi, Goose, OpenCode) to handle empty model_name in router mode Signed-off-by: Brian <brian@fedora> Signed-off-by: Brian <bmahabir@bu.edu>
Replace pi-llama-cpp with Arman's pi-llama-server@1.0.4 extension which handles provider registration internally via LLAMA_SERVER_URL. The provider id is now just "llama-server" instead of "llama-server=<url>". Also set --models-max default to 1 so the router unloads the current model before loading a new one, freeing GPU memory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian Mahabir <56164556+bmahabirbu@users.noreply.github.com>
Signed-off-by: Brian Mahabir <56164556+bmahabirbu@users.noreply.github.com>
6249811 to
851b157
Compare
2 participants
Summary
ramalama sandbox pi(no model) — serves all GGUF models from the storeramalama sandbox pi model1 model2— serves specified modelsramalama sandbox pi model1— unchanged single-model behavior_serve_routerinto_build_router_engine+serve_router_nonblockingfor sandbox reuseARGSto--promptnamed option (required since MODEL is nownargs="*")--models-maxoption to sandbox subparsersmodels/datakeys andname/idfields in/modelsresponse/v1/modelsto discover available model IDs and configure agents accordinglyScreencast.From.2026-06-30.00-26-20.mp4
This works best with the pi agent with the build in support for llama.cpp new router mode. if you want to test with this use container_build.sh build pi-agent so that u can have the container to test
right now the container isnt built to test yet
Signed-off-by: Brian brian@fedora
🤖 Generated with Claude Code