Rebuild CREW page on ListTemplate with in-body defaults editor - #219
Conversation
|
@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 👍 / 👎.
|
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 👍 / 👎.
|
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 👍 / 👎.
|
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 👍 / 👎.
|
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37f0053db1
ℹ️ 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".
| setFormError(""); | ||
| try { | ||
| if (behaviorDirty) { | ||
| await saveBehavior.mutateAsync({ |
There was a problem hiding this comment.
When behavior and context are both dirty and the context write/delete fails, this awaited mutation has already persisted the behavior and invalidated the config query. The resulting baselineKey update resets filesDraft to the old server context before the second mutation reports its error, so the user sees a failed save after losing the context edits while the behavior changes were still applied. Keep each draft dirty until its own mutation succeeds and report or recover partial completion.
Useful? React with 👍 / 👎.
|
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: 2dcf099a0c
ℹ️ 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".
| setReasoningIndex(initialReasoningIndex); | ||
| setApprovalPolicy(savedPolicy); | ||
| setConfirmDiscard(false); | ||
| setPending(false); |
There was a problem hiding this comment.
When both behavior and context are dirty, saveBehavior.mutateAsync waits for config invalidation; that changes behaviorBaselineKey and this effect sets pending to false even though save() immediately continues into saveContext.mutateAsync. During a slow context write the controls therefore re-enable, allowing another save or new edits; the later context refetch can overwrite those edits and concurrent mutations can race. Leave pending-state ownership with save()'s finally block rather than resetting it here.
AGENTS.md reference: AGENTS.md:L27-L28
Useful? React with 👍 / 👎.
|
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: 8b7bbadce3
ℹ️ 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".
| // keyed off the context query alone so a behavior save (or failed partial | ||
| // save) leaves an unsaved context draft intact. | ||
| useEffect(() => { | ||
| setFilesDraft(contextSectionsFromFiles(context.files)); |
There was a problem hiding this comment.
When a user edits one global-instructions section and then confirms deletion of a different saved section, the delete mutation invalidates and refetches the context query, changing dataUpdatedAt; this effect then replaces the entire filesDraft with the saved baseline and silently discards those unrelated unsaved edits. The immediate-delete flow should re-baseline only the deleted file while retaining the remaining draft changes.
Useful? React with 👍 / 👎.
|
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: 05ac60ebd1
ℹ️ 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".
| const contextEditable = !context.resource.isLoading | ||
| && !context.resource.isUnavailable | ||
| && !context.resource.isError; | ||
|
|
There was a problem hiding this comment.
When the accounts query resolves before useConsoleConfig, this component is mounted with config = [] and derives blank model, fallback, and reasoning baselines plus the built-in approval policy. If the user changes any field and saves before the config query finishes, saveConsoleAgentBehavior writes every behavior field with includeEmpty: true, clearing unrelated persisted overrides such as fallback, reasoning, and approval. Keep the new defaults editor unavailable until the config resource has loaded successfully, rather than treating an in-flight/error result as an empty configuration.
Useful? React with 👍 / 👎.
|
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: 462db51f0e
ℹ️ 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".
| const startEditing = (index: number) => { | ||
| setEditingIndexes((current) => new Set([...current, index])); | ||
| }; |
There was a problem hiding this comment.
When a user clicks the pencil on a second saved override—or on any saved override while a newly added row is open—this adds the index without clearing editingIndexes or newIndex, so multiple rows remain expanded even though the redesigned panel promises one open row at a time. Replace the set rather than extending it, and clear the new-row marker when appropriate.
Useful? React with 👍 / 👎.
|
Addressed in @codex review |
|
Codex Review: Didn't find any major issues. 🎉 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". |
- 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
- 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.
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.
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.
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.)
Break the combined EDIT DEFAULTS form into three distinct in-body surfaces: MODEL DEFAULTS (model/fallback/reasoning), DEFAULT PERMISSIONS (tool-approval editor), and GLOBAL INSTRUCTIONS (files, unchanged). The summary card's MODEL/FALLBACK/REASONING rows open model defaults; the PERMISSIONS row opens the standalone permissions surface. - EditDefaultsPanel: per-section titles/descriptions, three-way render split, drop the overrides scroll hack and overridesRef. - AgentToolsPanel: additive hideHeading prop so the permissions surface supplies its own title without duplicating the panel's heading; ConsoleConfigPage keeps the default. - ConsoleCrewPage: PERMISSIONS row opens the new "permissions" section.
…r save Split the single re-baseline effect into behavior-only and context-only effects. A behavior save invalidates the config query, which previously re-baselined filesDraft too — so a failed context write in a both-dirty save would clobber the user's unsaved context edits while the behavior changes stayed applied. The context draft now re-baselines solely off the context query, keeping each draft dirty until its own mutation succeeds.
- EditDefaultsPanel: DELETE on the GLOBAL INSTRUCTIONS section now commits
immediately (persists the file removal on confirm) rather than staging a
draft that needed SAVE — sends the saved baseline minus the target so other
unsaved section edits aren't written as a side effect; restores the section
on a failed write.
- ContextSectionsEditor: add an optional onDeleteSection host hook for that
immediate-delete path (staged draft-until-SAVE remains the default, e.g. the
agent editor); clamp long section-tab labels to two centered lines with
ellipsis + title tooltip so they no longer overflow and collide; make the
delete-confirm note reflect immediate vs staged ('can't be recovered' vs
'you must save for this and any other unsaved changes to take effect').
- EditDefaultsPanel: address codex P1 — 'pending' is owned solely by save()'s
try/finally; the behavior re-baseline effect no longer clears it, so a
behavior write that lands mid-combined-save can't re-enable the controls
while the awaited context write is still in flight.
An immediate delete invalidates the context query; its refetch would fire the context re-baseline effect and replace the whole filesDraft with the server baseline, discarding unrelated unsaved section edits. deleteSection already reconciles the draft optimistically (drops the deleted file, keeps every other edit), so suppress the one re-baseline that the delete's own refetch triggers via a ref flag (cleared on a failed delete so a genuine refetch still re-baselines).
The defaults editor derives its model/fallback/reasoning/approval baselines from config. If accounts resolved before useConsoleConfig, the page rendered with config=[] and a save could clear persisted overrides (saveBehavior writes every field with includeEmpty). Block the roster on an in-flight, errored, or unavailable config resource — while still rendering on a legitimately empty (loaded) config, where blank baselines are correct.
addRule enforces a single expanded row, but startEditing still extended editingIndexes, so pencil-editing a second saved override (or any saved row while a new row was open) left multiple rows expanded. Replace the set with the clicked index and clear the new-row marker so exactly one row is open.
23cc20e to
4bc817e
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bc817e734
ℹ️ 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".
| if (!initialTarget || !initialPath) { | ||
| return; |
There was a problem hiding this comment.
When the Files surface receives a bare /files route after a deep link, this early return leaves appliedLocationRef set to the previous /files/<target>/<path> key. If the user browses elsewhere in the Files UI or opens bare Files, then later opens that same deep link again, the guard below treats it as already applied and refuses to move the browser tab back to the linked path. Reset appliedLocationRef.current in this branch so a route that was cleared can be applied again.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
To use Codex here, create a Codex account and connect to github.
2 participants
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.