fix: re-resolve module imports before checking staleness

Adding a data member to a class in a module interface did not rebuild every
object compiled against the old layout. The build succeeded with no error or
warning and the resulting binary mixed both layouts, surfacing later as a
SIGSEGV in a destructor.

GetInterfacesAndImplementations scans a TU's `import X;` statements when the
source is declared. An import that matches neither a module in the
Configuration nor one reachable through `dependencies` was dropped on the
floor, leaving that TU with no staleness edge to the interface it consumes.
`dependencies` is frequently assigned *after* the scan — AddTest does exactly
that, resolving tests/<name>/main.cpp and only then returning a builder whose
.Dependencies() supplies the library — so consumers of a dependency's modules
routinely carried no edge at all. A layout change then rebuilt the library,
relinked the consumer, and kept the consumer's object as it was.

Unresolved names are now remembered on the partition/implementation as
pendingImports, and Configuration::ResolvePendingImports retries them against
the dependency DAG as it stands. Build() calls it immediately before comparing
mtimes, which closes the window for every caller rather than only the ones that
declare in the right order; TestBuilder::Dependencies also calls it so the
Configuration is coherent for anyone inspecting it before the build.

Resolves #27

implementations/Crafter.Build-Clang.cpp

@@ -25,9 +25,14 @@ namespace fs = std::filesystem;
 using namespace Crafter;
 
 
