Skip to content

fix(chat): preserve approval replies across early turn completion - #975

Open
Quine-rq wants to merge 1 commit into
fathah:mainfrom
Quine-rq:codex/fix-approval-completion-ack
Open

Quine-rq wants to merge 1 commit into
fathah:mainfrom
Quine-rq:codex/fix-approval-completion-ack

Conversation

@Quine-rq

Copy link
Copy Markdown
Contributor

A dashboard can emit message.complete before acknowledging an in-flight approval.respond. Desktop currently expires every approval on completion, so even a confirmed Allow or Deny response is returned as a failure and the card becomes unavailable.

Keep only already-submitted decisions pending until their response settles; unanswered queued requests still expire immediately. Failed acknowledgements after completion expire the retained request. Abort, disconnect, connection changes, and a new prompt invalidate it, so a late reply cannot interrupt a subsequent turn using the same runtime session.

Regression tests reproduce both Allow and Deny failures before the fix, and cover response rejection, unresolved acknowledgements, cancellation, disconnects, connection changes, and a new turn. Existing choice checks, request correlation, duplicate guards, and fail-closed behavior remain in place.

Validation:

  • 60 focused transport/approval tests passed.
  • Full suite: 2,316 tests across 221 files passed. Type checks, full uncached ESLint, lat check, production build and diff checks passed.
  • Tests exercise the real React transport hook with controlled gateway boundaries; packaged desktop/live-agent validation remains pending.

@greptile-apps

greptile-apps Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the retained approval is narrowly scoped to an already-submitted response and is invalidated before it can cross relevant lifecycle or turn boundaries.

Summary

This PR fixes an ordering race where message.complete can arrive before an in-flight approval.respond acknowledgement.

  • Retains only an already-submitted approval while its acknowledgement is pending.
  • Immediately expires unanswered approvals at turn completion.
  • Invalidates retained approvals on rejection, unresolved acknowledgement, cancellation, disconnect, connection change, unmount, or a new prompt.
  • Adds focused Allow and Deny regression coverage plus lifecycle and failure-path tests.
  • Documents the approval-completion lifecycle in the LAT architecture notes.
Diagram
sequenceDiagram
    participant U as User
    participant D as Desktop transport
    participant G as Dashboard gateway
    U->>D: Allow or Deny
    D->>G: approval.respond
    G-->>D: message.complete
    D->>D: Retain submitted approval only
    D->>D: Expire unanswered approvals
    alt Acknowledgement resolves exactly one request
        G-->>D: "resolved = 1"
        D-->>U: Confirm decision
    else Rejected or unresolved acknowledgement
        G-->>D: "Error or resolved != 1"
        D->>D: Expire retained approval
        D-->>U: Mark card unavailable
    else New prompt or lifecycle invalidation
        U->>D: New prompt / abort / connection change
        D->>D: Expire retained approval
        D->>G: Continue or interrupt safely
    end
Loading

Reviews (1) · Last reviewed commit: "fix(chat): preserve approval replies acr..."

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant