Skip to content

Chat Phase 2a — desktop admin UI + live signal (settings panel, context menu, slash menu, typing/thinking)#235

Merged
jaylfc merged 21 commits intomasterfrom
feat/chat-phase-2a-desktop-admin
Apr 19, 2026
Merged

Chat Phase 2a — desktop admin UI + live signal (settings panel, context menu, slash menu, typing/thinking)#235
jaylfc merged 21 commits intomasterfrom
feat/chat-phase-2a-desktop-admin

Conversation

@jaylfc
Copy link
Copy Markdown
Owner

@jaylfc jaylfc commented Apr 19, 2026

Summary

Completes Phase 1's control plane with a desktop UI:

  • Channel Settings slide-over (right-side, 360px, 4 sections): rename, topic, members, response mode, muted, hops + cooldown sliders — all wired to Phase 1's REST admin endpoints
  • Agent context menu (right-click any agent surface): DM @agent, (Un)mute, Remove, View info, Jump to settings
  • Slash autocomplete menu (composer /): fuzzy-filter per-agent commands, grouped by agent in groups, flat in DMs; picking inserts @<slug> /<cmd> satisfying the Phase 1 bare-slash guardrail
  • Typing / thinking footer: humans-typing indicator (composer keystroke → debounced POST /typing); agent-thinking indicator (bridge heartbeats POST /thinking around its LLM call)
  • Agent info popover (via context menu): framework, model, status
  • Bare-slash 400 surfacing: bare-slash messages in groups get inline error banner

Spec + plan

Backend

  • tinyagentos/chat/typing_registry.py — in-memory per-channel heartbeat tracker (human TTL 3s, agent TTL 45s)
  • tinyagentos/routes/chat.pyPOST /typing, POST /thinking (bearer-auth), GET /typing; broadcasts via chat_hub
  • tinyagentos/routes/framework.pyGET /api/frameworks/slash-commands returns per-agent command manifest
  • tinyagentos/frameworks.py — new optional slash_commands: [{name, description}] field on the 6 beta frameworks
  • tinyagentos/scripts/install_*.sh — all 6 bridges emit a best-effort thinking heartbeat around the LLM call

Frontend

  • desktop/src/apps/chat/ChannelSettingsPanel.tsx — slide-over
  • desktop/src/apps/chat/AgentContextMenu.tsx — right-click hub
  • desktop/src/apps/chat/SlashMenu.tsx — grouped fuzzy autocomplete
  • desktop/src/apps/chat/TypingFooter.tsx — humans + agents strip
  • desktop/src/lib/channel-admin-api.ts — REST client for Phase 1 admin endpoints
  • desktop/src/lib/use-typing-emitter.ts — debounced typing POST hook
  • desktop/src/apps/MessagesApp.tsx — integrates all four components + WS handlers + settings-fetch + agent-info popover + 400 surfacing

Test plan

  • Backend: TypingRegistry 9 unit tests + 4 route tests, slash-commands endpoint 2 tests, manifest-shape 2 tests — all pass
  • Frontend: component/helper tests for all 6 new units (vitest) — all pass
  • npm run build clean (tsc + vite)
  • Full frontend suite: 216 pass / 3 pre-existing snap-zones failures (unrelated)
  • Full backend suite: no new failures
  • E2E against live Pi: TAOS_E2E_URL=… pytest tests/e2e/test_chat_phase2a.py — env-gated; selectors may need tuning on first live run

Known / deferred

  • Phase 2b (next): threads, pins, read receipts, attachments, ephemeral messages. Not optional — the chat isn't "done" without them.
  • Mobile chat polish (separate phase): aims for Slack/Discord-level mobile UX. Phase 2a is desktop-only.
  • Per-framework typing phases (Phase 3 / task Cluster worker heartbeat → live BackendCatalog reporting #41): tom is calling search, tom is writing, etc. Phase 2a just says "is thinking".
  • Slash-commands discovery via /capabilities (future): the static manifest in frameworks.py is the MVP; a bridge-side capabilities endpoint is a later iteration.

Final review items addressed

  • Channel interface gained topic?: string; ChannelSettingsPanel now reads the channel's actual topic instead of hardcoded empty string (prevented topic data-loss on first blur).
  • LiveAgent interface extended with model?, status? to match /api/agents payload; agentInfoPopover no longer shows "unknown" for those fields.

Summary by CodeRabbit

  • New Features
    • Real-time human “typing” and agent “thinking” indicators (with agent heartbeats)
    • Slash-command autocomplete with keyboard navigation and validation/error feedback
    • Agent context menu (right-click): DM, mute/unmute, view info, jump to settings
    • Channel settings panel: name/topic, response mode, member & mute management
  • Tests
    • Extensive unit, integration, and end-to-end tests added for chat, typing, and slash commands
  • Documentation
    • Added implementation plan for chat Phase 2a
  • Chores
    • Test environment and test libs configured for browser-like JS testing; rebuilt desktop assets updated

jaylfc added 19 commits April 19, 2026 16:03
…gnal

15 bite-sized TDD tasks covering the 4 UI features + backend enablers:
slash_commands manifest, TypingRegistry, 3 ephemeral endpoints,
slash-commands route, thinking heartbeat in all 6 bridges, the 6 new
desktop components/hooks, integration into MessagesApp, and Playwright
coverage.
tsc -b was erroring on vitest test files because jest-dom matchers
(.toBeInTheDocument) aren't registered in the main tsconfig. Test files
are only ever run through vitest; they don't need to compile via the
production tsc pass. Excluding them keeps `npm run build` clean without
touching the vitest runner's own config.

Also removes unused React imports in ChannelSettingsPanel, SlashMenu,
and TypingFooter (not needed with react-jsx transform), and drops the
unused index variable in SlashMenu's rows.map call.
MessagesApp grew from 40k to ~50k (new slash menu, settings panel,
context menu, typing footer). Other bundles refresh fingerprints.
- Channel interface gained a topic field; ChannelSettingsPanel now
  receives the channel's actual topic instead of a hardcoded empty
  string, so blur-saves don't overwrite existing topics.
- LiveAgent interface / popover now reads framework/model/status from
  the actual /api/agents payload (extended the interface with model and
  status fields and dropped the 'as unknown as' casts).
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 19, 2026

📝 Walkthrough

Walkthrough

Adds Phase 2a chat admin and UX: typing/thinking registry and endpoints, slash-command manifests and autocomplete, channel admin REST client and UI (settings panel, agent context menu), typing emitter and footer, MessagesApp wiring, tests, and rebuilt desktop assets.

Changes

Cohort / File(s) Summary
Test & Tooling
desktop/package.json, desktop/tsconfig.json, desktop/vite.config.ts, desktop/vitest.setup.ts
Enable jsdom+Vitest globals, add testing libs, exclude tests from tsconfig, and register jest-dom setup.
Chat Admin API (client)
desktop/src/lib/channel-admin-api.ts, desktop/src/lib/__tests__/channel-admin-api.test.ts
New channel-admin REST client (patch/add/remove/mute/unmute) with consistent JSON error handling and unit tests.
Typing Emitter (client)
desktop/src/lib/use-typing-emitter.ts, desktop/src/lib/__tests__/use-typing-emitter.test.ts
Hook that debounces POST /typing per channel/author; tests for debounce and no-op when channelId is null.
Typing Registry & Routes (server)
tinyagentos/chat/typing_registry.py, tinyagentos/routes/chat.py, tests/test_chat_typing.py, tinyagentos/app.py
In-memory TypingRegistry with TTLs; POST /typing, POST /thinking (auth), GET /typing endpoints; app wiring to initialize registry; comprehensive unit + endpoint tests.
Slash Commands Manifest & Route
tinyagentos/frameworks.py, tinyagentos/routes/framework.py, tests/test_framework_manifest.py, tests/test_framework_slash_commands.py
Framework manifests extended with slash_commands; new GET /api/frameworks/slash-commands endpoint and tests validating per-agent command maps.
New Chat UI Components
desktop/src/apps/chat/AgentContextMenu.tsx, desktop/src/apps/chat/ChannelSettingsPanel.tsx, desktop/src/apps/chat/SlashMenu.tsx, desktop/src/apps/chat/TypingFooter.tsx
Four new React components: agent context menu (actions + async ops), channel settings slide-over (patch/members/muted/advanced), keyboard-navigable slash menu with fuzzy matching, and typing footer UI.
MessagesApp Integration
desktop/src/apps/MessagesApp.tsx, static/desktop/assets/MessagesApp-*.js
MessagesApp updated to fetch slash commands, perform REST validation for slash sends, integrate useTypingEmitter, separate human/agent typing state, render new UI surfaces (ChannelSettingsPanel, AgentContextMenu, SlashMenu, TypingFooter), and handle sendError. Bundles recompiled.
Component Tests
desktop/src/apps/chat/__tests__/*
Vitest + React Testing Library suites for AgentContextMenu, ChannelSettingsPanel, SlashMenu, TypingFooter validating rendering, interactions, keyboard behavior, and async flows.
Agent Bridge Scripts
tinyagentos/scripts/install_*.sh (hermes, langroid, openai-*, pocketflow, smolagents)
Bridge scripts now emit best-effort thinking start/end POSTs to /api/.../thinking around agent execution (start before run, end in finally).
Static Assets & HTML
static/desktop/assets/*, static/desktop/chat.html, static/desktop/index.html, static/desktop/assets/tokens-*.css
Rebuilt desktop bundles referencing new MessagesApp, updated hashed imports, replaced token CSS file with updated stylesheet; HTML asset references updated accordingly.
Misc / Build Cache
desktop/tsconfig.tsbuildinfo
Updated buildinfo root file list to include new chat modules and admin API.
Docs
docs/superpowers/plans/2026-04-19-chat-phase-2a-desktop-admin.md
New Phase 2a implementation plan documentation.
Test E2E
tests/e2e/test_chat_phase2a.py
Playwright E2E tests for slash menu insertion, channel settings toggle persistence, and agent context menu via right-click.
Deployer Tests Update
tests/test_deployer.py
Mocks adjusted to include push_file and minor manifest installation test updates.

Sequence Diagram(s)

sequenceDiagram
    participant User as User (Composer)
    participant Client as MessagesApp
    participant Emitter as useTypingEmitter
    participant API as /api/chat/channels/{id}/typing
    participant Registry as TypingRegistry
    participant Hub as WebSocket Hub

    User->>Client: Key press
    Client->>Emitter: emitter()
    alt Debounced (within 1s)
        Emitter->>Emitter: no-op
    else Emit
        Emitter->>API: POST /typing {author_id}
        API->>Registry: mark(channel, author, "human")
        API->>Hub: broadcast {type:"typing", kind:"human", slug:author}
    end
    Hub->>Client: WS onmessage typing/thinking
    Client->>Client: update typingHumans/typingAgents
    Client->>Client: render <TypingFooter/>
Loading
sequenceDiagram
    participant User as User (Composer)
    participant Client as MessagesApp
    participant Cmds as /api/frameworks/slash-commands
    participant Slash as SlashMenu
    participant Validator as /api/chat/messages (validation)
    participant WS as WebSocket

    User->>Client: Type "/"
    Client->>Cmds: GET slash-commands
    Cmds-->>Client: per-agent commands
    Client->>Slash: render commands
    User->>Slash: select command
    Slash-->>Client: onPick(slug, cmd)
    Client->>Client: insert command into composer
    User->>Client: click Send
    Client->>Validator: POST /api/chat/messages (validate)
    alt 200 OK
        Client->>WS: send message via WS
    else 400
        Client-->>Client: set sendError and show alert
    end
Loading
sequenceDiagram
    participant Admin as Admin UI
    participant Panel as ChannelSettingsPanel
    participant API as /api/chat/channels/{id}
    participant DB as Channel State

    Admin->>Panel: open settings
    Panel->>Panel: load channel (name, topic, members, muted)
    Admin->>Panel: edit field (blur)
    Panel->>API: PATCH /channels/{id} {field: value}
    API->>DB: update channel
    DB-->>API: success
    API-->>Panel: 200 OK
    Panel->>Panel: onChanged()
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰
I hopped and watched the typing light,
Slash commands dancing into sight,
Agents muted, panels slide in cheer,
Admin tools and signals clear,
Desktop chat—now bright and near!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: desktop admin UI components (settings panel, context menu, slash menu) and live signal features (typing/thinking indicators).

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/chat-phase-2a-desktop-admin

Comment @coderabbitai help to get the list of available commands and usage tips.

// Phase-2a: typing/thinking events for TypingFooter
if (data.type === "typing" && data.kind === "human") {
setTypingHumans((prev) => prev.includes(data.slug) ? prev : [...prev, data.slug]);
setTimeout(() => setTypingHumans((prev) => prev.filter((s) => s !== data.slug)), 3500);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: Uncancelled setTimeout causes memory leaks and crashes. When the component unmounts or switches channels this timeout will still execute, updating state on a destroyed component and causing React warnings, memory leaks, and potentially crashes. Store timeout IDs and clean them up in useEffect return functions.

Suggested change
setTimeout(() => setTypingHumans((prev) => prev.filter((s) => s !== data.slug)), 3500);
const timeout = setTimeout(() => setTypingHumans((prev) => prev.filter((s) => s !== data.slug)), 3500);
// Store timeout ID in ref for cleanup

setTypingAgents((prev) => prev.includes(data.user_id) ? prev : [...prev, data.user_id]);
setTimeout(() => {
setTypingUsers((prev) => prev.filter((t) => t.user !== data.user_id));
setTypingAgents((prev) => prev.filter((s) => s !== data.user_id));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: Uncancelled setTimeout causes memory leaks and crashes. This timeout is never cleared when unmounting or switching channels. Store timeout IDs in a ref and clean them up.

Suggested change
setTypingAgents((prev) => prev.filter((s) => s !== data.user_id));
const timeout = setTimeout(() => {

setSendError((body as { error?: string }).error || "couldn't send message");
return;
}
} catch {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL: Race condition sends duplicate messages. On network error the catch block swallows the exception and falls through to send the same message again over WebSocket, resulting in duplicate messages being posted to the channel.

Suggested change
} catch {
} catch {
// Network error - do NOT fall through to WS send, let user retry
setSendError("network error, please try again");
return;
}

@kilo-code-bot
Copy link
Copy Markdown

kilo-code-bot bot commented Apr 19, 2026

Code Review Summary

Status: 14 Issues Found (0 Resolved) | Recommendation: Address critical issues before merge

Overview

Severity Count
CRITICAL 3
WARNING 6
SUGGESTION 6

ℹ️ Incremental Update: No changes were made to address any previous review findings. All original issues remain open. Only test file tests/test_deployer.py received minor test fix updates (no new issues found).

Issue Details (click to expand)

CRITICAL

File Line Issue Status
desktop/src/apps/MessagesApp.tsx 325 Uncancelled setTimeout memory leak - typing timers are never cleared when switching channels or unmounting Open
desktop/src/apps/MessagesApp.tsx 373 Uncancelled setTimeout memory leak - legacy typing timers are never cleared Open
desktop/src/apps/MessagesApp.tsx 549 Race condition sending duplicate messages - network errors fall through to WS send after REST attempt Open

WARNING

File Line Issue Status
desktop/src/apps/MessagesApp.tsx 505 No request cancellation for slash command fetch - slow responses overwrite active channel state Open
desktop/src/apps/MessagesApp.tsx 302 Permanent typing indicators on WebSocket disconnect - no handler to clear state on close/error Open
desktop/src/apps/chat/AgentContextMenu.tsx 121 Global keydown listener leakage - listeners not removed when menu closes Open
desktop/src/apps/chat/ChannelSettingsPanel.tsx 238 Unsafe state updates from finally blocks - updates state on unmounted components Open
desktop/src/apps/chat/SlashMenu.tsx 176 Off-screen popover rendering - no viewport boundary checks Open
desktop/src/apps/chat/SlashMenu.tsx 91 Empty members array hides all agent commands Open

SUGGESTION

File Line Issue Status
desktop/src/apps/MessagesApp.tsx 1354 Silent DM channel creation failures - no user feedback on error Open
desktop/src/apps/MessagesApp.tsx 1385 No Escape key handling for agent info popover Open
desktop/src/apps/chat/ChannelSettingsPanel.tsx 1606 Missing loading states for admin operations Open
desktop/src/apps/chat/ChannelSettingsPanel.tsx 1621 Missing loading states for admin operations Open
desktop/src/apps/chat/ChannelSettingsPanel.tsx 1650 Missing loading states for admin operations Open
desktop/src/apps/chat/ChannelSettingsPanel.tsx 1661 Missing loading states for admin operations Open
Other Observations (not in diff)

Issues found in unchanged code that cannot receive inline comments:

File Line Issue Status
desktop/src/apps/MessagesApp.tsx 469 Typing timeout uses stale closure - incorrectly removes active typers after 3500ms Open
Files Reviewed (6 files)
  • desktop/package.json - 0 issues
  • desktop/src/apps/MessagesApp.tsx - 8 issues
  • desktop/src/apps/chat/AgentContextMenu.tsx - 2 issues
  • desktop/src/apps/chat/ChannelSettingsPanel.tsx - 3 issues
  • desktop/src/apps/chat/SlashMenu.tsx - 1 issue
  • tests/test_deployer.py - 0 issues (updated, no problems found)

Fix these issues in Kilo Cloud


Reviewed by seed-2-0-pro-260328 · 186,812 tokens

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
tinyagentos/scripts/install_openai-agents-sdk.sh (1)

1-151: ⚠️ Potential issue | 🟠 Major

Remove duplicate installer script.

Both install_openai-agents-sdk.sh and install_openai_agents_sdk.sh (in tinyagentos/scripts/) are identical. Keep one canonical version and remove the other to avoid maintenance drift and confusion.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tinyagentos/scripts/install_openai-agents-sdk.sh` around lines 1 - 151, There
are two identical installer scripts (install_openai-agents-sdk.sh and
install_openai_agents_sdk.sh); delete the duplicate (keep
install_openai-agents-sdk.sh as canonical), remove the redundant file from the
repo and any packaging/CI entries that reference the removed filename, and
confirm the systemd unit and service name
(taos-openai-agents-sdk-bridge.service) and environment variables used in the
remaining script remain unchanged so behavior is identical after deduplication.
desktop/src/apps/MessagesApp.tsx (1)

486-501: ⚠️ Potential issue | 🟡 Minor

Clear sendError when the draft or channel changes.

After a bare-slash 400, the alert sticks around even after the user adds @agent or switches channels, because sendError is only reset on the next successful send. That leaves a stale error state in the composer.

Suggested fix
   useEffect(() => {
     if (!selectedChannel) return;
@@
     fetchMessages(selectedChannel);
     markRead(selectedChannel);
     setTypingHumans([]);
     setTypingAgents([]);
+    setSendError(null);
   }, [selectedChannel, fetchMessages, markRead]);
@@
   const handleInputChange = (val: string) => {
+    setSendError(null);
     setInput(val);

Also applies to: 561-578

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@desktop/src/apps/MessagesApp.tsx` around lines 486 - 501, The sendError state
is not cleared when the user switches channels or edits the draft, leaving stale
error alerts; update the effect that runs on selectedChannel (the useEffect that
calls fetchMessages and markRead) to call setSendError(null) after
switching/joining the channel, and also clear sendError in the effect that
watches composer draft changes (the effect around lines 561-578) so that
setSendError(null) is invoked whenever the draft or channel changes.
🧹 Nitpick comments (4)
desktop/src/apps/chat/__tests__/TypingFooter.test.tsx (1)

5-26: Good test coverage with one minor gap.

The test suite covers key scenarios well. Consider adding a test for exactly two humans, which uses different formatting ("alice and bob are typing…") per the implementation's formatHumansTyping function.

💡 Optional: Add two-human test case
   it("shows N others when humans > 2", () => {
     render(<TypingFooter humans={["alice", "bob", "carol"]} agents={[]} />);
     expect(screen.getByText("alice and 2 others are typing…")).toBeInTheDocument();
   });
+  it("shows two humans by name", () => {
+    render(<TypingFooter humans={["alice", "bob"]} agents={[]} />);
+    expect(screen.getByText("alice and bob are typing…")).toBeInTheDocument();
+  });
   it("filters self out of humans", () => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@desktop/src/apps/chat/__tests__/TypingFooter.test.tsx` around lines 5 - 26,
Add a test to cover the two-human case: call render(<TypingFooter
humans={["alice","bob"]} agents={[]} />) and assert that the text "alice and bob
are typing…" is present (use screen.getByText(...)). This verifies the
formatting implemented by formatHumansTyping / TypingFooter for exactly two
humans and should be added alongside the existing tests in
TypingFooter.test.tsx.
tests/test_framework_slash_commands.py (1)

13-16: Make the command-name assertion order-independent.

Using body["tom"][0] makes the test brittle to harmless ordering changes.

Suggested assertion update
-    assert body["tom"][0]["name"] in ("help", "clear", "model")
+    names = {cmd["name"] for cmd in body["tom"]}
+    assert {"help", "clear", "model"} & names
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_framework_slash_commands.py` around lines 13 - 16, The test is
brittle because it asserts the first element body["tom"][0]["name"] equals one
of ("help","clear","model"), which breaks on harmless reordering; change the
assertion to be order-independent by collecting the command names from
body["tom"] (e.g., names = {cmd["name"] for cmd in body["tom"]}) and then assert
that the expected command names are present (using subset/superset or membership
checks) so the test passes regardless of list order. Ensure you still verify
body["tom"] is a list and that the required command names exist in the names
set.
tinyagentos/routes/framework.py (1)

120-123: Harden manifest serialization to avoid a full 500 on one bad command entry.

c["name"] assumes every item is a dict with name. A malformed entry would fail the whole endpoint response.

Suggested defensive serialization
-        cmds = fw.get("slash_commands") or []
-        out[slug] = [{"name": c["name"], "description": c.get("description", "")} for c in cmds]
+        cmds = fw.get("slash_commands") or []
+        normalized: list[dict[str, str]] = []
+        for c in cmds:
+            if not isinstance(c, dict):
+                continue
+            name = c.get("name")
+            if not name:
+                continue
+            normalized.append({
+                "name": str(name),
+                "description": str(c.get("description", "")),
+            })
+        out[slug] = normalized
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tinyagentos/routes/framework.py` around lines 120 - 123, The manifest
serialization assumes each slash command is a dict with a "name" key which
raises on malformed entries; update the logic around FRAMEWORKS/fw/cmds to
defensively build the list (instead of the current list comprehension) by
iterating cmds, skipping non-dict items, using c.get("name", "") and
c.get("description", "") (or otherwise falling back to safe defaults), and
optionally wrapping per-item access in a try/except to ensure one bad command
does not cause JSONResponse(out) to 500.
desktop/src/apps/chat/SlashMenu.tsx (1)

45-55: Unreachable “no commands available” branch.

The early return makes the rows.length === 0 UI block unreachable. Either remove that branch or keep the container and show the empty-state intentionally.

♻️ Simplify control flow
-  if (cmdRows.length === 0 && rows.length === 0) return null;
+  if (cmdRows.length === 0) return null;
@@
-      {rows.length === 0 ? (
-        <div className="px-3 py-2 text-xs text-shell-text-tertiary">(no commands available)</div>
-      ) : (
-        rows.map((row) => {
+      {rows.map((row) => {
           if (row.kind === "header") {
             return (
@@
-        })
-      )}
+      })}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@desktop/src/apps/chat/SlashMenu.tsx` around lines 45 - 55, The JSX branch
that renders "(no commands available)" is unreachable because SlashMenu returns
early when both cmdRows.length === 0 and rows.length === 0; either remove the
unreachable rows.length === 0 branch in the render or stop returning early so
the container can be shown for the empty-state. Concretely, in the SlashMenu
component adjust the early return involving cmdRows and rows (or remove the
rows.length === 0 conditional inside the returned JSX) so that the empty-state
UI for rows is actually reachable; keep references to cmdRows and rows to locate
the exact condition to change.
🤖 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/chat/AgentContextMenu.tsx`:
- Around line 38-48: The click handlers doMute and doRemove call async APIs
(muteAgent, unmuteAgent, removeChannelMember) but lack error handling and always
invoke onClose; add an onError callback prop and wrap the await calls in
try/catch so API errors are caught, call onError(error) inside the catch, and
only call onClose after a successful operation (i.e., move onClose into the try
after await, not in finally). Update doMute and doRemove to reference the new
onError prop and preserve existing early returns when channelId is missing.

In `@desktop/src/apps/chat/ChannelSettingsPanel.tsx`:
- Around line 176-179: The range input currently only persists changes onMouseUp
which misses keyboard and touch interactions; update the handler on the hops
slider (the input using value={hops}, setHops and apply) to call apply on onBlur
and onPointerUp (or onTouchEnd) in addition to/onstead of onMouseUp so keyboard
and touch users trigger saving, and keep the same reset callback (() =>
setHops(channel.settings.max_hops ?? 3)) after apply; make the same change for
the other similar range control in this component.

In `@desktop/src/apps/MessagesApp.tsx`:
- Around line 533-554: The current sendMessage function is calling the real POST
/api/chat/messages to "validate" slash commands which can trigger server-side
persistence and a 500 due to missing author_id; instead, stop using
/api/chat/messages for validation—either perform the slash-command
syntax/permission validation client-side in sendMessage (using input and
selectedChannel) or call a dedicated validation endpoint (e.g.,
/api/chat/validate) that does not persist messages; remove the fetch to
/api/chat/messages in sendMessage (or replace it with a call to the
non-persisting validator) and keep the WebSocket send via wsRef.current.send,
ensuring setSendError is used for validation failures to avoid double-posts.

In `@desktop/src/lib/use-typing-emitter.ts`:
- Around line 1-2: The debounce timestamp lastSentAt (a ref) is leaking between
channels so the first typing emit after a channel switch gets suppressed; locate
lastSentAt in use-typing-emitter (used by the hook/function that sends typing
events) and reset it whenever the active channel identifier changes (e.g., in a
useEffect watching channelId or similar) by setting lastSentAt.current to null/0
so the new channel starts fresh; ensure any debounce logic that reads lastSentAt
uses this ref so the first emit in the newly selected channel is allowed.

In `@tests/e2e/test_chat_phase2a.py`:
- Around line 57-60: Replace the ineffective visibility check after reopening
Channel settings: instead of expecting page.get_by_role("button", name="lively")
to be visible (that button is always rendered), assert that it is
selected/persisted by checking its selection indicator (e.g. aria-pressed,
aria-current, a selected CSS class or a data-selected attribute). Locate the
block after page.get_by_role("button", name="Close").click() and
page.get_by_role("button", name="Channel settings").click(), and change the
expect for page.get_by_role("button", name="lively") to assert the appropriate
attribute or class that represents the selected state (using
expect(...).to_have_attribute or expect(...).to_have_class).

In `@tests/test_chat_typing.py`:
- Line 128: The test unpacks a third value from _setup_client into token but
never uses it; change the variable name to _token to indicate it's intentionally
unused. Update the assignment "app, client, token = await
_setup_client(tmp_path)" to "app, client, _token = await
_setup_client(tmp_path)" so the test linter no longer flags an unused variable
while keeping the same returned values from _setup_client.

In `@tests/test_framework_manifest.py`:
- Around line 39-42: The test currently only checks that c.get("name") is truthy
and that description defaults to "" then is a str, which lets non-string names
or missing description keys slip through; update the loop over cmds to assert
the command dict shape more strictly by checking isinstance(c.get("name"), str)
(and optionally that c["name"] is non-empty) and asserting that "description" is
present in the dict and isinstance(c["description"], str); reference the loop
variables cmds, fw_id and the command dict c when making these assertion
changes.

In `@tinyagentos/chat/typing_registry.py`:
- Around line 38-46: The loop over self._entries currently skips expiry checks
for entries in other channels (using if ch != channel_id: continue), causing
stale entries to linger; change the logic so you always evaluate expires_at <
now for every (ch, slug) in self._entries and collect those keys into stale for
removal, but only append slug to out[kind] when ch == channel_id and the entry
is not expired; use the existing variables (self._entries, channel_id, now,
stale, out) and pop all stale keys after the scan to ensure expired entries
across channels are cleaned up.

In `@tinyagentos/routes/chat.py`:
- Around line 579-594: The post_typing handler trusts client-supplied author_id
and allows impersonation; change it to derive the author identity server-side
(e.g., use request.state.user.id or fall back to the fixed string "user" in
single-user mode) instead of reading body.get("author_id"), remove the
client-author_id validation, and pass the server-derived identity into
reg.mark(...) and the hub.broadcast payload (update references in the
post_typing function and ensure you still handle missing typing registry or hub
as before).

In `@tinyagentos/scripts/install_hermes.sh`:
- Around line 168-173: The current await of the thinking heartbeat POST (the
c.post call to "{BRIDGE_URL}/api/chat/channels/{ch_id}/thinking" with timeout=5,
used inside _thinking() and the duplicate call at lines ~230-234) blocks the
reply flow on slow networks; change it to a non-blocking fire-and-forget
background task instead of awaiting it. Replace the direct await with scheduling
the POST as an independent asyncio task (e.g., via asyncio.create_task) so the
model call path does not wait for the heartbeat; ensure the task captures and
logs exceptions internally to avoid unhandled exceptions and apply this change
to both occurrences (the _thinking() invocation and the duplicate POST).

---

Outside diff comments:
In `@desktop/src/apps/MessagesApp.tsx`:
- Around line 486-501: The sendError state is not cleared when the user switches
channels or edits the draft, leaving stale error alerts; update the effect that
runs on selectedChannel (the useEffect that calls fetchMessages and markRead) to
call setSendError(null) after switching/joining the channel, and also clear
sendError in the effect that watches composer draft changes (the effect around
lines 561-578) so that setSendError(null) is invoked whenever the draft or
channel changes.

In `@tinyagentos/scripts/install_openai-agents-sdk.sh`:
- Around line 1-151: There are two identical installer scripts
(install_openai-agents-sdk.sh and install_openai_agents_sdk.sh); delete the
duplicate (keep install_openai-agents-sdk.sh as canonical), remove the redundant
file from the repo and any packaging/CI entries that reference the removed
filename, and confirm the systemd unit and service name
(taos-openai-agents-sdk-bridge.service) and environment variables used in the
remaining script remain unchanged so behavior is identical after deduplication.

---

Nitpick comments:
In `@desktop/src/apps/chat/__tests__/TypingFooter.test.tsx`:
- Around line 5-26: Add a test to cover the two-human case: call
render(<TypingFooter humans={["alice","bob"]} agents={[]} />) and assert that
the text "alice and bob are typing…" is present (use screen.getByText(...)).
This verifies the formatting implemented by formatHumansTyping / TypingFooter
for exactly two humans and should be added alongside the existing tests in
TypingFooter.test.tsx.

In `@desktop/src/apps/chat/SlashMenu.tsx`:
- Around line 45-55: The JSX branch that renders "(no commands available)" is
unreachable because SlashMenu returns early when both cmdRows.length === 0 and
rows.length === 0; either remove the unreachable rows.length === 0 branch in the
render or stop returning early so the container can be shown for the
empty-state. Concretely, in the SlashMenu component adjust the early return
involving cmdRows and rows (or remove the rows.length === 0 conditional inside
the returned JSX) so that the empty-state UI for rows is actually reachable;
keep references to cmdRows and rows to locate the exact condition to change.

In `@tests/test_framework_slash_commands.py`:
- Around line 13-16: The test is brittle because it asserts the first element
body["tom"][0]["name"] equals one of ("help","clear","model"), which breaks on
harmless reordering; change the assertion to be order-independent by collecting
the command names from body["tom"] (e.g., names = {cmd["name"] for cmd in
body["tom"]}) and then assert that the expected command names are present (using
subset/superset or membership checks) so the test passes regardless of list
order. Ensure you still verify body["tom"] is a list and that the required
command names exist in the names set.

In `@tinyagentos/routes/framework.py`:
- Around line 120-123: The manifest serialization assumes each slash command is
a dict with a "name" key which raises on malformed entries; update the logic
around FRAMEWORKS/fw/cmds to defensively build the list (instead of the current
list comprehension) by iterating cmds, skipping non-dict items, using
c.get("name", "") and c.get("description", "") (or otherwise falling back to
safe defaults), and optionally wrapping per-item access in a try/except to
ensure one bad command does not cause JSONResponse(out) to 500.
🪄 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: 96505fc9-d1ac-4472-b6d6-4c017e6d94b6

📥 Commits

Reviewing files that changed from the base of the PR and between 329aa79 and 81fbac3.

⛔ Files ignored due to path filters (1)
  • desktop/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (46)
  • desktop/package.json
  • desktop/src/apps/MessagesApp.tsx
  • desktop/src/apps/chat/AgentContextMenu.tsx
  • desktop/src/apps/chat/ChannelSettingsPanel.tsx
  • desktop/src/apps/chat/SlashMenu.tsx
  • desktop/src/apps/chat/TypingFooter.tsx
  • desktop/src/apps/chat/__tests__/AgentContextMenu.test.tsx
  • desktop/src/apps/chat/__tests__/ChannelSettingsPanel.test.tsx
  • desktop/src/apps/chat/__tests__/SlashMenu.test.tsx
  • desktop/src/apps/chat/__tests__/TypingFooter.test.tsx
  • desktop/src/lib/__tests__/channel-admin-api.test.ts
  • desktop/src/lib/__tests__/use-typing-emitter.test.ts
  • desktop/src/lib/channel-admin-api.ts
  • desktop/src/lib/use-typing-emitter.ts
  • desktop/tsconfig.json
  • desktop/tsconfig.tsbuildinfo
  • desktop/vite.config.ts
  • desktop/vitest.setup.ts
  • docs/superpowers/plans/2026-04-19-chat-phase-2a-desktop-admin.md
  • static/desktop/assets/MCPApp-CXYkNRmi.js
  • static/desktop/assets/MessagesApp-CpdV4x56.js
  • static/desktop/assets/MessagesApp-DJJbqaHc.js
  • static/desktop/assets/ProvidersApp-CXu-cjI7.js
  • static/desktop/assets/SettingsApp-DaFS5S5t.js
  • static/desktop/assets/chat-Bi2r4C53.js
  • static/desktop/assets/main-BXOeBesV.js
  • static/desktop/assets/tokens-DIiUixLu.js
  • static/desktop/assets/tokens-f9GIns3r.css
  • static/desktop/assets/tokens-ib1qRNqW.css
  • static/desktop/chat.html
  • static/desktop/index.html
  • tests/e2e/test_chat_phase2a.py
  • tests/test_chat_typing.py
  • tests/test_framework_manifest.py
  • tests/test_framework_slash_commands.py
  • tinyagentos/app.py
  • tinyagentos/chat/typing_registry.py
  • tinyagentos/frameworks.py
  • tinyagentos/routes/chat.py
  • tinyagentos/routes/framework.py
  • tinyagentos/scripts/install_hermes.sh
  • tinyagentos/scripts/install_langroid.sh
  • tinyagentos/scripts/install_openai-agents-sdk.sh
  • tinyagentos/scripts/install_openai_agents_sdk.sh
  • tinyagentos/scripts/install_pocketflow.sh
  • tinyagentos/scripts/install_smolagents.sh
💤 Files with no reviewable changes (2)
  • static/desktop/assets/tokens-f9GIns3r.css
  • static/desktop/assets/MessagesApp-CpdV4x56.js

Comment on lines +38 to +48
const doMute = async () => {
if (!channelId) return;
try {
if (isMuted) await unmuteAgent(channelId, slug);
else await muteAgent(channelId, slug);
} finally { onClose(); }
};
const doRemove = async () => {
if (!channelId) return;
try { await removeChannelMember(channelId, slug); } finally { onClose(); }
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nP --type=tsx 'const do(Mute|Remove)\s*=\s*async' -A12 -B2 desktop/src/apps/chat/AgentContextMenu.tsx
rg -nP --type=tsx 'await (muteAgent|unmuteAgent|removeChannelMember)\(' -A4 -B2 desktop/src/apps/chat/AgentContextMenu.tsx

Repository: jaylfc/tinyagentos

Length of output: 117


🏁 Script executed:

cat desktop/src/apps/chat/AgentContextMenu.tsx

Repository: jaylfc/tinyagentos

Length of output: 2743


Add error handling to mute/remove async handlers.

doMute and doRemove call API functions that throw on failure, but lack catch blocks. This allows promise rejections to escape from click handlers without user feedback. The menu closes regardless of success or failure, creating poor UX when operations fail.

Add an onError callback prop and catch API errors:

💡 Suggested fix
 export type AgentContextMenuProps = {
   slug: string;
   channelId?: string;
   channelType?: string;
   isMuted?: boolean;
   x: number;
   y: number;
   onClose: () => void;
+  onError?: (message: string) => void;
   onDm?: (slug: string) => void;
   onViewInfo?: (slug: string) => void;
   onJumpToSettings?: (slug: string) => void;
 };

 export function AgentContextMenu({
   slug, channelId, channelType, isMuted,
-  x, y, onClose, onDm, onViewInfo, onJumpToSettings,
+  x, y, onClose, onError, onDm, onViewInfo, onJumpToSettings,
 }: AgentContextMenuProps) {
@@
   const doMute = async () => {
     if (!channelId) return;
     try {
       if (isMuted) await unmuteAgent(channelId, slug);
       else await muteAgent(channelId, slug);
+    } catch (e) {
+      onError?.(e instanceof Error ? e.message : "Failed to update mute state");
     } finally { onClose(); }
   };
   const doRemove = async () => {
     if (!channelId) return;
-    try { await removeChannelMember(channelId, slug); } finally { onClose(); }
+    try {
+      await removeChannelMember(channelId, slug);
+    } catch (e) {
+      onError?.(e instanceof Error ? e.message : "Failed to remove member");
+    } finally { onClose(); }
   };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@desktop/src/apps/chat/AgentContextMenu.tsx` around lines 38 - 48, The click
handlers doMute and doRemove call async APIs (muteAgent, unmuteAgent,
removeChannelMember) but lack error handling and always invoke onClose; add an
onError callback prop and wrap the await calls in try/catch so API errors are
caught, call onError(error) inside the catch, and only call onClose after a
successful operation (i.e., move onClose into the try after await, not in
finally). Update doMute and doRemove to reference the new onError prop and
preserve existing early returns when channelId is missing.

Comment on lines +176 to +179
type="range" min={1} max={10} value={hops}
onChange={(e) => setHops(Number(e.target.value))}
onMouseUp={() => apply({ max_hops: hops }, () => setHops(channel.settings.max_hops ?? 3))}
/>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Range settings may not persist for keyboard/touch interactions.

Saving only on onMouseUp misses non-mouse input paths. Persist on onBlur (and/or onPointerUp) so keyboard users can actually save changes.

♿ Suggested fix
 <input
   type="range" min={1} max={10} value={hops}
   onChange={(e) => setHops(Number(e.target.value))}
-  onMouseUp={() => apply({ max_hops: hops }, () => setHops(channel.settings.max_hops ?? 3))}
+  onPointerUp={() => apply({ max_hops: hops }, () => setHops(channel.settings.max_hops ?? 3))}
+  onBlur={() => apply({ max_hops: hops }, () => setHops(channel.settings.max_hops ?? 3))}
 />
@@
 <input
   type="range" min={0} max={60} value={cooldown}
   onChange={(e) => setCooldown(Number(e.target.value))}
-  onMouseUp={() => apply({ cooldown_seconds: cooldown }, () => setCooldown(channel.settings.cooldown_seconds ?? 5))}
+  onPointerUp={() => apply({ cooldown_seconds: cooldown }, () => setCooldown(channel.settings.cooldown_seconds ?? 5))}
+  onBlur={() => apply({ cooldown_seconds: cooldown }, () => setCooldown(channel.settings.cooldown_seconds ?? 5))}
 />

Also applies to: 184-187

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@desktop/src/apps/chat/ChannelSettingsPanel.tsx` around lines 176 - 179, The
range input currently only persists changes onMouseUp which misses keyboard and
touch interactions; update the handler on the hops slider (the input using
value={hops}, setHops and apply) to call apply on onBlur and onPointerUp (or
onTouchEnd) in addition to/onstead of onMouseUp so keyboard and touch users
trigger saving, and keep the same reset callback (() =>
setHops(channel.settings.max_hops ?? 3)) after apply; make the same change for
the other similar range control in this component.

Comment on lines +533 to 554
const sendMessage = async () => {
const text = input.trim();
if (!text || !selectedChannel || !wsRef.current || wsRef.current.readyState !== 1) return;
// If slash input, validate via REST before sending over WS
if (text.startsWith("/")) {
try {
const r = await fetch("/api/chat/messages", {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ channel_id: selectedChannel, content: text }),
});
if (r.status === 400) {
const body = await r.json().catch(() => ({}));
setSendError((body as { error?: string }).error || "couldn't send message");
return;
}
} catch {
/* network error — fall through to WS send */
}
}
setSendError(null);
wsRef.current.send(JSON.stringify({ type: "message", channel_id: selectedChannel, content: text }));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don’t call the real send endpoint just to validate slash commands.

/api/chat/messages is the message-creation route, not a pure validator. In tinyagentos/routes/chat.py:179-206, any valid slash input falls past the 400 guard and then reads body["author_id"]; this client call does not send that field, so it will 500 before the WS send. If you later add the missing fields here, this path will double-post because the HTTP route already persists and broadcasts the message.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@desktop/src/apps/MessagesApp.tsx` around lines 533 - 554, The current
sendMessage function is calling the real POST /api/chat/messages to "validate"
slash commands which can trigger server-side persistence and a 500 due to
missing author_id; instead, stop using /api/chat/messages for validation—either
perform the slash-command syntax/permission validation client-side in
sendMessage (using input and selectedChannel) or call a dedicated validation
endpoint (e.g., /api/chat/validate) that does not persist messages; remove the
fetch to /api/chat/messages in sendMessage (or replace it with a call to the
non-persisting validator) and keep the WebSocket send via wsRef.current.send,
ensuring setSendError is used for validation failures to avoid double-posts.

Comment on lines +1 to +2
import { useCallback, useRef } from "react";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Debounce timestamp leaks across channel switches.

lastSentAt persists between channels, so a quick channel switch can suppress the first typing emit in the new channel.

Suggested fix
-import { useCallback, useRef } from "react";
+import { useCallback, useEffect, useRef } from "react";
@@
   const lastSentAt = useRef(0);
+  useEffect(() => {
+    lastSentAt.current = 0;
+  }, [channelId]);

Also applies to: 14-20

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@desktop/src/lib/use-typing-emitter.ts` around lines 1 - 2, The debounce
timestamp lastSentAt (a ref) is leaking between channels so the first typing
emit after a channel switch gets suppressed; locate lastSentAt in
use-typing-emitter (used by the hook/function that sends typing events) and
reset it whenever the active channel identifier changes (e.g., in a useEffect
watching channelId or similar) by setting lastSentAt.current to null/0 so the
new channel starts fresh; ensure any debounce logic that reads lastSentAt uses
this ref so the first emit in the newly selected channel is allowed.

Comment on lines +57 to +60
page.get_by_role("button", name="Close").click()
page.get_by_role("button", name="Channel settings").click()
expect(page.get_by_role("button", name="lively")).to_be_visible()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Persistence assertion is ineffective (always true).

Reopening and checking that the “lively” button is visible does not verify saved mode, because that button is always rendered. Assert selected state instead.

✅ Suggested test fix
+import re
@@
-    page.get_by_role("button", name="Channel settings").click()
-    expect(page.get_by_role("button", name="lively")).to_be_visible()
+    page.get_by_role("button", name="Channel settings").click()
+    lively = page.get_by_role("button", name="lively")
+    expect(lively).to_have_class(re.compile(r"\bbg-emerald-500/30\b"))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/e2e/test_chat_phase2a.py` around lines 57 - 60, Replace the ineffective
visibility check after reopening Channel settings: instead of expecting
page.get_by_role("button", name="lively") to be visible (that button is always
rendered), assert that it is selected/persisted by checking its selection
indicator (e.g. aria-pressed, aria-current, a selected CSS class or a
data-selected attribute). Locate the block after page.get_by_role("button",
name="Close").click() and page.get_by_role("button", name="Channel
settings").click(), and change the expect for page.get_by_role("button",
name="lively") to assert the appropriate attribute or class that represents the
selected state (using expect(...).to_have_attribute or
expect(...).to_have_class).

Comment thread tests/test_chat_typing.py

@pytest.mark.asyncio
async def test_post_thinking_start_marks_registry(tmp_path):
app, client, token = await _setup_client(tmp_path)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Unused unpacked variable in test setup.

token is unused in this test case; rename to _token to satisfy lint and keep intent clear.

🧹 Minimal fix
-    app, client, token = await _setup_client(tmp_path)
+    app, client, _token = await _setup_client(tmp_path)
📝 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
app, client, token = await _setup_client(tmp_path)
app, client, _token = await _setup_client(tmp_path)
🧰 Tools
🪛 Ruff (0.15.10)

[warning] 128-128: Unpacked variable token is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_chat_typing.py` at line 128, The test unpacks a third value from
_setup_client into token but never uses it; change the variable name to _token
to indicate it's intentionally unused. Update the assignment "app, client, token
= await _setup_client(tmp_path)" to "app, client, _token = await
_setup_client(tmp_path)" so the test linter no longer flags an unused variable
while keeping the same returned values from _setup_client.

Comment on lines +39 to +42
for c in cmds:
assert isinstance(c, dict), f"{fw_id}: each command must be a dict"
assert c.get("name"), f"{fw_id}: command missing name"
assert isinstance(c.get("description", ""), str)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Tighten slash-command schema assertions to enforce key presence and types.

Line 41 and Line 42 currently allow invalid shapes (e.g., non-string name, missing description) to pass this contract test.

Proposed test hardening
         for c in cmds:
             assert isinstance(c, dict), f"{fw_id}: each command must be a dict"
-            assert c.get("name"), f"{fw_id}: command missing name"
-            assert isinstance(c.get("description", ""), str)
+            assert isinstance(c.get("name"), str) and c["name"].strip(), f"{fw_id}: command missing/invalid name"
+            assert "description" in c and isinstance(c["description"], str), f"{fw_id}: command missing/invalid description"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_framework_manifest.py` around lines 39 - 42, The test currently
only checks that c.get("name") is truthy and that description defaults to ""
then is a str, which lets non-string names or missing description keys slip
through; update the loop over cmds to assert the command dict shape more
strictly by checking isinstance(c.get("name"), str) (and optionally that
c["name"] is non-empty) and asserting that "description" is present in the dict
and isinstance(c["description"], str); reference the loop variables cmds, fw_id
and the command dict c when making these assertion changes.

Comment on lines +38 to +46
for (ch, slug), (kind, expires_at) in self._entries.items():
if ch != channel_id:
continue
if expires_at < now:
stale.append((ch, slug))
continue
out[kind].append(slug)
for k in stale:
self._entries.pop(k, None)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Stale-entry cleanup is channel-scoped, which can leak memory over time.

Expired entries in other channels are skipped before expiry checks, so they persist until that exact channel is queried again. This can accumulate stale records indefinitely.

💡 Suggested fix
     def list(self, channel_id: str) -> dict[str, list[str]]:
         now = _now()
         out: dict[str, list[str]] = {"human": [], "agent": []}
         stale: list[tuple[str, str]] = []
-        for (ch, slug), (kind, expires_at) in self._entries.items():
-            if ch != channel_id:
-                continue
-            if expires_at < now:
-                stale.append((ch, slug))
-                continue
-            out[kind].append(slug)
+        for (ch, slug), (kind, expires_at) in self._entries.items():
+            if expires_at <= now:
+                stale.append((ch, slug))
+                continue
+            if ch == channel_id:
+                out[kind].append(slug)
         for k in stale:
             self._entries.pop(k, 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.

Suggested change
for (ch, slug), (kind, expires_at) in self._entries.items():
if ch != channel_id:
continue
if expires_at < now:
stale.append((ch, slug))
continue
out[kind].append(slug)
for k in stale:
self._entries.pop(k, None)
for (ch, slug), (kind, expires_at) in self._entries.items():
if expires_at <= now:
stale.append((ch, slug))
continue
if ch == channel_id:
out[kind].append(slug)
for k in stale:
self._entries.pop(k, None)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tinyagentos/chat/typing_registry.py` around lines 38 - 46, The loop over
self._entries currently skips expiry checks for entries in other channels (using
if ch != channel_id: continue), causing stale entries to linger; change the
logic so you always evaluate expires_at < now for every (ch, slug) in
self._entries and collect those keys into stale for removal, but only append
slug to out[kind] when ch == channel_id and the entry is not expired; use the
existing variables (self._entries, channel_id, now, stale, out) and pop all
stale keys after the scan to ensure expired entries across channels are cleaned
up.

