Skip to content

feat(sync): PR-2 — vault URL from config + --vault-url flag, sync-memory.sh deprecation - #1446

Merged
qingyun-wu merged 2 commits into
staging-workspace-revampfrom
feat/sync-workspace-rename-pr2
Jun 4, 2026
Merged

qingyun-wu merged 2 commits into
staging-workspace-revampfrom
feat/sync-workspace-rename-pr2

Conversation

@qingyun-wu

@qingyun-wu qingyun-wu commented Jun 4, 2026 •

Copy link
Copy Markdown
Collaborator

Milestone status (workspace-revamp M0–M3)

Milestone Status
M0-1 (workspace structure) ✅ shipped (PR #1395)
M1-1 (migrate legacy workspace) ✅ shipped (sutando-migrate skill)
M0-2 (.claude_sutando/ in-repo) ✅ shipped
M1-2 (migrate .claude/ → .claude-sutando/) ✅ shipped
M2 (sync engine) 🟡 in-flight (this PR is part of M2)
M3 (docs sweep + dogfooding + E2E) ⏸ not yet started

PR-2 followup to #1445. Owner directive 2026-06-04: vault URL resolution should mirror how workspace path is resolved (config-file canonical), not env-var canonical. Includes deprecation banner on the legacy script (Option C: parallel deprecation, no behavior change for existing users).

Summary

  • Vault URL resolver: priority chain is now --vault-url <url> → sutando.config.local.json (vault.remote_url) → sutando.config.json (tracked default) → .env SUTANDO_MEMORY_REPO (deprecated; warn-and-honor for one release)
  • SUTANDO_VAULT env var (introduced in feat(sync-workspace): workspace-as-git-repo + memory translation layer (PR-1) #1445) removed — brand new, no users to deprecate; CLI flag + config-file is the canonical surface
  • --vault-url <url> CLI flag added (two forms: --vault-url X and --vault-url=X)
  • sync-memory.sh keeps working with a one-line deprecation banner + migration recipe in the header (silence with SUTANDO_SYNC_MEMORY_SUPPRESS_DEPRECATION=1)
  • Reference sweep (user-facing): crons.example.json + docs/memory-sync.md
  • 48/48 tests pass (was 39 in feat(sync-workspace): workspace-as-git-repo + memory translation layer (PR-1) #1445 v6); +9 new tests cover the new resolver chain, flag forms, env-var removal, priority, and deprecation banner

Why Option C (parallel deprecation, not exec-shim)

The two scripts implement different storage architectures:

  • sync-memory.sh: rsync to a separate ~/.sutando/memory-sync/ clone
  • sync-workspace.sh: workspace IS the git repo, branch-per-host topology

A exec sync-workspace.sh "$@" shim would silently auto-init users' workspaces on first cron fire — destructive. Option C keeps both functional through the transition; PR-2.1 removes sync-memory.sh after dogfooding.

Test plan

  • bash tests/sync-workspace.test.sh — expect Total: 48 — pass: 48, fail: 0
  • Smoke test --vault-url flag: bash scripts/sync-workspace.sh --vault-url <url> --status resolves cleanly
  • Smoke test config-file: write vault.remote_url into sutando.config.local.json, run --status, see URL in output
  • Smoke test legacy .env: only SUTANDO_MEMORY_REPO=... → see deprecation banner + URL resolves
  • Smoke test sync-memory.sh: invoke → see deprecation banner; set SUTANDO_SYNC_MEMORY_SUPPRESS_DEPRECATION=1 → no banner

Not in scope (PR-2.1 / later)

  • Removing sync-memory.sh entirely (waits for dogfooding observation)
  • Doc sweep of workspace-design.md / workspace-contract.md (architectural docs, out-of-scope for user-facing PR-2)
  • Internal-caller refs in health-check.py / core_heartbeat.py (those still use rsync flow; shim covers them during transition)

🤖 Generated with Claude Code

…ory.sh deprecation

PR-2 followup to #1445. Owner directive 2026-06-04: vault URL resolution
should mirror how workspace path is resolved (config-file canonical),
not env-var canonical. Includes deprecation banner on the legacy script
(Option C: parallel deprecation, no behavior change for existing users).

Changes:

1. Vault URL resolver (sync-workspace.sh)
   New priority chain replaces the env-canonical resolver:
     1. --vault-url <url> CLI flag (explicit, tests, one-shot)
     2. sutando.config.local.json → vault.remote_url (per-clone canonical)
     3. sutando.config.json → vault.remote_url (tracked default)
     4. .env SUTANDO_MEMORY_REPO (deprecated; warn-and-honor 1 release)

   SUTANDO_VAULT env var (introduced in #1445) REMOVED — brand new,
   no users to deprecate.

   .env grep-pipeline parsing replaced by direct env-var lookup
   (set -a; . .env was already loading SUTANDO_MEMORY_REPO into env
   above). Eliminates the var=$(grep | head | ...) set-e trap entirely
   (the bug class Mini caught in #1445 v4).

2. --vault-url CLI flag
   Two forms supported: `--vault-url <url>` and `--vault-url=<url>`.
   Error message + exit 2 when value missing.

3. sync-memory.sh deprecation banner
   One-line stderr warning on every invocation pointing at
   sync-workspace.sh + migration recipe in header. Silence with
   SUTANDO_SYNC_MEMORY_SUPPRESS_DEPRECATION=1. Functionality
   unchanged — existing users' crons keep working through transition.

4. Reference sweep (user-facing)
   - skills/schedule-crons/crons.example.json: sync-memory entry
     renamed to sync-workspace pointing at the new script
   - docs/memory-sync.md: PR-2 deprecation banner + migration recipe
     at the top; rest of the doc unchanged for legacy-flow users

5. Tests (48 pass total, was 39)
   - Tests 1-12 migrated to pass vault via --vault-url flag (was
     SUTANDO_VAULT env var via COMMON_ENV)
   - Test 13 (legacy alias): repurposed — verifies SUTANDO_MEMORY_REPO
     in .env still warn-and-honors
   - Test 14 (--vault-url flag): NEW — verifies flag resolves vault
     without any .env or env var
   - Test 15 (config-file vault): NEW — vault.remote_url in
     sutando.config.local.json resolves correctly
   - Test 16 (SUTANDO_VAULT removed): NEW — negative test, env var
     no longer honored
   - Test 17 (priority): NEW — --vault-url wins over .env legacy
   - Test 18 (deprecation banner): NEW — sync-memory.sh prints
     deprecation banner; SUPPRESS env var silences it

Not in scope (PR-2.1 / later):
- Removing sync-memory.sh entirely (waits for dogfooding observation)
- Doc sweep of workspace-design.md / workspace-contract.md
  (architectural docs; out-of-scope for user-facing PR-2)
- Removing health-check.py / core_heartbeat.py SUTANDO_MEMORY_REPO refs
  (same — those are internal callers using rsync flow; shim covers)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t 19

Pro's PR #1446 hold: "is sutando-config.sh vault-url implemented, or
does the helper silently return empty and fall through to deprecated
.env?" Pro couldn't see sutando-config.sh in the PR diff (helper
already existed in main from an earlier M2 PR).

Answer: vault-url IS implemented, and Test 15 already covers the
happy path (write vault.remote_url to config + assert resolves).

New Test 19 closes the explicit "silent-no-op" concern: stage a
config WITH NO vault.remote_url, NO --vault-url flag, only .env
SUTANDO_MEMORY_REPO. Verifies:

  1. sutando-config.sh vault-url returns empty (helper IS callable,
     just yields empty for absent field)
  2. --status exits 0 (no crash on empty config)
  3. Deprecation warning fires (correct fall-through path taken)
  4. Vault URL resolves via .env legacy

Proves the silent-no-op gracefully degrades — not a crash, not a loop,
correct priority ordering when each tier is empty/missing.

Tests: 48 → 52 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

@cla-assistant check

qingyun-wu added a commit that referenced this pull request Jun 4, 2026
…e/exclude)

PR-3 followup to #1445 + #1446. M2 milestone. Owner directive 2026-06-04:
let users customize what gets synced without editing the script. The
config schema already declared `vault.sync.{include,exclude}` (M2 design)
but PR-1's gitignore generator ignored it and used a hardcoded list.
PR-3 wires the config field into the gitignore generator.

Changes:

1. scripts/sutando-config.sh — new helpers
   `vault-sync-include` + `vault-sync-exclude` print the config arrays
   one path per line. Consumed by the gitignore composer.

2. scripts/sync-workspace.sh — config-driven carrier
   `_compose_gitignore_content` no longer hardcodes the carrier set.
   New `_emit_include_lines` helper handles ancestor-chain un-ignore
   (gitignore can't include a child whose ancestor is excluded by `*`).
   New `_emit_exclude_lines` helper emits carve-out denies after the
   includes (gitignore last-match wins).

   Paths ending in `/` treated as directories (ancestor chain + `!path/**`);
   paths without trailing `/` treated as files (ancestor chain + `!path`).

3. sutando.config.json — defaults updated to match real carrier
   Tracked default now declares the actual sync-workspace.sh carrier:
   `notes/`, `pending-questions.md`, `build_log.md`,
   `.claude-sutando/projects/*/memory/`. Previously the defaults claimed
   to track `skills/`, `tasks/archive/`, `state/cores/` etc., none of
   which sync-workspace.sh actually consumed — the schema lied. PR-3
   reconciles schema with reality. Empty `exclude` default.

   Also fixed: prior hardcoded `!build_log/` (directory) was wrong;
   the build log is `build_log.md` (file).

4. Semantics: REPLACE, not concat
   sutando.config.local.json's `sync.include` REPLACES the tracked
   default's array (standard JSON merge — arrays don't concat). Users
   wanting defaults + extras must include the defaults explicitly.
   Documented in Test 19 + on the helpers.

Tests: 48 → 54 pass. Three new tests:
- Test 19: defaults + custom include all emitted to .gitignore
- Test 20: include `data/` + exclude `data/secret/` → `data/foo.md`
  visible to git, `data/secret/key.txt` gitignored (verified with
  `git check-ignore`, not by parsing `git status` output)
- Test 21: ancestor-chain un-ignore for nested include `a/b/c/`
  emits `!a/`, `!a/b/`, `!a/b/c/`, `!a/b/c/**`

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qingyun-wu
qingyun-wu marked this pull request as ready for review June 4, 2026 06:45

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: LGTM — clean, consistent, and it folds in the #1445 review feedback. COMMENT (not changing your formal state).

This is exactly the right shape for PR-2: the vault URL now resolves through the same config-canonical chain the workspace path uses, so there's one mental model for "where does Sutando get its locations." Nice.

Things I like:

  1. The 4-tier resolver mirrors the workspace-path contract (--vault-url → sutando.config.local.json → sutando.config.json → .env SUTANDO_MEMORY_REPO). Config-canonical, env-deprecated — consistent with resolve_workspace, so nobody has to learn a second resolution order.
  2. SUTANDO_VAULT removed cleanly. It was introduced one PR ago (#1445) with zero users, so deleting it now (rather than carrying a deprecation cycle for a never-shipped var) keeps the surface honest. Good call to not accrete legacy.
  3. The set-e trap elimination — relying on the already-set -a; . .env-loaded SUTANDO_MEMORY_REPO env var instead of a fresh var=$(grep … | head | cut …) is the right fix for Mini's #1445 v4 Medium: it removes the whole "grep-returns-nonzero-under-set-e" + path-shaped-capture failure class, not just one instance.
  4. Both flag forms (--vault-url X via _consume_next and --vault-url=X via prefix) — covers the two ways people actually type it.
  5. Deprecation is Option-C parallel — sync-memory.sh keeps working with a one-line, suppressible banner + an inline migration recipe. No behavior change for existing users; they're nudged, not broken.

Non-blocking observations:

  • Priority 2/3 delegates to scripts/sutando-config.sh vault-url with 2>/dev/null || true — good that a missing/!malformed config degrades to the next tier rather than aborting. Worth a one-line test asserting "no config + no flag + no env → empty VAULT_URL → clean error", if not already in the 48.
  • The legacy .env warn-and-honor message fires every invocation while SUTANDO_MEMORY_REPO is set — fine for one release, but consider gating it behind the same _SUPPRESS_DEPRECATION knob so a user mid-migration isn't spammed twice (sync-memory banner + this one).
  • crons.example.json prompt now points at sync-workspace.sh — good catch updating the user-facing reference in the same PR rather than leaving it dangling.

Cross-PR linkage: sits cleanly between #1445 (PR-1: workspace-as-git-repo + the carrier .gitignore) and #1447 (PR-3: config-driven carrier). My one carry-over concern from the #1445 review — the !notes/** wholesale carrier tracking sensitive personal content — isn't touched here (it's PR-1/PR-3 territory); flagging only so it isn't lost in the stack.

Tests: 48/48 (was 39) with +9 for the resolver chain + flag forms — good coverage of the new surface.

Ready to merge from my read once the stack lands in order. Workspace-contract area, so final go/no-go is the owners'.

— Lucy (Mac Studio bot)

@qingyun-wu
qingyun-wu merged commit 5b51dde into staging-workspace-revamp Jun 4, 2026
5 checks passed
@qingyun-wu
qingyun-wu deleted the feat/sync-workspace-rename-pr2 branch June 4, 2026 09:19
qingyun-wu added a commit that referenced this pull request Jun 4, 2026
…e/exclude) (#1447)

* feat(sync): PR-2 — vault URL from config + --vault-url flag, sync-memory.sh deprecation

PR-2 followup to #1445. Owner directive 2026-06-04: vault URL resolution
should mirror how workspace path is resolved (config-file canonical),
not env-var canonical. Includes deprecation banner on the legacy script
(Option C: parallel deprecation, no behavior change for existing users).

Changes:

1. Vault URL resolver (sync-workspace.sh)
   New priority chain replaces the env-canonical resolver:
     1. --vault-url <url> CLI flag (explicit, tests, one-shot)
     2. sutando.config.local.json → vault.remote_url (per-clone canonical)
     3. sutando.config.json → vault.remote_url (tracked default)
     4. .env SUTANDO_MEMORY_REPO (deprecated; warn-and-honor 1 release)

   SUTANDO_VAULT env var (introduced in #1445) REMOVED — brand new,
   no users to deprecate.

   .env grep-pipeline parsing replaced by direct env-var lookup
   (set -a; . .env was already loading SUTANDO_MEMORY_REPO into env
   above). Eliminates the var=$(grep | head | ...) set-e trap entirely
   (the bug class Mini caught in #1445 v4).

2. --vault-url CLI flag
   Two forms supported: `--vault-url <url>` and `--vault-url=<url>`.
   Error message + exit 2 when value missing.

3. sync-memory.sh deprecation banner
   One-line stderr warning on every invocation pointing at
   sync-workspace.sh + migration recipe in header. Silence with
   SUTANDO_SYNC_MEMORY_SUPPRESS_DEPRECATION=1. Functionality
   unchanged — existing users' crons keep working through transition.

4. Reference sweep (user-facing)
   - skills/schedule-crons/crons.example.json: sync-memory entry
     renamed to sync-workspace pointing at the new script
   - docs/memory-sync.md: PR-2 deprecation banner + migration recipe
     at the top; rest of the doc unchanged for legacy-flow users

5. Tests (48 pass total, was 39)
   - Tests 1-12 migrated to pass vault via --vault-url flag (was
     SUTANDO_VAULT env var via COMMON_ENV)
   - Test 13 (legacy alias): repurposed — verifies SUTANDO_MEMORY_REPO
     in .env still warn-and-honors
   - Test 14 (--vault-url flag): NEW — verifies flag resolves vault
     without any .env or env var
   - Test 15 (config-file vault): NEW — vault.remote_url in
     sutando.config.local.json resolves correctly
   - Test 16 (SUTANDO_VAULT removed): NEW — negative test, env var
     no longer honored
   - Test 17 (priority): NEW — --vault-url wins over .env legacy
   - Test 18 (deprecation banner): NEW — sync-memory.sh prints
     deprecation banner; SUPPRESS env var silences it

Not in scope (PR-2.1 / later):
- Removing sync-memory.sh entirely (waits for dogfooding observation)
- Doc sweep of workspace-design.md / workspace-contract.md
  (architectural docs; out-of-scope for user-facing PR-2)
- Removing health-check.py / core_heartbeat.py SUTANDO_MEMORY_REPO refs
  (same — those are internal callers using rsync flow; shim covers)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(sync-workspace): address Pro #1446 silent-no-op concern with Test 19

Pro's PR #1446 hold: "is sutando-config.sh vault-url implemented, or
does the helper silently return empty and fall through to deprecated
.env?" Pro couldn't see sutando-config.sh in the PR diff (helper
already existed in main from an earlier M2 PR).

Answer: vault-url IS implemented, and Test 15 already covers the
happy path (write vault.remote_url to config + assert resolves).

New Test 19 closes the explicit "silent-no-op" concern: stage a
config WITH NO vault.remote_url, NO --vault-url flag, only .env
SUTANDO_MEMORY_REPO. Verifies:

  1. sutando-config.sh vault-url returns empty (helper IS callable,
     just yields empty for absent field)
  2. --status exits 0 (no crash on empty config)
  3. Deprecation warning fires (correct fall-through path taken)
  4. Vault URL resolves via .env legacy

Proves the silent-no-op gracefully degrades — not a crash, not a loop,
correct priority ordering when each tier is empty/missing.

Tests: 48 → 52 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(sync): PR-3 — config-driven gitignore carrier (vault.sync.include/exclude)

PR-3 followup to #1445 + #1446. M2 milestone. Owner directive 2026-06-04:
let users customize what gets synced without editing the script. The
config schema already declared `vault.sync.{include,exclude}` (M2 design)
but PR-1's gitignore generator ignored it and used a hardcoded list.
PR-3 wires the config field into the gitignore generator.

Changes:

1. scripts/sutando-config.sh — new helpers
   `vault-sync-include` + `vault-sync-exclude` print the config arrays
   one path per line. Consumed by the gitignore composer.

2. scripts/sync-workspace.sh — config-driven carrier
   `_compose_gitignore_content` no longer hardcodes the carrier set.
   New `_emit_include_lines` helper handles ancestor-chain un-ignore
   (gitignore can't include a child whose ancestor is excluded by `*`).
   New `_emit_exclude_lines` helper emits carve-out denies after the
   includes (gitignore last-match wins).

   Paths ending in `/` treated as directories (ancestor chain + `!path/**`);
   paths without trailing `/` treated as files (ancestor chain + `!path`).

3. sutando.config.json — defaults updated to match real carrier
   Tracked default now declares the actual sync-workspace.sh carrier:
   `notes/`, `pending-questions.md`, `build_log.md`,
   `.claude-sutando/projects/*/memory/`. Previously the defaults claimed
   to track `skills/`, `tasks/archive/`, `state/cores/` etc., none of
   which sync-workspace.sh actually consumed — the schema lied. PR-3
   reconciles schema with reality. Empty `exclude` default.

   Also fixed: prior hardcoded `!build_log/` (directory) was wrong;
   the build log is `build_log.md` (file).

4. Semantics: REPLACE, not concat
   sutando.config.local.json's `sync.include` REPLACES the tracked
   default's array (standard JSON merge — arrays don't concat). Users
   wanting defaults + extras must include the defaults explicitly.
   Documented in Test 19 + on the helpers.

Tests: 48 → 54 pass. Three new tests:
- Test 19: defaults + custom include all emitted to .gitignore
- Test 20: include `data/` + exclude `data/secret/` → `data/foo.md`
  visible to git, `data/secret/key.txt` gitignored (verified with
  `git check-ignore`, not by parsing `git status` output)
- Test 21: ancestor-chain un-ignore for nested include `a/b/c/`
  emits `!a/`, `!a/b/`, `!a/b/c/`, `!a/b/c/**`

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants