feat(agentapi): surface agent disk-op refusal reasons; v0.101.0 + CHANGELOG/REPORT
EjectDisk/Decommission switched from c.post (drops non-2xx body) to
postWithStatus + shared refusalError, so the agent's informative 403 body
("…decommission refused (role: X)") reaches the operator instead of a bare
"HTTP 403" (campaign F2 evidence gap). Generic post + other callers untouched.
Tests T-D1/T-D2/T-D3 + ok:false case; T-D1 red-proof shows the pre-fix bare
"HTTP 403". Bundles the v0.101.0 CHANGELOG entry (this + the F3 sync deadline).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6
This commit is contained in:
@@ -1,54 +1,61 @@
|
||||
# REPORT — TASK C2: one-click class-C file restore from the Tier-2 copy (closes F2)
|
||||
# REPORT — v0.101.0: campaign findings F3 (sync deadline) + F2 evidence gap (agent refusal surfacing)
|
||||
|
||||
**Date:** 2026-07-05 · **Class:** implementation (code + tests + deploy + live validation)
|
||||
**Baseline:** `main` @ `0313ecd` (v0.99.0, matched the task's confirmed baseline exactly; not behind origin)
|
||||
**Shipped:** **v0.100.0**, live on guest 9201 (`gitea.dooplex.hu/admin/felhom-controller:0.100.0`, healthy)
|
||||
**Outcome:** drill finding **F2 CLOSED** — the whole C-series (F1/F2/F3/O4 from `DRILL-appdata-restore-2026-07-04.md`) is now resolved.
|
||||
**Date:** 2026-07-06 · **Class:** implementation (code + tests + deploy + live validation)
|
||||
**Baseline:** `main` @ `af7ea0b` (v0.100.0, matched the task's confirmed baseline exactly).
|
||||
**Provenance:** `felhom.eu/documentation/audits/CAMPAIGN-nomercy-2026-07-06.md` findings F3, F2.
|
||||
**Companion repos:** catalog `d86e256` (F1 vaultwarden fix); felhom.eu audit addendum (F1/F2/F3 verdicts).
|
||||
|
||||
## Commits (all on `main`)
|
||||
## What changed
|
||||
|
||||
| Hash | What |
|
||||
|---|---|
|
||||
| `30b1110` | Part 1 — `RestoreTier2Files` engine + `rsyncRestoreMissing` + copier seam + tests |
|
||||
| `27aeb41` | Parts 2+3 — `POST /backup/tier2/restore` + "Fájlok visszaállítása" button + handler-guard test |
|
||||
| `85a5727` | Part 4 — CHANGELOG v0.100.0, CONTEXT (F2 closed), REUSE, controller/README |
|
||||
| felhom.eu `7e42ca6` | backup-architecture.md — class-C in-place path, additive-only semantics, reindex caveat |
|
||||
### F3 — git subprocess deadline (`internal/sync/sync.go`)
|
||||
`runGitInDir` took no context; a hung remote parked the sync goroutine in `cmd.Run()`, the `doSync`
|
||||
defer never ran, `syncing` stayed true, and every subsequent manual + periodic sync was refused
|
||||
("Szinkronizálás már folyamatban") until a controller restart. Now: `const gitCmdTimeout = 120s`;
|
||||
a `gitCmd` wrapper derives a fresh per-command `context.WithTimeout`; `runGitInDir` takes a
|
||||
`context.Context` and uses `exec.CommandContext`; on `ctx.Err()` the error names the timeout + the
|
||||
masked git args. All three call sites (clone via `runGit`, fetch, reset) route through it. Debounce
|
||||
and failed-sync-arms-debounce semantics unchanged.
|
||||
|
||||
**Files (code):** `internal/backup/{tier2_restore.go(new), backup.go(seam)}`, `internal/web/{server.go, handlers.go, templates/backups.html}`. Tests (new): `internal/backup/tier2_restore_test.go`, `internal/web/tier2_restore_handler_test.go`. Template gates (id + emoji) green.
|
||||
### F2 evidence gap — agent refusal surfacing (`internal/agentapi/client.go`)
|
||||
`EjectDisk` + `Decommission` used `c.post`, which drops any non-2xx body, flattening the agent's
|
||||
informative refusal to a bare `HTTP 403` (the exact campaign evidence:
|
||||
`agentapi: POST /disks/decommission: HTTP 403`). Both switched to the purpose-built
|
||||
`postWithStatus` + a new shared `refusalError(path, status, env)` that returns
|
||||
`agentapi: POST <path>: HTTP <code>: <agent reason, truncated ~300>` (and handles the ok:false-on-2xx
|
||||
business-refusal case). `truncateErr` mirrors `stacks.truncateStr`. The generic `post` and every
|
||||
other caller are untouched. `internal/web/storage_handlers.go` already wraps the client error into
|
||||
its Hungarian message (`"a meghajtó leszerelése sikertelen: %w"` / eject `err.Error()`), so the role
|
||||
reason now reaches the UI/API response verbatim — verified by reading the handler paths (no
|
||||
re-truncation layer).
|
||||
|
||||
## What shipped
|
||||
## Tests + red-proofs (non-hollow)
|
||||
|
||||
- **Engine** `Manager.RestoreTier2Files(stack) (int, error)`: in-place, **additive-only** restore of missing user files from the **recorded** Tier-2 copy (`CrossDriveBackup.DestinationPath` + the `backups/secondary/<stack>/appdata` literals from `RunTier2` — never a fresh `selectTier2Target`). Single-flight with backup/restore; **all refusals before any stop**, each with a customer-readable Hungarian reason (no copy / never ran / copy dir gone / Tier-2 drive disconnected / live drive disconnected or decommissioned); stop → copy → start → 90s health-wait; copy and restart errors surface (F17), health timeout only warns.
|
||||
- **Copier** `rsyncRestoreMissing`: `rsyncMirror`'s exec shape (mkdir, 60m ctx, trailing-slash contents copy, rsync's own exit code) with the opposite-direction flags — **`-a --ignore-existing --itemize-changes`**. Existing live files never overwritten; nothing ever deleted (the `--delete` trap). Count = `>f` itemize lines (pure `countRestoredFiles`); file names logged at DEBUG only, never INFO.
|
||||
- **Endpoint + UI**: `POST /backup/tier2/restore` (backupRestoreHandler-shaped guards) + the button on the **healthy Tier-2 row only** (Tier2Configured branch already excludes disconnected/inactive; extra `Tier2LastRun` gate), with the confirm dialog naming the additive-only contract + last-copy timestamp. Flash strings exactly per spec; n==0 is a success.
|
||||
- **Out of scope honoured:** no overwrite/point-in-time mode, no per-file selection, `recovery-unit/` untouched, offbox untouched, C1-report Obs 1/2/3 untouched.
|
||||
| Test | Asserts | Red-proof (pre-fix shape) |
|
||||
|---|---|---|
|
||||
| `TestRunGitInDir_CancelledContextKillsSubprocess` (T-C1) | cancelled ctx → error names the ctx cause + "deadline/killed", returns < 5s | reverted to `exec.Command` → **FAIL**: *"got nil (git --version ran to completion — the pre-fix behavior)"* |
|
||||
| `TestTriggerSync_FailureReleasesSyncingAndAllowsRetry` (T-C2) | failed sync carries "Git hiba", `syncing` false after, immediate retry hits debounce, post-debounce retry EXECUTES | (behavioral; guards the lockup) |
|
||||
| `TestDecommission_RefusalReasonSurfaced` (T-D1) | error contains "(role: system)" + "HTTP 403" | reverted to `c.post` → **FAIL**: *"agent refusal reason discarded — operator sees only: agentapi: POST /disks/decommission: HTTP 403"* |
|
||||
| `TestEjectDisk_RefusalReasonSurfaced` (T-D2) | error contains "(role: backup)" | — |
|
||||
| `TestDecommission_SuccessUnchanged` (T-D3) | 200 envelope decodes to result | — |
|
||||
| `TestDecommission_OkFalseBusinessRefusal` | ok:false-on-2xx reason surfaced | — |
|
||||
|
||||
## Tests & the §10 companion
|
||||
Both red-proofs demonstrated live (revert → FAIL with the wrong value shown → restore → pass;
|
||||
`git diff` clean). Green gate `go build ./... && go vet ./... && go test ./...` = **PASS** (exit 0);
|
||||
30 passing tests across `internal/sync` + `internal/agentapi` (+2 new test files, +6 new tests).
|
||||
|
||||
`go build ./... && go vet ./... && go test ./...` green after every commit. **Top-level test functions: 286 → 293 (+7).**
|
||||
## Deploy + live validation
|
||||
(Filled in during Phase 4/5 of this session — see below; secrets-safe.)
|
||||
|
||||
- Orchestration (seam): stop→copy→start order recorded; copier src = the recorded Tier-2 layout, dst = live `AppDataDir`; count passthrough.
|
||||
- Refusals C1–C5: each asserts the **non-effect** — `StopStack` never called, copier never invoked (C5 via a held running flag). C6 at the handler (`httptest`; nil backupMgr would panic if reached — it isn't).
|
||||
- Scenario D: `(0, nil)` success; copy-error surfacing (app still restarted).
|
||||
- FS-level semantics against **real rsync** (LookPath-skipped on Windows; **PASS on the build server**): deleted file restored byte-identical + counted; differing live file keeps live bytes; live-only file survives.
|
||||
- **Companion red-proof (run on the build server, where rsync exists):** flags swapped to `rsyncMirror`'s (`-a --delete`, no `--ignore-existing`) → the test failed **on both harms**: `live-edited b.txt was CLOBBERED: "BACKUP-VERSION"` **and** `live/c.txt: no such file or directory` (plus count 2≠1). Mutation reverted (`git status` clean on the build server), test green again.
|
||||
- Built + pushed `gitea.dooplex.hu/admin/felhom-controller:0.101.0`; deployed to guest 9201.
|
||||
- Scenario A (F1 acceptance): fresh email-off vaultwarden deploy healthy, `_ENABLE_SMTP=false`.
|
||||
- Scenario B (regression): app-email ON→healthy (`_ENABLE_SMTP=true`, host+from set) → OFF→healthy.
|
||||
- Sync smoke: `POST /api/sync` 2xx.
|
||||
- F2 diagnosis verdict: see the felhom.eu audit addendum.
|
||||
|
||||
## Deploy + live validation (§13, felhom-pve / guest 9201 only — Peti's box untouched)
|
||||
## NOT yet live-validated
|
||||
Clean backup/restore (F3-volume-tar / C2 Tier-2) coverage on the now-healthy vaultwarden — awaits
|
||||
the separate targeted P1–P3 campaign re-run (a runbook, not this task).
|
||||
|
||||
Build `./build.sh 0.100.0 --push` on 180; bootstrap deploy; verify `:0.100.0 Up (healthy)`, no fatal/panic.
|
||||
|
||||
**Method note:** the claude-in-chrome bridge disconnected mid-session, so the first pass verified the **rendered UI HTML** (button + confirm + hidden `stack_name`, exactly once on nextcloud's Tier-2 row) and exercised the restore as the **exact form POST the button submits**. After Chrome was reopened, the **real click was also validated end-to-end** (step 8 below) — the UI path is now fully click-proven, no proxy caveat remains.
|
||||
|
||||
1. **Fixtures:** sentinel `d982fa3b….bin` (sha `1e5f82d7…`) + `modify_me.txt` (sha `c5ca964b…`) planted in live `appdata/nextcloud/drill` → `POST /api/backup/tier2` → both in the Tier-2 copy on felhom-flash, shas identical.
|
||||
2. **Three fates staged after the copy:** `.bin` deleted live; `modify_me.txt` edited live (new sha `bd24729a…`); `live_only.txt` created live (sha `3c3474d5…`).
|
||||
3. **Restore:** `302 → flash "nextcloud: 1 fájl visszaállítva a másodlagos másolatból."` Log: `Tier-2 file restore for nextcloud: …/felhom-flash/…/appdata → …/felhom-usb/appdata/nextcloud (additive-only)` … `1 file(s) restored (15s)`.
|
||||
4. **All three fate assertions PASSED:** `.bin` back **byte-identical** (`1e5f82d7…`); `modify_me.txt` kept its **LIVE** sha (`bd24729a…`, NOT the backup's `c5ca964b…`); `live_only.txt` **survived** (`3c3474d5…`). Nextcloud healthy after restart.
|
||||
5. **Idempotency (Scenario D):** immediate rerun → `flash "Nincs hiányzó fájl — minden fájl megvan a helyén."` (success, 0 files).
|
||||
6. **Refusal non-effect:** sparkyfitness (no Tier-2 record) — the button is **absent** from its row (rendered-HTML count = 1, nextcloud only); direct POST → `flash_error "Fájl-visszaállítás sikertelen: nincs másodlagos fájlmásolat ehhez az alkalmazáshoz"`; container IDs + uptimes **unchanged** (never stopped).
|
||||
7. **Cleanup:** drill dir removed live; fresh Tier-2 run purged the copy side (mirror semantics); zero residue both sides; final `docker ps` all healthy.
|
||||
8. **Real-click UI validation (Chrome reopened):** fresh sentinel (`click.txt`, sha `6ce3342e…`) → Tier-2 run → deleted live → **the actual "Fájlok visszaállítása" button clicked in the dashboard** (Nextcloud row expanded; the native confirm dialog fired with the exact spec text incl. the live last-copy timestamp `2026-07-05T11:35:49Z`, auto-accepted via a test-only `window.confirm` override to avoid the extension-blocking dialog) → flash banner **"nextcloud: 1 fájl visszaállítva a másodlagos másolatból."** rendered on the page (screenshot) → file back **byte-identical** (`6ce3342e…`), nextcloud healthy. Cleanup repeated: zero residue, all healthy.
|
||||
|
||||
## Observations (not acted on)
|
||||
|
||||
1. **Reindex caveat is real but untested in-app** — the restored file returns at filesystem level; whether Nextcloud's own UI lists it may require `occ files:scan` (documented in backup-architecture.md; not exercised — the drill fixture lives outside Nextcloud's indexed user dirs).
|
||||
2. **Carried from C1 (still open, unchanged):** O4-residual/F3 interaction, compose stderr truncation, compose volume-label warnings, demo-box unauth/empty-CSRF exposure.
|
||||
## Not changed
|
||||
felhom-agent (read-only; F2 is diagnosis only). The generic `agentapi.post` error format, the sync
|
||||
debounce, and the other five smtp-mapped catalog templates.
|
||||
|
||||
Reference in New Issue
Block a user