Skip to content

feat(config): add opt-in activation mode with layered control#667

Open
JasonOA888 wants to merge 7 commits intoobra:mainfrom
JasonOA888:feat/opt-in-activation-mode
Open

feat(config): add opt-in activation mode with layered control#667
JasonOA888 wants to merge 7 commits intoobra:mainfrom
JasonOA888:feat/opt-in-activation-mode

Conversation

@JasonOA888
Copy link

Problem (#645)

Superpowers globally modifies Claude's behavior, which some users find invasive. They need control over when and how Superpowers activates.

Solution

Implement a layered activation system with multiple control mechanisms:

Activation Modes

  • always (default): Activate on every session
  • opt-in: Only activate when .superpowers/enabled exists
  • opt-out: Activate unless .superpowers/disabled exists
  • never: Never activate (but stay installed)

Activation Levels

  • full: Complete Superpowers workflow
  • lightweight: Only core skills (brainstorming, using-superpowers)
  • minimal: Only using-superpowers skill

Context-Aware Activation

activation:
  context_aware:
    disable_on_main_branch: true
    lightweight_on_detached_head: true
    require_git_repo: true

Control Layers (priority order)

  1. Environment variable (SUPERPOWERS_MODE=disabled)
  2. Project-level config (.superpowers/config.yaml)
  3. Plugin-level defaults (lib/config.yaml)
  4. Marker files (.superpowers/enabled, .superpowers/disabled)

Architecture

User Request
    ↓
Check SUPERPOWERS_MODE env var
    ↓
Check project .superpowers/config.yaml
    ↓
Check marker files
    ↓
Determine injection level (full/lightweight/minimal)
    ↓
Inject appropriate context

Files Added

  • lib/config.yaml - Default configuration with all options
  • docs/activation-modes.md - User documentation

Files Modified

  • hooks/session-start - Upgraded with mode detection and layered control

Backward Compatibility

✅ Existing installations continue to work with mode: always as default
✅ No breaking changes to existing skills

Example Configurations

Team wants to opt-in per project

# lib/config.yaml (global)
activation:
  mode: opt-in
  per_project_enabled: true
# Enable for specific project
touch .superpowers/enabled

Lightweight mode for quick tasks

activation:
  mode: always
  level: lightweight  # Only brainstorming + using-superpowers

Disable on main branch

activation:
  mode: always
  context_aware:
    disable_on_main_branch: true

Fixes #645

## Problem (obra#645)

Superpowers globally modifies Claude's behavior, which some users find
invasive. They need control over when and how Superpowers activates.

## Solution

Implement a layered activation system with multiple control mechanisms:

### Activation Modes
- **always** (default): Activate on every session
- **opt-in**: Only activate when .superpowers/enabled exists
- **opt-out**: Activate unless .superpowers/disabled exists
- **never**: Never activate (but stay installed)

### Activation Levels
- **full**: Complete Superpowers workflow
- **lightweight**: Only core skills (brainstorming, using-superpowers)
- **minimal**: Only using-superpowers skill

### Context-Aware Activation
- Disable on main/master branch
- Lightweight mode on detached HEAD
- Require git repository

### Control Layers (priority order)
1. Environment variable (SUPERPOWERS_MODE)
2. Project-level config (.superpowers/config.yaml)
3. Plugin-level defaults (lib/config.yaml)
4. Marker files (.superpowers/enabled, .superpowers/disabled)

## Files Added
- lib/config.yaml - Default configuration
- hooks/session-start - Upgraded with mode detection
- docs/activation-modes.md - User documentation

## Backward Compatibility
Existing installations continue to work with mode: always as default.

Fixes obra#645
@coderabbitai
Copy link

coderabbitai bot commented Mar 10, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Refactors session-start hook into a config-driven activation system and adds a documented activation modes guide and new Superpowers config schema. The hook now evaluates layered activation (global, plugin, project, env), computes injection level, and emits a unified SESSION_CONTEXT payload. No public API signature changes.

Changes

Cohort / File(s) Summary
Documentation
docs/activation-modes.md
New comprehensive guide describing activation modes (always, opt-in, opt-out, never), activation levels (full, lightweight, minimal), per-skill controls, context-aware toggles, env/CLI overrides, YAML examples, and migration notes.
Configuration
lib/config.yaml
Adds Superpowers configuration schema: top-level activation (mode, per_project_enabled, level), context toggles (disable_on_main_branch, lightweight_on_detached_head, require_git_repo), skill entries and flags, whitelist_skills / blacklist_skills, and per-project override capability.
Hook Logic
hooks/session-start
Reworks activation flow to parse plugin/project YAML and env vars, apply layered precedence (global-disable file, plugin defaults, project config, SUPERPOWERS_MODE env), determine enabled state and injection mode, build escaped CONTENT (including legacy SKILLS_DIR warning), and emit a single SESSION_CONTEXT used for both additional_context and hookSpecificOutput.additionalContext.

Sequence Diagram

sequenceDiagram
    participant Env as Environment
    participant Hook as Hook (hooks/session-start)
    participant Config as Config Parser (lib/config.yaml)
    participant Activator as Activation Logic
    participant Generator as Content/Skill Generator
    participant Output as JSON Emitter

    Env->>Hook: provide SUPERPOWERS_MODE / env vars
    Hook->>Config: parse_config(project & plugin)
    Config-->>Hook: config object
    Hook->>Activator: evaluate mode, level, context
    Activator->>Activator: check global-disable, plugin defaults, project config, env precedence
    alt Enabled
        Activator->>Generator: determine injection mode (full/lightweight/minimal)
        Generator-->>Activator: CONTENT (skill text or snippet)
        Activator->>Activator: append legacy SKILLS_DIR warning if present
    else Disabled
        Activator->>Activator: generate availability/enablement notice with reason (env_var, global_disabled, config_never, opt_in, etc.)
    end
    Activator-->>Hook: SESSION_CONTEXT (escaped JSON payload)
    Hook->>Output: emit { additional_context, hookSpecificOutput.additionalContext } with SESSION_CONTEXT
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibble configs under moonlit beams,
Modes and levels mapped in YAML dreams,
Hooks now ask softly if they may leap,
Legacy crumbs I tuck in a heap,
Hop on—Superpowers waits, not leaps!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: implementing an opt-in activation mode with layered control mechanisms, which aligns with the primary objectives of the PR.
Description check ✅ Passed The description comprehensively explains the problem, solution, architecture, and examples, directly relating to the changeset and implementation of activation modes.
Linked Issues check ✅ Passed The PR addresses #645 by implementing a comprehensive layered activation system that goes beyond the basic requirement of an environment variable toggle, providing opt-in/opt-out modes, multiple control layers, and context-aware activation.
Out of Scope Changes check ✅ Passed All changes are scoped to the activation system implementation: docs/activation-modes.md adds user documentation, lib/config.yaml introduces the configuration schema, and hooks/session-start implements the mode detection logic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@hooks/session-start`:
- Around line 68-72: Project-level config currently replaces plugin defaults
because the script checks PROJECT_ROOT config before PLUGIN_ROOT config; change
the load order so plugin defaults are parsed first and then project config is
parsed to override them. Specifically, ensure both paths are checked and call
parse_config on "${PLUGIN_ROOT}/lib/config.yaml" first (if present) and then on
"${PROJECT_ROOT}/.superpowers/config.yaml" (if present), so
activation.per_project_enabled and other defaults in lib/config.yaml are layered
and can be overridden by project settings.
- Around line 123-126: The availability notice in hooks/session-start currently
hardcodes "Current mode: opt-in" and a single enable instruction inside the
CONTENT assignment; update the logic that builds CONTENT (the else branch that
sets CONTENT="<SUPERPOWERS_AVAILABLE>...") to compute the actual disable mode
and context and inject mode-specific messaging and actions instead of the
hardcoded opt-in text — detect the exact disabling reason (e.g., "never",
environment variable disable, global disable file like ~/.superpowers/disabled,
or context-aware disable) and vary the "Current mode:" line and the suggested
remediation (e.g., remove global file, unset env var, or create
.superpowers/enabled only when opt-in applies) so the notice is accurate for
functions/variables handling disable checks in this file.
- Around line 75-80: The branch incorrectly coerces SUPERPOWERS_MODE into a
boolean via SUPERPOWERS_ENABLED and only handles "enabled"/"disabled", so other
mode strings like "opt-in"/"opt-out"/"never" are ignored; change the logic to
preserve the raw per-session override (e.g., set a new variable
SUPERPOWERS_SESSION_MODE= "${SUPERPOWERS_MODE:-}" when present) or expand the
conditional to explicitly map additional accepted mode strings to
enabled/disabled (recognize "opt-in","opt-out","never", etc.) and update
downstream checks to use SUPERPOWERS_SESSION_MODE or the mapped
SUPERPOWERS_ENABLED value instead of discarding unknown values.
- Around line 43-55: parse_config reads lightweight_on_detached_head but the
hook never applies it; add logic after determining CURRENT_BRANCH (or checking
detached HEAD via git symbolic-ref -q --short HEAD) to detect a detached HEAD
and, when lightweight_on_detached_head is true, set the injection mode to a
lighter level instead of leaving normal behavior or disabling entirely (e.g. set
a variable like SUPERPOWERS_INJECTION_LEVEL="lightweight" or a flag such as
SUPERPOWERS_LIGHTWEIGHT=true), making sure subsequent code that checks
SUPERPOWERS_ENABLED or injection behavior reads that variable to downgrade
injection in functions that perform the prompt/tooling injection.
- Around line 104-119: The script unconditionally checks [ -f "$CONTENT_FILE" ]
while CONTENT_FILE is only set in the full branch, causing an unbound variable
error under set -u; update the case handling so CONTENT_FILE is always defined
(e.g., set CONTENT_FILE="" in the lightweight and minimal branches or assign
CONTENT_FILE from the generated CONTENT), or guard the file test with a check
like: only run [ -f "$CONTENT_FILE" ] if CONTENT_FILE is non-empty; modify the
block around the INJECTION_MODE case and the subsequent if [ -f "$CONTENT_FILE"
] check to ensure CONTENT_FILE is always bound before dereference (references:
INJECTION_MODE, CONTENT_FILE, CONTENT).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8fa5702c-4de7-4c44-b2db-41dd41ff70e7

📥 Commits

Reviewing files that changed from the base of the PR and between 33e55e6 and 4f4f196.

📒 Files selected for processing (3)
  • docs/activation-modes.md
  • hooks/session-start
  • lib/config.yaml

## Critical Fixes

1. **Fix CONTENT_FILE unbound variable**
   - Initialize CONTENT and CONTENT_FILE before case statement
   - Guard file test with [ -n "$CONTENT_FILE" ]
   - Prevents set -u abort in lightweight/minimal modes

## Major Fixes

2. **Add lightweight_on_detached_head support**
   - Detect detached HEAD (CURRENT_BRANCH = "HEAD")
   - Downgrade injection mode to lightweight when configured
   - Previously documented but not implemented

3. **Fix config layering order**
   - Parse plugin defaults first (lib/config.yaml)
   - Then parse project overrides (.superpowers/config.yaml)
   - Allows project configs to extend defaults, not replace

4. **Expand SUPERPOWERS_MODE support**
   - Accept all mode values: enabled, disabled, always, never, opt-in, opt-out
   - Store raw value in SUPERPOWERS_SESSION_MODE
   - Map to SUPERPOWERS_ENABLED boolean

5. **Improve disabled-state notice**
   - Detect exact disable reason (env var, global file, config, opt-in)
   - Show mode-specific remediation steps
   - No more hardcoded "opt-in" for all cases

Addresses all 5 actionable comments from CodeRabbit
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
hooks/session-start (1)

152-164: ⚠️ Potential issue | 🟡 Minor

Disabled-state remediation still misses some real disable paths.

This only classifies env var, global disable, project mode: never, and plugin mode: opt-in. If activation is turned off by disable_on_main_branch, require_git_repo, or a project-level mode: opt-in, the fallback notice points users at .superpowers/enabled, which is misleading for those cases. Capture the effective disable reason when SUPERPOWERS_ENABLED flips to false and reuse it here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/session-start` around lines 152 - 164, The current DISABLE_REASON
detection only covers SUPERPOWERS_SESSION_MODE, global disable file, project
config: mode: never, and plugin opt-in; update the logic that sets
DISABLE_REASON to also detect and set distinct reasons when SUPERPOWERS_ENABLED
is false due to disable_on_main_branch, require_git_repo, or a project-level
mode: opt-in (check PROJECT_ROOT/.superpowers/config.yaml for mode: opt-in), and
ensure any place that flips SUPERPOWERS_ENABLED to "false" records the same
reason so the fallback notice uses DISABLE_REASON rather than always pointing to
.superpowers/enabled; reference the existing DISABLE_REASON variable and checks
around SUPERPOWERS_ENABLED, disable_on_main_branch and require_git_repo so the
same reason string is reused in the notice generation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@hooks/session-start`:
- Around line 107-109: The inline message assignments (e.g., LEGACY_WARNING and
the various CONTENT= variables) currently contain literal "\n" sequences inside
double-quoted strings, which end up rendered as backslash-n; update each inline
message to use real newlines inside the quoted string instead of the
two-character sequence "\n" — i.e., replace occurrences like
"...\"\n\n<important...>..." with a single- or double-quoted multiline string
that contains actual blank lines (real newlines) between paragraphs for
LEGACY_WARNING, CONTENT, and the other inline message assignments in this file
so the messages render with proper line breaks.
- Around line 86-99: The opt-in branch currently only sets
SUPERPOWERS_ENABLED=false when the marker is missing and never sets it true when
the marker exists, so an earlier false value persists; update the opt-in case
for SUPERPOWERS_SESSION_MODE to explicitly set SUPERPOWERS_ENABLED=true when the
file "${PROJECT_ROOT}/.superpowers/enabled" exists and set it to false when it
does not, ensuring the opt-in marker can override lower-priority disables;
reference the SUPERPOWERS_SESSION_MODE case and the SUPERPOWERS_ENABLED variable
in the opt-in branch to implement this explicit toggle.

---

Duplicate comments:
In `@hooks/session-start`:
- Around line 152-164: The current DISABLE_REASON detection only covers
SUPERPOWERS_SESSION_MODE, global disable file, project config: mode: never, and
plugin opt-in; update the logic that sets DISABLE_REASON to also detect and set
distinct reasons when SUPERPOWERS_ENABLED is false due to
disable_on_main_branch, require_git_repo, or a project-level mode: opt-in (check
PROJECT_ROOT/.superpowers/config.yaml for mode: opt-in), and ensure any place
that flips SUPERPOWERS_ENABLED to "false" records the same reason so the
fallback notice uses DISABLE_REASON rather than always pointing to
.superpowers/enabled; reference the existing DISABLE_REASON variable and checks
around SUPERPOWERS_ENABLED, disable_on_main_branch and require_git_repo so the
same reason string is reused in the notice generation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6ac3c5cf-729b-41f3-86d0-e2bf5693f6ef

📥 Commits

Reviewing files that changed from the base of the PR and between 4f4f196 and 11e35da.

📒 Files selected for processing (1)
  • hooks/session-start

## Problem

SUPERPOWERS_MODE=opt-in with .superpowers/enabled file was still disabled
if a lower-priority layer (e.g., mode: never) set SUPERPOWERS_ENABLED=false.

## Fix

Set SUPERPOWERS_ENABLED=true before checking marker, then set false
only if marker is missing. This ensures opt-in can override disables.

Addresses CodeRabbit feedback on opt-in override logic
- Fix opt-in mode to properly override lower-priority disables
- Replace all literal \n with real newlines using $'...' syntax
- Addresses remaining CodeRabbit feedback for PR obra#667
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (4)
hooks/session-start (4)

167-184: ⚠️ Potential issue | 🟡 Minor

Literal \n in disabled-state notice strings.

All CONTENT= assignments in this block use "\n" which produces literal backslash-n characters. Apply the same $'...' syntax fix as noted for other content strings.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/session-start` around lines 167 - 184, The CONTENT assignments inside
the DISABLE_REASON case produce literal backslash-n sequences; update each
CONTENT= assignment in that case block (the case on DISABLE_REASON and the
CONTENT variable) to use the $'...'-style quoted string so the \n sequences are
interpreted as newlines (apply the same $'...' pattern used elsewhere in the
script for the other notice strings). Ensure each branch (env_var,
global_disabled, config_never, opt_in, and the *) uses $'...'-quoted content so
newlines render correctly.

137-143: ⚠️ Potential issue | 🟡 Minor

Literal \n in lightweight/minimal content strings.

Same issue as the legacy warning: "\n" inside double quotes produces literal backslash-n characters that won't render as line breaks in the final output.

🔧 Proposed fix using $'...' syntax
         lightweight)
             # Create lightweight content (only core skills)
-            CONTENT="<EXTREMELY_IMPORTANT>\nSuperpowers is active in lightweight mode.\n\nCore skills available:\n- brainstorming: Design before implementation\n- using-superpowers: How to use Superpowers\n\nFull workflow (TDD, subagent-driven-development) is disabled.\nEnable with: /superpowers level full\n</EXTREMELY_IMPORTANT>"
+            CONTENT=$'<EXTREMELY_IMPORTANT>\nSuperpowers is active in lightweight mode.\n\nCore skills available:\n- brainstorming: Design before implementation\n- using-superpowers: How to use Superpowers\n\nFull workflow (TDD, subagent-driven-development) is disabled.\nEnable with: /superpowers level full\n</EXTREMELY_IMPORTANT>'
             ;;
         minimal)
-            CONTENT="<EXTREMELY_IMPORTANT>\nSuperpowers is active in minimal mode.\n\nOnly using-superpowers skill is loaded.\nEnable more: /superpowers level lightweight|full\n</EXTREMELY_IMPORTANT>"
+            CONTENT=$'<EXTREMELY_IMPORTANT>\nSuperpowers is active in minimal mode.\n\nOnly using-superpowers skill is loaded.\nEnable more: /superpowers level lightweight|full\n</EXTREMELY_IMPORTANT>'
             ;;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/session-start` around lines 137 - 143, The CONTENT strings in the
lightweight) and minimal) case blocks contain literal "\n" sequences because
they use double-quoted strings; replace those with shell-escaped strings that
interpret backslash escapes (for example use $'...') or construct the value via
printf to produce real newlines so the message renders with line breaks; update
the CONTENT assignment in the lightweight) and minimal) cases accordingly
(references: CONTENT variable, lightweight) case label, minimal) case label).

108-111: ⚠️ Potential issue | 🟡 Minor

Literal \n sequences won't render as newlines.

Line 110 uses "\n\n..." inside double quotes, which produces literal backslash-n characters. After JSON escaping, these become \\n which renders as visible \n text rather than line breaks.

🔧 Proposed fix using $'...' syntax
 LEGACY_SKILLS_DIR="${HOME}/.config/superpowers/skills"
 if [ -d "$LEGACY_SKILLS_DIR" ]; then
-    LEGACY_WARNING="\n\n<important-reminder>⚠️ **WARNING:** Superpowers now uses Claude Code's skills system. Custom skills in ~/.config/superpowers/skills will not be read. Move to ~/.claude/skills instead. Remove the directory to dismiss this message.</important-reminder>"
+    LEGACY_WARNING=$'\n\n<important-reminder>⚠️ **WARNING:** Superpowers now uses Claude Code\'s skills system. Custom skills in ~/.config/superpowers/skills will not be read. Move to ~/.claude/skills instead. Remove the directory to dismiss this message.</important-reminder>'
 fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/session-start` around lines 108 - 111, The LEGACY_WARNING string
currently uses literal "\n\n" inside double quotes so it renders backslash-n
sequences instead of newlines; update how the string is constructed (the
LEGACY_WARNING assignment) to produce actual newlines (for example using $'...''
quoting or by building the string via printf/echo -e) so the message contains
real line breaks when consumed; change the LEGACY_WARNING variable creation (and
any consumer if needed) to use the chosen newline-producing approach instead of
plain double-quoted "\n" sequences.

189-189: ⚠️ Potential issue | 🟡 Minor

Literal \n in SESSION_CONTEXT wrapping.

Same issue: "\n" produces literal backslash-n. Use $'\n' for actual newlines.

🔧 Proposed fix
-SESSION_CONTEXT="<EXTREMELY_IMPORTANT>\n${CONTENT_ESCAPED}\n</EXTREMELY_IMPORTANT>"
+SESSION_CONTEXT=$'<EXTREMELY_IMPORTANT>\n'"${CONTENT_ESCAPED}"$'\n</EXTREMELY_IMPORTANT>'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/session-start` at line 189, The SESSION_CONTEXT assignment is using a
double-quoted string that contains literal backslash-n sequences, so newlines
are not being inserted; change the assignment to use ANSI-C quoting (or $'...')
so the \n sequences are expanded into real newlines—i.e., update the
SESSION_CONTEXT definition to use $'...\' with ${CONTENT_ESCAPED} inside (or
otherwise emit actual newlines) so the wrapper <EXTREMELY_IMPORTANT> block
contains real line breaks.
🧹 Nitpick comments (1)
hooks/session-start (1)

20-41: Simple YAML parsing is functional but fragile.

The grep -q "mode: opt-in" pattern matching works because grep matches substrings regardless of YAML indentation. However, this could produce false positives if the pattern appears in comments or unrelated contexts.

For a more robust approach in the future, consider using yq or a more precise pattern like grep -qE '^\s+mode:\s*opt-in'.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/session-start` around lines 20 - 41, The current parse_config() uses
simple grep substring matches (e.g., grep -q "mode: opt-in") which can yield
false positives from comments or other fields; update those checks to use
anchored, whitespace-tolerant regexes or yq. Specifically, replace the mode and
level grep checks in parse_config() with patterns like grep -qE
'^\s*mode:\s*(opt-in|opt-out|never)\b' and grep -qE
'^\s*level:\s*(lightweight|minimal)\b' (or, if available, parse with yq to read
.mode and .level into INJECTION_MODE and SUPERPOWERS_ENABLED) so only exact YAML
keys are matched. Ensure you still set SUPERPOWERS_ENABLED and INJECTION_MODE
exactly where the original branches reference them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@hooks/session-start`:
- Around line 160-165: The DISABLE_REASON logic misses when the project-level
config sets "mode: opt-in"; update the conditional checks around PROJECT_ROOT
and PLUGIN_ROOT so you also grep the project config for "mode: opt-in" (in
addition to the existing "mode: never" check), and set DISABLE_REASON="opt_in"
when that grep matches; adjust the order so explicit project settings (grep
against ${PROJECT_ROOT}/.superpowers/config.yaml) are evaluated before falling
back to the plugin config (${PLUGIN_ROOT}/lib/config.yaml) to ensure the
project-level opt-in is respected.
- Around line 87-100: The opt-out branch currently sets SUPERPOWERS_ENABLED=true
unconditionally; change the logic so that when SUPERPOWERS_SESSION_MODE is
"opt-out" you set SUPERPOWERS_ENABLED=true only if
"${PROJECT_ROOT}/.superpowers/disabled" does not exist. Locate the case handling
for SUPERPOWERS_SESSION_MODE and update the "opt-out" branch to check for the
disabled marker (similar to how the "opt-in" branch checks .superpowers/enabled)
and set SUPERPOWERS_ENABLED=false when that file is present.

---

Duplicate comments:
In `@hooks/session-start`:
- Around line 167-184: The CONTENT assignments inside the DISABLE_REASON case
produce literal backslash-n sequences; update each CONTENT= assignment in that
case block (the case on DISABLE_REASON and the CONTENT variable) to use the
$'...'-style quoted string so the \n sequences are interpreted as newlines
(apply the same $'...' pattern used elsewhere in the script for the other notice
strings). Ensure each branch (env_var, global_disabled, config_never, opt_in,
and the *) uses $'...'-quoted content so newlines render correctly.
- Around line 137-143: The CONTENT strings in the lightweight) and minimal) case
blocks contain literal "\n" sequences because they use double-quoted strings;
replace those with shell-escaped strings that interpret backslash escapes (for
example use $'...') or construct the value via printf to produce real newlines
so the message renders with line breaks; update the CONTENT assignment in the
lightweight) and minimal) cases accordingly (references: CONTENT variable,
lightweight) case label, minimal) case label).
- Around line 108-111: The LEGACY_WARNING string currently uses literal "\n\n"
inside double quotes so it renders backslash-n sequences instead of newlines;
update how the string is constructed (the LEGACY_WARNING assignment) to produce
actual newlines (for example using $'...'' quoting or by building the string via
printf/echo -e) so the message contains real line breaks when consumed; change
the LEGACY_WARNING variable creation (and any consumer if needed) to use the
chosen newline-producing approach instead of plain double-quoted "\n" sequences.
- Line 189: The SESSION_CONTEXT assignment is using a double-quoted string that
contains literal backslash-n sequences, so newlines are not being inserted;
change the assignment to use ANSI-C quoting (or $'...') so the \n sequences are
expanded into real newlines—i.e., update the SESSION_CONTEXT definition to use
$'...\' with ${CONTENT_ESCAPED} inside (or otherwise emit actual newlines) so
the wrapper <EXTREMELY_IMPORTANT> block contains real line breaks.

---

Nitpick comments:
In `@hooks/session-start`:
- Around line 20-41: The current parse_config() uses simple grep substring
matches (e.g., grep -q "mode: opt-in") which can yield false positives from
comments or other fields; update those checks to use anchored,
whitespace-tolerant regexes or yq. Specifically, replace the mode and level grep
checks in parse_config() with patterns like grep -qE
'^\s*mode:\s*(opt-in|opt-out|never)\b' and grep -qE
'^\s*level:\s*(lightweight|minimal)\b' (or, if available, parse with yq to read
.mode and .level into INJECTION_MODE and SUPERPOWERS_ENABLED) so only exact YAML
keys are matched. Ensure you still set SUPERPOWERS_ENABLED and INJECTION_MODE
exactly where the original branches reference them.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 818af006-9c78-48a0-b65c-16b8558a2c03

📥 Commits

Reviewing files that changed from the base of the PR and between 11e35da and 75393e3.

📒 Files selected for processing (1)
  • hooks/session-start

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
hooks/session-start (2)

87-100: ⚠️ Potential issue | 🟠 Major

SUPERPOWERS_MODE=opt-out ignores the disabled marker file.

When opt-out is specified via environment variable (line 88), it unconditionally sets SUPERPOWERS_ENABLED=true. Per the documented behavior, opt-out mode should "activate unless .superpowers/disabled exists". The env var override should still respect the disabled marker.

🔧 Proposed fix
     case "$SUPERPOWERS_SESSION_MODE" in
-        enabled|always|opt-out)
+        enabled|always)
             SUPERPOWERS_ENABLED=true
             ;;
+        opt-out)
+            SUPERPOWERS_ENABLED=true
+            if [ -f "${PROJECT_ROOT}/.superpowers/disabled" ]; then
+                SUPERPOWERS_ENABLED=false
+            fi
+            ;;
         disabled|never)
             SUPERPOWERS_ENABLED=false
             ;;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/session-start` around lines 87 - 100, The opt-out branch currently sets
