Skip to content

set-lane-tree's fence accepts a poll from a finished operation, and its observation check accepts a partial one it completes with clean-looking defaults #114

Description

@brettheap

set-lane-tree's compare-and-swap compares the generation and the operation and nothing else, and its observation check asks whether the caller passed NONE of the five fields — not whether it passed all five. Two holes, one writer.

1. The fence never requires the lane to still be SWAPPING

      slt_fence=""
      [ -z "$slt_g" ]  || [ "$slt_g"  = "${slt_ng:-0}" ]    || slt_fence="generation is ${slt_ng:-0} and --generation named $slt_g"
      [ -z "$slt_op" ] || [ "$slt_op" = "${slt_no:-none}" ] || slt_fence="${slt_fence:+$slt_fence; }operation is ${slt_no:-none} and --operation named $slt_op"

SWAPPING -> SWAPPED deliberately keeps both values — it is the same operation reaching its commit point, and a fence that moved under it would refuse the very finalizer entitled to write:

    case "$sls_state" in
      SWAPPED) sls_gen="$sls_nowg"; [ -n "$sls_op" ] || sls_op="${sls_nowo:-none}" ;;

So set-lane-tree <lane> <path> --generation G --operation O still succeeds AFTER that operation has been finalized. A poll from operation O that was delayed past the finalizer — the lane-handoff poll_writer loop is exactly such a caller — overwrites the completed inventory with a reading taken before it. Nothing downstream can tell: the sidecar's own generation and operation match the lane's.

2. A partial observation is completed with clean-looking defaults

    if [ -z "$slt_b$slt_h$slt_u$slt_d$slt_n" ]; then
      slt_now=""; slt_nrc=0
      slt_now="$(lane_tree_now "$slt_path")" || slt_nrc=$?

The guard is all five empty, so naming ONE field skips the observation entirely and the other four fall to lane_tree_put's defaults:

    printf 'branch: %s\n'    "$(lane_sidecar_value "${ltp_branch:-unknown}")"
    printf 'upstream: %s\n'  "$(lane_sidecar_value "${ltp_up:-none}")"
    printf 'dirty: %s\n'     "${ltp_dirty:-0}"
    printf 'unpushed: %s\n'  "${ltp_unp:-0}"

set-lane-tree <lane> <path> --dirty 1 therefore records no upstream, 0 unpushed for a tree this process never read — and 0 unpushed with upstream none is precisely the record a later lane-reconcile reads as clean and published, which is the shape round 5 removed from the all-empty path and left standing on this one.

Why this is filed rather than fixed on #97

Copilot review rounds are capped at two per pull request (Brett Heap's ruling of 2026-09-16). #97 is at round six and took only the two safety holes it could not land with (the unreadable lifecycle snapshot, and this same writer's missing mutex, which is a few lines and serializes every sidecar write). Both findings here predate that round: the fence has compared those two values since it was written, and the all-five-empty guard has been the test since the first commit.

What's open

For the fence: include the lifecycle state in the comparison — at minimum, require the lane to be SWAPPING while an observation is filed under an operation, so that a finished operation stops accepting polls — or invalidate the operation id at finalization. For the observation: refuse a partial one (a usage error naming the fields that are missing), or read all five here from one lane_tree_now rather than mixing a caller's two with three this process invented.

Filed from Copilot round 6 on #97 — #97 (comment) and #97 (comment) — and named in openspec/changes/add-crash-consistent-lane-worktree-recovery/tasks.md section 7.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions