fix: shrink default OpenCode install surface and gate hooks-runtime#2140
Conversation
📝 WalkthroughWalkthroughThis PR introduces default installation behavior for the ChangesOpenCode default profile and target exclusions
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/lib/install-manifests.js (1)
550-553: ⚡ Quick winUse immutable owner updates to match project JS rules.
This block mutates arrays via
push; switch to immutable updates with spread for guideline compliance.Proposed refactor
for (const exclusion of targetDefaultExclusions) { - const owners = excludedModuleOwners.get(exclusion.moduleId) || []; - owners.push(`${target} default`); - excludedModuleOwners.set(exclusion.moduleId, owners); + const owners = excludedModuleOwners.get(exclusion.moduleId) || []; + excludedModuleOwners.set(exclusion.moduleId, [...owners, `${target} default`]); }As per coding guidelines, “ALWAYS create new objects, NEVER mutate in place — return a new copy instead of modifying existing state” and “Use spread operator for immutable updates in TypeScript/JavaScript instead of direct mutation.”
🤖 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 `@scripts/lib/install-manifests.js` around lines 550 - 553, The code mutates the array returned from excludedModuleOwners via owners.push; instead create a new array when adding the owner to keep updates immutable: when iterating targetDefaultExclusions use const owners = [...(excludedModuleOwners.get(exclusion.moduleId) || []), `${target} default`]; then call excludedModuleOwners.set(exclusion.moduleId, owners) so you never mutate the original array; update the block referencing targetDefaultExclusions, excludedModuleOwners, exclusion.moduleId and target accordingly.
🤖 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/lib/install-manifests.js`:
- Around line 515-517: The condition computing shouldUseTargetDefaultProfile
currently ignores excludedComponentIds, so a request that only supplies
exclusions will be treated as no explicit selection; update the condition in the
declaration of shouldUseTargetDefaultProfile to also require
excludedComponentIds.length === 0 (i.e., include excludedComponentIds in the &&
chain) so default profile auto-selection only happens when there are no
requested, explicit, included, or excluded component IDs.
---
Nitpick comments:
In `@scripts/lib/install-manifests.js`:
- Around line 550-553: The code mutates the array returned from
excludedModuleOwners via owners.push; instead create a new array when adding the
owner to keep updates immutable: when iterating targetDefaultExclusions use
const owners = [...(excludedModuleOwners.get(exclusion.moduleId) || []),
`${target} default`]; then call excludedModuleOwners.set(exclusion.moduleId,
owners) so you never mutate the original array; update the block referencing
targetDefaultExclusions, excludedModuleOwners, exclusion.moduleId and target
accordingly.
🪄 Autofix (Beta)
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
Run ID: 1840fd8f-171e-4ea6-bff9-9147d232f6b1
📒 Files selected for processing (2)
manifests/install-profiles.jsonscripts/lib/install-manifests.js
| const shouldUseTargetDefaultProfile = !requestedProfileId | ||
| && explicitModuleIds.length === 0 | ||
| && includedComponentIds.length === 0; |
There was a problem hiding this comment.
Exclude-only input should not trigger default profile auto-selection.
shouldUseTargetDefaultProfile ignores excludedComponentIds, so a request that only passes exclusions can unexpectedly install the target default profile instead of failing as “no explicit selection.”
Proposed fix
const shouldUseTargetDefaultProfile = !requestedProfileId
&& explicitModuleIds.length === 0
- && includedComponentIds.length === 0;
+ && includedComponentIds.length === 0
+ && excludedComponentIds.length === 0;📝 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.
| const shouldUseTargetDefaultProfile = !requestedProfileId | |
| && explicitModuleIds.length === 0 | |
| && includedComponentIds.length === 0; | |
| const shouldUseTargetDefaultProfile = !requestedProfileId | |
| && explicitModuleIds.length === 0 | |
| && includedComponentIds.length === 0 | |
| && excludedComponentIds.length === 0; |
🤖 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 `@scripts/lib/install-manifests.js` around lines 515 - 517, The condition
computing shouldUseTargetDefaultProfile currently ignores excludedComponentIds,
so a request that only supplies exclusions will be treated as no explicit
selection; update the condition in the declaration of
shouldUseTargetDefaultProfile to also require excludedComponentIds.length === 0
(i.e., include excludedComponentIds in the && chain) so default profile
auto-selection only happens when there are no requested, explicit, included, or
excluded component IDs.
…ffaan-m#2140) Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
|
Thanks for the merge, @affaan-m. The slimmer default OpenCode install surface with gated hooks-runtime is a cleaner first-run. |
What Changed
A new
opencodeprofile inmanifests/install-profiles.jsonships onlycommands-core,platform-configs, andworkflow-quality, intentionally excludinghooks-runtime.scripts/lib/install-manifests.jsselects this profile as the default when an OpenCode install requests no explicit profile, modules, or components, and registershooks-runtimeas a target-default exclusion so it is dropped from the resolved plan with a warning naming the opt-in command./install.sh --target opencode --modules hooks-runtime. An explicit--modules hooks-runtimestill installs it.Why This Change
Issue #2079 reports that installing ECC into OpenCode Desktop with the full module set (including
hooks-runtime) makes the provider go silent after restart; deleting.opencoderestores it. The owner labeled it P1 and asked ECC to make the default OpenCode install smaller and gatehooks-runtimebehind an explicit opt-in, which is repo-side install ergonomics rather than an OpenCode change. Claude Code defaults are untouched.Testing Done
node tests/run-all.js)Verified the default OpenCode plan selects only the three safe modules and excludes
hooks-runtime, that explicit--modules hooks-runtimestill installs it, that the Claude default profile is unchanged, and that the plan output names the excluded module and opt-in command.Type of Change
fix:Bug fixfeat:New featurerefactor:Code refactoringdocs:Documentationtest:Testschore:Maintenance/toolingci:CI/CD changesSecurity & Quality Checklist
Documentation
Fixes #2079
Summary by cubic
Make the OpenCode default install smaller and disable
hooks-runtimeby default to prevent the post-restart silence reported in #2079. Adds a newopencodeprofile and a target-level exclusion with a clear opt-in path.opencodeprofile withcommands-core,platform-configs,workflow-quality.--target opencodeand no profile/modules/components are provided.hooks-runtimeby default; plan prints warning with opt-in: ./install.sh --target opencode --modules hooks-runtime--modules hooks-runtimestill installs it.Written for commit 31f3556. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
opencodeinstallation profile with default setup including core commands, platform configurations, and quality workflow.Chores
hooks-runtimemodule is now opt-in only via--modules hooks-runtimeflag.