Skip to content

test(runtime): tolerate transient Dream lock contention#732

Merged
yacosta738 merged 3 commits into
mainfrom
fix/dream-transient-lock-test
May 1, 2026
Merged

test(runtime): tolerate transient Dream lock contention#732
yacosta738 merged 3 commits into
mainfrom
fix/dream-transient-lock-test

Conversation

@yacosta738
Copy link
Copy Markdown
Contributor

Summary

  • Fixes intermittent CI failure in memory::dream::tests::accepts_recorded_completed_session_and_creates_deterministic_artifact_ref
  • Root cause: test expected sessions_processed = 1 but got 0 when run_if_triggered() returned a Busy lock state on first attempt
  • Solution: added retry helper run_if_triggered_after_transient_busy() that waits for lock acquisition, matching pattern already used in session_trigger_fires_after_five_sessions test

Changes

  • Added run_if_triggered_after_transient_busy() test helper
  • Updated failing test to retry on transient lock contention
  • Added explicit DreamLockState::Acquired assertion before validating sessions_processed

Testing

  • Local verification: 20 consecutive runs passed
  • Serial test execution: all 12 dream tests pass
  • Pre-push Rust checks: passed

Fixes intermittent failure observed in https://github.com/dallay/corvus/actions/runs/25174664361/job/73803405198

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 1, 2026

Warning

Rate limit exceeded

@yacosta738 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 39 minutes and 58 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4ccebcce-1dfa-478d-bf80-05644402ae1f

📥 Commits

Reviewing files that changed from the base of the PR and between d302bde and 5a89354.

📒 Files selected for processing (1)
  • clients/agent-runtime/src/memory/dream.rs
📝 Walkthrough

Walkthrough

Test improvements to reduce flakiness in dream consolidation testing by adding a retry helper that waits for non-busy lock states and updates an existing test to verify the lock is properly acquired after consolidation.

Changes

Cohort / File(s) Summary
Dream Consolidation Tests
clients/agent-runtime/src/memory/dream.rs
Added retry helper (up to 10 attempts with sleeps) to handle temporary DreamLockState::Busy states during tests; updated existing test to use helper and assert final lock state is Acquired.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

area:rust

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title follows Conventional Commits style with 'test' prefix, is clearly related to the main change (addressing transient lock contention in Dream tests), and is well under 72 characters.
Description check ✅ Passed Description covers all key sections: summary with root cause, specific changes made, comprehensive testing performed, and reference to the related CI failure.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/dream-transient-lock-test

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
Review rate limit: 0/1 reviews remaining, refill in 39 minutes and 58 seconds.

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

@github-actions github-actions Bot added the size/s Denotes a small change size label May 1, 2026
@yacosta738 yacosta738 self-assigned this May 1, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

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 the current code and only fix it if needed.

Inline comments:
In `@clients/agent-runtime/src/memory/dream.rs`:
- Around line 637-649: Replace the duplicated inline retry loop in the test
function session_trigger_fires_after_five_sessions with a call to the existing
helper run_if_triggered_after_transient_busy(workspace_dir) so both places share
the same transient-busy retry behavior; locate the inline loop in
session_trigger_fires_after_five_sessions (the 10-iteration sleep-and-check
block) and remove it, invoking
run_if_triggered_after_transient_busy(workspace_dir) and using its returned
MemoryConsolidationReport instead of manually handling reports and Busy lock
state.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2248f2ee-0295-434a-90e1-cb666694b7cc

📥 Commits

Reviewing files that changed from the base of the PR and between 71c49c0 and d302bde.

📒 Files selected for processing (1)
  • clients/agent-runtime/src/memory/dream.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: pr-checks
  • GitHub Check: sonar
  • GitHub Check: semgrep-cloud-platform/scan
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (4)
clients/agent-runtime/src/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency

Files:

  • clients/agent-runtime/src/memory/dream.rs
clients/agent-runtime/**/*.rs

📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)

Run cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings, and cargo test for code validation, or document which checks were skipped and why

Files:

  • clients/agent-runtime/src/memory/dream.rs
**/*.rs

⚙️ CodeRabbit configuration file

**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.

Files:

  • clients/agent-runtime/src/memory/dream.rs
**/*

⚙️ CodeRabbit configuration file

**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.

Files:

  • clients/agent-runtime/src/memory/dream.rs
🧠 Learnings (1)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why

Applied to files:

  • clients/agent-runtime/src/memory/dream.rs
🔇 Additional comments (1)
clients/agent-runtime/src/memory/dream.rs (1)

669-672: Good assertion ordering to guard transient lock contention.

Asserting DreamLockState::Acquired before checking sessions_processed is the right guard for this flaky path.

Comment thread clients/agent-runtime/src/memory/dream.rs
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 1, 2026

Deploying corvus with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5a89354
Status: ✅  Deploy successful!
Preview URL: https://3d725b55.corvus-42x.pages.dev
Branch Preview URL: https://fix-dream-transient-lock-tes.corvus-42x.pages.dev

View logs

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 1, 2026

@yacosta738 yacosta738 merged commit e61b994 into main May 1, 2026
17 checks passed
@yacosta738 yacosta738 deleted the fix/dream-transient-lock-test branch May 1, 2026 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:rust size/s Denotes a small change size

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant