Skip to content

chore(ruvocal): refresh package-lock snapshot#1612

Open
mwebsterjr wants to merge 5 commits intoruvnet:mainfrom
mwebsterjr:main
Open

chore(ruvocal): refresh package-lock snapshot#1612
mwebsterjr wants to merge 5 commits intoruvnet:mainfrom
mwebsterjr:main

Conversation

@mwebsterjr
Copy link
Copy Markdown

No description provided.

Context: Resolve Windows daemon lifecycle issues and fix generated hook command path handling.

Changes: daemon.ts process launch hardening; settings-generator.ts relative hook command fixes; worker-daemon.ts signal/PID handling; package-lock.json refresh.

Impact: Improves Windows stability for daemon start/stop and hook execution.

Validation: Reviewed staged diff and committed nested repository changes.
Copilot AI review requested due to automatic review settings April 14, 2026 19:47
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refreshes npm lockfile snapshots (root and ruvocal) and also adjusts the V3 CLI daemon’s Windows behavior (signal handling, background spawn strategy, and PID ownership) plus a small Windows hook command generation tweak.

Changes:

  • Update package-lock.json snapshots (root and ruflo/src/ruvocal/) and bump root version metadata to 3.5.48.
  • Improve Windows daemon robustness: ignore SIGHUP on Windows, avoid shell: true, always spawn detached, and adjust PID singleton checks.
  • Add a new .claude “dual-mode” skill doc.

Reviewed changes

Copilot reviewed 4 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
v3/@claude-flow/cli/src/services/worker-daemon.ts Adjust signal handling (SIGHUP) and PID singleton detection to better support detached Windows daemons.
v3/@claude-flow/cli/src/init/settings-generator.ts Update Windows hook command generation to use relative script paths.
v3/@claude-flow/cli/src/commands/daemon.ts Change background daemon spawn behavior on Windows (no shell, detached=true) and shift PID lifecycle ownership to the child.
ruflo/src/ruvocal/package-lock.json Refresh ruvocal lock snapshot (dependency metadata changes).
package-lock.json Refresh root lock snapshot and dependency graph.
.claude/skills/dual-mode/SKILL.md Add documentation for dual-mode orchestration commands.
Files not reviewed (1)
  • ruflo/src/ruvocal/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 462 to +466
try {
const pid = parseInt(readFileSync(this.pidFile, 'utf-8').trim(), 10);
if (isNaN(pid)) return null;
// If PID file points to this process, it is not "another" daemon.
if (pid === process.pid) return null;
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

New singleton logic treats a PID file pointing at process.pid as “no other daemon”. Please add a unit test covering this scenario (PID file pre-created with current PID) to ensure start() does not incorrectly short-circuit and to prevent regressions around the PID ownership changes.

Copilot uses AI. Check for mistakes.
Comment on lines +323 to +327
// Detached Windows daemons can receive SIGHUP during parent/session changes.
// Ignore it there so background workers stay alive.
if (process.platform !== 'win32') {
process.on('SIGHUP', shutdown);
}
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

Windows now skips registering a SIGHUP shutdown handler. Given the platform-specific behavior, add a test (or a platform-guarded test) to lock in that Windows does not stop the daemon on SIGHUP while POSIX platforms do (or vice versa, depending on intended behavior).

Copilot uses AI. Check for mistakes.
Comment thread v3/@claude-flow/cli/src/commands/daemon.ts
Copy link
Copy Markdown

@xkonjin xkonjin left a comment

Choose a reason for hiding this comment

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

Review summary:

  • Good change to avoid shell-mode spawn on Windows; removing shell:true and using explicit args prevents quoted-path breakage and stale PID files.
  • The SIGHUP guard for Windows (ignoring it) and the PID self-check in worker-daemon.ts are both solid fixes.
  • The dual-mode SKILL.md addition is clear and well-structured.
  • One concern: the root package-lock.json downgrades version from 3.5.80 to 3.5.46. Was this intentional, or an artifact of the refresh? If accidental, please restore the higher version before merge.

No blocking security or bug issues in the code changes.

Copy link
Copy Markdown

@xkonjin xkonjin left a comment

Choose a reason for hiding this comment

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

Review summary:

  • Good fix removing shell:true on Windows to prevent quoted-path breakage.
  • SIGHUP guard and PID self-check in worker-daemon.ts are solid.
  • Dual-mode skill docs are well structured.
  • Note: root package-lock.json shows a version downgrade (3.5.80 -> 3.5.46). Please verify if that's intended.

LGTM.

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.

3 participants