Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the "Fork Session" feature would not work when the message text was unchanged. The issue was caused by an early return in the handleSave function that would exit before calling onMessageUpdate when the content was identical to the original, preventing the fork operation from executing.
Changes:
- Modified the early return condition in
UserMessage.tsxto only apply to 'edit' operations, allowing 'fork' operations to proceed regardless of content changes - Updated the
onMessageUpdatetype signature inProgressiveMessageList.tsxto explicitly include the optionaleditTypeparameter
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ui/desktop/src/components/UserMessage.tsx | Fixed early return logic to allow fork operations with unchanged content |
| ui/desktop/src/components/ProgressiveMessageList.tsx | Updated type signature to properly reflect editType parameter |
zanesq
left a comment
There was a problem hiding this comment.
Thanks! Tested locally LGTM
| renderMessage?: (message: Message, index: number) => React.ReactNode | null; | ||
| isStreamingMessage?: boolean; // Whether messages are currently being streamed | ||
| onMessageUpdate?: (messageId: string, newContent: string) => void; | ||
| onMessageUpdate?: (messageId: string, newContent: string, editType?: 'fork' | 'edit') => void; |
There was a problem hiding this comment.
do we need this change? I don't see the callers being updated to provide a value?
There was a problem hiding this comment.
UserMessage has this which has 3 args:
if (onMessageUpdate && message.id) {
onMessageUpdate(message.id, editContent, editType);
}
but ProgressiveMessageList defied it as only accepting 2 args, so was trying to fix the type
* origin/main: (42 commits) fix: use dynamic port for Tetrate auth callback server (#7228) docs: removing LLM Usage admonitions (#7227) feat(otel): respect standard OTel env vars for exporter selection (#7144) fix: fork session (#7219) Bump version numbers for 1.24.0 release (#7214) Move platform extensions into their own folder (#7210) fix: ignore deprecated skills extension (#7139) Add a goosed over HTTP integration test, and test the developer tool PATH (#7178) feat: add onFallbackRequest handler to McpAppRenderer (#7208) feat: add streaming support for Claude Code CLI provider (#6833) fix: The detected filetype is PLAIN_TEXT, but the provided filetype was HTML (#6885) Add prompts (#7212) Add testing instructions for speech to text (#7185) Diagnostic files copying (#7209) fix: allow concurrent tool execution within the same MCP extension (#7202) fix: handle missing arguments in MCP tool calls to prevent GUI crash (#7143) Filter Apps page to only show standalone Goose Apps (#6811) opt: use static for Regex (#7205) nit: show dir in title, and less... jank (#7138) feat(gemini-cli): use stream-json output and re-use session (#7118) ...
* origin/main: docs: playwright CLI skill tutorial (#7261) install node in goose dir (#7220) fix: relax test_basic_response assertion for providers returning reasoning_content (#7249) fix: handle reasoning_content for Kimi/thinking models (#7252) feat: sandboxing for macos (#7197) fix(otel): use monotonic_counter prefix and support temporality env var (#7234) Streaming markdown (#7233) Improve compaction messages to enable better post-compaction agent behavior (#7259) fix: avoid shell-escaping special characters except quotes (#7242) fix: use dynamic port for Tetrate auth callback server (#7228) docs: removing LLM Usage admonitions (#7227) feat(otel): respect standard OTel env vars for exporter selection (#7144) fix: fork session (#7219) Bump version numbers for 1.24.0 release (#7214) Move platform extensions into their own folder (#7210) fix: ignore deprecated skills extension (#7139) # Conflicts: # Cargo.lock # Cargo.toml
Summary
The
Fork Sessionfeature was not working if the message was not edited.In
UserMessage.tsx, the handleSave function had this flow:When you click "Fork Session", the most common case is that you haven't changed the message text — you just want to create a new session from that point. But the early return on the content-unchanged check prevented onMessageUpdate from ever being called. The edit textbox closed (setIsEditing(false)) but no fork happened.
Type of Change
AI Assistance
Testing
tested locally