Review round 2 (batch 2 + close-out): 02/07/09/10/11/14/15/16/inventory facts corrected at file:line (EDITS.md), undocumented decisions as notes (R-1265), rulings left for the operator (R-1266); register 190 -> 207 (R-1251..R-1267 opened: R-1251 P1 cut drive-app restore put back over newer data, R-1252 P2 rejected backup block deletes second-drive copies - reproduced on 9202); STATUS D43-D45; CONTEXT; audit README
gates / gates (push) Successful in 13m10s
gates / gates (push) Successful in 13m10s
This commit is contained in:
@@ -105,7 +105,9 @@ Tri-state: `""` (unknown) · `"running"` · `"stopped"`.
|
||||
|
||||
### In-flight — a marker file, one per owner
|
||||
|
||||
Two exist and they are deliberately **separate files**: `quiesce-state.json` (the whole-guest backup
|
||||
Two exist and they are deliberately **separate files** *(four in-flight records by 2026-10-24: also `update-journal.json`
|
||||
— the guarded update's crash journal read by `RecoverUpdates`, `internal/stacks/update.go:49,1306` — and
|
||||
`.felhom-install-pending` — an interrupted install, R-681, `install_interrupted.go:28`; noted 2026-10-24)*: `quiesce-state.json` (the whole-guest backup
|
||||
window, `internal/quiesce`) and `appstop-state.json` (app-data operations that stop an app —
|
||||
`backup.AppStopGuard`, covering the volume dump, offbox reconstitution and `.fab` export). **One
|
||||
file, one writer**: sharing would give one record two lifetimes, and one owner clearing the other's
|
||||
@@ -169,7 +171,8 @@ A recovery completing after the grace emits a `LATE RECOVERY` warning naming the
|
||||
|
||||
**Nothing is started without asking whether it may be.** `bootrecon.StartGate` is the one question
|
||||
the sweep asks per candidate, and it is **fail-safe: cannot determine ⇒ do not start.** Three holders
|
||||
answer it, and the last two only became reachable once the window widened past T+5 s:
|
||||
answer it *(four since slice 4: check „1b", a guarded update moving the app — `IsUpdating`, `cmd/controller/main.go:2459-2477`;
|
||||
noted 2026-10-24)*, and the last two only became reachable once the window widened past T+5 s:
|
||||
|
||||
| Holder | Why starting would be wrong |
|
||||
|---|---|
|
||||
@@ -218,6 +221,13 @@ own; every caller that is not the customer must decide for itself whether the ap
|
||||
|
||||
---
|
||||
|
||||
> *Dated note 2026-10-24 (review round 2): this inventory is v0.33's. On `main` (`321df1f`) 22 packages under
|
||||
> `controller/internal/` have no row here: `aftercrash`, `agentapi`, `appbackup`, `backupwindow`, `bootrecon`,
|
||||
> `bootstrap`, `channelhealth`, `crashboot`, `dockerexec`, `family`, `fillwatch`, `i18n`, `infra`, `logx`, `mailrelay`,
|
||||
> `mediawire`, `nightchain`, `offsiteapply`, `privpolicy`, `quiesce`, `sockheal`, `upwatch` (an `ls` of the tree). Each
|
||||
> package's own doc comment is its description; `sockheal` (re-heal after the guest's Docker socket is recreated,
|
||||
> R-860) and `channelhealth` (a periodic proof of the controller→agent channel) are in no architecture document.*
|
||||
|
||||
## 2. Classification table (per package/file)
|
||||
|
||||
### `cmd/`
|
||||
@@ -266,6 +276,11 @@ own; every caller that is not the customer must decide for itself whether the ap
|
||||
|---|---|---|---|
|
||||
| `client.go`,`zone.go`,`waf.go`,`geosync.go`,`countries.go` | **DELETE (→hub)** | The **hub** holds the CF API token and reconciles geo desired-state → WAF (doc 01 §5, doc 03 §2). The controller no longer calls the Cloudflare API — it reports geo desired-state up. The customer-facing geo *preference UI/data* stays (see `api/geo.go`). | needs-rework |
|
||||
|
||||
*As built (noted 2026-10-24, review round 2): the `cloudflare/` package is still wired — with `cf_api_token` set the
|
||||
controller builds a `GeoSyncManager` and calls the Cloudflare API (`cmd/controller/main.go:1992-2000`; the field is still in
|
||||
`config.go:160`). The DELETE classification above is the plan, not the state; whether any box sets the token is not
|
||||
readable from source.*
|
||||
|
||||
### `config/`, `crypto/`, `util/`
|
||||
| File | Class | Reason | Risk |
|
||||
|---|---|---|---|
|
||||
@@ -325,6 +340,7 @@ household's file names. The household's lost list is shown on the app page (`los
|
||||
| File | Class | Reason | Risk |
|
||||
|---|---|---|---|
|
||||
| `version.go` | **KEEP** | Semver parse / version string (still used for reporting). | clean |
|
||||
| *(as built 2026-10-24: both files below are LIVE — the updater decides the version and the floor and hands the container swap to the agent, `AgentSwapper`, `internal/selfupdate/updater.go:38-47`; `state.go` keeps `update-state.json`)* | | | |
|
||||
| `state.go` | **DELETE (obsolete)** | Self-update audit state — the agent owns controller updates now (doc 03 §11). | clean |
|
||||
| `updater.go` | **DELETE (→agent)** | Resolved (doc 03 §11): the controller is **agent-managed** — the agent snapshots → redeploys → health-gates → rolls back the controller. The controller's old self-update path (image pull + compose edit) is **removed**. | clean |
|
||||
|
||||
@@ -387,7 +403,8 @@ dashboard alert is an `Alert` with two placement fields: `PageOnly` (the pages i
|
||||
page) and `Inline` (rendered by the page template in place, not by the layout's banner). `GetAlerts` returns
|
||||
the list (endpoint-drift, agent-channel and dead-app alerts first, then the rest sorted error > warning > info,
|
||||
capped at five plus an overflow line), and `layout.html` paints only those that are not `Inline` and match the
|
||||
page; `GetInlineAlerts(page)` hands the dashboard and monitoring pages their inline ones.
|
||||
page *(as built since R-906 — noted 2026-10-24: pages use `GetBannerAlerts(page, lang)`, which filters per page BEFORE the
|
||||
cap, `internal/web/alerts.go:353`, `handlers.go:144`)*; `GetInlineAlerts(page)` hands the dashboard and monitoring pages their inline ones.
|
||||
|
||||
**Exactly one warning is inline today:** the „storage is not on a separate drive" health warning. It is
|
||||
`PageOnly: dashboard, monitoring` and `Inline: true`, so it sits quietly under the storage bars; **every other
|
||||
@@ -821,7 +838,7 @@ R-438. The three facts below are the ones a reader needs before touching any of
|
||||
`Syncer.copyTemplates` (`sync/sync.go:319`) walks every directory in the catalog cache and copies
|
||||
`docker-compose.yml` and `.felhom.yml` into the matching stack folder. **There is no test of whether
|
||||
the app is deployed.** The only guard is a sha256 content compare (`copyIfChanged`) and the only
|
||||
exclusion is `app.yaml`. Interval is `git.sync_interval`, default `15m` (`config/config.go:351`), plus
|
||||
exclusion is `app.yaml`. Interval is `git.sync_interval`, default `15m` (`config/config.go:351` then, `:399` now; `copyTemplates` is now `sync.go:436`), plus
|
||||
one immediate sync at controller start (`sync.go:98`).
|
||||
|
||||
**SINCE v0.235.0** this walk still happens and `.felhom.yml` is still copied unconditionally, but the
|
||||
@@ -835,6 +852,7 @@ and they stay that way until something else acts.
|
||||
|
||||
### 2. Every lifecycle path resolves that disagreement, silently, by upgrading
|
||||
|
||||
*(This section is the 2026-09 measurement; `UpdateStack` was deleted in v0.237.0 — the button calls `StartGuardedUpdate`, `09` §6.1 — noted 2026-10-24.)*
|
||||
`StartStack`, `RestartStack` and `UpdateStack` (`stacks/manager.go:1029 / 1133 / 1170`) all end in
|
||||
`docker compose up -d`. `up -d` makes the container match the file, **and pulls the image itself if it
|
||||
is absent** — measured at 18.3 s with a pull versus 0.5 s without, against a negative control
|
||||
@@ -887,7 +905,9 @@ measured; that the restore writes to that path is read, not measured.
|
||||
beállítások csak olvashatók.", `deploy.html`). This is a design, not an oversight: a changed value would
|
||||
need a guarded re-deploy that nothing performs. **Finding (2026-09-15):** the catalog field flag
|
||||
`locked_after_deploy` (`stacks/metadata.go`) is parsed and read by **no** controller code — every field
|
||||
is read-only after install whatever the catalog says. So a page must never tell a household to change
|
||||
is read-only after install whatever the catalog says. *(Dated note 2026-10-24: it is now read into the app's
|
||||
`LockedFields` (`internal/stacks/deploy.go:106,534`) and enforced only by `UpdateStackConfig`, which nothing calls — so
|
||||
„read-only whatever the catalog says" still holds in practice.)* So a page must never tell a household to change
|
||||
a value after install. **Vaultwarden (R-512)** follows the design instead of breaking it: registration
|
||||
is closed by default and the household is invited from the admin panel, measured to work without mail
|
||||
(stranger 400, invite 200, invited 200).
|
||||
|
||||
Reference in New Issue
Block a user