Extend VICE support #103

chrisgleissner · 2025-11-02T00:05:22Z

Extend VICE support so that as many of the C64 Bridge tools as possible can leverage VICE instead of a real C64U device.

Copilot

Pull Request Overview

This PR implements comprehensive VICE emulator support for the MCP server, adding:

A complete VICE Binary Monitor client with debugger capabilities (checkpoints, registers, stepping)
Process management and test infrastructure with mock server support
Platform-aware tool routing with supportedPlatforms metadata
Extensive test coverage using a mock BM server for CI environments

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/vice/viceClient.ts	Core Binary Monitor protocol client with extended debugger ops
src/vice/mockServer.ts	Lightweight BM stub for test environments
src/vice/process.ts	VICE process lifecycle management with Xvfb support
src/vice/readiness.ts	Helpers for polling BASIC READY and screen patterns
src/tools/debug.ts	Debugger tool module (checkpoints, registers, stepping)
src/tools/vice.ts	VICE-specific tools (display capture, resource access)
src/device.ts	ViceBackend implementation with long-lived process support
test/*	Comprehensive test coverage with platform-aware harness
scripts/run-tests.ts	Test matrix runner supporting multiple platforms/targets
doc/*	Updated documentation and rollout tracking

Copilot · 2025-11-02T00:07:45Z

src/vice/viceClient.ts

    const bytes = Buffer.from(text, "ascii");
    const body = Buffer.concat([Buffer.from([bytes.length & 0xff]), bytes]);
    await this.send(0x72, body);
  }


Missing blank line before new method section. The checkpoint methods start immediately after keyboardFeed without separation, while earlier methods (e.g., before line 195) maintain consistent spacing between functional groups.

Suggested change

}

}

Copilot · 2025-11-02T00:07:45Z

src/vice/readiness.ts

+async function resumeEmulation(bm: ViceClient): Promise<void> {
+  try {
+    await bm.exitMonitor();
+  } catch {
+    // Ignore errors while resuming; monitor may already be running.
+  }
+}


The function name resumeEmulation is ambiguous. The comment says 'monitor may already be running,' but the function name suggests starting emulation. Consider renaming to exitMonitorIfNeeded or ensureEmulationRunning for clarity.

Copilot · 2025-11-02T00:07:46Z

test/vice/viceSmokeTest.ts

+    0x0E,0x08,
+    0x0A,0x00,
+    0x99,


Removed inline comments explaining BASIC program structure ($080E pointer, line 10, PRINT token). The original comments improved readability for this low-level byte sequence. Consider restoring them or adding a docstring to buildHelloProgramBody.

Copilot · 2025-11-02T00:07:46Z

test/suites/mcpServerCallToolSuite.mjs

+function toolIsAvailable(ctx, toolName) {
+  if (typeof ctx.isToolSupported === "function") {
+    try {
+      return ctx.isToolSupported(toolName);
+    } catch {
+      return true;
+    }
+  }
+  return true;
+}


The function returns true on exception (line 53), which contradicts the function name toolIsAvailable. If isToolSupported throws, the tool support status is unknown, not confirmed as available. Consider returning false on exception or logging the error.

Copilot · 2025-11-02T00:07:46Z

src/device.ts

  constructor(config: ViceConfig) {
-    this.exe = config.exe || which("x64sc") || which("x64") || "x64sc";
+    const envBinary = configuredString(process.env.VICE_BINARY);
+    this.exe = configuredString(config.exe) ?? envBinary ?? which("x64sc") ?? which("x64") ?? "x64sc";


The environment variable takes precedence over config file (envBinary is checked before config.exe), but the configuration documentation in README.md suggests config files should override env defaults. This ordering contradicts typical precedence (explicit config > env > defaults).

Suggested change

this.exe = configuredString(config.exe) ?? envBinary ?? which("x64sc") ?? which("x64") ?? "x64sc";

this.exe = config.exe !== undefined && config.exe !== null

? config.exe

: envBinary ?? which("x64sc") ?? which("x64") ?? "x64sc";

Copilot · 2025-11-02T00:07:46Z

scripts/run-tests.ts

+const bunRuntime = (globalThis as { Bun?: any }).Bun;
+if (!bunRuntime || typeof bunRuntime.spawn !== "function") {
+  throw new Error("[run-tests] Bun runtime is required to execute tests");
+}
+const child = bunRuntime.spawn({


The script requires Bun runtime (line 108 error message), but the documentation states 'stay on the npm variants if Bun is not installed.' This hard requirement contradicts the documented fallback behavior and will break npm test on Node-only installations.

chrisgleissner · 2025-11-02T00:08:15Z

@codex Fix the failing tests against VICE:

skip) poweroff succeeds with valid response
(skip) poweroff handles failure response
(skip) poweroff handles exception
(skip) menu_button succeeds with valid response
(skip) menu_button handles failure response
(skip) menu_button handles exception
32 tests failed:
(fail) music_generate builds timeline and triggers SID sequence [2.00ms]
(fail) music_generate defaults to triangle waveform and best-practice ADSR
(fail) music_generate expression preset uses varied durations and reports preset
(fail) music_generate survives playback error and logs [1.00ms]
(fail) music_compile_and_play compiles SIDWAVE to PRG and runs on C64
(fail) music_compile_and_play can emit SID and use attachment playback
(fail) sidplay_file delegates to C64 client and returns metadata [1.00ms]
(fail) modplay_file delegates to C64 client
(fail) music_compile_and_play handles C64 firmware failure for PRG
(fail) music_compile_and_play handles C64 firmware failure for SID
(fail) music_compile_and_play wraps unexpected errors [1.00ms]
(fail) music_compile_and_play validates sidwave input
(fail) music_compile_and_play respects dryRun flag
(fail) sid_volume clamps and normalizes address [1.00ms]
(fail) sid_volume reports firmware failure
(fail) sid_volume wraps unexpected errors
(fail) sid_reset soft and hard [1.00ms]
(fail) sid_note_on passes parameters and returns metadata
(fail) sid_note_on surfaces firmware failure
(fail) sid_note_off and silence_all
(fail) sid_silence_all verify reports silence metrics [1.00ms]
(fail) sid_silence_all verify fails when residual audio remains
(fail) analyze_audio returns guidance when no keywords detected
(fail) analyze_audio wraps backend errors when keywords present [1.00ms]
(fail) record_and_analyze_audio returns error when backend missing
(fail) music_generate validates pattern input
(fail) stream_start forwards parsed payload
(fail) stream_start surfaces firmware failure
(fail) stream_stop forwards parsed payload [1.00ms]
(fail) stream_stop surfaces firmware failure
(fail) poweroff is unsupported on vice
(fail) menu_button is unsupported on vice [1.00ms]
 425 pass
 21 skip
 32 fail
 2836 expect() calls
Ran 478 tests across 50 files. [4.93s]
Error: Process completed with exit code 1.

chatgpt-codex-connector · 2025-11-02T00:16:48Z

Summary

Added a Bun-less fallback path to the matrix runner so npm test can spawn the Node harness when Bun is unavailable while preserving the computed environment. scripts/run-tests.tsL6-L147
Updated the Bun test wrapper to supply timeouts via the supported signature and treated tool-support probe failures as “unsupported” during call-tool assertions. test/helpers/test.jsL14-L67 test/suites/mcpServerCallToolSuite.mjsL48-L56
Honoured config precedence for the VICE binary, clarified the monitor-exit helper, restored method grouping, and documented the smoke test’s HELLO BASIC program layout. src/device.tsL238-L243 src/vice/readiness.tsL11-L120 src/vice/viceClient.tsL231-L238 test/vice/viceSmokeTest.tsL60-L76

Testing

✅ bun test test/audioModule.test.mjs --filter music_generate
✅ bun test test/machineControl.test.mjs --filter poweroff

View task →

chrisgleissner added 6 commits November 1, 2025 18:16

feat(vice): integrate BM client and mock server (phase 1)

80e491b

feat(vice): supervise emulator and inject prg

8f5650f

feat(debug): add c64_debug grouped tool

a3ba6b3

feat(vice): add emulator resource and display tools

31c5a17

feat(vice): promote vice backend to preview status

d4ad9c2

feat(vice): promote vice backend to preview status

c12487f

Copilot AI review requested due to automatic review settings November 2, 2025 00:05

chrisgleissner marked this pull request as draft November 2, 2025 00:07

Copilot AI reviewed Nov 2, 2025

View reviewed changes

chrisgleissner added 6 commits November 2, 2025 08:27

fix: Various Vice-related fixes

94a8769

fix: Various Vice-related fixes

e18eef1

fix: Various Vice-related fixes

9b04c73

test(vice): use actual VICE in tests

84fe0f6

test(vice): use actual VICE in tests

b5bea28

test(vice): Increased logs

3cac65a

chrisgleissner marked this pull request as ready for review November 2, 2025 11:17

test(vice): Merged in main

9f6e082

chrisgleissner changed the title ~~Added VICE support~~ Extend VICE support Nov 2, 2025

chrisgleissner added 3 commits November 2, 2025 11:25

test(vice): Fixed test reporter

dfe714f

test(vice): Ensure xvfb socket directory exists

b3ee08b

test(vice): More device interaction fixes

c4636cc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Extend VICE support #103

Extend VICE support #103

Uh oh!

chrisgleissner commented Nov 2, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 2, 2025

Uh oh!

Copilot AI Nov 2, 2025

Uh oh!

Copilot AI Nov 2, 2025

Uh oh!

Copilot AI Nov 2, 2025

Uh oh!

Copilot AI Nov 2, 2025

Uh oh!

Copilot AI Nov 2, 2025

Uh oh!

chrisgleissner commented Nov 2, 2025

Uh oh!

chatgpt-codex-connector bot commented Nov 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    this.exe = configuredString(config.exe) ?? envBinary ?? which("x64sc") ?? which("x64") ?? "x64sc";
+    this.exe = config.exe !== undefined && config.exe !== null
+      ? config.exe
+      : envBinary ?? which("x64sc") ?? which("x64") ?? "x64sc";

Extend VICE support #103

Are you sure you want to change the base?

Extend VICE support #103

Uh oh!

Conversation

chrisgleissner commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

chrisgleissner commented Nov 2, 2025

Uh oh!

chatgpt-codex-connector bot commented Nov 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chrisgleissner commented Nov 2, 2025 •

edited

Loading