test(hive): make the full suite pass under evaos-teams-managed - #104
Conversation
Nine tests passed under default features and failed once the Electric adapter was compiled in, invisible because no CI job enabled the feature. Two distinct causes, both test-side; no production behaviour changes. Six tests failed with 'Hive access is not authorized; complete Electric Sheep sign-in first'. The managed build is right to refuse signing without an authorized session - the fixtures simply predate that gate and are exercising signing, not the gate. They now authorize the session through the existing app_state::tests helper rather than weakening any assertion. Three app_state tests asserted the unmanaged outcome of an identity.key that disagrees with the keyring. Managed deliberately keeps the Keychain authoritative and removes the stale file, because the canonical identity is server-selected. Both outcomes are now pinned via a KEYRING_AUTHORITATIVE constant instead of scattered cfg pairs, which reads better and keeps the file inside its size ratchet. The store-failure case deserves a note: managed removes identity.key even when the fake store rejects writes. That is correct rather than a defect - managed attempts no adoption write at all, so a failing store cannot strand data. With all nine green, desktop-tauri-managed-check drops its evaos_teams filter and runs the full suite under the feature, which was the point of the issue. Closes #93
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📜 Recent review details⏰ Context from checks skipped due to timeout. (13)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2026-07-30T11:25:18.286ZApplied to files:
🔇 Additional comments (7)
📝 WalkthroughPriority Level: P4/NIT No actionable P0–P3 findings identified. The changes update test setup and managed-feature coverage only. They authorize managed signing sessions before signing tests and align Confidence: 92% WalkthroughThe managed Tauri check now runs the full feature-enabled test suite. Identity recovery tests distinguish managed and unmanaged authority. Signing-related fixtures authorize managed signing before key and relay operations. Confidence: 99%. ChangesManaged Tauri test coverage
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #104 - test(hive): make the full suite pass under evaos-teams-managed
Head: b6f286e0b2aa64bfe14f3c92c1bda18ec051bdb6 into main. Review event: COMMENT.
Provider: Codex CLI (existing OAuth session) (codex-cli-oauth, codex-cli, model gpt-5.6-luna).
Estimated review effort: 2/5 (~34 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
Justfile |
modified | +2/-6 | Changed file | Low |
desktop/src-tauri/src/app_state.rs |
modified | +1/-1 | Changed file | Low |
desktop/src-tauri/src/app_state_tests.rs |
modified | +30/-30 | Changed file | Low |
desktop/src-tauri/src/commands/channels_tests.rs |
modified | +2/-0 | Changed file | Low |
desktop/src-tauri/src/commands/profile.rs |
modified | +2/-0 | Changed file | Low |
desktop/src-tauri/src/managed_agents/persona_events/tests.rs |
modified | +4/-0 | Test coverage | Low |
desktop/src-tauri/src/relay_admission.rs |
modified | +4/-0 | Changed file | Low |
Review Signal
No validated inline findings.
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
No finding categories.
Validation and Proof
No required validation recommendation selected; rely on existing GitHub checks and human review.
Proof status: not_applicable - No required behavior proof selected for this changed surface.
Profile validation hints: Prefer correctness, security, data-loss, release, and regression findings over style-only feedback.
Profile proof expectations: Look for focused validation, rollback notes, and evidence appropriate to the changed surface.
Related Context
Related issues/PRs: #93, #91, #92, #96.
Suggested labels: tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Review status comment (sticky_status)
- Path instructions: none
- Label suggestions: none
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
evaOS review status: completedPR: #104 - test(hive): make the full suite pass under evaos-teams-managed evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #104 Review URL: #104 (review) |
|
Program sign-off for merge (cross-model review satisfied). Author side: Codex implementation. Review side (Claude, Lane E — documented in the lane record, restated here so the review lives on the PR per hub convention): (1) the 6 signing-gate failures were fixed with an authorized fixture rather than weakening the assertion — the gate itself stays strict; (2) the 3 keyring failures pin both outcomes via |
1 participant
Closes #93. Test-only — no production behaviour changes. With this,
desktop-tauri-managed-checkruns the full desktop suite under the feature instead of just the adapter module, which was the point of the issue.Before / after
cargo test(default)cargo test --features evaos-teams-managedGroup A — six fixtures never established a managed session
They failed with
"Hive access is not authorized; complete Electric Sheep sign-in first"where a signature was expected. That comes fromrequire_managed_authorization(evaos_teams/authorization.rs:104) viaAppState::signing_keys(app_state.rs:305).The managed build is right to refuse signing without an authorized session. These tests are exercising signing behaviour, not the gate — they simply predate it. So they now authorize the fixture; no assertion was weakened and the gate is not bypassed. It reuses the
authorize_managed_signing_testhelper that already existed inapp_state::tests(madepub(crate)) rather than inventing a new one.Group B — three
app_statetests encoded an intended behaviour differenceAn
identity.keythat disagrees with the keyring resolves differently by design (app_state.rs:501):identity.keyholds a later import that only reached the file — the file is the user's explicit intent.Both outcomes are now pinned. I used a single
KEYRING_AUTHORITATIVEconstant plus a smallexpected_keyhelper rather than scattering#[cfg]pairs through the assertions — it reads better and, not incidentally, keepsapp_state_tests.rsinside its size ratchet (that file is grandfathered at 1418 lines and may not grow; it lands at 1417).One case worth a reviewer's eye. In
present_keyring_mismatched_file_adoption_store_failure_boots_with_file_key, managed removesidentity.keyeven though the fake store rejects writes. I checked the production path rather than assuming: managed takes theensure_marker_then_cleanupbranch and attempts no adoption write at all, so a failing store cannot strand data. Correct, not a defect.Review note on process
Codex authored the mechanical change; I reviewed it and made two corrections rather than passing it through:
Both configurations, verified locally on this branch:
cargo fmt --check·just desktop-tauri-clippy·just desktop-tauri-managed-check(full suite, 1940/0) ·cargo test(1910/0) · file-size ratchet — all pass.Related: #92 (canary now builds the adapter), #96 (a ratchet failure skips every Rust step — the reason none of this was visible).