Skip to content

Bound local-activity timestamps and completion-wide persistence work #930

Open

Description

@rmcdaniel

Problem

The package-owned record_local_activity contract has two availability boundaries that are unsafe when a workflow-task completion is treated as a whole:

  1. duration_ms, heartbeat elapsed_ms, and retry backoff_seconds accept every non-negative platform integer, but the contract defines no safe timestamp range. The Rust Server port at Rust #325: record local activities like Server 2.11.1 server#766 reaches unchecked Chrono construction/arithmetic with these values and can unwind the request handler instead of returning a deterministic protocol refusal. The PHP implementation uses the same unbounded values in Carbon timestamp arithmetic, so the owning contract needs one portable bound and explicit refusal behavior.
  2. The limits of 100 commands per completion and 1,000 heartbeats per local-activity command compose to at least 100,300 history events in one transaction. Minimal JSON for that batch fits under the default request-body ceiling. Each heartbeat history event also repeats the running activity snapshot, including arguments, so a large valid payload can be persisted hundreds or thousands of times. Current history-budget thresholds are reporting/advisory and do not refuse this write before mutation.

Security review evidence and exact Rust line references: durable-workflow/server#766 (comment)

Required contract and implementation outcome

  • Define portable maximums or checked conversion rules for all three timing fields. Extreme and out-of-range values must receive a stable 4xx protocol refusal, never an exception/panic.
  • Preflight a deterministic whole-completion budget for projected durable events/rows and serialized history bytes before any mutation. Per-command heartbeat limits alone are insufficient.
  • Either stop repeating large immutable payloads in every heartbeat snapshot or include all repeated bytes in the admission budget without changing replay semantics accidentally.
  • Keep ownership/capability checks first where the existing contract requires them, but complete all resource/timestamp validation before the first durable write.
  • Add exact-boundary and one-over tests for timing, aggregate heartbeat/event work, and a large valid argument with heartbeats. Refusals must preserve the task lease and leave executions, attempts, history, projections, and receipts unchanged.
  • Qualify the package change through the PHP Server and the Rust port on SQLite, PostgreSQL, and MySQL. The Rust port must consume the package-owned rule rather than inventing a native-only limit.

Do not treat continue_as_new_recommended as enforcement; this admission guard protects the current transition before it can create the oversized history.

Activity

  1. rmcdaniel commented on Oct 25, 2026

    @rmcdaniel
    MemberAuthor

    Design for review before implementation. I own this under Server #325. The package ships the rule first; the PHP Server and the Rust port consume it.

    1. Timing maxima. These are new public constants on WorkflowCommandNormalizer, published in localActivityCommandContract(). Each out-of-range value gets the normalizer's ordinary 422 field error.

    Field Maximum
    attempts.*.duration_ms 604,800,000 (7 days)
    attempts.*.heartbeats.*.elapsed_ms 604,800,000 (7 days)
    attempts.*.backoff_seconds 86,400 (1 day)

    The bridge back-dates each attempt from now() by at most its duration plus the following backoff. With at most 100 attempts, the worst case is about 800 days. That stays representable in Carbon, in Chrono, and in every backend's timestamp column, including MySQL TIMESTAMP, which cannot go below 1970. No arithmetic can overflow before validation has run.

    2. Whole-completion event budget. Before the first write, the bridge projects the history events of the whole completion:

    • each record_local_activity command counts 1 scheduled event, then per attempt 1 started, its heartbeats and a retry if it isn't the last attempt, then 1 terminal event;
    • every other command counts its fixed events.

    If the total exceeds the existing StructuralLimits::historyTransactionSizeLimit() (workflows.v2.structural_limits.history_transaction_size, default 5,000), the completion is refused with the existing structural-limit refusal. That is the same guard the embedded WorkflowExecutor already applies to one transaction. The rule's budget is shared, not native-only, and the Rust port reads the same config and default.

    3. Repeated snapshot bytes. Every local-activity event repeats the activity snapshot, including the encoded arguments. Before writing, the bridge projects events_c × strlen(arguments_c) for each command and sums it across the completion. If the sum exceeds HistoryBudget::sizeBytesHardThreshold() (default 5 MiB), the completion is refused. The arguments are compared as the stored encoded strings, so PHP and Rust count identical bytes.

    I'd rather budget the repetition than drop arguments from the heartbeat snapshots. Dropping them would change the published history shape, and every replay and export consumer would have to follow.

    Order. Ownership and capability checks still come first. Then the normalizer runs, then the budget, then the first write. On refusal there is no execution, attempt, history or projection row, and no receipt, and the lease stays usable.

    Tests. Each timing field at the maximum and at one over, plus a value equal to the platform's largest integer. The event budget at exactly 5,000 and at 5,001. A large valid argument with heartbeats at the byte boundary and one over. A refusal followed by the same task completing normally. These run on SQLite, PostgreSQL and MySQL in the package, then through the PHP Server and the Rust port (Server #766).

    Please say if any constant or the byte measure should differ before I write it.

    🤖 Generated with Claude Code

  2. rmcdaniel commented on Oct 25, 2026

    @rmcdaniel
    MemberAuthor

    Security design review: the timing caps are a reasonable portable envelope, but the proposed byte/event admission design needs correction before implementation.

    1. HistoryBudget::sizeBytesHardThreshold() is currently an advisory continue-as-new threshold. Reusing it as a hard protocol refusal silently changes an operator-facing configuration from guidance to admission control. Add an explicitly hard structural byte limit with documented zero/custom semantics, or make the compatibility and migration decision explicit rather than repurposing the advisory key.
    2. sum(events_c * strlen(arguments_c)) is not serialized history-byte accounting. It omits event type and JSON structure, the repeated activity and attempt snapshots, heartbeat details, retry messages and metadata, terminal result/failure data, and events emitted by other commands plus signal-resume, condition-resolution, waiting, and terminal handling. Preflight every history event the transaction can emit with the same canonical measure used by HistoryBudget (strlen(event_type) + canonical serialized payload bytes), or use a documented conservative upper bound that cannot undercount.
    3. The issue requires durable events and rows. Count associated execution, attempt, failure, projection, receipt, and task mutations too, or define a separate explicit row budget. A history-event-only total is not evidence for a whole-transaction row bound.
    4. Use checked addition and multiplication for every aggregate and refuse on arithmetic overflow. Validate all timing fields before arithmetic, then keep checked Carbon/Chrono duration and timestamp operations as defense in depth. The present 7-day duration/elapsed and 1-day backoff caps fit the 100-attempt envelope, but current wall-clock distance from 1970 is not the portable safety proof.
    5. Place admission before recordAppliedSignalForSignalResume() and recordSatisfiedConditionWaitForSignalResume(), because those can mutate before the current command loop. Boundary tests should cover exact/one-over event and canonical-byte totals, aggregate non-history rows, custom and disabled configuration semantics, arithmetic overflow, every timing field, and a refused completion followed by successful reuse of the unchanged lease on all three databases.

    Please revise the design before writing the package implementation. The Rust port should consume the resulting published constants and formulas exactly.

  3. rmcdaniel commented on Oct 25, 2026

    @rmcdaniel
    MemberAuthor

    Revised design after review 6402480418. Nothing is published, and the earlier draft stays unpushed.

    1. Timing fields (unchanged caps, checked math)

    Field Maximum
    attempts.*.duration_ms 604,800,000
    attempts.*.heartbeats.*.elapsed_ms 604,800,000
    attempts.*.backoff_seconds 86,400

    These are public WorkflowCommandNormalizer constants. An out-of-range value gets its own 422 field error, even on a retried attempt, where today the missing-backoff message would replace it.

    Portable bound. The bridge back-dates attempts from the completion instant by at most MAX_LOCAL_ACTIVITY_ATTEMPTS × (604,800,000 + 86,400 × 1,000) = 69,120,000,000 ms.

    • That sum is computed with checked addition and multiplication. PHP refuses on overflow past PHP_INT_MAX; Rust uses checked_*.
    • The earliest derived timestamp is checked against the storage minimum before any write, refusing with 422 local_activity_timestamp_out_of_range. The minimum is 1970-01-01T00:00:01Z for MySQL TIMESTAMP and 0001-01-01 elsewhere.
    • Carbon and Chrono operations stay checked as defense in depth. Rust uses TimeDelta::try_milliseconds and checked_sub_signed.

    2. Whole-completion meter, in the history writer itself

    Projecting every path ahead of time would re-implement the bridge's branches (signal resume, condition resolution, waiting, every command, terminal handling and parent notifications) and could drift from them. Instead, the completion transaction carries a meter that charges what is actually written:

    • WorkflowHistoryEvent::record() is the only PHP writer of history, and native's append is the only Rust one. Each charges one event, plus its bytes in the HistoryBudget measure: strlen(event_type) plus the json_encode(payload, JSON_UNESCAPED_SLASHES|JSON_UNESCAPED_UNICODE) length.
    • Every durable row the transaction inserts or updates charges one row. On the PHP side that covers each V2 model's create and save inside the metered scope plus the projection writers; native counts in its transaction.
    • When a total would pass its limit, the meter throws before that write. The transaction rolls back, so no history, execution, attempt, failure, projection, task or receipt row survives. The bridge answers completed: false with reason: history_transaction_limit_exceeded and names the dimension. The task row's lease change is rolled back too, so the same lease can complete a smaller completion.
    • The meter opens before recordAppliedSignalForSignalResume() and recordSatisfiedConditionWaitForSignalResume(), so it covers them.

    Work before a refusal is bounded by the limits themselves. Results are deterministic: the same input reaches the same limit at the same write, and both implementations charge the same events, bytes and rows.

    Limits. All are hard StructuralLimits. A value of 0 disables that dimension, as every structural limit documents.

    Config Status Default
    structural_limits.history_transaction_size Existing; the embedded executor already enforces it 5,000 events
    structural_limits.history_transaction_bytes New: DW_V2_LIMIT_HISTORY_TRANSACTION_BYTES / WORKFLOW_V2_LIMIT_HISTORY_TRANSACTION_BYTES 16 MiB of event bytes
    structural_limits.history_transaction_rows New, same naming 20,000 rows

    HistoryBudget stays advisory.

    The 16 MiB default allows several MiB of legitimate payloads plus thousands of ~1.3 KiB fixed snapshots. The row default is four per event at the event limit.

    3. Cheap preflight for the amplifier

    The normalizer still refuses, as a 422 before the transaction, a completion whose local activities alone exceed the event limit. That count is exact: 2 + attempts + heartbeats + (attempts − 1) per command, summed with checked addition. The meter remains the authority for every other path and for bytes and rows.

    Tests (package, then PHP Server and Rust on SQLite, PostgreSQL and MySQL)

    • Each timing field at the maximum, one over, and PHP_INT_MAX / i64::MAX. The aggregate and storage-minimum guards are also driven directly.
    • Events, bytes and rows each at the exact limit and one over, with the meter's totals asserted equal to the rows actually present after an accepted completion.
    • A large valid argument with heartbeats crossing the byte limit.
    • A completion whose budget is spent through signal resume and a terminal command.
    • Custom limits and disabled (0) limits.
    • After every refusal: no new rows in any table, the task still leased to the same owner with the same expiry, and a smaller completion then succeeding on that lease.

    The Rust port takes the same constants, the same measure and the same configuration keys.

    🤖 Generated with Claude Code

  4. rmcdaniel commented on Oct 25, 2026

    @rmcdaniel
    MemberAuthor

    Security review of the revised design: the new hard StructuralLimits, writer-level event/byte meter, pre-signal placement, checked timing math, and advisory-only HistoryBudget resolve the prior findings. Two boundaries still need to be explicit before implementation.

    1. A database rollback does not roll back external payload storage. Multiple completion paths call ExternalPayloads::externalizeForNamespace() before their first model/history write; applyRecordLocalActivity() externalizes arguments, result, and exception first. That calls ExternalPayloadStorageDriver::put(), which is outside the database transaction. A later byte/row/event refusal can therefore leave a durable object even though every database row and the lease roll back. The returned URI can also change the eventual serialized event size. Do not delete blindly on failure because a content-addressed object may already be shared. Either complete a conservative admission pass before any put, or add an explicit transaction-aware object staging/promotion contract with safe reference accounting and rollback. Add a fake-driver regression proving a refused completion performs no put or leaves no orphan across every externalizable command path.
    2. Rows must mean affected durable rows, not ORM operations. Count inserts, updates, upserts, and deletes, including query-builder/raw/bulk/projection paths. Model hooks alone can miss those paths, and charging one for a bulk update does not bound work. Enumerate the completion write sites and either charge their known cardinality before execution or use a transaction-safe mechanism whose work is independently bounded. Tests should compare the meter with actual row deltas for single-row and bulk paths, including deletion/cancellation paths.

    The timing section and exact local-activity event formula are accepted for implementation once those two boundaries are incorporated. Keep limit exceptions outside the transaction body so nothing catches one and continues, and fail closed if the canonical JSON byte measure cannot be produced.

  5. rmcdaniel commented on Oct 25, 2026

    @rmcdaniel
    MemberAuthor

    Design v3: this addresses the two remaining boundaries in 6402567710. The timing caps, the exact local-activity event formula, the hard StructuralLimits and the writer-level event/byte meter stay as accepted in v2.

    1. No external put before admission: staged, content-addressed references

    Both built-in drivers derive the URI from the content alone, so the reference is known before anything is written:

    • FilesystemExternalPayloadStorage builds scheme://authority/[prefix/]codec/aa/sha256.
    • LocalFilesystemExternalPayloadStorage builds file://root/codec/aa/sha256.

    The change:

    • New interface. Add PredictableExternalPayloadStorageDriver extends ExternalPayloadStorageDriver with uriFor(string $sha256, string $codec): string, and implement it on both drivers. The URI the method returns must equal what put() returns for the same input. A test enforces that for both drivers and for prefixed and unprefixed configurations.
    • Inside a metered completion, ExternalPayloads::externalizeForNamespace() does not call put(). It computes the SHA-256 and the final stored envelope from uriFor(), and registers the bytes with the transaction's stage. The rows and events carry the final reference, so the meter charges the exact bytes that will be committed.
    • When the body finishes with every limit satisfied, the stage performs the puts in order, still inside the transaction and before commit. Each put() must return the predicted URI; a mismatch throws and rolls back.
    • A refusal (event, byte or row limit) happens before any stage flush, so it performs no put.
    • An infrastructure failure during the flush rolls the database back. Objects already written stay unreferenced and are never deleted, because content-addressed objects may be shared. That is the same exposure as today's write-first order, now limited to driver errors after admission.
    • Custom drivers without uriFor() fail closed. A metered completion that would externalize through one is refused before any put with reason: external_payload_staging_unsupported. Below the threshold, nothing is externalized and nothing changes.
    • Regression. A recording fake driver runs every externalizable completion path (local activity arguments, result and exception; activity schedule; workflow result, failure and continue-as-new arguments; update result; signal-resume payloads). It shows a refused completion records zero puts, and an accepted one records exactly the referenced objects. The Rust port refuses local-activity external storage today and will keep the same "no put before admission" rule wherever it externalizes.

    2. Rows: known cardinality, charged before each write

    The meter's row dimension charges each completion write site its row count before the statement runs, rather than counting ORM calls:

    • One-row writes. A model create, a save of one model, and a projection upsert of one key each charge 1, before execution.
    • Worker-driven multi-row writes. These are charged their exact count before execution: per local activity, 1 execution plus n attempts plus 1 failure when failed or timed out; per command, the tasks, timers, child runs and links it creates.
    • State-driven bulk cascades. Terminal and cancellation paths that update or delete pending activities, timers, children, signals, waits or tasks first read the ids they will change, under the locks they already hold, and charge that count before the bulk statement. They also stay bounded by the existing pending-count structural limits.
    • Audit. I'll enumerate every write site reachable from complete() and from the signal-resume and condition paths, including query-builder, raw and bulk writes, in a table in the PR. Each entry names its file, its line and how its cardinality is charged. A test compares the meter's row total with actual per-table row deltas for single-row, bulk and cancellation completions. The Rust port charges the same sites with the same counts.

    Exceptions and failure modes

    • Limit exceptions are thrown from the meter and caught only by the bridge, outside the DB::transaction callback. No inner code catches and continues.
    • If the canonical JSON byte measure cannot be produced (json_encode failure), the meter fails closed with reason: history_transaction_measure_unavailable, instead of falling back to serialize().

    Implementation starts once you accept or adjust this.

    🤖 Generated with Claude Code

  6. rmcdaniel commented on Oct 25, 2026

    @rmcdaniel
    MemberAuthor

    Security review of design v3: URI prediction, zero-put limit refusal, fail-closed custom drivers, and precharged write-site cardinality resolve the two original boundaries in principle. Three details must be added before implementation.

    1. Bound raw staged bytes, not only the eventual reference. The history-byte meter sees a short external-storage envelope, so many large arguments/results can otherwise pass while hashing, retaining, and uploading an unbounded transaction payload. Apply the existing per-payload hard limit to every raw externalizable command field before hashing, and add a checked transaction-wide hard limit for raw external-payload bytes (with documented zero/custom semantics). Charge before stage registration; define whether identical content is charged per occurrence for input-work protection even if object puts are deduplicated. Test exact/one-over aggregate bytes and prove refusal performs zero puts.
    2. Make retry/orphan behavior part of the contract. A stage must be new for every DB transaction callback attempt and discarded on rollback; deadlock/serialization retry must not retain entries or flush an earlier attempt. Built-in puts must be atomic, collision-safe, content-hash verifying and idempotent. A commit failure after flush may leave an unreachable object; bound that exposure by the stage-byte limit and document reachability-based cleanup/retention rather than deletion on rollback. Test partial flush failure, callback retry, commit failure, duplicate content, and predicted-URI mismatch.
    3. Bulk-row precharge must itself be bounded and cover implicit work. Do not materialize an unbounded ID set merely to decide it exceeds the row limit: read/count at most remaining+1 under the existing fence, then refuse or process. Charge matched statement cardinality before the statement, including database cascades/triggers and pivot/projection effects; assert the statement cannot affect more rows than charged. Disabled row limits need an independent existing pending-count bound or an explicit warning that this protection is disabled. The PR write-site table should identify every implicit cascade as well as PHP/Rust source calls.

    With those additions, v3 is accepted for implementation. Keep the meter/stage transaction-local, throw limit/measure failures outside inner catches, and preserve unchanged-lease reuse on refusal. No live or production action was performed.

  7. rmcdaniel commented on Oct 25, 2026

    @rmcdaniel
    MemberAuthor

    Write-site audit of DefaultWorkflowTaskBridge::complete() for the row and event accounting (read-only, against the package at b2a33691 plus my local, unpushed #930 draft). Bridge line numbers come from one snapshot, so the method names are the stable references.

    What changes in the #930 plan

    1. The fixed "at most 4 events" for other commands is wrong. Several commands and every terminal path write state-driven numbers of events:

      • the pending-signal closer on terminal events (≤5,000) and the pending-update closer (≤2 per accepted update, ≤500);
      • the continue-as-new message handoff (≤5,000 + 500);
      • ParentClosePolicyEnforcer, which cascades into open children recursively;
      • cancel_selection_operation (up to member_size leaves, each a full child cascade);
      • start_service_operation (≤10 handler attempts, each a nested start, signal or update);
      • parent notification (sibling events and up to group-size projections);
      • signal and condition resume (≤7 events before any command).

      The writer-level meter charges all of these as they're written. The normalizer's early check will count local activities exactly and every other command as one event, a lower bound, and will be described that way.

    2. checkpointCommands() (the checkpointLocalActivity* and checkpointCancellationScopePrefix entry points) runs signal resume and command application outside complete(). It gets the same meter.

    3. Deadline paths inside complete() (timeoutIfDeadlineExpired, cancelIfCleanupDeadlineExpired) do state-driven bulk work. Today it commits even when complete() answers completed: false. Under the meter, a run whose deadline close exceeds the budget would roll back on every completion attempt. Before choosing between metering them and leaving them to the unmetered watchdog close, I'll confirm the watchdog still closes such runs.

    4. Rows not tied to events:

      • memo and search-attribute inheritance (≤200 per child, retry or continue-as-new);
      • memo null-key deletes (one statement per key, no count limit);
      • CommandSequence backfill;
      • service-call row updates;
      • projection upserts that grow with the run (timeline per event, waits, timers, lineage), and observer-driven namespace backfills on links.

      The precharge table in the PR will list each, with its bound.

    Findings outside #930's scope (for Security triage)

    • start_service_operation passes worker-chosen namespace, caller_namespace, principal_* (roles and claims included) and service_call_id from the command into the service control plane. This may let a worker choose authorization context or namespace, or update another caller's service call.
    • In-transaction sleeps. start_service_operation retries sleep for their backoff inside the completion transaction, holding the task and run locks. A synchronous update wait polls and sleeps for a worker-chosen wait_timeout_seconds with no upper bound.
    • No pending-count limits for external workers. The guards for pending activities, timers and children run only on embedded executor paths. The bridge applies only the memo limit and reads the batch size, so state-driven cascades on the worker path are bounded only by what earlier completions built.
    • Exceptions caught inside the open transaction. projectRunBestEffort and the parent-close enforcer catch Throwable. The meter's tripped flag still refuses before commit, and the PR will include a test proving that.

    I'll take the #930 items in that order. The separate findings need owning issues.

    🤖 Generated with Claude Code

  8. rmcdaniel commented on Oct 26, 2026

    @rmcdaniel
    MemberAuthor

    Phase A is done on both sides.

    Found by the native acceptance matrix. A MySQL summary query that filesorted whole history rows ("Out of sort memory" for any event over 256 KiB) is fixed in the native Server.

    This issue stays open for Phase B: pre-write remaining + 1 accounting for the cascades and the other command paths, plus #933's pending-count bounds and a bounded recovery path for oversized legacy cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions