@@ -144,45 +144,85 @@ export function parseFallbackRunCorrelation(displayTitle: string | undefined | n
144144 return { prNumber, headSha : ( match [ 2 ] as string ) . toLowerCase ( ) } ;
145145}
146146
147- /** True when a fallback run for this EXACT (prNumber, headSha) is already queued or in progress -- checked
148- * by buildCapture before dispatching, so the existing recapture-poll retry (every 90s, up to 5 attempts,
149- * see PREVIEW_POLL_SECONDS/MAX_PREVIEW_POLLS in processors.ts) doesn't repeatedly re-dispatch while waiting
150- * for the SAME run's workflow_run completion. That matters because the workflow's own `concurrency: group:
147+ // ---------------------------------------------------------------------------------------------------------
148+ // Dispatch in-flight marker -- a persisted R2 sentinel, not a live GitHub API query (#4112 review fix).
149+ // ---------------------------------------------------------------------------------------------------------
150+
151+ const FALLBACK_DISPATCH_MARKER_NAMESPACE = "gittensory/fallback-dispatch/" ;
152+
153+ /** The workflow's own `timeout-minutes: 15` (visual-capture-fallback.yml) plus a buffer for GitHub's own
154+ * runner-queueing delay before the job even starts -- a marker older than this is treated as abandoned
155+ * (the run either finished without a webhook ever reaching us, or GitHub silently dropped the dispatch)
156+ * rather than blocking dispatch forever. */
157+ const FALLBACK_DISPATCH_MARKER_MAX_AGE_MS = 18 * 60 * 1000 ;
158+
159+ async function fallbackDispatchMarkerR2Key ( headSha : string ) : Promise < string > {
160+ const fingerprint = await sha256Hex ( `${ headSha } ) ;
161+ return `${ FALLBACK_DISPATCH_MARKER_NAMESPACE } ${ fingerprint . slice ( 0 , 40 ) } ;
162+ }
163+
164+ /** True when a fallback run for this head SHA was dispatched recently enough that it may still be
165+ * queued/in-progress -- checked by buildCapture BEFORE dispatching, so the existing recapture-poll retry
166+ * (every 90s, up to 5 attempts -- see PREVIEW_POLL_SECONDS/MAX_PREVIEW_POLLS in processors.ts, a 7.5-minute
167+ * window comfortably inside the workflow's own 15-minute timeout) doesn't repeatedly re-dispatch while a
168+ * build is still running. That matters because the workflow's own `concurrency: group:
151169 * visual-capture-fallback-${{ inputs.head_sha }}` + `cancel-in-progress: true` means a second dispatch for
152- * the same head SHA CANCELS the first -- without this check, a poll firing before a slow build finishes
153- * would cancel-and-restart it every 90s and the fallback could never complete. Queries GitHub's own run
154- * list rather than persisting new dispatch-tracking state, mirroring this pipeline's existing
155- * live-query-don't-persist pattern (getLatestDeploymentStatus, findPreviewUrlFromChecks). Fails OPEN (false,
156- * "nothing in flight") on any error -- a transient list-runs failure should still let the existing
157- * concurrency group be the backstop dedup, not silently stop the fallback from ever being tried. */
158- export async function hasInFlightFallbackDispatch ( params : {
159- token : string ;
160- repo : GitHubRepo ;
161- prNumber : number ;
162- headSha : string ;
163- rateLimitAdmissionKey ?: GitHubRateLimitAdmissionKey | undefined ;
164- } ) : Promise < boolean > {
165- const base = `https://api.github.com/repos/${ params . repo . owner } ${ params . repo . repo } ;
170+ * the same head SHA CANCELS the first -- without this check, a poll firing well within the 15-minute budget
171+ * would cancel-and-restart the run on every single poll and the fallback could never complete.
172+ *
173+ * A PERSISTED marker (not a live GitHub list-runs query) is deliberate: a freshly-dispatched run isn't
174+ * guaranteed to be visible via the Actions API the instant `dispatchVisualCaptureFallback` returns (GitHub's
175+ * own eventual consistency), so a live query taken right after dispatch could itself race and report
176+ * "nothing in flight" moments after a dispatch just succeeded. Writing the marker synchronously on a
177+ * successful dispatch closes that gap. Fails OPEN (false, "nothing in flight") on any read error -- a
178+ * transient R2 failure should still let the existing concurrency group be the backstop dedup, not silently
179+ * stop the fallback from ever being tried. */
180+ export async function isFallbackDispatchInFlight ( env : Env , headSha : string ) : Promise < boolean > {
181+ if ( ! env . REVIEW_AUDIT ) return false ;
166182 try {
167- const response = await timeoutFetch ( `${ base } ${ FALLBACK_WORKFLOW_FILE } , {
168- headers : githubApiHeaders ( params . token ) ,
169- signal : AbortSignal . timeout ( DEFAULT_TIMEOUT_MS ) ,
170- githubRateLimitAdmission : params . rateLimitAdmissionKey !== undefined ,
171- ...( params . rateLimitAdmissionKey ? { githubRateLimitAdmissionKey : params . rateLimitAdmissionKey } : { } ) ,
172- } ) ;
173- if ( ! response . ok ) return false ;
174- const payload = ( await response . json ( ) . catch ( ( ) => null ) ) as { workflow_runs ?: Array < { status ?: string ; display_title ?: string } > } | null ;
175- const headSha = params . headSha . toLowerCase ( ) ;
176- return ( payload ?. workflow_runs ?? [ ] ) . some ( ( run ) => {
177- if ( run . status !== "queued" && run . status !== "in_progress" ) return false ;
178- const correlation = parseFallbackRunCorrelation ( run . display_title ) ;
179- return correlation !== null && correlation . prNumber === params . prNumber && correlation . headSha === headSha ;
180- } ) ;
183+ const object = await env . REVIEW_AUDIT . get ( await fallbackDispatchMarkerR2Key ( headSha ) ) ;
184+ if ( ! object ) return false ;
185+ const text = await new Response ( object . body ) . text ( ) ;
186+ const marker = JSON . parse ( text ) as { dispatchedAt ?: number } ;
187+ if ( typeof marker . dispatchedAt !== "number" ) return false ;
188+ return Date . now ( ) - marker . dispatchedAt < FALLBACK_DISPATCH_MARKER_MAX_AGE_MS ;
181189 } catch {
182190 return false ;
183191 }
184192}
185193
194+ /** Record that a fallback dispatch just succeeded for this head SHA, so a subsequent buildCapture call
195+ * (e.g. the next recapture poll) sees it via isFallbackDispatchInFlight instead of re-dispatching. Best
196+ * effort -- a failed write just means the concurrency group's cancel-in-progress behavior is the only
197+ * remaining backstop, same as before this marker existed. */
198+ export async function markFallbackDispatched ( env : Env , headSha : string ) : Promise < void > {
199+ if ( ! env . REVIEW_AUDIT ) return ;
200+ try {
201+ const key = await fallbackDispatchMarkerR2Key ( headSha ) ;
202+ await env . REVIEW_AUDIT . put ( key , JSON . stringify ( { dispatchedAt : Date . now ( ) } ) , {
203+ httpMetadata : { contentType : "application/json" } ,
204+ } ) ;
205+ } catch {
206+ // best effort -- see doc comment above
207+ }
208+ }
209+
210+ /** Clear the in-flight marker once the dispatched run has settled (ANY conclusion -- success, failure,
211+ * cancelled, timed_out all mean "no longer in flight"), called from the workflow_run webhook handler in
212+ * processors.ts. Best effort -- if this never runs (a lost webhook delivery), FALLBACK_DISPATCH_MARKER_MAX_AGE_MS
213+ * is the fail-safe expiry so a genuinely stuck marker can't block retries forever. A try/catch (not just a
214+ * `.catch()` on the delete call) matters here: a minimal/partial R2Bucket implementation that doesn't
215+ * implement `delete` at all throws SYNCHRONOUSLY at the call site (`TypeError: ... is not a function`),
216+ * before any `.catch()` on its return value would even attach. */
217+ export async function clearFallbackDispatchMarker ( env : Env , headSha : string ) : Promise < void > {
218+ if ( ! env . REVIEW_AUDIT ) return ;
219+ try {
220+ await env . REVIEW_AUDIT . delete ( await fallbackDispatchMarkerR2Key ( headSha ) ) ;
221+ } catch {
222+ // best effort -- see doc comment above
223+ }
224+ }
225+
186226// ---------------------------------------------------------------------------------------------------------
187227// Minimal ZIP reader -- just enough to read a GitHub Actions artifact (STORED / DEFLATE entries only).
188228// ---------------------------------------------------------------------------------------------------------
0 commit comments