mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
102e5e05b8
* 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> * feat(skills): show that an agent-authored skill carries scripts This PR is what lets an agent author a skill containing a script. The Skills page could not show that it did: `skill_info` hardcoded `has_requirements: false` and `has_scripts: false`, so a scripted skill was indistinguishable from a prose-only one. The WebUI has rendered the chips since #6194 and the wire fields have existed since #7002 -- only the server never populated them. It was not just agent-authored skills. `portfolio`, a BUNDLED skill, ships four Python scripts (`weekly_report.py`, `backtest_strategy.py`, `concentration_warning.py`, `alert_if_health_below.py`) and has always displayed as prose-only. - `SkillSummary::has_scripts`, from one stat on the bundle's sibling `scripts` path. Absent is the common case and is not an error, so only a genuine backend failure is logged -- a skill listing must never fail because a skill has no scripts. - The bundled-summary path reads it from the embedded bundle files, so `portfolio` reports correctly there too. - `has_requirements` comes from `requires_skills`, which was already on the summary. Verified on a live production server: 33 skills listed, `portfolio` the one reporting `has_scripts`, six reporting `has_requirements`. Note: skill scripts still cannot EXECUTE under hosted multi-tenant -- `HostedMultiTenant` + `SecureDefault` resolves to `ProcessBackendKind::None`, which strips `builtin.shell`. That is deliberate pending the tenant sandbox. So the chip tells a multi-tenant user their skill has scripts the agent can read but not run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): pin the direct-install input contract at the capability boundary Four cases covering what a caller may and may not put in a `builtin.skill_install` input, asserted through runtime dispatch rather than against whichever helper currently normalizes the input. The normalizer has already moved once (host runtime -> ironclaw_extension_support, WS3) and is about to be merged across that move again. Written so the same four pass on both sides: a resolution that quietly re-tightens the inline arm fails the first one instead of silently dropping the capability this PR adds. The dividing line these pin is provenance, not shape: - `content` + `files` installs, and the script lands on disk verbatim - `bytes_base64` works on the direct arm too, not only the rewritten URL payload - `content` + `files` + `source`/`source_url` is still refused whole - a `../..` bundle path is refused and writes nothing outside the skill directory Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): repair the post-merge build and close the review findings Build breakage from the merge: main added two `SkillSummary` construction sites (`skill_learning`, `lifecycle_product_service`) that this branch's new `has_scripts` field left incomplete. Both are fixtures whose assertions do not depend on it, so both set `false` with a note saying why. CI, all under `-D warnings`: - Four `result_read` cap items in `ironclaw_threads::contract` were `pub` in a private module (`unreachable_pub`). Nothing outside the crate reads them, so they are `pub(crate)`. - Four constant-value asserts (two in the same contract module, two in `activation_strategy`) move into `const {}` blocks, which is what they always meant: they are compile-time invariants, not runtime checks. Review findings: - The stale-default doc note (coderabbit, ironloop) was against `ecabcb5fe`, before `4951d76bb` reverted the flip. Docs and `DEFAULT_SKILL_INJECTION_MODE` both say `Listing` with `full` as the opt-in, so there is nothing left to correct. - The unset-env branch is now reachable from a test (coderabbit). `skill_injection_mode_from_env_value` takes the lookup's `Result`, so the product default can be asserted without `remove_var` racing every other test in this binary. Covered along with `full`, trimming/case, empty, unrecognized, and non-unicode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * 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> * ci: raise the composition mass ceiling for this layer's assembly wiring CI's "Check composition mass budget" step reds this branch by 62 LOC. Not because this branch is large: main sits only 54 LOC under the effective ceiling (40,499 + 150 tolerance against 40,595 observed), so the gate currently trips on any PR adding more than that to composition, and this one adds skill-summary and product-surface assembly. Raised to the measured 40,711 in both places the gate pairs — `[gate].loc_ceiling` in the manifest and `COMPOSITION_ABSOLUTE_SRC_LOC` in `reborn_restructure_baselines.rs`, since a second ratchet fails when they disagree, which is how it enforces recording the change in the PR that causes it. Measured with `check-composition-budget.sh --print`, set to current rather than padded, per the manifest's own protocol. A raise is a reviewed decision by that file's rules, not routine wiring, so it is flagged here and in the PR body rather than left in a diff. The next wave close should re-ratchet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(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> * ci: classify skills/ in the Reborn test planner `Detect Reborn test scope` failed this PR outright: Reborn PR test planner failed: unclassified pull-request path: skills/ceo-setup/SKILL.md The planner fails closed on any path it cannot attribute, and nothing had ever taught it about `skills/`. This PR is the first to edit the bundled catalog (seven SKILL.md descriptions shortened to satisfy the routing-metadata lint's 250-char cap), so it is the first to hit it. Routed to `ironclaw_extension_host` through the existing `EMBEDDED_ASSET_OWNERS` table, which is the mechanism for exactly this: its `build.rs` walks `skills/` and embeds the catalog as `EMBEDDED_REBORN_SKILL_{SUMMARIES,BUNDLES}_JSON`. A SKILL.md edit changes what every fresh tenant is seeded with, so it must select that crate's lanes. The table's staleness test then rejected the entry — "routes to ironclaw_extension_host, which embeds nothing from it" — and it was right to, given what it could see: `_crates_embedding` only recognises `include_str!`/`include_bytes!` literals, and the skills catalog is embedded by a build script, so the crate's only `include_str!` points at `OUT_DIR` and names no asset path. Taught the detector the second mechanism rather than weakening the check: a build script that reads a tree must declare `cargo:rerun-if-changed` for it, and that declaration plus the tree's name is what a build-time embed looks like from outside. 66/66 planner tests pass. Also fixes two `skill_learning` refiner fixtures whose `keywords: [file, count]` this PR's own blocking lint refuses — every token generic, which is the `coding`-declares-`file` case the rule exists to stop. Now specific multi-word terms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(skills): pin the listing header this PR actually ships This is what ejected #6938 from the merge queue at 20:20, and it was a real failure, not the conflict I first blamed: `reborn_integration_skill_activate` asserted main's listing-header text verbatim, including "at most four active skills total per run". This PR rewrites that header and raises the cap to eight, so the assertion could only fail. It never ran on the PR lane. `tests/integration/` is the Reborn integration tier, which the merge queue runs and `pull_request` does not — so every local and PR check was green while the queued merge commit failed. That is the same blind spot as the `--all --lib --bins` clippy lane: a whole class of check that only exists at merge time. The assertion now pins the shipped wording, keeping its intent (deliberate model-invoked selection): the `builtin.skill_activate` opener, "activate it FIRST … instead of your own default approach", the eight-skill cap, the do-not-activate-adjacent rule, and the ambiguous-name instruction. Verified: `reborn_integration_skill_activate` 20/20, plus golden_payload, greeting, surface_disclosure and tool_disclosure green — the other suites that read prompt text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(skills): follow main's ProcessBackendKind::TenantSandbox -> UserSandbox rename CI failed to COMPILE this PR's `multi_tenant_skill_scripts_cannot_execute` test: main renamed the variant (keeping `#[serde(alias = "tenant_sandbox")]` for wire compatibility, which does nothing for a Rust path). Five references updated, prose included. My local build was green on the same commit because GitHub tests `refs/pull/6938/merge` — this branch merged with CURRENT main — while I had only checked my branch against the main I last merged. Merged main again so local and CI look at the same tree. `cargo check --workspace --all-targets` clean; the test itself passes 2/2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>