fix(support): проверить parent CF и live round-trip для #76 - #459
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds vendor ChangesSupport capability preflight
Issue
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ExecuteGate
participant McpSession
participant PrivateWorkspace
participant ReportWriter
ExecuteGate->>PrivateWorkspace: Create isolated inputs and runtime data
ExecuteGate->>McpSession: Run support, mutation, build, and dump steps
McpSession-->>ExecuteGate: Return tool results and integrity evidence
ExecuteGate->>PrivateWorkspace: Restore source preimages and verify markers
ExecuteGate->>ReportWriter: Persist sanitized verification report
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
tests/dev/test_verify_issue_76_roundtrip.py (1)
683-688: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBind the integrity-probe failure to the probe, not to a call count.
flaky_digestfails on the third_stat_tree_digestcall. The verifier calls_stat_tree_digesttwice during setup and twice insideinspect_input_integrity. The test therefore depends on the exact number of setup calls. If a later change adds or removes one_stat_tree_digestcall before the probe, the injected failure moves to a different call site. The test can then fail for an unrelated reason, or pass without exercising the probe path.Trigger the failure from the probe path instead. One option is to fail on the path that the probe inspects after the session was created, or to set a flag when the session factory runs and fail only after that flag is set.
♻️ Proposed refactor to decouple the test from the call count
original_digest = verifier._stat_tree_digest - calls = 0 + session_started = False def flaky_digest(path): - nonlocal calls - calls += 1 - if calls >= 3: + if session_started: raise RuntimeError("synthetic integrity probe failure") return original_digest(path) class StartFailingSession(ScriptedSession): def start(self, _required_tools) -> None: + nonlocal session_started + session_started = True raise verifier.SourceError("synthetic MCP start failure")🤖 Prompt for 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. In `@tests/dev/test_verify_issue_76_roundtrip.py` around lines 683 - 688, Update flaky_digest in the roundtrip test to trigger the synthetic failure based on the integrity-probe context rather than the calls counter. Bind the failure to the specific post-session probe path, or set a flag when the session factory runs and fail only afterward, while preserving normal digest behavior before the probe.
🤖 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 `@scripts/dev/verify-issue-76-roundtrip.py`:
- Around line 2537-2540: Update the successful cleanup branch in the report
finalization flow to recompute containsProprietaryParentConfiguration after
temporary.cleanup() succeeds, matching the failure branch’s treatment. Set it
based on the post-cleanup filesystem state so the written report no longer
claims proprietary payload remains; preserve the existing retained and
cleanupSucceeded updates.
In `@tests/dev/test_verify_issue_76_roundtrip.py`:
- Around line 1004-1005: In the locked object-rule receipt test, remove the
duplicated unica.meta.edit assertion and add an assertion that
unica.runtime.execute is absent from client.calls, preserving the existing
checks that mutation tools are not reached.
---
Nitpick comments:
In `@tests/dev/test_verify_issue_76_roundtrip.py`:
- Around line 683-688: Update flaky_digest in the roundtrip test to trigger the
synthetic failure based on the integrity-probe context rather than the calls
counter. Bind the failure to the specific post-session probe path, or set a flag
when the session factory runs and fail only afterward, while preserving normal
digest behavior before the probe.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b1092ae-a27b-4c93-a71b-f11c981a4568
📒 Files selected for processing (5)
crates/unica-coder/src/application/mod.rscrates/unica-coder/src/infrastructure/native_operations/support.rscrates/unica-coder/src/infrastructure/support_guard.rsscripts/dev/verify-issue-76-roundtrip.pytests/dev/test_verify_issue_76_roundtrip.py
|
Проверка новых замечаний спустя 60 минут завершена. Три замечания CodeRabbit проверены и исправлены в d829156: устранён stale proprietary-payload receipt после успешного cleanup, fail-closed тест теперь доказывает отсутствие meta/code/runtime вызовов, а injection ошибки integrity probe привязан к фазе session start вместо числа digest-вызовов. RED для cleanup воспроизведён до исправления; после исправлений зелёные 38/38 issue-specific и 222/222 tests/dev, py_compile и diff-check. Новых комментариев Игоря/zeegin за контрольный интервал нет. |
zeegin
left a comment
zeegin
left a comment
There was a problem hiding this comment.
Проверил head d829156 против актуального main a3a3c5c: блокирующих замечаний к PR нет. Локально на merge-result прошли cargo fmt, строгий Clippy, полный Rust workspace и 222/222 tests/dev; публичный контракт и архитектурный sync не изменены, все review threads закрыты. Единственное падение полного Python CI воспроизводится на чистом текущем main в design-документах из #328 и этим PR не внесено.
2 participants
Что меняется
Refs #76.
PR снимает только blocker воспроизведения на живой ИБ, описанный Игорем в комментарии #5255993808, и добавляет fail-closed защиту реального support-профиля CPM:
unica.support.editперед изменениемParentConfigurations.binпроверяет exact parent.cfдля каждой записи поставщика: post-image capability-операции переводит все vendor-rule slots вlocked, где платформа требует matchingExt/ParentConfigurations/<Vendor>.cf;support_guardне может обойти этот prerequisite;.cf, membership каталога иParentConfigurations.bin; при ошибке.binостаётся byte-for-byte неизменным;fullRebuildсинхронизирует support state с одноразовой ИБ, после чего random metadata/BSL markers загружаются обычным build безfullRebuild; exact source preimages восстанавливаются до explicit full dump;Последующий комментарий #5258600630 корректно возвращает #76 в открытую работу. Этот PR использует
Refs #76, не закрывает issue и не считает оставшуюся работуnot planned.Независимое ревью
После отдельного findings-first ревью исправлены найденные регрессии и закреплены RED→GREEN тестами:
lockedи multi-vendor случаи;TMPDIR/TMP/TEMP, cache и IBCMD--dataнаходятся внутри evidence;Итог независимого re-review: блокирующих, major и minor findings не осталось.
Архитектурный слой
unica.support.editне изменены.ConfigXmlChanged; отказ не публикует cache events.scripts/dev/и не входит в MCP surface или runtime package.REQ-SAFETY-PREVIEW-BY-DEFAULT,REQ-SAFETY-NO-PARTIAL-WRITE,INV-SOURCE-BOUND-PREIMAGESиINV-SOURCE-WRITE-CONTAINMENT.Live evidence
Повторный прогон выполнен уже после независимого ревью и hardening environment на одноразовых копиях
CPM_3_3_3_DemoиCPM_3_3_3_xml/src:aarch64-apple-darwin); Windows live round-trip этим evidence не подтверждён;8.3.27.2214, builderIBCMD, приватные--dataи temp roots;0.12.0, SHA-256cc80809e0e2b7da874b9f948f8eebbe1f911f5f1baae059f48d8d64a2fd49528, совпадает с generated manifest;УправлениеХолдингом 3.3.3.27, SHA-256159770a19708e186d49172f96632d7a84d20ec77daa13eba359e6ab0850bbddb;privateCopiesOnly: true);status=pass,exitCode=0; temporary evidence удалён (cleanupSucceeded: true).Проверка
Результат: Rust lib
2748 passed, 2 ignored; support-related57 passed;tests/dev—222 passed; issue-specific —38 passed; architecture/design —49 passed; clippy, format, package verification и guardrails — pass.Что PR намеренно не решает
Это не
Closes #76.Поставляемая Unica всё ещё закрепляет
v8-runner 0.5.1на7ce1b062843d86644fe55741dbe0ee79f7ca767d. Upstream issue alkoleft/v8-runner-rust#30 закрыта, но реализационный PR #39 остаётся открытым и конфликтующим. В закреплённом runner по-прежнему нет:requested / processed / skipped / conflictedreceipts и post-hashes;Поэтому applied partial/incremental dump остаётся fail-closed, а #76 — открытой активной работой с blocker зависимого runner. Этот PR подтверждает безопасный full round-trip на живой ИБ и делает его воспроизводимым.
Summary by CodeRabbit
Bug Fixes
Verification