Repository navigation
chore(wework): upgrade bundled Codex to 0.147.0 - #2498
Conversation
📝 WalkthroughWalkthroughThe PR updates bundled Codex binaries to ChangesCodex release update
Retry transcript reconciliation
Memory E2E validation
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Workbench
participant RuntimeHandler
participant TranscriptAssembler
Workbench->>Workbench: Capture failed message turnId or subtaskId
Workbench->>RuntimeHandler: Send retrySourceTurnId
RuntimeHandler->>RuntimeHandler: Record superseded transcript turn
RuntimeHandler->>TranscriptAssembler: Assemble transcript
TranscriptAssembler->>TranscriptAssembler: Remove superseded turn messages
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/handler/helpers/transcript.rs`:
- Around line 43-50: Update the messages.retain predicate to independently
inspect turnId, turn_id, subtaskId, and subtask_id, rejecting the message if any
present identity is in superseded_turn_ids rather than short-circuiting through
or_else. Add a test covering a message with both turn and subtask fields where
only subtaskId matches.
In `@executor/src/runtime_work/handler/tasks.rs`:
- Around line 1148-1152: The retry source ID flow must persist only canonical
nonblank identifiers: update retry_source_turn_id in
executor/src/runtime_work/handler/tasks.rs:1148-1152 to trim the selected value
before returning it, and update the comparison logic in
executor/src/runtime_work/handler/tasks.rs:423-434 to compare trimmed
identifiers or rely on the normalized retry_source_turn_id result.
In `@wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts`:
- Line 1181: Update the retry source selection in the workbench runtime
messaging flow to choose the first nonblank value between failedMessage.turnId
and failedMessage.subtaskId, rather than relying on nullish coalescing. Preserve
a valid turnId when it contains non-whitespace content, and fall back to
subtaskId when turnId is empty or whitespace-only.
🪄 Autofix
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: 50478156-3fb3-48fc-be1e-4069f26f1194
📒 Files selected for processing (8)
.github/workflows/wework-e2e.ymlexecutor/src/runtime_work/handler/helpers/transcript.rsexecutor/src/runtime_work/handler/queries.rsexecutor/src/runtime_work/handler/tasks.rsexecutor/src/runtime_work/handler/tests.rswework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/types/api.ts
| messages.retain(|message| { | ||
| string_field(message, "turnId") | ||
| .or_else(|| string_field(message, "turn_id")) | ||
| .or_else(|| string_field(message, "subtaskId")) | ||
| .or_else(|| string_field(message, "subtask_id")) | ||
| .map_or(true, |turn_id| { | ||
| !superseded_turn_ids.contains(&turn_id.as_str()) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Check every transcript identity field before retaining a message.
The or_else chain checks only turnId when a message also has subtaskId. If the retry source is the matching subtaskId, the failed message remains visible when turnId differs.
Reject a message when any of turnId, turn_id, subtaskId, or subtask_id matches a superseded ID. Add a test with both fields present and only subtaskId matching.
🤖 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/handler/helpers/transcript.rs` around lines 43 -
50, Update the messages.retain predicate to independently inspect turnId,
turn_id, subtaskId, and subtask_id, rejecting the message if any present
identity is in superseded_turn_ids rather than short-circuiting through or_else.
Add a test covering a message with both turn and subtask fields where only
subtaskId matches.
| fn retry_source_turn_id(payload: &Value) -> Option<String> { | ||
| string_field(payload, "retrySourceTurnId") | ||
| .or_else(|| string_field(payload, "retry_source_turn_id")) | ||
| .filter(|turn_id| !turn_id.trim().is_empty()) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Normalize retry source IDs before persisting them.
Line 1151 rejects blank values but preserves surrounding whitespace. Line 432 then treats whitespace variants as distinct IDs. This violates the unique nonblank ID contract and grows supersededTranscriptTurnIds unnecessarily.
executor/src/runtime_work/handler/tasks.rs#L1148-L1152: trim the selected identifier before returning it.executor/src/runtime_work/handler/tasks.rs#L423-L434: compare canonical trimmed identifiers, or rely on the normalized parser result.
📍 Affects 1 file
executor/src/runtime_work/handler/tasks.rs#L1148-L1152(this comment)executor/src/runtime_work/handler/tasks.rs#L423-L434
🤖 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/handler/tasks.rs` around lines 1148 - 1152, The
retry source ID flow must persist only canonical nonblank identifiers: update
retry_source_turn_id in executor/src/runtime_work/handler/tasks.rs:1148-1152 to
trim the selected value before returning it, and update the comparison logic in
executor/src/runtime_work/handler/tasks.rs:423-434 to compare trimmed
identifiers or rely on the normalized retry_source_turn_id result.
| address: state.currentRuntimeTask, | ||
| message: previousUserMessage.content, | ||
| clientUserMessageId: previousUserMessage.id, | ||
| retrySourceTurnId: failedMessage.turnId ?? failedMessage.subtaskId, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Select the first nonblank retry source ID.
?? selects an empty or whitespace-only turnId instead of a valid subtaskId. The executor then ignores the retry source, so it does not remove the failed turn from the transcript.
Proposed fix
- retrySourceTurnId: failedMessage.turnId ?? failedMessage.subtaskId,
+ retrySourceTurnId: failedMessage.turnId?.trim() || failedMessage.subtaskId?.trim(),📝 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.
| retrySourceTurnId: failedMessage.turnId ?? failedMessage.subtaskId, | |
| retrySourceTurnId: failedMessage.turnId?.trim() || failedMessage.subtaskId?.trim(), |
🤖 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 `@wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts` at line 1181,
Update the retry source selection in the workbench runtime messaging flow to
choose the first nonblank value between failedMessage.turnId and
failedMessage.subtaskId, rather than relying on nullish coalescing. Preserve a
valid turnId when it contains non-whitespace content, and fall back to subtaskId
when turnId is empty or whitespace-only.
cedc7be to
c422951
Compare
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 `@wework/e2e/desktop/task-flow.e2e.mjs`:
- Around line 186-193: The developer guides must match the concurrent-memory
thresholds used by the e2e test instead of documenting the obsolete 800 MiB
physical-footprint contract. Update the Chinese guide, then the English guide,
to document the growth and settled-sample-range thresholds, the corresponding
WEWORK_E2E_CONCURRENT_MEMORY_MAX_PEAK_GROWTH_KIB,
WEWORK_E2E_CONCURRENT_MEMORY_MAX_SETTLED_GROWTH_KIB, and
WEWORK_E2E_CONCURRENT_MEMORY_MAX_SETTLED_SAMPLE_RANGE_KIB variables, their KiB
units, and defaults of 320*1024, 256*1024, and 64*1024.
- Around line 3705-3717: Update captureStableTotalMemorySamples so it fails when
MEMORY_MAX_BASELINE_SAMPLES are collected without the recent range reaching
MEMORY_MAX_SAMPLE_RANGE_KIB, instead of returning an unstable baseline. Preserve
the existing successful return path once the minimum samples are available and
the range stabilizes, and ensure verifyConcurrentTaskMemory does not proceed
with unstable samples.
🪄 Autofix
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: 74f2dcf3-0fbc-4366-ac14-9f25cd5100bc
📒 Files selected for processing (7)
.github/workflows/wework-e2e.ymlexecutor/src/runtime_work/handler/tasks.rsexecutor/src/runtime_work/handler/tests.rswework/e2e/desktop/task-flow.e2e.mjswework/src/features/workbench/WorkbenchProvider.test.tsxwework/src/features/workbench/useWorkbenchRuntimeMessaging.tswework/src/types/api.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/wework-e2e.yml
- wework/src/features/workbench/useWorkbenchRuntimeMessaging.ts
- wework/src/types/api.ts
- executor/src/runtime_work/handler/tests.rs
- wework/src/features/workbench/WorkbenchProvider.test.tsx
- executor/src/runtime_work/handler/tasks.rs
| const CONCURRENT_MEMORY_MAX_PEAK_GROWTH_KIB = Number( | ||
| process.env.WEWORK_E2E_CONCURRENT_MEMORY_MAX_PEAK_GROWTH_KIB ?? 320 * 1024 | ||
| ) | ||
| const CONCURRENT_MEMORY_MAX_SETTLED_GROWTH_KIB = Number( | ||
| process.env.WEWORK_E2E_CONCURRENT_MEMORY_MAX_SETTLED_GROWTH_KIB ?? 256 * 1024 | ||
| ) | ||
| const CONCURRENT_MEMORY_MAX_SETTLED_SAMPLE_RANGE_KIB = Number( | ||
| process.env.WEWORK_E2E_CONCURRENT_MEMORY_MAX_SETTLED_SAMPLE_RANGE_KIB ?? 64 * 1024 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 \
'WEWORK_E2E_CONCURRENT_MEMORY_MAX_PHYSICAL_FOOTPRINT_KIB|800 MiB|concurrent-memory' \
docs wework .githubRepository: wecode-ai/Wegent
Length of output: 5254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== task-flow config and verify function =="
sed -n '175,198p' wework/e2e/desktop/task-flow.e2e.mjs
sed -n '3460,3565p' wework/e2e/desktop/task-flow.e2e.mjs
sed -n '3540,3575p' wework/e2e/desktop/task-flow.e2e.mjs
echo
echo "== docs surrounding concurrent-memory =="
sed -n '170,185p' docs/zh/wework/developer-guide/wework-e2e-automation.md
sed -n '170,195p' docs/en/wework/developer-guide/wework-e2e-automation.md
echo
echo "== variables referenced in task-flow =="
rg -n 'CONCURRENT_MEMORY_MAX_(PEAK|SETTLED)_GROWTH_KIB|CONCURRENT_MEMORY_MAX_SETTLED_SAMPLE_RANGE_KIB|CONCURRENT_MEMORY_MAX_PHYSICAL_FOOTPRINT_KIB|verifyConcurrentTaskMemory|physicalFootprint|physical_footprint' wework/e2e/desktop/task-flow.e2e.mjsRepository: wecode-ai/Wegent
Length of output: 16946
Keep the developer guides after the concurrent-memory threshold change.
wework/e2e/desktop/task-flow.e2e.mjs still uses the old 800 MiB absolute contract and WEWORK_E2E_CONCURRENT_MEMORY_MAX_PHYSICAL_FOOTPRINT_KIB. Update docs/zh/wework/developer-guide/wework-e2e-automation.md first, then docs/en/wework/developer-guide/wework-e2e-automation.md, to describe the growth and range thresholds, their KiB units, and the current defaults.
🤖 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 `@wework/e2e/desktop/task-flow.e2e.mjs` around lines 186 - 193, The developer
guides must match the concurrent-memory thresholds used by the e2e test instead
of documenting the obsolete 800 MiB physical-footprint contract. Update the
Chinese guide, then the English guide, to document the growth and
settled-sample-range thresholds, the corresponding
WEWORK_E2E_CONCURRENT_MEMORY_MAX_PEAK_GROWTH_KIB,
WEWORK_E2E_CONCURRENT_MEMORY_MAX_SETTLED_GROWTH_KIB, and
WEWORK_E2E_CONCURRENT_MEMORY_MAX_SETTLED_SAMPLE_RANGE_KIB variables, their KiB
units, and defaults of 320*1024, 256*1024, and 64*1024.
Source: Coding guidelines
| async function captureStableTotalMemorySamples(control, phase) { | ||
| const samples = [] | ||
| while (samples.length < MEMORY_MAX_BASELINE_SAMPLES) { | ||
| if (samples.length > 0) { | ||
| await new Promise(resolvePromise => setTimeout(resolvePromise, 1_000)) | ||
| } | ||
| samples.push(await captureTotalMemorySample(control, phase)) | ||
| if (samples.length < MEMORY_MIN_BASELINE_SAMPLES) continue | ||
| const recent = samples.slice(-MEMORY_SAMPLE_WINDOW_SIZE) | ||
| if (memorySampleRangeKiB(recent) <= MEMORY_MAX_SAMPLE_RANGE_KIB) break | ||
| } | ||
| return samples | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail when the baseline does not stabilize.
The loop returns after MEMORY_MAX_BASELINE_SAMPLES even when the recent sample range exceeds MEMORY_MAX_SAMPLE_RANGE_KIB. verifyConcurrentTaskMemory then uses this unstable baseline for all growth assertions. This can hide a regression or create a flaky failure.
Proposed fix
while (samples.length < MEMORY_MAX_BASELINE_SAMPLES) {
// ...
}
+ const baselineWindow = samples.slice(-MEMORY_SAMPLE_WINDOW_SIZE)
+ assert.ok(
+ memorySampleRangeKiB(baselineWindow) <= MEMORY_MAX_SAMPLE_RANGE_KIB,
+ 'The concurrent memory E2E could not capture a stable baseline window'
+ )
return samples
}📝 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.
| async function captureStableTotalMemorySamples(control, phase) { | |
| const samples = [] | |
| while (samples.length < MEMORY_MAX_BASELINE_SAMPLES) { | |
| if (samples.length > 0) { | |
| await new Promise(resolvePromise => setTimeout(resolvePromise, 1_000)) | |
| } | |
| samples.push(await captureTotalMemorySample(control, phase)) | |
| if (samples.length < MEMORY_MIN_BASELINE_SAMPLES) continue | |
| const recent = samples.slice(-MEMORY_SAMPLE_WINDOW_SIZE) | |
| if (memorySampleRangeKiB(recent) <= MEMORY_MAX_SAMPLE_RANGE_KIB) break | |
| } | |
| return samples | |
| } | |
| async function captureStableTotalMemorySamples(control, phase) { | |
| const samples = [] | |
| while (samples.length < MEMORY_MAX_BASELINE_SAMPLES) { | |
| if (samples.length > 0) { | |
| await new Promise(resolvePromise => setTimeout(resolvePromise, 1_000)) | |
| } | |
| samples.push(await captureTotalMemorySample(control, phase)) | |
| if (samples.length < MEMORY_MIN_BASELINE_SAMPLES) continue | |
| const recent = samples.slice(-MEMORY_SAMPLE_WINDOW_SIZE) | |
| if (memorySampleRangeKiB(recent) <= MEMORY_MAX_SAMPLE_RANGE_KIB) break | |
| } | |
| const baselineWindow = samples.slice(-MEMORY_SAMPLE_WINDOW_SIZE) | |
| assert.ok( | |
| memorySampleRangeKiB(baselineWindow) <= MEMORY_MAX_SAMPLE_RANGE_KIB, | |
| 'The concurrent memory E2E could not capture a stable baseline window' | |
| ) | |
| return samples | |
| } |
🤖 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 `@wework/e2e/desktop/task-flow.e2e.mjs` around lines 3705 - 3717, Update
captureStableTotalMemorySamples so it fails when MEMORY_MAX_BASELINE_SAMPLES are
collected without the recent range reaching MEMORY_MAX_SAMPLE_RANGE_KIB, instead
of returning an unstable baseline. Preserve the existing successful return path
once the minimum samples are available and the range stabilizes, and ensure
verifyConcurrentTaskMemory does not proceed with unstable samples.
Summary
0.146.0-alpha.9.2to stable0.147.0for macOS, Linux, and Windows targetsCompatibility
codex exec --full-autoflag.--strict-configwith the new sidecar.Validation
pnpm --filter wework exec vitest run scripts/prepare-codex-binary.test.mjspnpm --filter wework exec prettier --check ../docs/zh/wework/developer-guide/wework-macos-release.md ../docs/en/wework/developer-guide/wework-macos-release.mdCODEX_HOME=~/.wework/codex <bundled-codex> --strict-config --versionpnpm --filter wework ai:verify startflow, includingruntime.codex.ensure_startedandcodex.app_server_requestSummary by CodeRabbit
New Features
Bug Fixes
Documentation