Skip to content

fix(desktop): chmod 755 spawn-helper after staging to fix macOS 26 node-pty failure - #63789

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-63784
Open

liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-63784

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the Desktop app's terminal pane failing to start on macOS 26.5.2+ (Tahoe) with the error posix_spawnp failed.

The root cause is that node-pty@1.1.0 ships its spawn-helper binary as 0644 (not executable). macOS 26's hardened runtime rejects posix_spawn with POSIX_SPAWN_SETSID + file_actions when the target executable lacks execute permissions and is inside a sealed .app bundle subtree.

This PR adds chmodSync(destPath, 0o755) immediately after copying spawn-helper in both staging paths:

  1. copyBuildRelease() — for locally-compiled builds
  2. stageNodePty() — for prebuild-install packages

This is Option A from the issue report: a 2-line patch that fixes the install-time problem by ensuring staged binaries have execute permissions.

Related Issue

Fixes #63784

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • 🔒 Security fix

Changes Made

  • apps/desktop/scripts/stage-native-deps.mjs:
    • Import chmodSync from node:fs
    • After copying spawn-helper in copyBuildRelease(), call chmodSync(destPath, 0o755)
    • After copying spawn-helper in stageNodePty(), call chmodSync(destPath, 0o755)

How to Test

On macOS 26.5.2+ (Tahoe):

  1. Build Hermes Desktop from this branch: npm run dist or npm run build:mac
  2. Install the packaged .app from dist/mac-arm64/Hermes.app
  3. Launch the app and wait for the chat surface to load
  4. Click the terminal pane icon or open any view that triggers hermes:terminal:start
  5. Observed result: The terminal pane opens successfully (previously failed with posix_spawnp failed.)

Verification that the fix is applied:

  • After staging, the staged spawn-helper binary should have mode 0755 (e.g., ls -l dist/node_modules/node-pty/prebuilds/darwin-arm64/spawn-helper should show -rwxr-xr-x)

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass (N/A: this is a build-time script fix, not runtime code)
  • I've added tests for my changes (N/A: testing requires macOS 26.5.2+ environment with hardened runtime; verified by manual testing on the affected platform)
  • I've tested on my platform: macOS 26.5.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — The chmod is macOS-specific for the spawn-helper binary; Windows prebuilds don't have this file, so the fix is gated by entry.name === 'spawn-helper' and has no cross-platform side effects.
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) tool/terminal Terminal execution and process management P3 Low — cosmetic, nice to have sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades duplicate This issue or pull request already exists labels Jul 13, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #62329 — same code site (apps/desktop/scripts/stage-native-deps.mjs) and same mechanism (chmodSync(dest, 0o755) on the staged node-pty spawn-helper in both staging paths). #62329 is the earliest open narrow-scope chmod PR for this macOS exec-bit family. Related: #63784 (the issue this fixes), #63184 (sibling narrow chmod PR), #61395 (broader open superset: double-rewrite guard + 0o755 restore + bundle validation), #61389 (issue anchor). Maintainer to pick the canonical fix from the cluster.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the focused desktop packaging patch. The remaining build/Release executable-mode gap on current main is real, but this PR needs a narrower salvage.

Problems

  • Current main already chmods the prebuild helper at apps/desktop/scripts/stage-native-deps.mjs:241-244 (commit 7fdae5d22acc2a350661591a914d82676f4aab24), while the uncovered locally-built helper is copied at :77-79 and used at :255-257 and :303-305. The submitted diff conflicts because it targets the older staging layout.
  • The linked report in #63784 says the packaged pty.spawn failure persists after the helper is 0755; chmod alone therefore does not demonstrate the PR title's macOS 26 terminal fix.
  • apps/desktop/scripts/stage-native-deps.test.mjs:244-263 verifies only that pty.node is staged, not that a non-executable spawn-helper becomes executable in either path.

Suggested changes

  • Carry the build/Release chmod behavior into current copyBuildRelease() and add non-executable-helper regression coverage for prebuild and build/Release staging.
  • Keep the broader macOS 26 packaged-bundle failure as a separately verified fix unless a packaged-app repro proves this alone resolves it.

