mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 12:18:59 +00:00
fix(backup): repair tenant backup/restore (config_secrets order, hooks registry, restore conn check) (#1357)
Closes #1076, #1338. Fix A — tenant backup aborted with SQLSTATE 42703 "column id does not exist" because exportQuery() hardcoded ORDER BY id. Several tenant-scoped tables have composite PKs and no id column. Add a TableDef.OrderBy field, honor it in exportQuery(), and set it for every id-less registry table: config_secrets, agent_team_members, tenant_hook_budget, system_configs, builtin_tool_tenant_configs, skill_tenant_configs, user_agent_profiles. Fix B — hooks, tenant_hook_budget and webhook config were missing from the backup registry, silently dropping their data on backup/restore. Add hooks, tenant_hook_budget, webhooks (preserve) plus the hook_agents junction (via ParentJoin through hooks, composite PK), and mark hook_executions and webhook_calls as ephemeral in the skipped list. Fix C — restoring on a fresh server failed with "N active DB connection(s) detected" because the gateway's own pool connections were counted as active clients. Tag pool connections with application_name='goclaw' (pg.OpenDB) and exclude them in CheckActiveConnections; genuine external clients still block. Adds unit + integration regression tests, including an export-over-every- registered-table test that surfaced the additional id-less tables.
This commit is contained in:
1 parent
7475ff3d6c
commit
25eaa0166f
7 files changed
+228
-13
No files matched your search
@@ -79,8 +79,12 @@ func CheckActiveConnections(ctx context.Context, dsn string) (int, error) {
|
||||
defer db.Close()
|
||||
|
||||
var count int
|
||||
// Exclude the gateway's own pool connections, tagged application_name='goclaw'
|
||||
// (see pg.PoolApplicationName). Otherwise a running gateway blocks its own
|
||||
// restore on a fresh server (issue #1338). External clients are still counted.
|
||||
query := `SELECT COUNT(*) FROM pg_stat_activity
|
||||
WHERE datname = $1 AND pid <> pg_backend_pid()`
|
||||
WHERE datname = $1 AND pid <> pg_backend_pid()
|
||||
AND application_name <> 'goclaw'`
|
||||
if err := db.QueryRowContext(ctx, query, creds.DBName).Scan(&count); err != nil {
|
||||
return 0, fmt.Errorf("query pg_stat_activity: %w", err)
|
||||
}
|
||||
|
||||
@@ -53,6 +53,8 @@ func ValidateTableRegistry(ctx context.Context, db *sql.DB) []string {
|
||||
"pairing_requests": true, "paired_devices": true,
|
||||
"channel_pending_messages": true, "cron_run_logs": true,
|
||||
"team_user_grants": true,
|
||||
// Ephemeral hook/webhook run history — not restored (regenerated at runtime).
|
||||
"hook_executions": true, "webhook_calls": true,
|
||||
}
|
||||
|
||||
var warnings []string
|
||||
|
||||
@@ -19,6 +19,8 @@ type TableDef struct {
|
||||
HasTenantID bool // direct tenant_id column
|
||||
ScopeColumn string // explicit filter column when not tenant_id (e.g. tenants.id)
|
||||
ParentJoin string // JOIN clause for tables without direct tenant_id
|
||||
OrderBy string // deterministic export ordering; defaults to "id" when empty.
|
||||
// Tables without an `id` column (composite PKs) MUST set this to real columns.
|
||||
}
|
||||
|
||||
// TenantTables returns all tenant-scoped tables in FK dependency order (parents first).
|
||||
@@ -35,7 +37,7 @@ func TenantTables() []TableDef {
|
||||
{Name: "agents", Tier: 2, HasTenantID: true},
|
||||
{Name: "sessions", Tier: 2, HasTenantID: true},
|
||||
{Name: "api_keys", Tier: 2, HasTenantID: true},
|
||||
{Name: "config_secrets", Tier: 2, HasTenantID: true},
|
||||
{Name: "config_secrets", Tier: 2, HasTenantID: true, OrderBy: "key"}, // PK (key, tenant_id) — no id column
|
||||
{Name: "skills", Tier: 2, HasTenantID: true},
|
||||
{Name: "mcp_servers", Tier: 2, HasTenantID: true},
|
||||
{Name: "secure_cli_binaries", Tier: 2, HasTenantID: true},
|
||||
@@ -43,11 +45,13 @@ func TenantTables() []TableDef {
|
||||
{Name: "channel_instances", Tier: 2, HasTenantID: true},
|
||||
{Name: "agent_teams", Tier: 2, HasTenantID: true},
|
||||
{Name: "llm_providers", Tier: 2, HasTenantID: true},
|
||||
{Name: "hooks", Tier: 2, HasTenantID: true},
|
||||
{Name: "tenant_hook_budget", Tier: 2, HasTenantID: true, OrderBy: "tenant_id"}, // PK tenant_id — no id column
|
||||
|
||||
// Tier 3: FK to Tier 2
|
||||
{Name: "agent_context_files", Tier: 3, HasTenantID: true},
|
||||
{Name: "user_context_files", Tier: 3, HasTenantID: true},
|
||||
{Name: "user_agent_profiles", Tier: 3, HasTenantID: true},
|
||||
{Name: "user_agent_profiles", Tier: 3, HasTenantID: true, OrderBy: "agent_id, user_id"}, // PK (agent_id, user_id) — no id column
|
||||
{Name: "user_agent_overrides", Tier: 3, HasTenantID: true},
|
||||
{Name: "episodic_summaries", Tier: 3, HasTenantID: true},
|
||||
{Name: "memory_documents", Tier: 3, HasTenantID: true},
|
||||
@@ -59,7 +63,7 @@ func TenantTables() []TableDef {
|
||||
{Name: "agent_evolution_suggestions", Tier: 3, HasTenantID: true},
|
||||
{Name: "channel_contacts", Tier: 3, HasTenantID: true},
|
||||
{Name: "subagent_tasks", Tier: 3, HasTenantID: true},
|
||||
{Name: "agent_team_members", Tier: 3, HasTenantID: true},
|
||||
{Name: "agent_team_members", Tier: 3, HasTenantID: true, OrderBy: "team_id, agent_id"}, // PK (team_id, agent_id) — no id column
|
||||
{Name: "agent_links", Tier: 3, HasTenantID: true},
|
||||
{Name: "agent_shares", Tier: 3, HasTenantID: true},
|
||||
{Name: "agent_config_permissions", Tier: 3, HasTenantID: true},
|
||||
@@ -71,9 +75,18 @@ func TenantTables() []TableDef {
|
||||
{Name: "mcp_user_credentials", Tier: 3, HasTenantID: true},
|
||||
{Name: "secure_cli_agent_grants", Tier: 3, HasTenantID: true},
|
||||
{Name: "secure_cli_user_credentials", Tier: 3, HasTenantID: true},
|
||||
{Name: "system_configs", Tier: 3, HasTenantID: true},
|
||||
{Name: "builtin_tool_tenant_configs", Tier: 3, HasTenantID: true},
|
||||
{Name: "skill_tenant_configs", Tier: 3, HasTenantID: true},
|
||||
{Name: "system_configs", Tier: 3, HasTenantID: true, OrderBy: "key"}, // PK (key, tenant_id) — no id column
|
||||
{Name: "builtin_tool_tenant_configs", Tier: 3, HasTenantID: true, OrderBy: "tool_name"}, // PK (tool_name, tenant_id) — no id column
|
||||
{Name: "skill_tenant_configs", Tier: 3, HasTenantID: true, OrderBy: "skill_id"}, // PK (skill_id, tenant_id) — no id column
|
||||
{Name: "webhooks", Tier: 3, HasTenantID: true}, // FK -> agents, channel_instances
|
||||
// hook_agents has no tenant_id — filter via JOIN hooks; composite PK (hook_id, agent_id).
|
||||
{
|
||||
Name: "hook_agents",
|
||||
Tier: 3,
|
||||
HasTenantID: false,
|
||||
ParentJoin: "hook_agents vl JOIN hooks h ON vl.hook_id = h.id WHERE h.tenant_id = $1",
|
||||
OrderBy: "vl.hook_id, vl.agent_id",
|
||||
},
|
||||
|
||||
// Tier 4: FK to Tier 3
|
||||
{Name: "kg_relations", Tier: 4, HasTenantID: true},
|
||||
@@ -105,7 +118,13 @@ func (t TableDef) tenantFilterColumn() string {
|
||||
|
||||
func (t TableDef) exportQuery() (string, error) {
|
||||
if t.ParentJoin != "" {
|
||||
return fmt.Sprintf("SELECT vl.* FROM %s ORDER BY vl.id", t.ParentJoin), nil
|
||||
// ParentJoin aliases the child table as `vl`. Order by OrderBy when set
|
||||
// (required for composite-PK children like hook_agents that have no id).
|
||||
orderBy := t.OrderBy
|
||||
if orderBy == "" {
|
||||
orderBy = "vl.id"
|
||||
}
|
||||
return fmt.Sprintf("SELECT vl.* FROM %s ORDER BY %s", t.ParentJoin, orderBy), nil
|
||||
}
|
||||
|
||||
column := t.tenantFilterColumn()
|
||||
@@ -113,7 +132,11 @@ func (t TableDef) exportQuery() (string, error) {
|
||||
return "", fmt.Errorf("table %s: no tenant filter defined", t.Name)
|
||||
}
|
||||
|
||||
return fmt.Sprintf("SELECT * FROM %s WHERE %s = $1 ORDER BY id", t.Name, column), nil
|
||||
orderBy := t.OrderBy
|
||||
if orderBy == "" {
|
||||
orderBy = "id"
|
||||
}
|
||||
return fmt.Sprintf("SELECT * FROM %s WHERE %s = $1 ORDER BY %s", t.Name, column, orderBy), nil
|
||||
}
|
||||
|
||||
func (t TableDef) deleteQuery() (string, error) {
|
||||
|
||||
@@ -1,6 +1,76 @@
|
||||
package backup
|
||||
|
||||
import "testing"
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestExportQueryNeverOrdersByMissingIdColumn guards issue #1076/#1338: several
|
||||
// tenant-scoped tables have no `id` column (config_secrets PK is (key,tenant_id),
|
||||
// agent_team_members PK is (team_id,agent_id), tenant_hook_budget PK is tenant_id).
|
||||
// exportQuery() previously hardcoded `ORDER BY id`, so exporting them failed with
|
||||
// SQLSTATE 42703 "column id does not exist". Their registry entries must set an
|
||||
// OrderBy that references real columns.
|
||||
func TestExportQueryNeverOrdersByMissingIdColumn(t *testing.T) {
|
||||
idless := map[string]bool{
|
||||
"config_secrets": true,
|
||||
"agent_team_members": true,
|
||||
"tenant_hook_budget": true,
|
||||
"user_agent_profiles": true,
|
||||
"system_configs": true,
|
||||
"builtin_tool_tenant_configs": true,
|
||||
"skill_tenant_configs": true,
|
||||
}
|
||||
lookup := make(map[string]TableDef)
|
||||
for _, tbl := range TenantTables() {
|
||||
lookup[tbl.Name] = tbl
|
||||
}
|
||||
for name := range idless {
|
||||
tbl, ok := lookup[name]
|
||||
if !ok {
|
||||
t.Errorf("%s missing from backup registry", name)
|
||||
continue
|
||||
}
|
||||
q, err := tbl.exportQuery()
|
||||
if err != nil {
|
||||
t.Errorf("%s exportQuery error: %v", name, err)
|
||||
continue
|
||||
}
|
||||
if strings.Contains(q, "ORDER BY id") {
|
||||
t.Errorf("%s export orders by missing id column: %q", name, q)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestHookWebhookFamilyRegistered guards issue #1076: hooks + budget + webhook
|
||||
// config tables were missing from the backup registry, so their data was silently
|
||||
// dropped on tenant backup/restore. The junction hook_agents (no tenant_id) must
|
||||
// export via a ParentJoin through hooks and order by its composite PK.
|
||||
func TestHookWebhookFamilyRegistered(t *testing.T) {
|
||||
lookup := make(map[string]TableDef)
|
||||
for _, tbl := range TenantTables() {
|
||||
lookup[tbl.Name] = tbl
|
||||
}
|
||||
for _, name := range []string{"hooks", "tenant_hook_budget", "webhooks", "hook_agents"} {
|
||||
if _, ok := lookup[name]; !ok {
|
||||
t.Errorf("%s missing from backup registry", name)
|
||||
}
|
||||
}
|
||||
ha, ok := lookup["hook_agents"]
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
q, err := ha.exportQuery()
|
||||
if err != nil {
|
||||
t.Fatalf("hook_agents exportQuery error: %v", err)
|
||||
}
|
||||
if !strings.Contains(q, "JOIN hooks") {
|
||||
t.Errorf("hook_agents export must join hooks for tenant scope: %q", q)
|
||||
}
|
||||
if strings.Contains(q, "ORDER BY vl.id") {
|
||||
t.Errorf("hook_agents has no id column; export must not order by vl.id: %q", q)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTenantTablesIncludesTenantUsersAndTenantScope(t *testing.T) {
|
||||
tables := TenantTables()
|
||||
|
||||
@@ -5,15 +5,29 @@ import (
|
||||
"fmt"
|
||||
"log/slog"
|
||||
|
||||
_ "github.com/jackc/pgx/v5/stdlib"
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/stdlib"
|
||||
)
|
||||
|
||||
// PoolApplicationName tags every gateway pool connection via the PostgreSQL
|
||||
// application_name runtime parameter. Pre-restore safety checks use it to tell
|
||||
// the gateway's own connections apart from external clients (issue #1338).
|
||||
const PoolApplicationName = "goclaw"
|
||||
|
||||
// OpenDB creates a database/sql connection to Postgres using pgx driver.
|
||||
func OpenDB(dsn string) (*sql.DB, error) {
|
||||
db, err := sql.Open("pgx", dsn)
|
||||
config, err := pgx.ParseConfig(dsn)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("open postgres: %w", err)
|
||||
return nil, fmt.Errorf("parse postgres dsn: %w", err)
|
||||
}
|
||||
if config.RuntimeParams == nil {
|
||||
config.RuntimeParams = map[string]string{}
|
||||
}
|
||||
// Respect a caller-supplied application_name; otherwise tag as the gateway.
|
||||
if config.RuntimeParams["application_name"] == "" {
|
||||
config.RuntimeParams["application_name"] = PoolApplicationName
|
||||
}
|
||||
db := stdlib.OpenDB(*config)
|
||||
|
||||
db.SetMaxOpenConns(25)
|
||||
db.SetMaxIdleConns(10)
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
//go:build integration
|
||||
|
||||
package integration
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"os"
|
||||
"testing"
|
||||
|
||||
_ "github.com/jackc/pgx/v5/stdlib"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/backup"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store/pg"
|
||||
)
|
||||
|
||||
// TestBackup_CheckActiveConnections_ExcludesGatewayPool is a regression test for
|
||||
// issue #1338: restoring on a fresh server failed with "N active DB connection(s)
|
||||
// detected" because the gateway's own pool connections were counted as active
|
||||
// clients. Gateway pool connections are tagged application_name='goclaw' and must
|
||||
// be excluded; genuinely external connections must still be counted.
|
||||
func TestBackup_CheckActiveConnections_ExcludesGatewayPool(t *testing.T) {
|
||||
testDB(t) // skips if PG unavailable
|
||||
dsn := os.Getenv("TEST_DATABASE_URL")
|
||||
if dsn == "" {
|
||||
dsn = defaultTestDSN
|
||||
}
|
||||
ctx := context.Background()
|
||||
|
||||
base, err := backup.CheckActiveConnections(ctx, dsn)
|
||||
if err != nil {
|
||||
t.Fatalf("baseline CheckActiveConnections: %v", err)
|
||||
}
|
||||
|
||||
// A gateway-tagged pool connection must NOT be counted.
|
||||
pool, err := pg.OpenDB(dsn)
|
||||
if err != nil {
|
||||
t.Fatalf("open gateway pool: %v", err)
|
||||
}
|
||||
defer pool.Close()
|
||||
var one int
|
||||
if err := pool.QueryRowContext(ctx, "SELECT 1").Scan(&one); err != nil {
|
||||
t.Fatalf("warm gateway pool: %v", err)
|
||||
}
|
||||
|
||||
after, err := backup.CheckActiveConnections(ctx, dsn)
|
||||
if err != nil {
|
||||
t.Fatalf("CheckActiveConnections after gateway pool: %v", err)
|
||||
}
|
||||
if after != base {
|
||||
t.Errorf("gateway pool connection was counted as active: base=%d after=%d", base, after)
|
||||
}
|
||||
|
||||
// A non-gateway connection MUST be counted.
|
||||
external, err := sql.Open("pgx", dsn)
|
||||
if err != nil {
|
||||
t.Fatalf("open external conn: %v", err)
|
||||
}
|
||||
defer external.Close()
|
||||
if err := external.QueryRowContext(ctx, "SELECT 1").Scan(&one); err != nil {
|
||||
t.Fatalf("warm external conn: %v", err)
|
||||
}
|
||||
withExternal, err := backup.CheckActiveConnections(ctx, dsn)
|
||||
if err != nil {
|
||||
t.Fatalf("CheckActiveConnections with external: %v", err)
|
||||
}
|
||||
if withExternal <= after {
|
||||
t.Errorf("external connection not counted: after=%d withExternal=%d", after, withExternal)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,32 @@
|
||||
//go:build integration
|
||||
|
||||
package integration
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/nextlevelbuilder/goclaw/internal/backup"
|
||||
)
|
||||
|
||||
// TestBackup_ExportAllRegistryTables_NoSQLError is an end-to-end regression for
|
||||
// issue #1076/#1338: exporting config_secrets (and other composite-PK tables)
|
||||
// failed with SQLSTATE 42703 "column id does not exist" because exportQuery
|
||||
// hardcoded ORDER BY id. PostgreSQL validates the ORDER BY column at plan time,
|
||||
// so the export fails even for an empty tenant. Running ExportTable over every
|
||||
// registered table proves each query is valid against the real schema — including
|
||||
// the hook_agents ParentJoin.
|
||||
func TestBackup_ExportAllRegistryTables_NoSQLError(t *testing.T) {
|
||||
db := testDB(t)
|
||||
tenantID, _ := seedTenantAgent(t, db)
|
||||
|
||||
for _, tbl := range backup.TenantTables() {
|
||||
t.Run(tbl.Name, func(t *testing.T) {
|
||||
var buf bytes.Buffer
|
||||
if _, err := backup.ExportTable(context.Background(), db, tbl, tenantID, &buf); err != nil {
|
||||
t.Errorf("ExportTable(%s): %v", tbl.Name, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user