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 |
1 similar comment
|
/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 |
|
|
Spend limit reached | Chat Review history
deep-review v0.5.0 | Round 11 | Status: Panel review (13 reviewers) Last posted: Round 11, 35 findings (2 P1, 14 P2, 16 P3, 3 Nit), APPROVE. 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. Round 8Panel. CRF-59 through CRF-63 addressed by author. New findings: 2 P3, 1 Nit posted. Reviewed against 6739542..4360aa1. Round 9Panel. CRF-70 through CRF-72 addressed by author. No new findings. 12 reviewers + Netero, all clean. Reviewed against 6739542..98f073f. Round 10Panel. No new findings. 7 reviewers + Netero, all clean. Delta was a single SQL alias rename commit. Reviewed against 6739542..efe9fcb. Round 11Panel. Major simplification refactor: workspace skills always use /workspace/name, personal use /name, collision detection removed. 1 Nit posted. 12 reviewers + Netero. Reviewed against 84240da..b9d8ff2. 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.
|
/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.
|
@codex review
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4360aa1d3d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e9cb022f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45c87302de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4dd4682811
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…orkspace skills on each menu open
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Remove the /api/experimental/workspaces/{workspace}/skills endpoint.
The slash menu now reads workspace skills from the chat's pinned
context resources, which the chat page already fetches and which is
the same inventory read_skill resolves from. Before a chat binds a
workspace, the menu lists personal skills only, keeping their
triggers qualified while collisions are unknown.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 355323a706
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A chat that binds its workspace agent on the first sent turn pins its context during generation preparation, after the client's send-time refetch already ran, so watching clients kept a cached chat detail without pinned resources until a manual reload. Broadcast the context watch event once first-turn hydration pins the chat so the open chat refetches its pinned resources.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aebea850f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ls menu open while workspace skills load Unknown workspace skills (chat context still resolving) counted as an empty list, so the menu closed for users with no personal skills and the trigger plugin recorded the slash as dismissed, preventing the menu from reopening when workspace skills arrived. Treat unknown as still loading: keep the menu open and show a workspace loading row.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de14911101
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nd chats An unbound workspace selection (new chat form, or a picked workspace before the first send) has no pinned context to resolve, so the menu stayed in the workspace loading state and personal triggers stayed qualified indefinitely. Count only a chat-bound workspace as having workspace skills, and derive the menu list from the resolved chat detail so an unpinned bound chat resolves to a known-empty list instead of loading forever.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ee54e9e1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…pe menu skills HydrateAgentChatsContext pins every unpinned chat bound to the agent in one statement, but the first-turn path published a context event only for the current chat, leaving sibling chats with stale cached details. Return the hydrated chat IDs from the query and publish an event for each. The slash menu also rendered duplicate workspace skill names even though read_skill collapses duplicates first-wins; dedupe by name in workspaceSkillsFromChat to match resolution.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48eba19cba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…agent push An agent's first context push hydrates already-bound NULL-hash chats, but only dirtied chats got a watch event, so watching clients kept a stale chat detail with no pinned resources and the slash menu missed the newly pushed workspace skills. Publish a context event for every chat the push hydrated or dirtied.
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The context_dirty docstring covered only first-turn pins after agent push hydration also began publishing it, and three composer comments still described the pre-review gating (selected workspaces counting as bound, the pinned context rather than the chat detail resolving).
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Adds workspace skills to the agent chat slash menu, sourced entirely from the chat's pinned context resources (the single-chat GET response the page already fetches), the same inventory
read_skillresolves from. No new API endpoint is introduced.Personal entries insert
/name, or/personal/namewhen the name collides with a workspace skill or the chat's pinned context has not resolved yet; workspace entries insert/workspace/name. Qualified aliases stay searchable even when the displayed trigger is bare. Before a chat binds a workspace (new chat form, or a selected but unbound workspace), the menu lists personal skills only.Sending a message invalidates the chat detail query, and chatd broadcasts a context watch event when a first-turn bind pins the chat, so the menu picks up newly pinned context without a reload.
Makes
UpdateChatWorkspaceBindinga no-op when the requested workspace/build/agent binding is unchanged, preservingupdated_atso chat list ordering and watch events stay stable.Includes regression coverage for the no-op binding guard, pinned-context skill mapping, collision qualification, and skills menu behavior.
Refs CODAGT-474 (skills autocompleting in the editor).