diff --git a/nisse-dev/src/nisse_dev/control.py b/nisse-dev/src/nisse_dev/control.py index fe9be26..2b56541 100644 --- a/nisse-dev/src/nisse_dev/control.py +++ b/nisse-dev/src/nisse_dev/control.py @@ -23,7 +23,14 @@ from typing import Any, Protocol from nisse_dev.cancel import Cancel from nisse_dev.canonical import GateOpened, PlanSubmitted, UserMessage -from nisse_dev.gates import Approved, Gate, Ignored, PlanRevision, Rejected +from nisse_dev.gates import ( + Approved, + Gate, + Ignored, + PlanRevision, + Rejected, + command_form, +) from nisse_dev.git import GitError from nisse_dev.gitea import GiteaError from nisse_dev.llm import RoleMap @@ -165,6 +172,12 @@ class StubAgent: return SubmitPlan(f"[stub] {gate} plan for {project}{notes}") +def _project_name(text: str) -> str: + """The name in 'use ', as written: only the command word is + case-insensitive.""" + return text.strip().rstrip(".!")[len("use") :].strip() + + def _fresh_data() -> dict[str, Any]: return { "revs": {}, @@ -331,7 +344,7 @@ class Orchestrator: if msg.sender not in self._operators: return text = msg.text.strip() - if text == "stop": + if command_form(text) == "stop": # Straight to the flag, not behind the work in the queue: the # work is what it has to reach (A4). if self._worker.busy: @@ -434,9 +447,10 @@ class Orchestrator: state = self._store.get_state() current = state.state if state else IDLE text = msg.text.strip() - if text == "status": + command = command_form(text) + if command == "status": self._send_status(current) - elif text == "abandon": + elif command == "abandon": # Like status, a command in every waiting state: it takes # precedence over reading the word as an answer or feedback. if current == IDLE: @@ -444,8 +458,8 @@ class Orchestrator: else: self._abandon(msg.sender) elif current == IDLE: - if text.startswith("use "): - self._begin_project(text[4:].strip()) + if command.startswith("use "): + self._begin_project(_project_name(text)) else: self._transport.send_message("no active project; say 'use '") elif current in (AWAIT_ARCH_SIGNOFF, AWAIT_MILESTONE_SIGNOFF): @@ -592,7 +606,7 @@ class Orchestrator: name, rev = gate_data["name"], gate_data["rev"] milestone = self._data["milestone"] if gate_data["approved"]: - if text == f"start milestone {milestone}": + if command_form(text) == f"start milestone {milestone}": self._transition( EXECUTING, cause=f"operator command: start milestone {milestone}" ) @@ -941,14 +955,15 @@ class Orchestrator: ) def _handle_pr_open(self, text: str) -> None: - if text == "next milestone": + command = command_form(text) + if command == "next milestone": self._unwatch("review") self._data["milestone"] = int(self._data.get("milestone", 1)) + 1 self._data["gate"] = None self._data["pr"] = None self._transition(MILESTONE_PLANNING, cause="operator: next milestone") self._run_planning_turn(f"milestone-{self._data['milestone']}") - elif text == "project complete": + elif command == "project complete": self.stop_watching() self._data = {**_fresh_data()} self._project = None @@ -1022,9 +1037,9 @@ class Orchestrator: if pending is None: # Reached only if state was forced externally or a pending item # was lost; starting over must still work from here. - if text.startswith("use "): + if command_form(text).startswith("use "): self.stop_watching() - self._begin_project(text[4:].strip()) + self._begin_project(_project_name(text)) return self._transport.send_message( "waiting for direction, but I have no pending item to resume; " @@ -1061,7 +1076,7 @@ class Orchestrator: """Answer to the question a restart asks (A3 step 7). Never a blind resume: 'continue' re-runs the interrupted planning turn, or returns an interrupted milestone to its approved plan to be started again.""" - choice = text.strip().rstrip(".!").lower() + choice = command_form(text) if choice not in ("continue", "re-plan", "replan"): self._data["pending"] = pending self._transport.ask_question(_BOOT_CHOICES) diff --git a/nisse-dev/src/nisse_dev/gates.py b/nisse-dev/src/nisse_dev/gates.py index eb66853..420ae39 100644 --- a/nisse-dev/src/nisse_dev/gates.py +++ b/nisse-dev/src/nisse_dev/gates.py @@ -50,6 +50,14 @@ class Ignored: _APPROVE_RE = re.compile(r"^approve(?:\s+(rev-\d+))?$") +def command_form(text: str) -> str: + """How a message reads as a command: case, surrounding whitespace, runs + of inner whitespace and trailing '.' or '!' do not matter, so a phone's + "Approve." is still an approval. Only for matching; what the operator + wrote is what gets recorded and passed on.""" + return " ".join(text.strip().rstrip(".!").split()).lower() + + class Gate: def __init__(self, revision: PlanRevision, operators: AbstractSet[str]) -> None: self._revision = revision @@ -61,7 +69,7 @@ class Gate: if sender not in self._operators: return Ignored(notice=None) text = text.strip() - match = _APPROVE_RE.match(text) + match = _APPROVE_RE.match(command_form(text)) if match is None: return Rejected(feedback=text, approver=sender, channel=channel) named = match.group(1) diff --git a/nisse-dev/tests/test_control.py b/nisse-dev/tests/test_control.py index 2c08c2c..9a2c222 100644 --- a/nisse-dev/tests/test_control.py +++ b/nisse-dev/tests/test_control.py @@ -269,3 +269,17 @@ def test_use_starts_over_when_nothing_is_pending(h: Harness) -> None: h.store.transition(AWAIT_INPUT, cause="forced", project="demo", data={}) h.transport.receive("don", "use demo") assert h.state == AWAIT_ARCH_SIGNOFF + + +def test_commands_ignore_case_but_the_project_name_keeps_it(h: Harness) -> None: + h.transport.receive("don", "Use Demo.") + assert h.state == AWAIT_ARCH_SIGNOFF + assert h.store.get_state().project == "Demo" + h.transport.receive("don", "Status") + assert "AWAIT_ARCH_SIGNOFF" in h.outbound_texts()[-1] + h.transport.receive("don", "Approve") + h.transport.receive("don", "Approve.") + h.transport.receive("don", "Start Milestone 1") + assert h.state == PR_OPEN + h.transport.receive("don", "Project Complete!") + assert h.state == IDLE diff --git a/nisse-dev/tests/test_gates.py b/nisse-dev/tests/test_gates.py index f76ff16..e3928fc 100644 --- a/nisse-dev/tests/test_gates.py +++ b/nisse-dev/tests/test_gates.py @@ -1,3 +1,5 @@ +import pytest + from nisse_dev.gates import Approved, Gate, Ignored, PlanRevision, Rejected REVISION = PlanRevision(gate="arch", rev=2, content="the plan") @@ -46,3 +48,22 @@ def test_approve_with_trailing_words_is_rejection() -> None: # typed word approve, nothing else). result = gate().evaluate("don", "approve, but rename the module", "transport") assert isinstance(result, Rejected) + + +@pytest.mark.parametrize( + "text", ["Approve", "APPROVE", "Approve.", "approve!", " Approve rev-2 "] +) +def test_approval_ignores_case_and_trailing_punctuation(text: str) -> None: + # A phone keyboard's "Approve" used to count as rejection feedback (#64). + assert isinstance(gate().evaluate("don", text, channel="transport"), Approved) + + +def test_a_capitalised_stale_approval_is_still_refused() -> None: + result = gate().evaluate("don", "Approve REV-1", channel="transport") + assert isinstance(result, Ignored) + + +def test_a_sentence_containing_approve_is_still_feedback() -> None: + result = gate().evaluate("don", "I approve of most of it", channel="transport") + assert isinstance(result, Rejected) + assert result.feedback == "I approve of most of it" diff --git a/nisse-dev/tests/test_stop.py b/nisse-dev/tests/test_stop.py index 23cdb19..4b86534 100644 --- a/nisse-dev/tests/test_stop.py +++ b/nisse-dev/tests/test_stop.py @@ -5,6 +5,8 @@ import threading from pathlib import Path from typing import Any +import pytest + from nisse_dev.cancel import STOPPED_RESULT, Cancel from nisse_dev.canonical import AssistantText, ToolCall, ToolResult, UserMessage from nisse_dev.control import ( @@ -207,7 +209,8 @@ class BlocksUntilStopped: return ExecutorOutcome("completed", "never stopped") -def test_stop_reaches_work_running_on_the_worker(tmp_path: Path) -> None: +@pytest.mark.parametrize("word", ["stop", "Stop!", " STOP "]) +def test_stop_reaches_work_running_on_the_worker(tmp_path: Path, word: str) -> None: # The bug this design fixes: the work used to run on the transport's own # thread, so no message, stop included, was read until it finished. worker = SerialWorker() @@ -231,7 +234,7 @@ def test_stop_reaches_work_running_on_the_worker(tmp_path: Path) -> None: assert executor.running.wait(5) transport.receive("don", "status") # delivered while the milestone runs - transport.receive("don", "stop") + transport.receive("don", word) # a phone may capitalise it (#64) worker.call(lambda: None) # wait for the stop to be recorded texts = [m.text for m in transport.outbound]