feat: install prefers the plugin over hand-wired hooks (BRO-1929, v0.37.0)#96
Merged
Merged
Conversation
…oks (BRO-1929) When bstack is present as a skills-dir plugin, bootstrap/onboard/repair now enable bstack@skills-dir host-scope and skip hand-wiring the six hooks it provides (bstack-autoupdate, knowledge-wakeup, autonomous-posture, arc-continuation, leverage-sensor, l3-stability-pretool), migrating away any already-wired copy so a machine that adopts the plugin then re-runs the installer no longer double-fires. Non-plugin hooks (control-gate, conversation-bridge, knowledge-catalog-refresh, skill-freshness, auth-preflight, role-x) stay hand-wired. - New scripts/lib/plugin-preference.sh: single source of truth — detection, enable/enabled, and the canonical six-basename set (synced with hooks/hooks.json). - install-l3-stability.sh / install-rcs-stability.sh skip the L3-G0 and leverage-sensor merges when the plugin provides them (via BSTACK_PLUGIN_PREFERRED signal OR an already-enabled plugin — direct invocation is double-fire-safe too); their non-plugin work (G1 pre-commit, GH Actions, parameters.toml, audit dir) is unchanged. onboard is covered transitively (runs bootstrap first). - doctor §23/§24 count a plugin-provided loop-sensor/arc-continuation/ autonomous-posture as satisfied instead of gapping them. - Escape hatch BSTACK_NO_PLUGIN=1 forces the legacy hand-wire path. Covered by tests/plugin-preference.test.sh (12 cases: detection, basename membership, idempotent enable, real bootstrap in preferred/migration/fallback). No behavior change for an install without the plugin manifest. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…plugin (P20) Round-1 cross-model review (P20 Strata-B) found a BLOCKER + 3 MINORs; all fixed: - BLOCKER: the unified create-or-merge deduped hooks by basename across ALL blocks of an event, so control-gate (P2 Gate, wired under Bash/Write/Edit) collapsed to a single Bash matcher on a fresh install — silently dropping the Write/Edit gates (P2 "never bypassed" regression). Dedup is now MATCHER-SCOPED (key = matcher+basename) in both bootstrap.sh and repair.sh. Proven: fresh bootstrap now wires all 3 matchers; byte-identical on re-run (idempotent). - MINOR: base() used cmd.split()[0], breaking on a space in the install path (mis-keyed every hook → missed migration / duplicate wiring). Now strips only the trailing flag-args tail, preserving dir spaces. - MINOR: BSTACK_NO_PLUGIN=1 hand-wired the plugin hooks while an already-enabled plugin kept firing them → double-fire. The flag now also disables the plugin (enabledPlugins=false via bstack_disable_plugin); bstack_plugin_enabled honors it. - MINOR: repair --dry-run created .claude/ on disk; the mkdir is now dry-run-guarded. tests/plugin-preference.test.sh grows to 15 cases (adds the all-3-control-gate-matchers invariant on both preferred+fallback paths, and the BSTACK_NO_PLUGIN disable path). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e-state only (P20 r2)
Round-2 cross-model review (8/10, pass) confirmed all 4 round-1 fixes; addressed
the residual MINOR + 2 NITs it surfaced:
- MINOR: base() split on `\s+-`, truncating on a space-then-hyphen in the DIRECTORY
path (".../my repo - v2/ws") → silent loss of sibling same-matcher hooks
(knowledge-catalog-refresh P6) and defeated migration. `--throttle` is the only
arg any hook carries and it is double-dash, so base() now splits on `\s+--`
(bootstrap.sh + repair.sh + the shell twin). New test 11 (space-dash path) covers it.
- NIT: install-l3/install-rcs gated on `BSTACK_PLUGIN_PREFERRED=1 || bstack_plugin_enabled`;
the exported env var could leak into a same-shell direct invocation. Removed the
env coupling entirely — the installers enable the plugin (persisted) before the RCS
step, so `bstack_plugin_enabled` (disk state) is the single source of truth. Dropped
the now-dead `export BSTACK_PLUGIN_PREFERRED=1` from bootstrap/repair. Proven: direct
`install-rcs-stability.sh` skips when enabled, wires when not — no env var involved.
- NIT: aligned the shell twin bstack_plugin_provides_hook to the same double-dash rule.
Test suite now 16 cases; all pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… stale env ref) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What & why (BRO-1929, Phase 3 of BRO-1926)
With the scope-safe plugin shipped (0.35.0 + 0.36.0), the installers now prefer it. When bstack is present as a skills-dir plugin (
.claude-plugin/plugin.jsonunder~/.agents/skills/bstackor~/.claude/skills/bstack),bstack bootstrap/onboard/repair:bstack@skills-dirhost-scope (idempotentenabledPluginswrite in~/.claude/settings.json),bstack-autoupdate,knowledge-wakeup,autonomous-posture,arc-continuation,leverage-sensor,l3-stability-pretool), andleverage-sensorwriters racing; L3 budget double-counted). The hazard existed because the plugin's${CLAUDE_PLUGIN_ROOT}paths and the snippet's$BSTACK_REPOpaths differ, so the old name-only dedup never caught the overlap.Non-plugin hooks stay hand-wired exactly as before:
control-gate(P2, under Bash/Write/Edit),conversation-bridge(P1),knowledge-catalog-refresh(P6),skill-freshness(P7),auth-preflight,role-x.Design
scripts/lib/plugin-preference.sh— single source of truth: detection, enable/disable/enabled, and the canonical six-basename set (synced withhooks/hooks.json).install-l3-stability.sh/install-rcs-stability.shskip the L3-G0 andleverage-sensormerges when the plugin is enabled (disk state = single source of truth; direct invocation is double-fire-safe). Their non-plugin work (G1 pre-commit, GH Actions,rcs-parameters.toml, audit dir) is unchanged.onboardis covered transitively (runs bootstrap first).(matcher, basename)) socontrol-gatesurvives under all 3 PreToolUse matchers — a basename-only key would silently drop the Write/Edit gates on a fresh install.doctor§23/§24 count plugin-provided hooks as satisfied instead of gapping them.BSTACK_NO_PLUGIN=1forces the legacy hand-wire path and disables the plugin first (no double-fire). No manifest / no python3 → legacy path automatically.Test plan
tests/plugin-preference.test.sh— 16 cases, all green: detection (+BSTACK_NO_PLUGIN), basename membership, idempotent enable, and realbootstrapruns in preferred / migration / fallback /BSTACK_NO_PLUGIN/ space-dash-path modes, plus the all-three-control-gate-matchers invariant on both preferred+fallback. Idempotency proven byte-identical on re-run. shellcheck clean;bash -nclean; VERSION↔CHANGELOG match.P20 cross-model adversarial review (2 rounds)
base()split on\s+--so a hyphen in the install path can't truncate) + 2 NITs (env-leak removed; shell twin aligned).Closes BRO-1929.
🤖 Generated with Claude Code