fix(xlings): read the real subos_info wire format, and refuse a pre-upgrade cache

Carried over from the PR this work supersedes; the two defects and their
evidence are unchanged.

2026.8.8.1 shipped a reader for a format that does not exist. `envs` is an
object keyed by binding, the reader expected an array of {binding, decls}, so
is_array() was false, the loop never ran, and against a real subos every
variable came back unset -- LIBGL_DRIVERS_PATH, __EGL_VENDOR_LIBRARY_DIRS,
XDG_DATA_DIRS still the host's. The whole #352 fix was inert.

Ten unit tests and an e2e passed throughout, because every fixture was
hand-written in the same invented shape the parser expected. A fixture
composed from the same understanding as the parser cannot catch a
misunderstanding of the wire format; only one taken from the writer can.
RealXlingsCapture is that -- verbatim output from a real `xlings install
graphics`, reformatted for width and nothing else. Transcribing the parser
from xlings's own reader also caught a second divergence: xlings drops a
declaration whose op is neither "set" nor "prepend", and this accepted any.

Second defect: `mcpp run`'s fast path replayed a cache written before the
subos field existed, so the fix did not survive an upgrade -- and not just for
one run. The fast path's identity is the profile, cache mode and resource
list, and its fingerprint check compares an entry against itself; nothing
notices that a different mcpp wrote it. Measured on a real 2026.8.7.1 ->
2026.8.8.1 upgrade. An empty subosDir cannot stand in for "predates the
field", since a system toolchain legitimately has no subos, so presence is
tracked separately.

Author: speak-agent

src/build/execute.cppm

