Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):

• Closes: #1418

2. Or, if no issue exists, describe the change:

Problem:

GcsArtifactService.listVersions returns an empty list when the listing raises StorageException.
saveArtifactAndReturnBlob derives the next version number from that result
(versions.isEmpty() ? 0 : max(versions) + 1) and writes with no precondition. At the point where
the version number is chosen, an empty list can mean either that the artifact has no versions or
that the listing did not complete — so a 503, a 429, a timeout or IAM propagation makes the save
compute version 0 and write over the object already stored at version 0, while returning success and
version 0 to the caller.

listArtifactKeys issues the same storageClient.list(bucketName, BlobListOption.prefix(...)) call
and surfaces a failure as VerifyException. listVersions is the one whose result selects the blob
name a save writes to.

Solution:

Separate the listing from the decision about what a failed listing means. The query moves into a
private readVersions that lets StorageException propagate, and each caller applies its own
policy: listVersions catches it and returns an empty list as before, the save path catches it and
throws. Four pieces:

private ImmutableList<Integer> versionsBeforeSaving(
    String appName, String userId, String sessionId, String filename) {
  try {
    return readVersions(appName, userId, sessionId, filename);
  } catch (StorageException e) {
    throw new VerifyException("Failed to list artifact versions from GCS", e);
  }
}

listVersions has three callers besides the save path — loadArtifact, deleteArtifact, and
ArtifactController in the dev module — so removing the catch outright would change
all of them as well. Leaving it in place keeps the diff to the one caller that overwrites stored
data: those three behave exactly as before, and listVersions_storageException_returnsEmptyList
passes untouched. That is a scoping choice, not a claim that the empty list is right for them; the
issue lists them as related and uncovered. If you would rather the catch went away and those callers
moved with it, say so and I will make that change instead.

Testing Plan

Unit Tests:

• I have added or updated unit tests for my change.
• All unit tests pass locally.

GcsArtifactServiceTest: Tests run: 26, Failures: 0, Errors: 0, Skipped: 0. Both new tests were
confirmed failing on unmodified main by reverting GcsArtifactService.java alone and re-running —
Tests run: 26, Failures: 2, exactly these two:

Both also verify(mockStorage).list(...), so neither can pass by failing earlier for an unrelated
reason.

Manual End-to-End (E2E) Tests:

• Run against a real GCS bucket, two arms. Both save PAYLOAD-A through a fully-permissioned
  client, then save PAYLOAD-B to the same filename — arm 1 through an identity denied
  storage.objects.list, arm 2 through a client that can list. The bucket is inspected afterwards
  with a privileged credential, since the restricted identity cannot list.

The control arm is unchanged by the fix in both runs — versions [0, 1], both objects kept — so the
two arms differ only in which client performs the second save.

Checklist

• I have read the CONTRIBUTING.md document.
• My pull request contains a single commit.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have added tests that prove my fix is effective or that my feature works.
• New and existing unit tests pass locally with my changes.
• I have manually tested my changes end-to-end.
• Any dependent changes have been merged and published in downstream modules.