Skip to content

Commit 08c4cb7

Browse files
dmealingclaude
dmealing
and
claude
committed
fix(csharp): verify --codegen never saw the template-spec, so it convicted gen's own output
The C# half of the same defect fixed for Python in the previous commit. SP-1 §4 specified "--template-spec <path>, with a conventional default the port auto-discovers and the flag overriding", and that BOTH gen and verify read it. Only the flag on `gen` was built: VerifyCommand.RunCodegenDrift resolved its own list via GeneratorRegistry.Resolve and never looked for a spec, so a project that had just run `dotnet meta gen` failed its own drift gate — every spec-emitted file reported as "committed but a fresh regen would not emit it". The printed remedy loops. Regenerating cannot produce files the regen does not know about, and `verify` takes no --template-spec flag to be told about them. It lands on one of the two ports where the declarative spec is the ONLY consumer authoring path (the generator registry is closed), so an adopter doing the one thing their port supports fails their own gate. THE FIX is a shared resolver, not a second flag. GenCommand gains TemplateSpecPathFor + TemplateSpecGenerators; Run and RunCodegenDrift both go through them. The explicit flag wins and is used verbatim (a flag naming a missing file stays a hard error — silently ignoring a path the user typed would be worse); otherwise <projectRoot>/template-spec.json is used if present. `verify` has no flag, so discovery is the whole mechanism there — which is the point: a flag on verify would have to be repeated at every CI call site, and one that is forgotten reproduces the bug exactly. projectRoot is ProjectRootFor(metadataDir) — the metadata dir's parent. Not a new rule: it is already where the .gen-state manifest is anchored, and it is what the Python port uses. Both ports now agree on the filename AND the anchor, which is what makes one spec file work unchanged on either. The catch around generator construction widens from ArgumentException to the same set GenCommand.Run already handles (IOException / UnauthorizedAccessException / JsonException), since verify can now hit the spec-reading paths that throw them — a malformed spec must come back as `verify --codegen: <message>`, not an unhandled throw. TWO DIFFERENCES FROM THE PYTHON PORT, both verified rather than assumed: - C# does NOT have Python's second defect. Its CodegenDrift.ListFiles already enumerates "*", so non-.cs template output was always compared. Python's globbed "*.py", which made a deleted AND a corrupted template file report "in sync". - C# has NO jurisdiction guard on its `inCommitted && !inFresh` branch, so it convicts files it never wrote — the pre-0.24.3 TypeScript behaviour. That is a real and separate defect, it is NOT introduced here, and fixing it changes verify's verdict for every existing C# adopter, so it is reported rather than folded in. The discriminating test is the one that matters: the lazy fix for a gate that convicts its own output is to make it ignore what it does not recognise, which fixes the symptom by blinding the gate. So deleting a spec-generated file, and separately editing one, must both still fail. Plus: discovery works, the flag REPLACES discovery rather than adding to it, no spec file leaves behaviour unchanged, and a malformed discovered spec fails loudly (silently skipping it would put gen and verify back out of agreement, which is the whole defect). Proven by breaking: reverting both files (file copy, not stash) fails 4 of the 7; restoring passes 7. Full C# suite green across all four test projects — Cli 72, Render 291, Conformance 994, Codegen 374 (+1 skipped), and the project count on disk was checked against the count that reported, since `dotnet test` prints Passed! for a project that never built. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HSCgs7z19w8aGXGiceCohC
1 parent 5fbe5b1 commit 08c4cb7

3 files changed

Lines changed: 221 additions & 9 deletions

File tree

‎server/csharp/MetaObjects.Cli.Tests/VerifyTemplateSpecTests.cs‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
using MetaObjects.Cli;
2+
using Xunit;
3+
4+
namespace MetaObjects.Cli.Tests;
5+
6+
/// <summary>
7+
/// `dotnet meta verify --codegen` must regenerate with the SAME declarative template
8+
/// generators `gen` ran, or it convicts their committed output as stale.
9+
///
10+
/// SP-1 §4 specified "--template-spec &lt;path&gt;, with a conventional default the port
11+
/// auto-discovers and the flag overriding", and that BOTH gen and verify read it. Only
12+
/// the flag on `gen` was ever built: VerifyCommand resolved its own generator list via
13+
/// GeneratorRegistry.Resolve and never looked for a spec. The result was a drift gate
14+
/// failing a project that had just run `gen`, with a remedy that loops — regenerating
15+
/// cannot produce files the regen does not know about, and `verify` takes no
16+
/// --template-spec flag to be told.
17+
///
18+
/// The conventional path is &lt;projectRoot&gt;/template-spec.json, where projectRoot is
19+
/// <see cref="GenCommand.ProjectRootFor"/> — the metadata dir's parent, the same anchor
20+
/// the .gen-state manifest already uses.
21+
/// </summary>
22+
public sealed class VerifyTemplateSpecTests : IDisposable
23+
{
24+
private readonly string _tmp = Path.Combine(Path.GetTempPath(), "meta-vtspec-" + Guid.NewGuid().ToString("N"));
25+
private string MetaDir => Path.Combine(_tmp, "metaobjects");
26+
private string OutDir => Path.Combine(_tmp, "generated");
27+
private string TemplateRoot => Path.Combine(_tmp, "templates");
28+
private string DiscoveredSpec => Path.Combine(_tmp, "template-spec.json");
29+
30+
private const string Metadata = """
31+
{ "metadata.root": { "package": "acme", "children": [
32+
{ "object.entity": { "name": "Widget", "children": [
33+
{ "source.rdb": { "@table": "widgets" } },
34+
{ "field.long": { "name": "id" } },
35+
{ "field.string": { "name": "name", "@required": true } },
36+
{ "identity.primary": { "@fields": "id" } }
37+
]}}
38+
]}}
39+
""";
40+
41+
public VerifyTemplateSpecTests()
42+
{
43+
Directory.CreateDirectory(MetaDir);
44+
Directory.CreateDirectory(TemplateRoot);
45+
File.WriteAllText(Path.Combine(MetaDir, "meta.acme.json"), Metadata);
46+
File.WriteAllText(Path.Combine(TemplateRoot, "summary.mustache"), "name={{name}} pkg={{package}}\n");
47+
File.WriteAllText(DiscoveredSpec, """
48+
{ "generators": [
49+
{ "name": "summary", "template": "summary", "scope": "perEntity", "outputPattern": "{name}.summary.txt" }
50+
] }
51+
""");
52+
}
53+
54+
public void Dispose() { try { Directory.Delete(_tmp, recursive: true); } catch { } }
55+
56+
private VerifyCommand.Options CodegenOpts() => new()
57+
{
58+
MetadataDir = MetaDir,
59+
TemplatesRoot = TemplateRoot,
60+
TemplateRoot = TemplateRoot,
61+
OutDir = OutDir,
62+
Templates = false,
63+
Codegen = true,
64+
Db = false,
65+
};
66+
67+
private void Gen() =>
68+
Assert.True(
69+
GenCommand.Run(MetaDir, OutDir, "Acme.Generated", emitAbstractShapes: false,
70+
generatorNames: null, templateRoot: TemplateRoot).Ok);
71+
72+
[Fact]
73+
public void Gen_auto_discovers_the_conventional_spec()
74+
{
75+
Gen();
76+
Assert.True(File.Exists(Path.Combine(OutDir, "Widget.summary.txt")),
77+
"gen did not discover <projectRoot>/template-spec.json");
78+
}
79+
80+
[Fact]
81+
public void Verify_codegen_is_clean_right_after_gen()
82+
{
83+
Gen();
84+
// Non-vacuous: without discovery there is nothing here for verify to convict.
85+
Assert.True(File.Exists(Path.Combine(OutDir, "Widget.summary.txt")));
86+
87+
var r = VerifyCommand.RunSubverbs(CodegenOpts());
88+
Assert.Equal(0, r.ExitCode);
89+
}
90+
91+
[Fact]
92+
public void Verify_codegen_still_catches_a_deleted_template_file()
93+
{
94+
// THE DISCRIMINATING TEST. The lazy fix is to make verify ignore what it does
95+
// not recognise, which fixes the symptom by blinding the gate.
96+
Gen();
97+
File.Delete(Path.Combine(OutDir, "Widget.summary.txt"));
98+
99+
var r = VerifyCommand.RunSubverbs(CodegenOpts());
100+
Assert.NotEqual(0, r.ExitCode);
101+
}
102+
103+
[Fact]
104+
public void Verify_codegen_still_catches_an_edited_template_file()
105+
{
106+
Gen();
107+
File.WriteAllText(Path.Combine(OutDir, "Widget.summary.txt"), "tampered\n");
108+
109+
var r = VerifyCommand.RunSubverbs(CodegenOpts());
110+
Assert.NotEqual(0, r.ExitCode);
111+
}
112+
113+
[Fact]
114+
public void No_spec_file_leaves_behaviour_unchanged()
115+
{
116+
File.Delete(DiscoveredSpec);
117+
Gen();
118+
Assert.False(File.Exists(Path.Combine(OutDir, "Widget.summary.txt")));
119+
120+
var r = VerifyCommand.RunSubverbs(CodegenOpts());
121+
Assert.Equal(0, r.ExitCode);
122+
}
123+
124+
[Fact]
125+
public void An_explicit_spec_path_overrides_the_discovered_one()
126+
{
127+
var other = Path.Combine(_tmp, "other-spec.json");
128+
File.WriteAllText(other, """
129+
{ "generators": [
130+
{ "name": "flagged", "template": "summary", "scope": "perEntity", "outputPattern": "{name}.flagged.txt" }
131+
] }
132+
""");
133+
134+
Assert.True(
135+
GenCommand.Run(MetaDir, OutDir, "Acme.Generated", emitAbstractShapes: false,
136+
generatorNames: null, templateRoot: TemplateRoot, templateSpecPath: other).Ok);
137+
138+
Assert.True(File.Exists(Path.Combine(OutDir, "Widget.flagged.txt")), "the flag's spec did not run");
139+
Assert.False(File.Exists(Path.Combine(OutDir, "Widget.summary.txt")), "the flag must REPLACE discovery");
140+
}
141+
142+
[Fact]
143+
public void A_malformed_discovered_spec_is_a_clean_error()
144+
{
145+
File.WriteAllText(DiscoveredSpec, "{ not json");
146+
147+
var outcome = GenCommand.Run(MetaDir, OutDir, "Acme.Generated", emitAbstractShapes: false,
148+
generatorNames: null, templateRoot: TemplateRoot);
149+
150+
// Must fail loudly: silently skipping a broken spec puts gen and verify back
151+
// out of agreement, which is the defect this whole change exists to remove.
152+
Assert.False(outcome.Ok);
153+
}
154+
}

‎server/csharp/MetaObjects.Cli/GenCommand.cs‎

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,56 @@ public static Outcome Run(
8383
public static string ProjectRootFor(string metadataDir) =>
8484
Path.GetDirectoryName(Path.GetFullPath(metadataDir)) ?? Directory.GetCurrentDirectory();
8585

86+
/// <summary>
87+
/// The conventional declarative-template-spec file (SP-1 §4), discovered when
88+
/// <c>--template-spec</c> is not passed. VISIBLE and at the project root on
89+
/// purpose: this project splits author input (visible) from tool state (hidden
90+
/// <c>.metaobjects/</c>), a spec is authored rather than generated, and it belongs
91+
/// beside the <c>templates/</c> dir its refs resolve under. Same name and same
92+
/// anchor as the Python port.
93+
/// </summary>
94+
public const string TemplateSpecFileName = "template-spec.json";
95+
96+
/// <summary>
97+
/// The template-spec to use, or <c>null</c> for "no template generators".
98+
/// An explicit <c>--template-spec</c> always wins and is used verbatim (a flag
99+
/// naming a missing file stays a hard error at read time — silently ignoring a
100+
/// path the user typed would be worse). Otherwise
101+
/// <c>&lt;projectRoot&gt;/template-spec.json</c> is used IF it exists.
102+
///
103+
/// <para>EVERY path that builds a generator list must call this. The defect it
104+
/// exists to prevent is <c>gen</c> and <c>verify --codegen</c> resolving the spec
105+
/// differently: <c>gen</c> honoured the flag while <c>verify</c> built its own list
106+
/// and never looked, so verify regenerated WITHOUT the template generators and
107+
/// reported their committed output as stale — with a remedy that loops, since
108+
/// regenerating cannot produce files the regen does not know about.</para>
109+
/// </summary>
110+
public static string? TemplateSpecPathFor(string? projectRoot, string? explicitPath)
111+
{
112+
if (!string.IsNullOrEmpty(explicitPath)) return explicitPath;
113+
if (string.IsNullOrEmpty(projectRoot)) return null;
114+
var candidate = Path.Combine(projectRoot, TemplateSpecFileName);
115+
return File.Exists(candidate) ? candidate : null;
116+
}
117+
118+
/// <summary>
119+
/// The declarative Mustache generators for this project — empty when there is no
120+
/// spec at all. Throws the same exception set <see cref="Run(LoadResult, string,
121+
/// string, bool, IReadOnlyList{string}?, string?, string?, string?,
122+
/// ColumnNamingStrategy)"/> already catches, so a malformed spec surfaces as a
123+
/// clean error rather than an unhandled throw.
124+
/// </summary>
125+
public static IReadOnlyList<IGenerator> TemplateSpecGenerators(
126+
string? projectRoot, string? explicitPath, string? templateRoot)
127+
{
128+
var specPath = TemplateSpecPathFor(projectRoot, explicitPath);
129+
if (specPath is null) return [];
130+
using var doc = JsonDocument.Parse(File.ReadAllText(specPath));
131+
var spec = TemplateSpec.Parse(doc.RootElement);
132+
var provider = new FilesystemProvider(templateRoot ?? "templates");
133+
return TemplateSpec.ToGenerators(spec, provider).ToList();
134+
}
135+
86136
/// <summary>
87137
/// Same as the <c>metadataDir</c> overload above, but starting from an
88138
/// ALREADY-LOADED <paramref name="load"/> — used by the CLI's
@@ -109,13 +159,9 @@ public static Outcome Run(
109159
try
110160
{
111161
generators = GeneratorRegistry.Resolve(names, new GeneratorBuildContext(templateRoot)).ToList();
112-
if (!string.IsNullOrEmpty(templateSpecPath))
113-
{
114-
using var doc = JsonDocument.Parse(File.ReadAllText(templateSpecPath));
115-
var spec = TemplateSpec.Parse(doc.RootElement);
116-
var provider = new FilesystemProvider(templateRoot ?? "templates");
117-
generators.AddRange(TemplateSpec.ToGenerators(spec, provider));
118-
}
162+
// Explicit flag, else the conventional <projectRoot>/template-spec.json.
163+
// Resolved through the SAME helper verify uses — that shared call is the fix.
164+
generators.AddRange(TemplateSpecGenerators(projectRoot, templateSpecPath, templateRoot));
119165
}
120166
catch (Exception ex) when (ex is ArgumentException or IOException or UnauthorizedAccessException or JsonException)
121167
{

‎server/csharp/MetaObjects.Cli/VerifyCommand.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
//
1515
// Runs at the last fixed point before serve, never on the request path.
1616

17+
using System.Text.Json;
1718
using MetaObjects.Codegen;
1819
using MetaObjects.Loader;
1920
using MetaObjects.Render;
@@ -238,9 +239,20 @@ private static Codegen.CodegenDrift.Result RunCodegenDrift(Options opts)
238239
IReadOnlyList<IGenerator> generators;
239240
try
240241
{
241-
generators = GeneratorRegistry.Resolve(names, new GeneratorBuildContext(opts.TemplateRoot));
242+
var resolved = GeneratorRegistry
243+
.Resolve(names, new GeneratorBuildContext(opts.TemplateRoot))
244+
.ToList();
245+
// The declarative template generators, resolved by the SAME rule `gen` uses.
246+
// Without this, verify regenerated only the built-in suite and then convicted
247+
// every spec-emitted file as "committed but a fresh regen would not emit it" —
248+
// on a tree `gen` had just produced, with a remedy that loops. `verify` takes
249+
// no --template-spec flag, so discovery is the whole mechanism here.
250+
resolved.AddRange(GenCommand.TemplateSpecGenerators(
251+
GenCommand.ProjectRootFor(opts.MetadataDir), null, opts.TemplateRoot));
252+
generators = resolved;
242253
}
243-
catch (ArgumentException ex)
254+
catch (Exception ex) when (ex is ArgumentException or IOException
255+
or UnauthorizedAccessException or JsonException)
244256
{
245257
return new Codegen.CodegenDrift.Result { Clean = false, Error = $"verify --codegen: {ex.Message}" };
246258
}

0 commit comments

Comments
 (0)