Skip to content

fix(health-check): resolve notes-dir against WORKSPACE_DIR not REPO_DIR - #815

Merged
sonichi merged 4 commits into
sonichi:mainfrom
jayjaybeeuk:fix/health-check-notes-workspace-path
May 18, 2026
Merged

sonichi merged 4 commits into
sonichi:mainfrom
jayjaybeeuk:fix/health-check-notes-workspace-path

Conversation

@jayjaybeeuk

Copy link
Copy Markdown
Contributor

Problem

After PR #793's workspace migration, notes/.gitkeep was removed from the repo (state/.gitkeep too). The notes-dir health check now always reports missing on every machine:

✗ notes-dir    missing    /Users/.../sutando/notes

Root cause: health-check.py:693 calls shared_personal_path("notes", REPO_DIR) — passing the repo root as the workspace argument. Post-migration, notes live at $WORKSPACE/notes (~/.sutando/workspace/notes), not at <repo>/notes.

Fix

Pass WORKSPACE_DIR (already imported via resolve_workspace()) instead of REPO_DIR:

checks.append(check_directory(Path(shared_personal_path("notes", WORKSPACE_DIR)), "notes-dir"))

Verified

✓ notes-dir    ok    0 .md files

Health check passes locally after creating ~/.sutando/workspace/notes/.

Test plan

  • Run python3 src/health-check.py on a machine with no <repo>/notes dir but with ~/.sutando/workspace/notes/ — should show ✓ notes-dir
  • Run python3 src/health-check.py with SUTANDO_PRIVATE_DIR set — should still resolve correctly via shared_personal_path's private-dir branch

PR sonichi#793's workspace migration deleted notes/.gitkeep from the repo, so
the notes-dir health check now always fails on fresh pulls: it was
calling shared_personal_path("notes", REPO_DIR) which resolves to
<repo>/notes — a path that no longer exists. WORKSPACE_DIR
(~/.sutando/workspace) is the correct workspace root post-migration.
@CLAassistant

CLAassistant commented May 18, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Cold review (MacBook): LGTM, ship it. 3-line fix is correct.

Verified:

  • WORKSPACE_DIR = resolve_workspace() at line 42 → canonical workspace path (~/.sutando/workspace/ or $SUTANDO_WORKSPACE)
  • REPO_DIR = Path(__file__).parent.parent → public repo checkout
  • Post-PR #793 migration, notes/ lives in WORKSPACE_DIR/notes, not REPO_DIR/notes
  • Old code's shared_personal_path("notes", REPO_DIR) resolved to <repo>/notes/ — gone after #793 dropped the .gitkeep
  • Fix's WORKSPACE_DIR lands on the real notes dir

Doesn't conflict with #831: Different naming layer — this PR's WORKSPACE_DIR is the Python local variable in health-check.py (derived from resolve_workspace() which honors SUTANDO_WORKSPACE). #831 standardized the shell env-var naming (SUTANDO_WORKSPACE for workspace, SUTANDO_REPO_DIR for repo). The two are consistent — both call workspace path WORKSPACE/SUTANDO_WORKSPACE.

After #831 merges + cross-fleet rollout (just did MacBook's), ~/.sutando/workspace/notes becomes a symlink → memory-sync/notes. check_directory follows symlinks → resolves green. Stack lands cleanly.

Ship it.

🤖 cold review via Sutando-MacBook

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

Cold review — LGTM. 3-line fix for the exact false-positive I've been hitting in every health-check task all session.

Verified

  • Before: `shared_personal_path("notes", REPO_DIR)` resolves to `/notes` (which doesn't exist post #793 since the `.gitkeep` was removed)
  • After: `shared_personal_path("notes", WORKSPACE_DIR)` resolves to `~/.sutando/workspace/notes` (the canonical location)
  • This is the same workspace-vs-repo distinction PR #762 / #793 / #832 / #833 keep hammering home

Tested locally — health-check now reports notes-dir ✓ instead of ✗.

Ship it. This kills one of the two recurring "no action — false positive" lines I've been writing into every health-check task reply for the last 12+ hours.

sonichi added a commit that referenced this pull request May 18, 2026
#815) (#836)

* fix(workspace): honor SUTANDO_WORKSPACE in 5 scripts — same bug class 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)

* test(health-check): mock WORKSPACE_DIR not REPO_DIR for core-status check

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.

---------

Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM. Clean 3-line fix matching CLAUDE.md workspace contract (notes-dir lives under WORKSPACE_DIR not REPO_DIR per the contract). Real post-#793 regression. Ship.

@sonichi
sonichi merged commit a996b2f into sonichi:main May 18, 2026
2 checks passed
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
…IR (sonichi#815)

PR sonichi#793's workspace migration deleted notes/.gitkeep from the repo, so
the notes-dir health check now always fails on fresh pulls: it was
calling shared_personal_path("notes", REPO_DIR) which resolves to
<repo>/notes — a path that no longer exists. WORKSPACE_DIR
(~/.sutando/workspace) is the correct workspace root post-migration.

Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>
Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>
sonichi added a commit that referenced this pull request May 29, 2026
…solvers (closes #863)

Guards against re-introducing the workspace-vs-repo split-brain bug class
fixed in PR family #815-#859. Any future `process.cwd()` in TS code or
`Path.cwd()`/`os.getcwd()` in Python outside the resolver modules now fails CI.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
liususan091219 added a commit that referenced this pull request May 30, 2026
…solvers (closes #863) (#1322)

Guards against re-introducing the workspace-vs-repo split-brain bug class
fixed in PR family #815-#859. Any future `process.cwd()` in TS code or
`Path.cwd()`/`os.getcwd()` in Python outside the resolver modules now fails CI.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Susan Xueqing Liu <xliu127@stevens.edu>
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.

4 participants