From 1cce757a34df2e2329ece0aaa63c036cfb0db37d Mon Sep 17 00:00:00 2001 From: agentfield-bot Date: Fri, 25 Sep 2026 10:54:14 -0400 Subject: [PATCH 1/2] session: the waiting sentence and its sign-in reason land together A connect offer published the waiting lane before the desk held the sign-in sentence, so a reader could see that a person is needed with an empty reason. The row is banked while the agent lock is held, and the lane is filled before that lock is released. --- docs/design/questions/DESIGN.md | 1 + internal/manual/chat/questions.md | 4 +- internal/session/connect.go | 55 +++++++++++++------ .../waiting_sentence_regression_test.go | 30 ++++++++-- 4 files changed, 68 insertions(+), 22 deletions(-) diff --git a/docs/design/questions/DESIGN.md b/docs/design/questions/DESIGN.md index 09837975a0..84ba7bbe68 100644 --- a/docs/design/questions/DESIGN.md +++ b/docs/design/questions/DESIGN.md @@ -212,6 +212,7 @@ conversation with the same answers row. - **SCREEN-READER AND NARROW.** Every form has a linear shape; compare stacks under 80 cols; the reader tier never draws a dial (a number input instead) and never paces a reveal. - **HEADLESS.** `--once`, `codeaf engine`, a task lane: the policy applies and is printed (`asked: โ†’ 1 (default ยท nobody to ask)`); a kind with no pick lands `your call` and pauses; nothing hangs. - **NO MACHINERY VOCABULARY.** Never "prompt", "modal", "dialog", "approval gate" on screen. +- **THE SIGN-IN SENTENCE LANDS WITH THE WAIT.** A connect offer publishes the lane and the line `connect your account?` before either can be read. A reader that sees the session waiting on that offer already has the sentence. Home, the tab and the page are that one read. ## The model's door โ€” `ask` (D5 = A) diff --git a/internal/manual/chat/questions.md b/internal/manual/chat/questions.md index abeb890f7e..64509eafdc 100644 --- a/internal/manual/chat/questions.md +++ b/internal/manual/chat/questions.md @@ -1290,7 +1290,9 @@ and `2 not now`. A service connected by a KEY has no `1`: a bare yes to one of those connects nothing, so the question asks for the key in the message box under it, masked to a bullet a character with the count beside it, and `enter` sends it. The one answer it keeps is `2 not now`, because a question the turn is waiting on -with no visible no is a question nobody can end. +with no visible no is a question nobody can end. The waiting mark and the +sentence arrive as one fact: the moment a sign-in needs you, the line is already +`connect your account?`, on this page, on home and on the tab. **So does the harness lane's pair.** An offer to run a saved program is one line โ€” `run harness "research"?` with `1 run it` and `2 not now`. A finished harness diff --git a/internal/session/connect.go b/internal/session/connect.go index 0c4f61c00d..63d493abf8 100644 --- a/internal/session/connect.go +++ b/internal/session/connect.go @@ -389,30 +389,48 @@ func (a *Agent) askConnect(ctx context.Context, service connectStatus) (connectA name: strings.TrimSpace(service.Name), secret: service.keyed(), } - if a.connectAsks == nil { - a.connectAsks = make(map[string]connectAsk, 1) - } - a.connectAsks[id] = ask - // The turn's hub, read under the same lock that registers the wait: a tool - // runs inside a turn, and the turn's fan-out is where its question is seen. + // The turn's hub, read under the same lock that mints the id: a tool runs + // inside a turn, and the turn's fan-out is where its question is seen. hub := a.hub watched := a.config.AskConsent && hub != nil a.mu.Unlock() if !watched { - a.forgetConnect(id) + // Nothing is registered. A lane with nobody watching is not a question, + // and a map entry that exists only to be deleted is a moment where a + // reader could see the wait with no sentence. return connectAnswer{}, errNobodyWatching } - // THE OFFER IS RAISED THROUGH THE ONE DOOR AND BANKED AT THE DESK, with the - // lane's own event as its announcement (taskpresence.go's - // [Agent.presenceAskingWhole], which is [Agent.raiseQuestion] plus the row - // that says what the question is). Before that this lane - // spoke only to the window holding the turn: the question existed on the - // questions lane solely as something [Agent.OpenQuestions] derived at - // subscription time, so a second window learned of it by replay and was - // never told it had been answered or withdrawn. - letGo := a.presenceAskingWhole(a.connectQuestion(id, ask), func() { + // THE SENTENCE LANDS BEFORE THE LANE IS VISIBLE. waitingOnPerson reads the + // lane under a.mu and the sentence under the desk's own lock, one after the + // other. Putting the ask on connectAsks and only then banking the row let a + // reader report that a person is needed with an empty reason: the lane was + // already true and the desk did not have the line yet. The row is banked + // while a.mu is still held, and the map is filled before that lock is + // released, so the unlock is the first moment either half can be seen. + // + // THE OFFER IS STILL RAISED THROUGH THE ONE DOOR, with the lane's own event + // as its announcement (question.go's [Agent.raiseQuestion]). The desk row + // is [Agent.presenceAskingQuestion], the same half [Agent.presenceAskingWhole] + // banks, taken first so it can share this lock. Before that this lane spoke + // only to the window holding the turn: the question existed on the questions + // lane solely as something [Agent.OpenQuestions] derived at subscription + // time, so a second window learned of it by replay and was never told it + // had been answered or withdrawn. + q := a.connectQuestion(id, ask) + a.mu.Lock() + if a.closed { + a.mu.Unlock() + return connectAnswer{}, errAgentClosed + } + forgetDesk := a.presenceAskingQuestion(q) + if a.connectAsks == nil { + a.connectAsks = make(map[string]connectAsk, 1) + } + a.connectAsks[id] = ask + a.mu.Unlock() + letGo := a.raiseQuestion(q, func() { hub.send(Event{ Kind: EventConnectAsk, ConnectID: id, @@ -421,7 +439,10 @@ func (a *Agent) askConnect(ctx context.Context, service connectStatus) (connectA NeedsKey: ask.needsKey, }) }) - defer letGo() + defer func() { + forgetDesk() + letGo() + }() timer := time.NewTimer(connectAskTimeout) defer timer.Stop() diff --git a/internal/session/waiting_sentence_regression_test.go b/internal/session/waiting_sentence_regression_test.go index 3812e9cae7..c6f52ee3fb 100644 --- a/internal/session/waiting_sentence_regression_test.go +++ b/internal/session/waiting_sentence_regression_test.go @@ -42,12 +42,34 @@ func TestWaitingSentenceConnectSignIn(t *testing.T) { }() ctx, cancel := context.WithCancel(context.Background()) defer cancel() + // THE FIRST OBSERVATION IS THE ONE THAT USED TO BE EMPTY. The lane and the + // sentence are published together, so the first read that sees a person is + // needed already carries the sign-in line. Waiting on the map and then + // sleeping let that read land in the gap. + bad := make(chan personAsk, 1) + saw := make(chan struct{}, 1) + go func() { + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + waiting := agent.waitingOnPerson() + if !waiting.waiting { + continue + } + if !strings.Contains(waiting.reason, "connect your Notion account?") { + bad <- waiting + return + } + saw <- struct{}{} + return + } + }() go func() { _, _ = agent.askConnect(ctx, connectStatus{ID: "notion", Name: "Notion"}) }() - waitForPersonLane(t, agent, connectLaneActive) - - waiting := agent.waitingOnPerson() - if !waiting.waiting || !strings.Contains(waiting.reason, "connect your Notion account?") { + select { + case waiting := <-bad: t.Fatalf("sign-in says a person is needed without the sign-in sentence: waiting=%v reason=%q", waiting.waiting, waiting.reason) + case <-saw: + case <-time.After(2 * time.Second): + t.Fatal("the question lane never became active") } } From de3204a79a0c5b19db3c1d94f18ca6a5c38f0038 Mon Sep 17 00:00:00 2001 From: agentfield-bot Date: Fri, 25 Sep 2026 11:18:57 -0400 Subject: [PATCH 2/2] changes: note for #1502 --- docs/changes/unreleased/1502-waiting-sentence-race.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 docs/changes/unreleased/1502-waiting-sentence-race.md diff --git a/docs/changes/unreleased/1502-waiting-sentence-race.md b/docs/changes/unreleased/1502-waiting-sentence-race.md new file mode 100644 index 0000000000..a11796f8a8 --- /dev/null +++ b/docs/changes/unreleased/1502-waiting-sentence-race.md @@ -0,0 +1,8 @@ +--- +kind: fixed +title: a connect offer shows its sign-in reason together with the waiting state +pr: 1502 +surface: [engine] +invalidates: + - "TestWaitingSentenceConnectSignIn was a known CI flake that went green on rerun. The cause was real: a reader could see a conversation waiting on the person with an empty reason. The reason is now stored before the waiting state is published, and the test no longer flakes." +---