fix(subconscious): seed defaults into per-user workspace + fix Intelligence page stale log#462
Conversation
The subconscious engine was only constructed lazily on the first engine-routed RPC (trigger, tasks_add, status). Because handle_tasks_list bypasses the engine and reads the store directly, a fresh install showed an empty Subconscious panel until the user clicked "Run now", even though SubconsciousEngine::new() seeds the 3 default system tasks on construction. Separately, HeartbeatEngine::run() — the periodic tick loop — was never spawned in production code. The only callers of HeartbeatEngine were tests, so ticks never fired automatically; users had to trigger each evaluation manually. Both issues are fixed together in run_server_inner, following the existing start_if_enabled pattern used by voice, screen_intelligence, and autocomplete: 1. Call get_or_init_engine() at startup to construct the SubconsciousEngine eagerly, which runs seed_default_tasks via from_heartbeat_config. Construction is idempotent via OnceLock; seeding is idempotent by title match, so repeat startups do not duplicate the defaults. 2. Construct HeartbeatEngine with the heartbeat config and workspace_dir, then tokio::spawn heartbeat.run() so the periodic tick loop runs for the process lifetime. The loop re-acquires the shared engine via get_or_init_engine() on each tick. Guarded by config.heartbeat.enabled so users who disable the heartbeat get neither startup seeding nor the background loop. Add engine_construction_seeds_default_tasks integration test that locks in the invariant: constructing SubconsciousEngine on a fresh workspace_dir must leave the 3 default system tasks in the store, with no tick, trigger, or explicit seed call. Also asserts that reconstructing the engine on the same workspace does not duplicate the defaults. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Default system tasks seeded at sidecar startup into the pre-login global workspace (`~/.openhuman/workspace/`) instead of the per-user workspace (`~/.openhuman/users/<id>/workspace/`) the UI reads from after login. The engine singleton is built lazily via `get_or_init_engine()` and cached in a `OnceLock`. `Config::load_or_init` resolves `workspace_dir` from `active_user.toml` — which does not exist until after login. When the engine was constructed on startup it therefore seeded into the global default, then the frozen singleton kept pointing at that path for the rest of the session while RPC handlers like `tasks_list` re-loaded config per call and read from the correct per-user path, silently returning an empty list. Fix: - `subconscious/global.rs`: add `bootstrap_after_login()` (idempotent via `BOOTSTRAPPED: AtomicBool`) which builds the engine against the now-correct per-user workspace and spawns the heartbeat loop. Track the heartbeat `JoinHandle` in a static so it can be aborted cleanly. Add `reset_engine_for_user_switch()` that aborts the heartbeat, clears the engine option, and resets the bootstrap flag. - `core/jsonrpc.rs`: replace the unconditional eager init on startup with a conditional one that only bootstraps if `active_user.toml` already exists (so a user logged in from a previous session still gets the engine up immediately after restart). - `credentials/ops.rs`: call `bootstrap_after_login()` at the end of `verify_and_store_session` so a fresh login triggers seeding against the per-user workspace. Call `reset_engine_for_user_switch()` in `clear_session` so logout tears down the engine + heartbeat loop and a subsequent login rebuilds them against the new user. Verified locally: sidecar restart with no `active_user.toml` logs "bootstrap deferred — waiting for login"; post-login logs "seeded 3 tasks on init" + "heartbeat periodic loop spawned"; and `subconscious.tasks_list` returns the 3 system defaults from the per-user DB. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two related fixes for the Intelligence page freezing on a stale
subconscious activity-log snapshot while ticks kept progressing in the
sidecar.
Root cause (backend): the subconscious RPC handlers were the only
outlier in the entire JSON-RPC surface that called the raw
`Config::load_or_init()` instead of the shared
`load_config_with_timeout()` wrapper that every other domain schemas.rs
uses (cron, webhooks, voice, team, skills, service, referral, doctor,
…). `load_or_init` constructs a fresh `SecretStore` and runs a chain
of `decrypt_optional_secret` calls on every invocation, which may IPC
to the OS keychain — slow, unbounded, no caching. Under the Intelligence
page's 3-second poll (4 parallel RPCs × ~7 keychain round-trips each =
~28 keychain calls every 3s), this pileup was enough to pin the
frontend's `Promise.all` past the poll interval.
Root cause (frontend): `useSubconscious.refresh()` uses `fetchingRef`
as an in-flight guard. The ref is only cleared inside the `finally`
block that runs after `Promise.all` settles. With no per-RPC timeout
on the client side either, a single slow backend call would leave the
ref stuck `true`, and every subsequent 3s `setInterval` tick would
silently early-return at the top of `refresh`. The poller kept firing,
but every call was a no-op — so the UI froze on whatever snapshot it
last successfully fetched, even though the backend was still ticking
through new decisions.
Backend fix (`src/openhuman/subconscious/schemas.rs`):
- Replace the local `load_config()` helper body to delegate to
`crate::openhuman::config::load_config_with_timeout()`. Matches the
28 other domain schemas.rs files and brings subconscious handlers
under the same 30s bound used everywhere else.
Frontend fix (`app/src/hooks/useSubconscious.ts`):
- Add a `withTimeout` helper (2.5s per-RPC, strictly less than the
3s poll interval) that races each of the 4 parallel RPCs against
a timeout and resolves `null` on timeout — matching the existing
`.catch(() => null)` contract so downstream setState logic is
unchanged.
- Clear `fetchingRef.current = false` in the useEffect cleanup so a
late-returning request or a React Strict Mode double-mount in dev
can't leave the ref stuck `true` for the next mount.
Defense in depth: the backend bound prevents a permanent hang and
matches repo conventions, while the frontend bound guarantees the 3s
poll loop can never be pinned beyond one tick regardless of
server-side latency. Verified locally — `cargo check` clean,
`tsc --noEmit` clean, all 18 pre-existing warnings in unrelated modules.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds post-login bootstrapping and teardown for the SubconsciousEngine, changes subconscious config loading, updates frontend hook RPC calls to use a timeout wrapper and polling cleanup, and adds an integration test verifying idempotent seeding of default tasks. Changes
Sequence Diagram(s)sequenceDiagram
participant Frontend as Frontend App
participant Hook as useSubconscious Hook
participant RPC as JSON-RPC Server
participant Engine as SubconsciousEngine
participant DB as Workspace DB
Frontend->>Hook: trigger refresh()
Hook->>RPC: subconsciousTasksList (withTimeout)
Hook->>RPC: subconsciousEscalationsList (withTimeout)
Hook->>RPC: subconsciousLogList (withTimeout)
Hook->>RPC: subconsciousStatus (withTimeout)
par concurrent RPCs
RPC->>Engine: request data
Engine->>DB: query store
DB-->>Engine: results
Engine-->>RPC: response
end
alt timeout or rejection
Hook-->>Hook: withTimeout resolves to null
else success
RPC-->>Hook: data responses
end
Hook->>Frontend: update state (if results present)
sequenceDiagram
participant User as User
participant Client as Client/UI
participant Session as Session Manager
participant Core as Core Server
participant SubEngine as SubconsciousEngine
participant DB as Workspace DB
User->>Client: login
Client->>Session: store_session()
Session->>DB: persist session
Session->>SubEngine: bootstrap_after_login()
alt heartbeat.enabled == true
SubEngine->>SubEngine: load config
SubEngine->>DB: check/seed tasks
SubEngine->>SubEngine: spawn heartbeat (store JoinHandle)
else heartbeat disabled
SubEngine-->>Session: log skipped
end
User->>Client: logout / switch
Client->>Session: clear_session()
Session->>SubEngine: reset_engine_for_user_switch()
SubEngine->>SubEngine: abort heartbeat JoinHandle
SubEngine->>SubEngine: clear engine & BOOTSTRAPPED
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/hooks/useSubconscious.ts (1)
224-229: Consider adding a timeout rejection path for observability.Currently, timeouts are silently swallowed like any other error. If you want to distinguish between RPC failures and timeouts for debugging, you could log or track timeouts separately. However, given the existing
.catch(() => null)contract (per learnings, intentional for resilience), this is fine as-is for the current use case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/hooks/useSubconscious.ts` around lines 224 - 229, The timeout currently resolves to null and is indistinguishable from other failures; introduce a distinct timeout rejection so timeouts can be observed: add a TimeoutError class (e.g., class TimeoutError extends Error) and change withTimeout to race the original promise against a timeout promise that rejects with new TimeoutError(), keeping RPC_TIMEOUT_MS as the default; update callers of withTimeout (if any rely on the null-only contract) to catch TimeoutError separately or map it to null while logging/tracking the timeout for observability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/core/jsonrpc.rs`:
- Around line 706-744: The formatting in the async startup block around
config.heartbeat.enabled and the chained calls to
crate::openhuman::config::default_root_openhuman_dir().ok().and_then(...) plus
the await for
crate::openhuman::subconscious::global::bootstrap_after_login().await and the
log::info!/log::warn! macro calls is misformatted and failing cargo fmt; run
cargo fmt (or rustfmt) and ensure the chained method calls are line-broken
consistently and the log macros use standard spacing/parentheses so the block
compiles and passes cargo fmt --check, keeping the logic around checking
config.heartbeat.enabled, already_logged_in, default_root_openhuman_dir,
read_active_user_id, and bootstrap_after_login unchanged.
---
Nitpick comments:
In `@app/src/hooks/useSubconscious.ts`:
- Around line 224-229: The timeout currently resolves to null and is
indistinguishable from other failures; introduce a distinct timeout rejection so
timeouts can be observed: add a TimeoutError class (e.g., class TimeoutError
extends Error) and change withTimeout to race the original promise against a
timeout promise that rejects with new TimeoutError(), keeping RPC_TIMEOUT_MS as
the default; update callers of withTimeout (if any rely on the null-only
contract) to catch TimeoutError separately or map it to null while
logging/tracking the timeout for observability.
🪄 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: bb125aec-749f-4999-a80b-e9988f790680
📒 Files selected for processing (6)
app/src/hooks/useSubconscious.tssrc/core/jsonrpc.rssrc/openhuman/credentials/ops.rssrc/openhuman/subconscious/global.rssrc/openhuman/subconscious/integration_test.rssrc/openhuman/subconscious/schemas.rs
CI ran `cargo fmt --all -- --check` and flagged the conditional bootstrap block in `run_server_inner` — `let already_logged_in` should fold onto one line, the `.and_then` closure body should inline, the `match ... .await` chain should fold, and the short log!() calls should not break across lines. No behavior change. Fixes three jobs on PR tinyhumansai#462 that were all failing at the same `cargo fmt --all -- --check` step (Rust Quality, Rust Tests, Type Check TypeScript — the last one chains cargo fmt after its prettier check). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
~/.openhuman/users/<id>/workspace/) instead of the pre-login global workspace (~/.openhuman/workspace/), so they actually show up in the UI after login.active_user.tomlexists (either at startup for an existing session or after a fresh login), and torn down on logout so an account switch rebuilds against the new user.load_config_with_timeoutwrapper (matching the 28 other domainschemas.rsfiles) instead of rawConfig::load_or_init, bounding the config load path at 30s.useSubconsciouspoll loop is guarded against wedging: each of the 4 parallel RPCs is wrapped in a 2.5s client-side timeout, and the in-flight ref is cleared on unmount.Problem
After logging in on a fresh install, the Intelligence page showed an empty subconscious task list, even though the backend reported
heartbeat.enabled = true. Investigation uncovered two independent bugs that combined to produce the symptom.1. Engine seeded into the wrong workspace.
get_or_init_engine()caches aSubconsciousEnginein aOnceLock, constructed lazily with whateverconfig.workspace_dirwas active at the moment of first call.Config::load_or_init()resolvesworkspace_dirfromactive_user.toml(seeconfig/schema/load.rs::resolve_runtime_config_dirs), which does not exist until after login. The eager init atcore/jsonrpc.rsstartup therefore fired against the pre-login global default, and the constructor'sseed_default_tasks()inserted the 3 system defaults into~/.openhuman/workspace/subconscious/subconscious.db. After login,active_user.tomlwas written and subsequent handler calls re-resolved to~/.openhuman/users/<id>/workspace/subconscious/subconscious.db— a fresh, empty DB. The cached engine kept ticking against the pre-login global DB while the handlers read from the per-user one, and the UI correctly returned[].Verified with sqlite3:
2. Intelligence page froze on a stale snapshot while the backend kept ticking. Even after the seeding was fixed, the activity log stopped updating in the UI while
curlshowed the backend processing new ticks. Two compounding issues:status,tasks_list,log_list,escalations_list) called rawConfig::load_or_init()at the top. That was the only outlier in the entire JSON-RPC surface — 28 other domainschemas.rsfiles use the sharedload_config_with_timeout()wrapper for a reason.load_or_initconstructs a freshSecretStoreper call and runs a chain ofdecrypt_optional_secretcalls that may IPC to the OS keychain. Under the 3s poll x 4 parallel RPCs x ~7 decrypts each, the unbounded load path can pile up.useSubconscious.refresh()usesfetchingRefas an in-flight guard, only cleared inside thefinallyblock of anawait Promise.all(...). With no per-RPC timeout on the client either, a single slow call would leave the ref stucktrue, and every subsequent 3ssetIntervaltick would silently early-return. The poller kept firing but every call was a no-op; the UI froze on whatever snapshot it last successfully fetched.Solution
Backend — defer engine bootstrap until login (
src/openhuman/subconscious/global.rs)bootstrap_after_login(): idempotent per-process via aBOOTSTRAPPED: AtomicBool. Loads config (now thatworkspace_dirresolves correctly), builds the engine (which runsseed_default_tasksagainst the per-user DB), and spawns the heartbeat loop. Tracks the heartbeatJoinHandlein a static slot so it can be aborted cleanly later — the previous baretokio::spawndetached the handle and made the loop uncancellable.reset_engine_for_user_switch(): aborts the heartbeat handle, clears the engineOptioninside theOnceLock, and resetsBOOTSTRAPPEDto false. Without this, logout leaves the cached engine pinned to the previous user'sworkspace_dirand the next login would tick against the wrong DB.Backend — startup path (
src/core/jsonrpc.rs)Replaced the unconditional eager init with a conditional one:
config.heartbeat.enabled == false-> same log as before, no change.read_active_user_id(default_root_openhuman_dir())— if present (user already logged in from a previous session), kick the bootstrap now so the heartbeat starts without waiting for re-authentication."bootstrap deferred — waiting for login"and exit the block. The login RPC will trigger it later.Backend — login + logout hooks (
src/openhuman/credentials/ops.rs)store_sessionnow callsbootstrap_after_login()aftersession stored, so a fresh login triggers seeding against the per-user workspace it just created. Bootstrap failures are non-fatal (session already stored, we only warn and push a log entry).clear_sessionnow callsreset_engine_for_user_switch()after clearingactive_user.toml, tearing down the engine + heartbeat so a subsequent login rebuilds them against whichever user signs in next.Backend — config load timeout (
src/openhuman/subconscious/schemas.rs)One-line change to the local
load_config()helper: delegate tocrate::openhuman::config::load_config_with_timeout()instead of callingConfig::load_or_init()directly. Brings the subconscious handlers in line with the 28 other domain schemas.rs files and puts the config load path under the shared 30s bound.Frontend — poll guard (
app/src/hooks/useSubconscious.ts)withTimeout<T>(promise, ms = 2500)helper that races each of the 4 parallel RPCs inrefresh()against a 2.5s timeout (strictly less than the 3s poll interval so slow calls cannot stack across ticks). Resolvesnullon timeout, matching the existing.catch(() => null)contract so downstream setState logic is unchanged.fetchingRef.current = falseon unmount so a late-returning request or a React Strict Mode double-mount in dev cannot leave the ref stucktruefor the next mount.Submission Checklist
src/openhuman/subconscious/integration_test.rs(seed_then_query_tasks,engine_construction_seeds_default_tasks,seed_default_tasks_creates_system_tasks) and those still pass. Recommended follow-up coverage:bootstrap_after_loginidempotency (second call is a no-op),reset_engine_for_user_switchclears both engine andBOOTSTRAPPED,useSubconsciousrefresh no longer wedges when one RPC times out.tests/json_rpc_e2e.rs: start the sidecar with noactive_user.toml, callsubconscious.tasks_listand expect[], writeactive_user.toml+ call the session-verify RPC, then callsubconscious.tasks_listagain and expect the 3 seeded tasks in the per-user DB. Flagging for follow-up.global.rshave module + function-level docs explaining the lifecycle gating (bootstrap_after_login,reset_engine_for_user_switch), theBOOTSTRAPPEDinvariant, and why theOnceLock<Mutex<Option<JoinHandle<()>>>>shape is necessary.jsonrpc.rsexplains why we bootstrap-on-restart ifactive_user.tomlalready exists; thecredentials/ops.rshook explains the non-fatal handling; theschemas.rshelper body explains why rawload_or_initis dangerous and links to the 28-file convention; theuseSubconscious.tstimeout wrapper explains the 2.5s < 3s poll invariant and the ref-clear on unmount.Impact
store_session(one extra idempotent bootstrap call) andclear_session(one reset call). The 30s backend bound is a ceiling, not a regression — under normal conditionsload_config_with_timeoutcompletes in well under 50ms. The 2.5s frontend bound ensures the poll loop is never pinned beyond a single interval regardless of backend latency.seed_default_tasks()is idempotent by title (store.rstestseed_default_tasks_creates_system_tasksconfirms second call returns 0). Users with existing custom tasks are untouched. Users who were previously on the buggy version and had the 3 defaults seeded into their pre-login global workspace will on next launch see them seeded into their per-user workspace instead; the stale global entries are left behind harmlessly.Related
bootstrap_after_loginidempotency, pre-login -> post-login transition E2E).SecretStore+ decrypted config in-process so repeatedload_or_initcalls do not re-do the keychain work every RPC. This is the deeper root cause of the poll pileup; the current PR bounds the symptom but does not eliminate the waste.interval_minutes: 5currently behaves as a minimum gap between ticks (sleep = 5 min + tick duration), so observed cycles run 9-12 minutes when tick work is heavy. That is a separate semantic / docs question worth its own thread.Generated with Claude Code (claude.com/claude-code)
Summary by CodeRabbit
New Features
Bug Fixes
Tests