Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 5 additions & 1 deletion .ai/specs/2026-08-12-data-sync-run-scoped-cursor.md
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ caller of the fence in-tree; external callers passing a bare number in that slot
| # | Failure scenario | Severity | Affected area | Mitigation | Residual risk |
|---|---|---|---|---|---|
| 1 | A reset deletes the shared row for an opted-out entity type, which does not exist, and the next incremental run resumes from a stale mid-walk cursor — re-importing only the tail | High | Adapter reset flows | `resetResumePosition` ships and the one in-tree reset flow calls it; documented as a MUST in the module `AGENTS.md` and the framework docs | A third-party reset flow that deletes `SyncCursor` directly and does not call it stays exposed. Nothing in code forces the pairing |
| 2 | `resolveResumeCursor` resumes from a `paused` or `cancelled` run whose window differs from what the caller now intends (e.g. a narrowed backfill), so the new run inherits the old run's scan window | Medium | Opted-out entity types | `fullSync` starts from `null`; a `completed` latest run also yields `null`, so only a genuinely interrupted run is inherited | Real. Resuming an interrupted run is the intended behaviour, but "interrupted with a different window" is indistinguishable from "interrupted with the same window" without a window fingerprint on the run row |
| 2 | `resolveResumeCursor` resumes from a `paused` or `cancelled` run whose window differs from what the caller now intends (e.g. a narrowed backfill), so the new run inherits the old run's scan window | Medium | Opted-out entity types | `fullSync` starts from `null`; a `completed` latest run also yields `null`, so only a genuinely interrupted run is inherited. `.ai/specs/2026-08-31-data-sync-cursor-provenance.md` adds `cursorOrigin` + `cursorSourceRunId`, so an adapter can tell an inherited cursor from an explicit one and refuse the former | Reduced to the adapter's own judgement. Provenance makes the two cases distinguishable without a window fingerprint, but core still cannot tell "interrupted with a different window" from "interrupted with the same window" — an adapter that wants that distinction must encode it in its own cursor |
| 3 | An adapter's `persistsSharedCursor` disagrees between the write path (engine) and the read path (start paths) — e.g. two provider-key resolutions drift | Medium | All opted-out entity types | Both paths resolve the adapter through the single `resolveAdapterForIntegration` in `adapter-registry.ts` | Low; a non-deterministic predicate (reading mutable state) could still disagree between calls |
| 4 | An external caller passes a bare number as `commitBatchProgress`'s fifth argument after the fence moved into the options object | Low | External adapters | TypeScript rejects it at compile time | Only untyped/`any` call sites are affected, and they would have to be fencing manually |
| 5 | An opted-out entity type accumulates run rows and `resolveResumeCursor` sorts by `created_at`, which the `SyncRun` index does not cover | Low | Query performance | The leading indexed columns narrow the scan and only one row is fetched per run start | Negligible at realistic run-history sizes; worth revisiting if run retention grows large per entity type |
Expand Down Expand Up @@ -216,6 +216,10 @@ interleave is what this change is about, and the unit tests reproduce it.
## Changelog

- 2026-08-12 — implemented.
- 2026-08-31 — Risk #2's mitigation and residual risk updated: cursor provenance
(`.ai/specs/2026-08-31-data-sync-cursor-provenance.md`) makes an inherited cursor distinguishable
from an explicit one at the adapter, which is a lighter answer than the window fingerprint this
risk originally called for.
- 2026-08-13 — merged `develop`; folded the ownership fence and the shared-cursor flag into one
`CursorCommitOptions` object; added `resetResumePosition` and wired the Akeneo reset flow to it;
deduplicated provider-key resolution into `adapter-registry.ts`; added start-cursor and reset
Expand Down
299 changes: 299 additions & 0 deletions .ai/specs/2026-08-31-data-sync-cursor-provenance.md

Large diffs are not rendered by default.

