modules/gitea/README.md

@@ -35,8 +35,9 @@ why under `keeps_all`.
 **Nothing is deleted on partial knowledge.** A repository whose files could not be listed, or a
 manifest or lockfile that could not be fetched, is named under `repositories_unread` in every answer,
 and a real run then deletes nothing at all. A file that was fetched but is not readable (broken JSON) is
-named under `files_not_read`; what it would have named is not known, and the operator reads that list
-before a real run. Under `node_modules` nothing is read: that is what a lockfile already says. An empty
+named under `files_not_read`, and so is a lockfile entry of a shape the reader does not know (the rest of
+that file is still read); what it would have named is not known, so a real run then deletes nothing at all
+either. Under `node_modules` nothing is read: that is what a lockfile already says. An empty
 repository, or one without its default branch, holds nothing to read and is not a failure.
 
 A real run deletes each version through the forge's own interface (`DELETE

modules/gitea/cmd/npm-registry/retention.go

@@ -395,8 +395,8 @@ func retention(ctx context.Context, f *Forge, only string, keep int, dryRun bool
 		s.Repositories, countVersions(s), len(s.Packages), mib(p.RegistryBytes), len(p.Candidates), mib(p.Bytes)))
 	s.Said = append(s.Said, sizesSaid(s)...)
 	if len(s.NotRead) > 0 {
-		s.Said = append(s.Said, fmt.Sprintf("%d files were not read whole (files_not_read): a version only they name is "+
-			"kept by nothing here", len(s.NotRead)))
+		s.Said = append(s.Said, fmt.Sprintf("%d files were not read whole (files_not_read): what they name is not "+
+			"known, so a real run deletes nothing", len(s.NotRead)))
 	}
 	if len(s.Unread) > 0 {
 		s.Said = append(s.Said, fmt.Sprintf("%d repositories or files could not be read: what they depend on is not "+
@@ -410,6 +410,12 @@ func retention(ctx context.Context, f *Forge, only string, keep int, dryRun bool
 		return p, fmt.Errorf("refused: %d repositories or files could not be read, so what depends on what is not "+
 			"known; nothing was deleted (first: %s)", len(s.Unread), s.Unread[0])
 	}
+	// A file that was fetched but not read whole is a file of its repository that could not be read
+	// (novox/hq ADR 0251 §4): a version only it names would be deleted while in use.
+	if len(s.NotRead) > 0 {
+		return p, fmt.Errorf("refused: %d files were not read whole, so what they name is not known; nothing was "+
+			"deleted (first: %s)", len(s.NotRead), s.NotRead[0])
+	}
 	del, cancel := context.WithTimeout(context.Background(), budget)
 	defer cancel()
 	for _, c := range p.Candidates {

modules/gitea/cmd/npm-registry/retention_test.go

@@ -465,3 +465,26 @@ func TestALockfileOfEveryShapeKeepsWhatItPins(t *testing.T) {
 		t.Fatalf("0.2.0: %+v", got)
 	}
 }
+
+func TestAnUnreadableLockfileStopsARealRunBeforeAnythingIsDeleted(t *testing.T) {
+	f := aRegistry(t)
+	f.repos["acme/odd"] = map[string]string{"package-lock.json": `{"lockfileVersion":3,"packages":{`}
+	p, err := retention(context.Background(), f.serve(), "", 3, false, "tidy")
+	if err == nil || !strings.Contains(err.Error(), "nothing was deleted") || !strings.Contains(err.Error(), "acme/odd:package-lock.json") {
+		t.Fatalf("got %v", err)
+	}
+	if len(f.deleted) != 0 {
+		t.Fatalf("deleted %v while a lockfile was not read", f.deleted)
+	}
+	if p == nil || len(p.NotRead) != 1 || len(p.Deleted) != 0 {
+		t.Fatalf("plan %+v", p)
+	}
+	// And the dry run says so, without failing, and still says what would go.
+	dry, err := retention(context.Background(), f.serve(), "", 3, true, "")
+	if err != nil || len(dry.NotRead) != 1 || len(dry.Candidates) == 0 {
+		t.Fatalf("dry run: %v %+v", err, dry)
+	}
+	if !strings.Contains(strings.Join(dry.Said, "\n"), "1 files were not read whole") {
+		t.Errorf("said %v", dry.Said)
+	}
+}