Session-review implementation for the three sessions audited in
plans/2026-07-18-session-review-three-sessions.md. v0.7.11 → v0.7.12.
P0.1 — retry cap + investigate-before-retry (cmd/nomos/retrycap.go,
agent.go): after 3 identical failing run calls in a single turn, refuse
to dispatch the call again and return a directive to investigate *why*
(ps/strace/lsof) or surface the blocker. Per-turn scope so a fresh turn
after the operator responds can retry once more. Session 1e9c7691's 20+
identical chown retries (knfsd held a kernel lock on the exported NFS
dir) is the direct motivation.
P0.2 + P1.8 + P2.10 — SOUL.md guidance: hung command is not a failed
command (investigate before retry); ask before proposing a multi-step
migration; multi-goal sessions summarize the arc not just the last goal.
P1.3 — two new runbook entities in seeds/knowledge.yaml:
- nfs-exported-dir-mutation-hang (the knfsd fchownat lock procedure:
killall → exportfs -u → mutate → exportfs -a → verify)
- netbird-mgmt-oidc-race-after-upgrade (docker restart netbird-mgmt
after ~30s for the traefik/authentik OIDC race)
P1.4 — setGoal emits task.superseded event when prior goal is overwritten
by a different goal (store.go, TestSetGoal_SupersededEvent). Session
55927f0a had two set_goal calls with the first silently abandoned.
P1.5 — inspect_path MCP tool: runs mount/df/ls/stat for one path across
up to 8 targets in one parallel call, replacing the 15+ run-call
fact-gathering fan-out sessions 1 and 2 each spent on cross-target path
tracing (tools.go, server.go: inspectPathAcrossTargets, inspectOneTarget).
P1.6 — vm: target support in run via qm guest exec (no more SSH-hop
with nested quoting). Extracted shared resolveProxmoxHostSlug for
LXC + VM, with hosts-relationship fallback when attributes.host is
absent (server.go, tools.go). Session 55927f0a's SSH-hop workarounds
for vm:zimaos are the direct motivation.
Deferred (documented in plan): P1.7 (approval window auto-extend on
timeout) and P2.9 (long-running command PENDING detection) — both
addressed at lower cost by the retry cap. Session 3's poll-after-timeout
pattern already works; the cap protects against the failure mode.
130 lines
4.1 KiB
Go
130 lines
4.1 KiB
Go
package main
|
|
|
|
import (
|
|
"strings"
|
|
"testing"
|
|
)
|
|
|
|
func TestRunFailureKey_StableAcrossWhitespace(t *testing.T) {
|
|
cases := []struct{ a, b string }{
|
|
{"chown :10000 /mnt/media_local && chmod 2775 /mnt/media_local",
|
|
"chown :10000 /mnt/media_local && chmod 2775 /mnt/media_local"},
|
|
{"chown :10000 /mnt/media_local\n&& chmod 2775 /mnt/media_local",
|
|
"chown :10000 /mnt/media_local && chmod 2775 /mnt/media_local"},
|
|
{"chown :10000 /mnt/media_local && chmod 2775 /mnt/media_local ",
|
|
" chown :10000 /mnt/media_local && chmod 2775 /mnt/media_local"},
|
|
}
|
|
for i, c := range cases {
|
|
ka := runFailureKey("host:strong", c.a)
|
|
kb := runFailureKey("host:strong", c.b)
|
|
if ka != kb {
|
|
t.Errorf("case %d: keys differ for whitespace-equivalent commands:\n a=%q\n b=%q", i, c.a, c.b)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestRunFailureKey_DiffersByTarget(t *testing.T) {
|
|
a := runFailureKey("host:strong", "echo hi")
|
|
b := runFailureKey("host:hubris", "echo hi")
|
|
if a == b {
|
|
t.Error("keys should differ when target differs")
|
|
}
|
|
}
|
|
|
|
func TestRunFailureKey_DiffersByCommand(t *testing.T) {
|
|
a := runFailureKey("host:strong", "echo hi")
|
|
b := runFailureKey("host:strong", "echo bye")
|
|
if a == b {
|
|
t.Error("keys should differ when command differs")
|
|
}
|
|
}
|
|
|
|
func TestRunRetryTracker_CountsAndCaps(t *testing.T) {
|
|
r := newRunRetryTracker()
|
|
key := runFailureKey("host:strong", "chown :10000 /mnt/media_local")
|
|
for i := 1; i <= maxRunRetries; i++ {
|
|
if got := r.recordFailure(key); got != i {
|
|
t.Errorf("recordFailure #%d = %d, want %d", i, got, i)
|
|
}
|
|
}
|
|
// At the cap, failures() should report maxRunRetries, and the next
|
|
// identical call should be refused by the agent loop (failures() >=
|
|
// maxRunRetries).
|
|
if got := r.failures(key); got != maxRunRetries {
|
|
t.Errorf("failures = %d, want %d", got, maxRunRetries)
|
|
}
|
|
if r.failures(key) < maxRunRetries {
|
|
t.Errorf("cap should be enforced at maxRunRetries=%d", maxRunRetries)
|
|
}
|
|
}
|
|
|
|
func TestRunRetryTracker_PerTurnIsolation(t *testing.T) {
|
|
// Different keys don't interfere.
|
|
r := newRunRetryTracker()
|
|
k1 := runFailureKey("host:strong", "echo a")
|
|
k2 := runFailureKey("host:strong", "echo b")
|
|
r.recordFailure(k1)
|
|
r.recordFailure(k1)
|
|
if got := r.failures(k2); got != 0 {
|
|
t.Errorf("k2 failures = %d, want 0 (keys are isolated)", got)
|
|
}
|
|
}
|
|
|
|
func TestIsRunFailure(t *testing.T) {
|
|
cases := []struct {
|
|
desc string
|
|
tool string
|
|
result string
|
|
callErr error
|
|
want bool
|
|
}{
|
|
{"run with ERROR prefix", "run", "run on host:strong: ERROR ssh: signal: killed", nil, true},
|
|
{"run with exit error", "run", "run on lxc:caddy: ERROR exit status 1", nil, true},
|
|
{"run success (read-only auto)", "run", "run on host:strong (read_only, auto): hello", nil, false},
|
|
{"run success (assent window)", "run", "run on host:strong (config_mutation, auto via assent window): done", nil, false},
|
|
{"run queued for approval", "run", "run on host:strong requires approval (risk: config_mutation) — execution 019f4930 queued. Present the command and purpose to the operator and wait; do not re-request.", nil, false},
|
|
{"non-run tool", "get_entity", "lxc list result", nil, false},
|
|
{"callErr set (dispatch failure)", "run", "", errFake{}, true},
|
|
{"callErr set on non-run tool", "get_entity", "some result", errFake{}, true}, // callErr trumps name
|
|
}
|
|
for i, c := range cases {
|
|
got := isRunFailure(c.tool, c.result, c.callErr)
|
|
if got != c.want {
|
|
t.Errorf("case %d (%s): isRunFailure = %v, want %v", i, c.desc, got, c.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
type errFake struct{}
|
|
|
|
func (errFake) Error() string { return "fake dispatch error" }
|
|
|
|
func TestRunRetryDirective_Content(t *testing.T) {
|
|
d := runRetryDirective("host:strong", "chown :10000 /mnt/media_local", 3)
|
|
for _, want := range []string{
|
|
"Refused:",
|
|
"host:strong",
|
|
"3 times",
|
|
"retry cap hit",
|
|
"Do NOT retry",
|
|
"strace",
|
|
"ps aux",
|
|
"lsof",
|
|
"surface the blocker",
|
|
} {
|
|
if !strings.Contains(d, want) {
|
|
t.Errorf("directive missing %q; got:\n%s", want, d)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestItoa(t *testing.T) {
|
|
cases := map[int]string{0: "0", 1: "1", 9: "9", 10: "10", 42: "42",
|
|
100: "100", -1: "-1", -42: "-42"}
|
|
for in, want := range cases {
|
|
if got := itoa(in); got != want {
|
|
t.Errorf("itoa(%d) = %q, want %q", in, got, want)
|
|
}
|
|
}
|
|
}
|