Skip to content

🔒 Fence WorkflowRun transactions and unify savepoints - #396

Merged
taras merged 2 commits into
mainfrom
agent/issue-365-4-savepoint-fences
Aug 9, 2026
Merged

🔒 Fence WorkflowRun transactions and unify savepoints#396
taras merged 2 commits into
mainfrom
agent/issue-365-4-savepoint-fences

Conversation

@taras

@taras taras commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Why

Stack layer 4 of #365 replaces the savepoint/fencing portion of draft #387. The retained Workspace foundation needs operation-spanning savepoints and exact provider-owned transaction authority before a later layer can coordinate mutation, root capture, and journal publication safely.

This PR does not close #365.

What changes

Before:

Cloudflare DOFS initialization had synchronous savepoints, but private Workspace operations relied on a connection-wide open boolean and public Deno handles carried a discoverable raw-connection association. There was no operation savepoint whose decision waited for child teardown.

After:

Each authoritative connection has an opaque generation, each BEGIN IMMEDIATE has an exact active transaction identity, and provider-owned lease/handle registrations validate possession before SQLite is touched. Synchronous DOFS nesting and operation-spanning savepoints share one collision-free allocator. Operation savepoints run in a child scope, wait for teardown, and release or roll back synchronously; savepoint SQL failures poison the outer transaction so it cannot commit. After a successful ROLLBACK TO plus RELEASE, the shared rollback path invalidates both caches on the authoritative DOFS wrapper before outer transaction work resumes.

How it works

WorkflowRunDatabase.transact()
  → provider-owned active RunTransaction
  → exact lease/handle/token validation
  → shared SavepointManager
  → child-scope teardown
  → RELEASE, or ROLLBACK TO + RELEASE + cache invalidation

The structural active-path context remains refusal-only. Adapter-private contextual Workspace operations close over the provider registry and authorize work through exact identities; public WorkflowRunDatabase values expose no raw SQLite or DOFS connection.

Review guide

Start with: packages/workflow/tests/workspace-transaction.test.ts

Then review:

  1. packages/workflow/src/deno/connections.ts — generations, active transaction records, lease/token registration, and cache ownership
  2. packages/workflow/src/deno/savepoints.ts — the shared synchronous/operation lifecycle, rollback cache cleanup, and poisoning rules
  3. packages/workflow/src/deno/database.ts and transaction.ts — caller-owned transaction teardown and contextual savepoint installation
  4. packages/workflow/src/deno/workspace/private.ts — adapter-private exact-identity authorization
  5. Architecture and workflow-storage specification updates, including the WTX conformance inventory

Look carefully at:

What must stay true

  • The caller owns the only top-level transaction — DOFS uses the authoritative SQLite connection and emits no autonomous top-level BEGIN, COMMIT, or ROLLBACK; checked by WTX5 and the storage suites.
  • Child resources finish before an operation savepoint releases — enforced by an explicit Effection scope and checked by WTX1, WTX3, and WTX4.
  • Missing, foreign, fabricated, completed, closed, and stale authority is refused before SQL — enforced by exact provider-owned registrations and checked by WTX7–WTX9.
  • Public handles expose no raw connection — enforced by private WeakMap associations and checked by WTX9.
  • A successful savepoint rollback invalidates both authoritative DOFS caches before the outer transaction may continue and commit — checked by WTX10.
  • A rolled-back outer transaction separately invalidates the authoritative DOFS caches — retained from 🌳 Retain and restore immutable Workspace roots #393 and checked by WRR10/WRR10b.
  • Historical root recognition still checks each file size against its DOFS manifest — retained from 🌳 Retain and restore immutable Workspace roots #393 and checked by WRR11.

How to verify it

  • deno test -A packages/workflow/tests/workspace-transaction.test.ts packages/workflow/tests/workspace-root.test.ts packages/workflow/tests/workspace-root-restoration.test.ts packages/workflow/tests/workflow-run-storage.test.ts packages/workflow/tests/workflow-run-journal.test.ts — 16 suites / 103 steps pass.
  • deno task lint — 0 errors; formatting is clean.
  • deno task check — passes.
  • deno task test — 405 suites / 2,770 steps pass.
  • deno task check:jsrSuccess Dry run complete.
  • deno task verify — vendor, lint, check, Deno tests, JSR, tsc, Bun tests, and docs pass on both complete runs. Node alone reproduces the already-recorded CF7 post-test asynchronous-activity flake in 💥 Stabilize Node runtime test flake in config-api CF7 #371; all CF assertions pass, PR 🔒 Fence WorkflowRun transactions and unify savepoints #396's existing Node CI is green, and this stack does not import the unrelated pending fetch fix. Both verifier runs leave the tracked tree unchanged.
  • git diff --check origin/main...HEAD — passes.

Scope

Included

  • Provider-owned connection generations and per-BEGIN transaction identities
  • Provider-owned public-handle leases and adapter-private transaction tokens
  • One allocator shared by synchronous DOFS and operation-spanning savepoints
  • Connection-owned DOFS cache invalidation after successful savepoint rollback
  • Exact identity validation in private retained-root capture/restoration operations
  • Focused savepoint, cancellation, failure, cache-coherence, concurrency, and stale/foreign/fabricated authority tests
  • Tier WTX conformance inventory and present-tense architecture/workflow-storage contract updates

Intentionally unchanged

  • Provider-neutral durable-effect coordination and Workspace capabilities
  • Journal destinations/routing, secret-gate integration, and atomic mutation/root/event publication
  • Public filesystem effects, <File>, start/resume, history/fork, Worker Shell, Git, FUSE, subprocesses, workerd, garbage collection, and schema versions

New abstractions

  • ConnectionGeneration, TransactionIdentity, and WorkflowRunTransactionToken provide exact adapter-private authority that cannot be reconstructed from paths or structural lookalikes.
  • RunTransaction is the authoritative active record shared by transaction, savepoint, DOFS, and private Workspace validation.
  • The private Workspace contextual API lets composition scopes request validated adapter operations without placing callbacks or raw connections in structural context.
  • Each new abstraction has multiple concrete uses or a clear justification.
  • No speculative functionality is included.

Risks and limitations

  • The next coordination layer must possess the explicit adapter-private transaction token; it does not need to redesign generation or transaction identity.
  • This layer deliberately does not publish Workspace mutations or filtered durable results atomically.
  • Local complete verification is currently gated only by the open Node CF7 flake 💥 Stabilize Node runtime test flake in config-api CF7 #371, which is unchanged by this PR.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • Generated or mechanical changes are clearly identified.
  • The description matches the final diff and test results.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

PR #396: 🔒 Fence WorkflowRun transactions and unify savepoints

18 files, +1340 / -207

Scope

🔴 PR has 1547 lines changed. Split into focused PRs.

🟡 1547 lines changed. PRs under 400 receive more thorough review.

Structural

🟡 Type declarations with no consumers: SavepointManager.
Symbol Declared at Refs in diff Why flagged
SavepointManager packages/workflow/src/deno/connections.ts:17 1 referenced ≤1× within the added diff (pre-existing usages not counted)

Slop

✅ Slop indicators look low.

Static Analysis

✅ Oxlint found no issues.

Correctness

FILE: packages/workflow/src/deno/workspace/private.ts
PATTERN: Multiple redundant authorize() calls
CONCERN: Excessive, unexplained calls to authorize() in createDenoWorkspaceFilesystem (13 total) with no observable effect. This appears to be dead code or a wrapper indirection pattern without clear justification.
QUESTION: Why are 13 authorize() calls made in sequence with no intervening logic? Is this a bug or redundant scaffolding?


FILE: packages/workflow/src/deno/connections.ts
PATTERN: Empty class definitions with no methods
CONCERN: ConnectionGeneration, TransactionIdentity, and WorkflowRunTransactionToken classes only contain private fields. These appear to be speculative abstractions with no concrete implementation details or usage beyond type enforcement.
QUESTION: Are these classes necessary, or could their functionality be represented with simpler types (e.g., symbols or strings)?


FILE: packages/workflow/src/deno/savepoints.ts
PATTERN: Redundant error swallowing in report()
CONCERN: The report function in SavepointObservation contains a try...catch that silently discards errors. This could mask critical issues during savepoint observation.
QUESTION: Is the error suppression intentional, or is this a potential bug that could hide validation failures?

@taras
taras force-pushed the agent/issue-365-4-savepoint-fences branch from 1cbea52 to 454bad7 Compare August 9, 2026 00:47
@taras
taras marked this pull request as ready for review August 9, 2026 00:51
@taras
taras enabled auto-merge (squash) August 9, 2026 00:54
@taras
taras merged commit 5f5a725 into main Aug 9, 2026
11 checks passed
@taras
taras deleted the agent/issue-365-4-savepoint-fences branch August 9, 2026 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Commit Workspace mutations and journal results atomically

1 participant