Skip to content

open_file: return immediately with best-available recording (fix 18s timeout) - #353

Merged
sonichi merged 1 commit into
mainfrom
fix/open-file-nonblocking
Apr 15, 2026
Merged

open_file: return immediately with best-available recording (fix 18s timeout)#353
sonichi merged 1 commit into
mainfrom
fix/open-file-nonblocking

Conversation

@sonichi

@sonichi sonichi commented Apr 15, 2026 •
edited by liususan091219
Loading

Copy link
Copy Markdown
Owner

Summary

Fixes the root cause of "I couldn't find the recording at the standard location" on phone calls. The 18s polling loop waiting for the subtitled burn-in exceeded Gemini Live's tool-call timeout — the tool got cancelled mid-retry and the model reported a false negative even while the narrated file was sitting on disk.

  • Drops the 10-iteration retry loop.
  • findRecording() already returns best-available in priority order (subtitled > narrated > raw), so one synchronous call is enough.
  • Adds a subtitled_pending flag + a version field to the return payload.
  • When subtitled is pending, the returned instruction tells the model to proactively say: "I opened the narrated version. Subtitles are still being generated — want me to switch to the subtitled version when it's ready?" If the user says yes, the model waits ~30 seconds and calls open_file again, which picks up the subtitled version once the burn-in finishes.

What this does NOT do (yet)

The async-notification half of the owner's ask ("can we make the wait async?") — i.e., voice agent proactively tells user when subtitled is ready without the user asking. That's a bigger feature: needs a mid-call signal channel to voice-agent, plus background polling with dedicated state. Post-flood I'm cautious about any results/ iteration path, so I'm pausing that work and flagging it for a design conversation before shipping.

This PR gives the user-driven retry pattern — eliminates the timeout failure, restores a workable UX immediately.

Test plan

  • npx tsc --noEmit --skipLibCheck clean
  • Manual: restart voice-agent, start a recording, ask "open it" — tool returns within ~100ms instead of 18s, model speaks the pending message
  • Manual: call open_file again 30s later once subtitled is on disk — tool returns subtitled version

Diff

  • src/recording-tools.ts +33/-13

References

  • Phone-call diagnosis (with 18s timeout root cause): Discord thread 22:42 local
  • Owner directive: "let users know the subtitled file is not ready and ask them whether they want to wait" (Discord 22:02 local)
  • Prior meeting notes that captured this as a latent bug: notes/meetings/task-summary-1776292611357.md

🤖 Generated with Claude Code

Fixes #356

Chi @claude Chi
open_file: return immediately with best-available recording
c888e72 
Fixes the root cause of "I couldn't find the recording at the standard
location" during phone calls that owner diagnosed earlier today. The
18-second polling loop waiting for the subtitled version exceeded
Gemini Live's tool-call timeout, so the tool got cancelled mid-retry
and the model reported a false negative even when the narrated file
was on disk.

### What changes

- Remove the 10-iteration retry loop. `findRecording()` already
  returns the best-available version in priority order (subtitled >
  narrated > raw), so one synchronous call is enough.
- Compute a `subtitled_pending` flag when the returned version is not
  already subtitled AND it's a sutando recording (i.e. the background
  subtitle burn might still be running).
- When `subtitled_pending` is true, the returned `instruction` string
  tells the model to proactively inform the user: "I opened the
  narrated version. Subtitles are still being generated — want me to
  switch to the subtitled version when it's ready?" If the user says
  yes, the model can wait ~30 seconds and call open_file again, which
  will pick up the subtitled version once the burn-in finishes.
- Return payload gains a `version` field (`subtitled | narrated | raw`)
  so the model knows exactly what it's telling the user.

### What this doesn't do yet

**Async notification**: owner also asked "can we make the wait async?"
i.e., can the voice agent proactively tell the user when subtitled is
ready without the user having to ask. That's a bigger feature — it
needs a signal channel to voice-agent mid-call (post-flood we're
cautious about the results/ iteration path), plus background polling
with a dedicated state file. Pausing that work here and asking owner
for a design discussion first.

The current PR gives owner the non-blocking behavior + "user-driven
retry" pattern, which eliminates the timeout failure and ships a
workable UX immediately.

### Test plan

- [x] `npx tsc --noEmit --skipLibCheck` clean
- [ ] Manual: restart voice-agent, start a recording, ask "open it" —
      tool returns within ~100ms instead of 18s, model speaks the
      subtitled_pending message if applicable
- [ ] Manual: call open_file again 30s later with subtitled now on
      disk — tool returns subtitled version

### References

- Phone call diagnosis: `notes/meetings/task-summary-1776289358782.md`,
  `notes/meetings/task-summary-1776292611357.md`
- Reply with diagnosis: in result text at 22:42 local
- Owner design directive: "let users know the subtitled file is not
  ready and ask them whether they want to wait; if they do, can we
  make the wait async?" (via Discord 22:02 local)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sonichi
sonichi commented Apr 15, 2026
View reviewed changes

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

MacBook review: LGTM. Clean fix — removes the 18s blocking poll, returns immediately with best-available version, flags subtitled_pending for model-driven retry. The retry is now in the prompt instruction, not a blocking loop. Well within Gemini's tool-call timeout. No regressions to normal open_file path.

@sonichi
sonichi merged commit 2be13be into main Apr 15, 2026
1 check passed
@sonichi
sonichi deleted the fix/open-file-nonblocking branch April 15, 2026 23:16
@liususan091219 liususan091219 mentioned this pull request Apr 16, 2026
Closed
liususan091219 added a commit that referenced this pull request Apr 16, 2026
@liususan091219 @claude
test: add POC verification scripts for fix PRs #353, #355, #332, #325, …
20260ea 
…#354

