Skip to content

Commit 4ff287a

Browse files
fix(server): re-establish the sandbox image before each run (#1478)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 242c793 commit 4ff287a

8 files changed

Lines changed: 80 additions & 6 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"hephaestus": patch
3+
---
4+
5+
Practice reviews recover on their own when the host reclaims the agent image. The image is only referenced while a review runs, so a host that prunes unused images removes it between reviews — and because it was fetched once at startup, every later review failed to start its container and retried into the same failure. The image is now re-established before each run.

server/src/main/java/de/tum/cit/aet/hephaestus/agent/sandbox/docker/DockerSandboxConfiguration.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import com.github.dockerjava.httpclient5.ApacheDockerHttpClient;
77
import de.tum.cit.aet.hephaestus.agent.job.AgentJobRepository;
88
import de.tum.cit.aet.hephaestus.agent.proxy.MentorProxyCredentialRegistry;
9+
import de.tum.cit.aet.hephaestus.agent.runtime.AgentImageProperties;
910
import de.tum.cit.aet.hephaestus.agent.sandbox.InteractiveSandboxProperties;
1011
import de.tum.cit.aet.hephaestus.agent.sandbox.SandboxProperties;
1112
import de.tum.cit.aet.hephaestus.agent.sandbox.docker.interactive.DockerInteractiveSandboxAdapter;
@@ -132,13 +133,31 @@ public ExecutorService dockerWaitExecutor(SandboxProperties properties) {
132133
);
133134
}
134135

136+
@Bean
137+
public SandboxImageGuard sandboxImageGuard(
138+
DockerClientOperations ops,
139+
AgentImageProperties agentImageProperties,
140+
MeterRegistry meterRegistry
141+
) {
142+
return image ->
143+
ImagePullBootstrapperSupport.applyPolicy(
144+
image,
145+
agentImageProperties.pullPolicy(),
146+
ops,
147+
"sandbox.image.pull",
148+
meterRegistry,
149+
log
150+
);
151+
}
152+
135153
@Bean
136154
public SandboxContainerManager sandboxContainerManager(
137155
DockerClientOperations ops,
156+
SandboxImageGuard imageGuard,
138157
SandboxProperties properties,
139158
ExecutorService dockerWaitExecutor
140159
) {
141-
return new SandboxContainerManager(ops, properties, dockerWaitExecutor);
160+
return new SandboxContainerManager(ops, imageGuard, properties, dockerWaitExecutor);
142161
}
143162

144163
@Bean

