# Task 1926 — same-account portal push concurrency guard (design)

**Date:** 2026-07-26
**Task:** [`.tasks/1926-concurrent-same-account-portal-push-lets-the-pre-stage-clear-delete-the-live-tree.md`](../../../../.tasks/1926-concurrent-same-account-portal-push-lets-the-pre-stage-clear-delete-the-live-tree.md)
**Touches:** `platform/plugins/cloudflare/bin/portal-index-push.mjs`, `platform/plugins/cloudflare/mcp/__tests__/portal-index-push.test.ts`

## Problem

`pushAccount` stages an account's file index at `next = currentGeneration + 1`, flips
the `directory_state` pointer to `next`, then sweeps every other generation. Task 1923
added a pre-stage `DELETE FROM directory WHERE accountId = ? AND generation = ?` bound
to `next`, so a stage that died mid-batch could self-heal instead of colliding forever
on `UNIQUE (accountId, relPath, generation)`.

Two pushes of one account break that. The second writer reads `current = 5` before the
first writer's flip, computes `next = 6`, and by the time its pre-stage DELETE runs the
pointer already names generation 6. The DELETE removes the live tree. A client listing
in the gap between that DELETE and the re-insert sees an empty portal.

The overlap is reachable: the admin server's 60s loop calls `pushAllAccounts`, and a
hand-run `node portal-index-push.mjs --account <id>` calls the same `pushAccount` from a
separate OS process.

## What success looks like

Two concurrent pushes of one account never leave a reader seeing an empty or partial
tree, and never delete rows at the pointer's generation.

## Why the three candidate approaches in the task file were rejected

**A per-account advisory lock.** The two racers are separate OS processes. A JavaScript
mutex inside the admin server does not exist in the CLI process, so it guards nothing.
A lock that does work across processes has to live in D1, which means new columns, a
lease, and a lease expiry. A lease expiry reintroduces the wedge class Task 1923 just
removed: a holder that dies wedges the account until the lease runs out, and a push
slower than its lease keeps writing after another writer has taken over.

**`next` from `MAX(generation)` alone.** This stops two writers sharing a staging
generation, but the slower writer still flips the pointer down onto its own generation
after the faster writer has already swept it. The reader still sees an empty tree, so
the success criterion is not met.

**Scoping the clear to `generation = next AND generation != current`.** `current` is a
value read at the top of the push. By the time the DELETE runs, the pointer has moved.
Re-reading the pointer immediately before the DELETE narrows the window without closing
it, because the peer can flip between the re-read and the DELETE.

## Design

Three changes to `pushAccount`. No lock, no lease, no schema change, no new table.

### 1. Claim a generation above everything that exists

Keep the existing pointer read, and add one statement beside it:

```sql
SELECT MAX(generation) AS g FROM directory WHERE accountId = ?
```

`next = Math.max(current, maxStagedGeneration) + 1`.

A peer that has already inserted at least one row is visible in that MAX, so the two
writers claim different generations and neither stages into the other's rows.

Two statements rather than one nested `UNION ALL`: each is trivially readable, each is
trivially fakeable in a test, and the pointer read stays the push's first statement,
which an existing test asserts.

### 2. The flip only ever moves the pointer forward

```sql
INSERT INTO directory_state (accountId, currentGeneration) VALUES (?, ?)
ON CONFLICT (accountId) DO UPDATE SET currentGeneration = excluded.currentGeneration
WHERE excluded.currentGeneration > directory_state.currentGeneration
```

SQLite has supported a `WHERE` clause on upsert's `DO UPDATE` since 3.24, and D1 is
SQLite. This is what makes a losing writer harmless: it cannot drag readers back onto a
generation the winner has already swept.

The D1 client returns rows, not affected-row meta, so the flip is followed by a pointer
readback. That readback is the branch point:

- **Pointer equals `next`.** We won. Log `op=flip`, sweep, verify, return. Unchanged
  behaviour.
- **Pointer is above `next`.** A peer won. Log
  `op=stage-contended account=<id> generation=<next> pointer=<actual> action=superseded`
  and return without sweeping and without verifying. Both of those would measure the
  peer's tree against our expected row count, and `op=verify` would read as a failure
  when nothing failed.

Our staged rows at the losing generation are left in place. They are invisible (no
reader sees a generation the pointer does not name) and the winner's next sweep collects
them.

### 3. The sweep only ever removes older generations

```sql
DELETE FROM directory WHERE accountId = ? AND generation < ?
```

`< next` rather than `!= next`, so a writer never deletes a peer's higher in-flight
stage. Rows above `next` from an abandoned stage still converge: the following cycle
claims a generation above them and sweeps them once it flips.

### 4. The Task 1923 pre-stage clear is removed

Because `next` is now always above every existing generation for the account, the orphan
`SELECT COUNT(*) ... generation = next` can only ever return 0 and the pre-stage DELETE
can never match a row. Leaving unreachable code whose comment claims it is the wedge
heal would be false narration, so the orphan count, the DELETE, the `op=stage-clear` log
line and its two tests go.

Task 1923's outcome is preserved by construction and is stronger than before. A stage
that died mid-batch leaves rows at generation 6; the next run claims 7, so there is no
UNIQUE collision to freeze on, and the gen-6 orphans are swept after the flip. The heal
now happens with no DELETE before the flip at all, which is exactly what removes this
task's hazard rather than narrowing it.

This was confirmed with the operator before the spec was written, because Task 1926
lists Task 1923's heal as out of scope.

## Cases covered, and the one left fail-closed

**Peer reads the generation after we have staged a row.** It claims a higher generation.
Both pushes complete. The forward-only flip picks the winner. The reader sees the old
complete tree, then one complete new tree. Neither push fails.

**Both read the generation before either inserts.** Both claim the same generation. The
second writer's first INSERT hits `UNIQUE (accountId, relPath, generation)` and that push
throws. Both walks resolve the same exposed set and `walkExposed` emits the exposed dirs
in the same order, so the second writer collides on its first row and contributes
nothing. The reader is safe: the winner's tree is complete and the pointer never moved
backward. The loser logs `op=failed` and the next 60s cycle succeeds.

This second case is left fail-closed rather than retried. The task's success criterion is
about what a reader can see, and a loud failure that self-corrects one cycle later meets
it. A retry-on-collision path would be an invented requirement.

**The one residual, stated rather than fixed.** The claim above relies on both walks
starting with the same row. If the account's `SCHEMA.md` or `data-portal.json` changes in
the window between the two generation reads, the two walks can begin with different rows,
so the second writer contributes some non-overlapping rows at the shared generation before
colliding on a shared one. The winner then flips onto its own complete tree plus those few
extra rows. That is not an empty tree and not a partial one, so it does not breach the
success criterion, and the next cycle's sweep removes the whole generation. No task file is
filed for it because nothing is deferred: the case is inside what this design covers, and
its outcome is a stale row for one cycle rather than a reader-visible break.

## Observability

`op=stage-contended account=<id> generation=<next> pointer=<actual> action=superseded`
is emitted when a push's flip was superseded by a peer. This deviates from the task
file's `action=<waited|skipped>` wording: the design never waits and never skips work, so
neither value describes what happened. `superseded` names the actual outcome, and
`pointer=` names the generation that won, which is what an operator needs to correlate
the two runs.

`op=stage-clear` is retired with the code that emitted it. The task file's line that a
`stage-clear` with `cleared=` equal to the live tree size must never appear is satisfied
absolutely: no `stage-clear` line can appear at all.

## Testing

Both new tests drive two interleaved `pushAccount` calls against one shared fake store
and snapshot the reader's view after every statement. The assertion is the success
criterion stated directly: the visible tree is never empty and never a strict subset of
a complete tree.

1. **The task file's interleave.** Writer B reads the generation, then pauses. Writer A
   runs to completion. B resumes. Red on current code, because B's pre-stage DELETE binds
   the generation the pointer now names and the snapshot goes empty. Green after, because
   B collides on UNIQUE and A's tree is untouched. This test also asserts that no DELETE
   is ever issued naming the pointer's current generation, which is the other half of the
   success criterion.

2. **The higher-claim interleave.** Writer A stages fully but pauses before its flip.
   Writer B reads the generation, sees A's staged rows in the MAX, claims the next one up,
   and completes. A resumes and its flip is a no-op. Asserts the pointer never moves
   backward, that A emits `op=stage-contended` with `action=superseded`, and that A
   issues no sweep.

The existing 27 tests stay green apart from the two Task 1923 stage-clear tests, which
are removed with the code they cover.

## Scope boundaries

**In scope:** the generation claim, the forward-only flip, the backward-only sweep, the
removal of the pre-stage clear, and the two-writer regression tests.

**Out of scope:** INSERT batching (Task 1842). Cross-account contention (separate
generation counters, no contention). Any change to `runTargets`, `recordSuccess`, or the
portal reader in `files.ts`.

## Deployment

`portal-index-push.mjs` ships inside the installer payload and is inlined into the admin
server bundle. No D1 migration is required: every statement runs against the existing
`directory` and `directory_state` tables as defined in
`platform/plugins/cloudflare/skills/data-portal/schema.sql`.