[zkochan]

Follow-up to #84. Config files that are read as YAML were assembled by hand from template strings, which is correct only while no value needs quoting (a path with : or # in it, a string YAML would read as a number or boolean). JSON is valid YAML and JSON.stringify escapes everything, so these are now built as objects and written through a shared yamlText helper in benchmarkFixture.js:

• pnprServer.js: pnpr's pnpr.yaml and the registries.yaml it imports
• benchmarkFixture.js: pnpmWorkspaceYaml and the Yarn .yarnrc.yml
• benchmarkLockfileConflicts.js: the scenario pnpm-workspace.yaml
• index.js, populatePnprCache.js, setupPackageManagers.js: the minimumReleaseAge: 0 workspace manifests

The comments that explained each setting stay next to the matching object keys. Output is indented, so the generated files remain readable when debugging a run. .npmrc is INI rather than YAML, so it is unchanged.

Testing

• pnpm run lint and pnpm test pass.
• Local smoke test with pnpr 0.1.0-alpha.20 and pnpm 12.21.0 (startPnpr, mintToken, the latency proxies, verifyResolverIsUsed on a project that includes a @teambit package), on fresh storage and again on a copied, already-populated storage: pnpr accepts the config and the registries import, and pnpm reads the generated pnpm-workspace.yaml and resolves through the server.
• Yarn 6.0.0-rc.23 reads every key of a generated .yarnrc.yml (yarn config get).

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor
  • Workspace, package-manager, and registry configuration files are now generated from structured settings while preserving their existing options and routing behavior.
  • Empty pnpm workspace configurations no longer leave behind a configuration file.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The change adds yamlText, which serializes objects as indented JSON with a trailing newline, and uses it for pnpm workspace, Yarn, and pnpr configuration files. Existing configuration values and conditional file handling are retained.

Changes: Configuration YAML Serialization

Layer / File(s)	Summary
Shared serializer and fixture configuration benchmarkFixture.js	Adds yamlText and uses object-based serialization for pnpm workspace and Yarn configuration.
Workspace YAML writers benchmarkLockfileConflicts.js, index.js, populatePnprCache.js, setupPackageManagers.js	Uses yamlText to generate workspace configuration. The lockfile-conflict writer retains conditional removal when the workspace object is empty.
pnpr configuration serialization pnprServer.js	Uses yamlText for server and registry configuration while retaining the existing settings and routes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: Minimal · up to 2fb05

This refactor changes how benchmark configuration files are written, and the output is still valid YAML. No concrete merge-blocking risk was found.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title "refactor: write YAML config files with JSON.stringify" matches the main change. The PR replaces hand-assembled YAML strings with objects serialized through the shared yamlText helper, which…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "refactor: write YAML config files with JSON.stringify" matches the main change. The PR replaces hand-assembled YAML strings with objects serialized through the shared yamlText helper, which uses JSON.stringify, since JSON is valid YAML. The title is specific and concise. It names the approach rather than every affected file, which is acceptable.

Finishing Touches

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks the config lines,
Then finds the YAML neatly aligned.
The workspace files take shape with care,
Server settings join the share.
I twitch my nose and hop away!}

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: a9bbecb9-ed10-4572-8431-bbc8a654faea

Commits

Reviewing files that changed from the base of the PR and between c4542f1 and 2fb0527.

Files selected for processing: 6

• benchmarkFixture.js
• benchmarkLockfileConflicts.js
• index.js
• pnprServer.js
• populatePnprCache.js
• setupPackageManagers.js

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.

Recent review details

Context from checks skipped due to timeout: 1

• GitHub Check: CodeRabbit

Additional context used

ast-grep (0.51.1)

benchmarkFixture.js

[warning] 330-330: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFile(path.join(cwd, '.yarnrc.yml'), yamlText(yarnRc))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename)

benchmarkLockfileConflicts.js

