Update - #3
Merged
Merged
Update#3
Conversation
- Use a single tokio::signal::unix::Signal with recv() for both Ctrl+C presses, eliminating the drop/recreate gap where signals could be lost. - Add 15s timeout on runtime.shutdown() as a safety net so the process always exits even if background tasks hang. - Non-Unix platforms fall back to the single Ctrl+C handler.
) (nearai#4873) * test(slack): re-home approval→auth→final-reply delivery e2e (nearai#4847) Re-adds slack_approval_then_auth_resume_completes_without_second_approval, removed from nearai#4839 (commit b847f51) as born-broken due to a test-harness gap. With nearai#4843's single-flight delivery guard now in main, the production deliver_final_reply loop survives BlockedApproval → BlockedAuth → Completed on one delivery loop. Adds the BlockApprovalThenAuth TurnMode variant and a transition_blocked_approval_to_blocked_auth coordinator helper to stage the two-gate sequence. Test-only; no production changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(slack): drive approval→auth hop through the approval caller + deterministic completion (nearai#4847) Addresses PR nearai#4873 review: - Route the approval→auth transition through RecordingApprovalInteractionService::resolve (mode-aware on BlockApprovalThenAuth) instead of a coordinator backdoor; post the approve event and assert exactly one approval-service request. list_pending only surfaces the approval gate while the run is BlockedApproval. Removes the transition_blocked_approval_to_blocked_auth backdoor. Satisfies Test-Through-the-Caller. - complete_blocked_run transitions BlockedAuth→Completed in one locked mutation (no observable Running), removing the window where the delivery loop posts the working indicator and flakes the message-count assertion. - Drop the mid-test drain after the approve event: delivery loops are awaited by drain_immediate_ack_tasks, so draining while the run is still BlockedAuth blocked L1 until max_wait, leaving no loop to deliver the final reply. Poll for the async auth prompt instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…d run origin (nearai#4836) * feat(turns): runtime-context communication slice types and rendering Foundation for nearai#4828: TurnRunOrigin enum, run_origin threaded through SubmitTurnRequest/TurnRunState/LoopRunContext, CommunicationRuntimeContext (connected channels, delivery target, origin) rendered in the runtime context section. communication=None renders byte-identical to the nearai#4795 baseline, so existing fingerprints are unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(reborn): wire communication runtime-context slice and run origins Wires nearai#4828 end-to-end: - CommunicationContextProvider trait stamped at loop spawn; composition provider reads outbound preferences (2s timeout, degrades to Unknown, never blocks loop start); delivery_tools_visible derived from the visible capability surface - run_origin set at submit sites (WebUI chat, product inbound with adapter id, conversation inbound deriving trigger vs product from adapter kind) and persisted on TurnRunRecord so it survives restart - DeliveryTargetState::SetUnresolved keeps a stored preference from rendering as "none set" when no target provider registry is wired - extends existing loop_driver_host and default_system_prompt tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): apply code-review fixes to communication context slice - type adapter identity as ProductAdapterId end-to-end; typed is_trusted_trigger() predicate replaces string comparison; replay path carries the real adapter kind instead of an empty string - run_origin becomes a CommunicationContextProvider parameter so the provider returns a fully populated context (no post-mutation) - scheduled-trigger no-delivery-target warning renders unconditionally; only the tool-name sentence is gated on tool visibility - sanitize adapter/display strings at prompt render time - wire connected channels from the lifecycle facade behind an explicit pre-nearai#4778 channel-surface predicate (renders 'none' until the ProductAdapter surface projection lands); 500ms fetch timeout - provider unit tests plus run-origin assertions in existing contract and loop-driver tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: ProductContextFactory design spec (nearai#4828 follow-up) Design for ironclaw_product_context: a single ingress resolver that owns turn-origin/surface/owner classification, replacing the scattered run_origin plumbing. Generic ProductTurnContext persisted on the turn; live account state stays in the composition provider. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(reborn): address PR nearai#4836 review — origin integrity, honest channel state, concurrency - ScheduledTrigger origin now requires a Trusted binding policy, not just adapter text; untrusted inbound with adapter_kind "trigger" records ProductInbound (regression test added) - connected-channels renders Unknown (not a false "none") while channel classification is unavailable pre-nearai#4778 - outbound-preferences and lifecycle fetches run concurrently under one 500ms budget instead of sequential 500ms each - surface-state read logs on error instead of silently dropping it - is_trusted_trigger compares against ironclaw_triggers::TRIGGER_TRUSTED_ADAPTER_KIND - channel names sanitized at prompt render; child runs inherit parent origin - CommunicationContextProvider trait documented; mock captures all args - tests: WebUI origin in model request, ordinary inbound origin Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: ProductContextFactory implementation plan (nearai#4828 follow-up) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(turns): generic ProductTurnContext types, replacing TurnRunOrigin Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(product-context): ingress resolver crate (resolve_inbound/resolve_web_ui) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(turns): carry product_context on turns, drop run_origin Replace the deleted `TurnRunOrigin` enum and its `run_origin` field with `product_context: Option<ProductTurnContext>` on all four structs (`SubmitTurnRequest`, `TurnRunState`, `TurnRunRecord`, `LoopRunContext`) and the in-memory run record in `memory.rs`. - Rename `LoopRunContext::with_run_origin` → `with_product_context` - Replace `CommunicationRuntimeContext::run_origin: Option<TurnRunOrigin>` with `product_context: Option<ProductTurnContext>`; update `render_model_content` to match on `TurnOriginKind` instead of enum variants; update `CommunicationContextProvider` trait signature - Swap `pub use crate::TurnRunOrigin` → `pub use crate::ProductTurnContext` in `run_profile/mod.rs` - Rewrite origin serde tests in `agent_loop_host_contract.rs` to cover `ProductTurnContext` round-trips; rename old `run_origin` field tests - Rewrite `filesystem_turn_state_store_persists_run_origin_…` snapshot round-trip test using `ProductTurnContext` - Fix imports in all test files; zero `TurnRunOrigin`/`run_origin` references remain in the crate Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor: resolve product_context at the four ingress submit sites Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(reborn): migrate test mocks/fixtures to product_context Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: migrate remaining loop_support/host_runtime test literals to product_context Completes the run_origin → product_context field rename across the remaining test crates; fmt + clippy --all --all-features clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(product-context): address PR review — skip discarded fetch, thread surface_type, validate-on-deserialize, tests, docs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(product-context): seal ProductTurnContext construction + collapse resolver inputs - ProductTurnContext is #[non_exhaustive] with a new() constructor, so external crates can no longer mint a ScheduledTrigger origin via struct literal; the resolver is the single intended mint point (turn submission stays a trusted boundary — honest framing, not a hard seal) - resolver takes one InboundClassification {TrustedTrigger,TrustedOther,Untrusted} instead of a separable (TrustLevel, is_trigger_adapter) pair, so a mismatched pair is unrepresentable; TrustLevel removed - call sites collapse their policy+trigger signal into the single classification Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(runtime-context): render run origin from LoopRuntimeContext, not the communication provider Moves the persisted ProductTurnContext onto LoopRuntimeContext (sibling of the live communication state) and drops it from CommunicationRuntimeContext and the CommunicationContextProvider signature. Origin/surface/owner now render directly from the run context — independent of whether a communication provider exists — and provider fakes no longer carry state they don't own. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(runtime-context): degrade model-unsafe labels instead of failing the prompt External labels (channel name, delivery target display/channel, adapter) are now rendered via model_safe_label: sanitized, then validated against the same model-safe-text policy the prompt bundle enforces. A legitimate label that would still trip that policy (e.g. a channel named #secret-alerts / "authorization") degrades to a placeholder so it can never fail prompt construction — the slice degrades, not the whole bundle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(product-context): address 2nd-round PR review — docs, channel-list cap, surface tests, trigger-predicate layering Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(runtime-context): clarify scheduled-trigger warning requires known delivery state The no-delivery warning renders only when delivery state is known NoneSet (which requires the communication slice). When communication is absent the delivery state is unknown, so no warning is emitted — a target may exist and claiming otherwise would be wrong. Production triggered runs carry the slice. * fix(context-slice): address PR review — dedup, surface reuse, tests, docs - loop_driver_host: compute delivery_tools_visible from the captured visible_capabilities() surface instead of a second surface_state.current() scan; drop the redundant warn! degradation arm (JYDXp) - runtime_context: extract render_origin_line() helper; both render branches share one origin-line path, warning logic stays comm-gated (JYDXv) - inbound: add trusted_non_trigger_adapter_records_inbound_origin caller test for the TrustedOther -> Inbound branch (JYDXq) - origin: add deserialize_rejects_overlong_run_origin_adapter serde boundary test; point doc comment at resolve_inbound/resolve_web_ui as the mint points, new() as the low-level constructor (JYDXr, JYDXt) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * perf(context-slice): overlap advisory communication fetch with loop start The communication slice is advisory and must never block loop start, but the provider was awaited inline before prompt construction — adding up to the provider's 500ms timeout budget serially to every run (JYDXo). Make overlap a property of the provider contract instead of an ad-hoc spawn: - CommunicationContextProvider::communication_context (async, takes delivery_tools_visible) -> begin_communication_context (sync, returns a CommunicationContextFetch handle). delivery_tools_visible is surface- derived, not a fetch input, so it leaves the fetch signature entirely. - New CommunicationContextFetch: the provider drives the backend lookups concurrently (the production impl spawns); the caller joins later via resolve(delivery_tools_visible), which stamps the surface-derived flag onto the resolved context. - loop_driver_host starts the fetch right after run_context is bound, so its latency + timeout budget overlaps gate/dispatcher construction and capability-surface computation. In the common case the fetch is already resolved by prompt-build, adding ~0ms to the critical path; worst case is bounded by the same 500ms budget, now spent in parallel. No coverage loss vs the fail-fast alternative and no stale-cache risk. Provider unit tests updated to begin(...).resolve(flag); the host-level "capability in surface -> flag true" test now asserts the rendered tool-hint warning (end-to-end through resolve) instead of inspecting a recorded provider argument that no longer exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * obs(context-slice): debug! when trigger run renders with no comm slice The no-delivery safety warning only fires in the communication-present render branch. Production triggered runs are expected to always carry a communication slice, so a ScheduledTrigger reaching the origin-only branch means the warning is silently skipped — an invariant breach. Emit a debug! there so the breach is observable without changing rendered output (render stays correct: asserting "won't be delivered" without known delivery state would be wrong). Enforcing the invariant on the trigger composition path is tracked as a follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(context-slice): address PR nearai#4836 review — WebUI owner, observability, tests - runtime: turn_scope_for now carries the runtime's explicit owner via new_with_owner(Some(actor_user_id)); WebUI chat runs persist TurnOwner::Personal{user} instead of SharedAgent. Document send_user_message as a WebUI-only origin path (resolve_web_ui); non-WebUI ingress must resolve its own origin. New regression send_user_message_persists_personal_owner_for_webui. - communication_context: match JoinError explicitly (debug! + // silent-ok:) instead of .ok().flatten(); emit debug! on the timeout arm and both Err degradation paths before collapsing to Unknown; keep the plain (skipped) lifecycle None arm silent. - product_workflow: add shared_user_message_records_channel_surface_type asserting a BotMention shared route persists surface_type = Channel. - turns: add submit_child_run_inherits_parent_product_context asserting a child run carries the parent's product_context. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(context-slice): address PR nearai#4836 review — doc/comment hygiene - plan: remove developer-local absolute worktree paths (doc-hygiene rule); use repo-relative working-dir note and run command. - design spec: update the resolver API from the removed `TrustLevel + is_trigger_adapter` pair to `InboundClassification` ({TrustedTrigger, TrustedOther, Untrusted}) and the current `resolve_inbound(classification, adapter, surface_type, owner)` signature. - turns: rename stale `WebUiChat` references in a runtime-context test to `WebUi` to match the renamed origin variant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(context-slice): seal trigger origin via typed evidence; address PR nearai#4836 review Folds the nearai#4851 trust-seal follow-up into this PR. Trust seal (Option 7) — origin classification no longer re-derives trigger-ness from the adapter_kind string: - conversations: carry a typed `TrustedInboundKind::{Trigger,Other}` on `TrustedInboundTurnRequest`; the trusted-trigger submit seam sets `Trigger`. `handle_inbound_turn_inner` classifies from the typed kind on `BindingResolutionPolicy::Trusted`, not `is_trusted_trigger_adapter_kind(adapter_kind)`. A `ScheduledTrigger` origin is now a structural consequence of entering through the trusted-trigger seam (.claude/rules/types.md). Advisory comm slice: - communication_context: an actor-present JoinError now degrades to `Some(Unknown)` (not `None`, which is the no-actor sentinel) so the degrade-to-unknown contract holds and delivery_tools_visible is still stamped; regression test added. Tests: - conversations: InvalidRunOriginAdapter classification (→ SubmitRejected) and submit-key non-rotation regressions. Docs: - origin.rs / product_context AGENTS.md: correct the over-claimed "only place that can mint ScheduledTrigger" — `ProductTurnContext::new` is a low-level constructor, not a hard cross-crate seal (Rust has no friend-crate visibility); the enforced boundary is the typed trusted-trigger ingress seam. - plan + design spec: reconcile to the implemented `InboundClassification` 4-arg resolver and the typed trigger-evidence seal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(context-slice): address PR nearai#4836 review round — newtype shape, abort-on-drop, delivery tools - RunOriginAdapter: adopt the canonical newtype shape (Hash, shared validate(), into_inner, AsRef<str>, Display, From<_> for String) per types.md; align its byte bound to AdapterKind's 512 (via MAX_RUN_ORIGIN_ADAPTER_BYTES) so a valid long adapter kind is no longer narrowed/rejected before submit. Tests at the 512 boundary + a caller-level long-adapter-kind acceptance test. - CommunicationContextFetch: own an abort-on-drop JoinHandle instead of a boxed future. Dropping an unresolved fetch (e.g. early host-construction failure) now aborts the spawned backend lookup instead of detaching it; the type can only be built from a spawned handle, enforcing the concurrency contract. The actor-present JoinError → Some(Unknown) degrade moves into resolve(). Added a drop-before-resolve abort regression test. - loop_driver_host: delivery_tools_visible requires BOTH outbound delivery capabilities (list + set) before rendering guidance that names both tools, so a setter-only profile no longer prompts the model to call an unavailable lister. Caller-level test for the setter-only suppression. - triggers: direct test for is_trusted_trigger_adapter_kind. - composition: rename OUTBOUND_PREFERENCES_TIMEOUT → COMMUNICATION_CONTEXT_FETCH_TIMEOUT (now governs the whole communication fetch, not just delivery prefs). - product_context AGENTS.md: stop pointing at a nonexistent crate-local CLAUDE.md; point at root guardrails + .claude/rules. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(context-slice): address CodeRabbit round on 2011dd9 - runtime_context: `CommunicationContextFetch::resolve` awaits the handle via `as_mut()` instead of moving it out, so a `resolve` future dropped mid-await still lets `Drop` abort the task (the `take()` version detached on cancel). - communication_context: rewrite the abort-on-drop regression to use a drop guard whose `Drop` fires only when the task future is dropped (abort), so the test actually fails if abort-on-drop regresses (prior version's "completed" flag stayed false whether aborted or merely parked — false positive). - triggers: move `mod tests` to the bottom of trusted_submit.rs per repo layout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(context-slice): address review round on 6374bf3 - turns/status: RunOriginAdapter error text said "1..=256 bytes" but the bound is now 512; corrected to 512 (with a comment to keep it in sync with MAX_RUN_ORIGIN_ADAPTER_BYTES). - composition: add send_user_message_renders_webui_origin_in_model_request, asserting the runtime send path renders "Run origin: WebUI chat; replies render in this chat." into the model request (not just persisted owner). - .gitignore: drop the unrelated .codegraph/ entry added on this branch (tooling artifact, out of scope for this feature). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(context-slice): untrack accidentally-committed .codegraph/ state `.codegraph/daemon.pid` and `.codegraph/.gitignore` (machine-local CodeGraph daemon state) were swept into 2dfb5b9 by `git add -A` after the root `.gitignore` `.codegraph/` entry was removed. Restore the gitignore entry and untrack the directory — daemon PID/socket state must not be committed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…earai#4738) * feat(reborn): attachment web UX on the WebChat v2 SPA (nearai#4644) Wires the upload UX into the Reborn WebChat v2 SPA. The backend stack (registry, contract, ingress upload, MountView landing, extraction, context folding) already lands attachments and the timeline already returns their refs; the gaps were the frontend send/staging/render path and exposing the registry's accept= list. Backend: - product_workflow: WebUiAttachmentCapabilities + webui_attachment_capabilities() (registry accept tokens + the 10-file / 5 MiB / 10 MiB budgets decode_attachments enforces); re-exported. - webui_v2: GET /session returns an `attachments` block from that helper, so the SPA picker derives accept= from the server. No static frontend list to drift (kills the four-list class by construction). Frontend (ironclaw_webui_v2_static): - lib/attachments.js: stageFiles (file -> base64 + preview, validated against the server contract), formatBytes, isAcceptedFile, wire/render mappers. - hooks/useAttachmentConfig.js: reads session.attachments via the cached query. - chat-input.js: dead attach button -> real "+" picker with paste/drop staging, removable preview chips, capability-driven validation. - useChat.js: drop the unsupported-payload throw; staged files flow through as WebUiInboundAttachment and onto the optimistic bubble. - api.js: sendMessage carries attachments. - history-messages.js: project record.attachments into render cards so they survive refresh / thread switch (nearai#3272). - message-bubble.js: image thumbnails for cards with a preview. - en.js: real validation strings. Tests: - webui_v2 handler contract: /session advertises the registry accept + budgets. - product_workflow: get_timeline returns attachment refs on the user message (refresh-survival at the backend tier). - JS unit: stageFiles/limits/mappers, timeline->card projection; the two old useChat "reject" tests rewritten into acceptance tests. - e2e (test_reborn_gateway_smoke): v2-native land+persist+timeline, extracted-text-reaches-model (marker + canned mock reply), oversize reject, and a browser upload/refresh test (skip-guarded on webui-v2-beta, per the existing test_v2_* convention). - build.rs: exclude *.test.mjs (not just *.test.js) from the embedded bundle. Deferred (per nearai#4677): image pixels to the vision model and audio transcription; no attachment-bytes endpoint so post-refresh images render as cards; attachment-only sends (v2 requires non-empty content). * fix(webui-v2): handle */* accept token and skip-don't-abort on total-budget Addresses gemini review on the attachment staging helpers: - isAcceptedFile now treats the universal */* (and *) accept tokens as accept-anything, matching standard HTML file-input accept semantics. - stageFiles skips a file that would exceed maxTotalBytes (continue) instead of break, so a later smaller file can still fit the remaining budget; the over-budget notice is de-duplicated. - Two new unit tests lock both behaviours. * fix(reborn): land attachments through a read-write workspace mount The WebUI attachment lander wrote through rt.workspace_filesystem, which is intentionally read-only (it backs setup-marker reads — see local_dev_setup_marker_workspace_filesystem_is_read_only). Every upload therefore failed closed with PermissionDenied, surfaced to the browser as a bare 500 'Internal' with no detail because the lander's map_err discarded the underlying error. - webui_workspace_filesystem() now builds a read-write ScopedFilesystem over the same root (extension_filesystem) using the read-write workspace_mounts the agent's file_read/file_write tools resolve through, so a landed attachment is addressable at its recorded storage_key. - The lander now logs the underlying AttachmentLandingError (warn) before mapping to the sanitized 500, so a failure is diagnosable in the CLI. * observability: never let a sanitized 5xx leave the gateway un-logged Three layers so an internal error like the read-only attachment mount can't recur as a bare 'Internal' 500 with nothing in the CLI: 1. Boundary net: WebUiV2HttpError::into_response_parts now logs every server error (5xx) with code/kind/status/retryable. Surfacing internal errors is now a property of the single HTTP egress point, not of each call site remembering to log before it maps. 2. Carry the cause: new RebornServicesError::internal_from(source) logs the underlying backend error and returns the sanitized 500 — the cause is logged (never serialized to the client). The attachment lander uses it instead of map_err(|_| …Internal…) + a manual warn. 3. Guard the regression: .claude/rules/error-handling.md now flags map_err(|_| …) (a closure discarding the error binding) as a silent-failure anti-pattern alongside unwrap_or_default()/.ok()?, requiring the cause be carried/logged or annotated // silent-ok. Verified: ironclaw_webui_v2 handler contract 51/51, lander read-only test still maps to Internal, all three crates compile warning-free. * address CodeRabbit review on the attachment web UX - chat-input.js: serialize addFiles() staging through a queue + attachmentsRef so overlapping async stageFiles calls validate against the latest set (no over-admitting past the per-message budget). - useAttachmentConfig.js: filter non-string accept tokens so isAcceptedFile's token.trim() can't throw and break the picker. - reborn_services_contract.rs: assert the timeline ref's storage_key is non-empty (not just Some). - test_reborn_gateway_smoke.py: drop the unconditional skip on the attachment card e2e; gate on the _require_v2 runtime probe like its siblings; use the chat-composer testid. - webui_v2 handlers contract: registry accept tokens are explicit extensions (.png/.pdf/...), not image/* wildcards — assert an image extension. * fix(webui-v2): restore .test.mjs assets, complete attachment i18n, review fixes CI (Test ironclaw_webui_v2_static was the fail-fast root): - build.rs no longer excludes `*.test.mjs` from the embedded asset table. main reuses the asset table as a fixture for the `assets.rs` caller-level JS regression tests (asserting `.test.mjs` content via `asset_text`); this branch had unilaterally excluded `.test.mjs`, breaking those tests on rebase. - Complete the attachment i18n: the obsolete `chat.attachmentsUnsupported` ("not supported yet") key is replaced by the 8 granular `chat.attach*` keys in en, but the 10 other locales still had the stale key and lacked the new ones (locale key-set parity test failed). Translate all 8 (+ a new `chat.attachmentStagingFailed`) into ar/de/es/fr/hi/ja/ko/pt-BR/uk/zh-CN. Review: - chat-input addFiles: skip staging when the composer is disabled, and `.catch` the staging chain so an unexpected failure can't permanently reject the shared queue and drop every later add (surfaces a generic error). - e2e: target `input[type=file][multiple]` (the composer picker) so the test can't attach to the ambiguous Settings file input. - error-handling rule: `map_err(|_| …)` is not silent-ok-exemptible — a comment doesn't make the dropped cause reappear; carry or log it instead. * fix(attachments): advertise exact MIME types in the picker accept set The file picker's `accept` attribute was built from extension-only registry tokens (`.png,.pdf,…`). On macOS that makes Chromium hand NSOpenPanel an extension-based `allowedFileTypes` filter that renders folders non-navigable — double-clicking a folder dismisses the picker instead of opening it. Emit the exact MIME type alongside each extension (`image/png,.png,…`); MIME types map to UTIs that keep directory navigation working. Exact, never `image/*` wildcards, so the advertised set still equals the supported set; the JS validator already matches exact-MIME tokens. Registry + webui_v2 session tests updated. * fix(webui-v2): keep staged attachments across composer navigation The composer persisted the text draft across navigation (draft-store) but reset `attachments` to `[]` on every mount, so attaching a file on the new-chat screen, leaving, and returning silently dropped the file while the text came back. Give attachments a parallel per-key draft store. It is in-memory (not localStorage) because the staged files carry base64 bytes that would blow the ~5MB quota — so they survive SPA navigation but not a full page reload, which is the right trade-off for unsent files. The composer initializes `attachments` from the store, persists on change, re-reads on a conversation switch (with a prev-key guard so one conversation's files can't leak into another), and clears on a successful send. Sign-out drops the in-memory store too. Regression: assets.rs locks the draft-store exports and the chat-input wiring. * style: rustfmt the accept_tokens wildcard assertion * fix(webui-v2): stop serving *.test.mjs; address composer review nits - build.rs again excludes `*.test.mjs` from the embedded/served asset table, so Node unit tests are not shipped to /v2 clients. The 3 `assets.rs` regression tests that assert `.test.mjs` content now read it from disk (`source_text`), not the table — keeping the coverage without the exposure. - WebUiAttachmentCapabilities.accept doc no longer shows `image/*` wildcards; the registry advertises exact MIME types + extensions. - chat-input: don't show the drop overlay while the composer is disabled (onDragOver guards on `disabled`); keep `attachmentsRef` in lockstep on the remove and post-send paths so a same-tick add validates the current set. * fix(webui-v2): guard readAsDataUrl against non-string FileReader result A FileReader that resolves with a non-string result (null/ArrayBuffer) would crash splitDataUrl's .indexOf and break attachment staging. Guard the contract in readAsDataUrl so a non-string result rejects and is surfaced as chat.attachmentReadFailed instead. Regression test added in attachments.test.mjs (node --test); the hook only detects Rust tests. [skip-regression-check]
elliotBraem
pushed a commit
that referenced
this pull request
Jun 16, 2026
…earai#4559) * docs: trace commons agent onboarding design spec Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: address spec review findings (trust anchoring, key staging, consumption atomicity, replay validation) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: spec review round 2 nits (server-anchored tenant wording, pending-key cleanup) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: implementation plan for trace commons agent onboarding Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: address plan review findings (scope threading refactor, dispatch model, dev-deps, LazyLock hazard) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: plan review round 2 fixes (literal dep versions, context constructor threading depth) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: incorporate server-agent coordination feedback (optional community/profile/leaderboard URLs) From TraceCommons/trace-commons#136-nearai#141 comments: onboard response gains optional browser-surface navigation hints, sanitized client-side (HTTPS or dropped), never part of issuer trust anchoring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(traces): onboarding wire types matching trace-commons-server contract Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(traces): invite URL parsing with origin trust anchoring Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(traces): device keypair lifecycle with pending staging and self-signed workload JWTs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(traces): auth_mode and device_key_id policy fields with legacy-compatible defaults Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(traces): onboard() orchestration with trust anchoring and retry-safe key staging Wire invite parsing, device key staging, onboard POST, issuer origin trust anchoring, ingest_url HTTPS enforcement, keypair promotion, and policy write into onboard_at_dir(). Refactors invite.rs to extract pub(crate) is_https_or_loopback, origin_of, and host_only helpers shared with mod.rs (one source of truth for origin/bracket handling). Adds axum mock-issuer tests covering the happy path, mismatch rejection, terminal vs transient error key retention, insecure ingest URL, loopback ingest allowance, community URL sanitisation, and retry key reuse. Partial-failure lockout fix (spec §2.2): promote() no longer deletes the pending file. The flow now writes the tenant key file, then the policy, and only discards the pending file after BOTH durably succeed. If the policy write fails the pending key survives, so a retry reloads the same key (server idempotency returns the original registration) and harmlessly overwrites the tenant file — no permanent lockout from a consumed invite with a regenerated keypair. Regression test simulates a policy-write failure (policy.json pre-created as a non-empty dir so the atomic rename fails), asserts Err(Persist) with the pending key intact, then asserts a retry succeeds reusing the same device_key_id. Response validation (defense-in-depth): reject schema_version != the v1 response constant as MalformedResponse, and cross-check the response device_key_id against the locally derived id (we never trust the response value for policy; a disagreement is now treated as a tamper signal and rejected). Both covered by tests. The onboard response body is read with the 64 KB cap enforced per-chunk during streaming (mirroring read_bounded_trace_upload_claim_response) rather than buffering the whole body first, so a hostile server cannot force a large allocation. Also fixes a pre-existing test-isolation defect surfaced by the added load: the remote-request timeout test configured a 50ms timeout via the process-global IRONCLAW_TRACE_REMOTE_REQUEST_TIMEOUT_MS env var. set_var is process-global, so under parallel execution the 50ms value leaked into other tests' trace HTTP clients, producing spurious `operation timed out` failures against fast local mocks. Replace the env mutation with a task-scoped TEST_REMOTE_REQUEST_TIMEOUT_OVERRIDE task-local (visible only within the awaiting test's own task tree, zero production change; documents the spawn caveat), and decouple the timing assertion from a tight wall-clock race so it no longer flakes when reqwest's timer is delayed under an oversubscribed runtime. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(traces): device-key self-signed workload JWT branch in upload-claim refresh Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(engine): trace_commons onboard and status first-party tools with agent guidance Add two model-visible first-party capabilities to the Reborn engine: - builtin.trace_commons.onboard: drives operator-invite enrollment flow with explicit per-conversation consent gate (confirmed=true required before any network call); maps OnboardOutcome/OnboardError to clean agent-readable JSON - builtin.trace_commons.status: read-only enrollment state inspector Wires ironclaw_reborn_traces into ironclaw_host_runtime, creates schema files (schemas/builtin/trace-commons-{onboard,status}.{input,output}.v1.json) and prompt doc files (prompts/builtin/trace-commons-{onboard,status}.md) at the manifest-derived paths. Includes 11 unit tests covering input parsing, consent refusal, success/error value formatting, and status formatting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: add Task 11 — credits visibility (console display + agent-queryable balance) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(engine): e2e trace commons onboarding through capability dispatch Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(traces): document agent onboarding flow in trace-commons internal doc Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: correct Task 11 console scope (credit endpoint already exists; frontend = coordinate with designer) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(traces): trace_commons.credits agent-queryable balance tool Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(gateway): minimal Trace Commons credits card in settings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(traces): store upload-claim endpoint in policy; preserve primary onboard error; block metadata/link-local/multicast issuers Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(traces): route agent onboarding HTTP through host network-egress policy (nearai#4560) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * build: update Cargo.lock for trace-commons onboarding dev-deps Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(traces): drop orphaned schema/prompt files (main resolves builtin schemas inline; prompt_doc_ref dropped) Post-merge cleanup: main's first_party_tools now resolves builtin input schemas via the inline schemas.rs match (trace_commons arms added during the merge) and sets prompt_doc_ref: None for all builtins, so the physical trace-commons-*.json schema files and trace-commons-*.md prompt docs are no longer referenced. The onboard consent contract remains in the capability description and is enforced in dispatch_onboard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Fix Trace Commons invite hash contract * fix(traces): grant trace_commons capabilities in local-dev policy The three builtin.trace_commons.* capabilities were declared in the first-party package but had no [[grants]] entries in local_dev_capability_policy.toml, so local-dev runs (repl/serve) filtered them out of the model-visible tool surface entirely. The provider-level authority_effects ceiling had external_write, but the per-capability grants were never added. onboard gets the local_dev_wildcard egress profile (invite origins are operator-chosen; private/metadata IP ranges stay blocked by the shared enforcer). status/credits are read-only. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(traces): add Reborn e2e coverage for trace_commons first-party tools Closes the coverage gate failure: builtin.trace_commons.{onboard,status, credits} were declared in the first-party package but missing from REBORN_FIRST_PARTY_E2E_COVERED_CAPABILITIES, failing reborn_builtin_first_party_capability_e2e_coverage_is_complete on both the Reborn root tests and all-features CI jobs. Adds a trace_commons host-runtime harness (network policy populated so the onboard Network-effect obligation passes) and a parity test driving all three capabilities through the scripted model loop: onboard with confirmed=false exercises the deterministic consent gate with no network, status and credits return the unenrolled/zero-credit defaults. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(traces): community profile second opt-in (token mint + profile set) After device-key enrollment, public leaderboard attribution is a second, separate opt-in: IronClaw mints a short-lived profile token from the claim issuer with consent_scopes=[public_attribution] and empty allowed_uses (such a claim cannot submit traces), then either prints it for the web profile page or performs the profile update itself. The browser cannot sign device-key requests, so the token must be minted by IronClaw — previously this step was impossible and agent guidance invented flows. - ConsentScope::PublicAttribution mirrors the server protocol enum; default_allowed_uses_for_scope returns empty for it. - mint_profile_attribution_token_for_scope / set_community_profile_for_scope / withdraw_community_profile_for_scope reuse the hardened issuer HTTP path (allowlist validation, pinned DNS, no redirects, bounded reads, token never in errors). PUT/DELETE /v1/community/profile per the server contract; handle (3-32 ASCII alnum/-/_) and bio (<=280 bytes) validated client-side. - CLI: ironclaw-reborn traces profile token|set|withdraw. - Onboard tool next_steps now describes the profile second opt-in so agent guidance stops inventing browser login flows. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(traces): autonomous turn-end trace capture in the Reborn runtime The Reborn binary could onboard, report status/credits, and manage profiles, but never captured or submitted traces — the autonomous pipeline existed only in the v1 agent loop. This wires it into the Reborn runtime composition: - TraceCaptureTurnEventSink subscribes best-effort to the turn lifecycle bus (the existing turn_event_sink injection seam). On Completed/Failed events with an explicit owner it spawns a detached task that reads the owner's standing policy (one file read for non-enrolled users), loads the recent thread history (last 24 messages, 5 turns — v1 parity), adapts user/assistant text rows into the neutral ConversationMessage shape, redacts + scores locally, and queues + immediately flushes eligible envelopes. All failures are debug!-logged and never touch the turn lifecycle path. - A periodic flush worker (300s, 25/scope — v1 parity) retries queued envelopes for the runtime owner plus every scope observed since boot, with CancellationToken shutdown alongside the other workers. - TraceClientAutonomousCaptureRequest gains outcome_override so the lifecycle event's terminal status (authoritative in Reborn, where transcripts carry no structured outcome payload) marks failed turns as TaskSuccess::Failure; v1 passes None (no behavior change). - Tool-result rows and credit-notice delivery are documented follow-ups (refs-only records; no composition-level outbound channel surface). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(traces): end-to-end auto-capture through send_user_message Proves the full Reborn auto-submission chain with a real runtime: a completed turn for an enrolled owner scope lands a redacted envelope in that scope's submission queue with no manual trace command — turn completion -> lifecycle bus -> capture sink -> thread-history read -> redact/score -> eligibility -> queue (+ local-failing immediate flush leaves the entry for the retry worker). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Expose Trace Commons profile token tool * Expose Trace Commons profile set tool * Allow Trace Commons profile setup from agent * feat(webui-v2): Trace Commons credits card in WebChat v2 settings Adds GET /api/webchat/v2/traces/credit and a read-only Trace Commons settings tab to the v2 SPA, giving webui-v2-beta parity with the v1 console's credits card. - Route follows the descriptor system end to end: bearer-auth required, NoBody, 120/60 per-caller read rate limit; descriptor-driven body/rate-limit enforcement applies automatically. - RebornServicesApi::trace_credits derives the trace scope exclusively from the authenticated caller's user id (never from query/body) and reads contributor-local state via ironclaw_reborn_traces (policy + trace_credit_report), soft-falling back to an unenrolled zero-state on missing/unreadable local state, mirroring builtin.trace_commons.credits. - SPA: Trace Commons subtab (enrollment, pending/final credit, delayed ledger delta, submission counts, last submission/sync, recent credit explanations) with the server-authoritative framing and a not-enrolled empty state pointing at agent onboarding. - Tests: descriptor contract row, handler oneshot, and three composed- router serve tests (200 zero-state, 401 without bearer, enrolled policy reporting with per-test scope isolation). - Drive-by: cfg-gate openai_user_id in webui_serve.rs to clear a pre-existing unused-variable warning under default features. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Exempt Trace Commons profile setup from local-dev gate * Route Trace Commons profile writes to ingest * review(4559): address serrrfirat feedback - Drop stray working-note markdown files from the repo root (they rode in via an early origin/main merge and are not this PR's documentation). - trace_commons_dispatch_e2e: setup_base_dir is now a OnceLock that every test calls first — the previous 'single-threaded during init' claim was wrong under tokio's multi-threaded test runtime, and two of three tests skipped the setup entirely. - settings.js: extract shared appendDisplayGroup + declarative row defs; loadTraceCommonsCredits drops from ~120 lines of manual DOM to a rows array; also removes a double-escape (textContent + escapeHtml) on explanation lines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(webui-v2): add traceCommons i18n keys to all locales The credits card added the traceCommons.* key set to en.js only; the i18n consistency test (all_locales_share_the_en_key_set) requires every locale to carry the same key set. Adds translated entries to ar, de, es, fr, hi, ja, ko, pt-BR, uk, and zh-CN. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(reborn): fail loud with source context on malformed local-dev master key The local-dev secret store resolver read the cached key file (and the SECRETS_MASTER_KEY env fallback) and passed the material straight into SecretsCrypto::new several layers deep. A corrupt or low-entropy key (e.g. a 64-char all-zeros value, which passes the length floor but has one distinct byte) surfaced only as the opaque "Invalid master key", with no pointer to the file the operator must fix. - Add ironclaw_secrets::validate_master_key_material as the single source of truth for master-key rules; SecretsCrypto::new delegates to it. - resolve_local_dev_secret_master_key now validates at the source (cached file vs SECRETS_MASTER_KEY env) and returns a RebornBuildError::InvalidConfig naming the offending path/env var and the actual constraint, before any crypto is constructed. - A malformed env value is now rejected before being persisted to the cached key file (no more poisoned-cache state). Tests: malformed-file path-context rejection, malformed-env source-context rejection, valid cached file accepted. Refs nearai#4741 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(webui-v2): add Trace Commons credits card to chat sidebar Surface trace contribution credits at a glance in the chat sidebar, above the conversation list. Previously credits were only visible under Settings -> Trace Commons. - New SidebarTraceCredits component reuses the existing useTraceCredits hook (/api/webchat/v2/traces/credit) — no new endpoint. Renders only when enrolled; loading/error/not-enrolled render nothing to keep the sidebar clean. Shows final credit and accepted/submitted counts and clicks through to Settings -> Trace Commons for the full ledger. - useTraceCredits now refetches (60s interval + on window focus) so the card and the Settings tab reflect newly-accepted submissions live. - Add one compact i18n key (traceCommons.cardAccepted) across all 11 locales; reuse existing keys for the rest. - Source-shape regression test in assets.rs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(reborn-traces): reconstruct tool calls in turn-end trace capture The Reborn capture adapter dropped every tool-result row, so captured trace envelopes were text-only. That left the two highest-value scoring levers — replayability (0.20) and tool coverage (0.15) — permanently at zero, so even agentic tool-using turns scored as plain chat and stayed below the 0.35 submission gate. Nothing ever submitted. conversation_messages_from_records now reconstructs a `tool_calls` message from each run of ToolResultReference rows that carry `tool_result_provider_call` replay metadata, collapsing consecutive rows into one message positioned between the user message and the assistant response (the shape capture_turns_from_conversation_messages' per-turn lookahead consumes). Tool names always flow through so the value scorecard sees required_tools/replayable; raw tool payloads stay consent-gated downstream by include_tool_payloads. Rows without provider metadata remain dropped. TDD: - adapter unit tests: single tool call -> tool_calls message; consecutive calls collapse into one; ref without provider metadata still dropped. - integration guard: a captured tool-using turn's queued envelope carries replay.required_tools + replayable=true. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reborn-traces): read capture history from context window, not display projection Tool-call reconstruction (previous commit) had no data to work with: the capture history source read SessionThreadService::list_thread_history, whose product-display projection (history_message) hard-nulls tool_result_provider_call. So even though tool calls persist with full provider metadata, the adapter received None on every tool row, dropped them, and produced a text-only envelope that scored below the 0.35 submission gate. Nothing ever submitted. SessionThreadHistorySource now reads load_context_window (the model-context/replay view, which preserves tool_result_provider_call) and maps ContextMessage -> ThreadMessageRecord via context_window_to_records. This is the semantically correct source for trace capture anyway: the replay transcript, not the display transcript. TDD: a caller-level test (per .claude/rules/testing.md "test through the caller") drives SessionThreadHistorySource against a real InMemorySessionThreadService with an appended tool result, asserting the returned tool row keeps provider_call. Failed on list_thread_history (None), passes on load_context_window. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(reborn-traces): auto-submit traces with PII risk below High Previously any non-Low residual PII risk was blocked from auto-submission two ways: the manual-approval eligibility gate held everything != Low, and the value scorecard halved the score (privacy_gate Medium 0.5) and subtracted a 0.60-weighted penalty. A minimal tool trace scores ~0.36 at Low (barely over the 0.35 gate), so any Medium penalty collapsed it to 0 — nothing below High could ever submit. Treat below-High residual risk as clean for auto-submission (the deterministic redactor has already scrubbed detected PII): - trace_autonomous_eligibility manual-approval gate now holds only High (== High, was != Low). - privacy_gate: Low|Medium => 1.0 (was Medium 0.5); High => 0.0. - privacy_risk_score: Low|Medium => 0.0 (was Medium 0.5); High => 1.0. High remains fully blocked: privacy_gate zeros its score and the gate holds it for manual review. The 0.35 submission gate leaves no headroom for a partial Medium discount on a minimal trace, so below-High is clean rather than partially penalized. TDD: medium_pii_tool_trace_auto_submits_while_high_is_held asserts a Medium-risk tool trace clears 0.35 and auto-submits while High is held. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(reborn-traces): design for Trace Commons held-trace review Held traces are currently dropped on the autonomous capture path with no visibility or authorize path. This plan reuses the existing hold-sidecar machinery (TraceQueueHold / .held.json / read_trace_queue_holds_for_scope / ManualReview) and adds: retain held traces, surface a held count+list on the /traces/credit response, a card/tab UI, and a promote-as-is authorize endpoint. Four independently-shippable TDD slices. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(reborn-traces): retain manual-review held traces instead of dropping (slice 1) Autonomous turn-end capture dropped every held trace (logged at debug, envelope discarded), so PII-gated traces were unrecoverable and invisible. Slice 1 of the held-review feature retains manual-review holds: - TraceQueueEligibility::Hold now carries a typed TraceQueueHoldKind (ManualReview for the High residual-PII gate; PolicyGate for score / tool-allowlist / submission-class gates), replacing reason-string classification at the flush call site. - TraceClientAutonomousCaptureOutcome::Held carries the built envelope and its kind so callers can persist it. - New queue_trace_envelope_as_held_for_scope: queues the envelope plus a ManualReview .held.json sidecar under one scope lock; the flush worker already skips held sidecars, so it is retained but not submitted. - capture_turn_trace retains ManualReview holds and still drops PolicyGate holds (low-value traces never pollute the review surface). TDD: held-retain function (RED on missing sidecar -> GREEN), eligibility kind classification, and caller-level capture tests (an AWS-key message forces High PII -> retained ManualReview hold; a sub-threshold trace is dropped, not retained). Refs docs/plans/2026-06-10-trace-commons-held-review.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(webui-v2): surface manual-review held count + list on /traces/credit (slice 2) Held traces retained by slice 1 were invisible to the UI. Slice 2 surfaces them on the existing trace-credits response so one fetch powers the whole card/tab. - ironclaw_reborn_traces: manual_review_holds_for_scope() returns only ManualReview holds (excludes PolicyGate value-gates and transient RetryableSubmissionFailure retry holds), via an extracted retain_manual_review_holds filter. - RebornTraceCreditsResponse gains manual_review_hold_count + holds[] ({ submission_id, reason }). Sanitized: submission id and the already privacy-safe hold reason only, never raw trace content. TDD: retain_manual_review_holds filter unit test (excludes policy/retry), disk-level manual_review_holds_for_scope test, and the facade zero-state test asserts the new fields default empty. webui_v2 handler contract tests (42) still pass with the propagated fields. Refs docs/plans/2026-06-10-trace-commons-held-review.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(webui-v2): show held-for-review traces on card + Settings tab (slice 3) Surface the manual-review held count/list from slice 2 in the UI. Both render only when there are holds, so the common (nothing-held) state is unchanged. - Sidebar card: "{count} held for review" line when manual_review_hold_count > 0. - Settings -> Trace Commons tab: a "Held for review" section listing each held trace's sanitized reason + submission id from holds[]. - No hook/api change: fetchTraceCredits already returns the raw response, so credits.holds / credits.manual_review_hold_count are available. - Three i18n keys (cardHeld, heldTitle, heldDescription) across all 11 locales. The per-trace Authorize action ships with its endpoint in slice 4 (so the UI never offers a button that 404s). Source-shape assertions extended. Refs docs/plans/2026-06-10-trace-commons-held-review.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(webui-v2): authorize held traces for submission (slice 4) Complete the held-review feature with a promote-as-is authorize action across the stack. ironclaw_reborn_traces: - TraceContributionEnvelope gains `manual_review_authorized`; an authorized envelope submits past every gate in trace_autonomous_eligibility (the flush re-evaluates eligibility each pass, so removing the hold sidecar alone is not enough to promote). - authorize_manual_review_hold_for_scope: stamps the envelope (durable consent record) BEFORE removing the .held.json sidecar, so a crash between the two leaves the trace held (fail closed). Only ManualReview holds are authorizable; unknown submissions return Ok(false), not an error. ironclaw_product_workflow: - RebornServicesApi::authorize_trace_hold derives scope from the authenticated caller (the path submission id is never cross-scope authority), validates the id, and returns RebornTraceHoldAuthorizeResponse. ironclaw_webui_v2: - POST /api/webchat/v2/traces/holds/{submission_id}/authorize — NoBody, mutation rate limit, bearer auth. Descriptor + handler + router + contract table (now 46 routes). Frontend: - authorizeTraceHold api, an authorize mutation in useTraceCredits that invalidates the credits query on success, and a per-hold Authorize button on the Settings tab. `authorize`/`authorizing` i18n in all 11 locales. TDD: authorize promotes a High-PII held envelope past all gates; facade zero-state; webui_v2 descriptor/handler contracts; composition serve (47); source-shape assertions. clippy/fmt clean across crates. Refs docs/plans/2026-06-10-trace-commons-held-review.md Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(traces): loopback dev claim exception + profile_set consent gate Address the two codex P2 findings from review: - Preserve loopback claim uploads after onboarding: the loopback-HTTP dev invite form stores a loopback claim/ingest endpoint in the policy, but the claim/ingest validators required https and rejected loopback hosts, so a successful loopback onboarding could never mint a claim or submit credits. The validators and the pinned DNS resolution now honor the same literal-loopback exception as invite parsing (shared is_loopback_host predicate); for loopback hosts the pinned resolution additionally requires all resolved addresses to be loopback. Non-loopback http, internal hostnames, and private ranges stay rejected, and the issuer allowlist still applies. - Require explicit confirmation before community profile updates: trace_commons.profile_set now has the same hard confirmed=true input gate as onboarding — it short-circuits with consent_required before the enrollment check and any network write, since the capability is approval-gate-exempt in local-dev policy. Schema and manifest document the field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(merge): thread attachments field through trace-capture record construction main added ThreadMessageRecord.attachments (Vec<AttachmentRef>); the trace-capture reconstruction path and its two test helpers construct records and must set it. The capture path reconstructs records from a context window for redaction/scoring and carries no attachment refs of its own, so Vec::new() is correct. * fix(traces): adapt v1 autonomous capture to new Held variant shape The merge brought in slice 1 of the held-trace-review feature, which changed TraceClientAutonomousCaptureOutcome::Held from { submission_id, reason } to { kind, reason, envelope } so manual-review holds can be retained instead of dropped. The v1 autonomous-capture path in thread_ops.rs still matched the old shape, breaking the `--no-default-features --features libsql` build (and default build). Adapt the v1 path to the new shape and give it the same retain-or-drop parity as the Reborn capture path (ironclaw_reborn_composition::trace_capture): ManualReview holds are retained via queue_held_envelope_for_scope (the on-disk held queue is shared, so a v1-captured hold surfaces in the v2 review UI); policy/value gates are dropped as before, just logged. Behavior mirrors the tested Reborn path (send_user_message_auto_queues_trace_for_enrolled_scope); the v1 autonomous-capture path is a detached tokio::spawn with no unit-testable seam, so no focused regression test is added. [skip-regression-check] * fix(traces): set manual_review_authorized in reborn-cli test envelope fixture The merge brought in the held-trace-review manual_review_authorized field on TraceContributionEnvelope. The reborn-cli trace_queue test fixture constructs the envelope directly and missed the field, breaking `cargo clippy --all-features --tests` and `Tests (all-features)` (the fixture is test-only, so the libsql binary build did not surface it). Fresh queued envelopes are not yet authorized, so false is correct. [skip-regression-check] * test(traces): pass confirmed=true in profile_set parity step The trace_commons first-party-tools parity test invoked profile_set without confirmed=true and asserted the NotEnrolled enrollment-gate result. Commit 6bc776d added the public-attribution consent gate to dispatch_profile_set, which now short-circuits to consent_required before the enrollment check when confirmed is unset — so the test's NotEnrolled assertion failed (the gate output carries no error_code). Pass confirmed=true so the call clears the consent gate and reaches the enrollment check, deterministically returning NotEnrolled with no network (the scope never onboarded). Matches the unit-test pattern established for the other profile_set tests in the same change. [skip-regression-check] * fix(traces): onboarding-security + contribution correctness (coderabbit batch 1) Addresses 6 coderabbit findings in ironclaw_reborn_traces: - device_key.rs: re-assert 0o700 on pre-existing key dirs (not just on create), so broader perms on an existing device_keys/ or pending/ can't leave invite/tenant hashes enumerable. - device_key.rs: fail closed on load when on-disk public_key/device_key_id don't match the loaded private key (tampered/partial files no longer load an inconsistent identity that only fails later at remote auth). - invite.rs: scope the staged pending-key filename by invite ORIGIN, not just code, so two issuers reusing one invite code can't share a device key (invite_hash stays code-only as the server allowlist subject). - onboarding/mod.rs: reject ingest_url values with embedded userinfo before persisting, so a malicious onboarding response can't smuggle credentials into policy.json + outbound requests. - contribution.rs: preserve mount path prefixes when deriving the community-profile endpoint (mirrors trace_submission_status_endpoint); a prefixed deployment no longer 404s on profile PUT/DELETE. - contribution.rs: fail closed in trace_autonomous_eligibility on envelopes with no allowed-uses (public_attribution-only) instead of relying on the remote to bounce them. Updated two retry tests that encoded the cross-issuer key-sharing bug now fixed: they retried against a second mock on a different port; a new spawn_flaky_mock_issuer keeps the retry on the same origin so it exercises genuine same-issuer pending-key reuse. Added regression tests for each fix. * fix(trace-commons): address coderabbit review findings on nearai#4559 - index.html: add type="button" to the Trace Commons settings subtab to prevent accidental form submission. - settings.js + i18n/en.js: route the Trace Commons credits copy through I18n.t(...) and register the matching locale keys (matches the existing surface pattern; en-only like settings.traceCommons, fallback covers rest). - factory.rs: drive the malformed SECRETS_MASTER_KEY env case through the real caller resolve_local_dev_secret_master_key (via an env-parameterized inner) and assert the rejected key is never persisted to the cached file. - trace_commons_dispatch_e2e.rs: give each test a distinct user/extension scope so onboarding state can no longer bleed across tests. - local_dev_capability_policy.toml: exempt builtin.trace_commons.onboard from the REPL approval gate (it has its own confirmed=true consent gate, mirroring profile_set). - docs: fix the onboard prompt-file reference, match the held-trace JSON shape to RebornTraceHold (submission_id + reason only), and resolve the wire-protocol ownership split (types live locally in onboarding/protocol.rs, no shared trace-commons-protocol crate). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(traces): tenant-scoping + token leak + read-failure + unbounded scopes (coderabbit batch 2) Addresses the coupled backend findings: - Tenant-scope Trace Commons local state across the Reborn paths: new trace_scope_key(tenant, user) helper keys policy / device-key / credit / profile / capture state by tenant+user, so the same user id in two tenants no longer shares state. Applied in host_runtime trace_commons dispatchers, product_workflow credits/hold, and composition trace-capture (v1 stays user-only — legacy single-tenant). Updated the affected runtime/sink tests and added a non-owner attribution assertion. - Do not return the raw profile token from the model-visible profile_token capability: persist it to a 0600 <scope>/profile_token.jwt and return the file path + instructions instead, keeping the bearer credential off the LLM transcript. - Stop masking genuine local-state read failures as zero/not-enrolled: the status capability and the WebUI credits path now propagate a read/parse failure (NotFound is already softened inside read_*_for_scope) so an enrolled user with a corrupt policy file is not told they have nothing. - Bound ObservedTraceScopes: the periodic flush worker now prunes drained scopes (new trace_scope_has_pending_queue) after each tick, so the set is bounded by actual pending backlog instead of growing one entry per caller ever seen. Note: a v1 caller-level test for the ManualReview hold-retention path is not included — v1 ingress blocks secrets outright and the outbound leak detector redacts them, so the High-residual-PII condition that produces a ManualReview hold cannot be reproduced through process_user_input. The retention logic is identical to and covered by the Reborn-side capture_retains_manual_review_hold_for_high_pii_trace. * test(traces): enroll under tenant-scoped key in webui_v2_serve credits test trace_credits_reports_enrolled_for_caller_with_enabled_policy wrote the policy under the bare user id, but the credits route now keys local state by trace_scope_key(tenant, user). Enroll (and clean up) under the composite TENANT/user scope so the route sees the enrollment. * fix(factory): fail closed on explicit-but-unusable SECRETS_MASTER_KEY An explicitly-set-but-unusable local-dev master key silently fell through to generating + persisting a fresh key, leaving local-dev secrets encrypted under an unintended master key the operator never chose: - resolve_local_dev_secret_master_key used std::env::var(...).ok(), which drops VarError::NotUnicode -> treated as absent. Now only NotPresent is absent; a non-Unicode value returns InvalidConfig. - resolve_local_dev_secret_master_key_with_env collapsed a set-but-empty (or whitespace-only) value to None via .filter(). Now a set-but-empty value returns InvalidConfig instead of generating a key. Added resolve_local_dev_secret_master_key_rejects_set_but_empty_env_without_persisting asserting empty/whitespace env values fail closed and persist nothing. (coderabbit follow-up on nearai#3794) * fix(factory): reject empty SECRETS_MASTER_KEY before the cached-file read Follow-up to the prior fix: the empty-env rejection lived in the env branch, which only runs when no cached key file exists. On a rebuild where .reborn-local-dev-secrets-master-key already exists, the cached key was returned first, so an explicitly-set-but-empty SECRETS_MASTER_KEY was still silently ignored. Hoist the empty/whitespace rejection (and env normalization) above the cached-file read so it fails closed regardless of cached state. Added resolve_local_dev_secret_master_key_rejects_empty_env_even_with_cached_file asserting the empty env is rejected and the cached key is left unchanged. * fix(traces): address 14:54 coderabbit re-review (tenant-seed, IO errors, effects, test) Four outside-diff findings from the re-review: - runtime.rs: seed ObservedTraceScopes with the runtime owner's trace_scope_key(tenant, owner) composite, not the bare owner id, so startup pending-queue discovery matches how capture keys state; the enrolled-scope test cleanup now removes the composite scope dir too. - runtime.rs: the trace-queue polling test helper no longer swallows read_dir errors via unwrap_or_default() — only NotFound is the expected pre-capture fallback; any other IO error panics instead of masking as 'no queued traces'. - trace_commons.rs manifests + local_dev grants: onboard (device-key material) and profile_token (0600 token file) now declare Read/WriteFilesystem effects, and the local-dev grants allow them, so the effect model accurately models the local secret-material writes. - local_dev_authorization test: added local_dev_trace_commons_onboard_skips_approval_gate (the onboard exemption was the actual fix; the profile_set-only test would pass even if the onboard TOML exemption were dropped). * fix(factory): validate non-empty SECRETS_MASTER_KEY before the cached-file read Follow-up: the prior fix rejected an *empty* env value before the cached read but still validated a non-empty *malformed* value only after it. So a valid cache + SECRETS_MASTER_KEY=0000... silently ignored the explicit bad secret config on rebuilds. Move validate_resolved_master_key into the up-front env normalization so any explicit-but-unusable env key (empty OR malformed) fails closed regardless of cached state. Added resolve_local_dev_secret_master_key_rejects_malformed_env_even_with_cached_file. * fix(traces): address 15:41 coderabbit re-review (credits read-failure + 2 test guards) - trace_commons.rs dispatch_credits: stop masking genuine records read/parse failures as 'no records' (NotFound is already softened inside read_local_trace_records_for_scope); report RecordsReadFailed, mirroring dispatch_status. - runtime.rs trace-queue polling helper: fail loud on per-ENTRY read_dir IO errors too (map + unwrap_or_else panic) instead of filter_map(e.ok()), so a broken entry can't be silently dropped while claiming the queue holds one. - local_dev_authorization approval-gate test: assert the effects DO require approval without the exemption (local_dev_effects_require_approval), so the test can't pass via a non-gating default policy if the TOML exemption were dropped. * fix(traces): address Henri review — backend findings (atomic token, error mapping, validation, egress test) - persist_profile_token now writes atomically (unique 0600 temp + fsync + rename) so a reader never observes a half-written or overwritten bearer credential under overlapping mints (Henri perf/security Medium). - dispatch_onboard error mapping: OnboardError::DeviceKey is reported as a distinct DeviceKeyError (re-run onboarding) instead of being collapsed into PersistError's check-disk-and-permissions guidance (Henri bugs Medium). - parse_profile_set_input enforces the manifest's declared schema at parse time: handle 3-32 ASCII letters/digits/-/_, bio <= 280 bytes (Henri conventions Medium). Added schema-limit test. - Added dispatch_onboard_confirmed_without_host_egress_is_network_denied covering the NetworkDenied host-egress-miswiring branch (Henri tests Medium). * fix(traces): address Henri review — frontend findings (enrolled empty-state + polling) - v1 credits: TraceCreditResponse now carries `enrolled` (read from the standing policy), and settings.js keys the opt-in empty state on `!data.enrolled` instead of `!submissions_total` — an enrolled user with zero submissions now sees their zero-credit view, not the not-enrolled prompt (Henri bugs Medium). - useTraceCredits: each fetch rebuilds the full server-side credit view, so the aggressive 60s poll made an open tab steady O(history) work. Relaxed to a 5-min interval + staleTime + no background polling, keeping a focus refetch for liveness; mutation invalidation still updates promptly. Added a TODO to incrementalize the server-side view (Henri perf Medium). * perf(traces): memoize server-side credit view by on-disk input signature Bounds the trace-credits polling cost to O(new submissions) instead of O(total history). New scoped_credit_view(scope) caches the computed credit report + manual-review holds keyed by a cheap change signature (submissions file mtime+len, plus a hash of the held-trace sidecars). On the steady-state polling case (unchanged history) a request is a couple of stat()s + a clone rather than reading/parsing the full submissions file and re-aggregating. On any change the signature differs and it recomputes once. Cache is bounded (4096 scopes, cleared on overflow). Wired through the polled WebUI path (local_trace_credits_for_user) and the model-visible credits capability (dispatch_credits). Added scoped_credit_view_reflects_record_changes_via_signature covering the cache-hit path and signature-based invalidation on record changes. Completes the TODO from the Henri perf-review follow-up (#5). * fix(traces): gate profile_set behind runtime approval (Henri #1 High) profile_set publishes a public community profile (an external write to a public surface). Its `confirmed=true` input is model-controlled, so a prompt-injected or confused model could supply it. Make the runtime approval gate the primary, user-controlled consent control: - Drop `builtin.trace_commons.profile_set` from the local-dev approval-gate exemption list (keep `onboard`, which runs its own in-turn confirmed=true consent before the network POST). - Set profile_set's manifest default_permission to Ask (was Allow). - Split the local-dev authorization test into `local_dev_trace_commons_profile_set_requires_approval_gate` (asserts Decision::RequireApproval) and `local_dev_trace_commons_onboard_skips_approval_gate` (asserts Decision::Allow), via a shared `trace_commons_authorize_decision` helper that first asserts the effects would gate without an exemption. Also fix a pre-existing trace_commons harness gap: onboard + profile_token gained a WriteFilesystem effect (device-key persistence) but the `trace_commons_tools` harness allow-set was never updated, so those capabilities were filtered out of the model-visible surface and the parity/visibility tests failed with driver_unavailable. Grant WriteFilesystem in the harness allow-set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(traces): extract onboarding test harness to sibling file (Henri nearai#8) The onboarding module's ~840-line `#[cfg(test)] mod tests` block (mock issuer harness, retry/idempotency coverage, URL-validation tests) made `onboarding/mod.rs` a 1319-line file dominated by test scaffolding. Move the module body into `onboarding/tests.rs` declared `#[cfg(test)] mod tests;`, leaving mod.rs focused on production logic (now 480 lines). No test behavior changes; `use super::*;` still resolves to the onboarding module. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(webui-v2): update embedded-asset assertion for incrementalized credits poll The Henri #5 polling fix changed useTraceCredits.js from refetchInterval 60_000 to 300_000 (plus refetchIntervalInBackground: false and staleTime: 60_000), but the embedded-asset test in assets.rs still asserted the old 60_000 value and failed in CI. Update the assertion to lock the new infrequent-poll + paused-while-hidden + focus-refetch shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(traces): address CodeRabbit review + stale capability-policy test CodeRabbit findings on the gating/refactor commits: - Major: format_profile_token returned the absolute host path of the token file (token_file) on the model-visible surface, which violates the "never expose absolute paths" guideline. Replace with an opaque token_delivery marker; the token is still persisted 0600 for out-of-band retrieval by a bearer-auth UI/CLI. Update the message + test accordingly. - Major (fail-loud): profile_token_error_value and profile_set_error_value collapsed "could not read policy" into NotEnrolled, sending enrolled users back through onboarding on unreadable/corrupt state. Split into a distinct PolicyReadFailed result in both formatters (matches dispatch_status). - Minor: stale comment claiming profile_set is approval-gate-exempt (it is now PermissionMode::Ask and NOT exempt) — corrected. - Minor: inaccurate harness comments (profile_token writes profile_token.jwt not device-key material; yolo auto-approves all Trace Commons Ask-gated tools, not just onboard) — corrected. Also fix bundled_local_dev_capability_policy_parses, which still asserted the pre-gating policy shape: profile_set as exempt (now onboard exempt / profile_set NOT exempt), onboard's grant missing the read/write filesystem effects, and profile_token/profile_set sharing one effect-set assertion even though profile_token now carries WriteFilesystem and profile_set does not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(traces): collapse single-line use block after Path import removal rustfmt collapses `use std::{panic, path::PathBuf, sync::Arc}` to one line once Path was dropped; the prior commit skipped re-running fmt after that edit, reddening the Formatting CI check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(traces): consent-gate profile_token + drop fixed-origin profile URL (CodeRabbit) Two Major CodeRabbit security findings on the profile tools: - profile_token minted and persisted a bearer credential with no in-turn consent gate. PermissionMode::Ask can be auto-approved under local-yolo, so a model call could mint a credential without explicit per-conversation consent. Add a hard confirmed=true gate (schema + parse + consent_required short-circuit) before minting, mirroring dispatch_onboard / dispatch_profile_set. - format_profile_token and profile_set_success_value hardcoded https://tracecommons.ai/profile. The token is scoped to the user's ENROLLED issuer (which may be self-hosted or loopback), so steering the user to paste a bearer profile-management token at a fixed origin could leak it to the wrong host. Drop the fixed profile_url; route through the enrolled profile flow / local UI/CLI out of band. Tests: new dispatch_profile_token_without_confirmed_returns_consent_required_no_mint; existing without-enrollment test now passes confirmed=true; profile_set success test asserts no fixed origin; parity step mints with confirmed=true. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(traces): route agent-invoked profile writes through host egress (CodeRabbit #3) profile_token (upload-claim mint) and profile_set (community-profile PUT/DELETE) previously made network writes via the ironclaw_reborn_traces crate-local reqwest client, bypassing the host RuntimeHttpEgress pipeline (private-IP filtering, redaction, byte accounting) that onboard already uses. Add a `ContributionHttpSink` port (mirroring `OnboardingHttpSink`): when a sink is injected, the mint POST and the profile PUT/DELETE run through host egress; when `None`, the existing hardened crate-local client is used unchanged. host_runtime supplies `HostEgressContributionSink` (wraps RuntimeHttpEgress, sanitizes errors via stable_runtime_reason, never leaks URL/token), and dispatch_profile_token / dispatch_profile_set fail closed with NetworkDenied if egress is absent (after the enrollment pre-check, so a not-enrolled user still gets NotEnrolled guidance). The background trace-upload / status-sync worker and the CLI keep the crate-local client (pass `None`): that lane is a durable, model-input-free internal task that sends only already-redacted envelopes to the operator-enrolled endpoint and does its own SSRF/private-IP validation, so host egress adds complexity without security benefit. Justification recorded in a comment on `trace_remote_http_client`. New public surface: ContributionHttpSink/Request/Response/Error/Method, mint_profile_attribution_token_for_scope_via_sink, set_community_profile_for_scope_via_sink. Existing public fns keep their signatures (None path) so CLI/worker/tests are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
elliotBraem
pushed a commit
that referenced
this pull request
Jul 1, 2026
…ols (nearai#5061) * feat(reborn): skill-learning turn-end seam + extraction prompt Add the post-completion seam for learning reusable skills from successful runs, composed additively alongside trace capture (no behavior change to existing paths): - SkillLearningTurnEventSink: on a successful turn completion, reads the run transcript (load_context_window, preserving tool calls) and gates substantive runs (>=3 tool actions, >=5 messages) as skill-extraction candidates. Modeled on trace_capture.rs; detached, debug!-only. - CompositeTurnEventSink: fans the single turn_event_sink slot out to both trace capture and skill learning. - assets/prompts/skill_extraction.md: one-shot transcript -> SKILL.md prompt for the next increment (the distillation LLM call). - docs/plans: design + implementation log. Distillation, staging-for-approval, and the scoped skill write land in follow-up increments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(skill-learning): distillation logic crate (transcript -> SKILL.md) New leaf crate ironclaw_skill_learning owns the pure skill-learning logic, kept out of the composition root (per architecture guardrails) and reusable by both the autonomous sink and a future explicit CLI command: - distill_skill(transcript, &dyn SkillInferencePort) -> DistillOutcome: runs the extraction prompt through an abstracted inference port, then validates the output with ironclaw_skills::parse_skill_md (the SAME parser the install path uses) so a distilled skill is guaranteed installable. - parse_distillation: tolerates SKIP declines and accidental code-fence wraps; rejects chatty/invalid output. Inference is abstracted behind SkillInferencePort so the crate has no LLM/runtime/filesystem dependency. - Moves the extraction prompt here (co-located with the parser contract it must satisfy). 6 unit tests, clippy clean. Composition wiring (inference adapter over the runtime's non-run inference port + scoped write) lands next. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(skill-learning): wire distillation into the turn-end sink On a successful, substantive run, the skill-learning sink now actually distills a SKILL.md (instead of just logging a candidate): - SkillLearningInferenceAdapter bridges a strong-model LlmProvider to the logic crate's SkillInferencePort, passing the learning model as a per-request override (NEAR AI honours it) — so distillation runs against a STRONGER model than the run's, without touching the run's model gateway. - build_skill_learning_provider builds that provider from the run's resolved NEAR config with only the model overridden (IRONCLAW_SKILL_LEARNING_MODEL), reusing existing credentials. No churn to build_llm_gateway / the gateway return tuple. - The sink formats the run transcript (tool names included) and calls distill_skill, logging the distilled skill / skip / error. The scoped write + stage-for-approval land in the next increment. - Skill learning is gated on root-llm-provider (it needs an LLM) and is active only when the learning model is configured; otherwise only trace capture runs. CompositeTurnEventSink fans the single turn_event_sink slot to both. Verified: check (default + root-llm-provider) 0 warnings; test + clippy (root-llm-provider,test-support,libsql) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(skill-learning): install distilled skills (scoped write + safety scan) The skill-learning sink now persists the distilled skill so it appears in Settings->Skills and loads into the next run (per the user's "scan + visible" choice; the pre-approval gate is the next increment): - SkillWriter seam (composition trait): the sink depends on a small write abstraction; PortSkillWriter implements it over the runtime's existing RebornLocalSkillManagementPort (install_for_scope, falling back to update_for_scope on re-learn). Tests use a stub writer (no filesystem). - Scope is derived from the EVENT: ResourceScope::local_default(owner, ...) with tenant_id overridden to the run's tenant, so the write lands where the WebUI lists it and the next run reads it (NOT the `default` tenant). - Distilled content is injection-scanned (ironclaw_safety:: validate_trusted_trigger_prompt with a Sanitizer, mirroring the WebUI facade) before install — it becomes trusted prompt text in the next run. - Sink wiring now also requires local_runtime (the skill port lives there); reuses local_runtime.skill_management rather than building a new port. Verified: check (default + root-llm-provider) 0 warnings; test + clippy (root-llm-provider,test-support,libsql) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(skill-learning): live "learned a skill" bubble on WebChat v2 When a skill is distilled + installed, the sink now emits a live notification to the run's thread stream, rendered by the EXISTING WebChat v2 chat bubble (reuses the SkillActivation projection — zero new wire variants): - LiveProjectionPublisher::publish_skill_learned: publishes a SkillActivation live item from raw pieces (owner, turn scope, run_id, name, feedback), the post-run analogue of the in-run SkillActivationObserver (which only fires at prompt-build for skill selection). Gated on root-llm-provider. - SkillLearnedNotifier seam (same testable pattern as SkillWriter): LiveSkillLearnedNotifier wraps the publisher; the sink emits the bubble after a successful install. Tests use a stub notifier. - runtime wiring clones the live projection publisher before the milestone-sink builder consumes it, and passes a notifier into the sink. The learned skill already appears in the existing Settings->Skills page (installed live in the prior increment); this adds the in-chat moment. Pre-approval gate (decision #3) is the next increment. Verified: check (default + root-llm-provider) 0 warnings; test + clippy (root-llm-provider,test-support,libsql) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(skill-learning): refresh implementation log; rename refinement (drop GEPA) - Phase 2 renamed to "Skill Refinement (eval-driven reflective improvement)"; removed the "GEPA-lite" name (DSPy/Hermes term) per review. - Implementation log updated to reflect increments 2 (logic crate), 2b (sink wiring + the SystemInferencePort rejection), 3 (scoped install + scan), and 4 (live learned-skill bubble), plus the per-increment verification gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(skill-learning): e2e fixes from a live ironclaw-reborn run Validated the whole loop end-to-end against a running `ironclaw-reborn serve` with a NEAR AI `openai/gpt-5.5` learning model: a completed multi-tool run was distilled into a real SKILL.md (with pitfalls captured from the transcript), injection-scanned, installed under the correct (tenant=reborn-cli, user) scope, and shown in Settings->Skills. Three real bugs the run surfaced: - Drop the temperature override: reasoning models (gpt-5.x) reject any non-default temperature with HTTP 400 ("temperature does not support 0.2"). - Bump the distillation output ceiling to 16384: a reasoning learning model spends tokens on reasoning before emitting the SKILL.md, so a 4096 cap would truncate it. - Lower the eligibility gate to >=2 tool actions / >=3 messages: an efficient agent can complete a skill-worthy multi-step task in two tool calls (e.g. `shell` mkdir + batch write). The gate is only a cheap pre-filter; the learning model's own SKIP judgement is the real quality gate. - Loosen the extraction prompt: distill any multi-step tool procedure (capture the general repeatable procedure); only skip purely conversational runs. Also removes the temporary info-level diagnostics added while debugging. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(skill-learning): auto-activate learned skills on criteria match Local-dev composition hard-coded the skill selection mode to `ExplicitOnly`, so a learned skill only activated when the user typed `$name`/`/name`. That left the learn loop half-open: skills were distilled and installed but never reused unless named explicitly. Switch local-dev to `ExplicitAndCriteria` (the upstream default) so a learned skill auto-activates when a later request matches its keywords/patterns, closing the learn→reuse loop. Explicit mentions still force-activate; criteria selection is additive and bounded by `max_active_skills` / `max_context_tokens`. The selector-config unit test now locks `ExplicitAndCriteria` so a revert to explicit-only trips a clear failure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(skill-learning): durable learned-skill feedback + dedup consolidation Two gaps in the learn loop, both surfaced while dogfooding against a live ironclaw-reborn run: 1. No visible "learned a skill" feedback. The post-run sink published a live `SkillActivation` projection bubble, but that is ephemeral — only delivered to a stream connected at publish time, ~seconds after the run when distillation finishes. Add a DURABLE path: after install, append a finalized assistant note to the run's thread, so the feedback renders from `get_timeline` and survives a reload even when no live stream was open. The spawned extraction body is lifted into `ExtractionJob::run` so the durable announce is testable end-to-end through its caller (the spawn is otherwise fire-and-forget). Two regression tests also lock that the live `SkillActivation` bubble drains to the WebUI projection stream (fresh and resume-from-advanced-cursor paths). 2. Near-duplicate skills accreted. The distiller names the same kind of task slightly differently each run, so the user's skill list filled with siblings (file-create-read-count-summary, file-character-count-roundtrip, create-read-count-file-characters …) that never get reused together. Before installing, `PortSkillWriter` now lists existing learned skills and, when one covers the same ground (Jaccard over the combined name/keyword/tag token sets ≥ 0.45), refines it in place under its existing name instead of installing a second one. Only `User`-source skills are merge targets; system/registry skills are never touched. `update_skill` requires the document name to match the target, so the merged content is retargeted first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(skill-learning): self-evolving skill refinement on recurring tasks Builds on near-duplicate consolidation: when a learned task recurs and the freshly distilled candidate matches an existing learned skill, the existing skill is now *refined* in place rather than overwritten — the self-evolution step. The learning model folds the candidate's new evidence into the existing SKILL.md (converged steps, the UNION of real gotchas, a bumped version), so a skill gets strictly better each time its task comes around. - `ironclaw_skill_learning::refine_skill` + `parse_refinement` + `RefineOutcome`, driven by `prompts/skill_refinement.md`. Pure domain logic, validated by the install-path parser; tolerates a `KEEP` decline (existing already subsumes the candidate) and a code-fence wrap, same as distillation. - Composition `SkillRefiner`/`LlmSkillRefiner` seam: maps the model outcome to a `MergeAction` — `Replace` (refined, retargeted to the existing name, and injection-scanned), `KeepExisting` (leave the existing skill untouched), or `Overwrite` (fall back to plain consolidation when refinement is unavailable or the model output is unusable). The refined document is retargeted defensively (never trust the model to preserve the name) and re-scanned before install. - `PortSkillWriter` reads the existing skill and consults the refiner on the merge path; wired in `runtime.rs` from the same learning inference adapter. Unit-tested end to end through the refiner (replace+bump, model-rename retarget, keep, unparseable→overwrite) and in the logic crate (parse/keep/reject). The prompt's merge quality is verified live against the NEAR AI learning model. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(skill-evolution): log increments 5-8 (durable feedback, auto-consume, dedup, refinement) Records tonight's work and the one known gap: the live SkillActivation bubble is published but not delivered in the running server (empirically confirmed), its mechanism passes deterministic tests, and it could not be instrumented live without the NEAR AI key — so a durable timeline note is the reliable fix shipped instead. Carries forward the remaining work: pinning the live-SSE gap, an eval-driven refinement loop, the pre-approval gate, CLI commands, and a one-off consolidation of the siblings already on disk. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(skill-learning): live-validation fixes for durable feedback + refinement Found by re-running the loop end to end against a live ironclaw-reborn with the NEAR AI learning model (the in-memory fakes missed both): 1. Durable note never persisted. The durable store dedups assistant drafts by `turn_run_id` and returns the existing one, so `announce_learned_skill` reusing the run's id handed back the run's already-finalized reply and the finalize failed `MessageNotDraft` ("message … is not an assistant draft"). Use a distinct `skill-learned:{run_id}` id so the note is its own message. The regression test now seeds the run's finalized reply first (reproducing the collision the fresh-thread test missed) and asserts the note is a separate, finalized message. 2. Re-learning the SAME skill name overwrote the refined version instead of refining it. The distiller derives the name from the task, so it often repeats; the old path skipped the similarity check for the same name and fell to a plain install→update-on-conflict, resetting an evolved v2 back to a fresh v1. `find_merge_target` now routes BOTH an exact-name re-learn and a renamed sibling through refinement, so the version climbs consistently (verified live: create-read-count-file-characters v1 -> v2, "refined existing learned skill", skill count held at 3, durable note rendered, zero finalize errors). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(skill-evolution): record live end-to-end validation + the two fixes it found Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(skills): per-skill auto-activation flag honored by the selector Foundation for user-facing skill activation control. Adds a manifest `auto_activate` flag (frontmatter, defaults true so existing skills are unaffected) and has the activation selector honor it: a skill with `auto_activate: false` is excluded from criteria (keyword/regex) selection but stays available for an explicit `$name` / `/name` mention. State lives in the skill's own SKILL.md — no new storage layer. - `SkillManifest.auto_activate` (`#[serde(default = "default_auto_activate")]`). - `set_skill_auto_activate(content, enabled)`: line-edits the frontmatter flag, preserving the rest of the document byte-for-byte so a toggle does not reformat the skill (re-parses cleanly with the same name). Unit-tested (default-true, insert-then-replace). - `select_skill_activations` builds a criteria-candidate set filtered by `auto_activate`; explicit mentions still resolve against the full set. Existing skill construction sites updated for the new field; full workspace + reborn binary build green (266 crate tests pass). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(skills): API + DTO to toggle a skill's auto-activation Wires the per-skill auto-activation flag end to end on the backend so the WebChat v2 UI can flip it: - `POST /api/webchat/v2/skills/{name}/auto-activate` ({ enabled }) — reads the skill, line-edits the frontmatter flag via `set_skill_auto_activate`, re-scans it for injection (parity with install/update), and persists. Added as a default method on `SkillsProductFacade` / `RebornServicesApi` (fail-closed unavailable) with the real implementation in the composition facade, plus the handler, descriptor, route, and exports. Descriptor contract test updated. - `SkillSummary.auto_activate` + `RebornSkillInfo.auto_activate` so the skills list reports each skill's current state for the UI toggle (defaults true). Full backend chain builds (reborn binary green); ironclaw_skills, extension ports, webui_v2, and product_workflow test suites pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(webui): per-skill auto-activation toggle in Settings → Skills Adds an "Auto-activate: On/Off" switch to each manageable skill card. Off makes the skill explicit-only (`/name`); on restores keyword/criteria auto-activation. Wires `setSkillAutoActivate` through settings-api → useSkills mutation (invalidates the skills query) → SkillsTab handler → SkillGroup → SkillCard, reading the `auto_activate` field the v2 skills DTO now reports. Mirrors the existing skill install/update/remove mutation pattern. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(skills): global "auto-activate learned skills" master switch (live) Add a global toggle that disables default auto-activation while keeping explicit /name invocation. ON (default) selects ExplicitAndCriteria; OFF selects ExplicitOnly. It takes effect on the next turn with no restart, via one process-global Arc<AtomicBool> shared by reference between the activation selector (reads it every turn in select_skill_activations) and the WebUI skills facade (writes it). Not persisted by design — resets to ON on restart. Vertical: - factory.rs: RebornLocalRuntimeServices.skill_auto_activate_learned (default true), one instance shared with the selector and the facade. - activation.rs/skills.rs: thread the flag into SelectableSkillContextSource; gate the criteria branch on it. Explicit mentions always activate. - webui.rs: LocalSkillsProductFacade holds Option<Arc<AtomicBool>>; set_auto_activate_learned stores into it, list_skills surfaces it. When no flag-reading selector is wired (production assembly) the facade gets None and the toggle fails closed (503) instead of writing to an orphan flag — fixes a review finding where the production toggle silently no-oped and read back true. - reborn_services.rs/types.rs: facade + API trait method, delegation, and RebornSkillListResponse.auto_activate_learned DTO field (serde default true). - webui_v2: POST /api/webchat/v2/skills/auto-activate-learned route, handler, descriptor + contract row. - frontend: setAutoActivateLearned API, useSkills mutation, Settings → Skills LearnedAutoActivateCard master switch. Tests (regression): - global_auto_activate_flag_gates_criteria_and_honors_live_toggle: drives the real selector with a live flag flip (off → empty, flip on → activates). - set_auto_activate_learned_flips_shared_flag_and_surfaces_in_list. - set_auto_activate_learned_fails_closed_when_no_selector_is_wired. - set_auto_activate_learned_forwards_enabled_flag_to_facade (through the caller). - descriptor contract row. Live-validated against ironclaw-reborn serve: GET skills auto_activate_learned True → toggle OFF → False → toggle ON → True. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(skill-evolution): record increment 9 — global auto-activate master switch Document the live global toggle (shared Arc<AtomicBool>, ExplicitAndCriteria ⇄ ExplicitOnly, not persisted), the production orphan-flag review finding and its fail-closed fix, the regression tests, and the live end-to-end validation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(skill-learning): scope extraction eligibility to the completed run The post-turn ExtractionJob loads the recent THREAD window (no run filter) and the eligibility gate counted tool-result messages across that whole window. A trivial follow-up turn after a tool-heavy task could re-pass the gate on the previous run's stale tool results and re-distill it — wasted inference plus a stale-transcript refine that can regress an evolved skill. Count tool actions only for the completed run, read from the history projection (which keeps message kind + turn_run_id and only nulls the tool metadata the transcript needs). The full window is still used as the multi-turn distillation context, which is intentional. The producer writes turn_run_id = run_id.to_string(), matching self.run_id. Localized to skill_learning.rs — no change to the shared ContextMessage / agent-loop model-context path. Regression test: eligibility_counts_tool_actions_for_the_completed_run_only (trivial follow-up under a fresh run id over stale prior-run tool results does not distill; a run with its own tool actions reaches distillation). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(webui): make the skill auto-activation switch read as the global control it is The master switch gates the entire criteria-selection pass, so it affects every skill (learned, user-authored, and bundled), not only learned ones — but the Settings card said "Auto-activate learned skills". Rename the user-facing card to "Default skill auto-activation" with global wording (frontend strings only; behavior and the wire field are unchanged). Also give the card a light-red background and a black status line when disabled, as a persistent "default is off" cue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(skill-evolution): increment 10 — review-driven hardening Record the three review findings and their disposition: run-scoped extraction eligibility (fixed), the global master-switch relabel (fixed), and the deferred learned-skill prompt-injection approval gate with its residual risk and the reason the obvious low-risk mitigations don't apply (auto_activate=false is filtered out of criteria selection; trust attenuation needs a dedicated learned-skill source/dir first). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix skill manifest test fixture * Avoid bundled skill collision in runtime test * Avoid review keyword in filesystem skill test * Allow auto-activated skills in runtime asset test * fix(skill-learning): guard the two data-loss paths in learned-skill writes merge() now keeps the existing accumulated skill (KeepExisting) on a refiner error, an unparseable response, or a rejected injection scan of the merged doc, instead of overwriting it with the raw single-run candidate — a transient model hiccup must not discard a skill that accreted gotchas over many runs. Overwrite is reserved for the genuine no-existing-content case. install_or_update now matches SkillManagementErrorKind::Conflict specifically and fails loud on any other install error (filesystem/validation/resource), instead of treating every install failure as a name conflict and overwriting a live skill. Addresses review #1/#2 (data-loss/overwrite paths). Self-learning stays off by default (sink wired only with IRONCLAW_SKILL_LEARNING_MODEL + nearai), so these paths are unreachable in a default deployment; the broader hold-for-review / approval hardening lands in the stacked follow-up PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Address review feedback on skill activation --------- Co-authored-by: krishna <krishna@krishnadeMacBook-Air.local> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Robert Yan <mstr.raphael@gmail.com>
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
3 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
Change Type
Linked Issue
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warningscargo buildcargo test --features integrationif database-backed or integration behavior changedreview-prorpr-shepherd --fixwas run before requesting reviewSecurity Impact
Reborn Trust-Boundary Checklist
serde(default)fields fail closed or have migration tests.Transient,Permanent,Misconfigured,PolicyDeniedor equivalent).Database Impact
Blast Radius
Rollback Plan
Review Follow-Through
Review track: