From 3b36a5e6ea140add34a4266c60be43d9675736b0 Mon Sep 17 00:00:00 2001 From: JorisJonkers Agent Date: Mon, 13 Jul 2026 07:37:10 +0000 Subject: [PATCH 01/10] refactor(RunnerSessionBinder): split into collaborators to remove TooManyFunctions/LargeClass Extract RunnerBindingMetrics, RunnerSetupResolver, RunnerAgentSpawner, and RunnerProvisioningCoordinator so RunnerSessionBinder drops from 36 functions to 15 and under 200 lines, removing @Suppress("TooManyFunctions","LargeClass"). MAX_SPAWN_ATTEMPTS remains on RunnerSessionBinder.companion as a delegating constant for backward compatibility with existing tests. Add RunnerAgentSpawnerTest and RunnerBindingMetricsTest. --- .../sessionbinding/RunnerAgentSpawner.kt | 74 +++ .../sessionbinding/RunnerBindingMetrics.kt | 105 +++++ .../RunnerProvisioningCoordinator.kt | 110 +++++ .../sessionbinding/RunnerSessionBinder.kt | 422 +++--------------- .../sessionbinding/RunnerSetupResolver.kt | 102 +++++ .../sessionbinding/RunnerAgentSpawnerTest.kt | 129 ++++++ .../RunnerBindingMetricsTest.kt | 133 ++++++ 7 files changed, 705 insertions(+), 370 deletions(-) create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerAgentSpawner.kt create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetrics.kt create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerProvisioningCoordinator.kt create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSetupResolver.kt create mode 100644 api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerAgentSpawnerTest.kt create mode 100644 api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerAgentSpawner.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerAgentSpawner.kt new file mode 100644 index 0000000..5358b34 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerAgentSpawner.kt @@ -0,0 +1,74 @@ +package com.jorisjonkers.personalstack.agents.application.sessionbinding + +import com.jorisjonkers.personalstack.agents.application.exception.AgentRunnerUnavailableException +import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession +import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient +import org.slf4j.LoggerFactory +import org.springframework.web.client.ResourceAccessException + +/** + * Handles the spawn-with-retry loop for agent gateway sessions. Extracted from + * RunnerSessionBinder to keep that class below the TooManyFunctions threshold. + */ +internal class RunnerAgentSpawner( + private val gateway: AgentGatewayClient, + private val backoffInitialMs: Long, +) { + private val log = LoggerFactory.getLogger(RunnerAgentSpawner::class.java) + + fun spawnWithRetry( + workspace: Workspace, + session: WorkspaceAgentSession, + continuation: AgentGatewayClient.ContinuationMetadata?, + ): AgentGatewayClient.GatewayAgent { + var lastFailure: ResourceAccessException? = null + repeat(MAX_SPAWN_ATTEMPTS) { attempt -> + try { + return gateway.spawnAgent(buildSpawnRequest(workspace, session, continuation)) + } catch (ex: ResourceAccessException) { + lastFailure = ex + logRetry(workspace, ex, attempt) + } + } + throw AgentRunnerUnavailableException( + workspaceId = workspace.id, + runnerStatus = "ConnectionRefused", + retryAfterSeconds = AgentRunnerUnavailableException.DEFAULT_RETRY_AFTER_SECONDS, + cause = lastFailure, + ) + } + + private fun buildSpawnRequest( + workspace: Workspace, + session: WorkspaceAgentSession, + continuation: AgentGatewayClient.ContinuationMetadata?, + ) = AgentGatewayClient.SpawnAgentRequest( + workspace = workspace, + kind = session.kind, + stableSessionId = session.id, + epoch = session.epoch, + continuation = continuation, + resumeCliSessionId = session.cliSessionId, + ) + + private fun logRetry( + workspace: Workspace, + ex: ResourceAccessException, + attempt: Int, + ) { + val sleepMs = backoffInitialMs * (attempt + 1) + log.warn( + "agent spawn attempt {} for workspace {} failed: {} - retrying in {}ms", + attempt + 1, + workspace.id.value, + ex.message, + sleepMs, + ) + if (sleepMs > 0) Thread.sleep(sleepMs) + } + + companion object { + const val MAX_SPAWN_ATTEMPTS: Int = 3 + } +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetrics.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetrics.kt new file mode 100644 index 0000000..6239d88 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetrics.kt @@ -0,0 +1,105 @@ +package com.jorisjonkers.personalstack.agents.application.sessionbinding + +import com.jorisjonkers.personalstack.agents.application.exception.AgentRunnerUnavailableException +import com.jorisjonkers.personalstack.agents.application.exception.AgentSetupValidationException +import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel +import com.jorisjonkers.personalstack.agents.application.observability.ModeLabel +import com.jorisjonkers.personalstack.agents.application.observability.OperationLabel +import com.jorisjonkers.personalstack.agents.application.observability.OperationTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.OutcomeLabel +import com.jorisjonkers.personalstack.agents.application.observability.RunnerReprovisionTelemetry +import java.io.IOException +import java.time.Duration + +/** + * Records telemetry for runner session binding operations. Extracted from + * RunnerSessionBinder to keep that class below the TooManyFunctions threshold. + */ +internal class RunnerBindingMetrics( + private val telemetry: AgentsApiTelemetry, +) { + fun observeBinding( + operation: OperationLabel, + mode: ModeLabel, + block: () -> RunnerSessionBindingResult, + ): RunnerSessionBindingResult { + val startedAt = System.nanoTime() + return runCatching(block) + .onSuccess { result -> + val (outcome, reason) = + when (result) { + is RunnerSessionBindingResult.Bound -> OutcomeLabel.SUCCESS to FailureReasonLabel.NONE + is RunnerSessionBindingResult.Conflict -> bindingConflict() + is RunnerSessionBindingResult.Unavailable -> + OutcomeLabel.FAILURE to FailureReasonLabel.UPSTREAM_UNAVAILABLE + } + record(operation, mode, outcome, reason, startedAt) + }.onFailure { ex -> + record(operation, mode, OutcomeLabel.FAILURE, reasonClass(ex), startedAt) + }.getOrThrow() + } + + fun observeStage( + operation: OperationLabel, + mode: ModeLabel, + outcome: (T) -> Pair = { OutcomeLabel.SUCCESS to FailureReasonLabel.NONE }, + block: () -> T, + ): T { + val startedAt = System.nanoTime() + return runCatching(block) + .onSuccess { result -> + val (resultOutcome, reason) = outcome(result) + record(operation, mode, resultOutcome, reason, startedAt) + }.onFailure { ex -> + record(operation, mode, OutcomeLabel.FAILURE, reasonClass(ex), startedAt) + }.getOrThrow() + } + + fun recordReprovision( + outcome: OutcomeLabel, + reason: FailureReasonLabel, + startedAt: Long, + ) { + telemetry.recordRunnerReprovision(RunnerReprovisionTelemetry(outcome = outcome, reason = reason)) + record( + operation = OperationLabel.REPROVISION_RUNNER, + mode = ModeLabel.DURABLE, + outcome = outcome, + reason = reason, + startedAt = startedAt, + ) + } + + fun record( + operation: OperationLabel, + mode: ModeLabel, + outcome: OutcomeLabel, + reason: FailureReasonLabel, + startedAt: Long, + ) { + telemetry.recordOperation( + OperationTelemetry( + operation = operation, + mode = mode, + outcome = outcome, + reason = reason, + duration = Duration.ofNanos((System.nanoTime() - startedAt).coerceAtLeast(0)), + ), + ) + } + + fun bindingConflict(): Pair = + OutcomeLabel.FAILURE to FailureReasonLabel.CAPACITY + + fun reasonClass(ex: Throwable): FailureReasonLabel = + when (ex) { + is AgentSetupValidationException, + is IllegalArgumentException, + -> FailureReasonLabel.INVALID_REQUEST + + is AgentRunnerUnavailableException -> FailureReasonLabel.UPSTREAM_UNAVAILABLE + is IOException -> FailureReasonLabel.IO_ERROR + else -> FailureReasonLabel.UNKNOWN + } +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerProvisioningCoordinator.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerProvisioningCoordinator.kt new file mode 100644 index 0000000..4c27722 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerProvisioningCoordinator.kt @@ -0,0 +1,110 @@ +package com.jorisjonkers.personalstack.agents.application.sessionbinding + +import com.jorisjonkers.personalstack.agents.application.exception.AgentRunnerUnavailableException +import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel +import com.jorisjonkers.personalstack.agents.application.observability.OutcomeLabel +import com.jorisjonkers.personalstack.agents.application.workspacerunner.RunnerSetupTarget +import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient +import com.jorisjonkers.personalstack.agents.domain.port.AgentRunnerOrchestrator +import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepository +import org.slf4j.LoggerFactory + +/** + * Coordinates runner provisioning: scale-down, provision, await readiness, + * and record the reprovision telemetry. Extracted from RunnerSessionBinder to + * keep that class below the TooManyFunctions threshold. + */ +internal class RunnerProvisioningCoordinator( + private val orchestrator: AgentRunnerOrchestrator, + private val gateway: AgentGatewayClient, + private val workspaces: WorkspaceRepository, + private val metrics: RunnerBindingMetrics, + private val backoffInitialMs: Long, +) { + private val log = LoggerFactory.getLogger(RunnerProvisioningCoordinator::class.java) + + fun forceProvisionAndWait( + workspace: Workspace, + target: RunnerSetupTarget, + runnerGeneration: Long, + ): RunnerReady { + val startedAt = System.nanoTime() + return runCatching { provisionAndBuildReady(workspace, target, runnerGeneration) } + .onSuccess { + metrics.recordReprovision(OutcomeLabel.SUCCESS, FailureReasonLabel.NONE, startedAt) + }.onFailure { ex -> + val reason = metrics.reasonClass(ex) + metrics.recordReprovision(OutcomeLabel.FAILURE, reason, startedAt) + }.getOrThrow() + } + + fun isRunnerReadyFor( + workspace: Workspace, + target: RunnerSetupTarget, + ): Boolean { + val identity = target.spec.identity(workspace.runnerSetupGeneration) + return orchestrator.isReady(workspace, identity) && gateway.isReady(workspace) + } + + private fun provisionAndBuildReady( + workspace: Workspace, + target: RunnerSetupTarget, + runnerGeneration: Long, + ): RunnerReady { + val handle = + runCatching { + orchestrator.scaleDown(workspace) + orchestrator.provision(workspace, target.spec, runnerGeneration) + }.getOrElse { ex -> + log.warn("reprovision of workspace {} failed during scaleDown/provision", workspace.id.value, ex) + throw AgentRunnerUnavailableException( + workspaceId = workspace.id, + runnerStatus = "ReprovisionFailed", + cause = ex, + ) + } + val repointed = + workspace.withPodInfo( + podName = handle.podName, + pvcName = handle.pvcName, + gatewayEndpoint = handle.gatewayEndpoint, + ) + val saved = workspaces.save(repointed) + log.info("re-provisioned runner for workspace {} as pod {}", workspace.id.value, handle.podName) + return RunnerReady( + workspace = saved, + ready = awaitRunnerReady(saved, target, runnerGeneration), + provisioning = + RunnerProvisioningResult.Provisioned( + podName = handle.podName, + pvcName = handle.pvcName, + gatewayEndpoint = handle.gatewayEndpoint, + ), + ) + } + + private fun awaitRunnerReady( + workspace: Workspace, + target: RunnerSetupTarget, + runnerGeneration: Long, + ): Boolean { + val identity = target.spec.identity(runnerGeneration) + repeat(RunnerAgentSpawner.MAX_SPAWN_ATTEMPTS) { attempt -> + if (orchestrator.isReady(workspace, identity) && gateway.isReady(workspace)) return true + if (attempt < RunnerAgentSpawner.MAX_SPAWN_ATTEMPTS - 1) { + val sleepMs = backoffInitialMs * (attempt + 1) + if (sleepMs > 0) Thread.sleep(sleepMs) + } + } + return false + } +} + +internal data class RunnerReady( + val workspace: Workspace, + // The runner was reprovisioned; `ready` is whether it became ready within + // the synchronous window. When false the session is left awaiting rebind. + val ready: Boolean, + val provisioning: RunnerProvisioningResult, +) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBinder.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBinder.kt index 4db9dea..1298dde 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBinder.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBinder.kt @@ -1,39 +1,25 @@ package com.jorisjonkers.personalstack.agents.application.sessionbinding -import com.jorisjonkers.personalstack.agents.application.exception.AgentRunnerUnavailableException -import com.jorisjonkers.personalstack.agents.application.exception.AgentSetupValidationException import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel import com.jorisjonkers.personalstack.agents.application.observability.ModeLabel import com.jorisjonkers.personalstack.agents.application.observability.OperationLabel -import com.jorisjonkers.personalstack.agents.application.observability.OperationTelemetry import com.jorisjonkers.personalstack.agents.application.observability.OutcomeLabel -import com.jorisjonkers.personalstack.agents.application.observability.RunnerReprovisionTelemetry import com.jorisjonkers.personalstack.agents.application.sessionstatus.SessionStatusPublisher import com.jorisjonkers.personalstack.agents.application.setup.AgentSetupSelectionService -import com.jorisjonkers.personalstack.agents.application.setup.AgentSetupValidationInput import com.jorisjonkers.personalstack.agents.application.setup.AgentSetupValidationService import com.jorisjonkers.personalstack.agents.application.workspacerunner.RunnerSetupTarget import com.jorisjonkers.personalstack.agents.application.workspacerunner.RunnerUnavailableReason -import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupCatalogEntry -import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupId -import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupVersion import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupOperation -import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupProvisioningSpec import com.jorisjonkers.personalstack.agents.domain.model.Workspace -import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionStatus import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient -import com.jorisjonkers.personalstack.agents.domain.port.AgentRunnerOrchestrator import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceAgentSessionRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepository import org.slf4j.LoggerFactory import org.springframework.stereotype.Component import org.springframework.transaction.annotation.Transactional -import org.springframework.web.client.ResourceAccessException -import java.io.IOException -import java.time.Duration import java.time.Instant @Component @@ -58,7 +44,6 @@ class RunnerSetupDependencies( ) @Component -@Suppress("TooManyFunctions", "LargeClass") class RunnerSessionBinder( bindingDependencies: RunnerSessionBindingDependencies, setupDependencies: RunnerSetupDependencies, @@ -68,16 +53,27 @@ class RunnerSessionBinder( private val workspaces = bindingDependencies.workspaces private val sessions = bindingDependencies.sessions private val gateway = bindingDependencies.gateway - private val orchestrator = bindingDependencies.orchestrator private val tx = bindingDependencies.tx private val sessionStatus = notifications.sessionStatus - private val telemetry = notifications.telemetry - private val setupSelection = setupDependencies.setupSelection - private val setupValidation = setupDependencies.setupValidation + private val metrics = RunnerBindingMetrics(notifications.telemetry) + private val setupResolver = + RunnerSetupResolver( + setupDependencies.setupSelection, + setupDependencies.setupValidation, + ) + private val spawner = RunnerAgentSpawner(gateway, backoffInitialMs) + private val provisioning = + RunnerProvisioningCoordinator( + bindingDependencies.orchestrator, + gateway, + workspaces, + metrics, + backoffInitialMs, + ) private val log = LoggerFactory.getLogger(RunnerSessionBinder::class.java) override fun start(request: StartRunnerSessionBindingInput): RunnerSessionBindingResult = - observeBindingOperation(OperationLabel.START_SESSION, ModeLabel.fromRaw(request.runMode)) { + metrics.observeBinding(OperationLabel.START_SESSION, ModeLabel.fromRaw(request.runMode)) { startInternal(request) } @@ -85,14 +81,12 @@ class RunnerSessionBinder( val workspace = workspaces.findById(request.workspaceId) ?: throw NoSuchElementException("workspace not found: ${request.workspaceId.value}") - val target = resolveNewSessionSetup(workspace, request.kind, request.setupId, request.setupVersion) - // Validate readiness before persisting — genuine cold-start returns Unavailable with no row created. + val target = + setupResolver.resolveNewSessionSetup(workspace, request.kind, request.setupId, request.setupVersion) checkBindingReadiness(workspace, target)?.let { return it } val now = Instant.now() val session = newSession(request, workspace, target, now) - // Spawn before persisting so a gateway failure leaves no orphaned STARTING row. - val gatewayAgent = spawnAgentWithRetry(workspace, session, continuation = null) - // Persist as RUNNING+bound in one write — the UPSERT accepts this state directly. + val gatewayAgent = spawner.spawnWithRetry(workspace, session, continuation = null) val saved = persistBoundSession(workspace, session, gatewayAgent) ?: return RunnerSessionBindingResult.Unavailable( @@ -149,7 +143,7 @@ class RunnerSessionBinder( } override fun restart(request: RestartRunnerSessionBindingInput): RunnerSessionBindingResult = - observeBindingOperation(OperationLabel.REPROVISION_RUNNER, ModeLabel.INTERACTIVE) { + metrics.observeBinding(OperationLabel.REPROVISION_RUNNER, ModeLabel.INTERACTIVE) { restartInternal(request) } @@ -184,7 +178,7 @@ class RunnerSessionBinder( workspace: Workspace, request: RestartRunnerSessionBindingInput, ): RunnerSessionBindingResult { - val target = resolveRestartSetup(workspace, session, request) + val target = setupResolver.resolveRestartSetup(workspace, session, request) val leasedWorkspace = workspace.beginRunnerSetupRestart( setupId = target.entry.definition.id, @@ -204,7 +198,7 @@ class RunnerSessionBinder( } val ready = - runCatching { forceProvisionAndWait(leasedWorkspace, target, leasedWorkspace.runnerSetupGeneration) } + runCatching { provisioning.forceProvisionAndWait(leasedWorkspace, target, leasedWorkspace.runnerSetupGeneration) } .getOrElse { ex -> tx.markFailed(starting) tx.failWorkspaceSetupOperation(leasedWorkspace) @@ -265,7 +259,7 @@ class RunnerSessionBinder( } override fun ensureBound(request: EnsureRunnerSessionBoundInput): RunnerSessionBindingResult = - observeBindingOperation(OperationLabel.ATTACH_SESSION, ModeLabel.INTERACTIVE) { + metrics.observeBinding(OperationLabel.ATTACH_SESSION, ModeLabel.INTERACTIVE) { ensureBoundInternal(request) } @@ -281,7 +275,7 @@ class RunnerSessionBinder( "session does not belong to workspace: ${request.sessionId.value}" } ensureBoundGuard(session, workspace)?.let { return it } - val target = resolveSessionSetup(workspace, session) + val target = setupResolver.resolveSessionSetup(workspace, session) require( session.status == WorkspaceAgentSessionStatus.RUNNING || session.status == WorkspaceAgentSessionStatus.STARTING, @@ -298,9 +292,7 @@ class RunnerSessionBinder( workspace: Workspace, ): RunnerSessionBindingResult? = when { - // Session-level conflict: a restart is already in progress for this session. sessionHasPendingSetup(session) -> RunnerSessionBindingResult.Conflict(current = session) - // Workspace runner setup in progress → runner not yet available, retry later. workspaceSetupInProgress(workspace) -> RunnerSessionBindingResult.Unavailable( workspaceId = workspace.id, @@ -309,19 +301,6 @@ class RunnerSessionBinder( else -> null } - private fun runnerUnavailableForRebind( - workspace: Workspace, - target: RunnerSetupTarget, - ): RunnerSessionBindingResult.Unavailable? { - val reason = - when { - workspace.runnerBootLeaseId != null -> RunnerUnavailableReason.BOOT_LEASE_HELD.label - !isRunnerReadyFor(workspace, target) -> RunnerUnavailableReason.NOT_READY_AFTER_PROVISION.label - else -> return null - } - return RunnerSessionBindingResult.Unavailable(workspaceId = workspace.id, runnerStatus = reason) - } - private fun sessionHasPendingSetup(session: WorkspaceAgentSession): Boolean = session.pendingSetupId != null || session.pendingSetupVersion != null @@ -336,7 +315,9 @@ class RunnerSessionBinder( target: RunnerSetupTarget, ): RunnerSessionBindingResult.Bound? { val gatewayAgentId = session.gatewayAgentId ?: return null - if (session.status != WorkspaceAgentSessionStatus.RUNNING || !isRunnerReadyFor(workspace, target)) return null + if (session.status != WorkspaceAgentSessionStatus.RUNNING || !provisioning.isRunnerReadyFor(workspace, target)) { + return null + } return RunnerSessionBindingResult.Bound( workspace = workspace, session = session, @@ -359,7 +340,15 @@ class RunnerSessionBinder( workspace: Workspace, target: RunnerSetupTarget, ): RunnerSessionBindingResult { - runnerUnavailableForRebind(workspace, target)?.let { return it } + val unavailableReason = + when { + workspace.runnerBootLeaseId != null -> RunnerUnavailableReason.BOOT_LEASE_HELD.label + !provisioning.isRunnerReadyFor(workspace, target) -> RunnerUnavailableReason.NOT_READY_AFTER_PROVISION.label + else -> null + } + if (unavailableReason != null) { + return RunnerSessionBindingResult.Unavailable(workspaceId = workspace.id, runnerStatus = unavailableReason) + } val starting = session.beginGeneration(nextEpoch = session.epoch + 1) if (!tx.beginGeneration(session, starting)) { return RunnerSessionBindingResult.Conflict(current = sessions.findById(session.id)) @@ -385,8 +374,8 @@ class RunnerSessionBinder( ): RunnerSessionBindingResult { val gatewayAgent = runCatching { - observeStageOperation(OperationLabel.START_SESSION, ModeLabel.INTERACTIVE) { - spawnAgentWithRetry(workspace, session, continuation) + metrics.observeStage(OperationLabel.START_SESSION, ModeLabel.INTERACTIVE) { + spawner.spawnWithRetry(workspace, session, continuation) } }.getOrElse { ex -> tx.markFailed(session) @@ -394,10 +383,10 @@ class RunnerSessionBinder( } val bound = runCatching { - observeStageOperation( + metrics.observeStage( operation = OperationLabel.ATTACH_SESSION, mode = ModeLabel.INTERACTIVE, - outcome = { if (it) OutcomeLabel.SUCCESS to FailureReasonLabel.NONE else bindingConflict() }, + outcome = { if (it) OutcomeLabel.SUCCESS to FailureReasonLabel.NONE else metrics.bindingConflict() }, ) { tx.bind(session, gatewayAgent, promotePendingSetup) } @@ -426,332 +415,25 @@ class RunnerSessionBinder( workspace: Workspace, target: RunnerSetupTarget, ): RunnerSessionBindingResult.Unavailable? { - val unavailableReason = bindingUnavailableReason(workspace, target) ?: return null - return RunnerSessionBindingResult.Unavailable( - workspaceId = workspace.id, - runnerStatus = unavailableReason, - ) - } - - private fun bindingUnavailableReason( - workspace: Workspace, - target: RunnerSetupTarget, - ): String? = - when { - workspace.runnerSetupOperation != RunnerSetupOperation.IDLE || - workspace.pendingRunnerSetupId != null || - workspace.pendingRunnerSetupVersion != null -> - RunnerUnavailableReason.SETUP_OPERATION_IN_PROGRESS.label - workspace.runnerBootLeaseId != null -> RunnerUnavailableReason.BOOT_LEASE_HELD.label - !isRunnerReadyFor(workspace, target) -> RunnerUnavailableReason.NOT_READY_AFTER_PROVISION.label - else -> null - } - - private fun isRunnerReadyFor( - workspace: Workspace, - target: RunnerSetupTarget, - ): Boolean { - val identity = target.spec.identity(workspace.runnerSetupGeneration) - return orchestrator.isReady(workspace, identity) && gateway.isReady(workspace) - } - - private fun resolveNewSessionSetup( - workspace: Workspace, - kind: WorkspaceAgentKind, - setupId: AgentSetupId?, - setupVersion: AgentSetupVersion?, - ): RunnerSetupTarget { - if (setupId != null || setupVersion != null) { - require(setupId != null && setupVersion != null) { "setup id and version must be supplied together" } - return requireValidTarget(workspace, kind, null, setupSelection.requireSelectable(setupId, setupVersion)) - } - val current = - runCatching { - val entry = - setupSelection.requireSelectable( - workspace.currentRunnerSetupId, - workspace.currentRunnerSetupVersion, - ) - val result = - setupValidation.validate( - AgentSetupValidationInput( - workspace = workspace, - targetId = entry.definition.id, - targetVersion = entry.definition.version, - agentKind = kind, - ), - ) - if (result.valid) RunnerSetupTarget(entry, RunnerSetupProvisioningSpec.from(entry.definition)) else null - }.getOrNull() - if (current != null) return current - return requireValidTarget(workspace, kind, null, setupSelection.defaultSelectable()) - } - - private fun resolveRestartSetup( - workspace: Workspace, - session: WorkspaceAgentSession, - request: RestartRunnerSessionBindingInput, - ): RunnerSetupTarget { - require((request.targetSetupId == null) == (request.targetSetupVersion == null)) { - "target setup id and version must be supplied together" - } - val targetId = request.targetSetupId ?: session.currentSetupId - val targetVersion = request.targetSetupVersion ?: session.currentSetupVersion - return requireValidTarget( - workspace = workspace, - kind = session.kind, - session = session, - entry = setupSelection.requireSelectable(targetId, targetVersion), - ) - } - - private fun resolveSessionSetup( - workspace: Workspace, - session: WorkspaceAgentSession, - ): RunnerSetupTarget = - requireValidTarget( - workspace = workspace, - kind = session.kind, - session = session, - entry = setupSelection.requireSelectable(session.currentSetupId, session.currentSetupVersion), - ) - - private fun requireValidTarget( - workspace: Workspace, - kind: WorkspaceAgentKind, - session: WorkspaceAgentSession?, - entry: AgentSetupCatalogEntry, - ): RunnerSetupTarget { - setupValidation.requireValid( - AgentSetupValidationInput( - workspace = workspace, - targetId = entry.definition.id, - targetVersion = entry.definition.version, - session = session, - agentKind = kind, - ), - ) - return RunnerSetupTarget(entry, RunnerSetupProvisioningSpec.from(entry.definition)) - } - - private fun forceProvisionAndWait( - workspace: Workspace, - target: RunnerSetupTarget, - runnerGeneration: Long, - ): RunnerReady { - val startedAt = System.nanoTime() - return runCatching { provisionAndBuildReady(workspace, target, runnerGeneration) } - .onSuccess { - recordReprovision(OutcomeLabel.SUCCESS, FailureReasonLabel.NONE, startedAt) - }.onFailure { ex -> - val reason = reasonClass(ex) - recordReprovision(OutcomeLabel.FAILURE, reason, startedAt) - }.getOrThrow() - } - - private fun provisionAndBuildReady( - workspace: Workspace, - target: RunnerSetupTarget, - runnerGeneration: Long, - ): RunnerReady { - val handle = - runCatching { - orchestrator.scaleDown(workspace) - orchestrator.provision(workspace, target.spec, runnerGeneration) - }.getOrElse { ex -> - log.warn("reprovision of workspace {} failed during scaleDown/provision", workspace.id.value, ex) - throw AgentRunnerUnavailableException( - workspaceId = workspace.id, - runnerStatus = "ReprovisionFailed", - cause = ex, - ) - } - val repointed = - workspace.withPodInfo( - podName = handle.podName, - pvcName = handle.pvcName, - gatewayEndpoint = handle.gatewayEndpoint, - ) - val saved = workspaces.save(repointed) - log.info("re-provisioned runner for workspace {} as pod {}", workspace.id.value, handle.podName) - return RunnerReady( - workspace = saved, - ready = awaitRunnerReady(saved, target, runnerGeneration), - provisioning = - RunnerProvisioningResult.Provisioned( - podName = handle.podName, - pvcName = handle.pvcName, - gatewayEndpoint = handle.gatewayEndpoint, - ), - ) - } - - private fun recordReprovision( - outcome: OutcomeLabel, - reason: FailureReasonLabel, - startedAt: Long, - ) { - telemetry.recordRunnerReprovision(RunnerReprovisionTelemetry(outcome = outcome, reason = reason)) - recordOperation( - operation = OperationLabel.REPROVISION_RUNNER, - mode = ModeLabel.DURABLE, - outcome = outcome, - reason = reason, - startedAt = startedAt, - ) - } - - private fun awaitRunnerReady( - workspace: Workspace, - target: RunnerSetupTarget, - runnerGeneration: Long, - ): Boolean { - val identity = target.spec.identity(runnerGeneration) - repeat(MAX_SPAWN_ATTEMPTS) { attempt -> - if (orchestrator.isReady(workspace, identity) && gateway.isReady(workspace)) return true - if (attempt < MAX_SPAWN_ATTEMPTS - 1) { - val sleepMs = backoffInitialMs * (attempt + 1) - if (sleepMs > 0) Thread.sleep(sleepMs) - } - } - return false - } - - private fun spawnAgentWithRetry( - workspace: Workspace, - session: WorkspaceAgentSession, - continuation: AgentGatewayClient.ContinuationMetadata?, - ): AgentGatewayClient.GatewayAgent { - var lastFailure: ResourceAccessException? = null - repeat(MAX_SPAWN_ATTEMPTS) { attempt -> - try { - return gateway.spawnAgent(buildSpawnRequest(workspace, session, continuation)) - } catch (ex: ResourceAccessException) { - lastFailure = ex - logSpawnRetry(workspace, ex, attempt) + val unavailableReason = + when { + workspace.runnerSetupOperation != RunnerSetupOperation.IDLE || + workspace.pendingRunnerSetupId != null || + workspace.pendingRunnerSetupVersion != null -> + RunnerUnavailableReason.SETUP_OPERATION_IN_PROGRESS.label + workspace.runnerBootLeaseId != null -> RunnerUnavailableReason.BOOT_LEASE_HELD.label + !provisioning.isRunnerReadyFor(workspace, target) -> RunnerUnavailableReason.NOT_READY_AFTER_PROVISION.label + else -> null } + return unavailableReason?.let { + RunnerSessionBindingResult.Unavailable(workspaceId = workspace.id, runnerStatus = it) } - throw AgentRunnerUnavailableException( - workspaceId = workspace.id, - runnerStatus = "ConnectionRefused", - retryAfterSeconds = AgentRunnerUnavailableException.DEFAULT_RETRY_AFTER_SECONDS, - cause = lastFailure, - ) - } - - private fun buildSpawnRequest( - workspace: Workspace, - session: WorkspaceAgentSession, - continuation: AgentGatewayClient.ContinuationMetadata?, - ) = AgentGatewayClient.SpawnAgentRequest( - workspace = workspace, - kind = session.kind, - stableSessionId = session.id, - epoch = session.epoch, - continuation = continuation, - // Null on a fresh start; on revival this carries the prior - // native CLI id so the gateway resumes that conversation - // rather than spawning a blank one. - resumeCliSessionId = session.cliSessionId, - ) - - private fun logSpawnRetry( - workspace: Workspace, - ex: ResourceAccessException, - attempt: Int, - ) { - val sleepMs = backoffInitialMs * (attempt + 1) - log.warn( - "agent spawn attempt {} for workspace {} failed: {} - retrying in {}ms", - attempt + 1, - workspace.id.value, - ex.message, - sleepMs, - ) - if (sleepMs > 0) Thread.sleep(sleepMs) } - private fun observeBindingOperation( - operation: OperationLabel, - mode: ModeLabel, - block: () -> RunnerSessionBindingResult, - ): RunnerSessionBindingResult { - val startedAt = System.nanoTime() - return runCatching(block) - .onSuccess { result -> - val (outcome, reason) = - when (result) { - is RunnerSessionBindingResult.Bound -> OutcomeLabel.SUCCESS to FailureReasonLabel.NONE - is RunnerSessionBindingResult.Conflict -> bindingConflict() - is RunnerSessionBindingResult.Unavailable -> - OutcomeLabel.FAILURE to FailureReasonLabel.UPSTREAM_UNAVAILABLE - } - recordOperation(operation, mode, outcome, reason, startedAt) - }.onFailure { ex -> - recordOperation(operation, mode, OutcomeLabel.FAILURE, reasonClass(ex), startedAt) - }.getOrThrow() - } - - private fun observeStageOperation( - operation: OperationLabel, - mode: ModeLabel, - outcome: (T) -> Pair = { OutcomeLabel.SUCCESS to FailureReasonLabel.NONE }, - block: () -> T, - ): T { - val startedAt = System.nanoTime() - return runCatching(block) - .onSuccess { result -> - val (resultOutcome, reason) = outcome(result) - recordOperation(operation, mode, resultOutcome, reason, startedAt) - }.onFailure { ex -> - recordOperation(operation, mode, OutcomeLabel.FAILURE, reasonClass(ex), startedAt) - }.getOrThrow() - } - - private fun recordOperation( - operation: OperationLabel, - mode: ModeLabel, - outcome: OutcomeLabel, - reason: FailureReasonLabel, - startedAt: Long, - ) { - telemetry.recordOperation( - OperationTelemetry( - operation = operation, - mode = mode, - outcome = outcome, - reason = reason, - duration = Duration.ofNanos((System.nanoTime() - startedAt).coerceAtLeast(0)), - ), - ) - } - - private fun bindingConflict(): Pair = - OutcomeLabel.FAILURE to FailureReasonLabel.CAPACITY - - private fun reasonClass(ex: Throwable): FailureReasonLabel = - when (ex) { - is AgentSetupValidationException, - is IllegalArgumentException, - -> FailureReasonLabel.INVALID_REQUEST - - is AgentRunnerUnavailableException -> FailureReasonLabel.UPSTREAM_UNAVAILABLE - is IOException -> FailureReasonLabel.IO_ERROR - else -> FailureReasonLabel.UNKNOWN - } - companion object { - const val MAX_SPAWN_ATTEMPTS: Int = 3 + const val MAX_SPAWN_ATTEMPTS: Int = RunnerAgentSpawner.MAX_SPAWN_ATTEMPTS const val BACKOFF_INITIAL_MS: Long = 1_000 } - - private data class RunnerReady( - val workspace: Workspace, - // The runner was reprovisioned; `ready` is whether it became ready within - // the synchronous window. When false the session is left awaiting rebind. - val ready: Boolean, - val provisioning: RunnerProvisioningResult, - ) } @Component diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSetupResolver.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSetupResolver.kt new file mode 100644 index 0000000..3d1c2bf --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSetupResolver.kt @@ -0,0 +1,102 @@ +package com.jorisjonkers.personalstack.agents.application.sessionbinding + +import com.jorisjonkers.personalstack.agents.application.setup.AgentSetupSelectionService +import com.jorisjonkers.personalstack.agents.application.setup.AgentSetupValidationInput +import com.jorisjonkers.personalstack.agents.application.setup.AgentSetupValidationService +import com.jorisjonkers.personalstack.agents.application.workspacerunner.RunnerSetupTarget +import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupCatalogEntry +import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupId +import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupVersion +import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupProvisioningSpec +import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession + +/** + * Resolves which agent setup entry (and provisioning spec) to use for a given + * binding operation. Extracted from RunnerSessionBinder to keep that class + * below the TooManyFunctions threshold. + */ +internal class RunnerSetupResolver( + private val setupSelection: AgentSetupSelectionService, + private val setupValidation: AgentSetupValidationService, +) { + fun resolveNewSessionSetup( + workspace: Workspace, + kind: WorkspaceAgentKind, + setupId: AgentSetupId?, + setupVersion: AgentSetupVersion?, + ): RunnerSetupTarget { + if (setupId != null || setupVersion != null) { + require(setupId != null && setupVersion != null) { "setup id and version must be supplied together" } + return requireValidTarget(workspace, kind, null, setupSelection.requireSelectable(setupId, setupVersion)) + } + val current = + runCatching { + val entry = + setupSelection.requireSelectable( + workspace.currentRunnerSetupId, + workspace.currentRunnerSetupVersion, + ) + val result = + setupValidation.validate( + AgentSetupValidationInput( + workspace = workspace, + targetId = entry.definition.id, + targetVersion = entry.definition.version, + agentKind = kind, + ), + ) + if (result.valid) RunnerSetupTarget(entry, RunnerSetupProvisioningSpec.from(entry.definition)) else null + }.getOrNull() + if (current != null) return current + return requireValidTarget(workspace, kind, null, setupSelection.defaultSelectable()) + } + + fun resolveRestartSetup( + workspace: Workspace, + session: WorkspaceAgentSession, + request: RestartRunnerSessionBindingInput, + ): RunnerSetupTarget { + require((request.targetSetupId == null) == (request.targetSetupVersion == null)) { + "target setup id and version must be supplied together" + } + val targetId = request.targetSetupId ?: session.currentSetupId + val targetVersion = request.targetSetupVersion ?: session.currentSetupVersion + return requireValidTarget( + workspace = workspace, + kind = session.kind, + session = session, + entry = setupSelection.requireSelectable(targetId, targetVersion), + ) + } + + fun resolveSessionSetup( + workspace: Workspace, + session: WorkspaceAgentSession, + ): RunnerSetupTarget = + requireValidTarget( + workspace = workspace, + kind = session.kind, + session = session, + entry = setupSelection.requireSelectable(session.currentSetupId, session.currentSetupVersion), + ) + + private fun requireValidTarget( + workspace: Workspace, + kind: WorkspaceAgentKind, + session: WorkspaceAgentSession?, + entry: AgentSetupCatalogEntry, + ): RunnerSetupTarget { + setupValidation.requireValid( + AgentSetupValidationInput( + workspace = workspace, + targetId = entry.definition.id, + targetVersion = entry.definition.version, + session = session, + agentKind = kind, + ), + ) + return RunnerSetupTarget(entry, RunnerSetupProvisioningSpec.from(entry.definition)) + } +} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerAgentSpawnerTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerAgentSpawnerTest.kt new file mode 100644 index 0000000..97c2508 --- /dev/null +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerAgentSpawnerTest.kt @@ -0,0 +1,129 @@ +package com.jorisjonkers.personalstack.agents.application.sessionbinding + +import com.jorisjonkers.personalstack.agents.application.exception.AgentRunnerUnavailableException +import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionStatus +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceId +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceStatus +import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.springframework.web.client.ResourceAccessException +import java.time.Instant + +class RunnerAgentSpawnerTest { + private val gateway = mockk() + private val spawner = RunnerAgentSpawner(gateway, backoffInitialMs = 0) + + @Test + fun `spawnWithRetry succeeds on first attempt`() { + val ws = workspace() + val session = session(ws.id) + val expected = gatewayAgent("abc-1") + every { gateway.spawnAgent(any()) } returns expected + + val result = spawner.spawnWithRetry(ws, session, continuation = null) + + assertThat(result.id).isEqualTo("abc-1") + verify(exactly = 1) { gateway.spawnAgent(any()) } + } + + @Test + fun `spawnWithRetry retries on ResourceAccessException and succeeds`() { + val ws = workspace() + val session = session(ws.id) + val expected = gatewayAgent("abc-2") + every { gateway.spawnAgent(any()) } + .throws(ResourceAccessException("refused")) + .andThen(expected) + + val result = spawner.spawnWithRetry(ws, session, continuation = null) + + assertThat(result.id).isEqualTo("abc-2") + verify(exactly = 2) { gateway.spawnAgent(any()) } + } + + @Test + fun `spawnWithRetry throws AgentRunnerUnavailableException after all attempts fail`() { + val ws = workspace() + val session = session(ws.id) + every { gateway.spawnAgent(any()) } throws ResourceAccessException("refused") + + assertThrows { + spawner.spawnWithRetry(ws, session, continuation = null) + } + verify(exactly = RunnerAgentSpawner.MAX_SPAWN_ATTEMPTS) { gateway.spawnAgent(any()) } + } + + @Test + fun `spawnWithRetry forwards epoch and cliSessionId from session`() { + val ws = workspace() + val session = session(ws.id, epoch = 3, cliSessionId = "native-42") + val captured = mutableListOf() + every { gateway.spawnAgent(capture(captured)) } returns gatewayAgent("abc-3") + + spawner.spawnWithRetry(ws, session, continuation = null) + + assertThat(captured).hasSize(1) + assertThat(captured[0].epoch).isEqualTo(3) + assertThat(captured[0].resumeCliSessionId).isEqualTo("native-42") + } + + @Test + fun `spawnWithRetry forwards continuation metadata`() { + val ws = workspace() + val session = session(ws.id) + val continuation = AgentGatewayClient.ContinuationMetadata(reason = "restart", previousEpoch = 2) + val captured = mutableListOf() + every { gateway.spawnAgent(capture(captured)) } returns gatewayAgent("abc-4") + + spawner.spawnWithRetry(ws, session, continuation) + + assertThat(captured[0].continuation?.reason).isEqualTo("restart") + assertThat(captured[0].continuation?.previousEpoch).isEqualTo(2) + } + + private fun workspace() = + Workspace( + id = WorkspaceId.random(), + name = "demo", + repoUrl = null, + branch = null, + podName = null, + pvcName = null, + gatewayEndpoint = "http://runner:8090", + status = WorkspaceStatus.READY, + createdAt = Instant.now(), + updatedAt = Instant.now(), + ) + + private fun session( + workspaceId: WorkspaceId, + epoch: Long = 1, + cliSessionId: String? = null, + ) = WorkspaceAgentSession( + id = WorkspaceAgentSessionId.random(), + workspaceId = workspaceId, + kind = WorkspaceAgentKind.CLAUDE, + gatewayAgentId = null, + status = WorkspaceAgentSessionStatus.STARTING, + createdAt = Instant.now(), + updatedAt = Instant.now(), + epoch = epoch, + cliSessionId = cliSessionId, + ) + + private fun gatewayAgent(id: String) = + AgentGatewayClient.GatewayAgent( + id = id, + kind = WorkspaceAgentKind.CLAUDE, + cwd = "/workspace", + ) +} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt new file mode 100644 index 0000000..bf75c20 --- /dev/null +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt @@ -0,0 +1,133 @@ +package com.jorisjonkers.personalstack.agents.application.sessionbinding + +import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel +import com.jorisjonkers.personalstack.agents.application.observability.ModeLabel +import com.jorisjonkers.personalstack.agents.application.observability.OperationLabel +import com.jorisjonkers.personalstack.agents.application.observability.OperationTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.OutcomeLabel +import com.jorisjonkers.personalstack.agents.application.observability.RunnerReprovisionTelemetry +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceId +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows + +class RunnerBindingMetricsTest { + private val recording = RecordingTelemetry() + private val metrics = RunnerBindingMetrics(recording) + private val workspaceId = WorkspaceId.random() + + @Test + fun `observeBinding records success when block returns Bound`() { + val bound = + RunnerSessionBindingResult.Bound( + workspace = stubWorkspace(), + session = stubSession(), + gatewayAgent = stubGatewayAgent(), + provisioning = RunnerProvisioningResult.AlreadyReady, + ) + + val result = metrics.observeBinding(OperationLabel.START_SESSION, ModeLabel.INTERACTIVE) { bound } + + assertThat(result).isSameAs(bound) + assertThat(recording.operations).hasSize(1) + recording.operations.single().let { + assertThat(it.operation).isEqualTo(OperationLabel.START_SESSION) + assertThat(it.outcome).isEqualTo(OutcomeLabel.SUCCESS) + assertThat(it.reason).isEqualTo(FailureReasonLabel.NONE) + } + } + + @Test + fun `observeBinding records failure with CAPACITY reason when block returns Conflict`() { + val conflict = RunnerSessionBindingResult.Conflict(current = null) + + metrics.observeBinding(OperationLabel.ATTACH_SESSION, ModeLabel.INTERACTIVE) { conflict } + + assertThat(recording.operations.single().outcome).isEqualTo(OutcomeLabel.FAILURE) + assertThat(recording.operations.single().reason).isEqualTo(FailureReasonLabel.CAPACITY) + } + + @Test + fun `observeBinding records failure with UPSTREAM_UNAVAILABLE reason when block returns Unavailable`() { + val unavailable = RunnerSessionBindingResult.Unavailable(workspaceId = workspaceId, runnerStatus = "NotReady") + + metrics.observeBinding(OperationLabel.ATTACH_SESSION, ModeLabel.INTERACTIVE) { unavailable } + + assertThat(recording.operations.single().outcome).isEqualTo(OutcomeLabel.FAILURE) + assertThat(recording.operations.single().reason).isEqualTo(FailureReasonLabel.UPSTREAM_UNAVAILABLE) + } + + @Test + fun `observeBinding records failure and rethrows when block throws`() { + assertThrows { + metrics.observeBinding(OperationLabel.START_SESSION, ModeLabel.INTERACTIVE) { + throw IllegalStateException("boom") + } + } + assertThat(recording.operations.single().outcome).isEqualTo(OutcomeLabel.FAILURE) + assertThat(recording.operations.single().reason).isEqualTo(FailureReasonLabel.UNKNOWN) + } + + @Test + fun `recordReprovision emits both operation and reprovision telemetry`() { + metrics.recordReprovision(OutcomeLabel.SUCCESS, FailureReasonLabel.NONE, System.nanoTime()) + + assertThat(recording.operations).hasSize(1) + assertThat(recording.reprovisions).hasSize(1) + assertThat(recording.operations.single().operation).isEqualTo(OperationLabel.REPROVISION_RUNNER) + assertThat(recording.reprovisions.single().outcome).isEqualTo(OutcomeLabel.SUCCESS) + } + + @Test + fun `bindingConflict returns FAILURE and CAPACITY pair`() { + val (outcome, reason) = metrics.bindingConflict() + assertThat(outcome).isEqualTo(OutcomeLabel.FAILURE) + assertThat(reason).isEqualTo(FailureReasonLabel.CAPACITY) + } + + private fun stubWorkspace() = + com.jorisjonkers.personalstack.agents.domain.model.Workspace( + id = workspaceId, + name = "demo", + repoUrl = null, + branch = null, + podName = null, + pvcName = null, + gatewayEndpoint = null, + status = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceStatus.READY, + createdAt = java.time.Instant.now(), + updatedAt = java.time.Instant.now(), + ) + + private fun stubSession() = + com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession( + id = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId.random(), + workspaceId = workspaceId, + kind = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind.CLAUDE, + gatewayAgentId = null, + status = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionStatus.RUNNING, + createdAt = java.time.Instant.now(), + updatedAt = java.time.Instant.now(), + ) + + private fun stubGatewayAgent() = + com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient.GatewayAgent( + id = "agent-1", + kind = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind.CLAUDE, + cwd = "/workspace", + ) + + private class RecordingTelemetry : AgentsApiTelemetry { + val operations = mutableListOf() + val reprovisions = mutableListOf() + + override fun recordOperation(event: OperationTelemetry) { + operations += event + } + + override fun recordRunnerReprovision(event: RunnerReprovisionTelemetry) { + reprovisions += event + } + } +} From 6b26ebd23ce895b6930512727d9ec38368c62ef6 Mon Sep 17 00:00:00 2001 From: JorisJonkers Agent Date: Mon, 13 Jul 2026 07:37:16 +0000 Subject: [PATCH 02/10] refactor(HttpAgentGatewayClient): extract HeadlessJobGateway to remove TooManyFunctions/LargeClass Move headless-job logic (startHeadlessJob, pollHeadlessJob, private helpers, extension functions and DTOs) to HeadlessJobGateway. HttpAgentGatewayClient drops from 22 functions to 13, removing @file:Suppress("LargeClass") and @Suppress("TooManyFunctions"). Public API unchanged; headless field delegates. --- .../integration/HeadlessJobGateway.kt | 187 ++++++++++++++++++ .../integration/HttpAgentGatewayClient.kt | 184 ++--------------- 2 files changed, 198 insertions(+), 173 deletions(-) create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HeadlessJobGateway.kt diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HeadlessJobGateway.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HeadlessJobGateway.kt new file mode 100644 index 0000000..e20af99 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HeadlessJobGateway.kt @@ -0,0 +1,187 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.integration + +import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel +import com.jorisjonkers.personalstack.agents.application.observability.ModeLabel +import com.jorisjonkers.personalstack.agents.application.observability.OperationLabel +import com.jorisjonkers.personalstack.agents.application.observability.OperationTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.OutcomeLabel +import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind +import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient +import org.springframework.web.client.ResourceAccessException +import org.springframework.web.client.RestClient +import org.springframework.web.client.RestClientResponseException +import java.time.Duration +import java.time.Instant + +/** + * Handles headless job operations against the agent-gateway sidecar. + * Extracted from HttpAgentGatewayClient to keep that class below the + * TooManyFunctions and LargeClass thresholds. + */ +internal class HeadlessJobGateway( + private val restClient: RestClient, + private val telemetry: AgentsApiTelemetry, + private val endpoint: (Workspace) -> String, +) { + private data class HeadlessRequestBody( + val kind: WorkspaceAgentKind, + val prompt: String, + val cliSessionId: String? = null, + val stableSessionId: String? = null, + val epoch: Long? = null, + val continuation: ContinuationBody? = null, + val timeoutSeconds: Long? = null, + val enableKbHooks: Boolean = false, + val partialMessages: Boolean = false, + ) + + internal data class HeadlessJobDto( + val id: String, + val status: String, + val exitCode: Int? = null, + val output: String? = null, + ) + + fun startHeadlessJob(request: AgentGatewayClient.HeadlessJobRequest): AgentGatewayClient.HeadlessJob { + val dto = + restClient + .post() + .uri("${endpoint(request.workspace)}/agents/headless") + .body( + HeadlessRequestBody( + kind = request.kind, + prompt = request.prompt, + cliSessionId = request.cliSessionId, + stableSessionId = request.stableSessionId?.value?.toString(), + epoch = request.epoch, + continuation = request.continuation?.toBody(), + timeoutSeconds = request.timeoutSeconds, + enableKbHooks = request.enableKbHooks, + partialMessages = request.partialMessages, + ), + ).retrieve() + .body(HeadlessJobDto::class.java) + ?: error("empty response from gateway /agents/headless") + return dto.toDomain() + } + + fun pollHeadlessJob( + workspace: Workspace, + headlessJobId: String, + ): AgentGatewayClient.HeadlessJob { + val startedAt = Instant.now() + return observeJob(startedAt) { + restClient + .get() + .uri("${endpoint(workspace)}/agents/headless/$headlessJobId") + .retrieve() + .body(HeadlessJobDto::class.java) + ?: error("empty response from gateway /agents/headless/$headlessJobId") + } + } + + private fun observeJob( + startedAt: Instant, + fetch: () -> HeadlessJobDto, + ): AgentGatewayClient.HeadlessJob = + runCatching { + val dto = fetch() + Triple(dto.toDomain(), dto.toOutcomeLabel(), dto.toFailureReasonLabel()) + }.onSuccess { (_, outcome, reason) -> + recordPoll(startedAt, outcome, reason) + }.onFailure { ex -> + recordPoll(startedAt, OutcomeLabel.FAILURE, gatewayFailureReason(ex)) + }.getOrThrow() + .first + + private fun recordPoll( + startedAt: Instant, + outcome: OutcomeLabel, + reason: FailureReasonLabel, + ) { + telemetry.recordOperation( + OperationTelemetry( + operation = OperationLabel.OTHER, + mode = ModeLabel.HEADLESS, + outcome = outcome, + reason = reason, + duration = Duration.between(startedAt, Instant.now()), + ), + ) + } + + private fun HeadlessJobDto.toDomain(): AgentGatewayClient.HeadlessJob = + AgentGatewayClient.HeadlessJob( + id = id, + status = statusOrNull() ?: AgentGatewayClient.HeadlessStatus.FAILED, + exitCode = exitCode, + output = output, + ) + + private fun HeadlessJobDto.statusOrNull(): AgentGatewayClient.HeadlessStatus? = + runCatching { AgentGatewayClient.HeadlessStatus.valueOf(status) }.getOrNull() + + private fun HeadlessJobDto.toOutcomeLabel(): OutcomeLabel = + when (statusOrNull()) { + AgentGatewayClient.HeadlessStatus.RUNNING -> OutcomeLabel.SUCCESS + AgentGatewayClient.HeadlessStatus.COMPLETED -> OutcomeLabel.SUCCESS + AgentGatewayClient.HeadlessStatus.FAILED -> OutcomeLabel.FAILURE + AgentGatewayClient.HeadlessStatus.CANCELLED -> OutcomeLabel.CANCELLED + null -> OutcomeLabel.FAILURE + } + + private fun HeadlessJobDto.toFailureReasonLabel(): FailureReasonLabel = + when (statusOrNull()) { + AgentGatewayClient.HeadlessStatus.RUNNING -> FailureReasonLabel.NONE + AgentGatewayClient.HeadlessStatus.COMPLETED -> FailureReasonLabel.NONE + AgentGatewayClient.HeadlessStatus.FAILED -> FailureReasonLabel.PROCESS_EXITED + AgentGatewayClient.HeadlessStatus.CANCELLED -> FailureReasonLabel.CANCELLED + null -> FailureReasonLabel.fromRaw(status) + } + + private fun gatewayFailureReason(ex: Throwable): FailureReasonLabel = + when { + ex is ResourceAccessException && FailureReasonLabel.fromRaw(ex.message) == FailureReasonLabel.TIMEOUT -> + FailureReasonLabel.TIMEOUT + ex is ResourceAccessException -> FailureReasonLabel.UPSTREAM_UNAVAILABLE + ex is RestClientResponseException && + ex.statusCode.value() in setOf(HTTP_REQUEST_TIMEOUT, HTTP_GATEWAY_TIMEOUT) -> + FailureReasonLabel.TIMEOUT + ex is RestClientResponseException && ex.statusCode.is4xxClientError -> FailureReasonLabel.INVALID_REQUEST + ex is RestClientResponseException && ex.statusCode.is5xxServerError -> + FailureReasonLabel.UPSTREAM_UNAVAILABLE + ex is IllegalStateException && ex.message?.contains("gateway endpoint") == true -> + FailureReasonLabel.UPSTREAM_UNAVAILABLE + else -> FailureReasonLabel.fromRaw(ex.message) + } + + private companion object { + const val HTTP_REQUEST_TIMEOUT = 408 + const val HTTP_GATEWAY_TIMEOUT = 504 + } +} + +internal data class ContinuationBody( + val reason: String? = null, + val previousEpoch: Long? = null, + val fromSetupLabel: String? = null, + val toSetupLabel: String? = null, +) + +internal fun AgentGatewayClient.ContinuationMetadata.toBody(): ContinuationBody = + ContinuationBody( + reason = reason, + previousEpoch = previousEpoch, + fromSetupLabel = fromSetupLabel, + toSetupLabel = toSetupLabel, + ) + +internal fun ContinuationBody.toDomain(): AgentGatewayClient.ContinuationMetadata = + AgentGatewayClient.ContinuationMetadata( + reason = reason, + previousEpoch = previousEpoch, + fromSetupLabel = fromSetupLabel, + toSetupLabel = toSetupLabel, + ) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HttpAgentGatewayClient.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HttpAgentGatewayClient.kt index fb6567a..5f7c374 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HttpAgentGatewayClient.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/integration/HttpAgentGatewayClient.kt @@ -1,24 +1,14 @@ -@file:Suppress("LargeClass") - package com.jorisjonkers.personalstack.agents.infrastructure.integration import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry -import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel -import com.jorisjonkers.personalstack.agents.application.observability.ModeLabel -import com.jorisjonkers.personalstack.agents.application.observability.OperationLabel -import com.jorisjonkers.personalstack.agents.application.observability.OperationTelemetry -import com.jorisjonkers.personalstack.agents.application.observability.OutcomeLabel import com.jorisjonkers.personalstack.agents.domain.model.Workspace import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient import org.springframework.http.HttpStatusCode import org.springframework.stereotype.Component -import org.springframework.web.client.ResourceAccessException import org.springframework.web.client.RestClient -import org.springframework.web.client.RestClientResponseException import java.time.Duration -import java.time.Instant /** * Thin REST adapter for the agent-gateway sidecar. Each method is @@ -27,13 +17,16 @@ import java.time.Instant * its own beyond URI building and response mapping. * * One method per gateway endpoint keeps route parity visible in this adapter. + * Headless job operations are delegated to HeadlessJobGateway to keep class size + * within detekt thresholds. */ -@Suppress("TooManyFunctions") @Component class HttpAgentGatewayClient( private val restClient: RestClient, private val telemetry: AgentsApiTelemetry = AgentsApiTelemetry.NOOP, ) : AgentGatewayClient { + private val headless = HeadlessJobGateway(restClient, telemetry, ::endpoint) + private data class GatewayAgentDto( val id: String, val kind: WorkspaceAgentKind, @@ -53,13 +46,6 @@ class HttpAgentGatewayClient( val resumeCliSessionId: String? = null, ) - private data class ContinuationBody( - val reason: String? = null, - val previousEpoch: Long? = null, - val fromSetupLabel: String? = null, - val toSetupLabel: String? = null, - ) - private data class SendBody( val input: String, val enter: Boolean, @@ -98,6 +84,10 @@ class HttpAgentGatewayClient( val output: String, ) + private data class AgentIdleDto( + val idleMillis: Long? = null, + ) + private fun endpoint(workspace: Workspace): String = workspace.gatewayEndpoint ?: error("workspace ${workspace.id} not yet provisioned with a gateway endpoint") @@ -256,163 +246,11 @@ class HttpAgentGatewayClient( ?.let(Duration::ofMillis) }.getOrNull() - private data class AgentIdleDto( - val idleMillis: Long? = null, - ) - - private data class HeadlessRequestBody( - val kind: WorkspaceAgentKind, - val prompt: String, - val cliSessionId: String? = null, - val stableSessionId: String? = null, - val epoch: Long? = null, - val continuation: ContinuationBody? = null, - val timeoutSeconds: Long? = null, - val enableKbHooks: Boolean = false, - val partialMessages: Boolean = false, - ) - - private data class HeadlessJobDto( - val id: String, - val status: String, - val exitCode: Int? = null, - val output: String? = null, - ) - - override fun startHeadlessJob(request: AgentGatewayClient.HeadlessJobRequest): AgentGatewayClient.HeadlessJob { - val dto = - restClient - .post() - .uri("${endpoint(request.workspace)}/agents/headless") - .body( - HeadlessRequestBody( - kind = request.kind, - prompt = request.prompt, - cliSessionId = request.cliSessionId, - stableSessionId = request.stableSessionId?.value?.toString(), - epoch = request.epoch, - continuation = request.continuation?.toBody(), - timeoutSeconds = request.timeoutSeconds, - enableKbHooks = request.enableKbHooks, - partialMessages = request.partialMessages, - ), - ).retrieve() - .body(HeadlessJobDto::class.java) - ?: error("empty response from gateway /agents/headless") - return dto.toDomain() - } + override fun startHeadlessJob(request: AgentGatewayClient.HeadlessJobRequest): AgentGatewayClient.HeadlessJob = + headless.startHeadlessJob(request) override fun pollHeadlessJob( workspace: Workspace, headlessJobId: String, - ): AgentGatewayClient.HeadlessJob { - val startedAt = Instant.now() - return observeHeadlessJob(startedAt, ::recordHeadlessPoll) { - restClient - .get() - .uri("${endpoint(workspace)}/agents/headless/$headlessJobId") - .retrieve() - .body(HeadlessJobDto::class.java) - ?: error("empty response from gateway /agents/headless/$headlessJobId") - } - } - - private fun HeadlessJobDto.toDomain(): AgentGatewayClient.HeadlessJob = - AgentGatewayClient.HeadlessJob( - id = id, - status = statusOrNull() ?: AgentGatewayClient.HeadlessStatus.FAILED, - exitCode = exitCode, - output = output, - ) - - private fun HeadlessJobDto.statusOrNull(): AgentGatewayClient.HeadlessStatus? = - runCatching { - AgentGatewayClient.HeadlessStatus.valueOf(status) - }.getOrNull() - - private fun AgentGatewayClient.ContinuationMetadata.toBody(): ContinuationBody = - ContinuationBody( - reason = reason, - previousEpoch = previousEpoch, - fromSetupLabel = fromSetupLabel, - toSetupLabel = toSetupLabel, - ) - - private fun ContinuationBody.toDomain(): AgentGatewayClient.ContinuationMetadata = - AgentGatewayClient.ContinuationMetadata( - reason = reason, - previousEpoch = previousEpoch, - fromSetupLabel = fromSetupLabel, - toSetupLabel = toSetupLabel, - ) - - private fun observeHeadlessJob( - startedAt: Instant, - record: (Instant, OutcomeLabel, FailureReasonLabel) -> Unit, - fetch: () -> HeadlessJobDto, - ): AgentGatewayClient.HeadlessJob = - runCatching { - val dto = fetch() - Triple(dto.toDomain(), dto.toOutcomeLabel(), dto.toFailureReasonLabel()) - }.onSuccess { (_, outcome, reason) -> - record(startedAt, outcome, reason) - }.onFailure { ex -> - record(startedAt, OutcomeLabel.FAILURE, gatewayFailureReason(ex)) - }.getOrThrow() - .first - - private fun recordHeadlessPoll( - startedAt: Instant, - outcome: OutcomeLabel, - reason: FailureReasonLabel, - ) { - telemetry.recordOperation( - OperationTelemetry( - operation = OperationLabel.OTHER, - mode = ModeLabel.HEADLESS, - outcome = outcome, - reason = reason, - duration = Duration.between(startedAt, Instant.now()), - ), - ) - } - - private fun HeadlessJobDto.toOutcomeLabel(): OutcomeLabel = - when (statusOrNull()) { - AgentGatewayClient.HeadlessStatus.RUNNING -> OutcomeLabel.SUCCESS - AgentGatewayClient.HeadlessStatus.COMPLETED -> OutcomeLabel.SUCCESS - AgentGatewayClient.HeadlessStatus.FAILED -> OutcomeLabel.FAILURE - AgentGatewayClient.HeadlessStatus.CANCELLED -> OutcomeLabel.CANCELLED - null -> OutcomeLabel.FAILURE - } - - private fun HeadlessJobDto.toFailureReasonLabel(): FailureReasonLabel = - when (statusOrNull()) { - AgentGatewayClient.HeadlessStatus.RUNNING -> FailureReasonLabel.NONE - AgentGatewayClient.HeadlessStatus.COMPLETED -> FailureReasonLabel.NONE - AgentGatewayClient.HeadlessStatus.FAILED -> FailureReasonLabel.PROCESS_EXITED - AgentGatewayClient.HeadlessStatus.CANCELLED -> FailureReasonLabel.CANCELLED - null -> FailureReasonLabel.fromRaw(status) - } - - private fun gatewayFailureReason(ex: Throwable): FailureReasonLabel = - when { - ex is ResourceAccessException && FailureReasonLabel.fromRaw(ex.message) == FailureReasonLabel.TIMEOUT -> - FailureReasonLabel.TIMEOUT - ex is ResourceAccessException -> FailureReasonLabel.UPSTREAM_UNAVAILABLE - ex is RestClientResponseException && - ex.statusCode.value() in setOf(HTTP_REQUEST_TIMEOUT, HTTP_GATEWAY_TIMEOUT) -> - FailureReasonLabel.TIMEOUT - ex is RestClientResponseException && ex.statusCode.is4xxClientError -> FailureReasonLabel.INVALID_REQUEST - ex is RestClientResponseException && ex.statusCode.is5xxServerError -> - FailureReasonLabel.UPSTREAM_UNAVAILABLE - ex is IllegalStateException && ex.message?.contains("gateway endpoint") == true -> - FailureReasonLabel.UPSTREAM_UNAVAILABLE - else -> FailureReasonLabel.fromRaw(ex.message) - } - - private companion object { - const val HTTP_REQUEST_TIMEOUT = 408 - const val HTTP_GATEWAY_TIMEOUT = 504 - } + ): AgentGatewayClient.HeadlessJob = headless.pollHeadlessJob(workspace, headlessJobId) } From 133b29c89bb314a52e755b4ed5c09ab9d1708147 Mon Sep 17 00:00:00 2001 From: JorisJonkers Agent Date: Mon, 13 Jul 2026 07:37:25 +0000 Subject: [PATCH 03/10] refactor(Fabric8AgentRunnerOrchestrator): split pod/credential/state helpers to remove TooManyFunctions/LargeClass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract RunnerCredentialSecretManager (credential secret lifecycle), RunnerPodSpecBuilder (pod/pvc/service spec construction), and RunnerStateReader (pod label → RunnerState mapping). Orchestrator drops from 42 functions to 13, removing @Suppress("TooManyFunctions","LargeClass"). Update AgentRunnerObservabilityContractTest to read RunnerPodSpecBuilder.kt (where env/probe code now lives). Add RunnerStateReaderTest. --- .../k8s/Fabric8AgentRunnerOrchestrator.kt | 740 +----------------- .../k8s/RunnerCredentialSecretManager.kt | 115 +++ .../k8s/RunnerPodSpecBuilder.kt | 547 +++++++++++++ .../infrastructure/k8s/RunnerStateReader.kt | 47 ++ .../AgentRunnerObservabilityContractTest.kt | 16 +- .../k8s/RunnerStateReaderTest.kt | 113 +++ 6 files changed, 868 insertions(+), 710 deletions(-) create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerCredentialSecretManager.kt create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReader.kt create mode 100644 api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt index 85c1104..4ec81f3 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt @@ -1,7 +1,6 @@ package com.jorisjonkers.personalstack.agents.infrastructure.k8s import com.jorisjonkers.personalstack.agents.config.AgentRuntimeProperties -import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupId import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupVersion import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupProvisioningSpec @@ -11,25 +10,12 @@ import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialReposito import com.jorisjonkers.personalstack.agents.domain.port.AgentRunnerOrchestrator import com.jorisjonkers.personalstack.agents.domain.port.RepositoryRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepositoryRepository -import io.fabric8.kubernetes.api.model.ContainerBuilder -import io.fabric8.kubernetes.api.model.ContainerPortBuilder -import io.fabric8.kubernetes.api.model.EnvVarBuilder -import io.fabric8.kubernetes.api.model.PersistentVolumeClaim -import io.fabric8.kubernetes.api.model.PersistentVolumeClaimBuilder import io.fabric8.kubernetes.api.model.Pod -import io.fabric8.kubernetes.api.model.PodBuilder -import io.fabric8.kubernetes.api.model.Quantity -import io.fabric8.kubernetes.api.model.SecretBuilder -import io.fabric8.kubernetes.api.model.ServiceBuilder -import io.fabric8.kubernetes.api.model.Volume -import io.fabric8.kubernetes.api.model.VolumeBuilder -import io.fabric8.kubernetes.api.model.VolumeMountBuilder import io.fabric8.kubernetes.client.KubernetesClient import org.slf4j.LoggerFactory import org.springframework.beans.factory.ObjectProvider import org.springframework.context.annotation.Profile import org.springframework.stereotype.Component -import java.util.Base64 import java.util.concurrent.TimeUnit /** @@ -47,23 +33,22 @@ import java.util.concurrent.TimeUnit * HTTPS via a git credential helper, so repository access stays * scoped to repos the App is installed on. * - * fabric8's fluent builder chains naturally split into one helper per - * pod section (labels, env, mounts, volumes, container body); below - * the 15-function / LargeClass detekt thresholds would defeat the - * readability win. The class stays as a single orchestrator because - * every helper here operates on the same shared props + client. + * Pod construction, credential management, and state-reading are delegated to + * RunnerPodSpecBuilder, RunnerCredentialSecretManager, and RunnerStateReader. */ -@Suppress("TooManyFunctions", "LargeClass") @Component @Profile("!system-test") class Fabric8AgentRunnerOrchestrator( private val client: KubernetesClient, private val props: AgentRuntimeProperties, - private val credentialsProvider: ObjectProvider, - private val workspaceRepos: ObjectProvider, - private val repositories: ObjectProvider, + credentialsProvider: ObjectProvider, + workspaceRepos: ObjectProvider, + repositories: ObjectProvider, ) : AgentRunnerOrchestrator { private val log = LoggerFactory.getLogger(Fabric8AgentRunnerOrchestrator::class.java) + private val credentials = RunnerCredentialSecretManager(client, props, credentialsProvider) + private val podSpec = RunnerPodSpecBuilder(props, workspaceRepos, repositories, ::ownReleaseVersion) + private val stateReader = RunnerStateReader(::ownReleaseVersion) override fun provision(workspace: Workspace): AgentRunnerOrchestrator.RunnerHandle = provision(workspace, legacySetupSpec(), workspace.runnerSetupGeneration) @@ -80,15 +65,8 @@ class Fabric8AgentRunnerOrchestrator( pvc = "workspace-$short", service = "agent-runner-$short", ) - val resources = - RunnerResources( - workspace = workspace, - setup = setup, - runnerGeneration = runnerGeneration, - names = names, - credentialSecret = ensureCredentialSecret(workspace, short), - ) - applyResources(resources) + val credentialSecret = credentials.ensureCredentialSecret(workspace, short) + applyResources(workspace, setup, runnerGeneration, names, credentialSecret) val endpoint = "http://${names.service}.${props.namespace}.svc.cluster.local:${setup.gatewayPort}" log.info( "provisioned runner pod {} for workspace {} using setup {}@{} generation {}", @@ -105,35 +83,27 @@ class Fabric8AgentRunnerOrchestrator( ) } - private data class RunnerResourceNames( - val pod: String, - val pvc: String, - val service: String, - ) - - private data class RunnerResources( - val workspace: Workspace, - val setup: RunnerSetupProvisioningSpec, - val runnerGeneration: Long, - val names: RunnerResourceNames, - val credentialSecret: CredentialSecret?, - ) - - private fun applyResources(resources: RunnerResources) { + private fun applyResources( + workspace: Workspace, + setup: RunnerSetupProvisioningSpec, + runnerGeneration: Long, + names: RunnerResourceNames, + credentialSecret: RunnerCredentialSecretManager.CredentialSecret?, + ) { client .persistentVolumeClaims() .inNamespace(props.namespace) - .resource(pvc(resources.names.pvc)) + .resource(podSpec.pvc(names.pvc)) .serverSideApply() client .pods() .inNamespace(props.namespace) - .resource(pod(resources)) + .resource(podSpec.pod(workspace, setup, runnerGeneration, names, credentialSecret)) .serverSideApply() client .services() .inNamespace(props.namespace) - .resource(service(resources.names.service, resources.names.pod, resources.setup.gatewayPort)) + .resource(podSpec.service(names.service, names.pod, setup.gatewayPort)) .serverSideApply() } @@ -188,7 +158,7 @@ class Fabric8AgentRunnerOrchestrator( client .secrets() .inNamespace(props.namespace) - .withName(credentialSecretName(short)) + .withName(credentials.credentialSecretName(short)) .delete() log.info("destroyed runner pod and PVC for workspace {}", workspace.id) } @@ -201,12 +171,10 @@ class Fabric8AgentRunnerOrchestrator( .inNamespace(props.namespace) .withName(name) .get() ?: return null - return runnerState(pod) + return stateReader.runnerState(pod) } override fun isRunnerImageStale(workspace: Workspace): Boolean { - // Stale when the runner's pinned version differs from the release - // agents-api itself is on. Unknown on either side → never recycle. val current = runnerImageVersion(workspace) ?: return false val target = targetRunnerImageVersion() ?: return false return current != target @@ -216,16 +184,6 @@ class Fabric8AgentRunnerOrchestrator( override fun targetRunnerImageVersion(): String? = ownReleaseVersion() - /** - * The release version this agents-api process is running, baked into the - * image as `SERVICE_VERSION` (= the release-please tag, e.g. "v0.12.0"). - * The whole suite is published in lockstep, so this is also the target - * agent-runner version. Null on a local/dev build where it is unset or - * "unknown", which disables upgrade detection rather than guessing. - */ - private fun ownReleaseVersion(): String? = - System.getenv("SERVICE_VERSION")?.takeIf { it.isNotBlank() && it != "unknown" } - override fun isReady(workspace: Workspace): Boolean { val state = runnerState(workspace) ?: return false return state.containerReady && @@ -245,613 +203,15 @@ class Fabric8AgentRunnerOrchestrator( return state.containerReady && state.phase == "Running" && state.matches(expectedIdentity) } - private fun runnerState(pod: Pod): RunnerState { - val containerReady = - pod.status - ?.containerStatuses - ?.firstOrNull() - ?.ready ?: false - val labels = pod.metadata?.labels.orEmpty() - val annotations = pod.metadata?.annotations.orEmpty() - return RunnerState( - podName = pod.metadata?.name.orEmpty(), - workspaceId = labels[RunnerState.LABEL_WORKSPACE_ID], - setupId = labels[RunnerState.LABEL_SETUP_ID]?.let(::parseSetupId), - setupVersion = labels[RunnerState.LABEL_SETUP_VERSION]?.let(::parseSetupVersion), - setupHash = annotations[RunnerState.ANNOTATION_SETUP_HASH], - runnerGeneration = labels[RunnerState.LABEL_RUNNER_GENERATION]?.toLongOrNull(), - phase = pod.status?.phase, - containerReady = containerReady, - runnerImageVersion = - RunnerImageVersions.tagOf( - pod.spec - ?.containers - ?.firstOrNull() - ?.image, - ), - ) - } - - private fun parseSetupId(value: String): AgentSetupId? = runCatching { AgentSetupId(value) }.getOrNull() - - private fun parseSetupVersion(value: String): AgentSetupVersion? = - value.toLongOrNull()?.let { runCatching { AgentSetupVersion(it) }.getOrNull() } - - private fun credentialSecretName(short: String): String = "agent-runner-credentials-$short" - - private data class CredentialSecret( - val name: String, - val hasClaude: Boolean, - val hasClaudeCredentialsJson: Boolean, - val hasClaudeAccountJson: Boolean, - val hasCodex: Boolean, - val hasCodexConfig: Boolean, - ) - - private fun ensureCredentialSecret( - workspace: Workspace, - short: String, - ): CredentialSecret? { - val name = credentialSecretName(short) - val data = - credentialSecretData(workspace) - ?: run { - client - .secrets() - .inNamespace(props.namespace) - .withName(name) - .delete() - return null - } - client - .secrets() - .inNamespace(props.namespace) - .resource( - SecretBuilder() - .withNewMetadata() - .withName(name) - .withNamespace(props.namespace) - .withLabels( - mapOf( - "app.kubernetes.io/part-of" to "agent-runner", - "agent-runner/workspace-id" to short, - ), - ).endMetadata() - .withType("Opaque") - .withData(data) - .build(), - ).serverSideApply() - return CredentialSecret( - name = name, - hasClaude = data.containsKey("claude_oauth_token"), - hasClaudeCredentialsJson = data.containsKey("claude_credentials_json"), - hasClaudeAccountJson = data.containsKey("claude_account_json"), - hasCodex = data.containsKey("codex_auth_json"), - hasCodexConfig = data.containsKey("codex_config_toml"), - ) - } - - private fun credentialSecretData(workspace: Workspace): Map? { - val owner = workspace.ownerUserId?.takeIf { it.isNotBlank() } ?: return null - val store = credentialsProvider.ifAvailable ?: return null - val data = - buildMap { - val claude = loadCredential(store, owner, AgentCredentialProvider.CLAUDE) - claude?.payload?.get("oauth_token")?.takeIf { it.isNotBlank() }?.let { - put("claude_oauth_token", b64(it)) - } - claude?.payload?.get("credentials_json")?.takeIf { it.isNotBlank() }?.let { - put("claude_credentials_json", b64(it)) - } - claude?.payload?.get("account_json")?.takeIf { it.isNotBlank() }?.let { - put("claude_account_json", b64(it)) - } - val codex = loadCredential(store, owner, AgentCredentialProvider.CODEX) - val codexAuth = codex?.payload?.get("auth_json")?.takeIf { it.isNotBlank() } - val codexConfig = codex?.payload?.get("config_toml")?.takeIf { it.isNotBlank() } - if (codexAuth != null) { - put("codex_auth_json", b64(codexAuth)) - // config_toml is optional; the runner self-provisions one when absent. - codexConfig?.let { put("codex_config_toml", b64(it)) } - } - } - return data.takeIf { it.isNotEmpty() } - } - - private fun loadCredential( - store: AgentCredentialRepository, - owner: String, - provider: AgentCredentialProvider, - ) = runCatching { store.find(owner, provider) } - .onFailure { log.warn("could not load {} credential for workspace owner", provider) } - .getOrNull() - ?.takeUnless { it.valid == false } - - private fun b64(s: String): String = Base64.getEncoder().encodeToString(s.toByteArray()) - - private fun pvc(name: String): PersistentVolumeClaim = - PersistentVolumeClaimBuilder() - .withNewMetadata() - .withName(name) - .withNamespace(props.namespace) - .endMetadata() - .withNewSpec() - .withAccessModes("ReadWriteOnce") - .withStorageClassName(props.workspaceStorageClass) - .withNewResources() - .withRequests(mapOf("storage" to Quantity(props.workspaceStorageSize))) - .endResources() - .endSpec() - .build() - - // fabric8 builder chains can't be cleanly split into helpers - // because the intermediate fluent types are private; LongMethod - // is the natural shape here and suppressed with intent. The - // inline `mapOf(...)` calls are likewise intentional: extracting - // them into typed vals trips Kotlin overload resolution on - // fabric8 7.x's `withLabels` / `withRequests` / `withLimits`. - @Suppress("LongMethod") - private fun pod(resources: RunnerResources): Pod = - PodBuilder() - .withNewMetadata() - .withName(resources.names.pod) - .withNamespace(props.namespace) - .withLabels(podLabels(resources.workspace, resources.setup, resources.runnerGeneration)) - .withAnnotations(podAnnotations(resources.setup)) - .endMetadata() - .withNewSpec() - .withServiceAccountName(resources.setup.serviceAccount) - // The runner never calls the Kubernetes API itself — cluster - // reads go through the read-only kubernetes MCP server over - // HTTP. Don't project the SA token into the Pod, so the agent - // (which runs unsandboxed) holds no API credential at all and - // cannot reach the API server even if the SA were later granted - // RBAC by mistake. - .withAutomountServiceAccountToken(false) - .withNodeSelector(resources.setup.nodeSelector) - .withRestartPolicy("Always") - .withNewSecurityContext() - .withRunAsUser(RUN_AS_UID) - .withRunAsGroup(RUN_AS_GID) - .withFsGroup(FS_GROUP) - .withSupplementalGroups(podSupplementalGroups(resources.setup)) - .endSecurityContext() - .withInitContainers(agentStateInitContainer(resources.setup)) - .addNewContainer() - .withName("agent-runner") - // Pin to the release agents-api itself is on so the running - // version is a verifiable fact in the Pod spec, not :latest. - .withImage(RunnerImageVersions.pin(resources.setup.image, ownReleaseVersion())) - .withImagePullPolicy(resources.setup.imagePullPolicy) - .withPorts( - ContainerPortBuilder() - .withName("gateway") - .withContainerPort(resources.setup.gatewayPort) - .build(), - ).withEnv( - podEnv( - resources.workspace, - resources.setup, - resources.runnerGeneration, - resources.credentialSecret, - ), - ).withVolumeMounts(podVolumeMounts(resources.setup, resources.credentialSecret)) - // Startup probe gates liveness + readiness until the gateway's - // JVM has finished its cold start. Without it the liveness probe - // (failureThreshold 3 x 10s ~= 30s, no initial delay) killed the - // booting Spring Boot gateway before it bound :8090, which - // re-provisioned the runner in a loop and 503'd every - // start-session. 60 x 5s = 5 min of boot headroom. - .withNewStartupProbe() - .withNewHttpGet() - .withPath("/healthz") - .withNewPort("gateway") - .endHttpGet() - .withPeriodSeconds(STARTUP_PERIOD_SECONDS) - .withFailureThreshold(STARTUP_FAILURE_THRESHOLD) - .endStartupProbe() - .withNewReadinessProbe() - .withNewHttpGet() - .withPath("/healthz") - .withNewPort("gateway") - .endHttpGet() - .withPeriodSeconds(READINESS_PERIOD_SECONDS) - .withFailureThreshold(READINESS_FAILURE_THRESHOLD) - .endReadinessProbe() - .withNewLivenessProbe() - .withNewHttpGet() - .withPath("/healthz") - .withNewPort("gateway") - .endHttpGet() - .withPeriodSeconds(LIVENESS_PERIOD_SECONDS) - .endLivenessProbe() - .withNewResources() - .withRequests(mapOf("cpu" to Quantity(CPU_REQUEST), "memory" to Quantity(MEMORY_REQUEST))) - .withLimits(mapOf("cpu" to Quantity(CPU_LIMIT), "memory" to Quantity(MEMORY_LIMIT))) - .endResources() - .endContainer() - .withVolumes(podVolumes(resources.names.pvc, resources.credentialSecret, resources.setup)) - .endSpec() - .build() - - private fun podLabels( - workspace: Workspace, - setup: RunnerSetupProvisioningSpec, - runnerGeneration: Long, - ): Map = - mapOf( - "app.kubernetes.io/name" to "agent-runner", - "app.kubernetes.io/part-of" to "agent-runner", - RunnerState.LABEL_WORKSPACE_ID to workspace.id.short(), - RunnerState.LABEL_SETUP_ID to setup.setupId.value, - RunnerState.LABEL_SETUP_VERSION to setup.setupVersion.value.toString(), - RunnerState.LABEL_RUNNER_GENERATION to runnerGeneration.toString(), - ) - - private fun podAnnotations(setup: RunnerSetupProvisioningSpec): Map = - mapOf(RunnerState.ANNOTATION_SETUP_HASH to setup.setupHash) - - private fun podEnv( - workspace: Workspace, - setup: RunnerSetupProvisioningSpec, - runnerGeneration: Long, - credentialSecret: CredentialSecret?, - ) = buildList { - add(EnvVarBuilder().withName("HOME").withValue("/home/agent").build()) - add(EnvVarBuilder().withName("CODEX_HOME").withValue("/home/agent/.codex").build()) - add(EnvVarBuilder().withName("DEPLOYMENT_ENVIRONMENT").withValue("production").build()) - add(EnvVarBuilder().withName("OTEL_SERVICE_NAME").withValue("agent-gateway").build()) - add( - EnvVarBuilder() - .withName("OTEL_EXPORTER_OTLP_ENDPOINT") - .withValue("http://alloy.observability.svc.cluster.local:4318") - .build(), - ) - add(EnvVarBuilder().withName("OTEL_EXPORTER_OTLP_PROTOCOL").withValue("http/protobuf").build()) - // The runner Pod is the outer sandbox for the agent process. - // Docker socket access is the explicit host-equivalent exception - // for Testcontainers and Docker CLI workflows. IS_SANDBOX tells - // Claude Code so that --dangerously-skip-permissions runs without - // the bypass-mode warning + acceptance prompt. - add(EnvVarBuilder().withName("IS_SANDBOX").withValue("1").build()) - add(EnvVarBuilder().withName("AGENT_MCP_PROFILE").withValue(setup.mcpProfile).build()) - add(EnvVarBuilder().withName("AGENT_MCP_DIR").withValue(setup.mcpDir).build()) - setup.claudeMcpServersFile?.let { - add(EnvVarBuilder().withName("AGENT_MCP_SERVERS_FILE").withValue(it).build()) - } - setup.codexMcpServersFile?.let { - add(EnvVarBuilder().withName("AGENT_CODEX_MCP_FILE").withValue(it).build()) - } - setup.githubMcpToolsets?.let { - add(EnvVarBuilder().withName("GITHUB_MCP_TOOLSETS").withValue(it).build()) - } - setup.githubMcpExcludeTools?.let { - add(EnvVarBuilder().withName("GITHUB_MCP_EXCLUDE_TOOLS").withValue(it).build()) - } - add(EnvVarBuilder().withName("AGENT_GATEWAY_RUNNER_SETUP_ID").withValue(setup.setupId.value).build()) - add( - EnvVarBuilder() - .withName("AGENT_GATEWAY_RUNNER_SETUP_VERSION") - .withValue(setup.setupVersion.value.toString()) - .build(), - ) - add(EnvVarBuilder().withName("AGENT_GATEWAY_RUNNER_SETUP_HASH").withValue(setup.setupHash).build()) - add( - EnvVarBuilder() - .withName("AGENT_GATEWAY_RUNNER_GENERATION") - .withValue(runnerGeneration.toString()) - .build(), - ) - addAll(dockerEnv(setup)) - addAll(knowledgeEnv(setup)) - addAll(githubAppTokenEnv()) - addAll(agentCredentialEnv(credentialSecret)) - // REPO_URL/REPO_BRANCH drive the entrypoint's boot-time clone - // into /workspace/. Cloning in the runner removes the race that - // left repo-backed workspaces empty: the old create-time - // gateway.clone fired before the runner gateway was up and was - // swallowed. Only repo-backed workspaces carry a repoUrl. - workspace.repoUrl?.let { url -> - add(EnvVarBuilder().withName("REPO_URL").withValue(url).build()) - workspace.branch?.let { add(EnvVarBuilder().withName("REPO_BRANCH").withValue(it).build()) } - } - // REPO_URLS carries the workspace's additional repos (everything - // attached that is not the primary), as url#branch entries. The - // entrypoint clones each into /workspace/ over the - // App-token credential helper. - additionalRepoUrls(workspace).takeIf { it.isNotEmpty() }?.let { urls -> - add(EnvVarBuilder().withName("REPO_URLS").withValue(urls.joinToString(" ")).build()) - } - } - - private fun additionalRepoUrls(workspace: Workspace): List { - val links = workspaceRepos.ifAvailable ?: return emptyList() - val repos = repositories.ifAvailable ?: return emptyList() - return links - .findAllByWorkspaceId(workspace.id) - .filterNot { it.isPrimary } - .mapNotNull { link -> - repos.findById(link.repositoryId)?.let { repo -> "${repo.repoUrl}#${repo.defaultBranch}" } - } - } - - private fun dockerEnv(setup: RunnerSetupProvisioningSpec) = - if (!setup.dockerSocketEnabled) { - emptyList() - } else { - listOf( - EnvVarBuilder().withName("DOCKER_HOST").withValue("unix://${setup.dockerSocketPath}").build(), - EnvVarBuilder() - .withName("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE") - .withValue(setup.dockerSocketPath) - .build(), - EnvVarBuilder() - .withName("AGENT_RUNNER_NODE_HOST_IP") - .withNewValueFrom() - .withNewFieldRef() - .withFieldPath("status.hostIP") - .endFieldRef() - .endValueFrom() - .build(), - EnvVarBuilder() - .withName("TESTCONTAINERS_HOST_OVERRIDE") - .withValue("$(AGENT_RUNNER_NODE_HOST_IP)") - .build(), - ) - } - - // KB_URL + KB_BEARER_TOKEN are the exact names the knowledge-system - // install.sh hooks read; without the bearer every hook short-circuits - // to a no-op and the knowledge.* MCP tools are unreachable. - private fun knowledgeEnv(setup: RunnerSetupProvisioningSpec) = - listOf( - EnvVarBuilder().withName("KB_URL").withValue(setup.knowledgeBaseUrl).build(), - EnvVarBuilder() - .withName("KB_BEARER_TOKEN") - .withNewValueFrom() - .withNewSecretKeyRef() - .withName(setup.knowledgeBearerSecret) - .withKey(setup.knowledgeBearerSecretKey) - .endSecretKeyRef() - .endValueFrom() - .build(), - ) - - // The bearer is an optional Secret ref: an absent github-app Secret - // keeps the Pod starting and the `gh` wrapper degrades to a no-op. - private fun githubAppTokenEnv() = - listOf( - EnvVarBuilder().withName("GITHUB_APP_TOKEN_URL").withValue(props.githubAppTokenUrl).build(), - EnvVarBuilder() - .withName("GITHUB_APP_TOKEN_BEARER") - .withNewValueFrom() - .withNewSecretKeyRef() - .withName(props.githubAppBearerSecret) - .withKey(props.githubAppBearerSecretKey) - .withOptional(true) - .endSecretKeyRef() - .endValueFrom() - .build(), - ) - - private fun agentCredentialEnv(credentialSecret: CredentialSecret?) = - if (credentialSecret == null) { - emptyList() - } else { - buildList { - if (credentialSecret.hasClaudeCredentialsJson) { - add( - EnvVarBuilder() - .withName("AGENT_CLAUDE_CREDENTIALS_FILE") - .withValue("$AGENT_CREDENTIALS_MOUNT/claude_credentials_json") - .build(), - ) - } - if (credentialSecret.hasClaudeAccountJson) { - add( - EnvVarBuilder() - .withName("AGENT_CLAUDE_ACCOUNT_FILE") - .withValue("$AGENT_CREDENTIALS_MOUNT/claude_account_json") - .build(), - ) - } - if (credentialSecret.hasClaude && !credentialSecret.hasClaudeCredentialsJson) { - add( - EnvVarBuilder() - .withName("CLAUDE_CODE_OAUTH_TOKEN") - .withNewValueFrom() - .withNewSecretKeyRef() - .withName(credentialSecret.name) - .withKey("claude_oauth_token") - .endSecretKeyRef() - .endValueFrom() - .build(), - ) - } - if (credentialSecret.hasCodex) { - add( - EnvVarBuilder() - .withName("AGENT_CODEX_AUTH_JSON_FILE") - .withValue("$AGENT_CREDENTIALS_MOUNT/codex_auth_json") - .build(), - ) - } - if (credentialSecret.hasCodexConfig) { - add( - EnvVarBuilder() - .withName("AGENT_CODEX_CONFIG_TOML_FILE") - .withValue("$AGENT_CREDENTIALS_MOUNT/codex_config_toml") - .build(), - ) - } - } - } - - private fun podVolumeMounts( - setup: RunnerSetupProvisioningSpec, - credentialSecret: CredentialSecret?, - ) = buildList { - add(VolumeMountBuilder().withName("workspace").withMountPath("/workspace").build()) - addAll(agentStateVolumeMounts()) - if (credentialSecret != null) { - add( - VolumeMountBuilder() - .withName(AGENT_CREDENTIALS_VOLUME) - .withMountPath(AGENT_CREDENTIALS_MOUNT) - .withReadOnly(true) - .build(), - ) - } - if (setup.dockerSocketEnabled) { - add( - VolumeMountBuilder() - .withName(DOCKER_SOCKET_VOLUME) - .withMountPath(setup.dockerSocketPath) - .build(), - ) - } - // Declarative MCP server set; the entrypoint seeds it into - // ~/.claude.json. Optional volume, so an absent ConfigMap - // leaves the runner with no managed MCP servers. - add( - VolumeMountBuilder() - .withName("mcp-config") - .withMountPath(setup.mcpDir) - .withReadOnly(true) - .build(), - ) - } - - private fun agentStateInitContainer(setup: RunnerSetupProvisioningSpec) = - ContainerBuilder() - .withName("agent-state-init") - .withImage(RunnerImageVersions.pin(setup.image, ownReleaseVersion())) - .withImagePullPolicy(setup.imagePullPolicy) - .withCommand("/bin/sh", "-c") - .withArgs( - "mkdir -p /workspace/.agent-state/claude/projects " + - "/workspace/.agent-state/claude/backups " + - "/workspace/.agent-state/claude/todos " + - "/workspace/.agent-state/claude/shell-snapshots " + - "/workspace/.agent-state/codex/session-homes && " + - "chown -R 1000:1000 /workspace/.agent-state", - ).withVolumeMounts(VolumeMountBuilder().withName("workspace").withMountPath("/workspace").build()) - .withNewSecurityContext() - .withRunAsUser(0L) - .withRunAsGroup(0L) - .endSecurityContext() - .build() - - private fun agentStateVolumeMounts() = - listOf( - agentStateVolumeMount( - mountPath = "/home/agent/.claude/projects", - subPath = ".agent-state/claude/projects", - ), - agentStateVolumeMount( - mountPath = "/home/agent/.claude/backups", - subPath = ".agent-state/claude/backups", - ), - agentStateVolumeMount( - mountPath = "/home/agent/.claude/todos", - subPath = ".agent-state/claude/todos", - ), - agentStateVolumeMount( - mountPath = "/home/agent/.claude/shell-snapshots", - subPath = ".agent-state/claude/shell-snapshots", - ), - agentStateVolumeMount( - mountPath = "/home/agent/.codex/session-homes", - subPath = ".agent-state/codex/session-homes", - ), - ) - - private fun agentStateVolumeMount( - mountPath: String, - subPath: String, - ) = VolumeMountBuilder() - .withName("workspace") - .withMountPath(mountPath) - .withSubPath(subPath) - .build() - - private fun podVolumes( - workspacePvc: String, - credentialSecret: CredentialSecret?, - setup: RunnerSetupProvisioningSpec, - ) = buildList { - add(pvcVolume("workspace", workspacePvc)) - credentialSecret?.let { add(agentCredentialsVolume(it.name)) } - dockerSocketVolume(setup)?.let(::add) - add(mcpConfigVolume(setup)) - } - - private fun pvcVolume( - name: String, - claim: String, - ) = VolumeBuilder() - .withName(name) - .withNewPersistentVolumeClaim() - .withClaimName(claim) - .endPersistentVolumeClaim() - .build() - - private fun dockerSocketVolume(setup: RunnerSetupProvisioningSpec): Volume? = - if (!setup.dockerSocketEnabled) { - null - } else { - VolumeBuilder() - .withName(DOCKER_SOCKET_VOLUME) - .withNewHostPath() - .withPath(setup.dockerSocketPath) - .withType("Socket") - .endHostPath() - .build() - } - - private fun agentCredentialsVolume(credentialSecret: String): Volume = - VolumeBuilder() - .withName(AGENT_CREDENTIALS_VOLUME) - .withNewSecret() - .withSecretName(credentialSecret) - .endSecret() - .build() - - private fun mcpConfigVolume(setup: RunnerSetupProvisioningSpec): Volume = - VolumeBuilder() - .withName("mcp-config") - .withNewConfigMap() - .withName(setup.mcpServersConfigMap) - .withOptional(true) - .endConfigMap() - .build() - - private fun podSupplementalGroups(setup: RunnerSetupProvisioningSpec): List = - if (setup.dockerSocketEnabled) { - (listOf(RUN_AS_GID) + setup.dockerSocketSupplementalGroups).distinct() - } else { - listOf(RUN_AS_GID) - } - - private fun service( - name: String, - podName: String, - gatewayPort: Int, - ): io.fabric8.kubernetes.api.model.Service = - ServiceBuilder() - .withNewMetadata() - .withName(name) - .withNamespace(props.namespace) - .endMetadata() - .withNewSpec() - .withSelector(mapOf("agent-runner/workspace-id" to podName.substringAfter("agent-runner-"))) - .addNewPort() - .withName("gateway") - .withPort(gatewayPort) - .withNewTargetPort("gateway") - .endPort() - .endSpec() - .build() + /** + * The release version this agents-api process is running, baked into the + * image as `SERVICE_VERSION` (= the release-please tag, e.g. "v0.12.0"). + * The whole suite is published in lockstep, so this is also the target + * agent-runner version. Null on a local/dev build where it is unset or + * "unknown", which disables upgrade detection rather than guessing. + */ + private fun ownReleaseVersion(): String? = + System.getenv("SERVICE_VERSION")?.takeIf { it.isNotBlank() && it != "unknown" } private fun legacySetupSpec(): RunnerSetupProvisioningSpec = RunnerSetupProvisioningSpec( @@ -877,45 +237,19 @@ class Fabric8AgentRunnerOrchestrator( ) companion object { - // Pod security: non-root, with fsGroup so PVC mounts get the right owner. - private const val RUN_AS_UID = 1000L - private const val RUN_AS_GID = 1000L - private const val FS_GROUP = 1000L - private const val DOCKER_SOCKET_VOLUME = "docker-socket" - private const val AGENT_CREDENTIALS_VOLUME = "agent-credentials" - private const val AGENT_CREDENTIALS_MOUNT = "/var/run/secrets/agents/credentials" - // How long scaleDown waits for the old runner Pod to fully terminate // (and release the ReadWriteOnce workspace PVC) before returning, so a // back-to-back provision recreates it on a clean slate. private const val POD_TERMINATION_TIMEOUT_SECONDS = 90L - - // Resource sizing. One Pod hosts the gateway JVM, the agent CLIs - // (Claude Code, Codex) and the workspace's own processes in a - // single memory cgroup. At a 3Gi limit a real Claude session - // alongside the JVM tripped the cgroup OOM killer (exit 137), - // taking every tmux session in the Pod down at once. The gateway - // heap is now capped small (see agent-runner entrypoint), and the - // limit is raised to 10Gi so the CLIs and build tooling have real - // headroom; the request reserves a baseline that covers the JVM - // plus an idle CLI. - private const val CPU_REQUEST = "250m" - private const val MEMORY_REQUEST = "2Gi" - private const val CPU_LIMIT = "2000m" - private const val MEMORY_LIMIT = "10Gi" - - // Probe cadence. The startup probe loops 60×5s = 5 min so the - // gateway's JVM cold start (slower on a fresh image pull) - // completes before liveness can fire; readiness shares the same - // budget as a backstop. - private const val STARTUP_PERIOD_SECONDS = 5 - private const val STARTUP_FAILURE_THRESHOLD = 60 - private const val READINESS_PERIOD_SECONDS = 5 - private const val READINESS_FAILURE_THRESHOLD = 60 - private const val LIVENESS_PERIOD_SECONDS = 10 } } +internal data class RunnerResourceNames( + val pod: String, + val pvc: String, + val service: String, +) + /** * Pure helpers for agent-runner image-version handling, extracted so the * tag parsing and pinning logic is unit-testable without a Kubernetes cluster. diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerCredentialSecretManager.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerCredentialSecretManager.kt new file mode 100644 index 0000000..819b559 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerCredentialSecretManager.kt @@ -0,0 +1,115 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.k8s + +import com.jorisjonkers.personalstack.agents.config.AgentRuntimeProperties +import com.jorisjonkers.personalstack.agents.domain.model.AgentCredentialProvider +import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialRepository +import io.fabric8.kubernetes.api.model.SecretBuilder +import io.fabric8.kubernetes.client.KubernetesClient +import org.slf4j.LoggerFactory +import org.springframework.beans.factory.ObjectProvider +import java.util.Base64 + +/** + * Manages the per-workspace credential Secret in Kubernetes. Extracted from + * Fabric8AgentRunnerOrchestrator to keep that class below the TooManyFunctions + * and LargeClass thresholds. + */ +internal class RunnerCredentialSecretManager( + private val client: KubernetesClient, + private val props: AgentRuntimeProperties, + private val credentialsProvider: ObjectProvider, +) { + private val log = LoggerFactory.getLogger(RunnerCredentialSecretManager::class.java) + + internal data class CredentialSecret( + val name: String, + val hasClaude: Boolean, + val hasClaudeCredentialsJson: Boolean, + val hasClaudeAccountJson: Boolean, + val hasCodex: Boolean, + val hasCodexConfig: Boolean, + ) + + fun credentialSecretName(short: String): String = "agent-runner-credentials-$short" + + fun ensureCredentialSecret( + workspace: Workspace, + short: String, + ): CredentialSecret? { + val name = credentialSecretName(short) + val data = + credentialSecretData(workspace) + ?: run { + client + .secrets() + .inNamespace(props.namespace) + .withName(name) + .delete() + return null + } + client + .secrets() + .inNamespace(props.namespace) + .resource( + SecretBuilder() + .withNewMetadata() + .withName(name) + .withNamespace(props.namespace) + .withLabels( + mapOf( + "app.kubernetes.io/part-of" to "agent-runner", + "agent-runner/workspace-id" to short, + ), + ).endMetadata() + .withType("Opaque") + .withData(data) + .build(), + ).serverSideApply() + return CredentialSecret( + name = name, + hasClaude = data.containsKey("claude_oauth_token"), + hasClaudeCredentialsJson = data.containsKey("claude_credentials_json"), + hasClaudeAccountJson = data.containsKey("claude_account_json"), + hasCodex = data.containsKey("codex_auth_json"), + hasCodexConfig = data.containsKey("codex_config_toml"), + ) + } + + private fun credentialSecretData(workspace: Workspace): Map? { + val owner = workspace.ownerUserId?.takeIf { it.isNotBlank() } ?: return null + val store = credentialsProvider.ifAvailable ?: return null + val data = + buildMap { + val claude = loadCredential(store, owner, AgentCredentialProvider.CLAUDE) + claude?.payload?.get("oauth_token")?.takeIf { it.isNotBlank() }?.let { + put("claude_oauth_token", b64(it)) + } + claude?.payload?.get("credentials_json")?.takeIf { it.isNotBlank() }?.let { + put("claude_credentials_json", b64(it)) + } + claude?.payload?.get("account_json")?.takeIf { it.isNotBlank() }?.let { + put("claude_account_json", b64(it)) + } + val codex = loadCredential(store, owner, AgentCredentialProvider.CODEX) + val codexAuth = codex?.payload?.get("auth_json")?.takeIf { it.isNotBlank() } + val codexConfig = codex?.payload?.get("config_toml")?.takeIf { it.isNotBlank() } + if (codexAuth != null) { + put("codex_auth_json", b64(codexAuth)) + codexConfig?.let { put("codex_config_toml", b64(it)) } + } + } + return data.takeIf { it.isNotEmpty() } + } + + private fun loadCredential( + store: AgentCredentialRepository, + owner: String, + provider: AgentCredentialProvider, + ) = runCatching { store.find(owner, provider) } + .onFailure { log.warn("could not load {} credential for workspace owner", provider) } + .getOrNull() + ?.takeUnless { it.valid == false } + + private fun b64(s: String): String = Base64.getEncoder().encodeToString(s.toByteArray()) +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt new file mode 100644 index 0000000..cf6d209 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt @@ -0,0 +1,547 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.k8s + +import com.jorisjonkers.personalstack.agents.config.AgentRuntimeProperties +import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupProvisioningSpec +import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.port.RepositoryRepository +import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepositoryRepository +import com.jorisjonkers.personalstack.agents.infrastructure.k8s.RunnerCredentialSecretManager.CredentialSecret +import io.fabric8.kubernetes.api.model.ContainerBuilder +import io.fabric8.kubernetes.api.model.ContainerPortBuilder +import io.fabric8.kubernetes.api.model.EnvVarBuilder +import io.fabric8.kubernetes.api.model.PersistentVolumeClaim +import io.fabric8.kubernetes.api.model.PersistentVolumeClaimBuilder +import io.fabric8.kubernetes.api.model.Pod +import io.fabric8.kubernetes.api.model.PodBuilder +import io.fabric8.kubernetes.api.model.Quantity +import io.fabric8.kubernetes.api.model.ServiceBuilder +import io.fabric8.kubernetes.api.model.Volume +import io.fabric8.kubernetes.api.model.VolumeBuilder +import io.fabric8.kubernetes.api.model.VolumeMountBuilder +import org.springframework.beans.factory.ObjectProvider + +/** + * Builds Kubernetes resource specs (Pod, PVC, Service) for a workspace runner. + * Extracted from Fabric8AgentRunnerOrchestrator to keep that class below the + * TooManyFunctions and LargeClass thresholds. + * + * fabric8's fluent builder chains naturally split into one helper per pod section + * (labels, env, mounts, volumes, container body); each helper operates on the + * same shared props. The @Suppress("LongMethod") on pod() is intentional — + * the intermediate fluent types are package-private and cannot be split further. + */ +internal class RunnerPodSpecBuilder( + private val props: AgentRuntimeProperties, + private val workspaceRepos: ObjectProvider, + private val repositories: ObjectProvider, + private val ownReleaseVersion: () -> String?, +) { + fun pvc(name: String): PersistentVolumeClaim = + PersistentVolumeClaimBuilder() + .withNewMetadata() + .withName(name) + .withNamespace(props.namespace) + .endMetadata() + .withNewSpec() + .withAccessModes("ReadWriteOnce") + .withStorageClassName(props.workspaceStorageClass) + .withNewResources() + .withRequests(mapOf("storage" to Quantity(props.workspaceStorageSize))) + .endResources() + .endSpec() + .build() + + // fabric8 builder chains can't be cleanly split into helpers + // because the intermediate fluent types are private; LongMethod + // is the natural shape here and suppressed with intent. The + // inline `mapOf(...)` calls are likewise intentional: extracting + // them into typed vals trips Kotlin overload resolution on + // fabric8 7.x's `withLabels` / `withRequests` / `withLimits`. + @Suppress("LongMethod") + fun pod( + workspace: Workspace, + setup: RunnerSetupProvisioningSpec, + runnerGeneration: Long, + names: RunnerResourceNames, + credentialSecret: CredentialSecret?, + ): Pod = + PodBuilder() + .withNewMetadata() + .withName(names.pod) + .withNamespace(props.namespace) + .withLabels(podLabels(workspace, setup, runnerGeneration)) + .withAnnotations(podAnnotations(setup)) + .endMetadata() + .withNewSpec() + .withServiceAccountName(setup.serviceAccount) + // The runner never calls the Kubernetes API itself — cluster + // reads go through the read-only kubernetes MCP server over + // HTTP. Don't project the SA token into the Pod, so the agent + // (which runs unsandboxed) holds no API credential at all and + // cannot reach the API server even if the SA were later granted + // RBAC by mistake. + .withAutomountServiceAccountToken(false) + .withNodeSelector(setup.nodeSelector) + .withRestartPolicy("Always") + .withNewSecurityContext() + .withRunAsUser(RUN_AS_UID) + .withRunAsGroup(RUN_AS_GID) + .withFsGroup(FS_GROUP) + .withSupplementalGroups(podSupplementalGroups(setup)) + .endSecurityContext() + .withInitContainers(agentStateInitContainer(setup)) + .addNewContainer() + .withName("agent-runner") + // Pin to the release agents-api itself is on so the running + // version is a verifiable fact in the Pod spec, not :latest. + .withImage(RunnerImageVersions.pin(setup.image, ownReleaseVersion())) + .withImagePullPolicy(setup.imagePullPolicy) + .withPorts( + ContainerPortBuilder() + .withName("gateway") + .withContainerPort(setup.gatewayPort) + .build(), + ).withEnv(podEnv(workspace, setup, runnerGeneration, credentialSecret)) + .withVolumeMounts(podVolumeMounts(setup, credentialSecret)) + // Startup probe gates liveness + readiness until the gateway's + // JVM has finished its cold start. Without it the liveness probe + // (failureThreshold 3 x 10s ~= 30s, no initial delay) killed the + // booting Spring Boot gateway before it bound :8090, which + // re-provisioned the runner in a loop and 503'd every + // start-session. 60 x 5s = 5 min of boot headroom. + .withNewStartupProbe() + .withNewHttpGet() + .withPath("/healthz") + .withNewPort("gateway") + .endHttpGet() + .withPeriodSeconds(STARTUP_PERIOD_SECONDS) + .withFailureThreshold(STARTUP_FAILURE_THRESHOLD) + .endStartupProbe() + .withNewReadinessProbe() + .withNewHttpGet() + .withPath("/healthz") + .withNewPort("gateway") + .endHttpGet() + .withPeriodSeconds(READINESS_PERIOD_SECONDS) + .withFailureThreshold(READINESS_FAILURE_THRESHOLD) + .endReadinessProbe() + .withNewLivenessProbe() + .withNewHttpGet() + .withPath("/healthz") + .withNewPort("gateway") + .endHttpGet() + .withPeriodSeconds(LIVENESS_PERIOD_SECONDS) + .endLivenessProbe() + .withNewResources() + .withRequests(mapOf("cpu" to Quantity(CPU_REQUEST), "memory" to Quantity(MEMORY_REQUEST))) + .withLimits(mapOf("cpu" to Quantity(CPU_LIMIT), "memory" to Quantity(MEMORY_LIMIT))) + .endResources() + .endContainer() + .withVolumes(podVolumes(names.pvc, credentialSecret, setup)) + .endSpec() + .build() + + fun service( + name: String, + podName: String, + gatewayPort: Int, + ): io.fabric8.kubernetes.api.model.Service = + ServiceBuilder() + .withNewMetadata() + .withName(name) + .withNamespace(props.namespace) + .endMetadata() + .withNewSpec() + .withSelector(mapOf("agent-runner/workspace-id" to podName.substringAfter("agent-runner-"))) + .addNewPort() + .withName("gateway") + .withPort(gatewayPort) + .withNewTargetPort("gateway") + .endPort() + .endSpec() + .build() + + private fun podLabels( + workspace: Workspace, + setup: RunnerSetupProvisioningSpec, + runnerGeneration: Long, + ): Map = + mapOf( + "app.kubernetes.io/name" to "agent-runner", + "app.kubernetes.io/part-of" to "agent-runner", + RunnerState.LABEL_WORKSPACE_ID to workspace.id.short(), + RunnerState.LABEL_SETUP_ID to setup.setupId.value, + RunnerState.LABEL_SETUP_VERSION to setup.setupVersion.value.toString(), + RunnerState.LABEL_RUNNER_GENERATION to runnerGeneration.toString(), + ) + + private fun podAnnotations(setup: RunnerSetupProvisioningSpec): Map = + mapOf(RunnerState.ANNOTATION_SETUP_HASH to setup.setupHash) + + private fun podEnv( + workspace: Workspace, + setup: RunnerSetupProvisioningSpec, + runnerGeneration: Long, + credentialSecret: CredentialSecret?, + ) = buildList { + add(EnvVarBuilder().withName("HOME").withValue("/home/agent").build()) + add(EnvVarBuilder().withName("CODEX_HOME").withValue("/home/agent/.codex").build()) + add(EnvVarBuilder().withName("DEPLOYMENT_ENVIRONMENT").withValue("production").build()) + add(EnvVarBuilder().withName("OTEL_SERVICE_NAME").withValue("agent-gateway").build()) + add( + EnvVarBuilder() + .withName("OTEL_EXPORTER_OTLP_ENDPOINT") + .withValue("http://alloy.observability.svc.cluster.local:4318") + .build(), + ) + add(EnvVarBuilder().withName("OTEL_EXPORTER_OTLP_PROTOCOL").withValue("http/protobuf").build()) + // The runner Pod is the outer sandbox for the agent process. + // Docker socket access is the explicit host-equivalent exception + // for Testcontainers and Docker CLI workflows. IS_SANDBOX tells + // Claude Code so that --dangerously-skip-permissions runs without + // the bypass-mode warning + acceptance prompt. + add(EnvVarBuilder().withName("IS_SANDBOX").withValue("1").build()) + add(EnvVarBuilder().withName("AGENT_MCP_PROFILE").withValue(setup.mcpProfile).build()) + add(EnvVarBuilder().withName("AGENT_MCP_DIR").withValue(setup.mcpDir).build()) + setup.claudeMcpServersFile?.let { + add(EnvVarBuilder().withName("AGENT_MCP_SERVERS_FILE").withValue(it).build()) + } + setup.codexMcpServersFile?.let { + add(EnvVarBuilder().withName("AGENT_CODEX_MCP_FILE").withValue(it).build()) + } + setup.githubMcpToolsets?.let { + add(EnvVarBuilder().withName("GITHUB_MCP_TOOLSETS").withValue(it).build()) + } + setup.githubMcpExcludeTools?.let { + add(EnvVarBuilder().withName("GITHUB_MCP_EXCLUDE_TOOLS").withValue(it).build()) + } + add(EnvVarBuilder().withName("AGENT_GATEWAY_RUNNER_SETUP_ID").withValue(setup.setupId.value).build()) + add( + EnvVarBuilder() + .withName("AGENT_GATEWAY_RUNNER_SETUP_VERSION") + .withValue(setup.setupVersion.value.toString()) + .build(), + ) + add(EnvVarBuilder().withName("AGENT_GATEWAY_RUNNER_SETUP_HASH").withValue(setup.setupHash).build()) + add( + EnvVarBuilder() + .withName("AGENT_GATEWAY_RUNNER_GENERATION") + .withValue(runnerGeneration.toString()) + .build(), + ) + addAll(dockerEnv(setup)) + addAll(knowledgeEnv(setup)) + addAll(githubAppTokenEnv()) + addAll(agentCredentialEnv(credentialSecret)) + // REPO_URL/REPO_BRANCH drive the entrypoint's boot-time clone + // into /workspace/. Cloning in the runner removes the race that + // left repo-backed workspaces empty: the old create-time + // gateway.clone fired before the runner gateway was up and was + // swallowed. Only repo-backed workspaces carry a repoUrl. + workspace.repoUrl?.let { url -> + add(EnvVarBuilder().withName("REPO_URL").withValue(url).build()) + workspace.branch?.let { add(EnvVarBuilder().withName("REPO_BRANCH").withValue(it).build()) } + } + // REPO_URLS carries the workspace's additional repos (everything + // attached that is not the primary), as url#branch entries. The + // entrypoint clones each into /workspace/ over the + // App-token credential helper. + additionalRepoUrls(workspace).takeIf { it.isNotEmpty() }?.let { urls -> + add(EnvVarBuilder().withName("REPO_URLS").withValue(urls.joinToString(" ")).build()) + } + } + + private fun additionalRepoUrls(workspace: Workspace): List { + val links = workspaceRepos.ifAvailable ?: return emptyList() + val repos = repositories.ifAvailable ?: return emptyList() + return links + .findAllByWorkspaceId(workspace.id) + .filterNot { it.isPrimary } + .mapNotNull { link -> + repos.findById(link.repositoryId)?.let { repo -> "${repo.repoUrl}#${repo.defaultBranch}" } + } + } + + private fun dockerEnv(setup: RunnerSetupProvisioningSpec) = + if (!setup.dockerSocketEnabled) { + emptyList() + } else { + listOf( + EnvVarBuilder().withName("DOCKER_HOST").withValue("unix://${setup.dockerSocketPath}").build(), + EnvVarBuilder() + .withName("TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE") + .withValue(setup.dockerSocketPath) + .build(), + EnvVarBuilder() + .withName("AGENT_RUNNER_NODE_HOST_IP") + .withNewValueFrom() + .withNewFieldRef() + .withFieldPath("status.hostIP") + .endFieldRef() + .endValueFrom() + .build(), + EnvVarBuilder() + .withName("TESTCONTAINERS_HOST_OVERRIDE") + .withValue("$(AGENT_RUNNER_NODE_HOST_IP)") + .build(), + ) + } + + // KB_URL + KB_BEARER_TOKEN are the exact names the knowledge-system + // install.sh hooks read; without the bearer every hook short-circuits + // to a no-op and the knowledge.* MCP tools are unreachable. + private fun knowledgeEnv(setup: RunnerSetupProvisioningSpec) = + listOf( + EnvVarBuilder().withName("KB_URL").withValue(setup.knowledgeBaseUrl).build(), + EnvVarBuilder() + .withName("KB_BEARER_TOKEN") + .withNewValueFrom() + .withNewSecretKeyRef() + .withName(setup.knowledgeBearerSecret) + .withKey(setup.knowledgeBearerSecretKey) + .endSecretKeyRef() + .endValueFrom() + .build(), + ) + + // The bearer is an optional Secret ref: an absent github-app Secret + // keeps the Pod starting and the `gh` wrapper degrades to a no-op. + private fun githubAppTokenEnv() = + listOf( + EnvVarBuilder().withName("GITHUB_APP_TOKEN_URL").withValue(props.githubAppTokenUrl).build(), + EnvVarBuilder() + .withName("GITHUB_APP_TOKEN_BEARER") + .withNewValueFrom() + .withNewSecretKeyRef() + .withName(props.githubAppBearerSecret) + .withKey(props.githubAppBearerSecretKey) + .withOptional(true) + .endSecretKeyRef() + .endValueFrom() + .build(), + ) + + private fun agentCredentialEnv(credentialSecret: CredentialSecret?) = + if (credentialSecret == null) { + emptyList() + } else { + buildList { + if (credentialSecret.hasClaudeCredentialsJson) { + add( + EnvVarBuilder() + .withName("AGENT_CLAUDE_CREDENTIALS_FILE") + .withValue("$AGENT_CREDENTIALS_MOUNT/claude_credentials_json") + .build(), + ) + } + if (credentialSecret.hasClaudeAccountJson) { + add( + EnvVarBuilder() + .withName("AGENT_CLAUDE_ACCOUNT_FILE") + .withValue("$AGENT_CREDENTIALS_MOUNT/claude_account_json") + .build(), + ) + } + if (credentialSecret.hasClaude && !credentialSecret.hasClaudeCredentialsJson) { + add( + EnvVarBuilder() + .withName("CLAUDE_CODE_OAUTH_TOKEN") + .withNewValueFrom() + .withNewSecretKeyRef() + .withName(credentialSecret.name) + .withKey("claude_oauth_token") + .endSecretKeyRef() + .endValueFrom() + .build(), + ) + } + if (credentialSecret.hasCodex) { + add( + EnvVarBuilder() + .withName("AGENT_CODEX_AUTH_JSON_FILE") + .withValue("$AGENT_CREDENTIALS_MOUNT/codex_auth_json") + .build(), + ) + } + if (credentialSecret.hasCodexConfig) { + add( + EnvVarBuilder() + .withName("AGENT_CODEX_CONFIG_TOML_FILE") + .withValue("$AGENT_CREDENTIALS_MOUNT/codex_config_toml") + .build(), + ) + } + } + } + + private fun podVolumeMounts( + setup: RunnerSetupProvisioningSpec, + credentialSecret: CredentialSecret?, + ) = buildList { + add(VolumeMountBuilder().withName("workspace").withMountPath("/workspace").build()) + addAll(agentStateVolumeMounts()) + if (credentialSecret != null) { + add( + VolumeMountBuilder() + .withName(AGENT_CREDENTIALS_VOLUME) + .withMountPath(AGENT_CREDENTIALS_MOUNT) + .withReadOnly(true) + .build(), + ) + } + if (setup.dockerSocketEnabled) { + add( + VolumeMountBuilder() + .withName(DOCKER_SOCKET_VOLUME) + .withMountPath(setup.dockerSocketPath) + .build(), + ) + } + // Declarative MCP server set; the entrypoint seeds it into + // ~/.claude.json. Optional volume, so an absent ConfigMap + // leaves the runner with no managed MCP servers. + add( + VolumeMountBuilder() + .withName("mcp-config") + .withMountPath(setup.mcpDir) + .withReadOnly(true) + .build(), + ) + } + + private fun agentStateInitContainer(setup: RunnerSetupProvisioningSpec) = + ContainerBuilder() + .withName("agent-state-init") + .withImage(RunnerImageVersions.pin(setup.image, ownReleaseVersion())) + .withImagePullPolicy(setup.imagePullPolicy) + .withCommand("/bin/sh", "-c") + .withArgs( + "mkdir -p /workspace/.agent-state/claude/projects " + + "/workspace/.agent-state/claude/backups " + + "/workspace/.agent-state/claude/todos " + + "/workspace/.agent-state/claude/shell-snapshots " + + "/workspace/.agent-state/codex/session-homes && " + + "chown -R 1000:1000 /workspace/.agent-state", + ).withVolumeMounts(VolumeMountBuilder().withName("workspace").withMountPath("/workspace").build()) + .withNewSecurityContext() + .withRunAsUser(0L) + .withRunAsGroup(0L) + .endSecurityContext() + .build() + + private fun agentStateVolumeMounts() = + listOf( + agentStateVolumeMount( + mountPath = "/home/agent/.claude/projects", + subPath = ".agent-state/claude/projects", + ), + agentStateVolumeMount( + mountPath = "/home/agent/.claude/backups", + subPath = ".agent-state/claude/backups", + ), + agentStateVolumeMount( + mountPath = "/home/agent/.claude/todos", + subPath = ".agent-state/claude/todos", + ), + agentStateVolumeMount( + mountPath = "/home/agent/.claude/shell-snapshots", + subPath = ".agent-state/claude/shell-snapshots", + ), + agentStateVolumeMount( + mountPath = "/home/agent/.codex/session-homes", + subPath = ".agent-state/codex/session-homes", + ), + ) + + private fun agentStateVolumeMount( + mountPath: String, + subPath: String, + ) = VolumeMountBuilder() + .withName("workspace") + .withMountPath(mountPath) + .withSubPath(subPath) + .build() + + private fun podVolumes( + workspacePvc: String, + credentialSecret: CredentialSecret?, + setup: RunnerSetupProvisioningSpec, + ) = buildList { + add(pvcVolume("workspace", workspacePvc)) + credentialSecret?.let { add(agentCredentialsVolume(it.name)) } + dockerSocketVolume(setup)?.let(::add) + add(mcpConfigVolume(setup)) + } + + private fun pvcVolume( + name: String, + claim: String, + ) = VolumeBuilder() + .withName(name) + .withNewPersistentVolumeClaim() + .withClaimName(claim) + .endPersistentVolumeClaim() + .build() + + private fun dockerSocketVolume(setup: RunnerSetupProvisioningSpec): Volume? = + if (!setup.dockerSocketEnabled) { + null + } else { + VolumeBuilder() + .withName(DOCKER_SOCKET_VOLUME) + .withNewHostPath() + .withPath(setup.dockerSocketPath) + .withType("Socket") + .endHostPath() + .build() + } + + private fun agentCredentialsVolume(credentialSecret: String): Volume = + VolumeBuilder() + .withName(AGENT_CREDENTIALS_VOLUME) + .withNewSecret() + .withSecretName(credentialSecret) + .endSecret() + .build() + + private fun mcpConfigVolume(setup: RunnerSetupProvisioningSpec): Volume = + VolumeBuilder() + .withName("mcp-config") + .withNewConfigMap() + .withName(setup.mcpServersConfigMap) + .withOptional(true) + .endConfigMap() + .build() + + private fun podSupplementalGroups(setup: RunnerSetupProvisioningSpec): List = + if (setup.dockerSocketEnabled) { + (listOf(RUN_AS_GID) + setup.dockerSocketSupplementalGroups).distinct() + } else { + listOf(RUN_AS_GID) + } + + private companion object { + const val RUN_AS_UID = 1000L + const val RUN_AS_GID = 1000L + const val FS_GROUP = 1000L + const val DOCKER_SOCKET_VOLUME = "docker-socket" + const val AGENT_CREDENTIALS_VOLUME = "agent-credentials" + const val AGENT_CREDENTIALS_MOUNT = "/var/run/secrets/agents/credentials" + + // Probe cadence. The startup probe loops 60×5s = 5 min so the + // gateway's JVM cold start (slower on a fresh image pull) + // completes before liveness can fire; readiness shares the same + // budget as a backstop. + const val STARTUP_PERIOD_SECONDS = 5 + const val STARTUP_FAILURE_THRESHOLD = 60 + const val READINESS_PERIOD_SECONDS = 5 + const val READINESS_FAILURE_THRESHOLD = 60 + const val LIVENESS_PERIOD_SECONDS = 10 + + // Resource sizing. + const val CPU_REQUEST = "250m" + const val MEMORY_REQUEST = "2Gi" + const val CPU_LIMIT = "2000m" + const val MEMORY_LIMIT = "10Gi" + } +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReader.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReader.kt new file mode 100644 index 0000000..8cfce5f --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReader.kt @@ -0,0 +1,47 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.k8s + +import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupId +import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupVersion +import com.jorisjonkers.personalstack.agents.domain.model.RunnerState +import io.fabric8.kubernetes.api.model.Pod + +/** + * Reads runner Pod state and maps it to the domain RunnerState model. + * Extracted from Fabric8AgentRunnerOrchestrator to keep that class below + * the TooManyFunctions and LargeClass thresholds. + */ +internal class RunnerStateReader( + private val ownReleaseVersion: () -> String?, +) { + fun runnerState(pod: Pod): RunnerState { + val containerReady = + pod.status + ?.containerStatuses + ?.firstOrNull() + ?.ready ?: false + val labels = pod.metadata?.labels.orEmpty() + val annotations = pod.metadata?.annotations.orEmpty() + return RunnerState( + podName = pod.metadata?.name.orEmpty(), + workspaceId = labels[RunnerState.LABEL_WORKSPACE_ID], + setupId = labels[RunnerState.LABEL_SETUP_ID]?.let(::parseSetupId), + setupVersion = labels[RunnerState.LABEL_SETUP_VERSION]?.let(::parseSetupVersion), + setupHash = annotations[RunnerState.ANNOTATION_SETUP_HASH], + runnerGeneration = labels[RunnerState.LABEL_RUNNER_GENERATION]?.toLongOrNull(), + phase = pod.status?.phase, + containerReady = containerReady, + runnerImageVersion = + RunnerImageVersions.tagOf( + pod.spec + ?.containers + ?.firstOrNull() + ?.image, + ), + ) + } + + private fun parseSetupId(value: String): AgentSetupId? = runCatching { AgentSetupId(value) }.getOrNull() + + private fun parseSetupVersion(value: String): AgentSetupVersion? = + value.toLongOrNull()?.let { runCatching { AgentSetupVersion(it) }.getOrNull() } +} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/AgentRunnerObservabilityContractTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/AgentRunnerObservabilityContractTest.kt index 62c8074..3123f92 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/AgentRunnerObservabilityContractTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/AgentRunnerObservabilityContractTest.kt @@ -9,13 +9,14 @@ import java.nio.file.Path class AgentRunnerObservabilityContractTest { @Test fun `runner pod env pins gateway service identity and otlp transport`() { - val orchestrator = + // Pod env is built in RunnerPodSpecBuilder after the class split. + val podSpecBuilder = readProjectFile( "src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/" + - "Fabric8AgentRunnerOrchestrator.kt", + "RunnerPodSpecBuilder.kt", ) - assertThat(orchestrator).contains( + assertThat(podSpecBuilder).contains( "EnvVarBuilder().withName(\"OTEL_SERVICE_NAME\").withValue(\"agent-gateway\")", "EnvVarBuilder()", ".withName(\"OTEL_EXPORTER_OTLP_ENDPOINT\")", @@ -26,18 +27,19 @@ class AgentRunnerObservabilityContractTest { @Test fun `runner kubernetes probes remain on healthz`() { - val orchestrator = + // Probe configuration lives in RunnerPodSpecBuilder after the class split. + val podSpecBuilder = readProjectFile( "src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/" + - "Fabric8AgentRunnerOrchestrator.kt", + "RunnerPodSpecBuilder.kt", ) - assertThat(orchestrator).contains( + assertThat(podSpecBuilder).contains( ".withNewStartupProbe()", ".withNewReadinessProbe()", ".withNewLivenessProbe()", ) - assertThat(Regex("""\.withPath\("/healthz"\)""").findAll(orchestrator).count()).isEqualTo(3) + assertThat(Regex("""\.withPath\("/healthz"\)""").findAll(podSpecBuilder).count()).isEqualTo(3) } @Test diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt new file mode 100644 index 0000000..d757f1b --- /dev/null +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt @@ -0,0 +1,113 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.k8s + +import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupId +import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupVersion +import com.jorisjonkers.personalstack.agents.domain.model.RunnerState +import io.fabric8.kubernetes.api.model.ContainerStatusBuilder +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder +import io.fabric8.kubernetes.api.model.PodBuilder +import io.fabric8.kubernetes.api.model.PodSpec +import io.fabric8.kubernetes.api.model.PodSpecBuilder +import io.fabric8.kubernetes.api.model.PodStatus +import io.fabric8.kubernetes.api.model.ContainerBuilder +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test + +class RunnerStateReaderTest { + private val reader = RunnerStateReader { "v0.18.0" } + + @Test + fun `runnerState maps ready pod with all labels to domain model`() { + val pod = + PodBuilder() + .withMetadata( + ObjectMetaBuilder() + .withName("agent-runner-abc123") + .withLabels( + mapOf( + RunnerState.LABEL_WORKSPACE_ID to "abc123", + RunnerState.LABEL_SETUP_ID to "default", + RunnerState.LABEL_SETUP_VERSION to "1", + RunnerState.LABEL_RUNNER_GENERATION to "3", + ), + ).withAnnotations( + mapOf(RunnerState.ANNOTATION_SETUP_HASH to "sha-abc"), + ).build(), + ).withSpec( + PodSpecBuilder() + .addNewContainer() + .withName("agent-runner") + .withImage("ghcr.io/example/agent-runner:v0.18.0") + .endContainer() + .build(), + ).withStatus( + PodStatus().apply { + phase = "Running" + containerStatuses = + listOf( + ContainerStatusBuilder().withReady(true).build(), + ) + }, + ).build() + + val state = reader.runnerState(pod) + + assertThat(state.podName).isEqualTo("agent-runner-abc123") + assertThat(state.workspaceId).isEqualTo("abc123") + assertThat(state.setupId).isEqualTo(AgentSetupId("default")) + assertThat(state.setupVersion).isEqualTo(AgentSetupVersion(1)) + assertThat(state.setupHash).isEqualTo("sha-abc") + assertThat(state.runnerGeneration).isEqualTo(3L) + assertThat(state.phase).isEqualTo("Running") + assertThat(state.containerReady).isTrue() + assertThat(state.runnerImageVersion).isEqualTo("v0.18.0") + } + + @Test + fun `runnerState maps pod with missing container status as not ready`() { + val pod = + PodBuilder() + .withMetadata(ObjectMetaBuilder().withName("agent-runner-def456").build()) + .withStatus(PodStatus().apply { phase = "Pending" }) + .build() + + val state = reader.runnerState(pod) + + assertThat(state.containerReady).isFalse() + assertThat(state.phase).isEqualTo("Pending") + assertThat(state.runnerImageVersion).isNull() + } + + @Test + fun `runnerState returns null setupId and setupVersion when labels are missing`() { + val pod = + PodBuilder() + .withMetadata(ObjectMetaBuilder().withName("agent-runner-no-labels").build()) + .withStatus(PodStatus()) + .build() + + val state = reader.runnerState(pod) + + assertThat(state.setupId).isNull() + assertThat(state.setupVersion).isNull() + assertThat(state.runnerGeneration).isNull() + } + + @Test + fun `runnerState returns null setupVersion when label is not a valid long`() { + val pod = + PodBuilder() + .withMetadata( + ObjectMetaBuilder() + .withName("agent-runner-bad-labels") + .withLabels( + mapOf(RunnerState.LABEL_SETUP_VERSION to "not-a-number"), + ).build(), + ).withStatus(PodStatus()) + .build() + + val state = reader.runnerState(pod) + + assertThat(state.setupVersion).isNull() + } +} From 0242aa9148d146d4e48b66b1e53fb9333f67a1b2 Mon Sep 17 00:00:00 2001 From: JorisJonkers Agent Date: Mon, 13 Jul 2026 07:37:31 +0000 Subject: [PATCH 04/10] refactor(SessionAttachHandler): extract AttachPreconditionChecker to remove TooManyFunctions/LargeClass Move session lookup, workspace checks, rebind attempt, and attach recording to AttachPreconditionChecker. SessionAttachHandler drops from 25 outer-class functions to 13, removing @file:Suppress("TooManyFunctions","LargeClass"). outcomeOf/failureReasonOf moved to UpstreamHandler.companion. Add AttachPreconditionCheckerTest. --- .../ws/AttachPreconditionChecker.kt | 202 ++++++++++++ .../infrastructure/ws/SessionAttachHandler.kt | 311 ++++-------------- .../ws/AttachPreconditionCheckerTest.kt | 210 ++++++++++++ 3 files changed, 467 insertions(+), 256 deletions(-) create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt create mode 100644 api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt new file mode 100644 index 0000000..33abdf3 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt @@ -0,0 +1,202 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.ws + +import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.AgentKindLabel +import com.jorisjonkers.personalstack.agents.application.observability.AttachAttemptTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel +import com.jorisjonkers.personalstack.agents.application.observability.ModeLabel +import com.jorisjonkers.personalstack.agents.application.observability.OperationLabel +import com.jorisjonkers.personalstack.agents.application.observability.OperationTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.OutcomeLabel +import com.jorisjonkers.personalstack.agents.application.observability.RunModeLabel +import com.jorisjonkers.personalstack.agents.application.sessionbinding.EnsureRunnerSessionBoundInput +import com.jorisjonkers.personalstack.agents.application.sessionbinding.RunnerSessionBindingResult +import com.jorisjonkers.personalstack.agents.application.sessionbinding.RunnerSessionBindingService +import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupOperation +import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionStatus +import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceAgentSessionRepository +import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepository +import org.springframework.web.socket.CloseStatus +import org.springframework.web.socket.WebSocketSession +import java.time.Duration +import java.util.UUID + +/** + * Checks all preconditions required before opening a WebSocket bridge to the + * agent-gateway. Extracted from SessionAttachHandler to keep that class below + * the TooManyFunctions and LargeClass thresholds. + */ +internal class AttachPreconditionChecker( + private val sessions: WorkspaceAgentSessionRepository, + private val workspaces: WorkspaceRepository, + private val binding: RunnerSessionBindingService, + private val telemetry: AgentsApiTelemetry, +) { + /** Outcome of the full attach precondition check. */ + sealed interface AttachOutcome { + /** All preconditions passed; the attach can proceed. */ + data class Ready( + val sessionId: WorkspaceAgentSessionId, + val workspace: Workspace, + val gatewayAgentId: String, + val gatewayEndpoint: String, + val kind: AgentKindLabel, + ) : AttachOutcome + + /** A precondition failed; close the client with this status. */ + data class Rejected( + val reason: String, + val status: CloseStatus, + val failureReason: FailureReasonLabel, + val kind: AgentKindLabel = AgentKindLabel.OTHER, + ) : AttachOutcome + } + + /** + * Runs all attach preconditions (sessionId, session lookup, + * rebind path, status, workspace, setup guards, gateway binding) + * and returns either Ready (all pass) or Rejected (first failure). + * Records telemetry and closes the client socket on rejection. + * Returns null when the client was already rejected and closed. + */ + fun resolveAttach( + clientSession: WebSocketSession, + sessionIdOf: (WebSocketSession) -> WorkspaceAgentSessionId?, + ): AttachOutcome.Ready? { + val outcome = checkPreconditions(clientSession, sessionIdOf) + if (outcome is AttachOutcome.Rejected) { + recordAttach(outcome.kind, OutcomeLabel.FAILURE, outcome.failureReason) + clientSession.close(outcome.status.withReason(outcome.reason)) + return null + } + return outcome as AttachOutcome.Ready + } + + private fun checkPreconditions( + clientSession: WebSocketSession, + sessionIdOf: (WebSocketSession) -> WorkspaceAgentSessionId?, + ): AttachOutcome { + val sessionId = sessionIdOf(clientSession) + val agentSession = sessionId?.let { sessions.findById(it) } + if (sessionId == null || agentSession == null) { + return if (sessionId == null) { + AttachOutcome.Rejected("malformed sessionId", CloseStatus.BAD_DATA, FailureReasonLabel.INVALID_REQUEST) + } else { + AttachOutcome.Rejected("unknown session", CloseStatus.BAD_DATA, FailureReasonLabel.NOT_FOUND) + } + } + val kind = AgentKindLabel.fromRaw(agentSession.kind.name) + var resolvedSession = agentSession + var reboundWorkspace: Workspace? = null + if (agentSession.status == WorkspaceAgentSessionStatus.RUNNING && agentSession.gatewayAgentId == null) { + when (val result = binding.ensureBound(EnsureRunnerSessionBoundInput(sessionId = sessionId))) { + is RunnerSessionBindingResult.Bound -> { + resolvedSession = result.session + reboundWorkspace = result.workspace + } + is RunnerSessionBindingResult.Conflict -> + return AttachOutcome.Rejected( + "session binding changed", + CloseStatus.SERVICE_RESTARTED, + FailureReasonLabel.OTHER, + kind, + ) + is RunnerSessionBindingResult.Unavailable -> + return AttachOutcome.Rejected( + "runner provisioning", + CloseStatus.SERVICE_RESTARTED, + FailureReasonLabel.UPSTREAM_UNAVAILABLE, + kind, + ) + } + } + if (resolvedSession.status == WorkspaceAgentSessionStatus.STARTING) { + return AttachOutcome.Rejected( + "runner provisioning", + CloseStatus.SERVICE_RESTARTED, + FailureReasonLabel.UPSTREAM_UNAVAILABLE, + kind, + ) + } + val workspace = + reboundWorkspace ?: workspaces.findById(resolvedSession.workspaceId) + ?: return AttachOutcome.Rejected( + "workspace gone", + CloseStatus.SERVER_ERROR, + FailureReasonLabel.NOT_FOUND, + kind, + ) + if (isSetupTransitionInProgress(resolvedSession, workspace)) { + return AttachOutcome.Rejected( + "runner setup transition", + CloseStatus.SERVICE_RESTARTED, + FailureReasonLabel.UPSTREAM_UNAVAILABLE, + kind, + ) + } + val gatewayAgentId = + resolvedSession.gatewayAgentId + ?: return AttachOutcome.Rejected( + "session not bound to a gateway agent", + CloseStatus.SERVER_ERROR, + FailureReasonLabel.UPSTREAM_UNAVAILABLE, + kind, + ) + return workspace.gatewayEndpoint?.let { endpoint -> + AttachOutcome.Ready(sessionId, workspace, gatewayAgentId, endpoint, kind) + } ?: AttachOutcome.Rejected( + "workspace has no gateway endpoint", + CloseStatus.SERVER_ERROR, + FailureReasonLabel.UPSTREAM_UNAVAILABLE, + kind, + ) + } + + private fun isSetupTransitionInProgress( + agentSession: com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession, + workspace: Workspace, + ): Boolean = + agentSession.pendingSetupId != null || + agentSession.pendingSetupVersion != null || + workspace.runnerSetupOperation != RunnerSetupOperation.IDLE || + workspace.pendingRunnerSetupId != null || + workspace.pendingRunnerSetupVersion != null || + workspace.currentRunnerSetupId != agentSession.currentSetupId || + workspace.currentRunnerSetupVersion != agentSession.currentSetupVersion + + fun recordAttach( + kind: AgentKindLabel, + outcome: OutcomeLabel, + reason: FailureReasonLabel, + ) { + val event = + AttachAttemptTelemetry( + kind = kind, + runMode = RunModeLabel.INTERACTIVE, + outcome = outcome, + reason = reason, + ) + telemetry.recordAttachAttempt(event) + if (outcome == OutcomeLabel.FAILURE) telemetry.recordAttachFailure(event) + telemetry.recordOperation( + OperationTelemetry( + operation = OperationLabel.ATTACH_SESSION, + mode = ModeLabel.INTERACTIVE, + outcome = outcome, + reason = reason, + duration = Duration.ZERO, + ), + ) + } + + companion object { + fun sessionIdOf(session: WebSocketSession): WorkspaceAgentSessionId? { + val match = + Regex("/api/v1/ws/sessions/([^/]+)/attach").find(session.uri?.path ?: return null) + ?: return null + return runCatching { WorkspaceAgentSessionId(UUID.fromString(match.groupValues[1])) }.getOrNull() + } + } +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/SessionAttachHandler.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/SessionAttachHandler.kt index 46cabbb..b589371 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/SessionAttachHandler.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/SessionAttachHandler.kt @@ -1,26 +1,18 @@ -@file:Suppress("TooManyFunctions", "LargeClass") - package com.jorisjonkers.personalstack.agents.infrastructure.ws import com.jorisjonkers.personalstack.agents.application.idle.ConnectedClientTracker import com.jorisjonkers.personalstack.agents.application.idle.WorkspaceActivityTracker import com.jorisjonkers.personalstack.agents.application.observability.AgentKindLabel import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry -import com.jorisjonkers.personalstack.agents.application.observability.AttachAttemptTelemetry import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel import com.jorisjonkers.personalstack.agents.application.observability.ModeLabel import com.jorisjonkers.personalstack.agents.application.observability.OperationLabel import com.jorisjonkers.personalstack.agents.application.observability.OperationTelemetry import com.jorisjonkers.personalstack.agents.application.observability.OutcomeLabel -import com.jorisjonkers.personalstack.agents.application.observability.RunModeLabel import com.jorisjonkers.personalstack.agents.application.sessionbinding.EnsureRunnerSessionBoundInput -import com.jorisjonkers.personalstack.agents.application.sessionbinding.RunnerSessionBindingResult import com.jorisjonkers.personalstack.agents.application.sessionbinding.RunnerSessionBindingService import com.jorisjonkers.personalstack.agents.application.sessionstatus.SessionStatusPublisher -import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupOperation -import com.jorisjonkers.personalstack.agents.domain.model.Workspace import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId -import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionStatus import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceId import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceAgentSessionRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepository @@ -41,7 +33,6 @@ import java.net.URLDecoder import java.nio.charset.StandardCharsets import java.time.Duration import java.time.Instant -import java.util.UUID import java.util.concurrent.ConcurrentHashMap import java.util.concurrent.TimeUnit @@ -61,6 +52,9 @@ import java.util.concurrent.TimeUnit * already shows it and the terminal holds screen state, so buffering * raw PTY bytes (including ANSI escapes) into the transcript only * duplicated the display and wrote escape garbage into the DB. + * + * Precondition checking is delegated to AttachPreconditionChecker to keep this + * class below the TooManyFunctions and LargeClass thresholds. */ @Component class SessionAttachDependencies( @@ -78,12 +72,18 @@ class SessionAttachHandler( private val telemetry: AgentsApiTelemetry = AgentsApiTelemetry.NOOP, ) : AbstractWebSocketHandler() { private val sessions = dependencies.sessions - private val workspaces = dependencies.workspaces private val activity = dependencies.activity private val connected = dependencies.connected private val binding = dependencies.binding private val sessionStatus = dependencies.sessionStatus private val log = LoggerFactory.getLogger(SessionAttachHandler::class.java) + private val preconditions = + AttachPreconditionChecker( + sessions = dependencies.sessions, + workspaces = dependencies.workspaces, + binding = dependencies.binding, + telemetry = telemetry, + ) private data class Bridge( val sessionId: WorkspaceAgentSessionId, @@ -107,199 +107,18 @@ class SessionAttachHandler( }, ) - private data class ResolvedAttach( - val sessionId: WorkspaceAgentSessionId, - val workspaceId: WorkspaceId, - val upstreamUri: URI, - val kind: AgentKindLabel, - ) - private data class BrowserCursor( val epoch: String?, val offset: String?, ) - /** Outcome of the full attach precondition check. */ - private sealed interface AttachOutcome { - /** All preconditions passed; the attach can proceed. */ - data class Ready( - val sessionId: WorkspaceAgentSessionId, - val workspace: Workspace, - val gatewayAgentId: String, - val gatewayEndpoint: String, - val kind: AgentKindLabel, - ) : AttachOutcome - - /** A precondition failed; close the client with this status. */ - data class Rejected( - val reason: String, - val status: CloseStatus, - val failureReason: FailureReasonLabel, - val kind: AgentKindLabel = AgentKindLabel.OTHER, - ) : AttachOutcome - } - - /** - * Resolves the client WS into the upstream URI plus the - * workspace id we need to mark "active". Returns null when - * the client WS has already been closed with a labelled error. - */ - private fun resolveAttach(clientSession: WebSocketSession): ResolvedAttach? { - val outcome = checkAttachPreconditions(clientSession) - if (outcome is AttachOutcome.Rejected) return closeAndReturn(clientSession, outcome) - val ready = outcome as AttachOutcome.Ready - val upstreamUri = upstreamUri(ready.gatewayEndpoint, ready.gatewayAgentId, browserCursorOf(clientSession)) - return ResolvedAttach(ready.sessionId, ready.workspace.id, upstreamUri, ready.kind) - } - - private data class LoadedSession( - val sessionId: WorkspaceAgentSessionId, - val session: com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession, - val rejection: AttachOutcome.Rejected?, - ) - - private fun loadSession(clientSession: WebSocketSession): LoadedSession? { - val sessionId = sessionIdOf(clientSession) ?: return null - val agentSession = sessions.findById(sessionId) ?: return null - return LoadedSession(sessionId, agentSession, null) - } - - private fun sessionRejection(clientSession: WebSocketSession): AttachOutcome.Rejected = - if (sessionIdOf(clientSession) == null) { - AttachOutcome.Rejected("malformed sessionId", CloseStatus.BAD_DATA, FailureReasonLabel.INVALID_REQUEST) - } else { - AttachOutcome.Rejected("unknown session", CloseStatus.BAD_DATA, FailureReasonLabel.NOT_FOUND) - } - - /** - * Runs all attach preconditions (sessionId, session lookup, - * rebind path, status, workspace, setup guards, gateway binding) - * and returns either Ready (all pass) or Rejected (first failure). - */ - private fun checkAttachPreconditions(clientSession: WebSocketSession): AttachOutcome { - val loaded = loadSession(clientSession) ?: return sessionRejection(clientSession) - var agentSession = loaded.session - val kind = AgentKindLabel.fromRaw(agentSession.kind.name) - var reboundWorkspace: Workspace? = null - if (agentSession.status == WorkspaceAgentSessionStatus.RUNNING && agentSession.gatewayAgentId == null) { - val rebind = attemptRebind(loaded.sessionId, kind) - if (rebind.rejection != null) return rebind.rejection - agentSession = rebind.session ?: agentSession - reboundWorkspace = rebind.workspace - } - return checkWorkspacePreconditions(loaded.sessionId, agentSession, reboundWorkspace, kind) - } - - private data class RebindResult( - val session: com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession?, - val workspace: Workspace?, - val rejection: AttachOutcome.Rejected?, - ) - - private fun attemptRebind( - sessionId: WorkspaceAgentSessionId, - kind: AgentKindLabel, - ): RebindResult = - when (val result = binding.ensureBound(EnsureRunnerSessionBoundInput(sessionId = sessionId))) { - is RunnerSessionBindingResult.Bound -> - RebindResult(result.session, result.workspace, null) - is RunnerSessionBindingResult.Conflict -> - RebindResult( - null, - null, - AttachOutcome.Rejected( - "session binding changed", - CloseStatus.SERVICE_RESTARTED, - FailureReasonLabel.OTHER, - kind, - ), - ) - is RunnerSessionBindingResult.Unavailable -> - RebindResult( - null, - null, - AttachOutcome.Rejected( - "runner provisioning", - CloseStatus.SERVICE_RESTARTED, - FailureReasonLabel.UPSTREAM_UNAVAILABLE, - kind, - ), - ) - } - - private fun checkWorkspacePreconditions( - sessionId: WorkspaceAgentSessionId, - agentSession: com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession, - reboundWorkspace: Workspace?, - kind: AgentKindLabel, - ): AttachOutcome { - if (agentSession.status == WorkspaceAgentSessionStatus.STARTING) { - return AttachOutcome.Rejected( - "runner provisioning", - CloseStatus.SERVICE_RESTARTED, - FailureReasonLabel.UPSTREAM_UNAVAILABLE, - kind, - ) - } - val workspace = - reboundWorkspace ?: workspaces.findById(agentSession.workspaceId) - ?: return AttachOutcome.Rejected( - "workspace gone", - CloseStatus.SERVER_ERROR, - FailureReasonLabel.NOT_FOUND, - kind, - ) - return checkGatewayPreconditions(sessionId, agentSession, workspace, kind) - } - - private fun checkGatewayPreconditions( - sessionId: WorkspaceAgentSessionId, - agentSession: com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession, - workspace: Workspace, - kind: AgentKindLabel, - ): AttachOutcome { - if (isSetupTransitionInProgress(agentSession, workspace)) { - return AttachOutcome.Rejected( - "runner setup transition", - CloseStatus.SERVICE_RESTARTED, - FailureReasonLabel.UPSTREAM_UNAVAILABLE, - kind, - ) - } - val gatewayAgentId = - agentSession.gatewayAgentId - ?: return AttachOutcome.Rejected( - "session not bound to a gateway agent", - CloseStatus.SERVER_ERROR, - FailureReasonLabel.UPSTREAM_UNAVAILABLE, - kind, - ) - return workspace.gatewayEndpoint?.let { endpoint -> - AttachOutcome.Ready(sessionId, workspace, gatewayAgentId, endpoint, kind) - } ?: AttachOutcome.Rejected( - "workspace has no gateway endpoint", - CloseStatus.SERVER_ERROR, - FailureReasonLabel.UPSTREAM_UNAVAILABLE, - kind, - ) - } - - private fun closeAndReturn( - session: WebSocketSession, - rejection: AttachOutcome.Rejected, - ): ResolvedAttach? { - recordAttach(rejection.kind, OutcomeLabel.FAILURE, rejection.failureReason) - session.close(rejection.status.withReason(rejection.reason)) - return null - } - override fun afterConnectionEstablished(clientSession: WebSocketSession) { - val resolved = resolveAttach(clientSession) ?: return + val ready = preconditions.resolveAttach(clientSession, AttachPreconditionChecker::sessionIdOf) ?: return val upstreamHandler = UpstreamHandler( client = clientSession, - sessionId = resolved.sessionId, - workspaceId = resolved.workspaceId, + sessionId = ready.sessionId, + workspaceId = ready.workspace.id, relay = UpstreamRelayDependencies( sessions = sessions, @@ -309,25 +128,26 @@ class SessionAttachHandler( ), telemetry = telemetry, ) + val upstreamUri = upstreamUri(ready.gatewayEndpoint, ready.gatewayAgentId, browserCursorOf(clientSession)) val upstream = runCatching { client - .execute(upstreamHandler, resolved.upstreamUri.toString()) + .execute(upstreamHandler, upstreamUri.toString()) .get(UPSTREAM_HANDSHAKE_SECONDS, TimeUnit.SECONDS) }.getOrElse { - recordAttach(resolved.kind, OutcomeLabel.FAILURE, FailureReasonLabel.UPSTREAM_UNAVAILABLE) + preconditions.recordAttach(ready.kind, OutcomeLabel.FAILURE, FailureReasonLabel.UPSTREAM_UNAVAILABLE) clientSession.close(CloseStatus.SERVICE_RESTARTED.withReason("runner attach unavailable")) return } - recordAttach(resolved.kind, OutcomeLabel.SUCCESS, FailureReasonLabel.NONE) - bridges[clientSession.id] = Bridge(resolved.sessionId, resolved.workspaceId, upstream) - connected.attach(resolved.workspaceId) - activity.touch(resolved.workspaceId) + preconditions.recordAttach(ready.kind, OutcomeLabel.SUCCESS, FailureReasonLabel.NONE) + bridges[clientSession.id] = Bridge(ready.sessionId, ready.workspace.id, upstream) + connected.attach(ready.workspace.id) + activity.touch(ready.workspace.id) log.info( "attached client {} to session {} via {}", clientSession.id, - resolved.sessionId, - resolved.upstreamUri, + ready.sessionId, + upstreamUri, ) } @@ -393,13 +213,6 @@ class SessionAttachHandler( } } - private fun sessionIdOf(session: WebSocketSession): WorkspaceAgentSessionId? { - val match = - Regex("/api/v1/ws/sessions/([^/]+)/attach").find(session.uri?.path ?: return null) - ?: return null - return runCatching { WorkspaceAgentSessionId(UUID.fromString(match.groupValues[1])) }.getOrNull() - } - private fun browserCursorOf(session: WebSocketSession): BrowserCursor { val query = queryOf(session) return BrowserCursor( @@ -427,21 +240,6 @@ class SessionAttachHandler( private fun nonNegativeInteger(value: String?): String? = value?.takeIf { it.matches(NON_NEGATIVE_INTEGER) && it.toLongOrNull() != null } - private fun Workspace.hasRunnerSetupGuard(): Boolean = - runnerSetupOperation != RunnerSetupOperation.IDLE || - pendingRunnerSetupId != null || - pendingRunnerSetupVersion != null - - private fun isSetupTransitionInProgress( - agentSession: com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession, - workspace: Workspace, - ): Boolean = - agentSession.pendingSetupId != null || - agentSession.pendingSetupVersion != null || - workspace.hasRunnerSetupGuard() || - workspace.currentRunnerSetupId != agentSession.currentSetupId || - workspace.currentRunnerSetupVersion != agentSession.currentSetupVersion - private fun upstreamUri( gatewayBase: String, gatewayAgentId: String, @@ -465,6 +263,21 @@ class SessionAttachHandler( return builder.build().toUri() } + private fun recordAttachOperation( + outcome: OutcomeLabel, + reason: FailureReasonLabel, + ) { + telemetry.recordOperation( + OperationTelemetry( + operation = OperationLabel.ATTACH_SESSION, + mode = ModeLabel.INTERACTIVE, + outcome = outcome, + reason = reason, + duration = Duration.ZERO, + ), + ) + } + companion object { private const val UPSTREAM_HANDSHAKE_SECONDS = 5L @@ -492,38 +305,6 @@ class SessionAttachHandler( } } - private fun recordAttach( - kind: AgentKindLabel, - outcome: OutcomeLabel, - reason: FailureReasonLabel, - ) { - val event = - AttachAttemptTelemetry( - kind = kind, - runMode = RunModeLabel.INTERACTIVE, - outcome = outcome, - reason = reason, - ) - telemetry.recordAttachAttempt(event) - if (outcome == OutcomeLabel.FAILURE) telemetry.recordAttachFailure(event) - recordAttachOperation(outcome, reason) - } - - private fun recordAttachOperation( - outcome: OutcomeLabel, - reason: FailureReasonLabel, - ) { - telemetry.recordOperation( - OperationTelemetry( - operation = OperationLabel.ATTACH_SESSION, - mode = ModeLabel.INTERACTIVE, - outcome = outcome, - reason = reason, - duration = Duration.ZERO, - ), - ) - } - /** * Inbound from gateway: shovel the frame straight back to the * browser and record activity so the idle sweep knows the AI is @@ -637,5 +418,23 @@ class SessionAttachHandler( ), ) } + + companion object { + private fun outcomeOf(status: CloseStatus): OutcomeLabel = + when (status.code) { + CloseStatus.NORMAL.code -> OutcomeLabel.SUCCESS + CloseStatus.GOING_AWAY.code -> OutcomeLabel.CANCELLED + else -> OutcomeLabel.FAILURE + } + + private fun failureReasonOf(status: CloseStatus): FailureReasonLabel = + when (status.code) { + CloseStatus.NORMAL.code -> FailureReasonLabel.NONE + CloseStatus.GOING_AWAY.code -> FailureReasonLabel.CANCELLED + CloseStatus.BAD_DATA.code -> FailureReasonLabel.INVALID_REQUEST + CloseStatus.SERVICE_RESTARTED.code -> FailureReasonLabel.UPSTREAM_UNAVAILABLE + else -> FailureReasonLabel.OTHER + } + } } } diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt new file mode 100644 index 0000000..6b2e834 --- /dev/null +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt @@ -0,0 +1,210 @@ +package com.jorisjonkers.personalstack.agents.infrastructure.ws + +import com.jorisjonkers.personalstack.agents.application.observability.AgentKindLabel +import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.AttachAttemptTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel +import com.jorisjonkers.personalstack.agents.application.observability.OperationTelemetry +import com.jorisjonkers.personalstack.agents.application.observability.OutcomeLabel +import com.jorisjonkers.personalstack.agents.application.sessionbinding.EnsureRunnerSessionBoundInput +import com.jorisjonkers.personalstack.agents.application.sessionbinding.RunnerProvisioningResult +import com.jorisjonkers.personalstack.agents.application.sessionbinding.RunnerSessionBindingResult +import com.jorisjonkers.personalstack.agents.application.sessionbinding.RunnerSessionBindingService +import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupId +import com.jorisjonkers.personalstack.agents.domain.model.AgentSetupVersion +import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionStatus +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceId +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceStatus +import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient +import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceAgentSessionRepository +import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepository +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.web.socket.CloseStatus +import org.springframework.web.socket.WebSocketSession +import java.net.URI +import java.time.Instant + +class AttachPreconditionCheckerTest { + private val sessions = mockk() + private val workspaces = mockk() + private val binding = mockk() + private val telemetry = RecordingTelemetry() + private val checker = AttachPreconditionChecker(sessions, workspaces, binding, telemetry) + + private val sessionId = WorkspaceAgentSessionId.random() + private val workspaceId = WorkspaceId.random() + + @Test + fun `resolveAttach returns Ready when all preconditions pass`() { + val ws = workspace() + every { sessions.findById(sessionId) } returns agentSession(gatewayAgentId = "gw-1") + every { workspaces.findById(workspaceId) } returns ws + + val result = checker.resolveAttach(clientSession(), AttachPreconditionChecker::sessionIdOf) + + assertThat(result).isNotNull + assertThat(result?.gatewayAgentId).isEqualTo("gw-1") + assertThat(result?.gatewayEndpoint).isEqualTo("http://runner:8090") + } + + @Test + fun `resolveAttach rejects and closes when session id is malformed`() { + val client = mockk(relaxed = true) + every { client.uri } returns URI.create("ws://api/api/v1/ws/sessions/not-a-uuid/attach") + val closed = slot() + every { client.close(capture(closed)) } returns Unit + + val result = checker.resolveAttach(client, AttachPreconditionChecker::sessionIdOf) + + assertThat(result).isNull() + assertThat(closed.captured.code).isEqualTo(CloseStatus.BAD_DATA.code) + assertThat(telemetry.attachFailures.single().reason).isEqualTo(FailureReasonLabel.INVALID_REQUEST) + } + + @Test + fun `resolveAttach rejects and closes when session is not found`() { + every { sessions.findById(sessionId) } returns null + val client = clientSession() + every { client.close(any()) } returns Unit + + val result = checker.resolveAttach(client, AttachPreconditionChecker::sessionIdOf) + + assertThat(result).isNull() + assertThat(telemetry.attachFailures.single().reason).isEqualTo(FailureReasonLabel.NOT_FOUND) + } + + @Test + fun `resolveAttach rejects when session is still STARTING`() { + every { sessions.findById(sessionId) } returns + agentSession( + status = WorkspaceAgentSessionStatus.STARTING, + gatewayAgentId = null, + ) + val client = clientSession() + every { client.close(any()) } returns Unit + + val result = checker.resolveAttach(client, AttachPreconditionChecker::sessionIdOf) + + assertThat(result).isNull() + assertThat(telemetry.attachFailures.single().reason).isEqualTo(FailureReasonLabel.UPSTREAM_UNAVAILABLE) + } + + @Test + fun `resolveAttach rejects when workspace setup transition is pending`() { + every { sessions.findById(sessionId) } returns agentSession(gatewayAgentId = "gw-1") + every { workspaces.findById(workspaceId) } returns + workspace().copy( + pendingRunnerSetupId = AgentSetupId("gpu"), + pendingRunnerSetupVersion = AgentSetupVersion(2), + ) + val client = clientSession() + every { client.close(any()) } returns Unit + + val result = checker.resolveAttach(client, AttachPreconditionChecker::sessionIdOf) + + assertThat(result).isNull() + val closed = slot() + verify { client.close(capture(closed)) } + assertThat(closed.captured.reason).isEqualTo("runner setup transition") + assertThat(telemetry.attachFailures.single().reason).isEqualTo(FailureReasonLabel.UPSTREAM_UNAVAILABLE) + } + + @Test + fun `resolveAttach attempts rebind when session is RUNNING without gateway binding`() { + val rebound = agentSession(gatewayAgentId = "gw-fresh") + every { sessions.findById(sessionId) } returns agentSession(gatewayAgentId = null) + every { workspaces.findById(workspaceId) } returns workspace() + every { + binding.ensureBound(EnsureRunnerSessionBoundInput(sessionId = sessionId)) + } returns + RunnerSessionBindingResult.Bound( + workspace = workspace(), + session = rebound, + gatewayAgent = + AgentGatewayClient.GatewayAgent( + id = "gw-fresh", + kind = WorkspaceAgentKind.CLAUDE, + cwd = "/workspace", + ), + provisioning = RunnerProvisioningResult.AlreadyReady, + ) + + val result = checker.resolveAttach(clientSession(), AttachPreconditionChecker::sessionIdOf) + + assertThat(result).isNotNull + assertThat(result?.gatewayAgentId).isEqualTo("gw-fresh") + } + + @Test + fun `resolveAttach records bounded telemetry on success`() { + every { sessions.findById(sessionId) } returns agentSession(gatewayAgentId = "gw-1") + every { workspaces.findById(workspaceId) } returns workspace() + + // resolveAttach returns Ready without recording — caller records; direct call to recordAttach + checker.recordAttach(AgentKindLabel.CLAUDE, OutcomeLabel.SUCCESS, FailureReasonLabel.NONE) + + assertThat(telemetry.attachAttempts).hasSize(1) + assertThat(telemetry.attachAttempts.single().outcome).isEqualTo(OutcomeLabel.SUCCESS) + assertThat(telemetry.attachFailures).isEmpty() + } + + private fun clientSession(): WebSocketSession { + val client = mockk(relaxed = true) + every { client.uri } returns URI.create("ws://api/api/v1/ws/sessions/${sessionId.value}/attach") + return client + } + + private fun agentSession( + gatewayAgentId: String? = "gw-1", + status: WorkspaceAgentSessionStatus = WorkspaceAgentSessionStatus.RUNNING, + ) = WorkspaceAgentSession( + id = sessionId, + workspaceId = workspaceId, + kind = WorkspaceAgentKind.CLAUDE, + gatewayAgentId = gatewayAgentId, + status = status, + createdAt = Instant.now(), + updatedAt = Instant.now(), + ) + + private fun workspace() = + Workspace( + id = workspaceId, + name = "demo", + repoUrl = null, + branch = null, + podName = null, + pvcName = null, + gatewayEndpoint = "http://runner:8090", + status = WorkspaceStatus.READY, + createdAt = Instant.now(), + updatedAt = Instant.now(), + ) + + private class RecordingTelemetry : AgentsApiTelemetry { + val attachAttempts = mutableListOf() + val attachFailures = mutableListOf() + val operations = mutableListOf() + + override fun recordAttachAttempt(event: AttachAttemptTelemetry) { + attachAttempts += event + } + + override fun recordAttachFailure(event: AttachAttemptTelemetry) { + attachFailures += event + } + + override fun recordOperation(event: OperationTelemetry) { + operations += event + } + } +} From 56e38480bbfc184f2edee6ac3e65cf095305311a Mon Sep 17 00:00:00 2001 From: JorisJonkers Agent Date: Mon, 13 Jul 2026 08:06:31 +0000 Subject: [PATCH 05/10] fix: address cross-provider review findings on the class splits - Restore the AgentRunnerOrchestrator import in RunnerSessionBinder (referenced by RunnerSessionBindingDependencies). - Remove the retained @Suppress("LongMethod") on RunnerPodSpecBuilder.pod() by extracting completed fabric8 model objects (podMetadata, podSecurityContext, agentRunnerContainer, shared httpProbe, containerResources), and split env/volume building into RunnerContainerEnvBuilder and RunnerVolumeSpecBuilder so every class in the file stays under the 15-function detekt threshold. - Extract RunnerBindingGuards from RunnerSessionBinder (guard predicates and readiness checks), bringing the binder to 14 functions. - Decompose AttachPreconditionChecker.checkPreconditions into rebindIfUnbound, checkWorkspaceAndGateway, and gatewayOutcome to satisfy LongMethod and ReturnCount; resolveAttach now uses the companion sessionIdOf directly (a ClassName::companionMember reference does not compile). - Wrap the five over-120-column lines in RunnerSessionBinder. - Add the missing RunnerState import and drop the now-unused Pod import in the k8s adapters; drop unused PodSpec/ContainerBuilder test imports. - Update AgentRunnerObservabilityContractTest probe assertions to the shared httpProbe helper (single /healthz literal wired into all three probes). --- .../sessionbinding/RunnerBindingGuards.kt | 60 ++++ .../sessionbinding/RunnerSessionBinder.kt | 87 ++--- .../k8s/Fabric8AgentRunnerOrchestrator.kt | 1 - .../k8s/RunnerPodSpecBuilder.kt | 320 ++++++++++-------- .../ws/AttachPreconditionChecker.kt | 163 +++++---- .../infrastructure/ws/SessionAttachHandler.kt | 3 +- .../AgentRunnerObservabilityContractTest.kt | 12 +- .../k8s/RunnerStateReaderTest.kt | 2 - .../ws/AttachPreconditionCheckerTest.kt | 12 +- 9 files changed, 383 insertions(+), 277 deletions(-) create mode 100644 api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingGuards.kt diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingGuards.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingGuards.kt new file mode 100644 index 0000000..9ebd1ee --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingGuards.kt @@ -0,0 +1,60 @@ +package com.jorisjonkers.personalstack.agents.application.sessionbinding + +import com.jorisjonkers.personalstack.agents.application.workspacerunner.RunnerSetupTarget +import com.jorisjonkers.personalstack.agents.application.workspacerunner.RunnerUnavailableReason +import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupOperation +import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession + +/** + * Guard checks shared by the runner session binding flows. Extracted from + * RunnerSessionBinder to keep that class below the TooManyFunctions threshold. + */ +internal class RunnerBindingGuards( + private val provisioning: RunnerProvisioningCoordinator, +) { + fun hasSetupOperationInProgress( + session: WorkspaceAgentSession, + workspace: Workspace, + ): Boolean = sessionHasPendingSetup(session) || workspaceSetupInProgress(workspace) + + // Returns a terminal guard result when the session or workspace is blocked, null otherwise. + fun ensureBoundGuard( + session: WorkspaceAgentSession, + workspace: Workspace, + ): RunnerSessionBindingResult? = + when { + sessionHasPendingSetup(session) -> RunnerSessionBindingResult.Conflict(current = session) + workspaceSetupInProgress(workspace) -> + RunnerSessionBindingResult.Unavailable( + workspaceId = workspace.id, + runnerStatus = RunnerUnavailableReason.SETUP_OPERATION_IN_PROGRESS.label, + ) + else -> null + } + + fun checkBindingReadiness( + workspace: Workspace, + target: RunnerSetupTarget, + ): RunnerSessionBindingResult.Unavailable? { + val unavailableReason = + when { + workspaceSetupInProgress(workspace) -> RunnerUnavailableReason.SETUP_OPERATION_IN_PROGRESS.label + workspace.runnerBootLeaseId != null -> RunnerUnavailableReason.BOOT_LEASE_HELD.label + !provisioning.isRunnerReadyFor(workspace, target) -> + RunnerUnavailableReason.NOT_READY_AFTER_PROVISION.label + else -> null + } + return unavailableReason?.let { + RunnerSessionBindingResult.Unavailable(workspaceId = workspace.id, runnerStatus = it) + } + } + + private fun sessionHasPendingSetup(session: WorkspaceAgentSession): Boolean = + session.pendingSetupId != null || session.pendingSetupVersion != null + + private fun workspaceSetupInProgress(workspace: Workspace): Boolean = + workspace.pendingRunnerSetupId != null || + workspace.pendingRunnerSetupVersion != null || + workspace.runnerSetupOperation != RunnerSetupOperation.IDLE +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBinder.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBinder.kt index 1298dde..395543a 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBinder.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerSessionBinder.kt @@ -10,11 +10,11 @@ import com.jorisjonkers.personalstack.agents.application.setup.AgentSetupSelecti import com.jorisjonkers.personalstack.agents.application.setup.AgentSetupValidationService import com.jorisjonkers.personalstack.agents.application.workspacerunner.RunnerSetupTarget import com.jorisjonkers.personalstack.agents.application.workspacerunner.RunnerUnavailableReason -import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupOperation import com.jorisjonkers.personalstack.agents.domain.model.Workspace import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionStatus import com.jorisjonkers.personalstack.agents.domain.port.AgentGatewayClient +import com.jorisjonkers.personalstack.agents.domain.port.AgentRunnerOrchestrator import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceAgentSessionRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepository import org.slf4j.LoggerFactory @@ -70,6 +70,7 @@ class RunnerSessionBinder( metrics, backoffInitialMs, ) + private val guards = RunnerBindingGuards(provisioning) private val log = LoggerFactory.getLogger(RunnerSessionBinder::class.java) override fun start(request: StartRunnerSessionBindingInput): RunnerSessionBindingResult = @@ -83,7 +84,7 @@ class RunnerSessionBinder( ?: throw NoSuchElementException("workspace not found: ${request.workspaceId.value}") val target = setupResolver.resolveNewSessionSetup(workspace, request.kind, request.setupId, request.setupVersion) - checkBindingReadiness(workspace, target)?.let { return it } + guards.checkBindingReadiness(workspace, target)?.let { return it } val now = Instant.now() val session = newSession(request, workspace, target, now) val gatewayAgent = spawner.spawnWithRetry(workspace, session, continuation = null) @@ -157,22 +158,14 @@ class RunnerSessionBinder( require(session.workspaceId == request.workspaceId) { "session does not belong to workspace: ${request.sessionId.value}" } - if (session.generation != request.expectedGeneration || hasSetupOperationInProgress(session, workspace)) { + if (session.generation != request.expectedGeneration || + guards.hasSetupOperationInProgress(session, workspace) + ) { return RunnerSessionBindingResult.Conflict(current = session) } return performRestart(session, workspace, request) } - private fun hasSetupOperationInProgress( - session: WorkspaceAgentSession, - workspace: Workspace, - ): Boolean = - session.pendingSetupId != null || - session.pendingSetupVersion != null || - workspace.pendingRunnerSetupId != null || - workspace.pendingRunnerSetupVersion != null || - workspace.runnerSetupOperation != RunnerSetupOperation.IDLE - private fun performRestart( session: WorkspaceAgentSession, workspace: Workspace, @@ -198,12 +191,13 @@ class RunnerSessionBinder( } val ready = - runCatching { provisioning.forceProvisionAndWait(leasedWorkspace, target, leasedWorkspace.runnerSetupGeneration) } - .getOrElse { ex -> - tx.markFailed(starting) - tx.failWorkspaceSetupOperation(leasedWorkspace) - throw ex - } + runCatching { + provisioning.forceProvisionAndWait(leasedWorkspace, target, leasedWorkspace.runnerSetupGeneration) + }.getOrElse { ex -> + tx.markFailed(starting) + tx.failWorkspaceSetupOperation(leasedWorkspace) + throw ex + } return completeRestart(session, starting, ready, target, request) } @@ -274,7 +268,7 @@ class RunnerSessionBinder( require(session.workspaceId == workspaceId) { "session does not belong to workspace: ${request.sessionId.value}" } - ensureBoundGuard(session, workspace)?.let { return it } + guards.ensureBoundGuard(session, workspace)?.let { return it } val target = setupResolver.resolveSessionSetup(workspace, session) require( session.status == WorkspaceAgentSessionStatus.RUNNING || @@ -286,36 +280,15 @@ class RunnerSessionBinder( ?: checkReadinessAndRebind(session, workspace, target) } - // Returns a terminal guard result when the session or workspace is blocked, null otherwise. - private fun ensureBoundGuard( - session: WorkspaceAgentSession, - workspace: Workspace, - ): RunnerSessionBindingResult? = - when { - sessionHasPendingSetup(session) -> RunnerSessionBindingResult.Conflict(current = session) - workspaceSetupInProgress(workspace) -> - RunnerSessionBindingResult.Unavailable( - workspaceId = workspace.id, - runnerStatus = RunnerUnavailableReason.SETUP_OPERATION_IN_PROGRESS.label, - ) - else -> null - } - - private fun sessionHasPendingSetup(session: WorkspaceAgentSession): Boolean = - session.pendingSetupId != null || session.pendingSetupVersion != null - - private fun workspaceSetupInProgress(workspace: Workspace): Boolean = - workspace.pendingRunnerSetupId != null || - workspace.pendingRunnerSetupVersion != null || - workspace.runnerSetupOperation != RunnerSetupOperation.IDLE - private fun tryFastPathBound( session: WorkspaceAgentSession, workspace: Workspace, target: RunnerSetupTarget, ): RunnerSessionBindingResult.Bound? { val gatewayAgentId = session.gatewayAgentId ?: return null - if (session.status != WorkspaceAgentSessionStatus.RUNNING || !provisioning.isRunnerReadyFor(workspace, target)) { + if (session.status != WorkspaceAgentSessionStatus.RUNNING || + !provisioning.isRunnerReadyFor(workspace, target) + ) { return null } return RunnerSessionBindingResult.Bound( @@ -343,7 +316,8 @@ class RunnerSessionBinder( val unavailableReason = when { workspace.runnerBootLeaseId != null -> RunnerUnavailableReason.BOOT_LEASE_HELD.label - !provisioning.isRunnerReadyFor(workspace, target) -> RunnerUnavailableReason.NOT_READY_AFTER_PROVISION.label + !provisioning.isRunnerReadyFor(workspace, target) -> + RunnerUnavailableReason.NOT_READY_AFTER_PROVISION.label else -> null } if (unavailableReason != null) { @@ -386,7 +360,9 @@ class RunnerSessionBinder( metrics.observeStage( operation = OperationLabel.ATTACH_SESSION, mode = ModeLabel.INTERACTIVE, - outcome = { if (it) OutcomeLabel.SUCCESS to FailureReasonLabel.NONE else metrics.bindingConflict() }, + outcome = { changed -> + if (changed) OutcomeLabel.SUCCESS to FailureReasonLabel.NONE else metrics.bindingConflict() + }, ) { tx.bind(session, gatewayAgent, promotePendingSetup) } @@ -411,25 +387,6 @@ class RunnerSessionBinder( ) } - private fun checkBindingReadiness( - workspace: Workspace, - target: RunnerSetupTarget, - ): RunnerSessionBindingResult.Unavailable? { - val unavailableReason = - when { - workspace.runnerSetupOperation != RunnerSetupOperation.IDLE || - workspace.pendingRunnerSetupId != null || - workspace.pendingRunnerSetupVersion != null -> - RunnerUnavailableReason.SETUP_OPERATION_IN_PROGRESS.label - workspace.runnerBootLeaseId != null -> RunnerUnavailableReason.BOOT_LEASE_HELD.label - !provisioning.isRunnerReadyFor(workspace, target) -> RunnerUnavailableReason.NOT_READY_AFTER_PROVISION.label - else -> null - } - return unavailableReason?.let { - RunnerSessionBindingResult.Unavailable(workspaceId = workspace.id, runnerStatus = it) - } - } - companion object { const val MAX_SPAWN_ATTEMPTS: Int = RunnerAgentSpawner.MAX_SPAWN_ATTEMPTS const val BACKOFF_INITIAL_MS: Long = 1_000 diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt index 4ec81f3..187ed16 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt @@ -10,7 +10,6 @@ import com.jorisjonkers.personalstack.agents.domain.port.AgentCredentialReposito import com.jorisjonkers.personalstack.agents.domain.port.AgentRunnerOrchestrator import com.jorisjonkers.personalstack.agents.domain.port.RepositoryRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepositoryRepository -import io.fabric8.kubernetes.api.model.Pod import io.fabric8.kubernetes.client.KubernetesClient import org.slf4j.LoggerFactory import org.springframework.beans.factory.ObjectProvider diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt index cf6d209..6e1196d 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt @@ -2,40 +2,59 @@ package com.jorisjonkers.personalstack.agents.infrastructure.k8s import com.jorisjonkers.personalstack.agents.config.AgentRuntimeProperties import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupProvisioningSpec +import com.jorisjonkers.personalstack.agents.domain.model.RunnerState import com.jorisjonkers.personalstack.agents.domain.model.Workspace import com.jorisjonkers.personalstack.agents.domain.port.RepositoryRepository import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceRepositoryRepository import com.jorisjonkers.personalstack.agents.infrastructure.k8s.RunnerCredentialSecretManager.CredentialSecret +import io.fabric8.kubernetes.api.model.Container import io.fabric8.kubernetes.api.model.ContainerBuilder import io.fabric8.kubernetes.api.model.ContainerPortBuilder import io.fabric8.kubernetes.api.model.EnvVarBuilder +import io.fabric8.kubernetes.api.model.ObjectMeta +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder import io.fabric8.kubernetes.api.model.PersistentVolumeClaim import io.fabric8.kubernetes.api.model.PersistentVolumeClaimBuilder import io.fabric8.kubernetes.api.model.Pod import io.fabric8.kubernetes.api.model.PodBuilder +import io.fabric8.kubernetes.api.model.PodSecurityContext +import io.fabric8.kubernetes.api.model.PodSecurityContextBuilder +import io.fabric8.kubernetes.api.model.Probe +import io.fabric8.kubernetes.api.model.ProbeBuilder import io.fabric8.kubernetes.api.model.Quantity +import io.fabric8.kubernetes.api.model.ResourceRequirements +import io.fabric8.kubernetes.api.model.ResourceRequirementsBuilder import io.fabric8.kubernetes.api.model.ServiceBuilder import io.fabric8.kubernetes.api.model.Volume import io.fabric8.kubernetes.api.model.VolumeBuilder import io.fabric8.kubernetes.api.model.VolumeMountBuilder import org.springframework.beans.factory.ObjectProvider +private const val DOCKER_SOCKET_VOLUME = "docker-socket" +private const val AGENT_CREDENTIALS_VOLUME = "agent-credentials" +private const val AGENT_CREDENTIALS_MOUNT = "/var/run/secrets/agents/credentials" + /** * Builds Kubernetes resource specs (Pod, PVC, Service) for a workspace runner. * Extracted from Fabric8AgentRunnerOrchestrator to keep that class below the * TooManyFunctions and LargeClass thresholds. * - * fabric8's fluent builder chains naturally split into one helper per pod section - * (labels, env, mounts, volumes, container body); each helper operates on the - * same shared props. The @Suppress("LongMethod") on pod() is intentional — - * the intermediate fluent types are package-private and cannot be split further. + * The pod spec splits along completed fabric8 model objects: metadata, + * security context, the agent-runner container (with probes and resources), + * env vars (RunnerContainerEnvBuilder), and volumes (RunnerVolumeSpecBuilder). + * The inline `mapOf(...)` calls are intentional: extracting them into typed + * vals trips Kotlin overload resolution on fabric8 7.x's `withLabels` / + * `withRequests` / `withLimits`. */ internal class RunnerPodSpecBuilder( private val props: AgentRuntimeProperties, - private val workspaceRepos: ObjectProvider, - private val repositories: ObjectProvider, + workspaceRepos: ObjectProvider, + repositories: ObjectProvider, private val ownReleaseVersion: () -> String?, ) { + private val env = RunnerContainerEnvBuilder(props, workspaceRepos, repositories) + private val volumes = RunnerVolumeSpecBuilder() + fun pvc(name: String): PersistentVolumeClaim = PersistentVolumeClaimBuilder() .withNewMetadata() @@ -51,13 +70,6 @@ internal class RunnerPodSpecBuilder( .endSpec() .build() - // fabric8 builder chains can't be cleanly split into helpers - // because the intermediate fluent types are private; LongMethod - // is the natural shape here and suppressed with intent. The - // inline `mapOf(...)` calls are likewise intentional: extracting - // them into typed vals trips Kotlin overload resolution on - // fabric8 7.x's `withLabels` / `withRequests` / `withLimits`. - @Suppress("LongMethod") fun pod( workspace: Workspace, setup: RunnerSetupProvisioningSpec, @@ -66,12 +78,7 @@ internal class RunnerPodSpecBuilder( credentialSecret: CredentialSecret?, ): Pod = PodBuilder() - .withNewMetadata() - .withName(names.pod) - .withNamespace(props.namespace) - .withLabels(podLabels(workspace, setup, runnerGeneration)) - .withAnnotations(podAnnotations(setup)) - .endMetadata() + .withMetadata(podMetadata(workspace, setup, runnerGeneration, names)) .withNewSpec() .withServiceAccountName(setup.serviceAccount) // The runner never calls the Kubernetes API itself — cluster @@ -83,61 +90,10 @@ internal class RunnerPodSpecBuilder( .withAutomountServiceAccountToken(false) .withNodeSelector(setup.nodeSelector) .withRestartPolicy("Always") - .withNewSecurityContext() - .withRunAsUser(RUN_AS_UID) - .withRunAsGroup(RUN_AS_GID) - .withFsGroup(FS_GROUP) - .withSupplementalGroups(podSupplementalGroups(setup)) - .endSecurityContext() + .withSecurityContext(podSecurityContext(setup)) .withInitContainers(agentStateInitContainer(setup)) - .addNewContainer() - .withName("agent-runner") - // Pin to the release agents-api itself is on so the running - // version is a verifiable fact in the Pod spec, not :latest. - .withImage(RunnerImageVersions.pin(setup.image, ownReleaseVersion())) - .withImagePullPolicy(setup.imagePullPolicy) - .withPorts( - ContainerPortBuilder() - .withName("gateway") - .withContainerPort(setup.gatewayPort) - .build(), - ).withEnv(podEnv(workspace, setup, runnerGeneration, credentialSecret)) - .withVolumeMounts(podVolumeMounts(setup, credentialSecret)) - // Startup probe gates liveness + readiness until the gateway's - // JVM has finished its cold start. Without it the liveness probe - // (failureThreshold 3 x 10s ~= 30s, no initial delay) killed the - // booting Spring Boot gateway before it bound :8090, which - // re-provisioned the runner in a loop and 503'd every - // start-session. 60 x 5s = 5 min of boot headroom. - .withNewStartupProbe() - .withNewHttpGet() - .withPath("/healthz") - .withNewPort("gateway") - .endHttpGet() - .withPeriodSeconds(STARTUP_PERIOD_SECONDS) - .withFailureThreshold(STARTUP_FAILURE_THRESHOLD) - .endStartupProbe() - .withNewReadinessProbe() - .withNewHttpGet() - .withPath("/healthz") - .withNewPort("gateway") - .endHttpGet() - .withPeriodSeconds(READINESS_PERIOD_SECONDS) - .withFailureThreshold(READINESS_FAILURE_THRESHOLD) - .endReadinessProbe() - .withNewLivenessProbe() - .withNewHttpGet() - .withPath("/healthz") - .withNewPort("gateway") - .endHttpGet() - .withPeriodSeconds(LIVENESS_PERIOD_SECONDS) - .endLivenessProbe() - .withNewResources() - .withRequests(mapOf("cpu" to Quantity(CPU_REQUEST), "memory" to Quantity(MEMORY_REQUEST))) - .withLimits(mapOf("cpu" to Quantity(CPU_LIMIT), "memory" to Quantity(MEMORY_LIMIT))) - .endResources() - .endContainer() - .withVolumes(podVolumes(names.pvc, credentialSecret, setup)) + .withContainers(agentRunnerContainer(workspace, setup, runnerGeneration, credentialSecret)) + .withVolumes(volumes.podVolumes(names.pvc, credentialSecret, setup)) .endSpec() .build() @@ -161,6 +117,19 @@ internal class RunnerPodSpecBuilder( .endSpec() .build() + private fun podMetadata( + workspace: Workspace, + setup: RunnerSetupProvisioningSpec, + runnerGeneration: Long, + names: RunnerResourceNames, + ): ObjectMeta = + ObjectMetaBuilder() + .withName(names.pod) + .withNamespace(props.namespace) + .withLabels(podLabels(workspace, setup, runnerGeneration)) + .withAnnotations(podAnnotations(setup)) + .build() + private fun podLabels( workspace: Workspace, setup: RunnerSetupProvisioningSpec, @@ -178,7 +147,129 @@ internal class RunnerPodSpecBuilder( private fun podAnnotations(setup: RunnerSetupProvisioningSpec): Map = mapOf(RunnerState.ANNOTATION_SETUP_HASH to setup.setupHash) - private fun podEnv( + private fun podSecurityContext(setup: RunnerSetupProvisioningSpec): PodSecurityContext = + PodSecurityContextBuilder() + .withRunAsUser(RUN_AS_UID) + .withRunAsGroup(RUN_AS_GID) + .withFsGroup(FS_GROUP) + .withSupplementalGroups(podSupplementalGroups(setup)) + .build() + + private fun podSupplementalGroups(setup: RunnerSetupProvisioningSpec): List = + if (setup.dockerSocketEnabled) { + (listOf(RUN_AS_GID) + setup.dockerSocketSupplementalGroups).distinct() + } else { + listOf(RUN_AS_GID) + } + + private fun agentRunnerContainer( + workspace: Workspace, + setup: RunnerSetupProvisioningSpec, + runnerGeneration: Long, + credentialSecret: CredentialSecret?, + ): Container = + ContainerBuilder() + .withName("agent-runner") + // Pin to the release agents-api itself is on so the running + // version is a verifiable fact in the Pod spec, not :latest. + .withImage(RunnerImageVersions.pin(setup.image, ownReleaseVersion())) + .withImagePullPolicy(setup.imagePullPolicy) + .withPorts( + ContainerPortBuilder() + .withName("gateway") + .withContainerPort(setup.gatewayPort) + .build(), + ).withEnv(env.podEnv(workspace, setup, runnerGeneration, credentialSecret)) + .withVolumeMounts(volumes.podVolumeMounts(setup, credentialSecret)) + // Startup probe gates liveness + readiness until the gateway's + // JVM has finished its cold start. Without it the liveness probe + // (failureThreshold 3 x 10s ~= 30s, no initial delay) killed the + // booting Spring Boot gateway before it bound :8090, which + // re-provisioned the runner in a loop and 503'd every + // start-session. 60 x 5s = 5 min of boot headroom. + .withStartupProbe(httpProbe(STARTUP_PERIOD_SECONDS, STARTUP_FAILURE_THRESHOLD)) + .withReadinessProbe(httpProbe(READINESS_PERIOD_SECONDS, READINESS_FAILURE_THRESHOLD)) + .withLivenessProbe(httpProbe(LIVENESS_PERIOD_SECONDS)) + .withResources(containerResources()) + .build() + + private fun httpProbe( + periodSeconds: Int, + failureThreshold: Int? = null, + ): Probe { + val probe = + ProbeBuilder() + .withNewHttpGet() + .withPath("/healthz") + .withNewPort("gateway") + .endHttpGet() + .withPeriodSeconds(periodSeconds) + failureThreshold?.let { probe.withFailureThreshold(it) } + return probe.build() + } + + private fun containerResources(): ResourceRequirements = + ResourceRequirementsBuilder() + .withRequests( + mapOf("cpu" to Quantity(CPU_REQUEST), "memory" to Quantity(MEMORY_REQUEST)), + ).withLimits( + mapOf("cpu" to Quantity(CPU_LIMIT), "memory" to Quantity(MEMORY_LIMIT)), + ).build() + + private fun agentStateInitContainer(setup: RunnerSetupProvisioningSpec) = + ContainerBuilder() + .withName("agent-state-init") + .withImage(RunnerImageVersions.pin(setup.image, ownReleaseVersion())) + .withImagePullPolicy(setup.imagePullPolicy) + .withCommand("/bin/sh", "-c") + .withArgs( + "mkdir -p /workspace/.agent-state/claude/projects " + + "/workspace/.agent-state/claude/backups " + + "/workspace/.agent-state/claude/todos " + + "/workspace/.agent-state/claude/shell-snapshots " + + "/workspace/.agent-state/codex/session-homes && " + + "chown -R 1000:1000 /workspace/.agent-state", + ).withVolumeMounts(VolumeMountBuilder().withName("workspace").withMountPath("/workspace").build()) + .withNewSecurityContext() + .withRunAsUser(0L) + .withRunAsGroup(0L) + .endSecurityContext() + .build() + + private companion object { + const val RUN_AS_UID = 1000L + const val RUN_AS_GID = 1000L + const val FS_GROUP = 1000L + + // Probe cadence. The startup probe loops 60×5s = 5 min so the + // gateway's JVM cold start (slower on a fresh image pull) + // completes before liveness can fire; readiness shares the same + // budget as a backstop. + const val STARTUP_PERIOD_SECONDS = 5 + const val STARTUP_FAILURE_THRESHOLD = 60 + const val READINESS_PERIOD_SECONDS = 5 + const val READINESS_FAILURE_THRESHOLD = 60 + const val LIVENESS_PERIOD_SECONDS = 10 + + // Resource sizing. + const val CPU_REQUEST = "250m" + const val MEMORY_REQUEST = "2Gi" + const val CPU_LIMIT = "2000m" + const val MEMORY_LIMIT = "10Gi" + } +} + +/** + * Builds the agent-runner container env for a workspace runner Pod. + * Split from RunnerPodSpecBuilder to keep both classes below the + * TooManyFunctions threshold. + */ +internal class RunnerContainerEnvBuilder( + private val props: AgentRuntimeProperties, + private val workspaceRepos: ObjectProvider, + private val repositories: ObjectProvider, +) { + fun podEnv( workspace: Workspace, setup: RunnerSetupProvisioningSpec, runnerGeneration: Long, @@ -373,8 +464,15 @@ internal class RunnerPodSpecBuilder( } } } +} - private fun podVolumeMounts( +/** + * Builds the volumes and volume mounts for a workspace runner Pod. + * Split from RunnerPodSpecBuilder to keep both classes below the + * TooManyFunctions threshold. + */ +internal class RunnerVolumeSpecBuilder { + fun podVolumeMounts( setup: RunnerSetupProvisioningSpec, credentialSecret: CredentialSecret?, ) = buildList { @@ -409,25 +507,16 @@ internal class RunnerPodSpecBuilder( ) } - private fun agentStateInitContainer(setup: RunnerSetupProvisioningSpec) = - ContainerBuilder() - .withName("agent-state-init") - .withImage(RunnerImageVersions.pin(setup.image, ownReleaseVersion())) - .withImagePullPolicy(setup.imagePullPolicy) - .withCommand("/bin/sh", "-c") - .withArgs( - "mkdir -p /workspace/.agent-state/claude/projects " + - "/workspace/.agent-state/claude/backups " + - "/workspace/.agent-state/claude/todos " + - "/workspace/.agent-state/claude/shell-snapshots " + - "/workspace/.agent-state/codex/session-homes && " + - "chown -R 1000:1000 /workspace/.agent-state", - ).withVolumeMounts(VolumeMountBuilder().withName("workspace").withMountPath("/workspace").build()) - .withNewSecurityContext() - .withRunAsUser(0L) - .withRunAsGroup(0L) - .endSecurityContext() - .build() + fun podVolumes( + workspacePvc: String, + credentialSecret: CredentialSecret?, + setup: RunnerSetupProvisioningSpec, + ) = buildList { + add(pvcVolume("workspace", workspacePvc)) + credentialSecret?.let { add(agentCredentialsVolume(it.name)) } + dockerSocketVolume(setup)?.let(::add) + add(mcpConfigVolume(setup)) + } private fun agentStateVolumeMounts() = listOf( @@ -462,17 +551,6 @@ internal class RunnerPodSpecBuilder( .withSubPath(subPath) .build() - private fun podVolumes( - workspacePvc: String, - credentialSecret: CredentialSecret?, - setup: RunnerSetupProvisioningSpec, - ) = buildList { - add(pvcVolume("workspace", workspacePvc)) - credentialSecret?.let { add(agentCredentialsVolume(it.name)) } - dockerSocketVolume(setup)?.let(::add) - add(mcpConfigVolume(setup)) - } - private fun pvcVolume( name: String, claim: String, @@ -512,36 +590,4 @@ internal class RunnerPodSpecBuilder( .withOptional(true) .endConfigMap() .build() - - private fun podSupplementalGroups(setup: RunnerSetupProvisioningSpec): List = - if (setup.dockerSocketEnabled) { - (listOf(RUN_AS_GID) + setup.dockerSocketSupplementalGroups).distinct() - } else { - listOf(RUN_AS_GID) - } - - private companion object { - const val RUN_AS_UID = 1000L - const val RUN_AS_GID = 1000L - const val FS_GROUP = 1000L - const val DOCKER_SOCKET_VOLUME = "docker-socket" - const val AGENT_CREDENTIALS_VOLUME = "agent-credentials" - const val AGENT_CREDENTIALS_MOUNT = "/var/run/secrets/agents/credentials" - - // Probe cadence. The startup probe loops 60×5s = 5 min so the - // gateway's JVM cold start (slower on a fresh image pull) - // completes before liveness can fire; readiness shares the same - // budget as a backstop. - const val STARTUP_PERIOD_SECONDS = 5 - const val STARTUP_FAILURE_THRESHOLD = 60 - const val READINESS_PERIOD_SECONDS = 5 - const val READINESS_FAILURE_THRESHOLD = 60 - const val LIVENESS_PERIOD_SECONDS = 10 - - // Resource sizing. - const val CPU_REQUEST = "250m" - const val MEMORY_REQUEST = "2Gi" - const val CPU_LIMIT = "2000m" - const val MEMORY_LIMIT = "10Gi" - } } diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt index 33abdf3..40fefd2 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionChecker.kt @@ -1,7 +1,7 @@ package com.jorisjonkers.personalstack.agents.infrastructure.ws -import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry import com.jorisjonkers.personalstack.agents.application.observability.AgentKindLabel +import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry import com.jorisjonkers.personalstack.agents.application.observability.AttachAttemptTelemetry import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel import com.jorisjonkers.personalstack.agents.application.observability.ModeLabel @@ -14,6 +14,7 @@ import com.jorisjonkers.personalstack.agents.application.sessionbinding.RunnerSe import com.jorisjonkers.personalstack.agents.application.sessionbinding.RunnerSessionBindingService import com.jorisjonkers.personalstack.agents.domain.model.RunnerSetupOperation import com.jorisjonkers.personalstack.agents.domain.model.Workspace +import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId import com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionStatus import com.jorisjonkers.personalstack.agents.domain.port.WorkspaceAgentSessionRepository @@ -54,18 +55,26 @@ internal class AttachPreconditionChecker( ) : AttachOutcome } + /** Result of the rebind-if-unbound step: either a usable session or a rejection. */ + private sealed interface RebindOutcome { + data class Rebound( + val session: WorkspaceAgentSession, + val workspace: Workspace?, + ) : RebindOutcome + + data class Rejected( + val outcome: AttachOutcome.Rejected, + ) : RebindOutcome + } + /** * Runs all attach preconditions (sessionId, session lookup, * rebind path, status, workspace, setup guards, gateway binding) - * and returns either Ready (all pass) or Rejected (first failure). - * Records telemetry and closes the client socket on rejection. - * Returns null when the client was already rejected and closed. + * and returns either Ready (all pass) or null (rejected). Records + * telemetry and closes the client socket on rejection. */ - fun resolveAttach( - clientSession: WebSocketSession, - sessionIdOf: (WebSocketSession) -> WorkspaceAgentSessionId?, - ): AttachOutcome.Ready? { - val outcome = checkPreconditions(clientSession, sessionIdOf) + fun resolveAttach(clientSession: WebSocketSession): AttachOutcome.Ready? { + val outcome = checkPreconditions(clientSession) if (outcome is AttachOutcome.Rejected) { recordAttach(outcome.kind, OutcomeLabel.FAILURE, outcome.failureReason) clientSession.close(outcome.status.withReason(outcome.reason)) @@ -74,45 +83,70 @@ internal class AttachPreconditionChecker( return outcome as AttachOutcome.Ready } - private fun checkPreconditions( - clientSession: WebSocketSession, - sessionIdOf: (WebSocketSession) -> WorkspaceAgentSessionId?, - ): AttachOutcome { - val sessionId = sessionIdOf(clientSession) - val agentSession = sessionId?.let { sessions.findById(it) } - if (sessionId == null || agentSession == null) { - return if (sessionId == null) { - AttachOutcome.Rejected("malformed sessionId", CloseStatus.BAD_DATA, FailureReasonLabel.INVALID_REQUEST) - } else { - AttachOutcome.Rejected("unknown session", CloseStatus.BAD_DATA, FailureReasonLabel.NOT_FOUND) - } - } + private fun checkPreconditions(clientSession: WebSocketSession): AttachOutcome { + val sessionId = + sessionIdOf(clientSession) + ?: return AttachOutcome.Rejected( + "malformed sessionId", + CloseStatus.BAD_DATA, + FailureReasonLabel.INVALID_REQUEST, + ) + val agentSession = + sessions.findById(sessionId) + ?: return AttachOutcome.Rejected( + "unknown session", + CloseStatus.BAD_DATA, + FailureReasonLabel.NOT_FOUND, + ) val kind = AgentKindLabel.fromRaw(agentSession.kind.name) - var resolvedSession = agentSession - var reboundWorkspace: Workspace? = null - if (agentSession.status == WorkspaceAgentSessionStatus.RUNNING && agentSession.gatewayAgentId == null) { - when (val result = binding.ensureBound(EnsureRunnerSessionBoundInput(sessionId = sessionId))) { - is RunnerSessionBindingResult.Bound -> { - resolvedSession = result.session - reboundWorkspace = result.workspace - } - is RunnerSessionBindingResult.Conflict -> - return AttachOutcome.Rejected( + return when (val rebind = rebindIfUnbound(sessionId, agentSession, kind)) { + is RebindOutcome.Rejected -> rebind.outcome + is RebindOutcome.Rebound -> + checkWorkspaceAndGateway(sessionId, rebind.session, rebind.workspace, kind) + } + } + + // A RUNNING session without a gateway binding (runner restarted + // underneath it) is rebound through the binding service before the + // attach proceeds, so the bridge always targets a live agent. + private fun rebindIfUnbound( + sessionId: WorkspaceAgentSessionId, + agentSession: WorkspaceAgentSession, + kind: AgentKindLabel, + ): RebindOutcome { + if (agentSession.status != WorkspaceAgentSessionStatus.RUNNING || agentSession.gatewayAgentId != null) { + return RebindOutcome.Rebound(agentSession, workspace = null) + } + return when (val result = binding.ensureBound(EnsureRunnerSessionBoundInput(sessionId = sessionId))) { + is RunnerSessionBindingResult.Bound -> RebindOutcome.Rebound(result.session, result.workspace) + is RunnerSessionBindingResult.Conflict -> + RebindOutcome.Rejected( + AttachOutcome.Rejected( "session binding changed", CloseStatus.SERVICE_RESTARTED, FailureReasonLabel.OTHER, kind, - ) - is RunnerSessionBindingResult.Unavailable -> - return AttachOutcome.Rejected( + ), + ) + is RunnerSessionBindingResult.Unavailable -> + RebindOutcome.Rejected( + AttachOutcome.Rejected( "runner provisioning", CloseStatus.SERVICE_RESTARTED, FailureReasonLabel.UPSTREAM_UNAVAILABLE, kind, - ) - } + ), + ) } - if (resolvedSession.status == WorkspaceAgentSessionStatus.STARTING) { + } + + private fun checkWorkspaceAndGateway( + sessionId: WorkspaceAgentSessionId, + session: WorkspaceAgentSession, + reboundWorkspace: Workspace?, + kind: AgentKindLabel, + ): AttachOutcome { + if (session.status == WorkspaceAgentSessionStatus.STARTING) { return AttachOutcome.Rejected( "runner provisioning", CloseStatus.SERVICE_RESTARTED, @@ -121,41 +155,52 @@ internal class AttachPreconditionChecker( ) } val workspace = - reboundWorkspace ?: workspaces.findById(resolvedSession.workspaceId) + reboundWorkspace ?: workspaces.findById(session.workspaceId) ?: return AttachOutcome.Rejected( "workspace gone", CloseStatus.SERVER_ERROR, FailureReasonLabel.NOT_FOUND, kind, ) - if (isSetupTransitionInProgress(resolvedSession, workspace)) { - return AttachOutcome.Rejected( - "runner setup transition", - CloseStatus.SERVICE_RESTARTED, - FailureReasonLabel.UPSTREAM_UNAVAILABLE, - kind, - ) - } - val gatewayAgentId = - resolvedSession.gatewayAgentId - ?: return AttachOutcome.Rejected( + return gatewayOutcome(sessionId, session, workspace, kind) + } + + private fun gatewayOutcome( + sessionId: WorkspaceAgentSessionId, + session: WorkspaceAgentSession, + workspace: Workspace, + kind: AgentKindLabel, + ): AttachOutcome { + val gatewayAgentId = session.gatewayAgentId + val endpoint = workspace.gatewayEndpoint + return when { + isSetupTransitionInProgress(session, workspace) -> + AttachOutcome.Rejected( + "runner setup transition", + CloseStatus.SERVICE_RESTARTED, + FailureReasonLabel.UPSTREAM_UNAVAILABLE, + kind, + ) + gatewayAgentId == null -> + AttachOutcome.Rejected( "session not bound to a gateway agent", CloseStatus.SERVER_ERROR, FailureReasonLabel.UPSTREAM_UNAVAILABLE, kind, ) - return workspace.gatewayEndpoint?.let { endpoint -> - AttachOutcome.Ready(sessionId, workspace, gatewayAgentId, endpoint, kind) - } ?: AttachOutcome.Rejected( - "workspace has no gateway endpoint", - CloseStatus.SERVER_ERROR, - FailureReasonLabel.UPSTREAM_UNAVAILABLE, - kind, - ) + endpoint == null -> + AttachOutcome.Rejected( + "workspace has no gateway endpoint", + CloseStatus.SERVER_ERROR, + FailureReasonLabel.UPSTREAM_UNAVAILABLE, + kind, + ) + else -> AttachOutcome.Ready(sessionId, workspace, gatewayAgentId, endpoint, kind) + } } private fun isSetupTransitionInProgress( - agentSession: com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession, + agentSession: WorkspaceAgentSession, workspace: Workspace, ): Boolean = agentSession.pendingSetupId != null || diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/SessionAttachHandler.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/SessionAttachHandler.kt index b589371..b8d88ca 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/SessionAttachHandler.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/SessionAttachHandler.kt @@ -2,7 +2,6 @@ package com.jorisjonkers.personalstack.agents.infrastructure.ws import com.jorisjonkers.personalstack.agents.application.idle.ConnectedClientTracker import com.jorisjonkers.personalstack.agents.application.idle.WorkspaceActivityTracker -import com.jorisjonkers.personalstack.agents.application.observability.AgentKindLabel import com.jorisjonkers.personalstack.agents.application.observability.AgentsApiTelemetry import com.jorisjonkers.personalstack.agents.application.observability.FailureReasonLabel import com.jorisjonkers.personalstack.agents.application.observability.ModeLabel @@ -113,7 +112,7 @@ class SessionAttachHandler( ) override fun afterConnectionEstablished(clientSession: WebSocketSession) { - val ready = preconditions.resolveAttach(clientSession, AttachPreconditionChecker::sessionIdOf) ?: return + val ready = preconditions.resolveAttach(clientSession) ?: return val upstreamHandler = UpstreamHandler( client = clientSession, diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/AgentRunnerObservabilityContractTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/AgentRunnerObservabilityContractTest.kt index 3123f92..ff12337 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/AgentRunnerObservabilityContractTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/AgentRunnerObservabilityContractTest.kt @@ -9,7 +9,8 @@ import java.nio.file.Path class AgentRunnerObservabilityContractTest { @Test fun `runner pod env pins gateway service identity and otlp transport`() { - // Pod env is built in RunnerPodSpecBuilder after the class split. + // Pod env is built by RunnerContainerEnvBuilder (in RunnerPodSpecBuilder.kt) + // after the class split. val podSpecBuilder = readProjectFile( "src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/" + @@ -35,11 +36,12 @@ class AgentRunnerObservabilityContractTest { ) assertThat(podSpecBuilder).contains( - ".withNewStartupProbe()", - ".withNewReadinessProbe()", - ".withNewLivenessProbe()", + ".withStartupProbe(httpProbe(STARTUP_PERIOD_SECONDS, STARTUP_FAILURE_THRESHOLD))", + ".withReadinessProbe(httpProbe(READINESS_PERIOD_SECONDS, READINESS_FAILURE_THRESHOLD))", + ".withLivenessProbe(httpProbe(LIVENESS_PERIOD_SECONDS))", ) - assertThat(Regex("""\.withPath\("/healthz"\)""").findAll(podSpecBuilder).count()).isEqualTo(3) + // All three probes share the single httpProbe(...) helper on /healthz. + assertThat(Regex("""\.withPath\("/healthz"\)""").findAll(podSpecBuilder).count()).isEqualTo(1) } @Test diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt index d757f1b..4e8ba85 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt @@ -6,10 +6,8 @@ import com.jorisjonkers.personalstack.agents.domain.model.RunnerState import io.fabric8.kubernetes.api.model.ContainerStatusBuilder import io.fabric8.kubernetes.api.model.ObjectMetaBuilder import io.fabric8.kubernetes.api.model.PodBuilder -import io.fabric8.kubernetes.api.model.PodSpec import io.fabric8.kubernetes.api.model.PodSpecBuilder import io.fabric8.kubernetes.api.model.PodStatus -import io.fabric8.kubernetes.api.model.ContainerBuilder import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt index 6b2e834..731e6f5 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/ws/AttachPreconditionCheckerTest.kt @@ -49,7 +49,7 @@ class AttachPreconditionCheckerTest { every { sessions.findById(sessionId) } returns agentSession(gatewayAgentId = "gw-1") every { workspaces.findById(workspaceId) } returns ws - val result = checker.resolveAttach(clientSession(), AttachPreconditionChecker::sessionIdOf) + val result = checker.resolveAttach(clientSession()) assertThat(result).isNotNull assertThat(result?.gatewayAgentId).isEqualTo("gw-1") @@ -63,7 +63,7 @@ class AttachPreconditionCheckerTest { val closed = slot() every { client.close(capture(closed)) } returns Unit - val result = checker.resolveAttach(client, AttachPreconditionChecker::sessionIdOf) + val result = checker.resolveAttach(client) assertThat(result).isNull() assertThat(closed.captured.code).isEqualTo(CloseStatus.BAD_DATA.code) @@ -76,7 +76,7 @@ class AttachPreconditionCheckerTest { val client = clientSession() every { client.close(any()) } returns Unit - val result = checker.resolveAttach(client, AttachPreconditionChecker::sessionIdOf) + val result = checker.resolveAttach(client) assertThat(result).isNull() assertThat(telemetry.attachFailures.single().reason).isEqualTo(FailureReasonLabel.NOT_FOUND) @@ -92,7 +92,7 @@ class AttachPreconditionCheckerTest { val client = clientSession() every { client.close(any()) } returns Unit - val result = checker.resolveAttach(client, AttachPreconditionChecker::sessionIdOf) + val result = checker.resolveAttach(client) assertThat(result).isNull() assertThat(telemetry.attachFailures.single().reason).isEqualTo(FailureReasonLabel.UPSTREAM_UNAVAILABLE) @@ -109,7 +109,7 @@ class AttachPreconditionCheckerTest { val client = clientSession() every { client.close(any()) } returns Unit - val result = checker.resolveAttach(client, AttachPreconditionChecker::sessionIdOf) + val result = checker.resolveAttach(client) assertThat(result).isNull() val closed = slot() @@ -138,7 +138,7 @@ class AttachPreconditionCheckerTest { provisioning = RunnerProvisioningResult.AlreadyReady, ) - val result = checker.resolveAttach(clientSession(), AttachPreconditionChecker::sessionIdOf) + val result = checker.resolveAttach(clientSession()) assertThat(result).isNotNull assertThat(result?.gatewayAgentId).isEqualTo("gw-fresh") From d240a713bce26cff7b2ce9be472cf7693c5850de Mon Sep 17 00:00:00 2001 From: JorisJonkers Agent Date: Mon, 13 Jul 2026 08:12:14 +0000 Subject: [PATCH 06/10] fix: split RunnerContainerEnvBuilder.podEnv below the LongMethod threshold Decompose env assembly into baseEnv (identity, OTLP transport, sandbox flag), setupEnv (MCP profile/files, GitHub MCP toolsets, runner setup identity), and repoEnv (boot-time clone URLs); podEnv becomes a short buildList coordinator. Env var order and content are unchanged. --- .../k8s/RunnerPodSpecBuilder.kt | 79 +++++++++++-------- 1 file changed, 47 insertions(+), 32 deletions(-) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt index 6e1196d..2428582 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerPodSpecBuilder.kt @@ -275,23 +275,38 @@ internal class RunnerContainerEnvBuilder( runnerGeneration: Long, credentialSecret: CredentialSecret?, ) = buildList { - add(EnvVarBuilder().withName("HOME").withValue("/home/agent").build()) - add(EnvVarBuilder().withName("CODEX_HOME").withValue("/home/agent/.codex").build()) - add(EnvVarBuilder().withName("DEPLOYMENT_ENVIRONMENT").withValue("production").build()) - add(EnvVarBuilder().withName("OTEL_SERVICE_NAME").withValue("agent-gateway").build()) - add( + addAll(baseEnv()) + addAll(setupEnv(setup, runnerGeneration)) + addAll(dockerEnv(setup)) + addAll(knowledgeEnv(setup)) + addAll(githubAppTokenEnv()) + addAll(agentCredentialEnv(credentialSecret)) + addAll(repoEnv(workspace)) + } + + private fun baseEnv() = + listOf( + EnvVarBuilder().withName("HOME").withValue("/home/agent").build(), + EnvVarBuilder().withName("CODEX_HOME").withValue("/home/agent/.codex").build(), + EnvVarBuilder().withName("DEPLOYMENT_ENVIRONMENT").withValue("production").build(), + EnvVarBuilder().withName("OTEL_SERVICE_NAME").withValue("agent-gateway").build(), EnvVarBuilder() .withName("OTEL_EXPORTER_OTLP_ENDPOINT") .withValue("http://alloy.observability.svc.cluster.local:4318") .build(), + EnvVarBuilder().withName("OTEL_EXPORTER_OTLP_PROTOCOL").withValue("http/protobuf").build(), + // The runner Pod is the outer sandbox for the agent process. + // Docker socket access is the explicit host-equivalent exception + // for Testcontainers and Docker CLI workflows. IS_SANDBOX tells + // Claude Code so that --dangerously-skip-permissions runs without + // the bypass-mode warning + acceptance prompt. + EnvVarBuilder().withName("IS_SANDBOX").withValue("1").build(), ) - add(EnvVarBuilder().withName("OTEL_EXPORTER_OTLP_PROTOCOL").withValue("http/protobuf").build()) - // The runner Pod is the outer sandbox for the agent process. - // Docker socket access is the explicit host-equivalent exception - // for Testcontainers and Docker CLI workflows. IS_SANDBOX tells - // Claude Code so that --dangerously-skip-permissions runs without - // the bypass-mode warning + acceptance prompt. - add(EnvVarBuilder().withName("IS_SANDBOX").withValue("1").build()) + + private fun setupEnv( + setup: RunnerSetupProvisioningSpec, + runnerGeneration: Long, + ) = buildList { add(EnvVarBuilder().withName("AGENT_MCP_PROFILE").withValue(setup.mcpProfile).build()) add(EnvVarBuilder().withName("AGENT_MCP_DIR").withValue(setup.mcpDir).build()) setup.claudeMcpServersFile?.let { @@ -320,28 +335,28 @@ internal class RunnerContainerEnvBuilder( .withValue(runnerGeneration.toString()) .build(), ) - addAll(dockerEnv(setup)) - addAll(knowledgeEnv(setup)) - addAll(githubAppTokenEnv()) - addAll(agentCredentialEnv(credentialSecret)) - // REPO_URL/REPO_BRANCH drive the entrypoint's boot-time clone - // into /workspace/. Cloning in the runner removes the race that - // left repo-backed workspaces empty: the old create-time - // gateway.clone fired before the runner gateway was up and was - // swallowed. Only repo-backed workspaces carry a repoUrl. - workspace.repoUrl?.let { url -> - add(EnvVarBuilder().withName("REPO_URL").withValue(url).build()) - workspace.branch?.let { add(EnvVarBuilder().withName("REPO_BRANCH").withValue(it).build()) } - } - // REPO_URLS carries the workspace's additional repos (everything - // attached that is not the primary), as url#branch entries. The - // entrypoint clones each into /workspace/ over the - // App-token credential helper. - additionalRepoUrls(workspace).takeIf { it.isNotEmpty() }?.let { urls -> - add(EnvVarBuilder().withName("REPO_URLS").withValue(urls.joinToString(" ")).build()) - } } + private fun repoEnv(workspace: Workspace) = + buildList { + // REPO_URL/REPO_BRANCH drive the entrypoint's boot-time clone + // into /workspace/. Cloning in the runner removes the race that + // left repo-backed workspaces empty: the old create-time + // gateway.clone fired before the runner gateway was up and was + // swallowed. Only repo-backed workspaces carry a repoUrl. + workspace.repoUrl?.let { url -> + add(EnvVarBuilder().withName("REPO_URL").withValue(url).build()) + workspace.branch?.let { add(EnvVarBuilder().withName("REPO_BRANCH").withValue(it).build()) } + } + // REPO_URLS carries the workspace's additional repos (everything + // attached that is not the primary), as url#branch entries. The + // entrypoint clones each into /workspace/ over the + // App-token credential helper. + additionalRepoUrls(workspace).takeIf { it.isNotEmpty() }?.let { urls -> + add(EnvVarBuilder().withName("REPO_URLS").withValue(urls.joinToString(" ")).build()) + } + } + private fun additionalRepoUrls(workspace: Workspace): List { val links = workspaceRepos.ifAvailable ?: return emptyList() val repos = repositories.ifAvailable ?: return emptyList() From 5f24f7eeb637a58c2b3d1f89158f2e92d48662b3 Mon Sep 17 00:00:00 2001 From: JorisJonkers Agent Date: Mon, 13 Jul 2026 08:20:16 +0000 Subject: [PATCH 07/10] fix: chained-call newline in RunnerBindingMetricsTest --- .../application/sessionbinding/RunnerBindingMetricsTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt index bf75c20..5b1f1ac 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt @@ -102,7 +102,8 @@ class RunnerBindingMetricsTest { private fun stubSession() = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession( - id = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId.random(), + id = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId + .random(), workspaceId = workspaceId, kind = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind.CLAUDE, gatewayAgentId = null, From 296566aeb35b612a51c41f2b5990b4848b3be843 Mon Sep 17 00:00:00 2001 From: JorisJonkers Agent Date: Mon, 13 Jul 2026 08:27:56 +0000 Subject: [PATCH 08/10] fix: drop unused version lambda from RunnerStateReader --- .../infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt | 2 +- .../agents/infrastructure/k8s/RunnerStateReader.kt | 4 +--- .../agents/infrastructure/k8s/RunnerStateReaderTest.kt | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt index 187ed16..f892d98 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/Fabric8AgentRunnerOrchestrator.kt @@ -47,7 +47,7 @@ class Fabric8AgentRunnerOrchestrator( private val log = LoggerFactory.getLogger(Fabric8AgentRunnerOrchestrator::class.java) private val credentials = RunnerCredentialSecretManager(client, props, credentialsProvider) private val podSpec = RunnerPodSpecBuilder(props, workspaceRepos, repositories, ::ownReleaseVersion) - private val stateReader = RunnerStateReader(::ownReleaseVersion) + private val stateReader = RunnerStateReader() override fun provision(workspace: Workspace): AgentRunnerOrchestrator.RunnerHandle = provision(workspace, legacySetupSpec(), workspace.runnerSetupGeneration) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReader.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReader.kt index 8cfce5f..1f7d7ce 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReader.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReader.kt @@ -10,9 +10,7 @@ import io.fabric8.kubernetes.api.model.Pod * Extracted from Fabric8AgentRunnerOrchestrator to keep that class below * the TooManyFunctions and LargeClass thresholds. */ -internal class RunnerStateReader( - private val ownReleaseVersion: () -> String?, -) { +internal class RunnerStateReader { fun runnerState(pod: Pod): RunnerState { val containerReady = pod.status diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt index 4e8ba85..53fadbd 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/infrastructure/k8s/RunnerStateReaderTest.kt @@ -12,7 +12,7 @@ import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test class RunnerStateReaderTest { - private val reader = RunnerStateReader { "v0.18.0" } + private val reader = RunnerStateReader() @Test fun `runnerState maps ready pod with all labels to domain model`() { From 9c2f60d0c8b9ae521e661a2f0434c471a756f50e Mon Sep 17 00:00:00 2001 From: JorisJonkers Agent Date: Mon, 13 Jul 2026 08:34:30 +0000 Subject: [PATCH 09/10] fix: fit bindingConflict body expression on the signature line --- .../agents/application/sessionbinding/RunnerBindingMetrics.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetrics.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetrics.kt index 6239d88..5f8440d 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetrics.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetrics.kt @@ -89,8 +89,7 @@ internal class RunnerBindingMetrics( ) } - fun bindingConflict(): Pair = - OutcomeLabel.FAILURE to FailureReasonLabel.CAPACITY + fun bindingConflict(): Pair = OutcomeLabel.FAILURE to FailureReasonLabel.CAPACITY fun reasonClass(ex: Throwable): FailureReasonLabel = when (ex) { From 21372c09332f4cb5f12d22f838e95d3df6e31ac9 Mon Sep 17 00:00:00 2001 From: JorisJonkers Agent Date: Mon, 13 Jul 2026 08:41:45 +0000 Subject: [PATCH 10/10] fix: start multiline id expression on a new line --- .../application/sessionbinding/RunnerBindingMetricsTest.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt index 5b1f1ac..5a477e4 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/agents/application/sessionbinding/RunnerBindingMetricsTest.kt @@ -102,8 +102,9 @@ class RunnerBindingMetricsTest { private fun stubSession() = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSession( - id = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId - .random(), + id = + com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentSessionId + .random(), workspaceId = workspaceId, kind = com.jorisjonkers.personalstack.agents.domain.model.WorkspaceAgentKind.CLAUDE, gatewayAgentId = null,