[warning] 125-125: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(wsYamlPath, yamlText(workspace))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename)

index.js

[warning] 213-213: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(dir, 'pnpm-workspace.yaml'), yamlText({ packages: ['.'], minimumReleaseAge: 0 }), 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename)

pnprServer.js

[warning] 122-157: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(configPath, yamlText({
listen: ${PNPR_HOST}:${port},
// pnpr rewrites the tarball URLs it serves to this address. It has to be
// the address clients actually use, or every tarball would be fetched on a
// link the benchmark isn't emulating — and npm refuses outright to fetch a
// tarball from a host other than the registry it was told about. Set here
// rather than on the command line because the registries import below
// checks its routes against it too.
...(publicUrl ? { publicUrl } : {}),
// Accounts and the registries live in the database, which pnpr keeps
// inside the storage directory and binds the storage to. A storage
// populatePnprCache.js filled therefore arrives with the populate run's
// account and registries: the account is signed in to again (see
// mintToken) and the registries are replaced below.
storage: path.join(dir, 'storage'),
// Upstream content is cached under keys derived from this, and pnpr
// refuses to reuse a cache directory without one. The benchmark hands
// every run the storage populatePnprCache.js filled, so it has to be
// the same in all of them. Nothing here is secret: the server only ever
// listens on loopback.
secret: 'pnpm-benchmarks-pnpr-cache-secret',
auth: {
users: {
// The benchmark registers its own resolver user on startup.
maxUsers: 1,
},
},
resolver: { enabled: true },
routes: {
// The tar registry the accelerated scenario resolves against is this
// server's own, at its un-proxied address (see applyPnprServerRegistry),
// and the resolver refuses loopback addresses that are not listed here.
allowedPrivateNetworks: ['127.0.0.0/8'],
},
log: { format: 'pretty', level: logLevel },
}))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename)