-void Configuration::GetInterfacesAndImplementations(std::span<fs::path> interfaces, std::span<fs::path> implementations) {
-    auto resolveImport = [this](const std::string& importName, std::vector<Module*>& localDeps, std::vector<std::pair<Module*, fs::path>>& externalDeps) -> bool {
-        for(const std::unique_ptr<Module>& interface : this->interfaces) {
+namespace {
+    // Map one `import X;` name onto either a module this Configuration owns or
+    // one exported by a Configuration reachable through its dependency DAG,
+    // appending the matching staleness edge. False means nothing in reach
+    // provides X — either it's supplied from outside the graph (`std`) or the
+    // dependency isn't wired up *yet*, which is why callers remember the name.
+    bool ResolveImportName(Configuration& cfg, const std::string& importName, std::vector<Module*>& localDeps, std::vector<std::pair<Module*, fs::path>>& externalDeps) {
+        for(const std::unique_ptr<Module>& interface : cfg.interfaces) {
             if(interface->name == importName) {
                 localDeps.push_back(interface.get());
                 return true;
@@ -50,10 +55,38 @@ void Configuration::GetInterfacesAndImplementations(std::span<fs::path> interfac
             return false;
         };
 
-        for(Configuration* depCfg : this->dependencies) {
+        for(Configuration* depCfg : cfg.dependencies) {
             if (walk(depCfg)) return true;
         }
         return false;
+    }
+}
+
+void Configuration::ResolvePendingImports() {
+    // Discard the local-module hits: a name recorded as pending already failed
+    // to match this Configuration's own interfaces, and nothing adds interfaces
+    // between the scan and here. Only the external edge can newly appear.
+    std::vector<Module*> ignored;
+    auto sweep = [this, &ignored](std::vector<std::string>& pending, std::vector<std::pair<Module*, fs::path>>& externalDeps) {
+        std::erase_if(pending, [&](const std::string& name) {
+            return ResolveImportName(*this, name, ignored, externalDeps);
+        });
+    };
+    for(const std::unique_ptr<Module>& interface : interfaces) {
+        for(const std::unique_ptr<ModulePartition>& partition : interface->partitions) {
+            sweep(partition->pendingImports, partition->externalModuleDependencies);
+        }
+    }
+    for(Implementation& implementation : implementations) {
+        sweep(implementation.pendingImports, implementation.externalModuleDependencies);
+    }
+}
+
+void Configuration::GetInterfacesAndImplementations(std::span<fs::path> interfaces, std::span<fs::path> implementations) {
+    auto resolveImport = [this](const std::string& importName, std::vector<Module*>& localDeps, std::vector<std::pair<Module*, fs::path>>& externalDeps, std::vector<std::string>& pending) {
+        if (!ResolveImportName(*this, importName, localDeps, externalDeps)) {
+            pending.push_back(importName);
+        }
     };
 
     std::vector<std::tuple<fs::path, std::string, ModulePartition*, Module*>> tempModulePaths = std::vector<std::tuple<fs::path, std::string, ModulePartition*, Module*>>(interfaces.size());
@@ -128,7 +161,7 @@ void Configuration::GetInterfacesAndImplementations(std::span<fs::path> interfac
         std::sregex_iterator modCurrent(fileContent.begin(), fileContent.end(), modulePattern);
         while (modCurrent != lastMatch) {
             std::smatch match = *modCurrent;
-            resolveImport(match[1].str(), partition->moduleDependencies, partition->externalModuleDependencies);
+            resolveImport(match[1].str(), partition->moduleDependencies, partition->externalModuleDependencies, partition->pendingImports);
             ++modCurrent;
         }
     }
@@ -173,7 +206,7 @@ void Configuration::GetInterfacesAndImplementations(std::span<fs::path> interfac
                     while (modCurrent != lastMatch) {
                         std::smatch match2 = *modCurrent;
                         if (match2[1] != match[1]) {
-                            resolveImport(match2[1].str(), implementation.moduleDependencies, implementation.externalModuleDependencies);
+                            resolveImport(match2[1].str(), implementation.moduleDependencies, implementation.externalModuleDependencies, implementation.pendingImports);
                         }
                         ++modCurrent;
                     }
@@ -188,7 +221,7 @@ void Configuration::GetInterfacesAndImplementations(std::span<fs::path> interfac
             std::sregex_iterator lastMatch;
             while (currentMatch != lastMatch) {
                 std::smatch match2 = *currentMatch;
-                resolveImport(match2[1].str(), implementation.moduleDependencies, implementation.externalModuleDependencies);
+                resolveImport(match2[1].str(), implementation.moduleDependencies, implementation.externalModuleDependencies, implementation.pendingImports);
                 ++currentMatch;
             }
         }
@@ -224,6 +257,18 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
         }
     }
 
+    // Sources were scanned when they were declared, which for most callers is
+    // before `dependencies` exists — AddTest resolves tests/<name>/main.cpp and
+    // only then hands back a builder whose .Dependencies() supplies the library.
+    // Any `import <DepModule>;` in such a TU resolved to nothing and so carried
+    // no staleness edge, which meant a member added to a dependency's interface
+    // rebuilt the library, relinked the consumer, and silently kept the
+    // consumer's object compiled against the *old* class layout (issue #27).
+    // Re-resolving here — the last point before mtimes are compared, with the
+    // DAG fully wired — closes that window for every caller rather than for the
+    // ones that remember to declare in the right order.
+    config.ResolvePendingImports();
+
     // Auto-detect the WASI sysroot before any compile step runs so BuildStdPcm
     // and the main compile command see the same value. Linux-only — Windows
     // users supply cfg.sysroot pointing at their wasi-sdk install. Covers all

implementations/Crafter.Build-Test.cpp

@@ -483,6 +483,14 @@ TestBuilder& TestBuilder::Args(std::vector<std::string> a)  { Ref().args = std::
 TestBuilder& TestBuilder::Requires(std::string r)           { Ref().requires_.push_back(std::move(r)); return *this; }
 TestBuilder& TestBuilder::Dependencies(std::vector<Configuration*> d) {
     Ref().config.dependencies = std::move(d);
+    // AddTest already scanned tests/<name>/main.cpp, at which point this test
+    // had no dependencies, so every `import <DepModule>;` in it came back
+    // unresolved. Place them now that the libraries are known — without this
+    // the test's object carries no staleness edge to the interfaces it consumes
+    // and survives a layout change to them (issue #27). Build() re-runs the
+    // same sweep as a backstop; doing it here keeps the Configuration coherent
+    // for anyone inspecting it before the build.
+    Ref().config.ResolvePendingImports();
     return *this;
 }
 TestBuilder& TestBuilder::LinkFlag(std::string f)           { Ref().config.linkFlags.push_back(std::move(f)); return *this; }

interfaces/Crafter.Build-Clang.cppm

@@ -254,6 +254,15 @@ export namespace Crafter {
         // Lint rules for `crafter-build lint`. Populate via AddLintRule.
         std::vector<LintRule> lintRules;
         CRAFTER_API void GetInterfacesAndImplementations(std::span<fs::path> interfaces, std::span<fs::path> implementations);
+        // Retry the `import X;` names GetInterfacesAndImplementations could not
+        // place, against the dependency DAG as it stands now. Sources are
+        // scanned when they're declared, but `dependencies` is often assigned
+        // afterwards (AddTest does exactly this), and an import that resolved to
+        // nothing leaves no staleness edge — so a dependency's interface could
+        // change and this Configuration's objects would be silently reused
+        // against the new layout. Build() calls this before checking mtimes;
+        // calling it again is harmless.
+        CRAFTER_API void ResolvePendingImports();
         // Declare a test. Sources default to `tests/<name>/main.cpp` resolved
         // against this Configuration's path; target/march/mtune/sysroot/debug
         // are inherited from this Configuration so cross-arch projects don't

interfaces/Crafter.Build-Implementation.cppm

@@ -15,6 +15,9 @@ namespace Crafter {
         std::vector<Module*> moduleDependencies;
         std::vector<ModulePartition*> partitionDependencies;
         std::vector<std::pair<Module*, fs::path>> externalModuleDependencies;
+        // See ModulePartition::pendingImports — imports this TU declared that
+        // no reachable Configuration provided when the source was scanned.
+        std::vector<std::string> pendingImports;
         fs::path path;
         CRAFTER_API Implementation(fs::path&& path);
         CRAFTER_API bool Check(const fs::path& buildDir, const fs::path& pcmDir, fs::file_time_type sourceFloor = fs::file_time_type::min()) const;

interfaces/Crafter.Build-Interface.cppm

@@ -14,6 +14,13 @@ namespace Crafter {
         std::vector<Module*> moduleDependencies;
         std::vector<ModulePartition*> partitionDependencies;
         std::vector<std::pair<Module*, fs::path>> externalModuleDependencies;
+        // Names from `import X;` that matched neither a module in this
+        // Configuration nor one reachable through its dependencies at the time
+        // the source was scanned. Retried by ResolvePendingImports (see
+        // Crafter.Build:Clang) so a dependency wired up after
+        // GetInterfacesAndImplementations still produces a staleness edge.
+        // Names that never resolve (`std`, module-mapped externals) stay here.
+        std::vector<std::string> pendingImports;
         std::atomic<bool> compiled;
         bool needsRecompiling;
         bool checked = false;

project.cpp

@@ -107,6 +107,7 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
         cfg.AddTest("StaticLib").Dependencies({ CrafterBuildLib.get() });
         cfg.AddTest("ModuleInterface").Dependencies({ CrafterBuildLib.get() });
         cfg.AddTest("DependencyLink").Dependencies({ CrafterBuildLib.get() });
+        cfg.AddTest("IncrementalInterfaceChange").Dependencies({ CrafterBuildLib.get() });
         cfg.AddTest("ShaderCompile").Dependencies({ CrafterBuildLib.get() });
         cfg.AddTest("StandardArgs").Dependencies({ CrafterBuildLib.get() });
         cfg.AddTest("TestRunnerSpec").Dependencies({ CrafterBuildLib.get() });

tests/IncrementalInterfaceChange/fixture/lib/Widget-grown.cppm.in

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
+
+// Copied over lib/Widget.cppm mid-test to stand in for the interface edit from
+// issue #27: one extra data member, every signature and mangled name unchanged.
+// Kept as a .cppm.in so the module scanner never treats it as a source of its
+// own — and so the text does not have to live in a string literal inside the
+// test, where `export module Widget;` would make the test itself look like an
+// implementation unit of Widget.
+
+export module Widget;
+import std;
+
+export struct Widget {
+    std::string a;
+    std::string b;
+};
+
+export std::size_t WidgetSizeInLibrary();

tests/IncrementalInterfaceChange/fixture/lib/Widget.cpp

@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
+
+module Widget;
+import std;
+
+std::size_t WidgetSizeInLibrary() { return sizeof(Widget); }

tests/IncrementalInterfaceChange/fixture/lib/Widget.cppm

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
+
+export module Widget;
+import std;
+
+// The test rewrites this struct to add a member. Adding one changes the class
+// layout while leaving every signature and mangled name untouched, so nothing
+// downstream fails to link — a consumer object left over from before the change
+// keeps the old sizeof and quietly disagrees with the library.
+export struct Widget {
+    std::string a;
+};
+
+// Deliberately out-of-line, in the library's own implementation unit, so the
+// value reflects the layout the *library* was compiled against rather than the
+// caller's. An inline body would be instantiated from the (rebuilt) BMI in the
+// consumer and would agree with it by construction.
+export std::size_t WidgetSizeInLibrary();

tests/IncrementalInterfaceChange/fixture/main.cpp

@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
+
+import std;
+import Widget;
+
+// Prints "<size the consumer was compiled against> <size the library was
+// compiled against>" and exits 1 when they disagree. A mismatch is the
+// observable form of the mixed-layout binary a stale consumer object produces —
+// this reports it instead of waiting for the SIGSEGV that the real-world case
+// (issue #27) produced in a destructor.
+int main() {
+    std::size_t here = sizeof(Widget);
+    std::size_t inLibrary = WidgetSizeInLibrary();
+    std::print("{} {}", here, inLibrary);
+    return here == inLibrary ? 0 : 1;
+}

tests/IncrementalInterfaceChange/main.cpp

@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
+
+import std;
+import Crafter.Build;
+namespace fs = std::filesystem;
+using namespace Crafter;
+
+// Adding a data member to a class in a module interface must rebuild every
+// object compiled against the old layout. The dangerous shape (issue #27) is a
+// consumer whose sources were scanned *before* its `dependencies` were assigned:
+// its `import <DepModule>;` matched nothing, so its object carried no staleness
+// edge to the interface, and a layout change rebuilt the library, relinked the
+// consumer, and produced a binary mixing both layouts with no error or warning.
+//
+// `AddTest` is exactly that shape — it scans tests/<name>/main.cpp and only then
+// returns a builder whose .Dependencies() supplies the library — which is why
+// the original report saw the corruption in test executables specifically.
+
+namespace {
+    std::int32_t Failures = 0;
+
+    void Check(bool cond, std::string_view msg) {
+        if (!cond) {
+            std::println(std::cerr, "FAIL: {}", msg);
+            ++Failures;
+        }
+    }
+
+    // The fixture is mutated during the run, so work on a copy outside the repo.
+    fs::path StageFixture() {
+        fs::path source = fs::current_path() / "tests" / "IncrementalInterfaceChange" / "fixture";
+        fs::path staged = fs::temp_directory_path() / "crafter-build-incremental-interface-change";
+        fs::remove_all(staged);
+        fs::copy(source, staged, fs::copy_options::recursive);
+        return staged;
+    }
+
+    // Swap in the variant carrying the extra member. Comes from a file rather
+    // than a string literal here: the module scanner reads raw source, so an
+    // `export module Widget;` spelled inside this test would make the test look
+    // like an implementation unit of Widget.
+    void GrowWidget(const fs::path& staged) {
+        fs::copy_file(staged / "lib" / "Widget-grown.cppm.in", staged / "lib" / "Widget.cppm", fs::copy_options::overwrite_existing);
+        // copy_file carries the source's mtime across, which would leave the
+        // rewritten interface looking older than the BMI built from it.
+        fs::last_write_time(staged / "lib" / "Widget.cppm", fs::file_time_type::clock::now());
+    }
+
+    std::unique_ptr<Configuration> MakeLib(const fs::path& staged) {
+        auto lib = std::make_unique<Configuration>();
+        lib->path = staged / "lib";
+        lib->name = "widget";
+        lib->outputName = "widget";
+        lib->target = HostTarget();
+        lib->type = ConfigurationType::LibraryStatic;
+        std::array<fs::path, 1> ifaces = { "Widget" };
+        std::array<fs::path, 1> impls = { "Widget" };
+        lib->GetInterfacesAndImplementations(ifaces, impls);
+        return lib;
+    }
+
+    // Scan first, wire the dependency up afterwards — the ordering that used to
+    // silently drop the staleness edge.
+    Configuration MakeConsumerScannedBeforeDependencies(const fs::path& staged, Configuration* lib) {
+        Configuration app;
+        app.path = staged;
+        app.name = "widget-app";
+        app.outputName = "widget-app";
+        app.target = HostTarget();
+        app.type = ConfigurationType::Executable;
+        std::array<fs::path, 0> ifaces = {};
+        std::array<fs::path, 1> impls = { "main" };
+        app.GetInterfacesAndImplementations(ifaces, impls);
+        app.dependencies = { lib };
+        return app;
+    }
+
+    bool BuildOk(Configuration& app, std::string_view label) {
+        // A fresh depResults per pass: the map memoizes each Configuration's
+        // build for the duration of one pass, so reusing it would skip the
+        // library's second build entirely.
+        std::unordered_map<fs::path, std::shared_future<BuildResult>> depResults;
+        std::mutex depMutex;
+        BuildResult r = Build(app, depResults, depMutex);
+        if (!r.result.empty()) {
+            std::println(std::cerr, "FAIL: {} build failed: {}", label, r.result);
+            ++Failures;
+            return false;
+        }
+        return true;
+    }
+}
+
+int main() {
+    {
+        // The scan leaves the unmatched import recorded rather than forgotten,
+        // and ResolvePendingImports places it once the library is reachable.
+        fs::path staged = StageFixture();
+        std::unique_ptr<Configuration> lib = MakeLib(staged);
+        Configuration app;
+        app.path = staged;
+        app.name = "widget-app";
+        app.outputName = "widget-app";
+        app.target = HostTarget();
+        app.type = ConfigurationType::Executable;
+        std::array<fs::path, 0> ifaces = {};
+        std::array<fs::path, 1> impls = { "main" };
+        app.GetInterfacesAndImplementations(ifaces, impls);
+
+        Check(app.implementations.size() == 1, "consumer has one implementation");
+        if (app.implementations.size() == 1) {
+            const Implementation& impl = app.implementations[0];
+            Check(impl.externalModuleDependencies.empty(), "no external dep resolvable before dependencies are assigned");
+            Check(std::ranges::find(impl.pendingImports, "Widget") != impl.pendingImports.end(), "unresolved 'import Widget;' is recorded as pending");
+
+            app.dependencies = { lib.get() };
+            app.ResolvePendingImports();
+
+            Check(impl.externalModuleDependencies.size() == 1, "ResolvePendingImports adds the external module dep");
+            if (impl.externalModuleDependencies.size() == 1) {
+                Check(impl.externalModuleDependencies[0].first->name == "Widget", "external dep is the Widget module");
+                Check(impl.externalModuleDependencies[0].second == lib->PcmDir() / "Widget.pcm", "external dep points at the library's BMI");
+            }
+            Check(std::ranges::find(impl.pendingImports, "Widget") == impl.pendingImports.end(), "resolved import is no longer pending");
+
+            // Idempotent: a second sweep must not duplicate the edge (Build
+            // runs one unconditionally, on top of whatever callers already did).
+            app.ResolvePendingImports();
+            Check(impl.externalModuleDependencies.size() == 1, "ResolvePendingImports is idempotent");
+        }
+    }
+
+    {
+        // AddTest is the reported path: it scans the test source, then hands
+        // back a builder whose .Dependencies() names the library.
+        fs::path staged = StageFixture();
+        fs::create_directories(staged / "tests" / "Consumer");
+        fs::copy_file(staged / "main.cpp", staged / "tests" / "Consumer" / "main.cpp");
+        std::unique_ptr<Configuration> lib = MakeLib(staged);
+
+        Configuration app;
+        app.path = staged;
+        app.name = "host";
+        app.outputName = "host";
+        app.target = HostTarget();
+        app.type = ConfigurationType::Executable;
+        // AddTest resolves tests/<name>/main against the cwd, which for a real
+        // run is the directory holding project.cpp. Stand in the staged project
+        // for the declaration so the fixture's test source is the one scanned.
+        fs::path restore = fs::current_path();
+        fs::current_path(staged);
+        app.AddTest("Consumer").Dependencies({ lib.get() });
+        fs::current_path(restore);
+
+        Check(app.tests.size() == 1, "one test declared");
+        if (app.tests.size() == 1 && app.tests[0].config.implementations.size() == 1) {
+            const Implementation& impl = app.tests[0].config.implementations[0];
+            Check(impl.externalModuleDependencies.size() == 1, "AddTest(...).Dependencies() resolves the test's import of the library module");
+            Check(impl.pendingImports.empty() || std::ranges::find(impl.pendingImports, "Widget") == impl.pendingImports.end(), "test's 'import Widget;' is no longer pending");
+        }
+    }
+
+    {
+        // End to end, and deliberately without calling ResolvePendingImports:
+        // the guarantee under test is that Build() closes the window on its own,
+        // for consumers that never knew they had to ask.
+        fs::path staged = StageFixture();
+        std::unique_ptr<Configuration> lib = MakeLib(staged);
+        Configuration app = MakeConsumerScannedBeforeDependencies(staged, lib.get());
+        fs::path binary = app.BinDir() / "widget-app";
+        fs::path consumerObject = app.BuildDir() / "main_impl.o";
+
+        if (BuildOk(app, "first pass")) {
+            auto first = RunCommandWithTimeout(binary.string(), std::chrono::seconds(30));
+            Check(first.exitCode == 0 && !first.crashed && !first.timedOut, std::format("first pass agrees on the layout (exit={} output='{}')", first.exitCode, first.output));
+
+            fs::file_time_type objectBefore = fs::last_write_time(consumerObject);
+
+            // Same edit as the original report: one more member on a class in a
+            // module interface. Every signature and mangled name is unchanged,
+            // so a missed rebuild produces no diagnostic of any kind.
+            GrowWidget(staged);
+
+            if (BuildOk(app, "second pass")) {
+                Check(fs::last_write_time(consumerObject) > objectBefore, "consumer object is recompiled after the interface gains a member");
+
+                auto second = RunCommandWithTimeout(binary.string(), std::chrono::seconds(30));
+                Check(second.exitCode == 0 && !second.crashed && !second.timedOut, std::format("second pass agrees on the layout (exit={} output='{}')", second.exitCode, second.output));
+            }
+        }
+    }
+
+    if (Failures > 0) {
+        std::println(std::cerr, "{} assertions failed", Failures);
+        return 1;
+    }
+    return 0;
+}