Skip to content

Fixes from the live run: audio-only 0×0 warnings, snapshots saved to disk - #17

Merged
Tom-R-Main merged 1 commit into
mainfrom
improve/live-fixes
Sep 24, 2026
Merged

Tom-R-Main merged 1 commit into
mainfrom
improve/live-fixes

Conversation

@Tom-R-Main

Copy link
Copy Markdown
Owner

Stacked on #16.

Found live (OBS 32.2.2, after recovering OBS from the parallel-batch deadlock):

  • The read-only live suite passes: 6 passed, 2 skipped. That includes batch results lining up with their requests and snapshot plus dry-run restore finding nothing to change.
  • obs-apply-scene plan-only against "Demo": it reported "already matches", and for fit: "canvas" it planned only the 3 transform fields that differ.
  • Live take test (changes undone afterwards):
    • Set-up: obs-apply-scene added a muted synthetic 440 Hz tone (a media source playing a WAV, not system audio) behind the background.
    • Take: obs-take-start {expectSilent: true}, tone unmuted for 4 s, then obs-take-stop.
    • The monitor flagged "MCP Test Tone is audible in the recording at −38.1 dB" 0.1 s after the unmute; ffprobe measured the file at −37.9 dB.
    • Chapters landed at 0.01 / 4.02 / 8.02 s, control was claimed automatically, and the take log was written. 6 missed render frames were flagged (the Mac was locked).
    • No black stretch was reported, which was correct: the captured window was showing a paused video.
    • Clean-up: the tone input was removed, the scene's original order was verified, and the test recording and take log were deleted.

Fixed here

  1. Audio-only sources were flagged as 0×0. obs-apply-scene warned "MCP Test Tone renders at 0×0", and preflight did the same. Any scene with a microphone got a false warning, and the read-back after changing a mic waited a second for a size that never comes.
    • New rendersNoVideo(kind, settings): audio-capture kinds on every platform, plus ffmpeg_source/vlc_source playing an audio file.
    • Used by preflight, obs-apply-scene and the read-back after an input change.
  2. Snapshots were lost when the server exited. The test driver's server had exited, so "obs-restore undoes this" wasn't true afterwards.
    • Snapshots are now saved to OBS_MCP_STATE_DIR (default: the platform's app-data folder, e.g. ~/Library/Application Support/obs-mcp).
    • Written atomically, owner-readable only (0600), the last 20 kept.
    • Each server lists only the snapshots of the OBS instance it connects to.
    • A Vitest setup file gives every unit test a fresh state directory.

Tests: npm run check passes (210 unit, 14 e2e); live suite 6 passed, 2 skipped. New tests: the audio-only kinds; preflight with a mic; obs-apply-scene with an audio file; a mic settings change (no wait, no warning); a snapshot restored by a new server on the same OBS address (file mode 0600); per-instance listing; state-directory paths.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vjn8uBEa4mJshaQU3pFuqX

Microphones, desktop audio, and media sources playing an audio file are
always 0x0, but obs-preflight, obs-apply-scene, and the read-back after an
input change warned about them, and the read-back waited a second for them
to get a size. A shared rendersNoVideo check now skips them.

Snapshots lived in the server's memory, so obs-restore could not undo a
change once that server exited, which is what happened in the live test.
They are now saved to the state directory (OBS_MCP_STATE_DIR, or the
platform's app data folder), owner-readable only, the last 20, and listed
per OBS instance. Unit tests get a fresh state directory each.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vjn8uBEa4mJshaQU3pFuqX
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-24T04:23:41.865652Z b37e4a5 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b37e4a51b3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/tools/snapshots.ts
Comment on lines +114 to +115
} catch (error) {
logger.error(`Could not save snapshots to ${path}: ${error instanceof Error ? error.message : String(error)}`);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Propagate snapshot persistence failures

When the state directory is unwritable, the disk is full, or renameSync otherwise fails, this catch only logs the error and lets takeSnapshot return normally. Consequently obs-snapshot, obs-apply-scene, and obs-capture-window report that a restorable snapshot was saved and may proceed with changes, but that snapshot cannot be restored. Propagate the failure so callers do not promise an undo point that does not exist.

Useful? React with 👍 / 👎.

Comment thread src/tools/snapshots.ts
const list = snapshots(client);
list.push(snapshot);
if (list.length > MAX_SNAPSHOTS) list.shift();
saveAll([...loadAll(), snapshot]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Serialize cross-process snapshot updates

When two stdio server processes take snapshots concurrently, each can read the same snapshots.json, append only its own snapshot, and then atomically rename its stale list over the other process's update; one successfully reported snapshot is therefore lost. This is directly possible in the newly supported cross-agent workflow, and atomic rename prevents torn writes but not this read-modify-write race, so updates need cross-process serialization or independent per-snapshot files.

Useful? React with 👍 / 👎.

@Tom-R-Main
Tom-R-Main changed the base branch from improve/http-lease to main September 24, 2026 05:12
@Tom-R-Main
Tom-R-Main merged commit ca16917 into main Sep 24, 2026
4 checks passed
@Tom-R-Main
Tom-R-Main deleted the improve/live-fixes branch September 24, 2026 05:13
Tom-R-Main added a commit that referenced this pull request Sep 24, 2026
Fix the defects found reviewing PRs #3–#17
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.

1 participant