Each script reproduces the bug (before the fix) and verifies it's resolved
(after the fix). All POCs pass on current main.

- poc-pr353-open-file.sh (11/11) — 18s polling timeout in open_file
- poc-pr355-subtitled-pending.sh (9/9) — false positive subtitled_pending
- poc-pr332-team-tier-revert.sh (9/9) — team-tier -C /tmp broke codex
- poc-pr325-bodhi-dep.sh (7/7) — bodhi dep pointed at deleted repo
- poc-pr354-retention-sweep.sh — retention sweep for stale results

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@liususan091219 liususan091219 mentioned this pull request Apr 16, 2026
Closed
2 tasks done
@liususan091219

liususan091219 commented Apr 16, 2026

Copy link
Copy Markdown
Collaborator

POC: bash scripts/poc-pr353-open-file.sh (11/11 pass). Script in PR #358. Issue: #356

@liususan091219 liususan091219 mentioned this pull request Apr 16, 2026
Closed
sonichi pushed a commit that referenced this pull request Apr 16, 2026
Chi @claude Chi
refactor: decouple open_file from recording logic
51e3678 
Per owner's design directive: "The recording tool should return the
files and the default file to use. The caller should process the result
and report abnormality if any. Then open_file tool should be called with
filename."

Changes:

1. open_file: stripped recording-specific logic.
   - Requires `path` arg (get it from recording tool result).
   - `find_latest_recording: true` flag as explicit fallback — only
     called when model lost the path, not as silent default.
   - Removed: subtitled_pending flag, version detection, polling
     logic, findRecording() as default behavior.
   - Kept: known file aliases (diagnostics), QuickTime activation,
     playback-path write, duration/size metadata.

2. screen_record stop: returns explicit file list.
   - New `files` object: `{raw, narrated, subtitled, recommended}`.
   - `instruction` string tells the model exactly which path to pass
     to open_file and how to offer alternatives to the user.
   - Subtitle burn still happens synchronously on stop.

3. No changes to record_screen_with_narration — its auto-stop timer
   fires in a setTimeout and can't return files to the model through
   a tool result. The model already has the raw path from the start
   call; open_file with that path works. Full file-list return for
   record_screen_with_narration is a follow-up.

Net: +39/-38 (near-zero). This is a separation of concerns, not new
features. findRecording() and findFfmpegWithSubtitles() are preserved
as helpers; they just stop being called by default in open_file.

Supersedes the approach in #353 (non-blocking return + subtitled_pending)
and makes #355 (subtitled_pending gate) + #357 (ffmpeg-full path)
unnecessary as open_file changes — though #357's findFfmpegWithSubtitles
is still needed in the recording tool's burn path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sonichi added a commit that referenced this pull request Apr 16, 2026
@sonichi Chi @claude
refactor: decouple open_file from recording logic (#392)
1b016ea 
Per owner's design directive: "The recording tool should return the
files and the default file to use. The caller should process the result
and report abnormality if any. Then open_file tool should be called with
filename."

Changes:

1. open_file: stripped recording-specific logic.
   - Requires `path` arg (get it from recording tool result).
   - `find_latest_recording: true` flag as explicit fallback — only
     called when model lost the path, not as silent default.
   - Removed: subtitled_pending flag, version detection, polling
     logic, findRecording() as default behavior.
   - Kept: known file aliases (diagnostics), QuickTime activation,
     playback-path write, duration/size metadata.

2. screen_record stop: returns explicit file list.
   - New `files` object: `{raw, narrated, subtitled, recommended}`.
   - `instruction` string tells the model exactly which path to pass
     to open_file and how to offer alternatives to the user.
   - Subtitle burn still happens synchronously on stop.

3. No changes to record_screen_with_narration — its auto-stop timer
   fires in a setTimeout and can't return files to the model through
   a tool result. The model already has the raw path from the start
   call; open_file with that path works. Full file-list return for
   record_screen_with_narration is a follow-up.

Net: +39/-38 (near-zero). This is a separation of concerns, not new
features. findRecording() and findFfmpegWithSubtitles() are preserved
as helpers; they just stop being called by default in open_file.

Supersedes the approach in #353 (non-blocking return + subtitled_pending)
and makes #355 (subtitled_pending gate) + #357 (ffmpeg-full path)
unnecessary as open_file changes — though #357's findFfmpegWithSubtitles
is still needed in the recording tool's burn path.

Co-authored-by: Chi <wangchi@Chis-Mac-mini.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 28, 2026
Closed
Merged
liususan091219 added a commit that referenced this pull request Apr 30, 2026
@liususan091219 Xueqing Liu @claude
fix(tools): dedup + open-file generic + summon/recording reliability …
033f6cc 
…sweep (#546)

* fix(open-file): move openFileTool to inline-tools, decouple from recording

`openFileTool` lived in `src/recording-tools.ts` since #274 with
recording-flavored helpers (`findRecording()` fallback, `sutando-recording`
substring check, `/tmp/sutando-playback-path` write, `demoStateRef.value
= 'idle'` reset, ffprobe duration probe) coupled into the function body.
PR #353 introduced most of those; PR #392 cleaned the worst (made
`find_recording=true` opt-in), but kept the helpers as deliberate residuals.
This commit removes them entirely and moves the function to a generic
home.

Changes:

1. **Move `openFileTool` to `src/inline-tools.ts`.** Required `path` only.
   Stripped: `find_recording`, `findRecording()` fallback,
   `sutando-recording` substring write, `demoStateRef` reset,
   `recPath` rename → `filePath`, "play_video" instruction string,
   ffprobe duration extraction.

2. **Keep `fullscreen=true` flow in `openFileTool`** (per Chi's confirmation
   in #dev — fullscreen-on-open is general video-fullscreen UX, not
   recording-specific). The QuickTime force-open + present-mode
   AppleScript + Zoom-survives keystroke routing all stay. The runtime
   guard ("don't fullscreen=true when QT already has docs") stays.

3. **Re-export hygiene:**
   - `inline-tools.ts` line-17 re-export and line-18 import: dropped
     `openFileTool` (now defined locally in this file).
   - `browser-tools.ts` recording-tools re-export block: dropped
     `openFileTool`.
   - `recording-tools.ts`: removed definition + the misleading
     "polymorphic playRecordingTool" comment block above it. One-line
     marker pointing to the new location.

4. **Recording-flavored side effects relocated to recording-tools:**
   - `/tmp/sutando-playback-path` write moved to `screenRecordTool`
     stop handler AND its auto-stop `setTimeout` callback. Recording
     tool now stamps the playback hint itself when the recording
     finishes — no model courier through `open_file` needed.
   - ffprobe duration extraction moved to `screenRecordTool` stop
     result (`duration_seconds` field).
   - Stop handler instruction updated to mention both
     `open_file(path)` and `open_file(path, fullscreen=true)` so the
     model knows how to invoke each path of the demo flow.

README demo flow (`screen_record → open_file(path) → play_video`)
continues to work end-to-end: `screen_record stop` writes
`/tmp/sutando-playback-path` proactively, and `play_video`'s existing
read of that path (`startPlayback()` line ~454) is unchanged.

Refs: #274 (initial split), #315 (rename), #353 (introduced
coupling), #392 (partial cleanup, kept residuals), #408/#516/#520/#524/#526
(fullscreen + QuickTime fixes — fullscreen=true preserved as general UX).

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

