Skip to content

test(memory): share workspace env lock#2582

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
senamakel:fix/ship-and-babysit-autonomy
May 24, 2026
Merged

test(memory): share workspace env lock#2582
senamakel merged 2 commits into
tinyhumansai:mainfrom
senamakel:fix/ship-and-babysit-autonomy

Conversation

@senamakel
Copy link
Copy Markdown
Member

Summary

  • switch memory file RPC tests from a module-local mutex to the shared config test env lock
  • align these tests with the repo-wide convention for serializing OPENHUMAN_WORKSPACE mutations
  • prevent the memory file roundtrip test from racing unrelated suite tests that repoint the workspace env var

Problem

  • the previous PR on this branch merged while this babysit session was still active
  • after that merge, the branch gained a follow-up commit fixing a real CI failure in test / Rust Core Tests + Quality
  • the failing Rust test mutated OPENHUMAN_WORKSPACE under a module-local lock, which did not serialize against the rest of the suite’s shared env-var users

Solution

  • replace the local env_mutex() helper in src/openhuman/memory/ops/files.rs tests with crate::openhuman::config::TEST_ENV_LOCK
  • keep the existing per-test workspace guard for restore semantics, but use the shared lock for cross-module isolation
  • preserve runtime code paths; this change is test-only and targets the race that produced an empty memory listing in CI

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
    • Existing tests updated to use shared env-var isolation
  • Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/coverage.yml. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
    • Covered by existing Rust test module exercising the modified test harness
  • Coverage matrix updated — added/removed/renamed feature rows in docs/TEST-COVERAGE-MATRIX.md reflect this change (or N/A: behaviour-only change)
    • N/A: no feature matrix row changes for test-only isolation fix
  • All affected feature IDs from the matrix are listed in the PR description under ## Related
    • N/A: no matrix rows affected
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md)
    • N/A: no release-cut surface changed
  • Linked issue closed via Closes #NNN in the ## Related section
    • N/A: no linked issue for this follow-up test fix

Impact

  • test-only change in the Rust core crate
  • no runtime, product, migration, or compatibility impact
  • reduces flaky suite behavior by preventing cross-module env-var races

Related

  • Closes:
  • Follow-up PR(s)/TODOs:

AI Authored PR Metadata (required for Codex/Linear PRs)

Keep this section for AI-authored PRs. For human-only PRs, mark each field N/A.

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/ship-and-babysit-autonomy
  • Commit SHA: fd9e40499

Validation Run

  • pnpm --filter openhuman-app format:check
  • pnpm typecheck
  • Focused tests: attempted targeted cargo test for the touched memory file test module; local compile was lengthy, but the fix directly targets the CI-reproduced failure surface and the push reruns full CI
  • Rust fmt/check (if changed): pre-push hook ran repo rust fmt/check and Tauri rust check successfully
  • Tauri fmt/check (if changed): N/A because no Tauri code changed

Validation Blocked

  • command: cargo test --manifest-path Cargo.toml openhuman::memory::ops::files::tests:: -- --nocapture
  • error: local targeted run spent its time recompiling the large Rust crate and was interrupted so the CI fix could be pushed promptly
  • impact: relied on pre-push validation plus fresh CI rerun for final confirmation

Behavior Changes

  • Intended behavior change: Rust memory file tests now serialize OPENHUMAN_WORKSPACE mutations with the crate-wide shared env lock
  • User-visible effect: none at runtime; reduces CI flakiness in the Rust test suite

Parity Contract

  • Legacy behavior preserved: runtime memory file behavior is unchanged; only test synchronization changed
  • Guard/fallback/dispatch parity checks: N/A

Duplicate / Superseded PR Handling

  • Duplicate PR(s): merged predecessor #2581 covered the earlier skill-doc change on the same branch
  • Canonical PR: this PR
  • Resolution (closed/superseded/updated): follow-up PR after prior PR merged mid-babysit session

@senamakel senamakel requested a review from a team May 24, 2026 20:50
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 24, 2026

Warning

Review limit reached

@senamakel, we couldn't start this review because you've used your available PR reviews for now.

Your plan includes 5 reviews of capacity. Refill in 54 minutes and 6 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, 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 trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b5dcf0ad-1376-4743-b6fe-e8938d37f2cb

📥 Commits

Reviewing files that changed from the base of the PR and between 61cfabb and fd9e404.

📒 Files selected for processing (2)
  • .codex/skills/ship-and-babysit/SKILL.md
  • src/openhuman/memory/ops/files.rs

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

@senamakel senamakel merged commit a222808 into tinyhumansai:main May 24, 2026
28 of 30 checks passed
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