Merge pull request #316 from metaobjectsdev/fix/cross-port-source-resolution-review

fix: close 15 findings from the cross-port source-resolution review

Author: dmealing

fixtures/source-resolution-conformance/README.md

@@ -93,9 +93,36 @@ README.md
   loop) reports whichever error comes first in declaration order instead,
   diverging on exactly one of the two cases depending on which order it
   happens to process first.
-- **Unknown top-level config keys are IGNORED.** The file carries
-  TypeScript-owned keys no other port models. `schema_version` and `sources` are
-  the neutral subset; each port validates those strictly and ignores the rest.
+- **A TypeScript-owned top-level key does not affect source resolution in any
+  port.** `schema_version` and `sources` are the neutral subset every port
+  models; `pending_in_git` / `confidence_thresholds` / `extract` / `migrate`
+  are TypeScript's own, and `typescript-owned-top-level-keys-do-not-affect-
+  source-resolution` pins that their presence resolves the same file set
+  everywhere. Read that case name literally — it is narrower than "unknown
+  keys are ignored" on purpose. Those four keys are UNKNOWN to Java/C#/Python
+  (which ignore any key outside `schema_version`/`sources`, by design) but
+  KNOWN to TypeScript's own `ConfigSchema` (`sdk/src/config.ts`), which
+  recognizes and validates them as part of its own project state. A case
+  built only from keys TS recognizes cannot tell "TS ignored this because it
+  doesn't affect resolution" apart from "TS ignored this because it doesn't
+  affect resolution AND happened to also validate it" — the two are
+  indistinguishable from the outside, and only the first is what every other
+  port's "ignore the rest" behavior demonstrates.
+  **A genuinely unrecognized key (e.g. `"foo": 1`, unknown to all four ports)
+  is a real, confirmed, cross-port DIVERGENCE, not covered by this corpus.**
+  Verified empirically: `resolveCollection` (`collection.ts`) calls
+  `loadConfig`, which parses the WHOLE file through `ConfigSchema.parse` —
+  `.strict()` at the top level (`config.ts`) — so a key no version of
+  TypeScript has ever declared throws a `ZodError` and resolution never
+  reaches the source-listing step at all, while Java/C#/Python all resolve
+  successfully, silently ignoring it. Not added as a shared `expectFiles`
+  case here because doing so would need EITHER loosening `ConfigSchema`'s
+  top-level strictness (a reference-implementation behavior change with a
+  blast radius well beyond source resolution — every `loadConfig` caller,
+  not just this corpus) OR asserting a `true`-sentinel `expectError` that
+  TypeScript alone would satisfy, contradicting the other three ports'
+  actual success — neither of which this corpus is positioned to decide
+  unilaterally. Left as an open, human-reviewable follow-up.
 
 ## Order is deliberately NOT pinned
 

fixtures/source-resolution-conformance/cases.json

@@ -178,7 +178,7 @@
       "expectError": "ERR_COLLECTION_NOT_FOUND"
     },
     {
-      "name": "unknown-top-level-keys-are-ignored",
+      "name": "typescript-owned-top-level-keys-do-not-affect-source-resolution",
       "tree": {
         "model/meta.a.json": "{\"metadata.root\":{\"children\":[]}}"
       },

server/csharp/MetaObjects.Cli.Tests/MetadataDirFallbackTests.cs

@@ -131,6 +131,45 @@ public void Gen_with_no_positional_metadataDir_and_a_single_FILE_source_refuses_
         Assert.False(Directory.Exists(outDir));
     }
 
+    [Fact]
+    public void Gen_with_no_positional_metadataDir_excludes_pending_drafts()
+    {
+        // F5 — the ladder path used to discard SourceResolver.ResolveSources's own
+        // return value (the `_pending`-excluded file list) and hand
+        // MetaDataLoader.FromDirectory a bare directory instead, whose default
+        // DirectorySource.Options has ExcludePending = false — so a `_pending/`
+        // draft that TS/Java/Python all keep invisible to codegen leaked into the
+        // generated output here. `_pending/` is excluded at ANY depth under the
+        // declared source, matching the other three ports.
+        var modelDir = Path.Combine(_tmp, "model");
+        Directory.CreateDirectory(modelDir);
+        File.WriteAllText(Path.Combine(modelDir, "meta.acme.json"), Metadata);
+        var pendingDir = Path.Combine(modelDir, "_pending");
+        Directory.CreateDirectory(pendingDir);
+        File.WriteAllText(Path.Combine(pendingDir, "meta.draft.json"), """
+        { "metadata.root": { "package": "acme", "children": [
+          { "object.entity": { "name": "DraftWidget", "children": [
+            { "source.rdb": { "@table": "draft_widgets" } },
+            { "field.long": { "name": "id" } },
+            { "identity.primary": { "@fields": "id" } }
+          ]}}
+        ]}}
+        """);
+        var cfgDir = Path.Combine(_tmp, ".metaobjects");
+        Directory.CreateDirectory(cfgDir);
+        File.WriteAllText(
+            Path.Combine(cfgDir, "config.json"),
+            """{ "schema_version": 1, "sources": [ { "path": "model" } ] }""");
+
+        var outDir = Path.Combine(_tmp, "generated");
+        var (exitCode, stdout, stderr) = RunCli(_tmp, "gen", "--out", outDir, "--namespace", "Acme.Generated");
+
+        Assert.True(exitCode == 0, $"exit={exitCode}\nstdout={stdout}\nstderr={stderr}");
+        Assert.True(File.Exists(Path.Combine(outDir, "Subscriber.g.cs")), stdout + stderr);
+        Assert.False(File.Exists(Path.Combine(outDir, "DraftWidget.g.cs")),
+            "a _pending/ draft must never reach generated output: " + stdout + stderr);
+    }
+
     [Fact]
     public void Gen_with_an_explicit_positional_metadataDir_is_unaffected()
     {

server/csharp/MetaObjects.Cli/DocsCommand.cs

@@ -41,8 +41,20 @@ public sealed record Outcome(
     public static Outcome Run(
         string metadataDir, string outDir, string project, string ns,
         string apiSubDir = DefaultApiSubDir, string? modelBaseUrl = null)
+        => Run(MetaDataLoader.FromDirectory(metadataDir), outDir, project, ns, apiSubDir, modelBaseUrl);
+
+    /// <summary>
+    /// Same as the <c>metadataDir</c> overload above, but starting from an
+    /// ALREADY-LOADED <paramref name="load"/> — see the identical overload on
+    /// <see cref="GenCommand"/> for why (the CLI's config-ladder path resolves +
+    /// loads once via <c>MetaDataLoader.FromUris</c>, correctly excluding
+    /// <c>_pending</c> drafts; a second <c>FromDirectory</c> call here would both
+    /// re-walk the tree and silently lose that exclusion).
+    /// </summary>
+    public static Outcome Run(
+        LoadResult load, string outDir, string project, string ns,
+        string apiSubDir = DefaultApiSubDir, string? modelBaseUrl = null)
     {
-        var load = MetaDataLoader.FromDirectory(metadataDir);
         var loadErrors = load.Errors.Select(e => e.Code.ToString()).ToList();
         if (loadErrors.Count > 0)
             return new Outcome(loadErrors, []);

server/csharp/MetaObjects.Cli/GenCommand.cs

@@ -68,8 +68,24 @@ public static Outcome Run(string metadataDir, string outDir, string ns, bool emi
     public static Outcome Run(
         string metadataDir, string outDir, string ns, bool emitAbstractShapes,
         IReadOnlyList<string>? generatorNames, string? templateRoot, string? templateSpecPath = null)
+        => Run(MetaDataLoader.FromDirectory(metadataDir), outDir, ns, emitAbstractShapes,
+            generatorNames, templateRoot, templateSpecPath);
+
+    /// <summary>
+    /// Same as the <c>metadataDir</c> overload above, but starting from an
+    /// ALREADY-LOADED <paramref name="load"/> — used by the CLI's
+    /// <c>.metaobjects/config.json</c> ladder path (<c>Program.cs</c>'s
+    /// <c>ResolveMetadataDirOrExit</c>), which resolves AND loads the declared
+    /// source set itself via <see cref="MetaDataLoader.FromUris(System.Collections.Generic.IReadOnlyList{Uri})"/>
+    /// (honoring the <c>_pending</c>-draft exclusion <c>SourceResolver</c> applies).
+    /// Calling <see cref="MetaDataLoader.FromDirectory(string, DirectorySource.Options?, bool)"/>
+    /// again here would re-walk the directory tree a second time AND silently lose
+    /// that exclusion (<c>FromDirectory</c>'s own default is to include <c>_pending</c>).
+    /// </summary>
+    public static Outcome Run(
+        LoadResult load, string outDir, string ns, bool emitAbstractShapes,
+        IReadOnlyList<string>? generatorNames, string? templateRoot, string? templateSpecPath = null)
     {
-        var load = MetaDataLoader.FromDirectory(metadataDir);
         var loadErrors = load.Errors.Select(e => e.Code.ToString()).ToList();
         if (loadErrors.Count > 0)
             return new Outcome(loadErrors, null);

server/csharp/MetaObjects.Cli/Program.cs

@@ -88,7 +88,7 @@ static int RunGen(string[] rest)
 
     // Rung 1 (explicit positional) is honored as-is; an omitted metadataDir
     // falls back to the port-neutral .metaobjects/config.json ladder.
-    metadataDir = ResolveMetadataDirOrExit(metadataDir);
+    var resolvedMeta = ResolveMetadataDirOrExit(metadataDir);
 
     // Advisory: nudge a re-scaffold if the copied-in agent context predates this build.
     // Never throws, never changes the exit code (a missing/corrupt manifest is ignored).
@@ -97,7 +97,16 @@ static int RunGen(string[] rest)
     var generatorNames = generatorsCsv
         ?.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
 
-    var outcome = GenCommand.Run(metadataDir, outDir, ns, emitAbstractShapes, generatorNames, templateRoot, templateSpecPath);
+    // A ladder-resolved (non-null Files) source loads via the already-resolved,
+    // `_pending`-excluded file list (MetaDataLoader.FromUris) — never a second
+    // FromDirectory walk of resolvedMeta.Directory, which would both duplicate
+    // the walk ResolveMetadataDirOrExit already did AND silently include `_pending`.
+    var outcome = resolvedMeta.Files is { } files
+        ? GenCommand.Run(
+            MetaObjects.Loader.MetaDataLoader.FromUris(files.Select(f => new Uri(f)).ToList()),
+            outDir, ns, emitAbstractShapes, generatorNames, templateRoot, templateSpecPath)
+        : GenCommand.Run(
+            resolvedMeta.Directory, outDir, ns, emitAbstractShapes, generatorNames, templateRoot, templateSpecPath);
     if (!outcome.Ok)
     {
         foreach (var e in outcome.LoadErrors) Console.Error.WriteLine($"  load error: {e}");
@@ -141,13 +150,20 @@ static int RunDocs(string[] rest)
 
     // Rung 1 (explicit positional) is honored as-is; an omitted metadataDir
     // falls back to the port-neutral .metaobjects/config.json ladder.
-    metadataDir = ResolveMetadataDirOrExit(metadataDir);
+    var resolvedMeta = ResolveMetadataDirOrExit(metadataDir);
 
     // Default the project label to the input directory's leaf name (cosmetic — surfaces
     // in the AGENT-API header). Trailing-separator-safe.
-    project ??= new DirectoryInfo(Path.TrimEndingDirectorySeparator(Path.GetFullPath(metadataDir))).Name;
-
-    var outcome = DocsCommand.Run(metadataDir, outDir, project, ns, modelBaseUrl: modelBaseUrl);
+    project ??= new DirectoryInfo(Path.TrimEndingDirectorySeparator(Path.GetFullPath(resolvedMeta.Directory))).Name;
+
+    // See the identical comment in RunGen above: a ladder-resolved source loads
+    // via its already-resolved, `_pending`-excluded file list, never a second
+    // (unfiltered) directory walk.
+    var outcome = resolvedMeta.Files is { } files
+        ? DocsCommand.Run(
+            MetaObjects.Loader.MetaDataLoader.FromUris(files.Select(f => new Uri(f)).ToList()),
+            outDir, project, ns, modelBaseUrl: modelBaseUrl)
+        : DocsCommand.Run(resolvedMeta.Directory, outDir, project, ns, modelBaseUrl: modelBaseUrl);
     if (!outcome.Ok)
     {
         foreach (var e in outcome.LoadErrors) Console.Error.WriteLine($"  load error: {e}");
@@ -174,13 +190,27 @@ static int RunDocs(string[] rest)
 // docs, verify) so an omitted positional argument is never a hard requirement
 // wherever a project's config can name the location instead.
 //
-// Never returns null: either hands back a real directory, or prints a
-// diagnostic and terminates the process — callers may treat the result as
-// always-present and keep their existing (now-unreachable-when-omitted)
-// null checks for the OTHER positional/option they still require.
-static string ResolveMetadataDirOrExit(string? metadataDir)
+// The metadata-location ladder's result: always a directory (explicit-arg
+// back-compat, and cosmetic labeling even on the ladder path), and — when
+// resolution went through the .metaobjects/config.json ladder rather than an
+// explicit CLI argument — the ladder's OWN already-resolved, `_pending`-draft-
+// excluded file list too. A caller with a non-null Files must load via
+// MetaDataLoader.FromUris(Files) rather than FromDirectory(Directory): the
+// latter would both re-walk a tree this function already walked once (via
+// SourceResolver) AND silently lose the `_pending` exclusion, since
+// DirectorySource.Options.ExcludePending defaults to false at the loader
+// level (SourceResolver is the one place that turns it on). Declared at file
+// scope below the entry point (top-level-statement files require type
+// declarations to follow every top-level statement / local function).
+
+// Never exits without a usable result: either hands back a real directory
+// (+ file list, when ladder-resolved), or prints a diagnostic and terminates
+// the process — callers may treat the result as always-present and keep
+// their existing (now-unreachable-when-omitted) null checks for the OTHER
+// positional/option they still require.
+static ResolvedMetadata ResolveMetadataDirOrExit(string? metadataDir)
 {
-    if (metadataDir is not null) return metadataDir;
+    if (metadataDir is not null) return new ResolvedMetadata(metadataDir, null);
 
     var cwd = Directory.GetCurrentDirectory();
     try
@@ -190,11 +220,13 @@ static string ResolveMetadataDirOrExit(string? metadataDir)
 
         if (specs.Count == 0)
         {
-            // No declared sources — validate + apply the DEFAULT directory through
+            // No declared sources — resolve + apply the DEFAULT directory through
             // the same ladder the shared conformance corpus gates (raises
-            // ERR_COLLECTION_NOT_FOUND when the default is also absent).
-            _ = MetaObjects.Config.SourceResolver.ResolveCollection(cwd);
-            return Path.Combine(cwd, MetaObjects.Config.NeutralConfig.DefaultMetadataDir);
+            // ERR_COLLECTION_NOT_FOUND when the default is also absent). The
+            // returned file list IS the load — no second walk needed.
+            var defaultFiles = MetaObjects.Config.SourceResolver.ResolveCollection(cwd);
+            return new ResolvedMetadata(
+                Path.Combine(cwd, MetaObjects.Config.NeutralConfig.DefaultMetadataDir), defaultFiles);
         }
 
         if (specs.Count > 1)
@@ -213,9 +245,9 @@ static string ResolveMetadataDirOrExit(string? metadataDir)
 
         // Exactly one declared source. Resolve + validate it through the same
         // kind/existence checks ResolveSources applies (ERR_SOURCE_KIND_UNSUPPORTED /
-        // ERR_SOURCE_UNRESOLVED), then hand the loader that spec's OWN root — never
-        // the default directory name, which this project may not even have.
-        MetaObjects.Config.SourceResolver.ResolveSources(cwd, specs);
+        // ERR_SOURCE_UNRESOLVED) — its return value IS the (already `_pending`-
+        // excluded) file list to load, not just a validation signal to discard.
+        var files = MetaObjects.Config.SourceResolver.ResolveSources(cwd, specs);
         var rawPath = specs[0]["path"]; // guaranteed present: ResolveSources above
                                          // would already have thrown otherwise.
         var resolved = Path.IsPathRooted(rawPath) ? rawPath : Path.GetFullPath(Path.Combine(cwd, rawPath));
@@ -235,7 +267,7 @@ static string ResolveMetadataDirOrExit(string? metadataDir)
             throw new InvalidOperationException("unreachable");
         }
 
-        return resolved;
+        return new ResolvedMetadata(resolved, files);
     }
     catch (MetaObjects.MetaModelException e)
     {
@@ -306,7 +338,7 @@ static int RunVerify(string[] rest)
 
     // Rung 1 (explicit positional) is honored as-is; an omitted metadataDir
     // falls back to the port-neutral .metaobjects/config.json ladder.
-    metadataDir = ResolveMetadataDirOrExit(metadataDir);
+    var resolvedMeta = ResolveMetadataDirOrExit(metadataDir);
 
     // The templates gate needs a root. Bare verify (defaults to templates) and an
     // explicit --templates both require it; surface a clear usage error if absent.
@@ -328,7 +360,11 @@ static int RunVerify(string[] rest)
 
     var opts = new VerifyCommand.Options
     {
-        MetadataDir = metadataDir,
+        MetadataDir = resolvedMeta.Directory,
+        // A ladder-resolved source loads via this already-resolved,
+        // `_pending`-excluded file list (see VerifyCommand.LoadMetadata) — never a
+        // second (unfiltered) directory walk of MetadataDir.
+        MetadataFiles = resolvedMeta.Files,
         TemplatesRoot = templatesRoot,
         OutDir = outDir,
         Namespace = ns,
@@ -390,3 +426,6 @@ static int RunVerify(string[] rest)
 
     return result.ExitCode;
 }
+
+// See the doc comment on ResolveMetadataDirOrExit above.
+readonly record struct ResolvedMetadata(string Directory, IReadOnlyList<string>? Files);