Reconcile visibility-transition: counter accuracy in ambiguous create-error paths

[marcusrbrown]

Source: ce:review round 2 on feat/private-visibility-transition (adversarial, P3 conf 0.64)
Scope: scripts/reconcile-repos.ts — visibilityTransitionSucceeded counter in runIssueQueue

Problem

When issues.create returns an error but the alert was actually created on GitHub's side (e.g., network timeout AFTER the API processed the request, ambiguous 5xx, retry-induced double-write), the local counter increments failed even though the issue exists. Conversely, a successful API response could in theory be lost in transit.

Suggested fix (one of):

A: After an ambiguous create failure, check for the exact expected title via issues.listForRepo before incrementing failed. Reconciles the counter with reality at the cost of an extra API call per failure.

B: Document the counter semantics in JSDoc: "counts API acknowledgements, not confirmed-created alerts". Accept the imprecision.

Option B is fine for current scale (visibility transitions are rare). Option A becomes worthwhile if the counter is ever used for SLO/alerting.

Acceptance

• Counter semantics are unambiguous (either accurate or clearly documented).

Why not in the round-3 fix

P3 advisory. Round-3 was scoped to the new P1 + test gaps + shell-injection defense.

[fro-bot]

Triage complete. The issue accurately describes a real semantic gap in runIssueQueue on feat/private-visibility-transition.

Confirmed behavior

At scripts/reconcile-repos.ts:2434 the catch increments failed on any throw from callIssuesCreate. That includes the ambiguous failure modes the issue calls out:

• Network/transport error after GitHub already processed the POST /issues (issue created, counter says failed)
• 5xx where the request may or may not have been applied
• Retry-induced double-write where one of the two succeeded on GitHub but the wrapper saw an error

For visibility-transition entries specifically the existing dedup at lines 2372–2393 already does a listForRepo filtered by VISIBILITY_TRANSITION_LABEL before the create. That same listing infrastructure can be reused post-failure, so Option A is cheaper here than for the per-repo/rollup paths — no new pagination shape needed.

Reproduction

Not worth standing up a live repro for a P3. The failure mode is structurally obvious from the code: any rejection inside the try-block is funnelled into failed += 1 with no reconciliation against server-side state. The branch has no test asserting counter semantics under partial-failure conditions (grep on the test file shows no visibilityTransitionSucceeded assertions paired with a mocked-then-throwing issues.create).

Recommendation: Option B, with one structural nudge

The counter is consumed at reconcile-repos.ts:1541 as visibilityTransitionIssues in the run outcome and surfaced in the Step Summary (per the sibling issue #3333, that surfacing is itself still missing). It is not wired to any SLO or alerting today. Paying an extra listForRepo per failure to reconcile a counter that nobody graphs is premature precision.

Land Option B:

1. JSDoc on the return type at scripts/reconcile-repos.ts:2360: counters reflect API-acknowledged creates, not server-confirmed alert existence. Ambiguous failures are counted as failed even when the alert may exist on GitHub.
2. Mirror that note wherever the counter is rendered into the Step Summary (coordinate with Reconcile: visibilityTransitions counter missing from operator-readable Step Summary #3333).
3. Add a TODO referencing this issue so the upgrade path to Option A is obvious if the counter ever feeds an SLO.

If Option A is ever needed, the dedup listForRepo block above can be lifted into a small helper and called from the catch — the exact-title comparison ([INTEGRITY] Visibility transition for ${node_id}) already handles the lookup deterministically.

Suggested next steps

• Open a small PR against feat/private-visibility-transition with the JSDoc + TODO (Option B).
• Coordinate the Step Summary wording fix with Reconcile: visibilityTransitions counter missing from operator-readable Step Summary #3333 so the operator-visible surface stays consistent.
• Leave Reconcile visibility-transition: duplicate-skip is silent (no counter, no log) #3334 and Reconcile visibility-transition: legacy entries without node_id have no alertable subject #3335 to their own threads — they are separate counter/durability concerns on the same branch.

Related: #3333, #3334, #3335 (sibling triage findings on the same branch).

---

Run Summary

Field	Value
Event	issues
Repository	fro-bot/.github
Run ID	26150084496
Cache	hit
Session	ses_1bb8e13dfffeGK6HhVYVdbeI0J