* fix(tools): deduplicate open_url/capture_screen/type_text across browser-tools/inline-tools

Three tool names had two registered definitions (one in src/browser-tools.ts
and one in src/inline-tools.ts), which both inlineTools and ownerOnlyTools
arrays then picked up. Gemini Live rejects setup payloads with duplicate
function names (close 1011 "Internal error") — see
reference_gemini_1011_tool_name_conflict.md.

Canonical placement per layer responsibility:
- open_url      → browser-tools.ts  (browser-side action)
- capture_screen → inline-tools.ts  (inline screen capture)
- type_text     → inline-tools.ts  (inline keystroke / clipboard paste)

inline-tools.ts now re-exports openUrlTool from browser-tools so consumers
keep importing from inline-tools unchanged. Removed the duplicate
definitions on the non-canonical side; left a one-line marker comment so
greps for the old name still land somewhere meaningful.

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

* fix(tools): remove dead cancel_task duplicate from task-bridge

`cancelTask` was defined in src/task-bridge.ts:171 (Chi, commit bbc2631
2026-03-28) AND `cancelTaskTool` in src/inline-tools.ts:404. The
inline-tools version is the one registered with Gemini; the
task-bridge export was imported by src/voice-agent.ts:39 but never
referenced anywhere — dead import.

Confirmed safe to delete via independent grep:
- 0 call sites in src/
- 0 test references
- only consumer was the dead import

Removes the third source of the duplicate-tool-name 1011 setup error
documented in reference_gemini_1011_tool_name_conflict.md.

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

* fix(summon): use zoommtg:// deeplink + tighten meeting-window readiness check

Two bugs caused phone summon on Studio to log "Screen share started" while
nothing actually shared:

1. **HTTPS join URL unreliable on Zoom Workplace.** The `zoomRunning=true`
   branch was using `https://zoom.us/j/{id}?pwd=...`, which routes through
   the browser and doesn't reliably hand off to the in-app meeting on Zoom
   Workplace (Studio). Result: Zoom stayed on its home dashboard, no
   meeting joined. Replaced with the `zoommtg://` deeplink for both
   running-and-not-running cases — direct to the Zoom app, no browser hop.

2. **Readiness check counted any window.** The old check waited for `(count
   of windows of process "zoom.us") > 0`, which the always-on "Zoom
   Workplace" home dashboard already satisfies. So even with no meeting
   ever opened, the check passed, the share-screen menu fired into an
   inapplicable context, the AppleScript silently no-op'd, and the catch
   block still logged success. Now checks window names for `Zoom Meeting`
   / `zoom share` / `floating video` — matches `alreadyInMeeting`'s
   detection.

Verified 2026-04-29 phone summon on Studio (`/tmp/conversation-server.log`
15:54:27–15:54:36) showed the false-positive log; instrumented a11y dump
confirmed only "Zoom Workplace" was open after the supposed join.

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

* fix(summon): click Join by a11y description; tighten host-joined check

Phone summon on Studio kept missing the actual Join button: the cliclick
coordinate (`preview.x + 0.80*w, preview.y + 0.93*h`) didn't land on the
button on Zoom Workplace's preview layout. The meeting never opened,
the host-joined check then matched "Personal Meeting Room" (a preview
window!) on `winNames.includes('Meeting')` and reported `Host joined: true`
falsely, so the phone dial-in path fired anyway, and the new strict
share-readiness gate timed out 30s later. Net: `summon` returned error
30002ms after a chain of internal false positives.

