Repository navigation
feat(life_capture): Composio → life_capture bridge for Gmail + Calendar (A2) - #820
jwalin-shah wants to merge 41 commits into
Conversation
|
Warning Rate limit exceeded
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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces a foundational "life capture" system for local personal data indexing via SQLite and sqlite-vec, implements a curated-memory snapshot mechanism for stable agent prompts, refactors the iMessage scanner to use the new ingest pathway, updates installation/release infrastructure, and extends the agent session harness to thread curated snapshots through prompt rendering and sub-agent execution. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
692136c to
6dd0263
Compare
Drafts the wedge for OpenHuman: continuous ingestion of Gmail/Calendar/Slack/ iMessage into a local personal_index.db, with a Today brief and retrieval- tuned chat as the two surfaces. Three composable privacy modes (Convenience / Hybrid / Fully local) sharing one code path. Track 1 unblocks the ship pipeline; Track 2 builds the spine over 4 weeks. Reuses ingestion logic from ~/projects/inbox.
Two implementation plans drafted from the life-capture spec: - Track 1 (ship pipeline): fix Ubuntu installer smoke, land in-flight PRs tinyhumansai#806/tinyhumansai#786/tinyhumansai#788/tinyhumansai#797, wire Tauri auto-updater + signed Mac/Windows builds. - Life-Capture #1 (foundation): SQLite + sqlite-vec personal_index.db, Embedder trait + HostedEmbedder (OpenAI), PII redaction, quoted-thread stripping, hybrid retrieval (vector + keyword + recency), controller schema + RPC. End-to-end test with synthetic items. No ingestion or UI yet — those are subsequent milestone plans.
Refactors scripts/install.sh to expose resolve_asset_url and verify_asset_reachable. Adds scripts/test_install.sh that exercises the resolver against a committed fixture latest.json. Failures now report the resolved URL and the parse error instead of dying silently.
…le, keyword projection column
Adopts the Hermes pattern (NousResearch/hermes-agent, MIT) of an
agent-writable, char-bounded curated memory file pair. F15 builds the
char-bounded store with atomic writes; F16 wires a session-start
snapshot into the system prompt (preserves prefix cache); F17 exposes
memory.{add,replace,remove,read} through the controller dispatch so
both the agent loop and skills can use it.
Sits between TinyHumans synthesis (volatile) and personal_index.db
(raw events) — fills the "deliberately curated facts" gap that
neither covered.
Adds src/openhuman/life_capture/ module tree (stubs for embedder, index, migrations, quote_strip, redact, rpc, schemas, types, plus tests/) per Foundation Plan F1. Wires the module in src/openhuman/mod.rs. Adds sqlite-vec to deps and httpmock as a dev-dep. Reuses existing rusqlite/regex/once_cell/async-trait/tempfile already in tree (Option C — avoids sqlx/rusqlite libsqlite3-sys conflict).
…egex
Previously the country-code group required at least one digit (\+?\d{1,3}),
so '(415) 555-0123' (no leading country code) wouldn't match. Wrapped the
whole prefix in a non-capturing optional group so parenthesized area codes
match without a leading country code.
…te, char-bounded)
… table Loads sqlite_vec via sqlite3_auto_extension exactly once per process so every new connection picks up the vec0 module. PersonalIndex wraps a single rusqlite Connection in Arc<Mutex<_>> for async sharing; open() runs migrations and sets WAL+foreign_keys, open_in_memory() is for tests. vec_version() round-trip and a 1536-dim INSERT + MATCH query both verified.
…_id) dedupe Adds IndexWriter with upsert (ON CONFLICT(source, external_id) DO UPDATE — FTS stays in sync via triggers) and upsert_vector (DELETE + INSERT since vec0 has no ON CONFLICT for virtual table primary keys). Both wrap rusqlite work in spawn_blocking and share the PersonalIndex's Arc<Mutex<Connection>>. Exposes ensure_vec_extension_registered() crate-wide so the migrations test can register vec0 before opening its in-memory connection.
- keyword_search: FTS5 ranked by negated bm25 with snippet markers (« » …), plus the user:local ACL EXISTS clause so the same query shape works for the multi-token team v2 ACL without rewrites. - vector_search: sqlite-vec MATCH with k = ? clause (vec0 KNN requirement) and ORDER BY distance; score = 1/(1 + distance) so callers can blend it with keyword bm25 on the same monotonic scale. Shared ItemRow + into_hit() helper lets both queries reuse the same row shape; rusqlite query_map closures hand-build it because rusqlite has no sqlx FromRow equivalent.
Pulls oversampled candidates (k*3 min 20) from both keyword and vector legs, normalises each independently to [0,1], then re-ranks with 0.55 * vec_norm + 0.35 * kw_norm + 0.10 * exp(-age_days/30). Documents present in only one leg get 0 for the missing signal but still compete on the others; only items with neither signal are dropped. Same-vector twin items now break their tie by recency.
…rch) Adds two controllers exposed via the registry: - life_capture.get_stats — total items, per-source counts, last-ingest ts. - life_capture.search — hybrid (vector + keyword + recency) search with embed-then-rank, optional k (default 10, capped at 100). Runtime state (PersonalIndex + dyn Embedder) lives in life_capture::runtime behind a tokio OnceCell because handlers are stateless fn(Map<String,Value>) and have no per-call context object. F14 will call runtime::init() at app startup; until then handlers return a structured 'not initialised' error so the failure mode is loud, not silent. Schemas registered in core/all.rs alongside cron.
… → search) FakeEmbedder hashes input bytes into a deterministic sparse 1536-dim vector so the same text round-trips identically through vec0 — keeps the e2e test hermetic, no network call. Verifies that: - quote-strip drops the 'On … wrote:' block before indexing, - redact masks the email address before indexing, - the upserted item resurfaces via hybrid_search with both transformations preserved on the returned text.
After Config::load_or_init succeeds, open the personal_index.db at config.workspace_dir and register the life_capture runtime. Embedder is env-gated: OPENHUMAN_EMBEDDINGS_KEY > OPENAI_API_KEY OPENHUMAN_EMBEDDINGS_URL (default: https://api.openai.com/v1) OPENHUMAN_EMBEDDINGS_MODEL (default: text-embedding-3-small) If no key is set we still open the index file (so Plan #2's ingestion worker can write to it) but skip runtime registration — controllers then return the 'not initialised' error from runtime::get() instead of silently calling a misconfigured embedder. Config-schema integration deferred to a follow-up; env-driven keeps F14 non-invasive while we land the rest of the foundation.
…ection snapshot_pair(memory, user) returns a MemorySnapshot containing plain strings — no reference back to the stores — so taking a snapshot at session start and reusing it across turns gives a stable system-prompt prefix and lets the LLM prefix-cache hit on every subsequent turn. Plan #1 also calls for wiring this into chat.rs' OpenClaw context loader, but openhuman assembles agent prompts per-agent under src/openhuman/agent/agents/*/prompt.rs (not the plan's stale src-tauri/src/commands/chat.rs path); the prompt-builder integration is deferred to Plan #2 alongside the agent-context refactor.
… controllers Adds four controllers under the memory_curated namespace (the 'memory' namespace is already owned by the long-term memory subsystem): - memory_curated.read — read MEMORY.md or USER.md - memory_curated.add — append entry, char-bounded - memory_curated.replace — substring replace, char-bounded - memory_curated.remove — drop entries containing a needle 'file' input is a typed enum (memory|user) so adapters reject anything else at validation time. Runtime state lives in curated_memory::runtime behind a tokio OnceCell, mirroring life_capture's pattern: startup in jsonrpc.rs constructs both stores at <workspace>/memories/ with Hermes' char limits (memory: 2200, user: 1375) and registers the runtime. Handlers return 'not initialised' until init runs, so failure is loud.
…ings Findings from Codex + Gemini second-opinion review: 1. IndexWriter::upsert orphaned vectors on re-ingest (Codex). On (source, external_id) conflict the row's id was kept but the caller's fresh UUID was used for upsert_vector — the vector wrote under an id no row joined to. Fix: explicit SELECT-then-UPDATE-or-INSERT in the same transaction; mutates caller's Item.id to the canonical id (so the next upsert_vector lands on the right row) and orphan-deletes any vector already written under the wrong id. Signature change: upsert(&[Item]) -> upsert(&mut [Item]). 2. upsert_vector DELETE+INSERT was outside a transaction (Gemini) — a failed INSERT permanently lost the item's vector. Now wrapped in tx. 3. Runtime over-gated get_stats on the embedder (Codex). Split runtime into separate INDEX + EMBEDDER OnceCells: get_stats only requires index, search requires both. Index initialises whenever the workspace dir is reachable; embedder is env-gated as before. 4. Startup race (both reviewers): runtime init lived inside the post- serve tokio::spawn block, so axum::serve was already accepting before the OnceCells were set. Hoisted both bootstraps (curated_memory and life_capture) into helpers called inline before axum::serve. 5. runtime::get error message lied (Codex) — said 'set embeddings.api_key in config' but startup actually reads env vars. Fixed text. Bonus: rename controller namespace memory_curated -> curated_memory (Codex preference; nothing depends on it yet so renamed before clients do). Adds regression test for #1 (reingest_with_fresh_uuid_keeps_vector_findable). 4696 lib tests pass.
…emory namespace rename Pre-push hook auto-applied cargo fmt across the foundation files; also fixed the schemas.rs docstring still referencing the old memory_curated namespace.
Critical: - curated_memory/store.rs: propagate read errors instead of `unwrap_or_default`, which could rewrite MEMORY.md / USER.md from an empty baseline on transient I/O or permission failures. Reject empty needle in replace/remove. - curated_memory/rpc.rs: belt-and-suspenders empty-needle guard at the RPC boundary (remove with "" deletes every entry). - life_capture/index.rs: upsert_vector rejects orphan ids (items row missing), which would have inserted vectors that never join in vector_search. - life_capture/embedder.rs: validate response length matches input, indices are contiguous 0..n, and every vector matches dim() — prevents silent misalignment or wrong-dimensional vectors flowing into the 1536-wide sqlite-vec table. Added 30s request + 10s connect timeouts. Retrieval hardening: - life_capture/index.rs: keyword_search now escapes FTS5 operators by tokenizing on whitespace and wrapping each token as a quoted literal. Prevents errors or unintended matches from stray quotes, AND/OR/NEAR, column specifiers in user input. - life_capture/index.rs: hybrid_search applies q.sources / q.since / q.until via post-filter (single consistent pass across keyword+vector fusion). - life_capture/rpc.rs: handle_search validates embedder dim and response length against the fixed 1536-wide index column — clear RPC error instead of a cryptic sqlite-vec failure when the embeddings model is swapped. Docs: - docs/event-bus.md: remove erroneous `.await` on register_native_global (it is sync; the handler closure is async). - life-capture design spec: soften PII redaction claim — only regex is implemented today; light NER is flagged as a future enhancement.
Rendered prompts now surface runtime curated-memory writes: - `PromptContext` gains `curated_snapshot: Option<&MemorySnapshot>`. - `UserFilesSection` prefers the snapshot over the workspace-file loader when one is attached, and injects `USER.md` alongside `MEMORY.md` using a byte-compatible `inject_snapshot_content` helper. - `Session` carries `Option<Arc<MemorySnapshot>>`, populated by `ensure_curated_snapshot` on the first turn from `curated_memory::runtime::get()`. Reused across turns so prompt bytes stay frozen (KV-cache prefix contract) while mid-session `curated_memory.add/replace/remove` writes land on the NEXT session. - `ParentExecutionContext` inherits the snapshot so sub-agents render identical `MEMORY.md`/`USER.md` blocks as the parent. - Legacy workspace-file fallback preserved for embeds that don't initialise the curated-memory runtime (pure unit tests).
Loads a 32-item corpus across gmail/calendar/imessage/slack into an in-memory PersonalIndex and runs 12 queries through keyword / vector / hybrid paths, asserting must_contain / must_not_contain within per-query top-K prefixes. The vector leg uses a deterministic FNV-1a feature-hashing embedder (1536 dims, L2-normalized) so the test is offline and reproducible; real embedder swaps behind one call. Fixture reserves `relevant` and `pending` fields so recall@k / MRR / nDCG bolt onto the same JSON later. q-src-01 is marked pending: hybrid_search currently ignores Query.sources/since/until. Flip to false once filtering lands.
Previously every read grabbed the same `Arc<Mutex<Connection>>` as the writer, so `IndexReader` calls serialised behind in-flight ingests even though WAL would allow parallel readers. Split `PersonalIndex` into: - `writer: Arc<Mutex<Connection>>` — unchanged single-writer model. - `reader_pool: Option<Arc<r2d2::Pool<SqliteConnectionManager>>>` — four-connection read pool on file-backed indexes, built alongside the writer after migrations run. Each pooled connection gets `query_only=ON` as a belt-and-suspenders guard; sqlite-vec is loaded automatically via the process-wide auto-extension. `IndexReader` routes `keyword_search` / `vector_search` through a new `with_read_conn` helper that picks the pool when present and falls back to the writer lock otherwise. In-memory handles keep the single-connection layout — shared-cache URIs buy nothing at test-fixture scale and would force every test to grow a unique name.
- Add `life_capture.ingest` RPC controller (idempotent upsert by
`(source, external_id)`, embeds text + atomic vector replace).
- Switch iMessage scanner's `ingest_group()` to POST
`openhuman.life_capture_ingest` instead of `openhuman.memory_doc_ingest`,
so chat-day transcripts land in the PersonalIndex (the same store
`life_capture.search` reads from).
- Fix `life_capture.search` envelope: wrap hits in `{"hits": [...]}` per
the schema (previously returned a bare array — obs 3165).
- New `#[ignore]`d e2e at `tests/imessage_ingest_e2e.rs` proves the bridge:
ingest → search → re-ingest is idempotent (`total_items` unchanged,
`replaced=true`).
Adds a second subscriber on the `composio` domain that normalises Composio trigger webhooks into `life_capture::types::Item` and pushes them through the existing `rpc::handle_ingest` path — so Gmail and Google Calendar events land in the same PersonalIndex that iMessage transcripts already populate, keyed for idempotent upsert by `(source, external_id)`. Files added: - src/openhuman/life_capture/ingest/mod.rs (light export-only) - src/openhuman/life_capture/ingest/composio.rs (bridge + registrar) - tests/composio_ingest_e2e.rs (#[ignore]'d live-bus e2e) Files modified: - src/openhuman/life_capture/mod.rs (add `pub mod ingest;`) - src/core/jsonrpc.rs (call `register_life_capture_composio_bridge` from the existing `bootstrap_life_capture` once index + embedder are both ready; the existing `ComposioTriggerSubscriber` for triage + history is left completely untouched) Trigger handlers (case-insensitive match): - GMAIL_NEW_GMAIL_MESSAGE / GMAIL_NEW_MESSAGE → ingest_gmail - GOOGLECALENDAR_NEW_EVENT → ingest_calendar - GOOGLECALENDAR_EVENT_UPDATED → ingest_calendar - other → no-op Dedupe key per source: - Gmail: payload `messageId` / `id` (tried in fallback order) - Calendar: payload `id` / `eventId` / `uid` / `iCalUID` Ts normalisation accepts epoch seconds, epoch millis (Gmail `internalDate`), RFC-3339 strings, and bare `YYYY-MM-DD` dates. Test coverage: - 9 unit tests in composio.rs: Gmail + Calendar happy paths, calendar event_updated idempotency on same id, unknown/non-composio triggers are no-ops, normalize_* reject payloads missing id or text, ts parser accepts millis/seconds/RFC-3339/date. - tests/composio_ingest_e2e.rs: publishes a real `DomainEvent::ComposioTriggerReceived` through the global event bus, asserts the bridge wrote an item visible via `openhuman.life_capture_search` with `source=gmail`, ts normalised from millis, and a re-fire of the same messageId does not inflate `total_items`. Verification: - cargo check --manifest-path Cargo.toml ✓ - cargo test --lib life_capture ✓ (36 passed) - cargo test --test composio_ingest_e2e --no-run ✓ Out of scope (per v1 plan): iMessage scanner, chronicle, people/, eventkit/, skills, telegram, slack, notion, onboarding UI.
35206ca to
999231d
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/core/all.rs (1)
252-311:⚠️ Potential issue | 🟡 MinorAdd
namespace_descriptionentries forlife_captureandcurated_memory.
namespace_descriptionpowers the CLI help/--helpoutput for each namespace. The two new namespaces will returnNonehere, leaving their CLI entries un-described while every other namespace registered above has a one-liner. Cheap to add now and avoids drift in the catalogue users see.📝 Proposed additions
"learning" => Some( "User context enrichment — LinkedIn profile scraping and onboarding intelligence.", ), + "life_capture" => Some( + "Local personal index — search and stats over captured life data (Gmail, Calendar, iMessage, …).", + ), + "curated_memory" => Some( + "Agent-writable curated memory (MEMORY.md + USER.md) used for stable prompt snapshots.", + ), "notification" => Some(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/all.rs` around lines 252 - 311, Add two match arms to the namespace_description function to return descriptions for the new namespaces: add "life_capture" => Some("Capture of user life events, media, and context for personal history and recall.") and "curated_memory" => Some("Curated, reviewed memory entries for long-term persistence and summarized retrieval."); place these entries alongside the other namespace string arms in namespace_description so CLI help shows one-line descriptions for both new namespaces.app/src-tauri/src/imessage_scanner/mod.rs (1)
1-6:⚠️ Potential issue | 🟡 MinorStale module docstring — references the old RPC method.
The header comment still claims this scanner emits
openhuman.memory_doc_ingest, but the actual ingest call at line 421 now targetsopenhuman.life_capture_ingest. Update so future readers (and grep) land on the right method.📝 Proposed fix
//! iMessage local-database scanner. //! //! Reads `~/Library/Messages/chat.db` on macOS (read-only) and emits one -//! `openhuman.memory_doc_ingest` JSON-RPC call per `(chat_identifier, day)` +//! `openhuman.life_capture_ingest` JSON-RPC call per `(chat_identifier, day)` //! group — matching the convention codified in //! `docs/webview-integration-playbook.md` and used by the WhatsApp scanner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/imessage_scanner/mod.rs` around lines 1 - 6, Update the module-level docstring at the top of imessage_scanner::mod.rs to reflect the current RPC method name: replace references to `openhuman.memory_doc_ingest` with `openhuman.life_capture_ingest` so the comment matches the actual call made later in this module (see the ingest call in the code that invokes `openhuman.life_capture_ingest`). Ensure the rest of the descriptive text (about chat.db grouping and the referenced playbook) remains intact.scripts/install.sh (1)
277-304:⚠️ Potential issue | 🔴 CriticalFix the regex patterns — doubled backslashes break macOS asset matching.
Inside the single-quoted heredoc
<<'PY', Python receives the regex patterns with literal doubled backslashes. In a raw string,r"\\.dmg$"becomes the regex "literal backslash + any char + dmg + end", which cannot match real filenames likeOpenHuman_0.1.0_aarch64.dmg.Verification confirms all four patterns fail to match:
r"aarch64.*\\.app\\.tar\\.gz$"→ no matchr"aarch64\\.dmg$"→ no matchr"(x86_64-apple-darwin|x64).*\\.app\\.tar\\.gz$"→ no matchr"x64\\.dmg$"→ no matchThis breaks the GitHub-Release-API fallback silently (masked as "no compatible asset found"). Replace each doubled backslash with a single backslash:
r"aarch64\.dmg$",r"aarch64.*\.app\.tar\.gz$",r"(x86_64-apple-darwin|x64).*\.app\.tar\.gz$",r"x64\.dmg$".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/install.sh` around lines 277 - 304, The regex patterns used to select macOS assets (e.g., r"aarch64.*\\.app\\.tar\\.gz$", r"aarch64\\.dmg$", r"(x86_64-apple-darwin|x64).*\\.app\\.tar\\.gz$", r"x64\\.dmg$") are written with doubled backslashes inside the single-quoted heredoc (<<'PY'), causing them to look for literal backslashes and fail to match filenames; update those patterns to use single backslashes (e.g., r"aarch64\.dmg$", r"aarch64.*\.app\.tar\.gz$", r"(x86_64-apple-darwin|x64).*\.app\.tar\.gz$", r"x64\.dmg$") so the loops that iterate over names and set chosen (and the subsequent assets loop that compares asset.get("name") to chosen) correctly find macOS assets.
🧹 Nitpick comments (18)
src/openhuman/agent/agents/integrations_agent/prompt.rs (1)
171-171: LGTM — required compile-fix for the updatedPromptContextshape.Side note: with seven agent prompt-test files now manually spelling out every
PromptContextfield, the duplication is approaching the point where aPromptContext::test_default()builder (or a test-onlyDefaultimpl behind#[cfg(test)]) would make future field additions a one-line change instead of seven. Not blocking — flagging for a future cleanup pass.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/agent/agents/integrations_agent/prompt.rs` at line 171, The compile error is due to the updated PromptContext shape missing the curated_snapshot field; update the construction of PromptContext in agents/integrations_agent/prompt.rs to include curated_snapshot: None (matching the new struct field) so the struct literal compiles, and consider adding a PromptContext::test_default() or a test-only Default impl for PromptContext to reduce duplication across the seven agent prompt-test files in a future cleanup.src/openhuman/tools/ops.rs (1)
122-126: Add a regression-guard test forlife_capture_searchregistration.The new tool is unconditionally registered (matching the comment that surfaces a structured error at call time if
life_capture::runtimeisn't initialised yet), but there's no test pinning that contract. Every other always-on tool added to this list (spawn_subagent,complete_onboarding,current_time,curl, …) ships with a matchingassert!(names.contains(&"…"))regression guard; without one here, a future refactor that gates registration behind a feature/flag could silently strip the tool from the agent's tool list and slip through CI.♻️ Proposed test (drop next to the other regression guards)
#[test] fn all_tools_always_registers_life_capture_search() { // Regression guard: `life_capture_search` is always registered. // Surfacing a structured error from runtime::get_index/get_embedder // is the intended behavior when life-capture core hasn't initialised // — the tool itself must still appear in the registry. let tmp = TempDir::new().unwrap(); let security = Arc::new(SecurityPolicy::default()); let mem_cfg = MemoryConfig { backend: "markdown".into(), ..MemoryConfig::default() }; let mem: Arc<dyn Memory> = Arc::from(crate::openhuman::memory::create_memory(&mem_cfg, tmp.path()).unwrap()); let browser = BrowserConfig::default(); let http = crate::openhuman::config::HttpRequestConfig::default(); let cfg = test_config(&tmp); let tools = all_tools( Arc::new(Config::default()), &security, mem, &browser, &http, tmp.path(), &HashMap::new(), &cfg, ); let names: Vec<&str> = tools.iter().map(|t| t.name()).collect(); assert!( names.contains(&"life_capture_search"), "life_capture_search must always be registered; got: {names:?}" ); }As per coding guidelines: "Rust code must be unit-tested for new/changed behavior before stacking features".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/tools/ops.rs` around lines 122 - 126, Add a regression-guard unit test that ensures life_capture_search is always registered by calling all_tools and asserting the returned tool names contain "life_capture_search"; mirror the existing pattern used for other always-on tools (create a TempDir, SecurityPolicy, Memory via create_memory, BrowserConfig, HttpRequestConfig, test_config, call all_tools, collect names with .iter().map(|t| t.name()), and assert!(names.contains(&"life_capture_search"))). Place this test alongside the other registration guards (next to tests that assert presence of spawn_subagent/complete_onboarding/current_time/etc.) so future refactors that accidentally gate or remove LifeCaptureSearchTool::new() will fail CI.src/openhuman/life_capture/redact.rs (1)
8-17: Optional: anchorPHONEwith word boundaries to reduce over-matching in opaque IDs.
PHONEhas no\bat start/end, so digit substrings inside identifiers (tracking numbers, hex-adjacent IDs, etc.) can be partially rewritten as<PHONE>. The other three regexes use\band don't have this property. Since this redaction runs upstream of embedding + FTS indexing, an over-match merely costs you recall on those tokens — not a correctness issue — but tightening it is cheap:♻️ Suggested boundary tightening
static PHONE: Lazy<Regex> = Lazy::new(|| { Regex::new( r"(?x) + \b (?:\+?\d{1,3}[\s\-.])? # optional country code, e.g. '+1-' or '1.' (?:\(\d{2,4}\)|\d{2,4})[\s\-.]? \d{3}[\s\-.]?\d{3,4} + \b ", ) .unwrap() });Worth adding a regression test that a long opaque ID like
"order-1234567890123"is left untouched (currently the CC regex would also catch this — same caveat applies but it's a separate, well-known best-effort tradeoff).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/redact.rs` around lines 8 - 17, The PHONE regex (static PHONE: Lazy<Regex>) lacks word boundaries and can over-match digit substrings inside opaque IDs; update the regex to add \b anchors at the start and end of the pattern (i.e., wrap the whole pattern with \b ... \b or equivalent) so it only matches standalone phone-like tokens, and add a unit test asserting strings like "order-1234567890123" are not redacted (and other positive tests to ensure real phone numbers still redact).src/openhuman/life_capture/migrations/0001_init.sql (1)
9-9: Clarify or wire uptext_keyword.The column is declared with a comment promising a "raw values projection for FTS5/BM25 (Onyx pattern)", but
items_ftsindexessubject, textand all three triggers ignoretext_keyword. Future readers will reasonably assume it's indexed. Either:
- Index it in
items_fts(and update the AI/AD/AU triggers accordingly) so keyword search actually uses the unprocessed projection, or- Drop the column from this migration and re-introduce it in the migration that first uses it, or
- Rephrase the comment to make clear it's a placeholder reserved for a follow-up migration.
Option 3 is the lowest-risk if you want to keep the column shape stable for future readers/writers without touching trigger logic in this PR.
Also applies to: 30-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/migrations/0001_init.sql` at line 9, The column text_keyword is declared but not used by items_fts or the AI/AD/AU triggers, which is misleading; update this migration to either wire it up or make its intent explicit—prefer the low-risk option: keep the column but change its inline comment to state it's a reserved placeholder for a future migration (e.g., "reserved for raw values projection for FTS5/BM25; indexed and wired up in a follow-up migration"), and leave items_fts and the triggers (AI/AD/AU) unchanged so behavior stays stable.src/openhuman/agent/harness/session/types.rs (1)
135-136: Use the in-scopeArcfor consistency.
Arcis already imported at line 19 and used unqualified throughout this file (e.g., line 27Arc<dyn Provider>, line 142Arc<dyn ... PayloadSummarizer>). The new field could drop thestd::sync::prefix to match.♻️ Proposed tweak
- pub(super) curated_snapshot: - Option<std::sync::Arc<crate::openhuman::curated_memory::MemorySnapshot>>, + pub(super) curated_snapshot: Option<Arc<crate::openhuman::curated_memory::MemorySnapshot>>,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/agent/harness/session/types.rs` around lines 135 - 136, The field curated_snapshot is using the fully qualified std::sync::Arc type; change it to use the in-scope Arc to match the rest of the file (e.g., other usages like Arc<dyn Provider> and Arc<dyn PayloadSummarizer>). Locate the declaration of curated_snapshot (type Option<std::sync::Arc<crate::openhuman::curated_memory::MemorySnapshot>>) and replace std::sync::Arc with the unqualified Arc so the field reads Option<Arc<crate::openhuman::curated_memory::MemorySnapshot>>>, keeping the same crate path for MemorySnapshot.src/openhuman/life_capture/quote_strip.rs (1)
4-7: Edge case: wrappedOn … wrote:headers won't match.Gmail/Apple Mail occasionally wrap the citation header across two lines when the sender name + email is long (
On Mon, Apr 21, 2026 at 9:14 AM, Some Long Name\n<long.address@example.com> wrote:). Since.doesn't match newlines, the cut won't fire and the wrapped header + quoted block will leak through (the>-line filter catches the quotes, but the wrapped header stays). Not a release blocker — typical clients keep it on one line — but worth a(?s)variant or a permissive[\s\S]{1,400}if you see this in real Gmail payloads during A2 e2e.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/quote_strip.rs` around lines 4 - 7, The ON_DATE_WROTE regex currently uses .{1,200} which won't match when the "On … wrote:" header is wrapped across lines; update the pattern in the Lazy::new closure (where Regex::new is called for ON_DATE_WROTE) to allow newlines in that span (either add the DOTALL flag via (?s) or replace .{1,200} with a permissive [\s\S]{1,400} or similar) so wrapped headers like "On ...\n<email> wrote:" are matched; keep the word boundary and trailing wrote:\s*$ anchors intact and adjust the max length as needed.app/src-tauri/src/imessage_scanner/mod.rs (1)
413-452: Note:tsis UTC midnight of a local-tz day.
seconds_to_ymd(line 276) renders the day in local timezone, butparse_day_to_unixparses it back as UTC midnight. For non-UTC users this putstsup to ~24h off "actual local midnight" — fine for dedupe (the stable key isexternal_id, notts), but the comment on line 413 reads as iftsis the precise start-of-day in UTC. Either tighten the comment to "stable anchor derived from the local-tz day" or compute the local-tz midnight here so semantic ranking bytsmatches the user's calendar.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/imessage_scanner/mod.rs` around lines 413 - 452, The comment claims ts is "UTC midnight of a local-tz day" but seconds_to_ymd renders day in the local timezone while parse_day_to_unix currently produces UTC midnight, causing up-to ~24h mismatch; either update the comment to say ts is a stable anchor derived from the local-tz day, or change parse_day_to_unix to compute the local-tz midnight for the provided YYYY-MM-DD string and return its UTC unix-seconds instead (use chrono::Local or convert NaiveDate.and_hms_opt(0,0,0) with Local::from_local_datetime and then to_utc timestamp). Update the code in parse_day_to_unix (and keep seconds_to_ymd behavior) so ts semantics match the comment, or leave parse_day_to_unix as-is and tighten the comment to refer to a "stable anchor derived from the local-tz day" mentioning parse_day_to_unix, seconds_to_ymd, and the ts variable.tests/composio_ingest_e2e.rs (1)
67-72: Silent error-swallowing oninit_index/init_embeddermakes this test fragile if it ever stops being#[ignore].Both calls are wrapped in
let _ = …, so if the runtime singletons are already populated by an earlier test in the same binary the bridge is wired to the localidxwhile the JSON-RPC path (openhuman.life_capture_search/_get_stats) reads from the previously-installed runtime — the assertions then fail with no obvious diagnostic. Today this is masked by#[ignore], but if anyone removes that gate later this becomes a confusing flake.Either log/assert on the init result, or document the single-test invariant inline:
📝 Surface init mismatches loudly
- let _ = runtime::init_index(Arc::clone(&idx)).await; + if let Err(e) = runtime::init_index(Arc::clone(&idx)).await { + eprintln!("[composio_e2e] runtime::init_index already set ({e}) — this test must run alone"); + } let embedder: Arc<dyn Embedder> = Arc::new(DeterministicEmbedder { dim: 1536 }); - let _ = runtime::init_embedder(Arc::clone(&embedder)).await; + if let Err(e) = runtime::init_embedder(Arc::clone(&embedder)).await { + eprintln!("[composio_e2e] runtime::init_embedder already set ({e})"); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/composio_ingest_e2e.rs` around lines 67 - 72, The test currently silences errors from runtime::init_index and runtime::init_embedder which can hide mismatched singleton state; change the calls to check and propagate failures instead of using `let _ = ...` — call runtime::init_index(Arc::clone(&idx)).await and runtime::init_embedder(Arc::clone(&embedder)).await and assert or unwrap their Results (or log and panic) so any attempt to reinitialize with a different PersonalIndex (opened via PersonalIndex::open) or Embedder (DeterministicEmbedder) fails loudly before calling register_life_capture_composio_bridge; alternatively add an explicit inline comment documenting the single-test invariant if you must keep the silent behavior.src/openhuman/life_capture/runtime.rs (1)
26-38:init_index/init_embedderareasync fnbut never.await— drop the async.
OnceCell::setis synchronous. Marking these functionsasyncadds spurious async-coloring to startup with zero benefit. All six call sites (insrc/core/jsonrpc.rsand test files) use.awaitand would need it removed:♻️ Proposed change
-pub async fn init_index(idx: Arc<PersonalIndex>) -> Result<(), &'static str> { +pub fn init_index(idx: Arc<PersonalIndex>) -> Result<(), &'static str> { INDEX .set(idx) .map_err(|_| "life_capture index already initialised") } -pub async fn init_embedder(embedder: Arc<dyn Embedder>) -> Result<(), &'static str> { +pub fn init_embedder(embedder: Arc<dyn Embedder>) -> Result<(), &'static str> { EMBEDDER .set(embedder) .map_err(|_| "life_capture embedder already initialised") }Update call sites to remove
.await:
src/core/jsonrpc.rs(lines 940, 973)tests/imessage_ingest_e2e.rs(lines 73, 75)tests/composio_ingest_e2e.rs(lines 68, 70)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/runtime.rs` around lines 26 - 38, The functions init_index and init_embedder are declared async but only call synchronous OnceCell::set (INDEX.set and EMBEDDER.set); remove the async from both signatures so they are regular synchronous functions returning Result<(), &'static str>, and update all call sites that currently do `.await` on init_index/init_embedder to call them without awaiting (i.e., remove the `.await`), leaving their return type and error handling unchanged.src/openhuman/life_capture/embedder.rs (2)
97-99: Hard-codeddim() = 1536couples this struct to a single model family.
text-embedding-3-smallis 1536-dim, buttext-embedding-3-largeis 3072 and other providers vary. Sincemodelis already a configurable field, a future config swap will fail the dim-check at lines 87‑92 with a confusing error. Consider taking the dimension viaHostedEmbedder::new(...)or auto-detecting from the first response.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/embedder.rs` around lines 97 - 99, The dim() method currently returns a hard-coded 1536 which ties the embedder to one model; change HostedEmbedder so dimension is configurable (e.g., add a dim field set via HostedEmbedder::new(...)) or implement auto-detection by calling the embedding API once and storing the returned vector length, then have dim() return that stored value; update any construction sites that create HostedEmbedder to supply/derive the correct dimension and remove the fixed 1536 constant so model swaps won’t break the runtime check in the embedding validation logic.
54-95: Add logging around the external embeddings call.This is a remote HTTP dependency on the request-serving path. Consider adding
tracing::debug!with[life_capture:embedder]prefix at entry (input count, model, base_url host only — neverapi_key), andtracing::warn!onerror_for_status/JSON parse failures with the response status. As per coding guidelines: "Add substantial development-oriented logging at entry/exit points, branch decisions, external calls, retries/timeouts" and "Never log secrets, raw JWTs, API keys".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/embedder.rs` around lines 54 - 95, The embed_batch function makes an external HTTP call without development logging; add a tracing::debug! at the start of embed_batch that logs the input count, self.model, and only the host portion of self.base_url (do NOT log self.api_key or any secrets) with a prefix like "[life_capture:embedder]"; then wrap the HTTP response handling to emit tracing::warn! (also prefixed) when error_for_status() or the subsequent JSON deserialization fails, including the HTTP status code and any non-sensitive response context (but not body or api_key), and include a final tracing::debug! on success (e.g., returned embeddings count and dim) before returning.src/openhuman/curated_memory/store.rs (2)
58-60: Drop a more informative error likeadddoes.
addreports"char limit {} exceeded"(line 42) butreplacejust says"char limit". Trivial consistency win for callers reading the error.♻️ Tweak
if next.chars().count() > self.char_limit { - return Err(std::io::Error::new(std::io::ErrorKind::Other, "char limit")); + return Err(std::io::Error::new( + std::io::ErrorKind::Other, + format!("char limit {} exceeded", self.char_limit), + )); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/curated_memory/store.rs` around lines 58 - 60, The replace branch returns a non-descriptive error string "char limit" when next.chars().count() > self.char_limit; make it consistent with add by returning an error that includes the limit value and context (e.g., "char limit {} exceeded") so callers see the threshold—update the error created in replace to format the message with self.char_limit, matching the add implementation.
98-102:with_extension("md.tmp")is fragile ifMemoryFile::filename()ever stops being.md.
Path::with_extensionreplaces the existing extension. TodayMemoryFile::filename()always ends in.mdsomemory.md→memory.md.tmp, but if a future variant returnsnotes.json, this would write tonotes.md.tmpand silently lose the.json. Appending instead of replacing is more robust, e.g.path.with_file_name(format!("{}.tmp", path.file_name().unwrap().to_string_lossy())). Minor — current callers are fine.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/curated_memory/store.rs` around lines 98 - 102, The atomic_write function uses path.with_extension("md.tmp") which replaces the existing extension and will break if MemoryFile::filename() stops using .md; change it to append a .tmp suffix to the entire filename instead (use path.file_name() + ".tmp" and call path.with_file_name(...)) so e.g. atomic_write builds tmp via something like path.with_file_name(format!("{}.tmp", path.file_name().unwrap().to_string_lossy())); ensure you handle the None case for file_name (return an io::Error) and keep the rest of atomic_write (tokio::fs::write then tokio::fs::rename) unchanged.src/openhuman/life_capture/rpc.rs (2)
76-82: Constant duplicated with index.rs — extract to types.rs to keep dim authoritative.
INDEX_VECTOR_DIM = 1536here mirrorsdim() = 1536inembedder.rsand thevec0schema width inindex.rs. If a future migration changes the column width, three places have to move in lockstep. Promoting this to apub const VECTOR_DIM: usize = 1536;intypes.rs(orindex.rs) and importing it into both call sites would keep things honest.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/rpc.rs` around lines 76 - 82, The duplicate hard-coded vector dimension (INDEX_VECTOR_DIM = 1536) used in rpc.rs should be centralized: create a public constant pub const VECTOR_DIM: usize = 1536 in types.rs (or an existing index-related module), remove the local INDEX_VECTOR_DIM, and import VECTOR_DIM into rpc.rs to use in the embedder.dim() check (replace INDEX_VECTOR_DIM with VECTOR_DIM). Update any other modules (e.g., where vec0 schema width or embedder.rs currently assume 1536) to reference the same VECTOR_DIM to keep the dimension authoritative across INDEX_VECTOR_DIM, embedder.dim(), and index schema usages.
142-170: Item is committed before the embedding succeeds — retry semantics are okay, but document/log the partial state.If
embed_batchorupsert_vectorfails afterupsertreturns, the row exists initems/items_ftswith no vector. Keyword search will still surface it; vector search won't. The next re-ingest by the same(source, external_id)will re-embed and patch it, so this is self-healing — but only if the caller retries.Two suggestions:
- Log a
tracing::warn!(withitem_id/source/external_id) on the embed/upsert_vector error paths so partial states are observable.- Consider noting this contract in the rustdoc on
handle_ingestso callers of the RPC know to retry on error to backfill the vector.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/rpc.rs` around lines 142 - 170, The upsert of the Item (requested_id -> canonical_id) happens before embedding, so failures in embed_batch or upsert_vector leave a row without a vector; update the embed/upsert_vector error paths to emit a tracing::warn! that includes the item id (canonical_id or requested_id), source and external_id so partial states are observable, and ensure the warn is logged just before returning the error from both the embed_batch .map_err and the upsert_vector .map_err locations; additionally add a short rustdoc on the handle_ingest RPC function explaining that callers should retry on error to allow a subsequent ingest to backfill the missing vector.src/openhuman/life_capture/ingest/composio.rs (2)
364-378:log::debug!/log::warn!here while the rest of the file usestracing— pick one.The bridge's hot path uses
tracing::{debug,warn}!(lines 57‑112), but registration useslog::{debug,warn}!. Both routes ultimately reach the configured subscriber, but mixing facades makes filtering by target less predictable and adds a small papercut for anyone grepping with a single prefix. Prefertracingto stay consistent with the rest of the bridge and the codebase's[domain]prefix convention.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/ingest/composio.rs` around lines 364 - 378, Replace the mixed use of log macros with tracing in the bridge registration path: where BRIDGE_HANDLE is checked and the bridge is created with LifeCaptureComposioBridge::new and registered via subscribe_global, change the log::debug! and log::warn! calls to tracing::debug! and tracing::warn! (or import tracing::{debug, warn}) so the registration messages use the same tracing facade/target as the rest of the file and maintain consistent filtering and [domain] prefix behavior.
417-424:std::mem::forget(dir)leaks the temp dir for the process lifetime.The intent is "keep the directory alive past the function return so the SQLite file at
dbremains valid".TempDir::into_path()expresses that explicitly (and lets the OS reclaim it on test-runner cleanup) without going throughmem::forget. Lower priority since this is test-only.♻️ Proposed tweak
- async fn fresh_bridge() -> LifeCaptureComposioBridge { - let dir = tempdir().expect("tempdir"); - let db = dir.path().join("lc.db"); - std::mem::forget(dir); - let idx = Arc::new(PersonalIndex::open(&db).await.expect("open")); + async fn fresh_bridge() -> LifeCaptureComposioBridge { + let dir = tempdir().expect("tempdir").into_path(); + let db = dir.join("lc.db"); + let idx = Arc::new(PersonalIndex::open(&db).await.expect("open"));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/ingest/composio.rs` around lines 417 - 424, The test helper fresh_bridge currently calls std::mem::forget(dir) which improperly leaks the TempDir; replace that with TempDir::into_path() so the temp directory is explicitly preserved for the lifetime of the test-runner cleanup. Concretely: create the tempdir as before, call dir.into_path() (or assign its result to a new PathBuf) to construct the db path used to open PersonalIndex, remove the std::mem::forget call, and keep the rest (opening PersonalIndex, creating Arc<TestEmbedder>, and calling LifeCaptureComposioBridge::new) unchanged.src/openhuman/life_capture/schemas.rs (1)
226-272: Adapter logging — consider emitting trace/debug at entry/exit for each handler.These adapters cross the JSON-RPC → domain boundary and currently log nothing on entry, exit, or failure paths. A
tracing::debug!with a stable[life_capture:rpc]prefix and the controller name + key params (e.g.source,external_idfor ingest;kand a redactedtext.len()for search) would make request flows traceable without leaking PII (don't logtextbody ormetadata). As per coding guidelines: "Add substantial development-oriented logging at entry/exit points, branch decisions, external calls".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openhuman/life_capture/schemas.rs` around lines 226 - 272, Add development-oriented trace/debug logs to the RPC adapters: in handle_get_stats, handle_search, and handle_ingest emit tracing::debug! logs with a stable "[life_capture:rpc]" prefix at function entry (and at successful exit) and on error branches; log only non-PII controller identifiers — for handle_ingest include source and external_id, for handle_search include k and text.len() (redacted), and for handle_get_stats a simple entry/exit marker — do not log text bodies or metadata. Place entry logs before calling runtime::get_index()/runtime::get_embedder() and downstream RPC calls, place exit logs after the to_json(...) result is produced, and include error context when mapping runtime errors or serde errors to strings so failures are traceable without leaking sensitive content.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/install.sh`:
- Around line 369-371: The call to resolve_from_release_api is currently a bare
command under set -e so the script will exit on nonzero returns before
resolve_rc is captured; change the invocation to an errexit-safe form (e.g.,
call it inside an if condition like `if resolve_from_release_api; then ... fi`
or append `|| true` so the function can return a nonzero code without exiting),
then capture its exit code into resolve_rc and let the existing dry-run/DRY RUN
messaging branch run when resolve_rc != 0; update the invocation site around
resolve_from_release_api and the resolve_rc handling so the dry-run branch (the
code that prints "DRY RUN: no compatible artifact available yet") is reachable.
In `@src/core/jsonrpc.rs`:
- Around line 949-989: The current early return when OPENHUMAN_EMBEDDINGS_KEY /
OPENAI_API_KEY is missing prevents register_life_capture_composio_bridge from
being called, silently disabling Composio Gmail/Calendar ingest; instead, keep
the bridge wired regardless of an embedder by moving the call to
crate::openhuman::life_capture::ingest::register_life_capture_composio_bridge(Arc::clone(&idx),
embedder_optional) out of the early-return path (or create a no-op LocalEmbedder
placeholder and still construct/embed it) and change the log message around the
api_key check to clearly state that the embedder is not configured but the
Composio bridge remains registered (or will use a noop embedder) so ingest still
flows to life_capture even when embeddings are not configured.
In `@src/openhuman/life_capture/embedder.rs`:
- Around line 21-25: The current construction of the reqwest client assigns to
http using reqwest::Client::builder().build().unwrap_or_else(|_|
reqwest::Client::new()), which silently drops the configured timeouts if build()
fails; change this so the error is not swallowed — either propagate the build
error by returning anyhow::Result (update the enclosing constructor function to
return Result and bubble up the reqwest::Error) or replace unwrap_or_else with
expect() so the process fails loudly on startup; locate the http assignment
(reqwest::Client::builder(), .timeout(), .connect_timeout(), .build()) in
embedder.rs and update the surrounding function signature/return path
accordingly.
In `@src/openhuman/life_capture/ingest/composio.rs`:
- Around line 604-623: The test currently asserts a tautology and never calls
the bridge methods; replace the dead Stub and the local `name` tautology with an
actual bridge instance (use `fresh_bridge().await` or whichever async helper
constructs `LifeCaptureComposioBridge`) and call `name()` and `domains()` on
that instance, asserting `bridge.name() == "life_capture::composio_bridge"` and
that `bridge.domains()` contains the expected domain(s) (or equals the expected
slice), ensuring the test is async if needed and awaits `fresh_bridge()`; remove
the unused `Stub` impl and any unused imports.
- Around line 55-75: The Gmail/Calendar ingest flow currently passes raw text
from normalize_gmail/normalize_calendar through do_ingest → handle_ingest →
IndexWriter::upsert without redaction or quoted-reply stripping; update the
ingest path to run the extracted text (and any text-like metadata) through the
quote_strip and redact utilities before it reaches IndexWriter::upsert.
Concretely, amend ingest_gmail/ingest_calendar or centralize in
do_ingest/handle_ingest to call quote_strip::strip (or equivalent) first, then
redact::redact (or equivalent) on the resulting string(s), and pass the
sanitized subject/text/metadata onwards so IndexWriter::upsert never receives
raw unredacted content. Ensure you reference normalize_gmail/normalize_calendar
inputs and preserve timestamps/external_id while replacing text fields with the
sanitized versions.
In `@src/openhuman/life_capture/schemas.rs`:
- Around line 188-193: The schema declares the FieldSchema for "metadata" as
TypeSchema::Option(Box::new(TypeSchema::String)) but the request handler reads
it as a serde_json::Value (defaulting to an empty Object), so change the schema
to reflect the actual contract: update the FieldSchema for "metadata" to
TypeSchema::Option(Box::new(TypeSchema::Value)) or a concrete JSON object
variant so clients know to send JSON, and ensure the handler still uses the
serde_json::Value path; alternatively, if you prefer to keep TypeSchema::String,
modify the handler code that reads "metadata" to parse the incoming string into
serde_json::Value (and validate errors) instead of accepting arbitrary objects —
update whichever of FieldSchema::metadata or the handler's parsing logic you
choose so schema and handler agree.
In `@src/openhuman/tools/impl/life_capture/search.rs`:
- Around line 67-71: The code reads k from args into variable k (via
args.get("k").and_then(Value::as_u64).map_or(10, |v| v as usize)) but does not
enforce the documented/schema cap of 100; update the parsing so k is clamped to
the range [1,100] before calling handle_search (e.g., read the u64, convert
safely, then apply min/max/clamp) to ensure the value respect the
parameters_schema and prevents unbounded work by handle_search/hybrid_search.
In `@tests/fixtures/life_capture/corpus.json`:
- Around line 320-332: The negative set for the test case with id "q-src-01" is
missing an additional non-calendar Phoenix item; update the object for q-src-01
to add "phoenix-weather-noise-01" to the must_not_contain array (and keep
must_not_contain_in_top as-is) so the query will correctly assert that
calendar-only results exclude that Gmail item once the pending gate is removed.
In `@tests/imessage_ingest_e2e.rs`:
- Around line 159-167: The assertion that stats1["total_items"] == 1 is brittle
due to shared process-global PersonalIndex; before performing the first ingest,
fetch a baseline with post_rpc(&base, "openhuman.life_capture_get_stats",
json!({})) (reuse rpc_result_body to parse) and record baseline_total =
stats_baseline["total_items"], then after ingest compute delta =
stats1["total_items"] - baseline_total and assert delta == 1; update the test to
use these symbols (env_stats_baseline, stats_baseline, baseline_total,
env_stats1, stats1) so the test asserts a delta rather than an absolute total.
---
Outside diff comments:
In `@app/src-tauri/src/imessage_scanner/mod.rs`:
- Around line 1-6: Update the module-level docstring at the top of
imessage_scanner::mod.rs to reflect the current RPC method name: replace
references to `openhuman.memory_doc_ingest` with `openhuman.life_capture_ingest`
so the comment matches the actual call made later in this module (see the ingest
call in the code that invokes `openhuman.life_capture_ingest`). Ensure the rest
of the descriptive text (about chat.db grouping and the referenced playbook)
remains intact.
In `@scripts/install.sh`:
- Around line 277-304: The regex patterns used to select macOS assets (e.g.,
r"aarch64.*\\.app\\.tar\\.gz$", r"aarch64\\.dmg$",
r"(x86_64-apple-darwin|x64).*\\.app\\.tar\\.gz$", r"x64\\.dmg$") are written
with doubled backslashes inside the single-quoted heredoc (<<'PY'), causing them
to look for literal backslashes and fail to match filenames; update those
patterns to use single backslashes (e.g., r"aarch64\.dmg$",
r"aarch64.*\.app\.tar\.gz$", r"(x86_64-apple-darwin|x64).*\.app\.tar\.gz$",
r"x64\.dmg$") so the loops that iterate over names and set chosen (and the
subsequent assets loop that compares asset.get("name") to chosen) correctly find
macOS assets.
In `@src/core/all.rs`:
- Around line 252-311: Add two match arms to the namespace_description function
to return descriptions for the new namespaces: add "life_capture" =>
Some("Capture of user life events, media, and context for personal history and
recall.") and "curated_memory" => Some("Curated, reviewed memory entries for
long-term persistence and summarized retrieval."); place these entries alongside
the other namespace string arms in namespace_description so CLI help shows
one-line descriptions for both new namespaces.
---
Nitpick comments:
In `@app/src-tauri/src/imessage_scanner/mod.rs`:
- Around line 413-452: The comment claims ts is "UTC midnight of a local-tz day"
but seconds_to_ymd renders day in the local timezone while parse_day_to_unix
currently produces UTC midnight, causing up-to ~24h mismatch; either update the
comment to say ts is a stable anchor derived from the local-tz day, or change
parse_day_to_unix to compute the local-tz midnight for the provided YYYY-MM-DD
string and return its UTC unix-seconds instead (use chrono::Local or convert
NaiveDate.and_hms_opt(0,0,0) with Local::from_local_datetime and then to_utc
timestamp). Update the code in parse_day_to_unix (and keep seconds_to_ymd
behavior) so ts semantics match the comment, or leave parse_day_to_unix as-is
and tighten the comment to refer to a "stable anchor derived from the local-tz
day" mentioning parse_day_to_unix, seconds_to_ymd, and the ts variable.
In `@src/openhuman/agent/agents/integrations_agent/prompt.rs`:
- Line 171: The compile error is due to the updated PromptContext shape missing
the curated_snapshot field; update the construction of PromptContext in
agents/integrations_agent/prompt.rs to include curated_snapshot: None (matching
the new struct field) so the struct literal compiles, and consider adding a
PromptContext::test_default() or a test-only Default impl for PromptContext to
reduce duplication across the seven agent prompt-test files in a future cleanup.
In `@src/openhuman/agent/harness/session/types.rs`:
- Around line 135-136: The field curated_snapshot is using the fully qualified
std::sync::Arc type; change it to use the in-scope Arc to match the rest of the
file (e.g., other usages like Arc<dyn Provider> and Arc<dyn PayloadSummarizer>).
Locate the declaration of curated_snapshot (type
Option<std::sync::Arc<crate::openhuman::curated_memory::MemorySnapshot>>) and
replace std::sync::Arc with the unqualified Arc so the field reads
Option<Arc<crate::openhuman::curated_memory::MemorySnapshot>>>, keeping the same
crate path for MemorySnapshot.
In `@src/openhuman/curated_memory/store.rs`:
- Around line 58-60: The replace branch returns a non-descriptive error string
"char limit" when next.chars().count() > self.char_limit; make it consistent
with add by returning an error that includes the limit value and context (e.g.,
"char limit {} exceeded") so callers see the threshold—update the error created
in replace to format the message with self.char_limit, matching the add
implementation.
- Around line 98-102: The atomic_write function uses
path.with_extension("md.tmp") which replaces the existing extension and will
break if MemoryFile::filename() stops using .md; change it to append a .tmp
suffix to the entire filename instead (use path.file_name() + ".tmp" and call
path.with_file_name(...)) so e.g. atomic_write builds tmp via something like
path.with_file_name(format!("{}.tmp",
path.file_name().unwrap().to_string_lossy())); ensure you handle the None case
for file_name (return an io::Error) and keep the rest of atomic_write
(tokio::fs::write then tokio::fs::rename) unchanged.
In `@src/openhuman/life_capture/embedder.rs`:
- Around line 97-99: The dim() method currently returns a hard-coded 1536 which
ties the embedder to one model; change HostedEmbedder so dimension is
configurable (e.g., add a dim field set via HostedEmbedder::new(...)) or
implement auto-detection by calling the embedding API once and storing the
returned vector length, then have dim() return that stored value; update any
construction sites that create HostedEmbedder to supply/derive the correct
dimension and remove the fixed 1536 constant so model swaps won’t break the
runtime check in the embedding validation logic.
- Around line 54-95: The embed_batch function makes an external HTTP call
without development logging; add a tracing::debug! at the start of embed_batch
that logs the input count, self.model, and only the host portion of
self.base_url (do NOT log self.api_key or any secrets) with a prefix like
"[life_capture:embedder]"; then wrap the HTTP response handling to emit
tracing::warn! (also prefixed) when error_for_status() or the subsequent JSON
deserialization fails, including the HTTP status code and any non-sensitive
response context (but not body or api_key), and include a final tracing::debug!
on success (e.g., returned embeddings count and dim) before returning.
In `@src/openhuman/life_capture/ingest/composio.rs`:
- Around line 364-378: Replace the mixed use of log macros with tracing in the
bridge registration path: where BRIDGE_HANDLE is checked and the bridge is
created with LifeCaptureComposioBridge::new and registered via subscribe_global,
change the log::debug! and log::warn! calls to tracing::debug! and
tracing::warn! (or import tracing::{debug, warn}) so the registration messages
use the same tracing facade/target as the rest of the file and maintain
consistent filtering and [domain] prefix behavior.
- Around line 417-424: The test helper fresh_bridge currently calls
std::mem::forget(dir) which improperly leaks the TempDir; replace that with
TempDir::into_path() so the temp directory is explicitly preserved for the
lifetime of the test-runner cleanup. Concretely: create the tempdir as before,
call dir.into_path() (or assign its result to a new PathBuf) to construct the db
path used to open PersonalIndex, remove the std::mem::forget call, and keep the
rest (opening PersonalIndex, creating Arc<TestEmbedder>, and calling
LifeCaptureComposioBridge::new) unchanged.
In `@src/openhuman/life_capture/migrations/0001_init.sql`:
- Line 9: The column text_keyword is declared but not used by items_fts or the
AI/AD/AU triggers, which is misleading; update this migration to either wire it
up or make its intent explicit—prefer the low-risk option: keep the column but
change its inline comment to state it's a reserved placeholder for a future
migration (e.g., "reserved for raw values projection for FTS5/BM25; indexed and
wired up in a follow-up migration"), and leave items_fts and the triggers
(AI/AD/AU) unchanged so behavior stays stable.
In `@src/openhuman/life_capture/quote_strip.rs`:
- Around line 4-7: The ON_DATE_WROTE regex currently uses .{1,200} which won't
match when the "On … wrote:" header is wrapped across lines; update the pattern
in the Lazy::new closure (where Regex::new is called for ON_DATE_WROTE) to allow
newlines in that span (either add the DOTALL flag via (?s) or replace .{1,200}
with a permissive [\s\S]{1,400} or similar) so wrapped headers like "On
...\n<email> wrote:" are matched; keep the word boundary and trailing wrote:\s*$
anchors intact and adjust the max length as needed.
In `@src/openhuman/life_capture/redact.rs`:
- Around line 8-17: The PHONE regex (static PHONE: Lazy<Regex>) lacks word
boundaries and can over-match digit substrings inside opaque IDs; update the
regex to add \b anchors at the start and end of the pattern (i.e., wrap the
whole pattern with \b ... \b or equivalent) so it only matches standalone
phone-like tokens, and add a unit test asserting strings like
"order-1234567890123" are not redacted (and other positive tests to ensure real
phone numbers still redact).
In `@src/openhuman/life_capture/rpc.rs`:
- Around line 76-82: The duplicate hard-coded vector dimension (INDEX_VECTOR_DIM
= 1536) used in rpc.rs should be centralized: create a public constant pub const
VECTOR_DIM: usize = 1536 in types.rs (or an existing index-related module),
remove the local INDEX_VECTOR_DIM, and import VECTOR_DIM into rpc.rs to use in
the embedder.dim() check (replace INDEX_VECTOR_DIM with VECTOR_DIM). Update any
other modules (e.g., where vec0 schema width or embedder.rs currently assume
1536) to reference the same VECTOR_DIM to keep the dimension authoritative
across INDEX_VECTOR_DIM, embedder.dim(), and index schema usages.
- Around line 142-170: The upsert of the Item (requested_id -> canonical_id)
happens before embedding, so failures in embed_batch or upsert_vector leave a
row without a vector; update the embed/upsert_vector error paths to emit a
tracing::warn! that includes the item id (canonical_id or requested_id), source
and external_id so partial states are observable, and ensure the warn is logged
just before returning the error from both the embed_batch .map_err and the
upsert_vector .map_err locations; additionally add a short rustdoc on the
handle_ingest RPC function explaining that callers should retry on error to
allow a subsequent ingest to backfill the missing vector.
In `@src/openhuman/life_capture/runtime.rs`:
- Around line 26-38: The functions init_index and init_embedder are declared
async but only call synchronous OnceCell::set (INDEX.set and EMBEDDER.set);
remove the async from both signatures so they are regular synchronous functions
returning Result<(), &'static str>, and update all call sites that currently do
`.await` on init_index/init_embedder to call them without awaiting (i.e., remove
the `.await`), leaving their return type and error handling unchanged.
In `@src/openhuman/life_capture/schemas.rs`:
- Around line 226-272: Add development-oriented trace/debug logs to the RPC
adapters: in handle_get_stats, handle_search, and handle_ingest emit
tracing::debug! logs with a stable "[life_capture:rpc]" prefix at function entry
(and at successful exit) and on error branches; log only non-PII controller
identifiers — for handle_ingest include source and external_id, for
handle_search include k and text.len() (redacted), and for handle_get_stats a
simple entry/exit marker — do not log text bodies or metadata. Place entry logs
before calling runtime::get_index()/runtime::get_embedder() and downstream RPC
calls, place exit logs after the to_json(...) result is produced, and include
error context when mapping runtime errors or serde errors to strings so failures
are traceable without leaking sensitive content.
In `@src/openhuman/tools/ops.rs`:
- Around line 122-126: Add a regression-guard unit test that ensures
life_capture_search is always registered by calling all_tools and asserting the
returned tool names contain "life_capture_search"; mirror the existing pattern
used for other always-on tools (create a TempDir, SecurityPolicy, Memory via
create_memory, BrowserConfig, HttpRequestConfig, test_config, call all_tools,
collect names with .iter().map(|t| t.name()), and
assert!(names.contains(&"life_capture_search"))). Place this test alongside the
other registration guards (next to tests that assert presence of
spawn_subagent/complete_onboarding/current_time/etc.) so future refactors that
accidentally gate or remove LifeCaptureSearchTool::new() will fail CI.
In `@tests/composio_ingest_e2e.rs`:
- Around line 67-72: The test currently silences errors from runtime::init_index
and runtime::init_embedder which can hide mismatched singleton state; change the
calls to check and propagate failures instead of using `let _ = ...` — call
runtime::init_index(Arc::clone(&idx)).await and
runtime::init_embedder(Arc::clone(&embedder)).await and assert or unwrap their
Results (or log and panic) so any attempt to reinitialize with a different
PersonalIndex (opened via PersonalIndex::open) or Embedder
(DeterministicEmbedder) fails loudly before calling
register_life_capture_composio_bridge; alternatively add an explicit inline
comment documenting the single-test invariant if you must keep the silent
behavior.
🪄 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: 5d56ec5b-86da-4d95-83da-54582938622d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (72)
.github/workflows/installer-smoke.ymlCargo.tomlapp/src-tauri/src/imessage_scanner/mod.rsdocs/superpowers/plans/2026-04-21-imessage-live-harness.mddocs/superpowers/plans/2026-04-22-life-capture-01-foundation.mddocs/superpowers/plans/2026-04-22-track1-ship-pipeline.mddocs/superpowers/specs/2026-04-22-life-capture-layer-design.mdscripts/fixtures/latest.jsonscripts/install.shscripts/test_install.shsrc/core/all.rssrc/core/jsonrpc.rssrc/openhuman/agent/agents/archivist/prompt.rssrc/openhuman/agent/agents/code_executor/prompt.rssrc/openhuman/agent/agents/critic/prompt.rssrc/openhuman/agent/agents/help/prompt.rssrc/openhuman/agent/agents/integrations_agent/prompt.rssrc/openhuman/agent/agents/loader.rssrc/openhuman/agent/agents/morning_briefing/prompt.rssrc/openhuman/agent/agents/orchestrator/prompt.rssrc/openhuman/agent/agents/planner/prompt.rssrc/openhuman/agent/agents/researcher/prompt.rssrc/openhuman/agent/agents/summarizer/prompt.rssrc/openhuman/agent/agents/tool_maker/prompt.rssrc/openhuman/agent/agents/tools_agent/prompt.rssrc/openhuman/agent/agents/trigger_reactor/prompt.rssrc/openhuman/agent/agents/trigger_triage/prompt.rssrc/openhuman/agent/agents/welcome/prompt.rssrc/openhuman/agent/debug/mod.rssrc/openhuman/agent/harness/fork_context.rssrc/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/harness/session/runtime.rssrc/openhuman/agent/harness/session/turn.rssrc/openhuman/agent/harness/session/types.rssrc/openhuman/agent/harness/subagent_runner/ops.rssrc/openhuman/agent/prompts/mod.rssrc/openhuman/agent/prompts/types.rssrc/openhuman/agent/triage/escalation.rssrc/openhuman/agent/triage/evaluator.rssrc/openhuman/curated_memory/mod.rssrc/openhuman/curated_memory/rpc.rssrc/openhuman/curated_memory/runtime.rssrc/openhuman/curated_memory/schemas.rssrc/openhuman/curated_memory/store.rssrc/openhuman/curated_memory/types.rssrc/openhuman/learning/prompt_sections.rssrc/openhuman/life_capture/embedder.rssrc/openhuman/life_capture/index.rssrc/openhuman/life_capture/ingest/composio.rssrc/openhuman/life_capture/ingest/mod.rssrc/openhuman/life_capture/migrations.rssrc/openhuman/life_capture/migrations/0001_init.sqlsrc/openhuman/life_capture/migrations/0002_vec.sqlsrc/openhuman/life_capture/mod.rssrc/openhuman/life_capture/quote_strip.rssrc/openhuman/life_capture/redact.rssrc/openhuman/life_capture/rpc.rssrc/openhuman/life_capture/runtime.rssrc/openhuman/life_capture/schemas.rssrc/openhuman/life_capture/tests/e2e.rssrc/openhuman/life_capture/tests/mod.rssrc/openhuman/life_capture/types.rssrc/openhuman/mod.rssrc/openhuman/tools/impl/life_capture/mod.rssrc/openhuman/tools/impl/life_capture/search.rssrc/openhuman/tools/impl/mod.rssrc/openhuman/tools/ops.rstests/agent_harness_public.rstests/composio_ingest_e2e.rstests/fixtures/life_capture/corpus.jsontests/imessage_ingest_e2e.rstests/life_capture_retrieval_eval.rs
| resolve_from_release_api | ||
| resolve_rc=$? | ||
| if [ "${resolve_rc}" -ne 0 ]; then |
There was a problem hiding this comment.
set -e will short-circuit the new rc-2 fallback — script exits before resolve_rc is captured.
The script is run under set -euo pipefail (Line 6). With set -e active, a bare resolve_from_release_api statement that returns nonzero terminates the script immediately — Bash only suppresses errexit when the command is in an if/while/until condition, in a &&/|| chain, or inverted with !. So when the function hits its return 1 (curl/parse failed) or return 2 (no compatible asset) paths, line 370 (resolve_rc=$?) and the new dry-run branching at 372-388 are unreachable.
This regresses the very behaviour you added — a Linux user without a published asset can't even dry-run because the script dies at line 369 instead of falling into the friendly DRY RUN: no compatible artifact available yet exit.
🐛 Restore the errexit-safe call form
- resolve_from_release_api
- resolve_rc=$?
+ resolve_rc=0
+ resolve_from_release_api || resolve_rc=$?
if [ "${resolve_rc}" -ne 0 ]; thenOr, equivalently, keep the explicit guarded form:
- resolve_from_release_api
- resolve_rc=$?
- if [ "${resolve_rc}" -ne 0 ]; then
+ if ! resolve_from_release_api; then
+ resolve_rc=$?📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| resolve_from_release_api | |
| resolve_rc=$? | |
| if [ "${resolve_rc}" -ne 0 ]; then | |
| resolve_rc=0 | |
| resolve_from_release_api || resolve_rc=$? | |
| if [ "${resolve_rc}" -ne 0 ]; then |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/install.sh` around lines 369 - 371, The call to
resolve_from_release_api is currently a bare command under set -e so the script
will exit on nonzero returns before resolve_rc is captured; change the
invocation to an errexit-safe form (e.g., call it inside an if condition like
`if resolve_from_release_api; then ... fi` or append `|| true` so the function
can return a nonzero code without exiting), then capture its exit code into
resolve_rc and let the existing dry-run/DRY RUN messaging branch run when
resolve_rc != 0; update the invocation site around resolve_from_release_api and
the resolve_rc handling so the dry-run branch (the code that prints "DRY RUN: no
compatible artifact available yet") is reachable.
| let api_key = std::env::var("OPENHUMAN_EMBEDDINGS_KEY") | ||
| .or_else(|_| std::env::var("OPENAI_API_KEY")) | ||
| .ok(); | ||
| let Some(api_key) = api_key else { | ||
| log::info!( | ||
| "[life_capture] embedder not configured (set OPENAI_API_KEY or \ | ||
| OPENHUMAN_EMBEDDINGS_KEY); life_capture.search will return \ | ||
| 'embedder not configured', life_capture.get_stats remains available" | ||
| ); | ||
| return; | ||
| }; | ||
|
|
||
| let base_url = std::env::var("OPENHUMAN_EMBEDDINGS_URL") | ||
| .unwrap_or_else(|_| "https://api.openai.com/v1".into()); | ||
| let model = std::env::var("OPENHUMAN_EMBEDDINGS_MODEL") | ||
| .unwrap_or_else(|_| "text-embedding-3-small".into()); | ||
| let embedder: Arc<dyn crate::openhuman::life_capture::embedder::Embedder> = Arc::new( | ||
| crate::openhuman::life_capture::embedder::HostedEmbedder::new( | ||
| base_url, | ||
| api_key, | ||
| model.clone(), | ||
| ), | ||
| ); | ||
| if let Err(e) = | ||
| crate::openhuman::life_capture::runtime::init_embedder(Arc::clone(&embedder)).await | ||
| { | ||
| log::debug!("[life_capture] embedder init skipped: {e}"); | ||
| } else { | ||
| log::info!("[life_capture] embedder initialised — model={model}"); | ||
| } | ||
|
|
||
| // Alongside the existing `register_composio_trigger_subscriber()` (triage + | ||
| // history), install a second subscriber that routes Gmail + Google Calendar | ||
| // Composio triggers into the PersonalIndex. Safe to call here because both | ||
| // the index and embedder are now known-good, and the event bus registrar | ||
| // is idempotent. | ||
| crate::openhuman::life_capture::ingest::register_life_capture_composio_bridge( | ||
| Arc::clone(&idx), | ||
| embedder, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Composio Gmail/Calendar ingest is silently disabled when no embeddings key is set.
The early return at Line 958 short-circuits before register_life_capture_composio_bridge (Line 985), so on installs without OPENAI_API_KEY / OPENHUMAN_EMBEDDINGS_KEY the A2 bridge is never wired up — Composio triggers for GMAIL_NEW_GMAIL_MESSAGE etc. flow past life_capture entirely. The info log at Lines 953-957 only mentions search; it doesn't tell the user that ingest is also off, so this looks like the bridge "doesn't work" rather than "needs an embedder."
Suggest tightening the log to call out both failure modes so operators aren't surprised:
📝 Tighter log message
let Some(api_key) = api_key else {
log::info!(
"[life_capture] embedder not configured (set OPENAI_API_KEY or \
- OPENHUMAN_EMBEDDINGS_KEY); life_capture.search will return \
- 'embedder not configured', life_capture.get_stats remains available"
+ OPENHUMAN_EMBEDDINGS_KEY); life_capture.search will return \
+ 'embedder not configured', life_capture.get_stats remains available, \
+ and the Composio Gmail/Calendar bridge will NOT be registered \
+ (events will be dropped at the bus until an embedder is configured)"
);
return;
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/jsonrpc.rs` around lines 949 - 989, The current early return when
OPENHUMAN_EMBEDDINGS_KEY / OPENAI_API_KEY is missing prevents
register_life_capture_composio_bridge from being called, silently disabling
Composio Gmail/Calendar ingest; instead, keep the bridge wired regardless of an
embedder by moving the call to
crate::openhuman::life_capture::ingest::register_life_capture_composio_bridge(Arc::clone(&idx),
embedder_optional) out of the early-return path (or create a no-op LocalEmbedder
placeholder and still construct/embed it) and change the log message around the
api_key check to clearly state that the embedder is not configured but the
Composio bridge remains registered (or will use a noop embedder) so ingest still
flows to life_capture even when embeddings are not configured.
| let http = reqwest::Client::builder() | ||
| .timeout(std::time::Duration::from_secs(30)) | ||
| .connect_timeout(std::time::Duration::from_secs(10)) | ||
| .build() | ||
| .unwrap_or_else(|_| reqwest::Client::new()); |
There was a problem hiding this comment.
Silent fallback drops the configured timeouts.
If Client::builder().build() ever fails, the fallback reqwest::Client::new() returns a client with no timeouts — meaning a stuck embeddings endpoint can hang request threads indefinitely instead of failing in 30s. Either propagate the error from new (return anyhow::Result<Self>) or expect() here; a default-config fallback for an external-call client is worse than crashing loudly at startup.
🛡️ Proposed fix
- pub fn new(base_url: String, api_key: String, model: String) -> Self {
- let http = reqwest::Client::builder()
- .timeout(std::time::Duration::from_secs(30))
- .connect_timeout(std::time::Duration::from_secs(10))
- .build()
- .unwrap_or_else(|_| reqwest::Client::new());
- Self {
+ pub fn new(base_url: String, api_key: String, model: String) -> anyhow::Result<Self> {
+ let http = reqwest::Client::builder()
+ .timeout(std::time::Duration::from_secs(30))
+ .connect_timeout(std::time::Duration::from_secs(10))
+ .build()
+ .context("build hosted embedder http client")?;
+ Ok(Self {
base_url,
api_key,
model,
http,
- }
+ })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let http = reqwest::Client::builder() | |
| .timeout(std::time::Duration::from_secs(30)) | |
| .connect_timeout(std::time::Duration::from_secs(10)) | |
| .build() | |
| .unwrap_or_else(|_| reqwest::Client::new()); | |
| pub fn new(base_url: String, api_key: String, model: String) -> anyhow::Result<Self> { | |
| let http = reqwest::Client::builder() | |
| .timeout(std::time::Duration::from_secs(30)) | |
| .connect_timeout(std::time::Duration::from_secs(10)) | |
| .build() | |
| .context("build hosted embedder http client")?; | |
| Ok(Self { | |
| base_url, | |
| api_key, | |
| model, | |
| http, | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/life_capture/embedder.rs` around lines 21 - 25, The current
construction of the reqwest client assigns to http using
reqwest::Client::builder().build().unwrap_or_else(|_| reqwest::Client::new()),
which silently drops the configured timeouts if build() fails; change this so
the error is not swallowed — either propagate the build error by returning
anyhow::Result (update the enclosing constructor function to return Result and
bubble up the reqwest::Error) or replace unwrap_or_else with expect() so the
process fails loudly on startup; locate the http assignment
(reqwest::Client::builder(), .timeout(), .connect_timeout(), .build()) in
embedder.rs and update the surrounding function signature/return path
accordingly.
| async fn ingest_gmail(&self, payload: &Value) { | ||
| let Some((external_id, ts, subject, text, metadata)) = normalize_gmail(payload) else { | ||
| tracing::debug!( | ||
| "[life_capture:composio] gmail payload missing required fields — skipping" | ||
| ); | ||
| return; | ||
| }; | ||
| self.do_ingest(Source::Gmail, external_id, ts, subject, text, metadata) | ||
| .await; | ||
| } | ||
|
|
||
| async fn ingest_calendar(&self, payload: &Value) { | ||
| let Some((external_id, ts, subject, text, metadata)) = normalize_calendar(payload) else { | ||
| tracing::debug!( | ||
| "[life_capture:composio] calendar payload missing required fields — skipping" | ||
| ); | ||
| return; | ||
| }; | ||
| self.do_ingest(Source::Calendar, external_id, ts, subject, text, metadata) | ||
| .await; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether redaction/quote-strip is applied on the ingest path before
# items hit SQLite. Look for a redaction module and any callers from rpc.rs / index.rs.
fd -t f -e rs 'redact|pii|quote' src/openhuman/life_capture
rg -nP -C3 '(redact|pii|strip_quote|quote_strip)\s*\(' src/openhuman/life_capture
ast-grep --pattern 'pub async fn handle_ingest($$$) { $$$ }'
ast-grep --pattern 'pub async fn upsert($$$) -> $_ { $$$ }'Repository: tinyhumansai/openhuman
Length of output: 11884
🏁 Script executed:
# Find handle_ingest function definition
ast-grep --pattern 'pub async fn handle_ingest($_$, $_$, $_$, $_$, $_$, $_$) { $$$ }'
# Check normalize_gmail and normalize_calendar implementations
rg -n 'fn normalize_gmail|fn normalize_calendar' src/openhuman/life_capture
# Check do_ingest implementation in composio.rs
rg -n -A15 'async fn do_ingest' src/openhuman/life_capture/ingest/composio.rsRepository: tinyhumansai/openhuman
Length of output: 774
🏁 Script executed:
# Get full do_ingest implementation
sed -n '77,105p' src/openhuman/life_capture/ingest/composio.rs
# Get handle_ingest function signature and initial implementation
rg -n -A30 'async fn handle_ingest' src/openhuman/life_capture/rpc.rs
# Check if redaction is called anywhere in the do_ingest path
rg -n 'redact\|quote_strip' src/openhuman/life_capture/ingest/composio.rsRepository: tinyhumansai/openhuman
Length of output: 1734
🏁 Script executed:
# Get the rest of handle_ingest after line 152
sed -n '123,200p' src/openhuman/life_capture/rpc.rs
# Check if redaction happens in life_capture module initialization or somewhere in the pipeline
rg -n 'redact\|quote_strip' src/openhuman/life_capture/rpc.rsRepository: tinyhumansai/openhuman
Length of output: 1595
Apply redaction and quote-stripping before text is persisted.
normalize_gmail extracts the raw email body and passes it through do_ingest → handle_ingest → IndexWriter::upsert without any PII redaction or quoted-reply stripping. The redact and quote_strip modules exist and are unit-tested, but they are not wired into this ingest path; consequently, raw email bodies (including quoted reply chains and any incidentally-leaked secrets) persist to SQLite unredacted.
Redaction must be applied in the composio ingest flow before IndexWriter::upsert is called, consistent with the coding guideline: "Never log secrets, raw JWTs, API keys, credentials, or full PII."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/life_capture/ingest/composio.rs` around lines 55 - 75, The
Gmail/Calendar ingest flow currently passes raw text from
normalize_gmail/normalize_calendar through do_ingest → handle_ingest →
IndexWriter::upsert without redaction or quoted-reply stripping; update the
ingest path to run the extracted text (and any text-like metadata) through the
quote_strip and redact utilities before it reaches IndexWriter::upsert.
Concretely, amend ingest_gmail/ingest_calendar or centralize in
do_ingest/handle_ingest to call quote_strip::strip (or equivalent) first, then
redact::redact (or equivalent) on the resulting string(s), and pass the
sanitized subject/text/metadata onwards so IndexWriter::upsert never receives
raw unredacted content. Ensure you reference normalize_gmail/normalize_calendar
inputs and preserve timestamps/external_id while replacing text fields with the
sanitized versions.
| #[test] | ||
| fn bridge_has_stable_name_and_domain() { | ||
| // Construct with stubs just to exercise the trait methods. | ||
| struct Stub; | ||
| #[async_trait] | ||
| impl Embedder for Stub { | ||
| async fn embed_batch(&self, _t: &[&str]) -> anyhow::Result<Vec<Vec<f32>>> { | ||
| Ok(vec![]) | ||
| } | ||
| fn dim(&self) -> usize { | ||
| 1536 | ||
| } | ||
| } | ||
| // Lightweight non-init path — we don't actually need a live index | ||
| // for the trait introspection bits, but we do need a real | ||
| // PersonalIndex handle. Reuse the fresh_bridge pattern via a | ||
| // smoke check in the runtime test. | ||
| let name = "life_capture::composio_bridge"; | ||
| assert_eq!(name, name); // shape guard for the name constant | ||
| } |
There was a problem hiding this comment.
This test does not actually verify name() or domains() on the bridge.
The body just constructs an unused local name string and asserts assert_eq!(name, name) — a tautology. The Stub Embedder impl is also dead code. Either drop the test or rewrite it to call the trait methods on a real LifeCaptureComposioBridge instance (e.g. via fresh_bridge().await).
🧪 Proposed rewrite
- #[test]
- fn bridge_has_stable_name_and_domain() {
- // Construct with stubs just to exercise the trait methods.
- struct Stub;
- #[async_trait]
- impl Embedder for Stub {
- async fn embed_batch(&self, _t: &[&str]) -> anyhow::Result<Vec<Vec<f32>>> {
- Ok(vec![])
- }
- fn dim(&self) -> usize {
- 1536
- }
- }
- // Lightweight non-init path — we don't actually need a live index
- // for the trait introspection bits, but we do need a real
- // PersonalIndex handle. Reuse the fresh_bridge pattern via a
- // smoke check in the runtime test.
- let name = "life_capture::composio_bridge";
- assert_eq!(name, name); // shape guard for the name constant
- }
+ #[tokio::test]
+ async fn bridge_has_stable_name_and_domain() {
+ let bridge = fresh_bridge().await;
+ assert_eq!(bridge.name(), "life_capture::composio_bridge");
+ assert_eq!(bridge.domains(), Some(&["composio"][..]));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| fn bridge_has_stable_name_and_domain() { | |
| // Construct with stubs just to exercise the trait methods. | |
| struct Stub; | |
| #[async_trait] | |
| impl Embedder for Stub { | |
| async fn embed_batch(&self, _t: &[&str]) -> anyhow::Result<Vec<Vec<f32>>> { | |
| Ok(vec![]) | |
| } | |
| fn dim(&self) -> usize { | |
| 1536 | |
| } | |
| } | |
| // Lightweight non-init path — we don't actually need a live index | |
| // for the trait introspection bits, but we do need a real | |
| // PersonalIndex handle. Reuse the fresh_bridge pattern via a | |
| // smoke check in the runtime test. | |
| let name = "life_capture::composio_bridge"; | |
| assert_eq!(name, name); // shape guard for the name constant | |
| } | |
| #[tokio::test] | |
| async fn bridge_has_stable_name_and_domain() { | |
| let bridge = fresh_bridge().await; | |
| assert_eq!(bridge.name(), "life_capture::composio_bridge"); | |
| assert_eq!(bridge.domains(), Some(&["composio"][..])); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/life_capture/ingest/composio.rs` around lines 604 - 623, The
test currently asserts a tautology and never calls the bridge methods; replace
the dead Stub and the local `name` tautology with an actual bridge instance (use
`fresh_bridge().await` or whichever async helper constructs
`LifeCaptureComposioBridge`) and call `name()` and `domains()` on that instance,
asserting `bridge.name() == "life_capture::composio_bridge"` and that
`bridge.domains()` contains the expected domain(s) (or equals the expected
slice), ensuring the test is async if needed and awaits `fresh_bridge()`; remove
the unused `Stub` impl and any unused imports.
| FieldSchema { | ||
| name: "metadata", | ||
| ty: TypeSchema::Option(Box::new(TypeSchema::String)), | ||
| comment: "Optional JSON object with source-specific metadata.", | ||
| required: false, | ||
| }, |
There was a problem hiding this comment.
metadata schema type misrepresents the actual contract.
The metadata field is declared as TypeSchema::Option(Box::new(TypeSchema::String)), but the handler at lines 254‑257 reads it as an arbitrary Value (defaulting to an empty Object), and the comment itself says "Optional JSON object". Clients introspecting the schema will be told to send a JSON string, but the handler will silently drop a quoted-string value into the index without parsing it (and conversely accepts objects/arrays the schema disallows). Either change the type to a JSON object/value variant or have the handler parse a JSON string.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/life_capture/schemas.rs` around lines 188 - 193, The schema
declares the FieldSchema for "metadata" as
TypeSchema::Option(Box::new(TypeSchema::String)) but the request handler reads
it as a serde_json::Value (defaulting to an empty Object), so change the schema
to reflect the actual contract: update the FieldSchema for "metadata" to
TypeSchema::Option(Box::new(TypeSchema::Value)) or a concrete JSON object
variant so clients know to send JSON, and ensure the handler still uses the
serde_json::Value path; alternatively, if you prefer to keep TypeSchema::String,
modify the handler code that reads "metadata" to parse the incoming string into
serde_json::Value (and validate errors) instead of accepting arbitrary objects —
update whichever of FieldSchema::metadata or the handler's parsing logic you
choose so schema and handler agree.
| #[allow(clippy::cast_possible_truncation)] | ||
| let k = args | ||
| .get("k") | ||
| .and_then(Value::as_u64) | ||
| .map_or(10, |v| v as usize); |
There was a problem hiding this comment.
k is not actually capped at 100, contradicting the schema and description.
The parameters_schema declares "maximum": 100 and the description tells the model "capped at 100", but the parsing on Lines 67-71 forwards whatever u64 the model sends straight to handle_search. Schemas are advisory for LLM tool callers — the runtime needs to enforce the cap, otherwise a stray k: 100000 makes the embedder + hybrid_search do unbounded work.
♻️ Proposed change
- #[allow(clippy::cast_possible_truncation)]
- let k = args
- .get("k")
- .and_then(Value::as_u64)
- .map_or(10, |v| v as usize);
+ let k = args
+ .get("k")
+ .and_then(Value::as_u64)
+ .map_or(10, |v| v.clamp(1, 100) as usize);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[allow(clippy::cast_possible_truncation)] | |
| let k = args | |
| .get("k") | |
| .and_then(Value::as_u64) | |
| .map_or(10, |v| v as usize); | |
| let k = args | |
| .get("k") | |
| .and_then(Value::as_u64) | |
| .map_or(10, |v| v.clamp(1, 100) as usize); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/openhuman/tools/impl/life_capture/search.rs` around lines 67 - 71, The
code reads k from args into variable k (via
args.get("k").and_then(Value::as_u64).map_or(10, |v| v as usize)) but does not
enforce the documented/schema cap of 100; update the parsing so k is clamped to
the range [1,100] before calling handle_search (e.g., read the u64, convert
safely, then apply min/max/clamp) to ensure the value respect the
parameters_schema and prevents unbounded work by handle_search/hybrid_search.
| { | ||
| "id": "q-src-01", | ||
| "text": "phoenix", | ||
| "kind": "mixed", | ||
| "k": 5, | ||
| "sources": ["calendar"], | ||
| "pending": true, | ||
| "pending_reason": "hybrid_search ignores Query.sources/since/until today; runner skips assertions until the implementation branch honors the filter.", | ||
| "must_contain_in_top": 5, | ||
| "must_contain": ["phoenix-kickoff-cal-01"], | ||
| "must_not_contain_in_top": 5, | ||
| "must_not_contain": ["phoenix-postmortem-01", "phoenix-retro-slack-01", "phoenix-budget-01", "phoenix-demo-push-imsg-01"] | ||
| }, |
There was a problem hiding this comment.
q-src-01 negative set is incomplete.
Once the source filter is implemented and the pending gate is dropped, this query will assert that calendar-only results exclude phoenix-postmortem-01, phoenix-retro-slack-01, phoenix-budget-01, and phoenix-demo-push-imsg-01 — but it omits phoenix-weather-noise-01 (gmail, subject "Weekend forecast: Phoenix, Arizona"), which is also a non-calendar Phoenix-bearing item. The assertion will pass even if the filter erroneously lets through that gmail row.
🛠️ Suggested fix
"must_not_contain_in_top": 5,
- "must_not_contain": ["phoenix-postmortem-01", "phoenix-retro-slack-01", "phoenix-budget-01", "phoenix-demo-push-imsg-01"]
+ "must_not_contain": ["phoenix-postmortem-01", "phoenix-retro-slack-01", "phoenix-budget-01", "phoenix-demo-push-imsg-01", "phoenix-weather-noise-01"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/fixtures/life_capture/corpus.json` around lines 320 - 332, The negative
set for the test case with id "q-src-01" is missing an additional non-calendar
Phoenix item; update the object for q-src-01 to add "phoenix-weather-noise-01"
to the must_not_contain array (and keep must_not_contain_in_top as-is) so the
query will correctly assert that calendar-only results exclude that Gmail item
once the pending gate is removed.
| // Stats — should show exactly one item. | ||
| let env_stats1 = post_rpc(&base, "openhuman.life_capture_get_stats", json!({})).await; | ||
| let stats1 = rpc_result_body(&env_stats1); | ||
| assert_eq!( | ||
| stats1["total_items"], | ||
| json!(1), | ||
| "expected total_items=1 after first ingest, got {stats1}" | ||
| ); | ||
|
|
There was a problem hiding this comment.
total_items == 1 assertion is brittle when the OnceCell index is shared across tests.
Both #[ignore]d tests in this binary share the same process-global PersonalIndex (per the file header). Cargo runs integration tests in alphabetical order by default — today ingest_then_search... happens to sort before search_response_shape..., so test 1 sees an empty index. But that's an implicit ordering contract: anyone who renames a test, adds a third one, or runs with a different harness can flip ordering and break this assert with no useful diagnostic ("expected 1, got 2"). Test 2's seed (shape-guard:2026-04-22) is exactly the kind of sibling state that makes this hazard real.
Recommend taking a baseline before the first ingest and asserting the delta, e.g.:
♻️ Proposed change
+ // Baseline — other tests in this binary may have seeded the shared index.
+ let env_stats0 = post_rpc(&base, "openhuman.life_capture_get_stats", json!({})).await;
+ let baseline = rpc_result_body(&env_stats0)["total_items"].as_i64().unwrap_or(0);
+
// First ingest — should INSERT.
let env1 = post_rpc(
/* … */
).await;
/* … */
- // Stats — should show exactly one item.
+ // Stats — should show exactly one new item over baseline.
let env_stats1 = post_rpc(&base, "openhuman.life_capture_get_stats", json!({})).await;
let stats1 = rpc_result_body(&env_stats1);
- assert_eq!(
- stats1["total_items"],
- json!(1),
- "expected total_items=1 after first ingest, got {stats1}"
- );
+ assert_eq!(
+ stats1["total_items"].as_i64().unwrap(),
+ baseline + 1,
+ "expected total_items=baseline+1 after first ingest, got {stats1}"
+ );The later "stats unchanged on re-ingest" check (Lines 217-220) is already delta-based and robust — this just brings the first assertion in line.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/imessage_ingest_e2e.rs` around lines 159 - 167, The assertion that
stats1["total_items"] == 1 is brittle due to shared process-global
PersonalIndex; before performing the first ingest, fetch a baseline with
post_rpc(&base, "openhuman.life_capture_get_stats", json!({})) (reuse
rpc_result_body to parse) and record baseline_total =
stats_baseline["total_items"], then after ingest compute delta =
stats1["total_items"] - baseline_total and assert delta == 1; update the test to
use these symbols (env_stats_baseline, stats_baseline, baseline_total,
env_stats1, stats1) so the test asserts a delta rather than an absolute total.
999231d to
e4c6a87
Compare
…xtures Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…uplicates - Introduce CuratedMemoryPromptSnapshot in prompt types so CI builds without the curated_memory domain still resolve PromptContext. - Use that type for ParentExecutionContext, Agent session storage, and PromptContext; map from curated_memory::MemorySnapshot when taking snapshots. - Remove duplicate curated_snapshot fields from bad merges (struct literals and PromptContext construction). - Fix learning tests PromptContext initializer. Co-authored-by: Cursor <cursoragent@cursor.com>
CI failed with E0063 after ParentExecutionContext gained curated_snapshot; triage escalation, session turn builder, subagent tests, and integration stubs were still missing the field. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
Adds the A2 piece of the v1 plan: a Composio event subscriber that ingests Gmail messages and Google Calendar events into the
life_capturepersonal index.src/openhuman/life_capture/ingest/composio.rsdefiningLifeCaptureComposioBridge+EventHandlerimpl on thecomposiodomainGMAIL_NEW_GMAIL_MESSAGE/GMAIL_NEW_MESSAGE/GOOGLECALENDAR_NEW_EVENT/GOOGLECALENDAR_EVENT_UPDATEDexternal_id(Gmail messageId / Calendar event id)pick_strfromcomposio::providers(no duplication)bootstrap_life_captureinsrc/core/jsonrpc.rs— existingregister_composio_trigger_subscriber()is untouchedOnceLockDependency
feat/life-capture-foundation). Diff includes A1 (be1112f9— life_capture ingest controller + iMessage scanner wiring) on top of the A2 work. Once #817 merges, rebase will drop A1 from this diff.Test plan
tests/composio_ingest_e2e.rspublishes on the real global bus (#[ignore]'d so CI default run is unaffected — usecargo test -- --ignoredfor e2e)cargo build --testscleancargo test --lib life_capture— 36 passedSummary by CodeRabbit
New Features
--source-onlymode and better asset validationDependencies
Documentation