Skip to content

update-shape.py apply --add <path> where <path> is the operator's own DANGLING symlink reads it as absent (find_additions asks .exists()) and writes THROUGH it, so when the project's validators go red the rollback unlinks the operator's symlink, leaves the file it wrote at the link's target outside the root, and prints that the tree is exactly as it was #239

Closed

Description

@brettheap

Lane: openreposhape-4 (openRepoShape-4)

Found by this lane's adversarial review of the #202 branch (c022b64, the first bullet of its out-of-scope list), re-verified on main a5b09c3 today through the red-validators rollback, the path that exists on main (a5b09c3 has none of #202's rollback), and again on 54aa69a (the one commit since, #173, changes adopt-project.py and its tests only, so every file named below is byte-identical at both). It is pre-existing on main and not a regression of that branch. Not claimed.

What is wrong. update-shape.py apply --add <path> copies the standard's new file to <path> in the root after asking whether the root already has a file there. A symlink the operator keeps at <path> whose target does not exist reads as "nothing there", so check offers the path as upstream-added and apply takes it. The write goes THROUGH the link and creates the file at its target, which a root is free to point outside itself. If the project's own validators then go red (a refusal, "rolled back"), the undo removes the one thing at <path> it sees, which is now the operator's symlink, and leaves the file it wrote, while update-validators-red says "Every byte this command wrote has been rolled back; the tree is exactly as it was." AGENTS.md (Updating a project's shape, item 6) says the same of every red validator: "rolls every byte back". Two files change and the report claims neither did: one the operator made is deleted and one the tool made is left in a directory the tool was never pointed at.

# $U: a clean checkout at a5b09c3. gh is a PATH shim that logs its argv and exits 97 (never called). Every repository is a bare repository under $R/remotes; no network.
python3 $U/scaffold-project.py --org testorg --project Atlas --elected-by 'Test Human' --elected-on 2026-09-02 \
        --local-remote-dir $R/remotes --work-dir $R/work
git clone -q --recurse-submodules $R/remotes/Atlas.git $C
# a project cut before AGENTS-shape.md existed, so the standard "gained" it: the file and its `files:` row are removed in one commit
python3 - $C/contracts/shape-pin.yaml <<'PY'
import re, sys
path = sys.argv[1]
out, skip = [], False
for line in open(path).read().split('\n'):
    if re.match(r'\s*- path: "?AGENTS-shape.md"?\s*$', line):
        skip = True
        continue
    if skip and re.match(r'\s+sha256:', line):
        continue
    skip = False
    out.append(line)
open(path, 'w').write('\n'.join(out))
PY
git -C $C rm -q AGENTS-shape.md
git -C $C commit -q -m 'cut before AGENTS-shape' -- AGENTS-shape.md contracts/shape-pin.yaml
python3 $U/update-shape.py check --root $C --upstream $U
#   exit 1   upstream-added   AGENTS-shape.md      ("ADDED UPSTREAM: ... --add AGENTS-shape.md")
mkdir $R/outside
ln -s $R/outside/agents.md $C/AGENTS-shape.md      # the operator's own link; there is nothing at its end. `git status --porcelain`: `?? AGENTS-shape.md`
# make the project's own validators go red, as a hand edit or a bad merge would (committed, so the tree is otherwise clean)
sed -i -E 's|^topic: .*|topic: xf-project-wrong|' $C/project.yaml && git -C $C commit -q -m 'wrong topic' -- project.yaml
python3 $U/update-shape.py apply --root $C --upstream $U --yes --add AGENTS-shape.md
#   exit 2   added    AGENTS-shape.md (new upstream file, copied from templates/assembly-root/AGENTS-shape.md; its row is appended to the pin)
#            REFUSED update-validators-red: after the rewrite, scripts/validate-manifest.py refused. Every byte this command wrote has been rolled back; the tree is exactly as it was.
ls -l $C/AGENTS-shape.md      # No such file or directory: the operator's symlink is gone
ls -l $R/outside              # agents.md, 8218 bytes; its sha256 is the sha256 of $U/templates/assembly-root/AGENTS-shape.md
git -C $C status --porcelain  # (empty)

The same scaffold, with one thing changed in each row (one fixture each):

the operator's AGENTS-shape.md the run result on a5b09c3
no link (control) validators red exit 2, same refusal; AGENTS-shape.md absent, outside/ empty, git status --porcelain empty: the rollback is right
a dangling link, its directory exists validators red exit 2; the link is DELETED and outside/agents.md is left (above)
a dangling link, its directory exists validators green exit 0; the link stays and the standard's bytes are written to outside/agents.md, outside the root
a dangling link into a directory that does not exist a pinned copy also changed upstream exit 1 and a traceback, FileNotFoundError from write_added (update-shape.py:1618) through _write_copies (:1755) and cmd_apply (:1909); the copy it had already written is left ( M scripts/validate-manifest.py) and the link too (?? AGENTS-shape.md), with the pin unmoved, so a re-run reads the copy as already-at-target

