From df5ec5d47904bbda81e605fff9cb231aaa99b78b Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 17:07:00 +0530 Subject: [PATCH 01/11] test(driver): shared RunDriverSuite harness for cross-driver coverage --- internal/driver/_testing/fixtures.go | 34 ++++++++ internal/driver/_testing/suite.go | 121 +++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 internal/driver/_testing/fixtures.go create mode 100644 internal/driver/_testing/suite.go diff --git a/internal/driver/_testing/fixtures.go b/internal/driver/_testing/fixtures.go new file mode 100644 index 0000000..4894bc2 --- /dev/null +++ b/internal/driver/_testing/fixtures.go @@ -0,0 +1,34 @@ +// Package drivertesting holds the shared driver test harness. Imported by +// each driver's integration test under a //go:build integration tag. +package drivertesting + +import ( + "context" + "database/sql" + + "github.com/nixrajput/siphon/internal/driver" +) + +// Fixtures describes everything a driver needs to be exercised by the +// shared RunDriverSuite. Drivers populate this struct from their own +// integration setup (e.g. testcontainers). +type Fixtures struct { + // Profile points at a freshly-started, empty test database. + Profile driver.Profile + + // Seed runs arbitrary SQL/script to populate the test database with a + // known fixture. Called before Backup. + Seed func(ctx context.Context, db *sql.DB) error + + // VerifyRestore asserts the database state matches what Seed produced. + // Called after Restore round-trips through Backup. + VerifyRestore func(ctx context.Context, db *sql.DB) error + + // Cleanup tears down the test database. Called via t.Cleanup. + Cleanup func() + + // SQLOpener returns a *sql.DB connected to the same database. Used by + // Seed and VerifyRestore (they can't go through driver.Conn because + // it doesn't expose raw SQL). + SQLOpener func() (*sql.DB, error) +} diff --git a/internal/driver/_testing/suite.go b/internal/driver/_testing/suite.go new file mode 100644 index 0000000..8780e8a --- /dev/null +++ b/internal/driver/_testing/suite.go @@ -0,0 +1,121 @@ +package drivertesting + +import ( + "bytes" + "context" + "errors" + "testing" + "time" + + "github.com/nixrajput/siphon/internal/driver" + "github.com/nixrajput/siphon/internal/errs" +) + +// RunDriverSuite exercises the full Driver contract against a real +// database. ctor returns the driver-under-test; fx provides the env. +// +// Every driver should call this from a single TestSuite_ +// integration test; that yields uniform coverage of: +// - Connect / Close +// - Inspect +// - Backup round-trip via Restore + VerifyRestore +// - Cancel propagation (ctx cancellation kills subprocess + no temp file) +// - Sentinel error mapping on bad credentials +func RunDriverSuite(t *testing.T, ctor func() driver.Driver, fx Fixtures) { + t.Helper() + t.Cleanup(fx.Cleanup) + + d := ctor() + t.Run("Connect_And_Inspect", func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + conn, err := d.Connect(ctx, fx.Profile) + if err != nil { + t.Fatalf("Connect: %v", err) + } + defer func() { _ = conn.Close() }() + + if _, err := conn.Inspect(ctx); err != nil { + t.Fatalf("Inspect: %v", err) + } + }) + + t.Run("BackupRestore_Roundtrip", func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + defer cancel() + + // Open a raw SQL connection for seeding/verification. + db, err := fx.SQLOpener() + if err != nil { + t.Fatalf("SQLOpener: %v", err) + } + defer func() { _ = db.Close() }() + + if err := fx.Seed(ctx, db); err != nil { + t.Fatalf("Seed: %v", err) + } + + conn, err := d.Connect(ctx, fx.Profile) + if err != nil { + t.Fatalf("Connect: %v", err) + } + defer func() { _ = conn.Close() }() + + var dump bytes.Buffer + if err := conn.Backup(ctx, driver.BackupOpts{}, &dump); err != nil { + t.Fatalf("Backup: %v", err) + } + + // Round-trip: drop everything, then restore. + // (Drivers do this with their own "clean" flag in Restore.) + if err := conn.Restore(ctx, driver.RestoreOpts{Clean: true}, &dump); err != nil { + t.Fatalf("Restore: %v", err) + } + + if err := fx.VerifyRestore(ctx, db); err != nil { + t.Fatalf("VerifyRestore: %v", err) + } + }) + + t.Run("Cancel_PropagatesToSubprocess", func(t *testing.T) { + conn, err := d.Connect(context.Background(), fx.Profile) + if err != nil { + t.Fatalf("Connect: %v", err) + } + defer func() { _ = conn.Close() }() + + ctx, cancel := context.WithCancel(context.Background()) + ch := make(chan error, 1) + buf := &bytes.Buffer{} + go func() { ch <- conn.Backup(ctx, driver.BackupOpts{}, buf) }() + + time.Sleep(150 * time.Millisecond) + cancel() + + select { + case err := <-ch: + if err == nil { + t.Fatal("Backup returned nil after cancel; want non-nil error") + } + case <-time.After(10 * time.Second): + t.Fatal("Backup did not return within 10s after cancel — subprocess leak?") + } + }) + + t.Run("BadCredentials_ReturnsErrConnectionFailed", func(t *testing.T) { + bad := fx.Profile + bad.Password = "definitely-wrong-" + t.Name() + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + _, err := d.Connect(ctx, bad) + if err == nil { + t.Fatal("Connect succeeded with bad credentials") + } + if !errors.Is(err, errs.ErrConnectionFailed) { + t.Fatalf("err = %v; want errors.Is(err, ErrConnectionFailed)", err) + } + }) +} From 69d48858a684fd4b5f8d7cc5858d91ebb3b41977 Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 17:11:58 +0530 Subject: [PATCH 02/11] test(driver/postgres): use RunDriverSuite harness --- internal/driver/postgres/integration_test.go | 115 +++++++------------ 1 file changed, 42 insertions(+), 73 deletions(-) diff --git a/internal/driver/postgres/integration_test.go b/internal/driver/postgres/integration_test.go index e10446e..b5db50e 100644 --- a/internal/driver/postgres/integration_test.go +++ b/internal/driver/postgres/integration_test.go @@ -3,18 +3,18 @@ package postgres import ( - "bytes" "context" - "io" + "database/sql" + "fmt" "testing" - "time" pgctr "github.com/testcontainers/testcontainers-go/modules/postgres" "github.com/nixrajput/siphon/internal/driver" + drivertesting "github.com/nixrajput/siphon/internal/driver/_testing" ) -func startPostgres(t *testing.T) (driver.Profile, func()) { +func startPostgres(t *testing.T) (driver.Profile, func(), func() (*sql.DB, error)) { t.Helper() ctx := context.Background() c, err := pgctr.Run(ctx, "postgres:16-alpine", @@ -36,77 +36,46 @@ func startPostgres(t *testing.T) (driver.Profile, func()) { t.Fatalf("container mapped port: %v", err) } - return driver.Profile{ - Driver: "postgres", - Host: host, - Port: int(port.Num()), - User: "postgres", - Password: "postgres", - Database: "test", - SSLMode: "disable", - }, func() { - _ = c.Terminate(ctx) - } -} - -func TestIntegration_Connect_And_Inspect(t *testing.T) { - p, cleanup := startPostgres(t) - t.Cleanup(cleanup) - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - conn, err := Driver{}.Connect(ctx, p) - if err != nil { - t.Fatalf("Connect: %v", err) + prof := driver.Profile{ + Driver: "postgres", + Host: host, + Port: int(port.Num()), + User: "postgres", + Password: "postgres", + Database: "test", + SSLMode: "disable", } - defer func() { _ = conn.Close() }() - - if _, err := conn.Inspect(ctx); err != nil { - t.Fatalf("Inspect: %v", err) + cleanup := func() { _ = c.Terminate(ctx) } + opener := func() (*sql.DB, error) { + dsn := fmt.Sprintf("host=%s port=%d user=postgres password=postgres dbname=test sslmode=disable", prof.Host, prof.Port) + return sql.Open("pgx", dsn) } + return prof, cleanup, opener } -func TestIntegration_BackupRestore_Roundtrip(t *testing.T) { - p, cleanup := startPostgres(t) - t.Cleanup(cleanup) - - ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) - defer cancel() - - conn, err := Driver{}.Connect(ctx, p) - if err != nil { - t.Fatalf("Connect: %v", err) - } - defer func() { _ = conn.Close() }() - - // Seed - if _, execErr := conn.(*Conn).db.ExecContext(ctx, - `CREATE TABLE widgets(id int primary key, name text); INSERT INTO widgets VALUES (1,'wrench');`, - ); execErr != nil { - t.Fatalf("seed: %v", execErr) - } - - var buf bytes.Buffer - if err := conn.Backup(ctx, driver.BackupOpts{}, &buf); err != nil { - t.Fatalf("Backup: %v", err) - } - - // Drop the table - if _, execErr := conn.(*Conn).db.ExecContext(ctx, `DROP TABLE widgets;`); execErr != nil { - t.Fatalf("drop: %v", execErr) - } - - if err := conn.Restore(ctx, driver.RestoreOpts{}, io.NopCloser(&buf)); err != nil { - t.Fatalf("Restore: %v", err) - } - - var count int - row := conn.(*Conn).db.QueryRowContext(ctx, `SELECT COUNT(*) FROM widgets`) - if err := row.Scan(&count); err != nil { - t.Fatalf("post-restore SELECT: %v", err) - } - if count != 1 { - t.Fatalf("expected 1 widget after restore; got %d", count) - } +func TestSuite_Postgres(t *testing.T) { + prof, cleanup, opener := startPostgres(t) + drivertesting.RunDriverSuite(t, func() driver.Driver { return Driver{} }, + drivertesting.Fixtures{ + Profile: prof, + Cleanup: cleanup, + SQLOpener: opener, + Seed: func(ctx context.Context, db *sql.DB) error { + _, err := db.ExecContext(ctx, + `DROP TABLE IF EXISTS widgets; + CREATE TABLE widgets(id int primary key, name text); + INSERT INTO widgets VALUES (1,'wrench'),(2,'hammer');`) + return err + }, + VerifyRestore: func(ctx context.Context, db *sql.DB) error { + var count int + if err := db.QueryRowContext(ctx, `SELECT COUNT(*) FROM widgets`).Scan(&count); err != nil { + return err + } + if count != 2 { + return fmt.Errorf("expected 2 widgets after restore, got %d", count) + } + return nil + }, + }) } From 217e8a4e071e164010adc4973ae0c353aff76876 Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 17:16:45 +0530 Subject: [PATCH 03/11] test(driver/postgres): reuse buildDSN in integration opener --- internal/driver/postgres/integration_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/driver/postgres/integration_test.go b/internal/driver/postgres/integration_test.go index b5db50e..75d97c1 100644 --- a/internal/driver/postgres/integration_test.go +++ b/internal/driver/postgres/integration_test.go @@ -47,8 +47,7 @@ func startPostgres(t *testing.T) (driver.Profile, func(), func() (*sql.DB, error } cleanup := func() { _ = c.Terminate(ctx) } opener := func() (*sql.DB, error) { - dsn := fmt.Sprintf("host=%s port=%d user=postgres password=postgres dbname=test sslmode=disable", prof.Host, prof.Port) - return sql.Open("pgx", dsn) + return sql.Open("pgx", buildDSN(prof)) } return prof, cleanup, opener } From 46f46eea824f6061a80cacca688297a1b04b431d Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 17:21:37 +0530 Subject: [PATCH 04/11] feat(app): RequireCapability gate for verb pre-flight Add RequireCapability(d Deps, profileName string, requiredCap Capability) in the app layer. It resolves the profile to its driver internally and checks the driver's Capabilities, returning a CodeUser errs.Error wrapping ErrDriverUnsupported when the capability is missing. Taking a profile name (not a driver.Driver) keeps the CLI/TUI from having to import internal/driver, respecting the depguard boundary. --- internal/app/capgate.go | 93 ++++++++++++++++++++++++++++++++++++ internal/app/capgate_test.go | 89 ++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+) create mode 100644 internal/app/capgate.go create mode 100644 internal/app/capgate_test.go diff --git a/internal/app/capgate.go b/internal/app/capgate.go new file mode 100644 index 0000000..f30a513 --- /dev/null +++ b/internal/app/capgate.go @@ -0,0 +1,93 @@ +package app + +import ( + "github.com/nixrajput/siphon/internal/driver" + "github.com/nixrajput/siphon/internal/errs" +) + +// Capability identifies a specific feature flag a verb requires. The string +// values mirror the driver.Capabilities field semantics and appear in the +// user-facing hint when a driver lacks the capability. +type Capability string + +const ( + CapIncremental Capability = "incremental" + CapNativeStream Capability = "native-stream" + CapPerTable Capability = "per-table" + CapSchemaOnly Capability = "schema-only" + CapDataOnly Capability = "data-only" + CapParallel Capability = "parallel" + CapCompression Capability = "compression" + CapBinaryFormat Capability = "binary-format" + CapCrossEngineSource Capability = "cross-engine-source" + CapCrossEngineTarget Capability = "cross-engine-target" + CapCDC Capability = "cdc" + CapNativeBackpressure Capability = "native-backpressure" + CapCrossVersionIncremental Capability = "cross-version-incremental" +) + +// RequireCapability resolves profileName to its driver and checks that the +// driver supports requiredCap. It returns nil when supported, or a CodeUser +// errs.Error wrapping ErrDriverUnsupported (with an actionable hint) when not. +// +// Callers in the presentation layer (CLI/TUI) use this for verb pre-flight so +// an unsupported affordance fails fast with a clear message rather than +// crashing partway through. It takes a profile name (not a driver.Driver) so +// the CLI never has to import the driver package (depguard boundary). +func RequireCapability(d Deps, profileName string, requiredCap Capability) error { + resolved, err := d.Profiles.Resolve(profileName) + if err != nil { + return err + } + drv, err := d.Drivers.Get(resolved.Driver) + if err != nil { + return err + } + if driverSupports(drv, requiredCap) { + return nil + } + return &errs.Error{ + Op: "capgate." + string(requiredCap), + Code: errs.CodeUser, + Cause: errs.ErrDriverUnsupported, + Hint: drv.Name() + " driver does not support " + string(requiredCap), + } +} + +// driverSupports maps a Capability to the corresponding driver.Capabilities flag. +// +// Keep in sync with driver.Capabilities: a new field there needs a matching +// Cap* constant above and a case here. A missing case falls through to false +// (fail-safe: the gate blocks rather than silently permitting an unsupported op). +func driverSupports(drv driver.Driver, requiredCap Capability) bool { + c := drv.Capabilities() + switch requiredCap { + case CapIncremental: + return c.Incremental + case CapNativeStream: + return c.NativeStream + case CapPerTable: + return c.PerTable + case CapSchemaOnly: + return c.SchemaOnly + case CapDataOnly: + return c.DataOnly + case CapParallel: + return c.Parallel + case CapCompression: + return c.Compression + case CapBinaryFormat: + return c.BinaryFormat + case CapCrossEngineSource: + return c.CrossEngineSource + case CapCrossEngineTarget: + return c.CrossEngineTarget + case CapCDC: + return c.CDC + case CapNativeBackpressure: + return c.NativeBackpressure + case CapCrossVersionIncremental: + return c.CrossVersionIncremental + } + return false +} diff --git a/internal/app/capgate_test.go b/internal/app/capgate_test.go new file mode 100644 index 0000000..76d5bf7 --- /dev/null +++ b/internal/app/capgate_test.go @@ -0,0 +1,89 @@ +package app + +import ( + "context" + "errors" + "testing" + + "github.com/nixrajput/siphon/internal/config" + "github.com/nixrajput/siphon/internal/driver" + "github.com/nixrajput/siphon/internal/errs" + "github.com/nixrajput/siphon/internal/profile" + "github.com/nixrajput/siphon/internal/secrets" +) + +// capDriver is a driver whose Capabilities are fully controllable per-test. +// (The package-level fakeDriver in backup_test.go hardcodes its caps, so it +// can't exercise the capability mapping.) +type capDriver struct { + name string + caps driver.Capabilities +} + +func (d capDriver) Name() string { return d.name } +func (d capDriver) Capabilities() driver.Capabilities { return d.caps } +func (d capDriver) Connect(_ context.Context, _ driver.Profile) (driver.Conn, error) { + return nil, nil +} + +// capDeps builds a Deps with a real *profile.Store holding one profile "p" +// bound to driver "fake", whose driver reports the given capabilities. +func capDeps(caps driver.Capabilities) Deps { + cfg := &config.Config{Profiles: map[string]config.ProfileConfig{ + "p": {Driver: "fake", Host: "h", User: "u", Database: "d", Password: "pw"}, + }} + res := secrets.NewResolver(secrets.Passthrough{}) + ps := profile.New(cfg, res, func(*config.Config) error { return nil }) + return Deps{ + Profiles: ps, + Drivers: fakeGetter{d: capDriver{name: "fake", caps: caps}}, + } +} + +func TestRequireCapability_Supported(t *testing.T) { + deps := capDeps(driver.Capabilities{Parallel: true}) + if err := RequireCapability(deps, "p", CapParallel); err != nil { + t.Fatalf("RequireCapability(CapParallel) = %v; want nil", err) + } +} + +func TestRequireCapability_Unsupported_ReturnsErrDriverUnsupported(t *testing.T) { + deps := capDeps(driver.Capabilities{Parallel: false}) + err := RequireCapability(deps, "p", CapParallel) + if err == nil { + t.Fatal("RequireCapability(CapParallel) = nil; want error") + } + if !errors.Is(err, errs.ErrDriverUnsupported) { + t.Fatalf("errors.Is(err, ErrDriverUnsupported) = false; err = %v", err) + } + var ee *errs.Error + if !errors.As(err, &ee) { + t.Fatalf("err is not *errs.Error; got %T", err) + } + if ee.Code != errs.CodeUser { + t.Fatalf("Code = %v; want CodeUser", ee.Code) + } +} + +func TestRequireCapability_MappingMatchesFlags(t *testing.T) { + cases := []struct { + name string + cap Capability + caps driver.Capabilities + want bool + }{ + {"native-stream supported", CapNativeStream, driver.Capabilities{NativeStream: true}, true}, + {"native-stream unsupported", CapNativeStream, driver.Capabilities{}, false}, + {"incremental supported", CapIncremental, driver.Capabilities{Incremental: true}, true}, + {"cdc unsupported", CapCDC, driver.Capabilities{Incremental: true}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + err := RequireCapability(capDeps(tc.caps), "p", tc.cap) + got := err == nil + if got != tc.want { + t.Fatalf("RequireCapability(%s) supported = %v; want %v (err=%v)", tc.cap, got, tc.want, err) + } + }) + } +} From b1afe03251c0f09a6b49628f33544c1bafb6fb71 Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 17:29:24 +0530 Subject: [PATCH 05/11] feat(cli): gate --jobs and --stream by driver Capabilities --- internal/cli/backup.go | 5 +++++ internal/cli/sync.go | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/internal/cli/backup.go b/internal/cli/backup.go index 735b310..bd50477 100644 --- a/internal/cli/backup.go +++ b/internal/cli/backup.go @@ -29,6 +29,11 @@ func newBackupCmd() *cobra.Command { if err != nil { return err } + if parallel > 1 { + if err := app.RequireCapability(deps, profileName, app.CapParallel); err != nil { + return err + } + } ch, _, err := app.Backup(c.Context(), deps, app.BackupOpts{ Profile: profileName, IncludeTables: includeTables, diff --git a/internal/cli/sync.go b/internal/cli/sync.go index 2518b5f..46f9759 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -27,6 +27,13 @@ func newSyncCmd() *cobra.Command { if err != nil { return err } + if stream { + for _, name := range []string{fromName, toName} { + if err := app.RequireCapability(deps, name, app.CapNativeStream); err != nil { + return err + } + } + } ch, _, err := app.Sync(c.Context(), deps, app.SyncOpts{ From: fromName, To: toName, Stream: stream, Tables: tables, }) From 30465a757cec0a73fb656af2e61b3b942827dd3a Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 17:41:43 +0530 Subject: [PATCH 06/11] feat(driver/postgres): apply Retry policy to Connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Probe the connection via jobs.Retry (spec §4.3: 3 attempts with exponential backoff) so a briefly-unavailable server is not a hard fail. sql.Open stays a single lazy call; only PingContext is retried. On final failure the error still maps through wrapConnErr to errs.ErrConnectionFailed. Move Retry from internal/app to internal/jobs: the driver may not import internal/app (depguard driver-may-not-import-presentation), and driver already imports internal/jobs, so this avoids the import cycle. Retry had no callers; add a unit test for it in the jobs package. --- internal/driver/postgres/driver.go | 6 ++- internal/{app => jobs}/retry.go | 5 ++- internal/jobs/retry_test.go | 72 ++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 3 deletions(-) rename internal/{app => jobs}/retry.go (85%) create mode 100644 internal/jobs/retry_test.go diff --git a/internal/driver/postgres/driver.go b/internal/driver/postgres/driver.go index 72e82bc..e9024dd 100644 --- a/internal/driver/postgres/driver.go +++ b/internal/driver/postgres/driver.go @@ -11,6 +11,7 @@ import ( "github.com/nixrajput/siphon/internal/driver" "github.com/nixrajput/siphon/internal/errs" + "github.com/nixrajput/siphon/internal/jobs" _ "github.com/jackc/pgx/v5/stdlib" // register pgx as a database/sql driver ) @@ -45,7 +46,10 @@ func (Driver) Connect(ctx context.Context, p driver.Profile) (driver.Conn, error if err != nil { return nil, wrapConnErr(err) } - if err := db.PingContext(ctx); err != nil { + + // Probe the connection with bounded retry (spec §4.3: 3 attempts, + // exponential backoff) so a briefly-unavailable server isn't a hard fail. + if err := jobs.Retry(ctx, 3, func() error { return db.PingContext(ctx) }); err != nil { _ = db.Close() return nil, wrapConnErr(err) } diff --git a/internal/app/retry.go b/internal/jobs/retry.go similarity index 85% rename from internal/app/retry.go rename to internal/jobs/retry.go index 3b7d065..9d383ed 100644 --- a/internal/app/retry.go +++ b/internal/jobs/retry.go @@ -1,4 +1,4 @@ -package app +package jobs import ( "context" @@ -7,7 +7,8 @@ import ( ) // Retry runs op with exponential backoff + jitter, up to maxAttempts. -// Returns the last error if all attempts fail. +// Returns the last error if all attempts fail. Respects ctx cancellation +// between attempts. func Retry(ctx context.Context, maxAttempts int, op func() error) error { var err error delay := 100 * time.Millisecond diff --git a/internal/jobs/retry_test.go b/internal/jobs/retry_test.go new file mode 100644 index 0000000..8f3db8f --- /dev/null +++ b/internal/jobs/retry_test.go @@ -0,0 +1,72 @@ +package jobs + +import ( + "context" + "errors" + "testing" +) + +func TestRetry_SucceedsFirstAttempt(t *testing.T) { + calls := 0 + err := Retry(context.Background(), 3, func() error { + calls++ + return nil + }) + if err != nil { + t.Fatalf("Retry: %v", err) + } + if calls != 1 { + t.Fatalf("calls = %d; want 1", calls) + } +} + +func TestRetry_SucceedsLaterAttempt(t *testing.T) { + calls := 0 + err := Retry(context.Background(), 3, func() error { + calls++ + if calls < 2 { + return errors.New("transient") + } + return nil + }) + if err != nil { + t.Fatalf("Retry: %v", err) + } + if calls != 2 { + t.Fatalf("calls = %d; want 2", calls) + } +} + +func TestRetry_AllAttemptsFail(t *testing.T) { + calls := 0 + last := errors.New("boom") + err := Retry(context.Background(), 3, func() error { + calls++ + return last + }) + if !errors.Is(err, last) { + t.Fatalf("err = %v; want %v", err, last) + } + if calls != 3 { + t.Fatalf("calls = %d; want 3", calls) + } +} + +func TestRetry_ContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + calls := 0 + err := Retry(ctx, 3, func() error { + calls++ + return errors.New("transient") + }) + // First op runs, fails; the backoff select then sees ctx.Done() and + // returns ctx.Err() promptly without hanging. + if !errors.Is(err, context.Canceled) { + t.Fatalf("err = %v; want context.Canceled", err) + } + if calls != 1 { + t.Fatalf("calls = %d; want 1", calls) + } +} From 33486cafb50da9ee499d28ec560c36804d53d6b5 Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 17:48:37 +0530 Subject: [PATCH 07/11] docs: add DRIVERS.md contributor guide --- README.md | 2 + docs/DRIVERS.md | 257 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 259 insertions(+) create mode 100644 docs/DRIVERS.md diff --git a/README.md b/README.md index c6406f5..bfcc0ba 100644 --- a/README.md +++ b/README.md @@ -217,6 +217,8 @@ Distribution (Homebrew, Scoop, install script, signed release binaries) lands in Contributions are welcome. Please read [CONTRIBUTING.md](CONTRIBUTING.md), keep changes within the layered architecture (depguard will tell you if you stray), and make sure `make test` and `make lint` pass before opening a PR. +Adding a new database engine? See [docs/DRIVERS.md](docs/DRIVERS.md) for the driver contributor guide. + ## License [MIT](LICENSE) © [Nikhil Rajput](https://github.com/nixrajput) diff --git a/docs/DRIVERS.md b/docs/DRIVERS.md new file mode 100644 index 0000000..9f55578 --- /dev/null +++ b/docs/DRIVERS.md @@ -0,0 +1,257 @@ +# Adding a database driver + +This guide walks through adding a new database engine to siphon (e.g. MySQL in +Phase E). It reflects the real interfaces and conventions in the codebase — the +signatures below compile-match `internal/driver/driver.go`, so you can copy from +them. + +## Table of contents + +- [Overview](#overview) +- [The contract](#the-contract) +- [Capabilities](#capabilities) +- [Registration](#registration) +- [Integration tests](#integration-tests) +- [Error mapping](#error-mapping) + +## Overview + +Drivers live in `internal/driver//` — one package per engine (the Postgres +driver is `internal/driver/postgres/`). A driver is a compile-time Go package +that shells out to the engine's native tools for data movement (e.g. +`pg_dump`/`pg_restore`) and may use a client library for fast schema reads. + +siphon is strictly layered; imports flow **downward only**, enforced at lint +time by `golangci-lint`'s `depguard`: + +``` + internal/cli internal/tui presentation (Cobra · Bubble Tea) + └──────┬───────┘ + internal/app application verbs (backup, restore, sync, …) + │ + internal/driver/ database adapters — your code lives here + │ + config · secrets · profile · dumps · jobs · errs domain + support packages +``` + +**Drivers are stateless.** The `Driver` value carries no connection state; +state lives in the `Conn` returned by `Connect`, scoped per-verb. `Conn` is also +the unit of cancellation — the `ctx` passed to each verb must propagate to any +spawned subprocess so a cancel tears it down cleanly. + +## The contract + +Implement two interfaces from `internal/driver/driver.go`: + +```go +// Driver is the protocol-level abstraction for a database engine. +type Driver interface { + Name() string + Capabilities() Capabilities + Connect(ctx context.Context, p Profile) (Conn, error) +} + +// Conn is an open connection plus the verbs that operate on it. +type Conn interface { + Inspect(ctx context.Context) (*Schema, error) + Backup(ctx context.Context, opt BackupOpts, w io.Writer) error + Restore(ctx context.Context, opt RestoreOpts, r io.Reader) error + Verify(ctx context.Context, r io.Reader) (*VerifyReport, error) + Close() error +} +``` + +What each method does: + +- **`Name()`** — the lowercase engine name (`"postgres"`), matching a profile's + `Driver` field. Used by the registry and in user-facing messages. +- **`Capabilities()`** — declares what the engine supports (see below). +- **`Connect(ctx, p)`** — opens a connection from a `Profile` and returns a + `Conn`. Probe the connection here (the Postgres driver pings with a bounded + retry) and map failures to `errs.ErrConnectionFailed`. +- **`Inspect(ctx)`** — returns a `*Schema` (tables with row estimates and sizes). +- **`Backup(ctx, opt, w)`** — writes a dump to `w` (an `io.Writer`). +- **`Restore(ctx, opt, r)`** — reads a dump from `r` (an `io.Reader`). +- **`Verify(ctx, r)`** — reads a dump from `r` and returns a `*VerifyReport`. At + minimum, compute a checksum (the Postgres driver hashes the stream with SHA-256 + → `Checksum: "sha256:…"`). +- **`Close()`** — releases the connection. + +`Connect` receives a `Profile`. Secrets are already resolved before this struct +reaches `Connect` — never wire a `SecretRef` here: + +```go +type Profile struct { + Name string + Driver string + Host string + Port int + User string + Password string + Database string + SSLMode string +} +``` + +The options and result structs (`BackupOpts`, `RestoreOpts`, `Schema`, +`VerifyReport`) are also defined in `internal/driver/driver.go` — read that file +for their fields. + +## Capabilities + +`Capabilities` is a struct of 13 boolean flags. Each gates a UI affordance or +feature path: + +```go +type Capabilities struct { + Incremental bool + NativeStream bool + PerTable bool + SchemaOnly bool + DataOnly bool + Parallel bool + Compression bool + BinaryFormat bool + CrossEngineSource bool + CrossEngineTarget bool + CDC bool + NativeBackpressure bool + CrossVersionIncremental bool +} +``` + +**Declare these honestly.** The presentation layer gates affordances through +`app.RequireCapability(deps, profileName, cap)` (in `internal/app/capgate.go`) +before running a verb. If your driver declares `Parallel: false`, the CLI rejects +`--jobs N` (with `N > 1`) up front with an `errs.ErrDriverUnsupported` error and +an actionable hint — instead of crashing partway through. Over-declaring a +capability you don't actually support turns that clean rejection into a runtime +failure. + +When you add support for a capability, flip its flag — that single change lights +up the corresponding UI path. + +## Registration + +A driver registers itself with the process-wide registry from its package +`init()`, mirroring the `database/sql.Register` convention: + +```go +func init() { driver.Register(&Driver{}) } +``` + +`driver.Register(d)` panics if called twice for the same `Name()` — a duplicate +almost always signals a copy-paste bug worth surfacing loudly at startup. +`driver.Get(name)` resolves a profile's `Driver` string to the registered driver +(returning `errs.ErrDriverUnsupported` if none matches). Both live in +`internal/driver/registry.go`. + +`init()` only runs if the package is imported. The side-effect import that pulls +your driver in lives in **`internal/app/drivers.go`**: + +```go +import _ "github.com/nixrajput/siphon/internal/driver/postgres" // register the postgres driver +``` + +Add a matching blank import line there for your driver. + +> **Why `internal/app/drivers.go` and not `internal/cli/root.go`?** The depguard +> rule `cli-may-not-import-domain` forbids `internal/cli` from importing +> `internal/driver/**` — even a blank `_` import would fail lint. Presentation +> layers (CLI, TUI) reach drivers only through the application layer's +> `DefaultDrivers()`, which is backed by the registry. So the import that wires a +> driver into the build belongs in the app layer. + +## Integration tests + +Drivers get four contract tests for free from the shared harness at +`internal/driver/_testing/` (package `drivertesting`). Ship an +`integration_test.go` with a `//go:build integration` tag that calls +`drivertesting.RunDriverSuite`: + +```go +//go:build integration + +package mysql + +func TestSuite_MySQL(t *testing.T) { + prof, cleanup, opener := startMySQL(t) // your own setup, e.g. testcontainers + drivertesting.RunDriverSuite(t, func() driver.Driver { return Driver{} }, + drivertesting.Fixtures{ + Profile: prof, + Cleanup: cleanup, + SQLOpener: opener, + Seed: func(ctx context.Context, db *sql.DB) error { /* … */ }, + VerifyRestore: func(ctx context.Context, db *sql.DB) error { /* … */ }, + }) +} +``` + +`RunDriverSuite(t, ctor, fx)` runs four subtests against a real database: + +- **`Connect_And_Inspect`** — `Connect` succeeds and `Inspect` returns a schema. +- **`BackupRestore_Roundtrip`** — `Seed` → `Backup` → `Restore{Clean:true}` → + `VerifyRestore`. +- **`Cancel_PropagatesToSubprocess`** — cancelling the `ctx` mid-backup returns a + non-nil error promptly (no subprocess leak). +- **`BadCredentials_ReturnsErrConnectionFailed`** — `Connect` with a wrong + password returns an error matching `errors.Is(err, errs.ErrConnectionFailed)`. + +The `Fixtures` struct (in `internal/driver/_testing/fixtures.go`) you populate: + +- **`Profile`** — points at a freshly-started, empty test database. +- **`Seed(ctx, db)`** — runs SQL to populate a known fixture; called before Backup. +- **`VerifyRestore(ctx, db)`** — asserts the database state matches what `Seed` + produced; called after the Backup/Restore round-trip. +- **`Cleanup()`** — tears down the test database; called via `t.Cleanup`. +- **`SQLOpener()`** — returns a `*sql.DB` on the same database, used by `Seed` + and `VerifyRestore` (they can't go through `driver.Conn`, which doesn't expose + raw SQL). + +The worked example is `internal/driver/postgres/integration_test.go` — copy its +shape. The suite runs behind the `integration` build tag (`make test-integration`). + +## Error mapping + +Drivers (and verbs) return the structured error type from `internal/errs/errs.go`: + +```go +type Error struct { + Op string // the verb name, e.g. "backup", "restore" + Code Code // exit-code taxonomy bucket + Cause error // underlying cause; matched by errors.Is / errors.As + Hint string // user-actionable remediation, rendered in CLI/TUI display +} +``` + +`Code` is one of the exit-code buckets: `CodeOK` (0), `CodeUser` (1), +`CodeSystem` (2), `CodeIntegrity` (3), `CodePartial` (4), `CodeCancelled` (130), +`CodeTerminated` (143). + +Two requirements the harness and UI depend on: + +- **Bad credentials must surface as `errs.ErrConnectionFailed`.** The + `BadCredentials_ReturnsErrConnectionFailed` subtest asserts + `errors.Is(err, errs.ErrConnectionFailed)`. The Postgres driver does this with + a small `wrapConnErr` helper: + + ```go + func wrapConnErr(err error) error { + return &errs.Error{ + Op: "postgres.connect", + Code: errs.CodeSystem, + Cause: errs.ErrConnectionFailed, + Hint: err.Error(), + } + } + ``` + + Because `*errs.Error` unwraps to `Cause`, `errors.Is(err, errs.ErrConnectionFailed)` + matches. + +- **`Verify` computes a checksum.** At minimum, hash the dump stream (the + Postgres driver uses SHA-256 → `VerifyReport.Checksum = "sha256:…"`). + +Set `Code` to the bucket that matches the failure (`CodeUser` for bad input, +`CodeSystem` for infrastructure, `CodeIntegrity` for checksum/corruption, and so +on) so the CLI exits with the right POSIX code. From b4bc794fbcbc0b408701c7d39e571cb519ffa8a7 Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 18:04:39 +0530 Subject: [PATCH 08/11] docs: mark Phase D complete in README + CHANGELOG --- CHANGELOG.md | 7 +++++++ README.md | 4 ++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ef5c83..5ebf20d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,3 +10,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Phase A skeleton: Go module, Cobra CLI with placeholder subcommands, Bubble Tea TUI placeholder, Driver interface + registry, errs package, config stub, Makefile, golangci-lint with depguard, CI workflow. +- Phase B Postgres walking skeleton: `backup`, `restore`, `sync`, `verify`, `inspect`, `dumps`, and `profile` working end-to-end against PostgreSQL (shelling out to `pg_dump`/`pg_restore`, pgx for inspect), SHA-256 dump checksums with sidecar metadata, named profiles with `env:` secret refs, and POSIX exit-code taxonomy. +- Phase C interactive TUI dashboard: multi-panel Bubble Tea UI (profiles · dumps · jobs) with live job-progress subscription, backup/restore modal forms, an error overlay, and golden snapshot tests. +- Phase D driver-layer hardening: + - Shared cross-driver test harness `RunDriverSuite` under `internal/driver/_testing/`, giving every driver four contract tests for free (connect/inspect, backup→restore round-trip, cancel propagation, bad-credentials sentinel). The Postgres integration test now runs on it. + - Capability gating: `app.RequireCapability` resolves a profile to its driver and rejects unsupported affordances with a `CodeUser` error. Wired into the CLI so `backup --jobs N>1` is gated by `Parallel` and `sync --stream` by `NativeStream`. + - Connection-probe retry on Postgres `Connect` (3 attempts, exponential backoff) per spec §4.3; the generic `Retry` helper moved to `internal/jobs` to keep the driver layer free of an app-layer import. + - `docs/DRIVERS.md` contributor guide documenting the driver contract, registration, the test harness, capability flags, and error mapping. diff --git a/README.md b/README.md index bfcc0ba..eccd8a4 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ A single binary that turns the painful, error-prone sprawl of `pg_dump` → `pg_ --- > [!WARNING] -> **Pre-1.0 — active development.** Postgres backup/restore/sync/verify/inspect work end-to-end today (Phase B), and bare `siphon` opens an interactive multi-panel dashboard (Phase C). MySQL/MariaDB, incremental backups, and ops features are on the [roadmap](#roadmap). APIs, flags, and the on-disk dump format may change before 1.0. Track progress via the milestone tags (`phase-a`, `phase-b`, `phase-c`, …). +> **Pre-1.0 — active development.** Postgres backup/restore/sync/verify/inspect work end-to-end today (Phase B), and bare `siphon` opens an interactive multi-panel dashboard (Phase C). The driver layer is hardened with a shared cross-driver test harness, capability gating, and connection retry (Phase D), so MySQL/MariaDB can land mechanically next. MySQL/MariaDB, incremental backups, and ops features are on the [roadmap](#roadmap). APIs, flags, and the on-disk dump format may change before 1.0. Track progress via the milestone tags (`phase-a`, `phase-b`, `phase-c`, `phase-d`, …). ## Table of contents @@ -52,7 +52,7 @@ A single binary that turns the painful, error-prone sprawl of `pg_dump` → `pg_ | **A** — Skeleton | Go module, Cobra CLI, TUI placeholder, `Driver` interface + registry, `errs`/`config`/`secrets`/`profile` packages, golangci-lint + depguard, cross-platform CI | ✅ Complete | | **B** — Postgres walking skeleton | `backup`, `restore`, `sync`, `verify`, `inspect`, `dumps`, `config`, `profile` working end-to-end against PostgreSQL | ✅ Complete | | **C** — TUI dashboard | Multi-panel Bubble Tea dashboard (profiles · dumps · jobs) with live job progress, backup/restore modal forms, and snapshot tests | ✅ Complete | -| **D** — Driver hardening | Shared cross-driver test harness, capability gating, retry policy | ⏳ Planned | +| **D** — Driver hardening | Shared cross-driver test harness (`RunDriverSuite`), capability gating (`RequireCapability`), connection-probe retry, and a `docs/DRIVERS.md` contributor guide | ✅ Complete | | **E** — MySQL + MariaDB | Both drivers via a shared `_mysqlcommon` package | ⏳ Planned | | **F** — Advanced transfer | Incremental backups, bounded-buffer streaming, cross-engine sync, CDC | ⏳ Planned | | **G** — Ops features | Cloud storage, secret backends, profile groups + 2FA, team mode, audit log, retention, telemetry | ⏳ Planned | From 231b9d96ec09bd64cb39764967a0f766fd3a7042 Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 18:06:45 +0530 Subject: [PATCH 09/11] docs: update command table formatting in README --- README.md | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index eccd8a4..3b3f963 100644 --- a/README.md +++ b/README.md @@ -121,19 +121,19 @@ Exit codes follow a POSIX-friendly taxonomy (`0` ok, `1` user error, `2` system ## Commands -| Command | Description | -| ------------------------------------ | -------------------------------------------------------------- | -| `siphon backup [profile]` | Dump a database to a checksummed file in the catalog | -| `siphon restore [dump-id]` | Load a dump into a database (`--clean` to drop-and-recreate) | -| `siphon sync [from] [to]` | Backup + restore in one streamed pass | -| `siphon verify ` | Re-hash a dump and check it against its recorded checksum | -| `siphon inspect ` | Show tables, row estimates, and sizes for a profile | -| `siphon dumps list\|inspect\|prune` | List, inspect, or prune saved dumps | -| `siphon profile add\|list\|show\|rm` | Manage named connection profiles | -| `siphon config path\|edit` | Show or edit the config file | -| `siphon schedule` | Cron-managed recurring backups _(Phase G)_ | -| `siphon tunnel` | SSH tunnel helper _(Phase G)_ | -| `siphon` _(bare)_ | Launch the interactive multi-panel dashboard | +| Command | Description | +| ------------------------------------ | ------------------------------------------------------------ | +| `siphon backup [profile]` | Dump a database to a checksummed file in the catalog | +| `siphon restore [dump-id]` | Load a dump into a database (`--clean` to drop-and-recreate) | +| `siphon sync [from] [to]` | Backup + restore in one streamed pass | +| `siphon verify ` | Re-hash a dump and check it against its recorded checksum | +| `siphon inspect ` | Show tables, row estimates, and sizes for a profile | +| `siphon dumps list\|inspect\|prune` | List, inspect, or prune saved dumps | +| `siphon profile add\|list\|show\|rm` | Manage named connection profiles | +| `siphon config path\|edit` | Show or edit the config file | +| `siphon schedule` | Cron-managed recurring backups _(Phase G)_ | +| `siphon tunnel` | SSH tunnel helper _(Phase G)_ | +| `siphon` _(bare)_ | Launch the interactive multi-panel dashboard | Run `siphon --help` for full flags. From f981ff678ef8a2d85e21daa2ff865a2406d207ec Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 19:02:25 +0530 Subject: [PATCH 10/11] fix(cli): defer premature capability gates to Phase F + address review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove the `backup --jobs`→CapParallel and `sync --stream`→CapNativeStream preflight gates. Both guarded features that the current verbs do not yet implement: app.Sync ignores opt.Stream (always pipes Backup→Restore via io.Pipe), and pg_dump parallelism (-j/-Fd) is unwired ("not yet effective; Phase F"). Gating them now would falsely reject valid operations the moment a driver declaring NativeStream:false / Parallel:false lands, while gating a no-op feature. The RequireCapability helper + Capabilities flags stay; the gates get wired in Phase F alongside the features they guard. Also from CodeRabbit review on PR #3: - _testing/suite.go: the Cancel_PropagatesToSubprocess subtest could flake on fast hosts when the prior subtest's tiny fixture lets pg_dump finish before the 150ms cancel. Seed ~5 MiB via fx.SQLOpener so the dump is reliably in-flight at cancel time; keep the strong non-nil assertion. (Integration-tag only; not run in CI.) - DRIVERS.md: add a `text` language tag to the architecture-diagram fence (markdownlint MD040), and correct the capability section + README/CHANGELOG to describe gating as helper-available-but-not-yet-wired, not CLI-wired. --- CHANGELOG.md | 2 +- README.md | 2 +- docs/DRIVERS.md | 22 ++++++++----- internal/cli/backup.go | 5 --- internal/cli/sync.go | 7 ----- internal/driver/_testing/suite.go | 51 +++++++++++++++++++++++++++++++ 6 files changed, 67 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ebf20d..fcb9c99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Phase C interactive TUI dashboard: multi-panel Bubble Tea UI (profiles · dumps · jobs) with live job-progress subscription, backup/restore modal forms, an error overlay, and golden snapshot tests. - Phase D driver-layer hardening: - Shared cross-driver test harness `RunDriverSuite` under `internal/driver/_testing/`, giving every driver four contract tests for free (connect/inspect, backup→restore round-trip, cancel propagation, bad-credentials sentinel). The Postgres integration test now runs on it. - - Capability gating: `app.RequireCapability` resolves a profile to its driver and rejects unsupported affordances with a `CodeUser` error. Wired into the CLI so `backup --jobs N>1` is gated by `Parallel` and `sync --stream` by `NativeStream`. + - Capability gating helper: `app.RequireCapability` resolves a profile to its driver and rejects unsupported affordances with a `CodeUser` error. The helper and the `Capabilities` flags are in place; verbs are wired to it only where the gated feature is implemented. Streaming (`NativeStream`) and parallel backup (`Parallel`) are deferred to Phase F, so `sync --stream` / `backup --jobs` are not yet gated — the wiring lands with those features. - Connection-probe retry on Postgres `Connect` (3 attempts, exponential backoff) per spec §4.3; the generic `Retry` helper moved to `internal/jobs` to keep the driver layer free of an app-layer import. - `docs/DRIVERS.md` contributor guide documenting the driver contract, registration, the test harness, capability flags, and error mapping. diff --git a/README.md b/README.md index 3b3f963..8d981e7 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ A single binary that turns the painful, error-prone sprawl of `pg_dump` → `pg_ | **A** — Skeleton | Go module, Cobra CLI, TUI placeholder, `Driver` interface + registry, `errs`/`config`/`secrets`/`profile` packages, golangci-lint + depguard, cross-platform CI | ✅ Complete | | **B** — Postgres walking skeleton | `backup`, `restore`, `sync`, `verify`, `inspect`, `dumps`, `config`, `profile` working end-to-end against PostgreSQL | ✅ Complete | | **C** — TUI dashboard | Multi-panel Bubble Tea dashboard (profiles · dumps · jobs) with live job progress, backup/restore modal forms, and snapshot tests | ✅ Complete | -| **D** — Driver hardening | Shared cross-driver test harness (`RunDriverSuite`), capability gating (`RequireCapability`), connection-probe retry, and a `docs/DRIVERS.md` contributor guide | ✅ Complete | +| **D** — Driver hardening | Shared cross-driver test harness (`RunDriverSuite`), capability-gating helper (`RequireCapability`), Postgres connection-probe retry, and a `docs/DRIVERS.md` contributor guide | ✅ Complete | | **E** — MySQL + MariaDB | Both drivers via a shared `_mysqlcommon` package | ⏳ Planned | | **F** — Advanced transfer | Incremental backups, bounded-buffer streaming, cross-engine sync, CDC | ⏳ Planned | | **G** — Ops features | Cloud storage, secret backends, profile groups + 2FA, team mode, audit log, retention, telemetry | ⏳ Planned | diff --git a/docs/DRIVERS.md b/docs/DRIVERS.md index 9f55578..16c0cf2 100644 --- a/docs/DRIVERS.md +++ b/docs/DRIVERS.md @@ -24,7 +24,7 @@ that shells out to the engine's native tools for data movement (e.g. siphon is strictly layered; imports flow **downward only**, enforced at lint time by `golangci-lint`'s `depguard`: -``` +```text internal/cli internal/tui presentation (Cobra · Bubble Tea) └──────┬───────┘ internal/app application verbs (backup, restore, sync, …) @@ -120,13 +120,19 @@ type Capabilities struct { } ``` -**Declare these honestly.** The presentation layer gates affordances through -`app.RequireCapability(deps, profileName, cap)` (in `internal/app/capgate.go`) -before running a verb. If your driver declares `Parallel: false`, the CLI rejects -`--jobs N` (with `N > 1`) up front with an `errs.ErrDriverUnsupported` error and -an actionable hint — instead of crashing partway through. Over-declaring a -capability you don't actually support turns that clean rejection into a runtime -failure. +**Declare these honestly.** A verb pre-flight can gate an affordance through +`app.RequireCapability(deps, profileName, cap)` (in `internal/app/capgate.go`): +if the resolved driver doesn't support `cap`, it returns an +`errs.ErrDriverUnsupported` error with an actionable hint up front, instead of +crashing partway through. Over-declaring a capability you don't actually support +turns that clean rejection into a runtime failure. + +> **Status:** the gate helper and the `Capabilities` flags exist today, but the +> verbs are only gated where a feature is actually wired. Native streaming +> (`NativeStream`) and parallel backup (`Parallel`) are deferred to Phase F, so +> `sync --stream` / `backup --jobs` are not yet gated — the gate will be wired in +> alongside those features. Declare your flags honestly now so the gating is +> correct the moment a verb starts honoring them. When you add support for a capability, flip its flag — that single change lights up the corresponding UI path. diff --git a/internal/cli/backup.go b/internal/cli/backup.go index bd50477..735b310 100644 --- a/internal/cli/backup.go +++ b/internal/cli/backup.go @@ -29,11 +29,6 @@ func newBackupCmd() *cobra.Command { if err != nil { return err } - if parallel > 1 { - if err := app.RequireCapability(deps, profileName, app.CapParallel); err != nil { - return err - } - } ch, _, err := app.Backup(c.Context(), deps, app.BackupOpts{ Profile: profileName, IncludeTables: includeTables, diff --git a/internal/cli/sync.go b/internal/cli/sync.go index 46f9759..2518b5f 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -27,13 +27,6 @@ func newSyncCmd() *cobra.Command { if err != nil { return err } - if stream { - for _, name := range []string{fromName, toName} { - if err := app.RequireCapability(deps, name, app.CapNativeStream); err != nil { - return err - } - } - } ch, _, err := app.Sync(c.Context(), deps, app.SyncOpts{ From: fromName, To: toName, Stream: stream, Tables: tables, }) diff --git a/internal/driver/_testing/suite.go b/internal/driver/_testing/suite.go index 8780e8a..72a85ca 100644 --- a/internal/driver/_testing/suite.go +++ b/internal/driver/_testing/suite.go @@ -4,6 +4,8 @@ import ( "bytes" "context" "errors" + "strconv" + "strings" "testing" "time" @@ -11,6 +13,48 @@ import ( "github.com/nixrajput/siphon/internal/errs" ) +// seedCancelLoad bulk-loads a table of wide rows via the fixture's raw SQL +// connection so a subsequent Backup has enough data to stay in-flight past the +// cancel delay. Uses only portable SQL (CREATE TABLE + parameterized INSERT in +// one transaction), so it works for any engine the harness is pointed at. +func seedCancelLoad(t *testing.T, fx Fixtures) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + db, err := fx.SQLOpener() + if err != nil { + t.Fatalf("seedCancelLoad: SQLOpener: %v", err) + } + defer func() { _ = db.Close() }() + + if _, err := db.ExecContext(ctx, + `CREATE TABLE cancel_load (id integer primary key, payload text)`); err != nil { + t.Fatalf("seedCancelLoad: create table: %v", err) + } + + // ~5000 rows of ~1 KiB each ≈ 5 MiB — large enough that the dump can't + // complete within the 150ms cancel window, small enough to load quickly. + // The payload is a fixed safe literal (no user input), so it's inlined + // rather than parameterized — this keeps the SQL placeholder-agnostic + // ($1 vs ? differs by engine) so the harness stays portable across drivers. + payload := strings.Repeat("x", 1024) + tx, err := db.BeginTx(ctx, nil) + if err != nil { + t.Fatalf("seedCancelLoad: begin: %v", err) + } + for i := 0; i < 5000; i++ { + if _, err := tx.ExecContext(ctx, + "INSERT INTO cancel_load (id, payload) VALUES ("+strconv.Itoa(i)+", '"+payload+"')"); err != nil { + _ = tx.Rollback() + t.Fatalf("seedCancelLoad: insert %d: %v", i, err) + } + } + if err := tx.Commit(); err != nil { + t.Fatalf("seedCancelLoad: commit: %v", err) + } +} + // RunDriverSuite exercises the full Driver contract against a real // database. ctor returns the driver-under-test; fx provides the env. // @@ -79,6 +123,13 @@ func RunDriverSuite(t *testing.T, ctor func() driver.Driver, fx Fixtures) { }) t.Run("Cancel_PropagatesToSubprocess", func(t *testing.T) { + // Inflate the database so the dump takes long enough that cancel() + // reliably lands while pg_dump (or the engine's equivalent) is still + // streaming. With only the round-trip subtest's tiny fixture, the dump + // can finish in well under the cancel delay on a fast host, making + // Backup return a clean nil and the assertion below spuriously fail. + seedCancelLoad(t, fx) + conn, err := d.Connect(context.Background(), fx.Profile) if err != nil { t.Fatalf("Connect: %v", err) From 15e24ddb29f7abbea07141b54e9a09c6c1b903fb Mon Sep 17 00:00:00 2001 From: Nikhil Rajput Date: Sun, 31 May 2026 19:06:14 +0530 Subject: [PATCH 11/11] docs: update project status table formatting in README --- README.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 8d981e7..34166a8 100644 --- a/README.md +++ b/README.md @@ -47,16 +47,16 @@ A single binary that turns the painful, error-prone sprawl of `pg_dump` → `pg_ ## Project status -| Phase | What it delivers | Status | -| --------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | -| **A** — Skeleton | Go module, Cobra CLI, TUI placeholder, `Driver` interface + registry, `errs`/`config`/`secrets`/`profile` packages, golangci-lint + depguard, cross-platform CI | ✅ Complete | -| **B** — Postgres walking skeleton | `backup`, `restore`, `sync`, `verify`, `inspect`, `dumps`, `config`, `profile` working end-to-end against PostgreSQL | ✅ Complete | -| **C** — TUI dashboard | Multi-panel Bubble Tea dashboard (profiles · dumps · jobs) with live job progress, backup/restore modal forms, and snapshot tests | ✅ Complete | +| Phase | What it delivers | Status | +| --------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------- | +| **A** — Skeleton | Go module, Cobra CLI, TUI placeholder, `Driver` interface + registry, `errs`/`config`/`secrets`/`profile` packages, golangci-lint + depguard, cross-platform CI | ✅ Complete | +| **B** — Postgres walking skeleton | `backup`, `restore`, `sync`, `verify`, `inspect`, `dumps`, `config`, `profile` working end-to-end against PostgreSQL | ✅ Complete | +| **C** — TUI dashboard | Multi-panel Bubble Tea dashboard (profiles · dumps · jobs) with live job progress, backup/restore modal forms, and snapshot tests | ✅ Complete | | **D** — Driver hardening | Shared cross-driver test harness (`RunDriverSuite`), capability-gating helper (`RequireCapability`), Postgres connection-probe retry, and a `docs/DRIVERS.md` contributor guide | ✅ Complete | -| **E** — MySQL + MariaDB | Both drivers via a shared `_mysqlcommon` package | ⏳ Planned | -| **F** — Advanced transfer | Incremental backups, bounded-buffer streaming, cross-engine sync, CDC | ⏳ Planned | -| **G** — Ops features | Cloud storage, secret backends, profile groups + 2FA, team mode, audit log, retention, telemetry | ⏳ Planned | -| **H** — Distribution | GoReleaser, Homebrew tap, Scoop bucket, install script, docs site | ⏳ Planned | +| **E** — MySQL + MariaDB | Both drivers via a shared `_mysqlcommon` package | ⏳ Planned | +| **F** — Advanced transfer | Incremental backups, bounded-buffer streaming, cross-engine sync, CDC | ⏳ Planned | +| **G** — Ops features | Cloud storage, secret backends, profile groups + 2FA, team mode, audit log, retention, telemetry | ⏳ Planned | +| **H** — Distribution | GoReleaser, Homebrew tap, Scoop bucket, install script, docs site | ⏳ Planned | ## Requirements