Skip to content

Mark records synced via bulk PATCH /api/records #128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
grimicorn merged 2 commits into from
Aug 27, 2026
Merged

Mark records synced via bulk PATCH /api/records #128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8427a68
Mark records synced via bulk PATCH /api/records
grimicorn Aug 23, 2026
cdcb7ed
Apply code-review fixes: systemic abort, response-shape guard, tests
grimicorn Aug 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 14 additions & 52 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
import {
deleteRecords,
fetchAllRecords,
markRecordSynced,
markRecordsSynced,
MARK_SYNCED,
MARK_TIMED_OUT,
MarkSyncedOutcome,
MarkSyncedItem,
PENDING_STATUS,
} from '@/libs/records.js';
import { describeApiError, isSystemicApiFailure } from '@/libs/api.js';
Expand Down Expand Up @@ -50,14 +49,6 @@ import {

type Spinner = ReturnType<typeof yoctoSpinner>;

// Cap how many mark-synced PATCHes are in flight at once. A large first sync
// can write hundreds of records; firing one unbounded `Promise.all` over all
// of them risks rate-limit/connection failures exactly when the batch is
// biggest — and every failed mark stays pending and re-duplicates next run.
// Declared here (above the top-level `dispatch()` call) so the hoisted helpers
// don't hit its temporal dead zone when the default sync runs.
const MARK_SYNCED_CONCURRENCY = 10;

// Slug ownership (resolved `<slug>.md` path -> the uuid that wrote it), shared
// across every autoSync pass in this process rather than rebuilt per pass.
// autoSync deletes each pass's records server-side, so a later pass fetching a
Expand Down Expand Up @@ -430,46 +421,15 @@ function reportDeferredServerChanges(deferredRecords: WrittenRecord[]): void {
});
}

// Outcome of a whole mark-synced run. `outcomes` holds one entry per *attempted*
// record in the original order; on a timeout abort it's shorter than the input
// because the remaining batches were never sent. `timedOut` records whether a
// timeout stopped the run early so the caller can report the abort explicitly.
interface MarkSyncedRun {
outcomes: MarkSyncedOutcome[];
timedOut: boolean;
}

// PATCHes the written records synced in bounded-concurrency batches, stopping on
// the first timeout. A timeout means the server is hung, so firing the remaining
// batches would burn the full request timeout on each one before reporting;
// aborting leaves those records pending to retry next run, mirroring the push
// command's batch-abort. Non-timeout failures don't abort — the next record may
// still succeed.
async function markRecordsInBatches(
writtenRecords: WrittenRecord[],
): Promise<MarkSyncedRun> {
const outcomes: MarkSyncedOutcome[] = [];

for (
let start = 0;
start < writtenRecords.length;
start += MARK_SYNCED_CONCURRENCY
) {
const batch = writtenRecords.slice(start, start + MARK_SYNCED_CONCURRENCY);
const batchOutcomes = await Promise.all(
batch.map(({ record, filePath }) =>
markRecordSynced(record.uuid, filePath),
),
);

outcomes.push(...batchOutcomes);

if (batchOutcomes.includes(MARK_TIMED_OUT)) {
return { outcomes, timedOut: true };
}
}

return { outcomes, timedOut: false };
// 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-timeout abort live in
// `markRecordsSynced` (the records lib), keeping the API surface isolated there.
function toMarkSyncedItems(writtenRecords: WrittenRecord[]): MarkSyncedItem[] {
return writtenRecords.map(({ record, filePath }) => ({
uuid: record.uuid,
filePath,
}));
}

// Headline for the mark-synced failure report. A timeout abort reads
Expand Down Expand Up @@ -542,7 +502,9 @@ async function markWrittenRecordsSynced(

spinner.start('Marking records synced...');

const { outcomes, timedOut } = await markRecordsInBatches(writtenRecords);
const { outcomes, timedOut } = await markRecordsSynced(
toMarkSyncedItems(writtenRecords),
);
// 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 a timeout aborted the run before its batch). Evict settled records
Expand Down
251 changes: 196 additions & 55 deletions src/libs/records.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,15 @@ import {
export const PENDING_STATUS = 'pending';
const SYNCED_STATUS = 'synced';

// Result of marking one record synced. `MARK_SYNCED` — the server accepted the
// PATCH. `MARK_FAILED` — a non-timeout error; the record stays pending and the
// rest of the batch still runs. `MARK_TIMED_OUT` — the PATCH hit the request
// timeout, a signal the server is hung; the batch runner stops on the first one
// rather than paying the full timeout on every remaining record.
// Per-record result of a bulk mark-synced run (see `markRecordsSynced`, which
// PATCHes records in chunks of up to MAX_MARK_SYNCED_BATCH_SIZE). `MARK_SYNCED`
// — the server returned this record among the ones it updated. `MARK_FAILED` —
// the record's chunk failed (a non-timeout error), or the server didn't return
// this uuid (partial success); it stays pending to re-sync next run. A plain
// chunk failure doesn't stop the run, but a systemic one (auth/rate-limit/5xx)
// aborts the remaining chunks — see `markSyncedChunk`. `MARK_TIMED_OUT` — the
// record's chunk hit the request timeout, a signal the server is hung; the run
// stops there rather than paying the full timeout on every remaining chunk.
//
// Values are prefixed (`mark-*`) so they never collide with the wire
// `SYNCED_STATUS = 'synced'` above: these are internal outcome tags, not the
Expand Down Expand Up @@ -360,48 +364,125 @@ export const createRecord = async (
}
};

// Marks a single record synced after the CLI has written it to disk, via
// markpost's PATCH /api/records/[uuid] (server/api/records/[uuid].patch.ts),
// which accepts `status`, `syncedAt`, and `filePath`. This is the
// non-destructive counterpart to `deleteRecords`: with autoDelete off, moving
// the record out of `pending` is what stops the next run's pending-only fetch
// from re-writing it. `syncedAt` is injected (defaulting to now) so callers
// and tests can pin the timestamp. Content-Type mirrors createRecord/
// deleteRecords for consistency; markpost reads the body regardless.
// markpost's bulk update handler (server/api/records/index.patch.ts) caps each
// PATCH /api/records request at this many records (`MAX_UPDATE_BATCH_SIZE`); a
// larger `records[]` array is rejected with a 422. The CLI chunks to this size
// so a first sync of hundreds of records settles in `ceil(N / 100)` requests
// instead of one PATCH per record — the whole point of moving off the per-uuid
// endpoint (issue #123).
export const MAX_MARK_SYNCED_BATCH_SIZE = 100;

// One record the CLI wants marked synced: the uuid to update and the on-disk
// path markpost stores so its UI can show where the note landed.
export type MarkSyncedItem = {
uuid: string;
filePath: string;
};

// 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 OR a systemic auth/rate-limit/
// 5xx failure) 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. `timedOut` is true only when a
// timeout (not a systemic failure) caused the abort, so the caller can word its
// report accordingly.
export type MarkSyncedResult = {
outcomes: MarkSyncedOutcome[];
timedOut: boolean;
};

// The `records[]` item markpost's bulk PATCH expects: the uuid to match plus
// the attributes to set. The CLI always sets `status`, `syncedAt`, and
// `filePath` together — moving a written record out of `pending` so the next
// run's pending-only fetch skips it. `filePath` is sent deliberately: markpost
// stores it on the record so its UI can show where a synced note landed — the
// user's own local path going to their own account, not a third-party leak.
const buildBulkRecordPayload = (item: MarkSyncedItem, syncedAt: string) => {
return {
uuid: item.uuid,
status: SYNCED_STATUS,
syncedAt,
filePath: item.filePath,
};
};

// Reads the `meta.updated` count off a bulk-PATCH response, if present. markpost
// sends it alongside `data` (server/api/records/index.patch.ts); it's the
// corroborating signal used only when `data` itself is unreadable.
const updatedCountFromMeta = (
body: RecordListApiResponse,
): number | undefined => {
const meta = body.meta as { updated?: unknown } | undefined;

return typeof meta?.updated === 'number' ? meta.updated : undefined;
};

// Maps one chunk's request outcome to a per-item result. markpost returns the
// records it actually updated as the `data` collection (always an array;
// foreign/nonexistent uuids are silently dropped, mirroring the bulk delete
// endpoint), so a uuid present there was synced and one absent stays `pending`
// and is reported `MARK_FAILED` — that per-uuid diff is what gives the CLI real
// partial-failure detection across a 100-record chunk, rather than trusting a
// bare 2xx.
//
// Goes through `authedRequest` so the PATCH inherits the same request timeout
// as every other API call (a stalled connection can't hang the sync forever).
// Unlike the fetch helpers above, a failure here is logged rather than
// re-thrown: this is non-critical post-write bookkeeping (the file is already
// on disk), so a failed mark simply leaves the record `pending` to re-sync
// next run, which is far less disruptive than aborting the whole sync after
// files have landed.
// If `data` is ever NOT an array (an off-contract or proxied response the
// declared contract never produces), the per-uuid diff can't run, so fall back
// to the corroborating `meta.updated` count: a full count means the whole chunk
// was accepted (all `MARK_SYNCED`); anything else fails the chunk loud so its
// records retry next run rather than being silently reported synced.
const outcomesFromResponse = (
items: MarkSyncedItem[],
body: RecordListApiResponse,
): MarkSyncedOutcome[] => {
if (!Array.isArray(body.data)) {
const wholeChunkAccepted = updatedCountFromMeta(body) === items.length;

return items.map(() => (wholeChunkAccepted ? MARK_SYNCED : MARK_FAILED));
}

const updated = unwrapResourceCollection('markRecordsSynced', body, 'record');
const updatedUuids = new Set(updated.map((record) => record.uuid));

return items.map((item) =>
updatedUuids.has(item.uuid) ? MARK_SYNCED : MARK_FAILED,
);
};

// The result of PATCHing one chunk: a per-item outcome list plus whether the
// run should stop. `abort` is set when the failure dooms every remaining chunk
// too (a hung server or a systemic auth/rate-limit/5xx failure), so the caller
// stops rather than firing a burst it already knows will fail. `timedOut`
// distinguishes the hung-server case from a systemic one so the caller can word
// its report accordingly.
type MarkSyncedChunkResult = {
outcomes: MarkSyncedOutcome[];
abort: boolean;
timedOut: boolean;
};

// PATCHes one chunk (<= MAX_MARK_SYNCED_BATCH_SIZE records) synced in a single
// bulk request. Routes through the shared `authedRequest` seam (like every
// other call): it attaches the bearer token, asserts success (throwing on a
// non-2xx, an errors-carrying 2xx, or an unparseable body such as an HTML error
// page behind a 200), and inherits the request timeout so a stalled connection
// can't hang the sync forever. A failure here is logged, not re-thrown — this
// 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.
//
// Returns a three-way outcome rather than a bare boolean so the caller can
// tell a per-record failure (`MARK_FAILED`, keep going — the next record may
// succeed) apart from a timeout (`MARK_TIMED_OUT`). A timeout signals a hung
// server, so the caller stops the remaining batches instead of burning the
// full request timeout on every one; the record still stays `pending` either
// way. Reading the body back as a resource would mis-report a legitimate 2xx
// that carries no `data` (markpost's PATCH always returns the record, but a
// `data: null` shape still counts as success here) as a failure, wrongly
// warning the user of duplicates. `filePath` is sent deliberately — markpost
// stores it on the record so its UI can show where a synced note landed; it's
// the user's own local path going to their own account, not a third-party leak.
export const markRecordSynced = async (
uuid: string,
filePath: string,
syncedAt: string = new Date().toISOString(),
): Promise<MarkSyncedOutcome> => {
// A timeout maps every item to `MARK_TIMED_OUT` and aborts (a hung server would
// burn the full request timeout on every remaining chunk). A systemic failure
// (auth/rate-limit/5xx) also aborts — 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`). Any
// other (per-chunk) failure maps to `MARK_FAILED` without aborting — a later
// chunk may still succeed.
const markSyncedChunk = async (
items: MarkSyncedItem[],
syncedAt: string,
): Promise<MarkSyncedChunkResult> => {
try {
// Route through the shared authedRequest seam (like createRecord/
// fetchRecord): it attaches the bearer token and asserts success (throwing
// on a non-2xx or an errors-carrying 2xx, and on an unparseable body such
// as an HTML error page behind a 200), so a failure lands in the catch
// below rather than being mistaken for a silent success that leaves the
// record pending. We ignore the returned body — the caller only needs to
// know the server accepted the change.
await authedRequest(`/api/records/${encodeURIComponent(uuid)}`, {
const body = (await authedRequest('/api/records', {
method: 'PATCH',
headers: {
'Content-Type': 'application/vnd.api+json',
Expand All @@ -410,30 +491,90 @@ export const markRecordSynced = async (
data: {
type: 'records',
attributes: {
status: SYNCED_STATUS,
syncedAt,
filePath,
records: items.map((item) =>
buildBulkRecordPayload(item, syncedAt),
),
},
},
}),
});
})) as RecordListApiResponse;

return MARK_SYNCED;
return {
outcomes: outcomesFromResponse(items, body),
abort: false,
timedOut: false,
};
} catch (error) {
// Identify the chunk by its uuid range so a stderr reader can tell which
// records this failure left pending without cross-referencing the caller's
// own per-record report.
const firstUuid = items[0]?.uuid;
const lastUuid = items[items.length - 1]?.uuid;
logErrorMessage(
`markRecordSynced["${uuid}"]`,
`markRecordsSynced[${firstUuid}..${lastUuid}, ${items.length} record(s)]`,
error instanceof Error ? error.message : String(error),
);

// A timeout gets its own outcome so the caller can abort the remaining
// marks on the first one; every other error just leaves this record
// pending and lets the rest of the batch proceed.
if (error instanceof ApiTimeoutError) {
return MARK_TIMED_OUT;
return {
outcomes: items.map(() => MARK_TIMED_OUT),
abort: true,
timedOut: true,
};
}

return MARK_FAILED;
return {
outcomes: items.map(() => MARK_FAILED),
abort: isSystemicApiFailure(error),
timedOut: false,
};
}
};

// Marks written records synced after the CLI has written them to disk, via
// markpost's bulk PATCH /api/records (server/api/records/index.patch.ts). This
// is the non-destructive counterpart to `deleteRecords`: with autoDelete off,
// moving each record out of `pending` is what stops the next run's pending-only
// fetch from re-writing it. `syncedAt` is injected (defaulting to now) so
// callers and tests can pin the timestamp.
//
// Chunks the input into `ceil(N / MAX_MARK_SYNCED_BATCH_SIZE)` requests so a
// large first sync settles up to 100 records per PATCH instead of one request
// per record — the rate-limit/connection pressure that motivated issue #123.
// Chunks run sequentially (not in parallel): the previous per-record path
// bounded concurrency for exactly this reason, and one request per 100 records
// is already few enough that firing them serially keeps the burst small without
// a concurrency limiter.
//
// Returns one outcome per record in input order. A timeout or a systemic
// failure (auth/rate-limit/5xx) stops the run at that chunk rather than firing a
// burst that's already doomed; 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. `timedOut` is true only when a timeout (not a systemic failure)
// caused the stop, so the caller can word its report accordingly.
export const markRecordsSynced = async (
items: MarkSyncedItem[],
syncedAt: string = new Date().toISOString(),
): Promise<MarkSyncedResult> => {
const outcomes: MarkSyncedOutcome[] = [];

for (
let start = 0;
start < items.length;
start += MAX_MARK_SYNCED_BATCH_SIZE
) {
const chunk = items.slice(start, start + MAX_MARK_SYNCED_BATCH_SIZE);
const chunkResult = await markSyncedChunk(chunk, syncedAt);

outcomes.push(...chunkResult.outcomes);

if (chunkResult.abort) {
return { outcomes, timedOut: chunkResult.timedOut };
}
}

return { outcomes, timedOut: false };
};

export const fetchRecord = async (uuid: string): Promise<Record | null> => {
Expand Down
Loading