Repository navigation
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 385 files, which is 85 over the limit of 300. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (385)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Avoid dirtying the parent checkout with nested worktrees
When a user's repo does not already ignore .claude/, creating each worker checkout under repo_top/.claude/worktrees makes the parent checkout itself dirty; I checked this in a scratch repo and git status --porcelain reports ?? .claude/ immediately after git worktree add. That undermines the isolation guarantee and can leave/commit worker plumbing in the user's main tree, so place worktrees outside the repo or add a local exclude before creating them.
openhuman/src/openhuman/tinyagents/mod.rs
Line 597 in d7a525a
Preserve turn history when injecting collect messages
Forwarding collect messages during the run injects new user turns after the model may already have emitted assistant/tool messages, but the turn result later persists only messages_since_last_user(&run.messages). If a collect arrives after the first model/tool round, that injected user becomes the "last user" and all earlier assistant/tool messages from the same turn are dropped from outcome.conversation, so transcripts and tool-call records lose part of the turn; track the original user index or exclude injected collect turns from the persistence cut.
Include parent working-tree changes in head worktrees
For base_ref = head, this passes HEAD as the git worktree add commit-ish (per git worktree add -h, the command takes <path> [<commit-ish>]), so the worker starts from the last commit and never sees uncommitted parent edits. In the normal coding flow where the parent has already modified files before spawning isolated workers, those workers analyze and patch a stale tree; either require a clean parent or copy/stash-apply the current diff into the worker worktree.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…inyagents-migration
There was a problem hiding this comment.
💡 Codex Review
Keep worker worktrees out of the parent checkout
When isolation="worktree" runs in a repo that does not already ignore .claude/, this creates the linked worktree inside the parent working tree. I checked the Git behavior with git worktree add .claude/worktrees/w1 followed by git status --porcelain, which reports ?? .claude/; so merely spawning a worker dirties the user's checkout and can surface the worktree directory in later diffs/commits. Put these worktrees outside the repo or ensure the containing path is ignored before creating them.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
sanil-23
added a commit
to sanil-23/openhuman
that referenced
this pull request
Jul 3, 2026
…tinyagents migration + versioned world-diff migration Rebased onto current main (c43f796) which merged tinyhumansai#4399 (finish TinyAgents migration) after tinyhumansai#4430 — leaving main red (Rust Quality) from two semantic conflicts this repairs: - agent_registry loader: BuiltinAgent.graph_fn is now Option<fn()->AgentGraph>; wrap frontend_agent/reasoning_agent in Some(...) (E0308). - orchestration graph: tinyhumansai#4399 retired SqlRunLedgerCheckpointer for the tinyagents crate SqliteCheckpointer (graph_checkpoints.db). Migrate graph/mod.rs + the ops resume test to SqliteCheckpointer::open, mirroring delegation.rs (E0432). Also addresses CodeRabbit on the world-diff unique index: move it into a user_version-gated one-time migrate() (was CREATE UNIQUE INDEX IF NOT EXISTS — a no-op on already-created stores), de-dupe legacy race rows, and reconcile terminal_state so it can't point at a deleted duplicate's mutation. Test extended with distinct mutations + stale terminal_state + user_version assertions.
YellowSnnowmann
added a commit
to YellowSnnowmann/openhuman
that referenced
this pull request
Jul 3, 2026
Resolves a semantic conflict introduced by upstream tinyhumansai#4399 ("finish TinyAgents harness migration"): that change retired `SqlRunLedgerCheckpointer` (delegation.rs already moved to the crate's `SqliteCheckpointer`) but left dangling references, so `main` at c43f796 does not compile on its own: - orchestration/graph/mod.rs — imported + constructed the removed `SqlRunLedgerCheckpointer`; now opens `SqliteCheckpointer` against a dedicated `graph_checkpoints.db` under `workspace_dir`, matching the delegation-graph pattern. - orchestration/ops.rs — test used the removed checkpointer; swapped to `SqliteCheckpointer::open` at the same path/thread. - agent_registry/agents/loader.rs — `graph_fn` became `Option<fn() -> AgentGraph>`; the frontend/reasoning builtins were left as bare fn items (E0308). Wrapped in `Some(...)`. - orchestration/graph/state.rs — fixed the stale intra-doc link. Merge is otherwise conflict-free. Full gate green: fmt, clippy -p openhuman, lib check, checkpointer persistence test, and the channels suite.
sanil-23
added a commit
to sanil-23/openhuman
that referenced
this pull request
Jul 3, 2026
…ce mock tinyhumansai#4399 raised RUNTIME_SNAPSHOT_TTL 2s -> 10s to fix a rebuild-stampede perf bug, but app_state_snapshot_degrades_runtime_service_status_failures ages the cache out with a 2.1s sleep — fine for the old 2s TTL, now < the 10s one, so the test reads a stale cached snapshot from a prior test and never sees the injected 'forced status failure' (asserts service degrades to Unknown). Deterministic; only surfaced now because main (with tinyhumansai#4399) doesn't compile until tinyhumansai#4431 repairs the tinyhumansai#4399/tinyhumansai#4430 conflict, so this is the first branch to actually run it. Fix: when OPENHUMAN_SERVICE_MOCK is active (a test-only env hook production service status already honors), never serve from or write to the process-global runtime cache — the mock's state changes between calls, so caching masks the injected value and poisons later non-mocked reads. Full config_auth_app_state_connectivity_e2e binary: 50 passed.
This was referenced Jul 3, 2026
Merged
sanil-23
added a commit
to sanil-23/openhuman
that referenced
this pull request
Jul 3, 2026
…n review fixes Rebased onto current main (8aab529), which already carries the tinyhumansai#4399/tinyhumansai#4430 compile reconciliation (graph_fn Some, SqliteCheckpointer migration, MCP catalog) via tinyhumansai#4433 — so this is slimmed to the fixes main still lacks: - app_state: bypass the runtime-snapshot cache when OPENHUMAN_SERVICE_MOCK is active. tinyhumansai#4399 raised RUNTIME_SNAPSHOT_TTL 2s->10s, so the mock-service e2e (which ages the cache with a 2.1s sleep) reads a stale snapshot and never sees the injected failure. Bypassing read+write under the mock fixes it (full config_auth_app_state_connectivity_e2e binary: 50 passed). - orchestration/ops: split steering out of seed_state into apply_cycle_steering, invoked only after the has_new_work guard, so no-op wakes don't consume a cycle tick and prematurely expire steering (CodeRabbit). - orchestration/store: user_version-gated world-diff migration — drop legacy non-unique index, de-dupe race rows, unique index, reconcile terminal_state (M3gA-Mind + CodeRabbit) + test. - orchestration/graph/state: agent-scoped cycle_id (avoid cross-peer collision) + migration-seam note (Codex + M3gA-Mind). - orchestration/schemas: ingest-health MAX(last_message_at) excludes pinned master/subconscious windows (CodeRabbit). - frontend: orchestration socket listeners via socketService.on/off (queues through startup/reconnect) + messages Retry -> refresh() (CodeRabbit/Codex). - docs: orchestration.md fenced-block language (markdownlint).
This was referenced Jul 3, 2026
M3gA-Mind
added a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Jul 3, 2026
Rework of tinyhumansai#4390 onto post-tinyhumansai#4399 code. tinyhumansai#4399 relocated the tool policy gate from the deleted session/agent_tool_exec.rs to the tinyagents adapter middleware (ToolPolicyMiddleware), which still emitted bare denial lines. - Ports the deleted policy_denial.rs into the adapter seam as src/openhuman/tinyagents/policy_denial.rs: a `PolicyDenial` renderer that turns each denial into a structured `Blocked / Reason / Workaround / relay` message (with the concrete "raise the channel's agent-access tier" or "permitted alternative / ask for approval" workaround) so the agent relays the block to the user instead of dead-ending. - Swaps the three bare `format!` denials in ToolPolicyMiddleware (channel_permission_block session-forbidden + per-call permission-too-low, and the pluggable-policy Deny/RequireApproval branch in wrap_tool) for PolicyDenial::render(). 5 unit tests for the renderer (session-forbidden with/without required, permission-too-low, policy-denied, approval-required).
M3gA-Mind
added a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Jul 3, 2026
…dex protocol (tinyhumansai#4116) Rework of tinyhumansai#4388 onto post-tinyhumansai#4399 code. tinyhumansai#4399 deleted the original home (agent/harness/memory_protocol.rs did not exist yet; the enforcement was to live in the turn engine); the turn loop now runs on the tinyagents crate, so this re-expresses the protocol as a crate middleware. - Ports the pure `MemoryProtocolTracker` state machine as src/openhuman/agent/harness/memory_protocol.rs: classifies each memory tool call (IndexRead / Write / IndexUpdate / Other) and, on a write, flags a missing dedupe read or a stale index (a prior write never followed by update_memory_md). - Adds `MemoryProtocolMiddleware` in the tinyagents adapter seam: before_tool captures the classified op by call id; after_tool advances the tracker on success and appends a `[memory-protocol]` corrective note to the tool result so the model is nudged back onto the cycle; after_agent warns when the run ends with a stale index. Registered outermost in assemble_turn_harness so its after_tool runs after the byte-cap truncation, preserving the note. - Fixes a stale classification from the original PR: `memory_search` is not a real tool — replaced with the actual dedupe-read search tools (memory_vector_search / memory_chunk_context / memory_hybrid_search). 9 unit tests (classification + tracker ladder) + 4 middleware integration tests (corrective note, SKILL.md doesn't close the cycle, consolidated memory_tree ingest = write, failed write doesn't advance). All green.
M3gA-Mind
added a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Jul 3, 2026
…ent (tinyhumansai#4254, backend) Rework of tinyhumansai#4413's backend onto post-tinyhumansai#4399 code. tinyhumansai#4399 deleted the original classification hook (session/agent_tool_exec.rs) and the ProgressReporter trait; the tool loop now runs on the tinyagents crate, so the hook is re-homed on the adapter seam. - New `src/openhuman/tool_status/` module: `classify(error_text, timed_out)` and `describe(class)` map a tool outcome to a `ClassifiedFailure` (class + category + plain-language cause + next action). Pure, unit-tested (16 tests: all classes, timeout/unknown/precedence, category mapping, recoverability). - `AgentProgress::ToolCallCompleted` + `WebChannelEvent` gain a `failure` field carrying that classification to the chat "View processing" timeline. - Core hook + latent-bug fix: the crate's `AgentEvent::ToolCompleted` carries only call_id + tool_name, so the bridge previously hardcoded `success: true` (a failed tool never surfaced). `ToolOutcomeCaptureMiddleware::after_tool` now classifies each result and writes a `call_id → (success, failure)` side-channel (mirroring the existing `ToolNameMap`); the bridge reads it to emit real `success` + `failure`. Absent entry falls back to `(true, None)`. - `progress_bridge` serializes `failure` onto the `tool_result` socket event and the run ledger. Backend only. The frontend consumer (chat timeline "why / next" render) and i18n keys are a companion — they rebase cleanly onto current `main` from tinyhumansai#4413 (only the Rust hook conflicted with tinyhumansai#4399), so they are not re-included here to avoid reverting unrelated locale drift. NOTE: the middleware-before-event ordering that populates the side-channel is crate-internal; the fallback keeps behavior no worse than before if the event is ever projected first. Needs runtime validation once tinyhumansai#4442 lands (drive a deliberately-failing tool and confirm `success:false` + `failure` on the wire).
M3gA-Mind
added a commit
to M3gA-Mind/openhuman
that referenced
this pull request
Jul 3, 2026
…ed in tinyhumansai#4445 tinyhumansai#4413 originally shipped both the tool-failure classification backend and the chat "View processing" render. The backend conflicted with tinyhumansai#4399 (harness migration) and has been reworked separately in tinyhumansai#4445 (tool_status module + ToolOutcomeCaptureMiddleware + WebChannelEvent.failure). This reduces tinyhumansai#4413 to just the frontend that consumes tinyhumansai#4445's `failure` field off the tool_result socket event, rebased cleanly onto current main. Kept (21 files): - chat timeline render: ProcessingTranscriptView (+test), ChatRuntimeProvider, chatService, chatRuntimeSlice (+toolFailure test), turnState. - i18n: conversations.toolFailure.* keys across all 14 locales (3-way unioned onto current main so no existing translations are reverted). Dropped (now owned by tinyhumansai#4445): tool_status/, agent progress/observability, middleware, event_bus, socketio failure plumbing, and the deleted-file conflict on agent_tool_exec.rs. Verified: pnpm typecheck clean, i18n:check parity 0 missing/0 extra, 13 frontend tests pass (chatRuntimeSlice.toolFailure + ProcessingTranscriptView). Stacked on tinyhumansai#4445 (merge that first — it emits the field this renders).
senamakel
pushed a commit
that referenced
this pull request
Jul 3, 2026
senamakel
pushed a commit
that referenced
this pull request
Jul 3, 2026
This was referenced Jul 3, 2026
Closed
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
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
tinyagentscrate as the orchestration/model/context/events/cost/subagent/embeddings library, adapter-first (prove parity, then delete), perdocs/tinyagents-full-migration-plan/.ModelRegistrywith capability gating; SDK-owned cross-route fallback + retry classification; tool-call argument streaming viaToolCallDelta; one unifiedModelCatalogsnapshot.StoreEventJournal+ status store; single typed owner for subagent lifecycle events; summarization provenance.CapabilityRegistrydiagnostics +agent.registry_snapshotintrospection RPC + agent projection;EmbeddingModeladapter + retriever facade (MemoryLoaded); reasoning/cache-creation token accounting + embedding cost.worktree_context.rs(CWD now fromWorkspaceDescriptor) andSqlRunLedgerCheckpointer(durable graphs use the crateSqliteCheckpointer);reliable.rsclassifiers extracted toerror_classify.rs.Problem
OpenHuman maintained a large bespoke agent harness (retry/fallback, context reducers, event bridge, checkpointer, subagent pipeline, registries) that duplicated capabilities now shipped by
tinyagents1.3.x. Issue #4249 tracks converging onto the crate as the single source of truth for orchestration, keeping only product policy (prompts, registry, security/approval, credentials, UX projections), and deleting the superseded OpenHuman files.Solution
99-deletion-ledger.md. Deletions that would regress behavior the mock suite cannot validate (e.g.ReliableProvider's model-fallback/key-rotation/streaming-failover for non-turn callers) are explicitly left gated with documented rationale rather than forced.RUST_MIN_STACK=16777216 --test-threads=1runner (scripts/test-rust-with-mock.sh).agent.registry_snapshot).Submission Checklist
.github/workflows/pr-ci.yml; the conformance suites are green and changed-line coverage is reconciled against the CI gate during review (the diff spans many adapter-first slices each landed with tests).agent.registry_snapshotis a read-only diagnostic surface).Closes #4249in Related.Impact
SqliteCheckpointerat{workspace}/graph_checkpoints.db; nothing else read the old run-ledgergraph_checkpointstable, so pre-swap in-flight graphs expire and orphaned tasks reconcile at boot. Session dual-write is behindOPENHUMAN_SESSION_DUAL_WRITE(default off). Toolchain pinned to Rust 1.96.1 for crates.io rusqlite 0.40 / libsqlite3-sys 0.38 (cfg_select!).WorkspaceDescriptor(parity-tested). Registry diagnostics fail closed pre-dispatch.Related
reliable.rsReliableProviderstruct — needs routing non-turn callers through the crate;tool_filter.rs/tool_prep.rsownership flip;ThinkingForwarder— blocked on crateToolDeltalackingtool_name;progress_tracing.rsjournal-parity;CostBudgetMiddlewareUsageRecorded de-dup; 04.2 session-store read cutover on live user data).AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— no frontend source changes in the migration slices.pnpm typecheck— no frontend source changes in the migration slices.scripts/test-rust-with-mock.sh(RUST_MIN_STACK=16777216 --test-threads=1).GGML_NATIVE=OFF cargo check --manifest-path Cargo.tomlclean on Rust 1.96.1.app/src-taurisource changes in the migration slices.Validation Blocked
Behavior Changes
Parity Contract
Duplicate / Superseded PR Handling