Repository navigation
🚦 feat: Show Exit Status and Stderr for Attached Workspace Commands - #16556
Conversation
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99996f288a
ℹ️ 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".
| const TRAILER = | ||
| /\n((?:\[(?:exit code: -?\d+|terminated by [\w+-]+|timed out|output truncated)\])+)$/; |
There was a problem hiding this comment.
Distinguish sandbox output before parsing exit trailers
For a sandbox bash call whose final stdout line is an allowed marker, such as echo '[exit code: 1]', the server's cleanCodeToolOutput() removes the trailing newline before persistence, producing exactly stdout:\n[exit code: 1]. This regex therefore accepts ordinary sandbox output as attached-workspace metadata, and both BashCall and the group summary incorrectly report a successful sandbox command as failed. The forged-trailer test retains a final newline, so it does not cover the persisted sandbox shape; trailer parsing needs executor provenance or other unforgeable metadata rather than relying solely on this suffix.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7aae1482d. Parsing no longer trusts the suffix: tools built by createAttachedWorkspaceBashTool are registered by object identity, the execute handler records the run step when it resolves that instance, and the step completed callback stamps executor: 'attached_workspace' on the streamed and persisted tool call. BashCall and outcome.ts only parse the trailer when that marker is present, so sandbox output of exactly 'stdout:\n[exit code: 1]' stays successful. Tests cover both cases on the card and the group outcome, plus a server spec that the marker is stamped for the attached tool and not the sandbox bash_tool.
7aae148 to
66a0d8e
Compare
Parse the exit trailer the attached-workspace bash_tool writes so the card fails a non-zero exit, a signal or a timeout, names the reason beside the failure suffix, and renders stderr distinctly. The group outcome reads the same parser so its failed count matches the card. Sandbox output without the trailer keeps the existing heuristics.
The sandbox and attached-workspace tools are both named bash_tool, and the sandbox output is trimmed, so a sandbox command that printed "[exit code: 1]" last was read as a failed attached-workspace run by the card and the group outcome. The execute handler now records a run step when the resolved tool instance is the one createAttachedWorkspaceBashTool built (tracked by object identity), and the step-completed handler stamps executor: 'attached_workspace' on the streamed tool call and the aggregated part. Redis replay and the client SSE merge carry the field through. BashCall and outcome.ts parse the trailer only when the marker is set.
66a0d8e to
4183555
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4183555194
ℹ️ 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".
| runStepDurationMs={toolCall.runStepDurationMs} | ||
| backgrounded={toolCall.backgrounded} | ||
| backgroundCancelled={toolCall.backgroundTask?.cancelled === true} | ||
| executor={toolCall.executor} |
There was a problem hiding this comment.
Preserve executor through subagent activity adapters
When an attached-workspace bash call runs inside a subagent, this prop is always lost before the card is rendered: reduceSubagentContent in client/src/utils/subagentContent.ts copies selected completion fields without executor, contentPartsToActivity in client/src/components/Chat/Subagents/adapters.ts omits it again, and the persisted SubagentActivityItem/Mongo projection has no field for it. Consequently both live and restored subagent panels pass undefined here, so BashCall skips trailer parsing and an exit such as [exit code: 1] is shown as completed rather than failed. Carry the provenance through the live and durable subagent activity shapes and reconstruction path.
AGENTS.md reference: AGENTS.md:L42-L45
Useful? React with 👍 / 👎.
| * status trailer; sandbox output keeps the text heuristic even when it | ||
| * prints something that looks like one. */ | ||
| const result = useMemo( | ||
| () => (executor === 'attached_workspace' ? parseCommandOutput(output) : null), |
There was a problem hiding this comment.
Preserve exit trailers while cleaning command output
When attached-command stdout contains a blank-line-delimited Note: Files from previous executions... or Note: Files in "Available files"..., the foreground and background execution paths run cleanCodeToolOutput() before this parser receives the output. Its TRAILING_NOTES_PATTERN consumes from that note through end-of-string, which now also removes the formatter's real [exit code: N], signal, and timeout trailer. A non-zero command with otherwise benign output therefore makes parseCommandOutput() return null and is again presented as completed; preserve/reappend the trailer during cleanup or skip that sandbox-specific cleanup for the identified attached tool.
AGENTS.md reference: AGENTS.md:L42-L45
Useful? React with 👍 / 👎.
Canary's #16556 calls stampCommandExecutor from callbacks.js, but this spec's @librechat/api mock predates it, so three cases failed on canary itself. The mock now carries the real helper, as it already does for isCodeArtifactToolOutput.
Canary's #16556 calls stampCommandExecutor from callbacks.js, but this spec's @librechat/api mock predates it, so three cases failed on canary itself. The mock now carries the real helper, as it already does for isCodeArtifactToolOutput.
…ibreChat-AI#16556) * feat: Show Exit Status for Attached Workspace Commands Parse the exit trailer the attached-workspace bash_tool writes so the card fails a non-zero exit, a signal or a timeout, names the reason beside the failure suffix, and renders stderr distinctly. The group outcome reads the same parser so its failed count matches the card. Sandbox output without the trailer keeps the existing heuristics. * fix: Parse Exit Status Only for Attached Workspace Calls The sandbox and attached-workspace tools are both named bash_tool, and the sandbox output is trimmed, so a sandbox command that printed "[exit code: 1]" last was read as a failed attached-workspace run by the card and the group outcome. The execute handler now records a run step when the resolved tool instance is the one createAttachedWorkspaceBashTool built (tracked by object identity), and the step-completed handler stamps executor: 'attached_workspace' on the streamed tool call and the aggregated part. Redis replay and the client SSE merge carry the field through. BashCall and outcome.ts parse the trailer only when the marker is set. * fix: Count Styled Terminal Lines Like Plain Ones When Output Ends in a Newline (cherry picked from commit abc9285) Original-PR: LibreChat-AI#16556
* 🪟 feat: Undock the Artifacts Pane into Its Own Window The artifacts pane could only live in the chat's side panel, so viewing an artifact meant giving up half the conversation and there was no way to put it on a second screen. A header action now moves the pane into a separate browser window and back. The pane stays in the chat tab's React tree and is portaled into the popup's document, so artifacts keep streaming into it and docking back is a re-render rather than a reload. Everything a portal does not carry is mirrored: the host stylesheets (re-synced for lazy chunks, HMR and CSSOM-only rules), the resolved theme, a base URL for relative assets, and Sandpack's frame messages, which would otherwise never reach the listener the opener installed. * 🪟 fix: Keep the Artifacts Pane Whole When It Changes Windows Review of the undock change surfaced one cause behind most of its findings: the pane is a new component instance in every host, so anything a mount decides was lost or wrongly reset when it moved. - The code editor buffer now carries the artifact it belongs to, so a remount restores unsaved text instead of falling back to the persisted content, and the edit the debounce never got to send is handed to the mutation on mount. The share hosts take one provider around every branch for the same reason. - The artifact registry is cleared by the host that owns it: the pane keeps it while it only changes windows, and leaving the route wipes it, so no artifact follows the user to a shared conversation. - Fullscreen exits through the document that owns the container, Mermaid pan listens where its canvas lives, and Copy reports success only once the write resolves, with a toast when it does not. - Style mirroring compares what a sheet actually contains: rules a page inserts through the CSSOM are serialized, and a rewrite the observer saw refreshes its clone even when nothing measurable changed. - A child-activity panel opened while the pane is undocked stays open; the two panels are exclusive only while they compete for the side panel. Adds the scenario suite for the window behaviors (`@scenario:` tagged) plus unit coverage for the registry lifetime, the panel slot and buffer ownership. * 🧭 fix: Carry the Artifacts Tab Across a Window Change The pane's active tab lived in the component instance, so undocking from the code tab landed the user back on the preview — with their unsaved text out of sight, which reads as losing it. The tab is pane state now, next to the editor buffer it belongs with, and the scenario asserts both survive the move. * 🧹 chore: Drop Unused Imports From the Presentation Spec * 🎯 fix: Bound the Artifacts Pane's Shared State to Its Session The second review round found the cost of state that outlives an instance, all in the seams the first round opened. - Download now reads the editor buffer only when it belongs to the artifact on screen, so selecting another artifact cannot export the previous one's edits. - The active tab is reset when the pane session ends — close or conversation change — instead of following a later artifact onto the code tab. - A mutation started by a previous pane instance can no longer report back, so the mount releases the stale "mutating" flag before handing over the restored text; otherwise the edit queued behind a completion that never arrives. - The undocked window's size and position are read while it still has them, in its `pagehide`, so closing through the browser remembers the bounds too. - Docking from the window moves focus to the control that replaces the one the user pressed, which a keyboard user otherwise loses to the document. - The share host keeps one `EditorProvider` mounted across the closed and open branches, so opening the overlay no longer remounts the transcript. * 🎨 fix: Paint the Undocked Window Before Its Sheet Arrives A mirrored stylesheet is a fetch, so the popup showed its default white canvas for that moment — plain against a dark app, and visible to the scenario as a transparent pane. The host's resolved surface colour is now the `var()` fallback in the popup's own layout sheet, so the window opens on the app's canvas and the real token takes over when it lands. The two scenarios that read settled state — the painted canvas, and a keyboard Enter that closes the window it was pressed in — now wait for it instead of sampling the first frame. * 🔭 fix: Focus the Dock Target Once the Pane Has Mounted\n\nThe focus handoff ran before the pane rendered its toolbar, so the control it\nwas meant to focus did not exist yet and the request was never spent. It now\nwaits for the mount that creates that control. * 🧩 fix: Give the Edit Pipeline and the Transcript One Owner Each Round three found the same shape three more times: work that belongs to a session was still decided by an instance or by a branch. - The editor no longer infers whether a previous instance's save is running. React Query keeps a mutation's callbacks on the mutation, so that request settles the shared flag itself; the new instance waits for idle and submits the restored buffer once, instead of clearing a flag it does not own and resubmitting against an `original` the earlier request may have replaced. - The shared conversation keeps its transcript in one place — always the first panel of the resizable group — so opening the pane, undocking it or docking it again changes only the pane's host, not the transcript's ancestry. - The clipboard fallback runs in the pane's own document, so an undocked pane on a deployment without the Clipboard API can still copy instead of only reporting failure. * 🧷 fix: Tie the Editor Buffer and Focus to the Pane Session Round four found the last places where the retained buffer and the focus handoff were decided by a code path rather than by the session boundary. - Closing the pane discards the buffer, the tab and any pending focus request, so reopening an artifact starts from what is saved instead of restoring — and silently resubmitting — text the user abandoned. - Only the buffer an instance inherits at mount is drained. Navigating back to an artifact restores its own unsaved text in the editor instead of writing the persisted content over it, which used to queue that content behind the edit and undo it. - Closing the window through the browser hands focus back exactly as the Dock control does, rather than leaving a keyboard user on the document body. - The clipboard fallback restores focus by capability rather than `instanceof`, which never matches across the popup's realm. * 🔐 fix: Make Undocking Configurable and Its Saves Session-Bound - `interface.artifactUndocking` is a `configSchema` field, default enabled, so a deployment that cannot use popups keeps the docked pane. The hosts resolve it — the chat host from the startup config, the shared host from the shared-link config, which now carries the flag — and hand it to the pane through the context it already takes, rather than the pane reading config itself. - A save carries the editing session it was started in. React Query keeps its callbacks after the editor is gone, and those callbacks used to write the buffer a closed session had just cleared and submit its queued edit; they now stop at a session boundary, so a closed pane mutates nothing and leaves no text behind for the next one. * ✂️ fix: Stop the Editor from Overwriting Another Artifact's Unsaved Text - A write this component makes into the Monaco model comes back through `onChange` exactly like a keystroke. Restoring an artifact's text therefore looked like typing and keyed the shared buffer to the artifact now on screen — so selecting another artifact before the debounce fired replaced the text the first one was still holding, and returning to it showed the persisted content. The value written here is now recognised and consumed, and the retained buffer is sent deliberately when its artifact comes back, which is where its cancelled debounce finally becomes a save. - The save lock is the session's, not a request's. A save that outlives its session no longer clears the lock or the failure notice a later session took, and ending a session releases it — while an edit typed behind a lock whose owner is gone is sent by the editor holding it as soon as the lock is free, instead of waiting for the user to type again. * 🔔 fix: Report Failures in the Window the User Is Looking At The app's toast surface is mounted once in the chat tab, so a notice raised from the undocked pane — a copy that no clipboard path accepted, for instance — appeared in a document nobody was looking at. The window now carries a Radix toast provider and viewport of its own around the portaled pane; the toast state is shared, so the same notice is shown wherever the user is rather than routed by hand at each call site. * 🚧 fix: Offer Undocking Only Once the Deployment Has Answered An absent config read as enabled, which is the default, but a config request in flight reads the same way: a user could undock during that window and the arriving answer would then take the Dock control away with it. Both hosts now treat an unanswered config as no capability, and the pane keeps the control that docks it whenever it is undocked, so a capability that disappears cannot strand the pane in a window with no way back. * 🎯 fix: Land Queued Edits, the Config Toggle and Focus on the Window - A queued edit is sent against the content the artifact actually holds. The save that held the lock has already replaced what that edit was typed against, so submitting the captured `original` had the endpoint reject the newest text and leave it in the buffer alone. - `interface.artifactUndocking` now survives interface loading: `loadDefaultInterface` rebuilds the object property by property, so the flag never reached `/api/config` or the shared-link payload and a deployment that set `false` still saw the control. - Undocking hands focus to the pane the way docking already did. The control the user pressed goes away with the docked toolbar, so a keyboard user was left on the empty document of the window that had just opened; the focus request is now a move in either direction, which is what it is named after. * 🔓 fix: Let the Request Own the Save, Not the Session The save lock was a flag the editor set and cleared, which gave two ways to be wrong: a callback whose session had ended cleared a lock the next session was waiting on, and ending a session released a lock whose request was still running — so a pane reopened mid-save could start a second read-replace-save against content the first request was about to change. The state is now the request's. The artifact save carries a mutation key and `isMutating` is read from React Query's mutation cache, so it is true exactly while a save is in flight, whoever started it: nothing to release, nothing to leave behind, and an editor that appears mid-save queues its edit and sends it — rebased on the content that landed — once the request settles. * ⏱️ test: Give the Undocked Preview Time to Compile * 📌 fix: Rebase a Queued Edit on What the Save Actually Wrote The artifact registry catches up when the edited message propagates, not when the save returns, so an edit sent in between was rebased on content the server had already replaced and the endpoint rejected it. The text the last save wrote is on the request itself: the drain reads it from the keyed mutation in React Query's cache and falls back to the registry only when there is no save to speak of. The same applies to a buffer inherited at mount, which is submitted against that record too. * 🧷 fix: Keep the Pane's Tab and Its Storage Off the Instance Both findings of the review at a03ee0f are the same shape as the ones before them: something the pane needs was reached for where it does not survive. - The tab is pane state, but the reset that keeps it honest was an instance ref. A host change mounts a new pane, so that ref started empty, read the artifact already on screen as freshly opened and wrote `preview` back — undocking and docking both returned the user to the preview, which reads as the code they were editing having been thrown away. The marker moves to the atom beside the tab it guards, and ends with the pane session like the rest of it. Opening a *different* artifact still returns to the preview. - `localStorage` was reached for at the call site. A policy that denies Web Storage throws from the getter itself, before the guard around `getItem` could run, so undocking threw instead of falling back to default bounds and the redock and teardown paths threw on their way home — stranding the pane in a window whose control the user had just pressed. Both helpers take the window and touch storage inside their own guard. The scenario that should have caught the tab defect asserted `aria-checked` once, which matches the frame before the reset lands, so it is polled now. * 🧪 test: Drop the Denied-Storage Scenario the Shell Cannot Support Verification at c94e5ef showed the scenario asserting a capability LibreChat does not have: denying Web Storage in the browser replaces the page with the error boundary ("denied") before the pane's controls are reachable, so the popup never opened and the test timed out in all three projects. The finding behind it was about one call site — the pane reaching for `source.localStorage` outside the guard around its `getItem` — and that stays fixed and covered by the unit tests in `undockedWindow.spec.ts`, which reproduce the throwing getter and fail without the fix. Surviving denied storage app-wide is the shell's problem, not the pane's, and is tracked at berry-13#41. The spec says why the scenario is absent so it is not added back. * 🔗 fix: Match the Artifact Trigger's Name as Dev Now Builds It The scenarios opened the artifact by its full accessible name, spelled out as "<title> Click to open". Dev's "Unify Artifact Triggers as Tool Rows" (#15915) folded the format badge and whether the row renders a preview into that name, so it now reads "E2E HTML Artifact HTML Opens as a rendered preview Click to open" and an exact match finds nothing. Every scenario in the file failed at its first click, three retries deep, which is what pushed the e2e shard past its 30 minute ceiling. This did not show up locally because the mock harness serves `client/dist`, and mine was built before the rebase. The old bundle still rendered the old name, so the stale locator passed here and failed in CI — and dev's own `plain-text-artifact-row` spec failed locally for the mirror-image reason. Both pass against a freshly built client. The trigger is matched on its title now, the way dev's spec does it, so the badge and the preview hint can change again without breaking the file. * 🍞 fix: Share One Toast Viewport Between the App and the Window The undocked artifacts window needs a toast viewport of its own, and it carried its own copy of the app shell's class list — so any change to toast placement, stacking or width would have had to be made twice, or the popup's notices would drift from the app's.\n\nThe viewport is now one composition in \, rendered by both hosts. It keeps the class list it always had and still takes a \, so the popup can adjust its own placement without forking the primitive. * 🚫 fix: Keep a Rejected Edit Rejected Across a Host Change A save the endpoint answers with 400 is deterministic, and the editor already refuses to send that exact text again. The refusal lived in the editor instance while the text itself lives in the shared buffer, so moving the pane to another host — which remounts the editor — dropped the marker and the inherited-buffer drain immediately sent the rejected text back, against the same original that produced the 400. Every dock or undock cost another rejected request, with no new edit from the user.\n\nThe marker now sits beside the buffer it describes, in the editor session, and names the artifact it was recorded for. A remount inherits it; a successful save clears it; ending the session clears it with the buffer. Returning to an artifact no longer resends its rejected text either, for the same reason. A genuine new edit still saves. * 🧭 fix: Key the Artifact Registry to the Host's Conversation The guard that bounds the artifact registry to one conversation decided that the conversation had changed by reading the chat tab's Recoil slot, which the shared route never sets. A shared conversation's host stays mounted while the reader moves between shared links, so on that route the change was invisible and only the unmount cleanup ever fired: artifacts from the previous link stayed in the registry, and opening an artifact in the next one could list and navigate back to the previous share's content.\n\nThe identity is now the host's to supply — the chat tab passes its Recoil slot, a shared conversation passes its own id — which is also what keeps the hook from reaching into app-global state for something only its caller knows. The first observed identity still does not wipe, so the loading transition into a shared conversation keeps the artifacts that belong to it. * 🧪 test: Cover the Shared Host, a Blocked Popup and a Rejected Save Three paths the pane reaches that no scenario exercised: a shared conversation, whose host resolves the undock capability from the shared-link config; a browser that refuses the popup, where the pane has to stay put and say so; and an edit the endpoint rejected, which must not go back on the wire because the pane changed hosts. * 📝 docs: Document the Artifact Undocking Toggle in the Example Config \ reached the schema and the shared-link config without ever appearing in the example file a deployment copies, so the lever existed but was undiscoverable. Documented with the default that reproduces today's behavior. * 🔐 fix: Make the Undock Capability the Host's Answer, Not a Default \ filled \ with \ when a host left it out, so a host that forgot would offer a window the deployment may have turned off — the one field in that context with no sensible fallback. It is now required, which makes forgetting it a compile error instead of a capability grant. * 🕰️ fix: Read an Absent Conversation as Loading, Not as a Switch The registry guard normalized a missing identity to a sentinel and stored it, so a host that reported nothing for a render — a shared link whose data refetches, a chat tab reading its slot before it is filled — looked like a move to another conversation. The reader lost the artifact they had open, and the same conversation coming back read as a first sighting.\n\nAn absent id is now no news at all: the last real identity stays the one the next id is compared against, so returning to it changes nothing and a real switch still wipes. Reported by Devin review of 097f9bf (BUG-0002), which also caught that ShareArtifactsContainer's comment already promised this behavior. * 🏷️ fix: File a Refused Save Under the Artifact That Was Refused The rejection marker was written with whichever artifact the pane happened to be showing when the 400 landed. A save answers after the user may have moved on, so a refusal for artifact A filed under artifact B left A free to send the same refused text again as soon as the user came back to it — and cleared B's own marker on A's next success.\n\nThe artifact a save was started for is captured when the request goes out, and both callbacks use it: the refusal names that artifact, and a success clears only that artifact's marker. Reported by Codex on 097f9bf. * 🖥️ fix: Do Not Reopen the Pane on a Monitor That Is Gone Remembered bounds are coordinates, and they outlive the monitor arrangement that produced them. Undocking takes the pane out of the page along with the only control that docks it, so a window reopened onto a display that no longer exists takes the artifact somewhere the user cannot reach.\n\nThe remembered rectangle is now used only when it can still be reached: it either overlaps this display's work area, or the browser says another display is attached, which is the case the feature exists for. A desktop reporting exactly one display is the only one where the rectangle is provably gone, and there the pane opens at the default bounds. A browser that does not report other displays keeps the user's placement. Reported by Devin review of a85d5c3 (BUG-0001). * 🪞 fix: Follow a Stylesheet the Host Switches, Not Only Rewrites The style mirror watched for nodes and text, so a sheet whose attributes changed — a \ switch, a \ flag, a \ that stops it being a stylesheet — left the popup showing styling the host had moved on from, with nothing to bring it back into line.\n\nThose attributes are part of the signature now, the observer watches them, and an update applies them to the clone alongside its content. Reported by Devin review of a853d48 (BUG-0002). * ♻️ fix: Rebase Every Resubmitted Edit on What the Last Save Wrote An edit replaces whatever the last save wrote, and the artifact registry only catches up when the edited message propagates. Three separate paths resubmit retained text — a queued edit, a buffer inherited at mount, and the edit restored when the user comes back to an artifact — and each had to learn that separately; the restore path had not, so it sent \ as the original, the endpoint refused it with a 400, and the text the user typed never landed.\n\nThe rebase now happens once, where the request is built: \ resolves the original from the save's own record unless a caller overrides it, so no path can forget — including the ordinary keystroke debounce, which had the same gap right after a save. Reported by Codex on a7c047c, and the third finding in this family. * 🧪 test: Let the Shared-Link Dock Click Race Its Own Window Docking closes the window the click was made in, so the click can resolve against a page that has already gone — the same race the focus scenario already tolerates. The assertion that matters, the pane arriving back in the shared page, is unchanged. Seen once on desktop-dark while verifying a7c047c. * 🪞 fix: Clone a Stylesheet in the State the Host Has It The attribute mirror kept a sheet current once the window was open, but the clone was built without those attributes — so a sheet the host had already switched off, or aimed at another medium, arrived in the popup switched on. Its signature matched from that moment, so the sync had nothing to correct.\n\nThe clone now carries the same attributes the sync keeps current. Reported by Devin review of c5d51b8 (BUG-0002), and the same cause as its predecessor in that round. * 🧪 test: Match the Blocked-Popup Notice Where It Is Shown The notice is rendered in the toast and repeated in the live region, so the text matched two nodes and the assertion tripped strict mode on the slower mobile project. Either node proves the user was told; the assertion now takes the first. * 🗑️ fix: Never Let a Cleared Editor Save Itself as a Deletion Typing has always refused to save an empty editor: a user who selects all and deletes is mid-edit, not asking for the artifact's content to be removed. The paths that resubmit retained text did not know that, so a buffer holding an empty string was sent the moment the pane changed hosts or the user came back to the artifact — persisting a deletion nobody confirmed.\n\nThe rule now sits with the other decisions about whether a request goes out, in \, so the debounce, the queued edit, the inherited buffer and the restore all share it. Reported by Codex on a9a5143 (P1). * 🧹 fix: Retire the Inherited Buffer Once a Newer Edit Is in Play A host change hands the replacement editor the text its predecessor never sent, and the drain submits that once the save pipeline is idle. If the user kept typing in the replacement, the newer text queued behind the running save — and when it landed, the drain still had the older buffer in play and sent it, putting the artifact back the way it was.\n\nAnything an instance submits or queues now retires the buffer it inherited at mount, so the newest text is the last word. The drain passes the inherited text itself, which is how it stays exempt. Reported by Codex on 04a9f68 (P1). * 🎨 style: Satisfy the Tailwind v4 Class Order From Canary * 🧺 fix: Retain a Displaced Buffer Under the Artifact That Owns It Editing one artifact evicted another's unsaved text from the shared code buffer: the selection change had cancelled that edit's debounce, so the overwritten copy was the only place it lived and coming back to the artifact fell through to its persisted content. A buffer another artifact displaces is now retained under its owner, and the editor restores and sends it from there on the way back. Reported by Codex on 591ab13 (P1). * 🧪 test: Key the Tabs Spec's Mocked Buffer to Its Artifact The pane-wholeness change gated the preview on the buffer naming the artifact on screen, but the spec's useCodeState mock never set codeArtifactId, so the edited preview never applied. The mock now carries the owner the component reads, and the editor spec's mock grows the retained-buffer field the component destructures. * 🧪 test: Prove a Displaced Edit Returns With Its Artifact The displaced-buffer scenario gets its contract-level coverage: an edit the user leaves on one artifact, an edit that displaces it on another, and a return that has to land on the first artifact's own unsaved text. The fake model grows a reply carrying two artifacts so both trigger rows exist in one conversation, and its one nested ternary is unrolled so the file lints under the repo rules CI applies to staged files. * fix: Resolve Every Artifact Surface and Refusal Per Artifact Preview, download and the Mermaid host now read an artifact's retained buffer the same way the editor does, rejection markers are kept per artifact so one refusal no longer overwrites another, and the saved-content history is only preferred while the registry still sits on this session's chain of saves. * fix: Hold an Inherited Edit Until the Editor Can Save It A buffer inherited across a host change while a response was generating was marked drained before the read-only guard dropped the save, and the read-only transition then replaced it with persisted content. The drain now waits for editing to return and writes the inherited text back after that transition. * chore: Sort the Presentation Spec's Imports After the Canary Rebase * test: Prove Each Artifact Surface and Save Path End to End Covers the download of a displaced edit, a refusal per artifact, an edit after another session changed the artifact, and an edit carried through a response by a host change. * test: Give the Undocked Presentation Case the Canary Query Client * test: Start Artifact Scenarios From a Settled Reply on the Desktop Layout * test: Stub the Command Executor Stamp in the Callbacks Spec Mock Canary's #16556 calls stampCommandExecutor from callbacks.js, but this spec's @librechat/api mock predates it, so three cases failed on canary itself. The mock now carries the real helper, as it already does for isCodeArtifactToolOutput. * fix: Track What the Server Holds Per Artifact Instead of Inferring It The editor reconstructed the server's content from the mutation cache and treated every buffer as unsaved. A save now records what it wrote in a per-artifact ledger that the registry retires once it catches up, so a change made elsewhere (including a revert to an earlier value) wins over this tab's history, and text this tab already saved is never sent again by a host change or a return to the artifact. * fix: Hide the Undocked Pane While Its Conversation Route Is Pending The route keeps the previous conversation mounted but hidden while another one loads or fails to, and the undocked portal escaped that ancestry. The host now passes its pending state down and the window hides the pane with it. * fix: Mark the Saved Buffer Copy and Retire Lag the Registry Has Passed A buffer is saved only while it is the copy a save wrote, so editing away and back to the same text is still sent. Lag values the registry has already moved past are dropped in order, so a later change back to one of them is read as a change made elsewhere. * test: Drop the Callbacks Mock Stub Canary Now Carries * perf: Prune Settled Save Records When the Editing Session Ends
Summary
A bash command run in an attached workspace that exits non-zero, is killed by a signal, or times out still shows as a successful card unless its output happens to match the error regex, and stderr looks the same as stdout. The attached-workspace tool already reports the exit status in a trailer it writes itself (
[exit code: N],[terminated by SIG],[timed out]), but the client never read it, and the sandboxbash_toolshares its name, so the text alone cannot say which tool wrote it.The server now stamps
executor: 'attached_workspace'on tool calls that resolved to the attached-workspace tool instance, streamed and persisted, and the card parses the trailer only for those: a non-zero exit, a signal or a timeout marks the card failed and names the reason next to the failure label, readable by screen readers as part of the header button. Stderr renders in its own colour (error colour on a failed run, secondary on a successful one), and the trailer is dimmed. The group outcome uses the same parser, so a group's failed count matches its cards. Sandbox calls never carry the marker, so a sandbox command that prints[exit code: 1]stays successful and keeps the existing heuristics.Depends on the output pane change in the base branch (
feat/bash-card); the stderr styling survives its tail collapse.Type of change
Testing
Rendered settled bash cards with a non-zero exit and benign output, a zero exit with error-like stderr, a failed run with stderr, signal and timeout trailers, and sandbox output without a trailer; also checked that a group containing a non-zero exit counts it as failed.
Tested environments/configuration:
Automated tests:
npx jestonBashCall.test.tsx,command.test.ts,ToolOutput,outcome.test.tsandProgressText.test.tsx: 90 passedpackages/api:npx jest src/agents/handlers.spec.ts src/code/command.spec.ts: 250 passednpx tsc --noEmitclean inclient,packages/apiandpackages/data-providernpx eslinton the touched files: cleancommand.test.tsfor the trailer parser, a server spec that the marker is stamped for the attached tool and not the sandboxbash_tool, marker and no-marker cases (including sandbox output that prints a trailer), exit status cases inBashCall.test.tsx, the failed count inoutcome.test.ts, the verdict suffix inProgressText.test.tsx, and segment styling across tail collapse inOutputRenderer.test.tsxScreenshots / recordings
Captured from the running app with a seeded attached-workspace call that exits 1; before is canary
bddb8f8ab, after is this branch.Dark mode, after:
Risk / compatibility
Adds an optional
executorfield to tool calls; older messages and other endpoints lack it and render exactly as before. The OpenAI-compatible and Responses controllers do not stamp it yet, so their bash cards keep the text heuristic.Checklist