feat: assistant IA — approbation groupee des actions, bloc d'etapes, refresh UI et bouton Stop (BUG-074, BUG-075, BUG-076, BUG-077)
This commit is contained in:
+158
-2
@@ -104,6 +104,7 @@ async function main() {
|
||||
const bookslmMod = await import(pathToFileURL(path.join(JS_DIR, "bookslm.js")).href);
|
||||
const { BooksLM, MODE } = bookslmMod;
|
||||
const { collectOpenDocuments, documentsSignature } = bookslmMod;
|
||||
const { t } = await import(pathToFileURL(path.join(JS_DIR, "i18n.js")).href);
|
||||
const { state } = await import(pathToFileURL(path.join(JS_DIR, "state.js")).href);
|
||||
|
||||
// ── 1. Rewrite modal resolves with the typed instruction ──
|
||||
@@ -213,8 +214,14 @@ async function main() {
|
||||
const { readFileSync } = await import("node:fs");
|
||||
const src = readFileSync(path.join(JS_DIR, "bookslm.js"), "utf-8");
|
||||
const apply = src.slice(src.indexOf("async _applyConfirmation"));
|
||||
assert.match(apply, /payload:\s*msg\.payload\s*\?\s*\{[^\n]*\.\.\.msg\.payload[^\n]*\}\s*:\s*null/,
|
||||
"continuation must inherit the original request payload (no null payload)");
|
||||
// BUG-075: the whole batch is approved at once and the continuation keeps
|
||||
// streaming into the SAME message (its `payload` stays available, so the
|
||||
// BUG-046 null-payload regression cannot reappear).
|
||||
assert.match(apply, /confirm_all:\s*true/, "global approval flag sent");
|
||||
assert.match(apply, /await this\._streamResponse\(resp, msg, payload\)/,
|
||||
"resume streams into the original message (payload preserved)");
|
||||
assert.doesNotMatch(apply, /this\._messages\.push\(continuation\)/,
|
||||
"no fragmented continuation message per approval");
|
||||
assert.match(apply, /await this\._responseError\(resp\)/,
|
||||
"resume path must format non-OK responses via _responseError");
|
||||
});
|
||||
@@ -550,6 +557,50 @@ async function main() {
|
||||
assert.ok(summary.getAttribute("aria-label"), "the state is announced to screen readers");
|
||||
});
|
||||
|
||||
await test("the steps header previews the first action and counts actions only (BUG-074)", () => {
|
||||
const b = new BooksLM();
|
||||
// Two reasoning notes + one action → the header counts 1 action and
|
||||
// previews it; the notes must not inflate the counter.
|
||||
const calls = [
|
||||
{ name: "", ok: true, step: { key: "thought", params: { value: "je réfléchis" } } },
|
||||
{ name: "create_file", ok: true, step: { key: "file_create", params: { value: "a.md" } } },
|
||||
{ name: "", ok: true, step: { key: "thought", params: { value: "encore" } } },
|
||||
];
|
||||
const trace = b._renderToolActivity(calls, {}, false);
|
||||
const summary = trace.querySelector("summary");
|
||||
assert.equal(
|
||||
summary.querySelector(".bookslm-steps-label").textContent,
|
||||
t("ai.steps_count", { count: 1 }),
|
||||
"only actions are counted",
|
||||
);
|
||||
const title = summary.querySelector(".bookslm-steps-title");
|
||||
assert.ok(title, "the first action is previewed in the collapsed header");
|
||||
assert.ok(title.textContent.startsWith("— "), "preview is prefixed with a dash");
|
||||
assert.ok(title.textContent.includes(b._stepText(calls[1])),
|
||||
"preview shows the first action's label");
|
||||
// Several actions → plural label.
|
||||
const two = b._renderToolActivity([
|
||||
{ name: "create_file", ok: true, step: { key: "file_create", params: { value: "a.md" } } },
|
||||
{ name: "create_directory", ok: true, step: { key: "dir_create", params: { value: "d" } } },
|
||||
], {}, false);
|
||||
assert.equal(
|
||||
two.querySelector(".bookslm-steps-label").textContent,
|
||||
t("ai.steps_count_plural", { count: 2 }),
|
||||
"plural for several actions",
|
||||
);
|
||||
});
|
||||
|
||||
await test("_actionStepCount ignores reasoning notes (BUG-074)", () => {
|
||||
const b = new BooksLM();
|
||||
assert.equal(b._actionStepCount([
|
||||
{ step: { key: "thought" } },
|
||||
{ step: { key: "file_create" } },
|
||||
]), 1, "only the action is counted");
|
||||
assert.equal(b._actionStepCount([{ step: { key: "thought" } }]), 1,
|
||||
"a thoughts-only block still shows 1, never 0");
|
||||
assert.equal(b._actionStepCount([]), 0);
|
||||
});
|
||||
|
||||
await test("reasoning notes become « Réflexion » sub-sections, open state kept", () => {
|
||||
const b = new BooksLM();
|
||||
const msg = {};
|
||||
@@ -1206,14 +1257,119 @@ async function main() {
|
||||
assert.equal(msg.confirmation, null, "confirmation resolved");
|
||||
assert.ok(posted, "resumed via /agent");
|
||||
assert.equal(posted.confirm.error.tool, "edit_file");
|
||||
assert.equal(posted.confirm_all, true, "BUG-075: global approval flag sent");
|
||||
assert.deepEqual(posted.confirm_messages, [{ role: "system", content: "s" }]);
|
||||
// BUG-074: the continuation stays in the same message (cumulative steps).
|
||||
assert.equal(b._messages.length, 1, "no fragmented continuation message");
|
||||
const cont = b._messages[b._messages.length - 1];
|
||||
assert.equal(cont, msg, "streams into the original message");
|
||||
assert.equal(cont.content, "Fait.");
|
||||
assert.equal(cont.toolCalls.length, 1, "tool trace captured");
|
||||
b._panel.remove();
|
||||
localStorage.clear();
|
||||
});
|
||||
|
||||
await test("a batch confirmation lists every action and offers one approval (BUG-075)", () => {
|
||||
const b = new BooksLM();
|
||||
b._panel = b._render();
|
||||
document.body.appendChild(b._panel);
|
||||
b._fillConfirmationDiff = async () => {};
|
||||
const msg = {
|
||||
role: "assistant",
|
||||
content: "",
|
||||
confirmation: {
|
||||
pending: {
|
||||
error: { tool: "create_file", arguments: { vault: "V", path: "a.md" }, id: "1" },
|
||||
actions: [
|
||||
{ id: "1", tool: "create_directory", step: { key: "dir_create", params: { value: "proj" } }, arguments: { vault: "V", path: "proj" } },
|
||||
{ id: "2", tool: "create_file", step: { key: "file_create", params: { value: "proj/a.md" } }, arguments: { vault: "V", path: "proj/a.md", content: "x" } },
|
||||
],
|
||||
},
|
||||
messages: [],
|
||||
},
|
||||
};
|
||||
const card = b._renderConfirmationCard(msg);
|
||||
const rows = card.querySelectorAll(".bookslm-confirm-action");
|
||||
assert.equal(rows.length, 2, "one row per pending action");
|
||||
assert.ok(
|
||||
rows[0].querySelector("summary").textContent.includes(
|
||||
b._stepText({ step: { key: "dir_create", params: { value: "proj" } }, name: "create_directory" }),
|
||||
),
|
||||
"each row shows the action's human label",
|
||||
);
|
||||
const apply = card.querySelector(".bookslm-action-apply");
|
||||
assert.equal(apply.textContent, t("ai.action_apply_all", { count: 2 }),
|
||||
"a single global approval button");
|
||||
b._panel.remove();
|
||||
});
|
||||
|
||||
await test("single-action confirmation keeps the legacy Apply label (BUG-075)", () => {
|
||||
const b = new BooksLM();
|
||||
b._panel = b._render();
|
||||
document.body.appendChild(b._panel);
|
||||
b._fillConfirmationDiff = async () => {};
|
||||
const msg = {
|
||||
role: "assistant",
|
||||
content: "",
|
||||
confirmation: {
|
||||
pending: { error: { tool: "edit_file", arguments: { vault: "V", path: "a.md", content: "x" }, id: "1" } },
|
||||
messages: [],
|
||||
},
|
||||
};
|
||||
const card = b._renderConfirmationCard(msg);
|
||||
assert.equal(card.querySelectorAll(".bookslm-confirm-action").length, 0,
|
||||
"no batch list for a single action");
|
||||
assert.equal(card.querySelector(".bookslm-action-apply").textContent, t("ai.action_apply"));
|
||||
b._panel.remove();
|
||||
});
|
||||
|
||||
await test("the send button doubles as a Stop control while running (BUG-077)", () => {
|
||||
const b = new BooksLM();
|
||||
b._panel = b._render();
|
||||
document.body.appendChild(b._panel);
|
||||
const btn = b._panel.querySelector(".bookslm-btn-send");
|
||||
b._syncSendButton();
|
||||
assert.equal(btn.querySelector("i").getAttribute("data-lucide"), "arrow-up");
|
||||
assert.ok(!btn.classList.contains("is-stopping"), "idle: normal send button");
|
||||
|
||||
let aborted = false;
|
||||
b._isLoading = true;
|
||||
b._abortCtrl = { abort: () => { aborted = true; } };
|
||||
b._syncSendButton();
|
||||
assert.ok(btn.classList.contains("is-stopping"), "stop state applied while running");
|
||||
assert.equal(btn.querySelector("i").getAttribute("data-lucide"), "square");
|
||||
btn.click();
|
||||
assert.ok(aborted, "clicking while running aborts the request");
|
||||
b._isLoading = false;
|
||||
b._panel.remove();
|
||||
});
|
||||
|
||||
await test("_markStopped appends a visible stop marker (BUG-077)", () => {
|
||||
const b = new BooksLM();
|
||||
const msg = { content: "Début de réponse" };
|
||||
b._markStopped(msg);
|
||||
assert.ok(msg.content.includes("⏹"), "marker present");
|
||||
assert.ok(msg.content.startsWith("Début de réponse"), "partial answer kept");
|
||||
assert.ok(!msg.content.includes("⏹\n"), "marker separated from the answer");
|
||||
const empty = { content: "" };
|
||||
b._markStopped(empty);
|
||||
assert.ok(empty.content.includes("⏹"));
|
||||
assert.ok(!empty.content.startsWith("\n"), "no leading blank line for an empty answer");
|
||||
});
|
||||
|
||||
await test("document writes notify the viewer; path-less tools do not (BUG-076)", () => {
|
||||
const b = new BooksLM();
|
||||
const seen = [];
|
||||
const handler = (e) => seen.push(e.detail);
|
||||
window.addEventListener("obsigate:file-written", handler);
|
||||
b._notifyFileWritten({ name: "create_xlsx", ok: true, arguments: { vault: "V", path: "a.xlsx" } });
|
||||
b._notifyFileWritten({ name: "edit_file", ok: false, arguments: { vault: "V", path: "b.md" } });
|
||||
b._notifyFileWritten({ name: "replace_in_files", ok: true, arguments: { vault: "V" } });
|
||||
window.removeEventListener("obsigate:file-written", handler);
|
||||
assert.equal(seen.length, 1, "only the successful path-bearing write notifies");
|
||||
assert.equal(seen[0].path, "a.xlsx");
|
||||
});
|
||||
|
||||
// ── 29. Resizable panel bounds and persistence (#81) ──
|
||||
await test("panel width is clamped and persisted", async () => {
|
||||
localStorage.clear();
|
||||
|
||||
@@ -333,8 +333,13 @@ test("bookslm.js reports the edited document and the assistant writes", () => {
|
||||
assert.match(bookslmSrc, /this\._notifyFileWritten\(data\);/);
|
||||
const notify = bookslmSrc.match(/_notifyFileWritten\(data\) \{([\s\S]*?)\n \}/);
|
||||
assert.ok(notify, "_notifyFileWritten not found");
|
||||
for (const tool of ["edit_file", "append_to_file", "create_file", "restore_backup"]) {
|
||||
assert.ok(notify[1].includes(`'${tool}'`), `${tool} missing from the write tools`);
|
||||
// BUG-076: the write-tool list is a module-level set (documents included).
|
||||
assert.match(notify[1], /FILE_WRITE_TOOLS\.has\(data\.name\)/);
|
||||
const writeSet = bookslmSrc.match(/const FILE_WRITE_TOOLS = new Set\(\[([\s\S]*?)\]\);/);
|
||||
assert.ok(writeSet, "FILE_WRITE_TOOLS not found");
|
||||
for (const tool of ["edit_file", "append_to_file", "create_file", "restore_backup",
|
||||
"create_xlsx", "create_docx", "create_csv", "create_pdf"]) {
|
||||
assert.ok(writeSet[1].includes(`'${tool}'`), `${tool} missing from the write tools`);
|
||||
}
|
||||
assert.match(notify[1], /new CustomEvent\('obsigate:file-written'/);
|
||||
});
|
||||
|
||||
+34
-13
@@ -281,12 +281,13 @@ class TestConfirmationResume:
|
||||
assert assistant_tool_msgs[0]["tool_calls"][0]["id"] == "call_9"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_confirmation_with_parallel_calls_keeps_conversation_valid(self, monkeypatch):
|
||||
"""BUG-050: pausing on one tool call of a batch must answer the others.
|
||||
async def test_confirmation_batches_all_writes_and_keeps_conversation_valid(self, monkeypatch):
|
||||
"""BUG-075 (was BUG-050): one pause batches every mutating call.
|
||||
|
||||
The assistant message lists every tool call of the response, so the
|
||||
provider rejects the resumed turn when a ``tool_call_id`` has no tool
|
||||
result (the "create a folder and a file inside" scenario).
|
||||
result. Mutating calls of the batch are now all pending (one approval
|
||||
applies them together); no dangling id remains.
|
||||
"""
|
||||
_register(monkeypatch, "_write", lambda ctx, params: {"done": params}, risk=ToolRisk.WRITE)
|
||||
|
||||
@@ -297,14 +298,15 @@ class TestConfirmationResume:
|
||||
paused = await run_agent([{"role": "user", "content": "write both"}], ctx=_ctx(), llm=llm1)
|
||||
assert paused.stopped == STOP_CONFIRMATION_REQUIRED
|
||||
assert paused.pending["error"]["id"] == "1"
|
||||
|
||||
# The call that was not reached is answered right away; the pending one
|
||||
# gets its result on resume, when the user applies it.
|
||||
# Both mutating calls are batched into the single confirmation.
|
||||
actions = paused.pending["actions"]
|
||||
assert [a["id"] for a in actions] == ["1", "2"]
|
||||
assert actions[0]["step"]["key"] == "generic"
|
||||
# Nothing is executed nor deferred while waiting for the approval.
|
||||
answered = {m["tool_call_id"] for m in paused.messages if m.get("role") == "tool"}
|
||||
assert "2" in answered
|
||||
assert "1" not in answered
|
||||
assert answered == set()
|
||||
|
||||
# Resume: the pending call is applied, the next turn stays valid.
|
||||
# Resume: both pending calls are applied, the next turn stays valid.
|
||||
llm2 = ScriptedLLM([LLMResponse(content="ok")])
|
||||
resumed = await run_agent(
|
||||
[{"role": "user", "content": "write both"}],
|
||||
@@ -315,8 +317,8 @@ class TestConfirmationResume:
|
||||
)
|
||||
assert resumed.stopped == STOP_DONE
|
||||
assert resumed.content == "ok"
|
||||
assert len(resumed.tool_calls) == 1
|
||||
assert resumed.tool_calls[0].ok is True
|
||||
assert [r.name for r in resumed.tool_calls] == ["_write", "_write"]
|
||||
assert all(r.ok for r in resumed.tool_calls)
|
||||
# Before the resumed LLM call, every announced tool_call_id is answered.
|
||||
resumed_messages = llm2.calls[0]["messages"]
|
||||
assistant = next(
|
||||
@@ -326,12 +328,31 @@ class TestConfirmationResume:
|
||||
announced = {tc["id"] for tc in assistant["tool_calls"]}
|
||||
answered = {m["tool_call_id"] for m in resumed_messages if m.get("role") == "tool"}
|
||||
assert announced <= answered
|
||||
# The skipped call is flagged "deferred" so the model can re-issue it.
|
||||
# No deferred result: the batched calls were approved, not skipped.
|
||||
deferred = [
|
||||
m for m in resumed_messages
|
||||
if m.get("role") == "tool" and json.loads(m["content"]).get("status") == "deferred"
|
||||
]
|
||||
assert [m["tool_call_id"] for m in deferred] == ["2"]
|
||||
assert deferred == []
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_confirmation_runs_read_calls_of_the_batch_immediately(self, monkeypatch):
|
||||
"""Only mutating calls are batched; read-only calls of the turn run now."""
|
||||
_register(monkeypatch, "_write", lambda ctx, params: {"done": params}, risk=ToolRisk.WRITE)
|
||||
_register(monkeypatch, "_read", lambda ctx, params: {"value": 1})
|
||||
|
||||
llm1 = ScriptedLLM([LLMResponse(tool_calls=[
|
||||
ToolCall(id="1", name="_write", arguments={"x": 1}),
|
||||
ToolCall(id="2", name="_read", arguments={}),
|
||||
ToolCall(id="3", name="_write", arguments={"x": 3}),
|
||||
])])
|
||||
paused = await run_agent([{"role": "user", "content": "write and read"}], ctx=_ctx(), llm=llm1)
|
||||
assert paused.stopped == STOP_CONFIRMATION_REQUIRED
|
||||
# The read ran while the two writes are pending.
|
||||
assert [r.name for r in paused.tool_calls] == ["_read"]
|
||||
assert [a["id"] for a in paused.pending["actions"]] == ["1", "3"]
|
||||
answered = {m["tool_call_id"] for m in paused.messages if m.get("role") == "tool"}
|
||||
assert answered == {"2"}
|
||||
|
||||
|
||||
class TestAgentPermissions:
|
||||
|
||||
@@ -1065,6 +1065,81 @@ class TestBooksLMAgentEndpoint:
|
||||
assert "C'est fait." in resp2.text
|
||||
assert "event: message" in resp2.text
|
||||
|
||||
def test_agent_confirmation_batches_actions_and_confirm_all(self, bookslm_client, monkeypatch):
|
||||
"""BUG-075: one confirmation lists every mutating call of the turn and
|
||||
``confirm_all`` authorizes the rest of the run without pausing again."""
|
||||
import re
|
||||
|
||||
import backend.bookslm_routes as routes
|
||||
from backend.ai_chat import LLMResponse, ToolCall
|
||||
from backend.tools import registry
|
||||
from backend.tools.api import ToolRisk
|
||||
from backend.tools.registry import ToolSpec
|
||||
from backend.tools.schemas import ListVaultsInput
|
||||
|
||||
calls = []
|
||||
|
||||
def handler(ctx, params):
|
||||
calls.append(params)
|
||||
return {"ok": True}
|
||||
|
||||
spec = ToolSpec(
|
||||
name="_agent_write_batch",
|
||||
description="write for tests",
|
||||
input_model=ListVaultsInput,
|
||||
handler=handler,
|
||||
risk=ToolRisk.WRITE,
|
||||
)
|
||||
monkeypatch.setitem(registry._REGISTRY, "_agent_write_batch", spec)
|
||||
|
||||
responses = [LLMResponse(tool_calls=[
|
||||
ToolCall(id="c1", name="_agent_write_batch", arguments={}),
|
||||
ToolCall(id="c2", name="_agent_write_batch", arguments={}),
|
||||
])]
|
||||
|
||||
async def fake_chat_completion(messages, **kwargs):
|
||||
return responses.pop(0)
|
||||
|
||||
monkeypatch.setattr(routes, "chat_completion", fake_chat_completion)
|
||||
monkeypatch.setattr(routes, "_resolve_provider_name", lambda requested: "deepseek")
|
||||
|
||||
token, _ = _login_bookslm(bookslm_client)
|
||||
payload = {"vault": "TestVault", "directory": "", "message": "crée tout", "mode": "directory"}
|
||||
resp = bookslm_client.post(
|
||||
"/api/ai/bookslm/agent",
|
||||
json=payload,
|
||||
headers={"Authorization": f"Bearer {token}"},
|
||||
)
|
||||
assert "event: confirmation" in resp.text
|
||||
match = re.search(r"event: confirmation\ndata: (.*)", resp.text)
|
||||
assert match, resp.text
|
||||
confirmation = json.loads(match.group(1))
|
||||
# Both mutating calls are pending, not one.
|
||||
assert [a["tool"] for a in confirmation["pending"]["actions"]] == [
|
||||
"_agent_write_batch", "_agent_write_batch",
|
||||
]
|
||||
assert calls == [], "nothing runs before the approval"
|
||||
|
||||
# Resume with a global approval: the two pending calls run, and a third
|
||||
# mutating call of the same run runs without a second confirmation.
|
||||
responses.append(LLMResponse(tool_calls=[ToolCall(id="c3", name="_agent_write_batch", arguments={})]))
|
||||
responses.append(LLMResponse(content="Terminé."))
|
||||
resume_payload = dict(
|
||||
payload,
|
||||
confirm=confirmation["pending"],
|
||||
confirm_messages=confirmation["messages"],
|
||||
confirm_all=True,
|
||||
)
|
||||
resp2 = bookslm_client.post(
|
||||
"/api/ai/bookslm/agent",
|
||||
json=resume_payload,
|
||||
headers={"Authorization": f"Bearer {token}"},
|
||||
)
|
||||
assert resp2.status_code == 200
|
||||
assert "event: confirmation" not in resp2.text, "confirm_all must not pause again"
|
||||
assert len(calls) == 3, "the whole plan ran in one approval"
|
||||
assert "Terminé." in resp2.text
|
||||
|
||||
def test_agent_message_reports_effective_model(self, bookslm_client, monkeypatch):
|
||||
"""The SSE payload carries the model really used, not the raw request.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user