From 783957f226c6f607f84cd8857b90ba50a651645d Mon Sep 17 00:00:00 2001 From: apotema Date: Mon, 3 Aug 2026 17:56:02 -0300 Subject: [PATCH 1/5] feat(scenes): accept `@ref` target-override keys + engine version gate (labelle-engine#801) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scene/prefab override maps may now carry "@" keys (labelle-engine#801) that patch ref-named entities nested inside a referenced prefab's body. Assembler-side support: - pack rewrite: pass 1 moves flat @ pairs into the synthesized wrapper (leaving them outside would manufacture a hybrid entry and the engine would drop the target); pass 2 opens a component map for @ values in both flat and wrapped shapes, so pack-local component names under @ get the namespace rewrite (the labelle-toolkit#801 silent-miss class). - scene_name_lint: @ keys are structure, their CONTENTS are component refs (bare pack names under @ get the did-you-mean finding). - scene_manifest: @ counts as a flat entity-shape key (allowed at a flat reference root; trips HybridForm when mixed with a wrapper). - version gate: generate hard-errors when any scene/prefab uses @ keys but the pinned engine predates 2.11.0 — compat checking is MAJOR-only, so an old engine would silently DROP @ keys, which is the exact failure mode #801 exists to kill. Unparseable pins (local:, branch) pass permissively. --- src/codegen/scan/pack_refs.zig | 47 ++++++++++ src/codegen/scan/pack_refs/common.zig | 19 +++- src/codegen/scan/pack_refs/pass1.zig | 15 ++-- src/codegen/scan/pack_refs/pass2.zig | 18 +++- src/root.zig | 27 ++++++ src/scene_manifest.zig | 120 +++++++++++++++++++++++++- src/scene_manifest_test.zig | 61 +++++++++++++ src/scene_name_lint.zig | 52 ++++++++++- 8 files changed, 347 insertions(+), 12 deletions(-) diff --git a/src/codegen/scan/pack_refs.zig b/src/codegen/scan/pack_refs.zig index b2a10a11..110046df 100644 --- a/src/codegen/scan/pack_refs.zig +++ b/src/codegen/scan/pack_refs.zig @@ -897,3 +897,50 @@ test "bundleHeaderLegacyEntitiesOffset: dead entities on a header are flagged, n try std.testing.expect(std.mem.indexOf(u8, out, "\"components\": { \"sky__SkyBody\": { \"role\": \"sun\" }") != null); try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, out, "sky__SkyBody")); } + +test "rewritePackLocalRefs: component keys inside a wrapped @ target patch are rewritten (labelle-engine#801)" { + const allocator = std.testing.allocator; + const src = + \\{ "prefab": "base", "overrides": { "@slot": { "Worker": { "hp": 9 } } } } + ; + const out = try rewritePackLocalRefs(allocator, src, &.{"Worker"}, &.{}, "citizens"); + defer allocator.free(out); + + // The `@` key itself is structure — never namespaced … + try std.testing.expect(std.mem.indexOf(u8, out, "\"@slot\":") != null); + try std.testing.expect(std.mem.indexOf(u8, out, "citizens__@slot") == null); + // … but the pack component key INSIDE the target patch is. + try std.testing.expect(std.mem.indexOf(u8, out, "\"citizens__Worker\": { \"hp\": 9 }") != null); +} + +test "rewritePackLocalRefs: component keys inside a flat @ target patch are rewritten (labelle-engine#801)" { + const allocator = std.testing.allocator; + const src = + \\{ "prefab": "base", "@slot": { "Worker": { "hp": 9 } } } + ; + const out = try rewritePackLocalRefs(allocator, src, &.{"Worker"}, &.{}, "citizens"); + defer allocator.free(out); + + try std.testing.expect(std.mem.indexOf(u8, out, "citizens__Worker") != null); + try std.testing.expect(std.mem.indexOf(u8, out, "citizens__@slot") == null); +} + +test "wrapFlatEntityComponents: a flat @ pair moves into the synthesized wrapper (labelle-engine#801)" { + const allocator = std.testing.allocator; + // The flat pack-local `Worker` triggers the wrap; the `@slot` pair must + // ride into the SAME synthesized `overrides` wrapper — leaving it + // outside would manufacture a hybrid entry the author never wrote, and + // the engine would then warn wrapper-wins and DROP the target. + const src = + \\{ "prefab": "base", "Worker": { "hp": 1 }, "@slot": { "Position": { "x": 1 } } } + ; + const out = try rewritePackLocalRefs(allocator, src, &.{"Worker"}, &.{}, "citizens"); + defer allocator.free(out); + + const wrapper_at = std.mem.indexOf(u8, out, "\"overrides\": {").?; + const target_at = std.mem.indexOf(u8, out, "\"@slot\":").?; + // Exactly one wrapper, with the @ pair inside it. + try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, out, "\"overrides\": {")); + try std.testing.expect(target_at > wrapper_at); + try std.testing.expect(std.mem.indexOf(u8, out, "citizens__Worker") != null); +} diff --git a/src/codegen/scan/pack_refs/common.zig b/src/codegen/scan/pack_refs/common.zig index ed1d3245..a35f9f2f 100644 --- a/src/codegen/scan/pack_refs/common.zig +++ b/src/codegen/scan/pack_refs/common.zig @@ -136,6 +136,23 @@ pub fn isPascalCase(name: []const u8) bool { return name[0] >= 'A' and name[0] <= 'Z'; } +/// Byte-parity twin of the engine's `unified_format.zig` `isTargetKey` +/// (labelle-engine#801): `"@"` on a prefab reference's override map +/// targets a ref-named entity inside the referenced prefab's body. Like +/// PascalCase component keys, `@` keys are flat patch CONTENT at entity +/// scope — pass 1 must move them into the synthesized wrapper and pass 2 +/// must open a component map for their values, or the copy's shape drifts +/// from the author's meaning. +pub fn isTargetKey(name: []const u8) bool { + return name.len > 1 and name[0] == '@'; +} + +/// A flat patch-content key at entity scope: PascalCase component +/// (RFC #596 axis 2) or `@` target (labelle-engine#801). +pub fn isFlatComponentKey(name: []const u8) bool { + return isPascalCase(name) or isTargetKey(name); +} + /// The two array-valued entity-list keys the engine walks (`children` on any /// entity scope, legacy `entities` at file level). Shared between pass 2's /// `childArrayScope` and pass 1's flat-wrap recursion so the two walks can @@ -181,7 +198,7 @@ pub fn isOnlyMetaHeaderObject(src: []const u8, open: usize) bool { std.mem.eql(u8, key, "components") or std.mem.eql(u8, key, "overrides") or std.mem.eql(u8, key, "ref") or - isPascalCase(key)) + isFlatComponentKey(key)) { // Any entity-shape key disqualifies — this is an entity that // happens to carry a `meta` field, not a file header. diff --git a/src/codegen/scan/pack_refs/pass1.zig b/src/codegen/scan/pack_refs/pass1.zig index f8da7262..2ac2b88d 100644 --- a/src/codegen/scan/pack_refs/pass1.zig +++ b/src/codegen/scan/pack_refs/pass1.zig @@ -225,17 +225,22 @@ const FlatWrap = struct { var emitted_any = false; var wrapper_done = false; for (obj.pairs) |p| { - if (wrap and common.isPascalCase(p.key)) { - // Every Pascal pair moves into the single wrapper, emitted - // at the FIRST Pascal pair's position (with its leading - // trivia); later Pascal pairs were already emitted inside. + if (wrap and common.isFlatComponentKey(p.key)) { + // Every flat patch-content pair (PascalCase component or + // `@` target, labelle-engine#801) moves into the single + // wrapper, emitted at the FIRST such pair's position (with + // its leading trivia); later ones were already emitted + // inside. Leaving an `@` pair OUTSIDE the synthesized + // wrapper would manufacture a hybrid entry the author never + // wrote — and the engine would then warn wrapper-wins and + // drop the target. if (wrapper_done) continue; if (emitted_any) try out.append(allocator, ','); try out.appendSlice(allocator, s[p.lead_start..p.key_start]); try out.appendSlice(allocator, if (is_reference) "\"overrides\": {" else "\"components\": {"); var first = true; for (obj.pairs) |q| { - if (!common.isPascalCase(q.key)) continue; + if (!common.isFlatComponentKey(q.key)) continue; if (!first) try out.append(allocator, ','); if (first) { // Its original lead was spent on the wrapper key. diff --git a/src/codegen/scan/pack_refs/pass2.zig b/src/codegen/scan/pack_refs/pass2.zig index 782318ac..11473f3a 100644 --- a/src/codegen/scan/pack_refs/pass2.zig +++ b/src/codegen/scan/pack_refs/pass2.zig @@ -253,6 +253,11 @@ fn childScope(parent: ?Scope, pending_key: ?[]const u8) Scope { if (std.mem.eql(u8, k, "components") or std.mem.eql(u8, k, "overrides")) { break :blk .component_map; } + // A flat `@` target's value is a component map + // (labelle-engine#801) — its keys must be rewritten like + // any other component reference, or a pack-local name + // under `@` silently misses the namespace rewrite. + if (common.isTargetKey(k)) break :blk .component_map; } // Any other object field on an entity (`meta`, etc.) is opaque. break :blk .payload; @@ -264,8 +269,17 @@ fn childScope(parent: ?Scope, pending_key: ?[]const u8) Scope { // duplicate later `"root"` member lands here and stays dead-data // verbatim (CodeRabbit on #521). .file_container => .payload, - // A component's value, or anything already inside payload, stays opaque. - .component_map, .payload, .array_other => .payload, + // Inside a component map, a `@` target's value is ANOTHER + // component map (labelle-engine#801); a component's value is + // opaque payload. + .component_map => blk: { + if (pending_key) |k| { + if (common.isTargetKey(k)) break :blk .component_map; + } + break :blk .payload; + }, + // Anything already inside payload stays opaque. + .payload, .array_other => .payload, }; } diff --git a/src/root.zig b/src/root.zig index 36533b27..4bd6ad68 100644 --- a/src/root.zig +++ b/src/root.zig @@ -564,6 +564,33 @@ pub fn generate( const scene_manifests = try scene_manifest.parseSceneDir(allocator, scenes_target, jsonc_scene_names); defer scene_manifest.freeManifests(allocator, scene_manifests); + // ── `@` target-override version gate (labelle-engine#801) ────────── + // Compat checking is MAJOR-only (labelle-cli#269), so "new assembler + + // old engine pin" is a legal combo — and an engine older than the + // feature silently DROPS `@` keys, which is the exact silent no-op + // #801 exists to kill, reintroduced through the side door. Hard-error + // at generate instead. Covers scenes AND prefabs (a prefab body can + // use `@` on its own ref-array entries). Unparseable pins (`local:` + // dev overrides, branch pins) pass — see + // `engineSupportsTargetOverrides`. + if (!scene_manifest.engineSupportsTargetOverrides(cfg.engine_version)) { + const prefabs_target = try std.fs.path.join(allocator, &.{ target_dir, "prefabs" }); + defer allocator.free(prefabs_target); + const hit: ?[]const u8 = + try scene_manifest.findTargetKeyUsage(allocator, scenes_target, jsonc_scene_names) orelse + try scene_manifest.findTargetKeyUsage(allocator, prefabs_target, prefab_names); + if (hit) |offender| { + defer allocator.free(offender); + std.debug.print( + "labelle-assembler: '{s}' uses \"@\" target-override keys (labelle-engine#801), but the pinned engine v{s} predates them (needs >= v{s}).\n" ++ + " An older engine silently DROPS `@` keys — the exact failure this syntax replaces.\n" ++ + " Bump `engine_version` in project.labelle, or remove the `@` overrides.\n", + .{ offender, cfg.engine_version, scene_manifest.MIN_ENGINE_FOR_TARGET_OVERRIDES }, + ); + return error.EngineTooOldForTargetOverrides; + } + } + // ── Asset-Plugins Phase 1: scene auto-wiring (#575) ──────────────── // A scene that instantiates any prefab from a pack gets that pack's // non-lazy resources auto-added to its asset manifest, so a scene using diff --git a/src/scene_manifest.zig b/src/scene_manifest.zig index 6d3bef25..2dc73eda 100644 --- a/src/scene_manifest.zig +++ b/src/scene_manifest.zig @@ -127,6 +127,13 @@ fn isPascalCase(key: []const u8) bool { return c >= 'A' and c <= 'Z'; } +/// `"@"` target-override key (labelle-engine#801). Rides the flat +/// shape exactly like a PascalCase component key: it is patch CONTENT, so +/// it participates in flat-form detection and the hybrid-form gate. +fn isTargetKey(key: []const u8) bool { + return key.len > 1 and key[0] == '@'; +} + fn isAllowedTopLevelKey(key: []const u8) bool { // RFC #596 axis 2: PascalCase keys at the file's top level are // component declarations on the flat-form root entity. The audit @@ -135,6 +142,10 @@ fn isAllowedTopLevelKey(key: []const u8) bool { // here, so we accept any PascalCase key and rely on the loader's // warn-once path. This matches the audit's option C resolution. if (isPascalCase(key)) return true; + // `@` target-override keys on a flat-form reference root + // (labelle-engine#801) — content, not structure; the engine + // validates the ref names at load. + if (isTargetKey(key)) return true; for (ALLOWED_TOP_LEVEL_KEYS) |allowed| { if (std.mem.eql(u8, key, allowed)) return true; } @@ -290,6 +301,7 @@ fn hasFlatEntityShapeKey(obj: std.json.ObjectMap) bool { while (iter.next()) |entry| { const key = entry.key_ptr.*; if (isPascalCase(key)) return true; + if (isTargetKey(key)) return true; if (std.mem.eql(u8, key, "components")) return true; if (std.mem.eql(u8, key, "children")) return true; if (std.mem.eql(u8, key, "prefab")) return true; @@ -322,7 +334,7 @@ pub fn checkHybridForm(obj: std.json.ObjectMap) ?[]const u8 { var iter = obj.iterator(); while (iter.next()) |entry| { const key = entry.key_ptr.*; - if (isPascalCase(key)) { + if (isPascalCase(key) or isTargetKey(key)) { has_pascal = true; } else if (std.mem.eql(u8, key, "overrides")) { has_overrides = true; @@ -925,6 +937,112 @@ pub fn freeManifests(allocator: std.mem.Allocator, manifests: []const SceneManif allocator.free(manifests); } +// ── `@` target-override version gate (labelle-engine#801) ─────────────── + +/// True iff `src` uses a `"@":` object KEY anywhere — the flat or +/// wrapped `@` target-override syntax (labelle-engine#801). Textual scan +/// with colon lookahead so `"@name"` VALUES (the `@ref` value syntax) never +/// count. Conservative on malformed input: an unterminated string ends the +/// scan with "not used". +pub fn sourceUsesTargetKeys(src: []const u8) bool { + var i: usize = 0; + while (i < src.len) { + const c = src[i]; + if (c == '/' and i + 1 < src.len and src[i + 1] == '/') { + i = std.mem.indexOfScalarPos(u8, src, i, '\n') orelse src.len; + continue; + } + if (c == '/' and i + 1 < src.len and src[i + 1] == '*') { + const close = std.mem.indexOfPos(u8, src, i + 2, "*/"); + i = if (close) |q| q + 2 else src.len; + continue; + } + if (c == '"') { + const content_start = i + 1; + var j = content_start; + while (j < src.len) : (j += 1) { + if (src[j] == '\\' and j + 1 < src.len) { + j += 1; + continue; + } + if (src[j] == '"') break; + } + if (j >= src.len) return false; // unterminated — stop + const content = src[content_start..j]; + if (isTargetKey(content) and nextSignificantIsColonAt(src, j + 1)) return true; + i = j + 1; + continue; + } + i += 1; + } + return false; +} + +/// Colon lookahead for `sourceUsesTargetKeys` — true iff the next +/// significant byte (skipping whitespace/JSONC comments) is `:`, i.e. the +/// preceding string literal was an object key. +fn nextSignificantIsColonAt(src: []const u8, from: usize) bool { + var i = from; + while (i < src.len) { + const c = src[i]; + if (c == ' ' or c == '\t' or c == '\r' or c == '\n') { + i += 1; + continue; + } + if (c == '/' and i + 1 < src.len and src[i + 1] == '/') { + i = std.mem.indexOfScalarPos(u8, src, i, '\n') orelse src.len; + continue; + } + if (c == '/' and i + 1 < src.len and src[i + 1] == '*') { + const close = std.mem.indexOfPos(u8, src, i + 2, "*/"); + i = if (close) |q| q + 2 else src.len; + continue; + } + return c == ':'; + } + return false; +} + +/// First engine release that understands `@` target-override keys +/// (labelle-engine#801). Bump ONLY if the engine-side feature slips to a +/// later minor. +pub const MIN_ENGINE_FOR_TARGET_OVERRIDES = "2.11.0"; + +/// True iff the pinned engine version understands `@` target overrides. +/// PERMISSIVE on anything unparseable (`local:` overrides, branch pins): +/// the gate exists to catch the "new assembler, old engine pin" skew where +/// an old engine silently drops `@` keys — a pin we cannot parse is a +/// deliberate dev setup, not that trap. Compat checking is MAJOR-only +/// (labelle-cli#269), so this per-feature minimum is the only guard. +pub fn engineSupportsTargetOverrides(engine_version: []const u8) bool { + const min = std.SemanticVersion.parse(MIN_ENGINE_FOR_TARGET_OVERRIDES) catch unreachable; + const pin = std.SemanticVersion.parse(engine_version) catch return true; + return pin.order(min) != .lt; +} + +/// Scan every `.jsonc` under `dir` for `@` target-override keys; +/// returns the first file (allocator-owned name) that uses them, or null. +/// Missing/unreadable files are skipped — this is a version gate, not a +/// file validator (the real parse reports real errors). +pub fn findTargetKeyUsage( + allocator: std.mem.Allocator, + dir: []const u8, + names: []const []const u8, +) !?[]const u8 { + for (names) |name| { + const rel = try std.fmt.allocPrint(allocator, "{s}/{s}.jsonc", .{ dir, name }); + const source = std.Io.Dir.cwd().readFileAlloc(config.globalIo(), rel, allocator, .limited(1024 * 1024)) catch { + allocator.free(rel); + continue; + }; + defer allocator.free(source); + if (sourceUsesTargetKeys(source)) return rel; + allocator.free(rel); + } + return null; +} + + /// Read every `/.jsonc` (where `name` is one of `scene_names`, /// possibly with subfolder slashes), parse it, and return the manifest list in /// the same order as `scene_names`. diff --git a/src/scene_manifest_test.zig b/src/scene_manifest_test.zig index 36b98de1..9ecd2f0c 100644 --- a/src/scene_manifest_test.zig +++ b/src/scene_manifest_test.zig @@ -1108,3 +1108,64 @@ test "rfc596: file-header `{meta}` with extra keys is treated as an entity (posi const result = parseSceneSource(std.testing.allocator, "bad_header", "scenes/bad_header.jsonc", src); try std.testing.expectError(error.InvalidEntityShape, result); } + +// ── `@` target-override keys (labelle-engine#801) ──────────────────────── + +test "801: @ keys at a flat reference root parse cleanly" { + const allocator = std.testing.allocator; + const src = + \\{ "prefab": "machine", "@slot": { "Storage": { "capacity": 12 } } } + ; + const m = try parseSceneSource(allocator, "s", "s.jsonc", src); + defer freeManifest(allocator, m); +} + +test "801: @ key mixed with an overrides wrapper is HybridForm" { + const allocator = std.testing.allocator; + const src = + \\{ "prefab": "machine", "@slot": { "Storage": {} }, "overrides": { "Position": { "x": 1 } } } + ; + const result = parseSceneSource(allocator, "s", "s.jsonc", src); + try std.testing.expectError(error.HybridForm, result); +} + +test "801: checkHybridForm counts a @ key on the flat side" { + const allocator = std.testing.allocator; + var parsed = try std.json.parseFromSlice(std.json.Value, allocator, + \\{ "prefab": "x", "@slot": {}, "overrides": {} } + , .{}); + defer parsed.deinit(); + const conflict = checkHybridForm(parsed.value.object); + try std.testing.expect(conflict != null); + try std.testing.expectEqualStrings("overrides", conflict.?); +} + +test "801: sourceUsesTargetKeys detects @ KEYS but not @ values" { + // Wrapped-form key. + try std.testing.expect(scene_manifest.sourceUsesTargetKeys( + \\{ "prefab": "m", "overrides": { "@slot": { "Storage": {} } } } + )); + // Flat-form key. + try std.testing.expect(scene_manifest.sourceUsesTargetKeys( + \\{ "prefab": "m", "@slot": { "Storage": {} } } + )); + // `@ref` VALUE syntax must NOT count — it predates #801 by years. + try std.testing.expect(!scene_manifest.sourceUsesTargetKeys( + \\{ "components": { "Worker": { "target": "@storage" } } } + )); + // A commented-out @ key must not count either. + try std.testing.expect(!scene_manifest.sourceUsesTargetKeys( + \\{ "prefab": "m" } // "@slot": { } + )); +} + +test "801: engineSupportsTargetOverrides gates on 2.11.0, permissive on unparseable pins" { + const supports = scene_manifest.engineSupportsTargetOverrides; + try std.testing.expect(!supports("2.10.0")); + try std.testing.expect(supports("2.11.0")); + try std.testing.expect(supports("2.12.3")); + try std.testing.expect(supports("3.0.0")); + // `local:` dev overrides and branch pins cannot be compared — pass. + try std.testing.expect(supports("local:../labelle-engine")); + try std.testing.expect(supports("main")); +} diff --git a/src/scene_name_lint.zig b/src/scene_name_lint.zig index fb08f3e5..43e93c72 100644 --- a/src/scene_name_lint.zig +++ b/src/scene_name_lint.zig @@ -113,10 +113,22 @@ fn childScope(parent: ?Scope, pending_key: ?[]const u8) Scope { break :blk .component_map; } if (std.mem.eql(u8, k, "root")) break :blk .entity; + // A flat `@` target's value is a component map + // (labelle-engine#801). + if (isTargetKey(k)) break :blk .component_map; } break :blk .payload; }, - .component_map, .payload, .array_other => .payload, + // Inside a component map, a `@` target's value is ANOTHER + // component map (labelle-engine#801); a component's value is + // opaque payload. + .component_map => blk: { + if (pending_key) |k| { + if (isTargetKey(k)) break :blk .component_map; + } + break :blk .payload; + }, + .payload, .array_other => .payload, }; } @@ -142,6 +154,12 @@ fn isPascalCase(key: []const u8) bool { return key.len > 0 and key[0] >= 'A' and key[0] <= 'Z'; } +/// `"@"` target-override key (labelle-engine#801) — patch structure, +/// not a component reference; its VALUE is a component map. +fn isTargetKey(key: []const u8) bool { + return key.len > 1 and key[0] == '@'; +} + /// True iff the next significant byte at/after `from` (skipping whitespace /// and JSONC comments) is a `:` — i.e. the preceding string literal was an /// object key rather than a value. Pure lookahead. Mirrors @@ -249,8 +267,10 @@ pub fn collectComponentRefs(arena: std.mem.Allocator, src: []const u8) ![]CompRe if (is_key) { const scope = topScope(scope_stack.items) orelse .entity; const is_ref = switch (scope) { - // Every key in a component map is a component name. - .component_map => true, + // Every key in a component map is a component name — + // except a `@` target (labelle-engine#801), whose + // value opens another component map instead. + .component_map => !isTargetKey(content), // A flat-form PascalCase key on an entity is a component. .entity => isPascalCase(content), else => false, @@ -711,3 +731,29 @@ test "findBareLocalRefs: covers bundle and root-wrapper containers (#516)" { , &.{"SkyBody"}); try testing.expectEqual(@as(usize, 1), wrapper_refs.len); } + +test "collectComponentRefs: a @ target key is structure, its contents are refs (labelle-engine#801)" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const src = + \\{ "prefab": "x", "overrides": { "@slot": { "Worker": {} } } } + ; + const refs = try collectComponentRefs(arena, src); + try std.testing.expectEqual(@as(usize, 1), refs.len); + try std.testing.expectEqualStrings("Worker", refs[0].name); +} + +test "collectComponentRefs: flat @ target contents are refs (labelle-engine#801)" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const src = + \\{ "prefab": "x", "@slot": { "Worker": {} } } + ; + const refs = try collectComponentRefs(arena, src); + try std.testing.expectEqual(@as(usize, 1), refs.len); + try std.testing.expectEqualStrings("Worker", refs[0].name); +} From 2f1d4900fdf9cc56e88b7fa6e47da7fd10a4d9b2 Mon Sep 17 00:00:00 2001 From: apotema Date: Mon, 3 Aug 2026 18:23:11 -0300 Subject: [PATCH 2/5] fix(review): scope-aware gate detection, pack-source coverage, leak/limit hardening (round 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - sourceUsesTargetKeys moved to scene_name_lint's scope-walk model: payload @ keys (e.g. Config.@id data) no longer false-positive the gate; JSON-escaped \u0040 spellings now DO trip it - version gate scans pack SOURCE trees (prefabs/ + scenes/) via new findTargetKeyUsageInTree — pack files aren't staged when the gate runs, so a light pack's @ override slipped past before - findTargetKeyUsage: per-file buffer free (peak memory = one file), StreamTooLong is a hard error instead of a silent gate bypass, ceiling raised to 16 MiB --- src/root.zig | 19 ++++- src/scene_manifest.zig | 144 +++++++++++++++++++----------------- src/scene_manifest_test.zig | 25 +++++++ src/scene_name_lint.zig | 96 ++++++++++++++++++++++++ 4 files changed, 216 insertions(+), 68 deletions(-) diff --git a/src/root.zig b/src/root.zig index 4bd6ad68..c1776b63 100644 --- a/src/root.zig +++ b/src/root.zig @@ -576,9 +576,26 @@ pub fn generate( if (!scene_manifest.engineSupportsTargetOverrides(cfg.engine_version)) { const prefabs_target = try std.fs.path.join(allocator, &.{ target_dir, "prefabs" }); defer allocator.free(prefabs_target); - const hit: ?[]const u8 = + var hit: ?[]const u8 = try scene_manifest.findTargetKeyUsage(allocator, scenes_target, jsonc_scene_names) orelse try scene_manifest.findTargetKeyUsage(allocator, prefabs_target, prefab_names); + // Pack sources too: pack prefabs/scenes are not staged until + // `loadPackScans` later in generate, so scan each pack's SOURCE + // tree here — a light pack shipping an `@` override would + // otherwise slip past the gate (codex P1 / CodeRabbit on #650). + if (hit == null) { + for (pack_entries.items) |entry| { + const src_dir = try entry.resolveSrcDir(allocator, game_dir); + defer allocator.free(src_dir); + for ([_][]const u8{ "prefabs", "scenes" }) |sub| { + const sub_dir = try std.fs.path.join(allocator, &.{ src_dir, sub }); + defer allocator.free(sub_dir); + hit = try scene_manifest.findTargetKeyUsageInTree(allocator, sub_dir); + if (hit != null) break; + } + if (hit != null) break; + } + } if (hit) |offender| { defer allocator.free(offender); std.debug.print( diff --git a/src/scene_manifest.zig b/src/scene_manifest.zig index 2dc73eda..79393ceb 100644 --- a/src/scene_manifest.zig +++ b/src/scene_manifest.zig @@ -939,68 +939,13 @@ pub fn freeManifests(allocator: std.mem.Allocator, manifests: []const SceneManif // ── `@` target-override version gate (labelle-engine#801) ─────────────── -/// True iff `src` uses a `"@":` object KEY anywhere — the flat or -/// wrapped `@` target-override syntax (labelle-engine#801). Textual scan -/// with colon lookahead so `"@name"` VALUES (the `@ref` value syntax) never -/// count. Conservative on malformed input: an unterminated string ends the -/// scan with "not used". +/// True iff `src` uses `@` target-override syntax (labelle-engine#801). +/// Delegates to `scene_name_lint.sourceUsesTargetKeys` — the scope-aware +/// walker — so opaque component-payload keys (`{ "Config": { "@id": … } }`) +/// and `@ref` VALUES never count, while flat/wrapped `@` keys (including +/// the JSON-escaped `"\u0040…"` spelling) do. pub fn sourceUsesTargetKeys(src: []const u8) bool { - var i: usize = 0; - while (i < src.len) { - const c = src[i]; - if (c == '/' and i + 1 < src.len and src[i + 1] == '/') { - i = std.mem.indexOfScalarPos(u8, src, i, '\n') orelse src.len; - continue; - } - if (c == '/' and i + 1 < src.len and src[i + 1] == '*') { - const close = std.mem.indexOfPos(u8, src, i + 2, "*/"); - i = if (close) |q| q + 2 else src.len; - continue; - } - if (c == '"') { - const content_start = i + 1; - var j = content_start; - while (j < src.len) : (j += 1) { - if (src[j] == '\\' and j + 1 < src.len) { - j += 1; - continue; - } - if (src[j] == '"') break; - } - if (j >= src.len) return false; // unterminated — stop - const content = src[content_start..j]; - if (isTargetKey(content) and nextSignificantIsColonAt(src, j + 1)) return true; - i = j + 1; - continue; - } - i += 1; - } - return false; -} - -/// Colon lookahead for `sourceUsesTargetKeys` — true iff the next -/// significant byte (skipping whitespace/JSONC comments) is `:`, i.e. the -/// preceding string literal was an object key. -fn nextSignificantIsColonAt(src: []const u8, from: usize) bool { - var i = from; - while (i < src.len) { - const c = src[i]; - if (c == ' ' or c == '\t' or c == '\r' or c == '\n') { - i += 1; - continue; - } - if (c == '/' and i + 1 < src.len and src[i + 1] == '/') { - i = std.mem.indexOfScalarPos(u8, src, i, '\n') orelse src.len; - continue; - } - if (c == '/' and i + 1 < src.len and src[i + 1] == '*') { - const close = std.mem.indexOfPos(u8, src, i + 2, "*/"); - i = if (close) |q| q + 2 else src.len; - continue; - } - return c == ':'; - } - return false; + return @import("scene_name_lint.zig").sourceUsesTargetKeys(src); } /// First engine release that understands `@` target-override keys @@ -1023,7 +968,12 @@ pub fn engineSupportsTargetOverrides(engine_version: []const u8) bool { /// Scan every `.jsonc` under `dir` for `@` target-override keys; /// returns the first file (allocator-owned name) that uses them, or null. /// Missing/unreadable files are skipped — this is a version gate, not a -/// file validator (the real parse reports real errors). +/// file validator (the real parse reports real errors) — EXCEPT an +/// oversized file (`error.StreamTooLong`): a file too big to scan may +/// contain the keys the gate exists to catch, so skipping it would bypass +/// the gate (CodeRabbit on #650). The 16 MiB ceiling is far above any +/// real scene/prefab; each buffer is freed before the next file is read +/// so peak memory stays one file, not the sum (codex P2 on #650). pub fn findTargetKeyUsage( allocator: std.mem.Allocator, dir: []const u8, @@ -1031,17 +981,77 @@ pub fn findTargetKeyUsage( ) !?[]const u8 { for (names) |name| { const rel = try std.fmt.allocPrint(allocator, "{s}/{s}.jsonc", .{ dir, name }); - const source = std.Io.Dir.cwd().readFileAlloc(config.globalIo(), rel, allocator, .limited(1024 * 1024)) catch { - allocator.free(rel); - continue; + errdefer allocator.free(rel); + const source = std.Io.Dir.cwd().readFileAlloc(config.globalIo(), rel, allocator, .limited(16 * 1024 * 1024)) catch |err| switch (err) { + error.StreamTooLong => { + stderrPrint( + "labelle-assembler: '{s}' exceeds the 16 MiB scan ceiling for the target-override version gate — cannot verify it is free of `@` keys.\n", + .{rel}, + ); + return err; + }, + else => { + allocator.free(rel); + continue; + }, }; - defer allocator.free(source); - if (sourceUsesTargetKeys(source)) return rel; + const used = sourceUsesTargetKeys(source); + allocator.free(source); + if (used) return rel; allocator.free(rel); } return null; } +/// `findTargetKeyUsage` over every `.jsonc` in a directory TREE — used for +/// pack source dirs, whose file lists are not staged yet when the gate +/// runs (codex P1 / CodeRabbit on #650). A missing directory is fine +/// (packs need not ship prefabs or scenes). Returns the first offending +/// path (allocator-owned), or null. +pub fn findTargetKeyUsageInTree( + allocator: std.mem.Allocator, + dir_path: []const u8, +) !?[]const u8 { + const io = config.globalIo(); + var dir = std.Io.Dir.cwd().openDir(io, dir_path, .{ .iterate = true }) catch return null; + defer dir.close(io); + + var iter = dir.iterate(); + while (iter.next(io) catch return null) |entry| { + switch (entry.kind) { + .directory => { + const sub = try std.fs.path.join(allocator, &.{ dir_path, entry.name }); + defer allocator.free(sub); + if (try findTargetKeyUsageInTree(allocator, sub)) |hit| return hit; + }, + .file => { + if (!std.mem.endsWith(u8, entry.name, ".jsonc")) continue; + const rel = try std.fs.path.join(allocator, &.{ dir_path, entry.name }); + errdefer allocator.free(rel); + const source = std.Io.Dir.cwd().readFileAlloc(io, rel, allocator, .limited(16 * 1024 * 1024)) catch |err| switch (err) { + error.StreamTooLong => { + stderrPrint( + "labelle-assembler: '{s}' exceeds the 16 MiB scan ceiling for the target-override version gate — cannot verify it is free of `@` keys.\n", + .{rel}, + ); + return err; + }, + else => { + allocator.free(rel); + continue; + }, + }; + const used = sourceUsesTargetKeys(source); + allocator.free(source); + if (used) return rel; + allocator.free(rel); + }, + else => {}, + } + } + return null; +} + /// Read every `/.jsonc` (where `name` is one of `scene_names`, /// possibly with subfolder slashes), parse it, and return the manifest list in diff --git a/src/scene_manifest_test.zig b/src/scene_manifest_test.zig index 9ecd2f0c..faff1bb5 100644 --- a/src/scene_manifest_test.zig +++ b/src/scene_manifest_test.zig @@ -1169,3 +1169,28 @@ test "801: engineSupportsTargetOverrides gates on 2.11.0, permissive on unparsea try std.testing.expect(supports("local:../labelle-engine")); try std.testing.expect(supports("main")); } + +test "801: payload @ keys do not trip the gate (scope-aware detection)" { + // `@id` is ordinary component DATA — hard-failing generate on it + // would be a false positive (codex P2 on #650). + try std.testing.expect(!scene_manifest.sourceUsesTargetKeys( + \\{ "components": { "Config": { "@id": "x" } } } + )); + // Same shape one level deeper (payload stays payload). + try std.testing.expect(!scene_manifest.sourceUsesTargetKeys( + \\{ "prefab": "m", "overrides": { "Config": { "opts": { "@mode": 1 } } } } + )); +} + +test "801: the JSON-escaped @ spelling still trips the gate" { + // `"@slot"` decodes to `@slot` at engine load — the gate must + // see through the escape or an old pin silently drops the override + // (CodeRabbit on #650). + try std.testing.expect(scene_manifest.sourceUsesTargetKeys( + \\{ "prefab": "m", "\u0040slot": { "Storage": {} } } + )); + // Escaped spelling inside an overrides wrapper too. + try std.testing.expect(scene_manifest.sourceUsesTargetKeys( + \\{ "prefab": "m", "overrides": { "\u0040slot": { "Storage": {} } } } + )); +} diff --git a/src/scene_name_lint.zig b/src/scene_name_lint.zig index 43e93c72..d6cdf002 100644 --- a/src/scene_name_lint.zig +++ b/src/scene_name_lint.zig @@ -68,6 +68,102 @@ pub const builtin_component_names = [_][]const u8{ "VideoComponent", }; + +// ── `@` target-override usage detection (labelle-engine#801) ────────────── + +/// True iff `src` uses `@` target-override syntax (labelle-engine#801): a +/// `"@"` (or JSON-escaped `"\u0040"`) object KEY at an entity or +/// component-map scope. Scope-tracked with the same `childScope` model as +/// `collectComponentRefs`, so `@`-keys inside opaque component PAYLOAD +/// (`{ "Config": { "@id": "x" } }`) never count — payload keys are ordinary +/// data, and a false positive here would spuriously hard-fail the +/// assembler's engine-version gate (codex P2 on #650). `@ref` VALUES +/// (`"target": "@storage"`) never count either (colon lookahead). +/// +/// Allocation-free: the scope stack is a fixed 256-frame buffer; deeper +/// nesting (not a real scene file) bails conservatively with `false` — the +/// real parse rejects such a file long before the gate matters. +pub fn sourceUsesTargetKeys(src: []const u8) bool { + var stack_buf: [256]Scope = undefined; + var sp: usize = 0; + var pending_key: ?[]const u8 = null; + + var i: usize = 0; + while (i < src.len) { + const c = src[i]; + if (c == '/' and i + 1 < src.len and src[i + 1] == '/') { + i = std.mem.indexOfScalarPos(u8, src, i, '\n') orelse src.len; + continue; + } + if (c == '/' and i + 1 < src.len and src[i + 1] == '*') { + const close = std.mem.indexOfPos(u8, src, i + 2, "*/"); + i = if (close) |q| q + 2 else src.len; + continue; + } + if (c == '{') { + if (sp >= stack_buf.len) return false; // pathological nesting + stack_buf[sp] = childScope(if (sp > 0) stack_buf[sp - 1] else null, pending_key); + sp += 1; + pending_key = null; + i += 1; + continue; + } + if (c == '[') { + if (sp >= stack_buf.len) return false; + stack_buf[sp] = childArrayScope(if (sp > 0) stack_buf[sp - 1] else null, pending_key); + sp += 1; + pending_key = null; + i += 1; + continue; + } + if (c == '}' or c == ']') { + if (sp > 0) sp -= 1; + pending_key = null; + i += 1; + continue; + } + if (c == ',') { + pending_key = null; + i += 1; + continue; + } + if (c == '"') { + const content_start = i + 1; + var j = content_start; + while (j < src.len) : (j += 1) { + if (src[j] == '\\' and j + 1 < src.len) { + j += 1; + continue; + } + if (src[j] == '"') break; + } + if (j >= src.len) return false; // unterminated — stop + const content = src[content_start..j]; + const is_key = nextSignificantIsColon(src, j + 1); + if (is_key) { + const scope = if (sp > 0) stack_buf[sp - 1] else .entity; + if (scope == .entity or scope == .component_map) { + if (isTargetKey(content) or isEscapedTargetKey(content)) return true; + } + pending_key = content; + } else { + pending_key = null; + } + i = j + 1; + continue; + } + i += 1; + } + return false; +} + +/// A `"\u0040"` key — the JSON-escaped spelling of `@` that decodes +/// to a target key at engine load (CodeRabbit on #650). Raw-byte check: +/// backslash, `u0040`, then at least one ref character. +fn isEscapedTargetKey(content: []const u8) bool { + return content.len > 6 and std.mem.startsWith(u8, content, "\\u0040"); +} + // ── Component-reference collection ───────────────────────────────────────── /// One component-declaration reference found in a scene/prefab source: the From 7b062fa812805b7d54b470b6ed0e8859bd10e622 Mon Sep 17 00:00:00 2001 From: apotema Date: Mon, 3 Aug 2026 18:42:18 -0300 Subject: [PATCH 3/5] =?UTF-8?q?fix(review):=20round=202=20=E2=80=94=20esca?= =?UTF-8?q?ped-@=20parity=20in=20rewrite=20walkers,=20gate=20scans=20pack?= =?UTF-8?q?=20prefabs=20only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - common/lint isTargetKey now recognize the raw \u0040 spelling, so the rewrite walkers agree with the version-gate scanner: pass 1 moves an escaped target into the wrapper and pass 2 namespaces its contents (the PascalCase analog stays tracked in #651) - the pack version gate scans pack prefabs/ only — scanPack never stages a pack's scenes/, so an example scene must not impose an engine-version requirement --- src/codegen/scan/pack_refs.zig | 26 ++++++++++++++++++++++++++ src/codegen/scan/pack_refs/common.zig | 10 +++++++++- src/root.zig | 19 ++++++++++--------- src/scene_name_lint.zig | 7 +++++-- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/src/codegen/scan/pack_refs.zig b/src/codegen/scan/pack_refs.zig index 110046df..7f340909 100644 --- a/src/codegen/scan/pack_refs.zig +++ b/src/codegen/scan/pack_refs.zig @@ -944,3 +944,29 @@ test "wrapFlatEntityComponents: a flat @ pair moves into the synthesized wrapper try std.testing.expect(target_at > wrapper_at); try std.testing.expect(std.mem.indexOf(u8, out, "citizens__Worker") != null); } + +test "rewritePackLocalRefs: the JSON-escaped @ spelling opens a component map too (labelle-engine#801)" { + const allocator = std.testing.allocator; + // `"\u0040slot"` decodes to `@slot` at engine load — the raw-byte + // walkers must treat it as a target key or the pack rewrite is + // silently lost on a supported engine (codex round 2 on #650). + const src = + \\{ "prefab": "base", "overrides": { "\u0040slot": { "Worker": { "hp": 9 } } } } + ; + const out = try rewritePackLocalRefs(allocator, src, &.{"Worker"}, &.{}, "citizens"); + defer allocator.free(out); + try std.testing.expect(std.mem.indexOf(u8, out, "\"citizens__Worker\": { \"hp\": 9 }") != null); +} + +test "wrapFlatEntityComponents: a flat escaped-@ pair rides into the wrapper (labelle-engine#801)" { + const allocator = std.testing.allocator; + const src = + \\{ "prefab": "base", "Worker": { "hp": 1 }, "\u0040slot": { "Position": { "x": 1 } } } + ; + const out = try rewritePackLocalRefs(allocator, src, &.{"Worker"}, &.{}, "citizens"); + defer allocator.free(out); + const wrapper_at = std.mem.indexOf(u8, out, "\"overrides\": {").?; + const target_at = std.mem.indexOf(u8, out, "\"\\u0040slot\":").?; + try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, out, "\"overrides\": {")); + try std.testing.expect(target_at > wrapper_at); +} diff --git a/src/codegen/scan/pack_refs/common.zig b/src/codegen/scan/pack_refs/common.zig index a35f9f2f..02bf5447 100644 --- a/src/codegen/scan/pack_refs/common.zig +++ b/src/codegen/scan/pack_refs/common.zig @@ -144,7 +144,15 @@ pub fn isPascalCase(name: []const u8) bool { /// must open a component map for their values, or the copy's shape drifts /// from the author's meaning. pub fn isTargetKey(name: []const u8) bool { - return name.len > 1 and name[0] == '@'; + if (name.len > 1 and name[0] == '@') return true; + // The JSON-escaped spelling decodes to `@` at engine load. These + // walkers classify RAW bytes, so recognize it here too — otherwise + // pass 1 leaves an escaped target outside the synthesized wrapper + // and pass 2 treats its value as opaque payload, silently losing + // the pack-namespace rewrite on a supported engine (codex P2 on + // #650). The PascalCase analog (`\u0057orker`) is the broader + // pre-existing class tracked in #651. + return name.len > 6 and std.mem.startsWith(u8, name, "\\u0040"); } /// A flat patch-content key at entity scope: PascalCase component diff --git a/src/root.zig b/src/root.zig index c1776b63..1cca7691 100644 --- a/src/root.zig +++ b/src/root.zig @@ -579,20 +579,21 @@ pub fn generate( var hit: ?[]const u8 = try scene_manifest.findTargetKeyUsage(allocator, scenes_target, jsonc_scene_names) orelse try scene_manifest.findTargetKeyUsage(allocator, prefabs_target, prefab_names); - // Pack sources too: pack prefabs/scenes are not staged until + // Pack sources too: pack prefabs are not staged until // `loadPackScans` later in generate, so scan each pack's SOURCE - // tree here — a light pack shipping an `@` override would - // otherwise slip past the gate (codex P1 / CodeRabbit on #650). + // prefab tree here — a light pack shipping an `@` override + // would otherwise slip past the gate (codex P1 / CodeRabbit on + // #650). Prefabs ONLY: `scanPack` stages components/events/ + // prefabs/hooks and never a pack's `scenes/`, so an example or + // fixture scene must not impose an engine-version requirement + // on the game (codex P2, round 2). if (hit == null) { for (pack_entries.items) |entry| { const src_dir = try entry.resolveSrcDir(allocator, game_dir); defer allocator.free(src_dir); - for ([_][]const u8{ "prefabs", "scenes" }) |sub| { - const sub_dir = try std.fs.path.join(allocator, &.{ src_dir, sub }); - defer allocator.free(sub_dir); - hit = try scene_manifest.findTargetKeyUsageInTree(allocator, sub_dir); - if (hit != null) break; - } + const sub_dir = try std.fs.path.join(allocator, &.{ src_dir, "prefabs" }); + defer allocator.free(sub_dir); + hit = try scene_manifest.findTargetKeyUsageInTree(allocator, sub_dir); if (hit != null) break; } } diff --git a/src/scene_name_lint.zig b/src/scene_name_lint.zig index d6cdf002..297102f5 100644 --- a/src/scene_name_lint.zig +++ b/src/scene_name_lint.zig @@ -143,7 +143,7 @@ pub fn sourceUsesTargetKeys(src: []const u8) bool { if (is_key) { const scope = if (sp > 0) stack_buf[sp - 1] else .entity; if (scope == .entity or scope == .component_map) { - if (isTargetKey(content) or isEscapedTargetKey(content)) return true; + if (isTargetKey(content)) return true; } pending_key = content; } else { @@ -253,7 +253,10 @@ fn isPascalCase(key: []const u8) bool { /// `"@"` target-override key (labelle-engine#801) — patch structure, /// not a component reference; its VALUE is a component map. fn isTargetKey(key: []const u8) bool { - return key.len > 1 and key[0] == '@'; + if (key.len > 1 and key[0] == '@') return true; + // Raw JSON-escaped spelling — same rule as + // `pack_refs/common.isTargetKey` (codex P2 on #650). + return key.len > 6 and std.mem.startsWith(u8, key, "\\u0040"); } /// True iff the next significant byte at/after `from` (skipping whitespace From 24dae7adceb2c5a6a8b0c83bbd180c46aa3df3e2 Mon Sep 17 00:00:00 2001 From: apotema Date: Mon, 3 Aug 2026 18:52:34 -0300 Subject: [PATCH 4/5] =?UTF-8?q?fix(review):=20round=203=20=E2=80=94=20gate?= =?UTF-8?q?=20fails=20closed=20on=20unscannable=20nesting,=20branch=20pins?= =?UTF-8?q?=20classified=20by=20isSemverVersion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - sourceUsesTargetKeys returns true (fail closed) past the 256-frame scope stack: an unscannable file must not bypass the version gate - engineSupportsTargetOverrides consults config.isSemverVersion first so a digit-leading branch pin (2.10.0-feature) resolves as the git ref it is, not as a prerelease below the minimum --- src/scene_manifest.zig | 8 ++++++++ src/scene_manifest_test.zig | 22 ++++++++++++++++++++++ src/scene_name_lint.zig | 14 +++++++++----- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/scene_manifest.zig b/src/scene_manifest.zig index 79393ceb..8dea23f3 100644 --- a/src/scene_manifest.zig +++ b/src/scene_manifest.zig @@ -960,6 +960,14 @@ pub const MIN_ENGINE_FOR_TARGET_OVERRIDES = "2.11.0"; /// deliberate dev setup, not that trap. Compat checking is MAJOR-only /// (labelle-cli#269), so this per-feature minimum is the only guard. pub fn engineSupportsTargetOverrides(engine_version: []const u8) bool { + // Classify with the repo's own release predicate FIRST: a + // digit-leading branch pin like `2.10.0-feature` is resolved + // verbatim as a git ref by `versionToGitRef`, yet + // `SemanticVersion.parse` would happily read it as a prerelease + // below the minimum and hard-reject a dev branch that may well + // carry the feature (codex round 3 on #650). Only true release + // pins are compared; everything else is permissive. + if (!config.isSemverVersion(engine_version)) return true; const min = std.SemanticVersion.parse(MIN_ENGINE_FOR_TARGET_OVERRIDES) catch unreachable; const pin = std.SemanticVersion.parse(engine_version) catch return true; return pin.order(min) != .lt; diff --git a/src/scene_manifest_test.zig b/src/scene_manifest_test.zig index faff1bb5..146f73bf 100644 --- a/src/scene_manifest_test.zig +++ b/src/scene_manifest_test.zig @@ -1194,3 +1194,25 @@ test "801: the JSON-escaped @ spelling still trips the gate" { \\{ "prefab": "m", "overrides": { "\u0040slot": { "Storage": {} } } } )); } + +test "801: digit-leading branch pins stay permissive; deep nesting fails closed" { + const supports = scene_manifest.engineSupportsTargetOverrides; + // `2.10.0-feature` is a BRANCH ref (isSemverVersion=false), not a + // release below the minimum — permissive (codex round 3). + try std.testing.expect(supports("2.10.0-feature")); + try std.testing.expect(!supports("2.10.0")); + + // A file too deeply nested to scan must count as "uses" so the + // gate cannot be bypassed by pathological payload (codex round 3). + const allocator = std.testing.allocator; + var deep: std.ArrayList(u8) = .empty; + defer deep.deinit(allocator); + try deep.appendSlice(allocator, "{ \"components\": { \"Config\": "); + var i: usize = 0; + while (i < 300) : (i += 1) try deep.appendSlice(allocator, "{ \"x\": "); + try deep.appendSlice(allocator, "1"); + i = 0; + while (i < 300) : (i += 1) try deep.appendSlice(allocator, " }"); + try deep.appendSlice(allocator, " } }"); + try std.testing.expect(scene_manifest.sourceUsesTargetKeys(deep.items)); +} diff --git a/src/scene_name_lint.zig b/src/scene_name_lint.zig index 297102f5..c1cab264 100644 --- a/src/scene_name_lint.zig +++ b/src/scene_name_lint.zig @@ -80,9 +80,13 @@ pub const builtin_component_names = [_][]const u8{ /// assembler's engine-version gate (codex P2 on #650). `@ref` VALUES /// (`"target": "@storage"`) never count either (colon lookahead). /// -/// Allocation-free: the scope stack is a fixed 256-frame buffer; deeper -/// nesting (not a real scene file) bails conservatively with `false` — the -/// real parse rejects such a file long before the gate matters. +/// Allocation-free: the scope stack is a fixed 256-frame buffer. Deeper +/// nesting fails CLOSED (`true`): the caller is a version gate, and +/// returning `false` on an unscannable file would let an unexamined +/// `@` key slip past onto an engine that silently drops it — the exact +/// bypass the gate exists to prevent (codex round 3 on #650). A false +/// positive here costs one loud generate error naming the file; 256 +/// levels of nesting is not a real scene either way. pub fn sourceUsesTargetKeys(src: []const u8) bool { var stack_buf: [256]Scope = undefined; var sp: usize = 0; @@ -101,7 +105,7 @@ pub fn sourceUsesTargetKeys(src: []const u8) bool { continue; } if (c == '{') { - if (sp >= stack_buf.len) return false; // pathological nesting + if (sp >= stack_buf.len) return true; // fail closed — see doc stack_buf[sp] = childScope(if (sp > 0) stack_buf[sp - 1] else null, pending_key); sp += 1; pending_key = null; @@ -109,7 +113,7 @@ pub fn sourceUsesTargetKeys(src: []const u8) bool { continue; } if (c == '[') { - if (sp >= stack_buf.len) return false; + if (sp >= stack_buf.len) return true; // fail closed — see doc stack_buf[sp] = childArrayScope(if (sp > 0) stack_buf[sp - 1] else null, pending_key); sp += 1; pending_key = null; From 129d537ce28950d7bb1ee9d78a9a64b0be0d558a Mon Sep 17 00:00:00 2001 From: apotema Date: Tue, 4 Aug 2026 11:32:22 -0300 Subject: [PATCH 5/5] fix(gate): normalize abbreviated release pins, fail closed on release-shaped unparsables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The queued pre-merge fix from codex round 4: `2.10` is release-shaped (isSemverVersion) but unparsable by SemanticVersion — treating that as a permissive branch pin let an old-release pin bypass the gate. Normalize `X.Y` to `X.Y.0`; anything release-shaped that still fails to parse fails closed. --- src/scene_manifest.zig | 12 +++++++++++- src/scene_manifest_test.zig | 10 ++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/scene_manifest.zig b/src/scene_manifest.zig index 8dea23f3..cf02fcf5 100644 --- a/src/scene_manifest.zig +++ b/src/scene_manifest.zig @@ -969,7 +969,17 @@ pub fn engineSupportsTargetOverrides(engine_version: []const u8) bool { // pins are compared; everything else is permissive. if (!config.isSemverVersion(engine_version)) return true; const min = std.SemanticVersion.parse(MIN_ENGINE_FOR_TARGET_OVERRIDES) catch unreachable; - const pin = std.SemanticVersion.parse(engine_version) catch return true; + // A release-shaped pin that SemanticVersion cannot parse is the + // abbreviated `X.Y` form (isSemverVersion admits digits+dots with + // ≥1 dot) — normalize by appending `.0` and retry. Still + // unparsable (e.g. `1.2.3.4`) → FAIL CLOSED: once the release + // predicate matched, treating parse failure as a branch pin would + // let an old-release pin bypass the gate (codex round 4 on #650). + const pin = std.SemanticVersion.parse(engine_version) catch blk: { + var buf: [64]u8 = undefined; + const padded = std.fmt.bufPrint(&buf, "{s}.0", .{engine_version}) catch return false; + break :blk std.SemanticVersion.parse(padded) catch return false; + }; return pin.order(min) != .lt; } diff --git a/src/scene_manifest_test.zig b/src/scene_manifest_test.zig index 146f73bf..8f100e3f 100644 --- a/src/scene_manifest_test.zig +++ b/src/scene_manifest_test.zig @@ -1216,3 +1216,13 @@ test "801: digit-leading branch pins stay permissive; deep nesting fails closed" try deep.appendSlice(allocator, " } }"); try std.testing.expect(scene_manifest.sourceUsesTargetKeys(deep.items)); } + +test "801: abbreviated release pins normalize; release-shaped unparsables fail closed" { + const supports = scene_manifest.engineSupportsTargetOverrides; + // `2.10` is release-shaped → normalized to 2.10.0 → below minimum. + try std.testing.expect(!supports("2.10")); + // `2.11` normalizes to 2.11.0 → supported. + try std.testing.expect(supports("2.11")); + // Release-shaped but unparsable even padded → fail closed. + try std.testing.expect(!supports("1.2.3.4")); +}