Skip to content

tree_id_for is not injective and the recorded checkout is never compared with git's, so two trees can share one sidecar and a replaced repository reads as ok #116

Description

@brettheap

A tree in the lane inventory is identified by its path with everything outside the manifest-key set folded to -, and its repository is recorded and never checked. Two halves of one question: is this the tree the record is about?

1. tree_id_for is not injective

tree_id_for() {   # <absolute worktree path>
  tif_p="$(printf '%s' "${1-}" | tr -c 'A-Za-z0-9._-' '-' | tr -s '-')"

/tmp/a/b and /tmp/a-b both fold to tmp-a-b, and tr -s '-' squeezes repeats, so /tmp/a//b joins them. The id is the sidecar's FILE NAME, so the second tree's record replaces the first's: a lane with two valid worktrees loses one from lane-trees, from lane-reconcile, and from every classification the reconciliation makes — the lost one can only be rediscovered as unmanaged, with no stored observation to compare against, which is the difference between missing and possible-loss.

The digest that would separate them is already in the function and is applied only to long paths:

  if [ "${#tif_p}" -gt 180 ]; then
    tif_c="$(printf '%s' "${1-}" | cksum | awk '{print $1}')"
    tif_p="c$tif_c-$(printf '%s' "$tif_p" | tail -c 180)"
  fi

2. The recorded checkout is never compared with the repository git reports

lane_tree_put records it:

    printf 'checkout: %s\n'  "$(lane_sidecar_value "${ltp_co:-unknown}")"

and lane_reconcile recomputes branch, head, upstream, dirty and unpushed for every tree and compares those five — and not this one. If the path is occupied by a DIFFERENT checkout that happens to be on the same branch at the same commit (a re-clone, a git worktree move of something else onto the path, a restored backup), every observation reads ok and the report says the tree is current. A recovery then relaunches a writer into a repository that is not the one the record is about.

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. Neither of these is a small edit: the id is a MIGRATION — every sidecar already on disk is named by the current spelling, and changing the derivation renames all of them — and the repository check needs an identity that is stable across a clone (the common dir, or the origin URL, each with its own answer for a worktree of a worktree).

What's open

For the id: an injective encoding, or the cksum prefix on every path rather than on long ones only, plus a migration that reads the old name and writes the new one once. For the repository: record an identity git can answer for (git rev-parse --path-format=absolute --git-common-dir, or the origin URL) and compare it in lane_reconcile, classifying a mismatch as inconsistent rather than ok.

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