Repository navigation
fix(publish): discover before authenticating so an empty registry no-ops - #579
Conversation
Release Preview — v0.40.0 → v0.41.1
Merging this PR will create a new release PR with the following changes: Changelogmonorepo 0.40.0 → 0.41.1
Updated automatically by ReleaseKit |
The dispatcher ran each registry's authCheck before discovering targets, so an enabled stage with nothing to publish still demanded credentials — and a crateless repo (cargo enabled by default since the ecosystem-enablement change) failed the whole release with "CARGO_REGISTRY_TOKEN not set" after npm had already published. Discover first; if there are no targets, no-op the stage (skip auth and dispose entirely); only authCheck when there is something to publish. This matches the already-stated "enabled-but-empty stays side-effect-free" intent, restores it for auth, and fixes cargo, npm, and pub uniformly. A repo that does have crates but no token still fails authCheck as before. `discover` no longer takes the (unused) session, since it now runs first. Closes #578 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011i1KvoyxATU3bz5V5Sf1Xu
37cf59a to
5041be0
Compare
|
Strengthened the test coverage: alongside the mock-registry ordering test, added one that drives the real |
Closes #578.
Problem
The publish dispatcher ran
registry.authCheck()beforeregistry.discover()and the zero-targets check. Since #563 flippedpublish.cargo.enabledto defaulttrue, every repo now runs the cargo stage — andcargo.authCheckthrows on a missingCARGO_REGISTRY_TOKEN. So a repo with no crates fails the whole release on cargo auth, after npm has already published (roll-forward), leaving it published-but-untagged and posting a false publish-failure report. ReleaseKit hit this on its own 0.41.0 release.Fix
dispatcher.ts— discover first; if there are no targets, no-op the stage (skip auth and dispose); onlyauthCheckwhen there's something to publish:This restores the interface's already-documented intent — "an enabled-but-empty stage stays side-effect-free" — which the auth ordering had violated.
Scope / safety
npm.authCheckalso throws pre-discovery); a zero-npm-package repo would hit it. Pre-existing, rarer; fixed by the same reorder.pub.authCheckis a no-op; token setup lives inprepare, gated on targets). No behavior change.authCheck(targets > 0) — the real misconfiguration is preserved.Registry.discoverno longer receivessession(no implementation used it).Tests
Added a dispatcher regression test: an enabled registry with zero targets never calls
authCheckand resolves cleanly even whenauthCheckwould throw. Full gate green (publish 282 tests;pnpm turbo run lint typecheck test32/32).Sequencing
This needs to be on main before the 0.41.0 standing-PR retry — the retry re-runs the publish, and without this fix it will fail on cargo auth again.
🤖 Generated with Claude Code
Greptile Summary
This PR fixes a regression introduced in #563 where
cargo.enableddefaulted totrue, causing every repo (even those with no crates) to fail on a missingCARGO_REGISTRY_TOKENbecauseauthCheckran beforediscover. The fix reorders the dispatcher sodiscoverruns first and an enabled-but-empty stage no-ops cleanly without demanding credentials.dispatcher.ts: Movesdiscover()ahead ofauthCheck(); exits early with a debug log whentargets.length === 0, skipping auth and dispose (no session is created for the empty case).types.ts: Drops the unusedsessionparameter from thediscoverinterface signature, reflecting that no implementation used it and it no longer makes sense pre-auth.dispatcher.spec.ts: Adds a generic regression test (enabled + zero targets → authCheck never called) and a real-cargoRegistrysmoke test that exercises the exact failure path with noCARGO_REGISTRY_TOKEN.Confidence Score: 5/5
Safe to merge — the change is a targeted reorder of two calls in a single function, backed by regression tests covering the exact broken scenario.
Discover runs first, auth is only demanded when there is something to publish, and the finally block that calls dispose is correctly scoped after authCheck so an empty stage never creates a session to clean up. Tests cover the generic dispatcher contract and the specific cargo regression.
No files require special attention.
Important Files Changed
sessionparameter from thediscoverinterface signature and updates the JSDoc to document the new pre-auth ordering guarantee.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant D as dispatcher participant R as Registry D->>R: isEnabled(config) alt disabled R-->>D: false D-->>D: log disabled, return else enabled R-->>D: true D->>R: discover(ctx) R-->>D: targets[] alt "targets.length === 0" D-->>D: debug No targets, return (no auth, no dispose) else "targets.length > 0" D->>R: authCheck(ctx) R-->>D: session D->>R: prepare?(ctx, session) loop for each target D->>R: precheckSkip? / isPublished / publish end D->>R: dispose?(session) end end%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant D as dispatcher participant R as Registry D->>R: isEnabled(config) alt disabled R-->>D: false D-->>D: log disabled, return else enabled R-->>D: true D->>R: discover(ctx) R-->>D: targets[] alt "targets.length === 0" D-->>D: debug No targets, return (no auth, no dispose) else "targets.length > 0" D->>R: authCheck(ctx) R-->>D: session D->>R: prepare?(ctx, session) loop for each target D->>R: precheckSkip? / isPublished / publish end D->>R: dispose?(session) end endReviews (2): Last reviewed commit: "fix(publish): discover before authentica..." | Re-trigger Greptile