Comment on lines +579 to +594
async def post_typing(channel_id: str, body: dict, request: Request):
"""Mark a human user as typing in the channel. Ephemeral; TTL 3s."""
author_id = (body or {}).get("author_id")
if not author_id:
return JSONResponse({"error": "author_id required"}, status_code=400)
reg = getattr(request.app.state, "typing", None)
hub = getattr(request.app.state, "chat_hub", None)
if reg is None:
return JSONResponse({"error": "typing registry not configured"}, status_code=503)
reg.mark(channel_id, author_id, "human")
if hub is not None:
await hub.broadcast(channel_id, {
"type": "typing",
"kind": "human",
"slug": author_id,
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

/typing allows identity spoofing via client-supplied author_id.

The endpoint currently trusts arbitrary author_id from request body, so callers can impersonate other users in typing signals. Use server-derived identity (or fixed "user" in current single-user mode) instead of accepting a caller-provided author.

🔐 Suggested hardening
 async def post_typing(channel_id: str, body: dict, request: Request):
     """Mark a human user as typing in the channel. Ephemeral; TTL 3s."""
-    author_id = (body or {}).get("author_id")
-    if not author_id:
-        return JSONResponse({"error": "author_id required"}, status_code=400)
+    # Do not trust caller-supplied identity for typing signals.
+    author_id = "user"
@@
     reg.mark(channel_id, author_id, "human")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tinyagentos/routes/chat.py` around lines 579 - 594, The post_typing handler
trusts client-supplied author_id and allows impersonation; change it to derive
the author identity server-side (e.g., use request.state.user.id or fall back to
the fixed string "user" in single-user mode) instead of reading
body.get("author_id"), remove the client-author_id validation, and pass the
server-derived identity into reg.mark(...) and the hub.broadcast payload (update
references in the post_typing function and ensure you still handle missing
typing registry or hub as before).

Comment on lines +168 to +173
await c.post(
f"{BRIDGE_URL}/api/chat/channels/{ch_id}/thinking",
json={"slug": AGENT_NAME, "state": state},
headers={"Authorization": f"Bearer {LOCAL_TOKEN}"},
timeout=5,
)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Thinking heartbeat currently blocks reply flow on network slowness.

Awaiting _thinking() with a 5s timeout before/after the model call can add large per-message latency.

Suggested non-blocking approach
-    await _thinking(client, cid, "start")
+    asyncio.create_task(_thinking(client, cid, "start"))
     try:
         reply = await call_hermes(client, messages)
     finally:
-        await _thinking(client, cid, "end")
+        asyncio.create_task(_thinking(client, cid, "end"))

Also applies to: 230-234

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tinyagentos/scripts/install_hermes.sh` around lines 168 - 173, The current
await of the thinking heartbeat POST (the c.post call to
"{BRIDGE_URL}/api/chat/channels/{ch_id}/thinking" with timeout=5, used inside
_thinking() and the duplicate call at lines ~230-234) blocks the reply flow on
slow networks; change it to a non-blocking fire-and-forget background task
instead of awaiting it. Replace the direct await with scheduling the POST as an
independent asyncio task (e.g., via asyncio.create_task) so the model call path
does not wait for the heartbeat; ensure the task captures and logs exceptions
internally to avoid unhandled exceptions and apply this change to both
occurrences (the _thinking() invocation and the duplicate POST).

jaylfc added 2 commits April 19, 2026 19:45
)

Master commit 12696c6 added push_file to the deploy flow for frameworks
with an in-repo install script. test_deployer.py mocks the other subprocess
callouts (create_container, exec_in_container, add_proxy_device) but not
push_file, so every deploy-flow test on CI (ubuntu-latest, no incus binary)
fails with FileNotFoundError.

Fix is narrow: add push_file to every existing mock block, returning the
success sentinel (0, '') the real helper returns on a successful push.
Master commit 12696c6 routes frameworks with an in-repo install script
through the script path before the pip fallback. Two tests were asserting
pip-specific behavior on smolagents, which has a script. One fix updates
a failure-simulation mock to also recognise the script path as a framework
install command; the other switches to a framework slug that has no script
so the test continues exercising the pip path it's documenting.
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/test_deployer.py (1)

386-390: Broaden script-path failure match to include manifest script installs.

The condition only matches bash /tmp/install_..., but manifest script installs execute bash /tmp/install.sh (see tinyagentos/deployer.py:311-324). Including both keeps rollback coverage robust if this test setup ever routes through manifest-script install.

Proposed matcher tweak
-            if ("pip3 install" in cmd_str and "nonexistent-pkg" in cmd_str) or \
-                    cmd_str.startswith("bash /tmp/install_"):
+            if ("pip3 install" in cmd_str and "nonexistent-pkg" in cmd_str) or \
+                    cmd_str.startswith("bash /tmp/install_") or \
+                    cmd_str == "bash /tmp/install.sh":
                 return (1, "ERROR: No matching distribution found")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_deployer.py` around lines 386 - 390, The test's conditional that
matches script installs only checks for commands starting with "bash
/tmp/install_" so it misses manifest-script installs executed as "bash
/tmp/install.sh"; update the condition that inspects cmd_str (the branch in
tests/test_deployer.py where the pip install or script path is matched) to also
accept "bash /tmp/install.sh" (for example by matching either startswith("bash
/tmp/install_") OR equals/startswith "bash /tmp/install.sh" or more generally
startswith("bash /tmp/install")), ensuring rollback coverage regardless of
whether the installer came from the framework slug path or the manifest script
path referenced in tinyagentos/deployer.py.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/test_deployer.py`:
- Around line 386-390: The test's conditional that matches script installs only
checks for commands starting with "bash /tmp/install_" so it misses
manifest-script installs executed as "bash /tmp/install.sh"; update the
condition that inspects cmd_str (the branch in tests/test_deployer.py where the
pip install or script path is matched) to also accept "bash /tmp/install.sh"
(for example by matching either startswith("bash /tmp/install_") OR
equals/startswith "bash /tmp/install.sh" or more generally startswith("bash
/tmp/install")), ensuring rollback coverage regardless of whether the installer
came from the framework slug path or the manifest script path referenced in
tinyagentos/deployer.py.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4c4e1726-451a-4636-ba66-5c23e13bc63b

📥 Commits

Reviewing files that changed from the base of the PR and between 81fbac3 and c4f0dac.

📒 Files selected for processing (1)
  • tests/test_deployer.py

@jaylfc jaylfc merged commit d28dcfa into master Apr 19, 2026
12 of 14 checks passed
@jaylfc jaylfc deleted the feat/chat-phase-2a-desktop-admin branch April 19, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant