Walkthrough

This PR adds a new pkg CLI subcommand for reading, writing, deleting, and normalizing package.json fields. It wires the command into dispatch, adds recursive workspace execution, implements property-path helpers, and expands unit coverage.

Changes

pkg subcommand

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• pnpm/pnpm#12632: Extends the same CLI command-dispatch framework with a different CliCommand variant and routing arm.
• pnpm/pnpm#12687: Also adds a new CliCommand variant and matching dispatch path in the central CLI routing.
• pnpm/pnpm#12728: Updates the Rust CLI plumbing by adding a new subcommand variant and dispatch match arm.

Suggested labels: product: pacquet

Suggested reviewers: zkochan, qodo-free-for-open-source-projects

• X
• Mastodon
• Reddit
• LinkedIn

• Add pkg top-level CLI command with get, set, delete, and fix subcommands.
• Implement property-path manifest reads/writes with --json mode and unsafe-key rejection.
• Add unit tests covering output formatting, mutations, deletions, and manifest auto-fixes.

graph TD
  A["pacquet CLI (clap)"] --> B["CliCommand routing"] --> C["pkg command"] --> D["Property-path parser"]
  C --> E["PackageManifest"] --> F[("package.json")]
  C --> E

Loading

The following are alternative approaches to this PR:

1. Move set/delete helpers into `pacquet_config::property_path`

• ➕ Centralizes all property-path behavior (parse/get/set/delete) in one crate for reuse by future commands.
• ➕ Encourages consistent error handling and unsafe-key policy across the CLI.
• ➖ Requires expanding the config crate’s scope and API surface.
• ➖ May force a more generic mutation API than this command needs right now.

2. Adopt JSON Pointer / JSONPath library instead of pnpm property-path

• ➕ Leverages well-tested ecosystem parsing and mutation support.
• ➕ Potentially simpler implementation for set/delete semantics.
• ➖ Would likely diverge from pnpm’s observable CLI behavior and accepted syntax.
• ➖ Adds a dependency and migration complexity for parity-focused porting.

Recommendation: Current approach (pnpm-compatible property-path syntax + custom set/delete semantics) is appropriate for behavior parity. If additional commands need mutation through property paths, consider promoting the set/delete logic into pacquet_config::property_path to avoid duplication and keep semantics consistent.

Enhancement (5) +349 / -0

cli_args.rsExport new 'pkg' CLI args module +1/-0

Export new 'pkg' CLI args module

• Adds the 'pkg' module to the CLI args module list so it can be referenced by command parsing and dispatch.

pacquet/crates/cli/src/cli_args.rs

cli_command.rsRegister 'Pkg' as a top-level CLI command +3/-0

Register 'Pkg' as a top-level CLI command

• Introduces 'CliCommand::Pkg(PkgArgs)' and wires it into the clap command enum so 'pnpm pkg ...' is recognized.

pacquet/crates/cli/src/cli_args/cli_command.rs

dispatch.rsRoute 'Pkg' command to script-style dispatcher +1/-0

Route 'Pkg' command to script-style dispatcher

• Adds a dispatch arm mapping 'CliCommand::Pkg' to 'dispatch_script::pkg', keeping it synchronous like other manifest-only commands.

pacquet/crates/cli/src/cli_args/dispatch.rs

dispatch_script.rsAdd synchronous dispatcher for 'pkg' +6/-0

Add synchronous dispatcher for 'pkg'

• Defines 'dispatch_script::pkg' which runs 'PkgArgs::run' against the resolved 'manifest_path' without invoking install/lockfile pipelines.

pacquet/crates/cli/src/cli_args/dispatch_script.rs

pkg.rsImplement 'pkg get/set/delete/fix' with property-path support +338/-0

Implement 'pkg get/set/delete/fix' with property-path support

• Adds clap subcommand parsing for 'get', 'set', 'delete', and 'fix', including global '--json' behavior. Implements pnpm-style property-path reads, JSON-aware writes, array-element removal on delete, unsafe key rejection, and a 'fix' routine that removes common invalid manifest fields before saving.

pacquet/crates/cli/src/cli_args/pkg.rs

Tests (1) +251 / -0

tests.rsUnit tests for 'pkg' output and manifest mutations +251/-0

Unit tests for 'pkg' output and manifest mutations

• Adds tests for 'get' formatting behavior, selection semantics (no keys / single key / multiple keys), 'fix' cleanup rules, and property-path set/delete behavior including nested keys and array indices.

pacquet/crates/cli/src/cli_args/pkg/tests.rs

Description

dispatch_script::pkg always runs against ctx.manifest_path and ignores the global --recursive
execution mode, so pacquet -r pkg ... will silently operate only on the root project instead of
every selected workspace package (unlike pnpm’s implementation). This is an observable behavior
mismatch and can lead to partially-updated workspaces.

Code

pacquet/crates/cli/src/cli_args/dispatch_script.rs[R31-34]

+pub(super) fn pkg<'a>(ctx: &RunCtx<'a>, args: PkgArgs) -> miette::Result<CommandFuture<'a>> {
+    let result = args.run(ctx.manifest_path);
+    Ok(Box::pin(std::future::ready(result)))
+}

Evidence

The Rust dispatch path for pkg never checks ctx.recursive, while other commands (e.g., run)
do. The pnpm reference implementation explicitly supports recursive pkg execution via a dedicated
handler.

pacquet/crates/cli/src/cli_args/dispatch_script.rs[31-34]
pacquet/crates/cli/src/cli_args/dispatch_script.rs[45-60]
pnpm11/pkg-manifest/commands/src/pkg.ts[48-98]
pnpm11/pkg-manifest/commands/src/pkg.ts[86-93]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`Pkg` does not honor the global `-r/--recursive` mode. Unlike `run`/`exec`, `dispatch_script::pkg` unconditionally calls `args.run(ctx.manifest_path)` and never iterates workspace projects.
## Issue Context
Upstream pnpm’s `pkg` command has an explicit recursive path (`handleRecursiveCommand`) that runs `get` across selected workspace projects.
## Fix Focus Areas
- pacquet/crates/cli/src/cli_args/dispatch_script.rs[31-34]
- pacquet/crates/cli/src/cli_args/pkg.rs[92-113]
## Suggested fix
- Add a `run_recursive(...)` method for `PkgArgs` (similar to `RunArgs::run_recursive`).
- In `dispatch_script::pkg`, branch on `ctx.recursive` and call `args.run_recursive((ctx.config)()?, ctx.dir, ...)` (and/or include filter selection behavior consistent with other recursive commands).
- Ensure output format matches pnpm’s recursive `pkg get` behavior (a JSON map keyed by package name).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

pkg set/pkg delete/pkg fix persist via PackageManifest::save(), which rewrites
dependencies/devDependencies entries of the form runtime: into engines/devEngines.runtime
and can fail saving when those dependency fields exist but are not JSON objects. This means pkg
can unexpectedly change unrelated keys (or error) even when the user edits a completely different
path, diverging from the intended “raw package.json editor” behavior.

Code

pacquet/crates/cli/src/cli_args/pkg.rs[R168-215]

+fn pkg_set(manifest_path: &Path, pairs: &[String], json: bool) -> miette::Result<()> {
+    if pairs.is_empty() {
+        return Err(PkgError::SetMissingArgs.into());
+    }
+    let mut manifest =
+        PackageManifest::from_path(manifest_path.to_path_buf()).wrap_err("reading package.json")?;
+    let value = manifest.value_mut();
+    for pair in pairs {
+        let eq_index =
+            pair.find('=').ok_or_else(|| PkgError::SetInvalidArg { arg: pair.clone() })?;
+        let key = &pair[..eq_index];
+        let raw_value = &pair[eq_index + 1..];
+        let parsed_value: Value = if json {
+            serde_json::from_str(raw_value)
+                .map_err(|_| PkgError::SetJsonParse { value: raw_value.to_string() })?
+        } else {
+            Value::String(raw_value.to_string())
+        };
+        set_object_value_by_property_path(value, key, parsed_value)?;
+    }
+    manifest.save().wrap_err("saving package.json")?;
+    Ok(())
+}
+
+fn pkg_delete(manifest_path: &Path, keys: &[String]) -> miette::Result<()> {
+    if keys.is_empty() {
+        return Err(PkgError::DeleteMissingArgs.into());
+    }
+    for key in keys {
+        check_unsafe_key_in_path(key)?;
+    }
+    let mut manifest =
+        PackageManifest::from_path(manifest_path.to_path_buf()).wrap_err("reading package.json")?;
+    let value = manifest.value_mut();
+    for key in keys {
+        delete_object_value_by_property_path(value, key)?;
+    }
+    manifest.save().wrap_err("saving package.json")?;
+    Ok(())
+}
+
+fn pkg_fix(manifest_path: &Path) -> miette::Result<()> {
+    let mut manifest =
+        PackageManifest::from_path(manifest_path.to_path_buf()).wrap_err("reading package.json")?;
+    let value = manifest.value_mut();
+    fix_manifest(value);
+    manifest.save().wrap_err("saving package.json")?;
+    Ok(())

Evidence

The new pkg command always persists via PackageManifest::save(), and save() rewrites runtime
dependency entries and can error when dependency fields are not objects, creating unexpected side
effects/failures for a command that should only apply the user’s requested key edits.

pacquet/crates/cli/src/cli_args/pkg.rs[168-215]
pacquet/crates/package-manifest/src/lib.rs[179-190]
pacquet/crates/package-manifest/src/lib.rs[415-443]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pkg set/delete/fix` call `PackageManifest::save()`, but `save()` is not a raw serializer: it performs manifest-wide normalization (`convert_dependencies_to_engines_runtime`) and can reject manifests where `dependencies`/`devDependencies` are present but not objects. This causes `pkg` to mutate unrelated fields and/or fail for manifests that pnpm’s `pkg` command would still edit.
## Issue Context
`PackageManifest::save_and_get_written_value()` clones the JSON and applies runtime/deps folding before writing. That behavior is appropriate for pacquet’s runtime-engine contract, but `pkg` is intended to be a general package.json editor.
## Fix Focus Areas
- Implement a raw/escape-hatch save path that writes the in-memory manifest without calling `convert_dependencies_to_engines_runtime`.
- pacquet/crates/package-manifest/src/lib.rs[179-190]
- Use that raw save method from `pkg set/delete/fix` instead of `save()`.
- pacquet/crates/cli/src/cli_args/pkg.rs[168-215]
(Keep atomic write + permission preservation behavior consistent with `write_atomic`.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

In set_object_value_by_property_path, when an intermediate Segment::Index is applied while
current is neither an object nor array, the code replaces current with a container but does not
descend into the indexed slot. Subsequent segments then apply at the wrong level, producing an
incorrect JSON structure for paths that start with (or encounter) an index while the current value
is scalar.

Code

pacquet/crates/cli/src/cli_args/pkg.rs[R346-357]

+                } else {
+                    let replacement = if needs_array {
+                        let mut arr = Vec::with_capacity(index.saturating_add(1));
+                        arr.resize(index.saturating_add(1), Value::Null);
+                        Value::Array(arr)
+                    } else {
+                        let mut map = Map::new();
+                        map.insert(idx_to_string(*idx), Value::Null);
+                        Value::Object(map)
+                    };
+                    *current = replacement;
+                }

Evidence

The traversal assigns *current = replacement without updating the cursor; since
PackageManifest::from_path accepts any JSON Value (no enforcement that the root is an object),
this can occur in real runs against malformed/non-object package.json files and will misplace
subsequent writes.

pacquet/crates/cli/src/cli_args/pkg.rs[313-357]
pacquet/crates/package-manifest/src/lib.rs[122-128]
pacquet/crates/package-manifest/src/lib.rs[139-146]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
In `set_object_value_by_property_path`, the `Segment::Index` traversal has a branch where `current` is neither object nor array. That branch sets `*current = replacement` but never updates `current` to the newly created `arr[index]` (array case) or `obj[key]` (object case). Any remaining segments are then applied to the wrong node.
### Issue Context
This breaks nested sets for some valid property paths when the JSON root (or an intermediate value reached via index traversal) is scalar and must be replaced to continue.
### Fix Focus Areas
- pacquet/crates/cli/src/cli_args/pkg.rs[292-360]
### Suggested approach
After `*current = replacement;`:
- If `needs_array` is true: ensure the array has length `index + 1` and set `current = &mut arr[index]`.
- Else: ensure the object contains `key` and set `current = obj.get_mut(&key).unwrap()`.
Add a unit test that demonstrates the bug, e.g. starting with a scalar root and running a path like `[0].a` or similar, and verifying the nested structure is created under key/index `0` rather than at the root.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

pkg set uses Vec::resize(index+1, Null) to materialize arrays, so a single large index (allowed
up to 1<<20) can force allocation of ~1M elements; with --recursive this can be repeated across
many workspace projects. This is attacker-controlled CLI input and can lead to severe slowdown or
OOM (practical DoS), even though the index is capped.

Code

pacquet/crates/cli/src/cli_args/pkg.rs[R362-485]

+pub(crate) const MAX_ARRAY_INDEX: usize = 1 << 20;
+
+fn validate_index(idx: f64) -> Result<usize, PkgError> {
+    if idx.fract() != 0.0 || idx.is_sign_negative() || !idx.is_finite() {
+        return Err(PkgError::SetPathError { path: idx.to_string() });
+    }
+    let index = idx as usize;
+    if index > MAX_ARRAY_INDEX {
+        return Err(PkgError::SetPathError { path: idx.to_string() });
+    }
+    Ok(index)
+}
+
+fn idx_to_string(idx: f64) -> String {
+    if idx.fract() == 0.0 && idx.is_finite() { format!("{}", idx as i64) } else { idx.to_string() }
+}
+
+fn set_object_value_by_property_path(
+    root: &mut Value,
+    path: &str,
+    value: Value,
+) -> miette::Result<()> {
+    if path.is_empty() {
+        return Err(PkgError::EmptyPath.into());
+    }
+    check_unsafe_key_in_path(path)?;
+    let segments = parse_property_path(path)
+        .map_err(|err| miette::Report::new(PkgError::InvalidPropertyPath(err)))?;
+    if segments.is_empty() {
+        return Err(PkgError::EmptyPath.into());
+    }
+    let last_idx = segments.len() - 1;
+    let mut current = root;
+    for i in 0..last_idx {
+        let needs_array = matches!(&segments[i + 1], Segment::Index(_));
+        match &segments[i] {
+            Segment::Key(k) => {
+                if !current.is_object() {
+                    *current = Value::Object(Map::new());
+                }
+                let obj = current.as_object_mut().unwrap();
+                let entry = obj.get_mut(k);
+                let is_good = entry
+                    .is_some_and(|val| if needs_array { val.is_array() } else { val.is_object() });
+                if !is_good {
+                    let replacement = if needs_array {
+                        Value::Array(Vec::new())
+                    } else {
+                        Value::Object(Map::new())
+                    };
+                    obj.insert(k.clone(), replacement);
+                }
+                current = obj.get_mut(k).unwrap();
+            }
+            Segment::Index(idx) => {
+                let index = validate_index(*idx)?;
+                if current.is_object() {
+                    let key = idx_to_string(*idx);
+                    let obj = current.as_object_mut().unwrap();
+                    let entry = obj.get_mut(&key);
+                    let is_good = entry.is_some_and(|val| {
+                        if needs_array { val.is_array() } else { val.is_object() }
+                    });
+                    if !is_good {
+                        let replacement = if needs_array {
+                            Value::Array(Vec::new())
+                        } else {
+                            Value::Object(Map::new())
+                        };
+                        obj.insert(key.clone(), replacement);
+                    }
+                    current = obj.get_mut(&key).unwrap();
+                } else if current.is_array() {
+                    let arr = current.as_array_mut().unwrap();
+                    if index >= arr.len() {
+                        arr.resize(index.saturating_add(1), Value::Null);
+                    }
+                    let entry = &mut arr[index];
+                    let is_good = if needs_array { entry.is_array() } else { entry.is_object() };
+                    if !is_good {
+                        *entry = if needs_array {
+                            Value::Array(Vec::new())
+                        } else {
+                            Value::Object(Map::new())
+                        };
+                    }
+                    current = &mut arr[index];
+                } else {
+                    let replacement = if needs_array {
+                        let mut arr = Vec::with_capacity(index.saturating_add(1));
+                        arr.resize(index.saturating_add(1), Value::Null);
+                        Value::Array(arr)
+                    } else {
+                        let mut map = Map::new();
+                        map.insert(idx_to_string(*idx), Value::Null);
+                        Value::Object(map)
+                    };
+                    *current = replacement;
+                }
+            }
+        }
+    }
+    match &segments[last_idx] {
+        Segment::Key(k) => {
+            if !current.is_object() {
+                *current = Value::Object(Map::new());
+            }
+            current.as_object_mut().unwrap().insert(k.clone(), value);
+        }
+        Segment::Index(idx) => {
+            let index = validate_index(*idx)?;
+            if current.is_object() {
+                current.as_object_mut().unwrap().insert(idx_to_string(*idx), value);
+            } else if current.is_array() {
+                let arr = current.as_array_mut().unwrap();
+                if index >= arr.len() {
+                    arr.resize(index.saturating_add(1), Value::Null);
+                }
+                arr[index] = value;
+            } else {
+                let mut arr = Vec::with_capacity(index.saturating_add(1));
+                arr.resize(index.saturating_add(1), Value::Null);
+                arr[index] = value;
+                *current = Value::Array(arr);

Evidence

The setter validates indices up to MAX_ARRAY_INDEX, then grows arrays with resize(index+1),
which creates a dense vector of serde_json::Value entries. The recursive implementation applies
set_object_value_by_property_path to each selected project, multiplying the cost.

pacquet/crates/cli/src/cli_args/pkg.rs[164-187]
pacquet/crates/cli/src/cli_args/pkg.rs[362-373]
pacquet/crates/cli/src/cli_args/pkg.rs[434-439]
pacquet/crates/cli/src/cli_args/pkg.rs[476-485]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Setting `foo[big]=...` forces dense allocation via `Vec::resize`, which is expensive and can OOM at the current cap (1<<20). This is especially problematic under `--recursive` where the same input is applied to many manifests.
## Issue Context
`serde_json::Value::Array` is dense; unlike JS sparse arrays, resizing to a large index allocates and initializes every intermediate element.
## Fix Focus Areas
- pacquet/crates/cli/src/cli_args/pkg.rs[362-373]
- pacquet/crates/cli/src/cli_args/pkg.rs[434-439]
- pacquet/crates/cli/src/cli_args/pkg.rs[476-485]
## Suggested change
- Tighten the bound and/or add an additional growth guard:
- Example: reject if `index > MAX_ARRAY_INDEX` **or** if `index.saturating_sub(arr.len()) > MAX_ARRAY_GROWTH_DELTA`.
- Consider a smaller `MAX_ARRAY_INDEX` for a CLI editor, or make it configurable.
- Ensure the error clearly indicates the offending path/index (so users can fix scripts that accidentally pass huge indices).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

pkg get prints data read from PackageManifest::from_path, and the manifest reader uses
fs::read_to_string, which follows symlinks. In automation/CI that runs pacquet pkg get on
untrusted checkouts, a malicious repo can symlink package.json to a local JSON file and have its
contents printed to logs (local file read/exfiltration).

Code

pacquet/crates/cli/src/cli_args/pkg.rs[R105-112]

+    pub fn run(self, manifest_path: &Path) -> miette::Result<()> {
+        match self.command {
+            PkgSubcommand::Get(args) => {
+                let output = pkg_get(manifest_path, &args.keys, self.json)?;
+                if !output.is_empty() {
+                    println!("{output}");
+                }
+            }

Evidence

The new command prints the manifest output, and the existing manifest loader reads via
fs::read_to_string, which follows symlinks by default, enabling disclosure if package.json is a
symlink to another JSON file.

pacquet/crates/cli/src/cli_args/pkg.rs[105-112]
pacquet/crates/cli/src/cli_args/pkg.rs[218-223]
pacquet/crates/package-manifest/src/lib.rs[122-127]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pkg get` can disclose arbitrary local JSON files when `package.json` is a symlink, because the manifest reader follows symlinks and the command prints the content.
## Issue Context
This is most concerning in CI/tooling that processes untrusted repositories and logs command output.
## Fix Focus Areas
- pacquet/crates/cli/src/cli_args/pkg.rs[105-112]
- pacquet/crates/cli/src/cli_args/pkg.rs[218-223]
- pacquet/crates/package-manifest/src/lib.rs[122-127]
## Suggested change
- Add a symlink check for `package.json` before reading:
- For minimal surface-area change: in `pkg_get` (and the recursive-get loop), call `std::fs::symlink_metadata(manifest_path)` and error if `file_type().is_symlink()`.
- For a broader hardening: add an opt-in "no symlinks" read path in `PackageManifest` and use it from commands that print manifest contents.
- Ensure the error message clearly states symlinks are not allowed for this operation.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

pkg set/pkg delete parse each property path multiple times (unsafe-key check plus the actual
set/delete), adding avoidable CPU overhead when many keys are processed in one invocation. This is
especially wasteful in pkg delete, which can parse each key three times (pre-check loop + delete +
delete's own unsafe check).

Code

pacquet/crates/cli/src/cli_args/pkg.rs[R284-286]

+    check_unsafe_key_in_path(path)?;
+    let segments = parse_property_path(path)
+        .map_err(|err| miette::Report::new(PkgError::InvalidPropertyPath(err)))?;

Evidence

check_unsafe_key_in_path calls parse_property_path, but both set_object_value_by_property_path
and delete_object_value_by_property_path call parse_property_path again; pkg_delete also
pre-validates each key before calling delete, compounding the overhead.

pacquet/crates/cli/src/cli_args/pkg.rs[192-206]
pacquet/crates/cli/src/cli_args/pkg.rs[247-257]
pacquet/crates/cli/src/cli_args/pkg.rs[281-287]
pacquet/crates/cli/src/cli_args/pkg.rs[389-396]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`check_unsafe_key_in_path()` parses the property path, and then `set_object_value_by_property_path()` / `delete_object_value_by_property_path()` parse the same string again. For `pkg delete`, there is an additional pre-check loop in `pkg_delete`, so the same key can be parsed multiple times.
### Issue Context
This is a performance/maintainability issue on the hot path for `pkg set`/`pkg delete` with many keys.
### Fix Focus Areas
- pacquet/crates/cli/src/cli_args/pkg.rs[192-206]
- pacquet/crates/cli/src/cli_args/pkg.rs[247-257]
- pacquet/crates/cli/src/cli_args/pkg.rs[276-287]
- pacquet/crates/cli/src/cli_args/pkg.rs[389-396]
### Suggested approach
- Change `check_unsafe_key_in_path` to take already-parsed `&[Segment]` (or return parsed segments after validation).
- In `set_object_value_by_property_path` and `delete_object_value_by_property_path`, parse once, validate unsafe keys by iterating the segments, then proceed.
- In `pkg_delete`, remove the pre-check loop once delete validates segments itself (or keep the pre-check but make delete accept pre-parsed segments).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

PkgError::SetJsonParse formats with an extra trailing quote, producing malformed user-facing error
output for invalid --json values.

Code

pacquet/crates/cli/src/cli_args/pkg.rs[R24-26]

+    #[display(r#"Failed to parse value as JSON: "{value}""#)]
+    #[diagnostic(code(ERR_PNPM_PKG_SET_JSON_PARSE))]
+    SetJsonParse { value: String },

Evidence

The raw string literal includes "{value}"" (two quotes at the end), which will render an unmatched
trailing quote in the formatted message.

pacquet/crates/cli/src/cli_args/pkg.rs[24-26]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `SetJsonParse` error message contains an extra trailing quote, leading to malformed error output.
### Issue Context
This affects user-facing diagnostics when `pnpm pkg set --json key=<value>` fails to parse the value.
### Fix Focus Areas
- pacquet/crates/cli/src/cli_args/pkg.rs[24-26]
### Suggested fix
Change the display string from:
- `Failed to parse value as JSON: "{value}""`
To:
- `Failed to parse value as JSON: "{value}"`
(or similar), ensuring the rendered string has balanced quotes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

PkgError::SetPathError stores a path field but the display string does not include it, reducing
diagnosability for invalid index/path failures.

Code

pacquet/crates/cli/src/cli_args/pkg.rs[R40-43]

+    #[display("Cannot set property on a non-object or non-array value at path")]
+    #[diagnostic(code(ERR_PNPM_PKG_SET_PATH_ERROR))]
+    SetPathError { path: String },
+

Evidence

The display annotation is a fixed string and does not reference {path}, so the stored context is
not shown to the user.

pacquet/crates/cli/src/cli_args/pkg.rs[40-43]
pacquet/crates/cli/src/cli_args/pkg.rs[261-269]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SetPathError { path: String }` does not interpolate `path` into its display message, so users don’t see what segment/index caused the failure.
### Issue Context
`validate_index` constructs `SetPathError { path: idx.to_string() }` on invalid indices; without including `{path}` in the display, the error is much less actionable.
### Fix Focus Areas
- pacquet/crates/cli/src/cli_args/pkg.rs[40-43]
- pacquet/crates/cli/src/cli_args/pkg.rs[261-269]
### Suggested fix
Update the display to include the captured field, e.g.:
- `Cannot set property on a non-object or non-array value at path: {path}`
(or rename the variant / field if the message is intended to cover more than index validation).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Description

An empty property path parses to zero segments, so pkg get '' returns the entire manifest (because
the getter returns the root value when given an empty segment list). This is surprising behavior for
a “single key” query and can mask bugs in scripts that accidentally pass empty keys.

Code

pacquet/crates/cli/src/cli_args/pkg.rs[R119-137]

+fn get_output(manifest: &Value, keys: &[String], json: bool) -> miette::Result<String> {
+    if keys.len() == 1 {
+        let key = &keys[0];
+        let segments = parse_property_path(key).map_err(PkgError::InvalidPropertyPath)?;
+        match get_object_value_by_property_path(manifest, &segments) {
+            None => Ok(String::new()),
+            Some(found) => {
+                if json {
+                    serde_json::to_string_pretty(found).map_err(|e| miette::miette!("{e}"))
+                } else {
+                    match found {
+                        Value::String(s) => Ok(s.clone()),
+                        other => {
+                            serde_json::to_string_pretty(other).map_err(|e| miette::miette!("{e}"))
+                        }
+                    }
+                }
+            }
+        }

Evidence

The property-path parser explicitly treats the empty string as a valid path producing zero segments.
The getter returns the current value after iterating segments; with zero segments it returns the
root manifest, so pkg get with a single empty key prints the full package.json.

pacquet/crates/cli/src/cli_args/pkg.rs[120-137]
pacquet/crates/config/src/property_path/tests.rs[10-28]
pacquet/crates/config/src/property_path.rs[286-317]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`pkg get` treats a single empty key as a valid property path and returns the entire manifest because parsing yields an empty segment list and the getter returns the root value for empty paths. This is an edge case but can cause unexpected output and hide script bugs.
### Issue Context
`parse_property_path("")` returns `Ok(vec![])`, and `get_object_value_by_property_path` returns `Some(current)` when `property_path` is empty.
### Fix Focus Areas
- pacquet/crates/cli/src/cli_args/pkg.rs[119-137]
### Suggested fix
- In `get_output`, if `keys.len() == 1 && keys[0].is_empty()`, return an explicit `PkgError` (new error code) instead of treating it as "get root".
- Optionally apply the same empty-path rejection to `set`/`delete` for consistency, unless pnpm explicitly defines empty-path semantics for `pkg` (then add tests documenting it).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.