feat(storage): transactional agent authority - #1393
Conversation
Replace JSON authority with Runtime-owned SQLite and PostgreSQL storage, atomic domain mutations, durable receipts and notifications, and recoverable legacy upgrades and logical data transfers. Preserve artifact ownership, unknown domain data, rollback evidence and installed runtime portability. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
Activate verified authority after a successful full offline migration and initialize the isolated Library migration fixture with its own storage handle. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
Use durable revisions instead of millisecond timestamps when clearing search invalidation, and exercise offline migration activation through the real CLI. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
Keep bundled workflow loading independent of SQLite initialization and verify its real worker and storage lifecycle in a compiled isolated fixture. Align inbox assertions with atomic transcript publication and queue consumption. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
🧹 Oryn Mini · ✅ Task completeCode review · Attempt 1 · Cycle 1 Waiting for your decision: Merge once the pending required checks finish, or hold until the acknowledged follow-ups are tracked as linked issues? Run details · operational receipt{
"version": 1,
"workKey": "review:4ea7be348cc337c012711ac0",
"sourceKey": "4ea7be348cc337c012711ac0",
"intent": "review",
"phase": "completed",
"attempt": 1,
"cycle": 1,
"leaseID": "cc5475e2-f65e-440f-b38f-d442f2c1d6de",
"verdict": "no_action",
"triage": {
"category": "feature",
"priority": "P1",
"confidence": "high",
"reproduction": "source_proven",
"proof": "sufficient",
"decision": "keep_open",
"closeReason": "none",
"implementationCandidate": false,
"complexity": "large",
"evidence": [
{
"path": "packages/harness/src/session/part-write-buffer.ts",
"line": 57,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Success handler deletes the retained failure, so a later write for the same key self-heals instead of wedging the session (review blocker fixed)."
},
{
"path": "packages/harness/src/session/part-write-buffer.ts",
"line": 99,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "flushWhere retries retained failures at every drain boundary; keys with a newer buffered entry flush that entry and its success clears the failure."
},
{
"path": "packages/harness/test/session/part-write-buffer.test.ts",
"line": 83,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Regression test: a transient part failure does not block later drains of the same session."
},
{
"path": "packages/harness/src/storage/sqlite-worker.ts",
"line": 9,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Worker sets umask 077 before opening the database so the db and WAL sidecars stay owner-only (review finding S-1 fixed)."
},
{
"path": "packages/harness/src/storage/sqlite-driver.ts",
"line": 101,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Driver chmods db, -wal and -shm to 0600 on open, including sidecars left by older engines."
},
{
"path": "packages/harness/test/storage/transactional-store.test.ts",
"line": 173,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Regression test asserts the -wal sidecar mode is 0600."
},
{
"path": "packages/harness/src/storage/sqlite-driver.ts",
"line": 132,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Maintenance requests get a 600s deadline instead of the 30s shared deadline, so integrity_check no longer kills the worker (review finding P-2 fixed)."
},
{
"path": "packages/harness/src/storage/transactional-store.ts",
"line": 620,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "PRAGMA integrity_check is issued with { maintenance: true } to use the extended deadline."
},
{
"path": "packages/harness/src/storage/portable.ts",
"line": 46,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "authorityRecordRoots (plugin approvals/audit/lock/runtime-state/incompatible/intents, permissions, permission-rules, registry) defined with the consent-suppression rationale."
},
{
"path": "packages/product-runtime/src/cli/data/transfer.ts",
"line": 109,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Untrusted data merge refuses authority-adjacent record roots (review finding S-2 fixed)."
},
{
"path": "packages/product-runtime/src/cli/data/move.ts",
"line": 252,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Same-home relocation opts in via { trusted: true }, the only transfer allowed to carry authority records."
},
{
"path": "packages/harness/src/storage/bootstrap.ts",
"line": 256,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Bootstrap convenience import also refuses authority roots because the archive may originate from another home."
},
{
"path": "packages/product-runtime/test/cli/data-transfer.test.ts",
"line": 70,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Regression test: merge refuses authority records from another home while trusted relocation keeps them."
},
{
"path": "packages/harness/src/session/message-v2.ts",
"line": 1420,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "messageOrderSnapshot restored on Storage.transaction — the read-only snapshot rewrite was reverted (d32db644c) after the Cortex read-after-write regression, keeping the other fixes."
},
{
"path": "docs/decisions/implemented/architecture/2026-09-14-transactional-agent-authority.md",
"line": 3,
"sha": "7cbef7dfcc31f7fef5e4b2a0150b812608aea4c0",
"detail": "Implemented decision record documents the problem, decision, alternatives and consequences, linking agent-storage architecture and upgrade migration docs."
}
],
"risks": [
"This review is source-level; behavioral assurance relies on author-reported local runs and the partially completed CI matrix — PostgreSQL 18, Quality, Typecheck and several shards were still pending at snapshot.",
"Activation is one-way: downgrade needs a restored Home and loses post-snapshot changes; recovery depends on the immutable backup discipline being followed.",
"Message-order cache-hit reads still take the write transaction after the deliberate revert, so reads can queue behind the single SQLite writer under concurrent streaming.",
"storage_nodes rows are never pruned, so the index grows with historical writes on long-lived homes until the pruning follow-up lands."
],
"decisionNote": {
"question": "Merge once the pending required checks finish, or hold until the acknowledged follow-ups are tracked as linked issues?",
"options": [
"Merge as soon as the pending required checks finish; file the follow-up issues in parallel.",
"Hold merge until follow-up issues for storage_nodes pruning, retry consolidation, metadata() grammar and the read-path optimization exist and are linked.",
"Request a second independent review of the post-review fix commits before merging."
],
"recommendation": "Merge once the pending required checks finish — the approval is pinned to head and covers the fixes — and open the four follow-up issues in the same pass so the acknowledged work stays visible."
}
},
"run": "https://github.com/SII-Holos/synergy/actions/runs/34965022267",
"summary": "Waiting for your decision: Merge once the pending required checks finish, or hold until the acknowledged follow-ups are tracked as linked issues?\nValidation has not run. No repair patch was published by this attempt."
} |
🧹 Oryn Mini · 🧭 Your input is needed🟠 P1 · High · 💡 Feature · High confidence Why this PRThis PR replaces independent JSON file writes with a Runtime-owned transactional storage handle, so session, message, inbox, workflow, approval and registry mutations commit atomically on SQLite (default) or PostgreSQL 16–18, with immutable backups, resumable import and recovery paths for existing installs. It matters because cross-record invariants and plugin-consent records previously could tear or lose updates, and a single transient streaming-write error could wedge a session until restart. All five issues raised during review are fixed and source-verified at head; the message-order read-path optimization was reverted after a real Cortex… Full explanationThis PR replaces independent JSON file writes with a Runtime-owned transactional storage handle, so session, message, inbox, workflow, approval and registry mutations commit atomically on SQLite (default) or PostgreSQL 16–18, with immutable backups, resumable import and recovery paths for existing installs. It matters because cross-record invariants and plugin-consent records previously could tear or lose updates, and a single transient streaming-write error could wedge a session until restart. All five issues raised during review are fixed and source-verified at head; the message-order read-path optimization was reverted after a real Cortex read-after-write regression and remains a tracked follow-up. What changed
How it fits togetherThe Runtime Handle owns a transactional store committing session aggregates, receipts and notifications atomically on SQLite (subprocess IPC) or PostgreSQL; JSON installs upgrade via backup and import; the data CLI moves logical exports. flowchart TD
accTitle: Transactional agent storage at a glance
accDescr: The Runtime Handle owns a transactional store committing session aggregates, receipts and notifications atomically on SQLite (subprocess IPC) or PostgreSQL; JSON installs upgrade via backup and import; the data CLI moves logical exports.
n_0["Runtime Storage Handle"]
n_1["Transactional logical-record store"]
n_2["SQLite IPC worker subprocess"]
n_3["PostgreSQL driver"]
n_4["Session aggregate mutations"]
n_5["Durable notification outbox"]
n_6["JSON import and backups"]
n_7["data pack/merge/move CLI"]
n_0 -->|"owns"| n_1
n_1 -->|"SQLite contract"| n_2
n_1 -->|"same contract"| n_3
n_4 -->|"atomic commit"| n_1
n_1 -->|"publish after commit"| n_5
n_6 -->|"upgrade path"| n_1
n_7 -->|"logical exports"| n_1
Review findings🔵 Low · Message-order cache-hit reads still take the write transaction — packages/harness/src/session/message-v2.ts:1420 The P-1 read-path optimization (read-only snapshot for the cache probe) was deliberately reverted after it broke Cortex read-after-write visibility for finalized subagent turns (4 failing tests, root-caused and reproduced by the specialist review). messageOrderSnapshot again wraps the probe in Storage.transaction, so reads can queue behind the single SQLite writer under concurrent streaming. Correct trade-off for now; the optimization should only re-land once read visibility is proven. 🔵 Low · storage_nodes index grows without pruning — packages/harness/src/storage/transactional-store.ts:192 Node rows are insert-only (ON CONFLICT DO NOTHING) with no delete path, so the index grows with historical writes on long-lived homes; recursive scans and verification walk the full tree. Acknowledged non-blocking follow-up from the review thread. 🔵 Low · Maintainability follow-ups acknowledged by review — packages/harness/src/storage/transactional-store.ts The specialist review flagged duplicated retry/backoff policy across the store and driver layers and the metadata() key grammar as an undocumented coupling point for future indexed domains. Both were accepted as non-blocking; they should be tracked as issues so they are not lost after merge. More findings (1)🔵 Low · defer() intentionally fails fast on a retained failure — packages/harness/src/session/part-write-buffer.ts:17 After a transient part-write failure, defer() rethrows the retained error until a drain boundary retries it (flushWhere) or a successful writeNow clears it. This fail-fast is deliberate — it prevents silently buffering onto a broken store — and is covered by the regression tests. Future readers should not 'fix' it back into silent buffering. Before merge
Maintainer decision: Merge once the pending required checks finish, or hold until the acknowledged follow-ups are tracked as linked issues?
Recommendation: Merge once the pending required checks finish — the approval is pinned to head and covers the fixes — and open the four follow-up issues in the same pass so the acknowledged work stays visible. Things to watch
Verification🔎 Evidence supports the conclusion. Confirmed in source; not reproduced in a live run. No host validation commands were run for this report. Source evidence (15)
Discussion and CI reflect the snapshot read for this review. Current checks and approval are verified separately before merging. 🧹 Oryn Mini · Source 7cbef7d · Built with Synergy core |
Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
Regenerate the core configuration schema and update lifecycle, Push, observability and benchmark probes to use explicit storage ownership and committed records. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
Give the POSIX SQLite worker its own process group so SIGINT and SIGTERM cannot close authority before the Runtime drains cancellation evidence. Verify real group signals, terminal accounting and owner-loss recovery. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
Exercise real bootstrap, recovery, inspection and target switching through CLI commands. Keep fresh Runtime composition and coverage attribution consistent across test runners. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
Review Summary: Request Changes (1 blocker)This is a high-quality, unusually well-engineered refactor. Atomicity/crash-safety wiring verified sound across the session domain (derived indexes commit with canonical writes, durable outbox notifications never precede commit, correct two-phase journal saga, clean reserve-publish-discard staging, no JSON dual-write leftovers, Four issues should be addressed before merge — one blocking, three strongly recommended (all small, localized fixes): 🔴 Blocker:
|
…the writer A retained part failure now clears when a later write for the same key succeeds and retries at every drain boundary, so one transient storage error cannot wedge a session until Runtime restart. Message order reads use a read-only snapshot and escalate to a transaction only when the stored index needs rebuilding. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
…m the request deadline The worker enforces an owner-only umask so the database and its WAL sidecars never appear world-readable, and the driver re-tightens them to 0600 on open to repair sidecars from older engines. Integrity verification runs under an extended deadline instead of killing the worker mid-maintenance on large datasets. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
…rchive Grants, consent and trust decisions (plugin approvals, permissions, registry and related plugin state) never cross homes through an untrusted data merge or the bootstrap convenience import: an imported approval would silently satisfy the consent prompt for a later plugin install. Same-home relocation (data move) opts in explicitly. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
…ity transfer boundaries Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
|
All five findings from the review above are fixed and pushed (aaa5fd0..01a0cf7):
Docs updated (agent-storage architecture, decision record, migration guide). Full local validation: Non-blocking follow-ups from the review (storage_nodes pruning, retry-layer consolidation, |
The read-only snapshot rewrite of loadMessageOrder broke Cortex task output: subagent assistant messages were read back empty after their finalizing turn (summary extraction, final_response capture, usage reads and durable output all lost). Root cause was isolated by reverting only message-v2.ts at bf7813c while keeping the part-write-buffer recovery; the original messageOrderSnapshot semantics are restored until the read path can move off the writer without changing read-after-write visibility. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
|
Follow-up: CI regression fixed by reverting the message-order read-path change (d32db64). The push at bf7813c broke 4 Cortex tests in two shards (channels / product-platform): subagent assistant messages were read back empty after their finalizing turn — summary extraction found no messages, Root cause isolation (all reproduced locally):
The read-only snapshot rewrite of Kept intact: the PartWriteBuffer sticky-failure recovery (B-1), WAL sidecar 0600 permissions (S-1), authority-record rejection on untrusted merge (S-2), and the maintenance deadline exemption (P-2). Session suite re-verified after the revert: 781 pass / 0 fail. |
Keep transaction-owned parts out of streaming retries, quarantine malformed legacy session aggregates before owner migrations, and bundle the verified SQLite engine in macOS Harness archives. Cover rollback, released upgrades, and packed-artifact initialization with regression tests. Co-authored-by: synergy-agent <299070056+synergy-agent@users.noreply.github.com>
Agent authority now commits through an explicit Runtime-owned storage handle instead of independent JSON files. Session, message, inbox, workflow, approval and registry mutations can commit their records, derived indexes, receipts and notifications together. SQLite remains the default; PostgreSQL 16–18 implements the same contract with explicit namespace ownership.
Existing installations upgrade through an immutable byte-for-byte backup, resumable import checkpoints, registered owner migrations, relationship validation and a single activation boundary. Malformed historical Session records retain their evidence and block execution; global corruption stops activation. There is no live JSON mirror. Pack, merge and move use checksummed logical exports and preserve files and Git snapshots separately, including skipped conflicting Session aggregates.
The change also covers bounded streaming writes, stale-writer fencing, worker shutdown, interrupted Session import/fork cleanup, plugin installation recovery, target-switch recovery, patched macOS SQLite packaging, maintenance commands, generated contracts, architecture documentation and operational recovery guidance. Runtime failover and simultaneous replicas sharing a namespace remain separate architecture work.
Validation completed locally:
bun run quality:quick— all 17 gates passed, including typecheck, generated documentation, dead-code and workflow checks.bun test test/storagewith real PostgreSQL 16 — 61 tests passed, including the SQLite contract, owner-loss recovery and three released JSON upgrade fixtures.bun packages/product-runtime/script/build.ts --single --skip-installsucceeded.bun test test/cli/artifact.test.tsagainst the compiled product — all six isolated, outside-repository execution tests passed.CI includes a mandatory PostgreSQL 16/17/18 matrix in addition to the repository's existing Linux, Windows, coverage and installed-artifact checks. Required check results and the complete job matrix are attached to this pull request.