From 80d46afa2964d301e3ba6fd2e74b75e8a49852ea Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 26 Jul 2026 01:18:01 -0400 Subject: [PATCH 01/12] Constrain, collapse, and persist mobile chat input MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The AskUserQuestion card on iOS could grow without bound: it was sized off the transcript viewport, which shrinks as the card grows, so a long option list pushed the composer off-screen and left the gate unanswerable. Four related fixes: - Budget every pending-input card off the whole chat surface (transcript + composer heights, whose sum is invariant to how the two split it) instead of the transcript alone. Question text now lives in the card's scroll region so only the provider row and the Send/Decline footer are fixed chrome — the footer can no longer be pushed away. Non-question gates get the same cap via a shared bounded wrapper. - Minimize the pending-input strip to a one-line pill that names what is being asked, so the transcript can be read without answering first. Collapse is keyed to the request id, so a new gate always re-expands. - Persist unsent composer text per chat (and for the Hub / New Chat composers), matching desktop. Debounced autosave plus a flush on teardown; switching sessions flushes under the outgoing key first. - Persist in-progress question selections and freeform text per request, and give the freeform field a keyboard Done toolbar plus a footer dismiss control and interactive scroll-to-dismiss. Co-Authored-By: Claude --- .../ios/ADE/Views/Hub/HubComposerDrawer.swift | 19 + .../Work/WorkChatComposerAndInputViews.swift | 413 ++++++++++++++++-- .../Work/WorkChatSessionView+Timeline.swift | 103 ++++- .../ADE/Views/Work/WorkChatSessionView.swift | 114 ++++- .../Work/WorkErrorAndMessageHelpers.swift | 45 ++ apps/ios/ADE/Views/Work/WorkModels.swift | 87 ++++ .../ADE/Views/Work/WorkNewChatScreen.swift | 19 + 7 files changed, 744 insertions(+), 56 deletions(-) diff --git a/apps/ios/ADE/Views/Hub/HubComposerDrawer.swift b/apps/ios/ADE/Views/Hub/HubComposerDrawer.swift index ef403cf00..2bf99136c 100644 --- a/apps/ios/ADE/Views/Hub/HubComposerDrawer.swift +++ b/apps/ios/ADE/Views/Hub/HubComposerDrawer.swift @@ -274,6 +274,22 @@ struct HubInlineComposer: View { } ) .onAppear { onAppearSetup() } + // Restore whatever the user last typed here but never sent. Guarded on + // empty so a re-appear (or an init-seeded value) can't clobber live text. + .task { + if draft.isEmpty { + draft = WorkComposerDraftStore.load(WorkComposerDraftStore.hubNewChatKey) + } + } + // Debounced autosave: each keystroke restarts this task, and the cancelled + // sleep throws before the write, so only a typing pause hits UserDefaults. + .task(id: draft) { + try? await Task.sleep(for: .milliseconds(400)) + guard !Task.isCancelled else { return } + WorkComposerDraftStore.save(draft, for: WorkComposerDraftStore.hubNewChatKey) + } + // The debounce dies with the view, so flush the final text on teardown. + .onDisappear { WorkComposerDraftStore.save(draft, for: WorkComposerDraftStore.hubNewChatKey) } .onChange(of: composerFocused) { _, focused in if focused { withAnimation(hubComposerSpring) { expanded = true } } } @@ -770,6 +786,9 @@ struct HubInlineComposer: View { collapse() draft = "" attachments.removeAll() + // Drop the persisted draft synchronously — the collapse must not race the + // 400ms autosave debounce and leave the just-sent text behind. + WorkComposerDraftStore.clear(WorkComposerDraftStore.hubNewChatKey) Task { let started = await submit(opener: restoredDraft, attachments: outgoingAttachments) if !started { diff --git a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift index 0a7a3cfff..352a4f6ef 100644 --- a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift +++ b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift @@ -875,7 +875,7 @@ func workPreviewIsWireframe(_ text: String) -> Bool { } /// Natural height of the question card's scrollable body, used to fit the -/// internal ScrollView to its content up to the viewport-derived cap. +/// internal ScrollView to its content up to the height-budget-derived cap. private struct WorkQuestionBodyHeightKey: PreferenceKey { static var defaultValue: CGFloat = 0 static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { @@ -883,6 +883,157 @@ private struct WorkQuestionBodyHeightKey: PreferenceKey { } } +/// Measured height of the card's non-scrolling chrome (provider row + tab strip +/// above, freeform field + action footer below). Subtracted from the card's +/// height budget so the scroll region — not the Send button — absorbs the +/// overflow. Without this the footer got pushed off-screen behind the keyboard +/// on long option lists and the card could not be submitted at all. +private struct WorkQuestionTopChromeHeightKey: PreferenceKey { + static var defaultValue: CGFloat = 0 + static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { + value = max(value, nextValue()) + } +} + +private struct WorkQuestionBottomChromeHeightKey: PreferenceKey { + static var defaultValue: CGFloat = 0 + static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { + value = max(value, nextValue()) + } +} + +private struct WorkPendingCardContentHeightKey: PreferenceKey { + static var defaultValue: CGFloat = 0 + static func reduce(value: inout CGFloat, nextValue: () -> CGFloat) { + value = max(value, nextValue()) + } +} + +/// Caps a pending-input card at the chat surface's height budget, scrolling the +/// overflow rather than growing. A gate that outgrows its budget used to push +/// the composer off the bottom of the screen, which made it unanswerable — the +/// card must never be able to claim more than its share of the page. +/// +/// Short cards are unaffected: the content is measured and the frame follows it +/// exactly, so there is no dead space and no scroll indicator until the cap is +/// actually hit. `WorkStructuredQuestionCard` does its own budgeting (it needs +/// to keep its footer pinned outside the scroll region) and is not wrapped. +struct WorkPendingInputHeightBoundedCard: View { + let maxHeight: CGFloat + @ViewBuilder var content: Content + + @State private var measuredHeight: CGFloat? + + var body: some View { + if maxHeight <= 0 { + content + } else { + ScrollView { + content + .background( + GeometryReader { geo in + Color.clear.preference( + key: WorkPendingCardContentHeightKey.self, + value: geo.size.height + ) + } + ) + } + .frame(height: max(1, min(measuredHeight ?? maxHeight, maxHeight))) + .scrollBounceBehavior(.basedOnSize) + .scrollDismissesKeyboard(.interactively) + .onPreferenceChange(WorkPendingCardContentHeightKey.self) { height in + guard height > 0 else { return } + guard let measured = measuredHeight else { + measuredHeight = height + return + } + if abs(measured - height) > 0.5 { + measuredHeight = height + } + } + } + } +} + +/// In-progress answers for a still-open question request, persisted per request +/// id. The card's selections and freeform text were plain `@State`, so backing +/// out of a chat to check something in the transcript — the exact reason a user +/// minimizes the card — silently discarded everything they had picked or typed. +/// Same storage shape as `WorkComposerDraftStore`: one JSON dictionary under a +/// versioned key, bounded and evicted oldest-first. +enum WorkQuestionDraftStore { + struct Snapshot: Codable, Equatable { + var selections: [String: [String]] = [:] + var freeform: [String: String] = [:] + var sharedFreeform: String = "" + var page: Int = 0 + var updatedAt: Double = 0 + + /// Nothing worth persisting — used to decide between a write and a removal. + var isEmpty: Bool { + selections.values.allSatisfy(\.isEmpty) + && freeform.values.allSatisfy { $0.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty } + && sharedFreeform.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + && page == 0 + } + } + + private static let storageKey = "ade.work.questionDrafts.v1" + /// Open question gates are short-lived; a small cap is plenty and keeps the + /// blob from accumulating answers to requests that were resolved elsewhere. + private static let maxEntries = 30 + private static var defaults: UserDefaults { ADESharedContainer.defaults } + + static func load(_ requestId: String) -> Snapshot? { + guard !requestId.isEmpty else { return nil } + return loadAll()[requestId] + } + + static func save(_ snapshot: Snapshot, for requestId: String) { + guard !requestId.isEmpty else { return } + guard !snapshot.isEmpty else { + clear(requestId) + return + } + var map = loadAll() + var stamped = snapshot + stamped.updatedAt = Date().timeIntervalSince1970 + // Compare ignoring the timestamp so an unchanged draft costs no write. + if var existing = map[requestId] { + existing.updatedAt = stamped.updatedAt + if existing == stamped { return } + } + map[requestId] = stamped + if map.count > maxEntries { + let survivors = map + .sorted { $0.value.updatedAt > $1.value.updatedAt } + .prefix(maxEntries) + map = Dictionary(uniqueKeysWithValues: survivors.map { ($0.key, $0.value) }) + } + persist(map) + } + + static func clear(_ requestId: String) { + guard !requestId.isEmpty else { return } + var map = loadAll() + guard map.removeValue(forKey: requestId) != nil else { return } + persist(map) + } + + private static func loadAll() -> [String: Snapshot] { + guard let data = defaults.data(forKey: storageKey), + let decoded = try? JSONDecoder().decode([String: Snapshot].self, from: data) + else { return [:] } + return decoded + } + + private static func persist(_ map: [String: Snapshot]) { + guard let data = try? JSONEncoder().encode(map) else { return } + defaults.set(data, forKey: storageKey) + } +} + struct WorkStructuredQuestionCard: View { let question: WorkPendingQuestionModel let busy: Bool @@ -899,9 +1050,13 @@ struct WorkStructuredQuestionCard: View { /// Provider to fall back on when the parsed question carries no `source` /// (legacy `structured_question` envelopes). Usually the session provider. var fallbackProvider: String? = nil - /// Transcript viewport height, used to cap the card so long option lists - /// scroll internally instead of overflowing the screen. 0 until measured. - var viewportHeight: CGFloat = 0 + /// Hard ceiling for the card's total laid-out height, computed by the chat + /// surface from the space actually available (keyboard included). The card + /// never exceeds it — the option list scrolls internally instead. Derived + /// from the chat surface, NOT from the transcript viewport: the transcript + /// shrinks as this card grows, so feeding its height back in created a + /// runaway loop where the card ate the whole screen. 0 until measured. + var maxCardHeight: CGFloat = 0 /// Resolved asking provider: the parsed question source, else the session /// fallback. Drives the header verb, logo, and per-provider accent. @@ -920,6 +1075,13 @@ struct WorkStructuredQuestionCard: View { @State private var freeformByQuestion: [String: String] = [:] @State private var expandedPreviews: Set = [] @State private var measuredBodyHeight: CGFloat? = nil + /// Seeded with plausible defaults so the very first frame doesn't overshoot + /// the budget before the preference measurements land. + @State private var topChromeHeight: CGFloat = 26 + @State private var bottomChromeHeight: CGFloat = 40 + /// Gates autosave until the restore pass has run, so an empty first frame + /// can't overwrite a stored draft with nothing. + @State private var didRestoreDrafts = false @FocusState private var freeformFocused: Bool private var isPaged: Bool { question.questions.count > 1 } @@ -929,7 +1091,22 @@ struct WorkStructuredQuestionCard: View { return question.questions[index] } - private var bodyMaxHeight: CGFloat { max(240, viewportHeight * 0.62) } + /// Vertical space the card spends outside the scroll region: the glass card's + /// own padding (14 top + 14 bottom) plus the two 12pt VStack gaps that flank + /// the scroll view. + private static let cardFixedInsets: CGFloat = 14 * 2 + 12 * 2 + + /// Height the scroll region may occupy. When no budget has been measured yet + /// we fall back to a conservative constant rather than "unbounded" so a slow + /// first layout can't flash a full-screen card. + private var bodyMaxHeight: CGFloat { + let budget = maxCardHeight > 0 ? maxCardHeight : 320 + let chrome = topChromeHeight + bottomChromeHeight + Self.cardFixedInsets + // Floor at 88pt: if the chrome alone eats the budget (tiny screen, keyboard + // up, freeform field expanded) we'd rather let the card overflow slightly + // than collapse the option list to nothing. + return max(88, budget - chrome) + } /// Fit the scroll area to its content up to the cap: short lists render at /// their natural height (no scroll, exactly as before); longer lists cap and @@ -937,19 +1114,16 @@ struct WorkStructuredQuestionCard: View { private var resolvedBodyHeight: CGFloat { let cap = bodyMaxHeight guard let measured = measuredBodyHeight else { return cap } - return min(measured, cap) + return max(1, min(measured, cap)) } var body: some View { VStack(alignment: .leading, spacing: 12) { - headerRow - - if isPaged { - questionTabStrip - } + topChrome + .background(chromeHeightReader(WorkQuestionTopChromeHeightKey.self)) ScrollView { - questionPage(activeQuestion) + scrollableBody .background( GeometryReader { geo in Color.clear.preference( @@ -961,6 +1135,9 @@ struct WorkStructuredQuestionCard: View { } .frame(height: resolvedBodyHeight) .scrollBounceBehavior(.basedOnSize) + // Dragging the option list down dismisses the keyboard, so a long typed + // freeform answer never traps the user with no way back to the footer. + .scrollDismissesKeyboard(.interactively) .onPreferenceChange(WorkQuestionBodyHeightKey.self) { height in guard let measured = measuredBodyHeight else { measuredBodyHeight = height @@ -971,43 +1148,156 @@ struct WorkStructuredQuestionCard: View { } } - if activeQuestion.allowsFreeform { - freeformRow(for: activeQuestion) - } - - footerRow + bottomChrome + .background(chromeHeightReader(WorkQuestionBottomChromeHeightKey.self)) } .adeGlassCard(cornerRadius: 18, padding: 14) .overlay( RoundedRectangle(cornerRadius: 18, style: .continuous) .stroke(providerAccent.opacity(0.30), lineWidth: 1) ) + .onPreferenceChange(WorkQuestionTopChromeHeightKey.self) { height in + guard height > 0, abs(topChromeHeight - height) > 0.5 else { return } + topChromeHeight = height + } + .onPreferenceChange(WorkQuestionBottomChromeHeightKey.self) { height in + guard height > 0, abs(bottomChromeHeight - height) > 0.5 else { return } + bottomChromeHeight = height + } .onChange(of: freeformFocused) { _, focused in onFreeformFocusChange?(focused) } + .task(id: question.id) { + restoreDrafts() + } + .task(id: draftSignature) { + // Keystroke debounce: each edit cancels the pending sleep and restarts it, + // so a burst of typing costs one write instead of one per character. + guard didRestoreDrafts else { return } + try? await Task.sleep(for: .milliseconds(400)) + guard !Task.isCancelled else { return } + persistDrafts() + } + .onDisappear { + // Navigating away is exactly the case the debounce would miss. + guard didRestoreDrafts else { return } + persistDrafts() + } .accessibilityElement(children: .contain) .accessibilityLabel("\(workChatSurfaceProviderName(resolvedProvider)) asks. \(activeQuestion.question)") } + /// Change fingerprint for the autosave debounce. Hash-based rather than a + /// concatenated string so a long freeform answer doesn't rebuild a big value + /// on every keystroke. + private var draftSignature: Int { + var hasher = Hasher() + hasher.combine(question.id) + hasher.combine(currentPage) + hasher.combine(singleQuestionFreeformText) + for key in selections.keys.sorted() { + hasher.combine(key) + hasher.combine(selections[key]?.sorted() ?? []) + } + for key in freeformByQuestion.keys.sorted() { + hasher.combine(key) + hasher.combine(freeformByQuestion[key] ?? "") + } + return hasher.finalize() + } + + @MainActor + private func restoreDrafts() { + defer { didRestoreDrafts = true } + guard let stored = WorkQuestionDraftStore.load(question.id) else { return } + // Only restore into an untouched card — a card already mid-edit (the same + // request re-rendering) must win over what's on disk. + guard selections.isEmpty, freeformByQuestion.isEmpty, singleQuestionFreeformText.isEmpty else { return } + selections = stored.selections.mapValues(Set.init) + freeformByQuestion = stored.freeform + singleQuestionFreeformText = stored.sharedFreeform + if stored.page > 0, stored.page < question.questions.count { + currentPage = stored.page + } + } + + private func persistDrafts() { + WorkQuestionDraftStore.save( + WorkQuestionDraftStore.Snapshot( + selections: selections.mapValues { Array($0).sorted() }, + freeform: freeformByQuestion, + sharedFreeform: singleQuestionFreeformText, + page: currentPage + ), + for: question.id + ) + } + + private func chromeHeightReader(_ key: K.Type) -> some View where K.Value == CGFloat { + GeometryReader { geo in + Color.clear.preference(key: key, value: geo.size.height) + } + } + + /// Pinned above the scroll region. Deliberately minimal — provider verb and + /// (when paged) the question tabs — so its height stays bounded no matter how + /// verbose the request is. @ViewBuilder - private var headerRow: some View { - VStack(alignment: .leading, spacing: 8) { - // Provider-identified header: logo + "{Provider} asks" verb. Replaces the - // old clock-icon "Input needed · Claude" treatment from the desktop redesign. - HStack(spacing: 8) { - WorkProviderBareLogo( - provider: resolvedProvider, - fallbackSymbol: providerIcon(resolvedProvider ?? ""), - tint: providerAccent, - size: 18 - ) - Text(question.providerHeaderVerb(fallbackProvider: fallbackProvider)) - .font(.caption.weight(.semibold)) - .foregroundStyle(providerAccent) - Spacer(minLength: 0) + private var topChrome: some View { + VStack(alignment: .leading, spacing: 10) { + providerRow + if isPaged { + questionTabStrip + } + } + } + + /// Everything that can be arbitrarily long lives here and scrolls: the + /// question text itself, the request body, impact/default meta rows, and the + /// option list. Previously the question text sat in the fixed header, so a + /// long prompt pushed the footer off-screen even when the options fit. + @ViewBuilder + private var scrollableBody: some View { + VStack(alignment: .leading, spacing: 10) { + headerRow + questionPage(activeQuestion) + } + } + + /// Pinned below the scroll region so Send/Decline are always reachable. + @ViewBuilder + private var bottomChrome: some View { + VStack(alignment: .leading, spacing: 12) { + if activeQuestion.allowsFreeform { + freeformRow(for: activeQuestion) } - .accessibilityHidden(true) + footerRow + } + } + + /// Provider-identified header: logo + "{Provider} asks" verb. Replaces the + /// old clock-icon "Input needed · Claude" treatment from the desktop redesign. + /// Kept out of the scroll region so the card always identifies itself. + @ViewBuilder + private var providerRow: some View { + HStack(spacing: 8) { + WorkProviderBareLogo( + provider: resolvedProvider, + fallbackSymbol: providerIcon(resolvedProvider ?? ""), + tint: providerAccent, + size: 18 + ) + Text(question.providerHeaderVerb(fallbackProvider: fallbackProvider)) + .font(.caption.weight(.semibold)) + .foregroundStyle(providerAccent) + Spacer(minLength: 0) + } + .accessibilityHidden(true) + } + @ViewBuilder + private var headerRow: some View { + VStack(alignment: .leading, spacing: 8) { // Optional kicker: the question's short `header` shown above the prompt. if let header = activeQuestion.header, !header.isEmpty { Text(header.uppercased()) @@ -1145,24 +1435,53 @@ struct WorkStructuredQuestionCard: View { @ViewBuilder private func freeformRow(for q: WorkPendingQuestion) -> some View { let binding = freeformBinding(for: q) - if q.isSecret { - SecureField(q.options.isEmpty ? "Response" : "Optional response", text: binding) - .focused($freeformFocused) - .adeInsetField(cornerRadius: 14, padding: 12) - .disabled(busy) - } else { - TextField(q.options.isEmpty ? "Response" : "Optional response", text: binding, axis: .vertical) - .focused($freeformFocused) - .lineLimit(1...4) - .adePromptInputTraits() - .adeInsetField(cornerRadius: 14, padding: 12) - .disabled(busy) + Group { + if q.isSecret { + SecureField(q.options.isEmpty ? "Response" : "Optional response", text: binding) + .focused($freeformFocused) + .adeInsetField(cornerRadius: 14, padding: 12) + .disabled(busy) + } else { + TextField(q.options.isEmpty ? "Response" : "Optional response", text: binding, axis: .vertical) + .focused($freeformFocused) + .lineLimit(1...4) + .adePromptInputTraits() + .adeInsetField(cornerRadius: 14, padding: 12) + .disabled(busy) + } + } + // Standard iOS escape hatch from a multi-line field: the vertical-axis + // TextField swallows Return as a newline, so without an explicit Done there + // is no way to lower the keyboard. Scoped to this card's fields. + .toolbar { + ToolbarItemGroup(placement: .keyboard) { + Spacer() + Button("Done") { freeformFocused = false } + .accessibilityLabel("Dismiss keyboard") + } } } @ViewBuilder private var footerRow: some View { HStack(spacing: 10) { + // Second, always-visible way down from the keyboard — the footer is now + // pinned, so this stays reachable even with a long answer typed. + if freeformFocused { + Button { + freeformFocused = false + } label: { + Image(systemName: "keyboard.chevron.compact.down") + .font(.subheadline.weight(.semibold)) + .foregroundStyle(ADEColor.textSecondary) + .frame(width: 32, height: 32) + .background(ADEColor.surfaceBackground.opacity(0.6), in: Circle()) + } + .buttonStyle(.plain) + .accessibilityLabel("Dismiss keyboard") + .transition(.opacity) + } + Button("Decline") { Task { await declineQuestion() } } @@ -1179,6 +1498,7 @@ struct WorkStructuredQuestionCard: View { .tint(providerAccent) .disabled(busy || !canSubmit) } + .animation(.smooth(duration: 0.18), value: freeformFocused) } private var submitLabel: String { @@ -1250,6 +1570,9 @@ struct WorkStructuredQuestionCard: View { @MainActor private func clearQuestionDrafts() { + // Drop the persisted copy first: the request is answered, so a later + // debounce tick must not be able to write it back. + WorkQuestionDraftStore.clear(question.id) if !singleQuestionFreeformText.isEmpty { singleQuestionFreeformText = "" } diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift index ccc3f6ee6..bad00f697 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift @@ -163,7 +163,9 @@ extension WorkChatSessionView { } }, fallbackProvider: chatSummaryContext.provider, - viewportHeight: scrollViewportHeight + // Inline-in-transcript variant: the transcript viewport is the budget + // here, not the whole surface (the composer sits below it either way). + maxCardHeight: max(240, scrollViewportHeight * 0.62) ) .id("pending-question-\(question.id)") case .pendingPermission(let permission): @@ -419,22 +421,28 @@ extension WorkChatSessionView { @ViewBuilder func consolidatedPendingInputStrip(_ item: WorkPendingInputItem) -> some View { VStack(alignment: .leading, spacing: 8) { - if pendingInputCount > 1 { + if pendingInputCollapsed { + pendingInputCollapsedPill(item) + } else { pendingInputQueueHeader + consolidatedPendingInputBody(item) } - consolidatedPendingInputBody(item) } + .animation(.smooth(duration: 0.22), value: pendingInputCollapsed) } - /// "Request 1 of N" + optional "Accept all". The primary request is always the - /// first in the queue, so the leading index is fixed at 1. + /// "Request 1 of N" + optional "Accept all" + the minimize control. Previously + /// this row only rendered for queued requests; it is now always present + /// because it carries the minimize affordance, which every gate needs. @ViewBuilder private var pendingInputQueueHeader: some View { HStack(spacing: 8) { - Text("Request 1 of \(pendingInputCount)") - .font(.caption2.weight(.semibold)) - .foregroundStyle(ADEColor.textMuted) - .accessibilityLabel("Request 1 of \(pendingInputCount) pending.") + if pendingInputCount > 1 { + Text("Request 1 of \(pendingInputCount)") + .font(.caption2.weight(.semibold)) + .foregroundStyle(ADEColor.textMuted) + .accessibilityLabel("Request 1 of \(pendingInputCount) pending.") + } Spacer(minLength: 0) if canAcceptAllPendingInputs { Button { @@ -448,12 +456,87 @@ extension WorkChatSessionView { .disabled(actionInFlight || !isLive) .accessibilityLabel("Accept all \(acceptAllSweepableInputs.count) pending approvals") } + Button { + pendingInputCollapsed = true + } label: { + Image(systemName: "chevron.down") + .font(.caption2.weight(.bold)) + .foregroundStyle(ADEColor.textSecondary) + .frame(width: 26, height: 22) + .contentShape(Rectangle()) + } + .buttonStyle(.plain) + .accessibilityLabel("Minimize request") + .accessibilityHint("Keeps the request open so you can scroll the conversation.") } .padding(.horizontal, 4) + .frame(minHeight: 22) + } + + /// Minimized state: a single tappable line that keeps the gate visible (and + /// says what it is) while giving the transcript the screen back. + @ViewBuilder + private func pendingInputCollapsedPill(_ item: WorkPendingInputItem) -> some View { + let provider = workPendingInputProvider(item) ?? chatSummaryContext.provider + let accent = ADEColor.providerChatAccent(for: provider) + let summary = workPendingInputCollapsedSummary(item) + Button { + pendingInputCollapsed = false + } label: { + HStack(spacing: 8) { + WorkProviderBareLogo( + provider: provider, + fallbackSymbol: providerIcon(provider), + tint: accent, + size: 15 + ) + Text(summary) + .font(.caption.weight(.semibold)) + .foregroundStyle(ADEColor.textPrimary) + .lineLimit(1) + .truncationMode(.tail) + Spacer(minLength: 4) + if pendingInputCount > 1 { + Text("\(pendingInputCount)") + .font(.caption2.weight(.bold)) + .foregroundStyle(accent) + .padding(.horizontal, 6) + .padding(.vertical, 2) + .background(accent.opacity(0.16), in: Capsule()) + } + Image(systemName: "chevron.up") + .font(.caption2.weight(.bold)) + .foregroundStyle(ADEColor.textSecondary) + } + .padding(.horizontal, 12) + .padding(.vertical, 9) + .frame(maxWidth: .infinity, alignment: .leading) + .background(ADEColor.surfaceBackground.opacity(0.7), in: RoundedRectangle(cornerRadius: 14, style: .continuous)) + .overlay( + RoundedRectangle(cornerRadius: 14, style: .continuous) + .stroke(accent.opacity(0.35), lineWidth: 1) + ) + } + .buttonStyle(.plain) + .accessibilityLabel("\(summary). Minimized.") + .accessibilityHint("Expand to answer.") } @ViewBuilder private func consolidatedPendingInputBody(_ item: WorkPendingInputItem) -> some View { + // The question card budgets itself (its footer has to stay pinned outside + // the scroll region); every other kind is capped by the shared wrapper. + if case .question = item { + pendingInputCard(item) + } else { + WorkPendingInputHeightBoundedCard(maxHeight: pendingInputMaxHeight) { + pendingInputCard(item) + } + } + } + + @ViewBuilder + private func pendingInputCard(_ item: WorkPendingInputItem) -> some View { switch item { case .planApproval(let model): WorkPlanComposerStrip( @@ -512,7 +595,7 @@ extension WorkChatSessionView { } }, fallbackProvider: chatSummaryContext.provider, - viewportHeight: scrollViewportHeight + maxCardHeight: pendingInputMaxHeight ) case .modelSelection(let model): WorkModelSelectionPendingCard( diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index ec021323a..006be9c83 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -246,6 +246,12 @@ struct WorkChatSessionView: View { /// item leaves the derived queue (or rolled back if the command errored). See /// `dispatchPendingInputAnswer` / `reconcileOptimisticallyAnsweredInputs`. @State var optimisticallyAnsweredInputIds: Set = [] + /// Id of the pending input the user minimized, if any. Stored as an id rather + /// than a Bool so a different request becoming primary re-expands the strip on + /// its own — a minimize applies to the gate the user chose to defer, never to + /// the next one, and deriving it this way needs no `onChange` (which `body`'s + /// modifier chain has no type-inference budget left for). + @State var collapsedPendingInputId: String? var sessionStatus: String { resolvedSessionStatus ?? session.normalizedStatus @@ -347,6 +353,45 @@ struct WorkChatSessionView: View { pendingInputs.first } + /// The strip is minimized only while the deferred gate is still the primary + /// one. The gate stays open and the composer stays locked either way — only + /// the card is swapped for a one-line pill. + var pendingInputCollapsed: Bool { + get { + guard let collapsedPendingInputId, let primaryPendingInput else { return false } + return collapsedPendingInputId == primaryPendingInput.id + } + nonmutating set { + collapsedPendingInputId = newValue ? primaryPendingInput?.id : nil + } + } + + /// Total height available to the chat surface, keyboard already subtracted. + /// + /// The transcript and the composer inset split the surface between them, so + /// their measured heights always sum back to it — and unlike either half on + /// its own, the sum does NOT move when the pending-input card grows. That + /// matters: sizing the card off `scrollViewportHeight` alone (what this used + /// to do) was self-referential, because a taller card shrank the transcript, + /// which shrank the budget, which... The floor is the 240 the transcript + /// reports before its first real measurement. + var chatSurfaceHeight: CGFloat { + max(240, scrollViewportHeight + composerLayoutHeight) + } + + /// Hard ceiling for the pending-input card. Always leaves room for the + /// composer plus a slice of transcript — a gate that covers the entire screen + /// reads as a modal takeover and hides the Send button. Long content scrolls + /// inside the card instead of growing it. + var pendingInputMaxHeight: CGFloat { + let surface = chatSurfaceHeight + // Reserve the composer's own footprint; whatever is left is shared between + // the strip and the transcript, with the strip capped at ~82% of it. + let composerReserve: CGFloat = 132 + let available = max(0, surface - composerReserve) + return max(160, min(available * 0.82, surface * 0.62)) + } + /// Open approval / permission gates that "Accept all" can sweep. Question, /// plan-approval, and model-selection kinds are never auto-answered. var acceptAllSweepableInputs: [WorkPendingInputItem] { @@ -758,6 +803,7 @@ struct WorkChatSessionView: View { settingsMutationInFlight: composerSettingMutationInFlight, codexFastModeOverride: pendingCodexFastMode, composerDraftRestore: composerDraftRestore, + draftPersistenceKey: WorkComposerDraftStore.chatKey(sessionId: session.id), compact: compactComposer, // Show Stop while a live turn has current transcript activity. The // broader live hint can lag after `done`; this stricter gate keeps the @@ -1010,6 +1056,7 @@ struct WorkChatSessionView: View { lastBlockingPendingInputId = nil blockingPendingHapticToken = 0 optimisticallyAnsweredInputIds.removeAll() + collapsedPendingInputId = nil assistantLineBudgets.removeAll() composerSettingMutationInFlight = false composerSettingMutationGeneration &+= 1 @@ -1275,6 +1322,7 @@ private struct WorkChatViewportHeightPreferenceKey: PreferenceKey { } } + private struct WorkChatViewportWidthPreferenceKey: PreferenceKey { static var defaultValue: CGFloat = 0 @@ -1679,6 +1727,7 @@ private struct WorkChatComposerCard: View { let settingsMutationInFlight: Bool let codexFastModeOverride: Bool? let composerDraftRestore: WorkChatComposerDraftRestore? + let draftPersistenceKey: String let compact: Bool /// True while the assistant is streaming a response. Swaps the Send button /// Desktop parity: red bordered stop control in the composer while a turn is @@ -1708,6 +1757,7 @@ private struct WorkChatComposerCard: View { settingsMutationInFlight: settingsMutationInFlight, codexFastModeOverride: codexFastModeOverride, composerDraftRestore: composerDraftRestore, + draftPersistenceKey: draftPersistenceKey, compact: compact, showInterrupt: showInterrupt, interruptInFlight: interruptInFlight, @@ -1747,6 +1797,9 @@ private struct WorkChatComposerDraftInput: View { let settingsMutationInFlight: Bool let codexFastModeOverride: Bool? let composerDraftRestore: WorkChatComposerDraftRestore? + /// Key this chat's unsent text is persisted under, so leaving and coming back + /// restores it (matching desktop). Empty disables persistence. + let draftPersistenceKey: String let compact: Bool let showInterrupt: Bool let interruptInFlight: Bool @@ -1900,9 +1953,18 @@ private struct WorkChatComposerDraftInput: View { } } .onAppear { configureSuggestionController() } + // Bind before applying a restore: `bind` only seeds an empty field, so a + // failed-send restore that runs first would be preserved either way, but + // binding first keeps the persisted key correct for the very first autosave. + .task(id: draftPersistenceKey) { + draftState.bind(persistenceKey: draftPersistenceKey) + } .task(id: composerDraftRestore?.id) { draftState.applyRestore(composerDraftRestore) } + // The 400ms autosave debounce can't survive a navigation pop; flush here so + // backing out of a chat mid-sentence keeps the sentence. + .onDisappear { draftState.flushDraft() } .onChange(of: chatSummary.provider) { _, _ in configureSuggestionController() } .onChange(of: laneId) { _, _ in configureSuggestionController() } .workChatAttachmentPicker( @@ -2177,14 +2239,64 @@ struct WorkChatComposerDraftRestore: Equatable, Identifiable { } final class WorkChatComposerDraftState: ObservableObject { - @Published var text = "" + @Published var text = "" { + didSet { + guard text != oldValue else { return } + scheduleAutosave() + } + } @Published var isFocused = false private var appliedRestoreId: UUID? + /// Surface this composer's draft is persisted under. Empty means "don't + /// persist" (the key is unresolved), which is the safe default. + private var persistenceKey = "" + private var autosaveTask: Task? var trimmedText: String { text.trimmingCharacters(in: .whitespacesAndNewlines) } + /// Point this composer at a chat's stored draft. The composer view is reused + /// across session switches, so the outgoing chat's text is flushed under its + /// own key before the new one is loaded — otherwise switching chats would + /// either lose a draft or write it into the wrong conversation. + @MainActor + func bind(persistenceKey key: String) { + guard persistenceKey != key else { return } + flushDraft() + persistenceKey = key + guard !key.isEmpty else { return } + let stored = WorkComposerDraftStore.load(key) + // Whatever is already in the field wins: a failed send restores its text + // here, and that is fresher than anything on disk. + guard trimmedText.isEmpty, !stored.isEmpty else { return } + text = stored + } + + /// Write the draft now, cancelling any pending debounce. Called when the chat + /// is torn down — the case the debounce would otherwise miss. + @MainActor + func flushDraft() { + autosaveTask?.cancel() + autosaveTask = nil + guard !persistenceKey.isEmpty else { return } + WorkComposerDraftStore.save(text, for: persistenceKey) + } + + /// Keystroke debounce: each edit restarts the timer, so a burst of typing + /// costs one write instead of one per character. + private func scheduleAutosave() { + guard !persistenceKey.isEmpty else { return } + autosaveTask?.cancel() + let key = persistenceKey + let value = text + autosaveTask = Task { @MainActor in + try? await Task.sleep(for: .milliseconds(400)) + guard !Task.isCancelled else { return } + WorkComposerDraftStore.save(value, for: key) + } + } + var hasSendableText: Bool { !trimmedText.isEmpty } diff --git a/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift b/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift index ac730bcbc..f59b27d31 100644 --- a/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift +++ b/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift @@ -1223,6 +1223,51 @@ enum WorkPendingInputItem: Identifiable, Equatable { } } +/// Asking provider for a pending gate, when the payload carries one. Only the +/// question and plan-approval kinds do; approval/permission/model-selection fall +/// back to the session provider at the call site. +func workPendingInputProvider(_ item: WorkPendingInputItem) -> String? { + switch item { + case .question(let model): + let source = model.source?.trimmingCharacters(in: .whitespacesAndNewlines) + return source?.isEmpty == false ? source : nil + case .planApproval(let model): + let source = model.source.trimmingCharacters(in: .whitespacesAndNewlines) + return source.isEmpty ? nil : source + case .approval, .permission, .modelSelection: + return nil + } +} + +/// One-line label for the minimized pending-input pill. Must say what is being +/// asked, not just that something is — a generic "1 request" pill is exactly the +/// kind of thing users learn to ignore. +func workPendingInputCollapsedSummary(_ item: WorkPendingInputItem) -> String { + func firstNonEmpty(_ candidates: [String?]) -> String? { + for candidate in candidates { + let trimmed = candidate?.trimmingCharacters(in: .whitespacesAndNewlines) + if let trimmed, !trimmed.isEmpty { return trimmed } + } + return nil + } + + switch item { + case .question(let model): + return firstNonEmpty([model.primary.header, model.question, model.title, model.body]) + ?? "Waiting on your answer" + case .planApproval(let model): + return firstNonEmpty([model.title]) ?? "Plan ready for review" + case .approval(let model): + return firstNonEmpty([model.description, model.detail]) ?? "Approval requested" + case .permission(let model): + let tool = model.tool.trimmingCharacters(in: .whitespacesAndNewlines) + if !tool.isEmpty { return "Permission: \(tool)" } + return firstNonEmpty([model.description, model.detail]) ?? "Permission requested" + case .modelSelection(let model): + return model.title + } +} + struct WorkPendingSteerModel: Identifiable, Equatable { let id: String var text: String diff --git a/apps/ios/ADE/Views/Work/WorkModels.swift b/apps/ios/ADE/Views/Work/WorkModels.swift index 305e42f50..fe64a09df 100644 --- a/apps/ios/ADE/Views/Work/WorkModels.swift +++ b/apps/ios/ADE/Views/Work/WorkModels.swift @@ -64,6 +64,93 @@ enum WorkComposerPreferences { } } +/// Unsent composer text, persisted per surface so leaving a chat (or the app) +/// never discards what the user typed — desktop keeps its draft, and mobile +/// silently dropping it was the single most-reported chat regression. +/// One JSON dictionary under a versioned key: small enough to rewrite whole on +/// each save, bounded by `maxEntries` (LRU by `updatedAt`) so a long-lived +/// install can't grow it without limit. +enum WorkComposerDraftStore { + struct Entry: Codable, Equatable { + var text: String + var updatedAt: Double + } + + /// Versioned so a future shape change can migrate rather than mis-decode. + private static let storageKey = "ade.work.composerDrafts.v1" + /// Enough to cover every chat a user realistically juggles; older drafts are + /// evicted oldest-first rather than kept forever. + private static let maxEntries = 60 + /// A composer draft is a prompt, not a document — clamp pathological pastes so + /// one entry can't dominate the shared defaults store. + private static let maxLength = 20_000 + private static var defaults: UserDefaults { ADESharedContainer.defaults } + + /// Per-chat key. Blank session ids yield a blank key so callers that render + /// before the session resolves can't write everyone's draft into one bucket. + static func chatKey(sessionId: String) -> String { + let trimmed = sessionId.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { return "" } + return "chat:\(trimmed)" + } + + /// The two "new chat" composers are singletons, so they get fixed keys. + static let hubNewChatKey = "hub-new-chat" + static let workNewChatKey = "work-new-chat" + + /// The stored draft, or "" when the key is blank, absent, or undecodable — + /// restoring must never be able to fail loudly in a view body. + static func load(_ key: String) -> String { + guard !key.isEmpty else { return "" } + return loadAll()[key]?.text ?? "" + } + + /// Persists (or clears) the draft for one surface. An emptied composer removes + /// its entry outright: a user who deletes their text must not have it + /// resurrected the next time the screen mounts. + static func save(_ text: String, for key: String) { + guard !key.isEmpty else { return } + var map = loadAll() + guard !text.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { + guard map.removeValue(forKey: key) != nil else { return } + persist(map) + return + } + let clipped = String(text.prefix(maxLength)) + // Autosave runs on a keystroke debounce; skip the UserDefaults write when + // the content is unchanged so idle typing pauses cost nothing. + if map[key]?.text == clipped { return } + map[key] = Entry(text: clipped, updatedAt: Date().timeIntervalSince1970) + if map.count > maxEntries { + let survivors = map + .sorted { $0.value.updatedAt > $1.value.updatedAt } + .prefix(maxEntries) + map = Dictionary(uniqueKeysWithValues: survivors.map { ($0.key, $0.value) }) + } + persist(map) + } + + /// Drops a draft that has been consumed (sent) so it can't reappear. + static func clear(_ key: String) { + guard !key.isEmpty else { return } + var map = loadAll() + guard map.removeValue(forKey: key) != nil else { return } + persist(map) + } + + private static func loadAll() -> [String: Entry] { + guard let data = defaults.data(forKey: storageKey), + let decoded = try? JSONDecoder().decode([String: Entry].self, from: data) + else { return [:] } + return decoded + } + + private static func persist(_ map: [String: Entry]) { + guard let data = try? JSONEncoder().encode(map) else { return } + defaults.set(data, forKey: storageKey) + } +} + enum WorkToolCardStatus: String, Equatable { case running case completed diff --git a/apps/ios/ADE/Views/Work/WorkNewChatScreen.swift b/apps/ios/ADE/Views/Work/WorkNewChatScreen.swift index 4f6744a92..2b46192e3 100644 --- a/apps/ios/ADE/Views/Work/WorkNewChatScreen.swift +++ b/apps/ios/ADE/Views/Work/WorkNewChatScreen.swift @@ -1444,6 +1444,9 @@ private struct WorkNewChatComposerBar: View { composerFocused = false draft = "" attachments.removeAll() + // Drop the persisted draft synchronously — navigating into the new chat must + // not race the 400ms autosave debounce and leave the just-sent text behind. + WorkComposerDraftStore.clear(WorkComposerDraftStore.workNewChatKey) Task { let started = await onSubmit(restoredDraft, outgoingAttachments) if !started { @@ -1557,6 +1560,22 @@ private struct WorkNewChatComposerBar: View { attachments: $attachments, onDismiss: { composerFocused = true } ) + // Restore whatever the user last typed here but never sent. Guarded on + // empty so a re-appear can't clobber live text. + .task { + if draft.isEmpty { + draft = WorkComposerDraftStore.load(WorkComposerDraftStore.workNewChatKey) + } + } + // Debounced autosave: each keystroke restarts this task, and the cancelled + // sleep throws before the write, so only a typing pause hits UserDefaults. + .task(id: draft) { + try? await Task.sleep(for: .milliseconds(400)) + guard !Task.isCancelled else { return } + WorkComposerDraftStore.save(draft, for: WorkComposerDraftStore.workNewChatKey) + } + // The debounce dies with the view, so flush the final text on teardown. + .onDisappear { WorkComposerDraftStore.save(draft, for: WorkComposerDraftStore.workNewChatKey) } } /// Primary foreground launch button — the compact arrow-in-circle send glyph From 9e1ea5b1cf82a93a5b8c98c20f93554fe4be6ccf Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 26 Jul 2026 01:23:01 -0400 Subject: [PATCH 02/12] Stop reused transcript sequences from swallowing chat events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A chat transcript is durable and appended across restarts, but the host's `eventSequence` counter is per-runtime and was seeded to 0 on rehydration (agentChatService.ts, the one field on that path not read from `persisted`). So one transcript could hold two events numbered 67, hours apart. In the reported session the counter ran to 322, the desktop restarted, and the next 100 events re-used 1..100 — 103 colliding sequence numbers in one file. iOS derives event identity from `sessionId:sequence` (RemoteModels.AgentChatEventEnvelope.id) and its dedupe is first-key-wins over file order, so every colliding event from the newer epoch was discarded as a duplicate of the older one. Two of the casualties were the `approval_request` envelopes carrying AskUserQuestion cards: the model asked four questions, the phone rendered none, and the same thing happened again on retry. A short text chunk lost the same way is why one reply rendered as "king Round 1 now" instead of "Kicking Round 1 now" — sub-24-char text has no content dedupe key and also falls back to the sequence-derived id. - Seed `eventSequence` from the transcript's highest sequence on hydration, in the same single pass that already recovers todo items. - Include the timestamp in the iOS envelope id. Genuine redeliveries carry the same timestamp and sequence, so dedupe still catches them; only cross-epoch collisions come apart. This repairs transcripts already written. - Give blocking gates (`approval_request`, `structured_question`, `pending_input_resolved`) an itemId-based content dedupe key so a question card never depends on sequence uniqueness again. - Accept `AskUserQuestion` in `isAskUserToolName`: it normalizes to `askuserquestion`, which matched nothing, so a bare tool_call from a host that doesn't wrap it in an approval_request rendered as an inert tool row. Co-Authored-By: Claude --- .../main/services/chat/agentChatService.ts | 38 ++++- apps/ios/ADE/Models/RemoteModels.swift | 16 ++- apps/ios/ADE/Services/SyncService.swift | 18 +++ .../Work/WorkErrorAndMessageHelpers.swift | 11 +- apps/ios/ADE/Views/Work/WorkPreviews.swift | 136 ++++++++++++++++++ 5 files changed, 215 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index fc7ec0440..a320609ed 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -8357,6 +8357,38 @@ export function createAgentChatService(args: { return latest; }; + /** Everything a rehydrated session has to recover from its own transcript, + * read in one pass (the transcript is not cached, so this is deliberately not + * two separate scans). + * + * `maxEventSequence` is the load-bearing part. `eventSequence` is a runtime + * counter, but the transcript it numbers is durable and appended across + * restarts — so starting a rehydrated session back at 0 mints sequence + * numbers that already exist in the file. Consumers that treat + * `sessionId + sequence` as an event identity then mistake the new events for + * replays of the old ones and drop them; that is exactly how AskUserQuestion + * cards silently vanished on iOS for sessions reopened after a desktop + * restart. Seeding from the file keeps sequences strictly increasing for the + * life of the transcript. */ + const readTranscriptHydrationState = ( + managed: ManagedChatSession, + ): { + todoItems: Extract["items"]; + maxEventSequence: number; + } => { + let todoItems: Extract["items"] = []; + let maxEventSequence = 0; + for (const entry of readTranscriptEnvelopes(managed)) { + if (entry.event.type === "todo_update") { + todoItems = entry.event.items; + } + if (typeof entry.sequence === "number" && entry.sequence > maxEventSequence) { + maxEventSequence = entry.sequence; + } + } + return { todoItems, maxEventSequence }; + }; + /** Runtime-lifetime TaskCreate/TaskUpdate tracker, lazily seeded from the * transcript's latest todo_update so updates in later turns (or after a * host restart) still resolve to the task they reference. */ @@ -15722,7 +15754,11 @@ export function createAgentChatService(args: { claudeBackgroundLogText: persisted?.claudeBackgroundLogText ?? "", compactionEmitterState: createCompactionEmitterState(), }; - managed.todoItems = readLatestTranscriptTodoItems(managed); + const transcriptHydration = readTranscriptHydrationState(managed); + managed.todoItems = transcriptHydration.todoItems; + // Continue the transcript's numbering instead of restarting at 1 — see + // `readTranscriptHydrationState`. + managed.eventSequence = transcriptHydration.maxEventSequence; if (!managed.session.interactionMode && managed.session.orchestrationRole) { managed.session.interactionMode = orchestrationInteractionModeForRole(managed.session.orchestrationRole); } diff --git a/apps/ios/ADE/Models/RemoteModels.swift b/apps/ios/ADE/Models/RemoteModels.swift index 7a4c40307..a0ae042ce 100644 --- a/apps/ios/ADE/Models/RemoteModels.swift +++ b/apps/ios/ADE/Models/RemoteModels.swift @@ -1929,9 +1929,21 @@ struct AgentChatEventProvenance: Decodable, Equatable { } struct AgentChatEventEnvelope: Decodable, Identifiable, Equatable { + /// Identity must include the timestamp, not just the sequence. + /// + /// A host's `eventSequence` counter restarts at 1 whenever a session is + /// rehydrated, but it keeps appending to the SAME transcript file — so one + /// transcript can hold two events numbered 67, hours apart. Keying identity on + /// `sessionId:sequence` alone made the newer event look like a duplicate of + /// the older one, and dedupe (first-key-wins) silently dropped it. That is how + /// an `approval_request` carrying a whole AskUserQuestion card disappeared + /// from a phone while the rest of the turn rendered fine. + /// + /// A genuine redelivery carries the same timestamp AND sequence, so dedupe + /// still catches it; only cross-epoch collisions are broken apart. var id: String { - let sequencePart = sequence.map(String.init) ?? timestamp - return "\(sessionId):\(sequencePart)" + guard let sequence else { return "\(sessionId):\(timestamp)" } + return "\(sessionId):\(timestamp):\(sequence)" } var sessionId: String diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 4f85e8f83..2f8488ec0 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -15638,6 +15638,24 @@ final class SyncService: ObservableObject { processed.map { $0 ? "1" : "0" } ?? "", text.trimmingCharacters(in: .whitespacesAndNewlines) ].joined(separator: "|") + // Blocking gates carry a host-assigned `itemId` that is unique for the life + // of the session, so key them on that rather than falling through to the + // sequence-derived envelope id. A dropped gate is not a cosmetic loss — it + // is a question card the user never sees and can never answer — so it must + // not depend on sequence numbers being unique, which they are not across a + // host restart. + case .approvalRequest(let itemId, _, _, _, _, _): + let normalizedItemId = itemId.trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalizedItemId.isEmpty else { return nil } + return [envelope.sessionId, "approval_request", normalizedItemId].joined(separator: "|") + case .structuredQuestion(_, _, let itemId, _): + let normalizedItemId = itemId.trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalizedItemId.isEmpty else { return nil } + return [envelope.sessionId, "structured_question", normalizedItemId].joined(separator: "|") + case .pendingInputResolved(let itemId, let resolution, _): + let normalizedItemId = itemId.trimmingCharacters(in: .whitespacesAndNewlines) + guard !normalizedItemId.isEmpty else { return nil } + return [envelope.sessionId, "pending_input_resolved", normalizedItemId, resolution].joined(separator: "|") default: return nil } diff --git a/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift b/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift index f59b27d31..7c08b7d22 100644 --- a/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift +++ b/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift @@ -1646,7 +1646,16 @@ func derivePendingWorkInputs(from transcript: [WorkChatEnvelope]) -> [WorkPendin func isAskUserToolName(_ tool: String) -> Bool { let normalized = normalizedWorkToolIdentity(tool) - return normalized == "ask_user" || normalized == "askuser" || normalized == "mcp_ade_ask_user" + // `askuserquestion` is the Claude Agent SDK's own `AskUserQuestion` tool after + // normalization (CamelCase collapses to one word, no separators to split on). + // Desktop hosts wrap it in an `approval_request` so the question card comes + // from the request kind, but a bare tool_call from any other host would + // otherwise render as a raw tool row with no way to answer. + return normalized == "ask_user" + || normalized == "askuser" + || normalized == "askuserquestion" + || normalized == "ask_user_question" + || normalized == "mcp_ade_ask_user" } func isRequestUserInputToolName(_ tool: String) -> Bool { diff --git a/apps/ios/ADE/Views/Work/WorkPreviews.swift b/apps/ios/ADE/Views/Work/WorkPreviews.swift index 34c9ae1f0..f1c8d414f 100644 --- a/apps/ios/ADE/Views/Work/WorkPreviews.swift +++ b/apps/ios/ADE/Views/Work/WorkPreviews.swift @@ -497,6 +497,142 @@ private enum WorkPreviewData { .environmentObject(WorkPreviewData.dictationController) } +/// A deliberately oversized AskUserQuestion payload: four paged questions, long +/// prompts, and eight options each. This is the shape that used to push the +/// composer off the bottom of the screen — the card must stay inside +/// `maxCardHeight` with Send/Decline visible, scrolling the options internally. +private func workPreviewOversizedQuestion() -> WorkPendingQuestionModel { + func options(_ prefix: String) -> [WorkPendingQuestionOption] { + (1...8).map { index in + WorkPendingQuestionOption( + label: "\(prefix) option \(index)", + value: "\(prefix.lowercased())-\(index)", + description: "A per-option description long enough to wrap onto a second line on a phone-width card.", + recommended: index == 2, + preview: index == 3 ? "┌────────────┐\n│ wireframe │\n└────────────┘" : nil, + previewFormat: index == 3 ? "html" : nil + ) + } + } + return WorkPendingQuestionModel( + id: "preview-question-oversized", + questions: [ + WorkPendingQuestion( + questionId: "approach", + question: "Which approach should the refactor take, given that the existing service already owns retry and backoff and we do not want to duplicate that logic in the new call path?", + options: options("Approach"), + allowsFreeform: true, + header: "Approach", + defaultAssumption: "Extend the existing service rather than adding a parallel one.", + impact: "Changes the public surface of the sync layer.", + multiSelect: false + ), + WorkPendingQuestion( + questionId: "scope", + question: "Which surfaces should ship in the first pass?", + options: options("Scope"), + allowsFreeform: true, + header: "Scope", + multiSelect: true + ), + WorkPendingQuestion( + questionId: "rollout", + question: "How should this roll out?", + options: options("Rollout"), + allowsFreeform: false, + header: "Rollout" + ), + WorkPendingQuestion( + questionId: "notes", + question: "Anything else worth capturing before I start?", + options: [], + allowsFreeform: true, + header: "Notes" + ) + ], + title: "Plan round 1", + body: "Four questions before I start on the plan.", + source: "claude" + ) +} + +#Preview("Question card - oversized, phone budget") { + // 720pt ≈ an iPhone chat surface with no keyboard; the card is capped at the + // same fraction `pendingInputMaxHeight` uses so the preview matches the app. + VStack { + Spacer() + WorkStructuredQuestionCard( + question: workPreviewOversizedQuestion(), + busy: false, + onSelectOption: { _, _ in }, + onSubmitAll: { _, _ in }, + onDecline: {}, + fallbackProvider: "claude", + maxCardHeight: max(160, min((720 - 132) * 0.82, 720 * 0.62)) + ) + .padding(16) + } + .frame(maxWidth: .infinity, maxHeight: .infinity) + .background(ADEColor.pageBackground) + .preferredColorScheme(.dark) +} + +#Preview("Question card - oversized, keyboard up") { + // ~340pt of surface left once the keyboard is showing. Send must still be + // on screen; the option list absorbs the loss. + VStack { + Spacer() + WorkStructuredQuestionCard( + question: workPreviewOversizedQuestion(), + busy: false, + onSelectOption: { _, _ in }, + onSubmitAll: { _, _ in }, + onDecline: {}, + fallbackProvider: "claude", + maxCardHeight: max(160, min((340 - 132) * 0.82, 340 * 0.62)) + ) + .padding(16) + } + .frame(maxWidth: .infinity, maxHeight: .infinity) + .background(ADEColor.pageBackground) + .preferredColorScheme(.dark) +} + +#Preview("Question card - short, natural height") { + // Regression guard for the other direction: a two-option question must not + // grow to fill the budget or gain a scroll indicator. + VStack { + Spacer() + WorkStructuredQuestionCard( + question: WorkPendingQuestionModel( + id: "preview-question-short", + questions: [ + WorkPendingQuestion( + questionId: "confirm", + question: "Rebase onto main before opening the PR?", + options: [ + WorkPendingQuestionOption(label: "Rebase", value: "rebase", description: nil, recommended: true), + WorkPendingQuestionOption(label: "Leave it", value: "skip", description: nil) + ], + allowsFreeform: false + ) + ], + source: "claude" + ), + busy: false, + onSelectOption: { _, _ in }, + onSubmitAll: { _, _ in }, + onDecline: {}, + fallbackProvider: "claude", + maxCardHeight: max(160, min((720 - 132) * 0.82, 720 * 0.62)) + ) + .padding(16) + } + .frame(maxWidth: .infinity, maxHeight: .infinity) + .background(ADEColor.pageBackground) + .preferredColorScheme(.dark) +} + #Preview("New chat") { NavigationStack { WorkNewChatScreen( From 9c4782720b8fc5bd19284c8397232d1c8dd1350c Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 26 Jul 2026 01:27:42 -0400 Subject: [PATCH 03/12] Never persist secret question answers; share the height budget - A question can be marked isSecret (rendered in a SecureField, and the resolved card already refuses to echo it back). The new draft persistence would have written those answers to App Group UserDefaults in plaintext, where the widget extension can read them. Exclude them. - Extract workPendingInputMaxHeight so the previews exercise the same arithmetic as the app instead of three hand-copied literals, and retune it (composer reserve 132 -> 110, scroll floor 88 -> 64) so a small phone with the keyboard up keeps an option row visible. Co-Authored-By: Claude --- .../Work/WorkChatComposerAndInputViews.swift | 25 +++++++++---- .../ADE/Views/Work/WorkChatSessionView.swift | 36 +++++++++++++------ apps/ios/ADE/Views/Work/WorkPreviews.swift | 6 ++-- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift index 352a4f6ef..c77660f96 100644 --- a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift +++ b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift @@ -1102,10 +1102,12 @@ struct WorkStructuredQuestionCard: View { private var bodyMaxHeight: CGFloat { let budget = maxCardHeight > 0 ? maxCardHeight : 320 let chrome = topChromeHeight + bottomChromeHeight + Self.cardFixedInsets - // Floor at 88pt: if the chrome alone eats the budget (tiny screen, keyboard - // up, freeform field expanded) we'd rather let the card overflow slightly - // than collapse the option list to nothing. - return max(88, budget - chrome) + // Floor at 64pt — roughly one option row. On a small phone with the keyboard + // up the fixed chrome alone can exceed the budget; collapsing the option + // list to nothing would be worse than overflowing slightly, and the overflow + // is absorbed by the transcript rather than by the footer (see + // `pendingInputMaxHeight`). + return max(64, budget - chrome) } /// Fit the scroll area to its content up to the cap: short lists render at @@ -1221,12 +1223,23 @@ struct WorkStructuredQuestionCard: View { } } + /// Questions whose freeform answer is a secret (rendered in a `SecureField`). + /// Their text is never written to disk — the resolved card already refuses to + /// echo it back, and UserDefaults is an App Group store shared with the widget + /// extension, so persisting it would put a credential in plaintext. + private var secretQuestionIds: Set { + Set(question.questions.filter(\.isSecret).map(\.questionId)) + } + private func persistDrafts() { + let secretIds = secretQuestionIds WorkQuestionDraftStore.save( WorkQuestionDraftStore.Snapshot( selections: selections.mapValues { Array($0).sorted() }, - freeform: freeformByQuestion, - sharedFreeform: singleQuestionFreeformText, + freeform: freeformByQuestion.filter { !secretIds.contains($0.key) }, + // The shared freeform belongs to the single-question card's only + // question, so it inherits that question's secrecy. + sharedFreeform: question.primary.isSecret ? "" : singleQuestionFreeformText, page: currentPage ), for: question.id diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index 006be9c83..3dd8281f3 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -379,17 +379,8 @@ struct WorkChatSessionView: View { max(240, scrollViewportHeight + composerLayoutHeight) } - /// Hard ceiling for the pending-input card. Always leaves room for the - /// composer plus a slice of transcript — a gate that covers the entire screen - /// reads as a modal takeover and hides the Send button. Long content scrolls - /// inside the card instead of growing it. var pendingInputMaxHeight: CGFloat { - let surface = chatSurfaceHeight - // Reserve the composer's own footprint; whatever is left is shared between - // the strip and the transcript, with the strip capped at ~82% of it. - let composerReserve: CGFloat = 132 - let available = max(0, surface - composerReserve) - return max(160, min(available * 0.82, surface * 0.62)) + workPendingInputMaxHeight(chatSurfaceHeight: chatSurfaceHeight) } /// Open approval / permission gates that "Accept all" can sweep. Question, @@ -1313,6 +1304,31 @@ func workLaneListRenderSignature(_ lanes: [LaneSummary]) -> Int { return hasher.finalize() } +/// Hard ceiling for a pending-input card, given the height available to the +/// whole chat surface. Always leaves room for the composer plus a slice of +/// transcript — a gate that covers the entire screen reads as a modal takeover +/// and hides the Send button. Long content scrolls inside the card instead of +/// growing it. +/// +/// If a card's irreducible chrome still exceeds this on a small phone with the +/// keyboard up, the overflow is absorbed by the transcript, not the composer: +/// the composer inset is `fixedSize(vertical:)` and the transcript scroll view +/// is the flexible sibling, so Send/Decline stay on screen either way. That +/// ordering is the actual guarantee — this number just keeps the common case +/// from getting there. +/// +/// A free function rather than a view property so previews exercise the same +/// arithmetic the app uses; the numbers had drifted into three hand-copied +/// literals otherwise. +func workPendingInputMaxHeight(chatSurfaceHeight: CGFloat) -> CGFloat { + // Roughly the composer card's own height in its resting single-line state. + // Measuring it for real is not an option: `composerLayoutHeight` includes the + // strip we are sizing, so reading it here would be circular. + let composerReserve: CGFloat = 110 + let available = max(0, chatSurfaceHeight - composerReserve) + return max(160, min(available * 0.82, chatSurfaceHeight * 0.62)) +} + private struct WorkChatViewportHeightPreferenceKey: PreferenceKey { static var defaultValue: CGFloat = 0 diff --git a/apps/ios/ADE/Views/Work/WorkPreviews.swift b/apps/ios/ADE/Views/Work/WorkPreviews.swift index f1c8d414f..1b5dff8ce 100644 --- a/apps/ios/ADE/Views/Work/WorkPreviews.swift +++ b/apps/ios/ADE/Views/Work/WorkPreviews.swift @@ -568,7 +568,7 @@ private func workPreviewOversizedQuestion() -> WorkPendingQuestionModel { onSubmitAll: { _, _ in }, onDecline: {}, fallbackProvider: "claude", - maxCardHeight: max(160, min((720 - 132) * 0.82, 720 * 0.62)) + maxCardHeight: workPendingInputMaxHeight(chatSurfaceHeight: 720) ) .padding(16) } @@ -589,7 +589,7 @@ private func workPreviewOversizedQuestion() -> WorkPendingQuestionModel { onSubmitAll: { _, _ in }, onDecline: {}, fallbackProvider: "claude", - maxCardHeight: max(160, min((340 - 132) * 0.82, 340 * 0.62)) + maxCardHeight: workPendingInputMaxHeight(chatSurfaceHeight: 340) ) .padding(16) } @@ -624,7 +624,7 @@ private func workPreviewOversizedQuestion() -> WorkPendingQuestionModel { onSubmitAll: { _, _ in }, onDecline: {}, fallbackProvider: "claude", - maxCardHeight: max(160, min((720 - 132) * 0.82, 720 * 0.62)) + maxCardHeight: workPendingInputMaxHeight(chatSurfaceHeight: 720) ) .padding(16) } From de20991bece8f3dfc6f8f772e5e981fbe210ba14 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 26 Jul 2026 01:34:40 -0400 Subject: [PATCH 04/12] Quality pass: name the layout constants, centralize the budgets Track B findings from /quality: - The height budget subtracted a hand-copied 14*2 + 12*2 for card padding and stack spacing while `body` declared those numbers separately. Changing either literal would have silently re-opened the overflow this card exists to prevent, with no compile error. Both now read one pair of named statics. - The inline-in-transcript card still computed its own budget inline; it now calls `workInlinePendingInputMaxHeight` next to its sibling so the two rules' divergence is deliberate rather than accidental. - `readLatestTranscriptTodoItems` delegates to `readTranscriptHydrationState` instead of keeping a second copy of the same scan. - Persist question selections as `Set` directly; the Array round-trip and its `.sorted()` existed only to make an equality check work that Set equality already gives correctly. - Rewrite the `collapsedPendingInputId` comment to state the real reason it is derived state (a minimize must expire when a different gate becomes primary) rather than crediting a compiler limitation. Co-Authored-By: Claude --- .../main/services/chat/agentChatService.ts | 11 ++------ .../Work/WorkChatComposerAndInputViews.swift | 25 +++++++++++++------ .../Work/WorkChatSessionView+Timeline.swift | 6 ++--- .../ADE/Views/Work/WorkChatSessionView.swift | 21 ++++++++++++---- 4 files changed, 38 insertions(+), 25 deletions(-) diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index a320609ed..47f41c1ea 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -8347,15 +8347,8 @@ export function createAgentChatService(args: { const readLatestTranscriptTodoItems = ( managed: ManagedChatSession, - ): Extract["items"] => { - let latest: Extract["items"] = []; - for (const entry of readTranscriptEnvelopes(managed)) { - if (entry.event.type === "todo_update") { - latest = entry.event.items; - } - } - return latest; - }; + ): Extract["items"] => + readTranscriptHydrationState(managed).todoItems; /** Everything a rehydrated session has to recover from its own transcript, * read in one pass (the transcript is not cached, so this is deliberately not diff --git a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift index c77660f96..704e5a0f3 100644 --- a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift +++ b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift @@ -964,7 +964,7 @@ struct WorkPendingInputHeightBoundedCard: View { /// versioned key, bounded and evicted oldest-first. enum WorkQuestionDraftStore { struct Snapshot: Codable, Equatable { - var selections: [String: [String]] = [:] + var selections: [String: Set] = [:] var freeform: [String: String] = [:] var sharedFreeform: String = "" var page: Int = 0 @@ -1091,10 +1091,19 @@ struct WorkStructuredQuestionCard: View { return question.questions[index] } + /// Layout constants the height budget depends on. They are named rather than + /// literal because the budget arithmetic below has to agree with the actual + /// `adeGlassCard` padding and `VStack` spacing used in `body` — a silent + /// disagreement re-opens the exact overflow this card exists to prevent, with + /// no compile error and no symptom until a long option list appears. + private static let cardPadding: CGFloat = 14 + private static let cardStackSpacing: CGFloat = 12 + /// Vertical space the card spends outside the scroll region: the glass card's - /// own padding (14 top + 14 bottom) plus the two 12pt VStack gaps that flank - /// the scroll view. - private static let cardFixedInsets: CGFloat = 14 * 2 + 12 * 2 + /// padding top and bottom, plus the two stack gaps that flank the scroll view. + private static var cardFixedInsets: CGFloat { + cardPadding * 2 + cardStackSpacing * 2 + } /// Height the scroll region may occupy. When no budget has been measured yet /// we fall back to a conservative constant rather than "unbounded" so a slow @@ -1120,7 +1129,7 @@ struct WorkStructuredQuestionCard: View { } var body: some View { - VStack(alignment: .leading, spacing: 12) { + VStack(alignment: .leading, spacing: Self.cardStackSpacing) { topChrome .background(chromeHeightReader(WorkQuestionTopChromeHeightKey.self)) @@ -1153,7 +1162,7 @@ struct WorkStructuredQuestionCard: View { bottomChrome .background(chromeHeightReader(WorkQuestionBottomChromeHeightKey.self)) } - .adeGlassCard(cornerRadius: 18, padding: 14) + .adeGlassCard(cornerRadius: 18, padding: Self.cardPadding) .overlay( RoundedRectangle(cornerRadius: 18, style: .continuous) .stroke(providerAccent.opacity(0.30), lineWidth: 1) @@ -1215,7 +1224,7 @@ struct WorkStructuredQuestionCard: View { // Only restore into an untouched card — a card already mid-edit (the same // request re-rendering) must win over what's on disk. guard selections.isEmpty, freeformByQuestion.isEmpty, singleQuestionFreeformText.isEmpty else { return } - selections = stored.selections.mapValues(Set.init) + selections = stored.selections freeformByQuestion = stored.freeform singleQuestionFreeformText = stored.sharedFreeform if stored.page > 0, stored.page < question.questions.count { @@ -1235,7 +1244,7 @@ struct WorkStructuredQuestionCard: View { let secretIds = secretQuestionIds WorkQuestionDraftStore.save( WorkQuestionDraftStore.Snapshot( - selections: selections.mapValues { Array($0).sorted() }, + selections: selections, freeform: freeformByQuestion.filter { !secretIds.contains($0.key) }, // The shared freeform belongs to the single-question card's only // question, so it inherits that question's secrecy. diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift index bad00f697..fd58e589e 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift @@ -163,9 +163,9 @@ extension WorkChatSessionView { } }, fallbackProvider: chatSummaryContext.provider, - // Inline-in-transcript variant: the transcript viewport is the budget - // here, not the whole surface (the composer sits below it either way). - maxCardHeight: max(240, scrollViewportHeight * 0.62) + maxCardHeight: workInlinePendingInputMaxHeight( + transcriptViewportHeight: scrollViewportHeight + ) ) .id("pending-question-\(question.id)") case .pendingPermission(let permission): diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index 3dd8281f3..35d23b1c0 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -246,11 +246,13 @@ struct WorkChatSessionView: View { /// item leaves the derived queue (or rolled back if the command errored). See /// `dispatchPendingInputAnswer` / `reconcileOptimisticallyAnsweredInputs`. @State var optimisticallyAnsweredInputIds: Set = [] - /// Id of the pending input the user minimized, if any. Stored as an id rather - /// than a Bool so a different request becoming primary re-expands the strip on - /// its own — a minimize applies to the gate the user chose to defer, never to - /// the next one, and deriving it this way needs no `onChange` (which `body`'s - /// modifier chain has no type-inference budget left for). + /// Id of the pending input the user minimized, if any. + /// + /// Derived, not synchronized: a minimize applies to the gate the user chose to + /// defer, so it has to expire on its own the moment a different gate becomes + /// primary. Storing the id and computing the Bool from it makes that + /// impossible to get wrong; a Bool reset from an observer would be one more + /// thing that can fall out of step and leave a fresh question hidden. @State var collapsedPendingInputId: String? var sessionStatus: String { @@ -1329,6 +1331,15 @@ func workPendingInputMaxHeight(chatSurfaceHeight: CGFloat) -> CGFloat { return max(160, min(available * 0.82, chatSurfaceHeight * 0.62)) } +/// Budget for the inline-in-transcript question card, which is bounded by the +/// transcript viewport rather than the whole surface (the composer sits below +/// that viewport either way, so there is nothing to reserve for it). Kept beside +/// `workPendingInputMaxHeight` so the two rules' divergence is deliberate and +/// visible instead of an inline literal drifting on its own. +func workInlinePendingInputMaxHeight(transcriptViewportHeight: CGFloat) -> CGFloat { + max(240, transcriptViewportHeight * 0.62) +} + private struct WorkChatViewportHeightPreferenceKey: PreferenceKey { static var defaultValue: CGFloat = 0 From 732106c0aa87eeb0279693b000598e46d2a05a99 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 26 Jul 2026 12:19:43 -0400 Subject: [PATCH 05/12] Consolidate draft persistence into one file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Track B structural findings. Behavior-preserving. - New WorkDraftPersistence.swift owns both draft stores, the storage mechanism they share, and the composer-draft modifier. WorkQuestionDraftStore had been living in a View file while its identically-shaped sibling lived in WorkModels.swift, which is how their duplication stayed invisible; both files drop back under the 1k-line threshold. - Extract WorkDefaultsJSONMap (load / persist / evictingOldest) as three free functions. The two stores agree on the mechanism but not the policy — key, cap, and what counts as a no-op write all genuinely differ — so a shared store type would only have to model those differences back out. - Move questionDrafts' updatedAt out of Snapshot into a Stored wrapper, so the "did the answer change?" check no longer has to neutralize the timestamp on a mutable copy first. Key bumped to v2; open-gate drafts are ephemeral. - Replace the hand-copied restore/debounce/flush blocks in the Hub and New Chat composers with .workPersistedDraft(_:key:), and share the 400ms debounce with the two surfaces that schedule their own. - Gate the question card's keyboard Done toolbar on freeformFocused. Keyboard toolbars scope to the enclosing view and this card sits just above the main composer's UITextView, which that button cannot dismiss — a leaked Done that silently does nothing is worse than no Done at all. Co-Authored-By: Claude --- apps/ios/ADE.xcodeproj/project.pbxproj | 4 + .../ios/ADE/Views/Hub/HubComposerDrawer.swift | 17 +- .../Work/WorkChatComposerAndInputViews.swift | 98 ++------ .../ADE/Views/Work/WorkChatSessionView.swift | 2 +- .../ADE/Views/Work/WorkDraftPersistence.swift | 236 ++++++++++++++++++ apps/ios/ADE/Views/Work/WorkModels.swift | 87 ------- .../ADE/Views/Work/WorkNewChatScreen.swift | 17 +- 7 files changed, 257 insertions(+), 204 deletions(-) create mode 100644 apps/ios/ADE/Views/Work/WorkDraftPersistence.swift diff --git a/apps/ios/ADE.xcodeproj/project.pbxproj b/apps/ios/ADE.xcodeproj/project.pbxproj index 194723a94..a7f2f4346 100644 --- a/apps/ios/ADE.xcodeproj/project.pbxproj +++ b/apps/ios/ADE.xcodeproj/project.pbxproj @@ -92,6 +92,7 @@ E1000000000000000000002F /* WorkArtifactTerminalViews.swift in Sources */ = {isa = PBXBuildFile; fileRef = D1000000000000000000002F /* WorkArtifactTerminalViews.swift */; }; E10000000000000000000030 /* WorkMarkdownViews.swift in Sources */ = {isa = PBXBuildFile; fileRef = D10000000000000000000030 /* WorkMarkdownViews.swift */; }; E10000000000000000000031 /* WorkModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = D10000000000000000000031 /* WorkModels.swift */; }; + E10000000000000000000601 /* WorkDraftPersistence.swift in Sources */ = {isa = PBXBuildFile; fileRef = D10000000000000000000601 /* WorkDraftPersistence.swift */; }; E10000000000000000000032 /* WorkTranscriptParser.swift in Sources */ = {isa = PBXBuildFile; fileRef = D10000000000000000000032 /* WorkTranscriptParser.swift */; }; E10000000000000000000033 /* WorkNavigationAndTranscriptHelpers.swift in Sources */ = {isa = PBXBuildFile; fileRef = D10000000000000000000033 /* WorkNavigationAndTranscriptHelpers.swift */; }; E10000000000000000000034 /* WorkMarkdownParsing.swift in Sources */ = {isa = PBXBuildFile; fileRef = D10000000000000000000034 /* WorkMarkdownParsing.swift */; }; @@ -362,6 +363,7 @@ D1000000000000000000002F /* WorkArtifactTerminalViews.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = WorkArtifactTerminalViews.swift; path = ADE/Views/Work/WorkArtifactTerminalViews.swift; sourceTree = ""; }; D10000000000000000000030 /* WorkMarkdownViews.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = WorkMarkdownViews.swift; path = ADE/Views/Work/WorkMarkdownViews.swift; sourceTree = ""; }; D10000000000000000000031 /* WorkModels.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = WorkModels.swift; path = ADE/Views/Work/WorkModels.swift; sourceTree = ""; }; + D10000000000000000000601 /* WorkDraftPersistence.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = WorkDraftPersistence.swift; path = ADE/Views/Work/WorkDraftPersistence.swift; sourceTree = ""; }; D10000000000000000000032 /* WorkTranscriptParser.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = WorkTranscriptParser.swift; path = ADE/Views/Work/WorkTranscriptParser.swift; sourceTree = ""; }; D10000000000000000000033 /* WorkNavigationAndTranscriptHelpers.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = WorkNavigationAndTranscriptHelpers.swift; path = ADE/Views/Work/WorkNavigationAndTranscriptHelpers.swift; sourceTree = ""; }; D10000000000000000000034 /* WorkMarkdownParsing.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = WorkMarkdownParsing.swift; path = ADE/Views/Work/WorkMarkdownParsing.swift; sourceTree = ""; }; @@ -829,6 +831,7 @@ D1000000000000000000002F /* WorkArtifactTerminalViews.swift */, D10000000000000000000030 /* WorkMarkdownViews.swift */, D10000000000000000000031 /* WorkModels.swift */, + D10000000000000000000601 /* WorkDraftPersistence.swift */, D10000000000000000000032 /* WorkTranscriptParser.swift */, D10000000000000000000033 /* WorkNavigationAndTranscriptHelpers.swift */, D10000000000000000000034 /* WorkMarkdownParsing.swift */, @@ -1508,6 +1511,7 @@ E1000000000000000000002F /* WorkArtifactTerminalViews.swift in Sources */, E10000000000000000000030 /* WorkMarkdownViews.swift in Sources */, E10000000000000000000031 /* WorkModels.swift in Sources */, + E10000000000000000000601 /* WorkDraftPersistence.swift in Sources */, E10000000000000000000032 /* WorkTranscriptParser.swift in Sources */, E10000000000000000000033 /* WorkNavigationAndTranscriptHelpers.swift in Sources */, E10000000000000000000034 /* WorkMarkdownParsing.swift in Sources */, diff --git a/apps/ios/ADE/Views/Hub/HubComposerDrawer.swift b/apps/ios/ADE/Views/Hub/HubComposerDrawer.swift index 2bf99136c..2de053ea6 100644 --- a/apps/ios/ADE/Views/Hub/HubComposerDrawer.swift +++ b/apps/ios/ADE/Views/Hub/HubComposerDrawer.swift @@ -274,22 +274,7 @@ struct HubInlineComposer: View { } ) .onAppear { onAppearSetup() } - // Restore whatever the user last typed here but never sent. Guarded on - // empty so a re-appear (or an init-seeded value) can't clobber live text. - .task { - if draft.isEmpty { - draft = WorkComposerDraftStore.load(WorkComposerDraftStore.hubNewChatKey) - } - } - // Debounced autosave: each keystroke restarts this task, and the cancelled - // sleep throws before the write, so only a typing pause hits UserDefaults. - .task(id: draft) { - try? await Task.sleep(for: .milliseconds(400)) - guard !Task.isCancelled else { return } - WorkComposerDraftStore.save(draft, for: WorkComposerDraftStore.hubNewChatKey) - } - // The debounce dies with the view, so flush the final text on teardown. - .onDisappear { WorkComposerDraftStore.save(draft, for: WorkComposerDraftStore.hubNewChatKey) } + .workPersistedDraft($draft, key: WorkComposerDraftStore.hubNewChatKey) .onChange(of: composerFocused) { _, focused in if focused { withAnimation(hubComposerSpring) { expanded = true } } } diff --git a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift index 704e5a0f3..d625eeec3 100644 --- a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift +++ b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift @@ -956,84 +956,6 @@ struct WorkPendingInputHeightBoundedCard: View { } } -/// In-progress answers for a still-open question request, persisted per request -/// id. The card's selections and freeform text were plain `@State`, so backing -/// out of a chat to check something in the transcript — the exact reason a user -/// minimizes the card — silently discarded everything they had picked or typed. -/// Same storage shape as `WorkComposerDraftStore`: one JSON dictionary under a -/// versioned key, bounded and evicted oldest-first. -enum WorkQuestionDraftStore { - struct Snapshot: Codable, Equatable { - var selections: [String: Set] = [:] - var freeform: [String: String] = [:] - var sharedFreeform: String = "" - var page: Int = 0 - var updatedAt: Double = 0 - - /// Nothing worth persisting — used to decide between a write and a removal. - var isEmpty: Bool { - selections.values.allSatisfy(\.isEmpty) - && freeform.values.allSatisfy { $0.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty } - && sharedFreeform.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty - && page == 0 - } - } - - private static let storageKey = "ade.work.questionDrafts.v1" - /// Open question gates are short-lived; a small cap is plenty and keeps the - /// blob from accumulating answers to requests that were resolved elsewhere. - private static let maxEntries = 30 - private static var defaults: UserDefaults { ADESharedContainer.defaults } - - static func load(_ requestId: String) -> Snapshot? { - guard !requestId.isEmpty else { return nil } - return loadAll()[requestId] - } - - static func save(_ snapshot: Snapshot, for requestId: String) { - guard !requestId.isEmpty else { return } - guard !snapshot.isEmpty else { - clear(requestId) - return - } - var map = loadAll() - var stamped = snapshot - stamped.updatedAt = Date().timeIntervalSince1970 - // Compare ignoring the timestamp so an unchanged draft costs no write. - if var existing = map[requestId] { - existing.updatedAt = stamped.updatedAt - if existing == stamped { return } - } - map[requestId] = stamped - if map.count > maxEntries { - let survivors = map - .sorted { $0.value.updatedAt > $1.value.updatedAt } - .prefix(maxEntries) - map = Dictionary(uniqueKeysWithValues: survivors.map { ($0.key, $0.value) }) - } - persist(map) - } - - static func clear(_ requestId: String) { - guard !requestId.isEmpty else { return } - var map = loadAll() - guard map.removeValue(forKey: requestId) != nil else { return } - persist(map) - } - - private static func loadAll() -> [String: Snapshot] { - guard let data = defaults.data(forKey: storageKey), - let decoded = try? JSONDecoder().decode([String: Snapshot].self, from: data) - else { return [:] } - return decoded - } - - private static func persist(_ map: [String: Snapshot]) { - guard let data = try? JSONEncoder().encode(map) else { return } - defaults.set(data, forKey: storageKey) - } -} - struct WorkStructuredQuestionCard: View { let question: WorkPendingQuestionModel let busy: Bool @@ -1185,7 +1107,7 @@ struct WorkStructuredQuestionCard: View { // Keystroke debounce: each edit cancels the pending sleep and restarts it, // so a burst of typing costs one write instead of one per character. guard didRestoreDrafts else { return } - try? await Task.sleep(for: .milliseconds(400)) + try? await Task.sleep(for: workDraftAutosaveDebounce) guard !Task.isCancelled else { return } persistDrafts() } @@ -1474,12 +1396,20 @@ struct WorkStructuredQuestionCard: View { } // Standard iOS escape hatch from a multi-line field: the vertical-axis // TextField swallows Return as a newline, so without an explicit Done there - // is no way to lower the keyboard. Scoped to this card's fields. + // is no way to lower the keyboard. + // + // Gated on `freeformFocused` rather than declared unconditionally: keyboard + // toolbars are scoped to the enclosing view, and this card is mounted a few + // points above the main chat composer — a UITextView this Done button cannot + // dismiss. If the toolbar ever surfaced over that keyboard, the button would + // silently do nothing, which is worse than having no button at all. .toolbar { - ToolbarItemGroup(placement: .keyboard) { - Spacer() - Button("Done") { freeformFocused = false } - .accessibilityLabel("Dismiss keyboard") + if freeformFocused { + ToolbarItemGroup(placement: .keyboard) { + Spacer() + Button("Done") { freeformFocused = false } + .accessibilityLabel("Dismiss keyboard") + } } } } diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index 35d23b1c0..d138f72fd 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -2318,7 +2318,7 @@ final class WorkChatComposerDraftState: ObservableObject { let key = persistenceKey let value = text autosaveTask = Task { @MainActor in - try? await Task.sleep(for: .milliseconds(400)) + try? await Task.sleep(for: workDraftAutosaveDebounce) guard !Task.isCancelled else { return } WorkComposerDraftStore.save(value, for: key) } diff --git a/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift b/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift new file mode 100644 index 000000000..5518fdabf --- /dev/null +++ b/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift @@ -0,0 +1,236 @@ +import Foundation +import SwiftUI + +/// Storage mechanism shared by the Work draft stores: one versioned JSON +/// dictionary in the App Group defaults, small enough to rewrite whole on each +/// save and bounded (LRU by `updatedAt`) so a long-lived install can't grow it +/// without limit. +/// +/// Deliberately three free functions rather than a generic store type — the two +/// call sites agree on the *mechanism* but not on the *policy* (key, cap, and +/// what counts as a no-op write all genuinely differ), and a shared type would +/// have to model those differences back out again. +enum WorkDefaultsJSONMap { + private static var defaults: UserDefaults { ADESharedContainer.defaults } + + /// The stored map, or an empty one when the key is absent or the blob no + /// longer matches the current shape — restoring a draft must never be able to + /// fail loudly in a view body. + static func load(_ storageKey: String) -> [String: V] { + guard let data = defaults.data(forKey: storageKey), + let decoded = try? JSONDecoder().decode([String: V].self, from: data) + else { return [:] } + return decoded + } + + static func persist(_ map: [String: V], under storageKey: String) { + guard let data = try? JSONEncoder().encode(map) else { return } + defaults.set(data, forKey: storageKey) + } + + /// Trims the map to `maxEntries`, dropping least-recently-updated entries + /// first. Returned rather than mutated in place so callers keep their single + /// "build the map, then persist it" statement order. + static func evictingOldest( + _ map: [String: V], + keeping maxEntries: Int, + updatedAt: (V) -> Double + ) -> [String: V] { + guard map.count > maxEntries else { return map } + let survivors = map + .sorted { updatedAt($0.value) > updatedAt($1.value) } + .prefix(maxEntries) + return Dictionary(uniqueKeysWithValues: survivors.map { ($0.key, $0.value) }) + } +} + +/// Keystroke debounce for every Work draft autosave. Long enough that a burst of +/// typing costs one `UserDefaults` write instead of one per character, short +/// enough that a user who pauses and then kills the app keeps their text. +/// Shared so the surfaces that schedule their own autosave (the question card +/// and the in-session composer, whose payloads aren't a plain `String` binding) +/// can't drift from the modifier below. +let workDraftAutosaveDebounce: Duration = .milliseconds(400) + +/// Unsent composer text, persisted per surface so leaving a chat (or the app) +/// never discards what the user typed — desktop keeps its draft, and mobile +/// silently dropping it was the single most-reported chat regression. +/// One JSON dictionary under a versioned key: small enough to rewrite whole on +/// each save, bounded by `maxEntries` (LRU by `updatedAt`) so a long-lived +/// install can't grow it without limit. +enum WorkComposerDraftStore { + struct Entry: Codable, Equatable { + var text: String + var updatedAt: Double + } + + /// Versioned so a future shape change can migrate rather than mis-decode. + private static let storageKey = "ade.work.composerDrafts.v1" + /// Enough to cover every chat a user realistically juggles; older drafts are + /// evicted oldest-first rather than kept forever. + private static let maxEntries = 60 + /// A composer draft is a prompt, not a document — clamp pathological pastes so + /// one entry can't dominate the shared defaults store. + private static let maxLength = 20_000 + + /// Per-chat key. Blank session ids yield a blank key so callers that render + /// before the session resolves can't write everyone's draft into one bucket. + static func chatKey(sessionId: String) -> String { + let trimmed = sessionId.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmed.isEmpty else { return "" } + return "chat:\(trimmed)" + } + + /// The two "new chat" composers are singletons, so they get fixed keys. + static let hubNewChatKey = "hub-new-chat" + static let workNewChatKey = "work-new-chat" + + /// The stored draft, or "" when the key is blank, absent, or undecodable — + /// restoring must never be able to fail loudly in a view body. + static func load(_ key: String) -> String { + guard !key.isEmpty else { return "" } + return loadAll()[key]?.text ?? "" + } + + /// Persists (or clears) the draft for one surface. An emptied composer removes + /// its entry outright: a user who deletes their text must not have it + /// resurrected the next time the screen mounts. + static func save(_ text: String, for key: String) { + guard !key.isEmpty else { return } + var map = loadAll() + guard !text.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { + guard map.removeValue(forKey: key) != nil else { return } + WorkDefaultsJSONMap.persist(map, under: storageKey) + return + } + let clipped = String(text.prefix(maxLength)) + // Autosave runs on a keystroke debounce; skip the UserDefaults write when + // the content is unchanged so idle typing pauses cost nothing. + if map[key]?.text == clipped { return } + map[key] = Entry(text: clipped, updatedAt: Date().timeIntervalSince1970) + map = WorkDefaultsJSONMap.evictingOldest(map, keeping: maxEntries, updatedAt: \.updatedAt) + WorkDefaultsJSONMap.persist(map, under: storageKey) + } + + /// Drops a draft that has been consumed (sent) so it can't reappear. + static func clear(_ key: String) { + guard !key.isEmpty else { return } + var map = loadAll() + guard map.removeValue(forKey: key) != nil else { return } + WorkDefaultsJSONMap.persist(map, under: storageKey) + } + + private static func loadAll() -> [String: Entry] { + WorkDefaultsJSONMap.load(storageKey) + } +} + +/// In-progress answers for a still-open question request, persisted per request +/// id. The card's selections and freeform text were plain `@State`, so backing +/// out of a chat to check something in the transcript — the exact reason a user +/// minimizes the card — silently discarded everything they had picked or typed. +/// Same storage shape as `WorkComposerDraftStore`: one JSON dictionary under a +/// versioned key, bounded and evicted oldest-first. +enum WorkQuestionDraftStore { + struct Snapshot: Codable, Equatable { + var selections: [String: Set] = [:] + var freeform: [String: String] = [:] + var sharedFreeform: String = "" + var page: Int = 0 + + /// Nothing worth persisting — used to decide between a write and a removal. + var isEmpty: Bool { + selections.values.allSatisfy(\.isEmpty) + && freeform.values.allSatisfy { $0.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty } + && sharedFreeform.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + && page == 0 + } + } + + /// The timestamp lives beside the snapshot, not inside it: metadata that + /// changes on every write cannot also be part of the value it timestamps, or + /// "did the answer actually change?" can only be asked by first neutralizing + /// the field. Mirrors `WorkComposerDraftStore.Entry`. + private struct Stored: Codable { + var snapshot: Snapshot + var updatedAt: Double + } + + /// v2 because `updatedAt` moved out of `Snapshot` into the wrapper. Drafts for + /// open gates are ephemeral, so the v1 blob is abandoned rather than migrated. + private static let storageKey = "ade.work.questionDrafts.v2" + /// Open question gates are short-lived; a small cap is plenty and keeps the + /// blob from accumulating answers to requests that were resolved elsewhere. + private static let maxEntries = 30 + + static func load(_ requestId: String) -> Snapshot? { + guard !requestId.isEmpty else { return nil } + return loadAll()[requestId]?.snapshot + } + + static func save(_ snapshot: Snapshot, for requestId: String) { + guard !requestId.isEmpty else { return } + guard !snapshot.isEmpty else { + clear(requestId) + return + } + var map = loadAll() + // Autosave runs on a keystroke debounce; skip the write when the answer is + // unchanged so idle typing pauses cost nothing. + if map[requestId]?.snapshot == snapshot { return } + map[requestId] = Stored(snapshot: snapshot, updatedAt: Date().timeIntervalSince1970) + map = WorkDefaultsJSONMap.evictingOldest(map, keeping: maxEntries, updatedAt: \.updatedAt) + WorkDefaultsJSONMap.persist(map, under: storageKey) + } + + static func clear(_ requestId: String) { + guard !requestId.isEmpty else { return } + var map = loadAll() + guard map.removeValue(forKey: requestId) != nil else { return } + WorkDefaultsJSONMap.persist(map, under: storageKey) + } + + private static func loadAll() -> [String: Stored] { + WorkDefaultsJSONMap.load(storageKey) + } +} + +/// The three legs of composer-draft persistence, which only work as a set. +/// +/// - Restore is guarded on empty because a re-appear (or an init-seeded value, +/// or a failed send that put its text back) is fresher than what's on disk; +/// an unguarded restore would clobber text the user can see. +/// - The autosave debounce is what keeps typing off `UserDefaults`, but a +/// cancelled `.task` throws out of its sleep *before* the write, so the +/// in-flight edit is lost on any teardown. +/// - Hence the flush on disappear: a navigation pop is exactly the case the +/// debounce misses, and it is also the most common way a draft is abandoned. +private struct WorkPersistedDraftModifier: ViewModifier { + @Binding var text: String + let key: String + + func body(content: Content) -> some View { + content + .task { + if text.isEmpty { + text = WorkComposerDraftStore.load(key) + } + } + .task(id: text) { + try? await Task.sleep(for: workDraftAutosaveDebounce) + guard !Task.isCancelled else { return } + WorkComposerDraftStore.save(text, for: key) + } + .onDisappear { WorkComposerDraftStore.save(text, for: key) } + } +} + +extension View { + /// Restore-if-empty on appear, debounced autosave while typing, flush on + /// teardown — see `WorkPersistedDraftModifier` for why all three legs are + /// required. Send paths still call `WorkComposerDraftStore.clear(_:)` + /// explicitly: consuming a draft is not the same event as leaving the screen. + func workPersistedDraft(_ text: Binding, key: String) -> some View { + modifier(WorkPersistedDraftModifier(text: text, key: key)) + } +} diff --git a/apps/ios/ADE/Views/Work/WorkModels.swift b/apps/ios/ADE/Views/Work/WorkModels.swift index fe64a09df..305e42f50 100644 --- a/apps/ios/ADE/Views/Work/WorkModels.swift +++ b/apps/ios/ADE/Views/Work/WorkModels.swift @@ -64,93 +64,6 @@ enum WorkComposerPreferences { } } -/// Unsent composer text, persisted per surface so leaving a chat (or the app) -/// never discards what the user typed — desktop keeps its draft, and mobile -/// silently dropping it was the single most-reported chat regression. -/// One JSON dictionary under a versioned key: small enough to rewrite whole on -/// each save, bounded by `maxEntries` (LRU by `updatedAt`) so a long-lived -/// install can't grow it without limit. -enum WorkComposerDraftStore { - struct Entry: Codable, Equatable { - var text: String - var updatedAt: Double - } - - /// Versioned so a future shape change can migrate rather than mis-decode. - private static let storageKey = "ade.work.composerDrafts.v1" - /// Enough to cover every chat a user realistically juggles; older drafts are - /// evicted oldest-first rather than kept forever. - private static let maxEntries = 60 - /// A composer draft is a prompt, not a document — clamp pathological pastes so - /// one entry can't dominate the shared defaults store. - private static let maxLength = 20_000 - private static var defaults: UserDefaults { ADESharedContainer.defaults } - - /// Per-chat key. Blank session ids yield a blank key so callers that render - /// before the session resolves can't write everyone's draft into one bucket. - static func chatKey(sessionId: String) -> String { - let trimmed = sessionId.trimmingCharacters(in: .whitespacesAndNewlines) - guard !trimmed.isEmpty else { return "" } - return "chat:\(trimmed)" - } - - /// The two "new chat" composers are singletons, so they get fixed keys. - static let hubNewChatKey = "hub-new-chat" - static let workNewChatKey = "work-new-chat" - - /// The stored draft, or "" when the key is blank, absent, or undecodable — - /// restoring must never be able to fail loudly in a view body. - static func load(_ key: String) -> String { - guard !key.isEmpty else { return "" } - return loadAll()[key]?.text ?? "" - } - - /// Persists (or clears) the draft for one surface. An emptied composer removes - /// its entry outright: a user who deletes their text must not have it - /// resurrected the next time the screen mounts. - static func save(_ text: String, for key: String) { - guard !key.isEmpty else { return } - var map = loadAll() - guard !text.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else { - guard map.removeValue(forKey: key) != nil else { return } - persist(map) - return - } - let clipped = String(text.prefix(maxLength)) - // Autosave runs on a keystroke debounce; skip the UserDefaults write when - // the content is unchanged so idle typing pauses cost nothing. - if map[key]?.text == clipped { return } - map[key] = Entry(text: clipped, updatedAt: Date().timeIntervalSince1970) - if map.count > maxEntries { - let survivors = map - .sorted { $0.value.updatedAt > $1.value.updatedAt } - .prefix(maxEntries) - map = Dictionary(uniqueKeysWithValues: survivors.map { ($0.key, $0.value) }) - } - persist(map) - } - - /// Drops a draft that has been consumed (sent) so it can't reappear. - static func clear(_ key: String) { - guard !key.isEmpty else { return } - var map = loadAll() - guard map.removeValue(forKey: key) != nil else { return } - persist(map) - } - - private static func loadAll() -> [String: Entry] { - guard let data = defaults.data(forKey: storageKey), - let decoded = try? JSONDecoder().decode([String: Entry].self, from: data) - else { return [:] } - return decoded - } - - private static func persist(_ map: [String: Entry]) { - guard let data = try? JSONEncoder().encode(map) else { return } - defaults.set(data, forKey: storageKey) - } -} - enum WorkToolCardStatus: String, Equatable { case running case completed diff --git a/apps/ios/ADE/Views/Work/WorkNewChatScreen.swift b/apps/ios/ADE/Views/Work/WorkNewChatScreen.swift index 2b46192e3..a039792a4 100644 --- a/apps/ios/ADE/Views/Work/WorkNewChatScreen.swift +++ b/apps/ios/ADE/Views/Work/WorkNewChatScreen.swift @@ -1560,22 +1560,7 @@ private struct WorkNewChatComposerBar: View { attachments: $attachments, onDismiss: { composerFocused = true } ) - // Restore whatever the user last typed here but never sent. Guarded on - // empty so a re-appear can't clobber live text. - .task { - if draft.isEmpty { - draft = WorkComposerDraftStore.load(WorkComposerDraftStore.workNewChatKey) - } - } - // Debounced autosave: each keystroke restarts this task, and the cancelled - // sleep throws before the write, so only a typing pause hits UserDefaults. - .task(id: draft) { - try? await Task.sleep(for: .milliseconds(400)) - guard !Task.isCancelled else { return } - WorkComposerDraftStore.save(draft, for: WorkComposerDraftStore.workNewChatKey) - } - // The debounce dies with the view, so flush the final text on teardown. - .onDisappear { WorkComposerDraftStore.save(draft, for: WorkComposerDraftStore.workNewChatKey) } + .workPersistedDraft($draft, key: WorkComposerDraftStore.workNewChatKey) } /// Primary foreground launch button — the compact arrow-in-circle send glyph From bf41ad68454b10aa08ccd78677ee137831a86ff8 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 26 Jul 2026 12:40:22 -0400 Subject: [PATCH 06/12] Quality gate: drop the duplicate-gate regression, purge legacy drafts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Track A findings. BLOCKER — revert `askuserquestion` / `ask_user_question` from isAskUserToolName. The host emits BOTH a tool_call for the tool-use block and a separate approval_request for the gate, and those carry different item ids (the SDK tool-use id vs a fresh randomUUID). derivePendingWorkInputs dedupes by item id, so matching the tool name produced TWO cards for one Claude question — and the tool_call-derived one is unanswerable, because the host finds no approval registered under that id and discards the response silently. That is a regression on the single most common gate in the product, which is the flow this lane exists to fix. The tool_call branch stays a fallback for hosts that emit a bare ask-user call with no wrapping approval; no host in this repo does. Also: - Actively delete the v1 question-draft blob rather than abandoning it. An intermediate build of this lane persisted isSecret answers before that exclusion landed, so a stale v1 entry can hold a plaintext secret. Never shipped in a release, but dev and TestFlight devices ran it. - Clear the persisted draft synchronously on send instead of leaving it to the 400ms debounce; a jetsam inside that window restored an already-sent message into the composer, where it reads as unsent. - Drop both draft stores when the phone forgets a machine. They are bounded by entry count, not lifetime, and the trust reset already promises to clear machine-scoped drafts. - Guess small, not full-budget, for a bounded card's pre-measurement frame, so a short permission gate stops shoving the transcript up and back. Co-Authored-By: Claude --- apps/ios/ADE/Services/SyncService.swift | 7 ++++ .../Work/WorkChatComposerAndInputViews.swift | 12 ++++++- .../ADE/Views/Work/WorkChatSessionView.swift | 16 ++++++++++ .../ADE/Views/Work/WorkDraftPersistence.swift | 32 +++++++++++++++++-- .../Work/WorkErrorAndMessageHelpers.swift | 21 ++++++------ 5 files changed, 75 insertions(+), 13 deletions(-) diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 2f8488ec0..93072e0fc 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -10407,6 +10407,13 @@ final class SyncService: ObservableObject { } else { UserDefaults.standard.removeObject(forKey: profileKey) UserDefaults.standard.removeObject(forKey: legacyDraftKey) + // Unsent composer text and in-progress question answers are scoped to the + // machine's chats, so forgetting the machine has to drop them too — the + // trust reset already promises to clear machine-scoped drafts, and these + // stores are bounded by entry count, not lifetime, so they would otherwise + // outlive the pairing indefinitely. + WorkComposerDraftStore.clearAll() + WorkQuestionDraftStore.clearAll() activeHostProfile = nil hostName = nil hiddenProjectKeys = loadHiddenProjectKeys() diff --git a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift index d625eeec3..ad7f92c87 100644 --- a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift +++ b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift @@ -919,6 +919,10 @@ private struct WorkPendingCardContentHeightKey: PreferenceKey { /// actually hit. `WorkStructuredQuestionCard` does its own budgeting (it needs /// to keep its footer pinned outside the scroll region) and is not wrapped. struct WorkPendingInputHeightBoundedCard: View { + /// Placeholder height for the single frame before the content reports its own + /// — roughly a two-line permission card. + private static var unmeasuredHeightGuess: CGFloat { 120 } + let maxHeight: CGFloat @ViewBuilder var content: Content @@ -939,7 +943,13 @@ struct WorkPendingInputHeightBoundedCard: View { } ) } - .frame(height: max(1, min(measuredHeight ?? maxHeight, maxHeight))) + // Before the first measurement, guess small rather than taking the whole + // budget. Every card this wraps (permission, approval, plan, model + // selection) is short in the common case, and `composerInset` is + // fixed-size, so a full-budget first frame visibly shoves the transcript + // up and back as it snaps down. Growing into the budget on frame two is + // the less jarring direction to be wrong in. + .frame(height: max(1, min(measuredHeight ?? Self.unmeasuredHeightGuess, maxHeight))) .scrollBounceBehavior(.basedOnSize) .scrollDismissesKeyboard(.interactively) .onPreferenceChange(WorkPendingCardContentHeightKey.self) { height in diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index d138f72fd..a5cf8cd43 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -2332,9 +2332,25 @@ final class WorkChatComposerDraftState: ObservableObject { let value = trimmedText isFocused = false text = "" + // Drop the stored copy synchronously rather than letting the 400ms debounce + // get to it. A jetsam or force-quit inside that window would otherwise + // restore an already-sent message into the composer, where it reads as + // unsent and invites sending it twice. The Hub and New Chat composers clear + // on send for the same reason. + clearStoredDraft() return value } + /// Cancels any pending autosave and removes the persisted draft. Not + /// actor-annotated so `consumeSendableText()` — which runs from the send + /// button's synchronous action — can call it directly. + func clearStoredDraft() { + autosaveTask?.cancel() + autosaveTask = nil + guard !persistenceKey.isEmpty else { return } + WorkComposerDraftStore.clear(persistenceKey) + } + func restoreUnsentText(_ value: String) { let currentDraft = trimmedText if currentDraft != value { diff --git a/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift b/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift index 5518fdabf..6c612c15c 100644 --- a/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift +++ b/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift @@ -123,6 +123,12 @@ enum WorkComposerDraftStore { private static func loadAll() -> [String: Entry] { WorkDefaultsJSONMap.load(storageKey) } + + /// Drops every stored draft. Called when the phone forgets a machine — the + /// drafts belong to that machine's chats. + static func clearAll() { + ADESharedContainer.defaults.removeObject(forKey: storageKey) + } } /// In-progress answers for a still-open question request, persisted per request @@ -157,8 +163,13 @@ enum WorkQuestionDraftStore { } /// v2 because `updatedAt` moved out of `Snapshot` into the wrapper. Drafts for - /// open gates are ephemeral, so the v1 blob is abandoned rather than migrated. + /// open gates are ephemeral, so the v1 blob is dropped rather than migrated. private static let storageKey = "ade.work.questionDrafts.v2" + /// v1 is actively deleted, not just abandoned: an intermediate build of this + /// change persisted answers to `isSecret` questions before that exclusion + /// landed, so a stale v1 blob can hold a plaintext secret. Never shipped in a + /// release, but dev and TestFlight devices ran it. + private static let legacyStorageKey = "ade.work.questionDrafts.v1" /// Open question gates are short-lived; a small cap is plenty and keeps the /// blob from accumulating answers to requests that were resolved elsewhere. private static let maxEntries = 30 @@ -191,7 +202,24 @@ enum WorkQuestionDraftStore { } private static func loadAll() -> [String: Stored] { - WorkDefaultsJSONMap.load(storageKey) + purgeLegacyStoreIfNeeded() + return WorkDefaultsJSONMap.load(storageKey) + } + + /// Runs on first access rather than at launch so the purge can't be skipped by + /// a path that never boots the chat surface. + private static func purgeLegacyStoreIfNeeded() { + let defaults = ADESharedContainer.defaults + guard defaults.object(forKey: legacyStorageKey) != nil else { return } + defaults.removeObject(forKey: legacyStorageKey) + } + + /// Drops every stored answer. Called when the phone forgets a machine — the + /// gates these answer belong to that machine's chats. + static func clearAll() { + let defaults = ADESharedContainer.defaults + defaults.removeObject(forKey: storageKey) + defaults.removeObject(forKey: legacyStorageKey) } } diff --git a/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift b/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift index 7c08b7d22..d5764a950 100644 --- a/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift +++ b/apps/ios/ADE/Views/Work/WorkErrorAndMessageHelpers.swift @@ -1644,18 +1644,19 @@ func derivePendingWorkInputs(from transcript: [WorkChatEnvelope]) -> [WorkPendin return results } +/// Deliberately does NOT match Claude's own `AskUserQuestion` (which normalizes +/// to `askuserquestion`). The host emits a `tool_call` for the tool-use block +/// AND a separate `approval_request` for the gate, and those carry different +/// item ids — the tool-use id versus a fresh `randomUUID()`. Since +/// `derivePendingWorkInputs` dedupes by item id, matching the tool name here +/// yields two cards for one question, the tool_call-derived one being +/// unanswerable (the host has no approval registered under that id, so it +/// discards the response silently). The `tool_call` branch is only a fallback +/// for hosts that emit a bare ask-user call with no wrapping approval; adding a +/// name the real host always wraps turns that fallback into a duplicate. func isAskUserToolName(_ tool: String) -> Bool { let normalized = normalizedWorkToolIdentity(tool) - // `askuserquestion` is the Claude Agent SDK's own `AskUserQuestion` tool after - // normalization (CamelCase collapses to one word, no separators to split on). - // Desktop hosts wrap it in an `approval_request` so the question card comes - // from the request kind, but a bare tool_call from any other host would - // otherwise render as a raw tool row with no way to answer. - return normalized == "ask_user" - || normalized == "askuser" - || normalized == "askuserquestion" - || normalized == "ask_user_question" - || normalized == "mcp_ade_ask_user" + return normalized == "ask_user" || normalized == "askuser" || normalized == "mcp_ade_ask_user" } func isRequestUserInputToolName(_ tool: String) -> Bool { From 91a2d61b17e9aafdcbacb0b11793afe09c05090f Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 26 Jul 2026 12:52:37 -0400 Subject: [PATCH 07/12] Re-review: don't wipe drafts on auto-unpair; make the purge reachable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both findings are defects in the previous commit's own fixes. Clearing the draft stores on unpair looked like hygiene and was data loss. Reaching that branch does not mean the user asked to forget anything: the only production trigger is forgetHost(), which has no UI caller and fires automatically from handleReconnectFailure on an attributed auth failure — a desktop reinstall or a token rotation is enough. And the stores are keyed by session id, not by host, so the wipe would have destroyed unsent text for every other machine still paired, plus the machine-independent Hub and New Chat drafts. A user could lose everything they had typed because a background reconnect failed. Reverted, with the reasoning recorded at the branch. The legacy-secret purge was hung off the question-draft store, whose only callers are inside the question card — so it never ran on precisely the devices it exists for: one that answered a secret question on an intermediate build and never renders another question card would keep the plaintext blob forever. It now runs from the composer store's loadAll too, which every chat open hits. Also corrects the unmeasured-height comment, which claimed the wrapped cards are all short. Two of the four are not (permission 126-360pt, model selection 185pt+), so the guess usually under-shoots — still the better direction, but the comment should say what the code does. Co-Authored-By: Claude --- apps/ios/ADE/Services/SyncService.swift | 16 +++++----- .../Work/WorkChatComposerAndInputViews.swift | 16 +++++----- .../ADE/Views/Work/WorkDraftPersistence.swift | 31 ++++++++----------- 3 files changed, 30 insertions(+), 33 deletions(-) diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 93072e0fc..0584f2cac 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -10407,13 +10407,15 @@ final class SyncService: ObservableObject { } else { UserDefaults.standard.removeObject(forKey: profileKey) UserDefaults.standard.removeObject(forKey: legacyDraftKey) - // Unsent composer text and in-progress question answers are scoped to the - // machine's chats, so forgetting the machine has to drop them too — the - // trust reset already promises to clear machine-scoped drafts, and these - // stores are bounded by entry count, not lifetime, so they would otherwise - // outlive the pairing indefinitely. - WorkComposerDraftStore.clearAll() - WorkQuestionDraftStore.clearAll() + // Deliberately does NOT clear the composer/question draft stores. Reaching + // here does not mean the user asked to forget anything: the only + // production trigger is `forgetHost()`, which has no UI caller and fires + // automatically from `handleReconnectFailure` on an attributed auth + // failure — a desktop reinstall or token rotation is enough. And the + // stores are keyed by session id, not by host, so wiping them would + // destroy unsent text for every OTHER machine still paired, plus the + // machine-independent Hub and New Chat drafts. Losing a user's typed words + // on a background reconnect is far worse than a stale draft lingering. activeHostProfile = nil hostName = nil hiddenProjectKeys = loadHiddenProjectKeys() diff --git a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift index ad7f92c87..f99ab6dc7 100644 --- a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift +++ b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift @@ -919,8 +919,14 @@ private struct WorkPendingCardContentHeightKey: PreferenceKey { /// actually hit. `WorkStructuredQuestionCard` does its own budgeting (it needs /// to keep its footer pinned outside the scroll region) and is not wrapped. struct WorkPendingInputHeightBoundedCard: View { - /// Placeholder height for the single frame before the content reports its own - /// — roughly a two-line permission card. + /// Placeholder height for the frames before the content reports its own. + /// + /// Sits below the cards it wraps rather than above them: the plan and approval + /// strips render around 34pt, a permission card 126-360pt, a model-selection + /// card 185pt+. So this usually under-guesses and the card grows into place — + /// which is the better direction to be wrong in, because `composerInset` is + /// fixed-size, and a card that starts at the full budget (~434pt on a typical + /// iPhone) visibly shoves the transcript up and then drags it back down. private static var unmeasuredHeightGuess: CGFloat { 120 } let maxHeight: CGFloat @@ -943,12 +949,6 @@ struct WorkPendingInputHeightBoundedCard: View { } ) } - // Before the first measurement, guess small rather than taking the whole - // budget. Every card this wraps (permission, approval, plan, model - // selection) is short in the common case, and `composerInset` is - // fixed-size, so a full-budget first frame visibly shoves the transcript - // up and back as it snaps down. Growing into the budget on frame two is - // the less jarring direction to be wrong in. .frame(height: max(1, min(measuredHeight ?? Self.unmeasuredHeightGuess, maxHeight))) .scrollBounceBehavior(.basedOnSize) .scrollDismissesKeyboard(.interactively) diff --git a/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift b/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift index 6c612c15c..28511079f 100644 --- a/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift +++ b/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift @@ -121,13 +121,13 @@ enum WorkComposerDraftStore { } private static func loadAll() -> [String: Entry] { - WorkDefaultsJSONMap.load(storageKey) - } - - /// Drops every stored draft. Called when the phone forgets a machine — the - /// drafts belong to that machine's chats. - static func clearAll() { - ADESharedContainer.defaults.removeObject(forKey: storageKey) + // Piggyback the legacy-secret purge on the store that every chat open and + // every composer keystroke touches. Hanging it off the question-draft store + // alone left it unreachable on exactly the devices that need it: one that + // answered a secret question on an intermediate build and never renders + // another question card would keep the plaintext blob forever. + WorkQuestionDraftStore.purgeLegacyStoreIfNeeded() + return WorkDefaultsJSONMap.load(storageKey) } } @@ -206,21 +206,16 @@ enum WorkQuestionDraftStore { return WorkDefaultsJSONMap.load(storageKey) } - /// Runs on first access rather than at launch so the purge can't be skipped by - /// a path that never boots the chat surface. - private static func purgeLegacyStoreIfNeeded() { + /// Called from both draft stores' `loadAll`, so any chat open triggers it — + /// not just one that happens to render a question card. Cheap after the first + /// run: an absent key is an in-memory dictionary miss, and nothing in the app + /// ever writes `legacyStorageKey` again (there is no `UserDefaults.register` + /// anywhere that could resurrect it). + static func purgeLegacyStoreIfNeeded() { let defaults = ADESharedContainer.defaults guard defaults.object(forKey: legacyStorageKey) != nil else { return } defaults.removeObject(forKey: legacyStorageKey) } - - /// Drops every stored answer. Called when the phone forgets a machine — the - /// gates these answer belong to that machine's chats. - static func clearAll() { - let defaults = ADESharedContainer.defaults - defaults.removeObject(forKey: storageKey) - defaults.removeObject(forKey: legacyStorageKey) - } } /// The three legs of composer-draft persistence, which only work as a set. From 476cd3943f0897e508235152a60ca030586a1c74 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Sun, 26 Jul 2026 22:43:57 -0400 Subject: [PATCH 08/12] Pin the dropped-event and duplicate-gate bugs with tests; update docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five regression tests extending ADETests.swift (no new file — the chat-event dedupe seam already lives there). Each was verified to fail with its fix reverted, not just to pass with it: - reused transcript sequence across epochs keeps both events (fails without the timestamp in the envelope id) - a sub-24-char text chunk survives the same collision (this is the "king Round 1 now" truncation) - an identical redelivery still dedupes (proves widening identity didn't trade dropped events for duplicated ones) - a gate dedupes by itemId across different sequences - one Claude AskUserQuestion yields exactly one pending input (fails with "2 is not equal to 1" if isAskUserToolName matches the tool name, and shows the unanswerable tool_call card would have won) Also fixes a pre-existing flaky test: the lane-delete batch runner asserted array ordering between two deliberately concurrent deletes. It failed once in a full run and passed 3/3 in isolation. Start order is not part of that contract — only that both are in flight and concurrency is held at 2. Docs: the pending-input card's height budget, minimize affordance, and draft persistence; the event-identity invariant recorded as fragile wiring in the chat docs, as a gotcha in the iOS companion doc, and on the seq contract in ARCHITECTURE.md. Co-Authored-By: Claude --- apps/ios/ADETests/ADETests.swift | 203 +++++++++++++++++- docs/ARCHITECTURE.md | 2 +- docs/features/chat/composer-and-ui.md | 106 ++++++++- .../sync-and-multi-device/ios-companion.md | 78 ++++++- 4 files changed, 381 insertions(+), 8 deletions(-) diff --git a/apps/ios/ADETests/ADETests.swift b/apps/ios/ADETests/ADETests.swift index 4b022bc5e..4598ec805 100644 --- a/apps/ios/ADETests/ADETests.swift +++ b/apps/ios/ADETests/ADETests.swift @@ -4715,6 +4715,156 @@ final class ADETests: XCTestCase { XCTAssertEqual(service.chatEventHistory(sessionId: "session-1"), [original, tail]) } + /// A host's `eventSequence` restarts at 1 whenever a session is rehydrated, + /// but it keeps appending to the SAME transcript, so one transcript can hold + /// two events numbered 67 hours apart. Identity used to be `sessionId:sequence` + /// and dedupe is first-key-wins over file order, so the newer event was + /// discarded as a duplicate of the older one. On a real 425-event transcript + /// that destroyed 103 events — including the `approval_request` envelopes + /// carrying AskUserQuestion cards, which is why the phone showed no question. + @MainActor + func testReusedTranscriptSequenceKeepsBothEventsFromDifferentEpochs() async throws { + let service = SyncService(database: makeDatabase(baseURL: makeTemporaryDirectory())) + let firstEpoch = AgentChatEventEnvelope( + sessionId: "session-1", + timestamp: "2026-03-17T00:01:52.764Z", + event: .command( + command: "ls", + cwd: "/tmp", + output: "", + itemId: "cmd-1", + logicalItemId: nil, + turnId: "turn-1", + exitCode: 0, + durationMs: 3, + status: "completed" + ), + sequence: 67, + provenance: nil + ) + // Same sequence number, four hours later: the host restarted and its + // counter began again at 1. + let secondEpoch = AgentChatEventEnvelope( + sessionId: "session-1", + timestamp: "2026-03-17T04:16:22.165Z", + event: .approvalRequest( + itemId: "gate-1", + logicalItemId: nil, + kind: .toolCall, + description: "Which approach?", + turnId: "turn-2", + detail: nil + ), + sequence: 67, + provenance: nil + ) + + service.replaceChatEventHistory(sessionId: "session-1", events: [firstEpoch, secondEpoch]) + + let history = service.chatEventHistory(sessionId: "session-1") + XCTAssertEqual(history.count, 2, "A reused sequence number must not drop the newer event") + XCTAssertEqual(history, [firstEpoch, secondEpoch]) + XCTAssertNotEqual(firstEpoch.id, secondEpoch.id, "Envelope identity must not collide across sequence epochs") + } + + /// Short text has no content dedupe key (the text key requires >= 24 chars), + /// so it fell back to the sequence-derived id and was dropped by the same + /// collision. The user-visible symptom was a reply rendering as + /// "king Round 1 now" — the preceding 18-character chunk had vanished. + @MainActor + func testShortTextChunkSurvivesReusedTranscriptSequence() async throws { + let service = SyncService(database: makeDatabase(baseURL: makeTemporaryDirectory())) + let older = AgentChatEventEnvelope( + sessionId: "session-1", + timestamp: "2026-03-17T00:00:00.000Z", + event: .activity(activity: .thinking, detail: nil, turnId: "turn-1"), + sequence: 94, + provenance: nil + ) + let shortChunk = AgentChatEventEnvelope( + sessionId: "session-1", + timestamp: "2026-03-17T05:00:00.000Z", + event: .text(text: "No problem — re-as", messageId: "msg-9", turnId: "turn-2", itemId: "item-9"), + sequence: 94, + provenance: nil + ) + + service.replaceChatEventHistory(sessionId: "session-1", events: [older, shortChunk]) + + let history = service.chatEventHistory(sessionId: "session-1") + XCTAssertEqual(history.count, 2, "A sub-24-char text chunk must not be swallowed by a reused sequence") + XCTAssertTrue( + history.contains(where: { envelope in + if case .text(let text, _, _, _) = envelope.event { return text == "No problem — re-as" } + return false + }), + "The short text chunk must survive" + ) + } + + /// A genuine redelivery — identical timestamp AND sequence — must still + /// collapse, otherwise widening identity would trade dropped events for + /// duplicated ones. + @MainActor + func testIdenticalRedeliveryStillDedupes() async throws { + let service = SyncService(database: makeDatabase(baseURL: makeTemporaryDirectory())) + let event = AgentChatEventEnvelope( + sessionId: "session-1", + timestamp: "2026-03-17T00:00:00.000Z", + event: .approvalRequest( + itemId: "gate-1", + logicalItemId: nil, + kind: .toolCall, + description: "Which approach?", + turnId: "turn-1", + detail: nil + ), + sequence: 12, + provenance: nil + ) + + service.recordChatEventEnvelope(event) + service.mergeChatEventHistory(sessionId: "session-1", events: [event, event]) + + XCTAssertEqual(service.chatEventHistory(sessionId: "session-1"), [event]) + } + + /// Gates carry a session-unique `itemId`, so they now dedupe on that rather + /// than on the sequence-derived id. Re-delivering the same gate under a + /// different sequence must not produce a second card. + @MainActor + func testGateDedupesByItemIdAcrossDifferentSequences() async throws { + let service = SyncService(database: makeDatabase(baseURL: makeTemporaryDirectory())) + func gate(sequence: Int, timestamp: String) -> AgentChatEventEnvelope { + AgentChatEventEnvelope( + sessionId: "session-1", + timestamp: timestamp, + event: .approvalRequest( + itemId: "gate-shared", + logicalItemId: nil, + kind: .toolCall, + description: "Which approach?", + turnId: "turn-1", + detail: nil + ), + sequence: sequence, + provenance: nil + ) + } + + service.replaceChatEventHistory( + sessionId: "session-1", + events: [gate(sequence: 5, timestamp: "2026-03-17T00:00:00.000Z"), + gate(sequence: 9, timestamp: "2026-03-17T00:00:01.000Z")] + ) + + XCTAssertEqual( + service.chatEventHistory(sessionId: "session-1").count, + 1, + "One gate itemId must yield one pending-input event regardless of sequence" + ) + } + @MainActor func testDuplicateChatSubscribeSnapshotDoesNotAdvanceRevision() async throws { let service = SyncService(database: makeDatabase(baseURL: makeTemporaryDirectory())) @@ -10251,7 +10401,11 @@ final class ADETests: XCTestCase { await recorder.waitForStartedCount(2) let firstStartedIds = await recorder.startedIds() let firstMaxActiveCount = await recorder.maxActiveCount() - XCTAssertEqual(firstStartedIds, ["lane-a", "lane-b"]) + // Which of the two concurrent deletes wins the race to record itself first + // is not part of the contract — only that both are in flight and the runner + // holds the concurrency limit at 2. Asserting the array order made this test + // fail intermittently in CI. + XCTAssertEqual(Set(firstStartedIds), ["lane-a", "lane-b"]) XCTAssertEqual(firstMaxActiveCount, 2) await recorder.release() @@ -19815,6 +19969,53 @@ final class ADETests: XCTestCase { XCTAssertTrue(cards.isEmpty) } + /// The host emits BOTH a `tool_call` for Claude's `AskUserQuestion` tool-use + /// block AND a separate `approval_request` for the gate, under different item + /// ids (the SDK tool-use id vs a fresh randomUUID). `derivePendingWorkInputs` + /// dedupes by item id, so if `isAskUserToolName` matched the tool name the + /// user would get two cards for one question — and the tool_call-derived one + /// is unanswerable, because the host has no approval registered under that id + /// and discards the response silently. + func testClaudeAskUserQuestionYieldsExactlyOnePendingInput() { + let argsText = """ + {"questions":[{"id":"approach","question":"Which approach?","options":[{"label":"A","value":"a"}]}]} + """ + let detailText = """ + {"tool":"AskUserQuestion","source":"claude","request":{"kind":"structured_question","questions":[{"id":"approach","question":"Which approach?","options":[{"label":"A","value":"a"}]}]}} + """ + let transcript: [WorkChatEnvelope] = [ + WorkChatEnvelope( + sessionId: "chat-1", + timestamp: "2026-04-20T00:00:01.000Z", + sequence: 1, + event: .toolCall(tool: "AskUserQuestion", argsText: argsText, itemId: "toolu_abc", parentItemId: nil, turnId: "turn-1") + ), + WorkChatEnvelope( + sessionId: "chat-1", + timestamp: "2026-04-20T00:00:02.000Z", + sequence: 2, + event: .approvalRequest( + description: "Which approach?", + detail: detailText, + itemId: "11111111-2222-3333-4444-555555555555", + turnId: "turn-1" + ) + ), + ] + + let inputs = derivePendingWorkInputs(from: transcript) + XCTAssertEqual(inputs.count, 1, "One AskUserQuestion must not produce two pending-input cards") + guard case .question(let model) = inputs.first else { + return XCTFail("Expected the approval_request to surface as the pending question.") + } + XCTAssertEqual( + model.id, + "11111111-2222-3333-4444-555555555555", + "The card must come from the approval_request, whose itemId the host can actually resolve" + ) + XCTAssertEqual(model.questionId, "approach") + } + func testBuildWorkTimelineShowsNormalToolCallsOnMobile() { let transcript: [WorkChatEnvelope] = [ WorkChatEnvelope( diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8c7b4d8c7..dcf04914d 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -1147,7 +1147,7 @@ The sync subsystem is **owned by the ADE runtime** (`apps/ade-cli/src/services/s batch through the existing chunked envelope transport. - `hello_ok` can include the host's mobile project catalog and project-action feature flag. The iOS app shows a native project home until an active project is selected, can browse/open/create/clone projects on the paired machine when project actions are available, then drives `project_switch_request` / `project_switch_result`; the port stays stable across switches. - Bidirectional sync continues; inbound processing (envelope parse, gunzip, chunk reassembly, changeset decode + apply) runs off the main actor. On disconnect: a fast exponential-backoff burst, then an indefinite ~30 s slow-heartbeat retry — the phone never permanently gives up. `reconnectIfPossible` is guarded against overlapping runs. -- Chat streaming resumes by sequence: each `chat_event` carries a host-assigned per-session `seq` backed by a replay buffer; `chat_subscribe` passes `sinceSeq` so reconnects replay only the missed events. A per-session hydration barrier holds the live broadcaster and transcript pump until the snapshot ack, then resumes from the pre-capture logical byte offset so concurrent appends cannot overtake or fall between snapshot and stream. The subscribe ack also carries `turnActive` (live turn state from the agent chat service) so a phone subscribing mid-turn renders streaming/stop affordances immediately even when the byte-capped snapshot tail dropped the turn's start event. `chat.getTranscript` pages older history via an opaque cursor; full runtimes advertise append-stable `cursorKind: "byte"` offsets and the minimal headless fallback advertises `cursorKind: "index"`. When the host advertises the `crossProjectChat` feature flag, `chat_subscribe` can also name a foreign (non-active) project via `projectId`/`projectRootPath`; the host streams that project's transcript read-only straight off its `.ade` transcript files, so the all-projects Hub can open any project's chat without a project switch or runtime boot. Personal subscriptions instead send `chatScope: "personal"`; the host resolves the durable transcript and active-turn state through `PersonalChatScope` with no project id. +- Chat streaming resumes by sequence: each `chat_event` carries a host-assigned per-session `seq` backed by a replay buffer; `chat_subscribe` passes `sinceSeq` so reconnects replay only the missed events. `seq` is a resume cursor, not an event identity — it is unique only within one runtime lifetime, while the transcript it numbers is durable and keeps being appended across restarts, so a client that keys identity or dedupe on `sessionId + seq` alone will silently drop real events as phantom replays (see [features/chat/composer-and-ui.md](./features/chat/composer-and-ui.md#fragile-and-tricky-wiring)). Rehydrated sessions seed their counter from the transcript's maximum so numbering stays strictly increasing, and clients pair `seq` with the event timestamp (or, for blocking gates, the host-assigned `itemId`). A per-session hydration barrier holds the live broadcaster and transcript pump until the snapshot ack, then resumes from the pre-capture logical byte offset so concurrent appends cannot overtake or fall between snapshot and stream. The subscribe ack also carries `turnActive` (live turn state from the agent chat service) so a phone subscribing mid-turn renders streaming/stop affordances immediately even when the byte-capped snapshot tail dropped the turn's start event. `chat.getTranscript` pages older history via an opaque cursor; full runtimes advertise append-stable `cursorKind: "byte"` offsets and the minimal headless fallback advertises `cursorKind: "index"`. When the host advertises the `crossProjectChat` feature flag, `chat_subscribe` can also name a foreign (non-active) project via `projectId`/`projectRootPath`; the host streams that project's transcript read-only straight off its `.ade` transcript files, so the all-projects Hub can open any project's chat without a project switch or runtime boot. Personal subscriptions instead send `chatScope: "personal"`; the host resolves the durable transcript and active-turn state through `PersonalChatScope` with no project id. - User-message delivery is durable across that stream: accepted messages retain processed/unprocessed state, and unprocessed rows expose Run next / Edit / Dismiss through idempotent `chat.resolveUnprocessedMessage`. Turn stalls and diff --git a/docs/features/chat/composer-and-ui.md b/docs/features/chat/composer-and-ui.md index 216b9a21c..e4889de5d 100644 --- a/docs/features/chat/composer-and-ui.md +++ b/docs/features/chat/composer-and-ui.md @@ -46,6 +46,8 @@ subagents, computer use). The pane derives all visible state from the | `ChatPrPane.tsx` | Left floating PR pane for Work chat. Shows cached lane PR details immediately, then refreshes the linked PR row with the same targeted refresh path so pane toggles surface current merged/closed/check state without a broad PR sync. An unmapped lane PR (projection-derived, `pr.unmapped`) skips the refresh and checks/reviews enrichment — there is no DB row behind its synthetic `gh:` id. | | `ChatProposedPlanCard.tsx` | Composer-level plan approval card shown while input is locked. Renders the plan description or question text as rich markdown (`ChatMarkdown`) inside a scrollable container (capped at `min(34vh, 360px)`). Transcript plan events render through `AgentChatMessageList` / `CodexPlanCard`. | | `apps/ios/ADE/Views/Work/WorkPlanComposerViews.swift` | iOS composer-level plan approval strip. The live `plan_approval` gate renders as a compact full-width strip above the prompt box, opens a large markdown sheet for review, and sends Approve/Reject decisions through `chat.approve` with optional rejection feedback as `responseText`. It is one body of the consolidated pending-input strip (see [Cross-surface parity](#cross-surface-parity)) — the strip in `WorkChatSessionView+Timeline.swift` renders the current request (plan / approval / permission / question / model-selection), a "Request 1 of N" header, and an "Accept all" sweep when more than one gate is queued. | +| `apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift` | iOS prompt box, icon-only staged-steer strip, and `WorkStructuredQuestionCard` — the mobile question card. The card pins only a provider row (plus the question tab strip when paged) above its internal scroll region and the freeform field plus Send/Decline footer below it; the question text, request body, meta rows, and option list all scroll. `WorkPendingInputHeightBoundedCard` in the same file is the generic wrapper that caps the non-question gates. Both budget against `maxCardHeight` (see [Cross-surface parity](#cross-surface-parity)) and enable `.scrollDismissesKeyboard(.interactively)` so a long typed answer can never trap the user away from the footer. | +| `apps/ios/ADE/Views/Work/WorkDraftPersistence.swift` | iOS draft persistence. `WorkComposerDraftStore` keeps unsent composer text per chat (`chat:`) plus fixed keys for the Hub and New Chat composers; `WorkQuestionDraftStore` keeps in-progress question selections/freeform per request id. Both are versioned JSON dictionaries in App Group `UserDefaults` via `WorkDefaultsJSONMap`, LRU-evicted by `updatedAt` (60 composer entries, 30 question entries), with a 400 ms `workDraftAutosaveDebounce`. The `workPersistedDraft(_:key:)` view modifier packages the three legs a plain `String` binding needs: restore-if-empty on appear, debounced autosave, flush on disappear. | | `ChatModelSelectionPendingCard.tsx` | Full agent-briefing model picker for orchestration pending inputs. Shows description, touched files, run-after dependencies, provider/model controls, and submitting/cancel states without a recommended default model. | | `codex/CodexPlanCard.tsx` | Codex plan card rendered inline in the transcript for `plan` events. Shows plan state (Planning / Plan ready), step progress with status glyphs, and streaming plan text as rich markdown via `ChatMarkdown`. Completed plans with no discrete steps render the full markdown body inline; plans with steps offer a toggle to expand the raw markdown details (labelled "details" when complete, "live" while streaming). Handles missing `steps` arrays gracefully. | | `codex/CodexGoalCard.tsx`, `codex/CodexGoalBanner.tsx` | Codex goal surfaces. The card is the active desktop surface and routes edits, status changes, and clears through typed ADE APIs (`ade.agentChat.codex.*`) rather than prompt text. It shows objective, status, token count, and elapsed time, while hiding provider budgets because ADE keeps goals unlimited. The banner remains available for compact surfaces that need a horizontal goal strip. | @@ -872,6 +874,66 @@ surfaces an "Awaiting you" badge on the Lanes row and the Work grid tile (derived from exact pending-input counts, not idle CLI attention heuristics), and iOS fires a light haptic when a new blocking gate arrives. +**Height budget (iOS).** The strip is capped so a gate can never claim the +whole page. `workPendingInputMaxHeight(chatSurfaceHeight:)` (in +`WorkChatSessionView.swift`) returns +`max(160, min(available * 0.82, chatSurfaceHeight * 0.62))` where `available` +subtracts a fixed 110pt composer reserve. The input is `chatSurfaceHeight = +max(240, scrollViewportHeight + composerLayoutHeight)`, **not** the transcript +viewport: the transcript and the composer inset split the same surface, so +their sum is invariant to how the two divide it, while the viewport alone +shrinks as the card grows — feeding that back in was a runaway loop where the +card ate the screen. The composer reserve is a constant for the same reason; +`composerLayoutHeight` already includes the strip being sized, so measuring it +would be circular. Inline-in-transcript question cards use the separate +`workInlinePendingInputMaxHeight(transcriptViewportHeight:)` rule +(`max(240, viewport * 0.62)`) because the composer sits below that viewport +either way and there is nothing to reserve for. + +The card's own arithmetic subtracts its measured top/bottom chrome plus named +`cardPadding` / `cardStackSpacing` constants from that budget and floors the +scroll region at 64pt — roughly one option row. On a small phone with the +keyboard up the irreducible chrome can still exceed the budget; the overflow is +absorbed by the transcript, not the composer, because the composer inset is +`fixedSize(vertical:)` and the transcript scroll view is the flexible sibling. +That view ordering is the actual guarantee that Send/Decline stay on screen; +the number only keeps the common case from getting there. + +**Minimize (iOS).** The strip header carries a chevron that collapses the card +to a one-line pill showing the provider mark, a content-derived summary +(`workPendingInputCollapsedSummary` — the question header, plan title, or +`Permission: `, never a generic "1 request"), the queued count, and an +expand chevron. The gate stays open and the composer stays locked; only the +card is swapped out, so the user can scroll the conversation for the context +the question needs. State is a `collapsedPendingInputId`, with the boolean +derived from it — a minimize applies to the gate the user chose to defer, so it +must expire the moment a different gate becomes primary, and deriving makes +that impossible to get wrong. A keyboard `Done` toolbar item (gated on the +freeform field actually holding focus, because a toolbar declared +unconditionally would surface over the main composer's keyboard and silently do +nothing), a footer dismiss button, and interactive scroll-to-dismiss are the +three ways back out of the keyboard. + +**Draft persistence (iOS).** Mobile keeps unsent text the way desktop does. +`WorkComposerDraftStore` persists each chat's composer draft under +`chat:`, plus fixed keys for the Hub inline composer and the Work +New Chat composer; `WorkQuestionDraftStore` persists a still-open question's +selections, per-question freeform, shared freeform, and page index under the +request id, so backing out of the chat to check the transcript — the exact +reason a user minimizes the card — no longer discards what they picked. Both +autosave on a 400 ms debounce and flush on disappear, because a cancelled +`.task` throws out of its sleep before the write and a navigation pop is +precisely the case the debounce misses. Send clears the stored draft +**synchronously** rather than waiting out the debounce: a jetsam inside that +window would otherwise restore an already-sent message into the composer, where +it reads as unsent and invites sending it twice. Two deliberate exclusions: +answers to `isSecret` questions are never written (the backing store is App +Group `UserDefaults`, shared with the widget extension, so that would put a +credential on disk in plaintext), and unpair does not clear the stores (its only +production trigger fires automatically on an attributed auth failure, and the +stores are keyed by session, not by host, so clearing would destroy unsent text +for every other paired machine). + ### Per-runtime question richness (ceilings) Each runtime populates as much of the schema as its SDK exposes; the card @@ -1020,10 +1082,48 @@ These modules are pure and unit-testable: `data-composer-chip-text` and in the controlled draft; reconciliation must never replace sent text with a compact label. Metadata failures are expected and must degrade to the deterministic provider label or complete URL. +- **Chat event identity is never the sequence number alone.** + `eventSequence` is a runtime counter, but the transcript it numbers is + durable and appended across desktop restarts, so a rehydrated session + that restarts at 0 mints sequence numbers the file already contains — + one transcript can hold two events numbered 67, hours apart. Any + consumer keying identity on `sessionId + sequence` then mistakes the + newer event for a replay of the older one and drops it. Both halves of + the fix are load-bearing. Host side, + `readTranscriptHydrationState` (`agentChatService.ts`) seeds + `managed.eventSequence` from the transcript's max sequence in the same + pass that recovers todo items — one pass, because the transcript is + not cached — so sequences stay strictly increasing for the life of the + file. Client side, iOS's `AgentChatEventEnvelope.id` includes the + timestamp (`sessionId:timestamp:sequence`), so a genuine redelivery + (same timestamp *and* sequence) still collapses while cross-epoch + collisions do not. On a real 425-event transcript the old key destroyed + 103 events, including two `approval_request` envelopes carrying whole + AskUserQuestion cards and 31 short text chunks (short text has no + content dedupe key of its own — that requires >= 24 characters — so it + fell through to the sequence-derived id). Blocking gates additionally + get itemId-based content dedupe keys in + `SyncService.chatEventContentDedupeKey` (`approval_request`, + `structured_question`, `pending_input_resolved`): a dropped gate is a + question the user never sees and can never answer, so it must not + depend on sequence uniqueness at all. +- **`isAskUserToolName` deliberately does not match `AskUserQuestion`.** + For Claude's own ask-user tool the host emits *both* a `tool_call` (keyed + by the SDK tool-use id) and a separate `approval_request` (keyed by a + fresh `randomUUID`). iOS's `derivePendingWorkInputs` dedupes by item id, + so adding `askuserquestion` to that name list produces two cards for one + question — and the `tool_call`-derived one is unanswerable, because the + host has no approval registered under that id and discards the response + silently. The `tool_call` branch exists only as a fallback for hosts that + emit a bare ask-user call with no wrapping approval. - **Question drafts persistence.** Question answer state (selected - options + freeform drafts) is local to `InlineQuestionRequestCard`. If - the user navigates away and back, drafts reset. This is intentional to - avoid stale answers leaking across sessions. The card's one-time focus + options + freeform drafts) is local to `InlineQuestionRequestCard` on + desktop. If the user navigates away and back, drafts reset. This is + intentional to avoid stale answers leaking across sessions. iOS makes + the opposite call for the same surface — see + [Cross-surface parity](#cross-surface-parity) — because minimizing the + card to read the transcript is a normal step in answering it there, not + a session change. The card's one-time focus and entrance animation are guarded by module-level sets (`focusedQuestionCardKeys` / `enteredQuestionCardKeys`) so the virtualized list re-mounting the row mid-scroll doesn't re-steal focus diff --git a/docs/features/sync-and-multi-device/ios-companion.md b/docs/features/sync-and-multi-device/ios-companion.md index f2e158ade..fce745417 100644 --- a/docs/features/sync-and-multi-device/ios-companion.md +++ b/docs/features/sync-and-multi-device/ios-companion.md @@ -315,8 +315,18 @@ apps/ios/ │ │ │ # WorkChatAttachmentTray, │ │ │ # WorkChatComposerAndInputViews (compacted │ │ │ # icon-only staged-steer strip + the -│ │ │ # structured-question card: chip tab strip -│ │ │ # over a viewport-capped internal scroll), +│ │ │ # structured-question card: pinned provider +│ │ │ # row / tab strip above and freeform + +│ │ │ # Send/Decline footer below a +│ │ │ # budget-capped internal scroll, plus +│ │ │ # WorkPendingInputHeightBoundedCard for the +│ │ │ # non-question gates), +│ │ │ # WorkDraftPersistence (WorkComposerDraftStore +│ │ │ # per-chat/Hub/New-Chat composer text + +│ │ │ # WorkQuestionDraftStore per-request +│ │ │ # selections, over App Group UserDefaults; +│ │ │ # debounced autosave + workPersistedDraft +│ │ │ # view modifier), │ │ │ # WorkArtifactTerminalViews (in-thread │ │ │ # artifact card with a friendly │ │ │ # "Preview isn't available" fallback when @@ -1844,6 +1854,29 @@ different machine's cached limits. `chat_event` sends as delivered only when the socket accepts them; on backpressure it leaves the transcript offset unchanged and retries in order. Events without `seq` (older hosts) bypass the watermark entirely. +- **`seq` is a resume cursor, not an event identity.** The counter is + per-runtime, but the transcript it numbers is durable and keeps being + appended across desktop restarts, so the same transcript can contain two + events numbered 67 hours apart. The phone's dedupe is first-key-wins over + file order, so keying `AgentChatEventEnvelope.id` on `sessionId:sequence` + made the newer event look like a replay of the older one and silently + discarded it — on a real 425-event transcript that destroyed 103 events, + including the `approval_request` envelopes carrying AskUserQuestion cards + (the phone showed no question at all) and 31 short text chunks (short text + has no content dedupe key, which needs >= 24 characters, so it fell through + to the sequence-derived id and a reply rendered mid-word). The envelope id + now includes the timestamp, so a genuine redelivery — identical timestamp + *and* sequence — still collapses while cross-epoch collisions are broken + apart. Blocking gates go further and dedupe on their host-assigned + session-unique `itemId` in `SyncService.chatEventContentDedupeKey` + (`approval_request`, `structured_question`, `pending_input_resolved`), + because a dropped gate is not a cosmetic loss — it is a card the user never + sees and can never answer. Host-side, `readTranscriptHydrationState` + (`agentChatService.ts`) now seeds a rehydrated session's `eventSequence` + from the transcript's maximum instead of restarting at 0, so sequences stay + strictly increasing for the life of the file. Any new phone-side identity or + cache key must follow the same rule: sequence numbers are unique within a + runtime lifetime only. - **Transcript history pages through an opaque cursor.** `chat.getTranscript` responses carry `nextCursor`; the phone's `fetchChatTranscriptPage` requests strictly-older history with it. @@ -2053,7 +2086,46 @@ different machine's cached limits. only for approval/permission gates — never question, plan-approval, or model-selection — and flips `acceptForSession` on the current gate then accepts each remaining sweepable gate sequentially (stale itemIds no-op - on the host, so re-sends after auto-resolution are safe). + on the host, so re-sends after auto-resolution are safe). The strip can + also be minimized to a one-line pill (`collapsedPendingInputId`) that names + what is being asked; the gate stays open and the composer stays locked, and + the collapse expires on its own as soon as a different gate becomes primary + because the boolean is derived from the stored id rather than synchronized + alongside it. +- **Size a pending-input card from the chat surface, never from the + transcript viewport.** The transcript and the composer inset split the same + surface, so the viewport shrinks exactly as the card grows — budgeting off + it is self-referential and the card walks itself up to full screen. The + budget input is `chatSurfaceHeight = max(240, scrollViewportHeight + + composerLayoutHeight)`, whose sum does not move when the card resizes, and + `workPendingInputMaxHeight(chatSurfaceHeight:)` derives the cap from it. + The composer reserve inside that helper is a constant for the same reason: + `composerLayoutHeight` already includes the strip being sized. The card's + own chrome (provider row, tab strip, freeform field, Send/Decline footer) + is measured and subtracted so the scroll region absorbs overflow; when the + irreducible chrome still exceeds the budget on a small phone with the + keyboard up, the overflow lands on the transcript rather than the footer, + because the composer inset is `fixedSize(vertical:)` and the transcript is + the flexible sibling. That view ordering — not the number — is what + guarantees Send stays reachable. +- **Mobile keeps unsent text; it is a store, not view state.** + `WorkDraftPersistence.swift` holds `WorkComposerDraftStore` (composer text + per chat plus fixed Hub / New Chat keys) and `WorkQuestionDraftStore` + (in-progress question selections, freeform, and page per request id), both + versioned JSON dictionaries in App Group `UserDefaults`, LRU-capped, saved + on a 400 ms debounce and flushed on disappear because a cancelled `.task` + throws out of its sleep before the write. Three rules are load-bearing: + restore only into an empty field (a failed send that put its text back, or + a card already mid-edit, is fresher than disk); clear synchronously on send + rather than letting the debounce get there, or a jetsam inside that window + resurrects an already-sent message and invites a duplicate; and never write + an `isSecret` answer, because that defaults suite is shared with the widget + extension and would hold a credential in plaintext. Clearing a host profile + deliberately does **not** touch these stores — `forgetHost()` has no UI + caller and fires automatically from `handleReconnectFailure` on an + attributed auth failure, and the stores are keyed by session id rather than + by host, so wiping them would destroy unsent text for every other paired + machine plus the machine-independent Hub and New Chat drafts. - **Optimistic steers reconcile on the active-to-idle turn boundary.** A message the phone sends mid-turn is echoed as an optimistic "Sends after turn" row (`WorkQueuedSteerRow`) using the host-assigned steer id From ccb70ef098c8d29be22c9357b8fde0604170aac3 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 27 Jul 2026 04:18:59 -0400 Subject: [PATCH 09/12] Correct the maxCardHeight doc for the inline exception MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment warned that this budget never comes from the transcript viewport, but the same PR added workInlinePendingInputMaxHeight, which deliberately does exactly that for the inline-in-transcript variant — that card sits inside the transcript, so there is no feedback path to guard against. Says so now, and keeps the warning where it actually applies (the composer-anchored strip). Co-Authored-By: Claude --- .../Work/WorkChatComposerAndInputViews.swift | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift index f99ab6dc7..29be9fea7 100644 --- a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift +++ b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift @@ -982,12 +982,17 @@ struct WorkStructuredQuestionCard: View { /// Provider to fall back on when the parsed question carries no `source` /// (legacy `structured_question` envelopes). Usually the session provider. var fallbackProvider: String? = nil - /// Hard ceiling for the card's total laid-out height, computed by the chat - /// surface from the space actually available (keyboard included). The card - /// never exceeds it — the option list scrolls internally instead. Derived - /// from the chat surface, NOT from the transcript viewport: the transcript - /// shrinks as this card grows, so feeding its height back in created a - /// runaway loop where the card ate the whole screen. 0 until measured. + /// Hard ceiling for the card's total laid-out height, computed from the space + /// actually available (keyboard included). The card never exceeds it — the + /// option list scrolls internally instead. 0 until measured. + /// + /// For the composer-anchored strip — the live path — this comes from + /// `workPendingInputMaxHeight`, which budgets the whole chat surface and NOT + /// the transcript viewport: the transcript shrinks as this card grows, so + /// feeding its height back in created a runaway loop where the card ate the + /// screen. The inline-in-transcript variant is the deliberate exception; it + /// sits *inside* the transcript, so `workInlinePendingInputMaxHeight` budgets + /// it from the viewport with no such feedback path. var maxCardHeight: CGFloat = 0 /// Resolved asking provider: the parsed question source, else the session From 52f3ea6b247b87eb8631780ad1998204d7ab19ac Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 27 Jul 2026 04:30:14 -0400 Subject: [PATCH 10/12] Keep answers on a rejected submit; never persist secret selections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two P1 review findings, both real defects in this PR's own code. Codex: the isSecret exclusion filtered freeform answers but not selections. When a secret question carries options, the chosen option IS the secret answer, so it was being written to the App Group defaults in plaintext — the exact thing the SecureField exists to prevent. Selections are now filtered by the same secret id set. Greptile: submitAll cleared the drafts unconditionally, before the caller had a chance to roll back. On a rejected or failed send, dispatchPendingInputAnswer un-hides the card — and it came back empty, forcing the user to retype every answer. That defeats the whole point of this change. The three answer paths (submit, decline, tap-to-submit) now return whether the host accepted, and clear only on acceptance; dispatchPendingInputAnswer already reported this and the card was simply discarding it. The inline-in-transcript path reports the same way through runSessionAction. Adds a regression test that non-secret answers round-trip while an all-secret snapshot removes its entry outright rather than storing an empty husk. Co-Authored-By: Claude --- .../Work/WorkChatComposerAndInputViews.swift | 36 +++++++++++++------ .../Work/WorkChatSessionView+Timeline.swift | 9 +++-- apps/ios/ADE/Views/Work/WorkPreviews.swift | 18 +++++----- apps/ios/ADETests/ADETests.swift | 32 +++++++++++++++++ 4 files changed, 72 insertions(+), 23 deletions(-) diff --git a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift index 29be9fea7..79b26659d 100644 --- a/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift +++ b/apps/ios/ADE/Views/Work/WorkChatComposerAndInputViews.swift @@ -972,12 +972,17 @@ struct WorkStructuredQuestionCard: View { /// Tap-to-submit is only used for single-question single-select with options /// (the card invokes this directly from `optionRow`). Multi-question cards /// never call this — taps only update local state and submit via Send. - let onSelectOption: @MainActor (WorkPendingQuestionOption, String?) async -> Void + /// + /// Returns whether the answer was actually accepted. The card only discards + /// the user's work when it was: a rejected send rolls the optimistic hide + /// back and the card returns, and it must return with the answers still in it. + let onSelectOption: @MainActor (WorkPendingQuestionOption, String?) async -> Bool /// Aggregate submit: one map from questionId -> answer value, plus the /// shared freeform response (single-question only). The session action - /// forwards this as one `chat.respondToInput` call. - let onSubmitAll: @MainActor ([String: AgentChatInputAnswerValue], String?) async -> Void - let onDecline: @MainActor () async -> Void + /// forwards this as one `chat.respondToInput` call. Returns acceptance — see + /// `onSelectOption`. + let onSubmitAll: @MainActor ([String: AgentChatInputAnswerValue], String?) async -> Bool + let onDecline: @MainActor () async -> Bool var onFreeformFocusChange: ((Bool) -> Void)? = nil /// Provider to fall back on when the parsed question carries no `source` /// (legacy `structured_question` envelopes). Usually the session provider. @@ -1181,7 +1186,11 @@ struct WorkStructuredQuestionCard: View { let secretIds = secretQuestionIds WorkQuestionDraftStore.save( WorkQuestionDraftStore.Snapshot( - selections: selections, + // Selections are excluded for a secret question too, not just freeform: + // when such a question carries options, the chosen option value IS the + // secret answer, and persisting it would leak exactly what the + // SecureField exists to protect. + selections: selections.filter { !secretIds.contains($0.key) }, freeform: freeformByQuestion.filter { !secretIds.contains($0.key) }, // The shared freeform belongs to the single-question card's only // question, so it inherits that question's secrecy. @@ -1525,14 +1534,18 @@ struct WorkStructuredQuestionCard: View { let shared = singleQuestionFreeformText.trimmingCharacters(in: .whitespacesAndNewlines) return shared.isEmpty ? nil : shared }() - await onSubmitAll(answers, sharedFreeform) - clearQuestionDrafts() + // Only discard the answers once the host has accepted them. A failed send + // restores the card; it must come back with the user's work intact. + if await onSubmitAll(answers, sharedFreeform) { + clearQuestionDrafts() + } } @MainActor private func declineQuestion() async { - await onDecline() - clearQuestionDrafts() + if await onDecline() { + clearQuestionDrafts() + } } @MainActor @@ -1713,8 +1726,9 @@ struct WorkStructuredQuestionCard: View { if singleQuestionSingleSelect { let freeform = singleQuestionFreeformText.trimmingCharacters(in: .whitespacesAndNewlines) Task { @MainActor in - await onSelectOption(option, freeform.isEmpty ? nil : freeform) - clearQuestionDrafts() + if await onSelectOption(option, freeform.isEmpty ? nil : freeform) { + clearQuestionDrafts() + } } } } diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift index fd58e589e..cbfad768e 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView+Timeline.swift @@ -131,23 +131,26 @@ extension WorkChatSessionView { question: question, busy: actionInFlight || !isLive, onSelectOption: { option, freeform in - await runSessionAction { + await runSessionAction { () async -> Bool in await onRespondToQuestion( question.id, question.questionId, .string(option.value), freeform ) + return errorMessage == nil } }, onSubmitAll: { answers, freeform in - await runSessionAction { + await runSessionAction { () async -> Bool in await onSubmitQuestionAnswers(question.id, answers, freeform) + return errorMessage == nil } }, onDecline: { - await runSessionAction { + await runSessionAction { () async -> Bool in await onDeclineQuestion(question.id) + return errorMessage == nil } }, onFreeformFocusChange: { focused in diff --git a/apps/ios/ADE/Views/Work/WorkPreviews.swift b/apps/ios/ADE/Views/Work/WorkPreviews.swift index 1b5dff8ce..fe3a41153 100644 --- a/apps/ios/ADE/Views/Work/WorkPreviews.swift +++ b/apps/ios/ADE/Views/Work/WorkPreviews.swift @@ -564,9 +564,9 @@ private func workPreviewOversizedQuestion() -> WorkPendingQuestionModel { WorkStructuredQuestionCard( question: workPreviewOversizedQuestion(), busy: false, - onSelectOption: { _, _ in }, - onSubmitAll: { _, _ in }, - onDecline: {}, + onSelectOption: { _, _ in true }, + onSubmitAll: { _, _ in true }, + onDecline: { true }, fallbackProvider: "claude", maxCardHeight: workPendingInputMaxHeight(chatSurfaceHeight: 720) ) @@ -585,9 +585,9 @@ private func workPreviewOversizedQuestion() -> WorkPendingQuestionModel { WorkStructuredQuestionCard( question: workPreviewOversizedQuestion(), busy: false, - onSelectOption: { _, _ in }, - onSubmitAll: { _, _ in }, - onDecline: {}, + onSelectOption: { _, _ in true }, + onSubmitAll: { _, _ in true }, + onDecline: { true }, fallbackProvider: "claude", maxCardHeight: workPendingInputMaxHeight(chatSurfaceHeight: 340) ) @@ -620,9 +620,9 @@ private func workPreviewOversizedQuestion() -> WorkPendingQuestionModel { source: "claude" ), busy: false, - onSelectOption: { _, _ in }, - onSubmitAll: { _, _ in }, - onDecline: {}, + onSelectOption: { _, _ in true }, + onSubmitAll: { _, _ in true }, + onDecline: { true }, fallbackProvider: "claude", maxCardHeight: workPendingInputMaxHeight(chatSurfaceHeight: 720) ) diff --git a/apps/ios/ADETests/ADETests.swift b/apps/ios/ADETests/ADETests.swift index 4598ec805..a22565d1d 100644 --- a/apps/ios/ADETests/ADETests.swift +++ b/apps/ios/ADETests/ADETests.swift @@ -19969,6 +19969,38 @@ final class ADETests: XCTestCase { XCTAssertTrue(cards.isEmpty) } + /// A question marked `isSecret` renders its freeform in a `SecureField`, and + /// the resolved card refuses to echo the answer back. When such a question + /// also carries options, the CHOSEN OPTION is the secret answer — persisting + /// it to the App Group defaults (readable by the widget extension) leaks + /// exactly what the SecureField exists to protect. + func testSecretQuestionAnswersAreNeverPersisted() { + let requestId = "secret-req-\(UUID().uuidString)" + defer { WorkQuestionDraftStore.clear(requestId) } + + WorkQuestionDraftStore.save( + WorkQuestionDraftStore.Snapshot( + selections: ["public-q": ["keep-me"]], + freeform: ["public-q": "visible answer"], + sharedFreeform: "", + page: 0 + ), + for: requestId + ) + + let stored = WorkQuestionDraftStore.load(requestId) + XCTAssertEqual(stored?.selections["public-q"], ["keep-me"], "Non-secret answers must round-trip") + XCTAssertEqual(stored?.freeform["public-q"], "visible answer") + + // An all-empty snapshot removes the entry outright, so a card whose only + // answers were secret leaves nothing behind at all. + WorkQuestionDraftStore.save(WorkQuestionDraftStore.Snapshot(), for: requestId) + XCTAssertNil( + WorkQuestionDraftStore.load(requestId), + "A snapshot with every secret answer filtered out must remove the entry, not store an empty husk" + ) + } + /// The host emits BOTH a `tool_call` for Claude's `AskUserQuestion` tool-use /// block AND a separate `approval_request` for the gate, under different item /// ids (the SDK tool-use id vs a fresh randomUUID). `derivePendingWorkInputs` From 28d97a7195b85366303b17bc19b5c2085eeafad2 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 27 Jul 2026 04:44:45 -0400 Subject: [PATCH 11/12] Bound question draft payloads; fix a second wall-clock-flaky test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2: WorkComposerDraftStore clamps each draft to 20,000 characters and WorkQuestionDraftStore had no equivalent cap, so a pasted wall of text went into the App Group defaults whole — and because autosave decodes, re-encodes, and rewrites the entire map on the main actor, every later keystroke would pay for it. Clamps freeform, shared freeform, and option values to the same limit, with a test. Also fixes testWorkFilteredSessionsRetainsStaleStandaloneCliRowsAndChatOwnedShells, which passed and failed on alternating isolated runs of identical code. The fixtures take startedAt from wall-clock at construction, so whether the three sessions share a timestamp — and therefore whether the sort reaches its title tiebreak — depends on which second they were built in. The test's contract is retention, which it now asserts as a set, plus the one ordering guarantee that is real (the parent chat leads its owned rows). Full suite run twice, green both times. Co-Authored-By: Claude --- .../ADE/Views/Work/WorkDraftPersistence.swift | 22 +++++++++++++++++-- apps/ios/ADETests/ADETests.swift | 19 +++++++++++++++- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift b/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift index 28511079f..be9f90e3b 100644 --- a/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift +++ b/apps/ios/ADE/Views/Work/WorkDraftPersistence.swift @@ -173,18 +173,36 @@ enum WorkQuestionDraftStore { /// Open question gates are short-lived; a small cap is plenty and keeps the /// blob from accumulating answers to requests that were resolved elsewhere. private static let maxEntries = 30 + /// Matches `WorkComposerDraftStore.maxLength`. An answer is a reply, not a + /// document — and because autosave decodes, re-encodes, and rewrites the whole + /// map on the main actor, one pasted wall of text would otherwise turn every + /// subsequent keystroke into a visible stall. + private static let maxValueLength = 20_000 static func load(_ requestId: String) -> Snapshot? { guard !requestId.isEmpty else { return nil } return loadAll()[requestId]?.snapshot } - static func save(_ snapshot: Snapshot, for requestId: String) { + /// Clamps every free-text field (and host-supplied option value) so a single + /// paste cannot inflate the shared defaults store. + private static func bounded(_ snapshot: Snapshot) -> Snapshot { + var bounded = snapshot + bounded.freeform = snapshot.freeform.mapValues { String($0.prefix(maxValueLength)) } + bounded.sharedFreeform = String(snapshot.sharedFreeform.prefix(maxValueLength)) + bounded.selections = snapshot.selections.mapValues { values in + Set(values.map { String($0.prefix(maxValueLength)) }) + } + return bounded + } + + static func save(_ rawSnapshot: Snapshot, for requestId: String) { guard !requestId.isEmpty else { return } - guard !snapshot.isEmpty else { + guard !rawSnapshot.isEmpty else { clear(requestId) return } + let snapshot = bounded(rawSnapshot) var map = loadAll() // Autosave runs on a keystroke debounce; skip the write when the answer is // unchanged so idle typing pauses cost nothing. diff --git a/apps/ios/ADETests/ADETests.swift b/apps/ios/ADETests/ADETests.swift index a22565d1d..198ec01c3 100644 --- a/apps/ios/ADETests/ADETests.swift +++ b/apps/ios/ADETests/ADETests.swift @@ -17073,7 +17073,13 @@ final class ADETests: XCTestCase { searchText: "" ) - XCTAssertEqual(filtered.map(\.id), ["chat-parent", "shell-child", "legacy-cli"]) + // Retention is the contract this test names, not order. The fixtures take + // their `startedAt` from wall-clock at construction, so whether the three + // share a timestamp — and therefore whether the sort falls through to the + // title tiebreak — depends on which second they were built in. Asserting the + // sorted array made this fail intermittently. + XCTAssertEqual(Set(filtered.map(\.id)), ["chat-parent", "shell-child", "legacy-cli"]) + XCTAssertEqual(filtered.first?.id, "chat-parent", "The parent chat always leads its owned rows") } func testWorkFilteredSessionsPrioritizesWaitingBeforeActiveAndEnded() { @@ -19992,6 +19998,17 @@ final class ADETests: XCTestCase { XCTAssertEqual(stored?.selections["public-q"], ["keep-me"], "Non-secret answers must round-trip") XCTAssertEqual(stored?.freeform["public-q"], "visible answer") + // A pasted wall of text is clamped, so one paste can't inflate the shared + // defaults store or stall every later keystroke's autosave rewrite. + let huge = String(repeating: "x", count: 60_000) + WorkQuestionDraftStore.save( + WorkQuestionDraftStore.Snapshot(freeform: ["public-q": huge], sharedFreeform: huge), + for: requestId + ) + let clamped = WorkQuestionDraftStore.load(requestId) + XCTAssertEqual(clamped?.freeform["public-q"]?.count, 20_000, "Freeform answers must be clamped") + XCTAssertEqual(clamped?.sharedFreeform.count, 20_000, "Shared freeform must be clamped") + // An all-empty snapshot removes the entry outright, so a card whose only // answers were secret leaves nothing behind at all. WorkQuestionDraftStore.save(WorkQuestionDraftStore.Snapshot(), for: requestId) From 16db4e98450c857c3cbc0793aaed228782a8dde1 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 27 Jul 2026 04:59:08 -0400 Subject: [PATCH 12/12] Don't carry one chat's draft into another on session switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P1, and the most serious defect this PR has had. The chat composer view is reused across session switches. bind() flushed the outgoing text under the old key correctly, but then hit "whatever is already in the field wins" and returned early — so chat A's half-written message stayed visible in chat B, the next keystroke autosaved it over chat B's own stored draft, and it sat one tap away from being sent into the wrong conversation. The "field wins" rule exists for a real case (a failed send restores its text and that is fresher than disk), but it only applies to a freshly mounted composer. On a switch, the visible text belongs to the chat being left and has already been safely flushed; it must be replaced by the destination's own draft. Test reverts to red without the fix ("half-written message for chat A" is not equal to "") and also pins that switching away still flushes rather than loses, and that switching back restores. Co-Authored-By: Claude --- .../ADE/Views/Work/WorkChatSessionView.swift | 27 ++++++++++++---- apps/ios/ADETests/ADETests.swift | 32 +++++++++++++++++++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift index a5cf8cd43..abda65526 100644 --- a/apps/ios/ADE/Views/Work/WorkChatSessionView.swift +++ b/apps/ios/ADE/Views/Work/WorkChatSessionView.swift @@ -2290,14 +2290,29 @@ final class WorkChatComposerDraftState: ObservableObject { @MainActor func bind(persistenceKey key: String) { guard persistenceKey != key else { return } + // A blank previous key means this is the first bind of a freshly mounted + // composer; anything else is the view being reused for a different chat. + let isFirstBind = persistenceKey.isEmpty flushDraft() persistenceKey = key - guard !key.isEmpty else { return } - let stored = WorkComposerDraftStore.load(key) - // Whatever is already in the field wins: a failed send restores its text - // here, and that is fresher than anything on disk. - guard trimmedText.isEmpty, !stored.isEmpty else { return } - text = stored + let stored = key.isEmpty ? "" : WorkComposerDraftStore.load(key) + + guard !isFirstBind else { + // First mount: whatever is already in the field wins. A failed send + // restores its text here, and that is fresher than anything on disk. + guard trimmedText.isEmpty, !stored.isEmpty else { return } + text = stored + return + } + + // Session switch: the visible text belongs to the chat we just left, and it + // has already been flushed under that chat's key. It must NOT survive into + // this one — leaving it would show one conversation's draft in another, + // autosave it over the destination's own stored draft on the next + // keystroke, and put the wrong message one tap from being sent. + if text != stored { + text = stored + } } /// Write the draft now, cancelling any pending debounce. Called when the chat diff --git a/apps/ios/ADETests/ADETests.swift b/apps/ios/ADETests/ADETests.swift index 198ec01c3..42618532e 100644 --- a/apps/ios/ADETests/ADETests.swift +++ b/apps/ios/ADETests/ADETests.swift @@ -19975,6 +19975,38 @@ final class ADETests: XCTestCase { XCTAssertTrue(cards.isEmpty) } + /// The chat composer view is reused across session switches. Before this was + /// guarded, switching chats with text still in the box left that text visible + /// in the destination chat and autosaved it over the destination's own stored + /// draft on the next keystroke — one tap from sending the wrong message into + /// the wrong conversation. + @MainActor + func testComposerDraftDoesNotLeakAcrossSessionSwitch() { + let keyA = WorkComposerDraftStore.chatKey(sessionId: "sess-A-\(UUID().uuidString)") + let keyB = WorkComposerDraftStore.chatKey(sessionId: "sess-B-\(UUID().uuidString)") + defer { + WorkComposerDraftStore.clear(keyA) + WorkComposerDraftStore.clear(keyB) + } + + let state = WorkChatComposerDraftState() + state.bind(persistenceKey: keyA) + state.text = "half-written message for chat A" + + // Switch to a chat that has no draft of its own. + state.bind(persistenceKey: keyB) + XCTAssertEqual(state.text, "", "Chat A's text must not survive into chat B") + XCTAssertEqual( + WorkComposerDraftStore.load(keyA), + "half-written message for chat A", + "Switching away must flush the outgoing draft under its own key, not lose it" + ) + + // And switching back restores A's draft rather than B's empty box. + state.bind(persistenceKey: keyA) + XCTAssertEqual(state.text, "half-written message for chat A") + } + /// A question marked `isSecret` renders its freeform in a `SecureField`, and /// the resolved card refuses to echo the answer back. When such a question /// also carries options, the CHOSEN OPTION is the secret answer — persisting