Merge pull request #19 from MliroLirrorsIngenuity/chore/issue-sweep

chore: work the tracker items that need no cluster

.gitattributes

@@ -0,0 +1,8 @@
+# Everything in this repository is consumed on Linux: deploy/bootstrap.sh is embedded
+# verbatim by bootstrap_asset.go and piped into `bash -s`, the Dockerfiles and entrypoints
+# are read inside the images, and the operator ships YAML. Without eol=lf a Windows
+# checkout under core.autocrlf=true hands all of them CRs.
+* text=auto eol=lf
+
+# The gradle wrappers' Windows launchers are the one thing that wants CRLF.
+*.bat text eol=crlf

.github/workflows/ci.yml

@@ -0,0 +1,94 @@
+# Runs the checks on pull requests and on main.
+#
+# release.yml already runs `go vet` and `go test`, but only once a vX.Y.Z tag exists — by then
+# a red change is on the release path and the only remedy is a new tag. This is the same gate
+# moved to where it can still stop something, plus the panel suite, which nothing ran at all:
+# the release goes through the Dockerfile, and the Dockerfile runs `npm run build`, never
+# `npm test`.
+#
+# The push trigger is limited to main rather than every branch, for the reason release.yml is
+# not repeated here: a branch with an open PR would otherwise run the whole suite twice per
+# push, once for refs/heads/<branch> and once for refs/pull/N/merge. Those are different
+# concurrency groups, so neither cancels the other, and this repository is private and billed
+# for both. A branch with no PR open yet is the one case that loses coverage, and opening the
+# PR is what asks for the answer.
+name: ci
+
+on:
+  push:
+    branches: [main]
+  pull_request:
+
+permissions:
+  contents: read
+
+concurrency:
+  group: ci-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  go:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-go@v5
+        with:
+          go-version-file: go.mod
+
+      - run: go vet ./...
+      - run: go test ./...
+
+  shell:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      # bootstrap.sh is the only thing that ever runs on a fresh host, and nothing here can
+      # run it — it wants root, a package manager and k3s. Syntax plus the extracted-block
+      # checks in bootstrap_test.sh is the coverage that is reachable without a machine.
+      # Each file is parsed by the interpreter its own shebang names. A blanket `sh -n` is
+      # wrong and not obviously so: on a developer machine `sh` is usually bash and passes
+      # everything, while the runner's `sh` is dash and rejects bootstrap.sh at the first of
+      # its arrays. Honouring the shebang is what makes local and CI agree.
+      - name: Check shell syntax
+        run: |
+          for f in $(git ls-files '*.sh'); do
+            case "$(head -1 "$f")" in
+              *bash) bash -n "$f" || exit 1 ;;
+              *) sh -n "$f" || exit 1 ;;
+            esac
+          done
+
+      - run: sh deploy/bootstrap_test.sh
+
+  panel:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      # The Dockerfile's `FROM node:<major>` is the only place the panel's Node version is
+      # declared — there is no .nvmrc and no engines field. Reading it here rather than
+      # repeating the number keeps CI testing the version a release is actually built on;
+      # a second copy would drift silently the first time the image is bumped.
+      - name: Read the panel's Node version from the Dockerfile
+        id: node
+        run: |
+          version="$(sed -n 's/^FROM.*node:\([0-9][0-9]*\)-.*/\1/p' Dockerfile | head -1)"
+          [ -n "$version" ] || { echo "Dockerfile has no 'FROM ... node:<major>' line"; exit 1; }
+          echo "version=${version}" >> "$GITHUB_OUTPUT"
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: ${{ steps.node.outputs.version }}
+          cache: npm
+          cache-dependency-path: panel/package-lock.json
+
+      - run: npm ci
+        working-directory: panel
+
+      - run: npm test
+        working-directory: panel
+
+      - run: npm run typecheck
+        working-directory: panel

bootstrap_asset_test.go

@@ -73,9 +73,15 @@ func TestLobbyLuckPermsWiringIsConsistent(t *testing.T) {
 // default it can inherit — and nothing else in the install would notice it missing. The failure
 // surfaces only when a legacy player joins, on a host that installed cleanly.
 func TestBootstrapPinsViaBlockConnectionsOff(t *testing.T) {
-	// go:embed takes the working tree verbatim, and this repository pins no eol attribute, so
+	// go:embed takes the working tree verbatim, so the eol attribute is what keeps a Windows
-	// a Windows checkout embeds CRLF. Only the assertion spanning a line break below cares.
+	// checkout from compiling CRs into the installer. Assert it rather than normalizing them
-	script := strings.ReplaceAll(BootstrapScript(), "\r\n", "\n")
+	// away: the only assertion that would otherwise notice is the one spanning a line break
+	// below, and it would report a missing pin instead of the line endings.
+	script := BootstrapScript()
+	if strings.Contains(script, "\r\n") {
+		t.Fatal("embedded bootstrap.sh has CRLF line endings; .gitattributes pins *.sh to LF " +
+			"and this script is piped into `bash -s` on a Linux host")
+	}
 
 	const key = "serverside-blockconnections"
 	if !strings.Contains(script, key+": false") {

cmd/felis/nano.go

@@ -13,8 +13,16 @@ package main
 // off-loopback with -listen; see the flag below for why that is an explicit opt-in.
 //
 // This is the no-database delivery of the identical brain `felis api` mounts through its
-// route table (internal/api.HasJoinedHandler). `felis setup --nano` / the bootstrap nano
+// route table (internal/api.HasJoinedHandler). The bootstrap installer's nano choice — its
-// choice install this as the runtime service; here it just serves.
+// `[2] Felis-nano` prompt, or FELIS_INSTALL_MODE=nano — installs this as the
+// felis-nano.service unit; here it just serves.
+//
+// There is deliberately no `felis setup --nano`. setup re-images the host through the full
+// bootstrap TUI and carries no install-mode parameter anywhere (nothing in Go reads or sets
+// FELIS_INSTALL_MODE), so such a flag would either re-run the installer — which is what
+// pointing at the installer already does — or tear a full install down into a nano one,
+// which is an uninstall, not a flag. This is the same reasoning that makes setup's --dev
+// refuse and name the installer rather than pretend to choose a channel.
 
 import (
 	"context"

deploy/bootstrap.sh

@@ -30,6 +30,14 @@
 #   FELIS_INSTALL_MODE full|nano — skip the prompt (default: ask on a tty, else full)
 #   FELIS_NANO_LISTEN listen addr for `felis nano` (default: 127.0.0.1:8081 — loopback
 #                     only; set a private-network IP to serve an off-host proxy)
+#   FELIS_LEGACY_FORWARDING_SERVERS comma-separated backends that receive their identity
+#                     through the handshake address instead of modern forwarding
+#                     (default: legacy18). Read once at Velocity start, so changing it
+#                     means re-running this script and restarting the proxy.
+#   FELIS_VELOCITY_FORK_JAR path to a Felis-Legacy Velocity fork build to install as the
+#                     proxy instead of the stock download (default: unset, stock).
+#   FELIS_VELOCITY_FORK_JAR_SHA256 expected sha256 of that jar. REQUIRED whenever the jar
+#                     above is set; the install refuses on a mismatch.
 #   FELIS_GO_VERSION  Go toolchain used to build the nano binary (default: 1.26.4)
 #   FELIS_REPO_URL    git URL to build from   (raw script mode only)
 #   FELIS_VERSION_BOOTSTRAP release|dev — which version to install (default: release).
@@ -95,6 +103,11 @@ INSTALL_MODE="${FELIS_INSTALL_MODE:-}"
 # own players stop getting in. Same-host Velocity reaches 127.0.0.1 fine; a proxy on
 # another machine must opt in explicitly with FELIS_NANO_LISTEN=<private-ip>:8081.
 FELIS_NANO_LISTEN="${FELIS_NANO_LISTEN:-127.0.0.1:8081}"
+# Backends that take their forwarded identity through the handshake address instead of
+# proxy-wide modern forwarding. See write_velocity_service for why a protocol-47 backend
+# needs this. Overridable because adding a second 1.8 backend otherwise means editing this
+# script; it is still a restart-time list, not one that follows the CRs.
+FELIS_LEGACY_FORWARDING_SERVERS="${FELIS_LEGACY_FORWARDING_SERVERS:-legacy18}"
 FELIS_GO_VERSION="${FELIS_GO_VERSION:-1.26.4}"
 PKG_LOCK_TIMEOUT="${PKG_LOCK_TIMEOUT:-${APT_LOCK_TIMEOUT:-900}}"
 APT_LOCK_TIMEOUT="${APT_LOCK_TIMEOUT:-$PKG_LOCK_TIMEOUT}"
@@ -123,9 +136,20 @@ FELIS_VELOCITY_VERSION="${FELIS_VELOCITY_VERSION:-3.5.1}"
 #
 # Opt-in because it is unmeasured where it counts: FL-008's probe runs offline-mode
 # against a stub, and this jar would carry every real Mojang session on the server.
-# The build lives in Felis-Legacy and is not byte-reproducible, so there is no digest
-# to pin here — the jar is trusted because that probe certified the build.
 FELIS_VELOCITY_FORK_JAR="${FELIS_VELOCITY_FORK_JAR:-}"
+# Expected sha256 of that jar, REQUIRED whenever it is set. Case and internal spaces are
+# ignored, so whatever sha256sum, Get-FileHash or certutil printed can be pasted as-is.
+# No digest is hardcoded here:
+# the build lives in Felis-Legacy and has never been reproduced on a second machine, so
+# any constant this script carried would pin one machine's output rather than the fork.
+#
+# So this is not a supply-chain signature and does not pretend to be one — an operator
+# who can write the jar can write this value too. What it does buy: a path is not an
+# identity, and every re-run of this script re-checks it. A truncated copy, a stale build
+# left at the same path, or the two-patch jar where the three-patch one was meant all
+# change the digest and stop the install. Naming the digest once is what turns "whatever
+# is at that path today" into one specific build.
+FELIS_VELOCITY_FORK_JAR_SHA256="${FELIS_VELOCITY_FORK_JAR_SHA256:-}"
 # Temurin 25: Velocity 3.5 needs 21+, and 25 is also what a future Velocity 4 requires,
 # so the runtime does not have to move again when the pin does. Distro JDK packaging is
 # a lottery across four package managers — a tarball is one code path everywhere (same
@@ -1462,12 +1486,31 @@ install_jre() {
 
 install_velocity() {
   install_jre
-  local url tmp
+  local url tmp have want
   prepare_velocity_layout
   if [ -n "$FELIS_VELOCITY_FORK_JAR" ]; then
     [ -f "$FELIS_VELOCITY_FORK_JAR" ] \
       || die "FELIS_VELOCITY_FORK_JAR is not a readable file: ${FELIS_VELOCITY_FORK_JAR}"
-    log "installing the Felis-Legacy Velocity fork from ${FELIS_VELOCITY_FORK_JAR}"
+    # Hash stdin, never the path — same reason as install_via_plugins: sha256sum escapes its
+    # output line for a filename carrying a backslash or a newline, and the leading "\" that
+    # adds would fail every comparison below.
+    have="$(sha256sum <"$FELIS_VELOCITY_FORK_JAR" | cut -d' ' -f1)"
+    # Refuse rather than warn. This jar is the proxy every player connects through, and a
+    # warning in an install log is not a gate. The digest is printed so the first run after
+    # a deliberate rebuild is one copy-paste, not an investigation.
+    [ -n "$FELIS_VELOCITY_FORK_JAR_SHA256" ] || die \
+      "FELIS_VELOCITY_FORK_JAR_SHA256 is required whenever FELIS_VELOCITY_FORK_JAR is set.
+   The jar at that path hashes to ${have}.
+   Check that against the build you meant to install, then re-run with
+   FELIS_VELOCITY_FORK_JAR_SHA256=${have}"
+    # Normalise the operator's digest before comparing. sha256sum prints lowercase, but the
+    # build host is often Windows, where Get-FileHash prints uppercase and certutil has
+    # shipped both with and without spaces between the bytes. All three name the same jar,
+    # so comparing raw would refuse two of the three spellings and word it as tampering.
+    want="$(printf '%s' "$FELIS_VELOCITY_FORK_JAR_SHA256" | tr -d '[:space:]' | tr 'A-Z' 'a-z')"
+    [ "$have" = "$want" ] || die \
+      "FELIS_VELOCITY_FORK_JAR checksum mismatch: got ${have}, expected ${want}"
+    log "installing the Felis-Legacy Velocity fork from ${FELIS_VELOCITY_FORK_JAR} (sha256 ${have})"
     atomic_install_file "$FELIS_VELOCITY_FORK_JAR" "${VELOCITY_DIR}/velocity.jar" 0644 root root
   else
     log "resolving the newest Velocity ${FELIS_VELOCITY_VERSION} build"
@@ -1591,9 +1634,20 @@ install_velocity_service() {
   # behind ViaVersion, which strips modern forwarding's login-plugin-message when it down-translates
   # the proxy->backend pipeline to protocol 47; only the handshake field survives Via. The Felis
   # fork reads this list from -Dfelis.legacy-forwarding.servers and forwards those servers legacy;
-  # every other backend keeps modern+secret untouched. v1 hardcodes the one legacy backend; the
+  # every other backend keeps modern+secret untouched.
-  # upgrade path is to have the operator render this list from the MinecraftServer CRs.
+  #
-  local legacy_forwarding_servers="legacy18"
+  # The list is a JVM system property, so it is fixed for the life of the proxy process and a
+  # change needs a Velocity restart. FELIS_LEGACY_FORWARDING_SERVERS makes that reachable
+  # without editing this script, which is as far as a startup property can go. Having it follow
+  # the MinecraftServer CRs instead is a larger change: the forwarding decision lives in the
+  # fork's patch to Velocity core, not in the Felis plugin, so core would need to read state the
+  # plugin owns and refreshes.
+  #
+  # The -D below is double-quoted in ExecStart on purpose. The fork trims each element, so it
+  # accepts "legacy18, legacy112", but systemd splits ExecStart on whitespace before java ever
+  # sees it -- unquoted, that spelling would hand java a stray "legacy112" argument and the unit
+  # would not start. Quoting keeps the whole property one argv item.
+  local legacy_forwarding_servers="${FELIS_LEGACY_FORWARDING_SERVERS}"
   cat > "$VELOCITY_SERVICE" <<EOF
 [Unit]
 Description=Felis Velocity proxy (Mojang authentication + modern forwarding)
@@ -1605,7 +1659,7 @@ Type=simple
 User=${VELOCITY_USER}
 Group=${VELOCITY_USER}
 WorkingDirectory=${VELOCITY_DIR}
-ExecStart=${JRE_DIR}/bin/java -Xms512M -Xmx1G -XX:+UseG1GC -XX:+ParallelRefProcEnabled -XX:+AlwaysPreTouch -Dmojang.sessionserver=http://${api_ip}:8081/session/minecraft/hasJoined -Dfelis.legacy-forwarding.servers=${legacy_forwarding_servers} -jar ${VELOCITY_DIR}/velocity.jar
+ExecStart=${JRE_DIR}/bin/java -Xms512M -Xmx1G -XX:+UseG1GC -XX:+ParallelRefProcEnabled -XX:+AlwaysPreTouch -Dmojang.sessionserver=http://${api_ip}:8081/session/minecraft/hasJoined "-Dfelis.legacy-forwarding.servers=${legacy_forwarding_servers}" -jar ${VELOCITY_DIR}/velocity.jar
 Restart=on-failure
 RestartSec=5
 NoNewPrivileges=yes

deploy/bootstrap_test.sh

@@ -0,0 +1,70 @@
+#!/bin/sh
+# Checks for deploy/bootstrap.sh. Run it as: sh deploy/bootstrap_test.sh
+#
+# The script it tests cannot be run here — it wants root, a package manager, k3s and the
+# network — so each case extracts the block it is about out of bootstrap.sh verbatim and runs
+# that with die/log stubbed. Extraction rather than a transcribed copy is the point: a copy
+# passes forever after someone edits the original.
+set -u
+
+BS="${1:-$(dirname "$0")/bootstrap.sh}"
+[ -f "$BS" ] || { echo "no such script: $BS"; exit 1; }
+fails=0
+
+expect() { # label needle haystack
+  case "$3" in
+    *"$2"*) echo "PASS $1" ;;
+    *) echo "FAIL $1: expected <$2> in:"; echo "$3"; fails=$((fails + 1)) ;;
+  esac
+}
+
+# --- the FELIS_VELOCITY_FORK_JAR digest gate -------------------------------------------
+# This jar becomes the proxy every player connects through, so the interesting cases are the
+# two refusals, not the happy path.
+
+gate="$(awk '/have="\$\(sha256sum <"\$FELIS_VELOCITY_FORK_JAR"/,/^    log "installing the Felis-Legacy/' "$BS")"
+[ -n "$gate" ] || { echo "FAIL: no fork-jar digest gate found in $BS"; exit 1; }
+# awk runs an unmatched end pattern to EOF, which would quietly pipe the rest of bootstrap.sh
+# into the `sh -c` below. The emptiness check above only catches a broken start pattern.
+[ "$(printf '%s\n' "$gate" | wc -l)" -lt 40 ] \
+  || { echo "FAIL: the extracted block is not the gate -- did its last line move?"; exit 1; }
+
+jar="$(mktemp)"
+trap 'rm -f "$jar"' EXIT
+printf 'stand-in for a fork build\n' > "$jar"
+want="$(sha256sum <"$jar" | cut -d' ' -f1)"
+
+run_gate() { # digest
+  FELIS_VELOCITY_FORK_JAR="$jar" FELIS_VELOCITY_FORK_JAR_SHA256="$1" sh -c '
+    die() { printf "DIE: %s\n" "$*"; exit 1; }
+    log() { printf "LOG: %s\n" "$*"; }
+    '"$gate"
+}
+
+out="$(run_gate '')"
+expect "fork jar without a digest is refused" "DIE: FELIS_VELOCITY_FORK_JAR_SHA256 is required" "$out"
+expect "the refusal names the jar's real digest" "$want" "$out"
+
+out="$(run_gate 'deadbeef')"
+expect "a mismatched digest is refused" "checksum mismatch: got ${want}, expected deadbeef" "$out"
+
+out="$(run_gate "$want")"
+expect "the matching digest installs" "LOG: installing the Felis-Legacy Velocity fork" "$out"
+expect "the install line records the digest" "(sha256 ${want})" "$out"
+
+# The build host is usually Windows, where nothing prints the digest the way sha256sum does:
+# Get-FileHash returns uppercase and certutil has shipped the bytes space-separated. Feeding
+# the gate its own output can never catch that -- these two cases are the operator's paste.
+out="$(run_gate "$(printf '%s' "$want" | tr 'a-z' 'A-Z')")"
+expect "an uppercase digest is the same digest" "LOG: installing the Felis-Legacy Velocity fork" "$out"
+
+out="$(run_gate "$(printf '%s' "$want" | sed 's/../& /g')")"
+expect "a space-separated digest is the same digest" "LOG: installing the Felis-Legacy Velocity fork" "$out"
+
+# ---------------------------------------------------------------------------------------
+if [ "$fails" -eq 0 ]; then
+  echo "ALL PASS"
+else
+  echo "$fails FAILED"
+fi
+exit "$fails"

deploy/crd/felis.lolicon.best_minecraftservers.yaml

@@ -269,10 +269,6 @@ spec:
               storage:
                 description: Storage configures the world PVC.
                 properties:
-                  retainOnDelete:
-                    description: RetainOnDelete keeps the PVC when the MinecraftServer
-                      is deleted.
-                    type: boolean
                   size:
                     description: Size is the requested PVC capacity (e.g. "10Gi").
                     type: string

docs/deferred-seams.md

@@ -0,0 +1,113 @@
+# Deferred integration seams
+
+`INTEGRATION-ONLY` and `KNOWN-LIMITATION` are grep-able markers in the Go source.
+This file is the index of what each one currently means, so that reading the
+unfinished face of the system does not require re-deriving it from 34 comment
+sites.
+
+It exists for two reasons. The markers do not all mean the same thing — four
+distinct states share them, and "declared, nothing implements it" reads exactly
+like "implemented, but its I/O cannot be exercised from this repo". And a marker
+outlives the condition it describes: two of them were stale when this index was
+first assembled, both claiming as future work something that had already shipped.
+
+The pattern here is the one `internal/updater/doc.go` already uses for its own
+package — state the verification boundary in buckets, so a green test suite is not
+mistaken for a finished integration. This file is the same idea across the whole
+tree.
+
+## The marker collides with a different vocabulary in troubleshooting.md
+
+`docs/troubleshooting.md` uses `[INTEGRATION-ONLY]` for something else, defined in
+its own opening at `:19`: the symptom is produced by the kubelet, kaniko or a live
+handshake, so it cannot be reproduced from the repository. Those twelve marks say
+where a failure comes from. They are not unfinished work and are not indexed below.
+A grep across `*.md` and `*.go` returns both sets; only the Go ones are seams.
+
+## Declared, nothing implements it
+
+- `internal/updates/seams.go:32` — `Notifier`. `internal/mail` sends OTP over SMTP,
+  but nothing adapts it to this interface and no in-game channel exists. `felis
+  update` passes nil deliberately: a human typing the command is the notification.
+- `internal/updates/seams.go:43` — `Applier`. Nothing applies an update anywhere. A
+  nil applier is not silent — `Run` records `errNoApplier` against every planned
+  apply, so a mis-scheduled apply is loud rather than lost.
+- `internal/updater/gatherer_integration.go:22` — the two current-version seams
+  `NewSysGatherer` leaves nil, for the in-cluster path: the k8s read of the
+  control-plane Deployment image, and the Velocity jar inspection. Both are answered
+  on the host path (see "Built" below), so this gap is specific to a caller that has
+  a cluster client instead of the node.
+- `internal/api/handlers_updates.go:21,28` — the maintenance window persists and the
+  API serves it, but the in-cluster CronJob that would hand a real window to a runner
+  does not exist. `felis update` runs with a zero window, under which every
+  `Scheduled` component degrades to a notify, so no path can currently claim an
+  apply is under way.
+- `internal/submit/blobstore.go:40` — the uploads PVC is mounted into felis-api but
+  not into the Kaniko build Pod, so a submitted context is durable at the derived
+  location without yet being readable by the build that consumes it.
+
+## Built; only its I/O is unverifiable from this repo
+
+Code exists and is unit-tested against fakes. What is missing is a host, a cluster
+or a real upstream account to run it against — not an implementation.
+
+- `cmd/felis/tui_edge_apply.go:246,274,295` — the `nft` edge fence, its idempotent
+  teardown, and the cloudflared invocation.
+- `internal/cfsetup/runner.go:18` and `internal/cfsetup/cfsetup.go:329` — the real
+  Cloudflare Tunnel and Access API calls; `internal/cfsetup/cfsetup_test.go:11`
+  drives the whole flow through a fake.
+- `internal/api/console.go:39`, `internal/api/logstream.go:236`,
+  `internal/api/logstream.go:306`, `internal/fileedit/k8sjobs.go:45` — each needs a
+  live cluster (RCON, `pods/log` follow, a Job).
+- `internal/api/handlers_access.go:170,490` — parsing real vanilla and LuckPerms
+  command output.
+
+## Deliberately accepted, not scheduled to close
+
+These are decisions, not backlog. Each names the condition under which it would be
+worth revisiting.
+
+- `internal/api/pgrepo.go:281` — the quota check and `ClaimServer` are two statements
+  (audit #4 TOCTOU). Closeable only against a real Postgres.
+- `internal/api/api.go:671` — `cooldownLimiter` is process-local, so across N api
+  replicas a caller could draw up to N OTP codes per window. The intra-replica burst
+  is closed; cross-replica bounding needs a shared store, out of scope for a
+  single-replica install.
+- `internal/submit/submit.go:436` and `internal/submit/submit_test.go:351` — a
+  post-CAS `Approve`
+  failure leaves a row indistinguishable from the benign case, so `Approve` returns a
+  distinct error naming the running build rather than allowing a blind re-drive that
+  would double-push. The alternative ordering is worse.
+- `cmd/felis/tui_edge_apply.go:246`, second marker on the same site — the fence
+  targets nftables. On a firewalld host a reload can flush the standalone table;
+  firewalld-native coordination is not handled. A missing `nft` binary fails loud
+  rather than leaving the port open.
+- `internal/api/handlers_account.go:169` — the reclaim "start fresh vs inherit"
+  choice is CODE-ONLY on the Java/Velocity side; the link-status endpoint reports
+  link completion only and does not surface it.
+
+## Wired since the marker was written
+
+- `internal/api/handlers_email_otp.go:54,223,227` and `internal/api/api.go:84` —
+  SMTP shipped on
+  2026-07-20 (`internal/mail`, wired at `cmd/felis/api.go:264`). The nil-`Mailer`
+  branch that logs the code server-side is a runtime fallback for an install with no
+  `[smtp]` section, not an unbuilt feature. The comments are accurate; the reading
+  "Felis cannot send mail" is not.
+- `internal/config/config.go:117` — was stale. It described the upload transport as a
+  deferred integration after both backends had shipped (`LocalContextStore`,
+  `S3ContextStore`, selected in `cmd/felis/api.go` by the shape of the configured
+  base). Corrected in the change that added this file; what remains deferred is only
+  Kaniko's read, indexed above.
+- `internal/updater/doc.go:44` — was stale. Its REMAINING INTEGRATION bullet listed
+  the `felis update` CLI and the off-cluster Velocity jar read, both of which exist
+  (`cmd/felis/update.go`, `internal/updater/gatherer_host.go`). Corrected in the same
+  change; the two nil seams it also names are real and remain above.
+
+## Recorded outside the code
+
+- `deploy/limbo/README.md:139` — no NetworkPolicy locks the minecraft-namespace
+  egress or the control-namespace ingress today, which is why the login pod reaches
+  `felis-api-internal:8081`. This is a conditional obligation rather than a seam: if
+  a future deployment adds either lock, it must also open that path. Spec v4.1 §21
+  asks for those policies; `cmd/felis/manifests.go` renders the game-port one.

docs/troubleshooting.md

@@ -20,8 +20,8 @@ graded for how far the in-repo Go test suite proves the behaviour:
   containerd, Postgres, or the network, not by Felis Go code; you will see it in
   `kubectl describe` / pod logs, never in `MinecraftServer.status`.
 - **[INERT]** — the configuration field exists in the CRD but no controller
-  reads it. Tuning it does nothing. These are the highest-value traps and are
+  reads it. Tuning it does nothing. §12 lists the one field this still applies
-  collected in §10.
+  to, alongside the fields that *are* read and the condition each depends on.
 
 The operator never invents the parent domain; routing identity is
 `spec.subdomain` under the deployment zone. Examples below use
@@ -32,14 +32,21 @@ concrete host.
 
 ## 1. Server is stuck in `Starting` and never becomes `Running`
 
-`MinecraftServer.status.phase` stays `Starting`. The single most important fact:
+`MinecraftServer.status.phase` stays `Starting`. A start that never succeeds is
-**the operator has no start timeout.** A start that never succeeds loops in
+requeued every 5s until one of the two startup budgets expires, then escalated
-`Starting`, requeued every 5s, **forever** — it is never auto-escalated to
+to `Failed` — `StartupTimeout` if the pod never passed TCP readiness,
-`Failed`. [GO-TESTED: `TestReconcileRunning_RconProbeFailureStaysStarting`
+`ReadinessTimeout` if the RCON probe never succeeded. Both default to **300s**
-asserts the phase stays `Starting`.] Do **not** reach for
+when `spec.startup.timeoutSeconds` / `spec.startup.readinessTimeoutSeconds` are
-`spec.startup.timeoutSeconds` / `spec.startup.readinessTimeoutSeconds` — both are
+unset or `0`, and both are measured from `status.startRequestedAt`.
-[INERT] (§10). A hung start must be diagnosed from pod state, not from
+[GO-TESTED: `TestReconcileRunning_StartupTimeoutConvertsToFailed`,
-`MinecraftServer.status`.
+`TestReconcileRunning_ReadinessTimeoutConvertsToFailed`.]
+
+So `Starting` seen *once* is normal and
+`TestReconcileRunning_RconProbeFailureStaysStarting` asserts exactly that — a
+single failed probe must not flap the phase. `Starting` seen for longer than the
+budget means the reconcile loop is not running at all; check the operator's own
+logs before tuning anything. Either way the underlying cause is diagnosed from
+pod state, not from `MinecraftServer.status`.
 
 First, read the condition reason:
 
@@ -111,8 +118,10 @@ message is the verbatim dial error:
   port yet, RCON is disabled in `server.properties`, or `spec.rcon.port`
   (default 25575) is wrong. [INTEGRATION-ONLY for the live handshake.]
 
-The per-probe timeout is a fixed 5s in code — it is **not** derived from
+The per-probe timeout is a fixed 5s in code (`prober.go:45`, shortened further if
-`spec.startup.readinessTimeoutSeconds` ([INERT]).
+the reconcile context has a nearer deadline). It is **not** derived from
+`spec.startup.readinessTimeoutSeconds`, which is the deadline for the whole
+start, not for one probe — see §12.
 
 ---
 
@@ -463,14 +472,35 @@ store paths are [INTEGRATION-ONLY].
 
 ## 11. Idle auto-stop never fires; player count always shows 0
 
-**Idle auto-stop is entirely unimplemented in the operator.** `spec.idle.*`
+Both are implemented, and both hang off the same switch: **`spec.rcon.enabled`**.
-([INERT], §12) is read by no controller, and no idle controller is registered.
+Check it first.
-An empty `Running` server stays `Running`.
 
-Relatedly, `status.players.online` is **permanently 0**: the only writer of
+```sh
-`status.players` zeroes it on stop, and the RCON prober only dials + closes — it
+kubectl get minecraftserver <name> -o jsonpath='{.spec.rcon.enabled}'
-never runs `list`. Any panel reading `status.players.online` will always show
+```
-empty. Do not build alerting on it, and do not expect "empty server" automation.
+
+The player tally is a by-product of the RCON readiness probe — `prober.go:63`
+runs `list` on the same connection that just authenticated, and `parseListReply`
+extracts the tally from `There are (\d+) of a max of (\d+) players online`. With
+RCON disabled the probe never runs, `players` keeps its zero value, and
+`markRunningReady` (`reconciler.go:413`) writes that zero into
+`status.players.online`. So a permanent 0 means "never sampled", not "nobody
+online".
+
+Idle auto-stop (`reconciler.go:175`) reads that same tally, which is why it
+carries the RCON condition explicitly:
+
+```go
+if server.Spec.Rcon.Enabled && server.Spec.Idle.AutoStopEnabled && server.Spec.Idle.EmptySecondsBeforeStop > 0 {
+```
+
+The comment above it says why: with RCON off the zero tally "would read as
+'empty' and use to stop a server full of people". So the guard is deliberate —
+enabling `spec.idle.*` without RCON is a no-op by design, not a missing feature.
+
+Both fields set and still nothing happens? Then the probe is failing rather than
+disabled: the server would be stuck in `Starting` with `RconNotReachable`
+(`reconciler.go:156`), which is §1's symptom, not this one.
 
 Note the reaper's `last_active_at` (§10) is a *different* subsystem (Postgres
 business layer, bumped by join events) — it keeps worlds alive against the
@@ -478,22 +508,21 @@ reaper, but it does **not** auto-stop empty running servers.
 
 ---
 
-## 12. Inert configuration fields (highest-value traps)
+## 12. A configuration field seems to be ignored
 
-These CRD fields exist and validate, but **no controller reads them.** Setting
+Every field below is read by a controller. What varies is the condition that
-them has **no effect**. Verified by grep: each appears only in the type
+decides whether setting it does anything.
-definition and its deepcopy, never in a controller.
 
 | Field | What you might expect | Reality |
 |---|---|---|
-| `spec.startup.timeoutSeconds` | Start budget before `Failed` | **[INERT]** — no Starting→Failed timeout exists; a hung start loops forever (§1) |
+| `spec.startup.timeoutSeconds` | Start budget before `Failed` | Read by `startupTimedOut` (`reconciler.go:479`), called at `:126`. `0` or unset falls back to **300s**, then `markFailed("StartupTimeout")` |
-| `spec.startup.readinessTimeoutSeconds` | First-probe budget | **[INERT]** — probe timeout is a fixed 5s in code |
+| `spec.startup.readinessTimeoutSeconds` | First-probe budget | Read by `readinessTimedOut` (`reconciler.go:490`), called at `:157`. `0` or unset falls back to **300s**, then `markFailed("ReadinessTimeout")`. Not to be confused with the prober's own 5s dial timeout (`prober.go:45`) |
-| `spec.idle.autoStopEnabled` | Auto-stop empty servers | **[INERT]** — idle auto-stop unimplemented (§11) |
+| `spec.idle.autoStopEnabled` | Auto-stop empty servers | Read at `reconciler.go:175` — but gated on `spec.rcon.enabled`, since the player tally comes from the RCON probe (§11) |
-| `spec.idle.emptySecondsBeforeStop` | Empty grace period | **[INERT]** |
+| `spec.idle.emptySecondsBeforeStop` | Empty grace period | Same branch. Must be `> 0`; the guard treats `0` as "off", not "stop immediately" |
-| `spec.storage.retainOnDelete` | Keep/drop PVC on delete | **[INERT]** — world PVCs **always** survive server deletion; only the reaper ever deletes a world PVC (§13) |
 
-If a runbook tells someone to "tune `startup.timeoutSeconds`" for a hung start,
+Both startup budgets are measured from the same `status.startRequestedAt`, so
-it is wrong — use `kubectl describe pod` (§1a) instead.
+`readinessTimeoutSeconds` is not a budget *after* pod readiness — it is a
+deadline for the whole start, applied on the RCON-probe branch.
 
 ---
 
@@ -503,16 +532,29 @@ This is expected. The world PVC is a StatefulSet `VolumeClaimTemplate`. There is
 **no `persistentVolumeClaimRetentionPolicy` and no finalizer** anywhere in the
 operator. Deleting the `MinecraftServer` garbage-collects the StatefulSet, but
 StatefulSet deletion does **not** cascade to its template PVCs, and nothing else
-cleans them up. So the world PVC **always survives** server deletion, regardless
+cleans them up. So the world PVC **always survives** server deletion. The
-of `spec.storage.retainOnDelete` ([INERT], §12). The **only** code that deletes a
+**only** code that deletes a world PVC is the reaper, and only after a verified
-world PVC is the reaper, and only after a verified backup (§10). To reclaim a
+backup (§10). To reclaim a world PVC manually:
-world PVC manually:
 
 ```
 kubectl get pvc -l app.kubernetes.io/name=<name>
 kubectl delete pvc <pvc>      # irreversible — the world is gone
 ```
 
+`spec.storage.retainOnDelete` sat in the CRD and reached no controller. Spec
+v4.1 §5 asks for it — 「删除:finalizer 清 Service/STS/ConfigMap,PVC 按
+`retainOnDelete`」 — and neither half was ever built: there is no finalizer, and
+nothing read the field. It was removed rather than implemented, which is a
+deliberate departure from that line, recorded here because the spec is a frozen
+document and still says otherwise.
+
+The reasoning is that implementing it buys a second path that deletes a world —
+one that skips the reaper's verified-backup check — in order to restore a
+finalizer whose other listed duties (Service, StatefulSet, ConfigMap)
+ownerReference GC already performs. A CR still carrying the field keeps working:
+the API server prunes the unknown key on its next write, and nothing above
+changes, because retention was never conditional in the first place.
+
 ---
 
 ## 14. Metrics for diagnosis (spec §23)

internal/apis/felis/v1alpha1/minecraftserver_types.go

@@ -192,8 +192,6 @@ type StorageSpec struct {
 	Size string `json:"size,omitempty"`
 	// StorageClassName selects the StorageClass; empty uses the default.
 	StorageClassName string `json:"storageClassName,omitempty"`
-	// RetainOnDelete keeps the PVC when the MinecraftServer is deleted.
-	RetainOnDelete bool `json:"retainOnDelete,omitempty"`
 }
 
 // LifecycleSpec tunes graceful shutdown (spec §7). The operator injects a

internal/config/config.go

@@ -118,9 +118,12 @@ type RegistryConfig struct {
 	// modpack's Kaniko build context is pinned. It belongs to the §16 build
 	// subsystem's input domain (the build-context store), introduced by the
 	// user-directed modpack approval lane (see internal/submit package doc). The
-	// lane derives {UserUploadsContext}/{submissionID}/context.tar.gz; the upload
+	// lane derives {UserUploadsContext}/{submissionID}/context.tar.gz; both transports
-	// transport that places the blob there is a separate, deferred integration
+	// that place the blob there now ship (submit.LocalContextStore for a local path,
-	// (INTEGRATION-ONLY). It is kept distinct from [archive] on purpose — a world
+	// submit.S3ContextStore for an s3:// base, selected in cmd/felis by the shape of
+	// this value). What stays deferred is the far end — Kaniko reading that context
+	// from inside the build Pod (INTEGRATION-ONLY, see submit/blobstore.go). It is
+	// kept distinct from [archive] on purpose — a world
 	// archive (§19 WorldArchiver) and a build context (§16) are different artifacts
 	// with different lifecycles, so the two must not share a store binding.
 	UserUploadsContext string `toml:"user_uploads_context"`

internal/updater/doc.go

@@ -41,11 +41,15 @@
 //     read and a CLI read agree instead of the image masquerading as a prerelease. The
 //     CLI seam (execRunner) is wired for real; only its exec I/O is un-verified here.
 //
-//   - REMAINING INTEGRATION (genuinely I/O-bound — needs a node/cluster/mailbox): the
+//   - REMAINING INTEGRATION (genuinely I/O-bound — needs a cluster/mailbox): the two
-//     two current-version PRODUCING seams the gatherer still lacks — the k8s read of the
+//     current-version PRODUCING seams NewSysGatherer still leaves nil — the k8s read of
-//     control-plane Deployment's image (felis-api) and the off-cluster Velocity jar
+//     the control-plane Deployment's image (felis-api) and the Velocity jar inspection —
-//     inspection, both left nil so those components surface a gather error rather than a
+//     so an IN-CLUSTER caller surfaces a gather error rather than a wrong version. The
-//     wrong version — plus the concrete Notifier (SMTP + in-game) and Applier
+//     ON-HOST caller has both: NewHostGatherer (gatherer_host.go) answers felis-api from
-//     (control-plane image bump, cloudflared swap), the `felis update` CLI + CronJob
+//     the running binary's build stamp and Velocity from the installed jar's manifest,
-//     entry point, and the runtime append of the live Pinned Minecraft fleet.
+//     and that is what the built `felis update` CLI runs on. Still absent: the concrete
+//     Notifier (SMTP + in-game) and Applier (control-plane image bump, cloudflared swap)
+//     — the CLI passes nil for both on purpose, so it reports and never applies — the
+//     in-cluster CronJob entry point, and the runtime append of the live Pinned
+//     Minecraft fleet.
 package updater

plugins/README.md

@@ -1,12 +1,16 @@
 # Felis server-side plugins
 
-These are the in-cluster and edge plugins for Felis. Every module **except the
+These are the in-cluster and edge plugins for Felis. The Velocity proxy and the three loader
-lobby** ships the in-game first leg of the §10 account-link flow: a player who is already online
+mods ship the in-game first leg of the §10 account-link flow: a player who is already online
 (so Mojang has verified their UUID) runs `/link`; the plugin asks felis-api to
 mint a one-time code for that UUID and shows it in chat. The player then enters
 the code on the web console → **Account** page (the second leg), which binds the
 code to their logged-in account. The web side is already built.
 
+The **limbo** module reaches the same felis-api endpoint without a command: it is the login
+gate, so it mints the code on join for anyone not yet linked and holds them until they redeem
+it. The **paper** lobby ships neither — see below.
+
 The **Velocity** module additionally carries the §11 domain-autostart routing
 loop — recognizing each server's subdomain, registering backends dynamically,
 waking a sleeping target and holding the player until it is ready. It is a full
@@ -19,14 +23,22 @@ The **Paper** module is different in kind: it is the §12 lobby UI face. It ship
 to Velocity, which is the only side that ever talks to felis-api. See
 **[Lobby menu](#lobby-menu-§12)** below.
 
-| Module             | Platform                    | Target                              | Jar                          |
+| Module             | Platform                    | Target                              | Jar                          | Built by the installer |
-| ------------------ | --------------------------- | ----------------------------------- | ---------------------------- |
+| ------------------ | --------------------------- | ----------------------------------- | ---------------------------- | ---------------------- |
-| `velocity/`        | Velocity proxy plugin       | velocity-api 3.3.0-SNAPSHOT         | `felis-velocity-0.2.0.jar`   |
+| `velocity/`        | Velocity proxy plugin       | velocity-api 3.3.0-SNAPSHOT         | `felis-velocity-0.1.0.jar`   | yes                    |
-| `fabric/`          | Fabric server mod           | MC 1.20.1 / fabric-loader 0.16.x    | `felis-fabric-0.1.0.jar`     |
+| `limbo/`           | LOOHP/Limbo plugin (login)  | Limbo API / Java 17 bytecode        | `felis-limbo-0.1.0.jar`      | yes                    |
-| `forge/`           | Forge server mod            | MC 1.20.1 / Forge 47.3.0            | `felis-forge-0.1.0.jar`      |
+| `paper/`           | Paper server plugin (lobby) | paper-api 1.21.4-R0.1-SNAPSHOT      | `felis-paper-0.1.0.jar`      | yes                    |
-| `neoforge/`        | NeoForge server mod         | MC 1.20.4 / NeoForge 20.4.251       | `felis-neoforge-0.1.0.jar`   |
+| `fabric/`          | Fabric server mod           | MC 1.20.1 / fabric-loader 0.16.x    | `felis-fabric-0.1.0.jar`     | no                     |
-| `paper/`           | Paper server plugin (lobby) | paper-api 1.21.4-R0.1-SNAPSHOT      | `felis-paper-0.1.0.jar`      |
+| `forge/`           | Forge server mod            | MC 1.20.1 / Forge 47.3.0            | `felis-forge-0.1.0.jar`      | no                     |
-| `shared/`          | *(not built on its own)*    | —                                   | source compiled into each    |
+| `neoforge/`        | NeoForge server mod         | MC 1.20.4 / NeoForge 20.4.251       | `felis-neoforge-0.1.0.jar`   | no                     |
+| `shared/`          | *(not built on its own)*    | —                                   | source compiled into each    | source only            |
+
+"Built by the installer" is what `deploy/bootstrap.sh` produces, and it is the same set
+`bootstrap_asset.go` embeds into the felis binary for the TUI install path, which has no source
+checkout to build from. **The three loader mods are not in that set** — a finished install has
+no `felis-fabric`/`felis-forge`/`felis-neoforge` jar anywhere. They build from this checkout with
+the commands under [Building](#building) and are deployed by hand; the account-link flow they
+carry works, but nothing installs them for you.
 
 ## Architecture
 
@@ -176,6 +188,7 @@ preference):
 | Module      | Gradle      | Why                                                              |
 | ----------- | ----------- | --------------------------------------------------------------- |
 | `velocity`  | 9.5.1 (system) | plain `java` plugin — no loader Gradle plugin                |
+| `limbo`     | 9.5.1 (system), **JDK 21 toolchain** | plain `java` plugin; current LOOHP/Limbo releases ship class-file major 65, so the compiler JDK must be ≥ 21 to read them. It emits `release 17` bytecode, so the jar still loads on any Limbo running Java 17+ |
 | `fabric`    | 8.8 (wrapper)  | loom 1.7.4 uses `Problems.forNamespace`, removed in Gradle 9 |
 | `forge`     | 8.8 (wrapper)  | ForgeGradle 6 is Gradle-8-only                               |
 | `neoforge`  | 8.14 (wrapper) | NeoGradle 7.1.38 requires Gradle API ≥ 8.14                  |
@@ -185,18 +198,20 @@ preference):
 # Velocity — system Gradle is fine
 gradle -p plugins/velocity build
 
-# Paper — system Gradle too, but it compiles on a Java-21 toolchain (see table)
+# Paper and limbo — system Gradle too, but both compile on a Java-21 toolchain (see table)
 gradle -p plugins/paper build
+gradle -p plugins/limbo build
 
-# Fabric / Forge / NeoForge — use the per-module wrapper
+# Fabric / Forge / NeoForge — use the per-module wrapper. Nothing installs these; the jar you
+# want is the one this produces.
 plugins/fabric/gradlew   -p plugins/fabric   build
 plugins/forge/gradlew    -p plugins/forge    build
 plugins/neoforge/gradlew -p plugins/neoforge build
 ```
 
-Requires JDK 17 — **except `paper`, which needs a Java-21 toolchain available to
+Requires JDK 17 — **except `paper` and `limbo`, which need a Java-21 toolchain available to
-Gradle** (paper-api 1.21.4 is a Java-21 artifact; the rest of the suite is Java
+Gradle** (paper-api 1.21.4 is a Java-21 artifact and the Limbo API is compiled to major 65; the
-17). The first build of each mod downloads and remaps/decompiles Minecraft, so it
+rest of the suite is Java 17). The first build of each mod downloads and remaps/decompiles Minecraft, so it
 takes a few minutes; subsequent builds are fast. Jars land in each module's
 `build/libs/`.