fix(publish): an accepted npm upload no longer fails on a slow readback

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

packages/zig/src/cli/commands/package.zigmodified+96-11

Changes to packages/zig/src/cli/commands/package.zig

		⋯777 linesShow the 777 lines above this hunk@@ -778,6 +778,10 @@ pub const PublishOptions = struct {
778	778	/// stx release spent 100 to 120 seconds on each of its packages in turn,
779	779	/// three stages of ten minutes - and queued, those delays overlap.
780	780	deferred_readbacks: ?*ReadbackQueue = null,
	781	+ /// How long to wait for an accepted upload to become public before
	782	+ /// reporting it pending, in milliseconds (`--confirm-timeout`, seconds).
	783	+ /// Null uses `npm_readback_timeout_ms`.
	784	+ confirm_timeout_ms: ?u64 = null,
781	785	};
782	786
783	787	pub const DeferredReadback = publish_plan.DeferredReadback;
		⋯334 linesShow the 334 lines above this hunk@@ -1118,6 +1122,34 @@ test "a bounded timeout before sending an upload is retryable" {
1118	1122	try std.testing.expect(isTransientNetworkError(error.Timeout));
1119	1123	}
1120	1124
	1125	+test "the public readback has its own window, not the upload socket's" {
	1126	+ // The regression this pins: both were `npm_publish_timeout_ms`, so a
	1127	+ // readback gave up after the time allotted to a stalled PUT and reported
	1128	+ // a version that did go live as a failure (#245).
	1129	+ try std.testing.expect(npm_readback_timeout_ms != npm_publish_timeout_ms);
	1130	+ try std.testing.expect(npm_readback_timeout_ms > npm_publish_timeout_ms);
	1131	+
	1132	+ // Default, and --confirm-timeout overriding it.
	1133	+ try std.testing.expectEqual(npm_readback_timeout_ms, readbackTimeoutMs(.{}));
	1134	+ try std.testing.expectEqual(@as(u64, 1_800_000), readbackTimeoutMs(.{ .confirm_timeout_ms = 1_800_000 }));
	1135	+}
	1136	+
	1137	+test "an accepted upload completes even when npm has not shown it yet" {
	1138	+ const allocator = std.testing.allocator;
	1139	+
	1140	+ // npm accepted the bytes and the readback timed out. That used to be
	1141	+ // exit 1, which skipped the release binaries and the GitHub release for a
	1142	+ // version that went live a minute later — and the message forbids
	1143	+ // re-uploading, so there was no safe way to re-run (#245).
	1144	+ var pending = completeNpmPublication(allocator, "demo", "1.0.0", null, ".", .{}, false);
	1145	+ defer pending.deinit(allocator);
	1146	+ try std.testing.expectEqual(@as(u8, 0), pending.exit_code);
	1147	+
	1148	+ var confirmed = completeNpmPublication(allocator, "demo", "1.0.0", null, ".", .{}, true);
	1149	+ defer confirmed.deinit(allocator);
	1150	+ try std.testing.expectEqual(@as(u8, 0), confirmed.exit_code);
	1151	+}
	1152	+
