Skip to content

feat(mentra-bridge): track WIP bridge + wire to DeviceSession registry - #740

Closed
qingyun-wu wants to merge 1 commit into
mainfrom
feat/mentra-bridge-tracked
Closed

qingyun-wu wants to merge 1 commit into
mainfrom
feat/mentra-bridge-tracked

Conversation

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Summary

Promotes the owner's untracked WIP src/mentra-bridge.ts (322 lines) into a tracked file and wires it to the DeviceSession registry added in #739.

When a Mentra glasses session connects, the bridge now POSTs to /vision/register with the full DeviceProfile (mentra-live: 400×240 green HUD center, HD 119° camera, 3-mic array, tap gestures), and unregisters on teardown. Best-effort — silently falls back to legacy push-mode-only behavior if /vision/register 404s, so the bridge keeps working against pre-#739 voice-agent.

Depends on #739 for the runtime endpoint (graceful degradation when missing).

Diff scope

  • 320 of 322 lines are unchanged from the owner's working-tree WIP.
  • Added: registerDevice(), unregisterDevice(), call sites in onSession() and teardown().

Test plan

Roadmap impact

  • §5 Now item 2 (Mentra Live wire-up): scaffolding is in repo. Profile + capabilities flow through the registry; the output router (next PR) can consume them.
  • TODO 1 in the file header (DeviceSession + profile catalog): partially resolved — full output-router consumption is the follow-up.

🤖 Generated with Claude Code

Promotes the previously-untracked `src/mentra-bridge.ts` (322 lines, owner
WIP) into a tracked file and wires it to the DeviceSession registry that
PR #739 adds. When a Mentra glasses session connects, the bridge now:

- Posts to `/vision/register` with the full DeviceProfile (mentra-live:
  400×240 green HUD center, HD 119° camera, 3-mic array, tap gestures).
  Best-effort — silently falls back to legacy push-mode-only behavior if
  the endpoint 404s (so the bridge works against pre-#739 voice-agent
  too).
- Posts to `/vision/unregister` in the teardown handler so the registry
  doesn't leak entries after a glasses disconnect / app exit.

Roadmap impact:
- §5 Now item 2 (Mentra Live wire-up): scaffolding present in repo;
  the device profile now appears in `/vision/devices` when paired.
- TODO 1 from the file's own header ("DeviceSession + profile catalog"):
  partially resolved — bridge participates in the registry. The full
  output-router consumption is the next PR.

Test plan:
- [x] `npx tsc --noEmit` clean.
- [ ] Manual: with #739 merged + `@mentra/sdk` installed +
      `MENTRA_API_KEY` set, `tsx src/mentra-bridge.ts` → glasses pair →
      `curl :7847/vision/devices` shows `mentra-live-<sessionId>` as
      active with the correct profile.
- [ ] Manual against pre-#739 voice-agent: bridge logs "register
      endpoint unavailable; continuing in legacy mode" and the existing
      push-mode path still works.

Owner WIP context: 320 of 322 lines are unchanged from the working tree.
Added: `registerDevice()`, `unregisterDevice()`, the call sites in
`onSession()` and `teardown()`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
qingyun-wu added a commit that referenced this pull request May 16, 2026
…n/frame

Consumes #746's access-policy scaffold. Two thin hooks:

1. /vision/register — attachPolicy(session) right after register(). The
   defaulted policy comes from the profile's category (laptop=full,
   glasses=HUD+camera+voice, recorder=task-only, other=empty). Logged
   in the response so bridges can verify what they were granted.

2. /vision/frame — checks vision_frame_send capability of the currently-
   active DeviceSession. If denied: 403 + logDenial. The legacy browser
   path (no active DeviceSession, frames go through sessionRef) is
   unaffected — only registered devices hit the gate.

Risk:
- Backward compatible — browser/Watch path uses sessionRef, not the
  registry. activeForVision() returns null there, gate is skipped.
- Mentra glasses (registered via PR #740): category=glasses → defaults
  include vision_frame_send. No behavior change vs. today.
- Recorders (Plaud, Omi Pendant): category=recorder → vision_frame_send
  not in defaults. If a recorder ever registered + tried to push frames
  (which shouldn't happen — they don't have cameras), it'd be denied.
  Correct posture.
- 'Other'-category devices: empty default-allow → 403. Owner must
  explicitly opt them in via state/device-policy.json.

Test plan:
- [x] tsc --noEmit clean.
- [ ] Manual: browser /vision/frame still works (regression).
- [ ] Manual: register mentra-live → frames flow.
- [ ] Manual: register an 'other'-category device → /vision/frame 403s
      with logDenial output.
- [ ] Manual: add 'mystery-device' to state/device-policy.json with
      ['vision_frame_send'] → frames flow.

Depends on PR #746 (access policy). Targets feat/device-access-policy
as base.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.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 DeviceSession wiring on top of the WIP promotion. The graceful degradation against pre-#739 voice-agent is exactly the right shape for a bridge that has to ship before its dependency is universally deployed.

Things I like:

  • 404 graceful degradation in registerDevice() — when voice-agent predates #739 (no /vision/register endpoint), logs "register endpoint unavailable; continuing in legacy mode" and falls through to the existing push-mode-only path. Bridge keeps working across the rollout window without needing a coordinated deploy.
  • deviceId = 'mentra-live-${sessionId}' — uniquely scopes concurrent glasses pairings. Two Mentra units paired at once won't collide in the registry, and /vision/devices can show both with distinct IDs.
  • Profile capabilities sourced from the Mentra SDK's caps object — mic, camera.enabled, hud.enabled are derived rather than hardcoded. Defensive against a future Mentra device that ships without one of these (e.g., glasses with no display).
  • pushSourceLabel: SOURCE_LABEL passed in — preserves the existing push-mode source identification, so downstream vision-tools.ts keeps distinguishing mentra frames from browser/laptop.
  • Teardown ordering correct: stop frame timer → stop managed camera stream → stop vision push → unregisterDevice. Producers stopped before the registry slot is released. No torn-down session left active in the registry.
  • unregisterDevice fire-and-forget with empty catch — fine; if the bridge is teardown-racing the voice-agent dying, we don't want to error out the teardown chain on a network failure.

Two non-blocking observations:

  1. caps?: any defeats type safety. The caps parameter in registerDevice(deviceId, caps?: any) and call sites use caps?.hasMicrophone, caps?.hasCamera, etc. — these are real Mentra SDK fields. Casting via any means a future SDK rename (hasMicrophone → hasMic) silently breaks the profile inference (booleans become undefined, then !! → false, so the profile gets mic: false for devices that actually have one). Either:

    • Import the Mentra capabilities type from @mentra/sdk and use it.
    • Declare a local interface MentraCaps { hasMicrophone?: boolean; hasCamera?: boolean; hasDisplay?: boolean; modelName?: string } and use that.

    Either makes the SDK-rename failure mode a compile error instead of silent capability misreporting.

  2. HUD profile fields hardcoded for mentra-live. resolution: '400x240', color: 'green', position: 'center' matches the §7 fleet matrix entry. When the second Mentra variant lands (Even G2 = 576×288 lower-right), you'll need a lookup table — profileId === 'mentra-live' is the right branch point but it's an implicit assumption. A MENTRA_PROFILES: Record<string, Partial<DeviceProfile>> map keyed by SDK device model name would make the second-variant addition a one-row append rather than an if/else fork.

The "320 of 322 lines unchanged from owner's WIP" framing is the right way to lower this PR's review-risk — the dependency wiring is the only new logic, and it's defensive in the right places.

Ready to merge after #739 lands (which I already LGTM'd).

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

Reviewed: new mentra-bridge.ts streams glasses camera → /vision/frame at 3s thumbnails + tap-to-capture full frames + task emit. HUD render via session.layouts.showTextWall. Registers via /vision/register (PR #739 DeviceSession). WIP-tagged. Note: BEHIND main, rebase before merge. — Lucy (Mac Studio bot)

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

LGTM — first non-test consumer of the DeviceSession + vision-pipeline contract, with the right WIP discipline (lazy SDK load so tsc --noEmit stays green on every box; legacy-mode fallback when register endpoint is 404). The four TODO comments at the top are unusually disciplined — each names the §4/§5/§8 roadmap anchor it punts on, so future-me knows the deferrals are intentional, not oversights.

Things I like:

  1. Lazy SDK load via createRequire + loadSdk() (mentra-bridge.ts:64-79) — import of @mentra/sdk happens only inside start(). Means the file imports cleanly on any box without the SDK installed (CI, fresh checkout, peer node), and tsc --noEmit always passes. The alternative (top-level import) would have made this bridge a CI dependency rather than an optional component.

  2. Legacy-mode fallback on register 404 (mentra-bridge.ts:120-122) — if voice-agent predates the /vision/register endpoint, the bridge logs the version mismatch and continues in legacy push mode. So a partial deploy (newer mentra-bridge on a node with older voice-agent) doesn't brick the bridge; it degrades to the prior behavior. This is the kind of forward/backward compat care that makes rolling deploys safe.

  3. Per-session deviceId = mentra-live-${sessionId} (mentra-bridge.ts:226) — two simultaneous glasses pairings get distinct ids in the registry. Matches #739's Map<deviceId, DeviceSession> shape; without per-session ids, the second pair would silently displace the first via last-activate-wins.

  4. Frame-size sanity check (mentra-bridge.ts:283) — if (buf.byteLength < 2048) return skips blank/tiny frames before paying the POST cost. 2KB is a reasonable JPEG floor — even a blank 720p JPEG is ~5KB. Saves the round-trip on the obvious junk.

  5. Lazy push-mode re-entry on tick (mentra-bridge.ts:285-288) — if voice-agent comes up after the bridge starts, the next interval tries startVisionPush() again. So you can tsx src/mentra-bridge.ts first, then start voice-agent, and frames flow without restarting the bridge.

  6. Photo tap fires task even without live voice session (mentra-bridge.ts:332-334) — writeTask('I just took a photo with my glasses. What do you see?') runs unconditionally. So even if voice-agent is down, the task queue captures the user intent and the proactive loop picks it up later. This is the "drop in, react later" pattern that makes the bridge survive transient outages.

  7. Comprehensive teardown (mentra-bridge.ts:236-244) — clearInterval → stopManagedStream → stopVisionPush → unregisterDevice. Order matters (stop the producer before the consumer) and the try-wrapped unregisterDevice is best-effort. Same pattern as the discord-bridge's session cleanup.

  8. Four roadmap-anchored TODOs (mentra-bridge.ts:42-60) — §4 OC substrate adoption, §4 3-tier access control (mentra session implicitly owner-trusted), §5 Next device-aware output routing (showTextWall is static strings), §5 Next omnia-agent.yaml manifest. Each TODO names the bet/pitfall it's deferring, so a future reader can decide priority without re-reading the roadmap.

Non-blocking observations:

  1. MENTRA_PACKAGE defaults to com.sutando.glasses (mentra-bridge.ts:80) — that package id needs to be registered at portal.mentra.glass/ per the setup notes. If a user runs this without registering, the Mentra portal rejects the session at handshake. Worth surfacing in the connect-failure log path.

  2. writeTask(text) writes to TASKS_DIR without parents=true (mentra-bridge.ts:185) — if a fresh install starts the bridge before voice-agent has created tasks/, the writeFileSync throws ENOENT. Cheap fix: mkdirSync(TASKS_DIR, { recursive: true }) once at startup, or pass parents: true semantics. (See #762's mkdir fixes for the same class of bug.)

  3. onTranscription forwards all final text to writeTask (mentra-bridge.ts:343-348) — this is the §4 access-control TODO playing out: any voice text from the glasses session becomes a task with no access-tier metadata. When the access-control milestone lands, this needs access_tier: owner (or whatever the gate decides) injected into the task header.

  4. Push-mode failures silently drop frames (mentra-bridge.ts:289-290) — if (!ok) pushActive = false; // poll will retry next tick. If voice-agent goes down mid-session, the bridge keeps polling thumbnails but every POST 503s for the duration. A backoff (or "stop polling after N consecutive failures, resume on register-success") would reduce log spam.

  5. Hard-coded HUD shape in registerDevice (mentra-bridge.ts:108-112) — resolution: '400x240', color: 'green', position: 'center' baked into the bridge instead of read from caps. Once #759/#760 land, this PR could shrink to { deviceId, profileId: 'mentra-live' } and the server enriches from the OC catalog. Already noted in TODO #3 about output routing; just confirming the shrink path.

  6. onPhotoTaken writes a hard-coded prompt (mentra-bridge.ts:332) — "I just took a photo with my glasses. What do you see?" Fine for V1, but should become a configurable string (or template) once the bridge gets more use cases. Cheap follow-up.

  7. No package.json entry for @mentra/sdk — intentional (lazy load), but means setup requires the manual npm i @mentra/sdk step in the docstring. Once the bridge stabilizes, dropping it into optionalDependencies would let npm install discover it without breaking installs on non-glasses nodes.

Cross-PR linkages:

  • First real consumer of #739 DeviceSession — registerDevice() POSTs the full DeviceProfile shape today; will shrink to {deviceId, profileId} once #760 lands.
  • First real consumer of #747 access-policy gate — when this PR registers, #747's attachPolicy(session) runs against category: 'glasses', granting the glasses default-allowed capability set.
  • Pairs with #745 voice-agent wire — both PRs need to be live for the full glasses→HUD round trip. This bridge sends frames; #745 routes assistant replies back to HUD via routeReply.
  • §5 Now item 2 milestone — "Mentra wire-up." This PR is the milestone. The four TODOs at the top map the §5/§4 follow-ups.
  • OC adoption tension (TODO #1 / §4 bet) — this bridge talks directly to vision-tools, not through OC's device_register protocol. Deliberate shortcut named in the TODO so the next person knows the OC-adapter rewrite is the intended end state.
  • Workspace fallback (mentra-bridge.ts:84) — process.env.SUTANDO_WORKSPACE || process.cwd() is the TS-side pattern flagged in my #762 review obs 2 as the place where a workspace.ts shim should land.

Tests: None in this PR. WIP marker in the title is honest about the test gap. Worth adding before this goes from WIP→merged: (a) registerDevice POST shape against a mock vision-tools server, (b) frame-size threshold (1KB drops, 3KB passes), (c) teardown order on session disconnect. The lazy-SDK design means tests can mock loadSdk() cleanly without needing @mentra/sdk installed in CI.

WIP → not blocking on merge if Susan/qy want the bridge in-tree to dogfood. Otherwise the test gap + access-control TODO are the two items worth resolving before unmarking WIP.

— Lucy (Mac Studio bot)

qingyun-wu added a commit that referenced this pull request May 16, 2026
…dger

Second consumer of PR #749's per-tenant cost ledger. Mirrors voice-agent's
emit shape from PR #751: 1 request + total duration in ms + 1 tool_call
per session.toolCalls entry. Token counts deferred until bodhi exposes
in/out totals — same shape lights them up when ready.

device_id='phone-call:<callSid>' distinguishes phone-originated usage
from laptop/glasses in audit-summary (#750)'s by_device breakdown.
Non-owner calls emit with source='phone-conversation:non-owner' so
spam-attribution is preserved without leaking the caller number into
device_id keys.

Dynamic import + try/catch keeps cost-accounting an optional dep — if
the module is absent (older worktree, partial deploy), phone session
finalization silently skips emit rather than crash the call-end path.
Mirrors #751's defensive shape.

Test plan:
- [x] npx tsc --noEmit clean.
- [ ] Manual: complete a phone call → verify
      state/cost-accounting/usage-default.jsonl gains api_call(request=1)
      + api_call(ms=N) + per-tool entries with
      device_id='phone-call:<callSid>'.
- [ ] Manual: audit-summary --since 24h shows the Cost ledger
      populated with phone-call.<callSid> in by_device.

Depends on PR #749 (cost-accounting). Targets feat/cost-accounting-
scaffold as base. Independent of PR #751 — both emit into the same
ledger but live at separate call sites.

Roadmap impact:
- §5 Next 'Cost accounting' second consumer landed. Remaining emit
  sites: skill execution (gws-gmail-voice, image-generation, etc.) and
  the Mentra bridge (PR #740).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
qingyun-wu added a commit that referenced this pull request May 16, 2026
Activates PR #749's per-tenant cost-accounting ledger from voice-agent's
session-end path. Without this, the ledger stays empty and the cost
section of audit-summary (PR #750) reports 'No events recorded yet
(cost-accounting emitters not wired)'.

What ships:
- writeVoiceMetrics() now also emits to cost-accounting after the
  existing voice-metrics.jsonl write. Records:
  * 1 'request' event tagged with the voice model (gemini-2.5-flash
    today).
  * Total session duration in ms.
  * 1 'tool_call' per voiceToolCalls entry, with the tool name.
- Token in/out counts are intentionally NOT emitted yet — the realtime
  transport doesn't expose them today. When bodhi adds them (or we add
  a thin counter around sendFile/sendContent), the same emit point
  lights up the token totals; no schema change needed.
- Dynamic import of './cost-accounting.js' so the module is optional
  — older worktrees / partial deploys silently skip emit rather than
  crash the session-end path.

Device-aware shape: 'laptop' deviceId is hardcoded here because the
voice-agent's local session IS the laptop. Per-device emission for
Mentra glasses / phone calls / future devices lives in the bridge
that owns the relevant DeviceSession (PR #740 for Mentra; phone-
conversation's conversation-server.ts is the next consumer).

Test plan:
- [x] npx tsc --noEmit clean.
- [ ] Manual: start voice-agent, complete a session, verify
      `state/cost-accounting/usage-default.jsonl` exists with
      api_call(request=1, ms=N) + per-tool entries.
- [ ] Manual: `tsx src/audit-summary.ts --since 24h` shows the
      Cost ledger section populated.

Depends on PR #749 (cost-accounting scaffold). Targets feat/cost-
accounting-scaffold as base.

Roadmap impact:
- §5 Next 'Cost accounting' consumer landed for voice. Phone-
  conversation + skill execution + Mentra bridge are follow-ups (one
  call site per PR keeps review focused).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
qingyun-wu added a commit that referenced this pull request May 17, 2026
* feat(workspace): canonical default = ~/Library/Application Support/sutando/workspace

Add `src/workspace_default.py` helper that resolves SUTANDO_WORKSPACE → falls
back to `~/Library/Application Support/sutando/workspace/` (a subdir, not the
parent — the parent is Sutando.app's territory for its Chromium-style cache:
Cache/, GPUCache/, Cookies/, blob_storage/, etc.).

Replace the historic per-file fallback (`Path(__file__).resolve().parent.parent`)
in three callers — discord-bridge, telegram-bridge, dm-result — which resolved
to the script's repo root and (a) polluted git status with runtime artifacts on
bare-shell launches that forgot SUTANDO_WORKSPACE, and (b) when invoked from an
app-bundled src/ symlink, walked into the bundle so the bridge wrote tasks to
bundle-tasks/ while the watcher polled workspace-tasks/.

This is the same class of bug that just stranded three owner DMs at the old
Application Support path while the session watched the repo's `tasks/` — single
source of truth across components prevents the entire bug pattern.

Also add CLAUDE.md "Workspace contract" section codifying the convention.

Not in scope:
- mentra-bridge.ts (untracked WIP — tracked by PR #740; follow-up after merge)
- conversation-server.ts, web-client.ts (already merged with their own pattern)
- sync-memory.sh, session-handoff.sh (use SUTANDO_WORKSPACE for "repo path",
  a different semantic — separate decision)
- util_paths.py REPO_DIR (private-dir resolution; separate concern)

Tests: `tests/workspace-default.test.py` — 7 cases covering default location,
env override, ~-expansion, empty-string + whitespace fallbacks, and an explicit
anti-regression check that the default is NOT the repo root.

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

* fixup: default workspace = ~/.sutando/workspace/ (was App Support); parents=True for mkdir

Two CI-fail-driven adjustments:

1. **Default path change.** v1 defaulted to `~/Library/Application Support/sutando/
   workspace/` (macOS-canonical). On Linux CI runners that path is nonsensical
   (`/home/runner/Library/Application Support/...` makes no semantic sense, even
   though the filesystem accepts it). Per owner: switch to `~/.sutando/workspace/`
   — hidden, OS-neutral, home-relative. Same intent (avoid Sutando.app's
   Application Support territory + give workspace a dedicated dir) without
   pretending to be macOS-shaped on non-macOS hosts.

2. **mkdir(parents=True).** v1's bridge module-load code did `TASKS_DIR.mkdir
   (exist_ok=True)` without `parents=True`, which is fine when the parent
   directory tree already exists but raises `FileNotFoundError` on a fresh
   install or a Linux CI runner where `~/.sutando/workspace/` doesn't exist yet.
   Now `parents=True` everywhere the bridge auto-creates a dir from REPO
   (TASKS_DIR, RESULTS_DIR, STATE_DIR — both bridges).

Updated CLAUDE.md "Workspace contract" section + the Python helper docstring
to reflect the new default. Test `test_default_is_application_support_workspace_subdir`
renamed to `test_default_is_dot_sutando_workspace_under_home` with matching
assertions. All 7 helper tests green; the previously-failing CI test
(`discord-bridge-discord-state-detection`) reproduces green locally with
`SUTANDO_WORKSPACE=""`.

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

* fixup: auto-migrate runtime dirs from legacy repo-root fallback to ~/.sutando/workspace/

Addresses @sonichi's PR #762 review observation 1 — breaking change for nodes
that ran against the historic `Path(__file__).resolve().parent.parent` fallback
(e.g. MacBook with state in `~/Desktop/sutando/tasks/`). Without this fix, those
nodes would point at an empty new default post-restart and lose continuity until
the operator manually re-pointed SUTANDO_WORKSPACE or rsync'd state.

Migration semantics:
  • Fires only when SUTANDO_WORKSPACE is unset, the new default doesn't yet
    exist (or is empty), AND the legacy repo root has runtime evidence
    (task-* files in tasks/results/state/). Three-condition gate keeps env-
    pinned installs and fresh-clone CI runs untouched.
  • Moves tasks/, results/, state/ (the three runtime-state dirs) — not the
    whole repo. Source-side files we don't recognize are never touched.
  • Idempotent: second run sees a populated target and bails.
  • Non-destructive on collision: if a runtime dir already exists at the
    target, skip that dir (don't clobber). Caller decides which side wins.
  • Logged to stderr once per migration so operators see what moved.

Tests added (5 new, total 12 pass):
  • Migration moves runtime dirs when legacy has task-* files
  • Migration skips when legacy has no task-* (fresh checkout case)
  • Migration skips when target already has content (don't clobber)
  • Migration idempotent on second run
  • resolve_workspace with env set bypasses migration entirely

Also exposed a `migrate=False` kwarg on resolve_workspace() for tests that
want pure resolution semantics without filesystem side effects — existing
resolution tests updated accordingly.

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

* fixup(discord-bridge): resolve dm-result.py relative to bridge script (was REPO/src)

After PR #762's workspace-default change, REPO points at the runtime workspace
(e.g. ~/.sutando/workspace/ or $SUTANDO_WORKSPACE), NOT the repo root. The
dm-fallback path was invoking `REPO / "src" / "dm-result.py"` which now resolves
to `<workspace>/src/dm-result.py` — doesn't exist, the subprocess errored out
silently, owner DMs intended for the dm-fallback path stranded in `results/`.

dm-result.py is a code artifact (sibling of this script in src/), not a workspace
artifact, so resolving via `Path(__file__).resolve().parent` keeps the invocation
correct regardless of where the workspace lives. Matches the same pattern this
file already uses for `sys.path.insert` and the `workspace_default` import.

Caught live during the workspace migration this session — bridge log showed:
  [dm-fallback] dm-result.py failed on task-1778941463145.txt:
    can't open file '/Users/qingyunwu/Documents/github/sutando/workspace/src/dm-result.py'

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

* fixup(discord-bridge): self-rescue when launched with wrong python interpreter

Belt-and-braces defense against the bug class that has killed Discord twice
in this session and at least once yesterday: `python3 src/discord-bridge.py`
where $PATH resolves `python3` to an interpreter without discord.py
(miniconda is the chronic culprit on this Mac). Every prior fix patched the
specific launcher; the next launcher (cron, plist, shell script written 6
months from now) silently regresses it.

**At the top of discord-bridge.py** — if `import discord` raises
ModuleNotFoundError, probe a fixed list of candidates (`/opt/homebrew/bin/
python3`, `/usr/local/bin/python3`, versioned Homebrew Pythons) for one that
has discord.py and `os.execv` into it. Verified locally:
  • miniconda → detected, re-execs to /opt/homebrew/bin/python3 ✓
  • brew (has discord.py) → silent no-op ✓
If no candidate works, re-raise so the operator sees the real error.

**In startup.sh** — probe the same list at launcher time so the first
process is already the right one (no rescue dance, no wasted process,
cleaner log). Same fallback as the bridge's self-rescue.

**Why "Discord-is-important" deserves this**: the prior posture was "the
bridge crashes, the operator (or a respawn shim) restarts it, hopefully
that restart hits the right python." That's reactive and lossy — messages
land in `tasks/` but no `Replied:` ever follows, the owner doesn't know
delivery is broken until they ask. The self-rescue makes the bridge
self-healing within one process start; the startup.sh fix makes that
rescue rarely trigger in normal operation. Both layers needed because
neither is sufficient alone (startup.sh fix doesn't help with future
launchers; self-rescue alone wastes a process every spawn).

Caught this session: bridge died at ~22:08 yesterday (startup.sh→miniconda),
restarted by me at 07:35 PT with brew python, died again sometime between
07:50 and 08:23 (mechanism unclear — possibly an autorespawn from somewhere
that used plain python3). Hard to find the exact respawner; easier to make
both ends defensible.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qingyun-wu
qingyun-wu marked this pull request as draft May 17, 2026 05:29

@vidhuUC vidhuUC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

APPROVE

Clean promotion of owner WIP into tracked code. The DeviceSession wiring is minimal and correct — POST to /vision/register on connect, unregister on teardown, graceful 404 fallback for pre-#739 voice-agent. 320/322 lines unchanged from owner's working tree is reassuring.

The TODOs in the header are honest and well-scoped — access control, OC protocol migration, HUD routing via device profile, skill manifest. These are the right follow-ups in the right order.

One note: the implicit owner-trust for any paired Mentra session is called out as a TODO. Until that's addressed, this bridge should only be used in a single-owner setup (which is the current reality anyway). Acceptable for now.

Depends on #739. Merge after #739.

@qingyun-wu qingyun-wu closed this May 19, 2026
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
* feat(workspace): canonical default = ~/Library/Application Support/sutando/workspace

Add `src/workspace_default.py` helper that resolves SUTANDO_WORKSPACE → falls
back to `~/Library/Application Support/sutando/workspace/` (a subdir, not the
parent — the parent is Sutando.app's territory for its Chromium-style cache:
Cache/, GPUCache/, Cookies/, blob_storage/, etc.).

Replace the historic per-file fallback (`Path(__file__).resolve().parent.parent`)
in three callers — discord-bridge, telegram-bridge, dm-result — which resolved
to the script's repo root and (a) polluted git status with runtime artifacts on
bare-shell launches that forgot SUTANDO_WORKSPACE, and (b) when invoked from an
app-bundled src/ symlink, walked into the bundle so the bridge wrote tasks to
bundle-tasks/ while the watcher polled workspace-tasks/.

This is the same class of bug that just stranded three owner DMs at the old
Application Support path while the session watched the repo's `tasks/` — single
source of truth across components prevents the entire bug pattern.

Also add CLAUDE.md "Workspace contract" section codifying the convention.

Not in scope:
- mentra-bridge.ts (untracked WIP — tracked by PR sonichi#740; follow-up after merge)
- conversation-server.ts, web-client.ts (already merged with their own pattern)
- sync-memory.sh, session-handoff.sh (use SUTANDO_WORKSPACE for "repo path",
  a different semantic — separate decision)
- util_paths.py REPO_DIR (private-dir resolution; separate concern)

Tests: `tests/workspace-default.test.py` — 7 cases covering default location,
env override, ~-expansion, empty-string + whitespace fallbacks, and an explicit
anti-regression check that the default is NOT the repo root.

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

* fixup: default workspace = ~/.sutando/workspace/ (was App Support); parents=True for mkdir

Two CI-fail-driven adjustments:

1. **Default path change.** v1 defaulted to `~/Library/Application Support/sutando/
   workspace/` (macOS-canonical). On Linux CI runners that path is nonsensical
   (`/home/runner/Library/Application Support/...` makes no semantic sense, even
   though the filesystem accepts it). Per owner: switch to `~/.sutando/workspace/`
   — hidden, OS-neutral, home-relative. Same intent (avoid Sutando.app's
   Application Support territory + give workspace a dedicated dir) without
   pretending to be macOS-shaped on non-macOS hosts.

2. **mkdir(parents=True).** v1's bridge module-load code did `TASKS_DIR.mkdir
   (exist_ok=True)` without `parents=True`, which is fine when the parent
   directory tree already exists but raises `FileNotFoundError` on a fresh
   install or a Linux CI runner where `~/.sutando/workspace/` doesn't exist yet.
   Now `parents=True` everywhere the bridge auto-creates a dir from REPO
   (TASKS_DIR, RESULTS_DIR, STATE_DIR — both bridges).

Updated CLAUDE.md "Workspace contract" section + the Python helper docstring
to reflect the new default. Test `test_default_is_application_support_workspace_subdir`
renamed to `test_default_is_dot_sutando_workspace_under_home` with matching
assertions. All 7 helper tests green; the previously-failing CI test
(`discord-bridge-discord-state-detection`) reproduces green locally with
`SUTANDO_WORKSPACE=""`.

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

* fixup: auto-migrate runtime dirs from legacy repo-root fallback to ~/.sutando/workspace/

Addresses @sonichi's PR sonichi#762 review observation 1 — breaking change for nodes
that ran against the historic `Path(__file__).resolve().parent.parent` fallback
(e.g. MacBook with state in `~/Desktop/sutando/tasks/`). Without this fix, those
nodes would point at an empty new default post-restart and lose continuity until
the operator manually re-pointed SUTANDO_WORKSPACE or rsync'd state.

Migration semantics:
  • Fires only when SUTANDO_WORKSPACE is unset, the new default doesn't yet
    exist (or is empty), AND the legacy repo root has runtime evidence
    (task-* files in tasks/results/state/). Three-condition gate keeps env-
    pinned installs and fresh-clone CI runs untouched.
  • Moves tasks/, results/, state/ (the three runtime-state dirs) — not the
    whole repo. Source-side files we don't recognize are never touched.
  • Idempotent: second run sees a populated target and bails.
  • Non-destructive on collision: if a runtime dir already exists at the
    target, skip that dir (don't clobber). Caller decides which side wins.
  • Logged to stderr once per migration so operators see what moved.

Tests added (5 new, total 12 pass):
  • Migration moves runtime dirs when legacy has task-* files
  • Migration skips when legacy has no task-* (fresh checkout case)
  • Migration skips when target already has content (don't clobber)
  • Migration idempotent on second run
  • resolve_workspace with env set bypasses migration entirely

Also exposed a `migrate=False` kwarg on resolve_workspace() for tests that
want pure resolution semantics without filesystem side effects — existing
resolution tests updated accordingly.

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

* fixup(discord-bridge): resolve dm-result.py relative to bridge script (was REPO/src)

After PR sonichi#762's workspace-default change, REPO points at the runtime workspace
(e.g. ~/.sutando/workspace/ or $SUTANDO_WORKSPACE), NOT the repo root. The
dm-fallback path was invoking `REPO / "src" / "dm-result.py"` which now resolves
to `<workspace>/src/dm-result.py` — doesn't exist, the subprocess errored out
silently, owner DMs intended for the dm-fallback path stranded in `results/`.

dm-result.py is a code artifact (sibling of this script in src/), not a workspace
artifact, so resolving via `Path(__file__).resolve().parent` keeps the invocation
correct regardless of where the workspace lives. Matches the same pattern this
file already uses for `sys.path.insert` and the `workspace_default` import.

Caught live during the workspace migration this session — bridge log showed:
  [dm-fallback] dm-result.py failed on task-1778941463145.txt:
    can't open file '/Users/qingyunwu/Documents/github/sutando/workspace/src/dm-result.py'

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

* fixup(discord-bridge): self-rescue when launched with wrong python interpreter

Belt-and-braces defense against the bug class that has killed Discord twice
in this session and at least once yesterday: `python3 src/discord-bridge.py`
where $PATH resolves `python3` to an interpreter without discord.py
(miniconda is the chronic culprit on this Mac). Every prior fix patched the
specific launcher; the next launcher (cron, plist, shell script written 6
months from now) silently regresses it.

**At the top of discord-bridge.py** — if `import discord` raises
ModuleNotFoundError, probe a fixed list of candidates (`/opt/homebrew/bin/
python3`, `/usr/local/bin/python3`, versioned Homebrew Pythons) for one that
has discord.py and `os.execv` into it. Verified locally:
  • miniconda → detected, re-execs to /opt/homebrew/bin/python3 ✓
  • brew (has discord.py) → silent no-op ✓
If no candidate works, re-raise so the operator sees the real error.

**In startup.sh** — probe the same list at launcher time so the first
process is already the right one (no rescue dance, no wasted process,
cleaner log). Same fallback as the bridge's self-rescue.

**Why "Discord-is-important" deserves this**: the prior posture was "the
bridge crashes, the operator (or a respawn shim) restarts it, hopefully
that restart hits the right python." That's reactive and lossy — messages
land in `tasks/` but no `Replied:` ever follows, the owner doesn't know
delivery is broken until they ask. The self-rescue makes the bridge
self-healing within one process start; the startup.sh fix makes that
rescue rarely trigger in normal operation. Both layers needed because
neither is sufficient alone (startup.sh fix doesn't help with future
launchers; self-rescue alone wastes a process every spawn).

Caught this session: bridge died at ~22:08 yesterday (startup.sh→miniconda),
restarted by me at 07:35 PT with brew python, died again sometime between
07:50 and 08:23 (mechanism unclear — possibly an autorespawn from somewhere
that used plain python3). Hard to find the exact respawner; easier to make
both ends defensible.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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