Stop retrying a connected account that has failed 2953 times - #206
Merged
Merged
Conversation
sp_account.consecutive_failures was incremented on every failure and reset on every success, and then never read by anything. So an account whose posting had stopped working entirely was retried on every cadence tick, indefinitely. One Reddit account is at 2,953 consecutive failures with zero successes. Over the last seven days it was attempted 383 times and posted 0 times, each attempt launching a real browser. Nothing surfaced this: the campaign looked healthy, the account still read `active`, and the only trace was a `last_error` on rows nobody had reason to open. The browser path does disable an account, but only when it recognises a login wall. That is the failure you get when cookies die. It is not the failure you get when a site redesigns: then the selector simply stops matching and every attempt fails with a plain timeout, which no rule looked at. Reddit moved its composer to shreddit web components on 2026-08-14 and the account has failed on `shreddit-composer[name="body"]` 292 times since. So both post paths now stop at a ceiling of 10 consecutive failures and set status token_expired — the status the UI already renders as "reconnect", and one that account selection skips, so the retries stop and the user finds out. The rule lives in its own dependency-free module because the Next server and the Playwright worker both need it, and the server must not pull the browser stack into its bundle to read one number. This does not fix the selectors. The accounts that work are the OAuth ones; every failing account is cookie-auth, and reconnecting those over OAuth — which this codebase already implements for reddit, x, linkedin and facebook — skips the browser entirely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan35 finding(s) HIGH/CRITICAL: 3 | MEDIUM: 23 | LOW: 9
Snippets are redacted; ThreatCrush never prints matched credential material. |
ralyodio
added a commit
that referenced
this pull request
Aug 19, 2026
* docs(promote): reconcile the architecture doc with #205 and #206 The architecture doc was written in #204 and hasn't moved since, but two PRs landed under it the same day and both changed things it describes as settled. #205 added effectiveMix(): the ownership mix is narrowed to the classes a campaign can actually supply before any deficit is computed. §4.1 still described the raw configured mix, which is the version that mislabelled six production posts as via_fallback and was three posts from silencing the campaign. Someone reading §4 to extend the blend would have rebuilt the bug, so the rule and the reason it exists are now in the doc rather than only in the PR that fixed it. §4.2 gets the corresponding scope limit: covering for a class the campaign never had is not a fallback and is not capped as one. #206 added lib/sp/accountHealth.ts, which partly satisfies §14's "pause campaigns after repeated provider rejections" — at the account layer, not the campaign layer. Noted as half-built, because the Reddit provider will still need the campaign-level version: a subreddit rejection is a destination fact, not an account one. Docs only; no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Make a Promote publication happen at most once (#AC10, #AC14) The sweep decided and published in one pass, "claiming" a campaign by pushing next_run_at forward. That UPDATE carried no predicate on next_run_at, so it was a read-then-write: two sweeps that both read the row both won it. And the worker runs the sweep on a 60s interval *and* out-of-band whenever someone clicks "Post now", so overlapping runs are designed in, not rare. Nothing downstream was idempotent either. If the process died after postViaAccount() published but before the promo_post insert, there was no record it had happened — and last_promoted_at is only stamped at the end of the campaign, so the same link was still least-recently-promoted on the next tick and went out again. The user sees a duplicate; the logs show nothing. promo_job makes the intended publication the unit of work, written down before anything is sent. Two mechanisms: - Plan before publishing. Jobs are keyed on sha256(list, link, account, destination, kind, slot), where the slot is the next_run_at value the sweep observed as due. A racing sweep reads the same due row, derives the same keys, loses to the unique index, and gets nothing back — so it publishes nothing. Keying on the wall clock instead would give each sweep its own key and rebuild the bug. - Claim by compare-and-swap: update ... where id = ? and state = 'queued'. Read and write are one statement, so two workers cannot both see 'queued'. The campaign-level claim keeps its place but now carries its predicate. It is an optimization — it avoids duplicated work. The guarantee is in the job. AT MOST ONCE, ON PURPOSE. A job still 'publishing' past a 10 minute lease is failed, never retried. No provider we publish through accepts an idempotency key, so an interrupted publish has genuinely unknown outcome — it may be live. Re-running it is the duplicate this exists to prevent. The reaper closes it with the outcome recorded as unknown and leaves it in history for a human; the credit is not refunded, because refunding a post that did land is the other way to be wrong. Retry stays available for failures that provably happened before the publish call, which is what 'retrying' and attempt_count are for. Stray jobs are closed rather than left queued: nothing reclaims a queued job, so one stranded by a disconnected account or a credit pause would sit there forever misreporting the campaign as backed up. MIGRATION FIRST. planJobs() logs loudly and publishes nothing if promo_job is missing, rather than failing the select and stopping every campaign in silence the way source_mix did. That is a guard, not the fix — apply 20260819120000_promote_jobs.sql before this deploys. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Claim a campaign on "still due" rather than an exact timestamp match The claim predicate compared next_run_at to the exact string we read back. That is correct only if a timestamptz round-trips to a byte-identical value through PostgREST, and if it ever did not, the update would match nothing, no campaign would be claimed, and Promote would stop posting entirely with nothing in the logs that looks like a failure. That is the same silent-stop shape the source_mix migration cost us a day for, and it is not worth risking to save a predicate. Re-asserting the condition the select already used gives the same atomicity with none of that exposure: the minimum cadence is 300s, so whoever wins the claim pushes next_run_at well into the future and the loser's `lte` cannot match. The scheduling slot the idempotency key is built from is still the observed next_run_at, so racing sweeps still agree on the slot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(promote): match §9.2 to the claim predicate that shipped Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(promote): key promo_job.user_id to profiles, like every other promote table promo_list.user_id references public.profiles(id). Pointing promo_job at auth.users instead would let a job outlive the profile row the rest of the feature is keyed to, and would cascade differently from its own campaign. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Investigating why x/reddit/facebook weren't posting turned up something worse than a flake.
What's actually happening
Split is perfect, and it's not random:
oauthcookieshredditweb components; 292 failures onshreddit-composer[name="body"]alone.openai: 429 You have no credits remaining. That cleared on its own at 09:53 today.The bug this PR fixes
sp_account.consecutive_failureswas incremented on failure and reset on success — and never read by anything.The browser path does disable an account, but only on a recognised login wall. That's the cookies-are-dead failure. A site redesign produces a plain selector timeout instead, which no rule looked at, so the worker launched a real browser at a dead account every cadence tick for four days.
Both post paths now stop at 10 consecutive failures and set
token_expired— already the UI's "reconnect" status, and one account selection skips.The rule is its own dependency-free module: the Next server and the Playwright worker both need it, and the server must not pull the browser stack into its bundle to read one number.
What this does NOT fix
The selectors. The real fix is reconnecting those four over OAuth — already implemented here (
app/api/sp/oauth/{reddit,x,linkedin,facebook}/start) and demonstrably the only mode that works. Driving somebody else's redesigned DOM will keep breaking; their API won't.Tests
vitest run: 1648 passed / 0 failed.tsc --noEmitclean.🤖 Generated with Claude Code