From 028ac573989714a63ab727fb204234ddbb55881d Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 6 Aug 2026 21:17:11 +0200 Subject: [PATCH] fix(prompts): make template token substitution order-independent RenderTemplate replaced tokens one key at a time over a StringBuilder, so a token appearing inside an already-substituted value got substituted again on a later pass. The prompt briefs only escaped this because their callers happen to pass "tasks" last -- reordering the dictionary or adding a fourth token would have started rewriting user-authored task descriptions, which after the enhance phase carry file paths and config snippets. Single-pass regex over the template instead; unknown tokens still pass through. --- src/ClaudeDo.Data/PromptFiles.cs | 17 +++++---- tests/ClaudeDo.Data.Tests/PromptFilesTests.cs | 36 +++++++++++++++++++ 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/ClaudeDo.Data/PromptFiles.cs b/src/ClaudeDo.Data/PromptFiles.cs index b5a613fe..402e8940 100644 --- a/src/ClaudeDo.Data/PromptFiles.cs +++ b/src/ClaudeDo.Data/PromptFiles.cs @@ -1,6 +1,7 @@ using System.Security.Cryptography; using System.Text; using System.Text.Json; +using System.Text.RegularExpressions; namespace ClaudeDo.Data; @@ -193,14 +194,16 @@ public static class PromptFiles public static string Render(PromptKind kind, IReadOnlyDictionary values) => RenderTemplate(ReadOrDefault(kind), values); - /// Replace only the given {name} tokens; any other braces pass through untouched. + private static readonly Regex TokenPattern = new(@"\{(\w+)\}", RegexOptions.Compiled); + + /// Replace only the given {name} tokens; any other braces pass through untouched. + /// Single pass over the template, so a token appearing inside a substituted VALUE is never + /// re-substituted. That matters because the values are user-authored task titles and + /// descriptions: a sharpened description mentioning a literal {repo} must survive verbatim, + /// and it must not depend on the caller happening to order its dictionary correctly. public static string RenderTemplate(string template, IReadOnlyDictionary values) - { - var sb = new StringBuilder(template); - foreach (var (key, val) in values) - sb.Replace("{" + key + "}", val); - return sb.ToString(); - } + => TokenPattern.Replace(template, m => + values.TryGetValue(m.Groups[1].Value, out var val) ? val : m.Value); public static string DefaultFor(PromptKind kind) => kind switch { diff --git a/tests/ClaudeDo.Data.Tests/PromptFilesTests.cs b/tests/ClaudeDo.Data.Tests/PromptFilesTests.cs index a35d43e4..43b8f26f 100644 --- a/tests/ClaudeDo.Data.Tests/PromptFilesTests.cs +++ b/tests/ClaudeDo.Data.Tests/PromptFilesTests.cs @@ -22,6 +22,42 @@ public class PromptFilesTests Assert.Equal("## {Wochentag}, {dd.MM.yyyy} — 01.06.2026", outp); } + [Fact] + public void RenderTemplate_does_not_substitute_tokens_that_appear_inside_a_value() + { + // A sharpened task description can legitimately contain "{repo}" (a config snippet, a + // path placeholder). Substitution must be a single pass over the TEMPLATE, so injected + // values are never rescanned. + var outp = PromptFiles.RenderTemplate( + "Repo: {repo}\n\n{tasks}", + new Dictionary + { + ["repo"] = "C:\\real\\repo", + ["tasks"] = "- Fix the {repo} placeholder in the config template", + }); + + Assert.Equal("Repo: C:\\real\\repo\n\n- Fix the {repo} placeholder in the config template", outp); + } + + [Fact] + public void RenderTemplate_result_is_independent_of_dictionary_order() + { + const string template = "Scope: {scope}\nRepo: {repo}\n\n{tasks}"; + var task = "- Document {scope} and {repo} tokens"; + + var tasksLast = PromptFiles.RenderTemplate(template, new Dictionary + { + ["scope"] = "List: Bugs", ["repo"] = "C:\\repo", ["tasks"] = task, + }); + var tasksFirst = PromptFiles.RenderTemplate(template, new Dictionary + { + ["tasks"] = task, ["repo"] = "C:\\repo", ["scope"] = "List: Bugs", + }); + + Assert.Equal(tasksLast, tasksFirst); + Assert.Contains("- Document {scope} and {repo} tokens", tasksLast); + } + [Fact] public void DefaultFor_system_mentions_blocked_marker_and_scope() {