From 26e9bca8650739200e4f542f1181cffd97aa5a10 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 17 Oct 2026 17:27:06 +0000 Subject: [PATCH] fix(brain-sync): close review gaps in the case-collision hold - Windows: write the held-path list with LF and strip CR from helper output, so CRLF cannot let a held path through or blank case_names. - --publish-removals runs the outgoing case check before it pushes. - The cached outgoing verdict is keyed by every ref, not HEAD alone, so a moved remote baseline is re-checked. - The repair snippet folds only JSONL logs and lists snapshots and other files for a manual decision. Co-authored-by: Kkkkkuro <180382840+Kkkkkuro@users.noreply.github.com> Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-brain-sync | 14 +++++++++----- bin/gstack-brain-sync-index.py | 3 ++- docs/troubleshooting.md | 25 +++++++++++++++++-------- test/brain-sync.test.ts | 29 +++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 14 deletions(-) diff --git a/bin/gstack-brain-sync b/bin/gstack-brain-sync index f77639514..f8a80da6c 100755 --- a/bin/gstack-brain-sync +++ b/bin/gstack-brain-sync @@ -783,13 +783,13 @@ stage_paths() { # spellings, and a retained local commit can be pushed with an empty queue. # Also records the tree's case-colliding names for skill start. Without # python3 no drain can stage anything, so there is no new tree to check. A -# passing verdict is cached by HEAD's commit id, so the empty-queue drain at -# every skill boundary skips the tree walk until HEAD moves. A cached HEAD -# that falls behind origin is only pushed after a merge, which moves HEAD. +# passing verdict is cached by the commit ids of HEAD and every ref (origin's +# included, the comparison baseline), so the empty-queue drain at every skill +# boundary skips the tree walk until one of them moves. validate_outgoing_paths() { local out err key cached cache="$GSTACK_HOME/.brain-case-check" command -v python3 >/dev/null 2>&1 || return 0 - key=$(git -C "$GSTACK_HOME" rev-parse --verify -q HEAD 2>/dev/null) + key=$(git -C "$GSTACK_HOME" show-ref --head 2>/dev/null | cksum) cached=$(head -1 "$cache" 2>/dev/null || true) if [ "${cached%% *}" = "$key" ]; then [ -n "$ST_CASE_NAMES" ] || ST_CASE_NAMES=${cached#* } @@ -797,13 +797,14 @@ validate_outgoing_paths() { fi err=$(mktemp "${TMPDIR:-/tmp}/brain-sync-outgoing.XXXXXX") || return 0 if ! out=$(python3 "$SCRIPT_DIR/gstack-brain-sync-index.py" --outgoing "$GSTACK_HOME" 2>"$err"); then - ST_CASE_NAMES=$out + ST_CASE_NAMES=${out%$'\r'} write_status "error" "outgoing artifact path validation refused: a local commit adds a path that differs only by letter case from a path on the remote; local commits and queue preserved. Repair: https://github.com/garrytan/gstack/blob/main/docs/troubleshooting.md#artifact-case-collision" cat "$err" >&2 rm -f "$err" return 1 fi rm -f "$err" + out=${out%$'\r'} printf '%s\t%s\n' "$key" "$out" > "$cache" 2>/dev/null || true [ -n "$ST_CASE_NAMES" ] || ST_CASE_NAMES=$out } @@ -815,6 +816,8 @@ hold_case_collisions() { local paths_file="$1" out tr -d '\r' < "$paths_file" > "$paths_file.lf" && mv -f "$paths_file.lf" "$paths_file" || return 1 out=$(python3 "$SCRIPT_DIR/gstack-brain-sync-index.py" --hold "$GSTACK_HOME" "$paths_file" "$paths_file.case") || return 1 + out=${out%$'\r'} + tr -d '\r' < "$paths_file.case" > "$paths_file.case.lf" && mv -f "$paths_file.case.lf" "$paths_file.case" || return 1 ST_CASE_HELD=$(grep -c . "$paths_file.case" 2>/dev/null || true) ST_CASE_HELD=${ST_CASE_HELD:-0} [ "$ST_CASE_HELD" -gt 0 ] || return 0 @@ -1032,6 +1035,7 @@ publish_removals() { git --literal-pathspecs -C "$GSTACK_HOME" -c user.email="gstack@localhost" -c user.name="gstack-brain-sync" \ commit -q -m "sync: remove $n file(s) | $(date -u +%Y-%m-%dT%H:%M:%SZ)" --pathspec-from-file="$list" >/dev/null 2>&1 || return 1 guard_unpublished_range || return 2 + validate_outgoing_paths || return 2 GSTACK_HOME="$GSTACK_HOME" _receipted_git closed brain-sync "$(remote_host)" curated-memory-git-push "artifacts_sync_mode!=off" \ bash -c 'GIT_TERMINAL_PROMPT=0 git -c http.lowSpeedLimit=1024 -c http.lowSpeedTime=30 -C "$1" push origin HEAD 2>/dev/null' _ "$GSTACK_HOME" || return 2 date -u +%Y-%m-%dT%H:%M:%SZ > "$LAST_PUSH_FILE" diff --git a/bin/gstack-brain-sync-index.py b/bin/gstack-brain-sync-index.py index 7b6ac53d9..48c4a1ace 100644 --- a/bin/gstack-brain-sync-index.py +++ b/bin/gstack-brain-sync-index.py @@ -90,7 +90,8 @@ def hold(root, requested_file, held_file): held.add(path) colliding.append((key.count('/'), key)) break - Path(held_file).write_text(''.join(p + '\n' for p in sorted(held)), encoding='utf-8') + with open(held_file, 'w', encoding='utf-8', newline='\n') as f: + f.writelines(p + '\n' for p in sorted(held)) return groups[min(colliding)[1]] if colliding else () diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 589de0dbb..23cb3d1d7 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -641,7 +641,7 @@ machines read stale data (#3220). Sync now holds every queued file under such a path instead of staging it: the file stays queued, everything else keeps syncing, and the status is `collision` rather than `ok`. When only one spelling is tracked, that spelling keeps syncing and only the new alias is -held. `` are the colliding folder or file names; `gstack-brain-sync +held; when neither is tracked yet, both wait. `` are the colliding folder or file names; `gstack-brain-sync --status` lists the held paths in `message`. A local commit that would push a new alias to the remote (two machines created both spellings at once) is refused with status `error` and kept locally. @@ -658,28 +658,37 @@ unaffected; only the sync holds files. 2. Fold the other spelling's tracked files into the kept one. Read both spellings from git, because a Mac's working tree shows only one of them, and the working files too, because they hold the held, unsynced writes. - For JSONL logs, concatenate and drop duplicate lines: + This loop folds JSONL logs (concatenated, duplicate lines dropped) and + prints every other file for you to decide: ```bash cd ~/.gstack keep=projects/MyApp; drop=projects/myapp git -c core.ignorecase=false ls-files -- "$drop" | while IFS= read -r f; do rel=${f#"$drop"/} + case "$rel" in *.jsonl) ;; *) echo "decide by hand: $f"; continue ;; esac { git show ":$keep/$rel"; git show ":$f"; cat "$keep/$rel" "$f"; } 2>/dev/null | awk '!seen[$0]++' > "$HOME/.gstack-fold.tmp" mkdir -p "$keep/$(dirname "$rel")" && mv "$HOME/.gstack-fold.tmp" "$keep/$rel" git -c core.ignorecase=false add -f -- "$keep/$rel" git -c core.ignorecase=false rm -q --cached -- "$f" done + ``` + + For each file it prints (Markdown, JSON snapshots such as + `decisions.active.json`), compare `git show ":$keep/"` with + `git show ":$drop/"`, write the content to keep to `"$keep/"`, + then run the same `git -c core.ignorecase=false add -f -- "$keep/"` + and `git -c core.ignorecase=false rm -q --cached -- "$drop/"`. Never + concatenate a JSON snapshot. Then commit and push: + + ```bash git -c core.ignorecase=false commit -m "repair: fold $drop into $keep" git push origin HEAD ``` - For Markdown and other non-log files that exist under both spellings, - compare `git show ":$keep/"` with `git show ":$drop/"` and keep - the right content instead of concatenating. On Linux the two folders are - separate on disk: remove the dropped folder after the commit - (`rm -r "$drop"`). On a Mac or Windows they are the same folder, so do not - remove anything. + On Linux the two folders are separate on disk: remove the dropped folder + after the commit (`rm -r "$drop"`). On a Mac or Windows they are the same + folder, so do not remove anything. 3. Clear the held records of the dropped spelling and queue the kept files: `gstack-brain-sync --skip-file ` for each held path that `--status` lists, then `gstack-brain-enqueue ` and diff --git a/test/brain-sync.test.ts b/test/brain-sync.test.ts index cb0aad222..85a812262 100644 --- a/test/brain-sync.test.ts +++ b/test/brain-sync.test.ts @@ -743,6 +743,35 @@ describe('gstack-brain-sync per-file hold', () => { expect(spoolText()).toContain(WS_LOWER); }); + test('publish-removals refuses to push a retained commit that adds a case alias', () => { + init(); + const victim = 'projects/other/ceo-plans/remove.md'; + seedIndexedPaths(['projects/P/ceo-plans/old.md', victim]); + write('projects/P/ceo-plans/old.md', '# historical artifact\n', false); + const alias = 'projects/p/ceo-plans/local.md'; + write(alias, '# unsafe local alias\n', false); + expect(git(['add', '-f', '--', alias]).status).toBe(0); + expect(git(['-c', 'user.name=fixture', '-c', 'user.email=fixture@example.com', 'commit', '-qm', 'retained local alias']).status).toBe(0); + expect(drain().status).toBe(0); + expect(statusJson().status).toBe('error'); + expect(run(['gstack-brain-sync', '--publish-removals', '--yes']).status).toBe(1); + expect(remoteFiles()).not.toContain(alias); + expect(remoteFiles()).toContain(victim); + }); + + test('the cached outgoing verdict is re-checked when the remote baseline moves', () => { + init(); + const initial = git(['rev-parse', 'HEAD']).stdout.trim(); + seedIndexedPaths(['projects/MyApp/ceo-plans/a.md', 'projects/myapp/ceo-plans/b.md']); + expect(drain().status).toBe(0); + expect(statusJson().status).not.toBe('error'); + expect(git(['--git-dir=' + bareRemote, 'update-ref', 'refs/heads/main', initial]).status).toBe(0); + expect(git(['fetch', '-q', 'origin']).status).toBe(0); + expect(drain().status).toBe(0); + expect(statusJson().status).toBe('error'); + expect(statusJson().case_names).toBe('MyApp,myapp'); + }); + test('a tracked spelling keeps syncing while only the new case alias is held', () => { init(); seedIndexedPaths(['projects/MyApp/ceo-plans/a.md']);