Skip to content

Feature/offline sync - #11

Merged
finn-abel merged 5 commits into
mainfrom
feature/offline-sync
Aug 12, 2026
Merged

Feature/offline sync#11
finn-abel merged 5 commits into
mainfrom
feature/offline-sync

Conversation

@finn-abel

Copy link
Copy Markdown
Contributor

Phase 8 — Offline sync (build plan steps 41–43b, + a follow-up pass)

Five commits: the durable op log and change cursor (41), the push engine that replays a
technician's queue against the domain (42), the pull engine that tells a device what it missed
(43), the server side of photo/signature capture (43b), and a pass closing decisions nothing in
the remaining plan would ever force.

720 tests pass (625 unit, 95 integration); zero warnings in Debug and Release under nullable +
analyzers + warnings-as-errors; make check-contracts clean; both migrations apply to a database
that already has rows. No endpoint exists yet for any of this — steps 50 and 50b.

Domain Job.Notes/RecordNotes/CanRecordNotes (LWW), Job.Lines/RecordLine/JobLine (field-recorded work), Job.Unschedule() + JobUnscheduled, TimeWindow.LatenessOf
Domain Attachment aggregate, AttachmentKind, AttachmentId, StorageKey
Application Sync/SyncOpRecord/SyncOpId/SyncCursor, Sync/PushOps/, Sync/PullChanges/, Sync/FieldOps, Sync/SyncErrors, Sync/SyncRemoval
Application Abstractions/ISyncOpStore, ISyncCursorSource, ISyncChangeReader, IAttachmentStorage, IAttachmentRepository, SyncJobState/SyncStopState/SyncScopeChanges
Infrastructure Persistence/ChangeStamps (trigger-based change_seq on every table), SyncCursorSource, SyncOpStore, SyncChangeReader, AssignmentRepository.Remove (writes tombstone)
Infrastructure Attachments/LocalDiskAttachmentStorage, JobConfiguration (lines as jsonb), AppDbContext.StampVersions (owned-child changes bump root version)
Contracts SyncConflictReason.Unsupported; regenerated jobTransitions (Unschedule)
Migrations SyncOpLog, SyncRemovals, Attachments — three tables, one trigger function shared by all
push  ─▶ FindAppliedAsync(ids) ─▶ apply new ops in order ─▶ CanTransition?/CanRecordNotes? ─▶
          success carrying conflicts (never a failure) ─▶ log applied only ─▶ cursor taken last

pull  ─▶ cursor taken FIRST ─▶ WHERE change_seq >= cursor (inclusive: may repeat, never misses)
          scope = "my stops, and the jobs they're for" ─▶ removals from row-stamp OR tombstone

every write ─▶ BEFORE INSERT OR UPDATE trigger ─▶ change_seq := pg_current_xact_id()

Decisions a reviewer should weigh in on

The change stamp is a database trigger — the first logic this project has put in the database.
A cursor is only correct if everything a device hasn't seen is at or after it. An application-side
counter (interceptor or per-tenant sequence) has a hole: the number is allocated before the row
becomes visible, so a device can be handed a cursor above rows still invisible to it, and they're
then below its cursor forever. Fix: stamp with pg_current_xact_id() (the writing transaction's
own id) via BEFORE INSERT OR UPDATE, and take pg_snapshot_xmin(pg_current_snapshot()) as the
cursor — the oldest transaction that could still be in flight. Holds by construction; an
interceptor would only be correct while every save happens to sit in an explicit transaction.
Cost: a new table's trigger must be hand-written (SchemaTests catches a missing one); this
happened for real three times (SyncOpLog → SyncRemovals → Attachments) and the test caught it each
time I checked by removing one.

"Changed since" is inclusive (>=), so pull can resend a change. The cursor is the oldest
possibly-in-flight transaction, so missing the exclusive boundary is the only safe direction.
Correct side to fail on: every pulled change is a whole state, so resending writes the same thing
twice; an exclusive cursor could skip a late-committing write and never send it.

The watermark is cluster-wide, not per-tenant — one org's writes advance every org's cursor.
Costs an occasional empty pull response; the alternative is a per-tenant sequence with a lock held
to commit, i.e. serialized writes per org. Wrong trade for a ten-technician shop.

A stop leaves a technician's day two ways, and only one can report itself. Handed to another
technician: the row survives, its own stamp reports it. Deleted outright (re-optimization dropping
unplaceable work): nothing left to carry a stamp, so AssignmentRepository.Remove writes a
sync_removals tombstone in the same save. Rejected alternative: soft-deleting assignments, which
would make the unique index on assignments(job_id) partial and put a RemovedAt is null filter
on every existing query. A tombstone is additive; a soft delete changes everything that reads a
stop. The tombstone write lives in the repository (not the handler) so a future second
plan-dropping handler can't forget it — the alternative, a SaveChanges interceptor, is fragile
while EF is still building its change set.

Pull scope = "a job is in scope because a stop for it is mine." Needs no per-device state,
which is what keeps sync retriable from a cursor alone. Consequence: a job whose stop moved away
leaves the device's world silently via the stop's removal, with no second "this isn't yours"
event. This incidentally fixed step 37's old wart (a dropped job kept showing Scheduled with
nothing planned) on the phone's side; Job.Unschedule() (below) fixes the dispatcher's side.
Removals are deliberately over-emitted — named to a technician whether or not they ever held that
stop, because the server can't know what a device holds without per-device state.

The op log holds only applied operations; refusals aren't recorded. AppliedAt is not
nullable. A resent refusal is re-judged against current state rather than answered from memory —
correct while every refusal is re-derivable (it is, today).

A pushed batch never fails; conflicts ride inside a successful result.
TransactionBehavior rolls back a failed result, so reporting one bad op as failure would discard
every op that succeeded ahead of it — a technician's whole morning lost to one stale tap.
Mutation-verified: forcing a failure when conflicts exist fails exactly the two integration tests
asserting the batch still committed.

The build plan's domain didn't exist yet, so most of step 42's diff builds it. "Notes, line
items" as field ops assumed Job already had somewhere to put them; it didn't. Notes are one
overwritable field (LWW needs something to beat — a list can never conflict); recorded lines are
an append-only JobLine collection, deliberately not the invoicing LineItem (one is a record of
what happened, the other a statement of what's owed — merging them now would prejudge whether
invoicing derives from field work, which is Documents 6–7's call). Neither is refused by any job
status: a technician who drove out to a cancelled job has still spent the hour.

Recorded lines are one jsonb column on the job, not a table — never read without the job,
never queried alone, and being part of the job's own row is what makes a recorded part a change
to the job
, moving both the version stamp and the sync change stamp. Cost: EF leaves the owner's
change-tracker entry Unchanged when only a JSON child changes (confirmed with a throwaway
diagnostic), so StampVersions now walks owned children explicitly — which incidentally fixed the
same latent bug for customer locations and invoice lines, neither of which had ever bumped their
root's version either. Limit: a removed child is invisible to this walk (nothing removes one
today; step 47 will).

Contracts gained a third SyncConflictReason member, Unsupported, in a step whose text
named only an Application folder. The alternative — reject the whole batch when it contains an
operation type the server doesn't know — would let a device one release ahead poison its own
queue forever, since Entity/Type are strings specifically so unknown ops can be named. Adding
the member and refusing per-operation was the only option that doesn't strand a technician.

A StorageKey value object is derived, never given. It becomes a filesystem path today and a
bucket key later; a key built from anything a device sends (filename, path) is directory
traversal. It can only be constructed from a tenant + attachment id, both already-vouched values.
Cost: a fourth value object/converter for what could've been a string — worth it because it moves
the guarantee from "the adapter remembers to sanitize" to "the dangerous call can't be written."
Bytes are written before the metadata row (in UploadAttachmentCommand, step 50b's problem to
call): a mid-request death leaves an orphaned file rather than a row pointing at nothing — orphans
are inert litter, dangling rows are reachable and broken. Nothing sweeps orphans yet (no owner,
same class as unpruned tombstones).

Local-disk attachment storage is the shipped answer, not a placeholder — Document 1's
self-hosting promise needs nothing more for one machine. Its stated limit: two instances without
shared storage will write on one and 404 on the other; the bucket adapter is one class behind the
same port whenever that's needed.

The follow-up pass

DECISIONS.local.md had accumulated 19 entries with no owner in the remaining plan (steps
44–56 won't force them). Two get materially more expensive after step 52 publishes
@opendispatch/contracts to the client repos, so this was the moment to act, not later.

Fixed, three things:

  1. Clock skew. LWW notes trusted the device's claimed ClientTs outright, so a phone with a
    clock a day fast would win every conflict until that date passed, undetected. PushOpsHandler
    now clamps every observed instant to min(ClientTs, now) — including the payload's own
    completedAt — before the domain sees it, while the raw ClientTs still goes to the op
    log (the log is evidence; clamping it would hide the broken clock). Clamped, not rejected: the
    op itself isn't wrong. Mutation-verified: removing the clamp fails exactly three push tests.

  2. Job.Unschedule(). A job the optimizer drops loses its stop but kept Scheduled status —
    the board's one visibly false state. Fixed now rather than later because the transition table
    is exported to @opendispatch/contracts; today the cost is one make gen-contracts and a
    two-line diff, after step 52 it's a release across three repos. First backward edge in an
    otherwise linear machine: a plan can be withdrawn, work cannot (nothing returns from
    EnRoute onward). JobIntents.Drivable is unchanged — no request drives this, only the
    optimizer. Mutation-verified: skipping the withdrawal call fails exactly two optimizer tests.

  3. Lateness stated once. Lived only inside ObjectiveEvaluator; the web repo was next to
    need it and would've invented a second definition. Now TimeWindow.LatenessOf(start) is the
    rule (measured at work start, not end), used by the evaluator for both lateness and overtime,
    and sent to the board as BoardStop.LateBy rather than left to be re-derived client-side —
    arithmetic can't be exported safely the way the transition table is (data vs. logic).
    Mutation-verified: forcing it to always say "on time" fails the domain tests, two objective
    tests, three search tests, and the board test — five independent readers of one rule.

Decided, not coded: MediatR stays pinned at 12.5.0 (reviewed, not inherited — trades upstream
fixes for Document 1's no-licence-bill promise; reasoning now lives permanently in the csproj, not
this file). Unassigned-job reasons are committed to step 48 rather than left unowned (the
expensive half is the engine — Solution.Unassigned would need a reason per job, touching the
constructor invariants, ObjectiveEvaluator, Insertion, and both schedulers). Retention is
still open but is now one question instead of three, naming the gap that was missing:
sync_ops — not the tombstones or blobs — is the table that actually grows fastest and can't be
pruned freely, since dropping rows un-dedupes any device offline longer than the window.

Checked and left alone: the payment reference (zero risk pre-processor), the duplicated
window-overlap predicate (still two sites, both boundary-tested), the local-disk storage limit (a
deployment decision behind an existing port), the Microsoft.OpenApi pin (dependabot owns it),
the missing outbox (step 51 is the first real subscriber to need one), and everything whose answer
belongs to the client repos (Dispatched's schedulability, the skill-match override, pull's
over-emission).

What is deliberately not here

  • No /sync/push or /sync/pull endpoint — step 50, including the technician-from-principal
    resolution and the SyncConflictReason code mapping.
  • No horizon or paging on pull; a first sync returns everything a technician has ever had. Step
    50 owns it — can't be paging, since the cursor is a transaction watermark with no position
    inside a transaction.
  • No attachment upload endpoint, no attachments in GET /export — step 50b.
  • GenerateInvoice still ignores job.Lines — a product decision (what's billed isn't always
    what the visit took), left to Documents 6–7 or step 55.
  • No sweeping of orphaned blobs or pruning of tombstones/op-log rows — all recorded as open,
    unowned questions rather than silently deferred.

@finn-abel
finn-abel merged commit 2fc20b0 into main Aug 12, 2026
2 checks passed
@finn-abel
finn-abel deleted the feature/offline-sync branch August 12, 2026 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant