Repository navigation
fix: prevent Dependabot from adopting the parent pnpm workspace - #2060
Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
CI attempt 1 failed because Docker Hub rejected unauthenticated pulls of the existing |
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed b822c9dbce160401e6ea0f42ae913c94f8148eba against b4370e20d2c905dd9a581ad90dd0dd6d673c6fa8. No actionable correctness or security findings in the 15-file change.
The explicit npm declaration uses Dependabot's existing selector to keep these 13 fixtures out of the ancestor pnpm workspace. I checked the pinned updater implementation and the fixture setup paths. Dependency versions, registry settings, runtime commands, compatibility exclusions, cooldowns, and workflow permissions are unchanged. The independent security review covered all 15 changed files and found no issues.
Hosted CI passed for this head. Node 22, Node 24.3, and coverage each reported 7,835 passing tests and 32 skipped, including all seven registry-setup tests. The Node 22 published-package contract check, Windows checks, and CodeQL also passed.
I did not run local tests or installs. The author's isolated npm/Bun/Deno probes were not independently rerun. Actual hosted Dependabot updater success remains a separate post-merge check; passing PR CI does not establish it.
Dependabot adopted the repository’s ancestor pnpm lockfile/workspace when updating the independently managed integration manifests, causing the hosted updater to fail with
misconfigured_tooling. Declarenpm@11.12.1in all 13 fixture manifests so Dependabot selects npm and omits the pnpm workspace path. Dependency versions, lockfiles, fixture runtime commands, compatibility exclusions, cooldowns, credentials, and Actions permissions are unchanged.This follows the package-manager selection guard in the exact updater source used by the failed job. It addresses the remaining hosted updater failure after #2053.
Validation: seven focused tests passed. An isolated probe of the deployed selector reproduces implicit pnpm selection and verifies explicit npm selection across all 13 manifests; installation/version objects are stubbed, so this is not claimed as a hosted run. Real npm, Deno 2.9.7, and Bun 1.4.2 commands accept the declaration and request synthetic package metadata from local Verdaccio-compatible routing. No real package was installed by those probes.
The required local install/build/types/declarations/lint/format checks passed. The full suite passed 7,818 tests and reproduced the identical 12 unchanged tests failing from local proxy warnings, Docker access/mount limits, and macOS ACL permissions. The user approved documenting those environment limitations and requiring hosted CI; no tests or protections are disabled.
Refreshed against main after #2062 merged, so hosted CI obtains the existing Docker test image from its pinned ECR Public source. Two fresh review rounds found no blockers; the refreshed local stack reproduced exactly the same 12 environment failures with 7,818 passing tests and all other checks passing. The fixture fix remains 15 files and 33 added lines.
Maintainer security review requested for fixture package-manager selection. After merge, verify the actual fixture updater results, separately from PR CI and hosted YAML acceptance. The separate app-authored version-PR validation is complete, and Actions-token PR creation/approval has been disabled.