Skip to content

feat(chat): add workspace panel, tool call cards, and screen polish#450

Open
0r0b0r011 wants to merge 2 commits into
fathah:mainfrom
0r0b0r011:split/chat-workspace
Open

feat(chat): add workspace panel, tool call cards, and screen polish#450
0r0b0r011 wants to merge 2 commits into
fathah:mainfrom
0r0b0r011:split/chat-workspace

Conversation

@0r0b0r011

Copy link
Copy Markdown

Summary

Final deferred slice from the #438 split — chat workspace UI and minor screen polish:

  • Chat workspace panel: Side panel showing context folder path and latest tool output during chat
  • Tool call cards: Collapsible JSON syntax-highlighted cards replacing inline tool-call history rows
  • Memory: Split-pane edit with live markdown preview
  • Skills: Origin badges (marketplace/project/built-in) and path display on skill cards
  • Gateway: "Test connection" button per enabled platform

No IPC, preload, or dependency changes — renderer-only.

Context

Supersedes the remaining hunks from closed #438. Merge after the feature PRs (#444#449) or independently — this PR has no dependencies on vault, terminal, or profile wizard.

After all split PRs merge, main's oversized commits (817bb31, edc4cdf) can be dropped in favor of the merged branches.

Test plan

  • npm run typecheck
  • npm test -- tests/sessions-history-items.test.ts src/renderer/src/screens/Skills/Skills.test.tsx
  • npm run build
  • Manual: send chat with tool calls — verify workspace panel and tool call cards
  • Manual: edit memory entry — verify markdown preview pane
  • Manual: Skills tab — verify origin badges and paths
  • Manual: Gateway — enable platform, click "Test connection"

Full split stack

  1. chore(testing): add tester build scripts and guide #444 — Tester build tooling
  2. feat(terminal): add embedded terminal with node-pty and xterm #445 — Embedded terminal
  3. feat(vault): add encrypted credential vault with audit log #446 — Vault/credentials
  4. feat(files): add workspace file browser with path sandbox #447 — Files browser
  5. feat(profiles): add profile wizard, migration, and activation flow #448 — Profile wizard (depends on feat(vault): add encrypted credential vault with audit log #446)
  6. feat(ui): add dashboard, MCP hub, swarm, ecosystem, and color themes #449 — Dashboard/UI shell
  7. This PR — Chat workspace + screen polish

Made with Cursor

Side panel in chat for context folder and live tool output, JSON syntax-
highlighted tool call cards, memory edit preview, skill origin badges,
and gateway platform test-connection button.

Co-authored-by: Cursor <cursoragent@cursor.com>

@pmos69 pmos69 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed statically (didn't execute). This is the cleanest PR of the #438 split — renderer-only, no security issues, and it gets the CSS discipline right. One change request before it lands.

🔴 Change request — "Test connection" button doesn't test the connection

Gateway.tsx:

{platformEnabled[platform.key] && (
  <button ... onClick={async () => {
    const started = await window.hermesAPI.startGateway();
    if (started) await loadConfig();
  }}>Test connection</button>
)}

The button is rendered per enabled platform and labelled "Test connection", but the handler ignores platform entirely and just calls startGateway() — the same global action on every row. It never verifies the platform's credentials (e.g. whether the Telegram bot token actually authenticates). So the advertised feature ("Test connection button per enabled platform") isn't implemented — it's an N-times-duplicated "start gateway" button.

Please either:

  • wire it to a real per-platform credential/connection check, or
  • relabel it ("Start gateway") and render it once rather than per-platform.

🟡 Minor — cross-PR CSS coupling

This button uses .btn-secondary, which #449 currently restyles (--bg-elevated--bg-tertiary, see my review there). Harmless on its own, but if #449's .btn-secondary override isn't fixed first, this button inherits that colour change. Flagging so the two are coordinated.

✅ What's good (worth calling out)

  • No XSS despite rendering untrusted content: ToolCallCard uses react-syntax-highlighter (tokenised React elements, escaped children); WorkspacePanel renders tool output via <pre>{toolOutput}</pre> (React-escaped); Memory preview reuses the existing AgentMarkdown (react-markdown + remark-gfm, no rehype-raw, so raw HTML is escaped). No dangerouslySetInnerHTML anywhere — reusing the app's existing sanitised markdown renderer rather than rolling a new one is exactly right.
  • CSS is properly namespaced — every new selector is scoped (.tool-call-card, .workspace-panel-*, .memory-preview-*, .skill-origin-*), and it reuses .btn/.btn-secondary/.btn-sm rather than redefining them. This is the model the rest of the split should follow.
  • "No dependency changes" holdsreact-syntax-highlighter is already in package.json.

Net: fix or relabel the Test-connection button and this is good to go. Everything else (tool-call cards, workspace panel, markdown preview, skill origin badges) is solid.

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

2 participants