MagicBeanDev/Hookshot

Archived

Watch

Fork 0

Add an MCP server for scripted in-game testing #1025

Open

nicholasgower wants to merge 16 commits from nicholasgower/feature/mcp-ingame-testing into main

nicholasgower commented

2026-11-09 06:48:12 +00:00

(Migrated from github.com)

What this adds

scripts/hookshot_mcp.py, a Model Context Protocol server that turns a built Hookshot into
something a test can drive. It is a separate Python process: nothing is linked into hookshot.exe, and it needs no game
files of its own.

launch starts the game headless, muted, on Vulkan, script-only (no physical pad, keyboard or mouse), with its own run
folder, save folder and shader/pipeline cache.
press / stick / sequence write the game's existing live-input file (HOS_INPUT_LIVE) and wait for the game to log
that it read the line.
screenshot asks the renderer for one PNG of the next presented frame (Vulkan RX_VK_SHOT_FILE, WebGPU
RX_SCREENSHOT_FILE) and returns it inline as MCP image content plus a path on disk, with the present number.
status / logs / wait read the process state and the recent game log, and advance wall time.
stop asks the game to quit with the !quit live command, then terminates if it does not.
Resources: hookshot://log, hookshot://last-screenshot, hookshot://status.

Game-side seams added for this: HOS_INPUT_SCRIPT_ONLY, the analog stick tokens LX=, LY=, RX=, RY=, the !quit
live command, HOOKSHOT_NO_ENV_FILE, and the one-shot screenshot request files.

Docs: docs/mcp.md (install, host configuration, tools, launch options, what the server sets, limits). The tool and
resource tables in it are checked against the live server by tests/test_docs.py.

Why this matters for testing

Today the project has two testing speeds: game-free unit tests that run in seconds, and a person playing the game. Anything
in between — start the game, reach the title, open a menu, look at the frame — is done by hand, one run at a time, and
cannot be repeated after a change.

That is the gap this closes:

