Document OpenHuman portfolio readiness cleanup#1661
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR refactors app component state management by centralizing mode-switching callbacks, improves Socket.IO handler type safety, simplifies Conversations label tab derivation, removes lint suppressions, updates build ignore patterns, and documents portfolio readiness with validation evidence and cleanup timeline. ChangesApp State Management & Type Safety Refactoring
Infrastructure & Portfolio Readiness Documentation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
…plan # Conflicts: # app/src-tauri/Cargo.lock
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/superpowers/plans/2026-05-11-operator-mvp-plan.md (1)
324-401: ⚖️ Poor tradeoffConsider goal pack coordination strategy.
The MVP goal packs (Daily Brief, Reply Drafts, Meeting Prep, Engineering Review) each have clear outcomes and approval requirements, but the document doesn't address how conflicts are resolved when multiple goal packs target the same work item (e.g., Daily Brief wants to summarize a thread while Reply Drafts wants to draft a reply for it).
This might not be critical for initial MVP if goal packs are triggered manually and run sequentially, but consider documenting:
- Whether goal packs can run concurrently
- How proposals from different goal packs are prioritized or merged
- Whether work items are locked during proposal generation
This becomes relevant once scheduled triggers (line 157) are active and multiple goal packs observe the same sources.
🤖 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 `@docs/superpowers/plans/2026-05-11-operator-mvp-plan.md` around lines 324 - 401, Add a short "Goal Pack Coordination" section to the MVP Goal Packs that specifies how conflicts are resolved when multiple packs target the same work item: define whether goal packs (Daily Brief, Reply Drafts, Meeting Prep, Engineering Review) may run concurrently or only sequentially, describe a prioritization/merging rule for competing proposals (e.g., priority order or merge strategy), and state whether and how work items are locked during proposal generation and approval; ensure the section also notes behavior change when scheduled triggers are enabled and provides concrete examples of conflict resolution and required approvals.
🤖 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 `@docs/superpowers/plans/2026-05-11-operator-mvp-plan.md`:
- Around line 508-529: Gate 3 lacks failure/recovery behavior for approved
actions; update the approval state machine and related functions so approved
actions can transition to executed or failed and support retry/cancel flows:
extend the approval record schema to include states (pending, approved,
executed, failed) and failure metadata, modify
create_approval_request/approve_request to set and persist the new state field,
change execute_approved_action to detect transient vs permanent errors, record
partial-success semantics (e.g., if evidence recording fails) in record_evidence
by attaching failure reason and affected substeps, and add retry and cancel
handlers that validate state transitions; finally, add tests in ops_tests.rs to
cover approved->failed, retry paths, cancellation, and partial-success scenarios
referenced by preview_action, create_approval_request, approve_request,
execute_approved_action, and record_evidence.
- Around line 217-266: Add a concrete credential storage and security model to
the Connector Strategy: update the doc to state where connector credentials
referenced by operator_connector_accounts are stored (local keychain/agent vs
encrypted DB in control plane), encryption-at-rest and access controls, OAuth
token refresh flow and refresh-token handling, credential rotation/rotation
policy and revocation procedures, and whether control plane stores tokens or
only opaque references; explicitly reconcile this with the "raw private data
stays local" statement and the Composio execution wrapper/Gate 5 approval flow
so reviewers can verify how tokens are used, scoped, and audited during GoalPack
-> Proposal -> Preflight -> Approval -> Composio execute -> Verify -> Evidence.
- Around line 184-200: Add explicit fallback behavior for the inbox adapter:
define clear error states and retry/backoff semantics when the local inbox
server at 127.0.0.1:9849 is unreachable, specify which adapter functions
(list_actionable_threads, list_waiting_on_me, list_waiting_on_others,
list_calendar_context, draft_reply_for_thread, preflight_write,
explain_routing_decision) return empty results vs. errors, and document a
degraded mode that allows operator goal packs to function with reduced work
discovery (e.g., marking threads as unknown/missing and queuing background
retries). Also include user-visible messaging about the dependency (UI/CLI
warnings) and update the Gate 2 and Gate 6 acceptance criteria to require
handling/unambiguous signaling of this degraded state.
---
Nitpick comments:
In `@docs/superpowers/plans/2026-05-11-operator-mvp-plan.md`:
- Around line 324-401: Add a short "Goal Pack Coordination" section to the MVP
Goal Packs that specifies how conflicts are resolved when multiple packs target
the same work item: define whether goal packs (Daily Brief, Reply Drafts,
Meeting Prep, Engineering Review) may run concurrently or only sequentially,
describe a prioritization/merging rule for competing proposals (e.g., priority
order or merge strategy), and state whether and how work items are locked during
proposal generation and approval; ensure the section also notes behavior change
when scheduled triggers are enabled and provides concrete examples of conflict
resolution and required approvals.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4dc963a-b03d-47c4-919d-1876e1cfbb84
⛔ Files ignored due to path filters (1)
app/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.gitignoreCODEX_WORKPAD.mdapp/src/components/settings/panels/RecoveryPhrasePanel.tsxapp/src/lib/mcp/transport.tsapp/src/pages/Conversations.tsxapp/src/pages/Mnemonic.tsxapp/src/services/meetCallService.tsdocs/PORTFOLIO_READINESS.mddocs/superpowers/plans/2026-05-11-operator-mvp-plan.md
💤 Files with no reviewable changes (2)
- app/src/services/meetCallService.ts
- app/src/pages/Conversations.tsx
| OpenHuman calls a local inbox server when present: | ||
|
|
||
| ```text | ||
| OpenHuman operator domain -> inbox_adapter.rs -> http://127.0.0.1:9849 | ||
| ``` | ||
|
|
||
| Adapter functions: | ||
|
|
||
| - `list_actionable_threads` | ||
| - `list_waiting_on_me` | ||
| - `list_waiting_on_others` | ||
| - `list_calendar_context` | ||
| - `draft_reply_for_thread` | ||
| - `preflight_write` | ||
| - `explain_routing_decision` | ||
|
|
||
| This gives immediate reuse while keeping OpenHuman stable. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Specify fallback behavior when inbox server is unavailable.
The Phase 1 adapter assumes the local inbox server is reachable at 127.0.0.1:9849, but the document doesn't define fallback behavior when the server is down or not installed. Gate 6 (line 621) mentions "degraded state" for the inbox adapter, but that pattern should be established here in Phase 1 design.
Consider adding:
- Clear error states when adapter is unreachable
- Whether operator goal packs can function with degraded work discovery
- User-visible messaging about inbox server dependency
- Whether Gate 2 completion requires handling this scenario
🤖 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 `@docs/superpowers/plans/2026-05-11-operator-mvp-plan.md` around lines 184 -
200, Add explicit fallback behavior for the inbox adapter: define clear error
states and retry/backoff semantics when the local inbox server at 127.0.0.1:9849
is unreachable, specify which adapter functions (list_actionable_threads,
list_waiting_on_me, list_waiting_on_others, list_calendar_context,
draft_reply_for_thread, preflight_write, explain_routing_decision) return empty
results vs. errors, and document a degraded mode that allows operator goal packs
to function with reduced work discovery (e.g., marking threads as
unknown/missing and queuing background retries). Also include user-visible
messaging about the dependency (UI/CLI warnings) and update the Gate 2 and Gate
6 acceptance criteria to require handling/unambiguous signaling of this degraded
state.
|
|
||
| ## Connector Strategy | ||
|
|
||
| ### Bootstrap | ||
|
|
||
| Use Composio for breadth and speed: | ||
|
|
||
| - OAuth handoff | ||
| - long-tail connector discovery | ||
| - initial Gmail/GitHub/Linear/Slack/Notion-style coverage | ||
| - trigger discovery where useful | ||
|
|
||
| But do not expose generic Composio execution as the product API. Wrap it in operator policy: | ||
|
|
||
| ```text | ||
| GoalPack -> Proposal -> Preflight -> Approval -> Composio execute -> Verify -> Evidence | ||
| ``` | ||
|
|
||
| ### Core Connectors | ||
|
|
||
| Move high-value connectors to first-class/direct support when they become central to the product: | ||
|
|
||
| - Gmail | ||
| - Google Calendar | ||
| - GitHub | ||
| - Linear | ||
| - Google Drive/Docs | ||
| - Slack | ||
| - iMessage/local Messages bridge for power users | ||
|
|
||
| ### Source-Of-Truth Rule | ||
|
|
||
| Routing belongs in code. The model should not guess which account to use. | ||
|
|
||
| Examples: | ||
|
|
||
| - Replies route through the owning account/thread unless explicitly overridden. | ||
| - New Google writes use the configured default account. | ||
| - GitHub PR comments route to the repo/owner from the original item. | ||
| - Linear updates route to the issue/team from the original work item. | ||
|
|
||
| Each write preview must show: | ||
|
|
||
| - connector | ||
| - account | ||
| - target object | ||
| - action type | ||
| - irreversible risk | ||
| - evidence expected after execution | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Define connector credential storage and security model.
The connector strategy describes OAuth handoff and account usage but doesn't specify how connector credentials (OAuth tokens, API keys) are stored, encrypted, or rotated. The data model includes operator_connector_accounts (line 134), but security requirements aren't detailed.
Consider documenting:
- Credential storage (local keychain, encrypted DB, control plane?)
- OAuth token refresh strategy
- Credential rotation policy
- How this aligns with "raw private data stays local" (line 317)
- Whether control plane stores connector tokens or just references
This is a security consideration that should be resolved before Gate 5 (Composio execution wrapper).
🤖 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 `@docs/superpowers/plans/2026-05-11-operator-mvp-plan.md` around lines 217 -
266, Add a concrete credential storage and security model to the Connector
Strategy: update the doc to state where connector credentials referenced by
operator_connector_accounts are stored (local keychain/agent vs encrypted DB in
control plane), encryption-at-rest and access controls, OAuth token refresh flow
and refresh-token handling, credential rotation/rotation policy and revocation
procedures, and whether control plane stores tokens or only opaque references;
explicitly reconcile this with the "raw private data stays local" statement and
the Composio execution wrapper/Gate 5 approval flow so reviewers can verify how
tokens are used, scoped, and audited during GoalPack -> Proposal -> Preflight ->
Approval -> Composio execute -> Verify -> Evidence.
| ### Gate 3 - Approval And Evidence Loop | ||
|
|
||
| Files: | ||
|
|
||
| - `src/openhuman/operator/preflight.rs` | ||
| - `src/openhuman/operator/evidence.rs` | ||
| - `src/openhuman/operator/ops.rs` | ||
| - `src/openhuman/operator/ops_tests.rs` | ||
|
|
||
| Tasks: | ||
|
|
||
| - [ ] `preview_action` returns account, target, risk, and expected evidence. | ||
| - [ ] `create_approval_request` stores a pending approval. | ||
| - [ ] `approve_request` transitions pending -> approved. | ||
| - [ ] `execute_approved_action` refuses unapproved writes. | ||
| - [ ] `record_evidence` links evidence to run/proposal/work item. | ||
|
|
||
| Validation: | ||
|
|
||
| ```bash | ||
| cargo test operator:: --manifest-path app/src-tauri/Cargo.toml | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Define error recovery strategy for failed approved actions.
Gate 3 describes the approval/execution loop but doesn't specify what happens when an approved action fails during execution:
- Does the approval request stay "approved" or reset to a "failed" state?
- How does the user retry or cancel a failed action?
- What if execution partially succeeds (e.g., sends email but fails to record evidence)?
- How are transient vs permanent failures distinguished?
The "Always Visible" policy (lines 430-438) mentions "how to undo or recover when possible," but Gate 3 tasks don't include failure handling in the state machine. Consider adding to the approval request state machine: pending → approved → executed/failed, and defining retry/cancellation UX before Gate 3 completion.
🤖 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 `@docs/superpowers/plans/2026-05-11-operator-mvp-plan.md` around lines 508 -
529, Gate 3 lacks failure/recovery behavior for approved actions; update the
approval state machine and related functions so approved actions can transition
to executed or failed and support retry/cancel flows: extend the approval record
schema to include states (pending, approved, executed, failed) and failure
metadata, modify create_approval_request/approve_request to set and persist the
new state field, change execute_approved_action to detect transient vs permanent
errors, record partial-success semantics (e.g., if evidence recording fails) in
record_evidence by attaching failure reason and affected substeps, and add retry
and cancel handlers that validate state transitions; finally, add tests in
ops_tests.rs to cover approved->failed, retry paths, cancellation, and
partial-success scenarios referenced by preview_action, create_approval_request,
approve_request, execute_approved_action, and record_evidence.
|
Validation evidence for portfolio review gate:
|
Summary
Validation
Review notes
Summary by CodeRabbit
Refactor
Chores