test(nokv): port and adapt the pre-#423 functional regression gates onto the #465 baseline

[Allan10w]

Related Issue

Closes #462

Summary

Rebuilt on the current main baseline (after the #476 lifecycle rework) in
response to the second review. The gates keep the real SDK-to-server
harness (real in-process server, framed RPC, real SDK client) and revise
every test per the change request:

• crates/nokv/tests/concurrent_read_append.rs — real synchronized
  concurrency without sleeps: the writer waits on a read counter before
  the first append and after every intermediate append, so each round
  deterministically observes the base state, at least one intermediate
  prefix state, and the final state; observations are bounded and every
  one must be a legal committed prefix.
• crates/nokv/tests/append_semantics.rs — two barrier-synchronized
  writers with disjoint per-writer identity spaces; a racing append
  surfaces as a typed generation-CAS conflict and the caller retries
  with a fresh identity (the documented concurrency contract). Evidence
  is non-vacuous: measured execution intervals overlap, at least one
  typed conflict is observed, final generation equals base + append
  count, and every delta lands exactly once. Exact retry is asserted as
  the typed client-side revision fence (no double append); the
  response-loss path is covered by the fault-injection gate.
• crates/nokv/tests/object_fault_injection.rs — post-apply
  acknowledgement-loss injection at the ArtifactObjectStore boundary
  (the write durably happens, then a retryable backend error is
  returned). Both publish and append resume the same operation identity
  to typed durable outcomes: publication.replayed, exact generation
  and logical size, upload_stats.replayed >= 1, every block created
  exactly once, intact content, and no partial metadata.
• crates/nokv/tests/common/mod.rs — the shared harness adapted to the
  current serving surface (ShardBoot with LocalOnly recovery
  publication, ServerOptions handshake timeout and inflight-connection
  bound, namespace-bound stores).

Removed per review: the Unicode manifest-pattern fold (main already
carries the ASCII case-insensitive contract with its own unit tests),
the sleep-only paused-create test, and the duplicate immutable-store
test. No product code changes: the net diff is test files only.

Scope

• This PR changes one logical boundary only.
  (Functional regression gates under crates/nokv/tests/; no
  product, backend, protocol, or CI changes.)
• No unrelated refactor, benchmark, metadata model, Holt layout,
  object-store, agent interface, or docs change is mixed in.
• The linked issue describes the user-visible problem, design
  decision, or maintenance task this PR resolves.
• Any breaking change is intentional and documented. (None.)
• No compatibility shim, deprecated alias, or forwarding wrapper was
  added without a removal condition.

Change Size And Review

• I checked GitHub's additions plus deletions for this pull request.
  (Net diff against main: +1065 lines, test files only, below the
  5,000-line threshold.)

Code Contract (Code Changes Only)

• Package boundaries follow docs/development/code_contract.md.
  (Gate tests live in crates/nokv/tests/ alongside the existing
  CLI gate; the shared harness is the test-only tests/common/mod.rs.)
• Shared helpers reuse the standard library or existing repository
  helpers. New generic helper modules are domain-neutral and tested.
  (The harness is exercised by all three gate binaries.)
• File names and file placement follow the code contract.
• New types, interfaces, structs, fields, and functions use
  domain-specific names. (No new public types; test-only helpers.)
• New errors are in the owning package's errors.rs and carry
  stable error kinds when crossing package boundaries. (No new error
  types.)

Validation

• cargo test --workspace → 1079 passed; 0 failed
• cargo test -p nokv --test append_semantics → 6 passed
• cargo test -p nokv --test concurrent_read_append → 1 passed
  (3 synchronized rounds, ~6s)
• cargo test -p nokv --test object_fault_injection → 2 passed
• concurrency tests repeated 5 consecutive runs → all stable
• cargo clippy -p nokv --tests -- -D warnings → clean
• cargo fmt --all -- --check → clean
• Not run (with reason): GitHub Actions CI (no local runner).

Contributor Sign-off

• Every commit in this PR includes a DCO Signed-off-by trailer.

[Copilot]

Pull request overview

This PR restores the pre-#423 continuous restore→snapshot→restore workflow by adjusting the CLI backend snapshot-mint policy, fixes workbench_find’s manifest-pattern case-insensitive contract, and ports several pre-#423 functional regression gates (including a live “real etcd + real binary” restore-chain qualification) into the current path-native architecture.

Changes:

• Allow snapshot minting for restored workbenches (identified via durable restore-manifest presence), restoring the continuous restore-chain behavior.
• Make workbench_find’s manifest_pattern matching case-insensitive as documented, and add Rust unit tests to lock the contract.
• Add/port integration gates under crates/nokv-client/tests/ plus a live restore-chain Python gate, and wire them into CI.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
scripts/workbench/restore_chain_gate.py	New live gate driving the real nokv binary through create→commit→snapshot→restore→snapshot→restore.
scripts/workbench/restore_chain_gate_test.py	Unit/contract tests for the restore-chain gate helpers.
crates/nokv/src/backend.rs	Snapshot policy updated to accept restored workbenches; manifest_pattern matching fixed to be case-insensitive; added unit tests.
crates/nokv-client/tests/common/mod.rs	New shared in-process server + real SDK client harness for integration gates.
crates/nokv-client/tests/append_semantics.rs	New/ported append semantics regression coverage against the in-process server.
crates/nokv-client/tests/concurrent_read_append.rs	New/ported concurrent read-vs-append visibility regression gate.
crates/nokv-client/tests/object_fault_injection.rs	New/ported fault-injection gate validating ambiguous create handling + immutable object contract.
crates/nokv-client/Cargo.toml	Adds dev-dependencies to support the new integration-test harness.
crates/nokv-agent/src/lib.rs	Updates workbench_snapshot tool description to “committed or restored”.
Cargo.lock	Locks new dev-dependency resolution.
.github/workflows/rust.yml	Adds restore-chain gate (and tests) to CI + uploads evidence artifact.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Allan10w]

/review

[wchwawa]

Thanks for working through this recovery chain. Please use #465 as the integration baseline before continuing here. #465 fixes the lifecycle at the authority layer: a restored workbench is committed on arrival, so restore -> snapshot -> restore composes without a separate restore-manifest-presence eligibility path, and it also covers crash/replay and CoW reference safety.

The integration tests added here are still useful and worth preserving. Please consider adapting them to the APIs and invariants in #465, especially the real SDK-to-server append/read concurrency, CoW depth and rematerialization, object failure or ambiguous-write cases, and the restore -> snapshot -> restore regression. The adapted tests can strengthen the repository existing CI rather than introduce a second overlapping restore gate: reuse the current harness where possible, keep provider dependencies isolated, assert exact durable outcomes, and keep the runtime bounded.

With that split, the product-side shortcut can be superseded by #465 while the strongest integration coverage from this PR remains valuable.

[Allan10w]

Thanks — direction accepted. Rebased onto the #465 baseline; the restore-chain product path (restore-manifest-presence eligibility) is removed since #465 fixes the lifecycle at the authority layer.

What remains, adapted per your guidance:

• the three in-process gates (append_semantics, concurrent_read_append, object_fault_injection) plus their shared harness now speak the #465 serving surface: ShardBoot with explicit RecoveryPublicationMode::LocalOnly, a namespace-bound recovery object handle for bootstrap_shard, ServerOptions handshake timeout + inflight-connection bound, new GetPathRequest fields, and the fault-injection wrapper forwards the inner provider handle identity and its write-conformance admission receipt;
• one contract fix kept from the original PR: workbench_find promises case-insensitive literal matching, so find_request_matches_canonical_manifest folds both sides with Unicode case semantics (the #465 baseline matched ASCII-only via eq_ignore_ascii_case), pinned by four unit tests — happy to drop the Unicode semantics and keep the ASCII-only window match if you prefer that contract;
• net diff against main: +933/−3 lines, gates only.

Validation: cargo test --workspace 1066 passed 0 failed; the three gates 6+1+3 passed; clippy -D warnings clean; fmt clean.

[wchwawa]

Please retain the real SDK-to-server integration harness, but revise the gates before merging. Remove the Unicode manifest-matching change because the current contract is ASCII case-insensitive. Make the two-writer and read-versus-append tests exercise real synchronized concurrency with non-vacuous observations. Rework exact retry and ambiguous create to use the same operation identity and cover response loss after apply with typed durable outcomes. Remove the duplicate immutable-store and sleep-only tests.

[Allan10w]

Merged latest main into this branch (44c3e9bf), so the #485 Rust 1.98 Clippy fix and the new baseline are part of the head.

• No conflicts; the PR's four gate files are unchanged by the merge (net diff vs main is still test files only, +1065).
• Merge commit carries a DCO Signed-off-by trailer.
• Local validation re-run on the merged head: cargo test --workspace → 1079 passed / 0 failed; the three concurrency-sensitive gates stable across 5 consecutive runs (6+1+2 passed); cargo clippy --workspace --all-targets -- -D warnings and cargo fmt --all -- --check clean.

CI is now running against the updated head.

[Allan10w]

The nokv-workspace failure on this head is a flaky main-side test, not one of this PR's gates:

• Failing step: Test restore crash support; failing test: executor::tests::completed_fork_concurrent_finalize_replays_exact_manifest_publication — panicked with RpcFailure { code: PreconditionFailed, message: "restore is Ready, expected DestinationBuilding" } at crates/nokv-server/src/executor.rs:11179.
• This PR's net diff vs main is test files only (crates/nokv/tests/*, +1065); nokv-server is byte-identical to main on this head.
• The same test passed on main's own Rust workflow for 609a29b0 (run 32635913416), and passed 30/30 consecutive runs locally on the merged head. The test drives four barrier-synchronized concurrent finalizers and is timing-sensitive.
• Clippy passed on this head (the Rust 1.98 issue is resolved by the main merge); the rest of the job is green.

I do not have admin rights on NoKV-Lab/NoKV, so I cannot trigger a rerun myself. Could you rerun the failed nokv-workspace job, or I can push a signed no-op commit to re-trigger CI if you prefer.