Skip to content

fix check issue in windows#18

Merged
newbe36524 merged 1 commit intomainfrom
winfix
Feb 13, 2026
Merged

fix check issue in windows#18
newbe36524 merged 1 commit intomainfrom
winfix

Conversation

@newbe36524
Copy link
Contributor

@newbe36524 newbe36524 commented Feb 13, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Added code signing for Windows artifacts using Azure integration with automatic verification and failure notifications
    • Implemented single-instance lock to prevent multiple app instances from running simultaneously
    • Suppressed console windows on Windows during background script execution for a cleaner user experience
  • Bug Fixes

    • Fixed Windows dependency check script to auto-exit after completion
    • Improved process timeout handling with enhanced logging and termination logic
    • Enhanced cross-platform consistency for process management and error handling

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

📝 Walkthrough

Walkthrough

This pull request introduces four major features: Windows code signing with Azure Artifact Signing and signature verification in CI/CD, Electron single-instance lock enforcement across platforms, Windows console window suppression for spawned child processes, and Windows dependency check script auto-exit handling with improved timeout management.

Changes

Cohort / File(s) Summary
Code Signing Infrastructure
.github/workflows/build.yml, electron-builder.yml
Adds ENABLE_CODE_SIGNING environment toggle, signs Windows artifacts (.exe, .appx), verifies signatures using signtool, and sends failure notifications to HagiCode. Introduces signingHashAlgorithms configuration for SHA-256 signing.
Code Signing Scripts
scripts/sign-artifact.js, scripts/verify-signature.js
Implements artifact signing via Azure Trusted Signing, Key Vault, or signtool fallback with validation and error handling. Adds per-file and directory-wide signature verification with CI-friendly logging and strict mode support.
Single Instance Lock
src/main/main.ts, openspec/specs/electron-app/spec.md
Enforces single-instance lock using app.requestSingleInstanceLock(), exits if lock cannot be acquired, focuses existing window on second-instance launch. Includes cross-platform scenarios and window restoration handling.
Process Management Enhancements
src/main/dependency-manager.ts, src/main/web-service-manager.ts
Adds Windows console suppression (windowsHide), process termination helpers, 5-minute timeout handling with improved logging, isResolved guards against duplicate resolutions, and enhanced exit/error event logging with platform context.
Windows Dependency Check Fix
scripts/check.bat.template, scripts/check.sh.template, scripts/TEMPLATES.md
Provides script templates with explicit exit handling (exit /b 0/1 for Windows, exit 0/1 for Unix) to ensure automatic script termination after dependency checks without blocking commands.
Code Signing Design & Specification
openspec/changes/.../2026-02-13-code-signing-integration/design.md, openspec/changes/.../2026-02-13-code-signing-integration/proposal.md, openspec/changes/.../2026-02-13-code-signing-integration/specs/ci-cd/spec.md, openspec/changes/.../2026-02-13-code-signing-integration/tasks.md
Comprehensive design documentation for Azure Artifact Signing integration, CI/CD workflow modifications, multi-platform architecture, GitHub Actions examples, risk mitigation, migration phases, and rollback strategies.
Single Instance Lock Design & Specification
openspec/changes/.../2026-02-13-electron-single-instance-lock/proposal.md, openspec/changes/.../2026-02-13-electron-single-instance-lock/specs/electron-app/spec.md, openspec/changes/.../2026-02-13-electron-single-instance-lock/tasks.md
Design and implementation planning for preventing multiple app instances, handling second-instance events, window focus behavior, cross-platform scenarios, and testing strategy.
Windows Console Hiding Design & Specification
openspec/changes/.../2026-02-13-hide-windows-console-windows/proposal.md, openspec/changes/.../2026-02-13-hide-windows-console-windows/specs/dependency-management/spec.md, openspec/changes/.../2026-02-13-hide-windows-console-windows/specs/electron-app/spec.md, openspec/changes/.../2026-02-13-hide-windows-console-windows/tasks.md
Specifies windowsHide: true usage for child_process.spawn on Windows, documents cross-platform consistency, defines affected methods, and outlines implementation and testing steps.
Windows Dependency Check Auto-Exit Design & Specification
openspec/changes/.../2026-02-13-windows-dependency-check-script-auto-exit-fix/.../design.md, openspec/changes/.../2026-02-13-windows-dependency-check-script-auto-exit-fix/.../implementation-summary.md, openspec/changes/.../2026-02-13-windows-dependency-check-script-auto-exit-fix/.../proposal.md, openspec/changes/.../2026-02-13-windows-dependency-check-script-auto-exit-fix/.../specs/dependency-management/spec.md, openspec/changes/.../2026-02-13-windows-dependency-check-script-auto-exit-fix/.../tasks.md
Detailed design for Windows check.bat auto-exit fix, process timeout handling enhancements, cross-platform script templates, logging improvements, and comprehensive testing/rollback strategy.
Project Documentation
openspec/project.md
Adds Azure Artifact Signing and code-signing-integration documentation to project metadata, including enabled procedures, GitHub Secrets, supported platforms, and integration guidelines.

Sequence Diagram(s)

sequenceDiagram
    participant GA as GitHub Actions
    participant AZ as Azure Artifact<br/>Signing Service
    participant ST as Windows signtool
    participant NF as HagiCode-org/<br/>haginotifier

    GA->>GA: Build Windows artifacts<br/>(.exe, .appx)
    
    alt ENABLE_CODE_SIGNING enabled
        GA->>GA: Validate Azure credentials
        
        alt Credentials present
            GA->>AZ: Login with credentials<br/>(client ID/secret/tenant)
            activate AZ
            AZ-->>GA: Authentication token
            deactivate AZ
            
            GA->>AZ: Sign artifact(s)<br/>(SHA-256)
            activate AZ
            AZ-->>GA: Signed artifact
            deactivate AZ
            
            GA->>ST: Verify signature<br/>signtool verify
            activate ST
            ST-->>GA: Signature valid
            deactivate ST
            
            GA->>GA: ✓ Signing complete
        else Credentials missing
            GA->>GA: ⚠ Skip signing<br/>(warning logged)
        end
    else ENABLE_CODE_SIGNING disabled
        GA->>GA: Skip signing
    end
    
    alt Signing failed
        GA->>NF: Send failure notification<br/>(bilingual message,<br/>run link)
        activate NF
        NF-->>GA: Notification sent
        deactivate NF
        GA->>GA: ✗ Job fails
    else Signing succeeded
        GA->>GA: ✓ Job succeeds
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • onboarding issue fix #16: Modifies src/main/dependency-manager.ts with process spawning and termination logic enhancements overlapping this PR's Windows process handling changes.
  • add basic framework #1: Extends the build workflow and electron-builder.yml configuration by adding code-signing steps, signing scripts, and Azure credential plumbing to the same CI/CD files.
  • fix dependency installation issue #17: Introduces related runtime behavior changes to src/main/dependency-manager.ts and src/main/web-service-manager.ts with process termination helpers, timeout handling, and spawn option management.

Poem

🐰 With signing keys and locked instances held tight,
Windows consoles hidden, processes done right,
Timeouts enforced, scripts exit with grace,
Code flows securely through build's brisk pace!

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title does not clearly reflect the scope and complexity of this PR, which includes code signing integration, single-instance locking, Windows console hiding, and dependency check improvements across multiple systems. Revise the title to be more specific and comprehensive, such as 'Add code signing, single-instance lock, and Windows console suppression' or split into multiple PRs with focused titles.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 91.67% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch winfix

Comment @coderabbitai help to get the list of available commands and usage tips.

