Description

MakePublishManifestOptions no longer exposes readmeFile, so downstream consumers of the
published @pnpm/releasing.exportable-manifest package that pass readmeFile will break at compile
time and lose the ability to inject non-filesystem README content. The new `embedReadme +
projectDir` path only supports reading from disk.

Code

releasing/exportable-manifest/src/index.ts[R30-35]

hooks?: Hooks
modulesDir?: string
skipManifestObfuscation?: boolean
-  readmeFile?: string
+  embedReadme?: boolean
+  projectDir?: string
+}

Evidence

The public options type for createExportableManifest no longer includes readmeFile, and the
package is published under @pnpm/releasing.exportable-manifest, so external callers using the
removed option will break.

releasing/exportable-manifest/src/index.ts[28-35]
releasing/exportable-manifest/src/index.ts[83-90]
releasing/exportable-manifest/package.json[1-22]

Agent prompt

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

## Issue description
`@pnpm/releasing.exportable-manifest` is a published package, and this PR removes the public `readmeFile` option from `MakePublishManifestOptions`. This is a breaking change for external TypeScript consumers and also removes the capability to inject generated/virtual README content (the new API only supports reading a README from disk).
### Issue Context
The new implementation gates README I/O behind `embedReadme` and `projectDir`, but there is no longer an option to provide README content directly.
### Fix Focus Areas
- releasing/exportable-manifest/src/index.ts[28-90]
### Suggested fix
1. Re-introduce `readmeFile?: string` to `MakePublishManifestOptions` (keep it optional).
2. Update the embed logic to prefer the explicitly-provided `readmeFile` when `publishManifest.readme == null`:
- If `opts.readmeFile != null`, set `publishManifest.readme ??= opts.readmeFile`.
- Else, if `opts.embedReadme && (opts.projectDir ?? dir)`, read from disk.
3. (Optional) Mark `readmeFile` as deprecated in JSDoc if you want to migrate callers to lazy embedding over time, but keep it functional for compatibility.

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

Description

The new test unconditionally calls fs.promises.symlink(), which can throw on
platforms/environments that restrict symlink creation and will fail the test suite. This repo
already has platform-guarded filesystem tests, but this new test lacks such a guard.

Code

releasing/exportable-manifest/test/index.test.ts[R162-183]

+test('readme is not embedded when README.md is a symlink pointing outside the project', async () => {
+  const tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'pnpm-readme-'))
+  try {
+    const secretFile = path.join(tmpDir, 'secret.txt')
+    await fs.promises.writeFile(secretFile, 'secret content', 'utf8')
+    const projectDir = path.join(tmpDir, 'project')
+    await fs.promises.mkdir(projectDir)
+    await fs.promises.symlink(secretFile, path.join(projectDir, 'README.md'))
+
+    expect(await createExportableManifest(projectDir, {
+      name: 'foo',
+      version: '1.0.0',
+    }, {
+      ...defaultOpts,
+      embedReadme: true,
+    })).toStrictEqual({
+      name: 'foo',
+      version: '1.0.0',
+    })
+  } finally {
+    await fs.promises.rm(tmpDir, { recursive: true, force: true })
+  }

Evidence

The new test directly creates a symlink with no platform/permission guard, while other tests in this
repo explicitly skip on Windows when filesystem capabilities differ.

releasing/exportable-manifest/test/index.test.ts[162-184]
releasing/commands/test/publish/pack.ts[370-373]

Agent prompt

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

## Issue description
A newly added Jest test creates a symlink via `fs.promises.symlink(...)` without accounting for environments where symlink creation is disallowed (commonly Windows without Developer Mode/admin). This can make CI fail or tests flaky.
### Issue Context
The repo already uses platform guards for filesystem-behavior tests (e.g. skipping on win32).
### Fix Focus Areas
- releasing/exportable-manifest/test/index.test.ts[162-184]
### Suggested fix
- Skip the test on `process.platform === 'win32'`, **or**
- Attempt symlink creation and `test.skip()` (or early-return) when it fails with permission-related errors (e.g. `EPERM`).

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

Description

readReadmeFile() reads the first *readme.md match with fs.promises.readFile() without
verifying it is a regular file inside the project directory, so a README.md symlink can cause
arbitrary file contents to be embedded into publishManifest.readme. Since packPkg writes the
resulting manifest into package/package.json in the tarball, this can leak CI/local filesystem
data into the published artifact when embedReadme is enabled.

Code

releasing/exportable-manifest/src/index.ts[R36-41]

+async function readReadmeFile (projectDir: string): Promise<string | undefined> {
+  const files = await fs.promises.readdir(projectDir)
+  const readmePath = files.find(name => /readme\.md$/i.test(name))
+  const readmeFile = readmePath ? await fs.promises.readFile(path.join(projectDir, readmePath), 'utf8') : undefined
+
+  return readmeFile

Evidence

The README embedding path reads a filesystem entry without checking for symlinks/regular-file and
then places that content into the manifest, which is later written into the tarball's
package/package.json. This creates a concrete data-flow from disk file -> publishManifest.readme
-> published tarball manifest.

releasing/exportable-manifest/src/index.ts[36-41]
releasing/exportable-manifest/src/index.ts[84-89]
releasing/commands/src/publish/pack.ts[371-379]

Agent prompt

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

## Issue description
`readReadmeFile()` selects a README candidate from `readdir()` and reads it via `readFile(path.join(projectDir, readmePath))` without validating the resolved target is a regular file under `projectDir`. A `README.md` symlink (or a directory named `README.md`) can therefore cause unexpected reads and potentially leak content into the published `package.json` (because `packPkg` serializes the manifest into the tarball).
### Issue Context
This code runs only when `opts.embedReadme` is enabled and `publishManifest.readme` is missing, but in that case its output is embedded into the published tarball manifest.
### Fix Focus Areas
- releasing/exportable-manifest/src/index.ts[36-41]
- releasing/exportable-manifest/src/index.ts[84-89]
### Suggested fix
- Resolve and validate the README entry before reading:
- `const candidate = path.join(projectDir, readmePath)`
- Use `lstat()` to ensure it’s a regular file (and not a symlink). If you want to allow symlinks safely, resolve `realpath()` and ensure it stays within `realpath(projectDir)`.
- If invalid or unreadable (ENOENT/EISDIR), treat as “no README” (return `undefined`) rather than throwing, so `embedReadme` remains best-effort.

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

Description

createExportableManifest embeds README only when opts.embedReadme is true and opts.projectDir is
set, even though it already receives the package directory as the first argument. Because projectDir
is optional in MakePublishManifestOptions, callers can enable embedReadme and still silently get no
embedded README.

Code

releasing/exportable-manifest/src/index.ts[R85-87]

+  if (opts?.embedReadme && opts.projectDir) {
+    publishManifest.readme ??= await readReadmeFile(opts.projectDir)
}

Evidence

The README embedding logic requires opts.projectDir, but that field is optional in the options
type; meanwhile, the function already has dir available and pack.ts now passes the same directory
twice, indicating the API is easy to misuse and can silently skip embedding.

releasing/exportable-manifest/src/index.ts[28-35]
releasing/exportable-manifest/src/index.ts[45-48]
releasing/exportable-manifest/src/index.ts[85-87]
releasing/commands/src/publish/pack.ts[390-408]

Agent prompt

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

## Issue description
`createExportableManifest(dir, ...)` currently gates README embedding on `opts.projectDir`, which is optional, so `embedReadme: true` can be silently ignored.
### Issue Context
The function already receives `dir` and uses it for dependency conversions and hooks, but README reading uses only `opts.projectDir`.
### Fix Focus Areas
- releasing/exportable-manifest/src/index.ts[28-35]
- releasing/exportable-manifest/src/index.ts[45-48]
- releasing/exportable-manifest/src/index.ts[85-87]
### Suggested fix
- Prefer `dir` as the source of truth for README loading:
- `if (opts.embedReadme) { ... await readReadmeFile(dir) ... }`
- If you need a separate directory, make it non-optional when `embedReadme` is true (e.g., validate and throw if missing), or default to `opts.projectDir ?? dir`.

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

Walkthrough

README embedding is refactored so createExportableManifest reads README.md from the project directory when called with embedReadme: true; pack.ts no longer reads the README itself and tests create temporary README files to validate the new flow.

Changes

README Embedding Refactoring

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• zkochan

Poem

🐰 I nibble at README bits today,
Moving reading into the library's way.
Temp files sprout for tests to see,
Manifests grow lines of MD.
Hooray for cleaner passing play!

❌ Failed checks (1 warning)

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

• X
• Mastodon
• Reddit
• LinkedIn