chore(server): mentor capacity observability + dead-code cleanup

docs/contributor/agent/observability.mdx

@@ -0,0 +1,90 @@
+---
+id: agent-observability
+sidebar_position: 6
+title: Agent Observability
+description: Mentor and interactive-sandbox metrics — names, tags, intended alerts.
+---
+
+This page catalogues the Micrometer metrics emitted by the mentor pipeline and the interactive
+sandbox layer underneath it. Names are stable contracts with the dashboards — coordinate any
+rename through the on-call rotation before shipping.
+
+## Capacity & lifecycle
+
+### `interactive_sandbox.evicted_total{reason}`
+
+Counter, one increment per sandbox eviction.
+
+| Tag value | Meaning |
+| --- | --- |
+| `idle` | Session evicted because it crossed `hephaestus.mentor.idle-ttl-seconds`. |
+| `max_lifetime` | Session evicted because it crossed `hephaestus.mentor.max-lifetime-minutes` (absolute lifetime cap, default 60 min). |
+| `manual` | Explicit `close()` from the chat layer (e.g., poisoned sandbox after a `-32002`). |
+| `error` | Forced eviction due to a pump/writer fault. |
+| `natural_exit` | The runner subprocess exited cleanly. |
+| `daemon_unhealthy` | Docker daemon health check failed during reap. |
+
+`mentor.session.eviction{reason}` carries the same data with the legacy name retained for
+back-compat — both increment together. Alert on the SPI-aligned name.
+
+### `mentor.chat.outcome{outcome=CAPACITY_EXCEEDED}`
+
+Counter, increments when a turn is rejected because `InteractiveSandboxRegistry.tryRegister`
+returned `MAX_SESSIONS_PER_USER` or `MAX_SESSIONS_TOTAL`. **Distinguish from
+`outcome=ERROR` when alerting** — capacity rejections are policy-driven and expected under load
+(answer: add a replica or raise the cap); `ERROR` is a genuine failure that needs investigation.
+
+### `mentor.session.active`
+
+Gauge of currently-attached sandbox sessions on this replica. Useful as the denominator for
+capacity dashboards.
+
+## Frame pipeline
+
+### `interactive_sandbox.frame_ring.dropped_total{userId}`
+
+Per-user counter of ring-buffer overflows. Tagged by `userId` and **capped at 50 distinct
+users** by a `MeterFilter` (`MetricsCardinalityConfig`) — the 51st user's drops still hit the
+global counter but are not separately attributed.
+
+Per `(userId, sandboxId)` the increment is **debounced to at most once per second**. The buffer
+still drops every overflowed frame (no data loss in the *behaviour*; just the *metric* is
+rate-limited). Under sustained overflow you get a stable per-second signal instead of a flood.
+
+How to read it:
+
+- A spike on one user → that user's stream is producing tokens faster than the subscriber
+  can drain. Check their subscriber queue (`mentor.subscriber.dropped`) — if it's also high,
+  the SSE pipe is saturated.
+- Several users at once → the **ring is too small for production traffic**. Raise
+  `hephaestus.mentor.ring-buffer-frames` (default `512`) after confirming via this metric.
+  The default has **not yet been benchmarked against real Pi token-delta volumes** — it is the
+  starting point for the observation, not the final answer.
+
+### `mentor.ring.buffer.dropped`
+
+Global, untagged counter — always increments, regardless of cardinality cap or debounce.
+Useful for the gross "is the system dropping frames at all" question. The per-user counter
+above is for attribution; this one is for total volume.
+
+## Send / receive bytes
+
+`mentor.send.frame.bytes{direction=in|out}` — bytes pushed to runner stdin / received from
+runner stdout. A clean way to compute steady-state token throughput.
+
+`mentor.send.rejected{reason}` — `send()` rejections by reason (`queue_full`, `write_timeout`,
+`broken_pipe`, `closed`). `queue_full` rising = upstream is producing faster than the runner
+can absorb.
+
+## Tuning knobs (none auto-bumped in this PR)
+
+The defaults are conservative starting points:
+
+- `hephaestus.mentor.ring-buffer-frames=512` — observe `frame_ring.dropped_total` for a week
+  before deciding to raise; bumping it eats per-session heap proportionally.
+- `hephaestus.mentor.max-lifetime-minutes=60` — backstop against runners that accumulate
+  state or leak FDs over hours. Range `[5, 480]`. Too low evicts chatty users; too high lets
+  a leaky runner drink resources before idle eviction catches it.
+
+This page intentionally **does not** raise either default — the right number comes from the
+metrics, not from a guess.

