diff --git a/supabase/functions/hal-mcp/index.test.ts b/supabase/functions/hal-mcp/index.test.ts index c8075b1..3514524 100644 --- a/supabase/functions/hal-mcp/index.test.ts +++ b/supabase/functions/hal-mcp/index.test.ts @@ -740,18 +740,32 @@ Deno.test("resolveWhoamiPayload — userClaims without email: user_email coalesc }); // --------------------------------------------------------------------------- -// push_mission_context — batch upsert (#25) +// push_mission_context — per-observation targeted UPDATE (#74) // --------------------------------------------------------------------------- -Deno.test("push_mission_context — batch upsert succeeds: updated = rows.length", async () => { - const STATIC_KEY = "test-static-key-abc123"; - Deno.env.set("SUPABASE_SECRET_KEYS", JSON.stringify({ hal_api_key: STATIC_KEY })); +// One .update().eq().select() chain that resolves the given result. select("id") +// is the terminal awaited call, so it returns the promise. Records eq() calls on +// chain._eqCalls so tests can assert the correct row was targeted by note_id. +// deno-lint-ignore no-explicit-any +function noteUpdateChain(result: { data: unknown; error: unknown }): any { + const eqCalls: unknown[][] = []; // deno-lint-ignore no-explicit-any - const upsertMock: any = { - upsert: () => Promise.resolve({ error: null }), + const chain: any = { + update: () => chain, + eq: (col: string, val: unknown) => { eqCalls.push([col, val]); return chain; }, + select: () => Promise.resolve(result), }; + chain._eqCalls = eqCalls; + return chain; +} + +Deno.test("push_mission_context — per-observation update succeeds: updated counts matched rows", async () => { + const STATIC_KEY = "test-static-key-abc123"; + Deno.env.set("SUPABASE_SECRET_KEYS", JSON.stringify({ hal_api_key: STATIC_KEY })); + const chain1 = noteUpdateChain({ data: [{ id: "n1" }], error: null }); + const chain2 = noteUpdateChain({ data: [{ id: "n2" }], error: null }); // deno-lint-ignore no-explicit-any - const s = stub(_adminClient, "from", returnsNext([upsertMock as any])); + const s = stub(_adminClient, "from", returnsNext([chain1 as any, chain2 as any])); try { const res = await fetchHandler(callTool(STATIC_KEY, "push_mission_context", { observations: [ @@ -765,21 +779,21 @@ Deno.test("push_mission_context — batch upsert succeeds: updated = rows.length assertEquals(result.updated, 2); assertEquals(result.skipped, 0); assertEquals(result.errors, []); + assertEquals(chain1._eqCalls, [["id", "n1"]]); + assertEquals(chain2._eqCalls, [["id", "n2"]]); } finally { s.restore(); Deno.env.set("SUPABASE_SECRET_KEYS", DEFAULT_SECRET_KEYS); } }); -Deno.test("push_mission_context — batch upsert DB error returns isError with updated=0", async () => { +Deno.test("push_mission_context — DB error returns isError with updated=0", async () => { const STATIC_KEY = "test-static-key-abc123"; Deno.env.set("SUPABASE_SECRET_KEYS", JSON.stringify({ hal_api_key: STATIC_KEY })); - // deno-lint-ignore no-explicit-any - const upsertMock: any = { - upsert: () => Promise.resolve({ error: { message: "constraint violation" } }), - }; - // deno-lint-ignore no-explicit-any - const s = stub(_adminClient, "from", returnsNext([upsertMock as any])); + const s = stub(_adminClient, "from", returnsNext([ + // deno-lint-ignore no-explicit-any + noteUpdateChain({ data: null, error: { message: "constraint violation" } }) as any, + ])); try { const res = await fetchHandler(callTool(STATIC_KEY, "push_mission_context", { observations: [{ note_id: "n1", description: "Test" }], @@ -788,6 +802,7 @@ Deno.test("push_mission_context — batch upsert DB error returns isError with u assertEquals(body.result.isError, true); const result = JSON.parse(body.result.content[0].text); assertEquals(result.updated, 0); + assertEquals(result.errors[0].includes("n1"), true); assertEquals(result.errors[0].includes("constraint violation"), true); } finally { s.restore(); @@ -798,7 +813,16 @@ Deno.test("push_mission_context — batch upsert DB error returns isError with u Deno.test("push_mission_context — empty patch observation is skipped", async () => { const STATIC_KEY = "test-static-key-abc123"; Deno.env.set("SUPABASE_SECRET_KEYS", JSON.stringify({ hal_api_key: STATIC_KEY })); - // No from() call expected — rows array stays empty, upsert not called + // No from() call expected — the only observation has no updatable fields + let updateCalled = false; + // deno-lint-ignore no-explicit-any + const chain: any = { + update: () => { updateCalled = true; return chain; }, + eq: () => chain, + select: () => Promise.resolve({ data: [], error: null }), + }; + // deno-lint-ignore no-explicit-any + const s = stub(_adminClient, "from", returnsNext([chain as any])); try { const res = await fetchHandler(callTool(STATIC_KEY, "push_mission_context", { observations: [{ note_id: "n1" }], @@ -809,7 +833,9 @@ Deno.test("push_mission_context — empty patch observation is skipped", async ( assertEquals(result.updated, 0); assertEquals(result.skipped, 1); assertEquals(result.errors, []); + assertEquals(updateCalled, false); } finally { + s.restore(); Deno.env.set("SUPABASE_SECRET_KEYS", DEFAULT_SECRET_KEYS); } }); @@ -817,22 +843,94 @@ Deno.test("push_mission_context — empty patch observation is skipped", async ( Deno.test("push_mission_context — assessment does not set condition_index", async () => { const STATIC_KEY = "test-static-key-abc123"; Deno.env.set("SUPABASE_SECRET_KEYS", JSON.stringify({ hal_api_key: STATIC_KEY })); - let capturedRows: unknown[] = []; + let capturedFields: Record = {}; // deno-lint-ignore no-explicit-any - const upsertMock: any = { - upsert: (rows: unknown[]) => { - capturedRows = rows; - return Promise.resolve({ error: null }); - }, + const chain: any = { + update: (fields: Record) => { capturedFields = fields; return chain; }, + eq: () => chain, + select: () => Promise.resolve({ data: [{ id: "n1" }], error: null }), }; // deno-lint-ignore no-explicit-any - const s = stub(_adminClient, "from", returnsNext([upsertMock as any])); + const s = stub(_adminClient, "from", returnsNext([chain as any])); try { const res = await fetchHandler(callTool(STATIC_KEY, "push_mission_context", { observations: [{ note_id: "n1", assessment: "2" }], }, 603)); await parseMcpResponse(res); // consume stream so tool handler completes - assertEquals("condition_index" in (capturedRows[0] as Record), false); + assertEquals("condition_index" in capturedFields, false); + } finally { + s.restore(); + Deno.env.set("SUPABASE_SECRET_KEYS", DEFAULT_SECRET_KEYS); + } +}); + +Deno.test("push_mission_context — unknown note_id is reported in errors, not inserted", async () => { + const STATIC_KEY = "test-static-key-abc123"; + Deno.env.set("SUPABASE_SECRET_KEYS", JSON.stringify({ hal_api_key: STATIC_KEY })); + const s = stub(_adminClient, "from", returnsNext([ + // zero rows matched, no error — an UPDATE cannot insert + // deno-lint-ignore no-explicit-any + noteUpdateChain({ data: [], error: null }) as any, + ])); + try { + const res = await fetchHandler(callTool(STATIC_KEY, "push_mission_context", { + observations: [{ note_id: "does-not-exist", description: "orphan edit" }], + }, 607)); + const body = await parseMcpResponse(res); + assertEquals(body.result.isError, true); + const result = JSON.parse(body.result.content[0].text); + assertEquals(result.updated, 0); + assertEquals(result.errors.some((e: string) => e.includes("does-not-exist") && e.includes("not found")), true); + } finally { + s.restore(); + Deno.env.set("SUPABASE_SECRET_KEYS", DEFAULT_SECRET_KEYS); + } +}); + +Deno.test("push_mission_context — null data (no error) is treated as not found", async () => { + const STATIC_KEY = "test-static-key-abc123"; + Deno.env.set("SUPABASE_SECRET_KEYS", JSON.stringify({ hal_api_key: STATIC_KEY })); + const s = stub(_adminClient, "from", returnsNext([ + // deno-lint-ignore no-explicit-any + noteUpdateChain({ data: null, error: null }) as any, + ])); + try { + const res = await fetchHandler(callTool(STATIC_KEY, "push_mission_context", { + observations: [{ note_id: "does-not-exist", description: "orphan edit" }], + }, 609)); + const body = await parseMcpResponse(res); + assertEquals(body.result.isError, true); + const result = JSON.parse(body.result.content[0].text); + assertEquals(result.updated, 0); + assertEquals(result.errors.some((e: string) => e.includes("does-not-exist") && e.includes("not found")), true); + } finally { + s.restore(); + Deno.env.set("SUPABASE_SECRET_KEYS", DEFAULT_SECRET_KEYS); + } +}); + +Deno.test("push_mission_context — mixed batch: one succeeds, one not found", async () => { + const STATIC_KEY = "test-static-key-abc123"; + Deno.env.set("SUPABASE_SECRET_KEYS", JSON.stringify({ hal_api_key: STATIC_KEY })); + const chain1 = noteUpdateChain({ data: [{ id: "n1" }], error: null }); + const chain2 = noteUpdateChain({ data: [], error: null }); + // deno-lint-ignore no-explicit-any + const s = stub(_adminClient, "from", returnsNext([chain1 as any, chain2 as any])); + try { + const res = await fetchHandler(callTool(STATIC_KEY, "push_mission_context", { + observations: [ + { note_id: "n1", description: "Fissure façade" }, + { note_id: "n2", description: "orphan edit" }, + ], + }, 610)); + const body = await parseMcpResponse(res); + assertEquals(body.result.isError, true); + const result = JSON.parse(body.result.content[0].text); + assertEquals(result.updated, 1); + assertEquals(result.errors.length, 1); + assertEquals(result.errors[0].includes("n2") && result.errors[0].includes("not found"), true); + assertEquals(chain1._eqCalls, [["id", "n1"]]); + assertEquals(chain2._eqCalls, [["id", "n2"]]); } finally { s.restore(); Deno.env.set("SUPABASE_SECRET_KEYS", DEFAULT_SECRET_KEYS); @@ -1076,7 +1174,7 @@ Deno.test("push_mission_context — isError when obs succeed but crop fails", as Deno.env.set("SUPABASE_SECRET_KEYS", JSON.stringify({ hal_api_key: STATIC_KEY })); // deno-lint-ignore no-explicit-any - const notesMock: any = { upsert: () => Promise.resolve({ error: null }) }; + const notesMock: any = noteUpdateChain({ data: [{ id: "n1" }], error: null }); // deno-lint-ignore no-explicit-any const cropMock: any = { upsert: () => Promise.resolve({ error: { message: "crop upsert failed" } }) }; // deno-lint-ignore no-explicit-any diff --git a/supabase/functions/hal-mcp/index.ts b/supabase/functions/hal-mcp/index.ts index 728c07a..b0e05f8 100644 --- a/supabase/functions/hal-mcp/index.ts +++ b/supabase/functions/hal-mcp/index.ts @@ -420,27 +420,29 @@ registerTool( let skipped = 0; const errors: string[] = []; - const rows: Record[] = []; for (const obs of observations) { if (!obs.note_id) { skipped++; continue; } - const row: Record = { id: obs.note_id }; - if (obs.type) row.type = obs.type; - if (obs.description) row.description = obs.description; - if (obs.location) row.location = obs.location; - if (obs.zone) row.zone = obs.zone; - if (obs.assessment !== undefined) { - row.assessment = obs.assessment; + const fields: Record = {}; + if (obs.type) fields.type = obs.type; + if (obs.description) fields.description = obs.description; + if (obs.location) fields.location = obs.location; + if (obs.zone) fields.zone = obs.zone; + if (obs.assessment !== undefined) fields.assessment = obs.assessment; + if (obs.recommendations) fields.recommendations = obs.recommendations; + if (obs.metadata && Object.keys(obs.metadata).length) fields.metadata = obs.metadata; + if (Object.keys(fields).length === 0) { skipped++; continue; } + + const { data, error } = await db.from("edifice_notes") + .update(fields) + .eq("id", obs.note_id) + .select("id"); + if (error) { + errors.push(`note ${obs.note_id}: ${error.message}`); + } else if (!data || data.length === 0) { + errors.push(`note ${obs.note_id}: not found`); + } else { + updated++; } - if (obs.recommendations) row.recommendations = obs.recommendations; - if (obs.metadata && Object.keys(obs.metadata).length) row.metadata = obs.metadata; - if (Object.keys(row).length === 1) { skipped++; continue; } - rows.push(row); - } - - if (rows.length > 0) { - const { error } = await db.from("edifice_notes").upsert(rows); - if (error) errors.push(error.message); - else updated = rows.length; } if (building_id && building_description) {