implemented 05-artifact-store.md: filesystem artifact store with legacy-compatible naming, atomic persist from concurrent builds, retention pruning, and artifacts.rootDir config

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-07 10:08:40 +02:00
parent b379bc0a6b
commit 3c9eeda5da
14 changed files with 623 additions and 22 deletions
@@ -42,8 +42,13 @@ builds:
  # Changing this value requires a restart.
  maxConcurrent: 1

-# Build artifact retention.
+# Build artifact storage and retention.
 artifacts:
+  # Root directory for stored build artifacts.
+  # Empty means the platform default: $XDG_STATE_HOME (or ~/.local/state) plus /gittally/artifacts/<repo-key>,
+  # where <repo-key> is the sanitized absolute repository path.
+  # A leading ~/ expands to the home directory; a relative path is resolved against the repository.
+  rootDir: ""
  # number of builds to keep per branch
  retentionPerBranch: 3

@@ -40,3 +40,32 @@ Update `GitTallyConfig`, `InitCommand` templates, and `docs/configuration.md` to

 - `./gradlew ktlintFormat` then `./gradlew build` is green.
 - Step 04's executor persists artifacts through the real store (integration test).
+
+## Execution Notes (done 2026-07-07)
+
+Implemented as `FileArtifactStore` in `de.hoennig.gittally.artifacts`; build green, 12 new tests
+(`FileArtifactStoreTest`, `BuildExecutorArtifactIntegrationTest`, plus a `repoKey` case in `ArtifactKeysTest`).
+Deviations and decisions:
+
+- The `ArtifactStore` interface stays in `de.hoennig.gittally.build` (moving it would make `build` depend on `artifacts`).
+  It gained `prune(keptResults)` and `artifactDir(artifactKey)`; the `NoOpArtifactStore` placeholder was removed.
+- Interface gap from step 04 resolved by an additional parameter: `persist(build, stagingDir, workspace)`.
+  The store copies the configured `artifactDirs` out of the branch worktree itself, so the archived layout stays store knowledge.
+  `workspace` is null when a build crashed before its worktree was prepared; only the logs are stored then.
+- Key naming was reused from step 04's `ArtifactKeys` (UTC `Instant`, not local time like legacy); only `repoKey` was added.
+  The repo key sanitizes the absolute normalized working directory, not `git rev-parse --show-toplevel` — consistent
+  with step 04's decision to resolve everything relative to the working directory.
+- The atomic move does not move the staging directory directly: staging is a temp dir (usually under `/tmp`) and may
+  be on a different filesystem than the artifact root, where a move is not atomic.
+  Like legacy `persist_build_artifacts` (`$artifact_dir.tmp.$$`), everything is assembled in `.incoming-<key>` next to
+  the target and then moved with `ATOMIC_MOVE`; on failure the incoming dir is deleted, so no partial dirs appear.
+  The staging directory is deleted after a successful move.
+- The legacy archive layout was ported: `build/reports` archives as `reports/`, every other artifact dir below `reports/<dir>`.
+- Concurrency (builds run concurrently since the step 04 amendment): persists share a read lock — their target dirs are
+  disjoint because artifact keys are unique per build — while `prune` takes the write lock, so it never deletes mid-persist.
+  `prune` also removes `.incoming-*` leftovers of crashed persists, deletes symlinks without following them,
+  and returns the removed keys.
+- `artifactDir` rejects keys outside `[A-Za-z0-9._-]+` and anything resolving outside `<root>/branches/` (path traversal).
+- `artifacts.rootDir` supports a leading `~/` and resolves relative paths against the repository;
+  when empty, the default is `$XDG_STATE_HOME` (or `~/.local/state`) + `/gittally/artifacts/<repo-key>` as designed.
+- The bean is wired in `ArtifactsConfiguration` with the working directory defaulting to `.`, mirroring `BuildConfiguration`.
@@ -49,7 +49,7 @@ Foundation:
 Core engine:

 - [x] `04-build-executor.md` — async build execution with logs, cancellation, status transitions
- [ ] `05-artifact-store.md` — artifact persistence, naming, retention
+- [x] `05-artifact-store.md` — artifact persistence, naming, retention
 - [ ] `06-watcher.md` — branch watching, scheduling, auto-builds

 Server and UI:
@@ -0,0 +1,17 @@
+package de.hoennig.gittally.artifacts
+
+import de.hoennig.gittally.build.ArtifactStore
+import de.hoennig.gittally.config.ConfigLoader
+import org.springframework.context.annotation.Bean
+import org.springframework.context.annotation.Configuration
+
+@Configuration
+class ArtifactsConfiguration {
+    /**
+     * Store relative to the working directory, matching how `ConfigLoader` and the
+     * `BuildResultRepository` bean resolve their files. Nothing is touched until the
+     * first build persists, so the bean is safe outside a git repository.
+     */
+    @Bean
+    fun artifactStore(configLoader: ConfigLoader): ArtifactStore = FileArtifactStore(configLoader)
+}
@@ -0,0 +1,239 @@
+package de.hoennig.gittally.artifacts
+
+import de.hoennig.gittally.build.ArtifactKeys
+import de.hoennig.gittally.build.ArtifactStore
+import de.hoennig.gittally.build.BuildResult
+import de.hoennig.gittally.config.BranchConfig
+import de.hoennig.gittally.config.ConfigLoader
+import org.slf4j.LoggerFactory
+import java.io.IOException
+import java.nio.file.FileVisitResult
+import java.nio.file.Files
+import java.nio.file.LinkOption
+import java.nio.file.Path
+import java.nio.file.Paths
+import java.nio.file.SimpleFileVisitor
+import java.nio.file.StandardCopyOption
+import java.nio.file.attribute.BasicFileAttributes
+import java.util.concurrent.locks.ReentrantReadWriteLock
+import kotlin.concurrent.read
+import kotlin.concurrent.write
+
+/**
+ * Stores build artifacts on the filesystem under `<root>/branches/<artifactKey>/`.
+ * The root is `artifacts.rootDir` from the config, or the platform default
+ * `XDG_STATE_HOME` (falling back to `~/.local/state`) plus `/gittally/artifacts/<repo-key>` when unset —
+ * deliberately not `/tmp` like legacy, where artifacts vanished on reboot.
+ *
+ * Each build is assembled in a temporary directory next to its target and moved
+ * into place atomically, so a crashed persist never leaves a partial artifact dir.
+ * Builds persist concurrently (their keys are unique) under a shared read lock;
+ * [prune] takes the write lock so it never deletes a directory mid-persist.
+ */
+class FileArtifactStore(
+    private val configLoader: ConfigLoader,
+    private val workingDir: Path = Paths.get("."),
+    private val env: (String) -> String? = System::getenv,
+) : ArtifactStore {
+    private val log = LoggerFactory.getLogger(FileArtifactStore::class.java)
+
+    private val lock = ReentrantReadWriteLock()
+
+    override fun persist(
+        build: BuildResult,
+        stagingDir: Path,
+        workspace: Path?,
+    ) {
+        lock.read {
+            val branchesDir = Files.createDirectories(branchesDir())
+            val target = branchesDir.resolve(build.artifactKey)
+            val incoming = branchesDir.resolve(INCOMING_PREFIX + build.artifactKey)
+            try {
+                deleteRecursively(incoming)
+                Files.createDirectories(incoming)
+                copyChildren(stagingDir, incoming)
+                copyArtifactDirs(build, workspace, incoming)
+                deleteRecursively(target)
+                Files.move(incoming, target, StandardCopyOption.ATOMIC_MOVE)
+            } catch (e: Exception) {
+                deleteRecursively(incoming)
+                throw e
+            }
+            deleteRecursively(stagingDir)
+            log.info("persisted build artifacts of {} to {}", build.artifactKey, target)
+        }
+    }
+
+    override fun prune(keptResults: Collection<BuildResult>): List<String> {
+        lock.write {
+            val branchesDir = branchesDir()
+            if (!Files.isDirectory(branchesDir, LinkOption.NOFOLLOW_LINKS)) {
+                return emptyList()
+            }
+            val keptKeys = keptResults.map { it.artifactKey }.toSet()
+            val removed = mutableListOf<String>()
+            Files.list(branchesDir).use { children ->
+                children.forEach { child ->
+                    val key = child.fileName.toString()
+                    if (key !in keptKeys) {
+                        deleteRecursively(child)
+                        removed += key
+                    }
+                }
+            }
+            if (removed.isNotEmpty()) {
+                log.info("pruned {} artifact dir(s): {}", removed.size, removed)
+            }
+            return removed
+        }
+    }
+
+    override fun artifactDir(artifactKey: String): Path? {
+        if (!ARTIFACT_KEY_PATTERN.matches(artifactKey)) {
+            return null
+        }
+        val branchesDir = branchesDir()
+        val dir = branchesDir.resolve(artifactKey).normalize()
+        if (dir.parent != branchesDir) {
+            return null
+        }
+        return dir.takeIf { Files.isDirectory(it, LinkOption.NOFOLLOW_LINKS) }
+    }
+
+    private fun branchesDir(): Path = rootDir().resolve("branches")
+
+    private fun rootDir(): Path {
+        val configured =
+            configLoader
+                .load(workingDir)
+                .artifacts.rootDir
+                .trim()
+        if (configured.isNotEmpty()) {
+            return workingDir.resolve(expandHome(configured)).toAbsolutePath().normalize()
+        }
+        val stateHome =
+            env("XDG_STATE_HOME")?.takeIf { it.isNotBlank() }?.let { Paths.get(it) }
+                ?: Paths.get(System.getProperty("user.home"), ".local", "state")
+        return stateHome
+            .resolve("gittally")
+            .resolve("artifacts")
+            .resolve(ArtifactKeys.repoKey(workingDir))
+            .toAbsolutePath()
+            .normalize()
+    }
+
+    private fun expandHome(path: String): Path =
+        if (path == "~" || path.startsWith("~/")) {
+            Paths.get(System.getProperty("user.home"), path.removePrefix("~"))
+        } else {
+            Paths.get(path)
+        }
+
+    private fun copyArtifactDirs(
+        build: BuildResult,
+        workspace: Path?,
+        targetDir: Path,
+    ) {
+        if (workspace == null) {
+            log.warn("build {} has no workspace; storing only its logs", build.artifactKey)
+            return
+        }
+        for (artifactDir in branchConfig(build.branch).artifactDirs) {
+            if (artifactDir.isBlank()) {
+                continue
+            }
+            val source = workspace.resolve(artifactDir)
+            if (!Files.isDirectory(source)) {
+                log.info("build {} did not produce artifact directory {}; skipped", build.artifactKey, artifactDir)
+                continue
+            }
+            copyRecursively(source, targetDir.resolve(archivedPath(artifactDir)))
+        }
+    }
+
+    /** Legacy `archived_artefact_dir_path`: `build/reports` archives as `reports/`, everything else below `reports/<dir>`. */
+    private fun archivedPath(artifactDir: String): String =
+        if (artifactDir == "build/reports") {
+            "reports"
+        } else {
+            "reports/$artifactDir"
+        }
+
+    private fun branchConfig(branch: String): BranchConfig {
+        val branches = configLoader.load(workingDir).branches
+        return branches[branch] ?: branches["default"] ?: BranchConfig()
+    }
+
+    private fun copyChildren(
+        sourceDir: Path,
+        targetDir: Path,
+    ) {
+        if (Files.isDirectory(sourceDir)) {
+            copyRecursively(sourceDir, targetDir)
+        }
+    }
+
+    private fun copyRecursively(
+        source: Path,
+        target: Path,
+    ) {
+        Files.walkFileTree(
+            source,
+            object : SimpleFileVisitor<Path>() {
+                override fun preVisitDirectory(
+                    dir: Path,
+                    attrs: BasicFileAttributes,
+                ): FileVisitResult {
+                    Files.createDirectories(target.resolve(source.relativize(dir)))
+                    return FileVisitResult.CONTINUE
+                }
+
+                override fun visitFile(
+                    file: Path,
+                    attrs: BasicFileAttributes,
+                ): FileVisitResult {
+                    Files.copy(file, target.resolve(source.relativize(file)), StandardCopyOption.REPLACE_EXISTING)
+                    return FileVisitResult.CONTINUE
+                }
+            },
+        )
+    }
+
+    /** Does not follow symlinks — a link is deleted, its target is never touched. */
+    private fun deleteRecursively(path: Path) {
+        if (!Files.exists(path, LinkOption.NOFOLLOW_LINKS)) {
+            return
+        }
+        Files.walkFileTree(
+            path,
+            object : SimpleFileVisitor<Path>() {
+                override fun visitFile(
+                    file: Path,
+                    attrs: BasicFileAttributes,
+                ): FileVisitResult {
+                    Files.delete(file)
+                    return FileVisitResult.CONTINUE
+                }
+
+                override fun postVisitDirectory(
+                    dir: Path,
+                    exc: IOException?,
+                ): FileVisitResult {
+                    if (exc != null) {
+                        throw exc
+                    }
+                    Files.delete(dir)
+                    return FileVisitResult.CONTINUE
+                }
+            },
+        )
+    }
+
+    companion object {
+        /** Work-in-progress dir next to the target; unreferenced leftovers are cleaned up by [prune]. */
+        private const val INCOMING_PREFIX = ".incoming-"
+
+        /** Same character set the key sanitization produces; everything else is rejected on lookup. */
+        private val ARTIFACT_KEY_PATTERN = Regex("[A-Za-z0-9._-]+")
+    }
+}
@@ -1,5 +1,6 @@
 package de.hoennig.gittally.build

