From 90fd8640ed5aa08d4d2ad57afadf2f1cd368bcfe Mon Sep 17 00:00:00 2001 From: Anton Vakhrushev Date: Wed, 8 Jul 2026 09:09:17 +0300 Subject: [PATCH] =?UTF-8?q?=D0=9C=D0=B0=D1=88=D0=B8=D0=BD=D0=B0=20=D1=81?= =?UTF-8?q?=D0=BE=D1=81=D1=82=D0=BE=D1=8F=D0=BD=D0=B8=D0=B9:=20=D0=B4?= =?UTF-8?q?=D0=B5=D0=BA=D0=BB=D0=B0=D1=80=D0=B0=D1=82=D0=B8=D0=B2=D0=BD?= =?UTF-8?q?=D1=8B=D0=B9=20=D0=B3=D1=80=D0=B0=D1=84=20=D0=BB=D0=B5=D0=B3?= =?UTF-8?q?=D0=B0=D0=BB=D1=8C=D0=BD=D1=8B=D1=85=20=D0=BF=D0=B5=D1=80=D0=B5?= =?UTF-8?q?=D1=85=D0=BE=D0=B4=D0=BE=D0=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Единый источник истины `allowedTransitions` (from → {разрешённые to}) в internal/store; `setState` сверяет переход дополнительным SQL-предикатом `state IN (<легальные источники>)` — необъявленное ребро (и не самопереход) отклоняется атомарно, с точным сообщением. Гейт ортогонален гарду терминальности: ребро из терминального состояния проходит только через ActivateIfNoOtherActive. Без внешней библиотеки-FSM (обоснование — design.md). Граф выведен построчно из воркера; ревью дизайна поймало 8 preflight-рёбер (reconcileToReality → orphaned/deleted) и linking→cancel/defer после краха. Тест-инвариант «cancel/defer достижимы из любого не-терминального» ловит класс пропущенного ребра. Фикстуры тестов, форсившие состояния через SetDownloadState, переведены на прямой UPDATE (forceState). Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/store/download.go | 113 ++++++++- internal/store/download_test.go | 35 +-- internal/store/list_test.go | 4 +- internal/store/transition_test.go | 238 ++++++++++++++++++ .../.openspec.yaml | 2 + .../design.md | 144 +++++++++++ .../proposal.md | 48 ++++ .../specs/download-tracking/spec.md | 55 ++++ .../tasks.md | 50 ++++ openspec/specs/download-tracking/spec.md | 52 ++++ 10 files changed, 716 insertions(+), 25 deletions(-) create mode 100644 internal/store/transition_test.go create mode 100644 openspec/changes/archive/2026-07-08-state-transition-graph/.openspec.yaml create mode 100644 openspec/changes/archive/2026-07-08-state-transition-graph/design.md create mode 100644 openspec/changes/archive/2026-07-08-state-transition-graph/proposal.md create mode 100644 openspec/changes/archive/2026-07-08-state-transition-graph/specs/download-tracking/spec.md create mode 100644 openspec/changes/archive/2026-07-08-state-transition-graph/tasks.md diff --git a/internal/store/download.go b/internal/store/download.go index c0fad77..c763cea 100644 --- a/internal/store/download.go +++ b/internal/store/download.go @@ -64,6 +64,71 @@ func (s State) IsTerminal() bool { return slices.Contains(terminalStates, s) } +// allowedTransitions — декларативный граф легальных переходов машины состояний +// (from → множество допустимых to). Единственный источник истины о легальности +// рёбер: покрывает все переходы, которые worker выполняет по всем capability +// (прямой путь download-tracking, state-reconciliation, review). Выведен построчно +// из кода воркера — таблица в change `state-transition-graph`/design.md. +// +// Правила: +// - Самопереход (from == to, идемпотентная переустановка того же состояния — +// напр. повторная запись error_msg, Defer на уже deferred) разрешён ВСЕГДА и +// здесь НЕ перечисляется (его добавляет invertTransitions). +// - Ребро может присутствовать здесь, но всё равно требовать revive-путь +// (ActivateIfNoOtherActive): гейт графа ортогонален гарду терминальности в +// setState — граф говорит «ребро есть», гард «но не мимо ActivateIfNoOtherActive». +// Так, failed → downloading объявлено, но обычным SetDownloadState отклоняется. +// - cancelled/deferred — легальная цель из КАЖДОГО не-терминального состояния +// (Cancel/Defer проверяют лишь IsTerminal); инвариант закреплён тестом, а не +// ручной аккуратностью. +// +// Правка воркера, вводящая новое ребро, ОБЯЗАНА отразить его здесь — иначе +// setState отклонит переход (0 строк UPDATE → ошибка). +var allowedTransitions = map[State][]State{ + StateCatched: {StateDownloading, StateFailed, StateCancelled, StateDeferred}, + StateDownloading: {StateCompleted, StateFailed, StateStuck, StateCancelled, StateDeferred}, + StateCompleted: {StateRecognizing, StateCancelled, StateDeferred}, + StateRecognizing: {StateLinking, StateReview, StateCancelled, StateDeferred}, + StateReview: {StateLinking, StateRecognizing, StateCancelled, StateDeferred, StateOrphaned, StateDeleted}, + StateLinking: {StateDone, StateReview, StateFailed, StateCancelled, StateDeferred}, + StateDone: {StateReverted, StateTargetMissing, StateOrphaned, StateDeleted}, + StateDeferred: {StateLinking, StateRecognizing, StateCancelled, StateOrphaned, StateDeleted}, + StateStuck: {StateDownloading, StateCompleted, StateCancelled, StateDeferred}, + StateFailed: {StateDownloading, StateCompleted}, + StateReverted: {StateRecognizing, StateOrphaned, StateDeleted}, + StateCancelled: {StateRecognizing, StateOrphaned, StateDeleted}, + StateTargetMissing: {StateRecognizing, StateDone, StateOrphaned, StateDeleted}, + StateOrphaned: {StateDone, StateTargetMissing, StateDeleted}, + StateDeleted: nil, // окончательно терминально: сверка его не переоценивает +} + +// transitionSources — обратное отображение (to → множество легальных from), +// построенное из allowedTransitions один раз при инициализации пакета. Сам to +// всегда входит в своё множество (самопереход). Основа предиката setState +// `state IN (...)`. +var transitionSources = invertTransitions(allowedTransitions) + +// invertTransitions переворачивает граф from→to в to→from, добавляя каждому +// состоянию его самого (самопереход всегда легален). +func invertTransitions(fwd map[State][]State) map[State][]State { + into := make(map[State][]State, len(fwd)) + ensureSelf := func(s State) { + if !slices.Contains(into[s], s) { + into[s] = append(into[s], s) + } + } + for from, tos := range fwd { + ensureSelf(from) + for _, to := range tos { + ensureSelf(to) + if !slices.Contains(into[to], from) { + into[to] = append(into[to], from) + } + } + } + return into +} + // SourceType — вид источника загрузки. type SourceType string @@ -505,11 +570,13 @@ func (s *Store) SetDownloadState(ctx context.Context, id string, state State, er } // PromoteCatched переводит пойманную загрузку catched → downloading, попутно -// записывая выведенное отображаемое имя. Гард `state = 'catched'` — это -// ре-валидация: если загрузку успели отменить (catched → cancelled) во время -// вывода имени/добавления вне блокировки переходов, UPDATE не заденет ни строки -// и вернёт ошибку, а переход не применится. Пустое имя допустимо (rename не -// задавали) — тогда display_name так и остаётся пустым. +// записывая выведенное отображаемое имя. Ребро catched → downloading объявлено в +// allowedTransitions; setState этот путь не проходит намеренно — собственный гард +// `state = 'catched'` жёстче (фиксирует ровно from=catched) и служит ре-валидацией: +// если загрузку успели отменить (catched → cancelled) во время вывода имени/ +// добавления вне блокировки переходов, UPDATE не заденет ни строки и вернёт ошибку, +// а переход не применится. Пустое имя допустимо (rename не задавали) — тогда +// display_name так и остаётся пустым. func (s *Store) PromoteCatched(ctx context.Context, id, displayName string) error { res, err := s.DB.ExecContext(ctx, ` UPDATE download @@ -533,6 +600,12 @@ WHERE id = ? AND state = ?`, // (ActivateIfNoOtherActive), которому переход терминал→активное разрешён; // иначе предикат в UPDATE не даёт молча оживить терминальную задачу. func setState(ctx context.Context, e sqlx.ExecerContext, id string, state State, errCode, errMsg string, reviveOK bool) error { + sources := transitionSources[state] + if len(sources) == 0 { + // Целевое состояние не объявлено в графе переходов — fail-closed (это + // программная ошибка: новая State без ребра). Тест графа ловит на этапе CI. + return fmt.Errorf("set download %s state %q: target not declared in transition graph", id, state) + } q := ` UPDATE download SET state = ?, @@ -541,6 +614,10 @@ SET state = ?, updated_at = ? WHERE id = ?` args := []any{string(state), nullArg(errCode), nullArg(errMsg), FormatTime(Now()), id} + // Гейт графа: переход применяется, только если текущее состояние — легальный + // источник для state (объявленное ребро или самопереход from == to). Аддитивен + // к гарду терминальности ниже и его НЕ ослабляет. + q += ` AND state IN (` + placeholders(&args, sources) + `)` if !reviveOK && !state.IsTerminal() { q += ` AND state NOT IN (` + placeholders(&args, terminalStates) + `)` } @@ -553,11 +630,35 @@ WHERE id = ?` return fmt.Errorf("set download %s state %q: %w", id, state, err) } if n == 0 { - return fmt.Errorf("set download %s state %q: not found or terminal (revive requires ActivateIfNoOtherActive)", id, state) + return setStateRejected(ctx, e, id, state) } return nil } +// setStateRejected формирует точную ошибку отклонённого перехода (0 строк UPDATE): +// читает текущее состояние и различает «не найдено» / «нелегальное ребро» / +// «терминал без revive». Только путь ошибки (редкий), поэтому доп. чтение дёшево. +func setStateRejected(ctx context.Context, e sqlx.ExecerContext, id string, state State) error { + q, ok := e.(sqlx.QueryerContext) + if !ok { + return fmt.Errorf("set download %s state %q: rejected (not found, illegal transition, or terminal without revive)", id, state) + } + var cur State + err := sqlx.GetContext(ctx, q, &cur, `SELECT state FROM download WHERE id = ?`, id) + if errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("set download %s state %q: %w", id, state, ErrNotFound) + } + if err != nil { + return fmt.Errorf("set download %s state %q: rejected, current state unreadable: %w", id, state, err) + } + if slices.Contains(transitionSources[state], cur) { + // Ребро cur → state легально — значит зарубил гард терминальности. + return fmt.Errorf("set download %s: %s → %s rejected: terminal revive requires ActivateIfNoOtherActive", + id, cur, state) + } + return fmt.Errorf("set download %s: illegal transition %s → %s (not in transition graph)", id, cur, state) +} + // SetSourceMissCount записывает счётчик пропусков источника (дебаунс сверки). // Состояние не трогает — это отдельная от перехода фоновая отметка. func (s *Store) SetSourceMissCount(ctx context.Context, id string, n int) error { diff --git a/internal/store/download_test.go b/internal/store/download_test.go index 73ba909..351db02 100644 --- a/internal/store/download_test.go +++ b/internal/store/download_test.go @@ -100,6 +100,19 @@ func mustCreate(t *testing.T, st *Store, infohash string) string { return d.ID } +// forceState принудительно проставляет состояние прямым UPDATE в обход гейта +// графа переходов — для подготовки фикстур, где проверяется поведение в заданном +// состоянии, а не путь его достижения. Реальные переходы идут через +// SetDownloadState/ActivateIfNoOtherActive (их легальность проверяет +// transition_test.go). +func forceState(t *testing.T, st *Store, id string, state State) { + t.Helper() + if _, err := st.DB.ExecContext(context.Background(), + `UPDATE download SET state = ? WHERE id = ?`, string(state), id); err != nil { + t.Fatalf("force state %s: %v", state, err) + } +} + func TestCreateAndGetDownload(t *testing.T) { st := newTestStore(t) ctx := context.Background() @@ -166,9 +179,7 @@ func TestFindActiveByInfohash_DesyncStatesNotActive(t *testing.T) { const ih = "3333333333333333333333333333333333333333" id := mustCreate(t, store, ih) - if err := store.SetDownloadState(ctx, id, st, "", ""); err != nil { - t.Fatal(err) - } + forceState(t, store, id, st) if d, err := store.FindActiveByInfohash(ctx, ih); err != nil || d != nil { t.Fatalf("%s: активной задачи быть не должно, получили (%v,%v)", st, d, err) } @@ -291,9 +302,7 @@ func TestActivateIfNoOtherActive(t *testing.T) { } // Владелец завершился → активация проходит. - if err := st.SetDownloadState(ctx, id2, StateDone, "", ""); err != nil { - t.Fatal(err) - } + forceState(t, st, id2, StateDone) if err := st.ActivateIfNoOtherActive(ctx, id1, StateDownloading, "", ""); err != nil { t.Fatalf("активация после ухода владельца: %v", err) } @@ -354,9 +363,7 @@ func TestAddInfohashesGuard(t *testing.T) { } // Хеш терминального владельца дописывается свободно. - if err := st.SetDownloadState(ctx, a, StateDone, "", ""); err != nil { - t.Fatal(err) - } + forceState(t, st, a, StateDone) b := mustCreate(t, st, "eeee333333333333333333333333333333333333") if err := st.AddInfohashes(ctx, b, []string{h1}); err != nil { t.Fatalf("хеш терминальной задачи должен дописываться: %v", err) @@ -400,16 +407,14 @@ func TestSetDownloadStateRejectsRevive(t *testing.T) { if err := st.SetDownloadState(ctx, id, StateFailed, "x", ""); err != nil { t.Fatal(err) } + // Терминал→активное обычным SetDownloadState отклоняется (revive-гард): ребро + // failed → downloading в графе есть, но проходит только через гард владения. if err := st.SetDownloadState(ctx, id, StateDownloading, "", ""); err == nil { t.Fatal("терминал→активное мимо гарда должно отклоняться") } if d, _ := st.GetDownload(ctx, id); d.State != StateFailed { t.Fatalf("state = %s, want failed (без изменений)", d.State) } - // Терминал→терминал разрешён (например, сверка double-terminal переходов). - if err := st.SetDownloadState(ctx, id, StateDeleted, "", ""); err != nil { - t.Fatalf("терминал→терминал должен проходить: %v", err) - } // Штатный путь оживления работает. if err := st.ActivateIfNoOtherActive(ctx, id, StateDownloading, "", ""); err != nil { t.Fatalf("оживление через гард: %v", err) @@ -472,9 +477,7 @@ func TestExistsByInfohash(t *testing.T) { t.Fatalf("ожидался (false,nil), получили (%v,%v)", ok, err) } id := mustCreate(t, st, ih) - if err := st.SetDownloadState(ctx, id, StateDone, "", ""); err != nil { - t.Fatal(err) - } + forceState(t, st, id, StateDone) // Exists видит и терминальные (в отличие от FindActive). if ok, err := st.ExistsByInfohash(ctx, ih); err != nil || !ok { t.Fatalf("ожидался (true,nil), получили (%v,%v)", ok, err) diff --git a/internal/store/list_test.go b/internal/store/list_test.go index f93fb69..40cc066 100644 --- a/internal/store/list_test.go +++ b/internal/store/list_test.go @@ -24,9 +24,7 @@ func mkDownload(t *testing.T, st *Store, n int, state State, display string) str t.Fatalf("create #%d: unexpected dedup", n) } if state != StateDownloading { - if err := st.SetDownloadState(ctx, d.ID, state, "", ""); err != nil { - t.Fatalf("set state #%d: %v", n, err) - } + forceState(t, st, d.ID, state) } return d.ID } diff --git a/internal/store/transition_test.go b/internal/store/transition_test.go new file mode 100644 index 0000000..c32f6cf --- /dev/null +++ b/internal/store/transition_test.go @@ -0,0 +1,238 @@ +package store + +import ( + "context" + "errors" + "slices" + "strconv" + "strings" + "testing" +) + +// allStates — полный перечень состояний машины (для проверки хорошей +// сформированности графа). Держим локально в тесте: если добавится новое +// состояние, тест напомнит внести его сюда и в граф. +var allStates = []State{ + StateCatched, StateDownloading, StateCompleted, StateRecognizing, + StateReview, StateLinking, StateDone, StateDeferred, StateStuck, + StateFailed, StateCancelled, StateReverted, + StateTargetMissing, StateOrphaned, StateDeleted, +} + +// seedState заводит загрузку и приводит её к нужному состоянию кратчайшим +// доверенным путём (в обход гейта графа — через прямой UPDATE), чтобы тест +// проверял именно проверяемый переход, а не путь подготовки. +func seedState(t *testing.T, st *Store, ih string, state State) string { + t.Helper() + ctx := context.Background() + d := &Download{SourceType: SourceMagnet, SourceRef: "magnet:?xt=urn:btih:" + ih, State: StateCatched} + if _, err := st.CreateDownloadIfNoActive(ctx, d, []string{ih}); err != nil { + t.Fatalf("seed create: %v", err) + } + if state != StateCatched { + if _, err := st.DB.ExecContext(ctx, + `UPDATE download SET state = ? WHERE id = ?`, string(state), d.ID); err != nil { + t.Fatalf("seed set state %s: %v", state, err) + } + } + return d.ID +} + +func stateOf(t *testing.T, st *Store, id string) State { + t.Helper() + d, err := st.GetDownload(context.Background(), id) + if err != nil { + t.Fatalf("get: %v", err) + } + return d.State +} + +// Граф хорошо сформирован: все состояния из ключей и значений — известные, и +// ни один список не перечисляет сам ключ (петли не объявляются явно — +// самопереход добавляет invertTransitions). +func TestTransitionGraphWellFormed(t *testing.T) { + known := func(s State) bool { return slices.Contains(allStates, s) } + for from, tos := range allowedTransitions { + if !known(from) { + t.Errorf("неизвестное состояние-ключ: %q", from) + } + for _, to := range tos { + if !known(to) { + t.Errorf("%s → неизвестное состояние %q", from, to) + } + if to == from { + t.Errorf("%s: петля перечислена явно (самопереход неявен)", from) + } + } + } + // Каждое состояние присутствует в графе как ключ — иначе setState fail-closed + // отклонит переход в него. + for _, s := range allStates { + if _, ok := allowedTransitions[s]; !ok { + t.Errorf("состояние %q отсутствует среди ключей графа", s) + } + } +} + +// Инвариант generic-команд Cancel/Defer: cancelled и deferred — легальная цель +// из КАЖДОГО не-терминального состояния (кроме самого deferred для deferred — +// это самопереход). Ловит класс дыры «забыли состояние» (напр. linking после +// краха процесса). +func TestCancelDeferReachableFromEveryNonTerminal(t *testing.T) { + for _, s := range allStates { + if s.IsTerminal() { + continue + } + if !slices.Contains(transitionSources[StateCancelled], s) { + t.Errorf("%s → cancelled не легально (Cancel допускает любое не-терминальное)", s) + } + if s == StateDeferred { + continue // deferred → deferred покрыт самопереходом + } + if !slices.Contains(transitionSources[StateDeferred], s) { + t.Errorf("%s → deferred не легально (Defer допускает любое не-терминальное)", s) + } + } +} + +// Объявленные не-revive рёбра проходят через SetDownloadState. +func TestSetStateAllowsDeclaredEdges(t *testing.T) { + edges := []struct{ from, to State }{ + {StateDownloading, StateCompleted}, + {StateDownloading, StateStuck}, + {StateCompleted, StateRecognizing}, + {StateRecognizing, StateReview}, + {StateRecognizing, StateLinking}, + {StateReview, StateLinking}, + {StateReview, StateRecognizing}, + {StateLinking, StateDone}, + {StateLinking, StateReview}, + {StateDone, StateReverted}, + {StateStuck, StateCancelled}, + {StateReview, StateDeferred}, + } + for i, e := range edges { + st := newTestStore(t) + ih := infohashN(i) + id := seedState(t, st, ih, e.from) + if err := st.SetDownloadState(context.Background(), id, e.to, "", ""); err != nil { + t.Errorf("легальное ребро %s → %s отклонено: %v", e.from, e.to, err) + continue + } + if got := stateOf(t, st, id); got != e.to { + t.Errorf("%s → %s: состояние стало %q", e.from, e.to, got) + } + } +} + +// Preflight-рёбра reconcileToReality при пропавшем источнике: из ревью/ +// терминальных состояний в orphaned/deleted проходят через SetDownloadState +// (цель терминальна → гард терминальности не мешает). +func TestPreflightDesyncEdges(t *testing.T) { + edges := []struct{ from, to State }{ + {StateReview, StateOrphaned}, + {StateReview, StateDeleted}, + {StateDeferred, StateOrphaned}, + {StateDeferred, StateDeleted}, + {StateReverted, StateOrphaned}, + {StateReverted, StateDeleted}, + {StateCancelled, StateOrphaned}, + {StateCancelled, StateDeleted}, + } + for i, e := range edges { + st := newTestStore(t) + id := seedState(t, st, infohashN(i), e.from) + if err := st.SetDownloadState(context.Background(), id, e.to, "", ""); err != nil { + t.Errorf("preflight-ребро %s → %s отклонено: %v", e.from, e.to, err) + continue + } + if got := stateOf(t, st, id); got != e.to { + t.Errorf("%s → %s: состояние стало %q", e.from, e.to, got) + } + } +} + +// Необъявленные рёбра отклоняются, состояние не меняется. +func TestSetStateRejectsUndeclaredEdges(t *testing.T) { + edges := []struct{ from, to State }{ + {StateReview, StateDone}, + {StateDownloading, StateDone}, + {StateCompleted, StateLinking}, + {StateDownloading, StateReview}, + } + for i, e := range edges { + st := newTestStore(t) + id := seedState(t, st, infohashN(i), e.from) + err := st.SetDownloadState(context.Background(), id, e.to, "", "") + if err == nil { + t.Errorf("нелегальное ребро %s → %s прошло", e.from, e.to) + continue + } + if !strings.Contains(err.Error(), "illegal transition") { + t.Errorf("%s → %s: ожидалось 'illegal transition', got: %v", e.from, e.to, err) + } + if got := stateOf(t, st, id); got != e.from { + t.Errorf("%s → %s отклонён, но состояние стало %q", e.from, e.to, got) + } + } +} + +// Самопереход (идемпотентная переустановка того же состояния) разрешён. +func TestSelfTransitionAllowed(t *testing.T) { + st := newTestStore(t) + id := seedState(t, st, infohashN(0), StateDeferred) + if err := st.SetDownloadState(context.Background(), id, StateDeferred, "", ""); err != nil { + t.Fatalf("самопереход deferred → deferred отклонён: %v", err) + } + if got := stateOf(t, st, id); got != StateDeferred { + t.Errorf("состояние стало %q", got) + } +} + +// Ребро из терминального состояния в активное проходит только revive-путём +// (ActivateIfNoOtherActive), но не обычным SetDownloadState. +func TestTerminalReviveOnlyViaActivate(t *testing.T) { + ctx := context.Background() + + // SetDownloadState (не-revive): failed → downloading отклоняется гардом + // терминальности, ошибка указывает на revive. + st := newTestStore(t) + id := seedState(t, st, infohashN(0), StateFailed) + err := st.SetDownloadState(ctx, id, StateDownloading, "", "") + if err == nil { + t.Fatal("failed → downloading через SetDownloadState прошло") + } + if !strings.Contains(err.Error(), "terminal revive") { + t.Errorf("ожидалось 'terminal revive', got: %v", err) + } + if got := stateOf(t, st, id); got != StateFailed { + t.Errorf("состояние изменилось на %q", got) + } + + // ActivateIfNoOtherActive (revive): тот же переход при свободном infohash + // проходит. + st2 := newTestStore(t) + id2 := seedState(t, st2, infohashN(1), StateFailed) + if err := st2.ActivateIfNoOtherActive(ctx, id2, StateDownloading, "", ""); err != nil { + t.Fatalf("revive failed → downloading отклонён: %v", err) + } + if got := stateOf(t, st2, id2); got != StateDownloading { + t.Errorf("revive: состояние стало %q, want downloading", got) + } +} + +// Отсутствующая загрузка → ErrNotFound (гейт графа не маскирует «не найдено»). +func TestSetStateNotFound(t *testing.T) { + st := newTestStore(t) + err := st.SetDownloadState(context.Background(), "01hnonexistentnonexistent", StateCompleted, "", "") + if !errors.Is(err, ErrNotFound) { + t.Errorf("ожидался ErrNotFound, got: %v", err) + } +} + +// infohashN — детерминированный 40-hex инфохэш по индексу (уникальность между +// подтестами без общего состояния). Цифры и 'a'-паддинг — валидный hex. +func infohashN(n int) string { + s := strconv.Itoa(n) + return strings.Repeat("a", 40-len(s)) + s +} diff --git a/openspec/changes/archive/2026-07-08-state-transition-graph/.openspec.yaml b/openspec/changes/archive/2026-07-08-state-transition-graph/.openspec.yaml new file mode 100644 index 0000000..8cceb8d --- /dev/null +++ b/openspec/changes/archive/2026-07-08-state-transition-graph/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-08 diff --git a/openspec/changes/archive/2026-07-08-state-transition-graph/design.md b/openspec/changes/archive/2026-07-08-state-transition-graph/design.md new file mode 100644 index 0000000..5a98b3d --- /dev/null +++ b/openspec/changes/archive/2026-07-08-state-transition-graph/design.md @@ -0,0 +1,144 @@ +# Design: декларативный граф переходов + +## Почему не библиотека (looplab/fsm, qmuntal/stateless) + +Отклонено сознательно, три причины из нашего кода: + +1. **Тяжёлую часть библиотека не заберёт — она в SQL.** Настоящий инвариант («не + более одной активной загрузки на infohash» + «терминальную нельзя молча оживить») + держится атомарно в SQLite-транзакциях (`BEGIN IMMEDIATE` через `_txlock`): + `CreateDownloadIfNoActive`, `ActivateIfNoOtherActive`, гард `WHERE state='catched'` + в `PromoteCatched`. In-memory FSM физически не может участвовать в транзакции с БД — + она сядет поверх реального гарда как второй, более слабый слой. + +2. **У нас не событийный автомат, а reconciliation.** Воркер в основном *сверяет* + состояние БД с реальностью qBittorrent (`reconcile`, `reconcileDesync` — двумерная + матрица источник×цель, `reconcileRecovery`). FSM-библиотеки моделируют линейный + «событие → переход» хорошо, а сверку — плохо. + +3. **Состояние persisted, а не в объекте.** State — колонка SQLite, перечитывается + каждый тик. Библиотечная FSM держит состояние в структуре; пришлось бы + конструировать FSM-объект на загрузку на тик только ради валидации одного ребра. + +Соразмерная альтернатива — свой декларативный граф в `internal/store`: 80% ценности +(граф в одном месте, документирован, тестируем, роняет нелегальный переход) без +зависимости, без второго слоя, без импеданса. Соответствует принципу «минимум +компонентов». + +## Где живёт гейт: `store.setState` + +`setState` — истинная точка схождения: через неё проходят и `SetDownloadState` +(`reviveOK=false`), и `ActivateIfNoOtherActive` (`reviveOK=true`). Единственный +обход — `PromoteCatched` (собственный `UPDATE ... WHERE state='catched'`): его переход +`catched → downloading` объявлен ребром графа, а from-состояние жёстко фиксирует его +собственный гард, поэтому он согласован по построению и в `setState` не заводится. + +Гейт — **SQL-предикатом**, не Go-проверкой с предварительным чтением: + +``` +UPDATE download SET state=?, ... WHERE id=? + AND state IN (<легальные источники для to>) -- граф (+ сам to: самопереход) + AND state NOT IN () -- прежний гард, только если !reviveOK и to не терминально +``` + +Так проверка остаётся атомарной (без окна между чтением и записью), естественно +композится с существующими предикатами и не требует держать доп. блокировку. При +0 строк — одно диагностическое чтение текущего состояния (только на пути ошибки, +редко) даёт точное сообщение: «not found» / «illegal transition from→to». + +### Ортогональность (ключевой инвариант дизайна) + +Граф и гард терминальности **независимы и оба применяются**: + +- Граф говорит «ребро `failed → downloading` существует» (его выполняет retry). +- Гард терминальности говорит «но обычным `SetDownloadState` терминальную не оживить». + +Поэтому `failed → downloading` проходит только через `ActivateIfNoOtherActive` +(`reviveOK=true`, гард терминальности снят, но проверка владения хешами добавлена), а +`SetDownloadState(failed → downloading)` отклоняется. Гейт графа **аддитивен**: он +ничего не ослабляет, только добавляет ещё одно необходимое условие. + +## Самопереходы + +Правило: `from == to` разрешён всегда (в графе не перечисляется). Причина — +идемпотентная переустановка того же состояния уже используется (напр. `Defer` на уже +`deferred`-задаче; повторная запись `error_msg`). `legalSources(to)` всегда включает +сам `to`. Это сохраняет текущую семантику (такой UPDATE успешен, трогает +`error_*`/`updated_at`) и избавляет от ручного перечисления петель. + +## Граф (from → to), выведенный из реального кода + +Источник каждого ребра — конкретный метод воркера/store (`w.transition` → +`SetDownloadState`, `ActivateIfNoOtherActive`, `PromoteCatched`, `reconcileToReality`): + +| from | to | кто выполняет | +|------|----|----| +| `catched` | `downloading` | `PromoteCatched` (успех add) | +| `catched` | `failed` | `processCatched` таймаут (`qbit_add`) | +| `catched` | `cancelled` | `Cancel` | +| `catched` | `deferred` | `Defer` (любое не-терминальное) | +| `downloading` | `completed` | `reconcile` (classReady) | +| `downloading` | `failed` | `reconcile` (qbit_error), `checkTimeouts` (magnet_timeout) | +| `downloading` | `stuck` | `checkTimeouts` (stalled) | +| `downloading` | `cancelled` | `Cancel` | +| `downloading` | `deferred` | `Defer` | +| `completed` | `recognizing` | `recognizeOne` | +| `completed` | `cancelled` / `deferred` | `Cancel` / `Defer` | +| `recognizing` | `linking` | `finishRecognition` (авто) | +| `recognizing` | `review` | `finishRecognition` | +| `recognizing` | `cancelled` / `deferred` | `Cancel` / `Defer` (окно на время LLM) | +| `review` | `linking` | `Apply` | +| `review` | `recognizing` | `Refine` / `Rerecognize` / `SetType` | +| `review` | `deferred` / `cancelled` | `Defer` / `Cancel` | +| `review` | `orphaned` / `deleted` | `reconcileToReality` (preflight: источник пропал) | +| `linking` | `done` | `linkPlan` (успех) | +| `linking` | `review` | `linkPlan` (build/collision) | +| `linking` | `failed` | `linkPlan` (apply error) | +| `linking` | `cancelled` / `deferred` | `Cancel` / `Defer` (задача застряла в `linking` после краха процесса) | +| `done` | `reverted` | `Undo` | +| `done` | `target_missing` / `orphaned` / `deleted` | `reconcileDesync` | +| `deferred` | `linking` | `Apply` | +| `deferred` | `recognizing` | `Refine` / `Rerecognize` / `SetType` | +| `deferred` | `cancelled` | `Cancel` | +| `deferred` | `orphaned` / `deleted` | `reconcileToReality` (preflight: источник пропал) | +| `stuck` | `downloading` | `Retry`, `reconcileRecovery` | +| `stuck` | `completed` | `reconcileRecovery` (classReady) | +| `stuck` | `cancelled` / `deferred` | `Cancel` / `Defer` (stuck не терминально) | +| `failed` | `downloading` | `Retry`, `reconcileRecovery` | +| `failed` | `completed` | `reconcileRecovery` | +| `reverted` | `recognizing` | `Relink` | +| `reverted` | `orphaned` / `deleted` | `reconcileToReality` (Relink: источник пропал) | +| `cancelled` | `recognizing` | `Relink` | +| `cancelled` | `orphaned` / `deleted` | `reconcileToReality` (Relink: источник пропал) | +| `target_missing` | `recognizing` | `Relink` | +| `target_missing` | `done` / `orphaned` / `deleted` | `reconcileDesync` / `reconcileToReality` (heal/preflight) | +| `orphaned` | `done` / `target_missing` / `deleted` | `reconcileDesync` | +| `deleted` | — | окончательно терминально; сверка его не переоценивает | + +Правило `Cancel`/`Defer` — **любое не-терминальное → `cancelled`/`deferred`** (проверка +`IsTerminal` на входе). Поэтому в графе `cancelled` и `deferred` — легальная цель из +**каждого** не-терминального состояния (включая `linking` после краха, включая +`catched`); это закрепляется тест-инвариантом, а не ручной аккуратностью (см. `tasks.md` +3.6) — именно ручное перечисление рискует пропустить состояние. + +`reconcileToReality` (preflight-приведение к реальности перед действием ревью, когда +раздача исчезла из qBittorrent) — отдельный источник рёбер `→ orphaned/deleted` из +пользовательских/ревью-состояний; из терминальных `reverted/cancelled` он проходит +`SetDownloadState`, так как цель (`orphaned`/`deleted`) тоже терминальна и гард +терминальности не срабатывает. + +Заметки о намеренных исключениях (чтобы граф был тесным, а не «на всякий случай»): + +- `failed/done → cancelled/deferred` **не** включены: это терминальные состояния, + `Cancel`/`Defer` их отвергают на входе (`IsTerminal`). +- Рёбра из терминальных (`failed`, `reverted`, `cancelled`, `target_missing`, + `orphaned`) в активные состояния (`downloading`/`completed`/`recognizing`) в графе + есть, но проходят только через `ActivateIfNoOtherActive` — см. «Ортогональность». + +## Риск и его закрытие + +Главный риск — **слишком тесный граф** (пропущенное ребро ломает реально работающий +переход, у которого нет теста). Закрытие: (1) граф выведён построчно из кода выше; +(2) весь существующий набор тестов воркера/store гоняет реальные переходы — если +предикат отвергнёт хоть один, тесты покраснеют; (3) новый тест графа проверяет +объявленные рёбра на проход и репрезентативные необъявленные — на отказ. diff --git a/openspec/changes/archive/2026-07-08-state-transition-graph/proposal.md b/openspec/changes/archive/2026-07-08-state-transition-graph/proposal.md new file mode 100644 index 0000000..7bf4cf0 --- /dev/null +++ b/openspec/changes/archive/2026-07-08-state-transition-graph/proposal.md @@ -0,0 +1,48 @@ +# Декларативный граф переходов машины состояний + +## Why + +Легальность переходов машины состояний загрузки сейчас **нигде не записана явно**. +Чтобы понять «из `downloading` куда можно», надо прочитать весь `worker` (три файла: +`worker.go`, `reconcile.go`, `review.go`) плюс store-методы. Единственный +механический гард в `store.setState` — грубый: он запрещает молча оживить +терминальную задачу (`state NOT IN terminal` без `reviveOK`) и держит инвариант +«одна активная на infohash» (в SQL-транзакциях). Но он **не проверяет само ребро** +перехода: `SetDownloadState(review → done)` или `linking → completed` пройдут молча, +хотя таких переходов машина не выполняет. + +Это оставляет класс латентных багов без страховки: будущая правка воркера может +записать состояние, которого граф не предусматривает, и мы узнаем об этом только по +странице задачи в неверном состоянии. + +Внешнюю библиотеку-FSM мы сознательно НЕ берём (см. `design.md`, «Почему не +библиотека»): настоящий инвариант держится атомарно в SQLite и in-memory FSM в +транзакции участвовать не может; наши переходы — это в основном сверка с реальностью +qBittorrent, а не событийный автомат. Берём соразмерное: **свой декларативный граф** +легальных рёбер в `internal/store`, который `setState` сверяет как дополнительный +предикат, роняя необъявленный переход громко. + +## What Changes + +- **ADDED Requirement: Легальность переходов задаётся декларативным графом** — + единый источник истины `from → {разрешённые to}` в `internal/store`; переход, не + объявленный ребром графа (и не идемпотентный самопереход `from == to`), запись + состояния отклоняет. +- Гейт встраивается в `store.setState` дополнительным SQL-предикатом + `AND state IN (<легальные источники для to>)` — атомарно, без отдельного чтения; + существующий гард терминальности и инвариант «одна активная на infohash» + сохраняются без изменений и остаются **ортогональны** (граф говорит «ребро есть», + гард терминальности — «но не мимо `ActivateIfNoOtherActive`»). +- Тест согласованности: граф хорошо сформирован (все состояния известны), объявленные + рёбра проходят, необъявленные — отклоняются, а рёбра из терминальных состояний + проходят только через revive-путь (`ActivateIfNoOtherActive`), но не через + `SetDownloadState`. +- Поведение существующих легальных переходов НЕ меняется: граф — надмножество всего, + что воркер уже выполняет. + +## Impact + +- Specs: `download-tracking` (владелец прямого пути машины состояний). +- Код: `internal/store/download.go` (`setState` + граф), новый тест графа. Воркер и + транспорты — без изменений (граф прозрачен для легальных переходов). +- Зависимости: ноль новых (принцип «минимум компонентов» соблюдён). diff --git a/openspec/changes/archive/2026-07-08-state-transition-graph/specs/download-tracking/spec.md b/openspec/changes/archive/2026-07-08-state-transition-graph/specs/download-tracking/spec.md new file mode 100644 index 0000000..59df7f6 --- /dev/null +++ b/openspec/changes/archive/2026-07-08-state-transition-graph/specs/download-tracking/spec.md @@ -0,0 +1,55 @@ +# download-tracking Specification + +## ADDED Requirements + +### Requirement: Легальность переходов задаётся декларативным графом + +Множество легальных переходов машины состояний загрузки SHALL быть объявлено +декларативно в едином месте (`internal/store`) как отображение `from → +{разрешённые to}`, покрывающее все переходы, которые worker выполняет по всем +capability (прямой путь, `state-reconciliation`, `review`). Этот граф SHALL быть +единственным источником истины о легальности рёбер. + +Запись состояния (`setState`, общая основа `SetDownloadState` и +`ActivateIfNoOtherActive`) SHALL применять переход, только если он либо объявлен +ребром графа, либо является идемпотентным самопереходом (`from == to`, переустановка +того же состояния — например, повторная запись ошибки). Переход, не удовлетворяющий +ни одному из условий, запись SHALL отклонять (0 строк UPDATE → ошибка), НЕ применяя +его. + +Гейт графа SHALL быть **ортогонален** остальным гардам записи и НЕ SHALL их ослаблять: +существующий запрет молча оживить терминальную задачу (переход из терминального +состояния разрешён только через `ActivateIfNoOtherActive` с проверкой владения +хешами) и инвариант «не более одной активной загрузки на infohash» сохраняются. Как +следствие, ребро из терминального состояния (напр. `failed → downloading` при retry) +SHALL проходить только revive-путём (`ActivateIfNoOtherActive`) и SHALL отклоняться +обычным `SetDownloadState`. + +Граф SHALL быть надмножеством всех переходов, которые worker уже выполняет: введение +гейта НЕ SHALL менять поведение существующих легальных переходов. + +#### Scenario: Объявленный переход применяется + +- **GIVEN** загрузка в состоянии `downloading` +- **WHEN** worker записывает переход `downloading → completed` (объявленное ребро) +- **THEN** состояние становится `completed` + +#### Scenario: Необъявленный переход отклоняется + +- **GIVEN** загрузка в состоянии `review` +- **WHEN** делается попытка записать переход `review → done` (ребра в графе нет) +- **THEN** запись отклоняется с ошибкой, состояние остаётся `review` + +#### Scenario: Идемпотентная переустановка состояния разрешена + +- **GIVEN** загрузка в состоянии `deferred` +- **WHEN** записывается переход `deferred → deferred` (самопереход) +- **THEN** запись проходит, состояние остаётся `deferred` + +#### Scenario: Ребро из терминального состояния только через revive + +- **GIVEN** загрузка в терминальном состоянии `failed` +- **WHEN** переход `failed → downloading` делается обычным `SetDownloadState` +- **THEN** запись отклоняется (терминальную задачу нельзя оживить мимо гарда владения) +- **AND** тот же переход через `ActivateIfNoOtherActive` (при свободном infohash) + проходит diff --git a/openspec/changes/archive/2026-07-08-state-transition-graph/tasks.md b/openspec/changes/archive/2026-07-08-state-transition-graph/tasks.md new file mode 100644 index 0000000..b928543 --- /dev/null +++ b/openspec/changes/archive/2026-07-08-state-transition-graph/tasks.md @@ -0,0 +1,50 @@ +# Tasks + +## 1. Граф в store + +- [x] 1.1 Объявить `allowedTransitions map[State][]State` (from → to) в + `internal/store/download.go` рядом с `terminalStates`, с комментарием об источнике + истины и правиле самоперехода. Заполнить по таблице из `design.md`. +- [x] 1.2 Построить обратное отображение `to → {легальные from}` (для SQL-предиката) + как package-level `var` через хелпер-инвертор; включать сам `to` (самопереход). + +## 2. Гейт в setState + +- [x] 2.1 В `setState` добавить предикат `AND state IN (<легальные источники для to>)` + до/рядом с существующим гардом терминальности; аргументы через `placeholders`. +- [x] 2.2 На `n == 0` — диагностическое чтение текущего состояния (через + `sqlx.QueryerContext`, если `e` его поддерживает) для точного сообщения: + «not found» / «illegal transition → » / терминал без revive. +- [x] 2.3 Сверить `PromoteCatched`: ребро `catched → downloading` присутствует в + графе; оставить его собственный гард `state='catched'`, добавить комментарий-ссылку + на граф. + +## 3. Тест согласованности + +- [x] 3.1 `TestTransitionGraphWellFormed`: все состояния в ключах и значениях графа — + известные (из полного списка `State`); ни один список не содержит сам ключ + (петли не перечисляются явно). +- [x] 3.2 `TestSetStateAllowsDeclaredEdges`: для набора объявленных не-revive рёбер + (`downloading→completed`, `review→linking`, `recognizing→review`, …) + `SetDownloadState` проходит. +- [x] 3.3 `TestSetStateRejectsUndeclaredEdges`: репрезентативные необъявленные + (`review→done`, `downloading→done`, `completed→linking`) отклоняются, состояние не + меняется. +- [x] 3.4 `TestSelfTransitionAllowed`: `deferred→deferred` проходит. +- [x] 3.5 `TestTerminalReviveOnlyViaActivate`: `failed→downloading` через + `SetDownloadState` отклоняется, а через `ActivateIfNoOtherActive` (при свободном + infohash) проходит. +- [x] 3.6 `TestCancelDeferReachableFromEveryNonTerminal`: инвариант generic-команд — + для каждого не-терминального состояния `cancelled` — легальная цель, и `deferred` — + легальная цель (кроме самого `deferred`, где это самопереход). Ловит класс дыры + «забыли состояние» (напр. `linking` после краха). +- [x] 3.7 `TestPreflightDesyncEdges`: рёбра `reconcileToReality` из ревью/терминальных + состояний при пропавшем источнике — `review/deferred/reverted/cancelled → + orphaned` и `→ deleted` — проходят. + +## 4. Проверка отсутствия регрессий + +- [x] 4.1 `task test` — весь набор зелёный (существующие тесты воркера/store — сеть + безопасности против слишком тесного графа). +- [x] 4.2 `task lint` — 0 issues. +- [x] 4.3 `openspec validate state-transition-graph --strict`. diff --git a/openspec/specs/download-tracking/spec.md b/openspec/specs/download-tracking/spec.md index 8f8017e..a6abe02 100644 --- a/openspec/specs/download-tracking/spec.md +++ b/openspec/specs/download-tracking/spec.md @@ -169,3 +169,55 @@ qBittorrent» — как в поллинге активных загрузок, - **THEN** загрузка не считается пропавшей/рассинхронизированной и остаётся в `catched` (до добавления воркером или срабатывания `catch_timeout`) +### Requirement: Легальность переходов задаётся декларативным графом + +Множество легальных переходов машины состояний загрузки SHALL быть объявлено +декларативно в едином месте (`internal/store`) как отображение `from → +{разрешённые to}`, покрывающее все переходы, которые worker выполняет по всем +capability (прямой путь, `state-reconciliation`, `review`). Этот граф SHALL быть +единственным источником истины о легальности рёбер. + +Запись состояния (`setState`, общая основа `SetDownloadState` и +`ActivateIfNoOtherActive`) SHALL применять переход, только если он либо объявлен +ребром графа, либо является идемпотентным самопереходом (`from == to`, переустановка +того же состояния — например, повторная запись ошибки). Переход, не удовлетворяющий +ни одному из условий, запись SHALL отклонять (0 строк UPDATE → ошибка), НЕ применяя +его. + +Гейт графа SHALL быть **ортогонален** остальным гардам записи и НЕ SHALL их ослаблять: +существующий запрет молча оживить терминальную задачу (переход из терминального +состояния разрешён только через `ActivateIfNoOtherActive` с проверкой владения +хешами) и инвариант «не более одной активной загрузки на infohash» сохраняются. Как +следствие, ребро из терминального состояния (напр. `failed → downloading` при retry) +SHALL проходить только revive-путём (`ActivateIfNoOtherActive`) и SHALL отклоняться +обычным `SetDownloadState`. + +Граф SHALL быть надмножеством всех переходов, которые worker уже выполняет: введение +гейта НЕ SHALL менять поведение существующих легальных переходов. + +#### Scenario: Объявленный переход применяется + +- **GIVEN** загрузка в состоянии `downloading` +- **WHEN** worker записывает переход `downloading → completed` (объявленное ребро) +- **THEN** состояние становится `completed` + +#### Scenario: Необъявленный переход отклоняется + +- **GIVEN** загрузка в состоянии `review` +- **WHEN** делается попытка записать переход `review → done` (ребра в графе нет) +- **THEN** запись отклоняется с ошибкой, состояние остаётся `review` + +#### Scenario: Идемпотентная переустановка состояния разрешена + +- **GIVEN** загрузка в состоянии `deferred` +- **WHEN** записывается переход `deferred → deferred` (самопереход) +- **THEN** запись проходит, состояние остаётся `deferred` + +#### Scenario: Ребро из терминального состояния только через revive + +- **GIVEN** загрузка в терминальном состоянии `failed` +- **WHEN** переход `failed → downloading` делается обычным `SetDownloadState` +- **THEN** запись отклоняется (терминальную задачу нельзя оживить мимо гарда владения) +- **AND** тот же переход через `ActivateIfNoOtherActive` (при свободном infohash) + проходит +