oficina / fichas Public

Second review: entry types, explicit actors, and invariants with teeth

REQUEST CHANGES again, two P1s, both about rows that cannot be corrected
later. Verified before acting.

Entry types. AddEntry looked up the Call Log type by a constant regardless
of what it was asked for, so every entry kind bound to one type's field
definitions — and CreateCard would just as happily make a *card* of type
Call log. Entries are immutable (§3), so each such row is uncorrectable and
there is no backfill for an entry whose type was an accident. CardType
gains a Scope of card or entry, Entry gains a TypeID foreign key, and
typeByKey refuses the wrong scope on both paths.

Actors. ActorID was a bare int64, so zero meant both "member 0" and "the
app did this" — a handler that forgot to resolve the session wrote rows
indistinguishable from an honest machine write, and TriggerID was zero
everywhere so the honest machine write was indistinguishable from the
forgetful one in the other direction too. There is now an Actor type:
Person(id) or AppBecause(trigger, reason), validated in logEvent so no
write path can skip it. ActorID is a nullable foreign key to members.

Invariants that were conventions are now enforced. A hand-written migration
adds triggers: entries and entry values cannot be updated, entries cannot
be deleted, and a card cannot be hard-deleted — which previously cascaded
its values, entries and refs away while its events survived pointing at
nothing. "No function exists for it" is not an invariant.

Also: every store function takes a context and uses WithContext, because a
caller holding a transaction and passing the outer handle deadlocked the
single writer connection with no way to cancel it. The revision bump is
guarded on the revision it read and checks RowsAffected, so the invariant
is local rather than resting on the pool being one connection. An entry
landing is logged as entry_added on the CARD with the entry in ChildID, so
the card's revision chain is contiguous — it previously consumed a card
revision under object_kind=entry, which a contiguity check reads as broken.
ChangedFields is "[]" rather than the string "null", AddEntry's payload
order is deterministic, and a rolled-back id no longer escapes.

Both gates are stronger. The schema gate walked reflect.String only, so
`Amount int64` or `Birthday time.Time` would have passed — a salary and a
date of birth are content as readily as text. It now walks every column
type. The file gate only booted and seeded, so it never saw a value a
person typed; it now writes through the real store path and greps for the
values and the payload's own JSON keys. Its canary list had excluded
"Customer" on the claim that `t_customer` contains it, which is false —
strings.Contains is case-sensitive — so the exclusion weakened the gate for
no reason. Both gates caught real things while being fixed: the schema gate
failed on two plaintext columns added in this commit.

Ownership is written down rather than left undecided: OwnerID is
provenance, not authority, because fichas is one instance per team and the
team shares its cards.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev a45c9a71c40ed83f521085be2fa8aaff8f2b3a88 parent 15f8c22
10 files changed, +599 −149
  • docs/superpowers/specs/2026-09-19-fichas-design.md +5 −0
  • internal/fichas/catalogue.go +19 −3
  • internal/fichas/migrations/0001_init.sql +9 −5
  • internal/fichas/migrations/0002_immutable_entries.sql +43 −0
  • internal/fichas/migrations/schema.sql +9 −5
  • internal/fichas/models.go +60 −6
  • internal/fichas/models_test.go +50 −8
  • internal/fichas/store.go +154 −49
  • internal/fichas/store_test.go +151 −17
  • internal/fichastest/gates_test.go +99 −56

CI passed — run details

CI log
=== vet ===
go vet ./...
=== fmt ===
=== test ===
go test ./...
?   	amadan.net/oficina/fichas/cmd/fichas	[no test files]
ok  	amadan.net/oficina/fichas/internal/fichas	0.157s
?   	amadan.net/oficina/fichas/internal/fichas/icons	[no test files]
ok  	amadan.net/oficina/fichas/internal/fichas/seal	(cached)
ok  	amadan.net/oficina/fichas/internal/fichastest	0.039s
=== migration-check ===
go run github.com/carlosframework/rastrillo/cmd/rastrillo migration check
rastrillo migration check: models and migrations agree
diff --git a/docs/superpowers/specs/2026-09-19-fichas-design.md b/docs/superpowers/specs/2026-09-19-fichas-design.md
index 3afea8e..b67204e 100644
--- a/docs/superpowers/specs/2026-09-19-fichas-design.md
+++ b/docs/superpowers/specs/2026-09-19-fichas-design.md
@@ -57,6 +57,11 @@ apps, and a derived summary per card that cites its own entries.
Four objects. Two of them version (§5), two do not.
+- **Card type** — `(key, scope, label, fields)`. **Scope is `card` or
+ `entry`, never both**, added 2026-09-20: without it AddEntry bound every
+ entry to one type's definitions by a constant, and CreateCard would make
+ a card of type "Call log". Entries are immutable, so those are rows that
+ cannot be corrected.
- **Card** — a mutable entity. `(id, type, fields, created, revision)`.
Versions.
- **Entry** — append-only, belongs to a card. `(id, card_id, kind, body,
diff --git a/internal/fichas/catalogue.go b/internal/fichas/catalogue.go
index 143e8c5..6f4469c 100644
--- a/internal/fichas/catalogue.go
+++ b/internal/fichas/catalogue.go
@@ -72,8 +72,12 @@ type seedField struct {
}
type seedType struct {
- Key string
- Label string
+ Key string
+ Label string
+ // Scope is ScopeCard or ScopeEntry. A type is one or the other and
+ // never both: CreateCard refuses an entry type and AddEntry refuses a
+ // card type.
+ Scope string
Fields []seedField
}
@@ -83,6 +87,7 @@ var catalogue = []seedType{
{
Key: TypeCustomer,
Label: "Customer",
+ Scope: ScopeCard,
Fields: []seedField{
// Printed: high-cardinality, and the equality question is
// worth answering without the key — "is this the same
@@ -109,6 +114,7 @@ var catalogue = []seedType{
{
Key: TypeCallLog,
Label: "Call log",
+ Scope: ScopeEntry,
Fields: []seedField{
// Deliberately distinct from Entry.CreatedAt, which is
// plaintext because the feed needs to know a row was written.
@@ -160,6 +166,13 @@ func seedType_(tx *gorm.DB, s *seal.Sealer, t seedType) (CardType, error) {
var ct CardType
err := tx.Where("key = ?", t.Key).First(&ct).Error
if err == nil {
+ // Scope describes how every row under this type was written, so
+ // it may not drift any more than a field's Tier may.
+ if ct.Scope != t.Scope {
+ return ct, fmt.Errorf(
+ "seed: %s is stored with scope %q but the catalogue now says %q; a type is a card's or an entry's, never both",
+ t.Key, ct.Scope, t.Scope)
+ }
return ct, nil
}
if err != gorm.ErrRecordNotFound {
@@ -169,7 +182,10 @@ func seedType_(tx *gorm.DB, s *seal.Sealer, t seedType) (CardType, error) {
if err != nil {
return ct, fmt.Errorf("seed: sealing the label for %s: %w", t.Key, err)
}
- ct = CardType{Key: t.Key, SealedLabel: label}
+ if t.Scope != ScopeCard && t.Scope != ScopeEntry {
+ return ct, fmt.Errorf("seed: %s has scope %q, which is neither %q nor %q", t.Key, t.Scope, ScopeCard, ScopeEntry)
+ }
+ ct = CardType{Key: t.Key, Scope: t.Scope, SealedLabel: label}
if err := tx.Create(&ct).Error; err != nil {
return ct, fmt.Errorf("seed: creating %s: %w", t.Key, err)
}
diff --git a/internal/fichas/migrations/0001_init.sql b/internal/fichas/migrations/0001_init.sql
index 1b717f7..1fe3248 100644
--- a/internal/fichas/migrations/0001_init.sql
+++ b/internal/fichas/migrations/0001_init.sql
@@ -2,7 +2,9 @@ CREATE TABLE `members` (`id` integer PRIMARY KEY AUTOINCREMENT,`address_print` t
CREATE UNIQUE INDEX `idx_members_address_print` ON `members`(`address_print`);
-CREATE TABLE `card_types` (`id` integer PRIMARY KEY AUTOINCREMENT,`key` text,`sealed_label` blob,`created_at` datetime,`updated_at` datetime);
+CREATE TABLE `card_types` (`id` integer PRIMARY KEY AUTOINCREMENT,`key` text,`scope` text NOT NULL,`sealed_label` blob,`created_at` datetime,`updated_at` datetime);
+
+CREATE INDEX `idx_card_types_scope` ON `card_types`(`scope`);
CREATE UNIQUE INDEX `idx_card_types_key` ON `card_types`(`key`);
@@ -12,7 +14,7 @@ CREATE UNIQUE INDEX `idx_field_defs_key` ON `field_defs`(`key`);
CREATE INDEX `idx_field_defs_card_type_id` ON `field_defs`(`card_type_id`);
-CREATE TABLE `cards` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_type_id` integer NOT NULL,`revision` integer NOT NULL DEFAULT 1,`gone` numeric NOT NULL DEFAULT false,`offered` numeric NOT NULL DEFAULT false,`owner_id` integer,`created_at` datetime,`updated_at` datetime);
+CREATE TABLE `cards` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_type_id` integer NOT NULL,`revision` integer NOT NULL DEFAULT 1,`gone` numeric NOT NULL DEFAULT false,`offered` numeric NOT NULL DEFAULT false,`owner_id` integer,`created_at` datetime,`updated_at` datetime,CONSTRAINT `fk_cards_owner` FOREIGN KEY (`owner_id`) REFERENCES `members`(`id`) ON DELETE RESTRICT);
CREATE INDEX `idx_cards_updated_at` ON `cards`(`updated_at`);
@@ -28,10 +30,12 @@ CREATE TABLE `card_values` (`card_id` integer,`field_def_id` integer,`sealed_val
CREATE INDEX `idx_card_values_print` ON `card_values`(`print`);
-CREATE TABLE `entries` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_id` integer NOT NULL,`kind` text NOT NULL,`source` text,`actor_id` integer,`created_at` datetime,CONSTRAINT `fk_cards_entries` FOREIGN KEY (`card_id`) REFERENCES `cards`(`id`) ON DELETE CASCADE);
+CREATE TABLE `entries` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_id` integer NOT NULL,`kind` text NOT NULL,`source` text,`actor_id` integer,`type_id` integer NOT NULL,`created_at` datetime,CONSTRAINT `fk_entries_actor` FOREIGN KEY (`actor_id`) REFERENCES `members`(`id`) ON DELETE RESTRICT,CONSTRAINT `fk_cards_entries` FOREIGN KEY (`card_id`) REFERENCES `cards`(`id`) ON DELETE CASCADE);
CREATE INDEX `idx_entries_created_at` ON `entries`(`created_at`);
+CREATE INDEX `idx_entries_type_id` ON `entries`(`type_id`);
+
CREATE INDEX `idx_entries_actor_id` ON `entries`(`actor_id`);
CREATE INDEX `idx_entries_kind` ON `entries`(`kind`);
@@ -42,7 +46,7 @@ CREATE TABLE `entry_values` (`entry_id` integer,`field_def_id` integer,`sealed_v
CREATE INDEX `idx_entry_values_print` ON `entry_values`(`print`);
-CREATE TABLE `refs` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_id` integer NOT NULL,`entry_id` integer,`sealed_target` blob,`print` text,`created_at` datetime,CONSTRAINT `fk_entries_refs` FOREIGN KEY (`entry_id`) REFERENCES `entries`(`id`) ON DELETE CASCADE,CONSTRAINT `fk_cards_refs` FOREIGN KEY (`card_id`) REFERENCES `cards`(`id`) ON DELETE CASCADE);
+CREATE TABLE `refs` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_id` integer NOT NULL,`entry_id` integer,`sealed_target` blob,`print` text,`created_at` datetime,CONSTRAINT `fk_cards_refs` FOREIGN KEY (`card_id`) REFERENCES `cards`(`id`) ON DELETE CASCADE,CONSTRAINT `fk_entries_refs` FOREIGN KEY (`entry_id`) REFERENCES `entries`(`id`) ON DELETE CASCADE);
CREATE UNIQUE INDEX `idx_refs_card_print` ON `refs`(`print`);
@@ -50,7 +54,7 @@ CREATE INDEX `idx_refs_entry_id` ON `refs`(`entry_id`);
CREATE INDEX `idx_refs_card_id` ON `refs`(`card_id`);
-CREATE TABLE `events` (`id` integer PRIMARY KEY AUTOINCREMENT,`kind` text NOT NULL,`object_kind` text NOT NULL,`object_id` integer,`actor_id` integer,`revision` integer,`changed_fields` text,`payload_sealed` blob,`payload_erased_at` datetime,`trigger_id` integer,`created_at` datetime);
+CREATE TABLE `events` (`id` integer PRIMARY KEY AUTOINCREMENT,`kind` text NOT NULL,`object_kind` text NOT NULL,`object_id` integer,`actor_id` integer,`actor_reason` text,`child_id` integer,`revision` integer,`changed_fields` text,`payload_sealed` blob,`payload_erased_at` datetime,`trigger_id` integer,`created_at` datetime);
CREATE INDEX `idx_events_created_at` ON `events`(`created_at`);
diff --git a/internal/fichas/migrations/0002_immutable_entries.sql b/internal/fichas/migrations/0002_immutable_entries.sql
new file mode 100644
index 0000000..435cad7
--- /dev/null
+++ b/internal/fichas/migrations/0002_immutable_entries.sql
@@ -0,0 +1,43 @@
+-- Entries are immutable, and cards are deleted by being marked gone.
+--
+-- Both were true only by convention: there was no UpdateEntry, and
+-- DeleteCard set `gone` — but a stray `Save` on an entry_values row
+-- rewrote it, and a stray `Delete` on a card hard-deleted it AND cascaded
+-- its values, entries and refs away while the events survived pointing at
+-- nothing. "No function exists for it" is not an invariant.
+--
+-- Written by hand rather than generated: `rastrillo migration generate`
+-- diffs Models against migrations and knows nothing about triggers, so it
+-- would neither write these nor notice them missing.
+
+-- §3: an entry is a fact about a past moment. A correction is a new entry
+-- that supersedes, never an edit.
+CREATE TRIGGER entries_are_immutable
+BEFORE UPDATE ON entries
+BEGIN
+ SELECT RAISE(ABORT, 'entries are immutable: write a new entry that supersedes this one');
+END;
+
+CREATE TRIGGER entry_values_are_immutable
+BEFORE UPDATE ON entry_values
+BEGIN
+ SELECT RAISE(ABORT, 'entry values are immutable: write a new entry that supersedes this one');
+END;
+
+-- An entry may not be removed either: its event would outlive it and name
+-- a row that is not there.
+CREATE TRIGGER entries_are_not_deleted
+BEFORE DELETE ON entries
+WHEN (SELECT COUNT(*) FROM cards WHERE cards.id = OLD.card_id) > 0
+BEGIN
+ SELECT RAISE(ABORT, 'entries are not deleted: mark the card gone instead');
+END;
+
+-- §5 and suite ruling 14: retirement is by feed, never by erasure. A hard
+-- delete here would take the card's values, entries and refs with it
+-- through the cascades, silently and completely.
+CREATE TRIGGER cards_are_not_hard_deleted
+BEFORE DELETE ON cards
+BEGIN
+ SELECT RAISE(ABORT, 'cards are not deleted: set gone, which is what DeleteCard does');
+END;
diff --git a/internal/fichas/migrations/schema.sql b/internal/fichas/migrations/schema.sql
index a6efd7b..764b493 100644
--- a/internal/fichas/migrations/schema.sql
+++ b/internal/fichas/migrations/schema.sql
@@ -1,26 +1,28 @@
-- Generated by rastrillo migration generate; DO NOT EDIT.
-- The schema every migration in this directory adds up to.
-CREATE TABLE `card_types` (`id` integer PRIMARY KEY AUTOINCREMENT,`key` text,`sealed_label` blob,`created_at` datetime,`updated_at` datetime);
+CREATE TABLE `card_types` (`id` integer PRIMARY KEY AUTOINCREMENT,`key` text,`scope` text NOT NULL,`sealed_label` blob,`created_at` datetime,`updated_at` datetime);
CREATE TABLE `card_values` (`card_id` integer,`field_def_id` integer,`sealed_value` blob,`print` text,PRIMARY KEY (`card_id`,`field_def_id`),CONSTRAINT `fk_cards_values` FOREIGN KEY (`card_id`) REFERENCES `cards`(`id`) ON DELETE CASCADE);
-CREATE TABLE `cards` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_type_id` integer NOT NULL,`revision` integer NOT NULL DEFAULT 1,`gone` numeric NOT NULL DEFAULT false,`offered` numeric NOT NULL DEFAULT false,`owner_id` integer,`created_at` datetime,`updated_at` datetime);
+CREATE TABLE `cards` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_type_id` integer NOT NULL,`revision` integer NOT NULL DEFAULT 1,`gone` numeric NOT NULL DEFAULT false,`offered` numeric NOT NULL DEFAULT false,`owner_id` integer,`created_at` datetime,`updated_at` datetime,CONSTRAINT `fk_cards_owner` FOREIGN KEY (`owner_id`) REFERENCES `members`(`id`) ON DELETE RESTRICT);
-CREATE TABLE `entries` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_id` integer NOT NULL,`kind` text NOT NULL,`source` text,`actor_id` integer,`created_at` datetime,CONSTRAINT `fk_cards_entries` FOREIGN KEY (`card_id`) REFERENCES `cards`(`id`) ON DELETE CASCADE);
+CREATE TABLE `entries` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_id` integer NOT NULL,`kind` text NOT NULL,`source` text,`actor_id` integer,`type_id` integer NOT NULL,`created_at` datetime,CONSTRAINT `fk_entries_actor` FOREIGN KEY (`actor_id`) REFERENCES `members`(`id`) ON DELETE RESTRICT,CONSTRAINT `fk_cards_entries` FOREIGN KEY (`card_id`) REFERENCES `cards`(`id`) ON DELETE CASCADE);
CREATE TABLE `entry_values` (`entry_id` integer,`field_def_id` integer,`sealed_value` blob,`print` text,PRIMARY KEY (`entry_id`,`field_def_id`),CONSTRAINT `fk_entries_values` FOREIGN KEY (`entry_id`) REFERENCES `entries`(`id`) ON DELETE CASCADE);
-CREATE TABLE `events` (`id` integer PRIMARY KEY AUTOINCREMENT,`kind` text NOT NULL,`object_kind` text NOT NULL,`object_id` integer,`actor_id` integer,`revision` integer,`changed_fields` text,`payload_sealed` blob,`payload_erased_at` datetime,`trigger_id` integer,`created_at` datetime);
+CREATE TABLE `events` (`id` integer PRIMARY KEY AUTOINCREMENT,`kind` text NOT NULL,`object_kind` text NOT NULL,`object_id` integer,`actor_id` integer,`actor_reason` text,`child_id` integer,`revision` integer,`changed_fields` text,`payload_sealed` blob,`payload_erased_at` datetime,`trigger_id` integer,`created_at` datetime);
CREATE TABLE `field_defs` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_type_id` integer NOT NULL,`key` text,`sealed_label` blob,`sealed_options` blob,`kind` text,`tier` text,`ordinal` integer,`created_at` datetime,`updated_at` datetime,CONSTRAINT `fk_card_types_fields` FOREIGN KEY (`card_type_id`) REFERENCES `card_types`(`id`) ON DELETE RESTRICT);
CREATE TABLE `members` (`id` integer PRIMARY KEY AUTOINCREMENT,`address_print` text,`sealed_address` blob,`created_at` datetime,`updated_at` datetime);
-CREATE TABLE `refs` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_id` integer NOT NULL,`entry_id` integer,`sealed_target` blob,`print` text,`created_at` datetime,CONSTRAINT `fk_entries_refs` FOREIGN KEY (`entry_id`) REFERENCES `entries`(`id`) ON DELETE CASCADE,CONSTRAINT `fk_cards_refs` FOREIGN KEY (`card_id`) REFERENCES `cards`(`id`) ON DELETE CASCADE);
+CREATE TABLE `refs` (`id` integer PRIMARY KEY AUTOINCREMENT,`card_id` integer NOT NULL,`entry_id` integer,`sealed_target` blob,`print` text,`created_at` datetime,CONSTRAINT `fk_cards_refs` FOREIGN KEY (`card_id`) REFERENCES `cards`(`id`) ON DELETE CASCADE,CONSTRAINT `fk_entries_refs` FOREIGN KEY (`entry_id`) REFERENCES `entries`(`id`) ON DELETE CASCADE);
CREATE UNIQUE INDEX `idx_card_types_key` ON `card_types`(`key`);
+CREATE INDEX `idx_card_types_scope` ON `card_types`(`scope`);
+
CREATE INDEX `idx_card_values_print` ON `card_values`(`print`);
CREATE INDEX `idx_cards_card_type_id` ON `cards`(`card_type_id`);
@@ -41,6 +43,8 @@ CREATE INDEX `idx_entries_created_at` ON `entries`(`created_at`);
CREATE INDEX `idx_entries_kind` ON `entries`(`kind`);
+CREATE INDEX `idx_entries_type_id` ON `entries`(`type_id`);
+
CREATE INDEX `idx_entry_values_print` ON `entry_values`(`print`);
CREATE INDEX `idx_events_actor_id` ON `events`(`actor_id`);
diff --git a/internal/fichas/models.go b/internal/fichas/models.go
index c7be49a..75dcf32 100644
--- a/internal/fichas/models.go
+++ b/internal/fichas/models.go
@@ -36,6 +36,12 @@ var Models = []any{
// the suite contract's feed row, referential integrity, and the reach mark
// a sibling evaluates for itself. Everything else is sealed.
+// Scopes for CardType.Scope.
+const (
+ ScopeCard = "card"
+ ScopeEntry = "entry"
+)
+
// Member is a person this instance has seen. It exists so that "who owns
// this card" and "who wrote this entry" are FOREIGN KEYS rather than
// addresses repeated in the clear on every row.
@@ -93,6 +99,18 @@ type CardType struct {
// stable constants (f_customer), anything added later takes a
// generated one. It is plaintext because code selects on it.
Key string `gorm:"uniqueIndex;size:64"`
+ // Scope is whether this type describes a CARD or an ENTRY.
+ //
+ // It exists because AddEntry used to look up the Call Log type by a
+ // constant regardless of what it was asked for, which bound every
+ // entry kind to one type's field definitions — and CreateCard would
+ // just as happily make a *card* of type Call Log. Entries are
+ // immutable (§3), so every row written that way is a row that cannot
+ // be corrected; there is no backfill for an entry whose type was an
+ // accident of a constant.
+ //
+ // Plaintext under §4.1 case 4: this app's own vocabulary.
+ Scope string `gorm:"size:8;not null;index"`
// SealedLabel is what a person reads. Sealed even for built-ins, so
// that one rule covers the catalogue and whatever is added to it —
// a label that is sometimes plaintext is a rule nobody can apply.
@@ -151,7 +169,22 @@ type Card struct {
// so this must be answerable by SQL against the durable file.
Offered bool `gorm:"index;not null;default:false"`
// OwnerID is a Member row, never an address. See Member.
- OwnerID int64 `gorm:"index"`
+ //
+ // A POINTER, and the difference is load-bearing: as a bare int64 the
+ // zero value is indistinguishable from "nobody set this", so a
+ // handler that forgot to resolve the session produced a card owned by
+ // member 0 and nothing complained. NULL means the app made it; a
+ // number means a person did, and the foreign key means that person
+ // exists.
+ //
+ // Ownership here is PROVENANCE, not authority. fichas is one instance
+ // per team and the team shares its cards (§9), so OwnerID answers
+ // "who put this here", never "who may read it". If that ever changes,
+ // every store function needs the owner threaded through it and
+ // scope.OwnedBy on the lookup — and this comment is what says it was
+ // a decision rather than an omission.
+ OwnerID *int64 `gorm:"index"`
+ Owner *Member `gorm:"constraint:OnDelete:RESTRICT"`
CreatedAt time.Time
UpdatedAt time.Time `gorm:"index"`
@@ -187,10 +220,16 @@ type Entry struct {
// content.
Source string `gorm:"size:32"`
// ActorID names the real writer, per suite ruling 9 — a Member row,
- // never an address. Zero means the app wrote it as itself, which a
+ // never an address. NULL means the app wrote it as itself, which a
// generated entry does; it is never attributed to whoever triggered
- // it.
- ActorID int64 `gorm:"index"`
+ // it. See Actor in store.go: the app-as-itself case must be asked for
+ // explicitly and carries a reason, so that forgetting an argument
+ // cannot produce it.
+ ActorID *int64 `gorm:"index"`
+ Actor *Member `gorm:"constraint:OnDelete:RESTRICT"`
+ // TypeID is the entry's CardType, whose Scope is ScopeEntry. Kind is
+ // display and routing; this is what decides which FieldDefs apply.
+ TypeID int64 `gorm:"index;not null"`
// CreatedAt is when the row was written — the feed needs that.
// When the call actually happened is an EntryValue, sealed, because
// "when you called them" is content.
@@ -257,10 +296,25 @@ type Event struct {
Kind string `gorm:"index;size:32;not null"`
ObjectKind string `gorm:"size:16;not null;index:idx_events_object"`
ObjectID int64 `gorm:"index:idx_events_object"`
- // ActorID is a Member row, or zero for the app acting as itself.
+ // ActorID is a Member row, or NULL for the app acting as itself.
// Indexed because "who deleted this" is the one audit question the
// durable table has to answer without the key.
- ActorID int64 `gorm:"index"`
+ //
+ // No foreign key, deliberately, unlike Card.Owner and Entry.Actor:
+ // the log must outlive everything it describes, including a member
+ // row somebody removes.
+ ActorID *int64 `gorm:"index"`
+ // ActorReason is why the app acted, when ActorID is NULL —
+ // "summary regenerated", "intake form". Plaintext (§4.1 case 4) and
+ // required for an app actor, so that an honest machine write cannot
+ // be confused with a handler that forgot to resolve a session.
+ ActorReason string `gorm:"size:64"`
+ // ChildID names the object a card-scoped event is about when that is
+ // not the card itself — the entry that landed. Without it, an entry
+ // landing had to be logged under object_kind=entry while consuming a
+ // CARD revision, which left the card's own revision chain with gaps
+ // that a contiguity check reads as broken.
+ ChildID int64
// Revision is the object's revision AFTER this event. Restore needs
// it to check the diff chain is contiguous, and it is what lets the
// log answer "at what revision" for S1 rather than reading the
diff --git a/internal/fichas/models_test.go b/internal/fichas/models_test.go
index feba572..286c1ee 100644
--- a/internal/fichas/models_test.go
+++ b/internal/fichas/models_test.go
@@ -45,6 +45,35 @@ var plaintextColumns = map[string]string{
"Event.Kind": "routing: created, updated, deleted",
"Event.ObjectKind": "routing: card, entry",
"Event.ChangedFields": "JSON array of FieldDef ids — opaque, and §5 keeps it permanently",
+ "CardType.Scope": "card or entry — this app's vocabulary, and what stops an entry taking a card type",
+ "Event.ActorReason": "why the app acted as itself — our words, never a team's; required for an app actor",
+}
+
+// nonStringPlaintext is the same list for columns that are not strings.
+//
+// The first version of this gate walked reflect.String only, which meant
+// `Amount int64` or `Birthday time.Time` — a salary, a date of birth —
+// could be added to a model and pass. Numbers and dates are content as
+// readily as text is; the tier table seals next_action_date for exactly
+// that reason.
+var nonStringPlaintext = map[string]string{
+ "Member.ID": "row id", "Member.CreatedAt": "row timestamp", "Member.UpdatedAt": "row timestamp",
+ "CardType.ID": "row id", "CardType.CreatedAt": "row timestamp", "CardType.UpdatedAt": "row timestamp",
+ "FieldDef.ID": "row id", "FieldDef.CardTypeID": "foreign key", "FieldDef.Ordinal": "display order, ours",
+ "FieldDef.CreatedAt": "row timestamp", "FieldDef.UpdatedAt": "row timestamp",
+ "Card.ID": "row id", "Card.CardTypeID": "foreign key", "Card.OwnerID": "foreign key to members",
+ "Card.Revision": "S1 feed row", "Card.Gone": "S1 feed row", "Card.Offered": "the reach mark",
+ "Card.CreatedAt": "row timestamp", "Card.UpdatedAt": "S1 feed row (touched_at)",
+ "CardValue.CardID": "foreign key", "CardValue.FieldDefID": "foreign key",
+ "Entry.ID": "row id", "Entry.CardID": "foreign key", "Entry.TypeID": "foreign key",
+ "Entry.ActorID": "foreign key to members", "Entry.CreatedAt": "row timestamp",
+ "EntryValue.EntryID": "foreign key", "EntryValue.FieldDefID": "foreign key",
+ "Ref.ID": "row id", "Ref.CardID": "foreign key", "Ref.EntryID": "foreign key", "Ref.CreatedAt": "row timestamp",
+ "Event.ID": "row id", "Event.ObjectID": "foreign key by value; no constraint, the log outlives its object",
+ "Event.ChildID": "the entry a card-scoped event is about", "Event.ActorID": "member id, NULL for the app",
+ "Event.Revision": "the object's revision after this event", "Event.TriggerID": "the event that provoked this one",
+ "Event.PayloadErasedAt": "when the payload was erased; its absence is what §5 forbids being ambiguous",
+ "Event.CreatedAt": "row timestamp",
}
// FieldDef.Tier is plaintext for a reason worth stating because the design
@@ -64,14 +93,25 @@ func TestNoPlaintextColumnEscapesTheAllowlist(t *testing.T) {
st := reflect.TypeOf(m).Elem()
for i := 0; i < st.NumField(); i++ {
f := st.Field(i)
- if f.Type.Kind() != reflect.String {
+ ft := f.Type
+ for ft.Kind() == reflect.Pointer {
+ ft = ft.Elem()
+ }
+ // []byte is sealed by this app's convention, and the file
+ // gate proves it for what the app writes. Associations are
+ // not columns.
+ if ft.Kind() == reflect.Slice || (ft.Kind() == reflect.Struct && ft.String() != "time.Time") {
continue
}
name := st.Name() + "." + f.Name
- if _, ok := plaintextColumns[name]; !ok {
- t.Errorf("%s is a plaintext string column and is not on the allowlist.\n"+
- "If it must be plaintext, add it to plaintextColumns with the §4.1 case it falls under.\n"+
- "If it holds anything a person typed, seal it.", name)
+ list, kind := plaintextColumns, "string"
+ if ft.Kind() != reflect.String {
+ list, kind = nonStringPlaintext, ft.Kind().String()
+ }
+ if _, ok := list[name]; !ok {
+ t.Errorf("%s is a plaintext %s column and is not on the allowlist.\n"+
+ "If it must be plaintext, add it with the §4.1 case it falls under.\n"+
+ "If it holds anything a person typed — a number and a date are content too — seal it.", name, kind)
}
}
}
@@ -87,9 +127,11 @@ func TestAllowlistHasNoStaleEntries(t *testing.T) {
live[st.Name()+"."+st.Field(i).Name] = true
}
}
- for name := range plaintextColumns {
- if !live[name] {
- t.Errorf("plaintextColumns names %s, which no longer exists", name)
+ for _, list := range []map[string]string{plaintextColumns, nonStringPlaintext} {
+ for name := range list {
+ if !live[name] {
+ t.Errorf("the allowlist names %s, which no longer exists", name)
+ }
}
}
}
diff --git a/internal/fichas/store.go b/internal/fichas/store.go
index ace06ca..a8207c3 100644
--- a/internal/fichas/store.go
+++ b/internal/fichas/store.go
@@ -1,6 +1,7 @@
package fichas
import (
+ "context"
"encoding/json"
"fmt"
"sort"
@@ -24,11 +25,55 @@ const (
// Event kinds.
const (
- EventCreated = "created"
- EventUpdated = "updated"
- EventDeleted = "deleted"
+ EventCreated = "created"
+ EventUpdated = "updated"
+ EventDeleted = "deleted"
+ EventEntryAdded = "entry_added"
)
+// Actor is who is making a change, per suite ruling 9: "a write names its
+// real actor; no sibling ever acts as a person."
+//
+// A struct rather than an int64 because the app-as-itself case must be
+// ASKED FOR, never arrived at by default. As a bare int64 the zero value
+// meant both "member 0" and "fichas did this", so a handler that forgot to
+// resolve the session wrote rows indistinguishable from an honest machine
+// write — and TriggerID was zero everywhere too, so the honest machine
+// write was indistinguishable from the forgetful one in the other
+// direction as well.
+type Actor struct {
+ memberID *int64
+ trigger int64
+ reason string
+}
+
+// Person is a member of the team making a change through their own session.
+func Person(memberID int64) Actor { return Actor{memberID: &memberID} }
+
+// AppBecause is fichas acting as itself — a derived summary, an intake
+// form. It requires the event that provoked it and a reason, because §5's
+// honesty about a machine-written change is exactly those two things: the
+// summary names the entry that caused it, and is never attributed to
+// whoever triggered it.
+func AppBecause(triggerEventID int64, reason string) Actor {
+ return Actor{trigger: triggerEventID, reason: reason}
+}
+
+// valid refuses an actor that is neither a named person nor a declared app
+// action. logEvent calls it, so no write path can skip it.
+func (a Actor) valid() error {
+ if a.memberID != nil {
+ if *a.memberID == 0 {
+ return fmt.Errorf("fichas: member 0 is not a member; Person was called with a zero id")
+ }
+ return nil
+ }
+ if a.reason == "" {
+ return fmt.Errorf("fichas: an actor is neither a person nor a declared app action; use Person or AppBecause")
+ }
+ return nil
+}
+
// change is one field's before and after, as the sealed payload records it.
type change struct {
Field string `json:"field"`
@@ -36,6 +81,21 @@ type change struct {
After string `json:"after,omitempty"`
}
+// typeByKey loads a type and refuses it if its scope is not the one the
+// caller needs. This is what stops CreateCard making a card of type "Call
+// log", and AddEntry binding every entry to one type's field definitions
+// regardless of what it was asked for.
+func typeByKey(tx *gorm.DB, key, scope string) (CardType, error) {
+ var ct CardType
+ if err := tx.Where("key = ?", key).First(&ct).Error; err != nil {
+ return ct, fmt.Errorf("fichas: no type %s: %w", key, err)
+ }
+ if ct.Scope != scope {
+ return ct, fmt.Errorf("fichas: %s is a %s type, not a %s type", key, ct.Scope, scope)
+ }
+ return ct, nil
+}
+
// fieldsFor loads a card type's definitions, keyed by field key.
func fieldsFor(tx *gorm.DB, cardTypeID int64) (map[string]FieldDef, error) {
var defs []FieldDef
@@ -56,8 +116,14 @@ func fieldsFor(tx *gorm.DB, cardTypeID int64) (map[string]FieldDef, error) {
// is what a bounded window later erases. That split is §5's whole retention
// rule: after erasure a reader can still see THAT a field changed and who
// changed it, which is what an audit log is for.
-func logEvent(tx *gorm.DB, s *seal.Sealer, kind, objectKind string, objectID, actorID, revision, triggerID int64, changed []int64, changes []change) error {
+func logEvent(tx *gorm.DB, s *seal.Sealer, kind, objectKind string, objectID, childID, revision int64, actor Actor, changed []int64, changes []change) error {
+ if err := actor.valid(); err != nil {
+ return err
+ }
sort.Slice(changed, func(i, j int) bool { return changed[i] < changed[j] })
+ if changed == nil {
+ changed = []int64{} // "[]", never the string "null"
+ }
ids, err := json.Marshal(changed)
if err != nil {
return fmt.Errorf("fichas: encoding changed fields: %w", err)
@@ -78,11 +144,13 @@ func logEvent(tx *gorm.DB, s *seal.Sealer, kind, objectKind string, objectID, ac
Kind: kind,
ObjectKind: objectKind,
ObjectID: objectID,
- ActorID: actorID,
+ ChildID: childID,
+ ActorID: actor.memberID,
+ ActorReason: actor.reason,
Revision: revision,
ChangedFields: string(ids),
PayloadSealed: payload,
- TriggerID: triggerID,
+ TriggerID: actor.trigger,
}
if err := tx.Create(&e).Error; err != nil {
return fmt.Errorf("fichas: writing the event: %w", err)
@@ -152,20 +220,46 @@ func writeValues(tx *gorm.DB, s *seal.Sealer, cardID int64, defs map[string]Fiel
return changed, detail, nil
}
+// bumpRevision moves a card's revision, guarded on the revision it read.
+//
+// The guard makes the invariant LOCAL. Concurrent updates are safe today
+// only because rastrillo's writer pool is a single connection; a blind
+// UPDATE would silently start losing revisions the day that changes, and
+// the log would stop being a truthful account of what happened. Costs one
+// clause.
+func bumpRevision(tx *gorm.DB, card *Card, extra map[string]any) error {
+ next := card.Revision + 1
+ set := map[string]any{"revision": next, "updated_at": time.Now()}
+ for k, v := range extra {
+ set[k] = v
+ }
+ res := tx.Model(&Card{}).
+ Where("id = ? AND revision = ?", card.ID, card.Revision).
+ Updates(set)
+ if res.Error != nil {
+ return fmt.Errorf("fichas: bumping the revision: %w", res.Error)
+ }
+ if res.RowsAffected != 1 {
+ return fmt.Errorf("fichas: card %d moved under this write (expected revision %d)", card.ID, card.Revision)
+ }
+ card.Revision = next
+ return nil
+}
+
// CreateCard writes a card, its values and its creation event.
-func CreateCard(db *gorm.DB, s *seal.Sealer, typeKey string, actorID int64, values map[string]string) (Card, error) {
+func CreateCard(ctx context.Context, db *gorm.DB, s *seal.Sealer, typeKey string, actor Actor, values map[string]string) (Card, error) {
var card Card
- err := db.Transaction(func(tx *gorm.DB) error {
- var ct CardType
- if err := tx.Where("key = ?", typeKey).First(&ct).Error; err != nil {
- return fmt.Errorf("fichas: no card type %s: %w", typeKey, err)
+ err := db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
+ ct, err := typeByKey(tx, typeKey, ScopeCard)
+ if err != nil {
+ return err
}
defs, err := fieldsFor(tx, ct.ID)
if err != nil {
return err
}
- card = Card{CardTypeID: ct.ID, OwnerID: actorID, Revision: 1}
+ card = Card{CardTypeID: ct.ID, OwnerID: actor.memberID, Revision: 1}
if err := tx.Create(&card).Error; err != nil {
return fmt.Errorf("fichas: creating the card: %w", err)
}
@@ -173,9 +267,14 @@ func CreateCard(db *gorm.DB, s *seal.Sealer, typeKey string, actorID int64, valu
if err != nil {
return err
}
- return logEvent(tx, s, EventCreated, ObjectCard, card.ID, actorID, card.Revision, 0, changed, detail)
+ return logEvent(tx, s, EventCreated, ObjectCard, card.ID, 0, card.Revision, actor, changed, detail)
})
- return card, err
+ if err != nil {
+ // The transaction rolled back, so the id this carries names no
+ // row. Returning it invites a caller to use it.
+ return Card{}, err
+ }
+ return card, nil
}
// UpdateCard changes values on a card and logs exactly what moved.
@@ -183,8 +282,8 @@ func CreateCard(db *gorm.DB, s *seal.Sealer, typeKey string, actorID int64, valu
// A write that changes nothing writes no event. That is deliberate: a log
// full of no-op rows is a log nobody reads, and "who touched this" stops
// meaning anything if opening a form and pressing save counts.
-func UpdateCard(db *gorm.DB, s *seal.Sealer, cardID, actorID int64, values map[string]string) error {
- return db.Transaction(func(tx *gorm.DB) error {
+func UpdateCard(ctx context.Context, db *gorm.DB, s *seal.Sealer, cardID int64, actor Actor, values map[string]string) error {
+ return db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
var card Card
if err := tx.First(&card, cardID).Error; err != nil {
return fmt.Errorf("fichas: no card %d: %w", cardID, err)
@@ -203,22 +302,18 @@ func UpdateCard(db *gorm.DB, s *seal.Sealer, cardID, actorID int64, values map[s
if len(changed) == 0 {
return nil
}
- card.Revision++
- if err := tx.Model(&card).Updates(map[string]any{
- "revision": card.Revision,
- "updated_at": time.Now(),
- }).Error; err != nil {
- return fmt.Errorf("fichas: bumping the revision: %w", err)
+ if err := bumpRevision(tx, &card, nil); err != nil {
+ return err
}
- return logEvent(tx, s, EventUpdated, ObjectCard, card.ID, actorID, card.Revision, 0, changed, detail)
+ return logEvent(tx, s, EventUpdated, ObjectCard, card.ID, 0, card.Revision, actor, changed, detail)
})
}
// DeleteCard marks a card gone. The row stays and so does its log: §5 keeps
// the actor permanently precisely so "who deleted this" has somewhere to
// live, and suite ruling 14 retires by feed rather than by erasure.
-func DeleteCard(db *gorm.DB, s *seal.Sealer, cardID, actorID int64) error {
- return db.Transaction(func(tx *gorm.DB) error {
+func DeleteCard(ctx context.Context, db *gorm.DB, s *seal.Sealer, cardID int64, actor Actor) error {
+ return db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
var card Card
if err := tx.First(&card, cardID).Error; err != nil {
return fmt.Errorf("fichas: no card %d: %w", cardID, err)
@@ -226,15 +321,10 @@ func DeleteCard(db *gorm.DB, s *seal.Sealer, cardID, actorID int64) error {
if card.Gone {
return nil
}
- card.Revision++
- if err := tx.Model(&card).Updates(map[string]any{
- "gone": true,
- "revision": card.Revision,
- "updated_at": time.Now(),
- }).Error; err != nil {
- return fmt.Errorf("fichas: marking the card gone: %w", err)
+ if err := bumpRevision(tx, &card, map[string]any{"gone": true}); err != nil {
+ return err
}
- return logEvent(tx, s, EventDeleted, ObjectCard, card.ID, actorID, card.Revision, 0, nil, nil)
+ return logEvent(tx, s, EventDeleted, ObjectCard, card.ID, 0, card.Revision, actor, nil, nil)
})
}
@@ -244,9 +334,9 @@ func DeleteCard(db *gorm.DB, s *seal.Sealer, cardID, actorID int64) error {
// correction is a new entry that supersedes. The card's revision moves
// because the feed's consumers care that the card changed, and an entry
// landing IS the card changing.
-func AddEntry(db *gorm.DB, s *seal.Sealer, cardID, actorID int64, kind, source string, values map[string]string) (Entry, error) {
+func AddEntry(ctx context.Context, db *gorm.DB, s *seal.Sealer, cardID int64, actor Actor, typeKey, source string, values map[string]string) (Entry, error) {
var entry Entry
- err := db.Transaction(func(tx *gorm.DB) error {
+ err := db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
var card Card
if err := tx.First(&card, cardID).Error; err != nil {
return fmt.Errorf("fichas: no card %d: %w", cardID, err)
@@ -255,23 +345,29 @@ func AddEntry(db *gorm.DB, s *seal.Sealer, cardID, actorID int64, kind, source s
return fmt.Errorf("fichas: card %d is deleted", cardID)
}
- var ct CardType
- if err := tx.Where("key = ?", TypeCallLog).First(&ct).Error; err != nil {
- return fmt.Errorf("fichas: no call log type: %w", err)
+ ct, err := typeByKey(tx, typeKey, ScopeEntry)
+ if err != nil {
+ return err
}
defs, err := fieldsFor(tx, ct.ID)
if err != nil {
return err
}
- entry = Entry{CardID: cardID, Kind: kind, Source: source, ActorID: actorID}
+ entry = Entry{CardID: cardID, TypeID: ct.ID, Kind: typeKey, Source: source, ActorID: actor.memberID}
if err := tx.Create(&entry).Error; err != nil {
return fmt.Errorf("fichas: creating the entry: %w", err)
}
var changed []int64
var detail []change
- for key, v := range values {
+ keys := make([]string, 0, len(values))
+ for k := range values {
+ keys = append(keys, k)
+ }
+ sort.Strings(keys) // deterministic payload order, as writeValues has
+ for _, key := range keys {
+ v := values[key]
def, ok := defs[key]
if !ok {
return fmt.Errorf("fichas: %s is not a field of an entry", key)
@@ -295,14 +391,23 @@ func AddEntry(db *gorm.DB, s *seal.Sealer, cardID, actorID int64, kind, source s
detail = append(detail, change{Field: key, After: v})
}
- card.Revision++
- if err := tx.Model(&card).Updates(map[string]any{
- "revision": card.Revision,
- "updated_at": time.Now(),
- }).Error; err != nil {
- return fmt.Errorf("fichas: bumping the revision: %w", err)
- }
- return logEvent(tx, s, EventCreated, ObjectEntry, entry.ID, actorID, card.Revision, 0, changed, detail)
+ if err := bumpRevision(tx, &card, nil); err != nil {
+ return err
+ }
+ // TWO events, and the split is finding 9's. The entry's own
+ // creation carries no revision: a revision belongs to the card,
+ // and logging the entry under object_kind=entry while consuming a
+ // card revision left the CARD's chain with gaps that a
+ // contiguity check reads as broken. The card's event names the
+ // entry through ChildID, so "what happened at revision 2" is
+ // answerable from the card's own chain.
+ if err := logEvent(tx, s, EventCreated, ObjectEntry, entry.ID, 0, 0, actor, changed, detail); err != nil {
+ return err
+ }
+ return logEvent(tx, s, EventEntryAdded, ObjectCard, card.ID, entry.ID, card.Revision, actor, nil, nil)
})
- return entry, err
+ if err != nil {
+ return Entry{}, err
+ }
+ return entry, nil
}
diff --git a/internal/fichas/store_test.go b/internal/fichas/store_test.go
index 68ba4f6..992f7d7 100644
--- a/internal/fichas/store_test.go
+++ b/internal/fichas/store_test.go
@@ -1,19 +1,34 @@
package fichas
import (
+ "context"
"encoding/json"
+ "fmt"
"testing"
"amadan.net/oficina/fichas/internal/fichas/seal"
"gorm.io/gorm"
)
+func ctx() context.Context { return context.Background() }
+
func seeded(t *testing.T) (*gorm.DB, *seal.Sealer) {
t.Helper()
g, s := testDB(t), testSealer(t)
if err := Seed(g, s); err != nil {
t.Fatalf("Seed: %v", err)
}
+ // Members are a real foreign key now, so the people these tests name
+ // have to exist. Ids 1..10 cover every actor used below.
+ for i := 1; i <= 10; i++ {
+ pr, err := s.Print("f_member", []byte(fmt.Sprintf("person-%d@example.com", i)))
+ if err != nil {
+ t.Fatalf("print: %v", err)
+ }
+ if err := g.Create(&Member{AddressPrint: pr}).Error; err != nil {
+ t.Fatalf("creating member %d: %v", i, err)
+ }
+ }
return g, s
}
@@ -28,7 +43,7 @@ func events(t *testing.T, g *gorm.DB, kind string, id int64) []Event {
func TestCreateCardSealsAndLogs(t *testing.T) {
g, s := seeded(t)
- card, err := CreateCard(g, s, TypeCustomer, 7, map[string]string{
+ card, err := CreateCard(ctx(), g, s, TypeCustomer, Person(7), map[string]string{
FieldName: "Cnoc Teoranta",
FieldEmail: "ana@example.com",
FieldNotes: "spoke at the trade show",
@@ -36,7 +51,7 @@ func TestCreateCardSealsAndLogs(t *testing.T) {
if err != nil {
t.Fatalf("CreateCard: %v", err)
}
- if card.ID == 0 || card.Revision != 1 || card.OwnerID != 7 {
+ if card.ID == 0 || card.Revision != 1 || card.OwnerID == nil || *card.OwnerID != 7 {
t.Fatalf("card is %+v", card)
}
@@ -59,7 +74,7 @@ func TestCreateCardSealsAndLogs(t *testing.T) {
}
ev := events(t, g, ObjectCard, card.ID)
- if len(ev) != 1 || ev[0].Kind != EventCreated || ev[0].ActorID != 7 || ev[0].Revision != 1 {
+ if len(ev) != 1 || ev[0].Kind != EventCreated || ev[0].ActorID == nil || *ev[0].ActorID != 7 || ev[0].Revision != 1 {
t.Fatalf("events are %+v", ev)
}
var changed []int64
@@ -75,7 +90,7 @@ func TestCreateCardSealsAndLogs(t *testing.T) {
// find the row again — that is the entire point of it.
func TestCreateCardPrintsOnlyPrintedFields(t *testing.T) {
g, s := seeded(t)
- card, err := CreateCard(g, s, TypeCustomer, 1, map[string]string{
+ card, err := CreateCard(ctx(), g, s, TypeCustomer, Person(1), map[string]string{
FieldEmail: "Ana@Example.com", // capitalised on purpose
FieldNotes: "not a printed field",
})
@@ -110,9 +125,9 @@ func TestCreateCardPrintsOnlyPrintedFields(t *testing.T) {
func TestUpdateCardLogsBeforeAndAfter(t *testing.T) {
g, s := seeded(t)
- card, _ := CreateCard(g, s, TypeCustomer, 1, map[string]string{FieldStage: "Lead"})
+ card, _ := CreateCard(ctx(), g, s, TypeCustomer, Person(1), map[string]string{FieldStage: "Lead"})
- if err := UpdateCard(g, s, card.ID, 2, map[string]string{FieldStage: "Proposal"}); err != nil {
+ if err := UpdateCard(ctx(), g, s, card.ID, Person(2), map[string]string{FieldStage: "Proposal"}); err != nil {
t.Fatalf("UpdateCard: %v", err)
}
@@ -121,7 +136,7 @@ func TestUpdateCardLogsBeforeAndAfter(t *testing.T) {
t.Fatalf("%d events after one update, want 2", len(ev))
}
last := ev[1]
- if last.Kind != EventUpdated || last.ActorID != 2 || last.Revision != 2 {
+ if last.Kind != EventUpdated || last.ActorID == nil || *last.ActorID != 2 || last.Revision != 2 {
t.Fatalf("update event is %+v", last)
}
raw, err := s.Open(seal.PurposeEventPayload, last.PayloadSealed)
@@ -141,9 +156,9 @@ func TestUpdateCardLogsBeforeAndAfter(t *testing.T) {
// stops meaning anything if pressing save counts.
func TestUpdateCardWithNoChangeWritesNoEvent(t *testing.T) {
g, s := seeded(t)
- card, _ := CreateCard(g, s, TypeCustomer, 1, map[string]string{FieldStage: "Lead"})
+ card, _ := CreateCard(ctx(), g, s, TypeCustomer, Person(1), map[string]string{FieldStage: "Lead"})
- if err := UpdateCard(g, s, card.ID, 1, map[string]string{FieldStage: "Lead"}); err != nil {
+ if err := UpdateCard(ctx(), g, s, card.ID, Person(1), map[string]string{FieldStage: "Lead"}); err != nil {
t.Fatalf("UpdateCard: %v", err)
}
if n := len(events(t, g, ObjectCard, card.ID)); n != 1 {
@@ -160,16 +175,16 @@ func TestUpdateCardWithNoChangeWritesNoEvent(t *testing.T) {
// silently ignored field is a value a person believes they saved.
func TestWritingAnUnknownFieldFails(t *testing.T) {
g, s := seeded(t)
- if _, err := CreateCard(g, s, TypeCustomer, 1, map[string]string{"f_nonsense": "x"}); err == nil {
+ if _, err := CreateCard(ctx(), g, s, TypeCustomer, Person(1), map[string]string{"f_nonsense": "x"}); err == nil {
t.Fatal("an unknown field key was accepted")
}
}
func TestDeleteCardKeepsTheRowAndTheActor(t *testing.T) {
g, s := seeded(t)
- card, _ := CreateCard(g, s, TypeCustomer, 1, map[string]string{FieldName: "Cnoc Teoranta"})
+ card, _ := CreateCard(ctx(), g, s, TypeCustomer, Person(1), map[string]string{FieldName: "Cnoc Teoranta"})
- if err := DeleteCard(g, s, card.ID, 9); err != nil {
+ if err := DeleteCard(ctx(), g, s, card.ID, Person(9)); err != nil {
t.Fatalf("DeleteCard: %v", err)
}
var reread Card
@@ -181,23 +196,23 @@ func TestDeleteCardKeepsTheRowAndTheActor(t *testing.T) {
}
ev := events(t, g, ObjectCard, card.ID)
last := ev[len(ev)-1]
- if last.Kind != EventDeleted || last.ActorID != 9 {
+ if last.Kind != EventDeleted || last.ActorID == nil || *last.ActorID != 9 {
t.Fatalf("who deleted this is unanswerable: %+v", last)
}
}
func TestAddEntryLogsAndMovesTheCardRevision(t *testing.T) {
g, s := seeded(t)
- card, _ := CreateCard(g, s, TypeCustomer, 1, map[string]string{FieldName: "Cnoc Teoranta"})
+ card, _ := CreateCard(ctx(), g, s, TypeCustomer, Person(1), map[string]string{FieldName: "Cnoc Teoranta"})
- entry, err := AddEntry(g, s, card.ID, 3, "call", "manual", map[string]string{
+ entry, err := AddEntry(ctx(), g, s, card.ID, Person(3), TypeCallLog, "manual", map[string]string{
FieldBody: "talked about renewal",
FieldOccurredAt: "2026-09-20",
})
if err != nil {
t.Fatalf("AddEntry: %v", err)
}
- if entry.ID == 0 || entry.ActorID != 3 {
+ if entry.ID == 0 || entry.ActorID == nil || *entry.ActorID != 3 {
t.Fatalf("entry is %+v", entry)
}
@@ -219,7 +234,7 @@ func TestAChangeThatCannotBeLoggedDoesNotLand(t *testing.T) {
if err := g.Exec("DROP TABLE events").Error; err != nil {
t.Fatalf("dropping events: %v", err)
}
- if _, err := CreateCard(g, s, TypeCustomer, 1, map[string]string{FieldName: "Cnoc Teoranta"}); err == nil {
+ if _, err := CreateCard(ctx(), g, s, TypeCustomer, Person(1), map[string]string{FieldName: "Cnoc Teoranta"}); err == nil {
t.Fatal("a card was created while its event could not be written")
}
var cards int64
@@ -228,3 +243,122 @@ func TestAChangeThatCannotBeLoggedDoesNotLand(t *testing.T) {
t.Fatalf("%d card(s) survived a failed event write", cards)
}
}
+
+// A type is a card's or an entry's, never both. Before this, AddEntry
+// looked up the Call Log type by a constant regardless of what it was
+// asked for, and CreateCard would make a *card* of type Call Log — and
+// entries are immutable, so every row written that way is a row that
+// cannot be corrected.
+func TestATypeIsACardsOrAnEntrys(t *testing.T) {
+ g, s := seeded(t)
+ if _, err := CreateCard(ctx(), g, s, TypeCallLog, Person(1), nil); err == nil {
+ t.Error("a card was created with an entry type")
+ }
+ card, err := CreateCard(ctx(), g, s, TypeCustomer, Person(1), nil)
+ if err != nil {
+ t.Fatalf("CreateCard: %v", err)
+ }
+ if _, err := AddEntry(ctx(), g, s, card.ID, Person(1), TypeCustomer, "manual", nil); err == nil {
+ t.Error("an entry was added with a card type")
+ }
+}
+
+// The app-as-itself actor must be asked for, never arrived at by default.
+func TestAnActorIsAPersonOrADeclaredAppAction(t *testing.T) {
+ g, s := seeded(t)
+ if _, err := CreateCard(ctx(), g, s, TypeCustomer, Actor{}, nil); err == nil {
+ t.Error("a zero Actor was accepted; a handler that forgot the session would write as the app")
+ }
+ if _, err := CreateCard(ctx(), g, s, TypeCustomer, Person(0), nil); err == nil {
+ t.Error("member 0 was accepted as a person")
+ }
+
+ card, err := CreateCard(ctx(), g, s, TypeCustomer, AppBecause(0, "intake form"), nil)
+ if err != nil {
+ t.Fatalf("an app action with a reason was refused: %v", err)
+ }
+ ev := events(t, g, ObjectCard, card.ID)
+ if ev[0].ActorID != nil {
+ t.Error("an app action named a member")
+ }
+ if ev[0].ActorReason != "intake form" {
+ t.Errorf("the app's reason is %q", ev[0].ActorReason)
+ }
+}
+
+// Finding 9: an entry landing used to consume a CARD revision while being
+// logged under object_kind=entry, which left the card's own chain with
+// gaps that a contiguity check reads as broken.
+func TestTheCardsRevisionChainIsContiguous(t *testing.T) {
+ g, s := seeded(t)
+ card, _ := CreateCard(ctx(), g, s, TypeCustomer, Person(1), map[string]string{FieldName: "Cnoc Teoranta"})
+ if _, err := AddEntry(ctx(), g, s, card.ID, Person(1), TypeCallLog, "manual", map[string]string{FieldBody: "called"}); err != nil {
+ t.Fatalf("AddEntry: %v", err)
+ }
+ if err := UpdateCard(ctx(), g, s, card.ID, Person(1), map[string]string{FieldStage: "Proposal"}); err != nil {
+ t.Fatalf("UpdateCard: %v", err)
+ }
+
+ ev := events(t, g, ObjectCard, card.ID)
+ for i, e := range ev {
+ if e.Revision != int64(i+1) {
+ t.Fatalf("card event %d is at revision %d; the chain has a gap", i, e.Revision)
+ }
+ }
+ if ev[1].Kind != EventEntryAdded || ev[1].ChildID == 0 {
+ t.Fatalf("the entry landing is not named on the card's chain: %+v", ev[1])
+ }
+}
+
+// "No UpdateEntry exists" was a convention, not an invariant.
+func TestEntriesCannotBeRewrittenOrRemoved(t *testing.T) {
+ g, s := seeded(t)
+ card, _ := CreateCard(ctx(), g, s, TypeCustomer, Person(1), nil)
+ entry, err := AddEntry(ctx(), g, s, card.ID, Person(1), TypeCallLog, "manual", map[string]string{FieldBody: "as said"})
+ if err != nil {
+ t.Fatalf("AddEntry: %v", err)
+ }
+
+ if err := g.Model(&Entry{}).Where("id = ?", entry.ID).Update("source", "rewritten").Error; err == nil {
+ t.Error("an entry was rewritten")
+ }
+ if err := g.Exec("UPDATE entry_values SET print = 'x' WHERE entry_id = ?", entry.ID).Error; err == nil {
+ t.Error("an entry value was rewritten")
+ }
+ if err := g.Exec("DELETE FROM entries WHERE id = ?", entry.ID).Error; err == nil {
+ t.Error("an entry was deleted")
+ }
+}
+
+// A hard delete would cascade a card's values, entries and refs away while
+// its events survived pointing at nothing.
+func TestACardCannotBeHardDeleted(t *testing.T) {
+ g, s := seeded(t)
+ card, _ := CreateCard(ctx(), g, s, TypeCustomer, Person(1), map[string]string{FieldName: "Cnoc Teoranta"})
+ if err := g.Exec("DELETE FROM cards WHERE id = ?", card.ID).Error; err == nil {
+ t.Fatal("a card was hard-deleted; its cascades would have taken everything with it")
+ }
+ if err := DeleteCard(ctx(), g, s, card.ID, Person(1)); err != nil {
+ t.Fatalf("DeleteCard: %v", err)
+ }
+}
+
+// The partial-failure shape: a write that fails on the third key must
+// leave nothing behind from the first two.
+func TestAPartialWriteLeavesNothing(t *testing.T) {
+ g, s := seeded(t)
+ _, err := CreateCard(ctx(), g, s, TypeCustomer, Person(1), map[string]string{
+ FieldName: "Cnoc Teoranta",
+ FieldEmail: "ana@example.com",
+ "f_nonsense": "boom",
+ })
+ if err == nil {
+ t.Fatal("a card with an unknown field was created")
+ }
+ var cards, values int64
+ g.Model(&Card{}).Count(&cards)
+ g.Model(&CardValue{}).Count(&values)
+ if cards != 0 || values != 0 {
+ t.Fatalf("a failed write left %d card(s) and %d value(s)", cards, values)
+ }
+}
diff --git a/internal/fichastest/gates_test.go b/internal/fichastest/gates_test.go
index 39ec391..292959c 100644
--- a/internal/fichastest/gates_test.go
+++ b/internal/fichastest/gates_test.go
@@ -1,6 +1,7 @@
package fichastest
import (
+ "context"
"io"
"log/slog"
"os"
@@ -12,15 +13,70 @@ import (
"github.com/carlosframework/rastrillo/db"
)
+// canaries are plaintexts the app seals. None may appear in any file it
+// writes.
+//
+// The first version of this list had seven entries and excluded "Customer"
+// with a comment claiming the opaque key `t_customer` "legitimately
+// contains it". **That was simply false** — strings.Contains is
+// case-sensitive and `t_customer` does not contain `Customer` — so the
+// exclusion weakened the gate for no reason and taught the next reader that
+// weakening it was normal. It is back.
+var canaries = []string{
+ // Card type and field labels, sealed by Seed.
+ "Customer", "Call log", "Name", "Email", "Phone", "Stage",
+ "Next action", "Notes", "When", "What was said", "Duration",
+ // Sealed enum options.
+ "Lead", "Contacted", "Proposal", "Won", "Lost",
+ // Values written through the store below.
+ "Cnoc Teoranta", "ana@example.com", "renewal is due in March",
+ "talked about the renewal",
+ // The change payload's own JSON keys, which are sealed with it.
+ `"before"`, `"after"`,
+}
+
+// scan reads every file in dir and returns the (file, canary) pairs found.
+// Shared by the gate and by the test that proves the gate can fail.
+func scan(t *testing.T, dir string, canaries []string) []string {
+ t.Helper()
+ entries, err := os.ReadDir(dir)
+ if err != nil {
+ t.Fatalf("reading %s: %v", dir, err)
+ }
+ var hits []string
+ read := 0
+ for _, e := range entries {
+ if e.IsDir() {
+ continue
+ }
+ raw, err := os.ReadFile(filepath.Join(dir, e.Name()))
+ if err != nil {
+ t.Fatalf("reading %s: %v", e.Name(), err)
+ }
+ read++
+ for _, c := range canaries {
+ if strings.Contains(string(raw), c) {
+ hits = append(hits, e.Name()+": "+c)
+ }
+ }
+ }
+ if read == 0 {
+ t.Fatal("no files were read: this scan would pass vacuously")
+ }
+ return hits
+}
+
// TestPlaintextNeverReachesTheDatabase is the gate this app exists to pass.
-// memoria has one of the same shape (TestPlaintextNeverReachesTheDatabase),
-// Sheets has the schema-side twin, and fichas had neither until now.
+// memoria has one of the same shape; Sheets has the schema-side twin.
+//
+// It boots the real app, **writes through the real store path**, then reads
+// every byte on disk — database AND write-ahead log, because a value can
+// sit in the WAL for a long time before it is checkpointed, and a gate that
+// greps only the .db passes while the plaintext is on disk beside it.
//
-// It boots the real app over a real file, which seeds the catalogue, then
-// reads every byte the app wrote — the database AND its write-ahead log,
-// because a value can sit in the WAL for a long time before it is
-// checkpointed, and a gate that greps only the .db is a gate that passes
-// while the plaintext is on disk next to it.
+// The earlier version only booted and seeded, so it never saw a single
+// value a person typed — in the app whose entire premise is what happens to
+// those.
func TestPlaintextNeverReachesTheDatabase(t *testing.T) {
dir := t.TempDir()
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
@@ -32,69 +88,56 @@ func TestPlaintextNeverReachesTheDatabase(t *testing.T) {
if _, err := fichas.App(d, testOrigin, testSealer(t), logger); err != nil {
t.Fatalf("App: %v", err)
}
- // Close so the WAL is flushed and every byte is on disk to be read.
- if err := d.Close(); err != nil {
- t.Fatalf("Close: %v", err)
- }
- // Labels the seeded catalogue writes. Each is sealed, so none may
- // appear. "Customer" is deliberately NOT in this list: the card type's
- // opaque key is `t_customer`, which legitimately contains it, and a
- // canary that the schema is allowed to match is a canary that teaches
- // the next person to weaken the test.
- canaries := []string{
- "Call log",
- "Next action",
- "What was said",
- "Duration",
- "Lead",
- "Contacted",
- "Proposal",
+ s := testSealer(t)
+ pr, err := s.Print("f_member", []byte("ana@example.com"))
+ if err != nil {
+ t.Fatalf("print: %v", err)
+ }
+ if err := d.G.Create(&fichas.Member{AddressPrint: pr}).Error; err != nil {
+ t.Fatalf("creating a member: %v", err)
}
- entries, err := os.ReadDir(dir)
+ card, err := fichas.CreateCard(context.Background(), d.G, s, fichas.TypeCustomer, fichas.Person(1),
+ map[string]string{
+ fichas.FieldName: "Cnoc Teoranta",
+ fichas.FieldEmail: "ana@example.com",
+ fichas.FieldStage: "Lead",
+ })
if err != nil {
- t.Fatalf("reading %s: %v", dir, err)
+ t.Fatalf("CreateCard: %v", err)
}
- if len(entries) == 0 {
- t.Fatal("the app wrote no files: this gate would pass vacuously")
+ if err := fichas.UpdateCard(context.Background(), d.G, s, card.ID, fichas.Person(1),
+ map[string]string{fichas.FieldNotes: "renewal is due in March"}); err != nil {
+ t.Fatalf("UpdateCard: %v", err)
+ }
+ if _, err := fichas.AddEntry(context.Background(), d.G, s, card.ID, fichas.Person(1),
+ fichas.TypeCallLog, "manual",
+ map[string]string{fichas.FieldBody: "talked about the renewal"}); err != nil {
+ t.Fatalf("AddEntry: %v", err)
}
- checked := 0
- for _, e := range entries {
- if e.IsDir() {
- continue
- }
- raw, err := os.ReadFile(filepath.Join(dir, e.Name()))
- if err != nil {
- t.Fatalf("reading %s: %v", e.Name(), err)
- }
- checked++
- for _, c := range canaries {
- if strings.Contains(string(raw), c) {
- t.Errorf("%s contains the plaintext %q", e.Name(), c)
- }
- }
+ // Close so the WAL is checkpointed and every byte is on disk to read.
+ if err := d.Close(); err != nil {
+ t.Fatalf("Close: %v", err)
}
- if checked == 0 {
- t.Fatal("no files were read: this gate would pass vacuously")
+
+ for _, hit := range scan(t, dir, canaries) {
+ t.Errorf("plaintext on disk — %s", hit)
}
- t.Logf("checked %d file(s) for %d canaries", checked, len(canaries))
}
-// The gate above is only worth anything if it can fail. This proves the
-// grep actually looks: the same canary, written in the clear to the same
-// directory, must be found.
+// The gate is only worth something if it can fail, and it must be THIS
+// gate that fails, not a hand-rolled grep standing in for it. So: plant
+// every canary in a file in a directory and assert scan finds them all.
func TestTheGateCanFail(t *testing.T) {
dir := t.TempDir()
- if err := os.WriteFile(filepath.Join(dir, "leak.db"), []byte("x Next action x"), 0o600); err != nil {
+ planted := "header " + strings.Join(canaries, " ") + " footer"
+ if err := os.WriteFile(filepath.Join(dir, "planted.db"), []byte(planted), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
- raw, err := os.ReadFile(filepath.Join(dir, "leak.db"))
- if err != nil {
- t.Fatalf("ReadFile: %v", err)
- }
- if !strings.Contains(string(raw), "Next action") {
- t.Fatal("the grep this gate relies on does not find a plaintext canary")
+ if hits := scan(t, dir, canaries); len(hits) != len(canaries) {
+ t.Fatalf("scan found %d of %d planted canaries; the gate does not look for what it claims",
+ len(hits), len(canaries))
}
}