mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
3f1faa66b2f2edcc80b801e8a1a500877fa0b241
3841 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
3f1faa66b2 |
fix(webui): keep failure bubbles with their prompts (#7301)
* fix(webui): keep failure bubbles with their prompts * fix(webui): address ironloop review — preserve request failure pairs (#7301) * fix(webui): keep stream failures at refresh boundary (#7301) |
||
|
|
72bcf876b8 |
fix(webui): clarify no-progress run failures (#7422)
* fix(webui): clarify no-progress run failures * test(webui): cover recovery-required run failures * fix(webui): preserve durable phases on run failure |
||
|
|
432d9d0522 |
feat(webui): replace Logs toolbar selects with shared SelectMenu (#7335)
* feat(webui): use shared log level select menu * test(e2e): cover logs level SelectMenu * fix(tests): address logs select review feedback |
||
|
|
9faa6cd2b1 |
refactor(webui): share native file-picker interaction (#7337)
* refactor(webui): share file picker interaction (#7333) * test(e2e): cover same-file picker reselection (#7333) * test(webui): assert native file input reset (#7333) * fix(webui): address file picker review feedback (#7333) |
||
|
|
2042380731 |
chore: remove IronLoop small-fix role instructions (#7415)
* chore: remove IronLoop small-fix role instructions * ci: classify IronLoop configuration changes |
||
|
|
226bd491dd |
ci(canary): remove provider-matrix lanes and zizmor scan (#7418)
* ci(canary): remove provider-matrix lanes and zizmor scan * fix(ci): classify nextest config as exhaustive-plan change .config/nextest.toml is read by every Tests (Reborn) lane, so the fail-closed planner arm raised 'unclassified pull-request path' on any PR touching it, skipping all downstream Reborn lanes. Widen it to the exhaustive plan like crate deletions. |
||
|
|
9dd228a62a |
fix(ci): clear inherited main check failures (#7425)
* fix(ci): scope POSIX trace test import to Unix * fix(ci): run Windows WebUI setup with Bash * test(ci): pin Bash for Windows WebUI setup * fix(ci): clear remaining main check failures --------- Co-authored-by: italic-jinxin <106428113+italic-jinxin@users.noreply.github.com> |
||
|
|
4e05a033d2 |
fix(webui): let a failed run's error bubble capture a trace (#7370)
A terminal run failure renders as a `role: "error"` chat message, which never carried a `turnRunId` and was excluded from the existing "download run artifact" action (gated on `role === assistant`). The action-button row itself was also gated on `showActions || showRetryAction`, neither true for an error bubble, so the button stayed hidden even once its role check was relaxed. Net effect: a failed run had no way to capture a trace for it (#7369). Thread `turnRunId` onto the run-failure message at creation, in-place update, and through the unknown-to-known-run-id promotion path, and let `showArtifactAction` fire for error messages that carry one. The existing download handler needed no changes — it was already keyed only on `threadId`/`turnRunId`, not role. Fixes #7369. Co-authored-by: Sergey <sergey@Mac.attlocal.net> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> |
||
|
|
4a5b3a6910 |
fix(webui): restore scoped attachment reads and SSE tests (#7341)
* fix(composition): read landed attachments through the per-caller workspace mount
/projects/workspace/tenants/{tenant}/users/{user}, but the loop-host
attachment_read_port still read through the shared read-only fixed view
(services.workspace_filesystem), which resolves the workspace root. A
landed image therefore came back NotFound at model-gateway time and was
silently dropped, so vision-capable model payloads lost every inline
image (the duration-4 Playwright attachment failure).
Wire the read port over the same per-caller scoped handle the WebUI
lander uses (runtime_mounts::read_write_workspace_filesystem), mirroring
what #7062 already did for the channel-host assembly. Under the Shared
policy the handle is byte-identical to the old fixed view; under
PerCaller it now resolves the caller's subtree.
* test(playwright): reconcile legacy WebUI v2 suites to the fetch-based SSE transport
event-source-plus (fetch/ReadableStream). The legacy suites still faked
window.EventSource, so the app never opened a stream and every
duration-1/duration-4 legacy test that emitted frames failed with "no
EventSource stream is open".
- Extract the smoke suite's proven fetch fake into
install_fake_v2_event_stream() in reborn_webui_harness, extended to
record request URLs and headers for reconnect assertions.
- Port all seven legacy scenario files onto it, updating cursor/token
assertions to the header contract (Authorization bearer,
Last-Event-ID) instead of the retired token/after_cursor query
params.
- legacy_skills delete: use the shared in-app confirmation dialog
instead of a native browser dialog.
- legacy_dom_resource_limits reconnect-timer: assert the pending
reconnect is cancelled when the tab hides (the fetch transport
schedules retries internally).
- legacy_rendering: assert no live onerror/iframe/img nodes instead of
substring-scanning escaped text.
- extensions_api: restore the #6520 wire contract (retired
authenticated/active/needs_setup/has_auth/onboarding_state booleans
must be absent).
- tool_execution truncated-tool test: expect model_output_truncated
failure per #6845's no-recovery contract instead of an assistant
recovery message.
- streaming_run_control_api: drop the stream=true assertion for the
OpenAI-compatible mock, which rides the buffered fallback since
#7120 (rig-core cannot distinguish a complete stream from a truncated
one).
|
||
|
|
fa6e72d7c0 | test(tool-search): add large-catalog baseline (#7409) | ||
|
|
1439e5b360 |
fix(webui): render gemoji shortcodes in chat markdown (#7404)
Co-authored-by: aiworkbot <220660587+aiworkbot@users.noreply.github.com> |
||
|
|
cd38d8834e |
chore(agents): refresh codebase knowledge graph (#7412)
Co-authored-by: github-merge-queue <118344674+github-merge-queue@users.noreply.github.com> |
||
|
|
9fd1e638a6 |
feat(inspector): complete statistics, navigation, and localization (#7291)
* feat(inspector): add operator inspection API * docs(inspector): assign product service ownership * test(inspector): ratchet diagnostic contracts * feat(inspector): add debug panel shell * test(inspector): cover debug panel shell e2e * fix(inspector): stop diagnostics when panel closes * feat(inspector): add prompt inspection * fix(inspector): follow current webui ownership * feat(inspector): add model call statistics * test(inspector): cover model statistics e2e * fix(inspector): avoid uncollected tool metrics * test(inspector): cover prompt diagnostics e2e * test(inspector): align statistics e2e scope * fix(inspector): redact prompt metadata * fix(inspector): preserve per-call model identity * fix(inspector): classify prompt instruction sources * test(inspector): assert reported token usage * feat(inspector): add activity timeline and turn navigation * test(inspector): cover activity timeline in browser * fix(inspector): read current run before publishing activity * feat(inspector): add bounded tool execution details * test(inspector): cover bounded tool details in browser * fix(inspector): validate retained tool result sizes * test(inspector): add security and operator coverage * test(inspector): cover browser workflows end to end * fix(inspector): address review feedback * fix(inspector): retry transient snapshot failures * fix(inspector): address prompt diagnostic review findings * fix(inspector): follow debug query navigation * feat(inspector): complete frontend diagnostics * test(inspector): cover frontend parity in browser * fix(inspector): preserve stream terminal state * fix(inspector): capture full capability surface * fix(inspector): scope projection activity to its run * fix(inspector): harden activity diagnostics * fix(inspector): bound tool result diagnostic capture * fix(inspector): harden tool diagnostic pipeline * fix(llm): request usage for NEAR AI streams * fix(inspector): address prompt diagnostic review feedback * fix(webui): harden inspector stream coverage * fix(inspector): preserve debug session statistics * fix(inspector): keep diagnostics active while hidden * test(e2e): cover hidden inspector observation * fix inspector model call stats review findings * fix inspector refresh and truncation regressions * fix(inspector): address activity timeline review feedback * fix(inspector): harden activity lifecycle handling * fix(composition): move tool diagnostics to loop host * fix(inspector): keep a settled stream live and complete locale parity A live diagnostic update's debounced snapshot refresh was announcing LOADING, so an open, healthy stream read as "Connecting" indefinitely once a run settled — the settling stats update is the last one. That refresh is now a background read. Incomplete snapshot statistics no longer accumulate as real zeros, browser-session inspector state is namespaced by the authenticated caller, an evicted pinned run rejoins the latest turn instead of the oldest, tool status is localized, and the inspector strings now cover all ten locales. * test(inspector): put the inspector locale sidecar under the parity gate The inspector's English copy is registered from its lazy chunk instead of src/i18n/en.ts, so the all-locale parity test — which derives the required key set from en.ts — never covered those keys; a locale could drop one and fall back to English silently. The test now treats the English key set as the union of en.ts and a declared sidecar list. Keeping the copy in en.ts is not an option: measured, it puts /chat at 217.4 KB gzip against a 217.0 KB budget. * fix(inspector): reject malformed model breakdowns and correct locale copy A `calls_per_model` entry with a negative or non-integer `calls` passed the statistics decoder and was then coerced to zero during accumulation without marking the breakdown truncated, presenting a fabricated "0 calls" for a model. Every entry is now validated before a record is accepted. German turn navigation used "Zug" (a train, or a game move); it now reads "Runde", with the determiner agreement that noun requires. Spanish and Portuguese tool-status values were written feminine against a masculine "Estado"/"Status" label. * fix(inspector): bound the model breakdown before scanning and retaining it The statistics decoder validated every calls_per_model entry but never the array length, so an out-of-contract response was scanned in full and then retained by the accumulator for up to 128 runs. The host truncates this breakdown at MAX_MODELS_IN_STATS and reports it as truncated, so a longer array cannot conform; the client now mirrors that ceiling and rejects the record before the scan. * fix(inspector): align turn navigation with host diagnostic retention The browser offered 32 turns of navigation per thread while the host retained diagnostics for 2 runs per session, so every turn past the second rendered blank. Each layer was individually correct and the e2e scenario stopped at two turns, so nothing saw the dead zone. Retention moves to 4 and the navigation window mirrors it, pinned by a new architecture gate that reads both constants; the scenario now walks back two turns and asserts real activity. Retention is a ceiling as well as a default, and capture is unconditional, so 4 is a resident memory choice — roughly 80 MB worst case across the eight tracked sessions. * fix(composition): delimit the i18n bundle guard with an i18n-owned marker The guard sliced the concatenated chunk bundle from the i18n provider up to `QueryClient`, a symbol another module owns, so the segment's extent tracked Rollup's chunk boundaries. A split that merely folded react-query into the entry chunk removed that marker from everything appended after the provider and failed an i18n guard with no i18n change. It now ends on the AVAILABLE_LANGUAGES literal that follows the provider in the same module; string literals survive minification, and every existing assertion holds against the tighter segment. * fix(architecture): resolve the inspector gate's SPA path through crate_path The gate joined a family-nested literal onto the workspace root, the idiom crate_path exists to replace: a crate family move would have turned this into a read failure rather than a resolved path. It now names the SPA file in the logical flat spelling and resolves it, and the assertion reports the resolved path so the message still points at a file that exists. * test(inspector): follow a pinned turn explicitly when a new turn arrives The multi-turn scenario assumed the panel would jump to an arriving turn, but a selection the operator navigated to is deliberately sticky: the new turn widens the window without yanking them off the turn they are reading. The scenario now asserts that guarantee, then clicks Latest to follow, then walks back two turns as before. Verified by running the inspector scenarios locally rather than by reading, which is how this slipped through the first time. |
||
|
|
89285c8e70 |
fix(skills): one DB-backed tree for every skill mount, and make a skill's own commands runnable (closes #7168) (#7171)
* fix(reborn): inject skill bodies by default, not a one-line listing Reborn defaulted `SkillInjectionMode` to `Listing`, where a non-activated skill contributes only `- name: description` to context and its body loads only on an explicit `$name` mention or a `builtin.skill_activate` call. The intent was to save context budget. Benchmarking shows the model reads the menu and then never opens the skill. Over 30 runs with human-curated skills installed (SkillsBench/SkillLearnBench subset, `deepseek-v4-flash`, nearai/benchmarks#287): builtin.skill_list called in 30/30 runs builtin.skill_activate called in 3/30 runs a skill body actually read 0/30 runs So installed skills were effectively inert. Same 31 tasks, same skills, same model, varying only this default: no skills 78.5% curated skills, Listing 79.8% (+1.3pp -- skills bought almost nothing) curated skills, Full 85.6% (+7.1pp) For reference, harnesses that inject skill bodies unconditionally (Hermes, Claude Code) score 91.5% on these tasks with the same skills, so `Full` closes most but not all of that gap; the remainder is loop/verification behavior on a handful of multi-output tasks and is tracked separately. `Full` is already the library default in `SkillActivationSelectorConfig`; only the Reborn composition seam opted out. This restores it and adds a guard test so a revert is deliberate. `IRONCLAW_REBORN_SKILL_INJECTION=listing` still selects the previous behavior where context budget matters more than skills being used. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(skills): hot-swappable activation strategies so agent-authored skills are reusable Adds `skill.activation.v1`, a swappable-provider module in the shape of the memory-provider binding (`ironclaw_host_runtime::memory_binding`): named strategies, fail-closed resolution, behavior-preserving default, and a composition seam so nothing downstream names a concrete implementation. ## The bug it addresses `selector::score_skill` accumulates score ONLY from `activation.keywords` (+10/+5), `activation.tags` (+3) and `activation.patterns` (+20). A skill's `name` and `description` contribute nothing, and `select_skills` keeps a skill only `if score > 0`. That is fine for curated skills, which ship an `activation` block. It is fatal for skills an agent writes for itself: measured across the 31-task SkillsBench/SkillLearnBench subset in nearai/benchmarks#287, **0 of 30** agent-authored skills contained an `activation` block. Every one scored 0 and was permanently unselectable — the agent could create a skill via `builtin.skill_install` and then never reuse it, which makes self-improvement structurally impossible rather than merely weak. Claude Code has no such requirement: a skill is selectable from name and description alone. `ActivationStrategy::NameAndDescription` ports that contract. ## Design * `CriteriaOnly` (default) — today's rule, byte-identical. * `NameAndDescription` — whole-word name/description fallback, applied ONLY when the criteria pass scored 0, so a curated skill's explicit keywords always decide ordering and this can never reorder two skills that both declare metadata. `NAME_WORD_SCORE` (8) is deliberately below the selector's exact-keyword award (10). * `Disabled` — explicit mention / `skill_activate` only. * `ThirdParty { extension_id }` — production requires an admin override. Whole-word matching and a `MAX_FALLBACK_SCORE` cap keep it from over-selecting; over-selection is the failure mode that makes injecting an unrelated skill bank harmful (a whole-catalog injection took `xlsx_recover_data` 1.000 -> 0.271). ## Default stays behavior-preserving Reborn's default remains `CriteriaOnly`, opt in with `IRONCLAW_REBORN_SKILL_ACTIVATION=name_and_description`. Flipping the default changes three existing local-dev expectations (setup-marker suppression, the webui listing candidate, `skill_activate` context loading), so the strategy ships opt-in — the same discipline as the memory work, where the bundled native provider stays the default. ## Tests `cargo test -p ironclaw_skills --lib` — 239 passed, including: * `agent_authored_skill_unreachable_by_default_but_selected_under_name_strategy` — end-to-end via `prefilter_skills_with_options`: the same no-activation skill is dropped under `CriteriaOnly` and selected under `NameAndDescription`. * `name_strategy_does_not_select_an_irrelevant_skill` — no over-selection. * `name_hit_outranked_by_an_explicit_curated_keyword`, `whole_word_only_...`, `fallback_is_capped_...`, `stop_words_do_not_accumulate_score`. `cargo test -p ironclaw_first_party_extension_ports --lib` — 58 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(reborn): ship the Full skill-injection default as opt-in, not a flip The measurement in the previous commit stands: `Listing` leaves installed skills unread (`skill_list` 30/30 runs, a body actually opened 0/30) and `Full` is worth 79.8% -> 85.6% on the 31-task SkillsBench subset. But flipping the product default HANGS three existing local-dev tests, which drive a mock that expects the one-line listing candidate: * `local_dev_skill_activate_tool_loads_selected_skill_context` * `local_dev_webui_bundle_records_selectable_filesystem_skill_context` * `local_dev_runtime_wires_filesystem_skills_by_default_to_model_calls` Verified by bisect: all three hang on the previous commit alone, and pass with the default restored — the activation-strategy work is not implicated. Changing a documented product default in a way that turns CI red is a maintainer call, not something to force through, so `DEFAULT_SKILL_INJECTION_MODE` returns to `Listing` and `Full` ships as `IRONCLAW_REBORN_SKILL_INJECTION=full`. Both switches in this PR are now opt-in with the evidence attached, matching the memory-provider discipline where the bundled default is preserved. The guard test is retargeted to assert the current default, verify the opt-in path still resolves, and name the three tests that must be updated alongside a future flip. cargo test -p ironclaw_reborn_composition --lib -- skill_injection_mode \ local_dev_selector_config skill_activation # 14 passed, 0 failed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(threads): raise the result_read cap to 64 KiB, env-tunable A small per-request `result_read` cap turns one large file into a paging loop. On `manufacturing_equipment_maintenance` (nearai/benchmarks#287) reborn made 8 `read_file` calls and ZERO shell calls, hit the 24 KiB cap, then spent the whole turn paging — `result_read` at offset 24576, `handbook.pdf` at offsets 400/800/1200 — and never computed anything (`outputs_exist=0.00`). hermes, using shell to sample the same data, scored 0.522. * `TOOL_RESULT_RECORD_READ_MAX_BYTES` 24 KiB -> 64 KiB. This is the compile-time ceiling the model-observation envelope in `tool_result_reference.rs` is derived from (`* 2`, asserted at compile time), so 64 KiB here means a 128 KiB envelope — the reason not to go higher. * `TOOL_RESULT_RECORD_READ_DEFAULT_MAX_BYTES` = 64 KiB — the effective default. Enough that a typical data file or document page arrives in one read instead of a paging loop. * `IRONCLAW_TOOL_RESULT_READ_MAX_BYTES` overrides it, clamped to `[4, ceiling]`, so an override can never outgrow the envelope. Unparseable values fall back to the default rather than failing the run — a malformed tuning knob must not take down an agent. Unlike the skill-injection and skill-activation switches in this branch, this one does move the default: the paging loop is a silent capability loss rather than a behavior preference, and the knob exists for deployments that want the old size. cargo test -p ironclaw_threads --lib # 88 passed (85 existing + 3 new) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(skills): add always_available activation, Claude Code's actual contract `skill.activation.v1` gains a third binding, `always_available`: every installed skill is a candidate regardless of what it matches. This is what Claude Code and Hermes actually do. In both, a skill is a file in a directory the agent can read, so there is no gate for a correctly-installed skill to fail. Reborn's selector instead scores only `activation.keywords`/`tags`/ `patterns` and drops anything scoring 0 -- and `name_and_description` (this branch's earlier binding) only WIDENS that gate: it still needs a lexical hit, so an applicable skill phrased differently from the prompt is still discarded. The new test pins exactly that case -- a skill described as "cyclical component / growth path" against a prompt saying "hp filter" is dropped by both `criteria_only` AND `name_and_description`, and kept by `always_available`. Why it matters, measured on the 31-task SkillsBench/SkillLearnBench subset in nearai/benchmarks#287: 0 of 30 agent-authored skills contained an `activation` block, so under `criteria_only` a self-authored skill could never be selected again -- self-improvement was structurally impossible. Implementation is deliberately tiny: a `floor_score()` of 1 for this binding, applied via `.max()` in the selector's existing scoring loop. Ordering is untouched (a real keyword match still outranks a floor skill, so the context budget spends on the relevant skill first), and the existing budget -- not the score filter -- decides what is injected, which is also how Claude Code behaves. `floor_score()` is 0 for every other binding, so non-adopters are byte-identical. Default remains `criteria_only`; opt in with IRONCLAW_REBORN_SKILL_ACTIVATION=always_available. cargo test -p ironclaw_skills --lib # 241 passed (239 existing + 2 new) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(threads): drop the now-unused ceiling import Validation bounds against `contract::effective_tool_result_read_max_bytes()` (which applies the env override), so the compile-time ceiling is no longer referenced here. Removes an unused-import warning introduced by the 64 KiB cap commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * revert(threads): default result_read back to 24 KiB, keep the knob The raise to 64 KiB was never isolated: it shipped in a measurement arm alongside two other switches (skill activation, tool disclosure), so there is no evidence it changed anything. Defaulting it back keeps this crate byte-identical to pre-PR behavior. The paging trace that motivated it is real (`manufacturing_equipment_maintenance`, nearai/benchmarks#287: 8 `read_file` calls, zero shell calls, `result_read` at offset 24576, nothing computed) — but a real trace is not a measured fix, so the larger cap stays opt-in via IRONCLAW_TOOL_RESULT_READ_MAX_BYTES for whoever wants to measure it properly. The compile-time ceiling stays 64 KiB: it now bounds only how far the env override may reach, and still pins the derived model-observation envelope at 128 KiB. Net effect of this commit plus its parent: a new env knob, no default change. cargo test -p ironclaw_threads --lib # 88 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): design for agent-authored multi-file skill bundles @henrypark133 pushed back on "move skills to the filesystem" as an overhaul that a single aggregate result did not justify. He was right, and stratifying the data shows why: the entire filesystem gain sits in skills that ship files besides SKILL.md. ships resource files (n=16): inject 81.0% -> files 94.2% (+13.2pp, CI [+0.3, +26.2]) SKILL.md-only (n=11): inject 91.5% -> files 84.7% (-6.9pp, CI [-20.4, +6.7]) So filesystem-for-everything is a REGRESSION on 13 of 31 tasks, paid to fix the other 18. The mechanism is not "models prefer filesystems": 81 of the resources are executable (you cannot run pasted Python -- citation_check scored 0.000 with the script absent, 0.833 with it present), and the text resources are too large to inline (exceltable_in_ppt would be ~262k tokens folded into SKILL.md). The design therefore keeps storage, discovery and selection exactly as they are and adds ONE extension holding the already-existing `/skills` read_write mount: skill_write_file / skill_read_file / skill_list_files. Discovery already lists from the same root that mount writes to, so nothing needs plumbing. Executing a bundled script copies that one file into `/workspace`, which the agent already mounts. Documents two things the implementation must not miss: SkillBundleDescriptor exposes only `skill_md_path`, so bundle resources are un-advertisable without skill_list_files; and `FilesystemSkillBundleRoot::user` marks bundles Trusted, so an agent that can write executable scripts there needs a distinct trust level -- the real open question. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): state explicitly that creation, discovery and indexing are unchanged The crux of @henrypark133's objection. Spells out, per concern, that skill creation stays on the `skill_install` tool, discovery stays on the storage-agnostic `SkillBundleSource` trait with no new impl / trait method / descriptor change, and that there is no session-start index to migrate at all (selection is per-request; the only cache is a 5-minute TTL on catalog search). The single behavioral change remains the opt-in `always_available` selection predicate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): the write tool needs an authoring prompt that asks for code skill_write_file makes multi-file skills possible; it does not elicit them. Measured: 6 of 31 tasks finished with ZERO skill_install calls despite 'Saving the skill is required', and the authoring request only ever asks for prose (method, conventions, output contract). An agent following it writes prose whether or not a write tool exists. Adds the elicitation requirement and a falsifiable success criterion: agent-authored bundles are currently 100% prose (0 of 27 ship a resource file) against 18 of 31 curated skills. If that ratio does not move once the tool ships, the bottleneck was elicitation rather than capability and the tool alone will not move scores. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(skills): let an agent install skill bundles, not just prose Agents could only ever author the PROSE half of a skill. Measured on the 31-task SkillsBench/SkillLearnBench subset (nearai/benchmarks#287): **0 of 27** agent-authored skills shipped a single file besides SKILL.md, against **18 of 31** human-curated ones (79 .py scripts, 78 .xsd schemas, 84 .md references). So every later run re-derived the method from prose and could re-make the same mistake -- lake_warming's self-authored skill described its regression procedure in prose, the next run recomputed it slightly differently and missed the grader's p<0.05 threshold. This was NOT a missing capability. `install_skill` has always taken `files: &[SkillInstallFile]`, and `parse_install_files` has always read an `input["files"]` array. Two things made it unreachable: 1. `schemas/builtin/skill_install.input.v1.json` advertised only `name`/`content`/`url` AND set `additionalProperties: false` -- so a model sending `files` was not merely uninformed, it was REJECTED. Across 112 observed skill_install calls, 111 used exactly `['content','name']`, which is what the schema permits. 2. The only encodings were `bytes_base64` and a JSON array of byte integers. A bundle file an agent writes is a script, a reference doc or a schema fragment -- all UTF-8. Making those go through base64 costs ~33% more tokens and turns one encoding slip into an InputEncode failure of the whole install. Changes: - `parse_install_files` accepts `text` (UTF-8) alongside `bytes_base64`/`bytes`. `text` takes precedence when both are given, matching the documented preference. Binary payloads are unaffected. - the schema advertises `files` with `path` + `text`/`bytes_base64`, and the description tells the model WHY to use it: put a reusable computation in a script rather than describing it in prose, and have SKILL.md name the files it relies on. That last part matters because `SkillBundleDescriptor` exposes only `skill_md_path`, so a bundle cannot advertise its own resources. - prose-only installs are untouched: no `files` key still parses to an empty vec. cargo test -p ironclaw_first_party_extensions --lib install_files_encoding # 4 passed cargo test -p ironclaw_host_runtime --test tool_surface_contract # 43 passed cargo test -p ironclaw_reborn_composition --test product_live_adapters skill_install # 1 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(skills): stop rejecting an install that carries both content and files `skill_install_input` gated the direct-install arm on `!object.contains_key("files")`, so `content` + `files` matched NO arm and fell through to `_ => Err(InputEncode)`. An agent attaching a script had its ENTIRE install refused. `files` was reachable only on the URL-fetch arm, which builds the array itself. This was the third of three stacked gates hiding the same capability, and the one that actually bit. With the schema fixed to advertise `files` and a `text` encoding available, the model on the 31-task SkillsBench subset (nearai/benchmarks#287) immediately sent 18 correctly-shaped `{path, text}` entries across 9 calls -- `scripts/verify_bib.py`, `references/fake_patterns.json` -- and every one was rejected here. That is the real reason 0 of 27 agent-authored skills shipped a resource file while 18 of 31 human-curated ones do: not a missing capability, and not the model failing to try. `source`/`source_url` stay excluded from the direct arm: those record provenance and are set by the URL path, so an agent must not be able to forge them. cargo test -p ironclaw_host_runtime --lib skill_install_input # 4 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * style: rustfmt the skill-bundle and activation changes Test modules were appended programmatically without rustfmt, which is why Formatting, Code Style and Clippy all went red on this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): rewrite to match what was measured, not the abandoned design The doc recommended a three-tool extension plus a resource-gate. Both are superseded: the tools turned out to be redundant (install_skill already accepted files -- three stacked gates were hiding it), and the gate MEASURED WORSE than always advertising a readable path (-25.7pp on self-creation, -40.6pp vs claude-code), because an agent-authored skill is usually SKILK.md-only so the gate suppresses the one route the selector had not already closed. Rewritten around the durable findings: the three gates and how each masked the next, the 0-of-27 vs 18-of-31 measurement, the SkillBundleDescriptor enumeration gap, and the trust question. The gate is kept in the doc as a recorded negative result, since its stratified justification is persuasive and will be proposed again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): correct why always_available is not the default The previous note claimed the floor score overrides setup-marker suppression. It does not: `prefilter_skills_with_options` returns None for a satisfied marker BEFORE scoring, and the host-side filter in activation.rs already removed the candidate. What actually fails: all 32 bundled skills reach floor 1, so 3-4 unrelated ones land in plan.activations() in ActivationCriteria mode -- a mode that injects nothing under Listing. The defect exposed is that a criteria activation which injects no body is still recorded as an activation, so the count assertions stop being meaningful. Also records the sequencing against epic #6565 (Slice 0 first; Slice 5's bounded-shortlist rule constrains what an unbounded floor may do) and the measured detail that under Listing a zero-scoring skill is still listed -- the model just called skill_activate in only 3 of 30 runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(skills): a floor-only skill is ranked, not activated Three defects, all surfaced by trying to make `always_available` the default. It failed 8 tests in ironclaw_reborn_composition; all 8 now pass with the flag on AND off. 1. A criteria selection that injects nothing was still recorded as an activation. Under `SkillInjectionMode::Listing` an `ActivationCriteria` entry contributes no body -- `body_eligible_bundle_ids` already ignores that mode -- so with a floor score every installed skill "activated" on every turn. Concretely: all 32 bundled skills reach floor 1, and 3 of them (6000 token budget / 2000 default per-skill cost) landed in each plan, chosen by descriptor order because the score sort is stable. `SelectionOutcome` now returns those separately as `ranked_only`, and the activation path does not iterate them. They still reach the model through the listing, which is where they belonged. 2. `AlwaysAvailable` also enabled the name/description fallback, which manufactured fake merit: a bundled skill whose description shares one word with the message scored above zero and was reported as a genuine activation. Under `AlwaysAvailable` the fallback adds no reach at all (the floor already admits everything), so it is now scoped to `NameAndDescription`, where widening the match is the entire point. This is what kept `local_dev_runtime_suppresses_explicit_setup_skill_when_workspace_marker_exists` failing after (1). 3. Raising TOOL_RESULT_RECORD_READ_MAX_BYTES to 64 KiB was NOT the no-op this PR claimed. `tool_result_reference.rs` derives MAX_MODEL_OBSERVATION_BYTES from it (* 2), so the observation envelope silently doubled 48 KiB -> 128 KiB and preview truncation changed for every caller. It broke three tests whose fixtures are sized against the envelope ("fixture must exceed the preview cap"), independently of any activation setting. The contract ceiling is back to 24 KiB and the env override is bounded by a new TOOL_RESULT_READ_ENV_CEILING_BYTES that nothing is derived from -- so the knob can raise a single read without moving anyone else's behavior. Correcting the record on an earlier comment in this PR: the failures were never the setup-marker interaction. Marker suppression returns None before scoring, so a floor score cannot revive a suppressed skill. cargo test -p ironclaw_reborn_composition --lib # 634 passed IRONCLAW_REBORN_SKILL_ACTIVATION=always_available cargo test -p ironclaw_reborn_composition --lib # 634 passed cargo test -p ironclaw_skills --lib # 241 cargo test -p ironclaw_threads --lib # 88 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * revert(skills): remove the always_available strategy, it bought nothing Verified against this branch: `AlwaysAvailable` was a no-op for everything the model can observe, and it carried a regression. Removing it rather than wiring a compensating half. Why it bought nothing. Listing membership is decided by VISIBILITY, not selection (extension_ports/activation.rs partitions candidates on body-eligibility, and everything not body-eligible still goes into the listing), so the model was ALREADY shown every visible skill before this strategy existed. The floor score never added reach -- pre-C1 its only effect was listing ORDER, and after C1 excluded floor-only skills from activations even the ordering effect was gone, because the ranking input is derived from the activation list. `SelectionOutcome::ranked_only` had no production reader at all: allocated, populated, returned, dropped. Under `Full` a floor-only skill could never be injected either, since `context_candidates_for_plan` renders only activated bundles. The regression. The floor-only bookkeeping ran for every non-merit entry BEFORE `try_select`, so under this strategy a chain-loaded companion got its own loop iteration, was recorded as floor-only, and was then partitioned OUT of `selected` -- i.e. `A requires B` activated only `A`, where `CriteriaOnly` activates both. Strictly worse than the default for any bundle with companions, and order-dependent. The comment claiming this could not happen was wrong. Also removed: ~29 "budget exhausted" notes per turn that reached `feedback` and fired a SkillActivation live-projection event with empty skill_names, because floor-only skills still ran the budget loop and `BudgetFull` continues rather than breaks. Kept: `NameAndDescription`, which has a real effect (matching on name/description, not only `activation.keywords`/`tags`/`patterns`), and the `skill.activation.v1` seam. Corrects the record in two places that argued the opposite: the runtime.rs doc comment and docs/skills/agent_authored_bundles.md. The measured reachability gap is elicitation, not filtering -- `builtin.skill_activate` was called in 3 of 30 runs and a body read in 0 of 30 -- so the next step is the listing header, not a scoring change. Note the parity numbers in nearai/benchmarks#327 never depended on this strategy: those arms ran with it off. cargo test -p ironclaw_reborn_composition --lib # 634 passed cargo test -p ironclaw_skills --lib # 240 passed cargo test -p ironclaw_threads --lib # 88 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(skills): explain refusals, gate requirements, survive discovery limits Epic #6565, the missing/unusable half. Four causes it names, each of which was silent. ## A refusal now says why `select_named_skill_activations` reported a skill that exists but is not `Trusted` with the same string as a name that does not exist: "requested skill is not available". The two need opposite responses -- one means "try a different name", the other means "this needs promoting and no name will work" -- and the model got neither. This is the routine outcome of the model doing what it was told: the listing filters on visibility only while activation requires `Trusted`, and tenant-shared and URL-installed skills are `Installed`. So the listing advertises a skill and activation then refuses it. Deliberately does NOT enumerate alternatives, tempting as that was: `load_named_activation_candidate_set` scopes the candidate set to the requested names, so nothing else is loaded at that point and any "available: ..." list would be empty. I wrote that branch, found it could never fire, and removed it rather than ship a message that lies. Offering alternatives needs a wider descriptor load and belongs with #4428. ## Requirements are actually checked `requires.bins`, `requires.env` and `requires.config` were parsed into the manifest and never consulted. `check_requirements` exists, but its only callers are inside `SkillRegistry`, which has no consumers outside its own crate. A skill declaring a binary it needs was offered, activated cleanly, and failed later in the shell with nothing connecting the failure to the unmet requirement. Gated at ACTIVATION time, on both the explicit-mention and model-selected paths. Not at listing time: that would be three probes per visible skill on every prompt build and needs a caching design first. At activation it runs for the handful of skills being loaded, so the cost objection does not apply. Staying unusable is correct here -- the fix is that the reason reaches the model. ## One oversized root no longer erases itself `list_root` returned `BundleScanLimitExceeded` when a root held more than `max_bundles_per_root` directories, which removed EVERY skill in that root from the model's view. A catalog that grew past the cap lost all its skills at once, with no signal to the model and only a propagated error to the operator. Now it keeps the bundles that fit and warns about the truncation. ## Silent skips became warnings that name the reason Two `debug!` sites -- an invalid bundle directory name, and a manifest that fails validation (which covers the common authoring mistake of a directory name disagreeing with the manifest `name:`) -- meant a skill present on disk simply never appeared and nothing said why. Both are `warn!` with the error attached. ## Tests Three that pinned the old behaviour were rewritten rather than deleted, each with why: the two refusal-message assertions, and the scan-limit test that asserted total root loss. New coverage for the trust-vs-name distinction and for an unmet binary requirement being refused with the requirement named. `cargo test -p ironclaw_first_party_extension_ports -p ironclaw_loop_host` — 68 + 420 + 27 + 4 + 88 pass. fmt and clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): surface the refusal reason to the model, not just the projection The reason strings were the easy half. `skill_activate`'s result was built from `plan.selection.activations` only and **discarded `plan.selection.feedback` entirely**, so every reason the selector produces was constructed and thrown away. The model saw `{"activated":[],"count":0}` and had to guess whether it had used a bad name, hit a trust wall, or tripped an unmet requirement -- three situations that need three different responses. Caught by measurement rather than review: on the missing/unusable fixtures `usable` moved after the earlier commit but `diagnosed` stayed flat at 2/7, because improving the wording of a message nobody receives changes nothing. Adds `not_activated` alongside `activated`. Routine "activated after model selection" confirmations are filtered out -- next to `activated` they are noise and would dilute the refusals that matter. Output construction is extracted into `build_activation_output` so the contract is unit-testable in the same style as the rest of the module: a refusal carries its reason, a clean activation gains no empty field, and a mixed result reports both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(skills): the model decides, and the listing can never be hidden from it Epic #6941 criteria 1 and 8. Adopts @serrrfirat's position -- "I don't think we should statically regex match skill names to skill activation... we should show it to the model and it should decide" -- as the default rather than a flag. ## The default is now ExplicitOnly `SkillActivationSelectorConfig::default()` no longer runs the keyword/regex scorer. A profile that wants it must ask for `ExplicitAndCriteria` deliberately; nothing inherits it silently, which is how #5417 shipped. The scorer's own record is the argument for retiring it: * It produced #5417 -- `tech-debt-tracker` declares the keyword `hack`, so "search Hacker News for..." activated it. * Over 328 real prompts `coding` fired on ~220 through *legitimate whole-word* hits on `file`/`change`/`code`. No boundary rule or score threshold can fix that. * Measured against it, the model path made **zero** wrong selections across 28 tasks over an 88-skill catalog, at **94.8%** precision on what it did activate. The scorer is not deleted: it is still correct, still tested, and still reachable for a profile that opts in. It is simply no longer the thing that decides. ## And a trap that had to be fixed in the same commit In `Full` injection mode both context paths returned an EMPTY candidate set when nothing was active. That was survivable only while the scorer auto-activated something. With model-decides it would mean the model is never told a skill exists and therefore can never activate one -- flipping this default alone would have blinded the agent. Both paths now fall through to the listing. ## Tests Three new criterion tests: the default policy is pinned (so a silent revert fails), the listing survives with nothing activated in BOTH injection modes, and the listing stays inside a stated character budget at 200 skills -- with the scorer retired the listing IS the routing interface, so its size is a correctness property rather than a cosmetic one. Sixteen existing tests were updated rather than deleted, in two groups, each with its reason in the code: * Tests whose SUBJECT is the scorer now call `criteria_config()` to opt in. That is the point of the new default -- nothing gets the scorer by accident. Includes `prepare_grants_asset_reads_only_for_activated_bundles`, which uses criteria selection only as the mechanism to activate a bundle; I checked that one first because an asset-grant assertion failing could have been a real security regression rather than an expectation change. It was not. * Tests asserting `selected.is_empty()` now assert `assert_no_skill_body_disclosed(..)`. "No candidates" is no longer the right question; "no skill BODY reached the model" is what they were really protecting, and it is exact -- the listing is a *discoverable* candidate (`loaded_skill_md() == None`) while an activated skill is a *loaded* one. `cargo test -p ironclaw_first_party_extension_ports -p ironclaw_loop_host -p ironclaw_skills` -- 74 + 420 + 27 + 4 + 88 + 229 pass. fmt and clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): pin #5417 on the path that records the user message Epic #6941 criterion 2. Criteria selection needs a RECORDED user message (`take_message_for_run`), and the coordinator path never records one -- so a coordinator-path test passes vacuously and proves nothing. This records the message, which is what the product/WebUI surface does, and the issue itself reports "Run origin: WebUI chat". Asserts BOTH policies, including the uncomfortable one: * model-decides (the new default): no body is injected. Fixed. * criteria explicitly enabled: it STILL mis-activates on this branch. Asserted as a known residual rather than omitted. That second arm is the useful half. It shows the two changes are complementary rather than redundant: this PR removes the scorer from the decision, #6937's word-boundary matcher stops `hack` matching inside "Hacker" for any profile that opts the scorer back in. Neither alone closes #5417 on the criteria path, and pinning it here means a future reader cannot mistake model-decides for a complete fix. The assertion message says what to do when #6937 merges and the arm flips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): a truncated skill listing must say so `MAX_LISTED_SKILLS` is 100 and the listing is source-then-name ordered, so past the cap whole alphabetical tails vanish -- with no signal to the model and none to the operator. Found by walking into it. Running Benchmark A against a 227-skill catalog, `pdf`, `pptx`, `xlsx` and `timeseries-detrending` all sorted past position 100, so three of the first four tasks could not reach their own expected skill and the arm was measuring nothing. Nothing anywhere reported it; I only caught it by diffing the rendered listing against each task's expected set. That is the failure mode this epic exists to remove, and it was hiding in the listing itself. Now the listing states how many skills are hidden, and the host logs a warning with listed/hidden/total. This does NOT make a large catalog usable -- that needs `skill_search` (#4428), and a 227-skill listing costs ~9k tokens of prompt besides. What it does is turn a silent, invisible failure into a stated one, so a benchmark or a user hitting the cap finds out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): make "the model decides" reachable, and list every skill instead of the first 100 Two defects found by measuring this PR rather than reviewing it. Both made a claim the PR already made untrue in practice. **1. The ExplicitOnly default was dead code on the Reborn path.** `skill_activation_selector_config` pinned `ExplicitAndCriteria` at the call site, so changing the default in `activation.rs` could not affect any real Reborn user. The model-decides change looked like a behaviour change and was not one. No test in `ironclaw_first_party_extension_ports` could catch this, because those construct their own config; only a test on the value the composition layer actually returns can. `reborn_skill_selection_is_model_decided` is that test, and it fails if the mode is re-pinned. **2. The skill listing silently dropped everything past 100 skills.** `MAX_LISTED_SKILLS = 100` with source-then-name ordering meant whole alphabetical tails were rendered nowhere and logged nowhere. Measured on a 227-skill catalog: `pdf`, `pptx`, `xlsx` and `timeseries-detrending` all sorted past the cap, and three of the first four benchmark tasks could not reach their own expected skill. A skill the model cannot see is one it cannot activate, so this is indistinguishable from never having installed it — and with the scorer retired, the listing IS the routing interface, which makes its completeness a correctness property. The flat count cap becomes a character budget spent differently: every skill's name is listed, with per-entry descriptions shrinking as the catalog grows (250 chars at small sizes, 90 at 227), and entries are dropped only when even 60 chars will not fit — roughly past 380 skills. **This is not a context-size increase.** The budget is exactly what the old cap already permitted (`100 * (250 + 64)`). What changes is that it buys reachability for all skills rather than verbosity for the alphabetically lucky first hundred. When truncation does happen it is stated in the listing and warned with `listed`/`hidden`/`total`. `the_listing_stays_within_budget_at_two_hundred_skills` now asserts both that the listing fits its budget *and* that all 200 skills appear in it. The second assertion is the one the old cap violated: that test previously passed on budget alone while hiding half the catalog, which is how this survived. Beyond ~380 skills the answer is `skill_search` (#4428), not a bigger prompt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): raise the per-root bundle scan cap so a real catalog enumerates whole Found by measuring: after replacing the listing's 100-skill cap with a character budget, a 227-skill root STILL only reached the model as 100 skills. The listing was never the binding limit. `DEFAULT_MAX_BUNDLES_PER_ROOT = 100` truncates one layer earlier, at enumeration: `skill bundle root exceeds the per-root scan limit ... limit=100 skipped=127`. The 127 skipped bundles were invisible to the selector, to the listing, and to the model — the same outcome as never installing them. Two caps, and only the lower one decides, so raising the listing budget alone accomplished nothing. Raised to 512. The cap exists to bound an unbounded directory walk, not to bound a catalog; 512 keeps that protection (a bundle is one directory read plus a manifest parse, cached per root) while leaving real catalogs whole. Past it, truncation is still partial-and-warned rather than fatal, and the answer is `skill_search` (#4428) rather than a larger number. `a_two_hundred_and_twenty_seven_skill_root_enumerates_whole` asserts the default cap does not truncate a real catalog. The existing test only proved truncation *degrades gracefully*, which is why a default too low to fit anything real passed it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): let the scorer rank the listing, while the model alone decides Retiring the criteria scorer from ACTIVATION is right and stays. Retiring it from ORDERING was a mistake, and the benchmark caught it. Measured on the 31-task routing benchmark, 88 candidates, same model, paired: | | criteria on | ExplicitOnly | |--------------------------|-------------|--------------| | >=1 correct skill | 57.1% | 28.6% | | recall over expected set | 42.9% | 25.0% | | correct skill REQUESTED | 75.0% | 35.7% | | never called skill_activate | 25.0% | 57.1% | The model was not being refused -- refusals were 0% in both arms. It stopped asking. The scorer was the only thing making a long listing legible: with it off, the listing collapses to source-then-name alphabetical order, and the relevant skill sits among dozens of equally-weighted lines with nothing marking it. At 227 candidates this is worse, not better. So the scorer keeps its useful job and loses its harmful one. Under `ExplicitOnly` the prefilter still runs, and its output populates `SkillActivationSelection::ranking_only`, which feeds `criteria_ranked_bundle_ids` and therefore listing order ONLY. It never appends to `activations`, and it deliberately does not extend `feedback` (those notes explain activation decisions, and nothing was activated). This is the distinction @serrrfirat's objection actually draws. "Don't statically regex match skill names to skill activation" is not "don't use scoring to rank what the model is shown". Ordering a menu is not choosing from it. The host recommends; the model decides; a wrong recommendation costs a listing line rather than the skill budget. `explicit_only_ranks_the_listing_without_activating_anything` pins both halves in one test, because they pull in opposite directions: the matched skill must LEAD the listing, and no body may be disclosed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Revert "fix(skills): let the scorer rank the listing, while the model alone decides" This reverts |
||
|
|
6384fa6c02 |
ci(nightly): grant actions: read to the reborn-tests call contract (#7323)
reborn-tests.yml's coverage-report job requests job-level `actions:
read` since #7018 (it fetches the base commit's merged-lcov artifact for
the changed-line coverage gate). GitHub validates called-workflow
permissions at trigger time, and the nightly caller grants only
`contents: read` + `pull-requests: write` — so every scheduled run
since 2026-08-03 (first run at
|
||
|
|
c7ec378f4e |
chore(agents): refresh codebase knowledge graph (#7399)
Co-authored-by: github-merge-queue <118344674+github-merge-queue@users.noreply.github.com> |
||
|
|
0b5fcde996 |
feat!: a run acts as its invoker — remove shared-route subject binding (#7157 follow-ups) (#7377)
* feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted Re-landed PR #7157 on current main ( |
||
|
|
30ae2d50f6 |
feat(stress): scripted tool-call workload with durable write read-back (#7360) (#7382)
* feat(stress): scripted tool-call workload with durable write read-back (#7360) Phase 1 of issue #7360: teach the stress harness to drive real builtin and memory tool calls through the production capability path and verify their durable side effects. The api-user-capacity mock LLM sidecar learns a deterministic scripted state machine: the driver embeds an `ironclaw-stress-tool` marker in the user message, the sidecar emits the scripted tool call for a tool advertised in the request, the server executes it through the real capability host, and the driver verifies the read-back verdict in the final assistant message. Verdicts: confirmed / contended (same-user CAS race, counted) / leak (cross-user isolation, hard failure) / missing (write lost, hard failure) / undisclosed (tool never advertised). Scripts: write_file_roundtrip (write_file + read_file of a unique workspace path), memory_roundtrip / memory_grow / memory_mixed (ironclaw.memory.write/replace-append + read of the shared stress/shared.md target — every run doubles as a same-relative-path isolation check). --api-scripted-doc-sizes cycles 4 KiB..1 MiB documents with per-size buckets and submit-to-tool-visible / submit-to-finalize stage latencies; --api-hot-writers spawns concurrent same-user writers for hot-document CAS contention. Gated tools are exercised through the per-user Tools auto-approve setting enabled during setup via the production settings API. Wired as a nightly leg in the hosted-single-tenant Postgres job (the existing server stays up; the leg rebinds the mock sidecar on the same port). Unit coverage: marker parsing, per-op step sequencing, tool-name resolution (encoded/dotted/bare), verdict computation incl. leak precedence, disclosure fallback, timeline helpers, per-size summary buckets, and flag validation. * fix(stress): hot writers on distinct user threads, size floor, CI server lifecycle Review fixes for the #7360 Phase 1 scripted workload: - Hot writers now run on distinct threads of the first user instead of sharing one thread, so concurrent operations exercise real per-user memory-document CAS contention rather than per-thread turn serialization. setup_users creates and records one extra thread per hot writer for user 0; run_hot_writer picks its own thread. - Scripted document sizes are floored at 4 KiB (the token-dominated region below is meaningless and the issue's workloads start there); enforced in marker parsing and --api-scripted-doc-sizes validation. - The CI scripted leg runs inside the server's run block so the trap does not kill the server before it starts; artifacts upload together. - Wire-shape tests: mock_tool_call_response deserializes as the rig OpenAI CompletionResponse (stringified arguments, finish_reason tool_calls) and streaming tool-call chunks carry indexed delta tool_calls. * fix(stress): hot-writer client action ids collide with the primary writer A hot writer and the first user's regular writer shared the same user label and operation index, so their client_action_id values were identical and the server rejected the second submit with a 409 duplicate conflict. Include the scripted op prefix (h{k}-) in the operation ref so concurrent writers always submit distinct action ids. Found by a full local E2E run of the scripted leg against a real hosted-single-tenant server: after the fix, memory_roundtrip with one hot writer runs 9/9 clean (6 confirmed + 3 contended, 0 leaks) and memory_grow runs 8/8 confirmed. * fix(stress): address coderabbit review — verdict integrity, op-scoped tool counts, typed script key (#7382) - compute_verdict: verdict comes from read steps only (write echoes can no longer mask missing/contended) - timeline tool evidence: count tool results by sequence above the op's baseline instead of subtracting page-limited absolute counts - timeline verdict match: delimit prefix by trailing space so op 1 cannot terminate on op 10's message; parse_result_verdict aligns on substring - background users namespace markers as b{index} so cross-cohort leaks cannot read back as their own token - hot writers drain in a dedicated JoinSet (no run_virtual_user refills) - fraction chunks derive from cumulative boundaries so split writes persist exactly the configured size (regression test at 4097) - ScriptKey derives clap::ValueEnum: CLI, marker wire format, and parsing share one string mapping; --api-hot-writers rejects write_file_roundtrip - parse_marker bounds identity grammar; poisoned mutex recovery; sorted stage latencies; single conversation parse per completion request; CLI-level scripted validation test and doc-size bound coverage * test(stress): cover --api-wait-for-assistant gate in CLI-level scripted test (#7382) |
||
|
|
102e5e05b8 |
fix(skills): the model chooses the skill, not a keyword scorer (#6938)
* fix(reborn): inject skill bodies by default, not a one-line listing Reborn defaulted `SkillInjectionMode` to `Listing`, where a non-activated skill contributes only `- name: description` to context and its body loads only on an explicit `$name` mention or a `builtin.skill_activate` call. The intent was to save context budget. Benchmarking shows the model reads the menu and then never opens the skill. Over 30 runs with human-curated skills installed (SkillsBench/SkillLearnBench subset, `deepseek-v4-flash`, nearai/benchmarks#287): builtin.skill_list called in 30/30 runs builtin.skill_activate called in 3/30 runs a skill body actually read 0/30 runs So installed skills were effectively inert. Same 31 tasks, same skills, same model, varying only this default: no skills 78.5% curated skills, Listing 79.8% (+1.3pp -- skills bought almost nothing) curated skills, Full 85.6% (+7.1pp) For reference, harnesses that inject skill bodies unconditionally (Hermes, Claude Code) score 91.5% on these tasks with the same skills, so `Full` closes most but not all of that gap; the remainder is loop/verification behavior on a handful of multi-output tasks and is tracked separately. `Full` is already the library default in `SkillActivationSelectorConfig`; only the Reborn composition seam opted out. This restores it and adds a guard test so a revert is deliberate. `IRONCLAW_REBORN_SKILL_INJECTION=listing` still selects the previous behavior where context budget matters more than skills being used. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(skills): hot-swappable activation strategies so agent-authored skills are reusable Adds `skill.activation.v1`, a swappable-provider module in the shape of the memory-provider binding (`ironclaw_host_runtime::memory_binding`): named strategies, fail-closed resolution, behavior-preserving default, and a composition seam so nothing downstream names a concrete implementation. ## The bug it addresses `selector::score_skill` accumulates score ONLY from `activation.keywords` (+10/+5), `activation.tags` (+3) and `activation.patterns` (+20). A skill's `name` and `description` contribute nothing, and `select_skills` keeps a skill only `if score > 0`. That is fine for curated skills, which ship an `activation` block. It is fatal for skills an agent writes for itself: measured across the 31-task SkillsBench/SkillLearnBench subset in nearai/benchmarks#287, **0 of 30** agent-authored skills contained an `activation` block. Every one scored 0 and was permanently unselectable — the agent could create a skill via `builtin.skill_install` and then never reuse it, which makes self-improvement structurally impossible rather than merely weak. Claude Code has no such requirement: a skill is selectable from name and description alone. `ActivationStrategy::NameAndDescription` ports that contract. ## Design * `CriteriaOnly` (default) — today's rule, byte-identical. * `NameAndDescription` — whole-word name/description fallback, applied ONLY when the criteria pass scored 0, so a curated skill's explicit keywords always decide ordering and this can never reorder two skills that both declare metadata. `NAME_WORD_SCORE` (8) is deliberately below the selector's exact-keyword award (10). * `Disabled` — explicit mention / `skill_activate` only. * `ThirdParty { extension_id }` — production requires an admin override. Whole-word matching and a `MAX_FALLBACK_SCORE` cap keep it from over-selecting; over-selection is the failure mode that makes injecting an unrelated skill bank harmful (a whole-catalog injection took `xlsx_recover_data` 1.000 -> 0.271). ## Default stays behavior-preserving Reborn's default remains `CriteriaOnly`, opt in with `IRONCLAW_REBORN_SKILL_ACTIVATION=name_and_description`. Flipping the default changes three existing local-dev expectations (setup-marker suppression, the webui listing candidate, `skill_activate` context loading), so the strategy ships opt-in — the same discipline as the memory work, where the bundled native provider stays the default. ## Tests `cargo test -p ironclaw_skills --lib` — 239 passed, including: * `agent_authored_skill_unreachable_by_default_but_selected_under_name_strategy` — end-to-end via `prefilter_skills_with_options`: the same no-activation skill is dropped under `CriteriaOnly` and selected under `NameAndDescription`. * `name_strategy_does_not_select_an_irrelevant_skill` — no over-selection. * `name_hit_outranked_by_an_explicit_curated_keyword`, `whole_word_only_...`, `fallback_is_capped_...`, `stop_words_do_not_accumulate_score`. `cargo test -p ironclaw_first_party_extension_ports --lib` — 58 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(reborn): ship the Full skill-injection default as opt-in, not a flip The measurement in the previous commit stands: `Listing` leaves installed skills unread (`skill_list` 30/30 runs, a body actually opened 0/30) and `Full` is worth 79.8% -> 85.6% on the 31-task SkillsBench subset. But flipping the product default HANGS three existing local-dev tests, which drive a mock that expects the one-line listing candidate: * `local_dev_skill_activate_tool_loads_selected_skill_context` * `local_dev_webui_bundle_records_selectable_filesystem_skill_context` * `local_dev_runtime_wires_filesystem_skills_by_default_to_model_calls` Verified by bisect: all three hang on the previous commit alone, and pass with the default restored — the activation-strategy work is not implicated. Changing a documented product default in a way that turns CI red is a maintainer call, not something to force through, so `DEFAULT_SKILL_INJECTION_MODE` returns to `Listing` and `Full` ships as `IRONCLAW_REBORN_SKILL_INJECTION=full`. Both switches in this PR are now opt-in with the evidence attached, matching the memory-provider discipline where the bundled default is preserved. The guard test is retargeted to assert the current default, verify the opt-in path still resolves, and name the three tests that must be updated alongside a future flip. cargo test -p ironclaw_reborn_composition --lib -- skill_injection_mode \ local_dev_selector_config skill_activation # 14 passed, 0 failed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(threads): raise the result_read cap to 64 KiB, env-tunable A small per-request `result_read` cap turns one large file into a paging loop. On `manufacturing_equipment_maintenance` (nearai/benchmarks#287) reborn made 8 `read_file` calls and ZERO shell calls, hit the 24 KiB cap, then spent the whole turn paging — `result_read` at offset 24576, `handbook.pdf` at offsets 400/800/1200 — and never computed anything (`outputs_exist=0.00`). hermes, using shell to sample the same data, scored 0.522. * `TOOL_RESULT_RECORD_READ_MAX_BYTES` 24 KiB -> 64 KiB. This is the compile-time ceiling the model-observation envelope in `tool_result_reference.rs` is derived from (`* 2`, asserted at compile time), so 64 KiB here means a 128 KiB envelope — the reason not to go higher. * `TOOL_RESULT_RECORD_READ_DEFAULT_MAX_BYTES` = 64 KiB — the effective default. Enough that a typical data file or document page arrives in one read instead of a paging loop. * `IRONCLAW_TOOL_RESULT_READ_MAX_BYTES` overrides it, clamped to `[4, ceiling]`, so an override can never outgrow the envelope. Unparseable values fall back to the default rather than failing the run — a malformed tuning knob must not take down an agent. Unlike the skill-injection and skill-activation switches in this branch, this one does move the default: the paging loop is a silent capability loss rather than a behavior preference, and the knob exists for deployments that want the old size. cargo test -p ironclaw_threads --lib # 88 passed (85 existing + 3 new) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(skills): add always_available activation, Claude Code's actual contract `skill.activation.v1` gains a third binding, `always_available`: every installed skill is a candidate regardless of what it matches. This is what Claude Code and Hermes actually do. In both, a skill is a file in a directory the agent can read, so there is no gate for a correctly-installed skill to fail. Reborn's selector instead scores only `activation.keywords`/`tags`/ `patterns` and drops anything scoring 0 -- and `name_and_description` (this branch's earlier binding) only WIDENS that gate: it still needs a lexical hit, so an applicable skill phrased differently from the prompt is still discarded. The new test pins exactly that case -- a skill described as "cyclical component / growth path" against a prompt saying "hp filter" is dropped by both `criteria_only` AND `name_and_description`, and kept by `always_available`. Why it matters, measured on the 31-task SkillsBench/SkillLearnBench subset in nearai/benchmarks#287: 0 of 30 agent-authored skills contained an `activation` block, so under `criteria_only` a self-authored skill could never be selected again -- self-improvement was structurally impossible. Implementation is deliberately tiny: a `floor_score()` of 1 for this binding, applied via `.max()` in the selector's existing scoring loop. Ordering is untouched (a real keyword match still outranks a floor skill, so the context budget spends on the relevant skill first), and the existing budget -- not the score filter -- decides what is injected, which is also how Claude Code behaves. `floor_score()` is 0 for every other binding, so non-adopters are byte-identical. Default remains `criteria_only`; opt in with IRONCLAW_REBORN_SKILL_ACTIVATION=always_available. cargo test -p ironclaw_skills --lib # 241 passed (239 existing + 2 new) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(threads): drop the now-unused ceiling import Validation bounds against `contract::effective_tool_result_read_max_bytes()` (which applies the env override), so the compile-time ceiling is no longer referenced here. Removes an unused-import warning introduced by the 64 KiB cap commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * revert(threads): default result_read back to 24 KiB, keep the knob The raise to 64 KiB was never isolated: it shipped in a measurement arm alongside two other switches (skill activation, tool disclosure), so there is no evidence it changed anything. Defaulting it back keeps this crate byte-identical to pre-PR behavior. The paging trace that motivated it is real (`manufacturing_equipment_maintenance`, nearai/benchmarks#287: 8 `read_file` calls, zero shell calls, `result_read` at offset 24576, nothing computed) — but a real trace is not a measured fix, so the larger cap stays opt-in via IRONCLAW_TOOL_RESULT_READ_MAX_BYTES for whoever wants to measure it properly. The compile-time ceiling stays 64 KiB: it now bounds only how far the env override may reach, and still pins the derived model-observation envelope at 128 KiB. Net effect of this commit plus its parent: a new env knob, no default change. cargo test -p ironclaw_threads --lib # 88 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): design for agent-authored multi-file skill bundles @henrypark133 pushed back on "move skills to the filesystem" as an overhaul that a single aggregate result did not justify. He was right, and stratifying the data shows why: the entire filesystem gain sits in skills that ship files besides SKILL.md. ships resource files (n=16): inject 81.0% -> files 94.2% (+13.2pp, CI [+0.3, +26.2]) SKILL.md-only (n=11): inject 91.5% -> files 84.7% (-6.9pp, CI [-20.4, +6.7]) So filesystem-for-everything is a REGRESSION on 13 of 31 tasks, paid to fix the other 18. The mechanism is not "models prefer filesystems": 81 of the resources are executable (you cannot run pasted Python -- citation_check scored 0.000 with the script absent, 0.833 with it present), and the text resources are too large to inline (exceltable_in_ppt would be ~262k tokens folded into SKILL.md). The design therefore keeps storage, discovery and selection exactly as they are and adds ONE extension holding the already-existing `/skills` read_write mount: skill_write_file / skill_read_file / skill_list_files. Discovery already lists from the same root that mount writes to, so nothing needs plumbing. Executing a bundled script copies that one file into `/workspace`, which the agent already mounts. Documents two things the implementation must not miss: SkillBundleDescriptor exposes only `skill_md_path`, so bundle resources are un-advertisable without skill_list_files; and `FilesystemSkillBundleRoot::user` marks bundles Trusted, so an agent that can write executable scripts there needs a distinct trust level -- the real open question. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): state explicitly that creation, discovery and indexing are unchanged The crux of @henrypark133's objection. Spells out, per concern, that skill creation stays on the `skill_install` tool, discovery stays on the storage-agnostic `SkillBundleSource` trait with no new impl / trait method / descriptor change, and that there is no session-start index to migrate at all (selection is per-request; the only cache is a 5-minute TTL on catalog search). The single behavioral change remains the opt-in `always_available` selection predicate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): the write tool needs an authoring prompt that asks for code skill_write_file makes multi-file skills possible; it does not elicit them. Measured: 6 of 31 tasks finished with ZERO skill_install calls despite 'Saving the skill is required', and the authoring request only ever asks for prose (method, conventions, output contract). An agent following it writes prose whether or not a write tool exists. Adds the elicitation requirement and a falsifiable success criterion: agent-authored bundles are currently 100% prose (0 of 27 ship a resource file) against 18 of 31 curated skills. If that ratio does not move once the tool ships, the bottleneck was elicitation rather than capability and the tool alone will not move scores. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(skills): let an agent install skill bundles, not just prose Agents could only ever author the PROSE half of a skill. Measured on the 31-task SkillsBench/SkillLearnBench subset (nearai/benchmarks#287): **0 of 27** agent-authored skills shipped a single file besides SKILL.md, against **18 of 31** human-curated ones (79 .py scripts, 78 .xsd schemas, 84 .md references). So every later run re-derived the method from prose and could re-make the same mistake -- lake_warming's self-authored skill described its regression procedure in prose, the next run recomputed it slightly differently and missed the grader's p<0.05 threshold. This was NOT a missing capability. `install_skill` has always taken `files: &[SkillInstallFile]`, and `parse_install_files` has always read an `input["files"]` array. Two things made it unreachable: 1. `schemas/builtin/skill_install.input.v1.json` advertised only `name`/`content`/`url` AND set `additionalProperties: false` -- so a model sending `files` was not merely uninformed, it was REJECTED. Across 112 observed skill_install calls, 111 used exactly `['content','name']`, which is what the schema permits. 2. The only encodings were `bytes_base64` and a JSON array of byte integers. A bundle file an agent writes is a script, a reference doc or a schema fragment -- all UTF-8. Making those go through base64 costs ~33% more tokens and turns one encoding slip into an InputEncode failure of the whole install. Changes: - `parse_install_files` accepts `text` (UTF-8) alongside `bytes_base64`/`bytes`. `text` takes precedence when both are given, matching the documented preference. Binary payloads are unaffected. - the schema advertises `files` with `path` + `text`/`bytes_base64`, and the description tells the model WHY to use it: put a reusable computation in a script rather than describing it in prose, and have SKILL.md name the files it relies on. That last part matters because `SkillBundleDescriptor` exposes only `skill_md_path`, so a bundle cannot advertise its own resources. - prose-only installs are untouched: no `files` key still parses to an empty vec. cargo test -p ironclaw_first_party_extensions --lib install_files_encoding # 4 passed cargo test -p ironclaw_host_runtime --test tool_surface_contract # 43 passed cargo test -p ironclaw_reborn_composition --test product_live_adapters skill_install # 1 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(skills): stop rejecting an install that carries both content and files `skill_install_input` gated the direct-install arm on `!object.contains_key("files")`, so `content` + `files` matched NO arm and fell through to `_ => Err(InputEncode)`. An agent attaching a script had its ENTIRE install refused. `files` was reachable only on the URL-fetch arm, which builds the array itself. This was the third of three stacked gates hiding the same capability, and the one that actually bit. With the schema fixed to advertise `files` and a `text` encoding available, the model on the 31-task SkillsBench subset (nearai/benchmarks#287) immediately sent 18 correctly-shaped `{path, text}` entries across 9 calls -- `scripts/verify_bib.py`, `references/fake_patterns.json` -- and every one was rejected here. That is the real reason 0 of 27 agent-authored skills shipped a resource file while 18 of 31 human-curated ones do: not a missing capability, and not the model failing to try. `source`/`source_url` stay excluded from the direct arm: those record provenance and are set by the URL path, so an agent must not be able to forge them. cargo test -p ironclaw_host_runtime --lib skill_install_input # 4 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * style: rustfmt the skill-bundle and activation changes Test modules were appended programmatically without rustfmt, which is why Formatting, Code Style and Clippy all went red on this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): rewrite to match what was measured, not the abandoned design The doc recommended a three-tool extension plus a resource-gate. Both are superseded: the tools turned out to be redundant (install_skill already accepted files -- three stacked gates were hiding it), and the gate MEASURED WORSE than always advertising a readable path (-25.7pp on self-creation, -40.6pp vs claude-code), because an agent-authored skill is usually SKILK.md-only so the gate suppresses the one route the selector had not already closed. Rewritten around the durable findings: the three gates and how each masked the next, the 0-of-27 vs 18-of-31 measurement, the SkillBundleDescriptor enumeration gap, and the trust question. The gate is kept in the doc as a recorded negative result, since its stratified justification is persuasive and will be proposed again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): correct why always_available is not the default The previous note claimed the floor score overrides setup-marker suppression. It does not: `prefilter_skills_with_options` returns None for a satisfied marker BEFORE scoring, and the host-side filter in activation.rs already removed the candidate. What actually fails: all 32 bundled skills reach floor 1, so 3-4 unrelated ones land in plan.activations() in ActivationCriteria mode -- a mode that injects nothing under Listing. The defect exposed is that a criteria activation which injects no body is still recorded as an activation, so the count assertions stop being meaningful. Also records the sequencing against epic #6565 (Slice 0 first; Slice 5's bounded-shortlist rule constrains what an unbounded floor may do) and the measured detail that under Listing a zero-scoring skill is still listed -- the model just called skill_activate in only 3 of 30 runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(skills): a floor-only skill is ranked, not activated Three defects, all surfaced by trying to make `always_available` the default. It failed 8 tests in ironclaw_reborn_composition; all 8 now pass with the flag on AND off. 1. A criteria selection that injects nothing was still recorded as an activation. Under `SkillInjectionMode::Listing` an `ActivationCriteria` entry contributes no body -- `body_eligible_bundle_ids` already ignores that mode -- so with a floor score every installed skill "activated" on every turn. Concretely: all 32 bundled skills reach floor 1, and 3 of them (6000 token budget / 2000 default per-skill cost) landed in each plan, chosen by descriptor order because the score sort is stable. `SelectionOutcome` now returns those separately as `ranked_only`, and the activation path does not iterate them. They still reach the model through the listing, which is where they belonged. 2. `AlwaysAvailable` also enabled the name/description fallback, which manufactured fake merit: a bundled skill whose description shares one word with the message scored above zero and was reported as a genuine activation. Under `AlwaysAvailable` the fallback adds no reach at all (the floor already admits everything), so it is now scoped to `NameAndDescription`, where widening the match is the entire point. This is what kept `local_dev_runtime_suppresses_explicit_setup_skill_when_workspace_marker_exists` failing after (1). 3. Raising TOOL_RESULT_RECORD_READ_MAX_BYTES to 64 KiB was NOT the no-op this PR claimed. `tool_result_reference.rs` derives MAX_MODEL_OBSERVATION_BYTES from it (* 2), so the observation envelope silently doubled 48 KiB -> 128 KiB and preview truncation changed for every caller. It broke three tests whose fixtures are sized against the envelope ("fixture must exceed the preview cap"), independently of any activation setting. The contract ceiling is back to 24 KiB and the env override is bounded by a new TOOL_RESULT_READ_ENV_CEILING_BYTES that nothing is derived from -- so the knob can raise a single read without moving anyone else's behavior. Correcting the record on an earlier comment in this PR: the failures were never the setup-marker interaction. Marker suppression returns None before scoring, so a floor score cannot revive a suppressed skill. cargo test -p ironclaw_reborn_composition --lib # 634 passed IRONCLAW_REBORN_SKILL_ACTIVATION=always_available cargo test -p ironclaw_reborn_composition --lib # 634 passed cargo test -p ironclaw_skills --lib # 241 cargo test -p ironclaw_threads --lib # 88 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * revert(skills): remove the always_available strategy, it bought nothing Verified against this branch: `AlwaysAvailable` was a no-op for everything the model can observe, and it carried a regression. Removing it rather than wiring a compensating half. Why it bought nothing. Listing membership is decided by VISIBILITY, not selection (extension_ports/activation.rs partitions candidates on body-eligibility, and everything not body-eligible still goes into the listing), so the model was ALREADY shown every visible skill before this strategy existed. The floor score never added reach -- pre-C1 its only effect was listing ORDER, and after C1 excluded floor-only skills from activations even the ordering effect was gone, because the ranking input is derived from the activation list. `SelectionOutcome::ranked_only` had no production reader at all: allocated, populated, returned, dropped. Under `Full` a floor-only skill could never be injected either, since `context_candidates_for_plan` renders only activated bundles. The regression. The floor-only bookkeeping ran for every non-merit entry BEFORE `try_select`, so under this strategy a chain-loaded companion got its own loop iteration, was recorded as floor-only, and was then partitioned OUT of `selected` -- i.e. `A requires B` activated only `A`, where `CriteriaOnly` activates both. Strictly worse than the default for any bundle with companions, and order-dependent. The comment claiming this could not happen was wrong. Also removed: ~29 "budget exhausted" notes per turn that reached `feedback` and fired a SkillActivation live-projection event with empty skill_names, because floor-only skills still ran the budget loop and `BudgetFull` continues rather than breaks. Kept: `NameAndDescription`, which has a real effect (matching on name/description, not only `activation.keywords`/`tags`/`patterns`), and the `skill.activation.v1` seam. Corrects the record in two places that argued the opposite: the runtime.rs doc comment and docs/skills/agent_authored_bundles.md. The measured reachability gap is elicitation, not filtering -- `builtin.skill_activate` was called in 3 of 30 runs and a body read in 0 of 30 -- so the next step is the listing header, not a scoring change. Note the parity numbers in nearai/benchmarks#327 never depended on this strategy: those arms ran with it off. cargo test -p ironclaw_reborn_composition --lib # 634 passed cargo test -p ironclaw_skills --lib # 240 passed cargo test -p ironclaw_threads --lib # 88 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(skills): explain refusals, gate requirements, survive discovery limits Epic #6565, the missing/unusable half. Four causes it names, each of which was silent. ## A refusal now says why `select_named_skill_activations` reported a skill that exists but is not `Trusted` with the same string as a name that does not exist: "requested skill is not available". The two need opposite responses -- one means "try a different name", the other means "this needs promoting and no name will work" -- and the model got neither. This is the routine outcome of the model doing what it was told: the listing filters on visibility only while activation requires `Trusted`, and tenant-shared and URL-installed skills are `Installed`. So the listing advertises a skill and activation then refuses it. Deliberately does NOT enumerate alternatives, tempting as that was: `load_named_activation_candidate_set` scopes the candidate set to the requested names, so nothing else is loaded at that point and any "available: ..." list would be empty. I wrote that branch, found it could never fire, and removed it rather than ship a message that lies. Offering alternatives needs a wider descriptor load and belongs with #4428. ## Requirements are actually checked `requires.bins`, `requires.env` and `requires.config` were parsed into the manifest and never consulted. `check_requirements` exists, but its only callers are inside `SkillRegistry`, which has no consumers outside its own crate. A skill declaring a binary it needs was offered, activated cleanly, and failed later in the shell with nothing connecting the failure to the unmet requirement. Gated at ACTIVATION time, on both the explicit-mention and model-selected paths. Not at listing time: that would be three probes per visible skill on every prompt build and needs a caching design first. At activation it runs for the handful of skills being loaded, so the cost objection does not apply. Staying unusable is correct here -- the fix is that the reason reaches the model. ## One oversized root no longer erases itself `list_root` returned `BundleScanLimitExceeded` when a root held more than `max_bundles_per_root` directories, which removed EVERY skill in that root from the model's view. A catalog that grew past the cap lost all its skills at once, with no signal to the model and only a propagated error to the operator. Now it keeps the bundles that fit and warns about the truncation. ## Silent skips became warnings that name the reason Two `debug!` sites -- an invalid bundle directory name, and a manifest that fails validation (which covers the common authoring mistake of a directory name disagreeing with the manifest `name:`) -- meant a skill present on disk simply never appeared and nothing said why. Both are `warn!` with the error attached. ## Tests Three that pinned the old behaviour were rewritten rather than deleted, each with why: the two refusal-message assertions, and the scan-limit test that asserted total root loss. New coverage for the trust-vs-name distinction and for an unmet binary requirement being refused with the requirement named. `cargo test -p ironclaw_first_party_extension_ports -p ironclaw_loop_host` — 68 + 420 + 27 + 4 + 88 pass. fmt and clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): surface the refusal reason to the model, not just the projection The reason strings were the easy half. `skill_activate`'s result was built from `plan.selection.activations` only and **discarded `plan.selection.feedback` entirely**, so every reason the selector produces was constructed and thrown away. The model saw `{"activated":[],"count":0}` and had to guess whether it had used a bad name, hit a trust wall, or tripped an unmet requirement -- three situations that need three different responses. Caught by measurement rather than review: on the missing/unusable fixtures `usable` moved after the earlier commit but `diagnosed` stayed flat at 2/7, because improving the wording of a message nobody receives changes nothing. Adds `not_activated` alongside `activated`. Routine "activated after model selection" confirmations are filtered out -- next to `activated` they are noise and would dilute the refusals that matter. Output construction is extracted into `build_activation_output` so the contract is unit-testable in the same style as the rest of the module: a refusal carries its reason, a clean activation gains no empty field, and a mixed result reports both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(skills): the model decides, and the listing can never be hidden from it Epic #6941 criteria 1 and 8. Adopts @serrrfirat's position -- "I don't think we should statically regex match skill names to skill activation... we should show it to the model and it should decide" -- as the default rather than a flag. ## The default is now ExplicitOnly `SkillActivationSelectorConfig::default()` no longer runs the keyword/regex scorer. A profile that wants it must ask for `ExplicitAndCriteria` deliberately; nothing inherits it silently, which is how #5417 shipped. The scorer's own record is the argument for retiring it: * It produced #5417 -- `tech-debt-tracker` declares the keyword `hack`, so "search Hacker News for..." activated it. * Over 328 real prompts `coding` fired on ~220 through *legitimate whole-word* hits on `file`/`change`/`code`. No boundary rule or score threshold can fix that. * Measured against it, the model path made **zero** wrong selections across 28 tasks over an 88-skill catalog, at **94.8%** precision on what it did activate. The scorer is not deleted: it is still correct, still tested, and still reachable for a profile that opts in. It is simply no longer the thing that decides. ## And a trap that had to be fixed in the same commit In `Full` injection mode both context paths returned an EMPTY candidate set when nothing was active. That was survivable only while the scorer auto-activated something. With model-decides it would mean the model is never told a skill exists and therefore can never activate one -- flipping this default alone would have blinded the agent. Both paths now fall through to the listing. ## Tests Three new criterion tests: the default policy is pinned (so a silent revert fails), the listing survives with nothing activated in BOTH injection modes, and the listing stays inside a stated character budget at 200 skills -- with the scorer retired the listing IS the routing interface, so its size is a correctness property rather than a cosmetic one. Sixteen existing tests were updated rather than deleted, in two groups, each with its reason in the code: * Tests whose SUBJECT is the scorer now call `criteria_config()` to opt in. That is the point of the new default -- nothing gets the scorer by accident. Includes `prepare_grants_asset_reads_only_for_activated_bundles`, which uses criteria selection only as the mechanism to activate a bundle; I checked that one first because an asset-grant assertion failing could have been a real security regression rather than an expectation change. It was not. * Tests asserting `selected.is_empty()` now assert `assert_no_skill_body_disclosed(..)`. "No candidates" is no longer the right question; "no skill BODY reached the model" is what they were really protecting, and it is exact -- the listing is a *discoverable* candidate (`loaded_skill_md() == None`) while an activated skill is a *loaded* one. `cargo test -p ironclaw_first_party_extension_ports -p ironclaw_loop_host -p ironclaw_skills` -- 74 + 420 + 27 + 4 + 88 + 229 pass. fmt and clippy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): pin #5417 on the path that records the user message Epic #6941 criterion 2. Criteria selection needs a RECORDED user message (`take_message_for_run`), and the coordinator path never records one -- so a coordinator-path test passes vacuously and proves nothing. This records the message, which is what the product/WebUI surface does, and the issue itself reports "Run origin: WebUI chat". Asserts BOTH policies, including the uncomfortable one: * model-decides (the new default): no body is injected. Fixed. * criteria explicitly enabled: it STILL mis-activates on this branch. Asserted as a known residual rather than omitted. That second arm is the useful half. It shows the two changes are complementary rather than redundant: this PR removes the scorer from the decision, #6937's word-boundary matcher stops `hack` matching inside "Hacker" for any profile that opts the scorer back in. Neither alone closes #5417 on the criteria path, and pinning it here means a future reader cannot mistake model-decides for a complete fix. The assertion message says what to do when #6937 merges and the arm flips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): a truncated skill listing must say so `MAX_LISTED_SKILLS` is 100 and the listing is source-then-name ordered, so past the cap whole alphabetical tails vanish -- with no signal to the model and none to the operator. Found by walking into it. Running Benchmark A against a 227-skill catalog, `pdf`, `pptx`, `xlsx` and `timeseries-detrending` all sorted past position 100, so three of the first four tasks could not reach their own expected skill and the arm was measuring nothing. Nothing anywhere reported it; I only caught it by diffing the rendered listing against each task's expected set. That is the failure mode this epic exists to remove, and it was hiding in the listing itself. Now the listing states how many skills are hidden, and the host logs a warning with listed/hidden/total. This does NOT make a large catalog usable -- that needs `skill_search` (#4428), and a 227-skill listing costs ~9k tokens of prompt besides. What it does is turn a silent, invisible failure into a stated one, so a benchmark or a user hitting the cap finds out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): make "the model decides" reachable, and list every skill instead of the first 100 Two defects found by measuring this PR rather than reviewing it. Both made a claim the PR already made untrue in practice. **1. The ExplicitOnly default was dead code on the Reborn path.** `skill_activation_selector_config` pinned `ExplicitAndCriteria` at the call site, so changing the default in `activation.rs` could not affect any real Reborn user. The model-decides change looked like a behaviour change and was not one. No test in `ironclaw_first_party_extension_ports` could catch this, because those construct their own config; only a test on the value the composition layer actually returns can. `reborn_skill_selection_is_model_decided` is that test, and it fails if the mode is re-pinned. **2. The skill listing silently dropped everything past 100 skills.** `MAX_LISTED_SKILLS = 100` with source-then-name ordering meant whole alphabetical tails were rendered nowhere and logged nowhere. Measured on a 227-skill catalog: `pdf`, `pptx`, `xlsx` and `timeseries-detrending` all sorted past the cap, and three of the first four benchmark tasks could not reach their own expected skill. A skill the model cannot see is one it cannot activate, so this is indistinguishable from never having installed it — and with the scorer retired, the listing IS the routing interface, which makes its completeness a correctness property. The flat count cap becomes a character budget spent differently: every skill's name is listed, with per-entry descriptions shrinking as the catalog grows (250 chars at small sizes, 90 at 227), and entries are dropped only when even 60 chars will not fit — roughly past 380 skills. **This is not a context-size increase.** The budget is exactly what the old cap already permitted (`100 * (250 + 64)`). What changes is that it buys reachability for all skills rather than verbosity for the alphabetically lucky first hundred. When truncation does happen it is stated in the listing and warned with `listed`/`hidden`/`total`. `the_listing_stays_within_budget_at_two_hundred_skills` now asserts both that the listing fits its budget *and* that all 200 skills appear in it. The second assertion is the one the old cap violated: that test previously passed on budget alone while hiding half the catalog, which is how this survived. Beyond ~380 skills the answer is `skill_search` (#4428), not a bigger prompt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): raise the per-root bundle scan cap so a real catalog enumerates whole Found by measuring: after replacing the listing's 100-skill cap with a character budget, a 227-skill root STILL only reached the model as 100 skills. The listing was never the binding limit. `DEFAULT_MAX_BUNDLES_PER_ROOT = 100` truncates one layer earlier, at enumeration: `skill bundle root exceeds the per-root scan limit ... limit=100 skipped=127`. The 127 skipped bundles were invisible to the selector, to the listing, and to the model — the same outcome as never installing them. Two caps, and only the lower one decides, so raising the listing budget alone accomplished nothing. Raised to 512. The cap exists to bound an unbounded directory walk, not to bound a catalog; 512 keeps that protection (a bundle is one directory read plus a manifest parse, cached per root) while leaving real catalogs whole. Past it, truncation is still partial-and-warned rather than fatal, and the answer is `skill_search` (#4428) rather than a larger number. `a_two_hundred_and_twenty_seven_skill_root_enumerates_whole` asserts the default cap does not truncate a real catalog. The existing test only proved truncation *degrades gracefully*, which is why a default too low to fit anything real passed it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): let the scorer rank the listing, while the model alone decides Retiring the criteria scorer from ACTIVATION is right and stays. Retiring it from ORDERING was a mistake, and the benchmark caught it. Measured on the 31-task routing benchmark, 88 candidates, same model, paired: | | criteria on | ExplicitOnly | |--------------------------|-------------|--------------| | >=1 correct skill | 57.1% | 28.6% | | recall over expected set | 42.9% | 25.0% | | correct skill REQUESTED | 75.0% | 35.7% | | never called skill_activate | 25.0% | 57.1% | The model was not being refused -- refusals were 0% in both arms. It stopped asking. The scorer was the only thing making a long listing legible: with it off, the listing collapses to source-then-name alphabetical order, and the relevant skill sits among dozens of equally-weighted lines with nothing marking it. At 227 candidates this is worse, not better. So the scorer keeps its useful job and loses its harmful one. Under `ExplicitOnly` the prefilter still runs, and its output populates `SkillActivationSelection::ranking_only`, which feeds `criteria_ranked_bundle_ids` and therefore listing order ONLY. It never appends to `activations`, and it deliberately does not extend `feedback` (those notes explain activation decisions, and nothing was activated). This is the distinction @serrrfirat's objection actually draws. "Don't statically regex match skill names to skill activation" is not "don't use scoring to rank what the model is shown". Ordering a menu is not choosing from it. The host recommends; the model decides; a wrong recommendation costs a listing line rather than the skill budget. `explicit_only_ranks_the_listing_without_activating_anything` pins both halves in one test, because they pull in opposite directions: the matched skill must LEAD the listing, and no body may be disclosed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Revert "fix(skills): let the scorer rank the listing, while the model alone decides" This reverts |
||
|
|
9edf3fe394 |
test(inspector): add browser, security, and operator coverage (#7280)
* feat(inspector): add operator inspection API * docs(inspector): assign product service ownership * test(inspector): ratchet diagnostic contracts * feat(inspector): add debug panel shell * test(inspector): cover debug panel shell e2e * fix(inspector): stop diagnostics when panel closes * feat(inspector): add prompt inspection * fix(inspector): follow current webui ownership * feat(inspector): add model call statistics * test(inspector): cover model statistics e2e * fix(inspector): avoid uncollected tool metrics * test(inspector): cover prompt diagnostics e2e * test(inspector): align statistics e2e scope * fix(inspector): redact prompt metadata * fix(inspector): preserve per-call model identity * fix(inspector): classify prompt instruction sources * test(inspector): assert reported token usage * feat(inspector): add activity timeline and turn navigation * test(inspector): cover activity timeline in browser * fix(inspector): read current run before publishing activity * feat(inspector): add bounded tool execution details * test(inspector): cover bounded tool details in browser * fix(inspector): validate retained tool result sizes * test(inspector): add security and operator coverage * test(inspector): cover browser workflows end to end * fix(inspector): address review feedback * fix(inspector): retry transient snapshot failures * fix(inspector): address prompt diagnostic review findings * fix(inspector): follow debug query navigation * fix(inspector): preserve stream terminal state * fix(inspector): capture full capability surface * fix(inspector): scope projection activity to its run * fix(inspector): harden activity diagnostics * fix(inspector): bound tool result diagnostic capture * fix(inspector): harden tool diagnostic pipeline * fix(inspector): address prompt diagnostic review feedback * fix(webui): harden inspector stream coverage * fix inspector model call stats review findings * fix inspector refresh and truncation regressions * fix(inspector): address activity timeline review feedback * fix(inspector): harden activity lifecycle handling * fix(composition): move tool diagnostics to loop host * fix(inspector): address review findings |
||
|
|
5f7b5d3a5f |
test(disclosure): measure the Core delivery pair in the wide-catalog benchmark (#7393)
The representative fixture omitted the two tools this PR moved to Core, so the reduction floor could not see their standing advertised cost. Model both (a bounded deliver call, a no-argument lister), and re-pin the recorded baseline 82.9% -> 82.0% per the constant's own instructions — cause: core-set width plus the fixture change. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
a275895cc0 |
fix(live-qa): verify triggered Slack delivery through the two-lane contract (#7389)
* fix(live-qa): verify triggered Slack delivery through the two-lane contract Since #7157 a triggered fire's result is never pushed by the completion driver: the fire itself calls builtin.outbound_deliver, and the background-run notifier's triggered-run-delivery record describes NOTICE delivery only — a cleanly completed fire records `skipped`. The delivery cases still required that record to say `delivered`, which no longer exists for results, so qa_3d/qa_8d/qa_9b/qa_9d hard-failed every scheduled run from the first post-#7157 canary (2026-08-08 00:24 UTC) even though all four live fires verifiably delivered (three had the marker sitting in Slack history; the fourth was provider-confirmed). The waiter now verifies what the product actually guarantees: - success = the fire's durable outbound/deliveries model-delivery record for the exact run (delivered, expected DM) PLUS the independent Slack history read-back finding the marker; - notifier records: `skipped`/`no_default_configured`/`delivered` are healthy terminals, only `failed`/`denied` fail the case, and unknown future vocabulary surfaces through timeout diagnostics; - a completed outbound_deliver whose composed content lacks the marker fails deterministically (the qa_8d mode: the stale prompt bound the marker to the final answer, which is no longer the delivered payload); - the readback-inconclusive flake classification accepts an exactly-one-verified-send through either lane. Case prompts now bind the marker to the delivered Slack message itself (and still to the final answer), via one shared prompt-requirement helper. Also fixes the QA 6D-6E strict-scrub false positive: progressive tool disclosure (#6958) records tool_search output in traces, and the builtin.extension_register_hosted_mcp description's "bearer for a static API token or PAT sent as a Bearer token" prose tripped the bearer pattern, deleting the trace and failing the shard with all cases green. The bearer pattern now requires 16+ token-alphabet characters. All delivery-wait decision logic is pinned by new unit tests against the production record shapes captured from the failing canary artifacts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(live-qa): close review findings on the two-lane delivery contract - Gate marker_deliver_count on completed previews: a failed or in-flight outbound_deliver whose content carries the marker never reached Slack, and counting it could fake the exactly-one-verified-send inconclusive classification or suppress the deterministic markerless red. Fixture gains a failed marker-bearing preview, observed red before the fix. - Align emit_results_json.py's bearer pattern with the scrub script's 16-char floor so description prose in results.json is not mangled to "Bearer <REDACTED>"; prose-preservation regression added. - Namespace the readback-inconclusive evidence per lane (vendor_evidence/deliver_evidence) — both dicts carry parse_error_count and the flat merge let one overwrite the other. - Reuse the production root_filesystem schema helpers in the new test fixtures instead of a hand-written CREATE TABLE. - Document why the deterministic content check keys on skipped/no_default_configured rather than the whole healthy_terminal class: `delivered` includes a fire parked on an approval gate whose run resumes — and may deliver — after the notice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(live-qa): pin the exact 16-char bearer floor on both redaction rules The prose-preservation tests prove prose survives but not the threshold itself — a {15,} regression would have passed both. Pin the 15/16 boundary explicitly in the emitter suite and the shell scrubber suite, since the two rule sets are documented as kept in sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
a8ea9f3a1b |
test(telegram): pin reply anchoring and overlap busy-notice feedback (#6643, #6644) (#7364)
* test(telegram): pin reply anchoring and overlap busy-notice feedback Issues #6643 and #6644 reported Telegram messages vanishing and replies misattributed to the wrong prompt. Investigation showed both are already fixed on main: replies carry reply_to_message_id of their prompting message, a working indicator posts while a run is live, and a DM arriving mid-run gets an immediate anchored busy notice. Nothing pinned that contract, so it could silently regress. Pin it end-to-end through the production webhook, admission, observer, and wire evidence. Related #6643, #6644 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(telegram): prove the busy notice lands mid-run and pin every anchor Review findings on #7364: the busy notice was only observed after the first run released, so a regression deferring it to run completion would still pass — poll for it while the paused gateway holds the run open. The working indicator's anchor was unasserted — require exactly one indicator and one final reply anchored to the first message, one busy notice anchored to the second, and no other anchors in the race chat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(telegram): cover the bot-targeted /pair repair re-send Review finding on #7363: the alias leg drove only plain /pair CODE while the PR claims the bot-targeted form works via the generic @bot strip. Pin the composition directly: a bound sender re-sending a fresh code as /pair@bot CODE is serviced as the idempotent repair path and receives the already-paired notice, with the pairing left connected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: firat.sertgoz <firat.sertgoz@near.ai> |
||
|
|
78b1b6b126 |
feat(webui): add audited admin thread scraping (#7228)
* feat(webui): add audited admin thread scraping * docs(webui): assign thread scraping handlers * fix(admin): harden thread scraping review paths * test(admin): cover thread scrape target reset * test(admin): require scrape request before reset * fix(ci): assign thread scraping views to artifact owner * fix(review): address thread scraping follow-ups * fix(webui): ignore cancelled thread list updates * fix(webui): redact thread scraping errors * fix(webui): translate scraping errors at render * fix(webui): keep scraping copy out of chat entry * fix(webui): preserve lazy admin locale parity * docs(reborn): map thread scraping owners * fix(webui): invalidate stale trajectory downloads * fix(webui): address BenKurrek review — thread scrape gates, audit safety, tests (#7228) * fix(webui): close remaining review items — gate coverage, cursor precedence, run aborts, windowed transcript, react-query data layer (#7228) * style(webui): rustfmt the merged route pattern list |
||
|
|
a443a5f421 |
fix(ci): recapture extension_host coverage floor + run goldens on prompt-surface PRs (#7371)
* fix(ci): schedule the golden lane for prompt-surface production changes
A production change to the model-visible prompt surface (the capability
surface digest, the instruction bundle, the communication-context
renderer, or a shipped loop-tier prompt asset) ran only crate buckets on
the PR lane, so stale golden_payload snapshots surfaced first as a
merge-queue bounce (#7361, 2026-08-07: surface.rs changed the surface
digest; the PR lane never ran the golden bucket). Add a curated
prompt-surface owner table that ADDITIONALLY schedules the golden
integration lane without consuming the path's normal package
classification. Self-tested per entry plus a negative control pinning
that ordinary production changes keep the narrow plan.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(ci): recapture the extension_host coverage floor with measured wobble tolerance
The 2026-08-04 entry was an adjustment whose own text ordered the next
real recapture. Since then code left the crate (denominator 24529 ->
24415) while the ratio ROSE to 88.12%, and the same commit
|
||
|
|
7be9b840c0 |
chore(agents): refresh codebase knowledge graph (#7388)
Co-authored-by: github-merge-queue <118344674+github-merge-queue@users.noreply.github.com> |
||
|
|
9f71cbbb9e |
fix(loop): make routine delivery steering deterministic under disclosure (#7390)
A web-created routine asking for GitHub-issue summaries "in a Slack message" was created with a stored prompt instructing the fire to use the vendor send-message tool instead of a pinned builtin__outbound_deliver step; an identical retry produced the correct pinned step. Two compounding causes, both observed live: 1. builtin.outbound_deliver and builtin.outbound_delivery_targets_list were Discoverable-tier, so on a catalog past the defer threshold the bridged disclosure surface (default since #6958) drops them from visible_capabilities — and the delivery guidance block renders only while both are visible (delivery_tools_visible). trigger_create is Core, so the model could create routines while blind on the delivery lane and without the "'Send it to me' is bot delivery via builtin__outbound_deliver" steering. Whether the steering existed depended on whether an earlier tool_search happened to disclose the pair. Both tools are now Core, restoring #7157's guidance-iff-tools coupling as a deterministic fact. The wide-catalog reduction benchmark is unchanged (82.9%): its synthetic fixture carries no outbound tools. 2. The trigger_create description and its prompt-field schema said "never call builtin__outbound_deliver in a web-app-created routine". The clause is correct for the no-named-destination default, but creation turns over-apply it — the qa_8d canary creation verbatim reasoned "I'm in the web app, so there's no outbound delivery target to pin — let me use the Slack extension's tools for the send step" before recovering. Both texts now scope the no-delivery default to "no external destination named" and state the named-destination rule explicitly: reaching the user or anyone else on an external surface goes through builtin__outbound_deliver with a pinned target id, never through integration messaging tools (concrete extension names kept out per the specificity gate). Regression tests: the core-name census pins both tools with their capability ids; the description tests pin the scoped clause, the absence of the categorical never-clause, and the named-destination steering on both the tool description and the prompt-field schema. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
92a2549619 |
feat(inspector): add bounded tool execution details (#7279)
* feat(inspector): add operator inspection API * docs(inspector): assign product service ownership * test(inspector): ratchet diagnostic contracts * feat(inspector): add debug panel shell * test(inspector): cover debug panel shell e2e * fix(inspector): stop diagnostics when panel closes * feat(inspector): add prompt inspection * fix(inspector): follow current webui ownership * feat(inspector): add model call statistics * test(inspector): cover model statistics e2e * fix(inspector): avoid uncollected tool metrics * test(inspector): cover prompt diagnostics e2e * test(inspector): align statistics e2e scope * fix(inspector): redact prompt metadata * fix(inspector): preserve per-call model identity * fix(inspector): classify prompt instruction sources * test(inspector): assert reported token usage * feat(inspector): add activity timeline and turn navigation * test(inspector): cover activity timeline in browser * fix(inspector): read current run before publishing activity * feat(inspector): add bounded tool execution details * test(inspector): cover bounded tool details in browser * fix(inspector): validate retained tool result sizes * fix(inspector): address review feedback * fix(inspector): retry transient snapshot failures * fix(inspector): address prompt diagnostic review findings * fix(inspector): follow debug query navigation * fix(inspector): preserve stream terminal state * fix(inspector): capture full capability surface * fix(inspector): scope projection activity to its run * fix(inspector): harden activity diagnostics * fix(inspector): bound tool result diagnostic capture * fix(inspector): harden tool diagnostic pipeline * fix(inspector): address prompt diagnostic review feedback * fix inspector model call stats review findings * fix inspector refresh and truncation regressions * fix(inspector): address activity timeline review feedback * fix(inspector): harden activity lifecycle handling * fix(composition): move tool diagnostics to loop host * fix(inspector): cancel stale tool detail requests * fix(inspector): address bounded tool detail review * fix(inspector): validate tool detail responses |
||
|
|
6dccbfbcbd |
docs: fix live drift in extension, responses API, and channel docs (doc-truth PR 1/5) (#7375)
* docs: fix live drift in extension, responses API, and channel docs The public tutorial taught the retired manifest v2 authoring format ([[host_api]] / [capability_provider.tools] / runtime_credentials), which the v3 parser hard-rejects, and never mentioned origin_gate_matrix; the Responses API page claimed temperature is rejected (accepted 0.0-2.0 and forwarded), claimed model must be "default" (any well-formed name <= 256 bytes), claimed max_output_tokens is rejected (accepted and ignored by DTO policy), and omitted the required model field from every request example; the channel tutorial pointed at two files that no longer exist. - docs/extensions/building-a-tool.md: rewrite manifest sections to the v3 [[tools]] / [[tools.credentials]] / [auth.<vendor>] shape, document origin_gate_matrix (origins, policies, ratchet), correct the hosted-MCP [mcp] section, packaging via ironclaw_extension_support package modules, and v3 test references; drop the nonexistent script runtime kind. - docs/api/responses.mdx: correct model/temperature/tools/tool_choice rejection rules, document unknown-field tolerance, add the required model field to all 15 request examples. - docs/channels/building-a-channel.mdx: replace dead crates/ironclaw_first_party_extensions + available_extensions.rs registration instructions with the current package-directory mechanism. - docs/reborn/contracts/extensions.md: state that production manifests author v3 (lowering into the v2 resolved model described there); label the v2 examples as legacy. - docs/reborn/how-to-port-tool-to-reborn.md: superseded banner pointing at the v3 guides. Part of #7317 (doc-truth pipeline, PR 1 of 5). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: address Copilot and CodeRabbit review on doc-drift PR - responses.mdx: tool_choice is rejected only without external-tools wiring; with external tools enabled it passes validation and is currently ignored (validate_responses_supported_fields_with_external_tools never checks it). - building-a-tool.md: clarify that effect-derived host ports are validation vocabulary against the HostPortCatalog allowlist; adapters are built by host-runtime services after authorization/obligations, never from manifests. - how-to-port-tool-to-reborn.md: mark the decision tree's RuntimeKind targets historical (v3 accepts only wasm|first_party; MCP is top-level [mcp]; process/CLI work is the sandbox lane). - building-a-channel.mdx: document the user install flow — virtual package root /system/extensions/<id>/manifest.toml, ironclaw extension search / install <extension-id> (ID, not path), WebUI Extensions lifecycle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(responses): align the limits bullet with the corrected tool_choice claim The rejection list was corrected in the previous commit (tool_choice is rejected only without external-tools wiring); the "Limits and quirks" bullet still said "not supported ... rejected with 400". Same claim, one wording. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: apply verified code-review findings on the drift PR A full code review of this PR against live code surfaced claims the original drift pass got wrong or missed; every fix below was re-verified against the cited source before editing: - responses.mdx: standard `ironclaw serve` deployments always wire external tools (OpenAiCompatRouteMountPorts requires the store/resume pair; mount.rs wires them unconditionally), so `tools` is accepted and `tool_choice` is accepted-and-ignored on shipped binaries — the conditional 400s apply only to custom compositions without the wiring (now a Note). temperature is validated and carried in the submitted turn payload but not applied as a provider sampling parameter. Non-streaming wait timeout is 30 s (DEFAULT_RESPONSES_WAIT_TIMEOUT), not 120. usage on retrieval is read best-effort from persisted run state incl. USD cost (read_run_usage), not always zero. - building-a-tool.md: the [auth.example] oauth2_code recipe gains the required token_response map (deny_unknown_fields rejects the example as previously written); Gmail/Google Calendar corrected to first_party runtimes (their manifests declare kind = "first_party"); the worked api_key recipe is github's, not slack's; the tail "Quick implementation checklist" and reference list were still v2-era (script lane, assets/<extension>/ path, "manifest v2", v2.rs pointer) and now teach the v3 shape; composition/CLI package-naming claim narrowed (the binary does link slack/telegram adapter crates). - contracts/extensions.md: legacy-format paragraph no longer claims host-bundled packages ship v2 (none do), and origin_gate_matrix is attributed to capability.rs + building-a-tool.md instead of extension-runtime/overview.md §3, which does not mention it. - how-to-port banner: `script` manifest authoring is retired; the RuntimeKind::Script symbol survives as the process-sandbox lane's kind. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
cae1a04f95 |
feat(sandbox): add explicit Docker and Railway user sandbox profiles (#7214)
* feat(sandbox): add Docker and Railway user sandbox profiles * fix(sandbox): close local review gaps * test(sandbox): cover production local profile wiring * fix(ci): schedule sandbox paths and classify tests * fix(sandbox): bound Railway user lifecycle state * test(cli): cover sandbox profiles in profile list * test(architecture): shrink composition support debt * fix(sandbox): harden Railway lifecycle review gaps * fix(sandbox): fail closed on Railway provider errors * fix(sandbox): close remaining review gaps * test(sandbox): remove Railway timing flake * chore(composition): record sandbox assembly budget * fix(sandbox): retain Railway parse causes * fix(sandbox): address post-merge review findings * fix(sandbox): close railway lifecycle review gaps * fix(sandbox): retain failed railway cleanup state * feat(sandbox): enable direct egress for sandbox profiles * test(sandbox): keep public egress in live canary * chore(sandbox): document bounded slice safety * test(architecture): ratchet user sandbox contract growth * test(sandbox): strengthen profile regression coverage * test(sandbox): exercise harness validation at boundary * test(sandbox): place setup check with harness contracts * fix(sandbox): preserve nonzero Docker exit results * fix(host-runtime): resolve hosted tenant workspace mounts (#7214) * fix(sandbox): address review feedback on bounded startup (#7214) * fix(ci): derive sandbox Docker paths from crate inventory (#7214) * fix(sandbox): finalize Railway lifecycle and profile parity * test(sandbox): update hosted process failure contract * fix(sandbox): address CodeRabbit review feedback (#7214) |
||
|
|
254483d219 |
test(disclosure): pin wide-catalog schema-token reduction floor and make drift visible (#7372)
* test(disclosure): pin wide-catalog schema-token reduction and make drift visible The representative 91-tool disclosure benchmark asserted only the 50% floor from #6810 and printed its table, so a slow slide in advertised schema cost (a widening Core tier, a growing bridge schema) could eat most of the headroom without any test noticing. Extend the existing benchmark test rather than adding a second one: - keep the 50% floor as an explicit, message-carrying assertion; - pin the currently measured 82.9% reduction with a +/-2 point drift band, so material movement fails loudly and forces an explanation; - record the drift history (93.3% bridge-only -> 83.7% -> 82.9%) next to the baseline constant; - attribute advertised tokens to bridge vs Core tier in the emitted line, so a failure names its cause. Test-only; no production behavior changes. Refs #7166 (section 5, wide-catalog benchmark acceptance item). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(disclosure): attribute benchmark tokens by ToolTier, not name exclusion The advertised-cost breakdown treated every non-bridge definition as Core, so a promoted Discoverable tool reaching the advertised surface would have been charged to Core and misdiagnosed the drift it caused. Classify each advertised definition by the catalog entry's actual `ToolTier`, report promoted Discoverable tokens as their own bucket, and assert that bucket is empty — the benchmark promotes nothing, so a Discoverable tool appearing there means the selection path changed and should fail loudly rather than hide inside the Core figure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
90f5532fcb |
feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted (#7157)
* feat: explicit channel delivery tool — two lanes, notification channels, delivery heuristics deleted Re-landed PR #7157 on current main ( |
||
|
|
d27dba30f7 |
fix(extensions): chat "connect account" dead-end — already-connected signal, builtin description trust, docs (#7361)
* fix(extensions): host-bundled description trust + already-connected install confirmation
Two chat-side dead-ends from the 2026-08-07 Slack QA session (thread
e79a994f, run 251aec0b on ironclaw-qa-testing-libsql):
1. Host-bundled capability descriptions were description_trust=Untrusted,
so the loop-tier prompt-text denylist strict-scanned compiled-in text
and silently omitted builtin.extension_register_hosted_mcp from every
model prompt's capability surface ("browser authorization-code flow"
matched the "authorization" credential pattern). HostBundled is the
only source eligible for effective FirstParty/System trust, so its
repo-authored descriptions now cross the verified-catalog boundary
like signature/digest-verified registry installs. Untrusted provenance
(InstalledLocal, UserRegistered, unknown) keeps the strict scan.
2. When install-driven activation passed the credential gate because the
caller's declared requirements were all satisfied, the response never
said so — the model got only conditional guidance ("If WebChat shows
an account connection panel...") and deflected an explicit "connect
account" request to the web interface even though the account was
already connected. The install response now appends an explicit
already-connected confirmation exactly when declared requirements
were verified present for the calling user.
Regression tests: manager surface test pins VerifiedCatalog trust for all
model-visible lifecycle capabilities through the real host runtime;
instruction-bundle tests pin retain/omit behavior for auth-vocabulary
descriptions by trust; install-path tests pin the confirmation on the
seeded-credential path and its absence for credential-free extensions.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(channels): chat can drive the personal half of channel connect
The onboarding and channels pages claimed "asking the agent to connect a
channel doesn't work" and that the agent "may tell you it can't help".
That describes only the operator half (registering app/bot credentials).
The per-user half has shipped since early July: extension_install runs the
same activation credential gate as the Channels card, raises the in-chat
OAuth connection panel when the account is unconnected, and (as of the
sibling fix) confirms when it is already connected.
The self-knowledge protocol makes these pages the model's authority on
IronClaw's own capabilities, so the stale claim scripted the exact
refusal QA hit ("I can't initiate the Slack OAuth flow from here") on an
account that was already connected. Correct both pages to distinguish
the operator step from the chat-drivable personal connect.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* docs(channels): align slack and telegram setup notes with the connect contract
The slack page's operator-step note and the telegram troubleshooting
accordion still carried the blanket "asking the agent to connect will not
work" claim the overview/onboarding correction removed — same drift,
different phrasing (review catch on #7361, plus one more instance found
by a broader sweep). Both now state the two-step contract: the operator
half stays in the web interface; after it, chat drives the personal half
(install/activate -> in-chat connection or pairing panel, or an
already-connected confirmation).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(golden): recapture surface digests over the description-trust change
The queue run failed golden_payload because the branch predated current
main and its own surface.rs trust fix changes the surface digest. The
recaptured snapshots differ ONLY in the surface sha256 lines (verified
char-by-char) — no prompt text or capability-list changes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
|
df90072c4e |
fix(telegram): accept /pair as a pairing-code alias (#7363)
Users habitually type /pair from the earlier pairing flow. Keep every suggested wording on /start (the vendor deep-link convention) and accept /pair <CODE> as a declared inbound-code-prefix alias so those users pair instead of looping through the connect nudge. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
e6a650f1fe |
feat(reborn): enable progressive tool disclosure by default (#6958)
* feat(reborn): enable progressive tool disclosure by default * test(reborn): pin scripted QA tool disclosure * test(reborn): pin composition tool surfaces * test(reborn): pin hook runtime tool surface * test(reborn): make flat tool fixtures explicit * test(e2e): pin flat disclosure fixtures * test(e2e): pin responses fixtures to flat tools * chore(reborn): clarify tool disclosure wording |
||
|
|
d8fc8376b6 |
feat(inspector): add activity timeline and turn navigation (#7278)
* feat(inspector): add operator inspection API * docs(inspector): assign product service ownership * test(inspector): ratchet diagnostic contracts * feat(inspector): add debug panel shell * test(inspector): cover debug panel shell e2e * fix(inspector): stop diagnostics when panel closes * feat(inspector): add prompt inspection * fix(inspector): follow current webui ownership * feat(inspector): add model call statistics * test(inspector): cover model statistics e2e * fix(inspector): avoid uncollected tool metrics * test(inspector): cover prompt diagnostics e2e * test(inspector): align statistics e2e scope * fix(inspector): redact prompt metadata * fix(inspector): preserve per-call model identity * fix(inspector): classify prompt instruction sources * test(inspector): assert reported token usage * feat(inspector): add activity timeline and turn navigation * test(inspector): cover activity timeline in browser * fix(inspector): read current run before publishing activity * fix(inspector): address review feedback * fix(inspector): retry transient snapshot failures * fix(inspector): address prompt diagnostic review findings * fix(inspector): follow debug query navigation * fix(inspector): preserve stream terminal state * fix(inspector): capture full capability surface * fix(inspector): scope projection activity to its run * fix(inspector): harden activity diagnostics * fix(inspector): address prompt diagnostic review feedback * fix inspector model call stats review findings * fix inspector refresh and truncation regressions * fix(inspector): address activity timeline review feedback * fix(inspector): harden activity lifecycle handling |
||
|
|
81724a6859 |
feat(inspector): add model call statistics (#7277)
* feat(inspector): add operator inspection API * docs(inspector): assign product service ownership * test(inspector): ratchet diagnostic contracts * feat(inspector): add debug panel shell * test(inspector): cover debug panel shell e2e * fix(inspector): stop diagnostics when panel closes * feat(inspector): add prompt inspection * fix(inspector): follow current webui ownership * feat(inspector): add model call statistics * test(inspector): cover model statistics e2e * fix(inspector): avoid uncollected tool metrics * test(inspector): cover prompt diagnostics e2e * test(inspector): align statistics e2e scope * fix(inspector): redact prompt metadata * fix(inspector): preserve per-call model identity * fix(inspector): classify prompt instruction sources * test(inspector): assert reported token usage * fix(inspector): address review feedback * fix(inspector): retry transient snapshot failures * fix(inspector): address prompt diagnostic review findings * fix(inspector): follow debug query navigation * fix(inspector): preserve stream terminal state * fix(inspector): capture full capability surface * fix(inspector): address prompt diagnostic review feedback * fix inspector model call stats review findings * fix inspector refresh and truncation regressions |
||
|
|
5888190ca6 |
fix(json): add bounded collection analysis (#7339)
* feat(json): add bounded collection analysis * test(reborn): refresh JSON capability snapshots * fix(json): address review — exact integer aggregates, bounded errors, dedup helpers (#7299) * test(reborn): refresh reviewed JSON snapshots * test(reborn): restore scoped JSON root query * ci: retrigger Railway preview deploy --------- Co-authored-by: firat <> |
||
|
|
ce2d6f84a9 |
fix(host-runtime): wire WASM secret-exists to staged credentials (#7307) (#7329)
Third-party WASM guests (ironhub tools such as attio) gate on the
secret-exists host import before issuing any request, but production
wired the sandbox with the deny-all default, so the probe always
returned false: attio aborted pre-network with "API key not
configured" and the host classified the plain-string guest error as
operation_failed, never auth_required.
Introduce StagedWasmHostSecrets, a per-invocation WasmHostSecrets
implementation over the staged secret injection store: exists(name) is
true exactly when authorization leased and staged non-empty credential
material for (scope, capability_id, handle), read non-destructively so
the HTTP egress still receives the material. Wire it into
WasmRuntimeAdapter::host_for_scope on every host variant and plumb the
shared store through the builder.
Credential staging now rejects empty resolved material as
AuthRequired (obligation handler and host-driven staging), so a
configured-but-blank key surfaces the typed re-auth signal instead of
an opaque guest failure. No prose heuristics: the structured
{"kind":"auth_required"} guest contract remains the fallback.
Adds unit tests for the probe semantics and WASM contract tests with a
secret-exists probe component (staged -> true, absent -> false, empty
material -> AuthRequired staging error).
|
||
|
|
8b32989d0a |
Guidance unification: one canonical home per fact, a measured loader story, and a gate that keeps it true (#7306)
* ci(guidance): add check-guidance.py — guidance must reference the tree that exists Four mechanical drift classes become build failures: every repo path named by agent guidance (root AGENTS.md/CLAUDE.md, crates/** AGENTS/CLAUDE/ CONTRACT/README, .claude/rules/*.md, .claude/skills/*/SKILL.md) must resolve in the tracked tree; every rules/skills frontmatter paths: glob must match at least one tracked file (the dead-trigger class that let skills.md never fire); every crate directory appears in its family's AGENTS.md crate table (the guidance half of check-target-tree.py); and every crate has a README.md (measured 62/62, so it gates). Extraction is designed against false positives: fenced blocks, placeholder tokens, MCP method names, dated-correction (✎) lines, and 'check-guidance: path-ok' lines are not claims; resolution honors the citation forms measured on the live tree (root-relative, doc-relative, name-prefix, crate-qualified-by-context, module-relative within the citing crate). KNOWN_MISSING is a shrink-only suppression table — a row whose reference stops dangling fails the gate until deleted, and surviving rows print as warnings every run. Fails closed on unreadable files, unparseable frontmatter, broken crate discovery, and near-empty scans (floor constants). Self-test in test-check-guidance.py (23 cases, refusals first, real repository last), wired beside check-target-tree.py in code_style.yml; the test planner classifies all three paths as static-control (verified exit 0). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(guidance): consolidate crate-tier CLAUDE.md files; rename module specs to CONTRACT.md Steps 2+3 of the guidance unification (docs/reborn/guidance-conventions.md): - Rename the four Module Specs table specs CLAUDE.md -> CONTRACT.md (llm, filesystem, webui, composition), matching the identity/trust precedent. Charter gates repointed (llm module_charter, webui handlers_module_charter) and every live reference updated; pointer stubs left behind so tooling that loads CLAUDE.md still lands on the spec. - Fold the nine substantive out-of-table CLAUDE.md files: wasm, mcp, sandbox, auth, assistant, trace_commons, extension_manager become AGENTS.md-canonical (gates repointed with pinned phrases kept verbatim: the wasm_sandbox_core arch pin, mcp module_charter, auth module_charter, assistant reborn_services_module_charter); network and secrets fold into their README Invariants sections and drop the crate guidance pair entirely. - Mark with the convention's absence-claim annotation the five crate-tier lines grandfathered by check-guidance KNOWN_MISSING (llm CONTRACT.md x3, composition CONTRACT.md, hooks AGENTS.md) and mark trace_commons' prescribed tests/queue.rs mirror as prescriptive-future. - tests/CLAUDE.md: replace the retired root Current-Limitations citation with the measured ironclaw_observability description. End state: zero prose CLAUDE.md outside the Module Specs table at the crate tier (the four ironclaw_agent_loop src/tests directory guides stay, same footing as the tests-tree harness guides). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(guidance): unify the root pair — AGENTS.md canonical, CLAUDE.md adapter Step 1: root AGENTS.md (198 lines) and root CLAUDE.md (286 lines) shared zero identical lines — the forked-pair drift the guidance convention forbids at crate level, live at the root. Root AGENTS.md is now the canonical tool-neutral contract (build/run/debug commands, hard invariants including the unified extension model and the credential_name/extension_name identity rules, the Module Specs table — now uniformly CONTRACT.md and gaining the existing ironclaw_trust/CONTRACT.md row — testing discipline, tree map, discovery, change discipline; 152 lines). Root CLAUDE.md is an @AGENTS.md adapter plus the genuinely Claude-specific tail: skills/rules index, codebase-graph MCP recipes, and the REPL info!/warn! logging rule (51 lines). Cut while merging, each measured against the tree: the v1 Job State Machine (no such state machine exists under crates/), Current Limitations (stale — the observability claim no longer matches the crate), the Skills System section (.claude/rules/skills.md and the domain crate own it), Extracted Crates, the re-derivable key-traits list, and the long channel-onboarding narrative (now three lines pointing at crates/extensions/AGENTS.md and the worked slack example). Every live citation of the root pair's moved sections is repointed (crates/ routing map + README, the deslop-reborn command, types/type-placement rules, skills/common/config crate docs, a loop_host doc comment). The git-ignored .codebase-memory/artifact.json mention carries the absence-claim annotation for the check-guidance KNOWN_MISSING handoff. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(guidance): make CLAUDE.md a symlink to AGENTS.md at every tier The loader question is now measured, not assumed. Headless canary experiment with a discriminating control: a symlinked nested CLAUDE.md's target content IS injected when a file in that directory is read, an @AGENTS.md import inside a nested CLAUDE.md also expands, and a nested AGENTS.md alone is NOT read. So one uniform rule holds everywhere: wherever an AGENTS.md exists, CLAUDE.md sits beside it as a symlink — same bytes, zero maintenance, no second document to drift. 64 pointer stubs become symlinks. The four spec crates keep CONTRACT.md as canonical; their AGENTS.md routes there, so the spec stays one hop away while the working rules now auto-inject instead of costing a voluntary read. Also reconciled check-guidance.py's shrink-only KNOWN_MISSING table: all 8 rows deleted because the content pass fixed the underlying lines, and the three absence-claims the gate then surfaced carry markers. The table is empty. Caveat recorded for the convention: nested injection fires only below cwd, and appears not to fire in subagent sessions — family docs must stand alone when read deliberately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(guidance): amend the convention with measured loader mechanics and budgets The first version made crate AGENTS.md canonical and CLAUDE.md a pointer, which moved working rules out of Claude Code's auto-inject path. Records what was measured instead: subtree CLAUDE.md injects lazily, symlinks and @imports both carry content, nested AGENTS.md is not read natively, and injection does not fire in subagent sessions — so every doc must stand alone when read deliberately. Adds size budgets per tier, extends scope to .claude/rules and .claude/skills (where the worst drift was), names check-guidance.py as the enforcement with its suppression markers, warns that some guidance is test-parsed (including the heading-shadowing trap), and adds the remove/rename checklist that mirrors add. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(guidance): composition keeps a real CLAUDE.md, not a symlink The blanket symlink pass broke composition_root_embeds_no_prompt_content, and the gate is right to refuse: its ownership walks do not follow symlinks, so stepping over one would let it report clean on a subtree it never read. This crate keeps a regular pointer file, with the reason written in the file so the next person does not 'fix' the inconsistency back into a break. The uniform alias rule now has two stated exceptions: the root (real file, it carries a Claude-only tail) and composition (real file, this gate). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci(guidance): enforce the CLAUDE.md alias rule; scope path-ok to the marked reference The branch's central invariant — a `CLAUDE.md -> AGENTS.md` symlink beside every AGENTS.md at the root and under crates/ — was unguarded: the audit proved a committed symlink deletion left the gate green (a working-tree deletion only tripped the accidental "cannot read guidance file" refusal). Check 5 now judges the git index (`git ls-files -s` + `cat-file`): the alias must be tracked, mode 120000, targeting exactly `AGENTS.md`. The two real-file exceptions are named rows with reasons (the root adapter's Claude-only tail; composition's symlink-refusing ownership walks), and a row that stops matching the tree fails the gate rather than lingering. Sabotage-verified on the real tree: `git rm --cached` on an alias went red naming the pair; converting one to a tracked regular file went red; restore went green (65 aliases verified). Also from the audit: - A `path-ok` marker now vouches for the one reference immediately preceding it instead of exempting its whole line — the audit slipped a fresh dangling path onto a marked line and passed. The `✎` glyph stays line-scoped by documented design. Both in-tree marker usages already sit marker-after-reference and keep working. - Document the structural blind spot: a dead reference whose first segment died with its whole tree (the v1 `src/…` monolith) reads as historical narration and cannot be flagged; only review catches it. - Re-measure the fail-closed floor comment — the shipped one claimed 174 guidance files / ~800 references / 30 globs against a tree that measures 237 / ~2070 / 38 — and add a floor for alias-site discovery. Self-test grows six cases: index-deleted alias, regular-file alias, wrong-target alias, the load-bearing root exception row, exception rows matching reality, and the marker-narrowing exploit. The `--tracked-files` override marks symlinks as `<path> -> <target>`. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(guidance): repoint dead skill refs, record alias carve-outs, honest size budgets Content half of the guidance-unification audit fixes: - architecture-video SKILL.md told readers to read `src/tools/README.md` and `src/workspace/README.md` — the v1 monolith is gone (`git ls-files | grep -c '^src/'` is 0) and the gate structurally cannot flag first-segment-dead paths. Repointed at the Reborn successors: `crates/extensions/AGENTS.md` and `crates/domains/ironclaw_memory/README.md`. - guidance-conventions.md now records what only commit messages knew: the composition real-file exception beside the root one; the four sanctioned ironclaw_agent_loop sub-module CLAUDE.md guides; and the alias rule's actual scope (root + crates/**), naming the two out-of-scope AGENTS.md (docs/reborn/contracts, ironclaw_silk_decoder) instead of a "wherever" wording the tree contradicted. - Size budgets re-derived from measurement (family <=220, crate <=160) with the four crate-tier exceptions named and reasoned. The shipped <=150/<=80 numbers were exceeded by 3 family and 24 of 54 crate docs on day one, which made the budget unreadable as a signal. No document was padded or truncated to fit. - Root CLAUDE.md used the dated-correction glyph on the deliberately untracked `.codebase-memory/artifact.json` reference — suppression duty outside the glyph's documented historical-prose meaning. Swapped for `<!-- check-guidance: path-ok -->` beside the reference. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * review(7306): CodeRabbit triage — guidance gate runs for the files it governs, brace globs, honest floors, identity-column family tables, doc-truth fixes Trigger (Major, the inert-guard finding): fast-checks was gated on has_code, whose regex covers none of .claude/, the root AGENTS.md/CLAUDE.md pair, or docs/ — so a PR editing only a rule's paths: trigger skipped the gate built for exactly that change. New has_guidance output OR-s those surfaces into fast-checks only (clippy/JS lanes stay code-scoped); has_code keeps its pinned meaning. Pinned by a ws12_workflow_contracts.py row and verified by replaying representative change lists through the workflow's own extracted EREs. check-guidance.py: glob_to_regex now translates {a,b} brace alternation (nested; unmatched braces stay literal) so a legitimate crates/**/*.{rs,toml} trigger counts as live instead of being reported dead; MIN_RULE_GLOBS 1->20 and MIN_ALIAS_PAIRS 10->40 (~half of measured 38/65, so a degraded parser refuses instead of passing); family-table coverage now requires the crate in a row's identity (first) column — an incidental mention in another row's prose no longer counts (measured 0 regressions on the live tree). Self-tests: +3 (brace trigger end-to-end, duplicate KNOWN_MISSING rows, identity-column regression) and the real-repository case documents its deliberate git coupling. Floors sabotage-verified. Doc truth, measured against code: composition CONTRACT — WS stream shares SseCapacity (stream_events_ws try_acquire, pinned test) replacing 'No WS surface to bound', webui_v2_app returns Result<Router, WebuiServeError>; llm CONTRACT — the circuit breaker wraps failover (apply_decorator_chain order), not the reverse; filesystem CONTRACT — dependency rule now names the real manifest set (+libsql_runtime, +observability); extension_manager AGENTS — the loops layer flip landed (layer = "loops"); four stale 'has no CLAUDE.md' claims updated for the new symlink aliases (config, common, event_store x2); root AGENTS — clippy line gains -- -D warnings (CI denies warnings; unflagged clippy exits 0 with them) and the error bullet routes to .claude/rules/error-handling.md; assistant/webui validation sections document the real lane structure (self-dev-dep unifies test-support on, so the missing shape is the no-dev-deps production lane, the #7119 class). Stale pre-family paths in .rs prose: 594 crates/ironclaw_* citations measured; 130 sit in comments, of which 106 repointed to their family homes (every rewritten path verified to resolve), 10 of those needed deeper repoints (files that moved crates: capability_host.rs, channel_pairing.rs, approval_store_contract.rs, secret_store.rs, loop_contracts instruction_bundle.rs, assistant communication_context.rs, loop_host surface_disclosure.rs, resolver_tests.rs), 24 left deliberately (flat- spelling narration about the family move itself, deleted-crate history, synthetic fixture names, and two #6945-class pointers whose target is gone at every spelling). 464 string-literal citations left: the specificity test resolves legacy spellings through the crate inventory by design. Triage of PR #7306 review comments; no gate weakened, both alias exceptions preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * review(7306): drop the one comment repoint in tests/e2e_trace_runtime_policy_org_ceiling_yolo.rs reborn_pr_test_plan.py has no mapping for this root test (it matches neither the tests/reborn_* partition inventory nor any other arm), so ANY PR touching it fails 'Detect Reborn test scope' — a pre-existing planner gap, confirmed against origin/main with a one-file changed list. The stale crates/ironclaw_runtime_policy comment path inside it stays until the planner learns the file; noted for follow-up rather than smuggling planner surgery into a review-triage branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(contributing): stop annotating the loose iteration clippy line as 'zero warnings' Same class as the root AGENTS.md fix: unflagged clippy exits 0 with warnings, so the annotation overclaimed. CONTRIBUTING's two-tier design (loose iteration block, then a stricter pre-PR block that already carries -- -D warnings) is deliberate and stays; only the claim is aligned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: recount the frozen WebUI route table after the #7306 merge — 93 -> 97 #7236 (main) added the four operator inspector routes without bumping the stated counts; re-derived on the merged tree: rg -c 'pub const WEBUI_V2_ROUTE_' crates/product/ironclaw_webui/src/webui_v2/descriptors.rs -> 97. Updates the two live claims (webui README, PROPOSAL SS6.9.4 with its strike-through recount convention); historical/superseded 92-row mentions stay as written. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ci): the code-style roll-up must judge fast-checks for guidance-only PRs The has_guidance trigger made fast-checks RUN for .claude/ and root-pair changes, but the roll-up's has_code==false branch exits 0 before it ever reads fast-checks' result — so check-guidance.py could fail and Code Style would still report success. The gate ran and could never block: exactly the inert-guard shape this change exists to remove, reintroduced one layer up. Fixed the way main's docs-publication gate already does it — judged before the early exit, with the reason in a comment. Sabotage-verified: has_guidance=true plus fast-checks=failure now exits 1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
a849373c41 |
fix(slack): restore personal delivery and standardized canaries (#7300)
* fix(qa): align Slack canaries with messaging standard * fix(slack): restore recency-ordered message search * fix(slack): retain provisioned personal DM targets * test(slack): cover default sort and DM scope resolution |
||
|
|
12e5a30dad |
feat(loop-host): add schema-aware deferred tool search (#7273)
* feat(loop-host): add schema-aware deferred tool search * test(loop-host): strengthen tool search isolation coverage * test(loop-host): expand tool search relevance benchmark * docs(loop-host): clarify retrieval recall semantics * fix(loop-host): address serrrfirat review — harden tool search bounds (#7273) * fix(loop-host): address coderabbit review — gate test fixture (#7273) * fix(loop-host): address coderabbit review — honor full surface policy (#7273) |
||
|
|
cbb3515477 |
feat(inspector): add prompt inspection and Prompt tab (#7239)
* feat(inspector): add operator inspection API * docs(inspector): assign product service ownership * test(inspector): ratchet diagnostic contracts * feat(inspector): add debug panel shell * test(inspector): cover debug panel shell e2e * fix(inspector): stop diagnostics when panel closes * feat(inspector): add prompt inspection * fix(inspector): follow current webui ownership * test(inspector): cover prompt diagnostics e2e * fix(inspector): redact prompt metadata * fix(inspector): classify prompt instruction sources * fix(inspector): address review feedback * fix(inspector): retry transient snapshot failures * fix(inspector): address prompt diagnostic review findings * fix(inspector): follow debug query navigation * fix(inspector): preserve stream terminal state * fix(inspector): capture full capability surface * fix(inspector): address prompt diagnostic review feedback |
||
|
|
389aa99971 |
fix(auth): omit the OAuth scope parameter when the ceiling is empty (#7309)
* fix(auth): omit the OAuth scope parameter when the ceiling is empty `build_recipe_authorization_url` appended the scope parameter unconditionally, so a recipe with an empty scope ceiling emitted `scope=` — present but valueless. RFC 6749 §3.3 makes `scope` optional but requires at least one token when it appears, and authorization servers enforce that: Attio answers `scope=` with `400 invalid_scope` while accepting the identical request with the parameter absent. That blocked every hosted-MCP OAuth registration, which has no manifest-declared scopes and so always builds an empty ceiling. Guard the append on a non-empty scope list. Verified against both live servers whose recipes carry an empty ceiling. Attio rejects only the empty-valued form. Notion parses an omitted and an empty parameter identically (`"scope":[]` in the state it echoes back), so the bundled notion-mcp extension changes URL bytes and nothing else. Every other bundled manifest declares a non-empty ceiling and is byte-identical. Regression test extends `extension_oauth_start_enforces_the_recipe_scope_ceiling`, which already drives the real `POST /setup/oauth/start` route through `AuthEngine::prepare_oauth_flow` into this builder, with an empty-ceiling case asserting the key is absent rather than empty. Refs #7308 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(auth): pin the empty-ceiling authorize URL at the gate caller `build_recipe_authorization_url` has two production callers: the Settings route `POST /setup/oauth/start` and the turn-blocked capability gate. Only the former pinned the empty-ceiling case, and the gate is the caller that reaches it FIRST for a hosted-MCP vendor — a blocked turn builds the authorize URL before any Settings-initiated connect runs. Verified discriminating: with the guard in `build_recipe_authorization_url` defeated, this test fails with `&scope=&` in the emitted URL. Refs #7308 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(auth): keep OAuth rationale provider-neutral --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: serrrfirat <f@nuff.tech> |
||
|
|
28845435c8 |
feat(inspector): add debug panel shell and live diagnostics client (#7236)
* feat(inspector): add operator inspection API * docs(inspector): assign product service ownership * test(inspector): ratchet diagnostic contracts * feat(inspector): add debug panel shell * test(inspector): cover debug panel shell e2e * fix(inspector): stop diagnostics when panel closes * fix(inspector): follow current webui ownership * fix(inspector): address review feedback * fix(inspector): retry transient snapshot failures * fix(inspector): follow debug query navigation * fix(inspector): preserve stream terminal state |
||
|
|
aff7d90cb6 |
fix(reborn): make installed and agent-authored skills selectable, installable, and complete (#6745)
* fix(reborn): inject skill bodies by default, not a one-line listing Reborn defaulted `SkillInjectionMode` to `Listing`, where a non-activated skill contributes only `- name: description` to context and its body loads only on an explicit `$name` mention or a `builtin.skill_activate` call. The intent was to save context budget. Benchmarking shows the model reads the menu and then never opens the skill. Over 30 runs with human-curated skills installed (SkillsBench/SkillLearnBench subset, `deepseek-v4-flash`, nearai/benchmarks#287): builtin.skill_list called in 30/30 runs builtin.skill_activate called in 3/30 runs a skill body actually read 0/30 runs So installed skills were effectively inert. Same 31 tasks, same skills, same model, varying only this default: no skills 78.5% curated skills, Listing 79.8% (+1.3pp -- skills bought almost nothing) curated skills, Full 85.6% (+7.1pp) For reference, harnesses that inject skill bodies unconditionally (Hermes, Claude Code) score 91.5% on these tasks with the same skills, so `Full` closes most but not all of that gap; the remainder is loop/verification behavior on a handful of multi-output tasks and is tracked separately. `Full` is already the library default in `SkillActivationSelectorConfig`; only the Reborn composition seam opted out. This restores it and adds a guard test so a revert is deliberate. `IRONCLAW_REBORN_SKILL_INJECTION=listing` still selects the previous behavior where context budget matters more than skills being used. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(skills): hot-swappable activation strategies so agent-authored skills are reusable Adds `skill.activation.v1`, a swappable-provider module in the shape of the memory-provider binding (`ironclaw_host_runtime::memory_binding`): named strategies, fail-closed resolution, behavior-preserving default, and a composition seam so nothing downstream names a concrete implementation. ## The bug it addresses `selector::score_skill` accumulates score ONLY from `activation.keywords` (+10/+5), `activation.tags` (+3) and `activation.patterns` (+20). A skill's `name` and `description` contribute nothing, and `select_skills` keeps a skill only `if score > 0`. That is fine for curated skills, which ship an `activation` block. It is fatal for skills an agent writes for itself: measured across the 31-task SkillsBench/SkillLearnBench subset in nearai/benchmarks#287, **0 of 30** agent-authored skills contained an `activation` block. Every one scored 0 and was permanently unselectable — the agent could create a skill via `builtin.skill_install` and then never reuse it, which makes self-improvement structurally impossible rather than merely weak. Claude Code has no such requirement: a skill is selectable from name and description alone. `ActivationStrategy::NameAndDescription` ports that contract. ## Design * `CriteriaOnly` (default) — today's rule, byte-identical. * `NameAndDescription` — whole-word name/description fallback, applied ONLY when the criteria pass scored 0, so a curated skill's explicit keywords always decide ordering and this can never reorder two skills that both declare metadata. `NAME_WORD_SCORE` (8) is deliberately below the selector's exact-keyword award (10). * `Disabled` — explicit mention / `skill_activate` only. * `ThirdParty { extension_id }` — production requires an admin override. Whole-word matching and a `MAX_FALLBACK_SCORE` cap keep it from over-selecting; over-selection is the failure mode that makes injecting an unrelated skill bank harmful (a whole-catalog injection took `xlsx_recover_data` 1.000 -> 0.271). ## Default stays behavior-preserving Reborn's default remains `CriteriaOnly`, opt in with `IRONCLAW_REBORN_SKILL_ACTIVATION=name_and_description`. Flipping the default changes three existing local-dev expectations (setup-marker suppression, the webui listing candidate, `skill_activate` context loading), so the strategy ships opt-in — the same discipline as the memory work, where the bundled native provider stays the default. ## Tests `cargo test -p ironclaw_skills --lib` — 239 passed, including: * `agent_authored_skill_unreachable_by_default_but_selected_under_name_strategy` — end-to-end via `prefilter_skills_with_options`: the same no-activation skill is dropped under `CriteriaOnly` and selected under `NameAndDescription`. * `name_strategy_does_not_select_an_irrelevant_skill` — no over-selection. * `name_hit_outranked_by_an_explicit_curated_keyword`, `whole_word_only_...`, `fallback_is_capped_...`, `stop_words_do_not_accumulate_score`. `cargo test -p ironclaw_first_party_extension_ports --lib` — 58 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(reborn): ship the Full skill-injection default as opt-in, not a flip The measurement in the previous commit stands: `Listing` leaves installed skills unread (`skill_list` 30/30 runs, a body actually opened 0/30) and `Full` is worth 79.8% -> 85.6% on the 31-task SkillsBench subset. But flipping the product default HANGS three existing local-dev tests, which drive a mock that expects the one-line listing candidate: * `local_dev_skill_activate_tool_loads_selected_skill_context` * `local_dev_webui_bundle_records_selectable_filesystem_skill_context` * `local_dev_runtime_wires_filesystem_skills_by_default_to_model_calls` Verified by bisect: all three hang on the previous commit alone, and pass with the default restored — the activation-strategy work is not implicated. Changing a documented product default in a way that turns CI red is a maintainer call, not something to force through, so `DEFAULT_SKILL_INJECTION_MODE` returns to `Listing` and `Full` ships as `IRONCLAW_REBORN_SKILL_INJECTION=full`. Both switches in this PR are now opt-in with the evidence attached, matching the memory-provider discipline where the bundled default is preserved. The guard test is retargeted to assert the current default, verify the opt-in path still resolves, and name the three tests that must be updated alongside a future flip. cargo test -p ironclaw_reborn_composition --lib -- skill_injection_mode \ local_dev_selector_config skill_activation # 14 passed, 0 failed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(threads): raise the result_read cap to 64 KiB, env-tunable A small per-request `result_read` cap turns one large file into a paging loop. On `manufacturing_equipment_maintenance` (nearai/benchmarks#287) reborn made 8 `read_file` calls and ZERO shell calls, hit the 24 KiB cap, then spent the whole turn paging — `result_read` at offset 24576, `handbook.pdf` at offsets 400/800/1200 — and never computed anything (`outputs_exist=0.00`). hermes, using shell to sample the same data, scored 0.522. * `TOOL_RESULT_RECORD_READ_MAX_BYTES` 24 KiB -> 64 KiB. This is the compile-time ceiling the model-observation envelope in `tool_result_reference.rs` is derived from (`* 2`, asserted at compile time), so 64 KiB here means a 128 KiB envelope — the reason not to go higher. * `TOOL_RESULT_RECORD_READ_DEFAULT_MAX_BYTES` = 64 KiB — the effective default. Enough that a typical data file or document page arrives in one read instead of a paging loop. * `IRONCLAW_TOOL_RESULT_READ_MAX_BYTES` overrides it, clamped to `[4, ceiling]`, so an override can never outgrow the envelope. Unparseable values fall back to the default rather than failing the run — a malformed tuning knob must not take down an agent. Unlike the skill-injection and skill-activation switches in this branch, this one does move the default: the paging loop is a silent capability loss rather than a behavior preference, and the knob exists for deployments that want the old size. cargo test -p ironclaw_threads --lib # 88 passed (85 existing + 3 new) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(skills): add always_available activation, Claude Code's actual contract `skill.activation.v1` gains a third binding, `always_available`: every installed skill is a candidate regardless of what it matches. This is what Claude Code and Hermes actually do. In both, a skill is a file in a directory the agent can read, so there is no gate for a correctly-installed skill to fail. Reborn's selector instead scores only `activation.keywords`/`tags`/ `patterns` and drops anything scoring 0 -- and `name_and_description` (this branch's earlier binding) only WIDENS that gate: it still needs a lexical hit, so an applicable skill phrased differently from the prompt is still discarded. The new test pins exactly that case -- a skill described as "cyclical component / growth path" against a prompt saying "hp filter" is dropped by both `criteria_only` AND `name_and_description`, and kept by `always_available`. Why it matters, measured on the 31-task SkillsBench/SkillLearnBench subset in nearai/benchmarks#287: 0 of 30 agent-authored skills contained an `activation` block, so under `criteria_only` a self-authored skill could never be selected again -- self-improvement was structurally impossible. Implementation is deliberately tiny: a `floor_score()` of 1 for this binding, applied via `.max()` in the selector's existing scoring loop. Ordering is untouched (a real keyword match still outranks a floor skill, so the context budget spends on the relevant skill first), and the existing budget -- not the score filter -- decides what is injected, which is also how Claude Code behaves. `floor_score()` is 0 for every other binding, so non-adopters are byte-identical. Default remains `criteria_only`; opt in with IRONCLAW_REBORN_SKILL_ACTIVATION=always_available. cargo test -p ironclaw_skills --lib # 241 passed (239 existing + 2 new) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(threads): drop the now-unused ceiling import Validation bounds against `contract::effective_tool_result_read_max_bytes()` (which applies the env override), so the compile-time ceiling is no longer referenced here. Removes an unused-import warning introduced by the 64 KiB cap commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * revert(threads): default result_read back to 24 KiB, keep the knob The raise to 64 KiB was never isolated: it shipped in a measurement arm alongside two other switches (skill activation, tool disclosure), so there is no evidence it changed anything. Defaulting it back keeps this crate byte-identical to pre-PR behavior. The paging trace that motivated it is real (`manufacturing_equipment_maintenance`, nearai/benchmarks#287: 8 `read_file` calls, zero shell calls, `result_read` at offset 24576, nothing computed) — but a real trace is not a measured fix, so the larger cap stays opt-in via IRONCLAW_TOOL_RESULT_READ_MAX_BYTES for whoever wants to measure it properly. The compile-time ceiling stays 64 KiB: it now bounds only how far the env override may reach, and still pins the derived model-observation envelope at 128 KiB. Net effect of this commit plus its parent: a new env knob, no default change. cargo test -p ironclaw_threads --lib # 88 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): design for agent-authored multi-file skill bundles @henrypark133 pushed back on "move skills to the filesystem" as an overhaul that a single aggregate result did not justify. He was right, and stratifying the data shows why: the entire filesystem gain sits in skills that ship files besides SKILL.md. ships resource files (n=16): inject 81.0% -> files 94.2% (+13.2pp, CI [+0.3, +26.2]) SKILL.md-only (n=11): inject 91.5% -> files 84.7% (-6.9pp, CI [-20.4, +6.7]) So filesystem-for-everything is a REGRESSION on 13 of 31 tasks, paid to fix the other 18. The mechanism is not "models prefer filesystems": 81 of the resources are executable (you cannot run pasted Python -- citation_check scored 0.000 with the script absent, 0.833 with it present), and the text resources are too large to inline (exceltable_in_ppt would be ~262k tokens folded into SKILL.md). The design therefore keeps storage, discovery and selection exactly as they are and adds ONE extension holding the already-existing `/skills` read_write mount: skill_write_file / skill_read_file / skill_list_files. Discovery already lists from the same root that mount writes to, so nothing needs plumbing. Executing a bundled script copies that one file into `/workspace`, which the agent already mounts. Documents two things the implementation must not miss: SkillBundleDescriptor exposes only `skill_md_path`, so bundle resources are un-advertisable without skill_list_files; and `FilesystemSkillBundleRoot::user` marks bundles Trusted, so an agent that can write executable scripts there needs a distinct trust level -- the real open question. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): state explicitly that creation, discovery and indexing are unchanged The crux of @henrypark133's objection. Spells out, per concern, that skill creation stays on the `skill_install` tool, discovery stays on the storage-agnostic `SkillBundleSource` trait with no new impl / trait method / descriptor change, and that there is no session-start index to migrate at all (selection is per-request; the only cache is a 5-minute TTL on catalog search). The single behavioral change remains the opt-in `always_available` selection predicate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): the write tool needs an authoring prompt that asks for code skill_write_file makes multi-file skills possible; it does not elicit them. Measured: 6 of 31 tasks finished with ZERO skill_install calls despite 'Saving the skill is required', and the authoring request only ever asks for prose (method, conventions, output contract). An agent following it writes prose whether or not a write tool exists. Adds the elicitation requirement and a falsifiable success criterion: agent-authored bundles are currently 100% prose (0 of 27 ship a resource file) against 18 of 31 curated skills. If that ratio does not move once the tool ships, the bottleneck was elicitation rather than capability and the tool alone will not move scores. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(skills): let an agent install skill bundles, not just prose Agents could only ever author the PROSE half of a skill. Measured on the 31-task SkillsBench/SkillLearnBench subset (nearai/benchmarks#287): **0 of 27** agent-authored skills shipped a single file besides SKILL.md, against **18 of 31** human-curated ones (79 .py scripts, 78 .xsd schemas, 84 .md references). So every later run re-derived the method from prose and could re-make the same mistake -- lake_warming's self-authored skill described its regression procedure in prose, the next run recomputed it slightly differently and missed the grader's p<0.05 threshold. This was NOT a missing capability. `install_skill` has always taken `files: &[SkillInstallFile]`, and `parse_install_files` has always read an `input["files"]` array. Two things made it unreachable: 1. `schemas/builtin/skill_install.input.v1.json` advertised only `name`/`content`/`url` AND set `additionalProperties: false` -- so a model sending `files` was not merely uninformed, it was REJECTED. Across 112 observed skill_install calls, 111 used exactly `['content','name']`, which is what the schema permits. 2. The only encodings were `bytes_base64` and a JSON array of byte integers. A bundle file an agent writes is a script, a reference doc or a schema fragment -- all UTF-8. Making those go through base64 costs ~33% more tokens and turns one encoding slip into an InputEncode failure of the whole install. Changes: - `parse_install_files` accepts `text` (UTF-8) alongside `bytes_base64`/`bytes`. `text` takes precedence when both are given, matching the documented preference. Binary payloads are unaffected. - the schema advertises `files` with `path` + `text`/`bytes_base64`, and the description tells the model WHY to use it: put a reusable computation in a script rather than describing it in prose, and have SKILL.md name the files it relies on. That last part matters because `SkillBundleDescriptor` exposes only `skill_md_path`, so a bundle cannot advertise its own resources. - prose-only installs are untouched: no `files` key still parses to an empty vec. cargo test -p ironclaw_first_party_extensions --lib install_files_encoding # 4 passed cargo test -p ironclaw_host_runtime --test tool_surface_contract # 43 passed cargo test -p ironclaw_reborn_composition --test product_live_adapters skill_install # 1 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(skills): stop rejecting an install that carries both content and files `skill_install_input` gated the direct-install arm on `!object.contains_key("files")`, so `content` + `files` matched NO arm and fell through to `_ => Err(InputEncode)`. An agent attaching a script had its ENTIRE install refused. `files` was reachable only on the URL-fetch arm, which builds the array itself. This was the third of three stacked gates hiding the same capability, and the one that actually bit. With the schema fixed to advertise `files` and a `text` encoding available, the model on the 31-task SkillsBench subset (nearai/benchmarks#287) immediately sent 18 correctly-shaped `{path, text}` entries across 9 calls -- `scripts/verify_bib.py`, `references/fake_patterns.json` -- and every one was rejected here. That is the real reason 0 of 27 agent-authored skills shipped a resource file while 18 of 31 human-curated ones do: not a missing capability, and not the model failing to try. `source`/`source_url` stay excluded from the direct arm: those record provenance and are set by the URL path, so an agent must not be able to forge them. cargo test -p ironclaw_host_runtime --lib skill_install_input # 4 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * style: rustfmt the skill-bundle and activation changes Test modules were appended programmatically without rustfmt, which is why Formatting, Code Style and Clippy all went red on this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): rewrite to match what was measured, not the abandoned design The doc recommended a three-tool extension plus a resource-gate. Both are superseded: the tools turned out to be redundant (install_skill already accepted files -- three stacked gates were hiding it), and the gate MEASURED WORSE than always advertising a readable path (-25.7pp on self-creation, -40.6pp vs claude-code), because an agent-authored skill is usually SKILK.md-only so the gate suppresses the one route the selector had not already closed. Rewritten around the durable findings: the three gates and how each masked the next, the 0-of-27 vs 18-of-31 measurement, the SkillBundleDescriptor enumeration gap, and the trust question. The gate is kept in the doc as a recorded negative result, since its stratified justification is persuasive and will be proposed again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(skills): correct why always_available is not the default The previous note claimed the floor score overrides setup-marker suppression. It does not: `prefilter_skills_with_options` returns None for a satisfied marker BEFORE scoring, and the host-side filter in activation.rs already removed the candidate. What actually fails: all 32 bundled skills reach floor 1, so 3-4 unrelated ones land in plan.activations() in ActivationCriteria mode -- a mode that injects nothing under Listing. The defect exposed is that a criteria activation which injects no body is still recorded as an activation, so the count assertions stop being meaningful. Also records the sequencing against epic #6565 (Slice 0 first; Slice 5's bounded-shortlist rule constrains what an unbounded floor may do) and the measured detail that under Listing a zero-scoring skill is still listed -- the model just called skill_activate in only 3 of 30 runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(skills): a floor-only skill is ranked, not activated Three defects, all surfaced by trying to make `always_available` the default. It failed 8 tests in ironclaw_reborn_composition; all 8 now pass with the flag on AND off. 1. A criteria selection that injects nothing was still recorded as an activation. Under `SkillInjectionMode::Listing` an `ActivationCriteria` entry contributes no body -- `body_eligible_bundle_ids` already ignores that mode -- so with a floor score every installed skill "activated" on every turn. Concretely: all 32 bundled skills reach floor 1, and 3 of them (6000 token budget / 2000 default per-skill cost) landed in each plan, chosen by descriptor order because the score sort is stable. `SelectionOutcome` now returns those separately as `ranked_only`, and the activation path does not iterate them. They still reach the model through the listing, which is where they belonged. 2. `AlwaysAvailable` also enabled the name/description fallback, which manufactured fake merit: a bundled skill whose description shares one word with the message scored above zero and was reported as a genuine activation. Under `AlwaysAvailable` the fallback adds no reach at all (the floor already admits everything), so it is now scoped to `NameAndDescription`, where widening the match is the entire point. This is what kept `local_dev_runtime_suppresses_explicit_setup_skill_when_workspace_marker_exists` failing after (1). 3. Raising TOOL_RESULT_RECORD_READ_MAX_BYTES to 64 KiB was NOT the no-op this PR claimed. `tool_result_reference.rs` derives MAX_MODEL_OBSERVATION_BYTES from it (* 2), so the observation envelope silently doubled 48 KiB -> 128 KiB and preview truncation changed for every caller. It broke three tests whose fixtures are sized against the envelope ("fixture must exceed the preview cap"), independently of any activation setting. The contract ceiling is back to 24 KiB and the env override is bounded by a new TOOL_RESULT_READ_ENV_CEILING_BYTES that nothing is derived from -- so the knob can raise a single read without moving anyone else's behavior. Correcting the record on an earlier comment in this PR: the failures were never the setup-marker interaction. Marker suppression returns None before scoring, so a floor score cannot revive a suppressed skill. cargo test -p ironclaw_reborn_composition --lib # 634 passed IRONCLAW_REBORN_SKILL_ACTIVATION=always_available cargo test -p ironclaw_reborn_composition --lib # 634 passed cargo test -p ironclaw_skills --lib # 241 cargo test -p ironclaw_threads --lib # 88 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * revert(skills): remove the always_available strategy, it bought nothing Verified against this branch: `AlwaysAvailable` was a no-op for everything the model can observe, and it carried a regression. Removing it rather than wiring a compensating half. Why it bought nothing. Listing membership is decided by VISIBILITY, not selection (extension_ports/activation.rs partitions candidates on body-eligibility, and everything not body-eligible still goes into the listing), so the model was ALREADY shown every visible skill before this strategy existed. The floor score never added reach -- pre-C1 its only effect was listing ORDER, and after C1 excluded floor-only skills from activations even the ordering effect was gone, because the ranking input is derived from the activation list. `SelectionOutcome::ranked_only` had no production reader at all: allocated, populated, returned, dropped. Under `Full` a floor-only skill could never be injected either, since `context_candidates_for_plan` renders only activated bundles. The regression. The floor-only bookkeeping ran for every non-merit entry BEFORE `try_select`, so under this strategy a chain-loaded companion got its own loop iteration, was recorded as floor-only, and was then partitioned OUT of `selected` -- i.e. `A requires B` activated only `A`, where `CriteriaOnly` activates both. Strictly worse than the default for any bundle with companions, and order-dependent. The comment claiming this could not happen was wrong. Also removed: ~29 "budget exhausted" notes per turn that reached `feedback` and fired a SkillActivation live-projection event with empty skill_names, because floor-only skills still ran the budget loop and `BudgetFull` continues rather than breaks. Kept: `NameAndDescription`, which has a real effect (matching on name/description, not only `activation.keywords`/`tags`/`patterns`), and the `skill.activation.v1` seam. Corrects the record in two places that argued the opposite: the runtime.rs doc comment and docs/skills/agent_authored_bundles.md. The measured reachability gap is elicitation, not filtering -- `builtin.skill_activate` was called in 3 of 30 runs and a body read in 0 of 30 -- so the next step is the listing header, not a scoring change. Note the parity numbers in nearai/benchmarks#327 never depended on this strategy: those arms ran with it off. cargo test -p ironclaw_reborn_composition --lib # 634 passed cargo test -p ironclaw_skills --lib # 240 passed cargo test -p ironclaw_threads --lib # 88 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(skills): show that an agent-authored skill carries scripts This PR is what lets an agent author a skill containing a script. The Skills page could not show that it did: `skill_info` hardcoded `has_requirements: false` and `has_scripts: false`, so a scripted skill was indistinguishable from a prose-only one. The WebUI has rendered the chips since #6194 and the wire fields have existed since #7002 -- only the server never populated them. It was not just agent-authored skills. `portfolio`, a BUNDLED skill, ships four Python scripts (`weekly_report.py`, `backtest_strategy.py`, `concentration_warning.py`, `alert_if_health_below.py`) and has always displayed as prose-only. - `SkillSummary::has_scripts`, from one stat on the bundle's sibling `scripts` path. Absent is the common case and is not an error, so only a genuine backend failure is logged -- a skill listing must never fail because a skill has no scripts. - The bundled-summary path reads it from the embedded bundle files, so `portfolio` reports correctly there too. - `has_requirements` comes from `requires_skills`, which was already on the summary. Verified on a live production server: 33 skills listed, `portfolio` the one reporting `has_scripts`, six reporting `has_requirements`. Note: skill scripts still cannot EXECUTE under hosted multi-tenant -- `HostedMultiTenant` + `SecureDefault` resolves to `ProcessBackendKind::None`, which strips `builtin.shell`. That is deliberate pending the tenant sandbox. So the chip tells a multi-tenant user their skill has scripts the agent can read but not run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): pin the direct-install input contract at the capability boundary Four cases covering what a caller may and may not put in a `builtin.skill_install` input, asserted through runtime dispatch rather than against whichever helper currently normalizes the input. The normalizer has already moved once (host runtime -> ironclaw_extension_support, WS3) and is about to be merged across that move again. Written so the same four pass on both sides: a resolution that quietly re-tightens the inline arm fails the first one instead of silently dropping the capability this PR adds. The dividing line these pin is provenance, not shape: - `content` + `files` installs, and the script lands on disk verbatim - `bytes_base64` works on the direct arm too, not only the rewritten URL payload - `content` + `files` + `source`/`source_url` is still refused whole - a `../..` bundle path is refused and writes nothing outside the skill directory Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): repair the post-merge build and close the review findings Build breakage from the merge: main added two `SkillSummary` construction sites (`skill_learning`, `lifecycle_product_service`) that this branch's new `has_scripts` field left incomplete. Both are fixtures whose assertions do not depend on it, so both set `false` with a note saying why. CI, all under `-D warnings`: - Four `result_read` cap items in `ironclaw_threads::contract` were `pub` in a private module (`unreachable_pub`). Nothing outside the crate reads them, so they are `pub(crate)`. - Four constant-value asserts (two in the same contract module, two in `activation_strategy`) move into `const {}` blocks, which is what they always meant: they are compile-time invariants, not runtime checks. Review findings: - The stale-default doc note (coderabbit, ironloop) was against `ecabcb5fe`, before `4951d76bb` reverted the flip. Docs and `DEFAULT_SKILL_INJECTION_MODE` both say `Listing` with `full` as the opt-in, so there is nothing left to correct. - The unset-env branch is now reachable from a test (coderabbit). `skill_injection_mode_from_env_value` takes the lookup's `Result`, so the product default can be asserted without `remove_var` racing every other test in this binary. Covered along with `full`, trimming/case, empty, unrecognized, and non-unicode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(skills): state accurately why the inline arm now accepts a bundle The comment claimed a review decision had been "reversed on evidence", which overstates what happened and would read as overriding the team. What actually happened: the refusal predates #7141 entirely -- it lived in the host-runtime copy of this resolver, and #7141 carried it across the move to this crate verbatim, declining a reviewer's suggestion to relax it there. That was the right call for a move-only refactor. This PR is where the behavior change belongs, and it is made deliberately with the measurement attached. Comment only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci: raise the composition mass ceiling for this layer's assembly wiring CI's "Check composition mass budget" step reds this branch by 62 LOC. Not because this branch is large: main sits only 54 LOC under the effective ceiling (40,499 + 150 tolerance against 40,595 observed), so the gate currently trips on any PR adding more than that to composition, and this one adds skill-summary and product-surface assembly. Raised to the measured 40,711 in both places the gate pairs — `[gate].loc_ceiling` in the manifest and `COMPOSITION_ABSOLUTE_SRC_LOC` in `reborn_restructure_baselines.rs`, since a second ratchet fails when they disagree, which is how it enforces recording the change in the PR that causes it. Measured with `check-composition-budget.sh --print`, set to current rather than padded, per the manifest's own protocol. A raise is a reviewed decision by that file's rules, not routine wiring, so it is flagged here and in the PR body rather than left in a diff. The next wave close should re-ratchet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(threads): make the result_read env knob actually raise the cap serrrfirat, High: `IRONCLAW_TOOL_RESULT_READ_MAX_BYTES` was inert. It widened `validate_tool_result_record_read`, which sits DOWNSTREAM, while the caller-facing gate in `result_read.rs` stayed pinned to the compile-time `TOOL_RESULT_RECORD_READ_MAX_BYTES` (24 KiB) and the advertised schema still said `maximum: 24576`. A larger read was rejected before it could reach the widened validator, so setting the variable changed nothing. The gate and the schema now resolve `effective_tool_result_read_max_bytes()` per request. This also corrects a fix I made earlier in this PR for the wrong reason. Clippy flagged `effective_tool_result_read_max_bytes` as `unreachable_pub` and I narrowed it to `pub(crate)` — but it had no cross-crate caller precisely BECAUSE the wiring was missing. The lint was reporting the bug, not dead code. It is `pub` again, with the caller it was always supposed to have. Tested as a wiring identity (gate == effective cap, schema == gate) rather than by setting the env var: these tests run in-process and in parallel, so mutating process environment races every other test reading it, and the identity is exactly what regressed. Verified: workspace `cargo check --all-targets` clean, 12 test binaries green across loop_host and threads, fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(threads): satisfy the production-target lint lane CI's PR lane lints the DEFAULT target set (lib + bins, no tests or examples) with \`--all-features\`; I had been running \`--all --tests --examples\`, and the extra targets masked both of these. - \`TOOL_RESULT_READ_ENV_CEILING_BYTES\` was widened to \`pub\` alongside \`effective_tool_result_read_max_bytes\` in the previous commit, but only the function is re-exported from \`lib.rs\`, so the constant was unreachable-pub. Only that function reads it, so it is \`pub(crate)\`. - \`result_read.rs\` no longer reads \`TOOL_RESULT_RECORD_READ_MAX_BYTES\` now that the gate resolves the effective cap, so the import goes. Verified with the lane CI actually runs (\`cargo clippy --workspace --all-features -- -D warnings\`, no test targets), plus fmt and the loop_host/threads suites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(skills): cut the comment bloat on this layer Comment share of this PR's diff: 33% -> 26%, 471 -> 330 lines. The measurements that justify a constant stay; the retellings of how we got there go, since they are already in the commit history. One of these was a duplicate rather than verbosity: `DEFAULT_SKILL_ACTIVATION` carried an earlier draft stacked directly above its own replacement, so the file gave two competing accounts of the same default. I had removed that copy at the top of the stack only; removing it here means all three layers carry one version instead of conflicting on every merge. Also corrects a doc that contradicted the code, which Copilot flagged: `effective_tool_result_read_max_bytes` was documented as clamping to `TOOL_RESULT_RECORD_READ_MAX_BYTES` when it clamps to `TOOL_RESULT_READ_ENV_CEILING_BYTES` — the whole point of the separate ceiling. Comments only; no code touched. `--all-features` clippy on the production target set (the lane CI runs) clean, fmt clean, 19 test binaries green across skills / threads / loop_host / extension_support. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(skills): move the design doc under the fenced internal tree `docs: enforce the docs/ publication boundary` landed on main at 00:53 and dequeued this PR: every file under `docs/` must be either published (referenced from `docs.json` navigation) or fenced (by `docs/.mintignore`), and `docs/skills/agent_authored_bundles.md` was neither — so it would have been deployed to the public docs site and indexed. That is exactly what the gate exists to catch, and the doc predates the rule rather than breaking it. `.mintignore` is frozen by that same change ("all new internal material goes under internal/"), so the fix is the move, not a new fence entry. `docs/internal/` is already fenced. `scripts/ci/test_docs_publication_boundary.py`: 21/21 pass. Workspace `--all-features` clippy on the production target set clean, fmt clean, panic gate clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
43d0494c46 |
fix(filesystem): make libSQL FTS safe for natural-language recall (#7288)
* fix(filesystem): treat FTS filters as plain text * fix(filesystem): address review — PG stop list, term-match tests, DRY (#7288) Addresses multi-agent review on #7288: - Plain-FTS stop words now mirror PostgreSQL's fixed english stop list (shared/english.stop) verbatim, so in-memory/libSQL required terms match plainto_tsquery('english', ...) exactly; documents the remaining stemming divergence (FTS5 matches literal terms). - Exhaustive table-driven test pins every stop word (case-insensitive) plus required non-stop words (please/tell/would/could). - libsql FTS contract test adds a partial-match negative document so the FTS5 implicit-AND join is distinguishable from an OR join, and proves the negative doc is searchable by its own terms. - In-memory reference matcher now tokenizes stored text (whole-token matching, mirroring FTS5 unicode61) instead of substring containment, fixing contractions divergence; FTS queries are tokenized once per query instead of once per scanned record. - core_builtin harness: shared-filesystem variant reuses the recording harness assembly tail instead of re-copying it. * test(integration): prove memory recall is scope-isolated on the libSQL path (#7288) The proactive-recall scenario only checked that a never-written marker was absent, which says nothing about scope isolation. Seed a second user's MEMORY.md — word-for-word the canonical document apart from the marker — into the same libSQL composite, then assert the canonical user's explicit memory_search and proactive prompt both still return plum-42 and never the other user's marker, while that marker stays retrievable in its own scope. The seed goes through the native provider rather than a second actor's thread: this group pins capability dispatch to one fixed user, so a second actor's memory write would land in the canonical scope anyway. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
4a11fc832a |
fix(capabilities): unify disclosure and enforcement policy (#7233)
* fix(capabilities): unify disclosure and enforcement policy * fix: share resolved capability surface with host * fix: address capability surface review feedback * fix: map golden snapshots to Reborn test lane * refactor: unify visible surface filtering * test: enforce filtered batch outcome contract * fix: validate filtered batch suspension state * test: pin golden snapshot lane ownership * test: avoid artifact mapping order dependency * fix(review): address remaining PR feedback |
||
|
|
e54a9bb6b9 |
chore(deps): bump brace-expansion in /docs/internal/architecture-video (#7311)
Bumps and [brace-expansion](https://github.com/juliangruber/brace-expansion). These dependencies needed to be updated together. Updates `brace-expansion` from 1.1.13 to 1.1.18 - [Release notes](https://github.com/juliangruber/brace-expansion/releases) - [Commits](https://github.com/juliangruber/brace-expansion/compare/v1.1.13...v1.1.18) Updates `brace-expansion` from 2.0.3 to 2.1.4 - [Release notes](https://github.com/juliangruber/brace-expansion/releases) - [Commits](https://github.com/juliangruber/brace-expansion/compare/v1.1.13...v1.1.18) --- updated-dependencies: - dependency-name: brace-expansion dependency-version: 1.1.18 dependency-type: indirect - dependency-name: brace-expansion dependency-version: 2.1.4 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
03f1fee648 |
chore(deps): bump fast-uri in /docs/internal/architecture-video (#7312)
Bumps [fast-uri](https://github.com/fastify/fast-uri) from 3.1.4 to 3.1.5. - [Release notes](https://github.com/fastify/fast-uri/releases) - [Commits](https://github.com/fastify/fast-uri/compare/v3.1.4...v3.1.5) --- updated-dependencies: - dependency-name: fast-uri dependency-version: 3.1.5 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |