Skip to content

t1480: fix pulse sessions overlapping — run wrapper from repo path (GH#4513)#4532

Closed
alex-solovyev wants to merge 1 commit intomainfrom
bugfix/pulse-overlap-fix
Closed

t1480: fix pulse sessions overlapping — run wrapper from repo path (GH#4513)#4532
alex-solovyev wants to merge 1 commit intomainfrom
bugfix/pulse-overlap-fix

Conversation

@alex-solovyev
Copy link
Collaborator

@alex-solovyev alex-solovyev commented Mar 14, 2026

Summary

  • Root cause: The cron entry ran the deployed ~/.aidevops/agents/scripts/pulse-wrapper.sh instead of the repo version. Fixes merged to main (like the flock guard from GH#4409/t1478) were not effective until the next setup.sh run, creating a deployment gap that allowed 6 concurrent pulse sessions.
  • Fix: Change wrapper_script in setup.sh to point to ${_pulse_repo_dir}/.agents/scripts/pulse-wrapper.sh (the repo version). Since the canonical repo directory is always on main per the worktree-first workflow, this ensures fixes are immediately effective after merge.
  • After merge: Run ./setup.sh once to update the cron entry to use the new repo path. Future fixes will be effective immediately without needing setup.sh.

Changes

File Change
setup.sh Changed wrapper_script from deployed path to repo path; updated guard comment
.agents/scripts/pulse-wrapper.sh Updated header comment to reflect new invocation path

Verification

  • shellcheck setup.sh — zero violations
  • shellcheck .agents/scripts/pulse-wrapper.sh — zero violations
  • The cron entry will now reference the repo version, ensuring the flock guard (GH#4409) and all future fixes are immediately effective

Closes #4513

Summary by CodeRabbit

  • Chores
    • Updated pulse wrapper configuration to execute directly from the repository, enabling immediate deployment of fixes after merge without requiring setup.sh re-execution.
    • Updated documentation clarifying that the pulse wrapper is invoked by cron (Linux) or launchd (macOS) every 120 seconds, now running from the repo source directly.

…nt gap (GH#4513)

The cron/launchd entry previously pointed to the deployed copy at
~/.aidevops/agents/scripts/pulse-wrapper.sh, which is only updated
when setup.sh runs. This caused a deployment gap where fixes merged
to main (like the flock guard from GH#4409) were not effective until
the next manual setup.sh run — resulting in 6 concurrent pulse
sessions despite the fix being in the repo.

Change the wrapper_script path to use the repo version directly
($PULSE_DIR/.agents/scripts/pulse-wrapper.sh). The canonical repo
directory is always on main per the worktree-first workflow, so the
repo version is always the latest merged code.

Closes #4513
@gemini-code-assist
Copy link

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions github-actions bot added the bug Auto-created from TODO.md tag label Mar 14, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 14, 2026

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

Updates pulse-wrapper.sh invocation documentation and modifies setup.sh to reference the repo worktree version of the wrapper script directly, bypassing the deployed copy. This enables immediate application of pulse fixes post-merge without requiring setup.sh re-execution.

Changes

Cohort / File(s) Summary
Pulse Wrapper Documentation
.agents/scripts/pulse-wrapper.sh
Updated invocation documentation: clarified that script is called by cron (Linux) or launchd (macOS) every 120s from the repo, not a deployed copy, enabling immediate fix application.
Setup Script Path Refactoring
setup.sh
Replaced deployed pulse-wrapper.sh path with repo worktree reference (${_pulse_repo_dir}/.agents/scripts/pulse-wrapper.sh). Updated guard messaging from "wrapper must exist" to "wrapper must exist in the repo" with corresponding failure messages reflecting repo-based expectations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

bug

Suggested reviewers

  • marcusquinn

Poem

🔄 A pulse once spawned by launchd's tide,
Now runs straight from the repo guide,
No deploy delay, no stale state pain—
Fresh fixes flow through the merge lane! 🚀

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: running the pulse wrapper from the repo path instead of deployed copy to fix session overlaps (GH#4513).
Linked Issues check ✅ Passed The PR addresses the core issue: it eliminates the deployment gap preventing the flock guard from taking effect immediately after merge by ensuring cron runs the repo version.
Out of Scope Changes check ✅ Passed All changes are scoped to the documented objectives: updating wrapper_script path in setup.sh and updating documentation in pulse-wrapper.sh; no unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/pulse-overlap-fix
📝 Coding Plan
  • Generate coding plan for human review comments

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.

@marcusquinn
Copy link
Owner

Closing: superseded by PR #4525 which was already merged. PR #4525 addressed GH#4513 with the flock→mkdir-based atomic lock approach. This PR's approach (running wrapper from repo path) is a different fix for the same issue — if still needed, it should be filed as a separate follow-up issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Auto-created from TODO.md tag

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: pulse sessions overlapping — launchd fires new pulse before prior exits

2 participants