server/src/main/java/de/tum/cit/aet/hephaestus/agent/sandbox/docker/SandboxContainerManager.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public class SandboxContainerManager {
2626
private static final int SIGKILL_EXIT_CODE = 137;
2727

2828
private final DockerContainerOperations containerOps;
29+
private final SandboxImageGuard imageGuard;
2930
private final SandboxProperties properties;
3031

3132
/**
@@ -39,10 +40,12 @@ public class SandboxContainerManager {
3940

4041
public SandboxContainerManager(
4142
DockerContainerOperations containerOps,
43+
SandboxImageGuard imageGuard,
4244
SandboxProperties properties,
4345
ExecutorService dockerWaitExecutor
4446
) {
4547
this.containerOps = containerOps;
48+
this.imageGuard = imageGuard;
4649
this.properties = properties;
4750
this.dockerWaitExecutor = dockerWaitExecutor;
4851
}
@@ -53,6 +56,7 @@ public SandboxContainerManager(
5356
* @return the container ID
5457
*/
5558
public String createContainer(DockerOperations.ContainerSpec spec) {
59+
imageGuard.ensurePresent(spec.image());
5660
return containerOps.createContainer(spec);
5761
}
5862

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package de.tum.cit.aet.hephaestus.agent.sandbox.docker;
2+
3+
/**
4+
* Establishes that a sandbox image is on the daemon before a container is created from it.
5+
*
6+
* <p>The image is referenced only while a job runs, so a host that prunes unused images reclaims it
7+
* between jobs. Fetching it once at startup leaves the job's retry to fail identically, because
8+
* nothing re-pulls.
9+
*/
10+
@FunctionalInterface
11+
public interface SandboxImageGuard {
12+
void ensurePresent(String image);
13+
}

server/src/test/java/de/tum/cit/aet/hephaestus/agent/sandbox/docker/DockerSandboxLiveTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ void setUp() {
8080

8181
dockerOps = new DockerClientOperations(dockerClient);
8282
dockerWaitExecutor = Executors.newCachedThreadPool();
83-
containerManager = new SandboxContainerManager(dockerOps, properties, dockerWaitExecutor);
83+
containerManager = new SandboxContainerManager(dockerOps, image -> {}, properties, dockerWaitExecutor);
8484
networkManager = new SandboxNetworkManager(dockerOps, properties);
8585
workspaceManager = new SandboxWorkspaceManager(dockerOps);
8686
ContainerSecurityPolicy securityPolicy = new ContainerSecurityPolicy(properties, null);

server/src/test/java/de/tum/cit/aet/hephaestus/agent/sandbox/docker/RepositoryTreeStagingLiveTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ void setUp() {
100100
);
101101
DockerClientOperations dockerOps = new DockerClientOperations(dockerClient);
102102
dockerWaitExecutor = Executors.newCachedThreadPool();
103-
containerManager = new SandboxContainerManager(dockerOps, properties, dockerWaitExecutor);
103+
containerManager = new SandboxContainerManager(dockerOps, image -> {}, properties, dockerWaitExecutor);
104104
networkManager = new SandboxNetworkManager(dockerOps, properties);
105105
sandboxAdapter = new DockerSandboxAdapter(
106106
networkManager,

server/src/test/java/de/tum/cit/aet/hephaestus/agent/sandbox/docker/SandboxContainerManagerTest.java

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import de.tum.cit.aet.hephaestus.agent.sandbox.spi.SandboxException;
1313
import de.tum.cit.aet.hephaestus.testconfig.BaseUnitTest;
1414
import java.time.Duration;
15+
import java.util.ArrayList;
1516
import java.util.List;
1617
import java.util.Map;
1718
import java.util.concurrent.ExecutorService;
@@ -28,13 +29,14 @@ class SandboxContainerManagerTest extends BaseUnitTest {
2829
private DockerContainerOperations containerOps;
2930

3031
private SandboxContainerManager manager;
32+
private SandboxProperties properties;
3133
private ExecutorService executor;
3234

3335
private static final String CONTAINER_ID = "abc123";
3436

3537
@BeforeEach
3638
void setUp() {
37-
SandboxProperties properties = new SandboxProperties(
39+
properties = new SandboxProperties(
3840
"unix:///var/run/docker.sock",
3941
false,
4042
null,
@@ -49,14 +51,45 @@ void setUp() {
4951
null
5052
);
5153
executor = Executors.newSingleThreadExecutor();
52-
manager = new SandboxContainerManager(containerOps, properties, executor);
54+
manager = new SandboxContainerManager(containerOps, image -> {}, properties, executor);
5355
}
5456

5557
@AfterEach
5658
void tearDown() {
5759
executor.shutdownNow();
5860
}
5961

62+
@Nested
63+
class CreateContainer {
64+
65+
@Test
66+
void shouldEnsureImageIsPresentWhenCreatingContainer() {
67+
List<String> guarded = new ArrayList<>();
68+
SandboxContainerManager guardedManager = new SandboxContainerManager(
69+
containerOps,
70+
guarded::add,
71+
properties,
72+
executor
73+
);
74+
DockerOperations.ContainerSpec spec = new DockerOperations.ContainerSpec(
75+
"ghcr.io/example/agent:latest",
76+
List.of("true"),
77+
Map.of(),
78+
null,
79+
null,
80+
null,
81+
Map.of(),
82+
null,
83+
List.of()
84+
);
85+
when(containerOps.createContainer(spec)).thenReturn(CONTAINER_ID);
86+
87+
guardedManager.createContainer(spec);
88+
89+
assertThat(guarded).containsExactly("ghcr.io/example/agent:latest");
90+
}
91+
}
92+
6093
@Nested
6194
class WaitForCompletion {
6295

server/src/test/java/de/tum/cit/aet/hephaestus/agent/sandbox/docker/interactive/DockerInteractiveSandboxLiveTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ void setUp() throws Exception {
126126

127127
dockerOps = new DockerClientOperations(dockerClient);
128128
dockerWaitExecutor = Executors.newCachedThreadPool();
129-
containerManager = new SandboxContainerManager(dockerOps, sandboxProperties, dockerWaitExecutor);
129+
containerManager = new SandboxContainerManager(dockerOps, image -> {}, sandboxProperties, dockerWaitExecutor);
130130
networkManager = new SandboxNetworkManager(dockerOps, sandboxProperties);
131131
workspaceManager = new SandboxWorkspaceManager(dockerOps);
132132
securityPolicy = new ContainerSecurityPolicy(sandboxProperties, null);

0 commit comments

Comments
 (0)