Skip to content

feat(session): expose scoped forks for MOI conversations - #1147

Open
Lundomn wants to merge 6 commits into
matrixorigin:mainfrom
Lundomn:codex/feat-17540-moi-session-forks
Open

Lundomn wants to merge 6 commits into
matrixorigin:mainfrom
Lundomn:codex/feat-17540-moi-session-forks

Conversation

@Lundomn

@Lundomn Lundomn commented Oct 26, 2026

Copy link
Copy Markdown

Capability assessment: Unchanged: Agent chat, Context and cache keep their existing Beta profiles. Provider-authenticated access to canonical Session forks adds no execution authority, profile, maturity claim or external-effect replay guarantee.

Summary

MOI needs to branch from an exact completed conversation turn. This change admits its existing request-signed provider credentials to run cursor resolution, fork preparation and activation for continuation. The selected run must belong to the authenticated provider subject and workspace.

The existing Session fork coordinator activates a shared immutable prefix without acquiring a writer lease. The next ordinary conversation request obtains its own execution authority. Existing runtime and Edge user tokens retain their canonical-head prepare behavior: the authentication owner reports the verified mechanism so only request-HMAC credentials receive the new provider-scope prerequisite.

Related issue

Prerequisite for matrixorigin/matrixflow#17540. The MOI UI, attachment ownership and conversation publication are delivered separately in Matrixflow.

Change type

  • Feature
  • Bug fix
  • Documentation
  • Refactor or performance improvement
  • Test
  • Build, CI, or maintenance

User and compatibility impact

  • GET /sessions/{session_id}/runs/{run_id}/cursor resolves the exact retained cursor of a committed root conversation run.
  • POST /sessions/{session_id}/forks accepts request-signed provider credentials while preserving existing user-token behavior.
  • POST /sessions/{session_id}/forks/{fork_id}/activate-for-continuation activates the prepared prefix and returns its child head without a writer lease. Replays return the current head without rewinding subsequent turns.
  • Invalid signatures, nonce replay, wrong or absent durable scope, incomplete or unsupported source runs and unavailable retained roots fail with the documented stable error codes.

Existing list/get/abort, writer activation and Work fork routes retain their authentication and execution contracts. Fork creation copies no transcript rows, runs, writer leases, approvals or sandbox authority. Current canonical history and terminal transactions handle first continuation, nested branches and retained ancestry after intermediate Session deletion. No storage schema migration is introduced. Deploy this prerequisite before enabling MOI's branch action.

Architecture and complexity delta

  • Canonical owner changed or extended: the existing Session fork coordinator, request authentication verifier, Session HTTP handlers and router.
  • Existing implementations and callers searched: Session and Work forks, canonical history retention, terminal transcript settlement, provider authentication and ordinary MOI HTTP consumers.
  • Superseded code, states, tables, shims, or self-only tests removed: no parallel lifecycle or storage projection is introduced. Context materialization and Session deletion use the current upstream owners unchanged.
  • If parallel implementations remain, their boundary and retirement condition: N/A. Writer activation and continuation activation share the same fork owner; only writer activation obtains a lease.

Verification

Reviewed head: 4dee48ea3b3ad8aaffb26b036c20e048cfdfb60c, based on 203aed8ac9b4a08398725ade7fd043c0b7657f5b. Independent final source review passed on this exact head, including the feature implementation, final rebase and test-fixture-only delta. No unresolved material source finding remains. The selected local validation below is complete. Broader CI and deployment checks remain separate.

  • Full existing MOI consumer target: cargo test --locked -p astra-runtime --features moi-compat-tests --test moi_fresh_astra -- --include-ignored --nocapture --test-threads=1 passed all 10 tests, with zero ignored or filtered tests. This includes the original seven consumers, one complete branching journey and two authentication compatibility tests. The existing always-on Test: MOI fresh-Astra compatibility job executes this same target and feeds the online-core aggregate; no extra workflow is added.
  • Existing fork owner: cargo test --locked -p astra-services --lib session_fork_coordinator::tests -- --ignored --nocapture --test-threads=1 passed both database tests, including no-copy and million-token-prefix coverage.
  • Existing Work consumer: cargo test --locked -p astra-runtime --test work_http_db_it work_fork_is_exact_durable_owner_scoped_and_preserves_a_retained_prefix -- --ignored --nocapture --test-threads=1 passed.
  • Those three behavior runs used 551cf8bcfa3fe70ddf82fce7264acb79e2a5bed1. The subsequent documentation correction and TUI-only upstream rebase leave the feature behavior, dependencies and configuration unchanged. The final commit only warms the existing HTTP client before four pre-existing timing tests; no product code or original budget/assertion changes. Unaffected behavior evidence is reused on these verified deltas.
  • Feature-specific lint (verified on 9af0d50f, with unchanged feature files after rebase): cargo clippy --locked -p astra-runtime --features moi-compat-tests --test moi_fresh_astra -- -D warnings passed on 9af0d50f.
  • Original make check passed on 9af0d50f: workspace Clippy with warnings denied, formatting, cargo check --all-targets, SDK build and Web typecheck. The final rebase adds only upstream CLI TUI changes; the final runtime/CLI static supplement below passed, and unaffected checks are reused.
  • On b5319822 (unchanged by the final test-fixture delta), both existing canonical terminal database regressions passed: canonical_terminal_success_commits_evidence_usage_and_terminal_together and canonical_terminal_status_conflict_rolls_back_canonical_evidence_and_usage, each run with cargo test --locked -p astra-runtime --lib <case> -- --ignored --nocapture --test-threads=1 (one executed case each).
  • Original make test-contract passed on b5319822 (unchanged by the final test-fixture delta): 11 admin-contract, six HTTP-contract and two core settings-contract tests passed; the six existing ignored admin cases remain ignored.
  • The first runtime library inventory executed 5,680 cases: 5,676 passed and four short-timing cases failed before reaching the phase they assert; 105 originally ignored cases were skipped. The four failed cases reproduced in a focused run. Their existing HTTP client is now initialized before the measured request phase, matching nearby fixtures; all four passed their focused verification on the final head (0.966s) under the same CI profile and original assertions. The other 5,676 unmodified passing cases are reused by exact case identity. Original failures are retained, and the initial whole run is not reported as passing.
  • Runtime CI inventory is complete: all 60 default targets were enumerated under cargo nextest run --manifest-path Cargo.toml -p astra-runtime --profile ci, using each original --lib, --bin or --test target selector. The 6,171-case list has 5,984 runnable cases with passing evidence (5,676 reused + four corrected + 304 other cases executed on the final head) and 187 originally ignored cases. Twenty-two targets had no runnable cases and are recorded as enumeration only. All exact case identities and per-target results were retained; this is composite coverage, not a fresh single passing full-library invocation.
  • make test-history-work NEXTEST_OFFLINE_PROFILE=ci passed both originally ignored history-work cases on the final head (0.081s).
  • The complete turn-core/services/plan/prompts CI inventory (shard-c) passed on the final head: 92 targets, 5,458 passed, zero failed and 485 originally ignored cases. All four package selections remained enabled in every target batch under the original CI profile and budgets; no additional runnable case was filtered. Fifty batches had no runnable cases and are recorded as enumeration only. Exact target/test manifests and results were retained.
  • Final-head static supplement: cargo clippy --locked --manifest-path Cargo.toml -p astra-runtime -p astra-cli --all-targets -- -D warnings passed (7m38s). make format-check passed (15.01s). Both verified the frozen 2,592-file source set.
  • Public entrypoint exercised: the compatibility target uses real signed HTTP routes, disposable MatrixOne databases and captured model input; ordinary MOI consumers remain in the same target.
  • Unhappy paths exercised by the target: tampered method/path/body and nonce replay; wrong or absent durable scope; unsupported or unavailable source cursor; idempotency conflict/replay; continuation after activation replay; parent/sibling isolation; nested history retention and cleanup; legacy-token compatibility and conservative custom-auth provenance.
  • Database verification: current upstream fork-owner and retained Work-prefix regressions passed. The HTTP journey passed inherited-content, independent-continuation, no-history-copy, intermediate-deletion and final-cleanup checks. Both original atomic terminal success/rollback regressions passed on the final head. No schema change is introduced and no end-to-end latency or throughput claim is made.
  • SQL cost: source proof uses a fixed set of run/opening/marker/manifest reads for an independent root; inherited-boundary resolution adds reads per lineage hop. The canonical opening decoder validates up to its existing 8 MiB bound. Legacy list controls do not perform provider authorization per manifest. Legacy writer activation adds one fixed manifest reload. No arbitrary-depth constant-time or zero-overhead claim is made.
  • CI scope: the official classifier selects Rust, CLI, runtime, services, core and online checks. The local plan covers the affected runtime/services inventories; CLI/core and broader online inventories remain CI work. SDK/Web behavioral suites are not locally run. This is not a claim that the entire make test-offline or all CI passed.
  • Local environment: macOS arm64, Rust 1.97.0 and MatrixOne 4.2.5. Live model/provider, QA and production deployment checks have not run. GitHub CI is pending.

Final checklist

  • I added or updated tests at the layer that owns the behavior, or explained why no test is needed.
  • I updated public or design documentation for contract changes, or the change needs no documentation update.
  • I checked the diff for credentials, private URLs, customer data, generated files, and other sensitive information.
  • The PR title follows the repository's Conventional Commit format.

@Lundomn
Lundomn requested a review from XuPeng-SH as a code owner October 26, 2026 20:18
@Lundomn Lundomn self-assigned this Oct 26, 2026

@XuPeng-SH XuPeng-SH left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is ready to approve yet. The fork coordinator reuse is a good direction, but the complete provider workflow has the following gaps:

  1. Cross-workspace continuation can use another workspace's history. Provider identity is derived from provider ID and external subject, while ordinary /chat/stream resolves the session by user and session ID without checking provider scope. A user with a valid request for workspace B and the child session ID from workspace A can continue that child; a local isolated MatrixOne/HTTP test confirmed that the model request included A's inherited history. This chat behavior predates the PR, but the new fork API relies on it for continuation, so the workspace isolation needs to be enforced at the common session/run admission boundary.

  2. Re-forking an inherited head fails after deleting the middle ancestor. For A → B → C, with C still at its inherited head, the same fork request succeeds before deleting B and returns 403 session_fork_forbidden afterward. C's retained history still exists, but authorization follows the prefix to B, whose transcript and run records were deleted. The existing test checks continued chat after deletion, but not forking the retained head. Authorization evidence and retained history need compatible lifetimes without retaining execution authority.

  3. Interrupted prepare can leave a child and pin indefinitely. Prepare creates an active child session and a prepared manifest pin. Provider-signed callers cannot use the existing abort endpoint, and orphan cleanup requires the child session to be absent. Retrying with the same idempotency key can complete the operation, but cancellation or loss of the pending operation leaves no provider cleanup path. Please define and exercise recovery/cancellation for this lifecycle.

  4. Fork creation bypasses the configured session creation quota. The coordinator inserts the child session directly without the normal quota check or creation count. This coordinator behavior also predates the PR, but the new signed endpoint exposes it to provider clients. Unique idempotency keys can create children beyond a configured limit; retries must not be charged twice.

Separately, please assess the cost of loading and validating the full run opening during cursor lookup, prepare, and activation. Each opening can be up to 8 MiB; I have not benchmarked the impact. A bounded common read path would be preferable to a duplicate authorization projection.

Validation: I extended the existing HTTP fixture temporarily against an isolated MatrixOne instance and a mock model. It reproduced findings 1 and 2, including a successful re-fork before ancestor deletion and a 403 afterward. Findings 3 and 4 are based on the creation, authentication, quota and cleanup code paths. The PR's executed CI checks are green; the live regression lane was skipped.

@XuPeng-SH XuPeng-SH left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for the four findings in my preceding review, especially the reproduced cross-workspace history exposure and the retained-head re-fork failure after ancestor deletion.

This branch has not been deployed

No deployments
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.

2 participants