diff --git a/docs/demo.md b/docs/demo.md index 1c4ce9a..14c0dd1 100644 --- a/docs/demo.md +++ b/docs/demo.md @@ -132,8 +132,9 @@ role's restart policy. This exercises the health path with no agent and no auth. - `restarter` (restart_policy = always): restarts on every staleness, so it loops through `session.restarted` with a growing backoff. - `failstop` (restart_policy = never): the stale session is marked failed and - never restarted. The reconciler keeps the replica count by launching a fresh - replica, so `session.failed` recurs as each replacement in turn goes stale. + the role goes terminal. The reconciler freezes replacement spawns, so the + failed row and its pane stay visible for post-mortem. Recovery is + `marvel delete team` + re-apply, same as a saturated role. - `capped` (restart_policy = always, max_restarts = 1): restarts once, then hits its cap and emits `role.saturated` plus `session.failed`, and is not respawned. diff --git a/internal/otel/metrics.go b/internal/otel/metrics.go index b6e9e33..43718ba 100644 --- a/internal/otel/metrics.go +++ b/internal/otel/metrics.go @@ -22,7 +22,8 @@ func NewStdoutMeterProvider() (*sdkmetric.MeterProvider, error) { // NewContextGauge creates the marvel.agent.context_window_percent gauge. func NewContextGauge(meter metric.Meter) (metric.Float64Gauge, error) { - return meter.Float64Gauge("marvel.agent.context_window_percent", + return meter.Float64Gauge( + "marvel.agent.context_window_percent", metric.WithDescription("Agent context window usage as a percentage"), metric.WithUnit("%"), ) diff --git a/internal/otel/metrics_test.go b/internal/otel/metrics_test.go index 5db5dc1..c7edbcc 100644 --- a/internal/otel/metrics_test.go +++ b/internal/otel/metrics_test.go @@ -25,7 +25,8 @@ func TestNewStdoutMeterProvider(t *testing.T) { } // Recording should not panic. - gauge.Record(context.Background(), 42.5, + gauge.Record( + context.Background(), 42.5, metric.WithAttributes( attribute.String("workspace", "test"), attribute.String("team", "agents"), diff --git a/internal/team/controller.go b/internal/team/controller.go index 9c337c3..44be7d2 100644 --- a/internal/team/controller.go +++ b/internal/team/controller.go @@ -296,6 +296,30 @@ func (c *Controller) noteReapedCrash(r session.ReapedSession) { return } roleKey := r.Workspace + "/" + r.Team + "/" + r.Role + if role.RestartPolicy == api.RestartNever { + // The health path's never contract applies to the reap path + // too: a vacated pane under never goes terminal instead of + // being replaced after a backoff window. ReapDead already + // emitted session.crashed with the cause; session.failed here + // records the verdict. + c.freezeRole(r.Workspace, r.Team, r.Role) + _ = c.store.UpdateSession(r.Key, func(live *api.Session) error { + live.State = api.SessionFailed + return nil + }) + log.Printf("reap: session %s crashed (restart_policy=never), role %s frozen", + r.Key, roleKey) + events.Emit(c.Events, events.Event{ + Kind: events.KindSessionFailed, + Severity: events.SeverityWarning, + Workspace: r.Workspace, + Team: r.Team, + Role: r.Role, + Session: r.Key, + Message: "restart_policy=never, pane gone; role frozen", + }) + return + } if c.noteCrashAndBackoff(r.Workspace, r.Team, r.Role, role.MaxRestarts) { rh := c.roleHealth[roleKey] log.Printf("reap: session %s crashed (role %s restart #%d, next backoff=%s)", @@ -336,6 +360,21 @@ func (c *Controller) noteCrashAndBackoff(workspace, team, role string, maxRestar return true } +// freezeRole permanently blocks replacement spawns for a role by setting +// its BackoffUntil to the saturation sentinel. This is how +// restart_policy=never goes terminal: the first failure stops the role, +// so the reconciler must not repair the replica count with a fresh +// session every tick. The failed row and its pane stay visible for +// post-mortem. Recovery is the same as MaxRestarts saturation: delete +// the team and re-apply (ClearRoleHealthForTeam resets the freeze). +// See ArcavenAE/marvel#107, aae-orc-pyre. +func (c *Controller) freezeRole(workspace, team, role string) { + roleKey := workspace + "/" + team + "/" + role + rh := c.getRoleHealth(roleKey) + rh.BackoffUntil = saturationFreezeUntil + c.persistRoleHealth(roleKey, rh) +} + func (c *Controller) reconcileTeam(t *api.Team) { // Drain sessions whose role no longer exists in the manifest before // anything else. Manifest.Apply replaces live.Roles wholesale, so a @@ -610,7 +649,12 @@ func (c *Controller) applyRestartPolicy(sess *api.Session, t *api.Team, role *ap return nil }) sess.State = api.SessionFailed - log.Printf("health: session %s failed (restart_policy=never, failures=%d)", + // never means the role stops. Without the freeze, SessionFailed + // drops out of CountsAsAlive and the reconciler replaces the + // session every tick, uncapped and with no backoff — one live + // pane leaked per cycle (marvel#107, aae-orc-pyre). + c.freezeRole(t.Workspace, t.Name, role.Name) + log.Printf("health: session %s failed (restart_policy=never, failures=%d), role frozen", sess.Key(), sess.FailureCount) events.Emit(c.Events, events.Event{ Kind: events.KindSessionFailed, diff --git a/internal/team/controller_test.go b/internal/team/controller_test.go index 4d60953..264a36b 100644 --- a/internal/team/controller_test.go +++ b/internal/team/controller_test.go @@ -1283,6 +1283,103 @@ func TestSessionFailedEventOnRestartNever(t *testing.T) { } } +// TestRestartNeverFreezesRole covers aae-orc-pyre / marvel#107: a role +// with restart_policy=never whose session fails health must go terminal. +// Falsification: without the freezeRole call in applyRestartPolicy's +// RestartNever case, SessionFailed drops out of CountsAsAlive and the +// reconciler spawns a replacement on the next tick — the session count +// grows past 1 and this fails. +func TestRestartNeverFreezesRole(t *testing.T) { + skipIfNoTmux(t) + store, _, ctrl, cleanup := setup(t) + t.Cleanup(cleanup) + + createTeamFixture(t, store, "test-never-freeze", "squad", []api.Role{ + { + Name: "worker", Replicas: 1, + Runtime: api.Runtime{Name: "sleep", Command: "sleep", Args: []string{"300"}}, + RestartPolicy: api.RestartNever, + HealthCheck: &api.HealthCheck{Type: api.HealthCheckHeartbeat, Timeout: 1 * time.Millisecond, FailureThreshold: 1}, + }, + }) + + ctrl.ReconcileOnce() + sess := store.ListSessionsByTeamRole("test-never-freeze", "squad", "worker")[0] + if err := store.UpdateSession(sess.Key(), func(live *api.Session) error { + live.LastHeartbeat = time.Now().UTC().Add(-1 * time.Hour) + return nil + }); err != nil { + t.Fatalf("update heartbeat: %v", err) + } + + // The failing tick, then several repair opportunities. + for i := 0; i < 4; i++ { + ctrl.ReconcileOnce() + } + + got := store.ListSessionsByTeamRole("test-never-freeze", "squad", "worker") + if len(got) != 1 { + t.Fatalf("expected exactly 1 session after never failure (role terminal), got %d", len(got)) + } + if got[0].State != api.SessionFailed { + t.Fatalf("expected failed state, got %s", got[0].State) + } + rh, ok := ctrl.RoleHealthSnapshot("test-never-freeze", "squad", "worker") + if !ok { + t.Fatal("expected RoleHealth snapshot after never failure") + } + if !rh.BackoffUntil.After(time.Now().UTC().Add(100 * 365 * 24 * time.Hour)) { + t.Fatalf("expected far-future freeze, got BackoffUntil=%s", rh.BackoffUntil) + } +} + +// TestRestartNeverReapFreezesRole covers the reap-path half of +// aae-orc-pyre / marvel#107: a vacated pane under restart_policy=never +// must also go terminal. Falsification: without the RestartNever branch +// in noteReapedCrash, the crash gets ordinary backoff accounting and the +// reconciler replaces the session once the window elapses. +func TestRestartNeverReapFreezesRole(t *testing.T) { + skipIfNoTmux(t) + store, _, ctrl, cleanup := setup(t) + t.Cleanup(cleanup) + ring := events.NewRing(0) + ctrl.Events = ring + + clock := newTestClock(time.Date(2026, 4, 18, 0, 0, 0, 0, time.UTC)) + ctrl.now = clock.Now + + createTeamFixture(t, store, "test-never-reap", "squad", []api.Role{ + { + Name: "worker", Replicas: 1, + Runtime: api.Runtime{Name: "sleep", Command: "sleep", Args: []string{"300"}}, + RestartPolicy: api.RestartNever, + // No HealthCheck: isolates the reap path from the + // heartbeat-staleness path in evaluateHealth. + }, + }) + + ctrl.ReconcileOnce() + sess := store.ListSessionsByTeamRole("test-never-reap", "squad", "worker")[0] + + killPaneAndWait(t, sess.PaneID) + + ctrl.ReconcileOnce() // reap tick + clock.Advance(10 * time.Minute) // far past any ordinary backoff window + ctrl.ReconcileOnce() // repair opportunity — must refuse + + got := store.ListSessionsByTeamRole("test-never-reap", "squad", "worker") + if len(got) != 1 { + t.Fatalf("expected exactly 1 session after never reap (role terminal), got %d", len(got)) + } + if got[0].State != api.SessionFailed { + t.Fatalf("expected failed state after never reap, got %s", got[0].State) + } + failed := ring.Snapshot(events.Filter{Kind: events.KindSessionFailed, Session: sess.Key()}, 0) + if len(failed) == 0 { + t.Fatal("expected a session.failed event on the never reap path") + } +} + // TestSessionFailedEventOnSaturation covers aae-orc-96st: a role that // saturates MaxRestarts must emit events.KindSessionFailed in addition to // events.KindRoleSaturated. Falsification: without the emit in