@newbe36524 newbe36524 merged commit e1b5873 into main Feb 13, 2026
3 of 4 checks passed
@newbe36524 newbe36524 deleted the winfix branch February 13, 2026 15:17
Copy link

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

🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Line 108: The workflow currently hardcodes the signtool path with version
"10.0.22000.0"; replace that by discovering a valid signtool.exe dynamically
(don’t hardcode versions). Reuse or port the existing getSignToolPath() logic
from scripts/verify-signature.js (or call that script) to probe possible
locations and fallbacks (10.x versions, x64 without version, ProgramFiles(x86),
Windows Kits 8.1) and return the correct signtool.exe path, then use that
returned path instead of the fixed string; update the command that invokes
signtool.exe to use the discovered path variable.

In `@openspec/changes/archive/2026-02-13-code-signing-integration/design.md`:
- Around line 242-272: Move the Windows signingHashAlgorithms key under
signtoolOptions (i.e., set signtoolOptions.signingHashAlgorithms: ["sha256"]) so
electron-builder picks it up, and for macOS change identity to null (or remove
the custom "${env.APPLE_SIGNING_IDENTITY}" setting) so electron-builder will use
the standard CSC_NAME env var; ensure you provide CSC_NAME, CSC_LINK and
CSC_KEY_PASSWORD as build environment variables instead of relying on
APPLE_SIGNING_IDENTITY.
- Around line 184-200: The docs incorrectly use non-existent CLI commands (az
artifacts signing sign / az trusted-signing sign); replace that example by
instructing users to generate a metadata.json and invoke Windows SDK
signtool.exe together with Microsoft's Artifact Signing dlib
(Azure.CodeSigning.Dlib.dll) to perform signing; update the text to show
creating metadata.json with
Endpoint/CodeSigningAccountName/CertificateProfileName (and optional
CorrelationId) and demonstrate calling signtool.exe with /fd SHA256, /tr
timestamp, /td SHA256, /dlib pointing to Azure.CodeSigning.Dlib.dll and /dmdf
pointing to metadata.json to sign pkg/*.exe (and similarly for .appx) instead of
the az commands.

In
`@openspec/changes/archive/2026-02-13-code-signing-integration/specs/ci-cd/spec.md`:
- Around line 214-218: The spec defines an unused env var VERIFY_STRICT; either
remove VERIFY_STRICT from the variables table or add at least one scenario that
demonstrates its behavior. If you choose to keep it, add a scenario (and
examples) that sets VERIFY_STRICT to "true" and "false" and explains how the
code-signing verification changes (referencing ENABLE_CODE_SIGNING and
VERIFY_STRICT), update any sample CI snippets and expected outcomes in the
scenarios to show strict vs non-strict verification, and ensure the variables
table and any cross-references reflect the new scenario; if you choose to drop
it, remove the VERIFY_STRICT row from the variables table and any stray
references to VERIFY_STRICT in the spec.

In
`@openspec/changes/archive/2026-02-13-hide-windows-console-windows/proposal.md`:
- Line 32: Fix the typo in the Mermaid diagram node labeling: change "detected:
true" to "detached: true" in the diagram line containing the node text "B
-->|Windows| C[detected: true<br/>windowsHide: true]" so the Windows spawn
options correctly read "detached: true<br/>windowsHide: true".

In
`@openspec/changes/archive/2026-02-13-hide-windows-console-windows/specs/dependency-management/spec.md`:
- Around line 5-27: The requirement block titled "Requirement: Entry Point
Script Execution" is new and must be marked ADDED instead of MODIFIED; move the
entire block (including the three Windows scenarios and the Cross-platform
consistency scenario) out of any MODIFIED section and place it under the ADDED
requirements section, preserving the exact heading and scenario content (e.g.,
"Execute check script on Windows", "Execute install script on Windows", "Execute
start script on Windows", and "Cross-platform consistency") so the delta spec
reflects a new capability rather than an edited existing requirement.

In
`@openspec/changes/archive/2026-02-13-windows-dependency-check-script-auto-exit-fix/windows-dependency-check-script-auto-exit-fix/design.md`:
- Around line 24-27:
将中文中作状语的短语改为使用“地”作副词连接词以符合语法;具体把“提供健壮的超时机制处理挂起的脚本”改为“提供健壮地处理挂起脚本的超时机制”,把“确保所有代码路径都有适当的退出处理”改为“确保所有代码路径都有适当地退出处理”,把“控制台日志显示清晰的执行状态”改为“控制台日志清晰地显示执行状态”,并在文档中其它相同用法处(例如文中类似的
176-181、233-236 段落)统一进行相同的“地”修正以保持表述一致性。
- Around line 6-9: The two fenced code blocks that contain log snippets (the
block beginning with "[2026-02-13 20:53:21.937] [error]..." and the block
containing "[info] Script exited normally with code: 0" / "[warn] Script
terminated due to timeout (300s)" etc.) should include a language identifier to
satisfy markdownlint MD040; update each opening fence from ``` to ```text so
both log snippets are fenced as text (also apply the same change to the other
occurrence noted around lines 74-78).

In
`@openspec/changes/archive/2026-02-13-windows-dependency-check-script-auto-exit-fix/windows-dependency-check-script-auto-exit-fix/implementation-summary.md`:
- Around line 38-52: The implementation summary lists three template files
(scripts/check.bat.template, scripts/check.sh.template, scripts/TEMPLATES.md)
that are not present in the PR; either add those missing files to the scripts/
directory with the described contents (ensuring the exit lines and
check-result.json generation) or update implementation-summary.md to accurately
reflect the files actually included in this PR, referencing the exact filenames
(check.bat.template, check.sh.template, TEMPLATES.md) and removing or changing
the bullet entries accordingly.

In `@openspec/specs/electron-app/spec.md`:
- Around line 261-308: The spec's "Embedded web service port conflict
prevention" scenario references port 5000 which mismatches the application's
actual default embedded web service port (36556) declared in main.ts; update the
scenario to either use the real default port 36556 or replace the hardcoded
value with a placeholder like "configured port" so it reads generically,
ensuring the scenario text ("Embedded web service port conflict prevention") and
any assertions refer to the configured/default port used by the application (as
defined where the embedded web service is initialized in main.ts).

In `@scripts/sign-artifact.js`:
- Around line 117-118: Update the example command in scripts/sign-artifact.js
that currently shows "node scripts/sign-artifact.exe" to use the correct
JavaScript filename extension (change ".exe" to ".js"), so the example reads
"node scripts/sign-artifact.js" to accurately reflect how to run the
sign-artifact script.
- Around line 309-311: The fallback ProgramFiles string is double-escaped
("\\\\") causing an incorrect literal; update the fallback used when reading
process.env.ProgramFiles (the programFiles variable) to use a single backslash
escape like 'C:\\Program Files' or, better, avoid hardcoding backslashes and
rely on path.join by simply using 'Program Files' as part of the join with
path.join(programFiles || 'C:\\Program Files', 'Windows Kits', '10', 'bin',
'x64', 'signtool.exe') so signToolPath and the path.join call produce a valid
Windows path.
- Around line 265-293: The signWithAzureKeyVault function builds an invalid az
command; change it to call the correct subcommand "az keyvault key sign" and use
--vault-name (extract the vault name from config.azureSigningKeyUri) and --name
(key name) plus --algorithm (use a signature algorithm like RS256 from
config.azureSigningAlgorithm), remove --file, and pass the file's SHA-256 digest
via --digest (compute the file bytes, hash with SHA-256, base64-encode the
digest before invoking az). Update the args array accordingly, compute and
supply the digest string, and keep the existing spawn/Promise/error handling but
ensure errors include useful context.

In `@scripts/verify-signature.js`:
- Around line 103-108: The help text in scripts/verify-signature.js currently
lists supported file types but omits .msi even though SIGNABLE_EXTENSIONS
includes it; update the help/documentation block (the "Supported file types"
list near the usage/help output) to include "- .msi  - Windows installer
packages" (or similar) so the help matches SIGNABLE_EXTENSIONS, keeping
formatting consistent with the other entries.
- Around line 170-171: The fallback path strings for programFiles and
programFilesX86 use double-escaped backslashes; update the constants
programFiles and programFilesX86 in verify-signature.js to use normal Windows
path literals (single backslashes, e.g. "C:\Program Files" and "C:\Program Files
(x86)") or use forward-slash variants, mirroring the fix from sign-artifact.js
so the fallback paths are not double-escaped.

In `@src/main/dependency-manager.ts`:
- Around line 271-289: The terminateProcess helper currently calls
child.kill('SIGKILL') which on Windows only kills the immediate process; update
terminateProcess (referencing terminateProcess, child, this.platform, and
scriptPath) to detect Windows and run a Windows tree-kill using taskkill /F /T
with the child.pid (e.g., spawn or exec 'taskkill /PID <pid> /F /T'), ensure
child.pid exists before invoking taskkill, capture and log any errors from the
taskkill invocation, and fall back to child.kill('SIGKILL') if taskkill fails or
pid is missing to preserve current behavior on non-Windows platforms.

In `@src/main/main.ts`:
- Around line 24-38: The secondary-instance path after calling
app.requestSingleInstanceLock() currently calls app.quit() but continues
executing — ensure the process exits immediately by invoking process.exit(0)
right after app.quit() in the branch where gotSingleInstanceLock is false;
update the block that checks gotSingleInstanceLock (the code that logs and
quits) so that after log.info('[App] Single instance lock failed - another
instance is already running') and app.quit() you call process.exit(0) to prevent
further registration of IPC handlers and execution of app.whenReady()
initialization code.

In `@src/main/web-service-manager.ts`:
- Around line 247-268: The Windows branch in terminateProcess does not kill a
detached process tree; update terminateProcess to, when process.platform ===
'win32', check child.pid and call a Windows taskkill command (e.g., taskkill /T
/F <child.pid>) to terminate the whole tree, falling back to
child.kill('SIGKILL') if taskkill fails, and keep the existing try/catch to log
errors (referencing terminateProcess, child, isResolved, timeout). Ensure you
use a synchronous or properly awaited spawn/exec so termination completes before
resolving/rejecting, and include the pid in log messages for clarity.
🧹 Nitpick comments (6)
scripts/sign-artifact.js (1)

27-92: Consider extracting shared utilities.

The colors object, log(), and logCI() functions are duplicated between sign-artifact.js and verify-signature.js. Consider extracting these to a shared module to reduce duplication.

scripts/verify-signature.js (1)

210-213: Misleading signed: true when signtool is not found.

When signtool.exe is unavailable, the function returns { signed: true, method: 'skipped' }. While the main logic correctly excludes skipped files from the unsigned count (line 373), returning signed: true is semantically misleading and could cause confusion in downstream logic or logs.

♻️ Proposed fix
   if (!signToolPath) {
     logCI('signtool.exe not found. Skipping detailed verification.', 'warning');
-    return { signed: true, method: 'skipped' };
+    return { signed: null, method: 'skipped' };
   }

Then update formatResult to handle null:

+  if (result.signed === null || result.method === 'skipped') {
+    return `${colors.yellow}⊘ ${fileName}${colors.reset} (skipped - signtool not found)`;
+  }
+
   if (result.signed) {
.github/workflows/build.yml (2)

64-68: Secrets check may not work as intended.

The condition [ -z "${{ secrets.AZURE_CLIENT_ID }}" ] checks if the expanded secret is empty. However, if the secret is not set at all, GitHub Actions substitutes an empty string, so this should work. But be aware that if secrets are set to empty strings intentionally, this check won't distinguish between "not set" and "set to empty".

A more robust approach would be to check the env vars after they're set, since the env: block below (lines 90-94) maps secrets to environment variables.

♻️ Proposed fix
-          # Check if Azure credentials are available
-          if [ -z "${{ secrets.AZURE_CLIENT_ID }}" ] || [ -z "${{ secrets.AZURE_CLIENT_SECRET }}" ] || [ -z "${{ secrets.AZURE_TENANT_ID }}" ]; then
+          # Check if Azure credentials are available (via env vars)
+          if [ -z "$AZURE_CLIENT_ID" ] || [ -z "$AZURE_CLIENT_SECRET" ] || [ -z "$AZURE_TENANT_ID" ]; then
             echo "::warning::Azure credentials not configured. Skipping code signing."
             echo "To enable code signing, set AZURE_CLIENT_ID, AZURE_CLIENT_SECRET, and AZURE_TENANT_ID secrets."
             exit 0
           fi

96-124: Consider using the verify-signature.js script for consistency.

The signing step uses node scripts/sign-artifact.js, but the verification step has inline PowerShell logic. For consistency and to benefit from the robust signtool path discovery in scripts/verify-signature.js, consider using the script instead.

♻️ Proposed refactor
       # Verify signatures
       - name: Verify Windows signatures
         if: success() && env.ENABLE_CODE_SIGNING == 'true'
-        shell: pwsh
+        shell: bash
         run: |
           echo "Verifying code signatures..."
-
-          $artifacts = Get-ChildItem -Path pkg -Include *.exe,*.appx -Recurse -ErrorAction SilentlyContinue
-          $failed = $false
-
-          foreach ($artifact in $artifacts) {
-            Write-Host "Verifying: $($artifact.FullName)"
-            $result = & "$env:ProgramFiles\Windows Kits\10\bin\10.0.22000.0\x64\signtool.exe" verify /pa "$($artifact.FullName)" 2>&1
-            if ($LASTEXITCODE -ne 0) {
-              Write-Host "::error::Signature verification failed for: $($artifact.FullName)"
-              Write-Host $result
-              $failed = $true
-            } else {
-              Write-Host "Signature verified: $($artifact.Name)"
-            }
-          }
-
-          if ($failed) {
-            Write-Host "::error::One or more signature verifications failed"
-            exit 1
-          }
-
-          echo "All signatures verified successfully"
+          node scripts/verify-signature.js --all pkg || {
+            echo "::error::Signature verification failed"
+            exit 1
+          }
+        env:
+          VERIFY_STRICT: "true"
openspec/changes/archive/2026-02-13-code-signing-integration/design.md (1)

170-201: Azure CLI installation security consideration.

The script uses curl -sL https://aka.ms/InstallAzureCLIDeb | sudo bash to install Azure CLI. While this is Microsoft's official installation method, piping curl output directly to bash poses a security risk if the endpoint is compromised or redirects unexpectedly.

🔒 Consider adding verification or using package manager

Alternative approaches:

  1. Download the script first, verify its checksum, then execute
  2. Use the system package manager if Azure CLI is available
  3. Add a comment acknowledging this is Microsoft's official method
-    # 安装 Azure CLI
-    curl -sL https://aka.ms/InstallAzureCLIDeb | sudo bash
+    # 安装 Azure CLI (使用微软官方安装脚本)
+    # 注意: 生产环境建议使用包管理器或预装镜像
+    curl -sL https://aka.ms/InstallAzureCLIDeb | sudo bash
openspec/changes/archive/2026-02-13-code-signing-integration/specs/ci-cd/spec.md (1)

66-70: Clarify signing endpoint configuration options.

The scenario mentions both AZURE_SIGNING_ENDPOINT and AZURE_SIGNING_KEY_URI but doesn't specify when to use which. Based on the design document, AZURE_SIGNING_ENDPOINT is for Azure Trusted Signing, while AZURE_SIGNING_KEY_URI is for Azure Key Vault. Consider clarifying this distinction in the scenario.

📝 Proposed clarification
 #### Scenario: 签名端点配置
 
 **Given** GitHub Actions 工作流准备调用签名服务
-**When** 读取 `AZURE_SIGNING_ENDPOINT` 或 `AZURE_SIGNING_KEY_URI` 配置
+**When** 读取签名端点配置
+  - 使用 Azure Trusted Signing 时读取 `AZURE_SIGNING_ENDPOINT`
+  - 使用 Azure Key Vault 时读取 `AZURE_SIGNING_KEY_URI`
 **Then** 使用配置的端点进行签名请求
 **And** 配置缺失时终止构建并提示配置要求


foreach ($artifact in $artifacts) {
Write-Host "Verifying: $($artifact.FullName)"
$result = & "$env:ProgramFiles\Windows Kits\10\bin\10.0.22000.0\x64\signtool.exe" verify /pa "$($artifact.FullName)" 2>&1
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's check if scripts/verify-signature.js exists and look for path discovery logic
fd verify-signature.js

Repository: HagiCode-org/desktop

Length of output: 91


🏁 Script executed:

# Also check the workflow file around line 108 for context
sed -n '100,120p' .github/workflows/build.yml

Repository: HagiCode-org/desktop

Length of output: 928


🏁 Script executed:

# Search for any references to signtool or SDK paths in the codebase
rg -n 'signtool|Windows Kits.*bin' --type yaml --type ps1 --type js

Repository: HagiCode-org/desktop

Length of output: 91


🏁 Script executed:

# Read the verify-signature.js file to check for path discovery logic
cat -n scripts/verify-signature.js

Repository: HagiCode-org/desktop

Length of output: 12205


🏁 Script executed:

# Search for signtool and SDK paths more broadly
rg -n 'signtool|Windows Kits|10\.0\.' -i

Repository: HagiCode-org/desktop

Length of output: 7188


🏁 Script executed:

# Check if there are multiple references to this SDK path
rg -n '10\.0\.22000\.0'

Repository: HagiCode-org/desktop

Length of output: 192


Use dynamic SDK path discovery instead of hardcoded 10.0.22000.0 version.

The signtool path hardcodes SDK version 10.0.22000.0, which may not exist on all windows-latest runners as SDK versions vary.

Reuse the existing getSignToolPath() function from scripts/verify-signature.js (which checks multiple SDK versions and locations), or port that logic to PowerShell. That function already handles fallbacks to x64 without version, ProgramFiles(x86), and Windows Kit 8.1.

🛠️ Proposed fix - Use dynamic SDK path discovery
-            $result = & "$env:ProgramFiles\Windows Kits\10\bin\10.0.22000.0\x64\signtool.exe" verify /pa "$($artifact.FullName)" 2>&1
+            # Find signtool.exe dynamically
+            $sdkPath = Get-ChildItem "$env:ProgramFiles\Windows Kits\10\bin\*\x64\signtool.exe" -ErrorAction SilentlyContinue | Select-Object -First 1
+            if (-not $sdkPath) {
+              $sdkPath = Get-ChildItem "${env:ProgramFiles(x86)}\Windows Kits\10\bin\*\x64\signtool.exe" -ErrorAction SilentlyContinue | Select-Object -First 1
+            }
+            if (-not $sdkPath) {
+              Write-Host "::error::signtool.exe not found"
+              exit 1
+            }
+            $result = & $sdkPath.FullName verify /pa "$($artifact.FullName)" 2>&1

Alternatively, call the existing Node.js verification script or factor out the path discovery into a reusable module.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$result = & "$env:ProgramFiles\Windows Kits\10\bin\10.0.22000.0\x64\signtool.exe" verify /pa "$($artifact.FullName)" 2>&1
# Find signtool.exe dynamically
$sdkPath = Get-ChildItem "$env:ProgramFiles\Windows Kits\10\bin\*\x64\signtool.exe" -ErrorAction SilentlyContinue | Select-Object -First 1
if (-not $sdkPath) {
$sdkPath = Get-ChildItem "${env:ProgramFiles(x86)}\Windows Kits\10\bin\*\x64\signtool.exe" -ErrorAction SilentlyContinue | Select-Object -First 1
}
if (-not $sdkPath) {
Write-Host "::error::signtool.exe not found"
exit 1
}
$result = & $sdkPath.FullName verify /pa "$($artifact.FullName)" 2>&1
🤖 Prompt for AI Agents
In @.github/workflows/build.yml at line 108, The workflow currently hardcodes
the signtool path with version "10.0.22000.0"; replace that by discovering a
valid signtool.exe dynamically (don’t hardcode versions). Reuse or port the
existing getSignToolPath() logic from scripts/verify-signature.js (or call that
script) to probe possible locations and fallbacks (10.x versions, x64 without
version, ProgramFiles(x86), Windows Kits 8.1) and return the correct
signtool.exe path, then use that returned path instead of the fixed string;
update the command that invokes signtool.exe to use the discovered path
variable.

Comment on lines +184 to +200
# 签名所有 .exe 文件
for file in pkg/*.exe; do
echo "Signing $file"
az artifacts signing sign \
--endpoint "${{ secrets.AZURE_SIGNING_ENDPOINT }}" \
--file "$file" \
--hash-algorithm SHA256
done

# 签名所有 .appx 文件
for file in pkg/*.appx; do
echo "Signing $file"
az artifacts signing sign \
--endpoint "${{ secrets.AZURE_SIGNING_ENDPOINT }}" \
--file "$file" \
--hash-algorithm SHA256
done
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

Azure Trusted Signing CLI command syntax 2026 az sign

💡 Result:

As of February 13, 2026, there isn’t an az sign command for Azure Trusted Signing / Azure Artifact Signing. The Azure CLI piece is the az trustedsigning extension (management of accounts and certificate profiles), while actual file signing is done via Windows SDK signtool.exe plus Microsoft’s Artifact Signing dlib and a metadata JSON file. [1] [2]

Azure CLI (management) — az trustedsigning (no az sign)

The command group is:

  • az trustedsigning ...
  • az trustedsigning certificate-profile ... [2]

Examples of the published syntax include: [2]

  • az trustedsigning create -n <AccountName> -g <ResourceGroup> -l <location> --sku <Basic|Premium>
  • az trustedsigning list [-g <ResourceGroup>]
  • az trustedsigning certificate-profile list -g <ResourceGroup> --account-name <AccountName>
  • az trustedsigning certificate-profile show -g <ResourceGroup> --account-name <AccountName> -n <ProfileName>

Signing files (what you likely meant by “az sign”)

Microsoft’s documented signing invocation is SignTool + dlib (not an az sign command): [1]

  1. Create metadata.json: [1]
{
  "Endpoint": "https://<region>.codesigning.azure.net",
  "CodeSigningAccountName": "<Artifact Signing account name>",
  "CertificateProfileName": "<Certificate profile name>",
  "CorrelationId": "<Optional CorrelationId value>"
}
  1. Sign with signtool.exe (example syntax from Microsoft Learn): [1]
& "<Path to SDK bin folder>\x64\signtool.exe" sign /v /debug /fd SHA256 `
  /tr "http://timestamp.acs.microsoft.com" /td SHA256 `
  /dlib "<Path to Artifact Signing dlib bin folder>\x64\Azure.CodeSigning.Dlib.dll" `
  /dmdf "<Path to metadata file>\metadata.json" `
  <FileToSign>
  1. Install Client Tools via WinGet (optional but recommended by Microsoft): [1]
winget install -e --id Microsoft.Azure.ArtifactSigningClientTools

Replace the Azure CLI signing approach with the correct Artifact Signing method.

As of February 2026, az artifacts signing sign and az trusted-signing sign commands do not exist. The az trustedsigning CLI extension only manages accounts and certificate profiles; actual file signing requires Windows SDK signtool.exe with Microsoft's Artifact Signing dlib and a metadata.json configuration file.

Replace the code example with:

Correct approach: signtool.exe + Artifact Signing dlib
# Create metadata.json
@{
    Endpoint = "https://<region>.codesigning.azure.net"
    CodeSigningAccountName = "<account-name>"
    CertificateProfileName = "<profile-name>"
    CorrelationId = "<optional-id>"
} | ConvertTo-Json | Set-Content metadata.json

# Sign executables with signtool.exe
foreach ($file in Get-ChildItem "pkg/*.exe") {
    & "C:\Program Files (x86)\Windows Kits\10\bin\x64\signtool.exe" sign /v `
        /fd SHA256 `
        /tr "http://timestamp.acs.microsoft.com" /td SHA256 `
        /dlib "C:\Path\To\Azure.CodeSigning.Dlib.dll" `
        /dmdf "metadata.json" `
        $file.FullName
}
🤖 Prompt for AI Agents
In `@openspec/changes/archive/2026-02-13-code-signing-integration/design.md`
around lines 184 - 200, The docs incorrectly use non-existent CLI commands (az
artifacts signing sign / az trusted-signing sign); replace that example by
instructing users to generate a metadata.json and invoke Windows SDK
signtool.exe together with Microsoft's Artifact Signing dlib
(Azure.CodeSigning.Dlib.dll) to perform signing; update the text to show
creating metadata.json with
Endpoint/CodeSigningAccountName/CertificateProfileName (and optional
CorrelationId) and demonstrate calling signtool.exe with /fd SHA256, /tr
timestamp, /td SHA256, /dlib pointing to Azure.CodeSigning.Dlib.dll and /dmdf
pointing to metadata.json to sign pkg/*.exe (and similarly for .appx) instead of
the az commands.

Comment on lines +242 to +272
```yaml
# electron-builder.yml 添加签名配置
win:
target:
- portable
- nsis
- appx
icon: resources/icon.ico
# 签名配置(通过环境变量传递)
signingHashAlgorithms:
- sha256

mac:
target:
- target: default
arch:
- x64
- arm64
icon: resources/icon.icns
category: public.app-category.utilities
# 签名配置(通过环境变量传递)
identity: "${env.APPLE_SIGNING_IDENTITY}"

linux:
target:
- AppImage
- deb
- tar.gz
category: Utility
maintainer: newbe36524 <support@hagicode.com>
```
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

electron-builder code signing configuration Windows signingHashAlgorithms documentation

💡 Result:

signingHashAlgorithms is documented as part of electron-builder’s Windows signtool options (WindowsSigntoolConfiguration). It controls which digest algorithms are used when signing Windows binaries (allowed values: "sha1" and "sha256"). Default is dual-signing with both algorithms (["sha1","sha256"]), and for AppX packages sha256 is always used. [1]

Where to configure it

electron-builder exposes this under win.signtoolOptions (the Windows “Any Target” config documents signtoolOptions). [2]

Example (package.json):

{
  "build": {
    "win": {
      "signtoolOptions": {
        "signingHashAlgorithms": ["sha256"]
      }
    }
  }
}

Related note from the Windows code-signing docs

electron-builder notes that Windows is dual code-signed (SHA1 & SHA256) by default. [3]

Sources: [1] [2] [3]


🌐 Web query:

electron-builder macOS identity signing configuration YAML syntax

💡 Result:

electron-builder macOS signing identity — YAML config syntax

In electron-builder.yml (or electron-builder.yaml):

mac:
  identity: "Developer ID Application: Your Company (TEAMID)"

mac.identity is the certificate name to use for signing. You can also set it to:

  • nullskip signing entirely
  • "-" → use an ad-hoc identity (useful for ARM/universal dev builds)
    [1][2]
mac:
  identity: null   # no signing
mac:
  identity: "-"    # ad-hoc signing

Preferred approach (especially CI): use environment variables (not YAML)

electron-builder recommends using env vars like these instead of hardcoding identity in YAML:

  • CSC_LINK (p12/pfx URL/path/base64)
  • CSC_KEY_PASSWORD
  • CSC_NAME (macOS-only: certificate name from keychain)
  • CSC_IDENTITY_AUTO_DISCOVERY (true/false)
    [2][3]

MAS / provisioning profile (if you need it)

mas:
  provisioningProfile: "path/to/profile.provisionprofile"

provisioningProfile can be a path (absolute or relative to app root).
[4]

Sources: electron-builder macOS code signing docs and configuration reference. [1][2][3][4]


Correct electron-builder YAML configuration for Windows and macOS code signing.

Windows signingHashAlgorithms must be nested under signtoolOptions, not directly under win:. The macOS identity should use electron-builder's standard environment variables (CSC_NAME) instead of custom variable interpolation.

Corrected configuration example
win:
  target:
    - portable
    - nsis
    - appx
  icon: resources/icon.ico
  signtoolOptions:
    signingHashAlgorithms:
      - sha256

mac:
  target:
    - target: default
      arch:
        - x64
        - arm64
  icon: resources/icon.icns
  category: public.app-category.utilities
  # Use CSC_NAME environment variable instead of custom interpolation
  identity: null  # Will use CSC_NAME env var if set

Set CSC_NAME, CSC_LINK, and CSC_KEY_PASSWORD as environment variables during the build process.

🤖 Prompt for AI Agents
In `@openspec/changes/archive/2026-02-13-code-signing-integration/design.md`
around lines 242 - 272, Move the Windows signingHashAlgorithms key under
signtoolOptions (i.e., set signtoolOptions.signingHashAlgorithms: ["sha256"]) so
electron-builder picks it up, and for macOS change identity to null (or remove
the custom "${env.APPLE_SIGNING_IDENTITY}" setting) so electron-builder will use
the standard CSC_NAME env var; ensure you provide CSC_NAME, CSC_LINK and
CSC_KEY_PASSWORD as build environment variables instead of relying on
APPLE_SIGNING_IDENTITY.

Comment on lines +214 to +218
| 变量名称 | 描述 | 默认值 |
|---------|------|--------|
| `ENABLE_CODE_SIGNING` | 启用代码签名 | `"false"` |
| `VERIFY_STRICT` | 签名验证严格模式 | `"false"` |

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Unused environment variable in specification.

The VERIFY_STRICT environment variable is defined in the configuration reference but is not mentioned in any of the scenarios above. Either add scenarios that use this variable or remove it from the documentation.

🤖 Prompt for AI Agents
In
`@openspec/changes/archive/2026-02-13-code-signing-integration/specs/ci-cd/spec.md`
around lines 214 - 218, The spec defines an unused env var VERIFY_STRICT; either
remove VERIFY_STRICT from the variables table or add at least one scenario that
demonstrates its behavior. If you choose to keep it, add a scenario (and
examples) that sets VERIFY_STRICT to "true" and "false" and explains how the
code-signing verification changes (referencing ENABLE_CODE_SIGNING and
VERIFY_STRICT), update any sample CI snippets and expected outcomes in the
scenarios to show strict vs non-strict verification, and ensure the variables
table and any cross-references reflect the new scenario; if you choose to drop
it, remove the VERIFY_STRICT row from the variables table and any stray
references to VERIFY_STRICT in the spec.

```mermaid
flowchart TD
A[Spawn Called] --> B{Platform Check}
B -->|Windows| C[detected: true<br/>windowsHide: true]
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Typo: "detected" should be "detached".

The Mermaid diagram contains a typo in the Windows spawn options description.

📝 Proposed fix
-    B -->|Windows| C[detected: true<br/>windowsHide: true]
+    B -->|Windows| C[detached: true<br/>windowsHide: true]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
B -->|Windows| C[detected: true<br/>windowsHide: true]
B -->|Windows| C[detached: true<br/>windowsHide: true]
🤖 Prompt for AI Agents
In `@openspec/changes/archive/2026-02-13-hide-windows-console-windows/proposal.md`
at line 32, Fix the typo in the Mermaid diagram node labeling: change "detected:
true" to "detached: true" in the diagram line containing the node text "B
-->|Windows| C[detected: true<br/>windowsHide: true]" so the Windows spawn
options correctly read "detached: true<br/>windowsHide: true".

Comment on lines +103 to +108
Supported file types:
- .exe - Executable files
- .dll - Dynamic-link libraries
- .appx - Windows app packages
- .msix - Windows app packages

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Help text missing .msi file type.

The help documentation lists supported file types but omits .msi which is included in SIGNABLE_EXTENSIONS (line 124).

📝 Proposed fix
 Supported file types:
   - .exe  - Executable files
   - .dll  - Dynamic-link libraries
   - .appx - Windows app packages
   - .msix - Windows app packages
+  - .msi  - Windows installer packages
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Supported file types:
- .exe - Executable files
- .dll - Dynamic-link libraries
- .appx - Windows app packages
- .msix - Windows app packages
Supported file types:
- .exe - Executable files
- .dll - Dynamic-link libraries
- .appx - Windows app packages
- .msix - Windows app packages
- .msi - Windows installer packages
🤖 Prompt for AI Agents
In `@scripts/verify-signature.js` around lines 103 - 108, The help text in
scripts/verify-signature.js currently lists supported file types but omits .msi
even though SIGNABLE_EXTENSIONS includes it; update the help/documentation block
(the "Supported file types" list near the usage/help output) to include "- .msi 
- Windows installer packages" (or similar) so the help matches
SIGNABLE_EXTENSIONS, keeping formatting consistent with the other entries.

Comment on lines +170 to +171
const programFiles = process.env.ProgramFiles || 'C:\\\\Program Files';
const programFilesX86 = process.env['ProgramFiles(x86)'] || 'C:\\\\Program Files (x86)';
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Excessive backslash escaping in fallback paths.

Same issue as in sign-artifact.js - the path strings have double-escaped backslashes.

🐛 Proposed fix
-  const programFiles = process.env.ProgramFiles || 'C:\\\\Program Files';
-  const programFilesX86 = process.env['ProgramFiles(x86)'] || 'C:\\\\Program Files (x86)';
+  const programFiles = process.env.ProgramFiles || 'C:\\Program Files';
+  const programFilesX86 = process.env['ProgramFiles(x86)'] || 'C:\\Program Files (x86)';
🤖 Prompt for AI Agents
In `@scripts/verify-signature.js` around lines 170 - 171, The fallback path
strings for programFiles and programFilesX86 use double-escaped backslashes;
update the constants programFiles and programFilesX86 in verify-signature.js to
use normal Windows path literals (single backslashes, e.g. "C:\Program Files"
and "C:\Program Files (x86)") or use forward-slash variants, mirroring the fix
from sign-artifact.js so the fallback paths are not double-escaped.

Comment on lines +271 to +289
// Enhanced process termination helper for Windows detached processes
const terminateProcess = (reason: string) => {
if (isResolved) return;

log.warn(`[DependencyManager] Terminating script (${reason}):`, scriptPath);

if (this.platform === 'win32') {
// Windows: Try to kill the entire process group for detached processes
try {
// Negative PID kills the entire process group on Unix-like systems
// On Windows with detached mode, we need to handle this differently
child.kill('SIGKILL');
} catch (e) {
log.error('[DependencyManager] Failed to kill Windows process:', e);
}
} else {
child.kill('SIGKILL');
}
};
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Node.js child_process.kill SIGKILL Windows detached process tree termination

💡 Result:

On Windows, child_process.kill('SIGKILL') does not behave like POSIX “send SIGKILL to a process group / tree”. Windows doesn’t have POSIX signals; Node largely emulates kill() by force-terminating that one PID, and it does not automatically terminate the child’s descendants (process tree). (nodejs.org)

If you spawned with { detached: true }, the child is intentionally separated so it can keep running after the parent exits (often with its own console), which makes “kill everything that came from it” even less automatic. (nodejs.org)

Practical way to terminate a detached process tree on Windows

Use taskkill with /T (tree) and usually /F (force):

import { execFile } from "node:child_process";

function killTreeWindows(pid) {
  return new Promise((resolve, reject) => {
    execFile("taskkill", ["/PID", String(pid), "/T", "/F"], (err) =>
      err ? reject(err) : resolve()
    );
  });
}

Off-the-shelf option

Use tree-kill, which does this on Windows by running taskkill /pid PID /T /F. (npmjs.com)

Citations:


🏁 Script executed:

# Search for spawn calls in dependency-manager.ts to understand how processes are spawned
fd dependency-manager.ts --type f

Repository: HagiCode-org/desktop

Length of output: 94


🏁 Script executed:

# Look for the spawn calls and detached option usage in the file
rg -A 5 -B 5 "spawn\|child.kill" src/main/dependency-manager.ts | head -100

Repository: HagiCode-org/desktop

Length of output: 46


🏁 Script executed:

# Check the file size first
wc -l src/main/dependency-manager.ts

Repository: HagiCode-org/desktop

Length of output: 99


🏁 Script executed:

# Read the specific section mentioned in the review (271-289) and surrounding context
sed -n '250,310p' src/main/dependency-manager.ts | cat -n

Repository: HagiCode-org/desktop

Length of output: 2635


🏁 Script executed:

# Search for spawn calls with detached option to understand how processes are launched
rg "spawn\|detached" src/main/dependency-manager.ts -B 3 -A 3

Repository: HagiCode-org/desktop

Length of output: 46


Windows timeout termination will leave detached child processes running.

The code spawns processes with detached: true on Windows (line 12) but terminates them with child.kill('SIGKILL') (line 33), which on Windows only kills the immediate process—not the process tree. This leaves child processes orphaned, causing port/lock conflicts and resource leaks. Use taskkill /F /T to terminate the entire process tree on Windows.

💡 Suggested adjustment
-        if (this.platform === 'win32') {
-          // Windows: Try to kill the entire process group for detached processes
-          try {
-            // Negative PID kills the entire process group on Unix-like systems
-            // On Windows with detached mode, we need to handle this differently
-            child.kill('SIGKILL');
-          } catch (e) {
-            log.error('[DependencyManager] Failed to kill Windows process:', e);
-          }
-        } else {
-          child.kill('SIGKILL');
-        }
+        if (this.platform === 'win32') {
+          try {
+            spawn('taskkill', ['/F', '/T', '/PID', String(child.pid)], {
+              stdio: 'ignore',
+              windowsHide: true,
+            });
+          } catch (e) {
+            log.error('[DependencyManager] Failed to kill Windows process tree:', e);
+          }
+        } else {
+          child.kill('SIGKILL');
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Enhanced process termination helper for Windows detached processes
const terminateProcess = (reason: string) => {
if (isResolved) return;
log.warn(`[DependencyManager] Terminating script (${reason}):`, scriptPath);
if (this.platform === 'win32') {
// Windows: Try to kill the entire process group for detached processes
try {
// Negative PID kills the entire process group on Unix-like systems
// On Windows with detached mode, we need to handle this differently
child.kill('SIGKILL');
} catch (e) {
log.error('[DependencyManager] Failed to kill Windows process:', e);
}
} else {
child.kill('SIGKILL');
}
};
// Enhanced process termination helper for Windows detached processes
const terminateProcess = (reason: string) => {
if (isResolved) return;
log.warn(`[DependencyManager] Terminating script (${reason}):`, scriptPath);
if (this.platform === 'win32') {
try {
spawn('taskkill', ['/F', '/T', '/PID', String(child.pid)], {
stdio: 'ignore',
windowsHide: true,
});
} catch (e) {
log.error('[DependencyManager] Failed to kill Windows process tree:', e);
}
} else {
child.kill('SIGKILL');
}
};
🤖 Prompt for AI Agents
In `@src/main/dependency-manager.ts` around lines 271 - 289, The terminateProcess
helper currently calls child.kill('SIGKILL') which on Windows only kills the
immediate process; update terminateProcess (referencing terminateProcess, child,
this.platform, and scriptPath) to detect Windows and run a Windows tree-kill
using taskkill /F /T with the child.pid (e.g., spawn or exec 'taskkill /PID
<pid> /F /T'), ensure child.pid exists before invoking taskkill, capture and log
any errors from the taskkill invocation, and fall back to child.kill('SIGKILL')
if taskkill fails or pid is missing to preserve current behavior on non-Windows
platforms.

Comment on lines +24 to +38
/**
* Single Instance Lock
*
* Prevent multiple instances of the application from running simultaneously.
* When a second instance is launched, it exits immediately and focuses the
* existing instance's main window.
*/
const gotSingleInstanceLock = app.requestSingleInstanceLock();

if (!gotSingleInstanceLock) {
log.info('[App] Single instance lock failed - another instance is already running');
app.quit();
} else {
log.info('[App] Single instance lock acquired successfully');
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/main/main.ts | head -80

Repository: HagiCode-org/desktop

Length of output: 4123


🌐 Web query:

Electron app requestSingleInstanceLock recommended pattern best practices

💡 Result:

Recommended requestSingleInstanceLock() pattern (main process)

const { app, BrowserWindow } = require('electron');

let mainWindow;
let pendingSecondInstanceArgs = null;

// 1) Acquire the lock as early as possible
const gotLock = app.requestSingleInstanceLock(/* optional additionalData */);

if (!gotLock) {
  // 2) If we didn’t get the lock, quit immediately
  app.quit();
  process.exit(0);
}

// 3) In the primary instance, listen for “second-instance”
//    (this is emitted after app's "ready")
app.on('second-instance', (event, argv, workingDirectory, additionalData) => {
  // Focus/restore existing window
  if (mainWindow) {
    if (mainWindow.isMinimized()) mainWindow.restore();
    mainWindow.focus();
  } else {
    // If your window isn't created yet, cache args for later
    pendingSecondInstanceArgs = { argv, additionalData };
  }

  // Handle argv / additionalData here (e.g., open file, deep link, etc.)
});

app.whenReady().then(() => {
  mainWindow = new BrowserWindow(/* ... */);

  // If a second-instance arrived before window creation, handle it now
  if (pendingSecondInstanceArgs) {
    // handle pendingSecondInstanceArgs.argv / .additionalData
    pendingSecondInstanceArgs = null;
  }
});

Best practices / gotchas

  • Call app.requestSingleInstanceLock() very early (before creating windows / doing heavy startup). If it returns false, quit immediately. [1] (electronjs.org)
  • In the primary instance, handle app.on('second-instance', ...) to focus/restore your existing window and process the new launch’s inputs. [2] (electronjs.org)
  • Don’t rely on argv being exact: Electron notes ordering may change and extra args can be appended; if you need fidelity, prefer additionalData. [2] (electronjs.org)
  • On macOS, Finder launches are already single-instance-ish; for file/URL activations you should also register open-file / open-url handlers very early (even before ready) so you don’t miss them. [1] (electronjs.org)
  • Remember: Electron guarantees second-instance is emitted after ready, so if you must accept “open this file/link” before your window exists, buffer the request and handle it after window creation. [2] (electronjs.org)
[1] https://www.electronjs.org/docs/latest/api/app
[2] https://www.electronjs.org/docs/api/app/

Citations:


🏁 Script executed:

sed -n '80,200p' src/main/main.ts | head -100

Repository: HagiCode-org/desktop

Length of output: 3147


🏁 Script executed:

grep -n "app.whenReady\|app.on\|ipcMain.handle" src/main/main.ts | head -30

Repository: HagiCode-org/desktop

Length of output: 1877


🏁 Script executed:

grep -n "app.whenReady\|app.on('ready')" src/main/main.ts

Repository: HagiCode-org/desktop

Length of output: 103


🏁 Script executed:

sed -n '30,50p' src/main/main.ts && echo "---" && sed -n '1870,1890p' src/main/main.ts

Repository: HagiCode-org/desktop

Length of output: 1483


Add immediate process exit after app.quit() to prevent secondary instance initialization.

When requestSingleInstanceLock() fails, app.quit() alone does not guarantee immediate termination. The module continues executing, registers all IPC handlers unconditionally (lines 168+), and app.whenReady() will still initialize managers and services. Per Electron documentation, use process.exit(0) after app.quit() to ensure the secondary instance terminates before any initialization.

Recommended fix
if (!gotSingleInstanceLock) {
  log.info('[App] Single instance lock failed - another instance is already running');
  app.quit();
+  process.exit(0);
} else {
  log.info('[App] Single instance lock acquired successfully');
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Single Instance Lock
*
* Prevent multiple instances of the application from running simultaneously.
* When a second instance is launched, it exits immediately and focuses the
* existing instance's main window.
*/
const gotSingleInstanceLock = app.requestSingleInstanceLock();
if (!gotSingleInstanceLock) {
log.info('[App] Single instance lock failed - another instance is already running');
app.quit();
} else {
log.info('[App] Single instance lock acquired successfully');
}
/**
* Single Instance Lock
*
* Prevent multiple instances of the application from running simultaneously.
* When a second instance is launched, it exits immediately and focuses the
* existing instance's main window.
*/
const gotSingleInstanceLock = app.requestSingleInstanceLock();
if (!gotSingleInstanceLock) {
log.info('[App] Single instance lock failed - another instance is already running');
app.quit();
process.exit(0);
} else {
log.info('[App] Single instance lock acquired successfully');
}
🤖 Prompt for AI Agents
In `@src/main/main.ts` around lines 24 - 38, The secondary-instance path after
calling app.requestSingleInstanceLock() currently calls app.quit() but continues
executing — ensure the process exits immediately by invoking process.exit(0)
right after app.quit() in the branch where gotSingleInstanceLock is false;
update the block that checks gotSingleInstanceLock (the code that logs and
quits) so that after log.info('[App] Single instance lock failed - another
instance is already running') and app.quit() you call process.exit(0) to prevent
further registration of IPC handlers and execution of app.whenReady()
initialization code.

Comment on lines +247 to +268
// Enhanced process termination helper for Windows detached processes
const terminateProcess = (reason: string) => {
if (isResolved) return;

log.warn(`[WebService] Terminating start script (${reason}):`, scriptPath);

if (process.platform === 'win32') {
try {
child.kill('SIGKILL');
} catch (e) {
log.error('[WebService] Failed to kill Windows process:', e);
}
} else {
child.kill('SIGKILL');
}
};

// Set timeout (30 seconds for start) with enhanced logging
timeout = setTimeout(() => {
child.kill();
reject(new Error('Start script execution timeout'));
terminateProcess('timeout');
isResolved = true;
reject(new Error(`Start script execution timeout after ${this.startTimeout}ms`));
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Node.js child.kill('SIGKILL') Windows detached process tree termination behavior

💡 Result:

On Windows, ChildProcess#kill('SIGKILL') does not mean “kill the whole detached process tree”.

  • Windows doesn’t have real POSIX signals. In Node.js on Windows, the signal argument is mostly ignored and the target process is terminated forcefully (SIGKILL-like). Practically, child.kill('SIGKILL') is just “terminate that PID”. [1]
  • Killing a process on Windows does not automatically kill its children. Windows process termination (e.g., TerminateProcess) does not terminate child processes the target created, so grandchildren/orphans can remain running. [2]
  • detached: true makes this more likely, because the child can keep running independently after the parent exits on Windows (often with its own console), and it’s no longer coupled to the parent’s lifetime. [1]

What to do if you need “kill the whole tree” on Windows

Use taskkill with /T (tree) and usually /F (force):

  • /T = terminate the specified process and any child processes started by it [3]
  • /F = force termination (commonly required in practice) [3]

Many Node projects use tree-kill, which on Windows explicitly runs:
taskkill /pid PID /T /F [4]

Bottom line: child.kill('SIGKILL') on Windows is single-PID termination; it is not process-tree termination—especially with detached: true. Use taskkill /T /F (directly or via tree-kill) when you need to reliably kill the whole tree. [1][2][3][4]

Sources: [1] Node.js child_process docs, Windows signals + detached behavior; [2] Microsoft “Terminating a Process” (child processes not terminated); [3] Microsoft taskkill docs (/T, /F); [4] tree-kill npm docs (Windows uses taskkill /T /F).


🌐 Web query:

Windows taskkill /T /F vs Node.js child.kill() process tree

💡 Result:

What taskkill /T /F does (Windows)

  • taskkill /F forcefully ends the target process (no graceful shutdown).
  • taskkill /T also ends the target process’s child processes (i.e., it walks the parent→child relationship and kills the whole tree). [1]

So taskkill /PID <pid> /T /F is explicitly a “kill process tree, forcibly” operation on Windows. [1]

What child.kill() does (Node.js)

  • subprocess.kill() (aka child.kill()) targets only that one PID (the immediate child you spawned). It does not inherently kill the child’s own children (“grandchildren”). (Node’s docs even call out that child-of-child processes may survive.) [2]
  • On Windows, POSIX signals don’t really exist; Node treats kill() as an abrupt, forceful termination (signals are ignored except a small set, and the result is still “killed forcefully and abruptly”). [2]

Key difference (the practical answer)

  • taskkill /T /F = process-tree kill (recursive) + force. [1]
  • child.kill() = single-process kill (non-recursive). On Windows it’s also effectively forceful, but it’s still not a tree kill. [2]

Related gotcha: “process group” killing in Node on Windows

  • Node’s process.kill(pid, ...) supports “process group” semantics on POSIX (negative PIDs), but Windows will throw if you try to kill a process group. [3]

If you need taskkill /T behavior from Node on Windows, you typically have to implement it yourself (common approach: spawn taskkill /T /F for that PID), because child.kill() doesn’t provide a built-in process-tree kill. [1][2]

Sources: Microsoft taskkill docs [1]; Node.js child_process kill behavior (Windows + non-tree) [2]; Node.js process.kill() Windows process-group note [3].


🌐 Web query:

Node.js child_process SIGKILL Windows signal handling detached processes

💡 Result:

On Windows, Node.js does not have real POSIX signal semantics, so a lot of Unix patterns around SIGKILL, process groups, and “detached” behave differently:

  • SIGKILL can’t be handled (no listener). It always terminates immediately. Node documents that SIGKILL cannot have a listener installed and unconditionally terminates the process. [1]
  • Signals are mostly emulated on Windows. In Node’s child_process docs, Windows “signals do not exist”; the signal argument to subprocess.kill(signal) is largely ignored, and the child is killed forcefully/abruptly (effectively like SIGKILL), with only a small set of signal names recognized. [2]
  • detached: true on Windows means the child can keep running after the parent exits and will get its own console window (and “once enabled… cannot be disabled”). [3] This is not the same as Unix “new session + process group leader” behavior you might rely on for group-killing via negative PIDs.

Practical implications / how to kill

  1. Killing the detached child itself (single PID):

    • If you still have the PID, process.kill(child.pid) / child.kill() can terminate that PID, but don’t expect graceful signal delivery on Windows—termination is abrupt per Node docs. [2]
  2. Killing a detached process tree (child + grandchildren):

    • Node does not automatically kill the whole tree for you. On Windows the common approach is to use taskkill with /T (tree) and usually /F (force):
      • Microsoft documents taskkill /T as: ends the specified process and any child processes started by it. [4]

References

  1. Node.js process signal events (SIGKILL can’t be listened for; unconditional termination). (nodejs.org)
  2. Node.js child_process docs: Windows signal argument behavior / forceful termination. (nodejs.org)
  3. Node.js child_process docs: options.detached behavior on Windows. (nodejs.org)
  4. Microsoft taskkill docs: /T kills process tree. (learn.microsoft.com)

Citations:


Windows timeout termination does not kill detached process trees.

On Windows, child.kill('SIGKILL') terminates only the immediate process PID and does not kill child processes it spawned. This means a timed-out start script can leave the spawned service running, causing port conflicts. Use taskkill /T /F to reliably kill the entire process tree.

Suggested fix
-        if (process.platform === 'win32') {
-          try {
-            child.kill('SIGKILL');
-          } catch (e) {
-            log.error('[WebService] Failed to kill Windows process:', e);
-          }
-        } else {
-          child.kill('SIGKILL');
-        }
+        if (process.platform === 'win32') {
+          try {
+            spawn('taskkill', ['/F', '/T', '/PID', String(child.pid)], {
+              stdio: 'ignore',
+              windowsHide: true,
+            });
+          } catch (e) {
+            log.error('[WebService] Failed to kill Windows process tree:', e);
+          }
+        } else {
+          child.kill('SIGKILL');
+        }
🤖 Prompt for AI Agents
In `@src/main/web-service-manager.ts` around lines 247 - 268, The Windows branch
in terminateProcess does not kill a detached process tree; update
terminateProcess to, when process.platform === 'win32', check child.pid and call
a Windows taskkill command (e.g., taskkill /T /F <child.pid>) to terminate the
whole tree, falling back to child.kill('SIGKILL') if taskkill fails, and keep
the existing try/catch to log errors (referencing terminateProcess, child,
isResolved, timeout). Ensure you use a synchronous or properly awaited
spawn/exec so termination completes before resolving/rejecting, and include the
pid in log messages for clarity.

newbe36524 added a commit that referenced this pull request Feb 14, 2026
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Release Notes

* **New Features**
* Added code signing for Windows artifacts using Azure integration with
automatic verification and failure notifications
* Implemented single-instance lock to prevent multiple app instances
from running simultaneously
* Suppressed console windows on Windows during background script
execution for a cleaner user experience

* **Bug Fixes**
  * Fixed Windows dependency check script to auto-exit after completion
* Improved process timeout handling with enhanced logging and
termination logic
* Enhanced cross-platform consistency for process management and error
handling

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@coderabbitai coderabbitai bot mentioned this pull request Feb 16, 2026
@coderabbitai coderabbitai bot mentioned this pull request Feb 25, 2026
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.

1 participant