Skip to content

fix(reliability): make event matching atomic and close native lifecycle gaps - #260

Merged
ovasylenko merged 7 commits into
mainfrom
fix/reliability-review-final-oct23
Oct 23, 2026
Merged

ovasylenko merged 7 commits into
mainfrom
fix/reliability-review-final-oct23

Conversation

@ovasylenko

@ovasylenko ovasylenko commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Orch8 reliability review — 23 October 2026

Review complete. All suitable fixes are merged to main 77c60ae; all 22 main CI/Mobile jobs passed.

Scope: all 13 Cargo workspace crates, the standalone Node/Python native bindings, SDK contract checks, CI, and open pull requests. The review concentrated on durable state transitions, cancellation/restart, original execution claims, tenant boundaries, resource ownership, and failure recovery. It is a boundary-focused review, not a claim that every module or platform has exhaustive coverage.

Changes and evidence

Group Severity if unfixed Confirmed failure Correction and regression evidence
Original execution ownership Critical Late workers could mutate replacement execution state, outputs, composite control, timing, human-input acknowledgements and builtin state. Original claims now travel through ClaimedControl; PostgreSQL row locks and SQLite immediate transactions validate ownership with writes. Includes loops, races, cancellation, Saga, child creation/adoption and SLA state. PRs #249, #251, #257, #258.
Durable multi-write operations Critical An event could be consumed without persisted join progress, or a satisfied join could lose its notification; a failed restore could leave an incomplete instance. Event matching now commits consumption, authoritative wait progress and one notification together. Registration refresh retains independent matches; reset uses fresh wait generations. Native and encrypted PostgreSQL/SQLite trigger failures roll back all phases. Seven actual engine recovery/race cases pass. Restore writes were made atomic in #254.
Scheduling progress High Early parking, deferred composites and evaluation-budget yields could starve runnable siblings or bypass a loop's polling deadline. Durable polling deadlines, shared pending deferrals and fixed-point fallback retain progress and original ownership. Tests include restart, early wakes, nested races, sub-microsecond deadlines and 220-step/deferred branches. PRs #253 and #259.
Resource and SDK lifecycle High Dropped gRPC/native callers could leave live sessions or callbacks; failed native initialization discarded host handlers; native close retained idle SQLite connections and callback captures in unused, successful and failed initialization states. FCM could read unbounded bodies or wait forever after a successful status. Bounded admission and teardown, frozen handler registrations across retries, serialized native initialization/close, explicit owned-pool shutdown, one callback registry with retry retention and release on completed close, bounded FCM reads and status-only successful acknowledgement. A physical WAL-release control and lifecycle regressions pass; three callback lifetime probes failed before the correction and all seven shared native lifecycle cases pass after it. PRs #234, #236, #238, #240, #259 and #260.
Authorization and privacy High Backend-shared auth cache identity, webhook history/redelivery ownership and support-bundle aliases could cross intended boundaries or disclose credentials. Backend-owned cache identity, owner-scoped webhook actions, active-lease redelivery rejection, broader redaction, and stored/mobile privacy compatibility checks. PRs #235, #243, #245, #247, #248.
Backup consistency and completeness High Live WAL/checkpoint behavior, capped catalog queries, repeated externalized outputs, partial restore and changing PostgreSQL reads could produce incomplete or inconsistent backups. Restore dry-run could create or migrate its target. Consistent SQLite and read-only PostgreSQL snapshots, complete pagination, preserved externalized payloads, atomic restore and non-mutating dry-run snapshots. Success/error cleanup and retry cases were exercised. PRs #242, #246, #250, #254, #255, #259.
Startup and sandbox budgets High Readiness could precede real scheduler progress; repeated WASM control flow could exceed intended per-operation resources. Readiness follows scheduler progress; WASM control flow, fuel and timeline growth are bounded. Security baseline and SDK dependencies were patched and all lockfiles audited. PRs #233, #237, #241.
Contracts, portability and CI Medium Native-width collapse-key prefixes differed across targets; cold CI jobs exhausted time/disk budgets; receipt documentation drifted from the generated contract fingerprint. Fixed eight-byte prefixes preserve existing 64-bit keys; deterministic hash vector; compact build artifacts and appropriate cold-build gates; generated Rust/JavaScript clients and OpenAPI fingerprint checks. PRs #228, #231, #232, #239, #260 and #261. CI now executes benchmark smoke cases and audits every tracked Rust lockfile with --locked.

These are eight remediation groups, not eight independently counted defects. Repeated symptoms of the same ownership/transaction boundary are grouped together.

Crate coverage

Crate Reviewed boundaries and validation
orch8 Embedded scheduling, shutdown/drain ownership, owned storage close, native initialization and lifecycle; 142 runtime tests and 3 doctests passed at the final native lifecycle gate.
orch8-types Stored serde defaults, claim identity, event identity/receipt contract, validation and timestamp representations.
orch8-storage Both native databases and encryption wrappers; atomic writes, leases, cancellation, rollback, snapshots, restore, retention and routing. Latest focused transactional gate: 48 tests passed across all four backend variants.
orch8-engine Scheduler/evaluator ownership, composite reset and fairness, human/event gates, side-effect boundaries, pending signals and failure recovery. Latest component gate: 2,128 passed; seven additional/refined atomic recovery and race cases passed.
orch8-mobile Native callback ownership after caller cancellation, bounded listener admission, teardown, privacy and persisted-record defaults.
orch8-publisher Lifecycle admission, manifest mutation races and receipt/handler validation.
orch8-push Signed wakes, outbox/provider ownership, response bounds, retry classification, quarantine, expiry/replay capacity and portable collapse keys; 101 tests passed.
orch8-api Backend identity, real-router owner/tenant reachability, readiness, webhook actions and generated contract validation.
orch8-grpc Stream/session cleanup, lease wait bounds and dropped callers.
orch8-server Startup reconciliation, readiness/liveness, actual server-process fixtures and CI resource budgets.
orch8-cli Logical/physical backup completeness, snapshot consistency, payload restoration, dry-run behavior and cleanup on errors.
orch8-wasm Per-operation fuel/resource bounds, control-flow and repeat-use behavior.
orch8-e2e Real API/scheduler/process boundaries, worker/mobile handoffs, signing canonicalization and fixture cleanup.

The excluded Node and Python native manifests are checked separately with --locked; their shared Rust implementation is exercised through ordinary facade integration tests. Platform SDK workflows remain a separate merge gate.

Implementation anchors

The follow-up storage contract is in orch8-storage/src/lib.rs:1659 (register_event_wait) and :1677 (match_event_wait); both native events.rs implementations commit the three facts in one database transaction. The engine paths use the primitive in orch8-engine/src/event_correlation.rs. Original ownership is represented by orch8-engine/src/control.rs:27 (ClaimedControl). Explicit owned-pool shutdown is orch8/src/engine.rs:196; the shared Node/Python lifecycle is packages/native-common/durable.rs.

Regression entry points: orch8-storage/tests/event_match_transactions.rs, orch8-storage/tests/claimed_gate_registration.rs, orch8-engine/tests/event_atomic_recovery.rs, event_wait_iteration.rs, event_payload_reads.rs, prepark_claim_authority.rs, and orch8/tests/native_initialization_retry.rs. These exercise observable state and runtime effects rather than duplicating implementation branches.

Technical opportunity synthesis

The highest-impact structural correction is to combine immutable ownership capabilities, native database transactions and the existing fault-injecting backend matrix. This gives composite control, event matching, restoration and lifecycle code explicit commit boundaries that can be tested with real failures. It removes whole classes of check-then-write and partial-commit failures without a new framework or service split.

The concrete baseline was reproducible corruption, lost progress or retained resources. The acceptance criterion is that the same probes preserve the replacement owner's state, leave a recoverable pending fact on rollback, commit a unique notification, or release owned resources. No production incident rate, latency multiplier or financial ROI is asserted; those measurements were not available.

