`branch.publish` accepts a stale base and overwrites `main` (lost update)

Abierto
#691 1 comentario 0 reacciones 1 asignado Ver en GitHub

Los mantenedores suelen responder en 1 día

@mengmengmengqiang ya está trabajando en esto.

Desde el 15/11/2026.

Evaluación

Este issue todavía no se ha evaluado.

Descripción

api/calls bug core/history

Summary

branch.publish does not compare the Branch's base Layer with main's current
head. A Branch forked at Layer L1, while main has advanced to L2, publishes
successfully: the new Layer's parent is L2, but its root is the Branch's own
tree from L1. Every change in L2 that is not also in the Branch is dropped.

The call has no caller-supplied expected base or head, no HeadMoved refusal,
and no merge. It reports overwroteCount/overwrote after the fact, but the
publication has already happened and the command exits 0.

This is documented as "overwrite-only" (history/branch.rs:7), and an
overwrite is a legitimate operation (for example, deliberately resetting main
to a Branch). The bug is that overwrite is the only behavior and the
default, with no caller-supplied expectation and no refusal path: a caller
cannot ask for a safe publish, and the normal "fork, let main advance,
publish" flow selects data loss without failing. The post-hoc overwrote report
does not undo it.

Severity

High — lost update on the only write path to main. The call still
succeeds (kind: "Added", exit 0) and only reports the overwritten Layers after
the fact.

Affected versions / refs

  • LayerFS 2.0.0 (layerfs 2.0.0), tag layerfs_v2_0_0 in the tokenaissance
    fork at a6d84131bdbe17102531dd36a08da5f42a2592c4.
  • Present in upstream main at
    67310175ebba2b84efefb4df76f875a43487cf50 (2026-11-05): the same publish
    and add_layer code is in engine/crates/layerfs-store-service/src/history/branch.rs
    and engine/crates/layerfs-persistence/src/history/layerstack.rs.
  • Observed on project dsh/tokenaissance, Layers 3216923 → 32a2806b →
    32eecd72.

