Skip to content

fix(edge): make registry claim settlement idempotent - #636

Merged
gouhongshen merged 1 commit into
moi-devfrom
codex/fix-edge-registration-claim
Sep 2, 2026
Merged

fix(edge): make registry claim settlement idempotent#636
gouhongshen merged 1 commit into
moi-devfrom
codex/fix-edge-registration-claim

Conversation

@gouhongshen

Copy link
Copy Markdown
Collaborator

What type of PR is this?

  • feat (new feature)
  • fix (bug fix)
  • docs (documentation)
  • style (formatting, no code change)
  • refactor (code change that neither fixes a bug nor adds a feature)
  • perf (performance improvement)
  • test (adding or updating tests)
  • chore (maintenance, tooling)
  • build / ci (build or CI changes)

Which issue(s) this PR fixes

N/A — fixes the Edge Registry registration-claim failure observed in QA trace trace_39c691bb97457a4e139c0235451da391.

What this PR does / why we need it

Edge registration previously treated a zero affected-row count from finalize, release, or rollback as proof that the claim had been superseded. With MatrixOne cross-session visibility, a transition can temporarily observe the same claim in its preceding state. The Edge WebSocket path then removes a valid connection as if ownership had been lost, leaving the registry row stuck in finalized state with its claim retained.

This PR centralizes durable claim settlement in DatabaseEdgeRegistryService. Each transition now:

  • locks and reads the canonical registry row in a short transaction;
  • classifies the typed state as already applied, still owned, or superseded;
  • applies and verifies the transition before committing;
  • retries a bounded number of times while the same claim still owns the row;
  • returns false only after a different claim or missing owner is verified.

Finalize, release, and rollback are idempotent for their completed states. A genuinely newer generation remains protected from stale rollback or release.

Architecture and complexity delta

  • Canonical owner changed or extended: DatabaseEdgeRegistryService remains the sole owner of durable Edge registration transitions.
  • Existing implementations/callers searched: EdgeRegistryService, DatabaseEdgeRegistryService, and the Edge WebSocket registration/finalize/release lifecycle.
  • Superseded code, states, tables, shims, and self-only tests removed: three independent one-shot SQL transitions based on rows_affected were replaced by one typed settlement path. No state or table was added.
  • Net code/state/table delta: +503/-92 lines; one internal transition enum, one internal decision enum, and one internal persisted-state projection; zero schema/table/status additions.
  • If parallel implementations remain, the external boundary and retirement condition: non-durable EdgeRegistryService implementations remain for their existing backend boundary and do not manage durable claims.

Production wiring and verification

  • Public product entrypoint exercised: Edge WebSocket registration lifecycle tests edge_ws_auth_ok_precedes_claim_release_wait and claim_loss_after_pool_commit_removes_the_unpublished_connection.
  • Unhappy paths exercised: stale claim superseded by a newer generation, idempotent finalize/release/rollback, exact predecessor restoration, and cross-pod claim serialization.
  • Database schema/query/transaction/migration verified against a real database, or N/A with reason: verified against local MatrixOne 4.1.2 using all 9 edge_registry_ DB integration tests, including direct assertions of registration_state and registration_claim_id. No schema migration is included.

Additional verification:

  • cargo test -p astra-services multi_agent::edge_registry::tests --lib -- --nocapture
  • cargo test -p astra-services --test edge_dispatch_db_it edge_registry_ -- --ignored --test-threads=1 --nocapture
  • cargo test -p astra-runtime --test edge_ws_e2e edge_ws_auth_ok_precedes_claim_release_wait -- --nocapture
  • cargo test -p astra-runtime --test edge_ws_e2e claim_loss_after_pool_commit_removes_the_unpublished_connection -- --nocapture
  • cargo check -p astra-services
  • cargo check -p astra-runtime
  • cargo clippy -p astra-services --lib -- -D warnings
  • cargo fmt --all -- --check
  • git diff --check

@gouhongshen
gouhongshen marked this pull request as ready for review September 2, 2026 03:55
@gouhongshen
gouhongshen changed the base branch from main to moi-dev September 2, 2026 04:55
@gouhongshen
gouhongshen marked this pull request as draft September 2, 2026 04:55
@gouhongshen
gouhongshen marked this pull request as ready for review September 2, 2026 05:58
@gouhongshen
gouhongshen merged commit f8bb3f2 into moi-dev Sep 2, 2026
24 of 25 checks passed
gouhongshen added a commit that referenced this pull request Sep 3, 2026
## What type of PR is this?

- [ ] feat (new feature)
- [x] fix (bug fix)
- [ ] docs (documentation)
- [ ] style (formatting, no code change)
- [x] refactor (code change that neither fixes a bug nor adds a feature)
- [x] perf (performance improvement)
- [x] test (adding or updating tests)
- [ ] chore (maintenance, tooling)
- [ ] build / ci (build or CI changes)

## Which issue(s) this PR fixes

N/A — this follows the database-latency traces investigated with
matrixorigin/matrixflow#16389. It is rebased on current `moi-dev`,
including #636, and contains only the post-#632 runtime-latency work and
the canonical-compaction correctness fix exposed by end-to-end testing.

## What this PR does / why we need it

This PR supersedes #637. It carries the same verified commit tree from
the contributor fork so the upstream repository no longer owns the
feature branch.

High MatrixOne latency magnifies every serialized runtime read and
post-loop write. A warm, no-sandbox MOI turn was spending most of its
user-visible time before provider inference and after the final model
output.

### Reduce canonical turn admission round trips

- Loads the session head, active writer, and authority epochs as one
read-only admission snapshot.
- Atomically acquires the canonical writer and reserves the next turn in
one transaction while preserving cursor, epoch, lease, idempotency, and
fencing checks.
- Runs distributed admission, immutable-history materialization, and
canonical authority admission concurrently; every partial-success branch
explicitly releases the facts it acquired.
- Reuses the task-board snapshot loaded during resume for the current
turn and refreshes it after model/tool execution.

### Reduce runtime capability and lifecycle persistence cost

- Captures Agent Binding Tool/Skill discovery once and prepares the
runtime bundles from that frozen snapshot.
- Batches trace events and edges under the existing idempotency/session
lock instead of issuing a transaction per projection.
- Reads task-board state as one consistent MatrixOne statement and
batches messaging claim/release operations.
- Reuses durable provider-attempt, invocation, quota, usage, and run
state already loaded within the same lifecycle boundary.
- Keeps the merged quota reads on the same UTC day key as quota writers
by binding the existing runtime-computed date; this preserves one
database round trip without depending on the MatrixOne session timezone.
- Preserves remote composite-snapshot write-through at every
heavy-checkpoint boundary, so a process or pod crash cannot erase an
in-memory projection debt or hide the latest recoverable checkpoint from
a replacement runtime.
- Runs independent prompt, memory, admission, resume, terminal, usage,
transcript, hook, observer, promotion, and projection work concurrently
without moving any correctness dependency across its durable boundary.

### Restore terminal publication ordering while overlapping independent
writes

- Completes the existing best-effort usage, transcript-materialization,
CSL, hook-event, promotion, state, and context-trace projection attempts
before publishing terminal SSE, restoring the pre-optimization ordering
while overlapping independent stores.
- Keeps success gated by the canonical journal/context transaction,
durable provider invocation/attempt settlement, and fenced terminal
transition. Derived projections retain their established `moi-dev` error
semantics and do not turn a completed model response into a
failed/retried inference.
- Leaves only memory/workspace cleanup after client detachment; no
outbox, retry fallback, or additional database write is introduced.
- Caches compiled prompt static sections on `PipelineSession`: repeated
model rounds reuse them, while every newly created or restored session
reads the current prompt override files instead of inheriting a
process-global first-read value.

### Preserve canonical compaction correctness

- Separates the admitted manifest-chain root from the materialized
conversation-content root instead of comparing values from different
hash domains.
- Keeps manifest fencing at commit and content-prefix verification at
compaction, so unrelated history mutation remains fail-closed.
- Adds a regression test whose manifest root intentionally differs from
the conversation root, matching segmented canonical storage in
production.

### Keep CI contracts explicit after reordering

- Preserves the existing `peek_next_task_id failed` error context when
MatrixOne's atomic task-board snapshot rejects a corrupt allocator.
- Makes persistence journeys wait for the exact per-turn event types, so
earlier trace-detail rows cannot be mistaken for the turn's
context-trace event.
- Adopts the Rust 1.97 Clippy equivalents without changing runtime
behavior.

No heuristic fast path, tool/Skill suppression, schema migration, or
authorization relaxation is introduced.

## Architecture and complexity delta

- Canonical owner changed or extended: the session context coordinator
owns combined admission snapshots and atomic writer-plus-turn
reservation; runtime lifecycle owns terminal publication ordering and
typed canonical rewrite proof validation; `PipelineSession` owns its
immutable compiled prompt sections.
- Existing implementations/callers searched: server run admission,
session coordinator, inference attempt lifecycle, runtime capability
discovery, task board, messaging transport, trace/event persistence,
finalization, compaction, and CLI state construction.
- Superseded code, states, tables, shims, and self-only tests removed:
repeated preflight queries and serialized independent projection calls
are replaced in place; no compatibility shim or parallel store is added.
- Net code/state/table delta: ten non-merge commits plus one `moi-dev`
synchronization merge, 33 files; no database table or migration change.
- If parallel implementations remain, the external boundary and
retirement condition: N/A; prompt static-section reuse is owned by
`PipelineSession`, and all Server terminal durability uses one lifecycle
boundary.

## Production wiring and verification

- Public product entrypoint exercised: paired local MOI `/chat/stream`
traffic through Astra Server and EPH Sandbox, including warm follow-up
turns under fixed SQL latency.
- Unhappy paths exercised: authority/cursor conflicts, partial
distributed-admission failures, provider interruption,
retry/idempotency, missing runtime capability snapshots, and CLI
construction.
- Database schema/query/transaction/migration verified against a real
database, or `N/A` with reason: local MatrixOne-backed end-to-end
deployment was exercised; no schema or migration changes are present.
- `cargo fmt --all -- --check` — passed.
- `cargo check -p astra-runtime -p astra-cli` — passed after rebasing
onto current `moi-dev`.
- `cargo test -p astra-services session_context_coordinator --lib` — 13
passed after rebasing onto current `moi-dev`.
- `cargo check -p astra-services` — passed after the quota timezone
review fix.
- `cargo test -p astra-services resource_governor --lib` — 17 passed
after the quota timezone review fix.
- `cargo clippy -p astra-services --all-targets -- -D warnings` — passed
after the quota timezone review fix.
- `cargo test -p astra-runtime
compaction_commits_the_complete_replacement_projection --lib` — passed.
- `cargo test -p astra-runtime
unrelated_prefix_mutation_after_compaction_is_rejected --lib` — passed.
- `cargo test -p astra-runtime
compaction_cannot_authorize_an_already_mutated_prefix --lib` — passed.
- `cargo clippy -p astra-messaging -p astra-tools -p astra-runtime
--all-targets -- -D warnings` — passed.
- `cargo clippy -p astra-turn-core -p astra-runtime --all-targets -- -D
warnings` — passed after the review fixes.
- `cargo test -p astra-turn-core
static_sections_cache_is_scoped_to_pipeline_session` — passed.
- `cargo test -p astra-runtime --lib
production_fanout_batches_slow_durable_writes_before_terminal` — passed.
- `cargo test -p astra-runtime --lib
durable_stream_chat_persists_final_state` — passed.
- `cargo test -p astra-runtime heavy_checkpoint --lib` — 6 passed after
restoring per-checkpoint remote write-through.
- `cargo test -p astra-runtime
render_final_text_called_once_at_completion --lib` — passed; the
output-settled UI boundary remains before checkpoint settlement.
- `cargo test -p astra-services
assistant_output_settled_is_exposed_in_both_event_shapes --lib` —
passed.
- `cargo clippy -p astra-runtime --all-targets -- -D warnings` — passed
after the checkpoint review fix.
- `cargo check -p astra-cli --all-targets` and `cargo clippy -p
astra-cli --all-targets -- -D warnings` — passed after removing the
retired checkpoint-deferral fields from the CLI state initializer.
- `cargo test -p astra-runtime --test session_todos_cross_client_e2e
snapshot_restore_uses_existing_rows_when_matrixone_counter_is_zero --
--ignored --exact --test-threads=1` — passed against local MatrixOne.
- `cargo test -p astra-runtime --features bridge-e2e-hooks --test
system_matrix_http_e2e e2e_matrix_stream_context_trace_persistence --
--ignored --exact --test-threads=1` — passed against local MatrixOne.
- `cargo test -p astra-runtime --features bridge-e2e-hooks --test
system_matrix_http_e2e e2e_matrix_stream_multi_turn_persistence --
--ignored --exact --test-threads=1` — passed against local MatrixOne.
@XuPeng-SH
XuPeng-SH deleted the codex/fix-edge-registration-claim branch September 3, 2026 04:12
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