Regressions can be found without playing. A change to the renderer, the HLE runtime, input routing or the builder
can be checked by the same scripted sequence: it either produces the expected frames or it does not. The log, the input
lines and the PNGs are left in a run folder to compare.
It is the base for every further automated test. Input plus a screenshot plus a log is enough to test menus,
cutscenes, the pause and file menus, resolution and frame-generation settings, controller mappings and save paths
without a human at the keyboard.
Any MCP host can run the test, including agents. Claude Desktop, OpenCode, the MCP Inspector or a CI wrapper speak the
same protocol; the tools are plain JSON calls and the frame comes back as an image, so "is the title screen drawn?" is a
question a model can actually answer about a real build.
It unblocks dependent work. The save editor (#784) needs scripted input to reach a save point; it is on hold until
this lands.
It is cheap to keep running. One process, no window, ~10 GB, a clean exit code, and no dialog boxes anywhere.

Rebased, and the previous PR

This is the same branch as #764, which was closed to keep the queue clear with "please rebase on current main and reopen".
It is rebased onto main as it stands at the last push, and the game has been rebuilt from this branch
and driven end to end.

What I tested

Not only the unit tests: the server was run against a real build of this branch, over its real stdio transport, and every
tool was exercised. Build: a normal builder layout (hookshot.exe launcher, bin\, romfs), Vulkan back end, Windows,
13 min 47 s, 0 failed modules, 100.0000% of 17,646,501 instructions translated.

Game-free suite (what CI runs):

Check	Result
`tests/test_mcp_server.py`	PASS — protocol in memory and over stdio, tool and resource catalogue, input-line encoding, environment handling, `game_dir`, fake-game end to end
`tests/test_docs.py`	PASS — links, documented commands, MCP tables checked against the live server
`tests/test_mcp_scenario.py`	PASS — a passing scenario runs, an impossible expectation fails
`tests/test_links_case.py`	PASS — every relative link matches the path case on disk
`tests/test_pr_check.py`	PASS
`scripts/pr_check.py --base main`	PASS — 30 changed paths, none forbidden; LF endings; Conventional subjects and both trailers

The full game-free suite gave 301 passed, 5 failed, 20 skipped (run on Windows with llvm-mingw and the Vulkan
and SPIR-V headers, at --jobs 6 so the fixture builds are not starved). None of the five touches a file in this
PR: tests/test_antilag.py and nx2/test_nup_out_of_space.py fail identically in a clean origin/main worktree
at the base commit (the first is main's present-ahead refactor moving the Anti-Lag call out of present_image),
nx2/test_nup_paths.py needs symlink privileges this PC does not grant, tests/test_wayland_libdecor_hint.py
trips a clang-only -Wunused-but-set-global, and tests/test_precompile_compute_guard.py needs windows.h for
its --target=x86_64-pc-windows-msvc fixture. The four checks this PR adds, and the capture checks it adapts,
all pass.

Every tool against the real game, over stdio (34 checks):

Tool	What was verified
`launch`	cold run reached its first present at 85 s while 38,809 programs were prepared, warm run ~4 s; pid, run dir, log, input and request paths returned; `wait_ready` works
`status`	correct before launch, while running (renderer `Vulkan (hookshot_vk.dll)`, uptime, last screenshot) and after stop
`screenshot`	1280x720 PNG inline and to a chosen path, `present` parsed (e.g. "present 116"); the picture is the presented frame
`press`	`A` acknowledged in the game log, and it started New Game — the frame changed from the title screen to the intro cutscene
`stick`	left and right acknowledged; `LX=`, `LY=`, `RX=` in the log; the frame changed
`sequence`	a three-step line queued and acknowledged (`0:DOWN/120,400:DOWN/120,800:A/150`); the frame changed
`wait`	returns status; uptime advances
`logs`	filtered (`hid: live`) and unfiltered; every injected line appears
`stop`	clean `!quit`, exit code 0, process gone; a second `stop` is safe
resources	status (JSON), log (text/plain), last-screenshot (image/png, 1280x720)
error paths	before launch, duplicate launch, unknown button, centred stick, reserved `extra_env`, after stop — all clear messages

The visual half of that run was checked too: title screen, then the cutscene after A, then further changes after stick and
sequence — 12/12 visual checks. One detail worth knowing (now in the docs): the very first wait_ready screenshot of a cold
run is a black boot frame, because it returns on the first present while the shaders are still being prepared; the run above
waited for the title before asserting on the picture. The screenshots stayed on the development disk; no game-derived file is
in this PR.

Finally the server was installed as a project MCP server in OpenCode (V2 mcp.servers configuration) and the nine tools were
called through OpenCode itself — status, launch (with wait_ready), wait, press, stick, sequence, screenshot,
logs, stop — all returned the expected results, and the run ended with exit code 0.

Bug found and fixed by this testing

--game-dir and HOOKSHOT_MCP_GAME_DIR selected the folder's hookshot.exe. In a folder made by the builder or
make_game.py that is the setup launcher, which opens the installed builder (builder/setup/setup.c), not the game in
bin\; launch would have started the builder setup and no screenshot would ever have arrived. resolve_paths now prefers
bin\hookshot.exe and falls back to the top of the folder for a development build, and tests/test_mcp_server.py covers
both layouts.

docs/mcp.md also now gives the V2 OpenCode configuration (mcp.servers, no enabled field) and notes that wait_ready
returns on the first present, which on a cold cache is a boot frame while the shaders are still being prepared.

On the CLI-first feedback in #764

The points raised there — a CLI, files instead of inline images, state in run folders, waits on game state, a GPU lock — are
reasonable, and this keeps the door open for them. Every tool is a thin call into GameSession; the run state already lives
in a run folder (log.txt, input.txt, shot.request, shots/); screenshot(inline=false) returns a path instead of image
content; and input waits for the game's acknowledgement line rather than a fixed sleep. A CLI front end over the same
functions, a wait-on-log-state tool and a GPU lock are the natural next steps, and none of them need this PR to change shape.

settings() reads every row of the Insert menu — Graphics, Gameplay and Controls — with its key, label, value, range
and the text the player sees. When the cheat menu is on it also returns the Cheats page's eleven rows, the pouch's
items, the 31 teleport destinations and the inventory's 85 items with their counts.
set_setting(name, value) goes through the menu's own setter: it clamps, applies a quality preset or a control layout,
marks the settings Custom and writes hookshot_options.ini. A row that says it needs a restart still does.
cheat(name, value, item) runs a cheat row directly: heart protection, no-clip flight and the free camera; mark and
return; the clock; teleports by name or index; the age; the pouch; the inventory (a wanted count, or -1 to
unlock/relock).
launch(cheats=true) turns the cheat menu on with HOS_OOT_CHEATS=1, which is otherwise a line in hookshot_env.txt
before start.

On the game side these are three live-input commands — !settings, !setting <key> <value>, !cheat <name> <value> <extra> — answered in the game log, which the server reads back. The rows cross the renderer table (API 8) from
gpu/vk/vk_menu.cpp through host/rx_backend.c to host/nn_hid.c. The WebGPU back end has no Insert menu and says so.

Testing found and fixed one real defect: two cheat actions in quick succession overwrote each other (the menu kept a single
atomic), so an inventory edit sent right after a pouch edit was lost. They are a queue now, and both are taken.

Real-game verification against a build of this branch (Vulkan, a copied save): 23 settings listed with ranges and text;
set_setting("frame_pacing", 1), set_setting("text_speed", 3) and set_setting("frame_generation", 0) applied, shown by
settings() and written to hookshot_options.ini; an unknown key refused; heart protection and the free camera toggled
and read back; cheat("set_time", 18) made the game log "cheats: requested 18:00"; cheat("pouch", item="Bombs", value=5)
logged "cheats: pouch Item_Bomb +5"; cheat("teleport", item="Kakariko Village") moved Link from Death Mountain Trail to
Kakariko Village (screenshot, and "cheats: teleport child to KakarikoVillage_Child ... request sent"); mark_position
logged the position; two actions sent back to back both ran.

The game-free suite: 301 passed, 5 failed, 20 skipped — the five are main's own or this PC's toolchain, as
described above.

Not included

No game files, dumps, builds, saves, logs or screenshots. Test runs stayed under work/, which is ignored.

Why: the project's only in-game test today is a person playing. This adds an MCP server so an agent or a script can start a build, drive controller input and look at the frame, which makes in-game regressions repeatable. ## What this adds `scripts/hookshot_mcp.py`, a [Model Context Protocol](https://modelcontextprotocol.io) server that turns a built Hookshot into something a test can drive. It is a separate Python process: nothing is linked into `hookshot.exe`, and it needs no game files of its own. - **launch** starts the game headless, muted, on Vulkan, script-only (no physical pad, keyboard or mouse), with its own run folder, save folder and shader/pipeline cache. - **press / stick / sequence** write the game's existing live-input file (`HOS_INPUT_LIVE`) and wait for the game to log that it read the line. - **screenshot** asks the renderer for one PNG of the next presented frame (Vulkan `RX_VK_SHOT_FILE`, WebGPU `RX_SCREENSHOT_FILE`) and returns it inline as MCP image content plus a path on disk, with the present number. - **status / logs / wait** read the process state and the recent game log, and advance wall time. - **stop** asks the game to quit with the `!quit` live command, then terminates if it does not. - Resources: `hookshot://log`, `hookshot://last-screenshot`, `hookshot://status`. Game-side seams added for this: `HOS_INPUT_SCRIPT_ONLY`, the analog stick tokens `LX=`, `LY=`, `RX=`, `RY=`, the `!quit` live command, `HOOKSHOT_NO_ENV_FILE`, and the one-shot screenshot request files. Docs: `docs/mcp.md` (install, host configuration, tools, launch options, what the server sets, limits). The tool and resource tables in it are checked against the live server by `tests/test_docs.py`. ## Why this matters for testing Today the project has two testing speeds: game-free unit tests that run in seconds, and a person playing the game. Anything in between — start the game, reach the title, open a menu, look at the frame — is done by hand, one run at a time, and cannot be repeated after a change. That is the gap this closes: - **Regressions can be found without playing.** A change to the renderer, the HLE runtime, input routing or the builder can be checked by the same scripted sequence: it either produces the expected frames or it does not. The log, the input lines and the PNGs are left in a run folder to compare. - **It is the base for every further automated test.** Input plus a screenshot plus a log is enough to test menus, cutscenes, the pause and file menus, resolution and frame-generation settings, controller mappings and save paths without a human at the keyboard. - **Any MCP host can run the test, including agents.** Claude Desktop, OpenCode, the MCP Inspector or a CI wrapper speak the same protocol; the tools are plain JSON calls and the frame comes back as an image, so "is the title screen drawn?" is a question a model can actually answer about a real build. - **It unblocks dependent work.** The save editor (#784) needs scripted input to reach a save point; it is on hold until this lands. - **It is cheap to keep running.** One process, no window, ~10 GB, a clean exit code, and no dialog boxes anywhere. ## Rebased, and the previous PR This is the same branch as #764, which was closed to keep the queue clear with "please rebase on current main and reopen". It is rebased onto `main` as it stands at the last push, and the game has been rebuilt from this branch and driven end to end. ## What I tested Not only the unit tests: the server was run against a real build of this branch, over its real stdio transport, and every tool was exercised. Build: a normal builder layout (`hookshot.exe` launcher, `bin\`, `romfs`), Vulkan back end, Windows, 13 min 47 s, 0 failed modules, 100.0000% of 17,646,501 instructions translated. Game-free suite (what CI runs): | Check | Result | |---|---| | `tests/test_mcp_server.py` | PASS — protocol in memory and over stdio, tool and resource catalogue, input-line encoding, environment handling, `game_dir`, fake-game end to end | | `tests/test_docs.py` | PASS — links, documented commands, MCP tables checked against the live server | | `tests/test_mcp_scenario.py` | PASS — a passing scenario runs, an impossible expectation fails | | `tests/test_links_case.py` | PASS — every relative link matches the path case on disk | | `tests/test_pr_check.py` | PASS | | `scripts/pr_check.py --base main` | PASS — 30 changed paths, none forbidden; LF endings; Conventional subjects and both trailers | The full game-free suite gave 301 passed, 5 failed, 20 skipped (run on Windows with llvm-mingw and the Vulkan and SPIR-V headers, at `--jobs 6` so the fixture builds are not starved). None of the five touches a file in this PR: `tests/test_antilag.py` and `nx2/test_nup_out_of_space.py` fail identically in a clean `origin/main` worktree at the base commit (the first is main's present-ahead refactor moving the Anti-Lag call out of `present_image`), `nx2/test_nup_paths.py` needs symlink privileges this PC does not grant, `tests/test_wayland_libdecor_hint.py` trips a clang-only `-Wunused-but-set-global`, and `tests/test_precompile_compute_guard.py` needs `windows.h` for its `--target=x86_64-pc-windows-msvc` fixture. The four checks this PR adds, and the capture checks it adapts, all pass. Every tool against the real game, over stdio (34 checks): | Tool | What was verified | |---|---| | `launch` | cold run reached its first present at 85 s while 38,809 programs were prepared, warm run ~4 s; pid, run dir, log, input and request paths returned; `wait_ready` works | | `status` | correct before launch, while running (renderer `Vulkan (hookshot_vk.dll)`, uptime, last screenshot) and after stop | | `screenshot` | 1280x720 PNG inline and to a chosen path, `present` parsed (e.g. "present 116"); the picture is the presented frame | | `press` | `A` acknowledged in the game log, and it started New Game — the frame changed from the title screen to the intro cutscene | | `stick` | left and right acknowledged; `LX=`, `LY=`, `RX=` in the log; the frame changed | | `sequence` | a three-step line queued and acknowledged (`0:DOWN/120,400:DOWN/120,800:A/150`); the frame changed | | `wait` | returns status; uptime advances | | `logs` | filtered (`hid: live`) and unfiltered; every injected line appears | | `stop` | clean `!quit`, exit code 0, process gone; a second `stop` is safe | | resources | status (JSON), log (text/plain), last-screenshot (image/png, 1280x720) | | error paths | before launch, duplicate launch, unknown button, centred stick, reserved `extra_env`, after stop — all clear messages | The visual half of that run was checked too: title screen, then the cutscene after `A`, then further changes after `stick` and `sequence` — 12/12 visual checks. One detail worth knowing (now in the docs): the very first `wait_ready` screenshot of a cold run is a black boot frame, because it returns on the first present while the shaders are still being prepared; the run above waited for the title before asserting on the picture. The screenshots stayed on the development disk; no game-derived file is in this PR. Finally the server was installed as a project MCP server in OpenCode (V2 `mcp.servers` configuration) and the nine tools were called through OpenCode itself — `status`, `launch` (with `wait_ready`), `wait`, `press`, `stick`, `sequence`, `screenshot`, `logs`, `stop` — all returned the expected results, and the run ended with exit code 0. ## Bug found and fixed by this testing `--game-dir` and `HOOKSHOT_MCP_GAME_DIR` selected the folder's `hookshot.exe`. In a folder made by the builder or `make_game.py` that is the setup launcher, which opens the installed builder (`builder/setup/setup.c`), not the game in `bin\`; `launch` would have started the builder setup and no screenshot would ever have arrived. `resolve_paths` now prefers `bin\hookshot.exe` and falls back to the top of the folder for a development build, and `tests/test_mcp_server.py` covers both layouts. `docs/mcp.md` also now gives the V2 OpenCode configuration (`mcp.servers`, no `enabled` field) and notes that `wait_ready` returns on the first present, which on a cold cache is a boot frame while the shaders are still being prepared. ## On the CLI-first feedback in #764 The points raised there — a CLI, files instead of inline images, state in run folders, waits on game state, a GPU lock — are reasonable, and this keeps the door open for them. Every tool is a thin call into `GameSession`; the run state already lives in a run folder (`log.txt`, `input.txt`, `shot.request`, `shots/`); `screenshot(inline=false)` returns a path instead of image content; and input waits for the game's acknowledgement line rather than a fixed sleep. A CLI front end over the same functions, a wait-on-log-state tool and a GPU lock are the natural next steps, and none of them need this PR to change shape. ## Settings and the cheat menu (added since this PR opened) - `settings()` reads every row of the Insert menu — Graphics, Gameplay and Controls — with its key, label, value, range and the text the player sees. When the cheat menu is on it also returns the Cheats page's eleven rows, the pouch's items, the 31 teleport destinations and the inventory's 85 items with their counts. - `set_setting(name, value)` goes through the menu's own setter: it clamps, applies a quality preset or a control layout, marks the settings Custom and writes `hookshot_options.ini`. A row that says it needs a restart still does. - `cheat(name, value, item)` runs a cheat row directly: heart protection, no-clip flight and the free camera; mark and return; the clock; teleports by name or index; the age; the pouch; the inventory (a wanted count, or -1 to unlock/relock). - `launch(cheats=true)` turns the cheat menu on with `HOS_OOT_CHEATS=1`, which is otherwise a line in `hookshot_env.txt` before start. On the game side these are three live-input commands — `!settings`, `!setting <key> <value>`, `!cheat <name> <value> <extra>` — answered in the game log, which the server reads back. The rows cross the renderer table (API 8) from `gpu/vk/vk_menu.cpp` through `host/rx_backend.c` to `host/nn_hid.c`. The WebGPU back end has no Insert menu and says so. Testing found and fixed one real defect: two cheat actions in quick succession overwrote each other (the menu kept a single atomic), so an inventory edit sent right after a pouch edit was lost. They are a queue now, and both are taken. Real-game verification against a build of this branch (Vulkan, a copied save): 23 settings listed with ranges and text; `set_setting("frame_pacing", 1)`, `set_setting("text_speed", 3)` and `set_setting("frame_generation", 0)` applied, shown by `settings()` and written to `hookshot_options.ini`; an unknown key refused; heart protection and the free camera toggled and read back; `cheat("set_time", 18)` made the game log "cheats: requested 18:00"; `cheat("pouch", item="Bombs", value=5)` logged "cheats: pouch Item_Bomb +5"; `cheat("teleport", item="Kakariko Village")` moved Link from Death Mountain Trail to Kakariko Village (screenshot, and "cheats: teleport child to KakarikoVillage_Child ... request sent"); `mark_position` logged the position; two actions sent back to back both ran. The game-free suite: 301 passed, 5 failed, 20 skipped — the five are main's own or this PC's toolchain, as described above. ## Not included No game files, dumps, builds, saves, logs or screenshots. Test runs stayed under `work/`, which is ignored.

hckrman101 (Migrated from github.com) reviewed

2026-11-09 06:48:12 +00:00

hckrman101 commented

2026-11-09 18:39:57 +00:00

(Migrated from github.com)

Hi! This PR has merge conflicts with main now (a lot has landed today). Could you merge main in or rebase and resolve them? Happy to give it a proper review once it's up to date.

Hi! This PR has merge conflicts with `main` now (a lot has landed today). Could you merge `main` in or rebase and resolve them? Happy to give it a proper review once it's up to date.

nicholasgower commented

2026-11-09 20:31:29 +00:00

(Migrated from github.com)

As of now, all conflicts have been resolved.

hckrman101 (Migrated from github.com) reviewed

2026-11-09 23:23:57 +00:00

hckrman101 (Migrated from github.com) left a comment

One thing to fix before merge: the Linux Python source checks job fails on the new tests/test_docs.py (docs/development.md:14: link does not resolve: DESIGN.md). The file is docs/design.md, so the link only resolves on case-insensitive filesystems (Windows passes). That link is already on main, but your new test is what trips on it. Changing the link to design.md (line 14, and the DESIGN.md table entry around line 55) should fix it. The two known main failures (test_output_resolution / test_program_cache_cleanup) are not the cause.

Things I checked and am fine with:

Game-side changes are HOS_INPUT_LIVE / RX_VK_SHOT_FILE / RX_SCREENSHOT_FILE / HOS_INPUT_SCRIPT_ONLY / HOOKSHOT_NO_ENV_FILE gated; unset means no behaviour change. !cheat <number> keeps its old meaning.
tests/test_mcp_server.py, test_docs.py and test_shot_request.py are registered in tests/run_all.py; mcp is only needed for those two tests.

Notes, not blockers:

Trust model: the server runs as the user and trusts its MCP client. launch takes arbitrary exe and extra_env (only the control vars are reserved, so e.g. LD_PRELOAD or PATH pass through), and screenshot(path=...) unlinks and then writes whatever path it is given. Fine for a local dev tool, but worth one line in docs/mcp.md so nobody exposes it to an untrusted host.
!quit calls exit(0) from the live-input thread. You flush the log first, which is good; just be aware it skips any other shutdown work.
MKWVK_API_VERSION goes 9 to 10, so the exe and hookshot_vk.dll must be rebuilt together (expected, just flagging).
mcp==2.3.0 is a new pip dependency in the CI workflow; I did not verify that pin.
Nit: the PR body says '19 changed paths' but the PR now touches 23 files, so the description is a bit stale.

Thanks, this is a well-scoped change, and the split is clean: the MCP server is a separate stdio process (no network port, no auth surface), and the game-side hooks are all env-gated, so a normal run is unaffected. One thing to fix before merge: the Linux `Python source checks` job fails on the new `tests/test_docs.py` (`docs/development.md:14: link does not resolve: DESIGN.md`). The file is `docs/design.md`, so the link only resolves on case-insensitive filesystems (Windows passes). That link is already on main, but your new test is what trips on it. Changing the link to `design.md` (line 14, and the `DESIGN.md` table entry around line 55) should fix it. The two known main failures (test_output_resolution / test_program_cache_cleanup) are not the cause. Things I checked and am fine with: - Game-side changes are `HOS_INPUT_LIVE` / `RX_VK_SHOT_FILE` / `RX_SCREENSHOT_FILE` / `HOS_INPUT_SCRIPT_ONLY` / `HOOKSHOT_NO_ENV_FILE` gated; unset means no behaviour change. `!cheat <number>` keeps its old meaning. - `tests/test_mcp_server.py`, `test_docs.py` and `test_shot_request.py` are registered in `tests/run_all.py`; `mcp` is only needed for those two tests. Notes, not blockers: - Trust model: the server runs as the user and trusts its MCP client. `launch` takes arbitrary `exe` and `extra_env` (only the control vars are reserved, so e.g. `LD_PRELOAD` or `PATH` pass through), and `screenshot(path=...)` unlinks and then writes whatever path it is given. Fine for a local dev tool, but worth one line in `docs/mcp.md` so nobody exposes it to an untrusted host. - `!quit` calls `exit(0)` from the live-input thread. You flush the log first, which is good; just be aware it skips any other shutdown work. - `MKWVK_API_VERSION` goes 9 to 10, so the exe and `hookshot_vk.dll` must be rebuilt together (expected, just flagging). - `mcp==2.3.0` is a new pip dependency in the CI workflow; I did not verify that pin. - Nit: the PR body says '19 changed paths' but the PR now touches 23 files, so the description is a bit stale.

nicholasgower commented

2026-11-09 23:34:21 +00:00

(Migrated from github.com)

Thanks for the review. All addressed, plus the branch is rebased again onto current main (581afaf) — it had fallen behind again after the last comment, and the conflicts are gone.

The Linux Python source checks failure: docs/development.md now links [design notes](design.md) and its table entry says design.md, so the link resolves on a case-sensitive filesystem. python -I -B tests/test_docs.py passes here (Windows), and the path now matches exactly.
Trust model: docs/mcp.md has a new line under Safety and limits: the server runs as the user and trusts its host, launch starts any executable and passes extra_env through (only the control switches are reserved, so PATH / LD_PRELOAD included), and screenshot unlinks and writes any path it is given — use it with a trusted host, not an untrusted client.
!quit: intentional for a test run (flush, then exit(0); the server falls back to terminate/kill if the game does not go). It is not used outside test runs.
MKWVK_API_VERSION 9 → 10: yes, the executable and hookshot_vk.dll are always built together; the table gained settings_dump, setting_set and cheat.
mcp==2.3.0: verified here — the server and the new tests run on 2.3.0 (importlib.metadata.version("mcp") reports 2.3.0).
PR body: updated, it now says 24 changed paths, the current suite result (178 passed, 11 failed, 16 skipped on Windows) and the current base.

The 11 failures here are environment gaps (Vulkan/SPIR-V and Windows SDK headers, symlink privileges, and clang fixture quirks) and fail the same way on a clean origin/main worktree.

Thanks for the review. All addressed, plus the branch is rebased again onto current `main` (`581afaf`) — it had fallen behind again after the last comment, and the conflicts are gone. - **The Linux `Python source checks` failure**: `docs/development.md` now links `[design notes](design.md)` and its table entry says `design.md`, so the link resolves on a case-sensitive filesystem. `python -I -B tests/test_docs.py` passes here (Windows), and the path now matches exactly. - **Trust model**: `docs/mcp.md` has a new line under Safety and limits: the server runs as the user and trusts its host, `launch` starts any executable and passes `extra_env` through (only the control switches are reserved, so `PATH` / `LD_PRELOAD` included), and `screenshot` unlinks and writes any path it is given — use it with a trusted host, not an untrusted client. - **`!quit`**: intentional for a test run (flush, then `exit(0)`; the server falls back to terminate/kill if the game does not go). It is not used outside test runs. - **`MKWVK_API_VERSION` 9 → 10**: yes, the executable and `hookshot_vk.dll` are always built together; the table gained `settings_dump`, `setting_set` and `cheat`. - **`mcp==2.3.0`**: verified here — the server and the new tests run on 2.3.0 (`importlib.metadata.version("mcp")` reports 2.3.0). - **PR body**: updated, it now says 24 changed paths, the current suite result (178 passed, 11 failed, 16 skipped on Windows) and the current base. The 11 failures here are environment gaps (Vulkan/SPIR-V and Windows SDK headers, symlink privileges, and clang fixture quirks) and fail the same way on a clean `origin/main` worktree.

nicholasgower commented

2026-11-10 03:00:45 +00:00

(Migrated from github.com)

Follow-up: what the latest commits on the branch are, after the rebase onto current main (9a2e80e; head a7ce62d, 11 commits, 28 files).

Since the review

feat(scripts): add a scenario runner for scripted game checks - scripts/mcp_scenario.py runs a JSON scenario through the server's own session code (press, stick, sequence, wait, screenshot, settings, set_setting, cheat, expect_log), fails on the first unacknowledged input or unmatched expectation, writes a transcript and screenshots under work/mcp/scenarios/<name>/, and --build rebuilds with HOOKSHOT_KEEP_BUILD_FILES=1. The fake game moved to tests/fake_game.py; tests/test_mcp_scenario.py covers a passing run and an impossible expectation. This commit also adds the docs: a "Driving the game" guide in docs/mcp.md (start-up timing, the remake's control map, the pause menu's rules, what persists, the build/run loop) and a "Scenario file" section, plus the command in CONTRIBUTING.md.
tests/test_links_case.py (same commit) - fails a relative link whose path case differs from the file on disk. That is the DESIGN.md vs design.md failure the Linux job hit; the check is stdlib-only and does not need the mcp package, so it runs where test_docs.py skips.
docs: wait for the automatic checks after opening a pull request - the contributing rule to wait for Actions and push fixes for failures.
docs: fix the design link's case and note the MCP trust model - the docs/development.md link fix (the CI blocker) and the trust-model line in docs/mcp.md.

Before those

docs: map the in-game actions behind the MCP tools (docs/actions.md) - the research map for equipment, the held item and the ocarina: the action classes and entry functions, the pause-menu selection chain, the item grid, the player equipment words and the ocarina manager. The three tools themselves are not in this PR yet; this is the write-up of what was found and what is still open.
docs: require an MCP tool for every player action - the design rule in docs/design.md section 8 and CONTRIBUTING.md.
feat(mcp): drive the Insert menu's settings and the cheat menu from MCP - settings, set_setting, cheat, and launch(cheats=true).
Earlier: the server itself, the screenshot-request test, the docs-table test, the pre-PR check, and the --game-dir fix for builder-layout folders.

Checks: the previous head passed every job, including Python source checks (ubuntu-24.04); the run for this head is in progress, and I will push fixes if anything fails.

Follow-up: what the latest commits on the branch are, after the rebase onto current `main` (`9a2e80e`; head `a7ce62d`, 11 commits, 28 files). **Since the review** - `feat(scripts): add a scenario runner for scripted game checks` - `scripts/mcp_scenario.py` runs a JSON scenario through the server's own session code (`press`, `stick`, `sequence`, `wait`, `screenshot`, `settings`, `set_setting`, `cheat`, `expect_log`), fails on the first unacknowledged input or unmatched expectation, writes a transcript and screenshots under `work/mcp/scenarios/<name>/`, and `--build` rebuilds with `HOOKSHOT_KEEP_BUILD_FILES=1`. The fake game moved to `tests/fake_game.py`; `tests/test_mcp_scenario.py` covers a passing run and an impossible expectation. This commit also adds the docs: a "Driving the game" guide in `docs/mcp.md` (start-up timing, the remake's control map, the pause menu's rules, what persists, the build/run loop) and a "Scenario file" section, plus the command in `CONTRIBUTING.md`. - `tests/test_links_case.py` (same commit) - fails a relative link whose path case differs from the file on disk. That is the `DESIGN.md` vs `design.md` failure the Linux job hit; the check is stdlib-only and does not need the `mcp` package, so it runs where `test_docs.py` skips. - `docs: wait for the automatic checks after opening a pull request` - the contributing rule to wait for Actions and push fixes for failures. - `docs: fix the design link's case and note the MCP trust model` - the `docs/development.md` link fix (the CI blocker) and the trust-model line in `docs/mcp.md`. **Before those** - `docs: map the in-game actions behind the MCP tools` (`docs/actions.md`) - the research map for equipment, the held item and the ocarina: the action classes and entry functions, the pause-menu selection chain, the item grid, the player equipment words and the ocarina manager. The three tools themselves are not in this PR yet; this is the write-up of what was found and what is still open. - `docs: require an MCP tool for every player action` - the design rule in `docs/design.md` section 8 and `CONTRIBUTING.md`. - `feat(mcp): drive the Insert menu's settings and the cheat menu from MCP` - `settings`, `set_setting`, `cheat`, and `launch(cheats=true)`. - Earlier: the server itself, the screenshot-request test, the docs-table test, the pre-PR check, and the `--game-dir` fix for builder-layout folders. Checks: the previous head passed every job, including `Python source checks (ubuntu-24.04)`; the run for this head is in progress, and I will push fixes if anything fails.

hckrman101 (Migrated from github.com) reviewed

2026-11-10 03:37:36 +00:00

hckrman101 (Migrated from github.com) left a comment

No description provided.

nicholasgower commented

2026-11-10 04:16:18 +00:00

(Migrated from github.com)

A control bridge is out of scope of this PR.

nicholasgower commented

2026-11-10 06:43:25 +00:00

(Migrated from github.com)

Check status on this head (837a54b, rebased onto main at b2e1c5a6).

Passing: Windows Vulkan back end, AppImage, Hookshot Builder package, Native F2I lowering, workflow lint. The earlier Windows Vulkan failure was damage from the main merge (two tx_deadzone definitions, menu_take_cheat_action on the old cheat_action instead of the queue, and cheats_enabled where main now says cheats_on()); fixed in this head and compile-checked with the job's own compiler and Vulkan headers.

Failing, in the Python source checks: none of the tests this PR adds. The six failures (test_bc_raw_targets, test_display_startup_failure, test_parallel_init_order, test_parallel_rb_refresh, test_present_aspect, test_present_semaphores) reproduce identically on a clean origin/main worktree at b2e1c5a6, so they are main's fixture/source mismatches. Ubuntu adds test_dlss_actor (the checked-in shaders/dlss_actor_frag_spv.h no longer matches dlss_actor.frag) and test_menu_font_latin (fontconfig asked for a different font), both unrelated to this branch.

The PR's own tests pass on both platforms: test_mcp_server, test_mcp_scenario, test_docs, test_links_case.

Check status on this head (`837a54b`, rebased onto `main` at `b2e1c5a6`). **Passing:** Windows Vulkan back end, AppImage, Hookshot Builder package, Native F2I lowering, workflow lint. The earlier Windows Vulkan failure was damage from the `main` merge (two `tx_deadzone` definitions, `menu_take_cheat_action` on the old `cheat_action` instead of the queue, and `cheats_enabled` where `main` now says `cheats_on()`); fixed in this head and compile-checked with the job's own compiler and Vulkan headers. **Failing, in the Python source checks:** none of the tests this PR adds. The six failures (`test_bc_raw_targets`, `test_display_startup_failure`, `test_parallel_init_order`, `test_parallel_rb_refresh`, `test_present_aspect`, `test_present_semaphores`) reproduce identically on a clean `origin/main` worktree at `b2e1c5a6`, so they are main's fixture/source mismatches. Ubuntu adds `test_dlss_actor` (the checked-in `shaders/dlss_actor_frag_spv.h` no longer matches `dlss_actor.frag`) and `test_menu_font_latin` (fontconfig asked for a different font), both unrelated to this branch. The PR's own tests pass on both platforms: `test_mcp_server`, `test_mcp_scenario`, `test_docs`, `test_links_case`.

nicholasgower commented

2026-11-10 08:02:23 +00:00

(Migrated from github.com)

Rebased with upstream main.

nicholasgower commented

2026-11-10 17:14:33 +00:00

(Migrated from github.com)

I've used this PR to test game mechanics with raw user input based on deterministic input. As far as I can tell, this MCP server is ready for others to use and add features to. Let me know if you've tested this server and found something lacking.

nicholasgower commented

2026-11-10 21:46:59 +00:00

(Migrated from github.com)

I'm not planning on rebasing this any more. If a project maintainer would like to merge this, they will need to fix merge conflicts themselves.

Joey-Magnus (Migrated from github.com) requested changes

2026-11-11 21:45:49 +00:00

Joey-Magnus (Migrated from github.com) left a comment

vk_api.h:192-194 adds settings_dump, setting_set and cheat to MkwVkRenderer, but the diff doesn't touch MKWVK_API_VERSION, which is 17 on main. The earlier 9-to-10 bump was lost in a rebase. It needs a bump, and 18 is already claimed by several open PRs, so check for a collision.
nn_hid.c:856 reads HOS_INPUT_SCRIPT_ONLY with getenv(...) != NULL, so setting it to =0 turns it on. Since #2042 this should use env_on.
!quit calls exit(0) from the live-input thread (nn_hid.c:411). That's fine for a throwaway save folder, but it can interrupt a save write that's in progress.

The PR also bundles process changes that deserve their own decision: a design.md rule that every player action needs an MCP tool, a CONTRIBUTING rule, and scripts/pr_check.py. I'd split those out from the server itself.

The author has said they won't rebase this again, so whoever picks it up will need to resolve conflicts in vk_api.h, vk_menu.cpp, vk_menu.h, vk_replay.cpp, nn_hid.c, rx_backend.c, CONTRIBUTING.md and the source-regressions workflow. Before that happens, a few things in the current head need fixing: - vk_api.h:192-194 adds `settings_dump`, `setting_set` and `cheat` to `MkwVkRenderer`, but the diff doesn't touch `MKWVK_API_VERSION`, which is 17 on main. The earlier 9-to-10 bump was lost in a rebase. It needs a bump, and 18 is already claimed by several open PRs, so check for a collision. - nn_hid.c:856 reads `HOS_INPUT_SCRIPT_ONLY` with `getenv(...) != NULL`, so setting it to `=0` turns it on. Since #2042 this should use env_on. - `!quit` calls `exit(0)` from the live-input thread (nn_hid.c:411). That's fine for a throwaway save folder, but it can interrupt a save write that's in progress. The PR also bundles process changes that deserve their own decision: a design.md rule that every player action needs an MCP tool, a CONTRIBUTING rule, and `scripts/pr_check.py`. I'd split those out from the server itself.

github-sync referenced this pull request

2026-11-12 07:27:36 +00:00

feat(control): the Debug tab's Switch age over the bridge, so scripts reach the adult's maps #2694

github-sync referenced this pull request from a commit

2026-11-12 16:54:32 +00:00

feat(control): the Debug tab's Switch age over the bridge, so scripts reach the adult's maps (#2694)

github-sync referenced this pull request

2026-11-12 17:57:29 +00:00

ci: execute authored Vulkan fixtures on lavapipe #2866

github-sync commented

2026-11-12 17:57:37 +00:00

Owner

@mooki000 commented on GitHub · 2026-11-12 17:54 UTC · original

@nicholasgower, @Joey-Magnus: I would like to pick this up, since it is waiting for someone to rebase it. Nicholas, your commits keep your name, and if you would rather keep the pull request yourself, say so and I stand down.

What I plan, following the two reviews here and hckrman101's note on #764:

Rest it on the control bridge that has landed since this was written (docs/control.md, scripts/hookshot_ctl.py). The bridge already gives timed presses, sticks and sequences with replies, screenshots, the menu's options, teleport and age, waits and a clean quit. So the server becomes a thin layer over that client, and the game-side changes here are no longer needed: the three renderer-table entries and their API version bump, the !quit that exits from the input thread, the one-shot screenshot files. That removes every conflict Joey listed in vk_api.h, vk_menu.cpp, vk_menu.h, vk_replay.cpp, nn_hid.c and rx_backend.c. Where a switch is still read, it goes through env_on.
Command line first, MCP on top: every tool is a call the command-line client can already make, so nothing is MCP-only.
The process rules split out, as Joey asked: the design.md rule, the CONTRIBUTING rule and scripts/pr_check.py are not part of it.
Keep what is good here: the scenario runner, the fake game for game-free tests, the docs tables checked against the live server, the trust-model paragraph.
More than the nine tools, once that base is in: state without screenshots (map, position, loading, age from the bridge), waits on game state, saves, a run folder for each session, and a GPU lock for PCs that run several games at once.

It will come as a new pull request that names this one and carries its history, with a real-game run on Linux as evidence. I will say here when it is up.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 17:54 UTC · [original](https://github.com/NotRexed/Hookshot/pull/1025#issuecomment-6831552689) @nicholasgower, @Joey-Magnus: I would like to pick this up, since it is waiting for someone to rebase it. Nicholas, your commits keep your name, and if you would rather keep the pull request yourself, say so and I stand down. What I plan, following the two reviews here and hckrman101's note on #764: 1. **Rest it on the control bridge** that has landed since this was written (`docs/control.md`, `scripts/hookshot_ctl.py`). The bridge already gives timed presses, sticks and sequences with replies, screenshots, the menu's options, teleport and age, waits and a clean quit. So the server becomes a thin layer over that client, and the game-side changes here are no longer needed: the three renderer-table entries and their API version bump, the `!quit` that exits from the input thread, the one-shot screenshot files. That removes every conflict Joey listed in `vk_api.h`, `vk_menu.cpp`, `vk_menu.h`, `vk_replay.cpp`, `nn_hid.c` and `rx_backend.c`. Where a switch is still read, it goes through `env_on`. 2. **Command line first, MCP on top**: every tool is a call the command-line client can already make, so nothing is MCP-only. 3. **The process rules split out**, as Joey asked: the `design.md` rule, the CONTRIBUTING rule and `scripts/pr_check.py` are not part of it. 4. **Keep what is good here**: the scenario runner, the fake game for game-free tests, the docs tables checked against the live server, the trust-model paragraph. 5. **More than the nine tools**, once that base is in: state without screenshots (map, position, loading, age from the bridge), waits on game state, saves, a run folder for each session, and a GPU lock for PCs that run several games at once. It will come as a new pull request that names this one and carries its history, with a real-game run on Linux as evidence. I will say here when it is up.

github-sync commented

2026-11-12 17:57:38 +00:00

Owner

@mooki000 commented on GitHub · 2026-11-12 17:57 UTC · original

A correction to my note above, before anyone relies on it: I called the bridge's quit "clean". It is cleaner than !quit in that it does not exit from the input thread, but it does not wait for a save in progress either: it asks the renderer to close and then exits the process (host/control.c, the quit command). So Joey's third point holds for a server on the bridge too. The new pull request will deal with it there, not claim it solved.

Also for scope: the bridge's option_get leaves out the Debug tab's cheat rows, so of this pull request's cheats only teleport and age have a bridge equivalent today. Whether the others belong on the bridge or stay out is a question I will put in the new pull request rather than decide quietly.

> [@mooki000](https://github.com/mooki000) commented on GitHub · 2026-11-12 17:57 UTC · [original](https://github.com/NotRexed/Hookshot/pull/1025#issuecomment-6831627057) A correction to my note above, before anyone relies on it: I called the bridge's quit "clean". It is cleaner than `!quit` in that it does not exit from the input thread, but it does not wait for a save in progress either: it asks the renderer to close and then exits the process (`host/control.c`, the `quit` command). So Joey's third point holds for a server on the bridge too. The new pull request will deal with it there, not claim it solved. Also for scope: the bridge's `option_get` leaves out the Debug tab's cheat rows, so of this pull request's cheats only teleport and age have a bridge equivalent today. Whether the others belong on the bridge or stay out is a question I will put in the new pull request rather than decide quietly.

Commenting is not possible because the repository is archived.

No reviewers

hckrman101

Joey-Magnus