diff --git a/src/engine.zig b/src/engine.zig index d467bb1..ea06f10 100644 --- a/src/engine.zig +++ b/src/engine.zig @@ -860,6 +860,20 @@ pub const Engine = struct { }; running_state = new_state; + // issue #40: deep-merge per-step registry + // so {{steps.X.output}} resolves in downstream + // templates regardless of output_key. Uses + // applyDeepUpdates (not applyUpdates) so multiple + // steps coexist under state.steps. + if (cr.raw_output) |raw_out| { + if (try buildStepRegistryUpdate(alloc, node_name, raw_out)) |reg| { + running_state = state_mod.applyDeepUpdates(alloc, running_state, reg) catch |err| blk: { + log.warn("task node {s} failed to apply step registry: {}", .{ node_name, err }); + break :blk running_state; + }; + } + } + // Gap 3: Store result in cache if (cache_ttl) |ttl| cache_store: { const pt_s = getNodeField(alloc, node_json, "prompt_template") orelse break :cache_store; @@ -1144,10 +1158,12 @@ pub const Engine = struct { return TaskNodeResult{ .completed = .{ .state_updates = null } }; }; - // 2. Render prompt with graph template interpolation and optional store access. - const rendered_prompt = self.renderWorkflowTemplate(alloc, prompt_template, state_json, runtime, null) catch |err| { - log.err("template render failed for node {s}: {}", .{ node_name, err }); - return TaskNodeResult{ .failed = "template render failed" }; + // 2. Render prompt with the STRICT template engine: missing variables + // must fail the node visibly. Silent-empty here is the + // hallucination-on-ramp (issue #40). + const rendered_prompt = self.renderPromptTemplateStrict(alloc, prompt_template, state_json, runtime, null) catch |err| { + log.err("prompt template render failed for node {s}: {} -- unresolved variable or unknown expression", .{ node_name, err }); + return TaskNodeResult{ .failed = "prompt template render failed: unresolved variable" }; }; // 3. Get workers and select one @@ -1549,6 +1565,21 @@ pub const Engine = struct { return templates.renderTemplateWithStore(alloc, template, state_json, runtime.input_json, item_json, runtime.storeAccess(self.store_fetcher)); } + /// Strict variant for prompt_template interpolation. Propagates + /// UnresolvedReference / UnknownExpression / StepNotFound so a missing + /// variable fails the node visibly instead of producing an empty prompt + /// that the worker hallucinates from (issue #40). + fn renderPromptTemplateStrict( + self: *Engine, + alloc: std.mem.Allocator, + template: []const u8, + state_json: []const u8, + runtime: RuntimeBindings, + item_json: ?[]const u8, + ) ![]const u8 { + return templates.renderPromptTemplateStrict(alloc, template, state_json, runtime.input_json, item_json, runtime.storeAccess(self.store_fetcher)); + } + fn buildRuntimeBindings(self: *Engine, alloc: std.mem.Allocator, workflow_json: []const u8, state_json: []const u8, input_json: ?[]const u8) RuntimeBindings { return .{ .input_json = input_json, @@ -2104,6 +2135,37 @@ fn buildTaskStateUpdates(alloc: std.mem.Allocator, node_json: []const u8, output return serializeJsonValue(alloc, .{ .object = result }); } +/// Build a per-step registry update JSON for deep-merge into state.steps. +/// Returns `{"steps": { : {"output": } }}` or null if +/// node_name is empty. The engine applies this via state.applyDeepUpdates +/// (not applyUpdates) so multiple steps coexist under state.steps without +/// clobbering each other (issue #40). +fn buildStepRegistryUpdate(alloc: std.mem.Allocator, node_name: []const u8, output: []const u8) !?[]const u8 { + if (node_name.len == 0) return null; + + var arena = std.heap.ArenaAllocator.init(alloc); + defer arena.deinit(); + const a = arena.allocator(); + + const output_val = blk: { + const op = json.parseFromSlice(json.Value, a, output, .{}) catch null; + if (op) |p| break :blk p.value; + break :blk json.Value{ .string = output }; + }; + + var inner: json.ObjectMap = .empty; + try inner.put(a, "output", output_val); + + var step_entry: json.ObjectMap = .empty; + try step_entry.put(a, node_name, .{ .object = inner }); + + var root: json.ObjectMap = .empty; + try root.put(a, "steps", .{ .object = step_entry }); + + const serialized = try serializeJsonValue(a, .{ .object = root }); + return try alloc.dupe(u8, serialized); +} + /// Serialize completed_nodes set to JSON array. fn serializeCompletedNodes(alloc: std.mem.Allocator, completed_nodes: *std.StringHashMap(void)) ![]const u8 { var arr: std.ArrayListUnmanaged([]const u8) = .empty; @@ -3170,6 +3232,75 @@ test "buildTaskStateUpdates applies output_mapping from JSON output" { try std.testing.expect(std.mem.indexOf(u8, result, "\"feedback\":\"looks good\"") != null); } +test "buildStepRegistryUpdate wraps plain text output (issue #40)" { + const allocator = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(allocator); + defer arena.deinit(); + + const result = try buildStepRegistryUpdate(arena.allocator(), "a", "draft plan"); + try std.testing.expect(result != null); + try std.testing.expectEqualStrings("{\"steps\":{\"a\":{\"output\":\"draft plan\"}}}", result.?); +} + +test "buildStepRegistryUpdate returns null for empty node_name (issue #40)" { + const allocator = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(allocator); + defer arena.deinit(); + + const result = try buildStepRegistryUpdate(arena.allocator(), "", "anything"); + try std.testing.expect(result == null); +} + +test "issue #40 integration: two-step sequential state propagates via steps registry" { + // Acceptance #4: 2-step sequential with template MUST echo verbatim. + // Step a emits "SENTINEL_XYZ"; step b's template renders {{steps.a.output}} + // against the state built by applying a's state_updates + registry update. + const allocator = std.testing.allocator; + var arena = std.heap.ArenaAllocator.init(allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + // Step a: no output_key, output is a plain string. + const node_a = "{\"type\":\"task\"}"; + const updates_a = try buildTaskStateUpdates(alloc, node_a, "SENTINEL_XYZ"); + const reg_a = try buildStepRegistryUpdate(alloc, "a", "SENTINEL_XYZ"); + + // Apply a's updates onto empty state, then deep-merge the registry entry. + var state_after_a = try state_mod.applyUpdates(alloc, "{}", updates_a, "{}"); + if (reg_a) |r| { + state_after_a = try state_mod.applyDeepUpdates(alloc, state_after_a, r); + } + + // Step b: render template against state_after_a. + const rendered = try templates.renderTemplate( + alloc, + "Echo: {{steps.a.output}}", + state_after_a, + null, + null, + ); + try std.testing.expectEqualStrings("Echo: SENTINEL_XYZ", rendered); + + // Step b also writes its own output; the registry deep-merges so both + // a and b coexist under state.steps. + const node_b = "{\"type\":\"task\"}"; + const updates_b = try buildTaskStateUpdates(alloc, node_b, "downstream"); + const reg_b = try buildStepRegistryUpdate(alloc, "b", "downstream"); + + var state_after_b = try state_mod.applyUpdates(alloc, state_after_a, updates_b, "{}"); + if (reg_b) |r| { + state_after_b = try state_mod.applyDeepUpdates(alloc, state_after_b, r); + } + const rendered2 = try templates.renderTemplate( + alloc, + "a={{steps.a.output}} b={{steps.b.output}}", + state_after_b, + null, + null, + ); + try std.testing.expectEqualStrings("a=SENTINEL_XYZ b=downstream", rendered2); +} + test "getSendItemsPath prefers canonical items_key" { const allocator = std.testing.allocator; var arena = std.heap.ArenaAllocator.init(allocator); diff --git a/src/state.zig b/src/state.zig index 7db83f2..1291e4f 100644 --- a/src/state.zig +++ b/src/state.zig @@ -391,6 +391,26 @@ fn applyMerge(alloc: Allocator, old_json: ?[]const u8, update_json: []const u8) return try alloc.dupe(u8, result); } +/// Recursively merge `updates_json` into `state_json`. Unlike `applyUpdates`, +/// this bypasses reducer semantics and always deep-merges nested objects — +/// used for the per-step registry where multiple writers land under a shared +/// parent key (e.g. state.steps..output) without clobbering siblings +/// (issue #40). +pub fn applyDeepUpdates(alloc: Allocator, state_json: []const u8, updates_json: []const u8) ![]const u8 { + var arena = std.heap.ArenaAllocator.init(alloc); + defer arena.deinit(); + const arena_alloc = arena.allocator(); + + const state_parsed = json.parseFromSlice(json.Value, arena_alloc, state_json, .{}) catch { + return try alloc.dupe(u8, updates_json); + }; + const updates_parsed = try json.parseFromSlice(json.Value, arena_alloc, updates_json, .{}); + + const merged = try deepMerge(arena_alloc, state_parsed.value, updates_parsed.value); + const result = try serializeValue(arena_alloc, merged); + return try alloc.dupe(u8, result); +} + /// Recursively deep-merge two JSON objects. fn deepMerge(alloc: Allocator, base: json.Value, overlay: json.Value) !json.Value { if (base != .object or overlay != .object) { diff --git a/src/templates.zig b/src/templates.zig index dea3057..766bc94 100644 --- a/src/templates.zig +++ b/src/templates.zig @@ -42,6 +42,7 @@ pub const Context = struct { pub const RenderError = error{ UnterminatedExpression, UnknownExpression, + UnresolvedReference, InputFieldNotFound, StepNotFound, ItemNotAvailable, @@ -398,6 +399,16 @@ fn lookupJsonPath(alloc: Allocator, json_bytes: []const u8, path: []const u8) !? /// Resolve a template expression (the text inside `{{ }}`) to a string value. /// Handles state.X, input.X, item, item.X expressions. +/// +/// `strict` controls the missing-key policy: +/// - false (lenient): missing keys return empty string. Used by +/// processNewConditionals / isNewTruthy so `{% if X %}` evaluates +/// empty-as-false without raising. +/// - true (strict): missing keys return error.UnresolvedReference. Used by +/// renderPromptTemplateStrict for prompt_template interpolation so an +/// unresolved variable fails the node visibly instead of producing an +/// empty prompt that the worker hallucinates from (issue #40, +/// issue #40 acceptance). fn resolveNewExpression( alloc: Allocator, expr: []const u8, @@ -405,7 +416,45 @@ fn resolveNewExpression( input_json: ?[]const u8, item_json: ?[]const u8, store_access: ?StoreAccess, + strict: bool, ) ![]const u8 { + // Helper: turn a missing-value result into either empty string (lenient) + // or UnresolvedReference (strict). + const OrErr = struct { + fn emptyOrErr(s: bool, a: Allocator) ![]const u8 { + if (s) return error.UnresolvedReference; + return a.dupe(u8, "") catch return error.OutOfMemory; + } + }; + + // {{steps.X.Y}} — per-step output registry written by buildStepRegistryUpdate + // alongside output_key. Resolves against state.steps.X.Y. Errors visibly + // when the step is absent so the worker never receives an empty prompt + // and hallucinates from session context (issue #40). + if (std.mem.startsWith(u8, expr, "steps.")) { + const rest = expr["steps.".len..]; + if (rest.len == 0) return error.UnknownExpression; + + // rest is "."; re-anchor under state.steps.. + const path = try std.fmt.allocPrint(alloc, "steps.{s}", .{rest}); + defer alloc.free(path); + + const raw = state_mod.getStateValue(alloc, state_json, path) catch return error.StepNotFound; + if (raw) |r| { + const stripped = stripJsonQuotes(r); + if (stripped.ptr != r.ptr or stripped.len != r.len) { + const result = alloc.dupe(u8, stripped) catch return error.OutOfMemory; + alloc.free(r); + return result; + } + return r; + } + // Step registry is the canonical output-chaining mechanism; a missing + // step is always a hard error regardless of strict mode — silent empty + // here is exactly the aaq hallucination vector. + return error.StepNotFound; + } + if (std.mem.startsWith(u8, expr, "state.")) { // Use getStateValue which handles "state." prefix, nested paths, [-1] indexing const raw = try state_mod.getStateValue(alloc, state_json, expr); @@ -420,12 +469,12 @@ fn resolveNewExpression( } return r; } - return alloc.dupe(u8, "") catch return error.OutOfMemory; + return OrErr.emptyOrErr(strict, alloc); } if (std.mem.startsWith(u8, expr, "input.")) { const ij = input_json orelse { - return alloc.dupe(u8, "") catch return error.OutOfMemory; + return OrErr.emptyOrErr(strict, alloc); }; const field = expr["input.".len..]; const raw = try lookupJsonPath(alloc, ij, field); @@ -438,7 +487,7 @@ fn resolveNewExpression( } return r; } - return alloc.dupe(u8, "") catch return error.OutOfMemory; + return OrErr.emptyOrErr(strict, alloc); } if (std.mem.eql(u8, expr, "item")) { @@ -446,12 +495,12 @@ fn resolveNewExpression( const stripped = stripJsonQuotes(ij); return alloc.dupe(u8, stripped) catch return error.OutOfMemory; } - return alloc.dupe(u8, "") catch return error.OutOfMemory; + return OrErr.emptyOrErr(strict, alloc); } if (std.mem.startsWith(u8, expr, "item.")) { const ij = item_json orelse { - return alloc.dupe(u8, "") catch return error.OutOfMemory; + return OrErr.emptyOrErr(strict, alloc); }; const field = expr["item.".len..]; const raw = try lookupJsonPath(alloc, ij, field); @@ -464,7 +513,7 @@ fn resolveNewExpression( } return r; } - return alloc.dupe(u8, "") catch return error.OutOfMemory; + return OrErr.emptyOrErr(strict, alloc); } // {{config.X}} — alias for {{state.__config.X}} @@ -481,7 +530,7 @@ fn resolveNewExpression( } return r; } - return alloc.dupe(u8, "") catch return error.OutOfMemory; + return OrErr.emptyOrErr(strict, alloc); } if (std.mem.startsWith(u8, expr, "store.")) { @@ -502,10 +551,11 @@ fn resolveNewExpression( } return r; } - return alloc.dupe(u8, "") catch return error.OutOfMemory; + return OrErr.emptyOrErr(strict, alloc); } - // Unknown expression — return empty + // Unknown expression form (no recognized prefix). + if (strict) return error.UnknownExpression; return alloc.dupe(u8, "") catch return error.OutOfMemory; } @@ -519,7 +569,7 @@ fn isNewTruthy( item_json: ?[]const u8, store_access: ?StoreAccess, ) bool { - const value = resolveNewExpression(alloc, expr, state_json, input_json, item_json, store_access) catch return false; + const value = resolveNewExpression(alloc, expr, state_json, input_json, item_json, store_access, false) catch return false; defer alloc.free(value); if (value.len == 0) return false; @@ -682,7 +732,7 @@ pub fn renderTemplateWithStore( const raw_expr = preprocessed[after_open..close]; const expr = std.mem.trim(u8, raw_expr, " \t\n\r"); - const value = try resolveNewExpression(alloc, expr, state_json, input_json, item_json, store_access); + const value = try resolveNewExpression(alloc, expr, state_json, input_json, item_json, store_access, false); defer alloc.free(value); result.appendSlice(alloc, value) catch return error.OutOfMemory; @@ -701,8 +751,145 @@ pub fn renderTemplateWithStore( return result.toOwnedSlice(alloc) catch return error.OutOfMemory; } +/// Strict prompt-template renderer. Same syntax as `renderTemplateWithStore` +/// but propagates `UnresolvedReference` / `UnknownExpression` / `StepNotFound` +/// instead of swallowing them into an empty string. +/// +/// Conditionals (`{% if X %}`) still evaluate leniently inside this renderer +/// — they need empty-as-false semantics. Only the `{{...}}` interpolation +/// phase is strict. Use this for `prompt_template` rendering so a missing +/// variable fails the node visibly instead of producing an empty prompt +/// that the worker hallucinates from prior-session context +/// (issue #40, issue #40 acceptance). +pub fn renderPromptTemplateStrict( + alloc: Allocator, + template: []const u8, + state_json: []const u8, + input_json: ?[]const u8, + item_json: ?[]const u8, + store_access: ?StoreAccess, +) ![]const u8 { + // Phase 1: Process conditional blocks (lenient — empty-as-false). + const preprocessed = try processNewConditionals(alloc, template, state_json, input_json, item_json, store_access); + defer alloc.free(preprocessed); + + // Phase 2: Resolve {{expression}} substitutions (strict — errors propagate). + var result: std.ArrayListUnmanaged(u8) = .empty; + errdefer result.deinit(alloc); + + var pos: usize = 0; + + while (pos < preprocessed.len) { + if (std.mem.indexOfPos(u8, preprocessed, pos, "{{")) |open| { + result.appendSlice(alloc, preprocessed[pos..open]) catch return error.OutOfMemory; + + const after_open = open + 2; + if (std.mem.indexOfPos(u8, preprocessed, after_open, "}}")) |close| { + const raw_expr = preprocessed[after_open..close]; + const expr = std.mem.trim(u8, raw_expr, " \t\n\r"); + + const value = try resolveNewExpression(alloc, expr, state_json, input_json, item_json, store_access, true); + defer alloc.free(value); + + result.appendSlice(alloc, value) catch return error.OutOfMemory; + pos = close + 2; + } else { + return error.UnterminatedExpression; + } + } else { + result.appendSlice(alloc, preprocessed[pos..]) catch return error.OutOfMemory; + break; + } + } + + return result.toOwnedSlice(alloc) catch return error.OutOfMemory; +} + // ── New template engine tests ───────────────────────────────────────── +test "template steps.X.output resolves from state registry" { + // issue #40: {{steps.X.output}} must resolve against the per-step + // registry that buildTaskStateUpdates writes alongside output_key. + const alloc = std.testing.allocator; + const s = "{\"steps\":{\"a\":{\"output\":\"found data\"}}}"; + const result = try renderTemplate(alloc, "Echo: {{steps.a.output}}", s, null, null); + defer alloc.free(result); + try std.testing.expectEqualStrings("Echo: found data", result); +} + +test "template steps.X.output errors visibly when step missing" { + // issue #40 acceptance: unresolved steps reference must error, + // not silently return empty (which caused aaq session-contamination + // hallucinations). + const alloc = std.testing.allocator; + try std.testing.expectError( + error.StepNotFound, + renderTemplate(alloc, "Echo: {{steps.missing.output}}", "{}", null, null), + ); +} + +test "renderPromptTemplateStrict errors on missing state.X (issue #40 acceptance)" { + const alloc = std.testing.allocator; + try std.testing.expectError( + error.UnresolvedReference, + renderPromptTemplateStrict(alloc, "Echo: {{state.missing}}", "{}", null, null, null), + ); +} + +test "renderPromptTemplateStrict errors on missing input.X (issue #40)" { + const alloc = std.testing.allocator; + try std.testing.expectError( + error.UnresolvedReference, + renderPromptTemplateStrict(alloc, "Echo: {{input.missing}}", "{}", "{}", null, null), + ); +} + +test "renderPromptTemplateStrict errors on missing item (issue #40)" { + const alloc = std.testing.allocator; + try std.testing.expectError( + error.UnresolvedReference, + renderPromptTemplateStrict(alloc, "Item: {{item}}", "{}", null, null, null), + ); +} + +test "renderPromptTemplateStrict errors on unknown expression form (issue #40)" { + const alloc = std.testing.allocator; + try std.testing.expectError( + error.UnknownExpression, + renderPromptTemplateStrict(alloc, "Echo: {{bogus.thing}}", "{}", null, null, null), + ); +} + +test "renderPromptTemplateStrict still honors conditionals leniently (issue #40)" { + // {% if X %} must still evaluate empty-as-false even in strict mode; + // only the {{...}} interpolation phase is strict. + const alloc = std.testing.allocator; + const result = try renderPromptTemplateStrict( + alloc, + "{% if state.missing %}hidden{% endif %}visible", + "{}", + null, + null, + null, + ); + defer alloc.free(result); + try std.testing.expectEqualStrings("visible", result); +} + +test "renderPromptTemplateStrict resolves state.X when present (issue #40)" { + const alloc = std.testing.allocator; + const result = try renderPromptTemplateStrict( + alloc, + "Hello {{state.name}}", + "{\"name\":\"World\"}", + null, + null, + null, + ); + defer alloc.free(result); + try std.testing.expectEqualStrings("Hello World", result); +} + test "template state interpolation" { const alloc = std.testing.allocator; const s = "{\"name\":\"test\",\"count\":42}"; diff --git a/src/workflow_validation.zig b/src/workflow_validation.zig index ccb6cf6..44956e5 100644 --- a/src/workflow_validation.zig +++ b/src/workflow_validation.zig @@ -543,6 +543,12 @@ fn getJsonStringFromObj(obj: std.json.ObjectMap, key: []const u8) ?[]const u8 { } /// Scan `text` for {{state.KEY}} references and check them against schema. +/// +/// Note on `{{steps.X.output}}` (issue #40 canonical convention for +/// referencing a prior step's output): the engine populates the +/// `state.steps..output` registry at runtime via buildStepRegistryUpdate, +/// so these references are NOT checked against state_schema. They will fail +/// visibly at prompt-render time if the referenced step never executed. fn checkStateRefs( alloc: Allocator, errors: *std.ArrayListUnmanaged(ValidationError), @@ -558,6 +564,9 @@ fn checkStateRefs( const expr = text[open + 2 .. close]; pos = close + 2; + // {{steps.X.output}} — runtime-populated registry, skip schema check. + if (std.mem.startsWith(u8, expr, "steps.")) continue; + // Check if it's "state.KEY" if (std.mem.startsWith(u8, expr, "state.")) { const key = expr["state.".len..];