[warning] 227-277: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(registriesPath, yamlText({
registries: {
// The public npm registry. No packages: map, so it claims every
// name — the catch-all, and therefore last in the router's sources.
npmjs: {
type: 'upstream',
url: 'https://registry.npmjs.org/',
public: true,
},
// Bit publishes to a registry of its own, and alotta-packages
// depends on it. Declaring the scopes it serves is all the routing
// that is needed: every manager keeps installing from the one
// registry URL the benchmark hands it, and the names below are the
// only ones fetched from bit. Tarball URLs come back rewritten to
// pnpr's own address, so bit's packages cross the emulated link
// like every other package.
bit: {
type: 'upstream',
url: BIT_REGISTRY,
public: true,
packages: Object.fromEntries(BIT_SCOPES.map((scope) => [${scope}/*, null])),
},
// One URL in front of both: a router resolves a name to the first
// source whose declared packages claim it. defaultRegistry has to
// name this rather than a concrete registry — a concrete one
// resolves only its own namespace and answers Unclaimed for
// everything else, with no fall-through to a sibling.
main: {
type: 'router',
sources: ['bit', 'npmjs'],
},
// The same sources again, served as tars rather than .tgz files (see
// tarRegistryUrl). One storage behind both: a package pulled through
// either is cached once.
[TAR_REGISTRY]: {
type: 'router',
sources: ['bit', 'npmjs'],
tarballs: 'tar',
},
},
defaultRegistry: 'main',
routes: {
// The accelerated scenario asks the server to resolve against this
// address — its own un-proxied tar registry — so the resolver's metadata fetches
// stay on loopback instead of crossing the emulated link and coming back
// in through the front door (see applyPnprServerRegistry). The server
// resolves against whatever registry the client sends but only if that
// registry is declared, so the address has to be listed as a route here.
public: [{ registry: tarRegistryUrl(url) }],
},
}))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename)

populatePnprCache.js

[warning] 46-46: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(path.join(dir, 'pnpm-workspace.yaml'), yamlText({ packages: ['.'], minimumReleaseAge: 0 }), 'utf8')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename)

setupPackageManagers.js

[warning] 40-44: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(
path.join(setupDir, 'pnpm-workspace.yaml'),
yamlText({ packages: ['.'], minimumReleaseAge: 0 }),
'utf8'
)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename)

Additional comments: 6

benchmarkFixture.js: 1

225-247: LGTM!

Also applies to: 249-255, 308-317, 323-327, 330-330

benchmarkLockfileConflicts.js: 1

9-9: LGTM!

Also applies to: 119-122, 124-125

index.js: 1

9-9: LGTM!

Also applies to: 213-213

populatePnprCache.js: 1

8-8: LGTM!

Also applies to: 46-46

setupPackageManagers.js: 1

8-9: LGTM!

Also applies to: 42-42

pnprServer.js: 1

11-11: LGTM!

Also applies to: 122-123, 130-130, 136-136, 142-157, 227-277

[qodo-code-review]

PR Summary by Qodo

Serialize generated YAML configs as JSON to safely escape values

🐞 Bug fix ✨ Enhancement 🕐 20-40 Minutes

AI Description

• Replace hand-assembled YAML with object-based serialization to quote paths, URLs, and
 YAML-sensitive strings safely.
• Share an indented JSON-as-YAML helper across pnpr, pnpm, Yarn, and benchmark setup configs.
• Preserve existing settings and comments; leave INI-format .npmrc unchanged.

Diagram

graph TD
  A["Benchmark fixtures"] --> E(["yamlText helper"]) --> F["Generated config files"] --> G["Config readers"]
  B["pnpr server"] --> E --> F
  C["Conflict scenarios"] --> E --> F
  D["Setup scripts"] --> E --> F

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use the existing YAML writer

• ➕ Produces conventional YAML and supports YAML-specific values.
• ➖ Requires adapting synchronous write sites or adding a separate serialization path; these configs only need JSON-compatible values.

Recommendation: Keep the shared JSON-as-YAML helper. It safely escapes the values used here, preserves synchronous callers, and keeps serialization consistent without adding another API.

Files changed (6) +133 / -114

Refactor (6) +133 / -114

benchmarkFixture.jsAdd shared serializer for fixture and Yarn configs +47/-37

Add shared serializer for fixture and Yarn configs

• Adds 'yamlText' to emit indented JSON with a trailing newline. Builds pnpm workspace manifests and Yarn '.yarnrc.yml' from objects instead of interpolated YAML, preserving optional registry and scenario settings.

benchmarkFixture.js

benchmarkLockfileConflicts.jsSerialize conflict-scenario workspace settings +7/-4

Serialize conflict-scenario workspace settings

• Builds optional workspace globs and the v13 lockfile setting as an object. It still removes 'pnpm-workspace.yaml' when neither setting applies.

benchmarkLockfileConflicts.js

index.jsSerialize benchmark manager workspace manifest +2/-1

Serialize benchmark manager workspace manifest

• Uses 'yamlText' for the manager-installation workspace manifest, retaining 'minimumReleaseAge: 0' so installation can select recent releases.

index.js

pnprServer.jsSerialize pnpr server and registry-import configs +72/-70

Serialize pnpr server and registry-import configs

• Builds 'pnpr.yaml' and 'registries.yaml' as objects before writing them through 'yamlText'. Preserves server options, registry routing order, scope claims, and the public route.

pnprServer.js

populatePnprCache.jsSerialize cache-population workspace manifest +2/-1

Serialize cache-population workspace manifest

• Uses 'yamlText' for the workspace manifest in manager directories, retaining 'minimumReleaseAge: 0' during cache-population setup.

populatePnprCache.js

setupPackageManagers.jsSerialize bootstrap installer workspace manifest +3/-1

Serialize bootstrap installer workspace manifest

• Uses 'yamlText' when writing the bootstrap installer's workspace manifest, retaining the release-age override.

setupPackageManagers.js