Skip to content

fix(workspace-v0.8): strip $SUTANDO_WORKSPACE from resolver + colored warnings [DRAFT] - #1440

Merged
qingyun-wu merged 10 commits into
staging-workspace-revampfrom
fix/workspace-strip-env-override
Jun 4, 2026
Merged

qingyun-wu merged 10 commits into
staging-workspace-revampfrom
fix/workspace-strip-env-override

Conversation

@qingyun-wu

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

Copy link
Copy Markdown
Collaborator

Path B (workspace v0.8): strip $SUTANDO_WORKSPACE from resolver + bootstrap auto-migration

Implements the v0.8 target contract: $SUTANDO_WORKSPACE is no longer honored by src/sutando_config.{py,ts}. Workspace resolves only via sutando.config.{json,local.json} or the {repoRoot}/workspace baked-in default, per docs/workspace-contract-v0.8.md (#1384).

STATUS: DRAFT — awaiting Mini + Pro re-review after v2 safety hardening.

What's in this PR

Commit 529007d — resolver strip + colored warnings
Commit 6cf3e9d — bootstrap auto-migration + env-fallback strip
Commits 3f02c80, d005762, ed053d7 — SUTANDO_TEST_MODE escape hatch + test fixture opt-ins

Commits a0d6292 + df70545 — Mini's #design review v1 (B1+B2+B3+B4 safety hardening + tests):

  • B1 — realpath equality + _same_inode (stat -L) skip when env == resolved workspace
  • B2 — tar -czf BEFORE sutando-migrate.sh --commit; archive preserved on migrate-fail
  • B3 — _is_unsafe_for_migration deny-list (initial set: /, /usr, /etc + /private/etc, /var + /private/var, /System, /Library, /Applications, $HOME + Documents/Desktop/Downloads, $REPO + subdirs, paths with ..) applied at two points
  • B4 — _color_warn honors NO_COLOR; TS warnings drop literal ${envVal} / ${dotenvVal} / ${resolved} interpolations (c58270d parity)
  • Helpers extracted into src/migration_safety_helpers.sh for unit-testability
  • Tests: tests/migration-safety-helpers.test.sh (24 assertions) + tests/sutando-config.prod-path.test.ts (7 assertions)

Commit 0c496ef — Mini's #design review v2 (sub-blockers in v1):

  • B3 deny-list expansion — Mini flagged that v1 missed /tmp exact, descendants of $HOME/Documents/Desktop/Downloads (only exact dirs were denied), and common dotfile install targets. v2 adds:
    • /tmp + /private/tmp exact (subdirs remain safe for mktemp targets)
    • $HOME/Documents/*, $HOME/Desktop/*, $HOME/Downloads/* descendants
    • $HOME/.sutando + subpaths, EXCEPT $HOME/.sutando/workspace and its subpaths (intentional migration source — allow-exception checked first in case statement)
    • $HOME/.claude + subpaths
    • $HOME/.config + subpaths
  • B1 dev:ino — _same_inode now uses stat -L -f '%d:%i' (BSD) / -c '%d:%i' (GNU) instead of inode alone. Inode-only false-positives across unrelated filesystems that happen to reuse the same number; device:inode is the globally unique pair.
  • Python prod-path test twin — tests/sutando-config.prod-path.test.py mirrors the TS v1 test (7 assertions: NO_COLOR + non-TTY + no literal env/dotenv values + one-shot + config-driven path + baked-in default).
  • Helpers test expanded: 24 → 38 assertions (added /tmp subdir-allow, descendants of Documents/Desktop/Downloads, .sutando allow-exception via fake-realpath'd $HOME, .claude, .config).

Verification

  • bash tests/migration-safety-helpers.test.sh: 38/38 pass
  • python3 tests/sutando-config.prod-path.test.py: 7/7 pass
  • npx tsx --test tests/sutando-config.prod-path.test.ts: 7/7 pass
  • npx tsx --test tests/sutando-config.test.ts: 21/21 pass (no regression)
  • python3 tests/sutando-config.test.py: 30/30 pass (no regression)
  • bash -n on edited shells: syntax clean

Remaining work / known follow-up

  • Test-fixture migration — ~141 SUTANDO_WORKSPACE refs across 23 tests use the env as a tmp-workspace mechanism. The SUTANDO_TEST_MODE escape hatch is a transitional crutch; full migration to sutando.config.local.json fixtures + SUTANDO_REPO_DIR start-hint tracked under Path B.1b.
  • Full startup-migration integration test (Mini's "Test gaps" list, v1) — driver-level mock-migrate-and-tar end-to-end. The helpers-level test + dual-language prod-path tests cover the highest-risk units; the driver-level integration test is a separate follow-up (non-trivial setup, lower marginal value).
  • Symlink legacy dir as archive edge (Mini v2 flagged): tar -czf preserves symlink-entry by default (no -h); B1 realpath guard catches symlink→resolved up-front and skips the destructive block. Broken symlink fails the [ -d ] precondition. Documented in 0c496ef.

Cross-PR linkage

Review thread

qingyun-wu and others added 6 commits June 3, 2026 11:33
… warnings

Step 1 of Path B (single-PR strip per docs/workspace-contract-v0.8.md
target). The library-level resolver in src/sutando_config.{py,ts} no
longer honors $SUTANDO_WORKSPACE — workspace resolves only via
sutando.config.{json,local.json} or the {repoRoot}/workspace baked-in
default.

If $SUTANDO_WORKSPACE is set in the environment, the resolver fires a
ONE-TIME bold-red ANSI warning pointing at scripts/sutando-migrate.sh
for relocation of any data still living at the env-pointed path. Same
for stale .env declarations. Both warnings:

- Bold-red on TTY (operators notice + migrate); plain text on log
  captures (script / tee / journald / GitHub Actions) so escape codes
  don't pollute. NO_COLOR=1 honored per no-color.org convention.
- Omit the env value to avoid the path-shaped-warning trap (2026-06-02
  rogue-folder bug — bash tokenization on stderr re-capture).
- Direct the operator at `bash scripts/sutando-migrate.sh --dry-run`
  + `--commit` for relocation.

Next commits on this branch will add:
- src/startup.sh + src/init.sh auto-migration on bootstrap when
  $SUTANDO_WORKSPACE is set (per follow-up directive — fragile to
  expect operators to manually run sutando-migrate).
- scripts/sutando-migrate.sh --auto flag (non-interactive mode for
  bootstrap-triggered migration).
- ~141 SUTANDO_WORKSPACE refs in tests migrated to use
  sutando.config.local.json fixtures.
- Strip $SUTANDO_WORKSPACE fallback from init.sh, startup.sh,
  install-credential-proxy-launchd.sh.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 2 of Path B: bootstrap-side env handling.

**Auto-migration in src/startup.sh** (per owner directive 2026-06-03 18:31Z:
"we need to automatically trigger a migration for the user"):

After .env is sourced + validated, before init.sh runs:
- If $SUTANDO_WORKSPACE is set with data at the env-pointed path,
  invoke `bash scripts/sutando-migrate.sh --commit` to relocate into
  the v0.8 default (<repo>/workspace/).
- Sentinel `state/auth/migrated-from-env.txt` guards against re-running.
- Bold-red TTY banner so the operator notices the one-shot relocation.
- On migrate failure: abort startup (refuse to proceed with split state),
  point operator at `scripts/sutando-migrate.sh --dry-run` to diagnose.

**Compress-in-place after migration** (per owner directive 2026-06-03
18:36Z: "compress original folder in place — (1) reserve data, (2) avoid
old processes silently using it"):

After successful migrate-commit:
- `tar -czf <legacy>-pre-v0.8-<ts>.tar.gz` the original folder.
- `rm -rf` the original directory.
- Result: stale processes still pointing at the env-path now hit ENOENT
  instead of writing to a divergent directory; data is recoverable via
  `tar -xzf`.

**Belt-and-suspenders**: `unset SUTANDO_WORKSPACE` after migration so
child processes (init.sh, bridges, voice-agent, etc.) get v0.8 directly
and the resolver's deprecation nag stops firing on every subprocess
spawn.

**Strip env fallbacks** from:
- src/init.sh — was `elif [ -n "${SUTANDO_WORKSPACE:-}" ]`; now hard-fail
  if workspace_resolve.sh is unreachable. .env-drift nag updated to v0.8
  framing (env no longer honored, delete the .env line + move to
  sutando.config.local.json).
- src/workspace_resolve.sh — shared helper sourced by init.sh + several
  install-launchd scripts. Same hard-fail.
- src/install-credential-proxy-launchd.sh — same.
- src/startup.sh — workspace resolution falls back to $REPO/workspace
  (baked-in default) when the helper isn't present, no env.

Init.sh .env-drift warning now uses bold-red ANSI on TTY (matches the
resolver-side warnings in `529007d`); plain text in log captures.

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

CI was red after the resolver-strip commits (529007d, 6cf3e9d) because
~14 test files set `$SUTANDO_WORKSPACE` to redirect workspace to a
per-test tmp dir — and v0.8 ignores the env. Path B test-fixture
migration would be invasive (141 refs across 23+ files).

Cleaner path: add a private, test-only escape hatch in the resolver
that re-honors `$SUTANDO_WORKSPACE` silently when `SUTANDO_TEST_MODE=1`
is also set. The end-user v0.8 contract is unchanged: production
warns + ignores. Tests opt in explicitly.

Changes:

* src/sutando_config.{py,ts} — early-return when env+SUTANDO_TEST_MODE
  both set; warning + ignore otherwise (existing v0.8 behavior).
* tests/_helpers/temp-workspace.ts — helper sets both env vars;
  cleanup() clears both so subsequent in-process tests can exercise
  the production code path.
* tests/sutando-config.test.{py,ts} — invert the two env-precedence
  tests to assert v0.8 behavior (env ignored, local.json wins).
* tests/workspace-default.test.py — module-level setUpModule sets
  SUTANDO_TEST_MODE=1 (migration test classes redirect via env);
  resolve()-canonical equality on macOS /private symlink paths;
  inverted test confirms production-path env-ignore.
* tests/workspace-default-relative-env.test.py — per-test opt-in to
  SUTANDO_TEST_MODE.
* tests/{outbox-log,single-instance,bridges-sending-orphan-recovery}.test.py
  — setUp opt-in to SUTANDO_TEST_MODE.
* tests/{agent-state-endpoint,init-script,sutando-config-shell}.test.ts
  — spawn-env opt-in to SUTANDO_TEST_MODE.

Local test run: all 17 prior failures clear. Single pre-existing
flake unrelated to env-strip (`init.sh --preflight optional=2/8`
asserts host env state; passes on clean CI).
Owner directive 2026-06-03: this file is the operational source of truth
for v0.8; the contract-v0.8.md adjunct stays as design rationale.

* Title → "Workspace Contract — v0.8" with a Version line beneath.
* §1.1 final wording per owner: workspace is **always** at <repo>/workspace/
  by default; specified via config file; per-clone overrides via
  sutando.config.local.json. No env override.
* ⚠ callout listing 5 cwd-anchored features that degrade if override
  resolves outside repo root (CLAUDE.md auto-load, skills discovery,
  project-slug transcripts, hook scope, sync-workspace defaults).
* §1.2 Layout adds relay/, skills/, state/auth/.
* §1.4 Confidentiality expanded with "Mechanisms enforcing this:" —
  9 bullets covering: gitignore blanket, pre-commit + CI guards, access
  tiers, TOFU + allowlist, sandboxed delegation, result-body markers,
  memory split, state/auth/ per-host scoping, channel-confidentiality
  routing rule.
* §3 Migration with owner's 3-tier framing: auto (default) / ask-sutando
  / manual.
* §7 FAQ — 5 Q&A: override-mindfully, why-not-env, how-to-migrate,
  weird-place-recovery, external-SSD-symlink.
* §8 Shipping history table extended with #1431, #1432, #1437, and
  #1440 in flight.

Diff stat: +189/-86 lines on top of the prior #1424 baseline.
…NDO_TEST_MODE

Missed in 3f02c80 — this test uses dynamic `await import(...)` instead of
the setupTempWorkspace helper because resolveWorkspace() captures
SUTANDO_WORKSPACE at module-load time and ES `import` is hoisted. The
escape-hatch opt-in needs to land alongside the SUTANDO_WORKSPACE set.

Local: 5/5 pass.
CI was still red after d005762 because `quota-burn-rate.test.py` loads
`read-quota.py`, which exits 1 when `quota-state.json` is missing. The
test seeds quota-state.json into its tmp workspace, but without
SUTANDO_TEST_MODE the env was ignored and `read-quota.py` looked in
`<repo>/workspace/state/` — empty on CI clean checkout, hence exit 1
× 11 tests.

Locally those tests passed by accident — host has a real
`<repo>/workspace/state/quota-state.json`. CI clean checkout has no
workspace state, exposing the env-ignore as the cause.

Also patched 7 other py tests that set SUTANDO_WORKSPACE but don't
strictly depend on env-honor for correctness — adds the opt-in for
consistency + defense against future CI flakes.

Local: all 8 patched files pass.
@qingyun-wu
qingyun-wu marked this pull request as ready for review June 4, 2026 01:05
qingyun-wu and others added 4 commits June 3, 2026 19:24
Addresses 4 blockers Mini raised in #design 2026-06-04 02:09Z on the
v0.8 env-strip PR. The auto-migration block in src/startup.sh ran
sutando-migrate.sh --commit, then tar -czf'd the source dir, then
rm -rf'd it — a sequence that could delete the active workspace if
SUTANDO_WORKSPACE was symlink-equivalent to the resolved path, archive
the wrong (post-migration) state, or rm -rf an unsafe path on a malformed
env var. Fixed by adding 4 layered guards.

## Changes

src/migration_safety_helpers.sh (new — 4 sourceable functions):
  - _realpath: cross-platform (GNU realpath / BSD readlink -f / python fallback)
  - _same_inode: stat -L -f %i (BSD) || -L -c %i (GNU); follows symlinks via -L
  - _is_unsafe_for_migration: deny-list — /, /usr|/etc|/var|/bin|/sbin|/System|
    /Library|/Applications, $HOME + top-level subdirs, $REPO + anywhere under it,
    /private/etc|/private/var (macOS symlink targets), paths with surviving '..'.
  - _color_warn: bold-red on TTY *only* when NO_COLOR is unset

src/startup.sh (sources the helpers and restructures the migration block):
  - B1: realpath equality + _same_inode check before any migrate/compress/delete.
    Skip the entire block when env points at the resolved workspace.
  - B2: tar -czf BEFORE sutando-migrate.sh --commit. The tarball is now a true
    pre-migration recovery snapshot; if archive fails, abort before the
    destructive step. If migrate fails AFTER archive succeeds, the archive
    is preserved (caller can roll back).
  - B3: _is_unsafe_for_migration deny-list applied at TWO points — once up-front
    (refuse the whole block), again after successful migrate before rm -rf
    (catches state mutation between checks). Failed re-check preserves archive.
  - B4: _color_warn replaces the inline `[ -t 2 ]` check; honors NO_COLOR.
  - Sentinel now records the archive path for downstream tooling.

src/init.sh:
  - B4: honor NO_COLOR in the .env-deprecation warning (parallel structure to
    startup.sh — same `[ -t 2 ] && [ -z NO_COLOR ]` gate).
  - Drop literal `$_env_val` and `$WORKSPACE` interpolations from the warning
    text (parity with Python's c58270d safety pass — embedded /-bearing values
    were the trigger for the caller-side `mkdir -p "$captured"` rogue-folder
    regression).

src/sutando_config.ts (TS resolver):
  - B4: drop literal `${envVal}` from the SUTANDO_WORKSPACE-set warning at
    L283-294 and `${dotenvVal}` + `${resolved}` from the .env-drift warning at
    L312-325. Wording rephrased so it reads naturally without the path values.
    The existing NO_COLOR handling in _colorWarn (L188) already worked — only
    the path-leak side needed fixing.

## Verification

- bash tests/migration-safety-helpers.test.sh: 24/24 pass (unit-tests the 4
  guard functions: realpath against symlinks, same_inode follows symlinks,
  is_unsafe_for_migration covers /, /etc, /var, /System, $HOME and subdirs,
  $REPO and subdirs, non-existent, sneaky-symlink-to-$HOME, plus _color_warn
  TTY/NO_COLOR matrix).
- npx tsx --test tests/sutando-config.prod-path.test.ts: 7/7 pass (env-set
  production path: NO_COLOR=1 + non-TTY emit zero ANSI; env warning + .env
  warning contain no literal path values; warning is one-shot per process;
  resolver returns config-driven path or {repoRoot}/workspace default).
- tests/sutando-config.test.ts: 21/21 pass — no regression.
- tests/sutando-config.test.py: 30/30 pass — no regression.
- bash -n on edited shells: syntax clean.

## Known follow-up

Mini also asked for a full startup-migration integration test (same-path,
symlink path, empty legacy dir, migrate failure, archive contents, no-delete
on unsafe paths). The helpers-level test + TS prod-path test cover the
highest-risk units; a driver-level integration test that orchestrates the
full migration block (mocking sutando-migrate.sh --commit + tar) is a
separate follow-up — non-trivial setup, lower marginal value than the unit
coverage shipped here. Tracking in PR body.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two new test files, both passing locally, both pinned to the safety
hardening shipped in the parent commit.

## tests/migration-safety-helpers.test.sh (24 assertions)

Unit-tests src/migration_safety_helpers.sh — the 4 guard functions
extracted from src/startup.sh in the parent commit:

- _realpath against an existing file, a symlink, a non-existent path.
- _same_inode: file == itself, symlink == target (follows via -L),
  different files != each other.
- _is_unsafe_for_migration: positive coverage over /, /usr, /etc, /var,
  /System, $HOME, $HOME/{Documents,Desktop,Downloads}, $REPO, $REPO/src,
  non-existent paths, plus a sneaky `ln -s $HOME /tmp/...` that realpath
  resolves through; negative coverage on /tmp/<random>/* as safe.
- _color_warn: non-TTY → plain; NO_COLOR=1 → plain even on TTY (via
  `script` for emulation); TTY + NO_COLOR unset → ANSI.

Test fixture uses /tmp/<pid>/ explicitly (not `mktemp -d` which on macOS
lands under /var/folders/ → /private/var/... which the deny-list correctly
catches as a /var subpath).

## tests/sutando-config.prod-path.test.ts (7 assertions)

Mini called out that the existing TS suite leans on SUTANDO_TEST_MODE=1,
leaving the production path under-covered. New file exercises the env-set
+ no-TEST_MODE branch directly:

- NO_COLOR=1 → deprecation stderr has zero ANSI escapes.
- non-TTY stderr → zero ANSI escapes regardless of NO_COLOR.
- env-set warning text does NOT contain the literal SUTANDO_WORKSPACE
  value (B4 path-leak parity with c58270d on Python side).
- .env-drift warning text does NOT contain the literal .env value.
- Deprecation warning fires exactly once across 3 resolveWorkspace calls
  (one-shot guard via _legacyEnvWarnPrinted).
- env set + sutando.config.local.json present → resolver returns the
  config-driven path (config wins).
- env set + no config → resolver returns the baked-in {repoRoot}/workspace.

Stderr capture uses a process.stderr.write monkey-patch with a restorer in
the afterEach hook so tests don't pollute each other's captures.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

Addresses Mini's #design follow-up review (2026-06-04 02:30Z) on the
v1 of the safety hardening pass. Three sub-blockers + a Python coverage
gap.

## B3 — deny-list expansion (Mini: "still a blocker")

`_is_unsafe_for_migration` now denies the additional cases Mini called
out, where the prior pass left descendant paths or common dotfile
targets uncovered:

- `/tmp` + `/private/tmp` exact (subdirs like mktemp targets remain safe
  — the prior pass only denied the macOS-resolved /private/etc + /private/var)
- `$HOME/Documents/*`, `$HOME/Desktop/*`, `$HOME/Downloads/*` descendants
  (prior: only the exact dirs; `~/Documents/some-repo` was fair game)
- `$HOME/.sutando` exact + subpaths, EXCEPT `$HOME/.sutando/workspace`
  (and subpaths) — the workspace subpath IS the known legacy default
  we WANT to be able to migrate; the allow-exception is checked first
  via a case statement so the subsequent .sutando deny doesn't shadow it
- `$HOME/.claude` exact + subpaths (Claude Code's install location)
- `$HOME/.config` exact + subpaths (XDG install location)

## B1 — device:inode (Mini: "edge: same inode across unrelated fs")

`_same_inode` now uses `stat -L -f '%d:%i'` (BSD) / `-c '%d:%i'` (GNU)
instead of `%i` alone. Comparing inode numbers without the device id
false-positives when two filesystems happen to reuse the same inode
number — `device:inode` is what makes the pair globally unique.

## Test coverage expansion

`tests/migration-safety-helpers.test.sh` gains 14 new assertions:
- `/tmp` exact deny + `/tmp/<subdir>` safe (using mkdir'd test fixture)
- `/private/tmp` exact deny
- `$HOME/Documents|Desktop|Downloads/foo` descendant deny (3)
- `$HOME/.sutando` exact deny + non-workspace subpath deny + workspace
  subpath ALLOW (incl. nested) (4) — uses a fake $HOME under TEST_DIR
  with mkdir'd dir tree to test the allow-exception without touching
  the user's real ~/.sutando/ tree. Realpath's the fake-home so the
  $HOME case pattern matches the realpath'd /private/tmp/... paths
  (test-fixture-side workaround; real systems with $HOME=/Users/...
  don't hit this aliasing).
- `$HOME/.claude` + subpath deny (2)
- `$HOME/.config` + subpath deny (2)

Total: 38/38 pass (was 24/24 on v1).

## tests/sutando-config.prod-path.test.py (NEW — Python twin of v1 TS test)

Mini: "Python prod-path coverage is still thin." Mirrors the TS prod-
path test added in df70545; 7 assertions covering the env-set, no-
TEST_MODE branch:
- NO_COLOR=1 → zero ANSI in deprecation stderr
- non-TTY stderr (StringIO under unittest) → zero ANSI
- env-set warning has no literal env-var value (c58270d regression guard)
- .env-drift warning has no literal dotenv value
- warning is one-shot across 3 resolve_workspace() calls
- env set + sutando.config.local.json → resolver returns config-driven path
- env set + no config → resolver returns {repoRoot}/workspace (realpath'd
  to handle macOS /var → /private/var)

Uses contextmanager-style capture_stderr() with restorer in tearDown so
tests don't pollute each other's captures. Mirrors the TS test's
beforeEach/afterEach env-snapshot pattern.

## Verification

- bash tests/migration-safety-helpers.test.sh: 38/38 pass (was 24/24)
- python3 tests/sutando-config.prod-path.test.py: 7/7 pass (new)
- npx tsx --test tests/sutando-config.prod-path.test.ts: 7/7 pass (no regression)
- npx tsx --test tests/sutando-config.test.ts: 21/21 pass (no regression)
- python3 tests/sutando-config.test.py: 30/30 pass (no regression)

## Known follow-up (carried from v1)

- Full startup-migration integration test (Mini's "Test gaps" list — driver-
  level mock-migrate + tar end-to-end). Still a separate PR; the helpers
  unit + TS+Python prod-path tests cover the highest-risk units.
- Test-fixture migration (~141 SUTANDO_WORKSPACE refs across 23 tests). The
  SUTANDO_TEST_MODE escape hatch is transitional; full migration to
  sutando.config.local.json fixtures is tracked under Path B.1b.

## Symlink legacy dir as archive edge (Mini flagged)

If SUTANDO_WORKSPACE itself is a symlink to a real dir, `tar -czf` handles
this case by archiving the symlink target's content (BSD + GNU tar both
follow per `-h` default off, but `tar -C "$(dirname "$_ws_legacy")" "$(basename "$_ws_legacy")"`
operates on the entry as named — symlink entry is preserved in the archive
with `-h` off, archive-with-followed-content with `-h` on). The B1
realpath guard already converts the symlink target to its real path
for the same-path / inode comparisons, so a symlink→resolved-workspace
case is caught up-front and the destructive block is skipped. A
broken symlink fails the `[ -d "$_ws_legacy" ]` precondition and is
also skipped. Documented as a known-limitation note in the PR body.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

CI failure on `0c496ef` — `refuse new direct workspace-path resolution`
flagged legitimate `$HOME/.sutando/workspace` references in:

- `src/migration_safety_helpers.sh` (the deny-list allow-exception logic —
  the very point of the helper is to recognize the legacy default as an
  intended migration source)
- `tests/migration-safety-helpers.test.sh` (the test asserting that the
  allow-exception works)

The lint regex correctly catches new code that DOES workspace resolution
inline (the anti-pattern it was designed to block), but here the references
are in:
  (1) a NEW safety-helper file whose purpose IS to know about the legacy
      default — the literal path appears as a CASE PATTERN in the deny-list,
      not as resolver code; and
  (2) a SHELL test (the existing tests/ allowlist matched only .py and .ts;
      generalizing to also include .sh covers any future shell-test that
      needs to assert against literal paths).

Two-part fix:
- Add `src/migration_safety_helpers\.sh` to the ALLOWED files regex.
- Extend `tests/[^/]+\.(test\.)?(py|ts)$` → `(py|ts|sh)$` so shell tests
  in tests/ get the same exemption Python and TS tests already have.

Verified locally:
  `BASE_REF=origin/staging-workspace-revamp bash scripts/lint-workspace-resolution.sh --diff`
  → `✓ no new workspace-resolution anti-patterns in this diff.`

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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
qingyun-wu merged commit a343be3 into staging-workspace-revamp Jun 4, 2026
5 checks passed
@qingyun-wu
qingyun-wu deleted the fix/workspace-strip-env-override branch June 4, 2026 02:53
qingyun-wu added a commit that referenced this pull request Jun 4, 2026
…r (PR-1) (#1445)

* feat(sync-workspace): Phase 1 scaffold — bootstrap + canonical_id + dispatch

PR-1 Phase 1 of the workspace-vault sync rewrite per the 2026-06-04 #design
crystallization. Owner directive (option b — full revised PR-1, ~5h).

## What's in this commit (Phase 1 only)

Working scaffold for the new `scripts/sync-workspace.sh`. Phase 2/3/4 fill
in the actual sync logic; this commit is the structural foundation that
the subsequent phases hang off.

**Section 1 (Bootstrap):**
- SCRIPT_DIR + SCRIPT_PARENT resolution (realpath-aware)
- .env loader from SCRIPT_PARENT (non-interactive shells)
- REPO_DIR with auto-detect via CLAUDE.md+skills+/.git signature; SUTANDO_REPO_DIR override
- WORKSPACE_DIR via M0 helper at $SCRIPT_PARENT/scripts/sutando-config.sh (avoids
  stale-REPO_DIR-pin issue per feedback_stale_repo_dir_pin)
- VAULT_URL = SUTANDO_VAULT (canonical) OR SUTANDO_MEMORY_REPO (legacy alias,
  one release, warning on stderr)

**Section 2 (Logging + UI):**
- log(), color_warn() (NO_COLOR-aware per no-color.org), die()

**Section 3 (Lock):**
- acquire_lock with stale-cleanup at 10min (POSIX atomic mkdir, no flock dep)

**Section 4 (Canonical ID + projects.map.json):**
- canonical_id() = sha256-8 of vault URL (SUTANDO_VAULT_PROJECT_ID override)
- local_slug() = REPO_DIR with / replaced by - (matches Claude Code's auto-derived slug)
- PROJECTS_MAP_PATH() = $WORKSPACE/.sutando-vault/projects.map.json
- ensure_projects_map_entry() — WORKING (not a stub): bootstrap map if missing,
  idempotent if entry already matches, add/update entry otherwise

**Section 5 (.gitignore template):**
- DEFAULT_CARRIER_INCLUDES — paths to un-ignore (notes/, build_log/, pending-questions.md,
  .claude-sutando/projects/<CANONICAL_ID>/memory/, projects.map.json)
- DEFAULT_GITIGNORE_LINES — explicit-deny baseline (state/, tasks/, results/, logs/,
  data/, *.heartbeat, etc.) + "ignore everything" wildcard + ".env*"
- generate_gitignore() — stub (Phase 2 implements config merge + write)

**Section 6 (Subcommand stubs):**
- cmd_init, cmd_pull_only, cmd_push_only, cmd_default_bidirectional,
  cmd_status (PARTIALLY WORKING — shows resolved values), cmd_migrate_from_legacy,
  cmd_help (prints header)

**Section 7 (Dispatch):** case-statement on $1

## Smoke test passes

- `bash -n scripts/sync-workspace.sh` — syntax clean
- `bash scripts/sync-workspace.sh --status` — resolves WORKSPACE_DIR + REPO_DIR
  + VAULT_URL + canonical_id + local_slug correctly
- `bash scripts/sync-workspace.sh --help` — prints header
- `bash scripts/sync-workspace.sh foobar` — unknown-subcommand error, exit 2

## Phases 2-4 follow in subsequent commits

- Phase 2 (~1.5h): workspace-git-init, .gitignore generation from config,
  before-push memory copy, after-pull memory copy, branch-per-host push + fetch
- Phase 3 (~1.5h): 3-way merge first, --strategy-option=ours fallback,
  migration helper for ~/.sutando/memory-sync/ → workspace-as-git-repo,
  mass-deletion tripwire
- Phase 4 (~1h): tests/sync-workspace-{init,translation,conflict}.test.sh,
  ~20 reference updates, backward-compat shim at sync-memory.sh

Owner checkpoint pattern: pause + DM + #design status at end of each phase.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* feat(sync-workspace): Phase 2 — workspace-git-init + translation layer + bidirectional sync

Fleshes out the Phase 1 scaffold's subcommand stubs with working bodies.

## What's in this commit

**`generate_gitignore()`** (was stub) — now writes the actual .gitignore
to $WORKSPACE_DIR. Whitelist mode: `*` ignores everything by default;
selective un-ignore for the carrier set (notes/, build_log/, pending-questions.md,
.claude-sutando/projects/<CANONICAL_ID>/memory/, projects.map.json, plus
each ancestor un-ignore so git doesn't stop at parent). Hard-deny .env*,
*.heartbeat, *.alive, *.sentinel, *.pid regardless. User config merge
(vault.sync.{include,exclude}) deferred to Phase 4 / sibling PR — keeps
Phase 2 scope focused on the core plumbing.

**`_init_impl`** (was stub) — Phase 2 body for `cmd_init`:
  1. git init in $WORKSPACE_DIR if not already a repo
  2. Set origin to $VAULT_URL (idempotent — replace if URL changed)
  3. generate_gitignore (overwrites; operators use config for overrides)
  4. ensure_projects_map_entry (bootstraps map.json + this host's entry)
  5. mkdir canonical-id memory dir
  6. Stage + commit + push to refs/heads/host/<hostname>

**Translation layer** (NEW — `copy_localslug_to_canonical` +
`copy_canonical_to_localslug`):
  - Bidirectional bridge between Claude Code's auto-derived local-slug dir
    (gitignored, per-host) and the canonical-id mirror (tracked, vault-synced).
  - Uses `cp -a path/. target/` to copy CONTENTS not the dir; preserves
    metadata; creates target on demand.
  - Idempotent + safe when either side is empty (skip with log).

**`_pull_only_impl`** (was stub):
  1. git fetch --all
  2. Checkout host/<hostname> branch (create from origin/host/<hostname>
     remote-tracking if exists, else from current HEAD)
  3. Sort peer branches by last-push timestamp, oldest first (per design —
     minimizes per-step merge diff)
  4. git merge each peer into local branch
  5. On conflict: --ours fallback (git checkout --ours + add + commit)
  6. copy_canonical_to_localslug — Claude Code on this host sees peers' memory

**`_push_only_impl`** (was stub):
  1. copy_localslug_to_canonical — this host's memory writes propagate up
  2. ensure_projects_map_entry
  3. git add -A
  4. Mass-deletion tripwire (preserved from sync-memory.sh:439) — refuse
     push if >SUTANDO_SYNC_MAX_DELETE (default 50) files would be deleted
     unless SUTANDO_FORCE_SYNC=1
  5. git commit + git push origin HEAD:refs/heads/host/<hostname>

**`cmd_default_bidirectional`** — orchestrates pull-then-push. Pull failures
don't block push (`|| true`).

**Lock refactor:** split cmd_* into outer (acquire_lock) + _impl (lock-free)
so cmd_default_bidirectional can call both impls under one lock instead of
double-acquiring.

## Verification

- `bash -n scripts/sync-workspace.sh` — syntax clean (611 lines)
- `bash scripts/sync-workspace.sh --status` — resolves correctly

## Still Phase 3 (next commit)

- `cmd_migrate_from_legacy` body (move ~/.sutando/memory-sync/ →
  workspace-as-git-repo: rsync carrier-set files, git init, first push)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* feat(sync-workspace): Phase 3 — migration helper from legacy memory-sync

Last subcommand body for the script. Phase 4 (tests) comes next.

## `cmd_migrate_from_legacy` (was stub)

One-time migration helper for fleet hosts that have an existing
`~/.sutando/memory-sync/` clone. Steps:

1. Detect legacy clone at `$HOME/.sutando/memory-sync/` (or
   `$SUTANDO_MEMORY_SYNC_DIR` if set).
2. Curated copy into workspace's tracked paths:
   - `legacy/notes/` → `workspace/notes/` (handles symlink: unlink and copy
     content if `workspace/notes` already points into the legacy clone)
   - `legacy/memory/*.md` → `workspace/.claude-sutando/projects/<canonical>/memory/`
   - `legacy/pending-questions.md` (prefers `machine-<host>/pending-questions.md`
     if present) → `workspace/pending-questions.md`
   - `legacy/build_log.md` (similarly preferring machine-<host>) →
     `workspace/build_log/<hostname>.md` (per-host split per design)
3. Hands off to `_init_impl` to git-init + first push.
4. Prints operator-facing next-steps recipe (verify, run normal sync,
   only delete legacy after confirming).

## Safety properties

- Never deletes the legacy dir (operator controls cleanup).
- `cp -n` everywhere — never overwrites existing workspace content.
- Hard-fail if legacy dir missing or not a git repo (refuses to operate
  on the wrong source).
- Handles the symlink-into-legacy case for `workspace/notes` that
  sync-memory.sh's bootstrap creates.

## Phase 4 (next commit)

Tests (`tests/sync-workspace-{init,translation,conflict}.test.sh`).
Reference updates (README.md, src/, scripts/, skills/, docs/) DEFERRED
to PR-2 (the rename + cutover) — keeps PR-1 narrow: new opt-in script
+ tests, doesn't touch existing prod sync-memory.sh callers.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* test(sync-workspace): Phase 4 — 18-assertion integration test

End-to-end coverage of scripts/sync-workspace.sh against fixture
workspace + local bare-repo vault. Hermetic: never touches the operator's
real workspace or vault.

## Test isolation strategy

- `SUTANDO_WORKSPACE` + `SUTANDO_TEST_MODE=1` — M0 helper honors the env per
  the v0.8 escape hatch added in #1440.
- `SUTANDO_REPO_DIR` — fake sutando-checkout with `CLAUDE.md` + `skills/` +
  `.git`, populated with copies of `sync-workspace.sh` + `sutando-config.sh`
  + `sutando_config.py` so the script's auto-detect picks up the fixture.
- `SUTANDO_VAULT` — local `git init --bare` repo URL, no network.
- `FIXTURE_WS` realpath'd to handle macOS `/var/folders/` → `/private/var/`
  symlink alias (same lesson as `tests/migration-safety-helpers.test.sh`).

## Test coverage (18 assertions, 7 scenarios)

1. **--status before init** (3 assertions): WORKSPACE_DIR, VAULT_URL,
   canonical_id all present in output.
2. **--init creates expected artifacts** (8 assertions): `.git/`,
   `.gitignore`, `projects.map.json`, canonical memory dir, .gitignore
   whitelists `notes/` + canonical mirror, hard-denies `.env`, vault
   remote configured, `host/<hostname>` branch present in vault.
3. **Idempotent re-init**: re-running --init detects existing repo, no
   error (smoke-only — informational, not asserted hard).
4. **--push-only with no changes is a no-op** (1 assertion): clean tree →
   "nothing to push" path.
5. **Translation layer push-side** (2 assertions): write a memory file to
   local-slug, --push-only, verify it appears at canonical AND in the vault
   `host/<hostname>` branch (via `git --git-dir=$vault show`).
6. **Translation layer pull-side** (1 assertion): simulate a peer host by
   cloning the vault to a second workspace, write to canonical, push to
   `host/peerhost`. On host 1 run --pull-only, verify peer's memory file
   lands in our local-slug after canonical→local-slug copy.
7. **Mass-deletion tripwire** (2 assertions): create 60 notes, commit them,
   delete all 60, run --push-only — verify tripwire fires + staged
   deletions get reset (preserved behavior from sync-memory.sh).

## Result: 18/18 pass

`bash tests/sync-workspace.test.sh` → exit 0.

## Lessons surfaced during writing

- Sourcing the script with `bash -c '. script --help'` is fragile because
  `set -u` in the sourced script can trip on the sourcing shell's unset
  positional args. Better: re-derive expected values directly in the test
  rather than calling the script for them.
- Test subprocess pipes through `head -1` cause SIGPIPE → exit 141 →
  whole test suite aborts (bash --pipefail). Use `>/dev/null 2>&1` for
  fixture commands; reserve `head` for things you actually want to truncate.
- Bare git repos clone with `warning: remote HEAD refers to nonexistent ref`
  unless HEAD is set. Peer setup needs explicit base branch on clone:
  `git checkout -B host/peerhost origin/host/${HOST}`.

## Reference updates DEFERRED

Per the Phase 4 strategy in the prior commit: PR-1 does NOT modify
README.md, src/, scripts/cron-gate.sh, scripts/stage-readiness.sh,
skills/schedule-crons/*, docs/* to point at sync-workspace.sh. Those
references still call sync-memory.sh and continue to work unchanged.
PR-2 handles the rename + cutover, at which point a backward-compat shim
at sync-memory.sh redirects to sync-workspace.sh and the ~20 reference
updates land in one focused PR.

Phase 5 (open DRAFT PR) follows.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* refactor(sync-workspace): drop memory translation layer per owner simplification

Owner directive 2026-06-04 05:11Z + 05:15Z refinement:
> "Let's remove the Memory translation layer for .claude_sutando. Let's
> keep do simple pull."
> "track all .claude-sutando/projects/* /memory only the memory folder.
> There are many large files that we may not want to track."

## Removed (~140 lines)

- `canonical_id()` derivation (sha256-8 of vault URL)
- `local_slug()` computation function
- `PROJECTS_MAP_PATH()` + `ensure_projects_map_entry()` (projects.map.json)
- `copy_localslug_to_canonical()` + `copy_canonical_to_localslug()` (the
  bidirectional copy bridge)
- `SUTANDO_VAULT_PROJECT_ID` env var
- All call sites: `_init_impl` no longer creates canonical memory dir or
  bootstraps map.json; `_push_only_impl` no longer copies before push;
  `_pull_only_impl` no longer copies after pull; `cmd_status` no longer
  prints canonical_id / local_slug / map.json contents

## New gitignore pattern (per the 05:15Z refinement)

```gitignore
!.claude-sutando/
!.claude-sutando/projects/
!.claude-sutando/projects/*/         # un-ignore each slug DIR
!.claude-sutando/projects/*/memory/  # un-ignore memory subdir only
!.claude-sutando/projects/*/memory/** # un-ignore memory contents
```

Transcripts (`*.jsonl`), `tool-results/`, `file_history/`, etc. INSIDE
slug dirs stay gitignored — only the user-curated `memory/` subdirs sync.
Each host writes to its OWN slug; after pull, peer slug subdirs are
visible-not-merged.

## migrate-from-legacy adjusted

Legacy `memory/*.md` now copies to `workspace/.claude-sutando/projects/<local_slug>/memory/`
(this host's actual Claude Code-derived slug — `<REPO_DIR>` with `/` → `-`)
instead of the (now-removed) canonical-id mirror. So Claude Code on this
host reads from the migrated content as expected.

## Test updates (21/21 pass)

Rewrote `tests/sync-workspace.test.sh` for the simplified design:
- Test 1: --status no longer mentions canonical_id / projects.map.json
- Test 2: .gitignore content checks for `!.claude-sutando/projects/*/memory/`
  (not `<CANONICAL>/memory/`); verify no `.sutando-vault/projects.map.json`
  file is created
- Test 5 (renamed): write memory to local-slug → push → verify file appears
  in vault at the SAME local_slug path (no translation)
- Test 6 (new): write fake transcript to local-slug → verify NOT pushed
  (gitignored — only memory/ tracked)
- Test 7 (was 6): peer host pushes to its OWN slug → pull → verify peer
  slug subdir visible in host 1's workspace + host 1's own memory still
  present (no clobber)
- Test 8 (was 7): mass-deletion tripwire (unchanged)

## What this commit costs

Memory across hosts is now visible-not-merged. Cross-host memory sharing
requires the operator/agent to read `<peer_slug>/memory/` paths explicitly.
This trade-off was the explicit owner call: complexity of the translation
layer wasn't worth it for an aspiration ("merge memory across hosts") that
isn't actually a real-pain item yet.

## Diff stats

scripts/sync-workspace.sh: 735 → 551 lines (-184, +1 to nett -183 incl. doc)
tests/sync-workspace.test.sh: 253 → 282 lines (+29 — new tests for the
  gitignore-only-tracks-memory invariant; removed translation-layer asserts;
  added the transcript-not-pushed test)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix(sync-workspace): address Pro's 3 review findings (PR-1 v3)

Pro review at #design 05:23Z flagged 3 safety/data-loss gaps in the
simplified PR-1. All three addressed in this commit + tests.

## Fix #3 — .gitignore overwrite warning (smallest, ~50 lines)

**Risk:** `_init_impl` overwrote `$WORKSPACE/.gitignore` on every run, even
if the user had hand-edited it. Operator-authored content blocking the
carrier set would silently get reinstated by overwrite → data loss in
the vault.

**Fix:** split `generate_gitignore` into `_compose_gitignore_content`
(writes to stdout) + `generate_gitignore` (compose → diff against
existing → refuse to overwrite unless `--force-gitignore` flag set).

- If no existing .gitignore → write directly
- If existing identical to generated → no-op
- If existing differs → print unified diff to stderr + refuse
- `--force-gitignore` flag bypasses the refusal

New global flag: `FORCE_GITIGNORE` (parsed in Section 0).

## Fix #2 — pull-side mass-deletion tripwire (~35 lines)

**Risk:** The mass-deletion tripwire in `_push_only_impl` only caught
**staged** deletions (`--diff-filter=D` on `git diff --cached`). But
`git merge` during pull can delete files in the working tree directly
via merge — bypassing the staging area. A peer branch with 60+ file
deletions could land via merge → silently delete tracked files locally.

**Fix:** Snapshot `git rev-parse HEAD` + `git ls-files | wc -l` BEFORE
the fetch+merge. Post-merge, recount tracked files. If drop >
`SUTANDO_SYNC_MAX_DELETE` (default 50) AND `SUTANDO_FORCE_SYNC` != 1,
`git reset --hard <pre_pull_sha>` to roll back the merge + return 1.

## Fix #1 — --dry-run for --migrate-from-legacy (~40 lines)

**Risk:** `--migrate-from-legacy` performs destructive copies + `git
init` + `git push` against the vault, with no way to preview. Pro
explicitly asked: "is --dry-run implemented? ... Strongly recommend
test on a sacrificial clone before enabling on fleet."

**Fix:** Global `--dry-run` flag (parsed in Section 0, applies to all
subcommands). New helper `_do <cmd...>` runs the cmd OR prints
`DRY-RUN: would: ...` and returns 0. Applied in `_init_impl` (early
short-circuit), `_push_only_impl` (early short-circuit), and
`_migrate_from_legacy_impl` (per-step). `generate_gitignore` also
respects DRY_RUN (writes to /dev/null + prints "would write N lines").

## Section 0 — new global flag parser

```bash
DRY_RUN=0
FORCE_GITIGNORE=0

_args=()
for _arg in "$@"; do
    case "$_arg" in
        --dry-run)         DRY_RUN=1 ;;
        --force-gitignore) FORCE_GITIGNORE=1 ;;
        *)                 _args+=("$_arg") ;;
    esac
done
set -- "${_args[@]:-}"
```

Flags strip out of $@ before subcommand dispatch. Order-insensitive.

## Tests

Extended `tests/sync-workspace.test.sh` to 29/29 pass (was 21). Three new
test scenarios:

- **Test 9 — .gitignore overwrite warning (3 asserts):** modify the
  generated .gitignore in place, run --init → refused + user edit
  preserved; --force-gitignore → overwrites successfully.
- **Test 10 — pull-side tripwire (2 asserts):** push 60 notes from host 1
  with SUTANDO_FORCE_SYNC=1, peer clone deletes all 60 + pushes to
  host/peerhost2. Host 1 --pull-only → tripwire fires, working tree
  restored from pre-pull SHA. Note count unchanged (60 → 60).
- **Test 11 — --dry-run on migrate-from-legacy (3 asserts):** setup
  fake legacy memory-sync at $TEST_ROOT/fake-legacy-memory-sync with
  notes/memory/pending-questions/build_log. Run --dry-run; verify
  DRY-RUN markers in output, legacy notes NOT copied to workspace,
  workspace note count unchanged.

Diff stats: scripts/sync-workspace.sh 551 → 680 lines (+129);
tests/sync-workspace.test.sh 282 → ~370 lines (+88).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix(sync-workspace): address Mini #1445 v3 Medium findings

Two safety fixes to the workspace-vault sync script:

1. .gitignore diff display tripping set -euo pipefail (Medium)
   `diff -u $a $b | head -40` returns nonzero when files differ + may
   SIGPIPE on long output. With pipefail, the pipeline exit propagates
   and set -e exits the script before tmp_path cleanup, bypassing the
   "to overwrite anyway: pass --force-gitignore" guidance.

   Fix: `... || true` on the pipeline. Same pattern applied
   defensively to two other `| head` pipelines (status + help) per
   per-file pattern audit.

2. Pull-side tripwire bypassed by delete+add net zero (Medium)
   Old: pre_count - post_count = net change. A peer commit that
   deleted 60 files and added 60 had net=0, completely bypassing
   the tripwire while losing 60 files of original content.

   Fix: count actual deletions via
     git diff --name-only --diff-filter=D pre_pull_sha..HEAD
   Also adds percentage threshold (SUTANDO_SYNC_MAX_DELETE_PCT,
   default 50%) so catastrophic small-workspace cases — e.g. 20-of-30
   deletions — still trip below the absolute 50-file default.

Structural side-fix exposed by the new test: the --ours merge-
conflict fallback failed on DD-conflicts (both peers deleted the
same file) because `git checkout --ours -- $f` errors when the
file isn't on our side either. Added `git rm -f` fallback so the
merge resolves cleanly + the tripwire then sees the correct
deletion count and rolls back.

Tests: 29 → 31 pass. New Test 12 reproduces the delete-and-add
bypass + asserts both the actual-deletion-count tripwire fires
AND the working tree is restored to pre-pull state.

Mini Low #3 (--dry-run still mutates /tmp lock + log + mktemp)
acknowledged as acceptable — vault/workspace mutations covered;
/tmp is process-scoped + auto-cleaned.

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

* fix(sync-workspace): address Mini #1445 v4 review

Three more bounces from Mini's v4 review:

1. (Medium) bootstrap .env grep pipeline tripped set -euo pipefail
   `VAULT_URL=$(grep '^SUTANDO_VAULT=' .env | head -1 | ...)` —
   when .env exists without a SUTANDO_VAULT line, grep exits 1,
   pipefail propagates, and the assignment under set -e exits the
   script before the SUTANDO_MEMORY_REPO legacy-alias fallback runs.
   (Empirically verified — `var=$(...)` assignment does NOT exempt
   set -e from a failing subshell here, contrary to my earlier
   assumption.) Fix: trailing `|| true` on both .env grep pipelines
   so the intended fall-through semantics are restored.

2. (Low) rename detection in mass-deletion tripwire
   `git diff --diff-filter=D pre..HEAD` counts large renames as
   delete+add unless user-config enables rename detection. If the
   intent is true content loss, file moves must not trip the wire.
   Fix: add `-M` (default 50% similarity) on BOTH pull-side AND
   push-side tripwires — file moves now count as R, not D.

3. (Test gap) Test 12 was thin
   Only verified note-* count. Now also asserts:
     - no replacement-*.md files leaked into workspace
     - HEAD unchanged across repeated tripwire pulls
     - git status clean after tripwire (no staged/unmerged paths)
   Plus new Test 13 reproduces the bootstrap-pipeline bug from #1:
   a .env with only SUTANDO_MEMORY_REPO must complete --status
   without exiting (pre-fix this exited 1).

Side-fix on the test itself: `ls "$dir/replacement-"*.md` of a
non-matching glob exits 2 → pipefail trips test's set -e and
silently truncates output mid-assertion. Switched to `find`, which
returns 0 on no-match. The earlier ls usages worked only because
their globs happened to match.

Tests: 31 → 36 pass (Test 12 +3 assertions, Test 13 +2 assertions).

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

* test(sync-workspace): address Mini #1445 v6 test-hygiene findings

Mini caught the inconsistency: my v5 fix swapped one `ls glob | wc -l`
for `find`, but the older PRE/POST/RESTORED counters still used the
fragile pattern. Same pipefail trap as before — a degenerate fixture
(zero matches) silently aborts the test instead of reporting a FAIL.

Changes:

1. Replaced 3 remaining `ls "$dir/note-"*.md | wc -l` with `find`
   for consistent behavior on no-match fixtures.

2. Extended Test 12's HEAD-non-drift check from N=2 to N=10 iterations.
   The previous "pull twice + check" weakly proved idempotency; a tight
   loop with a drift counter is the real proof.

3. New Test 14 — canonical SUTANDO_VAULT=in-.env path. Test 13 covered
   the legacy-alias fallback; Test 14 covers the happy-path-via-.env
   case (vs Tests 1-12 which set SUTANDO_VAULT via process env). Three
   assertions: --status exits 0; legacy-alias warning NOT spuriously
   triggered; VAULT_URL string surfaced in --status output.

Tests: 36 → 39 pass. Script unchanged this round.

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

---------

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
…state, 2-space model

Part 4 of the v0.3.0 docs sweep (PR #1472).

**docs/workspace-design.md — full rewrite around 2-space model.**

Per owner directive 2026-06-04 03:11Z (memory `project_workspace_only_no_memory`): the prior 3-space framing (Code / State / Memory) is structurally stale. Memory is not a top-level peer of Workspace — it's a sub-path inside `<workspace>/.claude-sutando/projects/<slug>/memory/` that gets synced alongside notes, build_log, etc.

The shift: container identity (Code vs Workspace) and sync policy (per-sub-path, configured via `vault.sync.*`) are separated. The 3-space model conflated them.

Preserved from the prior doc:
- The bug-class history (~28 sites of workspace-vs-repo divergence pre-M0)
- The Decisions section as historical record explaining why the model changed

Rewrote:
- Title + status: "3-space" → "2-space"
- TL;DR table: 3 rows → 2 rows
- Quick-decision flowchart: simplified to "ships with every install?"
- Resolvers section: kept Code path, rewrote Workspace path with sub-path sync table
- New historical-note callout at top explaining the change

**docs/workspace-contract.md — update status note.**

"v0.8 (target shape)" → "v0.3.0 (ratified)". PR #1440 has merged; the
"after-#1440" hedge is now historical. Updates the version banner to
reflect the post-merge reality + cites the full PR ledger that
ratified the contract (#1395-1463 + #1440).

**docs/workspace-config.md — drop $SUTANDO_WORKSPACE from resolution order.**

Pre-1440 the env var was honored as a legacy escape hatch. Post-1440
it's no longer honored — only a deprecation warning + auto-migrate at
startup. Updated the resolution-order list to a 3-item flow (config
files + baked-in fallback only); added a separate Note explaining the
env var's current status for users migrating from old configurations.

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
…sweep summary

Part 5 of the v0.3.0 docs sweep (PR #1472).

**Added section — workspace-rollup line sharpening:**
- M0 line now explicitly says "supersedes the v0.1.0 `~/.sutando/workspace/` default"
  (option A from the 2026-06-06 01:41Z question — link the new entry
  to the v0.1.0 entry it replaces, rather than annotating the v0.1.0
  entry retroactively).
- Drops "$SUTANDO_WORKSPACE is honored as a legacy escape hatch" —
  no longer accurate post-#1440. Replaced with "no longer honored by
  the resolver; triggers deprecation warning + auto-migration".
- Cites PR #1440 in the M0 line directly.

**Changed section — additions:**
- `sync-memory.sh` deprecation timer made explicit: "deprecated and
  will be REMOVED in v0.4.0". Replaces the soft "deprecated in favor of"
  framing.
- New entry summarizing the v0.3.0 docs sweep itself: memory-sync→
  workspace-sync rename, release-process consolidation + proposal
  deletes, workspace-design 2-space rewrite, contract/config updates.

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
…state, 2-space model

Part 4 of the v0.3.0 docs sweep (PR #1472).

**docs/workspace-design.md — full rewrite around 2-space model.**

Per owner directive 2026-06-04 03:11Z (memory `project_workspace_only_no_memory`): the prior 3-space framing (Code / State / Memory) is structurally stale. Memory is not a top-level peer of Workspace — it's a sub-path inside `<workspace>/.claude-sutando/projects/<slug>/memory/` that gets synced alongside notes, build_log, etc.

The shift: container identity (Code vs Workspace) and sync policy (per-sub-path, configured via `vault.sync.*`) are separated. The 3-space model conflated them.

Preserved from the prior doc:
- The bug-class history (~28 sites of workspace-vs-repo divergence pre-M0)
- The Decisions section as historical record explaining why the model changed

Rewrote:
- Title + status: "3-space" → "2-space"
- TL;DR table: 3 rows → 2 rows
- Quick-decision flowchart: simplified to "ships with every install?"
- Resolvers section: kept Code path, rewrote Workspace path with sub-path sync table
- New historical-note callout at top explaining the change

**docs/workspace-contract.md — update status note.**

"v0.8 (target shape)" → "v0.3.0 (ratified)". PR #1440 has merged; the
"after-#1440" hedge is now historical. Updates the version banner to
reflect the post-merge reality + cites the full PR ledger that
ratified the contract (#1395-1463 + #1440).

**docs/workspace-config.md — drop $SUTANDO_WORKSPACE from resolution order.**

Pre-1440 the env var was honored as a legacy escape hatch. Post-1440
it's no longer honored — only a deprecation warning + auto-migrate at
startup. Updated the resolution-order list to a 3-item flow (config
files + baked-in fallback only); added a separate Note explaining the
env var's current status for users migrating from old configurations.

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
…sweep summary

Part 5 of the v0.3.0 docs sweep (PR #1472).

**Added section — workspace-rollup line sharpening:**
- M0 line now explicitly says "supersedes the v0.1.0 `~/.sutando/workspace/` default"
  (option A from the 2026-06-06 01:41Z question — link the new entry
  to the v0.1.0 entry it replaces, rather than annotating the v0.1.0
  entry retroactively).
- Drops "$SUTANDO_WORKSPACE is honored as a legacy escape hatch" —
  no longer accurate post-#1440. Replaced with "no longer honored by
  the resolver; triggers deprecation warning + auto-migration".
- Cites PR #1440 in the M0 line directly.

**Changed section — additions:**
- `sync-memory.sh` deprecation timer made explicit: "deprecated and
  will be REMOVED in v0.4.0". Replaces the soft "deprecated in favor of"
  framing.
- New entry summarizing the v0.3.0 docs sweep itself: memory-sync→
  workspace-sync rename, release-process consolidation + proposal
  deletes, workspace-design 2-space rewrite, contract/config updates.

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
…ch" — $SUTANDO_WORKSPACE removed in v0.8

CLAUDE.md L60 and 4 SKILL.md files asserted `$SUTANDO_WORKSPACE` is
"honored as a legacy escape hatch with a one-time deprecation warning."
That was the pre-v0.8 contract. Per `src/sutando_config.py:14` and
`:292-332`, post-v0.8 (#1440) the env var is no longer honored: if set,
a one-time warning fires and the value is **ignored**.

The prior SKILL.md sweep in this PR (commit 1b0889a) propagated the
wrong claim to 4 files; this commit corrects all 5 sites in one pass.

- `CLAUDE.md:60`
- `skills/proactive-loop/SKILL.md:42`
- `skills/obsidian-vault/SKILL.md:7`
- `skills/discord-voice/SKILL.md:75`
- `skills/db-browser/SKILL.md:17`

Caught by owner's "what else" sweep prompt 2026-06-06 12:11Z
(task-1780747866247) — verifying my own previous sweep against
`src/sutando_config.py` surfaced the regression in 1b0889a.

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
…orkspace, not a separate space

Per owner directive 2026-06-04 closing PR #1444 (memory
`project_workspace_only_no_memory`) + #1440 ratification: **Code +
Workspace** are the only 2 top-level spaces. Memory is a sub-path under
`<workspace>/.claude-sutando/projects/<slug>/memory/`. CLAUDE.md still
framed Memory as a separate space and listed the pre-M2 memory paths
(`$CLAUDE_CONFIG_DIR/projects/.../memory`) as default — both stale.

This commit rewrites three sites:

1. **L54 — Workspace contract intro:** 3-space → 2-space framing.
   "Code + Workspace ... sync is a property of sub-paths, not a separate
   container." Memory listed alongside other workspace sub-paths.
   `$SUTANDO_MEMORY_DIR` override + `$SUTANDO_PRIVATE_DIR` legacy alias
   noted but folded into the workspace section.

2. **L169-180 — Memory section paths:** rewritten to point at the
   correct location `<workspace>/.claude-sutando/projects/<slug>/memory/`
   instead of `$CLAUDE_CONFIG_DIR/projects/.../memory`. The build_log
   path also corrected to `<workspace>/build_log.md`.

3. **L297-298 — Learn-from-demonstration save paths:** same correction.
   Memory writes now target the workspace sub-path, not the legacy
   `$CLAUDE_CONFIG_DIR`-anchored path (which had been the pre-M2
   convention).

Caught by owner's "what else" prompt 2026-06-06 12:11Z
(task-1780747866247); owner directed apply via task-1780748116832.

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
…0 helper

Owner DM 2026-06-06 13:27Z (task-1780752451373 + task-1780752464055):
the docstring still claimed `~/.sutando/workspace` was "the canonical
default" and `$SUTANDO_WORKSPACE` was honored. Both wrong post-#1440 /
v0.8 — canonical default is `<repo>/workspace/` and the env var is no
longer honored for resolution.

The module previously re-implemented its own 3-step resolver (env-var,
.env-file walk-up, hardcoded `~/.sutando/workspace` default) — all three
steps are obsolete post-v0.8. Replaced with a thin delegate that walks
up to find `scripts/sutando-config.sh` and shells out to the canonical
M0 helper, so this script can never drift from the resolver contract
again.

The walk-up logic (5-level realpath() bounded walk) is retained because
the symlinks-into-the-repo concern still applies (when invoked via
`~/.claude/skills/agent-registry/...`). Symptom of the prior bug was
silent-data-loss-equivalent: registry writes to wrong dir while readers
look at the right one.

Smoke: resolves correctly to `/Users/qingyunwu/Documents/github/sutando/workspace`
on this MBP.

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
…ce/` Chi-fleet probe (post-#1440)

Owner DM 2026-06-06 13:28Z (task-1780752526451 + task-1780752549313:
"Why this is still not fixed?"): three sites in the submit-use-case
skill still referenced `/Users/wangchi/.sutando/workspace/` as Chi's
fleet-detection signal. Pre-#1440 that path existed on Chi's hosts;
post-#1440 the workspace is in-repo (`<repo>/workspace/`), so the
path-existence probe always returns false and the fleet-detection
silently broke.

3 sites updated:
- `skills/submit-use-case/scripts/submit_use_case.py` — `is_chi_fleet()`
  drops the `Path("/Users/wangchi/.sutando/workspace").exists()` check;
  retains the `SUTANDO_FLEET_OWNER=chi` env signal (Chi sets this once
  per host in `.env`, no longer needs the path probe).
- `skills/submit-use-case/SKILL.md` L59 — prose updated to match.
- `skills/submit-use-case/README.md` L64 — same.

If Chi-fleet auto-detection regresses (the env var isn't set on a host
that used to be detected via the path), the user can re-enable by
setting `SUTANDO_FLEET_OWNER=chi` in their `.env` — same setup
documented in the README.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
john-the-dev pushed a commit that referenced this pull request Aug 3, 2026
…istence (#2540)

* tidy(dashboard): extract schedule parsing, validation and atomic persistence

Move cron parsing, schedule validation and the locked read-modify-write
against per-host crons.json out of src/dashboard.py into a new focused
module, src/dashboard_schedules.py. dashboard.py keeps path resolution,
HTTP adaptation and rendering.

Behavior-preserving: no cron syntax broadened, no response object or note
string changed. dashboard.py 1071 -> 912 lines; all three os.replace sites
move to the module.

The split point: the adapter resolves the path, the domain module receives
it. dashboard.py keeps _crons_path() (workspace + host-label resolution is
deployment knowledge); dashboard_schedules.py takes a Path. Dashboard-facing
names are retained as thin wrappers so HTTP handlers are untouched, and
_CRONS_LOCK is re-exported as the SAME object -- a second lock would
silently serialize nothing.

Tests:
- tests/dashboard-schedules-module.test.py: 63 direct checks (cron token
  forms, fixed-datetime next-run, missing/malformed/non-list reads, atomic
  round trip, temp cleanup on failed write, add/cron-only edit/prompt-skill
  switch/delete, preservation of unrecognized scheduler fields, exact
  response objects, 24-thread concurrency, upsert racing delete).
- tests/dashboard-schedule-delegation.test.py: anti-regression gate. Asserts
  delegation is real and scans dashboard.py for the atomic-write primitives
  so a future route cannot rebuild its own transaction.

Both new guards were mutation-tested: removing _CRONS_LOCK drops the
concurrency test to 1/24 persisted, and reintroducing a route-local
os.replace trips the delegation gate.

One test seam moved. dashboard-editable-schedules.test.py widened the
read->write window by patching dash._read_crons; that patch is inert after
the extraction. Measured rather than assumed: the test is NOT vacuous (it
still fails without the lock), but its detection was no longer engineered --
it rode on filesystem timing. The patch now targets
dashboard_schedules.read_crons, and the module docstring records why the
transactions call read_crons through the module global.

Live loopback verification (18/18) drives the real Handler and real
do_POST/do_GET/do_DELETE against a disposable schedule file. Note for anyone
writing similar harnesses: $SUTANDO_WORKSPACE was retired by #1440 and is
now silently ignored, so using it as the "temporary workspace" seam resolves
to the real workspace and would rewrite the owner's live crons.json. This
harness redirects _crons_path() only.

Pre-existing and untouched: tests/dashboard-quota-staleness.test.py fails
check E4 at the parent commit (it asserts no warning glyph while rendering
the whole dashboard, so any other panel's glyph trips it). npm run test:py
is fail-fast and stops there identically at both commits: 63 files, exit 1.
No new failures. Fixing E4 belongs in its own PR.

Adds the architecture rule to CLAUDE.md, regenerates AGENTS.md, and adds a
"Presentation adapters vs domain/storage" section to
docs/architecture-boundaries.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVTTSas6EpK7QGnsDy8ZEG

* docs(src-map): index dashboard_schedules.py

Adding a module under src/ requires regenerating docs/src-map.md; CI's
"refuse docs/src-map.md stale vs src/" check caught it. Generated output
only -- the one-line purpose is taken from the module's own header comment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVTTSas6EpK7QGnsDy8ZEG

* chore(src-map): regenerate after merging main

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
sonichi added a commit that referenced this pull request Aug 4, 2026
…ting to the live queue

@john-the-dev's blocker on d1767d4: the test resolved the workspace from the
host config and wrote its probe into the caller's REAL queue — the directory
watch-tasks-stream.sh is concurrently watching. The watcher could claim and
execute the probe as a live owner task before the EXIT trap removed it, and
cleanup cannot retract a reply already delivered. A synthetic filename lowers
collision odds; it does not isolate a live consumer.

The test now builds its own workspace and pins it via the repo's supported
escape hatch (SUTANDO_TEST_MODE=1 + SUTANDO_WORKSPACE, src/sutando_config.py
~336 — the env var alone is ignored post-v0.8/#1440).

Passing a temp dir is not isolation, so the RESOLVED path is asserted before
anything is written: the test aborts with exit 1 if the workspace is not the
temp dir, or if it is the live one. Verified by disabling the hatch — it
refuses rather than falling back to writing live. Paths are compared with
`pwd -P` because mktemp hands back /tmp, a symlink to /private/tmp on macOS.

The legacy-direction probe (case 4) is the one write outside the temp tree,
because it must go where the BUG reads. It is safe and now guarded:
watch-tasks-stream.sh resolves TASKS_DIR from `sutando-config.sh workspace`
(line 38), so <repo>/tasks/ has no consumer, and the case skips if that path
ever equals the resolved queue.

Evidence: 5/5 pass with no env preset; live workspace file count unchanged and
`find <workspace> -name 'task-zz-hooktest-*'` empty; reverting the hook to the
repo-root form still fails exactly 3 of 5.

Stand: Echo Act IV Mini
john-the-dev pushed a commit that referenced this pull request Aug 5, 2026
…never blocked (#2657)

* fix(check-pending-tasks): the Stop hook watched <repo>/tasks/, so it never blocked

The hook exists to stop a session finishing while tasks are unprocessed. It
resolved its queue as `$(dirname "$0")/../tasks` — the repo root — while every
producer and consumer writes <workspace>/tasks/. On a default install those are
different directories, so it read an empty one and emitted `{}` on every Stop.

Proven both directions at origin/main with the same probe file: placed in
<workspace>/tasks/ the hook returns `{}`; placed in <repo>/tasks/ it returns a
block decision. The code works — it was pointed at a directory nothing writes.
The failure is silent because a broken run's only possible output is the same
`{}` a healthy empty queue produces.

Resolve via scripts/sutando-config.sh workspace, and fall back to
<repo>/workspace rather than <repo> — falling back to the repo root is the bug
being fixed and would re-disable the hook without a symptom.

The new test pins the direction, not just the happy path: a file in the legacy
<repo>/tasks/ must NOT block. Reverting only the hook fails 3 of its 5 cases.

Rebuild of the approved-then-parked #1272, not a cherry-pick: that patch used
${SUTANDO_WORKSPACE:-$HOME/.sutando/workspace}, and since v0.8/#1440 the env var
is no longer honored for resolution and the literal home path is the
hardcoded-path class REVIEW.md rejects.

Stand: Echo Act IV Mini

* test(check-pending-tasks): build an isolated workspace instead of writing to the live queue

@john-the-dev's blocker on d1767d4: the test resolved the workspace from the
host config and wrote its probe into the caller's REAL queue — the directory
watch-tasks-stream.sh is concurrently watching. The watcher could claim and
execute the probe as a live owner task before the EXIT trap removed it, and
cleanup cannot retract a reply already delivered. A synthetic filename lowers
collision odds; it does not isolate a live consumer.

The test now builds its own workspace and pins it via the repo's supported
escape hatch (SUTANDO_TEST_MODE=1 + SUTANDO_WORKSPACE, src/sutando_config.py
~336 — the env var alone is ignored post-v0.8/#1440).

Passing a temp dir is not isolation, so the RESOLVED path is asserted before
anything is written: the test aborts with exit 1 if the workspace is not the
temp dir, or if it is the live one. Verified by disabling the hatch — it
refuses rather than falling back to writing live. Paths are compared with
`pwd -P` because mktemp hands back /tmp, a symlink to /private/tmp on macOS.

The legacy-direction probe (case 4) is the one write outside the temp tree,
because it must go where the BUG reads. It is safe and now guarded:
watch-tasks-stream.sh resolves TASKS_DIR from `sutando-config.sh workspace`
(line 38), so <repo>/tasks/ has no consumer, and the case skips if that path
ever equals the resolved queue.

Evidence: 5/5 pass with no env preset; live workspace file count unchanged and
`find <workspace> -name 'task-zz-hooktest-*'` empty; reverting the hook to the
repo-root form still fails exactly 3 of 5.

Stand: Echo Act IV Mini
sonichi added a commit that referenced this pull request Aug 8, 2026
qingyun-wu's [P1]: the test built fixtures under the RESOLVED workspace, so a
checkout pointed at a real workspace created and deleted files there — leaving
`data/`, `data/generated/ep999-bundle/`, `data/usage/` and `data/memory-snapshots/`
behind, and writing a probe named `conversation.sqlite` when absent.

Now builds a fixture repo whose workspace is `<fixture>/workspace` and imports
`send_allowlist` from the fixture's own `src/`, so resolution lands inside the
tempdir. Verified: 0 probe artifacts and no `data/generated/` in either the
operator workspace or the worktree afterwards.

Two of their observations are pinned as assertions rather than fixed silently:

The suggested `SUTANDO_WORKSPACE=<tmpdir>` redirect does NOT work — v0.8 / #1440
stopped honoring it for resolution. A fixture repo with `sutando.config.local.json`
is the mechanism that does, matching `tests/vault-carrier-*`.

Their run inverted because `SEND_ALLOWED_PREFIXES` allows `/private/tmp/sutando-*`
and their worktree lived there, making every negative check pass by prefix. The
test now asserts its own fixture is not prefix-covered, so that fails loudly
instead of quietly inverting.

Regression value verified against origin/main, not `git checkout --` (which
restores this branch's committed fix): root absent fails 2 checks, and widening to
bare `data/` fails 5 including all three exposures and the symlink escape.

Stand: Echo Act IV Mini
sonichi added a commit that referenced this pull request Aug 16, 2026
…live state (#2738)

* fix(tests): #2615 rebound 2 of 9 import-time paths, so one still hit live state

Sutando-Pro found the next member while chasing something else and did not claim it.
Verified from the artifacts, and the class is larger than either of us said.

`slack-bridge.py` binds NINE module-level paths from the resolved workspace AT
IMPORT (AST-enumerated, module scope only):

  TASKS_DIR RESULTS_DIR STATE_DIR INBOX_DIR ARCHIVE_TASKS_DIR ARCHIVE_RESULTS_DIR
  OWNER_ACTIVITY_FILE ACCESS_BACKUP_FILE PENDING_REPLIES_FILE

#2615 rebound two of them. Its own fixture comment explains the mechanism exactly
right — a constant derived from STATE_DIR at import does not re-derive when STATE_DIR
is rebound later — and then enumerates one member of the class instead of the class.
PENDING_REPLIES_FILE (:494) is bound identically to OWNER_ACTIVITY_FILE (:103) and was
left aimed at the operator's real workspace.

The same fixture also sets SUTANDO_WORKSPACE for isolation. `resolve_workspace()`
ignores that variable (removed v0.8/#1440), so that line does nothing and the manual
rebinding was carrying the whole burden.

Rather than name nine constants and wait for a tenth, `rebind_workspace()` discovers
them by RELATIONSHIP: every module attribute that is a Path under the module's own
resolved root gets re-pointed at the temp root, preserving its relative position. A
constant added later is covered the day it is added.

Evidence, measured:

  live state/slack-pending-replies.json   4 entries, channels C1 / D_TEST
  pre-fix fixture style (2 names)         PENDING_REPLIES_FILE under the real root: True
  with rebind_workspace()                 nothing left under the real root
  tofu fixture                            7/7 pass, live file mtime unchanged

The coverage test has a control first: discovery must find >= 8 paths and specifically
STATE_DIR, OWNER_ACTIVITY_FILE and PENDING_REPLIES_FILE, because a vacuous discovery
would make every other assertion pass for free.

Scope stated honestly: this fixes SLACK's fixture. discord-bridge binds 12 such paths
and telegram-bridge 9 by the same pattern — the helper covers them, but their fixtures
have not been converted here, and I have not audited which of their tests leak.

Not touched: the 4 polluted live entries. Deleting rows from the operator's state is
his call, not a test fix.

I wrote two things wrong on the way and both are worth the reader knowing: I removed
`self._orig_state` while leaving two references to it, which failed 7/7 until fixed;
and the outbox rows Pro mentioned carry no status/sent field, so whether anything was
delivered is not determinable from that file — I am not asserting a send.

Stand: Echo Act IV Mini

* fix(tests): setdefault is not isolation, and the lint said so

Sutando-Pro corrected their own #2738 review: three checks were failing, not one —
they had read the rollup while two were still in flight. Verified at head 7680733:
3 FAILURE / 11 SUCCESS. Both actionable failures share one root cause, my test file.

`lint-hermetic-bridge-tests` was right: `os.environ.setdefault("CLAUDE_CONFIG_DIR", ...)`
is a NO-OP on any machine where that variable is already set — which is every developer
machine and this host — so the bridge resolved the operator's real channel allowlist at
import. The lint's own wording names it: "a comment saying 'hermetic' is not isolation."

The CI failure was the same file: the lint's test suite asserts "whole-tree run is green
on the current tree", so my violation failed that too. One fix closes both.

My first attempt still failed, and the lint's own tests explain why — they document the
detector's limits as executable cases: "isolation + load inside ONE function reads as
violation" and "temp dir routed through a variable reads as violation", against a
control of "direct temp assignment + unconditional seed IS clean". I had done both
forbidden things. Now module-level, `mkdtemp` inline, seed unconditional.

  lint-hermetic-bridge-tests: ok (70 bridge-importing tests scanned, 47 grandfathered,
                                  0 mitigated)
  tests/lint-hermetic-bridge-tests.test.py: PASS

Also added the five edge cases Pro enumerated for the coverage gate, since diff coverage
cannot be reached through a bridge import: the `_original_root` name-chain fallback, the
str-vs-Path split (a str root must be restored as a str), the `ValueError` skip for paths
outside the root, the `AssertionError` when no root exists, and discovery on a rootless
module. 9 tests, all pass.

Stand: Echo Act IV Mini

* fix(tests): the helper header claimed a scope the implementation does not have

The module docstring said it points *every* import-time path constant at a temp
root. It rebases only Paths under the module's own resolved root, so a second root
is untouched — which is why the TOFU run still reaches the live ~/.claude channel
config (grandfathered by the lint). Header now states that boundary.

Trimmed the three helper docstrings and the new test's module docstring to the
2-line comment limit. Behavior unchanged.

Stand: Echo Act IV Mini

* fix(tests): state the constraint without the PR reference

AGENTS.md forbids PR/issue/person references in comments. Two came from this
branch and both are gone:

- the docstring said "not rebound by #2615"; it now says what the reader needs —
  PENDING_REPLIES_FILE is derived the same way as OWNER_ACTIVITY_FILE, so a rebind
  helper covering one and not the other leaves a path on live state
- the test name carried the same number (`test_the_member_2615_missed_is_covered`)
  and is now `test_pending_replies_file_is_rebound_like_its_sibling`

The review named only the docstring. The method name is the same defect one line
up and this file is new in this branch, so both are mine to fix; catching it here
avoids a fourth round on one policy item.

Left alone: the seven #899 / #2118 / #2124 references in
tests/slack-bridge-tofu-enroll.test.py. Measured against main — none is in this
diff, so trimming them would smuggle an unrelated change into a fix.

No behavioural change. Both suites pass.

Stand: Echo Act IV Mini
qingyun-wu pushed a commit that referenced this pull request Aug 23, 2026
install/uninstall/pool-status ported from #884 (workspace via the M0
helper — env override retired post-#1440; plist PATH captured from the
installer instead of a literal); pool-lead-daemon.py binds PoolLead to
the live workspace with bounded-age liveness and metrics, unlinking its
beat on exit so followers degrade immediately; follower skill routed
through acquire_work.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
qingyun-wu pushed a commit that referenced this pull request Aug 24, 2026
… not prose

SKILL.md tells a follower its loop is /proactive-loop with "the rest of the
body unchanged", and separately tells it not to write core-status.json. Step 0
writes that file every pass, so the two cannot both be followed and only the
prose stands between a follower and the MAIN core's owner-facing record. The
Discord bridge renders `step` live and graceful-restart gates busy() on
"running + fresh ts" (#3156), so this is not only a misleading dashboard.

The guard is on the VALUE, not the key's presence. install-core-pool.sh
assigns 1..N; a main core carries something else -- 'legacy' on this host --
or nothing at all. A presence-keyed guard would silence a MAIN core, and a
core that never writes reads as idle, which authorises exactly the kill this
is meant to prevent. Unrecognised values therefore WRITE: silence is the
dangerous direction and must not be reachable by accident.

Mutation control, run both ways on the shipped script: the presence-keyed
variant fails 5 assertions including `SUTANDO_CORE_ID=legacy` no-opping. The
value-keyed one passes 19/19.

The suite drives the shipped script rather than a copy. `SUTANDO_WORKSPACE` is
retired (v0.8/#1440) so the write target cannot be redirected, and an earlier
attempt to isolate via a temp skeleton silently resolved to a real workspace
outside the repo. So it snapshots the live record and restores it in a finally,
and asserts byte-identity afterwards.

Reported by Sutando-Mini on #3314, which proposed the presence-keyed form; the
finding was right and the predicate needed changing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
qingyun-wu pushed a commit that referenced this pull request Aug 24, 2026
@john-the-dev's standing blocker. `src/claim_task.py::_workspace_root()` used
both mechanisms v0.8 / #1440 retired: it read `$SUTANDO_WORKSPACE`, which the
resolver no longer honours, then fell back to the hardcoded
`~/.sutando/workspace` that CLAUDE.md names as the anti-pattern.

His measurement is the reason this matters more than a wrong constant: on a
packaged desktop install that path EXISTS as a stale Jul-27 leftover holding only
`state/`. A claim would not error — it would create empty `tasks/`/`results/`
under the abandoned tree while the live queue sat elsewhere, and the pool would
read that as "no work".

Replaced with the canonical resolver, matching the convention every sibling in
src/ already uses. Verified in his own shape:

  before (his run):  claim_task=~/.sutando/workspace  canonical=.../space.ag2.app/workspace  SAME? NO
  after:             claim_task == canonical                                                SAME? YES
                     legacy ~/.sutando/workspace used?  NO
                     $SUTANDO_WORKSPACE=/tmp/... diverts it?  NO (retired, warns and is ignored)

3 suites touching claim_task pass.

Worth recording why this escaped: `scripts/lint-workspace-resolution.sh` DOES
catch it — it names `44:+ env = os.environ.get("SUTANDO_WORKSPACE")` on the
pre-fix range. It never ran. This PR's base is `feat/sutando-server-v0`, and a
stacked base filters the gated workflows out, so the head carries 2 check-runs
where a main-based sibling carries 17-19. The guard existed and was bypassed by
the base, which is the concrete cost of the phantom-green class.

Signed: @sutando-qingyun-001:ag2.space
qingyun-wu pushed a commit that referenced this pull request Aug 26, 2026
@john-the-dev's standing blocker. `src/claim_task.py::_workspace_root()` used
both mechanisms v0.8 / #1440 retired: it read `$SUTANDO_WORKSPACE`, which the
resolver no longer honours, then fell back to the hardcoded
`~/.sutando/workspace` that CLAUDE.md names as the anti-pattern.

His measurement is the reason this matters more than a wrong constant: on a
packaged desktop install that path EXISTS as a stale Jul-27 leftover holding only
`state/`. A claim would not error — it would create empty `tasks/`/`results/`
under the abandoned tree while the live queue sat elsewhere, and the pool would
read that as "no work".

Replaced with the canonical resolver, matching the convention every sibling in
src/ already uses. Verified in his own shape:

  before (his run):  claim_task=~/.sutando/workspace  canonical=.../space.ag2.app/workspace  SAME? NO
  after:             claim_task == canonical                                                SAME? YES
                     legacy ~/.sutando/workspace used?  NO
                     $SUTANDO_WORKSPACE=/tmp/... diverts it?  NO (retired, warns and is ignored)

3 suites touching claim_task pass.

Worth recording why this escaped: `scripts/lint-workspace-resolution.sh` DOES
catch it — it names `44:+ env = os.environ.get("SUTANDO_WORKSPACE")` on the
pre-fix range. It never ran. This PR's base is `feat/sutando-server-v0`, and a
stacked base filters the gated workflows out, so the head carries 2 check-runs
where a main-based sibling carries 17-19. The guard existed and was bypassed by
the base, which is the concrete cost of the phantom-green class.

Signed: @sutando-qingyun-001:ag2.space
(cherry picked from commit 05077e7)
qingyun-wu pushed a commit that referenced this pull request Aug 26, 2026
@john-the-dev's standing blocker. `src/claim_task.py::_workspace_root()` used
both mechanisms v0.8 / #1440 retired: it read `$SUTANDO_WORKSPACE`, which the
resolver no longer honours, then fell back to the hardcoded
`~/.sutando/workspace` that CLAUDE.md names as the anti-pattern.

His measurement is the reason this matters more than a wrong constant: on a
packaged desktop install that path EXISTS as a stale Jul-27 leftover holding only
`state/`. A claim would not error — it would create empty `tasks/`/`results/`
under the abandoned tree while the live queue sat elsewhere, and the pool would
read that as "no work".

Replaced with the canonical resolver, matching the convention every sibling in
src/ already uses. Verified in his own shape:

  before (his run):  claim_task=~/.sutando/workspace  canonical=.../space.ag2.app/workspace  SAME? NO
  after:             claim_task == canonical                                                SAME? YES
                     legacy ~/.sutando/workspace used?  NO
                     $SUTANDO_WORKSPACE=/tmp/... diverts it?  NO (retired, warns and is ignored)

3 suites touching claim_task pass.

Worth recording why this escaped: `scripts/lint-workspace-resolution.sh` DOES
catch it — it names `44:+ env = os.environ.get("SUTANDO_WORKSPACE")` on the
pre-fix range. It never ran. This PR's base is `feat/sutando-server-v0`, and a
stacked base filters the gated workflows out, so the head carries 2 check-runs
where a main-based sibling carries 17-19. The guard existed and was bypassed by
the base, which is the concrete cost of the phantom-green class.

Signed: @sutando-qingyun-001:ag2.space
(cherry picked from commit 05077e7)
qingyun-wu pushed a commit that referenced this pull request Aug 26, 2026
@john-the-dev's standing blocker. `src/claim_task.py::_workspace_root()` used
both mechanisms v0.8 / #1440 retired: it read `$SUTANDO_WORKSPACE`, which the
resolver no longer honours, then fell back to the hardcoded
`~/.sutando/workspace` that CLAUDE.md names as the anti-pattern.

His measurement is the reason this matters more than a wrong constant: on a
packaged desktop install that path EXISTS as a stale Jul-27 leftover holding only
`state/`. A claim would not error — it would create empty `tasks/`/`results/`
under the abandoned tree while the live queue sat elsewhere, and the pool would
read that as "no work".

Replaced with the canonical resolver, matching the convention every sibling in
src/ already uses. Verified in his own shape:

  before (his run):  claim_task=~/.sutando/workspace  canonical=.../space.ag2.app/workspace  SAME? NO
  after:             claim_task == canonical                                                SAME? YES
                     legacy ~/.sutando/workspace used?  NO
                     $SUTANDO_WORKSPACE=/tmp/... diverts it?  NO (retired, warns and is ignored)

3 suites touching claim_task pass.

Worth recording why this escaped: `scripts/lint-workspace-resolution.sh` DOES
catch it — it names `44:+ env = os.environ.get("SUTANDO_WORKSPACE")` on the
pre-fix range. It never ran. This PR's base is `feat/sutando-server-v0`, and a
stacked base filters the gated workflows out, so the head carries 2 check-runs
where a main-based sibling carries 17-19. The guard existed and was bypassed by
the base, which is the concrete cost of the phantom-green class.

Signed: @sutando-qingyun-001:ag2.space
(cherry picked from commit 05077e7)
qingyun-wu pushed a commit that referenced this pull request Aug 26, 2026
@john-the-dev's standing blocker. `src/claim_task.py::_workspace_root()` used
both mechanisms v0.8 / #1440 retired: it read `$SUTANDO_WORKSPACE`, which the
resolver no longer honours, then fell back to the hardcoded
`~/.sutando/workspace` that CLAUDE.md names as the anti-pattern.

His measurement is the reason this matters more than a wrong constant: on a
packaged desktop install that path EXISTS as a stale Jul-27 leftover holding only
`state/`. A claim would not error — it would create empty `tasks/`/`results/`
under the abandoned tree while the live queue sat elsewhere, and the pool would
read that as "no work".

Replaced with the canonical resolver, matching the convention every sibling in
src/ already uses. Verified in his own shape:

  before (his run):  claim_task=~/.sutando/workspace  canonical=.../space.ag2.app/workspace  SAME? NO
  after:             claim_task == canonical                                                SAME? YES
                     legacy ~/.sutando/workspace used?  NO
                     $SUTANDO_WORKSPACE=/tmp/... diverts it?  NO (retired, warns and is ignored)

3 suites touching claim_task pass.

Worth recording why this escaped: `scripts/lint-workspace-resolution.sh` DOES
catch it — it names `44:+ env = os.environ.get("SUTANDO_WORKSPACE")` on the
pre-fix range. It never ran. This PR's base is `feat/sutando-server-v0`, and a
stacked base filters the gated workflows out, so the head carries 2 check-runs
where a main-based sibling carries 17-19. The guard existed and was bypassed by
the base, which is the concrete cost of the phantom-green class.

Signed: @sutando-qingyun-001:ag2.space
(cherry picked from commit 05077e7)
qingyun-wu pushed a commit that referenced this pull request Aug 26, 2026
@john-the-dev's standing blocker. `src/claim_task.py::_workspace_root()` used
both mechanisms v0.8 / #1440 retired: it read `$SUTANDO_WORKSPACE`, which the
resolver no longer honours, then fell back to the hardcoded
`~/.sutando/workspace` that CLAUDE.md names as the anti-pattern.

His measurement is the reason this matters more than a wrong constant: on a
packaged desktop install that path EXISTS as a stale Jul-27 leftover holding only
`state/`. A claim would not error — it would create empty `tasks/`/`results/`
under the abandoned tree while the live queue sat elsewhere, and the pool would
read that as "no work".

Replaced with the canonical resolver, matching the convention every sibling in
src/ already uses. Verified in his own shape:

  before (his run):  claim_task=~/.sutando/workspace  canonical=.../space.ag2.app/workspace  SAME? NO
  after:             claim_task == canonical                                                SAME? YES
                     legacy ~/.sutando/workspace used?  NO
                     $SUTANDO_WORKSPACE=/tmp/... diverts it?  NO (retired, warns and is ignored)

3 suites touching claim_task pass.

Worth recording why this escaped: `scripts/lint-workspace-resolution.sh` DOES
catch it — it names `44:+ env = os.environ.get("SUTANDO_WORKSPACE")` on the
pre-fix range. It never ran. This PR's base is `feat/sutando-server-v0`, and a
stacked base filters the gated workflows out, so the head carries 2 check-runs
where a main-based sibling carries 17-19. The guard existed and was bypassed by
the base, which is the concrete cost of the phantom-green class.

Signed: @sutando-qingyun-001:ag2.space
(cherry picked from commit 05077e7)
sonichi added a commit that referenced this pull request Aug 27, 2026
Both xargs lanes sent every standalone suite through a shared resolved
workspace, so concurrent suites raced on it. The reviewer's pair reproduces:
discord-bridge-reply-directive fails when dm-result-adoption-gap appends
state/outbox.log inside its snapshot window.

The resolver already has the hatch this needs. SUTANDO_TEST_MODE=1 was added
in #1440 precisely to let the suite redirect the workspace to per-test tmp
dirs; no contract change is required. Each worker now gets its own mktemp -d
and removes it after.

  unpatched shape: reply_rc=1 dm_rc=0   (reproduced twice)
  patched shape:   reply_rc=0 dm_rc=0

Stand: Echo Act IV Mini
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.

1 participant