diff --git a/src/domain/models/method_execution_service.ts b/src/domain/models/method_execution_service.ts index 4e5455cd..43b5f4eb 100644 --- a/src/domain/models/method_execution_service.ts +++ b/src/domain/models/method_execution_service.ts @@ -39,6 +39,7 @@ import { } from "./data_writer.ts"; import { coerceMethodArgs } from "./zod_type_coercion.ts"; import { containsExpression } from "../expressions/expression_parser.ts"; +import type { Data } from "../data/data.ts"; /** * Maximum depth for recursive follow-up action processing. @@ -46,6 +47,22 @@ import { containsExpression } from "../expressions/expression_parser.ts"; */ const DEFAULT_MAX_FOLLOW_UP_DEPTH = 100; +/** + * Filters data entries to only those belonging to declared resource specs. + * Uses the `specName` tag that `writeResource()` auto-injects on every write. + */ +function filterDeclaredResourceData( + allData: Data[], + declaredResources: Record, +): Data[] { + const specNames = new Set(Object.keys(declaredResources)); + return allData.filter((d) => + d.tags["type"] === "resource" && + d.tags["specName"] !== undefined && + specNames.has(d.tags["specName"]) + ); +} + /** * Formats a Zod error into a human-readable string. */ @@ -264,7 +281,10 @@ export class DefaultMethodExecutionService implements MethodExecutionService { // Infer method kind for lifecycle checks const methodKind = inferMethodKind(methodName, method); - // Fast-fail: reject read/update on deleted resources + // Fast-fail: reject read/update on deleted resources. + // Only check declared resource data (tagged with specName matching a + // declared resource spec). Block only when ALL declared resource data + // is deleted — old historical data entries should not cause false positives. if (methodKind === "read" || methodKind === "update") { const resources = modelDef.resources ?? {}; if (Object.keys(resources).length > 0) { @@ -272,31 +292,35 @@ export class DefaultMethodExecutionService implements MethodExecutionService { context.modelType, context.modelId, ); - for (const data of existingData) { - if (data.isDeleted) { - // Read the deletion marker content for the timestamp - let deletedAt = "unknown"; - try { - const content = await context.dataRepository.getContent( - context.modelType, - context.modelId, - data.name, + const resourceData = filterDeclaredResourceData( + existingData, + resources, + ); + if ( + resourceData.length > 0 && resourceData.every((d) => d.isDeleted) + ) { + const deleted = resourceData[0]; + let deletedAt = "unknown"; + try { + const content = await context.dataRepository.getContent( + context.modelType, + context.modelId, + deleted.name, + ); + if (content) { + const marker = JSON.parse( + new TextDecoder().decode(content), ); - if (content) { - const marker = JSON.parse( - new TextDecoder().decode(content), - ); - if (marker.deletedAt) { - deletedAt = marker.deletedAt; - } + if (marker.deletedAt) { + deletedAt = marker.deletedAt; } - } catch { - // Use default "unknown" timestamp } - throw new UserError( - `Resource '${data.name}' was deleted at ${deletedAt} — run a 'create' method to re-create it first`, - ); + } catch { + // Use default "unknown" timestamp } + throw new UserError( + `Resource '${deleted.name}' was deleted at ${deletedAt} — run a 'create' method to re-create it first`, + ); } } } @@ -380,7 +404,8 @@ export class DefaultMethodExecutionService implements MethodExecutionService { await context.outputRepository.save(modelDef.type, methodName, output); } - // Write deletion markers after a successful delete method + // Write deletion markers after a successful delete method. + // Only mark declared resource data — untagged or non-resource data is left alone. if (methodKind === "delete") { const resources = modelDef.resources ?? {}; if (Object.keys(resources).length > 0) { @@ -388,7 +413,11 @@ export class DefaultMethodExecutionService implements MethodExecutionService { context.modelType, context.modelId, ); - for (const data of existingData) { + const resourceData = filterDeclaredResourceData( + existingData, + resources, + ); + for (const data of resourceData) { if (!data.isDeleted) { const marker = data.withDeletionMarker({ version: data.version + 1, diff --git a/src/domain/models/method_execution_service_test.ts b/src/domain/models/method_execution_service_test.ts index de850ef1..b48cfec1 100644 --- a/src/domain/models/method_execution_service_test.ts +++ b/src/domain/models/method_execution_service_test.ts @@ -1168,7 +1168,7 @@ Deno.test("executeWorkflow - delete method writes deletion markers for existing contentType: "application/json", lifetime: "infinite", garbageCollection: 10, - tags: { type: "resource" }, + tags: { type: "resource", specName: "my-resource" }, ownerDefinition: { ownerType: "model-method", ownerRef: "test/model:create", @@ -1237,7 +1237,7 @@ Deno.test("executeWorkflow - read after delete throws UserError", async () => { contentType: "application/json", lifetime: "infinite", garbageCollection: 10, - tags: { type: "resource" }, + tags: { type: "resource", specName: "my-resource" }, ownerDefinition: { ownerType: "model-method", ownerRef: "test/model:create", @@ -1302,7 +1302,7 @@ Deno.test("executeWorkflow - update after delete throws UserError", async () => contentType: "application/json", lifetime: "infinite", garbageCollection: 10, - tags: { type: "resource" }, + tags: { type: "resource", specName: "my-resource" }, ownerDefinition: { ownerType: "model-method", ownerRef: "test/model:create", @@ -1418,7 +1418,7 @@ Deno.test("executeWorkflow - delete skips already-deleted resources", async () = contentType: "application/json", lifetime: "infinite", garbageCollection: 10, - tags: { type: "resource" }, + tags: { type: "resource", specName: "my-resource" }, ownerDefinition: { ownerType: "model-method", ownerRef: "test/model:create", @@ -1479,7 +1479,7 @@ Deno.test("executeWorkflow - explicit kind overrides name inference for deletion contentType: "application/json", lifetime: "infinite", garbageCollection: 10, - tags: { type: "resource" }, + tags: { type: "resource", specName: "my-resource" }, ownerDefinition: { ownerType: "model-method", ownerRef: "test/model:create", @@ -1531,6 +1531,227 @@ Deno.test("executeWorkflow - explicit kind overrides name inference for deletion assertEquals(mockRepo.savedData.length, 0); }); +Deno.test("executeWorkflow - read succeeds when old data is deleted but current resource data is active", async () => { + const service = new DefaultMethodExecutionService(); + + // Old historical data entry (no specName tag) that was deleted — should NOT block + const oldDeletedData = Data.create({ + name: "old-vpc-id-123", + contentType: "application/json", + lifetime: "infinite", + garbageCollection: 10, + tags: { type: "resource" }, + ownerDefinition: { + ownerType: "model-method", + ownerRef: "test/model:create", + }, + lifecycle: "deleted", + }); + + // Current active resource data with specName tag + const activeData = Data.create({ + name: "my-resource", + contentType: "application/json", + lifetime: "infinite", + garbageCollection: 10, + tags: { type: "resource", specName: "my-resource" }, + ownerDefinition: { + ownerType: "model-method", + ownerRef: "test/model:create", + }, + }); + + const mockRepo = { + ...createMockDataRepo(), + findAllForModel: () => Promise.resolve([oldDeletedData, activeData]), + getContent: () => Promise.resolve(null), + }; + + const model: ModelDefinition = { + type: ModelType.create("test/read-mixed-data"), + version: "1", + resources: { + "my-resource": { + description: "A resource", + schema: z.object({}), + lifetime: "infinite", + garbageCollection: 10, + }, + }, + methods: { + get: { + description: "Get the resource", + arguments: z.object({}), + execute: () => Promise.resolve({}), + }, + }, + }; + + const definition = Definition.create({ + name: "test-definition", + globalArguments: {}, + }); + + const { context } = createTestContext({ modelType: model.type }); + const contextWithRepo: MethodContext = { + ...context, + dataRepository: mockRepo, + }; + + // Should succeed — old deleted data without specName should not block read + const result = await service.executeWorkflow( + definition, + model, + "get", + contextWithRepo, + ); + assertEquals(result !== undefined, true); +}); + +Deno.test("executeWorkflow - deletion markers only written for declared resource data, not untagged data", async () => { + const service = new DefaultMethodExecutionService(); + + // Declared resource data with specName tag — should get a deletion marker + const resourceData = Data.create({ + name: "my-resource", + contentType: "application/json", + lifetime: "infinite", + garbageCollection: 10, + tags: { type: "resource", specName: "my-resource" }, + ownerDefinition: { + ownerType: "model-method", + ownerRef: "test/model:create", + }, + version: 3, + }); + + // Untagged data (no specName) — should NOT get a deletion marker + const untaggedData = Data.create({ + name: "some-old-data", + contentType: "application/json", + lifetime: "infinite", + garbageCollection: 10, + tags: { type: "resource" }, + ownerDefinition: { + ownerType: "model-method", + ownerRef: "test/model:create", + }, + version: 2, + }); + + const mockRepo = createMockDataRepoWithData([resourceData, untaggedData]); + + const model: ModelDefinition = { + type: ModelType.create("test/delete-scoped-markers"), + version: "1", + resources: { + "my-resource": { + description: "A resource", + schema: z.object({}), + lifetime: "infinite", + garbageCollection: 10, + }, + }, + methods: { + delete: { + description: "Delete the resource", + arguments: z.object({}), + execute: () => Promise.resolve({}), + }, + }, + }; + + const definition = Definition.create({ + name: "test-definition", + globalArguments: {}, + }); + + const { context } = createTestContext({ modelType: model.type }); + const contextWithRepo: MethodContext = { + ...context, + dataRepository: mockRepo, + }; + + await service.executeWorkflow( + definition, + model, + "delete", + contextWithRepo, + ); + + // Only the declared resource data should get a deletion marker + assertEquals(mockRepo.savedData.length, 1); + assertEquals(mockRepo.savedData[0].data.name, "my-resource"); + assertEquals(mockRepo.savedData[0].data.lifecycle, "deleted"); +}); + +Deno.test("executeWorkflow - read blocked when all declared resource data is deleted", async () => { + const service = new DefaultMethodExecutionService(); + + // All declared resource data is deleted + const deletedResource = Data.create({ + name: "my-resource", + contentType: "application/json", + lifetime: "infinite", + garbageCollection: 10, + tags: { type: "resource", specName: "my-resource" }, + ownerDefinition: { + ownerType: "model-method", + ownerRef: "test/model:create", + }, + lifecycle: "deleted", + }); + + const markerContent = new TextEncoder().encode(JSON.stringify({ + deletedAt: "2026-03-07T10:00:00.000Z", + deletedByMethod: "delete", + })); + + const mockRepo = { + ...createMockDataRepo(), + findAllForModel: () => Promise.resolve([deletedResource]), + getContent: () => Promise.resolve(markerContent), + }; + + const model: ModelDefinition = { + type: ModelType.create("test/read-all-deleted"), + version: "1", + resources: { + "my-resource": { + description: "A resource", + schema: z.object({}), + lifetime: "infinite", + garbageCollection: 10, + }, + }, + methods: { + get: { + description: "Get the resource", + arguments: z.object({}), + execute: () => Promise.resolve({}), + }, + }, + }; + + const definition = Definition.create({ + name: "test-definition", + globalArguments: {}, + }); + + const { context } = createTestContext({ modelType: model.type }); + const contextWithRepo: MethodContext = { + ...context, + dataRepository: mockRepo, + }; + + // Should throw — all declared resource data is deleted + await assertRejects( + () => service.executeWorkflow(definition, model, "get", contextWithRepo), + UserError, + "was deleted at 2026-03-07T10:00:00.000Z", + ); +}); + // ---------- Unresolved Expression Tests ---------- Deno.test("execute - Proxy throws for any unresolved expression in globalArgs", async () => {