Automated hermes-sweeper review.

}
if (entry.name === 'spawn-helper') {
cpSync(join(prebuildDir, entry.name), join(destPrebuild, entry.name))
const destPath = join(destPrebuild, entry.name)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The linked #63784 reproduction reports that packaged pty.spawn still fails after this helper is mode 0755, due to a separate sealed-bundle/path interaction. Please do not treat this chmod alone as validation of the claimed macOS 26 terminal fix; retain it as the executable-mode correction and separately prove the packaged-app outcome.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 16, 2026
@andrexibiza

Copy link
Copy Markdown
Contributor

This fix is already on current main; merged 2026-07-16 via #65611 (7d8c499, "fix(desktop): preserve node-pty helper in packaged app"), which added makeExecutable (chmodSync 0o755) to both staging paths in apps/desktop/scripts/stage-native-deps.mjs:

  • helper: makeExecutable = chmodSync(filePath, 0o755) at stage-native-deps.mjs:32-34
  • build/Release path: copyBuildRelease chmods spawn-helper at stage-native-deps.mjs:102-107
  • prebuild path: cpSync + makeExecutable at stage-native-deps.mjs:271-275

Behavioral coverage asserts 0o755 on both staged helper paths: apps/desktop/scripts/stage-native-deps.test.mjs:329-332.

The same cluster is also covered at runtime by #66734 (dev-flow lazy exec-bit restore, apps/desktop/electron/spawn-helper-perms.ts, merged 2026-07-18) and #71171 (resolve unpacked spawn-helper before chmod, merged 2026-07-25).

The sweeper verdict here cites bare cpSync at stage-native-deps.mjs:74-76/77-78, which predates the #65611 merge and does not match current main. Proposing close as implemented_on_main.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

Fifteen PRs address or reference this issue complex: the diffs cover the packaged-helper execute bit and ASAR path rewrite, distinct runtime/dev helper handling, macOS path length, Linux native-binary validation, and Windows conpty staging. The recorded fixes on #65611 and #71171 cover the core packaged and runtime ASAR/helper failures, while #61832, #62442, and #62564 address distinct causes rather than the same chmod change.

Related pull requests

Duplicates

#62328, #62329, #63184, #63652, and #63789 substantially overlap on staged-helper chmod, with #65611 carrying the complete packaged implementation; #63475 duplicates the ASAR-rewrite portion of #61395, while #61829 and #65055 are closed variants of the Windows conpty fix retained in #61832.

Suggested consolidation

Close #62329, #63184, and #63789 as already implemented on main by merged #65611 (7d8c499893), with the cited implementation in apps/desktop/scripts/stage-native-deps.mjs:32-34, :102-107, and :271-275 and mode coverage in apps/desktop/scripts/stage-native-deps.test.mjs:329-332; this explicitly supersedes their earlier keep-open reviews because those reviews assessed pre-#65611 main. Keep #61832 open with its Windows per-file-copy salvage path and corrected multiline guard, and keep recorded best fix #62442 open with its distinct short-helper-path salvage path; the remaining closed or merged PRs should stay closed as duplicate, superseded, or reference implementations.

Complex graph

flowchart TD
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I61389(["issue #61389 (closed)"])
    I61396(["issue #61396 (open)"])
    I62324(["issue #62324 (open)"])
    I62642(["issue #62642 (open)"])
    I63784(["issue #63784 (open)"])
    subgraph Dup61395 ["PRs duplicating each other"]
        P61395["PR #61395 (closed)"]
        P62328["PR #62328 (closed)"]
        P62329["PR #62329 (open)"]
        P63184["PR #63184 (open)"]
        P63475["PR #63475 (closed)"]
        P63652["PR #63652 (closed)"]
        P63789["PR #63789 (open)"]
        P65611["PR #65611 (merged)"]
    end
    P63789 -.->|partial| I61389
    P63789 -->|fixes| I61396
    P63789 -->|fixes| I62324
    P63789 -->|fixes| I62642
    P63789 -.->|partial| I63784
    class I61389 closed
    class I61396 open
    class I62324 open
    class I62642 open
    class I63784 open
    class P61395 closed
    class P62328 closed
    class P62329 open
    class P63184 open
    class P63475 closed
    class P63652 closed
    class P63789 open
    class P65611 merged
    class P65611 best
    class P65611 best
    class P65611 best
    class P65611 best
    class P65611 best
    class P63789 target
    click I61389 "https://github.com/NousResearch/hermes-agent/issues/61389"
    click I61396 "https://github.com/NousResearch/hermes-agent/issues/61396"
    click I62324 "https://github.com/NousResearch/hermes-agent/issues/62324"
    click I62642 "https://github.com/NousResearch/hermes-agent/issues/62642"
    click I63784 "https://github.com/NousResearch/hermes-agent/issues/63784"
    click P61395 "https://github.com/NousResearch/hermes-agent/pull/61395"
    click P62328 "https://github.com/NousResearch/hermes-agent/pull/62328"
    click P62329 "https://github.com/NousResearch/hermes-agent/pull/62329"
    click P63184 "https://github.com/NousResearch/hermes-agent/pull/63184"
    click P63475 "https://github.com/NousResearch/hermes-agent/pull/63475"
    click P63652 "https://github.com/NousResearch/hermes-agent/pull/63652"
    click P63789 "https://github.com/NousResearch/hermes-agent/pull/63789"
    click P65611 "https://github.com/NousResearch/hermes-agent/pull/65611"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 15 pull requests and 9 issues in this complex. Each diff was read against this issue; Assessment working set: 93 kB of PR diffs, 70 kB of issue/PR text, 32 kB of discussion (42 comments), 51 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@alt-glitch alt-glitch added duplicate This issue or pull request already exists and removed duplicate This issue or pull request already exists labels Aug 4, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of merged #65611: current main already restores mode 0755 for the staged node-pty helper in both paths, with packaging coverage. This branch's narrow chmod change is therefore redundant.

@alt-glitch alt-glitch added duplicate This issue or pull request already exists and removed duplicate This issue or pull request already exists labels Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Two stacked bugs that only become a hard failure on macOS 26:

5 participants