Skip to content

Read package.json workspaces through one helper: five readers disagree on non-string items, null and empty lists #1900

Closed

Description

[agent] Filed by the scheduled architecture audit (audit-core). Register: register comment.

Kind: refactor. Register ID: A43.

Problem

The workspaces field of a package.json (the array form, or the object form's packages array) is parsed by hand in five places:

  • hosted::governing_root::workspace_patterns, governing_root.rs L1209-L1225.`` It skips non-string items, returns None when the field is absent, and is already used by `npm_crawler::skipped_members_of`.
  • npm_crawler::bun_workspace_pattern_members_sync, npm_crawler.rs L912-L937.``` A non-string item makes the whole read fail (pattern.as_str()?). An absent field or `null` reads as `Some([])`, and `!` patterns are dropped. The other caller in the same file hands `!` patterns on to `workspaces_include_as` (L124-L129).
  • update::channel::declares_workspaces, channel.rs L390-L399. It checks only for a non-empty array, so a list made only of non-string items counts as workspaces here, while workspace_patterns would return an empty list for it.
  • vendor/mod.rs, L237-L244. This is the yarn-classic workspace-risk warning.
  • patch::user_patches, L93-L104.

All five parse the same shape. Each one decides on its own what a malformed field means.

Proposed change

Move workspace_patterns into the npm-family format layer, for example formats::json::workspace_patterns(&serde_json::Value) -> Option<Vec<String>>, next to the existing package.json helpers. Call it from all five sites. The Bun walk then keeps only the expansion step. The open question for the PR: should a non-string item fail closed everywhere, or be skipped everywhere? It must be one answer, and the Bun walk's fail-closed rule is the stricter one.

Size: 5 files, about −60/+25 lines.

Acceptance criteria

  • Existing workspace tests pass: npm_crawler (Bun member and skipped-member tests), governing_root, update::channel and user_patches.
  • A new table test of the helper covers: an absent field, null, an empty array, the object form, a non-string item and a ! pattern.

Activity

  1. added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code
    on Nov 1, 2026
  2. mikolalysenko commented on Nov 1, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Triage: priority:p1 (npm-family package.json). No duplicate or open PR found. The PR still has to decide whether a non-string item fails closed or is skipped, as the issue notes.


    Generated by Claude Code

  3. mikolalysenko commented on Nov 2, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Claiming this issue (shared root cause: package.json workspaces parsed by hand in five places with different malformed-field rules). Branch: agent/fix-package-json-workspaces-reader. Claim-ID: 2026-11-02T04:21:18Z-15f6de


    Generated by Claude Code

  4. mikolalysenko commented on Nov 2, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Draft PR: #1971


    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p1refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions