Skip to content

Commit 5a1250e

Browse files
authored
Merge pull request #319 from metaobjectsdev/fix/init-print-only-and-csharp-symlink-cycle
fix: close the three open cross-port source-resolution items (print-only writes, C# symlink cycle, strict-config ruling)
2 parents 7142f7f + f3ba965 commit 5a1250e

14 files changed

Lines changed: 584 additions & 61 deletions

File tree

‎fixtures/source-resolution-conformance/README.md‎

Lines changed: 78 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ are read, `scope` filters what is emitted from them.
1111
## Shape
1212

1313
```
14-
cases.json # { cases: [{ name, tree, symlinks?, config, resolveFrom?, expectFiles?, expectError? }] }
14+
cases.json # { cases: [{ name, tree, symlinks?, config, resolveFrom?, expectFiles?, expectError?, errorIsNative? }] }
1515
README.md
1616
```
1717

@@ -31,6 +31,36 @@ README.md
3131
`real/…`) — see "Order is deliberately NOT pinned" below for the parallel
3232
point about `expectFiles` being exact strings, not just "the same underlying
3333
file by any name". Every port's runner must honor this key.
34+
35+
Following symlinks is what makes a symlink CYCLE reachable, so
36+
`a-symlink-cycle-is-an-error` gates the other half of the same contract: a
37+
directory symlink that revisits a directory already on the current walk
38+
branch must RAISE, not be walked. The failure it exists to catch is not a
39+
hang — it is silent nonsense. Left unguarded, a walk yields the same real
40+
file at ever-deeper phantom paths (`model/loop/model/loop/…/meta.a.json`),
41+
and because de-duplication keys on the LEXICAL path those are all distinct,
42+
so each is admitted as its own source and the same metadata loads over and
43+
over. Which error is raised is deliberately not pinned (hence `expectError:
44+
true`) — the ports raise their own native types. What is pinned is that a
45+
cycle is loud.
46+
47+
"Already on the current walk BRANCH" is the precise rule, not "already
48+
seen": the ancestor set must be carried down the recursion and never shared
49+
between siblings, so a directory legitimately reachable by two different
50+
symlinked paths — a diamond, not a cycle — still resolves rather than being
51+
falsely rejected.
52+
53+
This case is a FLOOR, and deliberately so. On Linux the kernel refuses to
54+
traverse past its own symlink-resolution depth (ELOOP around 40 levels), so
55+
an unguarded walk in some runtimes raises anyway — late, and for the wrong
56+
reason, but it raises, and `expectError: true` cannot tell that apart from a
57+
real guard. What the case DOES discriminate is a port that swallows that
58+
kernel error and reports success: C#'s `Directory.EnumerateFiles(...,
59+
AllDirectories)` defaults to `IgnoreInaccessible`, so before this case it
60+
completed normally and returned 41 phantom copies of one file. Because the
61+
floor cannot pin immediacy, each port additionally owns a unit test that the
62+
raise happens on REVISIT rather than at the kernel's limit — `sources.test.ts`,
63+
`test_sources.py`, `DirectorySourceTest.java`, `DirectorySourceTests.cs`.
3464
- **`config`** — written verbatim to `.metaobjects/config.json`, under the
3565
directory named by `resolveFrom` (project root when `resolveFrom` is absent).
3666
When `null`, no config file is created at all.
@@ -48,10 +78,26 @@ README.md
4878
`resolveFrom`), compared as an **UNORDERED SET**. See "Order is
4979
deliberately not pinned" below.
5080
- **`expectError`** — either a STRING error code the resolution must fail with
51-
exactly, or the literal `true` meaning "must raise, but which error/code is
52-
deliberately not pinned across ports" (see "Also deliberately NOT pinned:
53-
the malformed-config error code" below for why the latter form exists).
54-
Exactly one of `expectFiles` / `expectError` is present per case.
81+
exactly, or the literal `true` meaning "must raise the port's own coded
82+
metadata error, but WHICH code is deliberately not pinned across ports" (see
83+
"Also deliberately NOT pinned: the malformed-config error code" below for why
84+
the latter form exists). Exactly one of `expectFiles` / `expectError` is
85+
present per case. Note what `true` still pins in a port that HAS a coded error
86+
type: the type. Java, C# and Python all require `MetaDataException` /
87+
`MetaModelException` / `ParseError` on this arm, so a malformed config that
88+
crashes with a raw `NullPointerException` fails the case rather than passing
89+
it. (TypeScript alone cannot: it propagates the raw parser error, which is the
90+
whole reason the code is unpinned.)
91+
- **`errorIsNative`** — OPTIONAL boolean, only meaningful beside
92+
`expectError: true`. Means the failure surfaces as a PLATFORM-native error
93+
rather than the port's coded metadata error, so the type requirement above is
94+
lifted for this case alone. Exactly one case sets it:
95+
`a-symlink-cycle-is-an-error`, where the raise comes from the filesystem walk
96+
(`IOException` in C#, `FileSystemLoopException` in Java, `SymlinkLoopError` in
97+
Python) and never passes through a coded-error constructor. It exists so that
98+
admitting that one case did not silently relax the other six `true` cases from
99+
"a coded parse failure" to "anything at all" — which is precisely what a blanket
100+
widening of the runners does, and did.
55101

56102
## Semantics pinned here
57103

@@ -115,14 +161,33 @@ README.md
115161
`.strict()` at the top level (`config.ts`) — so a key no version of
116162
TypeScript has ever declared throws a `ZodError` and resolution never
117163
reaches the source-listing step at all, while Java/C#/Python all resolve
118-
successfully, silently ignoring it. Not added as a shared `expectFiles`
119-
case here because doing so would need EITHER loosening `ConfigSchema`'s
120-
top-level strictness (a reference-implementation behavior change with a
121-
blast radius well beyond source resolution — every `loadConfig` caller,
122-
not just this corpus) OR asserting a `true`-sentinel `expectError` that
123-
TypeScript alone would satisfy, contradicting the other three ports'
124-
actual success — neither of which this corpus is positioned to decide
125-
unilaterally. Left as an open, human-reviewable follow-up.
164+
successfully, silently ignoring it.
165+
166+
**RULED: the asymmetry is INTENDED, and neither side moves.** It is not a
167+
defect that leaked; it falls out of who owns the file. `.metaobjects/config.json`
168+
is TypeScript's, and TypeScript is the only port that models its whole
169+
vocabulary — so it is the only port that CAN tell a typo from a key a sibling
170+
owns. `.strict()` is what converts that knowledge into a diagnostic, and the
171+
hazard is specific and proven: a stripped `scopes` (for `scope`) silently means
172+
"everything in scope", and a stripped `migrate.scopee` silently governs the whole
173+
database. Java, C# and Python know only `schema_version` + `sources`; for them
174+
every other key is indistinguishable from a TS-owned one, so strictness there
175+
could only be imitated by embedding TypeScript's key list and keeping it in
176+
lockstep forever — at which point any port a release behind would REJECT a
177+
config a newer `meta` had just written. Tolerance is the only coherent behaviour
178+
for a partial reader, and strictness the only coherent behaviour for the owner.
179+
180+
So this stays OFF the shared corpus permanently — not deferred. A shared case
181+
asserts ONE outcome, and the correct outcome here differs by port BY DESIGN;
182+
adding one could only be done by making some port wrong. Each half is pinned in
183+
the port that owns it instead: TypeScript's in `sdk/test/config.test.ts`
184+
(unknown top-level key ⇒ throws, alongside the nested `scopes`/`scopee` cases),
185+
and the tolerant half in `SourceResolverTests.cs`,
186+
`tests/config/test_source_resolver.py` and `SourceResolverTest.java`
187+
(unknown top-level key ⇒ resolves normally). The
188+
forward-compatibility cost is real and accepted: a config written by a NEWER
189+
`meta` hard-fails an OLDER one. That is what `schema_version` is for, and a loud
190+
failure beats a `scope` that silently matched everything.
126191

127192
## Order is deliberately NOT pinned
128193

‎fixtures/source-resolution-conformance/cases.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,16 @@
264264
"symlinks": { "model/linked": "external" },
265265
"config": { "schema_version": 1, "sources": [{ "path": "model" }] },
266266
"expectFiles": ["model/meta.top.json", "model/linked/meta.linked.json"]
267+
},
268+
{
269+
"name": "a-symlink-cycle-is-an-error",
270+
"tree": {
271+
"model/meta.a.json": "{\"metadata.root\":{\"children\":[]}}"
272+
},
273+
"symlinks": { "model/loop": "model" },
274+
"config": { "schema_version": 1, "sources": [{ "path": "model" }] },
275+
"expectError": true,
276+
"errorIsNative": true
267277
}
268278
]
269279
}

‎server/csharp/MetaObjects.Conformance.Tests/DirectorySourceTests.cs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,73 @@ public void Expand_HonorsExcludeGlobs()
115115
Directory.Delete(dir, recursive: true);
116116
}
117117
}
118+
119+
/// <summary>
120+
/// The shared corpus (`a-symlink-cycle-is-an-error`) can only pin THAT a cycle
121+
/// raises, because on Linux the kernel's own ELOOP makes an unguarded walk raise
122+
/// eventually too. These two pin what it cannot: that the raise happens on REVISIT,
123+
/// and that it distinguishes a cycle from a diamond.
124+
/// </summary>
125+
[Fact]
126+
public void Expand_RaisesOnRevisit_NotAtTheKernelsSymlinkDepthLimit()
127+
{
128+
string dir = Path.Combine(Path.GetTempPath(), "ds_" + Path.GetRandomFileName());
129+
Directory.CreateDirectory(dir);
130+
try
131+
{
132+
File.WriteAllText(Path.Combine(dir, "meta.a.json"), "{}");
133+
Directory.CreateSymbolicLink(Path.Combine(dir, "loop"), dir);
134+
135+
var ex = Assert.Throws<IOException>(() => new DirectorySource(dir).Expand().ToList());
136+
Assert.Contains("symlink loop detected", ex.Message);
137+
138+
// Immediacy: the message names the FIRST revisit. Before the guard,
139+
// Directory.EnumerateFiles(..., AllDirectories) swallowed the kernel's ELOOP
140+
// (EnumerationOptions.IgnoreInaccessible) and returned ~40 phantom copies of
141+
// meta.a.json with no error at all — so asserting only "it threw" would have
142+
// been satisfied by a walk that had already descended 40 levels.
143+
Assert.Contains(Path.Combine(dir, "loop"), ex.Message);
144+
Assert.DoesNotContain(Path.Combine("loop", "loop"), ex.Message);
145+
}
146+
finally
147+
{
148+
Directory.Delete(dir, recursive: true);
149+
}
150+
}
151+
152+
[Fact]
153+
public void Expand_FollowsADiamond_WhichIsNotACycle()
154+
{
155+
string dir = Path.Combine(Path.GetTempPath(), "ds_" + Path.GetRandomFileName());
156+
Directory.CreateDirectory(dir);
157+
try
158+
{
159+
// shared/ is reachable twice — via one/ and via two/ — but never from
160+
// itself. The ancestor set must therefore be per-BRANCH: a set shared
161+
// across siblings would see the second arrival as a revisit and reject a
162+
// perfectly valid tree.
163+
var shared = Path.Combine(dir, "shared");
164+
Directory.CreateDirectory(shared);
165+
File.WriteAllText(Path.Combine(shared, "meta.shared.json"), "{}");
166+
Directory.CreateDirectory(Path.Combine(dir, "one"));
167+
Directory.CreateDirectory(Path.Combine(dir, "two"));
168+
Directory.CreateSymbolicLink(Path.Combine(dir, "one", "link"), shared);
169+
Directory.CreateSymbolicLink(Path.Combine(dir, "two", "link"), shared);
170+
171+
// One real file, reached by three distinct paths. Compared on FilePath, not
172+
// Id — Id is the bare filename, so all three share it and a distinctness
173+
// check there would pass on a single result just as happily.
174+
var paths = new DirectorySource(dir).Expand().Select(f => f.FilePath).ToList();
175+
176+
Assert.Equal(3, paths.Count);
177+
Assert.Equal(3, paths.Distinct().Count());
178+
Assert.Contains(Path.Combine(dir, "one", "link", "meta.shared.json"), paths);
179+
Assert.Contains(Path.Combine(dir, "two", "link", "meta.shared.json"), paths);
180+
Assert.Contains(Path.Combine(shared, "meta.shared.json"), paths);
181+
}
182+
finally
183+
{
184+
Directory.Delete(dir, recursive: true);
185+
}
186+
}
118187
}

‎server/csharp/MetaObjects.Conformance.Tests/SourceResolutionConformanceTests.cs‎

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,20 @@ private sealed record Case(
2727
// count silently goes stale; the structural description above does not.)
2828
string ResolveFrom,
2929
string[]? ExpectFiles,
30-
// A JSON string pins the exact error code raised; JSON `true` pins only
31-
// that resolution RAISES — the malformed-config error code is
32-
// deliberately not pinned cross-port (see the corpus README).
30+
// A JSON string pins the exact error code raised; JSON `true` leaves the
31+
// CODE unpinned but still requires the port's coded MetaModelException —
32+
// the malformed-config error code is deliberately not pinned cross-port
33+
// (see the corpus README), the TYPE is.
3334
JsonElement? ExpectError,
3435
// Optional: linkPath -> targetPath, both project-root-relative, materialized
3536
// AFTER `Tree` (I1 — a symlinked source root, or a symlinked subdirectory
3637
// inside a walked tree).
37-
Dictionary<string, string> Symlinks);
38+
Dictionary<string, string> Symlinks,
39+
// Optional: this case's failure surfaces as a PLATFORM-native error rather
40+
// than a coded MetaModelException, so the type requirement above is lifted
41+
// for it alone (the symlink-cycle case, whose raise comes from the
42+
// filesystem walk and never reaches a coded-error constructor).
43+
bool ErrorIsNative);
3844

3945
public static TheoryData<string> CaseNames()
4046
{
@@ -78,7 +84,8 @@ private static List<Case> LoadCases()
7884
foreach (var p in sl.EnumerateObject()) symlinks[p.Name] = p.Value.GetString()!;
7985
}
8086

81-
cases.Add(new Case(el.GetProperty("name").GetString()!, tree, cfg, resolveFrom, expectFiles, expectError, symlinks));
87+
bool errorIsNative = el.TryGetProperty("errorIsNative", out var ein) && ein.GetBoolean();
88+
cases.Add(new Case(el.GetProperty("name").GetString()!, tree, cfg, resolveFrom, expectFiles, expectError, symlinks, errorIsNative));
8289
}
8390
return cases;
8491
}
@@ -127,12 +134,21 @@ public void ResolvesTheSameFileSet(string name)
127134

128135
if (c.ExpectError is not null)
129136
{
130-
var ex = Assert.ThrowsAny<MetaModelException>(() => SourceResolver.ResolveCollection(invokeDir));
131-
// A string pins the exact code; `true` only pins that it raises —
132-
// see the ExpectError field doc above.
133-
if (c.ExpectError.Value.ValueKind == JsonValueKind.String)
137+
// A string pins the exact code. `true` leaves the CODE unpinned but
138+
// still requires MetaModelException — a malformed config that crashes
139+
// with a raw NullReferenceException must FAIL this case, not pass it.
140+
// `ErrorIsNative` lifts only the type requirement, for the one case
141+
// whose raise comes from the filesystem walk (IOException) and never
142+
// reaches a coded-error constructor; widening the whole arm instead
143+
// would quietly relax the six malformed-config cases to "anything".
144+
var ex = Assert.ThrowsAny<Exception>(() => SourceResolver.ResolveCollection(invokeDir));
145+
if (!c.ErrorIsNative)
134146
{
135-
Assert.Equal(c.ExpectError.Value.GetString(), ex.Code.ToString());
147+
var coded = Assert.IsAssignableFrom<MetaModelException>(ex);
148+
if (c.ExpectError.Value.ValueKind == JsonValueKind.String)
149+
{
150+
Assert.Equal(c.ExpectError.Value.GetString(), coded.Code.ToString());
151+
}
136152
}
137153
return;
138154
}

‎server/csharp/MetaObjects.Conformance.Tests/SourceResolverTests.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,45 @@ public void TwoUnresolvablePaths_ReportsTheContentFirstOne()
4242
Directory.Delete(root, recursive: true);
4343
}
4444
}
45+
46+
/// <summary>
47+
/// A genuinely unknown top-level key resolves normally here, and THROWS in
48+
/// TypeScript. That asymmetry is ruled INTENDED in the source-resolution corpus
49+
/// README: TypeScript owns this file and models its whole vocabulary, so only it
50+
/// can tell a typo from a key a sibling owns. This port models the neutral subset
51+
/// (`schema_version` + `sources`), for which every other key is indistinguishable
52+
/// from a TypeScript-owned one — imitating strictness would mean carrying TS's key
53+
/// list and rejecting a config a newer `meta` had just written.
54+
/// </summary>
55+
/// <remarks>
56+
/// Deliberately NOT a shared corpus case: a shared case asserts one outcome and
57+
/// the correct outcome differs by port, so adding one could only be done by making
58+
/// some port wrong. This is the tolerant half.
59+
/// </remarks>
60+
[Fact]
61+
public void AnUnknownTopLevelConfigKey_IsIgnored_NotRejected()
62+
{
63+
var root = Path.Combine(Path.GetTempPath(), "source-resolver-unknownkey-" + System.Guid.NewGuid().ToString("N"));
64+
Directory.CreateDirectory(root);
65+
try
66+
{
67+
var model = Path.Combine(root, "model");
68+
Directory.CreateDirectory(model);
69+
File.WriteAllText(Path.Combine(model, "meta.a.json"), "{\"metadata.root\":{\"children\":[]}}");
70+
71+
var dotMo = Path.Combine(root, ".metaobjects");
72+
Directory.CreateDirectory(dotMo);
73+
File.WriteAllText(Path.Combine(dotMo, "config.json"),
74+
"{\"schema_version\":1,\"sources\":[{\"path\":\"model\"}],\"foo\":1}");
75+
76+
var resolved = SourceResolver.ResolveCollection(root);
77+
78+
Assert.Single(resolved);
79+
Assert.Equal(Path.Combine(model, "meta.a.json"), resolved[0]);
80+
}
81+
finally
82+
{
83+
Directory.Delete(root, recursive: true);
84+
}
85+
}
4586
}

0 commit comments

Comments
 (0)