feat: add workspace skills to agent chat slash menu#25600
Conversation
Docs preview📖 View docs preview for |
There was a problem hiding this comment.
Good direction: this wires workspace skills through the API, SDK, and slash menu, and the stories cover the combined menu behavior. Netero found 1 P2, 2 P3s, and 1 Nit. Reviewer quote: "The slash menu does not receive the workspace selected for a new chat."
This is a first-pass Netero review only. The full review panel has not reviewed this PR yet; the panel will review after these findings are addressed. These are defects to address before spending parallel panel review time.
🤖 This review was automatically generated with Coder Agents.
|
/coder-agents-review |
There was a problem hiding this comment.
Good direction: the PR wires workspace skills through API, SDK, and the slash menu, and the R1 Netero fixes are present in the current head. The panel found 1 P1, 3 P2s, and 3 P3s. Reviewer quote: "It is unguarded glass."
The highest-risk issue is the new workspace skills endpoint dialing an agent after only read-level workspace access. The main UI contract issues are that qualified aliases are displayed but not searchable by typing the qualifier, and the menu can still insert bare aliases before both skill sources are authoritative.
🤖 This review was automatically generated with Coder Agents.
|
/coder-agents-review |
There was a problem hiding this comment.
The R2 fixes moved the PR in the right direction: agent access is now explicitly gated, endpoint failures no longer masquerade as empty skills, and the source-prefix stories cover the basic collision path. This round found 4 P2s and 2 P3s. Reviewer quote: "The result is a user can start selecting the conservative qualified alias that the menu showed, then lose it mid-query."
The main remaining issue is source-of-truth drift. Existing chats can treat stale or missing injected context as authoritative, live endpoint data can stay selectable while it is stale, and source-qualified aliases can disappear when the opposite source finishes loading. There is also one auth contract mismatch to resolve around whether this chat feature should accept app-connect or match chat workspace binding.
🤖 This review was automatically generated with Coder Agents.
|
/coder-agents-review |
|
/coder-agents-review |
There was a problem hiding this comment.
The latest round fixes the R3 items around SSH-only access, stale query data, qualified alias search, menu error messages, and the main persisted-context invalidation path. I found 1 P2 and 2 P3s remaining. Reviewer quote: "That reopens DEREM-6 for non-first chat agents."
The remaining blocker is still a source-of-truth mismatch for existing chats, but now narrowed to multi-agent workspaces where the chat-bound agent is not the page fallback agent. Two smaller issues remain around misconfiguration signaling and keyboard selection stability while skill sources arrive asynchronously.
🤖 This review was automatically generated with Coder Agents.
|
/coder-agents-review
|
|
/coder-agents-review |
|
|
Review in progress | Chat Review historydeep-review v0.5.0 | Round 8 | Status: Netero (first pass) Last posted: Round 7, 31 findings (2 P1, 14 P2, 14 P3, 1 Nit), REQUEST_CHANGES. Review Finding inventoryFindings
Contested and acknowledgedNone. Round logRound 1Netero-only. 1 P2, 2 P3, 1 Nit. Reviewed against ca1f6b1..c6f3c5c. Round 2Panel. DEREM-1 through DEREM-4 addressed by author. New findings: 1 P1, 3 P2, 3 P3 posted. Several test, nit, and design suggestions dropped or covered by posted functional findings. Reviewed against ca1f6b1..3fa20c9. Round 3Panel. DEREM-5 through DEREM-11 addressed by author. New findings: 4 P2, 2 P3 posted. Reviewed against ca1f6b1..7fca616. Round 4Panel. DEREM-25 through DEREM-30 addressed by author. New findings: 1 P2, 2 P3 posted. Reviewed against ca1f6b1..f0edb12. Round 5Panel. DEREM-35 through DEREM-37 addressed by author. New findings: 1 P2, 2 P3 posted. Reviewed against ca1f6b1..1233097. Round 6Panel. CRF-48 through CRF-50 addressed by author. New findings: 2 P2, 1 P3 posted. Reviewed against 6739542..34fa8b2. Round 7Panel. CRF-51 through CRF-53 addressed by author. New findings: 1 P1, 2 P2, 2 P3 posted. Reviewed against 6739542..addda28. About deep-reviewCRF = Coder Review Finding (P0-P4, Nit, Note)
|
There was a problem hiding this comment.
The current round fixes the non-first chat-agent case, agent selection errors, and the selection-key path for sources arriving. I found 1 P2 and 2 P3s. Reviewer quote: "The extinguisher has no gauge."
One source-of-truth issue remains during the initial page-load window for existing chats: until the workspace DTO supplies the bound agent, the composer can still fall back to live workspace skills instead of the persisted context the next chat turn will use. The other two findings are narrower regression and async-selection gaps.
🤖 This review was automatically generated with Coder Agents.
…-slash-menu # Conflicts: # docs/reference/api/chats.md # docs/reference/api/schemas.md # site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.stories.tsx # site/src/pages/AgentsPage/components/ChatMessageInput/PersonalSkillsTriggerMenu.tsx # site/src/pages/AgentsPage/components/ChatMessageInput/SkillsTriggerPlugin.tsx # site/src/pages/AgentsPage/utils/personalSkills.ts
|
/coder-agents-review
|
|
/coder-agents-review |
|
There was a problem hiding this comment.
The latest fixes address the chat-bound agent comparison, the delete-transition test hole, and selection identity for source arrivals. I found 2 P2s and 1 P3 remaining. Reviewer quote: "This is the same class as DEREM-6 and CRF-48."
Two remaining issues are still source-of-truth problems for existing chats: an empty cached workspace context and a stale chat detail cache can both make the slash menu use live workspace skill data that the next turn will not resolve. The third issue is a keyboard trap when the menu is open but no skill is selected.
site/src/api/queries/chats.ts:1185
P2 [CRF-52] Successful sends can leave the slash menu using a stale chat detail cache instead of the workspace skills that were just persisted. (Pariston)
A normal send is one of the paths that creates or changes that cache:
postChatMessagescallsSendMessage,persistInstructionFileswriteslast_injected_context, and subsequent turns reuseskillsFromPartsinstead of dialing live workspace skills.
The composer gets lastInjectedContext from chatQuery.data, but createChatMessage only invalidates debug runs and prompt history on success. The watched-chat merge path updates status, title, workspace, build, model, summary, unread, and timestamp, but not last_injected_context. After the first workspace-backed send persists skill A, the open page can still have an undefined or old last_injected_context; if the workspace skills change, opening / can fetch live skill B even though the next turn resolves against persisted skill A. Invalidate or update chatKey(chatId) after successful sends, or include last_injected_context in watched chat cache merges.
🤖
🤖 This review was automatically generated with Coder Agents.
|
/coder-agents-review |
|
There was a problem hiding this comment.
The latest changes fix the round 6 findings around stale chat detail, empty context fallback, and Tab behavior. I found 1 P1, 2 P2s, and 2 P3s. Reviewer quote: "The cache and replay history cannot diverge."
The highest-risk issue is that workspace rebinding clears the injected-context cache but leaves old context messages in the prompt replay path. The other findings are cache reconstruction, error detail, and release-coverage issues around the same workspace-skill surface.
🤖 This review was automatically generated with Coder Agents.
|
/coder-agents-review |
|
Adds a workspace skills API and SDK method that return skills exposed by active workspace agents, gated by workspace SSH access.
Updates the agent chat slash menu to query workspace skills alongside personal skills, qualify aliases when either source is not authoritative or names collide, keep qualified prefixes searchable, and keep the highlighted skill stable while skill sources load.
Reuses persisted chat context for existing chats when it belongs to the chat-bound agent, clears persisted context when workspace binding changes, and surfaces workspace agent load or selection errors in the menu.
Includes generated API/schema updates and regression coverage for authorization, agent failures, chat-agent selection failures, persisted context invalidation, qualified search, and menu error states.