Skip to content

cherry-pick: Honor install-script policy in custom component commands (v5.2) - #2575

Closed
github-actions[bot] wants to merge 3 commits into
v5.2from
cherry-pick/v5.2/pr-2572
Closed

github-actions[bot] wants to merge 3 commits into
v5.2from
cherry-pick/v5.2/pr-2572

Conversation

@github-actions

@github-actions github-actions Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Resolves the committed conflict markers from #2572's v5.2 backport. Custom component commands now honor the lifecycle-script policy, and the deprecated operation uses its validated policy while preserving v5.2's install arguments and scripts-enabled operation default.

For the human reviewer

  • Planning alternative adopted. Framing-Verdict: better-alternative-exists was resolved by keeping the narrower release-compatible implementation: retain v5.2 automatic installs and legacy dryRun, consume Joi conversions locally for the new policy, and drop copied tests for unrelated main behavior. Porting main's production-work predicates, audit flags, shared argument builder or dry-run changes would expand this backport beyond Honor install-script policy in custom component commands #2572.
  • Release behavior. This implements the already-approved Honor install-script policy in custom component commands #2572 policy. Existing custom commands suppress lifecycle scripts unless install_allow_scripts: true or root-config install.allowInstallScripts: true is set; cold installs relying on native-module or generation hooks need that opt-in. The warning goes to server logs. The v5.2.x release note should state: “install_command runs with npm lifecycle scripts disabled unless install_allow_scripts: true (operations API) or install.allowInstallScripts: true (root config) is set.” Arbitrary custom commands can override npm configuration, and package managers that ignore it are outside this best-effort enforcement.
  • Known upstream input gap. The unchanged deploy_component validator accepts string booleans but forwards the raw request. install_allow_scripts: "false" is truthy at the custom-command boundary; use JSON boolean false. The same mapping is present in source commit 7f0d5a08c; correcting it across deployment routes needs a separate upstream fix. The deprecated operation in this PR does consume converted string false values.
  • Merge method. Squash merge is recommended because the existing automation commit contains unparseable conflict markers. This repair adds a new commit and leaves the branch history intact. The PR remains draft for human merge from Dispatch.
  • CI baseline decision. Required Node 24 workflows have finished with failures in unchanged HNSW, certificate-verification and startup code. This old branch lacks v5.2's Node 24 issuer-chain fix #2827. Recommend merging current v5.2 (78e52844548477f97441ec6c9fb56a2a1b723ec5) into this branch as a new merge commit, then rerunning both workflows. A read-only merge preview is clean and leaves exactly this five-file backport relative to v5.2. No merge has been performed; dispatch is awaiting the owner's scope decision.
  • Review limitation. The full round completed with Claude, Gemini, Cursor Composer and domain adjudication. The final editorial delta has independent Claude/Gemini coverage, but the CLI killed its domain adjudication leg after 720 seconds without output. The exact-head receipt is valid; the aggregate ledger remains unconverged. No new code change was requested by the completed delta lenses.

Product and architecture tour

Install policy on the release line

How is the approved lifecycle policy applied without changing v5.2's installation conventions?

In components/Application.ts, the custom command keeps its arguments and passes a dedicated flag into nonInteractiveSpawn; child-environment construction removes case variants and sets npm_config_ignore_scripts=true when scripts are disallowed. Explicit opt-in retains inherited host restrictions. The existing automatic and declared package-manager paths stay as they were.

In utility/npmUtilities.ts, the private Joi validator returns converted values with the existing validation options. install_node_modules defaults to scripts enabled, accepts either policy spelling, rejects both together and adds --ignore-scripts for false. The operation retains install --force --omit=dev --json, its preparation lock and legacy dryRun behavior. Root DESIGN.md records the policy and command-enforcement boundary in the release branch's existing documentation structure.

Verification

  • npm run build passes. The unresolved snapshot failed with TS1185: Merge conflict marker encountered; no markers remain in tracked files.
  • npx mocha unitTests/components/applicationInstall.test.js unitTests/utility/npmUtilities.test.js: 12 passing. Component tests retain real npm undefined/false/true lifecycle-marker checks, warnings, exact custom argv and release-compatible automatic arguments. Deprecated-operation tests cover defaults, both spellings, string false conversion, operation metadata, invalid-policy rejection and legacy dryRun. Fixture registry audit is disabled only in the test environment, and environment/config state is restored.
  • Five policy regressions fail at the expected lifecycle/rejection assertions against pre-backport base 1af3d430363e and pass with this resolution; that baseline builds successfully.
  • Full npm run format:check, changed-file oxlint and git diff --check pass. Full npm run lint reports 13 existing warnings in files identical to fetched origin/v5.2; no changed-file warning.
  • Policy-specific proof is at the real child-process boundary. Existing operations-to-install mapping is unchanged; the full integration suite has no policy-specific deploy_component assertion.
  • Exact-head Node 24 dispatches both completed failure at 49f266b592a94fddebf78ce11a57f9c1c767ece3. Unit Test: the new install-policy suites passed; the sole failing resource test is HNSW greedy routing (unitTests/resources/vectorIndex.test.js:1009). Integration Tests: 20 jobs passed and 7 failed. CRL/OCSP shards 3/4 fail on Node, uWS and Windows (401 expected, 404 received); uWS shard 6 fails during Harper startup in eviction-index-orphan-removal-paths.test.ts before its assertions. These source/test areas are identical to the pre-backport base.
  • Current v5.2 at 78e52844548477f97441ec6c9fb56a2a1b723ec5 has successful unit and integration runs. It contains the matching TLS fix and later native/startup changes; merging it is a proposal to align the baseline, not a claim that the remaining failures are proven fixed.

Refs #2572

Complexity: medium

Generated by GPT-5 Codex.

Review-Coverage: authored=codex; ran=cursor-composer,gemini,claude; adjudicated=domain; declined=cursor-grok,cursor-kimi,cursor-muse; rounds=2; full=1 @ 49f266b

Human-Review-Need: 3 @ 49f266b

Apply npm's ignore-scripts configuration to custom component install commands when scripts are disallowed, and let install_node_modules pass its explicit policy through the shared argument builder without changing its historical default.

Co-Authored-By: GPT-5 Codex <noreply@openai.com>
kriszyp and others added 2 commits October 1, 2026 18:02
Keep v5.2's automatic install paths and legacy npm arguments while applying
#2572's custom-command and deprecated-operation lifecycle script policy.
Consume converted policy values locally, adapt the copied tests to release
semantics, and keep the design record in the release branch's root document.

Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: cherry-resolve-kriszyp_harper_2572-910d0d15
Remove unused workspace and production-install fixtures from the copied
argument tests, and state the current operation contract in the design notes.

Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Dispatch-Task: cherry-resolve-kriszyp_harper_2572-910d0d15
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