Agent-as-a-Model: implement the TURN in POST /v1/chat/completions (opencode host-server seam) - #2329
Agent-as-a-Model: implement the TURN in POST /v1/chat/completions (opencode host-server seam)#2329jaylfc wants to merge 1 commit into
Conversation
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
📝 WalkthroughWalkthroughThe agent-model API now executes authorized chat requests through isolated per-agent OpenCode servers. It reuses sessions, handles runtime and adapter failures, applies execution timeouts, and returns OpenAI-compatible completions. Tests cover success, configuration, missing agents, validation, and adapter errors. ChangesAgent model execution
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AgentModelAPI
participant OpenCodeServer
participant Adapter
Client->>AgentModelAPI: POST /v1/chat/completions
AgentModelAPI->>OpenCodeServer: Resolve agent and ensure runtime
AgentModelAPI->>Adapter: Send latest message
Adapter-->>AgentModelAPI: Stream response or error
AgentModelAPI-->>Client: Completion or structured error
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Closing per the card's own rules, without prejudice to the content: tsk-gkh4mi explicitly requires a live round-trip against a real deployed agent pasted in the PR ("a passing unit test alone is NOT sufficient evidence") and says to release the card rather than open an unverified PR — no such evidence is here. The required doc sweep (agent-coordination + agent-manual) and CHANGELOG are also absent. And this card is already being worked in open PR #2195 (hognek, currently blocked on 4 review findings + a rebase) — two PRs building the same slice is the collision class; #2195 has seniority. If #2195 stalls permanently, this branch can be revisited as a fix-forward base. Dispatch is globally paused (bus 2295) pending the dispatcher guard fixes; the double-offer of a PR-guarded card is now also recorded on tsk-5ypylc. |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tinyagentos/routes/agent_model_api.py (1)
133-135: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate the final message before starting the turn.
A list can contain non-object values.
messages[-1].get(...)then raises inside_drive, and the client receives a 500 response for an invalid request. The adapter also requires text content.Validate that the final message is an object with string
content, then return the existing 400 envelope when it is not.🤖 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 `@tinyagentos/routes/agent_model_api.py` around lines 133 - 135, Update the request validation around messages in the agent model API to validate the final message before invoking _drive: require messages[-1] to be an object with string content, and return the existing _bad_request 400 envelope when invalid. Preserve the current non-empty-list validation for other cases.
🤖 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 `@tinyagentos/routes/agent_model_api.py`:
- Around line 69-77: Update _resolve_agent to resolve agent IDs before names,
preventing a colliding name from taking precedence over the consented ID. If
name aliases remain supported, validate after resolution that the resolved
agent’s id is included in the consent binding’s agent_ids, and reject ambiguous
or unauthorized aliases.
- Around line 207-216: Update the _drive() coroutine to call adapter.close()
from a finally block so cleanup runs after successful execution, exceptions
during ensure_session or prompt, and cancellation. Keep the existing error
logging and result["error"] assignment in the except block, while ensuring close
is attempted on every exit path.
In `@tinyagentos/taos_agent_runtime.py`:
- Around line 190-191: Update the server reuse logic around existing and
existing_model so matching cached servers do not return early based only on
is_running(). Route both newly created and matching cached servers through await
server.ensure_running(...), preserving the existing model-mismatch behavior
while restoring the health check before reuse.
- Around line 187-229: Add a per-agent asyncio.Lock around the server lifecycle
flow, covering existing-server lookup, stopping/replacing the server, all
opencode state-map updates, and await server.ensure_running(). Reuse the same
lock for concurrent requests for each agent_id, while allowing different agents
to proceed independently.
- Around line 199-200: Update the llm_key fallback in the agent runtime
initialization to never call get_litellm_master_key or assign the installation
master key. Require a scoped per-agent virtual key, provisioning one through the
existing restricted-key flow when available, or reject the configuration when
llm_key is absent.
- Around line 204-206: Update ensure_agent_opencode_server() to derive a stable,
filesystem-safe directory name from agent_id, such as a normalized hash, before
constructing home. Use that sanitized name consistently for the data_dir-based
and fallback OpenCode home paths, preventing separators, traversal components,
or absolute agent IDs from escaping the intended directory.
---
Outside diff comments:
In `@tinyagentos/routes/agent_model_api.py`:
- Around line 133-135: Update the request validation around messages in the
agent model API to validate the final message before invoking _drive: require
messages[-1] to be an object with string content, and return the existing
_bad_request 400 envelope when invalid. Preserve the current non-empty-list
validation for other cases.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 37789d7e-584b-42a7-971d-d6476834be81
📒 Files selected for processing (3)
tests/test_routes_agent_model_api.pytinyagentos/routes/agent_model_api.pytinyagentos/taos_agent_runtime.py
| def _resolve_agent(config, agent_ref: str) -> dict | None: | ||
| """Return the agent dict for *agent_ref* (matched by name first, then id) or None.""" | ||
| for agent in config.agents: | ||
| if agent.get("name") == agent_ref: | ||
| return agent | ||
| for agent in config.agents: | ||
| if agent.get("id") == agent_ref: | ||
| return agent | ||
| return None |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Authorize the resolved agent ID, not a colliding name.
The consent binding authorizes agent_ids, but this function resolves a matching name before a matching ID. If one agent name equals another agent ID, a key consented for the ID can execute the named agent instead.
Resolve by ID first and reject ambiguous aliases. If name aliases remain supported, verify that agent["id"] is in the consent binding after resolution.
Proposed fix
def _resolve_agent(config, agent_ref: str) -> dict | None:
- """Return the agent dict for *agent_ref* (matched by name first, then id) or None."""
+ """Return the agent dict for an authorized agent ID, or None."""
for agent in config.agents:
- if agent.get("name") == agent_ref:
+ if agent.get("id") == agent_ref:
return agent
- for agent in config.agents:
- if agent.get("id") == agent_ref:
- return agent
return None📝 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.
| def _resolve_agent(config, agent_ref: str) -> dict | None: | |
| """Return the agent dict for *agent_ref* (matched by name first, then id) or None.""" | |
| for agent in config.agents: | |
| if agent.get("name") == agent_ref: | |
| return agent | |
| for agent in config.agents: | |
| if agent.get("id") == agent_ref: | |
| return agent | |
| return None | |
| def _resolve_agent(config, agent_ref: str) -> dict | None: | |
| """Return the agent dict for an authorized agent ID, or None.""" | |
| for agent in config.agents: | |
| if agent.get("id") == agent_ref: | |
| return agent | |
| return None |
🤖 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 `@tinyagentos/routes/agent_model_api.py` around lines 69 - 77, Update
_resolve_agent to resolve agent IDs before names, preventing a colliding name
from taking precedence over the consented ID. If name aliases remain supported,
validate after resolution that the resolved agent’s id is included in the
consent binding’s agent_ids, and reject ambiguous or unauthorized aliases.
| async def _drive() -> None: | ||
| try: | ||
| await adapter.ensure_session() | ||
| session_ids[model] = adapter.session_id | ||
| trace_id = uuid.uuid4().hex | ||
| await adapter.prompt(messages[-1].get("content", ""), trace_id=trace_id) | ||
| await adapter.close() | ||
| except Exception as exc: | ||
| logger.exception("agent_model_api: drive task error") | ||
| result["error"] = str(exc) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Close the adapter on every exit path.
adapter.close() only runs after a successful prompt(). A session-creation error, a prompt exception, or task cancellation skips it. Each failed request can retain an httpx client and its connections.
Move await adapter.close() into a finally block in _drive().
Proposed fix
async def _drive() -> None:
try:
await adapter.ensure_session()
session_ids[model] = adapter.session_id
trace_id = uuid.uuid4().hex
await adapter.prompt(messages[-1].get("content", ""), trace_id=trace_id)
- await adapter.close()
except Exception as exc:
logger.exception("agent_model_api: drive task error")
result["error"] = str(exc)
+ finally:
+ await adapter.close()📝 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.
| async def _drive() -> None: | |
| try: | |
| await adapter.ensure_session() | |
| session_ids[model] = adapter.session_id | |
| trace_id = uuid.uuid4().hex | |
| await adapter.prompt(messages[-1].get("content", ""), trace_id=trace_id) | |
| await adapter.close() | |
| except Exception as exc: | |
| logger.exception("agent_model_api: drive task error") | |
| result["error"] = str(exc) | |
| async def _drive() -> None: | |
| try: | |
| await adapter.ensure_session() | |
| session_ids[model] = adapter.session_id | |
| trace_id = uuid.uuid4().hex | |
| await adapter.prompt(messages[-1].get("content", ""), trace_id=trace_id) | |
| except Exception as exc: | |
| logger.exception("agent_model_api: drive task error") | |
| result["error"] = str(exc) | |
| finally: | |
| await adapter.close() |
🤖 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 `@tinyagentos/routes/agent_model_api.py` around lines 207 - 216, Update the
_drive() coroutine to call adapter.close() from a finally block so cleanup runs
after successful execution, exceptions during ensure_session or prompt, and
cancellation. Keep the existing error logging and result["error"] assignment in
the except block, while ensuring close is attempted on every exit path.
| existing = app_state.opencode_servers.get(agent_id) | ||
| existing_model = app_state.opencode_server_models.get(agent_id) | ||
|
|
||
| if existing is not None and existing.is_running() and existing_model == model: | ||
| return existing | ||
|
|
||
| if existing is not None: | ||
| try: | ||
| await existing.stop() | ||
| except Exception: | ||
| logger.debug("taos_agent_runtime: error stopping agent server", exc_info=True) | ||
|
|
||
| if llm_key is None: | ||
| llm_key = get_litellm_master_key(getattr(app_state, "data_dir", None)) | ||
|
|
||
| from tinyagentos.installers.port_allocator import allocate_host_port | ||
|
|
||
| data_dir = getattr(app_state, "data_dir", None) | ||
| home = str(data_dir / f"{agent_id}-opencode") if data_dir else f"{agent_id}-opencode" | ||
| port = allocate_host_port(f"opencode-{agent_id}") | ||
|
|
||
| llm_proxy = getattr(app_state, "llm_proxy", None) | ||
| litellm_base_url = ( | ||
| f"http://127.0.0.1:{llm_proxy.port}/v1" | ||
| if llm_proxy is not None | ||
| else "http://127.0.0.1:7834/v1" | ||
| ) | ||
|
|
||
| cfg = OpenCodeServerConfig( | ||
| home=home, | ||
| port=port, | ||
| server_password=password, | ||
| litellm_base_url=litellm_base_url, | ||
| litellm_key=llm_key, | ||
| model_ids=[model], | ||
| ) | ||
| server = OpenCodeServer(cfg) | ||
| app_state.opencode_servers[agent_id] = server | ||
| app_state.opencode_server_models[agent_id] = model | ||
| app_state.opencode_server_keys[agent_id] = llm_key | ||
| app_state.opencode_server_session_ids[agent_id] = None | ||
|
|
||
| await server.ensure_running(deadline_s=180.0) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Serialize lifecycle changes for each agent.
Two requests for the same agent can both observe an unavailable server during startup. The second request can replace the first server while its ensure_running() call is suspended. Both instances can then attempt to bind the same stable port.
Use a per-agent asyncio.Lock that covers lookup, replacement, state-map updates, and ensure_running().
🤖 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 `@tinyagentos/taos_agent_runtime.py` around lines 187 - 229, Add a per-agent
asyncio.Lock around the server lifecycle flow, covering existing-server lookup,
stopping/replacing the server, all opencode state-map updates, and await
server.ensure_running(). Reuse the same lock for concurrent requests for each
agent_id, while allowing different agents to proceed independently.
| if existing is not None and existing.is_running() and existing_model == model: | ||
| return existing |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Restore the health check before reuse.
is_running() only checks whether the child process has exited. It does not check whether OpenCode accepts requests. This early return bypasses ensure_running(), so a live but unhealthy server remains cached and turns fail instead of restarting it.
Remove the early return. Call await server.ensure_running(...) for both new and matching cached servers.
🤖 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 `@tinyagentos/taos_agent_runtime.py` around lines 190 - 191, Update the server
reuse logic around existing and existing_model so matching cached servers do not
return early based only on is_running(). Route both newly created and matching
cached servers through await server.ensure_running(...), preserving the existing
model-mismatch behavior while restoring the health check before reuse.
| if llm_key is None: | ||
| llm_key = get_litellm_master_key(getattr(app_state, "data_dir", None)) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not use the LiteLLM master key for an agent server.
When llm_key is absent, this assigns the installation master key to the OpenCode configuration. That key can perform LiteLLM admin operations, including virtual-key management. An agent runtime must receive only its scoped virtual key.
Provision a restricted per-agent key, or reject agent configurations that do not provide one.
🤖 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 `@tinyagentos/taos_agent_runtime.py` around lines 199 - 200, Update the llm_key
fallback in the agent runtime initialization to never call
get_litellm_master_key or assign the installation master key. Require a scoped
per-agent virtual key, provisioning one through the existing restricted-key flow
when available, or reject the configuration when llm_key is absent.
| data_dir = getattr(app_state, "data_dir", None) | ||
| home = str(data_dir / f"{agent_id}-opencode") if data_dir else f"{agent_id}-opencode" | ||
| port = allocate_host_port(f"opencode-{agent_id}") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate agent configuration validation and identifier producers.
rg -n -C 3 'agent_id|permitted_models|config\.agents|["'\'']id["'\'']|["'\'']name["'\'']' tinyagentos testsRepository: jaylfc/taOS
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^tinyagentos/|^tests/).*' | sed -n '1,200p'
echo "== precise agent_id source/validate usages =="
rg -n -C 4 'agent_id\s*=|agent_id=|startswith\("\."|re\.compile|ALLOWED|valid|allowed_chars|Path\(.*/|pathlib|opencode' tinyagentos tests --glob '*.py' | sed -n '1,240p'
echo "== taos_agent_runtime outline/slice =="
if [ -f tinyagentos/taos_agent_runtime.py ]; then
wc -l tinyagentos/taos_agent_runtime.py
ast-grep outline tinyagentos/taos_agent_runtime.py || true
sed -n '1,260p' tinyagentos/taos_agent_runtime.py
fiRepository: jaylfc/taOS
Length of output: 36064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== agents models and schemas =="
rg -n -C 5 'class .*Agent|Agent.*\\(|agents\\s*=|id_field|allow_list|permitted|jsonschema|pydantic|BaseModel|id=Field|name=Field|str_strip|agent_id|create_agent|list_agents|delete_agent' tinyagentos tests/routes tests --glob '*.py' | sed -n '1,360p'
echo "== route slices likely agent deploy/list =="
rg -n '`@router`\.(get|post|put|patch|delete).*agents|/api/agents|agent_id: str' tinyagentos/routes tests/routes --glob '*.py' | sed -n '1,240p'
echo "== precise behavioral path probe if pathlib available =="
python3 - <<'PY'
from pathlib import PurePosixPath, PureWindowsPath
for p in ["../../etc/evil", "/etc/evil", "../evil", "agent/with/slashes"]:
base = PurePosixPath("/data")
print(p, "=>", str(base / f"{p}-opencode"), "abs", abs(p), "relative components", ".." in PurePosixPath(p).parts)
PY
echo "== agent_id validation text =="
rg -n -C 3 'agent_id|id.*agent|validate|allowed|bad_chars|regex|re\.|Path\(\)\.resolve|is_relative_to|safe.' tinyagentos tests --glob '*.py' | sed -n '1,400p'Repository: jaylfc/taOS
Length of output: 414
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== agents models and schemas =="
rg -n -C 5 'class .*Agent|Agent\(agent_id|agents\s*=|id_field|allow_list|permitted|jsonschema|pydantic|BaseModel|id=Field|name=Field|str_strip|agent_id|create_agent|list_agents|delete_agent' tinyagentos tests/routes tests --glob '*.py' 2>/dev/null | sed -n '1,360p' || true
echo "== agent_id validation text =="
rg -n -C 3 'agent_id|id.*agent|validate|allowed|bad_chars|regex|re\.|Path\(\)\.resolve|is_relative_to|safe' tinyagentos tests --glob '*.py' 2>/dev/null | sed -n '1,400p' || true
echo "== route slices likely agent deploy/list =="
rg -n '`@router`\.(get|post|put|patch|delete).*agents|/api/agents|agent_id: str' tinyagentos/routes tests/routes --glob '*.py' 2>/dev/null | sed -n '1,260p' || true
echo "== pathlib / shell path expansion probe =="
tmp="$(mktemp -d)"
mkdir -p "$tmp/data"
for p in "goodagent" "../evil" "../../etc/evil" "/tmp/abs-escape"; do
base="data"
out="$(printf '%s\n' "$base/$p-opencode")"
echo "$p => $out => $(realpath "$tmp/$out" 2>/dev/null || true)"
done
rm -rf "$tmp"
printf '\n'
python3 - <<'PY'
from pathlib import Path
for p in ["goodagent", "../evil", "../../etc/evil", "/tmp/abs-escape"]:
base = Path("/data")
candidate = base / f"{p}-opencode"
print(p, "string", str(candidate), "parts", candidate.parts, "rel_to_data", str(candidate).startswith("/data") if p == "../evil" else "")
PYRepository: jaylfc/taOS
Length of output: 50367
Sanitize agent_id before using it in the OpenCode home directory.
ensure_agent_opencode_server() interpolates agent_id into home, and there is no path-safety guard. If agent_id contains / or .., the OpenCode home can be moved outside data_dir; an absolute agent_id makes data_dir ineffective. Derive the directory name from a stable hash and normalize before server startup.
🤖 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 `@tinyagentos/taos_agent_runtime.py` around lines 204 - 206, Update
ensure_agent_opencode_server() to derive a stable, filesystem-safe directory
name from agent_id, such as a normalized hash, before constructing home. Use
that sanitized name consistently for the data_dir-based and fallback OpenCode
home paths, preventing separators, traversal components, or absolute agent IDs
from escaping the intended directory.
| if kind == "delta": | ||
| result["content"] += reply.get("content", "") | ||
| elif kind == "final": | ||
| result["content"] = reply.get("content", result["content"]) |
There was a problem hiding this comment.
WARNING: Empty content in final reply overwrites accumulated deltas
reply.get("content", result["content"]) only falls back when the key is absent. If the adapter yields {"kind": "final", "content": ""}, the accumulated stream is replaced with an empty string.
| result["content"] = reply.get("content", result["content"]) | |
| result["content"] = reply.get("content") or result["content"] |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ) | ||
|
|
||
| app_state = request.app.state | ||
| password = getattr(app_state, "opencode_server_passwords", {}).get(model, "") |
There was a problem hiding this comment.
SUGGESTION: Missing password falls back to empty string
If opencode_server_passwords exists but lacks the model key (e.g., in tests or after manual state mutation), the empty-string password silently reaches OpenCodeConfig and causes an opaque auth failure. Consider raising or generating the missing password here to fail fast.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| app_state.opencode_server_keys[agent_id] = llm_key | ||
| app_state.opencode_server_session_ids[agent_id] = None | ||
|
|
||
| await server.ensure_running(deadline_s=180.0) |
There was a problem hiding this comment.
WARNING: Failed server remains in app_state before ensure_running completes
The server object is stored in app_state.opencode_servers at line 224, but await server.ensure_running(deadline_s=180.0) at line 229 may raise. On the next request for the same agent, the stale object is retrieved; is_running() returns False, stop() is attempted (and may also fail silently), and a new server is created — leaking the old one if it partially started.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| except Exception: | ||
| logger.debug("taos_agent_runtime: error during agent server stop", exc_info=True) | ||
| finally: | ||
| servers.pop(agent_id, None) |
There was a problem hiding this comment.
WARNING: stop_agent_opencode_server leaves stale entries in related dicts
servers.pop(agent_id) and session_ids.pop(agent_id) are cleaned up, but opencode_server_passwords, opencode_server_keys, and opencode_server_models retain the agent's entries. Future calls may reuse the stale password/model instead of regenerating, or the leaked key persists in memory after the server is gone.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Reviewed by step-3.7-flash · Input: 121.3K · Output: 31K · Cached: 1.3M |
CARD TITLE (intent, not commit subject): Agent-as-a-Model: implement the TURN in POST /v1/chat/completions (opencode host-server seam)
Autonomous build of board card tsk-gkh4mi.
Files:
tests/test_routes_agent_model_api.py | 203 +++++++++++++++++++++++++++++++---
tinyagentos/routes/agent_model_api.py | 155 +++++++++++++++++++++++---
tinyagentos/taos_agent_runtime.py | 92 +++++++++++++++
3 files changed, 418 insertions(+), 32 deletions(-)
Summary by CodeRabbit