refactor(project): Reset result cache state on source-changed retry

allTasksCompleted reset the index state when throwing
SourceChangedDuringBuildError but left resultCacheState untouched.
Because BuildServer reuses the same ProjectBuildCache instance across
retries, a prior prepareProjectBuildAndValidateCache that had
transitioned the field to NO_CACHE or FRESH_AND_IN_USE caused the retry
to trip the PENDING_VALIDATION invariant in the
RESTORING_DEPENDENCY_INDICES branch and fail with
"Unexpected result cache state after restoring dependency indices for
project XYZ: no_cache".

Author: matz3

packages/project/lib/build/cache/ProjectBuildCache.js

@@ -1288,6 +1288,12 @@ export default class ProjectBuildCache {
 			this.#combinedIndexState = INDEX_STATES.RESTORING_PROJECT_INDICES;
 			this.#sourceIndex = null;
 			this.#taskCache.clear();
+			// Result cache state must also be reset: prepareProjectBuildAndValidateCache may have
+			// already transitioned it to NO_CACHE or FRESH_AND_IN_USE in the aborted build. The
+			// retry's prepareProjectBuildAndValidateCache asserts PENDING_VALIDATION after the
+			// dependency-index restore step, so a leftover non-PENDING_VALIDATION value would
+			// trip that assertion.
+			this.#resultCacheState = RESULT_CACHE_STATES.PENDING_VALIDATION;
 
 			throw new SourceChangedDuringBuildError(this.#project.getName());
 		}

packages/project/test/lib/build/BuildServer.integration.js

@@ -3,6 +3,7 @@ import sinonGlobal from "sinon";
 import {fileURLToPath} from "node:url";
 import {setTimeout} from "node:timers/promises";
 import fs from "node:fs/promises";
+import {appendFileSync} from "node:fs";
 import {graphFromPackageDependencies} from "../../../lib/graph/graph.js";
 import {setLogLevel} from "@ui5/logger";
 import Cache from "../../../lib/build/cache/Cache.js";
@@ -762,6 +763,81 @@ test.serial(
 	}
 );
 
+// Regression: a build that hits the NO_CACHE state in prepareProjectBuildAndValidateCache
+// (because the source signature does not match anything in the persistent cache) and then
+// throws SourceChangedDuringBuildError from allTasksCompleted used to fail on retry with
+// "Unexpected result cache state after restoring dependency indices for project XYZ: no_cache".
+// The fix resets #resultCacheState to PENDING_VALIDATION in the source-changed branch.
+//
+// Repro recipe — must hit *all* of these conditions on the same ProjectBuildCache instance:
+//   1. A first build runs to completion, populating the persistent index + result cache.
+//   2. The project is invalidated (a real source change observed by the watcher) so the next
+//      reader request drives a second build.
+//   3. The second build's #initSourceIndex finds an existing index cache and transitions to
+//      RESTORING_DEPENDENCY_INDICES (rather than INITIAL, which short-circuits prepare).
+//   4. prepareProjectBuildAndValidateCache sees a source-signature mismatch against the
+//      persisted result cache and sets #resultCacheState = NO_CACHE.
+//   5. A *further* on-disk source change lands during the second build, but the watcher path
+//      is stubbed so the abort signal is never set. allTasksCompleted's revalidateSourceIndex
+//      then throws SourceChangedDuringBuildError instead of taking the abort path.
+test.serial("Source change during second build retries cleanly without no_cache error", async (t) => {
+	const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "library.d");
+	await fixtureTester.serveProject({
+		config: {excludedTasks: ["minify"]}
+	});
+
+	const changedFilePath = `${fixtureTester.fixturePath}/main/src/library/d/some.js`;
+	const originalContent = await fs.readFile(changedFilePath, {encoding: "utf8"});
+
+	// Build 1 — populates the on-disk index + result cache.
+	await fixtureTester.requestResource({resource: "/resources/library/d/some.js"});
+
+	// Invalidate the project for build 2 by touching the source file. Use the live watcher
+	// path here: a real modification needs to flow through _projectResourceChanged so the
+	// project transitions to INVALIDATED and the next request enqueues a rebuild.
+	await fs.writeFile(changedFilePath, originalContent + "\n// pre-build-2 change\n");
+	await setTimeout(500); // let the watcher fire and settle
+
+	// Now suppress further watcher-driven aborts. The mid-build modification below is meant
+	// to flow through #revalidateSourceIndex inside allTasksCompleted, *not* through the
+	// watcher — otherwise the abort path runs first and the no_cache assertion never fires.
+	t.context.sinon.stub(fixtureTester.buildServer, "_projectResourceChanged");
+
+	// During build 2's task pipeline, append a second on-disk change. Hook the first task
+	// that is *not* short-circuited from cache (replaceCopyright) so the synchronous write
+	// lands well before allTasksCompleted's #revalidateSourceIndex reads from disk.
+	let triggered = false;
+	const handler = (event) => {
+		if (
+			!triggered &&
+			event.projectName === "library.d" &&
+			event.status === "task-start" &&
+			event.taskName === "replaceCopyright"
+		) {
+			triggered = true;
+			appendFileSync(changedFilePath, "\n// mid-build-2 change\n");
+		}
+	};
+	process.on("ui5.project-build-status", handler);
+
+	let resource;
+	try {
+		// Without the fix this rejects with
+		// "Unexpected result cache state after restoring dependency indices for project XYZ: no_cache".
+		resource = await fixtureTester._reader.byPath("/resources/library/d/some.js");
+	} finally {
+		process.off("ui5.project-build-status", handler);
+	}
+
+	t.true(triggered, "Test setup precondition: source change handler fired during build 2");
+
+	const servedContent = await resource.getString();
+	t.true(servedContent.includes("pre-build-2 change"),
+		"Retry served content reflecting the pre-build-2 change");
+	t.true(servedContent.includes("mid-build-2 change"),
+		"Retry served content reflecting the mid-build-2 change");
+});
+
 function getFixturePath(fixtureName) {
 	return fileURLToPath(new URL(`../../fixtures/${fixtureName}`, import.meta.url));
 }