1121	1153	/// Reason a package should be skipped when publishing to npm, or null to publish.
1122	1154	///
1123	1155	/// Native (Zig) packages live on the Pantry registry, not npm: their source is
		⋯34 linesShow the 34 lines above this hunk@@ -1158,9 +1190,45 @@ fn completeVerifiedNpmPublication(
1158	1190	scripts: ?std.json.ObjectMap,
1159	1191	package_dir: []const u8,
1160	1192	options: PublishOptions,
	1193	+) CommandResult {
	1194	+ return completeNpmPublication(allocator, name, version, scripts, package_dir, options, true);
	1195	+}
	1196	+
	1197	+/// Finish an upload npm accepted, whether or not it is public yet.
	1198	+///
	1199	+/// A readback that times out used to return exit 1. But npm had accepted the
	1200	+/// bytes: the version lands, and the message says not to upload it again — so
	1201	+/// the only effect was to skip every release step after the publish (binaries,
	1202	+/// GitHub release) for a version that went live a minute later, with no safe
	1203	+/// way to re-run (#245). An accepted upload therefore completes, and the
	1204	+/// pending line says what is still outstanding.
	1205	+///
	1206	+/// This applies ONLY where npm accepted this run's upload. An upload whose
	1207	+/// outcome is unknown — a socket that died after the bytes, a 5xx or a
	1208	+/// redirect over a committed write — still fails, because there the version
	1209	+/// may never appear and a release built on it would reference nothing.
	1210	+///
	1211	+/// A mismatch is not pending either: `confirmPublication` returns
	1212	+/// `error.PublicationIntegrityMismatch` rather than false when the public
	1213	+/// tarball is not the one we sent, and that error propagates to a failure.
	1214	+/// What reaches `confirmed = false` is only "npm has not shown it yet".
	1215	+fn completeNpmPublication(
	1216	+ allocator: std.mem.Allocator,
	1217	+ name: []const u8,
	1218	+ version: []const u8,
	1219	+ scripts: ?std.json.ObjectMap,
	1220	+ package_dir: []const u8,
	1221	+ options: PublishOptions,
	1222	+ confirmed: bool,
1161	1223	) CommandResult {
1162	1224	const queued = if (options.deferred_readbacks) |queue| queue.items.len > 0 and std.mem.eql(u8, queue.items[queue.items.len - 1].name, name) and std.mem.eql(u8, queue.items[queue.items.len - 1].version, version) else false;
1163		- style.print("\n✓ {s} {s}@{s}\n", .{ if (queued) "Uploaded" else "Published", name, version });
	1225	+ if (!confirmed) {
	1226	+ // The same text release tooling already recognises, kept verbatim and
	1227	+ // still printed under this package's receipt — as a warning now, not
	1228	+ // an error, because the publish itself succeeded.
	1229	+ style.print(" {s}⚠{s} {s}\n", .{ style.yellow, style.reset, publish_plan.pending_publication });
	1230	+ }
	1231	+ style.print("\n✓ {s} {s}@{s}\n", .{ if (queued or !confirmed) "Uploaded" else "Published", name, version });
1164	1232	if (!options.ignore_scripts) {
1165	1233	if (scripts) |configured| {
1166	1234	_ = lib.lifecycle.runPostPublishScripts(allocator, configured, package_dir);
		⋯80 linesShow the 80 lines above this hunk@@ -1247,7 +1315,7 @@ fn publishSingleToNpm(
1247	1315	.message = try std.fmt.allocPrint(allocator, "Public version availability could not be verified: {s}. Upload was not started.", .{@errorName(err)}),
1248	1316	};
1249	1317	if (exists) {
1250		- if (!try precheck_client.confirmPublication(metadata.name, metadata.version, null, npm_publish_timeout_ms)) {
	1318	+ if (!try precheck_client.confirmPublication(metadata.name, metadata.version, null, readbackTimeoutMs(options))) {
1251	1319	return CommandResult.err(allocator, "Existing version is pending public tarball availability. Do not upload this version again.");
1252	1320	}
1253	1321	style.print("\n{s}↷{s} {s}@{s} already published — skipping\n", .{ style.dim, style.reset, metadata.name, metadata.version });
		⋯122 linesShow the 122 lines above this hunk@@ -1376,7 +1444,7 @@ fn publishSingleToNpm(
1376	1444	const deferred = options.readback and options.deferred_readbacks != null;
1377	1445	if (deferred) try deferReadback(allocator, options.deferred_readbacks.?, metadata.name, metadata.version, tarball_info.shasum, tarball_info.integrity, registry_url);
1378	1446	const confirm_started = io_helper.getMilliTimestamp();
1379		- const confirmed = !options.readback or deferred or try registry_client.confirmPublication(metadata.name, metadata.version, &tarball_info.integrity, npm_publish_timeout_ms);
	1447	+ const confirmed = !options.readback or deferred or try registry_client.confirmPublication(metadata.name, metadata.version, &tarball_info.integrity, readbackTimeoutMs(options));
1380	1448	const confirm_ms = io_helper.getMilliTimestamp() - confirm_started;
1381	1449	if (deferred)
1382	1450	style.print(" {s}accepted by npm; read back once a package needs it or the batch is uploaded{s}\n", .{ style.dim, style.reset })
		⋯2 linesShow the 2 lines above this hunk@@ -1385,7 +1453,7 @@ fn publishSingleToNpm(
1385	1453	else
1386	1454	style.print(" {s}accepted by npm; public readback left to the caller (--no-readback){s}\n", .{ style.dim, style.reset });
1387	1455	if (!confirmed) {
1388		- return CommandResult.err(allocator, publish_plan.pending_publication);
	1456	+ return completeNpmPublication(allocator, metadata.name, metadata.version, scripts_obj, package_dir, options, false);
1389	1457	}
1390	1458	// A queued readback reports "Published" when it confirms.
1391	1459	const done = if (deferred) "Uploaded" else "Published";
		⋯28 linesShow the 28 lines above this hunk@@ -1420,7 +1488,7 @@ fn publishSingleToNpm(
1420	1488	// for it contradicts the pre-check and, mid-release, strands
1421	1489	// the packages that already went out.
1422	1490	if (options.skip_existing) {
1423		- if (!try registry_client.confirmPublication(metadata.name, metadata.version, null, npm_publish_timeout_ms)) {
	1491	+ if (!try registry_client.confirmPublication(metadata.name, metadata.version, null, readbackTimeoutMs(options))) {
1424	1492	return CommandResult.err(allocator, "Claimed version is pending public metadata and tarball availability. Do not upload this version again.");
1425	1493	}
1426	1494	style.print("\n{s}↷{s} {s}@{s} already claimed on npm — skipping\n", .{ style.dim, style.reset, metadata.name, metadata.version });
		⋯34 linesShow the 34 lines above this hunk@@ -1461,7 +1529,7 @@ fn publishSingleToNpm(
1461	1529	// before the upload — no CI provider, a token npm would not
1462	1530	// exchange for a package with no trusted publisher — which used to
1463	1531	// wait two minutes per package for a version that was never sent.
1464		- if (result.sent and try registry_client.confirmPublication(metadata.name, metadata.version, &tarball_info.integrity, npm_publish_timeout_ms)) {
	1532	+ if (result.sent and try registry_client.confirmPublication(metadata.name, metadata.version, &tarball_info.integrity, readbackTimeoutMs(options))) {
1465	1533	style.print(
1466	1534	"\n{s}✓{s} {s}@{s} is on the registry — the publish landed, the response did not\n",
1467	1535	.{ style.green, style.reset, metadata.name, metadata.version },
		⋯89 linesShow the 89 lines above this hunk@@ -1557,7 +1625,7 @@ fn publishSingleToNpm(
1557	1625	// A request can commit before its response fails. Never send a
1558	1626	// second PUT after bytes may have reached the registry.
1559	1627	if (registry_client.token_upload_started) {
1560		- if (try registry_client.confirmPublication(metadata.name, metadata.version, &tarball_info.integrity, npm_publish_timeout_ms)) {
	1628	+ if (try registry_client.confirmPublication(metadata.name, metadata.version, &tarball_info.integrity, readbackTimeoutMs(options))) {
1561	1629	return completeVerifiedNpmPublication(allocator, metadata.name, metadata.version, scripts_obj, package_dir, options);
1562	1630	}
1563	1631	return CommandResult.err(allocator, "Upload outcome is pending public verification. Do not send another upload while this version may be staged.");
		⋯32 linesShow the 32 lines above this hunk@@ -1596,7 +1664,7 @@ fn publishSingleToNpm(
1596	1664	if (r.status_code == 408 or (r.status_code >= 500 and r.status_code < 600) or (r.status_code >= 300 and r.status_code < 400)) {
1597	1665	var owned_response = r;
1598	1666	defer owned_response.deinit(allocator);
1599		- if (try registry_client.confirmPublication(metadata.name, metadata.version, &tarball_info.integrity, npm_publish_timeout_ms)) {
	1667	+ if (try registry_client.confirmPublication(metadata.name, metadata.version, &tarball_info.integrity, readbackTimeoutMs(options))) {
1600	1668	return completeVerifiedNpmPublication(allocator, metadata.name, metadata.version, scripts_obj, package_dir, options);
1601	1669	}
1602	1670	return CommandResult.err(allocator, "Upload outcome is pending public verification. Do not send another upload while this version may be staged.");
		⋯33 linesShow the 33 lines above this hunk@@ -1636,8 +1704,8 @@ fn publishSingleToNpm(
1636	1704	if (options.readback and options.deferred_readbacks != null) {
1637	1705	try deferReadback(allocator, options.deferred_readbacks.?, metadata.name, metadata.version, tarball_info.shasum, tarball_info.integrity, registry_url);
1638	1706	style.print(" {s}accepted by npm; read back once a package needs it or the batch is uploaded{s}\n", .{ style.dim, style.reset });
1639		- } else if (options.readback and !try registry_client.confirmPublication(metadata.name, metadata.version, &tarball_info.integrity, npm_publish_timeout_ms)) {
1640		- return CommandResult.err(allocator, publish_plan.pending_publication);
	1707	+ } else if (options.readback and !try registry_client.confirmPublication(metadata.name, metadata.version, &tarball_info.integrity, readbackTimeoutMs(options))) {
	1708	+ return completeNpmPublication(allocator, metadata.name, metadata.version, scripts_obj, package_dir, options, false);
1641	1709	}
1642	1710	return completeVerifiedNpmPublication(allocator, metadata.name, metadata.version, scripts_obj, package_dir, options);
1643	1711	} else {
		⋯18 linesShow the 18 lines above this hunk@@ -1662,7 +1730,7 @@ fn publishSingleToNpm(
1662	1730	// See the OIDC path above: under `skip_existing` a conflict is the
1663	1731	// pre-check losing a race, not a mistake worth failing over.
1664	1732	if (options.skip_existing) {
1665		- if (!try registry_client.confirmPublication(metadata.name, metadata.version, null, npm_publish_timeout_ms)) {
	1733	+ if (!try registry_client.confirmPublication(metadata.name, metadata.version, null, readbackTimeoutMs(options))) {
1666	1734	return CommandResult.err(allocator, "Claimed version is pending public metadata and tarball availability. Do not upload this version again.");
1667	1735	}
1668	1736	const why: []const u8 = if (conflict_kind == .staged)
		⋯232 linesShow the 232 lines above this hunk@@ -1901,6 +1969,23 @@ const npm_publish_timeout_ms: u64 = 120_000;
1901	1969	/// that the upload should be sent again.
1902	1970	const deferred_readback_timeout_ms: u64 = 600_000;
1903	1971
	1972	+/// How long an immediate readback waits for npm to make an accepted upload
	1973	+/// public. Separate from `npm_publish_timeout_ms`, which bounds the upload
	1974	+/// socket: the two were the same value, so a readback gave up after the time
	1975	+/// allotted to a stalled PUT. npm's propagation regularly passes two minutes
	1976	+/// and has been observed around 25 minutes on a Craft release, which is what
	1977	+/// made `publish --npm` exit 1 on a version that did go live and skip every
	1978	+/// release step after it (#245). The batch path had already been raised to
	1979	+/// ten minutes for the same reason; this matches it. `--confirm-timeout`
	1980	+/// overrides it where a publisher knows its registry is slower still.
	1981	+const npm_readback_timeout_ms: u64 = 600_000;
	1982	+
	1983	+/// The readback window for this publish: `--confirm-timeout`, else the
	1984	+/// default above.
	1985	+fn readbackTimeoutMs(options: PublishOptions) u64 {
	1986	+ return options.confirm_timeout_ms orelse npm_readback_timeout_ms;
	1987	+}
	1988	+
1904	1989	const OIDCPublishTaskResult = anyerror!OIDCPublishResult;
1905	1990
1906	1991	fn attemptOIDCPublishTask(

packages/zig/src/lib.zigmodified+6-0

Changes to packages/zig/src/lib.zig

		⋯69 linesShow the 69 lines above this hunk@@ -70,6 +70,12 @@ test {
70	70	_ = ;
71	71	_ = ;
72	72	_ = ;
	73	+ // package.zig's own tests were never reached: refAllDecls does not pull
	74	+ // them in and nothing imported it for its own sake, so its five existing
	75	+ // tests had been dead since they were written. This is why publish_plan.zig
	76	+ // and publish_conflict.zig exist as separate files — testable logic was
	77	+ // moved out of package.zig rather than making its tests run.
	78	+ _ = ;
73	79	_ = ;
74	80	_ = ;
75	81	_ = ;

packages/zig/src/main.zigmodified+15-0

Changes to packages/zig/src/main.zig

		⋯898 linesShow the 898 lines above this hunk@@ -899,6 +899,17 @@ fn whoamiAction(ctx: *cli.BaseCommand.ParseContext) !void {
899	899	std.process.exit(result.exit_code);
900	900	}
901	901
	902	+/// `--confirm-timeout <seconds>` as milliseconds, or null for the default.
	903	+///
	904	+/// Seconds at the command line because the thing being waited on is npm's
	905	+/// propagation delay, which publishers talk about in minutes.
	906	+fn confirmTimeoutMs(ctx: *cli.BaseCommand.ParseContext) ?u64 {
	907	+ const raw = ctx.getOption("confirm-timeout") orelse return null;
	908	+ const seconds = std.fmt.parseInt(u64, std.mem.trim(u8, raw, " \t"), 10) catch return null;
	909	+ if (seconds == 0) return null;
	910	+ return seconds * 1000;
	911	+}
	912	+
902	913	fn publishAction(ctx: *cli.BaseCommand.ParseContext) !void {
903	914	const allocator = ctx.allocator;
904	915
		⋯24 linesShow the 24 lines above this hunk@@ -929,6 +940,7 @@ fn publishAction(ctx: *cli.BaseCommand.ParseContext) !void {
929	940	.skip_existing = !force_republish,
930	941	.ignore_scripts = ignore_scripts,
931	942	.readback = !ctx.hasOption("no-readback"),
	943	+ .confirm_timeout_ms = confirmTimeoutMs(ctx),
932	944	};
933	945
934	946	const result = try lib.commands.publishCommand(allocator, &[_][]const u8{}, options);
		⋯493 linesShow the 493 lines above this hunk@@ -1428,6 +1440,7 @@ fn registryPublishAction(ctx: *cli.BaseCommand.ParseContext) !void {
1428	1440	.skip_existing = !force_republish,
1429	1441	.ignore_scripts = ignore_scripts,
1430	1442	.readback = !ctx.hasOption("no-readback"),
	1443	+ .confirm_timeout_ms = confirmTimeoutMs(ctx),
1431	1444	};
1432	1445
1433	1446	const result = try lib.commands.publishCommand(allocator, paths.items, npm_options);
		⋯2,190 linesShow the 2,190 lines above this hunk@@ -3624,6 +3637,7 @@ pub fn main() !void {
3624	3637	_ = try npm_publish_cmd.addOption(npm_force_republish_opt);
3625	3638	const npm_no_readback_opt = cli.Option.init("no-readback", "no-readback", "Report an accepted upload without reading it back from the public registry (for a caller that verifies every package itself)", .bool);
3626	3639	_ = try npm_publish_cmd.addOption(npm_no_readback_opt);
	3640	+ _ = try npm_publish_cmd.addOption(cli.Option.init("confirm-timeout", "confirm-timeout", "Seconds to wait for an accepted upload to become public before reporting it pending (default 600)", .string));
3627	3641
3628	3642	const npm_ignore_scripts_opt = cli.Option.init("ignore-scripts", "ignore-scripts", "Don't run package lifecycle scripts (prepublishOnly/prepack/etc.); use with a separate build step", .bool);
3629	3643	_ = try npm_publish_cmd.addOption(npm_ignore_scripts_opt);
		⋯1,081 linesShow the 1,081 lines above this hunk@@ -4711,6 +4725,7 @@ pub fn main() !void {
4711	4725	_ = try publish_cmd.addOption(pub_ignore_scripts_opt);
4712	4726	const pub_no_readback_opt = cli.Option.init("no-readback", "no-readback", "Report an accepted npm upload without reading it back from the public registry (for a caller that verifies every package itself)", .bool);
4713	4727	_ = try publish_cmd.addOption(pub_no_readback_opt);
	4728	+ _ = try publish_cmd.addOption(cli.Option.init("confirm-timeout", "confirm-timeout", "Seconds to wait for an accepted npm upload to become public before reporting it pending (default 600)", .string));
4714	4729
4715	4730	const pub_paths_arg = cli.Argument.init("paths", "Package directories or globs to publish (e.g., './packages/*'); defaults to auto-detecting packages/", .string)
4716	4731	.withRequired(false)