P1: Make write retries safe and preserve typed HTTP errors and continuation #835

Open
opened 2026-10-02 13:23:03 +00:00 by kayg · 2 comments
Owner

Research follow-up under #484. Source snapshot: c4a61e8cf090170f35b1bed3350d9de20c83ecd5.

The shared request helper avoids retrying ambiguous write transport failures, but it retries cloneable writes after explicit transient HTTP responses.

Evidence:

  • crates/calternal-sync/src/remote.rs:send_with_retry retries 429/502/503/504 without testing the method or an idempotency guarantee. A gateway response does not prove the upstream write was not committed.
  • No common idempotency-key contract was found in the reviewed action registry/API.
  • The API has a useful stable ErrorEnvelope; the CLI flattens it to a message and sometimes classifies exit status by matching HTTP text.
  • Lists use different continuation models. Some bounded tool responses suggest paging/ranges even where the route has no such parameter.

Acceptance:

  1. Retry a write only under a documented safe guarantee; preserve safe-read retry and bounded backoff. Add a controlled committed-write-then-transient-response regression fixture. Duplicate execution was not reproduced in this review.
  2. Specify operation idempotency and replay semantics at the server contract, not by guessing from a tool name.
  3. Preserve typed status/code/details/retry metadata through CLI and tools. Evaluate compatible problem+json support without breaking existing clients.
  4. Document bounded limits, continuation, cursor expiry and overflow recovery per list/byte operation. Do not imply a range fallback on a route that cannot provide it.

Related #484; matrix F2/F3/M1 and source review Retry semantics. This is a reliability risk found in source, not a demonstrated data-loss incident.

Full evidence and decisions: docs/research/agent-surfaces.md on branch job/research-surfaces. No runtime change was made by the research job.

Research follow-up under #484. Source snapshot: `c4a61e8cf090170f35b1bed3350d9de20c83ecd5`. The shared request helper avoids retrying ambiguous write transport failures, but it retries cloneable writes after explicit transient HTTP responses. Evidence: - `crates/calternal-sync/src/remote.rs:send_with_retry` retries 429/502/503/504 without testing the method or an idempotency guarantee. A gateway response does not prove the upstream write was not committed. - No common idempotency-key contract was found in the reviewed action registry/API. - The API has a useful stable `ErrorEnvelope`; the CLI flattens it to a message and sometimes classifies exit status by matching HTTP text. - Lists use different continuation models. Some bounded tool responses suggest paging/ranges even where the route has no such parameter. Acceptance: 1. Retry a write only under a documented safe guarantee; preserve safe-read retry and bounded backoff. Add a controlled committed-write-then-transient-response regression fixture. Duplicate execution was not reproduced in this review. 2. Specify operation idempotency and replay semantics at the server contract, not by guessing from a tool name. 3. Preserve typed status/code/details/retry metadata through CLI and tools. Evaluate compatible problem+json support without breaking existing clients. 4. Document bounded limits, continuation, cursor expiry and overflow recovery per list/byte operation. Do not imply a range fallback on a route that cannot provide it. Related #484; matrix F2/F3/M1 and source review Retry semantics. This is a reliability risk found in source, not a demonstrated data-loss incident. Full evidence and decisions: `docs/research/agent-surfaces.md` on branch `job/research-surfaces`. No runtime change was made by the research job.
Author
Owner

#835 finding and coordination with #815/agentfix: calternal-sync::send_with_retry is the one shared helper used by CLI and Sync. Its transient HTTP branch currently replays writes regardless of commit uncertainty. This job will restrict that branch to safe reads and add a controlled committed-write/503 fixture. It will not create another helper. Contract replay declarations remain never for writes; neither an arbitrary Idempotency-Key header nor an HTTP verb proves server deduplication. Agentfix worktree inspected read-only; retry helper unchanged there at this point.

#835 finding and coordination with #815/agentfix: `calternal-sync::send_with_retry` is the one shared helper used by CLI and Sync. Its transient HTTP branch currently replays writes regardless of commit uncertainty. This job will restrict that branch to safe reads and add a controlled committed-write/503 fixture. It will not create another helper. Contract replay declarations remain `never` for writes; neither an arbitrary Idempotency-Key header nor an HTTP verb proves server deduplication. Agentfix worktree inspected read-only; retry helper unchanged there at this point.
Author
Owner

#835 implementation finding: CLI file transfers use RemoteClient, while generated actions use the direct CLI HTTP client. The former previously discarded server codes, details and Retry-After into Error::Server(status).

Minimal public addition: RemoteClient::with_http_details() opts the CLI into Error::ServerDetails(Box<HttpFailure>). Default daemon callers keep their existing status, cursor-expiry and precondition variants. CLI action and transfer failures now share read_http_failure, with one 128 KiB body budget and the common calternal-api envelope parser. Existing human error strings remain unchanged. Typed non-envelope failures (such as Undo conflict reports) also retain their body.

The shared retry helper remains the only retry implementation. It never replays POST, PUT, PATCH or DELETE after 429/502/503/504 or an uncertain transport error. An unrecognized key or a precondition does not grant replay. Credential-consuming GETs, co-edit upgrades and limited Public downloads explicitly declare no replay. Four pure POST previews/parsers are read-only but still declare no transport replay.

Browser client verification: 19 pass, 0 fail. New regression cases cover Retry-After, future envelope fields, typed conflict bodies, default daemon compatibility, and a committed-write counter over transient status responses. Full Rust gates and the real-server build are still pending on this loaded host. No full live parity result is claimed.

#835 implementation finding: CLI file transfers use `RemoteClient`, while generated actions use the direct CLI HTTP client. The former previously discarded server codes, details and Retry-After into `Error::Server(status)`. Minimal public addition: `RemoteClient::with_http_details()` opts the CLI into `Error::ServerDetails(Box<HttpFailure>)`. Default daemon callers keep their existing status, cursor-expiry and precondition variants. CLI action and transfer failures now share `read_http_failure`, with one 128 KiB body budget and the common calternal-api envelope parser. Existing human error strings remain unchanged. Typed non-envelope failures (such as Undo conflict reports) also retain their body. The shared retry helper remains the only retry implementation. It never replays POST, PUT, PATCH or DELETE after 429/502/503/504 or an uncertain transport error. An unrecognized key or a precondition does not grant replay. Credential-consuming GETs, co-edit upgrades and limited Public downloads explicitly declare no replay. Four pure POST previews/parsers are read-only but still declare no transport replay. Browser client verification: 19 pass, 0 fail. New regression cases cover Retry-After, future envelope fields, typed conflict bodies, default daemon compatibility, and a committed-write counter over transient status responses. Full Rust gates and the real-server build are still pending on this loaded host. No full live parity result is claimed.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal#835
No description provided.