GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — feat: load project skills in chats by jaaydenh · Pull Request #30838 · coder/coder Conversation
|
Project page / menu |
read_skill in a project chat |
|---|---|
![]() |
![]() |
Stack (merge top to bottom):
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Add sqlc queries for skills owned by a chat project and authorize them through the parent project in dbauthz: reading needs chat_project:read and writing needs chat_project:update. Skill.RBACObject keeps denying project rows so any unwrapped path fails closed. Add the chat_project_skill audit resource type and AuditableChatProjectSkill so the upcoming project skill API can record audit entries.
Add experimental routes to list, create, read, update and delete skills on a chat project. Reading needs read on the project and writes need update; callers who lack the permission get a 404 like the project instructions routes. The handlers reuse the shared skill create and update path, map the per-project cap and name index to 409, and audit every write as a chat_project_skill. Add matching ExperimentalClient methods.
Chats in a project now get the project's enabled skills as a new "project" skill source, read as the chat owner each turn. Subagents use the root chat's project. read_skill loads project skill bodies, and MergeSkills takes sources in merge order so new sources do not change its signature.
The "/" menu offers the skills of the chat's project, next to personal, organization and workspace skills, and /clear and /compact defer to a project skill with the same name. Documents project skills.
…g in the slash menu Organization and project skills load through one hook, and the menu renders its skill groups from one ordered list. Adds project loading and error stories.
…lify skill group offsets
…nd resolve emptiness in the hook
…naming in the composer
Cover the project-only skill tool check in the integration test, the project skill body loader's not-found mapping, and the chat-projects experiment gate in the composer.
There was an error while loading. Please reload this page.
Docs previewCheck off each page once it's been reviewed. If a page changes in a later push, its checkbox clears automatically so it gets a fresh look. Pages not yet wired into the docs navigation aren't listed here. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 681b37465f
Your 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".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
… request instead of menu markup
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. Nice work! 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". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Every observer of the project skills query now shares its retry policy, and a 403 or 404 after an earlier load also counts as no project skills.
…st access after project skills loaded
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 29646530af
Your 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".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
chatd loads project skills only when the chat owner owns the project or it is shared with them, while the skills list endpoint also honors role grants. The page and the composer now also consult the caller's project list, which holds only owned and shared projects, and keep project skills unknown while that list is unsettled. The composer tests mock the project list, seed caches with the exported chatProjectSkillsKey, and wait on query state instead of DOM absence.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: 89a2e9a0a2
Your 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".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
…il refetched A cached list can predate a share's revocation as well as its creation, so a project it contains is no more current than one it lacks.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. Another round soon, please! 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". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Turns check project access once with resolveUsableChatProjectID and then load instructions and skills, so resolveProjectInstructions had no production callers. Its tests now cover resolveUsableChatProjectID and loadProjectInstructions directly.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Codex Review: Didn't find any major issues. 🚀 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". |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…coder into chat-project-skills/chatd
There was an error while loading. Please reload this page.
…retry prompt for unusable projects A 403 or 404 from the project skills request means the caller lost access to the project, which the composer already treats as no project skills. The slash menu still showed "Could not load project skills" with a retry hint that could never succeed, so a project the caller cannot use no longer shows a loading or error status. A story covers the menu in that state.
|
@codex review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Here are some automated review suggestions for this pull request.
Reviewed commit: e9a4adf7c6
Your 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".
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| }, | ||
| parameters: { | ||
| queries: [ | ||
| { key: organizationSkills(mockOrganizationId).queryKey, data: [] }, |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Export and use the organization-skills query key
Export organizationSkillsKey and import it here instead of constructing the full query-options object solely to read .queryKey. The current setup couples the story to unrelated fetch configuration and bypasses the repository's canonical query-key pattern.
AGENTS.md reference: site/AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| const projectSkillsQuery = useQuery({ | ||
| ...chatProjectSkills(chatOrganizationId, chatProjectId ?? ""), | ||
| enabled: hasChatProject, | ||
| staleTime: 60_000, | ||
| }); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Extract the page-level project-skill orchestration
Move the added project-skills queries and usability derivation into a focused hook. Adding this request and state orchestration directly to an already roughly 870-line page makes the component harder to navigate and exceeds the repository's component-size guidance.
AGENTS.md reference: site/AGENTS.md:L46-L46
Useful? React with 👍 / 👎.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| const projectSource = useStoredSkillSource( | ||
| chatProjectSkills(organizationId ?? "", projectId ?? ""), | ||
| { hasSource: hasProject, isTriggerActive: hasSkillsTrigger }, | ||
| ); | ||
| const projectQuery = useQuery({ |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Extract the composer project-skill state pipeline
Move the project queries and their loading, error, usability, and filtering derivations into a focused hook. This block adds substantial server-state orchestration to an already roughly 1,167-line editor component, making the file increasingly difficult to maintain.
AGENTS.md reference: site/AGENTS.md:L46-L46
Useful? React with 👍 / 👎.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| }, | ||
| play: async ({ canvasElement }) => { | ||
| await typeInEditor(canvasElement, "/"); | ||
| // Built-in commands appear once the project source has resolved. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Remove the redundant wait narration
Delete this comment because it only restates that the immediately following wait looks for the built-in command; the story-level comment already explains why this state matters.
AGENTS.md reference: site/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.