Repository navigation
fix(workspace): honor SUTANDO_WORKSPACE in 5 scripts (audit followup to #815) - #836
Merged
Merged
Conversation
… as PR #815/#832/#833 Audit per owner's "REPO_DIR might be more places than #815" — confirmed 10 bugs across 5 scripts all hitting the same workspace-vs-repo distinction the bridges fixed pre-PR #762. Each was resolving per-user-state paths (logs/, results/, state/, build_log.md, core-status.json, pending-questions.md) to the repo root instead of $SUTANDO_WORKSPACE. ## src/health-check.py — 6 sites - L314 voice-agent.log freshness check → WORKSPACE_DIR/logs - L598 core-status.json proactive-loop liveness → WORKSPACE_DIR - L681 build_log.md critical-file check → WORKSPACE_DIR - L795 bridge log-staleness check → WORKSPACE_DIR/logs - L807 bridge heartbeat check → WORKSPACE_DIR/state - L1171 restart-on-fix subprocess stdout redirect → WORKSPACE_DIR/logs WORKSPACE_DIR was already wired in this file (line 42 from earlier work); just substituting the right call sites. Code paths (src/*.py, src/*.ts, node_modules, .env, Sutando.app binary, subprocess cwd) correctly stay as REPO_DIR. Effect: health-check no longer false-positives "log stale / heartbeat missing / sutando-app not running" when services are actually fine. Real issues (voice-agent code stale, discord-bridge code stale) now surface cleanly instead of being lost in the false-positive cascade. ## src/event_log.py - L53–54: REPO_DIR/logs → WORKSPACE_DIR/logs (LOGS_DIR module global) - L71: personal_path(...) workspace arg → WORKSPACE_DIR ## src/check-pending-questions.py - L20: WORKSPACE = Path(__file__).parent.parent → resolve_workspace() This one was particularly silent — the cron's been firing for weeks reading the wrong file. After fix: surfaces the 2 long-pending questions (May 9 email-organize, May 13 web-UI bundle/fork) that the friction detector also found. ## src/call-stats.py + src/scan-call-logs.py - Both hardcoded `Path(__file__).parent.parent / "results" / "calls" / ...` - Now use resolve_workspace() (same pattern as daily-insight.py PR #832 + friction-detector.py PR #833) call-stats verified post-fix: now reads 2 real calls; previously read 0 on workspace-only installs. ## What's NOT in this PR - health-check.py L693 (notes-dir) — leaving to James's PR #815 to avoid conflict; same bug, same fix. ## Verification $ python3 src/health-check.py # core-proactive-loop ok, no false positives $ python3 src/check-pending-questions.py # Notified: 2 pending questions $ python3 src/call-stats.py # 2 real calls, Friday busiest $ python3 src/scan-call-logs.py # No new calls to scan (total: 2)
6 tasks
…heck PR #836 moved core-status.json read from REPO_DIR to WORKSPACE_DIR (per-user runtime state, not code). The test still mocked REPO_DIR, so case e ('running with 5-day-old ts should be warn') silently failed against the real workspace's core-status.json instead of the temp fixture. Update write_status, with_repo_override, and case_b_status_malformed to mock WORKSPACE_DIR. All 17 cases pass. CI flagged by Chi in Discord, 2026-05-18.
sonichi
reviewed
May 18, 2026
sonichi
left a comment
sonichi
left a comment
Owner
There was a problem hiding this comment.
LGTM — closes the workspace-contract gap for the 5 Python scripts that were missed in #762/#815. Audit followup is the right framing.
Verified each migration:
src/call-stats.py+src/scan-call-logs.py— calls.jsonl + .scan-state.json → WORKSPACE. ✓ Runtime data.src/check-pending-questions.py— WORKSPACE redefined; PQ/results/LAST_NOTIFY all flow through. ✓src/event_log.py— LOGS_DIR → WORKSPACE; stand-identity lookup follows. ✓src/health-check.py— voice-log, status-path, build_log, heartbeats, bridge logs, restart-log all move. ✓ Critically: thesrc/{name}.pyspawn target stays REPO_DIR (correct — code-adjacent), only the log destination flips. Subtle distinction handled right.tests/health-check-stuck-loop.test.py— test mock updated to swaphc.WORKSPACE_DIRinstead ofhc.REPO_DIR. The renamed semantics docstring is the right addition.
Cross-checked against CLAUDE.md workspace contract: "All per-user mutable state — tasks/, results/, state/, data/, logs/, notes/, build_log.md, pending-questions.md, contextual-chips.json, core-status.json — lives under a single workspace directory." This PR aligns 5 stragglers with that contract.
Bonus correctness: the _voice_log_path() retains the launchd Application-Support fallback for Sutando.app installs — workspace path is the "general" fallback. Right behavior.
Non-blocking:
- No new test for the simpler 3 scripts (call-stats, scan-call-logs, check-pending-questions). Existing health-check-stuck-loop test covers the test-mockable case. Probably fine — the scripts are simple enough that the migration is mechanical.
- BEHIND main — needs update-branch before merge (same recipe as #831/#835/#821/#837).
CI already SUCCESS on both checks. Merge after update-branch.
5 tasks
qingyun-wu
added a commit
that referenced
this pull request
May 18, 2026
…udit PR #836 found 10 bug sites across 5 scripts that hardcoded the workspace path to the repo checkout. This pass catches three more scripts the first audit missed (they use REPO/REPO_DIR variable names, and one set of bug sites lives under skills/, which the earlier grep didn't scan). Same bug class. Each script now resolves the workspace via `workspace_default.resolve_workspace()` — the canonical helper introduced in PR #762. **Sites fixed (4 across 3 scripts):** - `src/archive-stale-results.py` — `RESULTS = REPO / "results"` resolved to `<repo>/results/` which doesn't exist post-#762. `if not RESULTS.is_dir()` short-circuited the whole sweep, so the DM-flood prevention this script was built for (2026-04-15 incident, 142-file flood) was silently defeated on every startup since #762. Now points at `$SUTANDO_WORKSPACE/results/`. - `skills/regression-search/scripts/find-regression.py` — `CALLS_FILE` resolved to `<repo>/results/calls/calls.jsonl` which doesn't exist. Every `find-regression "..."` query silently returned "calls file not found" instead of scanning the real call history (which lives in the workspace). - `skills/regression-search/scripts/diagnose-call.py` — both `CALLS_FILE` and `METRICS_FILE` hit the same class. `diagnose-call <sid>` always printed "No call matches" regardless of input. **Why these escaped #836's audit:** The original grep was `WORKSPACE = Path(__file__)` and `Path(__file__).parent.parent` restricted to `src/`. These three scripts (a) use `REPO`/`REPO_DIR` variable names instead of `WORKSPACE`, and (b) one set lives under `skills/`. Widening the grep to `Path(__file__)\.(resolve()\.)?parent\.parent` across `src/`, `scripts/`, and `skills/` surfaced them. **Verified:** - `DRY_RUN=1 python3 src/archive-stale-results.py` — now sees workspace results dir (previously printed "results/ missing — nothing to do"). - `find-regression "test"` — now scans real `calls.jsonl` (2 entries) and returns "No calls match 'test'" instead of "calls file not found". - Both regression-search imports resolve to the workspace dir via the same `workspace_default.resolve_workspace()` shared with health-check.py / event_log.py / etc. post-#836. **Follow-ups (none blocking):** - The remaining sites flagged by the widened grep are legitimate: REPO_DIR in agent-api.py / dashboard.py / github-webhook.py is used for code-adjacent paths (running health-check.py via REPO_DIR/src, git cwd, .env loading) and is the correct semantics. util_paths.py + health-check.py REPO_DIR are likewise code-adjacent post-#836.
sonichi
pushed a commit
that referenced
this pull request
May 18, 2026
…udit (#838) PR #836 found 10 bug sites across 5 scripts that hardcoded the workspace path to the repo checkout. This pass catches three more scripts the first audit missed (they use REPO/REPO_DIR variable names, and one set of bug sites lives under skills/, which the earlier grep didn't scan). Same bug class. Each script now resolves the workspace via `workspace_default.resolve_workspace()` — the canonical helper introduced in PR #762. **Sites fixed (4 across 3 scripts):** - `src/archive-stale-results.py` — `RESULTS = REPO / "results"` resolved to `<repo>/results/` which doesn't exist post-#762. `if not RESULTS.is_dir()` short-circuited the whole sweep, so the DM-flood prevention this script was built for (2026-04-15 incident, 142-file flood) was silently defeated on every startup since #762. Now points at `$SUTANDO_WORKSPACE/results/`. - `skills/regression-search/scripts/find-regression.py` — `CALLS_FILE` resolved to `<repo>/results/calls/calls.jsonl` which doesn't exist. Every `find-regression "..."` query silently returned "calls file not found" instead of scanning the real call history (which lives in the workspace). - `skills/regression-search/scripts/diagnose-call.py` — both `CALLS_FILE` and `METRICS_FILE` hit the same class. `diagnose-call <sid>` always printed "No call matches" regardless of input. **Why these escaped #836's audit:** The original grep was `WORKSPACE = Path(__file__)` and `Path(__file__).parent.parent` restricted to `src/`. These three scripts (a) use `REPO`/`REPO_DIR` variable names instead of `WORKSPACE`, and (b) one set lives under `skills/`. Widening the grep to `Path(__file__)\.(resolve()\.)?parent\.parent` across `src/`, `scripts/`, and `skills/` surfaced them. **Verified:** - `DRY_RUN=1 python3 src/archive-stale-results.py` — now sees workspace results dir (previously printed "results/ missing — nothing to do"). - `find-regression "test"` — now scans real `calls.jsonl` (2 entries) and returns "No calls match 'test'" instead of "calls file not found". - Both regression-search imports resolve to the workspace dir via the same `workspace_default.resolve_workspace()` shared with health-check.py / event_log.py / etc. post-#836. **Follow-ups (none blocking):** - The remaining sites flagged by the widened grep are legitimate: REPO_DIR in agent-api.py / dashboard.py / github-webhook.py is used for code-adjacent paths (running health-check.py via REPO_DIR/src, git cwd, .env loading) and is the correct semantics. util_paths.py + health-check.py REPO_DIR are likewise code-adjacent post-#836.
This was referenced May 18, 2026
sonichi
added a commit
that referenced
this pull request
May 18, 2026
closes #840) (#841) The describe block shares one spawned web-client child for all subtests. Tool-track 'working' set in one subtest can linger into the next when CI scheduling delays the async 'clear-tool' POST that the producing test issues at its tail. Two unrelated PRs flaked on this same suite today (#836 and #832), both on workspace-fix PRs that don't touch web-client.ts — confirms the flake is the test, not the SUT. Fix: add a beforeEach that POSTs idle to both tool and browser tracks + a 20ms flush window. Per-subtest baseline removes the order-dependency that's been latent for the test's lifetime. Verified: 9/9 subtests pass; full TS suite 162/162 pass. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 18, 2026
qingyun-wu
added a commit
that referenced
this pull request
May 18, 2026
…owup) Migrates voice-agent.ts's `WORKSPACE_DIR` + `CALL_RESULTS_DIR` constants to `resolveWorkspace()` (the canonical TS helper introduced in #821). Two const definitions cover ~15 call sites — all of which use the path for per-user runtime state (tasks/, results/, state/, conversation.log, briefings, audio, dynamic-content, PIDFILE, etc.). **Why this matters:** On hosts with `SUTANDO_WORKSPACE` set (e.g. this machine, where `SUTANDO_WORKSPACE=/Users/qingyunwu/Documents/github/sutando-workspace-001`), the pre-fix behavior was: 1. `WORKSPACE_DIR = process.env.WORKSPACE_DIR || <repo-path-via-import.meta.url>` - `WORKSPACE_DIR` env var was the *legacy* name (now `SUTANDO_WORKSPACE`). - Fallback resolved to the repo checkout, so voice-agent wrote tasks, results, conversation.log, briefings, the PIDFILE, audio dirs, etc. to the **repo**, while Sutando.app (post-#837), Python bridges (post-#762), and #842 inline-tools (just landed) all wrote/read from the **workspace**. 2. `CALL_RESULTS_DIR = <repo>/results/calls` — phone-call result records landed under the repo too. Net effect: split-brain on `SUTANDO_WORKSPACE`-enabled hosts. Phone-call `latest-result.json` lived under the repo; new tasks the voice-agent generated landed there too; bridges/Sutando.app couldn't see them. **Pattern (matches #821 / #836 / #842):** ```ts import { resolveWorkspace } from './workspace_default.js'; const WORKSPACE_DIR = resolveWorkspace(); ``` Module-init resolution, single source of truth. CALL_RESULTS_DIR becomes `join(WORKSPACE_DIR, 'results', 'calls')` — same workspace, just the subpath. **Doc-comment cleanup:** - Renamed env-doc at line 19 from `WORKSPACE_DIR` (legacy) to `SUTANDO_WORKSPACE` (canonical) with a pointer to workspace_default.ts. - Removed the stale "default to sutando/ so Claude Code subprocess picks up CLAUDE.md automatically" comment at line 96. Voice-agent doesn't spawn Claude Code anymore (`grep -n claude src/voice-agent.ts` is empty) — task-bridge handles that via the file pipeline. The dual-use rationale that conflated repo + workspace into one var is obsolete. **Verified:** - `npx tsc --noEmit` ✓ clean. - Diff is targeted: 1 file, 15+/-4. - All 15 existing call sites (lines 98, 142, 334, 366, 515, 547, 551, 553, 799, 985, 1110, 1111, 1113, 1299, 1302, 1387) keep their `WORKSPACE_DIR` reference — they automatically pick up the new resolution. No site-by-site edits required. **Operational note:** This change takes effect on the next voice-agent restart. Currently-running voice-agent processes keep their old paths until restart. No mid-flight disruption.
sonichi
added a commit
that referenced
this pull request
May 18, 2026
…ols sites (issue #839) (#842) Round-3 audit followup to #836/#833/#838 — first slice of the TS audit flagged in #839. Five bug sites across two files, all hitting the same class: code paths resolving workspace data against `process.env.WORKSPACE_DIR` (wrong env var name) or `process.cwd()` instead of `resolveWorkspace()`. **Sites fixed:** - `src/cartesia-tts.ts:29` (`getWorkspace`) — TTS audio output dir (`results/audio/<category>/<label>-<ts>.wav`). Pre-fix: `WORKSPACE_DIR` env (legacy name, not set) → `process.cwd()` fallback → audio written to whatever cwd the launcher had. Now resolves to `$SUTANDO_WORKSPACE`. - `src/inline-tools.ts:471, 472` (`cancel_task` tool) — `tasksDir` and `resultsDir` for the cancel-task lookup. The tool listed tasks from `<cwd>/tasks/` and wrote cancel results to `<cwd>/results/`. On hosts with `SUTANDO_WORKSPACE` set, the bridge writes tasks to the workspace but cancel_task was looking in the repo (or whatever cwd) — so voice- initiated cancels would never find an in-flight task and would silently write a stranded CANCEL_INSTRUCTION file. - `src/inline-tools.ts:775` (`show_view` tool) — `dynamic-content.json` for the menu-bar app's view toggle. Pre-fix the inline tool wrote to `<cwd>/dynamic-content.json` while Sutando.app (post-#837) reads from `$SUTANDO_WORKSPACE/dynamic-content.json`. Split-brain: voice toggle fired but the app never saw it on hosts with `SUTANDO_WORKSPACE`. - `src/inline-tools.ts:873` (`recent_context` tool) — voice-agent state context. Pre-fix the read path looked at `<cwd>/state/voice-session-context.json` but the writer (core via CLAUDE.md "Voice session context" section) writes to `$SUTANDO_WORKSPACE/state/`. Voice-agent's deictic-pronoun recovery ("what was the post?", "what's pending?") would return `{note: "no context recorded yet"}` even when the writer had populated the file. - `src/inline-tools.ts:772` (`NOTES_DIR`) — `sharedPersonalPath('notes', process.cwd())` passes cwd as the workspace argument to the helper. When `SUTANDO_PRIVATE_DIR` isn't set, the helper falls through to `join(workspace, 'notes')` which becomes `<cwd>/notes/` instead of `$SUTANDO_WORKSPACE/notes/`. read_note / search_notes silently look in the wrong place. **Pattern (matches #821):** ```ts import { resolveWorkspace } from './workspace_default.js'; const WORKSPACE_DIR = resolveWorkspace(); ``` Module-init resolution mirrors the Python `WORKSPACE_DIR = resolve_workspace()` canonical pattern. Single source of truth per module. **Not addressed in this PR (still in #839's backlog):** - `src/voice-agent.ts:97, 103` — `WORKSPACE_DIR`/`CALL_RESULTS_DIR`. The highest-traffic sites; defer to a focused review. - `src/util_paths.ts:26, 54` — the fallback inside the helper itself. Cleaner to fix every caller first, then either delete the fallback or switch it to `resolveWorkspace()` so the helper has a sensible default. - `src/inline-tools.ts:957, 1021` — `join(process.cwd(), 'skills')`. These are CODE-adjacent paths (skills ship with the code), so they need REPO_ROOT semantics, not WORKSPACE_DIR. Latent today because the launcher always cwd's to the repo, but worth a separate PR. - `src/mentra-bridge.ts:88` — file is currently untracked (WIP). **Verified:** - `npx tsc --noEmit` ✓ clean across the whole tree. - Diff is targeted: 2 files, 22+/-8. Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>
qingyun-wu
added a commit
that referenced
this pull request
May 18, 2026
…owup) Migrates voice-agent.ts's `WORKSPACE_DIR` + `CALL_RESULTS_DIR` constants to `resolveWorkspace()` (the canonical TS helper introduced in #821). Two const definitions cover ~15 call sites — all of which use the path for per-user runtime state (tasks/, results/, state/, conversation.log, briefings, audio, dynamic-content, PIDFILE, etc.). **Why this matters:** On hosts with `SUTANDO_WORKSPACE` set (e.g. this machine, where `SUTANDO_WORKSPACE=/Users/qingyunwu/Documents/github/sutando-workspace-001`), the pre-fix behavior was: 1. `WORKSPACE_DIR = process.env.WORKSPACE_DIR || <repo-path-via-import.meta.url>` - `WORKSPACE_DIR` env var was the *legacy* name (now `SUTANDO_WORKSPACE`). - Fallback resolved to the repo checkout, so voice-agent wrote tasks, results, conversation.log, briefings, the PIDFILE, audio dirs, etc. to the **repo**, while Sutando.app (post-#837), Python bridges (post-#762), and #842 inline-tools (just landed) all wrote/read from the **workspace**. 2. `CALL_RESULTS_DIR = <repo>/results/calls` — phone-call result records landed under the repo too. Net effect: split-brain on `SUTANDO_WORKSPACE`-enabled hosts. Phone-call `latest-result.json` lived under the repo; new tasks the voice-agent generated landed there too; bridges/Sutando.app couldn't see them. **Pattern (matches #821 / #836 / #842):** ```ts import { resolveWorkspace } from './workspace_default.js'; const WORKSPACE_DIR = resolveWorkspace(); ``` Module-init resolution, single source of truth. CALL_RESULTS_DIR becomes `join(WORKSPACE_DIR, 'results', 'calls')` — same workspace, just the subpath. **Doc-comment cleanup:** - Renamed env-doc at line 19 from `WORKSPACE_DIR` (legacy) to `SUTANDO_WORKSPACE` (canonical) with a pointer to workspace_default.ts. - Removed the stale "default to sutando/ so Claude Code subprocess picks up CLAUDE.md automatically" comment at line 96. Voice-agent doesn't spawn Claude Code anymore (`grep -n claude src/voice-agent.ts` is empty) — task-bridge handles that via the file pipeline. The dual-use rationale that conflated repo + workspace into one var is obsolete. **Verified:** - `npx tsc --noEmit` ✓ clean. - Diff is targeted: 1 file, 15+/-4. - All 15 existing call sites (lines 98, 142, 334, 366, 515, 547, 551, 553, 799, 985, 1110, 1111, 1113, 1299, 1302, 1387) keep their `WORKSPACE_DIR` reference — they automatically pick up the new resolution. No site-by-site edits required. **Operational note:** This change takes effect on the next voice-agent restart. Currently-running voice-agent processes keep their old paths until restart. No mid-flight disruption.
sonichi
pushed a commit
that referenced
this pull request
May 18, 2026
…owup) (#843) Migrates voice-agent.ts's `WORKSPACE_DIR` + `CALL_RESULTS_DIR` constants to `resolveWorkspace()` (the canonical TS helper introduced in #821). Two const definitions cover ~15 call sites — all of which use the path for per-user runtime state (tasks/, results/, state/, conversation.log, briefings, audio, dynamic-content, PIDFILE, etc.). **Why this matters:** On hosts with `SUTANDO_WORKSPACE` set (e.g. this machine, where `SUTANDO_WORKSPACE=/Users/qingyunwu/Documents/github/sutando-workspace-001`), the pre-fix behavior was: 1. `WORKSPACE_DIR = process.env.WORKSPACE_DIR || <repo-path-via-import.meta.url>` - `WORKSPACE_DIR` env var was the *legacy* name (now `SUTANDO_WORKSPACE`). - Fallback resolved to the repo checkout, so voice-agent wrote tasks, results, conversation.log, briefings, the PIDFILE, audio dirs, etc. to the **repo**, while Sutando.app (post-#837), Python bridges (post-#762), and #842 inline-tools (just landed) all wrote/read from the **workspace**. 2. `CALL_RESULTS_DIR = <repo>/results/calls` — phone-call result records landed under the repo too. Net effect: split-brain on `SUTANDO_WORKSPACE`-enabled hosts. Phone-call `latest-result.json` lived under the repo; new tasks the voice-agent generated landed there too; bridges/Sutando.app couldn't see them. **Pattern (matches #821 / #836 / #842):** ```ts import { resolveWorkspace } from './workspace_default.js'; const WORKSPACE_DIR = resolveWorkspace(); ``` Module-init resolution, single source of truth. CALL_RESULTS_DIR becomes `join(WORKSPACE_DIR, 'results', 'calls')` — same workspace, just the subpath. **Doc-comment cleanup:** - Renamed env-doc at line 19 from `WORKSPACE_DIR` (legacy) to `SUTANDO_WORKSPACE` (canonical) with a pointer to workspace_default.ts. - Removed the stale "default to sutando/ so Claude Code subprocess picks up CLAUDE.md automatically" comment at line 96. Voice-agent doesn't spawn Claude Code anymore (`grep -n claude src/voice-agent.ts` is empty) — task-bridge handles that via the file pipeline. The dual-use rationale that conflated repo + workspace into one var is obsolete. **Verified:** - `npx tsc --noEmit` ✓ clean. - Diff is targeted: 1 file, 15+/-4. - All 15 existing call sites (lines 98, 142, 334, 366, 515, 547, 551, 553, 799, 985, 1110, 1111, 1113, 1299, 1302, 1387) keep their `WORKSPACE_DIR` reference — they automatically pick up the new resolution. No site-by-site edits required. **Operational note:** This change takes effect on the next voice-agent restart. Currently-running voice-agent processes keep their old paths until restart. No mid-flight disruption.
Merged
2 of 3 tasks
Merged
4 tasks done
liususan091219
pushed a commit
that referenced
this pull request
May 18, 2026
…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>
sonichi
added a commit
that referenced
this pull request
May 18, 2026
…loses #840 properly) (#849) 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>
This was referenced May 18, 2026
This was referenced May 18, 2026
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
sonichi#815) (sonichi#836) * fix(workspace): honor SUTANDO_WORKSPACE in 5 scripts — same bug class as PR sonichi#815/sonichi#832/sonichi#833 Audit per owner's "REPO_DIR might be more places than sonichi#815" — confirmed 10 bugs across 5 scripts all hitting the same workspace-vs-repo distinction the bridges fixed pre-PR sonichi#762. Each was resolving per-user-state paths (logs/, results/, state/, build_log.md, core-status.json, pending-questions.md) to the repo root instead of $SUTANDO_WORKSPACE. ## src/health-check.py — 6 sites - L314 voice-agent.log freshness check → WORKSPACE_DIR/logs - L598 core-status.json proactive-loop liveness → WORKSPACE_DIR - L681 build_log.md critical-file check → WORKSPACE_DIR - L795 bridge log-staleness check → WORKSPACE_DIR/logs - L807 bridge heartbeat check → WORKSPACE_DIR/state - L1171 restart-on-fix subprocess stdout redirect → WORKSPACE_DIR/logs WORKSPACE_DIR was already wired in this file (line 42 from earlier work); just substituting the right call sites. Code paths (src/*.py, src/*.ts, node_modules, .env, Sutando.app binary, subprocess cwd) correctly stay as REPO_DIR. Effect: health-check no longer false-positives "log stale / heartbeat missing / sutando-app not running" when services are actually fine. Real issues (voice-agent code stale, discord-bridge code stale) now surface cleanly instead of being lost in the false-positive cascade. ## src/event_log.py - L53–54: REPO_DIR/logs → WORKSPACE_DIR/logs (LOGS_DIR module global) - L71: personal_path(...) workspace arg → WORKSPACE_DIR ## src/check-pending-questions.py - L20: WORKSPACE = Path(__file__).parent.parent → resolve_workspace() This one was particularly silent — the cron's been firing for weeks reading the wrong file. After fix: surfaces the 2 long-pending questions (May 9 email-organize, May 13 web-UI bundle/fork) that the friction detector also found. ## src/call-stats.py + src/scan-call-logs.py - Both hardcoded `Path(__file__).parent.parent / "results" / "calls" / ...` - Now use resolve_workspace() (same pattern as daily-insight.py PR sonichi#832 + friction-detector.py PR sonichi#833) call-stats verified post-fix: now reads 2 real calls; previously read 0 on workspace-only installs. ## What's NOT in this PR - health-check.py L693 (notes-dir) — leaving to James's PR sonichi#815 to avoid conflict; same bug, same fix. ## Verification $ python3 src/health-check.py # core-proactive-loop ok, no false positives $ python3 src/check-pending-questions.py # Notified: 2 pending questions $ python3 src/call-stats.py # 2 real calls, Friday busiest $ python3 src/scan-call-logs.py # No new calls to scan (total: 2) * test(health-check): mock WORKSPACE_DIR not REPO_DIR for core-status check PR sonichi#836 moved core-status.json read from REPO_DIR to WORKSPACE_DIR (per-user runtime state, not code). The test still mocked REPO_DIR, so case e ('running with 5-day-old ts should be warn') silently failed against the real workspace's core-status.json instead of the temp fixture. Update write_status, with_repo_override, and case_b_status_malformed to mock WORKSPACE_DIR. All 17 cases pass. CI flagged by Chi in Discord, 2026-05-18. --------- 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
…udit (sonichi#838) PR sonichi#836 found 10 bug sites across 5 scripts that hardcoded the workspace path to the repo checkout. This pass catches three more scripts the first audit missed (they use REPO/REPO_DIR variable names, and one set of bug sites lives under skills/, which the earlier grep didn't scan). Same bug class. Each script now resolves the workspace via `workspace_default.resolve_workspace()` — the canonical helper introduced in PR sonichi#762. **Sites fixed (4 across 3 scripts):** - `src/archive-stale-results.py` — `RESULTS = REPO / "results"` resolved to `<repo>/results/` which doesn't exist post-sonichi#762. `if not RESULTS.is_dir()` short-circuited the whole sweep, so the DM-flood prevention this script was built for (2026-04-15 incident, 142-file flood) was silently defeated on every startup since sonichi#762. Now points at `$SUTANDO_WORKSPACE/results/`. - `skills/regression-search/scripts/find-regression.py` — `CALLS_FILE` resolved to `<repo>/results/calls/calls.jsonl` which doesn't exist. Every `find-regression "..."` query silently returned "calls file not found" instead of scanning the real call history (which lives in the workspace). - `skills/regression-search/scripts/diagnose-call.py` — both `CALLS_FILE` and `METRICS_FILE` hit the same class. `diagnose-call <sid>` always printed "No call matches" regardless of input. **Why these escaped sonichi#836's audit:** The original grep was `WORKSPACE = Path(__file__)` and `Path(__file__).parent.parent` restricted to `src/`. These three scripts (a) use `REPO`/`REPO_DIR` variable names instead of `WORKSPACE`, and (b) one set lives under `skills/`. Widening the grep to `Path(__file__)\.(resolve()\.)?parent\.parent` across `src/`, `scripts/`, and `skills/` surfaced them. **Verified:** - `DRY_RUN=1 python3 src/archive-stale-results.py` — now sees workspace results dir (previously printed "results/ missing — nothing to do"). - `find-regression "test"` — now scans real `calls.jsonl` (2 entries) and returns "No calls match 'test'" instead of "calls file not found". - Both regression-search imports resolve to the workspace dir via the same `workspace_default.resolve_workspace()` shared with health-check.py / event_log.py / etc. post-sonichi#836. **Follow-ups (none blocking):** - The remaining sites flagged by the widened grep are legitimate: REPO_DIR in agent-api.py / dashboard.py / github-webhook.py is used for code-adjacent paths (running health-check.py via REPO_DIR/src, git cwd, .env loading) and is the correct semantics. util_paths.py + health-check.py REPO_DIR are likewise code-adjacent post-sonichi#836.
liususan091219
pushed a commit
to liususan091219/sutando-fork
that referenced
this pull request
May 28, 2026
closes sonichi#840) (sonichi#841) The describe block shares one spawned web-client child for all subtests. Tool-track 'working' set in one subtest can linger into the next when CI scheduling delays the async 'clear-tool' POST that the producing test issues at its tail. Two unrelated PRs flaked on this same suite today (sonichi#836 and sonichi#832), both on workspace-fix PRs that don't touch web-client.ts — confirms the flake is the test, not the SUT. Fix: add a beforeEach that POSTs idle to both tool and browser tracks + a 20ms flush window. Per-subtest baseline removes the order-dependency that's been latent for the test's lifetime. Verified: 9/9 subtests pass; full TS suite 162/162 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
…ols sites (issue sonichi#839) (sonichi#842) Round-3 audit followup to sonichi#836/sonichi#833/sonichi#838 — first slice of the TS audit flagged in sonichi#839. Five bug sites across two files, all hitting the same class: code paths resolving workspace data against `process.env.WORKSPACE_DIR` (wrong env var name) or `process.cwd()` instead of `resolveWorkspace()`. **Sites fixed:** - `src/cartesia-tts.ts:29` (`getWorkspace`) — TTS audio output dir (`results/audio/<category>/<label>-<ts>.wav`). Pre-fix: `WORKSPACE_DIR` env (legacy name, not set) → `process.cwd()` fallback → audio written to whatever cwd the launcher had. Now resolves to `$SUTANDO_WORKSPACE`. - `src/inline-tools.ts:471, 472` (`cancel_task` tool) — `tasksDir` and `resultsDir` for the cancel-task lookup. The tool listed tasks from `<cwd>/tasks/` and wrote cancel results to `<cwd>/results/`. On hosts with `SUTANDO_WORKSPACE` set, the bridge writes tasks to the workspace but cancel_task was looking in the repo (or whatever cwd) — so voice- initiated cancels would never find an in-flight task and would silently write a stranded CANCEL_INSTRUCTION file. - `src/inline-tools.ts:775` (`show_view` tool) — `dynamic-content.json` for the menu-bar app's view toggle. Pre-fix the inline tool wrote to `<cwd>/dynamic-content.json` while Sutando.app (post-sonichi#837) reads from `$SUTANDO_WORKSPACE/dynamic-content.json`. Split-brain: voice toggle fired but the app never saw it on hosts with `SUTANDO_WORKSPACE`. - `src/inline-tools.ts:873` (`recent_context` tool) — voice-agent state context. Pre-fix the read path looked at `<cwd>/state/voice-session-context.json` but the writer (core via CLAUDE.md "Voice session context" section) writes to `$SUTANDO_WORKSPACE/state/`. Voice-agent's deictic-pronoun recovery ("what was the post?", "what's pending?") would return `{note: "no context recorded yet"}` even when the writer had populated the file. - `src/inline-tools.ts:772` (`NOTES_DIR`) — `sharedPersonalPath('notes', process.cwd())` passes cwd as the workspace argument to the helper. When `SUTANDO_PRIVATE_DIR` isn't set, the helper falls through to `join(workspace, 'notes')` which becomes `<cwd>/notes/` instead of `$SUTANDO_WORKSPACE/notes/`. read_note / search_notes silently look in the wrong place. **Pattern (matches sonichi#821):** ```ts import { resolveWorkspace } from './workspace_default.js'; const WORKSPACE_DIR = resolveWorkspace(); ``` Module-init resolution mirrors the Python `WORKSPACE_DIR = resolve_workspace()` canonical pattern. Single source of truth per module. **Not addressed in this PR (still in sonichi#839's backlog):** - `src/voice-agent.ts:97, 103` — `WORKSPACE_DIR`/`CALL_RESULTS_DIR`. The highest-traffic sites; defer to a focused review. - `src/util_paths.ts:26, 54` — the fallback inside the helper itself. Cleaner to fix every caller first, then either delete the fallback or switch it to `resolveWorkspace()` so the helper has a sensible default. - `src/inline-tools.ts:957, 1021` — `join(process.cwd(), 'skills')`. These are CODE-adjacent paths (skills ship with the code), so they need REPO_ROOT semantics, not WORKSPACE_DIR. Latent today because the launcher always cwd's to the repo, but worth a separate PR. - `src/mentra-bridge.ts:88` — file is currently untracked (WIP). **Verified:** - `npx tsc --noEmit` ✓ clean across the whole tree. - Diff is targeted: 2 files, 22+/-8. 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
… followup) (sonichi#843) Migrates voice-agent.ts's `WORKSPACE_DIR` + `CALL_RESULTS_DIR` constants to `resolveWorkspace()` (the canonical TS helper introduced in sonichi#821). Two const definitions cover ~15 call sites — all of which use the path for per-user runtime state (tasks/, results/, state/, conversation.log, briefings, audio, dynamic-content, PIDFILE, etc.). **Why this matters:** On hosts with `SUTANDO_WORKSPACE` set (e.g. this machine, where `SUTANDO_WORKSPACE=/Users/qingyunwu/Documents/github/sutando-workspace-001`), the pre-fix behavior was: 1. `WORKSPACE_DIR = process.env.WORKSPACE_DIR || <repo-path-via-import.meta.url>` - `WORKSPACE_DIR` env var was the *legacy* name (now `SUTANDO_WORKSPACE`). - Fallback resolved to the repo checkout, so voice-agent wrote tasks, results, conversation.log, briefings, the PIDFILE, audio dirs, etc. to the **repo**, while Sutando.app (post-sonichi#837), Python bridges (post-sonichi#762), and sonichi#842 inline-tools (just landed) all wrote/read from the **workspace**. 2. `CALL_RESULTS_DIR = <repo>/results/calls` — phone-call result records landed under the repo too. Net effect: split-brain on `SUTANDO_WORKSPACE`-enabled hosts. Phone-call `latest-result.json` lived under the repo; new tasks the voice-agent generated landed there too; bridges/Sutando.app couldn't see them. **Pattern (matches sonichi#821 / sonichi#836 / sonichi#842):** ```ts import { resolveWorkspace } from './workspace_default.js'; const WORKSPACE_DIR = resolveWorkspace(); ``` Module-init resolution, single source of truth. CALL_RESULTS_DIR becomes `join(WORKSPACE_DIR, 'results', 'calls')` — same workspace, just the subpath. **Doc-comment cleanup:** - Renamed env-doc at line 19 from `WORKSPACE_DIR` (legacy) to `SUTANDO_WORKSPACE` (canonical) with a pointer to workspace_default.ts. - Removed the stale "default to sutando/ so Claude Code subprocess picks up CLAUDE.md automatically" comment at line 96. Voice-agent doesn't spawn Claude Code anymore (`grep -n claude src/voice-agent.ts` is empty) — task-bridge handles that via the file pipeline. The dual-use rationale that conflated repo + workspace into one var is obsolete. **Verified:** - `npx tsc --noEmit` ✓ clean. - Diff is targeted: 1 file, 15+/-4. - All 15 existing call sites (lines 98, 142, 334, 366, 515, 547, 551, 553, 799, 985, 1110, 1111, 1113, 1299, 1302, 1387) keep their `WORKSPACE_DIR` reference — they automatically pick up the new resolution. No site-by-site edits required. **Operational note:** This change takes effect on the next voice-agent restart. Currently-running voice-agent processes keep their old paths until restart. No mid-flight disruption.
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Owner asked: "PR #815 is just one example. There might be more of such issues." Audit confirmed 10 bugs across 5 scripts all hitting the same workspace-vs-repo distinction the bridges fixed pre-#762.
Each script was resolving per-user-state paths (`logs/`, `results/`, `state/`, `build_log.md`, `core-status.json`, `pending-questions.md`) to the repo root instead of `$SUTANDO_WORKSPACE`. On any workspace-only install (i.e. canonical setup post-#762), the scripts were reading/writing the wrong place — silently for some, with FileNotFoundError for others.
Files
Each follows the same canonical pattern as PR #832 (daily-insight) and #833 (friction-detector): `from workspace_default import resolve_workspace; WORKSPACE = resolve_workspace()`.
What's NOT in this PR
Verification
```
$ python3 src/health-check.py
✓ core-proactive-loop ok running (186s ago)
✓ sutando-app ok running (⌃C/⌃V/⌃M)
✓ task-queue ok 3 task(s), oldest 309s
⚠ voice-agent: stale (running but code is 998 min newer than process — restart needed) # REAL
⚠ discord-bridge: stale (running but code is 1117 min newer than process — restart needed) # REAL
$ python3 src/check-pending-questions.py
Notified: 2 pending questions # the May 9 + May 13 questions; cron was silently broken
$ python3 src/call-stats.py
☎️ Top callers:
+1-434-XXX-XXXX: 2
$ python3 src/scan-call-logs.py
No new calls to scan (total: 2, already scanned: 2).
```
The false-positive noise I've been logging on every health-check task for 18+ hours ("telegram-bridge log stale, discord-bridge log stale, sutando-app not running") is gone with this merge.
🤖 Generated with Claude Code