Skip to content

finding(app-shell): sanitizeChatMessagesForCache rebuilds tool parts without approval / pendingActionId, so a cache-fallback reload loses the approval affordance the server path keeps #9232

Description

@os-tesla

Found while implementing objectui#8442 (PR #9229). ⛔ Not fixed there — the dispatch split that card to the additive half only, and this is a different function in a different file. ⛔ Not claimed.

What

packages/app-shell/src/hooks/useChatConversation.ts, sanitizeChatMessagesForCache, measured on origin/main @ b775500af.

objectui#8442 stops hydratedMessagesToChatMessages dropping the AI SDK approval envelope and pendingActionId on the way OUT of persisted history. sanitizeChatMessagesForCache is the other direction — the localStorage cache WRITE — and it has the same omission going in.

It rebuilds each tool part field by field:

type, toolCallId, toolName, state, errorText?, output?

state is kept (tool.state ?? (tool.errorText ? 'output-error' : 'output-available')), so a cached approval-requested survives. Neither approval nor pendingActionId is written at all, and output is re-serialized only for replayOutcome / draftReview / proposedPlan — a pending-approval envelope is not one of the three, so the id is not recoverable from the cached output either.

⇒ After objectui#8442 lands, the two hydration sources disagree: the SERVER path yields an invocation useHitlInChat can index, and the CACHE-FALLBACK path yields one carrying approval-requested and nothing to decide with — which is precisely the shape objectui#8442 exists to end.

Why it matters

The cache fallback is not a corner: readMessageCache is what renders the thread when the server returns no messages. The function's own comment already records this exact hazard for a different key — it re-serializes the draft envelope because otherwise a cache-fallback reload "drops the draft Review N changes / Publish card". The approval affordance is the same class of loss and was not covered.

Confidence

Read from the source, not exercised in a browser. What is measured: the rebuilt part list contains neither key, and the cachedOutput ternary has no pending-approval arm. What is NOT measured: how often a real operator hits the cache-fallback path with an outstanding approval.

Shape of a fix

Symmetrical with the ADR-0033 draft handling that is already there — a pendingApprovalToCachedResult inverse that re-mints the minimal { status: 'pending_approval', pendingActionId } envelope the detector re-parses, plus carrying approval onto the part. ⚠️ Not purely mechanical (it invents a serializer and owes its own round-trip test), which is why it was filed rather than folded into PR #9229.

Dedup

REST /search/* is refused for this session by the egress proxy, so one targeted search_issues call was used instead and is declared here. Query covered the cache sanitizer and the dropped approval id; 6 hits, all read. Only objectui#8442 is in this area (it is the control that proves the query reaches this surface); the other five are Approvals-inbox cards (objectui#5553, objectui#2829, objectui#2762, objectui#2698) and objectui#4437, none of which names this function or either key.

Related

objectui#8442 · objectui#8426 · PR #9229


Generated by Claude Code

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions