max / makenotwork

Make artifact_download_redirect exercise the redirect The test's tail was nineteen lines of deliberation concluding the redirect could not be reached, and the function stopped at the artifact-create assert. Both halves of the premise are wrong now: BuildOptions takes a synckit_storage handle the caller keeps (synckit_paid_sync.rs already does this), and InMemoryStorage::put stands in for the client PUT the register route only presigns. So it asserts what it is named for: 302 to a presigned URL for the artifact's own key, plus a 404 for a target with no artifact. The deliberation is gone. GoingsOn mnw-server problem a5989f50.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: Max Johnson <me@maxj.phd> · 2026-08-23 20:26 UTC

Signed with PGP, not checked

Commit: bcf7dc34082689a6ae1636fb9a1a52113247fe04

Parent: 0dca20c

1 file changed, +47 insertions, -8 deletions

M server/tests/workflows/ota.rs +47 -8

			@@ -1,10 +1,11 @@
1	1		//! OTA update integration tests, slug management, releases, artifacts, updater endpoint.
2	2
3		-	use crate::harness::TestHarness;
	3	+	use crate::harness::{BuildOptions, TestHarness, storage::InMemoryStorage};
4	4		use makenotwork::db::{OtaReleaseId, SyncAppId, UserId};
5	5		use serde::Deserialize;
6	6		use serde_json::json;
7	7		use sqlx::PgPool;
	8	+	use std::sync::Arc;
8	9
9	10		/// The artifact-register route rejects signatures shorter than 40 chars (a real
10	11		/// base64-encoded minisign signature is well over that), so any test that
			@@ -38,6 +39,7 @@
38	39		#[derive(Deserialize)]
39	40		struct UploadArtifactResponse {
40	41		upload_url: String,
	42	+	s3_key: String,
41	43		}
42	44
43	45		#[derive(Deserialize)]
			@@ -123,6 +125,19 @@
123	125		TestHarness::with_synckit_storage().await
124	126		}
125	127
	128	+	/// Same harness, with the storage handle kept. The register-artifact route only
	129	+	/// presigns, so nothing ever lands in the backend; a download test has to put
	130	+	/// the object there itself, standing in for the client PUT.
	131	+	async fn harness_with_synckit_blobs() -> (TestHarness, Arc<InMemoryStorage>) {
	132	+	let mem = Arc::new(InMemoryStorage::new());
	133	+	let h = TestHarness::build(BuildOptions {
	134	+	synckit_storage: Some(mem.clone()),
	135	+	..Default::default()
	136	+	})
	137	+	.await;
	138	+	(h, mem)
	139	+	}
	140	+
126	141		/// Mark a release's artifacts scanned-clean (simulates a completed scan). New
127	142		/// artifacts start `pending` and are not advertised/downloadable until the scan
128	143		/// pipeline clears them; serve-path tests call this to reach the served state.
			@@ -812,7 +827,7 @@
812	827
813	828		#[tokio::test]
814	829		async fn artifact_download_redirect() {
815		-	let mut h = harness_with_synckit_storage().await;
	830	+	let (mut h, blobs) = harness_with_synckit_blobs().await;
816	831		let user_id = h.signup("otauser", "ota@example.com", "Password1!").await;
817	832		let (app_id, api_key) = create_sync_app_with_slug(&h.db, user_id, "dlapp").await;
818	833
			@@ -853,14 +868,38 @@
853	868		)
854	869		.await;
855	870		assert_eq!(resp.status, 201);
	871	+	let upload: UploadArtifactResponse = resp.json();
856	872
857		-	// The s3_key the download route would presign.
858		-	let _s3_key = format!("ota/{app_id}/1.0.0/linux/x86_64/artifact");
	873	+	// The client PUT the route presigned for, performed against the backend
	874	+	// directly: `presign_download` refuses a key with no object behind it, and
	875	+	// the download route checks `object_exists` before it presigns anything.
	876	+	blobs.put(&upload.s3_key, b"artifact bytes".to_vec());
	877	+	mark_artifacts_clean(&h.db, &release.id).await;
859	878
860		-	// The redirect itself is not exercised: `InMemoryStorage::presign_download`
861		-	// checks the object exists, the upload endpoint only presigns, and the
862		-	// harness does not expose `synckit_storage` for a manual injection. This
863		-	// test covers release and artifact creation only.
	879	+	let resp = h
	880	+	.client
	881	+	.get(&format!(
	882	+	"/api/sync/ota/dlapp/download/{}/linux/x86_64",
	883	+	release.id
	884	+	))
	885	+	.await;
	886	+	assert_eq!(resp.status, 302, "download: {}", resp.text);
	887	+	let expected = format!("http://test-storage/{}", upload.s3_key);
	888	+	assert_eq!(
	889	+	resp.header("location"),
	890	+	Some(expected.as_str()),
	891	+	"redirects to a presigned URL for the artifact's own key"
	892	+	);
	893	+
	894	+	// A target with no artifact 404s rather than redirecting somewhere wrong.
	895	+	let resp = h
	896	+	.client
	897	+	.get(&format!(
	898	+	"/api/sync/ota/dlapp/download/{}/darwin/aarch64",
	899	+	release.id
	900	+	))
	901	+	.await;
	902	+	assert_eq!(resp.status, 404, "no darwin artifact: {}", resp.text);
864	903		}
865	904
866	905		#[tokio::test]