diff --git a/.task/feature-rdm-two-trees-project-audit.md b/.task/feature-rdm-two-trees-project-audit.md index f72e33b..6eb2de3 100644 --- a/.task/feature-rdm-two-trees-project-audit.md +++ b/.task/feature-rdm-two-trees-project-audit.md @@ -5,6 +5,7 @@ - R0: Backend authoritative `origin/main` `5d5fc9891c0eb8994545ccb8b740535d9b99fb0e`; main was clean and equal to the fetched remote before this worktree was created. Core gitlink in this worktree is `5773e74086791731059730bd3726dc8c9cd1381f`. - Startup receipt: `/tmp/backend-w9-precheck-20261014-01.json` (SELF, exit 2, `SYNC_REQUIRED` for the separate Dev checkout `feature/rdm-increment-acceptance-note`, six commits behind Dev `origin/main`). This Backend task uses the independent-development path in Backend `AGENTS.md`; it does not write that Dev checkout or its task card. The receipt is preserved as a failure, not relabeled READY. - Approved affected design: `rdm-two-trees-stage6-audit-link-design-adjustment-r1.md`, SHA256 `385d9d7570217ddcce9fe60a60c99dc58c9a4875ad790584a724d09a210a6e52`; user approval in the task session covers this local Backend increment, not tag publication or Stage 7 acceptance. +- Approved PostgreSQL cutoff adjustment: `rdm-two-trees-stage6-w9-pg-audit-cutoff-adjustment-r1.md`, SHA256 `2357b715a7c8f3451b34cc25a578d655e8a48f3f26d9cf7561ea7a13998ca857`; scope is isolated Backend implementation/testing, with no real database migration or release authorization. ## Implemented local slice @@ -16,14 +17,19 @@ - Independent review of `a8fa2fe` found two P2 security defects: clock-based operation IDs could collide; and `NewScopedHost(owner,"rdm")` let another compiled plugin self-report the publisher's plugin ID. The new local increment uses 192 bits from `crypto/rand` for each host-generated operation ID. Both zero-offset and explicit-offset publication require a unique committed row for owner/project/operation; a historical duplicate rejects publication, including attempts to link another private command. Verifier registration returns a private `ProjectAuditPublisherFactory` for the registering plugin's background outbox; a generic scoped Host has no publisher method. Fixed-clock concurrent IDs, forged params, ambiguous private offset, forged scoped Host, and legitimate registered publisher each have focused tests. - The minted publisher exposes only `ProjectAuditPublisher`, not the full owner-scoped `Host`; tests reject attempts to use `NewScopedHost` with a self-reported `rdm` string as publisher authority. A publisher is a registration-time capability, while `IdempotencyKey` remains the separate retry identity. - Exact-source linked reads do not enumerate hidden links or reveal raw payload, private conversation ID, or content. Body disclosure and a broad project link list are not implemented; the public coverage reports the narrower capability. +- A PostgreSQL-only v33 default function now takes a table-specific transaction advisory lock before assigning `command_audit.id`, making supported implicit-ID writes commit in source-offset order. Migration and each PG append/project page verify CACHE 1, ascending non-cyclic positive IDs, the exact guarded default expression, and the next ID above the committed maximum; unsafe drift fails closed. SQLite remains single-writer. The owner-only project page reports `committed-prefix` only after the runtime check. Explicit online IDs and sequence maintenance remain prohibited; offline `storage-migrate` advances the owned sequence after copying IDs. ## Verification and remaining work - The session saw RED for the missing store and host interfaces, unscoped cutoff, cross-member private disclosure, and conversation-project reassignment before the respective fixes. These RED outputs were not captured by `make evidence`, so this card does not use them as formal evidence. - SELF receipt `/tmp/backend-w9-go-test-20261014-01.json`: `go test ./...`, exit 0, run on the pre-commit worktree based on R0. - SELF receipt `/tmp/backend-w9-build-20261014-01.json`: `CGO_ENABLED=0 go build ./cmd/agent-system`, exit 0, run on that pre-commit tree. Generated local binary removed after the build. -- SELF PostgreSQL receipt `/tmp/backend-w9-pg-cutoff-20261014-01.json`: isolated PostgreSQL 18.6 instance on port 55442 reproduced a low-ID transaction committing after a higher visible ID. `/tmp/backend-w9-pg-link-20261014-02.json` verifies the new link table, retry/conflict behavior, and the cutoff counterexample against isolated schemas. Both exited 0. The current PG cutoff means only `offset-upper-bound-only`; it is **not** a fixed historical visibility snapshot or safe incremental cursor. +- Historical SELF PostgreSQL receipt `/tmp/backend-w9-pg-cutoff-20261014-01.json`: isolated PostgreSQL 18.6 instance on port 55442 reproduced a low-ID transaction committing after a higher visible ID. `/tmp/backend-w9-pg-link-20261014-02.json` verifies the link table, retry/conflict behavior, and the pre-v33 cutoff counterexample. These receipts describe the old `offset-upper-bound-only` candidate, not the new v33 guarantee. - SELF PostgreSQL receipt `/tmp/backend-w9-pg-migrate-20261014-01.json`: isolated SQLite→PostgreSQL migration preserved one linked receipt and advanced its target sequence; the same migration ID replayed idempotently. The temporary PG instance was stopped and removed after verification. - SELF PostgreSQL receipt `/tmp/backend-w9-pg-operation-20261014-01.json`: a fresh isolated PG instance reran the link lookup, idempotency/conflict, and low-ID late-commit checks after trusted operation-offset resolution was added. The instance was stopped and removed. -- Integration constraint: this branch uses schema v31 (audit attribution) and v32 (links), while the independent W6 Backend branch also has a local v31. Neither branch has consumed the other's unmerged work. After the first branch lands, the later branch owner must sync the new main, renumber its unpublished migrations without rewriting an already released version, and rerun fresh/old SQLite and PG migration plus combined regression on the actual merged source. These isolated tests do not prove W6+W9 compatibility. -- Required next W9 work: add a committed-order waterline or materialize the authorized ID set at the first read, then prove with PG low-ID late commits that history and cross-page incremental reads neither omit nor duplicate operations. The Backend project-event source, actual RDM committed-event verifier/outbox consumer, run ID propagation from command dispatch, model/tool/background receipts, body/detail disclosure, broad authorized list pagination, history/export, and combined product verification remain unfinished. A domain commit followed by a lost Backend audit append leaves the link pending; recovery must retain and reconcile that unknown state without claiming a host receipt. The exact-link slice is not Stage 7 audit acceptance. +- New PG cross-page RED receipt `/tmp/backend-w9-pg-crosspage-red-20261014-01.json`: before v33, page 1 returned offsets 4 and 2, page 2 returned 1 after offset 3 committed, and the paged view omitted 3. The same test now passes with v33. Isolated PG tests also cover CACHE 10 counterexample/rejection, CACHE 1, sequence rewind into a vacant low ID, down/up with unchanged link IDs, advisory-lock timeout and rollback gaps, and SQLite→PG explicit-ID migration. +- Historical pre-commit SELF load receipt `/tmp/backend-w9-pg-load-precommit-20261014-01.json` captured an initial v33 variant whose unlocked status function later produced false `cutoff unsafe` errors under concurrency; it is **not** valid performance acceptance evidence. The corrected status check reads committed MAX before non-MVCC sequence state and does not lock readers; the default ID allocator alone holds the transaction advisory lock and rechecks after `nextval`. `TestProjectAuditConcurrentReadWriteNeverReportsFalseDrift` and repeated PG load/cross-page runs protect that ordering. Three-condition pre-commit SELF raw receipts: `/tmp/backend-w9-pg-load-readlock-precommit-20261014-01.json` uses a test-only locked status wrapper, while `/tmp/backend-w9-pg-load-unlocked2-precommit-20261014-01.json` uses the final MAX-first unlocked status; each run also logs its v32 direct-INSERT baseline, PostgreSQL 18.6 version, 192 writes per mode/8 workers, elapsed time, p95, and throughput. The wrapper adds overhead and all measurements vary with hardware/load; no performance threshold has been approved, so they are Stage 6 cost evidence only. +- Pre-commit SELF `/tmp/backend-w9-default-drift-red-20261014-01.json` showed that a replacement default containing the guarded function name but executing unguarded `nextval` was accepted; `/tmp/backend-w9-default-drift-green-20261014-01.json` verifies exact-expression rejection. Pre-commit SELF `/tmp/backend-w9-precommit-all-20261014-02.json` and `/tmp/backend-w9-precommit-pg-20261014-02.json` passed before this last guard tightening; fixed-HEAD runs are required below. +- After the guard tightening, pre-commit SELF receipts `/tmp/backend-w9-precommit-all-final-20261014-01.json` (`go test ./... -count=1`), `/tmp/backend-w9-precommit-pg-final-20261014-01.json` (isolated PG store/storage-migrate), and `/tmp/backend-w9-precommit-pg-race-20261014-01.json` (cross-page/concurrent read-write with race detector) all exited 0. These still identify the dirty pre-commit tree, so they do not replace fixed-HEAD verification. +- Integration constraint: this branch uses schema v31 (audit attribution), v32 (links), and v33 (PG cutoff), while the independent W6 Backend branch also has a local v31. Neither branch has consumed the other's unmerged work. After the first branch lands, the later branch owner must sync the new main, renumber its unpublished migrations without rewriting an already released version, and rerun fresh/old SQLite and PG migration plus combined regression on the actual merged source. These isolated tests do not prove W6+W9 compatibility. +- Required next W9 work: real RDM committed-event verifier/outbox consumer, Backend project-event source, run ID propagation, model/tool/background receipts, body/detail disclosure, broad authorized linked-operation pagination, history/export, and combined product verification remain unfinished. A domain commit followed by a lost Backend audit append leaves the link pending; recovery must retain and reconcile that unknown state without claiming a host receipt. Exact-link and owner-only paging do not satisfy Stage 7 audit acceptance. PostgreSQL v33 adds a global command-audit append lock; its measured wait/throughput cost has no approved performance threshold and needs human Stage 6 review. diff --git a/cmd/storage-migrate/main.go b/cmd/storage-migrate/main.go index 884ca51..3a4f366 100644 --- a/cmd/storage-migrate/main.go +++ b/cmd/storage-migrate/main.go @@ -752,30 +752,34 @@ func sqliteBool(v any) bool { // 后续应用插入会撞主键)。无序列表为空操作。 func advanceSequences(db *sql.DB, table string) error { rows, err := db.Query(` - SELECT pg_get_serial_sequence($1, column_name) + SELECT column_name, pg_get_serial_sequence(format('%I.%I',table_schema,table_name), column_name) FROM information_schema.columns - WHERE table_schema=current_schema() AND table_name=$1 - AND (column_default LIKE 'nextval%' OR is_identity='YES')`, table) + WHERE table_schema=current_schema() AND table_name=$1`, table) if err != nil { return err } defer rows.Close() - var seqs []string + var seqs []struct{ name, column string } for rows.Next() { - var s any - if err := rows.Scan(&s); err != nil { + var column string + var seq sql.NullString + if err := rows.Scan(&column, &seq); err != nil { return err } - if s != nil { - seqs = append(seqs, s.(string)) + if seq.Valid { + seqs = append(seqs, struct{ name, column string }{seq.String, column}) } } if err := rows.Err(); err != nil { return err } for _, s := range seqs { - if _, err := db.Exec(fmt.Sprintf(`SELECT setval('%s', COALESCE((SELECT MAX(id) FROM "%s"), 1))`, s, table)); err != nil { - return fmt.Errorf("setval %s: %w", s, err) + // Sequence ownership remains authoritative when a column's default is a + // guarded function rather than a literal nextval expression. + column := `"` + strings.ReplaceAll(s.column, `"`, `""`) + `"` + tableName := `"` + strings.ReplaceAll(table, `"`, `""`) + `"` + if _, err := db.Exec(fmt.Sprintf(`SELECT setval($1::regclass, COALESCE((SELECT MAX(%s) FROM %s), 1))`, column, tableName), s.name); err != nil { + return fmt.Errorf("setval %s (%s): %w", s.name, s.column, err) } } return nil diff --git a/internal/app/project_audit.go b/internal/app/project_audit.go index 8a005cc..43a7183 100644 --- a/internal/app/project_audit.go +++ b/internal/app/project_audit.go @@ -274,15 +274,11 @@ func (h inprocHost) QueryProjectAudit(ctx context.Context, q extension.ProjectAu if err != nil { return extension.ProjectAuditPage{}, err } - cutoffConsistency := "committed-prefix" - if h.caps.st.Dialect() == "postgres" { - cutoffConsistency = "offset-upper-bound-only" - } page := extension.ProjectAuditPage{ Receipts: make([]extension.HostAuditReceipt, 0, len(rows)), NextOffset: next, CutoffOffset: cutoff, Coverage: extension.ProjectAuditCoverage{ - CommandAudit: "actor-owned-project-bound-only", CutoffConsistency: cutoffConsistency, CrossMember: "unsupported", LegacyProjectBinding: "not-recorded", RunRelations: "unsupported", ModelReceipts: "unsupported", + CommandAudit: "actor-owned-project-bound-only", CutoffConsistency: "committed-prefix", CrossMember: "unsupported", LegacyProjectBinding: "not-recorded", RunRelations: "unsupported", ModelReceipts: "unsupported", BackgroundReceipts: "unsupported", ResourceBodies: "unsupported", }, } diff --git a/internal/store/audit.go b/internal/store/audit.go index d77301a..6894c02 100644 --- a/internal/store/audit.go +++ b/internal/store/audit.go @@ -4,6 +4,7 @@ import ( "database/sql" "encoding/json" "errors" + "fmt" "time" agentcommand "agent-command" @@ -13,6 +14,9 @@ import ( // owner 是租户身份(认证注入),与信封里的 Actor(发起主体)语义不同:查询按 // owner 过滤,跨租户审计不可见。 func (s *Store) AppendAudit(owner string, env agentcommand.AuditEnvelope) error { + if err := s.requireProjectAuditCommittedPrefix(); err != nil { + return err + } raw, err := json.Marshal(env) if err != nil { return err @@ -33,6 +37,24 @@ type ProjectCommandAuditRow struct { Envelope agentcommand.AuditEnvelope } +// ErrAuditCutoffUnsafe prevents a PostgreSQL page from claiming a fixed cutoff +// when the installed ID generator or sequence no longer proves commit order. +var ErrAuditCutoffUnsafe = errors.New("store: project audit cutoff unsafe") + +func (s *Store) requireProjectAuditCommittedPrefix() error { + if s.dialect.name() != "postgres" { + return nil + } // SQLite serializes writers. + var safe bool + if err := s.db.QueryRow(`SELECT project_audit_cutoff_safe('command_audit'::regclass)`).Scan(&safe); err != nil { + return fmt.Errorf("%w: %v", ErrAuditCutoffUnsafe, err) + } + if !safe { + return ErrAuditCutoffUnsafe + } + return nil +} + // ListProjectCommandAudits reads the project resolved from a trusted conversation // at append time. Reassigning or deleting that conversation cannot rewrite the // original project. Legacy rows without this snapshot are excluded: current @@ -42,6 +64,9 @@ func (s *Store) ListProjectCommandAudits(actorOwner, projectID string, before, c if actorOwner == "" || projectID == "" || before < 0 || cutoff < 0 || (before != 0 && cutoff == 0) { return nil, 0, 0, errors.New("project audit: invalid scope or cursor") } + if err := s.requireProjectAuditCommittedPrefix(); err != nil { + return nil, 0, 0, err + } if limit <= 0 || limit > 200 { limit = 50 } diff --git a/internal/store/migrate.go b/internal/store/migrate.go index 0c44edb..24a9763 100644 --- a/internal/store/migrate.go +++ b/internal/store/migrate.go @@ -866,6 +866,59 @@ func init() { "sqlite": {`DROP TABLE IF EXISTS project_audit_links`}, "postgres": {`DROP TABLE IF EXISTS project_audit_links`}, }}, + migration{version: 33, name: "project_audit_committed_prefix", + up: map[string][]string{ + // SQLite has one writer and one connection per Store; this migration + // only changes PostgreSQL's implicit command_audit ID allocation. + "sqlite": {`SELECT 1`}, + "postgres": { + `LOCK TABLE command_audit IN ACCESS EXCLUSIVE MODE`, + `CREATE FUNCTION project_audit_cutoff_safe(target regclass) RETURNS boolean LANGUAGE plpgsql VOLATILE AS $$ + DECLARE seq regclass; default_expr text; increment_by bigint; min_value bigint; start_value bigint; cache_size bigint; cycles boolean; + last_number numeric; was_called boolean; next_number numeric; min_id bigint; max_id bigint; + BEGIN + seq := pg_get_serial_sequence(target::text,'id')::regclass; + IF seq IS NULL THEN RETURN false; END IF; + SELECT pg_get_expr(d.adbin,d.adrelid) INTO default_expr FROM pg_attrdef d + JOIN pg_attribute a ON a.attrelid=d.adrelid AND a.attnum=d.adnum + WHERE d.adrelid=target AND a.attname='id'; + IF default_expr IS DISTINCT FROM format('next_command_audit_id(%L::regclass)',target::text) THEN RETURN false; END IF; + SELECT seqincrement,seqmin,seqstart,seqcache,seqcycle + INTO increment_by,min_value,start_value,cache_size,cycles FROM pg_sequence WHERE seqrelid=seq; + IF increment_by IS NULL OR increment_by<=0 OR min_value<=0 OR start_value<=0 OR cache_size<>1 OR cycles THEN RETURN false; END IF; + -- Supported writers allocate under the lock before they commit. Read + -- committed rows first, then the non-MVCC sequence state: a writer + -- committing between these reads can only advance the latter. + EXECUTE format('SELECT COALESCE(MIN(id),1),COALESCE(MAX(id),0) FROM %s',target) INTO min_id,max_id; + EXECUTE format('SELECT last_value,is_called FROM %s',seq) INTO last_number,was_called; + next_number := last_number; + IF was_called THEN next_number := last_number+increment_by; END IF; + RETURN min_id>0 AND next_number>0 AND next_number>max_id; + END $$`, + `CREATE FUNCTION next_command_audit_id(target regclass) RETURNS bigint LANGUAGE plpgsql VOLATILE AS $$ + DECLARE seq regclass; allocated bigint; max_id bigint; + BEGIN + PERFORM pg_advisory_xact_lock(715328491,target::oid::int4); + IF NOT project_audit_cutoff_safe(target) THEN RAISE EXCEPTION 'project audit cutoff sequence is unsafe'; END IF; + seq := pg_get_serial_sequence(target::text,'id')::regclass; + allocated := nextval(seq); + EXECUTE format('SELECT COALESCE(MAX(id),0) FROM %s',target) INTO max_id; + IF allocated<=0 OR allocated<=max_id THEN RAISE EXCEPTION 'project audit ID did not advance beyond committed rows'; END IF; + RETURN allocated; + END $$`, + `ALTER TABLE command_audit ALTER COLUMN id SET DEFAULT next_command_audit_id('command_audit'::regclass)`, + `DO $$ BEGIN IF NOT project_audit_cutoff_safe('command_audit'::regclass) THEN RAISE EXCEPTION 'project audit cutoff migration precondition failed'; END IF; END $$`, + }, + }, + down: map[string][]string{ + "sqlite": {`SELECT 1`}, + "postgres": { + `LOCK TABLE command_audit IN ACCESS EXCLUSIVE MODE`, + `ALTER TABLE command_audit ALTER COLUMN id SET DEFAULT nextval(pg_get_serial_sequence('command_audit','id')::regclass)`, + `DROP FUNCTION next_command_audit_id(regclass)`, + `DROP FUNCTION project_audit_cutoff_safe(regclass)`, + }, + }}, ) } diff --git a/internal/store/migrate_compat.go b/internal/store/migrate_compat.go index 9160ed1..a653f3b 100644 --- a/internal/store/migrate_compat.go +++ b/internal/store/migrate_compat.go @@ -37,6 +37,7 @@ var migrationBreaksOldReaders = map[int]bool{ 30: false, // project_event_publication_proofs:新表,旧读者不受影响 31: false, // command_audit_attribution_snapshot:仅加列/索引,旧读者不受影响 32: false, // project_audit_committed_links:新表,旧读者不受影响 + 33: false, // project_audit_committed_prefix:PG 审计 ID 默认函数,旧读者兼容 } // SchemaTarget 是本二进制会迁移到的 schema 版本。 diff --git a/internal/store/project_audit_sequence_test.go b/internal/store/project_audit_sequence_test.go new file mode 100644 index 0000000..022f2b8 --- /dev/null +++ b/internal/store/project_audit_sequence_test.go @@ -0,0 +1,500 @@ +package store + +import ( + "context" + "encoding/json" + "fmt" + "os" + "sort" + "strings" + "sync" + "testing" + "time" + + agentcommand "agent-command" +) + +func openProjectAuditPG(t *testing.T) *Store { + t.Helper() + if os.Getenv("AGENT_PG_TEST_DSN") == "" { + t.Skip("AGENT_PG_TEST_DSN unset: isolated PostgreSQL check NOT_RUN") + } + st := openTestStore(t) + if st.Dialect() != "postgres" { + t.Fatal("expected PostgreSQL") + } + return st +} + +func commandAuditSequenceName(t *testing.T, st *Store) string { + t.Helper() + var name string + if err := st.db.QueryRow(`SELECT pg_get_serial_sequence('command_audit','id')`).Scan(&name); err != nil { + t.Fatal(err) + } + if name == "" { + t.Fatal("command_audit has no owned sequence") + } + return name +} + +func TestProjectAuditSequenceMigrationRejectsCacheTen(t *testing.T) { + st := openProjectAuditPG(t) + if err := st.MigrateDown(32); err != nil { + t.Fatal(err) + } + seq := commandAuditSequenceName(t, st) + if _, err := st.db.Exec("ALTER SEQUENCE " + seq + " CACHE 10"); err != nil { + t.Fatal(err) + } + if err := st.migrate(); err == nil { + t.Fatal("migration accepted CACHE 10, which permits out-of-order IDs") + } + if _, err := st.db.Exec("ALTER SEQUENCE " + seq + " CACHE 1"); err != nil { + t.Fatal(err) + } + if err := st.migrate(); err != nil { + t.Fatalf("safe CACHE 1 migration: %v", err) + } +} + +func TestProjectAuditSequenceRuntimeRejectsConfigurationDrift(t *testing.T) { + st := openProjectAuditPG(t) + seq := commandAuditSequenceName(t, st) + if _, err := st.db.Exec("ALTER SEQUENCE " + seq + " CACHE 10"); err != nil { + t.Fatal(err) + } + if _, _, _, err := st.ListProjectCommandAudits("alice", "project", 0, 0, 10); err == nil { + t.Fatal("project audit reported a cutoff with CACHE 10") + } + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "unsafe-cache"}); err == nil { + t.Fatal("append allocated an ID with CACHE 10") + } + if _, err := st.db.Exec("ALTER SEQUENCE " + seq + " CACHE 1"); err != nil { + t.Fatal(err) + } + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "safe-cache"}); err != nil { + t.Fatalf("safe append after reset: %v", err) + } + if _, err := st.db.Exec("ALTER SEQUENCE " + seq + " CYCLE"); err != nil { + t.Fatal(err) + } + if _, _, _, err := st.ListProjectCommandAudits("alice", "project", 0, 0, 10); err == nil { + t.Fatal("read accepted cyclic audit IDs") + } + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "cyclic"}); err == nil { + t.Fatal("append accepted cyclic audit IDs") + } + if _, err := st.db.Exec("ALTER SEQUENCE " + seq + " NO CYCLE"); err != nil { + t.Fatal(err) + } + if _, err := st.db.Exec("ALTER SEQUENCE " + seq + " MINVALUE -10"); err != nil { + t.Fatal(err) + } + if _, _, _, err := st.ListProjectCommandAudits("alice", "project", 0, 0, 10); err == nil { + t.Fatal("read accepted a nonpositive audit ID range") + } + if _, err := st.db.Exec("ALTER SEQUENCE " + seq + " MINVALUE 1"); err != nil { + t.Fatal(err) + } + if _, err := st.db.Exec(`ALTER TABLE command_audit ALTER COLUMN id SET DEFAULT nextval(pg_get_serial_sequence('command_audit','id')::regclass)`); err != nil { + t.Fatal(err) + } + if _, _, _, err := st.ListProjectCommandAudits("alice", "project", 0, 0, 10); err == nil { + t.Fatal("read accepted an unguarded sequence default") + } + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "unguarded-default"}); err == nil { + t.Fatal("append accepted an unguarded sequence default") + } + if _, err := st.db.Exec(`ALTER TABLE command_audit ALTER COLUMN id SET DEFAULT CASE WHEN false THEN next_command_audit_id('command_audit'::regclass) ELSE nextval(pg_get_serial_sequence('command_audit','id')::regclass) END`); err != nil { + t.Fatal(err) + } + if _, _, _, err := st.ListProjectCommandAudits("alice", "project", 0, 0, 10); err == nil { + t.Fatal("read accepted a replaced default that merely mentions the guarded function") + } + if _, err := st.db.Exec(`ALTER TABLE command_audit ALTER COLUMN id SET DEFAULT next_command_audit_id('command_audit'::regclass)`); err != nil { + t.Fatal(err) + } +} + +func TestProjectAuditSequenceRejectsRewindIntoUnusedGap(t *testing.T) { + st := openProjectAuditPG(t) + p, err := st.CreateProject("alice", "A") + if err != nil { + t.Fatal(err) + } + c, err := st.CreateConversation("alice", string(p.ID), "work", "") + if err != nil { + t.Fatal(err) + } + appendAudit := func(id string) error { + return st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: id, ConversationID: string(c.ID), State: agentcommand.StateCommitted}) + } + if err := appendAudit("first"); err != nil { + t.Fatal(err) + } + seq := commandAuditSequenceName(t, st) + if _, err := st.db.Exec("SELECT nextval('" + seq + "'::regclass)"); err != nil { + t.Fatal(err) + } // consume one ID without a row + if err := appendAudit("third"); err != nil { + t.Fatal(err) + } + if _, err := st.db.Exec("SELECT setval('" + seq + "'::regclass,1)"); err != nil { + t.Fatal(err) + } + if err := appendAudit("rewound-into-gap"); err == nil { + t.Fatal("rewound sequence inserted a low ID into a committed-prefix gap") + } + if _, _, _, err := st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 10); err == nil { + t.Fatal("read claimed a safe cutoff after sequence rewind") + } + if _, err := st.db.Exec("SELECT setval('" + seq + "'::regclass,3)"); err != nil { + t.Fatal(err) + } + if err := appendAudit("recovered"); err != nil { + t.Fatalf("append after restoring sequence: %v", err) + } +} + +// Advisory locking alone does not fix cached sequence allocation: session A +// can hold 2..10 while session B commits 11, then A later commits 2. +func TestProjectAuditSequenceCacheTenDefeatsAdvisoryLock(t *testing.T) { + st := openProjectAuditPG(t) + for _, query := range []string{ + `CREATE TABLE cache_probe(id BIGSERIAL PRIMARY KEY)`, + `ALTER SEQUENCE cache_probe_id_seq CACHE 10`, + `CREATE FUNCTION cache_probe_next_id(target regclass) RETURNS bigint LANGUAGE plpgsql AS $$ + BEGIN PERFORM pg_advisory_xact_lock(715328491,target::oid::int4); + RETURN nextval(pg_get_serial_sequence(target::text,'id')::regclass); END $$`, + `ALTER TABLE cache_probe ALTER COLUMN id SET DEFAULT cache_probe_next_id('cache_probe'::regclass)`, + } { + if _, err := st.db.Exec(query); err != nil { + t.Fatal(err) + } + } + ctx := context.Background() + a, err := st.db.Conn(ctx) + if err != nil { + t.Fatal(err) + } + defer a.Close() + b, err := st.db.Conn(ctx) + if err != nil { + t.Fatal(err) + } + defer b.Close() + var first, other, late int64 + if err := a.QueryRowContext(ctx, `INSERT INTO cache_probe DEFAULT VALUES RETURNING id`).Scan(&first); err != nil { + t.Fatal(err) + } + if err := b.QueryRowContext(ctx, `INSERT INTO cache_probe DEFAULT VALUES RETURNING id`).Scan(&other); err != nil { + t.Fatal(err) + } + if err := a.QueryRowContext(ctx, `INSERT INTO cache_probe DEFAULT VALUES RETURNING id`).Scan(&late); err != nil { + t.Fatal(err) + } + if first != 1 || other != 11 || late != 2 { + t.Fatalf("CACHE 10 counterexample changed: A=%d B=%d A-late=%d", first, other, late) + } +} + +func TestProjectAuditSequenceNeverCommitsPreUpgradeCachedLowID(t *testing.T) { + st := openProjectAuditPG(t) + if err := st.MigrateDown(32); err != nil { + t.Fatal(err) + } + seq := commandAuditSequenceName(t, st) + if _, err := st.db.Exec("ALTER SEQUENCE " + seq + " CACHE 10"); err != nil { + t.Fatal(err) + } + ctx := context.Background() + a, err := st.db.Conn(ctx) + if err != nil { + t.Fatal(err) + } + defer a.Close() + b, err := st.db.Conn(ctx) + if err != nil { + t.Fatal(err) + } + defer b.Close() + insert := `INSERT INTO command_audit(time,command_id,source,entrance,state,operation_id,conversation_id,owner,payload) + VALUES ('2026-10-14T00:00:00Z','legacy','','','committed','','','alice','{}') RETURNING id` + var first, high, late int64 + if err := a.QueryRowContext(ctx, insert).Scan(&first); err != nil { + t.Fatal(err) + } + if err := b.QueryRowContext(ctx, insert).Scan(&high); err != nil { + t.Fatal(err) + } + if first != 1 || high != 11 { + t.Fatalf("old cached allocation: first=%d high=%d", first, high) + } + if _, err := st.db.Exec("ALTER SEQUENCE " + seq + " CACHE 1"); err != nil { + t.Fatal(err) + } + if err := st.migrate(); err != nil { + t.Fatalf("upgrade with idle cached connections: %v", err) + } + if err := a.QueryRowContext(ctx, insert).Scan(&late); err == nil && late <= high { + t.Fatalf("old connection committed late low ID %d below %d", late, high) + } + var count int + if err := st.db.QueryRow(`SELECT COUNT(*) FROM command_audit WHERE id=2`).Scan(&count); err != nil || count != 0 { + t.Fatalf("late cached row persisted: count=%d err=%v", count, err) + } +} + +func TestProjectAuditSequenceMigrationRoundTripPreservesLinks(t *testing.T) { + st := openProjectAuditPG(t) + if err := st.EnsureUser("alice", "Alice"); err != nil { + t.Fatal(err) + } + p, err := st.CreateProject("alice", "A") + if err != nil { + t.Fatal(err) + } + c, err := st.CreateConversation("alice", string(p.ID), "work", "") + if err != nil { + t.Fatal(err) + } + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "rdm.confirm", OperationID: "op-1", ConversationID: string(c.ID), State: agentcommand.StateCommitted}); err != nil { + t.Fatal(err) + } + rows, _, _, err := st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 10) + if err != nil { + t.Fatal(err) + } + offset := rows[0].Offset + link := ProjectAuditLink{HostAuditOffset: offset, ProjectID: string(p.ID), Owner: "alice", SourceKind: "domain-commit", SourceID: "commit-1", OperationID: "op-1", ResourceKind: "document", ResourceID: "doc-1", DisclosureClass: "metadata", IdempotencyKey: "outbox-1"} + if _, err := st.SaveProjectAuditLink(link); err != nil { + t.Fatal(err) + } + if err := st.MigrateDown(32); err != nil { + t.Fatalf("down to v32: %v", err) + } + if _, _, _, err := st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 10); err == nil { + t.Fatal("old default exposed a strong cutoff after down") + } + if _, row, err := st.ProjectAuditLinkedReceipt(string(p.ID), "domain-commit", "commit-1", offset); err != nil || row.Offset != offset { + t.Fatalf("link changed during down: %+v %v", row, err) + } + if err := st.migrate(); err != nil { + t.Fatalf("up to v33: %v", err) + } + rows, _, _, err = st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 10) + if err != nil || len(rows) != 1 || rows[0].Offset != offset { + t.Fatalf("restored audit: %+v %v", rows, err) + } + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "after-up", ConversationID: string(c.ID), State: agentcommand.StateCommitted}); err != nil { + t.Fatal(err) + } + rows, _, _, err = st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 10) + if err != nil || len(rows) != 2 || rows[0].Offset <= offset { + t.Fatalf("sequence not advanced after up: %+v %v", rows, err) + } +} + +func TestProjectAuditSequenceLockTimeoutAndRollback(t *testing.T) { + st := openProjectAuditPG(t) + p, err := st.CreateProject("alice", "A") + if err != nil { + t.Fatal(err) + } + c, err := st.CreateConversation("alice", string(p.ID), "work", "") + if err != nil { + t.Fatal(err) + } + tx, err := st.db.Begin() + if err != nil { + t.Fatal(err) + } + defer tx.Rollback() + insert := st.q(`INSERT INTO command_audit(time,command_id,source,entrance,state,operation_id,conversation_id,owner,payload,actor_name,project_id) + VALUES (?,?,'','','committed','',?,?,?,?,?) RETURNING id`) + args := []any{time.Now().UTC().Format(time.RFC3339Nano), "held", string(c.ID), "alice", `{}`, "Alice", string(p.ID)} + var heldID int64 + if err := tx.QueryRow(insert, args...).Scan(&heldID); err != nil { + t.Fatal(err) + } + ctx := context.Background() + conn, err := st.db.Conn(ctx) + if err != nil { + t.Fatal(err) + } + defer conn.Close() + if _, err := conn.ExecContext(ctx, `SET lock_timeout='150ms'`); err != nil { + t.Fatal(err) + } + var blockedID int64 + err = conn.QueryRowContext(ctx, insert, args...).Scan(&blockedID) + if err == nil || !strings.Contains(err.Error(), "lock timeout") { + t.Fatalf("second writer did not stop on advisory lock: id=%d err=%v", blockedID, err) + } + if _, err := conn.ExecContext(ctx, `RESET lock_timeout`); err != nil { + t.Fatal(err) + } + if err := tx.Rollback(); err != nil { + t.Fatal(err) + } + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "after-rollback", ConversationID: string(c.ID), State: agentcommand.StateCommitted}); err != nil { + t.Fatal(err) + } + rows, cutoff, _, err := st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 10) + if err != nil || len(rows) != 1 || rows[0].Offset <= heldID || cutoff != rows[0].Offset { + t.Fatalf("rollback gap affected cutoff: rows=%+v cutoff=%d held=%d err=%v", rows, cutoff, heldID, err) + } +} + +func TestProjectAuditSequenceConcurrentLoad(t *testing.T) { + st := openProjectAuditPG(t) + var serverVersion string + if err := st.db.QueryRow(`SHOW server_version`).Scan(&serverVersion); err != nil { + t.Fatal(err) + } + if err := st.EnsureUser("alice", "Alice"); err != nil { + t.Fatal(err) + } + p, err := st.CreateProject("alice", "A") + if err != nil { + t.Fatal(err) + } + c, err := st.CreateConversation("alice", string(p.ID), "work", "") + if err != nil { + t.Fatal(err) + } + const writers, each = 8, 24 + run := func(label string, write func(agentcommand.AuditEnvelope) error) (time.Duration, time.Duration) { + t.Helper() + durations := make([]time.Duration, writers*each) + errors := make(chan error, writers*each) + start := time.Now() + var wg sync.WaitGroup + for worker := 0; worker < writers; worker++ { + wg.Add(1) + go func(worker int) { + defer wg.Done() + for i := 0; i < each; i++ { + env := agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: fmt.Sprintf("%s-%d-%d", label, worker, i), ConversationID: string(c.ID), State: agentcommand.StateCommitted} + at := time.Now() + if err := write(env); err != nil { + errors <- err + return + } + durations[worker*each+i] = time.Since(at) + } + }(worker) + } + wg.Wait() + close(errors) + for err := range errors { + t.Fatalf("%s append: %v", label, err) + } + elapsed := time.Since(start) + sort.Slice(durations, func(i, j int) bool { return durations[i] < durations[j] }) + return elapsed, durations[len(durations)*95/100] + } + if err := st.MigrateDown(32); err != nil { + t.Fatal(err) + } + oldQuery := st.q(`INSERT INTO command_audit(time,command_id,source,entrance,state,operation_id,conversation_id,owner,payload,actor_name,project_id) + VALUES (?,?,?,?,?,?,?,?,?,COALESCE((SELECT name FROM users WHERE owner=?),''),COALESCE((SELECT project_id FROM conversations WHERE id=? AND owner=?),''))`) + oldElapsed, oldP95 := run("old", func(env agentcommand.AuditEnvelope) error { + raw, err := json.Marshal(env) + if err != nil { + return err + } + _, err = st.db.Exec(oldQuery, env.Time.UTC().Format(time.RFC3339Nano), env.CommandID, "", "", string(env.State), "", env.ConversationID, "alice", string(raw), env.Actor, env.ConversationID, "alice") + return err + }) + if err := st.migrate(); err != nil { + t.Fatalf("upgrade after baseline: %v", err) + } + statusMode := "MAX-first unlocked status" + if os.Getenv("AGENT_AUDIT_BENCH_READ_LOCK") == "1" { + // A test-only wrapper reconstructs the earlier status-read lock so the + // isolated load receipt can compare its cost against the final design. + if _, err := st.db.Exec(`ALTER FUNCTION project_audit_cutoff_safe(regclass) RENAME TO project_audit_cutoff_unlocked`); err != nil { + t.Fatal(err) + } + if _, err := st.db.Exec(`CREATE FUNCTION project_audit_cutoff_safe(target regclass) RETURNS boolean LANGUAGE plpgsql VOLATILE AS $$ + BEGIN PERFORM pg_advisory_xact_lock(715328491,target::oid::int4); + RETURN project_audit_cutoff_unlocked(target); END $$`); err != nil { + t.Fatal(err) + } + statusMode = "test-only locked status wrapper" + } + newElapsed, newP95 := run("guarded", func(env agentcommand.AuditEnvelope) error { return st.AppendAudit("alice", env) }) + t.Logf("isolated PostgreSQL %s audit append: %d writes per mode / %d workers; v32 direct INSERT=%s p95=%s (%.0f/s), v33 Store.AppendAudit with %s=%s p95=%s (%.0f/s); hardware/load dependent SELF only", + serverVersion, writers*each, writers, oldElapsed, oldP95, float64(writers*each)/oldElapsed.Seconds(), statusMode, newElapsed, newP95, float64(writers*each)/newElapsed.Seconds()) + seen := map[string]bool{} + var before, cutoff int64 + for { + page, fixed, next, err := st.ListProjectCommandAudits("alice", string(p.ID), before, cutoff, 100) + if err != nil { + t.Fatal(err) + } + if cutoff == 0 { + cutoff = fixed + } + for _, row := range page { + if seen[row.Envelope.CommandID] { + t.Fatalf("duplicate paged command: %s", row.Envelope.CommandID) + } + seen[row.Envelope.CommandID] = true + } + if next == 0 { + break + } + before = next + } + if len(seen) != 2*writers*each { + t.Fatalf("paged commands=%d want=%d", len(seen), 2*writers*each) + } +} + +func TestProjectAuditConcurrentReadWriteNeverReportsFalseDrift(t *testing.T) { + st := openProjectAuditPG(t) + p, err := st.CreateProject("alice", "A") + if err != nil { + t.Fatal(err) + } + c, err := st.CreateConversation("alice", string(p.ID), "work", "") + if err != nil { + t.Fatal(err) + } + const writers, writes, readers, reads = 6, 30, 4, 40 + errs := make(chan error, writers*writes+readers*reads) + var wg sync.WaitGroup + for worker := 0; worker < writers; worker++ { + wg.Add(1) + go func(worker int) { + defer wg.Done() + for i := 0; i < writes; i++ { + err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: fmt.Sprintf("write-%d-%d", worker, i), ConversationID: string(c.ID), State: agentcommand.StateCommitted}) + if err != nil { + errs <- fmt.Errorf("append: %w", err) + return + } + } + }(worker) + } + for reader := 0; reader < readers; reader++ { + wg.Add(1) + go func() { + defer wg.Done() + for i := 0; i < reads; i++ { + if _, _, _, err := st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 10); err != nil { + errs <- fmt.Errorf("read: %w", err) + return + } + } + }() + } + wg.Wait() + close(errs) + for err := range errs { + t.Fatal(err) + } +} diff --git a/internal/store/project_audit_test.go b/internal/store/project_audit_test.go index db91605..7234dd7 100644 --- a/internal/store/project_audit_test.go +++ b/internal/store/project_audit_test.go @@ -174,10 +174,10 @@ func TestProjectCommandAuditDoesNotMoveWithConversation(t *testing.T) { } } -// PostgreSQL sequence IDs are allocated before commit. This test records the -// current upper-bound semantics: a lower ID can appear after a page cutoff was -// captured, so callers must not describe the cutoff as a visibility snapshot. -func TestProjectAuditPostgresCutoffLateCommit(t *testing.T) { +// Under the old PostgreSQL SERIAL default, an uncommitted middle ID can be +// skipped after a higher committed row advances the page cursor past it. +// The committed-prefix implementation must keep the later writer waiting. +func TestProjectAuditPostgresCursorNeverSkipsLateCommit(t *testing.T) { if os.Getenv("AGENT_PG_TEST_DSN") == "" { t.Skip("AGENT_PG_TEST_DSN unset: PG concurrency check NOT_RUN") } @@ -196,7 +196,12 @@ func TestProjectAuditPostgresCutoffLateCommit(t *testing.T) { if err != nil { t.Fatal(err) } - env := agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "late-low-id", ConversationID: string(c.ID)} + for _, command := range []string{"lower-one", "lower-two"} { + if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: command, ConversationID: string(c.ID), State: agentcommand.StateCommitted}); err != nil { + t.Fatal(err) + } + } + env := agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "middle-pending", ConversationID: string(c.ID), State: agentcommand.StateCommitted} raw, err := json.Marshal(env) if err != nil { t.Fatal(err) @@ -206,29 +211,57 @@ func TestProjectAuditPostgresCutoffLateCommit(t *testing.T) { t.Fatal(err) } defer tx.Rollback() - var lowID int64 + var middleID int64 if err := tx.QueryRow(st.q(`INSERT INTO command_audit(time,command_id,source,entrance,state,operation_id,conversation_id,owner,payload,actor_name,project_id) - VALUES (?,?,'','','','',?,?,?,?,?) RETURNING id`), time.Now().UTC().Format(time.RFC3339Nano), "late-low-id", string(c.ID), "alice", string(raw), "Alice", string(p.ID)).Scan(&lowID); err != nil { + VALUES (?,?,'','','committed','',?,?,?,?,?) RETURNING id`), time.Now().UTC().Format(time.RFC3339Nano), "middle-pending", string(c.ID), "alice", string(raw), "Alice", string(p.ID)).Scan(&middleID); err != nil { t.Fatal(err) } - if err := st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "committed-high-id", ConversationID: string(c.ID)}); err != nil { - t.Fatal(err) + highDone := make(chan error, 1) + started := make(chan struct{}) + go func() { + close(started) + highDone <- st.AppendAudit("alice", agentcommand.AuditEnvelope{Time: time.Now(), Actor: "alice", CommandID: "higher-committed", ConversationID: string(c.ID), State: agentcommand.StateCommitted}) + }() + <-started + highCommittedEarly := false + select { + case err := <-highDone: + if err != nil { + t.Fatal(err) + } + highCommittedEarly = true + case <-time.After(250 * time.Millisecond): } - first, cutoff, _, err := st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 10) - if err != nil { - t.Fatal(err) - } - if len(first) != 1 || first[0].Envelope.CommandID != "committed-high-id" || lowID >= cutoff { - t.Fatalf("first page: rows=%+v low=%d cutoff=%d", first, lowID, cutoff) + var first []ProjectCommandAuditRow + var cutoff, next int64 + if highCommittedEarly { + first, cutoff, next, err = st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 2) + if err != nil { + tx.Rollback() + t.Fatal(err) + } } if err := tx.Commit(); err != nil { t.Fatal(err) } - later, fixed, _, err := st.ListProjectCommandAudits("alice", string(p.ID), 0, cutoff, 10) + if !highCommittedEarly { + if err := <-highDone; err != nil { + t.Fatal(err) + } + first, cutoff, next, err = st.ListProjectCommandAudits("alice", string(p.ID), 0, 0, 2) + if err != nil { + t.Fatal(err) + } + } + second, fixed, _, err := st.ListProjectCommandAudits("alice", string(p.ID), next, cutoff, 2) if err != nil { t.Fatal(err) } - if fixed != cutoff || len(later) != 2 || later[1].Envelope.CommandID != "late-low-id" { - t.Fatalf("late commit not exposed: rows=%+v cutoff=%d", later, fixed) + seen := map[string]bool{} + for _, row := range append(first, second...) { + seen[row.Envelope.CommandID] = true + } + if next == 0 || fixed != cutoff || len(seen) != 4 || !seen["middle-pending"] { + t.Fatalf("cross-page audit omitted a committed row: early=%v middleID=%d first=%+v second=%+v cutoff=%d next=%d", highCommittedEarly, middleID, first, second, cutoff, next) } } diff --git a/pkg/extension/extension.go b/pkg/extension/extension.go index 1812671..173e5f0 100644 --- a/pkg/extension/extension.go +++ b/pkg/extension/extension.go @@ -42,7 +42,9 @@ import ( // retryable ProjectAuditLinkRequest.HostAuditOffset=0 resolution. // 2.16.0-experimental: cryptographic operation IDs and registration-bound // ProjectAuditPublisherFactory; scoped Host string IDs cannot publish links. -const SurfaceVersion = "2.16.0-experimental" +// 2.17.0-experimental: PostgreSQL project audit pages require a verified +// committed-prefix ID generator; unsafe sequence/default drift rejects reads. +const SurfaceVersion = "2.17.0-experimental" // ErrNotLinked 表示进程没有空白导入 link 包,Main 无法启动服务。 var ErrNotLinked = errors.New("extension: runner not linked; blank-import agent-system-backend/pkg/extension/link") diff --git a/pkg/extension/project_audit.go b/pkg/extension/project_audit.go index ddca97c..d1592fe 100644 --- a/pkg/extension/project_audit.go +++ b/pkg/extension/project_audit.go @@ -102,9 +102,10 @@ type ProjectAuditPublisher interface { // ProjectAuditQuery reads one project-bound command-audit slice. Offset is a // host source offset, not a cross-domain event sequence. A nonzero BeforeOffset -// must be paired with the CutoffOffset returned on the first page. On PostgreSQL -// that cutoff bounds offsets but is not a fixed visibility snapshot: a lower ID -// from a still-open transaction may commit later. See Coverage.CutoffConsistency. +// must be paired with the CutoffOffset returned on the first page. PostgreSQL +// pages are served only while the installed generator and sequence prove a +// committed prefix; otherwise the host rejects the read instead of claiming a +// fixed snapshot. See Coverage.CutoffConsistency. type ProjectAuditQuery struct { ProjectID core.ID `json:"projectId"` BeforeOffset int64 `json:"beforeOffset,omitempty"`