All 3 cloud models converged on real concurrency bugs in the SQLite stores:
- AppendVersion (HIGH): the seq key was `SELECT MAX(seq)+1` then INSERT in two
un-transacted statements with a NON-unique index, AND the Scan error was
swallowed (seq stayed 0 on failure). Concurrent appends could both land the
same seq, silently breaking newest-first ordering. Now: one transaction, the
Scan error is propagated, the (skill_id, seq) index is UNIQUE (the loser of a
race fails loudly), and an empty SkillID is rejected.
- MarkScheduledRun / MarkAgentScheduledRun (all 3): replaced the Get→mutate→Save
read-modify-write (lost-update window) with a single atomic UPDATE using
json_set, so a concurrent Mark/edit can't clobber it. json_set keeps the JSON
blob's NextRunAt/LastScheduledRunAt consistent with the indexed column;
RFC3339Nano matches Go's time encoding so the blob still round-trips (tested).
- Open: actually applies PRAGMA busy_timeout=5000 (the doc advertised it but it
was never set) — a contended writer waits instead of erroring SQLITE_BUSY.
- budgetStore.Add: rejects NaN/Inf secondsUsed (would irrecoverably poison the
column).
Triaged-but-kept: plaintext webhook secret (documented design, high-entropy URL
key, pre-existing); SQL()/free-form `where` helpers (no untrusted input reaches
them — defense-in-depth notes only).
Core go.sum still free of host/DB deps; contrib/store green (incl. a json_set
blob-round-trip test).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
db.Audit() satisfies audit.Storage (all 17 methods) over SQLite: one indexed
row per run (+ a JSON inputs blob), one row per log event. Filter/list/walk
queries are indexed on the columns they filter (skill_id, caller_id,
parent_run_id, started_at); WalkParentChain follows parent_run_id with a
seen-set guard; LastRunBySkills is a grouped MAX.
Test covers run start/finish round-trip (inputs map + token roll-up), log
append + ordered read, parent/child + ancestor-chain walks, caller listing,
TopLevelOnly filter, and the last-run-per-skill map.
contrib/store now backs ALL four store seams — budget + persona + skill + audit
— so a host gets turnkey durable persistence (run history, budgets, agents,
skills) with zero store code. Core go.sum still has 0 sqlite refs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
db.Skills() satisfies skill.SkillStore over SQLite, same JSON-blob + indexed
columns pattern. Versions live in their own table (each SkillVersion embeds a
full Skill snapshot as JSON), ordered newest-first by an append seq.
Test: round-trip (Tools, ExposeAsChatbotTool), visibility listing
(public/shared/private with SharedWith filtered in Go), chatbot-exposed,
newest-first versions + GetVersionByID, scheduled-due query + MarkScheduledRun.
contrib/store now covers budget + persona + skill; audit store next.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
db.Personas() satisfies persona.Storage over SQLite. Each Agent is stored as a
JSON blob with extracted indexed columns (owner_id, name, webhook_secret,
chatbot_channel_filter, schedule, next_run_at) — so the WHOLE struct round-trips
(no domain<->GORM<->DB field-loss footgun) while the lookups stay indexable.
Test proves the round-trip preserves nested + map fields (SkillPalette,
StateReactEmoji), the owner/name + webhook + chatbot-filter queries, the
scheduled-due query, and MarkAgentScheduledRun clearing the due window.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Establish the nested persistence module — the architectural reason the core
stays lean: a SEPARATE go.mod carrying modernc.org/sqlite (pure Go, no cgo), so
the SQLite driver NEVER enters the executus core go.sum. A static-binary host
(gadfly) importing only the core stays static; a host wanting turnkey
persistence imports contrib/store.
- sqlite.go: store.Open(dsn) -> *DB (one SQLite file), accessor-per-seam.
- budget_store.go: db.Budget() satisfies budget.BudgetStorage; Add() does the
7-day window rollover atomically inside a transaction (concurrent Adds can't
race the read-modify-write — the in-memory store's one weak spot).
- Conformance test: budget.NewDBBudget over the SQLite store passes the SAME
rolling-window contract as the in-memory store.
- CI: a new step builds + tests contrib/store on its own AND asserts it carries
the sqlite driver the core forbids (proof the split works). Verified: core
go.sum has 0 sqlite refs; contrib/store go.sum has it.
persona/skill/audit SQLite stores follow next (same JSON-blob + indexed-columns
pattern, sidestepping the three-layer field-loss footgun).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The skill half of the persona/skill pair, as a clean redesign (not a faithful
lift of mort's 60-method skills.Storage kitchen sink):
- skill.go/skill_version.go/validate.go/inputs.go/schedule.go moved clean; the
only host couplings severed: llms.IsTierName -> model.IsTierName, and the
chatbot DefaultChatbotInputName const localized.
- store.go: a DELIBERATELY LEAN SkillStore — skill lifecycle (CRUD + visibility)
+ versioning + scheduling ONLY. The KV/file/quota sub-stores that were fused
into mort's interface are the tools/ store seams; email/channel grants stay
host concerns.
- runnable.go: Skill.ToRunnable() lowers a skill into run.RunnableAgent (flat
tool list, no palette — composition is a host concern); DueAt() helper.
- memory.go: NewMemory() — zero-dep in-process SkillStore (visibility filters,
newest-first versions).
Tests: ToRunnable mapping, visibility (public/shared/private) listing, version
ordering + lookup. No mort dependency (go.mod tidy clean); core imports ZERO
from skill.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
steve