diff --git a/modules/claude-code/README.md b/modules/claude-code/README.md index ef1edb75..3838be73 100644 --- a/modules/claude-code/README.md +++ b/modules/claude-code/README.md @@ -177,8 +177,10 @@ after expiry is never read as a choice. One call waits at most 60 s; the tool as most once a minute, and nothing enforces it. At most two questions wait at once, mesh-wide: each holds one of two slot keys in `questions`, taken by create or compare-and-set, so two nodes asking at once cannot both take the last, and a slot that cannot be read refuses the question. Questions alone never fill the router's three open asks, so one -is left for a proposed instruction section while two are open. A question the router holds no record of two minutes -after it was asked (its refusal not heard here) frees its slot and reads as ended. A refused question keeps none of +is left for a proposed instruction section while two are open. A question the router holds no record of, its refusal +not heard here, keeps its slot and reads as waiting until it expires: the router may still take it from its queue, +and no cancel can overtake it there (the queue offers work again after an error, and a cancel of an ask not yet +recorded is dropped); once expired, the router refuses it if taken late. A refused question keeps none of its words. An answer is the operator's word to the session and authorises nothing; a question never defaults. The home scope is registered through the tools **only where the home belongs to an agent account of its own** diff --git a/modules/claude-code/cmd/claude-code/questions.go b/modules/claude-code/cmd/claude-code/questions.go index 072d107f..e7c879f2 100644 --- a/modules/claude-code/cmd/claude-code/questions.go +++ b/modules/claude-code/cmd/claude-code/questions.go @@ -43,10 +43,11 @@ const QuestionPrefix = "q-" // create or by compare-and-set, so two nodes asking at once cannot both take the last one. const MostQuestions = asks.MostOpen - 1 -// RouterHoldsWithin is how long after a question is asked the router has recorded it: it records an ask before it -// takes its work. A question it holds no record of after that was refused (and the refusal not heard here) or never -// taken: it no longer holds a slot, and reads as ended unless a record of it appears after all. -var RouterHoldsWithin = 2 * time.Minute +// A question the router holds no record of, and whose refusal was not heard here, keeps its slot until it expires: +// the router may still have it in its queue (down, or backlogged), and neither the queue's order nor a cancel can +// take it back — the queue offers a piece of work again after an error, so a cancel may be taken before the ask it +// cancels, and a cancel of an ask not yet recorded is dropped. Once the question has expired the router refuses it +// when it is taken (asks.Ask.Check), so the slot is free and the question has ended. // slotKey is the key of one of the MostQuestions slots: not a question's key, so no question is ever read from it. func slotKey(n int) string { return fmt.Sprintf("slot-%d", n) } @@ -381,8 +382,8 @@ func labelsOf(q Question) string { return strings.Join(out, ", ") } -// questionOpen says whether a question still waits: kept, asked, not ended, not expired, and held by the router or -// asked less than RouterHoldsWithin ago. An unreadable record is open: a slot is never freed on a guess. +// questionOpen says whether a question still waits: kept, asked, not ended, not expired, and open in the router's +// record or not recorded yet. An unreadable record is open: a slot is never freed on a guess. func questionOpen(store QuestionStore, bus AskBus, id string, now time.Time) (bool, error) { q, ok, err := readQuestionErr(store, id) if err != nil { @@ -396,7 +397,7 @@ func questionOpen(store QuestionStore, bus AskBus, id string, now time.Time) (bo return true, err } if entry == nil { - return now.Sub(q.At) < RouterHoldsWithin, nil + return true, nil } var rec routerRecord if json.Unmarshal(entry.Value, &rec) != nil { @@ -559,9 +560,15 @@ func ReadAnswer(store QuestionStore, bus AskBus, key string, now time.Time) (map switch { case entry == nil && q.Ended != "": out["state"], out["words"] = AnswerEnded, q.Ended - case entry == nil && now.Sub(q.At) >= RouterHoldsWithin: - out["state"], out["words"] = AnswerEnded, "the router holds no record of it: refused (its refusal was not heard "+ - "here) or never taken; nothing waits for the operator's answer" + case entry == nil && !now.Before(q.Expires): + out["state"], out["words"] = AnswerEnded, "expired without the router recording it: refused (its refusal was not "+ + "heard here) or never taken; the router refuses it now if it is taken late" + out["now"] = "do what you said: " + q.OnExpiry + case entry == nil: + out["state"] = AnswerWaiting + out["words"] = "not yet recorded by the router: it may still be in the router's queue, or was refused without the " + + "refusal being heard here. It holds its slot, and the operator may still be asked, until it expires at " + + q.Expires.Format(time.RFC3339) case entry == nil, rec.State == "open", rec.Warrant == nil: out["state"] = AnswerWaiting if q.Ended != "" { diff --git a/modules/claude-code/cmd/claude-code/questions_test.go b/modules/claude-code/cmd/claude-code/questions_test.go index 5bde9b33..7bd5e7c0 100644 --- a/modules/claude-code/cmd/claude-code/questions_test.go +++ b/modules/claude-code/cmd/claude-code/questions_test.go @@ -314,6 +314,8 @@ type memQuestions struct { n uint64 // failGet fails every read of a key with this prefix. failGet string + // afterGet runs after a read, outside the lock: another node writing between a read and a write. + afterGet func(key string) } func newMemQuestions() *memQuestions { @@ -322,12 +324,17 @@ func newMemQuestions() *memQuestions { func (s *memQuestions) Get(key string) (json.RawMessage, uint64, bool, error) { s.mu.Lock() - defer s.mu.Unlock() if s.failGet != "" && strings.HasPrefix(key, s.failGet) { + s.mu.Unlock() return nil, 0, false, errors.New("the bus did not answer") } raw, ok := s.m[key] - return raw, s.rev[key], ok, nil + rev, after := s.rev[key], s.afterGet + s.mu.Unlock() + if after != nil { + after(key) + } + return raw, rev, ok, nil } func (s *memQuestions) put(key string, value any) error { raw, err := json.Marshal(value) @@ -431,22 +438,60 @@ func TestTheCapFailsClosed(t *testing.T) { } } -// A question whose refusal was not heard here, and of which the router holds no record, frees its slot once the -// router has had time to record it, and reads as ended — not open until it expires. -func TestAQuestionTheRouterNeverHeldFreesItsSlot(t *testing.T) { +// A question whose refusal was not heard here, and of which the router holds no record, keeps its slot and reads as +// waiting until it expires — the router may still take it from its queue — and only then frees its slot and ends. +func TestAQuestionTheRouterHasNotRecordedHoldsItsSlotUntilItExpires(t *testing.T) { p, _ := node(t, "g14") store, r := newMemQuestions(), newRouter() first := askMergeTonight(t, p, store, r, t0) askMergeTonight(t, p, store, r, t0) - delete(r.records, first) // refused, and the refusal never heard here - if _, err := AskQuestion(p, mergeTonight(), store, r, t0.Add(time.Minute)); err == nil { - t.Fatal("a slot was freed before the router had time to record the question") + delete(r.records, first) // the router down or backlogged, or its refusal never heard here + later := t0.Add(QuestionDefault - time.Minute) + if _, err := AskQuestion(p, mergeTonight(), store, r, later); err == nil { + t.Fatal("the slot of a question the router may still show was freed before it expired") } - if out, _ := ReadAnswer(store, r, first, t0.Add(RouterHoldsWithin)); out["state"] != AnswerEnded { - t.Errorf("read as %v", out) + out, _ := ReadAnswer(store, r, first, later) + if out["state"] != AnswerWaiting || !strings.Contains(out["words"].(string), "not yet recorded") { + t.Errorf("before expiry read as %v", out) } - if _, err := AskQuestion(p, mergeTonight(), store, r, t0.Add(RouterHoldsWithin)); err != nil { - t.Errorf("the slot of a question the router never held was not freed: %v", err) + expired := t0.Add(QuestionDefault) + if out, _ := ReadAnswer(store, r, first, expired); out["state"] != AnswerEnded || + out["now"] != "do what you said: I leave the pull request open" { + t.Errorf("at expiry read as %v", out) + } + if _, err := AskQuestion(p, mergeTonight(), store, r, expired); err != nil { + t.Errorf("the slot of an expired question was not freed: %v", err) + } +} + +// A slot is taken only at the revision read: when another node takes both slots between this node's read and its +// write, this node's writes are refused and the question is refused — a plain write would take a slot it lost. +func TestASlotIsTakenOnlyAtTheRevisionRead(t *testing.T) { + p, _ := node(t, "g14") + store, r := newMemQuestions(), newRouter() + // Two questions that ended: their slots are free to take again. + for _, id := range []string{askMergeTonight(t, p, store, r, t0), askMergeTonight(t, p, store, r, t0)} { + r.close(asks.Warrant{Ask: id, Outcome: asks.OutcomeExpired, At: t0.Add(time.Minute)}) + } + shanks, _ := node(t, "shanks") + other := newRouter() + store.afterGet = func(key string) { + if !strings.HasPrefix(key, "slot-") { + return + } + // Another node asks between this node's read of the slot and its write: its question waits. + q := Question{ID: newQuestionID(), Headline: "Another", Context: "c", Why: "w", OnExpiry: "e", By: "other", + From: shanks.Node, At: t0, Expires: t0.Add(time.Hour), + Options: []QuestionOption{{ID: "o1", Label: "A", Means: "a", Recommended: true}, {ID: "o2", Label: "B", Means: "b"}}} + _ = store.Put(q.ID, q) + _ = other.Publish(asks.AskSubject(Module), q.Ask(), q.ID) + r.records[q.ID] = other.records[q.ID] + _ = store.Put(key, slot{Question: q.ID, At: t0}) + } + _, err := AskQuestion(p, mergeTonight(), store, r, t0.Add(2*time.Minute)) + store.afterGet = nil + if err == nil || !strings.Contains(err.Error(), "questions already wait") { + t.Fatalf("a slot lost to another node was taken: %v", err) } }