Audit remediation: correctness, durability, performance, docs, and module structure#12
Merged
Conversation
Treat the RedactingSink as a security boundary: if the event cannot be serialized, redacted, and rebuilt, drop it rather than forward a record that may still contain unredacted secrets. Add a fast path that forwards the original record unchanged when no secrets are configured. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
The `required` check was nested inside the `properties` branch, so a schema declaring `required` without listing `properties` skipped validation and accepted calls that omitted mandatory arguments. Lift the `required` enforcement out so it fails closed independently of `properties`. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
The lexical path gate dropped leading `..` components, so a relative candidate like `ws/../../ws/secret` collapsed back onto the root name and was admitted even though it resolves into a same-named sibling outside the workspace. Anchor relative candidates and roots to the current directory and preserve escaping `..` during normalization so the gate fails closed. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…shared policy The strict blueprint gate (`CapabilityResolver::bind_blueprint`) routed every non-subgraph/router node through a model check, so `subagent` `agent` references were never validated and `repl_agent` `script` references were validated by no path at all — both admitted unregistered capabilities. Extract one shared kind-to-reference policy (`CapabilityResolver::classify_reference` + `reference_allowed`) and route all three binding gates through it — the compiler blueprint gate and both `Resolver` paths — so they cannot drift. Move the agent allowlist into `CapabilityResolver`, add a `scripts` allowlist and a `ComponentKind::Script` so scripts can be registered and bound. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
`parse(&[])` underflowed `tokens.len() - 1` in the token cursor and panicked. Reject an empty token slice up front with a parse error, since a well-formed stream always ends with an `Eof` sentinel. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Diagnostic::render panicked when a label span pointed past the end of the source: `caret_start` could exceed `line_end`, tripping `clamp`'s `min <= max` precondition (and risking an out-of-bounds slice). Clamp the caret range into the line's byte range, mirroring `SourceFile::snippet`. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Buffer the raw SSE byte stream as `Vec<u8>` and only lossily decode complete newline-terminated lines, so a multi-byte UTF-8 character split across two network chunks is reassembled before decoding instead of being corrupted into replacement characters. Drain any leftover buffered bytes at end-of-stream so a final `data:` event sent without a trailing newline (and without a `[DONE]` sentinel) is still processed. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…them
A mid-stream `{"error": ...}` SSE payload deserializes cleanly as an
all-defaults `ChatCompletionChunk`, so it was folded in as an empty chunk and
silently dropped. Parse each `data:` payload as a generic value first and, when
it carries an `error` object, emit a terminal `ProviderFailed` (with the
provider message and code) and stop the stream.
Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…variant round-trips
Both enums were internally tagged (`tag = "type"`). Internal tagging cannot
serialize newtype variants wrapping a non-map payload: `ModelStreamItem::Failed`
(a `String`) errored, and `StreamChunk::Values(json!(null))` and other scalar /
array `Value` payloads corrupted (`null` became `{}`). Switch both to adjacent
tagging (`{"type": …, "content": …}`), which keeps the ergonomic tuple variants
and all existing call sites unchanged while making every variant round-trip.
Add serde round-trip tests covering every variant of both enums.
Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Some OpenAI-compatible backends omit the tool-call `index` field, which
collapsed every parallel call onto slot 0. Make `index` optional and, when
absent, correlate fragments by id (new id opens a slot, known id reuses it,
id-less continuations append to the last slot). Also enumerate slots before
filtering in `into_response` and key the synthetic `tool-{slot}` fallback id to
the slot position so streamed delta ids always match the final call ids.
Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Zero-argument tool calls from OpenAI-compatible backends arrive with an empty
arguments string. Treat a blank arguments payload as `{}` in both the unary and
streamed reconstruction paths instead of failing it as malformed JSON.
Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
The provider never set any reqwest timeout and ignored ModelRequest::timeout_ms, so a stalled endpoint could hang a call forever. Give the shared client a sane connect timeout, honor an explicit per-request timeout_ms on both unary and streaming calls, and apply a default overall timeout to unary calls (streaming stays uncapped so long streams are not truncated). Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
o-series reasoning models (o1/o3/o4) reject `max_tokens` and require `max_completion_tokens`. Route the request's output-token cap to whichever field the target model accepts, add the new wire field, and reserve it from provider_options passthrough. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
translate_message flattened every message to its text, so image and JSON blocks were silently dropped even though the profile advertises image_in. Render user messages with non-text blocks as OpenAI content-parts (text + image_url), serialize JSON blocks into text, and fail closed with a validation error on provider-extension blocks that have no OpenAI representation. Text-only messages keep their plain-string wire shape. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
When a completed response carried usage only on its message (top-level usage None) and no UsageDelta streamed, finish overwrote message.usage with None. Merge the response, message, and streamed usage instead — prefer a present value, never overwrite a known usage with None — and keep both fields in sync. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Checkpoint and run ids were built from a process-local AtomicU64 that restarts at 0 in every new process, so a resumed thread restarted after a crash re-minted ids it had already used. That corrupts the parent-lineage map (prune could delete a live record's ancestor) and can make ResumeTarget::Checkpoint load the wrong record. Add a process nonce to harness/ids and expose new_run_id/new_checkpoint_id (`<prefix>-<nonce>-<seq>`), collision-free across restarts while next_seq keeps them ordered within a process. The graph executor now mints ids through those helpers instead of its own bare counter. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
The three checkpointer backends disagreed on `get(thread, Some(id))` when an id appeared more than once: the in-memory backend returned the first match while file and sqlite returned the last. Pin one semantic — last-write-wins, matching the append-only history and `get(None)` — fix the in-memory backend to `rfind`, and assert it in the checkpointer conformance suite so all backends stay interchangeable. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
The step's goto map was keyed by node id, so two Send activations of the same node that each returned a Command::goto collided: the second clobbered the first, and both branches then routed to the survivor's target — losing one branch's routing and duplicating the other. Key the map by active-set index instead, and pass each activation's own goto targets into `route`. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…rriers Interrupt and failure boundaries lost durable state three ways: - B2: the checkpoint only stored next_nodes (Vec<NodeId>), so every pending Send worker re-ran on resume with ctx.send_arg == None — breaking the map-reduce fanout the resumable-failure feature exists for. - B3: successors of branches that completed before the pause were never scheduled, so in parallel [a->x, b] where b interrupts, x silently never ran after resume. - B4: barrier (waiting-edge) arrivals were run-local, so a join whose first predecessor arrived before an interrupt could never satisfy its precondition after resume. Add pending_activations (node + send_arg) and barrier_arrivals to the Checkpoint, both #[serde(default)] for back-compat (old checkpoints load and fall back to next_nodes / an empty barrier set). At the interrupt/failure boundary the executor now routes the completed lower-index branches into their successors, appends the pending tail with args preserved, and persists the accumulated barrier arrivals; resume rebuilds activations and seeds the barrier map from the checkpoint. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
execute_run always started with parent_checkpoint = None, so the first boundary checkpoint written after a resume had no parent. That orphaned the pre-interrupt history: get_state_history stopped at the resume point and prune(keep_last) deleted the ancestors a live record still depended on. Thread the loaded checkpoint id into the run as the initial parent so the lineage spine stays connected. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
An embedded child graph that paused on a human-approval interrupt had its partial state treated as a completed output: the subgraph node returned NodeResult::Update(child.state) and the parent kept executing, so the interrupt was unreachable through the parent's resume API. Detect an interrupted child execution and re-emit its interrupt as the parent node's interrupt so the parent pauses too. On a resumed activation the child is now resumed from its own checkpoint (rather than re-run from scratch), carrying the parent's resume value down. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
A subgraph child runs under the parent's thread id but a distinct namespace, yet no backend filtered lookups on the stored namespace: a parent resume (or state inspection) could load the child's checkpoint, resuming a same-named node with the wrong role or failing with MissingNode. Add a default get_scoped(thread, id, namespace) to the Checkpointer trait (composed from list+get, so all three backends inherit it), route get_tuple through it, and scope the executor's resume/update/fork lookups to the graph's own namespace. This also completes nested resume: resuming the parent now resumes the child from its own namespaced checkpoint. Pinned in the checkpointer conformance suite. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Errors raised at the step boundary — a reducer merge, route resolution, or checkpoint persist — used `?` and unwound out of the run loop without calling fail_run, so observers saw the run stuck in Running forever. Route each boundary error through a fail_and_return helper that records the Failed status first. A failure-boundary persist error no longer replaces the original node error either: it just drops the resumable checkpoint reference and keeps reporting the node error. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
resume_from emitted GraphEvent::CheckpointSaved when it *loaded* a checkpoint, so durability observers counting saves double-counted every resume as a persist. Add a CheckpointRestored event for the read side and emit it on load instead. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
A command node routes dynamically via the Command it returns at runtime and has no static successors, so route() resolves it as a sink. Attributing a manual update_state write to such a node persisted an empty next_nodes and silently rendered the thread non-resumable. Reject it at write time with a clear Graph error instead. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Items complete out of order under buffer_unordered, so FailFast broke on the first error to *complete* and could return a later item's error instead of the first failure in input order (as documented). Track the lowest-index error and only stop once every earlier item has resolved, then cancel remaining work. Also corrects the stale module doc that claimed ordering came from `buffered`. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…ent_loop, observability These modules had no README.md despite being complex, multi-file public surfaces (onion middleware composition, provider translation/SSE decoding, the default agent loop, durable observability journals/sinks). Document design, public surface, and operational constraints for each, consistent with the repo's per-module README convention. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Document the superstep executor's parallel/retry/resumable-failure semantics, the Checkpointer trait and its file/sqlite/in-memory backends, and the channel-per-field reducer model, matching the repo's per-module README convention. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…graph, testkit Document durable graph observability (journals/sinks/Langfuse export), the managed child-work orchestration tool surface, graph-in-graph subgraph embedding, and the graph-level test doubles/assertions/conformance suites. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Document the graph runtime's module map and how authoring, execution, persistence, observability, recursion, and testing pieces fit together, linking to each submodule's own README. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
design.md had grown to 807 lines. Split the RLM feature map, CodeAct loop, examples, Rhai embedding plan, Python compatibility backend, safety, events, diagnostics, testkit, and milestones into operations.md, linked from design.md and the module README. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
.gitmodules pointed at an SSH URL (git@github.com:...), which requires SSH key setup even for read-only clones/CI. Switch to https and document the wiki/ submodule (what it is, how to fetch it, and that it should not be touched incidentally) in CONTRIBUTING.md. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…DOT ids - registry::AliasBinding was defined and used in the public RegistrySnapshot::aliases field but never re-exported from registry::mod or the crate root, so downstream code could observe it but not name its type. Re-export it alongside the rest of the registry surface. - Re-export the ModelCatalog family (ModelCatalog, ModelCatalogEntry, ModelCatalogSnapshot, ModelCatalogSource, ModelCapabilities, ModelPricing) at the crate root for parity with registry::mod, which already exposed them. - register_descriptor's doc comment claimed it was "reserved" for Store and Agent kinds, but Agent has its own dedicated register_agent method that does not call register_descriptor; the doc now names the actual fallback kinds (Store, Script, Middleware, Checkpointer, TaskStore, Listener). - RegistrySnapshot::to_dot interpolated component ids directly into Graphviz DOT quoted strings without escaping backslashes or double quotes, so a component id containing a `"` could break out of its quoted string and inject arbitrary DOT syntax. Escape both characters and add a regression test. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…retryable Fold the per-attempt retry decision into a single `RetryPolicy::should_retry_error` (classification + attempt cap) used by both the agent loop and RetryMiddleware, so the retry logic that previously carried the same off-by-one in two places now lives in exactly one place and cannot drift. ModelFallbackMiddleware now stops switching models on a non-retryable error (auth/validation/schema) instead of burning every backend on a failure that will recur identically. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…faces Extract the `depth + 1` / `max_depth` fail-closed check into a single `RunConfig::checked_child_depth`. SubAgent, SubAgentTool, and the REPL sub-run builtin previously hand-rolled the same guard three times; consolidating removes the drift that let one path bypass the depth limit. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Remove the graph-local `SEQ` AtomicU64 in `compiled` and draw every graph id (graph-*, subagent-*, testkit run ids) from `harness::ids::next_seq`, the same process-unique counter todos/goals/orchestration already use. Checkpoint ids keep delegating to `new_checkpoint_id` (restart-collision-free), so the durability fix is preserved. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Collapse five identical `now_ms()` copies (goals store, graph + harness observability, langfuse, store) into one `harness::ids::now_ms`, alongside the existing process-time primitives. Every timestamp now flows through one epoch/`unwrap_or(0)` implementation. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
Collapse the duplicated HTTP tails of invoke, stream, and list_models into `authorized` (bearer header), `send_checked` (send + transport-error + non-2xx status decoding, returning the raw response), and `post_json` (chat POST with timeout selection). Auth, timeout, and error/status handling now live in one place instead of being hand-repeated three times. Behavior is unchanged. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
… on error Replace seven near-identical stack-runner bodies (before/after agent, model, tool + on_tool_delta) with one `run_stack_hook!` macro that centralizes the event bracketing and on_error fan-out. The macro — and the two wrap-onion handlers — now emit `MiddlewareCompleted` on the error path too, so a hook that returns `Err` can no longer leave a dangling `MiddlewareStarted` with no matching `Completed` in the event stream. Adds a regression test and documents the balance guarantee in the module README. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
compiled/mod.rs had grown to ~2k lines mixing the superstep executor, state-inspection/time-travel API, and step-routing logic in one impl block. Split mechanically into executor.rs (run/resume/execute loop), state_api.rs (get_state/update_state/fork_state), and routing.rs (route/route_completed/interrupt-durability), keeping mod.rs for shared helpers and the builder/configuration impl. No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
library/mod.rs mixed 14 built-in middleware across 4 unrelated concerns in ~1450 lines. Split mechanically into resilience.rs (retry/timeout/fallback/rate-limit), budget.rs (token/cost tracking and enforcement), tool_policy.rs (allowlisting, policy, dynamic/ contextual selection, human approval), and observe.rs (structured- output validation, dynamic prompt, redaction, tracing). No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…call agent_loop/mod.rs had grown to ~1160 lines mixing public invoke* entry points, the ~400-line run_loop body, and model-invocation dispatch in one impl block. Split mechanically into entry.rs (invoke*/invoke_streaming*/drive), run_loop.rs (the core loop body and response-cache decision), and model_call.rs (cache-aware retry/ fallback dispatch plus the ModelBaseCall/ToolBaseCall impls). No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
providers/openai/mod.rs had grown to ~1360 lines mixing HTTP transport, request/response conversion, and SSE stream parsing. Split mechanically into transport.rs (OpenAiModel construction, presets, request building, the ChatModel impl), convert.rs (message/response translation), and sse.rs (SseState/OpenAiStreamAcc/sse_next). Public API (OpenAiModel) re-exported from mod.rs unchanged. No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…uthoring builtins.rs had grown to ~1175 lines covering five jobs (engine construction, single-shot capability calls, batched variants, graph authoring, and shared helpers). Converted to a builtins/ module directory and split mechanically into capabilities.rs (model_query/ tool_call/agent_query/graph_run), batched.rs (the *_batched variants), and authoring.rs (graph_define/validate/compile/diff/register), with mod.rs keeping engine construction and shared helpers. External path (session::builtins) unchanged. No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
CapabilityResolver, ReferenceClass, PrimaryReference, DEFAULT_NODE_KINDS, bind_capabilities, and bind_capabilities_with_registry move from compiler.rs into a new capability_resolver.rs sibling of resolver.rs, which already shares this binding policy (Resolver wraps a CapabilityResolver internally) — the binding-policy triplication the audit flagged is easier to see and keep in sync when the type lives next to its other consumer instead of buried in the compiler. Public API path updated at the crate root (lib.rs re-exports from the new module); internal callers (resolver.rs, registry/capability, tests, examples) updated to the new path. No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
test.rs had grown to ~1568 lines covering the whole pipeline in one file. Converted to a test/ module directory and split by pipeline phase: lexer, diagnostics, parser, compiler, extended_grammar, capability_binding, graph_materialisation, registry_binding, resolver, and provenance_diff_testkit, with mod.rs keeping the shared SUPPORT_AGENT fixture and module declarations. No behavior or coverage change — same 76 tests, same assertions. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…dule dirs
Both flat files violated the dir/{mod.rs,types.rs,test.rs} convention.
- harness/no_progress.rs -> no_progress/{mod.rs,types.rs,test.rs}
(ToolAttempt/NoProgress/LadderState/NoProgressTracker type defs
split from the escalation-ladder impl).
- harness/observability/langfuse.rs -> langfuse/{mod.rs,types.rs,test.rs}
(LangfuseAuth/LangfuseTraceConfig/LangfuseClient type defs split
from the client impl and payload helpers).
- graph/observability/langfuse/tests.rs renamed to test.rs to unify
naming with the harness module and the repo-wide test.rs convention.
Public API paths (re-exported from harness::no_progress and
harness::observability) unchanged. No behavior change.
Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
providers/mod.rs carried ~275 lines of MockModel constructors, the ChatModel impl, and token-estimation helpers alongside the module's own wiring/doc comment. Moved into mock.rs, leaving mod.rs with just module declarations and shared imports. MockModel's type definition already lived in types.rs per convention. No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…brary/ middleware/mod.rs hosted five full middleware impls (Logging, MessageTrim, ContextCompression, PromptCacheGuard, UsageAccounting) alongside the module's own stack-runner wiring, while every other built-in middleware already lived in library/. Moved MessageTrim/ContextCompression/PromptCacheGuard into a new library/context.rs and Logging/UsageAccounting into library/observe.rs (they fit the existing observation concern). mod.rs now holds only AgentRun helpers and the stack runner. No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
embeddings/mod.rs embedded a full HTTP provider (OpenAiEmbeddingModel)
as an inline `mod openai { ... }` block instead of its own file. Moved
to openai.rs, declared via `mod openai;`, mirroring the convention
used by harness::providers::openai. No behavior change.
Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
workspace/types.rs mixed plain type definitions with the allows/enforce path-gating logic and its lexical path-normalization helpers — the security-relevant behavior the module exists to provide. Moved into policy.rs, leaving types.rs with just WorkspaceDescriptor's fields and the WorkspaceIsolation trait. No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
goals/types.rs mixed the ThreadGoal/ThreadGoalStatus/GoalProgress data model with active_goal_context_block, a prompt-rendering function — presentation, not state. Moved to prompt.rs. No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
…cit list parallel/mod.rs did `pub use types::*;`; replaced with an explicit list (FailurePolicy, ItemOutcome, ParallelOptions, ParallelOutcome) so the module's public surface is discoverable without reading types.rs. No behavior change. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
# Conflicts: # src/harness/middleware/mod.rs
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (198)
Comment |
23 tasks
… compiles The builtins split left the shared *_impl helpers private in the new capabilities/batched/authoring submodules; under --all-features (repl) the cross-module calls in mod.rs and batched.rs failed to resolve. Local gates ran without --all-features, so CI's clippy-all-features job caught it. Claude-Session: https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: a4917a6e63
ℹ️ 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".
Comment on lines
+248
to
+251
| *self | ||
| .pending_reservation | ||
| .lock() | ||
| .unwrap_or_else(std::sync::PoisonError::into_inner) = estimated; |
There was a problem hiding this comment.
When one BudgetMiddleware instance services concurrent runs, each before_model overwrites this single pending_reservation; after_model/on_error then subtracts whichever amount was written last rather than the amount this call reserved. Two overlapping calls with different estimates can leave reserved_input_total permanently inflated after both finish, causing later requests to be rejected even though no work is in flight. Store reservations per call/run or release using a reservation token instead of shared middleware state.
Useful? React with 👍 / 👎.
| @@ -1 +1 @@ | |||
| Subproject commit 9321604d35fa34b53dc5f0383f70ae1b7410ac15 | |||
| Subproject commit b02f58269a4fc8742571f92e191ec893e9890eb0 | |||
There was a problem hiding this comment.
The repo-level AGENTS.md says the wiki submodule should not be edited as part of unrelated work and that pointer updates must be committed separately. This audit-remediation commit also changes library, test, and documentation files, so bundling the wiki gitlink bump here violates that review constraint and makes the wiki publish harder to review or revert independently.
Useful? React with 👍 / 👎.
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
1 participant
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
Full-codebase audit (2026-07-03) and remediation: ~60 verified correctness bugs, performance fixes, docs refresh, and structural cleanup, in 106 focused commits. Every fix ships with a test capturing its failure scenario; every work package passed an independent diff review.
Correctness
RedactingSinkno longer forwards unredacted records on redaction failure; tool schema validation rejectsrequired-without-properties; workspace path gate defeats leading-..sibling escape;bind_blueprintvalidatessubagent/repl_agentreferences through one shared policy (was: unregistered agents compiled fine); parser/diagnostic panics on empty input and out-of-range spans fixed.max_completion_tokensfor o-series, image content parts instead of silent drops.Sendargs), completed-branch successors, and barrier arrivals; parent lineage survives resume; subgraph interrupts propagate; namespace-aware checkpoint lookups; boundary errors fail the run instead of leaving itRunning.BudgetTrackerreservations atomic, per-run, fail-closed on poisoning, and released on model-call errors; REPL agent-call limit independent of model-call limit.blueprint_diffcovers all Blueprint fields (command/sends/retry/metadata + graph-level); routing conflicts and duplicate channels/graph ids are diagnosed; real line/col in parse errors; output limit enforced during evaluation.Performance
Per-run tool-schema caching, rolling concurrency window, in-place channel merges, linear summarization trimming, LRU-bounded response cache with stream-hashed keys, batched SSE draining, checkpoint IO on the blocking pool, atomic bulk memory replace, bounded middleware recorders and in-memory journals (durable offsets preserved across eviction), slimmer Langfuse batches with 207 partial-failure surfacing.
Docs & structure
openaiCargo feature from lib.rs/README; fixed README version/clone URL/dependency claims; rewrote AGENTS.md/CLAUDE.md to match the real tree; refreshed ROADMAP and spec; relocatedgoal.mdtodocs/sdk-gaps.md; split all five >500-line docs; wiki submodule now uses https and is documented.now_ms, shared openai transport, factored middleware stack-runners).compiled/{executor,state_api,routing},middleware/library/{resilience,budget,tool_policy,observe,context}, agent_loop, openai provider, REPL builtins, language compiler/tests); added module READMEs acrosssrc/harness/andsrc/graph/.origin/main(Add MicrocompactMiddleware: clear older tool-result bodies #9 MicrocompactMiddleware, feat(observability): Langfuse Agent-Graph-view metadata + host span-metadata injector #11 Langfuse graph exporter keys), porting the Microcompact impl into the newmiddleware/library/context.rslayout.Known follow-ups (intentionally deferred)
&mut RunContextrestructuring), true fire-and-forgetDurabilityMode::Async, Langfuse start-time capture, and the remaining low-priority memory-growth items (InMemoryAppendStore/InMemoryGraphStatusStore eviction, per-thread lock-map cleanup, REPL history cap, cold-path scans) — tracked in the audit report.RunLimits::max_concurrencywas removed (never enforced anywhere) — public API removal, no deprecation cycle.Test plan
cargo fmt --check,cargo clippy --all-targets -- -D warnings,cargo test(861 lib tests + 56 integration binaries + doctests) — all green, including after the origin/main merge.https://claude.ai/code/session_01Ca2v3JeHcZvpYYsEX4WpcN