Skip to content

fix(pi): ack messages after sendUserMessage succeeds#66

Open
philosophicalsmith-beep wants to merge 1 commit into
aannoo:mainfrom
philosophicalsmith-beep:fix/pi-ack-after-send
Open

fix(pi): ack messages after sendUserMessage succeeds#66
philosophicalsmith-beep wants to merge 1 commit into
aannoo:mainfrom
philosophicalsmith-beep:fix/pi-ack-after-send

Conversation

@philosophicalsmith-beep

Copy link
Copy Markdown

Summary

  • acknowledge Pi hcom messages immediately after sendUserMessage succeeds
  • prevents pendingAckId from staying set when Pi does not emit the deferred extension input ack

Problem

Pi delivery currently sets pendingAckId before calling sendUserMessage, then waits for the deferred input ack path to clear it. In practice, messages delivered through sendUserMessage(..., { deliverAs: "followUp" }) may not trigger that ack path reliably. Once pendingAckId remains set, later wake events skip delivery and unread hcom messages stay queued.

Observed locally before this change:

  • first hcom message reached the Pi agent
  • second message stayed unread
  • logs repeatedly showed notify_server.wake with the old pending_ack

Fix

After sendUserMessage returns successfully, call ackPending(...) right away. At that point the message has been handed to Pi's input queue, so hcom can safely mark it delivered instead of waiting on an extension input callback that may not arrive.

If sendUserMessage throws, the existing catch path still clears pendingAckId and reports the delivery error.

Verification

  • cargo check
  • cargo build --release
  • local Pi smoke test with patched binary/plugin:
    • sent three consecutive hcom requests to a Pi agent
    • received KAVA_ONE, KAVA_TWO, and KAVA_THREE
    • hcom pi-read --name kava returned []
    • logs showed plugin.deferred_ack with source: "sendUserMessage:followUp", followed by later wake events with pending_ack: null

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