mirror of
https://github.com/nearai/ironclaw.git
synced 2026-09-03 08:06:01 +08:00
c410ebbd36
* refactor(extensions): colocate packages under crates/extensions/ (WS2) Physical colocation only — no behaviour change. `crates/extensions/` now holds `ironclaw_extension_support/` (renamed from `ironclaw_first_party_extensions`) beside `packages/`, which carries 14 self-contained package directories: the twelve extension packages plus the two `[memory]` providers, each with its manifest, prompts, schemas, committed `wasm/` and the `wasm-src/` guest that produced it. `ironclaw_telegram_v2_adapter` is merged into `ironclaw_telegram_extension`, giving Telegram the one-crate-per-package shape Slack already had and the same four-crate contract-tier dependency set. 512 renames, 489 of them byte-identical; every content change in a moved test file is a path or crate-name repoint. Two silent registration traps this move created, both reproduced live before being fixed: * `classify-test-scope.sh` keys its arms on the crate directory basename, and package directories are named by extension identity, so all four moved package crates classified `has_reborn_tests=false` — the entire Reborn suite would have stopped running on a change to any of them, on a green PR. * A data-only package has no `Cargo.toml`, so five `wasm-src/` guests became the outermost manifest on their path and were promoted to first-class crates, putting ~12k lines of workspace-excluded, never-compiled guest code into the changed-coverage and composition-budget denominators. Both inventories now agree that a manifest declaring its own `[workspace]` table roots a different workspace and is not a crate of this one; `nested_workspace_root()` keeps those paths attributable so the fail-closed unattributable-path guard still means what it says. Adds the committed-`.wasm` freshness gate the tree never had: the rebuild job overwrites artifacts in the working tree before testing them, so a stale artifact shipped silently. It records a digest of each guest's sources rather than of the artifact, because the guest builds are not reproducible. The memory-provider enforcement is re-pointed to PROPOSAL §8.2's amended rule over both providers, with the three surviving dependents held as named shrink-only residue. The layer flip and binary-only linking do not land here: `host_runtime` constructs `NativeMemoryService` itself, so flipping first would create a kernel→products edge. WS3 owns the unblock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ci): make the classifier and the panic delta scan survive a move Three CI-surfaced defects, all the same shape — a gate that assumed the pre-move tree. `classify-test-scope.sh` REFUSED on `packages/github/manifest.toml`: a data-only package carries no `Cargo.toml` by design, so every file in it is attributable to no crate, and the fail-closed arm fired. The refusal was correct behaviour against an incomplete rule. A `package_asset_dir` predicate — anchored on the support crate so `packages/` is found as its sibling rather than by literal path — routes package data to the shared arm, which is exactly where it landed before the move as `first_party_extensions/assets/**`. The self-test had probed a package crate and a `wasm-src` guest but not a data-only asset; all three are probed now. `check_no_panics.py`'s delta scan had no rename detection, so a relocated file had no pre-image on its destination path and every one of its lines read as added: a pure `git mv` of `memory-native/src/repo/filesystem.rs` contributed 1,754 spurious added lines and failed the scan on six `unreachable!()` calls that are in the reviewed baseline. Both git invocations now pass `-M`, the added-line map is computed once for the whole range (pairing a rename needs both sides in one invocation), and the parser is split out and pinned by three tests: pure rename contributes nothing, rename-with-edits reports only the edited lines, creation still reports every line. `test-reborn-coverage.sh`'s M4 case had its fixture paths rewritten with the move while its assertions still named the old crate directories. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(checklist): record the rename-aware panic-scan fix * fix(ci): let the changed-coverage gate pair a crate-directory rename `diff_pathspecs()` built one `<crate>/src/**` pair per crate in the CURRENT inventory, which cannot name the SOURCE of a rename whose crate directory moved: `crates/ironclaw_memory_native/` is not in the inventory once the crate lives at `crates/extensions/packages/memory-native/`. `-M` therefore had nothing to pair against and every surviving line of a moved file landed in the denominator — measured on this PR, **33,026 added production lines** across 95 files, none of them an actual edit, against a 90% changed-line floor. Same defect `-M` was added for in #7005, one level up: there a file moved within a crate, here the crate itself moves. It fails in the expensive direction, because the floor then demands coverage for a pure `git mv`. The pathspec is now the crates root. Precision is unchanged — `parse_diff` already classifies each destination path with `is_production()` and drops everything else — so a non-move diff measures exactly what it measured before. On this PR the denominator falls to 2,357, of which 1,187 are test-path files the gate already excludes. Pinned by two tests: the pathspec contract, and an end-to-end regression that `git mv`s a whole crate into `extensions/packages/` and asserts zero added production lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(checklist): generalize the move-breaks-gates finding * fix(ci): teach the coverage gate that crate-root scaffolding is uninstrumentable Two classes of line that LLVM can never emit a coverage region for were not recognized, and each is enough on its own to fail a pure relocation, because one unclassified line defeats the `candidate_lines <= uninstrumentable_lines` escape: * **Inner attributes.** The classifier matched outer `#[...]` but not `#![...]`. Every crate root carries `#![forbid(unsafe_code)]`, so a moved `lib.rs` of pure `mod`/`pub use` declarations landed in "absent from coverage or contain no DA records" on the strength of that single line. Fixing it closes `packages/telegram/src/lib.rs` completely. * **`const`/`static` items.** Their initializers are compile-time and carry no region — `const MANIFEST: &str = include_str!(…)` least of all. Repointing an asset path is the entire Rust content of a package move, so four inventory modules had nothing but const declarations changed and read as "contributed no instrumented lines". Spans to the terminating `;` like `use`, so multi-line initializers are covered too. Both pinned by tests, including a negative assertion that a real function body stays measurable. What is left is exempted with per-site evidence rather than classified: nine `include_bytes!(concat!(…))` lines sitting inside `macro_rules!` BODIES across the same four modules. A macro body is template text — LLVM attributes the expansion to the expansion site, so those line numbers can never carry a DA record however many tests run. The expansions themselves are covered by `ironclaw_extension_support`'s 152 tests, and the changed content is a string literal inside `concat!`: a wrong path fails the build, which is stronger than a coverage hit, and `check-include-str-paths.sh` independently asserts all 119 include targets exist. Classifying macro template text correctly needs a real parser, not a lexer. Whole manifest re-validated through the gate's own loader: 83 exemptions, no stale path, no expired review date. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(telegram): cover the payload parser's fail-closed error paths The package move made `payload.rs` unpairable for `git diff -M` — the 2,166-line original split into four files under the 999-line budget and no destination clears the 50% rename threshold — so it reads as a new file with zero pre-existing exclusions, and 88 uncovered instrumented lines surfaced. They are almost all `PayloadParseError` construction sites and defensive branches on an untrusted-input boundary, which is worth testing rather than waiving. 18 tests in a new `src/tests/payload_errors.rs`, each driving a realistic malformed or hostile webhook body through BOTH `parse_telegram_update` and `normalize_telegram_update` so the two entry points cannot drift: missing update_id, the full chat-kind classification table, out-of-range UTF-16 mention windows, anonymous-admin replies, negative media-group message ids, attacker-authored media_group_id and display names, oversize and control-char message bodies, unsliceable bot_command entities, the leading-command rule, and every attachment slot. Changed-line coverage of `payload.rs`: 83.64% -> 95.91% (516/538). Production change is the four-line `#[path]` module declaration; no existing test was weakened or removed (89 -> 107 lib tests). Records one live defect rather than fixing it, since this is a move-only PR: Telegram voice notes and stickers hard-fail the ENTIRE update today. `collect_attachments` pairs voice with `audio/ogg` + `Voice` and sticker with `image/webp` + `Sticker`, but `validate_attachment_kind` (ironclaw_extension_contracts/src/external.rs:370) requires the kind to match the MIME base type unless it is `Other` — so both are rejected and the `?` aborts the parse. A well-formed voice message returns `InvalidExternalRef { kind: "attachment_descriptor" }` instead of being delivered. That is why those blocks had zero coverage. Pinned with an explicit "contract pin, not an endorsement" comment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>