fix: forward every documented Studio query filter and pin canonicalInputIdentity
Two review Minor findings against the brief itself, both closed: - tests/mocks/handlers/tech-log-studio.ts silently dropped documented query filters instead of mirroring the mock gateway it wraps: listStudioDocuments forwarded only q/limit (dropping kind, publicationStatus, nextAction, projectId, sort, cursor), listStudioCatalog forwarded only type (dropping q/cursor/limit), and listStudioPublications ignored all four of its parameters outright. Added a shared queryParams() helper and forward every field each operation's projectRequest actually emits, plus a regression test that narrows the fixture set by kind through the real HTTP gateway (confirmed it fails without the fix). - canonicalInputIdentity (on the idempotency-safety path, reused by Task 6) had no test. Added three tests against mutationIntent(): a large Korean payload stays within the byte bound, the same input retried twice yields an identical identity, and a mid-codepoint truncation cut leaves no replacement character (confirmed the third fails without the strip). The known collision limitation of any bounded-length identity scheme is documented in the test file rather than solved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
1d01a522de
commit
7424ed4594
@@ -2,12 +2,16 @@ import assert from "node:assert/strict";
|
|||||||
import { afterAll, afterEach, beforeAll, test } from "vitest";
|
import { afterAll, afterEach, beforeAll, test } from "vitest";
|
||||||
import { setupServer } from "msw/node";
|
import { setupServer } from "msw/node";
|
||||||
|
|
||||||
import { createHttpStudioGateway } from "../../../src/features/tech-log/adapters/http/http-studio-gateway.ts";
|
import {
|
||||||
|
createHttpStudioGateway,
|
||||||
|
mutationIntent,
|
||||||
|
} from "../../../src/features/tech-log/adapters/http/http-studio-gateway.ts";
|
||||||
import { createCsrfTokenProvider } from "../../../src/features/tech-log/adapters/http/studio-session-csrf.ts";
|
import { createCsrfTokenProvider } from "../../../src/features/tech-log/adapters/http/studio-session-csrf.ts";
|
||||||
import { createTechLogStudioHandlers } from "../../mocks/handlers/tech-log-studio.ts";
|
import { createTechLogStudioHandlers } from "../../mocks/handlers/tech-log-studio.ts";
|
||||||
import { FIXTURE_IDS } from "../../../src/features/tech-log/adapters/mock/fixtures.ts";
|
import { FIXTURE_IDS } from "../../../src/features/tech-log/adapters/mock/fixtures.ts";
|
||||||
import { isStudioGatewayError } from "../../../src/features/tech-log/application/ports/studio-gateway-error.ts";
|
import { isStudioGatewayError } from "../../../src/features/tech-log/application/ports/studio-gateway-error.ts";
|
||||||
import { TECH_LOG_STUDIO_CONTRIBUTION } from "../../../src/features/tech-log/contracts/tech-log-studio-contract-contribution.ts";
|
import { TECH_LOG_STUDIO_CONTRIBUTION } from "../../../src/features/tech-log/contracts/tech-log-studio-contract-contribution.ts";
|
||||||
|
import { MUTATION_INTENT_BOUNDS } from "../../../src/contracts/mutation-intent.ts";
|
||||||
|
|
||||||
const { handlers } = createTechLogStudioHandlers();
|
const { handlers } = createTechLogStudioHandlers();
|
||||||
const server = setupServer(...handlers);
|
const server = setupServer(...handlers);
|
||||||
@@ -123,6 +127,25 @@ test("wraps the raw port input into the operation's { document } shape", async (
|
|||||||
assert.equal(typeof created.id, "string");
|
assert.equal(typeof created.id, "string");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The MSW handlers wrap the reference mock gateway and must forward every
|
||||||
|
* query parameter `LIST_STUDIO_DOCUMENTS`'s `projectRequest` puts on the
|
||||||
|
* wire. A dropped parameter would silently return the unfiltered set — the
|
||||||
|
* fixtures have 7 documents total and exactly 1 QUESTION
|
||||||
|
* (`FIXTURE_IDS.edgeTokenQuestion`), so a `kind` filter that actually
|
||||||
|
* reaches the mock gateway must narrow the page from 7 to 1.
|
||||||
|
*/
|
||||||
|
test("forwards listDocuments query filters instead of dropping them", async () => {
|
||||||
|
const gateway = createHttpStudioGateway(await realDependencies());
|
||||||
|
const all = await gateway.listDocuments({});
|
||||||
|
const filtered = await gateway.listDocuments({ kind: "QUESTION" });
|
||||||
|
|
||||||
|
assert.ok(all.items.length > 1);
|
||||||
|
assert.equal(filtered.items.length, 1);
|
||||||
|
assert.ok(filtered.items.length < all.items.length);
|
||||||
|
assert.ok(filtered.items.every((item) => item.kind === "QUESTION"));
|
||||||
|
});
|
||||||
|
|
||||||
test("reads the dashboard through the canonical path", async () => {
|
test("reads the dashboard through the canonical path", async () => {
|
||||||
const gateway = createHttpStudioGateway(await realDependencies());
|
const gateway = createHttpStudioGateway(await realDependencies());
|
||||||
const dashboard = await gateway.getDashboard();
|
const dashboard = await gateway.getDashboard();
|
||||||
@@ -197,3 +220,57 @@ test("sends the CSRF header and an Idempotency-Key on every mutation", async ()
|
|||||||
assert.equal(captured[0]!.csrf, "csrf-test-token");
|
assert.equal(captured[0]!.csrf, "csrf-test-token");
|
||||||
assert.equal(captured[0]!.key, "csrf-1");
|
assert.equal(captured[0]!.key, "csrf-1");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* `canonicalInputIdentity` sits on the idempotency-safety path and Task 6
|
||||||
|
* reuses `mutationIntent()`, so its byte-truncation behaviour is pinned here
|
||||||
|
* rather than left implicit. These assert observable properties only — none
|
||||||
|
* of them reimplements the truncation to predict an exact string.
|
||||||
|
*
|
||||||
|
* A large Korean payload is deliberately chosen: each Hangul syllable is 3
|
||||||
|
* UTF-8 bytes, so a character-count-based truncation (the brief's original
|
||||||
|
* approach) would pass a 16,000-*character* check while still exceeding the
|
||||||
|
* 16,384-*byte* bound `defineMutationIntent` enforces, and would throw. 60,000
|
||||||
|
* bytes (20,000 repetitions) comfortably exceeds the bound.
|
||||||
|
*
|
||||||
|
* Known/accepted limitation, not fixed here: two different very large inputs
|
||||||
|
* that share a long common prefix can truncate to the same identity and
|
||||||
|
* collide. This is inherent to any bounded-length identity scheme, not
|
||||||
|
* specific to this truncation strategy, so it is left as-is.
|
||||||
|
*/
|
||||||
|
const BIG_KOREAN_PAYLOAD = { document: { bodyMarkdown: "가".repeat(20_000) } };
|
||||||
|
|
||||||
|
test("canonicalInputIdentity stays within the byte bound for a large Korean payload", () => {
|
||||||
|
const intent = mutationIntent(
|
||||||
|
"saveStudioDocument",
|
||||||
|
"byte-bound-1",
|
||||||
|
BIG_KOREAN_PAYLOAD,
|
||||||
|
);
|
||||||
|
const byteLength = new TextEncoder().encode(
|
||||||
|
intent.canonicalInputIdentity,
|
||||||
|
).byteLength;
|
||||||
|
assert.ok(byteLength <= MUTATION_INTENT_BOUNDS.canonicalInputIdentityMaxBytes);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("the same large input produces the identical canonicalInputIdentity on retry", () => {
|
||||||
|
const first = mutationIntent(
|
||||||
|
"saveStudioDocument",
|
||||||
|
"retry-attempt-1",
|
||||||
|
BIG_KOREAN_PAYLOAD,
|
||||||
|
);
|
||||||
|
const second = mutationIntent(
|
||||||
|
"saveStudioDocument",
|
||||||
|
"retry-attempt-2",
|
||||||
|
{ document: { bodyMarkdown: "가".repeat(20_000) } },
|
||||||
|
);
|
||||||
|
assert.equal(first.canonicalInputIdentity, second.canonicalInputIdentity);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("a mid-codepoint truncation cut leaves no replacement character behind", () => {
|
||||||
|
const intent = mutationIntent(
|
||||||
|
"saveStudioDocument",
|
||||||
|
"no-replacement-1",
|
||||||
|
BIG_KOREAN_PAYLOAD,
|
||||||
|
);
|
||||||
|
assert.ok(!intent.canonicalInputIdentity.includes("�"));
|
||||||
|
});
|
||||||
|
|||||||
@@ -23,6 +23,30 @@ async function json(work: () => Promise<unknown>, status = 200) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Forwards every query parameter a `projectRequest` puts on the wire, not
|
||||||
|
* just the ones a first pass happened to reach for. A handler that silently
|
||||||
|
* drops a documented filter lets a later filter test pass for the wrong
|
||||||
|
* reason — it would observe the *unfiltered* mock result and never learn the
|
||||||
|
* parameter never left the URL.
|
||||||
|
*/
|
||||||
|
function queryParams(
|
||||||
|
url: URL,
|
||||||
|
stringNames: readonly string[],
|
||||||
|
numberNames: readonly string[] = [],
|
||||||
|
): Record<string, string | number> {
|
||||||
|
const result: Record<string, string | number> = {};
|
||||||
|
for (const name of stringNames) {
|
||||||
|
const value = url.searchParams.get(name);
|
||||||
|
if (value !== null) result[name] = value;
|
||||||
|
}
|
||||||
|
for (const name of numberNames) {
|
||||||
|
const value = url.searchParams.get(name);
|
||||||
|
if (value !== null) result[name] = Number(value);
|
||||||
|
}
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
export function createTechLogStudioHandlers(
|
export function createTechLogStudioHandlers(
|
||||||
gateway = createMockStudioGateway(),
|
gateway = createMockStudioGateway(),
|
||||||
baseUrl = BASE,
|
baseUrl = BASE,
|
||||||
@@ -47,12 +71,14 @@ export function createTechLogStudioHandlers(
|
|||||||
),
|
),
|
||||||
http.get(`${baseUrl}/api/v1/studio/documents`, ({ request }) => {
|
http.get(`${baseUrl}/api/v1/studio/documents`, ({ request }) => {
|
||||||
const url = new URL(request.url);
|
const url = new URL(request.url);
|
||||||
const limit = url.searchParams.get("limit");
|
|
||||||
return json(() =>
|
return json(() =>
|
||||||
gateway.listDocuments({
|
gateway.listDocuments(
|
||||||
...(url.searchParams.get("q") ? { q: url.searchParams.get("q")! } : {}),
|
queryParams(
|
||||||
...(limit ? { limit: Number(limit) } : {}),
|
url,
|
||||||
} as never),
|
["q", "kind", "publicationStatus", "nextAction", "projectId", "sort", "cursor"],
|
||||||
|
["limit"],
|
||||||
|
) as never,
|
||||||
|
),
|
||||||
);
|
);
|
||||||
}),
|
}),
|
||||||
http.post(`${baseUrl}/api/v1/studio/documents`, async ({ request }) =>
|
http.post(`${baseUrl}/api/v1/studio/documents`, async ({ request }) =>
|
||||||
@@ -117,9 +143,14 @@ export function createTechLogStudioHandlers(
|
|||||||
),
|
),
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
http.get(`${baseUrl}/api/v1/studio/publications`, () =>
|
http.get(`${baseUrl}/api/v1/studio/publications`, ({ request }) => {
|
||||||
json(() => gateway.listPublications({})),
|
const url = new URL(request.url);
|
||||||
),
|
return json(() =>
|
||||||
|
gateway.listPublications(
|
||||||
|
queryParams(url, ["q", "type", "cursor"], ["limit"]) as never,
|
||||||
|
),
|
||||||
|
);
|
||||||
|
}),
|
||||||
http.post(
|
http.post(
|
||||||
`${baseUrl}/api/v1/studio/publications/:publicationId/unpublish`,
|
`${baseUrl}/api/v1/studio/publications/:publicationId/unpublish`,
|
||||||
async ({ request, params }) =>
|
async ({ request, params }) =>
|
||||||
@@ -139,7 +170,9 @@ export function createTechLogStudioHandlers(
|
|||||||
http.get(`${baseUrl}/api/v1/studio/catalog`, ({ request }) => {
|
http.get(`${baseUrl}/api/v1/studio/catalog`, ({ request }) => {
|
||||||
const url = new URL(request.url);
|
const url = new URL(request.url);
|
||||||
return json(() =>
|
return json(() =>
|
||||||
gateway.getCatalog({ type: url.searchParams.get("type") as never }),
|
gateway.getCatalog(
|
||||||
|
queryParams(url, ["type", "q", "cursor"], ["limit"]) as never,
|
||||||
|
),
|
||||||
);
|
);
|
||||||
}),
|
}),
|
||||||
],
|
],
|
||||||
|
|||||||
Reference in New Issue
Block a user