From 6a76ed3be43fd85cf89bcd9c3957167dd910dce6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 7 Sep 2026 22:30:39 +0000 Subject: [PATCH 1/2] fix: preflight gates so keeper Crashlanded contract failures are not model noise Treat already-covered growing zones as success-by-state (check-zone / engine). Validate work_priority against WorkTypeDef names and stop sending id/priority as work types. Queue only available research. Require a living doctor/patient pair before tend. Hide and refuse stockpile_delete when the endpoint is missing. Co-authored-by: Jason --- src/rle/harness/brief.py | 4 +- src/rle/harness/felix/agents/base_role.py | 10 +- src/rle/mcp/server.py | 6 +- src/rle/mcp/session.py | 9 +- src/rle/orchestration/action_executor.py | 143 ++++++---- src/rle/orchestration/action_resolver.py | 14 +- src/rle/orchestration/game_loop.py | 2 +- src/rle/orchestration/preflight.py | 250 +++++++++++++++++ src/rle/rimapi/api_catalog.py | 60 +++- src/rle/rimapi/client.py | 30 ++ src/rle/testing/mock_rimapi.py | 6 + tests/unit/test_action_executor.py | 16 +- tests/unit/test_action_resolver.py | 13 + tests/unit/test_brief.py | 4 + tests/unit/test_mcp_server.py | 1 + tests/unit/test_preflight.py | 319 ++++++++++++++++++++++ tests/unit/test_rimapi_client.py | 17 ++ 17 files changed, 814 insertions(+), 90 deletions(-) create mode 100644 src/rle/orchestration/preflight.py create mode 100644 tests/unit/test_preflight.py diff --git a/src/rle/harness/brief.py b/src/rle/harness/brief.py index 14ccc19..54eca5d 100644 --- a/src/rle/harness/brief.py +++ b/src/rle/harness/brief.py @@ -14,7 +14,7 @@ from pydantic import BaseModel, ConfigDict -from rle.rimapi.api_catalog import WRITE_CATALOG +from rle.rimapi.api_catalog import visible_write_catalog from rle.rimapi.schemas import GameState from rle.rimapi.sse_client import RimAPIEvent from rle.scenarios.schema import ScenarioConfig @@ -110,7 +110,7 @@ def action_catalog() -> list[dict[str, Any]]: out: list[dict[str, Any]] = [ {"action_type": "no_action", "description": "Do nothing this tick.", "params": {}}, ] - for name, raw in sorted(WRITE_CATALOG.items()): + for name, raw in sorted(visible_write_catalog().items()): entry = cast(dict[str, Any], raw) out.append({ "action_type": name, diff --git a/src/rle/harness/felix/agents/base_role.py b/src/rle/harness/felix/agents/base_role.py index 290dd7a..eea9783 100644 --- a/src/rle/harness/felix/agents/base_role.py +++ b/src/rle/harness/felix/agents/base_role.py @@ -56,7 +56,8 @@ "- blueprint: Place a building. params: {\"def_name\": \"Wall\", \"x\": int, " "\"z\": int, \"stuff_def\": \"WoodLog\", \"rotation\": 0}.\n" "- growing_zone: Create growing zone. params: {\"plant_def\": \"Plant_Potato\", " - "\"x1\": int, \"z1\": int, \"x2\": int, \"z2\": int}.\n" + "\"x1\": int, \"z1\": int, \"x2\": int, \"z2\": int}. " + "If a growing zone already covers the cells, do not recreate it.\n" "- stockpile_zone: Create stockpile. params: {\"x1\": int, \"z1\": int, " "\"x2\": int, \"z2\": int, \"name\": \"Supply\"}.\n" "- designate_area: Mine/harvest/deconstruct. params: {\"type\": \"Mine\", " @@ -72,8 +73,9 @@ "- time_assignment: Set schedule. params: {\"hours\": [18,19,20], " "\"assignment\": \"Joy\"}. target_colonist_id required.\n" "- bed_rest: Assign bed rest. target_colonist_id required.\n" - "- tend: Have doctor tend patient. params: {\"doctor_pawn_id\": int}. " - "target_colonist_id = patient.\n" + "- tend: Have a living doctor tend a living patient. " + "params: {\"doctor_pawn_id\": int}. target_colonist_id = patient. " + "Both must be distinct living colonists.\n" "- toggle_power: Toggle building power. params: {\"building_id\": int, " "\"power_on\": true}.\n" "- no_action: Do nothing (use when colonists are already productive).\n\n" @@ -117,7 +119,7 @@ "best Construction→Construction=1, best Intellectual→Research=1\n" "- growing_zone: use FARM SITE coordinates from MAP_SUMMARY, " "plant_def=Plant_Rice (fastest food). Create this zone ONCE — if a " - "growing zone already exists, do NOT create another (it will fail).\n\n" + "growing zone already exists, do NOT create another.\n\n" "TICK 2 (SHELTER — most critical):\n" "- blueprint Wall: place 5x5 rectangle using SHELTER SITE from MAP_SUMMARY. " "Use stuff_def=WoodLog. Leave one gap for a Door.\n" diff --git a/src/rle/mcp/server.py b/src/rle/mcp/server.py index b730540..64dd939 100644 --- a/src/rle/mcp/server.py +++ b/src/rle/mcp/server.py @@ -1,6 +1,6 @@ """RimAPI as an MCP tool server (requires the ``mcp`` extra). -One tool per ``WRITE_CATALOG`` entry (executed immediately through +One tool per visible ``WRITE_CATALOG`` entry (executed immediately through ``ActionExecutor`` and recorded in the tick ledger), a generic read tool over ``READ_CATALOG``, the harness-neutral brief, and ``end_turn``. Any MCP-capable coding agent can attach and play; the harness packages that do so live in @@ -15,7 +15,7 @@ from mcp.server.mcpserver import MCPServer from rle.mcp.session import McpSession -from rle.rimapi.api_catalog import READ_CATALOG, WRITE_CATALOG +from rle.rimapi.api_catalog import READ_CATALOG, visible_write_catalog SERVER_NAME = "rle" INSTRUCTIONS = ( @@ -80,7 +80,7 @@ def end_turn(summary: str = "") -> str: n = len(session.ledger.actions) return f"Turn ended after {n} action(s)." - for name, raw in sorted(WRITE_CATALOG.items()): + for name, raw in sorted(visible_write_catalog().items()): entry = cast(dict[str, Any], raw) server.add_tool( _make_action_tool(session, name), diff --git a/src/rle/mcp/session.py b/src/rle/mcp/session.py index c4a76e9..43bec9a 100644 --- a/src/rle/mcp/session.py +++ b/src/rle/mcp/session.py @@ -51,13 +51,19 @@ async def act( ) -> dict[str, Any]: """Execute one write immediately, record it, and return the outcome.""" self.ledger.require_active() + endpoint = resolve_endpoint(action_type) + if endpoint == "tend" and not target_colonist_id: + target_colonist_id = str( + (parameters or {}).get("patient_pawn_id") + or (parameters or {}).get("patient_id") + or "" + ) or None action = Action( action_type=action_type, target_colonist_id=target_colonist_id, parameters=dict(parameters or {}), reason=reason, ) - endpoint = resolve_endpoint(action_type) if endpoint == "no_action": self.ledger.record(action, None) return {"ok": True, "action_type": action_type, "note": "no-op recorded"} @@ -74,6 +80,7 @@ async def act( "error": outcome.error} result = await self.executor.execute( ActionPlan(role=self.ledger.harness_name, tick=self.ledger.game_tick, actions=[action]), + state=self.state, ) if result.outcomes: outcome = result.outcomes[0] diff --git a/src/rle/orchestration/action_executor.py b/src/rle/orchestration/action_executor.py index b1478eb..287d6bd 100644 --- a/src/rle/orchestration/action_executor.py +++ b/src/rle/orchestration/action_executor.py @@ -13,8 +13,18 @@ ExecutionResult, resolve_endpoint, ) +from rle.orchestration.preflight import ( + VANILLA_WORK_TYPES, + growing_zone_already_covers, + is_already_covered_error, + is_quarantined_write, + normalize_work_priorities, + research_target_status, + resolve_tend_pair, +) from rle.rimapi.api_catalog import WRITE_CATALOG -from rle.rimapi.client import RimAPIClient, RimAPIResponseError +from rle.rimapi.client import RimAPIClient, RimAPIConnectionError, RimAPIResponseError +from rle.rimapi.schemas import GameState __all__ = ["NEEDS_PAWN", "ActionExecutor", "ActionOutcome", "ExecutionResult"] @@ -47,31 +57,6 @@ def _extract_rimapi_error(detail: str) -> str: return detail -def _normalize_work_priorities(params: dict[str, Any]) -> dict[str, int]: - """Accept the parameter shapes models actually emit for work_priority. - - Documented shape is flat ``{"": <1-4>}``, but frontier models - also emit ``{"work_type": "Research", "priority": 2}`` and - ``{"work_priorities": {"Growing": 1, ...}}`` (issue #27). Passing those - through verbatim posted garbage like work="work_type" to RIMAPI. - """ - nested = params.get("work_priorities") - if isinstance(nested, dict): - return {str(work): int(pri) for work, pri in nested.items()} - if "work_type" in params: - return {str(params["work_type"]): int(params.get("priority", 1))} - flat = { - str(work): int(pri) for work, pri in params.items() - if isinstance(pri, int) and not isinstance(pri, bool) - } - if not flat: - raise ValueError( - 'work_priority requires {"": <1-4>} parameters ' - "(e.g. {\"Growing\": 1})" - ) - return flat - - class ActionExecutor: """Dispatches agent actions to RIMAPI write endpoints. @@ -85,15 +70,15 @@ def __init__(self, client: RimAPIClient) -> None: # Agents perseverate — they re-issue the same growing zone every tick. # Each repeat targets cells already owned by the first zone, which RIMAPI # rejects (and historically mislabelled as "Invalid plant definition"). - # We short-circuit overlapping repeats with an explicit error so the - # agent learns the zone exists instead of burning ticks on doomed calls - # (issue #33). + # Overlapping repeats are success-by-state (the farm already exists). self._created_growing_zones: list[tuple[int, int, int, int]] = [] # Same guard for stockpiles: RimWorld logs one "overwriting slot group # square" error PER CELL when a stockpile overlaps an existing one, # which spams the dev log (and pops it over the game when auto-open # is enabled). self._created_stockpile_zones: list[tuple[int, int, int, int]] = [] + self._tick_state: GameState | None = None + self._work_types: frozenset[str] | None = None @staticmethod def _rects_overlap( @@ -103,8 +88,16 @@ def _rects_overlap( bx1, bz1, bx2, bz2 = b return not (ax2 < bx1 or ax1 > bx2 or az2 < bz1 or az1 > bz2) - async def execute(self, plan: ActionPlan) -> ExecutionResult: - """Execute all actions in a plan, return summary + per-action outcomes.""" + async def execute( + self, plan: ActionPlan, state: GameState | None = None, + ) -> ExecutionResult: + """Execute all actions in a plan, return summary + per-action outcomes. + + ``state`` is the current colony snapshot used by preflight gates + (research availability, living doctor/patient). Harnesses that omit + it keep the write path; gates that need state are skipped. + """ + self._tick_state = state executed = 0 failed = 0 no_action_count = 0 @@ -158,6 +151,15 @@ async def _dispatch(self, action: Action, endpoint: str) -> None: cid = action.target_colonist_id or "" params = action.parameters + if is_quarantined_write(endpoint): + raise ValueError( + f"{endpoint} is quarantined — the endpoint is missing on " + "this RIMAPI build. Do not advertise or call it." + ) + + if endpoint == "tend" and (not cid or cid == "0"): + cid = str(params.get("patient_pawn_id") or params.get("patient_id") or "") + # Skip pawn-targeting actions with no valid colonist ID if endpoint in _NEEDS_PAWN and (not cid or cid == "0"): logger.info("Skipping %s: no valid colonist ID", endpoint) @@ -180,9 +182,22 @@ async def _dispatch(self, action: Action, endpoint: str) -> None: # -- Specialized handlers (parameter mapping for complex DTOs) ----------- + async def _allowed_work_types(self) -> frozenset[str]: + if self._work_types is not None: + return self._work_types + try: + names = await self._client.get_work_list() + except (RimAPIResponseError, RimAPIConnectionError, TypeError, AttributeError): + names = None + if isinstance(names, list) and names: + self._work_types = frozenset(str(item) for item in names) + else: + self._work_types = VANILLA_WORK_TYPES + return self._work_types + async def _h_work_priority(self, cid: str, params: dict[str, Any]) -> None: await self._client.set_work_priorities( - cid, _normalize_work_priorities(params), + cid, normalize_work_priorities(params, await self._allowed_work_types()), ) async def _h_draft(self, cid: str, params: dict[str, Any]) -> None: @@ -219,7 +234,8 @@ async def _h_bed_rest(self, cid: str, params: dict[str, Any]) -> None: await self._client.assign_bed_rest(cid, bed_building_id=params.get("bed_building_id")) async def _h_tend(self, cid: str, params: dict[str, Any]) -> None: - await self._client.administer_medicine(cid, doctor_id=params.get("doctor_id")) + doctor_id, patient_id = resolve_tend_pair(cid, params, self._tick_state) + await self._client.administer_medicine(patient_id, doctor_id=doctor_id) async def _h_blueprint(self, cid: str, params: dict[str, Any]) -> None: if "x" not in params or "z" not in params: @@ -240,22 +256,39 @@ async def _h_growing_zone(self, cid: str, params: dict[str, Any]) -> None: x2 = int(params.get("x2", x1 + 5)) z2 = int(params.get("z2", z1 + 5)) rect = (min(x1, x2), min(z1, z2), max(x1, x2), max(z1, z2)) + map_id = int(params.get("map_id", 0)) if any(self._rects_overlap(rect, prev) for prev in self._created_growing_zones): - raise ValueError( - "A growing zone already covers these cells — it was created " - "earlier this run. Do NOT recreate it; pick a different, " - "non-overlapping rectangle or move on to another task." + # Already satisfied this run — do not recreate or score as failure. + return + if await self._growing_zone_covered_on_map(map_id, x1, z1, x2, z2): + self._created_growing_zones.append(rect) + return + try: + await self._client.create_growing_zone( + map_id=map_id, + plant_def=params.get("plant_def", "Plant_Potato"), + x1=x1, + z1=z1, + x2=x2, + z2=z2, ) - await self._client.create_growing_zone( - map_id=int(params.get("map_id", 0)), - plant_def=params.get("plant_def", "Plant_Potato"), - x1=x1, - z1=z1, - x2=x2, - z2=z2, - ) + except RimAPIResponseError as exc: + if is_already_covered_error(_extract_rimapi_error(exc.detail)): + self._created_growing_zones.append(rect) + return + raise self._created_growing_zones.append(rect) + async def _growing_zone_covered_on_map( + self, map_id: int, x1: int, z1: int, x2: int, z2: int, + ) -> bool: + """Ask ``POST /builder/check-zone`` whether a zone already owns these cells.""" + try: + payload = await self._client.check_zone(map_id, x1, z1, x2, z2) + except (RimAPIResponseError, RimAPIConnectionError, AttributeError, TypeError): + return False + return growing_zone_already_covers(payload) + # RimWorld's StoragePriority is semantic; models reasonably emit the # words. Map them instead of crashing in int() (found in the 2026-06-11 # spread: fable5 sent priority="important"). @@ -324,8 +357,24 @@ async def _h_toggle_power(self, cid: str, params: dict[str, Any]) -> None: ) async def _h_research_target(self, cid: str, params: dict[str, Any]) -> None: - project = params.get("project", params.get("name", "")) - await self._client.set_research_target(project, force=params.get("force", False)) + project = str(params.get("project", params.get("name", ""))) + force = bool(params.get("force", False)) + research = None if self._tick_state is None else self._tick_state.research + if research is not None and not force: + status = research_target_status(project, research) + if status == "current": + return + if status == "finished": + raise ValueError( + f"Research project '{project}' is already finished." + ) + if status == "locked": + raise ValueError( + f"Research project '{project}' is not currently available " + "(prerequisites or research bench missing). Only queue " + "projects listed in research.available." + ) + await self._client.set_research_target(project, force=force) async def _h_research_stop(self, cid: str, params: dict[str, Any]) -> None: await self._client.stop_research() diff --git a/src/rle/orchestration/action_resolver.py b/src/rle/orchestration/action_resolver.py index b343d92..875449c 100644 --- a/src/rle/orchestration/action_resolver.py +++ b/src/rle/orchestration/action_resolver.py @@ -30,6 +30,7 @@ from pydantic import BaseModel, ConfigDict from rle.agents.actions import Action, ActionPlan, resolve_endpoint +from rle.orchestration.preflight import extract_work_priorities from rle.rimapi.schemas import GameState logger = logging.getLogger(__name__) @@ -89,17 +90,8 @@ def _winner_key(ta: _TaggedAction) -> tuple[int, int, float, int]: def _work_priorities(params: dict[str, Any]) -> dict[str, int]: - """Accepted work_priority shapes, tolerant of garbage.""" - nested = params.get("work_priorities") - if isinstance(nested, dict): - return {str(work): int(pri) for work, pri in nested.items() if isinstance(pri, int)} - if "work_type" in params: - pri = params.get("priority", 1) - return {str(params["work_type"]): int(pri)} if isinstance(pri, int) else {} - return { - str(work): pri for work, pri in params.items() - if isinstance(pri, int) and not isinstance(pri, bool) - } + """Accepted work_priority shapes; reserved DTO keys are not work types.""" + return extract_work_priorities(params) def _merge_work_priority(candidates: list[_TaggedAction]) -> Action: diff --git a/src/rle/orchestration/game_loop.py b/src/rle/orchestration/game_loop.py index ca15d54..560b3a0 100644 --- a/src/rle/orchestration/game_loop.py +++ b/src/rle/orchestration/game_loop.py @@ -429,7 +429,7 @@ async def run_tick(self) -> TickResult: # 4. Execute — unless the harness already applied its writes if step.execution is None: - exec_result = await self._executor.execute(step.plan) + exec_result = await self._executor.execute(step.plan, state=state) for outcome in exec_result.outcomes: self._emit( EventType.ACTION_EXEC, tick_num, diff --git a/src/rle/orchestration/preflight.py b/src/rle/orchestration/preflight.py new file mode 100644 index 0000000..78fe89c --- /dev/null +++ b/src/rle/orchestration/preflight.py @@ -0,0 +1,250 @@ +"""Harness-agnostic preflight gates at action dispatch. + +Keeper Crashlanded failures that are RLE-side contract bugs must not be +counted as model noise. These checks sit in ``ActionExecutor`` so every +harness benefits: + +* growing-zone cells already covered → success-by-state +* work_priority payloads aligned with WorkTypeDef / ``/api/v1/work-list`` +* research_target only when the project is available (prereqs / bench) +* tend requires a living doctor + patient pair +* stockpile_delete is quarantined when the endpoint is missing +""" + +from __future__ import annotations + +from typing import Any + +from rle.rimapi.api_catalog import QUARANTINED_WRITES +from rle.rimapi.schemas import ColonistData, GameState, ResearchData + +__all__ = [ + "ALREADY_COVERED_MARKERS", + "VANILLA_WORK_TYPES", + "WORK_PRIORITY_RESERVED_KEYS", + "extract_work_priorities", + "growing_zone_already_covers", + "is_already_covered_error", + "is_quarantined_write", + "living_colonists", + "normalize_work_priorities", + "research_target_status", + "resolve_tend_pair", +] + +# Vanilla WorkTypeDef.defName values. Live ``GET /api/v1/work-list`` wins +# when the client can fetch it; this is the offline / mock fallback. +VANILLA_WORK_TYPES: frozenset[str] = frozenset({ + "Firefighter", + "Patient", + "Doctor", + "PatientBedRest", + "BasicWorker", + "Warden", + "Handling", + "Cooking", + "Hunting", + "Construction", + "Growing", + "Mining", + "PlantCutting", + "Smithing", + "Tailoring", + "Art", + "Crafting", + "Hauling", + "Cleaning", + "Research", + "Childcare", +}) + +# Field names models copy from the old catalog / RIMAPI DTO. Never treat +# these as WorkTypeDef names when flattening a work_priority payload. +WORK_PRIORITY_RESERVED_KEYS: frozenset[str] = frozenset({ + "id", + "priority", + "pawn_id", + "colonist_id", + "target_colonist_id", + "work", + "work_type", + "work_priorities", + "reason", + "map_id", +}) + +# Engine / RIMAPI phrasing for "these cells already belong to a zone". +ALREADY_COVERED_MARKERS: tuple[str, ...] = ( + "already cover", + "already owned", + "cells already", + "owned by a zone", + "zone already", + "overlapping zone", + "already assigned to a zone", +) + + +def is_quarantined_write(endpoint: str) -> bool: + """True when the write must not be advertised or dispatched.""" + return endpoint in QUARANTINED_WRITES + + +def extract_work_priorities(params: dict[str, Any]) -> dict[str, int]: + """Shape-normalize work_priority parameters. Does not validate names. + + Accepts the documented flat ``{"": <0-4>}`` map, the nested + ``work_priorities`` object, and the single-type ``work`` / + ``work_type`` + ``priority`` shape. Reserved DTO keys (``id``, + ``priority``, …) are never treated as work types. + """ + nested = params.get("work_priorities") + if isinstance(nested, dict): + out: dict[str, int] = {} + for work, pri in nested.items(): + try: + out[str(work)] = int(pri) + except (TypeError, ValueError): + continue + return out + + work_name = params.get("work_type", params.get("work")) + if work_name is not None and str(work_name) and str(work_name) not in WORK_PRIORITY_RESERVED_KEYS: + raw_pri = params.get("priority", 1) + try: + return {str(work_name): int(raw_pri)} + except (TypeError, ValueError): + return {} + + flat: dict[str, int] = {} + for work, pri in params.items(): + if work in WORK_PRIORITY_RESERVED_KEYS: + continue + if isinstance(pri, bool) or not isinstance(pri, int): + continue + flat[str(work)] = pri + return flat + + +def normalize_work_priorities( + params: dict[str, Any], + allowed: frozenset[str] = VANILLA_WORK_TYPES, +) -> dict[str, int]: + """Extract + validate WorkTypeDef names and RimWorld priority 0–4.""" + pairs = extract_work_priorities(params) + if not pairs: + raise ValueError( + 'work_priority requires {"": <0-4>} parameters ' + '(e.g. {"Growing": 1}). Do not send id/priority as the payload; ' + "target_colonist_id is the pawn. Work types come from " + "GET /api/v1/work-list." + ) + lookup = {name.lower(): name for name in allowed} + validated: dict[str, int] = {} + unknown: list[str] = [] + for work, pri in pairs.items(): + canonical = lookup.get(work.lower()) + if canonical is None: + unknown.append(work) + continue + if pri < 0 or pri > 4: + raise ValueError( + f"work_priority {canonical}={pri} is out of range; " + "RimWorld priorities are 0 (disabled) through 4 (lowest)" + ) + validated[canonical] = pri + if unknown: + sample = ", ".join(sorted(allowed)[:8]) + raise ValueError( + f"Unknown WorkTypeDef {unknown!r}. Use names from " + f"/api/v1/work-list (e.g. {sample}, …)" + ) + return validated + + +def growing_zone_already_covers(payload: Any) -> bool: + """True when ``POST /builder/check-zone`` (or equivalent) says a zone owns cells. + + Handles both the develop ``issues.zones`` shape and the docs + ``occupied_cells`` summary. + """ + if not isinstance(payload, dict): + return False + issues = payload.get("issues", payload.get("Issues")) + if isinstance(issues, dict): + zones = issues.get("zones", issues.get("Zones")) + if isinstance(zones, list) and zones: + return True + occupied = payload.get("occupied_cells", payload.get("OccupiedCells")) + return isinstance(occupied, int) and occupied > 0 + + +def is_already_covered_error(message: str) -> bool: + """True when a RIMAPI/engine error means the zone already covers the cells.""" + text = message.lower() + return any(marker in text for marker in ALREADY_COVERED_MARKERS) + + +def research_target_status(project: str, research: ResearchData) -> str: + """Classify a research target against the current tree. + + Returns ``available``, ``current``, ``finished``, or ``locked``. + Comparison is case-insensitive on defName. + """ + name = project.strip() + if not name: + raise ValueError('research_target requires "project" (research defName)') + key = name.lower() + current = (research.current_project or "").strip() + if current and current.lower() == key: + return "current" + if any(item.lower() == key for item in research.completed): + return "finished" + if any(item.lower() == key for item in research.available): + return "available" + return "locked" + + +def living_colonists(colonists: list[ColonistData]) -> dict[str, ColonistData]: + """Colonists with health > 0, keyed by ``colonist_id``.""" + return {c.colonist_id: c for c in colonists if c.health > 0} + + +def resolve_tend_pair( + patient_id: str, + params: dict[str, Any], + state: GameState | None, +) -> tuple[str, str]: + """Return ``(doctor_id, patient_id)`` or raise if the pair is invalid. + + Patient comes from ``target_colonist_id`` / ``patient_pawn_id``. + Doctor comes from ``doctor_pawn_id`` / ``doctor_id``. When ``state`` + is present both must be living colonists and must be distinct. + """ + patient = str(params.get("patient_pawn_id") or params.get("patient_id") or patient_id or "") + doctor_raw = params.get("doctor_pawn_id") + if doctor_raw is None: + doctor_raw = params.get("doctor_id") + doctor = "" if doctor_raw is None else str(doctor_raw) + + if not patient or patient == "0": + raise ValueError( + "tend requires a living patient " + "(target_colonist_id or patient_pawn_id)" + ) + if not doctor or doctor == "0": + raise ValueError( + "tend requires a living doctor (doctor_pawn_id)" + ) + if doctor == patient: + raise ValueError("tend requires a distinct living doctor and patient") + + if state is None: + return doctor, patient + + living = living_colonists(state.colonists) + if patient not in living: + raise ValueError(f"tend patient {patient} is missing or not alive") + if doctor not in living: + raise ValueError(f"tend doctor {doctor} is missing or not alive") + return doctor, patient diff --git a/src/rle/rimapi/api_catalog.py b/src/rle/rimapi/api_catalog.py index e2faae7..c7edde3 100644 --- a/src/rle/rimapi/api_catalog.py +++ b/src/rle/rimapi/api_catalog.py @@ -10,6 +10,8 @@ from __future__ import annotations +from typing import Any + # -- GAME CONTROL (used by game loop, not agents) -------------------------- GAME_CONTROL = { @@ -291,6 +293,24 @@ }, } +# Writes that exist on some RIMAPI builds (upstream develop) but are +# missing from the deployed Workshop / rle-testing DLL. Advertising them +# makes models call a 404; those failures get scored as model noise. +# Hide from briefs/MCP and refuse at dispatch until the endpoint is live. +QUARANTINED_WRITES: frozenset[str] = frozenset({ + "stockpile_delete", +}) + + +def visible_write_catalog() -> dict[str, Any]: + """WRITE_CATALOG minus quarantined endpoints — what agents may see.""" + return { + name: entry + for name, entry in WRITE_CATALOG.items() + if name not in QUARANTINED_WRITES + } + + # -- WRITE ENDPOINTS (agents propose these as actions) ---------------------- WRITE_CATALOG = { @@ -298,11 +318,14 @@ "work_priority": { "method": "POST", "path": "/api/v1/colonist/work-priority", - "description": "Set a colonist's priority for a work type (1=highest, 4=lowest)", + "description": ( + "Set a colonist's WorkTypeDef priorities. " + "target_colonist_id is the pawn — do NOT send id/priority as fields. " + "Work type names come from GET /api/v1/work-list " + "(Growing, Mining, Research, …). 1=highest, 4=lowest, 0=disabled." + ), "params": { - "id": "int (colonist ID)", - "work": "string (e.g. Growing, Mining)", - "priority": "int (1-4)", + "": "int 0-4 (WorkTypeDef defName, e.g. Growing: 1, Mining: 2)", }, }, "draft": { @@ -354,8 +377,14 @@ "tend": { "method": "POST", "path": "/api/v1/pawn/medical/tend", - "description": "Have a doctor tend to a patient", - "params": {"patient_pawn_id": "int", "doctor_pawn_id": "int? (optional)"}, + "description": ( + "Have a living doctor tend a living patient. Both IDs are " + "required and must be distinct living colonists." + ), + "params": { + "patient_pawn_id": "int (or target_colonist_id)", + "doctor_pawn_id": "int (required living doctor)", + }, }, # Construction / Zones "blueprint": { @@ -384,7 +413,11 @@ "growing_zone": { "method": "POST", "path": "/api/v1/map/zone/growing", - "description": "Create a growing zone for food production", + "description": ( + "Create a growing zone for food production. If a growing zone " + "already covers the cells (check-zone / engine), the write is " + "treated as already satisfied — do not recreate." + ), "params": { "map_id": "int", "plant_def": "string (e.g. Plant_Potato)", @@ -414,7 +447,10 @@ "stockpile_delete": { "method": "DELETE", "path": "/api/v1/map/zone/stockpile/delete", - "description": "Delete a stockpile zone", + "description": ( + "Delete a stockpile zone. Quarantined: missing on the deployed " + "RIMAPI DLL — do not advertise or call." + ), "params": {"zone_id": "int"}, }, "designate_area": { @@ -450,8 +486,12 @@ "research_target": { "method": "POST", "path": "/api/v1/research/target", - "description": "Set the current research target", - "params": {"name": "string (defName)", "force": "bool? (bypass prerequisites)"}, + "description": ( + "Set the current research target. Only queue a project that is " + "available (prerequisites and research bench). Locked or " + "unfinished-prereq projects will be rejected." + ), + "params": {"project": "string (defName from research.available)"}, }, "research_stop": { "method": "POST", diff --git a/src/rle/rimapi/client.py b/src/rle/rimapi/client.py index 7c1c8d2..5a396ee 100644 --- a/src/rle/rimapi/client.py +++ b/src/rle/rimapi/client.py @@ -888,6 +888,18 @@ async def get_research(self) -> ResearchData: data = await self._get("/api/v1/research/summary") return ResearchData.model_validate(self._adapt_research(data)) + async def get_work_list(self) -> list[str]: + """WorkTypeDef defNames from ``GET /api/v1/work-list``.""" + data = await self._get("/api/v1/work-list") + if isinstance(data, dict): + raw = data.get("work", data.get("Work", [])) + if isinstance(raw, list): + return [str(item) for item in raw] + return [] + if isinstance(data, list): + return [str(item) for item in data] + return [] + async def get_threats(self) -> list[ThreatData]: try: data = await self._get("/api/v1/incidents?map_id=0") @@ -1212,6 +1224,24 @@ async def create_growing_zone( }, ) + async def check_zone( + self, + map_id: int, + x1: int, + z1: int, + x2: int, + z2: int, + ) -> Any: + """``POST /api/v1/builder/check-zone`` — cells free vs already zoned.""" + return await self._post( + "/api/v1/builder/check-zone", + json={ + "map_id": map_id, + "point_a": {"x": x1, "y": 0, "z": z1}, + "point_b": {"x": x2, "y": 0, "z": z2}, + }, + ) + async def create_stockpile_zone( self, map_id: int, diff --git a/src/rle/testing/mock_rimapi.py b/src/rle/testing/mock_rimapi.py index 585eee2..c8afbf0 100644 --- a/src/rle/testing/mock_rimapi.py +++ b/src/rle/testing/mock_rimapi.py @@ -56,6 +56,12 @@ "current_project": "electricity", "progress": 0.45, "completed": ["stonecutting"], "available": ["electricity", "battery", "smithing"], }, + "/api/v1/work-list": { + "work": [ + "Firefighter", "Patient", "Doctor", "Growing", "Mining", + "Research", "Hauling", "Construction", "Cooking", + ], + }, "/api/v1/incidents?map_id=0": {"incidents": []}, "/api/v1/game/state": { "name": "New Hope", "wealth": 8000.0, "day": 5, "tick": 300000, diff --git a/tests/unit/test_action_executor.py b/tests/unit/test_action_executor.py index 9b44a64..b09eef2 100644 --- a/tests/unit/test_action_executor.py +++ b/tests/unit/test_action_executor.py @@ -140,9 +140,8 @@ async def test_outcomes_preserve_order(self) -> None: class TestGrowingZoneIdempotency: - """Issue #33: agents re-issue the same growing zone every tick; repeats - overlap the first zone's cells and fail. The executor short-circuits - overlapping repeats with a clear error instead of doomed RIMAPI calls.""" + """Overlapping growing-zone repeats are success-by-state: the farm + already exists, so we must not recreate or score the repeat as failure.""" def _zone(self, x1: int, z1: int, x2: int, z2: int) -> Action: return Action( @@ -160,18 +159,13 @@ async def test_first_growing_zone_succeeds(self) -> None: assert result.executed == 1 client.create_growing_zone.assert_awaited_once() - async def test_overlapping_repeat_is_rejected(self) -> None: + async def test_overlapping_repeat_is_already_satisfied(self) -> None: client = AsyncMock() executor = ActionExecutor(client) - # First creation succeeds. await executor.execute(_make_plan(self._zone(132, 137, 139, 144))) - # Identical re-issue next tick: overlaps, must be blocked. result = await executor.execute(_make_plan(self._zone(132, 137, 139, 144))) - assert result.failed == 1 - assert result.executed == 0 - assert result.outcomes[0].error is not None - assert "already" in result.outcomes[0].error.lower() - # Only the first call reached RIMAPI. + assert result.executed == 1 + assert result.failed == 0 client.create_growing_zone.assert_awaited_once() async def test_non_overlapping_zone_allowed(self) -> None: diff --git a/tests/unit/test_action_resolver.py b/tests/unit/test_action_resolver.py index 2626af3..5c09555 100644 --- a/tests/unit/test_action_resolver.py +++ b/tests/unit/test_action_resolver.py @@ -513,6 +513,19 @@ def test_work_priority_merges_complementary_work_types(self) -> None: # Growing: RM wins (role_priority 3 vs 5). Construction/Hauling kept. assert work[0].parameters == {"Growing": 1, "Hauling": 2, "Construction": 1} + def test_work_priority_dto_id_is_not_a_work_type(self) -> None: + resolver = ActionResolver() + plans = [ + _plan("resource_manager", [ + Action(action_type="work_priority", + target_colonist_id="col_01", + parameters={"id": 184, "work": "Growing", "priority": 1}), + ]), + ] + result, _stats = resolver.resolve(plans, _make_state()) + work = [a for a in result.actions if a.action_type == "work_priority"] + assert work[0].parameters == {"Growing": 1} + def test_same_type_last_writer_on_equal_priority(self) -> None: resolver = ActionResolver() plans = [ diff --git a/tests/unit/test_brief.py b/tests/unit/test_brief.py index 0940fec..7ac9087 100644 --- a/tests/unit/test_brief.py +++ b/tests/unit/test_brief.py @@ -65,6 +65,10 @@ def test_includes_no_action_and_every_write(self) -> None: assert "no_action" in names assert {"work_priority", "draft", "blueprint", "growing_zone"} <= names + def test_quarantined_stockpile_delete_hidden(self) -> None: + names = {a["action_type"] for a in action_catalog()} + assert "stockpile_delete" not in names + class TestBrief: def test_brief_carries_goals_state_events_and_actions(self) -> None: diff --git a/tests/unit/test_mcp_server.py b/tests/unit/test_mcp_server.py index 949d6a3..a9f0da6 100644 --- a/tests/unit/test_mcp_server.py +++ b/tests/unit/test_mcp_server.py @@ -77,6 +77,7 @@ async def test_brief_state_actions_and_reads(self) -> None: names = {t.name for t in await server.list_tools()} assert {"get_brief", "get_state", "list_actions", "rimapi_read", "end_turn"} <= names assert {"work_priority", "draft", "blueprint", "growing_zone"} <= names + assert "stockpile_delete" not in names brief = _text(await server.call_tool("get_brief", {})) assert "## Scenario" in brief and "## Actions available" in brief diff --git a/tests/unit/test_preflight.py b/tests/unit/test_preflight.py new file mode 100644 index 0000000..5819c55 --- /dev/null +++ b/tests/unit/test_preflight.py @@ -0,0 +1,319 @@ +"""Unit tests for the five harness-agnostic dispatch preflight gates.""" + +from __future__ import annotations + +from unittest.mock import AsyncMock + +import pytest + +from rle.agents.actions import Action, ActionPlan +from rle.harness.brief import action_catalog +from rle.orchestration.action_executor import ActionExecutor +from rle.orchestration.preflight import ( + extract_work_priorities, + growing_zone_already_covers, + is_already_covered_error, + is_quarantined_write, + normalize_work_priorities, + research_target_status, + resolve_tend_pair, +) +from rle.rimapi.client import RimAPIResponseError +from rle.rimapi.schemas import ( + ColonistData, + ColonyData, + GameState, + MapData, + ResearchData, + ResourceData, + WeatherData, +) + + +def _colonist(cid: str, *, health: float = 0.9) -> ColonistData: + return ColonistData( + colonist_id=cid, + name=cid, + health=health, + mood=0.6, + skills={}, + traits=[], + current_job=None, + is_drafted=False, + needs={}, + injuries=[], + position=(0, 0), + ) + + +def _state( + *, + colonists: list[ColonistData] | None = None, + research: ResearchData | None = None, +) -> GameState: + return GameState( + colony=ColonyData( + name="T", wealth=1.0, day=1, tick=100, population=2, + mood_average=0.5, food_days=3.0, + ), + colonists=colonists or [_colonist("181"), _colonist("184")], + resources=ResourceData( + food=10, medicine=1, steel=1, wood=1, components=0, silver=0, power_net=0.0, + ), + map=MapData(size=(250, 250), biome="t", season="spring", temperature=10.0, structures=[]), + research=research or ResearchData( + current_project="Electricity", + progress=0.2, + completed=["Stonecutting"], + available=["Electricity", "Smithing"], + ), + threats=[], + weather=WeatherData(condition="clear", temperature=10.0, outdoor_severity=0.0), + timestamp=0.0, + ) + + +def _plan(*actions: Action) -> ActionPlan: + return ActionPlan(role="test", tick=1, actions=list(actions)) + + +# -- 1. Farm / growing zone already-satisfied -------------------------------- + + +class TestGrowingZoneAlreadySatisfied: + def test_check_zone_issues_zones_is_covered(self) -> None: + assert growing_zone_already_covers({ + "can_build": False, + "issues": {"zones": [{"x": 132, "z": 137, "zone_type": "Growing"}]}, + }) + + def test_check_zone_occupied_cells_shape(self) -> None: + assert growing_zone_already_covers({"occupied_cells": 6, "free_cells": 0}) + assert not growing_zone_already_covers({"occupied_cells": 0, "free_cells": 36}) + + def test_empty_or_garbage_is_not_covered(self) -> None: + assert not growing_zone_already_covers({}) + assert not growing_zone_already_covers(None) + assert not growing_zone_already_covers("ok") + + def test_engine_message_detected(self) -> None: + assert is_already_covered_error("A growing zone already covers these cells") + assert not is_already_covered_error("Invalid plant definition: Plant_Rice") + + async def test_in_memory_overlap_is_success_not_failure(self) -> None: + client = AsyncMock() + client.check_zone = AsyncMock(return_value={"occupied_cells": 0}) + executor = ActionExecutor(client) + zone = Action( + action_type="growing_zone", + parameters={"x1": 132, "z1": 137, "x2": 139, "z2": 144, "plant_def": "Plant_Rice"}, + ) + first = await executor.execute(_plan(zone)) + assert first.executed == 1 + repeat = await executor.execute(_plan(zone)) + assert repeat.executed == 1 + assert repeat.failed == 0 + client.create_growing_zone.assert_awaited_once() + + async def test_check_zone_occupied_skips_recreate(self) -> None: + client = AsyncMock() + client.check_zone = AsyncMock(return_value={ + "issues": {"zones": [{"zone_type": "Growing", "x": 60, "z": 40}]}, + }) + executor = ActionExecutor(client) + result = await executor.execute(_plan(Action( + action_type="growing_zone", + parameters={"x1": 60, "z1": 40, "x2": 67, "z2": 47}, + ))) + assert result.executed == 1 + assert result.failed == 0 + client.create_growing_zone.assert_not_awaited() + + async def test_engine_already_covered_error_is_success(self) -> None: + client = AsyncMock() + client.check_zone = AsyncMock(return_value={"occupied_cells": 0}) + client.create_growing_zone = AsyncMock(side_effect=RimAPIResponseError( + 500, + '{"success":false,"errors":["A growing zone already covers these cells"]}', + )) + executor = ActionExecutor(client) + result = await executor.execute(_plan(Action( + action_type="growing_zone", + parameters={"x1": 10, "z1": 10, "x2": 15, "z2": 15}, + ))) + assert result.executed == 1 + assert result.failed == 0 + + +# -- 2. Work-priority schema ------------------------------------------------- + + +class TestWorkPrioritySchema: + def test_id_priority_dto_is_not_treated_as_work_types(self) -> None: + pairs = extract_work_priorities({"id": 184, "work": "Growing", "priority": 1}) + assert pairs == {"Growing": 1} + + def test_flat_map_drops_reserved_keys(self) -> None: + pairs = extract_work_priorities({"Growing": 1, "id": 184, "priority": 2}) + assert pairs == {"Growing": 1} + + def test_invalid_work_type_rejected(self) -> None: + with pytest.raises(ValueError, match="Unknown WorkTypeDef"): + normalize_work_priorities({"id": 184, "priority": 1}) + + def test_valid_work_type_normalized(self) -> None: + assert normalize_work_priorities({"growing": 1}) == {"Growing": 1} + + def test_priority_out_of_range_rejected(self) -> None: + with pytest.raises(ValueError, match="out of range"): + normalize_work_priorities({"Growing": 9}) + + async def test_executor_rejects_id_priority_payload(self) -> None: + client = AsyncMock() + executor = ActionExecutor(client) + result = await executor.execute(_plan(Action( + action_type="work_priority", + target_colonist_id="184", + parameters={"id": 184, "priority": 1}, + ))) + assert result.failed == 1 + assert "WorkType" in (result.outcomes[0].error or "") + client.set_work_priorities.assert_not_awaited() + + async def test_executor_accepts_work_plus_priority(self) -> None: + client = AsyncMock() + executor = ActionExecutor(client) + result = await executor.execute(_plan(Action( + action_type="work_priority", + target_colonist_id="184", + parameters={"work": "Growing", "priority": 1, "id": 184}, + ))) + assert result.executed == 1 + client.set_work_priorities.assert_awaited_once_with("184", {"Growing": 1}) + + +# -- 3. Research availability ------------------------------------------------ + + +class TestResearchAvailability: + def test_available_current_finished_locked(self) -> None: + research = ResearchData( + current_project="Electricity", + progress=0.2, + completed=["Stonecutting"], + available=["Electricity", "Smithing"], + ) + assert research_target_status("Smithing", research) == "available" + assert research_target_status("electricity", research) == "current" + assert research_target_status("Stonecutting", research) == "finished" + assert research_target_status("Fabrication", research) == "locked" + + async def test_locked_research_fails_without_write(self) -> None: + client = AsyncMock() + executor = ActionExecutor(client) + result = await executor.execute( + _plan(Action(action_type="research_target", parameters={"project": "Fabrication"})), + state=_state(), + ) + assert result.failed == 1 + assert "not currently available" in (result.outcomes[0].error or "") + client.set_research_target.assert_not_awaited() + + async def test_available_research_is_queued(self) -> None: + client = AsyncMock() + executor = ActionExecutor(client) + result = await executor.execute( + _plan(Action(action_type="research_target", parameters={"project": "Smithing"})), + state=_state(), + ) + assert result.executed == 1 + client.set_research_target.assert_awaited_once_with("Smithing", force=False) + + async def test_current_project_is_already_satisfied(self) -> None: + client = AsyncMock() + executor = ActionExecutor(client) + result = await executor.execute( + _plan(Action(action_type="research_target", parameters={"project": "Electricity"})), + state=_state(), + ) + assert result.executed == 1 + client.set_research_target.assert_not_awaited() + + +# -- 4. Doctor + patient preflight ------------------------------------------- + + +class TestTendPair: + def test_missing_doctor_rejected(self) -> None: + with pytest.raises(ValueError, match="doctor"): + resolve_tend_pair("181", {}, None) + + def test_dead_doctor_rejected(self) -> None: + state = _state(colonists=[_colonist("181"), _colonist("184", health=0.0)]) + with pytest.raises(ValueError, match="doctor"): + resolve_tend_pair("181", {"doctor_pawn_id": "184"}, state) + + def test_dead_patient_rejected(self) -> None: + state = _state(colonists=[_colonist("181", health=0.0), _colonist("184")]) + with pytest.raises(ValueError, match="patient"): + resolve_tend_pair("181", {"doctor_pawn_id": "184"}, state) + + def test_valid_pair_returned(self) -> None: + state = _state() + assert resolve_tend_pair("181", {"doctor_pawn_id": "184"}, state) == ("184", "181") + + async def test_executor_blocks_tend_without_doctor(self) -> None: + client = AsyncMock() + executor = ActionExecutor(client) + result = await executor.execute( + _plan(Action(action_type="tend", target_colonist_id="181")), + state=_state(), + ) + assert result.failed == 1 + assert "doctor" in (result.outcomes[0].error or "") + client.administer_medicine.assert_not_awaited() + + async def test_executor_sends_living_pair(self) -> None: + client = AsyncMock() + executor = ActionExecutor(client) + result = await executor.execute( + _plan(Action( + action_type="tend", + target_colonist_id="181", + parameters={"doctor_pawn_id": "184"}, + )), + state=_state(), + ) + assert result.executed == 1 + client.administer_medicine.assert_awaited_once_with("181", doctor_id="184") + + +# -- 5. Stockpile-delete quarantine ------------------------------------------ + + +class TestStockpileDeleteQuarantine: + def test_endpoint_is_quarantined(self) -> None: + assert is_quarantined_write("stockpile_delete") + assert not is_quarantined_write("stockpile_zone") + + def test_not_advertised_in_brief(self) -> None: + names = {entry["action_type"] for entry in action_catalog()} + assert "stockpile_delete" not in names + assert "stockpile_zone" in names + + def test_work_priority_catalog_does_not_advertise_id_priority(self) -> None: + work = next(a for a in action_catalog() if a["action_type"] == "work_priority") + params = work["params"] + assert "id" not in params + assert "priority" not in params + + async def test_executor_refuses_without_http(self) -> None: + client = AsyncMock() + executor = ActionExecutor(client) + result = await executor.execute(_plan(Action( + action_type="stockpile_delete", + parameters={"zone_id": 3}, + ))) + assert result.failed == 1 + assert "quarantined" in (result.outcomes[0].error or "") + client.call.assert_not_awaited() diff --git a/tests/unit/test_rimapi_client.py b/tests/unit/test_rimapi_client.py index 45b95f3..97981b7 100644 --- a/tests/unit/test_rimapi_client.py +++ b/tests/unit/test_rimapi_client.py @@ -47,6 +47,10 @@ "/api/v1/pawn/job": {"success": True}, "/api/v1/map/building/power?buildingId=999&powerOn=false": {"success": True}, "/api/v1/map/zone/growing": {"success": True}, + "/api/v1/builder/check-zone": { + "can_build": True, + "issues": {"terrain": [], "ores": [], "buildings": [], "zones": []}, + }, "/api/v1/pawn/medical/bed-rest": {"success": True}, "/api/v1/pawn/medical/tend": {"success": True}, "/api/v1/jobs/make/equip": {"success": True}, @@ -189,6 +193,9 @@ def all_routes( ], "/api/v1/map/buildings?map_id=0": [], "/api/v1/research/summary": sample_research_dict, + "/api/v1/work-list": { + "work": ["Growing", "Mining", "Research", "Hauling", "Construction"], + }, "/api/v1/incidents?map_id=0": {"incidents": [sample_threat_dict]}, "/api/v1/game/state": sample_colony_dict, "/api/v1/map/weather?map_id=0": sample_weather_dict, @@ -440,6 +447,16 @@ async def test_create_growing_zone(self, mock_client: RimAPIClient) -> None: ) assert result["success"] is True + async def test_check_zone(self, mock_client: RimAPIClient) -> None: + result = await mock_client.check_zone(0, 50, 50, 55, 55) + assert result["can_build"] is True + assert result["issues"]["zones"] == [] + + async def test_get_work_list(self, mock_client: RimAPIClient) -> None: + names = await mock_client.get_work_list() + assert "Growing" in names + assert "Research" in names + async def test_assign_bed_rest(self, mock_client: RimAPIClient) -> None: result = await mock_client.assign_bed_rest("12345") assert result["success"] is True From 08ed7aea8d70c810378f1c2f2220af14f9227ce2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 7 Sep 2026 22:32:45 +0000 Subject: [PATCH 2/2] fix: accept skill= work_priority alias; allow re-targeting current research Felix integration fixtures emit {"skill": "growing", "priority": 1}. Treat skill as a WorkTypeDef alias. Re-queueing the current research project is allowed (it is available); only finished and locked projects are blocked. Co-authored-by: Jason --- src/rle/orchestration/action_executor.py | 2 -- src/rle/orchestration/preflight.py | 6 ++++-- tests/unit/test_preflight.py | 16 ++++++++++++---- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/rle/orchestration/action_executor.py b/src/rle/orchestration/action_executor.py index 287d6bd..35a56a3 100644 --- a/src/rle/orchestration/action_executor.py +++ b/src/rle/orchestration/action_executor.py @@ -362,8 +362,6 @@ async def _h_research_target(self, cid: str, params: dict[str, Any]) -> None: research = None if self._tick_state is None else self._tick_state.research if research is not None and not force: status = research_target_status(project, research) - if status == "current": - return if status == "finished": raise ValueError( f"Research project '{project}' is already finished." diff --git a/src/rle/orchestration/preflight.py b/src/rle/orchestration/preflight.py index 78fe89c..ca897fb 100644 --- a/src/rle/orchestration/preflight.py +++ b/src/rle/orchestration/preflight.py @@ -68,6 +68,7 @@ "target_colonist_id", "work", "work_type", + "skill", "work_priorities", "reason", "map_id", @@ -108,8 +109,9 @@ def extract_work_priorities(params: dict[str, Any]) -> dict[str, int]: continue return out - work_name = params.get("work_type", params.get("work")) - if work_name is not None and str(work_name) and str(work_name) not in WORK_PRIORITY_RESERVED_KEYS: + work_name = params.get("work_type", params.get("work", params.get("skill"))) + named = work_name is not None and str(work_name) + if named and str(work_name) not in WORK_PRIORITY_RESERVED_KEYS: raw_pri = params.get("priority", 1) try: return {str(work_name): int(raw_pri)} diff --git a/tests/unit/test_preflight.py b/tests/unit/test_preflight.py index 5819c55..93386e1 100644 --- a/tests/unit/test_preflight.py +++ b/tests/unit/test_preflight.py @@ -157,13 +157,20 @@ def test_flat_map_drops_reserved_keys(self) -> None: pairs = extract_work_priorities({"Growing": 1, "id": 184, "priority": 2}) assert pairs == {"Growing": 1} - def test_invalid_work_type_rejected(self) -> None: - with pytest.raises(ValueError, match="Unknown WorkTypeDef"): + def test_id_priority_only_has_no_work_type(self) -> None: + with pytest.raises(ValueError, match="Do not send id/priority"): normalize_work_priorities({"id": 184, "priority": 1}) + def test_unknown_work_type_rejected(self) -> None: + with pytest.raises(ValueError, match="Unknown WorkTypeDef"): + normalize_work_priorities({"NotAJob": 1}) + def test_valid_work_type_normalized(self) -> None: assert normalize_work_priorities({"growing": 1}) == {"Growing": 1} + def test_skill_alias_maps_to_work_type(self) -> None: + assert normalize_work_priorities({"skill": "growing", "priority": 1}) == {"Growing": 1} + def test_priority_out_of_range_rejected(self) -> None: with pytest.raises(ValueError, match="out of range"): normalize_work_priorities({"Growing": 9}) @@ -229,7 +236,8 @@ async def test_available_research_is_queued(self) -> None: assert result.executed == 1 client.set_research_target.assert_awaited_once_with("Smithing", force=False) - async def test_current_project_is_already_satisfied(self) -> None: + async def test_current_project_is_still_queued(self) -> None: + """Re-targeting the current project is allowed; it is available.""" client = AsyncMock() executor = ActionExecutor(client) result = await executor.execute( @@ -237,7 +245,7 @@ async def test_current_project_is_already_satisfied(self) -> None: state=_state(), ) assert result.executed == 1 - client.set_research_target.assert_not_awaited() + client.set_research_target.assert_awaited_once_with("Electricity", force=False) # -- 4. Doctor + patient preflight -------------------------------------------