8-point multi-aspect review of the batch PR; security/regressions were clean. 1. Lease leak: the #180 reorder moved `toolsFor` (which leases external MCP clients, refCount+1) ahead of buildSystemPrompt + forUser, but the only release (closeExternalClients) was bound to the streamText callbacks. A throw in between leaked the lease (refCount stuck, undici sockets held until restart). Define closeExternalClients right after the lease and wrap buildSystemPrompt+forUser in try/catch that closes-then-rethrows. 2. Cover the patch_node/delete_node dup-id refusal (#159 #6): extract the guard into a pure `assertUnambiguousMatch` (node-ops) and unit-test 0/1/>1. 3. Regress the body-before-title order (#159 #10): mock-HTTP test (collab fails fast against a server with no WS upgrade) asserts /pages/update (title) is NEVER posted when the body write fails — for updatePage AND updatePageJson. 4. CHANGELOG [Unreleased]: #180, #168 (Added); #163 (Fixed). 5. Add the missing en-US i18n keys (Back to references / {{label}}). 6. Drop the duplicate content/empty/blank cases in ai-chat.prompt.spec.ts (they repeat the buildMcpToolingBlock unit tests); keep only sandwich placement + both-safety-copies. 7. CI Postgres pg16 -> pg18 (match docker-compose). 8. jsonb decode seam: shared `parseJsonbValue(value, guard)` in database/utils.ts holds the legacy double-encoding self-heal in one place; parseToolAllowlist / parseModelConfig keep only a type-guard. Verified: server build + 124 unit + 15 integration; mcp 311; prettier clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
107 lines
3.5 KiB
JavaScript
107 lines
3.5 KiB
JavaScript
// Mock-HTTP regression for the body-before-title write order (#159 finding #10,
|
|
// PR #185 review pt 3). `updatePage` / `updatePageJson` must write the page BODY
|
|
// (collab) BEFORE the title (REST POST /pages/update), so a failed body write
|
|
// never leaves a NEW title over the OLD body (split-brain). We point the client
|
|
// at a mock server that serves auth + collab-token but has NO WebSocket upgrade
|
|
// handler, so the collab body write fails fast; we then assert the title was
|
|
// never POSTed. With the pre-fix (title-first) order, /pages/update WOULD be hit
|
|
// before the body failed.
|
|
import { test, after } from "node:test";
|
|
import assert from "node:assert/strict";
|
|
import http from "node:http";
|
|
import { DocmostClient } from "../../build/client.js";
|
|
|
|
function readBody(req) {
|
|
return new Promise((resolve) => {
|
|
let raw = "";
|
|
req.on("data", (c) => (raw += c));
|
|
req.on("end", () => resolve(raw));
|
|
});
|
|
}
|
|
function startServer(handler) {
|
|
return new Promise((resolve) => {
|
|
const server = http.createServer(handler);
|
|
server.listen(0, "127.0.0.1", () => {
|
|
const { port } = server.address();
|
|
resolve({ server, baseURL: `http://127.0.0.1:${port}/api` });
|
|
});
|
|
});
|
|
}
|
|
function sendJson(res, status, obj, extraHeaders = {}) {
|
|
res.writeHead(status, {
|
|
"Content-Type": "application/json",
|
|
...extraHeaders,
|
|
});
|
|
res.end(JSON.stringify(obj));
|
|
}
|
|
|
|
const openServers = [];
|
|
async function spawn(handler) {
|
|
const { server, baseURL } = await startServer(handler);
|
|
openServers.push(server);
|
|
return { server, baseURL };
|
|
}
|
|
after(async () => {
|
|
await Promise.all(openServers.map((s) => new Promise((r) => s.close(r))));
|
|
});
|
|
|
|
// A mock server that authenticates and hands out a collab token, tracks whether
|
|
// the title endpoint was hit, but has NO WS upgrade handler -> collab fails fast.
|
|
function makeServer() {
|
|
const state = { titlePosted: false };
|
|
const handler = async (req, res) => {
|
|
await readBody(req);
|
|
if (req.url === "/api/auth/login") {
|
|
sendJson(
|
|
res,
|
|
200,
|
|
{ success: true },
|
|
{
|
|
"Set-Cookie": "authToken=t; Path=/; HttpOnly",
|
|
},
|
|
);
|
|
return;
|
|
}
|
|
if (req.url === "/api/auth/collab-token") {
|
|
sendJson(res, 200, { data: { token: "collab-jwt" } });
|
|
return;
|
|
}
|
|
if (req.url === "/api/pages/update") {
|
|
state.titlePosted = true;
|
|
sendJson(res, 200, { data: {} });
|
|
return;
|
|
}
|
|
sendJson(res, 404, { message: "not found" });
|
|
};
|
|
return { state, handler };
|
|
}
|
|
|
|
test("updatePage does NOT POST the title when the body (collab) write fails (#159)", async () => {
|
|
const { state, handler } = makeServer();
|
|
const { baseURL } = await spawn(handler);
|
|
const client = new DocmostClient(baseURL, "u@e.com", "pw");
|
|
|
|
await assert.rejects(() =>
|
|
client.updatePage("page-1", "# Heading\n\nsome body", "New Title"),
|
|
);
|
|
assert.equal(
|
|
state.titlePosted,
|
|
false,
|
|
"title must NOT be posted when the body write failed (body-first order)",
|
|
);
|
|
});
|
|
|
|
test("updatePageJson does NOT POST the title when the body (collab) write fails (#159)", async () => {
|
|
const { state, handler } = makeServer();
|
|
const { baseURL } = await spawn(handler);
|
|
const client = new DocmostClient(baseURL, "u@e.com", "pw");
|
|
|
|
const doc = { type: "doc", content: [{ type: "paragraph" }] };
|
|
await assert.rejects(() => client.updatePageJson("page-1", doc, "New Title"));
|
|
assert.equal(
|
|
state.titlePosted,
|
|
false,
|
|
"title must NOT be posted when the body write failed (body-first order)",
|
|
);
|
|
});
|