fix: scope proposals to chat users
This commit is contained in:
zenord
@@ -5,7 +5,7 @@ import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import test from "node:test";
|
||||
import { AssistantManager, parseAssistantActions, parseWorkerResult } from "../src/acp/assistant-manager.js";
|
||||
import { AssistantManager, assistantKeyFor, parseAssistantActions, parseWorkerResult } from "../src/acp/assistant-manager.js";
|
||||
import { parseConfig } from "../src/config.js";
|
||||
import { DurableSessionStore } from "../src/core/durable-session-store.js";
|
||||
import { ProposalStore } from "../src/core/proposal-store.js";
|
||||
@@ -13,6 +13,7 @@ import { BotProfileResolver } from "../src/roles/role-registry.js";
|
||||
|
||||
const fixture = path.resolve("test/fixtures/fake-acp-agent.mjs");
|
||||
const CHAT_KEY = "webhook:chat-1";
|
||||
const CONVERSATION_KEY = assistantKeyFor(CHAT_KEY, "user-1");
|
||||
|
||||
interface Harness {
|
||||
home: string;
|
||||
@@ -80,7 +81,7 @@ async function closeHarness(harness: Harness): Promise<void> {
|
||||
await fs.promises.rm(harness.workspace, { recursive: true, force: true });
|
||||
}
|
||||
|
||||
const request = (text: string) => ({ platform: "webhook", chatId: "chat-1", userId: "user-1", text });
|
||||
const request = (text: string, userId = "user-1") => ({ platform: "webhook", chatId: "chat-1", userId, text });
|
||||
|
||||
async function waitFor(condition: () => boolean, timeoutMs = 8_000): Promise<void> {
|
||||
const deadline = Date.now() + timeoutMs;
|
||||
@@ -105,7 +106,7 @@ test("create proposal stays proposed until confirm; confirm starts the worker wi
|
||||
assert.equal(proposal.status, "proposed");
|
||||
assert.equal(proposal.ownerChatKey, CHAT_KEY);
|
||||
assert.equal(readLog(harness.logFile).filter((entry) => entry.method === "session/new" && entry.cwd === harness.workspace).length, 0);
|
||||
assert.ok(harness.store.getBinding(CHAT_KEY));
|
||||
assert.ok(harness.store.getBinding(CONVERSATION_KEY));
|
||||
|
||||
await harness.manager.prompt(request("confirm"));
|
||||
const working = harness.proposals.get(proposal.id)!;
|
||||
@@ -154,7 +155,7 @@ test("stop does not deadlock while a worker result is waiting to settle", async
|
||||
|
||||
fs.writeFileSync(gateFile, "go\n", { mode: 0o600 });
|
||||
await settleReached;
|
||||
const stopped = harness.manager.stop("webhook", "chat-1");
|
||||
const stopped = harness.manager.stop("webhook", "chat-1", "user-1");
|
||||
assert.ok(startSettle);
|
||||
void startSettle();
|
||||
assert.equal(await stopped, true);
|
||||
@@ -259,10 +260,10 @@ test("assistant tool activity fails closed and the next turn starts a clean sess
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
await assert.rejects(harness.manager.prompt(request("force tool")), /forbidden tool activity/);
|
||||
assert.equal(harness.store.getBinding(CHAT_KEY), undefined);
|
||||
assert.equal(harness.store.getBinding(CONVERSATION_KEY), undefined);
|
||||
const ok = await harness.manager.prompt(request("hello"));
|
||||
assert.equal(ok.text, "ok");
|
||||
assert.ok(harness.store.getBinding(CHAT_KEY));
|
||||
assert.ok(harness.store.getBinding(CONVERSATION_KEY));
|
||||
} finally {
|
||||
await closeHarness(harness);
|
||||
}
|
||||
@@ -311,6 +312,117 @@ test("restart recovery kills the persisted worker group and marks the working pr
|
||||
}
|
||||
});
|
||||
|
||||
test("proposals are owned per chat+user: a second user in the same chat cannot confirm, stop, cancel, or list them", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
await harness.manager.prompt(request("create proposal: hang", "user-a"));
|
||||
await harness.manager.prompt(request("hello", "user-b"));
|
||||
assert.equal(harness.store.stats().bindings, 2);
|
||||
const proposal = harness.proposals.list()[0]!;
|
||||
assert.equal(proposal.status, "proposed");
|
||||
assert.equal(proposal.requesterUserId, "user-a");
|
||||
|
||||
assert.equal(await harness.manager.confirm("webhook", "chat-1", "user-b"), false);
|
||||
await harness.manager.prompt(request("confirm", "user-b"));
|
||||
assert.equal(harness.proposals.get(proposal.id)!.status, "proposed");
|
||||
assert.equal(await harness.manager.cancel("webhook", "chat-1", "user-b"), false);
|
||||
await harness.manager.prompt(request("cancel proposal", "user-b"));
|
||||
assert.equal(harness.proposals.get(proposal.id)!.status, "proposed");
|
||||
assert.equal(await harness.manager.stop("webhook", "chat-1", "user-b"), false);
|
||||
assert.equal(harness.manager.listProposals("webhook", "chat-1", "user-b").length, 0);
|
||||
assert.equal(harness.manager.listProposals("webhook", "chat-1", "user-a").length, 1);
|
||||
|
||||
assert.equal(await harness.manager.confirm("webhook", "chat-1", "user-a"), true);
|
||||
assert.equal(harness.proposals.get(proposal.id)!.status, "working");
|
||||
|
||||
await harness.manager.prompt(request("hello again", "user-b"));
|
||||
const bPrompt = readLog(harness.logFile).find((entry) => entry.method === "session/prompt" && entry.text?.startsWith("[User message]\nhello again"));
|
||||
assert.ok(bPrompt?.text);
|
||||
assert.match(bPrompt.text!, /details hidden/);
|
||||
assert.doesNotMatch(bPrompt.text!, /Test proposal/);
|
||||
|
||||
assert.equal(await harness.manager.stop("webhook", "chat-1", "user-b"), false);
|
||||
assert.equal(harness.proposals.get(proposal.id)!.status, "working");
|
||||
assert.equal(await harness.manager.stop("webhook", "chat-1", "user-a"), true);
|
||||
assert.equal(harness.proposals.get(proposal.id)!.status, "cancelled");
|
||||
} finally {
|
||||
await closeHarness(harness);
|
||||
}
|
||||
});
|
||||
|
||||
test("start_next only starts the requesting user's own queued proposal", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
await harness.manager.prompt(request("create proposal: succeed", "user-a"));
|
||||
const first = harness.proposals.list()[0]!;
|
||||
await harness.manager.prompt(request("confirm", "user-a"));
|
||||
await waitFor(() => harness.proposals.get(first.id)!.status === "awaiting_user_confirmation");
|
||||
await harness.manager.prompt(request("create proposal: succeed", "user-a"));
|
||||
const second = harness.proposals.list().find((proposal) => proposal.id !== first.id)!;
|
||||
await harness.manager.prompt(request("confirm", "user-a"));
|
||||
assert.equal(harness.proposals.get(second.id)!.status, "queued");
|
||||
await harness.manager.prompt(request("confirm", "user-a"));
|
||||
assert.equal(harness.proposals.get(first.id)!.status, "completed");
|
||||
assert.equal(harness.proposals.get(second.id)!.status, "queued");
|
||||
|
||||
await harness.manager.prompt(request("start next", "user-b"));
|
||||
assert.equal(harness.proposals.get(second.id)!.status, "queued");
|
||||
await harness.manager.prompt(request("start next", "user-a"));
|
||||
assert.equal(harness.proposals.get(second.id)!.status, "working");
|
||||
await waitFor(() => harness.proposals.get(second.id)!.status === "awaiting_user_confirmation");
|
||||
} finally {
|
||||
await closeHarness(harness);
|
||||
}
|
||||
});
|
||||
|
||||
test("stop settles the latest awaiting proposal (success completes, failure fails, step cancels) without auto-starting the next", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
await harness.manager.prompt(request("create proposal: succeed", "user-a"));
|
||||
const first = harness.proposals.list()[0]!;
|
||||
await harness.manager.prompt(request("confirm", "user-a"));
|
||||
await waitFor(() => harness.proposals.get(first.id)!.status === "awaiting_user_confirmation");
|
||||
assert.equal(harness.proposals.get(first.id)!.pending?.kind, "success");
|
||||
|
||||
await harness.manager.prompt(request("create proposal: succeed", "user-a"));
|
||||
const second = harness.proposals.list().find((proposal) => proposal.id !== first.id)!;
|
||||
await harness.manager.prompt(request("confirm", "user-a"));
|
||||
assert.equal(harness.proposals.get(second.id)!.status, "queued");
|
||||
|
||||
await harness.manager.prompt(request("stop", "user-a"));
|
||||
assert.equal(harness.proposals.get(first.id)!.status, "completed");
|
||||
assert.ok(harness.proposals.get(first.id)!.finishedAt);
|
||||
assert.equal(harness.proposals.get(second.id)!.status, "queued");
|
||||
|
||||
assert.equal(await harness.manager.stop("webhook", "chat-1", "user-b"), false);
|
||||
assert.equal(harness.proposals.get(second.id)!.status, "queued");
|
||||
|
||||
await harness.manager.prompt(request("start next", "user-a"));
|
||||
await waitFor(() => harness.proposals.get(second.id)!.status === "awaiting_user_confirmation");
|
||||
assert.equal(await harness.manager.stop("webhook", "chat-1", "user-a"), true);
|
||||
assert.equal(harness.proposals.get(second.id)!.status, "completed");
|
||||
|
||||
await harness.manager.prompt(request("create proposal: fail", "user-a"));
|
||||
const third = harness.proposals.list().find((proposal) => proposal.id !== first.id && proposal.id !== second.id)!;
|
||||
await harness.manager.prompt(request("confirm", "user-a"));
|
||||
await waitFor(() => harness.proposals.get(third.id)!.status === "awaiting_user_confirmation");
|
||||
assert.equal(harness.proposals.get(third.id)!.pending?.kind, "failure");
|
||||
assert.equal(await harness.manager.stop("webhook", "chat-1", "user-a"), true);
|
||||
assert.equal(harness.proposals.get(third.id)!.status, "failed");
|
||||
|
||||
await harness.manager.prompt(request("create proposal: ask", "user-a"));
|
||||
const fourth = harness.proposals.list().find((proposal) => proposal.status === "proposed")!;
|
||||
await harness.manager.prompt(request("confirm", "user-a"));
|
||||
await waitFor(() => harness.proposals.get(fourth.id)!.status === "awaiting_user_confirmation");
|
||||
assert.equal(harness.proposals.get(fourth.id)!.pending?.kind, "step");
|
||||
assert.equal(await harness.manager.stop("webhook", "chat-1", "user-a"), true);
|
||||
assert.equal(harness.proposals.get(fourth.id)!.status, "cancelled");
|
||||
assert.equal(harness.manager.status("webhook", "chat-1", "user-a").workerRunning, false);
|
||||
} finally {
|
||||
await closeHarness(harness);
|
||||
}
|
||||
});
|
||||
|
||||
test("envelope parsers accept valid tails and reject invalid ones", () => {
|
||||
assert.deepEqual(
|
||||
parseAssistantActions(`text\n<GORI_ASSISTANT_ACTION_V1>{"reply":"hi","actions":[{"type":"stop"}]}</GORI_ASSISTANT_ACTION_V1>`),
|
||||
|
||||
Reference in New Issue
Block a user