Skip to content

docs(pnpr): authorize publication retries, clarify note URLs, store-enforced tombstones - #67

Merged
zkochan merged 5 commits into
mainfrom
rfc/implementation-clarifications
Nov 2, 2026
Merged

zkochan merged 5 commits into
mainfrom
rfc/implementation-clarifications

Conversation

@zkochan

@zkochan zkochan commented Nov 2, 2026 •

Copy link
Copy Markdown
Member

Summary

Clarifications found while implementing the merged RFCs in pnpm/pnpm#17553, pnpm/pnpm#17557 and pnpm/pnpm#17558:

  • Release notes: a publication entry's url includes any path prefix pnpr is served under. A client resolves it against the server origin and finds the note by url alone; org, id and name only identify the publication for display. The example's id is now a catalog generation (4182), which is what a publication id is.
  • Atomic workspace publication: a retry is authorized like any publication. The exception that returned a recorded outcome without current publish permission is gone. A principal whose publish permission was revoked after the commit gets the authorization error and reads the outcome from the publication or package instead. This matches how idempotency keys behave elsewhere, and a mutating route never returns what its own permission check refuses. The implementation no longer has to read a request body before rejecting a caller.
  • SQL metadata storage: the store refuses removed versions. The fragment cache relies on a version and revision always naming the same bytes. A publish-path tombstone check isn't enough, because recovering an interrupted publication bypasses it (found in review of feat(pnpr): keep hosted package metadata in SQLite pnpm#17551). The metadata store now keeps removed version identities and refuses to accept one again.

Written by an agent (Claude Code, claude-opus-5-5).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Clarified that publication IDs identify the catalog generation that committed them. Clients locate publication notes using the absolute server URL, including any path prefix; other publication details identify the publication for display.
    • Clarified that removed versions cannot be accepted again, including during recovery of an interrupted publication.
    • Updated retry rules: retries require current publication authorization. If authorization has been revoked, the retry returns an authorization error; read authorization is needed to check the recorded outcome. Reusing a key with a different request remains a catalog-scoped conflict.

zkochan and others added 2 commits November 2, 2026 10:44
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

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: 97cfc2ed-f3a4-4091-830b-d211e6796900
Commits

Reviewing files that changed from the base of the PR and between a223516 and f4490cd.

Files selected for processing: 2
  • pnpr/text/0000-atomic-workspace-publication.md
  • pnpr/text/0000-sql-metadata-storage.md

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

Recent review details
Context from checks skipped due to timeout: 1
  • GitHub Check: CodeRabbit
Additional comments: 2
pnpr/text/0000-atomic-workspace-publication.md: 1

113-114: LGTM!

Also applies to: 184-192

pnpr/text/0000-sql-metadata-storage.md: 1

128-128: Data Integrity & Integration

The recovery-time tombstone requirement is already defined.

The RFC explicitly requires interrupted-publication recovery to preserve the removed-version tombstone invariant. No further clarification is needed for this comment.

Walkthrough

The RFC text clarifies publication-note identifiers and URLs, metadata rules for removed versions, and authorization required to return a committed retry outcome.

Changes: Publication note contract

Layer / File(s) Summary
Publication fields
pnpr/text/0000-release-notes.md
The publication id is the catalog generation that committed it. Clients locate the note using the absolute-path url, including the pnpr serving prefix. The org, id, and name fields identify the publication for display.

Metadata storage invariant

Layer / File(s) Summary
Tombstone and cache invariant
pnpr/text/0000-sql-metadata-storage.md
The metadata store retains identities of removed versions and rejects their reacceptance, including during interrupted-publication recovery. The per-version cache key and rendering behavior remain unchanged.

Publication retry authorization

Layer / File(s) Summary
Authorized retry outcomes
pnpr/text/0000-atomic-workspace-publication.md
A matching retry returns its recorded outcome only after current publication authorization. Reusing the key for a different request remains a catalog-scoped conflict. A publisher whose permission was revoked must use read-authorized routes to learn the outcome.

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

Possibly related PRs

  • pnpm/rfcs#63: Defines the publication-note URL rule that this change also applies to publication entries.
  • pnpm/rfcs#60: Defines matching-retry outcomes, whose authorization rule this change updates.
  • pnpm/rfcs#64: Defines the retry authorization rule that this change reverses.
Merge Risk: Minimal · up to f4490

The clarifications have no identified merge-blocking risk.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title accurately summarizes the three main documentation changes: publication retry authorization, note URL clarification, and store-enforced tombstones. It is concise and specific.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)

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

A rabbit reads the notes at dawn,
The path and generation now are drawn.
Removed versions keep their names,
A matching retry checks its claims.
I hop past keys that do not align,
And nibble clover by the line.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Clarify publication note URLs and tombstone enforcement

📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Clarify that publication note URLs include pnpr’s serving prefix and are sufficient for retrieval.
• Identify publication IDs as catalog generations and align the example with that definition.
• Specify that the publish path checks tombstones before metadata storage to preserve cache
 assumptions.
Diagram

graph TD
  Index["Notes index"] --> URL["Publication URL"] --> Route["Publication note"]
  Publish["Publish path"] --> Check{"Tombstoned?"} -- "No" --> Store["Metadata store"] --> Cache["Fragment cache"]
  Check -- "Yes" --> Reject["Reject version"]
Loading
High-Level Assessment

Clarifying the existing RFC contracts is appropriate for this documentation-only PR. Deriving note URLs from registry bases or assigning tombstone enforcement to the metadata store would describe different behavior rather than resolve the implementation ambiguities.

Files changed (2) +19 / -14

Documentation (2) +19 / -14
0000-release-notes.mdDefine publication note URL and ID semantics +16/-13

Define publication note URL and ID semantics

• Specifies that a publication note URL includes any pnpr path prefix and is sufficient for clients to locate the note; the other publication fields serve identification and display. Changes the example ID and URL to use a catalog generation.

pnpr/text/0000-release-notes.md

0000-sql-metadata-storage.mdAssign tombstone checks to the publish path +3/-1

Assign tombstone checks to the publish path

• States that the publish path checks for tombstones before a version reaches metadata storage. Connects that check to the fragment cache’s assumption that a version and revision always identify the same bytes.

pnpr/text/0000-sql-metadata-storage.md

A principal whose publish permission was revoked after the commit reads the
outcome instead of replaying it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zkochan zkochan changed the title docs(pnpr): clarify publication note URLs and tombstone enforcement docs(pnpr): authorize publication retries, clarify note URLs and tombstones Nov 2, 2026

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix comments: Fix CodeRabbit comments on this PR

Recovery of an interrupted publication bypasses the publish path's tombstone
check, so the store itself must refuse a removed version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zkochan zkochan changed the title docs(pnpr): authorize publication retries, clarify note URLs and tombstones docs(pnpr): authorize publication retries, clarify note URLs, store-enforced tombstones Nov 2, 2026

@coderabbitai coderabbitai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Fix comments: Fix CodeRabbit comments on this PR

Comment thread pnpr/text/0000-sql-metadata-storage.md Resolved
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@zkochan
zkochan merged commit 3723c7a into main Nov 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant