fix: coordinate maintenance health and compaction status recovery (#3770)
This commit is contained in:
@@ -5,19 +5,95 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
"testing/synctest"
|
||||
"time"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
"github.com/jmoiron/sqlx"
|
||||
_ "github.com/mattn/go-sqlite3"
|
||||
"go.uber.org/zap"
|
||||
"go.uber.org/zap/zaptest/observer"
|
||||
|
||||
"github.com/kandev/kandev/internal/auth"
|
||||
authhttpmw "github.com/kandev/kandev/internal/auth/httpmw"
|
||||
authstore "github.com/kandev/kandev/internal/auth/store"
|
||||
"github.com/kandev/kandev/internal/common/config"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/db"
|
||||
"github.com/kandev/kandev/internal/persistence/requiredstores"
|
||||
"github.com/kandev/kandev/internal/system/maintenance"
|
||||
userstore "github.com/kandev/kandev/internal/user/store"
|
||||
)
|
||||
|
||||
func TestPersistenceMiddlewareRemainsAvailableDuringManagedMaintenance(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
conn, err := sqlx.Open("sqlite3", ":memory:")
|
||||
if err != nil {
|
||||
t.Fatalf("open sqlite: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = conn.Close() })
|
||||
conn.SetMaxOpenConns(1)
|
||||
if _, err := conn.Exec("CREATE TABLE tasks (id TEXT PRIMARY KEY)"); err != nil {
|
||||
t.Fatalf("create tasks table: %v", err)
|
||||
}
|
||||
pool := db.NewPool(conn, conn)
|
||||
tracker, err := requiredstores.NewTracker([]requiredstores.Descriptor{{
|
||||
ID: "task", OwnerPackage: "internal/task", RequiredTables: []string{"tasks"},
|
||||
}})
|
||||
if err != nil {
|
||||
t.Fatalf("NewTracker: %v", err)
|
||||
}
|
||||
if err := tracker.RecordSuccess("task"); err != nil {
|
||||
t.Fatalf("RecordSuccess: %v", err)
|
||||
}
|
||||
core, logs := observer.New(zap.DebugLevel)
|
||||
log, err := logger.NewFromZap(zap.New(core))
|
||||
if err != nil {
|
||||
t.Fatalf("NewFromZap: %v", err)
|
||||
}
|
||||
health := requiredstores.NewHealth(tracker, pool, log)
|
||||
if err := health.Check(context.Background()); err != nil {
|
||||
t.Fatalf("initial Check: %v", err)
|
||||
}
|
||||
|
||||
release, ok := maintenance.ForPool(pool).TryAcquire()
|
||||
if !ok {
|
||||
t.Fatal("maintenance lease unavailable")
|
||||
}
|
||||
t.Cleanup(release)
|
||||
tx, err := conn.Beginx()
|
||||
if err != nil {
|
||||
t.Fatalf("begin writer transaction: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = tx.Rollback() })
|
||||
|
||||
health.SetInterval(time.Millisecond)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
t.Cleanup(cancel)
|
||||
stop := health.Start(ctx)
|
||||
t.Cleanup(func() { _ = stop() })
|
||||
time.Sleep(time.Millisecond)
|
||||
synctest.Wait()
|
||||
|
||||
if !health.Healthy() {
|
||||
t.Fatal("required persistence became unhealthy during managed maintenance")
|
||||
}
|
||||
if count := logs.FilterMessage("required persistence probe deferred during database maintenance").Len(); count == 0 {
|
||||
t.Fatal("periodic health probe did not report maintenance deferral")
|
||||
}
|
||||
|
||||
router := gin.New()
|
||||
router.Use(requiredPersistenceMiddleware(health))
|
||||
router.GET("/api/v1/tasks", func(c *gin.Context) { c.Status(http.StatusOK) })
|
||||
rec := httptest.NewRecorder()
|
||||
router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/api/v1/tasks", nil))
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("stateful request status = %d, want %d", rec.Code, http.StatusOK)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestRequiredPersistenceMiddlewareBlocksStatefulTrafficButAllowsDiagnostics(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
tracker, err := requiredstores.NewTracker([]requiredstores.Descriptor{{
|
||||
|
||||
@@ -12,6 +12,8 @@ import (
|
||||
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/db"
|
||||
"github.com/kandev/kandev/internal/db/dialect"
|
||||
"github.com/kandev/kandev/internal/system/maintenance"
|
||||
)
|
||||
|
||||
const (
|
||||
@@ -83,6 +85,38 @@ func (h *Health) Check(ctx context.Context) error {
|
||||
return errors.Join(failures...)
|
||||
}
|
||||
|
||||
// MarkUnavailable records a destructive database transition before the
|
||||
// maintenance owner releases its admission lease. This keeps stateful
|
||||
// requests fail-closed while the process waits for the required restart.
|
||||
func (h *Health) MarkUnavailable() {
|
||||
if h == nil || h.tracker == nil {
|
||||
return
|
||||
}
|
||||
_ = h.recordUnavailable(errors.New("database maintenance requires restart"))
|
||||
}
|
||||
|
||||
// checkRuntime runs a periodic probe when the database can be admitted. SQLite
|
||||
// maintenance owns the same writer pool, so a busy maintenance lease defers
|
||||
// the probe instead of turning bounded writer contention into an unhealthy
|
||||
// state. Startup callers continue to use Check, which remains strict.
|
||||
func (h *Health) checkRuntime(ctx context.Context) (deferred bool, err error) {
|
||||
if h.isSQLite() {
|
||||
release, ok := maintenance.ForPool(h.pool).TryAcquire()
|
||||
if !ok {
|
||||
if h.log != nil {
|
||||
h.log.Debug("required persistence probe deferred during database maintenance")
|
||||
}
|
||||
return true, nil
|
||||
}
|
||||
defer release()
|
||||
}
|
||||
return false, h.Check(ctx)
|
||||
}
|
||||
|
||||
func (h *Health) isSQLite() bool {
|
||||
return h.pool != nil && h.pool.Writer() != nil && h.pool.Writer().DriverName() == dialect.SQLite3
|
||||
}
|
||||
|
||||
func (h *Health) ping(ctx context.Context) error {
|
||||
if err := h.pool.Writer().PingContext(ctx); err != nil {
|
||||
return fmt.Errorf("writer ping failed: %w", err)
|
||||
@@ -146,7 +180,8 @@ func (h *Health) run(ctx context.Context, interval time.Duration, done chan stru
|
||||
return
|
||||
case <-ticker.C:
|
||||
checkCtx, cancel := context.WithTimeout(ctx, probeTimeout)
|
||||
if err := h.Check(checkCtx); err != nil && h.log != nil {
|
||||
_, err := h.checkRuntime(checkCtx)
|
||||
if err != nil && h.log != nil {
|
||||
h.log.Warn("required persistence probe failed", zap.Error(err))
|
||||
}
|
||||
cancel()
|
||||
|
||||
@@ -2,6 +2,7 @@ package requiredstores
|
||||
|
||||
import (
|
||||
"context"
|
||||
"reflect"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -9,8 +10,214 @@ import (
|
||||
_ "github.com/mattn/go-sqlite3"
|
||||
|
||||
"github.com/kandev/kandev/internal/db"
|
||||
"github.com/kandev/kandev/internal/system/maintenance"
|
||||
)
|
||||
|
||||
func newSQLiteHealthFixture(t *testing.T, descriptors []Descriptor) (*sqlx.DB, *db.Pool, *Tracker, *Health) {
|
||||
t.Helper()
|
||||
conn, err := sqlx.Open("sqlite3", ":memory:")
|
||||
if err != nil {
|
||||
t.Fatalf("open sqlite: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = conn.Close() })
|
||||
conn.SetMaxOpenConns(1)
|
||||
|
||||
tracker, err := NewTracker(descriptors)
|
||||
if err != nil {
|
||||
t.Fatalf("NewTracker: %v", err)
|
||||
}
|
||||
pool := db.NewPool(conn, conn)
|
||||
health := NewHealth(tracker, pool, nil)
|
||||
return conn, pool, tracker, health
|
||||
}
|
||||
|
||||
func holdWriter(t *testing.T, conn *sqlx.DB) *sqlx.Tx {
|
||||
t.Helper()
|
||||
tx, err := conn.Beginx()
|
||||
if err != nil {
|
||||
t.Fatalf("begin writer transaction: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = tx.Rollback() })
|
||||
return tx
|
||||
}
|
||||
|
||||
func TestRuntimeHealthDefersDuringMaintenance(t *testing.T) {
|
||||
conn, pool, tracker, health := newSQLiteHealthFixture(t, []Descriptor{{
|
||||
ID: "first", OwnerPackage: "owner/first", RequiredTables: []string{"first"},
|
||||
}})
|
||||
if _, err := conn.Exec("CREATE TABLE first (id TEXT PRIMARY KEY)"); err != nil {
|
||||
t.Fatalf("create first table: %v", err)
|
||||
}
|
||||
if err := tracker.RecordSuccess("first"); err != nil {
|
||||
t.Fatalf("RecordSuccess: %v", err)
|
||||
}
|
||||
if err := health.Check(context.Background()); err != nil {
|
||||
t.Fatalf("initial Check: %v", err)
|
||||
}
|
||||
before := tracker.Snapshot()
|
||||
|
||||
release, ok := maintenance.ForPool(pool).TryAcquire()
|
||||
if !ok {
|
||||
t.Fatal("maintenance lease unavailable")
|
||||
}
|
||||
defer release()
|
||||
holdWriter(t, conn)
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 25*time.Millisecond)
|
||||
defer cancel()
|
||||
deferred, err := health.checkRuntime(ctx)
|
||||
if err != nil {
|
||||
t.Fatalf("checkRuntime error = %v, want nil while maintenance owns the pool", err)
|
||||
}
|
||||
if !deferred {
|
||||
t.Fatal("checkRuntime deferred = false, want true while maintenance owns the pool")
|
||||
}
|
||||
after := tracker.Snapshot()
|
||||
if !reflect.DeepEqual(after, before) {
|
||||
t.Fatalf("tracker changed during deferred probe: before=%#v after=%#v", before, after)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRuntimeHealthDeferralPreservesMixedStoreStates(t *testing.T) {
|
||||
conn, pool, tracker, health := newSQLiteHealthFixture(t, []Descriptor{
|
||||
{ID: "healthy", OwnerPackage: "owner/healthy", RequiredTables: []string{"healthy"}},
|
||||
{ID: "unhealthy", OwnerPackage: "owner/unhealthy", RequiredTables: []string{"unhealthy"}},
|
||||
})
|
||||
if _, err := conn.Exec("CREATE TABLE healthy (id TEXT PRIMARY KEY)"); err != nil {
|
||||
t.Fatalf("create healthy table: %v", err)
|
||||
}
|
||||
for _, id := range []string{"healthy", "unhealthy"} {
|
||||
if err := tracker.RecordSuccess(id); err != nil {
|
||||
t.Fatalf("RecordSuccess(%q): %v", id, err)
|
||||
}
|
||||
}
|
||||
if err := health.Check(context.Background()); err == nil {
|
||||
t.Fatal("initial Check returned nil with a missing table")
|
||||
}
|
||||
before := tracker.Snapshot()
|
||||
|
||||
release, ok := maintenance.ForPool(pool).TryAcquire()
|
||||
if !ok {
|
||||
t.Fatal("maintenance lease unavailable")
|
||||
}
|
||||
defer release()
|
||||
holdWriter(t, conn)
|
||||
|
||||
deferred, err := health.checkRuntime(context.Background())
|
||||
if err != nil {
|
||||
t.Fatalf("checkRuntime error = %v, want nil while maintenance owns the pool", err)
|
||||
}
|
||||
if !deferred {
|
||||
t.Fatal("checkRuntime deferred = false, want true while maintenance owns the pool")
|
||||
}
|
||||
after := tracker.Snapshot()
|
||||
if !reflect.DeepEqual(after, before) {
|
||||
t.Fatalf("mixed tracker state changed during deferred probe: before=%#v after=%#v", before, after)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStartupHealthDoesNotDefer(t *testing.T) {
|
||||
conn, pool, tracker, health := newSQLiteHealthFixture(t, []Descriptor{{
|
||||
ID: "first", OwnerPackage: "owner/first", RequiredTables: []string{"first"},
|
||||
}})
|
||||
if _, err := conn.Exec("CREATE TABLE first (id TEXT PRIMARY KEY)"); err != nil {
|
||||
t.Fatalf("create first table: %v", err)
|
||||
}
|
||||
if err := tracker.RecordSuccess("first"); err != nil {
|
||||
t.Fatalf("RecordSuccess: %v", err)
|
||||
}
|
||||
if err := health.Check(context.Background()); err != nil {
|
||||
t.Fatalf("initial Check: %v", err)
|
||||
}
|
||||
before, ok := tracker.Status("first")
|
||||
if !ok {
|
||||
t.Fatal("missing first status")
|
||||
}
|
||||
|
||||
release, ok := maintenance.ForPool(pool).TryAcquire()
|
||||
if !ok {
|
||||
t.Fatal("maintenance lease unavailable")
|
||||
}
|
||||
defer release()
|
||||
holdWriter(t, conn)
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 25*time.Millisecond)
|
||||
defer cancel()
|
||||
if err := health.Check(ctx); err == nil {
|
||||
t.Fatal("startup Check returned nil while its writer probe was blocked")
|
||||
}
|
||||
after, ok := tracker.Status("first")
|
||||
if !ok {
|
||||
t.Fatal("missing first status after Check")
|
||||
}
|
||||
if after.State != StateUnhealthy {
|
||||
t.Fatalf("startup Check state = %q, want %q", after.State, StateUnhealthy)
|
||||
}
|
||||
if !after.LastCheckedAt.After(before.LastCheckedAt) {
|
||||
t.Fatalf("startup Check did not record a newer check time: before=%v after=%v", before.LastCheckedAt, after.LastCheckedAt)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRuntimeHealthResumesAfterMaintenance(t *testing.T) {
|
||||
conn, pool, tracker, health := newSQLiteHealthFixture(t, []Descriptor{{
|
||||
ID: "first", OwnerPackage: "owner/first", RequiredTables: []string{"first"},
|
||||
}})
|
||||
if _, err := conn.Exec("CREATE TABLE first (id TEXT PRIMARY KEY)"); err != nil {
|
||||
t.Fatalf("create first table: %v", err)
|
||||
}
|
||||
if err := tracker.RecordSuccess("first"); err != nil {
|
||||
t.Fatalf("RecordSuccess: %v", err)
|
||||
}
|
||||
if err := health.Check(context.Background()); err != nil {
|
||||
t.Fatalf("initial Check: %v", err)
|
||||
}
|
||||
|
||||
release, ok := maintenance.ForPool(pool).TryAcquire()
|
||||
if !ok {
|
||||
t.Fatal("maintenance lease unavailable")
|
||||
}
|
||||
deferred, err := health.checkRuntime(context.Background())
|
||||
if err != nil || !deferred {
|
||||
t.Fatalf("checkRuntime during maintenance = deferred %v, err %v; want true, nil", deferred, err)
|
||||
}
|
||||
release()
|
||||
|
||||
if _, err := conn.Exec("DROP TABLE first"); err != nil {
|
||||
t.Fatalf("drop first table: %v", err)
|
||||
}
|
||||
deferred, err = health.checkRuntime(context.Background())
|
||||
if deferred {
|
||||
t.Fatal("checkRuntime deferred after maintenance released")
|
||||
}
|
||||
if err == nil {
|
||||
t.Fatal("checkRuntime returned nil with a missing table")
|
||||
}
|
||||
if health.Healthy() {
|
||||
t.Fatal("health remained healthy after a real post-maintenance failure")
|
||||
}
|
||||
probeLease, ok := maintenance.ForPool(pool).TryAcquire()
|
||||
if !ok {
|
||||
t.Fatal("failed probe leaked maintenance admission")
|
||||
}
|
||||
probeLease()
|
||||
|
||||
if _, err := conn.Exec("CREATE TABLE first (id TEXT PRIMARY KEY)"); err != nil {
|
||||
t.Fatalf("recreate first table: %v", err)
|
||||
}
|
||||
deferred, err = health.checkRuntime(context.Background())
|
||||
if deferred || err != nil {
|
||||
t.Fatalf("recovery check = deferred %v, err %v; want false, nil", deferred, err)
|
||||
}
|
||||
if !health.Healthy() {
|
||||
t.Fatal("health did not recover after the missing table was repaired")
|
||||
}
|
||||
successLease, ok := maintenance.ForPool(pool).TryAcquire()
|
||||
if !ok {
|
||||
t.Fatal("successful probe leaked maintenance admission")
|
||||
}
|
||||
successLease()
|
||||
}
|
||||
|
||||
func TestHealthCheckMarksMissingTableUnhealthyAndRecovers(t *testing.T) {
|
||||
conn, err := sqlx.Open("sqlite3", ":memory:")
|
||||
if err != nil {
|
||||
|
||||
@@ -64,6 +64,10 @@ type Service struct {
|
||||
jobs *jobs.Tracker
|
||||
log *logger.Logger
|
||||
|
||||
// PersistenceUnavailable marks required stores unhealthy before restore
|
||||
// closes the pool and leaves the process awaiting restart.
|
||||
PersistenceUnavailable func()
|
||||
|
||||
// RestoreQuiesce stops scheduling, active executions, and database-backed
|
||||
// workers before restore closes the shared database pool. Wired by the
|
||||
// backend composition root; tests may leave it nil.
|
||||
@@ -210,6 +214,9 @@ func (s *Service) runRestore(ctx context.Context, snapshotPath string) (map[stri
|
||||
_ = os.Remove(stagedPath)
|
||||
return nil, err
|
||||
}
|
||||
if s.PersistenceUnavailable != nil {
|
||||
s.PersistenceUnavailable()
|
||||
}
|
||||
if s.RestoreQuiesce != nil {
|
||||
if err := s.RestoreQuiesce(); err != nil {
|
||||
_ = os.Remove(stagedPath)
|
||||
|
||||
@@ -59,6 +59,9 @@ func (s *Service) runFactoryReset(ctx context.Context) (map[string]interface{},
|
||||
if err := s.requireSQLiteMaintenance("factory reset"); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if s.PersistenceUnavailable != nil {
|
||||
s.PersistenceUnavailable()
|
||||
}
|
||||
|
||||
if s.OrchestratorShutdown != nil {
|
||||
s.OrchestratorShutdown()
|
||||
|
||||
@@ -68,6 +68,10 @@ type Service struct {
|
||||
jobs *jobs.Tracker
|
||||
log *logger.Logger
|
||||
|
||||
// PersistenceUnavailable marks required stores unhealthy before a
|
||||
// destructive maintenance operation leaves the process awaiting restart.
|
||||
PersistenceUnavailable func()
|
||||
|
||||
// OrchestratorShutdown stops the orchestrator and active executions before
|
||||
// the factory-reset job runs. Wired by cmd/kandev. Tests pass a no-op.
|
||||
OrchestratorShutdown func()
|
||||
|
||||
@@ -120,9 +120,16 @@ func Provide(cfg *config.Config, log *logger.Logger, pool *db.Pool, eventBus bus
|
||||
}
|
||||
dbSvc := database.NewService(pool, databasePath, resetDirs, tracker, log)
|
||||
dbSvc.OrchestratorShutdown = wiring.OrchestratorShutdown
|
||||
markPersistenceUnavailable := func() {
|
||||
if wiring.PersistenceHealth != nil {
|
||||
wiring.PersistenceHealth.MarkUnavailable()
|
||||
}
|
||||
}
|
||||
dbSvc.PersistenceUnavailable = markPersistenceUnavailable
|
||||
|
||||
backupsSvc := backups.NewService(databasePath, pool, tracker, log)
|
||||
backupsSvc.OrchestratorShutdown = wiring.OrchestratorShutdown
|
||||
backupsSvc.PersistenceUnavailable = markPersistenceUnavailable
|
||||
retentionSvc := provideToolRetention(pool, backupsSvc, eventBus, log, wiring)
|
||||
dbSvc.DatabaseQuiesce = retentionQuiesce(retentionSvc, wiring.DatabaseQuiesce)
|
||||
restoreQuiesce := wiring.RestoreQuiesce
|
||||
|
||||
@@ -1,12 +1,14 @@
|
||||
package system
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
"github.com/jmoiron/sqlx"
|
||||
@@ -15,6 +17,8 @@ import (
|
||||
"github.com/kandev/kandev/internal/common/config"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/db"
|
||||
"github.com/kandev/kandev/internal/persistence/requiredstores"
|
||||
"github.com/kandev/kandev/internal/system/jobs"
|
||||
)
|
||||
|
||||
func TestConfiguredDatabasePath_ProvideListsSiblingBackups(t *testing.T) {
|
||||
@@ -109,6 +113,128 @@ func TestProvideWiresRestoreQuiesce(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestProvideMarksPersistenceUnavailableForFactoryReset(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
databasePath := filepath.Join(root, "custom", "named.db")
|
||||
pool := openSystemDatabasePathPool(t, databasePath)
|
||||
t.Cleanup(func() { _ = pool.Close() })
|
||||
if _, err := pool.Writer().Exec(`CREATE TABLE kandev_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL)`); err != nil {
|
||||
t.Fatalf("create kandev_meta: %v", err)
|
||||
}
|
||||
|
||||
tracker, err := requiredstores.NewTracker([]requiredstores.Descriptor{{
|
||||
ID: "settings", OwnerPackage: "internal/system/settings", RequiredTables: []string{"settings"},
|
||||
}})
|
||||
if err != nil {
|
||||
t.Fatalf("NewTracker: %v", err)
|
||||
}
|
||||
if err := tracker.RecordSuccess("settings"); err != nil {
|
||||
t.Fatalf("RecordSuccess: %v", err)
|
||||
}
|
||||
health := requiredstores.NewHealth(tracker, pool, nil)
|
||||
log, err := logger.NewFromZap(zap.NewNop())
|
||||
if err != nil {
|
||||
t.Fatalf("logger: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = log.Close() })
|
||||
|
||||
service := Provide(&config.Config{
|
||||
HomeDir: root,
|
||||
Database: config.DatabaseConfig{Driver: "sqlite", Path: databasePath},
|
||||
}, log, pool, nil, BuildInfo{}, Wiring{
|
||||
RequiredStores: tracker,
|
||||
PersistenceHealth: health,
|
||||
})
|
||||
if err := health.Check(context.Background()); err != nil {
|
||||
t.Fatalf("initial health check: %v", err)
|
||||
}
|
||||
jobID, err := service.Database.FactoryReset(context.Background(), "RESET")
|
||||
if err != nil {
|
||||
t.Fatalf("FactoryReset: %v", err)
|
||||
}
|
||||
job := waitForSystemJob(t, service.Jobs, jobID)
|
||||
if job.State != jobs.StateSucceeded {
|
||||
t.Fatalf("factory reset state = %s, want succeeded: %s", job.State, job.Message)
|
||||
}
|
||||
if health.Healthy() {
|
||||
t.Fatal("required persistence recovered before the frontend restart")
|
||||
}
|
||||
}
|
||||
|
||||
func TestProvideMarksPersistenceUnavailableForRestore(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
databasePath := filepath.Join(root, "custom", "named.db")
|
||||
pool := openSystemDatabasePathPool(t, databasePath)
|
||||
t.Cleanup(func() { _ = pool.Close() })
|
||||
|
||||
tracker, err := requiredstores.NewTracker([]requiredstores.Descriptor{{
|
||||
ID: "settings", OwnerPackage: "internal/system/settings", RequiredTables: []string{"settings"},
|
||||
}})
|
||||
if err != nil {
|
||||
t.Fatalf("NewTracker: %v", err)
|
||||
}
|
||||
if err := tracker.RecordSuccess("settings"); err != nil {
|
||||
t.Fatalf("RecordSuccess: %v", err)
|
||||
}
|
||||
health := requiredstores.NewHealth(tracker, pool, nil)
|
||||
log, err := logger.NewFromZap(zap.NewNop())
|
||||
if err != nil {
|
||||
t.Fatalf("logger: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = log.Close() })
|
||||
|
||||
service := Provide(&config.Config{
|
||||
HomeDir: root,
|
||||
Database: config.DatabaseConfig{Driver: "sqlite", Path: databasePath},
|
||||
}, log, pool, nil, BuildInfo{}, Wiring{
|
||||
RequiredStores: tracker,
|
||||
PersistenceHealth: health,
|
||||
})
|
||||
if err := health.Check(context.Background()); err != nil {
|
||||
t.Fatalf("initial health check: %v", err)
|
||||
}
|
||||
snapshotJobID := service.Backups.Create(context.Background())
|
||||
snapshotJob := waitForSystemJob(t, service.Jobs, snapshotJobID)
|
||||
if snapshotJob.State != jobs.StateSucceeded {
|
||||
t.Fatalf("snapshot state = %s, want succeeded: %s", snapshotJob.State, snapshotJob.Message)
|
||||
}
|
||||
snapshots, err := service.Backups.List()
|
||||
if err != nil || len(snapshots) != 1 {
|
||||
t.Fatalf("List() = %#v, %v; want one snapshot", snapshots, err)
|
||||
}
|
||||
|
||||
restoreJobID, err := service.Backups.Restore(context.Background(), snapshots[0].Name, "RESTORE")
|
||||
if err != nil {
|
||||
t.Fatalf("Restore: %v", err)
|
||||
}
|
||||
restoreJob := waitForSystemJob(t, service.Jobs, restoreJobID)
|
||||
if restoreJob.State != jobs.StateSucceeded {
|
||||
t.Fatalf("restore state = %s, want succeeded: %s", restoreJob.State, restoreJob.Message)
|
||||
}
|
||||
if health.Healthy() {
|
||||
t.Fatal("required persistence recovered before the frontend restart")
|
||||
}
|
||||
}
|
||||
|
||||
func waitForSystemJob(t *testing.T, tracker *jobs.Tracker, id string) *jobs.Job {
|
||||
t.Helper()
|
||||
ticker := time.NewTicker(5 * time.Millisecond)
|
||||
defer ticker.Stop()
|
||||
deadline := time.NewTimer(5 * time.Second)
|
||||
defer deadline.Stop()
|
||||
for {
|
||||
job := tracker.Get(id)
|
||||
if job != nil && (job.State == jobs.StateSucceeded || job.State == jobs.StateFailed) {
|
||||
return job
|
||||
}
|
||||
select {
|
||||
case <-ticker.C:
|
||||
case <-deadline.C:
|
||||
t.Fatalf("job %q did not finish", id)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func openSystemDatabasePathPool(t *testing.T, databasePath string) *db.Pool {
|
||||
t.Helper()
|
||||
writerRaw, err := db.OpenSQLite(databasePath)
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react";
|
||||
import { afterEach, beforeEach, expect, it, vi } from "vitest";
|
||||
import type { SettingsSaveContributor } from "@/components/settings/settings-save-provider";
|
||||
import type { ToolPayloadRetentionStatus } from "@/lib/types/tool-payload-retention";
|
||||
import type {
|
||||
ToolPayloadOperation,
|
||||
ToolPayloadRetentionStatus,
|
||||
} from "@/lib/types/tool-payload-retention";
|
||||
import * as api from "@/lib/api/domains/tool-payload-retention-api";
|
||||
import { ToolPayloadRetentionCard } from "./tool-payload-retention-card";
|
||||
vi.mock("@/lib/api/domains/tool-payload-retention-api");
|
||||
@@ -15,6 +18,9 @@ vi.mock("@/components/settings/settings-save-provider", () => ({
|
||||
}));
|
||||
const ENABLED_TEST_ID = "tool-payload-enabled";
|
||||
const AGE_TEST_ID = "tool-payload-age";
|
||||
const ERROR_TEST_ID = "tool-payload-error";
|
||||
const ANALYZE_TEST_ID = "tool-payload-analyze";
|
||||
const LAST_RUN_TEST_ID = "tool-payload-last-run";
|
||||
|
||||
const baseline: ToolPayloadRetentionStatus = {
|
||||
supported: true,
|
||||
@@ -31,6 +37,79 @@ async function open() {
|
||||
render(<ToolPayloadRetentionCard />);
|
||||
await screen.findByTestId(ENABLED_TEST_ID);
|
||||
}
|
||||
|
||||
it("clears a recovered status polling error without manual refresh", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const readError = new Error("status unavailable");
|
||||
vi.mocked(api.fetchToolPayloadRetention)
|
||||
.mockResolvedValueOnce(baseline)
|
||||
.mockRejectedValueOnce(readError)
|
||||
.mockResolvedValueOnce(baseline);
|
||||
render(<ToolPayloadRetentionCard />);
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(screen.getByTestId(ENABLED_TEST_ID)).toBeTruthy();
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(30_000);
|
||||
});
|
||||
expect(screen.getByTestId(ERROR_TEST_ID)).toBeTruthy();
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(30_000);
|
||||
});
|
||||
expect(screen.queryByTestId(ERROR_TEST_ID)).toBeNull();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves an action failure after status recovery", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const actionError = new Error("analysis failed");
|
||||
const failedRun: ToolPayloadOperation = {
|
||||
id: "failed-run",
|
||||
kind: "cleanup",
|
||||
state: "failed",
|
||||
scanned: 0,
|
||||
eligible_tasks: 0,
|
||||
eligible_messages: 0,
|
||||
removed_messages: 0,
|
||||
payload_bytes: 0,
|
||||
skipped: {},
|
||||
cutoff: "2026-01-01T00:00:00.000Z",
|
||||
started_at: "2026-01-01T00:00:00.000Z",
|
||||
finished_at: "2026-01-01T00:01:00.000Z",
|
||||
error: "cleanup_failed",
|
||||
age: baseline.policy.age,
|
||||
};
|
||||
vi.mocked(api.fetchToolPayloadRetention)
|
||||
.mockResolvedValueOnce(baseline)
|
||||
.mockResolvedValueOnce({ ...baseline, last_run: failedRun });
|
||||
vi.mocked(api.analyzeToolPayloadRetention).mockRejectedValueOnce(actionError);
|
||||
render(<ToolPayloadRetentionCard />);
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(screen.getByTestId(ENABLED_TEST_ID)).toBeTruthy();
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByTestId(ANALYZE_TEST_ID));
|
||||
await Promise.resolve();
|
||||
});
|
||||
expect(screen.getByTestId(ERROR_TEST_ID)).toBeTruthy();
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(30_000);
|
||||
});
|
||||
expect(screen.getByTestId(ERROR_TEST_ID)).toBeTruthy();
|
||||
expect(screen.getByTestId(LAST_RUN_TEST_ID).textContent).toMatch(/failed/i);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("requires an explicit backup choice before saving enablement and displays preparing", async () => {
|
||||
await open();
|
||||
fireEvent.click(screen.getByTestId(ENABLED_TEST_ID));
|
||||
@@ -58,7 +137,7 @@ it("analyzes a disabled draft without saving or enabling", async () => {
|
||||
await open();
|
||||
fireEvent.change(screen.getByTestId(AGE_TEST_ID), { target: { value: "4" } });
|
||||
vi.mocked(api.analyzeToolPayloadRetention).mockResolvedValue({ operation_id: "analysis-1" });
|
||||
fireEvent.click(screen.getByTestId("tool-payload-analyze"));
|
||||
fireEvent.click(screen.getByTestId(ANALYZE_TEST_ID));
|
||||
await waitFor(() =>
|
||||
expect(api.analyzeToolPayloadRetention).toHaveBeenCalledWith({ value: 4, unit: "months" }),
|
||||
);
|
||||
@@ -69,14 +148,14 @@ it("keeps member controls read only", async () => {
|
||||
admin = false;
|
||||
await open();
|
||||
expect(screen.getByTestId(ENABLED_TEST_ID).hasAttribute("disabled")).toBe(true);
|
||||
expect(screen.getByTestId("tool-payload-analyze").hasAttribute("disabled")).toBe(true);
|
||||
expect(screen.getByTestId(ANALYZE_TEST_ID).hasAttribute("disabled")).toBe(true);
|
||||
expect(api.analyzeToolPayloadRetention).not.toHaveBeenCalled();
|
||||
});
|
||||
it("rejects invalid ages without losing the disabled default", async () => {
|
||||
await open();
|
||||
fireEvent.change(screen.getByTestId(AGE_TEST_ID), { target: { value: "0" } });
|
||||
expect(contributor.canSave).toBe(false);
|
||||
expect(screen.getByTestId("tool-payload-analyze").hasAttribute("disabled")).toBe(true);
|
||||
expect(screen.getByTestId(ANALYZE_TEST_ID).hasAttribute("disabled")).toBe(true);
|
||||
});
|
||||
|
||||
it("preserves edits made during a pending save against its normalized response", async () => {
|
||||
|
||||
@@ -131,6 +131,82 @@ export async function resetRetention(page: Page) {
|
||||
expect(response.ok(), await response.text()).toBe(true);
|
||||
}
|
||||
|
||||
export async function failNextRetentionStatusRead(page: Page) {
|
||||
const pattern = `**${RETENTION_API}`;
|
||||
let reads = 0;
|
||||
await page.route(pattern, async (route) => {
|
||||
if (route.request().method() !== "GET") {
|
||||
await route.continue();
|
||||
return;
|
||||
}
|
||||
reads += 1;
|
||||
if (reads === 1) {
|
||||
await route.fulfill({
|
||||
status: 503,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({ code: "persistence_unavailable" }),
|
||||
});
|
||||
return;
|
||||
}
|
||||
await route.continue();
|
||||
});
|
||||
return async () => page.unroute(pattern);
|
||||
}
|
||||
|
||||
export async function reloadRetentionWithFakeClock(page: Page) {
|
||||
await page.clock.install();
|
||||
await page.reload();
|
||||
await expect(page.getByTestId("tool-payload-enabled")).toBeVisible();
|
||||
}
|
||||
|
||||
export async function recoverRetentionStatusPolling(page: Page, preserveError = false) {
|
||||
const failed = page.waitForResponse(
|
||||
(response) =>
|
||||
new URL(response.url()).pathname === RETENTION_API &&
|
||||
response.request().method() === "GET" &&
|
||||
response.status() === 503,
|
||||
);
|
||||
await page.clock.runFor(30_000);
|
||||
await failed;
|
||||
await expect(page.getByTestId("tool-payload-error")).toBeVisible();
|
||||
|
||||
const recovered = page.waitForResponse(
|
||||
(response) =>
|
||||
new URL(response.url()).pathname === RETENTION_API &&
|
||||
response.request().method() === "GET" &&
|
||||
response.status() === 200,
|
||||
);
|
||||
await page.clock.runFor(30_000);
|
||||
await recovered;
|
||||
if (preserveError) await expect(page.getByTestId("tool-payload-error")).toBeVisible();
|
||||
else await expect(page.getByTestId("tool-payload-error")).toHaveCount(0);
|
||||
}
|
||||
|
||||
export async function failRetentionAnalysis(page: Page, touch = false) {
|
||||
const pattern = `**${RETENTION_API}/analyze`;
|
||||
await page.route(pattern, async (route) => {
|
||||
if (route.request().method() === "POST") {
|
||||
await route.fulfill({
|
||||
status: 503,
|
||||
contentType: "application/json",
|
||||
body: JSON.stringify({ code: "persistence_unavailable" }),
|
||||
});
|
||||
return;
|
||||
}
|
||||
await route.continue();
|
||||
});
|
||||
const failed = page.waitForResponse(
|
||||
(response) =>
|
||||
new URL(response.url()).pathname === `${RETENTION_API}/analyze` &&
|
||||
response.request().method() === "POST" &&
|
||||
response.status() === 503,
|
||||
);
|
||||
await press(page.getByTestId("tool-payload-analyze"), touch);
|
||||
await failed;
|
||||
await expect(page.getByTestId("tool-payload-error")).toBeVisible();
|
||||
return async () => page.unroute(pattern);
|
||||
}
|
||||
|
||||
export async function press(locator: Locator, touch = false) {
|
||||
if (touch) await locator.tap();
|
||||
else await locator.click();
|
||||
|
||||
@@ -3,7 +3,11 @@ import {
|
||||
activateRetention,
|
||||
analyzeRetention,
|
||||
expectTouchTarget,
|
||||
failNextRetentionStatusRead,
|
||||
failRetentionAnalysis,
|
||||
readRetentionMessage,
|
||||
recoverRetentionStatusPolling,
|
||||
reloadRetentionWithFakeClock,
|
||||
resetRetention,
|
||||
RETENTION_ROUTE,
|
||||
runRetention,
|
||||
@@ -12,6 +16,38 @@ import {
|
||||
} from "../../helpers/tool-payload-retention";
|
||||
|
||||
test.describe("Tool payload retention on phones", () => {
|
||||
test("recovers a status polling error without manual refresh on a phone", async ({
|
||||
testPage: page,
|
||||
}) => {
|
||||
await page.setViewportSize({ width: 390, height: 844 });
|
||||
await resetRetention(page);
|
||||
await page.goto(RETENTION_ROUTE);
|
||||
await reloadRetentionWithFakeClock(page);
|
||||
const removeStatusRoute = await failNextRetentionStatusRead(page);
|
||||
try {
|
||||
await recoverRetentionStatusPolling(page);
|
||||
} finally {
|
||||
await removeStatusRoute();
|
||||
}
|
||||
});
|
||||
|
||||
test("preserves an action failure after status recovery on a phone", async ({
|
||||
testPage: page,
|
||||
}) => {
|
||||
await page.setViewportSize({ width: 390, height: 844 });
|
||||
await resetRetention(page);
|
||||
await page.goto(RETENTION_ROUTE);
|
||||
await reloadRetentionWithFakeClock(page);
|
||||
const removeStatusRoute = await failNextRetentionStatusRead(page);
|
||||
const removeActionRoute = await failRetentionAnalysis(page, true);
|
||||
try {
|
||||
await recoverRetentionStatusPolling(page, true);
|
||||
} finally {
|
||||
await removeActionRoute();
|
||||
await removeStatusRoute();
|
||||
}
|
||||
});
|
||||
|
||||
for (const width of [320, 390]) {
|
||||
test(`completes cleanup by touch at ${width}px without clipping`, async ({
|
||||
testPage: page,
|
||||
|
||||
@@ -5,7 +5,11 @@ import {
|
||||
activateRetention,
|
||||
analyzeRetention,
|
||||
PAYLOAD_TEXT,
|
||||
failNextRetentionStatusRead,
|
||||
failRetentionAnalysis,
|
||||
readRetentionMessage,
|
||||
recoverRetentionStatusPolling,
|
||||
reloadRetentionWithFakeClock,
|
||||
resetRetention,
|
||||
retentionStatus,
|
||||
RETENTION_ROUTE,
|
||||
@@ -15,6 +19,32 @@ import {
|
||||
} from "../../helpers/tool-payload-retention";
|
||||
|
||||
test.describe("Tool payload retention", () => {
|
||||
test("recovers a status polling error without manual refresh", async ({ testPage: page }) => {
|
||||
await resetRetention(page);
|
||||
await page.goto(RETENTION_ROUTE);
|
||||
await reloadRetentionWithFakeClock(page);
|
||||
const removeStatusRoute = await failNextRetentionStatusRead(page);
|
||||
try {
|
||||
await recoverRetentionStatusPolling(page);
|
||||
} finally {
|
||||
await removeStatusRoute();
|
||||
}
|
||||
});
|
||||
|
||||
test("preserves an action failure after status recovery", async ({ testPage: page }) => {
|
||||
await resetRetention(page);
|
||||
await page.goto(RETENTION_ROUTE);
|
||||
await reloadRetentionWithFakeClock(page);
|
||||
const removeStatusRoute = await failNextRetentionStatusRead(page);
|
||||
const removeActionRoute = await failRetentionAnalysis(page);
|
||||
try {
|
||||
await recoverRetentionStatusPolling(page, true);
|
||||
} finally {
|
||||
await removeActionRoute();
|
||||
await removeStatusRoute();
|
||||
}
|
||||
});
|
||||
|
||||
for (const choice of ["backup", "skip"] as const) {
|
||||
test(`analyzes while disabled and cleans old payloads after explicit ${choice}`, async ({
|
||||
testPage: page,
|
||||
|
||||
@@ -14,10 +14,12 @@ const status: ToolPayloadRetentionStatus = {
|
||||
};
|
||||
function deferred<T>() {
|
||||
let resolve!: (v: T) => void;
|
||||
const promise = new Promise<T>((r) => {
|
||||
let reject!: (cause: unknown) => void;
|
||||
const promise = new Promise<T>((r, j) => {
|
||||
resolve = r;
|
||||
reject = j;
|
||||
});
|
||||
return { promise, resolve };
|
||||
return { promise, resolve, reject };
|
||||
}
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
@@ -91,6 +93,77 @@ it("keeps saved policy on failure and releases the mutation lock for retry", asy
|
||||
});
|
||||
expect(result.current.error).toBeNull();
|
||||
});
|
||||
|
||||
it("clears a recovered status error on background polling", async () => {
|
||||
vi.useFakeTimers();
|
||||
let unmount: (() => void) | undefined;
|
||||
try {
|
||||
const readError = new Error("status unavailable");
|
||||
vi.mocked(api.fetchToolPayloadRetention)
|
||||
.mockResolvedValueOnce(status)
|
||||
.mockRejectedValueOnce(readError)
|
||||
.mockResolvedValueOnce(status);
|
||||
const rendered = renderHook(useToolPayloadRetention);
|
||||
unmount = rendered.unmount;
|
||||
await act(async () => {
|
||||
await Promise.resolve();
|
||||
await Promise.resolve();
|
||||
});
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(30_000);
|
||||
});
|
||||
expect(rendered.result.current.error).toBe(readError);
|
||||
await act(async () => {
|
||||
await vi.advanceTimersByTimeAsync(30_000);
|
||||
});
|
||||
expect(rendered.result.current.error).toBeNull();
|
||||
} finally {
|
||||
unmount?.();
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps action failures when a status read succeeds", async () => {
|
||||
const { result } = renderHook(useToolPayloadRetention);
|
||||
await waitFor(() => expect(result.current.status).toEqual(status));
|
||||
const actionError = new Error("analysis failed");
|
||||
vi.mocked(api.analyzeToolPayloadRetention).mockRejectedValueOnce(actionError);
|
||||
await act(async () => {
|
||||
await expect(result.current.analyze(status.policy.age)).rejects.toBe(actionError);
|
||||
});
|
||||
expect(result.current.error).toBe(actionError);
|
||||
vi.mocked(api.fetchToolPayloadRetention).mockResolvedValueOnce(status);
|
||||
await act(async () => {
|
||||
await result.current.reload();
|
||||
});
|
||||
expect(result.current.error).toBe(actionError);
|
||||
});
|
||||
|
||||
it("does not let a stale status failure replace a mutation result", async () => {
|
||||
const { result } = renderHook(useToolPayloadRetention);
|
||||
await waitFor(() => expect(result.current.status).toEqual(status));
|
||||
const old = deferred<ToolPayloadRetentionStatus>();
|
||||
vi.mocked(api.fetchToolPayloadRetention).mockReturnValueOnce(old.promise);
|
||||
let reload!: Promise<void>;
|
||||
act(() => {
|
||||
reload = result.current.reload();
|
||||
});
|
||||
const saved = {
|
||||
...status,
|
||||
policy: { ...status.policy, revision: 1, age: { value: 4, unit: "months" as const } },
|
||||
};
|
||||
vi.mocked(api.saveToolPayloadRetention).mockResolvedValue(saved);
|
||||
await act(async () => {
|
||||
await result.current.save(saved.policy);
|
||||
});
|
||||
const staleError = new Error("stale read failed");
|
||||
await act(async () => {
|
||||
old.reject(staleError);
|
||||
await reload;
|
||||
});
|
||||
expect(result.current.status).toEqual(saved);
|
||||
expect(result.current.error).toBeNull();
|
||||
});
|
||||
it("cancels the accepted operation and applies the returned status", async () => {
|
||||
const { result } = renderHook(useToolPayloadRetention);
|
||||
await waitFor(() => expect(result.current.status).toEqual(status));
|
||||
|
||||
@@ -15,7 +15,11 @@ type Lifetime = {
|
||||
generation: number;
|
||||
};
|
||||
type AcceptedOperation = { id: string; kind: "analysis" | "cleanup" };
|
||||
type Updates = { pending: (value: boolean) => void; error: (cause: unknown) => void };
|
||||
type Updates = {
|
||||
pending: (value: boolean) => void;
|
||||
actionError: (cause: unknown) => void;
|
||||
clearErrors: () => void;
|
||||
};
|
||||
|
||||
function operationObserved(status: ToolPayloadRetentionStatus, accepted: AcceptedOperation) {
|
||||
if (status.operation?.id === accepted.id) return true;
|
||||
@@ -26,7 +30,8 @@ function operationObserved(status: ToolPayloadRetentionStatus, accepted: Accepte
|
||||
function loadStatus(
|
||||
owner: Lifetime,
|
||||
accept: (value: ToolPayloadRetentionStatus) => void,
|
||||
fail: Updates["error"],
|
||||
recover: () => void,
|
||||
fail: (cause: unknown) => void,
|
||||
) {
|
||||
if (owner.mutating) return Promise.resolve();
|
||||
if (owner.reading) return owner.reading;
|
||||
@@ -35,7 +40,10 @@ function loadStatus(
|
||||
const request = api
|
||||
.fetchToolPayloadRetention()
|
||||
.then((next) => {
|
||||
if (current()) accept(next);
|
||||
if (current()) {
|
||||
accept(next);
|
||||
recover();
|
||||
}
|
||||
})
|
||||
.catch((cause: unknown) => {
|
||||
if (current()) fail(cause);
|
||||
@@ -60,13 +68,13 @@ async function performMutation<T>(
|
||||
const { epoch } = owner;
|
||||
const current = () => owner.mounted && owner.epoch === epoch;
|
||||
updates.pending(true);
|
||||
updates.error(null);
|
||||
updates.clearErrors();
|
||||
try {
|
||||
const result = await request();
|
||||
if (current()) accept(result);
|
||||
return result;
|
||||
} catch (cause) {
|
||||
if (current()) updates.error(cause);
|
||||
if (current()) updates.actionError(cause);
|
||||
throw cause;
|
||||
} finally {
|
||||
owner.mutating = false;
|
||||
@@ -113,9 +121,29 @@ function useRetentionLifetime(owner: { current: Lifetime }, reload: () => Promis
|
||||
};
|
||||
}, [owner, reload]);
|
||||
}
|
||||
function useRetentionMutation(
|
||||
owner: { current: Lifetime },
|
||||
setPending: (value: boolean) => void,
|
||||
setStatusError: (value: unknown) => void,
|
||||
setActionError: (value: unknown) => void,
|
||||
) {
|
||||
return useCallback(
|
||||
<T>(request: () => Promise<T>, accept: (value: T) => void) =>
|
||||
performMutation(owner.current, request, accept, {
|
||||
pending: setPending,
|
||||
actionError: setActionError,
|
||||
clearErrors: () => {
|
||||
setStatusError(null);
|
||||
setActionError(null);
|
||||
},
|
||||
}),
|
||||
[owner, setActionError, setPending, setStatusError],
|
||||
);
|
||||
}
|
||||
export function useToolPayloadRetention() {
|
||||
const [status, setStatus] = useState<ToolPayloadRetentionStatus | null>(null);
|
||||
const [error, setError] = useState<unknown>(null);
|
||||
const [statusError, setStatusError] = useState<unknown>(null);
|
||||
const [actionError, setActionError] = useState<unknown>(null);
|
||||
const [pending, setPending] = useState(false);
|
||||
const [acceptedId, setAcceptedId] = useState<string | null>(null);
|
||||
const acceptedOperation = useRef<AcceptedOperation | null>(null);
|
||||
@@ -138,12 +166,14 @@ export function useToolPayloadRetention() {
|
||||
setAcceptedId(null);
|
||||
}
|
||||
},
|
||||
setError,
|
||||
() => setStatusError(null),
|
||||
setStatusError,
|
||||
),
|
||||
[],
|
||||
);
|
||||
const refresh = useCallback(() => {
|
||||
setError(null);
|
||||
setStatusError(null);
|
||||
setActionError(null);
|
||||
return reload();
|
||||
}, [reload]);
|
||||
useAcceptedOperationRefresh(acceptedId, reload);
|
||||
@@ -152,11 +182,7 @@ export function useToolPayloadRetention() {
|
||||
status?.preparation.state === "pending" || status?.preparation.state === "running";
|
||||
const active = Boolean(acceptedId || preparing || status?.operation?.state === "running");
|
||||
useStatusPolling(reload, active, preparing);
|
||||
const perform = useCallback(
|
||||
<T>(request: () => Promise<T>, accept: (value: T) => void) =>
|
||||
performMutation(owner.current, request, accept, { pending: setPending, error: setError }),
|
||||
[],
|
||||
);
|
||||
const perform = useRetentionMutation(owner, setPending, setStatusError, setActionError);
|
||||
const acceptStatus = useCallback((next: ToolPayloadRetentionStatus) => {
|
||||
acceptedOperation.current = null;
|
||||
setStatus(next);
|
||||
@@ -196,7 +222,7 @@ export function useToolPayloadRetention() {
|
||||
);
|
||||
return {
|
||||
status,
|
||||
error,
|
||||
error: actionError ?? statusError,
|
||||
pending,
|
||||
active,
|
||||
preparing,
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
# ADR-2026-09-17-maintenance-health-probe-coordination: Coordinate runtime health probes with maintenance
|
||||
|
||||
**Status:** accepted
|
||||
**Date:** 2026-09-17
|
||||
**Area:** backend
|
||||
|
||||
## Context
|
||||
|
||||
SQLite has one writer connection. Vacuum holds that connection while runtime
|
||||
health probes need it for pings and schema checks. Probe deadlines therefore
|
||||
mark every required store unhealthy during successful maintenance.
|
||||
|
||||
The shared pool already has a maintenance admission guard. Required persistence
|
||||
must still fail startup and retain known runtime failures.
|
||||
|
||||
## Decision
|
||||
|
||||
Periodic SQLite probes participate in the existing maintenance guard. They try
|
||||
admission without waiting and retain the lease throughout the bounded probe.
|
||||
A busy guard defers the probe without changing store states or check timestamps.
|
||||
Startup checks remain strict. PostgreSQL probes remain unchanged.
|
||||
|
||||
The next scheduled tick resumes full probing after maintenance releases its lease.
|
||||
Deferred probes emit a bounded debug event. They never manufacture healthy state.
|
||||
Destructive SQLite restore and factory-reset operations use the existing
|
||||
unhealthy state to fail closed before quiescing or replacing data. Their
|
||||
restart-required result keeps database-backed work disabled until a fresh
|
||||
process initializes the database again.
|
||||
|
||||
## Consequences
|
||||
|
||||
Managed maintenance no longer causes false global persistence failures.
|
||||
The guard prevents a race between checking maintenance state and taking the writer.
|
||||
Actual faults that arise during maintenance can remain undetected until lease release.
|
||||
Diagnostics retain the last completed check time, and previous failures remain visible.
|
||||
This does not guarantee that writes complete promptly during vacuum.
|
||||
|
||||
The [required persistence decision](2026-09-05-required-internal-persistence.md)
|
||||
still governs startup and actual runtime failures.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
- Longer timeouts cannot bound vacuum duration and retain the false-failure race.
|
||||
- Reader-only checks lose the existing writer-health guarantee.
|
||||
- Skipping every probe while any job runs confuses job tracking with database ownership.
|
||||
- Bypassing HTTP health middleware hides actual failures and does not repair readiness.
|
||||
- A separate maintenance flag duplicates the existing admission state.
|
||||
@@ -0,0 +1,190 @@
|
||||
---
|
||||
created: 2026-09-17
|
||||
status: done
|
||||
requirements:
|
||||
- REQ-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007
|
||||
- REQ-SYSTEM-PAGE-TOOL-PAYLOAD-RETENTION-003
|
||||
system_design:
|
||||
- ../../specs/platform/system-design/postgres-domain-store-parity.md
|
||||
- ../../specs/system-page/system-design/tool-payload-retention.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Implementation Plan: Vacuum and compaction status recovery
|
||||
|
||||
## Overview
|
||||
|
||||
Prevent false persistence failures during managed SQLite maintenance. Clear a
|
||||
recovered compaction status error without hiding a failed user action.
|
||||
Implement the runtime guard first, then the independent status recovery path.
|
||||
Both work orders require TDD. Implementation is complete.
|
||||
|
||||
## Evidence and requirement conformance
|
||||
|
||||
On September 17, the retained backend log recorded these local times:
|
||||
|
||||
| Time (UTC+01:00) | Observation |
|
||||
| --- | --- |
|
||||
| 11:00:30 | Vacuum accepted with HTTP 202 |
|
||||
| 11:00:44 | Writer ping exceeded its deadline and persistence became unhealthy |
|
||||
| 11:00:51 | Compaction status GET returned HTTP 503 |
|
||||
| 11:02:09 | A second vacuum was accepted |
|
||||
| 11:02:36 through 11:05:07 | Status GETs returned HTTP 503 |
|
||||
| 11:05:38 | Status GET recovered to HTTP 200 |
|
||||
|
||||
Source: `/root/.kandev/logs/backend-logs-2026-09-17-000011.log` on the
|
||||
investigated host. This path is evidence, not an implementation dependency.
|
||||
|
||||
`database.Service.runVacuum` owns the shared maintenance lease and SQLite's
|
||||
single writer. `requiredstores.Health` independently pings that writer with
|
||||
a two-second deadline. `requiredPersistenceMiddleware` then blocks stateful
|
||||
requests with `persistence_unavailable`. The card maps that response to its
|
||||
generic error. `loadStatus` records GET errors but never clears them on success.
|
||||
|
||||
The existing requirements describe health recovery and visible operation states.
|
||||
They omit maintenance contention and read-error dismissal. This package adds
|
||||
criteria to those existing capabilities and amends their technical designs.
|
||||
Platform owns shared persistence health. System-page owns compaction status
|
||||
recovery because it owns the policy and its action lifecycle.
|
||||
|
||||
The assumption check found no unresolved product choice. Scope follows the
|
||||
diagnosed failure and preserves strict startup checks, real errors, and policy.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Runtime probe admission through the existing SQLite maintenance guard.
|
||||
- State and timestamp preservation when a periodic probe defers.
|
||||
- Automatic read-error recovery with separate action-error ownership.
|
||||
- Deterministic backend, hook, component, and desktop/phone browser regressions.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- Database migrations, vacuum optimization, automatic vacuum, or payload changes.
|
||||
- New public APIs, timeouts, feature flags, settings, or translated copy.
|
||||
- General HTTP retry behavior or a new maintenance banner.
|
||||
- Production database operations, push, or PR creation in this turn.
|
||||
|
||||
## Technical approach
|
||||
|
||||
The [platform design](../../specs/platform/system-design/postgres-domain-store-parity.md#sqlite-maintenance-coordination)
|
||||
and [decision](../../decisions/2026-09-17-maintenance-health-probe-coordination.md)
|
||||
define runtime admission. Add a narrow periodic-check boundary in `health.go`.
|
||||
Use `maintenance.ForPool` and `TryAcquire` only for SQLite runtime checks.
|
||||
Keep `Health.Check` strict for startup. Hold the lease for the whole check.
|
||||
Keep existing middleware and readiness behavior for actual failures.
|
||||
|
||||
The [compaction design](../../specs/system-page/system-design/tool-payload-retention.md#status-error-recovery)
|
||||
defines error ownership. Split read and action errors inside the existing hook.
|
||||
Keep the outward `error` field and existing component error translation.
|
||||
Honor lifetime and generation checks before accepting or clearing any result.
|
||||
|
||||
## ASCII UI preview
|
||||
|
||||
### UI-01: Compaction error recovery
|
||||
|
||||
Entry: Settings > System > Data & Logs > Database.
|
||||
The preview shows only the affected region. Spacing is illustrative.
|
||||
|
||||
```text
|
||||
Current, after a GET recovers:
|
||||
Messages compaction
|
||||
[The operation could not complete. Refresh status and retry.]
|
||||
[Refresh status]
|
||||
Tasks inactive for [3] [Months v] [Analyze savings]
|
||||
|
||||
Proposed, after a GET recovers:
|
||||
Messages compaction
|
||||
Tasks inactive for [3] [Months v] [Analyze savings]
|
||||
...
|
||||
|
||||
Proposed, unresolved action failure:
|
||||
Messages compaction
|
||||
[Existing translated action error]
|
||||
[Refresh status]
|
||||
Tasks inactive for [3] [Months v] [Analyze savings]
|
||||
```
|
||||
|
||||
On phones, the number and unit remain adjacent. Analyze savings stays on its
|
||||
own full-width row. The existing page owns scrolling and touch targets remain
|
||||
44px. Error visibility follows the same hook on both viewports. No overlay,
|
||||
navigation, focus, breakpoint, or layout change is planned.
|
||||
The shipped `ToolPayloadRetentionCard` and its mobile E2E are the local exemplar.
|
||||
The mobile guide's shared-state rule applies. A separate phone component is unnecessary.
|
||||
|
||||
Required structure: remove only recovered read errors. Keep failed actions and
|
||||
persisted failures visible. This maps to `AC-SYSTEM-PAGE-TOOL-PAYLOAD-RETENTION-003.7`
|
||||
and `.8`, with existing phone parity criterion `.5`.
|
||||
|
||||
## Tests
|
||||
|
||||
| Criteria | Planned evidence |
|
||||
| --- | --- |
|
||||
| Platform 007.6 | `TestRuntimeHealthDefersDuringMaintenance` in `health_test.go` |
|
||||
| Platform 007.7 | `TestRuntimeHealthDeferralPreservesMixedStoreStates` and `TestStartupHealthDoesNotDefer` |
|
||||
| Platform 007.8, existing 007.1/.3/.4 | `TestRuntimeHealthResumesAfterMaintenance`, existing missing-table recovery, and middleware regression |
|
||||
| System-page 003.7 | Hook test `clears a recovered status error on background polling` and rendered card recovery |
|
||||
| System-page 003.8 | Hook tests for action-error precedence, stale GET rejection, and persisted failure preservation |
|
||||
| System-page 003.5/.7/.8 | Desktop and mobile recovery scenarios in the existing retention specs |
|
||||
|
||||
At planning time, the full prefixes are in the work-order frontmatter. New test names were planned,
|
||||
not existing evidence. Use a held real writer connection and guard for a
|
||||
deterministic backend reproduction. Do not depend on a large database making
|
||||
vacuum slow. The guard fixture reproduces the proven resource conflict.
|
||||
|
||||
## E2E tests
|
||||
|
||||
Extend `apps/web/e2e/tests/system/tool-payload-retention.spec.ts` (`chromium`)
|
||||
and `mobile-tool-payload-retention.spec.ts` (`mobile-chrome`). Add status GET
|
||||
failure followed by success without Refresh status, and action failure followed
|
||||
by a successful GET. Keep real route rendering and existing mutation scenarios.
|
||||
Use controlled route responses and browser clock advancement for polling.
|
||||
Await the specific HTTP responses before DOM assertions. No fixed sleeps.
|
||||
This browser evidence covers presentation recovery. Backend tests cover the
|
||||
maintenance cause without exposing production test controls.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Coordinate runtime health with maintenance](task-01-runtime-health.md)
|
||||
- [x] [Task 02: Recover compaction status errors](task-02-status-recovery.md)
|
||||
|
||||
Execute sequentially. Task 02 does not technically depend on Task 01, but both
|
||||
must pass before delivery. No delegation is authorized.
|
||||
|
||||
## Documentation and companion packages
|
||||
|
||||
The completed [retention package](../tool-payload-retention/plan.md) and
|
||||
[store parity package](../postgres-domain-store-parity/plan.md) retain their
|
||||
historical results. This follow-up owns only the new criteria and regressions.
|
||||
Public docs need no change for this implementation. Existing health/recovery
|
||||
guidance was checked for claims affected by probe deferral. No labels change.
|
||||
|
||||
## Verification results
|
||||
|
||||
Implementation completed on 2026-09-17. Artifact validation on 2026-09-17:
|
||||
|
||||
- `python3 scripts/list-docs.py validate`: passed (286 decisions, 986 specifications).
|
||||
- `python3 scripts/lint-spec-files.test.py`: passed (36 tests).
|
||||
- `python3 scripts/lint-spec-files.py --all`: passed.
|
||||
- `git diff --check -- docs/specs docs/decisions docs/plans/vacuum-compaction-status`: passed.
|
||||
- Backend focused race tests passed for required-store health, maintenance,
|
||||
database, and persistence middleware.
|
||||
- Frontend focused tests passed: 23 tests across the hook and card files.
|
||||
- `pnpm run typecheck`: passed.
|
||||
- Desktop retention E2E passed: 4 tests on `chromium`.
|
||||
- Phone retention E2E passed: 4 tests on `mobile-chrome`, including the
|
||||
existing focused phone screenshot capture at 390px.
|
||||
- `git diff --check`: passed.
|
||||
|
||||
Product tests and browser checks from both work orders passed. The worktree
|
||||
contains the implementation and its permanent regression coverage; no push or
|
||||
PR was created.
|
||||
|
||||
## Risks
|
||||
|
||||
- Faults that arise during a maintenance lease remain undetected until its release.
|
||||
Prior unhealthy state and completed-check timestamps remain unchanged.
|
||||
- Restore/reset quiescence must continue to govern process teardown and readiness.
|
||||
- Clearing the shared error unconditionally would hide failed mutations.
|
||||
- A guard acquired only around the ping leaves schema checks exposed to the same race.
|
||||
@@ -0,0 +1,127 @@
|
||||
---
|
||||
id: "01-runtime-health"
|
||||
title: "Coordinate runtime health with maintenance"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007
|
||||
acceptance_criteria:
|
||||
- AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.1
|
||||
- AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.3
|
||||
- AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.4
|
||||
- AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.6
|
||||
- AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.7
|
||||
- AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.8
|
||||
system_design:
|
||||
- ../../specs/platform/system-design/postgres-domain-store-parity.md
|
||||
---
|
||||
|
||||
# Task 01: Coordinate runtime health with maintenance
|
||||
|
||||
## Summary
|
||||
|
||||
Prevent managed SQLite maintenance from causing a false unhealthy transition.
|
||||
Keep startup strict and preserve actual runtime failure detection.
|
||||
|
||||
## In scope
|
||||
|
||||
- Add a periodic-probe boundary that obtains the existing maintenance lease.
|
||||
- Defer busy SQLite probes without changing tracker state or timestamps.
|
||||
- Preserve the two-second check budget, 15-second interval, and PostgreSQL path.
|
||||
- Emit a debug deferral event and release acquired leases on every exit.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- HTTP allowlist changes, writer-pool sizing, schema changes, and UI changes.
|
||||
- Suppression of ordinary database failures outside managed maintenance.
|
||||
|
||||
## Acceptance
|
||||
|
||||
1. A held maintenance lease and writer do not change health or check timestamps.
|
||||
2. A mixed healthy/unhealthy catalog retains every state during deferral. Startup never skips its check.
|
||||
3. After release, full checks resume. Missing tables still cause HTTP 503 and recover after repair.
|
||||
|
||||
## TDD and test design
|
||||
|
||||
First add `TestRuntimeHealthDefersDuringMaintenance` in `health_test.go`.
|
||||
Use a real SQLite pool, initialized tracker, maintenance lease, and held writer.
|
||||
Drive the periodic-check boundary with a short test context. Before the fix,
|
||||
the probe times out and records unhealthy state. Assert that failure first.
|
||||
|
||||
Add `TestRuntimeHealthDeferralPreservesMixedStoreStates` with one healthy and
|
||||
one unhealthy store. Compare complete snapshots, including check timestamps.
|
||||
Add `TestStartupHealthDoesNotDefer` against strict `Health.Check`.
|
||||
Add `TestRuntimeHealthResumesAfterMaintenance` for release, fault detection,
|
||||
and repair. Assert lease release after successful and failed probes, and stop
|
||||
cancellation. Keep `TestProbeTablesHonorsContextDeadline` unchanged in meaning.
|
||||
|
||||
Extend `persistence_middleware_test.go` with
|
||||
`TestPersistenceMiddlewareRemainsAvailableDuringManagedMaintenance`.
|
||||
Exercise the periodic boundary, middleware, and real health tracker together.
|
||||
Use the existing middleware harness, not a synthetic always-healthy stub.
|
||||
Do not export an API solely for tests. An external-package integration test
|
||||
can drive `Start` with `SetInterval` and synchronize on observable probe events.
|
||||
Retain the existing unhealthy response and recovery tests.
|
||||
|
||||
## Verification
|
||||
|
||||
Run from the repository root:
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test -race ./internal/persistence/requiredstores ./internal/system/maintenance ./internal/system/database)
|
||||
(cd apps/backend && go test -race ./internal/backendapp -run 'Test.*(Persistence|RequiredStore|Health|Ready)')
|
||||
git diff --check
|
||||
```
|
||||
|
||||
The backend Makefile's generic test target is broader than this repair.
|
||||
Use these focused package commands and verify that the new test names run.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/persistence/requiredstores/health.go`
|
||||
- `apps/backend/internal/persistence/requiredstores/health_test.go`
|
||||
- `apps/backend/internal/backendapp/persistence_middleware_test.go`
|
||||
- `apps/backend/internal/system/maintenance/guard_test.go` if lease assertions need extension
|
||||
|
||||
## Dependencies
|
||||
|
||||
None. Existing maintenance owners already use the shared pool guard.
|
||||
|
||||
## Risks
|
||||
|
||||
Do not reverse lock order. The probe takes maintenance admission before database
|
||||
connections and releases it after all database work. Never hold a writer while
|
||||
waiting for maintenance admission. Preserve restore/reset shutdown behavior.
|
||||
The lease defers fault detection only while managed maintenance owns the pool.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- [Platform requirements](../../specs/platform/requirements/postgres-domain-store-parity.md), required-store health.
|
||||
- [Platform design](../../specs/platform/system-design/postgres-domain-store-parity.md), runtime health.
|
||||
- [Decision](../../decisions/2026-09-17-maintenance-health-probe-coordination.md).
|
||||
- Existing `health_test.go`, `guard_test.go`, and `persistence_middleware_test.go` patterns.
|
||||
|
||||
## Results
|
||||
|
||||
Implemented periodic SQLite health-probe admission through the shared
|
||||
maintenance guard. A busy guard now defers without changing tracker state or
|
||||
timestamps, while startup `Health.Check` remains strict and PostgreSQL keeps
|
||||
the existing path. Added real SQLite contention coverage for mixed state
|
||||
preservation, recovery, lease release, and middleware availability.
|
||||
|
||||
Review follow-up also wires destructive SQLite restore and factory-reset
|
||||
operations to mark required persistence unhealthy before quiescence or data
|
||||
replacement. Stateful middleware therefore fails closed while the accepted
|
||||
operation waits for the required frontend restart.
|
||||
|
||||
Verification passed:
|
||||
|
||||
- `go test -race ./internal/persistence/requiredstores ./internal/system/maintenance ./internal/system/database`
|
||||
- `go test -race ./internal/backendapp -run 'Test.*(Persistence|RequiredStore|Health|Ready)'`
|
||||
- `git diff --check`
|
||||
@@ -0,0 +1,156 @@
|
||||
---
|
||||
id: "02-status-recovery"
|
||||
title: "Recover compaction status errors"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-SYSTEM-PAGE-TOOL-PAYLOAD-RETENTION-003
|
||||
acceptance_criteria:
|
||||
- AC-SYSTEM-PAGE-TOOL-PAYLOAD-RETENTION-003.5
|
||||
- AC-SYSTEM-PAGE-TOOL-PAYLOAD-RETENTION-003.7
|
||||
- AC-SYSTEM-PAGE-TOOL-PAYLOAD-RETENTION-003.8
|
||||
system_design:
|
||||
- ../../specs/system-page/system-design/tool-payload-retention.md
|
||||
---
|
||||
|
||||
# Task 02: Recover compaction status errors
|
||||
|
||||
## Summary
|
||||
|
||||
Clear a recovered status-read error on the next successful background poll.
|
||||
Keep action errors independent so a successful GET cannot hide a failed command.
|
||||
|
||||
## In scope
|
||||
|
||||
- Separate read and action errors in `useToolPayloadRetention`.
|
||||
- Preserve its public `error` field, error translation, and manual dismissal behavior.
|
||||
- Keep lifetime, generation, pending, accepted-operation, and draft protections.
|
||||
- Extend hook, rendered component, desktop, and phone regression coverage.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- New layout, user-facing copy, retry policy, polling cadence, and backend APIs.
|
||||
- Automatic retries of save, analysis, cleanup, or cancellation.
|
||||
|
||||
## Acceptance
|
||||
|
||||
1. Failed GET then successful background GET removes the banner without clicking Refresh status.
|
||||
2. Successful GET retains action errors and persisted operation failures. Stale GET results change neither errors nor policy.
|
||||
3. Desktop and phone preserve policy edits, existing controls, and error recovery without duplicate mutations.
|
||||
|
||||
## ASCII UI preview
|
||||
|
||||
### UI-01: Compaction error recovery
|
||||
|
||||
Entry: Data & Logs > Database. Full preview: [plan](plan.md#ascii-ui-preview).
|
||||
|
||||
```text
|
||||
Read failure: Messages compaction
|
||||
[Existing error] [Refresh status]
|
||||
Recovery: Messages compaction
|
||||
Tasks inactive for [3] [Months v] [Analyze savings]
|
||||
Action error: Messages compaction
|
||||
[Existing action error] [Refresh status]
|
||||
```
|
||||
|
||||
Phones retain adjacent age/unit inputs and a separate full-width Analyze savings
|
||||
row. The existing route owns scrolling. Touch targets and focus behavior remain
|
||||
unchanged. Banner removal is required only for recovered reads (003.7).
|
||||
Action and persisted errors remain visible (003.8). Spacing is illustrative.
|
||||
|
||||
## TDD and test design
|
||||
|
||||
First add `clears a recovered status error on background polling` to the hook
|
||||
test file. Use fake timers: initial success, failed idle poll, successful idle
|
||||
poll. Assert that the current implementation leaves the error set.
|
||||
|
||||
Add cases for initial GET failure, repeated failures, action-error precedence,
|
||||
successful GET after failed save/analyze/run/cancel, and persisted failed status.
|
||||
Use deferred responses to cover stale GET success and failure around mutations,
|
||||
unmount, and remount. Preserve existing same-tick command serialization tests.
|
||||
Assert that recovery creates no mutation request and does not change drafts.
|
||||
|
||||
Add a rendered component regression with the real hook and mocked API module.
|
||||
Do not prove recovery by directly replacing the mocked hook's error value.
|
||||
|
||||
Add `recovers status polling without manual refresh` and
|
||||
`preserves action failures after status recovery` to both existing E2E files.
|
||||
Extract response scripting into the existing retention helper when shared.
|
||||
Intercept only the intended endpoint and HTTP method. Return a real successful
|
||||
status response after the injected failure. Advance the browser clock to the
|
||||
next poll, then await the specific GET before checking the banner.
|
||||
Use `.tap()` for phone actions. Preserve all existing cleanup and geometry cases.
|
||||
|
||||
## Verification
|
||||
|
||||
Run from the repository root. Install workspace dependencies once when absent.
|
||||
|
||||
```bash
|
||||
(cd apps && pnpm install --frozen-lockfile)
|
||||
(cd apps/web && pnpm exec vitest run hooks/domains/system/use-tool-payload-retention.test.ts components/settings/system/tool-payload-retention-card.test.tsx)
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
(cd apps/web && pnpm e2e:run --project chromium tests/system/tool-payload-retention.spec.ts)
|
||||
(cd apps/web && pnpm e2e:run --project mobile-chrome tests/system/mobile-tool-payload-retention.spec.ts)
|
||||
python3 scripts/list-docs.py validate
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
git diff --check
|
||||
```
|
||||
|
||||
Use managed E2E builds and run projects sequentially. Record discovered test
|
||||
counts, results, and a focused phone screenshot. Do not use production data.
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/hooks/domains/system/use-tool-payload-retention.ts`
|
||||
- `apps/web/hooks/domains/system/use-tool-payload-retention.test.ts`
|
||||
- `apps/web/components/settings/system/tool-payload-retention-card.test.tsx`
|
||||
- `apps/web/e2e/tests/system/tool-payload-retention.spec.ts`
|
||||
- `apps/web/e2e/tests/system/mobile-tool-payload-retention.spec.ts`
|
||||
- `apps/web/e2e/helpers/tool-payload-retention.ts`
|
||||
|
||||
## Dependencies
|
||||
|
||||
No code dependency on Task 01. Execute after Task 01 as the default package order.
|
||||
|
||||
## Risks
|
||||
|
||||
A shared unconditional `setError(null)` loses action failures. Keep error origin
|
||||
explicit. Apply the existing generation guard before any error update.
|
||||
Controlled browser failures prove UI recovery, not backend maintenance admission.
|
||||
Task 01 supplies that independent evidence.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- [Requirements](../../specs/system-page/requirements/tool-payload-retention.md), background operation and access.
|
||||
- [Design](../../specs/system-page/system-design/tool-payload-retention.md), status error recovery.
|
||||
- Existing hook, component, and retention E2E tests.
|
||||
|
||||
## Results
|
||||
|
||||
Implemented separate status-read and action error ownership in
|
||||
`useToolPayloadRetention`. A successful background status poll now clears only
|
||||
the recovered read error. Mutation failures and persisted failed-operation
|
||||
states remain visible, while stale status responses continue to obey the
|
||||
existing generation and operation guards.
|
||||
|
||||
Added hook and rendered-card regressions for read recovery, action-error
|
||||
preservation, and stale response ordering. Added controlled desktop and phone
|
||||
browser scenarios that fail one status read, recover on the next poll, and
|
||||
exercise touch actions on mobile. The existing phone cleanup test continues to
|
||||
capture the focused 390px card screenshot.
|
||||
|
||||
Verification passed:
|
||||
|
||||
- `pnpm exec vitest run hooks/domains/system/use-tool-payload-retention.test.ts components/settings/system/tool-payload-retention-card.test.tsx` (23 tests)
|
||||
- `pnpm run typecheck`
|
||||
- `pnpm e2e:run --project chromium tests/system/tool-payload-retention.spec.ts` (4 tests)
|
||||
- `pnpm e2e:run --project mobile-chrome tests/system/mobile-tool-payload-retention.spec.ts` (4 tests)
|
||||
- `python3 scripts/list-docs.py validate`
|
||||
- `python3 scripts/lint-spec-files.py --all`
|
||||
- `git diff --check`
|
||||
@@ -105,6 +105,9 @@ Each required store must initialize before Kandev becomes ready. External provid
|
||||
- **AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.3:** When a stateful caller reaches an unhealthy backend, it shall receive `503` with the stable code `persistence_unavailable` and a recovery action.
|
||||
- **AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.4:** When runtime database health recovers, readiness and stateful requests shall recover without a process restart.
|
||||
- **AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.5:** Required-store health shall not change the liveness contract of `GET /health`.
|
||||
- **AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.6:** Managed SQLite maintenance alone shall not change healthy persistence to unhealthy because it occupies the database writer.
|
||||
- **AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.7:** A deferred maintenance-time health check shall preserve each store's previous state and last check time. It shall not establish startup readiness or clear an existing failure.
|
||||
- **AC-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-007.8:** After maintenance releases the database, periodic health checks shall resume within the normal check interval. Actual database failures shall retain the existing readiness and caller error behavior.
|
||||
|
||||
### REQ-PLATFORM-POSTGRES-DOMAIN-STORE-PARITY-008: Contributor workflow
|
||||
|
||||
|
||||
@@ -106,6 +106,40 @@ One probe error makes aggregate persistence unhealthy. A later successful probe
|
||||
|
||||
Initialization errors remain fatal and cannot recover inside the same process. The process exits through the existing startup error path.
|
||||
|
||||
### SQLite maintenance coordination
|
||||
|
||||
Periodic SQLite checks use `maintenance.ForPool(pool).TryAcquire()` before
|
||||
`Health.Check`. The probe holds this lease until all checks finish. This prevents
|
||||
maintenance from taking the writer between a guard check and a database probe.
|
||||
The existing two-second probe context bounds the lease duration.
|
||||
|
||||
When admission fails, the periodic loop defers that check until its next tick.
|
||||
It does not call `RecordProbe`, change store states, or advance check timestamps.
|
||||
It emits a debug event with reason `maintenance_busy`, without database paths.
|
||||
An unhealthy store remains unhealthy, including alongside healthy stores.
|
||||
Ordinary writer contention without a maintenance lease retains current behavior.
|
||||
|
||||
Destructive SQLite restore and factory-reset operations notify required-store
|
||||
health before they quiesce workers or replace/drop data. The notifier records
|
||||
the existing `unhealthy` state while the maintenance lease is still held, so
|
||||
stateful middleware fails closed before the operation releases admission. The
|
||||
operation's `restart_required` result then requires a fresh process before
|
||||
database-backed work can resume. This does not add a new health state.
|
||||
|
||||
Startup calls to `Health.Check` remain strict and never use the deferral path.
|
||||
Missing pools remain failures. PostgreSQL uses its current probe path.
|
||||
The shared guard also covers backups, restore/reset, and retention batches.
|
||||
Their existing shutdown, quiescence, and error handling remain authoritative.
|
||||
There is no HTTP allowlist change or new health state.
|
||||
|
||||
After lease release, the next periodic tick performs a full probe within the
|
||||
normal 15-second interval. A failed operation cannot leave the lease held.
|
||||
Fault detection can be delayed for the duration of managed maintenance.
|
||||
Existing diagnostic timestamps expose the age of the last completed check.
|
||||
|
||||
See [maintenance probe coordination](../../../decisions/2026-09-17-maintenance-health-probe-coordination.md)
|
||||
and the [fix package](../../../plans/vacuum-compaction-status/plan.md).
|
||||
|
||||
## Caller errors
|
||||
|
||||
`GET /health` remains a pure liveness endpoint. Persistence state does not change its status or body.
|
||||
|
||||
@@ -120,6 +120,12 @@ rules, so that I understand the first cleanup and its recovery limits.
|
||||
- **AC-SYSTEM-PAGE-TOOL-PAYLOAD-RETENTION-003.6:** The first version shall support
|
||||
SQLite. Other database engines shall show an unavailable state and reject
|
||||
analysis and mutation requests without side effects.
|
||||
- **AC-SYSTEM-PAGE-TOOL-PAYLOAD-RETENTION-003.7:** After a status request fails,
|
||||
the next successful background status request shall clear that request error
|
||||
without a manual refresh. Saved policy and unsaved edits shall remain intact.
|
||||
- **AC-SYSTEM-PAGE-TOOL-PAYLOAD-RETENTION-003.8:** A successful status request
|
||||
shall not dismiss an unresolved user-action error or a recorded failed operation.
|
||||
Recovery shall not retry mutations or enable compaction automatically.
|
||||
|
||||
## Out of scope
|
||||
|
||||
@@ -132,3 +138,4 @@ rules, so that I understand the first cleanup and its recovery limits.
|
||||
|
||||
- [System design](../system-design/tool-payload-retention.md)
|
||||
- [Implementation plan](../../../plans/tool-payload-retention/plan.md)
|
||||
- [Vacuum and status recovery fix](../../../plans/vacuum-compaction-status/plan.md)
|
||||
|
||||
@@ -259,6 +259,29 @@ Payload bytes are not a prediction of compacted file bytes or backup duration.
|
||||
|
||||
## Settings surface and security
|
||||
|
||||
### Status error recovery
|
||||
|
||||
`useToolPayloadRetention` keeps status-read errors separate from action errors.
|
||||
A current successful GET clears only the status-read error. An action error
|
||||
takes display precedence and survives successful background reads. Explicit
|
||||
Refresh status and a new user action retain their existing dismissal behavior.
|
||||
Persisted operation failures remain part of the returned status.
|
||||
|
||||
The existing lifetime epoch and mutation generation checks apply to both error
|
||||
channels. A stale GET cannot clear a newer error or overwrite a mutation result.
|
||||
Polling continues at the existing cadence without automatic mutation retries.
|
||||
The hook keeps its outward `error` contract for `RetentionError`.
|
||||
Draft ownership stays in `useToolPayloadRetentionDraft`.
|
||||
|
||||
The [platform health design](../../platform/system-design/postgres-domain-store-parity.md#sqlite-maintenance-coordination)
|
||||
owns maintenance admission and readiness. The card does not infer database
|
||||
health from a local vacuum button state or suppress all HTTP 503 errors.
|
||||
Existing desktop and phone composition, focus behavior, and translated copy
|
||||
remain unchanged. The [fix package](../../../plans/vacuum-compaction-status/plan.md)
|
||||
defines regression coverage and the recovered-state preview.
|
||||
|
||||
### Controls and permissions
|
||||
|
||||
Add `ToolPayloadRetentionCard` beside existing retention settings, with its own
|
||||
`system:tool-payload-retention` save contributor. Reuse draft/save conflict
|
||||
handling, job status, backup list, and `useIsAdmin` conventions.
|
||||
|
||||
Reference in New Issue
Block a user