Skip to content

t4947: add cross-machine dispatch dedup via assignee check + pulse jitter #4948

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 2 commits into from
Mar 15, 2026
Merged

t4947: add cross-machine dispatch dedup via assignee check + pulse jitter #4948

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39f4875
fix: prevent duplicate dispatch across runners via assignee check + j…
marcusquinn Mar 15, 2026
540ac93
fix: validate PULSE_JITTER_MAX as numeric, use read -ra for assignee …
marcusquinn Mar 15, 2026
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
11 changes: 9 additions & 2 deletions .agents/scripts/commands/pulse.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,20 @@ Check external contributor gate before ANY merge (see Pre-merge checks below).
For each unassigned, non-blocked issue with no open PR and no active worker:

```bash
# Dedup guard (MANDATORY)
# Dedup guard (MANDATORY — all three checks required)
source ~/.aidevops/agents/scripts/pulse-wrapper.sh
RUNNER_USER=$(gh api user --jq '.login' 2>/dev/null || whoami)

# 1. Local process dedup (same machine only)
if has_worker_for_repo_issue NUMBER SLUG; then continue; fi
if ~/.aidevops/agents/scripts/dispatch-dedup-helper.sh is-duplicate "Issue #NUMBER: TITLE"; then continue; fi

# 2. Cross-machine assignee dedup (checks GitHub — visible to ALL runners)
# This is the primary guard against duplicate dispatch across machines.
# If another runner already assigned themselves, skip this issue.
if ~/.aidevops/agents/scripts/dispatch-dedup-helper.sh is-assigned NUMBER SLUG "$RUNNER_USER"; then continue; fi

# Assign and dispatch
RUNNER_USER=$(gh api user --jq '.login' 2>/dev/null || whoami)
gh issue edit NUMBER --repo SLUG --add-assignee "$RUNNER_USER" --add-label "status:queued" 2>/dev/null || true

~/.aidevops/agents/scripts/headless-runtime-helper.sh run \
Expand Down
81 changes: 80 additions & 1 deletion .agents/scripts/dispatch-dedup-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,69 @@
return 1
}

#######################################
# Check if a GitHub issue is already assigned to someone else.
#
# This is the primary cross-machine dedup guard. Process-based checks
# (is_duplicate, has_worker_for_repo_issue) only see local processes —
# they miss workers running on other machines. The GitHub assignee is
# the single source of truth visible to all runners.
#
# Args:
# $1 = issue number
# $2 = repo slug (owner/repo)
# $3 = (optional) current runner login — if assigned to self, not a dup
# Returns:
# exit 0 if assigned to someone else (do NOT dispatch)
# exit 1 if unassigned or assigned to self (safe to dispatch)
# Outputs: assignee info on stdout if assigned
#######################################
is_assigned() {
local issue_number="$1"
local repo_slug="$2"
local self_login="${3:-}"

if [[ -z "$issue_number" || -z "$repo_slug" ]]; then
# Missing args — cannot check, allow dispatch
return 1
fi

# Validate issue number is numeric
if [[ ! "$issue_number" =~ ^[0-9]+$ ]]; then
return 1
fi

# Query GitHub for current assignees
local assignees
assignees=$(gh issue view "$issue_number" --repo "$repo_slug" \
--json assignees --jq '[.assignees[].login] | join(",")' 2>/dev/null) || assignees=""

if [[ -z "$assignees" ]]; then
# No assignees — safe to dispatch
return 1
fi

# If assigned to self, not a duplicate
if [[ -n "$self_login" ]]; then
# Check if ALL assignees are self (could be multiple)
local dominated_by_self=true
local IFS=','

Check warning on line 327 in .agents/scripts/dispatch-dedup-helper.sh

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

.agents/scripts/dispatch-dedup-helper.sh#L327 

The special variable IFS affects how splitting takes place when expanding unquoted variables.
for assignee in $assignees; do
if [[ "$assignee" != "$self_login" ]]; then
dominated_by_self=false
break
fi
done
unset IFS

@gemini-code-assist gemini-code-assist Bot Mar 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

While using local IFS=',' with a for ... in loop is a common idiom for iterating over comma-separated values, it can be brittle if values contain whitespace. A more robust method in bash is to use read -ra to parse the values into an array. This avoids unintended word splitting and handles various edge cases more gracefully, aligning with best practices for parsing delimited data in shell scripts.

Suggested change
local IFS=','
for assignee in $assignees; do
if [[ "$assignee" != "$self_login" ]]; then
dominated_by_self=false
break
fi
done
unset IFS
local IFS=','
read -ra assignee_array <<< "$assignees"
unset IFS
for assignee in "${assignee_array[@]}"; do
if [[ "$assignee" != "$self_login" ]]; then
dominated_by_self=false
break
fi
done
References
  1. In shell scripts, the read command's delimiter (set via IFS) should always match the delimiter of the input data to ensure correct parsing. This suggestion introduces read for more robust parsing.

@marcusquinn marcusquinn Mar 15, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 540ac93 — switched to IFS=',' read -ra assignee_array for robust parsing.

if [[ "$dominated_by_self" == "true" ]]; then
return 1
fi
fi

printf 'ASSIGNED: issue #%s in %s is assigned to %s\n' "$issue_number" "$repo_slug" "$assignees"
return 0
}

#######################################
# Show help
#######################################
Expand All @@ -288,6 +351,8 @@
Usage:
dispatch-dedup-helper.sh extract-keys <title> Extract dedup keys from a title
dispatch-dedup-helper.sh is-duplicate <title> Check if already running (exit 0=dup, 1=safe)
dispatch-dedup-helper.sh is-assigned <issue> <slug> [self-login]
Check if issue is assigned (exit 0=assigned, 1=free)
dispatch-dedup-helper.sh list-running-keys List keys for all running workers
dispatch-dedup-helper.sh normalize <title> Normalize a title for comparison
dispatch-dedup-helper.sh help Show this help
Expand All @@ -298,12 +363,19 @@
# Output: issue-2300
# task-t1337

# Check before dispatching
# Check before dispatching (local process dedup)
if dispatch-dedup-helper.sh is-duplicate "Issue #2300: Fix auth"; then
echo "Already running — skip dispatch"
else
echo "Safe to dispatch"
fi

# Check before dispatching (cross-machine assignee dedup)
if dispatch-dedup-helper.sh is-assigned 2300 owner/repo mylogin; then
echo "Assigned to someone else — skip dispatch"
else
echo "Unassigned or assigned to self — safe to dispatch"
fi
HELP
return 0
}
Expand All @@ -330,6 +402,13 @@
}
is_duplicate "$1"
;;
is-assigned)
[[ $# -lt 2 ]] && {
echo "Error: is-assigned requires <issue-number> <repo-slug> [self-login]" >&2
return 1
}
is_assigned "$1" "$2" "${3:-}"
;;
list-running-keys)
list_running_keys
;;
Expand Down
20 changes: 20 additions & 0 deletions .agents/scripts/pulse-wrapper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,26 @@ set -euo pipefail
#######################################
export PATH="/bin:/usr/bin:/usr/local/bin:/opt/homebrew/bin:${PATH}"

#######################################
# Startup jitter — desynchronise concurrent pulse instances
#
# When multiple runners share the same launchd interval (120s), their
# pulses fire simultaneously, creating a race window where both evaluate
# the same issue before either can self-assign. A random 0-30s delay at
# startup staggers the pulses so the first runner to wake assigns the
# issue before the second runner evaluates it.
#
# PULSE_JITTER_MAX: max jitter in seconds (default 30, set to 0 to disable)
#######################################
PULSE_JITTER_MAX="${PULSE_JITTER_MAX:-30}"
if [[ "$PULSE_JITTER_MAX" -gt 0 ]]; then
# $RANDOM is 0-32767; modulo gives 0 to PULSE_JITTER_MAX
jitter_seconds=$((RANDOM % (PULSE_JITTER_MAX + 1)))
if [[ "$jitter_seconds" -gt 0 ]]; then
sleep "$jitter_seconds"
fi
fi
Comment on lines +77 to +84

@gemini-code-assist gemini-code-assist Bot Mar 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The script assumes PULSE_JITTER_MAX is an integer. If a non-integer value is provided (e.g., from an environment variable), the script could fail on the arithmetic comparison or calculation, especially with set -e. It's safer to validate that the variable is numeric before using it in an arithmetic context.

Suggested change
PULSE_JITTER_MAX="${PULSE_JITTER_MAX:-30}"
if [[ "$PULSE_JITTER_MAX" -gt 0 ]]; then
# $RANDOM is 0-32767; modulo gives 0 to PULSE_JITTER_MAX
jitter_seconds=$((RANDOM % (PULSE_JITTER_MAX + 1)))
if [[ "$jitter_seconds" -gt 0 ]]; then
sleep "$jitter_seconds"
fi
fi
PULSE_JITTER_MAX="${PULSE_JITTER_MAX:-30}"
if [[ "$PULSE_JITTER_MAX" =~ ^[0-9]+$ && "$PULSE_JITTER_MAX" -gt 0 ]]; then
# $RANDOM is 0-32767; modulo gives 0 to PULSE_JITTER_MAX
jitter_seconds=$((RANDOM % (PULSE_JITTER_MAX + 1)))
if [[ "$jitter_seconds" -gt 0 ]]; then
sleep "$jitter_seconds"
fi
fi
References
  1. When parsing delimited data from an untrusted source in a shell script, validate numeric fields before using them in calculations. This provides defense-in-depth against injection attacks that could result from delimiter shifting caused by malicious data. This applies to environment variables as untrusted input.

@marcusquinn marcusquinn Mar 15, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 540ac93 — added numeric validation with [[ "$PULSE_JITTER_MAX" =~ ^[0-9]+$ ]] before the arithmetic comparison.


# Use ${BASH_SOURCE[0]:-$0} for shell portability — BASH_SOURCE is undefined
# in zsh, which is the MCP shell environment. This fallback ensures SCRIPT_DIR
# resolves correctly whether the script is executed directly (bash) or sourced
Expand Down
Loading