Problem
crit push POSTs each comment via gh api and assigns the returned github_id locally. If the network call returns a transport error AFTER the server already accepted the POST (e.g., timeout reading the response, TCP RST during read), the local file does not get the github_id and the next push will POST again, creating a duplicate remote comment.
This is the classic at-least-once delivery problem. We have no idempotency key on the request, so retries can't be deduped server-side.
Suggested approach
Two options:
- Pull-before-push reconcile: before any POST, pull and dedupe by
(path, line, body, author, commit_sha) to drop already-present items. Cheap when comment count is small, but loses precision if the user is editing the same line multiple times.
- Stable client request id stored alongside
github_id (e.g., request_id populated client-side, posted in body markdown footer like <!-- crit:req:abc123 -->). On retry, dedupe by parsing that marker. Survives ghost-success.
Option 2 is more robust. Either way, decide before adding a roundtrip test — the test would otherwise be flaky.
Why no test yet
A reliable repro needs network mocking (kill connection between server-write and client-read). The roundtrip harness can't trigger this against the live API deterministically. Add the test once an idempotency strategy lands.
Source
Surfaced by the comment-scenario gap-finder ahead of PR #445.
Problem
crit pushPOSTs each comment viagh apiand assigns the returnedgithub_idlocally. If the network call returns a transport error AFTER the server already accepted the POST (e.g., timeout reading the response, TCP RST during read), the local file does not get thegithub_idand the next push will POST again, creating a duplicate remote comment.This is the classic at-least-once delivery problem. We have no idempotency key on the request, so retries can't be deduped server-side.
Suggested approach
Two options:
(path, line, body, author, commit_sha)to drop already-present items. Cheap when comment count is small, but loses precision if the user is editing the same line multiple times.github_id(e.g.,request_idpopulated client-side, posted in body markdown footer like<!-- crit:req:abc123 -->). On retry, dedupe by parsing that marker. Survives ghost-success.Option 2 is more robust. Either way, decide before adding a roundtrip test — the test would otherwise be flaky.
Why no test yet
A reliable repro needs network mocking (kill connection between server-write and client-read). The roundtrip harness can't trigger this against the live API deterministically. Add the test once an idempotency strategy lands.
Source
Surfaced by the comment-scenario gap-finder ahead of PR #445.