diff --git a/docs/changes/unreleased/965-doorstep-drained-on-open.md b/docs/changes/unreleased/965-doorstep-drained-on-open.md new file mode 100644 index 0000000000..1aabd04585 --- /dev/null +++ b/docs/changes/unreleased/965-doorstep-drained-on-open.md @@ -0,0 +1,15 @@ +--- +kind: fixed +title: an answer left for a closed conversation is applied as it opens +pr: 965 +surface: [engine, chat] +invalidates: + - "An answer home left on another conversation's doorstep rode that session's presence heartbeat alone (`Agent.drainAnswers`), which is the right beat for a window answering one that is RUNNING. A window shut for the night keeps nothing beating, so the answer sat in `answers.jsonl` until somebody happened to open the conversation — and even then until the new process's first five-second tick. From home the row said `answered · waiting for it to pick that up` for as long as a person cared to look, which was home saying it had answered and home being wrong." + - "The constructor now drains the doorstep itself, after `recoverTasks` has put back the graph a landing answers to and before `startPresence` starts the heartbeat that would race it. The answer goes through the same one door as every other (`Agent.ResolveQuestion`), so a landing answered from home while its conversation was closed is already settled when the window opens — before its first turn — rather than a question somebody answered yesterday still asking." +--- + +The person-facing shape does not change: home still leaves the answer on the other +window's doorstep and still says `answered · waiting for it to pick that up` until the +owning conversation applies it. What changed is when that is. A closed conversation used +to have no clock at all; opening it is now the act that picks the answer up, so a landing +answered from home is settled the moment its conversation next runs. diff --git a/internal/manual/chat/home.md b/internal/manual/chat/home.md index fdede417ca..ef96b911dd 100644 --- a/internal/manual/chat/home.md +++ b/internal/manual/chat/home.md @@ -168,8 +168,11 @@ is waiting on it. **Not yet a question the model raised with `ask`**: that row s Home leaves the answer on the other window's doorstep, so the row says `answered · waiting for it to pick that up` until that window applies it — a second or -two. If it could not be left at all, it says -`could not leave that answer — open the conversation and answer it there`. +two while that window is running, or the moment it next opens when it was shut: a +conversation drains its doorstep as it opens, so an answer you left for a closed +conversation is settled the next time it runs, before its first turn. If it could not be +left at all, it says `could not leave that answer — open the conversation and answer it +there`. **The first answer wins.** When two windows answer one question, the second is told who answered and what they chose; it is never merged into a third answer nobody gave. @@ -206,8 +209,8 @@ Three ways to settle it, and they are the same door: `[d] let aforge decide this one`; - **home** — walk onto its row in `to check` and press `1` or `2`. The conversation applies it on its own beat; if it is not running, the answer waits on its doorstep and is applied - the next time that conversation runs, and the row says - `answered · waiting for it to pick that up` meanwhile; + the moment that conversation next opens — the constructor drains the doorstep before + anything else — and the row says `answered · waiting for it to pick that up` meanwhile; - **the task's room** — enter on its roster row opens it, and the same chips stand at the foot of the page; `a`, `n`, `s` and `d` over an empty box answer it with nothing selected; diff --git a/internal/session/agent.go b/internal/session/agent.go index 55c7e79694..db542a06d3 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -388,6 +388,22 @@ func newAgent(config Config, client Completer) (*Agent, error) { // nobody will see until tomorrow (standing_run.go's registry). Close erases // it. registerLiveSession(agent) + // AND WHAT WAS ANSWERED WHILE NOBODY WAS HOME IS ANSWERED NOW. An answer left + // on this conversation's doorstep rides the presence heartbeat + // (answers.go's [Agent.drainAnswers]), which is the right beat for one + // window answering another that is RUNNING. It is the wrong one for the + // conversation that was NOT: a window shut for the night keeps nothing + // beating, so the answer the person left on home — accept this landing, no + // to that card — sat in answers.jsonl until the next time anybody happened + // to open the conversation, and even then until the first tick of the new + // process. From home's side the row said `answered · waiting for it to pick + // that up` for as long as the person cared to look, which is home saying it + // answered and home being wrong. The graph the landing answers to is back + // above, and the heartbeat that would race this drain starts below, so the + // doorstep is emptied here: the answer is applied through the same one door + // as every other, and the conversation opens already settled rather than + // asking a question somebody answered yesterday. + agent.drainAnswers() // AND THE SESSION STARTS SAYING IT IS HERE. The index above is what work // came to; this is the claim that a PROCESS is alive right now, which no // file on disk could otherwise make (taskpresence.go). It is last of the diff --git a/internal/session/answers_test.go b/internal/session/answers_test.go index bd8ef9b87e..2422965787 100644 --- a/internal/session/answers_test.go +++ b/internal/session/answers_test.go @@ -549,3 +549,79 @@ func TestAModelsOwnQuestionTravelsInPresenceAndAnAnswerComesBack(t *testing.T) { } waitForNoQuestion(t, dir) } + +// AN ANSWER LEFT WHILE THE CONVERSATION WAS CLOSED IS APPLIED AS IT OPENS, not +// on the heartbeat of a process that did not exist. This is the defect home's +// `answered · waiting for it to pick that up` was named for: the landing's +// answer went onto the doorstep of a conversation with no process behind it, +// and the only drain was a heartbeat nothing was running — so the row said +// waiting forever. The conversation's own constructor drains the doorstep now, +// once the graph the answer applies to is back. +func TestAnAnswerLeftWhileTheWindowWasShutIsAppliedAsItOpens(t *testing.T) { + // THE GROUND IS REAL. The answer this test sends is a NO, which settles the + // landing where it stands without reaching for its working copy — an accept + // would go looking for the branch's tree, which a checkpoint restored onto a + // fresh process does not carry, and that is the settle road's own affair and + // not the door under test. + repo := newTestRepo(t) + dir := filepath.Join(t.TempDir(), "cccc1111cccc2222") + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatalf("session folder: %v", err) + } + // The conversation's whole graph, written the way a close left it: one + // landing nobody could check, waiting on the person, with its work on a + // branch of the ground. The constructor's recovery reads exactly this. + document := taskDocument{ + Type: taskDocumentType, Version: taskFileVersion, Seq: 1, + Nodes: []taskRecord{{ + ID: 1, Title: "Port the parser", Brief: "port it", Acceptance: "it parses", + State: TaskUnverified, Ground: repo, Branch: "task/parser", + }}, + } + raw, err := json.Marshal(document) + if err != nil { + t.Fatalf("checkpoint: %v", err) + } + if err := os.WriteFile(filepath.Join(dir, placeTasks), raw, 0o600); err != nil { + t.Fatalf("writing the checkpoint: %v", err) + } + // AND THE ANSWER IS ON THE DOORSTEP BEFORE ANY PROCESS IS. That is the whole + // shape: the person answered from home, home wrote the file, and the + // conversation was a folder on a disk with nothing running. + if err := WriteAnswer(dir, QuestionLanding, 1, LandingNoKey); err != nil { + t.Fatalf("leaving the answer: %v", err) + } + + // OPENING THE CONVERSATION IS THE ACT UNDER TEST. No surface, no first turn, + // no five-second wait — the constructor is where the answer lands, because + // that is the first moment a process exists that can apply it. + agent, err := newAgent(Config{ + Workspace: repo, + Model: "test/model", + System: "SYSTEM", + Place: Place{Dir: dir, Workspace: repo}, + SessionFile: filepath.Join(dir, placeTranscript), + }, &scriptedCompleter{}) + if err != nil { + t.Fatalf("opening the conversation: %v", err) + } + t.Cleanup(func() { _ = agent.Close() }) + + node := agent.taskNode(1) + if node == nil { + t.Fatal("the landing did not come back with the conversation") + } + // THE ANSWER WAS APPLIED, THROUGH THE ONE DOOR, BEFORE ANY HEARTBEAT COULD + // HAVE. TaskFailed is the no: the person said the work does not hold and the + // node was failed where it stood. Anything still unverified is the old + // defect standing up again — the answer still on the doorstep, the row still + // saying waiting. + if state := node.stateNow(); state != TaskFailed { + t.Fatalf("the landing is %q after the conversation opened, want it refused — the doorstep answer sat waiting for a process", state) + } + // AND THE DOORSTEP IS EMPTY, so a heartbeat that does arrive finds nothing + // to apply a second time. + if _, err := os.Stat(AnswersPath(dir)); !os.IsNotExist(err) { + t.Fatalf("the doorstep was left behind: %v", err) + } +}