From b1bca987382205af072b990c05aef9ebcda724c3 Mon Sep 17 00:00:00 2001 From: Anton Vakhrushev Date: Wed, 8 Jul 2026 16:40:49 +0300 Subject: [PATCH] =?UTF-8?q?=D0=A0=D0=B5=D0=B2=D1=8C=D1=8E:=20preflight=20?= =?UTF-8?q?=D0=B3=D0=BE=D1=82=D0=BE=D0=B2=D0=BD=D0=BE=D1=81=D1=82=D0=B8=20?= =?UTF-8?q?=D0=B8=D1=81=D1=82=D0=BE=D1=87=D0=BD=D0=B8=D0=BA=D0=B0=20=D0=B4?= =?UTF-8?q?=D0=BB=D1=8F=20=D0=BA=D0=BE=D0=BC=D0=B0=D0=BD=D0=B4=20=D1=80?= =?UTF-8?q?=D0=B5=D0=B2=D1=8C=D1=8E=20(MAJOR-5)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Команды ревью проверяли только наличие раздачи в qBittorrent, но не её готовность. Недокачанную задачу можно припарковать в deferred, затем «Распознать заново» → recognizing → авто-раскладка (Rerecognize/Refine/ SetType не ставят force_review) → хардлинки на неполные файлы. Даже ручной Apply не имел preflight завершённости. Вводим ensureSourceReady (classify(t.State)==classReady) вместо ensureSourcePresent во всех командах, которым нужен источник (Relink/ Rerecognize/Refine/SetType), и inline-проверку класса в Apply — последний рубеж перед хардлинками. Недокачанный источник → отдельный sentinel ErrNotReady (409) с actionable-текстом «торрент ещё качается» в web и Telegram, без reconcile (состояние deferred/review легитимно). Change review-readiness-preflight заархивирован, дельта влита в openspec/specs/review. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/backlog/README.md | 1 - .../review-major5-readiness-preflight.md | 11 -- internal/httpapi/httpapi.go | 10 +- internal/httpapi/httpapi_test.go | 21 ++++ internal/tgbot/bot.go | 6 + internal/worker/errors.go | 7 ++ internal/worker/reconcile.go | 30 +++-- internal/worker/reconcile_test.go | 4 +- internal/worker/review.go | 20 ++-- internal/worker/review_test.go | 94 ++++++++++++++- .../.openspec.yaml | 2 + .../design.md | 107 ++++++++++++++++++ .../proposal.md | 56 +++++++++ .../specs/review/spec.md | 67 +++++++++++ .../tasks.md | 45 ++++++++ openspec/specs/review/spec.md | 35 +++++- 16 files changed, 475 insertions(+), 41 deletions(-) delete mode 100644 docs/backlog/review-major5-readiness-preflight.md create mode 100644 openspec/changes/archive/2026-07-08-review-readiness-preflight/.openspec.yaml create mode 100644 openspec/changes/archive/2026-07-08-review-readiness-preflight/design.md create mode 100644 openspec/changes/archive/2026-07-08-review-readiness-preflight/proposal.md create mode 100644 openspec/changes/archive/2026-07-08-review-readiness-preflight/specs/review/spec.md create mode 100644 openspec/changes/archive/2026-07-08-review-readiness-preflight/tasks.md diff --git a/docs/backlog/README.md b/docs/backlog/README.md index 6028792..a1dfb70 100644 --- a/docs/backlog/README.md +++ b/docs/backlog/README.md @@ -22,7 +22,6 @@ Tududi (проект `jellybit`) больше **не** держит беклог - [Ретеншн и очистка БД](retention-ochistka-bd.md) — Терминальные задачи (done/cancelled/failed/reverted), их попытки recognition с сырыми… - [Eval-харнес распознавания (корпус кейсов + метрика точности)](eval-harness-raspoznavaniya.md) — Распознавание — ядро продукта, но смена модели или правка промпта сейчас вслепую… - [Гейт дозаписи хешей в dedup-ветке CreateDownloadIfNoActive (F1)](review-f1-gate-dozapisi-heshey.md) — dedup-ветка дописывает все хеши в найденную задачу без гарда — риск инварианта ≤1 активной _(ревью 2026-07-08)_ -- [Readiness-preflight: запрет авто-раскладки недокачанных файлов (MAJOR-5)](review-major5-readiness-preflight.md) — САМОЕ ОПАСНОЕ: авто-раскладка недокачанных файлов рвёт целостность библиотеки _(ревью 2026-07-08)_ - [Retry/stall семантика: сброс базиса таймаута + простой от начала, а не от возраста торрента (MAJOR-1, MAJOR-2)](review-major1-2-retry-stall.md) — таймаут и простой отсчитываются от возраста торрента, а не от начала загрузки _(ревью 2026-07-08)_ - [Восстановление zombie downloading при пропаже источника из qBittorrent (MAJOR-3)](review-major3-zombie-downloading.md) — торрент пропал из qBittorrent в downloading → задача вечный зомби, никто не двигает _(ревью 2026-07-08)_ diff --git a/docs/backlog/review-major5-readiness-preflight.md b/docs/backlog/review-major5-readiness-preflight.md deleted file mode 100644 index e159836..0000000 --- a/docs/backlog/review-major5-readiness-preflight.md +++ /dev/null @@ -1,11 +0,0 @@ -# Readiness-preflight: запрет авто-раскладки недокачанных файлов (MAJOR-5) - -**Приоритет:** высокий · **Теги:** review-2026-07-08, lifecycle, data-integrity - -Ревью Fable 2026-07-08 (жизненный цикл). САМАЯ ОПАСНАЯ — целостность библиотеки. internal/worker/review.go (ensureSourcePresent проверяет присутствие источника, НЕ classify(t.State)==classReady). - -Сценарий: задача на 40% в downloading → user «Позже» (deferred, легально: граф допускает *→deferred) → «Распознать заново» (Rerecognize) → ensureSourcePresent проходит (торрент есть) → recognizing → LLM видит имена файлов (qBit отдаёт до завершения) → при уверенном матче res.Decision.Auto && !forceReview (Rerecognize НЕ ставит force_review, только Relink ставит) → авто linking → хардлинки на НЕДОКАЧАННЫЕ файлы → done → Jellyfin сканирует половину. Даже ручной review→Apply не имеет preflight завершённости. Обходит состояние completed, чей смысл (download-tracking «Готовность только когда файлы на месте») — финальность. - -Фикс: ensureSourceReady требует classify(t.State)==classReady для Rerecognize/Refine/SetType/Apply; иначе ErrConflict «торрент ещё качается». - -Вердикт: полноценный change (новое требование preflight на readiness). diff --git a/internal/httpapi/httpapi.go b/internal/httpapi/httpapi.go index 1726ab8..bda875f 100644 --- a/internal/httpapi/httpapi.go +++ b/internal/httpapi/httpapi.go @@ -747,15 +747,19 @@ func writeJSON(w http.ResponseWriter, status int, v any) { // classifyErr транслирует доменную ошибку в HTTP-статус и нейтральное // человекочитаемое сообщение публичного канала (без сырого err.Error() и // деталей реализации): ErrNotFound → 404, валидация источника -// (magnet.ErrNotMagnet) → 400, конфликт состояния (worker.ErrConflict) → 409, -// прочее → 500. Полная ошибка уже в логах на доменной границе — наружу отдаём -// только сообщение + корреляционный ключ. +// (magnet.ErrNotMagnet) → 400, недокачанный источник (worker.ErrNotReady) и +// конфликт состояния (worker.ErrConflict) → 409, прочее → 500. Полная ошибка +// уже в логах на доменной границе — наружу отдаём только сообщение + +// корреляционный ключ. func classifyErr(err error) (int, string) { switch { case errors.Is(err, store.ErrNotFound): return http.StatusNotFound, "не найдено" case errors.Is(err, magnet.ErrNotMagnet), errors.Is(err, torrent.ErrNotTorrent): return http.StatusBadRequest, "некорректный источник" + case errors.Is(err, worker.ErrNotReady): + // Источник ещё качается — actionable причина, показываем конкретно. + return http.StatusConflict, "торрент ещё качается, дождитесь докачки" case errors.Is(err, worker.ErrConflict): // Нормальный конфликт состояния (операция недопустима сейчас), не сбой. return http.StatusConflict, "действие недоступно в текущем состоянии" diff --git a/internal/httpapi/httpapi_test.go b/internal/httpapi/httpapi_test.go index 9b8a26d..5421300 100644 --- a/internal/httpapi/httpapi_test.go +++ b/internal/httpapi/httpapi_test.go @@ -226,6 +226,27 @@ func TestAPICommandConflict(t *testing.T) { } } +func TestAPICommandNotReady(t *testing.T) { + // Недокачанный источник (worker.ErrNotReady) → 409 с конкретным сообщением + // «торрент ещё качается», а не генерик «действие недоступно». + cmd := &fakeCommander{err: fmt.Errorf("apply: торрент ещё качается: %w", worker.ErrNotReady)} + srv := newServer(t, httpapi.Deps{Ingestor: &fakeIngestor{}, Commander: cmd, Reader: &fakeReader{}}) + + resp, err := http.Post(srv.URL+"/api/downloads/"+tid+"/cancel", "", nil) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusConflict { + t.Fatalf("status = %d, want 409", resp.StatusCode) + } + var got map[string]any + _ = json.NewDecoder(resp.Body).Decode(&got) + if msg, _ := got["error"].(string); !strings.Contains(msg, "качается") { + t.Errorf("error = %q, want содержащее «качается»", msg) + } +} + func TestIndexRenders(t *testing.T) { reader := &fakeReader{list: []store.Download{ {ID: tid, SourceType: store.SourceMagnet, SourceRef: "magnet:?xt=urn:btih:abc", State: store.StateDownloading}, diff --git a/internal/tgbot/bot.go b/internal/tgbot/bot.go index 13630a2..41d3de1 100644 --- a/internal/tgbot/bot.go +++ b/internal/tgbot/bot.go @@ -319,6 +319,12 @@ func (b *Bot) handleCallback(ctx context.Context, cq *tgbotapi.CallbackQuery) { } if err != nil { + if errors.Is(err, worker.ErrNotReady) { + // Источник ещё качается — actionable причина, показываем конкретно. + b.answer(cq.ID, "Торрент ещё качается") + b.send(chatID, opErr("Торрент ещё качается — дождитесь докачки", id), nil) + return + } b.answer(cq.ID, "Ошибка") b.send(chatID, opErr("Не удалось выполнить действие", id), nil) return diff --git a/internal/worker/errors.go b/internal/worker/errors.go index 4e9d42b..96980be 100644 --- a/internal/worker/errors.go +++ b/internal/worker/errors.go @@ -6,3 +6,10 @@ import "errors" // вне review/deferred, undo вне done). Это нормальный конфликт состояния, а не // сбой сервера: транспорт матчит его через errors.Is и отвечает 409, не 500. var ErrConflict = errors.New("conflict") + +// ErrNotReady — источник (раздача) присутствует в qBittorrent, но ещё не +// докачан (не в классе classReady), поэтому команда, ведущая к распознаванию +// или раскладке, отклонена: хардлинки на неполные файлы недопустимы. Отдельно +// от ErrConflict (тоже 409), потому что причина actionable — «дождись докачки» +// — и транспорт показывает её конкретным текстом, а не генериком конфликта. +var ErrNotReady = errors.New("source not ready") diff --git a/internal/worker/reconcile.go b/internal/worker/reconcile.go index acc7a0a..6323ca6 100644 --- a/internal/worker/reconcile.go +++ b/internal/worker/reconcile.go @@ -245,23 +245,33 @@ func recoveredState(state string) store.State { // --- Синхронный preflight перед действием (не доверяем state в БД) --- -// ensureSourcePresent синхронно (без дебаунса) проверяет, что раздача есть в -// qBittorrent прямо сейчас. При отсутствии приводит состояние к реальности и -// возвращает ErrConflict. Недоступность qBittorrent — честный отказ операции. -func (w *Worker) ensureSourcePresent(ctx context.Context, d *store.Download, op string) error { +// ensureSourceReady синхронно (без дебаунса) проверяет, что раздача есть в +// qBittorrent прямо сейчас И докачана (класс classReady). Команды ревью, ведущие +// к распознаванию или раскладке, работают только с готовым источником — иначе +// хардлинки легли бы на неполные файлы (qBittorrent отдаёт имена до завершения). +// - источник исчез → приводим состояние к реальности (orphaned/deleted) и +// ErrConflict; +// - источник есть, но ещё качается → ErrNotReady БЕЗ reconcile: нахождение +// задачи в review/deferred/… легитимно, приводить нечего. +// +// Недоступность qBittorrent — честный отказ операции. +func (w *Worker) ensureSourceReady(ctx context.Context, d *store.Download, op string) error { if len(d.Infohashes) == 0 { return fmt.Errorf("%s: download %s has no infohash", op, d.ID) } - _, ok, err := w.torrentByInfohash(ctx, d.HashList()) + t, ok, err := w.torrentByInfohash(ctx, d.HashList()) if err != nil { return fmt.Errorf("%s: %w", op, err) } - if ok { - return nil + if !ok { + // Источник пропал — немедленно приводим состояние к реальности. + w.reconcileToReality(ctx, *d, false) + return fmt.Errorf("%s: источник удалён из qBittorrent: %w", op, ErrConflict) } - // Источник пропал — немедленно приводим состояние к реальности. - w.reconcileToReality(ctx, *d, false) - return fmt.Errorf("%s: источник удалён из qBittorrent: %w", op, ErrConflict) + if classify(t.State) != classReady { + return fmt.Errorf("%s: торрент ещё качается: %w", op, ErrNotReady) + } + return nil } // reconcileToReality выводит и проставляет состояние по уже известному факту об diff --git a/internal/worker/reconcile_test.go b/internal/worker/reconcile_test.go index 8dea5ee..b0a793d 100644 --- a/internal/worker/reconcile_test.go +++ b/internal/worker/reconcile_test.go @@ -41,7 +41,9 @@ func newReconcileFixture(t *testing.T, state store.State, sourcePresent, makeTar var torrents []qbt.Torrent if sourcePresent { - torrents = []qbt.Torrent{{Hash: ihTest}} + // Раздача докачана (seeding) — classReady: preflight Relink требует + // готовности источника, а не только присутствия. + torrents = []qbt.Torrent{{Hash: ihTest, State: "uploading"}} } w := testWorkerWith(st, &fakeQbt{torrents: torrents}, nil, nil) w.cfg.SourceMissingThreshold = 1 // помечаем при первой же пропаже (без задержки) diff --git a/internal/worker/review.go b/internal/worker/review.go index d3c63d8..fca7a71 100644 --- a/internal/worker/review.go +++ b/internal/worker/review.go @@ -247,6 +247,11 @@ func (w *Worker) Apply(ctx context.Context, id string) error { w.reconcileToReality(ctx, *d, false) return fmt.Errorf("apply: источник удалён из qBittorrent: %w", ErrConflict) } + if classify(t.State) != classReady { + // Последний рубеж перед хардлинками: источник ещё качается — не + // линкуем неполные файлы (задача могла войти в review иным путём). + return fmt.Errorf("apply: торрент ещё качается: %w", ErrNotReady) + } w.transition(ctx, *d, store.StateLinking, "", "") if err := w.linkPlan(ctx, d, plan, tag, translatePath(t.SavePath, w.cfg.PathMap)); err != nil { @@ -321,7 +326,7 @@ func (w *Worker) linkPlan(ctx context.Context, d *store.Download, plan recognize // (cancelled) задачу: возвращает её на распознавание, и поллинг-цикл // перезапустит recognize. Авто-раскладку при этом не делаем — ручная // перепривязка всегда проходит через ревью с подтверждением (force_review). -// Источник (раздача в qBittorrent) для этого должен быть на месте. +// Источник (раздача в qBittorrent) для этого должен быть на месте и докачан. func (w *Worker) Relink(ctx context.Context, id string) error { w.mu.Lock() defer w.mu.Unlock() @@ -333,9 +338,10 @@ func (w *Worker) Relink(ctx context.Context, id string) error { if d.State != store.StateReverted && d.State != store.StateCancelled && d.State != store.StateTargetMissing { return fmt.Errorf("relink: download %s is in state %s (expected reverted/cancelled/target_missing): %w", id, d.State, ErrConflict) } - // Источник нужен для распознавания — проверяем синхронно (без дебаунса) и при - // его отсутствии приводим состояние к реальности (orphaned/deleted). - if err := w.ensureSourcePresent(ctx, d, "relink"); err != nil { + // Источник нужен для распознавания и должен быть докачан — проверяем + // синхронно (без дебаунса); отсутствие приводит состояние к реальности + // (orphaned/deleted), недокачанный — ErrNotReady. + if err := w.ensureSourceReady(ctx, d, "relink"); err != nil { return err } // Ручная перепривязка — всегда с подтверждением, без авто-раскладки. @@ -366,7 +372,7 @@ func (w *Worker) Rerecognize(ctx context.Context, id string) error { if err != nil { return err } - if err := w.ensureSourcePresent(ctx, d, "rerecognize"); err != nil { + if err := w.ensureSourceReady(ctx, d, "rerecognize"); err != nil { return err } ctx = w.scoped(ctx, capReview, id, d.PrimaryInfohash()) @@ -388,7 +394,7 @@ func (w *Worker) Refine(ctx context.Context, id string, hint string) error { if err != nil { return err } - if err := w.ensureSourcePresent(ctx, d, "refine"); err != nil { + if err := w.ensureSourceReady(ctx, d, "refine"); err != nil { return err } ctx = w.scoped(ctx, capReview, id, d.PrimaryInfohash()) @@ -413,7 +419,7 @@ func (w *Worker) SetType(ctx context.Context, id string, mediaType string) error if err != nil { return err } - if err := w.ensureSourcePresent(ctx, d, "set type"); err != nil { + if err := w.ensureSourceReady(ctx, d, "set type"); err != nil { return err } ctx = w.scoped(ctx, capReview, id, d.PrimaryInfohash()) diff --git a/internal/worker/review_test.go b/internal/worker/review_test.go index 87613f6..b1d56d2 100644 --- a/internal/worker/review_test.go +++ b/internal/worker/review_test.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "encoding/json" + "errors" "fmt" "io" "log/slog" @@ -107,7 +108,7 @@ func revertedDownload(id string) *store.Download { func TestRelink_RevertedToRecognizing(t *testing.T) { st := newMemStore() st.put(revertedDownload("1")) - qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, Name: "Show", SavePath: "/d"}}} + qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, Name: "Show", SavePath: "/d", State: "uploading"}}} w := testWorkerWith(st, qb, &fakeRecognizer{result: seriesResult()}, nil) if err := w.Relink(context.Background(), "1"); err != nil { @@ -126,7 +127,7 @@ func TestRelink_CancelledToRecognizing(t *testing.T) { d := revertedDownload("1") d.State = store.StateCancelled st.put(d) - qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, Name: "Show", SavePath: "/d"}}} + qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, Name: "Show", SavePath: "/d", State: "uploading"}}} w := testWorkerWith(st, qb, &fakeRecognizer{result: seriesResult()}, nil) if err := w.Relink(context.Background(), "1"); err != nil { @@ -156,7 +157,7 @@ func TestRerecognize_ReviewToRecognizing(t *testing.T) { d := completedDownload("1") d.State = store.StateReview st.put(d) - qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest}}} + qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, State: "uploading"}}} w := testWorkerWith(st, qb, &fakeRecognizer{}, nil) if err := w.Rerecognize(context.Background(), "1"); err != nil { @@ -219,6 +220,87 @@ func TestRelink_ForceReviewSkipsAuto(t *testing.T) { } } +// TestReviewCommands_RejectNotReadySource — readiness-preflight: команды, +// вводящие задачу в recognizing, отклоняют недокачанный (downloading-класс) +// источник с ErrNotReady и НЕ трогают состояние. Иначе недокачанную задачу +// можно было бы провести в авто-раскладку и захардлинкать неполные файлы. +func TestReviewCommands_RejectNotReadySource(t *testing.T) { + notReady := []qbt.Torrent{{Hash: ihTest, Name: "Show", SavePath: "/d", State: "downloading"}} + + // review/deferred-команды на задаче в deferred/review. + cases := []struct { + name string + state store.State + call func(w *Worker) error + }{ + {"rerecognize", store.StateDeferred, func(w *Worker) error { return w.Rerecognize(context.Background(), "1") }}, + {"refine", store.StateReview, func(w *Worker) error { return w.Refine(context.Background(), "1", "подсказка") }}, + {"set type", store.StateReview, func(w *Worker) error { return w.SetType(context.Background(), "1", "series") }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + st := newMemStore() + d := completedDownload("1") + d.State = tc.state + st.put(d) + w := testWorkerWith(st, &fakeQbt{torrents: notReady}, &fakeRecognizer{}, nil) + + if err := tc.call(w); !errors.Is(err, ErrNotReady) { + t.Fatalf("err = %v, want ErrNotReady", err) + } + if got := st.downloads["1"].State; got != tc.state { + t.Errorf("state = %q, want %q (не тронуто)", got, tc.state) + } + }) + } + + // Relink из reverted — тоже требует готовности источника. + t.Run("relink", func(t *testing.T) { + st := newMemStore() + st.put(revertedDownload("1")) + w := testWorkerWith(st, &fakeQbt{torrents: notReady}, &fakeRecognizer{}, nil) + + if err := w.Relink(context.Background(), "1"); !errors.Is(err, ErrNotReady) { + t.Fatalf("err = %v, want ErrNotReady", err) + } + if got := st.downloads["1"].State; got != store.StateReverted { + t.Errorf("state = %q, want reverted (не тронуто)", got) + } + if _, ok := st.overrides["1"][ovrForceReview]; ok { + t.Errorf("force_review проставлен, а не должен: отказ до записи override") + } + }) +} + +// TestApply_RejectsNotReadySource — последний рубеж: Apply не создаёт хардлинки +// на недокачанный источник, даже если задача оказалась в review. +func TestApply_RejectsNotReadySource(t *testing.T) { + st := newMemStore() + d := completedDownload("1") + d.State = store.StateReview + st.put(d) + planJSON, _ := json.Marshal(seriesResult().Plan) + st.recs = append(st.recs, &store.Recognition{ + ID: "1", DownloadID: "1", IsCurrent: true, Plan: store.NullString(string(planJSON)), + }) + lay, err := layout.New(layout.Config{MoviesDir: t.TempDir(), SeriesDir: t.TempDir()}, nil) + if err != nil { + t.Fatal(err) + } + qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, SavePath: "/d", State: "downloading"}}} + w := testWorkerWith(st, qb, &fakeRecognizer{}, lay) + + if err := w.Apply(context.Background(), "1"); !errors.Is(err, ErrNotReady) { + t.Fatalf("err = %v, want ErrNotReady", err) + } + if got := st.downloads["1"].State; got != store.StateReview { + t.Errorf("state = %q, want review (не тронуто)", got) + } + if len(st.links) != 0 { + t.Errorf("file_links = %d, want 0 (хардлинки не создаём)", len(st.links)) + } +} + // memStore — полноценный in-memory store для тестов Ф3. type memStore struct { downloads map[string]*store.Download @@ -691,7 +773,7 @@ func TestRefine_AddsHintAndRerecognizes(t *testing.T) { d := completedDownload("1") d.State = store.StateReview st.put(d) - qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest}}} + qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, State: "uploading"}}} w := testWorkerWith(st, qb, &fakeRecognizer{}, nil) if err := w.Refine(context.Background(), "1", "это второй сезон"); err != nil { @@ -713,7 +795,7 @@ func TestSetType(t *testing.T) { d := completedDownload("1") d.State = store.StateReview st.put(d) - qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest}}} + qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, State: "uploading"}}} w := testWorkerWith(st, qb, &fakeRecognizer{}, nil) if err := w.SetType(context.Background(), "1", "series"); err != nil { @@ -807,7 +889,7 @@ func newApplyFixture(t *testing.T, plan recognize.Plan) applyFixture { st.recs = append(st.recs, &store.Recognition{ ID: "1", DownloadID: "1", IsCurrent: true, Plan: store.NullString(string(planJSON)), }) - qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, SavePath: downloads, Category: "jellybit"}}} + qb := &fakeQbt{torrents: []qbt.Torrent{{Hash: ihTest, SavePath: downloads, Category: "jellybit", State: "uploading"}}} w := testWorkerWith(st, qb, &fakeRecognizer{}, lay) return applyFixture{w: w, st: st, downloads: downloads, movies: movies, series: series} diff --git a/openspec/changes/archive/2026-07-08-review-readiness-preflight/.openspec.yaml b/openspec/changes/archive/2026-07-08-review-readiness-preflight/.openspec.yaml new file mode 100644 index 0000000..8cceb8d --- /dev/null +++ b/openspec/changes/archive/2026-07-08-review-readiness-preflight/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-08 diff --git a/openspec/changes/archive/2026-07-08-review-readiness-preflight/design.md b/openspec/changes/archive/2026-07-08-review-readiness-preflight/design.md new file mode 100644 index 0000000..cc34d3f --- /dev/null +++ b/openspec/changes/archive/2026-07-08-review-readiness-preflight/design.md @@ -0,0 +1,107 @@ +## Context + +Preflight-проверки ревью не доверяют состоянию в БД и синхронно сверяют +источник с qBittorrent прямо перед действием. Сейчас эта сверка — только на +**присутствие** (`ensureSourcePresent` → `torrentByInfohash`, результат +торрента отбрасывается). Класс состояния торрента (`classify`) определяется +лишь в поллинге (`downloading → completed`) и в recovery. Дыра: между этими +слоями команды ревью могут ввести недокачанную задачу в `recognizing` (откуда +finishRecognition делает авто-раскладку при `Decision.Auto && !force_review`) +или прямо в `linking` (Apply) — и создать хардлинки на неполные файлы. + +`classify(state) == classReady` уже есть (`internal/worker/worker.go`) и +используется в `worker.go`/`reconcile.go`. `ErrConflict` — доменная ошибка, +транслируемая транспортами (HTTP 409, сообщение в UI/Telegram). + +## Goals / Non-Goals + +**Goals:** +- Ни одна команда ревью не может привести к хардлинкам на недокачанные файлы. +- Единый, легко формулируемый инвариант: команда, вводящая задачу в активную + обработку или раскладку, работает только с готовым источником. +- Отказ недокачанного источника не разрушает легитимное состояние задачи. + +**Non-Goals:** +- Менять поллинг/переходы `download-tracking` — путь `downloading → completed` + уже корректен, дыра только в ручных командах re-entry. +- Гарантировать готовность на весь горизонт распознавания (см. Risks — гонка + «проверили → LLM думает»); монотонность прогресса делает риск пренебрежимым, + а Apply-гейт закрывает финальный рубеж. + +## Decisions + +### D1. `ensureSourceReady` заменяет `ensureSourcePresent`, тот же контракт отказа + +Новый метод `ensureSourceReady(ctx, d, op)` в `reconcile.go`: берёт торрент +через `torrentByInfohash`; если не найден — `reconcileToReality(false)` + +`ErrConflict «источник удалён из qBittorrent»` (как сейчас); если найден, но +`classify(t.State) != classReady` — `ErrNotReady «op: торрент ещё качается»` +**без** `reconcileToReality` (см. D4 про отдельный sentinel). + +Единая точка держит инвариант в одном месте и одинаково сообщает причину для +всех команд. Все четыре вызова `ensureSourcePresent` в review.go заменяются на +ready-вариант, других вызовов нет — старый метод становится мёртвым и +**удаляется** (не оставляем неиспользуемый путь). + +### D2. Не звать `reconcileToReality`, когда источник есть, но не готов + +`reconcileToReality` выводит состояние из (`sourcePresent`, `targetPresent`) — +для «источник есть, качается» правильного целевого состояния нет: задача +легитимно в `deferred`/`review`. Приводить нечего — просто отказываем. Это +отличает «ещё качается» (временный отказ, задача не трогается) от «источник +исчез» (сверка к `orphaned`/`deleted`). + +### D3. Apply гейтит готовность inline, не через `ensureSourceReady` + +`Apply` уже берёт торрент сам (`torrentByInfohash` для `SavePath`) и при +отсутствии зовёт `reconcileToReality`. Добавляем проверку класса на уже +полученном торренте (`classify(t.State) != classReady → ErrNotReady «торрент +ещё качается»`), не делая второй запрос к qBittorrent. Так Apply — последний +рубеж перед `linkPlan`, даже если задача пришла в `review` иным путём. + +### D4. Отдельный sentinel `ErrNotReady` с конкретным сообщением + +Причина «торрент ещё качается» actionable (жди докачки) и не совпадает по +смыслу с обычным конфликтом состояния, поэтому не прячем её за генерик +`ErrConflict`. Вводим отдельный `worker.ErrNotReady` (409, как и `ErrConflict`, +но со своим текстом) — по образцу уже существующих `errManualSource`/ +`errInvalidCandidate`, чьи `.Error()` показываются пользователю. `ensureSourceReady` +и inline-проверка в `Apply` оборачивают им отказ: +`fmt.Errorf("%s: торрент ещё качается: %w", op, ErrNotReady)`. + +Трансляция в транспортах: +- **HTTP/htmx** (`internal/httpapi`): в `classifyErr` добавить кейс + `errors.Is(err, worker.ErrNotReady) → 409, «торрент ещё качается, дождитесь + докачки»` **выше** кейса `ErrConflict` (иначе, если сделать их + `errors.Is`-совместимыми, перехватит первый; делаем sentinel независимым от + `ErrConflict`, порядок кейсов роли тогда не играет, но держим явным). +- **Telegram** (`internal/tgbot`): в обработчике callback-действий добавить + ветку `errors.Is(err, worker.ErrNotReady)` → сообщение «Торрент ещё + качается…», иначе прежний генерик `opErr(...)`. Сырой `err.Error()` наружу + по-прежнему не отдаём — показываем фиксированный текст. + +`ErrNotReady` — **не** `errors.Is`-обёртка над `ErrConflict` (отдельная +sentinel-переменная): статус тот же (409), но текст различается, а смешение +усложнило бы `classifyErr`. + +## Risks / Trade-offs + +- **Гонка «проверили готовность → LLM распознаёт → авто-раскладка».** Между + ready-проверкой в команде и `finishRecognition` проходит вызов LLM. → + Прогресс докачки монотонен: готовый торрент готовым и остаётся (кроме редкой + перепроверки `checkingUP`, которая классифицируется как `classBusy` → не + ready, и раскладка просто не сматчится по путям). Финальный Apply-гейт (D3) и + сам факт, что авто-раскладка требует `Decision.Auto`, делают остаточный риск + пренебрежимым. +- **Строже к `Relink`, чем требовала задача.** Relink на недокачанном источнике + теперь отклоняется сразу, а не доходит до review. → На практике кандидаты + Relink (`reverted`/`cancelled`/`target_missing`) — уже завершённые раздачи, + гейт почти никогда не срабатывает; ранний отказ понятнее пользователю, чем + блок на последующем Apply. +- **qBittorrent недоступен.** Как и `ensureSourcePresent`: честный отказ + операции (ошибка проброшена), состояние не трогаем. + +## Migration Plan + +Чисто внутреннее ужесточение preflight, без миграций БД и изменения API/схем. +Деплой — обычная замена бинаря. Откат — возврат бинаря; данные не затрагиваются. diff --git a/openspec/changes/archive/2026-07-08-review-readiness-preflight/proposal.md b/openspec/changes/archive/2026-07-08-review-readiness-preflight/proposal.md new file mode 100644 index 0000000..57b7702 --- /dev/null +++ b/openspec/changes/archive/2026-07-08-review-readiness-preflight/proposal.md @@ -0,0 +1,56 @@ +## Why + +Команды ревью, которым нужен источник, проверяют лишь **наличие** раздачи в +qBittorrent (`ensureSourcePresent`), но не её **готовность** (файлы докачаны). +Недокачанную задачу можно легально припарковать в `deferred` (граф допускает +`*→deferred`), а затем «Распознать заново»: она уходит в `recognizing`, LLM +видит имена ещё не докачанных файлов (qBittorrent отдаёт их до завершения) и при +уверенном матче срабатывает авто-раскладка (Rerecognize/Refine/SetType не ставят +`force_review`) — хардлинки создаются на **неполные файлы**, задача уходит в +`done`, Jellyfin сканирует половину. Даже ручной `review → Применить` не имеет +preflight завершённости. Это обходит смысл состояния `completed` +(«готовность только когда файлы на месте») и портит целостность медиатеки — +самый опасный из выявленных дефектов жизненного цикла. + +## What Changes + +- Ввести синхронный preflight **готовности** источника `ensureSourceReady`: + источник должен не только присутствовать, но и быть в готовом к раскладке + классе (`classify(t.State) == classReady`). +- Заменить `ensureSourcePresent` на `ensureSourceReady` во **всех** командах + ревью, которым нужен источник: `Rerecognize`, `Refine`, `SetType`, `Relink`. + Единый инвариант — команда, вводящая задачу в активную обработку + (`recognizing`) или в раскладку, работает только с готовым источником. +- Добавить ту же проверку готовности в `Apply` (сейчас он берёт торрент + inline и не смотрит на его класс) — последний рубеж перед созданием + хардлинков. +- Недокачанный источник → отказ отдельным sentinel `ErrNotReady` (409) с + actionable-сообщением «торрент ещё качается», **без** `reconcileToReality`: + состояние `deferred`/`review`/… легитимно, приводить к реальности нечего — + просто не даём действовать. Транспорты (HTTP/htmx и Telegram) показывают + конкретный текст, а не генерик «действие недоступно». + +## Capabilities + +### New Capabilities + + +### Modified Capabilities +- `review`: команды ревью, которым нужен источник, SHALL проверять его + **готовность** (файлы докачаны), а не только наличие; недокачанный источник + → отказ без разрушающих действий. + +## Impact + +- Код: `internal/worker/errors.go` (новый sentinel `ErrNotReady`), + `internal/worker/reconcile.go` (новый `ensureSourceReady`, удаление + `ensureSourcePresent`), `internal/worker/review.go` (замена вызовов в + `Rerecognize`/`Refine`/`SetType`/`Relink`, добавление проверки в `Apply`). +- Транспорты: `internal/httpapi` (`classifyErr` — кейс `ErrNotReady` → 409 с + текстом «торрент ещё качается»), `internal/tgbot` (ветка `ErrNotReady` в + обработчике действий). Пользователь видит конкретную причину вместо тихой + авто-раскладки неполных файлов; трансляция — на внешней границе. +- Тесты: `internal/worker/review_test.go` — сценарии недокачанного источника + для затронутых команд. +- Данные: устраняет создание хардлинков на неполные файлы (целостность + медиатеки Jellyfin). diff --git a/openspec/changes/archive/2026-07-08-review-readiness-preflight/specs/review/spec.md b/openspec/changes/archive/2026-07-08-review-readiness-preflight/specs/review/spec.md new file mode 100644 index 0000000..b457b7c --- /dev/null +++ b/openspec/changes/archive/2026-07-08-review-readiness-preflight/specs/review/spec.md @@ -0,0 +1,67 @@ +## MODIFIED Requirements + +### Requirement: Команды ревью и их эффекты + +Экран ревью SHALL предоставлять команды: **Применить** (создать хардлинки по +эффективному плану), **Уточнить** (добавить подсказку → перераспознать), +**Распознать заново** (повторный прогон без новой подсказки), **Игнор файла**, +**Позже** (`deferred`), **Отклонить** (`cancelled`), **Undo** (снять созданные +ссылки → `reverted`) и **Привязать заново** (из +`reverted`/`cancelled`/`target_missing` → перераспознавание с ручным +подтверждением). Экран ревью MUST NOT содержать команду переключения типа +movie↔series: тип показывается read-only, а его корректировка выполняется +мягкой подсказкой через **Уточнить**. Команды из любого транспорта SHALL +сериализоваться worker'ом под единой блокировкой; применяется последняя валидная +команда. + +Команды, которым нужен источник (**Применить**, **Уточнить**, **Распознать +заново**, **Привязать заново**, а также фиксация типа), SHALL синхронно (без +дебаунса) проверять перед действием, что источник не только присутствует в +qBittorrent, но и **готов к раскладке** — раздача в готовом классе состояния +(`uploading`/`stalledUP`/`pausedUP`/… с учётом различий имён qBit v4/v5), +т.е. файлы докачаны. Если источник ещё качается (любое `downloading`-подобное +или переходное `moving`/`checking` состояние), команда SHALL отказывать с +конфликтом и причиной «торрент ещё качается», НЕ создавая хардлинки и НЕ меняя +состояние загрузки (её нахождение в `review`/`deferred`/… легитимно, приводить +к реальности нечего). Отсутствие источника в qBittorrent SHALL по-прежнему +приводить состояние к реальности (`orphaned`/`deleted`) и отказывать. Так +недокачанная задача не может пройти через перераспознавание в авто-раскладку +или ручное применение и захардлинкать неполные файлы, обойдя финальность +состояния `completed`. + +#### Scenario: Применение создаёт раскладку + +- **GIVEN** загрузка в `review` с эффективным планом +- **WHEN** пользователь выбирает «Применить» +- **THEN** создаются хардлинки по плану, задача переходит к раскладке + +#### Scenario: Отклонить и привязать заново + +- **GIVEN** загрузка в `review` +- **WHEN** пользователь «Отклонить», затем «Привязать заново» +- **THEN** задача уходит в `cancelled`, а затем снова на распознавание с ручным + подтверждением (авто-раскладка не делается) + +#### Scenario: Тип не переключается кнопкой + +- **GIVEN** загрузка в `review` с распознанным типом +- **WHEN** пользователь открывает экран ревью +- **THEN** отдельной команды/кнопки переключения movie↔series на экране нет +- **AND** тип показан read-only в инфо-части выбранного источника + +#### Scenario: Недокачанный источник отклоняет перераспознавание + +- **GIVEN** загрузка припаркована в `deferred`, а её раздача в qBittorrent ещё + качается (`downloading`, файлы не докачаны) +- **WHEN** пользователь выбирает «Распознать заново» (или «Уточнить»/«Привязать + заново»/фиксацию типа) +- **THEN** команда отклоняется с конфликтом и причиной «торрент ещё качается» +- **AND** загрузка остаётся в `deferred`, хардлинки не создаются, авто-раскладка + не запускается + +#### Scenario: Недокачанный источник отклоняет ручное применение + +- **GIVEN** загрузка в `review`, чья раздача в qBittorrent ещё качается +- **WHEN** пользователь выбирает «Применить» +- **THEN** команда отклоняется с конфликтом «торрент ещё качается», хардлинки + на неполные файлы не создаются, состояние загрузки не меняется diff --git a/openspec/changes/archive/2026-07-08-review-readiness-preflight/tasks.md b/openspec/changes/archive/2026-07-08-review-readiness-preflight/tasks.md new file mode 100644 index 0000000..1288758 --- /dev/null +++ b/openspec/changes/archive/2026-07-08-review-readiness-preflight/tasks.md @@ -0,0 +1,45 @@ +## 1. Preflight готовности источника + +- [x] 1.1 В `internal/worker/errors.go` добавить sentinel + `var ErrNotReady = errors.New(...)` (отдельный от `ErrConflict`). +- [x] 1.2 В `internal/worker/reconcile.go` добавить `ensureSourceReady(ctx, d, op)`: + найти торрент через `torrentByInfohash`; нет источника → + `reconcileToReality(false)` + `ErrConflict «источник удалён из qBittorrent»`; + есть, но `classify(t.State) != classReady` → `ErrNotReady «op: торрент ещё + качается»` без `reconcileToReality`. +- [x] 1.3 Заменить `ensureSourcePresent` на `ensureSourceReady` в командах + `Rerecognize`, `Refine`, `SetType`, `Relink` (`internal/worker/review.go`); + удалить осиротевший `ensureSourcePresent`. +- [x] 1.4 В `Apply` добавить проверку класса на уже полученном торренте + (`classify(t.State) != classReady → ErrNotReady «торрент ещё качается»`), + без второго запроса к qBittorrent. + +## 2. Трансляция ошибки в транспортах + +- [x] 2.1 `internal/httpapi` `classifyErr`: кейс + `errors.Is(err, worker.ErrNotReady) → 409, «торрент ещё качается, дождитесь + докачки»` (отдельно от генерик-`ErrConflict`). +- [x] 2.2 `internal/tgbot`: в обработчике callback-действий ветка + `errors.Is(err, worker.ErrNotReady)` → сообщение «Торрент ещё качается…», + иначе прежний `opErr(...)`. + +## 3. Тесты + +- [x] 3.1 Тест: недокачанный источник (`downloading`-класс) отклоняет + `Rerecognize`/`Refine`/`SetType`/`Relink` с `ErrNotReady`, состояние задачи + не меняется, авто-раскладка не запускается. +- [x] 3.2 Тест: недокачанный источник отклоняет `Apply` — хардлинки не + создаются, состояние не меняется. +- [x] 3.3 Тест: готовый источник (`classReady`) пропускает те же команды как + раньше (регресс не сломан); отсутствие источника по-прежнему приводит к + реальности и отказывает. +- [x] 3.4 Тест транспорта: `classifyErr(ErrNotReady) == 409` с конкретным + текстом (`internal/httpapi`). + +## 4. Ревью и сверка + +- [x] 4.1 `task test` и `task lint` зелёные. +- [x] 4.2 Ревью кода (второй чекпоинт) перед archive. +- [x] 4.3 `openspec validate review-readiness-preflight --strict` зелёный; + удалить `docs/backlog/review-major5-readiness-preflight.md` из беклога и + строку из индекса (суть переехала в спеку). diff --git a/openspec/specs/review/spec.md b/openspec/specs/review/spec.md index bc1b42e..1b5a62f 100644 --- a/openspec/specs/review/spec.md +++ b/openspec/specs/review/spec.md @@ -37,8 +37,22 @@ LLM; нет матча в базе или несколько кандидато movie↔series: тип показывается read-only, а его корректировка выполняется мягкой подсказкой через **Уточнить**. Команды из любого транспорта SHALL сериализоваться worker'ом под единой блокировкой; применяется последняя валидная -команда. Команды, которым нужен источник, SHALL проверять его наличие синхронно -перед действием. +команда. + +Команды, которым нужен источник (**Применить**, **Уточнить**, **Распознать +заново**, **Привязать заново**, а также фиксация типа), SHALL синхронно (без +дебаунса) проверять перед действием, что источник не только присутствует в +qBittorrent, но и **готов к раскладке** — раздача в готовом классе состояния +(`uploading`/`stalledUP`/`pausedUP`/… с учётом различий имён qBit v4/v5), +т.е. файлы докачаны. Если источник ещё качается (любое `downloading`-подобное +или переходное `moving`/`checking` состояние), команда SHALL отказывать с +конфликтом и причиной «торрент ещё качается», НЕ создавая хардлинки и НЕ меняя +состояние загрузки (её нахождение в `review`/`deferred`/… легитимно, приводить +к реальности нечего). Отсутствие источника в qBittorrent SHALL по-прежнему +приводить состояние к реальности (`orphaned`/`deleted`) и отказывать. Так +недокачанная задача не может пройти через перераспознавание в авто-раскладку +или ручное применение и захардлинкать неполные файлы, обойдя финальность +состояния `completed`. #### Scenario: Применение создаёт раскладку @@ -60,6 +74,23 @@ movie↔series: тип показывается read-only, а его корре - **THEN** отдельной команды/кнопки переключения movie↔series на экране нет - **AND** тип показан read-only в инфо-части выбранного источника +#### Scenario: Недокачанный источник отклоняет перераспознавание + +- **GIVEN** загрузка припаркована в `deferred`, а её раздача в qBittorrent ещё + качается (`downloading`, файлы не докачаны) +- **WHEN** пользователь выбирает «Распознать заново» (или «Уточнить»/«Привязать + заново»/фиксацию типа) +- **THEN** команда отклоняется с конфликтом и причиной «торрент ещё качается» +- **AND** загрузка остаётся в `deferred`, хардлинки не создаются, авто-раскладка + не запускается + +#### Scenario: Недокачанный источник отклоняет ручное применение + +- **GIVEN** загрузка в `review`, чья раздача в qBittorrent ещё качается +- **WHEN** пользователь выбирает «Применить» +- **THEN** команда отклоняется с конфликтом «торрент ещё качается», хардлинки + на неполные файлы не создаются, состояние загрузки не меняется + ### Requirement: Подсказка мягкая, override жёсткий Подсказка (`hint`) SHALL быть мягким сигналом — её интерпретирует LLM при