Stop writing an empty refusal-counter file on every stop of every session (issue 1749)
mesh/merge-gate head e3a714d5f0f0 merged into trunk c3564c1accf3 — pass: builds claude-code → ace, g14, novox, shanks; no bus step; every machine composes …
mesh/repo-check head e3a714d5f0f0 merged into trunk c3564c1accf3 — pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-…
mesh/delivery delivered
mesh/merge-gate head e3a714d5f0f0 merged into trunk c3564c1accf3 — pass: builds claude-code → ace, g14, novox, shanks; no bus step; every machine composes …
mesh/repo-check head e3a714d5f0f0 merged into trunk c3564c1accf3 — pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-…
mesh/delivery delivered
writeRefusalCounters wrote "{}" and, since it now routes through fsys
(this branch's earlier commit), created the waits folder on every single
stop, even for a session that never had a wait: one counter file per
session id, piling up on a node that uses the wait list at all. It now
removes the file instead of writing empty counters, a no-op when there
was never one to begin with, so a stop with nothing to record leaves
neither the file nor the folder; a refusal still writes it, and the
stop right after the wait is done (or let through) removes it again.
This commit is contained in:
@@ -113,14 +113,26 @@ func readRefusalCounters(waitsDir, session string) map[string]int {
|
|||||||
return m
|
return m
|
||||||
}
|
}
|
||||||
|
|
||||||
// writeRefusalCounters writes the counter file beside the waits file, through fsys like every other write
|
// writeRefusalCounters keeps the counter file beside the waits file in step with m, through fsys like every
|
||||||
// into the agent's home (novox/hq ADR 0266, issue 1749): called from the Stop hook, which already runs as
|
// other write into the agent's home (novox/hq ADR 0266, issue 1749): called from the Stop hook, which
|
||||||
// the agent's own account (fsys is then its zero value, a plain write, as before), and from the tool's
|
// already runs as the agent's own account (fsys is then its zero value, a plain write or remove, as
|
||||||
// AdoptName, which runs as the operator's account and must reach a confined node's agent home through the
|
// before), and from the tool's AdoptName, which runs as the operator's account and must reach a confined
|
||||||
// sudo helper fsys carries such a write through. Its caller decides what an error here means: harmless when
|
// node's agent home through the sudo helper fsys carries such a write through. Its caller decides what an
|
||||||
// the stop is already allowed, but when the stop was about to be refused, a cap that cannot be written is a
|
// error here means: harmless when the stop is already allowed, but when the stop was about to be refused, a
|
||||||
// cap that cannot take effect (ADR 0489 §5 "never loops", review round 1 item 2).
|
// cap that cannot be written is a cap that cannot take effect (ADR 0489 §5 "never loops", review round 1
|
||||||
|
// item 2).
|
||||||
|
//
|
||||||
|
// Empty, m is removed rather than written as "{}" (issue 1749, follow-up): a stop with nothing to record —
|
||||||
|
// every stop of every session with no waits, the ordinary case — must leave neither the file nor, the first
|
||||||
|
// time, the waits folder itself; removing a file that was never there is not an error.
|
||||||
func writeRefusalCounters(waitsDir, session string, m map[string]int) error {
|
func writeRefusalCounters(waitsDir, session string, m map[string]int) error {
|
||||||
|
path := refusalCountersPath(waitsDir, session)
|
||||||
|
if len(m) == 0 {
|
||||||
|
if err := fsys.Remove(path); err != nil && !errors.Is(err, fs.ErrNotExist) {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
raw, err := json.Marshal(m)
|
raw, err := json.Marshal(m)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
@@ -128,7 +140,7 @@ func writeRefusalCounters(waitsDir, session string, m map[string]int) error {
|
|||||||
if err := fsys.MkdirAll(waitsDir, 0o700); err != nil {
|
if err := fsys.MkdirAll(waitsDir, 0o700); err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
return fsys.WriteFile(refusalCountersPath(waitsDir, session), raw, 0o600)
|
return fsys.WriteFile(path, raw, 0o600)
|
||||||
}
|
}
|
||||||
|
|
||||||
// notice says a fail-open or let-through reason on both the log (stderr) and the screen: Claude Code shows a
|
// notice says a fail-open or let-through reason on both the log (stderr) and the screen: Claude Code shows a
|
||||||
|
|||||||
@@ -170,13 +170,72 @@ func TestRunStopKeepsItsCounterBesideTheWaitsFile(t *testing.T) {
|
|||||||
if !strings.Contains(stdout.String(), "systemMessage") || !strings.Contains(stdout.String(), "unwatched") {
|
if !strings.Contains(stdout.String(), "systemMessage") || !strings.Contains(stdout.String(), "unwatched") {
|
||||||
t.Fatalf("the let-through notice did not reach stdout: %s", stdout.String())
|
t.Fatalf("the let-through notice did not reach stdout: %s", stdout.String())
|
||||||
}
|
}
|
||||||
raw, err := os.ReadFile(refusalCountersPath(waitsDir, "sess-6"))
|
// Once let through, the counters are empty again: the file is removed, not left holding "{}" (issue
|
||||||
|
// 1749, follow-up).
|
||||||
|
if _, err := os.Stat(refusalCountersPath(waitsDir, "sess-6")); !os.IsNotExist(err) {
|
||||||
|
t.Fatalf("the counter file was not removed once its counters emptied: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Issue 1749, follow-up: a stop for a session with no waits at all leaves nothing behind — not the counter
|
||||||
|
// file, not even the waits folder a node that never added a wait would otherwise gain on every stop of
|
||||||
|
// every session.
|
||||||
|
func TestStopWithNoWaitsLeavesNoFolderAndNoFile(t *testing.T) {
|
||||||
|
waitsDir := filepath.Join(t.TempDir(), "waits") // not created: this is what the hook is handed cold
|
||||||
|
var stdout, stderr bytes.Buffer
|
||||||
|
stdin := strings.NewReader(`{"session_id":"sess-none","stop_hook_active":false}`)
|
||||||
|
if code := runStop(waitsDir, stdin, &stdout, &stderr); code != 0 {
|
||||||
|
t.Fatalf("code %d, stderr %s", code, stderr.String())
|
||||||
|
}
|
||||||
|
if _, err := os.Stat(waitsDir); !os.IsNotExist(err) {
|
||||||
|
t.Fatalf("a stop with no waits created the waits folder: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Issue 1749, follow-up: a refusal writes the counter file (there is something to record).
|
||||||
|
func TestStopWithARefusalWritesTheCounterFile(t *testing.T) {
|
||||||
|
waitsDir := t.TempDir()
|
||||||
|
f := WaitsFile{Session: "sess-7", At: stopT0, Waits: []Wait{openWait("w-7", 10)}}
|
||||||
|
writeWaitsFixture(t, filepath.Join(waitsDir, "sess-7.json"), f)
|
||||||
|
var stdout, stderr bytes.Buffer
|
||||||
|
stdin := strings.NewReader(`{"session_id":"sess-7","stop_hook_active":false}`)
|
||||||
|
if code := runStop(waitsDir, stdin, &stdout, &stderr); code != 2 {
|
||||||
|
t.Fatalf("code %d, stderr %s", code, stderr.String())
|
||||||
|
}
|
||||||
|
raw, err := os.ReadFile(refusalCountersPath(waitsDir, "sess-7"))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
var counters map[string]int
|
var counters map[string]int
|
||||||
if json.Unmarshal(raw, &counters) != nil || len(counters) != 0 {
|
if json.Unmarshal(raw, &counters) != nil || counters["w-7"] != 1 {
|
||||||
t.Fatalf("the counter file was not reset: %s", raw)
|
t.Fatalf("the counter file: %s", raw)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Issue 1749, follow-up: once the wait is done (no longer in the waits file), the next stop removes the
|
||||||
|
// counter file rather than leaving a stale "{}" behind.
|
||||||
|
func TestStopAfterTheWaitIsDoneRemovesTheCounterFile(t *testing.T) {
|
||||||
|
waitsDir := t.TempDir()
|
||||||
|
path := filepath.Join(waitsDir, "sess-8.json")
|
||||||
|
writeWaitsFixture(t, path, WaitsFile{Session: "sess-8", At: stopT0, Waits: []Wait{openWait("w-8", 10)}})
|
||||||
|
stdin := strings.NewReader(`{"session_id":"sess-8","stop_hook_active":false}`)
|
||||||
|
var stdout, stderr bytes.Buffer
|
||||||
|
if code := runStop(waitsDir, stdin, &stdout, &stderr); code != 2 {
|
||||||
|
t.Fatalf("refusal: code %d, stderr %s", code, stderr.String())
|
||||||
|
}
|
||||||
|
if _, err := os.Stat(refusalCountersPath(waitsDir, "sess-8")); err != nil {
|
||||||
|
t.Fatalf("the counter file was not written: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// The wait is done: it is no longer in the waits file (DoneWait removes it, waits.go).
|
||||||
|
writeWaitsFixture(t, path, WaitsFile{Session: "sess-8", At: stopT0, Waits: []Wait{}})
|
||||||
|
var stdout2, stderr2 bytes.Buffer
|
||||||
|
stdin2 := strings.NewReader(`{"session_id":"sess-8","stop_hook_active":false}`)
|
||||||
|
if code := runStop(waitsDir, stdin2, &stdout2, &stderr2); code != 0 {
|
||||||
|
t.Fatalf("after done: code %d, stderr %s", code, stderr2.String())
|
||||||
|
}
|
||||||
|
if _, err := os.Stat(refusalCountersPath(waitsDir, "sess-8")); !os.IsNotExist(err) {
|
||||||
|
t.Fatalf("the counter file was not removed after the wait was done: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user