Repository navigation
Conversation
The project dialog gets a templates multi-select with Select all, limited to templates that allow agents, and a default template with either a default preset or default parameter values. On a project page, the composer's workspace picker hides workspaces whose template is not one of the project's templates.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca890071c6
ℹ️ 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".
| (workspace) => | ||
| !project || | ||
| project.templates.template_ids.includes(workspace.template_id), |
There was a problem hiding this comment.
Filter workspaces for composer-selected projects
When a project is selected from the general /agents composer, project remains undefined because that selection is held as pickedProject inside AgentCreateForm, so this predicate leaves every workspace available. Selecting a workspace whose template is outside the chosen project's template_ids then sends both IDs and the server returns 400 instead of creating the chat. Filter against the internally selected project as well, or move the template filtering into AgentCreateForm.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a9b1e9a. The filter moved into AgentCreateForm and applies to the route's project or a project picked in the composer, so a saved workspace outside the picked project's templates is dropped before submit. Covered by two new tests in AgentCreateForm.test.tsx.
| <Input | ||
| id={id} | ||
| value={value.defaultParameters[parameter.name] ?? ""} | ||
| placeholder={parameter.default_value} |
There was a problem hiding this comment.
Validate template parameter defaults before saving
For numeric, option-constrained, required, or regex/range-validated parameters, this plain text input accepts arbitrary strings, while the project endpoint only checks that the parameter name exists. An invalid value is therefore persisted and later passed to workspace creation, where parameter resolution rejects it and agent workspace creation fails. Render and validate the TemplateVersionParameter metadata using the existing parameter-control machinery before allowing Save.
AGENTS.md reference: site/AGENTS.md:L41-L42
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed across the stack: the API now validates default parameter values with codersdk.ValidateWorkspaceBuildParameter (options, required, regex, and range) in f68e904, and the dialog shows template validation errors in the templates section (a9b1e9a, story EditTemplatesSaveError), since no form field matches those validation names.
| {templatesError ? ( | ||
| <p className="m-0 text-sm text-content-destructive"> | ||
| {getErrorMessage(templatesError, "Failed to load templates.")} | ||
| </p> | ||
| ) : templates ? ( |
There was a problem hiding this comment.
Preserve cached controls on refetch errors
If a background refetch fails after React Query has cached valid template data, this error-first branch hides the populated selector even though templates remains available; the preset and parameter branches repeat the same ordering. A transient focus refetch can therefore make all configuration controls disappear mid-edit. Prefer cached data when present and surface the refetch error alongside it. .claude/docs/FRONTEND_PATTERNS.mdL200-L202
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a9b1e9a: the templates, presets, and parameters sections now prefer cached data and only show the error when nothing is cached.
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. |
…er and dialog errors Filter the composer's workspaces for a project picked in the composer, show template validation errors in the project dialog, and keep cached template data visible when a refetch fails.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9b1e9a905
ℹ️ 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".
| </Button> | ||
| </div> | ||
| {templates ? ( | ||
| <MultiSelectCombobox |
There was a problem hiding this comment.
Give template removal controls accessible names
When one or more templates are selected, MultiSelectCombobox renders each chip-removal control and the clear-all control as icon-only buttons without accessible names, so screen-reader users cannot determine which template a button removes or distinguish it from clearing every selection. Add labels such as Remove ${option.label} and Clear all selections to the shared primitive.
AGENTS.md reference: site/AGENTS.md:L12-L12
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The icon-only chip and clear-all buttons are part of the shared MultiSelectCombobox primitive and affect every user of it, so I'd rather fix that in a separate PR for the component than in this feature PR. The combobox itself is named through cmdk's label.
| parameters: { | ||
| // Every story with an organization loads its templates. | ||
| queries: [MockDefaultOrganization, MockOrganization2].map( | ||
| (organization) => ({ | ||
| key: agentTemplates(organization.id).queryKey, |
There was a problem hiding this comment.
Add stories for the template query states
Because this meta-level fixture supplies a successful, nonempty template response to every organization story, Pixel never exercises the new top-level loading, request-error, or empty-result branches. Regressions in those states can therefore merge unseen; add dedicated stories that configure each query state explicitly.
AGENTS.md reference: site/AGENTS.md:L73-L73
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Added in 80c7813: NoEligibleTemplates (empty list, opened) and TemplatesLoadError (request rejected). The loading branch is a single skeleton that Pixel cannot capture deterministically without a never-resolving request, so I left it out.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80c78138c3
ℹ️ 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".
| default_preset_id: values.defaultPresetId || undefined, | ||
| }; | ||
| } | ||
| const parameters = Object.fromEntries( | ||
| Object.entries(values.defaultParameters).filter(([, value]) => value), |
There was a problem hiding this comment.
Reconcile defaults with the active template version
When a template publishes a new active version that removes a stored parameter or preset, the corresponding query no longer renders that default, but the form state retains it. Changing any template setting then serializes the hidden stale value here, and validateChatProjectDefaults rejects the save as an unknown parameter or invalid preset; reconcile stored defaults against the fetched active-version metadata before submission.
AGENTS.md reference: site/AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
| key: templateVersionPresetsKey(dockerTemplate.active_version_id), | ||
| data: [smallPreset], |
There was a problem hiding this comment.
Cover the nested default-query states
The newly added preset and parameter sections each render distinct empty and request-error branches, but these fixtures cover only successful, nonempty responses. Unlike the now-covered top-level template query states, failures or empty results from the preset and rich-parameter endpoints still have no Pixel baseline; add stories for those reachable nested states.
AGENTS.md reference: site/AGENTS.md:L73-L73
Useful? React with 👍 / 👎.
| } from "#/testHelpers/entities"; | ||
| import { ChatProjectDialog } from "./ChatProjectDialog"; | ||
|
|
||
| const dockerTemplate: Template = { |
There was a problem hiding this comment.
Prefix the new entity fixtures with mock
The new entity fixtures use names such as dockerTemplate, kubernetesTemplate, smallPreset, and projectWithDefaultPreset, contrary to FE9's required Mock* fixture convention; the analogous additions in the test file follow the same pattern. Rename these local variants with a mock prefix so fixtures remain recognizable and consistent with shared test entities.
AGENTS.md reference: site/AGENTS.md:L15-L15
Useful? React with 👍 / 👎.
| } from "#/components/Select/Select"; | ||
| import { Skeleton } from "#/components/Skeleton/Skeleton"; | ||
|
|
||
| export type ChatProjectTemplatesValues = { |
There was a problem hiding this comment.
Document the exported template form symbols
The exported ChatProjectTemplatesValues type lacks JSDoc, as do the newly exported chatProjectTemplatesValues helper and ChatProjectTemplatesFields component below it. Add concise contract-focused documentation for these exports as required by the repository's exported-symbol rule.
AGENTS.md reference: AGENTS.md:L87-L87
Useful? React with 👍 / 👎.
…pick templates only when creating a project The edit dialog goes back to the name, description, and icon. Templates are edited from the project page instead.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f42e6a8a2c
ℹ️ 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".
| <p className="m-0 text-sm text-content-destructive"> | ||
| {getErrorMessage(templatesError, "Failed to load templates.")} | ||
| </p> |
There was a problem hiding this comment.
Announce asynchronous query failures
When the templates request fails after the dialog opens, this plain paragraph replaces the loading skeleton without a live-region semantic, so screen-reader users are not notified that loading failed. The preset and parameter error branches repeat the same behavior; render these failures through the shared alert primitive or add an appropriate role="alert"/live region.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 75ea290: the template, preset, and parameter load errors now render with role="alert".
| }, | ||
| }; | ||
|
|
||
| export const NoEligibleTemplates: Story = { |
There was a problem hiding this comment.
Snapshot configured template states
The added stories cover an unconfigured form plus top-level empty and error results, but none selects templates and a default, so Pixel never renders the new default-template selector, preset selector, parameter mode, or parameter inputs. Add interaction stories that stop in those configured states; otherwise visual regressions in most of the new UI can merge unnoticed. This is separate from the existing nested empty/error-state finding.
AGENTS.md reference: site/AGENTS.md:L73-L73
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The configured states (default template, preset, parameter values, save error) are now edited from the project page, and #30898 snapshots them in ProjectTemplatesDialog.stories.tsx, which renders this same ChatProjectTemplatesFields component.
…ntil a story uses it
…nce template load errors, even with cached templates
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75ea290ce1
ℹ️ 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".
| // Every story with an organization loads its templates. | ||
| queries: [MockDefaultOrganization, MockOrganization2].map( | ||
| (organization) => ({ | ||
| key: agentTemplates(organization.id).queryKey, |
There was a problem hiding this comment.
Export the agent-template query key
The story constructs agentTemplates(...) solely to extract .queryKey here and again in NoEligibleTemplates, rather than importing a dedicated key helper. Export an agentTemplatesKey(organizationId) beside the query definition, use it inside agentTemplates, and import that key in the story so production and mock wiring share one query-key API.
AGENTS.md reference: site/AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
| onSubmit: fn(), | ||
| }, | ||
| parameters: { | ||
| // Every story with an organization loads its templates. |
There was a problem hiding this comment.
Remove the comment that restates the fixture wiring
This comment only narrates that the meta-level query fixture applies to the stories below, which is already evident from its placement and the organization mapping. Remove it so the added comments remain limited to non-obvious constraints or rationale.
AGENTS.md reference: site/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
Adds template selection to the create project dialog and filters the composer's workspace picker on project pages.
Stack (merge top to bottom):