fix(shortcuts): route accepts agent name OR id (was: id only) - #280
Conversation
AgentsApp passes agent.name to /api/agents/{ref}/shortcuts but the
route only matched against agent.id. Result: 404 -> empty list ->
AgentShortcutRow rendered null -> user saw no shortcut buttons.
The convention elsewhere in taOS (logs, agents/{name}/...) is name-
based, so accept both name and id at this route too. Renames the
helper from _get_agent_by_id to _get_agent_by_name_or_id and tries
name first, then id.
📝 WalkthroughWalkthroughAgent resolution in shortcut routes now matches by either Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Route as Server Route
participant Registry as Agent Registry
participant Shortcuts as Shortcuts Service
Client->>Route: GET /shortcuts?agent.name=alice (or agent.id=...)
Route->>Registry: _get_agent_by_name_or_id(query)
Registry-->>Route: matches agent (by name or id) / not found
Route->>Shortcuts: list_shortcuts(agent)
Shortcuts-->>Route: JSON array of shortcuts
Route-->>Client: HTTP 200 + JSON
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 8/10 reviews remaining, refill in 7 minutes and 14 seconds. Comment |
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (2 files)
Fix these issues in Kilo Cloud Reviewed by grok-code-fast-1:optimized:free · 155,115 tokens |
![kilo-code-bot[bot]](https://avatars.github.com/in/2193792?s=60&v=4){kind=small}
| agents = request.app.state.config.agents | ||
| for agent in agents: | ||
| if agent.get("id") == agent_id: | ||
| if agent.get("name") == agent_ref or agent.get("id") == agent_ref: |
There was a problem hiding this comment.
WARNING: Potential ambiguity in agent lookup
If multiple agents have the same name, this function returns the first match. This could lead to inconsistent behavior if names are not guaranteed to be unique. Consider ensuring name uniqueness or prioritizing ID matches.
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/routes/test_shortcuts_list.py (1)
88-105: ⚡ Quick winAdd a collision-case regression test for identifier precedence.
These tests cover both accepted identifiers, but they don’t lock the critical edge case: when one agent’s
idequals another agent’sname. Add one test asserting/api/agents/{ref}/shortcutsresolves bynamefirst to prevent future regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/routes/test_shortcuts_list.py` around lines 88 - 105, Add a regression test that verifies name takes precedence when an agent id collides with another agent's name: create two fixtures/agents (use existing test helpers like agent_with_shortcuts or new ones) where agent_a["name"] == agent_b["id"], then call test_client.get(f"/api/agents/{agent_a['name']}/shortcuts", headers=admin_auth_headers) and assert the response returns agent_a's shortcuts (status 200 and expected list contents), ensuring the lookup resolves by name first; name the test e.g. test_get_shortcuts_name_precedence_collision and reuse admin_auth_headers/test_client to mirror the existing tests test_get_shortcuts_by_name and test_get_shortcuts_by_id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tinyagentos/routes/shortcuts.py`:
- Around line 16-21: The function _get_agent_by_name_or_id currently uses a
single-pass "name OR id" match which can return an id-match before a name-match;
change the logic to perform a deterministic name-first lookup by first iterating
over request.app.state.config.agents and returning the agent where
agent.get("name") == agent_ref, and only if no name match is found do a second
pass returning the agent where agent.get("id") == agent_ref; keep the same 404
behavior if neither pass finds a match.
---
Nitpick comments:
In `@tests/routes/test_shortcuts_list.py`:
- Around line 88-105: Add a regression test that verifies name takes precedence
when an agent id collides with another agent's name: create two fixtures/agents
(use existing test helpers like agent_with_shortcuts or new ones) where
agent_a["name"] == agent_b["id"], then call
test_client.get(f"/api/agents/{agent_a['name']}/shortcuts",
headers=admin_auth_headers) and assert the response returns agent_a's shortcuts
(status 200 and expected list contents), ensuring the lookup resolves by name
first; name the test e.g. test_get_shortcuts_name_precedence_collision and reuse
admin_auth_headers/test_client to mirror the existing tests
test_get_shortcuts_by_name and test_get_shortcuts_by_id.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 70341d30-8cf2-49bf-a38d-064b37fbd9f8
📒 Files selected for processing (2)
tests/routes/test_shortcuts_list.pytinyagentos/routes/shortcuts.py
…allback) Single-pass 'name or id' matching is order-dependent — if an agent has id='tom' and another has name='tom', list order decides which wins. Use two passes: try name across all agents first, fall back to id only when no name matches. Addresses CodeRabbit review on PR #280.
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)
tinyagentos/routes/shortcuts.py (1)
84-109:⚠️ Potential issue | 🟠 MajorUse the resolved agent's canonical id when minting the ticket.
launch_shortcutcorrectly resolves the agent using a two-pass lookup (name first, then id), but line 104 passes the raw path parameteragent_idtomint_ticketinstead of the resolved agent's canonical id. This creates fragility downstream: the ticket and session store the raw value (which may be a name), and the proxy routes later use single-pass id-or-name resolution to look up the agent. If the agent list is reordered or a new agent is added, the single-pass resolver could match a different agent, bypassing the two-pass constraint that was enforced at launch time.Extract the canonical id from the resolved agent before minting:
Proposed fix
async def launch_shortcut( @@ - _ticket, token = mint_ticket( - agent_id=agent_id, + canonical_agent_id = agent.get("id", agent_id) + _ticket, token = mint_ticket( + agent_id=canonical_agent_id, shortcut_idx=idx, scope=scope, signing_key=signing_key, worker_url=worker_url, ttl=30, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/routes/shortcuts.py` around lines 84 - 109, The launch_shortcut flow currently resolves the agent with _get_agent_by_name_or_id(request, agent_id) but then passes the original path parameter agent_id into mint_ticket; change mint_ticket to receive the resolved agent's canonical id (e.g., agent["id"] or agent.get("id")) instead of the raw agent_id so the ticket/session record uses the resolved canonical id; update the call site where mint_ticket(...) is invoked (and any local variable you add, e.g., canonical_id) to pass that resolved id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tinyagentos/routes/shortcuts.py`:
- Around line 84-109: The launch_shortcut flow currently resolves the agent with
_get_agent_by_name_or_id(request, agent_id) but then passes the original path
parameter agent_id into mint_ticket; change mint_ticket to receive the resolved
agent's canonical id (e.g., agent["id"] or agent.get("id")) instead of the raw
agent_id so the ticket/session record uses the resolved canonical id; update the
call site where mint_ticket(...) is invoked (and any local variable you add,
e.g., canonical_id) to pass that resolved id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5326c855-067c-45fd-9c3a-a4ecab824e5e
📒 Files selected for processing (1)
tinyagentos/routes/shortcuts.py
1 participant
Summary
`AgentsApp` passes `agent.name` ("john") to `/api/agents/{ref}/shortcuts`, but the route's `_get_agent_by_id` helper only matched `agent.id` (a hex hash). Result: 404 → `shortcuts=[]` → `AgentShortcutRow` rendered null → no shortcut buttons in AgentsApp.
This was the actual reason agent shortcuts weren't showing in the iOS PWA — not a cache problem.
The fix renames the helper to `_get_agent_by_name_or_id` and tries the name first (matching the convention used by `/api/agents/{name}/logs` and similar existing routes), falling through to id. Both callers (`list_shortcuts`, `launch_shortcut`) updated.
`shortcut_proxy.py` was checked — its `_resolve_container_ip` and `_get_shortcuts_for_agent` already matched by both `id` and `name`, so no change needed there.
Test plan
Summary by CodeRabbit
New Features
Tests