packages/project/test/lib/build/cache/ProjectBuildCache.js

@@ -812,6 +812,79 @@ test("projectSourcesChanged after SourceChangedDuringBuildError does not corrupt
 	);
 });
 
+test("Retry after SourceChangedDuringBuildError when prior build set NO_CACHE: " +
+	"resultCacheState resets to PENDING_VALIDATION", async (t) => {
+	// Regression test for "Unexpected result cache state after restoring dependency indices
+	// for project ...: no_cache".
+	//
+	// Sequence:
+	//   1. Initial build: prepareProjectBuildAndValidateCache validates the result cache,
+	//      finds no match, transitions resultCacheState to NO_CACHE.
+	//   2. allTasksCompleted detects a source change during build (file changed after the
+	//      build started but before the watcher's abort signal propagated). It throws
+	//      SourceChangedDuringBuildError and resets indexState — but historically did not
+	//      reset resultCacheState, leaving it stuck on NO_CACHE.
+	//   3. BuildServer re-enqueues the project. The retry's prepareProjectBuildAndValidateCache
+	//      enters the RESTORING_DEPENDENCY_INDICES branch, which asserts resultCacheState ===
+	//      PENDING_VALIDATION. With the leftover NO_CACHE the assertion threw.
+	const project = createMockProject();
+	const cacheManager = createMockCacheManager();
+
+	const initialResource = createMockResource("/test.js", "hash1", 1000, 100, 1);
+	const changedResource = createMockResource("/test.js", "hash2", 2000, 200, 2);
+	let revalidate = false;
+	project.getSourceReader.callsFake(() => ({
+		byGlob: sinon.stub().callsFake(() => {
+			return Promise.resolve([revalidate ? changedResource : initialResource]);
+		}),
+		byPath: sinon.stub().resolves(initialResource)
+	}));
+
+	const indexCache = {
+		version: "1.0",
+		indexTree: {
+			version: 1,
+			indexTimestamp: 1000,
+			root: {
+				hash: "hash1",
+				children: {
+					"test.js": {
+						hash: "hash1",
+						metadata: {path: "/test.js", lastModified: 1000, size: 100, inode: 1}
+					}
+				}
+			}
+		},
+		tasks: []
+	};
+	cacheManager.readIndexCache.resolves(indexCache);
+	// readResultMetadata returns null by default → #findResultCache returns false → NO_CACHE.
+
+	const cache = await ProjectBuildCache.create(project, "sig", cacheManager);
+	await cache.initSourceIndex();
+
+	const mockDependencyReader = {
+		byGlob: sinon.stub().resolves([]),
+		byPath: sinon.stub().resolves(null)
+	};
+
+	// Step 1: initial build path — drives resultCacheState to NO_CACHE.
+	await cache.prepareProjectBuildAndValidateCache(mockDependencyReader);
+
+	// Step 2: source changed during build — flip the source reader to the modified resource.
+	revalidate = true;
+	const error = await t.throwsAsync(() => cache.allTasksCompleted());
+	t.true(error.message.includes("test.project"), "SourceChangedDuringBuildError thrown");
+
+	// Step 3: BuildServer retry — must NOT throw "Unexpected result cache state".
+	revalidate = false; // pretend the file is stable on the retry
+	await cache.initSourceIndex();
+	await t.notThrowsAsync(
+		() => cache.prepareProjectBuildAndValidateCache(mockDependencyReader),
+		"prepareProjectBuildAndValidateCache succeeds on retry"
+	);
+});
+
 test("prepareProjectBuildAndValidateCache: returns false for empty cache", async (t) => {
 	const project = createMockProject();
 	const cacheManager = createMockCacheManager();