fix(sync-workspace): allow pre-wsId flat branch past init guard (closes #1961 more) by john-the-dev · Pull Request #1997 · sonichi/sutando

john-the-dev · 2026-07-07T09:33:27Z

What

_assert_sync_initialized in scripts/sync-workspace.sh recognized only two proof-of-init sentinels (.git/info/exclude marker + .sutando-vault/ws-id). A workspace initialized under the pre-#1459 scheme sits on a flat host/<hostname> branch with neither sentinel, causing the guard to abort — before _migrate_flat_branch can run to upgrade that state.

Add a third sentinel: if HEAD is on the flat host/<hostname> branch (no wsId segment), allow the sync to proceed. _migrate_flat_branch carries history into the wsId branch and _host_ws_segment writes .sutando-vault/ws-id during that same run — so the guard passes cleanly on the follow-up invocation.

Test

sync-workspace.test.sh: 88/88 pass. Test 27 (pre-wsId flat branch upgrade) was the failing case.

Why split from #1970

This is a behavior change to sync-workspace.sh, not a shell test portability fix. Split at reviewer request (qingyun-wu 2026-07-07T05:41Z) so the sync-workspace change can be reviewed on its own merits.

sonichi#1961 more) _assert_sync_initialized rejected workspaces that had a flat `host/<host>` branch (no wsId segment) because they lacked both init sentinels (.git/info/exclude marker and .sutando-vault/ws-id). This caused the existing Test 27 to abort before _migrate_flat_branch could fire — the function designed to upgrade exactly this state. Add a third sentinel: if HEAD is on `host/<hostname>` with no wsId segment, the workspace was initialized under the pre-sonichi#1459 scheme. Allow the sync to proceed; _host_ws_segment writes .sutando-vault/ws-id later in the same run, before push-only sees the guard again. sync-workspace.test.sh: 88/88 pass (Test 27 was the pre-existing failure). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QB3qZQrwponfum2JFNvHeh

github-actions · 2026-07-07T09:33:50Z

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).

qingyun-wu · 2026-07-08T14:45:50Z

Automated review

[blocker] scripts/sync-workspace.sh:662-668 reintroduces the unsafe push path this guard is meant to stop. With this branch-name bypass, any workspace repo that happens to be on host/<hostname> is treated as "initialized" even when neither the generated whitelist marker nor .sutando-vault/ws-id exists. In that state _push_only_impl can still run git add -A over the whole workspace and commit secrets/media exactly like the original bug. The flat-branch exception needs an additional proof that this is a real pre-wsId sync repo, not just a matching branch name.

qingyun-wu · 2026-07-08T15:06:30Z

+    # will write .sutando-vault/ws-id before push-only runs — so this is safe to allow.
+    local _cur_branch
+    _cur_branch="$(git -C "$WORKSPACE_DIR" symbolic-ref --short HEAD 2>/dev/null || true)"
+    if [ "$_cur_branch" = "host/$(_host)" ]; then


Automated review: this new branch-name fast path weakens the init guard enough to reintroduce the "commit the whole workspace" failure mode it was added to prevent. An unrelated git repo (or any half-initialized workspace) can already have HEAD on host/<hostname> without ever having the generated .git/info/exclude or a ws-id; in that case _push_only_impl now proceeds to git add -A with no carrier-set filter. I think this should only bypass the guard if the flat branch is accompanied by some sync-owned marker, or after _migrate_flat_branch has proved it is a real pre-wsId vault repo.

qingyun-wu · 2026-07-08T15:40:13Z

Automated review:

scripts/sync-workspace.sh:662 makes host/<hostname> by itself a valid init sentinel. That re-opens the exact failure mode the guard was added to prevent: a stray repo with no generated .git/info/exclude and no .sutando-vault/ws-id can now bypass _assert_sync_initialized() just by being on that branch name, and the later git add -A will stage the whole workspace with no carrier-set filter. The branch name alone does not prove that _init_impl ever wrote the whitelist or ws-id, so this needs one more durable sentinel before allowing push/pull.

qingyun-wu · 2026-07-09T20:29:23Z

Automated mergeability review (2026-07-09)

Signal: NOT MERGEABLE

Current blocker(s):

Branch is behind current main; update/merge main and rerun checks before merging.

This is a conservative backlog triage signal. After the blockers are cleared, please rerun checks and request a focused review.

