Conversation
…ortcut, DashboardShortcut TypedDicts
…s to use full framework dicts
Also fix rfind-based separator split: HMAC-SHA256 sig is always 32 bytes, so the separator is at the fixed offset len(raw)-33. rfind misfired when 0x2e appeared in the sig bytes, causing intermittent invalid-signature errors on valid (expired/replayed) tokens.
…env/static and 60s LRU cache
Adds tinyagentos/cluster/worker_registry.py with get_local_worker() returning a deterministic signing key and local worker URL. Full implementation deferred to Tasks 22-23 (remote workers, key persistence). Also scaffolds tests/routes/ package for upcoming route tests.
…iltering Introduces the list-shortcuts route that returns only the shortcuts a user's capability set allows them to see. Adds: - tinyagentos/routes/shortcuts.py with the GET route - get_current_user FastAPI dependency added to tinyagentos/auth.py - tests/routes/conftest.py with test_client, admin/chat/shell auth header fixtures, seeded_agent_factory, and patch_worker_signing_key - tests/routes/test_shortcuts_list.py (5 tests, all passing)
…ints HMAC ticket
Extends tinyagentos/routes/shortcuts.py with the launch endpoint.
Mints a 30-second HMAC-SHA256 ticket via tinyagentos.shortcuts.tickets,
gets the local worker config from worker_registry, and returns
{redirect_url, expires_in: 30}. Returns 403 on capability mismatch,
404 for unknown agent or out-of-range idx.
Tests in tests/routes/test_shortcuts_launch.py (6 tests, all passing).
…hortcut cookie, 302
Adds tinyagentos/routes/shortcut_proxy.py with the /redeem endpoint.
Validates the HMAC-SHA256 ticket, creates an in-memory session, sets
the taos_shortcut httponly cookie, and 302-redirects to
/shortcut/terminal/{agent_id}/{idx} or /shortcut/dashboard/{agent_id}/{idx}/.
Exempts /redeem from AuthMiddleware (ticket provides its own auth).
Tests in tests/routes/test_shortcut_redeem.py (7 tests, all passing).
…bilities collision
…er port
Adds the /shortcut/dashboard/{agent}/{idx}/{path} route that validates the
taos_shortcut session cookie then reverse-proxies requests to the agent
container's dashboard port. Strips hop-by-hop headers and controller cookies
in both directions. Returns 401/403/404 for invalid sessions, 502/503/504
for upstream errors.
Also exempts /shortcut/ prefix from the main auth middleware (the route uses
its own shortcut session cookie for auth), adds respx + websockets to dev
deps, and adds websockets as a runtime dep for the upcoming WS proxy.
…xy via read_token_source Adds _build_auth_header() which reads a token via read_token_source (sync, wrapped in asyncio.to_thread) and injects Authorization: Bearer <token> or Authorization: Basic <b64> into upstream requests. auth.type=none or a null token_source skips injection. proxy_dashboard now calls this before forwarding.
…oard proxy
SSE responses are transparently streamed through the existing StreamingResponse
path — no special handling needed. Adds the WebSocket route at
/shortcut/dashboard/{agent}/{idx}/ws/{path} that validates the taos_shortcut
cookie then opens a websockets.connect() tunnel to the container, bidirectionally
piping messages until either side closes.
…n and scope check
…ead of default shell
…ng key on repeat calls
… mode AgentsApp is a single file (not a dir); component placed at desktop/src/components/AgentShortcutRow.tsx per plan fallback rule.
- Update AgentShortcutRow.onLaunch signature to pass full shortcut object instead of just idx, so callers can route by kind without extra lookups - Update AgentShortcutRow.test.tsx to assert new (agentId, shortcut) call - Add AgentShortcutRow below each AgentRow in the agents.map loop - Add handleShortcutLaunch stub (fetch + kind-based routing via openWindow) - New AgentsApp.test.tsx verifies shortcut row renders per agent with onLaunch
…AgentsApp
- dashboard kind: fetch launch endpoint, open BrowserApp via openWindow with
{ initialUrl: redirect_url }
- tui / container-terminal kind: extract ticket from redirect_url query param,
open TerminalApp via openWindow with { shortcut: { wsUrl, ticket } }
- kind comes from the shortcut object passed by AgentShortcutRow (no response
body parsing needed)
- 3 new tests in AgentsApp.shortcut-click.test.tsx cover all three routing paths
…ket frame on open
… show status indicator
…mount (idempotent)
|
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 a ticketed agent-shortcuts system: UI shortcut buttons mint HMAC tickets via a launch API, redeem creates a short-lived cookie session, and the server proxies dashboard HTTP/WS or bridges terminal WebSocket↔PTY via a local worker and container backends. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Desktop User
participant Desktop as Desktop App
participant API as Backend API
participant Worker as Local Worker
participant Container as Container / Upstream
User->>Desktop: Click agent shortcut
Desktop->>API: POST /api/agents/{id}/shortcuts/{idx}/launch
API->>Worker: get_local_worker() -> signing_key, worker_url
API->>API: mint_ticket(..., signing_key) -> token
API-->>Desktop: { redirect_url: "/redeem?t=<token>", expires_in: 30 }
Desktop->>API: GET /redeem?t=<token>
API->>API: validate_ticket -> create session, set taos_shortcut cookie
API-->>Desktop: 302 -> /shortcut/{scope}/{agent}/{idx}
alt Dashboard
Desktop->>Desktop: Open BrowserApp(initialUrl)
Desktop->>API: HTTP /shortcut/dashboard/{agent}/{idx}/...
API->>Container: proxied HTTP request (auth injection optional)
Container-->>API: response
API-->>Desktop: proxied response
else Terminal/TUI
Desktop->>Desktop: Open TerminalApp({ wsUrl, ticket })
Desktop->>API: WS /shortcut/terminal/{agent}/{idx}
API->>Container: spawn_pty(name, cmd) -> PtyHandle
loop IO
Desktop->>API: WS -> user input
API->>Container: PtyHandle.write(...)
Container->>API: PtyHandle.read() -> output
API-->>Desktop: WS -> output
end
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 11 minutes and 50 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (2)
desktop/src/apps/AgentsApp.tsx-1881-1898 (1)
1881-1898:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDon't swallow shortcut launch failures.
This path currently no-ops on non-2xx responses and throws on network/JSON errors, so capability/auth failures look like a dead button. Surface an error here the same way the other agent actions do.
Suggested fix
const handleShortcutLaunch = useCallback(async (agentId: string, shortcut: AgentShortcut) => { - const res = await fetch( - `/api/agents/${encodeURIComponent(agentId)}/shortcuts/${shortcut.idx}/launch`, - { method: "POST", headers: { Accept: "application/json" } } - ); - if (!res.ok) return; - const { redirect_url } = await res.json() as { redirect_url: string }; - const kind = shortcut.kind; - if (kind === "dashboard") { - const app = getApp("browser"); - if (app) openWindow("browser", app.defaultSize, { initialUrl: redirect_url }); - } else if (kind === "tui" || kind === "container-terminal") { - const url = new URL(redirect_url, window.location.href); - const ticket = url.searchParams.get("t") ?? redirect_url; - const app = getApp("terminal"); - if (app) openWindow("terminal", app.defaultSize, { shortcut: { wsUrl: redirect_url, ticket } }); - } + try { + const res = await fetch( + `/api/agents/${encodeURIComponent(agentId)}/shortcuts/${shortcut.idx}/launch`, + { method: "POST", headers: { Accept: "application/json" } } + ); + if (!res.ok) { + const err = await res.json().catch(() => ({})); + window.alert((err as { error?: string }).error ?? `Shortcut launch failed (${res.status})`); + return; + } + const { redirect_url } = await res.json() as { redirect_url?: string }; + if (!redirect_url) { + window.alert("Shortcut launch failed: missing redirect URL"); + return; + } + const kind = shortcut.kind; + if (kind === "dashboard") { + const app = getApp("browser"); + if (app) openWindow("browser", app.defaultSize, { initialUrl: redirect_url }); + } else if (kind === "tui" || kind === "container-terminal") { + const url = new URL(redirect_url, window.location.href); + const ticket = url.searchParams.get("t") ?? redirect_url; + const app = getApp("terminal"); + if (app) openWindow("terminal", app.defaultSize, { shortcut: { wsUrl: redirect_url, ticket } }); + } + } catch (err) { + window.alert(err instanceof Error ? err.message : "Shortcut launch failed"); + } }, [openWindow]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desktop/src/apps/AgentsApp.tsx` around lines 1881 - 1898, handleShortcutLaunch currently no-ops on non-OK responses and lets network/JSON errors bubble, making failures silent; wrap the fetch/JSON parse in try/catch, treat non-2xx responses as errors (parse any error message from res.json() or include status text) and surface them using the same error-reporting utility used by other agent actions in this file (instead of returning early). Specifically, update handleShortcutLaunch to catch exceptions and call the shared error handler (the same function other agent action handlers call), and only proceed to getApp/openWindow when the response is OK and redirect_url is present.tests/containers/test_lxc_pty.py-61-70 (1)
61-70:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid converting real integration failures into skips via broad exception handling.
Catching
Exceptionhere can hide actual test regressions as “environment unavailable”. Restrict skipping to expected environment/setup failures and let other exceptions fail the test.Suggested fix
def test_spawn_pty_integration(): """Real integration test — requires a running container taos-agent-test-pty.""" backend = LXCBackend() - try: - handle = backend.spawn_pty("test-pty", cmd=None) - handle.write(b"echo hello-from-pty\n") - import time - time.sleep(0.2) - output = handle.read(4096) - handle.close() - assert b"hello-from-pty" in output - except Exception as exc: - pytest.skip(f"Container taos-agent-test-pty not available: {exc}") + try: + handle = backend.spawn_pty("test-pty", cmd=None) + except (FileNotFoundError, OSError, RuntimeError) as exc: + pytest.skip(f"Container taos-agent-test-pty not available: {exc}") + return + + try: + handle.write(b"echo hello-from-pty\n") + import time + time.sleep(0.2) + output = handle.read(4096) + assert b"hello-from-pty" in output + finally: + handle.close()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/containers/test_lxc_pty.py` around lines 61 - 70, The test currently catches all exceptions around backend.spawn_pty/handle.write/read/close and turns any failure into pytest.skip, which masks real regressions; update the try/except to only catch expected environment/setup errors (e.g., the backend-specific container-not-found or connection errors such as ContainerNotFoundError, LXCException, OSError/ConnectionError or whatever exceptions backend.spawn_pty documents) and call pytest.skip only for those, while allowing other exceptions to propagate (or re-raise them) so genuine test failures are not converted to skips; reference backend.spawn_pty, handle.read, handle.close and pytest.skip when making this change.
🧹 Nitpick comments (1)
tests/routes/test_shortcut_terminal.py (1)
72-76: ⚡ Quick winAssert the specific WebSocket rejection here.
pytest.raises(Exception)will also pass on unrelated client/setup failures, so this test does not prove the missing-cookie path was rejected in the intended way. Please assert the concrete disconnect or denial type and its close/status code instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/routes/test_shortcut_terminal.py` around lines 72 - 76, The test currently catches any Exception; replace pytest.raises(Exception) with an assertion for the concrete WS denial used by the app (e.g., pytest.raises(starlette.websockets.WebSocketDisconnect)) when calling test_client.websocket_connect(f"/shortcut/terminal/{agent_id}/0") and then assert the disconnect's close code equals the expected denial code (or, if the client yields a ws object, read the close_code via ws.close_code and assert it equals the app's configured rejection/status code); reference test_client.websocket_connect, agent_id and ws.receive_text/close_code to locate and update the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@desktop/src/apps/__tests__/AgentsApp.shortcut-click.test.tsx`:
- Around line 1-3: The tests call setupFetch() which replaces global.fetch but
never restores it; save the original fetch before calling setupFetch (e.g.,
const originalFetch = global.fetch) and add an afterEach/teardown that restores
global.fetch = originalFetch and cleans mocks (vi.resetAllMocks() or
vi.restoreAllMocks()) so setupFetch() changes do not leak into other suites;
update the test file's beforeEach/afterEach blocks (or add an afterEach) to
perform the restore for the AgentsApp.shortcut-click tests and for the other
blocks around lines 78-113 that also call setupFetch().
In `@desktop/src/apps/__tests__/AgentsApp.test.tsx`:
- Line 1: The test suite replaces the process-global fetch and never restores
it, causing order-dependent failures; capture the original fetch (e.g., const
originalFetch = global.fetch) before stubbing and add an afterEach hook that
restores global.fetch = originalFetch (and optionally calls
vi.resetAllMocks()/vi.restoreAllMocks()) so the fetch stub introduced in this
file (used in beforeEach or individual it blocks) is always reverted after each
test.
In `@desktop/src/apps/BrowserApp.tsx`:
- Around line 54-58: BrowserApp currently only applies initialUrl to the iframe
DOM (via iframeRef and initialUrlApplied) which leaves React state (url,
inputValue, history) and external-mode launch behavior out of sync; change
BrowserApp to initialize and/or update React state from the initialUrl prop: set
url and inputValue to initialUrl (when provided and not already applied) and
push the initialUrl into the history state so
copy/refresh/open-in-tab/back-forward operate on the correct page, and ensure
the same logic runs when iframeRef is null (external mode) by using a useEffect
that reads initialUrl and updates url, inputValue, and history instead of
relying only on iframeRef or initialUrlApplied; reference BrowserApp,
initialUrl, initialUrlApplied, iframeRef, setUrl, setInputValue and your
history-handling functions to locate where to apply the changes.
In `@desktop/src/apps/TerminalApp.shortcut-ws.test.tsx`:
- Around line 5-22: The MockWebSocket class is missing the standard WebSocket
ready-state constants so references like WebSocket.OPEN/CONNECTING resolve to
undefined and cause readyState comparisons to be false-positively equal; add
static numeric constants (e.g. OPEN, CONNECTING, CLOSING, CLOSED) to
MockWebSocket and ensure the mock's default readyState uses
MockWebSocket.CONNECTING and tests can set instances' readyState to
MockWebSocket.OPEN when simulating an open socket; update any test setup that
references WebSocket.* to instead rely on these static fields on MockWebSocket
(class name: MockWebSocket, property: readyState, methods: send/close).
In `@desktop/tests/agent-shortcuts.spec.ts`:
- Around line 73-76: The test is using filter({ has: ... }) which looks for
descendants, but aria-label is on the button itself; update the locator to match
the button's own attribute. Replace the
page.locator(".agent-shortcut-btn").filter({ has:
page.locator('[aria-label*="shell" i], [aria-label*="terminal" i]') }).first()
(and the analogous dashBtn locator) with a single attribute selector that
targets the element itself, e.g. use
page.locator('.agent-shortcut-btn[aria-label*="shell" i],
.agent-shortcut-btn[aria-label*="terminal" i]').first() so the test matches the
button in AgentShortcutRow.tsx by its aria-label.
In `@tests/cluster/test_local_worker_enroll.py`:
- Around line 39-57: The test mutates
tinyagentos.cluster.local_worker._LOCAL_SIGNING_KEY and doesn't restore it;
update test_enroll_generates_random_signing_key to save the original value of
_LOCAL_SIGNING_KEY at start, run the enrollment checks (using
enroll_local_worker and ClusterManager as currently written), and finally
restore tinyagentos.cluster.local_worker._LOCAL_SIGNING_KEY to the saved
original (in a finally block or using try/finally) so module-global state is
preserved for other tests.
In `@tinyagentos/auth.py`:
- Around line 156-158: The code in _public_user builds caps as an empty list
when record.get("capabilities") is missing which treats legacy users as having
no capabilities; instead preserve absence by leaving capabilities as None (or
omitting the field) so old users retain default behavior. Change the assignment
raw_caps = record.get("capabilities"); caps = list(raw_caps) if raw_caps is not
None else None and ensure the returned dict for _public_user (and the similar
block later that creates a user dict at the other return around the same logic)
sets "capabilities": caps (or omits the key when caps is None) rather than an
empty list. This keeps legacy users from being locked out.
In `@tinyagentos/cluster/worker_protocol.py`:
- Around line 51-52: The signing_key field currently defaults to an empty byte
string via default_factory=bytes, which allows silent use of a known-empty
secret; change the declaration so a non-empty key must be provided and fail fast
when absent — either remove the default so signing_key: bytes becomes a required
init argument, or make it Optional[bytes] and add a __post_init__ validation in
the dataclass that raises ValueError if signing_key is None or signing_key ==
b""; update any construction sites to pass an explicit key and locate validation
logic near the dataclass that declares signing_key (use the field name
signing_key and implement __post_init__ on that class).
In `@tinyagentos/containers/lxc.py`:
- Around line 42-51: In close(), handle subprocess.TimeoutExpired from
self._proc.wait(timeout=5): after sending SIGTERM and closing self._master_fd,
wrap the self._proc.wait(timeout=5) call in a try/except that catches
subprocess.TimeoutExpired and in that except call self._proc.kill() (and
optionally wait again) so the child is deterministically terminated when incus
exec ignores SIGTERM; reference the close method and
self._proc.wait/self._proc.kill symbols when editing.
- Around line 26-30: The read() method in tinyagentos.containers.lxc.py should
handle PTY EOF by catching OSError(errno.EIO) raised by os.read on the master
FD; update the read(self, size: int = 4096) function to wrap
os.read(self._master_fd, size) in a try/except that catches OSError, checks if
e.errno == errno.EIO (or errno.EBADF if appropriate), and return b"" (and/or
trigger stream closure) instead of letting the exception propagate; ensure you
import errno at the top and reference self._master_fd and the read() method name
when applying the change.
In `@tinyagentos/frameworks.py`:
- Around line 216-221: The ValueError raised when shortcut validation fails
should be converted to a FrameworkManifestError so create_app() can swallow it;
in the except block where validate_shortcuts(shortcuts) raises ValueError, raise
FrameworkManifestError(f"framework '{fw_id}': {exc}") from exc (preserving the
original exception chain) instead of raising ValueError, referencing the
validate_shortcuts call and fw_id to locate the change.
In `@tinyagentos/routes/shortcut_proxy.py`:
- Around line 32-57: The in-memory session store (_sessions) used by
_new_session and _get_session makes session_id in the cookie process-local and
will cause intermittent 401s behind multiple workers; replace the process-local
state with a shared or client-persisted secure session: either store session
payload (agent_id, shortcut_idx, scope, expires_at) in a centralized store
(e.g., Redis) keyed by session_id and update _new_session/_get_session to
read/write that store, or embed the session payload into a signed/encrypted
cookie (removing reliance on _sessions) and validate/refresh expires_at in
_get_session; ensure you still refresh the TTL on access and revoke the old
_sessions usage everywhere that reads session state.
- Around line 219-223: proxy_dashboard currently always forwards to the shortcut
base path (base_path + "/") so any captured subpath ({path:path}) is lost;
update proxy_dashboard to append the incoming captured subpath (the route
parameter "path" or request.path_params.get("path")/path variable) to
upstream_path when building upstream_url (ensure you avoid double slashes when
path is empty), e.g. build upstream_path = base_path + ("" if not path else
f"/{path}") and then upstream_url = f"http://{ip}:{port}{upstream_path}"; apply
the same change to the analogous code block around the 322-337 region so
static/assets/deep links forward correctly.
- Around line 367-412: The socket handler currently accepts and forwards all
text frames directly into the PTY (in ws_to_pty / after websocket.accept), so
the initial {"type":"ticket", ticket} handshake from TerminalApp can be injected
into the shell; fix this by consuming and validating the ticket handshake before
any data is written to the PTY. Specifically, after computing shortcut =
_get_shortcut_from_cookie(...) (and before starting pty_to_ws/ws_to_pty or
writing to pty), read one initial message from websocket.receive_text(), attempt
to parse it as JSON and handle a {"type":"ticket", "ticket":...} object:
validate that ticket against the shortcut/ticket store (or fallback auth), close
the socket with 1008 on invalid/missing ticket, and do NOT call pty_handle.write
for that message; only once the ticket is consumed and validated call
websocket.accept() and start pty_to_ws/ws_to_pty loops. Ensure pty_to_ws and
ws_to_pty continue to ignore/skip the consumed handshake and only forward
subsequent frames.
- Around line 459-465: The client→upstream coroutine _client_to_upstream
currently calls websocket.receive_bytes(), which raises on text frames and
prematurely tears down that leg; change it to use the generic receive() (or try
receive_bytes then fall back) and handle both binary and text frames: detect
whether the incoming frame contains "bytes" or "text" and forward bytes directly
and text as UTF-8 encoded bytes (or use upstream.send_text if upstream expects
text). Update _client_to_upstream to catch only connection/IO exceptions and not
swallow text-frame events so the proxy remains healthy for dashboard sockets.
In `@tinyagentos/shortcuts/capabilities.py`:
- Around line 31-34: The membership check uses caps = user.get("capabilities")
without validating its type, which can lead to false positives for strings or
TypeError for unsupported types; update the function (the block using
user.get("capabilities"), variable caps and parameter cap) to first verify caps
is one of the expected container types used in the codebase (e.g., list, set,
frozenset, or tuple) and return False if not, then perform the membership test;
also update the function docstring to state that capabilities are stored as
lists by the auth system (and accepted container types) rather than only
set/frozenset.
In `@tinyagentos/shortcuts/tickets.py`:
- Around line 97-100: The current two-step JTI handling (calling
tracker.seen(jti) then tracker.record(jti, exp=...)) is racy; replace it with a
single atomic operation on the tracker (e.g., add and call a method like
tracker.consume_jti(jti, exp) or tracker.record_if_new(jti, exp) that does
check-and-set atomically) and raise ValueError("replayed jti") when that atomic
call indicates the jti already exists; update the code to call that new atomic
API instead of seen() + record() and implement the backend logic in the tracker
to perform the atomic check-and-set.
In `@tinyagentos/shortcuts/token_source.py`:
- Around line 79-84: The subprocess.run calls in token_source.py (the call that
assigns result and the later call that assigns the key result) can raise
subprocess.TimeoutExpired or OSError and currently bypass the intended "return
None on failure" behavior; wrap each subprocess.run invocation in a try/except
that catches subprocess.TimeoutExpired and OSError and returns None on
exception, ensuring the function (the routine that calls subprocess.run and
assigns result/key_result) returns None on failure and logs or silences the
error as appropriate.
- Around line 77-84: The container_env branch is vulnerable because it
interpolates source["var"] into a sh -c string; change the subprocess invocation
in the kind == "container_env" handling to call printenv directly (no shell) and
pass the var as a separate argument to subprocess.run (e.g., ["incus","exec",
container, "--","printenv", var]) and remove use of sh -c; before calling
subprocess, validate source["var"] against a safe env-var name pattern (letters,
digits, and underscores, not starting with a digit) and reject/raise on invalid
names, then use the subprocess result.stdout.strip() for the token handling and
handle missing/empty output appropriately.
---
Minor comments:
In `@desktop/src/apps/AgentsApp.tsx`:
- Around line 1881-1898: handleShortcutLaunch currently no-ops on non-OK
responses and lets network/JSON errors bubble, making failures silent; wrap the
fetch/JSON parse in try/catch, treat non-2xx responses as errors (parse any
error message from res.json() or include status text) and surface them using the
same error-reporting utility used by other agent actions in this file (instead
of returning early). Specifically, update handleShortcutLaunch to catch
exceptions and call the shared error handler (the same function other agent
action handlers call), and only proceed to getApp/openWindow when the response
is OK and redirect_url is present.
In `@tests/containers/test_lxc_pty.py`:
- Around line 61-70: The test currently catches all exceptions around
backend.spawn_pty/handle.write/read/close and turns any failure into
pytest.skip, which masks real regressions; update the try/except to only catch
expected environment/setup errors (e.g., the backend-specific
container-not-found or connection errors such as ContainerNotFoundError,
LXCException, OSError/ConnectionError or whatever exceptions backend.spawn_pty
documents) and call pytest.skip only for those, while allowing other exceptions
to propagate (or re-raise them) so genuine test failures are not converted to
skips; reference backend.spawn_pty, handle.read, handle.close and pytest.skip
when making this change.
---
Nitpick comments:
In `@tests/routes/test_shortcut_terminal.py`:
- Around line 72-76: The test currently catches any Exception; replace
pytest.raises(Exception) with an assertion for the concrete WS denial used by
the app (e.g., pytest.raises(starlette.websockets.WebSocketDisconnect)) when
calling test_client.websocket_connect(f"/shortcut/terminal/{agent_id}/0") and
then assert the disconnect's close code equals the expected denial code (or, if
the client yields a ws object, read the close_code via ws.close_code and assert
it equals the app's configured rejection/status code); reference
test_client.websocket_connect, agent_id and ws.receive_text/close_code to locate
and update the assertion.
🪄 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: bfc2de8c-5791-4a29-8601-2f2299839849
📒 Files selected for processing (59)
desktop/playwright.config.tsdesktop/src/apps/AgentsApp.tsxdesktop/src/apps/BrowserApp.initialUrl.test.tsxdesktop/src/apps/BrowserApp.tsxdesktop/src/apps/TerminalApp.shortcut-ui.test.tsxdesktop/src/apps/TerminalApp.shortcut-ws.test.tsxdesktop/src/apps/TerminalApp.tsxdesktop/src/apps/__tests__/AgentsApp.shortcut-click.test.tsxdesktop/src/apps/__tests__/AgentsApp.test.tsxdesktop/src/components/AgentShortcutRow.test.tsxdesktop/src/components/AgentShortcutRow.tsxdesktop/src/hooks/use-agent-shortcuts.test.tsdesktop/src/hooks/use-agent-shortcuts.tsdesktop/tests/agent-shortcuts.spec.tspyproject.tomltests/cluster/__init__.pytests/cluster/test_local_worker_enroll.pytests/cluster/test_worker_registration.pytests/containers/__init__.pytests/containers/test_lxc_pty.pytests/containers/test_pty_abstract.pytests/containers/test_pty_stubs.pytests/routes/__init__.pytests/routes/conftest.pytests/routes/test_shortcut_proxy.pytests/routes/test_shortcut_redeem.pytests/routes/test_shortcut_terminal.pytests/routes/test_shortcuts_launch.pytests/routes/test_shortcuts_list.pytests/shortcuts/__init__.pytests/shortcuts/test_auth_capabilities.pytests/shortcuts/test_capabilities.pytests/shortcuts/test_framework_shortcuts.pytests/shortcuts/test_frameworks_wire.pytests/shortcuts/test_jti_eviction.pytests/shortcuts/test_tickets.pytests/shortcuts/test_token_source.pytests/shortcuts/test_token_source_invalidation.pytests/shortcuts/test_types.pytests/shortcuts/test_validation.pytinyagentos/app.pytinyagentos/auth.pytinyagentos/auth_middleware.pytinyagentos/cluster/local_worker.pytinyagentos/cluster/worker_protocol.pytinyagentos/cluster/worker_registry.pytinyagentos/containers/apple_backend.pytinyagentos/containers/backend.pytinyagentos/containers/docker.pytinyagentos/containers/lxc.pytinyagentos/frameworks.pytinyagentos/routes/shortcut_proxy.pytinyagentos/routes/shortcuts.pytinyagentos/shortcuts/__init__.pytinyagentos/shortcuts/capabilities.pytinyagentos/shortcuts/tickets.pytinyagentos/shortcuts/token_source.pytinyagentos/shortcuts/types.pytinyagentos/shortcuts/validation.py
Comment on lines
+5
to
+22
| class MockWebSocket { | ||
| static instances: MockWebSocket[] = []; | ||
| url: string; | ||
| onopen: (() => void) | null = null; | ||
| onmessage: ((e: MessageEvent) => void) | null = null; | ||
| onclose: (() => void) | null = null; | ||
| sentMessages: unknown[] = []; | ||
| readyState = WebSocket.CONNECTING; | ||
|
|
||
| constructor(url: string) { | ||
| this.url = url; | ||
| MockWebSocket.instances.push(this); | ||
| } | ||
| send(data: unknown) { | ||
| this.sentMessages.push(data); | ||
| } | ||
| close() {} | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== MockWebSocket definition ==="
sed -n '1,80p' desktop/src/apps/TerminalApp.shortcut-ws.test.tsx
echo
echo "=== WebSocket state checks in desktop/src/apps ==="
rg -n -C2 'WebSocket\.(OPEN|CONNECTING|CLOSING|CLOSED)|readyState' desktop/src/appsRepository: jaylfc/tinyagentos
Length of output: 7390
Add the standard WebSocket state constants to the mock.
Once WebSocket is stubbed to MockWebSocket, references to WebSocket.OPEN and WebSocket.CONNECTING resolve against the mock class. Since the mock does not define these static constants, the test assigns readyState = undefined at line 51 and the production code's ready-state checks (if (ws.readyState === WebSocket.OPEN)) pass inadvertently with undefined === undefined, masking connection gating failures.
Suggested fix
class MockWebSocket {
static instances: MockWebSocket[] = [];
+ static CONNECTING = 0;
+ static OPEN = 1;
+ static CLOSING = 2;
+ static CLOSED = 3;
url: string;
onopen: (() => void) | null = null;
onmessage: ((e: MessageEvent) => void) | null = null;
onclose: (() => void) | null = null;
sentMessages: unknown[] = [];
readyState = WebSocket.CONNECTING;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop/src/apps/TerminalApp.shortcut-ws.test.tsx` around lines 5 - 22, The
MockWebSocket class is missing the standard WebSocket ready-state constants so
references like WebSocket.OPEN/CONNECTING resolve to undefined and cause
readyState comparisons to be false-positively equal; add static numeric
constants (e.g. OPEN, CONNECTING, CLOSING, CLOSED) to MockWebSocket and ensure
the mock's default readyState uses MockWebSocket.CONNECTING and tests can set
instances' readyState to MockWebSocket.OPEN when simulating an open socket;
update any test setup that references WebSocket.* to instead rely on these
static fields on MockWebSocket (class name: MockWebSocket, property: readyState,
methods: send/close).
Comment on lines
+31
to
+34
| caps = user.get("capabilities") | ||
| if caps is None: | ||
| return False | ||
| return cap in caps |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how "capabilities" is written/read across the repo so the accepted
# container types can be finalized without breaking persisted user records.
rg -n -C2 '\bcapabilities\b' --type=pyRepository: jaylfc/tinyagentos
Length of output: 50374
🏁 Script executed:
# First, let's see the full implementation of the function in question
head -50 tinyagentos/shortcuts/capabilities.pyRepository: jaylfc/tinyagentos
Length of output: 1073
🏁 Script executed:
# Search for usages of user_has_capability function
rg -n 'user_has_capability' --type=py -B2 -A2Repository: jaylfc/tinyagentos
Length of output: 2975
🏁 Script executed:
# Search for where user dicts are created with capabilities field (auth context)
rg -n '\.get\("capabilities"\)' --type=py -B3 -A3Repository: jaylfc/tinyagentos
Length of output: 1495
🏁 Script executed:
# Search for where user dict is created or capabilities is assigned
rg -n 'user.*=.*capabilities' --type=py -B2 -A2Repository: jaylfc/tinyagentos
Length of output: 763
🏁 Script executed:
# Search for all places where capabilities key is set in user/auth context
rg -n '\["capabilities"\]\s*=' --type=py -B2 -A2Repository: jaylfc/tinyagentos
Length of output: 726
🏁 Script executed:
# Check the auth module more thoroughly to see all user creation paths
wc -l tinyagentos/auth.py && sed -n '150,170p' tinyagentos/auth.pyRepository: jaylfc/tinyagentos
Length of output: 1035
Add type validation before the membership check.
The function's docstring claims capabilities should be a set or frozenset, but the production auth system (auth.py:157) stores and returns them as lists. More importantly, without type validation, a malformed type like a string could produce incorrect results (e.g., "c" in "chat" would incorrectly match on substring rather than capability membership), or raise a TypeError if the type doesn't support the in operator.
Validate that caps is one of the types actually used in the codebase:
Suggested fix
def user_has_capability(user: dict[str, Any], cap: str) -> bool:
caps = user.get("capabilities")
- if caps is None:
+ if not isinstance(caps, (list, set)):
return False
return cap in capsAdditionally, update the docstring to reflect that capabilities are stored as lists in the auth system, not sets/frozensets.
📝 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.
Suggested change
| caps = user.get("capabilities") | |
| if caps is None: | |
| return False | |
| return cap in caps | |
| caps = user.get("capabilities") | |
| if not isinstance(caps, (list, set)): | |
| return False | |
| return cap in caps |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/shortcuts/capabilities.py` around lines 31 - 34, The membership
check uses caps = user.get("capabilities") without validating its type, which
can lead to false positives for strings or TypeError for unsupported types;
update the function (the block using user.get("capabilities"), variable caps and
parameter cap) to first verify caps is one of the expected container types used
in the codebase (e.g., list, set, frozenset, or tuple) and return False if not,
then perform the membership test; also update the function docstring to state
that capabilities are stored as lists by the auth system (and accepted container
types) rather than only set/frozenset.
#274 - terminal shortcut socket: validate ticket handshake frame (defense-in-depth) - JtiTracker: atomic record_if_new() to close replay race - worker_registry: fail closed when no active manager (no fallback signing key) - token_source: printenv instead of sh -c for env var fetch (shell injection) - dashboard proxy: forward captured subpath in upstream URL - dashboard ws proxy: handle text and binary frames - tickets: reject signing keys shorter than 32 bytes - token_source: handle FileNotFoundError + TimeoutExpired around subprocess.run - tests/conftest: rewire patch_worker_signing_key for fail-closed worker_registry
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| `/api/agents/${encodeURIComponent(agentId)}/shortcuts/${shortcut.idx}/launch`, | ||
| { method: "POST", headers: { Accept: "application/json" } } | ||
| ); | ||
| if (!res.ok) return; |
There was a problem hiding this comment.
WARNING: Missing error handling for failed shortcut launch requests. If the fetch fails, users receive no feedback and the operation silently fails.
| const kind = shortcut.kind; | ||
| if (kind === "dashboard") { | ||
| const app = getApp("browser"); | ||
| if (app) openWindow("browser", app.defaultSize, { initialUrl: redirect_url }); |
There was a problem hiding this comment.
CRITICAL: No validation of redirect_url before passing as initialUrl to BrowserApp. Potential security risk if backend returns a malicious URL, leading to XSS in iframe.
| const app = getApp("browser"); | ||
| if (app) openWindow("browser", app.defaultSize, { initialUrl: redirect_url }); | ||
| } else if (kind === "tui" || kind === "container-terminal") { | ||
| const url = new URL(redirect_url, window.location.href); |
There was a problem hiding this comment.
WARNING: new URL() may throw if redirect_url is not a valid URL. Exception not caught, could crash the component.
| if (app) openWindow("browser", app.defaultSize, { initialUrl: redirect_url }); | ||
| } else if (kind === "tui" || kind === "container-terminal") { | ||
| const url = new URL(redirect_url, window.location.href); | ||
| const ticket = url.searchParams.get("t") ?? redirect_url; |
There was a problem hiding this comment.
WARNING: Fallback to redirect_url as ticket if no 't' param is present. This is likely incorrect and could pass invalid data to the WebSocket.
| const url = new URL(redirect_url, window.location.href); | ||
| const ticket = url.searchParams.get("t") ?? redirect_url; | ||
| const app = getApp("terminal"); | ||
| if (app) openWindow("terminal", app.defaultSize, { shortcut: { wsUrl: redirect_url, ticket } }); |
There was a problem hiding this comment.
CRITICAL: Passing HTTP redirect_url as wsUrl to WebSocket. WebSocket constructor requires WS or WSS scheme; this will fail to connect.
| if (!initialUrl || initialUrlApplied.current) return; | ||
| initialUrlApplied.current = true; | ||
| if (iframeRef.current) { | ||
| iframeRef.current.src = initialUrl; |
There was a problem hiding this comment.
CRITICAL: Setting iframe.src directly without validating initialUrl. Could load malicious content if URL is not sanitized.
| const wsUrl = shortcut.wsUrl; | ||
| const ticket = shortcut.ticket; | ||
|
|
||
| const ws = new WebSocket(wsUrl); |
There was a problem hiding this comment.
WARNING: WebSocket constructor called with unvalidated wsUrl. If not a valid WS URL, constructor may throw or fail silently.
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Resolved Issues
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (17 files)
Incremental update: Reviewed 10 additional changed files since previous review. 3 critical issues resolved; no new issues found. Fix these issues in Kilo Cloud Reviewed by grok-code-fast-1:optimized:free · 276,782 tokens |
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
tinyagentos/routes/shortcut_proxy.py (1)
33-58:⚠️ Potential issue | 🟠 Major
taos_shortcutsessions are still process-local.The cookie only carries
session_id; the authorization state stays in_sessions. After/redeem, any follow-up request that lands on a different worker/process will 401 even though the cookie is valid, which makes the shortcut flow unreliable behind load balancing or multiple workers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/routes/shortcut_proxy.py` around lines 33 - 58, The session state is kept in-process via the _sessions dict so session_id cookies won't work across workers; replace the in-memory store with a networked/session-backed store (e.g., Redis or the application's shared DB) and update _new_session and _get_session (and respect _SESSION_IDLE_TTL) to read/write TTLed entries from that shared store instead of _sessions; ensure _new_session writes a key for session_id with fields agent_id, shortcut_idx, scope and an expiry, and _get_session fetches that key, refreshes its TTL on access, deletes it on expiry, and raises the same HTTPException(401) semantics when missing or expired.
🤖 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/cluster/worker_registry.py`:
- Around line 35-42: The current check only verifies
_active_manager.get_worker("local") exists but doesn't ensure the returned
worker has required fields; update the local-worker retrieval logic to also
validate that worker.signing_key and worker.worker_url are present and
non-empty, and if either is missing raise a RuntimeError with the same
fail-closed semantics (e.g., "Local worker missing signing_key or worker_url");
locate the code around _active_manager.get_worker("local") and add explicit
checks for worker.signing_key and worker.worker_url before returning the dict so
callers never receive incomplete data.
In `@tinyagentos/routes/shortcut_proxy.py`:
- Around line 392-416: The PTY is being created before validating the initial
ticket handshake (function _get_container_pty and variable pty_handle), so move
the PTY spawn until after the handshake is verified: first call await
asyncio.wait_for(websocket.receive_text(), timeout=10.0) and validate the
JSON/frame ("type" == "ticket" and non-empty "ticket"); only if valid then call
await asyncio.to_thread(_get_container_pty, agent_name, cmd) to create
pty_handle. Preserve the existing websocket.close codes/reasons on
malformed/timeout cases and ensure that if you must create the PTY earlier for
some reason you still close pty_handle via await
asyncio.to_thread(pty_handle.close) on handshake failure; otherwise avoid
spawning the PTY until after successful handshake.
- Around line 471-480: The websocket upgrade path builds upstream_ws_url and
calls _ws_lib.connect without auth; modify the websocket proxy code (around
upstream_ws_url and async with _ws_lib.connect(...)) to forward the same auth
used by the HTTP dashboard proxy by calling the existing _build_auth_header()
and passing its result into the websockets.connect call via the appropriate
headers/extra_headers parameter (so the upstream receives the
Authorization/Basic header or other auth material). Ensure you reference
_build_auth_header(), upstream_ws_url, and the async with _ws_lib.connect(...)
invocation so the WS connection includes the same auth as the HTTP proxy.
- Around line 288-301: get_local_worker() may raise RuntimeError when the worker
registry isn't ready, causing an unhandled 500; wrap the call to
get_local_worker() (and extraction of signing_key) in a try/except that catches
RuntimeError and raises an HTTPException with a 503 status (e.g., detail "worker
not ready") before proceeding to validate_ticket(); keep the existing try/except
around validate_ticket() to translate ValueError into 401/invalid messages and
reference the same symbols get_local_worker(), signing_key, validate_ticket(),
_GLOBAL_JTI_TRACKER, and HTTPException when making the change.
- Around line 153-164: The helper that fetches a shortcut currently verifies
session identity via _get_session(session_id) and agent/idx but never checks the
stored session["scope"] recorded by _new_session(), allowing terminal sessions
to be replayed against dashboard routes; update this helper to assert
session["scope"] equals the expected scope for this route (e.g., "dashboard")
before returning shortcuts[idx], and if it does not match raise an
HTTPException(status_code=403) with a clear message like "Session scope
mismatch: expected dashboard" (do this check prior to accessing shortcuts[idx]
and keep existing agent/idx validations).
In `@tinyagentos/shortcuts/tickets.py`:
- Around line 151-153: The module-level singleton _GLOBAL_JTI_TRACKER =
JtiTracker() causes per-process replay state and must be replaced with a single
distributed/shared tracker instance created at application startup; remove the
module-level instantiation and instead instantiate a distributed-backed
JtiTracker (e.g., Redis/DB-backed variant) in your app startup/initializer and
inject or attach it to the app context (or pass it into the route handlers) so
all processes share the same replay state; update code that references
_GLOBAL_JTI_TRACKER to read the tracker from the app context or DI container
(the JtiTracker symbol and any routes importing _GLOBAL_JTI_TRACKER) and ensure
configuration for the distributed backend is plumbed into the startup factory.
In `@tinyagentos/shortcuts/token_source.py`:
- Around line 13-14: The _cache dict is accessed concurrently from threads,
causing races; add a module-level threading.Lock named e.g. _cache_lock and use
it to protect all reads, writes and invalidations of _cache (not _CACHE_TTL) —
wrap the body of read_token_source(), the logic that populates/updates _cache,
and invalidate_agent_cache() (and the other cache-manipulating blocks referenced
around the diff) in with _cache_lock: so dictionary iteration, pop/clear and
assignments are atomic and cannot race with a concurrent fill. Ensure you import
threading and use the lock everywhere _cache is touched.
---
Duplicate comments:
In `@tinyagentos/routes/shortcut_proxy.py`:
- Around line 33-58: The session state is kept in-process via the _sessions dict
so session_id cookies won't work across workers; replace the in-memory store
with a networked/session-backed store (e.g., Redis or the application's shared
DB) and update _new_session and _get_session (and respect _SESSION_IDLE_TTL) to
read/write TTLed entries from that shared store instead of _sessions; ensure
_new_session writes a key for session_id with fields agent_id, shortcut_idx,
scope and an expiry, and _get_session fetches that key, refreshes its TTL on
access, deletes it on expiry, and raises the same HTTPException(401) semantics
when missing or expired.
🪄 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: f404a917-5f28-440e-bb3f-0bf86c378e8c
📒 Files selected for processing (11)
tests/cluster/test_local_worker_enroll.pytests/routes/conftest.pytests/routes/test_shortcut_proxy.pytests/routes/test_shortcut_terminal.pytests/shortcuts/test_jti_eviction.pytests/shortcuts/test_tickets.pytests/shortcuts/test_token_source.pytinyagentos/cluster/worker_registry.pytinyagentos/routes/shortcut_proxy.pytinyagentos/shortcuts/tickets.pytinyagentos/shortcuts/token_source.py
✅ Files skipped from review due to trivial changes (1)
- tests/cluster/test_local_worker_enroll.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/shortcuts/test_token_source.py
- tests/shortcuts/test_jti_eviction.py
Comment on lines
+151
to
+153
| # Module-level singleton. Routes import this for convenience so they don't | ||
| # need to manage the tracker's lifecycle. | ||
| _GLOBAL_JTI_TRACKER: JtiTracker = JtiTracker() |
There was a problem hiding this comment.
Process-local JTI tracking breaks single-use tickets in clustered deployments.
_GLOBAL_JTI_TRACKER only records replays inside one Python process. In a multi-worker or restarted deployment, the same ticket can be redeemed once per process because each instance has its own tracker, which defeats the replay guarantee this flow is relying on.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tinyagentos/shortcuts/tickets.py` around lines 151 - 153, The module-level
singleton _GLOBAL_JTI_TRACKER = JtiTracker() causes per-process replay state and
must be replaced with a single distributed/shared tracker instance created at
application startup; remove the module-level instantiation and instead
instantiate a distributed-backed JtiTracker (e.g., Redis/DB-backed variant) in
your app startup/initializer and inject or attach it to the app context (or pass
it into the route handlers) so all processes share the same replay state; update
code that references _GLOBAL_JTI_TRACKER to read the tracker from the app
context or DI container (the JtiTracker symbol and any routes importing
_GLOBAL_JTI_TRACKER) and ensure configuration for the distributed backend is
plumbed into the startup factory.
- auth: legacy users without capabilities default to admin (primary/is_admin)
or new-user (chat only) caps so upgrades don't silently break access
- BrowserApp: keep initialUrl in React state (url/inputValue/history) so
address bar, copy, back/forward all reflect the launch URL; update test
to match actual proxy-url iframe src pattern
- frameworks: re-raise shortcut validation as FrameworkManifestError so
create_app() log-and-continue path handles bad shortcuts entries cleanly
- tests: restore global.fetch via vi.stubGlobal/unstubAllGlobals in both
AgentsApp test suites to prevent cross-test pollution
- tests: restore _LOCAL_SIGNING_KEY in try/finally to keep suite isolation
- shortcut_proxy: document in-memory _sessions limitation under multi-worker
deploy (sticky sessions required; Redis/DB deferred to future scale need)
- e2e: fix Playwright locator — filter({has}) matches descendants, not the
element's own aria-label; switch to CSS attribute selector on .agent-shortcut-btn
- test_auth_capabilities: add two legacy-record tests (primary -> admin caps,
secondary -> chat only) covering the M6 migration path
There was a problem hiding this comment.
♻️ Duplicate comments (4)
tinyagentos/routes/shortcut_proxy.py (4)
295-300:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle worker-registry unavailability explicitly in
/redeem.Line 295 can raise
RuntimeErrorbefore your currenttry/except, causing an unhandled 500 instead of a controlled 503.Suggested fix
- worker = get_local_worker() - signing_key: bytes = worker["signing_key"] - try: + worker = get_local_worker() + signing_key: bytes = worker["signing_key"] ticket = validate_ticket(t, signing_key=signing_key, tracker=_GLOBAL_JTI_TRACKER) + except RuntimeError as exc: + raise HTTPException(status_code=503, detail="shortcut worker unavailable") from exc except ValueError as exc: msg = str(exc)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/routes/shortcut_proxy.py` around lines 295 - 300, The call to get_local_worker() before validate_ticket() can raise RuntimeError and is not covered by the existing try/except, so update the /redeem flow to explicitly handle worker-registry unavailability: wrap the get_local_worker() call in its own try/except catching RuntimeError (from get_local_worker) and convert it into a controlled 503 response (or raise an HTTP 503) with a clear message before proceeding to calling validate_ticket(..., signing_key=signing_key, tracker=_GLOBAL_JTI_TRACKER); keep the existing validate_ticket ValueError handling separate so registry errors and ticket-validation errors map to 503 and 400/401 respectively.
143-171:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce session scope when resolving shortcut cookies.
Line 160 validates session existence, but Lines 162-166 only check agent/index. A session minted for one scope can be replayed against another route for the same index.
Suggested fix
def _get_shortcut_from_cookie( connection: Any, agent_name: str, idx: int, shortcuts: list[dict[str, Any]], + expected_scopes: set[str] | None = None, ) -> dict[str, Any]: @@ if session["shortcut_idx"] != idx: raise HTTPException(status_code=403, detail="Session shortcut index mismatch") + if expected_scopes is not None and session.get("scope") not in expected_scopes: + raise HTTPException(status_code=403, detail="Session scope mismatch") @@ - shortcut = _get_shortcut_from_cookie(request, agent_name, idx, shortcuts) + shortcut = _get_shortcut_from_cookie( + request, agent_name, idx, shortcuts, expected_scopes={"dashboard"} + ) @@ - shortcut = _get_shortcut_from_cookie(websocket, agent_name, idx, shortcuts) + shortcut = _get_shortcut_from_cookie( + websocket, + agent_name, + idx, + shortcuts, + expected_scopes={"container-terminal", "tui"}, + )Also applies to: 346-347, 381-381
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/routes/shortcut_proxy.py` around lines 143 - 171, The session returned by _get_session must also be checked for its scope to prevent replay across routes: update _get_shortcut_from_cookie to accept the expected scope (e.g., add a parameter like expected_scope) and after retrieving session = _get_session(session_id) validate session.get("scope") == expected_scope and raise HTTPException(403, detail="Session scope mismatch") if not; keep the existing agent/index checks and range check, and then update all callers of _get_shortcut_from_cookie (the sites around the other occurrences referenced) to pass the correct scope derived from the current route/context.
399-423:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDefer PTY creation until after handshake validation.
Line 400 allocates container PTY resources before validating the first frame (Lines 414-417). Invalid/timeout handshakes can still force shell allocation.
Suggested fix
- try: - pty_handle = await asyncio.to_thread(_get_container_pty, agent_name, cmd) - except Exception as exc: - logger.warning("terminal: spawn_pty failed for %s: %s", agent_name, exc) - await websocket.close(code=1011, reason="PTY spawn failed") - return - await websocket.accept() @@ except (json.JSONDecodeError, KeyError, asyncio.TimeoutError) as exc: await websocket.close(code=1008, reason=f"invalid handshake: {exc}") - await asyncio.to_thread(pty_handle.close) return + + try: + pty_handle = await asyncio.to_thread(_get_container_pty, agent_name, cmd) + except Exception as exc: + logger.warning("terminal: spawn_pty failed for %s: %s", agent_name, exc) + await websocket.close(code=1011, reason="PTY spawn failed") + return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/routes/shortcut_proxy.py` around lines 399 - 423, The code currently calls _get_container_pty (creating pty_handle) before validating the initial websocket handshake, so a malformed or timed-out handshake can still allocate PTY resources; change the flow to first await and validate websocket.receive_text() / json.loads(handshake) (the same checks around handshake.get("type") and "ticket") and only after the handshake is accepted call asyncio.to_thread(_get_container_pty, agent_name, cmd) to create pty_handle; ensure errors from _get_container_pty are caught and logged (logger.warning like "terminal: spawn_pty failed for %s: %s"), that the websocket is closed with code 1011 on PTY creation failure, and that pty_handle.close is invoked where appropriate when any later error occurs.
485-488:⚠️ Potential issue | 🟠 Major | ⚡ Quick winForward shortcut auth to upstream dashboard WebSocket connections.
HTTP proxy injects auth (Line 243), but WS proxy does not (Line 487). Dashboards that require auth on WS upgrades will fail after initial page load.
Please verify the installed
websocketsversion and pass_build_auth_header(...)into the upstream connect call using that version’s supported header argument.#!/bin/bash set -euo pipefail # 1) Identify websocket dependency/version declarations fd -HI 'pyproject.toml|requirements*.txt|poetry.lock|uv.lock|Pipfile|Pipfile.lock' \ | xargs -r rg -n "websockets" # 2) Confirm current code path lacks auth forwarding in WS proxy rg -n "_build_auth_header|_ws_lib\.connect\(" tinyagentos/routes/shortcut_proxy.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/routes/shortcut_proxy.py` around lines 485 - 488, The WebSocket proxy opens the upstream connection with _ws_lib.connect(upstream_ws_url) but does not forward auth like the HTTP proxy, so update the connect call in shortcut_proxy.py to pass the auth header built by _build_auth_header(...) (e.g., via the connect parameter extra_headers or headers depending on the installed websockets version) when calling _ws_lib.connect(upstream_ws_url, ...); ensure you import or reference _build_auth_header and pass its result as the appropriate header argument to _ws_lib.connect so upstream dashboard WS upgrades receive the same auth as HTTP.
🧹 Nitpick comments (1)
desktop/tests/agent-shortcuts.spec.ts (1)
52-53: Replace fixed sleeps with condition-based waits for more stable E2E runsAll four instances of
waitForTimeout(1500)(lines 52, 69, 96, 126) rely on fixed delays for test synchronization, which Playwright documentation advises against. Hard waits are brittle on slow CI and unnecessarily slow on fast runs. Use condition-based waits instead:
- Create a helper function that waits for actual UI elements to settle (e.g., agent cards, shortcut rows, buttons) rather than sleeping for a fixed duration.
- Apply it at all four locations.
Suggested refactor
+async function waitForAgentsUiSettled(page: Page): Promise<void> { + // Keep skip-tolerant behavior while avoiding fixed sleeps. + await page + .locator(".agent-card, .agent-shortcut-row, .agent-shortcut-btn") + .first() + .waitFor({ state: "visible", timeout: 8000 }) + .catch(() => {}); +} + test.describe("Agent shortcuts `@iphone-14`", () => { test("shortcut row appears on agent card when shortcuts available", async ({ page }) => { @@ - await page.waitForTimeout(1500); + await waitForAgentsUiSettled(page); @@ - await page.waitForTimeout(1500); + await waitForAgentsUiSettled(page); @@ - await page.waitForTimeout(1500); + await waitForAgentsUiSettled(page); @@ - await page.waitForTimeout(1500); + await waitForAgentsUiSettled(page);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@desktop/tests/agent-shortcuts.spec.ts` around lines 52 - 53, The tests use fixed sleeps await page.waitForTimeout(1500) (in agent-shortcuts.spec.ts) which are brittle; replace each occurrence with a condition-based wait by adding a helper (e.g., waitForAgentCardsToSettle, waitForShortcutRowsToAppear) that awaits specific UI states (locator.waitFor({ state: 'visible' }) or locator.count() changes, or page.waitForResponse() when relevant) and call that helper in place of the four waitForTimeout calls so the test waits for agent cards, shortcut rows or buttons to be ready instead of sleeping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tinyagentos/routes/shortcut_proxy.py`:
- Around line 295-300: The call to get_local_worker() before validate_ticket()
can raise RuntimeError and is not covered by the existing try/except, so update
the /redeem flow to explicitly handle worker-registry unavailability: wrap the
get_local_worker() call in its own try/except catching RuntimeError (from
get_local_worker) and convert it into a controlled 503 response (or raise an
HTTP 503) with a clear message before proceeding to calling validate_ticket(...,
signing_key=signing_key, tracker=_GLOBAL_JTI_TRACKER); keep the existing
validate_ticket ValueError handling separate so registry errors and
ticket-validation errors map to 503 and 400/401 respectively.
- Around line 143-171: The session returned by _get_session must also be checked
for its scope to prevent replay across routes: update _get_shortcut_from_cookie
to accept the expected scope (e.g., add a parameter like expected_scope) and
after retrieving session = _get_session(session_id) validate
session.get("scope") == expected_scope and raise HTTPException(403,
detail="Session scope mismatch") if not; keep the existing agent/index checks
and range check, and then update all callers of _get_shortcut_from_cookie (the
sites around the other occurrences referenced) to pass the correct scope derived
from the current route/context.
- Around line 399-423: The code currently calls _get_container_pty (creating
pty_handle) before validating the initial websocket handshake, so a malformed or
timed-out handshake can still allocate PTY resources; change the flow to first
await and validate websocket.receive_text() / json.loads(handshake) (the same
checks around handshake.get("type") and "ticket") and only after the handshake
is accepted call asyncio.to_thread(_get_container_pty, agent_name, cmd) to
create pty_handle; ensure errors from _get_container_pty are caught and logged
(logger.warning like "terminal: spawn_pty failed for %s: %s"), that the
websocket is closed with code 1011 on PTY creation failure, and that
pty_handle.close is invoked where appropriate when any later error occurs.
- Around line 485-488: The WebSocket proxy opens the upstream connection with
_ws_lib.connect(upstream_ws_url) but does not forward auth like the HTTP proxy,
so update the connect call in shortcut_proxy.py to pass the auth header built by
_build_auth_header(...) (e.g., via the connect parameter extra_headers or
headers depending on the installed websockets version) when calling
_ws_lib.connect(upstream_ws_url, ...); ensure you import or reference
_build_auth_header and pass its result as the appropriate header argument to
_ws_lib.connect so upstream dashboard WS upgrades receive the same auth as HTTP.
---
Nitpick comments:
In `@desktop/tests/agent-shortcuts.spec.ts`:
- Around line 52-53: The tests use fixed sleeps await page.waitForTimeout(1500)
(in agent-shortcuts.spec.ts) which are brittle; replace each occurrence with a
condition-based wait by adding a helper (e.g., waitForAgentCardsToSettle,
waitForShortcutRowsToAppear) that awaits specific UI states (locator.waitFor({
state: 'visible' }) or locator.count() changes, or page.waitForResponse() when
relevant) and call that helper in place of the four waitForTimeout calls so the
test waits for agent cards, shortcut rows or buttons to be ready instead of
sleeping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f96c220c-3051-4a7d-ba49-d0dbf4b3d961
📒 Files selected for processing (10)
desktop/src/apps/BrowserApp.initialUrl.test.tsxdesktop/src/apps/BrowserApp.tsxdesktop/src/apps/__tests__/AgentsApp.shortcut-click.test.tsxdesktop/src/apps/__tests__/AgentsApp.test.tsxdesktop/tests/agent-shortcuts.spec.tstests/cluster/test_local_worker_enroll.pytests/shortcuts/test_auth_capabilities.pytinyagentos/auth.pytinyagentos/frameworks.pytinyagentos/routes/shortcut_proxy.py
✅ Files skipped from review due to trivial changes (1)
- desktop/src/apps/BrowserApp.initialUrl.test.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/shortcuts/test_auth_capabilities.py
- desktop/src/apps/tests/AgentsApp.shortcut-click.test.tsx
- tests/cluster/test_local_worker_enroll.py
- tinyagentos/auth.py
- desktop/src/apps/BrowserApp.tsx
- AgentsApp click handler: http→ws / https→wss scheme conversion for WS shortcut URLs; extract 't' query param (was looking at wrong param name) - BrowserApp: validate initialUrl scheme (http/https only) before setting iframe.src — close XSS surface - token_source: add _cache_lock (threading.Lock) around all cache reads, writes, and invalidations for thread safety under asyncio.to_thread - shortcut_proxy terminal: validate ticket handshake BEFORE PTY spawn to prevent resource leak on bad handshake - shortcut_proxy dashboard WS: forward shortcut auth header to upstream via additional_headers - shortcut_proxy: _get_shortcut_from_cookie gains expected_scope param; dashboard route uses "dashboard", terminal route uses "terminal" to prevent cross-route session replay - shortcut_proxy /redeem: catch worker_registry RuntimeError → 503 (was unhandled 500) - worker_registry: fail closed on empty/short signing_key (<32 bytes) or empty worker_url - tickets: module docstring documenting in-process JTI limitation for cluster deploys
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tinyagentos/shortcuts/token_source.py (1)
11-11: 💤 Low valueEnv var regex rejects valid lowercase names.
The
_ENV_VAR_REpattern^[A-Z_][A-Z0-9_]*$only accepts uppercase environment variable names. This rejects valid POSIX env vars containing lowercase letters (e.g.,api_token,mySecret). If lowercase env vars are intentionally unsupported, consider documenting this; otherwise, relax the pattern.🔧 Suggested fix to accept lowercase
-_ENV_VAR_RE = re.compile(r"^[A-Z_][A-Z0-9_]*$") +_ENV_VAR_RE = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tinyagentos/shortcuts/token_source.py` at line 11, The regex _ENV_VAR_RE in token_source.py currently enforces only uppercase names; update it to accept lowercase by changing the pattern to allow letters of either case (e.g., use [A-Za-z_][A-Za-z0-9_]*), and ensure any validation logic that uses _ENV_VAR_RE (in the same module) continues to operate with the relaxed rule and update related tests or docs if they assumed uppercase-only names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@desktop/src/apps/BrowserApp.tsx`:
- Around line 190-192: The assignment iframeRef.current.src = safe bypasses the
app's proxy routing; change the imperative iframe update in the BrowserApp
component to use the proxied URL (call proxyUrl(...) on the same source used
elsewhere) so the iframe navigation goes through proxy routing—e.g., compute
proxied = proxyUrl(safe or the original url) and assign iframeRef.current.src =
proxied; update the same logic paths that rely on proxyUrl(url) to keep behavior
consistent.
---
Nitpick comments:
In `@tinyagentos/shortcuts/token_source.py`:
- Line 11: The regex _ENV_VAR_RE in token_source.py currently enforces only
uppercase names; update it to accept lowercase by changing the pattern to allow
letters of either case (e.g., use [A-Za-z_][A-Za-z0-9_]*), and ensure any
validation logic that uses _ENV_VAR_RE (in the same module) continues to operate
with the relaxed rule and update related tests or docs if they assumed
uppercase-only names.
🪄 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: 6ef666e3-66ae-422d-b24b-8e41c7f6eba0
📒 Files selected for processing (10)
desktop/src/apps/AgentsApp.tsxdesktop/src/apps/BrowserApp.initialUrl.test.tsxdesktop/src/apps/BrowserApp.tsxdesktop/src/apps/__tests__/AgentsApp.shortcut-click.test.tsxtests/routes/test_shortcut_proxy.pytests/shortcuts/test_token_source_thread_safety.pytinyagentos/cluster/worker_registry.pytinyagentos/routes/shortcut_proxy.pytinyagentos/shortcuts/tickets.pytinyagentos/shortcuts/token_source.py
🚧 Files skipped from review as they are similar to previous changes (4)
- desktop/src/apps/BrowserApp.initialUrl.test.tsx
- desktop/src/apps/tests/AgentsApp.shortcut-click.test.tsx
- desktop/src/apps/AgentsApp.tsx
- tinyagentos/shortcuts/tickets.py
Comment on lines
+190
to
+192
| if (iframeRef.current) { | ||
| iframeRef.current.src = safe; | ||
| } |
There was a problem hiding this comment.
Use the proxied URL for the imperative iframe assignment.
At Line 191, iframeRef.current.src = safe bypasses the proxy path used everywhere else (proxyUrl(url)) and can trigger a direct request before the rerender. Keep this consistent with proxy routing.
Suggested fix
if (iframeRef.current) {
- iframeRef.current.src = safe;
+ iframeRef.current.src = proxyUrl(safe);
}📝 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.
Suggested change
| if (iframeRef.current) { | |
| iframeRef.current.src = safe; | |
| } | |
| if (iframeRef.current) { | |
| iframeRef.current.src = proxyUrl(safe); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop/src/apps/BrowserApp.tsx` around lines 190 - 192, The assignment
iframeRef.current.src = safe bypasses the app's proxy routing; change the
imperative iframe update in the BrowserApp component to use the proxied URL
(call proxyUrl(...) on the same source used elsewhere) so the iframe navigation
goes through proxy routing—e.g., compute proxied = proxyUrl(safe or the original
url) and assign iframeRef.current.src = proxied; update the same logic paths
that rely on proxyUrl(url) to keep behavior consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@desktop/src/apps/BrowserApp.tsx`:
- Around line 173-202: The initialUrl direct assignment is being overwritten on
the next render because JSX uses src={proxyUrl(url)}; add a dedicated "direct
launch" flag (e.g. directLaunchRef or directLaunch state) and set it to true
when you set iframeRef.current.src = safe inside the useEffect (and set
initialUrlApplied.current = true), then change the iframe JSX binding so it uses
iframe.src = safe when directLaunch is true (otherwise src={proxyUrl(url)}), and
update refresh() (and any navigation handlers) to respect/clear that
directLaunch flag (clear it when the user navigates or calls refresh) so the
proxy path is gated until explicit navigation.
🪄 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: 498deff1-ea73-420a-bfbc-1447651bf797
📒 Files selected for processing (1)
desktop/src/apps/BrowserApp.tsx
Comment on lines
+173
to
+202
| // Apply initialUrl once on mount — idempotent across rerenders. | ||
| // Also sync url/inputValue/history so the address bar, copy, and | ||
| // back/forward all reflect the launch URL rather than DEFAULT_URL. | ||
| useEffect(() => { | ||
| if (!initialUrl || initialUrlApplied.current) return; | ||
| try { | ||
| const parsed = new URL(initialUrl, window.location.href); | ||
| if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { | ||
| console.warn("[BrowserApp] rejected initialUrl with non-http(s) scheme:", parsed.protocol); | ||
| return; | ||
| } | ||
| initialUrlApplied.current = true; | ||
| const safe = parsed.toString(); | ||
| setUrl(safe); | ||
| setInputValue(safe); | ||
| setHistory([safe]); | ||
| setHistoryIndex(0); | ||
| if (iframeRef.current) { | ||
| // initialUrl is set directly on the iframe (NOT via proxyUrl) because the | ||
| // agent-shortcuts feature redirects the browser straight to the worker — the | ||
| // worker sets its own session cookie via /redeem and serves dashboard | ||
| // content directly. Routing through the controller's proxyUrl wrapping would | ||
| // break that worker-direct cookie flow. | ||
| iframeRef.current.src = safe; | ||
| } | ||
| } catch (err) { | ||
| console.warn("[BrowserApp] invalid initialUrl:", initialUrl, err); | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l desktop/src/apps/BrowserApp.tsxRepository: jaylfc/tinyagentos
Length of output: 98
🏁 Script executed:
cat -n desktop/src/apps/BrowserApp.tsx | head -250Repository: jaylfc/tinyagentos
Length of output: 9588
🏁 Script executed:
cat -n desktop/src/apps/BrowserApp.tsx | sed -n '234,350p'Repository: jaylfc/tinyagentos
Length of output: 4715
🏁 Script executed:
cat -n desktop/src/apps/BrowserApp.tsx | sed -n '350,502p'Repository: jaylfc/tinyagentos
Length of output: 6754
Don't let the shortcut launch fall back to the proxy on rerender.
The useEffect sets iframeRef.current.src = safe for direct worker access, but then calls setUrl(safe), triggering a re-render. On re-render, the JSX binding src={proxyUrl(url)} overwrites the direct assignment, routing the iframe through the proxy and breaking the /redeem cookie handoff for agent shortcuts. Similarly, the refresh() function at line 114 also proxies the URL. Keep a dedicated direct-launch state or gate the proxy path until user navigation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@desktop/src/apps/BrowserApp.tsx` around lines 173 - 202, The initialUrl
direct assignment is being overwritten on the next render because JSX uses
src={proxyUrl(url)}; add a dedicated "direct launch" flag (e.g. directLaunchRef
or directLaunch state) and set it to true when you set iframeRef.current.src =
safe inside the useEffect (and set initialUrlApplied.current = true), then
change the iframe JSX binding so it uses iframe.src = safe when directLaunch is
true (otherwise src={proxyUrl(url)}), and update refresh() (and any navigation
handlers) to respect/clear that directLaunch flag (clear it when the user
navigates or calls refresh) so the proxy path is gated until explicit
navigation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Per-framework agent shortcut buttons in AgentsApp — three kinds (
container-terminal,tui,dashboard) declared in the framework registry, capability-filtered, with HMAC-ticket-based auth that's cluster + Tailscale-ready.tinyagentos/shortcuts/types.py,validation.py), capability model on user record (auth.py+shortcuts/capabilities.py), HMAC ticket mint/validate (shortcuts/tickets.py) with JTI replay tracker, container PTY abstraction (containers/backend.pyABC + LXC impl + Docker/Apple stubs), token-source reader with 60s LRU cache (shortcuts/token_source.py), worker registration extension (cluster/worker_protocol.py+local_worker.py), dashboard reverse proxy with auth injection + SSE/WebSocket (routes/shortcut_proxy.py), terminal websocket PTY bridge, and 6 beta frameworks populated with shortcut definitions.useAgentShortcutshook,AgentShortcutRowcomponent (mobile icon-only viauseIsMobile), AgentsApp wiring with click-handler routing by kind, TerminalAppshortcutprop (WS + ticket frame + UI suppression), BrowserAppinitialUrlprop.desktop/tests/agent-shortcuts.spec.tson iphone-14 profile, skip-tolerant for no-backend / no-seed-data runs.Built via brainstorm → spec → plan → subagent-driven implementation. 33 tasks across 16 phases. Spec at `docs/superpowers/specs/2026-04-30-agent-shortcuts-design.md` and plan at `docs/superpowers/plans/2026-04-30-agent-shortcuts-plan.md` (both gitignored, local-only).
Notable deviations from plan
rfind(b".")separator bugs in `validate_ticket` (HMAC signatures contain `0x2e`)Test plan
Open follow-ups (non-blocking)
Summary by CodeRabbit
New Features
Permissions