Add agent task orchestration#1768
Merged
senamakel merged 24 commits intoMay 16, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds tests and helpers for threads task-board RPCs (registration, put/get roundtrips, validation), a WorkspaceEnvGuard for tests, and switches CI Appium installation to a per-run runner-local launcher; also increases Rust coverage job timeout. ChangesThreads task-board test coverage & CI tweaks
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/scripts/e2e-run-session.sh (1)
316-323:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUnconditional
--no-sandboxon Linux contradicts documented intent and disables sandbox for all Linux runs.The comment at lines 303-305 documents that
--no-sandboxis a workaround for running as root (uid 0 in the CI container), yet the code applies it unconditionally to all Linux environments. The comment itself states "Apply only on Linux" but makes no uid distinction, while the documented reason is specifically for root execution.This creates an unnecessary security gap: developers running this script locally as non-root on Linux will have Chromium's sandbox disabled without justification. Either make the flag conditional on
[ "$(id -u)" -eq 0 ], or update the comment to reflect that--no-sandboxis applied unconditionally to all Linux builds (and document why this is acceptable for dev/local use).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/scripts/e2e-run-session.sh` around lines 316 - 323, The script currently appends "--no-sandbox" unconditionally to APP_ARGS in the Linux case; change this to only add "--no-sandbox" when running as root by testing the UID (e.g., only append "--no-sandbox" to APP_ARGS if [ "$(id -u)" -eq 0 ]), otherwise omit the flag and keep the other Linux flags ("--disable-dev-shm-usage", "--disable-gpu", "--no-zygote") and update the echo to reflect the actual APP_ARGS; touch the APP_ARGS modification in the Linux case in e2e-run-session.sh where APP_ARGS is built so non-root local Linux runs retain the sandbox.
🧹 Nitpick comments (8)
src/openhuman/agent/profiles.rs (3)
464-481: ⚡ Quick winAdd diagnostics logging for profile upsert validation and operations.
The
handle_profile_upserthandler performs validation (checking agent definition existence) but lacks diagnostics logging. Per coding guidelines, new/changed flows in Rust core should include verbose diagnostics logging with stable prefixes.Add logging for:
- Entry point with profile ID being upserted
- Validation branch decisions (agent definition found/not found)
- Store operation results
Example:
fn handle_profile_upsert(params: Map<String, Value>) -> ControllerFuture { Box::pin(async move { let p = deserialize_params::<ProfileUpsertParams>(params)?; tracing::debug!( profile_id = %p.profile.id, agent_id = %p.profile.agent_id, "[rpc][agent] profile_upsert: validating" ); if let Some(registry) = crate::openhuman::agent::harness::AgentDefinitionRegistry::global() { let agent_id = p.profile.agent_id.trim(); if !agent_id.is_empty() && registry.get(agent_id).is_none() { tracing::warn!( profile_id = %p.profile.id, agent_id = %agent_id, "[rpc][agent] profile_upsert: agent definition not found" ); return Err(format!("agent definition '{agent_id}' not found")); } } // ... }) }As per coding guidelines: "Use
log/tracingatdebugortracelevel for development-oriented diagnostics on new/changed flows, including logs at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error handling paths."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/profiles.rs` around lines 464 - 481, The handler handle_profile_upsert needs verbose diagnostics: add tracing::debug at entry showing p.profile.id and p.profile.agent_id after deserializing ProfileUpsertParams, emit a tracing::debug/tracing::warn when the AgentDefinitionRegistry::global() check runs to indicate whether registry exists and whether the trimmed agent_id was found or missing, and log the result of the store operation (success or error) before returning; use stable log prefixes like "[rpc][agent] profile_upsert" and include the agent_id/profile_id fields in each log.
150-180: ⚡ Quick winAdd diagnostics logging for board normalization and implicit blocker population.
The
normalise_boardfunction performs several implicit transformations (ID generation, blocker population from notes when status is Blocked) but lacks diagnostics logging.Add logging for:
- Entry with card count
- Cards with auto-generated IDs
- Automatic blocker population from notes (lines 172-174) — this is a non-obvious business rule
- Cards filtered out due to empty titles
Example:
pub fn normalise_board(board: &mut TaskBoard) { tracing::trace!( thread_id = %board.thread_id, card_count = board.cards.len(), "[agent:task_board] normalise_board: start" ); // ... if card.status == TaskCardStatus::Blocked && card.blocker.is_none() { card.blocker = card.notes.clone(); tracing::trace!( card_id = %card.id, "[agent:task_board] normalise_board: populated blocker from notes" ); } // ... let filtered = board.cards.len(); board.cards.retain(|card| !card.title.is_empty()); if filtered != board.cards.len() { tracing::debug!( removed = filtered - board.cards.len(), "[agent:task_board] normalise_board: filtered empty-title cards" ); } }As per coding guidelines: "Use
log/tracingatdebugortracelevel for development-oriented diagnostics on new/changed flows, including logs at entry/exit points, branch decisions."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/profiles.rs` around lines 150 - 180, The normalise_board function lacks diagnostic logging for its implicit transformations; update the normalise_board(&mut TaskBoard) implementation to emit tracing/debug logs: add an entry trace with board.thread_id and initial card_count, log each card that receives an auto-generated id (reference card.id generation code), emit a trace when you populate card.blocker from card.notes for cards with TaskCardStatus::Blocked and no blocker (the implicit blocker population), and after retaining non-empty-title cards log a debug/trace showing how many cards were removed; use tracing::trace or tracing::debug per guidelines.
533-561: ⚡ Quick winAdd diagnostics logging for task board RPC handlers.
The new task board handlers (
handle_task_board_get,handle_task_board_put) lack diagnostics logging. Per coding guidelines, new/changed flows should include verbose diagnostics logging with stable prefixes.Add logging for:
- Entry points with thread_id
- Config loading
- Board operations (get/put)
- Success paths with board card counts
Example:
fn handle_task_board_get(params: Map<String, Value>) -> ControllerFuture { Box::pin(async move { let p = parse::<TaskBoardGetParams>(params)?; let thread_id = p.thread_id.trim(); tracing::debug!(thread_id = %thread_id, "[rpc][threads] task_board_get: fetching"); let config = crate::openhuman::config::Config::load_or_init() .await .map_err(|e| format!("load config: {e}"))?; let board = crate::openhuman::agent::task_board::board_for_thread( &config.workspace_dir, thread_id, )?; tracing::debug!( thread_id = %thread_id, card_count = board.cards.len(), "[rpc][threads] task_board_get: success" ); Ok(serde_json::json!({ "taskBoard": board })) }) }As per coding guidelines: "Use
log/tracingatdebugortracelevel for development-oriented diagnostics on new/changed flows, including logs at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error handling paths."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/agent/profiles.rs` around lines 533 - 561, Add verbose diagnostics logging to the task board RPC handlers: in handle_task_board_get and handle_task_board_put, log entry with the parsed thread_id (from TaskBoardGetParams / TaskBoardPutParams) at debug/trace, log when calling crate::openhuman::config::Config::load_or_init() and its result, log calls to crate::openhuman::agent::task_board::board_for_thread() (or the board save/put operation) and any branch decisions/errors, and log success with the board card count (board.cards.len()) and a stable prefix like "[rpc][threads] task_board_get" or "[rpc][threads] task_board_put"; include error context in error logs.app/src/services/chatService.ts (1)
646-655: 💤 Low valueConsider consistent handling of optional RPC parameters.
profile_idis sent asparams.profileId ?? undefined(line 653), whilemodel_overrideis sent directly asparams.model(line 652). Sincemodelis now optional,model_overridemay beundefined.If the backend distinguishes between an omitted field and an explicit
undefinedvalue, this could cause issues. Consider using the same pattern for both optional parameters, or omitting them entirely when not provided.♻️ Proposed consistency fix
await callCoreRpc({ method: 'openhuman.channel_web_chat', params: { client_id: clientId, thread_id: params.threadId, message: params.message, - model_override: params.model, - profile_id: params.profileId ?? undefined, + ...(params.model !== undefined && { model_override: params.model }), + ...(params.profileId !== undefined && params.profileId !== null && { profile_id: params.profileId }), }, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/services/chatService.ts` around lines 646 - 655, The RPC call to 'openhuman.channel_web_chat' inconsistently handles optional params: profile_id is sent as params.profileId ?? undefined while model_override is sent directly as params.model; make them consistent by either using the same ?? undefined pattern for params.model (i.e., model_override: params.model ?? undefined) or build the params object conditionally so keys are omitted when their values are undefined; update the callCoreRpc invocation in chatService.ts (the call that passes client_id, thread_id, message, model_override, profile_id) to follow the chosen consistent approach.app/src/pages/Conversations.tsx (3)
1323-1372: 💤 Low valueConsider adding a cancel button to the profile creation form.
The form can only be closed by clicking the "+" button again, which isn't immediately obvious to users. Adding an explicit "Cancel" button would improve discoverability.
✨ Optional: Add cancel button
<button type="button" onClick={() => void handleCreateAgentProfile()} disabled={!profileDraft.name.trim()} className="h-8 rounded-lg bg-primary-500 px-3 text-xs font-medium text-white transition-colors hover:bg-primary-600 disabled:opacity-40"> Save </button> + <button + type="button" + onClick={() => setProfileDraftOpen(false)} + className="h-8 rounded-lg border border-stone-200 bg-white px-3 text-xs font-medium text-stone-700 transition-colors hover:bg-stone-50"> + Cancel + </button> </div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/Conversations.tsx` around lines 1323 - 1372, Add an explicit "Cancel" button next to the Save button in the profile creation block that closes the draft and resets inputs; implement it to call the existing state setters used here (e.g. setProfileDraft(...) to restore the initial/default profile object and setProfileDraftOpen(false) to close the form) or call any existing close/reset helper (if you have a handler like handleCloseProfileDraft). Make it type="button", visually styled as a secondary action, and ensure it does not submit or call handleCreateAgentProfile.
307-339: ⚡ Quick winConsider validating profile name uniqueness.
The function validates that
nameis non-empty but doesn't check if a profile with the same name already exists inagentProfiles. This could lead to confusing duplicate entries in the dropdown.🔍 Optional: Add duplicate name check
const handleCreateAgentProfile = async () => { const name = profileDraft.name.trim(); if (!name) return; + if (agentProfiles.some(p => p.name === name)) { + console.warn('[conversations] profile name already exists:', name); + return; + } const id = `profile-${globalThis.crypto.randomUUID().slice(0, 8)}`;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/Conversations.tsx` around lines 307 - 339, handleCreateAgentProfile currently only checks non-empty names but can create duplicate profile names; before calling agentProfilesApi.upsert, check the existing agentProfiles array (referencing the agentProfiles state) for a name match (case-insensitive, trimmed) and if found, abort creation and surface a validation error (e.g., return early and set a user-visible error or toast) instead of proceeding to upsert; ensure the duplicate check uses the trimmed name variable and leave existing logic that constructs id, profile object, and calls agentProfilesApi.upsert, setAgentProfiles, setSelectedAgentProfileId, and resetting profileDraft unchanged when no duplicate is found.
1048-1069: ⚡ Quick winConsider user feedback on task board update failure.
The function implements optimistic updates with rollback on error, which is correct. However, when
putTaskBoardfails, the card silently reverts to its original position without notifying the user. Consider displaying a brief error message or toast so users understand why their action didn't persist.💡 Optional: Add error feedback
} catch (error) { console.warn('[conversations] putTaskBoard failed:', error); dispatch(setTaskBoardForThread({ threadId: selectedThreadId, board: selectedTaskBoard })); + setSendError(chatSendError('task_update_failed', 'Failed to update task board. Please try again.')); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/pages/Conversations.tsx` around lines 1048 - 1069, handleMoveTaskCard currently does an optimistic update and rollback on putTaskBoard failure but gives no user feedback; modify the catch block to surface an error notification (e.g., show a toast or set an error state) so users know the move failed. Specifically, in handleMoveTaskCard's catch where it calls console.warn and dispatches setTaskBoardForThread to revert, also call the app's notification/toast API (or dispatch a showError action) with a short message like "Could not move task — changes were not saved" and include minimal error details if available; keep the existing rollback dispatch to restore UI state.src/openhuman/tools/impl/agent/todo_write.rs (1)
174-223: ⚡ Quick winAdd diagnostic logging for successful task board persistence.
The new task board persistence path lacks debug logging on the success case. Per coding guidelines, new behaviors should include verbose diagnostics with grep-friendly prefixes and correlation fields.
📊 Proposed diagnostic logging
let saved = TaskBoardStore::new(parent.workspace_dir.clone()).put(board)?; + tracing::debug!( + thread_id = %thread_id, + card_count = saved.cards.len(), + "[todowrite] task board persisted" + ); if let Some(tx) = parent.on_progress {As per coding guidelines: "All new/changed behavior in Rust core must include verbose diagnostics logging with stable grep-friendly prefixes like
[domain],[rpc]and correlation fields."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/tools/impl/agent/todo_write.rs` around lines 174 - 223, The persist_thread_board function currently omits a success diagnostic log; add a verbose, grep-friendly debug/info log immediately after TaskBoardStore::new(parent.workspace_dir.clone()).put(board)? returns (before sending AgentProgress) that records a stable prefix like "[taskboard]" or "[rpc]" plus correlation fields thread_id, parent workspace_dir (or workspace id if available) and the saved board id/count (from saved.cards.len() or saved.thread_id) so operators can trace persistence events; keep the log level as debug/info per project convention and ensure the message appears on the success path in persist_thread_board.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/services/api/agentProfilesApi.ts`:
- Around line 8-13: The unwrapEnvelope function currently returns (response as
Envelope<T>).data without guarding against data being undefined; update
unwrapEnvelope to check that response is an object with a 'data' property and
that (response as Envelope<T>).data !== undefined, and if data is undefined
either throw a descriptive error or return a sensible default/handle it
according to the callers' expectations (e.g., throw new Error('Envelope.data is
undefined in unwrapEnvelope') to preserve the T non-nullable contract);
reference the function name unwrapEnvelope and the Envelope type and its data
property when making the change.
In `@app/src/services/api/threadApi.ts`:
- Around line 142-158: getTaskBoard and putTaskBoard currently access
data.taskBoard directly and can throw/return undefined if the RPC envelope is
empty; update both to mirror getTurnState by using optional chaining on
unwrapEnvelope(response) and explicitly returning null when taskBoard is
missing, and update the functions' return types from Promise<TaskBoard> to
Promise<TaskBoard | null> (also adjust the callCoreRpc generic if needed) so
callers get a safe nullable result instead of an unsafe undefined.
In `@app/src/store/coreModeSlice.ts`:
- Around line 54-56: Replace the direct import.meta.env usage by re-exporting
the VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE value from your central config and
consuming that export in the slice: add an exported constant (e.g. export const
VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE =
import.meta.env.VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE) to
app/src/utils/config.ts, then import that constant into coreModeSlice (where the
current if checks import.meta.env.VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE) and use
it in the conditional to return { kind: 'local' } when it equals 'local'; keep
the exported name identical so callers need only change the import.
In `@app/test/e2e/helpers/shared-flows.ts`:
- Around line 251-255: Replace the single-pass marker check that sets finalCheck
by iterating billingMarkers and calling textExists with a retry loop: instead
implement the same polling pattern used elsewhere in this file (the retry+delay
loop that repeatedly checks markers) so the code repeatedly calls
textExists(marker) with short delays until a marker is found or timeout; update
the block that currently uses billingMarkers, finalCheck, and textExists to
perform polling per marker and break when a marker is detected to avoid
intermittent failures on slow WebView renders.
In `@app/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 209-217: Add the same failure-diagnostic pattern used earlier to
the billing handoff test: after navigateToBilling() and clearRequestLog(), call
the diagnostic helpers (dumpRequestLog() or print the request log and
dumpAccessibilityTree()) so you capture network/request context and the
accessibility tree before asserting textExists('Open dashboard') and
textExists('TinyHumans on the web'); if preferred wrap the two expect calls in a
try/catch that logs dumpRequestLog() and await dumpAccessibilityTree() on
failure to ensure useful diagnostics are emitted. Apply the same change to the
other billing tests referenced (the blocks around lines 224–238 and 240–254) so
each billing assertion emits request logs and dumpAccessibilityTree() on
failure.
In `@src/openhuman/agent/harness/session/builder.rs`:
- Around line 846-853: When adding the AgentProfilePromptSection to the
prompt_builder (the block that checks profile_prompt_suffix and calls
prompt_builder.add_section and AgentProfilePromptSection::new), insert a
debug-level diagnostic log (e.g., using tracing::debug! or the project's logging
macro) immediately before or after the add_section call indicating the profile
prompt section was injected; use a stable grep-friendly prefix like "[agent]"
and include correlation/context fields (request id, session id, or similar
available in scope) plus a trimmed indicator of the suffix (not the full content
if sensitive) so operators can trace profile activation in logs.
In `@src/openhuman/agent/profiles.rs`:
- Around line 81-125: The task-board persistence functions need verbose
diagnostics with a stable "[agent:task_board]" prefix: add
tracing::debug!/trace! logs at entry and exit of get, put, and delete (include
thread_id), at file existence checks (board_path() results - "board not
found"/"board loaded"), around normalization (e.g., normalise_cards/normalise
where cards length is available: "normalised {} cards"), and in all error paths
(include error text); ensure logs use key-value fields like thread_id=%thread_id
and card_count=board.cards.len() so they are grep-friendly and follow the
debug/trace levels per guidelines.
- Around line 55-181: Add tracing debug/trace diagnostics with a stable
"[agent:profiles]" prefix to all public methods (load, save, select, upsert,
delete, resolve) and key internal decisions: log entry/exit for each function
(including self.path() for load/save), branch outcomes like "profile not found",
"built-in merge" (when using built_in_default_profile or built_in_profiles),
"active profile fallback" (when resolving to DEFAULT_PROFILE_ID), state
transitions such as "upserting profile", "profile deleted", "active profile
changed", and all error-return paths (wrap error messages with
tracing::debug!/trace! before returning Err). Ensure messages reference the
profile id or state.profiles.len() where relevant and keep the prefix exactly
"[agent:profiles]" for grepability; use debug level for routine flows and trace
for very verbose internals.
In `@src/openhuman/agent/schemas.rs`:
- Around line 441-493: Add verbose RPC diagnostics to the four profile handler
functions (handle_profiles_list, handle_profile_select, handle_profile_upsert,
handle_profile_delete): at entry log a stable prefix like "[rpc][agent][entry]"
with the RPC name and correlation id (generate or accept a request id from
params), on successful exit log "[rpc][agent][exit]" with result summary (e.g.,
number of profiles and activeProfileId) and on errors log "[rpc][agent][error]"
with the error message and the same correlation id; ensure logs are emitted via
the project logger used elsewhere (matching existing diagnostics style) and
include the agent_id or profile_id where relevant to make tracing grep-friendly.
In `@src/openhuman/agent/task_board.rs`:
- Around line 134-140: Validate thread_id is not empty or only whitespace before
constructing file paths: in board_path use if thread_id.trim().is_empty() {
panic!("invalid thread_id: empty or whitespace") } (or return an Err in
functions that return Result) and apply the same check to the other
thread_id-taking methods referenced around lines 143–148 (e.g.,
read_board/write_board/delete_board or similarly named functions) so you trim
the input, reject blank IDs with a clear error message, and only then call
hex::encode and build the PathBuf.
- Around line 81-125: Add tracing/log calls to the TaskBoardStore flows to
record branches and errors: instrument get, put, and delete to emit debug/trace
events with a stable prefix (e.g., "task_board_store"), include correlation
fields like thread_id (board.thread_id or thread_id arg), op
("get"/"put"/"delete"), and outcome ("not_found","ok","error") and include the
error message on failures; place logs at key branches in get (path missing,
parse error, success), in put (ensure_dir success/failure, tempfile created,
fsync/persist success/failure, serialization error) and in delete (path missing,
remove_file success/failure), using the existing helpers board_path, ensure_dir,
normalise_board to source the thread id and path for the log fields.
- Around line 155-180: The loop sets card.order before you call
board.cards.retain(...), which leaves gaps when cards with empty titles are
removed; change the flow so you first trim fields and remove empty-title cards
(keep the trimming logic on each card as currently done), then iterate over the
filtered board.cards with enumerate to assign contiguous order values and update
updated_at — e.g., trim and normalize fields in the initial pass, call
board.cards.retain(|card| !card.title.is_empty()), then for (idx, card) in
board.cards.iter_mut().enumerate() set card.order = idx as u32 and
card.updated_at = now.clone(); ensure the blocker fallback logic
(TaskCardStatus::Blocked -> card.blocker = card.notes.clone()) still runs before
the retain so notes/blocker normalization is preserved.
In `@src/openhuman/channels/providers/web.rs`:
- Around line 1193-1204: Add a diagnostic log when handling
AgentProgress::TaskBoardUpdated before calling publish_web_channel_event: log at
debug/trace level with a stable grep-friendly prefix (e.g.
"web:event:task_board_updated") and include correlation fields client_id,
thread_id, request_id and a short serialized summary of the task_board (use
serde_json::to_string or to_value). Update the TaskBoardUpdated branch where
publish_web_channel_event(WebChannelEvent { ... }) is invoked to emit this log
entry (referencing AgentProgress::TaskBoardUpdated, publish_web_channel_event,
and WebChannelEvent) so logs provide consistent diagnostics for this new
behavior.
In `@src/openhuman/threads/schemas.rs`:
- Around line 533-560: Add verbose RPC diagnostics to the task board handlers by
instrumenting handle_task_board_get and handle_task_board_put with grep-friendly
log messages (e.g. "[rpc][task_board]") on entry and exit, key branches, and
error paths; include correlation fields like thread_id and any error details,
and use appropriate levels (trace/debug for entry/exit and branch decisions,
error for failures) so that load_or_init failures, board resolution
(board_for_thread), TaskBoardStore::put, and successful responses are all logged
with stable prefixes and contextual data.
- Around line 381-397: The ControllerSchema for "task_board_get" declares an
output field named "result" but the handler returns a JSON object keyed
"taskBoard"; update the outputs Vec for ControllerSchema entries that correspond
to task-board handlers (e.g., the ControllerSchema with function
"task_board_get") to use FieldSchema { name: "taskBoard", ty: TypeSchema::Json,
comment: "Task board payload.", required: true } (keep TypeSchema::Json and the
comment), and apply the same change to the other ControllerSchema entries that
return { "taskBoard": ... } so the schema key matches the actual handler
payload.
In `@src/openhuman/tools/impl/agent/spawn_parallel_agents.rs`:
- Around line 108-309: The execute method in spawn_parallel_agents.rs lacks the
required verbose diagnostic logging; add debug/info/error logs with stable
grep-friendly prefixes (e.g. "[domain]", "[rpc]") at the start and end of
execute and around each major branch/validation (missing tasks, tasks.len()<2,
outside agent turn, max_parallel check, registry missing, invalid agent id,
integrations_agent toolkit missing), before/after publish_global calls
(SubagentSpawned/SubagentCompleted/SubagentFailed), on each progress_sink send,
when building prepared tasks, and after join_all with per-task outcomes; ensure
logs include correlation fields (parent_session, task_id, agent_id, mode),
counts (prompt_chars, elapsed_ms, iterations), and any captured error details so
failures are logged with context, and use the project logging facility (e.g.,
tracing::debug/info/error) consistently with the required prefixes.
---
Outside diff comments:
In `@app/scripts/e2e-run-session.sh`:
- Around line 316-323: The script currently appends "--no-sandbox"
unconditionally to APP_ARGS in the Linux case; change this to only add
"--no-sandbox" when running as root by testing the UID (e.g., only append
"--no-sandbox" to APP_ARGS if [ "$(id -u)" -eq 0 ]), otherwise omit the flag and
keep the other Linux flags ("--disable-dev-shm-usage", "--disable-gpu",
"--no-zygote") and update the echo to reflect the actual APP_ARGS; touch the
APP_ARGS modification in the Linux case in e2e-run-session.sh where APP_ARGS is
built so non-root local Linux runs retain the sandbox.
---
Nitpick comments:
In `@app/src/pages/Conversations.tsx`:
- Around line 1323-1372: Add an explicit "Cancel" button next to the Save button
in the profile creation block that closes the draft and resets inputs; implement
it to call the existing state setters used here (e.g. setProfileDraft(...) to
restore the initial/default profile object and setProfileDraftOpen(false) to
close the form) or call any existing close/reset helper (if you have a handler
like handleCloseProfileDraft). Make it type="button", visually styled as a
secondary action, and ensure it does not submit or call
handleCreateAgentProfile.
- Around line 307-339: handleCreateAgentProfile currently only checks non-empty
names but can create duplicate profile names; before calling
agentProfilesApi.upsert, check the existing agentProfiles array (referencing the
agentProfiles state) for a name match (case-insensitive, trimmed) and if found,
abort creation and surface a validation error (e.g., return early and set a
user-visible error or toast) instead of proceeding to upsert; ensure the
duplicate check uses the trimmed name variable and leave existing logic that
constructs id, profile object, and calls agentProfilesApi.upsert,
setAgentProfiles, setSelectedAgentProfileId, and resetting profileDraft
unchanged when no duplicate is found.
- Around line 1048-1069: handleMoveTaskCard currently does an optimistic update
and rollback on putTaskBoard failure but gives no user feedback; modify the
catch block to surface an error notification (e.g., show a toast or set an error
state) so users know the move failed. Specifically, in handleMoveTaskCard's
catch where it calls console.warn and dispatches setTaskBoardForThread to
revert, also call the app's notification/toast API (or dispatch a showError
action) with a short message like "Could not move task — changes were not saved"
and include minimal error details if available; keep the existing rollback
dispatch to restore UI state.
In `@app/src/services/chatService.ts`:
- Around line 646-655: The RPC call to 'openhuman.channel_web_chat'
inconsistently handles optional params: profile_id is sent as params.profileId
?? undefined while model_override is sent directly as params.model; make them
consistent by either using the same ?? undefined pattern for params.model (i.e.,
model_override: params.model ?? undefined) or build the params object
conditionally so keys are omitted when their values are undefined; update the
callCoreRpc invocation in chatService.ts (the call that passes client_id,
thread_id, message, model_override, profile_id) to follow the chosen consistent
approach.
In `@src/openhuman/agent/profiles.rs`:
- Around line 464-481: The handler handle_profile_upsert needs verbose
diagnostics: add tracing::debug at entry showing p.profile.id and
p.profile.agent_id after deserializing ProfileUpsertParams, emit a
tracing::debug/tracing::warn when the AgentDefinitionRegistry::global() check
runs to indicate whether registry exists and whether the trimmed agent_id was
found or missing, and log the result of the store operation (success or error)
before returning; use stable log prefixes like "[rpc][agent] profile_upsert" and
include the agent_id/profile_id fields in each log.
- Around line 150-180: The normalise_board function lacks diagnostic logging for
its implicit transformations; update the normalise_board(&mut TaskBoard)
implementation to emit tracing/debug logs: add an entry trace with
board.thread_id and initial card_count, log each card that receives an
auto-generated id (reference card.id generation code), emit a trace when you
populate card.blocker from card.notes for cards with TaskCardStatus::Blocked and
no blocker (the implicit blocker population), and after retaining
non-empty-title cards log a debug/trace showing how many cards were removed; use
tracing::trace or tracing::debug per guidelines.
- Around line 533-561: Add verbose diagnostics logging to the task board RPC
handlers: in handle_task_board_get and handle_task_board_put, log entry with the
parsed thread_id (from TaskBoardGetParams / TaskBoardPutParams) at debug/trace,
log when calling crate::openhuman::config::Config::load_or_init() and its
result, log calls to crate::openhuman::agent::task_board::board_for_thread() (or
the board save/put operation) and any branch decisions/errors, and log success
with the board card count (board.cards.len()) and a stable prefix like
"[rpc][threads] task_board_get" or "[rpc][threads] task_board_put"; include
error context in error logs.
In `@src/openhuman/tools/impl/agent/todo_write.rs`:
- Around line 174-223: The persist_thread_board function currently omits a
success diagnostic log; add a verbose, grep-friendly debug/info log immediately
after TaskBoardStore::new(parent.workspace_dir.clone()).put(board)? returns
(before sending AgentProgress) that records a stable prefix like "[taskboard]"
or "[rpc]" plus correlation fields thread_id, parent workspace_dir (or workspace
id if available) and the saved board id/count (from saved.cards.len() or
saved.thread_id) so operators can trace persistence events; keep the log level
as debug/info per project convention and ensure the message appears on the
success path in persist_thread_board.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 82a254dd-39c9-44d9-8411-eef082a464f6
📒 Files selected for processing (51)
app/.prettierignoreapp/scripts/e2e-build.shapp/scripts/e2e-run-session.shapp/src/pages/Conversations.tsxapp/src/pages/__tests__/Conversations.render.test.tsxapp/src/pages/conversations/components/TaskKanbanBoard.tsxapp/src/pages/conversations/components/__tests__/TaskKanbanBoard.test.tsxapp/src/providers/ChatRuntimeProvider.tsxapp/src/services/__tests__/chatService.test.tsapp/src/services/api/agentProfilesApi.test.tsapp/src/services/api/agentProfilesApi.tsapp/src/services/api/threadApi.test.tsapp/src/services/api/threadApi.tsapp/src/services/chatService.tsapp/src/store/__tests__/chatRuntimeSlice.test.tsapp/src/store/chatRuntimeSlice.tsapp/src/store/coreModeSlice.tsapp/src/types/agentProfile.tsapp/src/types/turnState.tsapp/src/utils/config.tsapp/src/utils/tauriCommands/core.test.tsapp/src/utils/tauriCommands/core.tsapp/src/vite-env.d.tsapp/test/e2e/helpers/shared-flows.tsapp/test/e2e/specs/auth-access-control.spec.tsapp/test/e2e/specs/login-flow.spec.tsscripts/mock-api/routes/integrations.mjssrc/api/config.rssrc/core/socketio.rssrc/openhuman/agent/agents/orchestrator/agent.tomlsrc/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/mod.rssrc/openhuman/agent/profiles.rssrc/openhuman/agent/progress.rssrc/openhuman/agent/schemas.rssrc/openhuman/agent/task_board.rssrc/openhuman/channels/bus.rssrc/openhuman/channels/proactive.rssrc/openhuman/channels/providers/presentation.rssrc/openhuman/channels/providers/web.rssrc/openhuman/channels/providers/web_tests.rssrc/openhuman/screen_intelligence/tests.rssrc/openhuman/threads/schemas.rssrc/openhuman/threads/schemas_tests.rssrc/openhuman/threads/turn_state/mirror.rssrc/openhuman/threads/turn_state/types.rssrc/openhuman/tools/impl/agent/mod.rssrc/openhuman/tools/impl/agent/spawn_parallel_agents.rssrc/openhuman/tools/impl/agent/todo_write.rssrc/openhuman/tools/ops.rssrc/openhuman/tools/ops_tests.rs
|
Nie pisz do mnie
---------- Forwarded message ---------
Od: coderabbitai[bot] ***@***.***>
Date: czw., 14 maj 2026, 23:15
Subject: Re: [tinyhumansai/openhuman] Add agent task orchestration (PR
#1768)
To: tinyhumansai/openhuman ***@***.***>
Cc: Subscribed ***@***.***>
***@***.***[bot]* requested changes on this pull request.
*Actionable comments posted: 16*
Caution
Some comments are outside the diff and can’t be posted inline due to
platform limitations.
------------------------------
150-180: *⚡ Quick win*
*Add diagnostics logging for board normalization and implicit blocker
population.*
The normalise_board function performs several implicit transformations (ID
generation, blocker population from notes when status is Blocked) but lacks
diagnostics logging.
Add logging for:
- Entry with card count
- Cards with auto-generated IDs
- Automatic blocker population from notes (lines 172-174) — this is a
non-obvious business rule
- Cards filtered out due to empty titles
Example:
pub fn normalise_board(board: &mut TaskBoard) {
tracing::trace!(
thread_id = %board.thread_id,
card_count = board.cards.len(),
"[agent:task_board] normalise_board: start"
);
// ...
if card.status == TaskCardStatus::Blocked && card.blocker.is_none() {
card.blocker = card.notes.clone();
tracing::trace!(
card_id = %card.id,
"[agent:task_board] normalise_board: populated blocker from notes"
);
}
// ...
let filtered = board.cards.len();
board.cards.retain(|card| !card.title.is_empty());
if filtered != board.cards.len() {
tracing::debug!(
removed = filtered - board.cards.len(),
"[agent:task_board] normalise_board: filtered empty-title cards"
);
}}
As per coding guidelines: "Use log / tracing at debug or trace level for
development-oriented diagnostics on new/changed flows, including logs at
entry/exit points, branch decisions."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In ***@***.***/openhuman/agent/profiles.rs` around lines 150 - 180, The
normalise_board
function lacks diagnostic logging for its implicit transformations; update the
normalise_board(&mut TaskBoard) implementation to emit tracing/debug logs: add
an entry trace with board.thread_id and initial card_count, log each card that
receives an auto-generated id (reference card.id generation code), emit a trace
when you populate card.blocker from card.notes for cards with
TaskCardStatus::Blocked and no blocker (the implicit blocker population), and
after retaining non-empty-title cards log a debug/trace showing how many cards
were removed; use tracing::trace or tracing::debug per guidelines.
------------------------------
533-561: *⚡ Quick win*
*Add diagnostics logging for task board RPC handlers.*
The new task board handlers (handle_task_board_get, handle_task_board_put)
lack diagnostics logging. Per coding guidelines, new/changed flows should
include verbose diagnostics logging with stable prefixes.
Add logging for:
- Entry points with thread_id
- Config loading
- Board operations (get/put)
- Success paths with board card counts
Example:
fn handle_task_board_get(params: Map<String, Value>) -> ControllerFuture {
Box::pin(async move {
let p = parse::<TaskBoardGetParams>(params)?;
let thread_id = p.thread_id.trim();
tracing::debug!(thread_id = %thread_id, "[rpc][threads]
task_board_get: fetching");
let config = crate::openhuman::config::Config::load_or_init()
.await
.map_err(|e| format!("load config: {e}"))?;
let board = crate::openhuman::agent::task_board::board_for_thread(
&config.workspace_dir,
thread_id,
)?;
tracing::debug!(
thread_id = %thread_id,
card_count = board.cards.len(),
"[rpc][threads] task_board_get: success"
);
Ok(serde_json::json!({ "taskBoard": board }))
})}
As per coding guidelines: "Use log / tracing at debug or trace level for
development-oriented diagnostics on new/changed flows, including logs at
entry/exit points, branch decisions, external calls, retries/timeouts,
state transitions, and error handling paths."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In ***@***.***/openhuman/agent/profiles.rs` around lines 533 - 561, Add verbose
diagnostics logging to the task board RPC handlers: in handle_task_board_get and
handle_task_board_put, log entry with the parsed thread_id (from
TaskBoardGetParams / TaskBoardPutParams) at debug/trace, log when calling
crate::openhuman::config::Config::load_or_init() and its result, log calls to
crate::openhuman::agent::task_board::board_for_thread() (or the board save/put
operation) and any branch decisions/errors, and log success with the board card
count (board.cards.len()) and a stable prefix like "[rpc][threads]
task_board_get" or "[rpc][threads] task_board_put"; include error context in
error logs.
app/src/services/chatService.ts (1)
646-655: *💤 Low value*
*Consider consistent handling of optional RPC parameters.*
profile_id is sent as params.profileId ?? undefined (line 653), while
model_override is sent directly as params.model (line 652). Since model is
now optional, model_override may be undefined.
If the backend distinguishes between an omitted field and an explicit
undefined value, this could cause issues. Consider using the same pattern
for both optional parameters, or omitting them entirely when not provided.
♻️ Proposed consistency fix
await callCoreRpc({
method: 'openhuman.channel_web_chat',
params: {
client_id: clientId,
thread_id: params.threadId,
message: params.message,- model_override: params.model,-
profile_id: params.profileId ?? undefined,+ ...(params.model
!== undefined && { model_override: params.model }),+
...(params.profileId !== undefined && params.profileId !== null && {
profile_id: params.profileId }),
},
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In ***@***.***/src/services/chatService.ts` around lines 646 - 655, The RPC call to
'openhuman.channel_web_chat' inconsistently handles optional params: profile_id
is sent as params.profileId ?? undefined while model_override is sent directly
as params.model; make them consistent by either using the same ?? undefined
pattern for params.model (i.e., model_override: params.model ?? undefined) or
build the params object conditionally so keys are omitted when their values are
undefined; update the callCoreRpc invocation in chatService.ts (the call that
passes client_id, thread_id, message, model_override, profile_id) to follow the
chosen consistent approach.
app/src/pages/Conversations.tsx (3)
1323-1372: *💤 Low value*
*Consider adding a cancel button to the profile creation form.*
The form can only be closed by clicking the "+" button again, which isn't
immediately obvious to users. Adding an explicit "Cancel" button would
improve discoverability.
✨ Optional: Add cancel button
<button
type="button"
onClick={() => void handleCreateAgentProfile()}
disabled={!profileDraft.name.trim()}
className="h-8 rounded-lg bg-primary-500 px-3 text-xs
font-medium text-white transition-colors hover:bg-primary-600
disabled:opacity-40">
Save
</button>+ <button+
type="button"+ onClick={() =>
setProfileDraftOpen(false)}+ className="h-8 rounded-lg
border border-stone-200 bg-white px-3 text-xs font-medium
text-stone-700 transition-colors hover:bg-stone-50">+
Cancel+ </button>
</div>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In ***@***.***/src/pages/Conversations.tsx` around lines 1323 - 1372, Add an explicit
"Cancel" button next to the Save button in the profile creation block that
closes the draft and resets inputs; implement it to call the existing state
setters used here (e.g. setProfileDraft(...) to restore the initial/default
profile object and setProfileDraftOpen(false) to close the form) or call any
existing close/reset helper (if you have a handler like
handleCloseProfileDraft). Make it type="button", visually styled as a secondary
action, and ensure it does not submit or call handleCreateAgentProfile.
------------------------------
307-339: *⚡ Quick win*
*Consider validating profile name uniqueness.*
The function validates that name is non-empty but doesn't check if a
profile with the same name already exists in agentProfiles. This could lead
to confusing duplicate entries in the dropdown.
🔍 Optional: Add duplicate name check
const handleCreateAgentProfile = async () => {
const name = profileDraft.name.trim();
if (!name) return;+ if (agentProfiles.some(p => p.name === name))
{+ console.warn('[conversations] profile name already exists:',
name);+ return;+ }
const id = `profile-${globalThis.crypto.randomUUID().slice(0, 8)}`;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In ***@***.***/src/pages/Conversations.tsx` around lines 307 - 339,
handleCreateAgentProfile currently only checks non-empty names but can create
duplicate profile names; before calling agentProfilesApi.upsert, check the
existing agentProfiles array (referencing the agentProfiles state) for a name
match (case-insensitive, trimmed) and if found, abort creation and surface a
validation error (e.g., return early and set a user-visible error or toast)
instead of proceeding to upsert; ensure the duplicate check uses the trimmed
name variable and leave existing logic that constructs id, profile object, and
calls agentProfilesApi.upsert, setAgentProfiles, setSelectedAgentProfileId, and
resetting profileDraft unchanged when no duplicate is found.
------------------------------
1048-1069: *⚡ Quick win*
*Consider user feedback on task board update failure.*
The function implements optimistic updates with rollback on error, which is
correct. However, when putTaskBoard fails, the card silently reverts to its
original position without notifying the user. Consider displaying a brief
error message or toast so users understand why their action didn't persist.
💡 Optional: Add error feedback
} catch (error) {
console.warn('[conversations] putTaskBoard failed:', error);
dispatch(setTaskBoardForThread({ threadId: selectedThreadId,
board: selectedTaskBoard }));+
setSendError(chatSendError('task_update_failed', 'Failed to update
task board. Please try again.'));
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In ***@***.***/src/pages/Conversations.tsx` around lines 1048 - 1069,
handleMoveTaskCard
currently does an optimistic update and rollback on putTaskBoard failure but
gives no user feedback; modify the catch block to surface an error notification
(e.g., show a toast or set an error state) so users know the move failed.
Specifically, in handleMoveTaskCard's catch where it calls console.warn and
dispatches setTaskBoardForThread to revert, also call the app's
notification/toast API (or dispatch a showError action) with a short message
like "Could not move task — changes were not saved" and include minimal error
details if available; keep the existing rollback dispatch to restore UI state.
src/openhuman/tools/impl/agent/todo_write.rs (1)
174-223: *⚡ Quick win*
*Add diagnostic logging for successful task board persistence.*
The new task board persistence path lacks debug logging on the success
case. Per coding guidelines, new behaviors should include verbose
diagnostics with grep-friendly prefixes and correlation fields.
📊 Proposed diagnostic logging
let saved =
TaskBoardStore::new(parent.workspace_dir.clone()).put(board)?;+
tracing::debug!(+ thread_id = %thread_id,+ card_count =
saved.cards.len(),+ "[todowrite] task board persisted"+ );
if let Some(tx) = parent.on_progress {
As per coding guidelines: "All new/changed behavior in Rust core must
include verbose diagnostics logging with stable grep-friendly prefixes like
[domain], [rpc] and correlation fields."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In ***@***.***/openhuman/tools/impl/agent/todo_write.rs` around lines 174 - 223, The
persist_thread_board function currently omits a success diagnostic log; add a
verbose, grep-friendly debug/info log immediately after
TaskBoardStore::new(parent.workspace_dir.clone()).put(board)? returns (before
sending AgentProgress) that records a stable prefix like "[taskboard]" or
"[rpc]" plus correlation fields thread_id, parent workspace_dir (or workspace id
if available) and the saved board id/count (from saved.cards.len() or
saved.thread_id) so operators can trace persistence events; keep the log level
as debug/info per project convention and ensure the message appears on the
success path in persist_thread_board.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In ***@***.***/src/services/api/agentProfilesApi.ts`:
- Around line 8-13: The unwrapEnvelope function currently returns (response as
Envelope<T>).data without guarding against data being undefined; update
unwrapEnvelope to check that response is an object with a 'data' property and
that (response as Envelope<T>).data !== undefined, and if data is undefined
either throw a descriptive error or return a sensible default/handle it
according to the callers' expectations (e.g., throw new Error('Envelope.data is
undefined in unwrapEnvelope') to preserve the T non-nullable contract);
reference the function name unwrapEnvelope and the Envelope type and its data
property when making the change.
In ***@***.***/src/services/api/threadApi.ts`:
- Around line 142-158: getTaskBoard and putTaskBoard currently access
data.taskBoard directly and can throw/return undefined if the RPC envelope is
empty; update both to mirror getTurnState by using optional chaining on
unwrapEnvelope(response) and explicitly returning null when taskBoard is
missing, and update the functions' return types from Promise<TaskBoard> to
Promise<TaskBoard | null> (also adjust the callCoreRpc generic if needed) so
callers get a safe nullable result instead of an unsafe undefined.
In ***@***.***/src/store/coreModeSlice.ts`:
- Around line 54-56: Replace the direct import.meta.env usage by re-exporting
the VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE value from your central config and
consuming that export in the slice: add an exported constant (e.g. export const
VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE =
import.meta.env.VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE) to
app/src/utils/config.ts, then import that constant into coreModeSlice (where the
current if checks import.meta.env.VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE) and use
it in the conditional to return { kind: 'local' } when it equals 'local'; keep
the exported name identical so callers need only change the import.
In ***@***.***/test/e2e/helpers/shared-flows.ts`:
- Around line 251-255: Replace the single-pass marker check that sets finalCheck
by iterating billingMarkers and calling textExists with a retry loop: instead
implement the same polling pattern used elsewhere in this file (the retry+delay
loop that repeatedly checks markers) so the code repeatedly calls
textExists(marker) with short delays until a marker is found or timeout; update
the block that currently uses billingMarkers, finalCheck, and textExists to
perform polling per marker and break when a marker is detected to avoid
intermittent failures on slow WebView renders.
In ***@***.***/test/e2e/specs/auth-access-control.spec.ts`:
- Around line 209-217: Add the same failure-diagnostic pattern used earlier to
the billing handoff test: after navigateToBilling() and clearRequestLog(), call
the diagnostic helpers (dumpRequestLog() or print the request log and
dumpAccessibilityTree()) so you capture network/request context and the
accessibility tree before asserting textExists('Open dashboard') and
textExists('TinyHumans on the web'); if preferred wrap the two expect calls in a
try/catch that logs dumpRequestLog() and await dumpAccessibilityTree() on
failure to ensure useful diagnostics are emitted. Apply the same change to the
other billing tests referenced (the blocks around lines 224–238 and 240–254) so
each billing assertion emits request logs and dumpAccessibilityTree() on
failure.
In ***@***.***/openhuman/agent/harness/session/builder.rs`:
- Around line 846-853: When adding the AgentProfilePromptSection to the
prompt_builder (the block that checks profile_prompt_suffix and calls
prompt_builder.add_section and AgentProfilePromptSection::new), insert a
debug-level diagnostic log (e.g., using tracing::debug! or the project's logging
macro) immediately before or after the add_section call indicating the profile
prompt section was injected; use a stable grep-friendly prefix like "[agent]"
and include correlation/context fields (request id, session id, or similar
available in scope) plus a trimmed indicator of the suffix (not the full content
if sensitive) so operators can trace profile activation in logs.
In ***@***.***/openhuman/agent/profiles.rs`:
- Around line 81-125: The task-board persistence functions need verbose
diagnostics with a stable "[agent:task_board]" prefix: add
tracing::debug!/trace! logs at entry and exit of get, put, and delete (include
thread_id), at file existence checks (board_path() results - "board not
found"/"board loaded"), around normalization (e.g., normalise_cards/normalise
where cards length is available: "normalised {} cards"), and in all error paths
(include error text); ensure logs use key-value fields like thread_id=%thread_id
and card_count=board.cards.len() so they are grep-friendly and follow the
debug/trace levels per guidelines.
- Around line 55-181: Add tracing debug/trace diagnostics with a stable
"[agent:profiles]" prefix to all public methods (load, save, select, upsert,
delete, resolve) and key internal decisions: log entry/exit for each function
(including self.path() for load/save), branch outcomes like "profile not found",
"built-in merge" (when using built_in_default_profile or built_in_profiles),
"active profile fallback" (when resolving to DEFAULT_PROFILE_ID), state
transitions such as "upserting profile", "profile deleted", "active profile
changed", and all error-return paths (wrap error messages with
tracing::debug!/trace! before returning Err). Ensure messages reference the
profile id or state.profiles.len() where relevant and keep the prefix exactly
"[agent:profiles]" for grepability; use debug level for routine flows and trace
for very verbose internals.
In ***@***.***/openhuman/agent/schemas.rs`:
- Around line 441-493: Add verbose RPC diagnostics to the four profile handler
functions (handle_profiles_list, handle_profile_select, handle_profile_upsert,
handle_profile_delete): at entry log a stable prefix like "[rpc][agent][entry]"
with the RPC name and correlation id (generate or accept a request id from
params), on successful exit log "[rpc][agent][exit]" with result summary (e.g.,
number of profiles and activeProfileId) and on errors log "[rpc][agent][error]"
with the error message and the same correlation id; ensure logs are emitted via
the project logger used elsewhere (matching existing diagnostics style) and
include the agent_id or profile_id where relevant to make tracing grep-friendly.
In ***@***.***/openhuman/agent/task_board.rs`:
- Around line 134-140: Validate thread_id is not empty or only whitespace before
constructing file paths: in board_path use if thread_id.trim().is_empty() {
panic!("invalid thread_id: empty or whitespace") } (or return an Err in
functions that return Result) and apply the same check to the other
thread_id-taking methods referenced around lines 143–148 (e.g.,
read_board/write_board/delete_board or similarly named functions) so you trim
the input, reject blank IDs with a clear error message, and only then call
hex::encode and build the PathBuf.
- Around line 81-125: Add tracing/log calls to the TaskBoardStore flows to
record branches and errors: instrument get, put, and delete to emit debug/trace
events with a stable prefix (e.g., "task_board_store"), include correlation
fields like thread_id (board.thread_id or thread_id arg), op
("get"/"put"/"delete"), and outcome ("not_found","ok","error") and include the
error message on failures; place logs at key branches in get (path missing,
parse error, success), in put (ensure_dir success/failure, tempfile created,
fsync/persist success/failure, serialization error) and in delete (path missing,
remove_file success/failure), using the existing helpers board_path, ensure_dir,
normalise_board to source the thread id and path for the log fields.
- Around line 155-180: The loop sets card.order before you call
board.cards.retain(...), which leaves gaps when cards with empty titles are
removed; change the flow so you first trim fields and remove empty-title cards
(keep the trimming logic on each card as currently done), then iterate over the
filtered board.cards with enumerate to assign contiguous order values and update
updated_at — e.g., trim and normalize fields in the initial pass, call
board.cards.retain(|card| !card.title.is_empty()), then for (idx, card) in
board.cards.iter_mut().enumerate() set card.order = idx as u32 and
card.updated_at = now.clone(); ensure the blocker fallback logic
(TaskCardStatus::Blocked -> card.blocker = card.notes.clone()) still runs before
the retain so notes/blocker normalization is preserved.
In ***@***.***/openhuman/channels/providers/web.rs`:
- Around line 1193-1204: Add a diagnostic log when handling
AgentProgress::TaskBoardUpdated before calling publish_web_channel_event: log at
debug/trace level with a stable grep-friendly prefix (e.g.
"web:event:task_board_updated") and include correlation fields client_id,
thread_id, request_id and a short serialized summary of the task_board (use
serde_json::to_string or to_value). Update the TaskBoardUpdated branch where
publish_web_channel_event(WebChannelEvent { ... }) is invoked to emit this log
entry (referencing AgentProgress::TaskBoardUpdated, publish_web_channel_event,
and WebChannelEvent) so logs provide consistent diagnostics for this new
behavior.
In ***@***.***/openhuman/threads/schemas.rs`:
- Around line 533-560: Add verbose RPC diagnostics to the task board handlers by
instrumenting handle_task_board_get and handle_task_board_put with grep-friendly
log messages (e.g. "[rpc][task_board]") on entry and exit, key branches, and
error paths; include correlation fields like thread_id and any error details,
and use appropriate levels (trace/debug for entry/exit and branch decisions,
error for failures) so that load_or_init failures, board resolution
(board_for_thread), TaskBoardStore::put, and successful responses are all logged
with stable prefixes and contextual data.
- Around line 381-397: The ControllerSchema for "task_board_get" declares an
output field named "result" but the handler returns a JSON object keyed
"taskBoard"; update the outputs Vec for ControllerSchema entries that correspond
to task-board handlers (e.g., the ControllerSchema with function
"task_board_get") to use FieldSchema { name: "taskBoard", ty: TypeSchema::Json,
comment: "Task board payload.", required: true } (keep TypeSchema::Json and the
comment), and apply the same change to the other ControllerSchema entries that
return { "taskBoard": ... } so the schema key matches the actual handler
payload.
In ***@***.***/openhuman/tools/impl/agent/spawn_parallel_agents.rs`:
- Around line 108-309: The execute method in spawn_parallel_agents.rs lacks the
required verbose diagnostic logging; add debug/info/error logs with stable
grep-friendly prefixes (e.g. "[domain]", "[rpc]") at the start and end of
execute and around each major branch/validation (missing tasks, tasks.len()<2,
outside agent turn, max_parallel check, registry missing, invalid agent id,
integrations_agent toolkit missing), before/after publish_global calls
(SubagentSpawned/SubagentCompleted/SubagentFailed), on each progress_sink send,
when building prepared tasks, and after join_all with per-task outcomes; ensure
logs include correlation fields (parent_session, task_id, agent_id, mode),
counts (prompt_chars, elapsed_ms, iterations), and any captured error details so
failures are logged with context, and use the project logging facility (e.g.,
tracing::debug/info/error) consistently with the required prefixes.
---
Outside diff comments:
In ***@***.***/scripts/e2e-run-session.sh`:
- Around line 316-323: The script currently appends "--no-sandbox"
unconditionally to APP_ARGS in the Linux case; change this to only add
"--no-sandbox" when running as root by testing the UID (e.g., only append
"--no-sandbox" to APP_ARGS if [ "$(id -u)" -eq 0 ]), otherwise omit the flag and
keep the other Linux flags ("--disable-dev-shm-usage", "--disable-gpu",
"--no-zygote") and update the echo to reflect the actual APP_ARGS; touch the
APP_ARGS modification in the Linux case in e2e-run-session.sh where APP_ARGS is
built so non-root local Linux runs retain the sandbox.
---
Nitpick comments:
In ***@***.***/src/pages/Conversations.tsx`:
- Around line 1323-1372: Add an explicit "Cancel" button next to the Save button
in the profile creation block that closes the draft and resets inputs; implement
it to call the existing state setters used here (e.g. setProfileDraft(...) to
restore the initial/default profile object and setProfileDraftOpen(false) to
close the form) or call any existing close/reset helper (if you have a handler
like handleCloseProfileDraft). Make it type="button", visually styled as a
secondary action, and ensure it does not submit or call
handleCreateAgentProfile.
- Around line 307-339: handleCreateAgentProfile currently only checks non-empty
names but can create duplicate profile names; before calling
agentProfilesApi.upsert, check the existing agentProfiles array (referencing the
agentProfiles state) for a name match (case-insensitive, trimmed) and if found,
abort creation and surface a validation error (e.g., return early and set a
user-visible error or toast) instead of proceeding to upsert; ensure the
duplicate check uses the trimmed name variable and leave existing logic that
constructs id, profile object, and calls agentProfilesApi.upsert,
setAgentProfiles, setSelectedAgentProfileId, and resetting profileDraft
unchanged when no duplicate is found.
- Around line 1048-1069: handleMoveTaskCard currently does an optimistic update
and rollback on putTaskBoard failure but gives no user feedback; modify the
catch block to surface an error notification (e.g., show a toast or set an error
state) so users know the move failed. Specifically, in handleMoveTaskCard's
catch where it calls console.warn and dispatches setTaskBoardForThread to
revert, also call the app's notification/toast API (or dispatch a showError
action) with a short message like "Could not move task — changes were not saved"
and include minimal error details if available; keep the existing rollback
dispatch to restore UI state.
In ***@***.***/src/services/chatService.ts`:
- Around line 646-655: The RPC call to 'openhuman.channel_web_chat'
inconsistently handles optional params: profile_id is sent as params.profileId
?? undefined while model_override is sent directly as params.model; make them
consistent by either using the same ?? undefined pattern for params.model (i.e.,
model_override: params.model ?? undefined) or build the params object
conditionally so keys are omitted when their values are undefined; update the
callCoreRpc invocation in chatService.ts (the call that passes client_id,
thread_id, message, model_override, profile_id) to follow the chosen consistent
approach.
In ***@***.***/openhuman/agent/profiles.rs`:
- Around line 464-481: The handler handle_profile_upsert needs verbose
diagnostics: add tracing::debug at entry showing p.profile.id and
p.profile.agent_id after deserializing ProfileUpsertParams, emit a
tracing::debug/tracing::warn when the AgentDefinitionRegistry::global() check
runs to indicate whether registry exists and whether the trimmed agent_id was
found or missing, and log the result of the store operation (success or error)
before returning; use stable log prefixes like "[rpc][agent] profile_upsert" and
include the agent_id/profile_id fields in each log.
- Around line 150-180: The normalise_board function lacks diagnostic logging for
its implicit transformations; update the normalise_board(&mut TaskBoard)
implementation to emit tracing/debug logs: add an entry trace with
board.thread_id and initial card_count, log each card that receives an
auto-generated id (reference card.id generation code), emit a trace when you
populate card.blocker from card.notes for cards with TaskCardStatus::Blocked and
no blocker (the implicit blocker population), and after retaining
non-empty-title cards log a debug/trace showing how many cards were removed; use
tracing::trace or tracing::debug per guidelines.
- Around line 533-561: Add verbose diagnostics logging to the task board RPC
handlers: in handle_task_board_get and handle_task_board_put, log entry with the
parsed thread_id (from TaskBoardGetParams / TaskBoardPutParams) at debug/trace,
log when calling crate::openhuman::config::Config::load_or_init() and its
result, log calls to crate::openhuman::agent::task_board::board_for_thread() (or
the board save/put operation) and any branch decisions/errors, and log success
with the board card count (board.cards.len()) and a stable prefix like
"[rpc][threads] task_board_get" or "[rpc][threads] task_board_put"; include
error context in error logs.
In ***@***.***/openhuman/tools/impl/agent/todo_write.rs`:
- Around line 174-223: The persist_thread_board function currently omits a
success diagnostic log; add a verbose, grep-friendly debug/info log immediately
after TaskBoardStore::new(parent.workspace_dir.clone()).put(board)? returns
(before sending AgentProgress) that records a stable prefix like "[taskboard]"
or "[rpc]" plus correlation fields thread_id, parent workspace_dir (or workspace
id if available) and the saved board id/count (from saved.cards.len() or
saved.thread_id) so operators can trace persistence events; keep the log level
as debug/info per project convention and ensure the message appears on the
success path in persist_thread_board.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
------------------------------
ℹ️ Review info ⚙️ Run configuration
*Configuration used*: Organization UI
*Review profile*: CHILL
*Plan*: Pro
*Run ID*: 82a254dd-39c9-44d9-8411-eef082a464f6
📥 Commits
Reviewing files that changed from the base of the PR and between 7d8826f
<7d8826f>
and be77950
<be77950>
.
📒 Files selected for processing (51)
- app/.prettierignore
- app/scripts/e2e-build.sh
- app/scripts/e2e-run-session.sh
- app/src/pages/Conversations.tsx
- app/src/pages/__tests__/Conversations.render.test.tsx
- app/src/pages/conversations/components/TaskKanbanBoard.tsx
-
app/src/pages/conversations/components/__tests__/TaskKanbanBoard.test.tsx
- app/src/providers/ChatRuntimeProvider.tsx
- app/src/services/__tests__/chatService.test.ts
- app/src/services/api/agentProfilesApi.test.ts
- app/src/services/api/agentProfilesApi.ts
- app/src/services/api/threadApi.test.ts
- app/src/services/api/threadApi.ts
- app/src/services/chatService.ts
- app/src/store/__tests__/chatRuntimeSlice.test.ts
- app/src/store/chatRuntimeSlice.ts
- app/src/store/coreModeSlice.ts
- app/src/types/agentProfile.ts
- app/src/types/turnState.ts
- app/src/utils/config.ts
- app/src/utils/tauriCommands/core.test.ts
- app/src/utils/tauriCommands/core.ts
- app/src/vite-env.d.ts
- app/test/e2e/helpers/shared-flows.ts
- app/test/e2e/specs/auth-access-control.spec.ts
- app/test/e2e/specs/login-flow.spec.ts
- scripts/mock-api/routes/integrations.mjs
- src/api/config.rs
- src/core/socketio.rs
- src/openhuman/agent/agents/orchestrator/agent.toml
- src/openhuman/agent/harness/session/builder.rs
- src/openhuman/agent/mod.rs
- src/openhuman/agent/profiles.rs
- src/openhuman/agent/progress.rs
- src/openhuman/agent/schemas.rs
- src/openhuman/agent/task_board.rs
- src/openhuman/channels/bus.rs
- src/openhuman/channels/proactive.rs
- src/openhuman/channels/providers/presentation.rs
- src/openhuman/channels/providers/web.rs
- src/openhuman/channels/providers/web_tests.rs
- src/openhuman/screen_intelligence/tests.rs
- src/openhuman/threads/schemas.rs
- src/openhuman/threads/schemas_tests.rs
- src/openhuman/threads/turn_state/mirror.rs
- src/openhuman/threads/turn_state/types.rs
- src/openhuman/tools/impl/agent/mod.rs
- src/openhuman/tools/impl/agent/spawn_parallel_agents.rs
- src/openhuman/tools/impl/agent/todo_write.rs
- src/openhuman/tools/ops.rs
- src/openhuman/tools/ops_tests.rs
------------------------------
In app/src/services/api/agentProfilesApi.ts
<#1768 (comment)>
:
+function unwrapEnvelope<T>(response: Envelope<T> | T): T {
+ if (response && typeof response === 'object' && 'data' in response) {
+ return (response as Envelope<T>).data as T;
+ }
+ return response as T;
+}
*
------------------------------
In app/src/services/api/threadApi.ts
<#1768 (comment)>
:
+ getTaskBoard: async (threadId: string): Promise<TaskBoard> => {
+ const response = await callCoreRpc<{ data?: GetTaskBoardResponse }>({
+ method: 'openhuman.threads_task_board_get',
+ params: { thread_id: threadId },
+ });
+ const data = unwrapEnvelope(response);
+ return data.taskBoard;
+ },
+
+ putTaskBoard: async (threadId: string, cards: TaskBoardCard[]):
Promise<TaskBoard> => {
+ const response = await callCoreRpc<{ data?: PutTaskBoardResponse }>({
+ method: 'openhuman.threads_task_board_put',
+ params: { thread_id: threadId, cards },
+ });
+ const data = unwrapEnvelope(response);
+ return data.taskBoard;
+ },
*
------------------------------
In app/src/store/coreModeSlice.ts
<#1768 (comment)>
:
+ if (import.meta.env.VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE === 'local') {
+ return { kind: 'local' };
+ }
*🛠️ Refactor suggestion* | *🟠 Major* | *⚡ Quick win*
*Read E2E config via config.ts instead of import.meta.env directly.*
The coding guideline requires all VITE_* environment variables to be read
in app/src/utils/config.ts and re-exported; other files should import from
config.ts rather than accessing import.meta.env directly.
Add VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE to config.ts and import it here.
As per coding guidelines, environment variables should be centralized in
config.ts.
♻️ Proposed fix
In app/src/utils/config.ts, add:
export const E2E_RESTART_APP_AS_RELOAD =
import.meta.env.VITE_OPENHUMAN_E2E_RESTART_APP_AS_RELOAD ===
'true';+export const E2E_DEFAULT_CORE_MODE =+
(import.meta.env.VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE as string |
undefined) || '';
Then in app/src/store/coreModeSlice.ts:
+import { E2E_DEFAULT_CORE_MODE } from '../utils/config';+
export type CoreMode =
| { kind: 'unset' }
function deriveInitialMode(): CoreMode {- if
(import.meta.env.VITE_OPENHUMAN_E2E_DEFAULT_CORE_MODE === 'local') {+
if (E2E_DEFAULT_CORE_MODE === 'local') {
return { kind: 'local' };
}
📝 Committable suggestion
------------------------------
In app/test/e2e/helpers/shared-flows.ts
<#1768 (comment)>
:
+ let finalCheck = false;
+ for (const marker of billingMarkers) {
+ finalCheck = await textExists(marker);
+ if (finalCheck) break;
+ }
*
------------------------------
In app/test/e2e/specs/auth-access-control.spec.ts
<#1768 (comment)>
:
+ it('3.2.1 — billing dashboard entry point is stable', async () => {
await navigateToBilling();
clearRequestLog();
- await clickText('Upgrade', 10_000);
- console.log('[AuthAccess] Clicked Upgrade button');
- await browser.pause(3_000);
-
- const purchaseCall = await waitForRequest('POST',
'/payments/stripe/purchasePlan', 10_000);
- expect(purchaseCall).toBeDefined();
-
- if (purchaseCall?.body) {
- const bodyStr = typeof purchaseCall.body === 'string' ?
purchaseCall.body : '';
- console.log('[AuthAccess] Purchase request body:', bodyStr);
- }
-
- // Verify purchasing state appears
- const hasWaiting = (await textExists('Waiting')) || (await
textExists('Waiting for payment'));
- console.log(`[AuthAccess] Purchasing state visible: ${hasWaiting}`);
-
- // Switch mock to BASIC plan so polling clears the waiting state
- setMockBehavior('plan', 'BASIC');
- setMockBehavior('planActive', 'true');
- setMockBehavior('planExpiry', new Date(Date.now() + 30 *
86400000).toISOString());
-
- if (hasWaiting) {
- const disappeared = await waitForTextToDisappear('Waiting', 20_000);
- if (!disappeared) {
- throw new Error(
- '3.2.1 — "Waiting" spinner did not clear within 20s after
mock plan was set to BASIC'
- );
- }
- }
+ expect(await textExists('Open dashboard')).toBe(true);
+ expect(await textExists('TinyHumans on the web')).toBe(true);
- console.log('[AuthAccess] 3.2.1 — Upgrade purchase flow verified');
+ console.log('[AuthAccess] 3.2.1 — Billing dashboard entry point verified');
await navigateToHome();
*
------------------------------
In src/openhuman/agent/harness/session/builder.rs
<#1768 (comment)>
:
+ if let Some(suffix) = profile_prompt_suffix
+ .map(|s| s.trim().to_string())
+ .filter(|s| !s.is_empty())
+ {
+ prompt_builder = prompt_builder.add_section(Box::new(
+
crate::openhuman::agent::profiles::AgentProfilePromptSection::new(suffix),
+ ));
+ }
*
------------------------------
In src/openhuman/agent/profiles.rs
<#1768 (comment)>
:
+ parent.display()
+ )
+ })?;
+ let bytes = serde_json::to_vec_pretty(&state)
+ .map_err(|e| format!("serialize agent profiles: {e}"))?;
+ tmp.write_all(&bytes)
+ .map_err(|e| format!("write agent profiles tempfile: {e}"))?;
+ tmp.as_file()
+ .sync_all()
+ .map_err(|e| format!("fsync agent profiles tempfile: {e}"))?;
+ tmp.persist(&path)
+ .map_err(|e| format!("persist agent profiles {}: {e}",
path.display()))?;
+ Ok(state)
+ }
+
+ pub fn select(&self, profile_id: &str) ->
Result<AgentProfilesState, String> {
+ let mut state = self.load()?;
+ let profile_id = profile_id.trim();
+ if !state.profiles.iter().any(|p| p.id == profile_id) {
+ return Err(format!("agent profile '{profile_id}' not found"));
+ }
+ state.active_profile_id = profile_id.to_string();
+ self.save(state)
+ }
+
+ pub fn upsert(&self, profile: AgentProfile) ->
Result<AgentProfilesState, String> {
+ let mut state = self.load()?;
+ let profile = normalise_profile(profile);
+ let profile = if profile.id == DEFAULT_PROFILE_ID {
+ let mut default = built_in_default_profile();
+ default.name = profile.name;
+ default.description = profile.description;
+ default.model_override = profile.model_override;
+ default.temperature = profile.temperature;
+ default.system_prompt_suffix = profile.system_prompt_suffix;
+ default.allowed_tools = profile.allowed_tools;
+ default
+ } else {
+ AgentProfile {
+ built_in: profile.built_in
+ || built_in_profiles()
+ .iter()
+ .any(|builtin| builtin.id == profile.id),
+ ..profile
+ }
+ };
+
+ if let Some(existing) = state.profiles.iter_mut().find(|p|
p.id == profile.id) {
+ *existing = profile;
+ } else {
+ state.profiles.push(profile);
+ }
+ self.save(state)
+ }
+
+ pub fn delete(&self, profile_id: &str) ->
Result<AgentProfilesState, String> {
+ let profile_id = profile_id.trim();
+ if built_in_profiles()
+ .iter()
+ .any(|profile| profile.id == profile_id)
+ {
+ return Err(format!(
+ "built-in agent profile '{profile_id}' cannot be deleted"
+ ));
+ }
+ let mut state = self.load()?;
+ let before = state.profiles.len();
+ state.profiles.retain(|p| p.id != profile_id);
+ if state.profiles.len() == before {
+ return Err(format!("agent profile '{profile_id}' not found"));
+ }
+ if state.active_profile_id == profile_id {
+ state.active_profile_id = DEFAULT_PROFILE_ID.to_string();
+ }
+ self.save(state)
+ }
+
+ pub fn resolve(
+ &self,
+ requested_profile_id: Option<&str>,
+ ) -> Result<(AgentProfilesState, AgentProfile), String> {
+ let state = self.load()?;
+ let requested = requested_profile_id
+ .map(str::trim)
+ .filter(|id| !id.is_empty())
+ .unwrap_or(state.active_profile_id.as_str());
+ let profile = state
+ .profiles
+ .iter()
+ .find(|profile| profile.id == requested)
+ .or_else(|| {
+ state
+ .profiles
+ .iter()
+ .find(|profile| profile.id == DEFAULT_PROFILE_ID)
+ })
+ .cloned()
+ .unwrap_or_else(built_in_default_profile);
+ Ok((state, profile))
+ }
*
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/store/coreModeSlice.test.ts`:
- Around line 67-75: The test currently calls vi.doUnmock('../utils/config')
after assertions which will be skipped if import or expect throws; wrap the
mocking/import/assertion sequence in a try/finally so
vi.doUnmock('../utils/config') is always executed (or move the unmock to an
afterEach that always runs). Specifically, update the test that uses
vi.resetModules(), vi.doMock('../utils/config'), await
import('./coreModeSlice'), and expect(state.mode) to ensure
vi.doUnmock('../utils/config') is invoked in a finally block (or registered in
afterEach) to guarantee cleanup even on failure.
In `@src/openhuman/agent/schemas.rs`:
- Around line 820-837: WorkspaceEnvGuard currently unconditionally removes
OPENHUMAN_WORKSPACE on drop which can break other tests; modify
WorkspaceEnvGuard to store the previous environment value (use
Option<std::ffi::OsString> or Option<String>) when set(path: &Path) is called
(capture std::env::var_os("OPENHUMAN_WORKSPACE") before setting), save it as a
field on the struct, and then in Drop::drop restore the previous value (call
std::env::set_var if Some(prev) or std::env::remove_var if None) so the original
environment is preserved; update the WorkspaceEnvGuard struct definition and the
set and drop methods accordingly.
In `@src/openhuman/threads/schemas_tests.rs`:
- Around line 234-251: WorkspaceEnvGuard currently unconditionally removes
OPENHUMAN_WORKSPACE on drop which can clobber a pre-existing env; modify
WorkspaceEnvGuard to store the prior value when set(path: &Path) is called
(e.g., an Option<std::ffi::OsString> field), set the env variable to the
provided path, and in the Drop impl (drop(&mut self)) restore the previous
value: if the stored Option is Some(v) call
std::env::set_var("OPENHUMAN_WORKSPACE", v) else call
std::env::remove_var("OPENHUMAN_WORKSPACE"); update WorkspaceEnvGuard::set, the
struct definition, and impl Drop::drop accordingly.
In `@src/openhuman/tools/impl/agent/todo_write.rs`:
- Around line 164-170: The current code swallows all errors from persisted_board
and always returns Ok(ToolResult::success(body)); change this so real
persistence failures propagate as a tool error while still treating the "no
parent/thread context" case as non-fatal: inspect the Err(err) from
persisted_board in the todo_write implementation, detect the specific non-fatal
condition (the code-path/error that indicates missing parent/thread context),
and for other errors return an Err or ToolResult::failure (instead of
Ok(success)) with a clear error message including err; keep returning
Ok(ToolResult::success(body)) only for the non-fatal "no parent/thread context"
case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f1c9e66-ed2f-40c7-9c95-e26ecb10d339
📒 Files selected for processing (28)
app/scripts/e2e-run-session.shapp/src/components/settings/panels/__tests__/DeveloperOptionsPanel.test.tsxapp/src/pages/Conversations.tsxapp/src/pages/__tests__/Conversations.render.test.tsxapp/src/providers/__tests__/ChatRuntimeProvider.test.tsxapp/src/services/__tests__/chatService.test.tsapp/src/services/api/agentProfilesApi.test.tsapp/src/services/api/agentProfilesApi.tsapp/src/services/api/threadApi.test.tsapp/src/services/api/threadApi.tsapp/src/services/chatService.tsapp/src/store/coreModeSlice.test.tsapp/src/store/coreModeSlice.tsapp/src/test/setup.tsapp/src/utils/config.tsapp/test/e2e/helpers/shared-flows.tsapp/test/e2e/specs/auth-access-control.spec.tssrc/openhuman/agent/harness/session/builder.rssrc/openhuman/agent/profiles.rssrc/openhuman/agent/schemas.rssrc/openhuman/agent/task_board.rssrc/openhuman/agent/triage/escalation.rssrc/openhuman/channels/providers/web.rssrc/openhuman/threads/schemas.rssrc/openhuman/threads/schemas_tests.rssrc/openhuman/threads/turn_state/mirror_tests.rssrc/openhuman/tools/impl/agent/spawn_parallel_agents.rssrc/openhuman/tools/impl/agent/todo_write.rs
🚧 Files skipped from review as they are similar to previous changes (15)
- app/src/store/coreModeSlice.ts
- app/src/services/api/agentProfilesApi.ts
- app/src/services/api/threadApi.test.ts
- app/src/utils/config.ts
- app/src/services/api/threadApi.ts
- src/openhuman/threads/schemas.rs
- src/openhuman/agent/harness/session/builder.rs
- app/test/e2e/helpers/shared-flows.ts
- src/openhuman/tools/impl/agent/spawn_parallel_agents.rs
- app/test/e2e/specs/auth-access-control.spec.ts
- app/src/services/chatService.ts
- src/openhuman/channels/providers/web.rs
- src/openhuman/agent/task_board.rs
- src/openhuman/agent/profiles.rs
- app/src/pages/Conversations.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openhuman/tools/impl/agent/todo_write.rs (1)
123-179:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize items before storing and rendering them.
persist_thread_boardtrims IDs/titles/notes/blockers, butself.store.replace(items.clone())and the success body still use the raw input. That makes the in-memory todo state and tool output drift from the persisted board/UI for inputs like" Draft plan "or" note ".Suggested fix
let items: Vec<TodoItem> = serde_json::from_value(todos.clone()) .map_err(|e| anyhow::anyhow!("Invalid todos array: {e}"))?; + let items: Vec<TodoItem> = items + .into_iter() + .map(|mut item| { + item.content = item.content.trim().to_string(); + item.id = item.id.and_then(|value| { + let value = value.trim().to_string(); + (!value.is_empty()).then_some(value) + }); + item.notes = item.notes.and_then(|value| { + let value = value.trim().to_string(); + (!value.is_empty()).then_some(value) + }); + item.blocker = item.blocker.and_then(|value| { + let value = value.trim().to_string(); + (!value.is_empty()).then_some(value) + }); + item + }) + .collect(); if items.iter().any(|i| i.content.trim().is_empty()) { return Ok(ToolResult::error("todo `content` must not be empty"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/tools/impl/agent/todo_write.rs` around lines 123 - 179, The execute method stores and renders the raw TodoItem inputs before normalization, causing divergence from what persist_thread_board (which trims fields) saves; update execute to normalize items (trim IDs, titles/content, notes, and blocker strings and apply any other normalization persist_thread_board uses) before calling self.store.replace(items.clone()), before building the success body, and before passing them into persist_thread_board so the in-memory store, rendered message, and persisted board all use the same cleaned TodoItem values (adjust variable names in execute, operate on the Vec<TodoItem> before the in_progress check or immediately after deserialization, and reference TodoItem, TodoStatus, self.store.replace, and persist_thread_board).
🧹 Nitpick comments (2)
src/openhuman/service/restart.rs (1)
206-206: 💤 Low valueConsider a more descriptive panic message.
The current message "probe channel should stay open" could be enhanced to better explain the failure condition when debugging test failures.
💬 Suggested improvement
- None => panic!("probe channel should stay open"), + None => panic!("probe channel closed unexpectedly before receiving expected restart event (source='test', reason='integration')"),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/service/restart.rs` at line 206, Replace the terse panic!("probe channel should stay open") with a more descriptive message that explains the failure and includes relevant context: update the match arm handling the probe channel closure (the None branch where the code currently panics) to panic/expect with a message like "probe channel closed unexpectedly in <functionName> (probe id: {...}, state: {...})" or otherwise include any nearby identifiers (e.g., probe, probe_rx, service id) so test failures show which probe and where it failed; use format/expect to include debug info for available variables.src/openhuman/threads/schemas_tests.rs (1)
302-312: ⚡ Quick winMirror the blank-
thread_idassertion ontask_board_put.These tests only lock in the read-path validation. A write-path regression can still slip through and persist or route a board under an invalid ID.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/threads/schemas_tests.rs` around lines 302 - 312, Add a parallel test that ensures the write path rejects a blank thread_id: create an async tokio test (e.g., task_board_put_rejects_blank_thread_id) that sets up ENV_LOCK and WorkspaceEnvGuard exactly like task_board_get_rejects_blank_thread_id, then call handle_task_board_put with an object whose "thread_id" is whitespace, await expect_err, and assert the error contains "thread_id"; reference handle_task_board_put and name the test to mirror task_board_get_rejects_blank_thread_id so write-path validation is covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/tools/impl/agent/todo_write.rs`:
- Around line 244-249: The debug event currently logs the full workspace path
via parent.workspace_dir.display(), which can expose user PII; modify the
tracing::debug call that uses saved.thread_id and parent.workspace_dir to redact
or omit the full path—e.g., replace parent.workspace_dir.display() with a
sanitized value (only the workspace name via
workspace_dir.file_name()/to_string_lossy(), a truncated path, or a constant
like "<REDACTED>") while keeping card_count and thread_id intact so the log
still shows saved.thread_id and saved.cards.len() but no raw filesystem path.
- Around line 250-254: The current await on parent.on_progress.send blocks if
the bounded channel is full; replace the awaited send with a non-blocking
try_send on parent.on_progress (sending AgentProgress::TaskBoardUpdated { board:
saved.clone() } or otherwise cloning saved as needed) and handle Err by logging
at debug level with the prefix "[todowrite]" and relevant fields (e.g.,
thread_id, board id) so dropped progress events are recorded; alternatively
follow the emit_progress pattern used in emit_progress
(src/openhuman/agent/harness/session/turn.rs) to log the error instead of
silently discarding it.
---
Outside diff comments:
In `@src/openhuman/tools/impl/agent/todo_write.rs`:
- Around line 123-179: The execute method stores and renders the raw TodoItem
inputs before normalization, causing divergence from what persist_thread_board
(which trims fields) saves; update execute to normalize items (trim IDs,
titles/content, notes, and blocker strings and apply any other normalization
persist_thread_board uses) before calling self.store.replace(items.clone()),
before building the success body, and before passing them into
persist_thread_board so the in-memory store, rendered message, and persisted
board all use the same cleaned TodoItem values (adjust variable names in
execute, operate on the Vec<TodoItem> before the in_progress check or
immediately after deserialization, and reference TodoItem, TodoStatus,
self.store.replace, and persist_thread_board).
---
Nitpick comments:
In `@src/openhuman/service/restart.rs`:
- Line 206: Replace the terse panic!("probe channel should stay open") with a
more descriptive message that explains the failure and includes relevant
context: update the match arm handling the probe channel closure (the None
branch where the code currently panics) to panic/expect with a message like
"probe channel closed unexpectedly in <functionName> (probe id: {...}, state:
{...})" or otherwise include any nearby identifiers (e.g., probe, probe_rx,
service id) so test failures show which probe and where it failed; use
format/expect to include debug info for available variables.
In `@src/openhuman/threads/schemas_tests.rs`:
- Around line 302-312: Add a parallel test that ensures the write path rejects a
blank thread_id: create an async tokio test (e.g.,
task_board_put_rejects_blank_thread_id) that sets up ENV_LOCK and
WorkspaceEnvGuard exactly like task_board_get_rejects_blank_thread_id, then call
handle_task_board_put with an object whose "thread_id" is whitespace, await
expect_err, and assert the error contains "thread_id"; reference
handle_task_board_put and name the test to mirror
task_board_get_rejects_blank_thread_id so write-path validation is covered.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b592697d-0c66-41bb-8c2f-4d486fa10b19
📒 Files selected for processing (5)
app/src/store/coreModeSlice.test.tssrc/openhuman/agent/schemas.rssrc/openhuman/service/restart.rssrc/openhuman/threads/schemas_tests.rssrc/openhuman/tools/impl/agent/todo_write.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/store/coreModeSlice.test.ts
- src/openhuman/agent/schemas.rs
12 tasks
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/threads/schemas_tests.rs (1)
296-300: ⚡ Quick winAssert normalized fields from the persisted
getpayload, not just count.This currently proves normalization in the
putresponse, but not that persisted data stays normalized when read back viatask_board_get.Suggested test hardening
- assert_eq!(get["taskBoard"]["cards"].as_array().unwrap().len(), 2); + let cards = get["taskBoard"]["cards"] + .as_array() + .expect("cards array"); + assert_eq!(cards.len(), 2); + assert!(cards.iter().any(|c| c["id"] == "task-a" && c["order"] == 0)); + assert!(cards.iter().any(|c| c["id"] == "task-b" && c["blocker"] == "waiting"));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/openhuman/threads/schemas_tests.rs` around lines 296 - 300, The test only checks the number of cards returned by handle_task_board_get; instead, after calling handle_task_board_get and storing the result in get, iterate get["taskBoard"]["cards"] and assert the actual normalized fields/values match the expected persisted form (e.g., normalized card ids, field names and normalized values produced by the prior put). Update the assertions to verify specific fields on each card from get (using the get variable and the "taskBoard" -> "cards" path) equal the expected normalized representations rather than only checking length, so the test proves normalization persists through task_board_get.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/openhuman/threads/schemas_tests.rs`:
- Around line 296-300: The test only checks the number of cards returned by
handle_task_board_get; instead, after calling handle_task_board_get and storing
the result in get, iterate get["taskBoard"]["cards"] and assert the actual
normalized fields/values match the expected persisted form (e.g., normalized
card ids, field names and normalized values produced by the prior put). Update
the assertions to verify specific fields on each card from get (using the get
variable and the "taskBoard" -> "cards" path) equal the expected normalized
representations rather than only checking length, so the test proves
normalization persists through task_board_get.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 41a184a8-f209-4b45-ba63-f4563cbcaa9f
📒 Files selected for processing (6)
.github/workflows/coverage.yml.github/workflows/e2e-agent-review.yml.github/workflows/e2e.ymlsrc/openhuman/service/restart.rssrc/openhuman/threads/schemas_tests.rssrc/openhuman/tools/impl/agent/todo_write.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/e2e.yml
- src/openhuman/service/restart.rs
- src/openhuman/tools/impl/agent/todo_write.rs
Contributor
Author
|
@coderabbitai resolve |
Contributor
✅ Actions performedComments resolved and changes approved. |
Member
|
woot woot... task boards FTW |
graycyrus
requested changes
May 15, 2026
Contributor
graycyrus
left a comment
graycyrus
left a comment
There was a problem hiding this comment.
Walkthrough
This PR bundles three features — agent task boards (#1757), parallel agent orchestration (#1756), and selectable agent profiles (#1758) — plus a significant batch of E2E/CI maintenance. The Rust core and frontend implementations are solid, with good normalisation, atomic writes, debug logging, and test coverage on the CRUD paths. However, there are pattern deviations from the surrounding codebase and a notable gap in test coverage for the concurrent execution path.
Change Summary
| File | Change type | Description |
|---|---|---|
agent/profiles.rs |
New (708 lines) | Persistent profile store with CRUD, normalisation, built-in defaults |
agent/task_board.rs |
New (470 lines) | Per-thread task board store with hex-encoded file naming |
agent/schemas.rs |
Modified | 4 new RPC handlers for profile CRUD |
agent/progress.rs |
Modified | TaskBoardUpdated progress variant |
tools/impl/agent/spawn_parallel_agents.rs |
New (735 lines) | Concurrent subagent fan-out tool with validation and ownership boundaries |
tools/impl/agent/todo_write.rs |
Modified | Wired to persist task boards + Blocked status + progress emission |
channels/providers/web.rs |
Modified | Profile-aware session building, model/temperature overrides, tool visibility |
TaskKanbanBoard.tsx |
New (107 lines) | 4-column kanban with arrow-move buttons |
agentProfilesApi.ts |
New (50 lines) | Profile RPC client |
Conversations.tsx |
Modified | Profile selector, create form, task board integration |
chatRuntimeSlice.ts |
Modified | taskBoardByThread state |
ChatRuntimeProvider.tsx |
Modified | onTaskBoardUpdated handler |
chatService.ts |
Modified | task_board_updated event, profileId in send |
| E2E/CI files (15+) | Modified | Onboarding flow, billing handoff, Appium install, coverage timeout |
Per-file Analysis
Rust core
profiles.rs — Clean implementation. Good use of tempfile::NamedTempFile for atomic writes, BTreeMap for deterministic merge order, and thorough normalisation. Logging uses consistent [agent:profiles] prefix with tracing::debug!. Tests cover CRUD, edge cases (blank IDs, built-in deletion), and prompt section rendering.
task_board.rs — Follows the same atomic-write pattern as profiles. validate_thread_id prevents blank IDs. Hex-encoded filenames prevent path traversal. normalise_board handles empty titles, recomputes order, and copies notes to blocker when status is Blocked.
spawn_parallel_agents.rs — Well-structured validation pipeline: task count, parent context, registry check, integrations_agent toolkit check, ownership boundaries. Uses join_all for fan-out with per-task error isolation. Progress events are emitted for both success and failure paths.
todo_write.rs — Good integration: maps TodoItem to TaskBoardCard, persists via TaskBoardStore, emits TaskBoardUpdated progress. Uses try_send (non-blocking) for progress channel — tested explicitly with a full-channel scenario.
web.rs — Profile resolution is clean: profile model/temperature override takes precedence, then falls back to RPC params. Session cache invalidation correctly checks profile_signature to detect profile content changes. Tool visibility filtering via set_visible_tool_names is correct.
Frontend
TaskKanbanBoard.tsx — Clean, minimal component. Good accessibility with aria-label on move buttons, proper disabled state handling. No dynamic imports, no direct env access.
Conversations.tsx — Profile selector and create form work, but see inline comments about state management pattern deviation.
Red flags
- Very large PR (4606 additions, 63 files) — three independent features + E2E updates bundled together. Consider splitting in future.
- No labels on the PR.
- Three issues linked, all reasonably addressed with minor gaps noted below.
| const [inlineSuggestionValue, setInlineSuggestionValue] = useState(''); | ||
| const [sendError, setSendError] = useState<ChatSendError | null>(null); | ||
| const [sendAdvisory, setSendAdvisory] = useState<string | null>(null); | ||
| const [agentProfiles, setAgentProfiles] = useState<AgentProfile[]>([]); |
Contributor
There was a problem hiding this comment.
[major] Agent profile state (agentProfiles, selectedAgentProfileId) is managed via useState instead of Redux. This deviates from the project pattern where domain state that persists and is shared across the app lives in Redux slices (see accountsSlice.ts, threadSlice.ts, chatRuntimeSlice.ts). If Conversations unmounts and remounts, the profile list re-fetches; if another component needs profile data, it can't access it.
Suggestion: Create an agentProfileSlice.ts following the existing slice pattern, with a thunk for loading and actions for select/create. This also gives you resetUserScopedState wiring for free.
| .filter(Boolean); | ||
| const profile: AgentProfile = { | ||
| id, | ||
| name, |
Contributor
There was a problem hiding this comment.
[minor] console.warn is used here and in handleCreateAgentProfile below, but the surrounding provider code (ChatRuntimeProvider.tsx) uses debugFactory for namespaced structured logging (e.g., debug('[conversations] ...')). Using console.warn bypasses the debug namespace filtering.
Suggestion: Use const debug = debugFactory('conversations'); and debug(...) for consistency.
| @@ -85,6 +91,18 @@ type InputMode = 'text' | 'voice'; | |||
| type ReplyMode = 'text' | 'voice'; | |||
| const AUTOCOMPLETE_POLL_DEBOUNCE_MS = 320; | |||
| const AUTOCOMPLETE_MIN_CONTEXT_CHARS = 3; | |||
Contributor
There was a problem hiding this comment.
[minor] AGENT_PROFILE_AGENT_OPTIONS hardcodes the agent IDs (orchestrator, researcher, planner, critic). The backend already has built_in_profiles() which defines these — if a new built-in agent is added server-side, this dropdown won't include it. Consider deriving the options from the fetched agentProfiles list instead (each profile already has an agentId).
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { |
Contributor
There was a problem hiding this comment.
[major] Issue #1756 acceptance criteria requires "Multiple workers can run concurrently" and "Regression coverage added", but no test exercises actual concurrent execution. All 6 tests validate parameter rejection or immediate failures — none call run_subagent or verify that tasks actually fan out in parallel.
I understand mocking run_subagent is non-trivial, but at minimum a test that initialises the registry, sets up a parent context, and submits 2+ valid tasks (even if the subagent runs return quickly) would cover the join_all path and the result collection logic. Right now there's zero coverage of lines 337–456 (the actual fan-out + result assembly).
| &self, | ||
| _query: &str, | ||
| _limit: usize, | ||
| _opts: RecallOpts<'_>, |
Contributor
There was a problem hiding this comment.
[minor] NoopProvider and NoopMemory are identical to the ones defined in todo_write.rs tests (same file, same PR). This is a lot of boilerplate to duplicate. Consider extracting these to a shared test utility module (e.g., tools/impl/agent/test_helpers.rs or a #[cfg(test)] module in the parent mod.rs).
Contributor
|
Independent confirmation: the failing Reproduced locally on a clean upstream Given the maintainer review here is asking this PR to be split, would you prefer the auth_retry/client.rs fix be pulled out into a standalone PR that just unblocks main's CI? Happy to open one (deleting the outer |
Contributor
Author
|
@graycyrus thanks for the detailed review. I pushed
I left the duplicated All GitHub checks are green on |
Contributor
|
@Zavianx this PR has merge conflicts with main — please rebase/resolve before review. |
Zavianx
force-pushed
the
feat/agent-task-orchestration
branch
from
ade8fd2 to
9813f67
Compare
Contributor
Author
|
@graycyrus rebased this branch onto latest Local validation after the rebase:
GitHub now reports the PR as mergeable again; the fresh CI run is in progress. |
# Conflicts: # app/src/pages/__tests__/Conversations.render.test.tsx # src/api/config.rs # src/openhuman/accessibility/permissions_tests.rs # src/openhuman/config/schema/load.rs # src/openhuman/config/schema/load_tests.rs # src/openhuman/screen_intelligence/tests.rs
…e tests for profileId - Resolve all 6 merge conflicts intelligently combining both sides - Remove duplicate `execute_tool_once` method in composio/client.rs (PR leftover from manual merge; the canonical version using execute_tool_request_body is kept) - Update IME keyboard tests (from main) to include profileId field added by PR - Replace console.warn with namespaced debug() in Conversations.tsx per graycyrus review comment (minor)
Member
pr-manager-lite fixes appliedMerge conflicts resolved (6 files):
Bug fix: Removed duplicate Minor (graycyrus @355): Replaced 4 Pre-existing issues not introduced by this PR (deferred to human):
Deferred to human review (graycyrus [major]):
|
Add async thunk tests (pending/fulfilled/rejected) for all four agentProfileSlice thunks (load, select, upsert, delete), selector tests, and edge-case coverage for the applyProfileResponse fallback paths. Add clearTaskBoardForThread test to chatRuntimeSlice suite. Addresses coverage gaps flagged in PR review.
…ponse The roundtrip test previously only checked card count from the persisted get payload. Now also asserts threadId, card id, title, order, and blocker are preserved from the put normalization, proving that stored data stays normalized when read back. Addresses CodeRabbit comment on schemas_tests.rs:296.
# Conflicts: # app/src/components/settings/panels/__tests__/DeveloperOptionsPanel.test.tsx # app/src/pages/Conversations.tsx # app/src/utils/tauriCommands/core.test.ts # app/src/utils/tauriCommands/core.ts # app/test/e2e/helpers/shared-flows.ts # src/openhuman/channels/providers/web.rs
…acheFingerprint refactor
- Update core.test.ts expected debug message to match new IS_DEV_LIKE log
text ('dev mode → window.location.reload()')
- Add E2E_DEFAULT_CORE_MODE to DeveloperOptionsPanel test config mock
(pulled transitively by coreModeSlice; fixes missing-export Vitest error)
- Remove unused profile_signature import from web.rs (dropped when
SessionEntry migrated to SessionCacheFingerprint struct)
Adds a 'Tasks' tab to the Intelligence page that aggregates all agent task boards across conversations in one view. - New IntelligenceTasksTab component: merges live boards from chatRuntime.taskBoardByThread (real-time socket updates) with persisted boards from threadApi.listTurnStates. Live boards render a pulsing 'live' badge and sort first; threads are resolved to their title with shortId fallback for unknown threads. - Reuses the existing TaskKanbanBoard component (read-only; no onMove passed — editing stays in the Conversations page). - 8 Vitest unit tests covering loading, error, empty, board aggregation, title resolution, live priority, and count display. - Wired into Intelligence.tsx between Subconscious and Calls tabs.
…fix threadApi.ts unwrapEnvelope undefined check - Conversations.tsx: replace console.warn with namespaced debug() for the 120s silence timer log (addresses @graycyrus review) - threadApi.ts: guard against undefined data in unwrapEnvelope (mirrors the same fix already applied in agentProfilesApi.ts; addresses @coderabbitai review on threadApi.ts)
Member
pr-manager-lite: review items appliedMerge conflicts resolved (6 files):
Reviewer fixes applied:
New feature: agent task kanban board in Intelligence page
Quality suite results:
Outstanding items (deferred to human):
|
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Addressed ✅
Accepted design decisions
IntelligenceTasksTab — nice addition. Surfacing aggregated task boards in the Intelligence page as a read-only view, with editing scoped to the Conversations page, is a clean separation of concerns. Outstanding (deferred)
Would you like me to open a follow-up GitHub issue to track the (ﾉ◕ヮ◕)ﾉ*:･ﾟ✧ |
ParentExecutionContext no longer has a composio_client field after the main merge — remove the stale assignment from the two test fixtures so the core test crate compiles again.
# Conflicts: # src/openhuman/composio/client.rs
# Conflicts: # .github/workflows/e2e.yml
# Conflicts: # src/openhuman/composio/client.rs
…vider routing Upstream PR tinyhumansai#1858 (unified per-workload provider routing) made the subagent runner resolve a real Provider via the loaded Config's workload routing for ModelSpec::Hint agents, bypassing this test's mock ConcurrentProvider. The right fix is a workspace-isolated fixture that forces the workload factory to fall back to the parent provider — tracked as a follow-up.
# Conflicts: # src/openhuman/config/schema/load.rs
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
todowriteupdates into board events/RPCs, and show a kanban board in conversations.spawn_parallel_agentstool for concurrent subagent task execution with per-task progress/error summaries.Issues
Closes #1756
Closes #1757
Closes #1758
Testing
cargo fmt --allpnpm format:checkpnpm typecheckpnpm lint(passes with 32 existing warnings)pnpm test:coveragepnpm test:rustxvfb-run -a pnpm --filter openhuman-app test:e2e(login: 12 passing; auth/access: 9 passing)format:check,lint,compile,rust:check,lint:commands-tokensSummary by CodeRabbit
New Features
Chores