…fast path Review fix (qingyun-wu, PR sonichi#1997): the branch-name fast path in _assert_sync_initialized returned 0 whenever HEAD was on host/<hostname>, without checking any init marker. A half-initialized repo — HEAD parked on host/<hostname> but lacking both .git/info/exclude and .sutando-vault/ws-id — could slip past the guard into `git add -A`, the "commit the whole workspace" failure mode the guard exists to prevent. Tighten the fast path: still require a real whitelist to exist first — either the .git/info/exclude generator marker OR the pre-sonichi#1459 in-tree .gitignore `*` deny-all (that era's whitelist, which a genuine pre-wsId workspace carries — exactly what _migrate_flat_branch upgrades). If no whitelist is present, fall through to die(). This preserves the legitimate pre-wsId migration (Test 27, still 88/88) while closing the half-init hole. Updated the uninit-guard inline mirror to match and added T12 (flat branch + whitelist passes) / T13 (flat branch, no whitelist, blocked — the reviewer's exact case). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFp2p6XUSAQ1eAKKZeiZPt

github-actions · 2026-07-09T21:22:44Z

@cla-assistant check

qingyun-wu

Delta review (commit b5b0870 since my 2026-07-08 pass): LGTM — the fail-open concern is closed.

Prior point addressed: the branch-name fast path no longer lets a half-initialized repo (HEAD parked on host/<hostname>, no carrier-set filter) ride into git add -A. The fast path now additionally requires whitelist evidence — the generated .git/info/exclude marker or the pre-#1459 in-tree .gitignore deny-all (scripts/sync-workspace.sh:671-685). That is exactly the fail-closed shape I asked for, and it still admits the genuine pre-wsId layout (which always carries the * deny-all .gitignore that _migrate_flat_branch expects). Test T13 locks the half-init rejection in and T12 covers the legitimate flat-branch pass (tests/sync-workspace-uninit-guard.test.sh:135-167).

CI: all checks green (tsc + tests, diff coverage, all three refuse-lints, CLA, recheck).

Non-blocking: _has_exclude_marker is dead code — it is only set to 1 on the path that immediately return 0s (scripts/sync-workspace.sh:660-663), so the [ "$_has_exclude_marker" = "1" ] arm of the flat-branch condition can never be true. Harmless, but it reads as if the exclude marker could reach that branch. Dropping the variable (leaving just the legacy-.gitignore check) would say what the code actually does. Same dead arm exists in the test's inline mirror.

qingyun-wu · 2026-07-11T07:42:28Z

LGTM from code review.

Verified on the current head (b5b0870775bedd9e4ada0725fe41610c7842ef0e):

bash tests/sync-workspace-uninit-guard.test.sh passed in an isolated worktree at /private/tmp/sutando-pr1997.
I also re-checked the guard logic in scripts/sync-workspace.sh:654-687: the new flat-branch fast path now only accepts host/<hostname> when a real whitelist already exists (.git/info/exclude marker or the legacy deny-all .gitignore), so the earlier branch-name-only bypass is gone.

No actionable findings on this revision. Merge once required checks pass.

Qingyun's Personal Codex Agent

liususan091219 · 2026-07-12T20:10:15Z

Closing as duplicate (covered elsewhere) — already fixed (change already on main).
This PR's change is already delivered on main via #2026 — possibly incidentally (e.g. a refactor/rename carried the same fix).
Detector + audit evidence: opened 2026-07-07, #2026 (same sync-workspace init-guard area) merged 2026-07-08 AFTER it — correct date order; embedding similarity 0.674; codex judge "already_fixed".
Nothing is lost by closing — the covering change is already merged. If you believe #2026 does NOT fully cover this PR, please comment with the gap and reopen.

close-reason: duplicate/already_fixed per #2026

john-the-dev mentioned this pull request Jul 7, 2026

fix(tests): fix 8 shell suites for Ubuntu CI portability (closes #1961) #1970

Merged

Merge branch 'main' into fix/sync-workspace-prewsid-init-guard

6fb1bf8

john-the-dev marked this pull request as ready for review July 8, 2026 14:37

qingyun-wu reviewed Jul 8, 2026

View reviewed changes

john-the-dev mentioned this pull request Jul 8, 2026

fix(sync-workspace): isolate test lock dir + fix test 27 init guard #2026

Merged

5 tasks

qingyun-wu reviewed Jul 10, 2026

View reviewed changes

liususan091219 closed this Jul 12, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(sync-workspace): allow pre-wsId flat branch past init guard (closes #1961 more)#1997

fix(sync-workspace): allow pre-wsId flat branch past init guard (closes #1961 more)#1997
john-the-dev wants to merge 3 commits into
sonichi:mainfrom
john-the-dev:fix/sync-workspace-prewsid-init-guard

john-the-dev commented Jul 7, 2026

Uh oh!

github-actions Bot commented Jul 7, 2026

Uh oh!

qingyun-wu commented Jul 8, 2026

Uh oh!

qingyun-wu Jul 8, 2026

Uh oh!

qingyun-wu commented Jul 8, 2026

Uh oh!

qingyun-wu commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

qingyun-wu left a comment

Uh oh!

qingyun-wu commented Jul 11, 2026

Uh oh!

liususan091219 commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

john-the-dev commented Jul 7, 2026

What

Test

Why split from #1970

Uh oh!

github-actions Bot commented Jul 7, 2026

Coverage Gate

Uh oh!

qingyun-wu commented Jul 8, 2026

Uh oh!

qingyun-wu Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

qingyun-wu commented Jul 8, 2026

Uh oh!

qingyun-wu commented Jul 9, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu commented Jul 11, 2026

Uh oh!

liususan091219 commented Jul 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants