diff --git a/src/lembas/api/chats.py b/src/lembas/api/chats.py index 3303c4c..d75e825 100644 --- a/src/lembas/api/chats.py +++ b/src/lembas/api/chats.py @@ -7,6 +7,7 @@ import json import logging import time from collections.abc import AsyncIterator +from datetime import UTC, datetime from fastapi import APIRouter, Depends, Form, HTTPException, Request, status from fastapi.responses import HTMLResponse, Response, StreamingResponse @@ -405,6 +406,37 @@ async def post_message( if not content and not file_ids: return Response(status_code=status.HTTP_204_NO_CONTENT) + return _send(request, db, chat, user, content, file_ids=file_ids) + + +def _note_rewind(chat: Chat) -> None: + """Record that an agent chat's transcript went back and the machine did not. + + Deliberately no attempt to undo anything out there. The project directory is + somebody's real working tree, and deleting their work to match a rewound + transcript would be far worse than the inconsistency. So the model is told + instead -- see the `tool.agent_rewound` fragment -- and can look rather than + assume. + """ + if chat.kind == KIND_AGENT: + chat.rewound_at = datetime.now(UTC) + + +def _send( + request: Request, + db: Db, + chat: Chat, + user: User, + content: str, + *, + file_ids: list[str] | None = None, +) -> Response: + """Write a turn, start the reply, and hand back the pair of bubbles. + + Shared by the composer and by anything else that puts words into a + conversation on somebody's behalf -- carrying out a plan, for one. One path + rather than two, so a second way of sending cannot drift from the first. + """ user_message = chat_service.create_message(db, chat, ROLE_USER, content) if file_ids: files_service.claim(db, ids=file_ids, user_id=user.id, message_id=user_message.id) @@ -701,6 +733,7 @@ async def edit_message( if cutoff is None or compaction_service.moment(message) <= compaction_service.moment(cutoff): compaction_service.reset(chat) + _note_rewind(chat) db.commit() assistant = chat_service.create_message( @@ -714,6 +747,47 @@ async def edit_message( ) +@router.post("/{chat_id}/messages/{message_id}/execute-plan") +async def execute_plan( + request: Request, db: Db, user: RequiredUser, chat_id: str, message_id: str +) -> Response: + """Carry out a plan the model proposed. + + Switches to **Edit**, never Auto. The plan was written under a mode where + every command stopped for approval, and a button that also removed the + asking is not the button anybody pressed. + + The plan is sent back **marked as a quotation of the model's own words** + rather than as a bare instruction. A plan whose text came out of a file the + model read would otherwise arrive in the most trusted role in the + transcript, wearing the reader's authority -- which is precisely how an + injected instruction would like to arrive. + """ + chat = _owned_chat(db, chat_id, user.id) + message = db.get(Message, message_id) + if message is None or message.chat_id != chat.id or not message.plan_json: + raise HTTPException(status.HTTP_404_NOT_FOUND, "There is no plan on that message.") + if chat.kind != KIND_AGENT: + raise HTTPException(status.HTTP_409_CONFLICT, "This chat cannot act on anything.") + + plan = message.plan_json + steps = [str(s) for s in (plan.get("steps") or [])] + body = "\n".join(f"{n}. {step}" for n, step in enumerate(steps, start=1)) + + chat.agent_mode = agent_policy.MODE_EDIT + db.commit() + + content = ( + "Carry out the plan you proposed above:\n\n" + f"> **{plan.get('title') or 'The plan'}**\n" + + "\n".join(f"> {line}" for line in body.splitlines()) + + "\n\nWork through it in order. If a step turns out to be wrong, stop " + "and say so rather than improvising around it." + ) + log.info("%s executing a plan in chat %s", user.email, chat.id) + return _send(request, db, chat, user, content) + + @router.post("/{chat_id}/messages/{message_id}/stop") async def stop_message(db: Db, user: RequiredUser, chat_id: str, message_id: str) -> Response: """Ask a running generation to stop. @@ -959,6 +1033,7 @@ async def regenerate( message.error = "" message.complete = False message.model_id = chat.model_id + _note_rewind(chat) db.commit() # restart, not ensure: this is the one caller that reuses a Message row, and # the finished generation for it is still registered. diff --git a/src/lembas/services/agent/tools.py b/src/lembas/services/agent/tools.py index 5e7da45..28b15b0 100644 --- a/src/lembas/services/agent/tools.py +++ b/src/lembas/services/agent/tools.py @@ -237,6 +237,48 @@ def _no_connection_or_path(name: str, agent: AgentContext | None, path: str) -> return _refused(name, agent, path, "no path was given.") +# --- Proposing a plan ----------------------------------------------------------- +MAX_STEPS = 20 + + +async def _run_plan(context: ToolContext, args: dict[str, Any]) -> ToolOutcome: + """Record a plan and stop. + + Writes nothing and runs nothing, which is why it is `RISK_READ` and works in + Plan mode without asking. The loop notices the event and ends the reply + there: a plan followed by three more rounds of the model changing its mind + is not a plan. + """ + agent = _agent(context) + title = str(args.get("title") or "").strip() or "A plan" + steps = [str(s).strip() for s in (args.get("steps") or []) if str(s).strip()] + steps = steps[:MAX_STEPS] + + if not steps: + return ToolOutcome( + "A plan needs at least one step. Say what you would actually do.", + {"name": "plan_submit", "kind": "plan", "status": "error", + "error": "No steps.", "results": []}, + ) + + return ToolOutcome( + "Plan recorded. Stop here — they will read it and decide whether to " + "carry it out. Do not start doing it.", + { + "name": "plan_submit", + "kind": "plan", + "label": agent.label if agent else "", + "query": title, + "status": "ok", + "results": [], + # Read back by the loop, which puts it on the message so the + # Execute button sends exactly what was proposed rather than an + # approximation parsed out of the prose. + "plan": {"title": title, "steps": steps}, + }, + ) + + # --- The definitions ----------------------------------------------------------- def tool_defs(context: AgentContext | None = None) -> list[ToolDef]: """The agent tools, bound to one chat's machine. @@ -245,8 +287,12 @@ def tool_defs(context: AgentContext | None = None) -> list[ToolDef]: needs: it maps an offered tool *name* back to its family and has no chat to resolve. Their runners still work -- they report that the conversation is not connected to a machine, which is true. + + `plan_submit` is offered in Plan mode and nowhere else. It ends the reply, + and a model in Auto mode that proposed a plan instead of doing the work + would be obeying the wrong instinct at exactly the wrong moment. """ - return [ + defs = [ ToolDef( name="shell_run", family=FAMILY_AGENT, @@ -325,7 +371,37 @@ def tool_defs(context: AgentContext | None = None) -> list[ToolDef]: run=_run_list, risk=RISK_READ, ), + ToolDef( + name="plan_submit", + family=FAMILY_AGENT, + description=( + "Set out what you would do, as an ordered list of steps, and " + "stop. Use this to finish when you have been asked to plan " + "rather than to act: they will read it and decide whether to " + "carry it out. Each step should be one thing, concrete enough " + "to follow — name the files and the commands." + ), + parameters={ + "type": "object", + "properties": { + "title": {**_STRING, "description": "What the plan achieves, in a line."}, + "steps": { + "type": "array", + "items": _STRING, + "description": "The steps, in order.", + }, + }, + "required": ["title", "steps"], + }, + run=_run_plan, + # It writes nothing and runs nothing, so it needs no approval -- + # which is the point: Plan mode has to be able to finish. + risk=RISK_READ, + ), ] + if context is not None and context.mode != policy.MODE_PLAN: + return [tool for tool in defs if tool.name != "plan_submit"] + return defs __all__ = ["FAMILY_AGENT", "MAX_EVENT_CHARS", "tool_defs"] diff --git a/src/lembas/services/generation.py b/src/lembas/services/generation.py index 26aff81..72eb0db 100644 --- a/src/lembas/services/generation.py +++ b/src/lembas/services/generation.py @@ -125,6 +125,10 @@ class Generation: # A model that fills its own context with build logs has no room left to # answer with. output_bytes: int = 0 + # A plan proposed in Plan mode: {"title": str, "steps": [str, ...]}. Ends + # the reply and is written onto the message, so the Execute button sends + # exactly what was proposed rather than something parsed back out of prose. + plan: dict | None = None def touch(self) -> None: self.version += 1 @@ -440,10 +444,21 @@ async def _run(generation: Generation) -> None: generation.tool_events.append(outcome.event) generation.output_bytes += len(outcome.content) messages.append(tools_service.tool_turn(call, outcome.content)) + if outcome.event.get("plan"): + generation.plan = outcome.event["plan"] generation.touch() payload = {**payload, "messages": messages} + # A plan ends the turn. One more request so the model can say what + # it proposed and why -- a bubble containing only a card reads as + # though it had nothing to add -- but with the tools withdrawn, so + # "one more round" cannot become three rounds of it changing its + # mind about a plan the reader is being asked to approve. + if generation.plan is not None: + offered = [] + payload.pop("tools", None) + for kind, piece in splitter.flush(): (generation.reasoning if kind == REASONING else generation.content).append(piece) generation.touch() @@ -963,6 +978,7 @@ def _persist(generation: Generation, title: str, elapsed: float) -> None: message.reasoning = generation.thinking message.reasoning_ms = generation.reasoning_ms message.tool_calls_json = generation.tool_events + message.plan_json = generation.plan or {} message.usage_json = metrics_service.to_json( metrics_service.from_generation(generation) ) diff --git a/src/lembas/web/static/css/chat.css b/src/lembas/web/static/css/chat.css index 13c6f9d..9e4ae33 100644 --- a/src/lembas/web/static/css/chat.css +++ b/src/lembas/web/static/css/chat.css @@ -921,3 +921,31 @@ .composer__kind-agent { display: flex; gap: var(--sp-2); flex: 1 1 18rem; min-width: 0; } .composer__kind-agent .input { flex: 1; min-width: 0; } .select--sm, .input--sm { height: calc(var(--control-h) - 0.35rem); font-size: var(--text-xs); } + +/* --- A plan, and the way to carry it out ----------------------------------- */ +.plan { + margin: var(--sp-3) 0; + padding: var(--sp-4); + border: 1px solid var(--border-strong); + border-left: 3px solid var(--accent); + border-radius: var(--radius-md); + background: var(--surface); +} +.plan__title { + display: flex; + align-items: center; + gap: var(--sp-2); + margin: 0 0 var(--sp-3); + font-size: var(--text-base); + color: var(--ink); +} +.plan__steps { + margin: 0 0 var(--sp-4); + padding-left: var(--sp-5); + display: flex; + flex-direction: column; + gap: var(--sp-2); + color: var(--ink-muted); + line-height: var(--leading-relaxed); +} +.plan__note { color: var(--ink-faint); font-size: var(--text-xs); } diff --git a/src/lembas/web/templates/chat/_message.html b/src/lembas/web/templates/chat/_message.html index 1d901e6..e392cf8 100644 --- a/src/lembas/web/templates/chat/_message.html +++ b/src/lembas/web/templates/chat/_message.html @@ -193,6 +193,11 @@ leave an empty box under the file. #} {% endif %} + {% if not streaming and message.plan_json %} + {# What the model proposed in Plan mode, with the way to carry it out. #} + {% include "chat/_plan.html" %} + {% endif %} + {% if not streaming and message.role == "assistant" and message.usage_json %} {# Above the buttons, not among them: the actions row is things you press. #}
diff --git a/src/lembas/web/templates/chat/_plan.html b/src/lembas/web/templates/chat/_plan.html new file mode 100644 index 0000000..2a77a18 --- /dev/null +++ b/src/lembas/web/templates/chat/_plan.html @@ -0,0 +1,39 @@ +{% from "_macros.html" import icon %} +{# + A plan the model proposed in Plan mode. + + Rendered from `message.plan_json` rather than parsed back out of the prose, so + the Execute button sends exactly what was proposed. Every line is model output + and is escaped; a step is shown as text, never as Markdown, because a plan is + the last thing that should be able to emit a link. + + Execute switches the chat to Edit, never Auto -- the plan was written under a + mode where every command stopped for approval, and a button that also removed + the asking is not the button anybody pressed. The confirm dialog says so. +#} +
+

+ {{ icon("check", "icon--sm") }} + {{ message.plan_json.title or "A plan" }} +

+ +
    + {% for step in message.plan_json.steps %} +
  1. {{ step }}
    2. + {% endfor %} +
+ + {% if chat.kind == "agent" %} +
+ + Switches to Edit — commands still ask. +
+ {% endif %} +
diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py index c54c82a..a151c50 100644 --- a/tests/test_agent_tools.py +++ b/tests/test_agent_tools.py @@ -8,6 +8,7 @@ than a stub that always agrees. from __future__ import annotations import asyncio +import json as _json import pytest @@ -483,3 +484,170 @@ async def test_an_agent_chat_is_told_its_real_round_budget(db, user_id, machine) db, user, tools_service.resolve_tools(db, chat, user).schemas, chat ) assert values["max_rounds"] == "25" + + +# --- Plan mode's artifact --------------------------------------------------------- +def test_plan_submit_is_offered_only_in_plan_mode(db, user_id, machine): + """It ends the reply. A model in Auto mode that proposed a plan instead of + doing the work would be obeying the wrong instinct at the wrong moment.""" + user = db.get(User, user_id) + planning, _p = _setup(db, user_id, machine, mode=policy.MODE_PLAN) + assert "plan_submit" in _offered(db, planning, user) + + planning.agent_mode = policy.MODE_AUTO + db.commit() + assert "plan_submit" not in _offered(db, planning, user) + + +async def test_a_plan_ends_the_reply_and_lands_on_the_message( + db, user_id, machine, monkeypatch +): + chat, _profile = _setup(db, user_id, machine, mode=policy.MODE_PLAN) + message_id = _pending_reply(db, chat) + + plan = {"title": "Tidy the logs", "steps": ["Read the log", "Rotate it", "Restart"]} + rounds = [ + [_chunk("plan_submit", _json.dumps(plan))], + # The model gets one wordless round to explain itself. If it tries to + # act in it -- as this one does -- there are no tools to act with. + [_chunk("shell_run", '{"command": "rm -rf /"}'), _text("Here is what I would do.")], + ] + seen: list[dict] = [] + monkeypatch.setattr(generation_service, "stream_chat", _stub_stream(rounds, seen)) + + generation = generation_service.Generation(chat_id=chat.id, message_id=message_id) + await asyncio.wait_for(generation_service._run(generation), timeout=10) + + assert generation.plan == plan + assert len(seen) == 2, "one round to plan, one to say what it proposed" + assert "tools" not in seen[1], "the second round is offered nothing to act with" + assert len(generation.tool_events) == 1, "the shell call had no tool to reach" + + db.expire_all() + message = db.get(Message, message_id) + assert message.plan_json == plan + + +async def test_a_plan_with_no_steps_is_sent_back(db, user_id, machine, monkeypatch): + chat, _profile = _setup(db, user_id, machine, mode=policy.MODE_PLAN) + message_id = _pending_reply(db, chat) + monkeypatch.setattr( + generation_service, + "stream_chat", + _stub_stream( + [[_chunk("plan_submit", '{"title": "Nothing", "steps": []}')], [_text("Sorry.")]], + [], + ), + ) + + generation = generation_service.Generation(chat_id=chat.id, message_id=message_id) + await asyncio.wait_for(generation_service._run(generation), timeout=10) + + assert generation.plan is None + assert generation.tool_events[0]["status"] == "error" + + +# --- Carrying a plan out ------------------------------------------------------------ +def test_executing_a_plan_switches_to_edit_and_quotes_it(client, db, registered, machine): + """Edit, never Auto. The plan was written under a mode where every command + stopped for approval, and a button that also removed the asking is not the + button anybody pressed.""" + from sqlalchemy import select as _select + + user = db.scalar(_select(User)) + chat, _profile = _setup(db, user.id, machine, mode=policy.MODE_PLAN) + message = Message( + chat_id=chat.id, + role=ROLE_ASSISTANT, + content="Here is what I would do.", + complete=True, + plan_json={"title": "Tidy the logs", "steps": ["Read the log", "Rotate it"]}, + ) + db.add(message) + db.commit() + + response = client.post(f"/api/chats/{chat.id}/messages/{message.id}/execute-plan") + assert response.status_code == 200 + + db.refresh(chat) + assert chat.agent_mode == policy.MODE_EDIT + + sent = db.scalars( + _select(Message).where(Message.chat_id == chat.id, Message.role == "user") + ).all()[-1] + assert "Tidy the logs" in sent.content + assert "Read the log" in sent.content + # Quoted rather than stated. A plan whose text came out of a file the model + # read must not arrive wearing the reader's authority. + assert sent.content.lstrip().startswith("Carry out the plan you proposed above") + assert "> " in sent.content + + +def test_a_message_with_no_plan_cannot_be_executed(client, db, registered, machine): + from sqlalchemy import select as _select + + user = db.scalar(_select(User)) + chat, _profile = _setup(db, user.id, machine) + message = Message(chat_id=chat.id, role=ROLE_ASSISTANT, content="hi", complete=True) + db.add(message) + db.commit() + + assert ( + client.post(f"/api/chats/{chat.id}/messages/{message.id}/execute-plan").status_code + == 404 + ) + + +# --- Rewind --------------------------------------------------------------------------- +def test_editing_a_turn_records_that_the_machine_did_not_rewind( + client, db, registered, machine +): + """The transcript goes back; the project directory does not. Deleting + somebody's real working tree to match would be far worse than the + inconsistency, so the model is told instead.""" + from sqlalchemy import select as _select + + user = db.scalar(_select(User)) + chat, _profile = _setup(db, user.id, machine) + first = Message(chat_id=chat.id, role="user", content="do a thing", complete=True) + db.add(first) + db.commit() + + assert chat.rewound_at is None + client.post( + f"/api/chats/{chat.id}/messages/{first.id}/edit", data={"content": "do another thing"} + ) + db.refresh(chat) + assert chat.rewound_at is not None + + +def test_an_ordinary_chat_records_no_rewind(client, db, registered, machine): + from sqlalchemy import select as _select + + user = db.scalar(_select(User)) + chat, _profile = _setup(db, user.id, machine, kind="chat") + first = Message(chat_id=chat.id, role="user", content="hello", complete=True) + db.add(first) + db.commit() + + client.post(f"/api/chats/{chat.id}/messages/{first.id}/edit", data={"content": "hi"}) + db.refresh(chat) + assert chat.rewound_at is None + + +async def test_the_harness_warns_after_a_rewind(db, user_id, machine): + from datetime import UTC, datetime + + from lembas.services import harness + + chat, _profile = _setup(db, user_id, machine) + user = db.get(User, user_id) + offered = tools_service.resolve_tools(db, chat, user).schemas + + assert "was rewound" not in harness.compose(db, user, offered, chat) + + chat.rewound_at = datetime.now(UTC) + db.commit() + text = harness.compose(db, user, offered, chat) + assert "was rewound" in text + assert "still there" in text