fix(work): make establishment durable and identity-safe

[XuPeng-SH]

Summary

• persist semantic Work establishment before the synthetic lifecycle carrier
• recover exact pending operations by session and turn-chain identity
• separate provider-native tool identity from public aliases and fail closed when absent
• keep lifecycle phases monotonic and return errors instead of fake success on planning/assignment failure
• use fresh provider invocation IDs while retaining stable operation identity for retries

Verification

• cargo test -p astra-runtime --lib (5250 passed, 0 failed, 43 ignored)
• cargo test -p astra-runtime-env --lib (112 passed)
• make lint (passed)
• cargo fmt --all -- --check (passed)
• services full unit suite: one known flaky workspace-concurrency test failed in the full run and passed on isolated rerun; targeted Work schema test passed

The implementation does not alter prompt-visible cache-prefix assembly; it only carries typed runtime authority and durable recovery metadata.

[XuPeng-SH]

Deep review of f656d74, from first principles and without treating backward compatibility as a constraint.

Assessment: not ready to merge. Separating durable operation identity from delivery/invocation identity is the right foundation, but the recovery protocol still violates four correctness invariants detailed inline:

1. A committed continuation must resume the same proposal/task identities, not re-author a different plan.
2. An admitted semantic decision must be recovered before asking a model to decide again.
3. Deferred cancel/replace/add obligations must survive the same crash boundary as the initial plan.
4. An operation requiring activation must not become Complete without an actual assignment receipt.

These affect user-visible behavior: a transient failure can make continuation unretryable, a restarted turn can reject its own admitted request, a deferred cancellation/replacement can disappear, and a blocked assignment can be reported as a successful continuation.

Testing: the new MatrixOne operation-admission/CAS/concurrency tests are useful, but they do not exercise the host-to-handler recovery protocol at its transaction seams. Each inline finding includes a concrete regression scenario. Please use a fresh host/executor after injected failures and assert durable rows, exact proposal/task/attempt identities, and whether side effects were admitted—not only serialized status strings or hash/helper equality.

Verification performed:

• git diff --check: passed.
• scripts/schema/test_schema_inventory.py: 47 tests passed.
• scripts/harness/test_fresh_database_contract.py: 16 tests ran, with 2 environment errors because this runner denies Unix sockets; these are not being reported as PR regressions.
• Rust/Cargo is unavailable locally, so the Rust failure scenarios below are code-path findings, not claimed executed reproductions.
• GitHub Static Checks failed in Clippy at crates/runtime/src/turn/agentic_loop/lifecycle.rs:682–683 (unnecessary_lazy_evaluations; then should be then_some): Static Checks log. The Test Suite was still running when checked.

Submitting COMMENT because the authenticated account is also the PR author; this is not an approval.

[XuPeng-SH]

[P1] Resume the committed continuation instead of regenerating its proposal

Every nonterminal continuation reaches continue_bound_work(), including an operation already in AwaitingAssignment. That helper reloads the current plan context and allocates the first unused task-N IDs, while proposal_invocation above fixes the proposal ID to the operation ID.

For example, start with task-1, commit a continuation adding task-2, then fail assignment (or crash before recording the next phase). Retrying the same operation sees task-2 in the graph and proposes task-3 with a new context_id. tool_work_plan::propose finds the already accepted proposal under the same ID, but verify_retry_identity rejects the changed source_ref as work_plan_invocation_conflict, retryable=false. A transient failure has made the durable operation unrecoverable through its advertised retry.

Persist/reuse the original proposal context and task allocation, and resume assignment from the accepted proposal/phase instead of recompiling. Recovery must also handle a committed proposal whose phase advance did not commit; checking AwaitingAssignment alone does not cover that window.

Regression test: drive the real continuation handler with a bound graph; inject failures immediately after plan commit, after phase advance, and after assignment commit. Retry the same operation with a fresh invocation/executor. Assert one accepted proposal, exactly the original added task IDs, no duplicate attempt, and eventual Complete.

[XuPeng-SH]

[P1] Recover the persisted admission before recomputing semantic intent

The only production hydration call runs after the new semantic judgment and admit_semantic_work_operation(). Consider a crash after the pending operation is committed but before genesis binds Work. A fresh host is still unbound, so it runs the judge again. Even a wording change in goal/objective/expected_result changes start_work_operation_id(), which hashes the full arguments.

Admission then encounters the original pending operation. Since both requests belong to the same turn-chain, cancel_pending_for_new_turn correctly refuses replacement and admission returns ContractViolation. The ? here exits before hydrate_pending_work_establishment can recover the exact persisted payload. Recovery therefore depends on an LLM reproducing byte-identical output.

Load and validate pending operation authority during bootstrap, before new semantic admission/provider execution, and resume that immutable decision. Preserve the same-turn conflict guard; it is the ordering/source of authority that needs fixing.

