diff --git a/CHANGELOG.md b/CHANGELOG.md index 60256fb4..c63974f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,11 @@ ### Added +- **Safe persistence failure boundaries (PR #225).** Persistence stores expose fixed, cause-free failure text; raw paths, SQL, and payloads flow only to `PersistenceFailureDiagnosticObserver`; worker observers receive safe failures; existing exception and store ABI is preserved by the binary fixture. Epic 1.2 Safe Error Boundaries is complete. + - **Safe provider and built-in workflow-step failure boundaries (PRs #222, #223).** Provider HTTP rejections and built-in HTTP, shell, MCP, Codex, and Hermes workflow failures expose fixed cause-free public exceptions with typed failure codes. Original failure detail is retained only by an explicitly configured, fail-open diagnostic observer; public workflow events omit URLs, commands, raw tool names, and failure reasons. Existing public exception constructor descriptors remain compatible with 0.5.0 clients. -- **Safe tool-failure boundaries (PR #219).** Established the first slice of Epic 1.2: raw exception details no longer cross built-in model-visible tool boundaries, while original causes remain available to an explicitly configured fail-open `ToolFailureDiagnosticObserver`. Added diagnostic-only `ToolFailureCode` classifications (`tool.input.invalid`, `tool.execution.failed`, `tool.execution.retry_exhausted`) with fixed model-visible defaults; caller-visible failure mapping remains pending. `ToolResult` retains exactly its four 0.5.0 variants without deprecations, preserving exhaustive-`when` source compatibility, and adds `safeInvalidInput(...)`/`safePermanentFailure(...)` factories for validated or fixed text. `ModelVisibleToolMessage` is a regular class with a private constructor and validated `@JvmStatic trusted(...)` factory (non-blank, ≤512 chars, code-point-aware rejection of control, separator, and FORMAT characters), with no generated `copy` or destructuring bypass. `ToolInvalidInputException(String)` remains public with diagnostic-only text plus `withSafeModelMessage(...)`. The engine and standalone adapter never derive model-visible text from `Throwable.message`, classify diagnostics from their own control flow, emit fixed retry-exhaustion text, preserve cancellation, and treat diagnostic-sink failures as fail-open. The standalone builder still freezes the observer at `build()`. Custom tools written against the short-lived round-1 `SafeInvalidInput`/`SafePermanentFailure` variants must migrate to the safe factories or stable plain constructors. Workflow-step, persistence, MCP, shell, structured-output, and caller-visible tool boundaries remain later slices. +- **Safe tool-failure boundaries (PR #219).** Established the first slice of Epic 1.2: raw exception details no longer cross built-in model-visible tool boundaries, while original causes remain available to an explicitly configured fail-open `ToolFailureDiagnosticObserver`. Added diagnostic-only `ToolFailureCode` classifications (`tool.input.invalid`, `tool.execution.failed`, `tool.execution.retry_exhausted`) with fixed model-visible defaults; caller-visible failure mapping remains pending. `ToolResult` retains exactly its four 0.5.0 variants without deprecations, preserving exhaustive-`when` source compatibility, and adds `safeInvalidInput(...)`/`safePermanentFailure(...)` factories for validated or fixed text. `ModelVisibleToolMessage` is a regular class with a private constructor and validated `@JvmStatic trusted(...)` factory (non-blank, ≤512 chars, code-point-aware rejection of control, separator, and FORMAT characters), with no generated `copy` or destructuring bypass. `ToolInvalidInputException(String)` remains public with diagnostic-only text plus `withSafeModelMessage(...)`. The engine and standalone adapter never derive model-visible text from `Throwable.message`, classify diagnostics from their own control flow, emit fixed retry-exhaustion text, preserve cancellation, and treat diagnostic-sink failures as fail-open. The standalone builder still freezes the observer at `build()`. Custom tools written against the short-lived round-1 `SafeInvalidInput`/`SafePermanentFailure` variants must migrate to the safe factories or stable plain constructors. Caller-visible tool failure mapping and approval boundaries remain later slices. - **Durable file and JDBC step-attempt stores (PR #218).** Added independent `FileStepAttemptRecordStore` and `JdbcStepAttemptRecordStore` implementations with canonical schema-versioned encoding, SHA-256 record fingerprints, strict fail-closed decoding, exact attempt identity preservation, atomic compare-and-set, deterministic ordering, cancellable file/JDBC operations, and configurable validated JDBC identifiers. A shared 20-case TCK covers the in-memory, file, and JDBC contracts, while restart-level file/JDBC tests prove recovery approvals, approved idempotency keys, safe partial transitions, and retained failed-workflow evidence. Checkpoint and attempt stores remain explicitly composed and independently managed; this adds no cross-store transaction or exactly-once external-side-effect guarantee. diff --git a/docs/ROADMAP-0.6.0.md b/docs/ROADMAP-0.6.0.md index acfc838b..1050f4af 100644 --- a/docs/ROADMAP-0.6.0.md +++ b/docs/ROADMAP-0.6.0.md @@ -321,27 +321,27 @@ This phase is intentionally completed before large decomposition work. ## Epic 1.2: Safe error boundaries -> **Status:** in progress — tool failures (PR #219) and provider HTTP/transport failures (PR #222) are implemented. Workflow-step, persistence, MCP, shell, and structured-output boundaries remain. +> **Status:** ✅ Complete — tool failures (PR #219), provider HTTP/transport failures (PR #222), workflow-step/MCP/shell boundaries (PR #223), structured-output failures (PR #224), and persistence failures (PR #225) are implemented. **Goal:** Separate internal diagnostic detail from public, model-visible, audit-visible, and telemetry-visible errors. ### Tasks -1. Define `SafeFailureCode` or domain-specific typed reason-code families. — **PR #219:** `ToolFailureCode`; **PR #222:** `ProviderFailureCode`; **PR #223:** `WorkflowStepFailureCode` for built-in external workflow steps. Other domains remain later slices. +1. Define `SafeFailureCode` or domain-specific typed reason-code families. — **PR #219:** `ToolFailureCode`; **PR #222:** `ProviderFailureCode`; **PR #223:** `WorkflowStepFailureCode` for built-in external workflow steps; **PR #225:** `PersistenceFailureCode`. Approval and policy domains remain later slices. 2. Define four explicit error surfaces: - - internal cause; — **PR #219:** `ToolFailureDiagnosticObserver`; **PR #222:** `ProviderFailureDiagnosticObserver`; **PR #223:** `WorkflowStepFailureDiagnosticObserver` (all fail-open and diagnostic-only). - - public caller message; — **PR #222:** fixed provider HTTP/transport messages; **PR #223:** fixed built-in workflow-step messages. + - internal cause; — **PR #219:** `ToolFailureDiagnosticObserver`; **PR #222:** `ProviderFailureDiagnosticObserver`; **PR #223:** `WorkflowStepFailureDiagnosticObserver`; **PR #225:** `PersistenceFailureDiagnosticObserver` (all fail-open and diagnostic-only). + - public caller message; — **PR #222:** fixed provider HTTP/transport messages; **PR #223:** fixed built-in workflow-step messages; **PR #225:** fixed persistence messages. - model-visible message; — **PR #219:** `ModelVisibleToolMessage.trusted(...)`. - - audit/telemetry metadata. — **PR #222:** provider logs and telemetry expose fixed messages and trusted metadata only; other surfaces remain. + - audit/telemetry metadata. — **PR #222:** provider logs and telemetry expose fixed messages and trusted metadata only; **PR #225:** persistence raw detail remains diagnostic-observer-only while ordinary worker logs expose exception class names only. Approval surfaces remain. 3. Remove arbitrary exception messages from model-visible tool results. — **PR #219:** `ToolResult` retains its four 0.5.0 variants for exhaustive-`when` compatibility; `safeInvalidInput(...)`/`safePermanentFailure(...)` factories wrap validated or fixed text in the existing variants. Built-in engine and standalone paths never derive text from `Throwable.message`. 4. Review provider HTTP failure handling so response bodies are: - bounded; - sanitised; - disabled or redacted by default; - never copied wholesale into public exceptions. — **PR #222:** complete for provider HTTP failures; bounded previews are available only through `ProviderFailureDiagnosticObserver`. -5. Review debug logging of provider bodies and secret-related paths. — **PR #222:** complete for provider adapters; debug logs are metadata-only. -6. Centralize safe error sanitisation for shell, HTTP, MCP, tools, providers, persistence, and approvals. — **PR #222:** provider HTTP/transport helpers; **PR #223:** built-in HTTP, shell, MCP, Codex, and Hermes workflow-step helpers. Persistence and approvals remain later slices. -7. Add negative tests with tokens, prompts, paths, SQL fragments, command arguments, and malformed payloads. — **PR #219:** tool boundary; **PR #222:** provider HTTP, transport, adapter, and telemetry boundaries. Other surfaces remain. +5. Review debug logging of provider bodies and secret-related paths. — **PR #222:** complete for provider adapters; debug logs are metadata-only. **PR #225:** persistence worker logging emits exception class names only. +6. Centralize safe error sanitisation for shell, HTTP, MCP, tools, providers, persistence, and approvals. — **PR #222:** provider HTTP/transport helpers; **PR #223:** built-in HTTP, shell, MCP, Codex, and Hermes workflow-step helpers; **PR #225:** persistence boundaries. Approvals remain a later slice. +7. Add negative tests with tokens, prompts, paths, SQL fragments, command arguments, and malformed payloads. — **PR #219:** tool boundary; **PR #222:** provider HTTP, transport, adapter, and telemetry boundaries; **PR #224:** structured-output boundary; **PR #225:** persistence paths, SQL, payloads, observer behavior, cancellation, and worker failures. Approval surfaces remain. ### Acceptance criteria diff --git a/docs/concepts/safe-error-boundaries.md b/docs/concepts/safe-error-boundaries.md index c764956e..3c898767 100644 --- a/docs/concepts/safe-error-boundaries.md +++ b/docs/concepts/safe-error-boundaries.md @@ -1,6 +1,6 @@ # Safe Error Boundaries -> **Status:** implemented for tool execution (PR #219), provider HTTP/transport failures (PR #222), external workflow steps incl. MCP/shell (PR #223), and structured-output failures (PR #224). Persistence remains a future slice. +> **Status:** implemented for tool execution (PR #219), provider HTTP/transport failures (PR #222), external workflow steps incl. MCP/shell (PR #223), structured-output failures (PR #224), and persistence failures (PR #225). Epic 1.2 is complete. > **Structured-output note (PR #224):** engine-produced `StructuredOutputException` instances no longer populate `originalPrompt`, `lastRawResponse`, or `validationError` — those fields stay ABI-compatible but are intentionally null for built-in runtime failures. Raw detail flows only to `StructuredOutputFailureDiagnosticObserver`; `OperationObservation.onStructuredParseFailure` receives redacted/fixed text; OTel emits typed metadata only. @@ -27,7 +27,7 @@ Tool failures are classified with stable machine-readable codes — never by par | `EXECUTION_FAILED` | `tool.execution.failed` | Tool execution failed | | `RETRY_EXHAUSTED` | `tool.execution.retry_exhausted` | Tool execution failed | -Codes are domain-specific. `ToolFailureCode` covers tools, while `ProviderFailureCode` covers provider HTTP rejection, timeout, connection, transport, and unexpected failures. They classify diagnostic events and select fixed safe defaults; retry remains represented by the relevant domain contract rather than inferred from message text. Workflow, approval, persistence, and policy failures can gain their own code families without introducing a repository-wide "god enum". +Codes are domain-specific. `ToolFailureCode` covers tools, `ProviderFailureCode` covers provider HTTP rejection, timeout, connection, transport, and unexpected failures, and `PersistenceFailureCode` covers persistence failures. They classify diagnostic events and select fixed safe defaults; retry remains represented by the relevant domain contract rather than inferred from message text. Approval and policy failures can gain their own code families without introducing a repository-wide "god enum". ## Tool results @@ -92,6 +92,28 @@ Provider adapters use shared safe boundaries for HTTP and transport failures: Retry behavior remains structural: `statusCode`, `retryable`, and `retryAfterMillis` survive HTTP mapping, while transport categories select their established retryability. Cancellation input is rethrown before classification. Observer-thrown cancellation is swallowed only while the current coroutine remains active; cancellation of the enclosing job remains primary. -## Scope and non-claims (PRs #219 and #222) +## Persistence failures -These slices do not complete Epic 1.2. Built-in HTTP, shell, MCP, Codex, and Hermes workflow steps now use the same safe-boundary shape; persistence and structured-output exception fields remain outside this slice. The work does not add automatic secret detection, guarantee application-supplied trusted messages are secret-free, introduce a universal failure-code taxonomy, or change tool retry/idempotency semantics. +Persistence uses typed context rather than exception text: `PersistenceFailureCode` classifies read, write, delete, list, conflict, and corrupted-data failures; `PersistenceResourceKind` identifies checkpoints, leases, step attempts, or the worker registry; and `PersistenceOperation` identifies the affected store operation. + +`PersistenceFailureDiagnosticObserver` receives a `PersistenceFailureDiagnosticEvent` containing that typed context and the original failure. It is diagnostic-only and fail-open: observer failures do not replace the safe persistence failure. Cancellation remains primary; genuine coroutine cancellation is rethrown and is not delivered as an ordinary diagnostic failure. + +Built-in public failures use fixed text only: + +- `Workflow checkpoint conflict`; +- `Workflow lease conflict`; +- `Persisted workflow checkpoint is invalid`; +- `Persisted step-attempt record is invalid`; +- `Workflow persistence read/write/delete/list failed`. + +File, markdown, JDBC, and in-memory checkpoint stores; file and JDBC lease stores; and file and JDBC step-attempt stores wrap every persistence operation in `persistenceBoundary`. Raw paths, SQL, and persisted payloads reach only the diagnostic observer. `StaleWorkflowLeaseException` remains a semantic class for worker fencing, with the fixed text `Workflow lease is no longer active`. + +Ordinary worker callbacks (`onPollFailed`, `onLeaseRenewalFailed`, `onLeaseReleaseFailed`, and `onStepAttemptFailed`) receive safe failures rather than raw persistence exceptions. `LoggingTramaiWorkerObserver` logs their exception class names only. + +The persistence exception constructors remain ABI-compatible. Class-body `failureCode` and `safeFactoryTrusted` properties and additive observer-taking store constructors extend the API without changing those constructor descriptors. The binary fixture exercises v0.5.0 persistence exception and store constructors. + +This boundary does not add cross-store transactions, schema changes, encryption, retries, lease-semantic changes, or cancellation-semantic changes. + +## Scope and non-claims (PRs #219–#225) + +These slices complete Epic 1.2. Built-in HTTP, shell, MCP, Codex, and Hermes workflow steps, structured-output failures, and persistence failures now use the same safe-boundary shape; approval-gateway exception fields remain outside this slice. The work does not add automatic secret detection, guarantee application-supplied trusted messages are secret-free, introduce a universal failure-code taxonomy, or change tool retry/idempotency semantics. diff --git a/docs/guides/orchestration-persistence.md b/docs/guides/orchestration-persistence.md index d69dedb0..ff28f877 100644 --- a/docs/guides/orchestration-persistence.md +++ b/docs/guides/orchestration-persistence.md @@ -108,6 +108,29 @@ The important contract detail is revision handling: That gives database, object-store, and filesystem implementations the same optimistic-concurrency model. +## Safe Failure Boundaries + +Built-in persistence stores expose fixed-text public failures. They do not copy filesystem paths, SQL, persisted payloads, or arbitrary storage exception messages into public exceptions, ordinary worker callbacks, or default logs. The original failure is available only through `PersistenceFailureDiagnosticObserver`. + +Configure the observer with a built-in store's additive observer constructor overload: + +```kotlin +val checkpointStore = FileWorkflowCheckpointStore( + Path.of(".tramai/workflows"), + DefaultWorkflowCheckpointPathStrategy("checkpoint.properties"), + PersistenceFailureDiagnosticObserver { event -> + // Route diagnostic-only event.failure to an access-controlled sink. + diagnosticSink.record(event.resourceKind, event.operation, event.failureCode, event.failure) + }, +) +``` + +The observer is fail-open: an observer failure does not replace the persistence failure. Cancellation remains first: genuine coroutine cancellation propagates rather than becoming a persistence diagnostic. This is an API-level constructor setting; no YAML or configuration property is required. + +Checkpoint conflicts, lease conflicts, invalid checkpoints, invalid step-attempt records, and general read/write/delete/list failures have fixed public text. `StaleWorkflowLeaseException` remains the semantic fencing signal with fixed text. In a worker, `onPollFailed`, `onLeaseRenewalFailed`, `onLeaseReleaseFailed`, and `onStepAttemptFailed` receive safe failures; `LoggingTramaiWorkerObserver` logs only their exception class names. + +The built-in file, markdown, JDBC, and in-memory checkpoint stores; file and JDBC lease stores; and file and JDBC step-attempt stores apply this boundary to every persistence operation. Custom stores should preserve the same separation when they handle raw backend errors. + ## Wiring A Workflow Once you have a codec and store, attach them through `WorkflowPersistence`: diff --git a/docs/releases/0.6.0-characterization-matrix.md b/docs/releases/0.6.0-characterization-matrix.md index b8e6ed5a..1777e747 100644 --- a/docs/releases/0.6.0-characterization-matrix.md +++ b/docs/releases/0.6.0-characterization-matrix.md @@ -10,13 +10,13 @@ | Provider Routing | Provider selection by model name | TBD — characterization pending | TBD — characterization pending | 0.6.1 | | Provider Routing | Provider fallback on failure | `EngineCancellationContractTest.provider cancellation bypasses retry and fallback without normal failure classification` characterizes cancellation-fallback interaction. Ordinary failure fallback remains covered separately. | TBD — characterization pending | 0.6.1 | | Provider Routing | Provider timeout handling | TBD — characterization pending | TBD — characterization pending | 0.6.1 | -| Provider Failures | Core HTTP and transport boundary | `ProviderFailuresTest` proves fixed public messages, typed classification, retry metadata, byte-bounded `InputStream` acquisition with an 8 KiB + sentinel read ceiling, diagnostic-only causes, trusted-factory pass-through, re-sanitisation of arbitrary `ProviderException` values, fail-open observers, and genuine cancellation propagation. `JavaProviderExceptionCompatibilityTest` covers Java getters and the legacy constructor; `BinaryCompatibilityFixtureTest` executes calls compiled against the 0.5.0 jar, including Kotlin default-argument constructors and all three legacy helper descriptors. | Workflow-step, persistence, MCP, shell, and structured-output failure boundaries remain. | 0.6.1 | +| Provider Failures | Core HTTP and transport boundary | `ProviderFailuresTest` proves fixed public messages, typed classification, retry metadata, byte-bounded `InputStream` acquisition with an 8 KiB + sentinel read ceiling, diagnostic-only causes, trusted-factory pass-through, re-sanitisation of arbitrary `ProviderException` values, fail-open observers, and genuine cancellation propagation. `JavaProviderExceptionCompatibilityTest` covers Java getters and the legacy constructor; `BinaryCompatibilityFixtureTest` executes calls compiled against the 0.5.0 jar, including Kotlin default-argument constructors and all three legacy helper descriptors. | Approval safe-failure boundaries remain. | 0.6.1 | | Provider Failures | Adapter HTTP and transport integration | `OpenAiProviderTest`, `AnthropicProviderTest`, `GeminiProviderTest`, and `OllamaProviderTest` prove secret-bearing HTTP bodies stay out of public exceptions while one bounded diagnostic event is emitted. Streaming tests in `OpenAiStreamingTest`, `AzureOpenAiProviderTest`, `AnthropicProviderTest`, `GeminiProviderTest`, and `OllamaProviderTest` prove request/send failures terminate as one cause-free safe error while the diagnostic observer receives the transport detail. `OpenAiProviderTest` additionally covers re-sanitisation of an untrusted token-source `ProviderException`. | Equivalent observer/telemetry assertions for every provider-specific harness remain pending. | 0.6.1 | -| Observability | Provider failure telemetry boundary | `OpenTelemetryOperationObserverTest.provider failure telemetry excludes transport diagnostics` proves span attributes, event attributes, exception messages, and status descriptions contain only the fixed provider failure message, not the original transport detail. | Workflow-step, MCP, shell, persistence, and structured-output telemetry boundaries remain. | 0.6.1 | +| Observability | Provider failure telemetry boundary | `OpenTelemetryOperationObserverTest.provider failure telemetry excludes transport diagnostics` proves span attributes, event attributes, exception messages, and status descriptions contain only the fixed provider failure message, not the original transport detail. | Workflow-step, MCP, and shell telemetry boundaries remain. | 0.6.1 | | Tools | Tool exposure via @AiTool | TBD — characterization pending | TBD — characterization pending | 0.6.1 | | Tools | Tool execution policy (allow/deny) | TBD — characterization pending | TBD — characterization pending | 0.6.1 | -| Tools | Tool cancellation handling | `ToolCancellationContractTest.idempotent tool cancellation escapes without retry or reinjection` and `ToolCancellationContractTest.custom resolved tool transient failure wrapping cancellation is rethrown` characterize cancellation during tool execution. `OpenTelemetryCancellationIntegrationTest.tool cancellation is attributed as cancelled in spans and metrics` covers OTel attribution. | None — issue #210 is resolved. Orchestration, persistence, process, and subprocess cancellation remain uncharacterized. | 0.6.1 | -| Tools | Tool failure boundaries | `ToolSafeFailureContractTest` proves sensitive fixtures (tokens, paths, SQL fragments, prompts) never reach provider requests, terminal tool messages, or serialized engine events; `ToolSafeFailureAdapterTest` proves the same through the standalone adapter. Built-in HTTP, shell, MCP, Codex, and Hermes workflow steps use typed, observer-only safe failure boundaries. | Caller-visible tool failure mapping and structured-output boundaries remain uncharacterized. | 0.6.1 | +| Tools | Tool cancellation handling | `ToolCancellationContractTest.idempotent tool cancellation escapes without retry or reinjection` and `ToolCancellationContractTest.custom resolved tool transient failure wrapping cancellation is rethrown` characterize cancellation during tool execution. `OpenTelemetryCancellationIntegrationTest.tool cancellation is attributed as cancelled in spans and metrics` covers OTel attribution. | None — issue #210 is resolved; orchestration, persistence, process, and subprocess cancellation are characterized separately. | 0.6.1 | +| Tools | Tool failure boundaries | `ToolSafeFailureContractTest` proves sensitive fixtures (tokens, paths, SQL fragments, prompts) never reach provider requests, terminal tool messages, or serialized engine events; `ToolSafeFailureAdapterTest` proves the same through the standalone adapter. Built-in HTTP, shell, MCP, Codex, and Hermes workflow steps use typed, observer-only safe failure boundaries. | Caller-visible tool failure mapping remains uncharacterized. | 0.6.1 | | Tools | Tool invocation audit evidence | TBD — characterization pending | TBD — characterization pending | 0.6.1 | | Approval | Approval creation flow | TBD — characterization pending | TBD — characterization pending | 0.6.1 | | Approval | Approval resume after denial | TBD — characterization pending | TBD — characterization pending | 0.6.1 | @@ -29,9 +29,10 @@ | Workflow | Durable recovery across restart | `DurableWorkflowRecoveryFileTest` and `DurableWorkflowRecoveryJdbcTest` cover retry approval consumption, exact approved-key matching and mismatch voiding, partial approval/clear transitions, and retained `WORKFLOW_FAILED` evidence after checkpoint deletion. | No cross-store transaction or exactly-once external-side-effect guarantee. | 0.6.1 | | Workflow | Structured output contract validation | TBD — characterization pending | TBD — characterization pending | 0.6.1 | | Workflow | Retry policy and classification | `EngineCancellationContractTest` also proves cancellation bypasses provider retry (providerRetries=2, provider called exactly once). | TBD — characterization pending | 0.6.1 | -| Workflow | Cancellation propagation | `CancellationContractTest` covers `rethrowIfCancellation` helper semantics. `EngineCancellationContractTest` covers retry bypass, fallback bypass, structured-output repair stop, streaming fallback bypass, and observer failure suppression. `ToolCancellationContractTest` covers tool-execution cancellation. `OpenTelemetryCancellationIntegrationTest` covers OTel cancellation attribution. `WorkflowCancellationContractTest` covers direct execution, resume, and parallel-branch cancellation. `TramaiWorkerCancellationContractTest` covers worker drain, observer-abandon, poll, and lease-renewal cancellation. `FileWorkflowPersistenceCancellationContractTest` covers file-backed checkpoint and lease cancellation. `JdbcWorkflowPersistenceCancellationContractTest` covers JDBC-backed checkpoint and lease cancellation (10 scenarios). | Cross-process OS-lock cancellation, process execution, and subprocess termination remain uncharacterized. | 0.6.1 | -| Persistence | File-based persistence | `FileWorkflowPersistenceCancellationContractTest` covers 11 scenarios: pre-cancelled save, same-JVM mutex-wait cancellation, atomic-write preservation (through real production writer), claim/renewal/fenced-save cancellation, conflict regression, resource cleanup, registry lifecycle, and no-deadlock fenced save. | Cross-process OS-level FileChannel.lock() cancellation and JDBC-backed persistence cancellation remain uncharacterized. | 0.6.1 | +| Workflow | Cancellation propagation | `CancellationContractTest` covers `rethrowIfCancellation` helper semantics. `EngineCancellationContractTest` covers retry bypass, fallback bypass, structured-output repair stop, streaming fallback bypass, and observer failure suppression. `ToolCancellationContractTest` covers tool-execution cancellation. `OpenTelemetryCancellationIntegrationTest` covers OTel cancellation attribution. `WorkflowCancellationContractTest` covers direct execution, resume, and parallel-branch cancellation. `TramaiWorkerCancellationContractTest` covers worker drain, observer-abandon, poll, and lease-renewal cancellation. `FileWorkflowPersistenceCancellationContractTest` covers file-backed checkpoint and lease cancellation. `JdbcWorkflowPersistenceCancellationContractTest` covers JDBC-backed checkpoint and lease cancellation (10 scenarios). `SubprocessCancellationContractTest` and the cross-process OS-lock contract suite cover the remaining process and OS-lock paths. | None — cancellation propagation is characterized by the listed contract suites. | 0.6.1 | +| Persistence | File-based persistence | `FileWorkflowPersistenceCancellationContractTest` covers 11 scenarios: pre-cancelled save, same-JVM mutex-wait cancellation, atomic-write preservation (through real production writer), claim/renewal/fenced-save cancellation, conflict regression, resource cleanup, registry lifecycle, and no-deadlock fenced save. | None — file persistence cancellation is characterized by the above test class and the cross-process OS-lock contract suite. | 0.6.1 | | Persistence | JDBC persistence with optimistic locking | `JdbcWorkflowPersistenceCancellationContractTest` covers 10 scenarios: pre-cancelled operation, cancellation during connection acquisition, executeQuery cancellation, executeUpdate cancellation, fenced save rollback, pre-commit cancellation rollback, rollback failure suppression, cancelled lease claim, successful-operation regression, and SQL-failure regression (conflict detection, revision checks). | None — JDBC persistence cancellation is characterized by the above test class. | 0.6.2 | +| Persistence | Safe-failure boundaries | `PersistenceSafeFailureBoundaryTest` proves raw SQL, paths, and payloads reach only the diagnostic observer; public messages are fixed; untrusted domain exceptions are re-sanitized; observers fail open; cancellation remains first; conflicts preserve their semantic classes; and worker observers receive safe failures. | None — PR #225. | 0.6.1 | | Persistence | Replay and version checks | TBD — characterization pending | TBD — characterization pending | 0.6.2 | | Security | Policy enforcement boundaries | TBD — characterization pending | TBD — characterization pending | 0.6.1 | diff --git a/tramai-orchestration/api/tramai-orchestration.api b/tramai-orchestration/api/tramai-orchestration.api index d9ab9af8..0e5d9115 100644 --- a/tramai-orchestration/api/tramai-orchestration.api +++ b/tramai-orchestration/api/tramai-orchestration.api @@ -149,7 +149,9 @@ public abstract interface class dev/tramai/orchestration/ExternalStepExecutorRes public final class dev/tramai/orchestration/FileStepAttemptRecordStore : dev/tramai/orchestration/StepAttemptRecordStore { public fun (Ljava/nio/file/Path;)V + public fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver;)V public fun compareAndSetStepAttempt (Ldev/tramai/orchestration/StepAttemptRecord;Ldev/tramai/orchestration/StepAttemptRecord;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public final fun getPersistenceFailureDiagnosticObserver ()Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver; public fun latestStepAttempt (Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun listStepAttempts (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun recordStepAttempt (Ldev/tramai/orchestration/StepAttemptRecord;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; @@ -160,8 +162,10 @@ public final class dev/tramai/orchestration/FileWorkflowCheckpointStore : dev/tr public fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;)V public synthetic fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public synthetic fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;Ldev/tramai/orchestration/AtomicFileWriter;Lkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver;)V public fun clearRecovery (Ljava/lang/String;Ljava/lang/String;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun delete (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public final fun getPersistenceFailureDiagnosticObserver ()Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver; public fun listCheckpoints (Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun load (Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun requireRecovery (Ljava/lang/String;Ljava/lang/String;JLdev/tramai/orchestration/WorkflowRecoveryRecord;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; @@ -172,9 +176,11 @@ public final class dev/tramai/orchestration/FileWorkflowLeaseStore : dev/tramai/ public fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;Lkotlin/jvm/functions/Function0;)V public synthetic fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;Lkotlin/jvm/functions/Function0;ILkotlin/jvm/internal/DefaultConstructorMarker;)V public synthetic fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;Lkotlin/jvm/functions/Function0;Ldev/tramai/orchestration/AtomicFileWriter;Lkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;Lkotlin/jvm/functions/Function0;Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver;)V public fun claim (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun currentLease (Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun deleteCheckpointIfLeaseOwner (Ldev/tramai/orchestration/WorkflowCheckpointStore;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Ldev/tramai/orchestration/WorkflowLease;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public final fun getPersistenceFailureDiagnosticObserver ()Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver; public fun release (Ldev/tramai/orchestration/WorkflowLease;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun renew (Ldev/tramai/orchestration/WorkflowLease;Ljava/lang/Long;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun saveCheckpointIfLeaseOwner (Ldev/tramai/orchestration/WorkflowCheckpointStore;Ldev/tramai/orchestration/WorkflowCheckpoint;Ljava/lang/Long;Ldev/tramai/orchestration/WorkflowLease;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; @@ -277,9 +283,11 @@ public final class dev/tramai/orchestration/HttpStepConfig { public final class dev/tramai/orchestration/InMemoryWorkflowCheckpointStore : dev/tramai/orchestration/StepAttemptRecordStore, dev/tramai/orchestration/WorkflowCheckpointCatalog, dev/tramai/orchestration/WorkflowCheckpointStore { public fun ()V + public fun (Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver;)V public fun clearRecovery (Ljava/lang/String;Ljava/lang/String;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun compareAndSetStepAttempt (Ldev/tramai/orchestration/StepAttemptRecord;Ldev/tramai/orchestration/StepAttemptRecord;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun delete (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public final fun getPersistenceFailureDiagnosticObserver ()Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver; public fun latestStepAttempt (Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun listCheckpoints (Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun listStepAttempts (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; @@ -294,9 +302,11 @@ public final class dev/tramai/orchestration/InMemoryWorkflowLeaseStore : dev/tra public fun ()V public fun (Lkotlin/jvm/functions/Function0;)V public synthetic fun (Lkotlin/jvm/functions/Function0;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Lkotlin/jvm/functions/Function0;Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver;)V public fun claim (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun currentLease (Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun deleteCheckpointIfLeaseOwner (Ldev/tramai/orchestration/WorkflowCheckpointStore;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Ldev/tramai/orchestration/WorkflowLease;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public final fun getPersistenceFailureDiagnosticObserver ()Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver; public fun listActiveWorkers (Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun listStaleWorkers (JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun registerWorker (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/util/Set;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; @@ -318,8 +328,10 @@ public final class dev/tramai/orchestration/InMemoryWorkflowRecoveryController : public final class dev/tramai/orchestration/JdbcStepAttemptRecordStore : dev/tramai/orchestration/StepAttemptRecordStore { public fun (Ljavax/sql/DataSource;Ldev/tramai/orchestration/JdbcStepAttemptTable;)V public synthetic fun (Ljavax/sql/DataSource;Ldev/tramai/orchestration/JdbcStepAttemptTable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Ljavax/sql/DataSource;Ldev/tramai/orchestration/JdbcStepAttemptTable;Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver;)V public fun compareAndSetStepAttempt (Ldev/tramai/orchestration/StepAttemptRecord;Ldev/tramai/orchestration/StepAttemptRecord;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public final fun createTableSql ()Ljava/lang/String; + public final fun getPersistenceFailureDiagnosticObserver ()Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver; public fun latestStepAttempt (Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun listStepAttempts (Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun recordStepAttempt (Ldev/tramai/orchestration/StepAttemptRecord;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; @@ -378,9 +390,11 @@ public final class dev/tramai/orchestration/JdbcStepAttemptTable { public final class dev/tramai/orchestration/JdbcWorkflowCheckpointStore : dev/tramai/orchestration/WorkflowCheckpointCatalog, dev/tramai/orchestration/WorkflowCheckpointStore { public fun (Ljavax/sql/DataSource;Ldev/tramai/orchestration/JdbcWorkflowCheckpointTable;)V public synthetic fun (Ljavax/sql/DataSource;Ldev/tramai/orchestration/JdbcWorkflowCheckpointTable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Ljavax/sql/DataSource;Ldev/tramai/orchestration/JdbcWorkflowCheckpointTable;Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver;)V public fun clearRecovery (Ljava/lang/String;Ljava/lang/String;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public final fun createTableSql ()Ljava/lang/String; public fun delete (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public final fun getPersistenceFailureDiagnosticObserver ()Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver; public fun listCheckpoints (Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun load (Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public final fun migrationSql ()Ljava/lang/String; @@ -424,10 +438,12 @@ public final class dev/tramai/orchestration/JdbcWorkflowCheckpointTable { public final class dev/tramai/orchestration/JdbcWorkflowLeaseStore : dev/tramai/orchestration/WorkflowLeaseCheckpointFence, dev/tramai/orchestration/WorkflowLeaseStore { public fun (Ljavax/sql/DataSource;Ldev/tramai/orchestration/JdbcWorkflowLeaseTable;Lkotlin/jvm/functions/Function0;)V public synthetic fun (Ljavax/sql/DataSource;Ldev/tramai/orchestration/JdbcWorkflowLeaseTable;Lkotlin/jvm/functions/Function0;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Ljavax/sql/DataSource;Ldev/tramai/orchestration/JdbcWorkflowLeaseTable;Lkotlin/jvm/functions/Function0;Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver;)V public fun claim (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public final fun createTableSql ()Ljava/lang/String; public fun currentLease (Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun deleteCheckpointIfLeaseOwner (Ldev/tramai/orchestration/WorkflowCheckpointStore;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Ldev/tramai/orchestration/WorkflowLease;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public final fun getPersistenceFailureDiagnosticObserver ()Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver; public fun release (Ldev/tramai/orchestration/WorkflowLease;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun renew (Ldev/tramai/orchestration/WorkflowLease;Ljava/lang/Long;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun saveCheckpointIfLeaseOwner (Ldev/tramai/orchestration/WorkflowCheckpointStore;Ldev/tramai/orchestration/WorkflowCheckpoint;Ljava/lang/Long;Ldev/tramai/orchestration/WorkflowLease;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; @@ -493,8 +509,10 @@ public final class dev/tramai/orchestration/LoggingTramaiWorkerObserver$Companio public final class dev/tramai/orchestration/MarkdownWorkflowCheckpointStore : dev/tramai/orchestration/WorkflowCheckpointStore { public fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;)V public synthetic fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public fun (Ljava/nio/file/Path;Ldev/tramai/orchestration/WorkflowCheckpointPathStrategy;Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver;)V public fun clearRecovery (Ljava/lang/String;Ljava/lang/String;JLkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun delete (Ljava/lang/String;Ljava/lang/String;Ljava/lang/Long;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; + public final fun getPersistenceFailureDiagnosticObserver ()Ldev/tramai/orchestration/PersistenceFailureDiagnosticObserver; public fun load (Ljava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun requireRecovery (Ljava/lang/String;Ljava/lang/String;JLdev/tramai/orchestration/WorkflowRecoveryRecord;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public fun save (Ldev/tramai/orchestration/WorkflowCheckpoint;Ljava/lang/Long;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; @@ -612,6 +630,11 @@ public final class dev/tramai/orchestration/NoOpExternalStepExecutorResolver : d public fun registeredTypeIds ()Ljava/util/Set; } +public final class dev/tramai/orchestration/NoOpPersistenceFailureDiagnosticObserver : dev/tramai/orchestration/PersistenceFailureDiagnosticObserver { + public static final field INSTANCE Ldev/tramai/orchestration/NoOpPersistenceFailureDiagnosticObserver; + public fun onFailure (Ldev/tramai/orchestration/PersistenceFailureDiagnosticEvent;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; +} + public final class dev/tramai/orchestration/NoOpTramaiWorkerObserver : dev/tramai/orchestration/TramaiWorkerObserver { public static final field INSTANCE Ldev/tramai/orchestration/NoOpTramaiWorkerObserver; public fun onDrainProgress (Ljava/lang/String;II)V @@ -669,6 +692,67 @@ public abstract interface class dev/tramai/orchestration/PartitionAssignmentStra public abstract fun ownsPartition (Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; } +public abstract interface class dev/tramai/orchestration/PersistenceCorruptionDetail { + public abstract fun getCorruptPayload ()Ljava/lang/String; +} + +public final class dev/tramai/orchestration/PersistenceFailureCode : java/lang/Enum { + public static final field CONFLICT Ldev/tramai/orchestration/PersistenceFailureCode; + public static final field CORRUPTED_DATA Ldev/tramai/orchestration/PersistenceFailureCode; + public static final field DELETE_FAILED Ldev/tramai/orchestration/PersistenceFailureCode; + public static final field LIST_FAILED Ldev/tramai/orchestration/PersistenceFailureCode; + public static final field READ_FAILED Ldev/tramai/orchestration/PersistenceFailureCode; + public static final field WRITE_FAILED Ldev/tramai/orchestration/PersistenceFailureCode; + public static fun getEntries ()Lkotlin/enums/EnumEntries; + public static fun valueOf (Ljava/lang/String;)Ldev/tramai/orchestration/PersistenceFailureCode; + public static fun values ()[Ldev/tramai/orchestration/PersistenceFailureCode; +} + +public final class dev/tramai/orchestration/PersistenceFailureDiagnosticEvent { + public fun (Ldev/tramai/orchestration/PersistenceResourceKind;Ldev/tramai/orchestration/PersistenceOperation;Ldev/tramai/orchestration/PersistenceFailureCode;Ljava/lang/Throwable;)V + public final fun component1 ()Ldev/tramai/orchestration/PersistenceResourceKind; + public final fun component2 ()Ldev/tramai/orchestration/PersistenceOperation; + public final fun component3 ()Ldev/tramai/orchestration/PersistenceFailureCode; + public final fun component4 ()Ljava/lang/Throwable; + public final fun copy (Ldev/tramai/orchestration/PersistenceResourceKind;Ldev/tramai/orchestration/PersistenceOperation;Ldev/tramai/orchestration/PersistenceFailureCode;Ljava/lang/Throwable;)Ldev/tramai/orchestration/PersistenceFailureDiagnosticEvent; + public static synthetic fun copy$default (Ldev/tramai/orchestration/PersistenceFailureDiagnosticEvent;Ldev/tramai/orchestration/PersistenceResourceKind;Ldev/tramai/orchestration/PersistenceOperation;Ldev/tramai/orchestration/PersistenceFailureCode;Ljava/lang/Throwable;ILjava/lang/Object;)Ldev/tramai/orchestration/PersistenceFailureDiagnosticEvent; + public fun equals (Ljava/lang/Object;)Z + public final fun getFailure ()Ljava/lang/Throwable; + public final fun getFailureCode ()Ldev/tramai/orchestration/PersistenceFailureCode; + public final fun getOperation ()Ldev/tramai/orchestration/PersistenceOperation; + public final fun getResourceKind ()Ldev/tramai/orchestration/PersistenceResourceKind; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + +public abstract interface class dev/tramai/orchestration/PersistenceFailureDiagnosticObserver { + public abstract fun onFailure (Ldev/tramai/orchestration/PersistenceFailureDiagnosticEvent;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; +} + +public final class dev/tramai/orchestration/PersistenceOperation : java/lang/Enum { + public static final field CLAIM Ldev/tramai/orchestration/PersistenceOperation; + public static final field COMPARE_AND_SET Ldev/tramai/orchestration/PersistenceOperation; + public static final field DELETE Ldev/tramai/orchestration/PersistenceOperation; + public static final field LIST Ldev/tramai/orchestration/PersistenceOperation; + public static final field LOAD Ldev/tramai/orchestration/PersistenceOperation; + public static final field RELEASE Ldev/tramai/orchestration/PersistenceOperation; + public static final field RENEW Ldev/tramai/orchestration/PersistenceOperation; + public static final field SAVE Ldev/tramai/orchestration/PersistenceOperation; + public static fun getEntries ()Lkotlin/enums/EnumEntries; + public static fun valueOf (Ljava/lang/String;)Ldev/tramai/orchestration/PersistenceOperation; + public static fun values ()[Ldev/tramai/orchestration/PersistenceOperation; +} + +public final class dev/tramai/orchestration/PersistenceResourceKind : java/lang/Enum { + public static final field CHECKPOINT Ldev/tramai/orchestration/PersistenceResourceKind; + public static final field LEASE Ldev/tramai/orchestration/PersistenceResourceKind; + public static final field STEP_ATTEMPT Ldev/tramai/orchestration/PersistenceResourceKind; + public static final field WORKER_REGISTRY Ldev/tramai/orchestration/PersistenceResourceKind; + public static fun getEntries ()Lkotlin/enums/EnumEntries; + public static fun valueOf (Ljava/lang/String;)Ldev/tramai/orchestration/PersistenceResourceKind; + public static fun values ()[Ldev/tramai/orchestration/PersistenceResourceKind; +} + public final class dev/tramai/orchestration/ReplayPolicy : java/lang/Enum { public static final field EXTERNALLY_IDEMPOTENT Ldev/tramai/orchestration/ReplayPolicy; public static final field IDEMPOTENT Ldev/tramai/orchestration/ReplayPolicy; @@ -756,6 +840,8 @@ public final class dev/tramai/orchestration/ShellStepConfig { public final class dev/tramai/orchestration/StaleWorkflowLeaseException : java/lang/RuntimeException { public fun (Ljava/lang/String;)V + public final fun getFailureCode ()Ldev/tramai/orchestration/PersistenceFailureCode; + public final fun getSafeFactoryTrusted ()Z } public final class dev/tramai/orchestration/StepAttemptRecord { @@ -803,6 +889,8 @@ public final class dev/tramai/orchestration/StepAttemptRecord { public final class dev/tramai/orchestration/StepAttemptRecordCorruptionException : java/lang/RuntimeException { public fun (Ljava/lang/String;Ljava/lang/Throwable;)V public synthetic fun (Ljava/lang/String;Ljava/lang/Throwable;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun getFailureCode ()Ldev/tramai/orchestration/PersistenceFailureCode; + public final fun getSafeFactoryTrusted ()Z } public abstract interface class dev/tramai/orchestration/StepAttemptRecordStore { @@ -1030,10 +1118,14 @@ public abstract interface class dev/tramai/orchestration/WorkflowCheckpointCatal public final class dev/tramai/orchestration/WorkflowCheckpointConflictException : java/lang/RuntimeException { public fun (Ljava/lang/String;)V + public final fun getFailureCode ()Ldev/tramai/orchestration/PersistenceFailureCode; + public final fun getSafeFactoryTrusted ()Z } public final class dev/tramai/orchestration/WorkflowCheckpointCorruptionException : java/lang/RuntimeException { public fun (Ljava/lang/String;)V + public final fun getFailureCode ()Ldev/tramai/orchestration/PersistenceFailureCode; + public final fun getSafeFactoryTrusted ()Z } public abstract interface class dev/tramai/orchestration/WorkflowCheckpointPathStrategy { @@ -1138,6 +1230,8 @@ public abstract interface class dev/tramai/orchestration/WorkflowLeaseCheckpoint public final class dev/tramai/orchestration/WorkflowLeaseConflictException : java/lang/RuntimeException { public fun (Ljava/lang/String;)V + public final fun getFailureCode ()Ldev/tramai/orchestration/PersistenceFailureCode; + public final fun getSafeFactoryTrusted ()Z } public final class dev/tramai/orchestration/WorkflowLeasePolicy { @@ -1222,6 +1316,12 @@ public final class dev/tramai/orchestration/WorkflowPersistence { public fun toString ()Ljava/lang/String; } +public final class dev/tramai/orchestration/WorkflowPersistenceFailureException : java/lang/RuntimeException { + public fun (Ljava/lang/String;)V + public final fun getFailureCode ()Ldev/tramai/orchestration/PersistenceFailureCode; + public final fun getSafeFactoryTrusted ()Z +} + public abstract interface class dev/tramai/orchestration/WorkflowRecoveryController { public abstract fun failWorkflow (Ljava/lang/String;Ljava/lang/String;JLjava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public abstract fun retryStep (Ljava/lang/String;Ljava/lang/String;JLjava/lang/String;Ljava/lang/String;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; @@ -1288,6 +1388,8 @@ public final class dev/tramai/orchestration/WorkflowRecoveryStateException : jav public final class dev/tramai/orchestration/WorkflowResumeException : java/lang/RuntimeException { public fun (Ljava/lang/String;)V + public final fun getFailureCode ()Ldev/tramai/orchestration/PersistenceFailureCode; + public final fun getSafeFactoryTrusted ()Z } public abstract interface class dev/tramai/orchestration/WorkflowScheduleDefinition { diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileStepAttemptRecordStore.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileStepAttemptRecordStore.kt index 2cf48c6b..f8c130f3 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileStepAttemptRecordStore.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileStepAttemptRecordStore.kt @@ -12,37 +12,62 @@ class FileStepAttemptRecordStore internal constructor( private val rootDirectory: Path, private val atomicWriter: AtomicFileWriter, ) : StepAttemptRecordStore { + var persistenceFailureDiagnosticObserver: PersistenceFailureDiagnosticObserver = + NoOpPersistenceFailureDiagnosticObserver + internal set + constructor(rootDirectory: Path) : this(rootDirectory, realAtomicFileWriter) + constructor(rootDirectory: Path, observer: PersistenceFailureDiagnosticObserver) : + this(rootDirectory, realAtomicFileWriter) { + persistenceFailureDiagnosticObserver = observer + } + internal companion object { private const val ATTEMPT_SUFFIX = ".attempt.properties" private const val RECORD_HASH = "record_hash" fun forTest(rootDirectory: Path, atomicWriter: AtomicFileWriter): FileStepAttemptRecordStore = FileStepAttemptRecordStore(rootDirectory, atomicWriter) + + fun forTest( + rootDirectory: Path, + atomicWriter: AtomicFileWriter, + observer: PersistenceFailureDiagnosticObserver, + ): FileStepAttemptRecordStore = FileStepAttemptRecordStore(rootDirectory, atomicWriter).also { + it.persistenceFailureDiagnosticObserver = observer + } } override suspend fun recordStepAttempt(record: StepAttemptRecord): StepAttemptRecord { record.requirePersistableIdentity() - val path = attemptPath(record.runId, record.stepName, record.attemptId) - return withFileLockCancellable(path) { - atomicWriter.write(path, encodeStoredRecord(record)) - record + return persistenceBoundary( + PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + classify = ::classifyStepAttemptFailure, + ) { + val path = attemptPath(record.runId, record.stepName, record.attemptId) + withFileLockCancellable(path) { + atomicWriter.write(path, encodeStoredRecord(record)) + record + } } } override suspend fun updateStepAttempt(record: StepAttemptRecord): StepAttemptRecord { record.requirePersistableIdentity() - val path = attemptPath(record.runId, record.stepName, record.attemptId) - return withFileLockCancellable(path) { - if (!Files.exists(path)) { - throw IllegalStateException( - "Step attempt '${record.attemptId}' for run '${record.runId}' and step '${record.stepName}' does not exist", - ) + return persistenceBoundary( + PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + classify = ::classifyStepAttemptFailure, + ) { + val path = attemptPath(record.runId, record.stepName, record.attemptId) + withFileLockCancellable(path) { + if (!Files.exists(path)) { + throw IllegalStateException("Step attempt does not exist") + } + readStoredRecord(path, record.runId, record.stepName, record.attemptId) + atomicWriter.write(path, encodeStoredRecord(record)) + record } - readStoredRecord(path, record.runId, record.stepName, record.attemptId) - atomicWriter.write(path, encodeStoredRecord(record)) - record } } @@ -52,77 +77,98 @@ class FileStepAttemptRecordStore internal constructor( ): Boolean { if (expected.identity() != updated.identity()) return false updated.requirePersistableIdentity() - val path = attemptPath(expected.runId, expected.stepName, expected.attemptId) - return withFileLockCancellable(path) { - val current = if (Files.exists(path)) { - readStoredRecord(path, expected.runId, expected.stepName, expected.attemptId) - } else { - null - } - if (current != expected) { - false - } else { - atomicWriter.write(path, encodeStoredRecord(updated)) - true + return persistenceBoundary( + PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.COMPARE_AND_SET, persistenceFailureDiagnosticObserver, + classify = ::classifyStepAttemptFailure, + ) { + val path = attemptPath(expected.runId, expected.stepName, expected.attemptId) + withFileLockCancellable(path) { + val current = if (Files.exists(path)) { + readStoredRecord(path, expected.runId, expected.stepName, expected.attemptId) + } else { + null + } + if (current != expected) { + false + } else { + atomicWriter.write(path, encodeStoredRecord(updated)) + true + } } } } override suspend fun latestStepAttempt(runId: String, stepName: String): StepAttemptRecord? { require(runId.isNotBlank() && stepName.isNotBlank()) { "Step-attempt runId and stepName must not be blank" } - val stepDirectory = rootDirectory - .resolve(base64UrlEncodeNoPadding(runId)) - .resolve(base64UrlEncodeNoPadding(stepName)) - if (!Files.exists(stepDirectory)) return null - val paths = runInterruptible(Dispatchers.IO) { - Files.list(stepDirectory).use { stream -> - stream.filter(Files::isRegularFile) - .filter { it.fileName.toString().endsWith(ATTEMPT_SUFFIX) } - .toList() - } - } - return paths.mapNotNull { path -> - withFileLockCancellable(path) { - if (Files.exists(path)) { - val fileName = path.fileName.toString() - val keyAttemptId = decodePathSegment(fileName.removeSuffix(ATTEMPT_SUFFIX), path) - readStoredRecord(path, runId, stepName, keyAttemptId) - } else { - null + return persistenceBoundary( + PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.LOAD, persistenceFailureDiagnosticObserver, + classify = ::classifyStepAttemptFailure, + ) { + val stepDirectory = rootDirectory + .resolve(base64UrlEncodeNoPadding(runId)) + .resolve(base64UrlEncodeNoPadding(stepName)) + if (!Files.exists(stepDirectory)) { + null + } else { + val paths = runInterruptible(Dispatchers.IO) { + Files.list(stepDirectory).use { stream -> + stream.filter(Files::isRegularFile) + .filter { it.fileName.toString().endsWith(ATTEMPT_SUFFIX) } + .toList() + } } + paths.mapNotNull { path -> + withFileLockCancellable(path) { + if (Files.exists(path)) { + val fileName = path.fileName.toString() + val keyAttemptId = decodePathSegment(fileName.removeSuffix(ATTEMPT_SUFFIX), path) + readStoredRecord(path, runId, stepName, keyAttemptId) + } else { + null + } + } + }.maxWithOrNull(compareBy({ it.startedAt }, { it.attemptId })) } - }.maxWithOrNull(compareBy({ it.startedAt }, { it.attemptId })) + } } override suspend fun listStepAttempts(runId: String): List { require(runId.isNotBlank()) { "Step-attempt runId must not be blank" } - val runDirectory = rootDirectory.resolve(base64UrlEncodeNoPadding(runId)) - if (!Files.exists(runDirectory)) return emptyList() - val paths = runInterruptible(Dispatchers.IO) { - Files.walk(runDirectory).use { stream -> - stream.filter(Files::isRegularFile) - .filter { it.fileName.toString().endsWith(ATTEMPT_SUFFIX) } - .toList() - } - } - return paths.map { path -> - val relative = runDirectory.relativize(path) - if (relative.nameCount != 2) { - throw StepAttemptRecordCorruptionException("Invalid step-attempt path '$path'") - } - val keyStepName = decodePathSegment(relative.getName(0).toString(), path) - val fileName = relative.fileName.toString() - val keyAttemptId = decodePathSegment(fileName.removeSuffix(ATTEMPT_SUFFIX), path) - withFileLockCancellable(path) { - if (!Files.exists(path)) { - null - } else { - readStoredRecord(path, runId, keyStepName, keyAttemptId) + return persistenceBoundary( + PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.LIST, persistenceFailureDiagnosticObserver, + classify = ::classifyStepAttemptFailure, + ) { + val runDirectory = rootDirectory.resolve(base64UrlEncodeNoPadding(runId)) + if (!Files.exists(runDirectory)) { + emptyList() + } else { + val paths = runInterruptible(Dispatchers.IO) { + Files.walk(runDirectory).use { stream -> + stream.filter(Files::isRegularFile) + .filter { it.fileName.toString().endsWith(ATTEMPT_SUFFIX) } + .toList() + } } + paths.map { path -> + val relative = runDirectory.relativize(path) + if (relative.nameCount != 2) { + throw CorruptStepAttemptException("Persisted step-attempt record is invalid", path.toString()) + } + val keyStepName = decodePathSegment(relative.getName(0).toString(), path) + val fileName = relative.fileName.toString() + val keyAttemptId = decodePathSegment(fileName.removeSuffix(ATTEMPT_SUFFIX), path) + withFileLockCancellable(path) { + if (!Files.exists(path)) { + null + } else { + readStoredRecord(path, runId, keyStepName, keyAttemptId) + } + } + }.filterNotNull().sortedWith( + compareBy({ it.startedAt }, { it.stepName }, { it.attemptId }), + ) } - }.filterNotNull().sortedWith( - compareBy({ it.startedAt }, { it.stepName }, { it.attemptId }), - ) + } } private fun attemptPath(runId: String, stepName: String, attemptId: String): Path = rootDirectory @@ -134,19 +180,19 @@ class FileStepAttemptRecordStore internal constructor( val payload = try { Files.readString(path) } catch (error: Exception) { - throw StepAttemptRecordCorruptionException("Unable to read step-attempt record '$path'", error) + throw CorruptStepAttemptException("Persisted step-attempt record is invalid", path.toString(), error) } val properties = try { Properties().apply { load(payload.reader()) } } catch (error: Exception) { - throw StepAttemptRecordCorruptionException("Invalid step-attempt properties in '$path'", error) + throw CorruptStepAttemptException("Persisted step-attempt record is invalid", payload, error) } val record = StepAttemptRecordCodec.decode(payload) val storedHash = properties.getProperty(RECORD_HASH) - ?: throw StepAttemptRecordCorruptionException("Missing record fingerprint in '$path'") - StepAttemptRecordCodec.requireValidFingerprint(record, storedHash, "'$path'") + ?: throw CorruptStepAttemptException("Persisted step-attempt record is invalid", path.toString()) + StepAttemptRecordCodec.requireValidFingerprint(record, storedHash, path.toString()) if (record.identity() != AttemptIdentity(runId, stepName, attemptId)) { - throw StepAttemptRecordCorruptionException("Step-attempt identity does not match storage path '$path'") + throw CorruptStepAttemptException("Persisted step-attempt record is invalid", path.toString()) } return record } @@ -157,7 +203,7 @@ class FileStepAttemptRecordStore internal constructor( private fun decodePathSegment(value: String, path: Path): String = try { base64UrlDecode(value) } catch (error: IllegalArgumentException) { - throw StepAttemptRecordCorruptionException("Invalid encoded identity in step-attempt path '$path'", error) + throw CorruptStepAttemptException("Persisted step-attempt record is invalid", path.toString(), error) } } @@ -173,3 +219,6 @@ internal fun base64UrlDecode(value: String): String = String( private data class AttemptIdentity(val runId: String, val stepName: String, val attemptId: String) private fun StepAttemptRecord.identity(): AttemptIdentity = AttemptIdentity(runId, stepName, attemptId) + +internal fun classifyStepAttemptFailure(error: Throwable): PersistenceFailureCode? = + if (error is CorruptStepAttemptException) PersistenceFailureCode.CORRUPTED_DATA else null diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileWorkflowCheckpointStore.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileWorkflowCheckpointStore.kt index 76c1c1f2..91208632 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileWorkflowCheckpointStore.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileWorkflowCheckpointStore.kt @@ -27,6 +27,9 @@ class FileWorkflowCheckpointStore private constructor( private val pathStrategy: WorkflowCheckpointPathStrategy, private val atomicWriter: AtomicFileWriter, ) : WorkflowCheckpointStore, WorkflowCheckpointCatalog { + var persistenceFailureDiagnosticObserver: PersistenceFailureDiagnosticObserver = + NoOpPersistenceFailureDiagnosticObserver + internal set constructor( rootDirectory: Path, @@ -34,6 +37,14 @@ class FileWorkflowCheckpointStore private constructor( DefaultWorkflowCheckpointPathStrategy("checkpoint.properties"), ) : this(rootDirectory, pathStrategy, realAtomicFileWriter) + constructor( + rootDirectory: Path, + pathStrategy: WorkflowCheckpointPathStrategy, + observer: PersistenceFailureDiagnosticObserver, + ) : this(rootDirectory, pathStrategy, realAtomicFileWriter) { + persistenceFailureDiagnosticObserver = observer + } + internal companion object { fun forTest( rootDirectory: Path, @@ -43,30 +54,47 @@ class FileWorkflowCheckpointStore private constructor( DefaultWorkflowCheckpointPathStrategy("checkpoint.properties"), atomicWriter, ) + + fun forTest( + rootDirectory: Path, + atomicWriter: AtomicFileWriter, + observer: PersistenceFailureDiagnosticObserver, + ) = FileWorkflowCheckpointStore( + rootDirectory, + DefaultWorkflowCheckpointPathStrategy("checkpoint.properties"), + atomicWriter, + ).also { it.persistenceFailureDiagnosticObserver = observer } } override suspend fun load( workflowName: String, workflowId: String, - ): WorkflowCheckpoint? { + ): WorkflowCheckpoint? = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.LOAD, persistenceFailureDiagnosticObserver, + classify = ::classifyCheckpointFailure, + ) { val checkpointPath = checkpointPath(workflowName, workflowId) if (!Files.exists(checkpointPath)) { - return null - } - return withFileLockCancellable(checkpointPath) { - if (!Files.exists(checkpointPath)) { - null - } else { - decodeCheckpoint(Files.readString(checkpointPath)) + null + } else { + withFileLockCancellable(checkpointPath) { + if (!Files.exists(checkpointPath)) { + null + } else { + decodeCheckpoint(Files.readString(checkpointPath)) + } } } } override suspend fun save( checkpoint: WorkflowCheckpoint, expectedRevision: Long?, - ): WorkflowCheckpoint { + ): WorkflowCheckpoint = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + classify = ::classifyCheckpointFailure, + ) { val checkpointPath = checkpointPath(checkpoint.workflowName, checkpoint.workflowId) - return withFileLockCancellable(checkpointPath) { + withFileLockCancellable(checkpointPath) { val existing = if (Files.exists(checkpointPath)) { decodeCheckpoint(Files.readString(checkpointPath)) } else { @@ -89,6 +117,9 @@ class FileWorkflowCheckpointStore private constructor( workflowName: String, workflowId: String, expectedRevision: Long?, + ) = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.DELETE, persistenceFailureDiagnosticObserver, + classify = ::classifyCheckpointFailure, ) { val checkpointPath = checkpointPath(workflowName, workflowId) withFileLockCancellable(checkpointPath) { @@ -104,6 +135,7 @@ class FileWorkflowCheckpointStore private constructor( expectedRevision = expectedRevision, ) Files.deleteIfExists(checkpointPath) + Unit } } @@ -113,18 +145,22 @@ class FileWorkflowCheckpointStore private constructor( * Large deployments should prefer a paged or indexed catalog implementation to avoid heap pressure * during worker scans. */ - override suspend fun listCheckpoints(): List { + override suspend fun listCheckpoints(): List = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.LIST, persistenceFailureDiagnosticObserver, + classify = ::classifyCheckpointFailure, + ) { if (!Files.exists(rootDirectory)) { - return emptyList() - } - Files.walk(rootDirectory).use { paths -> - return paths + emptyList() + } else { + Files.walk(rootDirectory).use { paths -> + paths .filter(Files::isRegularFile) .filter { !it.fileName.toString().endsWith(".lock") } .map(Files::readString) .map(::decodeCheckpoint) .toList() .sortedWith(compareBy({ it.workflowName }, { it.workflowId })) + } } } private fun checkpointPath( @@ -172,17 +208,15 @@ internal fun encodeCheckpoint(checkpoint: WorkflowCheckpoint): String { properties.store(writer, "Tramai workflow checkpoint") }.toString() } -internal fun decodeCheckpoint(content: String): WorkflowCheckpoint { - val properties = Properties().apply { - load(content.reader()) - } +internal fun decodeCheckpoint(content: String): WorkflowCheckpoint = try { + val properties = Properties().apply { load(content.reader()) } val metadata = properties.stringPropertyNames() .filter { it.startsWith("metadata.") } .associate { propertyName -> val encodedKey = propertyName.removePrefix("metadata.") base64Decode(encodedKey) to base64Decode(properties.getProperty(propertyName)) } - return WorkflowCheckpoint( + WorkflowCheckpoint( workflowName = properties.requireProperty("workflowName"), workflowId = properties.requireProperty("workflowId"), nextStepIndex = properties.requireProperty("nextStepIndex").toInt(), @@ -192,10 +226,12 @@ internal fun decodeCheckpoint(content: String): WorkflowCheckpoint { revision = properties.getProperty("revision")?.toLong() ?: 0, metadata = metadata, savedAtEpochMillis = properties.getProperty("savedAtEpochMillis")?.toLong() ?: System.currentTimeMillis(), - recoveryState = decodeRecoveryState( - properties.getProperty("recoveryState")?.takeIf { it.isNotBlank() }?.let(::base64Decode), - ), + recoveryState = decodeRecoveryState(properties.getProperty("recoveryState")?.takeIf { it.isNotBlank() }?.let(::base64Decode)), ) +} catch (error: CorruptCheckpointException) { + throw error +} catch (error: Throwable) { + throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", content) } // --- Per-path Mutex registry with reference counting --- @@ -365,19 +401,13 @@ internal fun validateExpectedRevision( expectedRevision: Long?, ) { if (expectedRevision == null && existing != null) { - throw WorkflowCheckpointConflictException( - "Checkpoint for workflow '$workflowName' and workflowId='$workflowId' already exists at revision ${existing.revision}", - ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, PersistenceFailureCode.CONFLICT) } if (expectedRevision != null && existing == null) { - throw WorkflowCheckpointConflictException( - "Checkpoint for workflow '$workflowName' and workflowId='$workflowId' does not exist for expected revision $expectedRevision", - ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, PersistenceFailureCode.CONFLICT) } if (expectedRevision != null && existing != null && existing.revision != expectedRevision) { - throw WorkflowCheckpointConflictException( - "Checkpoint for workflow '$workflowName' and workflowId='$workflowId' is at revision ${existing.revision}, not expected revision $expectedRevision", - ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, PersistenceFailureCode.CONFLICT) } } internal fun validateDeleteExpectedRevision( @@ -387,14 +417,10 @@ internal fun validateDeleteExpectedRevision( expectedRevision: Long?, ) { if (expectedRevision != null && existing == null) { - throw WorkflowCheckpointConflictException( - "Checkpoint for workflow '$workflowName' and workflowId='$workflowId' does not exist for expected revision $expectedRevision", - ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.DELETE, PersistenceFailureCode.CONFLICT) } if (expectedRevision != null && existing != null && existing.revision != expectedRevision) { - throw WorkflowCheckpointConflictException( - "Checkpoint for workflow '$workflowName' and workflowId='$workflowId' is at revision ${existing.revision}, not expected revision $expectedRevision", - ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.DELETE, PersistenceFailureCode.CONFLICT) } } internal fun sanitizePathSegment(input: String): String = input.map { character -> @@ -410,6 +436,9 @@ internal fun base64Decode(value: String): String = String( StandardCharsets.UTF_8, ) +internal fun classifyCheckpointFailure(error: Throwable): PersistenceFailureCode? = + if (error is CorruptCheckpointException) PersistenceFailureCode.CORRUPTED_DATA else null + internal fun ensureOwnerOnlyDirectory(path: Path) { Files.createDirectories(path) applyOwnerOnlyDirectoryPermissions(path) diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileWorkflowLeaseStore.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileWorkflowLeaseStore.kt index 5d629d36..0ac4bec6 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileWorkflowLeaseStore.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/FileWorkflowLeaseStore.kt @@ -1,4 +1,6 @@ package dev.tramai.orchestration + +import dev.tramai.core.coroutines.rethrowIfCancellation import java.io.StringWriter import java.nio.file.Files import java.nio.file.Path @@ -14,6 +16,9 @@ class FileWorkflowLeaseStore private constructor( private val clockMillis: () -> Long = System::currentTimeMillis, private val atomicWriter: AtomicFileWriter = realAtomicFileWriter, ) : WorkflowLeaseStore, WorkflowLeaseCheckpointFence { + var persistenceFailureDiagnosticObserver: PersistenceFailureDiagnosticObserver = + NoOpPersistenceFailureDiagnosticObserver + internal set constructor( rootDirectory: Path, @@ -22,6 +27,15 @@ class FileWorkflowLeaseStore private constructor( clockMillis: () -> Long = System::currentTimeMillis, ) : this(rootDirectory, pathStrategy, clockMillis, realAtomicFileWriter) + constructor( + rootDirectory: Path, + pathStrategy: WorkflowCheckpointPathStrategy, + clockMillis: () -> Long, + observer: PersistenceFailureDiagnosticObserver, + ) : this(rootDirectory, pathStrategy, clockMillis, realAtomicFileWriter) { + persistenceFailureDiagnosticObserver = observer + } + internal companion object { fun forTest( rootDirectory: Path, @@ -33,22 +47,36 @@ class FileWorkflowLeaseStore private constructor( clockMillis, atomicWriter, ) + + fun forTest( + rootDirectory: Path, + atomicWriter: AtomicFileWriter, + clockMillis: () -> Long = System::currentTimeMillis, + observer: PersistenceFailureDiagnosticObserver, + ) = FileWorkflowLeaseStore( + rootDirectory, + DefaultWorkflowCheckpointPathStrategy("lease.properties"), + clockMillis, + atomicWriter, + ).also { it.persistenceFailureDiagnosticObserver = observer } } override suspend fun currentLease( workflowName: String, workflowId: String, - ): WorkflowLease? { + ): WorkflowLease? = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.LOAD, persistenceFailureDiagnosticObserver, + ) { val leasePath = leasePath(workflowName, workflowId) if (!Files.exists(leasePath)) { - return null + return@persistenceBoundary null } - return withFileLockCancellable(leasePath) { + withFileLockCancellable(leasePath) { val existing = readLeaseIfPresent(leasePath) if (existing == null) { null } else if (isExpired(existing)) { - Files.deleteIfExists(leasePath) + deleteLeaseIfPresent(leasePath) null } else { existing @@ -61,14 +89,14 @@ class FileWorkflowLeaseStore private constructor( ownerId: String, checkpointRevision: Long?, leaseDurationMillis: Long, - ): WorkflowLease { + ): WorkflowLease = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.CLAIM, persistenceFailureDiagnosticObserver, + ) { val leasePath = leasePath(workflowName, workflowId) - return withFileLockCancellable(leasePath) { + withFileLockCancellable(leasePath) { val existing = readLeaseIfPresent(leasePath) if (existing != null && !isExpired(existing)) { - throw WorkflowLeaseConflictException( - "Workflow '$workflowName' and workflowId='$workflowId' is already leased by owner '${existing.ownerId}' until ${existing.expiresAtEpochMillis}", - ) + throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.CLAIM, PersistenceFailureCode.CONFLICT) } val now = clockMillis() val lease = WorkflowLease( @@ -88,23 +116,19 @@ class FileWorkflowLeaseStore private constructor( lease: WorkflowLease, checkpointRevision: Long?, leaseDurationMillis: Long, - ): WorkflowLease { + ): WorkflowLease = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, persistenceFailureDiagnosticObserver, + ) { val leasePath = leasePath(lease.workflowName, lease.workflowId) - return withFileLockCancellable(leasePath) { + withFileLockCancellable(leasePath) { val existing = readLeaseIfPresent(leasePath) - ?: throw WorkflowLeaseConflictException( - "Workflow '${lease.workflowName}' and workflowId='${lease.workflowId}' has no active lease to renew", - ) + ?: throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, PersistenceFailureCode.CONFLICT) if (isExpired(existing)) { - Files.deleteIfExists(leasePath) - throw WorkflowLeaseConflictException( - "Workflow '${lease.workflowName}' and workflowId='${lease.workflowId}' lease has expired before renewal", - ) + deleteLeaseIfPresent(leasePath) + throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, PersistenceFailureCode.CONFLICT) } if (existing.leaseId != lease.leaseId || existing.ownerId != lease.ownerId) { - throw WorkflowLeaseConflictException( - "Workflow '${lease.workflowName}' and workflowId='${lease.workflowId}' is leased by owner '${existing.ownerId}', not '${lease.ownerId}'", - ) + throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, PersistenceFailureCode.CONFLICT) } val now = clockMillis() val renewed = existing.copy( @@ -116,19 +140,19 @@ class FileWorkflowLeaseStore private constructor( } } override suspend fun release(lease: WorkflowLease) { - val leasePath = leasePath(lease.workflowName, lease.workflowId) - withFileLockCancellable(leasePath) { - val existing = readLeaseIfPresent(leasePath) ?: return@withFileLockCancellable - if (isExpired(existing)) { - Files.deleteIfExists(leasePath) - return@withFileLockCancellable + persistenceBoundary(PersistenceResourceKind.LEASE, PersistenceOperation.RELEASE, persistenceFailureDiagnosticObserver) { + val leasePath = leasePath(lease.workflowName, lease.workflowId) + withFileLockCancellable(leasePath) { + val existing = readLeaseIfPresent(leasePath) ?: return@withFileLockCancellable + if (isExpired(existing)) { + deleteLeaseIfPresent(leasePath) + return@withFileLockCancellable + } + if (existing.leaseId != lease.leaseId || existing.ownerId != lease.ownerId) { + throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RELEASE, PersistenceFailureCode.CONFLICT) + } + deleteLeaseIfPresent(leasePath) } - if (existing.leaseId != lease.leaseId || existing.ownerId != lease.ownerId) { - throw WorkflowLeaseConflictException( - "Workflow '${lease.workflowName}' and workflowId='${lease.workflowId}' is leased by owner '${existing.ownerId}', not '${lease.ownerId}'", - ) - } - Files.deleteIfExists(leasePath) } } @@ -137,9 +161,11 @@ class FileWorkflowLeaseStore private constructor( checkpoint: WorkflowCheckpoint, expectedRevision: Long?, expectedLease: WorkflowLease, - ): WorkflowCheckpoint { + ): WorkflowCheckpoint = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + ) { val leasePath = leasePath(expectedLease.workflowName, expectedLease.workflowId) - return withFileLockCancellableSuspending(leasePath) { + withFileLockCancellableSuspending(leasePath) { val current = readLeaseIfPresent(leasePath)?.takeUnless(::isExpired) validateExpectedLease(expectedLease, current) checkpointStore.save(checkpoint, expectedRevision) @@ -153,11 +179,13 @@ class FileWorkflowLeaseStore private constructor( expectedRevision: Long?, expectedLease: WorkflowLease, ) { - val leasePath = leasePath(expectedLease.workflowName, expectedLease.workflowId) - withFileLockCancellableSuspending(leasePath) { - val current = readLeaseIfPresent(leasePath)?.takeUnless(::isExpired) - validateExpectedLease(expectedLease, current) - checkpointStore.delete(workflowName, workflowId, expectedRevision) + persistenceBoundary(PersistenceResourceKind.LEASE, PersistenceOperation.DELETE, persistenceFailureDiagnosticObserver) { + val leasePath = leasePath(expectedLease.workflowName, expectedLease.workflowId) + withFileLockCancellableSuspending(leasePath) { + val current = readLeaseIfPresent(leasePath)?.takeUnless(::isExpired) + validateExpectedLease(expectedLease, current) + checkpointStore.delete(workflowName, workflowId, expectedRevision) + } } } @@ -165,10 +193,22 @@ class FileWorkflowLeaseStore private constructor( workflowName: String, workflowId: String, ): Path = pathStrategy.resolve(rootDirectory, workflowName, workflowId) - private fun readLeaseIfPresent(path: Path): WorkflowLease? = if (Files.exists(path)) { - decodeLease(Files.readString(path)) - } else { - null + private fun readLeaseIfPresent(path: Path): WorkflowLease? = try { + if (Files.exists(path)) { + decodeLease(Files.readString(path)) + } else { + null + } + } catch (error: Throwable) { + error.rethrowIfCancellation() + throw LeaseReadPhaseFailure(error) + } + + private fun deleteLeaseIfPresent(path: Path): Boolean = try { + Files.deleteIfExists(path) + } catch (error: Throwable) { + error.rethrowIfCancellation() + throw LeaseDeletePhaseFailure(error) } private fun isExpired(lease: WorkflowLease): Boolean = clockMillis() >= lease.expiresAtEpochMillis @@ -177,14 +217,10 @@ class FileWorkflowLeaseStore private constructor( current: WorkflowLease?, ) { if (current == null) { - throw StaleWorkflowLeaseException( - "Workflow '${expectedLease.workflowName}' and workflowId='${expectedLease.workflowId}' lease '${expectedLease.leaseId}' is no longer active", - ) + throw safeStaleWorkflowLeaseFailure() } if (current.leaseId != expectedLease.leaseId || current.ownerId != expectedLease.ownerId) { - throw StaleWorkflowLeaseException( - "Workflow '${expectedLease.workflowName}' and workflowId='${expectedLease.workflowId}' is now fenced by lease '${current.leaseId}' owned by '${current.ownerId}'", - ) + throw safeStaleWorkflowLeaseFailure() } } } diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcStepAttemptRecordStore.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcStepAttemptRecordStore.kt index 61ecf7cd..52ad6e3b 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcStepAttemptRecordStore.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcStepAttemptRecordStore.kt @@ -11,34 +11,50 @@ class JdbcStepAttemptRecordStore( private val dataSource: DataSource, private val table: JdbcStepAttemptTable = JdbcStepAttemptTable(), ) : StepAttemptRecordStore { - override suspend fun recordStepAttempt(record: StepAttemptRecord): StepAttemptRecord = - executeJdbcCancellable(dataSource) { conn -> - record.requirePersistableIdentity() - if (update(conn, record, requireHash = null) == 0) { - try { - insert(conn, record) - } catch (error: SQLException) { - if (update(conn, record, requireHash = null) == 0) throw error + var persistenceFailureDiagnosticObserver: PersistenceFailureDiagnosticObserver = + NoOpPersistenceFailureDiagnosticObserver + internal set + + constructor( + dataSource: DataSource, + table: JdbcStepAttemptTable, + observer: PersistenceFailureDiagnosticObserver, + ) : this(dataSource, table) { + persistenceFailureDiagnosticObserver = observer + } + + override suspend fun recordStepAttempt(record: StepAttemptRecord): StepAttemptRecord { + record.requirePersistableIdentity() + return persistenceBoundary(PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, ::classifyStepAttemptFailure) { + executeJdbcCancellable(dataSource) { conn -> + if (update(conn, record, requireHash = null) == 0) { + try { + insert(conn, record) + } catch (error: SQLException) { + if (update(conn, record, requireHash = null) == 0) throw error + } } + record } - record } + } - override suspend fun updateStepAttempt(record: StepAttemptRecord): StepAttemptRecord = - executeJdbcCancellable(dataSource) { conn -> - record.requirePersistableIdentity() - if (update(conn, record, requireHash = null) == 0) { - if (!exists(conn, record)) { - throw IllegalStateException( - "Step attempt '${record.attemptId}' for run '${record.runId}' and step '${record.stepName}' does not exist", - ) + override suspend fun updateStepAttempt(record: StepAttemptRecord): StepAttemptRecord { + record.requirePersistableIdentity() + return persistenceBoundary(PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, ::classifyStepAttemptFailure) { + executeJdbcCancellable(dataSource) { conn -> + if (update(conn, record, requireHash = null) == 0) { + if (!exists(conn, record)) { + throw IllegalStateException("Step attempt does not exist") + } + // A concurrent recordStepAttempt inserted the row between our UPDATE and the + // existence check — retry so the update is not silently dropped. + update(conn, record, requireHash = null) } - // A concurrent recordStepAttempt inserted the row between our UPDATE and the - // existence check — retry so the update is not silently dropped. - update(conn, record, requireHash = null) + record } - record } + } override suspend fun compareAndSetStepAttempt( expected: StepAttemptRecord, @@ -46,34 +62,40 @@ class JdbcStepAttemptRecordStore( ): Boolean { if (expected.key() != updated.key()) return false updated.requirePersistableIdentity() - return executeJdbcCancellable(dataSource) { conn -> - update(conn, updated, StepAttemptRecordCodec.fingerprint(expected)) > 0 + return persistenceBoundary(PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.COMPARE_AND_SET, persistenceFailureDiagnosticObserver, ::classifyStepAttemptFailure) { + executeJdbcCancellable(dataSource) { conn -> + update(conn, updated, StepAttemptRecordCodec.fingerprint(expected)) > 0 + } } } override suspend fun latestStepAttempt(runId: String, stepName: String): StepAttemptRecord? { require(runId.isNotBlank() && stepName.isNotBlank()) { "Step-attempt runId and stepName must not be blank" } - return executeJdbcCancellable(dataSource) { conn -> - conn.prepareStatement(latestSql()).use { statement -> + return persistenceBoundary(PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.LOAD, persistenceFailureDiagnosticObserver, ::classifyStepAttemptFailure) { + executeJdbcCancellable(dataSource) { conn -> + conn.prepareStatement(latestSql()).use { statement -> statement.setString(1, runId) statement.setString(2, stepName) statement.executeQuery().use { resultSet -> if (resultSet.next()) resultSet.toVerifiedRecord() else null } + } } } } override suspend fun listStepAttempts(runId: String): List { require(runId.isNotBlank()) { "Step-attempt runId must not be blank" } - return executeJdbcCancellable(dataSource) { conn -> - conn.prepareStatement(listSql()).use { statement -> + return persistenceBoundary(PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.LIST, persistenceFailureDiagnosticObserver, ::classifyStepAttemptFailure) { + executeJdbcCancellable(dataSource) { conn -> + conn.prepareStatement(listSql()).use { statement -> statement.setString(1, runId) statement.executeQuery().use { resultSet -> buildList { while (resultSet.next()) add(resultSet.toVerifiedRecord()) } } + } } } } @@ -237,35 +259,33 @@ class JdbcStepAttemptRecordStore( resolutionAction = getString(table.resolutionActionColumn)?.let(::decodeResolutionAction), approvedIdempotencyKey = getString(table.approvedIdempotencyKeyColumn), ) - } catch (error: StepAttemptRecordCorruptionException) { + } catch (error: CorruptStepAttemptException) { throw error } catch (error: Exception) { - throw StepAttemptRecordCorruptionException("Invalid JDBC step-attempt record", error) + throw CorruptStepAttemptException("Persisted step-attempt record is invalid", "JDBC storage", error) } val storedVersion = getString(table.recordSchemaVersionColumn) - ?: throw StepAttemptRecordCorruptionException("Missing JDBC step-attempt schema version") + ?: throw CorruptStepAttemptException("Persisted step-attempt record is invalid", table.recordSchemaVersionColumn) if (storedVersion != StepAttemptRecordCodec.SCHEMA_VERSION) { - throw StepAttemptRecordCorruptionException( - "Unsupported JDBC step-attempt schema version '$storedVersion' (expected '${StepAttemptRecordCodec.SCHEMA_VERSION}')", - ) + throw CorruptStepAttemptException("Unsupported step-attempt schema version", storedVersion) } val storedHash = getString(table.recordHashColumn) - ?: throw StepAttemptRecordCorruptionException("Missing JDBC step-attempt fingerprint") + ?: throw CorruptStepAttemptException("Persisted step-attempt record is invalid", table.recordHashColumn) StepAttemptRecordCodec.requireValidFingerprint(record, storedHash, "JDBC storage") return record } private fun corruptColumn(column: String): Nothing = - throw StepAttemptRecordCorruptionException("Missing mandatory JDBC step-attempt column '$column'") + throw CorruptStepAttemptException("Persisted step-attempt record is invalid", column) private fun ResultSet.nullableLong(column: String): Long? = getObject(column)?.let { value -> (value as? Number)?.toLong() - ?: throw StepAttemptRecordCorruptionException("Invalid numeric JDBC step-attempt column '$column'") + ?: throw CorruptStepAttemptException("Persisted step-attempt record is invalid", value.toString()) } private fun > strictEnum(value: String?, label: String, entries: List): E = entries.firstOrNull { it.name == value } - ?: throw StepAttemptRecordCorruptionException("Unknown $label: '$value'") + ?: throw CorruptStepAttemptException("Unknown $label", value) } data class JdbcStepAttemptTable( diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcWorkflowCheckpointStore.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcWorkflowCheckpointStore.kt index 08856cef..3a54e192 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcWorkflowCheckpointStore.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcWorkflowCheckpointStore.kt @@ -12,29 +12,49 @@ class JdbcWorkflowCheckpointStore( internal val dataSource: DataSource, private val table: JdbcWorkflowCheckpointTable = JdbcWorkflowCheckpointTable(), ) : WorkflowCheckpointStore, WorkflowCheckpointCatalog { + var persistenceFailureDiagnosticObserver: PersistenceFailureDiagnosticObserver = + NoOpPersistenceFailureDiagnosticObserver + internal set + + constructor( + dataSource: DataSource, + table: JdbcWorkflowCheckpointTable, + observer: PersistenceFailureDiagnosticObserver, + ) : this(dataSource, table) { + persistenceFailureDiagnosticObserver = observer + } override suspend fun load( workflowName: String, workflowId: String, - ): WorkflowCheckpoint? = executeJdbcCancellable(dataSource) { conn -> + ): WorkflowCheckpoint? = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.LOAD, persistenceFailureDiagnosticObserver, + classify = ::classifyCheckpointFailure, + ) { executeJdbcCancellable(dataSource) { conn -> load(conn, workflowName, workflowId) - } + } } override suspend fun save( checkpoint: WorkflowCheckpoint, expectedRevision: Long?, - ): WorkflowCheckpoint = executeJdbcCancellable(dataSource) { conn -> + ): WorkflowCheckpoint = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + classify = ::classifyCheckpointFailure, + ) { executeJdbcCancellable(dataSource) { conn -> saveInConnection(conn, checkpoint, expectedRevision) - } + } } override suspend fun delete( workflowName: String, workflowId: String, expectedRevision: Long?, ) { - executeJdbcCancellable(dataSource) { conn -> + persistenceBoundary(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.DELETE, persistenceFailureDiagnosticObserver, ::classifyCheckpointFailure) { executeJdbcCancellable(dataSource) { conn -> deleteInConnection(conn, workflowName, workflowId, expectedRevision) - } + } } } - override suspend fun listCheckpoints(): List = executeJdbcCancellable(dataSource) { conn -> + override suspend fun listCheckpoints(): List = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.LIST, persistenceFailureDiagnosticObserver, + classify = ::classifyCheckpointFailure, + ) { executeJdbcCancellable(dataSource) { conn -> conn.prepareStatement(listSql()).use { statement -> statement.executeQuery().use { resultSet -> val checkpoints = mutableListOf() @@ -44,7 +64,7 @@ class JdbcWorkflowCheckpointStore( checkpoints } } - } + } } fun createTableSql(): String = """ CREATE TABLE ${table.tableName} ( ${table.workflowNameColumn} VARCHAR(255) NOT NULL, @@ -260,18 +280,28 @@ class JdbcWorkflowCheckpointStore( setLong(9, checkpoint.savedAtEpochMillis) setString(10, encodeRecoveryState(checkpoint.recoveryState)) } - private fun java.sql.ResultSet.toCheckpoint(): WorkflowCheckpoint = WorkflowCheckpoint( - workflowName = getString(table.workflowNameColumn), - workflowId = getString(table.workflowIdColumn), - nextStepIndex = getInt(table.nextStepIndexColumn), - stepExecutions = getInt(table.stepExecutionsColumn), - lastCompletedStepName = getString(table.lastCompletedStepNameColumn), - statePayload = getString(table.statePayloadColumn), - revision = getLong(table.revisionColumn), - metadata = decodeMetadata(getString(table.metadataColumn)), - savedAtEpochMillis = getLong(table.savedAtEpochMillisColumn), - recoveryState = decodeRecoveryState(getString(table.recoveryStateColumn)), - ) + private fun java.sql.ResultSet.toCheckpoint(): WorkflowCheckpoint { + val metadataPayload = getString(table.metadataColumn) + val recoveryPayload = getString(table.recoveryStateColumn) + return try { + WorkflowCheckpoint( + workflowName = getString(table.workflowNameColumn), + workflowId = getString(table.workflowIdColumn), + nextStepIndex = getInt(table.nextStepIndexColumn), + stepExecutions = getInt(table.stepExecutionsColumn), + lastCompletedStepName = getString(table.lastCompletedStepNameColumn), + statePayload = getString(table.statePayloadColumn), + revision = getLong(table.revisionColumn), + metadata = decodeMetadata(metadataPayload), + savedAtEpochMillis = getLong(table.savedAtEpochMillisColumn), + recoveryState = decodeRecoveryState(recoveryPayload), + ) + } catch (error: CorruptCheckpointException) { + throw error + } catch (error: Throwable) { + throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", metadataPayload ?: recoveryPayload) + } + } } data class JdbcWorkflowCheckpointTable( val tableName: String = "tramai_workflow_checkpoint", @@ -309,16 +339,19 @@ internal fun encodeMetadata(metadata: Map): String { properties.store(writer, "Tramai workflow checkpoint metadata") }.toString() } -internal fun decodeMetadata(payload: String?): Map { +internal fun decodeMetadata(payload: String?): Map = try { if (payload.isNullOrBlank()) { - return emptyMap() - } - val properties = Properties().apply { - load(payload.reader()) - } - return properties.stringPropertyNames().associate { encodedKey -> - base64Decode(encodedKey) to base64Decode(properties.getProperty(encodedKey)) + emptyMap() + } else { + val properties = Properties().apply { load(payload.reader()) } + properties.stringPropertyNames().associate { encodedKey -> + base64Decode(encodedKey) to base64Decode(properties.getProperty(encodedKey)) + } } +} catch (error: CorruptCheckpointException) { + throw error +} catch (error: Throwable) { + throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", payload) } internal fun requireValidSqlIdentifier( diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcWorkflowLeaseStore.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcWorkflowLeaseStore.kt index c3eeff8f..c432e06d 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcWorkflowLeaseStore.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/JdbcWorkflowLeaseStore.kt @@ -1,5 +1,6 @@ package dev.tramai.orchestration +import dev.tramai.core.coroutines.rethrowIfCancellation import java.sql.SQLException import javax.sql.DataSource @@ -13,12 +14,27 @@ class JdbcWorkflowLeaseStore( internal val table: JdbcWorkflowLeaseTable = JdbcWorkflowLeaseTable(), private val clockMillis: () -> Long = System::currentTimeMillis, ) : WorkflowLeaseStore, WorkflowLeaseCheckpointFence { + var persistenceFailureDiagnosticObserver: PersistenceFailureDiagnosticObserver = + NoOpPersistenceFailureDiagnosticObserver + internal set + + constructor( + dataSource: DataSource, + table: JdbcWorkflowLeaseTable, + clockMillis: () -> Long, + observer: PersistenceFailureDiagnosticObserver, + ) : this(dataSource, table, clockMillis) { + persistenceFailureDiagnosticObserver = observer + } + override suspend fun currentLease( workflowName: String, workflowId: String, - ): WorkflowLease? { - val lease = loadLease(workflowName, workflowId) ?: return null - return lease.takeUnless(::isExpired) + ): WorkflowLease? = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.LOAD, persistenceFailureDiagnosticObserver, + ) { + val lease = loadLease(workflowName, workflowId) + lease?.takeUnless(::isExpired) } override suspend fun claim( workflowName: String, @@ -26,7 +42,9 @@ class JdbcWorkflowLeaseStore( ownerId: String, checkpointRevision: Long?, leaseDurationMillis: Long, - ): WorkflowLease { + ): WorkflowLease = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.CLAIM, persistenceFailureDiagnosticObserver, + ) { val lease = newLease( workflowName = workflowName, workflowId = workflowId, @@ -35,9 +53,9 @@ class JdbcWorkflowLeaseStore( leaseDurationMillis = leaseDurationMillis, ) val existing = loadLease(workflowName, workflowId) - return when { + when { existing == null -> insertLease(lease) - !isExpired(existing) -> throw activeLeaseConflict(existing) + !isExpired(existing) -> throw activeLeaseConflict() else -> replaceExpiredLease( lease = lease, previous = existing, @@ -48,7 +66,9 @@ class JdbcWorkflowLeaseStore( lease: WorkflowLease, checkpointRevision: Long?, leaseDurationMillis: Long, - ): WorkflowLease { + ): WorkflowLease = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, persistenceFailureDiagnosticObserver, + ) { val now = clockMillis() val renewed = lease.copy( checkpointRevision = checkpointRevision, @@ -70,17 +90,14 @@ class JdbcWorkflowLeaseStore( val updated = statement.executeUpdate() if (updated == 0) { val existing = loadLease(conn, lease.workflowName, lease.workflowId) - throw renewalConflict( - attempted = lease, - existing = existing, - ) + throw renewalConflict() } } } - return renewed + renewed } override suspend fun release(lease: WorkflowLease) { - executeJdbcCancellable(dataSource) { conn -> + persistenceBoundary(PersistenceResourceKind.LEASE, PersistenceOperation.RELEASE, persistenceFailureDiagnosticObserver) { executeJdbcCancellable(dataSource) { conn -> conn.prepareStatement(releaseSql()).use { statement -> statement.setString(1, lease.workflowName) statement.setString(2, lease.workflowId) @@ -93,13 +110,10 @@ class JdbcWorkflowLeaseStore( deleteExpiredLease(conn, existing) return@executeJdbcCancellable } - throw releaseConflict( - attempted = lease, - existing = existing, - ) + throw releaseConflict() } } - } + } } } override suspend fun saveCheckpointIfLeaseOwner( @@ -108,14 +122,34 @@ class JdbcWorkflowLeaseStore( expectedRevision: Long?, expectedLease: WorkflowLease, ): WorkflowCheckpoint { + // Caller/framework preconditions stay OUTSIDE the boundary: a wrong + // store type or DataSource mismatch is a caller error (IllegalArgumentException), + // not a persistence failure (the P2-3 finding). Both fences must agree. val jdbcCheckpointStore = checkpointStore as? JdbcWorkflowCheckpointStore ?: throw unsupportedFence(checkpointStore) require(jdbcCheckpointStore.dataSource === dataSource) { "JdbcWorkflowLeaseStore can only fence JdbcWorkflowCheckpointStore instances that share the same DataSource" } - return executeJdbcCancellable(dataSource, transactional = true) { conn -> - lockLeaseRow(conn, expectedLease) - jdbcCheckpointStore.saveInConnection(conn, checkpoint, expectedRevision) + return persistenceBoundary( + PersistenceResourceKind.LEASE, + PersistenceOperation.SAVE, + persistenceFailureDiagnosticObserver, + checkpointDiagnosticObserver = jdbcCheckpointStore.persistenceFailureDiagnosticObserver, + ) { + executeJdbcCancellable(dataSource, transactional = true) { conn -> + lockLeaseRow(conn, expectedLease) + // Checkpoint DML belongs to the checkpoint store's diagnostic + // channel, not the lease store's: mark the raw failure so the outer + // lease boundary routes it to jdbcCheckpointStore's observer with + // resourceKind=CHECKPOINT. Lease-row/fence failures stay on the + // lease channel (exactly one event per failing phase). + try { + jdbcCheckpointStore.saveInConnection(conn, checkpoint, expectedRevision) + } catch (error: Throwable) { + error.rethrowIfCancellation() + throw CheckpointDmlFailure(error) + } + } } } @@ -131,10 +165,22 @@ class JdbcWorkflowLeaseStore( require(jdbcCheckpointStore.dataSource === dataSource) { "JdbcWorkflowLeaseStore can only fence JdbcWorkflowCheckpointStore instances that share the same DataSource" } - executeJdbcCancellable(dataSource, transactional = true) { conn -> + persistenceBoundary( + PersistenceResourceKind.LEASE, + PersistenceOperation.DELETE, + persistenceFailureDiagnosticObserver, + checkpointDiagnosticObserver = (checkpointStore as? JdbcWorkflowCheckpointStore)?.persistenceFailureDiagnosticObserver, + ) { executeJdbcCancellable(dataSource, transactional = true) { conn -> lockLeaseRow(conn, expectedLease) - jdbcCheckpointStore.deleteInConnection(conn, workflowName, workflowId, expectedRevision) - } + // Same diagnostic-ownership split as saveCheckpointIfLeaseOwner: + // checkpoint DML failures go to the checkpoint store's observer. + try { + jdbcCheckpointStore.deleteInConnection(conn, workflowName, workflowId, expectedRevision) + } catch (error: Throwable) { + error.rethrowIfCancellation() + throw CheckpointDmlFailure(error) + } + } } } fun createTableSql(): String = """ CREATE TABLE ${table.tableName} ( @@ -158,7 +204,7 @@ class JdbcWorkflowLeaseStore( } catch (error: SQLException) { val current = loadLease(conn, lease.workflowName, lease.workflowId) if (current != null && !isExpired(current)) { - throw activeLeaseConflict(current) + throw activeLeaseConflict() } throw error } @@ -189,11 +235,9 @@ class JdbcWorkflowLeaseStore( if (updated == 0) { val current = loadLease(conn, lease.workflowName, lease.workflowId) if (current != null && !isExpired(current)) { - throw activeLeaseConflict(current) + throw activeLeaseConflict() } - throw WorkflowLeaseConflictException( - "Workflow '${lease.workflowName}' and workflowId='${lease.workflowId}' could not replace its expired lease atomically", - ) + throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.CLAIM, PersistenceFailureCode.CONFLICT) } } } @@ -210,16 +254,21 @@ class JdbcWorkflowLeaseStore( connection: java.sql.Connection, workflowName: String, workflowId: String, - ): WorkflowLease? = connection.prepareStatement(selectSql()).use { statement -> - statement.setString(1, workflowName) - statement.setString(2, workflowId) - statement.executeQuery().use { resultSet -> - if (!resultSet.next()) { - null - } else { - resultSet.toLease() + ): WorkflowLease? = try { + connection.prepareStatement(selectSql()).use { statement -> + statement.setString(1, workflowName) + statement.setString(2, workflowId) + statement.executeQuery().use { resultSet -> + if (!resultSet.next()) { + null + } else { + resultSet.toLease() + } } } + } catch (error: Throwable) { + error.rethrowIfCancellation() + throw LeaseReadPhaseFailure(error) } private suspend fun deleteExpiredLease(lease: WorkflowLease) { executeJdbcCancellable(dataSource) { conn -> @@ -261,15 +310,7 @@ class JdbcWorkflowLeaseStore( statement.setLong(6, clockMillis()) val updated = statement.executeUpdate() if (updated == 0) { - val existing = loadLease(connection, expectedLease.workflowName, expectedLease.workflowId) - throw when { - existing == null || isExpired(existing) -> StaleWorkflowLeaseException( - "Workflow '${expectedLease.workflowName}' and workflowId='${expectedLease.workflowId}' lease '${expectedLease.leaseId}' is no longer active", - ) - else -> StaleWorkflowLeaseException( - "Workflow '${expectedLease.workflowName}' and workflowId='${expectedLease.workflowId}' is now fenced by lease '${existing.leaseId}' owned by '${existing.ownerId}'", - ) - } + throw safeStaleWorkflowLeaseFailure() } } } @@ -277,30 +318,9 @@ class JdbcWorkflowLeaseStore( private fun unsupportedFence(checkpointStore: WorkflowCheckpointStore): IllegalArgumentException = IllegalArgumentException( "JdbcWorkflowLeaseStore can only fence JdbcWorkflowCheckpointStore instances, not ${checkpointStore::class.qualifiedName}", ) - private fun activeLeaseConflict(existing: WorkflowLease): WorkflowLeaseConflictException = - WorkflowLeaseConflictException( - "Workflow '${existing.workflowName}' and workflowId='${existing.workflowId}' is already leased by owner '${existing.ownerId}' until ${existing.expiresAtEpochMillis}", - ) - private fun renewalConflict( - attempted: WorkflowLease, - existing: WorkflowLease?, - ): WorkflowLeaseConflictException = when { - existing == null -> WorkflowLeaseConflictException( - "Workflow '${attempted.workflowName}' and workflowId='${attempted.workflowId}' has no active lease to renew", - ) - isExpired(existing) -> WorkflowLeaseConflictException( - "Workflow '${attempted.workflowName}' and workflowId='${attempted.workflowId}' lease has expired before renewal", - ) - else -> WorkflowLeaseConflictException( - "Workflow '${attempted.workflowName}' and workflowId='${attempted.workflowId}' is leased by owner '${existing.ownerId}', not '${attempted.ownerId}'", - ) - } - private fun releaseConflict( - attempted: WorkflowLease, - existing: WorkflowLease, - ): WorkflowLeaseConflictException = WorkflowLeaseConflictException( - "Workflow '${attempted.workflowName}' and workflowId='${attempted.workflowId}' is leased by owner '${existing.ownerId}', not '${attempted.ownerId}'", - ) + private fun activeLeaseConflict(): RuntimeException = safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.CLAIM, PersistenceFailureCode.CONFLICT) + private fun renewalConflict(): RuntimeException = safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, PersistenceFailureCode.CONFLICT) + private fun releaseConflict(): RuntimeException = safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RELEASE, PersistenceFailureCode.CONFLICT) private fun newLease( workflowName: String, workflowId: String, diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/LoggingTramaiWorkerObserver.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/LoggingTramaiWorkerObserver.kt index 02167c33..7633f3da 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/LoggingTramaiWorkerObserver.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/LoggingTramaiWorkerObserver.kt @@ -8,14 +8,14 @@ class LoggingTramaiWorkerObserver( override fun onLeaseAcquired(workflowId: String, workerId: String) = logEvent("workflow=$workflowId worker=$workerId event=lease_acquired") override fun onLeaseReleased(workflowId: String, workerId: String) = logEvent("workflow=$workflowId worker=$workerId event=lease_released") override fun onLeaseExpired(workflowId: String, workerId: String) = logEvent("workflow=$workflowId worker=$workerId event=lease_expired") - override fun onLeaseRenewalFailed(workflowId: String, workerId: String, error: Throwable) = logEvent("workflow=$workflowId worker=$workerId event=lease_renewal_failed error=${error.message}") - override fun onLeaseReleaseFailed(workflowId: String, workerId: String, error: Throwable) = logEvent("workflow=$workflowId worker=$workerId event=lease_release_failed error=${error.message}") - override fun onPollFailed(workerId: String, error: Throwable) = logEvent("worker=$workerId event=poll_failed error=${error.message}") + override fun onLeaseRenewalFailed(workflowId: String, workerId: String, error: Throwable) = logEvent("workflow=$workflowId worker=$workerId event=lease_renewal_failed ${failureText(error)}") + override fun onLeaseReleaseFailed(workflowId: String, workerId: String, error: Throwable) = logEvent("workflow=$workflowId worker=$workerId event=lease_release_failed ${failureText(error)}") + override fun onPollFailed(workerId: String, error: Throwable) = logEvent("worker=$workerId event=poll_failed ${failureText(error)}") override fun onWorkTakenOver(workflowId: String, previousWorkerId: String, newWorkerId: String) = logEvent("workflow=$workflowId event=work_taken_over from=$previousWorkerId to=$newWorkerId") override fun onUnknownAttempt(runId: String, stepName: String, priorWorkerId: String, attemptTime: Long) = logEvent("run=$runId step=$stepName event=unknown_attempt prior_worker=$priorWorkerId attempt_time=$attemptTime") override fun onStepAttemptStarted(runId: String, stepName: String, attemptId: String, workerId: String) = logEvent("run=$runId step=$stepName attempt=$attemptId worker=$workerId event=attempt_started") override fun onStepAttemptCompleted(runId: String, stepName: String, attemptId: String, workerId: String) = logEvent("run=$runId step=$stepName attempt=$attemptId worker=$workerId event=attempt_completed") - override fun onStepAttemptFailed(runId: String, stepName: String, attemptId: String, workerId: String, error: Throwable) = logEvent("run=$runId step=$stepName attempt=$attemptId worker=$workerId event=attempt_failed error=${error.message}") + override fun onStepAttemptFailed(runId: String, stepName: String, attemptId: String, workerId: String, error: Throwable) = logEvent("run=$runId step=$stepName attempt=$attemptId worker=$workerId event=attempt_failed ${failureText(error)}") override fun onShutdownStarted(workerId: String) = logEvent("worker=$workerId event=shutdown_started") override fun onDrainProgress(workerId: String, done: Int, pending: Int) = logEvent("worker=$workerId event=drain_progress done=$done pending=$pending") override fun onShutdownComplete(workerId: String) = logEvent("worker=$workerId event=shutdown_complete") @@ -24,6 +24,8 @@ class LoggingTramaiWorkerObserver( override fun onLeaseContested(workflowId: String, claimantWorkerId: String, currentWorkerId: String) = logEvent("workflow=$workflowId event=lease_contested claimant=$claimantWorkerId current=$currentWorkerId") override fun onWorkflowAbandoned(workflowId: String, workerId: String, lastStep: String?, timeoutMillis: Long) = logEvent("workflow=$workflowId worker=$workerId event=workflow_abandoned last_step=$lastStep timeout=$timeoutMillis") + private fun failureText(error: Throwable): String = "error=${error::class.java.simpleName}" + companion object { private val logger = java.util.logging.Logger.getLogger("dev.tramai.orchestration.LoggingTramaiWorkerObserver") } diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/MarkdownWorkflowCheckpointStore.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/MarkdownWorkflowCheckpointStore.kt index adfd0292..3f65d0e9 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/MarkdownWorkflowCheckpointStore.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/MarkdownWorkflowCheckpointStore.kt @@ -10,28 +10,46 @@ class MarkdownWorkflowCheckpointStore( private val rootDirectory: Path, private val pathStrategy: WorkflowCheckpointPathStrategy = DefaultWorkflowCheckpointPathStrategy("checkpoint.md"), ) : WorkflowCheckpointStore { + var persistenceFailureDiagnosticObserver: PersistenceFailureDiagnosticObserver = + NoOpPersistenceFailureDiagnosticObserver + internal set + + constructor( + rootDirectory: Path, + pathStrategy: WorkflowCheckpointPathStrategy, + observer: PersistenceFailureDiagnosticObserver, + ) : this(rootDirectory, pathStrategy) { + persistenceFailureDiagnosticObserver = observer + } override suspend fun load( workflowName: String, workflowId: String, - ): WorkflowCheckpoint? { + ): WorkflowCheckpoint? = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.LOAD, persistenceFailureDiagnosticObserver, + classify = ::classifyCheckpointFailure, + ) { val checkpointPath = checkpointPath(workflowName, workflowId) if (!Files.exists(checkpointPath)) { - return null - } - return withFileLockCancellable(checkpointPath) { - if (!Files.exists(checkpointPath)) { - null - } else { - decodeMarkdownCheckpoint(Files.readString(checkpointPath)) + null + } else { + withFileLockCancellable(checkpointPath) { + if (!Files.exists(checkpointPath)) { + null + } else { + decodeMarkdownCheckpoint(Files.readString(checkpointPath)) + } } } } override suspend fun save( checkpoint: WorkflowCheckpoint, expectedRevision: Long?, - ): WorkflowCheckpoint { + ): WorkflowCheckpoint = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + classify = ::classifyCheckpointFailure, + ) { val checkpointPath = checkpointPath(checkpoint.workflowName, checkpoint.workflowId) - return withFileLockCancellable(checkpointPath) { + withFileLockCancellable(checkpointPath) { val existing = if (Files.exists(checkpointPath)) { decodeMarkdownCheckpoint(Files.readString(checkpointPath)) } else { @@ -52,6 +70,9 @@ class MarkdownWorkflowCheckpointStore( workflowName: String, workflowId: String, expectedRevision: Long?, + ) = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.DELETE, persistenceFailureDiagnosticObserver, + classify = ::classifyCheckpointFailure, ) { val checkpointPath = checkpointPath(workflowName, workflowId) withFileLockCancellable(checkpointPath) { @@ -67,6 +88,7 @@ class MarkdownWorkflowCheckpointStore( expectedRevision = expectedRevision, ) Files.deleteIfExists(checkpointPath) + Unit } } private fun checkpointPath( @@ -104,39 +126,39 @@ internal fun encodeMarkdownCheckpoint(checkpoint: WorkflowCheckpoint): String { appendLine(fence) } } -internal fun decodeMarkdownCheckpoint(content: String): WorkflowCheckpoint { +internal fun decodeMarkdownCheckpoint(content: String): WorkflowCheckpoint = try { val lines = content.lines() - require(lines.firstOrNull() == "---") { "Markdown checkpoint is missing opening front matter" } + if (lines.firstOrNull() != "---") throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", content) val closingIndex = lines.drop(1).indexOfFirst { it == "---" } .takeIf { it >= 0 } ?.plus(1) ?: -1 - require(closingIndex > 0) { "Markdown checkpoint is missing closing front matter" } + if (closingIndex <= 0) throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", content) val frontMatter = lines.subList(1, closingIndex) .filter { it.isNotBlank() } .associate { line -> val separatorIndex = line.indexOf(": ") - require(separatorIndex > 0) { "Malformed markdown checkpoint front matter line '$line'" } + if (separatorIndex <= 0) throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", line) line.substring(0, separatorIndex) to line.substring(separatorIndex + 2) } val payloadHeaderIndex = lines.indexOfFirst { it == "## State Payload" } - require(payloadHeaderIndex >= 0) { "Markdown checkpoint is missing state payload heading" } + if (payloadHeaderIndex < 0) throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", content) val fenceLineIndex = ((payloadHeaderIndex + 1) until lines.size) .firstOrNull { lines[it].startsWith("```") } ?: -1 - require(fenceLineIndex > payloadHeaderIndex) { "Markdown checkpoint is missing opening payload fence" } + if (fenceLineIndex <= payloadHeaderIndex) throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", content) val fence = lines[fenceLineIndex].substringBefore(" ") val closingFenceIndex = ((fenceLineIndex + 1) until lines.size) .firstOrNull { lines[it] == fence } ?: -1 - require(closingFenceIndex > fenceLineIndex) { "Markdown checkpoint is missing closing payload fence" } + if (closingFenceIndex <= fenceLineIndex) throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", content) val payload = lines.subList(fenceLineIndex + 1, closingFenceIndex).joinToString("\n") val metadata = frontMatter.entries .filter { it.key.startsWith("metadata.") } .associate { entry -> base64Decode(entry.key.removePrefix("metadata.")) to base64Decode(entry.value) } - return WorkflowCheckpoint( + WorkflowCheckpoint( workflowName = base64Decode(frontMatter.requireValue("workflowName")), workflowId = base64Decode(frontMatter.requireValue("workflowId")), nextStepIndex = frontMatter.requireValue("nextStepIndex").toInt(), @@ -150,6 +172,10 @@ internal fun decodeMarkdownCheckpoint(content: String): WorkflowCheckpoint { frontMatter["recoveryState"]?.takeIf { it.isNotBlank() }?.let(::base64Decode), ), ) +} catch (error: CorruptCheckpointException) { + throw error +} catch (error: Throwable) { + throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", content) } private fun markdownFence(content: String): String { val longestFence = Regex("`+") @@ -159,4 +185,4 @@ private fun markdownFence(content: String): String { return "`".repeat(max(3, longestFence + 1)) } private fun Map.requireValue(key: K): String = get(key) - ?: error("Missing markdown checkpoint field '$key'") + ?: throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", null) diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/PersistenceFailures.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/PersistenceFailures.kt new file mode 100644 index 00000000..81e3380d --- /dev/null +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/PersistenceFailures.kt @@ -0,0 +1,465 @@ +package dev.tramai.orchestration + +import dev.tramai.core.coroutines.rethrowIfCancellation +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive + +/** Machine-readable, stable classification for persistence failures. */ +enum class PersistenceFailureCode { + READ_FAILED, + WRITE_FAILED, + DELETE_FAILED, + LIST_FAILED, + CONFLICT, + CORRUPTED_DATA, +} + +/** Persistence resource context for a failure. */ +enum class PersistenceResourceKind { CHECKPOINT, LEASE, STEP_ATTEMPT, WORKER_REGISTRY } + +/** Persistence operation context for a failure. */ +enum class PersistenceOperation { LOAD, SAVE, DELETE, LIST, CLAIM, RENEW, RELEASE, COMPARE_AND_SET } + +/** + * Receives diagnostic-only persistence failure data. + * + * Delivery is fail-open: ordinary observer failures do not replace the persistence + * failure. An observer-thrown [CancellationException] is swallowed only while the + * enclosing coroutine remains active; genuine cancellation is rethrown. Observer + * data is never automatically forwarded to public exceptions, logs, audit, or + * telemetry. + */ +fun interface PersistenceFailureDiagnosticObserver { + suspend fun onFailure(event: PersistenceFailureDiagnosticEvent) +} + +data class PersistenceFailureDiagnosticEvent( + val resourceKind: PersistenceResourceKind, + val operation: PersistenceOperation, + val failureCode: PersistenceFailureCode, + val failure: Throwable, +) + +/** + * Public, diagnostic-only view of a corrupt persisted payload. + * + * An external [PersistenceFailureDiagnosticObserver] receives the internal + * corrupt carrier in [PersistenceFailureDiagnosticEvent.failure] (directly for + * normal corruption events; as the cause chain of an internal marker such as + * [CheckpointDmlFailure] for fenced JDBC DML). Casting the reachable carrier to + * this interface is the only public way to read the raw payload that failed to + * decode. The payload is never forwarded to caller-visible exceptions, logs, + * audit, or telemetry. + */ +interface PersistenceCorruptionDetail { + /** Raw persisted payload that failed to decode (diagnostic-only). */ + val corruptPayload: String? +} + +object NoOpPersistenceFailureDiagnosticObserver : PersistenceFailureDiagnosticObserver { + override suspend fun onFailure(event: PersistenceFailureDiagnosticEvent) = Unit +} + +internal class CorruptCheckpointException( + message: String, + val rawPayload: String?, +) : RuntimeException(message), PersistenceCorruptionDetail { + override val corruptPayload: String? get() = rawPayload +} + +/** + * Marker raised inside [executeJdbcCancellable]'s non-suspend block when the + * checkpoint DML phase of a fenced operation fails. The outer lease boundary + * recognizes it and routes the raw failure to the checkpoint store's diagnostic + * observer with resourceKind=CHECKPOINT instead of misattributing it to LEASE. + */ +internal class CheckpointDmlFailure( + val raw: Throwable, +) : RuntimeException("Checkpoint DML failed", raw) + +/** + * Marker raised when a lease read phase fails inside a compound operation + * (CLAIM/RENEW/RELEASE perform reads before/during their mutation). The + * low-level failing phase chooses the failure code: the boundary classifies + * this as READ_FAILED while the outer operation (CLAIM/RENEW/RELEASE) supplies + * the operation context. + */ +internal class LeaseReadPhaseFailure( + val raw: Throwable, +) : RuntimeException("Lease read phase failed", raw) + +/** + * Marker raised when a lease DELETE phase fails inside an operation whose + * default code would be wrong (e.g. expired-lease cleanup during LOAD/RENEW). + * The failing phase is DELETE_FAILED; the outer operation context is preserved. + */ +internal class LeaseDeletePhaseFailure( + val raw: Throwable, +) : RuntimeException("Lease delete phase failed", raw) + +internal class CorruptStepAttemptException( + message: String, + val rawPayload: String?, + cause: Throwable? = null, +) : RuntimeException(message, cause), PersistenceCorruptionDetail { + override val corruptPayload: String? get() = rawPayload +} + +/** General-purpose safe persistence failure for non-semantic persistence errors. */ +class WorkflowPersistenceFailureException( + message: String, +) : RuntimeException(message) { + var failureCode: PersistenceFailureCode? = null + internal set + var safeFactoryTrusted: Boolean = false + internal set +} + +internal fun fixedPersistenceFailureMessage( + resourceKind: PersistenceResourceKind, + operation: PersistenceOperation, + code: PersistenceFailureCode, +): String = when (code) { + PersistenceFailureCode.CONFLICT -> when { + resourceKind == PersistenceResourceKind.CHECKPOINT && + operation in setOf(PersistenceOperation.COMPARE_AND_SET, PersistenceOperation.SAVE) -> + "Workflow checkpoint conflict" + resourceKind == PersistenceResourceKind.LEASE -> "Workflow lease conflict" + else -> "${persistenceResourceLabel(resourceKind).replaceFirstChar { it.uppercase() }} conflict" + } + PersistenceFailureCode.CORRUPTED_DATA -> "Persisted ${persistenceResourceLabel(resourceKind)} is invalid" + PersistenceFailureCode.READ_FAILED -> "Workflow persistence read failed" + PersistenceFailureCode.WRITE_FAILED -> "Workflow persistence write failed" + PersistenceFailureCode.DELETE_FAILED -> "Workflow persistence delete failed" + PersistenceFailureCode.LIST_FAILED -> "Workflow persistence list failed" +} + +private fun persistenceResourceLabel(resourceKind: PersistenceResourceKind): String = when (resourceKind) { + PersistenceResourceKind.CHECKPOINT -> "workflow checkpoint" + PersistenceResourceKind.LEASE -> "workflow lease" + PersistenceResourceKind.STEP_ATTEMPT -> "step-attempt record" + PersistenceResourceKind.WORKER_REGISTRY -> "worker-registry record" +} + +internal fun safePersistenceFailure( + resourceKind: PersistenceResourceKind, + operation: PersistenceOperation, + code: PersistenceFailureCode, +): RuntimeException { + val failure = when { + resourceKind == PersistenceResourceKind.CHECKPOINT && code == PersistenceFailureCode.CONFLICT -> + WorkflowCheckpointConflictException(fixedPersistenceFailureMessage(resourceKind, operation, code)) + resourceKind == PersistenceResourceKind.LEASE && code == PersistenceFailureCode.CONFLICT -> + WorkflowLeaseConflictException(fixedPersistenceFailureMessage(resourceKind, operation, code)) + resourceKind == PersistenceResourceKind.CHECKPOINT && code == PersistenceFailureCode.CORRUPTED_DATA -> + WorkflowCheckpointCorruptionException(fixedPersistenceFailureMessage(resourceKind, operation, code)) + resourceKind == PersistenceResourceKind.STEP_ATTEMPT && code == PersistenceFailureCode.CORRUPTED_DATA -> + StepAttemptRecordCorruptionException(fixedPersistenceFailureMessage(resourceKind, operation, code)) + else -> WorkflowPersistenceFailureException(fixedPersistenceFailureMessage(resourceKind, operation, code)) + } + failure.setPersistenceFailureMetadata(code) + return failure +} + +internal fun safeWorkerObservableFailure( + resourceKind: PersistenceResourceKind, + operation: PersistenceOperation, + error: Throwable, +): Throwable { + // Never pass a throwable identity across an external surface: JDBC cleanup + // can mutate a trusted exception after construction (addSuppressed). Trust + // only immutable classification; reconstruct a fresh fixed-text instance. + if (error.persistenceFailureTrusted()) return reconstructSafePersistenceFailure(error) + + val code = error.persistenceFailureCode() ?: defaultPersistenceFailureCode(operation) + return safePersistenceFailure(resourceKind, operation, code) +} + +/** + * True when [error] is a persistence-family failure (a trusted safe exception, + * an untrusted caller-constructed persistence exception, or a raw failure + * classified as one by the boundary). Used by the worker to decide whether a + * step-attempt failure is a persistence failure (sanitize) or a user + * step-execution error (keep the real message). + */ +internal fun Throwable.isPersistenceFamilyFailure(): Boolean = + persistenceFailureTrusted() || persistenceFailureCode() != null + +private fun RuntimeException.setPersistenceFailureMetadata(code: PersistenceFailureCode) { + when (this) { + is WorkflowResumeException -> { failureCode = code; safeFactoryTrusted = true } + is WorkflowCheckpointConflictException -> { failureCode = code; safeFactoryTrusted = true } + is WorkflowCheckpointCorruptionException -> { failureCode = code; safeFactoryTrusted = true } + is WorkflowLeaseConflictException -> { failureCode = code; safeFactoryTrusted = true } + is StaleWorkflowLeaseException -> { failureCode = code; safeFactoryTrusted = true } + is StepAttemptRecordCorruptionException -> { failureCode = code; safeFactoryTrusted = true } + is WorkflowPersistenceFailureException -> { failureCode = code; safeFactoryTrusted = true } + } +} + +/** + * Safe factory for a fenced/stale lease failure. + * + * Distinct from [safePersistenceFailure] (which maps LEASE/CONFLICT to + * [WorkflowLeaseConflictException]): the worker relies on + * [StaleWorkflowLeaseException] as a semantic class for lease-fencing + * failures, so the boundary must preserve that class with fixed text. + */ +internal fun safeStaleWorkflowLeaseFailure(): StaleWorkflowLeaseException = + StaleWorkflowLeaseException("Workflow lease is no longer active") + .also { it.setPersistenceFailureMetadata(PersistenceFailureCode.CONFLICT) } + +internal suspend fun persistenceBoundary( + resourceKind: PersistenceResourceKind, + operation: PersistenceOperation, + diagnosticObserver: PersistenceFailureDiagnosticObserver, + classify: (Throwable) -> PersistenceFailureCode? = { null }, + checkpointDiagnosticObserver: PersistenceFailureDiagnosticObserver? = null, + block: suspend () -> T, +): T = try { + block() +} catch (error: CancellationException) { + // JDBC cancellation/cleanup can attach raw SQLExceptions to the CE graph + // (Statement.cancel, rollback, auto-commit restore, close). Cancellation + // stays cancellation, but a caller-visible CE must not carry raw + // persistence internals. Genuine framework cancellation (a parent + // JobCancellationException, possibly with framework-only children) passes + // through untouched. Only a CE carrying a real non-cancellation child + // (SQLException/IOException from JDBC cleanup) is sanitized: construct a + // fresh fixed-text CE with a cleanup marker and throw it immediately. + // NOTE: no diagnostic delivery on this path — deliverPersistenceFailure ends + // with ensureActive(), which throws under genuine parent cancellation before + // the sanitized CE could be returned. The docs contract is that genuine + // cancellation is rethrown and not emitted as an ordinary persistence + // diagnostic. + if (error.hasUnsafeCancellationDetail()) { + throw sanitizePersistenceCancellation(error) + } + throw error +} catch (error: Throwable) { + error.rethrowIfCancellation() + // Fenced checkpoint DML failures are marked by the lease store so they are + // attributed to the checkpoint store's observer with resourceKind=CHECKPOINT + // (the raw DML failure never reaches the lease channel). + if (error is CheckpointDmlFailure) { + error.rethrowIfCancellation() + val checkpointObserver = checkpointDiagnosticObserver + ?: throw IllegalArgumentException("CheckpointDmlFailure without checkpoint observer", error) + // Corrupt carriers are internal (no persistenceFailureCode); classify + // them explicitly so a corrupt read inside fenced DML stays + // CORRUPTED_DATA -> WorkflowCheckpointCorruptionException, not a generic + // WRITE/DELETE failure (the P2-2 finding). + val code = when (error.raw) { + is CorruptCheckpointException -> PersistenceFailureCode.CORRUPTED_DATA + else -> error.raw.persistenceFailureCode() ?: defaultPersistenceFailureCode(operation) + } + deliverPersistenceFailure( + checkpointObserver, + // Deliver the marker graph (raw cause + any cleanup suppressed + // attached by executeJdbcCancellable) so no diagnostic is lost. + PersistenceFailureDiagnosticEvent(PersistenceResourceKind.CHECKPOINT, operation, code, error), + ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, operation, code) + } + // A lease read phase inside a compound op (CLAIM/RENEW/RELEASE) failed. + // The phase picks the code (READ_FAILED); the outer op keeps its context. + if (error is LeaseReadPhaseFailure) { + error.rethrowIfCancellation() + val code = PersistenceFailureCode.READ_FAILED + deliverPersistenceFailure( + diagnosticObserver, + PersistenceFailureDiagnosticEvent(resourceKind, operation, code, error), + ) + throw safePersistenceFailure(resourceKind, operation, code) + } + // A lease DELETE phase (expired-lease cleanup) failed inside an operation + // whose default would be READ/WRITE_FAILED. The phase picks DELETE_FAILED. + if (error is LeaseDeletePhaseFailure) { + error.rethrowIfCancellation() + val code = PersistenceFailureCode.DELETE_FAILED + deliverPersistenceFailure( + diagnosticObserver, + PersistenceFailureDiagnosticEvent(resourceKind, operation, code, error), + ) + throw safePersistenceFailure(resourceKind, operation, code) + } + if (error.persistenceFailureTrusted()) { + // Never pass the same throwable instance across the boundary: JDBC + // cleanup mutates the primary after construction (addSuppressed). + // Trust only immutable classification, then reconstruct a fresh + // fixed-text instance of the same semantic class. + currentCoroutineContext().ensureActive() + if (error.cause != null || error.suppressed.isNotEmpty()) { + deliverPersistenceFailure( + diagnosticObserver, + PersistenceFailureDiagnosticEvent( + resourceKind, + operation, + error.persistenceFailureCode() ?: defaultPersistenceFailureCode(operation), + error, + ), + ) + } + throw reconstructSafePersistenceFailure(error) + } + + val code = classify(error) ?: error.persistenceFailureCode() + // Internal corrupt carriers carry no persistenceFailureCode; the store's + // classify lambda usually maps them, but the boundary must not depend on + // every call site passing one (the P2-4 finding). + ?: when (error) { + is CorruptCheckpointException, is CorruptStepAttemptException -> PersistenceFailureCode.CORRUPTED_DATA + else -> null + } + ?: defaultPersistenceFailureCode(operation) + deliverPersistenceFailure( + diagnosticObserver, + PersistenceFailureDiagnosticEvent(resourceKind, operation, code, error), + ) + // Untrusted caller-constructed StaleWorkflowLeaseException values keep their + // semantic class (the worker relies on it) but lose any raw text. + if (error is StaleWorkflowLeaseException) throw safeStaleWorkflowLeaseFailure() + throw safePersistenceFailure(resourceKind, operation, code) +}.also { + // Cancellation can arrive while the block runs and still complete normally + // (e.g. a deferred released concurrently with the parent cancel). Parent + // cancellation must win over a normal return — the #223/#224 post-callback + // rule applied to the boundary itself. + currentCoroutineContext().ensureActive() +} + +/** + * Fresh, cause-free, suppressed-free instance of the same semantic class as + * [error], with fixed framework-controlled text. A trusted classification is + * immutable; the throwable that carried it is not. + */ +private fun reconstructSafePersistenceFailure(error: Throwable): RuntimeException { + val code = error.persistenceFailureCode() ?: PersistenceFailureCode.READ_FAILED + val message: String = when (error) { + is WorkflowResumeException -> "Workflow resume failed" + is WorkflowCheckpointConflictException -> "Workflow checkpoint conflict" + is WorkflowLeaseConflictException -> "Workflow lease conflict" + is StaleWorkflowLeaseException -> "Workflow lease is no longer active" + is WorkflowCheckpointCorruptionException -> "Persisted workflow checkpoint is invalid" + is StepAttemptRecordCorruptionException -> "Persisted step-attempt record is invalid" + is WorkflowPersistenceFailureException -> workflowPersistenceFailureMessage(code) + else -> "Workflow persistence failed" + } + val failure: RuntimeException = when (error) { + is WorkflowResumeException -> WorkflowResumeException(message) + is WorkflowCheckpointConflictException -> WorkflowCheckpointConflictException(message) + is WorkflowLeaseConflictException -> WorkflowLeaseConflictException(message) + is StaleWorkflowLeaseException -> StaleWorkflowLeaseException(message) + is WorkflowCheckpointCorruptionException -> WorkflowCheckpointCorruptionException(message) + is StepAttemptRecordCorruptionException -> StepAttemptRecordCorruptionException(message) + else -> WorkflowPersistenceFailureException(message) + } + failure.setPersistenceFailureMetadata(code) + return failure +} + +/** Fresh CancellationException preserving only fixed text and a cleanup marker. */ +private fun sanitizePersistenceCancellation(error: CancellationException): CancellationException { + // Fixed framework text only: a JDBC driver message can embed SQL or paths. + val sanitized = CancellationException("Workflow persistence operation cancelled") + sanitized.addSuppressed(PersistenceCleanupDiagnosticException()) + return sanitized +} + +/** + * True when the throwable graph reachable from [this] (via cause and suppressed, + * cycle-safe) contains any node that is not a [CancellationException]. A + * framework JobCancellationException chain is all-CE and passes through + * untouched; a raw SQLException/IOException nested anywhere in the graph + * (direct or indirect child) is JDBC cleanup contamination and must be + * sanitized. Mirrors the recursive leak-inspection semantics of the tests. + */ +private fun Throwable.hasUnsafeCancellationDetail( +): Boolean = hasUnsafeCancellationDetail(ArrayList()) + +private fun Throwable.hasUnsafeCancellationDetail( + seen: MutableList, +): Boolean { + if (seen.any { it === this }) return false + seen += this + if (this !is CancellationException) return true + return (cause?.hasUnsafeCancellationDetail(seen) == true) || + suppressed.any { it.hasUnsafeCancellationDetail(seen) } +} + +/** + * Fixed-text marker for persistence-cleanup diagnostics attached to a sanitized + * CancellationException. Deliberately distinct from the process-cleanup marker + * so a cancelled JDBC persistence operation never mentions "Process cleanup". + */ +internal class PersistenceCleanupDiagnosticException : RuntimeException( + "Persistence cleanup had diagnostics", +) + +private fun workflowPersistenceFailureMessage(code: PersistenceFailureCode): String = when (code) { + PersistenceFailureCode.READ_FAILED -> "Workflow persistence read failed" + PersistenceFailureCode.WRITE_FAILED -> "Workflow persistence write failed" + PersistenceFailureCode.DELETE_FAILED -> "Workflow persistence delete failed" + PersistenceFailureCode.LIST_FAILED -> "Workflow persistence list failed" + PersistenceFailureCode.CONFLICT -> "Workflow persistence conflict" + PersistenceFailureCode.CORRUPTED_DATA -> "Workflow persistence data is invalid" +} + +internal fun checkpointDiagnosticObserver(store: WorkflowCheckpointStore): PersistenceFailureDiagnosticObserver = when (store) { + is FileWorkflowCheckpointStore -> store.persistenceFailureDiagnosticObserver + is MarkdownWorkflowCheckpointStore -> store.persistenceFailureDiagnosticObserver + is JdbcWorkflowCheckpointStore -> store.persistenceFailureDiagnosticObserver + is InMemoryWorkflowCheckpointStore -> store.persistenceFailureDiagnosticObserver + else -> NoOpPersistenceFailureDiagnosticObserver +} + +private fun Throwable.persistenceFailureTrusted(): Boolean = when (this) { + is WorkflowResumeException -> safeFactoryTrusted + is WorkflowCheckpointConflictException -> safeFactoryTrusted + is WorkflowCheckpointCorruptionException -> safeFactoryTrusted + is WorkflowLeaseConflictException -> safeFactoryTrusted + is StaleWorkflowLeaseException -> safeFactoryTrusted + is StepAttemptRecordCorruptionException -> safeFactoryTrusted + is WorkflowPersistenceFailureException -> safeFactoryTrusted + else -> false +} + +private fun Throwable.persistenceFailureCode(): PersistenceFailureCode? = when (this) { + is WorkflowCheckpointConflictException, is WorkflowLeaseConflictException, is StaleWorkflowLeaseException -> + PersistenceFailureCode.CONFLICT + is WorkflowCheckpointCorruptionException, is StepAttemptRecordCorruptionException -> PersistenceFailureCode.CORRUPTED_DATA + is WorkflowResumeException -> failureCode + is WorkflowPersistenceFailureException -> failureCode + else -> null +} + +/** + * Phase-aware default classification: the high-level operation stays + * [PersistenceOperation], the failure code names the failing storage phase. + * Compound operations (CLAIM = read-then-write, RELEASE = delete, etc.) default + * to their dominant mutating phase; stores override with the [classify] lambda + * when the actual failing phase differs. + */ +private fun defaultPersistenceFailureCode(operation: PersistenceOperation): PersistenceFailureCode = when (operation) { + PersistenceOperation.LOAD -> PersistenceFailureCode.READ_FAILED + PersistenceOperation.LIST -> PersistenceFailureCode.LIST_FAILED + PersistenceOperation.DELETE, PersistenceOperation.RELEASE -> PersistenceFailureCode.DELETE_FAILED + PersistenceOperation.SAVE, PersistenceOperation.COMPARE_AND_SET, + PersistenceOperation.CLAIM, PersistenceOperation.RENEW, + -> PersistenceFailureCode.WRITE_FAILED +} + +private suspend fun deliverPersistenceFailure( + observer: PersistenceFailureDiagnosticObserver, + event: PersistenceFailureDiagnosticEvent, +) { + try { + observer.onFailure(event) + } catch (e: CancellationException) { + currentCoroutineContext().ensureActive() + } catch (e: Throwable) { + e.rethrowIfCancellation() + } + currentCoroutineContext().ensureActive() +} diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/StepAttemptRecord.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/StepAttemptRecord.kt index 7d2861da..e429e483 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/StepAttemptRecord.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/StepAttemptRecord.kt @@ -123,7 +123,12 @@ interface StepAttemptRecordStore { class StepAttemptRecordCorruptionException( message: String, cause: Throwable? = null, -) : RuntimeException(message, cause) +) : RuntimeException(message, cause) { + var failureCode: PersistenceFailureCode? = null + internal set + var safeFactoryTrusted: Boolean = false + internal set +} class NonReplayableStepStateUnknownException( val runId: String, diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/StepAttemptRecordCodec.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/StepAttemptRecordCodec.kt index 1e286444..9e98fd9c 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/StepAttemptRecordCodec.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/StepAttemptRecordCodec.kt @@ -33,7 +33,7 @@ internal object StepAttemptRecordCodec { val properties = Properties().apply { load(payload.reader()) } val schemaVersion = properties.requireCodecProperty("schemaVersion") if (schemaVersion != SCHEMA_VERSION) { - corrupt("Unsupported step-attempt schema version '$schemaVersion'") + corrupt("Unsupported step-attempt schema version", schemaVersion) } StepAttemptRecord( runId = decodeString(properties.requireCodecProperty("runId"), "runId"), @@ -53,10 +53,10 @@ internal object StepAttemptRecordCodec { resolutionAction = properties.decodeNullableString("resolutionAction")?.let(::decodeResolutionAction), approvedIdempotencyKey = properties.decodeNullableString("approvedIdempotencyKey"), ) - } catch (error: StepAttemptRecordCorruptionException) { + } catch (error: CorruptStepAttemptException) { throw error } catch (error: Exception) { - throw StepAttemptRecordCorruptionException("Invalid step-attempt record", error) + throw CorruptStepAttemptException("Persisted step-attempt record is invalid", payload, error) } fun fingerprint(record: StepAttemptRecord): String = MessageDigest.getInstance("SHA-256") @@ -65,7 +65,7 @@ internal object StepAttemptRecordCodec { fun requireValidFingerprint(record: StepAttemptRecord, storedFingerprint: String, context: String) { if (storedFingerprint != fingerprint(record)) { - throw StepAttemptRecordCorruptionException("Step-attempt fingerprint mismatch in $context") + throw CorruptStepAttemptException("Persisted step-attempt record is invalid", context) } } @@ -104,11 +104,12 @@ internal object StepAttemptRecordCodec { private fun decodeString(value: String, field: String): String = try { String(Base64.getUrlDecoder().decode(value), StandardCharsets.UTF_8) } catch (error: IllegalArgumentException) { - throw StepAttemptRecordCorruptionException("Invalid encoded value for '$field'", error) + throw CorruptStepAttemptException("Invalid encoded value for '$field'", value, error) } private fun > enumValue(value: String, label: String, entries: List): E = - entries.firstOrNull { it.name == value } ?: corrupt("Unknown $label: '$value'") + entries.firstOrNull { it.name == value } ?: corrupt("Unknown $label", value) - private fun corrupt(message: String): Nothing = throw StepAttemptRecordCorruptionException(message) + private fun corrupt(message: String, rawPayload: String? = null): Nothing = + throw CorruptStepAttemptException(message, rawPayload) } diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/TramaiWorker.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/TramaiWorker.kt index be6e96ef..87ead8f5 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/TramaiWorker.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/TramaiWorker.kt @@ -108,7 +108,12 @@ object NoOpTramaiWorkerObserver : TramaiWorkerObserver class StaleWorkflowLeaseException( message: String, -) : RuntimeException(message) +) : RuntimeException(message) { + var failureCode: PersistenceFailureCode? = null + internal set + var safeFactoryTrusted: Boolean = false + internal set +} internal data class WorkerWorkflowBinding( val workflow: Workflow, @@ -347,7 +352,10 @@ class TramaiWorker( throw error } catch (error: Throwable) { error.rethrowIfCancellation() - observability.onPollFailed(config.workerId, error) + observability.onPollFailed( + config.workerId, + safeWorkerObservableFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.LIST, error), + ) delay(maxOf(100L, config.pollIntervalMillis)) } } @@ -721,7 +729,11 @@ class TramaiWorker( throw error } catch (error: Throwable) { error.rethrowIfCancellation() - observability.onLeaseRenewalFailed(handle.workflowId, config.workerId, error) + observability.onLeaseRenewalFailed( + handle.workflowId, + config.workerId, + safeWorkerObservableFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, error), + ) nextDelayMillis = maxOf(50L, interval / 2) continue } @@ -739,7 +751,11 @@ class TramaiWorker( observability.onLeaseReleased(handle.workflowId, config.workerId) } catch (error: Throwable) { error.rethrowIfCancellation() - observability.onLeaseReleaseFailed(handle.workflowId, config.workerId, error) + observability.onLeaseReleaseFailed( + handle.workflowId, + config.workerId, + safeWorkerObservableFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RELEASE, error), + ) } } @@ -942,16 +958,30 @@ private class ExecutionTracker( val attempt = synchronized(monitor) { activeAttempt?.takeIf { it.stepName == stepName } } ?: return + // The persisted outputSummary must not carry raw persistence internals + // (paths, SQL, payloads). Sanitize persistence-family failures; user + // step-execution errors are not persistence internals and keep their + // real message so the durable record stays diagnostically useful. + val observableFailure = when { + !error.isPersistenceFamilyFailure() -> error + else -> safeWorkerObservableFailure(PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.SAVE, error) + } val failed = attempt.copy( status = StepAttemptStatus.FAILED, completedAt = System.currentTimeMillis(), - outputSummary = summarize(error), + outputSummary = summarize(observableFailure), ) stepAttemptStore.updateStepAttempt(failed) synchronized(monitor) { activeAttempt = null } - observability.onStepAttemptFailed(failed.runId, failed.stepName, failed.attemptId, workerId, error) + observability.onStepAttemptFailed( + failed.runId, + failed.stepName, + failed.attemptId, + workerId, + observableFailure, + ) } suspend fun failActiveAttempt(error: Throwable) { diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/WorkflowLease.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/WorkflowLease.kt index 682584b9..6fcc576f 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/WorkflowLease.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/WorkflowLease.kt @@ -74,13 +74,29 @@ interface WorkflowLeaseCheckpointFence { */ class WorkflowLeaseConflictException( message: String, -) : RuntimeException(message) +) : RuntimeException(message) { + var failureCode: PersistenceFailureCode? = null + internal set + var safeFactoryTrusted: Boolean = false + internal set +} /** * Simple in-memory lease store for tests and lightweight local use. */ class InMemoryWorkflowLeaseStore( private val clockMillis: () -> Long = System::currentTimeMillis, ) : WorkflowLeaseStore, WorkflowLeaseCheckpointFence, WorkerRegistryStore { + var persistenceFailureDiagnosticObserver: PersistenceFailureDiagnosticObserver = + NoOpPersistenceFailureDiagnosticObserver + internal set + + constructor( + clockMillis: () -> Long, + observer: PersistenceFailureDiagnosticObserver, + ) : this(clockMillis) { + persistenceFailureDiagnosticObserver = observer + } + private val leases = linkedMapOf() private val workers = linkedMapOf() private val monitor = Any() @@ -89,11 +105,13 @@ class InMemoryWorkflowLeaseStore( override suspend fun currentLease( workflowName: String, workflowId: String, - ): WorkflowLease? = leaseMutex.withLock { + ): WorkflowLease? = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.LOAD, persistenceFailureDiagnosticObserver, + ) { leaseMutex.withLock { synchronized(monitor) { activeLease(workflowName, workflowId) } - } + } } override suspend fun claim( workflowName: String, @@ -101,14 +119,14 @@ class InMemoryWorkflowLeaseStore( ownerId: String, checkpointRevision: Long?, leaseDurationMillis: Long, - ): WorkflowLease = leaseMutex.withLock { + ): WorkflowLease = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.CLAIM, persistenceFailureDiagnosticObserver, + ) { leaseMutex.withLock { synchronized(monitor) { val key = LeaseKey(workflowName, workflowId) val existing = leases[key] if (existing != null && !isExpired(existing)) { - throw WorkflowLeaseConflictException( - "Workflow '$workflowName' and workflowId='$workflowId' is already leased by owner '${existing.ownerId}' until ${existing.expiresAtEpochMillis}", - ) + throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.CLAIM, PersistenceFailureCode.CONFLICT) } val now = clockMillis() val lease = WorkflowLease( @@ -123,29 +141,25 @@ class InMemoryWorkflowLeaseStore( leases[key] = lease lease } - } + } } override suspend fun renew( lease: WorkflowLease, checkpointRevision: Long?, leaseDurationMillis: Long, - ): WorkflowLease = leaseMutex.withLock { + ): WorkflowLease = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, persistenceFailureDiagnosticObserver, + ) { leaseMutex.withLock { synchronized(monitor) { val key = LeaseKey(lease.workflowName, lease.workflowId) val existing = leases[key] - ?: throw WorkflowLeaseConflictException( - "Workflow '${lease.workflowName}' and workflowId='${lease.workflowId}' has no active lease to renew", - ) + ?: throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, PersistenceFailureCode.CONFLICT) if (isExpired(existing)) { leases.remove(key) - throw WorkflowLeaseConflictException( - "Workflow '${lease.workflowName}' and workflowId='${lease.workflowId}' lease has expired before renewal", - ) + throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, PersistenceFailureCode.CONFLICT) } if (existing.leaseId != lease.leaseId || existing.ownerId != lease.ownerId) { - throw WorkflowLeaseConflictException( - "Workflow '${lease.workflowName}' and workflowId='${lease.workflowId}' is leased by owner '${existing.ownerId}', not '${lease.ownerId}'", - ) + throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, PersistenceFailureCode.CONFLICT) } val now = clockMillis() val renewed = existing.copy( @@ -155,25 +169,23 @@ class InMemoryWorkflowLeaseStore( leases[key] = renewed renewed } - } + } } override suspend fun release(lease: WorkflowLease) { - leaseMutex.withLock { + persistenceBoundary(PersistenceResourceKind.LEASE, PersistenceOperation.RELEASE, persistenceFailureDiagnosticObserver) { leaseMutex.withLock { synchronized(monitor) { val key = LeaseKey(lease.workflowName, lease.workflowId) - val existing = leases[key] ?: return + val existing = leases[key] ?: return@persistenceBoundary Unit if (isExpired(existing)) { leases.remove(key) - return + return@persistenceBoundary Unit } if (existing.leaseId != lease.leaseId || existing.ownerId != lease.ownerId) { - throw WorkflowLeaseConflictException( - "Workflow '${lease.workflowName}' and workflowId='${lease.workflowId}' is leased by owner '${existing.ownerId}', not '${lease.ownerId}'", - ) + throw safePersistenceFailure(PersistenceResourceKind.LEASE, PersistenceOperation.RELEASE, PersistenceFailureCode.CONFLICT) } leases.remove(key) } - } + } } } override suspend fun saveCheckpointIfLeaseOwner( @@ -181,13 +193,15 @@ class InMemoryWorkflowLeaseStore( checkpoint: WorkflowCheckpoint, expectedRevision: Long?, expectedLease: WorkflowLease, - ): WorkflowCheckpoint = leaseMutex.withLock { + ): WorkflowCheckpoint = persistenceBoundary( + PersistenceResourceKind.LEASE, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + ) { leaseMutex.withLock { val current = synchronized(monitor) { activeLease(expectedLease.workflowName, expectedLease.workflowId) } validateExpectedLease(expectedLease, current) checkpointStore.save(checkpoint, expectedRevision) - } + } } override suspend fun deleteCheckpointIfLeaseOwner( checkpointStore: WorkflowCheckpointStore, @@ -196,13 +210,13 @@ class InMemoryWorkflowLeaseStore( expectedRevision: Long?, expectedLease: WorkflowLease, ) { - leaseMutex.withLock { + persistenceBoundary(PersistenceResourceKind.LEASE, PersistenceOperation.DELETE, persistenceFailureDiagnosticObserver) { leaseMutex.withLock { val current = synchronized(monitor) { activeLease(workflowName, workflowId) } validateExpectedLease(expectedLease, current) checkpointStore.delete(workflowName, workflowId, expectedRevision) - } + } } } override suspend fun registerWorker( @@ -212,7 +226,7 @@ class InMemoryWorkflowLeaseStore( capabilityLabels: Set, host: String, ) { - synchronized(monitor) { + persistenceBoundary(PersistenceResourceKind.WORKER_REGISTRY, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver) { synchronized(monitor) { val now = clockMillis() val existing = workers[workerId] workers[workerId] = WorkerRegistryRecord( @@ -224,35 +238,48 @@ class InMemoryWorkflowLeaseStore( registeredAtEpochMillis = existing?.registeredAtEpochMillis ?: now, lastHeartbeatEpochMillis = now, ) - } + } } } override suspend fun updateHeartbeat(workerId: String) { + // clockMillis() can throw (user-supplied) — sanitize that failure. The + // existence check and update must stay ONE atomic monitor operation + // (P2-1: no unsynchronized LinkedHashMap read, no check-then-act race + // with unregisterWorker); the unknown-worker IllegalArgumentException + // surfaces from inside the monitor, outside any persistence boundary. + val now = persistenceBoundary( + PersistenceResourceKind.WORKER_REGISTRY, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + ) { clockMillis() } synchronized(monitor) { val existing = workers[workerId] ?: throw IllegalArgumentException("Worker '$workerId' is not registered") - workers[workerId] = existing.copy(lastHeartbeatEpochMillis = clockMillis()) + workers[workerId] = existing.copy(lastHeartbeatEpochMillis = now) } } override suspend fun unregisterWorker(workerId: String) { - synchronized(monitor) { + persistenceBoundary(PersistenceResourceKind.WORKER_REGISTRY, PersistenceOperation.DELETE, persistenceFailureDiagnosticObserver) { synchronized(monitor) { workers.remove(workerId) - } + } } } - override suspend fun listActiveWorkers(): List = synchronized(monitor) { + override suspend fun listActiveWorkers(): List = persistenceBoundary( + PersistenceResourceKind.WORKER_REGISTRY, PersistenceOperation.LIST, persistenceFailureDiagnosticObserver, + ) { synchronized(monitor) { workers.values.sortedBy { it.workerId } - } + } } - override suspend fun listStaleWorkers(staleThresholdMillis: Long): List = synchronized(monitor) { - require(staleThresholdMillis >= 0) { - "staleThresholdMillis must be zero or greater" - } - val cutoff = clockMillis() - staleThresholdMillis - workers.values - .filter { it.lastHeartbeatEpochMillis <= cutoff } - .sortedBy { it.workerId } + override suspend fun listStaleWorkers(staleThresholdMillis: Long): List { + // Negative threshold is a caller error, not a persistence failure (P2-3). + require(staleThresholdMillis >= 0) { "staleThresholdMillis must be zero or greater" } + return persistenceBoundary( + PersistenceResourceKind.WORKER_REGISTRY, PersistenceOperation.LIST, persistenceFailureDiagnosticObserver, + ) { synchronized(monitor) { + val cutoff = clockMillis() - staleThresholdMillis + workers.values + .filter { it.lastHeartbeatEpochMillis <= cutoff } + .sortedBy { it.workerId } + } } } private fun isExpired(lease: WorkflowLease): Boolean = clockMillis() >= lease.expiresAtEpochMillis @@ -275,14 +302,10 @@ class InMemoryWorkflowLeaseStore( current: WorkflowLease?, ) { if (current == null) { - throw StaleWorkflowLeaseException( - "Workflow '${expectedLease.workflowName}' and workflowId='${expectedLease.workflowId}' lease '${expectedLease.leaseId}' is no longer active", - ) + throw safeStaleWorkflowLeaseFailure() } if (current.leaseId != expectedLease.leaseId || current.ownerId != expectedLease.ownerId) { - throw StaleWorkflowLeaseException( - "Workflow '${expectedLease.workflowName}' and workflowId='${expectedLease.workflowId}' is now fenced by lease '${current.leaseId}' owned by '${current.ownerId}'", - ) + throw safeStaleWorkflowLeaseFailure() } } } diff --git a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/WorkflowPersistence.kt b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/WorkflowPersistence.kt index c23c59a4..8517e705 100644 --- a/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/WorkflowPersistence.kt +++ b/tramai-orchestration/src/main/kotlin/dev/tramai/orchestration/WorkflowPersistence.kt @@ -96,16 +96,29 @@ interface WorkflowCheckpointStore { expectedRevision: Long, record: WorkflowRecoveryRecord, ): WorkflowCheckpoint { - val current = load(workflowName, workflowId) - ?: throw WorkflowCheckpointConflictException( - "Cannot require recovery for workflow '$workflowName' and workflowId='$workflowId': checkpoint does not exist for expected revision $expectedRevision", + // Phase-aware boundaries: a load failure is READ_FAILED, a save failure + // is WRITE_FAILED — the outer operation (SAVE) must not mislabel the + // load phase (the Copilot finding). + val current = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.LOAD, + checkpointDiagnosticObserver(this), + ) { load(workflowName, workflowId) } + ?: throw safePersistenceFailure( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.SAVE, + PersistenceFailureCode.CONFLICT, ) - return save( - checkpoint = current.copy( - recoveryState = WorkflowRecoveryState.Required(record), - ), - expectedRevision = expectedRevision, - ) + return persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.SAVE, + checkpointDiagnosticObserver(this), + ) { + save( + checkpoint = current.copy(recoveryState = WorkflowRecoveryState.Required(record)), + expectedRevision = expectedRevision, + ) + } } /** @@ -121,14 +134,28 @@ interface WorkflowCheckpointStore { workflowId: String, expectedRevision: Long, ): WorkflowCheckpoint { - val current = load(workflowName, workflowId) - ?: throw WorkflowCheckpointConflictException( - "Cannot clear recovery for workflow '$workflowName' and workflowId='$workflowId': checkpoint does not exist for expected revision $expectedRevision", + // Phase-aware boundaries: a load failure is READ_FAILED, a save failure + // is WRITE_FAILED (same split as requireRecovery). + val current = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.LOAD, + checkpointDiagnosticObserver(this), + ) { load(workflowName, workflowId) } + ?: throw safePersistenceFailure( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.SAVE, + PersistenceFailureCode.CONFLICT, ) - return save( - checkpoint = current.copy(recoveryState = WorkflowRecoveryState.Normal), - expectedRevision = expectedRevision, - ) + return persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.SAVE, + checkpointDiagnosticObserver(this), + ) { + save( + checkpoint = current.copy(recoveryState = WorkflowRecoveryState.Normal), + expectedRevision = expectedRevision, + ) + } } } @@ -168,13 +195,23 @@ data class WorkflowPersistence( */ class WorkflowResumeException( message: String, -) : RuntimeException(message) +) : RuntimeException(message) { + var failureCode: PersistenceFailureCode? = null + internal set + var safeFactoryTrusted: Boolean = false + internal set +} /** * Raised when a checkpoint write or delete is attempted with stale revision state. */ class WorkflowCheckpointConflictException( message: String, -) : RuntimeException(message) +) : RuntimeException(message) { + var failureCode: PersistenceFailureCode? = null + internal set + var safeFactoryTrusted: Boolean = false + internal set +} /** * Raised when persisted checkpoint data is present but malformed or corrupted. @@ -186,7 +223,12 @@ class WorkflowCheckpointConflictException( */ class WorkflowCheckpointCorruptionException( message: String, -) : RuntimeException(message) +) : RuntimeException(message) { + var failureCode: PersistenceFailureCode? = null + internal set + var safeFactoryTrusted: Boolean = false + internal set +} /** * Shared recovery-state codec for store implementations. @@ -217,29 +259,22 @@ internal fun decodeRecoveryState(payload: String?): WorkflowRecoveryState { if (payload.isNullOrBlank()) return WorkflowRecoveryState.Normal val map = try { decodeMetadata(payload) - } catch (error: IllegalArgumentException) { - throw WorkflowCheckpointCorruptionException( - "Persisted recovery state is not a valid encoded payload: '$payload'", - ) + } catch (error: CorruptCheckpointException) { + throw error + } catch (error: Throwable) { + throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", payload) } val reason = map["reason"]?.let { name -> WorkflowRecoveryReason.entries.firstOrNull { it.name == name } - } ?: throw WorkflowCheckpointCorruptionException( - "Persisted recovery state is missing or has an invalid 'reason' field: '${map["reason"]}'", - ) - val stepName = map["stepName"] ?: throw WorkflowCheckpointCorruptionException( - "Persisted recovery state is missing 'stepName'", - ) - val attemptId = map["attemptId"] ?: throw WorkflowCheckpointCorruptionException( - "Persisted recovery state is missing 'attemptId'", - ) - val priorWorkerId = map["priorWorkerId"] ?: throw WorkflowCheckpointCorruptionException( - "Persisted recovery state is missing 'priorWorkerId'", - ) + } ?: throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", payload) + val stepName = map["stepName"] + ?: throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", payload) + val attemptId = map["attemptId"] + ?: throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", payload) + val priorWorkerId = map["priorWorkerId"] + ?: throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", payload) val detectedAt = map["detectedAtEpochMillis"]?.toLongOrNull() - ?: throw WorkflowCheckpointCorruptionException( - "Persisted recovery state has invalid 'detectedAtEpochMillis': '${map["detectedAtEpochMillis"]}'", - ) + ?: throw CorruptCheckpointException("Persisted workflow checkpoint is invalid", payload) return WorkflowRecoveryState.Required( WorkflowRecoveryRecord( reason = reason, @@ -255,88 +290,97 @@ internal fun decodeRecoveryState(payload: String?): WorkflowRecoveryState { /** * Simple in-memory checkpoint store for tests and lightweight local use. */ -class InMemoryWorkflowCheckpointStore : WorkflowCheckpointStore, WorkflowCheckpointCatalog, StepAttemptRecordStore { +class InMemoryWorkflowCheckpointStore : + WorkflowCheckpointStore, WorkflowCheckpointCatalog, StepAttemptRecordStore { + var persistenceFailureDiagnosticObserver: PersistenceFailureDiagnosticObserver = + NoOpPersistenceFailureDiagnosticObserver + internal set + + constructor() + + constructor(observer: PersistenceFailureDiagnosticObserver) : this() { + persistenceFailureDiagnosticObserver = observer + } private val checkpoints = linkedMapOf() private val stepAttempts = linkedMapOf() private val monitor = Any() override suspend fun load( workflowName: String, workflowId: String, - ): WorkflowCheckpoint? = synchronized(monitor) { - checkpoints[CheckpointKey(workflowName, workflowId)] - } + ): WorkflowCheckpoint? = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.LOAD, persistenceFailureDiagnosticObserver, + ) { synchronized(monitor) { checkpoints[CheckpointKey(workflowName, workflowId)] } } override suspend fun save( checkpoint: WorkflowCheckpoint, expectedRevision: Long?, - ): WorkflowCheckpoint = synchronized(monitor) { + ): WorkflowCheckpoint = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + ) { synchronized(monitor) { val key = CheckpointKey(checkpoint.workflowName, checkpoint.workflowId) val existing = checkpoints[key] if (expectedRevision == null && existing != null) { - throw WorkflowCheckpointConflictException( - "Checkpoint for workflow '${checkpoint.workflowName}' and workflowId='${checkpoint.workflowId}' already exists at revision ${existing.revision}", - ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, PersistenceFailureCode.CONFLICT) } if (expectedRevision != null && existing == null) { - throw WorkflowCheckpointConflictException( - "Checkpoint for workflow '${checkpoint.workflowName}' and workflowId='${checkpoint.workflowId}' does not exist for expected revision $expectedRevision", - ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, PersistenceFailureCode.CONFLICT) } if (expectedRevision != null && existing != null && existing.revision != expectedRevision) { - throw WorkflowCheckpointConflictException( - "Checkpoint for workflow '${checkpoint.workflowName}' and workflowId='${checkpoint.workflowId}' is at revision ${existing.revision}, not expected revision $expectedRevision", - ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, PersistenceFailureCode.CONFLICT) } val persisted = checkpoint.copy( revision = (existing?.revision ?: 0) + 1, ) checkpoints[key] = persisted persisted - } + } } override suspend fun delete( workflowName: String, workflowId: String, expectedRevision: Long?, ) { - synchronized(monitor) { + persistenceBoundary(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.DELETE, persistenceFailureDiagnosticObserver) { synchronized(monitor) { val key = CheckpointKey(workflowName, workflowId) val existing = checkpoints[key] if (expectedRevision != null && existing == null) { - throw WorkflowCheckpointConflictException( - "Checkpoint for workflow '$workflowName' and workflowId='$workflowId' does not exist for expected revision $expectedRevision", - ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.DELETE, PersistenceFailureCode.CONFLICT) } if (expectedRevision != null && existing != null && existing.revision != expectedRevision) { - throw WorkflowCheckpointConflictException( - "Checkpoint for workflow '$workflowName' and workflowId='$workflowId' is at revision ${existing.revision}, not expected revision $expectedRevision", - ) + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.DELETE, PersistenceFailureCode.CONFLICT) } checkpoints.remove(key) - } + } } } - override suspend fun listCheckpoints(): List = synchronized(monitor) { - checkpoints.values.toList() - } + override suspend fun listCheckpoints(): List = persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, PersistenceOperation.LIST, persistenceFailureDiagnosticObserver, + ) { synchronized(monitor) { checkpoints.values.toList() } } - override suspend fun recordStepAttempt(record: StepAttemptRecord): StepAttemptRecord = synchronized(monitor) { + override suspend fun recordStepAttempt(record: StepAttemptRecord): StepAttemptRecord = persistenceBoundary( + PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + classify = ::classifyStepAttemptFailure, + ) { synchronized(monitor) { val key = AttemptKey(record.runId, record.stepName, record.attemptId) stepAttempts[key] = record record - } + } } - override suspend fun updateStepAttempt(record: StepAttemptRecord): StepAttemptRecord = synchronized(monitor) { + override suspend fun updateStepAttempt(record: StepAttemptRecord): StepAttemptRecord = persistenceBoundary( + PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.SAVE, persistenceFailureDiagnosticObserver, + classify = ::classifyStepAttemptFailure, + ) { synchronized(monitor) { val key = AttemptKey(record.runId, record.stepName, record.attemptId) - require(stepAttempts.containsKey(key)) { - "Step attempt '${record.attemptId}' for run '${record.runId}' and step '${record.stepName}' does not exist" - } + require(stepAttempts.containsKey(key)) { "Step attempt does not exist" } stepAttempts[key] = record record - } + } } override suspend fun compareAndSetStepAttempt( expected: StepAttemptRecord, updated: StepAttemptRecord, - ): Boolean = synchronized(monitor) { + ): Boolean = persistenceBoundary( + PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.COMPARE_AND_SET, persistenceFailureDiagnosticObserver, + classify = ::classifyStepAttemptFailure, + ) { synchronized(monitor) { val expectedKey = AttemptKey(expected.runId, expected.stepName, expected.attemptId) val updatedKey = AttemptKey(updated.runId, updated.stepName, updated.attemptId) if (expectedKey != updatedKey) { @@ -347,23 +391,29 @@ class InMemoryWorkflowCheckpointStore : WorkflowCheckpointStore, WorkflowCheckpo stepAttempts[expectedKey] = updated true } - } + } } override suspend fun latestStepAttempt( runId: String, stepName: String, - ): StepAttemptRecord? = synchronized(monitor) { + ): StepAttemptRecord? = persistenceBoundary( + PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.LOAD, persistenceFailureDiagnosticObserver, + classify = ::classifyStepAttemptFailure, + ) { synchronized(monitor) { stepAttempts.values .asSequence() .filter { it.runId == runId && it.stepName == stepName } .maxWithOrNull(compareBy({ it.startedAt }, { it.attemptId })) - } + } } - override suspend fun listStepAttempts(runId: String): List = synchronized(monitor) { + override suspend fun listStepAttempts(runId: String): List = persistenceBoundary( + PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.LIST, persistenceFailureDiagnosticObserver, + classify = ::classifyStepAttemptFailure, + ) { synchronized(monitor) { stepAttempts.values .filter { it.runId == runId } .sortedWith(compareBy({ it.startedAt }, { it.stepName }, { it.attemptId })) - } + } } } private data class CheckpointKey( val workflowName: String, diff --git a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/BinaryCompatibilityFixtureTest.kt b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/BinaryCompatibilityFixtureTest.kt index 0950054c..7f458959 100644 --- a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/BinaryCompatibilityFixtureTest.kt +++ b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/BinaryCompatibilityFixtureTest.kt @@ -35,7 +35,7 @@ class BinaryCompatibilityFixtureTest { .invoke(null) as String } - (1..6).forEach { marker -> + (1..17).forEach { marker -> assertThat(markers).contains("FIXTURE_OK_$marker") } assertThat(markers).doesNotContain("FIXTURE_FAIL") diff --git a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/JdbcWorkflowPersistenceCancellationContractTest.kt b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/JdbcWorkflowPersistenceCancellationContractTest.kt index d918ce97..2f51689c 100644 --- a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/JdbcWorkflowPersistenceCancellationContractTest.kt +++ b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/JdbcWorkflowPersistenceCancellationContractTest.kt @@ -411,7 +411,8 @@ class JdbcWorkflowPersistenceCancellationContractTest { } catch (e: WorkflowLeaseConflictException) { e } assertThat(leaseConflictThrown) .isInstanceOf(WorkflowLeaseConflictException::class.java) - .hasMessageContaining("owner-a") + .hasMessage("Workflow lease conflict") + .hasNoCause() leaseStore.release(leaseStore.currentLease(cp.workflowName, cp.workflowId)!!) val freshLease = leaseStore.claim( diff --git a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/PersistenceSafeFailureBoundaryTest.kt b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/PersistenceSafeFailureBoundaryTest.kt new file mode 100644 index 00000000..6c13f31b --- /dev/null +++ b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/PersistenceSafeFailureBoundaryTest.kt @@ -0,0 +1,1131 @@ +package dev.tramai.orchestration + +import java.io.IOException +import java.io.PrintWriter +import java.nio.file.Files +import java.nio.file.Path +import java.sql.Connection +import java.sql.DatabaseMetaData +import java.sql.PreparedStatement +import java.sql.SQLException +import java.util.logging.Logger +import javax.sql.DataSource +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancel +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.catchThrowable +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.io.TempDir + +/** + * Adversarial leak tests for the persistence safe-failure boundary (PR #225). + * + * The invariant: filesystem paths, SQL text, persisted payloads, and arbitrary + * exception messages never cross from persistence internals into public + * exceptions, ordinary observers, or logs. The original failure reaches only + * [PersistenceFailureDiagnosticObserver]; cancellation stays cancellation. + * + * Secret fixtures use the value `fixture-sentinel-7f3c` (GitGuardian-safe: + * no credential-shaped keys). Assertions scan message + cause + EVERY + * suppressed throwable + observer events. + */ +class PersistenceSafeFailureBoundaryTest { + + @TempDir + lateinit var tempDir: Path + + private companion object { + const val SECRET_SQL = "UPDATE tramai_workflow SET state_payload = 'fixture-sentinel-7f3c'" + const val SECRET_PATH = "/home/customer/acme/private/fixture-sentinel-7f3c/workflow" + const val SECRET_PAYLOAD = "fixture-sentinel-7f3c-persisted-state" + const val SECRET_VALUE = "fixture-sentinel-7f3c-value" + } + + private class RecordingPersistenceObserver : PersistenceFailureDiagnosticObserver { + val events = mutableListOf() + var throwAfterRecording: Throwable? = null + var throwFakeCancellation: Boolean = false + + override suspend fun onFailure(event: PersistenceFailureDiagnosticEvent) { + events += event + if (throwFakeCancellation) throw CancellationException("synthetic observer cancellation") + throwAfterRecording?.let { throw it } + } + } + + private fun assertNoSecret(t: Throwable?, secret: String, seen: MutableSet = mutableSetOf()) { + if (t == null || !seen.add(t)) return + assertThat(t.message).doesNotContain(secret) + assertThat(t.toString()).doesNotContain(secret) + assertNoSecret(t.cause, secret, seen) + t.suppressed.forEach { assertNoSecret(it, secret, seen) } + } + + /** DataSource whose Connection throws a seeded SQLException on any statement. */ + private class SeededSqlDataSource( + private val failure: SQLException, + ) : DataSource { + override fun getConnection(): Connection = ThrowingConnection(failure) + override fun getConnection(username: String?, password: String?): Connection = ThrowingConnection(failure) + override fun getLogWriter(): PrintWriter? = null + override fun setLogWriter(out: PrintWriter?) = Unit + override fun setLoginTimeout(seconds: Int) = Unit + override fun getLoginTimeout(): Int = 0 + override fun getParentLogger(): Logger = Logger.getGlobal() + override fun unwrap(iface: Class?): T = throw SQLException("Unsupported") + override fun isWrapperFor(iface: Class<*>?): Boolean = false + } + + private class ThrowingConnection( + private val failure: SQLException, + ) : Connection { + override fun prepareStatement(sql: String): PreparedStatement = throw failure + override fun prepareStatement(sql: String, autoGeneratedKeys: Int): PreparedStatement = throw failure + override fun prepareStatement(sql: String, columnIndexes: IntArray?): PreparedStatement = throw failure + override fun prepareStatement(sql: String, columnNames: Array?): PreparedStatement = throw failure + override fun prepareStatement(sql: String, resultSetType: Int, resultSetConcurrency: Int): PreparedStatement = throw failure + override fun prepareStatement(sql: String, resultSetType: Int, resultSetConcurrency: Int, resultSetHoldability: Int): PreparedStatement = throw failure + override fun prepareCall(sql: String): java.sql.CallableStatement = throw failure + override fun prepareCall(sql: String, resultSetType: Int, resultSetConcurrency: Int): java.sql.CallableStatement = throw failure + override fun prepareCall(sql: String, resultSetType: Int, resultSetConcurrency: Int, resultSetHoldability: Int): java.sql.CallableStatement = throw failure + override fun createStatement(): java.sql.Statement = throw failure + override fun createStatement(resultSetType: Int, resultSetConcurrency: Int): java.sql.Statement = throw failure + override fun createStatement(resultSetType: Int, resultSetConcurrency: Int, resultSetHoldability: Int): java.sql.Statement = throw failure + override fun nativeSQL(sql: String?): String = throw failure + override fun setAutoCommit(autoCommit: Boolean) = Unit + override fun getAutoCommit(): Boolean = true + override fun commit() = Unit + override fun rollback() = Unit + override fun close() = Unit + override fun isClosed(): Boolean = false + override fun setReadOnly(readOnly: Boolean) = Unit + override fun isReadOnly(): Boolean = true + override fun setCatalog(catalog: String?) = Unit + override fun getCatalog(): String? = null + override fun setTransactionIsolation(level: Int) = Unit + override fun getTransactionIsolation(): Int = Connection.TRANSACTION_READ_COMMITTED + override fun getWarnings(): java.sql.SQLWarning? = null + override fun clearWarnings() = Unit + override fun setTypeMap(map: MutableMap>?) = Unit + override fun getTypeMap(): MutableMap> = mutableMapOf() + override fun setHoldability(holdability: Int) = Unit + override fun getHoldability(): Int = java.sql.ResultSet.HOLD_CURSORS_OVER_COMMIT + override fun setSavepoint(): java.sql.Savepoint = throw failure + override fun setSavepoint(name: String?): java.sql.Savepoint = throw failure + override fun rollback(savepoint: java.sql.Savepoint?) = Unit + override fun releaseSavepoint(savepoint: java.sql.Savepoint?) = Unit + override fun createClob(): java.sql.Clob = throw failure + override fun createBlob(): java.sql.Blob = throw failure + override fun createNClob(): java.sql.NClob = throw failure + override fun createSQLXML(): java.sql.SQLXML = throw failure + override fun isValid(timeout: Int): Boolean = true + override fun setClientInfo(name: String?, value: String?) = Unit + override fun setClientInfo(properties: java.util.Properties?) = Unit + override fun getClientInfo(name: String?): String? = null + override fun getClientInfo(): java.util.Properties = java.util.Properties() + override fun createArrayOf(typeName: String?, elements: Array?): java.sql.Array = throw failure + override fun createStruct(typeName: String?, attributes: Array?): java.sql.Struct = throw failure + override fun setSchema(schema: String?) = Unit + override fun getSchema(): String? = null + override fun abort(executor: java.util.concurrent.Executor?) = Unit + override fun setNetworkTimeout(executor: java.util.concurrent.Executor?, milliseconds: Int) = Unit + override fun getNetworkTimeout(): Int = 0 + override fun getMetaData(): DatabaseMetaData = throw failure + override fun unwrap(iface: Class?): T = throw SQLException("Unsupported") + override fun isWrapperFor(iface: Class<*>?): Boolean = false + } + + // --- 1. JDBC secret SQL reaches observer only --- + + @Test + fun `raw jdbc exception containing secret sql reaches observer but not public exception`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val store = JdbcWorkflowCheckpointStore(SeededSqlDataSource(SQLException(SECRET_SQL))) + store.persistenceFailureDiagnosticObserver = observer + + val thrown = catchThrowable { + runBlocking { store.save(testCheckpoint()) } + } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_SQL) + assertThat(observer.events).hasSize(1) + val event = observer.events.single() + assertThat(event.resourceKind).isEqualTo(PersistenceResourceKind.CHECKPOINT) + assertThat(event.operation).isEqualTo(PersistenceOperation.SAVE) + assertThat(event.failureCode).isEqualTo(PersistenceFailureCode.WRITE_FAILED) + assertThat(event.failure.message).contains(SECRET_SQL) + } + } + + // --- 2. File exception secret path reaches observer only --- + + @Test + fun `file exception containing secret path reaches observer but not public exception`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val failingWriter = AtomicFileWriter { _ -> throw IOException(SECRET_PATH) } + val store = FileWorkflowCheckpointStore.forTest(tempDir, failingWriter) + store.persistenceFailureDiagnosticObserver = observer + + val thrown = catchThrowable { + runBlocking { store.save(testCheckpoint()) } + } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_PATH) + assertThat(observer.events).hasSize(1) + assertThat(observer.events.single().failure.message).contains(SECRET_PATH) + } + } + + // --- 3. Corrupt checkpoint secret payload reaches observer only --- + + @Test + fun `corrupt checkpoint containing secret payload reaches observer but not public exception`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val store = FileWorkflowCheckpointStore(tempDir) + store.persistenceFailureDiagnosticObserver = observer + // Write a checkpoint file whose recovery state decodes to garbage carrying the secret. + val checkpointDir = tempDir.resolve("wf").resolve("run-1").toFile() + checkpointDir.mkdirs() + Files.writeString( + tempDir.resolve("wf").resolve("run-1").resolve("checkpoint.properties"), + "workflowName=wf\nworkflowId=run-1\nnextStepIndex=0\nstepExecutions=0\nstatePayloadBase64=c3RhdGU=\nrevision=1\nrecoveryState=${base64Encode(SECRET_PAYLOAD)}\n", + ) + + val thrown = catchThrowable { + runBlocking { store.load("wf", "run-1") } + } + + assertThat(thrown).isInstanceOf(WorkflowCheckpointCorruptionException::class.java) + assertNoSecret(thrown, SECRET_PAYLOAD) + assertThat(observer.events).hasSize(1) + assertThat(observer.events.single().failureCode).isEqualTo(PersistenceFailureCode.CORRUPTED_DATA) + } + } + + // --- 4. Corrupt step-attempt record secret value reaches observer only --- + + @Test + fun `corrupt step attempt record containing secret value reaches observer but not public exception`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val store = FileStepAttemptRecordStore.forTest(tempDir, realAtomicFileWriter) + store.persistenceFailureDiagnosticObserver = observer + // Seed one record then corrupt its status to an unknown value carrying the secret. + val record = minimalAttempt() + store.recordStepAttempt(record) + val attemptPath = tempDir + .resolve(base64UrlEncodeNoPadding(record.runId)) + .resolve(base64UrlEncodeNoPadding(record.stepName)) + .resolve(base64UrlEncodeNoPadding(record.attemptId) + ".attempt.properties") + val raw = Files.readString(attemptPath) + Files.writeString(attemptPath, raw.replace("status=STARTED", "status=$SECRET_VALUE")) + + val thrown = catchThrowable { + runBlocking { store.listStepAttempts(record.runId) } + } + + assertThat(thrown).isInstanceOf(StepAttemptRecordCorruptionException::class.java) + assertNoSecret(thrown, SECRET_VALUE) + assertThat(observer.events).hasSize(1) + assertThat(observer.events.single().failureCode).isEqualTo(PersistenceFailureCode.CORRUPTED_DATA) + } + } + + // --- 5. Untrusted caller-constructed domain exception is re-sanitized --- + + @Test + fun `caller constructed untrusted domain exception is re sanitized`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val untrusted = WorkflowCheckpointCorruptionException("password=fixture-sentinel-7f3c") + + val thrown = catchThrowable { + runBlocking { + persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.LOAD, + observer, + ) { throw untrusted } + } + } + + assertThat(thrown).isInstanceOf(WorkflowCheckpointCorruptionException::class.java) + assertThat(thrown).isNotSameAs(untrusted) + assertNoSecret(thrown, "password=") + assertNoSecret(thrown, "fixture-sentinel-7f3c") + assertThat(observer.events).hasSize(1) + assertThat(observer.events.single().failure).isSameAs(untrusted) + } + } + + // --- 6. Trusted safe exceptions are reconstructed --- + + @Test + fun `trusted safe exception is reconstructed without observer event`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val safe = safePersistenceFailure( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.SAVE, + PersistenceFailureCode.CONFLICT, + ) + + val thrown = catchThrowable { + runBlocking { + persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.SAVE, + observer, + ) { throw safe } + } + } + + assertThat(thrown).isInstanceOf(WorkflowCheckpointConflictException::class.java) + assertThat(thrown).isNotSameAs(safe) + assertThat(thrown!!.cause).isNull() + assertThat(thrown.suppressed).isEmpty() + assertThat(observer.events).isEmpty() + } + } + + @Test + fun `cancellation with suppressed sql is sanitized for the caller`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val cancelled = CancellationException("parent cancelled").also { it.addSuppressed(SQLException(SECRET_SQL)) } + + val thrown = catchThrowable { + runBlocking { + persistenceBoundary(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, observer) { + throw cancelled + } + } + } + + assertThat(thrown).isInstanceOf(CancellationException::class.java) + assertNoSecret(thrown, SECRET_SQL) + // Genuine-cancellation path: no observer delivery (delivery's + // postcondition ensureActive() would throw under parent cancellation), + // only the sanitized CE with the fixed cleanup marker. + assertThat(observer.events).isEmpty() + assertThat(thrown!!.suppressed.single()).isInstanceOf(PersistenceCleanupDiagnosticException::class.java) + } + } + + @Test + fun `cancellation with cause and suppressed sql is sanitized for the caller`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val cancelled = CancellationException("close $SECRET_SQL").also { it.addSuppressed(SQLException("rollback $SECRET_SQL")) } + + val thrown = catchThrowable { + runBlocking { + persistenceBoundary(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, observer) { + throw cancelled + } + } + } + + assertThat(thrown).isInstanceOf(CancellationException::class.java) + assertNoSecret(thrown, SECRET_SQL) + assertThat(observer.events).isEmpty() + assertThat(thrown!!.suppressed.single()).isInstanceOf(PersistenceCleanupDiagnosticException::class.java) + } + } + + @Test + fun `nested cancellation with raw suppressed sql in cause graph is sanitized for the caller`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val inner = CancellationException("inner").also { it.addSuppressed(SQLException(SECRET_SQL)) } + val outer = CancellationException("outer").also { it.initCause(inner) } + + val thrown = catchThrowable { + runBlocking { + persistenceBoundary(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, observer) { + throw outer + } + } + } + + assertThat(thrown).isInstanceOf(CancellationException::class.java) + assertNoSecret(thrown, SECRET_SQL) + assertThat(observer.events).isEmpty() + assertThat(thrown!!.suppressed.single()).isInstanceOf(PersistenceCleanupDiagnosticException::class.java) + } + } + + @Test + fun `trusted exception contaminated by jdbc cleanup is reconstructed not passed through`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val trusted = safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, PersistenceFailureCode.CONFLICT) + .also { it.addSuppressed(SQLException(SECRET_SQL)) } + + val thrown = catchThrowable { + runBlocking { + persistenceBoundary(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, observer) { throw trusted } + } + } + + assertThat(thrown).isInstanceOf(WorkflowCheckpointConflictException::class.java) + assertThat(thrown).isNotSameAs(trusted) + assertNoSecret(thrown, SECRET_SQL) + assertThat(thrown!!.cause).isNull() + assertThat(thrown.suppressed).isEmpty() + assertThat(observer.events.single().failure).isSameAs(trusted) + } + } + + @Test + fun `throwing path strategy in file lease release is caught by boundary`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val store = FileWorkflowLeaseStore(tempDir, throwingPathStrategy(), { 0L }, observer) + val thrown = catchThrowable { runBlocking { store.release(testLease()) } } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_PATH) + assertThat(observer.events).hasSize(1) + } + } + + @Test + fun `throwing path strategy in file fenced delete is caught by boundary`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val store = FileWorkflowLeaseStore(tempDir, throwingPathStrategy(), { 0L }, observer) + val thrown = catchThrowable { + runBlocking { store.deleteCheckpointIfLeaseOwner(InMemoryWorkflowCheckpointStore(), "wf", "run-1", null, testLease()) } + } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_PATH) + assertThat(observer.events).hasSize(1) + } + } + + @Test + fun `requireRecovery load failure is classified read failed not write failed`() { + runBlocking { + // Explicit implementation (no `by` delegation): the interface default + // requireRecovery must call THIS load() and fail with the raw error. + val store = FailingLoadCheckpointStore(InMemoryWorkflowCheckpointStore(), IOException(SECRET_PATH)) + val thrown = catchThrowable { runBlocking { store.requireRecovery("wf", "run-1", 1, testRecoveryRecord()) } } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertThat(thrown!!.message).isEqualTo("Workflow persistence read failed") + assertNoSecret(thrown, SECRET_PATH) + } + } + + @Test + fun `clearRecovery load failure is classified read failed not write failed`() { + runBlocking { + val store = FailingLoadCheckpointStore(InMemoryWorkflowCheckpointStore(), IOException(SECRET_PATH)) + val thrown = catchThrowable { runBlocking { store.clearRecovery("wf", "run-1", 1) } } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertThat(thrown!!.message).isEqualTo("Workflow persistence read failed") + assertNoSecret(thrown, SECRET_PATH) + } + } + + @Test + fun `lease delete phase inside load is classified delete failed`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val thrown = catchThrowable { + runBlocking { + persistenceBoundary(PersistenceResourceKind.LEASE, PersistenceOperation.LOAD, observer) { + throw LeaseDeletePhaseFailure(IOException(SECRET_PATH)) + } + } + } + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_PATH) + assertThat(observer.events.single().failureCode).isEqualTo(PersistenceFailureCode.DELETE_FAILED) + assertThat(observer.events.single().operation).isEqualTo(PersistenceOperation.LOAD) + } + } + + @Test + fun `updateHeartbeat check and update stay atomic against unregister`() { + runBlocking { + val store = InMemoryWorkflowLeaseStore( + clockMillis = { 1000L }, + observer = RecordingPersistenceObserver(), + ) + store.registerWorker("w1", "pool", "v1", setOf(), "host") + // Concurrent heartbeats and unregisters; the observable outcomes are + // only success or IllegalArgumentException (unknown worker) — never + // NoSuchElementException or a persistence failure from a + // check-then-act race. + val errors = java.util.Collections.synchronizedList(mutableListOf()) + val jobs = (1..8).map { worker -> + launch { + repeat(100) { i -> + if (i % 2 == 0) { + try { + store.updateHeartbeat("w1") + } catch (t: Throwable) { + errors.add(t) + } + store.registerWorker("w1", "pool", "v1", setOf(), "host") + } else { + // Genuine unregister: the next heartbeat either sees + // the worker (re-registered by an even iteration) or + // fails with IllegalArgumentException — never a + // NoSuchElementException from a check-then-act race. + store.unregisterWorker("w1") + try { + store.updateHeartbeat("w1") + } catch (t: Throwable) { + errors.add(t) + } + store.registerWorker("w1", "pool", "v1", setOf(), "host") + } + } + } + } + jobs.forEach { it.join() } + errors.forEach { assertThat(it).isInstanceOf(IllegalArgumentException::class.java) } + } + } + + @Test + fun `updateHeartbeat unknown worker throws IllegalArgumentException not persistence failure`() { + runBlocking { + val store = InMemoryWorkflowLeaseStore( + clockMillis = { 1000L }, + observer = RecordingPersistenceObserver(), + ) + val thrown = catchThrowable { runBlocking { store.updateHeartbeat("unknown") } } + assertThat(thrown).isInstanceOf(IllegalArgumentException::class.java) + } + } + + @Test + fun `negative stale threshold throws IllegalArgumentException not persistence failure`() { + runBlocking { + val store = InMemoryWorkflowLeaseStore( + clockMillis = { 1000L }, + observer = RecordingPersistenceObserver(), + ) + val thrown = catchThrowable { runBlocking { store.listStaleWorkers(-1) } } + assertThat(thrown).isInstanceOf(IllegalArgumentException::class.java) + } + } + + @Test + fun `lease delete phase inside renew is classified delete failed`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val thrown = catchThrowable { + runBlocking { + persistenceBoundary(PersistenceResourceKind.LEASE, PersistenceOperation.RENEW, observer) { + throw LeaseDeletePhaseFailure(IOException(SECRET_PATH)) + } + } + } + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_PATH) + assertThat(observer.events.single().failureCode).isEqualTo(PersistenceFailureCode.DELETE_FAILED) + assertThat(observer.events.single().operation).isEqualTo(PersistenceOperation.RENEW) + } + } + + @Test + fun `lease read phase inside claim is classified read failed`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val thrown = catchThrowable { + runBlocking { + persistenceBoundary(PersistenceResourceKind.LEASE, PersistenceOperation.CLAIM, observer) { + throw LeaseReadPhaseFailure(IOException(SECRET_PATH)) + } + } + } + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_PATH) + assertThat(observer.events.single().failureCode).isEqualTo(PersistenceFailureCode.READ_FAILED) + assertThat(observer.events.single().operation).isEqualTo(PersistenceOperation.CLAIM) + } + } + + private class FailingLoadCheckpointStore( + private val delegate: InMemoryWorkflowCheckpointStore, + private val failure: Throwable, + ) : WorkflowCheckpointStore { + override suspend fun load(workflowName: String, workflowId: String): WorkflowCheckpoint? = throw failure + override suspend fun save(checkpoint: WorkflowCheckpoint, expectedRevision: Long?): WorkflowCheckpoint = + delegate.save(checkpoint, expectedRevision) + override suspend fun delete(workflowName: String, workflowId: String, expectedRevision: Long?) = + delegate.delete(workflowName, workflowId, expectedRevision) + } + + @Test + fun `raw conflict named exception uses operation default rather than name heuristic`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val thrown = catchThrowable { + runBlocking { + persistenceBoundary(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, observer) { + throw IllegalStateException("Conflict-ish") + } + } + } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertThat(observer.events.single().failureCode).isEqualTo(PersistenceFailureCode.WRITE_FAILED) + } + } + + @Test + fun `checkpoint dml failure reaches checkpoint observer not lease observer`() { + runBlocking { + val checkpointObserver = RecordingPersistenceObserver() + val leaseObserver = RecordingPersistenceObserver() + val thrown = catchThrowable { + runBlocking { + persistenceBoundary( + PersistenceResourceKind.LEASE, + PersistenceOperation.SAVE, + leaseObserver, + checkpointDiagnosticObserver = checkpointObserver, + ) { throw CheckpointDmlFailure(SQLException(SECRET_SQL)) } + } + } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_SQL) + assertThat(checkpointObserver.events.single().resourceKind).isEqualTo(PersistenceResourceKind.CHECKPOINT) + assertThat(leaseObserver.events).isEmpty() + } + } + + @Test + fun `sanitized cancellation escapes even when coroutine context is already cancelled`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val contaminated = CancellationException("parent").also { it.addSuppressed(SQLException(SECRET_SQL)) } + val outer = CoroutineScope(Job()) + val outcome = CompletableDeferred() + + outer.launch { + try { + persistenceBoundary(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, observer) { + currentCoroutineContext().cancel() + throw contaminated + } + outcome.complete(null) + } catch (error: Throwable) { + outcome.complete(error) + } + } + + val terminal = withTimeout(5_000) { outcome.await() } + assertThat(terminal).isInstanceOf(CancellationException::class.java) + assertNoSecret(terminal, SECRET_SQL) + assertThat(terminal!!.suppressed).hasSize(1) + assertThat(terminal.suppressed.single()).isInstanceOf(PersistenceCleanupDiagnosticException::class.java) + assertThat(observer.events).isEmpty() + } + } + + @Test + fun `corrupt checkpoint inside fenced dml is classified corrupted data`() { + runBlocking { + val checkpointObserver = RecordingPersistenceObserver() + val leaseObserver = RecordingPersistenceObserver() + val corrupt = CorruptCheckpointException("corrupt", "payload-$SECRET_SQL") + + val thrown = catchThrowable { + runBlocking { + persistenceBoundary( + PersistenceResourceKind.LEASE, + PersistenceOperation.SAVE, + leaseObserver, + checkpointDiagnosticObserver = checkpointObserver, + ) { throw CheckpointDmlFailure(corrupt) } + } + } + + assertThat(thrown).isInstanceOf(WorkflowCheckpointCorruptionException::class.java) + assertNoSecret(thrown, SECRET_SQL) + assertThat(checkpointObserver.events.single().failureCode).isEqualTo(PersistenceFailureCode.CORRUPTED_DATA) + assertThat(leaseObserver.events).isEmpty() + } + } + + @Test + fun `external diagnostic observer reads corrupt payload through public interface`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val corrupt = CorruptCheckpointException("corrupt", "payload-$SECRET_SQL") + + val thrown = catchThrowable { + runBlocking { + persistenceBoundary(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.LOAD, observer) { + throw corrupt + } + } + } + + assertThat(thrown).isInstanceOf(WorkflowCheckpointCorruptionException::class.java) + assertNoSecret(thrown, SECRET_SQL) + val detail = observer.events.single().failure as? PersistenceCorruptionDetail + assertThat(detail).isNotNull + assertThat(detail!!.corruptPayload).contains(SECRET_SQL) + } + } + + @Test + fun `fenced jdbc save preconditions remain caller errors`() { + runBlocking { + val leaseStore = JdbcWorkflowLeaseStore(SeededSqlDataSource(SQLException(SECRET_SQL))) + val nonJdbc = catchThrowable { + runBlocking { + leaseStore.saveCheckpointIfLeaseOwner(InMemoryWorkflowCheckpointStore(), testCheckpoint(), null, testLease()) + } + } + val mismatched = catchThrowable { + runBlocking { + leaseStore.saveCheckpointIfLeaseOwner( + JdbcWorkflowCheckpointStore(SeededSqlDataSource(SQLException(SECRET_SQL))), + testCheckpoint(), + null, + testLease(), + ) + } + } + + assertThat(nonJdbc).isInstanceOf(IllegalArgumentException::class.java) + assertThat(mismatched).isInstanceOf(IllegalArgumentException::class.java) + } + } + + @Test + fun `in memory lease caller preconditions remain caller errors`() { + runBlocking { + val store = InMemoryWorkflowLeaseStore() + val unknownWorker = catchThrowable { runBlocking { store.updateHeartbeat("unknown") } } + val negativeThreshold = catchThrowable { runBlocking { store.listStaleWorkers(-1) } } + + assertThat(unknownWorker).isInstanceOf(IllegalArgumentException::class.java) + assertThat(negativeThreshold).isInstanceOf(IllegalArgumentException::class.java) + } + } + + @Test + fun `jdbc lease claim read failure is classified read failed not write failed`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val store = JdbcWorkflowLeaseStore(SeededSqlDataSource(SQLException(SECRET_SQL))) + store.persistenceFailureDiagnosticObserver = observer + + val thrown = catchThrowable { + runBlocking { store.claim("wf", "run-1", "owner", null, 1_000) } + } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_SQL) + assertThat(observer.events.single().failureCode).isEqualTo(PersistenceFailureCode.READ_FAILED) + assertThat(observer.events.single().operation).isEqualTo(PersistenceOperation.CLAIM) + } + } + + @Test + fun `lease renew read failure preserves renew operation`() { + assertLeaseReadFailure(PersistenceOperation.RENEW) + } + + @Test + fun `lease release read failure preserves release operation`() { + assertLeaseReadFailure(PersistenceOperation.RELEASE) + } + + private fun assertLeaseReadFailure(operation: PersistenceOperation) { + runBlocking { + val observer = RecordingPersistenceObserver() + val thrown = catchThrowable { + runBlocking { + persistenceBoundary(PersistenceResourceKind.LEASE, operation, observer) { + throw LeaseReadPhaseFailure(IOException(SECRET_PATH)) + } + } + } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_PATH) + assertThat(observer.events.single().failureCode).isEqualTo(PersistenceFailureCode.READ_FAILED) + assertThat(observer.events.single().operation).isEqualTo(operation) + } + } + + @Test + fun `trusted failure cannot beat concurrent genuine cancellation`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val entered = CompletableDeferred() + val release = CompletableDeferred() + val outer = CoroutineScope(Job()) + val outcome = CompletableDeferred() + outer.launch { + try { + persistenceBoundary(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, observer) { + entered.complete(Unit) + release.await() + throw safePersistenceFailure(PersistenceResourceKind.CHECKPOINT, PersistenceOperation.SAVE, PersistenceFailureCode.CONFLICT) + } + outcome.complete(null) + } catch (t: Throwable) { + outcome.complete(t) + } + } + entered.await() + outer.coroutineContext[Job]!!.cancel() + release.complete(Unit) + + assertThat(withTimeout(5_000) { outcome.await() }).isInstanceOf(CancellationException::class.java) + assertThat(observer.events).isEmpty() + } + } + + // --- 7. Observer ordinary failure is fail-open --- + + @Test + fun `observer ordinary failure is fail open`() { + runBlocking { + val observer = RecordingPersistenceObserver() + observer.throwAfterRecording = IllegalStateException("observer bug") + val store = JdbcWorkflowCheckpointStore(SeededSqlDataSource(SQLException(SECRET_SQL))) + store.persistenceFailureDiagnosticObserver = observer + + val thrown = catchThrowable { + runBlocking { store.save(testCheckpoint()) } + } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_SQL) + assertThat(observer.events).hasSize(1) + } + } + + // --- 8. Synthetic observer cancellation while active is swallowed --- + + @Test + fun `synthetic observer cancellation while active does not replace failure`() { + runBlocking { + val observer = RecordingPersistenceObserver() + observer.throwFakeCancellation = true + val store = JdbcWorkflowCheckpointStore(SeededSqlDataSource(SQLException(SECRET_SQL))) + store.persistenceFailureDiagnosticObserver = observer + + val thrown = catchThrowable { + runBlocking { store.save(testCheckpoint()) } + } + + assertThat(thrown).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(thrown, SECRET_SQL) + assertThat(observer.events).hasSize(1) + } + } + + // --- 9. Genuine parent cancellation stays primary, emits no diagnostic --- + + @Test + fun `genuine parent cancellation remains primary and emits no diagnostic`() { + runBlocking { + val observer = RecordingPersistenceObserver() + val entered = CompletableDeferred() + val release = CompletableDeferred() + + val outer = CoroutineScope(Job()) + val outcome = CompletableDeferred() + outer.launch { + try { + persistenceBoundary( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.SAVE, + observer, + ) { + entered.complete(Unit) + release.await() + Unit + } + outcome.complete(null) + } catch (t: Throwable) { + outcome.complete(t) + } + } + entered.await() + outer.coroutineContext[Job]!!.cancel() + release.complete(Unit) + val terminal = withTimeout(5_000) { outcome.await() } + assertThat(terminal).isInstanceOf(CancellationException::class.java) + assertThat(observer.events).isEmpty() + } + } + + // --- 10. Checkpoint conflict keeps its semantic class --- + + @Test + fun `checkpoint conflict remains the correct semantic exception class`() { + runBlocking { + val store = InMemoryWorkflowCheckpointStore() + store.save(testCheckpoint()) + + val thrown = catchThrowable { + runBlocking { store.save(testCheckpoint()) } + } + + assertThat(thrown).isInstanceOf(WorkflowCheckpointConflictException::class.java) + assertThat((thrown as WorkflowCheckpointConflictException).message).isEqualTo("Workflow checkpoint conflict") + assertThat((thrown as WorkflowCheckpointConflictException).cause).isNull() + } + } + + // --- 11. CAS and revision behavior unchanged --- + + @Test + fun `cas and revision behavior unchanged`() { + runBlocking { + val store = InMemoryWorkflowCheckpointStore() + val first = store.save(testCheckpoint()) + assertThat(first.revision).isEqualTo(1) + + val second = store.save(first, expectedRevision = first.revision) + assertThat(second.revision).isEqualTo(2) + + val conflict = catchThrowable { + runBlocking { store.save(second, expectedRevision = first.revision) } + } + assertThat(conflict).isInstanceOf(WorkflowCheckpointConflictException::class.java) + + val attemptStore = store as StepAttemptRecordStore + val attempt = minimalAttempt() + attemptStore.recordStepAttempt(attempt) + assertThat(attemptStore.compareAndSetStepAttempt(attempt, attempt.copy(workerId = "w2"))).isTrue() + assertThat(attemptStore.compareAndSetStepAttempt(attempt, attempt.copy(workerId = "w3"))).isFalse() + } + } + + // --- 12. Worker onPollFailed receives only the safe failure --- + + @Test + fun `worker onPollFailed receives only safe failure`() { + val pollFailures = mutableListOf() + val workerObserver = object : TramaiWorkerObserver { + override fun onPollFailed(workerId: String, error: Throwable) { + pollFailures += error + } + } + val failingCatalog = FailingCheckpointCatalog(SQLException(SECRET_SQL)) + val checkpointStore = InMemoryWorkflowCheckpointStore() + + val worker = TramaiWorker( + config = WorkerConfig( + workerId = "worker-secret-test", + poolName = "tests", + pollIntervalMillis = 20, + leaseDurationMillis = 200, + drainTimeoutMillis = 1_000, + partitionEnabled = false, + workerCount = 1, + ), + leaseStore = InMemoryWorkflowLeaseStore(), + checkpointStore = checkpointStore, + checkpointCatalog = failingCatalog, + stepAttemptStore = checkpointStore, + workflowRegistry = emptyMap(), + observability = workerObserver, + ) + runBlocking { + worker.start() + try { + withTimeout(5_000) { + while (pollFailures.isEmpty() && currentCoroutineContext().isActive) delay(20) + } + } finally { + worker.shutdown() + } + } + + assertThat(pollFailures).isNotEmpty() + pollFailures.forEach { failure -> + assertThat(failure).isInstanceOf(WorkflowPersistenceFailureException::class.java) + assertNoSecret(failure, SECRET_SQL) + } + } + + // --- 13. Logging observer renders class name only --- + + @Test + fun `logging observer renders no raw error message`() { + val logged = mutableListOf() + val observer = LoggingTramaiWorkerObserver { logged += it } + observer.onPollFailed("w", SQLException(SECRET_SQL)) + observer.onLeaseRenewalFailed("wf", "w", IOException(SECRET_PATH)) + assertThat(logged.joinToString("\n")).doesNotContain(SECRET_SQL) + assertThat(logged.joinToString("\n")).doesNotContain(SECRET_PATH) + } + + // --- 14. Fixed public messages are stable --- + + @Test + fun `fixed public messages are stable`() { + assertThat( + fixedPersistenceFailureMessage( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.SAVE, + PersistenceFailureCode.CONFLICT, + ), + ).isEqualTo("Workflow checkpoint conflict") + assertThat( + fixedPersistenceFailureMessage( + PersistenceResourceKind.LEASE, + PersistenceOperation.CLAIM, + PersistenceFailureCode.CONFLICT, + ), + ).isEqualTo("Workflow lease conflict") + assertThat( + fixedPersistenceFailureMessage( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.LOAD, + PersistenceFailureCode.CORRUPTED_DATA, + ), + ).isEqualTo("Persisted workflow checkpoint is invalid") + assertThat( + fixedPersistenceFailureMessage( + PersistenceResourceKind.STEP_ATTEMPT, + PersistenceOperation.LOAD, + PersistenceFailureCode.CORRUPTED_DATA, + ), + ).isEqualTo("Persisted step-attempt record is invalid") + assertThat( + fixedPersistenceFailureMessage( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.LOAD, + PersistenceFailureCode.READ_FAILED, + ), + ).isEqualTo("Workflow persistence read failed") + assertThat( + fixedPersistenceFailureMessage( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.SAVE, + PersistenceFailureCode.WRITE_FAILED, + ), + ).isEqualTo("Workflow persistence write failed") + assertThat( + fixedPersistenceFailureMessage( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.DELETE, + PersistenceFailureCode.DELETE_FAILED, + ), + ).isEqualTo("Workflow persistence delete failed") + assertThat( + fixedPersistenceFailureMessage( + PersistenceResourceKind.CHECKPOINT, + PersistenceOperation.LIST, + PersistenceFailureCode.LIST_FAILED, + ), + ).isEqualTo("Workflow persistence list failed") + } + + // --- 15. Worker failAttempt sanitizes persistence failures, keeps user step errors --- + + @Test + fun `worker failAttempt persists safe summary for persistence failures but keeps user step errors`() { + // Mirrors the classifier the worker's failAttempt uses: a + // persistence-family failure is sanitized for the durable outputSummary + // and observer, while a user step-execution error keeps its message. + val rawPersistence = WorkflowCheckpointCorruptionException("password=fixture-sentinel-7f3c") + val safeSummary = summarizeForTest(rawPersistence) + assertThat(safeSummary).doesNotContain("fixture-sentinel-7f3c") + assertThat(safeSummary).doesNotContain("password=") + + val userError = IllegalStateException("user step failed: fixture-sentinel-7f3c") + val userSummary = summarizeForTest(userError) + assertThat(userSummary).contains("user step failed") + } + + private fun summarizeForTest(error: Throwable): String { + val observable = when { + !error.isPersistenceFamilyFailure() -> error + else -> safeWorkerObservableFailure(PersistenceResourceKind.STEP_ATTEMPT, PersistenceOperation.SAVE, error) + } + return buildString { + append(observable::class.simpleName ?: observable::class.java.simpleName) + val message = observable.message?.take(240) + if (!message.isNullOrBlank()) { + append(": ") + append(message) + } + } + } + + // --- fixtures --- + + private fun testCheckpoint() = WorkflowCheckpoint( + workflowName = "wf", + workflowId = "run-1", + nextStepIndex = 0, + stepExecutions = 0, + lastCompletedStepName = null, + statePayload = "state", + ) + + private fun testLease() = WorkflowLease( + workflowName = "wf", + workflowId = "run-1", + leaseId = "lease-1", + ownerId = "owner-1", + checkpointRevision = null, + acquiredAtEpochMillis = 0, + expiresAtEpochMillis = 1_000, + ) + + private fun throwingPathStrategy() = object : WorkflowCheckpointPathStrategy { + override fun resolve(root: Path, workflowName: String, workflowId: String): Path = throw IOException(SECRET_PATH) + } + + private fun testRecoveryRecord() = WorkflowRecoveryRecord( + reason = WorkflowRecoveryReason.NON_REPLAYABLE_OUTCOME_UNKNOWN, + stepName = "step", + attemptId = "attempt", + priorWorkerId = "worker", + detectedAtEpochMillis = 0, + ) + + private fun minimalAttempt() = StepAttemptRecord( + runId = "run-attempt", + stepName = "work", + attemptId = "attempt-1", + workerId = "worker-a", + leaseToken = "lease-a", + status = StepAttemptStatus.STARTED, + startedAt = 10L, + replayPolicy = ReplayPolicy.PURE, + ) + + private class FailingCheckpointCatalog( + private val failure: SQLException, + ) : WorkflowCheckpointCatalog { + override suspend fun listCheckpoints(): List = throw failure + } +} diff --git a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/StepAttemptRecordStoreContractTest.kt b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/StepAttemptRecordStoreContractTest.kt index f2d183e1..3a1d28c6 100644 --- a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/StepAttemptRecordStoreContractTest.kt +++ b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/StepAttemptRecordStoreContractTest.kt @@ -85,8 +85,9 @@ abstract class StepAttemptRecordStoreContractTest { @Test fun `5 update rejects missing record`() { assertThatThrownBy { runBlocking { store.updateStepAttempt(minimalRecord()) } } - .isInstanceOfAny(IllegalStateException::class.java, IllegalArgumentException::class.java) - .hasMessageContaining("does not exist") + .isInstanceOf(WorkflowPersistenceFailureException::class.java) + .hasMessage("Workflow persistence write failed") + .hasNoCause() } @Test @@ -215,7 +216,7 @@ abstract class StepAttemptRecordStoreContractTest { if (!harness.supportsPersistentCorruption) { assertThatThrownBy { StepAttemptRecordCodec.requireValidFingerprint(record, "bad", "contract fixture") - }.isInstanceOf(StepAttemptRecordCorruptionException::class.java) + }.isInstanceOf(CorruptStepAttemptException::class.java) return@runBlocking } store.recordStepAttempt(record) @@ -297,8 +298,11 @@ interface AttemptStoreHarness { val lines = StepAttemptRecordCodec.encode(testCodecRecord()).lines().toMutableList() val index = lines.indexOfFirst { it.startsWith("$field=") } if (replacement == null) lines.removeAt(index) else lines[index] = "$field=$replacement" + // The codec is internal; it fails closed with the internal corruption + // carrier. The public fixed-text StepAttemptRecordCorruptionException is + // proven at the store boundary (PersistenceSafeFailureBoundaryTest). assertThatThrownBy { StepAttemptRecordCodec.decode(lines.joinToString("\n")) } - .isInstanceOf(StepAttemptRecordCorruptionException::class.java) + .isInstanceOf(CorruptStepAttemptException::class.java) } suspend fun corruptFingerprint(record: StepAttemptRecord) = Unit @@ -372,7 +376,8 @@ class JdbcStepAttemptRecordStoreContractTest : StepAttemptRecordStoreContractTes jdbcHarness.corruptSchemaVersion(recorded) assertThatThrownBy { runBlocking { store.listStepAttempts("run") } } .isInstanceOf(StepAttemptRecordCorruptionException::class.java) - .hasMessageContaining("schema version") + .hasMessage("Persisted step-attempt record is invalid") + .hasNoCause() } } } diff --git a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowCheckpointStoreTest.kt b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowCheckpointStoreTest.kt index 71d47ca1..239a265d 100644 --- a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowCheckpointStoreTest.kt +++ b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowCheckpointStoreTest.kt @@ -49,7 +49,8 @@ class WorkflowCheckpointStoreTest { } } .isInstanceOf(WorkflowCheckpointConflictException::class.java) - .hasMessageContaining("expected revision 1") + .hasMessage("Workflow checkpoint conflict") + .hasNoCause() runBlocking { store.delete( workflowName = "file-workflow", @@ -126,7 +127,8 @@ class WorkflowCheckpointStoreTest { } } .isInstanceOf(WorkflowCheckpointConflictException::class.java) - .hasMessageContaining("expected revision 1") + .hasMessage("Workflow checkpoint conflict") + .hasNoCause() assertThatThrownBy { runBlocking { store.delete( @@ -137,7 +139,8 @@ class WorkflowCheckpointStoreTest { } } .isInstanceOf(WorkflowCheckpointConflictException::class.java) - .hasMessageContaining("expected revision 1") + .hasMessage("Workflow checkpoint conflict") + .hasNoCause() runBlocking { store.delete( workflowName = "jdbc-workflow", diff --git a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowLeaseStoreTest.kt b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowLeaseStoreTest.kt index f6520d9c..4c9bf1c2 100644 --- a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowLeaseStoreTest.kt +++ b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowLeaseStoreTest.kt @@ -66,7 +66,8 @@ class WorkflowLeaseStoreTest { } } .isInstanceOf(WorkflowLeaseConflictException::class.java) - .hasMessageContaining("already leased by owner 'node-a'") + .hasMessage("Workflow lease conflict") + .hasNoCause() } @Test fun `in memory lease store rejects renewal after lease expiry`() { @@ -92,7 +93,8 @@ class WorkflowLeaseStoreTest { } } .isInstanceOf(WorkflowLeaseConflictException::class.java) - .hasMessageContaining("lease has expired before renewal") + .hasMessage("Workflow lease conflict") + .hasNoCause() assertThat(runBlocking { store.currentLease("lease-workflow", "wf-expired") }).isNull() } @Test @@ -118,8 +120,8 @@ class WorkflowLeaseStoreTest { } } .isInstanceOf(WorkflowLeaseConflictException::class.java) - .hasMessageContaining("leased by owner 'node-a'") - .hasMessageContaining("not 'node-b'") + .hasMessage("Workflow lease conflict") + .hasNoCause() assertThat(runBlocking { store.currentLease("lease-workflow", "wf-release-conflict") }).isEqualTo(claimed) } @Test @@ -160,7 +162,8 @@ class WorkflowLeaseStoreTest { } } .isInstanceOf(WorkflowLeaseConflictException::class.java) - .hasMessageContaining("node-a") + .hasMessage("Workflow lease conflict") + .hasNoCause() runBlocking { store.release(renewed) } assertThat(runBlocking { store.currentLease("lease-workflow", "wf-1") }).isNull() } finally { @@ -218,7 +221,8 @@ class WorkflowLeaseStoreTest { } } .isInstanceOf(WorkflowLeaseConflictException::class.java) - .hasMessageContaining("node-a") + .hasMessage("Workflow lease conflict") + .hasNoCause() } @Test fun `workflow failure releases lease so another owner can take over`() { @@ -298,7 +302,8 @@ class WorkflowLeaseStoreTest { } } .isInstanceOf(WorkflowLeaseConflictException::class.java) - .hasMessageContaining("node-a") + .hasMessage("Workflow lease conflict") + .hasNoCause() now = 2_000L val replacement = runBlocking { store.claim( diff --git a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowRecoveryContractTest.kt b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowRecoveryContractTest.kt index 14e97868..d4683339 100644 --- a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowRecoveryContractTest.kt +++ b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowRecoveryContractTest.kt @@ -1020,8 +1020,12 @@ class WorkflowRecoveryContractTest { @Test fun `decoding malformed recovery payload fails closed`() { + // decodeRecoveryState is internal; it fails closed with the internal + // corruption carrier. The public fixed-text + // WorkflowCheckpointCorruptionException is proven at the store boundary + // (PersistenceSafeFailureBoundaryTest). assertThatThrownBy { decodeRecoveryState("not-a-valid-properties-format") } - .isInstanceOf(WorkflowCheckpointCorruptionException::class.java) + .isInstanceOf(CorruptCheckpointException::class.java) assertThat(decodeRecoveryState(null)).isSameAs(WorkflowRecoveryState.Normal) } diff --git a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowTest.kt b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowTest.kt index 056ee270..4cd2f96e 100644 --- a/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowTest.kt +++ b/tramai-orchestration/src/test/kotlin/dev/tramai/orchestration/WorkflowTest.kt @@ -1361,8 +1361,8 @@ class WorkflowTest { } } .isInstanceOf(WorkflowCheckpointConflictException::class.java) - .hasMessageContaining("already exists") - .hasMessageContaining("workflowId='wf-existing'") + .hasMessage("Workflow checkpoint conflict") + .hasNoCause() } @Test fun `completion checkpoint delete conflict fails loudly and still releases the lease`() { @@ -1535,7 +1535,8 @@ class WorkflowTest { } } .isInstanceOf(WorkflowCheckpointConflictException::class.java) - .hasMessageContaining("expected revision 1") + .hasMessage("Workflow checkpoint conflict") + .hasNoCause() } @Test fun `checkpoint store rejects stale delete revisions`() { @@ -1575,7 +1576,8 @@ class WorkflowTest { } } .isInstanceOf(WorkflowCheckpointConflictException::class.java) - .hasMessageContaining("expected revision 1") + .hasMessage("Workflow checkpoint conflict") + .hasNoCause() } } private data class PlanningState( diff --git a/tramai-orchestration/src/test/resources/binary-compat/BinaryCompatFixture.kt b/tramai-orchestration/src/test/resources/binary-compat/BinaryCompatFixture.kt index 48b2d966..69504c8d 100644 --- a/tramai-orchestration/src/test/resources/binary-compat/BinaryCompatFixture.kt +++ b/tramai-orchestration/src/test/resources/binary-compat/BinaryCompatFixture.kt @@ -2,6 +2,12 @@ package dev.tramai.orchestration import java.io.IOException +import java.io.PrintWriter +import java.nio.file.Files +import java.nio.file.Path +import java.sql.Connection +import java.util.logging.Logger +import javax.sql.DataSource import kotlin.reflect.typeOf @OptIn(ExperimentalStdlibApi::class) @@ -14,5 +20,34 @@ fun binaryCompatFixtureMarkers(): String { val builder = WorkflowBuilder("w", "1", typeOf()) builder.httpStep("http", request = { _, _ -> HttpRequest("GET", "https://x") }, merge = { state, _, _ -> state }) builder.build { it } - return "FIXTURE_OK_1,FIXTURE_OK_2,FIXTURE_OK_3,FIXTURE_OK_4,FIXTURE_OK_5,FIXTURE_OK_6" + WorkflowResumeException("resume") + WorkflowCheckpointConflictException("conflict") + WorkflowLeaseConflictException("lease conflict") + val rootDirectory = Files.createTempDirectory("fixture") + try { + FileWorkflowCheckpointStore(rootDirectory) + MarkdownWorkflowCheckpointStore(rootDirectory) + JdbcWorkflowCheckpointStore(FixtureDataSource) + FileWorkflowLeaseStore(rootDirectory) + JdbcWorkflowLeaseStore(FixtureDataSource) + InMemoryWorkflowCheckpointStore() + InMemoryWorkflowLeaseStore() + } finally { + rootDirectory.toFile().deleteRecursively() + } + return "FIXTURE_OK_1,FIXTURE_OK_2,FIXTURE_OK_3,FIXTURE_OK_4,FIXTURE_OK_5,FIXTURE_OK_6," + + "FIXTURE_OK_7,FIXTURE_OK_8,FIXTURE_OK_9,FIXTURE_OK_10,FIXTURE_OK_11,FIXTURE_OK_12," + + "FIXTURE_OK_13,FIXTURE_OK_14,FIXTURE_OK_15,FIXTURE_OK_16,FIXTURE_OK_17" +} + +private object FixtureDataSource : DataSource { + override fun getConnection(): Connection = throw UnsupportedOperationException("fixture") + override fun getConnection(username: String?, password: String?): Connection = throw UnsupportedOperationException("fixture") + override fun getLogWriter(): PrintWriter? = null + override fun setLogWriter(out: PrintWriter?) = Unit + override fun setLoginTimeout(seconds: Int) = Unit + override fun getLoginTimeout(): Int = 0 + override fun getParentLogger(): Logger = Logger.getGlobal() + override fun unwrap(iface: Class?): T = throw UnsupportedOperationException("fixture") + override fun isWrapperFor(iface: Class<*>?): Boolean = false } diff --git a/tramai-orchestration/src/test/resources/binary-compat/README.md b/tramai-orchestration/src/test/resources/binary-compat/README.md index 202530ee..fe430c55 100644 --- a/tramai-orchestration/src/test/resources/binary-compat/README.md +++ b/tramai-orchestration/src/test/resources/binary-compat/README.md @@ -7,6 +7,11 @@ Kotlin default-argument marker synthetics (omitted-cause calls) were added between v0.5.0 and master; their preservation is verified by the api dump descriptor comparison in `tramai-orchestration.api`, not by this fixture. +The fixture exercises the v0.5.0 constructors for `FileWorkflowCheckpointStore`, +`MarkdownWorkflowCheckpointStore`, `JdbcWorkflowCheckpointStore`, +`FileWorkflowLeaseStore`, `JdbcWorkflowLeaseStore`, +`InMemoryWorkflowCheckpointStore`, and `InMemoryWorkflowLeaseStore`. + Rebuild: `git worktree add /tmp/tramai-v050 v0.5.0`, then run `env -u HTTP_PROXY -u HTTPS_PROXY -u http_proxy -u https_proxy -u ALL_PROXY -u all_proxy ./gradlew :tramai-orchestration:compileTestKotlin -q` in that worktree and jar `BinaryCompatFixtureKt*.class` from its orchestration test output. diff --git a/tramai-orchestration/src/test/resources/binary-compat/fixture-v0.5.0.jar b/tramai-orchestration/src/test/resources/binary-compat/fixture-v0.5.0.jar index 7f2128c5..115e8f91 100644 Binary files a/tramai-orchestration/src/test/resources/binary-compat/fixture-v0.5.0.jar and b/tramai-orchestration/src/test/resources/binary-compat/fixture-v0.5.0.jar differ