server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/mentor/chat/MentorChatMetrics.java

@@ -39,6 +39,12 @@ public enum Outcome {
         IN_FLIGHT_CONFLICT_LOCAL("in_flight_conflict_local"),
         IN_FLIGHT_CONFLICT_DB("in_flight_conflict_db"),
         REJECTED("rejected"),
+        /**
+         * Sandbox registration was denied because a capacity cap fired — either per-user or
+         * per-replica. Distinct from {@link #ERROR} so capacity-driven alerts don't bury a
+         * genuine failure (and vice versa).
+         */
+        CAPACITY_EXCEEDED("capacity_exceeded"),
         ERROR("error");
 
         private final String tag;

server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/mentor/chat/MentorChatService.java

@@ -20,6 +20,7 @@
 import de.tum.in.www1.hephaestus.agent.mentor.chat.wire.TranslatorState;
 import de.tum.in.www1.hephaestus.agent.mentor.chat.wire.UIMessageChunk;
 import de.tum.in.www1.hephaestus.agent.sandbox.spi.AttachedSandbox;
+import de.tum.in.www1.hephaestus.agent.sandbox.spi.InteractiveSandboxCapacityExceededException;
 import de.tum.in.www1.hephaestus.agent.sandbox.spi.InteractiveSandboxService;
 import de.tum.in.www1.hephaestus.agent.sandbox.spi.InteractiveSandboxSpec;
 import de.tum.in.www1.hephaestus.gitprovider.user.User;
@@ -306,6 +307,20 @@ private MentorChatMetrics.Outcome runTurnInternal(
             persistence.interrupt(cookie, state, timeout);
             channel.completeWithError("Mentor turn timed out before completion.");
             outcome = MentorChatMetrics.Outcome.TIMEOUT;
+        } catch (InteractiveSandboxCapacityExceededException capacity) {
+            // Per-user or per-replica cap — surfaceable on the dashboard as a capacity signal,
+            // NOT an error. INFO (not WARN): hitting the cap is policy-driven, expected behaviour
+            // under load; alerts on `mentor.turn.completed{outcome=CAPACITY_EXCEEDED}` should split
+            // from `outcome=ERROR` so on-call doesn't get paged when the fleet just needs scaling.
+            log.info(
+                "Mentor turn rejected by sandbox capacity (threadId={}, scope={}): {}",
+                request.threadId(),
+                capacity.scope(),
+                capacity.getMessage()
+            );
+            persistence.interrupt(cookie, state, capacity);
+            channel.completeWithError(userFacingError(capacity));
+            outcome = MentorChatMetrics.Outcome.CAPACITY_EXCEEDED;
         } catch (ClientDisconnectedException disconnect) {
             // Browser closed mid-turn (tab close, refresh, network blip). This is NOT a turn
             // failure: the runner subscription keeps draining and `handleEvent` will still call
@@ -501,6 +516,11 @@ private static void verifyProtocol(JsonNode hello) {
      * Raw message stays in the WARN log for ops.
      */
     private static String userFacingError(Throwable e) {
+        if (e instanceof InteractiveSandboxCapacityExceededException capacity) {
+            return capacity.scope() == InteractiveSandboxCapacityExceededException.Scope.PER_USER
+                ? "You have too many mentor sessions open right now — close one and try again."
+                : "Mentor service is at capacity — please retry in a moment.";
+        }
         if (e instanceof MentorRunnerException) {
             return "Mentor service hit an unexpected error — please retry.";
         }

server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/sandbox/InteractiveSandboxProperties.java

@@ -25,6 +25,11 @@
  *     encoded character can be up to 4 bytes, so the on-wire memory ceiling is roughly 4× this).
  *     A longer line is treated as a stream-level fault and the session terminates {@code ERROR}.
  *     Without this bound a hostile runner could OOM the app-server.
+ * @param maxLifetimeMinutes absolute lifetime cap. A session attached this long ago is evicted
+ *     on the next reap tick regardless of recent activity — a backstop against runners that
+ *     accumulate state, drift on context, or leak file descriptors over time. Range {@code [5, 480]}
+ *     is a guard rail: too low and chatty users get evicted mid-conversation; too high and a
+ *     leaky runner can chew through the daemon's resources before idle eviction catches it.
  */
 @Validated
 @ConfigurationProperties(prefix = "hephaestus.mentor")
@@ -39,5 +44,6 @@ public record InteractiveSandboxProperties(
     @DefaultValue("30") @Min(1) int attachFirstFrameTimeoutSeconds,
     @DefaultValue("3") @Min(1) int maxSessionsPerUser,
     @DefaultValue("50") @Min(1) int maxSessionsTotal,
-    @DefaultValue("1048576") @Min(1024) int maxFrameChars
+    @DefaultValue("1048576") @Min(1024) int maxFrameChars,
+    @DefaultValue("60") @Min(5) @Max(480) int maxLifetimeMinutes
 ) {}

server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/interactive/DockerAttachedSandboxAdapter.java

@@ -29,7 +29,8 @@
  * Per-session adapter: owns the docker-exec subprocess, JSONL pump + writer, ring buffer, and
  * subscriber fan-out. State transitions {@code ATTACHED → CLOSING → CLOSED} are CAS-guarded.
  */
-public final class DockerAttachedSandboxAdapter implements AttachedSandbox, StdinWriteWatchdog.StallTarget {
+public final class DockerAttachedSandboxAdapter
+    implements AttachedSandbox, StdinWriteWatchdog.StallTarget, InteractiveSandboxRegistry.ReapTarget {
 
     private static final Logger log = LoggerFactory.getLogger(DockerAttachedSandboxAdapter.class);
 
@@ -246,8 +247,13 @@ public void onWriteTimeout() {
         process.destroyForcibly();
     }
 
-    /** Forced close with a specific reason and the configured default grace. Idempotent. */
-    void terminate(EvictionReason reason) {
+    /**
+     * Forced close with a specific reason and the configured default grace. Idempotent. Public
+     * by virtue of {@link InteractiveSandboxRegistry.ReapTarget}, but {@code DockerAttachedSandbox}
+     * is itself a package-private surface — only the registry can resolve it.
+     */
+    @Override
+    public void terminate(EvictionReason reason) {
         if (!state.compareAndSet(AttachedSandboxState.ATTACHED, AttachedSandboxState.CLOSING)) {
             return;
         }
@@ -301,6 +307,12 @@ Instant attachedAt() {
         return attachedAt;
     }
 
+    /** SPI exposure of {@link #attachedAt}: required by the registry's lifetime reaper. */
+    @Override
+    public Instant createdAt() {
+        return attachedAt;
+    }
+
     private void onFrame(JsonNode frame, int wireBytes) {
         lastActivityAt = Instant.now();
         if (!firstFrame.isDone()) {
@@ -393,6 +405,10 @@ private void runClose(Duration graceTimeout) {
             EvictionReason reason = terminalReason.get();
             if (reason == null) reason = EvictionReason.ERROR;
             metrics.evictionsBy(reason).increment();
+            metrics.interactiveEvictedBy(reason).increment();
+            // Free the per-(user,sandbox) debounce entry: the 50-userId cardinality cap bounds
+            // the absolute table size, but evicting per session keeps churn under control.
+            metrics.evictDropDebounce(userId, sessionId);
 
             state.set(AttachedSandboxState.CLOSED);
             try {

server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/interactive/DockerInteractiveSandboxAdapter.java

@@ -12,6 +12,7 @@
 import de.tum.in.www1.hephaestus.agent.sandbox.docker.SandboxWorkspaceManager;
 import de.tum.in.www1.hephaestus.agent.sandbox.spi.AttachedSandbox;
 import de.tum.in.www1.hephaestus.agent.sandbox.spi.EvictionReason;
+import de.tum.in.www1.hephaestus.agent.sandbox.spi.InteractiveSandboxCapacityExceededException;
 import de.tum.in.www1.hephaestus.agent.sandbox.spi.InteractiveSandboxException;
 import de.tum.in.www1.hephaestus.agent.sandbox.spi.InteractiveSandboxService;
 import de.tum.in.www1.hephaestus.agent.sandbox.spi.InteractiveSandboxSpec;
@@ -222,10 +223,15 @@ public AttachedSandbox attach(InteractiveSandboxSpec spec) {
                 }
                 case MAX_SESSIONS_PER_USER, MAX_SESSIONS_TOTAL -> {
                     metrics.attachFailureMaxSessions.increment();
-                    throw new InteractiveSandboxException(
-                        outcome == InteractiveSandboxRegistry.RegistrationOutcome.MAX_SESSIONS_PER_USER
-                            ? "Per-user session cap exceeded"
-                            : "Per-replica session cap exceeded"
+                    boolean perUser = outcome == InteractiveSandboxRegistry.RegistrationOutcome.MAX_SESSIONS_PER_USER;
+                    // Typed exception so the chat layer can route to the CAPACITY_EXCEEDED metric
+                    // outcome without string-matching on the message — alerts that split user-cap
+                    // from global-cap need the structured scope.
+                    throw new InteractiveSandboxCapacityExceededException(
+                        perUser
+                            ? InteractiveSandboxCapacityExceededException.Scope.PER_USER
+                            : InteractiveSandboxCapacityExceededException.Scope.GLOBAL,
+                        perUser ? "Per-user session cap exceeded" : "Per-replica session cap exceeded"
                     );
                 }
                 case REGISTERED -> registered = true;
@@ -288,7 +294,14 @@ private DockerAttachedSandboxAdapter buildSandbox(
         String networkId,
         PiProcessHandle process
     ) {
-        FrameRingBuffer ring = new FrameRingBuffer(properties.ringBufferFrames(), metrics.ringBufferDropped);
+        // Route every drop through the metrics facade so the per-user `frame_ring.dropped_total`
+        // counter sees the userId + sandboxId for debounce, while the global counter still ticks.
+        final java.util.UUID sandboxId = spec.sessionId();
+        final String userId = spec.userId();
+        FrameRingBuffer ring = new FrameRingBuffer(
+            properties.ringBufferFrames(),
+            () -> metrics.recordRingBufferDrop(userId, sandboxId)
+        );
         DockerAttachedSandboxAdapter.LifecycleOps lifecycleOps = new DockerAttachedSandboxAdapter.LifecycleOps() {
             @Override
             public void stopContainer(String cid, int graceSeconds) {

server/application-server/src/main/java/de/tum/in/www1/hephaestus/agent/sandbox/docker/interactive/FrameRingBuffer.java

@@ -1,37 +1,47 @@
 package de.tum.in.www1.hephaestus.agent.sandbox.docker.interactive;
 
 import com.fasterxml.jackson.databind.JsonNode;
-import io.micrometer.core.instrument.Counter;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 
 /**
  * Bounded ring buffer of frames with drop-oldest on overflow. Frames carry a monotonic sequence
  * number; {@link #snapshotSince} lets a subscriber resume without duplicates from a known cursor.
+ *
+ * <p>The {@code onDrop} callback fires exactly once per evicted frame. The buffer itself stays
+ * metric-agnostic — the callback can route to one or more counters (debounced or not) without
+ * pulling Micrometer into the buffer's surface. Callbacks must be cheap and non-blocking; they
+ * run while holding the buffer's monitor.
  */
 final class FrameRingBuffer {
 
     private final int capacity;
     private final ArrayDeque<Entry> entries;
-    private final Counter droppedCounter;
+    private final Runnable onDrop;
     private long nextSequence;
 
-    FrameRingBuffer(int capacity, Counter droppedCounter) {
+    FrameRingBuffer(int capacity, Runnable onDrop) {
         if (capacity <= 0) {
             throw new IllegalArgumentException("capacity must be positive, got: " + capacity);
         }
         this.capacity = capacity;
         this.entries = new ArrayDeque<>(capacity);
-        this.droppedCounter = droppedCounter;
+        this.onDrop = Objects.requireNonNull(onDrop, "onDrop");
         this.nextSequence = 0L;
     }
 
     synchronized long offer(JsonNode frame) {
         long seq = nextSequence++;
         if (entries.size() == capacity) {
             entries.removeFirst();
-            droppedCounter.increment();
+            try {
+                onDrop.run();
+            } catch (RuntimeException ignored) {
+                // A metric callback throwing must not corrupt the buffer's state. The observability
+                // path is best-effort; the buffer's correctness guarantees are unconditional.
+            }
         }
         entries.addLast(new Entry(seq, frame));
         return seq;