16 changes: 16 additions & 0 deletions BACKWARD_COMPATIBILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -395,3 +395,19 @@ Issue #3852 removed the non-cryptographic passkey verification shape from `Passk
**Why the deprecation protocol does not apply.** The protocol exists to give downstream authors a bridge release. Here the request shape being removed *is* the vulnerability: both values it compared are disclosed by the server, so a bridge would keep the passkey second factor bypassable for a minor version in both login MFA and sudo step-up. A security fix that leaves the hole open is not a fix.

**Migration path.** Send `startAuthentication()` output as `payload.response`. The first-party `PasskeyChallengeVerify` component already does, so shipped UIs are unaffected. Credentials enrolled through the setup path's client-supplied `publicKey` shortcut are **not** reliably rendered unusable by this change — depending on what the client supplied, such a row holds either a key nobody can sign with or a keypair the enroller controls, and the second kind produces assertions this change accepts. That shortcut is a separate open surface (#5296); operator-facing remediation is in [`UPGRADE_NOTES.md`](UPGRADE_NOTES.md).

## Data Sync Cursor Provenance (2026-08-31)

[`.ai/specs/2026-08-31-data-sync-cursor-provenance.md`](.ai/specs/2026-08-31-data-sync-cursor-provenance.md) tells an adapter where the cursor it is being handed came from, so a cursor that encodes scope can be refused when it was silently inherited from an earlier run without also refusing a legitimate Retry or a queue redelivery. **All changes are additive** and pass the contract-surface checks above:

| Surface | Change | Classification |
|---------|--------|----------------|
| Type interfaces (`StreamImportInput`, `StreamExportInput`) | One new **optional** field each: `cursorOrigin?: CursorOrigin` | ✓ ADDITIVE (category 2, Type interface, optional field) |
| Type interface (`StartDataSyncRunInput`) | Two new **optional** fields: `cursorOrigin?`, `cursorSourceRunId?`. Omitting them yields `'explicit'` when a cursor was supplied and `'none'` when it was not, so both existing out-of-module callers (`sync_excel`, `sync_akeneo`) are unaffected | ✓ ADDITIVE (optional fields with a behaviour-preserving default) |
| Import paths / exports (`@open-mercato/core/modules/data_sync/lib/*`) | New exports: `CursorOrigin` and `ResolvedStartCursor` types, `resolveStartCursorWithOrigin`, `deliveredCursorOrigin` (new file `lib/cursor-origin.ts`), `SyncRunService.resolveResumeCursorWithSource`. Nothing removed or renamed | ✓ ADDITIVE (category 4, new exports) |
| Function signatures (`resolveStartCursor`, `SyncRunService.resolveResumeCursor`) | Unchanged. Both become delegating wrappers over the provenance-aware variants and return exactly what they returned before; `resolveStartCursor` gains a `@deprecated` tag only | ✓ ADDITIVE (category 3, no signature change; deprecation tag carries no runtime effect) |
| Database schema (`sync_runs`) | Two new **nullable, no-default** columns: `cursor_origin text null`, `cursor_source_run_id uuid null`, added by `Migration20260831120000`. No rename, removal, type narrowing or index change; both are metadata-only `ADD COLUMN` operations with no table rewrite | ✓ ADDITIVE (category 8, add new columns) |
| HTTP response shapes (`GET /api/data_sync/runs`, `GET /api/data_sync/runs/[id]`) | Two new nullable fields per run: `cursorOrigin`, `cursorSourceRunId`. No existing field changed, removed or renamed | ✓ ADDITIVE (new response fields) |
| HTTP request shapes (`POST /api/data_sync/run`, `POST /api/data_sync/runs/[id]/retry`) | No change — no zod schema, parameter or status code differs. Both routes persist the origin they already resolved | ✓ n/a |
| Runtime behaviour | Byte-identical cursor resolution on every path. Adapters that ignore `cursorOrigin`, and runs written before the migration (which read `null` and are reported to adapters as an absent field rather than a guessed origin), behave exactly as before | ✓ Behaviour-preserving |
| API route URLs, HTTP methods, event IDs, widget spot IDs, ACL features, DI names, notification IDs, CLI commands, generated files | No change | ✓ n/a |
52 changes: 52 additions & 0 deletions apps/docs/docs/framework/modules/integrations-data-sync.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,58 @@ await em.nativeDelete(SyncCursor, { integrationId, entityType, direction: 'impor
when nothing was interrupted, so it is safe to call unconditionally, including for entity types that
do keep the shared row.

## Cursor provenance

Both resolutions above hand the adapter a position the operator never named. That is the intended
incremental behaviour, and it is invisible: pressing **Run** means *"continue whatever ran last"*.

For a cursor that is purely a position, that is fine. For a cursor that encodes **scope** — filters,
date or id bounds, a dry-run flag — it is not. A fresh, unfiltered run that inherits a failed run's
date window walks only that window and still finishes `completed`, having skipped everything outside
it. And the adapter cannot tell that apart from a Retry that was deliberately told to resume, because
both arrive carrying a cursor and the previous run's parameters.

So the engine says where the cursor came from:

```ts
async *streamImport(input: StreamImportInput) {
if (input.cursorOrigin === 'inherited') {
// This position came from a run nobody in this request asked for. Our cursor pins a scope,
// so continuing from it would silently narrow the walk.
throw new Error('[internal] refusing an inherited cursor; start a full sync instead')
}
// ...
}
```

| `cursorOrigin` | Meaning |
|---|---|
| `none` | No cursor. Start from the beginning. |
| `explicit` | The caller named this cursor — a Retry resuming the previous run's own position, or a provider flow that computed one. |
| `inherited` | Core resolved it from prior state the caller never named: the shared `sync_cursors` row, or the last incomplete run. |
| `self` | This run's own committed progress, handed back after a queue redelivery. |

The field is absent for runs created before provenance shipped, so adapters that ignore it — and
adapters reading older runs — behave exactly as before.

**`self` is why refusing `inherited` is safe.** The engine hands over `run.cursor`, not
`initialCursor`, so once a batch has committed, the position is the adapter's own output whatever the
run started from. A redelivered job would otherwise re-enter the adapter with the run's start-time
label still attached, and an adapter refusing `inherited` would restart from the top on every worker
hiccup. `self` is derived from `batchesCompleted`, so the run row records where the run *started*
while the adapter is told what it is *being handed*.

`inherited` does not say which prior state it came from. `sync_runs.cursor_source_run_id` — returned
as `cursorSourceRunId` by both run read APIs — names the run for the previous-run case and is `null`
when the position came from the shared row. The run detail page uses exactly that split to tell an
operator either *"continued where an earlier run stopped"*, with a link, or *"continued from the
saved incremental position"*.

Adapters aside, any code that starts a run should label what it resolved. Use
`resolveStartCursorWithOrigin` rather than the deprecated `resolveStartCursor`, and pass its
`origin` / `sourceRunId` through to `startDataSyncRun`. A caller that omits them is taken at its
word: `explicit` when it supplied a cursor, `none` when it did not.

## Provider-owned environment preconfiguration

New integration providers should not require manual admin setup after every fresh install when deployment already knows the credentials and defaults. The provider package should own that bootstrap path itself.
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/modules/data_sync/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ packages/core/src/modules/data_sync/
│ ├── id-mapping.ts # External ID ↔ local ID lookup and storage
│ ├── queue.ts # Queue helper for enqueuing sync jobs
│ ├── batch-stream.ts # Drives adapter streams; one root span per batch
│ ├── cursor-origin.ts # Provenance of the cursor being handed to the adapter this delivery
│ ├── start-cursor.ts # Resolves a start position and where it came from
│ ├── run-parameters.ts # Validate/coerce operator run parameters vs. adapter declaration
│ ├── sync-engine.ts # Orchestrates streaming import/export with progress
│ └── sync-run-service.ts # CRUD for SyncRun + cursor management
Expand Down Expand Up @@ -205,6 +207,8 @@ If the sync provider needs bootstrap credentials, mappings, locales, channels, o
- **Shared cursor opt-out**: An adapter returning `persistsSharedCursor(entityType) === false` keeps that entity type's cursor on the run row only — use it for whole-table backfills whose cursor is one run's scan state, not a durable log position. Those entity types resolve an incremental start position from the most recent run (`resolveResumeCursor`) instead of the shared row, and from `null` when that run completed
- **Resetting an opt-out**: A reset flow that deletes the shared `SyncCursor` row MUST also call `syncRunService.resetResumePosition(integrationId, entityType, direction, scope)`. An opted-out entity type has no shared row to delete, so deleting only that leaves the resume position on the last interrupted run and the next incremental run re-imports just the tail of the walk it was reset against. The call is a no-op when nothing is interrupted, so make it unconditionally
- **Resume**: Retry reads the last successful cursor, resumes from there
- **Cursor provenance**: Every run records where its start cursor came from in `sync_runs.cursor_origin`, plus `cursor_source_run_id` when it came from a run. Adapters receive it as `StreamImportInput.cursorOrigin` / `StreamExportInput.cursorOrigin`: `none` (no cursor), `explicit` (the caller named it — a Retry resuming the previous run's own position), `inherited` (core resolved it from the shared row or the last incomplete run), `self` (this run's own committed progress, on a redelivery). An adapter whose cursor encodes SCOPE — filters, date/id bounds, dry-run flags — and not just a position SHOULD refuse `inherited`, because a fresh run inheriting a stranger run's window walks only that window and still reports `completed`. Start paths MUST label what they resolved: use `resolveStartCursorWithOrigin` (not the deprecated `resolveStartCursor`) and pass `cursorOrigin` / `cursorSourceRunId` to `startDataSyncRun`; a caller that omits them gets `explicit` when it supplied a cursor and `none` when it did not
- **Delivered vs stored origin**: the column is the provenance of the run's START cursor and never changes; what the adapter receives is `deliveredCursorOrigin(run)` (`lib/cursor-origin.ts`), which reports `self` once `batchesCompleted > 0`. The engine hands over `run.cursor`, not `initialCursor`, so after the first commit the position is the adapter's own whatever the run started from — without that distinction an adapter refusing `inherited` would reject its own resume on every queue redelivery. `inherited` alone does not say WHICH prior state: read `cursorSourceRunId` (set for the previous-run case, null for the shared `sync_cursors` row)
- **Progress**: Linked to `ProgressJob` via `progressJobId` for `ProgressTopBar` display
- **Cancellation**: The engine polls `progressService.isCancellationRequested()` in the batch handler AND on the heartbeat tick while a batch is still in flight, aborting `StreamImportInput.signal` / `StreamExportInput.signal`. Adapters SHOULD honour the signal wherever the work is divisible (per page, per record, around a long flush) and `return` — with the `return` ABOVE the `yield`, never below it, or the engine commits a cursor for a half-applied page. Adapters that ignore the signal keep the old between-batches behavior.
- **Tracing**: The engine emits one **root** span per batch (`data_sync.import.batch` / `data_sync.export.batch`) linked back to the run, covering the adapter's read *and* the engine's bookkeeping. Adapters MUST NOT hand-roll their own batch span — they cannot root it, so a multi-day run would ride on the single sampling decision taken for the request that triggered it. Inner spans an adapter creates nest under the batch span normally. The final read — the one that finds the stream drained — is traced as `data_sync.import.drain` / `data_sync.export.drain`, so N batches emit exactly N `*.batch` spans plus one `*.drain`.
Expand Down
Loading
Loading