Skip to content

fix(sync-workspace): refuse push/pull when .git present but sync uninit (Lucy's Bug #4) - #1483

Merged
qingyun-wu merged 1 commit into
staging-workspace-revampfrom
fix/sync-workspace-uninit-guard
Jun 6, 2026
Merged

fix(sync-workspace): refuse push/pull when .git present but sync uninit (Lucy's Bug #4)#1483
qingyun-wu merged 1 commit into
staging-workspace-revampfrom
fix/sync-workspace-uninit-guard

Conversation

@qingyun-wu

@qingyun-wu qingyun-wu commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes Bug #4 of 5 from Lucy's Maddy v0.8 migration report (2026-06-06 #design).

Maddy ran plain `bash scripts/sync-workspace.sh` on a workspace where `/.git` existed but the sync layer was never initialized — no `.git/info/exclude` whitelist marker, no `.sutando-vault/ws-id`. Result: `git add -A` staged the ENTIRE workspace tree (credentials, media, vendor caches) and pushed it.

Two-tier guard

Tier 1 (already present): `.git` presence check at the top of `_pull_only_impl` + `_push_only_impl`. Dies with `is not a git repo; run --init first` when `.git` is absent.

Tier 2 (this fix): `_assert_sync_initialized` runs AFTER the tier-1 check. Accepts EITHER sentinel as proof of real init:

If neither is present:

```
push-only: has .git but sync was never initialized (no whitelist marker in .git/info/exclude and no .sutando-vault/ws-id). Refusing to push — git add -A here would commit the WHOLE workspace tree with NO carrier-set filter. Run: bash scripts/sync-workspace.sh --init (or set SUTANDO_SYNC_SKIP_INIT_GUARD=1 to bypass at your own risk)
```

Override

`SUTANDO_SYNC_SKIP_INIT_GUARD=1` for operators on a legacy pre-marker init.

Tests

`tests/sync-workspace-uninit-guard.test.sh`:

  • 7 structural assertions: helper declared, called in both impls, override honored, marker grep present, ws-id grep present, guard runs AFTER the .git presence check (so .git-absent case still dies with the right tier-1 message).
  • 4 function-logic E2E: uninit fails, marker passes, ws-id passes, override bypasses.

Function extracted into a test-local shell to avoid sourcing the script (which would trigger the dispatcher + acquire the global `/tmp/sync-workspace.lock.d` lock — would collide with any concurrent host cron). The structural tests prove the wiring; the function-logic tests prove the logic.

ALL TESTS PASS.

Diff stats

2 files changed, +165 lines (36 in sync-workspace.sh, 129 test).

Related

v0.3.0 milestone table

Milestone Status
M0 (in-repo workspace default) ✓ merged
M1 (auto-recovery / runtime safeguards) ✓ merged
M2 (per-runtime CLAUDE_CONFIG_DIR) ✓ merged
M3 (v0.3.0 cutover) tracked in #1472

Test plan

  • Read the new `_assert_sync_initialized` helper — confirm it accepts both sentinels
  • Run `bash tests/sync-workspace-uninit-guard.test.sh` locally — confirm ALL TESTS PASS
  • On a workspace with sync properly initialized, confirm plain run still works (regression check)
  • On a workspace with stray .git (no marker, no ws-id), confirm plain run refuses with the expected error

🤖 Generated with Claude Code

@qingyun-wu @claude
fix(sync-workspace): refuse push/pull when .git present but sync unin…
09fe11d 
…it (Lucy's Bug #4)

Lucy's Maddy v0.8 migration report (2026-06-06 #design): plain
`bash scripts/sync-workspace.sh` silent-committed the entire workspace
tree on a Maddy install whose <workspace>/.git existed but where
sync was never properly initialized — no `.git/info/exclude` whitelist
marker, no `.sutando-vault/ws-id`. `git add -A` then staged every file
including credentials, media, vendor caches.

Two-tier guard:

1. **First tier (already present)**: `.git` directory presence check at
   the top of `_pull_only_impl` + `_push_only_impl`. Dies with
   "is not a git repo; run --init first" when `.git` is absent.

2. **Second tier (this fix)**: `_assert_sync_initialized` runs AFTER
   the `.git` check. Accepts EITHER sentinel as proof of real init:
   - `.git/info/exclude` contains "Generated by sync-workspace.sh" marker
   - `.sutando-vault/ws-id` exists (PR #1459)

   If neither is present, dies with a clear error pointing to `--init`:
   "$WORKSPACE_DIR has .git but sync was never initialized (no whitelist
   marker ... and no .sutando-vault/ws-id). Refusing to push — git add -A
   here would commit the WHOLE workspace tree with NO carrier-set filter."

Override for operators on legacy pre-marker inits:
`SUTANDO_SYNC_SKIP_INIT_GUARD=1`

Tests: `tests/sync-workspace-uninit-guard.test.sh`
- 7 structural assertions: helper declared, called in both impls,
  override honored, marker + ws-id grep present, guard runs AFTER
  the .git presence check (right error message on .git-absent)
- 4 function-logic E2E: uninit fails, marker passes, ws-id passes,
  override bypasses. Function extracted to a test-local shell to
  avoid sourcing the script (which would trigger dispatcher +
  acquire the global lock).

Bug #4 of 5 from Lucy's Maddy migration report.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sonichi
sonichi approved these changes Jun 6, 2026
View reviewed changes

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Two-tier guard is the right design: SKIP_INIT_GUARD bypass → exclude-with-signature check → ws-id check → die with actionable error. The die message names the exact safety risk ('git add -A would commit the WHOLE workspace tree with NO carrier-set filter') and the exact fix ('run --init'). Lucy validated all 3 cases including a real hit on Studio's actual workspace (.git without init markers). CI pending but logic is solid. Stand: Echo Act IV Pro

@qingyun-wu
qingyun-wu merged commit 60c31ab into staging-workspace-revamp Jun 6, 2026
4 checks passed
@qingyun-wu
qingyun-wu deleted the fix/sync-workspace-uninit-guard branch June 6, 2026 03:54
@qingyun-wu qingyun-wu mentioned this pull request Jun 6, 2026
Merged
4 tasks
@sonichi sonichi mentioned this pull request Jun 6, 2026
Merged
7 tasks
qingyun-wu added a commit that referenced this pull request Jun 6, 2026
@qingyun-wu @claude
docs(release-prep): fold Chi's CHANGELOG additions + KNOWN_ISSUES ope…
f078456 
…rator checklist + workspace-sync node_modules

Per Chi's PR #1472 comments (#design 2026-06-06):

**CHANGELOG.md v0.3.0 additions:**
- Fixed → Migration subsection: Lucy's Maddy v0.8 5-bug sweep (#1475 #1478 #1482 #1483 #1484)
- Fixed → Cross-platform: #1476 (uname -s branch for preflight byte count)
- Fixed → Security: #1452 #1462 #1466 (execSync → execFileSync sweep)
- Added: #1467 (bridges inject skill instructions into owner task files)
- Workspace contract rollup → M1: also reference #1473 (pre-flight scan + per-file progress reporter)

**KNOWN_ISSUES.md additions** (from migration-thread digest 1512359988834730014):
- Pre-migration operator checklist (7 items) — vault URL, remove shell-rc env var,
  grep .git/info/exclude, confirm wsId, stagger hosts, restart services,
  post-migration sanity check
- Identity-collision rule — exactly one host at a time can hold a Discord token
  (also applies to Twilio + other per-identity creds)

**docs/workspace-sync.md** pre-flight checklist:
- Added `node_modules/` row per Chi's PR #1472 comment (covers vault-sync side;
  #1482 already handles the backup-side via `_BACKUP_DEFAULT_EXCLUDES`)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
qingyun-wu added a commit that referenced this pull request Jun 6, 2026
@qingyun-wu @claude
docs(sweep round 2 cont'd) + fix(task-bridge): CI host-cli check + Mi…
85b4843 
…ni's 3 doc catches

Two concerns, one commit per owner directive (task-1780750027567 +
task-1780749875243):

### CI fix (host-cli-dependency-snapshot)

`src/task-bridge.ts:261` hardcoded `process.env.CLAUDE_CONFIG_DIR ||
join(homedir(), '.claude')` — anti-pattern caught by
`tests/host-cli-dependency-snapshot.test.py`. Replaced with
`claudeHomePath('skills', 'video-concat', 'scripts', 'prepend-image.sh')`
from `src/util_paths.ts`, which already handles
$CLAUDE_CONFIG_DIR → $CLAUDE_HOME → ~/.claude/ precedence with the
documented deprecation warning. Also dropped the now-unused `homedir`
import.

### Mini's 3 catches on PR #1472 docs (#design 2026-06-06 12:44Z)

3 of Mini's 4 catches; #1 (`KNOWN_ISSUES.md` L7 contradiction) already
shipped in `102d318`.

#2 — `docs/workspace-sync.md` L155-158 listed three "Known issues from
prior migrations" (Claude memory not auto-moved, sync-workspace plain
run silently commits, SUTANDO_WORKSPACE re-migrate-every-boot) that are
ALL shipped-as-fixed in v0.3.0 per the CHANGELOG (#1475, #1483, #1478
respectively). Section relabeled "What used to break (resolved in
v0.3.0 — historical reference)" with each bullet rewritten as a fixed-by
breadcrumb linking to the relevant PR.

#3 — `docs/workspace-sync.md` L60-62 framed sync rules as living in
the workspace's `.gitignore` with `.git/info/exclude` as parenthetical
alternative. Per #1460, the carrier writes its sync rules to
`.git/info/exclude` EXCLUSIVELY — `.gitignore` is operator-tracked and
must stay clean of carrier-set rules. Flipped the framing: lead with
`.git/info/exclude`, with explanation of why `.gitignore` is the wrong
place.

#4 — `docs/workspace-sync.md` L146 said `bash src/health-check.py` —
the file is Python, not shell. Changed to `python3 src/health-check.py`.

Stand: Echo Act IV (Mini) ready to LGTM once these land.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qingyun-wu qingyun-wu mentioned this pull request Jun 8, 2026
Merged
qingyun-wu added a commit that referenced this pull request Jun 8, 2026
@qingyun-wu @claude
fix(sync-workspace): isolate --init so it can't hijack a parent repo …
40c7777 
…(submodule case) (#1546)

Two compounding bugs let `sync-workspace.sh --init` operate on a PARENT
repo's worktree when the workspace is nested inside another git repo
(e.g. a git submodule):

1. generate_exclude ran `mkdir -p .git/info` even under --dry-run, leaving
   a STUB `.git/` (lone `info/`, no HEAD). A later --init saw `.git`
   present and skipped `git init`.
2. _init_impl decided "already a repo" from a bare `-d .git` check, which
   the stub passes. With no real repo in the workspace, git walked UP to
   the parent worktree, so every remote/add/commit/push hijacked it:
   rewrote its origin to the vault URL, committed its whole tree, and
   pushed that to the vault.

Observed on a Mac mini where the sutando checkout is a submodule of an
outer repo and the workspace is the in-repo `<repo>/workspace`. A clean
`git init` isolates fine even when nested (verified) — the bug was the
stub plus the dir-existence-only check.

Fix:
- generate_exclude no longer mkdirs under --dry-run (a dry-run must never
  mutate state).
- _init_impl decides isolation by the resolved toplevel
  (`git rev-parse --show-toplevel` -ef "$WORKSPACE_DIR"): it git-inits when
  the dir is not its own toplevel, and a post-init fail-safe `die`s if the
  fresh repo still resolves to a parent — refusing rather than corrupting
  it. This also makes the nested/submodule layout a supported config.

Complements the push/pull guard from #1483 (which covers _push_only_impl /
_pull_only_impl, not _init_impl).

Tests: tests/sync-workspace-init-isolation.test.sh — 4 structural + 6 E2E
(reproduce the stub/nested hijack, assert the parent repo is untouched).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sonichi sonichi sonichi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@qingyun-wu @sonichi