Regression test: persist admission, terminate the host before genesis, and recreate it with the same turn-chain but a judge deliberately returning different task text (and a separate unavailable-judge case). Recovery must use the original operation/graph without creating another row or dispatching the regenerated plan.

[XuPeng-SH]

[P1] Restore deferred graph obligations together with the lifecycle carrier

This recovery path restores only start_work arguments. A Required decision also contains deferred_graph_mutations, but canonical_start_work_payload persists only goal/tasks/activation plus turn_chain_id. The deferred mutations are copied into pending_work_graph_mutations only by the fresh-admission branch of take_admitted_work_establishment_call; an already hydrated pending carrier returns before that branch.

If the user requested "after the first result, cancel/replace the second task", a crash after genesis but before establishment finishes restores the original task graph while losing that cancellation/replacement. The new host initializes the mutation list empty, and Work-bound admission normally skips the judge. Both the pending mutation prompt and the execution gate therefore disappear, allowing the original second task to proceed. The existing semantic admission test asserts an in-memory mutation count, so it does not protect this boundary.

Persist the admitted mutation obligations and their consumed/pending state, then restore them before dispatch. Their durability must last until the corresponding accepted graph mutations, not merely until establishment is marked Complete.

Regression test: admit two tasks plus an exact deferred cancel/replace, restart after genesis and again after establishment completion, settle the first task, and assert that the original second task cannot execute before the required mutation is accepted exactly once.

[XuPeng-SH]

[P1] Require an assigned result before completing an activating operation

continue_bound_work treats every non-error, parseable JSON response from execute_run_next_work_item as successful assignment, then returns status=continued with dispatch_error=null. This block consequently seals the operation as Complete without checking next_task.status. The initial-establishment branch has the same issue.

The scheduler explicitly returns non-error statuses needs_recovery, blocked, in_flight, and complete as well as assigned. A concrete continuation case is an existing failed/undelivered task: next_foreground_task prioritizes NeedsRecovery over newly appended Ready tasks. No attempt is assigned, yet start_work reports "execute_assigned_task_then_call_settle_work_item" and permanently removes the operation from pending recovery.

Use a typed assignment outcome and validate the assigned item/attempt identity before committing Complete for activation=start. Non-assignment outcomes need an explicit blocked/recovery disposition, not an execution-success receipt; activation=defer should remain a separate intentional no-assignment case.

Regression test: exercise start_work against a bound graph with a failed undelivered task and against an in-flight task owned by another run. Assert no false Complete or execute-assigned receipt and no unauthorized attempt. Cover the full outcome matrix plus the successful resumed-assignment case.

[XuPeng-SH]

Deep re-review of commit 433de14.

Assessment: changes are still needed. The durable-recovery design has improved, but the new deterministic mutation path can execute a replacement before the task that the user explicitly required to finish first. See the inline finding and deterministic identity counterexample.

Previous review follow-up (source inspection, not a claim that the MatrixOne tests ran here):

• Pending operation hydration now precedes semantic classification and is repeated as an execution barrier.
• The complete admission decision is persisted with the canonical payload.
• Plan retry identity excludes the changing context_id while retaining the typed mutation payload; additions derive stable IDs from the operation.
• Activation requires an assigned receipt matching the active attempt, item revision, and executor run.
• Bound start_work now rejects an unrelated continuation and directs it to the planning surface, so the previous continuation-regeneration path is no longer the same implementation.

The remaining issue illustrates an architectural distinction: durable/idempotent identity does not encode execution precedence. Preserving exact mutations across a crash is useful only if their timing and ordering still satisfy the user's request. The new database replay tests exercise valuable boundaries, but their single-initial-task replacement case and assignment-count assertions do not cover an earlier outstanding task plus replacement/addition ordering. The changed work_implicit_replacement_journey case still explicitly asks for the first task to finish before replacement work, but does not assert which task is assigned first.

Review scope: fetched the live PR diff and history; traced establishment/admission/recovery, proposal replay and scheduler selection, and sampled the new deferred invocation identity, context compaction, and shell isolation paths. This is a risk-directed review of a very large change (325 files, approximately 60k added lines), not certification of every changed path. Please also refresh the PR description: its current five-item summary does not describe the substantial tool-surface, prompt/context, shell, handoff, and completion changes in this head.

Validation actually performed:

• git diff --check against dd69973: passed.
• scripts/schema/test_schema_inventory.py: 47 tests passed.
• scripts/harness/test_fresh_database_contract.py: 16 tests ran; 14 passed and 2 errored because this environment denies Unix sockets. These are environment failures, not reported regressions.
• Harbor adapter tests could not import because the harbor package is unavailable.
• Executed an independent Python calculation of the exact source hash inputs and canonical item ordering for the inline counterexample. This is a deterministic algorithm check, not an executed Rust/MatrixOne handler reproduction.
• Rust/Cargo is unavailable locally; no Rust unit, database integration, or live-provider/harness run is claimed.
• GitHub Static Checks and Test Suite were still in progress on this exact head; PR Title passed.

The head was rechecked before submission. Submitting COMMENT because the connected account XuPeng-SH is also the PR author; this is not an approval.

[XuPeng-SH]

[P2] Preserve requested execution order when materializing replacement tasks

This proposal inserts every admitted addition/replacement with no dependency edges, and execute_start_work applies it before assigning the first task. compile_graph_addition_items gives the new task a hash-based ID, while WorkTaskExecutionSnapshot::from_parts sorts IDs and next_foreground_task selects the first ready item. Consequently a replacement can run before an outstanding earlier task. This directly contradicts the checked-in work_implicit_replacement_journey prompt: finish the first task, then cancel/replace the second and complete its outcome.

A deterministic example using the exact hash rules: owner=owner-review, session=session-review, turn-chain=turn-7, and start_work arguments {"goal":"Verify README then Cargo package","activation":"start","tasks":[{"objective":"Read README first line","expected_result":"README evidence"},{"objective":"Read Cargo package name","expected_result":"Cargo evidence"}]} produce operation tool-adef960669db1b8f7dd5abeb2a298d9a02649b6cf4023e6574f4554c59b0a0ca. Admit cancel(task-2) plus add of its outstanding outcome (or replace(task-2)). The addition becomes task-0dfd2277d17a079b91d3352a463e01e801ec52d562c425c1-1, which sorts before task-1. Both are immediately ready, so the replacement is assigned first. Changing the session/turn identity can change user-visible execution order.

Keep stable operation IDs, but represent explicit user precedence separately and carry it through replacement/addition; if the mutation itself is required after settlement, retain that trigger instead of applying it unconditionally during establishment. Do not serialize genuinely independent tasks merely to mask the hash ordering.

Add a real handler/scheduler regression with two initial tasks and a replacement/addition ID sorting before task-1. Assert the first assignment is task-1, the replacement cannot execute until the required predecessor is delivered, and the same order survives fresh-executor recovery after mutation commit. The existing one-task replacement replay test cannot catch this.

[XuPeng-SH]

Re-review of c648bb8, focusing on the 24-file update since 433de14 and its surrounding execution/recovery paths.

No new actionable blocking finding in the reviewed update. The previous P2 ordering issue is addressed at the implementation level; I am not claiming an executed database reproduction of the fix.

Evidence:

• Explicit task prerequisites now survive semantic admission, the canonical payload, and initial graph compilation. Immediate replacement inherits its target's predecessors, so a replacement ID sorting before task-1 no longer grants readiness ahead of that predecessor. Independent tasks remain independent.
• Mutation timing is represented separately from task precedence. Delayed groups are reconstructed from immutable establishment payloads even after establishment completes; accepted proposal IDs identify already-applied groups. Trigger reads use the original item revision's delivery, rather than equating an execution outcome with delivery.
• The scheduler exposes a due-mutation barrier, and begin_attempt also checks due mutations and dependency satisfaction under the assignment transaction. Settlement reconciles due groups before advancing to another task. This makes the boundary more than a prompt instruction.
• Reviewed the terminal-cut recovery path after settlement/mutation commit and the new same-group replacement endpoint substitution, cross-trigger prerequisite checks, and cycle rejection.
• The guidance-race change preserves a typed GuidancePending cause, distinguishes it from cancellation/owner loss, and routes it through normal guidance consumption before rebuilding a request. Its error path does not simply retry stale provider input.
• Turn evaluation now distinguishes successful tool observations from the settled run outcome.

Test quality is materially better. The three new MatrixOne handler tests cover a deliberately adverse replacement hash, delayed cancel/add before and after delivery with fresh-executor recovery, and terminal-cut recovery across both commit windows. Compiler tests also cover independent peers, same-group substitutions, invalid references/cycles, and incompatible cross-trigger retirements. The guidance test covers applicable guidance, missing guidance, and an unavailable scope.

Validation limits:

• Executed git diff --check for 433de14..c648bb8: passed.
• Rust/Cargo is unavailable locally, so I did not execute Rust tests, the MatrixOne integration tests, or the live-provider journey. The three new handler tests are explicitly ignored unless the database integration lane is enabled; ordinary unit-suite success alone does not establish that they passed.
• GitHub Static Checks and PR Title passed on this head; Test Suite was still in progress at the final check.
• This is a focused re-review of the new delta, building on the prior review, not a fresh exhaustive certification of all 334 files in the full PR. Earlier test results were not rerun or represented as results for this commit.

The head was rechecked immediately before submission. Submitting COMMENT because the connected account XuPeng-SH is the PR author.