# AGENTS.md (pacquet) Guidance for AI coding agents working in `pnpm/`. **Read [`../AGENTS.md`](../AGENTS.md) first.** It covers the conventions that apply across the whole monorepo — GitHub PR workflow, signing agent-authored content, conventional commit messages, code-reuse philosophy, "never ignore test failures," and the PR-conflict resolution script. This file specializes those rules for pacquet's Rust code and adds pacquet-only ones. ## What this project is `pacquet` is the [pnpm](https://pnpm.io) CLI implemented in Rust. It is one of two parallel implementations of the same package manager — the other is the TypeScript pnpm CLI (the workspaces outside `pnpm/`). The two are kept behaviorally identical: the same commands, flags, defaults, error codes, file formats, lockfile shape, and directory layout. pacquet is not a downstream port that trails the TypeScript CLI; it is a source of truth in its own right, at near-complete feature parity, and the two stacks are developed together. ## The cardinal rule **pacquet and the TypeScript pnpm CLI must stay behaviorally identical.** They are parallel implementations of one package manager, developed together at near-complete feature parity. Any user-visible change — a command, flag, default, error code or message, lockfile/manifest/state-file format, log emission parsed by `@pnpm/cli.default-reporter`, store layout, or hook semantic — must land in both stacks at the same time. The repo-wide statement of this obligation lives in [`../AGENTS.md`](../AGENTS.md#keep-pnpm-and-pacquet-in-sync); this section is the pacquet-side detail. Neither stack is downstream of the other. You are not "porting from" the TypeScript code: when you implement or change behavior in pacquet, make the equivalent change in the TypeScript workspaces in the same PR, and vice versa. If you genuinely can't (different expertise, scope too large, or the other stack hasn't grown the surrounding feature yet), ship your side and say so in the PR description so the matching commits can follow before it lands. Working rules: 1. **Keep the two implementations in agreement.** When you touch behavior in pacquet, find the counterpart in the TypeScript workspaces — they live at the repo root (`pnpm/` for the CLI entry, `pkg-manager/`, `resolving/`, `lockfile/`, `store/`, `fetching/`, `config/`, `hooks/`, and so on; see the [repo-structure section](../AGENTS.md#repository-structure)) — and change it there too. The two must agree on logic, edge cases, config resolution, error messages, and file/lockfile formats. 2. **Match observable behavior, not structure.** Structural similarity (similar function decomposition and names) is a convenience for cross-referencing, not a requirement. What must match is what a user or a downstream tool can observe. 3. **Don't diverge unilaterally.** Do not add a feature, flag, or quirk to one stack without the other, and do not "fix" a behavior in only one. A genuine bug present in both is fixed in both. 4. **Log emissions are part of behavioral identity.** A function that fires `pnpm:` events through the reporter must use the same call site, payload, and ordering in both stacks so `@pnpm/cli.default-reporter` parses pacquet's NDJSON the same way it parses the TypeScript CLI's. See [Reporter / log events](./CODE_STYLE_GUIDE.md#reporter--log-events) in the style guide for the convention (channel mapping, threading `R: Reporter`, emit-site placement, recording-fake tests). 5. **Prefer real fixtures; reach for the dependency-injection seam only when they can't cover the branch.** Most happy paths and error paths should be tested with a `tempfile::TempDir`, the mocked registry, or an integration test that spawns the actual binary. Use the DI seam — a capability trait on the `Host` provider, threaded as `Sys: ` — only for branches a real fixture can't reach portably: filesystem error kinds (`PermissionDenied`, `ENOSPC`, …), deterministic time, shared process-global state a test would otherwise mutate (`env::set_var`, `set_current_dir`, the umask, …), or the external-service happy paths in features like `pnpm login` (2FA) and `pnpm publish` (OIDC / provenance) when those land. See [Dependency injection for tests](./CODE_STYLE_GUIDE.md#dependency-injection-for-tests) in the style guide for the gating rule, the names (`Sys`, `Host`, `Fs*`, `Clock`, `EnvVar`, …), the eight principles, and the `modules-yaml` worked example. If the intended behavior is unclear or looks wrong, stop and ask the user rather than guessing. ## Modeling branded string types TypeScript pnpm leans on *branded* string types. A branded string is a plain string narrowed by a phantom property (for example, `type PkgName = string & { __brand: 'PkgName' }`), so the type system can track intent that the runtime cannot see. Some brands are stamped through a validating constructor. Others are minted with a bare `as` type assertion and have no runtime check at all. Both stacks must preserve that distinction, because it is part of the public contract pnpm exposes through manifest, lockfile, state, and config files. The TypeScript brand and the Rust newtype must agree on validation policy. Rules for a Rust newtype standing in for a branded string type ("the TypeScript side" below is its TypeScript counterpart): 1. **Declare a newtype wrapper.** Do not collapse the brand into a plain `String` or `&str`. Give the type its own struct so misuse is a type error in pacquet too. 2. **If upstream always validates before construction, validate too.** When every brand site in pnpm runs through a checking factory, pacquet's wrapper must construct only via `TryFrom` and/or `FromStr`. Do not provide an infallible public constructor that takes an arbitrary string. 3. **If upstream never validates, just brand for type-safety.** Some upstream brands exist purely to keep the type system from confusing one string slot with another. For example, a brand may exist to prevent a `PkgId` from being passed where a `PkgName` is expected, even though the value is never validated at runtime. In that case the Rust wrapper should expose an infallible `From` (and `From<&str>` when convenient). The type-safety win is the whole point, and no validator is needed. 4. **If upstream occasionally constructs without validation, expose `from_str_unchecked`.** When pnpm sometimes mints the brand via a bare `as` assertion, skipping its validator, add a `from_str_unchecked` (or similarly named) constructor on the Rust side so callers can opt into the same unchecked path explicitly. Keep the validating constructor as well. `from_str_unchecked` is the escape hatch, not the default. 5. **Match upstream serde behavior.** If the branded type crosses a JSON, YAML, or INI boundary (manifest files, lockfiles, state files, config files, and similar), wire the wrapper into serde so the validation policy survives serialization: - `#[serde(try_from = "String")]` for deserialization, so deserialized values go through the validator. - `#[serde(into = "String")]` for serialization. Use both when the type is round-tripped. 6. **Derive simple conversions with `derive_more`.** When the conversion impls implied by the rules above are mechanical (a one-liner that wraps or unwraps the inner field), use `#[derive(derive_more::From)]` and `#[derive(derive_more::Into)]` rather than handwriting an `impl` block. Fall back to a manual `impl` only when the conversion needs custom logic, such as validation or normalization. `derive_more` is already a workspace dependency. 7. **String-literal unions become `enum`s.** If upstream uses a string literal type or a union of string literals (for example, `'auto' | 'always' | 'never'`), model it as a Rust `enum`, not a newtype wrapper. The set of valid values is closed, so encode that. 8. **Template literal types are branded strings.** If upstream uses a string template literal type (for example, ``` `${string}@${string}` ```), treat it the same as a branded string type. Use a newtype wrapper with the validation discipline from rules 2 through 5 above. ## Follow the project guides 1. Follow the contributing guide in [`CONTRIBUTING.md`](./CONTRIBUTING.md), and **ALWAYS** double-check before committing. It covers commit message format, writing style, setup, and the automated checks to run before committing. 2. Follow the code style guide in [`CODE_STYLE_GUIDE.md`](./CODE_STYLE_GUIDE.md), and **ALWAYS** double-check before committing. It covers code-level conventions not enforced by tooling: imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching, `pipe-trait`, error handling, test layout, logging during tests, and cloning of `Arc` and `Rc`. ## Repo layout (inside `pnpm/`) - `crates/` — library and binary crates that make up pacquet. - `cli`, `package-manager`, `package-manifest`, `lockfile`, `store-dir`, `tarball`, `registry`, `network`, `npmrc`, `fs`, `executor`, `diagnostics`, `testing-utils`. - `tasks/` — developer tooling: `integrated-benchmark`, `micro-benchmark`, `registry-mock`. - `CONTRIBUTING.md` — commit-message format, writing style, setup, and the automated checks to run before submitting. Read it before submitting code. - `CODE_STYLE_GUIDE.md` — manual code-style conventions beyond what `cargo fmt`, `taplo`, and clippy enforce: imports, modules, naming, ownership and borrowing, trait bounds, pattern matching, `pipe-trait`, error handling, test layout, and `Arc`/`Rc` cloning. Read it before submitting code. The Rust workspace (`Cargo.toml`, `Cargo.lock`, `rust-toolchain.toml`, `justfile`, `.cargo/`, `.taplo.toml`, etc.) lives at the **repo root**, not inside `pnpm/`. Run `cargo` and `just` from the repo root. ## Commands Prefer `just` recipes when one fits; drop down to `cargo` / `taplo` / etc. directly when you need flags the recipe doesn't expose (e.g. filtering tests by crate or name — see below). - `just ready` — run the same checks CI runs (typos, fmt, check, test, lint). Run this before declaring a task complete. - `just test` — `cargo nextest run`. - `just lint` — `cargo clippy --locked --workspace --all-targets -- --deny warnings`. - `just check` — `cargo check --locked --workspace --all-targets`. - `just fmt` — `cargo fmt` + `taplo format`. - `just cli -- ` — run the pacquet binary. - `just registry-mock ` — manage the mock registry used by tests. - `just integrated-benchmark ` — compare revisions or compare against pnpm itself (see `CONTRIBUTING.md`). Warnings are errors (`--deny warnings` in lint). Do not silence them with `#[allow(...)]` unless there is a specific, justified reason. ## Tests - Tests live alongside the code they exercise (standard Cargo layout) plus integration tests under each crate's `tests/`. Shared pacquet fixtures live under `crates/testing-utils/src/fixtures/`; registry package fixtures live under `../pnpr/.fixtures/packages/`. - `pacquet-cli`'s end-to-end tests are a single Cargo target: each file under `crates/cli/tests/suite/` is a module of `tests/suite/main.rs`. Put a new test file there and declare it in `main.rs`. A file added directly under `crates/cli/tests/` becomes its own binary instead, and every such binary statically links the whole dependency graph — `pnpr` included — so it costs another ~240 MB to link on each change to the crate. `cargo nextest` runs every test in its own process regardless of how many binaries there are. Paths in file-relative macros (`include_str!`, `include_bytes!`) resolve against the containing file, so one written for `tests/` needs an extra `../` under `tests/suite/`. Watch for this when porting a test from a branch that predates the layout. - Snapshot tests use `insta`. When an intentional change alters a snapshot, review the diff carefully, then accept with `cargo insta review`. Never accept snapshot changes blindly. - Tests that need the mocked registry start `pnpr` through `pacquet-testing-utils`; `cargo test` / `cargo nextest run` should not require a separate `just registry-mock launch` step. - When a behavior change spans both stacks, keep their tests in sync — give pacquet a Rust test for the same scenario the TypeScript stack covers (and vice versa) whenever it translates. Matching test coverage is the easiest way to prove behavioral parity. - The active test-porting plan lives in [`plans/TEST_PORTING.md`](./plans/TEST_PORTING.md). It enumerates the upstream TypeScript tests scheduled to be ported (with file paths and line numbers) and the conventions expected of the ports — `known_failures` modules, `pacquet_testing_utils::allow_known_failure!` at the not-yet-implemented boundary, and the practice of temporarily breaking the subject under test to verify the ported test actually catches the regression. Consult it before adding ported tests, and update its checkboxes as items land. - When temporarily breaking an implementation (to prove a test catches the regression, or for any other experiment), revert with `git restore `, never by moving a saved backup copy into place. Cargo's freshness check is mtime-based, and an mtime-preserving restore leaves the binary compiled from the broken source looking fresh — later test runs then fail in impossible-looking, "flaky" ways with nothing pointing at the stale artifact. Details in the "Test the tests" section of [`plans/TEST_PORTING.md`](./plans/TEST_PORTING.md). If test outcomes ever flip with no code change, `touch` the implementation file and rerun before debugging anything else. ### No "tolerant" tests for missing tools Tests must not be tolerant of a missing build / runtime environment by silently `return`-ing early when a tool isn't found. Patterns like: ```rust fn skip_if_no_git() -> bool { if std::process::Command::new("git").arg("--version").output().is_err() { eprintln!("skipping: `git` not on PATH"); return true; } false } #[test] fn my_test() { if skip_if_no_git() { return; } // ... } ``` are forbidden. If the test needs a tool, just call into it and let the existing `.unwrap()` / `.expect(...)` panic when the tool is absent — a failing test in an under-provisioned environment is the correct signal. Tolerance defeats the purpose of testing: if the environment really doesn't have the required tools, that's the *environment's* fault and it needs to be fixed. This applies in particular to `git`, `node`, and `npm` — git is ubiquitous on developer machines, and Node.js is a documented prerequisite for building pnpm. There is no realistic environment in which pacquet's tests should run *and* these tools should be absent. The only marginally acceptable exception is platform-locked tools — APIs or binaries that exist on one OS but not another. Even then, prefer `#[cfg_attr(target_os = "windows", ignore = "...")]` (or the matching `#[cfg(unix)]` gate already used in this crate for `/bin/sh` shims) over a runtime probe-and-skip helper. The gate is visible to `cargo test` and shows up in the test report; a silent `return` does not. ### Running tests narrowly Running the full suite is slow. While iterating, target what you're working on: ```sh # One crate cargo nextest run -p pacquet-lockfile # One test by name substring cargo nextest run -p pacquet-lockfile # One integration test file cargo nextest run -p pacquet-lockfile --test # One module of pacquet-cli's suite (the suite is a single target, so the # former per-file `--test ` is a module filter here) cargo nextest run -p pacquet-cli -E 'test(/^::/)' ``` Run `just ready` (full suite) before handing the PR off. ## Style `CODE_STYLE_GUIDE.md` is the source of truth. Highlights: - Choose owned vs. borrowed parameters to minimize copies; widen to the most encompassing type (`&Path` over `&PathBuf`, `&str` over `&String`) when it doesn't force extra copies. - Prefer `Arc::clone(&x)` / `Rc::clone(&x)` over `x.clone()` for reference- counted types, so the cost is visible at the call site. - Follow the test-logging guidance in the style guide — log before non- `assert_eq!` assertions, `dbg!` complex structures, skip logging for simple scalar `assert_eq!`. - Follow [Rust API Guidelines](https://rust-lang.github.io/api-guidelines/naming.html) for naming. - **No star imports inside module bodies.** Write `use super::{Foo, bar}` instead of `use super::*;`, and the same for any other glob whose target is a module you control. Two forms stay allowed: external-crate preludes such as `use rayon::prelude::*;` and root-of-module re-exports such as `pub use submodule::*;` in a `lib.rs`. See the "No star imports" section in `CODE_STYLE_GUIDE.md`. ### Comments Same baseline as [`../AGENTS.md`](../AGENTS.md#comments): write code that explains itself; comments are for the non-obvious *why*, not a translation of the *what*. Rust-specific additions: - **Doc comments (`///`, `//!`) are rustdoc-visible API documentation.** Use them for item contracts. Put implementation-only rationale in regular `//` comments. - **Tests are documentation. Do not duplicate them in prose.** If a behavioral scenario, edge case, failure mode, or worked example is already captured by a test (its name, its setup, its assertions), do not also narrate it in the doc comment on the implementation. The doc comment should state the contract once; the test demonstrates the behavior. The same applies in reverse: a test's own doc comment should not re-explain what the asserts already say, only the *why* if it is not obvious. - **`// SAFETY:`, `// TODO:`, and similar prefixes are the exception.** They signal hidden invariants or known follow-ups that a reader cannot recover from the code alone. Prefer renaming, restructuring, or extracting a helper over leaving a comment. Reach for prose only when the right names and types genuinely cannot carry the information. ### Preserve existing method chains When editing existing code, do not break a method chain (including `pipe-trait` `.pipe(...)` chains) into intermediate `let` bindings unless you can justify the rewrite. Valid justifications include a chain that fails to compile after your edit, a borrow checker rejection, a meaningful performance win from splitting it up, or any other concrete reason the chain cannot stay as it is. Refactoring for style alone is not a justification when the task is something else. Keep the surrounding code shape intact and confine your edits to what the task asks for. When the change you need can fit inside the existing chain, keep it there. For example, swapping a `PathBuf::from` allocation for a `Path::new` borrow: ```diff output .stdout .pipe(String::from_utf8) .expect("convert stdout to UTF-8") .trim_end() - .pipe(PathBuf::from) + .pipe(Path::new) .parent() .expect("parent of root manifest") .to_path_buf() ``` Do not flatten the chain just because you happen to be editing nearby: ```diff -output - .stdout - .pipe(String::from_utf8) - .expect("convert stdout to UTF-8") - .trim_end() - .pipe(PathBuf::from) - .parent() - .expect("parent of root manifest") - .to_path_buf() +let stdout = String::from_utf8(output.stdout).expect("convert stdout to UTF-8"); +Path::new(stdout.trim_end()).parent().expect("parent of root manifest").to_path_buf() ``` If you do need to break a chain, state the justification in your reply, the commit message, or the PR description so a reviewer can confirm the rewrite was warranted. If the rewrite is purely stylistic, raise it with the user as its own change rather than including it in an unrelated edit. ## Code reuse (pacquet specifics) The general "search before you write / extract shared code / prefer mature crates / keep deps at the right level" rules from [`../AGENTS.md`](../AGENTS.md#code-reuse-and-avoiding-duplication) apply. Pacquet-specific notes: - Shared helpers tend to live in `crates/fs`, `crates/testing-utils`, and `crates/diagnostics` — check there first. - Check whether the workspace already depends on something suitable (see `[workspace.dependencies]` in the root `Cargo.toml`) before adding a new dependency. - **Keep dependencies at the right level.** Add a new dependency to the specific crate that needs it, not to the workspace root or to a shared crate unless multiple crates actually depend on it. ## Errors and diagnostics User-facing errors go through `miette` via the `pacquet-diagnostics` crate. Match pnpm's error codes and messages where pnpm defines them — error codes are part of the public contract, not implementation detail. See for the canonical list. ## Commit and PR hygiene - Keep commits focused. A bug fix commit should not also refactor or reformat unrelated code. - When a change has a counterpart in the TypeScript pnpm CLI, land both together; if they must be split, cross-reference the matching PR so a reviewer can confirm the two stacks stay in sync. - Run `just ready` before pushing. - The repo-wide husky `pre-push` hook runs `pnpm/scripts/pre-push-rust.sh`, which checks `rustfmt`, `taplo`, `cargo clippy` (with `--all-targets -D warnings`), `cargo doc` (with `RUSTDOCFLAGS=-D warnings`), and `cargo dylint`. Make sure your environment can run cargo (the hook needs it) before pushing; `cargo-dylint` is detected at runtime and skipped with a warning if not installed. ### Commit messages Conventional Commits applies (see [`../AGENTS.md`](../AGENTS.md#commit-messages) for the full type list). Use a scope that names the crate or area being touched, matching the existing history (`git log --oneline` for examples). Pacquet adds one type beyond the standard list: - `bench`: benchmark-only changes. Examples (from this repo's history): ``` fix(network): set explicit timeouts on default reqwest client feat(lockfile): support npm-alias dependencies in snapshots perf(store-dir): share one read-only StoreIndex across cache lookups ``` ## Things not to do - Do not add a feature, flag, or behavior to one stack without making the same change to the other. The two move together. - Do not change lockfile format, store layout, `.npmrc` semantics, or CLI surface in only one stack — those are the shared contract and must change in both at once. - A dependency that is already declared in `[workspace.dependencies]` in the root `Cargo.toml` may be added to any crate that needs it. - Do not add a dependency that is not already declared in the workspace without an explicit human request. If there is a clear benefit and justification for pulling in a new third-party crate, ask the human to approve it and to add it to `[workspace.dependencies]` rather than adding it yourself. Consult `deny.toml` when evaluating candidates. - Do not introduce `unsafe` without a clear justification and review. - Do not disable lints, tests, or CI checks to make a PR green.