Repository navigation
fix(KEN-3993): prepare collection replacements in one core owner - #4985
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Blocking findings affect revision ownership and reporting or registration after post-write failures.
2 open findings
What changed in this PR
Moves collection preparation and compatibility checks into core, allowing atomic, order-independent collection replacement planning.
Changes:
- Adds a shared prepared collection plan and compatibility validation.
- Simplifies CLI collection installation to one plan and transaction.
- Updates subscription announcements, tests, architecture documentation, and changelog.
| File | Description |
|---|---|
crates/core/src/engine/ops/add.rs |
Introduces reusable prepared-addition planning. |
crates/core/src/engine/ops.rs |
Exposes prepared additions internally. |
crates/core/src/source_ops/collection.rs |
Prepares and plans whole collections. |
crates/core/src/source_ops/subscribe.rs |
Supports multi-subscription announcements. |
crates/core/src/source_ops.rs |
Exports the collection preparation API. |
crates/core/tests/collections.rs |
Tests replacement order and refusal atomicity. |
crates/cli/src/commands/add_collection.rs |
Consumes one core collection plan. |
docs/architecture/sources.md |
Documents collection compatibility guarantees. |
changelog.d/fixed/collection-replacements.md |
Records the consumer-facing fix. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
Author
Completed issues
QA metrics
Recommendations processedFixed in PR
Skipped
Fix rounds: 2. All accepted fixes passed review. |
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Oct 13, 2026
A collection checks and installs one prepared core manifest, installation record and reading selection. Its compatible replacements no longer retain historical minimum refusals while another repository plans first. Control: a disposable copy of base 135af66 compiled the new fixture with its driver adapted only to the shipped per-step core add calls. The compatible first-order install failed at CatalogNeedsEngine. The lane test passes both orders and proves byte-identical manifest and installation record on retained-reading refusal. Removed a_failed_step_still_names_what_it_wrote because the collection now applies one transactional plan. Its premise of an earlier collection step committing before a later step fails no longer exists.
Address PR review The subscription announcement owner accepts the complete subscription set, describes a shared manifest write once and retains a note for each subscription. Every single-subscription caller uses the same owner. Collection comments describe one prepared plan and one transactional apply. The change affects comments and human-readable preview text. Test assertions remain unchanged; no human wording is pinned. The subscription and CLI collection suites passed with preflight and document limits. Required merge-group CI remains pending in the bound validation receipt.
Address PR comments Collection snapshot reading stays in the existing core preparation owner. Snapshot holds apply only to explicitly requested members; automatic skills retain their following or held revisions. Direct add --hold still holds its complete expansion. Control: the regression test compiled against disposable base 9dd4c61 without driver adaptation. Its first collection row wrote Some(snapshot) for a following automatic skill and failed the typed assertion expecting None. The control built under its own root/tmp directory and the root was removed. The final collection suite passed, including old non-member holds, explicit skill membership and direct add --hold.
vanillagreen-fleet-lanes
Bot
force-pushed
the
ken-3993
branch
from
October 13, 2026 21:10
9074003 to
3c61668
Compare
Contributor
Author
Completed issues
Done when evidence
Recommendations processed
Downstream handoff
Close-out evidence
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completed issues
Requirements and proof
crates/core/tests/collections.rsinstalls each order through the core collection operation.crates/core/src/engine/ops/add.rsandcrates/core/src/source_ops/collection.rsown the prepared inputs.crates/cli/src/commands/add_collection.rsconsumes the core collection plan. The core export changes expose that operation.docs/architecture/sources.mdrecords the whole-collection check. The changelog records the consumer fix.Test plan
CatalogNeedsEngine.Review dispositions
Fixed stale CLI comments, the shared subscription description and the automatic-skill snapshot hold regression.
Declined the request to pin human-readable preview wording in tests. The core annotation owner joins the full subscription set and keeps each note. No program parses that wording. The repository test rules forbid those wording assertions.
Declined the shared CLI apply-count claim. The base first step used the same post-write error path and zero-count assignment. No newly armed producer or observed review-born symptom was found.
Restack validation
docs/architecture/sources.md. It keeps main's all-scope record-settlement rule and this branch's complete-collection preparation rule.3c616684c037c9edd401952219b710fe6ca47a0a. Exact-head required CI passed.Merge decision
Exit: 75
Expected head:
3c616684c037c9edd401952219b710fe6ca47a0amerge-route: queue cause=queue-occupiedGitHub queued the restacked head. The lane owns the queue wait. The overseer parks the lane during that wait and resumes it for close-out after merge.