Impact

  • Any parallel-branch workflow where main advances before a Branch publishes
    is a lost-update race. This is the primary multi-agent workflow the Store is
    built for.
  • The parent pointer makes the history look like a normal linear publication:
    the new Layer is a child of the Layer it overwrote, but its tree is the
    Branch's stale tree. Walking parents does not reveal the loss; only the tree
    or the post-hoc overwrote list does.
  • Automation that checks kind == "Added" (or the command's exit code) treats
    the publication as success. overwroteCount is easy to ignore and is not a
    refusal.
  • Recovery needs a manual merge and republish. While that is happening, main
    can move again and the same race recurs; merge and publish are separate calls.
  • The explicit commit form (branch.publish(project, branch, commit)) has the
    same problem when that Commit's base is stale.

Real incident

In dsh/tokenaissance:

Layer source branch parent root overwroteCount
3216923… gc-t13 32b5c67… 27872fc… 0
32a2806b… task-t1.3b 3216923… 0b3851c… 0
32eecd72… task-spec-usename 32a2806b… eee390a… 1

task-spec-usename was forked at 3216923… (branch get reports
base = 3216923…, head = commit 12186e9… with root eee390a…). It published
Layer 32eecd72…:

  • parent = 32a2806b… (the T1.3b Layer it never contained),
  • root = eee390a… — exactly the Branch's own Commit root,
  • overwroteCount = 1, overwrote = [32a2806b…].

32a2806b…'s root 0b3851c… disappeared from main's tree. The Branch was
later merged and republished to restore it.

Minimal reproduction

Uses an isolated LayerFS home and Store; it never touches a real project. The
essential commands are below; the standalone script also asserts the
parent/root/overwrote invariants (run against layerfs 2.0.0).

# The native Store needs ext4/xfs/btrfs; do not put this under /tmp.
BASE=${LFS_REPRO_BASE:-$HOME}
export LAYERFS_HOME=$(mktemp -d -p "$BASE" lfs-repro-home.XXXXXX)
chmod 700 "$LAYERFS_HOME"
SEED=$(mktemp -d -p "$BASE" lfs-repro-seed.XXXXXX)
printf 'base\n' > "$SEED/base.txt"
layerfs store create scratch >/dev/null
layerfs project init scratch/p "$SEED" >/dev/null

# Both Branches fork from the same main head.
layerfs branch fork scratch/p feat-a main >/dev/null
layerfs branch fork scratch/p feat-b main >/dev/null
printf 'from A\n' | layerfs file write scratch/p feat-a /a.txt -m 'A adds a.txt' >/dev/null
printf 'from B\n' | layerfs file write scratch/p feat-b /b.txt -m 'B adds b.txt' >/dev/null

layerfs branch publish scratch/p feat-a   # main: base + a.txt
layerfs branch publish scratch/p feat-b   # feat-b's base is now stale
layerfs file list scratch/p main /

Observed (IDs vary per run):

fork base (main): 321460d3ae7d
publish A: layer=32b3acb81c10 parent=321460d3ae7d root=01e3f9058b9c overwrote=0  main files=['a.txt', 'base.txt']
publish B: layer=32031ba1fd9c parent=32b3acb81c10 root=658f286efcd1 overwrote=1  main files=['b.txt', 'base.txt']

BUG REPRODUCED: publish accepted a branch forked from the old main.
B's parent is A's layer, but B's root is B's own tree, so A's a.txt vanished.

The second publish is accepted (kind: "Added", exit 0), its parent is the
first publish's Layer, and its overwrote list names that Layer — yet a.txt
is gone from main.

Root cause

1. Publish has no caller-supplied expected base or head

engine/crates/layerfs-api/src/branch/publish.rs:9-14 accepts only
{ branch, commit? }. The CLI and JS agent pass exactly that
(api/cli/src/branch/publish.js:3-8, api/agent/src/branch/publish.js:1-4).
There is no way for a caller to say "publish only if main is still at the
Layer I forked from".

2. The "expected" values are self-referential

engine/crates/layerfs-store-service/src/history/branch.rs:592-601 builds the
AddLayerRequest like this:

.add_layer(&AddLayerRequest {
    stack: stack.id,
    branch: branch.id,
    commit,
    expected_stack_head: stack.head_layer,     // read in this same call
    expected_branch_base: branch.base_layer,   // read in this same call
})

Both "expected" values are copied from the state the call just read. They can
only catch a change between that read and the write transaction. They cannot
express "this Branch was forked at L1; refuse if main is no longer L1".

In engine/crates/layerfs-persistence/src/history/layerstack.rs:292-317:

  • :300 checks branch.head_commit == Some(request.commit) — tautological for
    a head publish.
  • :303 checks branch.base_layer == request.expected_branch_base — true by
    construction unless the Branch moved inside the call window.
  • :306 checks commit.base_layer == branch.base_layer — true for a normal
    Branch Commit.
  • :312 checks stack.head_layer == request.expected_stack_head — true unless
    main moved inside the call window.

Nothing checks branch.base_layer == stack.head_layer.

3. add_layer deliberately allows a stale base

engine/crates/layerfs-persistence/src/history/layerstack.rs:318-319 says:

// Overwrite-only: the new Layer sits on the stack head, whatever main
// gained since the Branch's base.

It then builds the Layer at :330-339 with:

parent: Some(stack.head_layer),  // current main head
root: commit.root,               // the Branch's own (stale) tree
overwrote_count: (head - covered) as u64,

So the Layer's parent says "child of L2" while its root says "L1 plus the
Branch's changes". ADVANCE_STACK at :360-379 CASes only against the same
in-call expected head, so it cannot detect the stale Branch either. The
overwrote_count is computed after the fact at :327-331.

The idempotent published_source early return at :281-291 is before all of
these checks and returns UpToDate; a fix must preserve it.

4. branch.merge differs, but is not atomic with publish

engine/crates/layerfs-persistence/src/history/merge.rs:45-74 requires both
sides to still be at the nodes the caller resolved; a moved main is
StackMoved and a moved Branch is HeadMoved, and nothing is written. The
merge itself computes a real merge base
(engine/crates/layerfs-store-service/src/history/branch.rs:366) and merges the
two roots (:424). When one side is main, the merged Branch's base becomes
the main Layer (:508-517), so publishing the merge result is a fast-forward
with overwroteCount = 0.

That is why "merge then publish" is the manual recovery path. But merge and
publish are two calls: main can advance between them, and publish will accept
the now-stale merged base again. The recovery is not race-free either.

5. workspace.commit handles a moved head; publish does not

engine/crates/layerfs-persistence/src/history/commit.rs:85-107 has a real
expected_head/expected_base check (HeadMoved). More importantly,
engine/crates/layerfs-store-service/src/store/commit.rs:238-267 acquires the
publication lane, re-reads the Branch inside it, and merges the candidate into a
moved head before constructing the CommitRequest. The commit path both checks
and merges-on-move.

branch.publish never calls commit_branch; it calls add_layer directly,
with no lane, no in-lane re-read, and no merge-on-move. The HeadMoved check
that protects workspace commits is simply not on the publish path.

6. The publication lane exists but publish does not use it

core/crates/layerfs-history/src/contract/catalog.rs:203-208 defines
publish_lane: "no other publication, in this process or another, can
interleave with the caller's read of a head and its publication against it."
PublishLane is at :239; the implementation is
engine/crates/layerfs-persistence/src/history/catalog.rs:155-158; the
workspace commit uses it at engine/crates/layerfs-store-service/src/store/commit.rs:238-243.

A lane by itself does not prevent this bug: it only orders publications, and
the second stale publication still overwrites. It is necessary if publish starts
doing a read-merge-publish sequence (auto-merge), and useful for a stable
read+CAS, but the stale-base check itself must run inside the write transaction.

Proposed fix

Primary: refuse a stale-base publish by default, with an explicit escape hatch

Server-enforced, single-call, small change:

  1. Extend the publish request/arguments with:
    • expected_base (Layer id, optional),
    • expected_head (Commit id, optional),
    • force (bool, default false; same meaning as workspace.commit's
      --force: publish over a moved head).
  2. In history::branch::publish, acquire publish_lane, re-read the stack and
    Branch inside it, and resolve
    expected_base = caller.expected_base.unwrap_or(branch.base_layer).
  3. In add_layer, after the published_source early return, add:
// A Branch forked at an older main must be merged first. Without `force`,
// publishing it would replace main's root with the Branch's stale tree.
if !request.force && request.expected_branch_base != stack.head_layer {
    return Err(HistoryError::StackMoved {
        expected: request.expected_branch_base,
        actual: stack.head_layer,
    });
}

StackMoved already maps to the wire code HeadMoved
(engine/crates/layerfs-store-service/src/history/lookup.rs:44-45), so no
new control code is needed. If a distinct StaleBase code is preferred, it
is a new wire value and needs the usual compatibility work; reusing
HeadMoved is the smaller change.
4. On refusal, say what to do:
"main moved since <branch> forked at <base>; merge main into <branch> and publish again, or pass --force".
Have publish construct the SdkRefusal with
detail: { expected_base, actual_base }; lookup::history currently drops
the StackMoved detail and would need extending if the detail is wanted on
every call.
5. force = true skips the new check and preserves the current overwrite
behavior, still reporting overwroteCount/overwrote. This is the migration
path for anyone who intentionally resets main to a branch.
6. CLI/SDK options come from the call table, so --expected-base,
--expected-head, and --force are generated for the shell, Node and Python
surfaces; update typings and help tests.

Default behavior after the fix:

  • Branch base == current main head: unchanged fast-forward publication.
  • Branch base != current main head: refused HeadMoved unless --force.
  • Same Commit already published: still UpToDate (the early return is before
    the new check).

Backward compatibility: this is a behavior change for stale publishes. Ship it
as a minor release with a release note and the --force escape, or stage it as
described in the fallback. The old behavior is exactly the data-loss behavior,
so keeping it as the default is not a safe option.

Fallback: opt-in CAS only, with a safe CLI default

If maintainers treat "publish overwrites" as a contract that cannot change by
default:

  • Add expected_base/expected_head as opt-in arguments and enforce them
    inside add_layer exactly as above.
  • Make the CLI pass expected_base = branch.base_layer by default, so shell
    users are protected without a flag; SDK callers must opt in.
  • Emit a prominent warning when overwroteCount > 0 (not a refusal).
  • Document the SDK default as unsafe; flip the default in the next major.

This prevents the common CLI path from losing updates but leaves SDK/automation
callers on the unsafe default until they adopt the option.

Follow-up: auto-merge on publish

After the CAS fix, branch.publish can merge instead of refusing when the base
is stale:

  • take the same lane,
  • merge main's head root with the Branch Commit's root using the Branch base
    as the merge base,
  • clean merge: create a Layer with the merged root and parent = main head;
    return kind: "Merged", mergeBase, lww, conflicts: [];
  • conflict: return Conflict, write nothing, leave main unchanged.

This gives the best UX (no manual merge), but turns publish from a bounded
metadata operation into a content merge with conflict semantics. It needs a
conflict policy, provenance for the synthesized root, and response/API changes.
It belongs in a later minor, not in the fix for the lost update.

Solution comparison

Option Prevents lost update? Atomic single call? API / compatibility Cost Verdict
(a) expected-base/head CAS, refuse by default, --force escape Yes Yes: check, insert and stack advance are one write transaction; lane for the read+CAS New optional args + behavior change; existing overwrite users pass --force; reuses HeadMoved Small Primary
(b) auto-merge on publish Yes Yes, inside the lane New Merged/Conflict outcomes; publish becomes a content merge; provenance and LWW decisions Large Follow-up
(c) CLI/SDK re-read main, merge before publish Only with (a) underneath No: read, merge and publish are separate calls; TOCTOU No server change, but creates branches/commits as a side effect; only covers that front end Medium Sugar, not a fix alone
(d) server-side serialization/lane only No: ordering only; the stale publish still overwrites Yes for ordering None Small Necessary adjunct, not a fix

Recommendation: (a) + (d) now, (b) later, **(c) only as optional CLI
ergonomics after (a).

Acceptance tests

  1. stale_base_publish_is_refused: fork A and B from the same main; publish A;
    publish B -> HeadMoved; main head and root are still A's; B's Branch is
    unchanged; no new Layer was written.
  2. force_keeps_overwrite_semantics: the same setup with force: true ->
    Added; B's Layer parent is A's Layer; B's root is B's Commit root;
    overwroteCount == 1 and overwrote == [A's layer].
  3. publish_after_merge_is_fast_forward: fork B; publish A; merge main and B
    into M; publish M -> Added; parent is A's Layer; tree has both files;
    overwroteCount == 0.
  4. republish_is_uptodate_after_main_moved: publish B fast-forward; publish A;
    republish the same B Commit -> UpToDate, not HeadMoved; main unchanged.
  5. expected_base_and_head_cas: stale expected_base -> HeadMoved; current
    expected_base -> Added; wrong expected_head -> HeadMoved.
  6. concurrent_stale_publishes: two processes publish two Branches from the
    same base at once; exactly one Added, the other HeadMoved; main's root
    is the winner's root and no Layer is silently skipped.
  7. cli_refusal_and_force: shell branch publish on a stale Branch exits 125
    with HeadMoved and the merge/--force message; --force exits 0 and the
    text output includes overwrote_count.
  8. node_and_python_surfaces: the new options and the HeadMoved detail are
    exposed in both SDKs and their type definitions.

Workarounds until fixed

  • Before publishing, compare branch get <branch>.base with branch get main's
    saved head. If they differ, branch merge <tmp> main <branch> and publish
    <tmp>; repeat if main moves again. This is what the incident recovery did,
    but it is not atomic.
  • Never publish a Branch whose base is not main's current head, unless the
    overwrite is intentional.
  • Treat a non-zero overwroteCount as a failed publication: inspect
    overwrote and restore the skipped Layers by merging before republishing.

References

  • engine/crates/layerfs-store-service/src/history/branch.rs:7 — "Publishing
    is overwrite-only and reports the main Layers it skipped."
  • engine/crates/layerfs-store-service/src/history/branch.rs:573-664 — publish.
  • engine/crates/layerfs-persistence/src/history/layerstack.rs:265-381 —
    add_layer; :292-317 checks; :318-319 overwrite-only comment; :330-339
    Layer; :360-379 stack advance.
  • engine/crates/layerfs-persistence/src/history/merge.rs:45-74 — merge CAS.
  • engine/crates/layerfs-persistence/src/history/commit.rs:85-107 —
    commit_branch HeadMoved.
  • engine/crates/layerfs-store-service/src/store/commit.rs:238-267 — lane,
    in-lane re-read and merge-on-move for workspace commits.
  • core/crates/layerfs-history/src/contract/catalog.rs:203-208, :239 —
    publish_lane.
  • Real incident: dsh/tokenaissance Layers 3216923…, 32a2806b…,
    32eecd72…; Branch task-spec-usename (base = 3216923…,
    head = 12186e9…, root eee390a…).
Lenguaje dominante
Rust
Estrellas
2.1k
Forks
110
Merge medio
23 h 52 min
PR fusionados (30 d)
138

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de Ephemeral-AI-Lab/layerfs

Todos los issues de Ephemeral-AI-Lab/layerfs

Issues similares

Más issues de Rust

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.