test: exercise engine snapshot updates under real parallelism

[abdallahsamabd]

Summary

Adds concurrency coverage for the policy engine so registration, hot reload, and the route cache are exercised the way a host actually uses them: one PolicyEngine behind Arc, invokes in flight, mutations on other threads.

The engine already serializes writers with runtime_write. This change does not touch that. It makes the test suite able to see real OS-thread overlap, assert that a successful register is not dropped, and check the generation / snapshot Release/Acquire pairing.

What changed

• Engine tests for registration, unregister, load_config / from_config, and the route cache now run with #[tokio::test(flavor = "multi_thread")].
• New seeded stress tests in crates/ppe-core/tests/engine_concurrency.rs:
  • eight threads barrier-register distinct plugins; every Ok must still be visible
  • N invoke tasks vs M mutators (register / unregister / annotate / reload); lost-update assertion, coherent invoke, cache refill after quiesce
  • replay with PPE_STRESS_SEED (optional PPE_STRESS_OPS, PPE_STRESS_INVOKERS, PPE_STRESS_MUTATORS)
• Loom model in crates/ppe-core/tests/loom_generation_snapshot.rs: if a reader Acquire-loads a higher generation, it must see the snapshot stored before the Release bump.
• Nightly ThreadSanitizer job (.github/workflows/nightly-tsan.yml, make test-tsan) runs the stress tests under TSan. PR CI is unchanged.
• CONTRIBUTING.md: when a new test needs flavor = "multi_thread".

Closes #22.

Test plan

• cargo test -p praxis-policy-core --test engine_concurrency --test loom_generation_snapshot
• cargo test -p praxis-policy-core --lib
• Optional: PPE_STRESS_SEED=12345 cargo test -p praxis-policy-core --test engine_concurrency stress_invoke -- --nocapture
• Confirm PR CI (make test) stays green; TSan is nightly-only

[praxis-bot]

PR Review

Summary: Well-structured concurrency test suite. Two issues: the published counter in mutator_loop may over-count for no-op mutations (risking generation assertion flakes), and the blanket multi_thread switch on 25 existing tests contradicts the CONTRIBUTING.md guidance added in this same PR.

Severity	Count
Critical	0
Large	0
Medium	2

[praxis-bot]

[Medium] published += 1 is unconditional here, but multiple mutators race to remove the same annotation ("tool", TOOL, None, HOOK). When a second mutator hits case 3 after the first already removed the annotation, the call is a no-op. If the engine does not bump config_generation for a no-op removal, published over-counts and the later assertion generation >= generation_at_start + published can fail non-deterministically.

Same pattern at line 431 (case 1): published += 1 is outside the if engine.unregister(&name).is_some() check, so a failed unregister still increments the counter. The unique-name-per-mutator design makes this less likely in practice, but the code doesn't enforce that invariant at this level.

Suggestion: guard published += 1 behind the success check in both cases, or if the engine always bumps generation even for no-op mutations, add a comment on the assertion explaining that contract.

[abdallahsamabd]

unregister now increments published only on Some. remove_route_annotation still increments: mutate_runtime always publishes and bumps generation, even when the key is missing, so that count is a snapshot publish, not a hit. The assertion stays generation >= start + published

[praxis-bot]

[Medium] The CONTRIBUTING.md section added in this PR says: "Sequential logic — one engine, one task, no shared mutation — stays on current_thread." But this test and several others in the 25 switched here (e.g. test_from_config_creates_manager, test_from_config_unknown_kind_rejected) appear to be sequential single-task tests with no concurrent mutation.

Either narrow the multi_thread changes to tests that actually share + mutate concurrently, or update the CONTRIBUTING.md guidance to explain why blanket multi-thread is preferred for all engine tests.

[abdallahsamabd]

Intentional for this set of surfaces, not every engine test. CONTRIBUTING now says registration / unregister / load_config / route-cache tests use multi_thread even as a single task, because that is the runtime a host uses. Overlap is asserted in engine_concurrency and test_manager_arc_shareable_with_concurrent_dispatch_and_registration. Sequential tests that don’t touch those APIs stay on current_thread

[praxis-bot]

PR Review

Summary: Force-pushed since last review. Both previous Medium findings are resolved: published += 1 in the mutator loop is now guarded behind success checks (cases 0, 1, 4), with a clear comment explaining why case 3 counts unconditionally (mutate_runtime always bumps generation). The CONTRIBUTING.md guidance is revised to justify blanket multi_thread for engine-surface tests while keeping current_thread for tests that don't touch shared surfaces.

No new findings.

Severity	Count
Critical	0
Large	0
Medium	0