Repository navigation
fix(executor): keep runtime task status in memory - #2399
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR separates transient execution state from persisted task metadata. It preserves in-memory state during index reloads, normalizes local task links during collection, and updates message and rollback guards to use active-task status. ChangesRuntime execution-state synchronization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LocalTaskCollection
participant RuntimeTaskLink
participant RuntimeWorkStore
participant TaskHandler
LocalTaskCollection->>RuntimeTaskLink: apply local execution-state normalization
RuntimeTaskLink-->>LocalTaskCollection: report whether state changed
LocalTaskCollection->>RuntimeWorkStore: update_task_execution_state
TaskHandler->>RuntimeTaskLink: check is_active_local_task
RuntimeTaskLink-->>TaskHandler: active or inactive result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
qdaxb
marked this pull request as ready for review
August 4, 2026 03:38
qdaxb
enabled auto-merge
August 4, 2026 03:38
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@executor/src/runtime_work/store.rs`:
- Around line 138-143: Update update_task_execution_state so its
update_task_with_persistence closure copies execution state only when the
persisted task and runtime_state have matching archive states; otherwise leave
the refreshed task unchanged to preserve archive intent.
- Around line 207-214: The refresh-and-write workflow in RuntimeWorkStore must
be atomic across processes, not only protected by the in-process index Mutex.
Update refresh_index_from_disk_if_changed() and write_index() so signature
checking, disk reload, runtime-state merge, mutation, and replacement of the
shared index occur while holding an inter-process lock for the index file;
ensure all writers use the same lock and preserve deleted-task markers. Add a
two-process concurrency test covering simultaneous upserts and verifying neither
process’s update is lost.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a1669029-d22b-4e85-bbea-dd3bc8b79393
📒 Files selected for processing (6)
executor/src/runtime_work/handler/collection.rsexecutor/src/runtime_work/handler/helpers/routing.rsexecutor/src/runtime_work/handler/tasks.rsexecutor/src/runtime_work/response.rsexecutor/src/runtime_work/store.rsexecutor/tests/app_runtime_work_native_update_contract.rs
Comment on lines
+138
to
+143
| pub fn update_task_execution_state(&self, runtime_state: &RuntimeTaskLink) { | ||
| self.update_task_with_persistence( | ||
| &runtime_state.local_task_id, | ||
| |task| task.copy_execution_state_from(runtime_state), | ||
| false, | ||
| ); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Prevent execution-state updates from removing archive intent.
A caller can hold an active task snapshot while another process archives the same task. Line 152 refreshes the archived task from disk, but Line 141 then copies the stale status into it. A later persistent mutation serializes archived: false.
Only copy execution state when the stored task and runtime_state have the same archive state.
Proposed fix
pub fn update_task_execution_state(&self, runtime_state: &RuntimeTaskLink) {
self.update_task_with_persistence(
&runtime_state.local_task_id,
- |task| task.copy_execution_state_from(runtime_state),
+ |task| {
+ if (task.status == "archived") == (runtime_state.status == "archived") {
+ task.copy_execution_state_from(runtime_state);
+ }
+ },
false,
);
}As per coding guidelines, fix the primary flow rather than hiding defects behind fallbacks.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub fn update_task_execution_state(&self, runtime_state: &RuntimeTaskLink) { | |
| self.update_task_with_persistence( | |
| &runtime_state.local_task_id, | |
| |task| task.copy_execution_state_from(runtime_state), | |
| false, | |
| ); | |
| pub fn update_task_execution_state(&self, runtime_state: &RuntimeTaskLink) { | |
| self.update_task_with_persistence( | |
| &runtime_state.local_task_id, | |
| |task| { | |
| if (task.status == "archived") == (runtime_state.status == "archived") { | |
| task.copy_execution_state_from(runtime_state); | |
| } | |
| }, | |
| false, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@executor/src/runtime_work/store.rs` around lines 138 - 143, Update
update_task_execution_state so its update_task_with_persistence closure copies
execution state only when the persisted task and runtime_state have matching
archive states; otherwise leave the refreshed task unchanged to preserve archive
intent.
Source: Coding guidelines
Comment on lines
+207
to
214
| let mut disk_index = read_index_from_path(&self.index_path); | ||
| if let Ok(mut index) = self.index.lock() { | ||
| for (task_id, disk_task) in &mut disk_index.tasks { | ||
| if let Some(current_task) = index.tasks.get(task_id) { | ||
| disk_task.preserve_runtime_state_from(current_task); | ||
| } | ||
| } | ||
| *index = disk_index; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f . | rg '(^|/)(Cargo\.toml|.*(lock|store).*\.rs)$' | sort
rg -n -C2 'lock_exclusive|lock_shared|try_lock|flock|FileExt|fs2' \
executor shared Cargo.toml 2>/dev/null || trueRepository: wecode-ai/Wegent
Length of output: 307
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== store outline =="
ast-grep outline executor/src/runtime_work/store.rs --view expanded || true
echo "== relevant store.rs lines =="
nl -ba executor/src/runtime_work/store.rs | sed -n '1,260p'
echo "== runtime_work references =="
rg -n -C2 'RuntimeWork(store|Store|Task).*|store::|RuntimeStore|runtime_work|preserve_runtime_state_from|read_index_from_path|write_index_to_path|lock\(' executor/src | sed -n '1,260p'
echo "== lock dependency presence in cargo manifests =="
rg -n 'fs2|flock|lock_exclusive|lock_shared|try_lock|lockfile|FileExt|tokio::sync::Mutex|lock' Cargo.toml */Cargo.toml 2>/dev/null || trueRepository: wecode-ai/Wegent
Length of output: 3023
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== executor/src/runtime_work/store.rs lines 1-280 =="
awk '{printf "%5d\t%s\n", NR, $0}' executor/src/runtime_work/store.rs | sed -n '1,280p'
echo "== executor/src/runtime_work/store.rs lines 280-430 =="
awk '{printf "%5d\t%s\n", NR, $0}' executor/src/runtime_work/store.rs | sed -n '280,430p'
echo "== concurrent/shared index usages =="
rg -n -C3 'RuntimeWorkStore|runtime_work_store|shared index|concurrent|two process|fs2|flock|FileExt|lock_exclusive|lock_shared|try_lock|mutex\(\)|sync::Mutex|lock\(\)' executor shared wework Cargo.toml 2>/dev/null || trueRepository: wecode-ai/Wegent
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cargo tooling =="
command -v cargo || true
command -v rustc || true
echo "== rust std write/read lock docs presence in installed docs? =="
command -v rustdoc || trueRepository: wecode-ai/Wegent
Length of output: 146
Make shared-index mutation atomic across processes.
RuntimeWorkStore protects in-process state with Mutex, but refresh_index_from_disk_if_changed() reads the file outside that lock and write_index() writes the on-disk file after the Mutex is released. In a two-process scenario, one process can stale-load, then overwrite another process’s newer task or deleted-task marker when it renames the temp index file. Move the signature check/reload/mutate/write critical section under an inter-process file lock for the shared index file, and add a concurrent two-process test for concurrent upserts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@executor/src/runtime_work/store.rs` around lines 207 - 214, The
refresh-and-write workflow in RuntimeWorkStore must be atomic across processes,
not only protected by the in-process index Mutex. Update
refresh_index_from_disk_if_changed() and write_index() so signature checking,
disk reload, runtime-state merge, mutation, and replacement of the shared index
occur while holding an inter-process lock for the index file; ensure all writers
use the same lock and preserve deleted-task markers. Add a two-process
concurrency test covering simultaneous upserts and verifying neither process’s
update is lost.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@executor/src/runtime_work/store.rs`:
- Around line 301-315: The restore logic around legacy_status currently
preserves terminal values in RuntimeTaskLink.status; change it so restored
non-archived tasks always use "active", while archived tasks use "archived".
Retain any terminal outcome through turn_status instead, and add a migration
test covering a persisted task with status "done" to verify this behavior.
- Around line 329-334: Update is_terminal_turn_status to recognize the codex
success aliases “complete” and “succeeded” as terminal statuses alongside
“completed”. Add or extend tests covering both alternate statuses while
preserving the existing failure, interruption, cancellation, and completion
cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 153b840d-d7a9-462f-b1f4-f042ffc9fd48
📒 Files selected for processing (1)
executor/src/runtime_work/store.rs
Comment on lines
+301
to
+315
| let restored_status = legacy_status | ||
| .filter(|status| { | ||
| matches!( | ||
| status.trim().to_ascii_lowercase().as_str(), | ||
| "done" | "cancelled" | "canceled" | "failed" | ||
| ) | ||
| }) | ||
| .unwrap_or_else(|| "active".to_owned()); | ||
| task.insert( | ||
| "status".to_owned(), | ||
| Value::String(if archived { | ||
| "archived".to_owned() | ||
| } else { | ||
| restored_status | ||
| }), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject persisted terminal task statuses.
The PR requires persisted terminal execution statuses to be rejected. This code restores legacy "done", "cancelled", and "failed" values into RuntimeTaskLink.status.
Keep status as "active" after restore unless archived is true. Preserve the terminal outcome only in turn_status. Add a migration test for a persisted task with status: "done".
Proposed fix
- let restored_status = legacy_status
- .filter(|status| {
- matches!(
- status.trim().to_ascii_lowercase().as_str(),
- "done" | "cancelled" | "canceled" | "failed"
- )
- })
- .unwrap_or_else(|| "active".to_owned());
task.insert(
"status".to_owned(),
Value::String(if archived {
"archived".to_owned()
} else {
- restored_status
+ "active".to_owned()
}),
);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@executor/src/runtime_work/store.rs` around lines 301 - 315, The restore logic
around legacy_status currently preserves terminal values in
RuntimeTaskLink.status; change it so restored non-archived tasks always use
"active", while archived tasks use "archived". Retain any terminal outcome
through turn_status instead, and add a migration test covering a persisted task
with status "done" to verify this behavior.
Comment on lines
+329
to
334
| fn is_terminal_turn_status(status: &str) -> bool { | ||
| matches!( | ||
| status.replace(['_', '-'], "").to_ascii_lowercase().as_str(), | ||
| "completed" | "done" | "failed" | "error" | "interrupted" | "cancelled" | "canceled" | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline executor/src/runtime_work/transcript.rs --view expanded
rg -n -C 6 \
'normalized_phase_or_status|fn turn_status|turn_status\s*=|turn_status:' \
executor/src/runtime_work executor/src/agents/codex/run_state.rs
rg -n -C 4 \
'is_terminal_turn_status|"complete"|"completed"|"succeeded"' \
executor/src/runtime_work executor/src/agents/codex/run_state.rsRepository: wecode-ai/Wegent
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the actual assignments/read paths for turn_status without broad output.
for f in \
executor/src/runtime_work/store.rs \
executor/src/runtime_work/response.rs \
executor/src/runtime_work/transcript.rs \
executor/src/agents/codex/run_state.rs \
executor/src/runtime_work/handler/collection.rs \
executor/src/runtime_work/handler/tasks.rs \
executor/src/runtime_work/handler/helpers/routing.rs
do
echo "==== outline $f ===="
ast-grep outline "$f" --view compact 2>/dev/null | sed -n '1,180p' || true
done
printf '\n==== direct turn_status occurrences (filtered) ====\n'
rg -n -C 4 '\bturn_status\b' executor/src/runtime_work executor/src/agents/codex/run_state.rs | sed -n '1,240p'
printf '\n==== codex run_state completion predicates ====\n'
sed -n '320,370p' executor/src/agents/codex/run_state.rs
printf '\n==== transcript turn_status and normalized conversion ====\n'
sed -n '850,870p' executor/src/runtime_work/transcript.rs
sed -n '1475,1485p' executor/src/runtime_work/transcript.rs
printf '\n==== store predicate and trimming calls ====\n'
sed -n '260,335p' executor/src/runtime_work/store.rsRepository: wecode-ai/Wegent
Length of output: 22976
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n==== response turn_status derivation ====\n'
sed -n '830,900p' executor/src/runtime_work/response.rs
sed -n '200,230p' executor/src/runtime_work/response.rs
printf '\n==== transcript completion predicates ====\n'
sed -n '838,850p' executor/src/runtime_work/transcript.rs
printf '\n==== handler collection mapping ====\n'
sed -n '745,762p' executor/src/runtime_work/handler/collection.rs
printf '\n==== handler routing fallback mapping ====\n'
sed -n '1,18p' executor/src/runtime_work/handler/helpers/routing.rs
sed -n '210,226p' executor/src/runtime_work/response.rs
printf '\n==== all literal runtime/status strings that may affect turn_status matching ====\n'
rg -n -C 2 '"complete"|"completed"|"succeeded"|"done"|"cancelled"|"canceled"|"interrupted"|"aborted"|"failed"|"failure"|"error"|"systemerror"|"running"|"completed"' executor/src/runtime_work executor/src/agents/codex/run_state.rsRepository: wecode-ai/Wegent
Length of output: 50373
Align terminal turn-status aliases with codex completion handling.
is_terminal_turn_status is used when archiving/restoring tasks and currently preserves only "completed" for successful turns. Codex treats "complete" and "succeeded" as completed, so include codex success variants in this predicate and add tests for the alternate statuses.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@executor/src/runtime_work/store.rs` around lines 329 - 334, Update
is_terminal_turn_status to recognize the codex success aliases “complete” and
“succeeded” as terminal statuses alongside “completed”. Add or extend tests
covering both alternate statuses while preserving the existing failure,
interruption, cancellation, and completion cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
status,running,thread_status, and non-terminalturn_statusout of the shared runtime task indexturn_statusvalues as durable turn history without treating them as live execution statearchivedmetadata and migrate legacystatus: archivedentries on readWhy
Production and development Wework processes can share the same runtime task index. Persisted execution status allowed one process to overwrite another process's live
runningstate, so a running task could briefly appear completed until opening the task caused the live state to be derived again.Live execution status is process-local runtime state and must not be persisted or accepted from another process. Terminal turn outcomes remain durable history for completed, failed, interrupted, or cancelled turns.
Impact
Running and terminal task presentation remains stable within the owning Wework process when another process writes the shared task index. Archive state remains durable and compatible with existing index files.
Validation
cargo fmt --manifest-path executor/Cargo.toml -- --checkcargo test --manifest-path executor/Cargo.toml --lib runtime_work::(201 passed)cargo test --manifest-path executor/Cargo.toml --lib(590 passed, 1 ignored)cargo test --manifest-path executor/Cargo.toml(all unit and integration tests passed)node wework/e2e/desktop/run-checkpoints.mjs --segment core-task-flowgit diff --checkSummary by CodeRabbit