Skip to content

Commit e71af6c

Browse files
committed
fix(incident): stop suggesting a per-incident rewrite after a verification miss
/incident/comment is a single batch POST covering the whole incident_ids list (lte=100 on the wire) — there is no separate per-incident write to retry. The previous message told the agent not to retry the full batch but to "write it again for that incident alone" on a verification miss, which carries the identical duplication risk at incident-granularity that this whole redesign exists to eliminate at batch-granularity: a miss here proves nothing about absence, only that presence could not be confirmed, so a rewrite risks a duplicate on a write that most likely already landed. Drop the rewrite suggestion. Point at a read-only manual check (`fduty incident timeline <id>`) instead, and say plainly that neither a full-batch retry nor a single-incident rewrite is warranted by a missed confirmation.
1 parent dd164b1 commit e71af6c

2 files changed

Lines changed: 22 additions & 14 deletions

File tree

internal/cli/command_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -771,8 +771,8 @@ func TestCommandIncidentCommentFailsWhenTextNotFoundWithinBudget(t *testing.T) {
771771
if strings.Contains(err.Error(), "corrupted") {
772772
t.Fatalf("[not-found] must not claim corruption it has not established: %v", err)
773773
}
774-
if !strings.Contains(err.Error(), "do not retry this command against the full batch") {
775-
t.Fatalf("[not-found] must warn against a full-batch retry: %v", err)
774+
if !strings.Contains(err.Error(), "Do not write the comment again") {
775+
t.Fatalf("[not-found] must warn against rewriting, at any granularity: %v", err)
776776
}
777777
if strings.Contains(out, "Commented on") {
778778
t.Fatalf("[not-found] must not report success:\n%s", out)
@@ -910,7 +910,7 @@ func TestCommandIncidentCommentTransportErrorCarriesAntiRetryWarning(t *testing.
910910
if !strings.Contains(err.Error(), "inc-1") {
911911
t.Fatalf("[transport-error] expected inc-1 named in the error: %v", err)
912912
}
913-
if !strings.Contains(err.Error(), "do not retry this command against the full batch") {
913+
if !strings.Contains(err.Error(), "Do not write the comment again") {
914914
t.Fatalf("[transport-error] must carry the same anti-retry framing as the not-found case: %v", err)
915915
}
916916
if strings.Contains(err.Error(), "corrupted") {

internal/cli/incident.go

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -965,19 +965,26 @@ const maxIncidentFeedVerifyPages = 20
965965
// assumption about /incident/feed's undocumented default sort) at all.
966966
//
967967
// It checks every id in ctx.Args before returning, rather than stopping at
968-
// the first problem: the error text below tells the caller the write already
969-
// succeeded for the WHOLE batch and to fix only the listed incident(s)
970-
// individually, so that claim has to actually be backed by having examined
971-
// every id — reporting only the first failure would leave any incident after
972-
// it silently, permanently uncommented while the agent believes it handled
973-
// everything the error mentioned.
968+
// the first problem: /incident/comment is a single batch POST covering the
969+
// whole list (incident_ids has an lte=100 batch cap on the wire, not a
970+
// per-id write), so the error text below has to have actually examined every
971+
// id before it can say what it says about the batch as a whole — reporting
972+
// only the first failure would leave any incident after it silently,
973+
// permanently unexamined while the agent believes it handled everything the
974+
// error mentioned.
974975
//
975976
// Neither a page-budget miss nor a transport error while re-fetching the feed
976977
// means the write was corrupted — corruption can no longer even be observed
977978
// by this check, since it never looks at any text other than an exact match.
978-
// Both are reported as "could not confirm," with identical anti-retry
979-
// framing, because both follow a write the API already reported as
980-
// successful.
979+
// Both are reported as "could not confirm," with identical framing, because
980+
// both follow a write the API already reported as successful, and because
981+
// this check can only confirm presence, never confirm absence: a miss here
982+
// is not evidence the comment is missing, so it is not grounds to write it
983+
// again either — for the whole batch (there is no per-incident write to
984+
// retry, only the one batch write already made) or for a single listed
985+
// incident (which risks a duplicate on a write that most likely landed and
986+
// simply couldn't be located within budget). The safe next step is to look,
987+
// not to write.
981988
func verifyIncidentCommentsWritten(ctx *RunContext, want string) error {
982989
var problems []string
983990
for _, id := range ctx.Args {
@@ -994,8 +1001,9 @@ func verifyIncidentCommentsWritten(ctx *RunContext, want string) error {
9941001
}
9951002
return fmt.Errorf(
9961003
"comment verification could not confirm %d of %d incident(s) — %s. "+
997-
"The write API already reported success for all %d incident(s) in this batch, so do not retry this command against the full batch: that would duplicate the comment on incidents already confirmed. "+
998-
"Check the listed incident(s) manually, and if a comment is genuinely missing, write it again for that incident alone.",
1004+
"The write API already reported success for all %d incident(s) in this batch (a single POST covering the whole list), and not finding a match here does not mean a comment is missing — this check can only confirm presence, never confirm absence. "+
1005+
"Do not write the comment again, for the full batch or for the incident(s) listed above alone: either risks a duplicate on a write that most likely already landed. "+
1006+
"Run `fduty incident timeline <id>` on the listed incident(s) to check by hand before deciding on anything further.",
9991007
len(problems), len(ctx.Args), strings.Join(problems, "; "), len(ctx.Args))
10001008
}
10011009

0 commit comments

Comments
 (0)