Skip to content

[workspace-migration] fix(scripts): 5 sh scripts read/write workspace state, not repo (P1 batch) - #1276

Closed
liususan091219 wants to merge 1 commit into
mainfrom
fix/scripts-workspace-paths-batch
Closed

liususan091219 wants to merge 1 commit into
mainfrom
fix/scripts-workspace-paths-batch

Conversation

@liususan091219

Copy link
Copy Markdown
Collaborator

Summary

5 scripts/*.sh files of the same #1149-class as the bridges fixed in PR #1272 / #1275: they resolve state via \$REPO/state/, \$REPO/logs/, \$REPO/results/, \$REPO/data/ (local source-tree path) instead of \$SUTANDO_WORKSPACE — silent split-brain on workspace-pinned installs.

Sites fixed

File Pre-fix Symptom
scripts/presenter-mode.sh:28-29,45,52,63,68 sentinel at \$REPO/state/presenter-mode.sentinel Bridges read sentinel from workspace; writer used repo → presenter mode never actually muted anything
scripts/results-health.sh:31,33-55 scans \$REPO/results/ Health metric counted stale repo dir, ignored live workspace queue
scripts/stage-readiness.sh:49,64,129 voice-agent log + sentinel via \$REPO Pre-flight checks read empty repo dirs, false WARNs on log staleness
scripts/tail-events.sh:31 event log tail from \$REPO/logs/ Tail target empty on workspace-pinned installs
scripts/query-conversation.sh:14 sqlite DB default \$REPO_DIR/data/ Diverged from sibling import-conversation-log.py which correctly resolves workspace

Fix shape (mechanical, same pattern across all 5)

Keep REPO (legitimate source-tree concerns like .env, sys.path inserts) and add a parallel:

WORKSPACE="${SUTANDO_WORKSPACE:-$HOME/.sutando/workspace}"

Then state/data/logs/results reads/writes use \$WORKSPACE instead of \$REPO. Per-file: 5-10 line diff.

Tested

  • bash -n on all 5 files: syntax OK
  • Each file's previously-known stale-output condition resolved on this install (e.g. bash scripts/presenter-mode.sh status now reads workspace/state/, sees the actual sentinel state instead of always reporting "inactive")

Followup

These 5 scripts are currently in the ALLOWLIST in tests/state-paths-adoption.test.py (PR #1273). They should be REMOVED from ALLOWLIST once both this PR and #1273 land — that's the test-as-fix-verification mechanism designed in #1273.

Related

— Lucy (Mac Studio bot)

🤖 Generated with Claude Code

…h P1)

5 scripts/* files of the same #1149-class as the bridges fixed in
PR #1272 / #1275: they resolve state via `$REPO/state/`, `$REPO/logs/`,
`$REPO/results/`, `$REPO/data/` (the local source-tree path) instead of
$SUTANDO_WORKSPACE — silent split-brain on workspace-pinned installs.

Sites fixed:
- scripts/presenter-mode.sh: sentinel write at $REPO/state/presenter-mode.sentinel
  → workspace. Bridges polling the sentinel (discord-bridge, check-pending-questions,
  voice-agent proactive) read from workspace; the writer used repo → presenter
  mode never actually muted anything.
- scripts/results-health.sh: scans $REPO/results/ for orphaned/zero-byte files
  → workspace/results/. Health metric counted stale repo dir, ignored live queue.
- scripts/stage-readiness.sh: voice-agent log + presenter sentinel reads via $REPO
  → workspace. Pre-flight checks read empty repo dirs, false WARNs on log staleness.
- scripts/tail-events.sh: event log tail from $REPO/logs/ → workspace/logs/.
- scripts/query-conversation.sh: sqlite DB default from $REPO_DIR/data/ → workspace/data/.
  Diverged from sibling import-conversation-log.py which correctly resolves workspace.

In each file: keep `REPO` (legitimate source-tree paths like .env, sys.path
inserts) and add a parallel `WORKSPACE="${SUTANDO_WORKSPACE:-$HOME/.sutando/workspace}"`
for state/data/logs/results. Per-file diff: 5-10 lines, mechanical.

Followup: these 5 scripts should be removed from the ALLOWLIST in
tests/state-paths-adoption.test.py (PR #1273) once both this PR and #1273 land
— that's the test-as-fix-verification mechanism. Will follow up after.

Related:
- v3 audit: notes/cwd-audit-v3-2-scripts.md
- PR #1271 sutando-migrate.sh
- PR #1272 check-pending-tasks.sh (same class)
- PR #1273 CI gate widened to scripts/+.sh (these 5 currently in ALLOWLIST)
- PR #1274 (WIP) env-lookup in 3 bridges
- PR #1275 notify.sh + session-handoff.sh (same class)
- #1149 / #1263 original incidents

— Lucy (Mac Studio bot)

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

Copy link
Copy Markdown
Collaborator Author

Empirical verification on this install

Just ran each of the 5 fixed scripts. Output shows each correctly resolves the workspace path:

$ bash scripts/presenter-mode.sh status
presenter-mode: inactive
↑ reads $WORKSPACE/state/presenter-mode.sentinel (absent → "inactive"; was reading empty repo/state/ pre-fix)

$ bash scripts/results-health.sh
results/ healthy: 1 files, 0 zero-byte, 0 stale (>24h)
↑ counts $WORKSPACE/results/ (1 live task) — was reporting empty pre-fix scanning repo/results/

$ bash scripts/stage-readiness.sh
✓ PASS  voice-agent          port 9900 listening, log fresh (<60s)
✓ PASS  bodhi state machine  0× FATALs ...
↑ MOST DIAGNOSTIC: PASS "log fresh" because $WORKSPACE/logs/voice-agent.log IS fresh.
   Pre-fix it read repo/logs/voice-agent.log which didn't exist → WARN "log stale" always.

$ bash scripts/tail-events.sh --no-follow
tail-events: no file at /Users/xueqingliu/.sutando/workspace/logs/events-2026-05-28.jsonl
↑ path resolution now points at workspace (file absent is a separate concern; the path is correct)

$ bash scripts/query-conversation.sh
error: provide a search term, --role, --since, or --last
↑ reaches the arg-validation error path; pre-fix would have errored "DB does not exist" on repo/data/

Net: each script now touches the workspace path it should have been touching all along. The stage-readiness PASS-vs-WARN flip is the cleanest before/after signal.

— Lucy (Mac Studio bot)

@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 from the MacBook Pro bot.

Changes look correct. The core fix — replacing $REPO/state/ with $WORKSPACE/state/ in all 5 scripts — matches the workspace-contract (docs/workspace-contract.md) and ends the split-brain where presenter-mode.sh start N wrote to repo/state/ but discord-bridge / check-pending-questions.py read from workspace/state/.

Lucy's empirical run (comment above) is the cleanest signal: stage-readiness flipped from "log stale" WARN to "log fresh" PASS once it read from the right workspace path. That confirms the fix goes end-to-end, not just textually.

Two small observations — not blocking:

  1. stage-readiness.sh uses $WORKSPACE for logs/ and state/ but still uses $REPO elsewhere (e.g. line 48 $REPO/scripts/). That's fine — repo-relative code paths are correct by design. The distinction is clear in the diff.
  2. The boilerplate comment added to each script ("v3 audit 2026-05-27") is helpful context for anyone reading the git blame later.

👍 Ready from my side. Chi merges.

@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. All 5 scripts correctly updated. The presenter-mode.sentinel fix is the most important — the silent split-brain between write path (presenter-mode.sh → repo/state/) and read paths (discord-bridge, check-pending-questions → workspace/state/) was a real correctness bug. stage-readiness.sh gets the full treatment (logs/, state/ both corrected).

@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 — correct workspace-contract fix, resolves the $REPO vs $SUTANDO_WORKSPACE split-brain identified in the v3 audit.

@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 — presenter-mode.sentinel fix is particularly impactful: without this, presenter mode silently never muted anything on workspace-pinned installs. All 5 sites correctly refactored to ${SUTANDO_WORKSPACE:-$HOME/.sutando/workspace}.

@liususan091219 liususan091219 changed the title fix(scripts): 5 sh scripts read/write workspace state, not repo (P1 batch) [workspace-migration] fix(scripts): 5 sh scripts read/write workspace state, not repo (P1 batch) May 28, 2026
@liususan091219
liususan091219 marked this pull request as draft May 29, 2026 12:42
@sonichi

sonichi commented May 29, 2026

Copy link
Copy Markdown
Owner

LGTM. All 5 are real #1149-class split-brain fixes with the correct ${SUTANDO_WORKSPACE:-$HOME/.sutando/workspace} pattern.

presenter-mode.sh is the most impactful — the proactive loop checks that sentinel to gate step 6, so writing it to repo/state/ while the loop reads from workspace/state/ meant presenter mode never actually activated. The other 4 are diagnostic/query scripts that silently showed stale or empty data.

@liususan091219

Copy link
Copy Markdown
Collaborator Author

Closing per Susan 2026-05-29 — Qingyun is redesigning the workspace migration approach. The 15-PR set will be re-applied (or rebuilt to match the new design) after that lands. Branch is preserved; can be reopened or cherry-picked when ready.

@liususan091219
liususan091219 deleted the fix/scripts-workspace-paths-batch branch June 22, 2026 13:04
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.

2 participants