Conversation
|
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
📝 WalkthroughWalkthroughThis PR adds an OpenSpec change for exclusive adapter targeting in Merge Risk: 🟡 Moderate · up to The PR plans exclusive adapter targeting that purges receipt-owned state from non-target adapters. Merge readiness is moderate because lock ordering and interruption recovery are not fully defined, which could permit stale ownership decisions or rematerialize files and MCP configuration into adapters intended to stay purged; the removal and verification task inconsistencies should also be clarified or explicitly accepted. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description includes complete Why, Details, and Verification sections. It clearly explains that the PR adds planning artifacts only, documents key design decisions and constraints, and states the verification performed. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (7 skipped: 7 unsupported.) ✨ 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 |
beb94b4 to
799e322
Compare
50b4b8e to
6c71c97
Compare
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
6c71c97 to
9cdacdf
Compare
799e322 to
71610cd
Compare
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
💡 Codex Reviewfacets/openspec/changes/filter-installation-by-adapter/design.md Lines 104 to 105 in beb94b4  Compose overlapping asset transitions before purging
When a target adapter and a purge adapter resolve an asset to the same physical path, the required pre-mutation planning can mark the target install as Line 56 in beb94b4 Do not auto-approve MCP declarations during bootstrap
When a repository facet introduces or changes an MCP declaration, or an existing native entry requires takeover, every routine AGENTS.md reference: scripts/AGENTS.md:L1-L2 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
4579b17 to
79bf40e
Compare
eXamadeus
changed the title
--adapter exclusive targeting spec for installation commands--adapter targeting for installation commands
![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: 79bf40ecba
ℹ️ 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".
|
|
||
| There is no deferred reconciliation state and no adapter-specific catch-up metadata. A later operation simply applies its own target/purge partition. An unfiltered operation targets every installed materialization-capable adapter and purges none. | ||
|
|
||
| The existing adapter-agnostic removal-refinement path SHALL be disabled globally rather than expanded to prove role-aware target/purge transitions. Every removal SHALL resolve complete remaining target state before mutation, using cached content when available and source/network resolution on cache miss. This intentionally changes the execution strategy for unfiltered removal: a cold-cache invocation without source availability SHALL fail unchanged instead of completing through receipt-only refinement. Target selection, placement postconditions, and receipt-driven deletion authority remain unchanged. |
There was a problem hiding this comment.
Disabling refinement here does not update the existing Removing a facet uninstalls it requirement in openspec/specs/installation/spec.md, which still requires fully tracked removal-only operations to refine locally without fetching and includes cold-cache offline-success scenarios. Applying this delta therefore leaves the normative specification requiring the exact path this design and task 4.5 remove, so implementation and acceptance tests cannot satisfy both; add a MODIFIED replacement for that requirement and its affected scenarios.
Useful? React with 👍 / 👎.
|
|
||
| ### Requirement: Unsupported MCP adapters are reported completely | ||
|
|
||
| When active MCP declarations exist and any target adapter declares no MCP support, the command SHALL fail before prompting or mutation. When the receipt owns an effective MCP identity requiring deletion from a purge adapter and that adapter cannot safely plan the removal, the command SHALL also fail before prompting or mutation. One error SHALL identify every affected adapter and SHALL give actionable remediation. |
There was a problem hiding this comment.
When there are no active MCP declarations but the receipt owns a stale server that must be deleted from a target adapter—for example after removing or omitting the last declaration—neither condition here requires that target to support planning the deletion; cleanup capability is checked only for purge adapters. The modified requirement previously covered receipt-owned identities needing deletion on every selected adapter, and without that gate the operation can drop the receipt claim while leaving the native entry orphaned. Require cleanup support for targets whenever receipt-owned MCP deletion is pending.
Useful? React with 👍 / 👎.
Greptile SummaryThe PR adds an OpenSpec plan for exclusive, repeatable
Confidence Score: 5/5The planning-only PR appears safe to merge. No blocking failure remains. Reviews (3): Last reviewed commit: "Plan exclusive `--adapter` targeting for..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Adds the OpenSpec change `filter-installation-by-adapter` — proposal, design, delta specs, and task plan. Planning artifacts only; no product code changes, so `facet add`, `facet install`, and `facet remove` behave exactly as before. Explicit targets are exclusive: each named adapter receives the complete desired project state and every other installed capable adapter is purged of receipt-owned assets and MCP entries in the same transaction. That is what keeps the machine-local receipt adapter-agnostic at schema `0.4` — no adapter dimension, no migration. The plan also retires the adapter-agnostic removal-refinement fast path globally rather than teaching it target and purge roles. Every removal resolves the complete remaining target state from cache or source before mutating, so a cold-cache removal without source access fails unchanged instead of completing receipt-only. Also gitignores the local OpenSpec archive and `adversarial/` review artifacts.
79bf40e to
9a66e9e
Compare
Dismissed because a newer commit was pushed; Greptile will re-review the current head.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a66e9e4e5
ℹ️ 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".
| - **WHEN** a user provides a flag that is not declared by a command without strict validation | ||
| - **THEN** the command handler SHALL NOT receive that flag |
There was a problem hiding this comment.
For passthrough commands such as modify, this replacement requires every undeclared flag to be omitted before the handler runs, even though modify depends on open-ended flags such as --adapter-<name> and the newly added scenario at lines 118–121 says those flags must continue to reach the command. Because a MODIFIED requirement replaces the existing contract, implementing lines 228–229 would silently break adapter-specific modifications; define an explicit passthrough policy that forwards undeclared flags while keeping installation commands strict.
Useful? React with 👍 / 👎.
|
|
||
| The target resolver SHALL operate on this source-independent request rather than reading argv itself. Today, absence of CLI values resolves to `all`. A future project configuration MAY provide an `exclusive` default request, while explicit CLI values override it, without changing the engine contract. | ||
|
|
||
| Semantic selection SHALL inspect the complete installed adapter set. A filtered invocation SHALL NOT launch the picker or implicitly install an adapter. Every requested target and every adapter that must be purged MUST load successfully and expose the capabilities required for its side of the transition. Any failure SHALL abort before the project lock or mutation and SHALL identify the affected adapter. |
There was a problem hiding this comment.
When another installation commits between this preflight and lock acquisition, the active declarations and receipt-owned MCP identities used to decide whether a target or purge adapter needs MCP support can change. For example, an MCP-incapable purge adapter can appear to have no work before the lock and then require cleanup after a concurrent commit; the implementation must either revalidate after locking, contradicting this requirement, or risk completing without the promised purge. Keep source-independent name/load checks here, but derive and validate work-sensitive capabilities from locked state before mutation.
Useful? React with 👍 / 👎.
| ### Requirement: Commands declare per-command flags | ||
|
|
||
| The system SHALL support per-command flag declarations on command definitions. The router SHALL parse per-command flags via the argument parser and pass the parsed values to command handlers alongside positional arguments. Flag declarations SHALL support boolean values, single string values, and repeatable string values. A command MAY require strict validation that rejects undeclared flags before its handler runs. |
There was a problem hiding this comment.
Because this is a MODIFIED replacement, enumerating only boolean, string, and repeatable values drops the canonical guarantee that a flag may declare a short alias which maps to the same handler value. Existing commands rely on that behavior (update --latest/-L and --interactive/-i), and the accompanying help replacement also omits alias rendering, so an implementation following the delta can remove those supported spellings. Preserve the short-alias parsing and help requirements while extending the definition with repeatable values and strictness.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Team
Run ID: 55ac1159-4842-4774-a133-5ae673f265b5
📒 Files selected for processing (7)
openspec/.gitignoreopenspec/changes/filter-installation-by-adapter/.openspec.yamlopenspec/changes/filter-installation-by-adapter/design.mdopenspec/changes/filter-installation-by-adapter/proposal.mdopenspec/changes/filter-installation-by-adapter/specs/cli/spec.mdopenspec/changes/filter-installation-by-adapter/specs/installation/spec.mdopenspec/changes/filter-installation-by-adapter/tasks.md
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
|
|
||
| There is no deferred reconciliation state and no adapter-specific catch-up metadata. A later operation simply applies its own target/purge partition. An unfiltered operation targets every installed materialization-capable adapter and purges none. | ||
|
|
||
| The existing adapter-agnostic removal-refinement path SHALL be disabled globally rather than expanded to prove role-aware target/purge transitions. Every removal SHALL resolve complete remaining target state before mutation, using cached content when available and source/network resolution on cache miss. This intentionally changes the execution strategy for unfiltered removal: a cold-cache invocation without source availability SHALL fail unchanged instead of completing through receipt-only refinement. Target selection, placement postconditions, and receipt-driven deletion authority remain unchanged. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exclude unfiltered undeclared removals from mandatory content resolution.
Line 144 requires every removal to resolve the complete remaining state and fetch on a cache miss. openspec/changes/filter-installation-by-adapter/specs/cli/spec.md Lines 340-347 and openspec/changes/filter-installation-by-adapter/specs/installation/spec.md Lines 451-463 require an all-undeclared remove without explicit targets to succeed with no state changes. A cold cache can make the literal Line 144 rule fail that required no-op.
After the under-lock commit proves that no requested name is declared and no explicit targets were supplied, retain adapter discovery and availability checks but skip content resolution and physical reconciliation.
Proposed clarification
-Every removal SHALL resolve complete remaining target state before mutation, using cached content when available and source/network resolution on cache miss.
+Every removal that changes desired state, or an explicit-target removal that must reconcile placement, SHALL resolve complete remaining target state before mutation.
+An all-undeclared removal without explicit targets SHALL complete after the required under-lock and adapter-availability checks without content resolution or physical mutation.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The existing adapter-agnostic removal-refinement path SHALL be disabled globally rather than expanded to prove role-aware target/purge transitions. Every removal SHALL resolve complete remaining target state before mutation, using cached content when available and source/network resolution on cache miss. This intentionally changes the execution strategy for unfiltered removal: a cold-cache invocation without source availability SHALL fail unchanged instead of completing through receipt-only refinement. Target selection, placement postconditions, and receipt-driven deletion authority remain unchanged. | |
| The existing adapter-agnostic removal-refinement path SHALL be disabled globally rather than expanded to prove role-aware target/purge transitions. Every removal that changes desired state, or an explicit-target removal that must reconcile placement, SHALL resolve complete remaining target state before mutation. An all-undeclared removal without explicit targets SHALL complete after the required under-lock and adapter-availability checks without content resolution or physical mutation. This intentionally changes the execution strategy for unfiltered removal: a cold-cache invocation without source availability SHALL fail unchanged instead of completing through receipt-only refinement. Target selection, placement postconditions, and receipt-driven deletion authority remain unchanged. |
| ## 4. Engine Scope and Asset Purge — Implementation | ||
|
|
||
| - [ ] 4.1 Pause: Switch model for implementation. | ||
| - [ ] 4.2 Implement: Introduce and export the tagged `InstallationAdapterTargets` contract and role-specific helper accessors, then migrate runAdd, runInstall, runRemove, and existing tests through an all-target compatibility helper without changing unfiltered behavior. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Clarify the unfiltered-removal exception.
Line 50 requires runRemove to preserve unfiltered behavior. Line 53 intentionally changes cold-cache unfiltered removal by requiring source or network resolution. Narrow the no-change requirement to add/install, or state the removal exception explicitly.
| - [ ] 10.5 Implement: Update the affected specification pages with target/purge terminology, adapter-agnostic receipt authority, transactional ordering, consent, frozen behavior, and the absence of persisted targets in manifest and lockfile. | ||
| - [ ] 10.6 Implement: Update the affected installation, troubleshooting, and custom-adapter guides with exclusive placement, purge failures, shared MCP visibility, and later unfiltered rematerialization; leave README, navigation, authoring docs, and SDK references unchanged after verification. | ||
| - [ ] 10.7 Implement: Add the required release changeset or equivalent package-release metadata without editing `docs/changelog/index.mdx`. | ||
| - [ ] 10.8 Verify: Run the complete `bun check` pipeline, fix formatting with `bun format` when required, and verify tests, types, lint, end-to-end behavior, documentation consistency, and unchanged receipt schema `0.4`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep formatting fixes outside Verify.
Line 116 places bun format inside a Verify step. The step protocol defines Verify as check-only and requires stopping after a failed check. Move formatting to an Implement task, then rerun step 10.8.
This branch has not been deployed
1 participant
Why
Projects with multiple installed adapters currently materialize every facet into every compatible adapter. There is no way to say "this project's Facet-managed state belongs in OpenCode only" for a given invocation without hand-deleting files afterwards.
This PR is the plan for that feature, not the feature: it adds the OpenSpec change
filter-installation-by-adapter(proposal, design, delta specs, task plan). No product code changes —facet add,facet install, andfacet removebehave exactly as they do today.Details
The central decision is that explicit
--adaptertargets are exclusive, not additive. Every named adapter receives the complete desired project state, and every other installed materialization-capable adapter is purged of all receipt-owned assets and MCP entries in the same atomic operation.That choice is what keeps the rest of the design small. Leaving non-target adapters untouched would require adapter-scoped receipt history so later filtered updates and removals could still prove what they were allowed to delete. Because no successful operation leaves project-owned state behind in a discoverable non-target adapter, the machine-local receipt stays adapter-agnostic at schema
0.4— no adapter dimension, no version bump, no migration.Constraints worth reviewing:
One deliberate behavioral regression outside the new flag: the adapter-agnostic removal-refinement fast path is retired globally rather than taught target/purge roles. Every removal will resolve complete remaining target state from cache or source before mutating, so a cold-cache
facet removewith no source access will fail unchanged instead of completing receipt-only. Design decision 7 records the trade-off.The task plan sequences this across ten phases: CLI parsing and flag strictness, engine scope migration and asset purge, MCP purge and frozen mode, scope reporting and failure remedies, then integration, documentation, and release metadata.
Verification
Nothing to run — the diff is planning artifacts plus two
.gitignoreentries. All four artifacts are complete perbun openspec status, and each went through independent adversarial authoring, comparison review, and reconciliation.Summary by CodeRabbit
New Features
--adapter <name>targeting across installation commands.Chores