@@ -68,6 +68,11 @@ struct BuildCacheEntry {
     // property and would otherwise be unknowable on the fast path -- which
     // has no toolchain to derive it from.
     std::string subosDir;
+    // Was the line present at all? An EMPTY subosDir is a legitimate answer
+    // (a system toolchain outside the xpkgs store has no subos), so it cannot
+    // stand in for "this cache predates the field" -- and those two need
+    // opposite treatment: the first runs, the second must rebuild once.
+    bool        subosRecorded = false;
     // The resolved profile this entry was built for. Entries used to be keyed
     // by target triple alone, and the fast paths only refuse to run when an
     // EXPLICIT --profile/--dev/--release is passed — so a bare `mcpp build`
@@ -154,7 +159,8 @@ std::vector<BuildCacheEntry> read_build_cache(const std::filesystem::path& proje
         // using the cache at all -- the program would work once and then
         // silently stop finding its runtime data.
         if (haveNextLine && line.starts_with("subos=")) {
-            e.subosDir = line.substr(6);
+            e.subosDir      = line.substr(6);
+            e.subosRecorded = true;
             haveNextLine = static_cast<bool>(std::getline(f, line));
         }
         // Optional profile line. Same back-compat contract as the two blocks
@@ -201,7 +207,8 @@ void write_build_cache(const std::filesystem::path& projectRoot,
     // Insert at front (MRU).
     BuildCacheEntry newEntry{targetTriple, outputDir.string(), ninjaProgram, fingerprintHex,
                              runtimeEnvKey, runtimeEnvValue, std::move(runTargets),
-                             runEnvKey, runEnvValue, subosDir, profile, cacheMode};
+                             runEnvKey, runEnvValue, subosDir, /*subosRecorded=*/true,
+                             profile, cacheMode};
     entries.insert(entries.begin(), std::move(newEntry));
 
     // Trim to LRU capacity.
@@ -774,6 +781,18 @@ std::optional<int> try_fast_run(const std::filesystem::path& projectRoot,
         ninjaProgram = ninjaProgram.substr(1, ninjaProgram.size() - 2);
     if (match->runtimeEnvKey.empty())
         return std::nullopt; // old cache entry; go through prepare_build once
+    // Written before this mcpp knew about subos environments (mcpp#352). Taking
+    // the fast path here would run the program without them -- which is the
+    // defect this field exists to fix, surviving an upgrade.
+    //
+    // It survives it for a long time, too: the fast path's identity is the
+    // profile, the cache mode and the resource list, and its fingerprint check
+    // compares a cached entry against ITSELF. Neither notices that a different
+    // mcpp wrote the entry, so without this line an upgraded mcpp would reuse a
+    // pre-upgrade build until something else happened to invalidate it. Measured
+    // on a real upgrade from 2026.8.7.1, not reasoned about.
+    if (!match->subosRecorded)
+        return std::nullopt; // predates `subos=`; rebuild once, then it is there
 
     // P1: verify fingerprint matches the outputDir basename.
     if (!match->fingerprint.empty()) {

src/xlings/subos_info.cppm

@@ -131,28 +131,39 @@ Info read(const std::filesystem::path& subosDir) {
     if (auto v = it->find("runtime"); v != it->end() && v->is_string())
         info.runtime = v->get<std::string>();
 
-    if (auto envs = it->find("envs"); envs != it->end() && envs->is_array()) {
-        for (auto const& p : *envs) {
-            if (!p.is_object()) continue;
+    // `envs` is an OBJECT keyed by binding, whose values are arrays of
+    // declarations:
+    //
+    //   "envs": { "mesa@25.0.7.1": [ {"var":…,"op":…,"value":…}, … ], … }
+    //
+    // Transcribed from xlings's own reader (core/subos/manifest.cppm), not
+    // from a model of it. The first version of this file expected an array of
+    // {binding, decls} objects — a shape xlings never writes — and its tests
+    // hand-wrote JSON in that same invented shape, so both agreed and both
+    // were wrong. Against a real subos the loop simply never ran and every
+    // variable came back unset, silently. That is why the fixture below is a
+    // verbatim capture of real output rather than something composed here.
+    if (auto envs = it->find("envs"); envs != it->end() && envs->is_object()) {
+        for (auto e = envs->begin(); e != envs->end(); ++e) {
+            if (!e.value().is_array()) continue;
             Provider prov;
-            if (auto b = p.find("binding"); b != p.end() && b->is_string())
-                prov.binding = b->get<std::string>();
-            if (auto ds = p.find("decls"); ds != p.end() && ds->is_array()) {
-                for (auto const& d : *ds) {
-                    if (!d.is_object()) continue;
-                    EnvDecl e;
-                    if (auto x = d.find("var");   x != d.end() && x->is_string())
-                        e.var = x->get<std::string>();
-                    if (auto x = d.find("op");    x != d.end() && x->is_string())
-                        e.op = x->get<std::string>();
-                    if (auto x = d.find("value"); x != d.end() && x->is_string())
-                        e.value = x->get<std::string>();
-                    // A declaration with no variable name is not a partial
-                    // declaration to be guessed at — it is malformed input,
-                    // and the right thing is to leave it out rather than
-                    // invent a name for it.
-                    if (!e.var.empty()) prov.decls.push_back(std::move(e));
-                }
+            prov.binding = e.key();
+            for (auto const& d : e.value()) {
+                if (!d.is_object()) continue;
+                EnvDecl decl;
+                if (auto x = d.find("var");   x != d.end() && x->is_string())
+                    decl.var = x->get<std::string>();
+                if (auto x = d.find("op");    x != d.end() && x->is_string())
+                    decl.op = x->get<std::string>();
+                if (auto x = d.find("value"); x != d.end() && x->is_string())
+                    decl.value = x->get<std::string>();
+                // xlings drops a declaration whose var is empty or whose op is
+                // neither "set" nor "prepend". Matched exactly: a reader that
+                // is more permissive than the writer will one day apply
+                // something the writer considers malformed.
+                if (decl.var.empty()) continue;
+                if (decl.op != "set" && decl.op != "prepend") continue;
+                prov.decls.push_back(std::move(decl));
             }
             info.providers.push_back(std::move(prov));
         }

tests/e2e/200_subos_env_reaches_program.sh

@@ -14,6 +14,12 @@
 # know what any of these variables mean -- it carries whatever the subos
 # declares -- and a test naming LIBGL_DRIVERS_PATH would quietly suggest
 # otherwise.
+#
+# The JSON below is xlings's REAL shape: `envs` is an object keyed by binding,
+# whose values are arrays of declarations. The first version of this test wrote
+# an array of {binding, decls} -- a shape xlings never produces -- and it
+# passed, because the reader had been written from the same misunderstanding.
+# Do not "simplify" this structure; it is a wire format, not a convenience.
 set -euo pipefail
 
 TMP=$(mktemp -d)
@@ -28,9 +34,9 @@ mkdir -p "$subos/usr/lib/dri"
 cat > "$subos/.xlings.json" <<'EOF'
 { "workspace": {},
   "subos_info": { "schema_version": 1, "runtime": "glibc@2.39",
-    "envs": [ { "binding": "probe@1", "decls": [
+    "envs": { "probe@1": [
       { "var": "MCPP_E2E_PROBE", "op": "prepend",
-        "value": "${subosdir}/usr/lib/dri" } ] } ] } }
+        "value": "${subosdir}/usr/lib/dri" } ] } } }
 EOF
 
 cd "$TMP"
@@ -106,6 +112,33 @@ echo "$out2" | grep -q 'PROBE=(unset)' || {
     exit 1
 }
 
+# 2b. A cache written before this mcpp knew about subos environments must NOT
+#     be replayed by the fast path. Simulated by stripping the field, which is
+#     exactly what an older mcpp's cache looks like.
+#
+#     Without this the fix survives an upgrade in name only: the fast path's
+#     identity is the profile, the cache mode and the resource list, and its
+#     fingerprint check compares a cached entry against itself -- so nothing
+#     notices that a different mcpp wrote it, and an upgraded mcpp would keep
+#     running the pre-upgrade build with no subos environment at all.
+cache="$TMP/hello/target/.build_cache"
+[ -f "$cache" ] || { echo "no build cache to age"; exit 1; }
+grep -q '^subos=' "$cache" || { echo "cache has no subos= line to strip"; exit 1; }
+grep -v '^subos=' "$cache" > "$cache.old" && mv "$cache.old" "$cache"
+aged=$(MCPP_SUBOS_DIR="$subos" "$MCPP" run 2>&1) || {
+    echo "run against an aged cache failed:"; echo "$aged"; exit 1; }
+echo "$aged" | grep -q 'Resolving toolchain' || {
+    echo "an aged cache was replayed by the fast path — the subos environment"
+    echo "  would be missing for every run after an upgrade:"
+    echo "$aged"
+    exit 1
+}
+echo "$aged" | grep -q "PROBE=$subos/usr/lib/dri" || {
+    echo "the rebuild after an aged cache did not apply the environment:"
+    echo "$aged"; exit 1; }
+grep -q '^subos=' "$cache" || {
+    echo "the rebuild did not record subos= , so every later run repeats it"; exit 1; }
+
 # 3. A subos with no self-description degrades quietly and still runs. This is
 #    the state of every subos created before xlings grew the block, so it must
 #    not be an error.

tests/unit/test_subos_info.cpp

@@ -37,14 +37,14 @@ TEST(SubosInfo, ReadsRuntimeAndEnvDeclarations) {
       "subos_info": {
         "schema_version": 1,
         "runtime": "glibc@2.39",
-        "envs": [
-          { "binding": "mesa@25.0.7.1", "decls": [
+        "envs": {
+          "mesa@25.0.7.1": [
             { "var": "LIBGL_DRIVERS_PATH", "op": "prepend",
               "value": "${subosdir}/usr/lib/dri" },
             { "var": "XDG_DATA_DIRS", "op": "prepend",
               "value": "${subosdir}/share" }
-          ]}
-        ]
+          ]
+        }
       }
     })");
     auto info = su::read(t.dir);
@@ -64,8 +64,8 @@ TEST(SubosInfo, ReadsRuntimeAndEnvDeclarations) {
 TEST(SubosInfo, ResolvesSubosdirPlaceholder) {
     Tmp t;
     t.write(R"({"subos_info":{"schema_version":1,"runtime":"glibc@2.39",
-      "envs":[{"binding":"mesa@1","decls":[
-        {"var":"LIBGL_DRIVERS_PATH","op":"prepend","value":"${subosdir}/usr/lib/dri"}]}]}})");
+      "envs":{"mesa@1":[
+        {"var":"LIBGL_DRIVERS_PATH","op":"prepend","value":"${subosdir}/usr/lib/dri"}]}}})");
     auto env = su::resolve_env(su::read(t.dir), t.dir);
     ASSERT_EQ(env.size(), 1u);
     EXPECT_EQ(env[0].first, "LIBGL_DRIVERS_PATH");
@@ -81,11 +81,9 @@ TEST(SubosInfo, ResolvesSubosdirPlaceholder) {
 // EGL vendor directory. `prepend` joins them; it must not drop either.
 TEST(SubosInfo, PrependJoinsProvidersInOrder) {
     Tmp t;
-    t.write(R"({"subos_info":{"schema_version":1,"runtime":"glibc@2.39","envs":[
-      {"binding":"a-mesa@1","decls":[
-        {"var":"V","op":"prepend","value":"${subosdir}/one"}]},
-      {"binding":"b-vendor@1","decls":[
-        {"var":"V","op":"prepend","value":"${subosdir}/two"}]}]}})");
+    t.write(R"({"subos_info":{"schema_version":1,"runtime":"glibc@2.39","envs":{
+      "a-mesa@1":[{"var":"V","op":"prepend","value":"${subosdir}/one"}],
+      "b-vendor@1":[{"var":"V","op":"prepend","value":"${subosdir}/two"}]}}})");
     auto env = su::resolve_env(su::read(t.dir), t.dir);
     ASSERT_EQ(env.size(), 1u);
     const auto sep = mcpp::platform::env::path_list_separator();
@@ -98,9 +96,9 @@ TEST(SubosInfo, PrependJoinsProvidersInOrder) {
 // would otherwise grow the variable without bound.
 TEST(SubosInfo, PrependDeduplicates) {
     Tmp t;
-    t.write(R"({"subos_info":{"schema_version":1,"runtime":"glibc@2.39","envs":[
-      {"binding":"a@1","decls":[{"var":"V","op":"prepend","value":"${subosdir}/x"}]},
-      {"binding":"b@1","decls":[{"var":"V","op":"prepend","value":"${subosdir}/x"}]}]}})");
+    t.write(R"({"subos_info":{"schema_version":1,"runtime":"glibc@2.39","envs":{
+      "a@1":[{"var":"V","op":"prepend","value":"${subosdir}/x"}],
+      "b@1":[{"var":"V","op":"prepend","value":"${subosdir}/x"}]}}})");
     auto env = su::resolve_env(su::read(t.dir), t.dir);
     ASSERT_EQ(env.size(), 1u);
     // One entry, not two. The de-duplication has to split on the PLATFORM's
@@ -114,9 +112,9 @@ TEST(SubosInfo, PrependDeduplicates) {
 // `set` replaces rather than joins — xlings's own precedence.
 TEST(SubosInfo, SetReplaces) {
     Tmp t;
-    t.write(R"({"subos_info":{"schema_version":1,"runtime":"glibc@2.39","envs":[
-      {"binding":"a@1","decls":[{"var":"V","op":"prepend","value":"/one"}]},
-      {"binding":"b@1","decls":[{"var":"V","op":"set","value":"/two"}]}]}})");
+    t.write(R"({"subos_info":{"schema_version":1,"runtime":"glibc@2.39","envs":{
+      "a@1":[{"var":"V","op":"prepend","value":"/one"}],
+      "b@1":[{"var":"V","op":"set","value":"/two"}]}}})");
     auto env = su::resolve_env(su::read(t.dir), t.dir);
     ASSERT_EQ(env.size(), 1u);
     EXPECT_EQ(env[0].second, "/two");
@@ -177,4 +175,78 @@ TEST(SubosInfo, FamilyOfMirrorsXlings) {
     EXPECT_EQ(su::family_of("glibc"), "linux-x86_64-glibc");
 }
 
+
+// A VERBATIM capture of what a real xlings wrote, after `xlings install
+// graphics` on an NVIDIA host. Reformatted for width and nothing else -- keys,
+// nesting and spelling are as found on disk.
+//
+// This test exists because its absence shipped a broken feature. The first
+// version of this file hand-wrote every fixture in a shape the reader also
+// expected and xlings never produces: `envs` as an array of {binding, decls}.
+// Ten tests passed against a format that does not exist, and against a real
+// subos the released build applied no variables at all -- silently, because
+// "no providers" and "nothing declared" look identical.
+//
+// A fixture composed from the same understanding as the parser cannot catch
+// that. Only one taken from the writer can.
+TEST(SubosInfo, RealXlingsCapture) {
+    Tmp t;
+    t.write(R"({
+      "subos_info": {
+        "created_at": "2026-08-08T01:40:00Z",
+        "created_by": "xlings 2026.8.7.1",
+        "runtime": "glibc@2.39",
+        "schema_version": 1,
+        "envs": {
+          "mesa@25.0.7.1": [
+            {"op": "prepend", "value": "${subosdir}/usr/lib/dri", "var": "LIBGL_DRIVERS_PATH"},
+            {"op": "prepend", "value": "${subosdir}/share/glvnd/egl_vendor.d", "var": "__EGL_VENDOR_LIBRARY_DIRS"},
+            {"op": "prepend", "value": "${subosdir}/share", "var": "XDG_DATA_DIRS"}
+          ],
+          "nvidia-gl-host-link@0.1.1": [
+            {"op": "prepend", "value": "${subosdir}/share/glvnd/egl_vendor.d", "var": "__EGL_VENDOR_LIBRARY_DIRS"}
+          ]
+        }
+      },
+      "workspace": {}
+    })");
+
+    auto info = su::read(t.dir);
+    ASSERT_TRUE(info.present);
+    EXPECT_EQ(info.runtime, "glibc@2.39");
+    ASSERT_EQ(info.providers.size(), 2u);
+    EXPECT_EQ(info.providers[0].binding, "mesa@25.0.7.1");
+    EXPECT_EQ(info.providers[1].binding, "nvidia-gl-host-link@0.1.1");
+
+    auto env = su::resolve_env(info, t.dir);
+    ASSERT_EQ(env.size(), 3u) << "all three graphics variables must be produced";
+
+    std::map<std::string, std::string> byVar;
+    for (auto& [k, v] : env) byVar[k] = v;
+    const auto sep = mcpp::platform::env::path_list_separator();
+    EXPECT_EQ(byVar["LIBGL_DRIVERS_PATH"], t.dir.string() + "/usr/lib/dri");
+    EXPECT_EQ(byVar["XDG_DATA_DIRS"],      t.dir.string() + "/share");
+    // Both providers name the same vendor directory; de-duplication must
+    // leave exactly one, or libglvnd sees it twice and enumerates the device
+    // twice -- which is a defect xlings hit on its own side.
+    EXPECT_EQ(byVar["__EGL_VENDOR_LIBRARY_DIRS"],
+              t.dir.string() + "/share/glvnd/egl_vendor.d");
+    EXPECT_EQ(byVar["__EGL_VENDOR_LIBRARY_DIRS"].find(sep), std::string::npos);
+}
+
+// xlings drops a declaration whose op it does not recognise. A reader more
+// permissive than its writer eventually applies something the writer meant to
+// reject, so this asserts the same refusal rather than a tolerant guess.
+TEST(SubosInfo, UnknownOpIsDroppedLikeXlingsDrops) {
+    Tmp t;
+    t.write(R"({"subos_info":{"schema_version":1,"runtime":"glibc@2.39","envs":{
+      "a@1":[{"var":"V","op":"append","value":"/nope"},
+             {"var":"W","op":"prepend","value":"/yes"},
+             {"var":"","op":"prepend","value":"/no-name"}]}}})");
+    auto env = su::resolve_env(su::read(t.dir), t.dir);
+    ASSERT_EQ(env.size(), 1u);
+    EXPECT_EQ(env[0].first, "W");
+    EXPECT_EQ(env[0].second, "/yes");
+}
+
 }  // namespace