Two fixes:

1. **Join click via a11y description, not coordinates.** Iterate `buttons of
   w whose description is "Join"` across all zoom.us windows, click the
   first one. The Join button's a11y description is stable across Zoom
   Workplace layouts (verified by direct dump 2026-04-29).

2. **Host-joined check uses the strict meeting-window set.** Same
   `Zoom Meeting | zoom share | floating video` set as the share-readiness
   gate. Plain `Meeting` was matching preview windows.

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

* fix(summon): bump tool timeout to 60s — Studio Zoom takes ~30s to open meeting

Phone summon test #4 (2026-04-29 16:06): a11y Join click landed correctly,
but the meeting window took 31s to appear after the click on Studio Zoom
Workplace. Bodhi's default tool timeout is 30s, so the tool returned
error 30001ms BEFORE the share-menu fired. The share itself eventually
fired (logged 16:06:57) but by then the assistant had already told the
caller "I'm having some trouble summoning."

`60_000` covers the worst-case meeting-open + share-menu + mute AppleScripts.

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

* fix(record-with-narration): write playback-path on auto-stop so play_video can find the file

After PR #546 made open_file generic (no findRecording fallback), there
was a behavioral gap: record_screen_with_narration's auto-stop didn't
persist a playback-path hint, while screen_record stop did. Result on
2026-04-29 phone test: user said "open the recording" → model called
open_file with no path → got generic "no path provided" error → user
gave up.

This commit closes the gap WITHOUT re-coupling open_file to recording:
- record_screen_with_narration auto-stop now writes /tmp/sutando-playback-path
  with the best-available file (subtitled > narrated > raw), matching
  screen_record stop's existing pattern.
- Tool description appends "After auto-stop, to play back or open the
  recording, call play_video — it auto-finds the file. Do NOT call
  open_file unless you have the absolute path." to nudge the model
  toward the correct tool.

open_file remains general-purpose; recording-tools owns the playback-hint
write. play_video already reads /tmp/sutando-playback-path with a
findRecording fallback.

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

* fix(record-with-narration): return predicted recording paths so model can pass to open_file

The 2026-04-29 phone-summon test still failed on "open the recorded video"
even after `19ecfb8` wrote `/tmp/sutando-playback-path` on auto-stop —
because the model called `open_file` (the natural verb match for "open"),
not `play_video`, and `open_file` is generic and needs a path arg.

Now `record_screen_with_narration` captures `record.py start`'s JSON output
to extract the absolute `recording_path` and computes the
`narrated_path = path.replace('.mov', '-narrated.mov')`. Both paths
are returned in the start result + spelled out in the `message` field
("After auto-stop, the file will be at ... — pass that exact path to
open_file when the user asks to open the recording.").

The model now has the path inline from the moment recording starts and can
pass it to open_file (or any other path-taking tool) without guessing. No
recording-specific code in open_file. No reliance on the model picking up
the prior tool description hint.

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

* fix(record-with-narration): return subtitled_path so model can open the burned-in version

After 35cce47 returned `recording_path` + `narrated_path`, the model still
couldn't open the subtitled version because it didn't know that path. The
2026-04-29 21:00 phone test had subtitle burn succeed (file at
/tmp/sutando-recording-X-narrated-subtitled.mov) but the model called
open_file three times with narrated/raw paths only — Susan kept saying
"this is not the one with the subtitle".

Now the start result includes `subtitled_path` and the message string
spells out the priority order: pass subtitled_path to open_file when the
user asks for "the recording" / "with subtitles". Fall back to
narrated_path only when subtitled burn failed (e.g., libass missing,
which is now known-fixed via the homebrew-ffmpeg/ffmpeg tap).

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

* fix(recording/open-file): clarify subtitle support + nudge model away from path hallucination

Two regressions from phone summon test #7 (CA5c9c0a711cfcacc2cc5458c1d5eca04d
21:09 ET):

1. **Model hallucinated "I can't add subtitles"** — record_screen_with_narration's
   tool description didn't mention that subtitle burn happens automatically when
   the live transcript exists. User said "add subtitle", model refused, even
   though the auto-stop callback ALWAYS calls burnLiveTranscriptSubtitles when
   liveTranscriptRecordingStart > 0. Description now states subtitles ARE always
   burned in — never refuse a "with subtitles" request.

2. **Model hallucinated path `/tmp/screen_recording_with_narration.mov`** when
   asked to open the recording — invented a generic name instead of using the
   `subtitled_path` returned by start. Two-pronged nudge:
   - Recording-tools description: "Do NOT invent file paths — only use the exact
     paths returned by this tool" + clarifies subtitled_path is the right one
     for "with subtitles".
   - open_file's `File not found` error explicitly says "Do not invent paths"
     and routes to play_video for unknown-path recording-open.

`open_file` itself remains generic — only its error string text changed, no
recording-specific code. play_video already has the findRecording fallback.

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

* fix(open-file): strip ALL video-related logic — back to truly generic open

Per Susan: open_file should not contain any video-specific behavior. The
fullscreen=true flow forced QuickTime, ran a present-mode AppleScript with
30-iteration polling, and had a guard against re-opening when QT already
had documents. All video-specific.

Removed:
- `fullscreen` parameter
- QT force-open via `open -a "QuickTime Player"`
- Path-matched present-mode AppleScript (Cmd+Ctrl+F)
- QT-already-has-doc guard
- Description text about video presentation, .mp4/.mov, "fullscreen on a
  video that is already open"

Now: `execFileSync('open', [filePath])` — LaunchServices picks the user's
default app for that file type. Mirrors what a human running `open foo.x`
in a terminal would get.

For fullscreen on a just-opened video, the model should call the existing
`fullscreen` tool as a separate step. Two-tool sequence is simpler than
the polled AppleScript embedded in open_file.

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

* fix: address Mini PR #546 review — summon retry/return, generic fullscreen, soften subtitle claim

Mini's review on PR #546:

**1. summon Join click + return values (Medium)** — the prior split (one-shot
Join + 30s readiness) let a late preview-window race leave the meeting
un-joined while readiness ran out the clock; the tool then still returned
"summoned" / "joined with screen sharing". Now: combined 60s loop where
each iteration first checks for a real meeting window (Zoom Meeting / zoom
share / floating video), then if not present retries the a11y Join click.
Track `zoomReady` and `shareStarted` accurately and surface them in the
return payload + status string (`summoned` | `joined_share_failed` |
`meeting_window_not_detected`). instruction text reflects what actually
happened.

**2. summon timeout (Medium)** — 60s was still tight per Mini's worst-case
calc (Join 10s + readiness 60s + share 15s + mute/cleanup 10s + dial-in
host wait 20s ≈ 115s). Bumped to 120s.

**3. record_screen_with_narration subtitle claim (Low)** — softened "IS
automatically burned" to "are attempted automatically when both transcript
text and a libass-capable ffmpeg are present". Burn can silently skip on
missing transcript / unreadable narrated / missing libass; honesty matters
more than over-promising. Description still says "never refuse a 'with
subtitles' request" so the model doesn't gate refusal on apparent failure.

**Bonus (Susan):** open_file gets a *generic* `fullscreen` parameter
back — but app-agnostic this time. Just `open <path>` (LaunchServices
routes to default app) → 1.5s settle → System Events Cmd+Ctrl+F. Works for
video in QuickTime, PDF in Preview, web pages in Chrome — no QT
hardcoding, no path-match polling. Replaces the QT-specific present-mode
flow with a one-keystroke generic toggle.

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

* fix(summon): address Mini PR #546 round-2 — dialIn order + 1-line instruction fix

Medium #1 (code change): dialIn previously ran a separate host-joined wait
BEFORE the readiness/Join-retry loop. Preview-still-up race made dialIn
fire against an unjoined meeting. Restructured: readiness/Join-retry loop
runs FIRST and gates the dialIn block on zoomReady. Removed redundant
standalone host-joined wait.

Low #2 (text only): instruction in phoneJoined branch falsely claimed
"Screen is shared..." when shareStarted=false. One-line fix:
`(shareStarted ? 'Screen is shared and ' : '')` prefix.

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

---------

Co-authored-by: Xueqing Liu <xueqingliu@Xueqings-Mac-Studio.local>
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
@sonichi Chi @claude
open_file: return immediately with best-available recording (sonichi#353
badccee 
)

Fixes the root cause of "I couldn't find the recording at the standard
location" during phone calls that owner diagnosed earlier today. The
18-second polling loop waiting for the subtitled version exceeded
Gemini Live's tool-call timeout, so the tool got cancelled mid-retry
and the model reported a false negative even when the narrated file
was on disk.

### What changes

- Remove the 10-iteration retry loop. `findRecording()` already
  returns the best-available version in priority order (subtitled >
  narrated > raw), so one synchronous call is enough.
- Compute a `subtitled_pending` flag when the returned version is not
  already subtitled AND it's a sutando recording (i.e. the background
  subtitle burn might still be running).
- When `subtitled_pending` is true, the returned `instruction` string
  tells the model to proactively inform the user: "I opened the
  narrated version. Subtitles are still being generated — want me to
  switch to the subtitled version when it's ready?" If the user says
  yes, the model can wait ~30 seconds and call open_file again, which
  will pick up the subtitled version once the burn-in finishes.
- Return payload gains a `version` field (`subtitled | narrated | raw`)
  so the model knows exactly what it's telling the user.

### What this doesn't do yet

**Async notification**: owner also asked "can we make the wait async?"
i.e., can the voice agent proactively tell the user when subtitled is
ready without the user having to ask. That's a bigger feature — it
needs a signal channel to voice-agent mid-call (post-flood we're
cautious about the results/ iteration path), plus background polling
with a dedicated state file. Pausing that work here and asking owner
for a design discussion first.

The current PR gives owner the non-blocking behavior + "user-driven
retry" pattern, which eliminates the timeout failure and ships a
workable UX immediately.

### Test plan

- [x] `npx tsc --noEmit --skipLibCheck` clean
- [ ] Manual: restart voice-agent, start a recording, ask "open it" —
      tool returns within ~100ms instead of 18s, model speaks the
      subtitled_pending message if applicable
- [ ] Manual: call open_file again 30s later with subtitled now on
      disk — tool returns subtitled version

### References

- Phone call diagnosis: `notes/meetings/task-summary-1776289358782.md`,
  `notes/meetings/task-summary-1776292611357.md`
- Reply with diagnosis: in result text at 22:42 local
- Owner design directive: "let users know the subtitled file is not
  ready and ask them whether they want to wait; if they do, can we
  make the wait async?" (via Discord 22:02 local)

Co-authored-by: Chi <wangchi@Chis-Mac-mini.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
@sonichi Chi @claude
refactor: decouple open_file from recording logic (sonichi#392)
64ddafa 
Per owner's design directive: "The recording tool should return the
files and the default file to use. The caller should process the result
and report abnormality if any. Then open_file tool should be called with
filename."

Changes:

1. open_file: stripped recording-specific logic.
   - Requires `path` arg (get it from recording tool result).
   - `find_latest_recording: true` flag as explicit fallback — only
     called when model lost the path, not as silent default.
   - Removed: subtitled_pending flag, version detection, polling
     logic, findRecording() as default behavior.
   - Kept: known file aliases (diagnostics), QuickTime activation,
     playback-path write, duration/size metadata.

2. screen_record stop: returns explicit file list.
   - New `files` object: `{raw, narrated, subtitled, recommended}`.
   - `instruction` string tells the model exactly which path to pass
     to open_file and how to offer alternatives to the user.
   - Subtitle burn still happens synchronously on stop.

3. No changes to record_screen_with_narration — its auto-stop timer
   fires in a setTimeout and can't return files to the model through
   a tool result. The model already has the raw path from the start
   call; open_file with that path works. Full file-list return for
   record_screen_with_narration is a follow-up.

Net: +39/-38 (near-zero). This is a separation of concerns, not new
features. findRecording() and findFfmpegWithSubtitles() are preserved
as helpers; they just stop being called by default in open_file.

Supersedes the approach in sonichi#353 (non-blocking return + subtitled_pending)
and makes sonichi#355 (subtitled_pending gate) + sonichi#357 (ffmpeg-full path)
unnecessary as open_file changes — though sonichi#357's findFfmpegWithSubtitles
is still needed in the recording tool's burn path.

Co-authored-by: Chi <wangchi@Chis-Mac-mini.local>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
liususan091219 added a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
@liususan091219 Xueqing Liu @claude
fix(tools): dedup + open-file generic + summon/recording reliability …
e4de248 
…sweep (sonichi#546)

* fix(open-file): move openFileTool to inline-tools, decouple from recording

`openFileTool` lived in `src/recording-tools.ts` since sonichi#274 with
recording-flavored helpers (`findRecording()` fallback, `sutando-recording`
substring check, `/tmp/sutando-playback-path` write, `demoStateRef.value
= 'idle'` reset, ffprobe duration probe) coupled into the function body.
PR sonichi#353 introduced most of those; PR sonichi#392 cleaned the worst (made
`find_recording=true` opt-in), but kept the helpers as deliberate residuals.
This commit removes them entirely and moves the function to a generic
home.

Changes:

1. **Move `openFileTool` to `src/inline-tools.ts`.** Required `path` only.
   Stripped: `find_recording`, `findRecording()` fallback,
   `sutando-recording` substring write, `demoStateRef` reset,
   `recPath` rename → `filePath`, "play_video" instruction string,
   ffprobe duration extraction.

2. **Keep `fullscreen=true` flow in `openFileTool`** (per Chi's confirmation
   in #dev — fullscreen-on-open is general video-fullscreen UX, not
   recording-specific). The QuickTime force-open + present-mode
   AppleScript + Zoom-survives keystroke routing all stay. The runtime
   guard ("don't fullscreen=true when QT already has docs") stays.

3. **Re-export hygiene:**
   - `inline-tools.ts` line-17 re-export and line-18 import: dropped
     `openFileTool` (now defined locally in this file).
   - `browser-tools.ts` recording-tools re-export block: dropped
     `openFileTool`.
   - `recording-tools.ts`: removed definition + the misleading
     "polymorphic playRecordingTool" comment block above it. One-line
     marker pointing to the new location.

4. **Recording-flavored side effects relocated to recording-tools:**
   - `/tmp/sutando-playback-path` write moved to `screenRecordTool`
     stop handler AND its auto-stop `setTimeout` callback. Recording
     tool now stamps the playback hint itself when the recording
     finishes — no model courier through `open_file` needed.
   - ffprobe duration extraction moved to `screenRecordTool` stop
     result (`duration_seconds` field).
   - Stop handler instruction updated to mention both
     `open_file(path)` and `open_file(path, fullscreen=true)` so the
     model knows how to invoke each path of the demo flow.

README demo flow (`screen_record → open_file(path) → play_video`)
continues to work end-to-end: `screen_record stop` writes
`/tmp/sutando-playback-path` proactively, and `play_video`'s existing
read of that path (`startPlayback()` line ~454) is unchanged.

Refs: sonichi#274 (initial split), sonichi#315 (rename), sonichi#353 (introduced
coupling), sonichi#392 (partial cleanup, kept residuals), sonichi#408/sonichi#516/sonichi#520/sonichi#524/sonichi#526
(fullscreen + QuickTime fixes — fullscreen=true preserved as general UX).

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

* fix(tools): deduplicate open_url/capture_screen/type_text across browser-tools/inline-tools

Three tool names had two registered definitions (one in src/browser-tools.ts
and one in src/inline-tools.ts), which both inlineTools and ownerOnlyTools
arrays then picked up. Gemini Live rejects setup payloads with duplicate
function names (close 1011 "Internal error") — see
reference_gemini_1011_tool_name_conflict.md.

Canonical placement per layer responsibility:
- open_url      → browser-tools.ts  (browser-side action)
- capture_screen → inline-tools.ts  (inline screen capture)
- type_text     → inline-tools.ts  (inline keystroke / clipboard paste)

inline-tools.ts now re-exports openUrlTool from browser-tools so consumers
keep importing from inline-tools unchanged. Removed the duplicate
definitions on the non-canonical side; left a one-line marker comment so
greps for the old name still land somewhere meaningful.

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

