⋯ 777 lines Show 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 lines Show 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 lines Show 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 lines Show 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 lines Show 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 lines Show 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 lines Show 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 lines Show 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 lines Show 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 lines Show 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 lines Show 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 lines Show 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 lines Show 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(