diff --git a/README.md b/README.md index 5d8d1f8..06a1381 100644 --- a/README.md +++ b/README.md @@ -33,20 +33,21 @@ The game only listens on 127.0.0.1. **3. Register the MCP server.** Requires [uv](https://docs.astral.sh/uv/). -Claude Code: +Claude Code (run from this repo's root): ```bash -claude mcp add factorio --scope user -e FACTORIO_UDP_PORT=34200 -- uv run --directory /home/faab/Documents/Code/Factorio/claude-co-engineer/server claude-co-engineer-mcp +claude mcp add factorio --scope user -e FACTORIO_UDP_PORT=34200 -- uv run --directory "$PWD/server" claude-co-engineer-mcp ``` -Claude Desktop — add to `claude_desktop_config.json` (Settings → Developer → Edit Config), then restart it: +Claude Desktop — add to `claude_desktop_config.json` (Settings → Developer → Edit Config), with +`/path/to/claude-co-engineer` replaced by where you cloned this repo, then restart it: ```json { "mcpServers": { "factorio": { "command": "uv", - "args": ["run", "--directory", "/home/faab/Documents/Code/Factorio/claude-co-engineer/server", "claude-co-engineer-mcp"], + "args": ["run", "--directory", "/path/to/claude-co-engineer/server", "claude-co-engineer-mcp"], "env": { "FACTORIO_UDP_PORT": "34200" } } } @@ -86,6 +87,10 @@ default, and are purely visual. your world changes, turn off **Allow Lua queries** in Settings → Mod settings → Map. Every query is logged to `factorio-current.log`. +In multiplayer, `run_lua` is off unless an admin also turns on **Allow Lua queries in multiplayer**: +the Lua runs for everyone, and text other players put in the world (station names, labels) ends up +in what Claude reads, so it could be talked into changing things. + ## Asking from inside the game Type `/ask ` in the game chat (e.g. `/ask why is my green science slow?`). The answer @@ -100,16 +105,17 @@ the places it mentions get highlighted. Follow-up questions continue the same co This needs: - the `claude` CLI logged in with your subscription (`claude auth login`, once); -- the helper running. Either start it by hand: +- the helper running. Either start it by hand from this repo's root: ```bash - ~/Documents/Code/Factorio/claude-co-engineer/server/.venv/bin/claude-co-engineer-ask + uv run --directory server claude-co-engineer-ask ``` - or install it as a systemd user service that starts at login: + or install it as a systemd user service that starts at login (from this repo's root; this fills in + its path): ```bash - cp server/claude-co-engineer-ask.service ~/.config/systemd/user/ && systemctl --user daemon-reload && systemctl --user enable --now claude-co-engineer-ask + uv sync --directory server && mkdir -p ~/.config/systemd/user && sed "s|/path/to/claude-co-engineer|$PWD|" server/claude-co-engineer-ask.service > ~/.config/systemd/user/claude-co-engineer-ask.service && systemctl --user daemon-reload && systemctl --user enable --now claude-co-engineer-ask ``` Its log shows each question and answer: `journalctl --user -u claude-co-engineer-ask -f`. @@ -120,7 +126,7 @@ your plan's usage. It's per player, so in multiplayer everyone picks their own. The dropdown entries are aliases, which the `claude` CLI resolves to the newest model of that name, so a new Sonnet or Opus is used automatically. To force any other model, uncomment -`FACTORIO_ASK_MODEL_OVERRIDE` in `server/claude-co-engineer-ask.service` (an alias or a full model ID) +`FACTORIO_ASK_MODEL_OVERRIDE` in `~/.config/systemd/user/claude-co-engineer-ask.service` (an alias or a full model ID) and run `systemctl --user daemon-reload && systemctl --user restart claude-co-engineer-ask`. It wins over the in-game setting for everyone. @@ -128,7 +134,8 @@ Claude only gets the Factorio tools in these sessions: no shell, files or web. I running, the game tells you after 3 seconds. The helper's port is a map setting (Ask helper port, default 34201). Helper environment variables: `FACTORIO_ASK_PORT`, `FACTORIO_ASK_MODEL` (model when the game doesn't send one, default `sonnet`), `FACTORIO_ASK_MODEL_OVERRIDE` (always use this model), -`FACTORIO_ASK_TIMEOUT` (300 s), `FACTORIO_ASK_CLAUDE` (path to the CLI). +`FACTORIO_ASK_TIMEOUT` (300 s), `FACTORIO_ASK_MAX_CONCURRENT` (Claude runs at once across all players, +default 2), `FACTORIO_ASK_CLAUDE` (path to the CLI). The helper only accepts packets from the game's UDP port. ## Ratio calculator diff --git a/mod/claude-co-engineer/control.lua b/mod/claude-co-engineer/control.lua index 0e67d9a..bdc93a4 100644 --- a/mod/claude-co-engineer/control.lua +++ b/mod/claude-co-engineer/control.lua @@ -289,9 +289,18 @@ end -- --------------------------------------------------------------------------------------------- -- Query handlers. Everything here only reads game state, except run_lua which runs whatever it's given. +-- Falls back to the default if the setting isn't registered (e.g. settings.lua changed since startup). +local function global_setting(name, default) + local setting = settings.global[name] + if setting == nil then return default end + return setting.value +end + +-- In multiplayer, run_lua runs on every peer and could be steered by text other players put in the world +-- (station names, labels), so it also needs the multiplayer setting, which only admins can change. local function lua_allowed() - local setting = settings.global["claude-co-engineer-allow-lua"] - return setting == nil or setting.value + if not global_setting("claude-co-engineer-allow-lua", true) then return false end + return not game.is_multiplayer() or global_setting("claude-co-engineer-allow-lua-multiplayer", false) end function handlers.ping(args, event) @@ -1595,7 +1604,9 @@ function handlers.screenshot(args, event) local center = (args.x and args.y) and { x = args.x, y = args.y } or ctx.origin -- At zoom 1 a tile is 32 pixels. local zoom = width / (width_tiles * 32) - local path = "claude-co-engineer/" .. (args.name or ("shot-" .. game.tick)) .. ".jpg" + local name = args.name or ("shot-" .. game.tick) + if type(name) ~= "string" or not name:match("^[%w-]+$") then error("name must be letters, digits and dashes", 0) end + local path = "claude-co-engineer/" .. name .. ".jpg" game.take_screenshot({ -- In multiplayer only the requesting instance renders; a headless server can't render at all. @@ -1710,6 +1721,9 @@ end function handlers.run_lua(args, event) if not lua_allowed() then + if game.is_multiplayer() then + error("Lua queries are disabled in multiplayer unless an admin enables 'Allow Lua queries in multiplayer' in the mod settings", 0) + end error("Lua queries are disabled in the mod settings (Allow Lua queries)", 0) end local code = args.code or error("code is required") @@ -1783,10 +1797,8 @@ function handlers.ask_answer(args, event) return {} end --- Falls back to the default if the setting isn't registered (e.g. settings.lua changed since startup). local function ask_port() - local setting = settings.global["claude-co-engineer-ask-port"] - return setting and setting.value or 34201 + return global_setting("claude-co-engineer-ask-port", 34201) end local function send_to_helper(player, payload) diff --git a/mod/claude-co-engineer/locale/en/locale.cfg b/mod/claude-co-engineer/locale/en/locale.cfg index 2603e41..516dadc 100644 --- a/mod/claude-co-engineer/locale/en/locale.cfg +++ b/mod/claude-co-engineer/locale/en/locale.cfg @@ -1,10 +1,12 @@ [mod-setting-name] claude-co-engineer-allow-lua=Allow Lua queries +claude-co-engineer-allow-lua-multiplayer=Allow Lua queries in multiplayer claude-co-engineer-ask-port=Ask helper port claude-co-engineer-ask-model=/ask model [mod-setting-description] claude-co-engineer-allow-lua=Lets the Co-Engineer run arbitrary Lua to answer questions the built-in queries can't. It is told to only read, but Lua can't be sandboxed to read-only; disable this if you want a hard guarantee that nothing changes. +claude-co-engineer-allow-lua-multiplayer=Also allow Lua queries in multiplayer games. Off by default: the Lua runs for everyone, and text other players put in the world (station names, labels) could talk Claude into changing things. claude-co-engineer-ask-port=UDP port the claude-co-engineer-ask helper listens on. /ask sends your question there. claude-co-engineer-ask-model=Which Claude model answers your /ask questions. Faster models use less of your plan's usage; larger ones give more thorough answers. diff --git a/mod/claude-co-engineer/settings.lua b/mod/claude-co-engineer/settings.lua index 2cd0b67..90c9f52 100644 --- a/mod/claude-co-engineer/settings.lua +++ b/mod/claude-co-engineer/settings.lua @@ -6,6 +6,14 @@ data:extend({ default_value = true, order = "a", }, + { + -- Runtime-global settings can only be changed by admins in multiplayer. + type = "bool-setting", + name = "claude-co-engineer-allow-lua-multiplayer", + setting_type = "runtime-global", + default_value = false, + order = "a-b", + }, { type = "int-setting", name = "claude-co-engineer-ask-port", diff --git a/server/claude-co-engineer-ask.service b/server/claude-co-engineer-ask.service index d3d055b..53b955b 100644 --- a/server/claude-co-engineer-ask.service +++ b/server/claude-co-engineer-ask.service @@ -2,7 +2,8 @@ Description=Claude Co-Engineer: answers in-game /ask questions with Claude [Service] -ExecStart=/home/faab/Documents/Code/Factorio/claude-co-engineer/server/.venv/bin/claude-co-engineer-ask +# /path/to/claude-co-engineer is replaced with the repo's location when installing (see README). +ExecStart=/path/to/claude-co-engineer/server/.venv/bin/claude-co-engineer-ask Environment=FACTORIO_UDP_PORT=34200 Environment=FACTORIO_ASK_PORT=34201 # Uncomment to force a model for every /ask, ignoring the in-game setting (alias or full model ID): diff --git a/server/claude_co_engineer/ask.py b/server/claude_co_engineer/ask.py index 6fe5d8f..c264b45 100644 --- a/server/claude_co_engineer/ask.py +++ b/server/claude_co_engineer/ask.py @@ -23,6 +23,8 @@ DEFAULT_MODEL = os.environ.get("FACTORIO_ASK_MODEL", "sonnet") # Wins over the in-game setting, e.g. to try a model the dropdown doesn't list yet. MODEL_OVERRIDE = os.environ.get("FACTORIO_ASK_MODEL_OVERRIDE") TIMEOUT = float(os.environ.get("FACTORIO_ASK_TIMEOUT", "300")) +MAX_CONCURRENT = int(os.environ.get("FACTORIO_ASK_MAX_CONCURRENT", "2")) +MAX_QUESTION = 2000 # characters; longer questions are cut off WORKDIR = Path(os.environ.get("FACTORIO_ASK_WORKDIR", "~/.cache/claude-co-engineer/ask")).expanduser() SYSTEM_PROMPT = """\ @@ -66,6 +68,8 @@ class AskServer(asyncio.DatagramProtocol): self.game = GameLink(GAME_PORT) self.sessions: dict[int, str] = {} # player_index -> Claude session id self.locks: dict[int, asyncio.Lock] = {} + self.running = asyncio.Semaphore(MAX_CONCURRENT) # claude processes across all players + self.tasks: set[asyncio.Task] = set() # keeps running asks referenced until they finish WORKDIR.mkdir(parents=True, exist_ok=True) self.mcp_config = WORKDIR / "mcp.json" self.mcp_config.write_text(json.dumps({ @@ -79,16 +83,25 @@ class AskServer(asyncio.DatagramProtocol): })) def datagram_received(self, data: bytes, addr) -> None: + # The mod sends from the game's --enable-lua-udp port; other local senders are ignored. + if addr[:2] != ("127.0.0.1", GAME_PORT): + return try: message = json.loads(data) except ValueError: return - player = message.get("player_index") + if not isinstance(message, dict) or not isinstance(player := message.get("player_index"), int): + return if message.get("type") == "reset": self.sessions.pop(player, None) log(f"player {player}: conversation reset") elif message.get("type") == "ask": - asyncio.ensure_future(self.handle_ask(message)) + ask_id, question = message.get("id"), message.get("question") + if not isinstance(ask_id, int) or not isinstance(question, str) or not question.strip(): + return + task = asyncio.ensure_future(self.handle_ask(ask_id, player, question[:MAX_QUESTION], message.get("model"))) + self.tasks.add(task) + task.add_done_callback(self.tasks.discard) async def send(self, op: str, **args) -> None: try: @@ -96,14 +109,13 @@ class AskServer(asyncio.DatagramProtocol): except GameError as e: log(f"couldn't reach the game for {op}: {e}") - async def handle_ask(self, message: dict) -> None: - ask_id, player, question = message["id"], message.get("player_index"), message["question"] - model = MODEL_OVERRIDE or message.get("model") or DEFAULT_MODEL - if not re.fullmatch(r"[\w.-]+", model): + async def handle_ask(self, ask_id: int, player: int, question: str, model: str | None) -> None: + model = MODEL_OVERRIDE or model or DEFAULT_MODEL + if not isinstance(model, str) or not re.fullmatch(r"[\w.-]+", model): model = DEFAULT_MODEL log(f"player {player} asks #{ask_id} ({model}): {question}") await self.send("ask_ack", ask_id=ask_id) - async with self.locks.setdefault(player, asyncio.Lock()): + async with self.locks.setdefault(player, asyncio.Lock()), self.running: try: answer, is_error = await asyncio.wait_for(self.run_claude(ask_id, player, question, model), TIMEOUT) except TimeoutError: @@ -114,8 +126,9 @@ class AskServer(asyncio.DatagramProtocol): await self.send("ask_answer", ask_id=ask_id, player_index=player, text=answer, error=is_error) async def run_claude(self, ask_id: int, player: int, question: str, model: str) -> tuple[str, bool]: + # The question goes in on stdin: as an argument, one starting with "-" would be parsed as a CLI option. args = [ - CLAUDE, "-p", question, + CLAUDE, "-p", "--model", model, "--mcp-config", str(self.mcp_config), "--strict-mcp-config", "--tools", "", # no shell, file or web tools; only the game @@ -127,9 +140,11 @@ class AskServer(asyncio.DatagramProtocol): args += ["--resume", self.sessions[player]] process = await asyncio.create_subprocess_exec( - *args, cwd=WORKDIR, stdin=asyncio.subprocess.DEVNULL, + *args, cwd=WORKDIR, stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, limit=16 * 1024 * 1024, ) + process.stdin.write(question.encode()) + process.stdin.close() stderr_task = asyncio.ensure_future(process.stderr.read()) result, last_status = None, None try: diff --git a/server/claude_co_engineer/game_link.py b/server/claude_co_engineer/game_link.py index c98642e..868c94b 100644 --- a/server/claude_co_engineer/game_link.py +++ b/server/claude_co_engineer/game_link.py @@ -18,7 +18,9 @@ class _Protocol(asyncio.DatagramProtocol): self.link = link def datagram_received(self, data: bytes, addr: Any) -> None: - self.link._on_packet(data) + # The game replies from its --enable-lua-udp port; ignore anything else on localhost. + if addr[:2] == self.link.peer: + self.link._on_packet(data) class GameLink: @@ -39,6 +41,8 @@ class GameLink: async def _ensure_open(self) -> asyncio.DatagramTransport: if self._transport is None or self._transport.is_closing(): loop = asyncio.get_running_loop() + # The address replies come from, resolved so a hostname like "localhost" matches. + self.peer = (socket.gethostbyname(self.host), self.game_addr[1]) sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) # Large replies arrive as a burst of ~32KB packets; give the kernel room to queue them. sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 8 * 1024 * 1024) diff --git a/server/claude_co_engineer/server.py b/server/claude_co_engineer/server.py index 586607a..46a356d 100644 --- a/server/claude_co_engineer/server.py +++ b/server/claude_co_engineer/server.py @@ -268,7 +268,9 @@ async def screenshot( meta = await game.request( "screenshot", x=x, y=y, width_tiles=width_tiles, alt_mode=alt_mode, daylight=daylight, surface=surface, name=name ) - path = SCRIPT_OUTPUT / meta.pop("path") + # Built from our own name rather than the reply, so a reply can't point the unlink below elsewhere. + meta.pop("path", None) + path = SCRIPT_OUTPUT / "claude-co-engineer" / f"{name}.jpg" try: data = await _read_finished_jpeg(path) finally: