diff --git a/crates/hyperion/src/net/protocol/join.rs b/crates/hyperion/src/net/protocol/join.rs index 1d629036..ff0461ea 100644 --- a/crates/hyperion/src/net/protocol/join.rs +++ b/crates/hyperion/src/net/protocol/join.rs @@ -216,8 +216,8 @@ pub fn enter_world( // registry the client was sent in configuration, so the two cannot drift. let day_time = world.get::<&WorldTime>(|world_time| world_time.day_time); let overworld_clock = registries::WORLD_CLOCK - .id_of("minecraft:overworld") - .ok_or_else(|| anyhow::anyhow!("world_clock registry has no minecraft:overworld"))?; + .id_of(LEVEL) + .ok_or_else(|| anyhow::anyhow!("world_clock registry has no {LEVEL}"))?; send( compose, connection_id, diff --git a/nix/e2e.nix b/nix/e2e.nix index 94b85fba..633e4096 100644 --- a/nix/e2e.nix +++ b/nix/e2e.nix @@ -196,6 +196,24 @@ let # `timeout` around this script sends TERM, and bash runs an EXIT trap on # a signal only when that signal is trapped too. Without this a check # that hits its cap leaves the stack running. + # + # WITH this, it still does, and do not read the paragraph above as saying + # otherwise (ENG-11370). A trap is dispatched between commands, never + # during one, and this script spends its whole run inside the foreground + # `python3 ... | tee` pipeline below. The TERM is queued there until the + # client finishes on its own -- which is the thing the cap exists to + # bound. Measured: `smash-e2e` declares `timeout = 480`, its derivation + # carries `timeout 480 hyperion-e2e-driver`, and the run took 633s with + # the client still logging at 631.23s. `timeout` did fire (nix reports + # exit 124) and stopped nothing. So today the cap only relabels the exit + # code of a run that completed, which also makes "timed out" and "failed + # its assertions" indistinguishable from outside. + # + # The fix is to background the client and `wait` on it, because bash does + # interrupt `wait` to dispatch a trap; ENG-11370 carries the patch and the + # two directions it has to be watched failing in first. Left undone here + # deliberately: this is shared by every e2e gate and getting it wrong + # turns all of them green. trap 'exit 143' TERM INT fail() { diff --git a/nix/java/VanillaTrace.java b/nix/java/VanillaTrace.java index f8abaa7b..d2c06fb5 100644 --- a/nix/java/VanillaTrace.java +++ b/nix/java/VanillaTrace.java @@ -495,7 +495,16 @@ private Entity spawn(ServerLevel level, Scenario.EntitySpec spec) { /// A run that spends this many ticks waiting for chunks has hit something /// other than ordinary chunk loading, and saying so beats emitting a trace /// of an entity that never moved. - private static final int WARMUP_LIMIT = 600; + /// + /// The counter is ticks, not seconds, and chunk generation runs on worker + /// threads while the tick loop spins, so a host with fewer cores to spare + /// burns more ticks getting to the same place. Measured warmups on a + /// GitHub hosted runner were 45, 78, 104, 149 and 174 ticks in one run + /// that then blew past 600 on the sixth recording, so 600 was tight enough + /// to be a load sensor rather than a stuck-run detector. Four times the + /// worst observed value keeps it a guard against genuinely stuck chunks + /// without firing on a busy machine. + private static final int WARMUP_LIMIT = 2400; @Override protected void tickServer(BooleanSupplier haveTime) { @@ -580,6 +589,28 @@ protected void onServerExit() { } catch (IOException e) { throw new UncheckedIOException(e); } + // A short trace is a failed run, and it has to exit non-zero here or + // nothing downstream can tell. Minecraft catches anything thrown from + // `tickServer` -- including this recorder's own WARMUP_LIMIT guard -- + // turns it into a crash report, and then shuts the server down + // *cleanly*, which lands in this method. Exiting 0 with whatever + // samples happened to exist wrote a zero-sample trace, and the seed + // comparison in nix/differential.nix then reported the run as + // "arrow-crosswind-shot is not reproducible: seeds 4242 and 8675309 + // disagree" -- a physics claim about a run that never recorded a tick. + // `ticks + 1` because index 0 is the state before the first tick. + int wanted = scenario.ticks + 1; + if (samples.size() != wanted) { + LOGGER.error( + "{}: recorded {} of {} samples at seed {}. The run did not finish; see the" + + " crash report or the exception above for why, and do not read this" + + " trace as a disagreement about physics.", + scenario.name, + samples.size(), + wanted, + seed); + System.exit(1); + } System.exit(0); } diff --git a/tools/completions-check.py b/tools/completions-check.py index f9c72b82..d31b7f98 100755 --- a/tools/completions-check.py +++ b/tools/completions-check.py @@ -493,22 +493,33 @@ def run(args): "answer out of the graph and never ask" % (name_arg.name, name_arg.suggests) ) - set_node = path_of(graph, ["perms", "set"]) - if set_node is None: - raise SystemExit("the graph has no /perms set, only %s" % literals) - set_args = arguments_under(graph, set_node) - if len(set_args) != 1 or set_args[0].suggests != ASK_SERVER: - raise SystemExit("/perms set does not lead to one ask_server argument") + # `/perms get`, not `/perms set`. This client joins as `Normal`, and + # ENG-10871 gave every subcommand its own group precisely so the tree a + # `Normal` player is sent stops carrying `set`. Reading a permission is + # still `Normal`, so `get` is the subcommand that proves the ask_server + # wiring for a player argument. + if path_of(graph, ["perms", "set"]) is not None: + raise SystemExit( + "a Normal player was sent /perms set in the command tree. Granting " + "a permission is an Admin subcommand (ENG-10871); offering it is " + "how the escalation used to be discoverable" + ) + get_node = path_of(graph, ["perms", "get"]) + if get_node is None: + raise SystemExit("the graph has no /perms get, only %s" % literals) + get_args = arguments_under(graph, get_node) + if len(get_args) != 1 or get_args[0].suggests != ASK_SERVER: + raise SystemExit("/perms get does not lead to one ask_server argument") check.prove( "an argument tells the client to ask the server", - "/kit <%s> is %s with provider %s, and /perms set <%s> is %s with the " + "/kit <%s> is %s with provider %s, and /perms get <%s> is %s with the " "same provider" % ( name_arg.name, name_arg.parser, name_arg.suggests, - set_args[0].name, - set_args[0].parser, + get_args[0].name, + get_args[0].parser, ), ) @@ -609,10 +620,11 @@ def run(args): # 7. The second live source: a different tag, owned by a different crate, # reached through the same relation. - # Both spellings, because `set` and `get` are two nodes carrying the one - # clap id `player`: a declaration that wired only the first would pass - # on `set` and offer nothing on `get`. - for command in ("/perms set ", "/perms get "): + # `get` only, and not `set`: this client is `Normal`, so since ENG-10871 + # `set` is not in the tree it was sent and a vanilla client would never + # press tab there. `set` and `get` are two nodes carrying the one clap id + # `player`, so the relation this proves is the same one either way. + for command in ("/perms get ",): players = client.suggestions_for(command) if players.entries != [args.name]: raise SystemExit( @@ -621,9 +633,9 @@ def run(args): ) check.prove( "a player argument offers whoever is connected", - "the one connected player, %s, is the one name offered under both " - "/perms set and /perms get, queried through (Suggests, Player) at the " - "moment tab was pressed" % args.name, + "the one connected player, %s, is the one name offered under " + "/perms get, queried through (Suggests, Player) at the moment tab was " + "pressed" % args.name, ) return check.report()