read(Path configDir) {
ErrorCode.ERR_MALFORMED_JSON);
}
+ // `JsonParser.parseString` ALWAYS parses leniently, no matter how the
+ // caller would like to configure it — a well-known Gson quirk: both
+ // `JsonParser.parseReader` and `Gson#fromJson(JsonReader, ...)` force
+ // `setLenient(true)` on the reader internally before walking it, so
+ // unquoted keys, single-quoted strings, and `NaN`/`Infinity` literals all
+ // parse clean — silently accepting a file that is not actually valid JSON,
+ // contradicting this class's own javadoc and diverging from every other
+ // port's stock JSON parser (TS `JSON.parse`, Python `json.loads`, C#
+ // `System.Text.Json`, all strict by default). A `JsonReader` walked
+ // directly with `setLenient(false)` is the only way to get genuinely
+ // strict parsing out of Gson; used here purely to VALIDATE (the walked
+ // structure is discarded — `JsonParser.parseString` below still builds the
+ // actual `JsonElement` tree, unchanged).
+ try (JsonReader strict = new JsonReader(new StringReader(content))) {
+ strict.setLenient(false);
+ strict.skipValue();
+ if (strict.peek() != JsonToken.END_DOCUMENT) {
+ throw new MalformedJsonException("trailing content after the top-level value");
+ }
+ } catch (IOException e) {
+ throw new MetaDataException(
+ path + " exists but could not be parsed as JSON: " + e.getMessage(),
+ ErrorCode.ERR_MALFORMED_JSON);
+ }
+
JsonElement parsed;
try {
parsed = JsonParser.parseString(content);
diff --git a/server/java/metadata/src/main/java/com/metaobjects/config/SourceResolver.java b/server/java/metadata/src/main/java/com/metaobjects/config/SourceResolver.java
index 74b7e7f3a..e500b26e1 100644
--- a/server/java/metadata/src/main/java/com/metaobjects/config/SourceResolver.java
+++ b/server/java/metadata/src/main/java/com/metaobjects/config/SourceResolver.java
@@ -18,13 +18,16 @@
import com.metaobjects.ErrorCode;
import com.metaobjects.MetaDataException;
import com.metaobjects.loader.DirectorySource;
+import com.metaobjects.loader.FileSource;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import java.util.stream.Stream;
/**
* Turns a declared source SET ({@code .metaobjects/config.json}'s {@code sources}, or
@@ -57,6 +60,16 @@ private SourceResolver() {}
* an unresolved PATH regardless of which is declared first
* ({@code unsupported-kind-precedes-unresolved-path-when-path-is-declared-first}/
* {@code -second}).
+ *
+ * Pass 2 processes the validated specs in CONTENT order (natural string
+ * ordering of each spec's {@code path}), not declared order — mirroring the
+ * reference implementation's {@code orderedPathSpecs}
+ * ({@code sources.ts}: kind-validated, then sorted by
+ * {@code JSON.stringify(spec)}, which for a validated {@code path}-only spec
+ * reduces to sorting by the path string alone). This does not change the
+ * resolved file SET — de-duplication is order-independent — only which
+ * declared path's {@code ERR_SOURCE_UNRESOLVED} fires first when more than one
+ * is simultaneously unresolvable, which order-independence alone cannot pin.
*/
public static List resolveSources(Path configDir, List> specs) {
// Pass 1 — kind validation across the WHOLE set, no filesystem I/O yet.
@@ -71,6 +84,7 @@ public static List resolveSources(Path configDir, List
}
pathSpecs.add(rawPath);
}
+ Collections.sort(pathSpecs);
// Pass 2 — resolve each validated path spec against the filesystem.
LinkedHashSet seen = new LinkedHashSet<>();
@@ -97,8 +111,15 @@ public static List resolveSources(Path configDir, List
// DirectorySource directly gets every file) — this CLI-facing
// resolver turns it ON, since `_pending/` is the TypeScript CLI's
// pending/promote-workflow concept, not a loader concept.
- new DirectorySource(target, new DirectorySource.Options().setExcludePending(true)).expand()
- .forEach(fs -> seen.add(fs.getPath().toAbsolutePath().normalize()));
+ // try-with-resources: expand() wraps Files.walk(), which the JDK
+ // documents as requiring prompt closing of its underlying
+ // directory-handle resource ("must be used within a try-with-resources
+ // statement or similar control structure") — a plain `.forEach()`
+ // with no `close()` leaks it on every directory source resolved.
+ try (Stream stream =
+ new DirectorySource(target, new DirectorySource.Options().setExcludePending(true)).expand()) {
+ stream.forEach(fs -> seen.add(fs.getPath().toAbsolutePath().normalize()));
+ }
} else {
seen.add(target.toAbsolutePath().normalize());
}
diff --git a/server/java/metadata/src/main/java/com/metaobjects/loader/uri/URIHelper.java b/server/java/metadata/src/main/java/com/metaobjects/loader/uri/URIHelper.java
index c5adca2fd..b6c1e47ff 100644
--- a/server/java/metadata/src/main/java/com/metaobjects/loader/uri/URIHelper.java
+++ b/server/java/metadata/src/main/java/com/metaobjects/loader/uri/URIHelper.java
@@ -40,7 +40,16 @@ public static URIModel toURIModel( String in ) {
}
public static URIModel toURIModel( URI in ) {
- return toURIModel( in.toString() );
+ // NOT `in.toString()`: a URI built by `constructValidatedURI` below
+ // quotes characters that need it (a space -> "%20") in its string
+ // form, but `parseURIModel` is a dumb substring splitter with no
+ // percent-decoding of its own — feeding it the ENCODED string would
+ // hand a source string containing a literal "%20" downstream to
+ // `new File(...)`, silently looking for the wrong path.
+ // `getSchemeSpecificPart()` returns the DECODED form, so reassembling
+ // ":" round-trips back to exactly what
+ // was originally passed to `constructValidatedURI`.
+ return toURIModel( in.getScheme() + ":" + in.getSchemeSpecificPart() );
}
public static URI toURI( String in ) {
@@ -58,20 +67,31 @@ public static URI constructURI(String uriType, String uriSourceType, String sour
}
public static URI constructValidatedURI(String uriType, String uriSourceType, String source, Map args) {
- String uriStr = uriType + ":" + uriSourceType + ":" + source;
+ String ssp = uriSourceType + ":" + source;
if ( args != null && !args.isEmpty() ) {
- uriStr += ";";
+ ssp += ";";
boolean first = true;
for (String key : args.keySet() ) {
if ( first ) first = false;
- else uriStr += "&";
- uriStr += key+"="+args.get(key);
+ else ssp += "&";
+ ssp += key+"="+args.get(key);
}
}
try {
- return new URI(uriStr );
+ // The single-String `new URI(String)` constructor requires the input
+ // to already be fully well-formed RFC 2396 syntax and THROWS on any
+ // character that needs quoting — a raw space in an absolute
+ // filesystem path (e.g. a checkout under ".../My Projects/...") is
+ // the common case, and it is more common than the colon ambiguity
+ // this method's caller was previously hardened against. The 3-arg
+ // (scheme, scheme-specific-part, fragment) constructor QUOTES illegal
+ // characters instead of rejecting them — `toURIModel(URI)` above
+ // undoes the quoting on the way back out, so the round trip is
+ // lossless (verified: identical `toString()` output to the old
+ // single-arg construction whenever `source` needed no quoting at all).
+ return new URI(uriType, ssp, null);
} catch( URISyntaxException e ) {
- throw new IllegalArgumentException( "URI Syntax exception ["+uriStr+"]: "+ e.getMessage(), e );
+ throw new IllegalArgumentException( "URI Syntax exception ["+uriType+":"+ssp+"]: "+ e.getMessage(), e );
}
}
diff --git a/server/java/metadata/src/test/java/com/metaobjects/config/NeutralConfigTest.java b/server/java/metadata/src/test/java/com/metaobjects/config/NeutralConfigTest.java
index 7dd719138..82d5d86ee 100644
--- a/server/java/metadata/src/test/java/com/metaobjects/config/NeutralConfigTest.java
+++ b/server/java/metadata/src/test/java/com/metaobjects/config/NeutralConfigTest.java
@@ -69,6 +69,29 @@ public void schemaVersionAsFloatLiteralIsAcceptedLikeTheOtherThreePorts() throws
assertEquals(1, cfg.getSources().size());
}
+ @Test
+ public void unquotedKeysAndSingleQuotedStringsAreRejected() throws IOException {
+ // F8 — Gson's `JsonParser.parseString` ALWAYS parses leniently regardless
+ // of any JsonReader configuration (a well-known Gson quirk: both
+ // `JsonParser.parseReader` and `Gson.fromJson(JsonReader, ...)` force
+ // `setLenient(true)` internally before walking the reader). That silently
+ // accepted unquoted keys and single-quoted strings — syntax the class
+ // javadoc promises is rejected, and which TS's `JSON.parse`, Python's
+ // `json.loads`, and C#'s `System.Text.Json` all reject.
+ Path dir = writeConfig("{schema_version: 1, sources: [{path: 'model'}]}");
+ MetaDataException ex = assertThrows(MetaDataException.class, () -> NeutralConfig.read(dir));
+ assertEquals(ErrorCode.ERR_MALFORMED_JSON, ex.getCode().orElseThrow());
+ }
+
+ @Test
+ public void nanAndInfinityLiteralsAreRejected() throws IOException {
+ // Same Gson-leniency quirk as above, for the numeric-literal extensions
+ // (`NaN` / `Infinity` / `-Infinity`) lenient mode also accepts.
+ Path dir = writeConfig("{ \"schema_version\": NaN, \"sources\": [] }");
+ MetaDataException ex = assertThrows(MetaDataException.class, () -> NeutralConfig.read(dir));
+ assertEquals(ErrorCode.ERR_MALFORMED_JSON, ex.getCode().orElseThrow());
+ }
+
@Test
public void schemaVersionNonIntegralRaisesRatherThanTruncating() throws IOException {
// Regression: Gson's JsonPrimitive#getAsInt() TRUNCATES a non-integral
diff --git a/server/java/metadata/src/test/java/com/metaobjects/config/SourceResolverTest.java b/server/java/metadata/src/test/java/com/metaobjects/config/SourceResolverTest.java
new file mode 100644
index 000000000..c33d4f108
--- /dev/null
+++ b/server/java/metadata/src/test/java/com/metaobjects/config/SourceResolverTest.java
@@ -0,0 +1,68 @@
+/*
+ * Copyright 2003 Doug Mealing LLC dba Meta Objects
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.metaobjects.config;
+
+import com.metaobjects.MetaDataException;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Focused unit coverage for {@link SourceResolver#resolveSources} shapes not gated
+ * by the shared {@code source-resolution-conformance} corpus.
+ */
+public class SourceResolverTest {
+
+ private static Map pathSpec(String path) {
+ Map m = new LinkedHashMap<>();
+ m.put("path", path);
+ return m;
+ }
+
+ // F12 — Pass 2 resolves in CONTENT order (natural string ordering of each
+ // spec's `path`), not declared order, mirroring the TypeScript reference's
+ // `orderedPathSpecs` (`sources.ts`: kind-validated, then sorted by
+ // `JSON.stringify(spec)`, which for a validated `path`-only spec reduces to
+ // the path string alone — verified empirically: `resolveSources(dir,
+ // [{path:"zzz-missing"},{path:"aaa-missing"}])` names "aaa-missing", the
+ // content-first one, even though "zzz-missing" is declared first). With
+ // BOTH paths unresolvable, only a port that content-sorts before Pass 2
+ // names "aaa-missing" here; a declared-order implementation names
+ // "zzz-missing" instead.
+ @Test
+ public void twoUnresolvablePathsReportsTheContentFirstOne() throws IOException {
+ Path root = Files.createTempDirectory("source-resolver-order-");
+ try {
+ List> specs = List.of(pathSpec("zzz-missing"), pathSpec("aaa-missing"));
+ MetaDataException ex = assertThrows(MetaDataException.class,
+ () -> SourceResolver.resolveSources(root, specs));
+ assertTrue("expected \"aaa-missing\" (content-first) in: " + ex.getMessage(),
+ ex.getMessage().contains("aaa-missing"));
+ assertTrue("must NOT name \"zzz-missing\" (declared-first, content-second): " + ex.getMessage(),
+ !ex.getMessage().contains("zzz-missing"));
+ } finally {
+ Files.delete(root);
+ }
+ }
+}
diff --git a/server/python/src/metaobjects/cli.py b/server/python/src/metaobjects/cli.py
index 1a337d0eb..655a62747 100644
--- a/server/python/src/metaobjects/cli.py
+++ b/server/python/src/metaobjects/cli.py
@@ -256,19 +256,32 @@ def _load_root(
registered ``trace-helper`` generator exists to consume — the generator was reachable
from the CLI while its input was not.
"""
- if providers:
- from metaobjects.core_types import core_providers
-
- result = MetaDataLoader.from_directory(
- metadata_dir,
- providers=[*core_providers, *providers],
- strict=strict,
- libraries=libraries,
- )
- else:
- result = MetaDataLoader.from_directory(
- metadata_dir, strict=strict, libraries=libraries
- )
+ try:
+ if providers:
+ from metaobjects.core_types import core_providers
+
+ result = MetaDataLoader.from_directory(
+ metadata_dir,
+ providers=[*core_providers, *providers],
+ strict=strict,
+ libraries=libraries,
+ )
+ else:
+ result = MetaDataLoader.from_directory(
+ metadata_dir, strict=strict, libraries=libraries
+ )
+ except OSError as e:
+ # `DirectorySource.expand()` walks via `iterdir()` (not `rglob()`, so it
+ # can follow a symlinked subdirectory — the I1 fix) and raises a plain
+ # OSError (FileNotFoundError / NotADirectoryError, or `SymlinkLoopError`
+ # on a symlink cycle) when `metadata_dir` itself can't be walked. Nothing
+ # upstream of this call catches it, so it would otherwise surface as a
+ # raw Python traceback instead of the coded "error: failed to load
+ # metadata" every other `_load_root` failure prints. Mirrors the
+ # TypeScript reference, which wraps the equivalent `readdir` failure in
+ # a clean `Error` at the same boundary (`metadata-files.ts`,
+ # `listMetadataFiles`: "cannot read metadata directory ...").
+ return None, [f"cannot read metadata directory {metadata_dir}: {e}"]
if result.errors:
msgs = [f"{e.code}: {e.message}" for e in result.errors]
return None, msgs
@@ -279,6 +292,7 @@ def _load_root_from_paths(
paths: list[str],
strict: bool = False,
providers: list[object] | None = None,
+ libraries: list[str] | None = None,
) -> tuple[MetaData | None, list[str]]:
"""Load metadata from an explicit file list rather than a single directory.
@@ -287,17 +301,20 @@ def _load_root_from_paths(
individual files, which a single ``from_directory`` call cannot express —
so this loads each resolved file as its own ``file://`` source via
:meth:`MetaDataLoader.from_uris`. Mirrors :func:`_load_root`'s ``strict``/
- ``providers`` contract exactly.
+ ``providers``/``libraries`` contract exactly.
"""
uris = [Path(p).resolve().as_uri() for p in paths]
if providers:
from metaobjects.core_types import core_providers
result = MetaDataLoader.from_uris(
- uris, providers=[*core_providers, *providers], strict=strict
+ uris,
+ providers=[*core_providers, *providers],
+ strict=strict,
+ libraries=libraries,
)
else:
- result = MetaDataLoader.from_uris(uris, strict=strict)
+ result = MetaDataLoader.from_uris(uris, strict=strict, libraries=libraries)
if result.errors:
msgs = [f"{e.code}: {e.message}" for e in result.errors]
return None, msgs
@@ -543,7 +560,22 @@ def _cmd_docs(args: argparse.Namespace) -> int:
paths = _resolve_metadata_location_or_print_error(config, root_dir)
if paths is None:
return 1
- root, errors = _load_root_from_paths(paths, providers=providers)
+ docs_libraries: list[str] | None = None
+ if config is not None:
+ # `config` was already loaded above (to resolve rung 2's `metadata`
+ # key) — its OWN `providers`/`libraries` must reach this load too,
+ # exactly as `gen`'s config-mode load does (`_cmd_gen_config`), or a
+ # project relying on either (e.g. `libraries: ["ai"]` to resolve
+ # `extends: metaobjects::ai::LlmCallBase`) fails `docs` with
+ # `ERR_UNRESOLVED_SUPER` while `gen` on the same config succeeds.
+ config_providers, config_providers_ok = _config_providers(config)
+ if not config_providers_ok:
+ return 1
+ providers = [*providers, *config_providers]
+ docs_libraries = config.libraries
+ root, errors = _load_root_from_paths(
+ paths, providers=providers, libraries=docs_libraries
+ )
project_default = root_dir.name
else:
root, errors = _load_root(args.metadata_dir, providers=providers)
diff --git a/server/python/src/metaobjects/config/neutral_config.py b/server/python/src/metaobjects/config/neutral_config.py
index b1075ba0e..feedadea4 100644
--- a/server/python/src/metaobjects/config/neutral_config.py
+++ b/server/python/src/metaobjects/config/neutral_config.py
@@ -49,7 +49,12 @@ def read_neutral_config(config_dir: Path) -> NeutralConfig | None:
)
version = raw.get("schema_version")
- if version != 1:
+ # `isinstance(version, bool)` must be checked FIRST and separately: Python's
+ # `bool` is a subclass of `int` and `True == 1`, so a bare `version != 1`
+ # check accepts `schema_version: true` — a divergence from Java's
+ # `isNumber()` and C#'s `ValueKind != Number`, both of which reject a JSON
+ # boolean outright.
+ if isinstance(version, bool) or not isinstance(version, (int, float)) or version != 1:
raise ParseError(
f"{path}: unsupported schema_version {version!r} (expected 1)",
code=ErrorCode.ERR_COLLECTION_NOT_FOUND,
diff --git a/server/python/src/metaobjects/config/source_resolver.py b/server/python/src/metaobjects/config/source_resolver.py
index 4f5ea94af..fd7ea7d6b 100644
--- a/server/python/src/metaobjects/config/source_resolver.py
+++ b/server/python/src/metaobjects/config/source_resolver.py
@@ -72,9 +72,18 @@ def resolve_sources(config_dir: Path, specs: list[dict[str, str]]) -> list[Path]
# Whole-list kind validation FIRST — see `_validate_kinds`.
_validate_kinds(specs)
+ # Resolve in CONTENT order (ordinal path-string sort), not declared order —
+ # mirrors `orderedPathSpecs` in `sources.ts` (kind-validated, then sorted by
+ # `JSON.stringify(spec)`, which for a validated `path`-only spec reduces to
+ # the path string alone). Does not change the resolved file SET (the `seen`
+ # de-dup below is order-independent); only decides which declared path's
+ # `ERR_SOURCE_UNRESOLVED` fires first when more than one is simultaneously
+ # unresolvable.
+ ordered_specs = sorted(specs, key=lambda s: s["path"])
+
seen: dict[Path, None] = {}
- for spec in specs:
+ for spec in ordered_specs:
raw = Path(spec["path"])
target = raw if raw.is_absolute() else (config_dir / raw)
diff --git a/server/python/src/metaobjects/loader/meta_data_loader.py b/server/python/src/metaobjects/loader/meta_data_loader.py
index cb16a04de..e391bb381 100644
--- a/server/python/src/metaobjects/loader/meta_data_loader.py
+++ b/server/python/src/metaobjects/loader/meta_data_loader.py
@@ -189,13 +189,25 @@ def from_uris(
uris: list[str],
providers: list[Provider] | None = None,
strict: bool = False,
+ libraries: list[str] | None = None,
) -> LoadResult:
"""Load metadata from a list of URIs (file:// or http(s)://).
``strict`` (ADR-0023) — see :meth:`from_directory`.
+
+ ``libraries`` — see :meth:`from_directory`; prepended the same way, so a
+ caller resolving an explicit file list (e.g. the CLI's
+ ``.metaobjects/config.json`` ``sources`` rung) can opt in to a shipped
+ library package exactly like a single-directory load can.
"""
loader = cls(providers=providers, strict=strict)
- return loader.load([UriSource(u) for u in uris])
+ sources: list[MetaDataSource] = []
+ if libraries:
+ from metaobjects.library import library_sources
+
+ sources.extend(library_sources(libraries))
+ sources.extend(UriSource(u) for u in uris)
+ return loader.load(sources)
@classmethod
def from_string(
diff --git a/server/python/tests/codegen/test_cli_config_gen.py b/server/python/tests/codegen/test_cli_config_gen.py
index 26f9f8078..d694553ac 100644
--- a/server/python/tests/codegen/test_cli_config_gen.py
+++ b/server/python/tests/codegen/test_cli_config_gen.py
@@ -224,6 +224,30 @@ def test_gen_flag_path_ignores_config_when_present(tmp_path: Path) -> None:
assert (out / "Program.py").exists()
+def test_gen_missing_metadata_dir_errors_cleanly(tmp_path: Path, capsys) -> None:
+ """An explicit that does not exist must fail with a coded
+ CLI error, not an uncaught FileNotFoundError traceback. `DirectorySource`
+ switched from `rglob` to a manual `iterdir()` walk (the symlink-following
+ fix) — `iterdir()` raises OSError on a directory that never existed;
+ nothing on the `_load_root` path caught it."""
+ missing = tmp_path / "does" / "not" / "exist"
+ rc = main(["gen", str(missing), "--out", str(tmp_path / "out")])
+ assert rc == 1
+ err = capsys.readouterr().err
+ assert "error: failed to load metadata" in err
+
+
+def test_gen_metadata_dir_is_a_file_errors_cleanly(tmp_path: Path, capsys) -> None:
+ """Same as above, for the `NotADirectoryError` arm: resolves
+ to a plain file rather than a directory."""
+ not_a_dir = tmp_path / "somefile.txt"
+ not_a_dir.write_text("not a directory")
+ rc = main(["gen", str(not_a_dir), "--out", str(tmp_path / "out")])
+ assert rc == 1
+ err = capsys.readouterr().err
+ assert "error: failed to load metadata" in err
+
+
def test_gen_no_args_no_yaml_falls_back_to_neutral_config(
tmp_path: Path, monkeypatch
) -> None:
diff --git a/server/python/tests/config/test_neutral_config.py b/server/python/tests/config/test_neutral_config.py
index 2f9f58e64..b24cc178e 100644
--- a/server/python/tests/config/test_neutral_config.py
+++ b/server/python/tests/config/test_neutral_config.py
@@ -65,6 +65,17 @@ def test_wrong_schema_version_raises(tmp_path: Path) -> None:
assert e.value.code == ErrorCode.ERR_COLLECTION_NOT_FOUND
+def test_boolean_schema_version_raises(tmp_path: Path) -> None:
+ # Python's `True == 1` is a language quirk the other three ports don't
+ # share (Gson's `isNumber()` and System.Text.Json's `ValueKind` both
+ # reject a JSON boolean outright) — a boolean must never satisfy the
+ # `schema_version == 1` check.
+ _write_config(tmp_path, {"schema_version": True, "sources": [{"path": "model"}]})
+ with pytest.raises(ParseError) as e:
+ read_neutral_config(tmp_path)
+ assert e.value.code == ErrorCode.ERR_COLLECTION_NOT_FOUND
+
+
def test_null_sources_raises(tmp_path: Path) -> None:
# A present `sources: null` is present-but-wrong-typed, same as a bare
# object — it must not silently read as "absent" (see the shared
diff --git a/server/python/tests/config/test_source_resolver.py b/server/python/tests/config/test_source_resolver.py
index 00bab4bf8..5dfef894c 100644
--- a/server/python/tests/config/test_source_resolver.py
+++ b/server/python/tests/config/test_source_resolver.py
@@ -65,6 +65,20 @@ def test_kind_error_precedes_unresolved_path_regardless_of_order(tmp_path: Path)
assert e_resource_first.value.code == ErrorCode.ERR_SOURCE_KIND_UNSUPPORTED
+def test_two_unresolvable_paths_reports_the_content_first_one(tmp_path: Path) -> None:
+ # F12 — Pass 2 resolves in CONTENT order (ordinal path-string sort), not
+ # declared order, mirroring the TypeScript reference's `orderedPathSpecs`
+ # (verified empirically: `resolveSources(dir, [{path:"zzz-missing"},
+ # {path:"aaa-missing"}])` names "aaa-missing", the content-first one, even
+ # though "zzz-missing" is declared first). With BOTH paths unresolvable,
+ # only the port that content-sorts before Pass 2 names "aaa-missing" here;
+ # a declared-order implementation would name "zzz-missing" instead.
+ with pytest.raises(ParseError) as e:
+ resolve_sources(tmp_path, [{"path": "zzz-missing"}, {"path": "aaa-missing"}])
+ assert "aaa-missing" in str(e.value)
+ assert "zzz-missing" not in str(e.value)
+
+
def test_collection_falls_back_to_default_dir(tmp_path: Path) -> None:
(tmp_path / "metaobjects").mkdir()
(tmp_path / "metaobjects" / "a.json").write_text("{}")
diff --git a/server/python/tests/conformance/test_source_resolution_conformance.py b/server/python/tests/conformance/test_source_resolution_conformance.py
index a929d0784..ec5bee412 100644
--- a/server/python/tests/conformance/test_source_resolution_conformance.py
+++ b/server/python/tests/conformance/test_source_resolution_conformance.py
@@ -134,3 +134,46 @@ def test_docs_with_no_positional_falls_back_to_neutral_config(
rc = main(["docs", "--out", str(out)])
assert rc == 0
assert (out / "api" / "python" / "README.md").exists()
+
+
+def test_docs_with_metaobjects_config_yaml_honors_declared_libraries(
+ tmp_path: Path, monkeypatch
+) -> None:
+ """`docs`'s no-positional branch loads `metaobjects.config.yaml` (rung 2 of
+ the ladder) purely to read its `metadata` key, then reloads via
+ `_load_root_from_paths` — which dropped both `config.providers` and
+ `config.libraries`, even though `config` was already sitting right there.
+ A project declaring `libraries: ["ai"]` and `extends:
+ metaobjects::ai::LlmCallBase` must resolve through `docs` exactly as it
+ already does through `gen` / `verify --codegen` (see
+ `test_shipped_library_ai.py::TestTheCliCanLoadTheLibrary`).
+ """
+ (tmp_path / "metadata").mkdir()
+ (tmp_path / "metadata" / "meta.app.yaml").write_text(
+ "metadata:\n"
+ " package: app::trace\n"
+ " children:\n"
+ " - object.entity:\n"
+ " name: AdopterCall\n"
+ " extends: metaobjects::ai::LlmCallBase\n"
+ " children:\n"
+ " - source.rdb: { table: adopter_call, role: primary }\n"
+ " - identity.primary: { name: id, fields: [\"spanId\"] }\n",
+ encoding="utf-8",
+ )
+ (tmp_path / "metaobjects.config.yaml").write_text(
+ "metadata: metadata\n"
+ 'libraries: ["ai"]\n'
+ "targets:\n"
+ " main:\n"
+ " outDir: out\n",
+ encoding="utf-8",
+ )
+
+ from metaobjects.cli import main
+
+ monkeypatch.chdir(tmp_path)
+ out = tmp_path / "docs-out"
+ rc = main(["docs", "--out", str(out)])
+ assert rc == 0
+ assert (out / "api" / "python" / "README.md").exists()
diff --git a/server/typescript/packages/cli/src/commands/init.ts b/server/typescript/packages/cli/src/commands/init.ts
index 25134f17d..e82fa7033 100644
--- a/server/typescript/packages/cli/src/commands/init.ts
+++ b/server/typescript/packages/cli/src/commands/init.ts
@@ -368,6 +368,13 @@ async function writeConfigFile(opts: InitOptions, result: InitResult, agentDir:
log.warn(priorContent);
result.warnings.push("invalid .metaobjects/config.json replaced with defaults");
await writeFresh();
+ // F11 — matches the OTHER two `writeFresh()` call sites above: this IS a
+ // fresh write (a destructive one, replacing content that could not be
+ // parsed), not a no-op. Omitting this left it in neither `created` nor
+ // `preserved`, so the `--config-only` CLI summary (which keys on
+ // `result.created.includes(...)` alone) reported "already exists — left
+ // untouched" for a config it had just overwritten with defaults.
+ result.created.push(".metaobjects/config.json");
}
}
diff --git a/server/typescript/packages/cli/src/commands/migrate.ts b/server/typescript/packages/cli/src/commands/migrate.ts
index 5becfd328..5e1817766 100644
--- a/server/typescript/packages/cli/src/commands/migrate.ts
+++ b/server/typescript/packages/cli/src/commands/migrate.ts
@@ -49,6 +49,7 @@ import {
type D1Runner,
type SchemaProvenance,
type SchemaSnapshot,
+ type TableDescriptor,
} from "@metaobjectsdev/migrate-ts";
import {
buildWranglerExecuteArgs,
@@ -303,8 +304,9 @@ function summarizeChanges(changes: Change[]): Record {
}
/**
- * The qualified names of tables/views this diff proposes to DROP that the committed
- * snapshot never contained — i.e. objects this toolchain never managed (#313).
+ * The qualified names of tables/views/constraints this diff proposes to DROP that
+ * the committed snapshot never contained — i.e. objects this toolchain never
+ * managed (#313).
*
* FAILS OPEN. No snapshot on disk, or one that cannot be read, yields an empty list:
* a project that has never generated one is not in an error state, and refusing there
@@ -312,10 +314,21 @@ function summarizeChanges(changes: Change[]): Record {
* migrate's own error to raise elsewhere, with its own message, not a silent refusal
* here.
*
- * Names come from `qualifiedDbName` and nothing else. Three independent sets already
- * have to agree on this spelling — the diff's identity maps, the `@unmanaged`
- * exclusion set, and the out-of-scope set — and a fourth encoding of "absent schema
- * means public" would silently un-guard every object it disagreed about.
+ * Table/view names come from `qualifiedDbName` and nothing else. Three independent
+ * sets already have to agree on this spelling — the diff's identity maps, the
+ * `@unmanaged` exclusion set, and the out-of-scope set — and a fourth encoding of
+ * "absent schema means public" would silently un-guard every object it disagreed
+ * about.
+ *
+ * `drop-fk`/`drop-check`/constraint-backed `drop-index` are checked at the
+ * CONSTRAINT grain, not just the table's: the emitter's `IF EXISTS` on the
+ * enclosing `ALTER TABLE` (the SQL half of this same #313 gap) only stops the
+ * replay from failing outright — it says nothing about whether AUTHORING the
+ * drop was ever supposed to be permission-free. A table can be fully managed
+ * while carrying a constraint another tool added directly against the live
+ * database; that constraint's name is absent from the snapshotted table's own
+ * `foreignKeys`/`checks`/`indexes`, exactly like a whole unmanaged table is
+ * absent from `snapshot.tables`.
*/
async function snapshotAbsentDrops(changes: Change[], snapPath: string): Promise {
let snapshot: SchemaSnapshot | null;
@@ -326,17 +339,31 @@ async function snapshotAbsentDrops(changes: Change[], snapPath: string): Promise
}
if (snapshot === null) return [];
- const managed = new Set();
- for (const t of snapshot.tables) managed.add(qualifiedDbName(t));
- for (const v of snapshot.views) managed.add(qualifiedDbName(v));
+ const managedTables = new Map();
+ for (const t of snapshot.tables) managedTables.set(qualifiedDbName(t), t);
+ const managedViews = new Set();
+ for (const v of snapshot.views) managedViews.add(qualifiedDbName(v));
const absent: string[] = [];
for (const c of changes) {
- const name =
- c.kind === "drop-table" ? qualifiedDbName({ name: c.table, schema: c.schema })
- : c.kind === "drop-view" ? qualifiedDbName({ name: c.view, schema: c.schema })
- : undefined;
- if (name !== undefined && !managed.has(name)) absent.push(name);
+ if (c.kind === "drop-table") {
+ const name = qualifiedDbName({ name: c.table, schema: c.schema });
+ if (!managedTables.has(name)) absent.push(name);
+ } else if (c.kind === "drop-view") {
+ const name = qualifiedDbName({ name: c.view, schema: c.schema });
+ if (!managedViews.has(name)) absent.push(name);
+ } else if (c.kind === "drop-fk" || c.kind === "drop-check" || c.kind === "drop-index") {
+ const tableName = qualifiedDbName({ name: c.table, schema: c.schema });
+ const table = managedTables.get(tableName);
+ const constraintName = c.kind === "drop-fk" ? c.fk : c.kind === "drop-check" ? c.check : c.index;
+ const recorded =
+ c.kind === "drop-fk" ? table?.foreignKeys
+ : c.kind === "drop-check" ? table?.checks
+ : table?.indexes;
+ if (recorded === undefined || !recorded.some((d) => d.name === constraintName)) {
+ absent.push(`${tableName}.${constraintName}`);
+ }
+ }
}
return absent;
}
diff --git a/server/typescript/packages/cli/src/commands/verify.ts b/server/typescript/packages/cli/src/commands/verify.ts
index f3c702ddd..840c39301 100644
--- a/server/typescript/packages/cli/src/commands/verify.ts
+++ b/server/typescript/packages/cli/src/commands/verify.ts
@@ -332,16 +332,25 @@ export async function verifyCommand(
// and a gate that is quiet when it checked nothing cannot be told from one that
// passed. Every migration is pending against a fresh engine, so an empty
// `pending` means the directory held none.
+ //
+ // This return is for TIER 1 ONLY: an empty chain trivially "applies" (there is
+ // nothing that could fail), so tier 1 is done. Tier 2 is NOT done — its job is
+ // "does the replay reproduce the committed snapshot?", and a wrong
+ // `migrate.outDir` or a project adopted via `migrate baseline --from-db` (whose
+ // own chain is empty by construction) both look identical to this point. Return
+ // ONLY when `--replay-snapshot` was not requested; otherwise fall through so an
+ // empty replay is still compared against a snapshot that may record dozens of
+ // tables, rather than reporting success having compared nothing.
if (applied.pending.length === 0) {
log.info(`meta verify --replay: no committed migrations — nothing to replay`);
- return 0;
+ if (!flags.replaySnapshot) return 0;
+ } else {
+ log.info(
+ `meta verify --replay — the committed chain applies to an empty ${dialect} database ` +
+ `(${applied.applied.length} migration(s)).`,
+ );
}
- log.info(
- `meta verify --replay — the committed chain applies to an empty ${dialect} database ` +
- `(${applied.applied.length} migration(s)).`,
- );
-
if (!flags.replaySnapshot) return 0;
return await runReplaySnapshotTier(engine, dialect, dir);
} finally {
diff --git a/server/typescript/packages/cli/test/init.test.ts b/server/typescript/packages/cli/test/init.test.ts
index de0ba0e26..788bf3530 100644
--- a/server/typescript/packages/cli/test/init.test.ts
+++ b/server/typescript/packages/cli/test/init.test.ts
@@ -290,6 +290,30 @@ describe("init() --config-only", () => {
expect(result.warnings).toContain("invalid .metaobjects/config.json replaced with defaults");
const cfg = JSON.parse(readFileSync(join(cwd, ".metaobjects", "config.json"), "utf8"));
expect(cfg.sources).toEqual([]);
+ // F11 — this destructive replacement must be reported in `result.created`
+ // (matching the OTHER two `writeFresh()` call sites in `writeConfigFile`),
+ // not silently omitted from both `created` and `preserved`. The CLI's
+ // `--config-only` summary keys on `result.created.includes(...)` alone to
+ // choose between "Wrote ..." and "already exists — left untouched." —
+ // without this, a config the caller just DESTROYED and replaced with
+ // defaults is reported as "left untouched", the opposite of what happened.
+ expect(result.created).toContain(".metaobjects/config.json");
+ });
+
+ test("preserving a valid existing config is reported separately from a fresh write", async () => {
+ // The sibling of the case above: a VALID existing config is genuinely left
+ // untouched (merged in place via saveConfig, not replaced with defaults) —
+ // `result.preserved`, not `result.created`, is the correct bucket for it.
+ mkdirSync(join(cwd, ".metaobjects"), { recursive: true });
+ writeFileSync(
+ join(cwd, ".metaobjects", "config.json"),
+ JSON.stringify({ schema_version: 1, sources: [{ path: "model" }] }),
+ );
+
+ const result = await init({ cwd, configOnly: true, force: true });
+
+ expect(result.preserved).toContain(".metaobjects/config.json");
+ expect(result.created).not.toContain(".metaobjects/config.json");
});
});
diff --git a/server/typescript/packages/cli/test/migrate-drop-unmanaged.test.ts b/server/typescript/packages/cli/test/migrate-drop-unmanaged.test.ts
index bc326014c..fa550fab7 100644
--- a/server/typescript/packages/cli/test/migrate-drop-unmanaged.test.ts
+++ b/server/typescript/packages/cli/test/migrate-drop-unmanaged.test.ts
@@ -145,3 +145,122 @@ describe("meta migrate refuses a snapshot-absent drop", () => {
expect(await run(f)).toBe(0);
});
});
+
+// F10 — the guard above only inspected `drop-table`/`drop-view`, so a `drop-fk`
+// (or `drop-check`/constraint-backed `drop-index`) on a table the committed
+// snapshot never recorded that FK for slid through with no permission check at
+// all — even though the resulting migration cannot replay against a fresh
+// database (the emit fix in `emit/postgres.ts` closes the SQL half of #313;
+// this is the "the diff isn't even refused" half named alongside it).
+describe("meta migrate refuses a snapshot-absent FK drop (F10)", () => {
+ const FK_MODEL = JSON.stringify({
+ "metadata.root": {
+ package: "acme::platform",
+ children: [
+ {
+ "object.entity": {
+ name: "Job",
+ children: [
+ { "source.rdb": { name: "src", "@table": "jobs" } },
+ { "field.long": { name: "id" } },
+ { "identity.primary": { name: "pk", "@fields": ["id"], "@generation": "increment" } },
+ ],
+ },
+ },
+ {
+ "object.entity": {
+ name: "Order",
+ children: [
+ { "source.rdb": { name: "src", "@table": "orders" } },
+ { "field.long": { name: "id" } },
+ // A plain field, not a declared relationship — the live FK below is
+ // extra beyond anything the metadata asks for, exactly the "another
+ // tool owns this constraint" shape.
+ { "field.long": { name: "jobId" } },
+ { "identity.primary": { name: "pk", "@fields": ["id"], "@generation": "increment" } },
+ ],
+ },
+ },
+ ],
+ },
+ });
+
+ /**
+ * `jobs` + `orders`, where `orders.job_id` carries a live FK to `jobs` that the
+ * metadata never declares — so the diff proposes `drop-fk`. `seedSnapshot`
+ * decides whether the committed snapshot recorded that FK as part of `orders`'
+ * own shape (`fk-managed`, what a prior migration in THIS chain creating it
+ * would produce) or not (`fk-unmanaged`, another tool's constraint).
+ */
+ async function fkFixture(seedSnapshot: "fk-managed" | "fk-unmanaged"): Promise {
+ const root = await mkdtemp(join(tmpdir(), "drop-unmanaged-fk-"));
+ dirs.push(root);
+ await mkdir(join(root, "metaobjects"), { recursive: true });
+ await writeFile(join(root, "metaobjects", "meta.platform.json"), FK_MODEL, "utf8");
+ await mkdir(join(root, ".metaobjects", "migrations"), { recursive: true });
+ await writeFile(
+ join(root, ".metaobjects", "config.json"),
+ JSON.stringify({ schema_version: 1, migrate: { dialect: "sqlite" } }),
+ "utf8",
+ );
+
+ const db = join(root, "t.db");
+ const k = await buildKyselyFromUrl(`file:${db}`, "sqlite");
+ try {
+ await sql`CREATE TABLE "jobs" (id INTEGER NOT NULL PRIMARY KEY)`.execute(k.db);
+ await sql`CREATE TABLE "orders" (id INTEGER NOT NULL PRIMARY KEY, job_id INTEGER REFERENCES "jobs"(id))`.execute(k.db);
+ } finally {
+ await k.close();
+ }
+
+ const jobsTable: TableDescriptor = {
+ name: "jobs", columns: [{ name: "id", sqlType: { kind: "integer", bits: 64 }, nullable: false }],
+ indexes: [], foreignKeys: [], checks: [], primaryKey: ["id"],
+ };
+ const ordersColumns = [
+ { name: "id", sqlType: { kind: "integer", bits: 64 } as const, nullable: false },
+ { name: "job_id", sqlType: { kind: "integer", bits: 64 } as const, nullable: true },
+ ];
+ // Matches the synthesized-name convention both introspect/sqlite.ts and
+ // expected-schema.ts use for an unnamed sqlite FK: `__fk`.
+ const fk = { name: "orders_job_id_fk", columns: ["job_id"], refTable: "jobs", refColumns: ["id"] };
+ const ordersTable: TableDescriptor = {
+ name: "orders", columns: ordersColumns, indexes: [], checks: [], primaryKey: ["id"],
+ foreignKeys: seedSnapshot === "fk-managed" ? [fk] : [],
+ };
+
+ await writeSnapshot(snapshotPath(join(root, ".metaobjects", "migrations"), "sqlite"), {
+ tables: [jobsTable, ordersTable],
+ views: [],
+ });
+ return { root, db };
+ }
+
+ const runFk = (f: Fixture, extra: string[] = []): Promise =>
+ migrateCommand(
+ ["--from-db", "--db", `file:${f.db}`, "--dialect", "sqlite", "--slug", "x",
+ "--allow", ["drop-fk", ...extra].join(",")],
+ f.root,
+ );
+
+ test("refuses, and writes nothing", async () => {
+ const f = await fkFixture("fk-unmanaged");
+ expect(await runFk(f)).toBe(2);
+ expect(await migrationCount(f.root)).toBe(0);
+ });
+
+ test("--allow drop-unmanaged lets it through", async () => {
+ const f = await fkFixture("fk-unmanaged");
+ expect(await runFk(f, ["drop-unmanaged"])).toBe(0);
+ expect(await migrationCount(f.root)).toBe(1);
+ });
+
+ // The non-false-fire: a chain removing an FK it created itself (recorded in
+ // its own snapshot) needs no special permission beyond the ordinary `drop-fk`
+ // gate — this is what keeps the guard from breaking a normal FK removal.
+ test("a drop for an FK the snapshot DOES record proceeds without the flag", async () => {
+ const f = await fkFixture("fk-managed");
+ expect(await runFk(f)).toBe(0);
+ expect(await migrationCount(f.root)).toBe(1);
+ });
+});
diff --git a/server/typescript/packages/cli/test/verify-replay.test.ts b/server/typescript/packages/cli/test/verify-replay.test.ts
index 0686a6188..3917f7a23 100644
--- a/server/typescript/packages/cli/test/verify-replay.test.ts
+++ b/server/typescript/packages/cli/test/verify-replay.test.ts
@@ -12,7 +12,7 @@
* against a real project on disk.
*/
import { describe, test, expect, afterAll } from "bun:test";
-import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
+import { mkdtemp, mkdir, writeFile, rm, readdir, stat } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { parseVerifyArgs } from "../src/lib/args.js";
@@ -222,4 +222,32 @@ describe("verify --replay-snapshot runs BOTH tiers", () => {
await withApplyingChain(root);
expect(await verifyCommand(["--replay-snapshot"], root)).toBe(0);
});
+
+ // F6 — the empty-chain early return in runReplayVerify fired unconditionally,
+ // before runReplaySnapshotTier ever ran — so `--replay-snapshot` against an
+ // EMPTY chain reported success having compared NOTHING, even when a real
+ // committed snapshot on disk records tables the (empty) replay could never
+ // produce. Reproduces both hazards the surrounding comment names: a
+ // `migrate.outDir` that no longer points at the real chain, and a project
+ // adopted via `migrate baseline --from-db` (whose own chain is empty by
+ // construction). Tier 1 (`--replay`) is right to pass here — an empty chain
+ // trivially "applies" — but tier 2 owes a real answer, and that answer must be
+ // "does not reproduce the snapshot", not silence mistaken for a pass.
+ test("an empty chain with a real committed snapshot fails under --replay-snapshot", async () => {
+ const root = await project();
+ await withGeneratedChain(root); // writes a chain AND a snapshot recording "jobs"
+
+ // Empty the chain while leaving the committed snapshot (a sibling file, not
+ // inside any migration folder) untouched — the exact shape of a stale
+ // `migrate.outDir` or a baseline-from-db adoption.
+ const migrationsDir = join(root, ".metaobjects", "migrations");
+ for (const entry of await readdir(migrationsDir)) {
+ if ((await stat(join(migrationsDir, entry))).isDirectory()) {
+ await rm(join(migrationsDir, entry), { recursive: true, force: true });
+ }
+ }
+
+ expect(await verifyCommand(["--replay"], root)).toBe(0); // tier 1: trivially applies
+ expect(await verifyCommand(["--replay-snapshot"], root)).toBe(1); // tier 2: must not pass vacuously
+ });
});
diff --git a/server/typescript/packages/migrate-ts/src/emit/postgres.ts b/server/typescript/packages/migrate-ts/src/emit/postgres.ts
index 68ea5d2ec..a44504a85 100644
--- a/server/typescript/packages/migrate-ts/src/emit/postgres.ts
+++ b/server/typescript/packages/migrate-ts/src/emit/postgres.ts
@@ -129,12 +129,20 @@ function renderUp(c: Change): string {
// schema adopted from Drizzle has constraint-backed unique indexes.
// Both arms carry the #313 `IF EXISTS`: they are two renderings of the SAME
// `drop-index` change, and guarding one would leave the change kind half-covered.
+ // The constraint-backed arm ALSO guards the enclosing `ALTER TABLE` (not just
+ // the constraint name) — see the `drop-fk`/`drop-check` comment below for why.
case "drop-index":
return c.restore?.constraint !== undefined
- ? `ALTER TABLE ${quoteQualified(c.table, c.schema)} DROP CONSTRAINT IF EXISTS ${quote(c.index)};`
+ ? `ALTER TABLE IF EXISTS ${quoteQualified(c.table, c.schema)} DROP CONSTRAINT IF EXISTS ${quote(c.index)};`
: `DROP INDEX IF EXISTS ${quoteIndexQualified(c.index, c.schema)};`;
case "add-fk": return renderAddFk(c.table, c.schema, c.fk);
- case "drop-fk": return `ALTER TABLE ${quoteQualified(c.table, c.schema)} DROP CONSTRAINT IF EXISTS ${quote(c.fk)};`;
+ // #313 (constraint-level): `DROP CONSTRAINT IF EXISTS` alone only guards the
+ // constraint NAME — Postgres still requires the TABLE to exist to parse an
+ // `ALTER TABLE` at all, so a table another tool owns (never created by any
+ // migration in this chain) still killed the replay with `relation "x" does
+ // not exist`. Postgres supports `ALTER TABLE IF EXISTS` directly; using it
+ // closes the gap the same way `DROP TABLE IF EXISTS` above already does.
+ case "drop-fk": return `ALTER TABLE IF EXISTS ${quoteQualified(c.table, c.schema)} DROP CONSTRAINT IF EXISTS ${quote(c.fk)};`;
// `drop-check` IS produced by the diff — diff/index.ts:579 and :592 both push it,
// and an evolved `field.enum @values` is a live producer. (A comment here used to
// claim these arms were unreachable "declared, not yet produced" stubs; that was
@@ -148,7 +156,8 @@ function renderUp(c: Change): string {
// references the dropped constraint. SQLite is already replay-safe by construction;
// guarding Postgres makes the two dialects agree.
case "add-check": return `ALTER TABLE ${quoteQualified(c.table, c.schema)} ADD CONSTRAINT ${quote(c.check.name)} CHECK (${c.check.expression});`;
- case "drop-check": return `ALTER TABLE ${quoteQualified(c.table, c.schema)} DROP CONSTRAINT IF EXISTS ${quote(c.check)};`;
+ // Same `ALTER TABLE IF EXISTS` gap as `drop-fk` above.
+ case "drop-check": return `ALTER TABLE IF EXISTS ${quoteQualified(c.table, c.schema)} DROP CONSTRAINT IF EXISTS ${quote(c.check)};`;
case "create-view": return renderCreateView(c.view, c.schema, /* orReplace */ false);
case "drop-view": return renderDropView(c);
case "replace-view": return renderCreateView(c.view, c.schema, /* orReplace */ true);
diff --git a/server/typescript/packages/migrate-ts/test/check-evolution/drop-check-down.test.ts b/server/typescript/packages/migrate-ts/test/check-evolution/drop-check-down.test.ts
index 485adb6b0..43b31b69e 100644
--- a/server/typescript/packages/migrate-ts/test/check-evolution/drop-check-down.test.ts
+++ b/server/typescript/packages/migrate-ts/test/check-evolution/drop-check-down.test.ts
@@ -9,8 +9,9 @@ describe("drop-check: restore down + allow gating", () => {
test("drop-check with restore → down re-adds the constraint", () => {
const c = { kind: "drop-check", table: "orders", check: CHK.name, restore: CHK, status: { state: "allowed" } } as unknown as Change;
const r = emit([c], { dialect: "postgres" });
- // #313 — the forward drop carries IF EXISTS; the down (asserted below) stays bare.
- expect(r.up).toContain(`ALTER TABLE "orders" DROP CONSTRAINT IF EXISTS "orders_qty_numeric_chk";`);
+ // #313 — the forward drop carries IF EXISTS on both the constraint AND the
+ // enclosing ALTER TABLE (F10); the down (asserted below) stays bare.
+ expect(r.up).toContain(`ALTER TABLE IF EXISTS "orders" DROP CONSTRAINT IF EXISTS "orders_qty_numeric_chk";`);
expect(r.down).toContain(`ALTER TABLE "orders" ADD CONSTRAINT "orders_qty_numeric_chk" CHECK (qty >= 1);`);
});
test("drop-check is blocked unless allow.dropCheck", () => {
diff --git a/server/typescript/packages/migrate-ts/test/check/emit-postgres-check.test.ts b/server/typescript/packages/migrate-ts/test/check/emit-postgres-check.test.ts
index 52e03a2d6..db3b0c714 100644
--- a/server/typescript/packages/migrate-ts/test/check/emit-postgres-check.test.ts
+++ b/server/typescript/packages/migrate-ts/test/check/emit-postgres-check.test.ts
@@ -24,6 +24,9 @@ describe("emit postgres — checks", () => {
const r = emit([{ kind: "drop-check", table: "orders", check: "orders_status_chk", status: ALLOWED } as unknown as Change], { dialect: "postgres" });
// #313 — the forward drop carries IF EXISTS. The add-check test above asserts the
// matching DOWN, which stays bare; the two together pin the direction split.
- expect(r.up).toContain(`ALTER TABLE "orders" DROP CONSTRAINT IF EXISTS "orders_status_chk";`);
+ // F10 — `IF EXISTS` on the enclosing `ALTER TABLE` too, not just the
+ // constraint name: `DROP CONSTRAINT IF EXISTS` alone still requires the
+ // TABLE to exist, so a table another tool owns still failed the replay.
+ expect(r.up).toContain(`ALTER TABLE IF EXISTS "orders" DROP CONSTRAINT IF EXISTS "orders_status_chk";`);
});
});
diff --git a/server/typescript/packages/migrate-ts/test/emit-drop-if-exists.test.ts b/server/typescript/packages/migrate-ts/test/emit-drop-if-exists.test.ts
index c45ac7c59..a51223830 100644
--- a/server/typescript/packages/migrate-ts/test/emit-drop-if-exists.test.ts
+++ b/server/typescript/packages/migrate-ts/test/emit-drop-if-exists.test.ts
@@ -36,6 +36,10 @@ describe("forward drops tolerate an absent object (#313)", () => {
expect(up).toContain('DROP INDEX IF EXISTS "idx_gone";');
});
+ // F10 — `DROP CONSTRAINT IF EXISTS` alone only guards the constraint NAME;
+ // the enclosing `ALTER TABLE` needs its own `IF EXISTS` too, or a table this
+ // toolchain never created (only #285's constraint-backed index sits on it)
+ // still fails the replay with `relation "t" does not exist`.
test("postgres drop-index, constraint-backed (#285)", () => {
const { up } = renderPostgres([
{
@@ -46,12 +50,12 @@ describe("forward drops tolerate an absent object (#313)", () => {
restore: { name: "uq_gone", columns: ["a"], unique: true, constraint: "unique" },
},
]);
- expect(up).toContain('ALTER TABLE "t" DROP CONSTRAINT IF EXISTS "uq_gone";');
+ expect(up).toContain('ALTER TABLE IF EXISTS "t" DROP CONSTRAINT IF EXISTS "uq_gone";');
});
test("postgres drop-fk", () => {
const { up } = renderPostgres([{ kind: "drop-fk", table: "t", fk: "fk_gone", status: ALLOWED }]);
- expect(up).toContain('ALTER TABLE "t" DROP CONSTRAINT IF EXISTS "fk_gone";');
+ expect(up).toContain('ALTER TABLE IF EXISTS "t" DROP CONSTRAINT IF EXISTS "fk_gone";');
});
// drop-check IS produced by the diff (diff/index.ts:579, :592) — an evolved
@@ -61,7 +65,7 @@ describe("forward drops tolerate an absent object (#313)", () => {
const { up } = renderPostgres([
{ kind: "drop-check", table: "t", check: "t_qty_chk", status: ALLOWED },
]);
- expect(up).toContain('ALTER TABLE "t" DROP CONSTRAINT IF EXISTS "t_qty_chk";');
+ expect(up).toContain('ALTER TABLE IF EXISTS "t" DROP CONSTRAINT IF EXISTS "t_qty_chk";');
});
test("sqlite drop-table", () => {
diff --git a/server/typescript/packages/migrate-ts/test/integrity/replay-emitted-chain.test.ts b/server/typescript/packages/migrate-ts/test/integrity/replay-emitted-chain.test.ts
index 5206cd2a1..5f0499633 100644
--- a/server/typescript/packages/migrate-ts/test/integrity/replay-emitted-chain.test.ts
+++ b/server/typescript/packages/migrate-ts/test/integrity/replay-emitted-chain.test.ts
@@ -88,6 +88,48 @@ describe("an EMITTED chain applies to an empty database (#313)", () => {
}
});
+ // F10 — the CONSTRAINT-level analogue of the REPORTED case above: `theirs` is
+ // never created by this chain, and the diff proposes dropping an FK, a CHECK,
+ // and a constraint-backed (unique) index on it — the three change kinds whose
+ // renderer guards the CONSTRAINT name with `IF EXISTS` but leaves the
+ // enclosing `ALTER TABLE` bare, so it still fails against a virgin database
+ // with `relation "theirs" does not exist`; the #313 guarantee was partial for
+ // exactly these three kinds. Postgres-only: SQLite emits no standalone
+ // statement for drop-fk/drop-check at all (folded into a table recreate that
+ // rebuilds from the EXPECTED descriptor), so it cannot exhibit this bug.
+ test("postgres: dropping an fk/check/constraint-backed-index on a table the chain never created still applies", async () => {
+ const dir = mkdtempSync(join(tmpdir(), "replay-emitted-unmanaged-constraint-"));
+ const engine = await openReplayEngine("postgres");
+ const unmanagedConstraintDrops: Change[] = [
+ {
+ kind: "create-table",
+ status: ALLOWED,
+ table: {
+ name: "mine",
+ columns: [{ name: "id", sqlType: { kind: "integer", bits: 64 }, nullable: false }],
+ indexes: [],
+ foreignKeys: [],
+ checks: [],
+ primaryKey: ["id"],
+ },
+ },
+ { kind: "drop-fk", table: "theirs", fk: "theirs_owner_fk", status: ALLOWED },
+ { kind: "drop-check", table: "theirs", check: "theirs_status_chk", status: ALLOWED },
+ {
+ kind: "drop-index", table: "theirs", index: "theirs_code_uniq", status: ALLOWED,
+ restore: { name: "theirs_code_uniq", columns: ["code"], unique: true, constraint: "unique" },
+ },
+ ];
+ try {
+ await writeMigration(emit(unmanagedConstraintDrops, { dialect: "postgres" }), { dir, slug: "init" });
+ const applied = await applyPending(engine.db, dir, { dryRun: false, dialect: "postgres" });
+ expect(applied.applied).toHaveLength(1);
+ } finally {
+ await engine.dispose();
+ rmSync(dir, { recursive: true, force: true });
+ }
+ });
+
// The control: without it the cases above could pass because `applyPending`
// swallows a failing statement rather than because the emitter stopped writing
// one. This proves the assertion has teeth.
diff --git a/server/typescript/packages/migrate-ts/test/unit/emit-postgres.test.ts b/server/typescript/packages/migrate-ts/test/unit/emit-postgres.test.ts
index afcf65795..9a9f13379 100644
--- a/server/typescript/packages/migrate-ts/test/unit/emit-postgres.test.ts
+++ b/server/typescript/packages/migrate-ts/test/unit/emit-postgres.test.ts
@@ -165,6 +165,19 @@ describe("renderPostgres — indexes + FKs", () => {
expect(norm(up)).toBe(`DROP INDEX IF EXISTS "old_idx";`);
});
+ // F10 — the constraint-backed arm ALSO guards the enclosing ALTER TABLE, not
+ // just the constraint name (see the `drop-fk` test below for the failure mode).
+ test("drop-index (constraint-backed)", () => {
+ const changes: Change[] = [{
+ kind: "drop-index", table: "work_items", index: "work_items_message_id_uniq", status: ALLOWED,
+ restore: { name: "work_items_message_id_uniq", columns: ["message_id"], unique: true, constraint: "unique" },
+ }];
+ const { up } = emit(changes, { dialect: "postgres" });
+ expect(norm(up)).toBe(
+ `ALTER TABLE IF EXISTS "work_items" DROP CONSTRAINT IF EXISTS "work_items_message_id_uniq";`,
+ );
+ });
+
test("add-fk with ON DELETE CASCADE", () => {
const changes: Change[] = [{
kind: "add-fk", table: "weeks",
@@ -182,10 +195,15 @@ describe("renderPostgres — indexes + FKs", () => {
);
});
+ // F10 — `DROP CONSTRAINT IF EXISTS` alone only guards the constraint NAME;
+ // Postgres still requires the TABLE to exist to parse `ALTER TABLE` at all, so
+ // this failed to replay against a virgin database for a table another tool
+ // owns (never created by any migration in the chain) with `relation "x" does
+ // not exist`. `ALTER TABLE IF EXISTS` closes that gap.
test("drop-fk", () => {
const changes: Change[] = [{ kind: "drop-fk", table: "weeks", fk: "weeks_program_id_fk", status: ALLOWED }];
const { up } = emit(changes, { dialect: "postgres" });
- expect(norm(up)).toBe(`ALTER TABLE "weeks" DROP CONSTRAINT IF EXISTS "weeks_program_id_fk";`);
+ expect(norm(up)).toBe(`ALTER TABLE IF EXISTS "weeks" DROP CONSTRAINT IF EXISTS "weeks_program_id_fk";`);
});
});
diff --git a/server/typescript/packages/sdk/src/metadata-files.ts b/server/typescript/packages/sdk/src/metadata-files.ts
index bcddc4679..3cc1ad54e 100644
--- a/server/typescript/packages/sdk/src/metadata-files.ts
+++ b/server/typescript/packages/sdk/src/metadata-files.ts
@@ -15,7 +15,7 @@
// sides import is the fix; a lazy `await import()` inside `loadMemory` is not
// — that hides the cycle rather than removing it.
import { extname, join } from "node:path";
-import { readdir, stat } from "node:fs/promises";
+import { readdir, realpath, stat } from "node:fs/promises";
/**
* The DEFAULT value of `sources` — the directory scanned when
@@ -86,10 +86,42 @@ const PENDING_DIR = "_pending";
* read the directory itself still throws — that is the "you have no metadata
* here" case callers report.
*
+ * A symlink CYCLE (e.g. `metaobjects/link -> ..`) is a loud error rather than
+ * unbounded recursion — see {@link listMetadataFilesGuarded}.
+ *
* Format selection (parsing) happens downstream in `FileSource` from
* `@metaobjectsdev/metadata`, which infers the parser from file extension.
*/
export async function listMetadataFiles(dir: string): Promise {
+ return listMetadataFilesGuarded(dir, new Set());
+}
+
+/**
+ * {@link listMetadataFiles}'s recursive worker, carrying the REAL (symlink-
+ * resolved) ancestor directories already on this walk branch.
+ *
+ * This walk follows symlinked directories on purpose (`stat`, not `lstat`,
+ * below — matching `DirectorySource` in `@metaobjectsdev/metadata`), so an
+ * unguarded directory symlink that revisits an ancestor recurses forever:
+ * Java and Python both added this exact guard when this PR promoted
+ * symlink-following to a cross-port contract; the TypeScript reference itself
+ * did not, even though the corpus cites it as authoritative. `ancestors` is
+ * extended only on the recursive call (never mutated in place), so it
+ * reflects the current branch, not siblings visited earlier at the same
+ * level — a directory legitimately reachable via two different symlinked
+ * paths (not a cycle) is not falsely rejected.
+ */
+async function listMetadataFilesGuarded(dir: string, ancestors: ReadonlySet): Promise {
+ // `realpath` failing (e.g. `dir` vanished between being listed and now) is
+ // not this guard's problem — fall back to the given path and let `readdir`
+ // below raise its own coded error.
+ const real = await realpath(dir).catch(() => dir);
+ if (ancestors.has(real)) {
+ throw new Error(`symlink loop detected while expanding metadata directory: ${dir} revisits ${real}`);
+ }
+ const nextAncestors = new Set(ancestors);
+ nextAncestors.add(real);
+
let entries: string[];
try {
entries = await readdir(dir);
@@ -120,7 +152,7 @@ export async function listMetadataFiles(dir: string): Promise {
// Recurse into subdirectories after collecting files at this level.
// `subdirs` is already in sorted order (built from the sorted `entries` above).
for (const sub of subdirs) {
- paths.push(...(await listMetadataFiles(sub)));
+ paths.push(...(await listMetadataFilesGuarded(sub, nextAncestors)));
}
return paths;
}
diff --git a/server/typescript/packages/sdk/test/sources.test.ts b/server/typescript/packages/sdk/test/sources.test.ts
index 07f503686..ac2d10bf5 100644
--- a/server/typescript/packages/sdk/test/sources.test.ts
+++ b/server/typescript/packages/sdk/test/sources.test.ts
@@ -124,6 +124,19 @@ describe("resolveSources", () => {
expect(out).toHaveLength(2);
});
+ test("a symlink cycle raises a clear error, not a hang or a stack overflow", async () => {
+ // F7 — the reference walk (`listMetadataFiles`, `metadata-files.ts`) follows a
+ // symlinked directory via `stat` (not `lstat`) with no loop guard. This PR
+ // promoted symlink-following to a cross-port contract and Java/Python both
+ // added cycle detection when they picked it up; the reference itself did not.
+ // `model/loop -> model` is self-referential: walking into `loop` re-lists
+ // `model` (now reached as `model/loop`), which contains `loop` again, forever.
+ write("model/meta.a.json");
+ const { symlinkSync } = await import("node:fs");
+ symlinkSync(join(root, "model"), join(root, "model/loop"), "dir");
+ await expect(resolveSources(root, [{ path: "model" }])).rejects.toThrow();
+ });
+
test("a dangling symlink inside a source directory is skipped, not a raw ENOENT crash", async () => {
// DirectorySource in @metaobjectsdev/metadata catches and skips exactly
// this case (directory-source.ts). Before the fix, the bare `stat()` in