Skip to content

fix: remove postinstall script to support pnpm strictDepBuilds - #576

Merged
zh-lx merged 1 commit into
mainfrom
fix/remove-postinstall-script
Jul 31, 2026
Merged

fix: remove postinstall script to support pnpm strictDepBuilds#576
zh-lx merged 1 commit into
mainfrom
fix/remove-postinstall-script

Conversation

@zh-lx

@zh-lx zh-lx commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Summary

Removes the postinstall lifecycle script from @code-inspector/core and its associated verify-terminal-runtime.js helper.

The postinstall script did two things:

  1. Fix execute permissions on node-pty's spawn-helper binary
  2. Probe PTY spawning to verify it works

Both are already performed at runtime in ai-terminal.ts (ensureNodePtySpawnHelperExecutable + verifyPtySpawn), so the postinstall was purely redundant.

Removing it unblocks users running pnpm with strictDepBuilds: true, which rejects all lifecycle scripts by default for security reasons.

Closes #575

Changes

  • packages/core/package.json — removed postinstall script and "scripts" from files
  • packages/core/scripts/verify-terminal-runtime.js — deleted (logic lives in ai-terminal.ts)
  • test/core/scripts/verify-terminal-runtime.test.ts — deleted (no longer needed)

Test Plan

  • Install @code-inspector/core in a project with pnpm strictDepBuilds: true — should succeed without whitelisting
  • Verify terminal feature still works at runtime (PTY check runs lazily via attachTerminalWebSocket)
  • Existing CI passes

The postinstall hook in @code-inspector/core ran verify-terminal-runtime.js
to fix node-pty spawn-helper permissions and probe PTY spawning. This same
work is already performed at runtime in ai-terminal.ts
(ensureNodePtySpawnHelperExecutable + verifyPtySpawn), making the postinstall
redundant.

Removing the postinstall unblocks users running pnpm with strictDepBuilds:true,
which rejects lifecycle scripts by default for security reasons.

Closes #575

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Remove @code-inspector/core postinstall to support pnpm strictDepBuilds

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Remove @code-inspector/core postinstall hook to unblock pnpm strictDepBuilds installs.
• Drop the bundled terminal verification script and its now-obsolete unit tests.
• Rely on existing runtime checks (spawn-helper chmod + PTY probe) in ai-terminal.
Diagram

graph TD
  A["Consumer install (pnpm)"] --> B["@code-inspector/core"] --> C{"Lifecycle scripts?"} --> D["No postinstall"]
  B --> E["Runtime server"] --> F["attachTerminalWebSocket"] --> G["Fix spawn-helper perms"] --> H["PTY spawn probe"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep postinstall; document pnpm allow-scripts/trustedDependencies
  • ➕ Terminal issues are surfaced at install time, before runtime traffic.
  • ➕ No behavior change to when verification runs.
  • ➖ Still blocked by pnpm strictDepBuilds defaults (primary reported issue).
  • ➖ Encourages weakening supply-chain protections to use the package.
2. Expose an explicit CLI/diagnostic command (opt-in verification)
  • ➕ Gives users a deterministic way to preflight terminal support in CI.
  • ➕ Avoids lifecycle scripts while still allowing early failure detection.
  • ➖ Adds extra surface area and user steps; still not automatic.
  • ➖ Duplicates runtime verification logic unless carefully shared.
3. Upstream/patch-level fix in node-pty packaging or helper perms
  • ➕ Addresses root cause (spawn-helper execute bit) for all consumers.
  • ➕ Could reduce need for any chmod logic in @code-inspector/core.
  • ➖ Longer lead time and coordination; not guaranteed to land quickly.
  • ➖ Does not cover PTY spawn probe failures unrelated to permissions.

Recommendation: Proceed with this PR’s approach: removing the postinstall is the most compatible and security-aligned fix for pnpm strictDepBuilds, and the terminal checks already exist in the runtime initialization path (attachTerminalWebSocket). Consider adding a short doc note (separately) clarifying that terminal capability is validated lazily at runtime and how to inspect the failure reason if terminal mode is disabled.

Files changed (1) +1 / -3

Other (1) +1 / -3
package.jsonRemove postinstall and stop shipping scripts directory +1/-3

Remove postinstall and stop shipping scripts directory

• Removes the postinstall lifecycle script that executed terminal runtime verification during installation. Updates the published files list to exclude the scripts directory, aligning packaging with runtime-only terminal checks and avoiding pnpm strictDepBuilds install failures.

packages/core/package.json

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

Grey Divider


Remediation recommended

1. Spawn-helper chmod too narrow 🐞 Bug ☼ Reliability
Description
Removing the @code-inspector/core postinstall hook leaves ensureNodePtySpawnHelperExecutable() as
the only permission repair, and it only chmods the prebuilds/<platform>-<arch>/spawn-helper path. If
node-pty’s helper binary is present in a different on-disk location in a given installation, execute
permissions will no longer be repaired and PTY startup can fail at runtime.
Code

packages/core/package.json[48]

-    "postinstall": "node ./scripts/verify-terminal-runtime.js",
Evidence
The PR removes the postinstall entry from packages/core/package.json, so installation-time
repair no longer runs. The remaining runtime repair function only targets the
prebuilds/<platform>-<arch>/spawn-helper path and is invoked from attachTerminalWebSocket(),
making it the sole remaining permission-repair mechanism.

packages/core/package.json[40-49]
packages/core/src/ai/server/ai-terminal.ts[104-118]
packages/core/src/ai/server/ai-terminal.ts[653-688]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR removes the installation-time `postinstall` verification/repair hook. The runtime repair helper (`ensureNodePtySpawnHelperExecutable`) currently only attempts to fix execute permissions for a single `prebuilds/<platform>-<arch>/spawn-helper` location, which is not necessarily the only possible location of node-pty’s helper binary.

## Issue Context
`attachTerminalWebSocket()` calls `ensureNodePtySpawnHelperExecutable()` before running the PTY self-test. With `postinstall` removed, any helper locations not covered by the runtime chmod attempt will never be repaired.

## Fix Focus Areas
- packages/core/src/ai/server/ai-terminal.ts[104-118]
- packages/core/src/ai/server/ai-terminal.ts[653-688]
- packages/core/package.json[40-48]

## Suggested change
Expand `ensureNodePtySpawnHelperExecutable()` to try chmod on a small set of candidate helper paths derived from the resolved `node-pty/package.json` directory (not just `prebuilds/.../spawn-helper`). Only add execute bits (e.g., OR with `0o111`) rather than broadening permissions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

"build:client:watch": " vite build --config ./vite.client.config.ts --watch",
"clear": "rimraf ./dist && rimraf ./types",
"build": "pnpm clear && tsc && pnpm build:server && pnpm build:client",
"postinstall": "node ./scripts/verify-terminal-runtime.js",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. Spawn-helper chmod too narrow 🐞 Bug ☼ Reliability

Removing the @code-inspector/core postinstall hook leaves ensureNodePtySpawnHelperExecutable() as
the only permission repair, and it only chmods the prebuilds/<platform>-<arch>/spawn-helper path. If
node-pty’s helper binary is present in a different on-disk location in a given installation, execute
permissions will no longer be repaired and PTY startup can fail at runtime.
Agent Prompt
## Issue description
The PR removes the installation-time `postinstall` verification/repair hook. The runtime repair helper (`ensureNodePtySpawnHelperExecutable`) currently only attempts to fix execute permissions for a single `prebuilds/<platform>-<arch>/spawn-helper` location, which is not necessarily the only possible location of node-pty’s helper binary.

## Issue Context
`attachTerminalWebSocket()` calls `ensureNodePtySpawnHelperExecutable()` before running the PTY self-test. With `postinstall` removed, any helper locations not covered by the runtime chmod attempt will never be repaired.

## Fix Focus Areas
- packages/core/src/ai/server/ai-terminal.ts[104-118]
- packages/core/src/ai/server/ai-terminal.ts[653-688]
- packages/core/package.json[40-48]

## Suggested change
Expand `ensureNodePtySpawnHelperExecutable()` to try chmod on a small set of candidate helper paths derived from the resolved `node-pty/package.json` directory (not just `prebuilds/.../spawn-helper`). Only add execute bits (e.g., OR with `0o111`) rather than broadening permissions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (565bd66) to head (05bd083).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #576   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           37        37           
  Lines         7032      7032           
  Branches      1986      1986           
=========================================
  Hits          7032      7032           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zh-lx
zh-lx merged commit 7860a66 into main Jul 31, 2026
5 checks passed
@zh-lx
zh-lx deleted the fix/remove-postinstall-script branch July 31, 2026 01:44
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.

Support for pnpm strictDepBuilds

1 participant