update: releases come from one repository; retire the former-name fallback

docs/changes/unreleased/1832-one-release-repository.md

@@ -0,0 +1,15 @@
+---
+kind: removed
+title: releases come from one repository, and a 404 there is final
+pr: 1832
+surface: [build]
+invalidates:
+  - "After a 404 from Agent-Field/codeaf, the updater (selecting a release, looking up a tag's publish moment, downloading) and scripts/install.sh asked the repository's former name as a second source. They no longer do: a 404 from Agent-Field/codeaf ends the selection or install, and no other repository is asked."
+  - "The installer used whatever repository a caller wrote on the release. It now always downloads from Agent-Field/codeaf, so no caller can point an install at a second origin."
+---
+
+The former name is only GitHub's redirect to the renamed repository (the two
+report one repository id), so it never held a release the real one lacked; the
+fallback could only ever have reached a repository created under that name
+later. The former ASSET name is still tried inside the same release, because
+v0.1.0, the one stable published before the rename, carries it.

internal/release/install_test.go

@@ -291,25 +291,37 @@ func TestInstallerGetsLatestStableAndFinishesWithVersion(t *testing.T) {
 	}
 }
 
-// H10: a 404 from the current repository makes release discovery retry the
-// former repository and complete the install from there.
-func TestH10InstallerFallsBackToLegacyRepository(t *testing.T) {
-	github := newInstallGitHub(t, "v1.2.3")
-	github.oldRepositoryOnly = true
-	run := runInstaller(t, github, nil, "CODEAF_NO_MODIFY_PATH=1")
-	if run.code != 0 {
-		t.Fatalf("fallback install exit %d:\n%s", run.code, run.output)
-	}
+// onlyTheReleaseRepository fails the test for any request that named a
+// repository other than the one releases come from.
+func onlyTheReleaseRepository(t *testing.T, github *installGitHub) {
+	t.Helper()
 	github.mu.Lock()
 	defer github.mu.Unlock()
-	var current, legacy bool
+	if len(github.requests) == 0 {
+		t.Fatal("the installer asked nothing")
+	}
 	for _, request := range github.requests {
-		current = current || request.URL.Path == "/repos/Agent-Field/codeaf/releases/latest"
-		legacy = legacy || request.URL.Path == "/repos/Agent-Field/aforge-v2/releases/latest" // legacy-name
+		path := request.URL.Path
+		if !strings.HasPrefix(path, "/repos/Agent-Field/codeaf/") && !strings.HasPrefix(path, "/Agent-Field/codeaf/") {
+			t.Fatalf("the installer asked another repository: %s", path)
+		}
 	}
-	if !current || !legacy {
-		t.Fatalf("repository requests current=%v legacy=%v", current, legacy)
+}
+
+// THE INSTALLER ASKS ONE REPOSITORY. The fake answers under the former name
+// too, as a repository squatting on it could; a 404 from the real one still
+// ends the install, and nothing is asked of the other.
+func TestInstallerAsksOnlyTheReleaseRepository(t *testing.T) {
+	github := newInstallGitHub(t, "v1.2.3")
+	github.oldRepositoryOnly = true
+	run := runInstaller(t, github, nil, "CODEAF_NO_MODIFY_PATH=1")
+	if run.code == 0 {
+		t.Fatalf("an install with no release in the release repository succeeded:\n%s", run.output)
 	}
+	if _, err := os.Stat(filepath.Join(run.installDir, "codeaf")); err == nil {
+		t.Fatal("a binary was written although the release repository had none")
+	}
+	onlyTheReleaseRepository(t, github)
 }
 
 // H10: a release containing only the former asset spelling is installed under
@@ -327,24 +339,27 @@ func TestH10InstallerFallsBackToLegacyAssetName(t *testing.T) {
 	}
 }
 
-func TestPinnedFormerReleaseKeepsAssetAndChecksumInTheFormerRepository(t *testing.T) {
-	github := newInstallGitHub(t, "v0.2.0")
-	github.oldRepositoryOnly = true
+// A release published before the rename carries the former asset name; pinning
+// it still installs it, with its asset and checksum both read from the release
+// repository.
+func TestPinnedFormerReleaseTakesItsFormerAssetFromTheReleaseRepository(t *testing.T) {
+	github := newInstallGitHub(t, "v0.1.0")
 	github.legacyAssetOnly = true
-	run := runInstaller(t, github, []string{"--version", "v0.2.0"}, "CODEAF_NO_MODIFY_PATH=1")
+	run := runInstaller(t, github, []string{"--version", "v0.1.0"}, "CODEAF_NO_MODIFY_PATH=1")
 	if run.code != 0 {
-		t.Fatalf("pinned compatibility install exit %d:\n%s", run.code, run.output)
+		t.Fatalf("pinned former release exit %d:\n%s", run.code, run.output)
 	}
 	github.mu.Lock()
-	defer github.mu.Unlock()
 	var asset, checksum bool
 	for _, request := range github.requests {
-		asset = asset || strings.Contains(request.URL.Path, "/Agent-Field/aforge-v2/releases/download/v0.2.0/aforge-") // legacy-name
-		checksum = checksum || request.URL.Path == "/Agent-Field/aforge-v2/releases/download/v0.2.0/checksums.txt"     // legacy-name
+		asset = asset || strings.Contains(request.URL.Path, "/Agent-Field/codeaf/releases/download/v0.1.0/aforge-") // legacy-name
+		checksum = checksum || request.URL.Path == "/Agent-Field/codeaf/releases/download/v0.1.0/checksums.txt"
 	}
+	github.mu.Unlock()
 	if !asset || !checksum {
-		t.Fatalf("former repository asset=%v checksum=%v", asset, checksum)
+		t.Fatalf("release repository asset=%v checksum=%v", asset, checksum)
 	}
+	onlyTheReleaseRepository(t, github)
 }
 
 func TestInstallerRunsAdoptionBeforeCreatingTheStateRoot(t *testing.T) {

internal/update/client.go

@@ -79,8 +79,13 @@ const (
 	downloadCeiling     = 15 * time.Minute
 )
 
+// primaryRepository is THE ONE ORIGIN a release is selected from and
+// downloaded from. The repository's former name is not a second one: GitHub
+// answers it only as a redirect to this repository, so it never held a release
+// this one lacks, and asking it after a 404 here could only ever reach a
+// repository somebody created under that name later — a stranger to every
+// checksum this program trusts.
 const primaryRepository = "Agent-Field/codeaf"
-const legacyRepository = "Agent-Field/aforge-v2" // legacy-name
 
 // Client talks to the release API and release asset host.
 type Client struct {
@@ -318,7 +323,9 @@ type Choice struct {
 	Published time.Time
 }
 
-// Release is one selected GitHub release and the repository that answered.
+// Release is one selected GitHub release. Repository names where it was
+// selected and is always [primaryRepository]; it is a record of that fact and
+// never an address — nothing reads it to decide where to ask.
 type Release struct {
 	Tag                string
 	Repository         string
@@ -354,16 +361,7 @@ func (c *Client) Select(ctx context.Context, choice Choice) (Release, error) {
 	if channel != "stable" && channel != "rc" && channel != "dev" && channel != "staging" {
 		return Release{}, fmt.Errorf("channel must be stable, rc, dev, or staging")
 	}
-	for index, repository := range []string{primaryRepository, legacyRepository} { // legacy-name
-		release, err := c.selectRepository(ctx, repository, channel, strings.TrimSpace(choice.Running))
-		if err == nil {
-			return release, nil
-		}
-		if !isStatus(err, http.StatusNotFound) || index == 1 {
-			return Release{}, err
-		}
-	}
-	return Release{}, errors.New("no release repository answered")
+	return c.selectRelease(ctx, channel, strings.TrimSpace(choice.Running))
 }
 
 // TagPublished asks the release API for ONE exact tag's publish moment, and it
@@ -385,34 +383,28 @@ func (c *Client) TagPublished(ctx context.Context, tag string) (time.Time, bool)
 	if strings.TrimSpace(c.APIBase) == "" {
 		return time.Time{}, false
 	}
-	for index, repository := range []string{primaryRepository, legacyRepository} { // legacy-name
-		rawURL := strings.TrimRight(c.APIBase, "/") + "/repos/" + repository + "/releases/tags/" + url.PathEscape(tag)
-		body, err := c.get(ctx, rawURL, "application/vnd.github+json", true, "release API")
-		if err != nil {
-			if isStatus(err, http.StatusNotFound) && index == 0 {
-				continue
-			}
-			return time.Time{}, false
-		}
-		var row apiRelease
-		if json.Unmarshal(body, &row) != nil || strings.TrimSpace(row.TagName) != tag {
-			return time.Time{}, false
-		}
-		moment := row.stamp()
-		if moment.IsZero() {
-			return time.Time{}, false
-		}
-		return moment, true
+	rawURL := strings.TrimRight(c.APIBase, "/") + "/repos/" + primaryRepository + "/releases/tags/" + url.PathEscape(tag)
+	body, err := c.get(ctx, rawURL, "application/vnd.github+json", true, "release API")
+	if err != nil {
+		return time.Time{}, false
+	}
+	var row apiRelease
+	if json.Unmarshal(body, &row) != nil || strings.TrimSpace(row.TagName) != tag {
+		return time.Time{}, false
+	}
+	moment := row.stamp()
+	if moment.IsZero() {
+		return time.Time{}, false
 	}
-	return time.Time{}, false
+	return moment, true
 }
 
-func (c *Client) selectRepository(ctx context.Context, repository, channel, running string) (Release, error) {
+func (c *Client) selectRelease(ctx context.Context, channel, running string) (Release, error) {
 	suffix := "releases/latest"
 	if channel != "stable" {
 		suffix = "releases?per_page=100"
 	}
-	rawURL := strings.TrimRight(c.APIBase, "/") + "/repos/" + repository + "/" + suffix
+	rawURL := strings.TrimRight(c.APIBase, "/") + "/repos/" + primaryRepository + "/" + suffix
 	body, err := c.get(ctx, rawURL, "application/vnd.github+json", true, "release API")
 	if err != nil {
 		return Release{}, err
@@ -425,7 +417,7 @@ func (c *Client) selectRepository(ctx context.Context, repository, channel, runn
 		if Kind(row.TagName) != "stable" {
 			return Release{}, fmt.Errorf("the latest release did not name a stable codeaf tag")
 		}
-		return Release{Tag: row.TagName, Repository: repository, PublishedAt: row.stamp()}, nil
+		return Release{Tag: row.TagName, Repository: primaryRepository, PublishedAt: row.stamp()}, nil
 	}
 	var rows []apiRelease
 	if err := json.Unmarshal(body, &rows); err != nil {
@@ -448,12 +440,15 @@ func (c *Client) selectRepository(ctx context.Context, repository, channel, runn
 		return Release{}, fmt.Errorf("no %s build has been published yet", channel)
 	}
 	return Release{
-		Tag: newest.TagName, Repository: repository, PublishedAt: newest.stamp(),
+		Tag: newest.TagName, Repository: primaryRepository, PublishedAt: newest.stamp(),
 		RunningPublishedAt: runningPublished,
 	}, nil
 }
 
+// assetURL names one file of a release on the download host. The repository in
+// it is the constant, not anything carried on the release, so no caller can
+// point a download at a second origin.
 func (c *Client) assetURL(release Release, name string) string {
 	base := strings.TrimRight(c.DownloadBase, "/")
-	return base + "/" + release.Repository + "/releases/download/" + url.PathEscape(release.Tag) + "/" + url.PathEscape(name)
+	return base + "/" + primaryRepository + "/releases/download/" + url.PathEscape(release.Tag) + "/" + url.PathEscape(name)
 }

internal/update/install.go

@@ -76,9 +76,6 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
 	if release.Tag == "" {
 		return InstallResult{}, errors.New("the release tag is empty")
 	}
-	if release.Repository == "" {
-		release.Repository = primaryRepository
-	}
 	// ONE ACQUIRER, INSIDE THE SHARED INSTALLER. Every road that replaces this
 	// file — the chat surface's background install and `codeaf update` alike —
 	// comes through here, so the lock and the on-disk re-read cannot be
@@ -144,21 +141,12 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
 			return InstallResult{}, &RefusalError{Reason: reason}
 		}
 	}
-	var (
-		body      []byte
-		checksums []byte
-		err       error
-	)
-	for index, repository := range installRepositories(release.Repository) {
-		release.Repository = repository
-		body, asset, checksums, err = options.Client.downloadRelease(ctx, release, asset, goos, goarch, extension)
-		if err == nil {
-			break
-		}
-		if !isStatus(err, http.StatusNotFound) || index == 1 {
-			return InstallResult{}, err
-		}
-	}
+	// THE BYTES AND THE CHECKSUM COME FROM THE ONE ORIGIN ([primaryRepository]),
+	// whatever repository a caller wrote on the release ([Client.assetURL]
+	// reads only the constant): a 404 there is an answer, never a reason to go
+	// and ask somewhere else. The result records it.
+	release.Repository = primaryRepository
+	body, asset, checksums, err := options.Client.downloadRelease(ctx, release, asset, goos, goarch, extension)
 	if err != nil {
 		return InstallResult{}, err
 	}
@@ -198,13 +186,11 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
 	return InstallResult{Release: release, Path: target}, nil
 }
 
-func installRepositories(first string) []string {
-	if first == legacyRepository {
-		return []string{legacyRepository}
-	}
-	return []string{primaryRepository, legacyRepository} // legacy-name
-}
-
+// downloadRelease fetches the asset and its checksums from one release. The
+// former asset name is still tried, IN THE SAME RELEASE, for the one stable
+// published before the rename (v0.1.0): a person may still name that tag to roll
+// back to it, and checksums.txt beside it is what vouches for whichever name
+// answered.
 func (c *Client) downloadRelease(ctx context.Context, release Release, asset, goos, goarch, extension string) ([]byte, string, []byte, error) {
 	shownAsset := asset
 	body, err := c.downloadAsset(ctx, c.assetURL(release, asset), shownAsset)