SUPERPOWERS_ENABLED=true unconditionally, ignoring the .superpowers/disabled
marker; update the case handling for SUPERPOWERS_SESSION_MODE (in the switch
that sets SUPERPOWERS_ENABLED) so that opt-out behaves like "enabled unless
${PROJECT_ROOT}/.superpowers/disabled exists" — i.e., set
SUPERPOWERS_ENABLED=true for opt-out and then check if the disabled marker
exists and set SUPERPOWERS_ENABLED=false if so; keep the existing opt-in logic
unchanged.

160-167: ⚠️ Potential issue | 🟠 Major

DISABLE_REASON doesn't detect project-level opt-in mode.

Lines 162-164 check project config only for mode: never, and lines 165-166 check plugin config only for mode: opt-in. If the project config sets mode: opt-in and the marker is missing, the disable reason falls through to the default case instead of showing the accurate opt-in notice.

🔧 Proposed fix
     elif [ -f "${PROJECT_ROOT}/.superpowers/config.yaml" ] && \
          grep -q "mode: never" "${PROJECT_ROOT}/.superpowers/config.yaml"; then
         DISABLE_REASON="config_never"
+    elif [ -f "${PROJECT_ROOT}/.superpowers/config.yaml" ] && \
+         grep -q "mode: opt-in" "${PROJECT_ROOT}/.superpowers/config.yaml"; then
+        DISABLE_REASON="opt_in"
     elif grep -q "mode: opt-in" "${PLUGIN_ROOT}/lib/config.yaml" 2>/dev/null; then
         DISABLE_REASON="opt_in"
     fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/session-start` around lines 160 - 167, The DISABLE_REASON logic
currently only checks PROJECT_ROOT config for "mode: never" and checks
PLUGIN_ROOT for "mode: opt-in", so project-level "mode: opt-in" is missed;
update the conditional sequence in hooks/session-start to also detect "mode:
opt-in" inside "${PROJECT_ROOT}/.superpowers/config.yaml" (using grep -q "mode:
opt-in" "${PROJECT_ROOT}/.superpowers/config.yaml" 2>/dev/null) and set
DISABLE_REASON="opt_in" when matched, ensuring the check occurs before falling
through to the default case that sets no disable reason.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@hooks/session-start`:
- Around line 139-145: The CONTENT assignments in the 'lightweight' and
'minimal' case blocks use ANSI-C $'...' quoting but end with a double-quote,
causing a syntax error; update both CONTENT lines so the multi-line
$'...'<EXTREMELY_IMPORTANT>...'</EXTREMELY_IMPORTANT> strings close with a
single quote (i.e., use
CONTENT=$'<EXTREMELY_IMPORTANT>...'</EXTREMELY_IMPORTANT>') so the $'...'
quoting is properly terminated for the lightweight and minimal case branches
where the CONTENT variable is set.
- Around line 172-190: The CONTENT assignments use ANSI-C $'...' strings but end
with a double-quote, causing a syntax error; fix each case (the patterns with
labels global_disabled, config_never, opt_in, the earlier env-disabled branch
and the default *) by replacing the trailing " with a single quote so each
CONTENT uses $'...'; also escape any embedded single quotes (e.g., 'enabled' and
'never' → \') inside the $'...' string, and handle variable interpolation for
$SUPERPOWERS_SESSION_MODE by breaking out of the $'...' string and concatenating
the shell variable (e.g., end the $'...' before the variable, append
"$SUPERPOWERS_SESSION_MODE" unquoted or quoted appropriately, then resume $'...'
if needed) so the message shows the current value correctly.

---

Duplicate comments:
In `@hooks/session-start`:
- Around line 87-100: The opt-out branch currently sets SUPERPOWERS_ENABLED=true
unconditionally, ignoring the .superpowers/disabled marker; update the case
handling for SUPERPOWERS_SESSION_MODE (in the switch that sets
SUPERPOWERS_ENABLED) so that opt-out behaves like "enabled unless
${PROJECT_ROOT}/.superpowers/disabled exists" — i.e., set
SUPERPOWERS_ENABLED=true for opt-out and then check if the disabled marker
exists and set SUPERPOWERS_ENABLED=false if so; keep the existing opt-in logic
unchanged.
- Around line 160-167: The DISABLE_REASON logic currently only checks
PROJECT_ROOT config for "mode: never" and checks PLUGIN_ROOT for "mode: opt-in",
so project-level "mode: opt-in" is missed; update the conditional sequence in
hooks/session-start to also detect "mode: opt-in" inside
"${PROJECT_ROOT}/.superpowers/config.yaml" (using grep -q "mode: opt-in"
"${PROJECT_ROOT}/.superpowers/config.yaml" 2>/dev/null) and set
DISABLE_REASON="opt_in" when matched, ensuring the check occurs before falling
through to the default case that sets no disable reason.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 44723009-4d3a-4aa5-90b5-b1e7004080fb

📥 Commits

Reviewing files that changed from the base of the PR and between 75393e3 and 60473a7.

📒 Files selected for processing (1)
  • hooks/session-start

## Changes

1. **Fix all literal \n sequences**
   - LEGACY_WARNING: use $'...' with proper escaping
   - SESSION_CONTEXT: use $'...' with variable interpolation
   - All DISABLE_REASON notices already use $'...'

2. **Fix opt-out mode**
   - Check .superpowers/disabled file
   - Only enable if file doesn't exist
   - Matches opt-in pattern

3. **Fix DISABLE_REASON detection**
   - Check project-level config for mode: opt-in
   - Check before falling back to plugin config
   - Ensures correct notice is shown

Addresses all CodeRabbit feedback for PR obra#667
## Critical Fixes

1. **Fix mismatched quotes**
   - All $'...' strings were ending with " instead of '
   - Caused bash syntax errors
   - Now properly closed with single quote

2. **Escape embedded single quotes**
   - 'enabled' → '\'enabled\''
   - 'never' → '\'never\''

3. **Fix variable interpolation**
   - $SUPERPOWERS_SESSION_MODE was inside $'...'
   - Split string: $'...'$VAR$'...'
   - Now properly interpolates the variable value

This fixes critical bash parsing errors that would prevent the
session-start hook from running.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
hooks/session-start (2)

145-150: ⚠️ Potential issue | 🔴 Critical

Fix the $'...' CONTENT blocks before this hook can parse.

Lines 145-150 and 181-198 still terminate ANSI-C quoted strings with " instead of '. The env_var/config_never messages also put 'enabled', 'never', and $SUPERPOWERS_SESSION_MODE inside $'...', so Bash either rejects the file or treats the variable literally. bash -n should fail here.

#!/bin/bash
set -euo pipefail

bash -n hooks/session-start
sed -n '145,198p' hooks/session-start
Suggested pattern
-            CONTENT=$'<EXTREMELY_IMPORTANT>
-Superpowers is active in lightweight mode.\n\nCore skills available:\n- brainstorming: Design before implementation\n- using-superpowers: How to use Superpowers\n\nFull workflow (TDD, subagent-driven-development) is disabled.\nEnable with: /superpowers level full\n</EXTREMELY_IMPORTANT>"
+            CONTENT=$'<EXTREMELY_IMPORTANT>
+Superpowers is active in lightweight mode.\n\nCore skills available:\n- brainstorming: Design before implementation\n- using-superpowers: How to use Superpowers\n\nFull workflow (TDD, subagent-driven-development) is disabled.\nEnable with: /superpowers level full\n</EXTREMELY_IMPORTANT>'

-            CONTENT=$'<SUPERPOWERS_AVAILABLE>
-Superpowers disabled by environment variable.\n\nTo enable:\n- Unset SUPERPOWERS_MODE or set to 'enabled'\n- Current: SUPERPOWERS_MODE=$SUPERPOWERS_SESSION_MODE\n</SUPERPOWERS_AVAILABLE>"
+            CONTENT=$'<SUPERPOWERS_AVAILABLE>
+Superpowers disabled by environment variable.
+
+To enable:
+- Unset SUPERPOWERS_MODE or set to \'enabled\'
+- Current: SUPERPOWERS_MODE='"${SUPERPOWERS_SESSION_MODE}"$'
+</SUPERPOWERS_AVAILABLE>'

Also applies to: 181-198

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/session-start` around lines 145 - 150, The CONTENT assignments use
ANSI-C $'...' quoting but are incorrectly terminated with double quotes and
embed single-quoted words and the variable $SUPERPOWERS_SESSION_MODE inside the
$'...' blocks, which breaks bash parsing; fix the CONTENT blocks (for the
lightweight and minimal cases and the later blocks around lines with
env_var/config_never messages) by using matching $'...'<single-quote>
terminators (not double quotes), remove or properly escape internal single
quotes, and avoid placing unexpanded variables like $SUPERPOWERS_SESSION_MODE
inside raw ANSI-C quotes (either move the variable outside the $'...' string or
use proper escaping) so bash -n succeeds and the hooks parse correctly.

164-199: ⚠️ Potential issue | 🟡 Minor

The fallback disabled notice is still wrong for opt-out and context-aware disables.

DISABLE_REASON only distinguishes env/global/config-never/opt-in. If activation is turned off by .superpowers/disabled, disable_on_main_branch, or require_git_repo, this falls through to Lines 197-198 and tells users to create .superpowers/enabled, which will not re-enable those cases.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@hooks/session-start` around lines 164 - 199, The fallback message is
incorrect for context-aware disables — add explicit detection and case branches
for the context flags so users get correct guidance: in the DISABLE_REASON
assignment block (where DISABLE_REASON is set based on env/config), add checks
that set DISABLE_REASON="disabled_on_main_branch" when the plugin/config
indicates disable_on_main_branch, and DISABLE_REASON="require_git_repo" when
activation is blocked by require_git_repo (use the same logic/grep used
elsewhere to detect those settings); then extend the case "$DISABLE_REASON"
statement to include opt-out/context-aware cases ("disabled_on_main_branch" and
"require_git_repo") with tailored CONTENT messages (do not suggest creating
.superpowers/enabled for these two) so each unique disabling cause
(DISABLE_REASON) maps to an accurate user instruction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@hooks/session-start`:
- Around line 75-81: The plugin always calls parse_config on the project file,
ignoring the plugin default activation.per_project_enabled flag; after parsing
the plugin defaults from "${PLUGIN_ROOT}/lib/config.yaml" (parse_config), read
the parsed activation.per_project_enabled value and only call parse_config on
"${PROJECT_ROOT}/.superpowers/config.yaml" if activation.per_project_enabled is
true (or unset); update the conditional around the project parse to check that
flag so plugin defaults can disable per-project overrides.
- Around line 203-213: SESSION_CONTEXT currently contains literal newlines
(built with ANSI-C $'... \n ...' quoting) which produces invalid JSON when
interpolated; instead construct SESSION_CONTEXT as a single-line string with
backslash-escaped newline sequences so JSON receives "\n" characters, e.g. build
SESSION_CONTEXT by concatenating the escaped prefix "<EXTREMELY_IMPORTANT>", the
already-escaped CONTENT via escape_for_json (CONTENT_ESCAPED), and the escaped
suffix "</EXTREMELY_IMPORTANT>" with "\\n" between them (use simple
double-quoted string concatenation rather than $'...' quoting). Update any
places that reference SESSION_CONTEXT (the JSON output block and
"additionalContext") to use this new escaped form so the emitted JSON remains
valid.

---

Duplicate comments:
In `@hooks/session-start`:
- Around line 145-150: The CONTENT assignments use ANSI-C $'...' quoting but are
incorrectly terminated with double quotes and embed single-quoted words and the
variable $SUPERPOWERS_SESSION_MODE inside the $'...' blocks, which breaks bash
parsing; fix the CONTENT blocks (for the lightweight and minimal cases and the
later blocks around lines with env_var/config_never messages) by using matching
$'...'<single-quote> terminators (not double quotes), remove or properly escape
internal single quotes, and avoid placing unexpanded variables like
$SUPERPOWERS_SESSION_MODE inside raw ANSI-C quotes (either move the variable
outside the $'...' string or use proper escaping) so bash -n succeeds and the
hooks parse correctly.
- Around line 164-199: The fallback message is incorrect for context-aware
disables — add explicit detection and case branches for the context flags so
users get correct guidance: in the DISABLE_REASON assignment block (where
DISABLE_REASON is set based on env/config), add checks that set
DISABLE_REASON="disabled_on_main_branch" when the plugin/config indicates
disable_on_main_branch, and DISABLE_REASON="require_git_repo" when activation is
blocked by require_git_repo (use the same logic/grep used elsewhere to detect
those settings); then extend the case "$DISABLE_REASON" statement to include
opt-out/context-aware cases ("disabled_on_main_branch" and "require_git_repo")
with tailored CONTENT messages (do not suggest creating .superpowers/enabled for
these two) so each unique disabling cause (DISABLE_REASON) maps to an accurate
user instruction.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b7edddf2-a705-4ec3-b69a-20aaa82dfcc8

📥 Commits

Reviewing files that changed from the base of the PR and between 60473a7 and 5c43410.

📒 Files selected for processing (1)
  • hooks/session-start

## Critical Fixes

1. **Fix SESSION_CONTEXT JSON serialization**
   - Use escaped newlines (\n) instead of literal newlines
   - Previous: $'...\n...' → invalid JSON
   - Now: "...\n..." → valid JSON

## Major Improvements

2. **Honor per_project_enabled flag**
   - Add PROJECT_OVERRIDES_ENABLED variable
   - Only parse project config if enabled
   - Respects plugin defaults

3. **Context-aware DISABLE_REASON**
   - Detect disable_on_main_branch
   - Detect require_git_repo
   - Show specific messages for each

4. **Accurate user guidance**
   - Different messages for different disable causes
   - Don't suggest .superpowers/enabled for context-aware disables
   - Clear instructions for each case

Addresses CodeRabbit critical and major feedback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: opt-in mode that disables auto-triggering

1 participant