Repository navigation
fix(workspace): honor SUTANDO_WORKSPACE in 3 more scripts (round 2 audit, followup to #836) - #838
Merged
Merged
Conversation
…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.
qingyun-wu
force-pushed
the
fix/workspace-path-bugs-batch-2
branch
from
May 18, 2026 18:59
bd30461 to
7c90d56
Compare
sonichi
reviewed
May 18, 2026
sonichi
left a comment
Owner
There was a problem hiding this comment.
LGTM. Same mechanical shape as PR 836 — 3 scripts (diagnose-call.py, find-regression.py, archive-stale-results.py) migrate from Path-walkup to resolve_workspace() for runtime data (results/calls, data/call-metrics, results/). Each diff has the bug-citation comment explaining why the old path was silently no-op'ing post-PR 762. Helpful for future readers. Merging.
This was referenced May 18, 2026
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>
This was referenced May 18, 2026
Merged
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
…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>
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
PR #836 fixed 10 bug sites across 5 scripts hitting the workspace-vs-repo class. Owner asked the question "PR #815 is just one example. There might be more of such issues." — the answer was yes, more sites existed, and #836 was "all I could find from the first sweep." A widened grep across
src/,scripts/, andskills/(the previous audit only coveredsrc/) surfaces 3 more scripts × 4 more bug sites, same bug class.Sites fixed
src/archive-stale-results.pyRESULTS)RESULTS = REPO / \"results\"resolved to<repo>/results/which doesn't exist post-#762.if not RESULTS.is_dir()short-circuited the entire sweep, so the DM-flood prevention this script exists for (2026-04-15 incident, 142-file flood) has been silently defeated on every startup since the workspace migration. Active impact: startup never archives.skills/regression-search/scripts/find-regression.pyCALLS_FILE)<repo>/results/calls/calls.jsonl(doesn't exist). Everyfind-regression \"...\"query printed "calls file not found" instead of scanning real call history.skills/regression-search/scripts/diagnose-call.pyCALLS_FILE+METRICS_FILE)diagnose-call <sid>always printed "No call matches" regardless of input.Why these escaped #836's audit
#836's grep was
WORKSPACE = Path(__file__)andPath(__file__).parent.parentrestricted tosrc/. These three scripts (a) useREPO/REPO_DIRas the variable name instead ofWORKSPACE, and (b) one pair lives underskills/. Widening toPath(__file__)\.(resolve()\.)?parent\.parentacrosssrc/,scripts/, andskills/surfaced them.The remaining sites flagged by the widened grep are legitimate —
REPO_DIRinagent-api.py,dashboard.py,github-webhook.py,util_paths.py, andhealth-check.py(post-#836) is used for code-adjacent paths (REPO_DIR/src/health-check.py, git cwd,.envloading). Those are the correct semantics.Fix pattern
Each script now uses the canonical helper:
Verified locally
DRY_RUN=1 python3 src/archive-stale-results.py— now reaches the sweep loop (previously short-circuited at "results/ missing — nothing to do").python3 skills/regression-search/scripts/find-regression.py \"test\"— now scans realcalls.jsonl(2 entries) and returns "No calls match 'test'" instead of "calls file not found: /results/calls/calls.jsonl".WORKSPACE/CALLS_FILE/METRICS_FILEagainst\$SUTANDO_WORKSPACE(or~/.sutando/workspace/default).Test plan
python3 -c \"import importlib...\"round-trip on each —RESULTS/CALLS_FILE/METRICS_FILEresolve to workspace paths that exist.Followup audit status
This brings the running total to 13 bug sites across 8 scripts in the workspace-vs-repo class (#815 + #832 + #833 + #836 + this PR). One more sweep is worth doing for completeness — I'll check TS callers and any
new URL('..', import.meta.url)patterns in a follow-up if anything turns up.