Skip to content

fix: acknowledge delivered invocation input - #867

Draft
zhongkechen wants to merge 4 commits into
mainfrom
codex/fix-861-invocation-input-ack
Draft

zhongkechen wants to merge 4 commits into
mainfrom
codex/fix-861-invocation-input-ack

Conversation

@zhongkechen

Copy link
Copy Markdown
Contributor

Issue #, if available

Fixes #861.

Description of changes

Invocation input construction cleared pending update IDs before delivery succeeded,
while a valid checkpoint-free resume did not acknowledge the input sequence. Retain
those IDs until a valid response, then consume only operation versions covered by
that input. Failed delivery retains updates for retry, newer changes stay pending,
and an invocation acknowledgment cannot retreat a newer checkpoint watermark.

Regression coverage uses wait completion, so this fix is independent of #860's
callback producer change. Public-handler tests cover memory/filesystem stores,
failure before SDK input delivery, legacy stored state, multiple resumes, and saved
steps executing once. No existing test expectations change.

The existing
test_pending_response_is_valid_when_an_operation_completed_after_the_handler_saw_it
fixture now supplies concrete operation/update collections read by the real
acknowledgment path; its assertions are unchanged.

Design

  • Design note (rule 3): not needed; this repairs acknowledgment behavior using
    existing fields and methods, with no public API change.
  • Simpler options considered (rule 2): moving the existing unconditional clear to
    invocation completion would consume newer changes. Reuse the existing
    per-operation versions and advance_handler_seen; no new delivery ledger is
    needed. Keep the watermark monotonic while pruning IDs covered by each actual
    acknowledgment, including equal/older acknowledgments.
  • Root cause (rule 5): preparing input was treated as successful delivery, and
    successful invocation completion never acknowledged checkpoint-free input.
  • Net change: +9 source lines; zero public API members added or removed.

Assumptions

Decisions not covered by the issue or the design note

Validation

  • Python 3.13.17, Hatch 1.16.5.
  • Final focused regression suite against baseline d7ae955: 26 failed, 10 passed;
    baseline import path verified before running. With this fix: 36 passed.
  • hatch run dev-testing:typecheck: passed (137 source files).
  • Package hatch fmt --check: passed.
  • hatch run dev-testing:test -q --tb=short: 1,784 passed. Reran with loopback
    access after the sandbox blocked the HTTP test servers.
  • hatch run dev-testing:js-examples: 123 test files passed, 150 tests passed,
    zero retries
    ; four selected files have no cloud tests. JS SDK commit
    3deb8389531d35642e2d4657a69575522dc7a7ea.
  • git diff --check: passed.

Checklist

  • Every new branch has a test that fails without it (rule 4).
  • The first commit adds regression tests; before/after evidence is above (rule 5).
  • No existing test expectations changed (rule 5).

By submitting this pull request, I confirm that you can use, modify, copy, and
redistribute this contribution, under the terms of your choice.

This branch has not been deployed

No deployments
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.

[Testing]: Acknowledge invocation input without losing pending updates

1 participant