Rebuild CREW page on ListTemplate with in-body defaults editor#219
Rebuild CREW page on ListTemplate with in-body defaults editor#219jess-cat wants to merge 5 commits into
Conversation
- CREW (/crew, /settings/crew) rebuilt onto the shared ListTemplate: agents-only rows, a DEFAULTS summary card, and an in-body EditDefaultsPanel that replaces the roster (with a "← AGENTS" back link + ✕) - Extract shared ContextSectionsEditor from AgentEditor's CONTEXT tab (file tabs, section editor, per-section delete); AgentEditor now consumes it - AgentToolsPanel: DEFAULT PERMISSIONS section title + description, TOOL APPROVAL and OVERRIDES as field labels, ADD OVERRIDE link, context-aware default copy, shared agentToolApprovalOptions - ListTemplate: opt-in scrollBody (bounded scroll chain), listContent, actionExtra and leading slots - Mobile: compact DEFAULTS disclosure with auto-collapse on open, 44px tap targets, responsive reflow at 1024/480 - a11y/DS: restore ListRow focus ring, Segmented ariaLabel, IconButton edit glyph, tokenized ContextSectionsEditor colors
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60a42f20eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| label="ADD OVERRIDE" | ||
| disabled={disabled} | ||
| onClick={() => onChange(addRule(policy))} | ||
| disabled={disabled || hasNewRow} |
There was a problem hiding this comment.
In the new-agent flow, this panel does not get an external baseline/save update before the user clicks CREATE, and newIndex is only cleared by removing the row or by an external policy reset. After the first ADD OVERRIDE, hasNewRow therefore stays true and this disables the only control for adding another rule, so users can no longer create an agent with multiple tool-approval overrides in one draft.
AGENTS.md reference: AGENTS.md:L11-L14
Useful? React with 👍 / 👎.
| if (!initialTarget || !initialPath) { | ||
| return; | ||
| } | ||
| const locationKey = `${initialTarget} ${initialPath}`; |
There was a problem hiding this comment.
This template literal embeds an actual NUL byte in the .tsx file. With this byte present, normal rg searches report FilesSurfaceSummary.tsx as binary and skip text matches unless --text is used, so future repo-wide searches and scripted edits can silently miss this component; use an escaped delimiter such as \u0000 instead.
Useful? React with 👍 / 👎.
- AgentToolsPanel: relax the ADD OVERRIDE guard so it's never disabled - adding an override collapses the current row to a read-only summary and opens a fresh one on top, so multiple overrides can be staged in a single draft (fixes the create-agent single-override cap). Still keeps one row expanded at a time. - FilesSurfaceSummary: replace the raw NUL byte in the deep-link locationKey separator with an escaped backslash-u-0000 so tools do not treat the file as binary.
|
Addressed both findings in
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ed8d66714
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| setModelIndex(initialModelIndex); | ||
| setFallbackIndex(initialFallbackIndex); | ||
| setReasoningIndex(initialReasoningIndex); | ||
| setApprovalPolicy(savedPolicy); |
There was a problem hiding this comment.
In the CREW defaults editor, if a user changes the TOOL APPROVAL default and then clicks RESET, this only restores approvalPolicy in state; AgentToolsPanel renders Segmented, which keeps its own selState after the first click and ignores later value prop changes. The draft becomes clean and SAVE is disabled, but the segmented control can still display the discarded permission, making the visible default approval state wrong until the user manually clicks another segment. Consider forcing the tools panel/segmented control to remount on reset or making Segmented fully controlled when value is supplied.
Useful? React with 👍 / 👎.
Segmented cached its local selState after the first click and ignored later `value` prop changes, so a RESET in the CREW defaults editor restored the draft (SAVE disabled) but left the TOOL APPROVAL segment displaying the discarded selection. Reset selState whenever `props.value` changes so the control stays controlled when the parent drives it; uncontrolled callers pass a static value and are unaffected.
|
Addressed in @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0633b87a68
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return currentTabs.map((tab) => tab.id === linkedTab.id && tab.kind === "browser" | ||
| ? { ...tab, path: linkedTab.path, commandInput: "", commandInputKey: tab.commandInputKey + 1, searchQuery: "" } | ||
| : tab); | ||
| }); | ||
| setActiveTabId(linkedTab.id); |
There was a problem hiding this comment.
This route-driven navigation updates the browser tab directly, unlike the normal navigateBrowserTab path below that clears createState and delete feedback. If a user has CREATE NEW open with a filename/content and then follows a /files/<target>/<path> deep link or browser back/forward entry to another directory, the old create draft remains visible against the new tab's base path, so pressing CREATE can write that stale draft into the wrong directory. Clear the create/delete transient state when applying the deep link, or route this through the same navigation helper.
Useful? React with 👍 / 👎.
The deep-link effect repointed the active browser tab at a new target/directory but, unlike navigateBrowserTab, left createState and deleteFeedback intact. An open CREATE draft could then resolve against the newly-linked directory and write into the wrong place. Clear both when applying a deep link, matching the normal navigation path.
|
Addressed in @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48bdd88ce7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| rows={rows} | ||
| emptyObject="AGENTS" | ||
| connectLabel="NEW AGENT" | ||
| onConnect={onCreateAgent} |
There was a problem hiding this comment.
When the CREW page is rendered as /settings/crew, this onCreateAgent prop is openSettingsNewAgent, which calls navigateSettingsRoute directly instead of guardedSettingsNavigate. Because this change adds an in-body defaults editor while leaving the action bar mounted, a user can edit defaults/global instructions and then click NEW AGENT; that immediately unmounts the dirty editor and drops the draft without the unsaved-changes prompt. Route this action through the unsaved guard (or disable/hide it while the editor is dirty/open) for the settings-crew context.
Useful? React with 👍 / 👎.
In /settings/crew the action bar (incl. NEW AGENT) stays mounted alongside the in-body defaults editor, but onCreateAgent called navigateSettingsRoute directly, bypassing the unsaved-changes guard. Editing defaults/global instructions then clicking NEW AGENT dropped the draft with no prompt. Route it through guardedSettingsNavigate so a dirty editor prompts first. (The non-settings /crew context already routes NEW AGENT through the guarded openShellSurface.)
|
Addressed in @codex review |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Summary
Rebuilds the CREW page (
/crew,/settings/crew) onto the sharedListTemplatewith an in-body defaults editor, plus the surrounding extraction/DS work.What changed
ListTemplate: agents-only rows, a DEFAULTS summary card in the action column, and an in-bodyEditDefaultsPanelthat replaces the roster (with a← AGENTSback link + ✕). Every summary row clicks through to its editor section (model/fallback/reasoning → defaults, PERMISSIONS → overrides, GLOBAL INSTRUCTIONS → context); one unified SAVE persists behavior + approval + context together.ContextSectionsEditorextracted from AgentEditor's CONTEXT tab (file tabs, section name + content editor, per-section delete w/ confirm). AgentEditor now consumes it via anactionsslot.AgentToolsPanelrestructured:DEFAULT PERMISSIONSsection title + description,TOOL APPROVALandOVERRIDESas field labels (matching the create-agent form rhythm),ADD OVERRIDElink, context-aware default copy, sharedagentToolApprovalOptions.ListTemplategained opt-inscrollBody(bounded scroll chain so the list body scrolls while the action column stays anchored), pluslistContent/actionExtra/leadingslots.!importantoverride),SegmentedariaLabel,IconButtoneditglyph, tokenizedContextSectionsEditorcolors.Testing
Known non-issue
ChatTranscript.test.tsfails at import (DOMPurifyaddHookunder vitest) — pre-existing onmain, unrelated to this branch.