Why. Three steps each treat the link as an ordinary path. find_additions builds Addition(copy, (root / copy.path).exists(), data) (update-shape.py:755), and Path.exists() follows a link, so a dangling one is present=False; resolve_additions refuses update-add-would-overwrite only for a present path (:794-806), and check never reports a path the root already has. Rollback.write_added (:1605-1622) records the path in created (:1617) BEFORE path.write_bytes(data) (:1618), which opens the path for writing, follows the link and creates what it points at. And Rollback.undo (:1622-1637) removes a created path with elif path.exists(): path.unlink() (:1636-1637): the write has made the link resolve, so exists() is true and unlink() removes THE LINK, not the file the write created at its end, which nothing in the ledger knows about. _refuse_red_validators (:1843-1859, the sentence at :1855-1856) and _rewrite_readme's refusal (:1834) then say the tree is exactly as it was. A FileNotFoundError is neither a Refusal nor a CommandFailed, so cmd_apply's except arms (:1921-1927) never reach ledger.undo() for the last row. The same file already holds the principle for the README: ReadmeShapeLine returns README_SYMLINK for a link, "NOT WRITTEN THROUGH, EVER ... a root is free to point outside itself" (:1077-1085, test tests/test_update_shape.py:1290); --add has no counterpart.

What it should do. Two changes, the first of which is what the issue asks for.

  1. A symlink at an --add path is the operator's, and it is refused before anything is written. Ask os.path.lexists, or is_symlink() or exists(), at :755 so the path is present and resolve_additions refuses it, either as update-add-would-overwrite (its text, "Two files with one name is a merge", fits a link too) or as a refusal of its own that names the link and what it points at and says a write through it would land outside the root. That is the README posture, and it also covers the green run in the table, which today puts the standard's file outside the root with exit 0. check then stops offering the path, as it does for any path the root already has.
  2. The ledger records only what it made. write_added creates the file with os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL) (POSIX open(2) fails on any existing path, a link included, whatever the link points at) and appends to created only after the create succeeded, so undo can never unlink something this run did not make and never leave a file it did. This is the guard for a refusal that is not made at find_additions; on its own it turns the red run into a traceback, so it is a companion to (1), not a replacement. write_added's parent loop (:1614) asks .exists() of each parent too; a dangling link as a PARENT directory is not exercised here, and the author should look at it with the same eye.

How to test. In tests/test_update_shape_add.py, with the root and standard fixtures and ADDED (AGENTS-shape-test.md), beside test_add_refuses_to_overwrite_a_file_the_root_already_has (:419) and test_a_red_validator_rolls_the_added_file_back_out (:433), and with @WINDOWS_SKIP as test_a_symlinked_readme_is_never_written_through (tests/test_update_shape.py:1290) does for a link: (a) a dangling link at ADDED whose directory exists in tmp_path outside the root: apply --add ADDED exits 2 with the new refusal before any write, and a snapshot of the root and of the outside directory (os.lstat, os.readlink, file bytes, git status --porcelain, the pin) is identical before and after; (b) the same link with the red-validator setup of :437-442: the same identical-snapshot assertion, which is the run that fails on main today (link gone, outside/agents.md created), written as an invariant so it holds whichever of the two changes is made; (c) a link into a directory that does not exist, with a pinned copy also changed upstream (the fixture's c2 only adds files, so the test edits one template and commits it, as the repro above did): exit 2, no traceback, and the changed copy is not left written; (d) a green run with the link refuses instead of writing outside the root, and the controls hold: an absent path still adds, and a red validator still rolls it back. Reverting (1) to .exists() makes (a)-(c) fail; making undo unlink an uncreated path makes (b) fail.

Related: #202 (PR #233 reworks Rollback for a failure after the copies; its review named this and found the undo predates it), #178 and #183 (the adopt and scaffold materializer write through a root-kept link the same way, for a different tool)

Activity

  1. brettheap commented on Oct 11, 2026

    @brettheap
    ContributorAuthor

    CLAIMED — lane openRepoShape-2, session 224cb7ce-54c8-4501-bfa8-7a7e398984a0@Eagle, 2026-10-11T22:15:07Z, for #239

    Logged in lanes/log/openRepoShape-2.md at b3f0df9e55e5a793045aca7d4f260005b39cc9aa.

    The three reads (lane-collision-protocol Rule 1):

    1. existing `CLAIMED —` comments on opensoft/openRepoShape#239:
       none
    2. `gh pr list --repo opensoft/openRepoShape --state all --search "#239"`, kept where the row or its body names `#239`:
       none
    3. `git ls-remote --heads git@github.com:opensoft/openRepoShape` branches naming `239` as a whole token:
       none
    
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