From afdbfac7a8ffe2e15a85ea836ba6586853575bf4 Mon Sep 17 00:00:00 2001 From: Minseong Choi Date: Tue, 28 Jul 2026 17:27:01 +0900 Subject: [PATCH] docs: index the deferred integration seams and correct two stale markers INTEGRATION-ONLY and KNOWN-LIMITATION are grep-able, but the grep answers the wrong question. Thirty-four Go sites share the two markers and they carry four different meanings: "declared, nothing implements it" reads exactly like "implemented, only its I/O is unreachable from here", and neither reads differently from a limitation that was accepted on purpose and is not coming back. docs/deferred-seams.md sorts them, following the bucketed shape internal/updater/doc.go already uses for its own package rather than starting a second convention. Sorting them turned up two markers that had outlived the condition they describe. config.go called the modpack upload transport a deferred integration after both backends had shipped -- LocalContextStore and S3ContextStore, selected in cmd/felis by the shape of user_uploads_context, with the uploads PVC mounted and the felis-uploads-s3 Secret rendered. What is still deferred is the far end: Kaniko reading that context from inside the build Pod. updater/doc.go listed the `felis update` CLI and the off-cluster Velocity jar read under REMAINING INTEGRATION. Both exist -- cmd/felis/update.go, and gatherer_host.go, which answers Velocity from the installed jar's manifest and felis-api from the running binary's build stamp. The two nil seams that bullet also names are real, but they belong to the in-cluster gatherer only, so the bullet now says which caller has what and which is still empty. The index also records the collision that makes a naive grep misleading: 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, not that something is unbuilt, and are excluded. Both code changes are comments. Every file:line the index cites was checked against the line it points at. --- docs/deferred-seams.md | 113 ++++++++++++++++++++++++++++++++++++++ internal/config/config.go | 9 ++- internal/updater/doc.go | 18 +++--- 3 files changed, 130 insertions(+), 10 deletions(-) create mode 100644 docs/deferred-seams.md diff --git a/docs/deferred-seams.md b/docs/deferred-seams.md new file mode 100644 index 0000000..7c53a6b --- /dev/null +++ b/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. diff --git a/internal/config/config.go b/internal/config/config.go index cd8daaa..d3769ef 100644 --- a/internal/config/config.go +++ b/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 - // transport that places the blob there is a separate, deferred integration - // (INTEGRATION-ONLY). It is kept distinct from [archive] on purpose — a world + // lane derives {UserUploadsContext}/{submissionID}/context.tar.gz; both transports + // that place the blob there now ship (submit.LocalContextStore for a local path, + // 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"` diff --git a/internal/updater/doc.go b/internal/updater/doc.go index 1452d3d..df4db98 100644 --- a/internal/updater/doc.go +++ b/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 -// two current-version PRODUCING seams the gatherer still lacks — the k8s read of the -// control-plane Deployment's image (felis-api) and the off-cluster Velocity jar -// inspection, both left nil so those components surface a gather error rather than a -// wrong version — plus the concrete Notifier (SMTP + in-game) and Applier -// (control-plane image bump, cloudflared swap), the `felis update` CLI + CronJob -// entry point, and the runtime append of the live Pinned Minecraft fleet. +// - REMAINING INTEGRATION (genuinely I/O-bound — needs a cluster/mailbox): the two +// current-version PRODUCING seams NewSysGatherer still leaves nil — the k8s read of +// the control-plane Deployment's image (felis-api) and the Velocity jar inspection — +// so an IN-CLUSTER caller surfaces a gather error rather than a wrong version. The +// ON-HOST caller has both: NewHostGatherer (gatherer_host.go) answers felis-api from +// the running binary's build stamp and Velocity from the installed jar's manifest, +// 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