* fix(tools): remove dead cancel_task duplicate from task-bridge

`cancelTask` was defined in src/task-bridge.ts:171 (Chi, commit bbc2631
2026-03-28) AND `cancelTaskTool` in src/inline-tools.ts:404. The
inline-tools version is the one registered with Gemini; the
task-bridge export was imported by src/voice-agent.ts:39 but never
referenced anywhere — dead import.

Confirmed safe to delete via independent grep:
- 0 call sites in src/
- 0 test references
- only consumer was the dead import

Removes the third source of the duplicate-tool-name 1011 setup error
documented in reference_gemini_1011_tool_name_conflict.md.

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

* fix(summon): use zoommtg:// deeplink + tighten meeting-window readiness check

Two bugs caused phone summon on Studio to log "Screen share started" while
nothing actually shared:

1. **HTTPS join URL unreliable on Zoom Workplace.** The `zoomRunning=true`
   branch was using `https://zoom.us/j/{id}?pwd=...`, which routes through
   the browser and doesn't reliably hand off to the in-app meeting on Zoom
   Workplace (Studio). Result: Zoom stayed on its home dashboard, no
   meeting joined. Replaced with the `zoommtg://` deeplink for both
   running-and-not-running cases — direct to the Zoom app, no browser hop.

2. **Readiness check counted any window.** The old check waited for `(count
   of windows of process "zoom.us") > 0`, which the always-on "Zoom
   Workplace" home dashboard already satisfies. So even with no meeting
   ever opened, the check passed, the share-screen menu fired into an
   inapplicable context, the AppleScript silently no-op'd, and the catch
   block still logged success. Now checks window names for `Zoom Meeting`
   / `zoom share` / `floating video` — matches `alreadyInMeeting`'s
   detection.

Verified 2026-04-29 phone summon on Studio (`/tmp/conversation-server.log`
15:54:27–15:54:36) showed the false-positive log; instrumented a11y dump
confirmed only "Zoom Workplace" was open after the supposed join.

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

* fix(summon): click Join by a11y description; tighten host-joined check

Phone summon on Studio kept missing the actual Join button: the cliclick
coordinate (`preview.x + 0.80*w, preview.y + 0.93*h`) didn't land on the
button on Zoom Workplace's preview layout. The meeting never opened,
the host-joined check then matched "Personal Meeting Room" (a preview
window!) on `winNames.includes('Meeting')` and reported `Host joined: true`
falsely, so the phone dial-in path fired anyway, and the new strict
share-readiness gate timed out 30s later. Net: `summon` returned error
30002ms after a chain of internal false positives.

Two fixes:

1. **Join click via a11y description, not coordinates.** Iterate `buttons of
   w whose description is "Join"` across all zoom.us windows, click the
   first one. The Join button's a11y description is stable across Zoom
   Workplace layouts (verified by direct dump 2026-04-29).

2. **Host-joined check uses the strict meeting-window set.** Same
   `Zoom Meeting | zoom share | floating video` set as the share-readiness
   gate. Plain `Meeting` was matching preview windows.

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

* fix(summon): bump tool timeout to 60s — Studio Zoom takes ~30s to open meeting

Phone summon test sonichi#4 (2026-04-29 16:06): a11y Join click landed correctly,
but the meeting window took 31s to appear after the click on Studio Zoom
Workplace. Bodhi's default tool timeout is 30s, so the tool returned
error 30001ms BEFORE the share-menu fired. The share itself eventually
fired (logged 16:06:57) but by then the assistant had already told the
caller "I'm having some trouble summoning."

`60_000` covers the worst-case meeting-open + share-menu + mute AppleScripts.

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

* fix(record-with-narration): write playback-path on auto-stop so play_video can find the file

After PR sonichi#546 made open_file generic (no findRecording fallback), there
was a behavioral gap: record_screen_with_narration's auto-stop didn't
persist a playback-path hint, while screen_record stop did. Result on
2026-04-29 phone test: user said "open the recording" → model called
open_file with no path → got generic "no path provided" error → user
gave up.

This commit closes the gap WITHOUT re-coupling open_file to recording:
- record_screen_with_narration auto-stop now writes /tmp/sutando-playback-path
  with the best-available file (subtitled > narrated > raw), matching
  screen_record stop's existing pattern.
- Tool description appends "After auto-stop, to play back or open the
  recording, call play_video — it auto-finds the file. Do NOT call
  open_file unless you have the absolute path." to nudge the model
  toward the correct tool.

open_file remains general-purpose; recording-tools owns the playback-hint
write. play_video already reads /tmp/sutando-playback-path with a
findRecording fallback.

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

* fix(record-with-narration): return predicted recording paths so model can pass to open_file

The 2026-04-29 phone-summon test still failed on "open the recorded video"
even after `19ecfb8` wrote `/tmp/sutando-playback-path` on auto-stop —
because the model called `open_file` (the natural verb match for "open"),
not `play_video`, and `open_file` is generic and needs a path arg.

