fix(anolisa): exclude Python bytecode caches from adapter bundle digests - #2276
fix(anolisa): exclude Python bytecode caches from adapter bundle digests#2276ikunkun-sys wants to merge 1 commit into
Conversation
![qoderai[bot]](https://avatars.githubusercontent.com/in/1396158?s=60&v=4){kind=small}
There was a problem hiding this comment.
[P1] SourceVersionMatches 条件在旧收据 / 缺版本场景下的文案和引导略弱，建议在 Unknown 分支对“旧收据但当前版本已解析”给出更清晰的重新启用提示。
[P1] 组件更新后 adapter_actions 的版本比较路径依赖 StateStore 中的安装记录，需确认 RPM 原地升级等所有版本变更流程都在写 state 时同步更新，否则可能出现未提示重新启用的漏报。
[P2] Manager 侧将 SourceVersionMatches = Unknown 时统一把 Healthy 降级为 Unknown，可能在部分环境里让 adapter 永远无法达到 Healthy；建议只在“明确读取当前版本失败”的情况下执行该降级，旧收据或未声明版本的场景保留 driver 的 Healthy 判定。
🤖 Generated by Qoder • View workflow run
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a45a30b528
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 4ec67cfef3377b32e6d8125d9533825901fdeffc...a45a30b52834bff03b269a325f0e6da7cdb41edc
[P1] Do not use the component version as the adapter source identity
Location: src/anolisa/crates/anolisa-core/src/adapter/claim.rs:135
source_freshness returns Current whenever the two version strings match, while this PR removes
every resource-content check. After enable, editing a managed hook such as hooks/pii_text.py, or
installing an RPM revision that replaces adapter files without changing component.version, leaves
SourceVersionMatches=true and can keep adapter status healthy even though executable source
changed. The inverse also occurs: a component version bump with byte-identical adapter files is
marked degraded, although the unchanged-bundle update path intentionally emits no action.
Possible direction: persist and compare an adapter source revision, or a digest of immutable
delivered files that excludes declared runtime outputs. Add regression coverage proving
__pycache__ stays healthy while a same-version managed-file change degrades.
|
@kongche-jbw Thanks — responding to the three parts of your review: Same-version content changes (edited hook, RPM revision with unchanged Version bump with byte-identical adapter files now prompts re-enable. Intentional. The receipt asserts "this enable corresponds to component@X"; once the installed component is Y, that assertion is stale regardless of whether the adapter bytes moved — registration/config state may still differ, and re-enable is cheap and idempotent, re-recording the version. The system-update path answers a different question ("did this transaction change the bundle source") and intentionally stays byte-based; the two paths disagreeing on this edge is the semantic difference, not an inconsistency. Regression coverage: added in 36935e8 — |
9d5ae09 to
2393d34
Compare
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 4ec67cfef3377b32e6d8125d9533825901fdeffc...2393d34b9b5a9f965937b033e0adec52e264c181
[P1] Preserve an adapter-source revision for copied bundles
Location: src/anolisa/crates/anolisa-core/src/adapter/claim.rs:135
source_freshness treats equal component.version values as current and all drivers now stop
checking source contents. For copy-mode drivers, this is not covered by install-layer integrity:
cosh copies the bundle into its extension directory at enable. If an RPM release changes that
bundle without changing semantic component.version, rpm -V verifies the new package source
while the copied extension remains old; status still reports SourceVersionMatches=true and
Healthy.
Possible direction: persist a revision for immutable delivered adapter files (excluding runtime
outputs) or a package/source revision, and add a production-path test where same-version source
changes leave the copied destination stale.
[P1] Resolve the live version from the provenance-selected datadir
Location: src/anolisa/crates/anolisa-core/src/adapter/manager.rs:3558
current_contract_version searches datadir_roots in their original order. A supported system
layout orders the local datadir before the packaged datadir, and existing tests explicitly allow a
stale local contract alongside a snapshot whose provenance points at the package contract. In that
layout, a package update from 0.6 to 0.7 still returns the stale local 0.6; a 0.6 receipt stays
True and healthy. The new regression test uses a single datadir, so it does not cover this case.
Possible direction: prioritize the datadir selected by snapshot provenance/contract origin at both
enable and status, then add the two-root out-of-band update case.
2393d34 to
dc97c93
Compare
|
@kongche-jbw Responding to the two P1s from the second review round: [P1] Resolve the live version from the provenance-selected datadir — valid, fixed in 36935e8. [P1] Preserve an adapter-source revision for copied bundles — the gap is real; I'd like to scope it as a follow-up rather than fold it into this PR. You're right that my earlier install-layer argument doesn't cover the copy-mode case: after a same-version package re-release, The reason I'd still keep it out of this PR: a sound replacement needs per-file vocabulary, not a bolt-on. Re-digesting the source only for copy-mode drivers forks staleness semantics per driver, and any whole-tree digest that later gets reused for a link-mode driver resurrects #2252. The design that covers it properly is a manifest-aware revision of the declared immutable delivered files (runtime outputs excluded by declaration, in the spirit of RPM's |
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 1110be279bbc0dadfcd36a796f3a251070ced4cd...dc97c93210d147e87cc816008001690e8ab5e862
[P1] Preserve source identity when the version is unchanged
src/anolisa/crates/anolisa-core/src/adapter/claim.rs:135 treats matching version strings as
current after all drivers stop comparing delivered contents. For Cosh,
src/anolisa/crates/anolisa-core/src/adapter/cosh.rs:239 copies the bundle at enable, while status
only checks marker and manifest presence. A same-version RPM or raw re-release can therefore update
the source while the framework keeps executing old copied files, yet SourceVersionMatches=true
and the adapter stays healthy. This also drops the original issue's requirement that managed source
and manifest changes remain detectable.
Possible direction: persist a revision or digest for declared immutable adapter inputs and compare
the copy against the source, excluding declared runtime outputs. Add a production-path regression
where __pycache__ stays healthy but a same-version managed-file change degrades.
|
@kongche-jbw This round's P1 is the same copy-mode gap as the previous round — it is now tracked in #2279 (filed just before this review landed), with exactly the direction proposed here: a revision over declared immutable adapter inputs, the copy-vs-source comparison excluding declared runtime outputs, and the paired regressions ( On the new point about #2252's own requirement ("真正的源码、manifest 或受管理资源变化仍应被检测"): what stays detectable after this PR — every ANOLISA-mediated update (the version moves, so both |
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 1110be279bbc0dadfcd36a796f3a251070ced4cd...dc97c93210d147e87cc816008001690e8ab5e862
[P1] Preserve source identity for copied adapters
At src/anolisa/crates/anolisa-core/src/adapter/claim.rs:135, equal
component.version values are current after all drivers stop checking source contents.
Cosh copies the bundle only during enable at
src/anolisa/crates/anolisa-core/src/adapter/cosh.rs:239. A same-version RPM/raw
re-release can therefore replace the source while the framework continues running its old copy,
yet status remains healthy. Filing #2279 documents the regression but does not satisfy #2252's
requirement that managed source changes remain detectable in this change.
Possible direction: retain a revision for declared immutable inputs on copy-mode adapters and
compare source with the installed copy, excluding declared runtime outputs. Add a regression where
__pycache__ stays healthy while a same-version managed-file change degrades.
|
Accepted. I’m okay treating the copy-mode same-version source-revision gap as a non-blocking follow-up in #2279 for this PR. The issue captures the regression surface and the paired acceptance tests clearly. This acceptance is scoped to that gap. One P1 remains in the new live-version path. At The new two-root test covers a valid provenance sidecar. Pre-provenance snapshots remain ambiguous: content matching can select a stale local 0.6 contract before the packaged root, so an out-of-band packaged update to 0.7 is still masked. I consider these part of #2276’s new live-version detection and would address them here rather than under the copy-mode follow-up. |
dc97c93 to
36935e8
Compare
|
@kongche-jbw Implemented in 36935e8 — the copy-mode gap is now closed inside this PR, and with a stronger check than the pre-PR digest: Mechanism. Your two acceptance criteria, as tests:
#2279 narrows to what genuinely needs new vocabulary: sealed revisions for declared immutable inputs, drivers whose copies live in framework-internal locations ANOLISA cannot address (claude_code/openclaw/qoder CLI installs), and hermes skill directories (status resolves no manifest data today). |
|
Re-reviewed
One non-blocking residual remains under the already accepted #2279 follow-up: subset semantics cannot detect a managed source file being deleted, because the stale file is then only an “extra” in the executed copy. A local regression assertion confirms this still reports |
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: e5a212f9ab3e115e860698e059209e1c275201fd...36935e8ce11c029699e179d1ffa2eaffe66b7f0e
[P1] Detect files removed from copied bundles
Location: src/anolisa/crates/anolisa-core/src/adapter/util.rs:25
copy_divergence walks only the current source and ignores every extra path in the executed
copy. If a same-version release removes hooks/old.py, the Cosh or Hermes copy keeps executing
that stale file, yet the comparison is empty and status can remain Healthy. This leaves a common
direction of the copy-mode staleness gap undetected.
Possible direction: retain the managed delivered-file set or an equivalent source revision while
excluding declared runtime outputs. Add a production-path regression that removes a delivered
file after enable and expects status to degrade.
[P1] Do not treat malformed live contracts as absent
Location: src/anolisa/crates/anolisa-core/src/adapter/manager.rs:3568
The .ok() collapses ParseError and Io together with an unavailable live contract. With a
0.6 receipt/snapshot and a malformed or unreadable live 0.7 delivery contract, this falls back to
the snapshot's 0.6 and can report SourceVersionMatches=True and Healthy. A broken live source
must not be accepted as proof that the recorded version is current.
Possible direction: fall back only for ContractError::Unavailable; surface parse/read failures
as an unknown source version. Add the malformed-live-contract status regression.
[P1] Avoid choosing an ambiguous pre-provenance contract
Location: src/anolisa/crates/anolisa-core/src/adapter/manager.rs:3567
For a snapshot without a provenance sidecar, content matching can select the first local 0.6
contract when both local and packaged roots initially contain 0.6. If only the packaged contract
later moves to 0.7, that stale local root remains preferred and the 0.6 receipt stays Healthy. The
new two-root regression always writes provenance, so pre-upgrade snapshots remain uncovered.
Possible direction: treat a pre-provenance multi-root match as ambiguous or derive authority from
the installed backend. Add the same two-root out-of-band update test without the sidecar.
[P1] Compare Hermes against the files it actually copies
Location: src/anolisa/crates/anolisa-core/src/adapter/hermes.rs:315
Hermes enable deliberately omits the source skills/ subtree from the plugin copy, but status
passes the entire resource root to copy_divergence. A valid plugin adapter with bundled skills
therefore reports those intentionally separate files as missing and degrades immediately after a
successful enable.
Possible direction: compare the same source projection used by copy_bundle_excluding_skills
and verify skill resources separately. Add an enable-to-status test for a Hermes plugin that also
declares a bundled skill.
36935e8 to
d7995e3
Compare
ikunkun-sys
changed the title
|
Scope change (d7995e3): this PR has been reduced to the minimal mitigation for #2252 — bundle digests now skip The version-staleness redesign previously on this branch (and the copy-vs-source verification it grew during review) is withdrawn from this PR and parked as the formal follow-up under #2279 — the review findings raised here (out-of-band contract resolution, copy-mode removal direction, multi-root provenance, hermes skills projection) are all recorded there and remain valid against that design. Earlier review threads on this PR refer to the withdrawn revisions. Known residual of the stopgap, stated plainly: a planted |
|
Thanks for the scope reset. I confirmed that the previous version-staleness/copy-mode findings no longer apply to the current five-file diff and are now captured in #2279. One P1 remains, specific to the minimal digest change: [P1] Preserve compatibility with receipts sealed by the old digest semantics
I reproduced this through the production codex enable/status path by replacing the newly written receipt digest with the pre-PR digest of the same tree containing Please version the new digest semantics (for example, a new digest prefix/field) and handle legacy receipts explicitly. A regression should cover a pre-PR receipt sealed while bytecode already exists, followed by status under the new implementation. The explicitly accepted undigested-loadable- |
d7995e3 to
ca3bb68
Compare
|
@kongche-jbw Valid catch — and the population it hits is exactly the users who re-enabled as the #2252 workaround (their receipts sealed the tree with bytecode included). Fixed in ca3bb68. Mechanism: instead of versioning the digest format, the comparison accepts a match under either semantics — Regressions added:
Also updated the PR description accordingly. |
|
The dual-semantics fallback fixes only the case where the legacy-sealed bytecode remains byte-for-byte unchanged. It still misses the normal #2252 path after upgrade. A legacy receipt records source + cache A. If a later hook run regenerates only that
I added the missing step to The current test checks only an unchanged legacy tree before modifying README, so it does not cover cache regeneration. Because the legacy digest cannot be transformed back into the old bytecode-excluded digest, accepting either current-tree digest does not provide ongoing compatibility. This still needs explicit semantic versioning plus migration/re-enable handling (or another honest legacy transition), and the regression must include cache A -> cache B. |
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: e5a212f9ab3e115e860698e059209e1c275201fd...ca3bb68ed68da4b17880b59de1ac9b2d91172947
[P1] Persist legacy seals under the new digest semantics
At src/anolisa/crates/anolisa-core/src/adapter/util.rs:59, a successful legacy fallback is
returned only as true, so the receipt remains sealed with the old all-files digest. A first
post-upgrade status can therefore be Healthy, but if a later hook adds another .pyc or
regenerates one, neither the bytecode-excluding digest nor the legacy digest matches and
ResourceBundleMatches becomes false. This preserves the #2252 false positive for legacy
receipts after their next runtime cache change.
Possible direction: distinguish a legacy match and persist the new bytecode-excluding seal during
the upgrade/status migration. Add a production-path regression for legacy receipt -> successful
status/migration -> new .pyc -> Healthy, while a managed source edit still degrades.
Link-mode adapters (qwencode, codex) execute the resource root in place, so the first hook run writes hooks/__pycache__/*.pyc into the tree the enable-time digest sealed and flips a healthy adapter to degraded. Skip __pycache__/ directories and *.pyc files in every bundle digest — the narrow runtime-derived exclusion alibaba#2252 itself proposes. Manifest, hook sources, and all other managed files stay digested, so real tampering and same-version changes remain detectable. New seals carry an explicit semantics marker (sha256/2:), so future verdicts are deterministic. Legacy seals (bytecode included, e.g. the re-enable workaround population) match when the tree reproduces them under either semantics; when their bytecode caches have churned the old cache bytes are unrecoverable, so status reports Unknown with a re-enable hint instead of a false degrade, and one re-enable reseals with the marker. The broader staleness redesign is tracked in alibaba#2279. Closes alibaba#2252 Signed-off-by: ikunkun-sys <jiawa.syx@alibaba-inc.com>
ca3bb68 to
61aa45c
Compare
|
@kongche-jbw Your cache A → B extension of the repro is correct — ca3bb68 only rescued byte-identical caches. Fixed in 61aa45c, taking your semantic-versioning direction with one deliberate deviation on the migration half: What 61aa45c does.
Why not persist the reseal during status. Regressions now cover your exact flow: |
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: 39a6460871a0089c9e746862ae8eb3e45a4fe5f4...61aa45c7aa3365252f0adab8e6192e8ef3e7c36a
[P1] Mark OpenClaw and Hermes seals with the v2 prefix
At src/anolisa/crates/anolisa-core/src/adapter/openclaw.rs:2602 and
src/anolisa/crates/anolisa-core/src/adapter/hermes.rs:823, the private digest
helpers now exclude bytecode but still serialize the seal as bare sha256:.
verify_seal interprets that prefix as a pre-upgrade, bytecode-including seal.
For a newly enabled OpenClaw or Hermes adapter whose resource tree contains
__pycache__, changing any managed source file makes neither recomputed digest
match, so the verifier returns LegacyUndecidable instead of Changed. The
bundle condition becomes Unknown and stale_enabled_claims emits no re-enable
action, masking real drift while claiming that a new receipt came from an
earlier release.
Possible direction: use the shared digest_tree or emit sha256/2: from both
private helpers. Add OpenClaw and Hermes production-path regressions that assert
the new receipt prefix and that managed-file drift stays False/Changed when
__pycache__ is present.
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
PR 核心思路正确：在所有 bundle digest 路径排除 Python 字节码缓存，并通过语义版本化 seal（sha256/2:）确保旧收据的向后兼容。测试覆盖较为充分，但 OpenClaw 和 Hermes 的 enable 路径存在一个阻塞性 bug，导致新启用的适配器在有 __pycache__ 时立刻进入 Unknown 状态。
审查结论
request changes
详细意见
🔴 必须修改（阻塞合并）
-
[
src/anolisa/crates/anolisa-core/src/adapter/openclaw.rs私有digest_tree（约第 2590 行）] enable 写入 baresha256:前缀，verify_seal将其视为旧收据。OpenClaw 的私有digest_tree在排除字节码后仍然输出sha256:{hex}（旧语义前缀），而bundle_match_condition已改用super::util::verify_seal。这意味着：新 enable 时若资源树含__pycache__，写入的是旧格式 seal；首次 status 调用时verify_seal走旧收据分支，当前 bytecode-excluded 摘要 ≠ 旧 bytecode-included 摘要，且树中有__pycache__，因此返回LegacyUndecidable，状态变为 Unknown 并提示 re-enable——即全新 enable 的适配器立即自降级。修复方向：将私有digest_tree改为调用super::util::digest_tree（已携带sha256/2:前缀且排除字节码），或将输出格式改为format!("{}{:x}", super::util::SEAL_PREFIX, hasher.finalize())。这与kongche-jbw在最新 review（commit61aa45c）中指出的问题一致，本次审查予以确认并标注同一阻塞点。 -
[
src/anolisa/crates/anolisa-core/src/adapter/hermes.rs私有digest_tree（约第 810 行）] 同上，Hermes 私有digest_tree也仍输出 baresha256:前缀。修复方式同 OpenClaw：使用共享的super::util::digest_tree或显式输出SEAL_PREFIX。需要增加 OpenClaw 和 Hermes 的 enable-to-status 回归测试，断言新 seal 前缀为sha256/2:且__pycache__存在时ResourceBundleMatches=True。
🟡 建议修改（不阻塞但推荐）
-
[
src/anolisa/crates/anolisa-core/src/adapter/util.rs:59] 遗留收据成功匹配后未自动迁移 seal（kongche-jbw先前指出；本次 commit 仍未处理）。legacy fallback 返回SealVerdict::Matched后，receipt 中的bundle_digest仍保留旧的sha256:格式。下次 hook 运行写入新.pyc后，LegacyUndecidable就会出现，用户仍需手动 re-enable。建议在verify_seal中区分 legacy match 与 v2 match，或在调用方检测到 legacy match 时自动将 seal 升级写回 receipt，并补充 "legacy receipt → 首次成功 status → 新 .pyc 写入 → 仍 Healthy"的端到端回归测试。PR body 中将此问题 defer 到 #2279，接受程度视迭代周期而定。 -
[
src/anolisa/crates/anolisa-core/src/adapter/util.rs:65（verify_seal未识别前缀分支）] 对于完全无法识别的 seal 前缀，当前直接返回Some(SealVerdict::Changed)，而非None（Unknown）。这里 Changed 意味着"树被确定性地改变了"，但实际上我们只是无法解析 seal 格式，无法做出确定性判断。可考虑改为返回None或新增SealVerdict::UnrecognizedFormat，以区分"可证明改变"与"无法判断"。当前选择虽保守（不会假 Healthy），但会产生 false degraded。
🟢 值得肯定
is_python_bytecode谓词提取为共享公共函数，所有 digest 路径（包括digest_bundle_tree）统一调用，排除逻辑一致，无遗漏。sha256/2:语义版本化设计干净：新 seal 与旧 seal 在字节层面可区分，verify_seal的两路处理逻辑清晰。- 测试覆盖了核心路径：
digest_tree_ignores_python_bytecode_caches、verify_seal_handles_legacy_receipts_and_cache_churn、以及两个完整的 codex e2e 场景。遗留收据兼容路径有端到端覆盖，信心较高。 - PR body 对已知残留风险（planted
.pyc）和后续工作（#2279）有明确记录，不掩盖问题。
3 participants
Problem
Fixes #2252.
Link-mode adapters (qwencode, codex) execute the resource root in place, so the first hook run writes
hooks/__pycache__/*.pycinto the very tree the enable-time digest sealed.adapter statusre-hashes the tree, sees the bytecode caches, and flips a healthy adapter todegraded/resource bundle changed since enable— running the installed hooks is enough to break the adapter's own health report.Change
Minimal mitigation, following the narrow exclusion direction #2252 itself proposes ("让 bundle digest 明确排除可安全识别的运行时派生文件"):
is_python_bytecode(shared predicate):__pycache__/directories and stray*.pycfiles are skipped by every bundle digest —util::digest_tree(cosh/codex/claude-code/qoder/qwencode), the openclaw and hermes private copies, and the system-update source snapshot (digest_bundle_tree).sha256/2:<hex>and always compare under the semantics that wrote them, so verdicts stay deterministic under any bytecode churn. Legacy seals (baresha256:, bytecode included — notably the [anolisa] sec-core Hook 生成 __pycache__ 导致 bundle drift 误报 #2252 re-enable workaround population) are matched when the tree reproduces them under either semantics; on bytecode-free trees a mismatch is still decidablyChanged. When a legacy seal's caches have churned, the original cache bytes are unrecoverable, so all comparison sites (verify_seal) report a distinctUnknownwithre-enable to refresh the seal— never a false Degraded, never a fake Healthy. One re-enable reseals with the marker. Persisting the reseal automatically (status is read-only and lock-free) is recorded in [anolisa] Track adapter source revision for copy-mode adapters (same-version content changes) #2279.Known residual, deliberately accepted for the stopgap: a planted
.pycis loadable by CPython yet no longer digested. That, together with the broader staleness redesign (version-based receipts, copy-vs-source verification for copy-mode adapters), is tracked in #2279 as the formal follow-up.Testing
cargo test --workspacegreen; clippy/fmt clean.digest_tree_ignores_python_bytecode_caches(unit): bytecode caches don't change the digest; source edits still do.codex_status_stays_healthy_when_hooks_write_bytecode_caches(e2e): enable → hook run writes__pycache__into the executed root → status staysHealthywithResourceBundleMatches=True.codex_status_accepts_receipts_sealed_with_pre_exclusion_digests(e2e) +verify_seal_handles_legacy_receipts_and_cache_churn(unit): a legacy-sealed receipt staysHealthyon an unchanged tree; bytecode regeneration (cache A → B) yieldsUnknown+ re-enable hint, never a falseDegraded; after re-enable the v2 seal shrugs off further churn and a managed-file edit degrades.