diff --git a/src/index.ts b/src/index.ts index 0b55fa4..724f7bc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -4,14 +4,16 @@ import { deleteRecords, fetchAllRecords, markRecordsSynced, - MARK_ABORTED, MARK_SYNCED, - MARK_TIMED_OUT, MarkSyncedItem, - MarkSyncedStop, + MarkAbortReason, PENDING_STATUS, } from '@/libs/records.js'; -import { describeApiError, isSystemicApiFailure } from '@/libs/api.js'; +import { + describeApiError, + isPermanentApiFailure, + isSystemicApiFailure, +} from '@/libs/api.js'; import { buildWritePreview, ensureOutputDirectory, @@ -426,7 +428,9 @@ function reportDeferredServerChanges(deferredRecords: WrittenRecord[]): void { // Projects each written record down to the `{ uuid, filePath }` shape the bulk // mark-synced call needs, preserving order so the returned outcomes stay aligned -// to `writtenRecords` by index. Chunking and the stop-on-abort logic live in +// to `writtenRecords` by index. Chunking and the stop-on-abort logic (timeout, a +// systemic failure with a permanent one also stopping the daemon, or a repeated +// request-shape 4xx) live in // `markRecordsSynced` (the records lib), keeping the API surface isolated there. function toMarkSyncedItems(writtenRecords: WrittenRecord[]): MarkSyncedItem[] { return writtenRecords.map(({ record, filePath }) => ({ @@ -435,44 +439,60 @@ function toMarkSyncedItems(writtenRecords: WrittenRecord[]): MarkSyncedItem[] { })); } -// How a mark-synced run ended, for the failure report: the stop reason plus how -// many pending records the run never reached. Bundled because they're only ever -// meaningful together (the abort description), so the headline can't be handed a -// count that belongs to a different reason. -type MarkStopReport = { - reason: MarkSyncedStop; - unattemptedCount: number; -}; - -// Headline for the mark-synced failure report. An abort reads differently from a -// scatter of per-record failures: it stopped the run early, so the pending count -// can fold in records never attempted after the abort. `unattemptedCount` is how -// many of those pending records were never sent (the chunks after the stop), so -// the abort wording only claims "the rest were not attempted" when that's true — -// an abort on the final chunk leaves nothing unattempted. All cases leave the -// listed records pending on the server. +// Headline for the mark-synced failure report. A permanent, timeout, transient, or +// request-shape abort all stop the run early (so the count can include records +// never attempted) and each reads differently from a scatter of per-record +// failures. Every daemon-aware clause is derived from the SAME (`abortReason`, +// `autoSyncEnabled`) the caller's returned stop signal uses, so the message can't +// claim a stop — or a "next run" — that won't happen (a one-shot `markpost sync` +// never had a daemon; mirroring the delete path, which says nothing about +// auto-sync). `hasUnattempted` is whether the abort left a trailing chunk unsent, +// so the wording only claims records were skipped when some actually were. function markFailureHeadline( pendingCount: number, - stop: MarkStopReport, + abortReason: MarkAbortReason, + { + autoSyncEnabled, + hasUnattempted, + }: { + autoSyncEnabled: boolean; + hasUnattempted: boolean; + }, ): string { - if (stop.reason === MARK_TIMED_OUT) { - return `Timed out marking records synced — stopped after the first timeout; ${pendingCount} record(s) still pending on the server, they may be re-written next run.`; + if (abortReason === 'permanent') { + const daemonClause = autoSyncEnabled ? ', so auto-sync was stopped;' : ';'; + // Note the unattempted tail like the transient branch does — the abort left + // later chunks unsent, so the count is more than just the records that failed. + const skipped = hasUnattempted ? ' (some were never attempted)' : ''; + // Don't prescribe `markpost config` — a 403 (plan limit / sign-ups disabled) + // isn't a token problem. markRecordsSynced already logged the failing chunk's + // case-specific reason to stderr; point the user at that. + return `Failed to mark ${pendingCount} record(s) synced${skipped} — a permanent error (authentication or a forbidden account) will recur every pass${daemonClause} the record(s) remain pending on the server. Fix the cause reported above and sync again.`; } - if (stop.reason === MARK_ABORTED) { - const notAttemptedClause = - stop.unattemptedCount > 0 ? ' and the rest were not attempted' : ''; - return `Aborted marking records synced — the server rejected the request wholesale (a 400/422), so every record would fail the same way${notAttemptedClause}; ${pendingCount} record(s) still pending on the server, they may be re-written next run.`; + if (abortReason === 'timeout') { + return `Timed out marking records synced — stopped after the batch that first timed out; ${pendingCount} record(s) still pending on the server, they may be re-written next run.`; } - // The generic branch also covers a systemic abort (auth/rate-limit/5xx), which - // stops the run early with no distinct stop reason — so surface how many of the - // pending records were never sent rather than implying all N were attempted. - const neverAttemptedClause = - stop.unattemptedCount > 0 - ? ` (${stop.unattemptedCount} never attempted — the run stopped early)` + if (abortReason === 'transient') { + // A systemic error (rate limit / 5xx) aborted the run to back off. Only claim + // records were skipped if a trailing chunk was actually unsent; the "re-written + // next run" hedge matches the timeout/generic branches (soft — true whenever + // the user next syncs, daemon or not), so it isn't gated on autoSyncEnabled. + const skipped = hasUnattempted ? ', so some were never attempted' : ''; + return `Failed to mark ${pendingCount} record(s) synced — a systemic error stopped the run early${skipped}; they remain pending on the server, they may be re-written next run.`; + } + + if (abortReason === 'request-shape') { + // Two consecutive chunks were rejected the same categorical way (a 400/422), + // so the request envelope itself is wrong and every record would fail alike. + const notAttemptedClause = hasUnattempted + ? ' and the rest were not attempted' : ''; - return `Failed to mark ${pendingCount} record(s) synced — written locally but still pending on the server${neverAttemptedClause}; they may be re-written next run.`; + return `Aborted marking records synced — the server rejected the request wholesale (a 400/422), so every record would fail the same way${notAttemptedClause}; ${pendingCount} record(s) still pending on the server, they may be re-written next run.`; + } + + return `Failed to mark ${pendingCount} record(s) synced — written locally but still pending on the server; they may be re-written next run.`; } // Surfaces mark-synced failures loudly (never as success): an unmarked record @@ -482,10 +502,10 @@ function markFailureHeadline( function reportMarkFailures( failures: WrittenRecord[], markedCount: number, - stop: MarkStopReport, + headline: string, spinner: Spinner, ): void { - spinner.error(markFailureHeadline(failures.length, stop)); + spinner.error(headline); failures.forEach(({ record, filePath }) => { // Sanitize the composed line: record.uuid comes from the same untrusted API // response as a title, and filePath embeds the user-configured output path — @@ -521,21 +541,27 @@ function forgetSettledRecords( // Marks every written record synced on the server after a write, so the next // run's pending-only fetch skips them — the autoDelete-off path's -// non-destructive equivalent of the delete step. +// non-destructive equivalent of the delete step. Returns whether the autoSync +// daemon should stop: true only when a permanent failure (dead token / forbidden +// account) struck AND a daemon was running (`autoSyncEnabled`), so the caller can +// break the loop into the same doomed PATCH every pass — the mark-synced +// counterpart of the delete path's `deletePermanentlyFailed`. async function markWrittenRecordsSynced( writtenRecords: WrittenRecord[], spinner: Spinner, writtenState: Map, -): Promise { + { autoSyncEnabled }: { autoSyncEnabled: boolean }, +): Promise { if (writtenRecords.length === 0) { - return; + return false; } spinner.start('Marking records synced...'); - const { outcomes, stoppedBy } = await markRecordsSynced( + const { outcomes, abortReason } = await markRecordsSynced( toMarkSyncedItems(writtenRecords), ); + const permanentlyFailed = abortReason === 'permanent'; // A record is settled only when its mark-synced outcome is MARK_SYNCED; it is // pending if its mark failed or was never attempted (its outcome is undefined // because an abort — a timeout, a systemic failure, or a request-shape 4xx — @@ -554,20 +580,38 @@ async function markWrittenRecordsSynced( settled.map(({ record }) => record.uuid), ); + // The daemon-stop signal the caller returns. The headline derives its + // daemon-aware clauses from the same (abortReason, autoSyncEnabled), so the + // message can never claim a stop — or a "next run" — that won't happen. + const stoppingAutoSync = permanentlyFailed && autoSyncEnabled; + if (pending.length > 0) { - // Records the run never reached: an abort/timeout stops before later chunks, - // so their outcome index is undefined and they have no per-record outcome. - const unattemptedCount = writtenRecords.length - outcomes.length; + // An abort leaves a trailing chunk unsent, so `outcomes` is shorter than the + // input — the headline uses this to only claim records were skipped when some + // actually were. + const hasUnattempted = outcomes.length < writtenRecords.length; + // Compose the headline here — this function holds the abort reason and the + // daemon state — so reportMarkFailures takes a ready string instead of + // drilling several adjacent, transposable args. + const headline = markFailureHeadline(pending.length, abortReason, { + autoSyncEnabled, + hasUnattempted, + }); reportMarkFailures( pending, writtenRecords.length - pending.length, - { reason: stoppedBy, unattemptedCount }, + headline, spinner, ); - return; + return stoppingAutoSync; } spinner.success(`Marked ${writtenRecords.length} records synced!`); + // Return the same stop signal from both exits so the daemon-stop decision has a + // single source. With zero pending this is always false (a permanent abort maps + // its chunk to MARK_FAILED, so it can't reach here), but deriving it rather than + // hardcoding keeps the two paths from drifting. + return stoppingAutoSync; } // Ends a truncated sync on the truncation warning, never on a green success @@ -902,13 +946,18 @@ async function runDefaultSync(dryRun = false): Promise { // next run's pending-only fetch skips them instead of re-writing duplicate // files. if (!autoDelete) { - await markWrittenRecordsSynced( + const markStopsAutoSync = await markWrittenRecordsSynced( settleableRecords, spinner, processWrittenState, + { autoSyncEnabled: autoSync }, ); reportIncompleteSync(recordsResult.partial); - return autoSync; + // A permanent mark-synced failure (dead token / forbidden account) recurs + // every pass, so — like the delete path — stop the autoSync daemon instead + // of rescheduling into the same doomed PATCH; a transient one keeps + // autoSync alive to retry next pass. + return markStopsAutoSync ? false : autoSync; } // Delete Records — skipped when nothing is settleable (a bare DELETE with an @@ -939,8 +988,7 @@ async function runDefaultSync(dryRun = false): Promise { const deleteMeta = await deleteRecords( settleableRecords.map(({ record }) => record.uuid), ).catch((error: unknown) => { - deletePermanentlyFailed = - isSystemicApiFailure(error) && error.isPermanent; + deletePermanentlyFailed = isPermanentApiFailure(error); // Sanitize before printing, same threat as the outer catch: a // server- or API-derived message can embed an escape. console.error( @@ -999,7 +1047,9 @@ async function runDefaultSync(dryRun = false): Promise { // A permanent failure (dead token, forbidden account) won't clear on // retry — stop the autoSync daemon. A transient one (rate-limit/5xx) is // worth another pass, so keep autoSync alive to retry ("retry shortly"). - return error.isPermanent ? false : autoSync; + // Go through the shared guard (like the mark-synced and delete paths) so + // the permanence rule stays in the API seam, not re-derived here. + return isPermanentApiFailure(error) ? false : autoSync; } spinner.error('Something went wrong!'); diff --git a/src/libs/api.ts b/src/libs/api.ts index 1eea4e5..eed653c 100644 --- a/src/libs/api.ts +++ b/src/libs/api.ts @@ -194,6 +194,16 @@ export const isSystemicApiFailure = ( ): error is ApiRequestError => error instanceof ApiRequestError && error.isSystemic; +// Narrowing guard: true only for a PERMANENT failure (a dead token / forbidden +// account) that won't clear on a blind retry. Keeps the permanence rule inside +// the API seam so callers deciding whether to stop an autoSync daemon (the +// sync's mark-synced and delete paths) don't re-derive it. `isPermanent` already +// implies systemic, so no separate systemic check is needed. +export const isPermanentApiFailure = ( + error: unknown, +): error is ApiRequestError => + error instanceof ApiRequestError && error.isPermanent; + // Narrowing guard: true only for a request-shape `ApiRequestError` (a 400/422 // rejection — NOT a per-record 404, an auth 401/403, or a transient 429). Lets a // bulk caller TAG the outcome so it can decide, after seeing a SECOND chunk agree diff --git a/src/libs/records.ts b/src/libs/records.ts index d4cc120..2194607 100644 --- a/src/libs/records.ts +++ b/src/libs/records.ts @@ -2,6 +2,7 @@ import { ApiTimeoutError, authedRequest, isFatalRequestError, + isPermanentApiFailure, isSystemicApiFailure, logApiFailure, unwrapResourceAttributes, @@ -42,6 +43,13 @@ const SYNCED_STATUS = 'synced'; // synced the run aborts rather than fire the same doomed request again (a lone // rejection isn't enough — see `markRecordsSynced`). // +// Whether an abort ALSO stops the autoSync daemon is a run-level decision, not a +// per-record tag: a permanent systemic failure (dead token / forbidden account: +// 401/403) recurs every pass, so the run reports `abortReason: 'permanent'` (see +// `MarkAbortReason`) and the caller shuts the daemon down. A transient systemic +// failure (429/5xx) still aborts the remaining chunks but keeps the daemon alive +// to retry next pass. Either way the affected records stay `MARK_FAILED`. +// // Values are prefixed (`mark-*`) so they never collide with the wire // `SYNCED_STATUS = 'synced'` above: these are internal outcome tags, not the // status string sent to the server, and an accidental cross-comparison should @@ -57,10 +65,6 @@ export type MarkSyncedOutcome = | typeof MARK_TIMED_OUT | typeof MARK_ABORTED; -// Why a mark-synced run stopped early (a hung server or a categorically wrong -// request), or null if every record was attempted. -export type MarkSyncedStop = typeof MARK_TIMED_OUT | typeof MARK_ABORTED | null; - // markpost paginates with a cursor: each response's `links.next` embeds the // `page[after]` cursor to request the following page, and is `null` once // `meta.hasMore` is false. Extracting it from the link (rather than @@ -394,19 +398,33 @@ export type MarkSyncedItem = { filePath: string; }; +// Why a mark-synced run stopped early, or `null` if it ran every chunk. One +// discriminant (not adjacent booleans) so the reason can't be self-contradictory. +// `'timeout'` — a chunk hit the request timeout (hung server), retried next pass. +// `'permanent'` — a chunk hit a permanent systemic failure (dead token / forbidden +// account: 401/403) that recurs every pass, so the caller ALSO stops the autoSync +// daemon. `'transient'` — a chunk hit a transient systemic failure (429/5xx): the +// run stops early to back off (trailing chunks skipped) but the daemon stays alive +// to retry, and the caller can say the run stopped short. `'request-shape'` — two +// consecutive chunks were rejected with the SAME request-shape 4xx (400/422) and +// nothing had synced, so the payload envelope itself is wrong: the run aborts (its +// stopping chunk re-tagged `MARK_ABORTED`) but the daemon stays alive, since the +// next pass may build a valid request. `null` — the run completed every chunk (any +// failures were per-chunk, not systemic). +export type MarkAbortReason = + 'timeout' | 'permanent' | 'transient' | 'request-shape' | null; + // Outcome of a whole bulk mark-synced run. `outcomes` holds one entry per // record in the ORIGINAL input order, so the caller can align each result back // to its record by index. On an abort (a timeout, a systemic auth/rate-limit/ -// 5xx failure, or a request-shape 4xx) it's shorter than the input: the chunk -// that aborted and every chunk after it were never confirmed, so those trailing -// records have no outcome and the caller treats them as still pending. -// `stoppedBy` records which distinctly-reported reason ended the run early — a -// timeout (`MARK_TIMED_OUT`) or a wholesale request-shape rejection -// (`MARK_ABORTED`) — or null when the run finished or stopped on a systemic -// failure (reported as a plain mark failure), so the caller can word its report. +// 5xx failure, or a repeated request-shape 4xx) it's shorter than the input: the +// chunk that aborted and every chunk after it were never confirmed, so those +// trailing records have no outcome and the caller treats them as still pending. +// `abortReason` records why (if) the run stopped early — the caller words its +// report from it and, in particular, stops the daemon only on `'permanent'`. export type MarkSyncedResult = { outcomes: MarkSyncedOutcome[]; - stoppedBy: MarkSyncedStop; + abortReason: MarkAbortReason; }; // The `records[]` item markpost's bulk PATCH expects: the uuid to match plus @@ -466,31 +484,23 @@ const outcomesFromResponse = ( ); }; -// How a chunk ended, from the chunk's OWN perspective — the run-level decision to -// stop is `markRecordsSynced`'s, which also weighs prior chunks. `STOP_TIMEOUT` -// (hung server) and `STOP_SYSTEMIC` (auth/rate-limit/5xx) each doom every -// remaining chunk, so the caller aborts immediately. `STOP_REQUEST_SHAPE` (a -// 400/422) means the payload envelope looks wrong, but the caller only aborts -// once a SECOND consecutive chunk is rejected with the SAME error (see -// `markRecordsSynced`) rather than strand records behind a single, possibly -// isolated rejection. `null` is a clean chunk or a plain per-chunk failure the -// caller runs past. Named constants (not bare literals) so the discriminant a -// third function might compare against can't silently drift on a typo. -const STOP_TIMEOUT = 'timeout'; -const STOP_SYSTEMIC = 'systemic'; -const STOP_REQUEST_SHAPE = 'request-shape'; -type ChunkStop = - typeof STOP_TIMEOUT | typeof STOP_SYSTEMIC | typeof STOP_REQUEST_SHAPE | null; - -// The result of PATCHing one chunk: a per-item outcome list, how the chunk ended, -// and (for a `request-shape` stop only) the server's error message. The caller -// compares that message across chunks so a categorical envelope rejection (the -// same message twice) aborts, while two different per-record rejections that only -// happen to both 4xx do not — it keeps running past those. Null for every other -// stop kind. +// The result of PATCHing one chunk, from the chunk's OWN perspective: a per-item +// outcome list, why (if) the chunk failed in a way that bears on the run, and (for +// a `'request-shape'` failure only) the server's error message. `abortReason` +// carries WHY: `'timeout'` and `'permanent'` distinguish the hung-server and +// dead-token cases (the latter also stops the daemon), `'transient'` a systemic +// 429/5xx (aborts this run, daemon lives), and `'request-shape'` a 400/422 whose +// envelope looks wrong. `'timeout' | 'permanent' | 'transient'` each doom every +// remaining chunk, so `markRecordsSynced` aborts on them immediately; a lone +// `'request-shape'` does NOT — the run aborts only once a SECOND consecutive chunk +// is rejected with the SAME `message` (see `markRecordsSynced`), so `message` lets +// the caller compare a categorical envelope rejection (same message twice) against +// two isolated per-record rejections that only happen to both 4xx. `null` is the +// plain success/per-chunk-failure case that doesn't abort. `message` is null for +// every non-request-shape result. type MarkSyncedChunkResult = { outcomes: MarkSyncedOutcome[]; - stop: ChunkStop; + abortReason: MarkAbortReason; message: string | null; }; @@ -503,21 +513,25 @@ type MarkSyncedChunkResult = { // is non-critical post-write bookkeeping (the files are already on disk), so a // failed chunk simply leaves its records `pending` to re-sync next run. // -// A timeout maps every item to `MARK_TIMED_OUT` and reports `STOP_TIMEOUT` (a -// hung server would burn the full request timeout on every remaining chunk). A -// request-shape 4xx (a malformed-payload 400 or a contract-validation 422) maps -// every item to `MARK_FAILED` and reports `STOP_REQUEST_SHAPE` plus the server's -// error message: the chunk was attempted and rejected, so its records are a plain -// failure UNLESS the run actually aborts — `markRecordsSynced` re-tags only the -// chunk it stops on to `MARK_ABORTED`, so a completed run never leaves a stray -// `MARK_ABORTED`. A systemic failure (auth/rate-limit/5xx) reports `STOP_SYSTEMIC` -// — it will recur for every remaining chunk, so the caller backs off rather than -// hammering a server that just rejected the burst (the same rule the fetch helpers -// apply via `isSystemicApiFailure`) — but stays `MARK_FAILED`, reported as a plain -// failure. Any other (per-chunk) failure maps to `MARK_FAILED` with `stop: null` — -// a later chunk may still succeed. A 4xx delivered as an HTML error page (a -// WAF/proxy interstitial) throws unparseable before it can be classified, so it -// degrades to that plain failure rather than aborting on a misread status. +// A timeout maps every item to `MARK_TIMED_OUT` and reports `abortReason: +// 'timeout'` (a hung server would burn the full request timeout on every +// remaining chunk). A PERMANENT systemic failure (dead token / forbidden account: +// 401/403) maps every item to `MARK_FAILED` and reports `'permanent'` so the +// caller aborts AND additionally stops the autoSync daemon, which can't clear it +// on retry (matching the delete path). A transient systemic failure (rate-limit/ +// 5xx) reports `'transient'` — the caller aborts to back off rather than hammering +// a server that just rejected the burst (the same rule the fetch helpers apply via +// `isSystemicApiFailure`), but keeps the daemon alive. A request-shape 4xx (a +// malformed-payload 400 or a contract-validation 422) maps every item to +// `MARK_FAILED` and reports `'request-shape'` plus the server's error message: the +// chunk was attempted and rejected, so its records are a plain failure UNLESS the +// run actually aborts — `markRecordsSynced` aborts only on a SECOND consecutive +// same-message rejection and re-tags just the chunk it stops on to `MARK_ABORTED`, +// so a completed run never leaves a stray `MARK_ABORTED`. Any other (per-chunk) +// failure maps to `MARK_FAILED` with `abortReason: null` — a later chunk may still +// succeed. A 4xx delivered as an HTML error page (a WAF/proxy interstitial) throws +// unparseable before it can be classified, so it degrades to that plain failure +// rather than aborting on a misread status. const markSyncedChunk = async ( items: MarkSyncedItem[], syncedAt: string, @@ -542,7 +556,7 @@ const markSyncedChunk = async ( return { outcomes: outcomesFromResponse(items, body), - stop: null, + abortReason: null, message: null, }; } catch (error) { @@ -556,32 +570,53 @@ const markSyncedChunk = async ( error instanceof Error ? error.message : String(error), ); + // A timeout gets its own outcome and aborts so the caller doesn't pay the + // full request timeout on every remaining chunk. if (error instanceof ApiTimeoutError) { return { outcomes: items.map(() => MARK_TIMED_OUT), - stop: STOP_TIMEOUT, + abortReason: 'timeout', message: null, }; } - // A request-shape 4xx and any other (per-chunk/systemic) failure both leave - // the whole chunk pending, so they share this outcome list; only the stop - // classification differs. + // Every failed record in the chunk stays `MARK_FAILED` (pending, retried next + // run); only the `abortReason` classification differs. A PERMANENT failure + // (dead token / forbidden account: 401/403) reports `'permanent'` and also + // stops the daemon. A transient systemic failure (rate-limit/5xx that may be a + // blip) reports `'transient'` to back off — a sustained 429 stops after the + // first chunk rather than firing the whole burst — but keeps the daemon alive. + // A request-shape 4xx (400/422) reports `'request-shape'` plus the server's + // message so the caller can abort only on a SECOND consecutive same-message + // rejection. A plain per-chunk failure is `null` and doesn't abort, since a + // later chunk may still succeed. const failedOutcomes: MarkSyncedOutcome[] = items.map(() => MARK_FAILED); + if (isPermanentApiFailure(error)) { + return { + outcomes: failedOutcomes, + abortReason: 'permanent', + message: null, + }; + } + + if (isSystemicApiFailure(error)) { + return { + outcomes: failedOutcomes, + abortReason: 'transient', + message: null, + }; + } + if (isFatalRequestError(error)) { return { outcomes: failedOutcomes, - stop: STOP_REQUEST_SHAPE, + abortReason: 'request-shape', message: error.message, }; } - return { - outcomes: failedOutcomes, - stop: isSystemicApiFailure(error) ? STOP_SYSTEMIC : null, - message: null, - }; + return { outcomes: failedOutcomes, abortReason: null, message: null }; } }; @@ -626,9 +661,9 @@ const withAbortedTail = ( // strand syncable records behind an unconfirmed abort. On any stop the trailing // records get no outcome and stay `pending` (their outcome index is `undefined`, // which the caller reads as not-synced). A plain per-chunk failure doesn't abort — -// a later chunk may still succeed. `stoppedBy` names the distinctly-reported stop -// reason (`MARK_TIMED_OUT` or `MARK_ABORTED`) or is null when the run finished or -// stopped on a systemic failure, so the caller can word its report accordingly. +// a later chunk may still succeed. `abortReason` carries why (if) the run stopped +// early — the caller words its report from it and stops the autoSync daemon only +// on `'permanent'` (see `MarkAbortReason`). export const markRecordsSynced = async ( items: MarkSyncedItem[], syncedAt: string = new Date().toISOString(), @@ -645,22 +680,26 @@ export const markRecordsSynced = async ( const chunk = items.slice(start, start + MAX_MARK_SYNCED_BATCH_SIZE); const { outcomes: chunkOutcomes, - stop, + abortReason, message, } = await markSyncedChunk(chunk, syncedAt); outcomes.push(...chunkOutcomes); anySynced = anySynced || chunkOutcomes.includes(MARK_SYNCED); - if (stop === STOP_TIMEOUT) { - return { outcomes, stoppedBy: MARK_TIMED_OUT }; - } - - if (stop === STOP_SYSTEMIC) { - return { outcomes, stoppedBy: null }; + // A timeout or a systemic failure (permanent or transient) dooms every + // remaining chunk, so abort here and surface why, leaving the trailing chunks + // unsent (their records get no outcome, read as pending). Only `'permanent'` + // additionally stops the daemon; that decision lives in the caller. + if ( + abortReason === 'timeout' || + abortReason === 'permanent' || + abortReason === 'transient' + ) { + return { outcomes, abortReason }; } - if (stop !== STOP_REQUEST_SHAPE) { + if (abortReason !== 'request-shape') { // Reset so the match below stays CONSECUTIVE: a clean or plain-failure // chunk between two identical rejections breaks the "envelope is wrong" // evidence, so it must not count toward the two-in-a-row abort. @@ -679,14 +718,14 @@ export const markRecordsSynced = async ( if (!anySynced && message !== null && message === lastRequestShapeMessage) { return { outcomes: withAbortedTail(outcomes, chunkOutcomes.length), - stoppedBy: MARK_ABORTED, + abortReason: 'request-shape', }; } lastRequestShapeMessage = message; } - return { outcomes, stoppedBy: null }; + return { outcomes, abortReason: null }; }; export const fetchRecord = async (uuid: string): Promise => { diff --git a/tests/index.test.ts b/tests/index.test.ts index 291e9d1..a69bc81 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -82,16 +82,17 @@ const mockRecord: Record = { // (chunk boundaries and the timeout abort are covered in tests/libs/records.test.ts). // `markResultBy` maps each item's uuid to an outcome; `markResultAll` is the // common "every record shares one outcome" shorthand. Both always report every -// record attempted (`stoppedBy: null`, full-length outcomes) — an abort produces -// a SHORTER outcomes array, so timeout/abort cases use an explicit -// `mockResolvedValue({ outcomes: [...], stoppedBy: MARK_TIMED_OUT })` instead. +// record attempted (`abortReason: null`, full-length outcomes) — an abort produces +// a SHORTER outcomes array, so timeout/permanent/request-shape abort cases use an +// explicit `mockResolvedValue({ outcomes: [...], abortReason: 'timeout' | +// 'permanent' | 'request-shape' })` instead of these. const markResultBy = (outcomeFor: (uuid: string) => MarkSyncedOutcome) => async ( items: { uuid: string; filePath: string }[], ): Promise => ({ outcomes: items.map((item) => outcomeFor(item.uuid)), - stoppedBy: null, + abortReason: null, }); const markResultAll = (outcome: MarkSyncedOutcome) => @@ -1311,7 +1312,7 @@ describe('index', () => { // outcome). The short outcomes array models the real timeout abort. vi.mocked(markRecordsSynced).mockResolvedValue({ outcomes: [MARK_SYNCED, MARK_TIMED_OUT], - stoppedBy: MARK_TIMED_OUT, + abortReason: 'timeout', }); await import('@/index.js'); @@ -1363,10 +1364,10 @@ describe('index', () => { ); // uuid-0 synced, uuid-1's chunk was rejected as a request-shape 4xx (abort), // uuid-2 never attempted (no outcome) — the short outcomes array models the - // real abort, and stoppedBy drives the abort-specific headline. + // real abort, and abortReason drives the abort-specific headline. vi.mocked(markRecordsSynced).mockResolvedValue({ outcomes: [MARK_SYNCED, MARK_ABORTED], - stoppedBy: MARK_ABORTED, + abortReason: 'request-shape', }); await import('@/index.js'); @@ -1429,7 +1430,7 @@ describe('index', () => { // attempted." vi.mocked(markRecordsSynced).mockResolvedValue({ outcomes: [MARK_ABORTED, MARK_ABORTED], - stoppedBy: MARK_ABORTED, + abortReason: 'request-shape', }); await import('@/index.js'); @@ -1469,12 +1470,12 @@ describe('index', () => { vi.mocked(writeMarkdown).mockImplementation( (record: Record) => `/mock/output/${record.uuid}.md`, ); - // A systemic abort (auth/rate-limit/5xx) has no distinct stop reason - // (stoppedBy null) but stops early: only uuid-0 was attempted, so uuid-1 and - // uuid-2 were never sent and the generic headline must say so. + // A transient systemic abort (rate-limit/5xx) stops early: only uuid-0 was + // attempted, so uuid-1 and uuid-2 were never sent and the transient headline + // must say some were skipped. vi.mocked(markRecordsSynced).mockResolvedValue({ outcomes: [MARK_FAILED], - stoppedBy: null, + abortReason: 'transient', }); await import('@/index.js'); @@ -1483,7 +1484,7 @@ describe('index', () => { expect.stringContaining('Failed to mark 3 record(s) synced'), ); expect(mockSpinner.error).toHaveBeenCalledWith( - expect.stringContaining('2 never attempted'), + expect.stringContaining('so some were never attempted'), ); expect(process.exitCode).toBe(1); }); @@ -1568,7 +1569,7 @@ describe('index', () => { // only the timed-out one is pending — no never-attempted tail. vi.mocked(markRecordsSynced).mockResolvedValue({ outcomes: [MARK_SYNCED, MARK_TIMED_OUT], - stoppedBy: MARK_TIMED_OUT, + abortReason: 'timeout', }); await import('@/index.js'); @@ -1615,7 +1616,7 @@ describe('index', () => { // All three non-synced records are pending and the run uses timeout wording. vi.mocked(markRecordsSynced).mockResolvedValue({ outcomes: [MARK_SYNCED, MARK_FAILED, MARK_TIMED_OUT], - stoppedBy: MARK_TIMED_OUT, + abortReason: 'timeout', }); await import('@/index.js'); @@ -1921,6 +1922,249 @@ describe('index', () => { expect(scheduledAutoSync).toBe(true); }); + // Drives a mark-sync (autoDelete off) of `count` records where the bulk + // `markRecordsSynced` resolves the given `result`, capturing what the run + // reports back to the scheduler. The chunking + abort itself lives in the + // records lib (covered in tests/libs/records.test.ts); here we pin index's own + // job — turning a `MarkSyncedResult` into the settle/report and the daemon-stop + // decision. A `result` with a SHORTER `outcomes` array than `count` models the + // real abort, where trailing chunks were never sent. + const arrangeMarkSync = async ({ + count, + result, + autoSync, + }: { + count: number; + result: MarkSyncedResult; + autoSync: boolean; + }): Promise<{ scheduledAutoSync: boolean | undefined }> => { + const records: Record[] = Array.from({ length: count }, (_item, index) => ({ + uuid: `uuid-${index}`, + title: `Title ${index}`, + content: `Content ${index}`, + createdAt: '2024-01-01T00:00:00Z', + })); + const { fetchAllRecords, markRecordsSynced } = await import( + '@/libs/records.js' + ); + const { writeMarkdown } = await import('@/libs/markdown.js'); + const { fetchSettings } = await import('@/libs/settings.js'); + const { runSyncWithAutoSchedule } = await import('@/libs/scheduler.js'); + const { default: yoctoSpinner } = await import('yocto-spinner'); + + const capture: { scheduledAutoSync: boolean | undefined } = { + scheduledAutoSync: undefined, + }; + vi.mocked(runSyncWithAutoSchedule).mockImplementationOnce( + async (runSync) => { + capture.scheduledAutoSync = await runSync(); + }, + ); + vi.mocked(yoctoSpinner).mockReturnValue(mockSpinner); + vi.mocked(fetchSettings).mockResolvedValue( + mockSettings({ autoDelete: false, autoSync }), + ); + vi.mocked(fetchAllRecords).mockResolvedValue({ + ok: true, + records, + partial: false, + }); + vi.mocked(writeMarkdown).mockImplementation( + (record: Record) => `/mock/output/${record.uuid}.md`, + ); + vi.mocked(markRecordsSynced).mockResolvedValue(result); + + return capture; + }; + + // The mark-synced counterpart of the delete-permanent-failure test: with + // autoDelete off, a permanent mark-synced failure (dead token / forbidden + // account) surfaces as `abortReason: 'permanent'`. The sync must fail loud AND + // stop rescheduling the autoSync daemon — otherwise it wakes every few minutes + // and re-PATCHes the same records against a server it already knows will + // reject, re-writing them as duplicates every pass (issue #133). + it('stops autoSync from rescheduling on a permanent mark-synced failure', async () => { + const capture = await arrangeMarkSync({ + count: 1, + autoSync: true, + result: { outcomes: [MARK_FAILED], abortReason: 'permanent' }, + }); + + await import('@/index.js'); + + expect(mockSpinner.error).toHaveBeenCalledWith( + expect.stringContaining('auto-sync was stopped'), + ); + expect(process.exitCode).toBe(1); + // A permanent failure recurs, so the scheduler must not spin another pass. + expect(capture.scheduledAutoSync).toBe(false); + }); + + // A per-chunk NON-systemic failure (a per-record MARK_FAILED inside an + // otherwise-successful response, or a non-systemic 4xx) is `abortReason: null`: + // it doesn't abort and must keep autoSync alive — the record is still pending + // and the next pass retries. The systemic-blip (5xx/429) case now maps to + // `'transient'`, covered by the test below. + it('keeps autoSync alive after a per-chunk (non-systemic) mark-synced failure', async () => { + const capture = await arrangeMarkSync({ + count: 1, + autoSync: true, + result: { outcomes: [MARK_FAILED], abortReason: null }, + }); + + await import('@/index.js'); + + // Match text unique to the generic headline — 'still pending on the server' + // alone also appears in the timeout headline, so it wouldn't catch a + // failure wrongly routed through the timeout branch. + expect(mockSpinner.error).toHaveBeenCalledWith( + expect.stringContaining('written locally but still pending on the server'), + ); + expect(process.exitCode).toBe(1); + // A non-aborting failure: the daemon must retry, so autoSync is preserved. + expect(capture.scheduledAutoSync).toBe(true); + }); + + // The other side of the discriminant: a TIMEOUT abort must NOT stop the daemon + // — the server may un-hang, so the next pass should retry. Guards against a + // regression that treats any abort reason as a stop. The short outcomes array + // (2 outcomes for 3 records) models the real abort leaving a trailing chunk + // unsent. + it('keeps autoSync alive on a timeout abort', async () => { + const capture = await arrangeMarkSync({ + count: 3, + autoSync: true, + result: { + outcomes: [MARK_SYNCED, MARK_TIMED_OUT], + abortReason: 'timeout', + }, + }); + + await import('@/index.js'); + + expect(mockSpinner.error).toHaveBeenCalledWith( + expect.stringContaining('Timed out marking records synced'), + ); + expect(process.exitCode).toBe(1); + // A timeout can clear, so the daemon must retry next pass. + expect(capture.scheduledAutoSync).toBe(true); + }); + + // A TRANSIENT systemic abort (a 429/5xx that stopped the run to back off) must + // read differently from a scatter of per-record failures — some records were + // never attempted — yet keep the daemon alive to retry. The short outcomes + // array models the aborted run leaving a trailing chunk unsent. + it('reports a transient abort as stopped-early and keeps autoSync alive', async () => { + const capture = await arrangeMarkSync({ + count: 3, + autoSync: true, + result: { outcomes: [MARK_SYNCED, MARK_FAILED], abortReason: 'transient' }, + }); + + await import('@/index.js'); + + // uuid-1 failed plus uuid-2 never attempted = two pending, with the + // stopped-early wording (not the generic per-record failure line). + const headline = vi + .mocked(mockSpinner.error) + .mock.calls.map(([message]) => String(message)) + .find((message) => message.includes('a systemic error stopped the run early')); + expect(headline).toBeDefined(); + expect(headline).toContain('2 record(s)'); + // A trailing chunk was unsent, so the "never attempted" clause appears. + expect(headline).toContain('so some were never attempted'); + expect(process.exitCode).toBe(1); + // A transient error can clear, so the daemon must retry next pass. + expect(capture.scheduledAutoSync).toBe(true); + }); + + // The transient headline's "never attempted" clause is conditional: when the + // abort lands on the LAST chunk (outcomes length == count, no unattempted tail), + // it must NOT claim records were skipped — the same false-claim guard the + // permanent branch has for its daemon clause. + it('does not claim records were skipped on a transient abort with no unattempted tail', async () => { + await arrangeMarkSync({ + count: 2, + autoSync: true, + result: { outcomes: [MARK_SYNCED, MARK_FAILED], abortReason: 'transient' }, + }); + + await import('@/index.js'); + + const headline = vi + .mocked(mockSpinner.error) + .mock.calls.map(([message]) => String(message)) + .find((message) => message.includes('a systemic error stopped the run early')); + expect(headline).toBeDefined(); + // Only uuid-1 is pending, and it was attempted — no skipped tail to claim. + expect(headline).toContain('1 record(s)'); + expect(headline).not.toContain('never attempted'); + expect(process.exitCode).toBe(1); + }); + + // A permanent mark-synced failure on a plain one-shot `markpost sync` + // (autoDelete off, autoSync off) must NOT claim "auto-sync was stopped" — there + // was no daemon. The headline still guides the user to fix the cause, but a + // cron log must not read a false statement about what the tool did. + it('does not claim auto-sync was stopped when it was never on', async () => { + await arrangeMarkSync({ + count: 1, + autoSync: false, + result: { outcomes: [MARK_FAILED], abortReason: 'permanent' }, + }); + + await import('@/index.js'); + + const headline = vi + .mocked(mockSpinner.error) + .mock.calls.map(([message]) => String(message)) + .find((message) => message.includes('a permanent error')); + expect(headline).toBeDefined(); + expect(headline).not.toContain('auto-sync was stopped'); + expect(process.exitCode).toBe(1); + }); + + // Guards index's `outcomes[index]` alignment when a permanent abort leaves a + // trailing tail unattended. The bulk call is mocked to return a deliberately + // short outcomes array (20 entries for 25 records) standing in for an abort — + // chunk boundaries themselves live in tests/libs/records.test.ts — with uuid-13 + // failed inside it. index must count uuid-13 plus the five never-attempted + // (uuid-20..24) as six pending, list uuid-13 (not a settled record like + // uuid-0), report 19 marked, and stop the daemon. An off-by-one in the settle + // filter would strand the wrong records. + it('counts pending correctly and stops the daemon on a permanent abort', async () => { + const outcomes: MarkSyncedOutcome[] = Array.from( + { length: 20 }, + (_item, index) => (index === 13 ? MARK_FAILED : MARK_SYNCED), + ); + const capture = await arrangeMarkSync({ + count: 25, + autoSync: true, + result: { outcomes, abortReason: 'permanent' }, + }); + + await import('@/index.js'); + + // uuid-13 failed plus the five never-attempted (uuid-20..24) = six pending. + expect(mockSpinner.error).toHaveBeenCalledWith( + expect.stringContaining('Failed to mark 6 record(s) synced'), + ); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('Marked 19 record(s) synced despite the above.'), + ); + // The failed record is listed pending; a settled record (uuid-0) is not — + // proving the outcome/index alignment holds when the outcomes array is + // truncated by an abort. + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining('! uuid-13 -> /mock/output/uuid-13.md'), + ); + expect(console.error).not.toHaveBeenCalledWith( + expect.stringContaining('! uuid-0 -> /mock/output/uuid-0.md'), + ); + expect(capture.scheduledAutoSync).toBe(false); + expect(process.exitCode).toBe(1); + }); + // Tests 1 and 2 share the same arrange: two records where mockRecord's write // throws and the second (def-456) succeeds. Extracted per rule of three so // the two assertions read on their own. The write outcome is keyed off the diff --git a/tests/libs/api.test.ts b/tests/libs/api.test.ts index 8a38cb1..8629f26 100644 --- a/tests/libs/api.test.ts +++ b/tests/libs/api.test.ts @@ -13,6 +13,7 @@ import { getApiToken, getBaseUrl, isFatalRequestError, + isPermanentApiFailure, isSystemicApiFailure, logApiFailure, rethrowIfTimeout, @@ -491,6 +492,32 @@ describe('isSystemicApiFailure', () => { }); }); +describe('isPermanentApiFailure', () => { + // Gates the autoSync daemon shutdown, so its two boundaries matter: a + // permanent 401/403 is true; a systemic-but-transient 429/5xx is false (the + // daemon retries those); and anything that isn't a systemic ApiRequestError is + // false so the caller keeps its per-item handling. + it('is true only for a permanent (auth) ApiRequestError', () => { + expect(isPermanentApiFailure(new ApiRequestError('nope', 401))).toBe(true); + expect(isPermanentApiFailure(new ApiRequestError('nope', 403))).toBe(true); + }); + + it('is false for a transient systemic ApiRequestError (429/5xx)', () => { + expect(isPermanentApiFailure(new ApiRequestError('nope', 429))).toBe(false); + expect(isPermanentApiFailure(new ApiRequestError('nope', 503))).toBe(false); + }); + + it('is false for a non-permanent 4xx ApiRequestError', () => { + expect(isPermanentApiFailure(new ApiRequestError('nope', 422))).toBe(false); + }); + + it('is false for a plain Error or non-error value', () => { + expect(isPermanentApiFailure(new Error('network down'))).toBe(false); + expect(isPermanentApiFailure('boom')).toBe(false); + expect(isPermanentApiFailure(undefined)).toBe(false); + }); +}); + describe('describeSystemicFailure', () => { it('labels an auth failure with its status and message', () => { const error = new ApiRequestError('Invalid or missing token', 401); diff --git a/tests/libs/records.test.ts b/tests/libs/records.test.ts index 84e4989..1bba9da 100644 --- a/tests/libs/records.test.ts +++ b/tests/libs/records.test.ts @@ -1305,7 +1305,7 @@ describe('markRecordsSynced', () => { global.fetch = vi.fn(); const result = await markRecordsSynced([]); expect(global.fetch).not.toHaveBeenCalled(); - expect(result).toEqual({ outcomes: [], stoppedBy: null }); + expect(result).toEqual({ outcomes: [], abortReason: null }); }); it('marks every record synced in a single request at exactly the batch size', async () => { @@ -1318,7 +1318,7 @@ describe('markRecordsSynced', () => { expect(result.outcomes.every((outcome) => outcome === MARK_SYNCED)).toBe( true, ); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBe(null); }); it('splits one-over-the-batch-size into two requests (ceil(N/100))', async () => { @@ -1345,7 +1345,7 @@ describe('markRecordsSynced', () => { true, ); expect(result.outcomes).toHaveLength(250); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBe(null); }); it('pairs each record its own uuid, filePath, and syncedAt across chunks', async () => { @@ -1388,7 +1388,7 @@ describe('markRecordsSynced', () => { MARK_SYNCED, MARK_SYNCED, ]); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBe(null); }); it('aligns a per-record failure to its index when it lands in a later chunk', async () => { @@ -1409,7 +1409,7 @@ describe('markRecordsSynced', () => { index === 120 ? outcome === MARK_FAILED : outcome === MARK_SYNCED, ), ).toBe(true); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBe(null); }); it('aborts remaining chunks on a timeout and marks the timed-out chunk pending', async () => { @@ -1428,7 +1428,7 @@ describe('markRecordsSynced', () => { const result = await markRecordsSynced(items(250)); // Only two requests fire — the third chunk is never attempted. expect(global.fetch).toHaveBeenCalledTimes(2); - expect(result.stoppedBy).toBe(MARK_TIMED_OUT); + expect(result.abortReason).toBe('timeout'); // Chunk 1 synced (100), chunk 2 all timed out (100); chunk 3 has no outcome. expect(result.outcomes).toHaveLength(200); expect( @@ -1457,7 +1457,7 @@ describe('markRecordsSynced', () => { const result = await markRecordsSynced(items(250)); // All three chunks are attempted — no abort. expect(global.fetch).toHaveBeenCalledTimes(3); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBe(null); expect(result.outcomes).toHaveLength(250); expect( result.outcomes.slice(0, 100).every((outcome) => outcome === MARK_FAILED), @@ -1476,7 +1476,7 @@ describe('markRecordsSynced', () => { mockFetch({ data: [], meta: { updated: 0 } }); const result = await markRecordsSynced(items(3)); expect(result.outcomes).toEqual([MARK_FAILED, MARK_FAILED, MARK_FAILED]); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBe(null); }); // Off-contract safety net: the declared contract always sends `data` as an @@ -1486,7 +1486,7 @@ describe('markRecordsSynced', () => { mockFetch({ meta: { updated: 3 } }); const result = await markRecordsSynced(items(3)); expect(result.outcomes).toEqual([MARK_SYNCED, MARK_SYNCED, MARK_SYNCED]); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBe(null); }); // `data: null` is off-contract (markpost always sends the updated array), so @@ -1514,9 +1514,10 @@ describe('markRecordsSynced', () => { expect(result.outcomes).toEqual([MARK_FAILED, MARK_FAILED, MARK_FAILED]); }); - it('aborts remaining chunks on a systemic failure (e.g. 401) without a timeout flag', async () => { - // A 401 (or any auth/rate-limit/5xx) will recur for every remaining chunk, - // so the run backs off after the first rather than hammering the server. + it('aborts remaining chunks and reports permanent on a 401 failure', async () => { + // A 401 (dead token) is a PERMANENT systemic failure: it recurs for every + // remaining chunk and every future pass, so the run backs off after the first + // chunk AND reports `abortReason: 'permanent'` so the caller stops the daemon. global.fetch = vi.fn().mockResolvedValue({ ok: false, status: 401, @@ -1526,8 +1527,8 @@ describe('markRecordsSynced', () => { const result = await markRecordsSynced(items(250)); // Only the first chunk is attempted — the other two are never sent. expect(global.fetch).toHaveBeenCalledTimes(1); - // A systemic abort is not a timeout, so the caller uses the failure wording. - expect(result.stoppedBy).toBeNull(); + // A permanent abort — the caller both fails loud and stops the autoSync daemon. + expect(result.abortReason).toBe('permanent'); // The attempted chunk is all pending; the unsent chunks have no outcome. expect(result.outcomes).toHaveLength(100); expect(result.outcomes.every((outcome) => outcome === MARK_FAILED)).toBe( @@ -1535,6 +1536,27 @@ describe('markRecordsSynced', () => { ); }); + it('aborts remaining chunks and reports transient on a 503 failure', async () => { + // A 503 (or 429) is a TRANSIENT systemic failure: it aborts the remaining + // chunks to back off (it may recur), reported as `abortReason: 'transient'` + // so the caller can say the run stopped early — but it must NOT stop the + // daemon, since a lone 5xx can be a blip and the next pass should retry. + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 503, + json: () => Promise.resolve({}), + }); + + const result = await markRecordsSynced(items(250)); + expect(global.fetch).toHaveBeenCalledTimes(1); + // Aborted the run with a non-permanent reason — the daemon stays alive. + expect(result.abortReason).toBe('transient'); + expect(result.outcomes).toHaveLength(100); + expect(result.outcomes.every((outcome) => outcome === MARK_FAILED)).toBe( + true, + ); + }); + it('marks a whole chunk MARK_FAILED when the request rejects with an error response', async () => { mockFetch( { data: { errors: [{ title: 'Unprocessable', detail: 'bad batch' }] } }, @@ -1542,7 +1564,7 @@ describe('markRecordsSynced', () => { ); const result = await markRecordsSynced(items(3)); expect(result.outcomes).toEqual([MARK_FAILED, MARK_FAILED, MARK_FAILED]); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBe(null); }); it('marks a whole chunk MARK_FAILED on a network failure', async () => { @@ -1560,6 +1582,22 @@ describe('markRecordsSynced', () => { expect(result.outcomes).toEqual([MARK_FAILED]); }); + // A forbidden account (403) is permanent like a dead token (401, covered + // above): it recurs every pass, so the chunk aborts with `abortReason: + // 'permanent'` and the caller stops the autoSync daemon. Its records stay + // MARK_FAILED (pending). The 401 and transient-503 counterparts sit with the + // other chunk-abort tests above. + it('aborts with a permanent reason on a forbidden (403) failure', async () => { + global.fetch = vi.fn().mockResolvedValue({ + ok: false, + status: 403, + json: () => Promise.resolve({ data: { errors: [] } }), + }); + const result = await markRecordsSynced(items(2)); + expect(result.abortReason).toBe('permanent'); + expect(result.outcomes).toEqual([MARK_FAILED, MARK_FAILED]); + }); + // Reject every chunk the given way (a 400/422 error response), so a test can // drive the two-chunk request-shape confirmation without hand-writing the mock. const mockAllChunksReject = (status: number, detail: string) => { @@ -1587,7 +1625,7 @@ describe('markRecordsSynced', () => { const result = await markRecordsSynced(items(250)); // Only two requests fire — the third chunk is never attempted. expect(global.fetch).toHaveBeenCalledTimes(2); - expect(result.stoppedBy).toBe(MARK_ABORTED); + expect(result.abortReason).toBe('request-shape'); // Chunk 1 was run past (MARK_FAILED); only the stopping chunk 2 is MARK_ABORTED. expect(result.outcomes).toHaveLength(200); expect( @@ -1603,7 +1641,7 @@ describe('markRecordsSynced', () => { const result = await markRecordsSynced(items(250)); expect(global.fetch).toHaveBeenCalledTimes(2); - expect(result.stoppedBy).toBe(MARK_ABORTED); + expect(result.abortReason).toBe('request-shape'); expect(result.outcomes).toHaveLength(200); expect( result.outcomes.slice(0, 100).every((outcome) => outcome === MARK_FAILED), @@ -1636,7 +1674,7 @@ describe('markRecordsSynced', () => { // Messages differ per chunk, so no abort — all three chunks are attempted and // rejected as plain per-chunk failures. expect(global.fetch).toHaveBeenCalledTimes(3); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBeNull(); expect(result.outcomes).toHaveLength(250); expect(result.outcomes.every((outcome) => outcome === MARK_FAILED)).toBe( true, @@ -1667,7 +1705,7 @@ describe('markRecordsSynced', () => { // Chunk 2 (network error) resets the consecutiveness, so chunk 3 doesn't // confirm chunk 1 — all three fire and nothing aborts. expect(global.fetch).toHaveBeenCalledTimes(3); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBeNull(); expect(result.outcomes).toHaveLength(250); expect(result.outcomes.every((outcome) => outcome === MARK_FAILED)).toBe( true, @@ -1699,7 +1737,7 @@ describe('markRecordsSynced', () => { // All three chunks are attempted — one rejection doesn't abort, and the run // completes, so chunk 1 stays a plain MARK_FAILED (never MARK_ABORTED). expect(global.fetch).toHaveBeenCalledTimes(3); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBeNull(); expect( result.outcomes.slice(0, 100).every((outcome) => outcome === MARK_FAILED), ).toBe(true); @@ -1733,7 +1771,7 @@ describe('markRecordsSynced', () => { // Chunk 1 synced, so chunks 2 and 3 both fire despite being rejected, and the // run completes — the rejected chunks stay plain MARK_FAILED. expect(global.fetch).toHaveBeenCalledTimes(3); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBeNull(); expect( result.outcomes.slice(0, 100).every((outcome) => outcome === MARK_SYNCED), ).toBe(true); @@ -1768,7 +1806,7 @@ describe('markRecordsSynced', () => { const result = await markRecordsSynced(items(250)); // All three chunks are attempted — a 404 is not a categorical abort. expect(global.fetch).toHaveBeenCalledTimes(3); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBeNull(); expect( result.outcomes.slice(0, 100).every((outcome) => outcome === MARK_FAILED), ).toBe(true); @@ -1778,10 +1816,10 @@ describe('markRecordsSynced', () => { }); // A 429 is systemic (rate-limit): it aborts the run to back off, but is NOT a - // request-shape rejection — it maps to MARK_FAILED with a null stop reason (the - // plain-failure wording), never MARK_ABORTED. Guards the fatal-request abort - // against catching a transient rate-limit. - it('treats a 429 as a systemic abort (MARK_FAILED, not MARK_ABORTED)', async () => { + // request-shape rejection — it maps to MARK_FAILED with `abortReason: 'transient'` + // (the daemon stays alive), never MARK_ABORTED / 'request-shape'. Guards the + // fatal-request abort against catching a transient rate-limit. + it('treats a 429 as a transient systemic abort (MARK_FAILED, not MARK_ABORTED)', async () => { global.fetch = vi.fn().mockResolvedValue({ ok: false, status: 429, @@ -1796,7 +1834,7 @@ describe('markRecordsSynced', () => { const result = await markRecordsSynced(items(250)); // Systemic abort after the first chunk — the other two never fire. expect(global.fetch).toHaveBeenCalledTimes(1); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBe('transient'); expect(result.outcomes).toHaveLength(100); expect(result.outcomes.every((outcome) => outcome === MARK_FAILED)).toBe( true, @@ -1824,7 +1862,7 @@ describe('markRecordsSynced', () => { const result = await markRecordsSynced(items(250)); // The unparseable body degrades to a plain failure, so the run continues. expect(global.fetch).toHaveBeenCalledTimes(3); - expect(result.stoppedBy).toBeNull(); + expect(result.abortReason).toBeNull(); expect( result.outcomes.slice(0, 100).every((outcome) => outcome === MARK_FAILED), ).toBe(true);