From 684b22a154f04a4e3f015eacff739f65ca65a5d7 Mon Sep 17 00:00:00 2001 From: Alice Moore <86723305+3mdistal@users.noreply.github.com> Date: Fri, 21 Aug 2026 00:41:02 +0000 Subject: [PATCH 01/28] fix: preserve Content database property mutations --- .changeset/calm-tools-preserve-inputs.md | 5 ++ packages/core/src/eval/agent-runner.ts | 3 + packages/core/src/eval/runner.spec.ts | 1 + packages/core/src/eval/types.ts | 5 ++ .../content/actions/add-database-item.ts | 4 +- .../content/actions/update-database-item.ts | 2 +- .../actions/upsert-database-item-by-key.ts | 4 +- templates/content/package.json | 1 + templates/content/parity/README.md | 13 +++- .../database-row-property-input.test.ts | 29 ++++++++ .../__tests__/eval-scenario-coverage.test.ts | 68 ++++++++++++++++++- templates/content/parity/eval-scenarios.ts | 22 ++++++ ...n-database-create-property-preservation.ts | 45 ++++++++++++ templates/content/parity/scenario-to-eval.ts | 56 +++++++++++++++ 14 files changed, 250 insertions(+), 8 deletions(-) create mode 100644 .changeset/calm-tools-preserve-inputs.md create mode 100644 templates/content/parity/__tests__/database-row-property-input.test.ts create mode 100644 templates/content/parity/run-database-create-property-preservation.ts diff --git a/.changeset/calm-tools-preserve-inputs.md b/.changeset/calm-tools-preserve-inputs.md new file mode 100644 index 00000000000..268e397a4ac --- /dev/null +++ b/.changeset/calm-tools-preserve-inputs.md @@ -0,0 +1,5 @@ +--- +"@agent-native/core": patch +--- + +Expose model-produced tool inputs to eval scorers so argument-level agent behavior can be verified. diff --git a/packages/core/src/eval/agent-runner.ts b/packages/core/src/eval/agent-runner.ts index b8599bd85b0..3b370ae7f27 100644 --- a/packages/core/src/eval/agent-runner.ts +++ b/packages/core/src/eval/agent-runner.ts @@ -120,6 +120,7 @@ export async function createAgentRunner( let text = ""; const toolCalls: string[] = []; + const toolCallDetails: Array<{ name: string; input: unknown }> = []; let ok = true; let error: string | undefined; @@ -134,6 +135,7 @@ export async function createAgentRunner( break; case "tool_start": toolCalls.push(event.tool); + toolCallDetails.push({ name: event.tool, input: event.input }); break; case "error": ok = false; @@ -165,6 +167,7 @@ export async function createAgentRunner( return { text, toolCalls, + toolCallDetails, ok, error, runId, diff --git a/packages/core/src/eval/runner.spec.ts b/packages/core/src/eval/runner.spec.ts index fef74b3690b..3fa997625cb 100644 --- a/packages/core/src/eval/runner.spec.ts +++ b/packages/core/src/eval/runner.spec.ts @@ -299,6 +299,7 @@ describe("createAgentRunner over a mocked runAgentLoop (no real model)", () => { const out = await runner.runAgent({ prompt: "hi" }); expect(out.text).toBe("Hello world"); expect(out.toolCalls).toEqual(["search"]); + expect(out.toolCallDetails).toEqual([{ name: "search", input: {} }]); expect(out.ok).toBe(true); // End-to-end: a contains scorer over the real collected text. diff --git a/packages/core/src/eval/types.ts b/packages/core/src/eval/types.ts index 3648416981a..a99a0849a9f 100644 --- a/packages/core/src/eval/types.ts +++ b/packages/core/src/eval/types.ts @@ -31,6 +31,11 @@ export interface AgentRunOutput { readonly text: string; /** Names of tools/actions the agent invoked, in call order. */ readonly toolCalls: readonly string[]; + /** Tool names and model-produced inputs, in call order. */ + readonly toolCallDetails?: readonly { + readonly name: string; + readonly input: unknown; + }[]; /** Whether the run completed without a terminal error event. */ readonly ok: boolean; /** Terminal error message, if the run errored. */ diff --git a/templates/content/actions/add-database-item.ts b/templates/content/actions/add-database-item.ts index 318f9984a2a..35ca1087410 100644 --- a/templates/content/actions/add-database-item.ts +++ b/templates/content/actions/add-database-item.ts @@ -20,7 +20,9 @@ const schema = databaseMutationEnvelopeSchema.extend({ propertyValues: z .record(z.string(), z.unknown()) .optional() - .describe("Strict property values keyed by property definition ID"), + .describe( + "Strict property values keyed by property definition ID. Use exact property definition IDs as keys. Include every schema-valid writable property value the user explicitly requested; when the request contains at least one such value, never pass an empty object. Do not invent or clear unmentioned properties.", + ), }); export default defineAction({ diff --git a/templates/content/actions/update-database-item.ts b/templates/content/actions/update-database-item.ts index 889c33c15da..3f79784ffcb 100644 --- a/templates/content/actions/update-database-item.ts +++ b/templates/content/actions/update-database-item.ts @@ -20,7 +20,7 @@ const schema = databaseMutationEnvelopeSchema.extend({ .record(z.string(), z.unknown()) .optional() .describe( - "Sparse strict patch keyed by property definition ID; omitted fields are preserved and explicit null clears a value", + "Sparse strict patch keyed by property definition ID; omitted fields are preserved and explicit null clears a value. Use exact property definition IDs as keys. Include every schema-valid writable property value the user explicitly requested; when the request contains at least one such value, never pass an empty object. Do not invent or clear unmentioned properties.", ), }); diff --git a/templates/content/actions/upsert-database-item-by-key.ts b/templates/content/actions/upsert-database-item-by-key.ts index c9a86fb303d..334d57f0555 100644 --- a/templates/content/actions/upsert-database-item-by-key.ts +++ b/templates/content/actions/upsert-database-item-by-key.ts @@ -21,7 +21,9 @@ const schema = databaseMutationEnvelopeSchema.extend({ propertyValues: z .record(z.string(), z.unknown()) .optional() - .describe("Sparse strict values keyed by property definition ID"), + .describe( + "Sparse strict values keyed by property definition ID. Use exact property definition IDs as keys. Include every schema-valid writable property value the user explicitly requested; when the request contains at least one such value, never pass an empty object. Do not invent or clear unmentioned properties.", + ), }); export default defineAction({ diff --git a/templates/content/package.json b/templates/content/package.json index 5c5e767d472..6cead972bce 100644 --- a/templates/content/package.json +++ b/templates/content/package.json @@ -12,6 +12,7 @@ "test:parity": "vitest --run parity", "test:parity-capabilities": "vitest --run parity actions/content-database-lifecycle.db.test.ts actions/bind-content-database-source-field.db.test.ts actions/_local-file-documents.test.ts actions/builder-source-review-gates.db.test.ts", "eval:parity": "agent-native eval parity", + "eval:property-preservation": "tsx parity/run-database-create-property-preservation.ts", "format.fix": "oxfmt --write .", "typecheck": "agent-native typecheck", "migrate:production": "tsx scripts/migrate-production.ts", diff --git a/templates/content/parity/README.md b/templates/content/parity/README.md index 05ac6c5b355..4c84876e573 100644 --- a/templates/content/parity/README.md +++ b/templates/content/parity/README.md @@ -10,7 +10,7 @@ PR 2.2 adds two executable tiers: existing action tests, make no model calls, and require no private provider credentials. - Gated agent evals run through `agent-native eval parity`. They are opt-in via - `CONTENT_PARITY_EVALS=1`, capped to four initial scenarios, and should be + `CONTENT_PARITY_EVALS=1`, kept to a small explicit scenario set, and should be reserved for manual or nightly checks. ## Deterministic Checks @@ -33,14 +33,23 @@ cd templates/content CONTENT_PARITY_EVALS=1 ANTHROPIC_API_KEY=... ./node_modules/.bin/agent-native eval parity ``` +The database-create property-preservation regression has a dedicated +fixture-only runner so it can inspect model-produced arguments without loading +or executing unrelated Content actions: + +```bash +CONTENT_PARITY_EVALS=1 pnpm eval:property-preservation +``` + With `CONTENT_PARITY_EVALS` unset, parity evals return skipped rows and do not call the agent runner. The CLI still exits `0`, but both readable and JSON reports mark each row with `status: "skipped"` and a `skipReason` such as `Skipped because CONTENT_PARITY_EVALS is unset`. -With the gate set, the eval files run the four PR 2.2 scenarios: +With the gate set, the eval files include these scenarios: - `database-source-scope` +- `database-create-property-preservation` - `document-search-edit` - `local-file-source-truth` - `builder-source-review-readonly` diff --git a/templates/content/parity/__tests__/database-row-property-input.test.ts b/templates/content/parity/__tests__/database-row-property-input.test.ts new file mode 100644 index 00000000000..e3ed8af8a20 --- /dev/null +++ b/templates/content/parity/__tests__/database-row-property-input.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; + +import addDatabaseItem from "../../actions/add-database-item"; +import updateDatabaseItem from "../../actions/update-database-item"; +import upsertDatabaseItemByKey from "../../actions/upsert-database-item-by-key"; + +const rowMutationActions = [ + ["add-database-item", addDatabaseItem], + ["update-database-item", updateDatabaseItem], + ["upsert-database-item-by-key", upsertDatabaseItemByKey], +] as const; + +describe("database row property inputs", () => { + it.each(rowMutationActions)( + "%s tells the agent to preserve explicitly requested writable values", + (_name, action) => { + const propertyValues = action.tool.parameters.properties.propertyValues; + expect(propertyValues.description).toContain( + "Include every schema-valid writable property value the user explicitly requested", + ); + expect(propertyValues.description).toContain( + "never pass an empty object", + ); + expect(propertyValues.description).toContain( + "Do not invent or clear unmentioned properties", + ); + }, + ); +}); diff --git a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts index 65fb938762f..b3d39b79350 100644 --- a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts +++ b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts @@ -37,10 +37,11 @@ describe("Content parity eval scenarios", () => { expect(invalid).toEqual([]); }); - it("keeps PR 2.5 capped to the five bundled gated scenarios", () => { + it("keeps the bundled gated scenarios explicit", () => { expect(parityEvalScenarios.map((scenario) => scenario.id).sort()).toEqual([ "builder-source-review-readonly", "database-bulk-row-reliability", + "database-create-property-preservation", "database-source-scope", "document-search-edit", "local-file-source-truth", @@ -83,14 +84,16 @@ describe("Content parity eval scenarios", () => { ); expect(report.failed).toBe(0); - expect(report.skipped).toBe(5); + expect(report.skipped).toBe(6); expect(report.results.every((row) => row.status === "skipped")).toBe(true); }); it("runs scorer-backed evals when the gate is set", async () => { process.env.CONTENT_PARITY_EVALS = "1"; - const scenario = parityEvalScenarios[0]; + const scenario = parityEvalScenarios.find( + (candidate) => candidate.id === "database-source-scope", + )!; const evalCase = scenarioToEval(scenario); const row = await scoreEval(evalCase, { runAgent: vi.fn(async () => ({ @@ -141,4 +144,63 @@ describe("Content parity eval scenarios", () => { expect(expectedToolsScore?.reason).toContain("remove-database-items"); expect(row.status).toBe("failed"); }); + + it("fails when database creation drops explicitly requested properties", async () => { + process.env.CONTENT_PARITY_EVALS = "1"; + const scenario = parityEvalScenarios.find( + (candidate) => candidate.id === "database-create-property-preservation", + )!; + const evalCase = scenarioToEval(scenario); + const row = await scoreEval(evalCase, { + runAgent: vi.fn(async () => ({ + text: scenario.successSignals.join("\n"), + toolCalls: ["add-database-item"], + toolCallDetails: [ + { name: "add-database-item", input: { propertyValues: {} } }, + ], + ok: true, + runId: "content-parity:empty-property-values", + durationMs: 1, + })), + engine: {} as never, + model: "test-model", + analyzeContext: vi.fn(), + }); + + expect( + row.scores.find((score) => score.scorer === "expected_property_values"), + ).toMatchObject({ passed: false, score: 0 }); + expect(row.status).toBe("failed"); + }); + + it("accepts exact database creation properties without extras", async () => { + process.env.CONTENT_PARITY_EVALS = "1"; + const scenario = parityEvalScenarios.find( + (candidate) => candidate.id === "database-create-property-preservation", + )!; + const evalCase = scenarioToEval(scenario); + const row = await scoreEval(evalCase, { + runAgent: vi.fn(async () => ({ + text: scenario.successSignals.join("\n"), + toolCalls: ["add-database-item"], + toolCallDetails: [ + { + name: "add-database-item", + input: { propertyValues: scenario.expectedPropertyValues }, + }, + ], + ok: true, + runId: "content-parity:exact-property-values", + durationMs: 1, + })), + engine: {} as never, + model: "test-model", + analyzeContext: vi.fn(), + }); + + expect( + row.scores.find((score) => score.scorer === "expected_property_values"), + ).toMatchObject({ passed: true, score: 1 }); + expect(row.status).toBe("passed"); + }); }); diff --git a/templates/content/parity/eval-scenarios.ts b/templates/content/parity/eval-scenarios.ts index b8bcfb76ddd..996de81c622 100644 --- a/templates/content/parity/eval-scenarios.ts +++ b/templates/content/parity/eval-scenarios.ts @@ -8,9 +8,31 @@ export interface ParityEvalScenario { prompt: string; successSignals: string[]; expectedTools?: string[]; + expectedPropertyValues?: Record; } export const parityEvalScenarios: ParityEvalScenario[] = [ + { + id: "database-create-property-preservation", + title: "Database create property preservation", + capabilityIds: ["database.rows"], + gateEnv: "CONTENT_PARITY_EVALS", + defaultState: "skipped", + requiresPrivateCredentials: false, + prompt: + "Create exactly one row in the already-verified fixture Content database. Do not rediscover or alter the target. Use target spaceId fixture_personal_space, databaseId fixture_feedback_database, and documentId fixture_feedback_document; expectedSchemaRevision fixture_schema_revision; title [FIXTURE] preserve explicit properties; idempotencyKey fixture-create-property-preservation-v1. Set property fixture_status_property to status-cannot-verify and property fixture_evidence_property to Baseline fixture preserve-me. No Blocks value was requested. Call add-database-item once with every exact target constraint and both property values, then report its result truthfully.", + successSignals: [ + "Uses add-database-item once for the exact fixture target.", + "Preserves both explicitly requested writable property values.", + "Does not invent a Blocks value or another property.", + "Reports an action failure rather than claiming a row was created if the fixture is unavailable.", + ], + expectedTools: ["add-database-item"], + expectedPropertyValues: { + fixture_status_property: "status-cannot-verify", + fixture_evidence_property: "Baseline fixture preserve-me", + }, + }, { id: "database-bulk-row-reliability", title: "Bulk database row reliability", diff --git a/templates/content/parity/run-database-create-property-preservation.ts b/templates/content/parity/run-database-create-property-preservation.ts new file mode 100644 index 00000000000..1713a9fd52c --- /dev/null +++ b/templates/content/parity/run-database-create-property-preservation.ts @@ -0,0 +1,45 @@ +import { createAgentRunner, runEvals } from "@agent-native/core/eval"; + +import addDatabaseItem from "../actions/add-database-item.ts"; +import { parityEvalScenarios } from "./eval-scenarios.ts"; +import { scenarioToEval } from "./scenario-to-eval.ts"; + +const scenario = parityEvalScenarios.find( + (candidate) => candidate.id === "database-create-property-preservation", +); +if (!scenario) { + throw new Error("Missing database create property preservation scenario."); +} + +const evalCase = scenarioToEval(scenario); +evalCase.scorers = evalCase.scorers.filter( + (scorer) => + scorer.name === "expected_tools" || + scorer.name === "expected_property_values", +); + +const runner = await createAgentRunner({ + actions: { + "add-database-item": { + ...addDatabaseItem, + run: async (input) => ({ fixtureOnly: true, received: input }), + }, + }, + systemPrompt: + "You are Content's AI document assistant. Use the registered Content action and preserve exact user-supplied target constraints, property IDs, and property values. Never invent fields or claim an action succeeded when it failed.", +}); + +const report = await runEvals([evalCase], runner, { persist: false }); +console.log( + JSON.stringify( + { + engine: runner.engine.name, + model: runner.model, + report, + }, + null, + 2, + ), +); + +process.exitCode = report.failed === 0 ? 0 : 1; diff --git a/templates/content/parity/scenario-to-eval.ts b/templates/content/parity/scenario-to-eval.ts index 6385b1d8c21..103d45cab10 100644 --- a/templates/content/parity/scenario-to-eval.ts +++ b/templates/content/parity/scenario-to-eval.ts @@ -30,6 +30,59 @@ function expectedToolScorer(expectedTools: string[]) { }); } +function normalizePropertyValues(input: unknown): Record { + if (!input || typeof input !== "object" || Array.isArray(input)) return {}; + const propertyValues = (input as Record).propertyValues; + if (!propertyValues) return {}; + if (!Array.isArray(propertyValues)) { + return typeof propertyValues === "object" + ? (propertyValues as Record) + : {}; + } + return Object.fromEntries( + propertyValues.flatMap((entry) => { + if (!entry || typeof entry !== "object") return []; + const { propertyId, value } = entry as Record; + return typeof propertyId === "string" ? [[propertyId, value]] : []; + }), + ); +} + +function expectedPropertyValuesScorer(expected: Record) { + return createScorer< + AgentRunOutput, + { + received: Record; + missing: string[]; + unexpected: string[]; + } + >({ + name: "expected_property_values", + analyze(run) { + const detail = run.toolCallDetails?.find( + (call) => call.name === "add-database-item", + ); + const received = normalizePropertyValues(detail?.input); + const missing = Object.entries(expected) + .filter(([propertyId, value]) => received[propertyId] !== value) + .map(([propertyId]) => propertyId); + const unexpected = Object.keys(received).filter( + (propertyId) => !(propertyId in expected), + ); + return { received, missing, unexpected }; + }, + generateScore({ missing, unexpected }) { + return missing.length === 0 && unexpected.length === 0 ? 1 : 0; + }, + generateReason({ analysis: { received, missing, unexpected } }) { + if (missing.length === 0 && unexpected.length === 0) { + return "Agent preserved every expected property ID and exact value without inventing another property."; + } + return `Received propertyValues ${JSON.stringify(received)}; missing or changed: ${missing.join(", ") || "none"}; unexpected: ${unexpected.join(", ") || "none"}`; + }, + }); +} + export function scenarioToEval(scenario: ParityEvalScenario): Eval { const name = `content-parity:${scenario.id}`; @@ -52,6 +105,9 @@ export function scenarioToEval(scenario: ParityEvalScenario): Eval { ...(scenario.expectedTools?.length ? [expectedToolScorer(scenario.expectedTools)] : []), + ...(scenario.expectedPropertyValues + ? [expectedPropertyValuesScorer(scenario.expectedPropertyValues)] + : []), ], }); } From 74eb67f387a624a2555159a14c0c2f4cf086d6e5 Mon Sep 17 00:00:00 2001 From: Alice Alexandra Moore <86723305+3mdistal@users.noreply.github.com> Date: Thu, 20 Aug 2026 21:17:39 -0400 Subject: [PATCH 02/28] fix: normalize Content AI property entries --- .../actions/_database-property-input.ts | 53 +++++++++++++++++++ .../content/actions/add-database-item.ts | 19 ++++--- .../content/actions/update-database-item.ts | 22 +++++--- .../actions/upsert-database-item-by-key.ts | 22 +++++--- .../database-row-property-input.test.ts | 53 ++++++++++++++++--- templates/content/parity/eval-scenarios.ts | 2 +- templates/content/parity/scenario-to-eval.ts | 4 +- 7 files changed, 146 insertions(+), 29 deletions(-) create mode 100644 templates/content/actions/_database-property-input.ts diff --git a/templates/content/actions/_database-property-input.ts b/templates/content/actions/_database-property-input.ts new file mode 100644 index 00000000000..dd98815eb26 --- /dev/null +++ b/templates/content/actions/_database-property-input.ts @@ -0,0 +1,53 @@ +import { ActionContractError } from "@agent-native/core"; +import { z } from "zod"; + +export const databasePropertyValuesSchema = z + .record(z.string(), z.unknown()) + .optional() + .describe( + "Programmatic property values keyed by exact property definition ID.", + ); + +export const databasePropertyEntriesSchema = z + .array( + z.object({ + propertyId: z + .string() + .min(1) + .describe("Exact immutable property definition ID"), + value: z.unknown().describe("Schema-valid value for this property"), + }), + ) + .max(1_000) + .optional() + .describe( + "Property values as explicit entries. Include one entry for every schema-valid writable property value the user requested, using the exact immutable property definition ID. When at least one value was requested, never pass an empty array. Do not invent or clear unmentioned properties.", + ); + +export function normalizeDatabasePropertyInput(input: { + propertyEntries?: Array<{ propertyId: string; value: unknown }>; + propertyValues?: Record; +}): Record | undefined { + if (input.propertyEntries && input.propertyValues) { + throw new ActionContractError( + "Provide propertyEntries or propertyValues, not both.", + { errorCode: "AMBIGUOUS_PROPERTY_INPUT" }, + ); + } + if (!input.propertyEntries) return input.propertyValues; + + const values: Record = {}; + for (const entry of input.propertyEntries) { + if (Object.prototype.hasOwnProperty.call(values, entry.propertyId)) { + throw new ActionContractError( + `Property entry ${entry.propertyId} was provided more than once.`, + { + errorCode: "DUPLICATE_PROPERTY_INPUT", + details: { propertyId: entry.propertyId }, + }, + ); + } + values[entry.propertyId] = entry.value; + } + return values; +} diff --git a/templates/content/actions/add-database-item.ts b/templates/content/actions/add-database-item.ts index 35ca1087410..629872af8b3 100644 --- a/templates/content/actions/add-database-item.ts +++ b/templates/content/actions/add-database-item.ts @@ -3,6 +3,11 @@ import { buildDeepLink } from "@agent-native/core/server"; import { z } from "zod"; import type { ContentDatabaseRowMutationResult } from "../shared/api.js"; +import { + databasePropertyEntriesSchema, + databasePropertyValuesSchema, + normalizeDatabasePropertyInput, +} from "./_database-property-input.js"; import { createDatabaseRow, databaseMutationEnvelopeSchema, @@ -17,17 +22,14 @@ const schema = databaseMutationEnvelopeSchema.extend({ .max(500) .optional() .describe("New row page title"), - propertyValues: z - .record(z.string(), z.unknown()) - .optional() - .describe( - "Strict property values keyed by property definition ID. Use exact property definition IDs as keys. Include every schema-valid writable property value the user explicitly requested; when the request contains at least one such value, never pass an empty object. Do not invent or clear unmentioned properties.", - ), + propertyValues: databasePropertyValuesSchema, + propertyEntries: databasePropertyEntriesSchema, }); export default defineAction({ description: "Create one row in an exact ordinary Content database using its discovered schema revision. Strictly validates every non-Blocks property, applies the side effect once per idempotency key, and returns a verified receipt with stable row identity.", + agentInputSchema: schema.omit({ propertyValues: true }), publicAgent: { expose: true, readOnly: false, @@ -54,7 +56,10 @@ export default defineAction({ }, }, run: async (args): Promise => { - const result = await createDatabaseRow(args); + const result = await createDatabaseRow({ + ...args, + propertyValues: normalizeDatabasePropertyInput(args), + }); const response = await getContentDatabaseResponse( result.receipt.target.databaseId, { diff --git a/templates/content/actions/update-database-item.ts b/templates/content/actions/update-database-item.ts index 3f79784ffcb..66bac08d6b4 100644 --- a/templates/content/actions/update-database-item.ts +++ b/templates/content/actions/update-database-item.ts @@ -3,6 +3,11 @@ import { buildDeepLink } from "@agent-native/core/server"; import { z } from "zod"; import type { ContentDatabaseRowMutationResult } from "../shared/api.js"; +import { + databasePropertyEntriesSchema, + databasePropertyValuesSchema, + normalizeDatabasePropertyInput, +} from "./_database-property-input.js"; import { databaseMutationEnvelopeSchema, updateDatabaseRow, @@ -16,17 +21,16 @@ const schema = databaseMutationEnvelopeSchema.extend({ .min(1) .describe("Row revision returned by get-content-database"), title: z.string().trim().min(1).max(500).optional(), - propertyValues: z - .record(z.string(), z.unknown()) - .optional() - .describe( - "Sparse strict patch keyed by property definition ID; omitted fields are preserved and explicit null clears a value. Use exact property definition IDs as keys. Include every schema-valid writable property value the user explicitly requested; when the request contains at least one such value, never pass an empty object. Do not invent or clear unmentioned properties.", - ), + propertyValues: databasePropertyValuesSchema, + propertyEntries: databasePropertyEntriesSchema.describe( + "Sparse property patch as explicit entries; omitted fields are preserved and explicit null clears a value. Include one entry for every schema-valid writable property value the user requested, using the exact immutable property definition ID. When at least one value was requested, never pass an empty array. Do not invent or clear unmentioned properties.", + ), }); export default defineAction({ description: "Sparsely update one exact Content database row by stable item and document IDs. Requires schema and row revisions, validates every non-Blocks property, and returns a verified idempotent receipt.", + agentInputSchema: schema.omit({ propertyValues: true }), schema, http: { method: "PUT" }, audit: { @@ -44,7 +48,11 @@ export default defineAction({ : "Updated Content database row"; }, }, - run: updateDatabaseRow, + run: (args) => + updateDatabaseRow({ + ...args, + propertyValues: normalizeDatabasePropertyInput(args), + }), link: ({ result }) => { const documentId = (result as ContentDatabaseRowMutationResult | null) ?.receipt.row.documentId; diff --git a/templates/content/actions/upsert-database-item-by-key.ts b/templates/content/actions/upsert-database-item-by-key.ts index 334d57f0555..8dbcb7f6033 100644 --- a/templates/content/actions/upsert-database-item-by-key.ts +++ b/templates/content/actions/upsert-database-item-by-key.ts @@ -3,6 +3,11 @@ import { buildDeepLink } from "@agent-native/core/server"; import { z } from "zod"; import type { ContentDatabaseRowMutationResult } from "../shared/api.js"; +import { + databasePropertyEntriesSchema, + databasePropertyValuesSchema, + normalizeDatabasePropertyInput, +} from "./_database-property-input.js"; import { databaseMutationEnvelopeSchema, upsertDatabaseRow, @@ -18,17 +23,16 @@ const schema = databaseMutationEnvelopeSchema.extend({ "Use null to assert the key is absent and create; use the discovered row revision to update an existing key", ), title: z.string().trim().min(1).max(500).optional(), - propertyValues: z - .record(z.string(), z.unknown()) - .optional() - .describe( - "Sparse strict values keyed by property definition ID. Use exact property definition IDs as keys. Include every schema-valid writable property value the user explicitly requested; when the request contains at least one such value, never pass an empty object. Do not invent or clear unmentioned properties.", - ), + propertyValues: databasePropertyValuesSchema, + propertyEntries: databasePropertyEntriesSchema.describe( + "Sparse property values as explicit entries. Include one entry for every schema-valid writable property value the user requested, using the exact immutable property definition ID. When at least one value was requested, never pass an empty array. Do not invent or clear unmentioned properties.", + ), }); export default defineAction({ description: "Create or sparsely update one Content database row by that database's explicitly configured natural key. Requires schema and row compare-and-swap revisions and returns a verified idempotent receipt.", + agentInputSchema: schema.omit({ propertyValues: true }), schema, audit: { recordInputs: false, @@ -45,7 +49,11 @@ export default defineAction({ : "Upserted Content database row by natural key"; }, }, - run: upsertDatabaseRow, + run: (args) => + upsertDatabaseRow({ + ...args, + propertyValues: normalizeDatabasePropertyInput(args), + }), link: ({ result }) => { const documentId = (result as ContentDatabaseRowMutationResult | null) ?.receipt.row.documentId; diff --git a/templates/content/parity/__tests__/database-row-property-input.test.ts b/templates/content/parity/__tests__/database-row-property-input.test.ts index e3ed8af8a20..2c2338289e0 100644 --- a/templates/content/parity/__tests__/database-row-property-input.test.ts +++ b/templates/content/parity/__tests__/database-row-property-input.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; +import { normalizeDatabasePropertyInput } from "../../actions/_database-property-input"; import addDatabaseItem from "../../actions/add-database-item"; import updateDatabaseItem from "../../actions/update-database-item"; import upsertDatabaseItemByKey from "../../actions/upsert-database-item-by-key"; @@ -14,16 +15,56 @@ describe("database row property inputs", () => { it.each(rowMutationActions)( "%s tells the agent to preserve explicitly requested writable values", (_name, action) => { - const propertyValues = action.tool.parameters.properties.propertyValues; - expect(propertyValues.description).toContain( - "Include every schema-valid writable property value the user explicitly requested", + const properties = action.tool.parameters.properties; + expect(properties).not.toHaveProperty("propertyValues"); + const propertyEntries = properties.propertyEntries; + expect(propertyEntries.type).toBe("array"); + expect(propertyEntries.items.properties.propertyId.description).toContain( + "Exact immutable property definition ID", ); - expect(propertyValues.description).toContain( - "never pass an empty object", + expect(propertyEntries.description).toContain( + "Include one entry for every schema-valid writable property value the user requested", ); - expect(propertyValues.description).toContain( + expect(propertyEntries.description).toContain( + "never pass an empty array", + ); + expect(propertyEntries.description).toContain( "Do not invent or clear unmentioned properties", ); }, ); + + it("normalizes model-friendly entries into the strict action contract", () => { + expect( + normalizeDatabasePropertyInput({ + propertyEntries: [ + { propertyId: "status-id", value: "ready" }, + { propertyId: "evidence-id", value: "preserve me" }, + ], + }), + ).toEqual({ + "status-id": "ready", + "evidence-id": "preserve me", + }); + }); + + it("rejects duplicate property entries instead of silently overwriting", () => { + expect(() => + normalizeDatabasePropertyInput({ + propertyEntries: [ + { propertyId: "status-id", value: "ready" }, + { propertyId: "status-id", value: "changed" }, + ], + }), + ).toThrow(/provided more than once/); + }); + + it("rejects ambiguous entry and record inputs", () => { + expect(() => + normalizeDatabasePropertyInput({ + propertyEntries: [{ propertyId: "status-id", value: "ready" }], + propertyValues: { "status-id": "ready" }, + }), + ).toThrow(/not both/); + }); }); diff --git a/templates/content/parity/eval-scenarios.ts b/templates/content/parity/eval-scenarios.ts index 996de81c622..c459e1f376d 100644 --- a/templates/content/parity/eval-scenarios.ts +++ b/templates/content/parity/eval-scenarios.ts @@ -20,7 +20,7 @@ export const parityEvalScenarios: ParityEvalScenario[] = [ defaultState: "skipped", requiresPrivateCredentials: false, prompt: - "Create exactly one row in the already-verified fixture Content database. Do not rediscover or alter the target. Use target spaceId fixture_personal_space, databaseId fixture_feedback_database, and documentId fixture_feedback_document; expectedSchemaRevision fixture_schema_revision; title [FIXTURE] preserve explicit properties; idempotencyKey fixture-create-property-preservation-v1. Set property fixture_status_property to status-cannot-verify and property fixture_evidence_property to Baseline fixture preserve-me. No Blocks value was requested. Call add-database-item once with every exact target constraint and both property values, then report its result truthfully.", + "Create exactly one row in the already-verified fixture Content database. Do not rediscover or alter the target. Use target spaceId fixture_personal_space, databaseId fixture_feedback_database, and documentId fixture_feedback_document; expectedSchemaRevision fixture_schema_revision; title [FIXTURE] preserve explicit properties; idempotencyKey fixture-create-property-preservation-v1. Set property fixture_status_property to status-cannot-verify and property fixture_evidence_property to Baseline fixture preserve-me. No Blocks value was requested. Call add-database-item once with every exact target constraint and both property entries, then report its result truthfully.", successSignals: [ "Uses add-database-item once for the exact fixture target.", "Preserves both explicitly requested writable property values.", diff --git a/templates/content/parity/scenario-to-eval.ts b/templates/content/parity/scenario-to-eval.ts index 103d45cab10..c657048da4c 100644 --- a/templates/content/parity/scenario-to-eval.ts +++ b/templates/content/parity/scenario-to-eval.ts @@ -32,7 +32,9 @@ function expectedToolScorer(expectedTools: string[]) { function normalizePropertyValues(input: unknown): Record { if (!input || typeof input !== "object" || Array.isArray(input)) return {}; - const propertyValues = (input as Record).propertyValues; + const propertyValues = + (input as Record).propertyEntries ?? + (input as Record).propertyValues; if (!propertyValues) return {}; if (!Array.isArray(propertyValues)) { return typeof propertyValues === "object" From fd85e17054cb04954cd150e1485e44bc94b36f0b Mon Sep 17 00:00:00 2001 From: Alice Alexandra Moore <86723305+3mdistal@users.noreply.github.com> Date: Fri, 21 Aug 2026 07:43:48 -0400 Subject: [PATCH 03/28] fix: canonicalize Content mutation inputs --- .../actions/_database-property-input.ts | 20 +++ .../content/actions/add-database-item.ts | 9 +- .../content/actions/update-database-item.ts | 8 +- .../actions/upsert-database-item-by-key.ts | 8 +- .../database-row-property-input.test.ts | 44 ++++++- .../__tests__/eval-scenario-coverage.test.ts | 75 +++++++++++ templates/content/parity/scenario-to-eval.ts | 119 ++++++++++++++---- 7 files changed, 240 insertions(+), 43 deletions(-) diff --git a/templates/content/actions/_database-property-input.ts b/templates/content/actions/_database-property-input.ts index dd98815eb26..30552617c3e 100644 --- a/templates/content/actions/_database-property-input.ts +++ b/templates/content/actions/_database-property-input.ts @@ -51,3 +51,23 @@ export function normalizeDatabasePropertyInput(input: { } return values; } + +export function canonicalizeDatabasePropertyInput< + T extends { + propertyEntries?: Array<{ propertyId: string; value: unknown }>; + propertyValues?: Record; + }, +>( + input: T, +): Omit & { + propertyValues?: Record; +} { + const { propertyEntries, propertyValues, ...canonicalInput } = input; + return { + ...canonicalInput, + propertyValues: normalizeDatabasePropertyInput({ + propertyEntries, + propertyValues, + }), + }; +} diff --git a/templates/content/actions/add-database-item.ts b/templates/content/actions/add-database-item.ts index 629872af8b3..8666c33bdfd 100644 --- a/templates/content/actions/add-database-item.ts +++ b/templates/content/actions/add-database-item.ts @@ -4,9 +4,9 @@ import { z } from "zod"; import type { ContentDatabaseRowMutationResult } from "../shared/api.js"; import { + canonicalizeDatabasePropertyInput, databasePropertyEntriesSchema, databasePropertyValuesSchema, - normalizeDatabasePropertyInput, } from "./_database-property-input.js"; import { createDatabaseRow, @@ -56,10 +56,9 @@ export default defineAction({ }, }, run: async (args): Promise => { - const result = await createDatabaseRow({ - ...args, - propertyValues: normalizeDatabasePropertyInput(args), - }); + const result = await createDatabaseRow( + canonicalizeDatabasePropertyInput(args), + ); const response = await getContentDatabaseResponse( result.receipt.target.databaseId, { diff --git a/templates/content/actions/update-database-item.ts b/templates/content/actions/update-database-item.ts index 66bac08d6b4..4fcd904e532 100644 --- a/templates/content/actions/update-database-item.ts +++ b/templates/content/actions/update-database-item.ts @@ -4,9 +4,9 @@ import { z } from "zod"; import type { ContentDatabaseRowMutationResult } from "../shared/api.js"; import { + canonicalizeDatabasePropertyInput, databasePropertyEntriesSchema, databasePropertyValuesSchema, - normalizeDatabasePropertyInput, } from "./_database-property-input.js"; import { databaseMutationEnvelopeSchema, @@ -48,11 +48,7 @@ export default defineAction({ : "Updated Content database row"; }, }, - run: (args) => - updateDatabaseRow({ - ...args, - propertyValues: normalizeDatabasePropertyInput(args), - }), + run: (args) => updateDatabaseRow(canonicalizeDatabasePropertyInput(args)), link: ({ result }) => { const documentId = (result as ContentDatabaseRowMutationResult | null) ?.receipt.row.documentId; diff --git a/templates/content/actions/upsert-database-item-by-key.ts b/templates/content/actions/upsert-database-item-by-key.ts index 8dbcb7f6033..93dcb1ef43d 100644 --- a/templates/content/actions/upsert-database-item-by-key.ts +++ b/templates/content/actions/upsert-database-item-by-key.ts @@ -4,9 +4,9 @@ import { z } from "zod"; import type { ContentDatabaseRowMutationResult } from "../shared/api.js"; import { + canonicalizeDatabasePropertyInput, databasePropertyEntriesSchema, databasePropertyValuesSchema, - normalizeDatabasePropertyInput, } from "./_database-property-input.js"; import { databaseMutationEnvelopeSchema, @@ -49,11 +49,7 @@ export default defineAction({ : "Upserted Content database row by natural key"; }, }, - run: (args) => - upsertDatabaseRow({ - ...args, - propertyValues: normalizeDatabasePropertyInput(args), - }), + run: (args) => upsertDatabaseRow(canonicalizeDatabasePropertyInput(args)), link: ({ result }) => { const documentId = (result as ContentDatabaseRowMutationResult | null) ?.receipt.row.documentId; diff --git a/templates/content/parity/__tests__/database-row-property-input.test.ts b/templates/content/parity/__tests__/database-row-property-input.test.ts index 2c2338289e0..aaa40f1e3d9 100644 --- a/templates/content/parity/__tests__/database-row-property-input.test.ts +++ b/templates/content/parity/__tests__/database-row-property-input.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; -import { normalizeDatabasePropertyInput } from "../../actions/_database-property-input"; +import { + canonicalizeDatabasePropertyInput, + normalizeDatabasePropertyInput, +} from "../../actions/_database-property-input"; +import { digest } from "../../actions/_database-row-mutation"; import addDatabaseItem from "../../actions/add-database-item"; import updateDatabaseItem from "../../actions/update-database-item"; import upsertDatabaseItemByKey from "../../actions/upsert-database-item-by-key"; @@ -67,4 +71,42 @@ describe("database row property inputs", () => { }), ).toThrow(/not both/); }); + + it("removes the model-only representation before canonical hashing", () => { + const canonical = canonicalizeDatabasePropertyInput({ + idempotencyKey: "same-intent", + propertyEntries: [ + { propertyId: "status-id", value: "ready" }, + { propertyId: "evidence-id", value: "preserve me" }, + ], + }); + + expect(canonical).toEqual({ + idempotencyKey: "same-intent", + propertyValues: { + "status-id": "ready", + "evidence-id": "preserve me", + }, + }); + expect(canonical).not.toHaveProperty("propertyEntries"); + }); + + it("gives equivalent entry and record inputs the same canonical digest", () => { + const fromEntries = canonicalizeDatabasePropertyInput({ + idempotencyKey: "same-intent", + propertyEntries: [ + { propertyId: "status-id", value: "ready" }, + { propertyId: "evidence-id", value: "preserve me" }, + ], + }); + const fromRecord = canonicalizeDatabasePropertyInput({ + idempotencyKey: "same-intent", + propertyValues: { + "evidence-id": "preserve me", + "status-id": "ready", + }, + }); + + expect(digest(fromEntries)).toBe(digest(fromRecord)); + }); }); diff --git a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts index b3d39b79350..f7aba9894c3 100644 --- a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts +++ b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts @@ -203,4 +203,79 @@ describe("Content parity eval scenarios", () => { ).toMatchObject({ passed: true, score: 1 }); expect(row.status).toBe("passed"); }); + + it.each([ + { + name: "duplicate property entries", + toolCallDetails: [ + { + name: "add-database-item", + input: { + propertyEntries: [ + { propertyId: "parity-text-property-id", value: "preserve me" }, + { propertyId: "parity-text-property-id", value: "preserve me" }, + { propertyId: "parity-status-property-id", value: "ready" }, + ], + }, + }, + ], + }, + { + name: "ambiguous property formats", + toolCallDetails: [ + { + name: "add-database-item", + input: { + propertyEntries: [ + { propertyId: "parity-text-property-id", value: "preserve me" }, + { propertyId: "parity-status-property-id", value: "ready" }, + ], + propertyValues: { + "parity-text-property-id": "preserve me", + "parity-status-property-id": "ready", + }, + }, + }, + ], + }, + { + name: "an extra row mutation", + toolCallDetails: [ + { + name: "add-database-item", + input: { + propertyEntries: [ + { propertyId: "parity-text-property-id", value: "preserve me" }, + { propertyId: "parity-status-property-id", value: "ready" }, + ], + }, + }, + { name: "update-database-item", input: {} }, + ], + }, + ])("rejects $name", async ({ toolCallDetails }) => { + process.env.CONTENT_PARITY_EVALS = "1"; + const scenario = parityEvalScenarios.find( + (candidate) => candidate.id === "database-create-property-preservation", + )!; + const evalCase = scenarioToEval(scenario); + const row = await scoreEval(evalCase, { + runAgent: vi.fn(async () => ({ + text: scenario.successSignals.join("\n"), + toolCalls: toolCallDetails.map((call) => call.name), + toolCallDetails, + ok: true, + runId: "content-parity:invalid-property-input", + durationMs: 1, + })), + engine: {} as never, + model: "test-model", + analyzeContext: vi.fn(), + }); + + expect( + row.scores.find((score) => score.scorer === "expected_property_values"), + ).toMatchObject({ passed: false, score: 0 }); + expect(row.status).toBe("failed"); + }); }); diff --git a/templates/content/parity/scenario-to-eval.ts b/templates/content/parity/scenario-to-eval.ts index c657048da4c..d6766456a38 100644 --- a/templates/content/parity/scenario-to-eval.ts +++ b/templates/content/parity/scenario-to-eval.ts @@ -30,26 +30,68 @@ function expectedToolScorer(expectedTools: string[]) { }); } -function normalizePropertyValues(input: unknown): Record { - if (!input || typeof input !== "object" || Array.isArray(input)) return {}; - const propertyValues = - (input as Record).propertyEntries ?? - (input as Record).propertyValues; - if (!propertyValues) return {}; - if (!Array.isArray(propertyValues)) { - return typeof propertyValues === "object" - ? (propertyValues as Record) - : {}; +function analyzePropertyValues(input: unknown): { + received: Record; + invalid: string[]; +} { + if (!input || typeof input !== "object" || Array.isArray(input)) { + return { received: {}, invalid: ["tool input is not an object"] }; } - return Object.fromEntries( - propertyValues.flatMap((entry) => { - if (!entry || typeof entry !== "object") return []; - const { propertyId, value } = entry as Record; - return typeof propertyId === "string" ? [[propertyId, value]] : []; - }), - ); + const record = input as Record; + const hasEntries = record.propertyEntries !== undefined; + const hasValues = record.propertyValues !== undefined; + if (hasEntries && hasValues) { + return { + received: {}, + invalid: ["propertyEntries and propertyValues were both provided"], + }; + } + if (hasValues) { + if ( + !record.propertyValues || + typeof record.propertyValues !== "object" || + Array.isArray(record.propertyValues) + ) { + return { received: {}, invalid: ["propertyValues is not a record"] }; + } + return { + received: record.propertyValues as Record, + invalid: [], + }; + } + if (!hasEntries || !Array.isArray(record.propertyEntries)) { + return { received: {}, invalid: ["propertyEntries is not an array"] }; + } + + const received: Record = {}; + const invalid: string[] = []; + for (const entry of record.propertyEntries) { + if (!entry || typeof entry !== "object" || Array.isArray(entry)) { + invalid.push("propertyEntries contains a non-object entry"); + continue; + } + const { propertyId, value } = entry as Record; + if (typeof propertyId !== "string" || propertyId.length === 0) { + invalid.push("propertyEntries contains an invalid propertyId"); + continue; + } + if (Object.prototype.hasOwnProperty.call(received, propertyId)) { + invalid.push(`propertyEntries contains duplicate ID ${propertyId}`); + continue; + } + received[propertyId] = value; + } + return { received, invalid }; } +const databaseRowMutationTools = new Set([ + "add-database-item", + "update-database-item", + "upsert-database-item-by-key", + "duplicate-database-items", + "remove-database-items", +]); + function expectedPropertyValuesScorer(expected: Record) { return createScorer< AgentRunOutput, @@ -57,30 +99,57 @@ function expectedPropertyValuesScorer(expected: Record) { received: Record; missing: string[]; unexpected: string[]; + invalid: string[]; + mutationCalls: string[]; } >({ name: "expected_property_values", analyze(run) { - const detail = run.toolCallDetails?.find( + const mutationCalls = (run.toolCallDetails ?? []) + .filter((call) => databaseRowMutationTools.has(call.name)) + .map((call) => call.name); + const createCalls = (run.toolCallDetails ?? []).filter( (call) => call.name === "add-database-item", ); - const received = normalizePropertyValues(detail?.input); + const analysis = analyzePropertyValues(createCalls[0]?.input); + const invalid = [...analysis.invalid]; + if (createCalls.length !== 1) { + invalid.push( + `expected exactly one add-database-item call, received ${createCalls.length}`, + ); + } + if (mutationCalls.length !== 1) { + invalid.push( + `expected exactly one row mutation, received ${mutationCalls.length}`, + ); + } + const received = analysis.received; const missing = Object.entries(expected) .filter(([propertyId, value]) => received[propertyId] !== value) .map(([propertyId]) => propertyId); const unexpected = Object.keys(received).filter( (propertyId) => !(propertyId in expected), ); - return { received, missing, unexpected }; + return { received, missing, unexpected, invalid, mutationCalls }; }, - generateScore({ missing, unexpected }) { - return missing.length === 0 && unexpected.length === 0 ? 1 : 0; + generateScore({ missing, unexpected, invalid }) { + return missing.length === 0 && + unexpected.length === 0 && + invalid.length === 0 + ? 1 + : 0; }, - generateReason({ analysis: { received, missing, unexpected } }) { - if (missing.length === 0 && unexpected.length === 0) { + generateReason({ + analysis: { received, missing, unexpected, invalid, mutationCalls }, + }) { + if ( + missing.length === 0 && + unexpected.length === 0 && + invalid.length === 0 + ) { return "Agent preserved every expected property ID and exact value without inventing another property."; } - return `Received propertyValues ${JSON.stringify(received)}; missing or changed: ${missing.join(", ") || "none"}; unexpected: ${unexpected.join(", ") || "none"}`; + return `Received propertyValues ${JSON.stringify(received)}; mutations: ${mutationCalls.join(", ") || "none"}; missing or changed: ${missing.join(", ") || "none"}; unexpected: ${unexpected.join(", ") || "none"}; invalid: ${invalid.join("; ") || "none"}`; }, }); } From 75344400b32aa39343e775df620697882050fc07 Mon Sep 17 00:00:00 2001 From: Alice Alexandra Moore <86723305+3mdistal@users.noreply.github.com> Date: Fri, 21 Aug 2026 07:52:10 -0400 Subject: [PATCH 04/28] test: require successful Content mutation evals --- packages/core/src/eval/agent-runner.ts | 30 +++- packages/core/src/eval/runner.spec.ts | 23 ++- packages/core/src/eval/types.ts | 5 +- .../__tests__/eval-scenario-coverage.test.ts | 153 +++++++++++++----- templates/content/parity/eval-scenarios.ts | 27 +++- templates/content/parity/scenario-to-eval.ts | 63 +++++++- 6 files changed, 250 insertions(+), 51 deletions(-) diff --git a/packages/core/src/eval/agent-runner.ts b/packages/core/src/eval/agent-runner.ts index 3b370ae7f27..0c316ec3e7e 100644 --- a/packages/core/src/eval/agent-runner.ts +++ b/packages/core/src/eval/agent-runner.ts @@ -120,7 +120,14 @@ export async function createAgentRunner( let text = ""; const toolCalls: string[] = []; - const toolCallDetails: Array<{ name: string; input: unknown }> = []; + const toolCallDetails: Array<{ + name: string; + id?: string; + input: unknown; + completed?: boolean; + isError?: boolean; + result?: string; + }> = []; let ok = true; let error: string | undefined; @@ -135,8 +142,25 @@ export async function createAgentRunner( break; case "tool_start": toolCalls.push(event.tool); - toolCallDetails.push({ name: event.tool, input: event.input }); + toolCallDetails.push({ + name: event.tool, + id: event.id, + input: event.input, + }); + break; + case "tool_done": { + const detail = event.id + ? toolCallDetails.find((call) => call.id === event.id) + : toolCallDetails.find( + (call) => call.name === event.tool && !call.completed, + ); + if (detail) { + detail.completed = true; + detail.isError = event.isError === true; + detail.result = event.result; + } break; + } case "error": ok = false; error = event.error; @@ -167,7 +191,7 @@ export async function createAgentRunner( return { text, toolCalls, - toolCallDetails, + toolCallDetails: toolCallDetails.map(({ id: _id, ...detail }) => detail), ok, error, runId, diff --git a/packages/core/src/eval/runner.spec.ts b/packages/core/src/eval/runner.spec.ts index 3fa997625cb..d0b5df129c5 100644 --- a/packages/core/src/eval/runner.spec.ts +++ b/packages/core/src/eval/runner.spec.ts @@ -276,7 +276,18 @@ describe("createAgentRunner over a mocked runAgentLoop (no real model)", () => { const runLoop = vi.fn( async (opts: { send: (e: AgentChatEvent) => void }) => { opts.send({ type: "text", text: "Hello " }); - opts.send({ type: "tool_start", tool: "search", input: {} }); + opts.send({ + type: "tool_start", + tool: "search", + id: "search-1", + input: {}, + }); + opts.send({ + type: "tool_done", + tool: "search", + id: "search-1", + result: '{"ok":true}', + }); opts.send({ type: "text", text: "world" }); return { inputTokens: 0, @@ -299,7 +310,15 @@ describe("createAgentRunner over a mocked runAgentLoop (no real model)", () => { const out = await runner.runAgent({ prompt: "hi" }); expect(out.text).toBe("Hello world"); expect(out.toolCalls).toEqual(["search"]); - expect(out.toolCallDetails).toEqual([{ name: "search", input: {} }]); + expect(out.toolCallDetails).toEqual([ + { + name: "search", + input: {}, + completed: true, + isError: false, + result: '{"ok":true}', + }, + ]); expect(out.ok).toBe(true); // End-to-end: a contains scorer over the real collected text. diff --git a/packages/core/src/eval/types.ts b/packages/core/src/eval/types.ts index a99a0849a9f..d9f17f8fef6 100644 --- a/packages/core/src/eval/types.ts +++ b/packages/core/src/eval/types.ts @@ -31,10 +31,13 @@ export interface AgentRunOutput { readonly text: string; /** Names of tools/actions the agent invoked, in call order. */ readonly toolCalls: readonly string[]; - /** Tool names and model-produced inputs, in call order. */ + /** Tool names, model-produced inputs, and execution outcomes in call order. */ readonly toolCallDetails?: readonly { readonly name: string; readonly input: unknown; + readonly completed?: boolean; + readonly isError?: boolean; + readonly result?: string; }[]; /** Whether the run completed without a terminal error event. */ readonly ok: boolean; diff --git a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts index f7aba9894c3..02bc2906626 100644 --- a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts +++ b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts @@ -7,6 +7,22 @@ import { scenarioToEval } from "../scenario-to-eval"; const OLD_GATE = process.env.CONTENT_PARITY_EVALS; +function successfulCreateCall( + scenario: (typeof parityEvalScenarios)[number], + propertyInput: Record, +) { + return { + name: "add-database-item", + input: { + ...scenario.expectedCreateEnvelope, + ...propertyInput, + }, + completed: true, + isError: false, + result: '{"fixtureOnly":true}', + }; +} + afterEach(() => { if (OLD_GATE === undefined) { delete process.env.CONTENT_PARITY_EVALS; @@ -156,7 +172,7 @@ describe("Content parity eval scenarios", () => { text: scenario.successSignals.join("\n"), toolCalls: ["add-database-item"], toolCallDetails: [ - { name: "add-database-item", input: { propertyValues: {} } }, + successfulCreateCall(scenario, { propertyValues: {} }), ], ok: true, runId: "content-parity:empty-property-values", @@ -184,10 +200,9 @@ describe("Content parity eval scenarios", () => { text: scenario.successSignals.join("\n"), toolCalls: ["add-database-item"], toolCallDetails: [ - { - name: "add-database-item", - input: { propertyValues: scenario.expectedPropertyValues }, - }, + successfulCreateCall(scenario, { + propertyValues: scenario.expectedPropertyValues, + }), ], ok: true, runId: "content-parity:exact-property-values", @@ -205,59 +220,57 @@ describe("Content parity eval scenarios", () => { }); it.each([ - { + (scenario: (typeof parityEvalScenarios)[number]) => ({ name: "duplicate property entries", toolCallDetails: [ - { - name: "add-database-item", - input: { - propertyEntries: [ - { propertyId: "parity-text-property-id", value: "preserve me" }, - { propertyId: "parity-text-property-id", value: "preserve me" }, - { propertyId: "parity-status-property-id", value: "ready" }, - ], - }, - }, + successfulCreateCall(scenario, { + propertyEntries: [ + { propertyId: "parity-text-property-id", value: "preserve me" }, + { propertyId: "parity-text-property-id", value: "preserve me" }, + { propertyId: "parity-status-property-id", value: "ready" }, + ], + }), ], - }, - { + }), + (scenario: (typeof parityEvalScenarios)[number]) => ({ name: "ambiguous property formats", toolCallDetails: [ - { - name: "add-database-item", - input: { - propertyEntries: [ - { propertyId: "parity-text-property-id", value: "preserve me" }, - { propertyId: "parity-status-property-id", value: "ready" }, - ], - propertyValues: { - "parity-text-property-id": "preserve me", - "parity-status-property-id": "ready", - }, + successfulCreateCall(scenario, { + propertyEntries: [ + { propertyId: "parity-text-property-id", value: "preserve me" }, + { propertyId: "parity-status-property-id", value: "ready" }, + ], + propertyValues: { + "parity-text-property-id": "preserve me", + "parity-status-property-id": "ready", }, - }, + }), ], - }, - { + }), + (scenario: (typeof parityEvalScenarios)[number]) => ({ name: "an extra row mutation", toolCallDetails: [ + successfulCreateCall(scenario, { + propertyEntries: [ + { propertyId: "parity-text-property-id", value: "preserve me" }, + { propertyId: "parity-status-property-id", value: "ready" }, + ], + }), { - name: "add-database-item", - input: { - propertyEntries: [ - { propertyId: "parity-text-property-id", value: "preserve me" }, - { propertyId: "parity-status-property-id", value: "ready" }, - ], - }, + name: "update-database-item", + input: {}, + completed: true, + isError: false, + result: "{}", }, - { name: "update-database-item", input: {} }, ], - }, - ])("rejects $name", async ({ toolCallDetails }) => { + }), + ])("rejects invalid property behavior", async (buildCase) => { process.env.CONTENT_PARITY_EVALS = "1"; const scenario = parityEvalScenarios.find( (candidate) => candidate.id === "database-create-property-preservation", )!; + const { toolCallDetails } = buildCase(scenario); const evalCase = scenarioToEval(scenario); const row = await scoreEval(evalCase, { runAgent: vi.fn(async () => ({ @@ -278,4 +291,60 @@ describe("Content parity eval scenarios", () => { ).toMatchObject({ passed: false, score: 0 }); expect(row.status).toBe("failed"); }); + + it.each([ + { + name: "wrong target", + mutate(call: ReturnType) { + return { + ...call, + input: { + ...(call.input as Record), + target: { + ...((call.input as Record).target as Record< + string, + unknown + >), + databaseId: "wrong-database", + }, + }, + }; + }, + }, + { + name: "failed execution", + mutate(call: ReturnType) { + return { ...call, isError: true, result: "fixture rejected" }; + }, + }, + ])("rejects $name", async ({ mutate }) => { + process.env.CONTENT_PARITY_EVALS = "1"; + const scenario = parityEvalScenarios.find( + (candidate) => candidate.id === "database-create-property-preservation", + )!; + const call = mutate( + successfulCreateCall(scenario, { + propertyValues: scenario.expectedPropertyValues, + }), + ); + const evalCase = scenarioToEval(scenario); + const row = await scoreEval(evalCase, { + runAgent: vi.fn(async () => ({ + text: scenario.successSignals.join("\n"), + toolCalls: ["add-database-item"], + toolCallDetails: [call], + ok: true, + runId: "content-parity:rejected-create", + durationMs: 1, + })), + engine: {} as never, + model: "test-model", + analyzeContext: vi.fn(), + }); + + expect( + row.scores.find((score) => score.scorer === "expected_property_values"), + ).toMatchObject({ passed: false, score: 0 }); + expect(row.status).toBe("failed"); + }); }); diff --git a/templates/content/parity/eval-scenarios.ts b/templates/content/parity/eval-scenarios.ts index c459e1f376d..3c26553e2bc 100644 --- a/templates/content/parity/eval-scenarios.ts +++ b/templates/content/parity/eval-scenarios.ts @@ -9,6 +9,17 @@ export interface ParityEvalScenario { successSignals: string[]; expectedTools?: string[]; expectedPropertyValues?: Record; + expectedCreateEnvelope?: { + target: { + authorityScope: { kind: "personal"; id: string }; + spaceId: string; + databaseId: string; + databaseDocumentId: string; + }; + expectedSchemaRevision: string; + idempotencyKey: string; + title: string; + }; } export const parityEvalScenarios: ParityEvalScenario[] = [ @@ -20,7 +31,7 @@ export const parityEvalScenarios: ParityEvalScenario[] = [ defaultState: "skipped", requiresPrivateCredentials: false, prompt: - "Create exactly one row in the already-verified fixture Content database. Do not rediscover or alter the target. Use target spaceId fixture_personal_space, databaseId fixture_feedback_database, and documentId fixture_feedback_document; expectedSchemaRevision fixture_schema_revision; title [FIXTURE] preserve explicit properties; idempotencyKey fixture-create-property-preservation-v1. Set property fixture_status_property to status-cannot-verify and property fixture_evidence_property to Baseline fixture preserve-me. No Blocks value was requested. Call add-database-item once with every exact target constraint and both property entries, then report its result truthfully.", + "Create exactly one row in the already-verified fixture Content database. Do not rediscover or alter the target. Use target authorityScope { kind: personal, id: fixture-owner@example.test }, spaceId fixture_personal_space, databaseId fixture_feedback_database, and databaseDocumentId fixture_feedback_document; expectedSchemaRevision fixture_schema_revision; title [FIXTURE] preserve explicit properties; idempotencyKey fixture-create-property-preservation-v1. Set property fixture_status_property to status-cannot-verify and property fixture_evidence_property to Baseline fixture preserve-me. No Blocks value was requested. Call add-database-item once with every exact target constraint and both property entries, then report its result truthfully.", successSignals: [ "Uses add-database-item once for the exact fixture target.", "Preserves both explicitly requested writable property values.", @@ -32,6 +43,20 @@ export const parityEvalScenarios: ParityEvalScenario[] = [ fixture_status_property: "status-cannot-verify", fixture_evidence_property: "Baseline fixture preserve-me", }, + expectedCreateEnvelope: { + target: { + authorityScope: { + kind: "personal", + id: "fixture-owner@example.test", + }, + spaceId: "fixture_personal_space", + databaseId: "fixture_feedback_database", + databaseDocumentId: "fixture_feedback_document", + }, + expectedSchemaRevision: "fixture_schema_revision", + idempotencyKey: "fixture-create-property-preservation-v1", + title: "[FIXTURE] preserve explicit properties", + }, }, { id: "database-bulk-row-reliability", diff --git a/templates/content/parity/scenario-to-eval.ts b/templates/content/parity/scenario-to-eval.ts index d6766456a38..514fa94ceaf 100644 --- a/templates/content/parity/scenario-to-eval.ts +++ b/templates/content/parity/scenario-to-eval.ts @@ -92,7 +92,40 @@ const databaseRowMutationTools = new Set([ "remove-database-items", ]); -function expectedPropertyValuesScorer(expected: Record) { +function matchesCreateEnvelope( + input: Record, + expected: NonNullable, +): boolean { + const target = input.target; + if (!target || typeof target !== "object" || Array.isArray(target)) { + return false; + } + const actualTarget = target as Record; + const authorityScope = actualTarget.authorityScope; + if ( + !authorityScope || + typeof authorityScope !== "object" || + Array.isArray(authorityScope) + ) { + return false; + } + const actualAuthority = authorityScope as Record; + return ( + actualAuthority.kind === expected.target.authorityScope.kind && + actualAuthority.id === expected.target.authorityScope.id && + actualTarget.spaceId === expected.target.spaceId && + actualTarget.databaseId === expected.target.databaseId && + actualTarget.databaseDocumentId === expected.target.databaseDocumentId && + input.expectedSchemaRevision === expected.expectedSchemaRevision && + input.idempotencyKey === expected.idempotencyKey && + input.title === expected.title + ); +} + +function expectedPropertyValuesScorer( + expected: Record, + expectedEnvelope?: ParityEvalScenario["expectedCreateEnvelope"], +) { return createScorer< AgentRunOutput, { @@ -123,6 +156,27 @@ function expectedPropertyValuesScorer(expected: Record) { `expected exactly one row mutation, received ${mutationCalls.length}`, ); } + const createInput = createCalls[0]?.input; + if ( + expectedEnvelope && + (!createInput || + typeof createInput !== "object" || + Array.isArray(createInput) || + !matchesCreateEnvelope( + createInput as Record, + expectedEnvelope, + )) + ) { + invalid.push( + "create target, schema revision, idempotency key, or title did not match the fixture", + ); + } + if (!createCalls[0]?.completed || createCalls[0]?.isError) { + invalid.push("add-database-item did not complete successfully"); + } + if (!run.ok) { + invalid.push("agent run did not complete successfully"); + } const received = analysis.received; const missing = Object.entries(expected) .filter(([propertyId, value]) => received[propertyId] !== value) @@ -177,7 +231,12 @@ export function scenarioToEval(scenario: ParityEvalScenario): Eval { ? [expectedToolScorer(scenario.expectedTools)] : []), ...(scenario.expectedPropertyValues - ? [expectedPropertyValuesScorer(scenario.expectedPropertyValues)] + ? [ + expectedPropertyValuesScorer( + scenario.expectedPropertyValues, + scenario.expectedCreateEnvelope, + ), + ] : []), ], }); From 0e7b09d59debbbb8154acbeb6c038fe52f7824e8 Mon Sep 17 00:00:00 2001 From: Alice Alexandra Moore <86723305+3mdistal@users.noreply.github.com> Date: Fri, 21 Aug 2026 16:06:51 -0400 Subject: [PATCH 05/28] chore: publish branch work in packages/core, templates/content (7 files) --- packages/core/src/eval/agent-runner.ts | 2 + packages/core/src/eval/runner.spec.ts | 25 ++++++- packages/core/src/eval/types.ts | 1 + .../actions/_database-property-input.ts | 5 +- .../database-row-property-input.test.ts | 33 ++++++++ .../__tests__/eval-scenario-coverage.test.ts | 75 +++++++++++++++++++ templates/content/parity/scenario-to-eval.ts | 50 ++++++++++++- 7 files changed, 186 insertions(+), 5 deletions(-) diff --git a/packages/core/src/eval/agent-runner.ts b/packages/core/src/eval/agent-runner.ts index 0c316ec3e7e..c986194a0e2 100644 --- a/packages/core/src/eval/agent-runner.ts +++ b/packages/core/src/eval/agent-runner.ts @@ -125,6 +125,7 @@ export async function createAgentRunner( id?: string; input: unknown; completed?: boolean; + completedSideEffect?: boolean; isError?: boolean; result?: string; }> = []; @@ -156,6 +157,7 @@ export async function createAgentRunner( ); if (detail) { detail.completed = true; + detail.completedSideEffect = event.completedSideEffect; detail.isError = event.isError === true; detail.result = event.result; } diff --git a/packages/core/src/eval/runner.spec.ts b/packages/core/src/eval/runner.spec.ts index d0b5df129c5..389844d1dd1 100644 --- a/packages/core/src/eval/runner.spec.ts +++ b/packages/core/src/eval/runner.spec.ts @@ -287,6 +287,20 @@ describe("createAgentRunner over a mocked runAgentLoop (no real model)", () => { tool: "search", id: "search-1", result: '{"ok":true}', + completedSideEffect: true, + }); + opts.send({ + type: "tool_start", + tool: "update", + id: "update-1", + input: {}, + }); + opts.send({ + type: "tool_done", + tool: "update", + id: "update-1", + result: '{"ok":false}', + completedSideEffect: false, }); opts.send({ type: "text", text: "world" }); return { @@ -309,15 +323,24 @@ describe("createAgentRunner over a mocked runAgentLoop (no real model)", () => { const out = await runner.runAgent({ prompt: "hi" }); expect(out.text).toBe("Hello world"); - expect(out.toolCalls).toEqual(["search"]); + expect(out.toolCalls).toEqual(["search", "update"]); expect(out.toolCallDetails).toEqual([ { name: "search", input: {}, completed: true, + completedSideEffect: true, isError: false, result: '{"ok":true}', }, + { + name: "update", + input: {}, + completed: true, + completedSideEffect: false, + isError: false, + result: '{"ok":false}', + }, ]); expect(out.ok).toBe(true); diff --git a/packages/core/src/eval/types.ts b/packages/core/src/eval/types.ts index d9f17f8fef6..81e32e03a86 100644 --- a/packages/core/src/eval/types.ts +++ b/packages/core/src/eval/types.ts @@ -36,6 +36,7 @@ export interface AgentRunOutput { readonly name: string; readonly input: unknown; readonly completed?: boolean; + readonly completedSideEffect?: boolean; readonly isError?: boolean; readonly result?: string; }[]; diff --git a/templates/content/actions/_database-property-input.ts b/templates/content/actions/_database-property-input.ts index 30552617c3e..053de8f77af 100644 --- a/templates/content/actions/_database-property-input.ts +++ b/templates/content/actions/_database-property-input.ts @@ -36,7 +36,10 @@ export function normalizeDatabasePropertyInput(input: { } if (!input.propertyEntries) return input.propertyValues; - const values: Record = {}; + const values: Record = Object.create(null) as Record< + string, + unknown + >; for (const entry of input.propertyEntries) { if (Object.prototype.hasOwnProperty.call(values, entry.propertyId)) { throw new ActionContractError( diff --git a/templates/content/parity/__tests__/database-row-property-input.test.ts b/templates/content/parity/__tests__/database-row-property-input.test.ts index aaa40f1e3d9..6e31ef055b7 100644 --- a/templates/content/parity/__tests__/database-row-property-input.test.ts +++ b/templates/content/parity/__tests__/database-row-property-input.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { canonicalizeDatabasePropertyInput, + databasePropertyEntriesSchema, normalizeDatabasePropertyInput, } from "../../actions/_database-property-input"; import { digest } from "../../actions/_database-row-mutation"; @@ -72,6 +73,23 @@ describe("database row property inputs", () => { ).toThrow(/not both/); }); + it("preserves __proto__ as an ordinary property definition ID", () => { + const propertyEntries = databasePropertyEntriesSchema.parse([ + { propertyId: "__proto__", value: "preserve me" }, + ]); + const normalized = normalizeDatabasePropertyInput({ + propertyEntries, + }); + + expect(normalized).toBeDefined(); + expect(Object.getPrototypeOf(normalized)).toBeNull(); + expect(Object.keys(normalized!)).toEqual(["__proto__"]); + expect(Object.prototype.hasOwnProperty.call(normalized, "__proto__")).toBe( + true, + ); + expect(normalized?.["__proto__"]).toBe("preserve me"); + }); + it("removes the model-only representation before canonical hashing", () => { const canonical = canonicalizeDatabasePropertyInput({ idempotencyKey: "same-intent", @@ -109,4 +127,19 @@ describe("database row property inputs", () => { expect(digest(fromEntries)).toBe(digest(fromRecord)); }); + + it("includes __proto__ property values in the canonical digest", () => { + const withPrototypeNamedProperty = canonicalizeDatabasePropertyInput({ + idempotencyKey: "same-intent", + propertyEntries: [{ propertyId: "__proto__", value: "preserve me" }], + }); + const withoutProperty = canonicalizeDatabasePropertyInput({ + idempotencyKey: "same-intent", + propertyValues: {}, + }); + + expect(digest(withPrototypeNamedProperty)).not.toBe( + digest(withoutProperty), + ); + }); }); diff --git a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts index 02bc2906626..bf0892c049c 100644 --- a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts +++ b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts @@ -18,6 +18,7 @@ function successfulCreateCall( ...propertyInput, }, completed: true, + completedSideEffect: true, isError: false, result: '{"fixtureOnly":true}', }; @@ -317,6 +318,80 @@ describe("Content parity eval scenarios", () => { return { ...call, isError: true, result: "fixture rejected" }; }, }, + { + name: "skipped side effect", + mutate(call: ReturnType) { + return { ...call, completedSideEffect: false }; + }, + }, + { + name: "an extra top-level field", + mutate(call: ReturnType) { + return { + ...call, + input: { + ...(call.input as Record), + hallucinated: true, + }, + }; + }, + }, + { + name: "an extra target field", + mutate(call: ReturnType) { + const input = call.input as Record; + return { + ...call, + input: { + ...input, + target: { + ...(input.target as Record), + hallucinated: true, + }, + }, + }; + }, + }, + { + name: "an extra authority field", + mutate(call: ReturnType) { + const input = call.input as Record; + const target = input.target as Record; + return { + ...call, + input: { + ...input, + target: { + ...target, + authorityScope: { + ...(target.authorityScope as Record), + hallucinated: true, + }, + }, + }, + }; + }, + }, + { + name: "an extra property-entry field", + mutate(call: ReturnType) { + const input = call.input as Record; + const { propertyValues, ...withoutPropertyValues } = input; + return { + ...call, + input: { + ...withoutPropertyValues, + propertyEntries: Object.entries( + propertyValues as Record, + ).map(([propertyId, value]) => ({ + propertyId, + value, + hallucinated: true, + })), + }, + }; + }, + }, ])("rejects $name", async ({ mutate }) => { process.env.CONTENT_PARITY_EVALS = "1"; const scenario = parityEvalScenarios.find( diff --git a/templates/content/parity/scenario-to-eval.ts b/templates/content/parity/scenario-to-eval.ts index 514fa94ceaf..6f7e50d378a 100644 --- a/templates/content/parity/scenario-to-eval.ts +++ b/templates/content/parity/scenario-to-eval.ts @@ -30,6 +30,18 @@ function expectedToolScorer(expectedTools: string[]) { }); } +function hasExactKeys( + record: Record, + expectedKeys: readonly string[], +): boolean { + const actualKeys = Object.keys(record).sort(); + const sortedExpectedKeys = [...expectedKeys].sort(); + return ( + actualKeys.length === sortedExpectedKeys.length && + actualKeys.every((key, index) => key === sortedExpectedKeys[index]) + ); +} + function analyzePropertyValues(input: unknown): { received: Record; invalid: string[]; @@ -63,7 +75,10 @@ function analyzePropertyValues(input: unknown): { return { received: {}, invalid: ["propertyEntries is not an array"] }; } - const received: Record = {}; + const received: Record = Object.create(null) as Record< + string, + unknown + >; const invalid: string[] = []; for (const entry of record.propertyEntries) { if (!entry || typeof entry !== "object" || Array.isArray(entry)) { @@ -71,6 +86,14 @@ function analyzePropertyValues(input: unknown): { continue; } const { propertyId, value } = entry as Record; + if ( + !hasExactKeys(entry as Record, ["propertyId", "value"]) + ) { + invalid.push( + "propertyEntries contains an entry with unrecognized fields", + ); + continue; + } if (typeof propertyId !== "string" || propertyId.length === 0) { invalid.push("propertyEntries contains an invalid propertyId"); continue; @@ -111,6 +134,22 @@ function matchesCreateEnvelope( } const actualAuthority = authorityScope as Record; return ( + hasExactKeys(input, [ + "target", + "expectedSchemaRevision", + "idempotencyKey", + "title", + input.propertyEntries === undefined + ? "propertyValues" + : "propertyEntries", + ]) && + hasExactKeys(actualTarget, [ + "authorityScope", + "spaceId", + "databaseId", + "databaseDocumentId", + ]) && + hasExactKeys(actualAuthority, ["kind", "id"]) && actualAuthority.kind === expected.target.authorityScope.kind && actualAuthority.id === expected.target.authorityScope.id && actualTarget.spaceId === expected.target.spaceId && @@ -171,7 +210,11 @@ function expectedPropertyValuesScorer( "create target, schema revision, idempotency key, or title did not match the fixture", ); } - if (!createCalls[0]?.completed || createCalls[0]?.isError) { + if ( + !createCalls[0]?.completed || + createCalls[0]?.completedSideEffect !== true || + createCalls[0]?.isError + ) { invalid.push("add-database-item did not complete successfully"); } if (!run.ok) { @@ -182,7 +225,8 @@ function expectedPropertyValuesScorer( .filter(([propertyId, value]) => received[propertyId] !== value) .map(([propertyId]) => propertyId); const unexpected = Object.keys(received).filter( - (propertyId) => !(propertyId in expected), + (propertyId) => + !Object.prototype.hasOwnProperty.call(expected, propertyId), ); return { received, missing, unexpected, invalid, mutationCalls }; }, From d01629941a4bddd18d7186747706525f8441995c Mon Sep 17 00:00:00 2001 From: Alice Moore <86723305+3mdistal@users.noreply.github.com> Date: Mon, 24 Aug 2026 17:13:50 +0000 Subject: [PATCH 06/28] fix: make Content database mutations agent-safe --- .../actions/_database-property-input.ts | 172 ++++++++++++++++-- .../content/actions/_database-row-mutation.ts | 103 +++++++++-- .../content/actions/add-database-item.ts | 6 +- .../roadmap-capability-projection.db.test.ts | 109 ++++++----- .../content/actions/update-database-item.ts | 8 +- .../upsert-database-item-by-key.db.test.ts | 47 ++++- .../actions/upsert-database-item-by-key.ts | 8 +- .../database-row-property-input.test.ts | 148 ++++++++++++--- .../__tests__/eval-scenario-coverage.test.ts | 119 +++++++++--- templates/content/parity/eval-scenarios.ts | 21 ++- templates/content/parity/scenario-to-eval.ts | 101 ++++++++-- 11 files changed, 681 insertions(+), 161 deletions(-) diff --git a/templates/content/actions/_database-property-input.ts b/templates/content/actions/_database-property-input.ts index 053de8f77af..5e89b4ccbcc 100644 --- a/templates/content/actions/_database-property-input.ts +++ b/templates/content/actions/_database-property-input.ts @@ -1,6 +1,129 @@ import { ActionContractError } from "@agent-native/core"; import { z } from "zod"; +const nullable = (schema: T) => + z.union([schema, z.null()]); + +const propertyIdSchema = z + .string() + .min(1) + .describe("Exact immutable property definition ID"); + +const stringPropertyEntry = ( + propertyType: "text" | "place" | "phone" | "url" | "email", + valueDescription: string, +) => + z + .object({ + propertyId: propertyIdSchema, + propertyType: z.literal(propertyType), + value: nullable(z.string()).describe(valueDescription), + }) + .strict(); + +const optionPropertyEntry = (propertyType: "select" | "status") => + z + .object({ + propertyId: propertyIdSchema, + propertyType: z.literal(propertyType), + value: nullable(z.string()).describe( + "Exact option ID or exact option label from the discovered property contract; null explicitly clears the value", + ), + }) + .strict(); + +export const databasePropertyEntrySchema = z.discriminatedUnion( + "propertyType", + [ + stringPropertyEntry("text", "Text value; null explicitly clears the value"), + stringPropertyEntry( + "place", + "Place text; null explicitly clears the value", + ), + stringPropertyEntry( + "phone", + "Phone text; null explicitly clears the value", + ), + stringPropertyEntry( + "url", + "Absolute http/https URL; null explicitly clears the value", + ), + stringPropertyEntry( + "email", + "Email address; null explicitly clears the value", + ), + z + .object({ + propertyId: propertyIdSchema, + propertyType: z.literal("number"), + value: nullable(z.number().finite()).describe( + "Finite number; use a JSON number rather than numeric text, or null to explicitly clear", + ), + }) + .strict(), + z + .object({ + propertyId: propertyIdSchema, + propertyType: z.literal("checkbox"), + value: nullable(z.boolean()).describe( + "Boolean; use true or false rather than text, or null to explicitly clear", + ), + }) + .strict(), + optionPropertyEntry("select"), + optionPropertyEntry("status"), + z + .object({ + propertyId: propertyIdSchema, + propertyType: z.literal("multi_select"), + value: nullable(z.array(z.string())).describe( + "Option IDs or exact option labels from the discovered property contract; null explicitly clears the value", + ), + }) + .strict(), + z + .object({ + propertyId: propertyIdSchema, + propertyType: z.literal("date"), + value: nullable( + z.union([ + z.string(), + z + .object({ + start: z.string(), + end: z.string().optional(), + includeTime: z.boolean().optional(), + }) + .strict(), + ]), + ).describe( + "ISO date/date-time string or { start, end?, includeTime? }; null explicitly clears the value", + ), + }) + .strict(), + z + .object({ + propertyId: propertyIdSchema, + propertyType: z.literal("person"), + value: nullable(z.array(z.string())).describe( + "Person identifiers from the discovered property contract; null explicitly clears the value", + ), + }) + .strict(), + z + .object({ + propertyId: propertyIdSchema, + propertyType: z.literal("files_media"), + value: nullable(z.array(z.string())).describe( + "Absolute http/https file URLs; null explicitly clears the value", + ), + }) + .strict(), + ], +); + +export type DatabasePropertyEntry = z.infer; + export const databasePropertyValuesSchema = z .record(z.string(), z.unknown()) .optional() @@ -9,37 +132,41 @@ export const databasePropertyValuesSchema = z ); export const databasePropertyEntriesSchema = z - .array( - z.object({ - propertyId: z - .string() - .min(1) - .describe("Exact immutable property definition ID"), - value: z.unknown().describe("Schema-valid value for this property"), - }), - ) + .array(databasePropertyEntrySchema) .max(1_000) .optional() .describe( - "Property values as explicit entries. Include one entry for every schema-valid writable property value the user requested, using the exact immutable property definition ID. When at least one value was requested, never pass an empty array. Do not invent or clear unmentioned properties.", + "Typed property values as explicit entries. Copy each propertyType from the discovered mutation contract and include one entry for every writable property value the user requested, using the exact immutable property definition ID. When at least one value was requested, never pass an empty array. Do not invent or clear unmentioned properties.", ); export function normalizeDatabasePropertyInput(input: { - propertyEntries?: Array<{ propertyId: string; value: unknown }>; + propertyEntries?: DatabasePropertyEntry[]; propertyValues?: Record; -}): Record | undefined { +}): { + propertyValues: Record | undefined; + propertyTypeAssertions: Record | undefined; +} { if (input.propertyEntries && input.propertyValues) { throw new ActionContractError( "Provide propertyEntries or propertyValues, not both.", { errorCode: "AMBIGUOUS_PROPERTY_INPUT" }, ); } - if (!input.propertyEntries) return input.propertyValues; + if (!input.propertyEntries) { + return { + propertyValues: input.propertyValues, + propertyTypeAssertions: undefined, + }; + } const values: Record = Object.create(null) as Record< string, unknown >; + const propertyTypes: Record = Object.create(null) as Record< + string, + string + >; for (const entry of input.propertyEntries) { if (Object.prototype.hasOwnProperty.call(values, entry.propertyId)) { throw new ActionContractError( @@ -51,26 +178,33 @@ export function normalizeDatabasePropertyInput(input: { ); } values[entry.propertyId] = entry.value; + propertyTypes[entry.propertyId] = entry.propertyType; } - return values; + return { + propertyValues: values, + propertyTypeAssertions: propertyTypes, + }; } export function canonicalizeDatabasePropertyInput< T extends { - propertyEntries?: Array<{ propertyId: string; value: unknown }>; + propertyEntries?: DatabasePropertyEntry[]; propertyValues?: Record; }, >( input: T, ): Omit & { propertyValues?: Record; + propertyTypeAssertions?: Record; } { const { propertyEntries, propertyValues, ...canonicalInput } = input; + const normalized = normalizeDatabasePropertyInput({ + propertyEntries, + propertyValues, + }); return { ...canonicalInput, - propertyValues: normalizeDatabasePropertyInput({ - propertyEntries, - propertyValues, - }), + propertyValues: normalized.propertyValues, + propertyTypeAssertions: normalized.propertyTypeAssertions, }; } diff --git a/templates/content/actions/_database-row-mutation.ts b/templates/content/actions/_database-row-mutation.ts index 5bab2f32e37..5b8372030a2 100644 --- a/templates/content/actions/_database-row-mutation.ts +++ b/templates/content/actions/_database-row-mutation.ts @@ -34,11 +34,13 @@ import { } from "./_position-utils.js"; import { nanoid } from "./_property-utils.js"; +const databaseMutationAuthorityScopeSchema = z.discriminatedUnion("kind", [ + z.object({ kind: z.literal("personal"), id: z.string().min(1) }), + z.object({ kind: z.literal("organization"), id: z.string().min(1) }), +]); + export const databaseMutationTargetSchema = z.object({ - authorityScope: z.discriminatedUnion("kind", [ - z.object({ kind: z.literal("personal"), id: z.string().min(1) }), - z.object({ kind: z.literal("organization"), id: z.string().min(1) }), - ]), + authorityScope: databaseMutationAuthorityScopeSchema, spaceId: z.string().min(1).describe("Exact Content space ID"), databaseId: z.string().min(1).describe("Exact Content database ID"), databaseDocumentId: z @@ -47,8 +49,35 @@ export const databaseMutationTargetSchema = z.object({ .describe("Exact page ID backing the Content database"), }); +export const databaseMutationTargetInputSchema = z.object({ + authorityScope: databaseMutationAuthorityScopeSchema + .optional() + .describe( + "Optional legacy assertion only. Agents must omit it; the authenticated server derives authority from the selected database.", + ), + spaceId: z + .string() + .min(1) + .describe("Exact Content space ID returned by database discovery"), + databaseId: z + .string() + .min(1) + .describe( + "Exact Content database ID returned by database discovery; never derive it from a title or number in the request", + ), + databaseDocumentId: z + .string() + .min(1) + .describe( + "Exact page ID backing the database, returned by database discovery", + ), +}); + +export const databaseMutationAgentTargetSchema = + databaseMutationTargetInputSchema.omit({ authorityScope: true }); + export const databaseMutationEnvelopeSchema = z.object({ - target: databaseMutationTargetSchema, + target: databaseMutationTargetInputSchema, expectedSchemaRevision: z .string() .min(1) @@ -59,6 +88,9 @@ export const databaseMutationEnvelopeSchema = z.object({ export type DatabaseMutationTarget = z.infer< typeof databaseMutationTargetSchema >; +export type DatabaseMutationTargetInput = z.infer< + typeof databaseMutationTargetInputSchema +>; type DatabaseRow = typeof schema.contentDatabases.$inferSelect; type DefinitionRow = typeof schema.documentPropertyDefinitions.$inferSelect; @@ -82,11 +114,12 @@ export interface RowSnapshot { export type DatabaseRowMutationOperation = "create" | "update" | "upsert"; export interface CreateDatabaseRowMutationInput { - target: DatabaseMutationTarget; + target: DatabaseMutationTargetInput; expectedSchemaRevision: string; idempotencyKey: string; title?: string; propertyValues?: Record; + propertyTypeAssertions?: Record; } export interface UpdateDatabaseRowMutationInput extends CreateDatabaseRowMutationInput { @@ -211,7 +244,7 @@ function acceptedShape(type: DocumentPropertyType): string { } export async function loadContext( - target: DatabaseMutationTarget, + target: DatabaseMutationTargetInput, role: "viewer" | "editor", db: Db = getDb(), accessAlreadyResolved = false, @@ -260,8 +293,9 @@ export async function loadContext( ? { kind: "organization" as const, id: database.orgId } : { kind: "personal" as const, id: database.ownerEmail }; if ( - target.authorityScope.kind !== authorityScope.kind || - target.authorityScope.id !== authorityScope.id || + (target.authorityScope !== undefined && + (target.authorityScope.kind !== authorityScope.kind || + target.authorityScope.id !== authorityScope.id)) || database.spaceId !== target.spaceId || database.documentId !== target.databaseDocumentId || databaseDocument.spaceId !== target.spaceId || @@ -322,7 +356,12 @@ export async function getDatabaseMutationContract( ); return { target: { - ...target, + authorityScope: context.database.orgId + ? { kind: "organization", id: context.database.orgId } + : { kind: "personal", id: context.database.ownerEmail }, + spaceId: context.database.spaceId!, + databaseId: context.database.id, + databaseDocumentId: context.database.documentId, }, schemaRevision: context.schemaRevision, naturalKeyPropertyId: context.database.naturalKeyPropertyId, @@ -695,14 +734,45 @@ export function revisionPropertyIds(context: MutationContext) { ); } -function payloadDigest( +export function databaseMutationPayloadDigest( operation: DatabaseRowMutationOperation, input: | CreateDatabaseRowMutationInput | UpdateDatabaseRowMutationInput | UpsertDatabaseRowMutationInput, ) { - return digest({ operation, ...input }); + const { propertyTypeAssertions: _propertyTypeAssertions, ...canonicalInput } = + input; + return digest({ operation, ...canonicalInput }); +} + +function assertPropertyTypeAssertions( + context: MutationContext, + assertions: Record | undefined, +) { + if (!assertions) return; + const definitionsById = new Map( + context.definitions.map((definition) => [definition.id, definition]), + ); + for (const [propertyId, assertedType] of Object.entries(assertions)) { + const definition = definitionsById.get(propertyId); + if (!definition) { + throw new ActionContractError( + `Unknown property definition "${propertyId}".`, + { + errorCode: "UNKNOWN_PROPERTY", + details: { propertyId }, + statusCode: 400, + }, + ); + } + if (definition.type !== assertedType) { + invalidProperty( + definition, + `typed entry declared propertyType "${assertedType}" but the discovered property type is "${definition.type}"`, + ); + } + } } function resultForReceipt( @@ -1117,7 +1187,8 @@ export async function createDatabaseRow( input: CreateDatabaseRowMutationInput, ): Promise { const initial = await loadContext(input.target, "editor"); - const inputDigest = payloadDigest("create", input); + assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); + const inputDigest = databaseMutationPayloadDigest("create", input); const replay = await replayReceipt( initial, input.idempotencyKey, @@ -1185,8 +1256,9 @@ export async function updateDatabaseRow( input: UpdateDatabaseRowMutationInput, ): Promise { const initial = await loadContext(input.target, "editor"); + assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); await assertAccess("document", input.documentId, "editor"); - const inputDigest = payloadDigest("update", input); + const inputDigest = databaseMutationPayloadDigest("update", input); const replay = await replayReceipt( initial, input.idempotencyKey, @@ -1265,6 +1337,7 @@ export async function upsertDatabaseRow( input: UpsertDatabaseRowMutationInput, ): Promise { const initial = await loadContext(input.target, "editor"); + assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); const replayKeyPropertyId = initial.database.naturalKeyPropertyId; const replayKeyDefinition = replayKeyPropertyId ? initial.definitions.find( @@ -1285,7 +1358,7 @@ export async function upsertDatabaseRow( "must match the upsert keyValue when provided in propertyValues", ); } - const inputDigest = payloadDigest("upsert", input); + const inputDigest = databaseMutationPayloadDigest("upsert", input); const replay = await replayReceipt( initial, input.idempotencyKey, diff --git a/templates/content/actions/add-database-item.ts b/templates/content/actions/add-database-item.ts index 8666c33bdfd..423d675b0bc 100644 --- a/templates/content/actions/add-database-item.ts +++ b/templates/content/actions/add-database-item.ts @@ -10,6 +10,7 @@ import { } from "./_database-property-input.js"; import { createDatabaseRow, + databaseMutationAgentTargetSchema, databaseMutationEnvelopeSchema, } from "./_database-row-mutation.js"; import { getContentDatabaseResponse } from "./_database-utils.js"; @@ -25,11 +26,14 @@ const schema = databaseMutationEnvelopeSchema.extend({ propertyValues: databasePropertyValuesSchema, propertyEntries: databasePropertyEntriesSchema, }); +const agentSchema = schema + .extend({ target: databaseMutationAgentTargetSchema }) + .omit({ propertyValues: true }); export default defineAction({ description: "Create one row in an exact ordinary Content database using its discovered schema revision. Strictly validates every non-Blocks property, applies the side effect once per idempotency key, and returns a verified receipt with stable row identity.", - agentInputSchema: schema.omit({ propertyValues: true }), + agentInputSchema: agentSchema, publicAgent: { expose: true, readOnly: false, diff --git a/templates/content/actions/roadmap-capability-projection.db.test.ts b/templates/content/actions/roadmap-capability-projection.db.test.ts index 648cf0dc711..221bbf1d193 100644 --- a/templates/content/actions/roadmap-capability-projection.db.test.ts +++ b/templates/content/actions/roadmap-capability-projection.db.test.ts @@ -143,6 +143,7 @@ describe("private roadmap capability projection", () => { databaseId, name, type, + naturalKey: name === "Capability ID", }), ); const property = configured.properties.find( @@ -158,28 +159,45 @@ describe("private roadmap capability projection", () => { const firstReceipts = new Map< string, - { itemId: string; documentId: string } + { itemId: string; documentId: string; rowRevision: string } >(); + const discovered = await asUser(OWNER, () => + getDatabase.run({ databaseId, limit: 1, offset: 0 }), + ); + if (!("database" in discovered) || !discovered.mutationContract) + throw new Error("Projection database has no mutation contract."); + const mutationEnvelope = { + target: { + spaceId: discovered.mutationContract.target.spaceId, + databaseId: discovered.mutationContract.target.databaseId, + databaseDocumentId: + discovered.mutationContract.target.databaseDocumentId, + }, + expectedSchemaRevision: discovered.mutationContract.schemaRevision, + }; + const propertyValuesFor = (capability: Capability) => ({ + [keyPropertyId]: capability.id, + [propertyIds.get("State")!]: capability.state, + [propertyIds.get("Publicness")!]: capability.publicness, + [propertyIds.get("User promise")!]: capability.userPromise, + [propertyIds.get("Source revision")!]: SOURCE_REVISION, + }); for (const capability of capabilities) { const receipt = await asUser(OWNER, () => upsert.run({ - databaseId, - keyPropertyId, + ...mutationEnvelope, + idempotencyKey: `roadmap-projection-${capability.id}`, keyValue: capability.id, + expectedRowRevision: null, title: capability.name, - body: capability.body, - propertyValues: { - [propertyIds.get("State")!]: capability.state, - [propertyIds.get("Publicness")!]: capability.publicness, - [propertyIds.get("User promise")!]: capability.userPromise, - [propertyIds.get("Source revision")!]: SOURCE_REVISION, - }, + propertyValues: propertyValuesFor(capability), }), ); - expect(receipt.status).toBe("created"); + expect(receipt.receipt.outcome).toBe("created"); firstReceipts.set(capability.id, { - itemId: receipt.itemId, - documentId: receipt.documentId, + itemId: receipt.receipt.row.itemId, + documentId: receipt.receipt.row.documentId, + rowRevision: receipt.receipt.row.rowRevision, }); } @@ -191,55 +209,53 @@ describe("private roadmap capability projection", () => { throw new Error("Changed Capability is missing its first receipt."); const changedReceipt = await asUser(OWNER, () => upsert.run({ - databaseId, - keyPropertyId, + ...mutationEnvelope, + idempotencyKey: "roadmap-projection-change", keyValue: changedCapability.id, + expectedRowRevision: changedIdentity.rowRevision, title: `${changedCapability.name} — changed`, }), ); - expect(changedReceipt).toMatchObject({ - status: "updated", - ...changedIdentity, + expect(changedReceipt.receipt).toMatchObject({ + outcome: "updated", + row: { + itemId: changedIdentity.itemId, + documentId: changedIdentity.documentId, + }, }); const restoredReceipt = await asUser(OWNER, () => upsert.run({ - databaseId, - keyPropertyId, + ...mutationEnvelope, + idempotencyKey: "roadmap-projection-restore", keyValue: changedCapability.id, + expectedRowRevision: changedReceipt.receipt.row.rowRevision, title: changedCapability.name, - body: changedCapability.body, - propertyValues: { - [propertyIds.get("State")!]: changedCapability.state, - [propertyIds.get("Publicness")!]: changedCapability.publicness, - [propertyIds.get("User promise")!]: changedCapability.userPromise, - [propertyIds.get("Source revision")!]: SOURCE_REVISION, - }, + propertyValues: propertyValuesFor(changedCapability), }), ); - expect(restoredReceipt).toMatchObject({ - status: "updated", - ...changedIdentity, + expect(restoredReceipt.receipt).toMatchObject({ + outcome: "updated", + row: { + itemId: changedIdentity.itemId, + documentId: changedIdentity.documentId, + }, }); for (const capability of capabilities) { const receipt = await asUser(OWNER, () => upsert.run({ - databaseId, - keyPropertyId, + ...mutationEnvelope, + idempotencyKey: `roadmap-projection-${capability.id}`, keyValue: capability.id, + expectedRowRevision: null, title: capability.name, - body: capability.body, - propertyValues: { - [propertyIds.get("State")!]: capability.state, - [propertyIds.get("Publicness")!]: capability.publicness, - [propertyIds.get("User promise")!]: capability.userPromise, - [propertyIds.get("Source revision")!]: SOURCE_REVISION, - }, + propertyValues: propertyValuesFor(capability), }), ); - expect(receipt).toMatchObject({ - status: "unchanged", - ...firstReceipts.get(capability.id), + expect(receipt.receipt).toMatchObject({ + outcome: "created", + idempotency: { result: "replayed" }, + row: firstReceipts.get(capability.id), }); } @@ -282,7 +298,14 @@ describe("private roadmap capability projection", () => { expect(readbackIds).toEqual( new Set(capabilities.map((capability) => capability.id)), ); - expect(readbackIdentity).toEqual(firstReceipts); + expect(readbackIdentity).toEqual( + new Map( + [...firstReceipts].map(([key, { itemId, documentId }]) => [ + key, + { itemId, documentId }, + ]), + ), + ); const uniqueRoot = await asUser(OWNER, () => searchDocuments.run({ diff --git a/templates/content/actions/update-database-item.ts b/templates/content/actions/update-database-item.ts index 4fcd904e532..3ceb2e3e65e 100644 --- a/templates/content/actions/update-database-item.ts +++ b/templates/content/actions/update-database-item.ts @@ -9,6 +9,7 @@ import { databasePropertyValuesSchema, } from "./_database-property-input.js"; import { + databaseMutationAgentTargetSchema, databaseMutationEnvelopeSchema, updateDatabaseRow, } from "./_database-row-mutation.js"; @@ -23,14 +24,17 @@ const schema = databaseMutationEnvelopeSchema.extend({ title: z.string().trim().min(1).max(500).optional(), propertyValues: databasePropertyValuesSchema, propertyEntries: databasePropertyEntriesSchema.describe( - "Sparse property patch as explicit entries; omitted fields are preserved and explicit null clears a value. Include one entry for every schema-valid writable property value the user requested, using the exact immutable property definition ID. When at least one value was requested, never pass an empty array. Do not invent or clear unmentioned properties.", + "Sparse typed property patch as explicit entries; omitted fields are preserved and explicit null clears a value. Copy each propertyType from the discovered mutation contract and include one entry for every writable property value the user requested, using the exact immutable property definition ID. When at least one value was requested, never pass an empty array. Do not invent or clear unmentioned properties.", ), }); +const agentSchema = schema + .extend({ target: databaseMutationAgentTargetSchema }) + .omit({ propertyValues: true }); export default defineAction({ description: "Sparsely update one exact Content database row by stable item and document IDs. Requires schema and row revisions, validates every non-Blocks property, and returns a verified idempotent receipt.", - agentInputSchema: schema.omit({ propertyValues: true }), + agentInputSchema: agentSchema, schema, http: { method: "PUT" }, audit: { diff --git a/templates/content/actions/upsert-database-item-by-key.db.test.ts b/templates/content/actions/upsert-database-item-by-key.db.test.ts index a817f07ffa1..b0cc74cf97a 100644 --- a/templates/content/actions/upsert-database-item-by-key.db.test.ts +++ b/templates/content/actions/upsert-database-item-by-key.db.test.ts @@ -95,7 +95,6 @@ function envelope( ) { return { target: { - authorityScope: discovered.target.authorityScope, spaceId: discovered.target.spaceId, databaseId: discovered.target.databaseId, databaseDocumentId: discovered.target.databaseDocumentId, @@ -215,6 +214,52 @@ describe("reliable Content database row mutations", () => { ); }); + it("derives authority and validates typed agent property entries against the discovered contract", async () => { + const ids = await fixture(); + const evidenceId = await addProperty({ + ...ids, + name: "Evidence", + type: "text", + }); + const discovered = await contract(ids.databaseId); + const input = { + ...envelope(discovered, "typed-agent-create"), + title: "Typed agent row", + propertyEntries: [ + { + propertyId: evidenceId, + propertyType: "text" as const, + value: "preserve me", + }, + ], + }; + + const created = await asOwner(() => createRow.run(input)); + expect(created.receipt.target.authorityScope).toEqual({ + kind: "personal", + id: OWNER, + }); + expect(created.receipt.readback.propertyValues[evidenceId]).toBe( + "preserve me", + ); + + await expect( + asOwner(() => + createRow.run({ + ...input, + idempotencyKey: "typed-agent-mismatched-type", + propertyEntries: [ + { + propertyId: evidenceId, + propertyType: "number", + value: 3314, + }, + ], + }), + ), + ).rejects.toMatchObject({ errorCode: "INVALID_PROPERTY_VALUE" }); + }); + it("creates every supported non-Blocks value without coercion and returns one durable verified receipt", async () => { const ids = await fixture(); const propertyIds = { diff --git a/templates/content/actions/upsert-database-item-by-key.ts b/templates/content/actions/upsert-database-item-by-key.ts index 93dcb1ef43d..0dd5d4a7e4d 100644 --- a/templates/content/actions/upsert-database-item-by-key.ts +++ b/templates/content/actions/upsert-database-item-by-key.ts @@ -9,6 +9,7 @@ import { databasePropertyValuesSchema, } from "./_database-property-input.js"; import { + databaseMutationAgentTargetSchema, databaseMutationEnvelopeSchema, upsertDatabaseRow, } from "./_database-row-mutation.js"; @@ -25,14 +26,17 @@ const schema = databaseMutationEnvelopeSchema.extend({ title: z.string().trim().min(1).max(500).optional(), propertyValues: databasePropertyValuesSchema, propertyEntries: databasePropertyEntriesSchema.describe( - "Sparse property values as explicit entries. Include one entry for every schema-valid writable property value the user requested, using the exact immutable property definition ID. When at least one value was requested, never pass an empty array. Do not invent or clear unmentioned properties.", + "Sparse typed property values as explicit entries. Copy each propertyType from the discovered mutation contract and include one entry for every writable property value the user requested, using the exact immutable property definition ID. When at least one value was requested, never pass an empty array. Do not invent or clear unmentioned properties.", ), }); +const agentSchema = schema + .extend({ target: databaseMutationAgentTargetSchema }) + .omit({ propertyValues: true }); export default defineAction({ description: "Create or sparsely update one Content database row by that database's explicitly configured natural key. Requires schema and row compare-and-swap revisions and returns a verified idempotent receipt.", - agentInputSchema: schema.omit({ propertyValues: true }), + agentInputSchema: agentSchema, schema, audit: { recordInputs: false, diff --git a/templates/content/parity/__tests__/database-row-property-input.test.ts b/templates/content/parity/__tests__/database-row-property-input.test.ts index 6e31ef055b7..037dc942b93 100644 --- a/templates/content/parity/__tests__/database-row-property-input.test.ts +++ b/templates/content/parity/__tests__/database-row-property-input.test.ts @@ -5,7 +5,7 @@ import { databasePropertyEntriesSchema, normalizeDatabasePropertyInput, } from "../../actions/_database-property-input"; -import { digest } from "../../actions/_database-row-mutation"; +import { databaseMutationPayloadDigest } from "../../actions/_database-row-mutation"; import addDatabaseItem from "../../actions/add-database-item"; import updateDatabaseItem from "../../actions/update-database-item"; import upsertDatabaseItemByKey from "../../actions/upsert-database-item-by-key"; @@ -24,11 +24,13 @@ describe("database row property inputs", () => { expect(properties).not.toHaveProperty("propertyValues"); const propertyEntries = properties.propertyEntries; expect(propertyEntries.type).toBe("array"); - expect(propertyEntries.items.properties.propertyId.description).toContain( + expect(JSON.stringify(propertyEntries.items)).toContain( "Exact immutable property definition ID", ); + expect(JSON.stringify(propertyEntries.items)).toContain("propertyType"); + expect(JSON.stringify(propertyEntries.items)).not.toContain('"value":{}'); expect(propertyEntries.description).toContain( - "Include one entry for every schema-valid writable property value the user requested", + "include one entry for every writable property value the user requested", ); expect(propertyEntries.description).toContain( "never pass an empty array", @@ -36,6 +38,7 @@ describe("database row property inputs", () => { expect(propertyEntries.description).toContain( "Do not invent or clear unmentioned properties", ); + expect(properties.target.properties).not.toHaveProperty("authorityScope"); }, ); @@ -43,13 +46,27 @@ describe("database row property inputs", () => { expect( normalizeDatabasePropertyInput({ propertyEntries: [ - { propertyId: "status-id", value: "ready" }, - { propertyId: "evidence-id", value: "preserve me" }, + { + propertyId: "status-id", + propertyType: "status", + value: "ready", + }, + { + propertyId: "evidence-id", + propertyType: "text", + value: "preserve me", + }, ], }), ).toEqual({ - "status-id": "ready", - "evidence-id": "preserve me", + propertyValues: { + "status-id": "ready", + "evidence-id": "preserve me", + }, + propertyTypeAssertions: { + "status-id": "status", + "evidence-id": "text", + }, }); }); @@ -57,8 +74,16 @@ describe("database row property inputs", () => { expect(() => normalizeDatabasePropertyInput({ propertyEntries: [ - { propertyId: "status-id", value: "ready" }, - { propertyId: "status-id", value: "changed" }, + { + propertyId: "status-id", + propertyType: "status", + value: "ready", + }, + { + propertyId: "status-id", + propertyType: "status", + value: "changed", + }, ], }), ).toThrow(/provided more than once/); @@ -67,7 +92,13 @@ describe("database row property inputs", () => { it("rejects ambiguous entry and record inputs", () => { expect(() => normalizeDatabasePropertyInput({ - propertyEntries: [{ propertyId: "status-id", value: "ready" }], + propertyEntries: [ + { + propertyId: "status-id", + propertyType: "status", + value: "ready", + }, + ], propertyValues: { "status-id": "ready" }, }), ).toThrow(/not both/); @@ -75,27 +106,70 @@ describe("database row property inputs", () => { it("preserves __proto__ as an ordinary property definition ID", () => { const propertyEntries = databasePropertyEntriesSchema.parse([ - { propertyId: "__proto__", value: "preserve me" }, + { + propertyId: "__proto__", + propertyType: "text", + value: "preserve me", + }, ]); const normalized = normalizeDatabasePropertyInput({ propertyEntries, }); - expect(normalized).toBeDefined(); - expect(Object.getPrototypeOf(normalized)).toBeNull(); - expect(Object.keys(normalized!)).toEqual(["__proto__"]); - expect(Object.prototype.hasOwnProperty.call(normalized, "__proto__")).toBe( - true, - ); - expect(normalized?.["__proto__"]).toBe("preserve me"); + expect(Object.getPrototypeOf(normalized.propertyValues)).toBeNull(); + expect(Object.keys(normalized.propertyValues!)).toEqual(["__proto__"]); + expect( + Object.prototype.hasOwnProperty.call( + normalized.propertyValues, + "__proto__", + ), + ).toBe(true); + expect(normalized.propertyValues?.["__proto__"]).toBe("preserve me"); + expect(normalized.propertyTypeAssertions?.["__proto__"]).toBe("text"); + }); + + it("requires a schema-visible type and its matching JSON value shape", () => { + expect(() => + databasePropertyEntriesSchema.parse([ + { propertyId: "count-id", value: 3314 }, + ]), + ).toThrow(/propertyType/); + expect(() => + databasePropertyEntriesSchema.parse([ + { + propertyId: "count-id", + propertyType: "number", + value: "3314", + }, + ]), + ).toThrow(); + expect( + databasePropertyEntriesSchema.parse([ + { + propertyId: "count-id", + propertyType: "number", + value: 3314, + }, + ]), + ).toEqual([ + { propertyId: "count-id", propertyType: "number", value: 3314 }, + ]); }); it("removes the model-only representation before canonical hashing", () => { const canonical = canonicalizeDatabasePropertyInput({ idempotencyKey: "same-intent", propertyEntries: [ - { propertyId: "status-id", value: "ready" }, - { propertyId: "evidence-id", value: "preserve me" }, + { + propertyId: "status-id", + propertyType: "status", + value: "ready", + }, + { + propertyId: "evidence-id", + propertyType: "text", + value: "preserve me", + }, ], }); @@ -105,6 +179,10 @@ describe("database row property inputs", () => { "status-id": "ready", "evidence-id": "preserve me", }, + propertyTypeAssertions: { + "status-id": "status", + "evidence-id": "text", + }, }); expect(canonical).not.toHaveProperty("propertyEntries"); }); @@ -113,8 +191,16 @@ describe("database row property inputs", () => { const fromEntries = canonicalizeDatabasePropertyInput({ idempotencyKey: "same-intent", propertyEntries: [ - { propertyId: "status-id", value: "ready" }, - { propertyId: "evidence-id", value: "preserve me" }, + { + propertyId: "status-id", + propertyType: "status", + value: "ready", + }, + { + propertyId: "evidence-id", + propertyType: "text", + value: "preserve me", + }, ], }); const fromRecord = canonicalizeDatabasePropertyInput({ @@ -125,21 +211,29 @@ describe("database row property inputs", () => { }, }); - expect(digest(fromEntries)).toBe(digest(fromRecord)); + expect(databaseMutationPayloadDigest("create", fromEntries)).toBe( + databaseMutationPayloadDigest("create", fromRecord), + ); }); it("includes __proto__ property values in the canonical digest", () => { const withPrototypeNamedProperty = canonicalizeDatabasePropertyInput({ idempotencyKey: "same-intent", - propertyEntries: [{ propertyId: "__proto__", value: "preserve me" }], + propertyEntries: [ + { + propertyId: "__proto__", + propertyType: "text", + value: "preserve me", + }, + ], }); const withoutProperty = canonicalizeDatabasePropertyInput({ idempotencyKey: "same-intent", propertyValues: {}, }); - expect(digest(withPrototypeNamedProperty)).not.toBe( - digest(withoutProperty), - ); + expect( + databaseMutationPayloadDigest("create", withPrototypeNamedProperty), + ).not.toBe(databaseMutationPayloadDigest("create", withoutProperty)); }); }); diff --git a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts index bf0892c049c..4e00f6e9bc6 100644 --- a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts +++ b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts @@ -24,6 +24,45 @@ function successfulCreateCall( }; } +function typedExpectedEntries(scenario: (typeof parityEvalScenarios)[number]) { + return Object.entries(scenario.expectedPropertyValues ?? {}).map( + ([propertyId, value]) => ({ + propertyId, + propertyType: scenario.expectedPropertyTypes?.[propertyId], + value, + }), + ); +} + +const discoveryCalls = ["list-content-databases", "get-content-database"]; + +function successfulDiscoveryDetails( + scenario: (typeof parityEvalScenarios)[number], +) { + return [ + { + name: "list-content-databases", + input: { title: "PR #3314 feedback" }, + completed: true, + isError: false, + result: JSON.stringify({ + databases: [scenario.expectedCreateEnvelope?.target], + }), + }, + { + name: "get-content-database", + input: { + databaseId: scenario.expectedCreateEnvelope?.target.databaseId, + }, + completed: true, + isError: false, + result: JSON.stringify({ + mutationContract: scenario.expectedCreateEnvelope, + }), + }, + ]; +} + afterEach(() => { if (OLD_GATE === undefined) { delete process.env.CONTENT_PARITY_EVALS; @@ -171,9 +210,10 @@ describe("Content parity eval scenarios", () => { const row = await scoreEval(evalCase, { runAgent: vi.fn(async () => ({ text: scenario.successSignals.join("\n"), - toolCalls: ["add-database-item"], + toolCalls: [...discoveryCalls, "add-database-item"], toolCallDetails: [ - successfulCreateCall(scenario, { propertyValues: {} }), + ...successfulDiscoveryDetails(scenario), + successfulCreateCall(scenario, { propertyEntries: [] }), ], ok: true, runId: "content-parity:empty-property-values", @@ -199,10 +239,11 @@ describe("Content parity eval scenarios", () => { const row = await scoreEval(evalCase, { runAgent: vi.fn(async () => ({ text: scenario.successSignals.join("\n"), - toolCalls: ["add-database-item"], + toolCalls: [...discoveryCalls, "add-database-item"], toolCallDetails: [ + ...successfulDiscoveryDetails(scenario), successfulCreateCall(scenario, { - propertyValues: scenario.expectedPropertyValues, + propertyEntries: typedExpectedEntries(scenario), }), ], ok: true, @@ -226,9 +267,21 @@ describe("Content parity eval scenarios", () => { toolCallDetails: [ successfulCreateCall(scenario, { propertyEntries: [ - { propertyId: "parity-text-property-id", value: "preserve me" }, - { propertyId: "parity-text-property-id", value: "preserve me" }, - { propertyId: "parity-status-property-id", value: "ready" }, + { + propertyId: "fixture_evidence_property", + propertyType: "text", + value: "Baseline fixture preserve-me", + }, + { + propertyId: "fixture_evidence_property", + propertyType: "text", + value: "Baseline fixture preserve-me", + }, + { + propertyId: "fixture_status_property", + propertyType: "status", + value: "status-cannot-verify", + }, ], }), ], @@ -238,8 +291,16 @@ describe("Content parity eval scenarios", () => { toolCallDetails: [ successfulCreateCall(scenario, { propertyEntries: [ - { propertyId: "parity-text-property-id", value: "preserve me" }, - { propertyId: "parity-status-property-id", value: "ready" }, + { + propertyId: "fixture_evidence_property", + propertyType: "text", + value: "Baseline fixture preserve-me", + }, + { + propertyId: "fixture_status_property", + propertyType: "status", + value: "status-cannot-verify", + }, ], propertyValues: { "parity-text-property-id": "preserve me", @@ -253,8 +314,16 @@ describe("Content parity eval scenarios", () => { toolCallDetails: [ successfulCreateCall(scenario, { propertyEntries: [ - { propertyId: "parity-text-property-id", value: "preserve me" }, - { propertyId: "parity-status-property-id", value: "ready" }, + { + propertyId: "fixture_evidence_property", + propertyType: "text", + value: "Baseline fixture preserve-me", + }, + { + propertyId: "fixture_status_property", + propertyType: "status", + value: "status-cannot-verify", + }, ], }), { @@ -276,8 +345,14 @@ describe("Content parity eval scenarios", () => { const row = await scoreEval(evalCase, { runAgent: vi.fn(async () => ({ text: scenario.successSignals.join("\n"), - toolCalls: toolCallDetails.map((call) => call.name), - toolCallDetails, + toolCalls: [ + ...discoveryCalls, + ...toolCallDetails.map((call) => call.name), + ], + toolCallDetails: [ + ...successfulDiscoveryDetails(scenario), + ...toolCallDetails, + ], ok: true, runId: "content-parity:invalid-property-input", durationMs: 1, @@ -376,16 +451,14 @@ describe("Content parity eval scenarios", () => { name: "an extra property-entry field", mutate(call: ReturnType) { const input = call.input as Record; - const { propertyValues, ...withoutPropertyValues } = input; return { ...call, input: { - ...withoutPropertyValues, - propertyEntries: Object.entries( - propertyValues as Record, - ).map(([propertyId, value]) => ({ - propertyId, - value, + ...input, + propertyEntries: ( + input.propertyEntries as Array> + ).map((entry) => ({ + ...entry, hallucinated: true, })), }, @@ -399,15 +472,15 @@ describe("Content parity eval scenarios", () => { )!; const call = mutate( successfulCreateCall(scenario, { - propertyValues: scenario.expectedPropertyValues, + propertyEntries: typedExpectedEntries(scenario), }), ); const evalCase = scenarioToEval(scenario); const row = await scoreEval(evalCase, { runAgent: vi.fn(async () => ({ text: scenario.successSignals.join("\n"), - toolCalls: ["add-database-item"], - toolCallDetails: [call], + toolCalls: [...discoveryCalls, "add-database-item"], + toolCallDetails: [...successfulDiscoveryDetails(scenario), call], ok: true, runId: "content-parity:rejected-create", durationMs: 1, diff --git a/templates/content/parity/eval-scenarios.ts b/templates/content/parity/eval-scenarios.ts index 3c26553e2bc..d8a91fc765e 100644 --- a/templates/content/parity/eval-scenarios.ts +++ b/templates/content/parity/eval-scenarios.ts @@ -9,9 +9,9 @@ export interface ParityEvalScenario { successSignals: string[]; expectedTools?: string[]; expectedPropertyValues?: Record; + expectedPropertyTypes?: Record; expectedCreateEnvelope?: { target: { - authorityScope: { kind: "personal"; id: string }; spaceId: string; databaseId: string; databaseDocumentId: string; @@ -31,24 +31,29 @@ export const parityEvalScenarios: ParityEvalScenario[] = [ defaultState: "skipped", requiresPrivateCredentials: false, prompt: - "Create exactly one row in the already-verified fixture Content database. Do not rediscover or alter the target. Use target authorityScope { kind: personal, id: fixture-owner@example.test }, spaceId fixture_personal_space, databaseId fixture_feedback_database, and databaseDocumentId fixture_feedback_document; expectedSchemaRevision fixture_schema_revision; title [FIXTURE] preserve explicit properties; idempotencyKey fixture-create-property-preservation-v1. Set property fixture_status_property to status-cannot-verify and property fixture_evidence_property to Baseline fixture preserve-me. No Blocks value was requested. Call add-database-item once with every exact target constraint and both property entries, then report its result truthfully.", + "Find the fixture Content database titled PR #3314 feedback — do not treat 3314 as its database ID. Discover its exact database target and mutation contract through Content actions, then create exactly one row titled [FIXTURE] preserve explicit properties with idempotency key fixture-create-property-preservation-v1. Set Status to status-cannot-verify and Evidence to Baseline fixture preserve-me using the discovered immutable property IDs and property types. Omit authority identity because the authenticated server owns it. No Blocks value was requested. Call add-database-item once, then report its result truthfully.", successSignals: [ - "Uses add-database-item once for the exact fixture target.", + "Discovers the exact database and mutation contract before creating.", + "Uses add-database-item once for the discovered stable target without authoring authority identity.", "Preserves both explicitly requested writable property values.", "Does not invent a Blocks value or another property.", "Reports an action failure rather than claiming a row was created if the fixture is unavailable.", ], - expectedTools: ["add-database-item"], + expectedTools: [ + "list-content-databases", + "get-content-database", + "add-database-item", + ], expectedPropertyValues: { fixture_status_property: "status-cannot-verify", fixture_evidence_property: "Baseline fixture preserve-me", }, + expectedPropertyTypes: { + fixture_status_property: "status", + fixture_evidence_property: "text", + }, expectedCreateEnvelope: { target: { - authorityScope: { - kind: "personal", - id: "fixture-owner@example.test", - }, spaceId: "fixture_personal_space", databaseId: "fixture_feedback_database", databaseDocumentId: "fixture_feedback_document", diff --git a/templates/content/parity/scenario-to-eval.ts b/templates/content/parity/scenario-to-eval.ts index 6f7e50d378a..39e985bb597 100644 --- a/templates/content/parity/scenario-to-eval.ts +++ b/templates/content/parity/scenario-to-eval.ts @@ -44,10 +44,15 @@ function hasExactKeys( function analyzePropertyValues(input: unknown): { received: Record; + receivedTypes: Record; invalid: string[]; } { if (!input || typeof input !== "object" || Array.isArray(input)) { - return { received: {}, invalid: ["tool input is not an object"] }; + return { + received: {}, + receivedTypes: {}, + invalid: ["tool input is not an object"], + }; } const record = input as Record; const hasEntries = record.propertyEntries !== undefined; @@ -55,6 +60,7 @@ function analyzePropertyValues(input: unknown): { if (hasEntries && hasValues) { return { received: {}, + receivedTypes: {}, invalid: ["propertyEntries and propertyValues were both provided"], }; } @@ -64,30 +70,50 @@ function analyzePropertyValues(input: unknown): { typeof record.propertyValues !== "object" || Array.isArray(record.propertyValues) ) { - return { received: {}, invalid: ["propertyValues is not a record"] }; + return { + received: {}, + receivedTypes: {}, + invalid: ["propertyValues is not a record"], + }; } return { received: record.propertyValues as Record, - invalid: [], + receivedTypes: {}, + invalid: ["propertyValues bypassed the typed agent input"], }; } if (!hasEntries || !Array.isArray(record.propertyEntries)) { - return { received: {}, invalid: ["propertyEntries is not an array"] }; + return { + received: {}, + receivedTypes: {}, + invalid: ["propertyEntries is not an array"], + }; } const received: Record = Object.create(null) as Record< string, unknown >; + const receivedTypes: Record = Object.create(null) as Record< + string, + string + >; const invalid: string[] = []; for (const entry of record.propertyEntries) { if (!entry || typeof entry !== "object" || Array.isArray(entry)) { invalid.push("propertyEntries contains a non-object entry"); continue; } - const { propertyId, value } = entry as Record; + const { propertyId, propertyType, value } = entry as Record< + string, + unknown + >; if ( - !hasExactKeys(entry as Record, ["propertyId", "value"]) + !hasExactKeys(entry as Record, [ + "propertyId", + "propertyType", + "value", + ]) ) { invalid.push( "propertyEntries contains an entry with unrecognized fields", @@ -98,13 +124,20 @@ function analyzePropertyValues(input: unknown): { invalid.push("propertyEntries contains an invalid propertyId"); continue; } + if (typeof propertyType !== "string" || propertyType.length === 0) { + invalid.push( + `propertyEntries contains an invalid type for ${propertyId}`, + ); + continue; + } if (Object.prototype.hasOwnProperty.call(received, propertyId)) { invalid.push(`propertyEntries contains duplicate ID ${propertyId}`); continue; } received[propertyId] = value; + receivedTypes[propertyId] = propertyType; } - return { received, invalid }; + return { received, receivedTypes, invalid }; } const databaseRowMutationTools = new Set([ @@ -124,15 +157,6 @@ function matchesCreateEnvelope( return false; } const actualTarget = target as Record; - const authorityScope = actualTarget.authorityScope; - if ( - !authorityScope || - typeof authorityScope !== "object" || - Array.isArray(authorityScope) - ) { - return false; - } - const actualAuthority = authorityScope as Record; return ( hasExactKeys(input, [ "target", @@ -144,14 +168,10 @@ function matchesCreateEnvelope( : "propertyEntries", ]) && hasExactKeys(actualTarget, [ - "authorityScope", "spaceId", "databaseId", "databaseDocumentId", ]) && - hasExactKeys(actualAuthority, ["kind", "id"]) && - actualAuthority.kind === expected.target.authorityScope.kind && - actualAuthority.id === expected.target.authorityScope.id && actualTarget.spaceId === expected.target.spaceId && actualTarget.databaseId === expected.target.databaseId && actualTarget.databaseDocumentId === expected.target.databaseDocumentId && @@ -163,6 +183,7 @@ function matchesCreateEnvelope( function expectedPropertyValuesScorer( expected: Record, + expectedTypes: Record | undefined, expectedEnvelope?: ParityEvalScenario["expectedCreateEnvelope"], ) { return createScorer< @@ -195,6 +216,45 @@ function expectedPropertyValuesScorer( `expected exactly one row mutation, received ${mutationCalls.length}`, ); } + const orderedCalls = run.toolCalls; + const listIndex = orderedCalls.indexOf("list-content-databases"); + const inspectIndex = orderedCalls.indexOf("get-content-database"); + const createIndex = orderedCalls.indexOf("add-database-item"); + if ( + listIndex < 0 || + inspectIndex <= listIndex || + createIndex <= inspectIndex + ) { + invalid.push("database discovery did not precede the create mutation"); + } + const listCall = (run.toolCallDetails ?? []).find( + (call) => call.name === "list-content-databases", + ); + const inspectCall = (run.toolCallDetails ?? []).find( + (call) => call.name === "get-content-database", + ); + const listInput = listCall?.input as Record | undefined; + const inspectInput = inspectCall?.input as + | Record + | undefined; + if ( + expectedEnvelope && + (listInput?.title !== "PR #3314 feedback" || + inspectInput?.databaseId !== expectedEnvelope.target.databaseId) + ) { + invalid.push( + "discovery did not resolve the requested title to the exact create target", + ); + } + for (const [propertyId, expectedType] of Object.entries( + expectedTypes ?? {}, + )) { + if (analysis.receivedTypes[propertyId] !== expectedType) { + invalid.push( + `property ${propertyId} did not declare discovered type ${expectedType}`, + ); + } + } const createInput = createCalls[0]?.input; if ( expectedEnvelope && @@ -278,6 +338,7 @@ export function scenarioToEval(scenario: ParityEvalScenario): Eval { ? [ expectedPropertyValuesScorer( scenario.expectedPropertyValues, + scenario.expectedPropertyTypes, scenario.expectedCreateEnvelope, ), ] From 3097054346823e73370e7f7a7d92f47d79e14e01 Mon Sep 17 00:00:00 2001 From: "Builder.io" Date: Tue, 25 Aug 2026 13:02:02 +0000 Subject: [PATCH 07/28] fix: address review feedback on Content mutation parity - Exclude legacy authorityScope from the mutation payload digest so retries through old callers replay instead of colliding on IDEMPOTENCY_KEY_REUSED. - Reject empty propertyEntries arrays instead of silently canonicalizing them to a no-op mutation. - Check durable replay before typed property assertion validation so a retried committed mutation replays even if the property's type later changed. - Register list-content-databases and get-content-database fixtures in the dedicated property-preservation runner so discovery can succeed before the create call. --- .../actions/_database-property-input.ts | 1 + .../content/actions/_database-row-mutation.ts | 19 ++-- ...n-database-create-property-preservation.ts | 97 ++++++++++++++++++- 3 files changed, 110 insertions(+), 7 deletions(-) diff --git a/templates/content/actions/_database-property-input.ts b/templates/content/actions/_database-property-input.ts index 5e89b4ccbcc..4a9a6bf5485 100644 --- a/templates/content/actions/_database-property-input.ts +++ b/templates/content/actions/_database-property-input.ts @@ -133,6 +133,7 @@ export const databasePropertyValuesSchema = z export const databasePropertyEntriesSchema = z .array(databasePropertyEntrySchema) + .min(1) .max(1_000) .optional() .describe( diff --git a/templates/content/actions/_database-row-mutation.ts b/templates/content/actions/_database-row-mutation.ts index 5b8372030a2..33f07ab716b 100644 --- a/templates/content/actions/_database-row-mutation.ts +++ b/templates/content/actions/_database-row-mutation.ts @@ -741,9 +741,13 @@ export function databaseMutationPayloadDigest( | UpdateDatabaseRowMutationInput | UpsertDatabaseRowMutationInput, ) { - const { propertyTypeAssertions: _propertyTypeAssertions, ...canonicalInput } = - input; - return digest({ operation, ...canonicalInput }); + const { + propertyTypeAssertions: _propertyTypeAssertions, + target, + ...canonicalInput + } = input; + const { authorityScope: _authorityScope, ...stableTarget } = target ?? {}; + return digest({ operation, ...canonicalInput, target: stableTarget }); } function assertPropertyTypeAssertions( @@ -1187,7 +1191,6 @@ export async function createDatabaseRow( input: CreateDatabaseRowMutationInput, ): Promise { const initial = await loadContext(input.target, "editor"); - assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); const inputDigest = databaseMutationPayloadDigest("create", input); const replay = await replayReceipt( initial, @@ -1195,6 +1198,7 @@ export async function createDatabaseRow( inputDigest, ); if (replay) return replay; + assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); assertSchema(initial, input.expectedSchemaRevision); const values = await normalizePatch(initial, input.propertyValues); const result = await withMutationLocks(initial.database, () => @@ -1215,6 +1219,7 @@ export async function createDatabaseRow( tx as unknown as Db, ); if (lockedReplay) return lockedReplay; + assertPropertyTypeAssertions(locked, input.propertyTypeAssertions); assertSchema(locked, input.expectedSchemaRevision); await touchContentDatabase( tx as unknown as Db, @@ -1256,7 +1261,6 @@ export async function updateDatabaseRow( input: UpdateDatabaseRowMutationInput, ): Promise { const initial = await loadContext(input.target, "editor"); - assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); await assertAccess("document", input.documentId, "editor"); const inputDigest = databaseMutationPayloadDigest("update", input); const replay = await replayReceipt( @@ -1265,6 +1269,7 @@ export async function updateDatabaseRow( inputDigest, ); if (replay) return replay; + assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); assertSchema(initial, input.expectedSchemaRevision); const values = await normalizePatch(initial, input.propertyValues); const result = await withMutationLocks(initial.database, () => @@ -1285,6 +1290,7 @@ export async function updateDatabaseRow( tx as unknown as Db, ); if (lockedReplay) return lockedReplay; + assertPropertyTypeAssertions(locked, input.propertyTypeAssertions); assertSchema(locked, input.expectedSchemaRevision); const updated = await updateInsideTransaction( tx as unknown as Db, @@ -1337,7 +1343,6 @@ export async function upsertDatabaseRow( input: UpsertDatabaseRowMutationInput, ): Promise { const initial = await loadContext(input.target, "editor"); - assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); const replayKeyPropertyId = initial.database.naturalKeyPropertyId; const replayKeyDefinition = replayKeyPropertyId ? initial.definitions.find( @@ -1365,6 +1370,7 @@ export async function upsertDatabaseRow( inputDigest, ); if (replay) return replay; + assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); assertSchema(initial, input.expectedSchemaRevision); const keyPropertyId = initial.database.naturalKeyPropertyId; if (!keyPropertyId) { @@ -1425,6 +1431,7 @@ export async function upsertDatabaseRow( tx as unknown as Db, ); if (lockedReplay) return lockedReplay; + assertPropertyTypeAssertions(locked, input.propertyTypeAssertions); assertSchema(locked, input.expectedSchemaRevision); if (locked.database.naturalKeyPropertyId !== keyPropertyId) { conflict("SCHEMA_REVISION_CONFLICT", "The natural key changed."); diff --git a/templates/content/parity/run-database-create-property-preservation.ts b/templates/content/parity/run-database-create-property-preservation.ts index 1713a9fd52c..ac1f4bc7553 100644 --- a/templates/content/parity/run-database-create-property-preservation.ts +++ b/templates/content/parity/run-database-create-property-preservation.ts @@ -1,6 +1,8 @@ import { createAgentRunner, runEvals } from "@agent-native/core/eval"; import addDatabaseItem from "../actions/add-database-item.ts"; +import getContentDatabase from "../actions/get-content-database.ts"; +import listContentDatabases from "../actions/list-content-databases.ts"; import { parityEvalScenarios } from "./eval-scenarios.ts"; import { scenarioToEval } from "./scenario-to-eval.ts"; @@ -11,6 +13,12 @@ if (!scenario) { throw new Error("Missing database create property preservation scenario."); } +const FIXTURE_SPACE_ID = "fixture_personal_space"; +const FIXTURE_DATABASE_ID = "fixture_feedback_database"; +const FIXTURE_DOCUMENT_ID = "fixture_feedback_document"; +const FIXTURE_SCHEMA_REVISION = "fixture_schema_revision"; +const FIXTURE_DATABASE_TITLE = "PR #3314 feedback"; + const evalCase = scenarioToEval(scenario); evalCase.scorers = evalCase.scorers.filter( (scorer) => @@ -20,13 +28,100 @@ evalCase.scorers = evalCase.scorers.filter( const runner = await createAgentRunner({ actions: { + "list-content-databases": { + ...listContentDatabases, + run: async () => ({ + databases: [ + { + databaseId: FIXTURE_DATABASE_ID, + documentId: FIXTURE_DOCUMENT_ID, + spaceId: FIXTURE_SPACE_ID, + title: FIXTURE_DATABASE_TITLE, + description: "", + }, + ], + pagination: { + offset: 0, + limit: 50, + totalItems: 1, + returnedItems: 1, + hasMore: false, + nextOffset: null, + }, + }), + }, + "get-content-database": { + ...getContentDatabase, + run: async () => ({ + database: { + id: FIXTURE_DATABASE_ID, + documentId: FIXTURE_DOCUMENT_ID, + spaceId: FIXTURE_SPACE_ID, + title: FIXTURE_DATABASE_TITLE, + naturalKeyPropertyId: null, + viewConfig: { + activeViewId: "fixture_view", + views: [], + sorts: [], + filters: [], + columnWidths: {}, + }, + createdAt: new Date(0).toISOString(), + updatedAt: new Date(0).toISOString(), + }, + properties: [], + items: [], + source: null, + mutationContract: { + target: { + authorityScope: { + kind: "personal", + id: "fixture-owner@example.com", + }, + spaceId: FIXTURE_SPACE_ID, + databaseId: FIXTURE_DATABASE_ID, + databaseDocumentId: FIXTURE_DOCUMENT_ID, + }, + schemaRevision: FIXTURE_SCHEMA_REVISION, + naturalKeyPropertyId: null, + properties: [ + { + id: "fixture_status_property", + name: "Status", + type: "status", + writable: true, + sourceManaged: false, + acceptedShape: null, + options: { + options: [ + { + id: "status-cannot-verify", + name: "Cannot verify", + color: "gray", + }, + ], + }, + }, + { + id: "fixture_evidence_property", + name: "Evidence", + type: "text", + writable: true, + sourceManaged: false, + acceptedShape: null, + options: {}, + }, + ], + }, + }), + }, "add-database-item": { ...addDatabaseItem, run: async (input) => ({ fixtureOnly: true, received: input }), }, }, systemPrompt: - "You are Content's AI document assistant. Use the registered Content action and preserve exact user-supplied target constraints, property IDs, and property values. Never invent fields or claim an action succeeded when it failed.", + "You are Content's AI document assistant. Use the registered Content actions and preserve exact user-supplied target constraints, property IDs, and property values. Never invent fields or claim an action succeeded when it failed.", }); const report = await runEvals([evalCase], runner, { persist: false }); From 55eb6fd82eeb14a34da3c55b585deefe6c505cee Mon Sep 17 00:00:00 2001 From: "Builder.io" Date: Tue, 25 Aug 2026 13:26:23 +0000 Subject: [PATCH 08/28] fix: require successful discovery before accepting the property preservation create call expected_property_values now requires list-content-databases and get-content-database to complete without error and validates their result payloads (discovered database and mutation contract target/schema revision) match the fixture create target, instead of only checking call order and input shape. --- templates/content/parity/scenario-to-eval.ts | 66 ++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/templates/content/parity/scenario-to-eval.ts b/templates/content/parity/scenario-to-eval.ts index 39e985bb597..27ab3697c61 100644 --- a/templates/content/parity/scenario-to-eval.ts +++ b/templates/content/parity/scenario-to-eval.ts @@ -181,6 +181,18 @@ function matchesCreateEnvelope( ); } +function parseToolResultJson(result: string | undefined): unknown { + if (!result) return undefined; + try { + return JSON.parse(result); + } catch { + // coercion-ok: an unparseable tool result fails the same "did not supply + // the expected target/contract" scorer checks below as a missing one — + // both are non-passing evidence, not a distinction the eval needs to make. + return undefined; + } +} + function expectedPropertyValuesScorer( expected: Record, expectedTypes: Record | undefined, @@ -246,6 +258,60 @@ function expectedPropertyValuesScorer( "discovery did not resolve the requested title to the exact create target", ); } + if ( + !listCall?.completed || + listCall.isError || + !inspectCall?.completed || + inspectCall.isError + ) { + invalid.push("database discovery calls did not complete successfully"); + } + if (expectedEnvelope) { + const listResult = parseToolResultJson(listCall?.result) as + | { databases?: Array> } + | undefined; + const discoveredDatabase = listResult?.databases?.find( + (database) => + database.databaseId === expectedEnvelope.target.databaseId, + ); + const discoveredDocumentId = + discoveredDatabase?.documentId ?? + discoveredDatabase?.databaseDocumentId; + if ( + !discoveredDatabase || + discoveredDocumentId !== expectedEnvelope.target.databaseDocumentId || + discoveredDatabase.spaceId !== expectedEnvelope.target.spaceId + ) { + invalid.push( + "list-content-databases result did not supply the expected create target", + ); + } + const inspectResult = parseToolResultJson(inspectCall?.result) as + | { + mutationContract?: { + target?: Record; + schemaRevision?: string; + expectedSchemaRevision?: string; + }; + } + | undefined; + const contractTarget = inspectResult?.mutationContract?.target; + const contractSchemaRevision = + inspectResult?.mutationContract?.schemaRevision ?? + inspectResult?.mutationContract?.expectedSchemaRevision; + if ( + !contractTarget || + contractTarget.spaceId !== expectedEnvelope.target.spaceId || + contractTarget.databaseId !== expectedEnvelope.target.databaseId || + contractTarget.databaseDocumentId !== + expectedEnvelope.target.databaseDocumentId || + contractSchemaRevision !== expectedEnvelope.expectedSchemaRevision + ) { + invalid.push( + "get-content-database result did not supply the expected mutation contract", + ); + } + } for (const [propertyId, expectedType] of Object.entries( expectedTypes ?? {}, )) { From 9e760783396e9e69ff4ec588e6107fd768506cf3 Mon Sep 17 00:00:00 2001 From: Alice Moore <86723305+3mdistal@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:47:10 +0000 Subject: [PATCH 09/28] fix: preserve Content mutation replay compatibility --- packages/core/src/eval/agent-runner.ts | 6 ++ packages/core/src/eval/runner.spec.ts | 4 ++ packages/core/src/eval/types.ts | 2 + .../content/actions/_database-row-mutation.ts | 67 ++++++++++++++++--- .../database-row-property-input.test.ts | 33 ++++++++- .../__tests__/eval-scenario-coverage.test.ts | 18 ++++- templates/content/parity/scenario-to-eval.ts | 33 +++++++++ 7 files changed, 150 insertions(+), 13 deletions(-) diff --git a/packages/core/src/eval/agent-runner.ts b/packages/core/src/eval/agent-runner.ts index c986194a0e2..4527c6bb949 100644 --- a/packages/core/src/eval/agent-runner.ts +++ b/packages/core/src/eval/agent-runner.ts @@ -124,6 +124,8 @@ export async function createAgentRunner( name: string; id?: string; input: unknown; + startedAtEventIndex: number; + completedAtEventIndex?: number; completed?: boolean; completedSideEffect?: boolean; isError?: boolean; @@ -131,12 +133,14 @@ export async function createAgentRunner( }> = []; let ok = true; let error: string | undefined; + let eventIndex = 0; const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), timeoutMs); const started = Date.now(); const send = (event: AgentChatEvent): void => { + const currentEventIndex = eventIndex++; switch (event.type) { case "text": text += event.text; @@ -147,6 +151,7 @@ export async function createAgentRunner( name: event.tool, id: event.id, input: event.input, + startedAtEventIndex: currentEventIndex, }); break; case "tool_done": { @@ -157,6 +162,7 @@ export async function createAgentRunner( ); if (detail) { detail.completed = true; + detail.completedAtEventIndex = currentEventIndex; detail.completedSideEffect = event.completedSideEffect; detail.isError = event.isError === true; detail.result = event.result; diff --git a/packages/core/src/eval/runner.spec.ts b/packages/core/src/eval/runner.spec.ts index 389844d1dd1..b80f197ef81 100644 --- a/packages/core/src/eval/runner.spec.ts +++ b/packages/core/src/eval/runner.spec.ts @@ -328,6 +328,8 @@ describe("createAgentRunner over a mocked runAgentLoop (no real model)", () => { { name: "search", input: {}, + startedAtEventIndex: 1, + completedAtEventIndex: 2, completed: true, completedSideEffect: true, isError: false, @@ -336,6 +338,8 @@ describe("createAgentRunner over a mocked runAgentLoop (no real model)", () => { { name: "update", input: {}, + startedAtEventIndex: 3, + completedAtEventIndex: 4, completed: true, completedSideEffect: false, isError: false, diff --git a/packages/core/src/eval/types.ts b/packages/core/src/eval/types.ts index 81e32e03a86..dd597fb50e0 100644 --- a/packages/core/src/eval/types.ts +++ b/packages/core/src/eval/types.ts @@ -35,6 +35,8 @@ export interface AgentRunOutput { readonly toolCallDetails?: readonly { readonly name: string; readonly input: unknown; + readonly startedAtEventIndex?: number; + readonly completedAtEventIndex?: number; readonly completed?: boolean; readonly completedSideEffect?: boolean; readonly isError?: boolean; diff --git a/templates/content/actions/_database-row-mutation.ts b/templates/content/actions/_database-row-mutation.ts index 33f07ab716b..e9977486c32 100644 --- a/templates/content/actions/_database-row-mutation.ts +++ b/templates/content/actions/_database-row-mutation.ts @@ -750,6 +750,29 @@ export function databaseMutationPayloadDigest( return digest({ operation, ...canonicalInput, target: stableTarget }); } +export function legacyDatabaseMutationPayloadDigest( + operation: DatabaseRowMutationOperation, + input: + | CreateDatabaseRowMutationInput + | UpdateDatabaseRowMutationInput + | UpsertDatabaseRowMutationInput, + authorityScope = input.target.authorityScope, +) { + const { propertyTypeAssertions: _propertyTypeAssertions, ...legacyInput } = + input; + return digest({ + operation, + ...legacyInput, + target: { ...legacyInput.target, authorityScope }, + }); +} + +function authorityScopeForContext(context: MutationContext) { + return context.database.orgId + ? ({ kind: "organization", id: context.database.orgId } as const) + : ({ kind: "personal", id: context.database.ownerEmail } as const); +} + function assertPropertyTypeAssertions( context: MutationContext, assertions: Record | undefined, @@ -795,9 +818,7 @@ function resultForReceipt( }, ): ContentDatabaseRowMutationResult { const target = { - authorityScope: context.database.orgId - ? ({ kind: "organization", id: context.database.orgId } as const) - : ({ kind: "personal", id: context.database.ownerEmail } as const), + authorityScope: authorityScopeForContext(context), spaceId: context.database.spaceId!, databaseId: context.database.id, databaseDocumentId: context.database.documentId, @@ -844,7 +865,7 @@ function resultForReceipt( async function replayReceipt( context: MutationContext, idempotencyKey: string, - expectedPayloadDigest: string, + expectedPayloadDigests: readonly string[], db: Db = getDb(), ): Promise { const [stored] = await db @@ -863,7 +884,7 @@ async function replayReceipt( ), ); if (!stored) return null; - if (stored.payloadDigest !== expectedPayloadDigest) { + if (!expectedPayloadDigests.includes(stored.payloadDigest)) { conflict( "IDEMPOTENCY_KEY_REUSED", "This idempotency key was already used for a different row mutation.", @@ -1192,10 +1213,18 @@ export async function createDatabaseRow( ): Promise { const initial = await loadContext(input.target, "editor"); const inputDigest = databaseMutationPayloadDigest("create", input); + const replayDigests = [ + inputDigest, + legacyDatabaseMutationPayloadDigest( + "create", + input, + authorityScopeForContext(initial), + ), + ]; const replay = await replayReceipt( initial, input.idempotencyKey, - inputDigest, + replayDigests, ); if (replay) return replay; assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); @@ -1215,7 +1244,7 @@ export async function createDatabaseRow( const lockedReplay = await replayReceipt( locked, input.idempotencyKey, - inputDigest, + replayDigests, tx as unknown as Db, ); if (lockedReplay) return lockedReplay; @@ -1263,10 +1292,18 @@ export async function updateDatabaseRow( const initial = await loadContext(input.target, "editor"); await assertAccess("document", input.documentId, "editor"); const inputDigest = databaseMutationPayloadDigest("update", input); + const replayDigests = [ + inputDigest, + legacyDatabaseMutationPayloadDigest( + "update", + input, + authorityScopeForContext(initial), + ), + ]; const replay = await replayReceipt( initial, input.idempotencyKey, - inputDigest, + replayDigests, ); if (replay) return replay; assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); @@ -1286,7 +1323,7 @@ export async function updateDatabaseRow( const lockedReplay = await replayReceipt( locked, input.idempotencyKey, - inputDigest, + replayDigests, tx as unknown as Db, ); if (lockedReplay) return lockedReplay; @@ -1364,10 +1401,18 @@ export async function upsertDatabaseRow( ); } const inputDigest = databaseMutationPayloadDigest("upsert", input); + const replayDigests = [ + inputDigest, + legacyDatabaseMutationPayloadDigest( + "upsert", + input, + authorityScopeForContext(initial), + ), + ]; const replay = await replayReceipt( initial, input.idempotencyKey, - inputDigest, + replayDigests, ); if (replay) return replay; assertPropertyTypeAssertions(initial, input.propertyTypeAssertions); @@ -1427,7 +1472,7 @@ export async function upsertDatabaseRow( const lockedReplay = await replayReceipt( locked, input.idempotencyKey, - inputDigest, + replayDigests, tx as unknown as Db, ); if (lockedReplay) return lockedReplay; diff --git a/templates/content/parity/__tests__/database-row-property-input.test.ts b/templates/content/parity/__tests__/database-row-property-input.test.ts index 037dc942b93..63e2a41e337 100644 --- a/templates/content/parity/__tests__/database-row-property-input.test.ts +++ b/templates/content/parity/__tests__/database-row-property-input.test.ts @@ -5,7 +5,10 @@ import { databasePropertyEntriesSchema, normalizeDatabasePropertyInput, } from "../../actions/_database-property-input"; -import { databaseMutationPayloadDigest } from "../../actions/_database-row-mutation"; +import { + databaseMutationPayloadDigest, + legacyDatabaseMutationPayloadDigest, +} from "../../actions/_database-row-mutation"; import addDatabaseItem from "../../actions/add-database-item"; import updateDatabaseItem from "../../actions/update-database-item"; import upsertDatabaseItemByKey from "../../actions/upsert-database-item-by-key"; @@ -216,6 +219,34 @@ describe("database row property inputs", () => { ); }); + it("retains the authority-bearing legacy digest for receipt replay", () => { + const input = canonicalizeDatabasePropertyInput({ + idempotencyKey: "existing-receipt", + target: { + authorityScope: { kind: "personal", id: "owner@example.com" }, + spaceId: "space-id", + databaseId: "database-id", + databaseDocumentId: "database-document-id", + }, + propertyValues: { "status-id": "ready" }, + }); + + expect(legacyDatabaseMutationPayloadDigest("create", input)).not.toBe( + databaseMutationPayloadDigest("create", input), + ); + const withoutAuthoredAuthority = { + ...input, + target: { ...input.target, authorityScope: undefined }, + }; + expect( + legacyDatabaseMutationPayloadDigest( + "create", + withoutAuthoredAuthority, + input.target.authorityScope, + ), + ).toBe(legacyDatabaseMutationPayloadDigest("create", input)); + }); + it("includes __proto__ property values in the canonical digest", () => { const withPrototypeNamedProperty = canonicalizeDatabasePropertyInput({ idempotencyKey: "same-intent", diff --git a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts index 4e00f6e9bc6..9edfa373c3d 100644 --- a/templates/content/parity/__tests__/eval-scenario-coverage.test.ts +++ b/templates/content/parity/__tests__/eval-scenario-coverage.test.ts @@ -17,6 +17,8 @@ function successfulCreateCall( ...scenario.expectedCreateEnvelope, ...propertyInput, }, + startedAtEventIndex: 4, + completedAtEventIndex: 5, completed: true, completedSideEffect: true, isError: false, @@ -43,6 +45,8 @@ function successfulDiscoveryDetails( { name: "list-content-databases", input: { title: "PR #3314 feedback" }, + startedAtEventIndex: 0, + completedAtEventIndex: 1, completed: true, isError: false, result: JSON.stringify({ @@ -54,10 +58,22 @@ function successfulDiscoveryDetails( input: { databaseId: scenario.expectedCreateEnvelope?.target.databaseId, }, + startedAtEventIndex: 2, + completedAtEventIndex: 3, completed: true, isError: false, result: JSON.stringify({ - mutationContract: scenario.expectedCreateEnvelope, + mutationContract: { + ...scenario.expectedCreateEnvelope, + properties: Object.entries(scenario.expectedPropertyTypes ?? {}).map( + ([id, type]) => ({ + id, + type, + writable: true, + sourceManaged: false, + }), + ), + }, }), }, ]; diff --git a/templates/content/parity/scenario-to-eval.ts b/templates/content/parity/scenario-to-eval.ts index 27ab3697c61..12a4aa36c23 100644 --- a/templates/content/parity/scenario-to-eval.ts +++ b/templates/content/parity/scenario-to-eval.ts @@ -245,6 +245,7 @@ function expectedPropertyValuesScorer( const inspectCall = (run.toolCallDetails ?? []).find( (call) => call.name === "get-content-database", ); + const createCall = createCalls[0]; const listInput = listCall?.input as Record | undefined; const inspectInput = inspectCall?.input as | Record @@ -266,6 +267,18 @@ function expectedPropertyValuesScorer( ) { invalid.push("database discovery calls did not complete successfully"); } + if ( + listCall?.completedAtEventIndex === undefined || + inspectCall?.startedAtEventIndex === undefined || + listCall.completedAtEventIndex >= inspectCall.startedAtEventIndex || + inspectCall.completedAtEventIndex === undefined || + createCall?.startedAtEventIndex === undefined || + inspectCall.completedAtEventIndex >= createCall.startedAtEventIndex + ) { + invalid.push( + "database discovery results were not available before dependent calls started", + ); + } if (expectedEnvelope) { const listResult = parseToolResultJson(listCall?.result) as | { databases?: Array> } @@ -292,6 +305,7 @@ function expectedPropertyValuesScorer( target?: Record; schemaRevision?: string; expectedSchemaRevision?: string; + properties?: Array>; }; } | undefined; @@ -311,6 +325,25 @@ function expectedPropertyValuesScorer( "get-content-database result did not supply the expected mutation contract", ); } + const discoveredProperties = new Map( + (inspectResult?.mutationContract?.properties ?? []).map( + (property) => [property.id, property], + ), + ); + for (const [propertyId, expectedType] of Object.entries( + expectedTypes ?? {}, + )) { + const property = discoveredProperties.get(propertyId); + if ( + property?.type !== expectedType || + property.writable !== true || + property.sourceManaged === true + ) { + invalid.push( + `discovery did not supply writable property ${propertyId} with type ${expectedType}`, + ); + } + } } for (const [propertyId, expectedType] of Object.entries( expectedTypes ?? {}, From 7632a42ae45160e273535db1aef10e8af7d934fa Mon Sep 17 00:00:00 2001 From: Alice Moore <86723305+3mdistal@users.noreply.github.com> Date: Thu, 3 Sep 2026 23:42:05 +0000 Subject: [PATCH 10/28] docs(content): preserve planning and suggested-edits artifacts --- plans/shape-content-page-info-word-count.md | 278 ++++++++++++++++ ...pe-content-sidebar-human-qa-regressions.md | 188 +++++++++++ ...iable-feature-flag-directory-resolution.md | 309 ++++++++++++++++++ ...02-content-suggested-edits-parity-shape.md | 303 +++++++++++++++++ .../server/lib/suggested-edits.test.ts | 24 ++ .../content/server/lib/suggested-edits.ts | 81 +++++ .../content/server/plugins/feature-flags.ts | 5 + .../content/server/plugins/suggested-edits.ts | 13 + .../templates/content/shared/feature-flags.ts | 11 + 9 files changed, 1212 insertions(+) create mode 100644 plans/shape-content-page-info-word-count.md create mode 100644 plans/shape-content-sidebar-human-qa-regressions.md create mode 100644 plans/shape-reliable-feature-flag-directory-resolution.md create mode 100644 templates/content/docs/solutions/2026-09-02-content-suggested-edits-parity-shape.md create mode 100644 wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/lib/suggested-edits.test.ts create mode 100644 wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/lib/suggested-edits.ts create mode 100644 wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/plugins/feature-flags.ts create mode 100644 wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/plugins/suggested-edits.ts create mode 100644 wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/shared/feature-flags.ts diff --git a/plans/shape-content-page-info-word-count.md b/plans/shape-content-page-info-word-count.md new file mode 100644 index 00000000000..155bb271e1a --- /dev/null +++ b/plans/shape-content-page-info-word-count.md @@ -0,0 +1,278 @@ +# Content Page Info Blocks-field word counts + +## Summary + +Show the Page's content-bearing Blocks fields in the existing Page Info rail, with each field's word count as secondary information, and expose the same field-scoped calculation through a read-only `get-blocks-field-word-count` Action. + +This replaces the earlier design, which displayed one Page-level **Word count** row and exposed `get-document-word-count`. The roadmap makes the stronger boundary explicit: a Page owns one or more Blocks fields, and each Blocks field owns one editable rich-content body. Word count therefore describes a Blocks field, not the Page as a scalar property. + +Implementation must happen in a new dedicated Git worktree created from current `origin/main`. Shape may update only this governing artifact; product code, branches, and worktrees remain untouched until `/work` is invoked for this revision. + +## Context + +The Page Info control opens `DocumentInfoPanel`, which currently shows the description and, for database-member Pages, scalar Properties. `DocumentProperties` intentionally excludes Blocks properties because those fields render as editable body content rather than scalar rows. + +The Content roadmap defines the relevant boundary: + +- A Page owns stable identity, title, access, top-level Properties, and one or more Blocks fields. +- A Blocks field owns one editable rich-content body, stable field identity, and its own revision and recovery boundary. +- The primary `Content` field uses `documents.content`; additional Blocks fields use `document_block_field_contents`, keyed by Page and Property identity. + +Content already uses the shared `countWords()` helper for Blocks-field values. Its current lightweight, Markdown-aware semantics are the calculation contract for this slice. + +## Problem or opportunity + +A Page-level **Word count** row looks like another scalar Property and implies one canonical body. That becomes misleading when a Page contains multiple Blocks columns such as `Content`, `Research notes`, or `Draft introduction`. + +The Info rail can instead explain the Page's content structure: name each Blocks field and attach its count. Agents need the same field boundary so they can request one exact measurement without fetching rich content or guessing which body “the document” means. + +## Desired outcome + +- Page Info contains a compact **Content** section listing every Blocks field the current viewer may see. +- Each row uses the field's display name as the primary label and a localized word count as secondary information. +- A Page with only its primary field shows one compact `Content` row; Pages with additional Blocks columns show one row per field in schema order. +- Counts follow each field's current value. The actively edited field reflects local unsaved/debounced text without an extra request. +- `get-blocks-field-word-count` returns the count for one authorized field addressed by `documentId` plus optional `propertyId` (omitted for the primary field). +- Empty content returns numeric `0` from the Action and localized zero-word copy in the UI. +- Description, scalar Properties, Blocks-field editing, access behavior, source behavior, and existing Actions remain unchanged. + +## Options considered + +1. **One Page-level Word count and `get-document-word-count`.** Rejected because it resembles another Property and conflicts with the one-or-more-Blocks-fields model. +2. **One total across all Blocks fields.** Rejected because it hides which body contributes what and becomes ambiguous when field access differs. +3. **A Content section with per-field counts and a field-scoped Action.** Chosen because UI and agent vocabulary align with the roadmap. +4. **A plural `get-page-blocks-word-counts` Action.** Deferred as a possible convenience projection; the atomic contract should first address one exact field. + +## Recommended direction + +### UI + +Add a compact **Content** section to `DocumentInfoPanel`, before scalar Properties: + +```text +Content + +Content 1,284 words +Research notes 326 words +Draft introduction 91 words +``` + +`Content` is user-facing section vocabulary; `Blocks field` remains technical and Action vocabulary. Do not label the measurement as a standalone Property, add a total, introduce cards or a summary strip, or duplicate editable content. + +Use the same ordered Blocks definitions and values as the Page editor. The primary field's active count comes from current editor content. Any actively editable additional field must likewise use its editor-owned value rather than a stale snapshot. The UI must not call the Action merely to render Info. + +Respect field visibility and Page/database access before rendering a name or count. Local-file Pages retain their implicit primary Content field and do not gain database Properties. + +### Action + +Create a read-only `get-blocks-field-word-count` Action: + +```ts +{ documentId: string; propertyId?: string } +``` + +Omit `propertyId` for the primary Content field. Return unformatted data: + +```ts +{ + documentId: string; + propertyId: string | null; + name: string; + primary: boolean; + wordCount: number; +} +``` + +Resolve the exact field through existing Page/database membership and Blocks-property helpers. Enforce Page and field access before reading. For the primary field, use the live-editor flush handshake. For an additional field, use its field-scoped storage/freshness boundary and fail loudly if an exact fresh value cannot be established; never coerce unreadable or unavailable to zero. + +Reuse `countWords()` and return no formatted English string. Document how primary and additional fields are addressed in Content's agent instructions. + +### Feature checklist + +- UI: Page Info **Content** section with ordered per-field counts. +- Action: `get-blocks-field-word-count` over one exact authorized field. +- Instructions: document Action addressing in `templates/content/AGENTS.md`. +- Application state: no new key; existing current-Page and Info state identify the surface. +- Localization: add section/count copy across every configured Content locale. +- Changelog: record the visible Content improvement. + +## Constraints + +- Work in a new dedicated worktree; do not switch or modify the shared checkout beyond this shaping artifact. +- Keep Blocks fields separate from scalar Properties even when defined by database columns. +- Preserve `countWords()` semantics; improved tokenization is separate work. +- Exclude title, description, scalar Properties, comments, Discussion, inaccessible embedded text, and sibling fields from a field's count. +- Preserve access-before-computation; denied, missing, unavailable, and unreadable remain distinct from zero. +- Do not add a custom API route, schema change, dependency, feature flag, aggregate total, or application-state key. +- Update all configured locale catalogs and run changed-copy guards. + +## Risks and assumptions + +- Primary-field live-collaboration and additional-field freshness may use different seams. Work must trace and test both. +- Concurrent schema or membership changes must produce an honest conflict/not-found outcome, not a count for the wrong field. +- The shared counter is intentionally lightweight. Consistency matters more here than a new linguistic tokenizer. +- A future Page aggregate needs explicit access and aggregation semantics and is not implied by this feature. + +## Open questions + +None for the first slice. The user-facing heading is **Content**; the domain and Action term is **Blocks field**. + +## Architecture grounding and fit + +- **Demonstrated callers:** a Content reader opens Info to understand a Page's content fields; an authorized agent requests one exact field's word count. +- **Existing primitives:** `DocumentInfoPanel`; deliberate Blocks exclusion in `DocumentProperties`; current editor content; ordered Blocks definitions/values; additional-field storage; Blocks identity helpers; `countWords()`; `defineAction`; access resolution; primary live-editor flush. +- **Ownership boundaries:** the Page owns the field collection; each Blocks field owns one counted body; the active editor owns the freshest local projection; the shared helper owns count semantics; Actions own authorized external reads. +- **Legacy contracts:** Info description and scalar Properties, editing placement, Blocks storage/identity, local-file behavior, document and field Actions, permissions, and source behavior. +- **Shared vocabulary:** **Content** section, **Blocks field** domain object, `wordCount` result, `get-blocks-field-word-count` Action. +- **Smallest compatible delta:** one compact Info projection and one field-scoped read Action. +- **Deferred:** Page totals, plural batch Action, reading time, character/selection counts, generic statistics, new counting semantics, Comments, and Discussion. +- **Reversibility:** additive UI, Action, tests, instructions, and localized copy; no migration or persisted state. +- **Direct evidence:** roadmap Page/Blocks-field records, `DocumentInfoPanel.tsx`, `DocumentProperties.tsx`, `_property-utils.ts`, `_blocks-field-identity.ts`, `pull-document.ts`, `properties.ts`, and Blocks action tests. +- **Inference:** Work must confirm the exact editor seam for unsaved additional-field values. +- **Unresolved owner questions:** none. + +This is a local refinement of `content.author.document-editor` and `content.object.blocks-field`; it makes the roadmap contract visible rather than changing it. + +## Replacement acceptance story + +Successful-user story: while viewing or editing a Page, a person opens Info and sees each accessible content-bearing Blocks field by name with its immediately current word count; an authorized agent requests one exact field and receives the same field-only measurement without retrieving rich content. + +Required assertions: + +1. Shared tests cover empty, singular, plural, Markdown punctuation, fenced code, and exclusion of title and sibling fields. +2. Info renders localized **Content** rows in schema order for standalone, single-field database, and multiple-field database Pages without treating Blocks fields as scalar Properties. +3. Each count is field-only; primary and additional fields do not contribute to one another. +4. Editing the active primary or additional field updates its count without calling the Action, while sibling counts remain correct. +5. Hidden or inaccessible fields reveal neither name nor count; missing/unreadable fields are not rendered or returned as zero. +6. The Action addresses primary by `documentId` and additional by `documentId` plus `propertyId`, returns structured numeric data, rejects non-Blocks/inaccessible/deleted/mismatched fields, and uses the correct freshness boundary. +7. Registration/types, agent instructions, changelog, locales, focused tests, formatting, typecheck, i18n guards, and product-impact checks pass. +8. In the real editor at desktop and narrow width, one and several rows remain compact, long names do not crowd counts, and live edits change the expected row. + +Acceptance policy: + +- Modality: `real-interface` +- Independence: `preferred` +- Custody: `same-context-allowed` +- Interface: local Content editor at desktop and narrow viewport with one-field and multiple-field Pages +- Rationale: automated proof covers domain behavior; same-context interface proof proportionately verifies hierarchy, density, truncation, and live updates. + +## Material change record + +`WORK PAUSED — RETURNING TO SHAPE` was triggered because the outcome, governing architecture, Action vocabulary, and acceptance story changed. + +### Old fingerprint (`shape-v1`) + +- Outcome: one Page-body count shown as metadata. +- Architecture: the Page's canonical body is the counted unit. +- Action: `get-document-word-count --id `. +- Acceptance: one body-only count; additional Blocks fields deferred. + +### Approved replacement (`shape-v2`) + +- Outcome: Page Info lists every accessible Blocks field with its own count. +- Architecture: each Blocks field is counted; the Page is the container and Info projection. +- Action: `get-blocks-field-word-count`, addressed by Page plus optional Blocks-property identity. +- Acceptance: prove multi-field UI, field isolation, access, primary/additional freshness, and exact field-scoped Action behavior. + +Alice approved this direction with: “yeah, that makes sense. shape the revision.” This authorizes the artifact revision only; `/work` remains the implementation handoff. + +## Architecture fingerprint + +```yaml +authoritySchemaVersion: 3 +stage: shape +authority-source: "Alice approved the per-Blocks-field direction and requested: shape the revision." +authorized-scope: + repositories: [/home/teenylilmonkey/Developer/agent-native] + product-surfaces: [Content editor Page Info rail, Content Action surface] + outcome: Each accessible Blocks field is visible by name with its own count in Page Info and measurable through one field-scoped authorized Action. +allowed-mutations: [artifact-write] +write-targets: + artifacts: [plans/shape-content-page-info-word-count.md] +governing-artifact: + path: plans/shape-content-page-info-word-count.md + revision: shape-v2 +architecture-fingerprint: + outcome: Each accessible Blocks field is visible by name with its own count in Page Info and measurable through one field-scoped authorized Action. + shipping-surfaces: + - id: content-template + repository: /home/teenylilmonkey/Developer/agent-native + product-surface: Content editor Page Info rail and Content Action registry + constituency: Authorized Content readers, writers, and agents + durable-destination: Agent Native Content template on the dedicated task branch + integration-action: push + governing-architecture: Page Info projects ordered accessible Blocks fields while each field remains the independently counted and authorized body; UI uses editor-owned current values and the Action uses the appropriate field storage and freshness boundary. + acceptance-story: + id: content-page-info-blocks-field-word-count-v2 + summary: A person sees one current count per accessible Blocks field in Info, and an authorized agent receives the same field-only count for one exact field. + required-assertions: + - Field-only count semantics are tested. + - Info handles standalone, single-field, and multiple-field Pages without treating Blocks fields as scalar Properties. + - UI counts update from current primary/additional editor values with sibling isolation. + - Field names and counts obey access and unavailable-state boundaries. + - The Action resolves one exact field, uses its correct freshness boundary, and returns numeric output. + - Instructions, registration, changelog, locales, checks, and desktop/narrow real-interface acceptance pass. + acceptance-policy: + modality: real-interface + independence: preferred + custody: same-context-allowed + interface: Local Content editor at desktop and narrow width with one-field and multiple-field Pages + rationale: Automated proof covers domain behavior; same-context interface proof verifies hierarchy, density, truncation, and live updates. + risk-strategy: + kind: system-ready + production-validation-after-merge: false +architecture-grounding: + applicability: required + reason: A public Action must honor Page, Blocks-field, access, storage, and collaboration boundaries. + status: grounded + demonstrated-callers: + - Content reader opening Page Info + - Authorized agent requesting one exact Blocks field count + existing-primitives: + - DocumentInfoPanel and DocumentProperties + - editor-owned field values + - Blocks definitions, storage, and identity helpers + - shared countWords() + - defineAction, access resolution, and primary live-editor flush + ownership-boundaries: + - Page owns the field collection and Info projection + - Each Blocks field owns its counted body and revision boundary + - Active editor owns freshest local value + - Action boundary owns authorized external reads + legacy-contracts: + - Existing Info description and scalar Properties + - Existing Blocks editing, storage, identity, and visibility + - Existing document/Blocks Actions, local-file, and source behavior + shared-vocabulary: [Content section, Blocks field, get-blocks-field-word-count, wordCount] + smallest-compatible-delta: One per-field Info projection and one exact field-scoped Action over the existing counter. + deferred-capabilities: [Page aggregate, plural batch Action, generic text statistics, new count semantics, non-Page field owners] + reversibility: Additive code and copy only; no schema or persisted-state change. + direct-evidence: + - templates/content/docs/product/architecture.md + - templates/content/docs/product/capabilities/content.object.page.md + - templates/content/docs/product/capabilities/content.object.blocks-field.md + - templates/content/app/components/editor/DocumentInfoPanel.tsx + - templates/content/app/components/editor/DocumentProperties.tsx + - templates/content/actions/_property-utils.ts + - templates/content/actions/_blocks-field-identity.ts + - templates/content/actions/pull-document.ts + - templates/content/shared/properties.ts + inferences: [Work must confirm the editor seam for unsaved additional-field values.] + unresolved-owner-questions: [] +delegation-ceiling: [] +acceptance-state: + status: pending + summary: Revised Shape is complete; Work must implement and produce frozen automated and real-interface evidence in a new worktree. + blockers: [Work has not been invoked for shape-v2.] + last-land-packet: null +ledger-revision: content-page-info-blocks-field-word-count-shape-v2 +status: active +task-attention: shape-complete +execution-placement: + kind: local + worktree-required: true +``` + +## Next steps + +Invoke `/work plans/shape-content-page-info-word-count.md`. Work should first create the dedicated worktree from current `origin/main`, then implement and verify this exact `shape-v2` fingerprint without modifying or switching the shared checkout. diff --git a/plans/shape-content-sidebar-human-qa-regressions.md b/plans/shape-content-sidebar-human-qa-regressions.md new file mode 100644 index 00000000000..f580b16a420 --- /dev/null +++ b/plans/shape-content-sidebar-human-qa-regressions.md @@ -0,0 +1,188 @@ +# Shape: Fix Content sidebar human-QA regressions + +## Summary + +Ship one implementation PR for Agent-Native Content that makes sidebar interactions settle in the state the user just requested, at every supported viewport. The PR owns four confirmed sidebar regressions: closing search must restore the normal tree, creating a child must reveal it under its parent, mobile navigation must dismiss the drawer, and compact layouts must preserve usable content width. It also adds latency instrumentation and budgets for these interactions. + +The broader hosted cold-load investigation remains in the existing vault task, **Bound Content page-load latency across cold and cached loads**. This PR may remove a sidebar-owned request waterfall discovered while implementing these fixes, but it must not claim to solve whole-page DCL/FCP/load latency without separate root-cause evidence and hosted acceptance. + +## Context + +Human QA ran against Agent-Native Content revision `9e760783396e9e69ff4ec588e6107fd768506cf3`. Its durable evidence is linked from vault task **Fix Content sidebar human-QA regressions** (`f42d3f60-35c2-4cef-b715-ef01d8613881`) and the AI chat **QA Agent-Native Content Sidebar 2026-09-02**. The visual packet is at `C:/Users/emdis/.codex/visualizations/2026/09/02/content-sidebar-human-qa-v15-9e76078339`. + +The focused sidebar suite passed 103 tests across 11 files, showing that the current coverage does not exercise the failed settled states. Measured acknowledgement times were already fast for expand, search, and Settings; the defects are primarily correctness and perceived-readiness failures rather than slow click handlers. + +The repository instruction references a `content-product-development` skill that is absent from this checkout. This shape therefore uses the current Content code, tests, QA evidence, and the governing `frontend-design`, `native-navigation`, `client-side-routing`, `real-time-sync`, `performance`, and `verifying-changes` guidance. Restoring that missing product-contract skill is not part of this PR. + +## Problem + +The sidebar has several independent-looking symptoms caused by state transitions that do not complete atomically from the user's perspective: + +1. The search button only toggles `isSearching`; when it closes a populated search, `searchQuery` remains non-empty, so the hidden input leaves the sidebar trapped in the results branch. +2. Child creation navigates immediately, but neither optimistic creation nor the non-optimistic success path records the parent as expanded. The active-child ancestor derivation depends on list data catching up, so the destination can be active while absent from the visible tree. +3. Mobile drawer dismissal is passed through `onNavigate`, but some nested database-item paths return or navigate through lower-level link behavior without one single guaranteed close-at-navigation boundary. +4. `useIsMobile()` switches at the `md` boundary, so 768px becomes desktop and reserves the hard minimum 240px sidebar. The main database remains technically present but horizontally constrained/clipped. +5. Human QA also observed a `VisualEditor` render-phase update warning and repeated `list-document-properties` requests for a missing welcome-document ID. These are important diagnostics, but neither is yet proven to originate in the sidebar boundary. + +## Desired outcome + +A person can search, create and open nested content, and use the sidebar across mobile, compact, and desktop widths without corrective clicks, stale chrome, hidden destinations, or content-width breakage. Every sidebar interaction acknowledges immediately and reaches the correct settled state within a small, explicit budget. + +## Options considered + +### A. Patch each event handler independently + +Clear the query in the search button, expand a parent in child-create callbacks, add more `onNavigate` calls, and move the mobile breakpoint. This is quick but leaves navigation completion distributed across buttons, links, mutations, and database views; the same mobile and reveal defects can recur through an unpatched path. + +### B. Centralize sidebar transition helpers and keep existing primitives + +Introduce small app-owned helpers for `closeSearch`, `revealDocument`, and mobile navigation settlement. Continue using React Router links for pure navigation, imperative navigation only after mutations, React Query optimistic cache updates, the existing expansion state, and the existing Sheet. Make the compact responsive policy explicit in `Layout` rather than adding another sidebar implementation. + +This is the recommended direction because it fixes the boundary while preserving native navigation and the persistent app shell. + +### C. Redesign the sidebar or move its state into a new shared framework service + +This would exceed the demonstrated need, increase localization and compatibility surface, and delay correction of bounded regressions. There is no evidence that a new framework abstraction is required. + +## Recommended direction + +Implement option B as the smallest compatible delta: + +- Make search dismissal one operation that closes the input and clears the query. Escape, toggle-close, result selection, route changes, and mobile drawer dismissal must converge on that operation. +- Make child creation reveal its destination before navigation: add the parent to user-expanded state synchronously, inject the optimistic child where supported, and preserve the reveal across reconciliation. For non-optimistic sources, keep a short-lived pending reveal keyed by created ID until refreshed list data can derive the ancestry. +- Close the mobile Sheet from a route-settlement boundary owned by `Layout`, while retaining explicit `onNavigate` for post-mutation flows. Do not convert real links into buttons; normal click, Ctrl/Cmd-click, middle-click, and keyboard semantics must remain intact. A modified/new-tab click must not dismiss the current drawer. +- Treat compact portrait/tablet widths as overlay navigation rather than reserving 240px. Use one named layout media-query policy shared by rendering and tests; preserve the current persistent desktop rail above that range and the existing narrow-desktop agent-panel behavior. +- Add development/test timing markers around user input, route commit, drawer/search settlement, and destination readiness. Budgets are acceptance assertions, not permanent analytics or a new telemetry system. +- During implementation, trace the missing-document property request and the `VisualEditor` warning only far enough to classify them. Fix either in this PR only if the root cause is directly introduced by a sidebar transition and the focused fix stays within the named files/boundary. Otherwise attach reproducible evidence to the existing latency task or a separate editor task; do not bury unrelated changes in this PR. + +No feature flag is warranted: these are bounded corrections to already-shipped behavior, and the acceptance story exercises the real interface before merge. + +## Shipping surface + +- **Repository:** `BuilderIO/agent-native`, current branch at work handoff; no branch operation is implied by this shape. +- **Product surface:** Agent-Native Content authenticated app shell, document sidebar/tree, nested database navigation, and responsive layout. +- **Constituency:** Content users navigating personal/organization/local spaces on mouse, keyboard, touch, mobile, compact tablet, and desktop viewports. +- **Durable destination:** one reviewable GitHub implementation PR in `BuilderIO/agent-native`, plus the existing linked teenylilthoughts task retaining QA provenance and deferred hosted-load work. +- **Ordinary integration action:** reviewed PR merge to `main`; the normal beta publisher then deploys the Content template for post-merge smoke verification. No production promotion is shaped here. +- **Execution placement:** local. The change and focused real-interface verification do not require framework compute or a persistent remote workload. + +## Governing architecture + +- Keep `Layout` mounted once above route content; navigation must not remount the app shell. +- Preserve React Router `` behavior for pure navigation. Use imperative `navigate()` only for post-mutation redirects already owned by the sidebar workflow. +- Keep writes on existing Content actions and cache/state synchronization on React Query plus the existing action/sync substrate. Do not add REST routes, polling, or a second event stream. +- Keep transient search, drawer, and pending-reveal state in the UI. Persist only existing user expansion preferences through the current application-state mechanism. +- Keep a single `DocumentSidebar` rendering model. Responsive behavior changes its container mode, not its data or action contract. +- Prefer optimistic UI and immediate navigation; reconcile or roll back loudly on mutation failure. +- Make one layout breakpoint policy authoritative instead of combining an implicit toolkit mobile boundary with unrelated CSS visibility classes. + +## Architecture grounding and fit + +### Demonstrated caller and request + +The demonstrated caller is a Content user operating the left sidebar to close search, create/open a nested page or database, and navigate at 390px, 768px, and 1280px widths. The request is that each interaction feel instantaneous and leave the interface visibly settled. + +### Existing primitives and seams + +- `DocumentSidebar.tsx` owns `isSearching`, `searchQuery`, expansion state, optimistic document creation, and post-mutation navigation. +- `Layout.tsx` owns mobile Sheet state, the `onNavigate` close callback, sidebar width, and the mobile/desktop container decision. +- `DocumentTreeItem` and `ContentFilesSidebarView` render nested items and real navigation surfaces. +- React Query action hooks and existing query keys own cache reconciliation. +- `useDbSync`/action invalidation are the existing cross-writer freshness seam; they should not be expanded into broad invalidation. + +### Boundaries and legacy contracts + +- **Service/data owner:** existing Content actions and SQL-backed document model remain authoritative. +- **UI owner:** Content's `DocumentSidebar` and `Layout` own these transient transitions. +- **Vocabulary:** document, parent, active ancestor, expanded, search query, drawer, route settlement, destination readiness. +- **Unchanged contracts:** sidebar resize persistence on desktop; collapsed 48px rail; content-space selection; local-file-mode mutation behavior; DnD; favorites/trash; native link semantics; agent sidebar persistence; action names and payloads; localization catalogs unless new user-facing copy is introduced. + +### Smallest compatible delta + +Centralize three transition helpers, make the layout breakpoint explicit, and add behavioral tests/timing evidence. Do not create a shared package abstraction, new route, database schema, feature flag, or telemetry product. + +### Evidence classification + +- **Direct evidence:** the four settled-state failures and timings from the Human QA packet; current state ownership and handlers in `DocumentSidebar.tsx` and `Layout.tsx`; existing passing focused tests. +- **Inference to verify during Work:** closing on route settlement will cover every same-tab navigation path; active-ancestor lag is the reason the optimistic child remains hidden; overlay mode is the least disruptive 768px correction. +- **Unresolved but non-blocking:** root cause of the editor warning, missing welcome-document property calls, and whole-page cold-load latency. These do not change the sidebar's public contract. + +## Acceptance story + +Acceptance modality is **real-interface**. Independence is **preferred** and custody is **same-context-allowed** because the changes are reversible authenticated UI fixes with no destructive data contract; focused automated tests and proportional review still precede the browser pass. The final acceptance pass should ideally be performed by a context that did not implement the fixes, using the frozen script and original QA seed data. + +Every assertion below must pass on the implementation commit, with a screenshot or trace attached to the PR/task and no new console error or failed 4xx/5xx request attributable to the flow: + +1. **Search settlement:** at 1280×800, 768×1024, and 390×844, open search, enter a zero-result query, then close it with the search toggle and separately with Escape. The normal sidebar tree is visible, the query is cleared, reopening starts empty, and focus is sensible. +2. **Child reveal:** from a collapsed parent, create a child page and a child database. The route changes immediately, the parent is expanded before the destination editor/view becomes usable, and the active child is visible without a manual expand or refresh. Repeat for optimistic and non-optimistic source paths that are available in the fixture. +3. **Mobile drawer settlement:** at 390×844, normal-click a page, nested child, child database, favorite, Settings, and a search result. Same-tab navigation closes the drawer by route commit. Ctrl/Cmd-click or middle-click preserves native new-tab behavior and does not mutate the current drawer/route. +4. **Compact layout:** verify widths 767, 768, 834, 1024, 1099, and 1100px. Compact widths use overlay navigation and leave the primary database/editor usable without viewport-level horizontal clipping; desktop keeps the persistent resizable/collapsible sidebar. Crossing the boundary does not strand an open drawer or agent panel. +5. **Failure recovery:** force document/database creation failure. The optimistic child and route roll back, the prior sidebar state is restored, and the error is visible rather than represented as success. +6. **Performance budgets:** input acknowledgement (pressed/focus/optimistic route or drawer motion) occurs within 100ms; local settled UI for search close, expand, and drawer close occurs within 200ms; mutation-backed child creation shows its optimistic destination within 200ms. Record median and worst of five warm repetitions. Network persistence may finish later without blocking the optimistic state. Any regression beyond the budget blocks acceptance or requires an explicit return to Shape. +7. **Diagnostics:** confirm whether the tested flows emit the prior `VisualEditor` render-phase warning or request `list-document-properties` for an absent document. If reproduced and sidebar-caused, fix and retest in this PR. If not sidebar-caused, capture the route, request ID/document ID, timing, and console stack in the linked follow-up rather than claiming resolution. + +Automated proof must include focused component/state tests for search dismissal, reveal reconciliation/rollback, mobile route-close semantics, and the responsive boundary, followed by the Content typecheck and applicable formatting/i18n guards. The real-page pass must drive the exact flows above against the implementation commit; source-string layout assertions alone are insufficient. + +## Constraints + +- Do not change branches during shaping or infer branch authority for Work. +- Preserve unrelated worktree changes. +- Do not add user-facing copy unless necessary; if copy changes, update configured locales and run both i18n guards. +- Do not use a feature flag, new dependency, new API route, broad query invalidation, manual polling, or browser `alert`/`confirm`/`prompt`. +- Keep the PR reviewable as one sidebar/layout correction. Whole-page server cold-start work stays in its existing task unless direct evidence proves a tiny sidebar-owned cause. + +## Risks and assumptions + +- Closing the drawer on every location change could also close it after modified-click navigation or external state sync; route-settlement logic must distinguish current-tab commits. +- Persisting an automatically revealed parent as user intent may make it stay open longer than expected. The implementation should explicitly decide whether creation reveal is durable user expansion or transient pending reveal and test that contract. +- A wider overlay breakpoint changes tablet muscle memory. The acceptance widths freeze the intended behavior and prevent a one-pixel 767/768 split from returning accidentally. +- Timing in dev mode is noisy. Use browser performance marks for interaction budgets and retain the existing hosted latency task for DCL/FCP/load conclusions. +- QA seed data may include missing legacy documents. Diagnostics must distinguish fixture corruption from a product request waterfall. + +## Open questions + +No user decision blocks implementation. During Work, the implementer must answer from code and behavior whether creation reveal should become a persisted expansion or remain transient; choose the smallest behavior that keeps the newly created child visible while preserving the current rule that navigation-derived ancestor expansion does not permanently alter user preference. + +## Next steps + +1. Invoke Work against this exact artifact and re-read the current branch/diff before editing. +2. Add failing behavioral tests for the four confirmed regressions and failure rollback. +3. Implement the centralized transitions and explicit responsive policy using existing primitives. +4. Run focused tests, Content typecheck/format/required guards, then the complete real-interface acceptance script with performance marks and console/network inspection. +5. Attach evidence to the implementation PR and the linked vault task. Keep broader hosted-load results on the existing latency task. + +## Architecture fingerprint + +```yaml +authoritySchemaVersion: 3 +stage: shape +ledgerRevision: 1 +authoritySource: user request on 2026-09-03 to shape an implementation PR from the completed Content sidebar Human QA and triage evidence +governingArtifact: plans/shape-content-sidebar-human-qa-regressions.md +allowedMutations: + - artifact-write:plans/shape-content-sidebar-human-qa-regressions.md +outcome: Content sidebar interactions settle immediately and correctly across mobile, compact, and desktop layouts +shippingSurfaces: + - repository: BuilderIO/agent-native + productSurface: Agent-Native Content authenticated app shell and document sidebar + constituency: Content users across mouse, keyboard, touch, mobile, compact, and desktop + durableDestination: one reviewable GitHub implementation PR plus linked teenylilthoughts task evidence + integrationAction: reviewed merge to main followed by normal beta deployment and smoke verification +governingArchitecture: app-owned centralized sidebar transition helpers, existing React Router links and post-mutation navigation, existing action/React Query/sync substrate, one responsive DocumentSidebar with Layout-owned container policy +acceptance: + modality: real-interface + independence: preferred + custody: same-context-allowed + interface: Agent-Native Content browser UI at frozen mobile, compact, and desktop viewport widths + assertions: search settlement; child reveal; mobile drawer settlement with native link semantics; compact layout; rollback; interaction budgets; console and network diagnostics +riskStrategy: + rollout: no feature flag + rationale: bounded reversible fixes to shipped behavior with pre-merge real-interface acceptance +executionPlacement: local +architectureGrounding: required-and-complete +sourceRevision: 9e760783396e9e69ff4ec588e6107fd768506cf3 +deferredWork: + - whole-page hosted cold-load latency investigation + - editor warning unless proven sidebar-caused + - missing welcome-document property request unless proven sidebar-caused +``` diff --git a/plans/shape-reliable-feature-flag-directory-resolution.md b/plans/shape-reliable-feature-flag-directory-resolution.md new file mode 100644 index 00000000000..ec102a60dfa --- /dev/null +++ b/plans/shape-reliable-feature-flag-directory-resolution.md @@ -0,0 +1,309 @@ +# Reliable feature-flag target resolution + +## Answer + +The code fix should preserve Dispatch as the authority for app origins while making directory failure observable and retryable for Analytics control-plane mutations. + +Today `fetchOrgApps()` intentionally collapses every directory failure into `[]`. That is correct for best-effort cross-app discovery, whose legacy contract is silent local-only degradation, but it is the wrong primitive for a verified write. `setWorkspaceFeatureFlag()` calls that best-effort primitive in a new serverless invocation and treats an empty result as “target not found.” A transient directory error and a genuine absent target are therefore indistinguishable. The beta trace demonstrated exactly this boundary: a list request resolved Content and read its flag, while two later mutation requests failed in the `directory` phase before sending anything to Content. + +The smallest compatible delta is: + +1. Add a failure-aware Core directory lookup beside `fetchOrgApps()` (or an explicit strict result mode on the same underlying implementation). It should return a typed result that distinguishes `available` from `unavailable`, and retain the existing `fetchOrgApps(): OrgApp[]` wrapper unchanged for legacy best-effort callers. +2. For the failure-aware path, classify no configuration/auth, timeout/network, authorization, invalid response, and a successful empty directory separately enough that callers never mistake infrastructure failure for “no app.” Do not cache failed/empty infrastructure results for verified mutations. +3. In Analytics, use the failure-aware lookup in `resolveTargetApp()` and retry only retryable transport/unavailable outcomes with a small fixed budget (recommended: one retry with short jitter, inside the existing request timeout budget). A successful directory response that lacks `appId` remains a non-retryable directory/target-not-found failure. +4. Continue deriving the target origin exclusively from the server-authenticated directory response. Do not accept an origin or reusable target URL from the browser request. +5. Keep the already-landed mutation transaction unchanged after resolution: scoped A2A token, target write, structured persistence validation, independent read-back, and exact rollback verification. + +## Evidence + +- `packages/core/src/mcp/org-directory.ts` documents and implements `fetchOrgApps()` as returning `[]` on every error, including no token, non-2xx, timeout, parse failure, and unreachable directory. It also caches empty/failed results for ten seconds. +- `templates/analytics/server/lib/workspace-feature-flags.ts` uses `fetchOrgApps()` in `resolveTargetApp()` and maps both an empty failure result and a real missing app to the same `directory` phase. +- The same Analytics module separately uses `fetchOrgApps()` for fleet listing, so a successful list does not bind the next serverless mutation invocation to the same in-memory cache or runtime instance. +- Live beta evidence on 2026-08-24: Content and `content.a2a-receiver-ownership` were successfully listed as Off; two exact `set-workspace-feature-flag` POSTs later returned HTTP 503 with phase `directory`; neither request reached Content. This proves both intermittent directory resolution and the safe exact-error transport from PR #3313. + +## Architecture constraints + +- Demonstrated caller: authenticated Analytics operator clicks **Enable for me**, which invokes `set-workspace-feature-flag` for Content. +- Existing primitives: Core org-directory client, service-org A2A authentication, Dispatch `/_agent-native/org/apps`, Analytics action surface, and target-local `set-feature-flag` / `list-feature-flags` actions. +- Ownership boundaries: Dispatch owns the trusted organization app registry and app origins; Core owns directory transport/auth semantics; Analytics owns orchestration and operator-facing failure classification; Content owns flag persistence and evaluation. +- Legacy contract: ordinary `list_apps` / `ask_app` discovery must continue to degrade to `[]` without throwing when the directory is absent or unavailable. +- Security contract: the browser supplies only `appId`, flag key, and operation. It must not be able to select an arbitrary target origin. +- Smallest compatible delta: expose typed strict directory resolution from the same Core implementation and consume it only in verified Analytics management paths with a bounded retry. +- Deferred: durable distributed directory caches, browser-issued signed target handles, changing the Dispatch registry, or redesigning A2A authentication. +- Reversibility: the strict path is additive; the legacy wrapper and target mutation protocol remain intact. +- Unresolved owner questions: none. Current code and the live request trace establish the service boundary and compatibility requirement. + +## Frozen destination + +```yaml +authoritySchemaVersion: 3 +stage: shape +authority-source: "Alice: Okay, so what code fix in agent-native would be needed? $shape" +authorized-scope: + repositories: [BuilderIO/agent-native] + product-surfaces: [Analytics beta feature-flag management, Core org-directory client] + outcome: Verified feature-flag mutations tolerate a transient directory lookup failure without weakening trusted target resolution. +allowed-mutations: [artifact-write] +write-targets: + artifacts: [plans/shape-reliable-feature-flag-directory-resolution.md] +architecture-fingerprint: + outcome: Make trusted feature-flag target resolution failure-aware and retry transient directory failure once. + shipping-surfaces: + - id: agent-native-feature-flag-directory + repository: BuilderIO/agent-native + product-surface: Analytics feature-flag management and Core directory client + constituency: authenticated organization feature-flag operators + durable-destination: agent-native main and automatic beta deploy + integration-action: merge + governing-architecture: Dispatch remains the target-origin authority; Core exposes typed best-effort and strict directory semantics; Analytics retries only typed transient resolution failures before its existing verified target transaction. + acceptance-story: + id: reliable-feature-flag-directory-v1 + summary: An authenticated operator can enable the Content flag, see verified enabled read-back, turn it Off, and see verified Off read-back even when the first directory attempt transiently fails. + required-assertions: + - Existing best-effort fetchOrgApps callers still receive [] for unavailable or unconfigured directory state. + - Strict directory lookup distinguishes unavailable infrastructure from a successful response with no matching app. + - A mutation retries one typed transient directory failure and then sends exactly one target write. + - A successful directory response missing the requested app does not retry or call a target. + - Persistent directory failure sends no target write and preserves the directory failure phase. + - Browser input cannot provide or override the target origin. + - The target mutation remains scoped, permission-checked, persistence-validated, independently read back, and auditable. + - On deployed beta, enable-for-current-user reads back Enabled for you, then rollback reads back Off. + acceptance-policy: + modality: real-interface + independence: preferred + custody: same-context-allowed + interface: Authenticated in-app browser on beta Analytics Feature flags targeting Content. + rationale: Unit tests can prove retry and trust-boundary semantics; the deployed cross-app transaction must also be exercised through the real operator UI. Same-context evidence is allowed because the change is reversible, flag-scoped, and rollback is part of acceptance. + risk-strategy: + kind: system-ready + production-validation-after-merge: false +architecture-grounding: + applicability: required + status: grounded + demonstrated-callers: [Analytics set-workspace-feature-flag -> Content] + existing-primitives: [Core fetchOrgApps, Dispatch org apps endpoint, service-org A2A auth, Analytics verified flag transaction] + ownership-boundaries: [Dispatch registry authority, Core transport semantics, Analytics orchestration, Content persistence] + legacy-contracts: [Best-effort cross-app discovery silently degrades to an empty list] + shared-vocabulary: [best-effort directory lookup, strict directory lookup, unavailable, target-not-found] + smallest-compatible-delta: Add typed strict resolution beside the legacy wrapper and use one bounded retry in Analytics mutations. + deferred-capabilities: [distributed cache, signed browser target handles, directory redesign] + reversibility: Additive Core API and narrow Analytics consumer change. + direct-evidence: [packages/core/src/mcp/org-directory.ts, templates/analytics/server/lib/workspace-feature-flags.ts, 2026-08-24 beta network trace] + inferences: [The intermittent beta result is most likely a transient directory/auth/runtime failure; response status detail is currently erased by fetchOrgApps.] + unresolved-owner-questions: [] +acceptance-state: + status: pending + summary: Shape is complete; implementation and deployed beta enable/read-back/rollback evidence remain pending. + blockers: [] +ledger-revision: shape-reliable-feature-flag-directory-v1 +status: active +task-attention: shape-complete +``` + +## Recommendation + +Proceed to Work with the additive strict Core API plus the narrow Analytics retry. Do not solve this by trusting the list response's browser-visible origin, lengthening the empty-result cache, or retrying the target write itself. + +## Sources + +- `packages/core/src/mcp/org-directory.ts` +- `packages/core/src/mcp/org-directory.spec.ts` +- `templates/analytics/server/lib/workspace-feature-flags.ts` +- `templates/analytics/server/lib/workspace-feature-flags.spec.ts` +- Authenticated beta Analytics network trace, 2026-08-24 + +## Return to Shape — beta deployment boundary + +WORK PAUSED — RETURNING TO SHAPE + +Work produced exact commit `57ab7e53a54b1ab09bdf8b30e35f95e2dd5b8507`, with focused tests, typechecks, all guards, and independent technical review passing. It then established that `.github/workflows/deploy-beta-sites-prebuilt.yml` deploys only the mirrored `main` revision; `workflow_dispatch` also resolves `main`. No PR-head beta or deploy-preview surface exists. The frozen story requires the exact revision on beta before merge while `production-validation-after-merge` is false, so its ordering is impossible without changing the risk strategy or acceptance interface. + +### Options + +1. **Recommended — feature-flagged post-merge beta validation.** Add a new Analytics-owned default-off flag around strict directory retry. Before merge, prove it Off on beta. After merge and automatic beta deployment, enable it only for the designated operator through Analytics' local `set-feature-flag` action, execute enable/read-back/rollback against Content, then turn the rollout flag Off again. This preserves the real-interface story and makes post-merge validation reversible. +2. Replace beta acceptance with a local real-interface integration environment. This can prove the browser/action flow before merge but is weaker evidence for the deployed Dispatch/Analytics/Content boundary that actually failed. +3. Add a PR-head beta deployment mechanism. This expands repository deployment architecture and is disproportionate to this bounded fix. + +### Old fingerprint + +- Governing architecture: strict Core lookup plus bounded Analytics retry, with no rollout gate around the retry. +- Acceptance: exact revision must pass authenticated beta enable/read-back/rollback before merge. +- Risk: `system-ready`, `production-validation-after-merge: false`. + +### Proposed replacement fingerprint + +- Outcome and shipping surface: unchanged. +- Governing architecture: Dispatch remains authoritative; Core exposes typed strict results; Analytics retries only timeout, network, rate-limit, and server failures, gated by a new Analytics-owned default-off rollout flag. +- Acceptance: focused contracts, typechecks, guards, independent review, and proof that the new retry flag is Off on beta permit code-ready merge; after automatic beta deployment, target the designated operator, run Content enable/read-back/rollback, then return both the Content flag and retry rollout flag to Off. +- Risk: `feature-flagged`, `production-validation-after-merge: true`. + +Replacement acceptance story: + +1. Legacy `fetchOrgApps()` preserves empty-list degradation. +2. Strict lookup distinguishes unavailable infrastructure from successful empty results. +3. Only timeout, network, HTTP 429, and HTTP 5xx receive one pre-write retry. +4. Missing target and every non-transient failure make zero target calls. +5. Browser input cannot override the directory-derived origin. +6. The target transaction remains scoped, persistence-validated, independently read back, and auditable. +7. The new Analytics retry rollout flag is proven Off on beta before merge. +8. After automatic beta deployment, enable the retry rollout only for the designated operator. +9. Through authenticated beta Analytics, enable `content.a2a-receiver-ownership` and read back **Enabled for you**. +10. Roll back `content.a2a-receiver-ownership` and read back **Off**. +11. Return the Analytics retry rollout flag to Off and verify it is Off. + +Lifecycle state: WORK PAUSED — RETURNING TO SHAPE +Authority: Shape, proposed ledger `shape-reliable-feature-flag-directory-v2`; implementation mutations invalidated pending Alice's approval of the replacement fingerprint and acceptance story. +Task attention: alice-decision; title preserved +Next: approve the recommended feature-flagged replacement, then `/work plans/shape-reliable-feature-flag-directory-resolution.md` + +### Approval + +Alice approved the recommended feature-flagged replacement in the calling task on 2026-08-24. Work authority resumes against `shape-reliable-feature-flag-directory-v2`; the replacement fingerprint and assertions above are authoritative. + +## Return to Shape — reliable directory service + +WORK PAUSED — RETURNING TO SHAPE + +The v2 implementation made directory failures visible and gave Analytics one safe pre-write retry. Live beta investigation then established that this is only a caller-side mitigation: the authenticated Dispatch directory request itself performs variable-cost, uncached discovery behind Core's hard four-second deadline. The same deployed Analytics session alternated between a complete directory and `directoryStatus: unavailable` without a deployment or credential change. Dispatch health and unauthenticated route checks remained fast, isolating the unstable work to the authenticated org resolution and discovery path. + +The broader fix should keep PR #3513's typed Core result, success-only caching, transient classification, Analytics rollout flag, and one pre-write retry. It should also make the Dispatch directory a bounded service rather than relying on the generic best-effort discovery path. + +### Material delta + +- **Old outcome:** verified feature-flag mutations survive one transient directory failure. +- **Proposed outcome:** authenticated organization directory reads are bounded and observable, while verified feature-flag mutations still survive one genuinely transient failure. +- **Old architecture:** generic `discoverAgents()` remains unchanged; Core and Analytics classify and retry its failures. +- **Proposed architecture:** Core owns a strict, purpose-built directory discovery primitive; Dispatch uses it for `/_agent-native/org/apps`; the existing best-effort `discoverAgents()` and `fetchOrgApps()` contracts remain unchanged for legacy callers; PR #3513 remains the strict client and orchestration layer. +- **Old acceptance:** prove retry classification plus one deployed beta mutation transaction. +- **Replacement acceptance:** additionally prove bounded query/latency behavior, exact directory membership semantics, honest server failure, authenticated endpoint behavior, and repeated real beta reads before the mutation transaction. +- **Risk strategy:** remains feature-flagged for Analytics retry behavior with post-merge beta validation. The Dispatch data-access correction is an always-on compatibility fix and must pass exact contract and performance assertions before merge. + +### Options + +1. **Recommended — strict batched directory discovery plus single-flight success caching.** Add a Core directory-specific read that obtains remote-agent manifests through one projected, portable store query, resolves independent workspace metadata concurrently, and returns a typed failure rather than silently dropping a failed layer. Dispatch coalesces concurrent refreshes and caches only complete successful results per organization for the existing 60-second freshness window. This removes the four sequential list calls and per-resource read waterfall while preserving one source of truth. +2. Parallelize the existing `resourceList()` and `resourceGet()` loops inside `discoverAgents()`. This is smaller, but query count still grows with the number of manifests, changes a broad best-effort primitive for every consumer, and still cannot distinguish a complete directory from a partial fallback. +3. Increase the four-second timeout and rely on PR #3513's retry. This reduces observed failures without bounding the work or revealing why the endpoint failed. It leaves the outage mechanism intact and is not a root fix. +4. Persist a separate durable directory snapshot. This can provide strong stale-while-error behavior, but introduces synchronization, invalidation, and removal semantics that are disproportionate to the demonstrated caller. Defer it unless the batched strict read cannot meet the measured service budget. + +### Recommended service contract + +1. Core adds a directory-specific strict discovery result beside the generic best-effort agent discovery API. +2. Its successful result is complete: built-in templates, eligible remote-agent resource overrides, and sibling workspace apps retain the same precedence, URL normalization, hidden-template filtering, self-filtering, and organization scope as today's Dispatch response. +3. Remote manifests are read in a bounded number of database round trips through a projected resource-store helper. Query count must not grow per manifest. +4. Independent static/workspace inputs are resolved concurrently where safe. No unbounded `Promise.all` over individual database reads replaces the current serial waterfall. +5. Failure to read an authoritative dynamic layer is a typed failure, not a successful partial directory. Dispatch returns a non-2xx response and records a structured, secret-free failure category and stage duration. +6. Dispatch coalesces simultaneous refreshes and caches only complete successful results per organization for at most 60 seconds. It does not cache failures or serve an older directory after a failed refresh; a withdrawn app must not be silently resurrected. +7. Core's strict HTTP client preserves the typed result and success-only cache from PR #3513. The existing `fetchOrgApps(): OrgApp[]` wrapper continues silent empty-list degradation for legacy cross-app discovery. +8. Analytics retains the default-off `analytics.resilient-fleet-flag-directory` gate and makes at most one retry for typed timeout, network, rate-limit, or server failures before any target write. Missing target, authorization, configuration, and invalid-response failures do not retry. +9. The four-second client deadline remains initially. Work may change it only from measured local/preview evidence and must record the resulting directory service-level objective in tests and code; merely lengthening it is not acceptance. +10. The beta deployment workflow gains an authenticated directory acceptance check or an equivalent trusted fixture-backed probe. Existing `/` and `/_agent-native/health` smoke checks remain but are not directory evidence. + +### Architecture grounding and fit + +- **Demonstrated caller:** authenticated Analytics `list-workspace-feature-flags` and `set-workspace-feature-flag` call Dispatch `GET /_agent-native/org/apps` for the operator's organization. +- **Existing primitives:** static `getBuiltinAgents()`, generic `discoverAgents()`, resource store, workspace-app manifest and metadata, Dispatch A2A verification and same-org resolution, Core `fetchOrgApps()`, PR #3513's strict result, and Analytics' verified target transaction. +- **Ownership boundaries:** Dispatch owns authenticated organization membership and trusted app origins; Core agent discovery owns registry composition and resource-store access; Core org-directory transport owns timeout/cache/error semantics; Analytics owns fleet orchestration and operator-facing retry; target apps own flag authorization and persistence. +- **Legacy contracts:** generic discovery remains best-effort; hidden templates stay hidden; resource and workspace overrides preserve precedence; directory callers cannot choose target origins; A2A and same-org checks remain unchanged; target writes remain exactly-once, scoped, audited, persistence-validated, and independently read back. +- **Smallest compatible delta:** introduce a strict batched sibling beside generic discovery, use it only in Dispatch's authenticated directory, and retain PR #3513 above it. +- **Deferred capabilities:** durable distributed snapshots, stale-while-error serving, registry schema changes, browser-issued target handles, generalized service discovery, and deployment-system migration repair. +- **Reversibility:** the new path is isolated to one authenticated endpoint; the old discovery and client wrappers remain available; the Analytics behavior stays behind its default-off rollout flag. +- **Direct evidence:** `packages/core/src/server/agent-discovery.ts` performs four sequential list reads and one `resourceGet()` per manifest; `templates/dispatch/server/plugins/org-apps-directory.ts` runs it inside the authenticated request; `packages/core/src/mcp/org-directory.ts` enforces a four-second deadline and historically collapsed all failures to `[]`; authenticated beta alternated between unavailable and complete without configuration drift; Dispatch health and unauthenticated auth rejection remained sub-second; beta deploy smoke does not exercise the authenticated directory route. +- **Inference:** the variable authenticated discovery work is the strongest supported outage mechanism. Existing telemetry cannot identify the exact slow database operation because the current client erases status and timing. +- **Unresolved owner questions:** none. The proposed boundary retains current service and datastore ownership without changing a public protocol. + +### Replacement acceptance story + +An authenticated organization operator can repeatedly load the complete feature-flag fleet and perform a verified, reversible Content flag mutation even when one directory transport attempt fails, without Analytics trusting browser-provided origins or Dispatch returning partial membership as success. + +Required assertions: + +1. Generic `discoverAgents()` and `fetchOrgApps()` preserve their best-effort fallback behavior for existing callers. +2. Strict directory discovery returns the same built-in, remote override, workspace app, hidden-template, precedence, URL, org-scope, and self-filter results as the existing successful endpoint. +3. Strict discovery uses a constant bounded number of database round trips as remote-manifest count grows and does not perform an N+1 resource-content waterfall. +4. Under a representative cold fixture and injected database latency, authenticated directory work completes inside the documented service budget and the Core four-second deadline. +5. An authoritative layer failure produces a typed non-2xx directory failure with secret-free stage/duration telemetry; it never returns a successful partial or empty directory. +6. Concurrent same-org requests share one in-flight refresh; only complete success is cached; failure is not cached; organization cache keys do not cross tenants. +7. A withdrawn app does not reappear after the success-cache freshness window, including when the next refresh fails. +8. PR #3513's strict client distinguishes configuration, authentication, timeout/network, rate-limit, server, invalid-response, successful-empty, and target-not-found outcomes while its legacy wrapper still returns `[]`. +9. Analytics retries exactly once only for the frozen transient classes and performs exactly one target write after recovery; persistent or non-transient failure performs zero target writes. +10. Browser input cannot provide or override a target origin, and all A2A audience, organization, scope, permission, audit, persistence-validation, read-back, and rollback contracts remain intact. +11. Repository tests, typechecks, guards, performance-contract tests, and independent technical review pass on the exact PR head. +12. Before merge, the Analytics retry rollout flag is proven Off on beta. +13. After automatic beta deployment, an authenticated operator performs ten consecutive fleet reloads without a directory outage, then enables `content.a2a-receiver-ownership`, reads back **Enabled for you**, rolls it back, and reads back **Off**. +14. After acceptance, `analytics.resilient-fleet-flag-directory` is returned to Off and verified Off. + +Acceptance policy: + +- **Modality:** real-interface, backed by automated contract and performance evidence. +- **Independence:** preferred. +- **Custody:** same-context-allowed. +- **Interface:** authenticated beta Analytics Feature flags, targeting the beta Dispatch directory and beta Content flag actions; automated tests use deterministic strict-directory fixtures with injected latency and failures. +- **Rationale:** query shape, classification, and exactly-once semantics are deterministic and belong in automated tests. The deployed cross-service behavior still requires the real operator UI. Same-context custody is proportionate because both flags are reversible and rollback is mandatory; an independent technical reviewer should inspect the shared Core/Dispatch boundary. + +### Proposed frozen destination + +```yaml +authoritySchemaVersion: 3 +stage: shape +authority-source: "Alice: $shape a fix for the broader issue. Keep this existing fix as part of it, too, as needed." +authorized-scope: + repositories: [BuilderIO/agent-native] + product-surfaces: [Dispatch authenticated organization app directory, Core directory discovery and transport, Analytics feature-flag fleet management] + outcome: Make authenticated organization directory reads bounded and observable while retaining safe transient recovery for verified feature-flag operations. +allowed-mutations: [artifact-write] +write-targets: + artifacts: [plans/shape-reliable-feature-flag-directory-resolution.md] +architecture-fingerprint: + outcome: Make the Dispatch organization directory a bounded, failure-honest service and retain PR 3513's typed strict lookup and one pre-write Analytics retry. + shipping-surfaces: + - id: agent-native-org-directory-reliability + repository: BuilderIO/agent-native + product-surface: Core and Dispatch organization directory plus Analytics feature-flag fleet management + constituency: source-blind Agent Native developers and authenticated organization feature-flag operators + durable-destination: BuilderIO/agent-native main, automatic beta deployment, and PR 3513 + integration-action: merge + governing-architecture: Core composes the authoritative directory through a strict bounded batched read while preserving generic best-effort discovery; Dispatch authenticates, scopes, coalesces, and exposes that result; Core transport classifies it; Analytics alone decides bounded pre-write retry. + acceptance-story: + id: reliable-org-directory-v3 + summary: Repeated authenticated fleet reads return complete directory membership, and a verified reversible Content flag mutation survives one transient pre-write directory failure without weakening origin trust or returning partial membership as success. + required-assertions: [legacy compatibility, exact membership parity, bounded query count, latency budget, failure honesty and telemetry, tenant-safe single-flight success cache, withdrawal freshness, typed client classification, bounded pre-write retry, security and verified transaction parity, exact-head checks and review, beta rollout flag Off before merge, ten beta reloads plus enable/read-back/rollback after deploy, rollout flag Off after acceptance] + acceptance-policy: + modality: real-interface + independence: preferred + custody: same-context-allowed + interface: Authenticated beta Analytics Feature flags plus deterministic strict-directory latency/failure fixtures. + rationale: Automated evidence proves bounded data access and failure semantics; the deployed cross-service story must also pass through the real operator surface. Independent technical review is preferred for the shared Core/Dispatch seam. + risk-strategy: + kind: feature-flagged + production-validation-after-merge: true +architecture-grounding: + applicability: required + reason: This changes a shared cross-service registry and authenticated platform endpoint. + status: grounded + demonstrated-callers: [Analytics list-workspace-feature-flags, Analytics set-workspace-feature-flag] + existing-primitives: [getBuiltinAgents, discoverAgents, resource store, workspace app manifest and metadata, Dispatch org apps endpoint, A2A auth, strict org-directory result from PR 3513] + ownership-boundaries: [Core composes registry data, Dispatch owns authenticated org authority, Core transport owns failure semantics, Analytics owns retry orchestration, target apps own flag state] + legacy-contracts: [best-effort generic discovery, exact membership precedence, hidden-template filtering, directory-derived origins, same-org A2A authorization, verified target transaction] + shared-vocabulary: [best-effort agent discovery, strict organization directory discovery, complete success, authoritative layer failure, transient pre-write retry] + smallest-compatible-delta: Add a strict batched directory sibling in Core, consume it only in Dispatch, and retain PR 3513's strict transport and Analytics retry. + deferred-capabilities: [durable distributed directory snapshot, stale-while-error, registry schema, generalized service discovery, deployment migration repair] + reversibility: One endpoint opts into the new strict path; legacy paths remain; Analytics retry remains default-off. + direct-evidence: [agent-discovery.ts query waterfall, org-apps-directory.ts request path, org-directory.ts four-second deadline and failure collapse, authenticated beta alternation, sub-second health/auth checks, beta smoke coverage gap] + inferences: [authenticated dynamic discovery is the dominant timeout source; exact slow query awaits new telemetry] + unresolved-owner-questions: [] +acceptance-state: + status: pending + summary: WORK PAUSED — RETURNING TO SHAPE; the broader fingerprint and replacement acceptance story await Alice's approval. + blockers: [Alice approval of reliable-org-directory-v3] +ledger-revision: shape-reliable-org-directory-v3-proposed +status: return-to-shape +task-attention: return-to-shape +``` + +### Recommendation + +Approve v3 and resume Work on PR #3513. Keep the existing exact-head changes, add the strict batched Dispatch directory path and its performance/failure evidence, update the PR description to explain the root issue, and keep the PR in draft until the replacement acceptance packet is complete. + +### V3 approval + +Alice approved `reliable-org-directory-v3` and invoked Work in the calling task on 2026-08-24. Work authority resumes on existing draft PR #3513 against ledger `work-reliable-org-directory-v3-r1`. The schema-v3 acceptance policy is reconciled as written: real-interface modality, preferred independence, and same-context-allowed custody. diff --git a/templates/content/docs/solutions/2026-09-02-content-suggested-edits-parity-shape.md b/templates/content/docs/solutions/2026-09-02-content-suggested-edits-parity-shape.md new file mode 100644 index 00000000000..975dfed9711 --- /dev/null +++ b/templates/content/docs/solutions/2026-09-02-content-suggested-edits-parity-shape.md @@ -0,0 +1,303 @@ +--- +title: "Content Suggested Edits parity shape" +date: 2026-09-02 +status: shape-complete +authoritySchemaVersion: 3 +ledgerRevision: content-suggested-edits-shape-r1 +governingArtifactRevision: content-suggested-edits-shape-r1 +--- + +# Content Suggested Edits parity + +## Summary + +Add **Suggested Edits** to Agent Native Content with behavioral parity to Notion's observed feature: a commenter, editor, admin, owner, or authorized agent can propose page-body text changes without changing the canonical page; reviewers inspect each proposal in place, discuss it, and accept or reject it durably. The first shipped slice deliberately matches Notion's narrow page-body boundary rather than prematurely implementing Content's broader typed-diff roadmap. + +The implementation should extend Core's existing review domain with executable suggestions and let Content register the document-specific operation and renderer adapter. Content owns page semantics, supported blocks, editor mode, and canonical mutation. Core owns proposal identity, thread/disposition lifecycle, permissions integration, notifications, audit/history seams, and shared actions. Suggestions are not comments with overloaded metadata, recovery versions, raw Yjs updates, or draft copies of pages. + +## Human problem and stakes + +Today, a Content collaborator can either comment without showing the exact desired edit or directly edit the canonical document. Agents face the same binary choice. Reviewers must translate prose feedback into changes or trust a direct rewrite after the fact. + +After this feature, a collaborator can make the intended revision directly in the familiar editor while the canonical document stays unchanged. The owner can understand the proposed result in context, discuss it, and make an attributable accept/reject decision. This matters most for agent-assisted editing and comment-level collaborators: both can contribute exact changes without receiving direct-edit authority. + +## Exact parity boundary + +“Parity” means parity with the firsthand Notion behavior observed on 2026-09-01, not parity with every future capability in Content's `Review changes in place` roadmap. + +| Behavior | Content contract | +| --- | --- | +| Enter mode | `•••` page menu exposes **Suggest edits**; active mode is labeled **Suggesting** in the page header. | +| Exit mode | Header control or page menu stops suggesting; pending proposals remain untouched. | +| Permissions | Owner/admin/editor can edit and suggest. Commenter can suggest and comment but cannot directly edit. Viewer remains read-only. Server actions enforce the same matrix as the UI and agent. | +| Add | Inserted text is provisional and stored as an Add operation. | +| Delete | Deleted text remains represented until acceptance and is stored as a Delete operation. | +| Replace | Replacing a selected range is one atomic Replace operation with exact before/after text. | +| Formatting | Supported inline mark changes are typed operations, initially Bold, Italic, Underline, Strikethrough, Code, and Link add/remove/change where the renderer can preserve exact material. | +| New text block | A new supported block is one proposal, even if its internal operation contains a boundary plus content insertion. | +| Review | Each suggestion has author, timestamp, typed summary, Accept, Reject, reactions, replies, and a more menu. Decisions apply immediately and idempotently. | +| History | Pending, accepted, and rejected suggestions remain visible in **All discussions**. Resolution removes decision controls but not the thread. | +| Thread tools | Mark unread, copy deep link, and mute replies are available for suggestion threads. | +| Notifications | Page owner/relevant participants receive the existing durable notification flow for new suggestions, replies, mentions, reactions where policy calls for it, and dispositions. | +| Agent parity | Agents create the same suggestion objects through shared actions; they do not directly mutate canonical content while operating in suggestion mode. | +| Locking | A locked/read-only page disables suggestion creation even if the role would otherwise permit it. Existing proposals remain readable according to access. | +| Scope exclusions | No title, icon, cover, database property, inline database, media/embed, local-file, source-owned, or peek/preview suggestion editing in the first release. Unsupported content remains read-only in suggesting mode. | +| Bulk decisions | No Accept all / Reject all in parity v1. Content's future filtered-review capability remains separate. | + +One intentional correction to Notion's observed UI: a block badge reports suggestions and replies separately instead of inflating “suggestions” when someone replies. + +## Architecture grounding and fit + +### Demonstrated caller + +The demonstrated caller is an authenticated Content user or accountable agent opening a canonical Content Page and requesting “Suggest edits,” then proposing a supported body change for later review. + +### Existing primitives and seams + +- `templates/content/app/components/editor/VisualEditor.tsx` is the TipTap/ProseMirror editing surface and already participates in Yjs collaboration. +- `templates/content/server/plugins/collab.ts` and Core's collaboration substrate already synchronize human and agent changes, cursor state, and editor reconciliation. +- Content already exposes Page access as `canComment` and `canEdit`; Core sharing already has viewer/commenter/editor/admin/owner role ordering. +- Content's comments already provide text anchors, replies, mentions, resolution, and a comments rail. +- `packages/core/src/review` already owns reusable access-scoped review comments, threads, mentions, notifications, and review status actions. +- `packages/core/src/history` and automatic action audit provide reusable version/history and actor attribution donors. +- The approved Content records `content.diff.in-place`, `content.diff.filtered-review`, `content.diff.ai-assist`, `content.version.field-history`, and feature 7 already define the broader destination. + +### Ownership boundaries + +- **Core review domain:** stable suggestion/change/thread/decision types, stores, access-scoped list/create/reply/react/mute/unread/decide actions, idempotency, durable attribution, and notification hooks. +- **Content domain:** supported page-body operation grammar, document access/lock/source checks, base revision calculation, canonical apply transaction, comment-rail integration, and editor rendering. +- **TipTap editor:** ephemeral composition and provisional visual presentation. It is not the durable source of suggestion truth. +- **Yjs collaboration:** transports live canonical editor state and presence. Pending suggestions must not be written into the canonical Y.Doc as if accepted. +- **SQL:** owns durable suggestions, operations, thread state, dispositions, and canonical Content. Large editor snapshots or Yjs blobs are not copied into suggestion rows. +- **Audit/history:** records actual proposal and decision actions. A pending suggestion is not a committed Content revision; acceptance creates the canonical mutation and its normal history/audit record. + +### Smallest compatible delta + +Extend the existing Core review seam with a generic executable-suggestion lifecycle and a registered resource adapter. Implement only Content document-body text and inline-mark operations first. Reuse the existing Content comments rail presentation through one review controller rather than creating a second discussion system. + +Do not begin with the generic cross-object typed graph promised by feature 7. The first slice establishes the stable lifecycle and Content adapter that the broader graph can later extend without changing user-visible semantics. + +### Legacy contracts that remain unchanged + +- Direct editor changes by editor/admin/owner continue to update the canonical document normally. +- Ordinary comments retain their current identities, anchors, reply/resolve behavior, and Notion comment synchronization. +- Existing whole-document versions remain recovery snapshots; they are not reclassified as suggestions. +- SQL remains canonical for Content body; Yjs remains the live collaboration transport and reconciliation layer. +- Local-file and externally source-owned pages retain their current authority and synchronization rules. +- Existing sharing role names stay fixed; only commenter capability copy changes to truthfully include suggesting for Content resources. + +### Evidence classification + +Direct evidence comes from the verified Notion interaction memo and screenshots, Content schema/actions/editor code, Core review/history/sharing code, and approved Content product records. The proposed Core adapter shape is an architectural inference from those seams. No unresolved domain-owner question changes the public contract; storage details and ProseMirror decoration technique remain implementation choices. + +## Durable model + +Add a Core-owned suggestion aggregate with append-only decisions: + +- `review_suggestions`: id, resource type/id, adapter kind/version, thread id, author/actor/run context, base revision token, status (`pending`, `accepted`, `rejected`, `stale`, `superseded`), summary, timestamps, access scope, and metadata. +- `review_suggestion_operations`: stable operation id, suggestion id, ordinal, operation kind, field/target identity, before/after payload, anchor/context, dependencies, and payload schema version. +- `review_suggestion_decisions`: stable idempotency key, suggestion id, reviewer, decision, observed base, outcome, failure/conflict detail, and timestamp. +- Reuse Core review comments/threads for replies, mentions, reactions, resolution, mute, unread, and deep links; link the thread to the suggestion ID explicitly. + +For Content v1, the operation grammar is `insert_text`, `delete_text`, `replace_text`, `add_text_block`, and `set_inline_mark`. Each operation records the affected Blocks field, ProseMirror-compatible range/shape, exact before/after material, surrounding anchor context, and a base digest. The durable payload is typed JSON, not serialized ProseMirror transactions or Yjs client updates. + +Suggestion creation validates that all operations are supported, belong to one Page body, and match the observed base. It does not mutate `documents.content` or create a document version. + +Acceptance runs one Content-owned transaction: re-resolve access and feature flag; load the current body; verify or safely rebase the exact operation; apply it through the canonical Content serializer; update `documents.content` and `updatedAt`; create the ordinary Content version/history/audit effects; append the accepted decision; and emit normal action/collaboration sync. If any step fails, neither the canonical content nor the disposition commits. Reject appends only the decision/disposition and leaves canonical content unchanged. + +## Interaction design + +The mode should feel like Content, not like a separate diff application: + +1. The ordinary page editor stays in place. Unsupported page controls and blocks become non-editable while suggesting. +2. Provisional additions and formatting render inline; deletions remain visible with subdued strike treatment. Semantic tokens distinguish proposed material without relying on color alone. +3. A compact numbered marker aligns to each affected block. Selecting it opens the existing right utility rail on the exact suggestion thread. +4. The rail shows a typed operation summary first, then Accept/Reject for authorized reviewers, replies/reactions, and thread controls. It distinguishes counts for edits and replies. +5. **All discussions** filters ordinary comments and suggestion threads by Pending, Accepted, Rejected, and Resolved without hiding historical decisions. +6. Deep links reopen the Page, scroll to the current or historical anchor, and focus the thread. If the anchor is stale or deleted, the rail shows the retained before/after material and honest stale state. +7. Keyboard users can enter/exit suggesting, traverse markers, inspect before/after text, decide, reply, and return focus to the editor. Screen readers receive operation kind, before/after material, author, status, and affected block context. + +No explanatory banner, duplicate page heading, or permanent review dashboard is added. Suggesting state lives in the header and contextual rail. + +## Action surface + +Core/shared actions: + +- `create-resource-suggestion` +- `list-resource-suggestions` +- `get-resource-suggestion` +- `decide-resource-suggestion` +- `reply-review-comment` / existing thread actions +- `react-to-review-comment`, `set-review-thread-unread`, and `set-review-thread-muted` where missing + +Content registers a `document` suggestion adapter that implements `validateProposal`, `preview`, `apply`, `resolveAnchor`, and `describeOperation`. UI calls the same actions through `useActionQuery`/`useActionMutation`; agent tools expose the identical schemas. Direct agent edits remain available when authorized, but an explicit “suggest” request must use suggestion actions and return suggestion IDs/deep links rather than claim the Page changed. + +## Delivery plan + +### Slice 1 — lifecycle and permissions + +Add the Core suggestion types/store/registry/actions, Content adapter registration, default-off `content-suggested-edits` flag, role/lock/source enforcement, idempotent accept/reject, action audit targets, and focused database tests. This slice may use fixture operations before editor composition exists. + +### Slice 2 — editor composition and in-place rendering + +Add Suggesting mode to `VisualEditor`, operation capture for supported text/mark changes, provisional decorations, block markers, header/menu entry/exit, pending persistence, reload restoration, and narrow/keyboard/accessibility states. Prevent pending material from entering canonical autosave/Yjs reconciliation. + +### Slice 3 — discussion and history parity + +Unify suggestion threads with the Content comments rail; add replies, mentions, reactions, unread, mute, deep links, pending/accepted/rejected history filters, notifications, and honest orphan/stale presentation. + +### Slice 4 — agent parity and rollout proof + +Teach Content's agent instructions/actions to propose rather than directly edit when asked, expose inspection links, group a run's related suggestions without bulk-deciding them, verify live cross-client updates, and dogfood behind the flag before wider rollout. + +### Deferred beyond parity v1 + +- Titles, Properties, database cells/schema, media, embeds, and arbitrary registry blocks. +- Accept/reject all, filtered bulk review, dependency-safe sets, and agent-generated review summaries. +- Named Versions, cross-Version merge, general typed change graphs, code review, and external-provider suggestion synchronization. +- Local-file suggestions and offline portable suggestion representation. + +These are plausible extensions of the stable lifecycle, not requirements for Notion parity. + +## Risks and controls + +- **Canonical leakage:** pending edits accidentally autosave or enter Yjs. Control with a separate suggestion editor transaction filter/state and tests that canonical Markdown/Y.Doc/drafts remain byte-identical until acceptance. +- **Stale acceptance:** current material changes after proposal. Control with base token plus exact before material and contextual anchor; fail as stale unless the adapter proves a unique safe rebase. +- **Commenter escalation:** proposal or acceptance bypasses role limits. Control at action/adaptor boundaries; commenter may create but only editor/admin/owner may accept/reject unless future policy explicitly changes. +- **Dual discussion models:** Content comments and Core review threads drift. Control by adopting the Core controller/store for suggestion threads and incrementally adapting ordinary Content comments rather than creating a third store. +- **History ambiguity:** proposal creation looks like committed content. Control with distinct proposal/decision events; only acceptance emits canonical Content mutation history. +- **External-source corruption:** suggestions apply to source-owned or syncing bodies. Control by excluding local-file, Notion-linked/source-owned conflict states, and other non-local authority in v1. +- **Payload drift:** editor schema evolves. Control with versioned operation payloads and adapter-owned migration/degraded rendering. + +## Acceptance story + +The acceptance interface is the real Content Page editor in a deployed beta surface with two authenticated test users (commenter and editor) plus an accountable agent run. Independence is preferred and custody may remain in the same context because the interaction is consequential but reversible, while technical review and automated invariants cover the persistence and authorization risk. + +Required assertions: + +1. A commenter enters Suggesting, creates Add/Delete/Replace/new-text-block/format suggestions, exits and reloads, and the canonical document and another viewer's canonical rendering remain unchanged. +2. An editor sees inline markers, exact before/after material, author/time, replies/reactions, and independently accepts one suggestion and rejects another; only the accepted operation changes canonical Content. +3. Accepted and rejected threads remain discoverable in All discussions with durable actor, time, decision, replies, reactions, and working deep links. +4. Viewer creation and commenter acceptance are denied identically through UI and actions; lock/source/unsupported-block constraints fail closed without partial state. +5. An agent asked to suggest creates inspectable pending suggestions through the same actions and does not directly modify canonical content. +6. Concurrent canonical editing produces either a proven safe rebase or an explicit stale/conflict state; it never overwrites newer material or reports a no-op as accepted. +7. Retry of create/accept/reject is idempotent; simulated persistence failure leaves canonical content and suggestion disposition mutually consistent. +8. Two open clients receive proposal, reply, reaction, and disposition updates through shared sync without extra EventSource connections or editor jitter. +9. Keyboard and screen-reader workflows can enter/exit mode, identify operation types and before/after material, traverse suggestions, decide, reply, and restore focus. +10. With the feature flag Off, current editing, comments, sharing, history, agent direct-edit behavior, local files, and source sync remain unchanged. + +Automated coverage is required for model/store/action/permission/idempotency/stale/persistence/canonical-isolation behavior. Real-interface evidence is required for the complete editor workflow at desktop and narrow widths. A proportional independent technical review is required for Core review boundary, permission checks, and Yjs/canonical isolation. + +## Architecture fingerprint + +```yaml +stage: shape +authority-source: "User invoked $shape and requested exact Suggested Edits feature parity for Agent Native Content." +authorized-scope: + repositories: + - /home/teenylilmonkey/Developer/agent-native + product-surfaces: + - Agent Native Content page editor + - Agent Native Core review substrate + outcome: Notion-parity Suggested Edits for supported Content page-body text and inline formatting +allowed-mutations: + - artifact-write +write-targets: + artifacts: + - templates/content/docs/solutions/2026-09-02-content-suggested-edits-parity-shape.md +governing-artifact: + path: templates/content/docs/solutions/2026-09-02-content-suggested-edits-parity-shape.md + revision: content-suggested-edits-shape-r1 +architecture-fingerprint: + outcome: Commenters, editors, and agents can propose supported page-body edits without changing canonical Content until an authorized reviewer accepts them. + shipping-surfaces: + - id: agent-native-content-template + repository: agent-native + product-surface: templates/content deployed application + constituency: authenticated Content page collaborators and accountable agents + durable-destination: agent-native repository main plus deployed Content beta/production surfaces + integration-action: merge + - id: agent-native-core-packages + repository: agent-native + product-surface: Core review/action/client packages consumed by templates + constituency: Agent Native app developers and review-capable applications + durable-destination: agent-native repository main and publishable Core package release + integration-action: merge + governing-architecture: Core owns the reusable executable-suggestion lifecycle and Content registers page-body operation, authorization, apply, and renderer behavior while SQL remains canonical and Yjs remains live transport only. + acceptance-story: + id: content-suggested-edits-parity-v1 + summary: A commenter or agent proposes supported edits in the ordinary Content editor; an authorized editor discusses and decides each proposal; only accepted material reaches canonical Content and every state remains attributable and recoverable. + required-assertions: + - proposal creation preserves canonical content + - selective accept/reject applies exactly the decided operation + - discussion and resolved history persist with deep links + - UI, agent, and action permission parity fails closed + - stale/concurrent edits never overwrite newer content + - retries and persistence failures remain atomic and idempotent + - cross-client sync and accessibility work through the real editor + - flag-off legacy behavior remains unchanged + acceptance-policy: + modality: real-interface + independence: preferred + custody: same-context-allowed + interface: deployed Content beta Page editor with commenter/editor users and an accountable agent run + rationale: The user-visible workflow requires real editor proof; reversible review actions permit same-context custody while authorization and canonical-isolation seams receive independent technical review. + risk-strategy: + kind: feature-flagged + production-validation-after-merge: true +architecture-grounding: + applicability: required + reason: The feature crosses shared review, permissions, history, action, collaboration, and Content domain boundaries. + status: grounded + demonstrated-callers: + - Content Page collaborator or accountable agent requesting Suggest edits + existing-primitives: + - Core review comments/threads/notifications/status + - Core sharing roles and action audit + - Core history/version donor + - Content TipTap/Yjs editor and comments rail + - Content document access/actions/version snapshots + ownership-boundaries: + - Core owns reusable suggestion lifecycle and shared actions + - Content owns document operation semantics and canonical application + - SQL owns durable truth; Yjs owns live collaborative transport + legacy-contracts: + - direct editing, comments, versions, sharing, local files, source sync, and agent direct edits remain unchanged when suggestion mode is not requested or flag is off + shared-vocabulary: + - Suggested Edit + - Suggesting + - suggestion operation + - suggestion thread + - pending + - accepted + - rejected + - stale + smallest-compatible-delta: Core executable-suggestion lifecycle plus a Content page-body text/mark adapter and ordinary-editor presentation. + deferred-capabilities: + - generic typed Property/Database/media proposals + - filtered bulk review + - named Versions and selective cross-Version merge + - local-file and provider suggestion synchronization + reversibility: A default-off app-owned feature flag keeps dormant code inactive; suggestion tables are additive; pending proposals never alter canonical content. + direct-evidence: + - verified Notion Suggested Edits interaction memo dated 2026-09-01 + - packages/core/src/review and packages/core/src/history + - templates/content editor, actions, schema, comments, collaboration plugin + - Content feature 7 and diff/history/access capability records + inferences: + - extending Core review with an adapter registry is the smallest reusable implementation boundary + - typed JSON operations are more durable than persisted ProseMirror transactions or Yjs updates + unresolved-owner-questions: [] +delegation-ceiling: [] +acceptance-state: + status: pending + summary: Shape is complete; implementation and current acceptance evidence have not begun. + blockers: [] + last-land-packet: null +ledger-revision: content-suggested-edits-shape-r1 +status: active +``` + +## Next step + +Invoke `/work templates/content/docs/solutions/2026-09-02-content-suggested-edits-parity-shape.md` to implement the frozen first release. Work should begin with Slice 1 and preserve the exact parity boundary; discovering that the operation model cannot maintain canonical isolation or safe stale detection is a return-to-shape condition rather than permission to degrade silently. diff --git a/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/lib/suggested-edits.test.ts b/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/lib/suggested-edits.test.ts new file mode 100644 index 00000000000..1b7455a9a0d --- /dev/null +++ b/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/lib/suggested-edits.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from "vitest"; + +import { applySuggestedEdit, assertSuggestedEditTarget, suggestedEditDigest } from "./suggested-edits.js"; + +describe("Content suggested edit adapter", () => { + it("keeps exact-base application canonical and digestable", () => { + const body = "Hello world"; + const result = applySuggestedEdit({ body, source: "canonical" }, { operation: "replace", before: "world", after: "Content" }, suggestedEditDigest(body)); + expect(result).toEqual({ state: "applied", body: "Hello Content" }); + }); + + it("rebases only unique context and reports stale/conflict otherwise", () => { + const edit = { operation: "replace" as const, before: "world", after: "Content" }; + expect(applySuggestedEdit({ body: "Hello world" }, edit, "old").state).toBe("applied"); + expect(applySuggestedEdit({ body: "Hello there" }, edit, "old").state).toBe("stale"); + expect(applySuggestedEdit({ body: "world and world" }, edit, "old").state).toBe("conflict"); + }); + + it("fails closed for source-owned, locked, or unsupported targets", () => { + expect(() => assertSuggestedEditTarget({ body: "x", source: "local-file" })).toThrow(); + expect(() => assertSuggestedEditTarget({ body: "x", locked: true })).toThrow(); + expect(() => assertSuggestedEditTarget({ body: "x", unsupported: true })).toThrow(); + }); +}); diff --git a/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/lib/suggested-edits.ts b/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/lib/suggested-edits.ts new file mode 100644 index 00000000000..687a30760b1 --- /dev/null +++ b/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/lib/suggested-edits.ts @@ -0,0 +1,81 @@ +import { createHash } from "node:crypto"; + +export type SuggestedEditOperation = + | "add" + | "delete" + | "replace" + | "new-text-block" + | "format"; + +export interface SuggestedEdit { + operation: SuggestedEditOperation; + before: string; + after: string; + anchor?: string; +} + +export interface SuggestedEditTarget { + body: string; + source?: "database" | "local-file" | "provider" | "canonical"; + locked?: boolean; + unsupported?: boolean; +} + +export interface SuggestedEditApplyResult { + state: "applied" | "stale" | "conflict"; + body: string; + reason?: string; +} + +const unsupportedSources = new Set(["database", "local-file", "provider"]); + +export function suggestedEditDigest(body: string): string { + return createHash("sha256").update(body, "utf8").digest("hex"); +} + +export function assertSuggestedEditTarget(target: SuggestedEditTarget): void { + if (target.locked) throw new Error("Suggested edits are unavailable on a locked page."); + if (target.unsupported || (target.source && unsupportedSources.has(target.source))) { + throw new Error("Suggested edits are unavailable for this document source or content."); + } +} + +export function applySuggestedEdit( + target: SuggestedEditTarget, + edit: SuggestedEdit, + baseDigest: string, +): SuggestedEditApplyResult { + assertSuggestedEditTarget(target); + if (suggestedEditDigest(target.body) === baseDigest) { + return { state: "applied", body: applyExact(target.body, edit) }; + } + + const context = edit.before || edit.anchor; + if (!context) return { state: "stale", body: target.body, reason: "The page changed since this suggestion was created." }; + const first = target.body.indexOf(context); + if (first < 0) return { state: "stale", body: target.body, reason: "The suggested material is no longer present." }; + if (first !== target.body.lastIndexOf(context)) { + return { state: "conflict", body: target.body, reason: "The suggested context occurs more than once." }; + } + return { + state: "applied", + body: applyAtContext(target.body, edit, first, context), + }; +} + +function applyExact(body: string, edit: SuggestedEdit): string { + if (edit.operation === "new-text-block") return body ? `${body}\n\n${edit.after}` : edit.after; + const index = edit.before ? body.indexOf(edit.before) : -1; + if (index < 0 && edit.operation !== "add") throw new Error("Suggested material is not present in the canonical page."); + if (index < 0) return body ? `${body}\n\n${edit.after}` : edit.after; + return applyAtContext(body, edit, index, edit.before); +} + +function applyAtContext(body: string, edit: SuggestedEdit, index: number, context: string): string { + const end = index + context.length; + if (edit.operation === "delete") return body.slice(0, index) + body.slice(end); + if (edit.operation === "add" || edit.operation === "new-text-block") { + return body.slice(0, end) + edit.after + body.slice(end); + } + return body.slice(0, index) + edit.after + body.slice(end); +} diff --git a/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/plugins/feature-flags.ts b/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/plugins/feature-flags.ts new file mode 100644 index 00000000000..6c833cf3fa9 --- /dev/null +++ b/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/plugins/feature-flags.ts @@ -0,0 +1,5 @@ +import { createFeatureFlagsPlugin } from "@agent-native/core/server"; + +import { CONTENT_FEATURE_FLAGS } from "../../shared/feature-flags.js"; + +export default createFeatureFlagsPlugin({ flags: CONTENT_FEATURE_FLAGS }); diff --git a/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/plugins/suggested-edits.ts b/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/plugins/suggested-edits.ts new file mode 100644 index 00000000000..5896e705901 --- /dev/null +++ b/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/server/plugins/suggested-edits.ts @@ -0,0 +1,13 @@ +import { registerReviewableResource } from "@agent-native/core/review"; + +import { CONTENT_SUGGESTED_EDITS_FLAG } from "../../shared/feature-flags.js"; +import { assertSuggestedEditTarget, applySuggestedEdit } from "../lib/suggested-edits.js"; + +/** Registers Content's page-body adapter once Core's executable suggestion API is available. */ +export default async function suggestedEditsPlugin() { + registerReviewableResource({ + resourceType: "document", + featureFlag: CONTENT_SUGGESTED_EDITS_FLAG.key, + adapter: { assertTarget: assertSuggestedEditTarget, apply: applySuggestedEdit }, + } as never); +} diff --git a/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/shared/feature-flags.ts b/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/shared/feature-flags.ts new file mode 100644 index 00000000000..b5f624fa0cd --- /dev/null +++ b/wsl.localhost/NixOS/home/teenylilmonkey/.codex/worktrees/content-suggested-edits/agent-native/templates/content/shared/feature-flags.ts @@ -0,0 +1,11 @@ +import { defineFeatureFlag, defineFeatureFlags } from "@agent-native/core/feature-flags"; + +export const CONTENT_SUGGESTED_EDITS_FLAG = defineFeatureFlag({ + key: "content.suggested-edits", + displayName: "Content suggested edits", + description: "Allow authorized collaborators and agents to propose page-body edits for review.", +}); + +export const CONTENT_FEATURE_FLAGS = defineFeatureFlags([ + CONTENT_SUGGESTED_EDITS_FLAG, +]); From 7c409e8bf43fdbe73ca13bdc2e7220d21af7534c Mon Sep 17 00:00:00 2001 From: 3mdistal <86723305+3mdistal@users.noreply.github.com> Date: Mon, 21 Sep 2026 23:01:09 -0400 Subject: [PATCH 11/28] feat(content): clarify page and collection creation --- .../editor/DocumentEditor.layout.test.ts | 30 ++--- .../app/components/editor/DocumentEditor.tsx | 113 ++++++++--------- .../editor/SlashCommandMenu.test.ts | 12 +- .../components/editor/SlashCommandMenu.tsx | 76 +++++++++++- .../components/editor/body-hydration.test.ts | 42 ++++--- .../app/components/editor/body-hydration.ts | 14 ++- .../sidebar/DocumentSidebar.layout.test.ts | 17 ++- .../components/sidebar/DocumentSidebar.tsx | 115 +++++++++++++----- .../components/sidebar/DocumentTreeItem.tsx | 2 +- templates/content/app/i18n-data.ts | 64 ++++++++++ templates/content/app/i18n/zh-TW.ts | 6 + 11 files changed, 352 insertions(+), 139 deletions(-) diff --git a/templates/content/app/components/editor/DocumentEditor.layout.test.ts b/templates/content/app/components/editor/DocumentEditor.layout.test.ts index 8aa7bdb1fda..4dd93521c96 100644 --- a/templates/content/app/components/editor/DocumentEditor.layout.test.ts +++ b/templates/content/app/components/editor/DocumentEditor.layout.test.ts @@ -78,9 +78,8 @@ describe("document editor layout", () => { expect(toolbar).toContain( "disabled={!canEdit || revealLocalSource.isPending}", ); - expect(toolbar).toContain( - "disabled={!canEdit}\n onSelect={() => void handleCopyLocalAbsolutePath()}", - ); + expect(toolbar).toContain("disabled={!canEdit}"); + expect(toolbar).toContain("handleCopyLocalAbsolutePath()"); }); it("publishes unsaved local content to the synchronous conflict guard", () => { @@ -254,28 +253,23 @@ describe("document editor layout", () => { expect(documentEditorTitleRegionClassName(false)).toContain("pb-8"); }); - it("offers page or database after an optimistic blank page opens", () => { + it("keeps the editor open and offers collection conversion while the body is empty", () => { const source = readFileSync( new URL("./DocumentEditor.tsx", import.meta.url), { encoding: "utf8" }, ); - expect(source).toContain("const showNewDocumentTypeChooser ="); - expect(source).toContain("!document.database?.systemRole"); - expect(source).toContain("isEffectivelyEmptyDocumentContent(localContent)"); - expect(source).toContain("const handleChoosePage = useCallback"); - expect(source).toContain( - "await createDatabase.mutateAsync({ documentId })", - ); + expect(source).toContain("const showCreateCollectionStarter ="); + expect(source).toContain("createCollectionStarterIsVisible({"); + expect(source).toContain("content: localContent"); + expect(source).toContain("const handleCreateCollection = useCallback"); + expect(source).toContain("title: localTitleRef.current"); expect(source).toContain("isDatabaseChoicePending("); expect(source).toContain("document,\n createDatabase.isPending"); - expect(source).toContain( - "disabled={!editorCanEdit || databaseChoicePending}", - ); - expect(source).toContain('{t("sidebar.page")}'); - expect(source).toContain('{t("sidebar.database")}'); - expect(source.indexOf("if (showNewDocumentTypeChooser)")).toBeLessThan( - source.indexOf("const primaryEditor ="), + expect(source).toContain("disabled={databaseChoicePending}"); + expect(source).toContain('{t("editor.createCollection")}'); + expect(source.indexOf("const primaryEditor =")).toBeLessThan( + source.indexOf("{showCreateCollectionStarter ? ("), ); }); diff --git a/templates/content/app/components/editor/DocumentEditor.tsx b/templates/content/app/components/editor/DocumentEditor.tsx index 54369dcbea2..2a11372e6ce 100644 --- a/templates/content/app/components/editor/DocumentEditor.tsx +++ b/templates/content/app/components/editor/DocumentEditor.tsx @@ -92,8 +92,8 @@ import { cn } from "@/lib/utils"; import { documentBodyHydrationIsPending, + createCollectionStarterIsVisible, isEffectivelyEmptyDocumentContent, - newDocumentPageChoiceIsDisabled, } from "./body-hydration"; import { BuilderBodySyncingNotice } from "./BuilderBodySyncingNotice"; import type { CommentTextAnchor } from "./comment-anchors"; @@ -636,7 +636,6 @@ function DocumentEditorBody({ const pushDocumentToNotion = usePushDocumentToNotion(documentId); const [localTitle, setLocalTitle] = useState(""); const [localContent, setLocalContent] = useState(""); - const [newDocumentTypeChosen, setNewDocumentTypeChosen] = useState(false); const [localContentUpdatedAt, setLocalContentUpdatedAt] = useState< string | null >(document.updatedAt ?? null); @@ -879,7 +878,6 @@ function DocumentEditorBody({ if (prevDocIdRef.current !== documentId) { prevDocIdRef.current = documentId; isInitializedRef.current = false; - setNewDocumentTypeChosen(false); if (saveTimeoutRef.current) { clearTimeout(saveTimeoutRef.current); saveTimeoutRef.current = null; @@ -1942,29 +1940,42 @@ function DocumentEditorBody({ document, createDatabase.isPending, ); - const showNewDocumentTypeChooser = - canEdit && - !isLocalFileDocument && - !isDatabasePage && - !newDocumentTypeChosen && - !localTitle.trim() && - !document.description?.trim() && - isEffectivelyEmptyDocumentContent(localContent); - const handleChoosePage = useCallback(() => { - setNewDocumentTypeChosen(true); - requestAnimationFrame(() => titleInputRef.current?.focus()); - }, []); - const handleChooseDatabase = useCallback(async () => { + const showCreateCollectionStarter = createCollectionStarterIsVisible({ + canEdit, + bodyHydrationPending, + isLocalFileDocument, + isDatabasePage, + isCollectionItem: Boolean( + document.databaseMembership && + !contentSpaces.some( + (space) => + space.filesDatabaseId === document.databaseMembership?.databaseId, + ), + ), + content: localContent, + }); + const handleCreateCollection = useCallback(async () => { try { - await createDatabase.mutateAsync({ documentId }); - setNewDocumentTypeChosen(true); + const saved = await handleContentSaveNow(localContentRef.current); + if (!saved) throw new Error(t("empty.genericError")); + await createDatabase.mutateAsync({ + documentId, + title: localTitleRef.current, + description: document.description ?? undefined, + }); } catch (error) { toast.error(t("sidebar.failedCreateDatabase"), { description: error instanceof Error ? error.message : t("empty.genericError"), }); } - }, [createDatabase, documentId, t]); + }, [ + createDatabase, + document.description, + documentId, + handleContentSaveNow, + t, + ]); const defaultIcon = defaultIconKind === "database" && !isDatabasePage ? (