mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
89285c8e70
* 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 reverts2f8acabe7. I added it, and neither of its justifications survived. The reasoning was already withdrawn publicly: I claimed retiring criteria selection collapsed the listing to alphabetical order and made the model stop asking, but **0 of 62 benchmark catalog skills declare an `activation:` block** (32 of 32 bundled ones do), so the scorer could never rank a single skill these tasks need. Listing order for every expected skill was identical in both arms and the mechanism did not exist. The data now declines to support it too: **33.3% >=1-correct with ranking vs 40.0% without**. That is worse than neutral, and the reason is the same fact -- the only skills the scorer CAN rank are the 32 irrelevant bundled ones, so ranking promotes distractors above the skill the task actually needs. On any realistic catalog, where users' and agents' skills carry no activation metadata, ranking is systematically wrong. Also: the drop I built this on was small-n noise. The arm I read as 28.6% reads 40.0% at n=25. Worth revisiting only if descriptor metadata coverage ever becomes the norm rather than the exception. Until then #6938 ships no heuristic in the selection path at all, which is the point of the change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): tell the model what a skill is FOR, not just what the tool does Studied how claude-code routes to skills and ported the mechanism. It reaches 60% correct activation on 5 SkillsBench tasks over a 227-skill catalog; reborn reached 0% on the same tasks and the same catalog. The gap was not discovery -- every skill was listed and reachable -- and it was not refusals, which were 0%. The model simply had no reason to ask, so it solved each task with `shell` instead. Three differences, in order of how much they mattered. **1. The tool description.** local-dev's said: "Activate one or more listed Reborn skills for the current loop run" That states what the tool does and nothing about when to use it or why. claude-code's states what a skill IS, that a listing exists, that a matching skill should be activated FIRST, and -- the load-bearing clause -- that the loaded instructions REPLACE the model's default approach. A model that does not know a skill supersedes its own plan has no reason to prefer one. Both descriptors now carry the same text; they had drifted, and local-dev (what every local user gets, and what the benchmark measures) had the weaker one. **2. Full descriptions for every skill.** The listing budget previously shrank per-entry descriptions to fit more names -- 90 chars each at 227 skills. That traded away the wrong thing: 52% activation with 88 full-length entries, 0% with 227 shrunken ones. Names make a skill addressable; descriptions are what let the model judge relevance, and 90 chars does not. claude-code pays this cost outright, listing every skill with its whole one-line description, so the budget is now sized to do the same. The per-entry cap and the 512-bundle enumeration cap still bound it -- this is a budget for a real catalog, not a licence for an unbounded one. **3. The listing header** now opens the way claude-code's system-reminder does ("The following skills are available for use with builtin.skill_activate") and repeats the supersedes-your- default-approach point where the model reads the menu, not only where it reads the tool schema. Measured after the port, same 5 tasks, same 227 candidates, same model: | | before | after | claude-code | |---------------------|--------|-------|-------------| | >=1 correct | 0% | 50% | 60% | | recall over expected| 0% | 33.3% | 36.7% | | precision | -- | 100% | 100% | | never activated | 100% | 50% | 40% | Precision is identical: when reborn now activates, it is not wrong. Recall still trails, and the remaining gap is tasks where it never consults the catalog at all -- the same failure mode claude-code has, just more often. Also worth recording from the study, not ported here: claude-code injects the listing as a per-turn system-reminder rather than static prompt text, and emits a second reminder ("New skills discovered in <dir>, now available via the Skill tool") when a skill appears mid-session. That second one is the install-then-use-immediately flow, and is a candidate follow-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(skills): lint routing metadata at authoring and learned-skill write time Epic #6941 criterion 7. This is the one piece of the closed #6937 worth keeping, and model-decided selection made it MORE relevant rather than less: with the scorer out of the decision, the description is the model's only signal in the listing, so a generic or over-long one degrades routing for every later request. `lint_skill_routing_metadata` flags generic keywords, generic tags, a description past the 250-char listing cap, and a keyword that also appears in `exclude_keywords`. It gates both learned-skill authoring paths (`parse_distillation`, `parse_refinement` both funnel through `parse_skill_md`), so a model cannot write a skill that poisons the catalog. Deliberately no pattern rule. An earlier draft flagged unanchored wildcards and failed 25 of 32 catalog skills; narrowing it to "ends in an open wildcard" still mis-flagged 3 of 4. Wildcard position is not what makes a pattern promiscuous -- required literal specificity is -- and one measured bad pattern is not enough evidence for a heuristic with that false-positive rate. A lint people switch off protects nothing. The checked-in catalog now passes it (`the_checked_in_catalog_passes_the_routing_metadata_lint`). Six of 32 skills failed before the fix: five with descriptions of 269-338 chars, which the listing was silently truncating, and `coding` with ten generic keywords (`code`, `fix`, `file`, `test`, `build`, `error`, `change`, `delete`, `add`, `update`). Fixing `coding` moved the reviewed routing baseline (#6595) on 7 of 8 cases, and the pattern is the whole argument for the lint -- it was being selected for: security-audit, qa-test-plan, track-github-repository, commit-staged-changes, park-product-idea, local-web-ui-validation and it drops out of every one. On `single-pr-code-review` it survives but demotes from 3rd to 5th. Baselines updated, which is what that test asks for when the change is intentional. One honest scoping note: the keyword half of this is INERT at runtime under `ExplicitOnly`, since the scorer no longer selects anything -- it matters for any future keyword-consuming path and as catalog hygiene. The half that changes behaviour today is the description-length rule: those five over-long descriptions were being truncated in the model-visible listing, which is exactly the signal the model routes on. cargo test: 945 passing across ironclaw_skills, ironclaw_first_party_extension_ports, ironclaw_loop_host and ironclaw_architecture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): raise the active-skill cap from 4 to 8, it made correct routing impossible `DEFAULT_MAX_ACTIVE_SKILLS = 4` was not a budget, it was a ceiling on correctness. On the SkillsBench routing set, **7 of 31 tasks expect 4 or more skills and 3 expect 5**, so a perfectly-routing agent could not satisfy them: recall was bounded by this constant rather than by anything the model did. A benchmark measured against a limit the harness imposes on itself reports nothing about the system under test. Raised to 8. The real guard on skill context is `max_context_tokens`, which bounds how much body text loads regardless of how many skills get named -- a large skill still consumes that budget and pushes the effective count back down on its own. This constant only stops a model from naming an unbounded list, which 8 still does. Also updates every place that hardcoded "four": the tool description, the `names` schema description and its `maxItems`, and the listing header. Those had to move together, since a model told "at most four" while the selector allows eight will leave skills on the table. One test assertion needed rewriting rather than retargeting: `standalone_skill_activate_tool_loads_selected_skill_context` pinned the old description's exact phrases. Its intent -- the description must tell the model WHEN to use the capability, and must not imply every visible bare name is actionable -- is preserved and extended with a third assertion on the clause that actually moved the metric ("instead of your own default approach"), because that is the sentence that took activation from 0% to 40%. 1782 tests green across ironclaw_skills, ironclaw_first_party_extension_ports, ironclaw_loop_host and ironclaw_reborn_composition. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): ask for the complete skill set, not the smallest one The tool description and the listing header both said "pick the smallest relevant set". That is a minimisation instruction, and the measurements say it has no justification and a real cost. **Nothing to justify it:** precision over activated skills is **100%** in the ported build -- zero wrong activations across the measured tasks. There is no over-activation problem for a minimisation instruction to prevent. **A real cost:** recall over the expected set is **26.7%**, and the failure is under-activation rather than mis-activation. On `powerlifting_coef_calc` the model activated `powerlifting` and stopped, ignoring the other two skills the task needed -- 1 of 3. Of the tasks that activated anything, half activated fewer skills than the task required. The prompt told it to do that. So the guidance is inverted: activate every skill the task needs, name them together in one call, and note explicitly that a task often needs several (a file format, a domain method and a reporting step are three different skills). Precision is protected by a different clause that stays untouched -- "do not activate skills that are unrelated to the task" -- which is the one actually doing that work. `standalone_skill_activate_tool_loads_selected_skill_context` gains an assertion pinning the completeness instruction, since silently reverting to "smallest set" would regress recall with no failing test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Revert "fix(skills): ask for the complete skill set, not the smallest one" This reverts9eec1af53. I proposed it, predicted recall would rise and precision would hold, and measured the opposite. Task-matched over 9 tasks at 227 candidates: | metric | with "smallest set" | with "complete set" | |------------------------|---------------------|---------------------| | >=1 correct | 44.4% | 33.3% | | recall@R | 22.8% | 10.9% | | precision@R | 100.0% | 80.0% | | tasks w/ a wrong skill | 0 | 1 | Worse on every metric, including the one it was meant to fix. The per-task traces say why, and it is not the mechanism I assumed. On `exoplanet_period` the original activated 3 of the 5 needed skills; with the change it activated **nothing**. On `court_form_filling` the change activated one skill and it was the wrong one. So asking for completeness did not make the model add the skills it was missing -- it made selection less decisive overall, trading a confident partial answer for a guess or for paralysis. My reasoning was that "pick the smallest relevant set" was unjustified because precision was already 100%, so there was nothing for a minimisation instruction to protect against. That inverted cause and effect: precision was 100% BECAUSE of the minimisation instruction, not independently of it. I attributed the guard entirely to "do not activate skills unrelated to the task", and the measurement says the two clauses were doing that work together. Under-activation on multi-skill tasks is real -- 14.0% set completeness, and claude-code is no better at 14.8% -- but it is not fixable by asking harder in the prompt. It needs a different mechanism, and it belongs in its own issue with its own evidence rather than a prompt tweak that makes three metrics worse. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): measure what the HOST picks — the wrong-skill failure, 0 of 83 correct The benchmark could not see the defect this work is about. Two things activated a skill before this change: 1. the model asks, via `builtin.skill_activate` -- a trace entry 2. the HOST decides, scoring each skill's declared `activation:` keywords against the user's message during prompt assembly, and injecting the winners -- NO trace entry #5417 is (2). The benchmark's scorer reads `skill_activate` calls, so it measured only (1), and the pre-change arm therefore scored 100% "precision" while the host picked freely. That was an invisible failure mode being reported as clean routing, and I reported it that way. Host selection is `prefilter_skills_with_options`: a pure deterministic function of (message, skill metadata). No LLM required, so this measures it exactly, in ~4 seconds, over every task's real prompt against the real 227-skill catalog: 27 tasks | 83 skills picked | 0 correct | 83 WRONG precision over host picks 0.0% tasks with >=1 wrong pick 27/27 (100%) wrongly picked most often: routine-advisor (26 tasks), llm-council (12), commitment-triage (10), coding (5) `earthquake_plate`, a geospatial task, gets `routine-advisor`, `new-project`, `commit`, `llm-council`. `citation_check` gets `security-review`, `review-readiness`, `llm-council`. It is 0% rather than merely poor for a structural reason: the only skills carrying `activation:` metadata are ironclaw's own bundled 32, and **none of those is ever a benchmark task's expected skill**. So every host pick is necessarily wrong, and it fills all four activation slots with them, displacing the skills the task actually needed. Keyword scoring did not rank badly here -- it had nothing correct available to rank. Against the model-decided path measured on the same catalog: 22 of 23 activations correct (95.7%). That is the before/after on the metric this epic is named after, and it was hidden because the two paths are observed through different channels. Assertions are upper bounds rather than equalities, so catalog drift will not fail this spuriously while a return to host-side picking still trips it. Needs the benchmarks checkout for its corpus; skips cleanly when absent, and `NEARAI_BENCH_ROOT` overrides the location. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): pin that a skill's scripts cannot execute under multi-tenant hosting #6745 lets a skill ship `scripts/*.py`, which is what makes a learned skill reusable rather than a prose description. @serrrfirat asked the right question on the epic: "What if it's a malicious script and we run it on host and ggs." The answer today is that a multi-tenant agent has nothing to run it WITH. `builtin.shell` is the only process-port-backed builtin, and it is removed from the capability package outright when the resolved process backend cannot execute, so the script sits inert in the skill store with no tool able to invoke it. That guarantee was a chain of three unasserted inferences -- `HostedMultiTenant` → `RuntimeProfile::SecureDefault` → `ProcessBackendKind::None` → shell removed. Every link is correct and none was pinned, so changing any one of them would silently enable script execution for every tenant. This asserts the property directly. The second test is the anti-vacuity half: execution-capable backends MUST still expose `builtin.shell`. Without it the first assertion would pass just as happily if the capability were renamed or dropped everywhere, proving nothing about the multi-tenant case. `TenantSandbox` is deliberately asserted as execution-CAPABLE rather than blocked, because it is @henrypark133's sandbox work: `crates/ironclaw_process_sandbox` is already a complete Docker backend (`--cap-drop ALL`, `no-new-privileges`, `readonly_rootfs`, `--network none`, non-root uid) with no non-test caller. When it is wired, multi-tenant execution becomes safe *because it is sandboxed*, and the first assertion should be revisited rather than deleted. Until then composition refuses a policy requesting `TenantSandbox` without a port (`MissingTenantSandboxProcessPort`), so the unsafe combination cannot be configured. Scope note, unchanged: execution is gated, INSTALLATION is not. A multi-tenant agent can still write `scripts/*.py` into its own store -- inert, but present. Gating the install path needs a profile flag plumbed through static schema resolution and is documented on the epic rather than half-implemented. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): reconcile the install-rejection test with #6745, which allows content + files `builtin_skill_install_rejects_hidden_url_install_fields` asserted that a `content` + `files` install is refused. That expectation came from `main`; #6745 deliberately changed the handler to accept it, and the merge left the two disagreeing -- so the test failed for the right reason and against the intended behaviour. Rejecting `content` + `files` WAS the bug. An agent attaching `scripts/analyze.py` had its entire install refused, which is why 0 of 27 agent-authored skills shipped a resource file while 18 of 31 human-curated ones do. A skill that cannot carry a script is a prose description, and reusing it means re-deriving the method every time. Removed that case; kept `source` and `source_url`. Those are set by the URL-fetch path to record provenance, so accepting them on a direct install would let an agent label its own output as fetched from a trusted URL. Renamed to `builtin_skill_install_rejects_forged_provenance_fields`, which is what the test now checks, and the doc comment records why the `files` case was removed so a future reader does not restore it as an oversight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): the descriptor lint must not refuse to write a skill the agent authored The lint gated `parse_distillation` and `parse_refinement`, so a learned skill failing it was never written. Run against the skills agents actually wrote during the self-creation runs: 26 agent-authored skills 19 would be REFUSED by the write gate 15 have no description whatsoever **Scope, stated precisely.** The lint sits only on the DISTILLATION path — ironclaw's automatic skill-learning. It does NOT sit on `skill_install`/`skill_manage`, which is what the benchmark's `_selfcreate` tasks use ("you MUST save a reusable skill"), so those 26 skills were written through an unlinted path and the benchmark's self-creation arm would not have broken. What the 19-of-26 figure shows is what distillation WOULD refuse when handed output of the same shape as real agent-authored skills. That is an inference about the product's automatic learning path, not a demonstrated break of the measured arm, and I am not going to overstate it. It matters anyway, because distillation is the mechanism by which the product learns skills without being told to. Refusing 3 of every 4 of them for a missing description would quietly disable it. The lint is now split by who pays for the defect. **Blocking** — `lint_skill_routing_metadata_blocking`, rules about declared activation TERMS. A generic keyword poisons routing for *other* skills: `coding` declares `file` and `change`, and on the reviewed baseline corpus it was being selected for security audits, QA plans and commit staging. That cost is paid by every later request, so refusing the write is proportionate. Only 4 of 26 authored skills declare keywords or tags at all, so this gates without blocking authoring while still stopping a model that tries to declare `file`. **Advisory** — `lint_skill_routing_metadata_advisory`, rules about the skill's own description. Empty or over-long hurts that skill's discoverability and nothing else, and refusing the write hurts it strictly more: the agent produced something that works and gets nothing. These now warn with the skill name attached, and are recorded rather than enforced. `lint_skill_routing_metadata` still returns both, so authoring-time UI and CI keep the full list; only the write gate narrowed. `agent_authored_skills_pass_the_lint.rs` pins it against the real corpus and skips cleanly when the stores are absent. It reports the advisory count too, so the 19 real description problems stay visible rather than silently tolerated — worth fixing in the authoring prompt, which is a different change from refusing the write. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): drive the PRODUCTION composition, and draw the boundary of disk-seeded validation Asked whether this work would hold in production. It is a fair question, and the answer turned out to be "we do not know yet", with a specific reason. This builds the real thing — `RebornCompositionProfile::Production` over libSQL under the hosted multi-tenant policy (scoped-virtual filesystem, brokered secrets, network deny, ask-always approvals, no process backend). No mounts, no env switches. It opens a conversation and completes a skill-execution turn, so skills ARE wired on the production path. But a skill written to `tenants/<t>/users/<u>/skills/` on the host filesystem activates **nothing** there: production resolves the skill store through the scoped-virtual filesystem, not the host disk. Measured, not inferred — explicit `$name` activation returned an empty activation set. That is the storage contract rather than a bug, and it has a consequence worth stating plainly: **every other validation of this work seeds skills on disk, so none of it exercises production's storage path.** The benchmark mounts `system/skills`; the local-dev tests write files. Both are real paths for their own profiles and neither is production's. The same class of mistake has now appeared four times in this work, each time as an artifact of the instrument rather than the system: a copy-out scanning disk while the store is libSQL; agent-authored skills escaping to the host's `~/.claude/skills`; a benchmark scoring only what the model requested while the host picked freely; and a phase-2 arm that swapped in 195 unrelated skills and called the result routing. This test exists so the next reader does not make it a fifth. Closing the gap needs a seam this crate does not expose: installing a skill into the production scoped-virtual store from a test, i.e. driving `builtin.skill_install` through the production capability port instead of writing bytes to a directory. That is infrastructure work with an owner other than this PR, so it is recorded rather than approximated. A test that seeds disk and asserts success would report production coverage it does not have, which is worse than no test. The assertion is therefore the negative one, with a message telling a future reader what to do if it ever flips: if a disk-seeded skill becomes visible, the storage contract changed and the disk-seeding validation elsewhere finally covers production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): drive the PRODUCTION composition and seed its DB-backed virtual filesystem Asked for a real production E2E rather than more local-dev validation. This is as far as it goes without infrastructure I do not own, and the boundary is stated rather than papered over. **Proven.** `RebornCompositionProfile::Production` over libSQL, under the hosted multi-tenant policy (scoped-virtual filesystem, brokered secrets, network deny, ask-always approvals, no process backend), builds, opens a conversation, and completes a skill-execution turn. Its DB-backed virtual filesystem then ACCEPTS a skill write at a tenant/user-scoped path via `LibSqlRootFilesystem::write_file`. That write is the new part, and it took two corrections to get right. The first version wrote to the host disk, which production does not read at all -- activation returned an empty set. The second wrote to the virtual filesystem before building the runtime and failed with `no such table: root_filesystem_entries`, because migrations run at build time. Both are recorded in the test so the next reader does not repeat them. **Not proven.** A skill written there is not DISCOVERED. Either the production bundle source scans a different scoped root, or it enumerates once at build time and does not observe a later write. That is answerable by whoever owns the production composition; trying paths until one passes would yield a test proving only that I found a path. The assertion is therefore the negative one, with instructions to invert it once discovery is wired -- at which point this becomes the end-to-end production claim the epic wants. A failing expectation a reader can act on beats a comment nobody reads. **Why this matters more than it looks.** Every other validation in this work seeds skills on disk: the benchmark mounts `system/skills`, the local-dev tests write files. Both are real paths for their own profiles and neither is production's. So the routing and self-creation numbers describe local-dev behaviour, and that limit belongs next to them rather than discovered later by someone else. The same class of error has appeared five times here, always the instrument rather than the system: a copy-out scanning disk while the store is libSQL; authored skills escaping to the host's `~/.claude/skills`; a benchmark counting only model requests while the host picked freely; a phase-2 arm that swapped in 195 unrelated skills and called it routing; and now disk-seeding on a profile that reads a database. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): full production E2E — a skill in the DB-backed store is activatable, fresh and after restart The production claim, verified rather than inferred. `RebornCompositionProfile::Production` over libSQL under the hosted multi-tenant policy (scoped-virtual filesystem, brokered secrets, network deny, ask-always approvals, no process backend). No mounts, no env switches, no local-dev. Two tests: * a skill written into production's virtual filesystem is activatable by name in the same session; * and it is still activatable by a runtime BUILT AFTER it was written -- the realistic shape, since a tenant installs a skill in one session and uses it in a later one against the same database. Reaching this took three corrections, all mine, and each is recorded in the test because each produces a plausible-looking test that proves nothing: 1. **Seeded the host disk.** Production reads a scoped-virtual filesystem, so activation came back empty -- the skill was never in the store. Every other validation in this work seeds disk, which is why none of it spoke to production. 2. **Seeded before building.** Migrations create `root_filesystem_entries` at build time, so the write failed with `no such table`. 3. **Used `/tenants/<t>/users/<u>/skills`.** The real mount is `/projects/tenants/<t>/users/<u>/skills` (`scoped_skill_context_mount_view`). Without the `/projects` prefix the write lands where nothing scans, which is what made this look like an unanswerable infrastructure question rather than a wrong string. The restart test is what isolated (3) from a build-time-caching explanation: it failed too, which ruled out enumeration timing and left the path. Worth keeping for that reason alone. Consequence for the rest of the work: the routing and self-creation numbers were measured on local-dev, and this establishes that the mechanism they exercise is reachable on production with the same activation contract. That is the gap this PR previously documented as open, now closed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): read skills from the DB-backed tree the writer uses — closes #7168 An agent installs a skill, gets `{"installed":true}`, sees it in `skill_list` for the rest of the session, and it is then gone forever: absent from Settings → Skills, unactivatable in every later conversation. Reproduced by hand on the WebUI, not in a benchmark. The reader and the writer were pointed at different trees: | side | function | `/skills` resolves to | backend | |---|---|---|---| | write (`skill_install`) | `production_skill_management_mount_view` | `/tenants/{t}/users/{u}/skills` | **database** | | read (discovery) | `scoped_skill_context_mount_view` | `/projects/tenants/{t}/users/{u}/skills` | **host disk** | `mount_database_roots` routes `/tenants` to the database; `mount_host_disk_roots` routes `/projects` to the host disk. So the install landed in the DB and discovery listed the disk. Nothing reports this: the install returns success, `skill_list` reflects the writer's own view within the session, and the failure surfaces only as "the model ignored its own skill" — which reads as a model-quality problem. Adds `production_skill_context_mount_view`, the read-side mirror of the write view, and wires it on the production path. Skills now live entirely in the virtual filesystem, database-backed, which is also the only coherent answer under hosted multi-tenancy: a host-disk path for tenant skill data is wrong on its own terms when there is no host disk for a tenant. `/system/skills` stays where the writer leaves it so bundled skills keep resolving. Moving that tree into the database too is the remaining step to skills being wholly DB-backed, and it needs bundled seeding to write into the database at boot rather than ship on disk. Guarded by `production_skill_read_and_write_mounts_resolve_to_the_same_tree`, which resolves a probe path through both views and fails naming the divergence. It is a mount comparison rather than an install-then-list round trip on purpose: the two views are the entire bug surface, it runs in milliseconds, and a round trip would report "skill not found" without saying why. A second assertion pins the tree to `/tenants/`, so a future change cannot quietly move skills back onto disk. The two existing production E2E tests failed on this commit until their seed paths moved from `/projects/tenants/...` to `/tenants/...`, which is the evidence the read tree actually moved rather than the assertion being tautological. Both now pass fresh and after restart. Also drops a stale assertion requiring a `names` property on the `skill_activate` schema; that input became a single `skill` string earlier in this PR. cargo test: ironclaw_reborn_composition 554, ironclaw_skills 181, production_runtime_skills 3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): one DB-backed tree for every skill mount, and never install a skill discovery skips Skill mounts were three views over two trees. The agent's in-run skill port and discovery both resolved `/skills` to `/projects/tenants/<t>/users/<u>/skills` on the host disk; Settings -> Skills and the product capabilities resolved it to `/tenants/...` in the database. So `skill_install` inside a turn wrote to the disk, Settings listed the database, and the skill was gone for good: `installed: true`, present in `skill_list` for the rest of that turn, absent from Settings, unactivatable in every later conversation (nearai/ironclaw#7168). Every skill mount now derives from a single `db_backed_skill_grants`, so a reader cannot drift from a writer. `/system/skills` stays on the host disk, where bundled seeding writes; the two disk-backed views are deleted rather than left available to wire in again. The first attempt at this fixed only the branch hosted multi-tenant Postgres takes. Local-dev, local-storage production and hosted single-tenant supply their own `workspace_filesystems`, so they kept the disk reader and stayed broken in exactly the reported way. Both readers are now pinned against the writer by separate tests. Two consequences handled here: - Skills that already sit on the host disk -- from the legacy backfill, or from any agent install before this change -- are imported into the database at boot. Copied, not moved, and a database entry always wins, so it is idempotent and a downgrade is not destructive. Without this an upgrade silently drops every skill the user had. - A manifest with an empty `description:` is repaired instead of persisted. Discovery rejects such a bundle (`InvalidSkillBundle`) and only warns, so the skill was accepted and then skipped forever. Measured with a real model: asked to save a reusable skill, it wrote frontmatter carrying `name:` alone. The description is derived from the skill's own opening prose. Repaired rather than refused because `SkillManagementCapabilityError` carries no message -- a refusal reaches the model as "the tool input could not be encoded", naming neither the field nor the fix, so the authoring turn would just be lost. Synthesized frontmatter for plain-markdown installs had the same defect and now carries a description too. Verified end to end on a live local-dev server with a real model: the agent authored a skill mid-turn, it appeared in Settings -> Skills, survived a server restart, and a fresh conversation activated it with zero `skipping skill bundle` warnings. Tests that seeded skills on the host disk were testing the tree the runtime no longer reads; they now seed and assert against the database. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): seed built-in skills in production, and make skill files readable Two gaps found by driving the real product. 1. Hosted multi-tenant production shipped with ZERO built-in skills. The bundled seeder is only reachable from `bootstrap_standalone_host`, which the Postgres path does not run -- correctly, since it writes through a host-disk filesystem and a tenant there has no host disk. But `/system/skills` IS mounted on that path, to the database, and nothing ever wrote to it. Settings -> Skills read an empty root and said "No skills installed" while local-dev listed all 32, with nothing logged either way. Every helper in the seeder already took `&dyn RootFilesystem`; only the entry point was disk-bound. Extracted `ensure_bundled_reborn_skills_installed_in` and seeded through the production filesystem at boot. The marker, install lock, and stale-skill removal are unchanged, so it stays idempotent across boots and safe when several instances share one database. `create_dir_all` is now best-effort: it walks up to `/system`, which is not a known virtual root, so it can never succeed for a root that is itself a mount -- and `RootFilesystem::create_dir_all` is documented as deprecated anyway, because the entry plane infers directories from path prefixes. 2. Skill files were unreachable from the ordinary filesystem tools. Skill mounts are granted to the skill capabilities only, so `read_file` saw nothing but `workspace`. Observed on a real production turn: the model installed a skill, tried to read it back to verify it, got "does not resolve inside an available scoped root (available roots: workspace)", burned a tool call, and fell back to `skill_activate`. A parity gap, not just a poor error: in Claude Code a SKILL.md is a file, so models are trained to read it, and skills reference sibling files (`references/*.md`, `scripts/*.py`) that progressive disclosure expects the agent to open on demand -- dead ends without a readable path. The run's filesystem view now includes the skill roots READ-ONLY; writes stay exclusive to `skill_install`/`skill_update`, which validate the manifest. Also fixes two skill_learning refiner tests that were failing on this branch: the fixture's `keywords: [file, count]` trips the blocking generic-keyword lint, so refinement returned `UnusableRoutingMetadata` and the merge silently degraded to `KeepExisting`. Reporting `has_scripts`/`has_requirements` to the Skills page is NOT here -- it belongs with #6745, which is what lets an agent author a skill containing a script. Co-Authored-By: Claude Opus 5 (1M context) <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): e2e for skill self-creation across a restart and a second conversation The loop #6941 item 4 is about cannot be covered hermetically: it needs a live server, a real model, a restart, and two separate conversations. Every hermetic guard around it passed while the product was broken, because each tested a layer in isolation -- the mount views agreed in the shape the test constructed, the install reported success, discovery listed the tree the test had seeded. The failure only appears when the writer and reader are the real ones, chosen by the real composition, with a restart in between so nothing in memory carries the result. Phase A covers a prose skill: authored mid-turn, listed in Settings, stored in the database and NOT on the host disk, surviving a restart, and activated by a new conversation with no 'skipping skill bundle' warnings. Phase B covers a skill carrying a runnable script (the #6745 feature): the bundle holds scripts/*.py in the database, Settings reports has_scripts, and a new conversation activates it. B5 asserts the bundled script is actually EXECUTED, and is reported as a known gap rather than a failure unless E2E_REQUIRE_SCRIPT_EXEC=1. builtin.shell spawns a host process while the script exists only in the DB-backed virtual filesystem, so there is no path to run: the agent tries 'ls -la skills/<name>/<script>.py || echo NOT FOUND' and falls back to 'python3 -c <algorithm re-typed inline>'. Right answers, wrong mechanism, and it defeats the argument for shipping a script at all. Flip the env var when that is fixed and the gap becomes an assertion. Runs on its own port and IRONCLAW_REBORN_HOME so it never disturbs a dev server. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(test): create the e2e log dir before redirecting into it `nohup … > "$LOG_DIR/server-1.log"` fails silently when the directory does not exist, so the server never launched and the log that would have explained it was never created either -- the run reported "server failed to start" with an empty log, which reads like a product failure and was not one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: remove an agent-written artifact committed by mistake `tenants/reborn-cli/users/reborn-cli/convert_labs.py` is not source. An agent wrote it into the repository working tree during a live WebUI test and `git add -A` swept it into2a63accb4. It is also evidence of a real defect, now recorded on #6941: `builtin.shell` inherits the SERVER PROCESS's working directory, so a relative path in an agent's shell command lands wherever the operator launched `ironclaw serve` -- here, the checkout itself. The same run also created `skills/egfr-ckdepi/` and a `tenants/<t>/users/<u>/skills/` mirror at the repo root. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(test): stop reporting a failure when zero bundles were skipped `grep -c` exits 1 when the count is zero, so `|| echo 0` appended a second zero and the compare against "0" failed -- the healthy case reported "FAIL A6 0". Same shape of bug as the missing log directory: the test was wrong, not the product. Also records why B5's execution check is best-effort: display-preview subtitles redact paths, so `python3 <path>` cannot be attributed to the bundle from the preview alone. The inline `python3 -c` fallback is the reliable signal, which makes a false negative possible and a false positive not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(shell): default a command's working directory to the workspace, not the server's cwd `resolve_local_host_workdir` fell back to `std::env::current_dir()` when a caller named no workdir, so a shell command with a relative path ran wherever the operator launched `ironclaw serve`. Observed on a live local-dev server started from an ironclaw checkout. An agent asked to save a skill wrote into the REPOSITORY: `skills/egfr-ckdepi/{SKILL.md,scripts/}`, a `tenants/<t>/users/<u>/skills/` mirror, and two loose `.py` files. One of them (`convert_labs.py`, from an earlier turn) was then swept into a commit by `git add -A` and had to be removed. In production the same behaviour puts agent writes in whatever directory the service was started from. It also made the agent's own tools disagree, which is the expensive part. `write_file` resolves through the scoped virtual filesystem; the shell resolved against this cwd. The same relative path therefore meant two different places: the model wrote SKILL.md and scripts/egfr.py, could not see them from the shell, ran `ls`/`glob`/`find /` (the last dying on the 10s shell cap) to work out why, concluded from `ls skills/` -- the checkout's own bundled-skill sources -- that skills live at the repo root, and rewrote everything there. Twelve of that turn's twenty-one tool calls went on this, and its final verification checked the copy in the repo rather than the installed bundle. Now: the `/workspace` alias by name, any other registered alias next, and the process cwd only when a caller registered no aliases at all -- the ambient-host case, which has no workspace to resolve against. An explicitly named workdir is unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Revert "fix(shell): default a command's working directory to the workspace, not the server's cwd" This reverts commita81cba9de9. * chore: remove a second agent-written artifact swept in by git add -A `tenants/reborn-cli/users/reborn-cli/ckd_epi_egfr.py` was written into the repository by an agent during the e2e run and picked up by `git add -A` in0e58adff8, the same way `convert_labs.py` was in2a63accb4. Not a product defect. Local-dev sets `workspace_root = std::env::current_dir()` (runtime/mod.rs), so the agent's workspace IS the directory `ironclaw serve` was launched from -- deliberately, so it works on the project in front of you. Launching it from a checkout therefore puts agent writes in that checkout. The lesson is for the operator, not the code: run the dev server from a scratch directory, and stage files explicitly rather than with -A. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): run the e2e in a scratch workspace and prove the checkout is untouched Local-dev resolves `workspace_root` from the SERVER'S CURRENT DIRECTORY (`build_standalone_local_runtime_services_input`), so whatever directory `serve` starts in becomes the agent's workspace. The test launched the server from the repository root, which made the repository the workspace: agent writes landed among tracked files, and two agent-written scripts were swept into commits by `git add -A` before being removed again. That is the operator's mistake, not the product's -- production runs the service in its own directory. So the test now does too: `serve` is launched from a scratch workspace, and two checks close the loop. One diffs `git status --porcelain` before and after and fails if the checkout changed at all; the other prints what the agent actually wrote into its workspace, which is the useful signal for whether a skill's script can ever be run from there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): add a production-profile mode, where the whole filesystem is the database The local-dev shape is not production, and the difference is the source of most of the filesystem confusion in these traces: local-dev /projects and /projects/workspace are HOST DISK, and the workspace root is the server's own cwd. builtin.shell exists (LocalHost backend), so the agent has two namespaces -- virtual aliases and host paths -- that can disagree. production production_database_root_filesystem routes /tenants, /projects, /memory and /system/* to the DATABASE, so the workspace is DB-backed too and there is no host disk at all. HostedMultiTenant + SecureDefault resolves to ProcessBackendKind::None, which strips builtin.shell. One namespace, nothing to disagree with it. E2E_PROFILE=production writes the storage/policy config the production build fails closed without, recreates its database so 'still listed after a restart' cannot pass on stale rows, and reads bundle contents back out of Postgres rather than libSQL. B5 inverts in that mode: the question is not whether a skill's script executed -- it cannot, by policy -- but whether the shell was correctly withheld. A shell call under hosted multi-tenant would be a policy escape, which matters more than the script. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(test): write the production config after recreating the home, not before The production block wrote $HOME_DIR/config.toml and the run then did `rm -rf "$HOME_DIR"`, deleting it -- so the build failed closed with 'profile=production requires [storage] backend = "postgres"', which reads like a misconfiguration and was an ordering bug in the fixture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(coding): answer list_dir "/" with the roots, and say why a read-only root refused Two failures an agent hit on a production-profile server, both diagnostics rather than missing capability, and both making the traces unreadable. `list_dir "/"` failed with "path is not under an available scoped root (available roots: skills, system skills, tenant-shared skills, workspace)". The offending path renders BLANK because the safe-summary encoder maps `/` to a space, so the message named nothing. The agent was doing something reasonable -- asking what the filesystem contains before writing to it -- and the roots it wanted were already being computed for that very error. `/` now lists the mount aliases. `apply_patch` on a skill file failed with only "the tool was denied filesystem access". `/skills` is deliberately read-only for the filesystem tools -- writes go through `skill_install`/`skill_update`, which validate the manifest discovery requires -- but nothing said so, and the agent fell back to skill_remove + skill_install to edit its own skill. The denial now names the path, lists the writable roots, and mentions the skill tools ONLY for a skill root: an existing test caught the first version telling an agent to use `skill_update` for a read-only workspace, which is nonsense. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): stop a skill instructing execution where the environment forbids it A skill body that says "execute it with python3 scripts/egfr.py" is a trap under ProcessBackendKind::None: there is no shell and no interpreter, so the instruction cannot be followed. A model told to do something impossible does not stop -- it improvises. Measured on a production-profile server. Asked to apply its own eGFR skill, the agent activated it, read scripts/egfr.py three times, enumerated its tools and correctly concluded it had no way to execute anything, hand-expanded Taylor series for ln/exp, and then POSTed the patient's creatinine and age to api.mathjs.org -- three times, receiving 46.493475453297044 and 31.625515257780567, the numbers in its final answer. Correct results, obtained by shipping clinical values to a third-party service from a tenant runtime. So when no process backend exists, a skill body mentioning execution now carries an explicit note: the instruction cannot be followed here, apply the documented method directly from the skill text, and do not call an external service to compute. The last clause is the point, not decoration. Narrow by design: only bodies that actually mention execution (scripts/, python3, bash, ...) get the note. Appending it everywhere would spend context and train the model to skim past it. defaults to true, so no existing shape changes. Derived from the resolved policy in filesystem_skill_context_source, which means this needs no edit when the tenant sandbox lands: the moment HostedMultiTenant resolves to TenantSandbox instead of None, it flips to true and the note disappears. docs/skills/multi_tenant_enablement.md records the full enablement path -- what already works on multi-tenant, why execution is off, the flip points in order (this note, then giving a bundle a path the sandbox can reach, then making the e2e's B5 a hard assertion via E2E_REQUIRE_SCRIPT_EXEC=1, then re-deciding network egress), and the profile matrix. The e2e gains a single-tenant mode, which is the shape most deployments run: Postgres storage with the local-host runtime policy, so a real process backend and a host-disk workspace. E2E_PROFILE is now local-dev | single-tenant | multi-tenant, with 'production' kept as an alias for multi-tenant, and each shape recreates its own database. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(filesystem): report which virtual paths a host process can open There were three authorities on where a virtual path lives: the composite's mount table, DiskFilesystem's own mount_local table, and HostProcessPort's separate workdir_aliases list built at composition from a different value. They disagreed silently. Under hosted single-tenant the file tools resolve /workspace to <workspace_root>/tenants/<t>/users/<u> (per-caller scoping) while the shell's alias resolves the same string to <workspace_root>. So an agent wrote scripts/egfr.py with write_file, ran , and landed three directories above its own file. Neither side errored -- the script simply was not there, and the model gave up and re-typed the algorithm into . makes host reachability a property the filesystem reports, from the same table that serves the data: DiskFilesystem answers through the same joiner every read and write uses, database backends answer None by the trait default, and the composite routes by longest prefix exactly as it does for reads. is the load-bearing case. A skill bundle lives in the database, so no host process can ever open it, and a caller that asks now learns that instead of spawning against a path that does not exist. Also adds SkillBundleSource::list_skill_bundle_files (default empty, implemented for the filesystem source with file-count and depth caps) -- a bundle's files could be read one known path at a time but never enumerated, which is required to copy one somewhere a process can reach. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(skills): stage an activated skill's files where its own commands can run A skill body says `python3 scripts/egfr.py`. That is only meaningful from the skill's own directory, and the bundle lives in the database, so no host process could ever open it. Agents did not fail cleanly: one ran `cd /workspace && python3 scripts/egfr.py` against a path three directories from its own file, another invented a `tenants/<t>/users/<u>/skills/...` tree in its workspace, and both ended by re-typing the algorithm into `python3 -c` -- losing the exact thing a shipped script exists to preserve. On activation, a bundle's non-manifest files are now copied into `.skills/<name>/` in the caller's workspace and the body is told the directory to run them from. SKILL.md is not staged: it is already model context, and a second copy invites edits discovery never reads. The directory is DERIVED, not assumed. `/workspace` does not mean one thing: under per-caller scoping the file tools resolve it to `<root>/tenants/<t>/users/<u>` while the shell's alias -- registered once at composition, knowing nothing about callers -- resolves it to `<root>`. Assuming either spelling produces a path that works for one tool and silently misses for the other, which is the bug being fixed. So the stager asks the filesystem where the staged directory really is, asks where the shell's `/workspace` really is, and expresses one relative to the other via `host_path_for`. Gated on a writable workspace and on `process_execution_available`: hosted multi-tenant has neither a shell nor a writable workspace today, so nothing is staged there and a body promising execution keeps the "cannot execute processes" note. When the tenant sandbox lands, staged files sit under the workspace the sandbox already binds, so the path works there with no second mechanism. Every failure degrades rather than propagates -- no stager, no execution backend, an unreadable bundle, or a failed write all mean "no staged path", and the skill still activates with its instructions intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): express the staged path against the shell's workspace, and name workdir Two defects in the first staging attempt, both found by running it. The derived path was measured against the FILE TOOLS' workspace root, so under per-caller scoping the two per-caller segments cancelled and the note said `/workspace/.skills/<name>` -- which the shell resolves to <root>/.skills/<name>, missing tenants/<t>/users/<u> entirely. It is now measured against the root the shell's alias actually resolves to, which composition owns. And the note merely stated the directory. The model then ran the body's `python3 scripts/egfr.py` with no working directory at all and missed the file, exactly as before. The note now names the shell's `workdir` parameter and shows it in use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(shell): scope /workspace to the caller, as the file tools already do One alias named two directories in one process. `scoped_workspace_mount_view` resolves `/workspace` to `<root>/tenants/<t>/users/<u>`, while `HostProcessPort`'s alias list -- built once at composition, before any caller exists -- resolved it to `<root>`. So an agent wrote `scripts/egfr.py` through `write_file`, ran `python3 scripts/egfr.py` in the shell, and landed three directories above its own file. Neither side errored. It then re-typed the algorithm inline, and in another run invented a `tenants/<t>/users/<u>/` tree by hand trying to reconcile the two. The port already receives the caller's `ResourceScope` on every request, so it now derives the same subtree per request rather than depending on a composition-time value it cannot know. Only `/workspace` is narrowed; `/host` and the raw host-home aliases are ambient by construction and have no per-caller subtree. This affects every file an agent writes and then runs, not just skills. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): hermetic two-thread fixture -- author a scripted skill, then run it Distilled from ten live demo runs against real models. Every one failed somewhere in this sequence and no existing hermetic test caught any of them, because each tested a layer while the failures lived between layers: 1. skill_install reported success and the skill never appeared again -- writer and reader resolved /skills to different trees (#7168). 2. A manifest with no `description:` installed fine and was skipped by discovery forever, with only a warn. 3. Thread 2 activated the skill, read scripts/egfr.py, and could not execute it: the bundle lives in the database, so no host process can open it. 4. Deprived of execution, one agent POSTed the patient's creatinine to api.mathjs.org to do the arithmetic. 5. After staging landed, the path the model was TOLD still missed, because /workspace means <root>/tenants/<t>/users/<u> to the file tools and <root> to the shell. So the fixture asserts the chain rather than a layer. It installs through the real product capability, then TEARS DOWN AND REBUILDS the runtime over the same store -- that is what makes it a later conversation, and a fixture holding one runtime open cannot see #7168 at all. It then activates by name, locates the staged bundle, and runs `python3 scripts/egfr.py` from the staged directory exactly as the skill body instructs. The script prints a marker that re-derived arithmetic cannot produce, so "it ran" is not inferred from a log line. The staged bundle is found by SEARCH, not by assuming a path. That caught a real thing on the first run: standalone uses the shared workspace policy, so there is no tenants/<t>/users/<u> segment, and a fixture hardcoding either spelling would have tested the spelling instead of the mechanism. A second fixture covers the description case directly: a manifest carrying `name:` alone must still activate in a later conversation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): advertise the plain workspace path, not a doubled one The staged directory handed to the model was derived by measuring it against the shell's workspace root and expressing one relative to the other. That was correct while the shell and the file tools disagreed about `/workspace`, and became wrong the moment they were unified: it emitted `/workspace/tenants/<t>/users/<u>/.skills/<name>`, which BOTH tools then resolved beneath the per-caller root a second time. The doubled directory does not exist, so every shell command failed with `Failed to spawn command: No such file or directory (os error 2)` -- which reads like a missing interpreter and is not -- and `list_dir` on the same advertised path failed too. An agent following its own skill's instructions could run nothing, and went back to copying files by hand. Now simply `/workspace/.skills/<name>`, which is correct precisely because `HostProcessPort` applies the same caller scoping the mount view applies. Pinned where the string is produced, since the two-thread fixture cannot catch this class: staging writes through the caller's own view, so the bytes land correctly even when the advertised string is wrong. That limitation is now recorded in the fixture. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(coding): treat / as the workspace in every tool, not just list_dir `scoped_path_input` mapped `""` and `.` to the workspace but passed `/` through, and `ScopedPath::new("/")` rejects the bare root. The resulting summary rendered the offending path BLANK, because the safe-summary encoder maps `/` to a space: path is not under an available scoped root (available roots: skills, system skills, tenant-shared skills, workspace) Agents hit it constantly -- a leading-wildcard glob, or looking at the root to see what exists -- and were told nothing. `list_dir` was special-cased earlier; `glob`, `grep`, `read_file`, `write_file` and `apply_patch` all still failed this way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(coding): fixture the blank-path failure and the write-then-read round trip Two regressions from live demo traces, neither covered. `/` failing with `path is not under an available scoped root` -- the offending path rendering blank because the safe-summary encoder maps `/` to a space. Now table-driven over glob, grep and list_dir, so one tool cannot be fixed in isolation again while the others keep failing (which is exactly what happened: list_dir was special-cased and the rest were not). And the write-then-read round trip on a relative path, which is the invariant an agent depends on when it authors a script and then runs it -- the one that silently broke when `/workspace` meant two different directories to different tools. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(skills): report where a skill is stored and where it can be run Thread 1 of the demo spent most of its tool calls working out something the tools should have told it. After installing a skill carrying a script it tried to execute the file from `/skills/<name>` -- the read-only, database-backed store that no process can open -- then `ls`'d around, hand-copied the script into its workspace, ran the copy, compared the two, and closed by asserting that "the installed skill's script is directly executable from the skill root". That is false in every deployment, and it said it because nothing said otherwise. So `skill_install` and `skill_list` now report both places: `store_path_read_only` and, for a bundle carrying files, `runnable_path_after_activation`. Both tool descriptions state the rule once -- the skill root is never executable; bundled files become runnable in the workspace on activation. Not staged at install time: the skill capability holds the skill-management mount view and no workspace view, so staging there would mean writing outside the view that authorizes it. Activation is one call away and already stages, so the agent needs the path, not an earlier copy. `runnable_skill_dir` is the one place the spelling lives, and it carries a note that `bundle_staging` must agree with it -- if they drift, the symptom is an agent running a command in a directory that does not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): make the install-then-use ordering explicit instead of discoverable Reporting the runnable path was not enough. An agent installed a skill, took the path out of the install result, and read it immediately -- before activating -- so it got the generic "can't access your workspace file" twice, activated, and only then succeeded. Two wasted calls and a confusing trace, for an ordering the tools knew and did not state. Two changes, both at the point of the mistake: `skill_install` now returns `bundled_files_runnable` as a structure -- `requires_first` (skill_activate with this name), `then_at` (the path), and why -- rather than a field named `..._after_activation` that reads as a label instead of a precondition. And a miss under `.skills/<name>` now says so: "does not exist yet ... call skill_activate with name=<name> first, then read or run it from there", in both read_file and list_dir. A missing file anywhere else is still just a missing file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * style: clear the CI gates on the changed crates `cargo clippy --all --tests -- -D warnings` and the panic checker are merge gates, and both failed on this branch. All of it is mine. - `activation.rs` tested `is_some_and(...)` and then `expect`ed the same Option, so the invariant lived in a comment rather than the types. Bound by pattern instead, which the production panic gate accepts. - `bundle_staging` kept a `shell_workspace_root` field that went dead when the derived path was replaced by the plain workspace spelling; removed here, along with the parameter it was threaded through and the composition constant that fed it. - Needless borrows, `sort_by` -> `sort_by_key`, `&[x.clone()]` -> `slice::from_ref`, a single-element `for` loop, an orphaned doc comment left behind by an earlier deletion, and four `pub` items in `ironclaw_threads::contract` that nothing outside the crate uses. - Constant-valued `assert!`s moved into `const {}` blocks. Also verified: composition mass budget (6.48% of 23.98% ceiling), hermetic env guard, include_str/Docker COPY coverage, and no tracked files matching .gitignore. 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> * fix(skills): gate criteria activation on requirements, and address the review The Major finding first, because it is a real hole: `unmet_requirements_refusal` was wired into the explicit-mention loop and `select_named_skill_activations` but NOT into the criteria (keyword/regex) loop, so a skill declaring an unmet `requires.bins`/`env`/`config` still auto-activated "cleanly". That is the worse half of the two: a criteria selection is the one the user never asked for by name, so nothing at all connects the later shell failure back to the missing binary. Same gate, same message, now on both paths. Also from review: - The refusal message rendered `SkillTrust` with `{:?}`. It goes to the model, so it renders through `Display` (`installed`/`trusted`) -- which is also the spelling `ironclaw_skills` documents as the one that gates content exposure. Two tests asserting the debug spelling move with it. - The truncation warning fired on every prompt-context build. A catalog over the budget stays over it, so that repeated one line for the life of the process. Now warned when the hidden count CHANGES, which is the only new information; the model-visible hidden-count message stays unconditional. - Corrected a comment claiming ExplicitAndCriteria is the default (it is ExplicitOnly; `criteria_config()` opts in). And the caller-level coverage the reviewer asked for, which `crates/ironclaw_skills/AGENTS.md` now also requires for changes to skill-content exposure: three cases driven through `SkillActivationHandler::invoke` with a capturing result writer, asserting the PERSISTED payload -- clean activation, trust refusal, unknown name. The builder-level tests could not see the defect this contract exists to fix, which was a payload built correctly and then not delivered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * style: rustfmt the skill-reachability files `cargo fmt --all --check` was red on five files this branch touches. Formatting only; no behavior, no reordering beyond rustfmt's own import sort. 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> * test(skills): pin the criteria-path requirement gate, fix two lints The gate added in the previous commit changed production behavior with zero test movement, which is the signal that nothing covered it. Confirmed by removing it again: the new case fails at the body-disclosure assertion, i.e. the skill with the absent binary really did activate and its prompt really did reach the model. Asserted through `set_activation_observer` rather than a return value, because that is where the criteria path's feedback actually goes -- it is the seam the live projection consumes, so a refusal invisible there is invisible in the product. Two lints under `-D warnings`, both pre-existing on this branch: - `set_activation_observer` returns a `Result` that the new test dropped. - An orphaned doc comment for `criteria_config()` sat above `assert_no_skill_body_disclosed`, documenting the wrong function. Moved onto the function it describes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): clear this branch's CI gates and one stale assertion A test breakage that predates the merge: `standalone_skill_activate_tool_loads_ selected_skill_context` required the `skill_activate` schema to advertise a `names` property, which directly contradicts the `skill`-is-a-string assertion two lines above it. The schema was narrowed to one skill per call on this branch and this assertion was not updated. Inverted to pin the actual decision -- a legacy `names` array is still ACCEPTED by `parse_skill_activate_names` so an in-flight caller does not hard-fail, but advertising it is what invited the multi-skill calls that produced every wrong activation in the 29-run measurement. Gates: - `check_no_panics.py`: `plan.expect("checked above")` after an `is_some_and` guard is now bound by pattern. Equivalent today; only the pattern form stays correct if the condition is edited, which is why the gate flags the other. - clippy `-D warnings`: a one-element `for` loop in `multi_tenant_skill_scripts_cannot_execute` (named as one backend instead, so a second non-executing backend needs its own case and message rather than a silent extra iteration), and two `sort_by` comparators that are `sort_by_key`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): address the review — one blocking path bug, one broken invariant **Blocking (ironloop, High).** `/tenant-shared/skills` resolved to `/tenants/<t>/tenant-shared/skills`, repeating the alias inside the target. Tenant-shared state lives under `/tenants/<t>/shared` — that is what `invocation_mount_view` resolves the alias to, and both siblings (`reborn-projects`, `reborn-identity`) follow it. Nothing writes or migrates the misspelled subtree, so a tenant that HAD shared skills would silently stop discovering them. Fixed, and pinned by `tenant_shared_skills_resolve_under_the_canonical_shared_subtree`, which asserts against the sibling layout rather than a hardcoded string. **The PR's own invariant was half true (2 findings, Medium).** "Every skill mount view derives from one DB-backed tree" — except a disk-backed `skill_management_mount_view()` survived and fed `PolicyApprovalLeaseTermsProvider`, in production, not just tests. So the lease terms a user approved for `skill_install` named `/projects/skills` while the install wrote to `/tenants/<t>/users/<u>/skills`. It is now derived per gate from the gate's own scope (`skill_mounts_for`), exactly as workspace mounts already were, and the disk-backed view is deleted along with the runtime field and test accessor that carried it. The `skill_mounts_for_test` helper became a scope-taking free function, so the harness asserts the same tree production mints. Real bugs: - `insert_frontmatter_description` took `lines().next()` as the `---` delimiter, but the parser tolerates leading blank lines. A skill starting with one had `description:` inserted above the delimiter, turning a repairable install into a failed parse. - `import_host_disk_skills_into_database` ran every boot and the disk copy is deliberately left behind, so a skill the user REMOVED came back at the next restart. Now gated on a one-shot marker under `/system/settings`: a migration, not a standing sync. - `is_skill_alias` matched every `*skills` alias, so a denied write to `/system/skills` or `/tenant-shared/skills` was told to use `skill_install`, which cannot write either. - `.skills` is now actually in `DEFAULT_EXCLUDED_DIRS`, which the staging module doc already claimed. Staged copies no longer show up in a workspace glob/grep. - Staged bundles are cached by caller + bundle + content hash. `body_context` runs on every turn's activation path and was re-walking, re-reading and re-writing an unchanged bundle each time. - `host_path_for` is removed from `RootFilesystem` and both `ScopedFilesystem` wrappers. It had no production consumer: staging writes through the filesystem, so the reachability probe it was added for is not needed. Its contract test goes with it. Docs that contradicted the code: the `/system/skills` note claimed host-disk-only while this PR seeds it into Postgres for multi-tenant (now describes both, and says the composite decides); `ensure_manifest_description` carried two stacked doc blocks, an earlier "refuse at the write" draft above the "repair, don't refuse" one that matches the code. `.gitignore` now covers `/tenants/`, the local-dev agent workspace — an agent-written script from a demo session is how the stray file in this PR's first comparison got there. Also: one arch-gate false positive from my own prose ("no notion of enumeration" tripped the concrete-extension check — reworded rather than allowlisted, keeping the register at zero), and two ratchet baselines LOWERED for the debt this PR deletes (frozen count 51->50, aggregate members 276->275), which #7147's own doc comment requires in the deleting PR. Verified: workspace `cargo check --all-targets` and `clippy -D warnings` clean; full `ironclaw_architecture` suite 37/37; reborn_composition lib 508 + integration suites green; skills/filesystem/extension_support/first_party_extension_ports/host_runtime/extension_host suites green; panic gate, hermetic-env, include-str and composition-budget gates OK. Not done, deliberately, and left for a follow-up: the seven "add a test for X" findings (bundle-enumeration bounds, staged_path traversal, runnable_dir literal duplication, NO_PROCESS_EXECUTION_NOTE end to end, the advertised install/list fields, derive_install_description escaping, update_skill blank-description repair) and the has_scripts one-stat-per-skill listing cost, which this PR's plan already recorded as separate. 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> * ci: correct the composition ceiling to the clean measured count 41,414 was measured on a working tree a bad `git stash pop` had polluted. The clean count is 41,096, and the 318-LOC difference read as unclaimed headroom — which the gate's own C11 self-test refuses, since a ceiling that far above the live count is the inert-ratchet failure the absolute bound exists to prevent. Corrected in both paired places, with the wrong figure and its cause recorded in the manifest rather than quietly overwritten. Gate green, 76/76 self-tests pass, both ratchets green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(skills): cut 346 lines of comment prose, and three duplicated blocks with it The narrative belongs in commit messages, where it already is. Kept the one-liners that stop a regression and the measurements that justify a constant; dropped the retellings. Three of the deletions were duplicated blocks, not verbosity: an earlier draft left stacked above its own replacement above `SKILL_LISTING_*` and `DEFAULT_SKILL_ACTIVATION`, and a copy-paste of the criteria-gate comment repeated verbatim. Each read as two competing explanations of one thing. Comment share of this stack's diff: 29% -> 25%, 1,970 -> 1,624 lines. Composition production LOC 41,096 -> 40,995, so `[gate].loc_ceiling` and its paired `COMPOSITION_ABSOLUTE_SRC_LOC` come DOWN by 101 rather than being inherited; the manifest note records both earlier figures and why each moved. No behavior touched: the only non-comment deletions in this diff are two assertion strings replaced with shorter ones, plus the two paired budget numbers. Verified: both threads of the demo fixture pass (`thread_one_authors_a_scripted_skill_and_thread_ two_executes_it` and `a_skill_installed_without_a_description_is_still_discoverable`, 2.26s), `fmt --check` clean, workspace `clippy -D warnings` clean, 49 test binaries green across skills / first_party_extension_ports / extension_support / architecture, and the budget (incl. its 76 self-tests), panic and hermetic-env gates pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): drive the demo's second thread through a mocked model and the real shell The existing two-thread fixture proves the staged bytes land somewhere runnable, and says in its own comment that it "cannot catch a wrong ADVERTISED path" -- it finds the script by walking the workspace and runs it with `std::process::Command`. That is the half the demo actually failed on: the string handed to the model was `/workspace/tenants/<t>/users/<u>/.skills/<name>`, the shell and the file tools resolved it a second time beneath the per-caller root, and every command died with `Failed to spawn command: No such file or directory`. This closes it. Thread 1 installs the skill with its script; thread 2 is a fresh runtime over the same store, and `SkillShellGateway` -- a mocked model -- does what the real one does: reads the activated body, PARSES the workdir out of the staged-files note rather than reconstructing it, and calls the real `builtin.shell` capability with that workdir. Everything between the model and the file is the production path: activation, staging, the mount views, the process port. Verified by reintroducing the bug rather than trusting a green run. With `runnable_dir` emitting the old per-caller spelling again, this fails with left: "/workspace/tenants/two-thread-tenant/users/two-thread-owner/.skills/egfr-calc" right: "/workspace/.skills/egfr-calc" and passes once the fix is restored. All three skill fixtures green in 2.67s; the file sits under `src/runtime/tests/`, so the composition mass count and its ceiling are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): narrow STAGED_SKILLS_DIRNAME after the crate move CI's clippy runs `--all-features`, which I had not; it flags what the plain run did not. After `bundle_staging` moved into `ironclaw_loop_host`, the constant was re-exported from the module but not from `lib.rs`, leaving it both unreachable-pub and unused. Nothing outside this crate reads it — the only other mention is a comment in the coding tools' config — so it is `pub(crate)` and off the module's export list. All three demo fixtures still pass (2.82s), `--all-features` clippy clean, fmt clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): bound the listing budget by the snippet cap it ships inside Two review findings, both real, both from the same blind spot: a constant was checked against what looked reasonable rather than against the limit downstream actually enforces. **The listing could take the runtime down (serrrfirat, High).** `LISTING_CHAR_BUDGET` was `512 * (250 + 64)` = 160,768 chars, and the listing ships as ONE model-visible snippet. `skill_context.rs` rejects a snippet over `LOOP_CONTEXT_SNIPPET_MODEL_CONTENT_MAX_BYTES` (65,536) with `ContextBudgetExceeded`, which is a hard error that fails the whole skill-context build — not a truncation. So past roughly 209 full-length entries a large catalog did not list fewer skills, it failed. The budget is now derived FROM that cap with headroom for the header and hidden-count note, and a `const` assert ties the two together so the old value cannot come back: restoring it fails the build outright with `evaluation panicked: the rendered listing must fit the single snippet it ships as`. A test asserts the rendered listing in BYTES (512 entries of multibyte descriptions, the worst case the enumeration cap allows) rather than in chars against the budget, because the cap is a byte cap. **`ActivationStrategy::Disabled` was inert (coderabbit, Major).** `criteria_enabled()` had no production caller at all — only its own unit test — so binding `Disabled` still ran keyword activation, exactly like `CriteriaOnly`. It is now the third gate on the criteria path, next to the global auto-activate switch and the selection mode. Verified: 635 + 54 + 4 tests green across loop_host, `--all-features` clippy clean (which is what CI runs), fmt clean. 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> * fix(skills): record the listing-budget const assert for the panic gate \`check_no_panics.py\` flags the \`assert!\` in the new \`const _: () = ...\` block. It cannot panic at runtime — rustc evaluates it, so restoring the old budget fails the BUILD — but the scanner reads the macro, not the const context. Suppressed with the inline \`// safety:\` rationale the tool documents, placed INSIDE the call's span where the scanner looks for it, rather than parked in the reviewed-invariant baseline: the baseline is for real runtime panics that were audited, and this is not one. (I tried the baseline first; it then correctly reported the entry as stale once the inline marker took effect.) All three panic checks pass: diff-scoped, reborn baseline (50 invariants, unchanged), and self-test. 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): cut the comment bloat on this layer Same pass as the layer below. The blocks that went were retellings: how the requirement-gating module came to be deleted and restored, three paragraphs on why the listing budget is the size it is, the full derivation of the active-skill cap. What stays is the measurement that justifies each number, because deleting those makes the constants look arbitrary. `gating.rs` lost the most (-24): its module doc explained the delete-and-restore history at length, which belongs in #6943's trail, not at the top of the file. Comments only. Production-target `--all-features` clippy clean, fmt clean, panic gate clean, 13 test binaries green across skills and loop_host. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: resolve conflicts with main and collapse the diff to this layer Main had advanced past #6938's merge, so the branch was carrying its own now-merged copy of the middle layer and diffing that against main. Merging current main drops it: 77 files/+7559 -> 44 files/+3152. Where a conflict was comment prose only, main's wording wins -- re-trimming text that already merged is churn this PR should not carry. Ours is kept only where the code differs: the bundle-staging body_context calls, the process_execution_available plumbing, and skills::gating, which main never received. approval/tests.rs takes main's file-split, with the skill-mounts argument dropped at its 7 call sites since skill_mounts_for now derives it per gate. docs/skills/agent_authored_bundles.md was resurrected by the merge and is removed; multi_tenant_enablement.md moves under docs/internal/skills/ for the publication boundary gate. Composition ratchet re-recorded 40_747 -> 41_149, ceiling and arch-test constant together. * test(arch): ratchet the struct-debt member baseline down to 274 skill_mounts_for derives the skill mount view from the gate, so the member that carried it is production-used rather than test-support-only. The gate requires the baseline to drop in the PR that removes the debt, otherwise the 1-member gap is untracked slack a later change could spend silently. * test(composition): seed production skills through the production namespace The merge took main's copy of these two seed paths, but they belong to main's scoped_skill_* mount views, which resolve under /projects. The production runtime reaches skills through db_backed_skill_grants, whose namespace is /tenants/<t>/users/<u>/skills — pinned by production_skill_management_mounts_use_production_namespace. Seeding at /projects/... wrote outside the tree the runtime scans, so both tests saw an empty activation set. * ci: map the skill self-creation e2e script in the Reborn test planner Repo-root scripts/ is deliberately not prefix-classified, so adding e2e-skill-self-creation.sh raised 'unmapped test or CI path' and failed Detect Reborn test scope, which skipped every downstream Reborn lane. It is referenced by no workflow and needs live model credentials no lane has, so it selects no lane -- the same decision already recorded for run-reborn-webui.sh. * chore: drop staged skill bundles from the tree and ignore .skills/ Activation stages a skill's bundle into the workspace, so running the agent from the repo left 16 staged files committed under ironclaw_cli/. They are build output of the feature this PR adds, never source, and they inflated the diff by ~1500 lines. * fix(skills): key the host-disk skill import per skill, not per store A skill dropped into the store after the first boot was never imported, and since skills read only from the database it stayed invisible — permanently, because the store-wide marker outlives every restart. The marker is not gratuitous: the disk copy survives a deletion made through the product, so an import that only checks "is it already in the database?" copies a removed skill straight back. Keying the marker per skill keeps the migration one-shot per skill, which picks up a newly appearing one on the next boot without resurrecting anything. Both directions are pinned, and they fail in opposite directions on the old code: a_skill_appearing_on_disk_after_the_first_import_is_still_imported fails before this change, an_imported_skill_deleted_from_the_database_is_not_resurrected fails if the marker is simply removed. * test(skills): seed user skills before boot, and cover bundle staging The three skill_activate cases seeded a user skill through the capability harness AFTER the group was built. That worked while skills were read from the host disk; they are read from the database tree now, and the store is migrated into it at boot, so a later write was never picked up and the run listed the system catalog with none of the user's own skills. Seeds them through the harness instead, which writes the store before the runtime boots — the ordering rule with_system_skill_fixture already documents, and the shape a real user's existing skills have. Both ids come from the group's already-resolved binding, since actor_user_id is an opaque hash that cannot be rebuilt from the profile's owner string. Also covers bundle_staging, which had one test for a 200-line file. The path-traversal guard in staged_path had none at all despite being the boundary that keeps a model-authored bundle inside its own directory; removing the guard now fails a_bundle_path_that_escapes_the_skill_directory_is_refused. The rest pin the best-effort contract: a bundle with nothing stageable advertises no workdir, and one unsafe entry does not cost the safe files. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>