Keep independent federation/outbox leases and terminal compensation separate from scheduler-owned claims. Preserve append-only real step outputs, documented explicit federation call_key identity, existing terminal-wait behavior, and acknowledgement semantics. External side effects still require their documented idempotency/retry contracts.

Pull-request dispositions

The original CI request was the Criterion nested-Tokio-runtime panic in evaluate_flat_10_steps (failed job). PR #225 merged the benchmark correction as 769b2e4a; the replacement Benchmarks run completed successfully. This verifies benchmark execution, not a performance improvement.

The recent reviewed and merged history additionally includes #228 and #231–#259 (30 PRs). Before #260, runtime PR #259 was merged only after all 22 checks and both CI/Mobile workflows succeeded on exact head 2eb960804605f393ca777a1b33e645d708aa0cd7. Main 363b6e0ac9c09b6be695ca85ec16b7d63cc11ac5 subsequently passed CI 39888731788 and Mobile 39888731789.

Not suitable as submitted, left open deliberately:

PR Reason
#222, #226 Claimed performance improvement lacks representative benchmark evidence; the sorted-vector approach adds a different lookup/construction trade-off.
#223, #227 Proposed IDOR helpers do not establish a reachable unguarded route; existing middleware/caller coverage must be reconciled before claiming a security fix.
#229 Node 26 type surface exceeds the configured Node 22 runtime contract.
#230 TypeScript 7 exceeds the pinned typescript-eslint peer support.

PR #260 merged as main 32569e9a46712a7596184b72d7d3754556db7455 at 06:53:22 UTC. Reviewed head 4123274a396f1713cfea800b92b6a76028fe37bb (local 27b360e2) and merged main have identical source tree 038cc0e275e52dffe922936b882cc4bb662e938e; the source diff is empty. Previous head 76451ca9 passed 21 checks, including Mobile, E2E, Docker, fuzz, lint and smoke checks. Its full CI unit job exposed two PostgreSQL fixture failures: unchanged persisted EventWait rows had microsecond timestamps while the expected input retained nanoseconds. Explicit nanosecond input reproduced both failures locally. The correction compares the complete persisted row before/after reset, preserving all ownership and 405-deletion assertions; matching fixtures also force nanoseconds independently of the host clock. All 48 native/encrypted PostgreSQL/SQLite cases and strict workspace Clippy pass. Only those two test files changed; production code is unchanged from 76451ca9.

All 22 checks completed successfully before merge on exact reviewed head 4123274a; both complete CI and Mobile workflows succeeded. The merge used --match-head-commit after rechecking head/base, feedback and the complete matrix. New main CI remains in progress; the complete Mobile workflow passed on exact main 32569e9a. API-contract publication succeeded. The live OpenAPI serves HTTP 200 and its canonical fingerprint matches the reviewed dc801bbd… exactly; the live authoring schema also serves correctly with name and blocks required.

SDK repository audit

All 13 SDK/contract/mobile-example repositories were checked for open PRs and remote branch heads. Each has only main; none has an open PR. Existing local SDK checkouts match their remote main heads. The remaining local Node feat/workflow-builder-docs branch is historical: its first two commits are the exact head of merged sdk-node#1, and its worker-protocol commit is patch-equivalent to merged sdk-node#2. No new source work was found there.

Current main-head checks Repositories
All recorded checks completed successfully sdk-node (Node 18/20/22), sdk-go (Go 1.21/1.22/1.23), sdk-python (Python 3.10–3.13), sdk-expo (package plus iOS/Android example builds), sdk-contract (Node 20/22 conformance kit), sdk-php (PHP 8.2–8.4), sdk-ruby (Ruby 3.1–3.4), sdk-dotnet (.NET 8 on Linux/Windows), sdk-java (Java 17/21)
No check record on current main head react-native-orch8, orch8-flutter, orch8-mobile-swift, mobile-examples

The absent check records are reported explicitly; the engine's separate current-head Mobile workflow supplies additional platform evidence, not an exact-tree test claim for those four repositories.

Verification record

Final main 77c60aeb9d4aeabd0ace5a66348831c68bb65b40 has the exact reviewed source tree ae988ae56d6d1896add23421b863f02781b42506; its source diff against reviewed head 89747e94 is empty. All ten CI jobs and all twelve Mobile jobs completed successfully. The final Linux log confirms 9,910 passed, zero failed, ten existing ignored cases across 238 groups, plus 40 successful Criterion smoke cases. Strict workspace/all-target Clippy, warning-free docs, standalone locked Node/Python checks, generated Rust/JavaScript client compile-and-execute, the API fingerprint and all five locked dependency-audit configurations passed. The full Benchmarks workflow passed on the identical reviewed source before merge.

An additional cargo check --locked --workspace --all-features --all-targets passed without warnings on macOS arm64 and the same source tree. Workspace server dependencies already enable NATS, Wasmtime and other server integrations; this check additionally covers Kafka/Redis combinations and combined generator/test targets. Local types/push crypto regressions passed 806 cases on the patched dependency graph; those overlap the full suite rather than adding an independent 806-test total.

The shared native callback correction was tested with three actual weak-lifetime failures before the fix and all seven shared lifecycle cases passing afterward, including physical SQLite WAL-release and callback release after unused, successful and failed initialization. Actual native modules were built on macOS arm64 with Node 26.5.1 and Python 3.14.6: all three Node and three Python hard-exit/restart/retry tests passed, plus three Python weak-reference release states. Those host probes exercised the callback-corrected code before the last dependency-pin refresh; current locked native checks and the shared Rust suites validate the refreshed graph. Python used the README's documented macOS dynamic-lookup linker flags. These are not Node 22 native-runtime claims.

Native PostgreSQL and SQLite, including encryption wrappers, were exercised with real transaction failure injection, lease replacement, rollback and retry scenarios. PostgreSQL timestamp regressions compare complete persisted rows and deliberately supply nanoseconds, retaining original-claim and deletion assertions. The ten ignored cases are two external broker/emulator tests and eight documentation examples. No aggregate line-coverage percentage is claimed.

Dependency exceptions are explicit in deny.toml: ZIP extraction advisories affect APIs not used by the vendored Swagger build path; the optional UniFFI CLI's unmaintained template dependency is documented separately. The audit also records the pinned inout yanked-version warning. These are not silently reported as an empty raw audit.

Provider protocol tests use controlled HTTP peers, not live Apple/Firebase delivery. CI/platform builds are not a physical-device or multi-day production soak. Collapse keys preserve the existing 64-bit format; no actual 32-bit runtime execution is claimed.

Follow-up policy

Re-review the relevant boundary when adding a mutable backend operation, a new composite controller, a native callback lifecycle, an event/notification path or an independently leased external effect. Add an actual handoff, mid-write rollback or caller-cancellation case before claiming that boundary safe. Require representative measurements for performance-only changes. Keep schema/client fingerprint checks and all backend variants in CI.

Documentation closure: this report consolidates overlapping ownership/atomicity findings, distinguishes local/merged evidence and measured/unmeasured claims, and links implementation intent to executable regression cases. It does not recommend a documentation portal, architecture rewrite or speculative new infrastructure.

Benchmark and dependency-audit follow-up

Closure audit found that evaluate_flat_10_steps configured one step and ignored its evaluator outcome. The ten-step invariant reproduced the failure before correction. PR #261 supplies ten matching steps, requires successful evaluator completion and scheduler counts, gives flatten fixtures unique IDs and accurate size labels, verifies saved output retrieval, and adds both Criterion functional smoke targets to CI. All 33 engine and seven storage smoke cases and strict benchmark Clippy passed. The full Benchmarks run39925463401 also passed on benchmark head 8ab3c484.

That head's CI then caught newly published RUSTSEC-2026-0385 in ctutils 0.4.3. Upstream 0.4.4 fixes constant-time mask generation and 32-bit u128 comparisons. The PR now upgrades all four tracked Rust lockfiles to 0.4.4. Adding the missing fuzz-workspace advisory check exposed retained TLS advisories RUSTSEC-2026-0382/0383; its rustls-webpki/root-certs pins now match the patched versions already in main. Its stale jsonschema/serde_ignored manifest requirements were synchronized. CI audits every tracked Rust workspace with --locked, so it rejects uncommitted resolution drift. No advisory exception was added.

All five locked audit configurations passed, including default workspace policy, all-feature advisories and independent Node/Python/fuzz graphs. On current head 89747e94529b5178563c1624e30ca11d6f76196d, all 705 types and 101 push library tests passed (806 total, zero failures or ignored), covering encryption, authentication and signing. This is functional regression evidence; upstream provides constant-time verification. The source tree is ae988ae56d6d1896add23421b863f02781b42506.

PR #261 merged at 09:41:46 UTC as 77c60aeb9d4aeabd0ace5a66348831c68bb65b40, with source tree ae988ae56d6d1896add23421b863f02781b42506 identical to reviewed head 89747e94. Every one of the 22 normal checks and all 23 individual jobs across CI, Mobile and full Benchmarks completed successfully before merge. Actual Linux results: 9,910 passed, zero failed, ten existing ignored cases across 238 groups, plus 40 successful Criterion smoke cases. Strict workspace/all-target Clippy, docs, native bindings, generated clients and all five locked audit configurations passed.

New-main CI and Mobile both completed successfully on exact merge commit 77c60ae, with all 22 individual jobs independently verified against the reviewed job matrix. The final repository audit confirms all 33 reported engine fixes are in main; all 13 SDK repositories remain main-only with no open PRs, and the six previously held engine PRs were subsequently closed after the final delegated disposition. Nine SDK main heads have successful recorded checks; four distribution/example heads still have no check records, as listed above. The original user checkout is clean and preserved, and the reviewed worktree is clean.

The live OpenAPI and authoring schemas both return HTTP 200. Canonical OpenAPI fingerprint dc801bbddae427283cf589fb234f3ba7bdcc11d60ab43ab007a26ae0a2b83fc2 matches current source, with 248 routes and required authoring fields name and blocks. Publication was last triggered by the API changes in main 32569e9a; the seven files in #261 do not match the publication path filters, and the contract remains unchanged. Renamed flatten series start fresh; no throughput improvement is claimed. All completed temporary build caches are removed; source, logs and evidence are retained.

Final PR disposition and merged-branch cleanup

Following the final delegated decision, PRs #222, #223, #226, #227, #229 and #230 are closed without merging. The performance proposals lacked representative evidence; the authorization proposals did not establish the claimed reachable flaw; the dependency bumps conflicted with the configured runtime/compiler-lint support. Each PR records its closure reason. There are now zero open engine PRs, and all thirteen SDK repositories also have no open PRs.

Removed 53 proven merged local engine branches, one historical Node SDK branch whose changes were already incorporated by sdk-node#1 and sdk-node#2, and 29 proven merged remote engine branches. Squash-merged aliases were checked against merged PR ancestry or exact main-history trees. Remote deletion used an atomic push with expected-tip leases; recoverable branch-history bundles were retained.

The clean original engine checkout is now on current main 77c60ae. Clean worktree snapshots remain available at detached HEADs. The review-fixes branch/worktree is preserved because it contains uncommitted changes, verified byte-for-byte unchanged; pr182-rebase retains a unique unmerged commit. Declined, unmerged remote branches were preserved rather than treated as merged work. No source changes were introduced by this cleanup.

@ovasylenko
ovasylenko marked this pull request as ready for review October 23, 2026 05:06
@ovasylenko
ovasylenko merged commit 32569e9 into main Oct 23, 2026
22 checks passed
@ovasylenko
ovasylenko deleted the fix/reliability-review-final-oct23 branch October 23, 2026 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant