Skip to content

fix(core-status): TS readers honor SUTANDO_WORKSPACE + isolate tests (closes #840 properly) - #849

Merged
sonichi merged 1 commit into
mainfrom
fix/core-status-path-and-test-isolation
May 18, 2026
Merged

sonichi merged 1 commit into
mainfrom
fix/core-status-path-and-test-isolation

Conversation

@sonichi

@sonichi sonichi commented May 18, 2026

Copy link
Copy Markdown
Owner

Summary

Two stacked issues fixed in one shot. Closes #840 properly (PR #841's beforeEach was a partial fix).

1. Production bug — silent path divergence

Python writers migrated core-status.json to $SUTANDO_WORKSPACE in #836 per the CLAUDE.md workspace contract. TS readers in src/web-client.ts:readCoreStatus and src/inline-tools.ts:getCoreStatusTool still read it from REPO_ROOT via import.meta.url-relative paths. Same silent-divergence shape as #821/#842/#843 fixed for other paths — voice-agent reading "core idle" while Python actually wrote "core working" elsewhere.

Fix: both TS readers now join(WORKSPACE_DIR, 'core-status.json'). WORKSPACE_DIR comes from resolveWorkspace().

2. Test flake (#840)

tests/agent-state-endpoint.test.ts and tests/get-core-status-tool.test.ts both wrote to <REPO_ROOT>/core-status.json. node:test runs files in parallel by default. The get-core-status-tool test writes status:'running' as one of its cases right when agent-state-endpoint asserts state:'listening'. Race manifested as expected listening, got working on whichever subtest called effectiveAgentState() during the foreign 'running' write.

Two PRs (#836, #832) already flaked on this today; PR #843's CI also failed but merged via non-required-check semantics. PR #841 added per-subtest beforeEach reset which helped within-suite ordering but didn't address the cross-file race.

Fix: each test process now creates its own mkdtempSync workspace dir and exports SUTANDO_WORKSPACE=$TEMP_DIR. The spawned web-client inherits the env; the in-process tool import reads the same temp dir. No more shared state, no more race.

Test plan

  • tests/agent-state-endpoint.test.ts — 9/9 pass
  • tests/get-core-status-tool.test.ts — 6/6 pass
  • Running both together (the failure mode pre-fix): 15/15 pass
  • Full TS suite: 173/173 pass

🤖 Generated with Claude Code

@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.

Right fix for both stacked issues. CI green on both checks (tsc+tests, license/cla).

(1) Production bug — silent path divergence. Verified the audit is complete: grepping src/ for core-status.json shows exactly two TS readers — src/inline-tools.ts:609 (getCoreStatusTool) and src/web-client.ts:3082 (readCoreStatus), both fixed here. WORKSPACE_DIR is correctly imported via resolveWorkspace() (the canonical #821 helper) at the top of both files. Comment lineage references (#836/#842/#843) accurately frames the fix.

Subtle thing done right: in web-client.ts:readCoreStatus, the statSync(url) call ALSO switched to statSync(statusPath) — both the read AND the mtime probe migrated. Easy one to miss.

(2) Test isolation. Per-test-process mkdtempSync + SUTANDO_WORKSPACE env override is the proper fix vs the prior beforeEach neutralize-via-write approach (#841 patch). Spawning the child web-client with SUTANDO_WORKSPACE: TEMP_WORKSPACE in the env block correctly propagates the isolation into the child process, so concurrent test files can't race the shared <REPO_ROOT>/core-status.json. Comment correctly attributes the voice-state.json stash/restore as a still-live hack pending a separate migration — honest framing.

Answer to your questions:

  1. Other TS readers of core-status.json: none missed. Two readers total in TS, both in this PR. (Python side has src/agent-api.py:286 and src/health-check.py:598 — out of scope; health-check still reads REPO_DIR/core-status.json which is technically the workspace contract gap from #836's tail, but it's the next PR's problem.)

  2. voice-state.json migration follow-up: YES, definitely. Same anti-pattern:

    • src/voice-agent.ts:841 writes voice-state.json via relative path (CWD-dependent — currently writeFileSync('voice-state.json', …) from voice-agent's working dir).
    • src/web-client.ts:3107 reads new URL('../voice-state.json', import.meta.url) (REPO_ROOT).
      Same silent divergence risk if anything reads the wrong location. The test file even comments this as an outstanding migration. Worth a one-shot follow-up PR that migrates both write + read sides to WORKSPACE_DIR / 'voice-state.json' and removes the stash/restore dance from the test.

Ready to merge.

— Lucy (Mac Studio bot)

…loses #840 properly)

Two stacked issues fixed in one shot.

**Production bug:** Python writers migrated core-status.json to
$SUTANDO_WORKSPACE in #836 (per CLAUDE.md workspace contract), but TS
readers in `src/web-client.ts:readCoreStatus` and `src/inline-tools.ts:
getCoreStatusTool` still read it from REPO_ROOT via `import.meta.url`-
relative paths. Same silent-divergence shape as #821/#842/#843 fixed for
other paths — voice-agent reading "core idle" while Python actually wrote
"core working" elsewhere. Fix: both TS readers now `join(WORKSPACE_DIR,
'core-status.json')`.

**Test flake (#840):** `tests/agent-state-endpoint.test.ts` and `tests/
get-core-status-tool.test.ts` both wrote to `<REPO_ROOT>/core-status.json`
in their fixtures. node:test runs files in parallel by default; the
get-core-status-tool test writes `status:'running'` as one of its cases
right when agent-state-endpoint is asserting `state:'listening'`. Race
manifested as `expected listening, got working` on whichever subtest
happened to call effectiveAgentState() during the foreign 'running' write.

Real fix is test isolation: each test process creates its own
`mkdtempSync` workspace dir and exports `SUTANDO_WORKSPACE=$TEMP_DIR`.
The spawned web-client inherits the env; the in-process tool import reads
the same temp dir. No more shared state, no more race.

PR #841's `beforeEach` reset stayed (still useful for tool-track
sequencing within the single describe block), but #840 wasn't fully closed
by that — this PR closes it for real.

Verified:
- agent-state-endpoint.test.ts: 9/9 pass
- get-core-status-tool.test.ts: 6/6 pass
- Running both together (the failure mode): 15/15 pass
- Full TS suite: 173/173 pass

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

@qingyun-wu qingyun-wu 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.

LGTM — clean two-in-one fix. Caught a real silent-divergence bug that the workspace audit family missed, plus the proper structural fix for #840 (race) that supersedes #802/#841.

Verified:

  1. Production bug is real. Confirmed by grepping for core-status.json post-#836: Python writers (src/health-check.py:598, etc.) write to WORKSPACE_DIR / "core-status.json". TS readers in web-client.ts:readCoreStatus and inline-tools.ts:getCoreStatusTool were still resolving via import.meta.url-relative or repoDir joins. On $SUTANDO_WORKSPACE-set hosts: voice-agent reading "core idle" while Python actually wrote "core working" elsewhere — exact split-brain shape of #821/#842/#843. My round-3 TS audit missed these; thanks for catching.

  2. The test isolation is the proper fix. PR #802 (which I closed earlier as superseded by #841) only patched one subtest's pre-clear. PR #841 added a per-subtest beforeEach reset that helped within-suite ordering but didn't address the cross-file race between agent-state-endpoint and get-core-status-tool running in parallel via node:test's default file-parallel mode. The mkdtempSync per-process workspace approach eliminates the race entirely.

  3. process.env.SUTANDO_WORKSPACE = TEMP_WORKSPACE BEFORE the import in get-core-status-tool.test.ts is the right pattern. src/inline-tools.ts resolves WORKSPACE_DIR once at module init (post-#842) — set the env, then dynamic-import. Module load order matters.

  4. Spawned web-client correctly receives SUTANDO_WORKSPACE=TEMP_WORKSPACE in agent-state-endpoint's spawn env. The web-client subprocess will then call resolveWorkspace() against the per-test temp dir. ✓

Followup worth flagging (non-blocking):

voice-state.json is the next workspace-contract leak — explicitly retained at REPO_ROOT in this PR. The test file already calls it out:

"This file still lives at REPO_ROOT (readVoiceState not yet migrated — separate PR); stash/restore retained until that's fixed."

Worth either landing as a small followup or filing as an issue so it doesn't drift. Same fix shape as the core-status.json migration here.

Minor (non-blocking): The new comment in inline-tools.ts says "same workspace-contract fix as #821/#842/#843" — could specifically credit #842 since it's the PR that introduced the WORKSPACE_DIR const this PR now reuses. Nit.

CI green per the PR view. Ship.

Bonus context: this also retroactively closes the loose end from my closed PR #802 — the proper structural fix turned out to be cross-process isolation, not per-subtest pre-clears. Worth the cleanup.

@sonichi
sonichi merged commit 67ea369 into main May 18, 2026
2 checks passed
@sonichi
sonichi deleted the fix/core-status-path-and-test-isolation branch May 18, 2026 20:30
qingyun-wu added a commit that referenced this pull request May 18, 2026
Followup to #849 — same workspace-contract fix for `voice-state.json`
that #849 just landed for `core-status.json`. Issue #853 filed
explicitly to track this.

**Sites migrated:**

- `src/voice-agent.ts:853` (writer) — was `writeFileSync('voice-state.json', ...)`
  (cwd-relative; effectively REPO_ROOT when launched from there).
  Now `writeFileSync(join(WORKSPACE_DIR, 'voice-state.json'), ...)`.
  `WORKSPACE_DIR` is already in scope post-#843.
- `src/web-client.ts:3137` (reader) — was
  `new URL('../voice-state.json', import.meta.url)` (REPO_ROOT-relative).
  Now `join(WORKSPACE_DIR, 'voice-state.json')`. `WORKSPACE_DIR` is
  already in scope post-#821.

**Test isolation update (mirrors #849's pattern):**

- `tests/agent-state-endpoint.test.ts` — `VOICE_STATE_PATH` was joined
  against REPO_ROOT (`dirname(fileURLToPath(import.meta.url))`). Now
  joined against the per-process `TEMP_WORKSPACE` introduced in #849.
  The 3 subtests at lines 192/211/230 that write/unlink the file pick
  up the new path automatically.
- Dropped the `savedVoiceState` stash/restore — TEMP_WORKSPACE is fresh
  per test run + `rmSync`'d on teardown, so no prod-file leak is
  possible.
- Removed now-unused imports: `readFileSync`, `existsSync`, `dirname`,
  `fileURLToPath`. Net: 32+/-23 in the test file.

**Why this bug bites:**

Pre-fix, voice-agent's writer used `writeFileSync('voice-state.json', ...)`
— a cwd-relative path. On hosts where the bridge / voice-agent was
launched from the repo, cwd ≈ REPO_ROOT and both sides aligned. But:

- On `$SUTANDO_WORKSPACE`-set hosts, the writer kept writing to repo
  while the reader (post-#849, post-#842) read from $SUTANDO_WORKSPACE.
- On any host where the launcher's cwd drifted from REPO_ROOT (IDE
  spawn, `npx tsx` from elsewhere), the writer landed in an
  unexpected location.

`dm-result.py`'s voiceConnected query goes through web-client's
`/sse-status` → `readVoiceState()` → now uniformly resolves through
WORKSPACE_DIR. Same path for the writer. Split-brain closed.

**Verified:**

- `npx tsc --noEmit` ✓ clean.
- Diff is targeted: 3 files, 29 insertions / 23 deletions.

**Audit family — TS side**: this is the last known core-state file
left at REPO_ROOT. After this lands, all workspace runtime data
flows through `$SUTANDO_WORKSPACE` consistently across Python +
TS + Swift writers and readers.
sonichi added a commit that referenced this pull request May 18, 2026
…) (#855)

Followup to #849 — same workspace-contract fix for `voice-state.json`
that #849 just landed for `core-status.json`. Issue #853 filed
explicitly to track this.

**Sites migrated:**

- `src/voice-agent.ts:853` (writer) — was `writeFileSync('voice-state.json', ...)`
  (cwd-relative; effectively REPO_ROOT when launched from there).
  Now `writeFileSync(join(WORKSPACE_DIR, 'voice-state.json'), ...)`.
  `WORKSPACE_DIR` is already in scope post-#843.
- `src/web-client.ts:3137` (reader) — was
  `new URL('../voice-state.json', import.meta.url)` (REPO_ROOT-relative).
  Now `join(WORKSPACE_DIR, 'voice-state.json')`. `WORKSPACE_DIR` is
  already in scope post-#821.

**Test isolation update (mirrors #849's pattern):**

- `tests/agent-state-endpoint.test.ts` — `VOICE_STATE_PATH` was joined
  against REPO_ROOT (`dirname(fileURLToPath(import.meta.url))`). Now
  joined against the per-process `TEMP_WORKSPACE` introduced in #849.
  The 3 subtests at lines 192/211/230 that write/unlink the file pick
  up the new path automatically.
- Dropped the `savedVoiceState` stash/restore — TEMP_WORKSPACE is fresh
  per test run + `rmSync`'d on teardown, so no prod-file leak is
  possible.
- Removed now-unused imports: `readFileSync`, `existsSync`, `dirname`,
  `fileURLToPath`. Net: 32+/-23 in the test file.

**Why this bug bites:**

Pre-fix, voice-agent's writer used `writeFileSync('voice-state.json', ...)`
— a cwd-relative path. On hosts where the bridge / voice-agent was
launched from the repo, cwd ≈ REPO_ROOT and both sides aligned. But:

- On `$SUTANDO_WORKSPACE`-set hosts, the writer kept writing to repo
  while the reader (post-#849, post-#842) read from $SUTANDO_WORKSPACE.
- On any host where the launcher's cwd drifted from REPO_ROOT (IDE
  spawn, `npx tsx` from elsewhere), the writer landed in an
  unexpected location.

`dm-result.py`'s voiceConnected query goes through web-client's
`/sse-status` → `readVoiceState()` → now uniformly resolves through
WORKSPACE_DIR. Same path for the writer. Split-brain closed.

**Verified:**

- `npx tsc --noEmit` ✓ clean.
- Diff is targeted: 3 files, 29 insertions / 23 deletions.

**Audit family — TS side**: this is the last known core-state file
left at REPO_ROOT. After this lands, all workspace runtime data
flows through `$SUTANDO_WORKSPACE` consistently across Python +
TS + Swift writers and readers.

Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>
qingyun-wu added a commit that referenced this pull request May 18, 2026
Per Qingyun's 2026-05-18T20:56Z ask: "draft a design doc to share and
align with other developer." Triggered by the workspace audit churn
(~28 sites in 13 files, 16 PRs landed today) that closed the runtime-
data divergence bug class but exposed the lack of a shared vocabulary.

The doc names three concentric spaces:

- Code (`$SUTANDO_REPO_DIR`) — public git, persistent
- State (`$SUTANDO_WORKSPACE`) — per-machine, ephemeral
- Memory (`$SUTANDO_PRIVATE_DIR`) — fleet-wide private git, persistent

Memory has a `machine-<host>/` subdivision for per-Mac files
(stand-identity, pending-questions) and a top-level for shared content
(notes/, build_log.md, memory/). This matches the existing
`personalPath` / `sharedPersonalPath` helpers in `src/util_paths.{py,ts}`
— the design just names what already works.

Includes:
- Resolvers per language (Python, TS, Swift) for each space.
- "What goes here" + "what doesn't" examples for each.
- Multi-machine model + sync rules.
- Test isolation pattern from #849 codified as discipline.
- Migration cost analysis (low — mostly docs).
- 6 open decisions for team RFC discussion (naming, env-var rename,
  host-id strategy, State sync exceptions, secrets, Sutando.app
  bundle reconciliation).

Status: RFC. Companion tracking issue for the open decisions. Complements
`docs/workspace-contract.md` (implementation reference) with the why
+ taxonomy.

Drafted at `notes/workspace-design-draft-2026-05-18.md` (workspace,
not in repo). This is the repo-side promotion after discussion.
sonichi pushed a commit that referenced this pull request May 19, 2026
* test: extract setupTempWorkspace() helper (closes #862)

Mini's #849 + #855 per-process workspace pattern was inline in two
test files. Extracting to a shared helper so future workspace-touching
tests adopt it by default rather than reinventing.

## The helper

`tests/_helpers/temp-workspace.ts` exports:

```ts
export function setupTempWorkspace(name: string): {
    workspace: string;
    cleanup: () => void;
}
```

Module-level call. Sets `process.env.SUTANDO_WORKSPACE` to a fresh
`mkdtempSync(... 'sutando-test-<name>-')` so:

1. The code under test's `resolveWorkspace()` at module init resolves
   to the temp dir.
2. Any spawned subprocess can be passed `SUTANDO_WORKSPACE: workspace`
   in its env so its own `resolveWorkspace()` resolves identically.

`cleanup()` is idempotent — safe to call in `after()` without
guarding.

## Refactor

- `tests/agent-state-endpoint.test.ts` — replaces inline
  `mkdtempSync` + `rmSync` + `process.env.SUTANDO_WORKSPACE = ...`
  with `setupTempWorkspace('agent-state')`. Spawn env passes
  `SUTANDO_WORKSPACE: TEMP_WORKSPACE` unchanged.
- `tests/get-core-status-tool.test.ts` — same refactor with name
  `'getcore'`.

Net: 22+/22- (per-test files shrink, helper file adds the
abstraction).

## Verified

- `npx tsc --noEmit` ✓ clean.
- `npx tsx --test tests/get-core-status-tool.test.ts` — all 6 cases
  pass. (Note: `node --test` directly doesn't work for `.ts` test
  files; tests run via the existing `npm test` script which uses
  `tsx --test`.)

## Closes

#862

## What I'm NOT doing

- Doc note in `docs/workspace-design.md` test-isolation section —
  deferred to the same PR that ratifies the RFC (#858), so the doc's
  test-isolation section can reference the helper that's now in
  main. Cleaner than landing the doc reference before the helper
  itself is in main.

* fix(tests): allowlist tests/_helpers/ in .gitignore, add temp-workspace helper

Issue #862 follow-through. Previous commit cf26b53 was silently incomplete:
.gitignore line 42 (`tests/*`) shadowed the new helper file, so the commit
contained only the test refactors and the branch was broken at HEAD.

- Add `!tests/_helpers/` + `!tests/_helpers/*.ts` exceptions (matching the
  existing `!tests/*.test.ts` pattern for canonical test sources).
- Add the helper itself (`setupTempWorkspace(name)`) that two tests already
  imported from in cf26b53.

Closes #862.
qingyun-wu added a commit that referenced this pull request May 19, 2026
Per Qingyun's 2026-05-18T20:56Z ask: "draft a design doc to share and
align with other developer." Triggered by the workspace audit churn
(~28 sites in 13 files, 16 PRs landed today) that closed the runtime-
data divergence bug class but exposed the lack of a shared vocabulary.

The doc names three concentric spaces:

- Code (`$SUTANDO_REPO_DIR`) — public git, persistent
- State (`$SUTANDO_WORKSPACE`) — per-machine, ephemeral
- Memory (`$SUTANDO_PRIVATE_DIR`) — fleet-wide private git, persistent

Memory has a `machine-<host>/` subdivision for per-Mac files
(stand-identity, pending-questions) and a top-level for shared content
(notes/, build_log.md, memory/). This matches the existing
`personalPath` / `sharedPersonalPath` helpers in `src/util_paths.{py,ts}`
— the design just names what already works.

Includes:
- Resolvers per language (Python, TS, Swift) for each space.
- "What goes here" + "what doesn't" examples for each.
- Multi-machine model + sync rules.
- Test isolation pattern from #849 codified as discipline.
- Migration cost analysis (low — mostly docs).
- 6 open decisions for team RFC discussion (naming, env-var rename,
  host-id strategy, State sync exceptions, secrets, Sutando.app
  bundle reconciliation).

Status: RFC. Companion tracking issue for the open decisions. Complements
`docs/workspace-contract.md` (implementation reference) with the why
+ taxonomy.

Drafted at `notes/workspace-design-draft-2026-05-18.md` (workspace,
not in repo). This is the repo-side promotion after discussion.
qingyun-wu added a commit that referenced this pull request May 19, 2026
…#858)

* docs: workspace-design RFC — 3-space model (Code/State/Memory)

Per Qingyun's 2026-05-18T20:56Z ask: "draft a design doc to share and
align with other developer." Triggered by the workspace audit churn
(~28 sites in 13 files, 16 PRs landed today) that closed the runtime-
data divergence bug class but exposed the lack of a shared vocabulary.

The doc names three concentric spaces:

- Code (`$SUTANDO_REPO_DIR`) — public git, persistent
- State (`$SUTANDO_WORKSPACE`) — per-machine, ephemeral
- Memory (`$SUTANDO_PRIVATE_DIR`) — fleet-wide private git, persistent

Memory has a `machine-<host>/` subdivision for per-Mac files
(stand-identity, pending-questions) and a top-level for shared content
(notes/, build_log.md, memory/). This matches the existing
`personalPath` / `sharedPersonalPath` helpers in `src/util_paths.{py,ts}`
— the design just names what already works.

Includes:
- Resolvers per language (Python, TS, Swift) for each space.
- "What goes here" + "what doesn't" examples for each.
- Multi-machine model + sync rules.
- Test isolation pattern from #849 codified as discipline.
- Migration cost analysis (low — mostly docs).
- 6 open decisions for team RFC discussion (naming, env-var rename,
  host-id strategy, State sync exceptions, secrets, Sutando.app
  bundle reconciliation).

Status: RFC. Companion tracking issue for the open decisions. Complements
`docs/workspace-contract.md` (implementation reference) with the why
+ taxonomy.

Drafted at `notes/workspace-design-draft-2026-05-18.md` (workspace,
not in repo). This is the repo-side promotion after discussion.

* docs(workspace-design): address sonichi's PR review nits (#1 + #4)

Two small clarifications from sonichi's #858 review:

1. **"Memory falls back to State" is a feature, not a gap.** Previously
   framed as impl gap #3 in the open-decisions section, which reads
   like a TODO. Reframe inline at the Memory section: the fallback is
   intentional onboarding-friction reduction; fresh installs work
   without `SUTANDO_PRIVATE_DIR`. The lifecycle distinction is
   documented but not enforced. Stops a future contributor from
   "fixing" this by removing the fallback.

2. **IDE / editor config angle.** Added a short section before
   "Relationship to other docs" covering `.cursorrules`, `.claude/`,
   `.vscode/`. The 3-space model doesn't have a clean home for these
   — answer is the existing IDE-aware convention (`.gitignore`
   per-user variants, check in shared ones). Flagging because new
   contributors ask "should .cursorrules go in Memory?" and the
   answer (no, stays in Code-or-untracked per IDE convention) isn't
   obvious from the 3-space framing alone.

Two other nits from the review are followup PRs, not addressed here:

- (#2) Extract `mkdtempSync` test-isolation pattern to
  `tests/_helpers/temp-workspace.ts`. Worth a small follow-up — Mini
  offered to take it.
- (#3) File the 7-item implementation-gaps list as separate issues
  so they don't bit-rot in the doc. Will do after owner sign-off on
  the alignment direction in #857.

Mini's votes on the 6 alignment decisions in #857 are mostly aligned
with the proposal; the deltas (per-read deprecation logging,
`SUTANDO_HOST_LABEL` override, opt-in state-sync allowlist, secrets
stay at .env) are good refinements worth incorporating into the doc
once owner confirms direction.

* docs(workspace-design): surface host-CLI dependency abstractly (no specific paths)

Per Qingyun's 2026-05-18T21:25Z follow-up: even if we keep the
specifics internal, we should acknowledge in the public design that
Sutando depends on its host CLI's user-home for some agent-state
surfaces — so a future swap to Codex / gemini-cli / etc. has a known
migration cost.

Adds a "Host CLI dependency surface" section before "Relationship to
other docs":

- Lists abstract dependency categories (agent memory, bridge auth,
  skill discovery, slash-command config). NO specific paths.
- Calls out high-cost vs low-cost re-bind classes if we ever switch
  host CLIs.
- Notes the "interop loss" angle — slash commands provided by the
  host CLI evaporate on swap.
- Today's policy: surface lives in workspace-private engineering note,
  publication of paths defers until host CLI conventions stabilize.
- Points contributors at the team channel to request the inventory
  when needed.

Engineering inventory (specific paths, read/write inventory, per-path
migration cost analysis) lives at
`<workspace>/notes/host-cli-dependency-surface-2026-05-18.md` — not
in this repo, per owner directive.

* docs(workspace-design): comprehensive + self-contained (folded decisions inline)

Per Qingyun's 2026-05-18T21:30Z direction: "make it as comprehensive
and self-contained as possible so we can discuss across team members."

Three structural improvements:

1. **Added "Why this doc exists" section up front** — names the audit
   churn that motivated the doc (16 PRs, ~28 sites) so a first-time
   reader has context without bouncing to PR history.

2. **Added "Quick decision: which space?" flowchart-ish section** —
   3-step decision-flow for "where does this new path go." Catches the
   most common new-contributor question.

3. **Folded the 6 open decisions inline** with current proposal +
   alternatives + rationale + input-so-far for each. Previously linked
   out to #857; now self-contained. Each decision is its own
   discussable unit. Decision #4 (State sync) explicitly notes the
   shared-task-queue sub-thread and that the `state/fleet/` refinement
   is still open.

4. **Added "doc cadence" open question at the bottom** — async vs
   synchronous review of the RFC. Owner's call.

The doc now stands on its own — a team member can read it, see all 6
decisions, see what's been said about each, and react without needing
to crawl Discord history or PR comments. The host-CLI section stays
abstract per owner directive (specific paths remain internal-only).

Length: 169 → 235 lines (~40% growth, mostly in the decisions section).

* docs(claude-md): point Workspace contract section at docs/workspace-design.md

Per Qingyun's 2026-05-18T21:33Z ask: yes, CLAUDE.md should reference
the new 3-space design doc so per-session agents know the vocabulary
without re-deriving it from call-site spelunking.

One-line edit at the top of the Workspace contract section:

- Names the three spaces (Code / State / Memory) with their canonical
  env vars.
- Points at `docs/workspace-design.md` for the full model + the
  "Quick decision: which space?" flowchart.

Minimal — does NOT duplicate the design doc content into CLAUDE.md;
the existing Workspace contract section already covers resolution +
helpers + the SUTANDO_REPO_DIR clarification, which is the
per-session-load info. The new pointer is just so agents know where
to look when they need the design.

Length: +1 paragraph (3 lines).

* docs(workspace-design): reference the claudeHomePath helper now landed in #860

PR #860 (Sutando-Mini) landed the `claudeHomePath(subpath)` /
`claude_home_path(*subpath)` helper in `src/util_paths.{ts,py}`.
This was Mini's implementation of host-CLI discipline #1 (greppable
surface) from the workspace-design RFC sub-thread.

Adds one paragraph to the public Host CLI section pointing at the
helper: "Sutando code that needs to read or write inside the host
CLI's home directory goes through a single helper... One canonical
resolver = one grep target = a future swap is a 1-day grep+replace
rather than re-architecture."

Specific paths still stay internal per the established rule. This
edit names the HELPER (now public in main) and the directive for new
code; the inventory of paths the helper resolves remains in the
workspace-private engineering note.

Keeps the doc consistent with main and gives new contributors a
clear "what to do when you need to touch ~/.claude/" answer without
expanding the public scope.

* docs(workspace-design): mark all 6 decisions Ratified + link tracking issues

Convergence pass on PR #858 per owner's "let's iterate, want to converge"
ask. All 6 decisions had multiple endorsements and no objections — flip
them from "Input so far: Endorsed" to "Ratified" and point at follow-up
tracking issues where there's impl work to do:

- Decision 2 (env var rename) → #870
- Decision 3 (SUTANDO_HOST_LABEL) → #871
- Decision 4 (State sync .sync-allowlist + state/fleet/ design) → #872

Decisions 1, 5, 6 are policy ratifications with no impl follow-up:
- 1 (names): vocabulary lands with merge
- 5 (secrets at .env): policy, cross-Mac keys workstream deferred separately
- 6 (Sutando.app bundle): tracked under pre-existing 2026-05-13 issue

Addresses sonichi's nit #3 from PR review ("file impl-gap list as
separate issues"). With this commit, nits #1, #2, #3, #4 are all
resolved (per PR description table).
VasiliyRad added a commit to AG2Platform/stando that referenced this pull request May 22, 2026
…O_WORKSPACE

## Why this test is needed

Of the last 250 commits on this repo, roughly **9 are fixes** of the same
shape: a source module writes/reads to `tasks/`, `results/`, `state/`,
or `notes/` without going through `resolve_workspace()`, so on hosts
where `SUTANDO_WORKSPACE` is set the module writes one place while
another component reads another — split-brain that strands owner DMs,
loses voice-agent state, or pollutes `git status`.

Sample of the historical fix chain:
  - PR sonichi#843, PR sonichi#855, PR sonichi#849 — voice-agent state honor SUTANDO_WORKSPACE
  - b51659e fix(bridges,webhook,archive,event-log,notify): route runtime state through state_dir/state_path
  - ab537d0 fix(voice,tmux-status): mkdir data/ before metrics write
  - 38ae961 fix(health-check): resolve .env via state_path
  - 1f29861 mini: heartbeat + core-status paths honor SUTANDO_HOME
  - 8e93c14 merge(CP-0): fix test suite + 2 missed runtime-state paths
  - PR #27 (this round): anchor relative SUTANDO_WORKSPACE absolute

Each was a one-off "found another one, patched another one" fix. The
underlying class — a new source file written without the workspace
contract in mind — keeps producing instances.

## Why this is the right fix

A preventative test that fails when a new source file references
runtime-state directories without using the canonical resolver. The
fix surface for any new violation becomes: "add a one-line import,
or add the file to the test's ALLOWLIST with a justification."
Eliminates the bug class at the boundary instead of chasing instances.

## Why this is a good-quality change

- **Maintainability:** future contributors get clean test-failure
  feedback at PR time instead of a 3am incident in prod after an
  install with SUTANDO_WORKSPACE set.
- **Testability:** the test is read-only static analysis (no source
  changes, no fixtures, no networking). Runs in <100ms.
- **Robustness:** closes the loop on the workspace contract — the
  resolver was canonical but un-enforced.
- **Low risk:** zero source changes in this PR. ALL behavior
  preserved. Just adds the enforcement test.

## Current state of the codebase per this test

**Test passes against current main**, with 7 known violators
explicitly allowlisted (with `MIGRATION-PENDING` comments):

  - src/call-stats.py — CALLS_FILE = repo_root/results/calls/calls.jsonl
  - src/check-pending-questions.py — reads from repo_root
  - src/daily-insight.py — CALLS_FILE = repo_root/results/calls/calls.jsonl
  - src/detect-learned-skills.py — TASKS_ARCHIVE = repo_root/tasks/archive
  - src/friction-detector.py — RESULTS_DIR = repo_root/results
  - src/scan-call-logs.py — CALLS_FILE + STATE_FILE in repo_root

Each is a latent SUTANDO_WORKSPACE bug on env-set hosts (writes to
the wrong location). Listed in ALLOWLIST so the migration is
visible and a NEW file using the anti-pattern fails the test, while
the known set remains tracked for follow-up.

PLUS one intentional canonical-bypass:
  - src/core_heartbeat.py — intentionally dep-free per its own
    comment (runs before other Sutando modules are loaded); inlines
    the workspace resolution rather than importing.

## What the test catches

For every `src/*.py` and `src/*.ts` file: if it references any of
`tasks/`, `results/`, `state/`, `notes/`, `data/`, `logs/` as a
path component AND doesn't import the canonical resolver
(`workspace_default.resolve_workspace` / `workspace_default.resolveWorkspace`)
or the convenience wrapper (`state_paths.state_dir/state_path` /
`state-paths.stateDir/statePath`) AND isn't in the ALLOWLIST → the
test fails with a precise file-and-line pointer.

Two additional sanity tests:
  - `test_canonical_modules_themselves_are_present`: catches a
    refactor that accidentally deletes the canonical module.
  - `test_allowlist_entries_actually_exist`: forces ALLOWLIST to
    stay honest — a stale entry (file renamed/deleted) fails.

## Test plan

- [x] `python3 tests/state-paths-adoption.test.py` — 3/3 pass
- [x] All other `tests/*.test.py` — 23/23 still pass (no source change)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
…loses sonichi#840 properly) (sonichi#849)

Two stacked issues fixed in one shot.

**Production bug:** Python writers migrated core-status.json to
$SUTANDO_WORKSPACE in sonichi#836 (per CLAUDE.md workspace contract), but TS
readers in `src/web-client.ts:readCoreStatus` and `src/inline-tools.ts:
getCoreStatusTool` still read it from REPO_ROOT via `import.meta.url`-
relative paths. Same silent-divergence shape as sonichi#821/sonichi#842/sonichi#843 fixed for
other paths — voice-agent reading "core idle" while Python actually wrote
"core working" elsewhere. Fix: both TS readers now `join(WORKSPACE_DIR,
'core-status.json')`.

**Test flake (sonichi#840):** `tests/agent-state-endpoint.test.ts` and `tests/
get-core-status-tool.test.ts` both wrote to `<REPO_ROOT>/core-status.json`
in their fixtures. node:test runs files in parallel by default; the
get-core-status-tool test writes `status:'running'` as one of its cases
right when agent-state-endpoint is asserting `state:'listening'`. Race
manifested as `expected listening, got working` on whichever subtest
happened to call effectiveAgentState() during the foreign 'running' write.

Real fix is test isolation: each test process creates its own
`mkdtempSync` workspace dir and exports `SUTANDO_WORKSPACE=$TEMP_DIR`.
The spawned web-client inherits the env; the in-process tool import reads
the same temp dir. No more shared state, no more race.

PR sonichi#841's `beforeEach` reset stayed (still useful for tool-track
sequencing within the single describe block), but sonichi#840 wasn't fully closed
by that — this PR closes it for real.

Verified:
- agent-state-endpoint.test.ts: 9/9 pass
- get-core-status-tool.test.ts: 6/6 pass
- Running both together (the failure mode): 15/15 pass
- Full TS suite: 173/173 pass

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
…nichi#853) (sonichi#855)

Followup to sonichi#849 — same workspace-contract fix for `voice-state.json`
that sonichi#849 just landed for `core-status.json`. Issue sonichi#853 filed
explicitly to track this.

**Sites migrated:**

- `src/voice-agent.ts:853` (writer) — was `writeFileSync('voice-state.json', ...)`
  (cwd-relative; effectively REPO_ROOT when launched from there).
  Now `writeFileSync(join(WORKSPACE_DIR, 'voice-state.json'), ...)`.
  `WORKSPACE_DIR` is already in scope post-sonichi#843.
- `src/web-client.ts:3137` (reader) — was
  `new URL('../voice-state.json', import.meta.url)` (REPO_ROOT-relative).
  Now `join(WORKSPACE_DIR, 'voice-state.json')`. `WORKSPACE_DIR` is
  already in scope post-sonichi#821.

**Test isolation update (mirrors sonichi#849's pattern):**

- `tests/agent-state-endpoint.test.ts` — `VOICE_STATE_PATH` was joined
  against REPO_ROOT (`dirname(fileURLToPath(import.meta.url))`). Now
  joined against the per-process `TEMP_WORKSPACE` introduced in sonichi#849.
  The 3 subtests at lines 192/211/230 that write/unlink the file pick
  up the new path automatically.
- Dropped the `savedVoiceState` stash/restore — TEMP_WORKSPACE is fresh
  per test run + `rmSync`'d on teardown, so no prod-file leak is
  possible.
- Removed now-unused imports: `readFileSync`, `existsSync`, `dirname`,
  `fileURLToPath`. Net: 32+/-23 in the test file.

**Why this bug bites:**

Pre-fix, voice-agent's writer used `writeFileSync('voice-state.json', ...)`
— a cwd-relative path. On hosts where the bridge / voice-agent was
launched from the repo, cwd ≈ REPO_ROOT and both sides aligned. But:

- On `$SUTANDO_WORKSPACE`-set hosts, the writer kept writing to repo
  while the reader (post-sonichi#849, post-sonichi#842) read from $SUTANDO_WORKSPACE.
- On any host where the launcher's cwd drifted from REPO_ROOT (IDE
  spawn, `npx tsx` from elsewhere), the writer landed in an
  unexpected location.

`dm-result.py`'s voiceConnected query goes through web-client's
`/sse-status` → `readVoiceState()` → now uniformly resolves through
WORKSPACE_DIR. Same path for the writer. Split-brain closed.

**Verified:**

- `npx tsc --noEmit` ✓ clean.
- Diff is targeted: 3 files, 29 insertions / 23 deletions.

**Audit family — TS side**: this is the last known core-state file
left at REPO_ROOT. After this lands, all workspace runtime data
flows through `$SUTANDO_WORKSPACE` consistently across Python +
TS + Swift writers and readers.

Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
… (sonichi#865)

* test: extract setupTempWorkspace() helper (closes sonichi#862)

Mini's sonichi#849 + sonichi#855 per-process workspace pattern was inline in two
test files. Extracting to a shared helper so future workspace-touching
tests adopt it by default rather than reinventing.

## The helper

`tests/_helpers/temp-workspace.ts` exports:

```ts
export function setupTempWorkspace(name: string): {
    workspace: string;
    cleanup: () => void;
}
```

Module-level call. Sets `process.env.SUTANDO_WORKSPACE` to a fresh
`mkdtempSync(... 'sutando-test-<name>-')` so:

1. The code under test's `resolveWorkspace()` at module init resolves
   to the temp dir.
2. Any spawned subprocess can be passed `SUTANDO_WORKSPACE: workspace`
   in its env so its own `resolveWorkspace()` resolves identically.

`cleanup()` is idempotent — safe to call in `after()` without
guarding.

## Refactor

- `tests/agent-state-endpoint.test.ts` — replaces inline
  `mkdtempSync` + `rmSync` + `process.env.SUTANDO_WORKSPACE = ...`
  with `setupTempWorkspace('agent-state')`. Spawn env passes
  `SUTANDO_WORKSPACE: TEMP_WORKSPACE` unchanged.
- `tests/get-core-status-tool.test.ts` — same refactor with name
  `'getcore'`.

Net: 22+/22- (per-test files shrink, helper file adds the
abstraction).

## Verified

- `npx tsc --noEmit` ✓ clean.
- `npx tsx --test tests/get-core-status-tool.test.ts` — all 6 cases
  pass. (Note: `node --test` directly doesn't work for `.ts` test
  files; tests run via the existing `npm test` script which uses
  `tsx --test`.)

## Closes

sonichi#862

## What I'm NOT doing

- Doc note in `docs/workspace-design.md` test-isolation section —
  deferred to the same PR that ratifies the RFC (sonichi#858), so the doc's
  test-isolation section can reference the helper that's now in
  main. Cleaner than landing the doc reference before the helper
  itself is in main.

* fix(tests): allowlist tests/_helpers/ in .gitignore, add temp-workspace helper

Issue sonichi#862 follow-through. Previous commit cf26b53 was silently incomplete:
.gitignore line 42 (`tests/*`) shadowed the new helper file, so the commit
contained only the test refactors and the branch was broken at HEAD.

- Add `!tests/_helpers/` + `!tests/_helpers/*.ts` exceptions (matching the
  existing `!tests/*.test.ts` pattern for canonical test sources).
- Add the helper itself (`setupTempWorkspace(name)`) that two tests already
  imported from in cf26b53.

Closes sonichi#862.
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
…sonichi#858)

* docs: workspace-design RFC — 3-space model (Code/State/Memory)

Per Qingyun's 2026-05-18T20:56Z ask: "draft a design doc to share and
align with other developer." Triggered by the workspace audit churn
(~28 sites in 13 files, 16 PRs landed today) that closed the runtime-
data divergence bug class but exposed the lack of a shared vocabulary.

The doc names three concentric spaces:

- Code (`$SUTANDO_REPO_DIR`) — public git, persistent
- State (`$SUTANDO_WORKSPACE`) — per-machine, ephemeral
- Memory (`$SUTANDO_PRIVATE_DIR`) — fleet-wide private git, persistent

Memory has a `machine-<host>/` subdivision for per-Mac files
(stand-identity, pending-questions) and a top-level for shared content
(notes/, build_log.md, memory/). This matches the existing
`personalPath` / `sharedPersonalPath` helpers in `src/util_paths.{py,ts}`
— the design just names what already works.

Includes:
- Resolvers per language (Python, TS, Swift) for each space.
- "What goes here" + "what doesn't" examples for each.
- Multi-machine model + sync rules.
- Test isolation pattern from sonichi#849 codified as discipline.
- Migration cost analysis (low — mostly docs).
- 6 open decisions for team RFC discussion (naming, env-var rename,
  host-id strategy, State sync exceptions, secrets, Sutando.app
  bundle reconciliation).

Status: RFC. Companion tracking issue for the open decisions. Complements
`docs/workspace-contract.md` (implementation reference) with the why
+ taxonomy.

Drafted at `notes/workspace-design-draft-2026-05-18.md` (workspace,
not in repo). This is the repo-side promotion after discussion.

* docs(workspace-design): address sonichi's PR review nits (sonichi#1 + sonichi#4)

Two small clarifications from sonichi's sonichi#858 review:

1. **"Memory falls back to State" is a feature, not a gap.** Previously
   framed as impl gap sonichi#3 in the open-decisions section, which reads
   like a TODO. Reframe inline at the Memory section: the fallback is
   intentional onboarding-friction reduction; fresh installs work
   without `SUTANDO_PRIVATE_DIR`. The lifecycle distinction is
   documented but not enforced. Stops a future contributor from
   "fixing" this by removing the fallback.

2. **IDE / editor config angle.** Added a short section before
   "Relationship to other docs" covering `.cursorrules`, `.claude/`,
   `.vscode/`. The 3-space model doesn't have a clean home for these
   — answer is the existing IDE-aware convention (`.gitignore`
   per-user variants, check in shared ones). Flagging because new
   contributors ask "should .cursorrules go in Memory?" and the
   answer (no, stays in Code-or-untracked per IDE convention) isn't
   obvious from the 3-space framing alone.

Two other nits from the review are followup PRs, not addressed here:

- (sonichi#2) Extract `mkdtempSync` test-isolation pattern to
  `tests/_helpers/temp-workspace.ts`. Worth a small follow-up — Mini
  offered to take it.
- (sonichi#3) File the 7-item implementation-gaps list as separate issues
  so they don't bit-rot in the doc. Will do after owner sign-off on
  the alignment direction in sonichi#857.

Mini's votes on the 6 alignment decisions in sonichi#857 are mostly aligned
with the proposal; the deltas (per-read deprecation logging,
`SUTANDO_HOST_LABEL` override, opt-in state-sync allowlist, secrets
stay at .env) are good refinements worth incorporating into the doc
once owner confirms direction.

* docs(workspace-design): surface host-CLI dependency abstractly (no specific paths)

Per Qingyun's 2026-05-18T21:25Z follow-up: even if we keep the
specifics internal, we should acknowledge in the public design that
Sutando depends on its host CLI's user-home for some agent-state
surfaces — so a future swap to Codex / gemini-cli / etc. has a known
migration cost.

Adds a "Host CLI dependency surface" section before "Relationship to
other docs":

- Lists abstract dependency categories (agent memory, bridge auth,
  skill discovery, slash-command config). NO specific paths.
- Calls out high-cost vs low-cost re-bind classes if we ever switch
  host CLIs.
- Notes the "interop loss" angle — slash commands provided by the
  host CLI evaporate on swap.
- Today's policy: surface lives in workspace-private engineering note,
  publication of paths defers until host CLI conventions stabilize.
- Points contributors at the team channel to request the inventory
  when needed.

Engineering inventory (specific paths, read/write inventory, per-path
migration cost analysis) lives at
`<workspace>/notes/host-cli-dependency-surface-2026-05-18.md` — not
in this repo, per owner directive.

* docs(workspace-design): comprehensive + self-contained (folded decisions inline)

Per Qingyun's 2026-05-18T21:30Z direction: "make it as comprehensive
and self-contained as possible so we can discuss across team members."

Three structural improvements:

1. **Added "Why this doc exists" section up front** — names the audit
   churn that motivated the doc (16 PRs, ~28 sites) so a first-time
   reader has context without bouncing to PR history.

2. **Added "Quick decision: which space?" flowchart-ish section** —
   3-step decision-flow for "where does this new path go." Catches the
   most common new-contributor question.

3. **Folded the 6 open decisions inline** with current proposal +
   alternatives + rationale + input-so-far for each. Previously linked
   out to sonichi#857; now self-contained. Each decision is its own
   discussable unit. Decision sonichi#4 (State sync) explicitly notes the
   shared-task-queue sub-thread and that the `state/fleet/` refinement
   is still open.

4. **Added "doc cadence" open question at the bottom** — async vs
   synchronous review of the RFC. Owner's call.

The doc now stands on its own — a team member can read it, see all 6
decisions, see what's been said about each, and react without needing
to crawl Discord history or PR comments. The host-CLI section stays
abstract per owner directive (specific paths remain internal-only).

Length: 169 → 235 lines (~40% growth, mostly in the decisions section).

* docs(claude-md): point Workspace contract section at docs/workspace-design.md

Per Qingyun's 2026-05-18T21:33Z ask: yes, CLAUDE.md should reference
the new 3-space design doc so per-session agents know the vocabulary
without re-deriving it from call-site spelunking.

One-line edit at the top of the Workspace contract section:

- Names the three spaces (Code / State / Memory) with their canonical
  env vars.
- Points at `docs/workspace-design.md` for the full model + the
  "Quick decision: which space?" flowchart.

Minimal — does NOT duplicate the design doc content into CLAUDE.md;
the existing Workspace contract section already covers resolution +
helpers + the SUTANDO_REPO_DIR clarification, which is the
per-session-load info. The new pointer is just so agents know where
to look when they need the design.

Length: +1 paragraph (3 lines).

* docs(workspace-design): reference the claudeHomePath helper now landed in sonichi#860

PR sonichi#860 (Sutando-Mini) landed the `claudeHomePath(subpath)` /
`claude_home_path(*subpath)` helper in `src/util_paths.{ts,py}`.
This was Mini's implementation of host-CLI discipline sonichi#1 (greppable
surface) from the workspace-design RFC sub-thread.

Adds one paragraph to the public Host CLI section pointing at the
helper: "Sutando code that needs to read or write inside the host
CLI's home directory goes through a single helper... One canonical
resolver = one grep target = a future swap is a 1-day grep+replace
rather than re-architecture."

Specific paths still stay internal per the established rule. This
edit names the HELPER (now public in main) and the directive for new
code; the inventory of paths the helper resolves remains in the
workspace-private engineering note.

Keeps the doc consistent with main and gives new contributors a
clear "what to do when you need to touch ~/.claude/" answer without
expanding the public scope.

* docs(workspace-design): mark all 6 decisions Ratified + link tracking issues

Convergence pass on PR sonichi#858 per owner's "let's iterate, want to converge"
ask. All 6 decisions had multiple endorsements and no objections — flip
them from "Input so far: Endorsed" to "Ratified" and point at follow-up
tracking issues where there's impl work to do:

- Decision 2 (env var rename) → sonichi#870
- Decision 3 (SUTANDO_HOST_LABEL) → sonichi#871
- Decision 4 (State sync .sync-allowlist + state/fleet/ design) → sonichi#872

Decisions 1, 5, 6 are policy ratifications with no impl follow-up:
- 1 (names): vocabulary lands with merge
- 5 (secrets at .env): policy, cross-Mac keys workstream deferred separately
- 6 (Sutando.app bundle): tracked under pre-existing 2026-05-13 issue

Addresses sonichi's nit sonichi#3 from PR review ("file impl-gap list as
separate issues"). With this commit, nits sonichi#1, sonichi#2, sonichi#3, sonichi#4 are all
resolved (per PR description table).
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.

Flaky test: agent-state-endpoint subtests fail intermittently on CI runners

3 participants