Skip to content

t1439: fix pulse status drift and worker cap reporting#4140

Merged
marcusquinn merged 3 commits intomainfrom
bugfix/pulse-status-max-workers
Mar 11, 2026
Merged

t1439: fix pulse status drift and worker cap reporting#4140
marcusquinn merged 3 commits intomainfrom
bugfix/pulse-status-max-workers

Conversation

@marcusquinn
Copy link
Owner

@marcusquinn marcusquinn commented Mar 11, 2026

Summary

  • read the wrapper log before the legacy pulse log so pulse status reflects the latest run metadata.
  • load config helpers safely in the wrapper and fall back cleanly so max worker reporting stays available.

Closes #4139

Summary by CodeRabbit

  • Chores
    • Improved session pulse tracking by checking multiple log sources and reliably reporting "never" when no pulse timestamp is found.
    • Added separate wrapper-level logging to isolate wrapper start events from main logs.
    • Added a defensive fallback for missing configuration so the process continues with a one-time warning instead of failing.

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

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 402 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Wed Mar 11 16:03:33 UTC 2026: Code review monitoring started
Wed Mar 11 16:03:34 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 402

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 402
  • VULNERABILITIES: 0

Generated on: Wed Mar 11 16:03:37 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4ba15355-922f-4773-a143-c3108ad6a188

📥 Commits

Reviewing files that changed from the base of the PR and between d8621a7 and e195fe3.

📒 Files selected for processing (1)
  • .agents/scripts/pulse-wrapper.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • .agents/scripts/pulse-wrapper.sh

Walkthrough

The PR adds WRAPPER_LOGFILE and updates pulse-session helper to prefer wrapper logs when determining last pulse time (returns "never" if not found). The wrapper script gains a defensive config_get fallback and routes pulse start logging to WRAPPER_LOGFILE.

Changes

Cohort / File(s) Summary
Pulse Session Helper
.agents/scripts/pulse-session-helper.sh
Added readonly WRAPPER_LOGFILE, updated get_last_pulse_time to check WRAPPER_LOGFILE then LOGFILE for the latest "Starting pulse at" timestamp, and return "never" if none found. Added shellcheck source suppression while sourcing config-helper.sh.
Pulse Wrapper / Config Fallback
.agents/scripts/pulse-wrapper.sh
Introduced a defensive local config_get(key, default) fallback that emits a one-time warning and returns defaults if external config helper is missing. Added/initialized WRAPPER_LOGFILE and wrote pulse start entries to WRAPPER_LOGFILE in run_pulse.

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

A wrapper log hums where pulses tread,
When helpers wander off, defaults are read.
Timestamps found, or "never" we show —
Small guards keep the heartbeat aglow. 💫

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references pulse status drift and worker cap reporting, which align with the main objectives of fixing pulse status reporting and honoring max worker configuration in the linked issue.
Linked Issues check ✅ Passed The code changes implement all primary objectives: reading wrapper log before legacy pulse log to fix status drift [#4139], adding fallback config handling for safe wrapper operation, and routing pulse start logs appropriately.
Out of Scope Changes check ✅ Passed All changes are directly scoped to addressing pulse status reporting accuracy and wrapper configuration handling as specified in issue #4139, with no extraneous modifications.

✏️ 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-status-max-workers

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
Contributor

@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: 1

🧹 Nitpick comments (1)
.agents/scripts/pulse-wrapper.sh (1)

58-65: Silent fallback obscures config-helper load failures.

The fallback config_get correctly enables graceful degradation when config-helper.sh fails to load, but it activates without any log entry. When this fallback is used, MAX_WORKERS_CAP and QUALITY_DEBT_CAP_PCT silently revert to hardcoded defaults (8 and 30), potentially causing subtle behavioral differences that are difficult to diagnose.

Consider adding a one-time log entry when the fallback is activated:

♻️ Proposed enhancement for observability
 if ! type config_get >/dev/null 2>&1; then
+	echo "[pulse-wrapper] WARN: config-helper.sh unavailable — using default config values" >>"${HOME}/.aidevops/logs/pulse.log"
 	config_get() {
 		local _key="$1"
 		local default_value="$2"
 		printf '%s\n' "$default_value"
 		return 0
 	}
 fi

As per coding guidelines: "Automation scripts - focus on: Clear logging and feedback".

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

In @.agents/scripts/pulse-wrapper.sh around lines 58 - 65, The fallback
config_get implementation currently returns defaults silently; modify it to emit
a one-time warning when used so callers like MAX_WORKERS_CAP and
QUALITY_DEBT_CAP_PCT don't silently fall back: detect when config_get is not
available, install the fallback config_get function that prints the default
value but also writes a single warning message (to stderr or the script logger)
the first time it is invoked (use a static/local flag inside config_get to
ensure the log is emitted only once), include the function name "config_get" and
mention which keys defaulted when logging to aid debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.agents/scripts/pulse-session-helper.sh:
- Line 31: The "Starting pulse at" metadata is being written to LOGFILE
(pulse.log) instead of the wrapper-specific WRAPPER_LOGFILE, so update the
pulse-wrapper.sh code that emits that message (the lines that currently echo/log
"Starting pulse at"—search for that exact string) to append to the
WRAPPER_LOGFILE variable instead of LOGFILE; ensure it uses the same append
semantics and directory creation used elsewhere (matching how WRAPPER_LOGFILE is
defined in pulse-session-helper.sh) so wrapper-specific metadata lands in
${WRAPPER_LOGFILE} (not ${LOGFILE}).

---

Nitpick comments:
In @.agents/scripts/pulse-wrapper.sh:
- Around line 58-65: The fallback config_get implementation currently returns
defaults silently; modify it to emit a one-time warning when used so callers
like MAX_WORKERS_CAP and QUALITY_DEBT_CAP_PCT don't silently fall back: detect
when config_get is not available, install the fallback config_get function that
prints the default value but also writes a single warning message (to stderr or
the script logger) the first time it is invoked (use a static/local flag inside
config_get to ensure the log is emitted only once), include the function name
"config_get" and mention which keys defaulted when logging to aid debugging.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a607c01a-bcbb-4003-a8a6-692d3c9e61a2

📥 Commits

Reviewing files that changed from the base of the PR and between 5b52dfa and d8621a7.

📒 Files selected for processing (2)
  • .agents/scripts/pulse-session-helper.sh
  • .agents/scripts/pulse-wrapper.sh

@gemini-code-assist
Copy link

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the reliability and robustness of the pulse session management. It addresses issues where pulse status could drift by prioritizing the most recent log data and improves system stability by implementing defensive programming for configuration loading, ensuring critical features like worker capacity reporting remain functional under various conditions.

Highlights

  • Pulse Status Accuracy: The system now reads the wrapper log before the legacy pulse log to ensure the pulse status accurately reflects the latest run metadata.
  • Robust Configuration Loading: Configuration helpers are now loaded safely in the wrapper script with a clean fallback mechanism, ensuring that max worker reporting remains available even if dependencies are not fully loaded.
Changelog
  • .agents/scripts/pulse-session-helper.sh
    • Introduced WRAPPER_LOGFILE constant for the pulse wrapper log.
    • Modified get_last_pulse_time to check pulse-wrapper.log before pulse.log for the latest pulse timestamp.
  • .agents/scripts/pulse-wrapper.sh
    • Sourced config-helper.sh to enable configuration loading.
    • Implemented a fallback config_get function to ensure worker cap reporting remains functional even if config-helper.sh fails to load.
Activity
  • No human activity (comments, reviews) was detected on this pull request yet.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@marcusquinn
Copy link
Owner Author

Dispatching fix worker.

  • Model: default headless rotation (anthropic/claude-sonnet-4-6 or openai/gpt-5.3-codex)
  • Branch: bugfix/pulse-status-max-workers
  • Scope: address changes-requested feedback and stabilize CI
  • Attempt: 1 of 1
  • Direction: push fixes to existing branch and keep issue linkage intact

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces two fixes to improve the reliability of pulse session tracking. First, it addresses status drift by prioritizing the wrapper log over the legacy pulse log, ensuring the latest run metadata is always reflected. Second, it enhances robustness by adding a fallback mechanism for configuration loading, which keeps worker cap reporting available even if helper scripts are missing. The changes are logical and well-implemented. I have one minor suggestion to improve code clarity in the new fallback function.

Comment on lines +60 to +61
local _key="$1"
local default_value="$2"

Choose a reason for hiding this comment

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

medium

The _key variable is assigned but never used. It can be removed to make the code cleaner and avoid potential linter warnings.

Suggested change
local _key="$1"
local default_value="$2"
local default_value="$2"

@github-actions
Copy link

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 402 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Wed Mar 11 16:48:54 UTC 2026: Code review monitoring started
Wed Mar 11 16:48:55 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 402

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 402
  • VULNERABILITIES: 0

Generated on: Wed Mar 11 16:48:57 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@marcusquinn
Copy link
Owner Author

Addressed the remaining CodeRabbit feedback in : the fallback path now emits a one-time warning only when it is actually used, and the message explicitly says default config values are being applied.

@marcusquinn
Copy link
Owner Author

Addressed the remaining CodeRabbit feedback in .agents/scripts/pulse-wrapper.sh: the fallback config_get path now emits a one-time warning only when it is actually used, and the message explicitly says default config values are being applied.

@github-actions
Copy link

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 402 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Wed Mar 11 16:56:17 UTC 2026: Code review monitoring started
Wed Mar 11 16:56:18 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 402

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 402
  • VULNERABILITIES: 0

Generated on: Wed Mar 11 16:56:20 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@sonarqubecloud
Copy link

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.

t1439: Fix pulse status last-pulse reporting and honor configured max worker cap

1 participant