* feat: edit/revert sent messages via server revert API Wires the mobile client up to the opencode server's session.revert / session.unrevert endpoints (the same primitive the desktop TUI uses to edit the last message). Long-press a user message bubble -> "Edit message" reverts it server-side and prefills the composer with its text; a banner offers Undo while the revert is pending (it's only cleaned up server-side on the next prompt). Degrades gracefully with an alert on older servers that 404 the /revert route. Closes #56 * fix(revert): address code review findings on edit/revert message flow - Confirm before overwriting an in-progress composer draft when editing a sent message (F2) - Restore reverted message's file attachments into the composer, not just its text (F3) - Distinguish 401/403 from other revert failures with an accurate "Authentication failed" message instead of a generic one (F4) - Exclude optimistic "temp-" message IDs from the revert cutoff comparison so concurrently-sent messages aren't hidden (F9) --------- Co-authored-by: engineer <engineer@gray-knight-m1.local>
This commit is contained in:
@@ -16,12 +16,16 @@ interface Props {
|
||||
message: Message
|
||||
parts: Part[]
|
||||
isDark: boolean
|
||||
// Only wired up for user messages — long-press opens the "Edit message" /
|
||||
// revert action sheet. Identified by messageID (not a closure over parts)
|
||||
// so it stays correct even if the memo below bails on a stale render.
|
||||
onLongPress?: (messageID: string) => void
|
||||
}
|
||||
|
||||
// TODO: Replace with streamdown-rn once React 19 types PR lands - it has
|
||||
// built-in block-level memoization that eliminates re-renders for stable blocks
|
||||
export const MessageBubble = memo(
|
||||
function MessageBubble({ message, parts, isDark }: Props) {
|
||||
function MessageBubble({ message, parts, isDark, onLongPress }: Props) {
|
||||
const isUser = message.role === "user"
|
||||
|
||||
const textParts = parts.filter((p) => p.type === "text")
|
||||
@@ -32,7 +36,10 @@ export const MessageBubble = memo(
|
||||
const reasoning = reasoningParts.map((p) => p.text).join("\n") || ""
|
||||
|
||||
return (
|
||||
<View
|
||||
<TouchableOpacity
|
||||
activeOpacity={isUser && onLongPress ? 0.7 : 1}
|
||||
onLongPress={isUser && onLongPress ? () => onLongPress(message.id) : undefined}
|
||||
disabled={!isUser || !onLongPress}
|
||||
style={[
|
||||
s.bubble,
|
||||
isUser ? s.user : s.assistant,
|
||||
@@ -101,7 +108,7 @@ export const MessageBubble = memo(
|
||||
{message.cost ? ` · $${message.cost.toFixed(4)}` : ""}
|
||||
</Text>
|
||||
)}
|
||||
</View>
|
||||
</TouchableOpacity>
|
||||
)
|
||||
},
|
||||
(prev, next) => {
|
||||
@@ -109,6 +116,7 @@ export const MessageBubble = memo(
|
||||
// This prevents completed messages from re-rendering during streaming
|
||||
if (prev.message.id !== next.message.id) return false
|
||||
if (prev.isDark !== next.isDark) return false
|
||||
if (prev.onLongPress !== next.onLongPress) return false
|
||||
if (prev.parts.length !== next.parts.length) return false
|
||||
// Compare the last part's text content - this is what changes during streaming
|
||||
const prevLast = prev.parts[prev.parts.length - 1]
|
||||
|
||||
32
src/lib/prompt-from-parts.test.ts
Normal file
32
src/lib/prompt-from-parts.test.ts
Normal file
@@ -0,0 +1,32 @@
|
||||
import { test } from "node:test"
|
||||
import assert from "node:assert/strict"
|
||||
import { extractPromptFromParts } from "./prompt-from-parts.ts"
|
||||
import type { Part } from "./sdk.ts"
|
||||
|
||||
test("extractPromptFromParts: joins multiple text parts with newlines", () => {
|
||||
const parts: Part[] = [
|
||||
{ id: "p1", messageID: "m1", type: "text", text: "hello" },
|
||||
{ id: "p2", messageID: "m1", type: "text", text: "world" },
|
||||
]
|
||||
assert.deepEqual(extractPromptFromParts(parts), { text: "hello\nworld", files: [] })
|
||||
})
|
||||
|
||||
test("extractPromptFromParts: collects file parts separately from text", () => {
|
||||
const filePart: Part = { id: "p2", messageID: "m1", type: "file", mime: "image/jpeg", url: "data:..." }
|
||||
const parts: Part[] = [{ id: "p1", messageID: "m1", type: "text", text: "check this" }, filePart]
|
||||
assert.deepEqual(extractPromptFromParts(parts), { text: "check this", files: [filePart] })
|
||||
})
|
||||
|
||||
test("extractPromptFromParts: ignores non-text/file parts (tool, reasoning)", () => {
|
||||
const parts: Part[] = [
|
||||
{ id: "p1", messageID: "m1", type: "reasoning", text: "thinking..." },
|
||||
{ id: "p2", messageID: "m1", type: "tool", tool: "bash" },
|
||||
{ id: "p3", messageID: "m1", type: "text", text: "final answer" },
|
||||
]
|
||||
assert.deepEqual(extractPromptFromParts(parts), { text: "final answer", files: [] })
|
||||
})
|
||||
|
||||
test("extractPromptFromParts: handles undefined/empty input", () => {
|
||||
assert.deepEqual(extractPromptFromParts(undefined), { text: "", files: [] })
|
||||
assert.deepEqual(extractPromptFromParts([]), { text: "", files: [] })
|
||||
})
|
||||
20
src/lib/prompt-from-parts.ts
Normal file
20
src/lib/prompt-from-parts.ts
Normal file
@@ -0,0 +1,20 @@
|
||||
// Pure helper: turn a reverted message's parts back into editable prompt
|
||||
// state (text + file attachments), mirroring the TUI's revert-to-edit
|
||||
// reduce (packages/tui/src/routes/session/dialog-message.tsx). Kept
|
||||
// dependency-free so it's testable under plain `node --test`.
|
||||
import type { Part } from "./sdk"
|
||||
|
||||
export interface PromptFromParts {
|
||||
text: string
|
||||
files: Part[]
|
||||
}
|
||||
|
||||
export function extractPromptFromParts(parts: Part[] | undefined): PromptFromParts {
|
||||
const list = parts || []
|
||||
const text = list
|
||||
.filter((p) => p.type === "text" && p.text)
|
||||
.map((p) => p.text)
|
||||
.join("\n")
|
||||
const files = list.filter((p) => p.type === "file")
|
||||
return { text, files }
|
||||
}
|
||||
@@ -35,6 +35,13 @@ export interface Session {
|
||||
deletions: number
|
||||
files: number
|
||||
}
|
||||
// Present while a message (and everything after it) is pending revert —
|
||||
// the server keeps the underlying messages until the next prompt/summarize
|
||||
// call runs cleanup (or the revert is undone via session.unrevert).
|
||||
revert?: {
|
||||
messageID: string
|
||||
partID?: string
|
||||
}
|
||||
}
|
||||
|
||||
export interface Message {
|
||||
@@ -163,6 +170,18 @@ export interface HealthResponse {
|
||||
|
||||
const REQUEST_TIMEOUT_MS = 30_000
|
||||
|
||||
// Thrown by request() on a non-2xx response. Carries the HTTP status so
|
||||
// callers can distinguish e.g. 404 (older server, endpoint missing) from
|
||||
// other failures without parsing the message string.
|
||||
export class ApiError extends Error {
|
||||
status: number
|
||||
constructor(status: number, body: string) {
|
||||
super(`API Error: ${status} - ${body}`)
|
||||
this.name = "ApiError"
|
||||
this.status = status
|
||||
}
|
||||
}
|
||||
|
||||
function createHeaders(config: ClientConfig): HeadersInit {
|
||||
return buildRequestHeaders(config)
|
||||
}
|
||||
@@ -177,7 +196,7 @@ async function request<T>(config: ClientConfig, path: string, options: RequestIn
|
||||
|
||||
if (!response.ok) {
|
||||
const error = await response.text()
|
||||
throw new Error(`API Error: ${response.status} - ${error}`)
|
||||
throw new ApiError(response.status, error)
|
||||
}
|
||||
|
||||
return response.json()
|
||||
@@ -365,6 +384,20 @@ export function createClient(config: ClientConfig) {
|
||||
const qs = messageID ? `?messageID=${messageID}` : ""
|
||||
return request<unknown[]>(config, `/session/${sessionID}/diff${qs}`)
|
||||
},
|
||||
|
||||
// Marks messageID (and everything after it) as pending revert. The
|
||||
// underlying messages aren't deleted until the next prompt runs
|
||||
// cleanup, or the revert is undone with unrevert() below.
|
||||
revert: (sessionID: string, messageID: string, partID?: string) =>
|
||||
request<Session>(config, `/session/${sessionID}/revert`, {
|
||||
method: "POST",
|
||||
body: JSON.stringify(partID ? { messageID, partID } : { messageID }),
|
||||
}),
|
||||
|
||||
unrevert: (sessionID: string) =>
|
||||
request<Session>(config, `/session/${sessionID}/unrevert`, {
|
||||
method: "POST",
|
||||
}),
|
||||
},
|
||||
|
||||
permission: {
|
||||
|
||||
@@ -1,9 +1,10 @@
|
||||
import { create } from "zustand"
|
||||
import type { Session, Message, Part, Event, MessageWithParts, Client } from "../lib/sdk"
|
||||
import { ApiError, type Session, type Message, type Part, type Event, type MessageWithParts, type Client } from "../lib/sdk"
|
||||
import { useConnections } from "./connections"
|
||||
import { useSettings } from "./settings"
|
||||
import { addBreadcrumb } from "../lib/sentry"
|
||||
import { AnalyticsEvent, track } from "../lib/analytics"
|
||||
import { extractPromptFromParts, type PromptFromParts } from "../lib/prompt-from-parts"
|
||||
|
||||
// Helper to convert API response to our internal format
|
||||
function parseMessages(response: MessageWithParts[]): { messages: Message[]; parts: Record<string, Part[]> } {
|
||||
@@ -50,10 +51,16 @@ interface SessionsState {
|
||||
abortSession: () => Promise<void>
|
||||
refreshMessages: () => Promise<void>
|
||||
|
||||
// Revert (edit sent message) / unrevert (undo the pending revert)
|
||||
revertToMessage: (messageID: string) => Promise<RevertResult>
|
||||
unrevertSession: () => Promise<void>
|
||||
|
||||
// Event handling
|
||||
handleEvent: (event: Event) => void
|
||||
}
|
||||
|
||||
export type RevertResult = ({ ok: true } & PromptFromParts) | { ok: false; reason: "unsupported" | "auth" | "error" }
|
||||
|
||||
// Sessions the user aborted since they last went busy. Mirrors events.ts's
|
||||
// erroredSessions: SessionStatus has no "aborted" variant — an aborted run
|
||||
// still ends with a busy -> idle transition — so without this mark a
|
||||
@@ -341,6 +348,54 @@ export const useSessions = create<SessionsState>((set, get) => ({
|
||||
}
|
||||
},
|
||||
|
||||
// Marks messageID (and everything after it) as pending revert, so the
|
||||
// user can re-edit and resend it. The server keeps the underlying
|
||||
// messages until the next prompt runs cleanup, or unrevertSession() below
|
||||
// undoes it — so this only flips session.revert, it doesn't delete
|
||||
// anything itself. Returns the reverted message's text/files so the
|
||||
// caller can prefill the composer.
|
||||
revertToMessage: async (messageID) => {
|
||||
const client = clientFor(get().currentSession?.directory)
|
||||
const session = get().currentSession
|
||||
if (!client || !session) return { ok: false, reason: "error" }
|
||||
|
||||
try {
|
||||
const updated = await client.session.revert(session.id, messageID)
|
||||
set((state) => ({
|
||||
currentSession: state.currentSession?.id === session.id ? updated : state.currentSession,
|
||||
}))
|
||||
return { ok: true, ...extractPromptFromParts(get().parts[messageID]) }
|
||||
} catch (err) {
|
||||
if (err instanceof ApiError) {
|
||||
// Older servers (pre session.revert) 404 on this route — degrade
|
||||
// gracefully instead of surfacing a generic error.
|
||||
if (err.status === 404) return { ok: false, reason: "unsupported" }
|
||||
// Expired/invalid credentials — distinct from a generic failure so
|
||||
// the caller can point the user at reconnecting rather than "retry".
|
||||
if (err.status === 401 || err.status === 403) return { ok: false, reason: "auth" }
|
||||
}
|
||||
console.error("Failed to revert message:", err)
|
||||
set({ error: "Failed to revert message" })
|
||||
return { ok: false, reason: "error" }
|
||||
}
|
||||
},
|
||||
|
||||
unrevertSession: async () => {
|
||||
const client = clientFor(get().currentSession?.directory)
|
||||
const session = get().currentSession
|
||||
if (!client || !session) return
|
||||
|
||||
try {
|
||||
const updated = await client.session.unrevert(session.id)
|
||||
set((state) => ({
|
||||
currentSession: state.currentSession?.id === session.id ? updated : state.currentSession,
|
||||
}))
|
||||
} catch (err) {
|
||||
console.error("Failed to unrevert session:", err)
|
||||
set({ error: "Failed to restore reverted messages" })
|
||||
}
|
||||
},
|
||||
|
||||
handleEvent: (event) => {
|
||||
const { currentSession } = get()
|
||||
if (!currentSession) return
|
||||
|
||||
Reference in New Issue
Block a user