正式版前调研+优化: mattpocock/skills 出处钉死 + 契约/schema 对账闸 + 六条复查落账 - #212
Conversation
…ct/schema drift gate
The mattpocock/skills internalization record claimed `unverified-upstream`
and `UNKNOWN-RESEARCH-ONLY` while carrying an ADR digest that no longer
matched the file it named -- and `repin` reported the tree clean, because
that pin lived only inside the `revision` string with no `{path, sha256}`
declaration to anchor it (the blind spot `declared_pins` documents about
itself, #133).
- Pin the upstream revision to 84fdeffd12f2ee307994d1eb6feb48173b6e0502
and record the verified MIT license (LICENSE blob f1dd2c09), replacing
the "verify before any use" obligations with the real MIT terms.
- Re-pin the ADR digest and declare it as `{path, sha256}` so `repin` can
resync it.
- Split the test contract: `assert_research_candidate` pins the
never-checked shape, so a verified record kept passing the unverified
contract. `assert_verified_research_candidate` asserts the opposite, and
the mattpocock test now asserts the one remaining gap is the security
review -- verified provenance is not a read of what the upstream skills
actually do.
Separately, every `(schema, type)` pair `registered_contract()` accepts must
now publish `orchestration/schemas/<id>.schema.json` (#206). The registry is
`match` arms, so the check reads the arms out of the source rather than
keeping a second hand-maintained list, and feeds every parsed pair back
through `registered_contract()` so a drifting parse fails instead of
silently checking nothing. Two false-green traps: fewer than 40 arms means
the scanner went blind (it caught the first implementation, which truncated
at the wrong `#[cfg(test)]` and saw only 32 arms), and the `AWAITING_SCHEMA`
exemption list may only shrink. Baseline: 41 registered contracts, 24 with
schemas, 4 markdown views, 8 under the native umbrella schema, 5 real gaps.
Refs #133, #206, #210
|
📋 At a glance ✅ Health gate: passed 🔎 More signals (2)🗺️ Change map flowchart LR
subgraph PR ["Changed in this PR (1 with dependents)"]
f_crates_code_intel_cli_src_artifact_ref_rs[".../src/artifact_ref.rs 🔥"]:::changed
end
f_crates_code_intel_cli_src_admissibility_rs[".../src/admissibility.rs"]
f_crates_code_intel_cli_src_artifact_ref_rs --> f_crates_code_intel_cli_src_admissibility_rs
f_crates_code_intel_cli_src_assistance_adapter_rs[".../src/assistance_adapter.rs"]
f_crates_code_intel_cli_src_artifact_ref_rs --> f_crates_code_intel_cli_src_assistance_adapter_rs
f_crates_code_intel_cli_src_builtin_provider_evidence_rs[".../src/builtin_provider_evidence.rs"]
f_crates_code_intel_cli_src_artifact_ref_rs --> f_crates_code_intel_cli_src_builtin_provider_evidence_rs
f_crates_code_intel_cli_src_capability_rs[".../src/capability.rs"]
f_crates_code_intel_cli_src_artifact_ref_rs --> f_crates_code_intel_cli_src_capability_rs
more(["+17 more dependents"])
PR --> more
classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Solid arrows: code that imports the changed files (21 direct dependents, from the last indexed snapshot). Dashed: history/tests. 🔥 Hotspots touched (2)
📊 See the full report for this PR |
|
Warning Review limit reached
Next review available in: 48 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds contract/schema registry checks, updates Matt Pocock Skills provenance to verified revision and license evidence, refreshes three integration digests, and records these changes and deferred items in the changelog. ChangesRegistry and provenance reconciliation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 17: Update the CHANGELOG entry’s command examples to use the compiled
CLI: replace every bare “repin” with “code-intel repin” and every “repin
--write” with “code-intel repin --write”, while leaving the surrounding
explanation unchanged.
In `@crates/code-intel-cli/src/artifact_ref.rs`:
- Around line 3254-3275: Replace the substring search in the native umbrella
coverage check within the registered-schema loop with parsed JSON Schema
inspection. Traverse the applicable oneOf structure and require each native
schema ID to appear as a branch reference, while preserving the existing
markdown handling and unpublished reporting.
- Around line 3162-3168: Update the validation around AWAITING_SCHEMA and its
checks near the registered-schema handling to compare exemptions against an
approved CI baseline or merge-base policy input, requiring issue metadata for
every approved entry. Reject newly added exemptions while preserving detection
of stale entries, and only update the CHANGELOG claim once this non-expansion
gate is enforced.
- Around line 3204-3212: Update the registry-arm scanning logic around the
literals collection and registered_contract validation so every non-literal
registry arm is accounted for, rather than silently omitted when its pattern is
not exactly two string literals. Prefer deriving the iterable registry data from
the same complete arm representation used by registered_contract, while
preserving the existing REPOSITORY_ITERATION_* handling and schema-publication
checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6aaddea6-2f75-489a-a6e0-8c735c5d0c26
📒 Files selected for processing (5)
CHANGELOG.mdcrates/code-intel-cli/src/artifact_ref.rscrates/code-intel-cli/tests/internalization_record.rsorchestration/integrations.jsonorchestration/internalization/mattpocock-skills.json
Code Intel change risk
Top signals
revspec: |
…ify umbrella coverage structurally Review found three real gaps in the contract/schema gate, one of which defeated the gate for a whole class of arms. - A match arm the scanner could not parse was silently skipped. A future `(NEW_SCHEMA, NEW_TYPE) =>` arm would produce no pair, leave the arm count healthy, and let that contract escape the schema check entirely -- the arm-count guard only catches wholesale blindness, not one dropped arm. Constant-form arms now resolve through the file's `const NAME: &str` items, and an arm that still will not resolve is a hard failure naming the family and the pattern. Proven by temporarily adding an arm over `&'static str` constants: the gate reports `uses a form the registry scanner cannot resolve`. This also removes the `REPOSITORY_ITERATION_*` special case, which is now just another resolved arm. - The family list is no longer written here either: it is read from `registered_contract`'s own dispatch body, so a new family reaches the scanner by the same edit that reaches production. Guarded by a minimum family count. - Native umbrella coverage was a substring search, so an id appearing in a title, an example, or an unrelated field counted as published. It now walks `oneOf` -> `$ref` -> `$defs/<target>/properties/schema/const`. Also: - Replace the per-contract `Path::exists` with one directory listing. A filesystem call inside a loop over 41 contracts is the finding class this repository is trying to burn down. - Correct an overclaim: `AWAITING_SCHEMA` entries are checked for going stale, but nothing stops the list from growing. Gating that needs a merge-base or CI-held baseline; noted against #210 rather than claimed. - CHANGELOG command examples lead with the compiled `code-intel` CLI. Refs #206, #210
1 participant
正式版发布前那波调研 + 优化。调研落成两条可核对的机制改动，其余五条复查落成 issue 与已知延后项清单——没有把「查过了」和「修好了」混在一起写。
一、mattpocock/skills 吸收：出处从「没查」变成钉住
内化记录里
revision: "unverified-upstream; local-adr-sha256:1431a4df…"，licenseUNKNOWN-RESEARCH-ONLY。实测：84fdeffd12f2ee307994d1eb6feb48173b6e0502（2026-08-06），license 实为 MIT（LICENSE blobf1dd2c09…），义务从「查清前禁止一切」换成真实的 MIT 保留声明义务；docs/adr/0006-*.md的实际 digest 是7a0e7da5…，而repin报clean。原因是 pin 只活在revision串里，旁边没有{path, sha256}声明，正是declared_pins模块文档写明的自身盲区。本轮补上声明并重钉，这枚 pin 进入repin --write的可解析面；assert_research_candidate钉的是「完全没查」的形状（unverified-upstream+UNKNOWN-RESEARCH-ONLY+ license/upstream-revision 两条 gap）。核实完还套这张断言，等于核实完继续通过未核实契约。新增assert_verified_research_candidate，并断言 mattpocock 记录只剩gap:mattpocock-skills:security-review一条——核实出处不等于读过上游 skill 到底干什么，这条继续挡住它离开 research。吸收的方法论侧（deletion test、「一个 adapter 是假 seam，两个才算」、expand→migrate→contract）没有写进代码，而是直接拿来跑了下面的复查，产出 #210 / #211 与 #206 的方向建议。
二、契约登记表 ↔ schema 发布面对账闸（#206 第一刀）
registered_contract()接受的每一对(schema, type)现在都必须有orchestration/schemas/<id>.schema.json。设计要点是不另立第二份清单：registry 就是那些
match臂，判据从artifact_ref.rs自己的源码把臂读出来，再把读出的每一对喂回registered_contract()核对——解析一旦跟真臂漂移，喂回去就不认，测试炸而不是静默检查空气。另写一份 id 数组等于制造下一个「写入方和校验方各维护一份现实」。两道假绿灯陷阱：
#[cfg(test)]截断源码，漏掉整个 native code-evidence 家族，只解析出 32 条；AWAITING_SCHEMA豁免表只减不增，表里 id 一旦有 schema 文件立刻红（已实测：临时补一个文件会得到now publishes a schema — delete it from AWAITING_SCHEMA）。markdown 与 native 两类豁免都是机械核对（前者要求
<x>.v1也已注册，后者要求伞状 schema 实测包含该 id），不是注释里的口头承诺。基线：41 个已注册契约 / 24 个已发布 schema / 4 个 markdown 视图 / 8 个走伞状 schema / 5 个真缺口（#210）。
零生产代码改动，不碰巨石拆分——按 #206 自己「先分开决策，再动代码」的约束办。source of truth 定在哪仍未拍板，建议与迁移方法写在 #206 的评论里。
三、复查产出（不在本 PR 里修）
content_contract.rs通过：6 个消费者，sha256_hex73 处 /reject_duplicate_json_keys50 处 /validate_artifact_ref_shape16 处调用，删掉它复杂度真会散回 150+ 个点。真接缝，不动。repin有机制（本 PR 期间当场抓到一次），participants 侧orchestration.rs::reconcile_production_registry有双向对账，没机制的只有契约身份。详见 #206 评论。main.rs97 行 1 函数，不是架构目标（churn 来自接线）。sentrux_gate.rs1816 /sentrux_analysis.rs1405 /capability_inventory.rs1275 行均破巨石线，已由.sentrux/baseline.json按身份豁免——是豁免，不是消失。[Unreleased] > Notes，见下。四、正式版已知延后项（CHANGELOG 已明写）
579 条 I/O-in-loop 静态发现未处理 ·
artifact_ref.rs1.85/10、3807 行仍是仓内最差且本轮只加检查未拆 · 33 个 god file 按身份豁免 · 5 个已注册契约无 schema（#210）· 内化记录 104 枚本地 digest pin 未用可解析形状声明，其中 37 枚已对不上任何在册文件而repin仍报 clean（#133）。五、期间实测到的三条既有 issue 的活样本
repin --write之后再编辑同一文件，pin 永久落出 stale 判据（它既不等于 HEAD 也不等于工作树），repin报 clean、--write也不写，最后是 PowerShell 合同测试抓到的。完整复现步骤已贴。ponytail.json用了repin可解析的形状。this_repository_has_no_resolved_import_cycles全量并行红、单跑绿。失败的恰是引用环判据，而按 bug(gate): 引用环判据把「引擎没跑起来」和「仓库有环」报成同一个红灯 #192 它把「引擎没跑起来」和「仓库有环」报成同一个红——这个红灯因此不可读。验证
cargo test全绿（646 + 274 + 275 + 273 + … 全部 test binary，exit 0）cargo fmt --all -- --check干净code-intel repin --repo .clean（且这次是真 clean——手工修正了 repin 盲区：连续两轮未提交编辑后 pin 孤儿化，repin 假报 clean #129 造成的那 3 处）code-intel sentrux gate .无退化：Quality 4423→4436，Coupling 67.09→65.47，Cycles 0→0，God files 33→33pwsh legacy/scripts/tests/test-atomic-capability-contract.ps1ok（96 outcome matrix cases，17 toolchain evidence capabilities）pwsh legacy/scripts/tests/test-project-management-support.ps1passedcode-intel change risk origin/main..HEAD：score 57，percentile 51，level low，testAsymmetry=falseRefs #129, #133, #178, #192, #206, #210, #211