Skip to content

fix(ci): restore test workflow execution - #1999

Merged
qdaxb merged 6 commits into
mainfrom
fix/ci-test-workflow
Jul 16, 2026
Merged

qdaxb merged 6 commits into
mainfrom
fix/ci-test-workflow

Conversation

@qdaxb

@qdaxb qdaxb commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

What changed

  • Move the executor Cargo target-directory setup out of the job-level expression context.
  • Export CARGO_TARGET_DIR through GITHUB_ENV using the runner-provided RUNNER_TEMP environment variable.

Why

The Tests workflow referenced runner.temp from a job-level env block, where the runner context is unavailable. GitHub rejected the entire workflow during parsing, so none of the unit-test jobs, including Test Wework, were created while the separate Wework E2E workflow continued to run.

Impact

Pull requests and protected-branch pushes can run the normal Tests workflow again. Executor build artifacts remain on the runner temporary disk.

Validation

  • actionlint -ignore 'shellcheck reported issue' -ignore 'runner of .* action is too old' .github/workflows/test.yml
  • git diff --check origin/main...HEAD

Summary by CodeRabbit

  • Bug Fixes

    • Improved macOS executor signal-handler compatibility.
    • Increased reliability of runtime, IPC, backend, and streaming behavior tests.
    • Improved handling of interrupted executor tasks and runtime authentication paths.
  • Documentation

    • Clarified required module checks for pull requests.
    • Documented the distinction between module tests and separate end-to-end test workflows.
  • Chores

    • Updated continuous integration coverage reporting and Rust environment setup.
    • Improved test failure handling and build-cache verification.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Updates CI workflows and testing documentation, while aligning executor source and contract tests with current runtime, IPC, backend path, streaming, synchronization, and signal-handling behavior.

Changes

CI testing and documentation

Layer / File(s) Summary
Workflow execution and coverage updates
.github/workflows/test.yml
CI jobs source Cargo environment files, configure CARGO_TARGET_DIR through GITHUB_ENV, handle shared-test exit code 134, and upgrade Codecov actions.
Workflow test documentation
docs/en/developer-guide/testing.md, docs/zh/developer-guide/testing.md
Testing guides document ordered module checks, summary-job behavior, and separate E2E workflows.

Executor contract tests

Layer / File(s) Summary
Executor compatibility contracts
executor/src/bin/wegent-executor.rs, executor/src/browser_mcp.rs, executor/tests/build_entrypoint_contract.rs
Signal-handler casting, browser request arguments, and dev-sidecar cache assertions are updated.
Runtime transcript contracts
executor/tests/app_runtime_work_contract.rs
Obsolete transcript-cache and running-tool coverage is removed, provider paging coverage is renamed, and transcript-read expectations are updated.
Send flow synchronization and fixtures
executor/tests/app_runtime_work_send_contract.rs
Environment locking, transcript assertions, cancellation timing, and fake Codex turn scripts are updated.
IPC environment locking and shutdown
executor/tests/local_app_ipc_contract.rs
IPC tests use standard-library environment locks and await aborted server tasks before cleanup.
Backend paths and streaming events
executor/tests/local_backend_contract.rs
Backend tests derive runtime auth paths, serialize environment changes, collect large-message deltas, and broaden environment helper compilation.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested reviewers: ficohu

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: restoring the CI test workflow so it executes again.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ci-test-workflow

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@qdaxb
qdaxb force-pushed the fix/ci-test-workflow branch from cc5ca83 to 8c0fec0 Compare July 16, 2026 17:17
@qdaxb
qdaxb marked this pull request as ready for review July 16, 2026 17:24
@qdaxb
qdaxb merged commit fba6e2c into main Jul 16, 2026
26 of 27 checks passed
@qdaxb
qdaxb deleted the fix/ci-test-workflow branch July 16, 2026 17:27

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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/tests/app_runtime_work_send_contract.rs`:
- Line 8: Update the test lock initialization and acquisition around StdMutex
and StdMutexGuard so a poisoned mutex is recovered instead of causing a later
.expect(...) failure. Preserve the lock’s serialization behavior by accepting
the guard from the poison error and returning a usable StdMutexGuard to callers.
🪄 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

Run ID: 96a72981-bda0-43ad-a188-9159a4a419ed

📥 Commits

Reviewing files that changed from the base of the PR and between 2bfd2bd and 8c0fec0.

📒 Files selected for processing (10)
  • .github/workflows/test.yml
  • docs/en/developer-guide/testing.md
  • docs/zh/developer-guide/testing.md
  • executor/src/bin/wegent-executor.rs
  • executor/src/browser_mcp.rs
  • executor/tests/app_runtime_work_contract.rs
  • executor/tests/app_runtime_work_send_contract.rs
  • executor/tests/build_entrypoint_contract.rs
  • executor/tests/local_app_ipc_contract.rs
  • executor/tests/local_backend_contract.rs

fs,
path::{Path, PathBuf},
sync::OnceLock,
sync::{Mutex as StdMutex, MutexGuard as StdMutexGuard, OnceLock},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the test file and inspect the relevant section with line numbers.
ast-grep outline executor/tests/app_runtime_work_send_contract.rs --view expanded || true
echo '--- file excerpt ---'
sed -n '1,120p' executor/tests/app_runtime_work_send_contract.rs | cat -n

echo '--- search for env_lock usage ---'
rg -n "env_lock\(|StdMutex|OnceLock" executor/tests/app_runtime_work_send_contract.rs executor/tests -g '!**/target/**' || true

Repository: wecode-ai/Wegent

Length of output: 32066


Recover from a poisoned test lock.

A panic while this guard is held poisons the mutex, so later tests fail at .expect(...) even though the lock is only for serialization.

Proposed fix
         _guard: LOCK
             .get_or_init(|| StdMutex::new(()))
             .lock()
-            .expect("environment lock should be available"),
+            .unwrap_or_else(|poisoned| poisoned.into_inner()),
🤖 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/tests/app_runtime_work_send_contract.rs` at line 8, Update the test
lock initialization and acquisition around StdMutex and StdMutexGuard so a
poisoned mutex is recovered instead of causing a later .expect(...) failure.
Preserve the lock’s serialization behavior by accepting the guard from the
poison error and returning a usable StdMutexGuard to callers.

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