+import java.nio.file.Path
 import java.security.MessageDigest
 import java.time.Instant

@@ -16,6 +17,9 @@ object ArtifactKeys {
        startedAt: Instant,
    ): String = "${branchKey(branch)}-${sanitize(startedAt.toString())}-${sha256Prefix("$branch\t$startedAt")}"

+    /** Legacy `repository_key`: the sanitized absolute repository path. */
+    fun repoKey(repoDir: Path): String = sanitize(repoDir.toAbsolutePath().normalize().toString())
+
    private fun sanitize(value: String): String = value.replace(Regex("[^A-Za-z0-9._-]"), "_")

    private fun sha256Prefix(value: String): String =
@@ -1,29 +1,31 @@
 package de.hoennig.gittally.build

-import org.slf4j.LoggerFactory
-import org.springframework.stereotype.Component
 import java.nio.file.Path

 /**
- * Takes over the staging directory of a finished build.
- * The real store (naming, retention, serving) arrives in step 05.
+ * Persists the artifacts of finished builds (logs plus configured report directories)
+ * and prunes them together with the result retention.
+ * Implemented by `de.hoennig.gittally.artifacts.FileArtifactStore`.
 */
 interface ArtifactStore {
+    /**
+     * Takes over the staging directory of the finished [build] (log files) plus the
+     * configured `artifactDirs` from [workspace] and stores them under the build's
+     * artifact key. [workspace] is null when the build crashed before its workspace
+     * was prepared; only the logs are stored then.
+     */
    fun persist(
        build: BuildResult,
        stagingDir: Path,
+        workspace: Path?,
    )
-}

-/** Placeholder until step 05: logs and leaves the staging directory untouched. */
-@Component
-class NoOpArtifactStore : ArtifactStore {
-    private val log = LoggerFactory.getLogger(NoOpArtifactStore::class.java)
+    /**
+     * Deletes all stored artifact directories whose keys are not in [keptResults];
+     * call after repository retention pruning. Returns the removed artifact keys.
+     */
+    fun prune(keptResults: Collection<BuildResult>): List<String>

-    override fun persist(
-        build: BuildResult,
-        stagingDir: Path,
-    ) {
-        log.info("artifact store not implemented yet; leaving build output of {} in {}", build.artifactKey, stagingDir)
-    }
+    /** The stored artifact directory for [artifactKey], or null if none exists. */
+    fun artifactDir(artifactKey: String): Path?
 }
@@ -111,6 +111,7 @@ class BuildExecutor(
    private fun execute(build: ActiveBuild) {
        var slot: Semaphore? = null
        var finalStatus = BuildStatus.FAILED
+        var workspace: Path? = null
        try {
            slot = slotsFor(build.workingDir)
            slot.acquire()
@@ -120,13 +121,14 @@ class BuildExecutor(
            }
            build.running = true
            transition(build, BuildStatus.RUNNING, duration = null)
-            val workspace =
+            val preparedWorkspace =
                workspaces.prepare(
                    branch = build.runningBuild.branch,
                    commit = build.runningBuild.commit,
                    repoDir = build.workingDir,
                )
-            val exitCode = runBuildCommands(build, workspace)
+            workspace = preparedWorkspace
+            val exitCode = runBuildCommands(build, preparedWorkspace)
            finalStatus =
                when {
                    build.cancelled.get() -> BuildStatus.CANCELLED
@@ -141,7 +143,7 @@ class BuildExecutor(
            val duration = Duration.between(build.runningBuild.startedAt, Instant.now())
            val result = transition(build, finalStatus, duration)
            try {
-                artifactStore.persist(result, build.runningBuild.stagingDir)
+                artifactStore.persist(result, build.runningBuild.stagingDir, workspace)
            } catch (e: Exception) {
                log.warn("could not persist artifacts of {}: {}", result.artifactKey, e.message)
            }
@@ -117,8 +117,10 @@ class InitCommand(
              # how many branches may build at the same time (at most one build per branch regardless)
              maxConcurrent: 1

-            # Build artifact retention.
+            # Build artifact storage and retention.
            artifacts:
+              # root directory for stored artifacts; empty = XDG_STATE_HOME (or ~/.local/state) + /gittally/artifacts/<repo-key>
+              rootDir: ""
              # number of builds to keep per branch
              retentionPerBranch: 3

@@ -33,6 +33,11 @@ data class BuildsConfig(

 data class ArtifactsConfig(
    val retentionPerBranch: Int = 3,
+    /**
+     * Root directory for stored build artifacts; empty means the platform default
+     * `XDG_STATE_HOME` (or `~/.local/state`) + `/gittally/artifacts/<repo-key>`.
+     */
+    val rootDir: String = "",
 )

 data class WatcherConfig(
@@ -0,0 +1,63 @@
+package de.hoennig.gittally.artifacts
+
+import de.hoennig.gittally.build.BranchWorkspaces
+import de.hoennig.gittally.build.BuildExecutor
+import de.hoennig.gittally.build.FileBuildResultRepository
+import de.hoennig.gittally.build.ProcessBuildRunner
+import de.hoennig.gittally.config.ConfigLoader
+import de.hoennig.gittally.gitea.GiteaClient
+import io.kotest.assertions.nondeterministic.eventually
+import io.kotest.core.spec.style.FunSpec
+import io.kotest.matchers.nulls.shouldNotBeNull
+import io.kotest.matchers.shouldBe
+import io.kotest.matchers.string.shouldContain
+import io.mockk.mockk
+import org.springframework.context.ApplicationEventPublisher
+import java.nio.file.Files
+import kotlin.time.Duration.Companion.seconds
+
+/** Step 05 acceptance: the step 04 executor persists artifacts through the real store. */
+class BuildExecutorArtifactIntegrationTest : FunSpec() {
+    init {
+        test("the executor persists a successful build's logs and reports through the real store") {
+            val workingDir = Files.createTempDirectory("gittally-artifact-integration-test")
+            val root = workingDir.resolve("artifact-root")
+            Files.writeString(
+                workingDir.resolve(".gittally.yml"),
+                """
+                artifacts:
+                  rootDir: "$root"
+                branches:
+                  default:
+                    buildCommand: "mkdir -p build/reports && echo report-content > build/reports/index.html && echo built-ok"
+                    cleanCommand: ""
+                    artifactDirs:
+                      - build/reports
+                """.trimIndent(),
+            )
+            val workspace = Files.createDirectories(workingDir.resolve("workspace"))
+            val store = FileArtifactStore(ConfigLoader(), workingDir)
+            val executor =
+                BuildExecutor(
+                    repository = FileBuildResultRepository(workingDir.resolve("build-results.json")),
+                    configLoader = ConfigLoader(),
+                    giteaClient = mockk<GiteaClient>(relaxed = true),
+                    buildRunner = ProcessBuildRunner(),
+                    workspaces = BranchWorkspaces { _, _, _ -> workspace },
+                    artifactStore = store,
+                    eventPublisher = ApplicationEventPublisher { },
+                )
+
+            val build = executor.startBuild("main", "abc123", workingDir)
+
+            lateinit var artifactDir: java.nio.file.Path
+            eventually(30.seconds) {
+                artifactDir = store.artifactDir(build.artifactKey).shouldNotBeNull()
+            }
+            Files.readString(artifactDir.resolve("build.stdout.log")) shouldContain "built-ok"
+            Files.readString(artifactDir.resolve("build.log")) shouldContain "built-ok"
+            Files.readString(artifactDir.resolve("reports/index.html")) shouldContain "report-content"
+            Files.exists(build.stagingDir) shouldBe false
+        }
+    }
+}
@@ -0,0 +1,228 @@
+package de.hoennig.gittally.artifacts
+
+import de.hoennig.gittally.build.ArtifactKeys
+import de.hoennig.gittally.build.BuildResult
+import de.hoennig.gittally.build.BuildStatus
+import de.hoennig.gittally.config.ConfigLoader
+import io.kotest.assertions.throwables.shouldThrow
+import io.kotest.core.spec.style.FunSpec
+import io.kotest.matchers.collections.shouldContainExactly
+import io.kotest.matchers.nulls.shouldBeNull
+import io.kotest.matchers.nulls.shouldNotBeNull
+import io.kotest.matchers.shouldBe
+import java.io.IOException
+import java.nio.file.Files
+import java.nio.file.Path
+import java.time.Duration
+import java.time.Instant
+import kotlin.concurrent.thread
+import kotlin.io.path.listDirectoryEntries
+
+class FileArtifactStoreTest : FunSpec() {
+    private val startedAt = Instant.parse("2026-07-07T10:00:00Z")
+
+    private class Harness {
+        val workingDir: Path = Files.createTempDirectory("gittally-store-test")
+        val root: Path = workingDir.resolve("artifact-root")
+        val store = FileArtifactStore(ConfigLoader(), workingDir)
+
+        init {
+            Files.writeString(
+                workingDir.resolve(".gittally.yml"),
+                """
+                artifacts:
+                  rootDir: "$root"
+                branches:
+                  default:
+                    artifactDirs:
+                      - build/reports
+                      - build/doc
+                """.trimIndent(),
+            )
+        }
+
+        fun branchesDir(): Path = root.resolve("branches")
+    }
+
+    private fun buildResult(
+        branch: String = "main",
+        startedAt: Instant = this.startedAt,
+    ) = BuildResult(
+        branch = branch,
+        commit = "abc123",
+        status = BuildStatus.SUCCESS,
+        startedAt = startedAt,
+        duration = Duration.ofSeconds(10),
+        artifactKey = ArtifactKeys.buildKey(branch, startedAt),
+    )
+
+    private fun stagingDir(): Path {
+        val dir = Files.createTempDirectory("gittally-staging-test")
+        Files.writeString(dir.resolve("build.stdout.log"), "out")
+        Files.writeString(dir.resolve("build.stderr.log"), "err")
+        Files.writeString(dir.resolve("build.log"), "live")
+        return dir
+    }
+
+    private fun workspace(
+        h: Harness,
+        vararg artifactFiles: String,
+    ): Path {
+        val workspace = Files.createDirectories(h.workingDir.resolve("workspace"))
+        for (file in artifactFiles) {
+            val path = workspace.resolve(file)
+            Files.createDirectories(path.parent)
+            Files.writeString(path, "content of $file")
+        }
+        return workspace
+    }
+
+    init {
+        test("persist stores logs and configured artifact directories under the artifact key") {
+            val h = Harness()
+            val build = buildResult()
+            val staging = stagingDir()
+            val workspace = workspace(h, "build/reports/tests/index.html", "build/doc/readme.txt")
+
+            h.store.persist(build, staging, workspace)
+
+            val artifactDir = h.branchesDir().resolve(build.artifactKey)
+            Files.readString(artifactDir.resolve("build.stdout.log")) shouldBe "out"
+            Files.readString(artifactDir.resolve("build.stderr.log")) shouldBe "err"
+            Files.readString(artifactDir.resolve("build.log")) shouldBe "live"
+            // legacy layout: build/reports archives as reports/, other dirs below reports/<dir>
+            Files.exists(artifactDir.resolve("reports/tests/index.html")) shouldBe true
+            Files.exists(artifactDir.resolve("reports/build/doc/readme.txt")) shouldBe true
+            Files.exists(staging) shouldBe false
+        }
+
+        test("a missing artifact directory is skipped") {
+            val h = Harness()
+            val build = buildResult()
+            val workspace = workspace(h, "build/doc/readme.txt") // no build/reports
+
+            h.store.persist(build, stagingDir(), workspace)
+
+            val artifactDir = h.branchesDir().resolve(build.artifactKey)
+            Files.exists(artifactDir.resolve("reports/build/doc/readme.txt")) shouldBe true
+            Files.exists(artifactDir.resolve("reports/tests")) shouldBe false
+        }
+
+        test("persist without a workspace stores only the logs") {
+            val h = Harness()
+            val build = buildResult()
+
+            h.store.persist(build, stagingDir(), workspace = null)
+
+            val artifactDir = h.branchesDir().resolve(build.artifactKey)
+            Files.readString(artifactDir.resolve("build.log")) shouldBe "live"
+            Files.exists(artifactDir.resolve("reports")) shouldBe false
+        }
+
+        test("a failing persist leaves no partial artifact directory") {
+            val h = Harness()
+            val build = buildResult()
+            val workspace = workspace(h, "build/reports/secret.txt")
+            Files.setPosixFilePermissions(workspace.resolve("build/reports/secret.txt"), emptySet())
+
+            shouldThrow<IOException> {
+                h.store.persist(build, stagingDir(), workspace)
+            }
+
+            h.branchesDir().listDirectoryEntries() shouldBe emptyList()
+        }
+
+        test("the artifact root defaults to XDG_STATE_HOME plus the repo key") {
+            val workingDir = Files.createTempDirectory("gittally-store-test")
+            val stateHome = Files.createTempDirectory("gittally-state-home-test")
+            val store =
+                FileArtifactStore(ConfigLoader(), workingDir) { name ->
+                    if (name == "XDG_STATE_HOME") stateHome.toString() else null
+                }
+            val build = buildResult()
+
+            store.persist(build, stagingDir(), workspace = null)
+
+            val expectedDir =
+                stateHome
+                    .resolve("gittally/artifacts")
+                    .resolve(ArtifactKeys.repoKey(workingDir))
+                    .resolve("branches")
+                    .resolve(build.artifactKey)
+            Files.readString(expectedDir.resolve("build.log")) shouldBe "live"
+        }
+
+        test("prune deletes exactly the unreferenced artifact directories") {
+            val h = Harness()
+            val kept = buildResult(branch = "main")
+            val alsoKept = buildResult(branch = "feature/x")
+            val dropped = buildResult(branch = "main", startedAt = startedAt.plusSeconds(60))
+            listOf(kept, alsoKept, dropped).forEach {
+                Files.createDirectories(h.branchesDir().resolve(it.artifactKey))
+            }
+
+            val removed = h.store.prune(listOf(kept, alsoKept))
+
+            removed shouldContainExactly listOf(dropped.artifactKey)
+            Files.exists(h.branchesDir().resolve(kept.artifactKey)) shouldBe true
+            Files.exists(h.branchesDir().resolve(alsoKept.artifactKey)) shouldBe true
+            Files.exists(h.branchesDir().resolve(dropped.artifactKey)) shouldBe false
+        }
+
+        test("prune removes leftover incoming directories of crashed persists") {
+            val h = Harness()
+            val leftover = h.branchesDir().resolve(".incoming-${buildResult().artifactKey}")
+            Files.createDirectories(leftover)
+            Files.writeString(leftover.resolve("build.log"), "partial")
+
+            val removed = h.store.prune(emptyList())
+
+            removed shouldContainExactly listOf(leftover.fileName.toString())
+            Files.exists(leftover) shouldBe false
+        }
+
+        test("prune deletes a symlink without touching its target outside the root") {
+            val h = Harness()
+            val outside = Files.createTempDirectory("gittally-outside-test")
+            Files.writeString(outside.resolve("keep-me.txt"), "precious")
+            Files.createDirectories(h.branchesDir())
+            val link = h.branchesDir().resolve("evil-link")
+            Files.createSymbolicLink(link, outside)
+
+            val removed = h.store.prune(emptyList())
+
+            removed shouldContainExactly listOf("evil-link")
+            Files.exists(link) shouldBe false
+            Files.readString(outside.resolve("keep-me.txt")) shouldBe "precious"
+        }
+
+        test("artifactDir returns the stored directory and rejects unknown or unsafe keys") {
+            val h = Harness()
+            val build = buildResult()
+            h.store.persist(build, stagingDir(), workspace = null)
+
+            h.store.artifactDir(build.artifactKey).shouldNotBeNull() shouldBe
+                h.branchesDir().resolve(build.artifactKey)
+            h.store.artifactDir("unknown-key").shouldBeNull()
+            h.store.artifactDir("..").shouldBeNull()
+            h.store.artifactDir("../secrets").shouldBeNull()
+        }
+
+        test("concurrent persists and a prune do not interfere") {
+            val h = Harness()
+            val builds = (1..8).map { buildResult(startedAt = startedAt.plusSeconds(it.toLong())) }
+            val stagings = builds.associateWith { stagingDir() }
+
+            val persists =
+                builds.map { build ->
+                    thread { h.store.persist(build, stagings.getValue(build), workspace = null) }
+                }
+            val prune = thread { h.store.prune(builds) }
+            (persists + prune).forEach { it.join() }
+
+            builds.forEach { build ->
+                Files.readString(h.branchesDir().resolve(build.artifactKey).resolve("build.log")) shouldBe "live"
+            }
+        }
+    }
+}
@@ -5,6 +5,7 @@ import io.kotest.matchers.shouldBe
 import io.kotest.matchers.shouldNotBe
 import io.kotest.matchers.string.shouldContain
 import io.kotest.matchers.string.shouldMatch
+import java.nio.file.Paths
 import java.time.Instant

 class ArtifactKeysTest : FunSpec() {
@@ -34,5 +35,9 @@ class ArtifactKeysTest : FunSpec() {
            key shouldContain ArtifactKeys.branchKey("main")
            key shouldContain "2026-07-07T10_00_00Z"
        }
+
+        test("repoKey sanitizes the absolute repository path") {
+            ArtifactKeys.repoKey(Paths.get("/home/user/my repo")) shouldBe "_home_user_my_repo"
+        }
    }
 }
@@ -130,7 +130,7 @@ class BuildExecutorTest : FunSpec() {
            verify { h.giteaClient.publishStatus("abc123", BuildStatus.PENDING, any(), null, h.workingDir) }
            verify { h.giteaClient.publishStatus("abc123", BuildStatus.RUNNING, any(), null, h.workingDir) }
            verify { h.giteaClient.publishStatus("abc123", BuildStatus.SUCCESS, any(), null, h.workingDir) }
-            verify { h.artifactStore.persist(match { it.status == BuildStatus.SUCCESS }, build.stagingDir) }
+            verify { h.artifactStore.persist(match { it.status == BuildStatus.SUCCESS }, build.stagingDir, h.workingDir) }
        }

        test("build commands run in the workspace prepared for the branch") {