Now `record_screen_with_narration` captures `record.py start`'s JSON output
to extract the absolute `recording_path` and computes the
`narrated_path = path.replace('.mov', '-narrated.mov')`. Both paths
are returned in the start result + spelled out in the `message` field
("After auto-stop, the file will be at ... — pass that exact path to
open_file when the user asks to open the recording.").

The model now has the path inline from the moment recording starts and can
pass it to open_file (or any other path-taking tool) without guessing. No
recording-specific code in open_file. No reliance on the model picking up
the prior tool description hint.

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

* fix(record-with-narration): return subtitled_path so model can open the burned-in version

After 35cce47 returned `recording_path` + `narrated_path`, the model still
couldn't open the subtitled version because it didn't know that path. The
2026-04-29 21:00 phone test had subtitle burn succeed (file at
/tmp/sutando-recording-X-narrated-subtitled.mov) but the model called
open_file three times with narrated/raw paths only — Susan kept saying
"this is not the one with the subtitle".

Now the start result includes `subtitled_path` and the message string
spells out the priority order: pass subtitled_path to open_file when the
user asks for "the recording" / "with subtitles". Fall back to
narrated_path only when subtitled burn failed (e.g., libass missing,
which is now known-fixed via the homebrew-ffmpeg/ffmpeg tap).

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

* fix(recording/open-file): clarify subtitle support + nudge model away from path hallucination

Two regressions from phone summon test sonichi#7 (CA5c9c0a711cfcacc2cc5458c1d5eca04d
21:09 ET):

1. **Model hallucinated "I can't add subtitles"** — record_screen_with_narration's
   tool description didn't mention that subtitle burn happens automatically when
   the live transcript exists. User said "add subtitle", model refused, even
   though the auto-stop callback ALWAYS calls burnLiveTranscriptSubtitles when
   liveTranscriptRecordingStart > 0. Description now states subtitles ARE always
   burned in — never refuse a "with subtitles" request.

2. **Model hallucinated path `/tmp/screen_recording_with_narration.mov`** when
   asked to open the recording — invented a generic name instead of using the
   `subtitled_path` returned by start. Two-pronged nudge:
   - Recording-tools description: "Do NOT invent file paths — only use the exact
     paths returned by this tool" + clarifies subtitled_path is the right one
     for "with subtitles".
   - open_file's `File not found` error explicitly says "Do not invent paths"
     and routes to play_video for unknown-path recording-open.

`open_file` itself remains generic — only its error string text changed, no
recording-specific code. play_video already has the findRecording fallback.

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

* fix(open-file): strip ALL video-related logic — back to truly generic open

Per Susan: open_file should not contain any video-specific behavior. The
fullscreen=true flow forced QuickTime, ran a present-mode AppleScript with
30-iteration polling, and had a guard against re-opening when QT already
had documents. All video-specific.

Removed:
- `fullscreen` parameter
- QT force-open via `open -a "QuickTime Player"`
- Path-matched present-mode AppleScript (Cmd+Ctrl+F)
- QT-already-has-doc guard
- Description text about video presentation, .mp4/.mov, "fullscreen on a
  video that is already open"

Now: `execFileSync('open', [filePath])` — LaunchServices picks the user's
default app for that file type. Mirrors what a human running `open foo.x`
in a terminal would get.

For fullscreen on a just-opened video, the model should call the existing
`fullscreen` tool as a separate step. Two-tool sequence is simpler than
the polled AppleScript embedded in open_file.

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

* fix: address Mini PR sonichi#546 review — summon retry/return, generic fullscreen, soften subtitle claim

Mini's review on PR sonichi#546:

**1. summon Join click + return values (Medium)** — the prior split (one-shot
Join + 30s readiness) let a late preview-window race leave the meeting
un-joined while readiness ran out the clock; the tool then still returned
"summoned" / "joined with screen sharing". Now: combined 60s loop where
each iteration first checks for a real meeting window (Zoom Meeting / zoom
share / floating video), then if not present retries the a11y Join click.
Track `zoomReady` and `shareStarted` accurately and surface them in the
return payload + status string (`summoned` | `joined_share_failed` |
`meeting_window_not_detected`). instruction text reflects what actually
happened.

**2. summon timeout (Medium)** — 60s was still tight per Mini's worst-case
calc (Join 10s + readiness 60s + share 15s + mute/cleanup 10s + dial-in
host wait 20s ≈ 115s). Bumped to 120s.

**3. record_screen_with_narration subtitle claim (Low)** — softened "IS
automatically burned" to "are attempted automatically when both transcript
text and a libass-capable ffmpeg are present". Burn can silently skip on
missing transcript / unreadable narrated / missing libass; honesty matters
more than over-promising. Description still says "never refuse a 'with
subtitles' request" so the model doesn't gate refusal on apparent failure.

**Bonus (Susan):** open_file gets a *generic* `fullscreen` parameter
back — but app-agnostic this time. Just `open <path>` (LaunchServices
routes to default app) → 1.5s settle → System Events Cmd+Ctrl+F. Works for
video in QuickTime, PDF in Preview, web pages in Chrome — no QT
hardcoding, no path-match polling. Replaces the QT-specific present-mode
flow with a one-keystroke generic toggle.

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

* fix(summon): address Mini PR sonichi#546 round-2 — dialIn order + 1-line instruction fix

Medium sonichi#1 (code change): dialIn previously ran a separate host-joined wait
BEFORE the readiness/Join-retry loop. Preview-still-up race made dialIn
fire against an unjoined meeting. Restructured: readiness/Join-retry loop
runs FIRST and gates the dialIn block on zoomReady. Removed redundant
standalone host-joined wait.

Low sonichi#2 (text only): instruction in phoneJoined branch falsely claimed
"Screen is shared..." when shareStarted=false. One-line fix:
`(shareStarted ? 'Screen is shared and ' : '')` prefix.

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

---------

Co-authored-by: Xueqing Liu <xueqingliu@Xueqings-Mac-Studio.local>
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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

open_file 18s polling timeout causes false 'recording not found' on phone calls

2 participants

@sonichi @liususan091219