Skip to content

GH#2926: fix supervisor pulse consent model — default OFF, persist consent, never silently re-enable #2936

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcusquinn merged 1 commit into from
Mar 5, 2026
Merged

GH#2926: fix supervisor pulse consent model — default OFF, persist consent, never silently re-enable #2936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .agents/configs/aidevops.defaults.jsonc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,9 @@
"orchestration": {
// Enable the autonomous supervisor pulse scheduler.
// Dispatches workers, merges PRs, evaluates results.
// Requires explicit user consent during setup — defaults to OFF.
// Env override: AIDEVOPS_SUPERVISOR_PULSE
"supervisor_pulse": true,
"supervisor_pulse": false,

// Enable daily git pull --ff-only on clean repos.
// Env override: AIDEVOPS_REPO_SYNC
Expand Down
6 changes: 3 additions & 3 deletions .agents/configs/feature-toggles.conf.defaults
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,10 @@ manage_claude_config=true
# Supervisor Pulse
# -----------------------------------------------------------------------------
# Enable the autonomous supervisor pulse scheduler (dispatches workers,
# merges PRs, evaluates results). Set to false to disable autonomous
# orchestration entirely.
# merges PRs, evaluates results). Requires explicit user consent during
# setup — defaults to OFF. Set to true to enable autonomous orchestration.
# Env override: AIDEVOPS_SUPERVISOR_PULSE
supervisor_pulse=true
supervisor_pulse=false

# -----------------------------------------------------------------------------
# Repo Sync
Expand Down
225 changes: 147 additions & 78 deletions setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,78 +743,132 @@ main() {
fi
fi

# Enable supervisor pulse scheduler
# Supervisor pulse scheduler — consent-gated autonomous orchestration.
# Uses pulse-wrapper.sh which handles dedup, orphan cleanup, and RAM-based concurrency.
# macOS: launchd plist invoking wrapper | Linux: cron entry invoking wrapper
# The plist is ALWAYS regenerated on setup.sh to pick up config changes (env vars,
# thresholds). Only the first-install prompt is gated on _pulse_installed.
# Respects config: aidevops config set orchestration.supervisor_pulse false
if is_feature_enabled supervisor_pulse 2>/dev/null; then
local wrapper_script="$HOME/.aidevops/agents/scripts/pulse-wrapper.sh"
local pulse_label="com.aidevops.aidevops-supervisor-pulse"
local _aidevops_dir
_aidevops_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"

# Detect if pulse is already installed (any platform)
local _pulse_installed=false
if [[ "$(uname -s)" == "Darwin" ]]; then
local pulse_plist="$HOME/Library/LaunchAgents/${pulse_label}.plist"
if _launchd_has_agent "$pulse_label"; then
_pulse_installed=true
fi
fi
if [[ "$_pulse_installed" == "false" ]] && crontab -l 2>/dev/null | grep -qF "pulse-wrapper" 2>/dev/null; then
_pulse_installed=true
# thresholds). Only the first-install prompt is gated on consent state.
#
# Consent model (GH#2926):
# - Default OFF: supervisor_pulse defaults to false in all config layers
# - Explicit consent required: user must type "y" (prompt defaults to [y/N])
# - Consent persisted: written to config.jsonc so it survives updates
# - Never silently re-enabled: if config says false, skip entirely
# - Non-interactive: only installs if config explicitly says true
local wrapper_script="$HOME/.aidevops/agents/scripts/pulse-wrapper.sh"
local pulse_label="com.aidevops.aidevops-supervisor-pulse"
local _aidevops_dir
_aidevops_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"

# Read explicit user consent from config.jsonc (not merged defaults).
# Empty = user never configured this; "true"/"false" = explicit choice.
local _pulse_user_config=""
if type _jsonc_get_raw &>/dev/null && [[ -f "${JSONC_USER:-$HOME/.config/aidevops/config.jsonc}" ]]; then
_pulse_user_config=$(_jsonc_get_raw "${JSONC_USER:-$HOME/.config/aidevops/config.jsonc}" "orchestration.supervisor_pulse")
fi

# Also check legacy .conf user override
if [[ -z "$_pulse_user_config" && -f "${FEATURE_TOGGLES_USER:-$HOME/.config/aidevops/feature-toggles.conf}" ]]; then
local _legacy_val
_legacy_val=$(grep -E '^supervisor_pulse=' "${FEATURE_TOGGLES_USER:-$HOME/.config/aidevops/feature-toggles.conf}" 2>/dev/null | tail -1 | cut -d= -f2)
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 5, 2026

Choose a reason for hiding this comment

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

medium

The 2>/dev/null here should be removed. Since the existence of the file is checked on line 771, this grep should not fail with "file not found". Suppressing stderr here could mask other issues, like file permission errors, preventing the user from understanding why their legacy configuration is not being read.

Suggested change
_legacy_val=$(grep -E '^supervisor_pulse=' "${FEATURE_TOGGLES_USER:-$HOME/.config/aidevops/feature-toggles.conf}" 2>/dev/null | tail -1 | cut -d= -f2)
_legacy_val=$(grep -E '^supervisor_pulse=' "${FEATURE_TOGGLES_USER:-$HOME/.config/aidevops/feature-toggles.conf}" | tail -1 | cut -d= -f2)
References
  1. Avoid using 2>/dev/null to suppress errors on file operations if the file's existence has already been verified by a preceding check. This is redundant for 'file not found' errors and can mask other important issues like permissions problems.

if [[ -n "$_legacy_val" ]]; then
_pulse_user_config="$_legacy_val"
fi
fi

# Detect opencode binary location
local opencode_bin
opencode_bin=$(command -v opencode 2>/dev/null || echo "/opt/homebrew/bin/opencode")
# Also check env var override (highest priority)
if [[ -n "${AIDEVOPS_SUPERVISOR_PULSE:-}" ]]; then
_pulse_user_config="$AIDEVOPS_SUPERVISOR_PULSE"
fi

# First install: prompt user. Upgrades: always regenerate silently.
local _do_install=true
if [[ "$_pulse_installed" == "false" && -f "$wrapper_script" ]]; then
if [[ "$NON_INTERACTIVE" != "true" ]]; then
echo ""
echo "The supervisor pulse enables autonomous orchestration:"
echo " - Dispatches AI workers to implement tasks from GitHub issues"
echo " - Merges passing PRs, observes outcomes, files improvement issues"
echo " - 4-hourly strategic review (opus-tier) for queue health"
echo " - Circuit breaker pauses dispatch on consecutive failures"
echo ""
read -r -p "Enable supervisor pulse? [Y/n]: " enable_pulse
if [[ ! "$enable_pulse" =~ ^[Yy]?$ && -n "$enable_pulse" ]]; then
_do_install=false
print_info "Skipped. Enable later: ./setup.sh (re-run)"
# Determine action based on consent state
local _do_install=false
local _pulse_lower
_pulse_lower=$(echo "$_pulse_user_config" | tr '[:upper:]' '[:lower:]')

if [[ "$_pulse_lower" == "false" ]]; then
# User explicitly declined — never prompt, never install
_do_install=false
elif [[ "$_pulse_lower" == "true" ]]; then
# User explicitly consented — install/regenerate
_do_install=true
elif [[ -z "$_pulse_user_config" ]]; then
# No explicit config — fresh install or never configured
if [[ "$NON_INTERACTIVE" == "true" ]]; then
# Non-interactive: default OFF, do not install without consent
_do_install=false
elif [[ -f "$wrapper_script" ]]; then
# Interactive: prompt with default-no
echo ""
echo "The supervisor pulse enables autonomous orchestration."
echo "It will act under your GitHub identity and consume API credits:"
echo " - Dispatches AI workers to implement tasks from GitHub issues"
echo " - Creates PRs, merges passing PRs, files improvement issues"
echo " - 4-hourly strategic review (opus-tier) for queue health"
echo " - Circuit breaker pauses dispatch on consecutive failures"
echo ""
read -r -p "Enable supervisor pulse? [y/N]: " enable_pulse
if [[ "$enable_pulse" =~ ^[Yy]$ ]]; then
_do_install=true
# Record explicit consent
if type cmd_set &>/dev/null; then
cmd_set "orchestration.supervisor_pulse" "true" 2>/dev/null || true
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 5, 2026

Choose a reason for hiding this comment

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

high

The 2>/dev/null here should be removed. If cmd_set fails, the error should be visible to the user so they understand why their consent wasn't persisted. Silently failing means they will be re-prompted on the next run, which undermines a key goal of this change. The || true correctly prevents the script from exiting.

Suggested change
cmd_set "orchestration.supervisor_pulse" "true" 2>/dev/null || true
cmd_set "orchestration.supervisor_pulse" "true" || true
References
  1. In shell scripts with 'set -e' enabled, use '|| true' to prevent the script from exiting on a command's failure, but do not suppress stderr with '2>/dev/null'. This allows important error messages to be visible for debugging, which is crucial for a command that is persisting user consent.

fi
else
_do_install=false
# Record explicit decline so we never re-prompt on updates
if type cmd_set &>/dev/null; then
cmd_set "orchestration.supervisor_pulse" "false" 2>/dev/null || true
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 5, 2026

Choose a reason for hiding this comment

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

high

The 2>/dev/null here should be removed. If cmd_set fails, the error should be visible to the user so they understand why their choice to decline wasn't persisted. Silently failing means they will be re-prompted on the next run, which undermines a key goal of this change. The || true correctly prevents the script from exiting.

Suggested change
cmd_set "orchestration.supervisor_pulse" "false" 2>/dev/null || true
cmd_set "orchestration.supervisor_pulse" "false" || true
References
  1. In shell scripts with 'set -e' enabled, use '|| true' to prevent the script from exiting on a command's failure, but do not suppress stderr with '2>/dev/null'. This allows important error messages to be visible for debugging, which is crucial for a command that is persisting user consent.

fi
print_info "Skipped. Enable later: aidevops config set orchestration.supervisor_pulse true && ./setup.sh"
fi
elif [[ "$_pulse_installed" == "false" ]]; then
# Wrapper not deployed yet — skip (will install on next run after rsync)
_do_install=false
fi
fi

if [[ "$_do_install" == "true" && -f "$wrapper_script" ]]; then
mkdir -p "$HOME/.aidevops/logs"
# Guard: wrapper must exist
if [[ "$_do_install" == "true" && ! -f "$wrapper_script" ]]; then
# Wrapper not deployed yet — skip (will install on next run after rsync)
_do_install=false
fi

if [[ "$(uname -s)" == "Darwin" ]]; then
# macOS: use launchd plist with wrapper
local pulse_plist="$HOME/Library/LaunchAgents/${pulse_label}.plist"
# Detect if pulse is already installed (for upgrade messaging)
local _pulse_installed=false
if [[ "$(uname -s)" == "Darwin" ]]; then
local pulse_plist="$HOME/Library/LaunchAgents/${pulse_label}.plist"
if _launchd_has_agent "$pulse_label"; then
_pulse_installed=true
fi
fi
if [[ "$_pulse_installed" == "false" ]] && crontab -l 2>/dev/null | grep -qF "pulse-wrapper" 2>/dev/null; then
_pulse_installed=true
fi

# Unload old plist if upgrading
if _launchd_has_agent "$pulse_label"; then
launchctl unload "$pulse_plist" 2>/dev/null || true
pkill -f 'Supervisor Pulse' 2>/dev/null || true
fi
# Detect opencode binary location
local opencode_bin
opencode_bin=$(command -v opencode 2>/dev/null || echo "/opt/homebrew/bin/opencode")

# Also clean up old label if present
local old_plist="$HOME/Library/LaunchAgents/com.aidevops.supervisor-pulse.plist"
if [[ -f "$old_plist" ]]; then
launchctl unload "$old_plist" 2>/dev/null || true
rm -f "$old_plist"
fi
if [[ "$_do_install" == "true" ]]; then
mkdir -p "$HOME/.aidevops/logs"

if [[ "$(uname -s)" == "Darwin" ]]; then
# macOS: use launchd plist with wrapper
local pulse_plist="$HOME/Library/LaunchAgents/${pulse_label}.plist"

# Write the plist (always regenerated to pick up config changes)
cat >"$pulse_plist" <<PLIST
# Unload old plist if upgrading
if _launchd_has_agent "$pulse_label"; then
launchctl unload "$pulse_plist" 2>/dev/null || true
pkill -f 'Supervisor Pulse' 2>/dev/null || true
fi

# Also clean up old label if present
local old_plist="$HOME/Library/LaunchAgents/com.aidevops.supervisor-pulse.plist"
if [[ -f "$old_plist" ]]; then
launchctl unload "$old_plist" 2>/dev/null || true
rm -f "$old_plist"
fi

# Write the plist (always regenerated to pick up config changes)
cat >"$pulse_plist" <<PLIST
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
Expand Down Expand Up @@ -853,27 +907,42 @@ main() {
</plist>
PLIST

if launchctl load "$pulse_plist" 2>/dev/null; then
if [[ "$_pulse_installed" == "true" ]]; then
print_info "Supervisor pulse updated (launchd config regenerated)"
else
print_info "Supervisor pulse enabled (launchd, every 2 min)"
fi
if launchctl load "$pulse_plist" 2>/dev/null; then
if [[ "$_pulse_installed" == "true" ]]; then
print_info "Supervisor pulse updated (launchd config regenerated)"
else
print_warning "Failed to load supervisor pulse LaunchAgent"
print_info "Supervisor pulse enabled (launchd, every 2 min)"
fi
else
# Linux: use cron entry with wrapper
# Remove old-style cron entries (direct opencode invocation)
(
crontab -l 2>/dev/null | grep -v 'aidevops: supervisor-pulse'
echo "*/2 * * * * OPENCODE_BIN=${opencode_bin} PULSE_DIR=${_aidevops_dir} /bin/bash ${wrapper_script} >> $HOME/.aidevops/logs/pulse-wrapper.log 2>&1 # aidevops: supervisor-pulse"
) | crontab - 2>/dev/null || true
if crontab -l 2>/dev/null | grep -qF "aidevops: supervisor-pulse"; then
print_info "Supervisor pulse enabled (cron, every 2 min). Disable: crontab -e and remove the supervisor-pulse line"
else
print_warning "Failed to install supervisor pulse cron entry. See runners.md for manual setup."
fi
print_warning "Failed to load supervisor pulse LaunchAgent"
fi
else
# Linux: use cron entry with wrapper
# Remove old-style cron entries (direct opencode invocation)
(
crontab -l 2>/dev/null | grep -v 'aidevops: supervisor-pulse'
echo "*/2 * * * * OPENCODE_BIN=${opencode_bin} PULSE_DIR=${_aidevops_dir} /bin/bash ${wrapper_script} >> $HOME/.aidevops/logs/pulse-wrapper.log 2>&1 # aidevops: supervisor-pulse"
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 5, 2026

Choose a reason for hiding this comment

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

security-medium medium

The setup.sh script constructs a crontab entry by concatenating variables opencode_bin and _aidevops_dir without proper quoting or sanitization. If the repository is cloned into a directory with a malicious name (e.g., containing shell metacharacters like ; or $()), or if the opencode binary path contains such characters, an attacker could achieve arbitrary command execution when the cron job runs. This is a classic command injection vulnerability where untrusted input (the directory name) is used to build a command that is later executed by a shell.

Suggested change
echo "*/2 * * * * OPENCODE_BIN=${opencode_bin} PULSE_DIR=${_aidevops_dir} /bin/bash ${wrapper_script} >> $HOME/.aidevops/logs/pulse-wrapper.log 2>&1 # aidevops: supervisor-pulse"
echo "*/2 * * * * OPENCODE_BIN='${opencode_bin}' PULSE_DIR='${_aidevops_dir}' /bin/bash '${wrapper_script}' >> '$HOME/.aidevops/logs/pulse-wrapper.log' 2>&1 # aidevops: supervisor-pulse"

) | crontab - 2>/dev/null || true
if crontab -l 2>/dev/null | grep -qF "aidevops: supervisor-pulse"; then
print_info "Supervisor pulse enabled (cron, every 2 min). Disable: crontab -e and remove the supervisor-pulse line"
else
print_warning "Failed to install supervisor pulse cron entry. See runners.md for manual setup."
fi
fi
elif [[ "$_pulse_lower" == "false" && "$_pulse_installed" == "true" ]]; then
# User explicitly disabled but pulse is still installed — clean up
if [[ "$(uname -s)" == "Darwin" ]]; then
local pulse_plist="$HOME/Library/LaunchAgents/${pulse_label}.plist"
if _launchd_has_agent "$pulse_label"; then
launchctl unload "$pulse_plist" 2>/dev/null || true
rm -f "$pulse_plist"
pkill -f 'Supervisor Pulse' 2>/dev/null || true
print_info "Supervisor pulse disabled (launchd agent removed per config)"
fi
else
if crontab -l 2>/dev/null | grep -qF "pulse-wrapper" 2>/dev/null; then
crontab -l 2>/dev/null | grep -v 'aidevops: supervisor-pulse' | crontab - 2>/dev/null || true
print_info "Supervisor pulse disabled (cron entry removed per config)"
fi
fi
fi
Expand Down Expand Up @@ -976,8 +1045,8 @@ PLIST
echo "• Session miner - Extracts learning from past sessions"
echo "• Circuit breaker - Pauses dispatch on consecutive failures"
echo ""
echo " The supervisor pulse was offered during setup. If skipped, enable later:"
echo " See: ~/.aidevops/agents/scripts/commands/runners.md 'Pulse Scheduler Setup'"
echo " Supervisor pulse (autonomous orchestration) requires explicit consent."
echo " Enable: aidevops config set orchestration.supervisor_pulse true && ./setup.sh"
echo ""
echo " Run /onboarding in your AI assistant to configure services interactively."
echo ""
Expand Down
Loading