From 94a6c2eb5d93751394e8fcde6a0610fc0002c1c1 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:05:47 -0600 Subject: [PATCH 1/9] Plan one lock model for every lockfile reader Lockfile discovery (vex/discover), the lock inventory (vendor/lock_inventory) and the hosted and vendored writers each read the same project files their own way: two Socket-origin classifiers, three Python precedence rules, second walks of package-lock, yarn, vlt and pnpm locks, a second parse of uv/pylock/script locks, five Cargo.toml dependency-table walks, three readers of Maven's checksums.sha256. The inventory also has no NuGet, JVM, Hatch or Deno models. docs/design/lock-model-unification.md describes the target, one per-project model whose entries carry an Origin (Registry, Hosted, Vendored, Other, Rejected), file selection by install tree (precedence within a tree, union across trees), and the stack of PRs that gets there. scripts/loc-budget.py counts implementation lines (no comments, no cfg(test) items, no test-only modules) for two groups in scripts/loc-budget.json: lock-readers (30,898 lines today) and lock-writers (88,363). The lint-ecosystems job runs it with --warn, which annotates a group over budget but never fails, so bug fixes in these files are not blocked; each PR in the stack lowers the budget as it deletes code. No product code changes. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 6 + docs/design/lock-model-unification.md | 243 +++++++++++++++++++++++ scripts/loc-budget.json | 35 ++++ scripts/loc-budget.py | 271 ++++++++++++++++++++++++++ scripts/tests/test_loc_budget.py | 140 +++++++++++++ 5 files changed, 695 insertions(+) create mode 100644 docs/design/lock-model-unification.md create mode 100644 scripts/loc-budget.json create mode 100755 scripts/loc-budget.py create mode 100644 scripts/tests/test_loc_budget.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a6b071c44..d440fecd6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -208,6 +208,12 @@ jobs: - name: Python — test native installer harnesses run: python3 -B -m unittest discover -s scripts/tests -v + - name: Rust — lock model size budget + # Annotates, never fails: a bug fix elsewhere may add lines to these + # files. The lock-model PRs lower the budget as they delete code + # (docs/design/lock-model-unification.md#size-budget). + run: python3 -B scripts/loc-budget.py --warn + - name: Shell — shellcheck the curl|sh installer # shellcheck is pre-installed on ubuntu-latest. run: shellcheck --shell=sh scripts/install.sh diff --git a/docs/design/lock-model-unification.md b/docs/design/lock-model-unification.md new file mode 100644 index 000000000..9da9c15cb --- /dev/null +++ b/docs/design/lock-model-unification.md @@ -0,0 +1,243 @@ +# One lock model for every reader + +Status: in progress (stacked PRs `lock-model/*`). Owner decisions are dated +2026-11-04. + +Paths below are relative to `crates/socket-patch-core/src/`. + +## Problem + +Three groups of code read the same project files, and each reads them its own +way: + +* **Lockfile discovery** (`vex/discover/`) finds Socket wiring: the hosted + pins and vendored paths in lockfiles and package-manager configs. It is not + only for `vex`. `scan`, `vendor`, the hosted engine, the hosted unwind, + rollout and ledger liveness all call it. +* **The lock inventory** (`vendor/lock_inventory/`) lists every package a + lockfile resolves. `scan` and `get` use it to supplement the installed-tree + crawl, `vendor` uses it to fetch pristine artifacts, `repair` uses it to + recover ledger entries, and ledger liveness uses its every-lock view. +* **The writers** (`patch/redirect/`, `hosted/`, `vendor/`) parse the same + files again to plan their edits. + +`formats/` was started as the shared home ("one model per lockfile format"), +but only pnpm has both an entry view and a wiring view there. A survey of +every format on 2026-11-04 found the duplicated work below. What discovery +adds on top of a walk (the contests between locks, ledger claims and +attestation rules) is real policy and stays. + +| Duplicated work | Where | +|---|---| +| Two Socket-origin classifiers | discovery's `DiscoverCtx::locate` / `hosted_uuid` checks a Socket host allowlist. The inventory's `parse_vendor_path` / hosted-URL-leaf matching accepts any host. `utils/poetry_lock::lock_wiring` uses a third, looser substring test. | +| Three Python precedence rules | the inventory's chain (uv by parse success, then poetry or pdm if they have entries, then Pipfile.lock plus requirements), `formats/governing_locks.rs` (uv, poetry, pdm, Pipfile.lock by presence) and discovery (every lock, plus the cross-lock contest). They disagree on four common layouts. | +| Second walks of one file | `vendor/npm_origin.rs` walks package-lock `packages` again. Yarn writers call `scan_blocks` directly, and the Berry rewriter uses a third model (`formats/yarn/stanzas.rs`). The vlt writers use a separate line grammar. pnpm's `InstalledPackages` and `vendored_npm_uuids`, and the hosted planner's whole-text `grammar::entries`, skip the env-document filter. | +| Second parses of one file | `vex/discover/pypi_locks.rs` parses uv, pylock and script locks itself. The inventory parses them again. `deno.lock` is parsed by `crawlers/deno_crawler.rs` for discovery, the hosted engine and the npm flavor probe. | +| Repeated helper parsers | two Python lock listers, two `-r` include walkers, two PEP 508 direct-reference parsers, five walks of `Cargo.toml` dependency tables, two Rush lock listings, three readers of Maven's `checksums.sha256`, two Gradle Socket-index parsers, two routes to sbt resolution evidence | +| Two file-selection layers | the npm flavor router (`lock_inventory/npm_family.rs`) picks one family and one lock. Discovery reads every lock and sweeps the ignored ones with `recognize_ignored`. | + +The inventory also has gaps. It has no models for NuGet, Maven, Gradle, sbt, +Hatch or Deno, and it reads no `composer.lock` that starts with a BOM, which +discovery reads. + +## Goal + +One per-project lock model. Every consumer reads it, and the end state has +fewer implementation lines than today. `scripts/loc-budget.py` tracks the +lines (see [Size budget](#size-budget)). + +Non-goals: changing what any mode writes, or adding new hosted or vendored +backends. + +## The model + +### Entries + +Every reader yields `LockEntry` records. Each record holds the package +coordinates as the lock spells them, the integrity the lock records, the +resolved location, a source span for writers that splice the file, and an +`Origin`: + +```rust +pub enum Origin { + /// Resolves from a registry the package manager would use anyway. + Registry { resolved: Option }, + /// Routes to a Socket-hosted artifact. + Hosted { uuid: String, url: String }, + /// Routes to a committed `.socket/vendor///` artifact. + Vendored { uuid: String, artifact_rel: String }, + /// git, path, link, workspace, or a URL on another host. + Other(OtherSource), + /// Looks like Socket wiring but fails validation. Keeps the identity + /// the entry would have had, so contests and recognition still see it. + Rejected { would_be: Option, diag: Diag }, +} +``` + +Formats with install semantics that discovery's contests need also carry +flags: bundled, link, the v2 mirror, a `hasShrinkwrap` ancestor, Yarn's +`live`, Berry's checksum cache key, and so on. These are fields of the format's +record, not new side channels. + +One classifier, `formats::origin`, decides `Origin`. It is discovery's +validators moved, not rewritten: + +* the Socket host allowlist and the last-canonical-uuid rule + (`hosted_uuid`); +* `vendor_ref`, `vendor_ref_decorated` and `vendor_uuid_dir`; +* the `socket-patch-` name grammar; +* the validating purl builders. + +The inventory's any-host matching goes away. That is a behavior change only +where a non-allowlisted host serves a URL that looks like Socket's, and it +lands in the install-tree PR with a golden review. + +### Joins + +For JVM and NuGet, origin is not a property of one entry: + +* **Maven:** a pom dependency's `-socket.` version joins one + ``. +* **Gradle:** a lockfile entry joins a row of `.socket/gradle/hosted-index.tsv` + or `.socket/vendor/gradle-index.tsv`. +* **sbt:** pins live in owned files and are confirmed by resolution evidence. +* **NuGet:** `packages.lock.json` entries join `nuget.config`'s + ``. + +So a reader works on a project view, not on a single text. A reader may read +several files and still yield one list of records. The same applies to Cargo, +where `Cargo.toml` `registry =` pins join `.cargo/config.toml` registries, +and to Go, where `go.mod` `replace` directives join `go.sum` lines. + +### Install trees + +File selection is one function over `formats::registry()`: + +```rust +pub struct InstallTree { + /// The files the package manager reads for this tree, governing first. + pub files: Vec, + /// Present files the package manager ignores in favor of `files`. + /// They are swept for Socket identities (discovery rule 11) and never + /// read as wiring. + pub shadowed: Vec, +} +``` + +Rule: **precedence within a tree, union across trees.** + +* npm: `npm-shrinkwrap.json` governs and `package-lock.json` is shadowed. + pnpm: `pnpm-lock.yaml` governs `shrinkwrap.yaml`. Bun: `bun.lock` governs + `bun.lockb`, which `bun_text_lock_drives` already decides. +* **Python (owner decision):** each tool's lock is its own tree, because + which lock is installed depends on which tool runs. uv.lock, poetry.lock, + pdm.lock, Pipfile.lock, each pylock, each PEP 723 script lock, and + `requirements.txt` with its `-r` includes are separate trees. + Discovery's cross-lock contest stays. It is the safety check that withholds + a uv ref when poetry.lock pins the same release unpatched. The inventory + unions the trees, so `scan` sees a patch wired into an exported + `requirements.txt` beside `poetry.lock`. The inventory's precedence chain + and `governing_locks`' presence rule are deleted. The vendor router's + question ("which lock do I write?") becomes a query over the trees. +* **Deno (owner decision: deprecated but supported):** `deno install` reads + `deno.lock` for a `package.json` project's npm dependencies and never reads + the npm-family locks. `deno.lock` and the npm-family lock are one tree with + undecided governance: the files don't say which tool fills + `node_modules`. Today's outcome is kept: a ref in the npm-family lock stays + a ref but is marked unattested. It is now expressed as a tree property, + not the `UnwiredCopy` side channel. +* Rush subspace locks, workspace-member locks and Go workspace members are + trees of their own, as discovery already treats them. + +### Consumers + +* **Inventory**: every entry of every tree, read by the package it resolves. + Socket entries are listed as the package they replace, as today, so a + rescan of a wired checkout still lists it. Then path-safety guards and + `dedup_prefer_integrity`. `inventory_project_every_lock` becomes the + same list without the dedup. +* **Discovery**: the `Hosted`, `Vendored` and `Rejected` entries of each tree + become refs and diagnostics, the shadowed files are swept, and the contests + and ledger claims run as today. The `Registry` and `Other` entries of + other trees replace the `resolved_elsewhere` collection that each extractor + feeds by hand today. +* **Ledger liveness** reads `Origin` directly instead of looking for a uuid + inside the inventory's `resolved` URL. +* **Writers** plan over the same records, using the source spans. + +## JVM and NuGet support (owner decision: in scope) + +What committed files can give the inventory: + +| Ecosystem | Coordinates | Integrity | +|---|---|---| +| NuGet | every `(id, resolved)` per target framework in `packages.lock.json` | `contentHash`, which needs a new `LockIntegrity::NugetContentHash`. For signed packages, and nuget.org signs all of them, it hashes the archive with the signature stripped (`formats/nuget/package.rs::package_content_hash`), not the raw `.nupkg` bytes. | +| Gradle | the resolved GAVs per configuration in `gradle.lockfile` (settings and buildscript locks tagged as build logic) | `verification-metadata.xml`, which has editors but no reader today. `sha256` and `sha1` map to existing variants. `sha512` needs `Sha512Hex`. | +| Maven | the root pom's direct and managed GAVs only. Maven has no lockfile, so transitive dependencies are not in committed files. | `.mvn/checksums/checksums.sha256` lines, normally only for Socket pins | +| sbt | Socket pins only (owned files). No committed file resolves registry entries. | the jar sha256 in the owned file | + +Vendor fetch refuses entries without a verifier, and JVM and NuGet vendoring +take their bytes from caches, registry sidecars or the installed directory +anyway. So the new entries first serve `scan`/`get`, discovery and ledger +liveness. Ledger liveness also needs a `SourceKind` for "mapped to a +non-Socket NuGet source" and the Gradle equivalent, since these entries carry +no URL. + +## Deno + +Deno stays supported while deprecated. It is ported like any other format, so +deprecating it later means deleting one reader and one tree rule. The +`deno.lock` reader moves from `crawlers/deno_crawler.rs` into `formats/` and +serves the crawler, discovery, the hosted engine and the npm flavor probe. +Deprecation messaging is separate from this work. + +## Size budget + +`scripts/loc-budget.py` counts implementation lines: code outside comments, +outside `#[cfg(test)]` items and outside test-only modules. It compares the +groups in `scripts/loc-budget.json` with their maxima. CI's +`lint-ecosystems` job runs it with `--warn`, which annotates a group over +budget but never fails, so another PR's bug fix in one of these files never +fails on it. There are two groups: +`lock-readers` (`formats/`, `vex/discover/`, `vendor/lock_inventory/`, +`gradle/`, the Python lock helpers in `utils/`, `vendor/npm_origin.rs` and +`crawlers/deno_crawler.rs`) and `lock-writers` (`patch/redirect/`, `hosted/` +and the rest of `vendor/`). At 7bde3ba55 they hold 30,898 and 88,363 lines. +Each PR in this stack +lowers the maxima with `--lower` in the same commit. The NuGet and JVM PRs +add code and raise the readers' maximum by hand, saying why. + +## Stack + +Each PR is based on the one before. PRs 1 to 11 must leave +`tests/fixtures/vex-discover-golden/` and the redirect goldens byte for byte +unchanged. Only PR 12 may change them, and its golden diff is reviewed line +by line. + +| PR | Branch | Change | +|---|---|---| +| 0 | `lock-model/0-design` | This document and the size budget | +| 1 | `lock-model/1-origin` | `LockEntry`, `Origin` and `formats::origin` (discovery's classifier, moved). Discovery calls it. The inventory doesn't yet. | +| 2 | `lock-model/2-pnpm` | Pilot. One pnpm reader feeds the inventory, discovery and the hosted planner. Removes `InstalledPackages`, `vendored_npm_uuids`, the whole-text planner walk and the second Rush listing. | +| 3 | `lock-model/3-npm` | package-lock and shrinkwrap. Folds `vendor/npm_origin.rs`'s second walk into the records. | +| 4 | `lock-model/4-yarn` | Classic and Berry. One block model for every reader and writer, `BerryStanzas` included. | +| 5 | `lock-model/5-bun-vlt` | `bun.lock`, `bun.lockb` and vlt. The vlt writers move off their line grammar. | +| 6 | `lock-model/6-deno` | The `deno.lock` reader in `formats/`, for all four of its callers | +| 7 | `lock-model/7-python` | One lister, one parse per lock, one include walker and one PEP 508 parser. `pypi_locks.rs` stops parsing TOML itself. Poetry wiring uses `formats::origin`. Hatch gets inventory entries. | +| 8 | `lock-model/8-cargo-go` | One `Cargo.toml` dependency-table walk. Go `replace` and `go.sum` records. | +| 9 | `lock-model/9-gem-composer` | Gemfile.lock and composer.lock records. The inventory stops dropping BOM-prefixed `composer.lock` files. | +| 10 | `lock-model/10-nuget` | NuGet records with the config join, `NugetContentHash`, and inventory entries | +| 11 | `lock-model/11-jvm` | One `checksums.sha256` reader, one Gradle index parser, a `verification-metadata.xml` reader, then Gradle-lock and root-pom inventory entries and sbt's owned-file entries | +| 12 | `lock-model/12-install-trees` | Install-tree selection, Python trees, Deno governance, the single classifier for the inventory, liveness on `Origin`. Deletes the npm flavor router's lock choice, the Python precedence rules, `resolved_elsewhere` and the `UnwiredCopy` plumbing. | +| 13 | `lock-model/13-cleanup` | Removes the raw-text liveness fallbacks if the model makes them redundant, plus whatever is left of `vendor/lock_inventory/`. Final budget. | + +## Open questions + +* Whether discovery's hosted NuGet refs, which set `Sri("sha512-" + + contentHash)`, are verified anywhere against raw bytes. Socket-served + packages may be unsigned, in which case the two hashes agree. PR 10 checks + this before it adds `NugetContentHash`. +* Whether the raw-text fallbacks (`hosted_wiring_in_files`, + `vendored_wiring_live`) can go entirely. They exist for formats that no + reader reads and for patch servers outside the allowlist. PR 13 decides + from what is left by then. diff --git a/scripts/loc-budget.json b/scripts/loc-budget.json new file mode 100644 index 000000000..28d387299 --- /dev/null +++ b/scripts/loc-budget.json @@ -0,0 +1,35 @@ +{ + "groups": { + "lock-readers": { + "paths": [ + "crates/socket-patch-core/src/formats/", + "crates/socket-patch-core/src/vex/discover/", + "crates/socket-patch-core/src/vendor/lock_inventory/", + "crates/socket-patch-core/src/vendor/npm_origin.rs", + "crates/socket-patch-core/src/gradle/", + "crates/socket-patch-core/src/utils/python_lock.rs", + "crates/socket-patch-core/src/utils/poetry_lock.rs", + "crates/socket-patch-core/src/utils/pdm_lock.rs", + "crates/socket-patch-core/src/utils/requirements.rs", + "crates/socket-patch-core/src/utils/lock_fragments.rs", + "crates/socket-patch-core/src/utils/hatch.rs", + "crates/socket-patch-core/src/utils/pipenv.rs", + "crates/socket-patch-core/src/crawlers/deno_crawler.rs" + ], + "max": 30898 + }, + "lock-writers": { + "paths": [ + "crates/socket-patch-core/src/patch/redirect/", + "crates/socket-patch-core/src/hosted/", + "crates/socket-patch-core/src/vendor/" + ], + "exclude": [ + "crates/socket-patch-core/src/vendor/lock_inventory/", + "crates/socket-patch-core/src/vendor/npm_origin.rs", + "crates/socket-patch-core/src/vendor/test_support" + ], + "max": 88363 + } + } +} diff --git a/scripts/loc-budget.py b/scripts/loc-budget.py new file mode 100755 index 000000000..7ef6795a4 --- /dev/null +++ b/scripts/loc-budget.py @@ -0,0 +1,271 @@ +#!/usr/bin/env python3 +"""Count implementation lines of Rust code and compare them to a budget. + +A line counts when it holds code outside a comment. These never count: + +* blank and comment-only lines (doc comments included); +* items gated by ``#[cfg(test)]`` or ``#[cfg(all(test, ...))]`` (inline + ``mod tests { ... }`` blocks, test-only helpers and ``use`` lines); +* files declared as ``#[cfg(test)] mod name;`` and every module below them; +* files under a ``tests/`` or ``benches/`` directory. + +Budgets live in ``scripts/loc-budget.json``. Each group names path prefixes +(relative to the repo root), optional prefixes to leave out, and the most +implementation lines the group may hold: + + {"groups": {"readers": {"paths": ["crates/x/src/formats/"], + "exclude": ["crates/x/src/formats/old/"], + "max": 123}}} + +Usage: + + scripts/loc-budget.py # print every group's count + scripts/loc-budget.py --check # exit 1 when a group is over budget + scripts/loc-budget.py --warn # annotate groups over budget, exit 0 + scripts/loc-budget.py --lower # rewrite each max down to its count + scripts/loc-budget.py --files GROUP # per-file counts for one group + +``--check`` and ``--warn`` print a GitHub ``::warning::`` annotation per +group over budget; CI uses ``--warn`` so a bug fix that adds lines to these +files never fails on it. ``--lower`` never raises a max: a PR that grows a group raises the +number by hand, where a reviewer sees it. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import subprocess +import sys +from dataclasses import dataclass, field +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +BUDGET_FILE = REPO / "scripts" / "loc-budget.json" + +CFG_TEST = re.compile(r"#\[cfg\(\s*(?:test|all\(\s*test\b[^\]]*)\)\]") +RAW_STRING = re.compile(r'b?r(#*)"') +CHAR_LITERAL = re.compile(r"'(?:\\(?:x[0-9a-fA-F]{2}|u\{[0-9a-fA-F]+\}|.)|[^\\'])'", re.S) +MOD_DECL = re.compile( + r'(?:#\[path\s*=\s*"([^"]+)"\]\s*)?(?:pub(?:\([^)]*\))?\s+)?mod\s+(\w+)\s*;\s*$' +) + +CODE, COMMENT, STRING = 0, 1, 2 + + +def classify(src: str) -> bytearray: + """Mark every character of Rust source as code, comment or literal.""" + n = len(src) + kind = bytearray(n) + i = 0 + + def ident_before(at: int) -> bool: + return at > 0 and (src[at - 1].isalnum() or src[at - 1] == "_") + + while i < n: + c = src[i] + if src.startswith("//", i): + j = src.find("\n", i) + j = n if j < 0 else j + kind[i:j] = bytes([COMMENT]) * (j - i) + i = j + continue + if src.startswith("/*", i): + depth, j = 0, i + while j < n: + if src.startswith("/*", j): + depth += 1 + j += 2 + elif src.startswith("*/", j): + depth -= 1 + j += 2 + if depth == 0: + break + else: + j += 1 + kind[i:j] = bytes([COMMENT]) * (j - i) + i = j + continue + if c in "rb" and not ident_before(i): + m = RAW_STRING.match(src, i) + if m: + close = '"' + m.group(1) + j = src.find(close, m.end()) + j = n if j < 0 else j + len(close) + kind[i:j] = bytes([STRING]) * (j - i) + i = j + continue + if c == '"' or (src.startswith('b"', i) and not ident_before(i)): + j = i + (2 if c == "b" else 1) + while j < n and src[j] != '"': + j += 2 if src[j] == "\\" else 1 + j = min(j + 1, n) + kind[i:j] = bytes([STRING]) * (j - i) + i = j + continue + if c == "'": + m = CHAR_LITERAL.match(src, i) + if m: + kind[i : m.end()] = bytes([STRING]) * (m.end() - i) + i = m.end() + continue + i += 1 + return kind + + +@dataclass +class FileCount: + code: int + test: int + test_mods: list[tuple[str, str | None]] = field(default_factory=list) + + +def count_source(src: str) -> FileCount: + """Count code lines in one Rust file, splitting off ``cfg(test)`` items.""" + kind = classify(src) + n = len(src) + excluded = bytearray(n) + test_mods: list[tuple[str, str | None]] = [] + for m in CFG_TEST.finditer(src): + start = m.start() + if kind[start] != CODE or excluded[start]: + continue + # The gated item ends at its first top-level `;` or at the brace + # closing its first `{`. + j, depth, end = m.end(), 0, n + while j < n: + if kind[j] == CODE: + ch = src[j] + if ch in "{([": + depth += 1 + elif ch in "})]": + depth -= 1 + if depth == 0 and ch == "}": + end = j + 1 + break + elif ch == ";" and depth == 0: + end = j + 1 + break + j += 1 + item = src[m.end() : end] + decl = MOD_DECL.search(item) + if decl and "{" not in item: + test_mods.append((decl.group(2), decl.group(1))) + excluded[start:end] = b"\x01" * (end - start) + + code_lines: set[int] = set() + test_lines: set[int] = set() + line = 1 + for k, ch in enumerate(src): + if ch == "\n": + line += 1 + elif not ch.isspace() and kind[k] != COMMENT: + (test_lines if excluded[k] else code_lines).add(line) + return FileCount(len(code_lines), len(test_lines - code_lines), test_mods) + + +def resolve_module(declaring: str, name: str, path_attr: str | None) -> str | None: + """The file a ``mod name;`` in ``declaring`` loads, if it exists.""" + d, base = os.path.split(declaring) + owns_dir = base in ("mod.rs", "lib.rs", "main.rs") + sub = d if owns_dir else os.path.join(d, base[:-3]) + if path_attr: + candidates = [os.path.join(sub, path_attr), os.path.join(d, path_attr)] + else: + candidates = [os.path.join(sub, name + ".rs"), os.path.join(sub, name, "mod.rs")] + for c in candidates: + if (REPO / c).is_file(): + return os.path.normpath(c) + return None + + +def rust_files() -> list[str]: + out = subprocess.check_output(["git", "ls-files", "*.rs"], cwd=REPO, text=True) + return [ + f + for f in out.split() + if not re.search(r"(^|/)(tests|benches)/", f) + ] + + +def count_repo() -> dict[str, int]: + """Implementation lines per tracked Rust file (test-only files count 0).""" + files = rust_files() + counts = {f: count_source((REPO / f).read_text(encoding="utf-8", errors="replace")) for f in files} + test_files: set[str] = set() + for f, fc in counts.items(): + for name, path_attr in fc.test_mods: + target = resolve_module(f, name, path_attr) + if target: + test_files.add(target) + # A module below a test-only module is test-only too. + roots = {t[:-3] if not t.endswith("/mod.rs") else os.path.dirname(t) for t in test_files} + for f in files: + if any(f.startswith(r + "/") for r in roots): + test_files.add(f) + return {f: (0 if f in test_files else fc.code) for f, fc in counts.items()} + + +def in_group(path: str, group: dict) -> bool: + return any(path.startswith(p) for p in group["paths"]) and not any( + path.startswith(p) for p in group.get("exclude", []) + ) + + +def group_total(per_file: dict[str, int], group: dict) -> int: + return sum(c for f, c in per_file.items() if in_group(f, group)) + + +def load_budget() -> dict: + return json.loads(BUDGET_FILE.read_text()) + + +def main(argv: list[str] | None = None) -> int: + ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + mode = ap.add_mutually_exclusive_group() + mode.add_argument("--check", action="store_true", help="exit 1 when a group is over budget") + mode.add_argument("--warn", action="store_true", help="annotate groups over budget, exit 0") + mode.add_argument("--lower", action="store_true", help="lower each max to its current count") + mode.add_argument("--files", metavar="GROUP", help="per-file counts for one group") + args = ap.parse_args(argv) + + budget = load_budget() + groups = budget["groups"] + per_file = count_repo() + + if args.files: + group = groups[args.files] + rows = sorted(((c, f) for f, c in per_file.items() if c and in_group(f, group)), reverse=True) + for c, f in rows: + print(f"{c:7d} {f}") + print(f"{sum(c for c, _ in rows):7d} total") + return 0 + + over = [] + for name, g in groups.items(): + total = group_total(per_file, g) + cap = g["max"] + status = "over" if total > cap else "ok" + print(f"{name:24s} {total:8d} / {cap:8d} {status}") + if total > cap: + over.append((name, total, cap)) + if args.lower and total < cap: + g["max"] = total + print(f"{'all implementation':24s} {sum(per_file.values()):8d}") + + if args.lower: + BUDGET_FILE.write_text(json.dumps(budget, indent=2) + "\n") + if (args.check or args.warn) and over: + for name, total, cap in over: + print( + f"::warning title=loc budget::{name} has {total} implementation lines, " + f"over its budget of {cap} (scripts/loc-budget.json)" + ) + return 1 if args.check else 0 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/test_loc_budget.py b/scripts/tests/test_loc_budget.py new file mode 100644 index 000000000..f3156b1aa --- /dev/null +++ b/scripts/tests/test_loc_budget.py @@ -0,0 +1,140 @@ +"""Tests for scripts/loc-budget.py's Rust line counter.""" + +import importlib.util +import json +import sys +import unittest +from pathlib import Path + +SCRIPT = Path(__file__).resolve().parent.parent / "loc-budget.py" +spec = importlib.util.spec_from_file_location("loc_budget", SCRIPT) +loc_budget = importlib.util.module_from_spec(spec) +# dataclasses resolve annotations through sys.modules. +sys.modules["loc_budget"] = loc_budget +spec.loader.exec_module(loc_budget) + + +def count(src: str): + return loc_budget.count_source(src) + + +class CountSourceTest(unittest.TestCase): + def test_blank_and_comment_lines_do_not_count(self): + src = "//! module doc\n\n/// item doc\nfn a() {}\n/* block\n comment */\n" + self.assertEqual(count(src).code, 1) + + def test_nested_block_comment(self): + src = "/* outer /* inner */ still comment */\nfn a() {}\n" + self.assertEqual(count(src).code, 1) + + def test_cfg_test_module_is_excluded(self): + src = ( + "fn a() {}\n" + "#[cfg(test)]\n" + "mod tests {\n" + " #[test]\n" + " fn t() { assert!(true); }\n" + "}\n" + "fn b() {}\n" + ) + fc = count(src) + self.assertEqual(fc.code, 2) + self.assertEqual(fc.test, 5) + + def test_cfg_all_test_is_excluded_but_cfg_any_is_not(self): + src = ( + "#[cfg(all(test, unix))]\nfn only_test() {}\n" + "#[cfg(any(test, feature = \"x\"))]\nfn maybe_shipped() {}\n" + ) + self.assertEqual(count(src).code, 2) + + def test_braces_inside_strings_and_chars_do_not_end_the_item(self): + src = ( + "#[cfg(test)]\n" + "mod tests {\n" + ' const S: &str = "}";\n' + " const C: char = '}';\n" + ' const R: &str = r#"}"#;\n' + " // }\n" + "}\n" + "fn shipped() {}\n" + ) + self.assertEqual(count(src).code, 1) + + def test_lifetimes_are_not_char_literals(self): + src = ( + "#[cfg(test)]\n" + "fn helper<'a>(s: &'a str) -> &'a str { s }\n" + "fn shipped<'b>(s: &'b str) -> &'b str { s }\n" + ) + self.assertEqual(count(src).code, 1) + + def test_cfg_test_inside_a_string_is_ignored(self): + src = 'const DOC: &str = "#[cfg(test)]";\nfn a() {}\n' + self.assertEqual(count(src).code, 2) + + def test_test_module_declarations_are_reported(self): + src = ( + "#[cfg(test)]\nmod tests;\n" + '#[cfg(test)]\n#[path = "x_tests.rs"]\nmod x_tests;\n' + "mod shipped;\n" + ) + fc = count(src) + self.assertEqual(fc.test_mods, [("tests", None), ("x_tests", "x_tests.rs")]) + self.assertEqual(fc.code, 1) + + +class ModeTest(unittest.TestCase): + """--check fails over budget, --warn only annotates.""" + + def setUp(self): + self.saved = (loc_budget.load_budget, loc_budget.count_repo) + loc_budget.load_budget = lambda: { + "groups": { + "g": {"paths": ["a/"], "exclude": ["a/skip/"], "max": 10}, + } + } + loc_budget.count_repo = lambda: {"a/x.rs": 8, "a/y.rs": 5, "a/skip/z.rs": 100, "b/w.rs": 1} + + def tearDown(self): + loc_budget.load_budget, loc_budget.count_repo = self.saved + + def run_main(self, *argv): + import contextlib + import io + + out = io.StringIO() + with contextlib.redirect_stdout(out): + code = loc_budget.main(list(argv)) + return code, out.getvalue() + + def test_exclude_and_over_budget(self): + code, out = self.run_main("--check") + self.assertEqual(code, 1) + self.assertIn("g 13 / 10 over", out) + self.assertIn("::warning title=loc budget::g has 13", out) + + def test_warn_annotates_but_passes(self): + code, out = self.run_main("--warn") + self.assertEqual(code, 0) + self.assertIn("::warning title=loc budget::g has 13", out) + + def test_report_mode_never_fails(self): + code, out = self.run_main() + self.assertEqual(code, 0) + self.assertNotIn("::warning", out) + + +class BudgetFileTest(unittest.TestCase): + def test_budget_file_is_well_formed(self): + budget = json.loads(loc_budget.BUDGET_FILE.read_text()) + self.assertTrue(budget["groups"]) + for name, group in budget["groups"].items(): + self.assertIsInstance(group["max"], int, name) + self.assertTrue(group["paths"], name) + for p in group["paths"]: + self.assertTrue((loc_budget.REPO / p).exists(), f"{name}: {p} does not exist") + + +if __name__ == "__main__": + unittest.main() From a08a113053e6e93841c203bd48ecda9796e7080d Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:13:50 -0600 Subject: [PATCH 2/9] Keep the origin classifier out of formats/ formats/ models are guarded to stay free of I/O and of the host policy (formats/mod.rs architecture_tests bans hosted_patch_uuid there), so the classifier is a top-level `origin` module and a project layer applies it. Spell out the three layers and where each part of the classifier moves from. Co-Authored-By: Claude Opus 5.5 --- docs/design/lock-model-unification.md | 38 ++++++++++++++++++++------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/docs/design/lock-model-unification.md b/docs/design/lock-model-unification.md index 9da9c15cb..19084b71c 100644 --- a/docs/design/lock-model-unification.md +++ b/docs/design/lock-model-unification.md @@ -51,6 +51,19 @@ backends. ## The model +### Layers + +1. **`formats/`: pure models.** Text in, records out. Each record carries + the raw location strings the lock spells (resolved URL, tarball, path, + source). The existing architecture test keeps these models free of I/O + and of the host policy, and that rule stays. +2. **`origin`: the one classifier.** A top-level module, not part of + `formats/`, because it applies the host policy. See [Origin](#origin). +3. **The project layer.** Reads files through `ProjectView`, picks install + trees, joins files where a format needs it, and turns each record's raw + location into an `Origin`. Discovery, the inventory and the writers all + consume it. + ### Entries Every reader yields `LockEntry` records. Each record holds the package @@ -79,14 +92,21 @@ flags: bundled, link, the v2 mirror, a `hasShrinkwrap` ancestor, Yarn's `live`, Berry's checksum cache key, and so on. These are fields of the format's record, not new side channels. -One classifier, `formats::origin`, decides `Origin`. It is discovery's -validators moved, not rewritten: +### Origin + +One classifier, the `origin` module, decides `Origin`. It is moved, not +rewritten, from two places: + +* from `patch::redirect`: `hosted_patch_uuid` (Socket's patch server or a + configured origin, the last canonical-uuid path segment) and + `SOCKET_PATCH_SERVER_HOST`. The hosted unwind already calls these + directly; +* from discovery: `OriginPolicy::hosted_uuid` (the same rule with a memoized + fast path for plain registry URLs), `vendor_ref`, `vendor_ref_decorated` + and `vendor_uuid_dir`, `locate`, `Wired`, and the `socket-patch-` + name grammar. -* the Socket host allowlist and the last-canonical-uuid rule - (`hosted_uuid`); -* `vendor_ref`, `vendor_ref_decorated` and `vendor_uuid_dir`; -* the `socket-patch-` name grammar; -* the validating purl builders. +The validating purl builders already live in `utils::purl`. The inventory's any-host matching goes away. That is a behavior change only where a non-allowlisted host serves a URL that looks like Socket's, and it @@ -217,13 +237,13 @@ by line. | PR | Branch | Change | |---|---|---| | 0 | `lock-model/0-design` | This document and the size budget | -| 1 | `lock-model/1-origin` | `LockEntry`, `Origin` and `formats::origin` (discovery's classifier, moved). Discovery calls it. The inventory doesn't yet. | +| 1 | `lock-model/1-origin` | The `origin` module: discovery's classifier and the redirect's hosted-URL rule, moved verbatim. Discovery and the redirect re-export it. | | 2 | `lock-model/2-pnpm` | Pilot. One pnpm reader feeds the inventory, discovery and the hosted planner. Removes `InstalledPackages`, `vendored_npm_uuids`, the whole-text planner walk and the second Rush listing. | | 3 | `lock-model/3-npm` | package-lock and shrinkwrap. Folds `vendor/npm_origin.rs`'s second walk into the records. | | 4 | `lock-model/4-yarn` | Classic and Berry. One block model for every reader and writer, `BerryStanzas` included. | | 5 | `lock-model/5-bun-vlt` | `bun.lock`, `bun.lockb` and vlt. The vlt writers move off their line grammar. | | 6 | `lock-model/6-deno` | The `deno.lock` reader in `formats/`, for all four of its callers | -| 7 | `lock-model/7-python` | One lister, one parse per lock, one include walker and one PEP 508 parser. `pypi_locks.rs` stops parsing TOML itself. Poetry wiring uses `formats::origin`. Hatch gets inventory entries. | +| 7 | `lock-model/7-python` | One lister, one parse per lock, one include walker and one PEP 508 parser. `pypi_locks.rs` stops parsing TOML itself. Poetry wiring uses `origin`. Hatch gets inventory entries. | | 8 | `lock-model/8-cargo-go` | One `Cargo.toml` dependency-table walk. Go `replace` and `go.sum` records. | | 9 | `lock-model/9-gem-composer` | Gemfile.lock and composer.lock records. The inventory stops dropping BOM-prefixed `composer.lock` files. | | 10 | `lock-model/10-nuget` | NuGet records with the config join, `NugetContentHash`, and inventory entries | From 0006f141d5a847ca488827ae136f99ed155f4609 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:22:16 -0600 Subject: [PATCH 3/9] Make one set of behaviors the goal, hosted semantics first Owner decisions: inconsistencies between discovery, the inventory and the two modes are bugs to remove, not behavior to preserve; hosted and vendored share one core and differ only in where a rewired entry points; when in doubt the hosted mode's behavior wins, and every such call is listed in the doc so it can be overruled. Behavior changes are allowed per PR, listed, pinned by tests and shown as golden diffs. First decision recorded: pnpm 11's env-lockfile document counts as its own install tree, as the hosted planner already treats it. Co-Authored-By: Claude Opus 5.5 --- docs/design/lock-model-unification.md | 53 +++++++++++++++++++++------ 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/docs/design/lock-model-unification.md b/docs/design/lock-model-unification.md index 19084b71c..742d272fe 100644 --- a/docs/design/lock-model-unification.md +++ b/docs/design/lock-model-unification.md @@ -42,12 +42,36 @@ discovery reads. ## Goal -One per-project lock model. Every consumer reads it, and the end state has -fewer implementation lines than today. `scripts/loc-budget.py` tracks the -lines (see [Size budget](#size-budget)). - -Non-goals: changing what any mode writes, or adding new hosted or vendored -backends. +One per-project lock model and one set of behaviors. Every consumer reads +the model, and the end state has fewer implementation lines than today. +`scripts/loc-budget.py` tracks the lines (see [Size budget](#size-budget)). + +## Principles (owner decisions, 2026-11-04) + +1. **One set of behaviors.** Discovery (VEX), the inventory, hosted mode and + vendored mode answer each question the same way: which files count, + which entries are installed, what is Socket wiring, what a malformed + entry means. An inconsistency between them is a bug to remove, not + behavior to preserve. +2. **Hosted and vendored share the core.** The two modes differ only in where + a rewired entry points (a Socket URL or a committed `.socket/vendor/` + path). Everything else, from reading the lock to choosing entries and + refusing unsafe states, is one implementation. +3. **When in doubt, hosted semantics are correct.** Where the modes or + readers disagree and the right answer isn't clear, the hosted mode's + current behavior wins. Every such call is listed under + [Decisions](#decisions-under-the-hosted-rule), so it can be overruled. +4. **Behavior changes are allowed, and visible.** A PR that aligns behavior + lists each change in its description, adds a test that pins the new + behavior, and shows its golden diffs. + +Non-goal: adding new hosted or vendored backends. + +## Decisions under the hosted rule + +| Question | Readers before | Decision | +|---|---|---| +| Is pnpm 11's env-lockfile document (the first YAML document, holding `configDependencies` / `packageManagerDependencies`) part of the project? | The hosted planner and the v9 vendored in-use check read it. `PnpmLock` (inventory and discovery), `InstalledPackages` and the legacy vendored check skip it (#1949). | It counts. pnpm installs config dependencies (into its config store, not `node_modules`), and the hosted planner patches them. The document becomes its own install tree. | ## The model @@ -229,16 +253,21 @@ add code and raise the readers' maximum by hand, saying why. ## Stack -Each PR is based on the one before. PRs 1 to 11 must leave -`tests/fixtures/vex-discover-golden/` and the redirect goldens byte for byte -unchanged. Only PR 12 may change them, and its golden diff is reviewed line -by line. +Each PR is based on the one before. A PR that changes behavior follows +principle 4: it lists every change, pins it with a test and shows its golden +diffs (`tests/fixtures/vex-discover-golden/`, `tests/fixtures/redirect/`). + +Each format PR does two things for its format: one reader model that +discovery, the inventory and both modes consume, with every divergence +resolved under the principles; and, where the hosted and vendored planners +duplicate each other, one planner with the target as its only parameter. +A format whose planner merge is large splits into two PRs, readers first. | PR | Branch | Change | |---|---|---| | 0 | `lock-model/0-design` | This document and the size budget | | 1 | `lock-model/1-origin` | The `origin` module: discovery's classifier and the redirect's hosted-URL rule, moved verbatim. Discovery and the redirect re-export it. | -| 2 | `lock-model/2-pnpm` | Pilot. One pnpm reader feeds the inventory, discovery and the hosted planner. Removes `InstalledPackages`, `vendored_npm_uuids`, the whole-text planner walk and the second Rush listing. | +| 2 | `lock-model/2-pnpm` | Pilot. One pnpm entry walk for the inventory, discovery, the store crawler, the hosted planner and the vendored in-use check. The env-lockfile document becomes its own tree (see Decisions). | | 3 | `lock-model/3-npm` | package-lock and shrinkwrap. Folds `vendor/npm_origin.rs`'s second walk into the records. | | 4 | `lock-model/4-yarn` | Classic and Berry. One block model for every reader and writer, `BerryStanzas` included. | | 5 | `lock-model/5-bun-vlt` | `bun.lock`, `bun.lockb` and vlt. The vlt writers move off their line grammar. | @@ -248,7 +277,7 @@ by line. | 9 | `lock-model/9-gem-composer` | Gemfile.lock and composer.lock records. The inventory stops dropping BOM-prefixed `composer.lock` files. | | 10 | `lock-model/10-nuget` | NuGet records with the config join, `NugetContentHash`, and inventory entries | | 11 | `lock-model/11-jvm` | One `checksums.sha256` reader, one Gradle index parser, a `verification-metadata.xml` reader, then Gradle-lock and root-pom inventory entries and sbt's owned-file entries | -| 12 | `lock-model/12-install-trees` | Install-tree selection, Python trees, Deno governance, the single classifier for the inventory, liveness on `Origin`. Deletes the npm flavor router's lock choice, the Python precedence rules, `resolved_elsewhere` and the `UnwiredCopy` plumbing. | +| 12 | `lock-model/12-install-trees` | Install-tree selection across formats, Python trees, Deno governance, the single classifier for the inventory, liveness on `Origin`. Deletes the npm flavor router's lock choice, the Python precedence rules, `resolved_elsewhere` and the `UnwiredCopy` plumbing. | | 13 | `lock-model/13-cleanup` | Removes the raw-text liveness fallbacks if the model makes them redundant, plus whatever is left of `vendor/lock_inventory/`. Final budget. | ## Open questions From 950f9fe8128806679a1fcb600d2475dc385c1411 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 10:34:01 -0600 Subject: [PATCH 4/9] Catalog where readers and modes disagree docs/design/lock-model-divergences.md lists every question on which discovery, the inventory, hosted and vendored mode answer differently today, across all ecosystems, with the resolution under the principles and flags for false attestations and target-specific differences. Owner decisions recorded: the modes are ground truth over the readers; vendored mode's Maven profile and reactor support and its scala-cli wiring are back-ported to hosted as the planners merge (the JVM PR splits into readers, Maven planner, and Gradle/sbt/scala-cli). One call is pending: whether hosted keeps rewriting a shrinkwrap.yaml that pnpm never reads. Corrects the install-tree section: npm-shrinkwrap.json does not shadow package-lock.json; every consumer but the inventory already reads both. Co-Authored-By: Claude Opus 5.5 --- docs/design/lock-model-divergences.md | 136 ++++++++++++++++++++++++++ docs/design/lock-model-unification.md | 29 ++++-- 2 files changed, 157 insertions(+), 8 deletions(-) create mode 100644 docs/design/lock-model-divergences.md diff --git a/docs/design/lock-model-divergences.md b/docs/design/lock-model-divergences.md new file mode 100644 index 000000000..1fb560ff6 --- /dev/null +++ b/docs/design/lock-model-divergences.md @@ -0,0 +1,136 @@ +# Where the readers and modes disagree + +Companion to [lock-model-unification.md](lock-model-unification.md). Each +row is a question that discovery (VEX), the lock inventory, hosted mode and +vendored mode answer differently today, with its resolution under that +document's [principles](lock-model-unification.md#principles-owner-decisions-2026-11-04). +Paths are relative to `crates/socket-patch-core/src/`. Line numbers are from +the survey of 2026-11-04 and will drift. + +Flags: + +* **false attestation**: a reader attests a copy the install may not use + today. Fixing these is a correctness priority. +* **needs owner**: following hosted would be unsafe or would delete a + working feature. Listed under "Calls that go against hosted" for the + owner to decide. +* **target-specific**: the difference comes from where the entry points + (a Socket URL or a committed path) and stays. + +## Calls that go against hosted (owner decided 2026-11-04) + +Owner: the false Maven attestations get fixed, the modes are ground truth +over the readers, and the vendored mode's profile, reactor and scala-cli +support is back-ported so hosted gains it as the planners merge. P3 and P4 +follow from the install-tree decision. + +| # | Question | Hosted today | Decision | +|---|---|---|---| +| J1 | Does a base-version Maven dependency declared in a `` block block attestation? | Warns, still pins the top level and counts the package as redirected (`patch/redirect/mod.rs` ~8744) | Vendored's rule: any profile declaration at the base version makes the ref unattested (fixed now in discovery), and the merged planner rewrites profile declarations in both modes. | +| J3 | Which Maven poms count? | Refuses a reactor root (~8572) | The reactor reader becomes the shared core, so hosted gains reactor support. | +| J12 | scala-cli | Guidance only (`patch/redirect/scala_guidance.rs`) | Back-port: hosted wires scala-cli through the merged planner (a same-GAV Socket repository instead of a suffixed version). | +| P3 | Is a shadowed `pdm.lock` rewritten? | Only when it governs (`pdm_drives`, ~564) | Rewrite it like every other tree, per the install-tree decision. | +| P4 | Are `-r` includes of `requirements.txt` rewritten? | Root file only (`patch/redirect/requirements.rs` ~208) | Follow includes, per the install-tree decision. | +| N11 | Is a `shrinkwrap.yaml` beside `pnpm-lock.yaml` rewritten? | Yes; its all-or-nothing gate covers both files | **Pending.** Proposed: treat it as shadowed (swept for stale identities, never written). pnpm never reads it, and an unparseable debris key in it can refuse the dependency in every lock. | + +## JVM + +| # | Question | Today | Resolution | Flag | +|---|---|---|---|---| +| J2 | Does an executable classifier variant (`tests`, native) of the patched package block attestation? | Hosted skips the dependency. Discovery marks it unattested. Vendored only warns, then `entry_wired_checked` attests (`vendor/jvm/maven_reactor.rs` ~1884, `vendor/jvm/apply.rs` ~1042). | Hosted | false attestation | +| J4 | How are `${prop}` versions resolved, and does `.mvn/maven.config` win? | Discovery: one level of pom properties, no `-D` user properties. Hosted refuses `${…}`. Vendored: user properties, parent chain, profiles, depth 16. | One interpolator (vendored's). Hosted's refusal stays a writer policy. | false attestation | +| J5 | Do Gradle lockfiles gate liveness? | Hosted and discovery check every lock. Vendored reads none (`vendor/jvm/gradle.rs` ~1227). | Hosted | false attestation | +| J6 | Is sbt resolution evidence required before attesting? | Hosted: no integrity without verified evidence. Vendored: `wired_checked` only checks the owned-file row; discovery's `vendored_check` diagnostics are never consumed. | Hosted. One evidence route (`ResolutionDoc` + `gate::check_new`). | false attestation | +| J7 | Another version of a mapped NuGet id is locked | Both writers refuse. Discovery emits a ref per locked version, keyed by raw text, not `normalize_nuget_version`. | Hosted: a diagnostic, no ref | false attestation | +| J8 | What counts as "wired" for vendored Maven liveness and GC? | Vendored: substring match on masked text, so a suffix in ``, a profile or `` counts. Discovery and hosted use the scoped `PomScope`. | Scoped reader | | +| J9 | `checksums.sha256` and the `TRUSTED_CHECKSUMS_ON` switch | Four readers with different splitting and switch tests. Hosted's merge silently drops lines it can't split on two spaces. | One reader with the hosted grammar. The merge keeps lines it can't parse. | | +| J10 | Gradle Socket index rows | Two parsers. Hosted validates the uuid, URL and hashes; vendored accepts any token and uses a raw `split('\t')`. Vendored checks only root settings; hosted checks every build. | One row grammar, hosted's validation | | +| J11 | `verification-metadata.xml` | Two editors, no reader | One editor; a reader for the inventory | | +| J13 | NuGet config scope | Writers read inherited parent configs; discovery doesn't. Discovery refuses a disabled Socket source; the writers don't check ``. | Shared inheritance read. The writers adopt the disabled-source refusal. | | +| J14 | NuGet lock enumeration | In-memory views fall back to the root lock. The unwind bypasses the snapshot view (`governed_locks_on_disk`). | One enumeration over `ProjectView` | | +| J15 | JVM liveness on a read error | `entry_wired` is `unwrap_or(true)` for `scan`/`vendor`; `vex` uses the checked variant | Fail closed everywhere | false attestation | + +## Python + +| # | Question | Today | Resolution | Flag | +|---|---|---|---|---| +| P1 | Which files count? | Hosted rewrites every file except a non-governing `pdm.lock`. Vendored picks one winner and warns that the rest stay unpatched. The inventory uses a chain. Discovery reads every lock with the cross-lock contest. `formats/governing_locks.rs` still gates four callers. | One tree per tool lock (owner decision). Vendored wires every tree. `governing_locks` goes. | | +| P5 | Does the inventory list Socket entries? | It keeps hosted entries (with the patched artifact's URL and hash) but drops vendored uv path sources | Keep both, read as the package they replace. Check that vendor's pristine fetch and `repair` never take patched bytes from a hosted entry. | | +| P6 | What counts as Socket wiring? | Discovery and the unwind use the host allowlist. The inventory accepts any host. Ledger liveness uses `resolved.contains(uuid)`. `poetry_lock::lock_wiring` uses `contains("/patch/pypi/")`. | The `origin` allowlist everywhere | false attestation (liveness substring) | +| P7 | uv workspace roots | Hosted refuses members only. Vendored refuses roots too. | Vendored wires roots, after a test that a member's own `[tool.uv.sources]` can't override the root | | +| P8 | `uv.lock` without `pyproject.toml` | Hosted edits the lock; discovery attests it. Vendored refuses. | Vendored proceeds | | +| P9 | uv 0.2 `[[distribution]]` locks | Hosted rewrites. Vendored refuses (no portable path). The unwind refuses to restore them. | Vendored's refusal is target-specific and stays. The unwind must restore what hosted writes. | target-specific | +| P13 | PDM `lock_version` | Writers use an allowlist; discovery a denylist; the inventory checks nothing | The allowlist everywhere | | +| P14 | `pipfile-spec` | Writers require 6; discovery and the inventory accept any | 6 everywhere | | + +## Cargo, Go, Ruby, Composer + +| # | Question | Today | Resolution | Flag | +|---|---|---|---|---| +| G1 | Which `go.work` and members count? | Discovery: a root `go.work` and in-root members only, ignoring an ancestor `go.work`, `GOWORK` and `-modfile`. Hosted and vendored resolve the workspace go would use (`vendor/go_workspace.rs`). | `go_workspace::load` everywhere | false attestation | +| G16 | Go inventory | Root `go.sum` only, lenient parse, lists `patch.socket.dev/gopatch/` lines as packages | Discovery's model and workspace | | +| C2 | Which manifests decide whether a Cargo lock entry is used? | Hosted pins root and member manifests; vendored reads members. Discovery reads only the root `Cargo.toml`. | Members everywhere | false attestation | +| C11 | `cargo vendor` / local-registry source replacement | Hosted refuses; vendored and discovery ignore it | Stays | target-specific | +| R10 | `gems.rb` / `gems.locked` | Hosted wires the pair bundler loads. Vendored refuses any `gems.rb`. | Vendored supports it | | +| R15 | Malformed Gemfile.lock | Discovery reports it unparseable; the inventory lists what parsed | Discovery's rule everywhere | | +| X12 | `composer.lock` with a BOM | Discovery strips it. The inventory drops the file. Vendored refuses to plan, errors on revert, and reports nothing stranded. | Strip one BOM everywhere. Check that GC never trusts the empty "stranded" answer. | | + +## npm family + +| # | Question | Today | Resolution | Flag | +|---|---|---|---|---| +| N1 | pnpm `gitBranchLockfile` with a branch lock present: is the root lock the install tree? | Hosted withholds the root locks and refuses (`hosted/engine.rs` ~930). Vendored refuses. Discovery and the inventory never check the setting. | Hosted | false attestation | +| N2 | npm user patches (#711) and `bun patch` (#367) | Hosted skips that one dependency and records it as refused. Vendored refuses the whole npm v4 lock, or the one bun dependency. Discovery never reads `patchedDependencies` / `"patched"` and attests a pre-#711 hosted pin. | Hosted's per-dependency skip, in discovery too | false attestation | +| N3 | A workspace member's stray npm/bun/vlt lock | Discovery treats it as absent. Hosted and vendored refuse. The inventory reads it as the project's lock. | The inventory ignores it | | +| N4 | pnpm 11's env-lockfile document | See [Decisions](lock-model-unification.md#decisions-under-the-hosted-rule). `main_document` takes the text after the last `---`, so a crafted multi-document lock chooses its own "project". | Its own tree; documents identified by content, not position | | +| N5 | What counts as Socket wiring | Discovery and the unwind use the allowlist. The inventory records hosted URLs on any host. `parse_vendor_path` accepts `.socket/vendor/` anywhere, so a `../.socket/vendor/…` entry from a sibling checkout is "ours" to the vendored writer and reverter. The hosted pnpm "vendored?" check is a substring heuristic. | The `origin` classifier everywhere | revert could rewrite another project's wiring | +| N6 | `npm-shrinkwrap.json` and `package-lock.json` | Hosted, vendored and discovery read both, with a contest. The inventory reads the shrinkwrap only. | Both, everywhere | | +| N7 | npm lockfileVersion | Hosted rewrites v1 and v4. Vendored accepts v2/v3 only. The inventory ignores v1. Discovery reads v1 and v2+. | Hosted, once vendored `file:` wiring under npm 6 and npm 12.1 v4 has fixtures | | +| N8 | npm `file:` directory link identity (#1481) | Hosted and discovery give a versionless link its source directory's `name@version`. Vendored matches the entry's own version, wiring the registry copies with no warning. | Hosted | silent vendored success | +| N9 | yarn classic blocks reached only through an alias | Hosted skips them. Vendored rewrites them. Discovery counts an alias block as an install of the real package. | Back-port: hosted rewrites alias blocks (vendored supports what hosted lacks) | | +| N10 | Locks of several npm-family package managers in one project | Hosted rewrites every family; discovery reads every family with the contest. The inventory and vendored pick one family (vlt > bun > pnpm > yarn > npm). | Union (separate trees) | | +| N11 | `shrinkwrap.yaml` beside `pnpm-lock.yaml` | Hosted reads and splices it, and its all-or-nothing gate covers both. Discovery sweeps it as ignored. The inventory reads it only as a fallback. Vendored never reads it. | Pending owner decision (see above) | needs owner | +| N12 | Rush common and subspace locks | Hosted and discovery always read them. The inventory reads them only when there is no root lock (#2003 aligns the listing itself). Vendored refuses Rush. | Always read. Vendored's refusal stays. | target-specific (vendored) | +| N13 | Plug'n'Play | Vendored refuses. The hosted berry preflight has no PnP gate. The inventory returns an error, so `scan` sees nothing in a project hosted pinned. | The inventory reads PnP locks. Vendored keeps its refusal. | target-specific (vendored) | +| N14 | pnpm `lockfile=false` | Hosted refuses. Discovery withholds hosted refs, keeps vendored ones. Vendored proceeds. The inventory ignores the setting. | The modes stay as they are; the inventory follows discovery | target-specific | +| N15 | CRLF `pnpm-lock.yaml` / `pnpm-workspace.yaml` | Vendored refuses; the rest handle it | Handle it everywhere | | +| N16 | Lock versions | pnpm: vendored accepts only 5.4/6.0/9.0; hosted any but early shrinkwrap. vlt: vendored refuses a lock with no version; hosted rewrites and warns. | Hosted | | +| N17 | Bun workspaces, yarn classic offline mirror | Refused differently by the two modes | Stays | target-specific | +| N18 | An unparseable npm lock | Hosted warns and skips. Vendored refuses the primary lock. Discovery diagnoses. The inventory returns nothing silently. | Discovery's diagnostic, surfaced by every consumer | | +| N19 | yarn.lock with a stray top-level line | Discovery rejects the file; hosted and the inventory scan its blocks | Discovery's rule | | +| N20 | Yarn's last-block-wins (`live`) | Discovery respects it; the inventory and both hosted yarn rewriters don't | Respect it everywhere | | +| N21 | Accepted npm-family integrity | The inventory records any string as SRI; discovery requires an SRI pin (`sha1-` included) | Discovery's check | | + +## Fixes for false attestations on main + +These are fixed directly against `main`, ahead of the stack, one PR each. +The stack rebases over them. + +| # | Fix | +|---|---| +| J1, J4 | Maven discovery: a profile declaration or a `.mvn/maven.config` user property that resolves the base version leaves the ref unattested | +| J2, J8, J15 | Vendored Maven liveness: executable classifiers block, "wired" is read from scoped declarations, read errors fail closed | +| J5 | Vendored Gradle liveness reads the lock files | +| J6 | Vendored sbt attestation requires resolution evidence | +| J7 | NuGet discovery: no ref for a version the writers refuse | +| N1, N2, G1, C2, P6 | Next batch: pnpm `gitBranchLockfile`, npm/bun user patches, Go workspaces, Cargo members, the liveness substring match | + +## Planner merges (hosted and vendored) + +Estimated implementation lines removable by giving each format one planner +whose only parameter is the target: + +| Format | Today | Estimate | +|---|---|---| +| uv.lock | Vendored's text surgery in `vendor/pypi_uv.rs` duplicates hosted's `PythonLockSession` | 1,000–1,400 | +| NuGet | `add_nuget_source` / `rewrite_nuget` against `vendor/nuget_feed.rs`'s config edit, which says it mirrors it | ~400 | +| Gradle | Build discovery, apply-line helpers, index parse and merge, owned script, verification edits | 500–700 | +| Maven | Hosted is a root-only subset of the reactor planner. Four pom parsers collapse to one (`maven_reactor::Doc` survives). | largest; split readers first | +| sbt | Build-source refusals and per-pin decisions; one evidence route | 150–250 | +| Python router | `detect_pypi_flavor` and hosted's fixed chain become one query over trees | ~300 | +| requirements, Pipenv, Poetry, PDM | Shared lexers or rewriters already; preflight and matchers duplicated | a few hundred | +| Cargo | Dependency-table walk, member enumeration, CRLF handling | a few hundred | +| Go | Version-selected and in-graph checks | ~100 | +| Gem | Pair selection and spec-block moves | a few hundred | +| Composer | Regex splice against JSON re-serialization; one locator and one splice | ~300 | +| Unwind | The hosted `upstream/*` modules mirror the vendored unplans and become one unplan per format | follows the planner merges | diff --git a/docs/design/lock-model-unification.md b/docs/design/lock-model-unification.md index 742d272fe..f70540e55 100644 --- a/docs/design/lock-model-unification.md +++ b/docs/design/lock-model-unification.md @@ -57,10 +57,15 @@ the model, and the end state has fewer implementation lines than today. a rewired entry points (a Socket URL or a committed `.socket/vendor/` path). Everything else, from reading the lock to choosing entries and refusing unsafe states, is one implementation. -3. **When in doubt, hosted semantics are correct.** Where the modes or - readers disagree and the right answer isn't clear, the hosted mode's - current behavior wins. Every such call is listed under - [Decisions](#decisions-under-the-hosted-rule), so it can be overruled. +3. **When in doubt, the modes are ground truth.** Where a reader (discovery, + the inventory) disagrees with what hosted or vendored mode writes, the + mode wins. Between the two modes, hosted's behavior wins, except where + vendored supports something hosted lacks: then the support is + back-ported to hosted as the planners merge (Maven profiles and reactors, + scala-cli). Every call is listed under + [Decisions](#decisions-under-the-hosted-rule) and in + [lock-model-divergences.md](lock-model-divergences.md), so it can be + overruled. 4. **Behavior changes are allowed, and visible.** A PR that aligns behavior lists each change in its description, adds a test that pins the new behavior, and shows its golden diffs. @@ -170,9 +175,15 @@ pub struct InstallTree { Rule: **precedence within a tree, union across trees.** -* npm: `npm-shrinkwrap.json` governs and `package-lock.json` is shadowed. - pnpm: `pnpm-lock.yaml` governs `shrinkwrap.yaml`. Bun: `bun.lock` governs - `bun.lockb`, which `bun_text_lock_drives` already decides. +* npm: `npm-shrinkwrap.json` and `package-lock.json` are both read, as + hosted, vendored and discovery already do, with discovery's contest + between the two. The inventory, which reads only the shrinkwrap today, + follows. pnpm: `pnpm-lock.yaml` governs `shrinkwrap.yaml`. Bun: `bun.lock` + governs `bun.lockb`, which `bun_text_lock_drives` already decides. +* npm-family locks of different package managers in one project (for + example `pnpm-lock.yaml` beside `yarn.lock`) are separate trees, as hosted + and discovery treat them. The inventory and vendored mode stop picking one + family. * **Python (owner decision):** each tool's lock is its own tree, because which lock is installed depends on which tool runs. uv.lock, poetry.lock, pdm.lock, Pipfile.lock, each pylock, each PEP 723 script lock, and @@ -276,7 +287,9 @@ A format whose planner merge is large splits into two PRs, readers first. | 8 | `lock-model/8-cargo-go` | One `Cargo.toml` dependency-table walk. Go `replace` and `go.sum` records. | | 9 | `lock-model/9-gem-composer` | Gemfile.lock and composer.lock records. The inventory stops dropping BOM-prefixed `composer.lock` files. | | 10 | `lock-model/10-nuget` | NuGet records with the config join, `NugetContentHash`, and inventory entries | -| 11 | `lock-model/11-jvm` | One `checksums.sha256` reader, one Gradle index parser, a `verification-metadata.xml` reader, then Gradle-lock and root-pom inventory entries and sbt's owned-file entries | +| 11a | `lock-model/11a-jvm-readers` | One pom model (`maven_reactor::Doc`), one interpolator, one `checksums.sha256` reader, one Gradle index grammar, a `verification-metadata.xml` reader, then Gradle-lock and pom inventory entries and sbt's owned-file entries | +| 11b | `lock-model/11b-maven-planner` | One Maven planner for both modes. Hosted gains reactor and profile support. | +| 11c | `lock-model/11c-gradle-sbt-scala` | One Gradle and one sbt planner, one evidence route. Hosted gains scala-cli wiring. | | 12 | `lock-model/12-install-trees` | Install-tree selection across formats, Python trees, Deno governance, the single classifier for the inventory, liveness on `Origin`. Deletes the npm flavor router's lock choice, the Python precedence rules, `resolved_elsewhere` and the `UnwiredCopy` plumbing. | | 13 | `lock-model/13-cleanup` | Removes the raw-text liveness fallbacks if the model makes them redundant, plus whatever is left of `vendor/lock_inventory/`. Final budget. | From a396590bc69beada3eb132c5398ea0d33fe6b8d7 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 4 Nov 2026 11:18:35 -0600 Subject: [PATCH 5/9] Record user-patch and repo-root decisions Owner decisions: a dependency the user patched themselves is never patched or attested by default, in every mode and package manager, unless the user explicitly overrides; and no reader or writer reads project files outside the git repository root. Co-Authored-By: Claude Opus 5.5 --- docs/design/lock-model-unification.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/design/lock-model-unification.md b/docs/design/lock-model-unification.md index f70540e55..847dd1492 100644 --- a/docs/design/lock-model-unification.md +++ b/docs/design/lock-model-unification.md @@ -77,6 +77,8 @@ Non-goal: adding new hosted or vendored backends. | Question | Readers before | Decision | |---|---|---| | Is pnpm 11's env-lockfile document (the first YAML document, holding `configDependencies` / `packageManagerDependencies`) part of the project? | The hosted planner and the v9 vendored in-use check read it. `PnpmLock` (inventory and discovery), `InstalledPackages` and the legacy vendored check skip it (#1949). | It counts. pnpm installs config dependencies (into its config store, not `node_modules`), and the hosted planner patches them. The document becomes its own install tree. | +| Does Socket patch a dependency the user patched themselves (npm `patchedDependencies` / `"patched"`, `bun patch`, pnpm `patchedDependencies`, yarn berry `patch:`, vlt patches)? | Hosted skips npm and bun user patches (#711, #367). Vendored refuses them. Discovery attested them until #2010. pnpm, yarn and vlt user patches are not detected anywhere. | **Owner decision:** never, by default, in every mode and every package manager. A Socket pin under a user patch is not attested. Only an explicit user override patches it. | +| May a reader or writer read project files outside the git repository root? | Several do: vendored paths spelled `../.socket/vendor/…`, an ancestor `go.work`, out-of-root `-r` includes, parent NuGet configs. | **Owner decision:** no. Every project-file read is bounded to the git repository root (the project root when there is no repository). Machine-level tool configuration (for example `GOENV`, the user `NuGet.Config`) is listed separately for review. | ## The model From 0d683937b3f725518af2bf17f263ac9cff06cf2f Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 23:41:25 -0600 Subject: [PATCH 6/9] Fix the loc-budget counter's cfg(test) and module bugs Review of #2001 found the counter miscounting in several shapes, each now pinned by a test in scripts/tests/test_loc_budget.py: - a cfg(test) struct field, enum variant, parameter or match arm drove the scan below the item's depth and swallowed the code after it; the item now ends at a top-level `,` or before the bracket closing its container - `#[cfg(test)] use a::{..};` left its trailing `;` line counted - cfg predicates are parsed: `all(unix, test)` and nested `all` count, `not(test)` doesn't, and a file opening with `#![cfg(test)]` is test-only - `#[path]` is found among any attributes around the item and resolves against the declaring file's directory, as rustc does - `mod x;` inside a test-only inline module (vex::discover's `testing { mod golden; }`) marks that file test-only - `git ls-files -z` keeps paths with spaces whole, and only the group files plus their module declarers are read - `--files` with an unknown group is a usage error, not a KeyError golden.rs (435 lines) and two `};` lines leave the counts, so the budget drops to 30462 / 88362. Co-Authored-By: Claude Opus 5.5 --- scripts/loc-budget.json | 4 +- scripts/loc-budget.py | 321 ++++++++++++++++++++++++++----- scripts/tests/test_loc_budget.py | 170 +++++++++++++++- 3 files changed, 442 insertions(+), 53 deletions(-) diff --git a/scripts/loc-budget.json b/scripts/loc-budget.json index 28d387299..62f8fe636 100644 --- a/scripts/loc-budget.json +++ b/scripts/loc-budget.json @@ -16,7 +16,7 @@ "crates/socket-patch-core/src/utils/pipenv.rs", "crates/socket-patch-core/src/crawlers/deno_crawler.rs" ], - "max": 30898 + "max": 30462 }, "lock-writers": { "paths": [ @@ -29,7 +29,7 @@ "crates/socket-patch-core/src/vendor/npm_origin.rs", "crates/socket-patch-core/src/vendor/test_support" ], - "max": 88363 + "max": 88362 } } } diff --git a/scripts/loc-budget.py b/scripts/loc-budget.py index 7ef6795a4..b4e024218 100755 --- a/scripts/loc-budget.py +++ b/scripts/loc-budget.py @@ -4,9 +4,13 @@ A line counts when it holds code outside a comment. These never count: * blank and comment-only lines (doc comments included); -* items gated by ``#[cfg(test)]`` or ``#[cfg(all(test, ...))]`` (inline - ``mod tests { ... }`` blocks, test-only helpers and ``use`` lines); -* files declared as ``#[cfg(test)] mod name;`` and every module below them; +* items gated by a ``cfg`` that implies ``test`` -- ``#[cfg(test)]``, + ``#[cfg(all(unix, test))]`` -- with their other attributes: inline + ``mod tests { ... }`` blocks, test-only helpers, ``use`` lines, struct + fields, enum variants, parameters and match arms; +* files declared as ``#[cfg(test)] mod name;`` (also with ``#[path]``, or + inside a test-only inline module), files that open with + ``#![cfg(test)]``, and every module below them; * files under a ``tests/`` or ``benches/`` directory. Budgets live in ``scripts/loc-budget.json``. Each group names path prefixes @@ -45,12 +49,17 @@ REPO = Path(__file__).resolve().parent.parent BUDGET_FILE = REPO / "scripts" / "loc-budget.json" -CFG_TEST = re.compile(r"#\[cfg\(\s*(?:test|all\(\s*test\b[^\]]*)\)\]") +# `#[cfg(` or `#![cfg(`; implies_test parses the predicate. +CFG_ATTR = re.compile(r"#\s*(!)?\s*\[\s*cfg\s*\(") +CFG_TOKEN = re.compile(r'"(?:\\.|[^"\\])*"|\w+|\S') RAW_STRING = re.compile(r'b?r(#*)"') CHAR_LITERAL = re.compile(r"'(?:\\(?:x[0-9a-fA-F]{2}|u\{[0-9a-fA-F]+\}|.)|[^\\'])'", re.S) -MOD_DECL = re.compile( - r'(?:#\[path\s*=\s*"([^"]+)"\]\s*)?(?:pub(?:\([^)]*\))?\s+)?mod\s+(\w+)\s*;\s*$' +IDENT = re.compile(r"(?:r#)?[A-Za-z_]\w*") +ITEM_KEYWORDS = frozenset( + "async const crate enum extern fn impl let macro_rules mod static struct trait type union unsafe use".split() ) +MOD_ITEM = re.compile(r"(? bool: class FileCount: code: int test: int - test_mods: list[tuple[str, str | None]] = field(default_factory=list) + # (name, #[path] value, enclosing inline modules) for each file module a + # test-only item declares. + test_mods: list[tuple[str, str | None, tuple[str, ...]]] = field(default_factory=list) + # The file opens with an inner `#![cfg(test)]`. + test_file: bool = False + + +def implies_test(pred: str) -> bool: + """Whether a ``cfg`` predicate can only hold when ``test`` is set: + ``test`` itself, an ``all(...)`` with such an argument, or an ``any(...)`` + whose every argument is one. ``not(...)`` never is.""" + toks = CFG_TOKEN.findall(pred) + pos = 0 + + def parse() -> bool: + nonlocal pos + if pos >= len(toks): + return False + name = toks[pos] + pos += 1 + if pos < len(toks) and toks[pos] == "=": + pos += 2 # key = "value" + return False + if pos < len(toks) and toks[pos] == "(": + pos += 1 + args = [] + while pos < len(toks) and toks[pos] != ")": + if toks[pos] == ",": + pos += 1 + continue + args.append(parse()) + pos += 1 + if name == "all": + return any(args) + if name == "any": + return bool(args) and all(args) + return False + return name == "test" + + return parse() and pos == len(toks) + + +def _next_code(src: str, kind: bytearray, j: int) -> int: + n = len(src) + while j < n and (kind[j] == COMMENT or src[j].isspace()): + j += 1 + return j + + +def _match_close(src: str, kind: bytearray, j: int) -> int | None: + """Index of the bracket closing the one at ``j``.""" + depth = 0 + for k in range(j, len(src)): + if kind[k] != CODE: + continue + if src[k] in "{([": + depth += 1 + elif src[k] in "})]": + depth -= 1 + if depth == 0: + return k + return None + + +def _attrs_before(src: str, kind: bytearray, pos: int) -> tuple[int, list[str]]: + """The outer attributes written directly before ``pos``: (start, texts).""" + attrs: list[str] = [] + while True: + q = pos - 1 + while q >= 0 and (kind[q] == COMMENT or src[q].isspace()): + q -= 1 + if q < 0 or src[q] != "]" or kind[q] != CODE: + break + depth, r = 0, q + while r >= 0: + if kind[r] == CODE and src[r] == "]": + depth += 1 + elif kind[r] == CODE and src[r] == "[": + depth -= 1 + if depth == 0: + break + r -= 1 + h = r - 1 + while h >= 0 and src[h].isspace(): + h -= 1 + if r < 0 or h < 0 or src[h] != "#": + break + attrs.append(src[h : q + 1]) + pos = h + return pos, attrs + + +def _starts_item(src: str, kind: bytearray, j: int) -> bool: + """Whether the code at ``j`` is an item or statement, rather than a + struct field, enum variant, parameter or match arm.""" + n = len(src) + while True: + j = _next_code(src, kind, j) + if j < n and src[j] == "#": # another attribute + k = _next_code(src, kind, j + 1) + close = _match_close(src, kind, k) if k < n and src[k] == "[" else None + if close is None: + return True + j = close + 1 + continue + break + if j >= n or src[j] == "{": + return True + m = IDENT.match(src, j) + if not m: + return False + word, after = m.group(0), _next_code(src, kind, m.end()) + if word == "pub": + if after < n and src[after] == "(": + after = _next_code(src, kind, (_match_close(src, kind, after) or after) + 1) + m = IDENT.match(src, after) + if not m: + return True + word, after = m.group(0), _next_code(src, kind, m.end()) + # `pub name: Type` is a field. + return word in ITEM_KEYWORDS or not (src.startswith(":", after) and not src.startswith("::", after)) + return word in ITEM_KEYWORDS or (after < n and src[after] == "!") # or a macro call + + +def _item_end(src: str, kind: bytearray, pos: int) -> int: + """End of the item that starts at ``pos``. + + An item ends at its first top-level ``;``, at the brace closing its first + ``{`` (and a ``;`` right after it), or just before a bracket that closes + the enclosing container. A field, variant, parameter or match arm also + ends at its first top-level ``,``. + """ + n = len(src) + is_field = not _starts_item(src, kind, pos) + depth, angle, in_expr = 0, 0, False + for j in range(pos, n): + if kind[j] != CODE: + continue + ch = src[j] + if ch in "{([": + depth += 1 + elif ch in "})]": + if depth == 0: + return j + depth -= 1 + if depth == 0 and ch == "}": + nxt = _next_code(src, kind, j + 1) + if nxt < n and (src[nxt] == ";" or (is_field and src[nxt] == ",")): + return nxt + 1 + # `Variant { .. } => arm` and `S { .. }.method()` go on. + if not is_field or nxt >= n or src[nxt] not in "=.?": + return j + 1 + elif depth == 0: + if ch == ";": + return j + 1 + if is_field: + prev = src[j - 1] + if ch == ">" and prev == "=": + in_expr, angle = True, 0 # past a match arm's `=>` + elif in_expr: + pass + elif ch == "<": + angle += 1 + elif ch == ">" and prev != "-": + angle = max(0, angle - 1) + if ch == "," and angle == 0: + return j + 1 + return n + + +def _file_mods(src: str, kind: bytearray, lo: int, hi: int) -> list[tuple[str, str | None, tuple[str, ...]]]: + """The ``mod name;`` declarations in ``src[lo:hi]``, each with its + ``#[path]`` and the inline modules around it.""" + out = [] + stack: list[tuple[str, int]] = [] + for m in MOD_ITEM.finditer(src, lo, hi): + at = m.start() + if kind[at] != CODE: + continue + while stack and stack[-1][1] < at: + stack.pop() + name = m.group(1) + if m.group(2) == "{": + close = _match_close(src, kind, m.end() - 1) + stack.append((name, hi if close is None else close)) + continue + _, attrs = _attrs_before(src, kind, at) + paths = [pm.group(1) for pm in map(PATH_ATTR.search, attrs) if pm] + out.append((name, paths[0] if paths else None, tuple(s for s, _ in stack))) + return out def count_source(src: str) -> FileCount: @@ -127,32 +325,29 @@ def count_source(src: str) -> FileCount: kind = classify(src) n = len(src) excluded = bytearray(n) - test_mods: list[tuple[str, str | None]] = [] - for m in CFG_TEST.finditer(src): + test_mods: list[tuple[str, str | None, tuple[str, ...]]] = [] + test_file = False + for m in CFG_ATTR.finditer(src): start = m.start() if kind[start] != CODE or excluded[start]: continue - # The gated item ends at its first top-level `;` or at the brace - # closing its first `{`. - j, depth, end = m.end(), 0, n - while j < n: - if kind[j] == CODE: - ch = src[j] - if ch in "{([": - depth += 1 - elif ch in "})]": - depth -= 1 - if depth == 0 and ch == "}": - end = j + 1 - break - elif ch == ";" and depth == 0: - end = j + 1 - break - j += 1 - item = src[m.end() : end] - decl = MOD_DECL.search(item) - if decl and "{" not in item: - test_mods.append((decl.group(2), decl.group(1))) + close = _match_close(src, kind, m.end() - 1) + if close is None: + continue + attr_end = _next_code(src, kind, close + 1) + if attr_end >= n or src[attr_end] != "]" or not implies_test(src[m.end() : close]): + continue + if m.group(1): + # `#![cfg(test)]` outside any block gates the whole file. + opened = sum(1 if c == "{" else -1 for c, k in zip(src[:start], kind) if k == CODE and c in "{}") + if opened == 0: + test_file = True + excluded = bytearray(b"\x01" * n) + break + continue + end = _item_end(src, kind, attr_end + 1) + start, _ = _attrs_before(src, kind, start) + test_mods.extend(_file_mods(src, kind, start, end)) excluded[start:end] = b"\x01" * (end - start) code_lines: set[int] = set() @@ -163,41 +358,66 @@ def count_source(src: str) -> FileCount: line += 1 elif not ch.isspace() and kind[k] != COMMENT: (test_lines if excluded[k] else code_lines).add(line) - return FileCount(len(code_lines), len(test_lines - code_lines), test_mods) + return FileCount(len(code_lines), len(test_lines - code_lines), test_mods, test_file) -def resolve_module(declaring: str, name: str, path_attr: str | None) -> str | None: - """The file a ``mod name;`` in ``declaring`` loads, if it exists.""" +def is_file(rel: str) -> bool: + return (REPO / rel).is_file() + + +def resolve_module(declaring: str, name: str, path_attr: str | None, inline: tuple[str, ...] = ()) -> str | None: + """The file a ``mod name;`` in ``declaring`` loads, if it exists. + + ``inline`` names the inline ``mod x { ... }`` blocks around the + declaration. Like rustc, an out-of-line ``#[path]`` resolves against the + declaring file's own directory, and one inside inline modules against + their directory. + """ d, base = os.path.split(declaring) owns_dir = base in ("mod.rs", "lib.rs", "main.rs") - sub = d if owns_dir else os.path.join(d, base[:-3]) + sub = os.path.join(d if owns_dir else os.path.join(d, base[:-3]), *inline) if path_attr: - candidates = [os.path.join(sub, path_attr), os.path.join(d, path_attr)] + candidates = [os.path.join(sub if inline else d, path_attr)] else: candidates = [os.path.join(sub, name + ".rs"), os.path.join(sub, name, "mod.rs")] for c in candidates: - if (REPO / c).is_file(): + if is_file(c): return os.path.normpath(c) return None def rust_files() -> list[str]: - out = subprocess.check_output(["git", "ls-files", "*.rs"], cwd=REPO, text=True) - return [ - f - for f in out.split() - if not re.search(r"(^|/)(tests|benches)/", f) - ] + out = subprocess.check_output(["git", "ls-files", "-z", "*.rs"], cwd=REPO, text=True) + return [f for f in out.split("\0") if f and not re.search(r"(^|/)(tests|benches)/", f)] + + +def in_scope(files: list[str], groups: dict) -> list[str]: + """The group files and every file that can declare one of them as a + module: the files beside them and each ancestor directory's module file.""" + member = {f for f in files if any(in_group(f, g) for g in groups.values())} + dirs = {os.path.dirname(f) for f in member} + declarers: set[str] = set() + for d in dirs: + while d: + declarers.update(d + ext for ext in (".rs", "/mod.rs", "/lib.rs", "/main.rs")) + d = os.path.dirname(d) + return [f for f in files if f in member or f in declarers or os.path.dirname(f) in dirs] + +def count_repo(groups: dict | None = None) -> dict[str, int]: + """Implementation lines per tracked Rust file (test-only files count 0). -def count_repo() -> dict[str, int]: - """Implementation lines per tracked Rust file (test-only files count 0).""" + With ``groups``, reads only the files those groups and their module + declarations need. + """ files = rust_files() + if groups is not None: + files = in_scope(files, groups) counts = {f: count_source((REPO / f).read_text(encoding="utf-8", errors="replace")) for f in files} - test_files: set[str] = set() + test_files = {f for f, fc in counts.items() if fc.test_file} for f, fc in counts.items(): - for name, path_attr in fc.test_mods: - target = resolve_module(f, name, path_attr) + for name, path_attr, inline in fc.test_mods: + target = resolve_module(f, name, path_attr, inline) if target: test_files.add(target) # A module below a test-only module is test-only too. @@ -233,7 +453,9 @@ def main(argv: list[str] | None = None) -> int: budget = load_budget() groups = budget["groups"] - per_file = count_repo() + if args.files and args.files not in groups: + ap.error(f"unknown group {args.files!r}; groups: {', '.join(groups)}") + per_file = count_repo(groups) if args.files: group = groups[args.files] @@ -253,7 +475,8 @@ def main(argv: list[str] | None = None) -> int: over.append((name, total, cap)) if args.lower and total < cap: g["max"] = total - print(f"{'all implementation':24s} {sum(per_file.values()):8d}") + union = sum(c for f, c in per_file.items() if any(in_group(f, g) for g in groups.values())) + print(f"{'all groups':24s} {union:8d}") if args.lower: BUDGET_FILE.write_text(json.dumps(budget, indent=2) + "\n") diff --git a/scripts/tests/test_loc_budget.py b/scripts/tests/test_loc_budget.py index f3156b1aa..d5a676d63 100644 --- a/scripts/tests/test_loc_budget.py +++ b/scripts/tests/test_loc_budget.py @@ -80,9 +80,140 @@ def test_test_module_declarations_are_reported(self): "mod shipped;\n" ) fc = count(src) - self.assertEqual(fc.test_mods, [("tests", None), ("x_tests", "x_tests.rs")]) + self.assertEqual(fc.test_mods, [("tests", None, ()), ("x_tests", "x_tests.rs", ())]) self.assertEqual(fc.code, 1) + # Review findings on #2001. + + def test_cfg_test_struct_field_does_not_swallow_following_code(self): + src = ( + "struct S {\n" + " #[cfg(test)]\n" + " probe: HashMap,\n" + " real: u32,\n" + "}\n" + "fn shipped() {}\n" + ) + fc = count(src) + self.assertEqual(fc.code, 4) + self.assertEqual(fc.test, 2) + + def test_cfg_test_pub_field_last_without_comma(self): + src = "struct S {\n real: u32,\n #[cfg(test)]\n pub probe: u32\n}\nfn shipped() {}\n" + self.assertEqual(count(src).code, 4) + + def test_cfg_test_enum_variant_and_match_arm(self): + src = ( + "enum E {\n" + " A,\n" + " #[cfg(test)]\n" + " B { x: u8 },\n" + " C,\n" + "}\n" + "fn f(e: E) -> u8 {\n" + " match e {\n" + " #[cfg(test)]\n" + " E::B { .. } => 1,\n" + " _ => 0,\n" + " }\n" + "}\n" + ) + fc = count(src) + self.assertEqual(fc.code, 9) + self.assertEqual(fc.test, 4) + + def test_cfg_test_fn_param(self): + src = "fn f(\n a: u8,\n #[cfg(test)] b: u8,\n) -> u8 {\n a\n}\nfn g() {}\n" + self.assertEqual(count(src).code, 6) + + def test_cfg_test_braced_use_consumes_its_semicolon(self): + src = "#[cfg(test)]\nuse a::{\n b,\n c,\n};\nfn shipped() {}\n" + fc = count(src) + self.assertEqual(fc.code, 1) + self.assertEqual(fc.test, 5) + + def test_cfg_all_with_test_not_first_and_not_test(self): + src = ( + "#[cfg(all(unix, test))]\nfn a() {}\n" + "#[cfg(all(unix, all(windows, test)))]\nfn b() {}\n" + "#[cfg(all(unix, not(test)))]\nfn shipped() {}\n" + "#[cfg(any(test, test))]\nfn c() {}\n" + ) + self.assertEqual(count(src).code, 2) + + def test_file_level_cfg_test(self): + src = "//! helpers\n#![cfg(test)]\n\nfn a() {}\nmod nested;\n" + fc = count(src) + self.assertTrue(fc.test_file) + self.assertEqual(fc.code, 0) + src = "#![cfg(not(test))]\nfn a() {}\n" + self.assertFalse(count(src).test_file) + self.assertEqual(count(src).code, 2) + + def test_path_attr_after_other_attrs(self): + src = ( + "#[cfg(test)]\n" + '#[path = "x_tests.rs"]\n' + "#[allow(dead_code)]\n" + "mod x_tests;\n" + ) + self.assertEqual(count(src).test_mods, [("x_tests", "x_tests.rs", ())]) + + def test_inline_test_mod_records_nested_file_modules(self): + src = ( + "#[cfg(test)]\n" + "mod support {\n" + " mod fixtures;\n" + " pub mod inner {\n" + ' #[path = "h.rs"]\n' + " mod helpers;\n" + " }\n" + "}\n" + ) + self.assertEqual( + count(src).test_mods, + [("fixtures", None, ("support",)), ("helpers", "h.rs", ("support", "inner"))], + ) + + +class ResolveModuleTest(unittest.TestCase): + def setUp(self): + self.saved = loc_budget.is_file + self.files = set() + loc_budget.is_file = lambda p: p in self.files + + def tearDown(self): + loc_budget.is_file = self.saved + + def test_path_attr_is_relative_to_the_declaring_files_dir(self): + # Rust resolves an out-of-line #[path] against the declaring file's + # directory, even for a non-mod.rs file. + self.files = {"src/a/x_tests.rs", "src/a/x/x_tests.rs"} + self.assertEqual( + loc_budget.resolve_module("src/a/x.rs", "t", "x_tests.rs", ()), "src/a/x_tests.rs" + ) + + def test_inline_module_path(self): + self.files = {"src/a/x/support/fixtures.rs", "src/a/support/fixtures.rs"} + self.assertEqual( + loc_budget.resolve_module("src/a/x.rs", "fixtures", None, ("support",)), + "src/a/x/support/fixtures.rs", + ) + self.assertEqual( + loc_budget.resolve_module("src/a/mod.rs", "fixtures", None, ("support",)), + "src/a/support/fixtures.rs", + ) + + +class RustFilesTest(unittest.TestCase): + def test_paths_with_spaces_survive(self): + saved = loc_budget.subprocess.check_output + loc_budget.subprocess.check_output = lambda *a, **k: "a b/c.rs\0x/tests/t.rs\0d.rs\0" + try: + self.assertEqual(loc_budget.rust_files(), ["a b/c.rs", "d.rs"]) + finally: + loc_budget.subprocess.check_output = saved + class ModeTest(unittest.TestCase): """--check fails over budget, --warn only annotates.""" @@ -94,7 +225,7 @@ def setUp(self): "g": {"paths": ["a/"], "exclude": ["a/skip/"], "max": 10}, } } - loc_budget.count_repo = lambda: {"a/x.rs": 8, "a/y.rs": 5, "a/skip/z.rs": 100, "b/w.rs": 1} + loc_budget.count_repo = lambda groups=None: {"a/x.rs": 8, "a/y.rs": 5, "a/skip/z.rs": 100, "b/w.rs": 1} def tearDown(self): loc_budget.load_budget, loc_budget.count_repo = self.saved @@ -123,6 +254,41 @@ def test_report_mode_never_fails(self): code, out = self.run_main() self.assertEqual(code, 0) self.assertNotIn("::warning", out) + self.assertIn("all groups 13", out) + + def test_files_lists_one_group(self): + code, out = self.run_main("--files", "g") + self.assertEqual(code, 0) + self.assertIn(" 8 a/x.rs", out) + self.assertIn(" 13 total", out) + + def test_files_with_an_unknown_group_is_a_usage_error(self): + import contextlib + import io + + with contextlib.redirect_stderr(io.StringIO()) as err, self.assertRaises(SystemExit) as cm: + loc_budget.main(["--files", "nope"]) + self.assertEqual(cm.exception.code, 2) + self.assertIn("unknown group 'nope'", err.getvalue()) + + +class InScopeTest(unittest.TestCase): + def test_reads_group_files_siblings_and_declarers_only(self): + files = [ + "c/src/lib.rs", + "c/src/a/mod.rs", + "c/src/a/g/mod.rs", + "c/src/a/g/x.rs", + "c/src/a/g/deep/y.rs", + "c/src/a/sib.rs", + "c/src/other/z.rs", + "d/src/lib.rs", + ] + groups = {"g": {"paths": ["c/src/a/g/"], "max": 1}} + self.assertEqual( + loc_budget.in_scope(files, groups), + ["c/src/lib.rs", "c/src/a/mod.rs", "c/src/a/g/mod.rs", "c/src/a/g/x.rs", "c/src/a/g/deep/y.rs"], + ) class BudgetFileTest(unittest.TestCase): From 75d575e6c405f29d4510ae28f07544d3e965155c Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 5 Nov 2026 23:44:23 -0600 Subject: [PATCH 7/9] Record the 2026-11-05 owner decisions; link the fix PRs Review of #2001: the Decisions table said every read is bounded to the repository root, while G1, J13 and P4 still said to read an ancestor go.work, parent NuGet configs and every include. The owner narrowed the rule on 2026-11-05, and the table now records it with the rest of that day's calls: - the bound applies to workspace runs only; --global / -g / --global-prefix runs are unbounded, and without a git checkout upward lookups behave as before - a marker file above the repository root (below home) makes hosted and vendored refuse with a message naming it - out-of-repository -r includes stay bounded in workspace runs - N11: shrinkwrap.yaml beside pnpm-lock.yaml is shadowed, unless the project pins pnpm 1 / 2, where it governs and vendored refuses - discovery withholds pnpm root and member-lock attestations whenever a pnpm-lock..yaml exists - user patches: the override is --allow-patch-conflicts, and vex never attests under a user patch G1, J9, J13, P4, P9, X12, N1, N2, N4, N5, N11, N12 and N14 name the open PRs that fix them, and the fixes-on-main table gains a PR column. The problem table now counts four checksums.sha256 readers, as J9 does. The stack table splits PR 2 into 2a / 2b with their numbers, and the size budget section says why CI only warns and gives the corrected counts. Co-Authored-By: Claude Opus 5.5 --- docs/design/lock-model-divergences.md | 55 +++++++++++++++------------ docs/design/lock-model-unification.md | 49 +++++++++++++++--------- 2 files changed, 60 insertions(+), 44 deletions(-) diff --git a/docs/design/lock-model-divergences.md b/docs/design/lock-model-divergences.md index 1fb560ff6..2a2003e2b 100644 --- a/docs/design/lock-model-divergences.md +++ b/docs/design/lock-model-divergences.md @@ -17,12 +17,14 @@ Flags: * **target-specific**: the difference comes from where the entry points (a Socket URL or a committed path) and stays. -## Calls that go against hosted (owner decided 2026-11-04) +## Calls that go against hosted (owner decided 2026-11-04 and 2026-11-05) Owner: the false Maven attestations get fixed, the modes are ground truth over the readers, and the vendored mode's profile, reactor and scala-cli support is back-ported so hosted gains it as the planners merge. P3 and P4 -follow from the install-tree decision. +follow from the install-tree decision. N11 was decided on 2026-11-05. + +"(draft)" after a PR number means that PR is open and not merged yet. | # | Question | Hosted today | Decision | |---|---|---|---| @@ -30,8 +32,8 @@ follow from the install-tree decision. | J3 | Which Maven poms count? | Refuses a reactor root (~8572) | The reactor reader becomes the shared core, so hosted gains reactor support. | | J12 | scala-cli | Guidance only (`patch/redirect/scala_guidance.rs`) | Back-port: hosted wires scala-cli through the merged planner (a same-GAV Socket repository instead of a suffixed version). | | P3 | Is a shadowed `pdm.lock` rewritten? | Only when it governs (`pdm_drives`, ~564) | Rewrite it like every other tree, per the install-tree decision. | -| P4 | Are `-r` includes of `requirements.txt` rewritten? | Root file only (`patch/redirect/requirements.rs` ~208) | Follow includes, per the install-tree decision. | -| N11 | Is a `shrinkwrap.yaml` beside `pnpm-lock.yaml` rewritten? | Yes; its all-or-nothing gate covers both files | **Pending.** Proposed: treat it as shadowed (swept for stale identities, never written). pnpm never reads it, and an unparseable debris key in it can refuse the dependency in every lock. | +| P4 | Are `-r` includes of `requirements.txt` rewritten? | Root file only (`patch/redirect/requirements.rs` ~208) | Follow includes, per the install-tree decision. An include outside the repository root stays bounded in workspace runs: never read or rewritten (owner, 2026-11-05; #2025 (draft)). | +| N11 | Is a `shrinkwrap.yaml` beside `pnpm-lock.yaml` rewritten? | Yes; its all-or-nothing gate covers both files | **Owner decision (2026-11-05): shadowed.** Swept for stale identities, never written, and out of the all-or-nothing gate. pnpm never reads it, and an unparseable debris key in it can refuse the dependency in every lock. When the project's pnpm evidence pins pnpm 1 / 2, `shrinkwrap.yaml` governs instead and vendored mode refuses. Fixed in #2029 (draft). | ## JVM @@ -43,10 +45,10 @@ follow from the install-tree decision. | J6 | Is sbt resolution evidence required before attesting? | Hosted: no integrity without verified evidence. Vendored: `wired_checked` only checks the owned-file row; discovery's `vendored_check` diagnostics are never consumed. | Hosted. One evidence route (`ResolutionDoc` + `gate::check_new`). | false attestation | | J7 | Another version of a mapped NuGet id is locked | Both writers refuse. Discovery emits a ref per locked version, keyed by raw text, not `normalize_nuget_version`. | Hosted: a diagnostic, no ref | false attestation | | J8 | What counts as "wired" for vendored Maven liveness and GC? | Vendored: substring match on masked text, so a suffix in ``, a profile or `` counts. Discovery and hosted use the scoped `PomScope`. | Scoped reader | | -| J9 | `checksums.sha256` and the `TRUSTED_CHECKSUMS_ON` switch | Four readers with different splitting and switch tests. Hosted's merge silently drops lines it can't split on two spaces. | One reader with the hosted grammar. The merge keeps lines it can't parse. | | +| J9 | `checksums.sha256` and the `TRUSTED_CHECKSUMS_ON` switch | Four readers with different splitting and switch tests. Hosted's merge silently drops lines it can't split on two spaces. | One reader with the hosted grammar. The merge keeps lines it can't parse. Fixed in #2016 (draft). | | | J10 | Gradle Socket index rows | Two parsers. Hosted validates the uuid, URL and hashes; vendored accepts any token and uses a raw `split('\t')`. Vendored checks only root settings; hosted checks every build. | One row grammar, hosted's validation | | | J11 | `verification-metadata.xml` | Two editors, no reader | One editor; a reader for the inventory | | -| J13 | NuGet config scope | Writers read inherited parent configs; discovery doesn't. Discovery refuses a disabled Socket source; the writers don't check ``. | Shared inheritance read. The writers adopt the disabled-source refusal. | | +| J13 | NuGet config scope | Writers read inherited parent configs; discovery doesn't. Discovery refuses a disabled Socket source; the writers don't check ``. | Shared inheritance read, bounded to the repository root in workspace runs; global runs, and runs without a git checkout, read parent configs as before (owner, 2026-11-05; #2025 (draft)). The writers adopt the disabled-source refusal. | | | J14 | NuGet lock enumeration | In-memory views fall back to the root lock. The unwind bypasses the snapshot view (`governed_locks_on_disk`). | One enumeration over `ProjectView` | | | J15 | JVM liveness on a read error | `entry_wired` is `unwrap_or(true)` for `scan`/`vendor`; `vex` uses the checked variant | Fail closed everywhere | false attestation | @@ -59,7 +61,7 @@ follow from the install-tree decision. | P6 | What counts as Socket wiring? | Discovery and the unwind use the host allowlist. The inventory accepts any host. Ledger liveness uses `resolved.contains(uuid)`. `poetry_lock::lock_wiring` uses `contains("/patch/pypi/")`. | The `origin` allowlist everywhere | false attestation (liveness substring) | | P7 | uv workspace roots | Hosted refuses members only. Vendored refuses roots too. | Vendored wires roots, after a test that a member's own `[tool.uv.sources]` can't override the root | | | P8 | `uv.lock` without `pyproject.toml` | Hosted edits the lock; discovery attests it. Vendored refuses. | Vendored proceeds | | -| P9 | uv 0.2 `[[distribution]]` locks | Hosted rewrites. Vendored refuses (no portable path). The unwind refuses to restore them. | Vendored's refusal is target-specific and stays. The unwind must restore what hosted writes. | target-specific | +| P9 | uv 0.2 `[[distribution]]` locks | Hosted rewrites. Vendored refuses (no portable path). The unwind refuses to restore them. | Vendored's refusal is target-specific and stays. The unwind must restore what hosted writes (#2014 (draft)). | target-specific | | P13 | PDM `lock_version` | Writers use an allowlist; discovery a denylist; the inventory checks nothing | The allowlist everywhere | | | P14 | `pipfile-spec` | Writers require 6; discovery and the inventory accept any | 6 everywhere | | @@ -67,32 +69,32 @@ follow from the install-tree decision. | # | Question | Today | Resolution | Flag | |---|---|---|---|---| -| G1 | Which `go.work` and members count? | Discovery: a root `go.work` and in-root members only, ignoring an ancestor `go.work`, `GOWORK` and `-modfile`. Hosted and vendored resolve the workspace go would use (`vendor/go_workspace.rs`). | `go_workspace::load` everywhere | false attestation | +| G1 | Which `go.work` and members count? | Discovery: a root `go.work` and in-root members only, ignoring an ancestor `go.work`, `GOWORK` and `-modfile`. Hosted and vendored resolve the workspace go would use (`vendor/go_workspace.rs`). | `go_workspace::load` everywhere (discovery: #2011 (draft)). In workspace runs it never reads above the repository root, and a `go.work` there makes the modes refuse and discovery withhold; global runs, and runs without a git checkout, find the ancestor `go.work` as before (owner, 2026-11-05; #2025 (draft)). | false attestation | | G16 | Go inventory | Root `go.sum` only, lenient parse, lists `patch.socket.dev/gopatch/` lines as packages | Discovery's model and workspace | | | C2 | Which manifests decide whether a Cargo lock entry is used? | Hosted pins root and member manifests; vendored reads members. Discovery reads only the root `Cargo.toml`. | Members everywhere | false attestation | | C11 | `cargo vendor` / local-registry source replacement | Hosted refuses; vendored and discovery ignore it | Stays | target-specific | | R10 | `gems.rb` / `gems.locked` | Hosted wires the pair bundler loads. Vendored refuses any `gems.rb`. | Vendored supports it | | | R15 | Malformed Gemfile.lock | Discovery reports it unparseable; the inventory lists what parsed | Discovery's rule everywhere | | -| X12 | `composer.lock` with a BOM | Discovery strips it. The inventory drops the file. Vendored refuses to plan, errors on revert, and reports nothing stranded. | Strip one BOM everywhere. Check that GC never trusts the empty "stranded" answer. | | +| X12 | `composer.lock` with a BOM | Discovery strips it. The inventory drops the file. Vendored refuses to plan, errors on revert, and reports nothing stranded. | Strip one BOM everywhere. Check that GC never trusts the empty "stranded" answer. The inventory part in #2017 (draft). | | ## npm family | # | Question | Today | Resolution | Flag | |---|---|---|---|---| -| N1 | pnpm `gitBranchLockfile` with a branch lock present: is the root lock the install tree? | Hosted withholds the root locks and refuses (`hosted/engine.rs` ~930). Vendored refuses. Discovery and the inventory never check the setting. | Hosted | false attestation | -| N2 | npm user patches (#711) and `bun patch` (#367) | Hosted skips that one dependency and records it as refused. Vendored refuses the whole npm v4 lock, or the one bun dependency. Discovery never reads `patchedDependencies` / `"patched"` and attests a pre-#711 hosted pin. | Hosted's per-dependency skip, in discovery too | false attestation | +| N1 | pnpm `gitBranchLockfile` with a branch lock present: is the root lock the install tree? | Hosted withholds the root locks and refuses (`hosted/engine.rs` ~930). Vendored refuses. Discovery and the inventory never check the setting. | Hosted. Discovery withholds pnpm root and member-lock attestations whenever any `pnpm-lock..yaml` exists, whatever the settings or environment say (owner, 2026-11-05; #2010 (draft)). | false attestation | +| N2 | npm user patches (#711) and `bun patch` (#367) | Hosted skips that one dependency and records it as refused. Vendored refuses the whole npm v4 lock, or the one bun dependency. Discovery never reads `patchedDependencies` / `"patched"` and attests a pre-#711 hosted pin. | Hosted's per-dependency skip, in discovery too, for every package manager's user patches (owner decision; #2010 (draft) for npm and bun, #2018 (draft) for pnpm, yarn and vlt). The override is the existing `--allow-patch-conflicts`; `vex` never attests under a user patch, even with it. | false attestation | | N3 | A workspace member's stray npm/bun/vlt lock | Discovery treats it as absent. Hosted and vendored refuse. The inventory reads it as the project's lock. | The inventory ignores it | | -| N4 | pnpm 11's env-lockfile document | See [Decisions](lock-model-unification.md#decisions-under-the-hosted-rule). `main_document` takes the text after the last `---`, so a crafted multi-document lock chooses its own "project". | Its own tree; documents identified by content, not position | | -| N5 | What counts as Socket wiring | Discovery and the unwind use the allowlist. The inventory records hosted URLs on any host. `parse_vendor_path` accepts `.socket/vendor/` anywhere, so a `../.socket/vendor/…` entry from a sibling checkout is "ours" to the vendored writer and reverter. The hosted pnpm "vendored?" check is a substring heuristic. | The `origin` classifier everywhere | revert could rewrite another project's wiring | +| N4 | pnpm 11's env-lockfile document | See [Decisions](lock-model-unification.md#decisions-under-the-hosted-rule). `main_document` takes the text after the last `---`, so a crafted multi-document lock chooses its own "project". | Its own tree; documents identified by content, not position. Fixed in #2009 (draft). | | +| N5 | What counts as Socket wiring | Discovery and the unwind use the allowlist. The inventory records hosted URLs on any host. `parse_vendor_path` accepts `.socket/vendor/` anywhere, so a `../.socket/vendor/…` entry from a sibling checkout is "ours" to the vendored writer and reverter. The hosted pnpm "vendored?" check is a substring heuristic. | The `origin` classifier everywhere. The vendored writers, reverters and GC claim only this project's vendor paths in #2025 (draft); the pnpm readers and planner follow in #2009 / #2029 (draft). | revert could rewrite another project's wiring | | N6 | `npm-shrinkwrap.json` and `package-lock.json` | Hosted, vendored and discovery read both, with a contest. The inventory reads the shrinkwrap only. | Both, everywhere | | | N7 | npm lockfileVersion | Hosted rewrites v1 and v4. Vendored accepts v2/v3 only. The inventory ignores v1. Discovery reads v1 and v2+. | Hosted, once vendored `file:` wiring under npm 6 and npm 12.1 v4 has fixtures | | | N8 | npm `file:` directory link identity (#1481) | Hosted and discovery give a versionless link its source directory's `name@version`. Vendored matches the entry's own version, wiring the registry copies with no warning. | Hosted | silent vendored success | | N9 | yarn classic blocks reached only through an alias | Hosted skips them. Vendored rewrites them. Discovery counts an alias block as an install of the real package. | Back-port: hosted rewrites alias blocks (vendored supports what hosted lacks) | | | N10 | Locks of several npm-family package managers in one project | Hosted rewrites every family; discovery reads every family with the contest. The inventory and vendored pick one family (vlt > bun > pnpm > yarn > npm). | Union (separate trees) | | -| N11 | `shrinkwrap.yaml` beside `pnpm-lock.yaml` | Hosted reads and splices it, and its all-or-nothing gate covers both. Discovery sweeps it as ignored. The inventory reads it only as a fallback. Vendored never reads it. | Pending owner decision (see above) | needs owner | -| N12 | Rush common and subspace locks | Hosted and discovery always read them. The inventory reads them only when there is no root lock (#2003 aligns the listing itself). Vendored refuses Rush. | Always read. Vendored's refusal stays. | target-specific (vendored) | +| N11 | `shrinkwrap.yaml` beside `pnpm-lock.yaml` | Hosted reads and splices it, and its all-or-nothing gate covers both. Discovery sweeps it as ignored. The inventory reads it only as a fallback. Vendored never reads it. | Shadowed (owner, 2026-11-05; see above). Fixed in #2029 (draft). | | +| N12 | Rush common and subspace locks | Hosted and discovery always read them. The inventory reads them only when there is no root lock (#2003 aligns the listing itself). Vendored refuses Rush. | Always read. Vendored's refusal stays. Fixed in #2009 (draft). | target-specific (vendored) | | N13 | Plug'n'Play | Vendored refuses. The hosted berry preflight has no PnP gate. The inventory returns an error, so `scan` sees nothing in a project hosted pinned. | The inventory reads PnP locks. Vendored keeps its refusal. | target-specific (vendored) | -| N14 | pnpm `lockfile=false` | Hosted refuses. Discovery withholds hosted refs, keeps vendored ones. Vendored proceeds. The inventory ignores the setting. | The modes stay as they are; the inventory follows discovery | target-specific | +| N14 | pnpm `lockfile=false` | Hosted refuses. Discovery withholds hosted refs, keeps vendored ones. Vendored proceeds. The inventory ignores the setting. | The modes stay as they are; the inventory follows discovery. Fixed in #2009 (draft). | target-specific | | N15 | CRLF `pnpm-lock.yaml` / `pnpm-workspace.yaml` | Vendored refuses; the rest handle it | Handle it everywhere | | | N16 | Lock versions | pnpm: vendored accepts only 5.4/6.0/9.0; hosted any but early shrinkwrap. vlt: vendored refuses a lock with no version; hosted rewrites and warns. | Hosted | | | N17 | Bun workspaces, yarn classic offline mirror | Refused differently by the two modes | Stays | target-specific | @@ -103,17 +105,20 @@ follow from the install-tree decision. ## Fixes for false attestations on main -These are fixed directly against `main`, ahead of the stack, one PR each. +These are fixed directly against `main`, ahead of the stack, in their own PRs. The stack rebases over them. -| # | Fix | -|---|---| -| J1, J4 | Maven discovery: a profile declaration or a `.mvn/maven.config` user property that resolves the base version leaves the ref unattested | -| J2, J8, J15 | Vendored Maven liveness: executable classifiers block, "wired" is read from scoped declarations, read errors fail closed | -| J5 | Vendored Gradle liveness reads the lock files | -| J6 | Vendored sbt attestation requires resolution evidence | -| J7 | NuGet discovery: no ref for a version the writers refuse | -| N1, N2, G1, C2, P6 | Next batch: pnpm `gitBranchLockfile`, npm/bun user patches, Go workspaces, Cargo members, the liveness substring match | +| # | Fix | PRs | +|---|---|---| +| J1, J4 | Maven discovery: a profile declaration or a `.mvn/maven.config` user property that resolves the base version leaves the ref unattested | #2005, #2020, #2026 (draft) | +| J2, J8, J15 | Vendored Maven liveness: executable classifiers block, "wired" is read from scoped declarations, read errors fail closed | #2006 (draft) | +| J5 | Vendored Gradle liveness reads the lock files | #2004 (draft) | +| J6 | Vendored sbt attestation requires resolution evidence | #2008, #2030 (draft) | +| J7 | NuGet discovery: no ref for a version the writers refuse | #2007 (draft) | +| N1, N2 | pnpm branch locks and npm/bun user patches in discovery; pnpm, yarn and vlt user patches everywhere | #2010, #2018 (draft) | +| G1 | Go discovery reads the governing `go.work` | #2011 (draft) | +| C2, P6 | Cargo members in discovery; liveness matches uuids by origin, not by substring | #2012, #2019, #2024 (draft) | +| G1, J13, P4, N5 | Reads bounded to the repository root in workspace runs (owner, 2026-11-05) | #2025 (draft) | ## Planner merges (hosted and vendored) diff --git a/docs/design/lock-model-unification.md b/docs/design/lock-model-unification.md index 847dd1492..b14c27ac5 100644 --- a/docs/design/lock-model-unification.md +++ b/docs/design/lock-model-unification.md @@ -1,7 +1,7 @@ # One lock model for every reader Status: in progress (stacked PRs `lock-model/*`). Owner decisions are dated -2026-11-04. +2026-11-04 unless the row says 2026-11-05. Paths below are relative to `crates/socket-patch-core/src/`. @@ -33,7 +33,7 @@ attestation rules) is real policy and stays. | Three Python precedence rules | the inventory's chain (uv by parse success, then poetry or pdm if they have entries, then Pipfile.lock plus requirements), `formats/governing_locks.rs` (uv, poetry, pdm, Pipfile.lock by presence) and discovery (every lock, plus the cross-lock contest). They disagree on four common layouts. | | Second walks of one file | `vendor/npm_origin.rs` walks package-lock `packages` again. Yarn writers call `scan_blocks` directly, and the Berry rewriter uses a third model (`formats/yarn/stanzas.rs`). The vlt writers use a separate line grammar. pnpm's `InstalledPackages` and `vendored_npm_uuids`, and the hosted planner's whole-text `grammar::entries`, skip the env-document filter. | | Second parses of one file | `vex/discover/pypi_locks.rs` parses uv, pylock and script locks itself. The inventory parses them again. `deno.lock` is parsed by `crawlers/deno_crawler.rs` for discovery, the hosted engine and the npm flavor probe. | -| Repeated helper parsers | two Python lock listers, two `-r` include walkers, two PEP 508 direct-reference parsers, five walks of `Cargo.toml` dependency tables, two Rush lock listings, three readers of Maven's `checksums.sha256`, two Gradle Socket-index parsers, two routes to sbt resolution evidence | +| Repeated helper parsers | two Python lock listers, two `-r` include walkers, two PEP 508 direct-reference parsers, five walks of `Cargo.toml` dependency tables, two Rush lock listings, four readers of Maven's `checksums.sha256`, two Gradle Socket-index parsers, two routes to sbt resolution evidence | | Two file-selection layers | the npm flavor router (`lock_inventory/npm_family.rs`) picks one family and one lock. Discovery reads every lock and sweeps the ignored ones with `recognize_ignored`. | The inventory also has gaps. It has no models for NuGet, Maven, Gradle, sbt, @@ -77,8 +77,12 @@ Non-goal: adding new hosted or vendored backends. | Question | Readers before | Decision | |---|---|---| | Is pnpm 11's env-lockfile document (the first YAML document, holding `configDependencies` / `packageManagerDependencies`) part of the project? | The hosted planner and the v9 vendored in-use check read it. `PnpmLock` (inventory and discovery), `InstalledPackages` and the legacy vendored check skip it (#1949). | It counts. pnpm installs config dependencies (into its config store, not `node_modules`), and the hosted planner patches them. The document becomes its own install tree. | -| Does Socket patch a dependency the user patched themselves (npm `patchedDependencies` / `"patched"`, `bun patch`, pnpm `patchedDependencies`, yarn berry `patch:`, vlt patches)? | Hosted skips npm and bun user patches (#711, #367). Vendored refuses them. Discovery attested them until #2010. pnpm, yarn and vlt user patches are not detected anywhere. | **Owner decision:** never, by default, in every mode and every package manager. A Socket pin under a user patch is not attested. Only an explicit user override patches it. | -| May a reader or writer read project files outside the git repository root? | Several do: vendored paths spelled `../.socket/vendor/…`, an ancestor `go.work`, out-of-root `-r` includes, parent NuGet configs. | **Owner decision:** no. Every project-file read is bounded to the git repository root (the project root when there is no repository). Machine-level tool configuration (for example `GOENV`, the user `NuGet.Config`) is listed separately for review. | +| Does Socket patch a dependency the user patched themselves (npm `patchedDependencies` / `"patched"`, `bun patch`, pnpm `patchedDependencies`, yarn berry `patch:`, vlt patches)? | Hosted skips npm and bun user patches (#711, #367). Vendored refuses them. Discovery attested them until #2010. pnpm, yarn and vlt user patches are not detected anywhere. | **Owner decision:** never, by default, in every mode and every package manager. A Socket pin under a user patch is not attested. **2026-11-05:** the only override is the existing `--allow-patch-conflicts`, and `vex` never attests a pin under a user patch, even with it (#2010, #2018). | +| May a reader or writer read project files outside the git repository root? | Several do: vendored paths spelled `../.socket/vendor/…`, an ancestor `go.work`, out-of-root `-r` includes, parent NuGet configs. | **Owner decision:** no. Every project-file read is bounded to the git repository root (the project root when there is no repository). Machine-level tool configuration (for example `GOENV`, the user `NuGet.Config`) is listed separately for review. **2026-11-05:** the bound applies to workspace runs only. Global runs (`--global` / `-g` / `--global-prefix`) read anywhere. Without a git checkout, upward lookups (an ancestor `go.work`, parent NuGet configs, workspace and settings files) behave as before (#2025). | +| In a workspace run, what if a file the tool honors sits above the repository root (an ancestor `pnpm-workspace.yaml`, `.npmrc`, `go.work`, `.cargo/config.toml`)? | Each walk reads it, or stops at a different place. | **Owner decision, 2026-11-05:** such a marker file above the repository root and below the home directory makes hosted and vendored mode refuse with a message naming the file, and readers withhold. Nothing at or above home counts (#2025). | +| Do out-of-repository `-r` includes of `requirements.txt` count? | Hosted rewrites the root file only; the inventory and discovery follow includes wherever they point. | **Owner decision, 2026-11-05:** includes are followed (P4), but an include outside the repository root stays bounded in workspace runs: never read or rewritten (#2025). | +| Is a `shrinkwrap.yaml` beside `pnpm-lock.yaml` read (N11)? | Hosted rewrites both under one all-or-nothing gate. Discovery sweeps it as ignored. The inventory reads it as a fallback. Vendored never reads it. | **Owner decision, 2026-11-05:** it is shadowed: swept for stale identities, never rewritten, and outside the gate. If the project's pnpm evidence (install record or `package.json` `packageManager`) pins pnpm 1 or 2, which read only `shrinkwrap.yaml`, then `shrinkwrap.yaml` governs and vendored mode refuses the project (#2029). | +| Does a pnpm branch lock (`pnpm-lock..yaml`) make the root lock unreliable (N1)? | Hosted withholds the root locks and refuses when `gitBranchLockfile` is set. Vendored refuses. Discovery and the inventory never check. | **Owner decision, 2026-11-05:** discovery withholds pnpm root and member-lock attestations whenever any `pnpm-lock..yaml` exists, whatever the settings or environment say (#2010). | ## The model @@ -251,18 +255,24 @@ Deprecation messaging is separate from this work. ## Size budget `scripts/loc-budget.py` counts implementation lines: code outside comments, -outside `#[cfg(test)]` items and outside test-only modules. It compares the -groups in `scripts/loc-budget.json` with their maxima. CI's -`lint-ecosystems` job runs it with `--warn`, which annotates a group over -budget but never fails, so another PR's bug fix in one of these files never -fails on it. There are two groups: -`lock-readers` (`formats/`, `vex/discover/`, `vendor/lock_inventory/`, -`gradle/`, the Python lock helpers in `utils/`, `vendor/npm_origin.rs` and -`crawlers/deno_crawler.rs`) and `lock-writers` (`patch/redirect/`, `hosted/` -and the rest of `vendor/`). At 7bde3ba55 they hold 30,898 and 88,363 lines. -Each PR in this stack -lowers the maxima with `--lower` in the same commit. The NuGet and JVM PRs -add code and raise the readers' maximum by hand, saying why. +outside items whose `cfg` implies `test` and outside test-only modules. It +compares the groups in `scripts/loc-budget.json` with their maxima. There are +two groups: `lock-readers` (`formats/`, `vex/discover/`, +`vendor/lock_inventory/`, `gradle/`, the Python lock helpers in `utils/`, +`vendor/npm_origin.rs` and `crawlers/deno_crawler.rs`) and `lock-writers` +(`patch/redirect/`, `hosted/` and the rest of `vendor/`). At 7bde3ba55 they +hold 30,462 and 88,362 lines. + +CI's `lint-ecosystems` job runs it with `--warn`, which annotates a group +over budget but never fails. That is deliberate. These groups cover most of +the lockfile code, and while the stack is open, bug fixes keep landing on +`main` in the same files (the false-attestation fixes below are a dozen +PRs). A failing check would block each of them on a number this campaign +owns, or push them to raise the maximum by hand, which hides the stack's +progress. The stack is what lowers the numbers: each PR runs `--check` +locally and lowers the maxima with `--lower` in the same commit. The NuGet +and JVM PRs add code and raise the readers' maximum by hand, saying why. Once +the stack lands, `--warn` can become `--check`. ## Stack @@ -278,9 +288,10 @@ A format whose planner merge is large splits into two PRs, readers first. | PR | Branch | Change | |---|---|---| -| 0 | `lock-model/0-design` | This document and the size budget | -| 1 | `lock-model/1-origin` | The `origin` module: discovery's classifier and the redirect's hosted-URL rule, moved verbatim. Discovery and the redirect re-export it. | -| 2 | `lock-model/2-pnpm` | Pilot. One pnpm entry walk for the inventory, discovery, the store crawler, the hosted planner and the vendored in-use check. The env-lockfile document becomes its own tree (see Decisions). | +| 0 | `lock-model/0-design` (#2001) | This document and the size budget | +| 1 | `lock-model/1-origin` (#2002) | The `origin` module: discovery's classifier and the redirect's hosted-URL rule, moved verbatim. Callers import it from `crate::origin`. | +| 2a | `lock-model/2a-pnpm-readers` (#2009) | Pilot, readers. One pnpm entry walk for the inventory, discovery, the store crawler, the hosted planner and the vendored in-use check. The env-lockfile document becomes its own tree (see Decisions). | +| 2b | `lock-model/2b-pnpm-planner` (#2029) | Pilot, planner. One pnpm rewire planner for both modes. `shrinkwrap.yaml` beside `pnpm-lock.yaml` is shadowed (see Decisions). | | 3 | `lock-model/3-npm` | package-lock and shrinkwrap. Folds `vendor/npm_origin.rs`'s second walk into the records. | | 4 | `lock-model/4-yarn` | Classic and Berry. One block model for every reader and writer, `BerryStanzas` included. | | 5 | `lock-model/5-bun-vlt` | `bun.lock`, `bun.lockb` and vlt. The vlt writers move off their line grammar. | From 4b1fc91dd2ec5d7e7412b69827d96f8f72607f17 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 09:24:26 +0000 Subject: [PATCH 8/9] Keep cfg(test) crate-visible fields and const generics from swallowing code loc-budget.py only recognized `pub name: Type` as a field, so a #[cfg(test)] on a `crate name: Type` field or a `const N: usize` generic parameter was read as an item. _item_end then didn't stop at the comma and excluded the rest of the struct or parameter list from the implementation count. Treat `crate` followed by a name like `pub`, and read `const` as a parameter when the attribute follows `<` or `,`, where no item can start. A `crate::path` statement and a plain const item keep their current handling. Reported by Bugbot on #2001. Co-Authored-By: Claude --- scripts/loc-budget.py | 28 +++++++++++++++++++++------- scripts/tests/test_loc_budget.py | 22 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/scripts/loc-budget.py b/scripts/loc-budget.py index b4e024218..7f9b3a0f4 100755 --- a/scripts/loc-budget.py +++ b/scripts/loc-budget.py @@ -220,9 +220,18 @@ def _attrs_before(src: str, kind: bytearray, pos: int) -> tuple[int, list[str]]: return pos, attrs -def _starts_item(src: str, kind: bytearray, j: int) -> bool: +def _prev_code(src: str, kind: bytearray, j: int) -> int: + while j >= 0 and (kind[j] == COMMENT or src[j].isspace()): + j -= 1 + return j + + +def _starts_item(src: str, kind: bytearray, j: int, in_list: bool = False) -> bool: """Whether the code at ``j`` is an item or statement, rather than a - struct field, enum variant, parameter or match arm.""" + struct field, enum variant, parameter, generic parameter or match arm. + + ``in_list`` says the attribute follows a ``<`` or ``,``, where an item + can't start, so ``const N: usize`` there is a const generic parameter.""" n = len(src) while True: j = _next_code(src, kind, j) @@ -240,8 +249,9 @@ def _starts_item(src: str, kind: bytearray, j: int) -> bool: if not m: return False word, after = m.group(0), _next_code(src, kind, m.end()) - if word == "pub": - if after < n and src[after] == "(": + # `crate name: Type` is a crate-visible field, but `crate::f()` is a path. + if word == "pub" or (word == "crate" and IDENT.match(src, after)): + if word == "pub" and after < n and src[after] == "(": after = _next_code(src, kind, (_match_close(src, kind, after) or after) + 1) m = IDENT.match(src, after) if not m: @@ -249,10 +259,12 @@ def _starts_item(src: str, kind: bytearray, j: int) -> bool: word, after = m.group(0), _next_code(src, kind, m.end()) # `pub name: Type` is a field. return word in ITEM_KEYWORDS or not (src.startswith(":", after) and not src.startswith("::", after)) + if word == "const" and in_list: + return False # `const N: usize` in a generic parameter list return word in ITEM_KEYWORDS or (after < n and src[after] == "!") # or a macro call -def _item_end(src: str, kind: bytearray, pos: int) -> int: +def _item_end(src: str, kind: bytearray, pos: int, in_list: bool = False) -> int: """End of the item that starts at ``pos``. An item ends at its first top-level ``;``, at the brace closing its first @@ -261,7 +273,7 @@ def _item_end(src: str, kind: bytearray, pos: int) -> int: ends at its first top-level ``,``. """ n = len(src) - is_field = not _starts_item(src, kind, pos) + is_field = not _starts_item(src, kind, pos, in_list) depth, angle, in_expr = 0, 0, False for j in range(pos, n): if kind[j] != CODE: @@ -345,8 +357,10 @@ def count_source(src: str) -> FileCount: excluded = bytearray(b"\x01" * n) break continue - end = _item_end(src, kind, attr_end + 1) start, _ = _attrs_before(src, kind, start) + prev = _prev_code(src, kind, start - 1) + in_list = prev >= 0 and kind[prev] == CODE and src[prev] in "<," + end = _item_end(src, kind, attr_end + 1, in_list) test_mods.extend(_file_mods(src, kind, start, end)) excluded[start:end] = b"\x01" * (end - start) diff --git a/scripts/tests/test_loc_budget.py b/scripts/tests/test_loc_budget.py index d5a676d63..541552e1c 100644 --- a/scripts/tests/test_loc_budget.py +++ b/scripts/tests/test_loc_budget.py @@ -122,6 +122,28 @@ def test_cfg_test_enum_variant_and_match_arm(self): self.assertEqual(fc.code, 9) self.assertEqual(fc.test, 4) + def test_cfg_test_crate_visible_field(self): + src = "struct S {\n #[cfg(test)]\n crate probe: u32,\n real: u32,\n}\nfn shipped() {}\n" + fc = count(src) + self.assertEqual(fc.code, 4) + self.assertEqual(fc.test, 2) + + def test_cfg_test_const_generic_param(self): + src = "struct S<\n #[cfg(test)] const N: usize,\n T,\n> {\n t: T,\n}\nfn shipped() {}\n" + fc = count(src) + self.assertEqual(fc.code, 6) + self.assertEqual(fc.test, 1) + + def test_cfg_test_const_item_is_still_an_item(self): + src = "#[cfg(test)]\nconst N: u8 = 1;\nfn shipped() {}\n" + fc = count(src) + self.assertEqual(fc.code, 1) + self.assertEqual(fc.test, 2) + + def test_crate_path_statement_is_not_a_field(self): + src = "fn f() {\n #[cfg(test)]\n crate::probe();\n real();\n}\n" + self.assertEqual(count(src).code, 3) + def test_cfg_test_fn_param(self): src = "fn f(\n a: u8,\n #[cfg(test)] b: u8,\n) -> u8 {\n a\n}\nfn g() {}\n" self.assertEqual(count(src).code, 6) From d6ccde3e9300b496febe0f22fde447dce5d626ff Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 12:54:39 +0000 Subject: [PATCH 9/9] Name real symbols and paths in the lock-model design doc Follow-up PRs grep for the names this doc cites. OriginPolicy::hosted_uuid exists nowhere in the tree; the function is DiscoverCtx::hosted_uuid (its memoized fast path is the hosted_hosts cell). The npm flavor router lives at vendor/lock_inventory/npm_family.rs, since doc paths are relative to src/. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011f9aeNYq2GHyrsNnPBepcC --- docs/design/lock-model-unification.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/design/lock-model-unification.md b/docs/design/lock-model-unification.md index b14c27ac5..f6d8196f2 100644 --- a/docs/design/lock-model-unification.md +++ b/docs/design/lock-model-unification.md @@ -34,7 +34,7 @@ attestation rules) is real policy and stays. | Second walks of one file | `vendor/npm_origin.rs` walks package-lock `packages` again. Yarn writers call `scan_blocks` directly, and the Berry rewriter uses a third model (`formats/yarn/stanzas.rs`). The vlt writers use a separate line grammar. pnpm's `InstalledPackages` and `vendored_npm_uuids`, and the hosted planner's whole-text `grammar::entries`, skip the env-document filter. | | Second parses of one file | `vex/discover/pypi_locks.rs` parses uv, pylock and script locks itself. The inventory parses them again. `deno.lock` is parsed by `crawlers/deno_crawler.rs` for discovery, the hosted engine and the npm flavor probe. | | Repeated helper parsers | two Python lock listers, two `-r` include walkers, two PEP 508 direct-reference parsers, five walks of `Cargo.toml` dependency tables, two Rush lock listings, four readers of Maven's `checksums.sha256`, two Gradle Socket-index parsers, two routes to sbt resolution evidence | -| Two file-selection layers | the npm flavor router (`lock_inventory/npm_family.rs`) picks one family and one lock. Discovery reads every lock and sweeps the ignored ones with `recognize_ignored`. | +| Two file-selection layers | the npm flavor router (`vendor/lock_inventory/npm_family.rs`) picks one family and one lock. Discovery reads every lock and sweeps the ignored ones with `recognize_ignored`. | The inventory also has gaps. It has no models for NuGet, Maven, Gradle, sbt, Hatch or Deno, and it reads no `composer.lock` that starts with a BOM, which @@ -136,7 +136,7 @@ rewritten, from two places: configured origin, the last canonical-uuid path segment) and `SOCKET_PATCH_SERVER_HOST`. The hosted unwind already calls these directly; -* from discovery: `OriginPolicy::hosted_uuid` (the same rule with a memoized +* from discovery: `DiscoverCtx::hosted_uuid` (the same rule with a memoized fast path for plain registry URLs), `vendor_ref`, `vendor_ref_decorated` and `vendor_uuid_dir`, `locate`, `Wired`, and the `socket-patch-` name grammar.