Skip to content

Fix hidden npm workspace members left unpatched (#1827) - #1829

Open
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-npm-hidden-workspace-members
Open

Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-npm-hidden-workspace-members

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 29, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1827

Summary

Agent mode now crawls a declared npm/yarn/Bun workspace member that lives
in a hidden directory, for example a JavaScript GitHub Action kept at
.github/actions/<name> or an explicit .config/tool entry. Before
this change, scan --mode agent / apply left that member's
node_modules copy unpatched, apply --check said "in sync", and
vex attested not_affected. Hosted vex's installed-copy check missed
the same copy.

Root cause

The project walk prunes hidden and build dirs. #1627 added
skipped_members_of (crates/socket-patch-core/src/crawlers/npm_crawler.rs)
to add back declared members the walk prunes, but only members whose
path crosses a SKIP_DIRS name, and its own walk refused every
dot-named dir. So a hidden member was never added back.

Fix

  • The member walk follows a hidden dir where the pattern spells the
    dot, either in the literal prefix (.github/actions/*,
    .config/tool) or in the glob part (build/.*,
    {.github/actions,tools}/*). A path through a hidden dir is matched
    with no wildcard matching a dot component, which is how pnpm, Bun and
    vlt read patterns.
  • A member is added back when its path crosses a hidden name or a
    SKIP_DIRS name. Both rules now come from one walk_prunes_name
    helper that the main walk's workspace_children also uses, so the
    two can't drift apart again.
  • docs/ecosystems.md describes the rule.

No changes to the npm/PyPI/gem wrappers: the crawl runs only in the
Rust core.

Test evidence

Each new test fails on main and passes with the fix:

Test without fix with fix
crawler_npm_e2e::crawl_all_finds_declared_members_in_hidden_dirs FAILED ok
in_process_alternate_installers::apply_patches_workspace_member_in_a_hidden_dir (.github/actions/x glob, .config/tool literal) FAILED ok
in_process_alternate_installers::apply_check_and_vex_see_unpatched_member_in_a_hidden_dir FAILED ok
crawler_npm_e2e::crawl_all_still_skips_undeclared_hidden_dirs (control) ok ok
  • cargo fmt --all -- --check: clean.
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • crawler_npm_e2e 125/125, core crawlers:: unit tests 780/780,
    in_process_alternate_installers 42/42.
  • cargo test --workspace --all-features --no-fail-fast: everything
    passes except 11 tests that need things this sandbox doesn't have. Some
    need a non-root user (chmod-based "unremovable"/write-failure tests in
    repair, in_process_redirect, and core vendor::pypi_*,
    vlt_heal, copy_tree). The other two are e2e_bun_lockb tests,
    one of which needs patches-api.socket.dev. None of them touch the npm
    crawler. CI is the authority on these.
  • Real npm 10.9.4 repro from the issue (packages/* +
    .github/actions/*, hand-staged manifest for left-pad@1.2.0):
    before apply, apply --check exits 1 and vex writes no
    not_affected. apply patches both packages/y and
    .github/actions/x copies, and apply --check then exits 0.

Per-issue checklist

/code-review high findings

Fixed in 3b0c72a:

  • Walking into every hidden dir let a ** pattern spend its 20k-dir
    budget in .git / .next and drop real members. Build output such
    as .output/server and hidden worktrees could also become "members",
    and Bun/pnpm projects were read with npm's dot rule. Now a hidden dir
    counts only where a pattern spells its dot.
  • The duplicated prune predicate is now one shared walk_prunes_name.
  • The cheap listing checks now run before the multi-dialect glob match.

Bugbot (fixed in a1eb7df): a brace pattern such as {.github,packages}/**
let the walk descend into every hidden dir because the unexpanded remainder
contained a dot. Braces are now expanded before the walk, each alternative
walks from its own literal prefix, and only an alternative that spells a dot
descends into hidden dirs, with one shared budget per pattern.

Not fixed (on purpose):


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
A workspace member declared under a hidden directory, such as a
GitHub Action kept at .github/actions/<name> or an explicit
.config/tool entry, was never crawled in agent mode. Its nested
node_modules copy stayed unpatched while apply --check reported in
sync and vex attested not_affected.

The member walk that adds back declared members the project walk
prunes now descends into hidden dirs and keeps members whose path
crosses a hidden name, as it already did for build-dir names. Each
package manager's glob reading still decides whether a wildcard
matches a dot component.

Fixes #1827

Assisted-by: Claude Code:claude-opus-5-5
The previous commit let wildcards walk into every hidden dir. A **
pattern could then spend its walk budget inside .git or .next and
drop real members, and build output such as .output/server could
be crawled as a member that no package manager installed.

A hidden dir now counts only where the workspaces pattern spells
its dot (.github/actions/*, .config/tool), the way pnpm, Bun and
vlt read patterns. The walk and the member check also share one
pruned-name rule with the main walk, and run the cheap listing
checks before the glob match.

Refs #1827

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 29, 2026 15:33
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

1 similar comment
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/crawlers/npm_crawler.rs Outdated
A pattern such as {.github,packages}/** has a dot only in one brace
alternative, but the walk treated the whole pattern as spelling a
dot and descended into every hidden dir, .git and .next included.
That could use up the walk's dir budget and drop real members.

Each brace alternative now walks from its own literal prefix, and
only an alternative that spells a dot descends into hidden dirs.
The alternatives of one pattern share its dir budget.

Refs #1827

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit a1eb7df. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 29, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at a1eb7df9887e673a16302ea1c9dccc1776f16b71.

  • CI: 31/31 run checks green (19 skipped by path filters); no main-wide failures.
  • Bugbot: reviewed a1eb7df with no new issues; the one earlier thread is resolved and outdated.
  • Mergeable: yes, no conflicts with main.
  • For the reviewer: the npm workspace-member walk now follows declared members under hidden dirs (.github/, .config/). Check workspace_globs.rs, where it decides which hidden dirs are crawled.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. Agent-mode crawls of npm, yarn and Bun projects now include declared workspace members that sit in a hidden directory, such as .github/actions/<name> or .config/tool. Before this, apply left those members' node_modules unpatched while apply --check and vex reported them as fine (#1827). A hidden directory is followed only when a workspaces pattern spells its dot, so ** never walks into .git or .next.

Risk: low. The change is in one function (skipped_members_of) that only adds crawl roots for declared members, and both the walk's prune rule and the member rule now come from one helper. The worst case is an extra declared member getting crawled. Each pattern keeps its directory budget.

Look here

  • npm_crawler.rs:102: walk_prunes_name, now shared by the main walk and the member add-back.
  • npm_crawler.rs:143: braces are expanded before the walk, and all alternatives share one budget.
  • npm_crawler.rs:184 and :221: the walk descends into a hidden directory only when the glob part names a dot.
  • npm_crawler.rs:207: a path through a hidden directory is matched with dot: false (the pnpm/Bun/vlt reading).
  • crawler_npm_e2e.rs:1046 and :1098: positive cases, plus negative controls (.output/server, .git/build, packages/.hidden, undeclared .cache/tool).

Verified

  • Read the full diff: about 300 lines across 6 files, with no unrelated edits and no debug leftovers. CHANGELOG.md is untouched.
  • Cross-platform: paths still split on both / and \\.
  • Ran cargo test -p socket-patch-core --test crawler_npm_e2e hidden locally on the head: 3/3 pass.
  • CI on the head: ci-ok and clippy are green. Burn-down reports 31/31 run checks green.
  • Bugbot reviewed a1eb7df with no new issues. 0 review threads are unresolved.

Changes I made: none.

Open questions: none blocking. The PR body lists two limits it leaves out on purpose: packages/.hidden reached only through a wildcard under npm/yarn's dot: true, and the members of a hidden nested workspace. Neither was crawled on main either.

Auto-merge is armed, so approving sends this straight to the merge queue.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

3 participants