feat(pnpr): config-selectable networked-SQLite auth backend (#12199 phase 3)#12206
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
📝 WalkthroughWalkthroughRefactors auth into async trait-based backends, adds a libsql (Turso) backend and config, rewires server auth to async identity resolution with per-package write locks, and updates tests/docs to the new async APIs. ChangesPluggable Async Auth Backend System
Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler as HTTP Handler
participant Resolver as resolve_identity
participant Backend as UserBackend/TokenBackend
participant DB as libsql Database
Client->>Handler: request with Authorization
Handler->>Resolver: resolve_identity(headers, auth_state)
Resolver->>Backend: lookup token / verify basic (await)
Backend-->>Resolver: identity / none
Resolver-->>Handler: Identity
Handler->>Handler: authorize(identity, action, package)
Handler->>Handler: acquire PackageLocks guard (per-package)
Handler->>DB: perform packument read/modify/write (if needed)
Handler-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Micro-Benchmark ResultsLinux |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12206 +/- ##
==========================================
- Coverage 87.94% 87.88% -0.07%
==========================================
Files 277 278 +1
Lines 32014 32075 +61
==========================================
+ Hits 28156 28188 +32
- Misses 3858 3887 +29 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Review Summary by QodoConfig-selectable networked-SQLite auth backend with embedded replica support
WalkthroughsDescription• Pluggable auth backends (users + tokens) via trait objects - Local htpasswd/SQLite (default), in-memory, or networked libsql/Turso - Config-selectable via new backend.libsql: YAML block • Embedded replica support for libsql backend - Local read cache with configurable sync interval bounds token-revocation lag • Async auth access path with split identity resolution - identify now async; enforce_access split into async resolve_identity + sync authorize - Search endpoint resolves caller once, authorizes each result synchronously • Per-package serialization guard prevents lost-update race - Striped mutex locks serialize same-package writers on one instance - Cross-replica S3 ETag CAS documented as follow-up work Diagramflowchart LR
Config["Config: backend.libsql block"]
AuthState["AuthState::load async"]
LocalBackend["Local: UserStore + TokenStore"]
LibsqlBackend["LibsqlAuth: shared networked DB"]
Replica["Embedded replica: local read cache"]
Config -->|Libsql settings| AuthState
AuthState -->|Local| LocalBackend
AuthState -->|Libsql| LibsqlBackend
LibsqlBackend -->|replicaPath set| Replica
Identify["identify async"]
ResolveIdentity["resolve_identity async"]
Authorize["authorize sync"]
Identify -->|split| ResolveIdentity
ResolveIdentity -->|then| Authorize
PackageLocks["PackageLocks: striped mutexes"]
Publish["publish read-modify-write"]
DistTag["dist-tag change read-modify-write"]
Unpublish["partial-unpublish read-modify-write"]
PackageLocks -->|guards| Publish
PackageLocks -->|guards| DistTag
PackageLocks -->|guards| Unpublish
File Changes1. pnpr/crates/pnpr/src/auth.rs
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pnpr/crates/pnpr/src/auth/libsql_backend.rs`:
- Around line 190-220: The current libsql helper methods lookup, find_by_key,
and list_for_user swallow query errors and return None/Vec::new(), causing
outages to appear as 401/404/200; change their signatures to return
Result<Option<String>, RegistryError> (for lookup), Result<Option<TokenRecord>,
RegistryError> (for find_by_key) and Result<Vec<(String, TokenRecord)>,
RegistryError> (for list_for_user), remove the .ok()?/.await.ok()? error
swallowing and instead map libsql errors into RegistryError::Libsql (preserving
the source error), and update call sites that use find_by_key (the usage around
lines 223-227) to propagate the Result and convert failures into HTTP 5xx via
RegistryError::Libsql rather than turning them into not-found/empty successes.
Ensure row parsing errors are also propagated as RegistryError::Libsql.
In `@pnpr/crates/pnpr/src/config.rs`:
- Around line 628-631: The LibsqlSettings passed into BackendConfig::Libsql
currently leaves replicaPath as-is so relative paths are resolved against the
process CWD; update the match that constructs BackendConfig::Libsql to
canonicalize and resolve LibsqlSettings.replicaPath against the directory of the
parsed config file (the same logic used for storage/cache/auth.*.file): if
replicaPath is relative, join it with the config file's parent directory and
normalize (using Path/PathBuf operations) and then construct
BackendConfig::Libsql with the adjusted LibsqlSettings; reference the
BackendConfig::Libsql variant, the LibsqlSettings struct and its replicaPath
field when making this change.
In `@pnpr/crates/pnpr/src/server.rs`:
- Around line 232-236: The serve function loads backend-aware AuthState via
AuthState::load and then builds its router with router_with_auth(config, auth),
but serve_listener still constructs an in-memory-only router via router(config),
causing inconsistent auth behavior; update serve_listener to accept (or load)
the same backend-aware AuthState and replace any router(config) calls with
router_with_auth(config, auth) (or change its signature to take AuthState and
call router_with_auth inside), and update all call sites to pass the AuthState
so both serve and serve_listener use the same backend-backed auth initialization
(refer to serve, serve_listener, router_with_auth, router, and AuthState::load).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0198cadb-0de3-4e1f-bba5-73d1146e89ad
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
Cargo.tomlcspell.jsonpnpr/crates/pnpr/Cargo.tomlpnpr/crates/pnpr/config.yamlpnpr/crates/pnpr/src/auth.rspnpr/crates/pnpr/src/auth/libsql_backend.rspnpr/crates/pnpr/src/auth/libsql_backend/tests.rspnpr/crates/pnpr/src/auth/tests.rspnpr/crates/pnpr/src/config.rspnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/src/error.rspnpr/crates/pnpr/src/lib.rspnpr/crates/pnpr/src/server.rspnpr/crates/pnpr/tests/auth_persistence.rspnpr/crates/pnpr/tests/auth_publish.rspnpr/crates/pnpr/tests/auth_user_endpoints.rspnpr/npm/pnpr/README.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ubuntu-latest / Node.js 24 / Test
- GitHub Check: Run benchmark on ubuntu-latest
🧰 Additional context used
📓 Path-based instructions (2)
pnpr/**/pnpr/**/Cargo.toml
📄 CodeRabbit inference engine (pnpr/AGENTS.md)
Declare new shared dependencies in the root [workspace.dependencies] and use { workspace = true } in pnpr crate's Cargo.toml
Files:
pnpr/crates/pnpr/Cargo.toml
pnpr/**/pnpr/**/*.rs
📄 CodeRabbit inference engine (pnpr/AGENTS.md)
pnpr/**/pnpr/**/*.rs: Follow the pacquet code-style guide (../pacquet/CODE_STYLE_GUIDE.md) for Rust-level conventions including imports, naming, ownership, and error handling
Follow the pacquet contributing guide (../pacquet/CONTRIBUTING.md) for test layout and Rust conventions
Files:
pnpr/crates/pnpr/src/error.rspnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/tests/auth_publish.rspnpr/crates/pnpr/src/auth/libsql_backend/tests.rspnpr/crates/pnpr/tests/auth_user_endpoints.rspnpr/crates/pnpr/tests/auth_persistence.rspnpr/crates/pnpr/src/lib.rspnpr/crates/pnpr/src/auth/tests.rspnpr/crates/pnpr/src/auth/libsql_backend.rspnpr/crates/pnpr/src/config.rspnpr/crates/pnpr/src/server.rspnpr/crates/pnpr/src/auth.rs
🧠 Learnings (34)
📓 Common learnings
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12189
File: pacquet/crates/cli/src/cli_args/install.rs:0-0
Timestamp: 2026-06-04T14:55:48.516Z
Learning: In `pacquet/crates/cli/src/cli_args/install.rs` (pnpm/pnpm repo), the `install_via_pnpr` function intentionally forwards the **full** `state.config.auth_headers` map to the pnpr server (not filtered to only the declared default/named registries). This is required for correctness: transitive dependencies can be scope-routed to registries not in the explicit registry list, or pinned to tarball URLs on hosts present in `.npmrc` but not a declared registry. Filtering to the declared registries silently drops tokens those sub-dependencies need, causing 401s on the server. pnpr uses the forwarded map to attach the right token per fetched URL exactly as a local install does (`AuthHeaders::for_url`). The pnpr-server's own credential is sent separately in the `Authorization` header and is excluded from the body map. Do NOT flag this as a credential-leakage issue — the rationale is documented in a comment at both call sites.
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Do not add a dependency not already declared in `[workspace.dependencies]` without explicit human request; ask for approval and request addition to the workspace root `Cargo.toml`
Applied to files:
Cargo.tomlpnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/**/Cargo.toml : Declare new shared dependencies in the root [workspace.dependencies] and use { workspace = true } in pnpr crate's Cargo.toml
Applied to files:
Cargo.tomlpnpr/crates/pnpr/Cargo.tomlpnpr/crates/pnpr/src/lib.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/Cargo.toml : Keep dependencies at the right level — add a new dependency to the specific crate that needs it, not to the workspace root or shared crate unless multiple crates depend on it
Applied to files:
Cargo.tomlpnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/Cargo.toml : Check whether the workspace already depends on something suitable in `[workspace.dependencies]` in the root `Cargo.toml` before adding a new dependency
Applied to files:
Cargo.tomlpnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/crates/**/Cargo.toml : Cargo.toml files for new registry-only crates must use the pnpr- prefix for the package name
Applied to files:
pnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/crates/**/Cargo.toml : New registry-only crates must be placed under pnpr/crates/<short-name>/ and named pnpr-<short-name> in Cargo.toml, never using the pacquet- prefix
Applied to files:
pnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-19T19:23:00.981Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11752
File: pacquet/crates/config/src/lib.rs:1062-1073
Timestamp: 2026-05-19T19:23:00.981Z
Learning: In `pacquet/crates/config/src/lib.rs`, `modules_dir` does not need a `!virtual_store_dir_explicit` guard on its workspace re-anchor because `modules_dir` is in pnpm's `excludedPnpmKeys` (filtered out by `WorkspaceSettings::clear_workspace_only_fields`) and therefore can only be set by workspace yaml (applied immediately after) or env vars (applied later in the cascade) — not by global `config.yaml`. `virtual_store_dir`, by contrast, IS settable from global config and requires the `if !virtual_store_dir_explicit` guard to survive the workspace-root re-anchor.
Applied to files:
pnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Prefer existing pacquet-* crates over writing new code; check pacquet-tarball, pacquet-crypto-hash, pacquet-crypto-shasums-file, pacquet-package-manifest, pacquet-network, pacquet-registry, pacquet-fs, and pacquet-diagnostics before implementing non-trivial functionality
Applied to files:
pnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-21T00:33:05.035Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11789
File: pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs:145-146
Timestamp: 2026-05-21T00:33:05.035Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs`, the guard `bare.starts_with("workspace:.")` is intentionally broad — matching pnpm upstream's identical `startsWith('workspace:.')` check. All dot-prefixed workspace forms including `workspace:.foo` are intentionally handed off to the local-resolver, which handles them as `link:`-style directory specs via its prefix-stripping regex. Do not suggest narrowing this to `workspace:./` or `workspace:../` only.
Applied to files:
pnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-20T22:49:17.652Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11787
File: pacquet/crates/catalogs-resolver/src/lib.rs:156-167
Timestamp: 2026-05-20T22:49:17.652Z
Learning: In pacquet's `catalogs-resolver` crate (`pacquet/crates/catalogs-resolver/src/lib.rs`), the protocol detection pattern `catalog_lookup.split(':').next().unwrap_or("")` is an intentional byte-for-byte port of pnpm's upstream JavaScript `getProtocol`/split logic in `catalogs/resolver/src/resolveFromCatalog.ts#L95`. A bare value like `"workspace"` (without a colon) is deliberately classified as the `"workspace"` protocol, matching upstream behavior. pacquet's cardinal rule is to match upstream pnpm behavior including quirks; any behavioral change must land in pnpm first and then be ported here.
Applied to files:
pnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-21T00:33:05.035Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11789
File: pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs:145-146
Timestamp: 2026-05-21T00:33:05.035Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/resolve_from_workspace.rs`, the guard `bare.starts_with("workspace:.")` is intentionally broad — matching pnpm upstream's identical `startsWith('workspace:.')` check at `resolving/npm-resolver/src/index.ts`. All dot-prefixed workspace forms including `workspace:.foo` are intentionally passed through to the local-resolver, which handles them as `link:`-style directory specs via its prefix-stripping regex. Do not suggest narrowing this to `workspace:./` or `workspace:../` only.
Applied to files:
pnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-06-02T14:39:24.423Z
Learnt from: KSXGitHub
Repo: pnpm/pnpm PR: 11938
File: pacquet/crates/config/src/lib.rs:959-966
Timestamp: 2026-06-02T14:39:24.423Z
Learning: In the pnpm/pnpm pacquet Rust port (`pacquet/crates/config/src/lib.rs`), `Config.extra_bin_paths` is intentionally left empty (`Vec::new()`) until workspace fan-out support for `run`/`exec` is implemented. It mirrors pnpm's `Config.extraBinPaths` (the workspace-root `node_modules/.bin`), which is also empty outside a workspace. Populating it before the workspace-root is resolved would put a non-existent path on `PATH`, so it should only be derived once workspace support lands. Do not flag this as a bug or missing derivation in reviews.
Applied to files:
pnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-24T21:11:04.272Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11915
File: pacquet/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs:553-617
Timestamp: 2026-05-24T21:11:04.272Z
Learning: In the pacquet Rust port (pnpm/pnpm repo), the `ResolvedPackage.optional` AND-folding on revisit intentionally mirrors pnpm's `resolveDependencies.ts:1627-1648` behavior: only the directly-revisited package's `optional` flag is updated; transitive descendants are not re-walked. pnpm CLI corrects stale optional flags downstream via `copyDependencySubGraph` BFS in `lockfile/pruner/src/index.ts:160-205`, which tracks a `nonOptional` set and re-stamps any package reachable by an all-non-optional path. Pacquet does not yet have this pruner equivalent, so the stale flags flow directly through `dependencies_graph_to_lockfile.rs:409` → `create_virtual_store.rs:762` → `installability.rs:394`. A follow-up to port `copyDependencySubGraph` is planned; until then, do not flag the resolver-layer optional propagation gap as a bug in pacquet PRs — it is intentional parity with pnpm's resolver layer.
Applied to files:
pnpr/crates/pnpr/Cargo.toml
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Prefer real fixtures over dependency-injection seams — use `tempfile::TempDir`, the mocked registry, or integration tests spawning the actual binary for happy paths and most error paths; use the DI seam only for filesystem error kinds, deterministic time, shared process-global state, or external-service happy paths
Applied to files:
pnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/src/auth/libsql_backend/tests.rspnpr/crates/pnpr/src/auth/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Prefer `#[cfg_attr(target_os = "windows", ignore = "...")]` (or matching `#[cfg(unix)]` gates) over runtime probe-and-skip helpers for platform-locked tools
Applied to files:
pnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/src/auth/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/*.rs : Tests are documentation — do not duplicate test scenarios, edge cases, failure modes, or worked examples in prose when they are already captured by tests
Applied to files:
pnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/tests/auth_publish.rspnpr/crates/pnpr/src/auth/libsql_backend/tests.rspnpr/crates/pnpr/src/auth/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Follow test-logging guidance — log before non-`assert_eq!` assertions, `dbg!` complex structures, skip logging for simple scalar `assert_eq!`
Applied to files:
pnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/src/auth/libsql_backend/tests.rspnpr/crates/pnpr/src/auth/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Tests that need the mocked registry should start `pnpr` through `pacquet-testing-utils`; `cargo test` / `cargo nextest run` should not require a separate `just registry-mock launch` step
Applied to files:
pnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/tests/auth_publish.rspnpr/crates/pnpr/src/auth/libsql_backend/tests.rspnpr/crates/pnpr/tests/auth_persistence.rspnpr/crates/pnpr/src/auth/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Consult `plans/TEST_PORTING.md` before adding ported tests and update its checkboxes as items land; use `known_failures` modules and `pacquet_testing_utils::allow_known_failure!` at the not-yet-implemented boundary
Applied to files:
pnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/src/auth/libsql_backend/tests.rspnpr/crates/pnpr/src/auth/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Port relevant pnpm tests to Rust tests whenever they translate when porting behavior from pnpm
Applied to files:
pnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/tests/auth_publish.rspnpr/crates/pnpr/src/auth/libsql_backend/tests.rspnpr/crates/pnpr/src/auth/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/tests/**/*.rs : Use snapshot tests with `insta` and carefully review diffs when intentional changes alter snapshots; accept with `cargo insta review` only after careful review
Applied to files:
pnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/tests/auth_publish.rspnpr/crates/pnpr/src/auth/libsql_backend/tests.rspnpr/crates/pnpr/tests/auth_persistence.rspnpr/crates/pnpr/src/auth/tests.rs
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/**/*.rs : Follow the pacquet contributing guide (../pacquet/CONTRIBUTING.md) for test layout and Rust conventions
Applied to files:
pnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/tests/auth_publish.rspnpr/crates/pnpr/src/auth/libsql_backend/tests.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/*.rs : Do not use star imports inside module bodies — write `use super::{Foo, bar}` instead of `use super::*;` for modules you control
Applied to files:
pnpr/crates/pnpr/src/config/tests.rspnpr/crates/pnpr/src/lib.rs
📚 Learning: 2026-05-20T19:41:03.063Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:41:03.063Z
Learning: In the pacquet project (Rust), the semver crate used is `node-semver` (version ~2.2.0), NOT `nodejs-semver`. These are two distinct crates. `node-semver` only exposes `Range::satisfies` as its public satisfaction API — there is no separate `satisfies_with_prerelease` method. Prerelease-tolerant matching is handled inline by retrying against the stripped `MAJOR.MINOR.PATCH` base when `Range::satisfies` rejects a prerelease version.
Applied to files:
pnpr/crates/pnpr/tests/auth_publish.rs
📚 Learning: 2026-05-25T14:58:11.105Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11931
File: pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs:560-589
Timestamp: 2026-05-25T14:58:11.105Z
Learning: In `pacquet/crates/resolving-npm-resolver/src/create_npm_resolution_verifier.rs`, all per-`(registry, name[, version])` caches in `NpmResolutionVerifier` (`published_at`, `full_meta`, `full_meta_for_trust`, `abbreviated_meta`, `local_meta`) intentionally use the same pattern: lock → miss-check → release lock → await fetch/load → re-acquire lock → insert. This uniform pattern is deliberate; do not flag individual caches for using it. The known follow-up improvement (replacing the pattern with `tokio::sync::OnceCell` per key inside a `Mutex<HashMap<…>>`) is tracked as a future structural change to cover all five caches simultaneously.
Applied to files:
pnpr/crates/pnpr/tests/auth_publish.rspnpr/crates/pnpr/src/server.rspnpr/crates/pnpr/src/auth.rs
📚 Learning: 2026-05-23T16:55:36.507Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11878
File: pacquet/crates/cli/tests/lockfile_verification.rs:158-162
Timestamp: 2026-05-23T16:55:36.507Z
Learning: In `pacquet/crates/cli/tests/lockfile_verification.rs`, the `trust_lockfile_skips_verification` and `trust_lockfile_cli_flag_skips_verification` tests intentionally do NOT assert `output.status.success()`. The hand-rolled fixture lockfile uses a placeholder integrity hash (`sha512-AAA…`), so the install always fails the downstream tarball integrity check regardless of the supply-chain gate. The contract being tested is "gate-skipped, not install-succeeded"; asserting success would require generating a real lockfile via the `generate_lockfile` pattern (see `hoist.rs`) which is considered not worth the extra wiring for an opt-out smoke test.
Applied to files:
pnpr/crates/pnpr/tests/auth_publish.rspnpr/crates/pnpr/src/auth/tests.rs
📚 Learning: 2026-05-25T12:36:42.202Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T12:36:42.202Z
Learning: User-visible changes (CLI flags, defaults, environment variables, lockfile/manifest/state-file formats, error codes/messages, log emissions, store layout, hook semantics) in pnpm must be mirrored to pacquet in the same PR
Applied to files:
pnpr/npm/pnpr/README.mdpnpr/crates/pnpr/src/lib.rs
📚 Learning: 2026-05-29T18:03:24.797Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pnpr/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:24.797Z
Learning: Applies to pnpr/**/pnpr/**/*.rs : Follow the pacquet code-style guide (../pacquet/CODE_STYLE_GUIDE.md) for Rust-level conventions including imports, naming, ownership, and error handling
Applied to files:
pnpr/crates/pnpr/src/lib.rspnpr/crates/pnpr/src/auth/tests.rspnpr/crates/pnpr/src/server.rs
📚 Learning: 2026-05-29T18:03:15.372Z
Learnt from: CR
Repo: pnpm/pnpm PR: 0
File: pacquet/AGENTS.md:0-0
Timestamp: 2026-05-29T18:03:15.372Z
Learning: Applies to pacquet/**/*.rs : External-crate preludes such as `use rayon::prelude::*;` and root-of-module re-exports such as `pub use submodule::*;` in a `lib.rs` are allowed exceptions to the no-star-imports rule
Applied to files:
pnpr/crates/pnpr/src/lib.rs
📚 Learning: 2026-06-04T14:55:48.516Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12189
File: pacquet/crates/cli/src/cli_args/install.rs:0-0
Timestamp: 2026-06-04T14:55:48.516Z
Learning: In `pacquet/crates/cli/src/cli_args/install.rs` (pnpm/pnpm repo), the `install_via_pnpr` function intentionally forwards the **full** `state.config.auth_headers` map to the pnpr server (not filtered to only the declared default/named registries). This is required for correctness: transitive dependencies can be scope-routed to registries not in the explicit registry list, or pinned to tarball URLs on hosts present in `.npmrc` but not a declared registry. Filtering to the declared registries silently drops tokens those sub-dependencies need, causing 401s on the server. pnpr uses the forwarded map to attach the right token per fetched URL exactly as a local install does (`AuthHeaders::for_url`). The pnpr-server's own credential is sent separately in the `Authorization` header and is excluded from the body map. Do NOT flag this as a credential-leakage issue — the rationale is documented in a comment at both call sites.
Applied to files:
pnpr/crates/pnpr/src/lib.rspnpr/crates/pnpr/src/server.rs
📚 Learning: 2026-06-04T14:40:25.306Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12189
File: pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs:435-439
Timestamp: 2026-06-04T14:40:25.306Z
Learning: In `pacquet/crates/package-manager/src/install_with_fresh_lockfile.rs` (pnpm/pnpm repo), the pnpr install accelerator always invokes `Install` with `lockfile_only: true` (hard-coded in `pnpr/crates/pnpr/src/install_accelerator/resolve.rs`). Under `lockfile_only: true`:
1. The `PrefetchingResolver` wrapper is skipped — the bare `inner_resolver` is used instead, so `PrefetchContext { config }` is never constructed.
2. The function returns before `CreateVirtualStore` is reached, so `install_package_by_snapshot` and its `config.auth_headers` fetch path are never hit.
pnpr's tarball fetch is handled separately in `resolve::fetch_uncached`, which independently receives the request-scoped `auth_headers`. Therefore, `auth_override` only needs to be threaded into the resolver-side components (NpmResolver, TarballResolver, NamedRegistryResolver) — not into PrefetchingResolver or CreateVirtualStore — on the pnpr path. For local installs (`lockfile_only: false`), `auth_override` is always `None` a...
Applied to files:
pnpr/crates/pnpr/src/server.rs
📚 Learning: 2026-06-04T20:24:28.672Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 12198
File: pnpr/crates/pnpr/src/storage.rs:469-477
Timestamp: 2026-06-04T20:24:28.672Z
Learning: In `pnpr/crates/pnpr/src/storage.rs` (pnpm/pnpm repo, Rust), `Store::list_package_names` intentionally uses `fs::try_exists(...).await.unwrap_or(false)` and `if let Ok(mut inner) = fs::read_dir(...)` — NOT `?`-propagation — for per-entry checks. This is deliberate best-effort / verdaccio-style search behavior: (1) `try_exists(stray_file/package.json)` returns `ENOTDIR` (not `NotFound`) for a stray non-package file in the store root, so `?` would fail the entire search; (2) the `@`-scope `read_dir` would fail on a non-directory `@`-named entry; (3) switching to `DirEntry::file_type()` would stop following symlinked package dirs. Failures that DO propagate are preserved: opening the store root itself, and `next_entry()` during the walk. Do not suggest blanket `?`-propagation for these per-entry checks.
Applied to files:
pnpr/crates/pnpr/src/server.rs
📚 Learning: 2026-05-20T20:41:50.322Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11773
File: pacquet/crates/package-manager/src/install_package_from_registry.rs:111-114
Timestamp: 2026-05-20T20:41:50.322Z
Learning: In pacquet (pnpm/pnpm repo, Rust codebase), `install_package_from_registry` is the npm-only install path. The npm resolver always stamps `ResolveResult.id` (a `PkgResolutionId`) as `nameversion`. Parsing it back through `PkgNameVer` with `.expect()` is intentional — a parse failure means a mis-dispatch bug, not malformed external input. Per pacquet's CLAUDE.md: "Don't add error handling, fallbacks, or validation for scenarios that can't happen. Trust internal code and framework guarantees." Do not suggest replacing such `.expect()` calls with graceful error handling.
Applied to files:
pnpr/crates/pnpr/src/server.rs
🔇 Additional comments (17)
Cargo.toml (1)
83-83: LGTM!Also applies to: 108-108
pnpr/crates/pnpr/Cargo.toml (1)
35-35: LGTM!Also applies to: 48-48
pnpr/crates/pnpr/tests/auth_user_endpoints.rs (1)
412-412: LGTM!Also applies to: 422-422
pnpr/crates/pnpr/src/server.rs (1)
67-70: LGTM!Also applies to: 76-117, 151-151, 326-326, 382-382, 385-385, 550-550, 576-576, 612-612, 703-705, 717-718, 742-744, 753-753, 766-767, 771-771, 792-793, 795-795, 827-827, 830-831, 833-833, 873-873, 926-931, 1075-1079, 1086-1086, 1163-1163, 1179-1182, 1202-1202, 1236-1236, 1258-1258, 1330-1330, 1334-1337, 1436-1451, 1453-1459, 1461-1461, 1470-1470, 1480-1480, 1487-1497, 1652-1652
pnpr/crates/pnpr/tests/auth_publish.rs (1)
1162-1201: LGTM!pnpr/crates/pnpr/src/config.rs (1)
110-115: LGTM!Also applies to: 130-178, 381-385, 435-439, 491-492, 511-512, 652-653
pnpr/crates/pnpr/src/config/tests.rs (1)
2-3: LGTM!Also applies to: 166-237
pnpr/crates/pnpr/config.yaml (1)
26-39: LGTM!pnpr/crates/pnpr/src/error.rs (1)
124-127: LGTM!Also applies to: 186-186
pnpr/crates/pnpr/src/auth/libsql_backend.rs (1)
1-189: LGTM!Also applies to: 231-262
pnpr/crates/pnpr/src/auth/libsql_backend/tests.rs (1)
1-73: LGTM!pnpr/npm/pnpr/README.md (1)
184-239: LGTM!cspell.json (1)
135-135: LGTM!Also applies to: 163-163, 332-332, 369-369
pnpr/crates/pnpr/src/auth.rs (1)
3-27: LGTM!Also applies to: 29-35, 41-57, 59-63, 66-98, 101-149, 206-213, 215-216, 223-285, 356-360, 387-400, 414-437, 460-477, 612-637
pnpr/crates/pnpr/src/lib.rs (1)
24-31: LGTM!pnpr/crates/pnpr/src/auth/tests.rs (1)
1-3: LGTM!Also applies to: 31-33, 88-91, 107-107, 172-173, 195-195, 227-227, 231-231, 237-240, 256-256, 264-264
pnpr/crates/pnpr/tests/auth_persistence.rs (1)
76-76: LGTM!Also applies to: 97-97, 147-149, 176-176, 228-228
Integrated-Benchmark Report (Linux)Each scenario has pacquet rows (direct install) and pnpr rows (the same client through the pnpr install accelerator), so pnpr@HEAD vs pacquet@HEAD is the pnpr-vs-direct ratio. Cold-store scenarios wipe the client store between runs (warm server); hot-store scenarios keep it warm. The pacquet@HEAD rows feed the pacquet Bencher testbed; the pnpr@HEAD rows feed the pnpr testbed. Scenario: Isolated linker: fresh restore, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 4.826881417360001,
"stddev": 0.09473217778817193,
"median": 4.809941929360001,
"user": 2.4345114399999996,
"system": 3.7641434000000005,
"min": 4.68543435536,
"max": 5.02599038836,
"times": [
4.813744979360001,
4.7650554323600005,
5.02599038836,
4.819826084360001,
4.79437074336,
4.68543435536,
4.937975053360001,
4.806138879360001,
4.846756191360001,
4.77352206636
]
},
{
"command": "pacquet@main",
"mean": 4.76253858836,
"stddev": 0.017176149893778248,
"median": 4.76041817786,
"user": 2.42151154,
"system": 3.7743607,
"min": 4.739355269360001,
"max": 4.79484085036,
"times": [
4.76233877536,
4.77987647536,
4.768383770360001,
4.7531383513600005,
4.74640630836,
4.75849758036,
4.79484085036,
4.7476483833600005,
4.739355269360001,
4.774900119360001
]
},
{
"command": "pnpr@HEAD",
"mean": 2.04963351496,
"stddev": 0.0912948223238211,
"median": 2.03233145786,
"user": 2.5777410400000003,
"system": 3.3103119999999997,
"min": 1.95355511636,
"max": 2.22336241336,
"times": [
2.18696535536,
2.22336241336,
2.03522811336,
1.95355511636,
1.97573671336,
2.08632736036,
2.00553953836,
2.03479916136,
2.02986375436,
1.9649576233600001
]
},
{
"command": "pnpr@main",
"mean": 2.04138319686,
"stddev": 0.06397278321446219,
"median": 2.04318055386,
"user": 2.5658099400000003,
"system": 3.3541076,
"min": 1.95729411136,
"max": 2.17151638136,
"times": [
1.95729411136,
2.10304568536,
2.04832233336,
2.17151638136,
2.05972307536,
1.98944005036,
2.02994723636,
1.9657970093600001,
2.05070731136,
2.03803877436
]
}
]
}Scenario: Isolated linker: fresh restore, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 0.6440456222400001,
"stddev": 0.015889002748706287,
"median": 0.6410521918400001,
"user": 0.35311269999999995,
"system": 1.3120192000000002,
"min": 0.62492685684,
"max": 0.68171191484,
"times": [
0.68171191484,
0.64089473584,
0.64120964784,
0.64357927784,
0.6464346068400001,
0.6574164488400001,
0.6365107818400001,
0.62492685684,
0.63720121884,
0.6305707328400001
]
},
{
"command": "pacquet@main",
"mean": 0.66240717174,
"stddev": 0.05625219694219729,
"median": 0.64198050734,
"user": 0.35458809999999996,
"system": 1.3028089999999999,
"min": 0.63037698284,
"max": 0.81134953884,
"times": [
0.7022638038400001,
0.64101094084,
0.63480378784,
0.64295007384,
0.64638407684,
0.6334725548400001,
0.6358107848400001,
0.64564917284,
0.63037698284,
0.81134953884
]
},
{
"command": "pnpr@HEAD",
"mean": 0.6569532387400001,
"stddev": 0.04029515230349852,
"median": 0.6384833298400001,
"user": 0.3576898,
"system": 1.3064627999999998,
"min": 0.6270227018400001,
"max": 0.74116855384,
"times": [
0.7191463798400001,
0.66060208284,
0.6518597568400001,
0.6270227018400001,
0.63772897784,
0.6392376818400001,
0.63175428584,
0.6310881808400001,
0.6299237858400001,
0.74116855384
]
},
{
"command": "pnpr@main",
"mean": 0.73042482314,
"stddev": 0.08294190876734191,
"median": 0.70112001384,
"user": 0.3509884,
"system": 1.3266475999999998,
"min": 0.64850584584,
"max": 0.88240819784,
"times": [
0.7493249108400001,
0.88240819784,
0.64850584584,
0.6882367678400001,
0.6949818228400001,
0.70084293084,
0.7013970968400001,
0.7133109658400001,
0.65165204684,
0.87358764584
]
}
]
}Scenario: Isolated linker: fresh install, cold cache + cold store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 2.12860122906,
"stddev": 0.056565128804019364,
"median": 2.12524631416,
"user": 3.4018621599999994,
"system": 2.9790390399999995,
"min": 2.06157880716,
"max": 2.23537366916,
"times": [
2.06157880716,
2.16432669116,
2.07126064316,
2.17423971716,
2.16390376816,
2.06410847016,
2.1201461511599997,
2.13034647716,
2.23537366916,
2.10072789616
]
},
{
"command": "pacquet@main",
"mean": 2.1438451346600003,
"stddev": 0.024729793599264225,
"median": 2.14888038666,
"user": 3.4314978600000003,
"system": 2.99611084,
"min": 2.10188727816,
"max": 2.16858953216,
"times": [
2.16382950616,
2.13672856116,
2.10188727816,
2.1337186841599998,
2.15804578316,
2.10570119916,
2.13971499016,
2.16503535416,
2.16858953216,
2.1652004581599997
]
},
{
"command": "pnpr@HEAD",
"mean": 2.12656720576,
"stddev": 0.018865566455748124,
"median": 2.12434319016,
"user": 3.36341106,
"system": 3.0109477399999998,
"min": 2.10143205916,
"max": 2.16675201716,
"times": [
2.16675201716,
2.1193627291599997,
2.1102251061599997,
2.11382067716,
2.12439514016,
2.13259842716,
2.12554346516,
2.12429124016,
2.10143205916,
2.14725119616
]
},
{
"command": "pnpr@main",
"mean": 2.1430386706599998,
"stddev": 0.03462054051416306,
"median": 2.16046140316,
"user": 3.45205676,
"system": 3.0072986399999997,
"min": 2.07899992116,
"max": 2.18321455216,
"times": [
2.16473848816,
2.17111261316,
2.1181810781599997,
2.18321455216,
2.07899992116,
2.16883964816,
2.1616544261599997,
2.15926838016,
2.11503273716,
2.10934486216
]
}
]
}Scenario: Isolated linker: fresh install, hot cache + hot store
BENCHMARK_REPORT.json{
"results": [
{
"command": "pacquet@HEAD",
"mean": 1.2436758810800002,
"stddev": 0.013296156759287775,
"median": 1.24131898638,
"user": 1.35697714,
"system": 1.6889414799999998,
"min": 1.2296633618800001,
"max": 1.27372831988,
"times": [
1.2387346888800002,
1.2322205208800001,
1.2407226528800002,
1.2419153198800001,
1.2509434078800001,
1.2514815648800002,
1.27372831988,
1.24762867488,
1.2296633618800001,
1.2297202988800002
]
},
{
"command": "pacquet@main",
"mean": 1.2841890362800001,
"stddev": 0.08443455562888985,
"median": 1.2561541313800002,
"user": 1.3450378399999998,
"system": 1.70650218,
"min": 1.22716111988,
"max": 1.5005812348800003,
"times": [
1.2586152968800002,
1.2399662658800001,
1.23473194288,
1.22716111988,
1.5005812348800003,
1.3578260338800001,
1.2415997808800001,
1.2681199818800002,
1.2595957398800002,
1.25369296588
]
},
{
"command": "pnpr@HEAD",
"mean": 1.2650196404800003,
"stddev": 0.061091192213525734,
"median": 1.25102472788,
"user": 1.35792544,
"system": 1.6966857799999997,
"min": 1.22491762788,
"max": 1.43437701188,
"times": [
1.2339650078800002,
1.25166527388,
1.22491762788,
1.2503841818800001,
1.43437701188,
1.2529208788800001,
1.2647692238800001,
1.2321235408800002,
1.2667133938800001,
1.2383602638800002
]
},
{
"command": "pnpr@main",
"mean": 1.28861189568,
"stddev": 0.06849838854360879,
"median": 1.2768529558800001,
"user": 1.3547317399999999,
"system": 1.7075459800000001,
"min": 1.23156470188,
"max": 1.46816008988,
"times": [
1.2878603438800003,
1.31341355988,
1.2677116428800002,
1.2489067468800001,
1.46816008988,
1.2922573828800001,
1.23277647988,
1.25747373988,
1.2859942688800001,
1.23156470188
]
}
]
} |
|
| Branch | pr/12206 |
| Testbed | pacquet |
🚨 1 Alert
| Benchmark | Measure Units | View | Benchmark Result (Result Δ%) | Upper Boundary (Limit %) |
|---|---|---|---|---|
| isolated-linker.fresh-restore.cold-cache.cold-store | Latency seconds (s) | 📈 plot 🚷 threshold 🚨 alert (🔔) | 4.83 s(+54.17%)Baseline: 3.13 s | 3.76 s (128.48%) |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| isolated-linker.fresh-install.cold-cache.cold-store | 📈 view plot 🚷 view threshold | 2,128.60 ms(-6.76%)Baseline: 2,282.82 ms | 2,739.38 ms (77.70%) |
| isolated-linker.fresh-install.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 1,243.68 ms(-12.34%)Baseline: 1,418.73 ms | 1,702.47 ms (73.05%) |
| isolated-linker.fresh-restore.cold-cache.cold-store | 📈 view plot 🚷 view threshold 🚨 view alert (🔔) | 4,826.88 ms(+54.17%)Baseline: 3,130.87 ms | 3,757.04 ms (128.48%) |
| isolated-linker.fresh-restore.hot-cache.hot-store | 📈 view plot 🚷 view threshold | 644.05 ms(-5.58%)Baseline: 682.08 ms | 818.50 ms (78.69%) |
…kage publish guard Make pnpr's auth state (users + tokens) pluggable so the registry can run as stateless, horizontally-scaled replicas — phase 3 of #12199. Auth records now sit behind narrow async `UserBackend` / `TokenBackend` traits, built once at startup into `Arc<dyn ...>` handles. Three impls, config-selected: the local htpasswd + SQLite default, in-memory, and a networked libsql/Turso backend (`LibsqlAuth`) that stores both records in one shared database so several replicas observe a consistent set of users and tokens. The `tokens` table DDL is shared verbatim with the local backend; users (which the local backend keeps in htpasswd) move into a `users` table. Selected via a top-level `backend.libsql:` YAML block. Token lookups are on the request hot path, so the libsql backend also supports an embedded replica (`replicaPath` + `syncIntervalSecs`): reads hit a local file libsql keeps current in the background, writes go to the primary. `syncIntervalSecs` bounds read staleness (token-revocation lag). `identify` / `enforce_access` are now async; the latter is split into an async `resolve_identity` plus a sync `authorize`, so the search endpoint resolves the caller once and authorizes each candidate synchronously. Also closes the same-instance lost-update window in the read-modify-write packument flows (publish, dist-tag change, partial-unpublish): a striped per-package lock serializes same-package writers on one instance. The cross-replica half (S3 If-Match / ETag CAS) is documented as follow-up. Written by an agent (Claude Code, claude-opus-4-8).
…e_listener Address review feedback: - Token/user reads (`lookup`, `find_by_key`, `list_for_user`, `verify`, and `identify`) now return `Result` instead of swallowing backend errors into `None`/empty. A networked-auth-DB outage surfaces as a 5xx instead of masquerading as 401/404/empty-200. The local in-memory backends are infallible and just return `Ok(...)`. Added a test that a dropped table makes the reads error rather than report "not found". - Resolve a relative `backend.libsql.replicaPath` against the config file's directory, matching `storage` and the auth file paths, so `./auth-replica.db` lands next to the config rather than in the CWD. - `serve_listener` now loads the configured auth backends via `AuthState::load` (like `serve`) instead of falling back to in-memory auth, so persisted/libsql users and tokens aren't ignored depending on the entrypoint. Written by an agent (Claude Code, claude-opus-4-8).
|
Rebased onto latest
Green locally: 178 lib + all integration tests, plus Written by an agent (Claude Code, claude-opus-4-8). |
Per review (Gemini): in the networked backend the user-count check and the insert were two separate statements, so a concurrent burst could see count == max-1 and all insert, pushing past the cap. Fold the guard into the insert — `INSERT ... SELECT ... WHERE (SELECT COUNT(*) FROM users) < max ON CONFLICT DO NOTHING` — so the count and the insert are evaluated atomically. Writes serialize on the primary, so the cap now holds across replicas too. Keep the cheap pre-check to skip hashing when the cap is already full, and disambiguate a zero row-count (concurrent same-username insert vs. cap reached). New `registration_cap_is_strict_under_concurrency` test asserts a burst of 6 against a cap of 1 admits exactly one. Written by an agent (Claude Code, claude-opus-4-8).
|
Went through a second-opinion review (Gemini). Disposition of the five observations:
Only #1 warranted a code change; the rest are intentional/by-design with reasoning above. Written by an agent (Claude Code, claude-opus-4-8). |
* chore: enable clippy::pedantic lint group for pacquet workspace * style(pacquet): comply with clippy::pedantic Apply clippy's machine-applicable pedantic fixes across the workspace (inlined format args, removed needless borrows/closures, added must_use, etc.), fix a few doc-comment backtick nits, and drop pointless #[inline(always)] on trivial accessors. Opt specific pedantic lints back out in [workspace.lints.clippy] with documented justifications, grouped into false positives, library-API hygiene that doesn't fit an internal CLI, suggestions that conflict with the cardinal rule of porting pnpm 1:1, and opinionated style. * style: taplo-format Cargo.toml lint table * style(pnpr): comply with clippy::pedantic in merged auth backend code Re-apply pedantic compliance to the networked-SQLite auth backend that landed on main (#12186, #12199/#12206): doc-comment backticks, #[must_use] on constructors and status_code, i64::from over `as`, map_or, and a method-reference closure. * docs(clippy): trim and inline the pedantic allow-list comments * docs(clippy): note perfectionist supersedes many_single_char_names * docs(clippy): note pnpm-mirroring rationale on structure/naming lints * docs(clippy): mark unused_async as deferred pending audit * style: enable clippy::match_wildcard_for_single_variants * refactor: enable clippy::unused_self Convert two self-less private methods (overrides pick_most_specific, tarball head_only_result) to associated functions. * refactor: enable clippy::ref_option Widen engine_json to Option<&str>; #[expect] the two serde serialize_with helpers, which serde must call as f(&field, ser). * perf: enable clippy::trivially_copy_pass_by_ref Pass the 1-byte Copy types NodeLinker and FilterWorkspaceProjectsOptions by value; #[expect] the serde skip_serializing_if helper is_false. * perf: enable clippy::assigning_clones Use clone_from for seven field assignments to reuse allocations. * style: enable clippy::manual_let_else Convert 27 match/if-let guards to let-else; preserve the non-UTF-8 skip rationale comment in the directory walker. * style: enable clippy::default_trait_access Name the concrete type on Default::default() call sites; #[expect] two struct-literal test fixtures where naming each field type would force ~20 imports. * refactor: enable clippy::format_push_string Replace push_str(&format!(...)) with write!/writeln! into the target String (local 'use std::fmt::Write as _'); writeln! preserves the exact LF/CRLF shell-shim output. * refactor: enable clippy::needless_pass_by_value Take by reference where the argument is only read (incl. dropping some redundant clones in resolve_peers' recursion). Where converting would cascade badly, #[expect] with a reason: functions that destructure/consume the arg (build_resolve_result, PrefetchingResolver, S3Store::new), the by-value `impl IntoIterator + Clone` in build_direct_deps_by_importer, and the serde/test helpers whose owned fixtures keep call sites clean. * fix(perfectionist): satisfy dylint after format_push_string changes Add trailing commas to the multi-line writeln! shell-shim templates (macro_trailing_comma) and merge the new `fmt::Write as _` imports into each file's existing `use std::{...}` block (import_granularity). * docs(clippy): explain missing_errors_doc suppression; mark missing_panics_doc deferred * fix(perfectionist): collapse fmt::{self, Write as _} in work_env imports The format_push_string Write import landed as a sibling fmt:: path next to the existing fmt import; merge them so import_granularity passes. * style: enable clippy::return_self_not_must_use Add #[must_use] to the WorkspaceTreeCtx builder methods, matching the #[must_use] already on the parallel TreeCtx builders. * perf: enable clippy::large_stack_arrays Heap-allocate the 64 KiB read buffer in verify_file_integrity with a Vec instead of placing it on the stack. * chore(clippy): enable clippy::nursery group Enable the nursery lint group on the pacquet/pnpr workspace and bring the code into compliance. Fixed in code: - iter_on_single_items: [x].into_iter()/.iter() -> std::iter::once - equatable_if_let: pattern match -> equality check (the install_accelerator rewrite wraps in a multi-line matches!, which gets a trailing comma for perfectionist::macro_trailing_comma) - needless_pass_by_ref_mut: load_pending_row/apply_write_msg take &StoreIndex Opted back out in Cargo.toml, each with a documented justification: use_self, too_long_first_doc_paragraph, missing_const_for_fn, option_if_let_else, significant_drop_tightening, redundant_pub_crate, derive_partial_eq_without_eq, branches_sharing_code, useless_let_if_seq, single_option_map, iter_with_drain, literal_string_with_formatting_args, collection_is_never_read. Dropped the now-redundant individual nursery warns (needless_collect, or_fun_call, redundant_clone) the group now covers, plus the default-on unnecessary_lazy_evaluations. Kept clone_on_ref_ptr and if_then_some_else_none (restriction lints not enabled by any group). * style: bring merged main code into clippy pedantic compliance The 17 commits merged from main predate this branch's pedantic/nursery lint config, so their new code tripped pedantic lints. Apply the machine-applicable fixes (uninlined_format_args, if_not_else, elidable_lifetime_names, must_use_candidate, single_match_else, map_unwrap_or, default_trait_access, assigning_clones, doc_markdown, ...) and re-add the documented #[expect(needless_pass_by_value)] on S3Store::new that this branch had carried on the now-replaced file. * style: bring merged main code into clippy pedantic compliance The 28 commits merged from main predate this branch's lint config, so their new code tripped pedantic lints. Apply the machine-applicable fixes (uninlined_format_args, manual_let_else, needless_raw_string_hashes, redundant_closure_for_method_calls, map_unwrap_or, elidable_lifetime_names, doc_markdown, ...) plus a few by hand: - derive Copy on LinkSlotsParallel (all fields are Copy/refs) to clear needless_pass_by_value without a signature change - deduplicate_all takes &[Vec<DepPath>] (it only borrows the duplicates) - pick_most_specific becomes an associated fn (it never used self) - default_trait_access -> concrete types; assigning_clones -> clone_from; format_push_string -> write! - #[expect] with reasons where a fix would churn main's feature code: needless_pass_by_value on the recursive resolve_node and a test helper, and float_cmp on two deterministic-fixture assertions * style: enable clippy::allow_attributes and allow_attributes_without_reason Both are restriction lints (not implied by any group), enabled alongside the existing clone_on_ref_ptr / if_then_some_else_none. Convert every #[allow(...)] (including one nested in cfg_attr) to #[expect(...)]; all already carried a reason, so allow_attributes_without_reason is satisfied. Drop two now-redundant suppressions surfaced by the conversion: a duplicated #[expect(too_many_arguments)] on fetch_and_extract_zip_once (a prior merge left both an allow and an expect), and the #[expect(dead_code)] on MissingPeerInfo's fields (the #[derive(Debug, Clone)] already reads them, so dead_code never fired). clone_on_ref_ptr was already enabled. mod_module_files is intentionally NOT enabled: it mandates mod.rs, the opposite of the flat module.rs pattern this project requires (CODE_STYLE_GUIDE.md, enforced by perfectionist::flat_module_pattern). * style: enable clippy::mod_module_files to enforce the flat module layout mod_module_files bans mod.rs files, enforcing the flat module.rs pattern this project already uses (0 mod.rs in the tree, so no violations). Update CODE_STYLE_GUIDE.md to cite it as the enforcer; perfectionist's flat_module_pattern is being retired in favor of this Clippy rule. * fix(perfectionist): trailing comma on wrapped assert_eq! in workspace_yaml tests The default_trait_access fix lengthened the assert_eq! so fmt wrapped it to multi-line, which perfectionist::macro_trailing_comma requires to end with a trailing comma. * fix(fs): use cfg_attr expect instead of allow for Windows-unused mode args With clippy::allow_attributes enabled, the #[cfg_attr(windows, allow(unused))] on make_file_executable and the ensure_file/write_atomic mode params fails Windows CI. Switch to #[cfg_attr(windows, expect(unused, reason = ...))]; on Windows the lint fires (Unix mode unused there) so the expectation is fulfilled, and the attribute stays inert on Unix. * fix(fs): drop the Windows unused suppression on ensure_file's mode arg ensure_file forwards mode to verify_or_rewrite unconditionally, so it is used on Windows too; the #[cfg_attr(windows, expect(unused))] was therefore unfulfilled and failed Windows CI under -D warnings. write_atomic and make_file_executable keep their expect — they use mode/file only under #[cfg(unix)], so the lint fires (and the expectation holds) on Windows. * chore(git): revert "fix(fs): drop the Windows unused suppression on ensure_file's mode arg" This reverts commit 1d617c3. * chore(git): revert "fix(fs): use cfg_attr expect instead of allow for Windows-unused mode args" This reverts commit 155e4a3. * chore(git): revert "style: enable clippy::allow_attributes and allow_attributes_without_reason" This reverts commit a47d792. * style: bring merged main code into clippy compliance + fix merge mismatch - Add & at the two run_postinstall_hooks / run_project_lifecycle_scripts call sites: this branch widened lifecycle.rs to take &RunPostinstallHooks, but main's by-value call sites came in via the conflict resolution. - pedantic fixes on main's new code: must_use_candidate, unnested_or_patterns, manual_let_else, default_trait_access, iter_on_single_items, and trivially_copy_pass_by_ref (map_node_linker takes NodeLinker by value). --------- Co-authored-by: Claude <noreply@anthropic.com>
What
Implements the auth half of #12199 (phase 3) — making pnpr's remaining per-instance state pluggable so the registry can run as stateless, horizontally-scaled replicas.
Auth records behind config-selected backends
Users + tokens now sit behind narrow async
UserBackend/TokenBackendtraits, built once at startup intoArc<dyn …>handles (the same build-once pattern #12198 used for the hosted store). Three implementations:LibsqlAuthstores both records in one shared database, so several stateless replicas observe a consistent set of users and tokens. Thetokenstable DDL is shared verbatim with the local backend (a DB can migrate between them); users — which the local backend keeps in htpasswd — move into auserstable.Selected via a new top-level YAML block:
When the block is absent, auth stays on local disk exactly as before.
Embedded-replica read acceleration
Token lookups are on the request hot path, so against a remote primary every read would be a network round-trip. With
replicaPathset,LibsqlAuthbuilds a libsql embedded replica: reads hit a local file that libsql keeps current in the background; writes go to the primary.syncIntervalSecsis the freshness knob that bounds token-revocation lag.Async access path
identify/enforce_accessare now async (a networked lookup is async).enforce_accessis split into an asyncresolve_identity+ a syncauthorize, so the search endpoint resolves the caller once and authorizes each candidate synchronously (no async-in-retain).Concurrent-publish guard (cross-cutting follow-up from the issue)
Closes the same-instance lost-update window in the three read-modify-write packument flows (publish, dist-tag change, partial-unpublish): a striped per-package lock serializes same-package writers on one instance while letting different packages proceed in parallel. The cross-replica half (S3
If-Match/ ETag CAS) is documented in-code as the remaining piece — the issue files it under "fix when we get there," and it belongs with the multi-writer S3 publish work, not this auth branch.Tests
All green —
cargo test -p pnpr:LibsqlAuthtests (run against an in-memory libsql DB — same driver + SQL, no server) andbackend.libsqlconfig-parsing tests (incl. the replica options).concurrent_publishes_of_distinct_versions_all_surviveintegration test for the publish guard.cargo fmt,clippy,RUSTDOCFLAGS=-D warnings cargo doc, Dylint perfectionist, andtaplo.Docs
backend.libsql(incl. embedded replica) documented in the bundledconfig.yamland thepnprnpm README, mirroring how the S3 backend was documented in #12198.Notes
Written by an agent (Claude Code, claude-opus-4-8).
Summary by CodeRabbit
New Features
Configuration
Documentation